-
Notifications
You must be signed in to change notification settings - Fork 463
fix(server): Implement MCP version negotiation. #341
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
Conversation
According to [the MCP spec](https://modelcontextprotocol.io/specification/2025-03-26/basic/lifecycle#version-negotiation): > If the server supports the requested protocol version, it MUST respond with > the same version. Otherwise, the server MUST respond with another protocol > version it supports. This adds `mcp.ValidProtocolVersions`, a slice with all currently specified MCP versions. In the `server` package we check if the client provided a known MCP version and, if so, return that version. Otherwise `LATEST_PROTOCOL_VERSION` is returned as previously.
WalkthroughA new exported variable listing valid MCP protocol versions was added. The server now negotiates protocol versions by checking if the client’s version is valid and responds accordingly. The initialization handler uses this negotiation logic. A test was introduced to verify protocol version negotiation behavior for various client requests. Changes
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
server/server_test.go (1)
1949-2024
: Excellent comprehensive test coverage for protocol version negotiation!The test function thoroughly validates the MCP protocol version negotiation behavior according to the specification requirements. The test cases cover all critical scenarios:
- ✅ Supported client version → server responds with client's version
- ✅ Latest version request → server responds with latest
- ✅ Unsupported future version → server responds with latest supported
- ✅ Unsupported old version → server responds with latest supported
The test structure is clean and follows Go testing best practices.
Consider adding a test case for edge scenarios to make the test even more robust:
+ { + name: "Client requests empty version - should respond with server's latest", + clientVersion: "", + expectedVersion: mcp.LATEST_PROTOCOL_VERSION, + },This would ensure the server handles malformed or missing protocol version requests gracefully.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mcp/types.go
(1 hunks)server/server.go
(3 hunks)server/server_test.go
(1 hunks)
🔇 Additional comments (4)
mcp/types.go (1)
101-105
:✅ Verification successful
LGTM! Clean implementation of centralized protocol version management.
The
ValidProtocolVersions
variable provides a clean, centralized way to manage supported MCP protocol versions. The inclusion of both the older "2024-11-05" version and the latest version demonstrates proper backward compatibility support.I'd like to verify that the server implementation correctly uses this variable for version negotiation. Could you run the following script to confirm the usage:
🏁 Script executed:
#!/bin/bash # Description: Verify that ValidProtocolVersions is used correctly in the server implementation # Expected: Find references to ValidProtocolVersions in server negotiation logic echo "Searching for ValidProtocolVersions usage in server code:" rg -A 5 -B 5 "ValidProtocolVersions" echo -e "\nSearching for protocol negotiation logic:" rg -A 10 "protocolVersion|ProtocolVersion" --type goLength of output: 29870
Confirmed ValidProtocolVersions is used correctly in negotiation logic
The server’s
protocolVersion
method inserver/server.go
referencesmcp.ValidProtocolVersions
viaslices.Contains
, returning the client’s version if it’s in the list, otherwise falling back toLATEST_PROTOCOL_VERSION
. Initialize handlers and existing tests cover both the legacy"2024-11-05"
version and the latest"2025-03-26"
:
- server/server.go
func (s *MCPServer) protocolVersion(clientVersion string) string { if slices.Contains(mcp.ValidProtocolVersions, clientVersion) { return clientVersion } return mcp.LATEST_PROTOCOL_VERSION }- Handlers assign
ProtocolVersion: s.protocolVersion(request.Params.ProtocolVersion),- Tests in
server/stdio_test.go
,server/sse_test.go
,server/streamable_http_test.go
, etc., assert correct negotiation for both supported and unsupported client versions.Everything looks solid—no further action needed.
server/server.go (3)
9-9
: LGTM: Import addition supports protocol negotiation.The addition of the
slices
import is necessary for theslices.Contains
function used in the new protocol version negotiation logic.
554-554
: LGTM: Proper protocol version negotiation implementation.The change from hardcoded
mcp.LATEST_PROTOCOL_VERSION
tos.protocolVersion(request.Params.ProtocolVersion)
correctly implements the MCP specification requirement for protocol version negotiation. The server now properly considers the client's requested version instead of always responding with the latest version.
574-580
: LGTM: Protocol negotiation logic correctly implements MCP specification.The
protocolVersion
method properly implements the MCP specification requirements:
- Returns the client's requested version if it exists in
mcp.ValidProtocolVersions
- Falls back to
mcp.LATEST_PROTOCOL_VERSION
if the client's version is not supported- Uses
slices.Contains
for efficient membership checking- Is thread-safe since it only reads immutable data
This implementation should resolve the compatibility issues with clients like Claude Desktop that expect proper version negotiation.
I missed reviewing this yesterday. Now it's already merged. Regardless, this looks good to me. But are there any breaking changes to the specification between the two versions that has caused breaking changes to the SDK? If that's the case, then we need to enable/disable things based on the client supported MCP spec version right? I'm not sure if it is the case but thought of mentioning it here. Maybe we can look at how other SDKs handle this. cc: @octo @robert-jackson-glean |
Description
Fix protocol version negotiation to comply with MCP specification
Problem
The current server implementation violates the MCP protocol specification for version negotiation. It always responds with
LATEST_PROTOCOL_VERSION
regardless of client capabilities, causing some clients (e.g. Claude Desktop) to disconnect.Root Cause
According to the MCP spec:
Our server was not implementing this negotiation correctly, leading to Claude Desktop to terminate the MCP server immediately.
Solution
Implement protocol version negotiation per MCP specification:
This fixes compatibility issues with clients like Claude Desktop that expect proper version negotiation.
Type of Change
Checklist
MCP Spec Compliance
Summary by CodeRabbit