diff --git a/pkg/mcp/revision.go b/pkg/mcp/revision.go index 2f2e7e58f9..22dc9aebe6 100644 --- a/pkg/mcp/revision.go +++ b/pkg/mcp/revision.go @@ -116,11 +116,16 @@ var passthroughMetaKeys = map[string]struct{}{ } // modernSignalMetaKeys is the INGRESS/detection set consumed by hasModernSignal: -// the reserved keys a Legacy client never sets, whose presence — independent of -// whether the value is well-formed — is itself a claim of the Modern revision -// and must not be silently downgraded to Legacy. Only a malformed/absent -// protocolVersion alongside one of them turns into a rejection, never a -// downgrade. +// the reserved keys whose presence — independent of whether the value is +// well-formed — is a claim of the Modern revision when nothing else contradicts +// it, and must not then be silently downgraded to Legacy. +// +// "Nothing else contradicts it" is load-bearing. Real Legacy clients DO set +// these keys (the ChatGPT connector sets them while negotiating 2025-11-25), +// so an explicit non-Modern MCP-Protocol-Version header outranks them — see +// ClassifyRevision. The claim stands where the key is the only signal present, +// and there a malformed/absent protocolVersion alongside one of them turns into +// a rejection, never a downgrade. // // It is deliberately narrower than what StripReservedMeta removes: logLevel is // excluded so a request carrying only logLevel — which go-sdk's @@ -444,6 +449,25 @@ func ClassifyRevision(method string, meta map[string]any, protoHeader string) (R bodyVersion, hasBodyVersion := stringMetaValue(meta, metaKeyProtocolVersion) if !hasBodyVersion { + if protoHeader != "" && protoHeader != MCPVersionModern { + // An explicit non-Modern header is the client's negotiated + // declaration and outranks a stray reserved _meta key that carries + // no version of its own. Without this, merely including e.g. + // clientInfo flipped an otherwise-fine Legacy request from accepted + // to -32020 -- the same request with no reserved key at all is + // classified Legacy by hasModernSignal above, so rejecting this one + // was incoherent rather than strict. It broke every tools/call from + // the ChatGPT connector, which negotiates 2025-11-25 and sets + // reserved keys without a _meta protocolVersion (#6188). + // + // Modern enforcement is untouched: a Modern header with no _meta + // version still falls through to the mismatch below, and a reserved + // key with no header at all still yields MissingModernMetadata -- + // which is where "a reserved key is a claim of Modern that must not + // be silently downgraded" (see modernSignalMetaKeys) actually + // applies, because there the key is the only signal present. + return RevisionLegacy, nil + } if protoHeader != "" { return RevisionModern, &HeaderMismatchError{Header: protoHeader, Body: ""} } diff --git a/pkg/mcp/revision_test.go b/pkg/mcp/revision_test.go index a70c50839b..8d4b793f14 100644 --- a/pkg/mcp/revision_test.go +++ b/pkg/mcp/revision_test.go @@ -290,12 +290,34 @@ func TestClassifyRevision(t *testing.T) { }, }, { - name: "modern signal via reserved key with non-modern header is a header mismatch", + // #6188: the ChatGPT connector negotiates 2025-11-25 and sets + // reserved keys without a _meta protocolVersion. The explicit + // non-Modern header is its negotiated declaration and outranks the + // stray key; rejecting this broke every tools/call it sent. Note + // the same request WITHOUT the reserved key is classified Legacy by + // hasModernSignal, so rejecting it was incoherent, not strict. + name: "legacy: reserved key with an explicit non-modern header", method: "tools/call", meta: map[string]any{ metaKeyClientCapabilities: map[string]any{}, }, protoHeader: "2025-11-25", + expectedRev: RevisionLegacy, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, + { + // Modern enforcement is untouched: a Modern header with no _meta + // protocolVersion is still the malformed shape go-sdk sends for a + // removed RPC, and still -32020. + name: "modern header with reserved key but no body version is still a mismatch", + method: "tools/call", + meta: map[string]any{ + metaKeyClientCapabilities: map[string]any{}, + }, + protoHeader: MCPVersionModern, expectedRev: RevisionModern, checkErr: func(t *testing.T, err error) { t.Helper() @@ -303,10 +325,26 @@ func TestClassifyRevision(t *testing.T) { var mismatchErr *HeaderMismatchError require.ErrorAs(t, err, &mismatchErr) assert.Equal(t, CodeHeaderMismatch, mismatchErr.Code()) - assert.Equal(t, "2025-11-25", mismatchErr.Header) + assert.Equal(t, MCPVersionModern, mismatchErr.Header) assert.Empty(t, mismatchErr.Body) }, }, + { + // An unrecognised header is still non-Modern, so it is classified + // Legacy for the same reason. Version validity is the transport's + // job (isSupportedMCPVersion), not the classifier's. + name: "legacy: reserved key with an unrecognised header", + method: "tools/call", + meta: map[string]any{ + metaKeyClientInfo: map[string]any{"name": "openai-mcp"}, + }, + protoHeader: "1999-01-01", + expectedRev: RevisionLegacy, + checkErr: func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + }, + }, { name: "legacy: initialize with nil meta", method: "initialize",