Skip to content

Conversation

@bfineran
Copy link

@bfineran bfineran commented Jul 11, 2025

Adds a pre-set type field to SseServerParameters, StreamableHttpParameters, and StdioServerParameters so serialized parameters will easily be loaded into the correct class.

Motivation and Context

All fields of SseServerParameters overlap with StreamableHttpParameters, so a third party application that would want to connect to multiple session with a config such as:

class ClientConfig(BaseModel):
    mcp_configs: List[Union[StreamableHttpParameters, SseServerParameters]]

would not have a way to be instantiated from a serialized config (like a yaml file) since the following is ambiguous.

mcp_configs:
  - url: ""

Additionally, if someone were to serialize either the sse or http config classes they would not be able to reload the second class listed in the union.

Adding the preset type field should fix this in a backwards compatible way.

How Has This Been Tested?

Existing tests cover backwards compatibility, confirmed this PR addresses the described use case

Breaking Changes

No, added fields include the only valid Literal default.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@bfineran bfineran force-pushed the session-config-type-field branch from e455823 to 55cf14d Compare July 11, 2025 18:59
@bfineran bfineran force-pushed the session-config-type-field branch from 7119405 to cae5231 Compare July 11, 2025 19:21
@bfineran bfineran changed the title add type field to client session config BaseModels [fix] add type field to client session config BaseModels Jul 14, 2025
Copy link
Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary. You can handle the serialization logic in your application.

@felixweinberger
Copy link
Contributor

felixweinberger commented Sep 22, 2025

Hi @bfineran thank you for your contribution! And apologies for the time it took to get back tot his.

I don't think this is necessary. You can handle the serialization logic in your application.

Agree with this, this seems like a concern that should be handled at the application layer and not in the SDK - within the SDK we can leverage isinstance to compare the Pydantic Types of already instantiated objects.

If you're storing these configurations somehow and need to discriminate between different types, you could have a wrapper class that does this for you without having to modify the SDK types for it.

Something like this:

  class ServerConfig(BaseModel):
      type: Literal["stdio", "sse", "streamable_http"]
      config: dict[str, Any]

      def to_server_params(self) -> Union[StdioServerParameters, ...]:
          if self.type == "stdio":
              # do stuff
          elif self.type == "sse":
              # do stuff
          elif self.type == "streamable_http":
              # do stuff

@felixweinberger felixweinberger added invalid This doesn't seem right needs motivation When a PR is submitted without clear intent or motivation for the changes. labels Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

invalid This doesn't seem right needs motivation When a PR is submitted without clear intent or motivation for the changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants