-
Notifications
You must be signed in to change notification settings - Fork 319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce Initialization options that are passed to ServerSession #16
Conversation
mcp_python/server/session.py
Outdated
@dataclass | ||
class InitializationOptions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a Pydantic model here for consistency?
mcp_python/server/session.py
Outdated
class InitializationOptions: | ||
server_name: str | ||
server_version: str | ||
capabilities: ServerCapabilities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we infer this by default, instead of always requiring it to be passed in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rather be explicit about it for now. I'll keep it in mind however.
mcp_python/server/__init__.py
Outdated
@@ -276,13 +276,27 @@ async def handler(req: CompleteRequest): | |||
|
|||
return decorator | |||
|
|||
def get_capabilities(self) -> ServerCapabilities: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test for this?
mcp_python/server/__init__.py
Outdated
capabilities: dict[str, dict[str, Any] | None] = { | ||
"prompts": {} if ListPromptsRequest in self.request_handlers else None, | ||
"resources": {} if ListResourcesRequest in self.request_handlers else None, | ||
"tools": {} if ListPromptsRequest in self.request_handlers else None, | ||
"logging": {} if SetLevelRequest in self.request_handlers else None, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this code would be simpler if this is just passed in directly to the ServerCapabilities
constructor. I guess the point is to do the None
filtering, though—I wish we could figure out how to get Pydantic to do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I talked with Claude about this. There might be ways do this with pydantic, but we both prefer the more explicit approach. I added a helper function so people can pass it via app.create_initialization_option()
. If you feel strongly about it, i am happy to change it.
a1afc21
to
3b5b4a4
Compare
We need a way for servers to pass initialization options to the session. This is the beginning of this.
3b5b4a4
to
cc342a0
Compare
Depends on:
There is currently no way for server implementations to pass capabilities, server name and server version to the protocol. We now introduce an
InitializationOption
object that can be passed. Users of this are free to use the newget_capabilities
to fill capabilities based on the defined handlers. This will allow patterns such asThis is mostly an RFC. I am not sold on the specific approach but want to get something going so that we can correctly negotiate capabilities between our Zed implementation of the protocol and Python servers.