mcp: derive Mcp-Protocol-Version header from the outgoing message#1107
Open
jan-xyz wants to merge 1 commit into
Open
Conversation
streamableClientConn only set the Mcp-Protocol-Version header from c.initializedResult, which is populated exclusively through the unexported clientConnection.sessionUpdated hook. That hook is invoked only via a type assertion on the Connection returned by a Transport (see Client.Connect), so it is silently skipped whenever that Connection is wrapped by another type exposing just the exported Connection interface: Go does not promote unexported interface methods across an embedded interface boundary. This is a common pattern for transports that need to intercept traffic, e.g. to filter notifications. c.initializedResult then stays nil for the life of the session. This silently breaks every SEP-2575 (>= 2026-07-28) request sent through such a wrapped connection, since servers that validate the header per spec reject the request with 400 Bad Request, closing the connection. Every SEP-2575 request already carries its own _meta.protocolVersion field. Reading the header value from there when a message is available is a more reliable source of truth than initializedResult and fixes the header regardless of how the Connection was constructed. Co-authored-by: eu.anthropic.claude-sonnet-5 via Crush <charmcrush@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
streamableClientConn.setMCPHeaderssetsMcp-Protocol-Versiononly fromc.initializedResult, which is populated exclusively through the unexportedclientConnection.sessionUpdatedhook.Client.Connectreaches that hook via a type assertion on theConnectionreturned by theTransport:That assertion silently fails whenever the
Connectionis wrapped by another type that exposes only the exportedConnectioninterface.c.initializedResultthen staysnilfor the life of the session, and everySEP-2575(>=2026-07-28) request sent through that connection is missing the header. Servers that validate the header per spec reject the request with400 Bad Request, closing the connection.What
Every SEP-2575 request already carries its own
_meta.protocolVersionfield viainjectRequestMeta.setMCPHeadersnow reads the protocol version from the outgoing message first, falling back to
c.initializedResultand then the request context.This is currently breaking the GitHub MCP with the 1.7-pre releases of this module.