mcp: prefer the negotiated protocol version over the request context (#1163)
setMCPHeaders resolved Mcp-Protocol-Version from the request context before c.initializedResult. #1107 established the opposite order and said so in its description; the code it replaced also consulted the context last. The two branches were transposed by the switch-to-if rewrite in #1115, whose subject was an unrelated nil-pointer check. The context branch serves the pre-negotiation server/discover probe, which sets its own value in Client.Connect. Once a version has been negotiated it is the authoritative one for that connection, so initializedResult should win. _meta.protocolVersion from the outgoing message stays on top, so #1109 stays fixed. The existing table built its request with http.NewRequest, so protocolVersionFromContext returned "" in every case and the two orderings were indistinguishable, which is why the transposition went unnoticed. Adds a ctxVersion dimension and three cases pinning the precedence. Fixes #1162 Co-authored-by: Guglielmo Colombo <guglielmoc@google.com>
This commit is contained in:
parent
1fcaf0c393
commit
b0408f2821
2 changed files with 30 additions and 2 deletions
|
|
@ -2446,10 +2446,10 @@ func (c *streamableClientConn) setMCPHeaders(req *http.Request, msg jsonrpc.Mess
|
|||
}
|
||||
if pv := protocolVersionFromMessage(msg); pv != "" {
|
||||
req.Header.Set(protocolVersionHeader, pv)
|
||||
} else if pv := protocolVersionFromContext(req.Context()); pv != "" {
|
||||
req.Header.Set(protocolVersionHeader, pv)
|
||||
} else if c.initializedResult != nil {
|
||||
req.Header.Set(protocolVersionHeader, c.initializedResult.ProtocolVersion)
|
||||
} else if pv := protocolVersionFromContext(req.Context()); pv != "" {
|
||||
req.Header.Set(protocolVersionHeader, pv)
|
||||
}
|
||||
if c.sessionID != "" {
|
||||
req.Header.Set(sessionIDHeader, c.sessionID)
|
||||
|
|
|
|||
|
|
@ -1384,6 +1384,7 @@ func TestStreamableClientConnSetMCPHeaders_ProtocolVersion(t *testing.T) {
|
|||
name string
|
||||
initializedResult *InitializeResult
|
||||
msg jsonrpc.Message
|
||||
ctxVersion string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
|
|
@ -1422,6 +1423,30 @@ func TestStreamableClientConnSetMCPHeaders_ProtocolVersion(t *testing.T) {
|
|||
msg: req(1, methodListTools, &ListToolsParams{}),
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
// A process that is both a server and a client (a proxy) carries the
|
||||
// inbound request's version on the context of the outgoing call. The
|
||||
// version this connection negotiated must win over it.
|
||||
name: "initializedResult preferred over request context",
|
||||
initializedResult: &InitializeResult{ProtocolVersion: protocolVersion20251125},
|
||||
msg: req(1, methodListTools, &ListToolsParams{}),
|
||||
ctxVersion: protocolVersion20260728,
|
||||
want: protocolVersion20251125,
|
||||
},
|
||||
{
|
||||
name: "request context used when initializedResult unset",
|
||||
initializedResult: nil,
|
||||
msg: req(1, methodListTools, &ListToolsParams{}),
|
||||
ctxVersion: protocolVersion20260728,
|
||||
want: protocolVersion20260728,
|
||||
},
|
||||
{
|
||||
name: "message meta preferred over request context",
|
||||
initializedResult: nil,
|
||||
msg: req(1, methodListTools, &ListToolsParams{Meta: Meta{MetaKeyProtocolVersion: protocolVersion20251125}}),
|
||||
ctxVersion: protocolVersion20260728,
|
||||
want: protocolVersion20251125,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
@ -1430,6 +1455,9 @@ func TestStreamableClientConnSetMCPHeaders_ProtocolVersion(t *testing.T) {
|
|||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if tt.ctxVersion != "" {
|
||||
httpReq = httpReq.WithContext(context.WithValue(httpReq.Context(), protocolVersionContextKey{}, tt.ctxVersion))
|
||||
}
|
||||
if err := conn.setMCPHeaders(httpReq, tt.msg); err != nil {
|
||||
t.Fatalf("setMCPHeaders: %v", err)
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue