mcp: derive Mcp-Protocol-Version header from the outgoing message (#1107)
`streamableClientConn.setMCPHeaders` sets `Mcp-Protocol-Version` only
from `c.initializedResult`, which is populated exclusively through the
unexported `clientConnection.sessionUpdated` hook. `Client.Connect`
reaches that hook via a type assertion on the `Connection` returned by
the `Transport`:
```go
if hc, ok := cs.mcpConn.(clientConnection); ok {
hc.sessionUpdated(cs.state)
}
```
That assertion silently fails whenever the `Connection` is wrapped by
another type that exposes only the exported `Connection` interface.
`c.initializedResult` then stays `nil` for the life of the session, and
every `SEP-2575` (>= `2026-07-28`) request sent through that connection
is missing the header. 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 via `injectRequestMeta`. `setMCPHeaders` now reads the protocol
version from the outgoing message first, falling back to
`c.initializedResult` and then the request context.
This is currently breaking the GitHub MCP with the 1.7-pre releases of
this module.
Fixes #1109
Co-authored-by: eu.anthropic.claude-sonnet-5 via Crush <charmcrush@users.noreply.github.com>
This commit is contained in:
parent
827f90ba0c
commit
2781b8f7e3
2 changed files with 97 additions and 7 deletions
|
|
@ -2249,7 +2249,7 @@ func (c *streamableClientConn) Write(ctx context.Context, msg jsonrpc.Message) e
|
|||
req.Header.Set("Content-Type", "application/json")
|
||||
req.Header.Set("Accept", "application/json, text/event-stream")
|
||||
|
||||
if err := c.setMCPHeaders(req); err != nil {
|
||||
if err := c.setMCPHeaders(req, msg); err != nil {
|
||||
// Failure to set headers means that the request was not sent.
|
||||
// Wrap with ErrRejected so the jsonrpc2 connection doesn't set writeErr
|
||||
// and permanently break the connection.
|
||||
|
|
@ -2373,7 +2373,7 @@ func (c *streamableClientConn) Write(ctx context.Context, msg jsonrpc.Message) e
|
|||
return nil
|
||||
}
|
||||
|
||||
func (c *streamableClientConn) setMCPHeaders(req *http.Request) error {
|
||||
func (c *streamableClientConn) setMCPHeaders(req *http.Request, msg jsonrpc.Message) error {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
|
||||
|
|
@ -2403,10 +2403,13 @@ func (c *streamableClientConn) setMCPHeaders(req *http.Request) error {
|
|||
}
|
||||
}
|
||||
}
|
||||
if c.initializedResult != nil {
|
||||
switch {
|
||||
case protocolVersionFromMessage(msg) != "":
|
||||
req.Header.Set(protocolVersionHeader, protocolVersionFromMessage(msg))
|
||||
case c.initializedResult != nil:
|
||||
req.Header.Set(protocolVersionHeader, c.initializedResult.ProtocolVersion)
|
||||
} else if v := protocolVersionFromContext(req.Context()); v != "" {
|
||||
req.Header.Set(protocolVersionHeader, v)
|
||||
case protocolVersionFromContext(req.Context()) != "":
|
||||
req.Header.Set(protocolVersionHeader, protocolVersionFromContext(req.Context()))
|
||||
}
|
||||
if c.sessionID != "" {
|
||||
req.Header.Set(sessionIDHeader, c.sessionID)
|
||||
|
|
@ -2415,6 +2418,23 @@ func (c *streamableClientConn) setMCPHeaders(req *http.Request) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
// protocolVersionFromMessage recovers the SEP-2575 `_meta.protocolVersion`
|
||||
// value from an outgoing JSON-RPC request, if present. It returns "" for
|
||||
// notifications, responses, requests without a `_meta.protocolVersion`, or a
|
||||
// nil msg.
|
||||
func protocolVersionFromMessage(msg jsonrpc.Message) string {
|
||||
req, ok := msg.(*jsonrpc.Request)
|
||||
if !ok {
|
||||
return ""
|
||||
}
|
||||
meta := extractRequestMeta(req.Params)
|
||||
if meta == nil {
|
||||
return ""
|
||||
}
|
||||
v, _ := meta[MetaKeyProtocolVersion].(string)
|
||||
return v
|
||||
}
|
||||
|
||||
func (c *streamableClientConn) handleJSON(requestSummary string, resp *http.Response) {
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
resp.Body.Close()
|
||||
|
|
@ -2676,7 +2696,7 @@ func (c *streamableClientConn) connectSSE(ctx context.Context, lastEventID strin
|
|||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := c.setMCPHeaders(req); err != nil {
|
||||
if err := c.setMCPHeaders(req, nil); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if lastEventID != "" {
|
||||
|
|
@ -2712,7 +2732,7 @@ func (c *streamableClientConn) Close() error {
|
|||
if err != nil {
|
||||
c.closeErr = err
|
||||
} else {
|
||||
if err := c.setMCPHeaders(req); err != nil {
|
||||
if err := c.setMCPHeaders(req, nil); err != nil {
|
||||
c.closeErr = err
|
||||
} else if resp, err := c.client.Do(req); err != nil {
|
||||
c.closeErr = err
|
||||
|
|
|
|||
|
|
@ -1364,6 +1364,76 @@ func TestStreamableClientConnect_DiscoverSuccess(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// TestStreamableClientConnSetMCPHeaders_ProtocolVersion covers
|
||||
// streamableClientConn.setMCPHeaders' selection of the Mcp-Protocol-Version
|
||||
// header value.
|
||||
//
|
||||
// Ordinarily initializedResult is populated by sessionUpdated, called
|
||||
// through a type assertion to the unexported clientConnection interface
|
||||
// (see Client.Connect). That assertion silently fails, leaving
|
||||
// initializedResult nil for the life of the session, whenever the
|
||||
// Connection returned by a Transport is wrapped by another type exposing
|
||||
// only the exported Connection interface (a real pattern for transports
|
||||
// that intercept traffic, e.g. to filter notifications): Go does not
|
||||
// promote unexported interface methods across an embedded interface
|
||||
// boundary. Every SEP-2575 (>= 2026-07-28) request already carries its own
|
||||
// `_meta.protocolVersion` field, so setMCPHeaders falls back to reading it
|
||||
// from the outgoing message when initializedResult is unset.
|
||||
func TestStreamableClientConnSetMCPHeaders_ProtocolVersion(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
initializedResult *InitializeResult
|
||||
msg jsonrpc.Message
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "message meta wins when initializedResult unset",
|
||||
initializedResult: nil,
|
||||
msg: req(1, methodListTools, &ListToolsParams{Meta: Meta{MetaKeyProtocolVersion: protocolVersion20260728}}),
|
||||
want: protocolVersion20260728,
|
||||
},
|
||||
{
|
||||
name: "initializedResult used when message has no meta",
|
||||
initializedResult: &InitializeResult{ProtocolVersion: protocolVersion20251125},
|
||||
msg: req(1, methodListTools, &ListToolsParams{}),
|
||||
want: protocolVersion20251125,
|
||||
},
|
||||
{
|
||||
name: "initializedResult used for nil message (GET/DELETE)",
|
||||
initializedResult: &InitializeResult{ProtocolVersion: protocolVersion20251125},
|
||||
msg: nil,
|
||||
want: protocolVersion20251125,
|
||||
},
|
||||
{
|
||||
name: "message meta preferred over stale initializedResult",
|
||||
initializedResult: &InitializeResult{ProtocolVersion: protocolVersion20251125},
|
||||
msg: req(1, methodListTools, &ListToolsParams{Meta: Meta{MetaKeyProtocolVersion: protocolVersion20260728}}),
|
||||
want: protocolVersion20260728,
|
||||
},
|
||||
{
|
||||
name: "no header when neither source is set",
|
||||
initializedResult: nil,
|
||||
msg: req(1, methodListTools, &ListToolsParams{}),
|
||||
want: "",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
conn := &streamableClientConn{initializedResult: tt.initializedResult}
|
||||
httpReq, err := http.NewRequest(http.MethodPost, "http://test.invalid", nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := conn.setMCPHeaders(httpReq, tt.msg); err != nil {
|
||||
t.Fatalf("setMCPHeaders: %v", err)
|
||||
}
|
||||
if got := httpReq.Header.Get(protocolVersionHeader); got != tt.want {
|
||||
t.Errorf("Mcp-Protocol-Version header = %q, want %q", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestStreamableClientConnect_DiscoverMethodNotFound verifies that Client.Connect
|
||||
// falls back to the legacy initialize handshake when the server responds to
|
||||
// server/discover with a JSON-RPC "Method not found" error.
|
||||
|
|
|
|||
Loading…
Reference in a new issue