Skip to content

Conversation

@Kehrlann
Copy link
Contributor

The SDK does not support full version negotiation. However, for compatibility reasons, we want to make our client transports configurable so that users can set the range of "acceptable" spec versions

@Kehrlann Kehrlann force-pushed the dgarnier/configurable-supported-protocol-version branch from 86ae5fe to 1042cb9 Compare November 17, 2025 14:41
@Kehrlann Kehrlann requested a review from tzolov November 17, 2025 21:14
@Kehrlann Kehrlann marked this pull request as ready for review November 18, 2025 09:04
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
@Kehrlann Kehrlann force-pushed the dgarnier/configurable-supported-protocol-version branch from 2554994 to c293f09 Compare November 18, 2025 10:36
Copy link
Contributor

@tzolov tzolov left a comment

Choose a reason for hiding this comment

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

Nice job @Kehrlann . I like the test utils.
Left a comment about the name of the "working" protocolVersion used by the clients.
IMO latestSupportedProtocolVersion reflects only the initial state. After the re-initialization (once we implement this) the version can be downgraded to match server's perferences.


private final List<String> supportedProtocolVersions;

private final String latestSupportedProtocolVersion;
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should call this selectedProtocolVersion or negotiatedProtocolVersion.
After we implement the negotiation this value can change if the server request a lower version.

Copy link
Contributor Author

@Kehrlann Kehrlann Nov 18, 2025

Choose a reason for hiding this comment

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

The negotiated protocol version is not a property of the transport itself. I think it will be a property of the io.modelcontextprotocol.client.LifecycleInitializer.Initialization, and we'll pass it through the reactor context.

Copy link
Contributor

Choose a reason for hiding this comment

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

i guess you are right. Anyway will think about this when we start the work. Will mere the PR as it is

@tzolov tzolov merged commit 87bdf1e into main Nov 18, 2025
1 check passed
@tzolov tzolov deleted the dgarnier/configurable-supported-protocol-version branch November 18, 2025 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants