Let an explicit non-Modern header outrank a reserved key - #6231
Open
amirejaz wants to merge 1 commit into
Open
Conversation
ClassifyRevision rejected any non-initialize request that carried a reserved _meta signal key, no _meta protocolVersion, and a non-empty MCP-Protocol-Version header -- with -32020, before the backend was ever contacted. The ChatGPT connector negotiates 2025-11-25 and sets reserved keys without a _meta protocolVersion, so every tools/call it sent was rejected with a user-visible error and no operator escape (#6188). The rejection was incoherent rather than strict: the same request with no reserved key at all is classified Legacy by hasModernSignal, so merely including clientInfo flipped an otherwise-fine Legacy request from accepted to rejected. An explicit non-Modern header is the client's negotiated declaration and outranks a stray key that carries no version of its own. Modern enforcement is unchanged. A Modern header with no _meta version is still -32020 -- that is the malformed shape go-sdk sends for a removed RPC, pinned by TestIntegration_Modern_RealBackend_LoggingContract. 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" actually applies, because there the key is the only signal present. The modernSignalMetaKeys doc is updated to say so. Closes #6188 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
amirejaz
requested review from
ChrisJBurns,
JAORMX,
blkt,
jhrozek and
rdimitrov
as code owners
August 6, 2026 17:14
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.
Summary
ClassifyRevisionrejects any non-initializerequest that simultaneously carries a reserved_metasignal key, no_metaprotocolVersion, and a non-emptyMCP-Protocol-Versionheader — with-32020, before the backend is ever contacted.The ChatGPT connector negotiates
2025-11-25and sets reserved keys without a_metaprotocolVersion, so everytools/callit sends is rejected in about a millisecond, with the error surfaced to the end user and no operator escape:Reported and diagnosed in #6188, which includes a standalone reproduction against the exported classifier.
The rejection was incoherent rather than strict. The same request with no reserved key is classified Legacy by
hasModernSignal, so merely includingclientInfoflipped an otherwise-fine Legacy request from accepted to rejected:_meta2025-11-25{}2025-11-25{clientCapabilities}An explicit non-Modern header is the client's negotiated declaration and outranks a stray reserved key that carries no version of its own.
Type of change
Test plan
task test)task lint-fix)TestClassifyRevisiongains two Legacy cases (explicit non-Modern header, and an unrecognised header) and one that pins the unchanged Modern behaviour. Both new Legacy cases were verified to fail without the production change.One existing case is rewritten rather than added to:
"modern signal via reserved key with non-modern header is a header mismatch"pinned exactly the shape this fixes. It is now"legacy: reserved key with an explicit non-modern header"with the rationale inline.Full
task test: only the 10 pre-existingpkg/plugins/pluginsvcfailures, which reproduce identically on cleanorigin/main.Does this introduce a user-facing change?
Yes. MCP clients that negotiate a Legacy protocol version while including reserved
_metakeys — the ChatGPT connector among them — can call tools again. Previously everytools/callfrom such a client was rejected with a 400 before reaching the backend.Special notes for reviewers
This reverses a deliberate, documented decision, so it deserves scrutiny.
modernSignalMetaKeysstated that a reserved key's presence "is itself a claim of the Modern revision and must not be silently downgraded to Legacy." That principle is sound where the key is the only signal — and it still holds there: a reserved key with no header at all still yieldsMissingModernMetadata. What the principle did not anticipate is a real client that declares Legacy in the header and sets reserved keys. I have updated that doc comment rather than leaving it contradicting the code.Modern enforcement is untouched. A Modern header with no
_metaversion is still-32020. That is the malformed shape go-sdk sends for a removed RPC and is pinned byTestIntegration_Modern_RealBackend_LoggingContract; that contract stays green.Unrecognised headers now classify Legacy too (e.g.
1999-01-01), which is deliberate: version validity is the transport's job (isSupportedMCPVersion/strictProtocolValidation), not the classifier's, and the same header with no reserved key already classified Legacy.Relationship to #6184. Same
-32020symptom, different axis, and neither subsumes the other. #6184 fixed a caller sending a removed RPC on a Modern-negotiated session; this fixes ingress classification for a Legacy-negotiated client. Both regressed in v0.41.0 via #5834/#5884.Generated with Claude Code