Let an explicit non-Modern header outrank a reserved key

[amirejaz]

Summary

ClassifyRevision rejects any non-initialize request that simultaneously carries a reserved _meta signal key, no _meta protocolVersion, and a non-empty MCP-Protocol-Version header — with -32020, before the backend is ever contacted.

The ChatGPT connector negotiates 2025-11-25 and sets reserved keys without a _meta protocolVersion, so every tools/call it sends is rejected in about a millisecond, with the error surfaced to the end user and no operator escape:

MCP-Protocol-Version header "2025-11-25" does not match _meta protocol version ""

Reported and diagnosed in #6188, which includes a standalone reproduction against the exported classifier.

Closes #6188

The rejection was incoherent rather than strict. The same request with no reserved key is classified Legacy by hasModernSignal, so merely including clientInfo flipped an otherwise-fine Legacy request from accepted to rejected:

header	_meta	before	after
2025-11-25	{}	Legacy	Legacy
2025-11-25	{clientCapabilities}	-32020	Legacy

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

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• Linting (task lint-fix)

TestClassifyRevision gains 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-existing pkg/plugins/pluginsvc failures, which reproduce identically on clean origin/main.

Does this introduce a user-facing change?

Yes. MCP clients that negotiate a Legacy protocol version while including reserved _meta keys — the ChatGPT connector among them — can call tools again. Previously every tools/call from 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. modernSignalMetaKeys stated 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 yields MissingModernMetadata. 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 _meta version is still -32020. That is the malformed shape go-sdk sends for a removed RPC and is pinned by TestIntegration_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 -32020 symptom, 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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.72%. Comparing base (d163115) to head (6b33933).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6231   +/-   ##
=======================================
  Coverage   72.71%   72.72%           
=======================================
  Files         742      742           
  Lines       77299    77301    +2     
=======================================
+ Hits        56209    56214    +5     
+ Misses      17126    17124    -2     
+ Partials     3964     3963    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.