feat(mcp-server): serve both protocol eras, adding modern 2026-07-28 - #4393
Open
gilgardosh wants to merge 2 commits into
Open
gilgardosh wants to merge 2 commits into
gilgardosh wants to merge 2 commits into
Conversation
The modern-era detector fired. Production logs show clients probing server/discover with 2026-07-28 and falling back to initialize within a second — five for five, two client families, three users. Nothing broke, but the spec is explicit that a modern-only client against a legacy-only server fails outright, and we were relying on clients continuing to offer a fallback they are under no obligation to keep. Era is chosen per request as the spec prescribes: modern per-request `_meta` gets modern semantics, anything else (initialize included) gets legacy. The modern path adds server/discover, `_meta` validation, header/body agreement (incl. the =?base64?..?= sentinel), resultType, _meta.serverInfo and cache hints on tools/list. The legacy path is byte-for-byte unchanged, which is the whole design: a dual-era client decides our era from the shape of our replies, so drift there would stop the fallback every current client depends on. dispatchMcpRequest is untouched; the branch is above it; both eras share dispatchToolMethods, extracted verbatim. A test diffs legacy responses against main — 46,310 bytes, identical. HTTP status is contract, not decoration: modern framing failures use 400/404 because a client reads the body of a 400 to detect a modern server. Also: DELETE /mcp -> 405, and an unexpected Origin is logged rather than rejected — we have never read the header and will enforce against evidence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gilgardosh
temporarily deployed
to
accounter-fullstack
September 7, 2026 07:01 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
gilgardosh
temporarily deployed
to
accounter-fullstack
September 7, 2026 07:01 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There’s a concrete runtime correctness issue in header handling (unsafe cast can mis-validate multi-value headers) and a documented -32021 modern framing failure path that is currently not implemented anywhere.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates @accounter/mcp-server to serve both MCP protocol eras on the same /mcp endpoint: legacy handshake-based requests continue to be handled byte-for-byte as before, while modern 2026-07-28 requests (identified by per-request _meta) are dispatched with modern semantics, including modern discovery and HTTP status behavior.
Changes:
- Add a modern-era dispatcher (
2026-07-28) with per-request_metaparsing/validation,server/discover, header/body agreement checks, and modern result envelopes. - Add a dual-era boundary (
dispatchMcpBodyDualEra) that selects legacy vs modern behavior per request while keeping legacy behavior stable. - Operational/doc updates:
DELETE /mcpreturns405, unexpectedOriginis logged (not enforced), and docs/changeset describe the dual-era behavior and observability.
File summaries
| File | Description |
|---|---|
| packages/mcp-server/src/server.ts | Adds DELETE /mcp handler returning 405 to mirror the existing GET /mcp behavior. |
| packages/mcp-server/src/mcp/modern.ts | Introduces the modern-era (2026-07-28) request classification, header validation, server/discover, and modern response shaping with HTTP statuses. |
| packages/mcp-server/src/mcp/jsonrpc.ts | Adds MCP-spec error code constants and helpers for modern framing errors (-32020, -32022). |
| packages/mcp-server/src/mcp/handler.ts | Adds dual-era body dispatch, threads request headers for modern validation, logs unexpected Origin, and sends era-specific HTTP status codes. |
| packages/mcp-server/src/mcp/tests/dual-era.test.ts | Adds tests for era selection, modern discovery shape, and modern framing failure status/error-code behavior. |
| packages/mcp-server/README.md | Documents dual-era semantics, including status-code differences and discover behavior. |
| packages/mcp-server/docs/operations-runbook.md | Adds operational guidance for modern-era served-call logs and status meanings. |
| packages/mcp-server/docs/connector-gaps-and-decisions.md | Records evidence/decision trail motivating the migration. |
| .changeset/mcp-dual-era-2026-07-28.md | Publishes the change as a patch with rationale and behavior summary. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
The latest changes of this PR are not available as |
Review follow-ups on the dual-era migration. **Repeated headers were a real bug.** `req.headers` values are `string | string[]` — Node merges a repeated header into an array — and the unchecked cast made a *present* header read as absent, so the modern path would reject a request for a missing header it had in fact been sent. A proxy duplicating a header is enough to trigger it. Now normalized at the boundary: a repeat with one distinct value is that value; conflicting values are joined so header/body comparison fails with both shown, which is correct — a request that says two different things is what this validation is for. **-32021 was defined but never emitted**, while the README, runbook and changeset all listed it as a modern framing failure. It is for a server that needs a client capability the client did not declare; this one serves only tools, which require none. Removed, with a note saying why, rather than left as a reserved code with no tested meaning behind it. (The `-32602` path for a missing required `_meta` field is a different rule and stays.) **Mcp-Name source is now per-method.** The comment said `params.name` while the code also accepted `params.uri`. The transport's table maps them differently — tools/call and prompts/get use `name`, resources/read uses `uri` — so a single set would validate the wrong field for one of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gilgardosh
temporarily deployed
to
accounter-fullstack
September 7, 2026 07:14 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
gilgardosh
temporarily deployed
to
accounter-fullstack
September 7, 2026 07:15 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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
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.
The detector from #4310 fired, so this is the migration it was watching for.
The evidence
Production logs, 2026-09-06/07 — five probes, five fallbacks, all within a second:
Two client families, three users. Nothing was broken — the fallback worked every time. But the
spec's compatibility matrix says a modern-only client against a legacy-only server fails
outright, and we were relying on clients continuing to offer a fallback they are under no
obligation to keep.
The logs also showed something nobody predicted: every handshake was already a version mismatch,
clients asking for
2025-11-25and getting2025-06-18. Closing that intermediate rung wasconsidered and rejected — cosmetic, costs nothing observable, and leaves the actual exposure
untouched. Reasoning recorded in
connector-gaps-and-decisions.md.The design, in one line
Two dispatchers, one boundary that chooses — which is what the spec literally prescribes:
The legacy path is byte-for-byte unchanged
This is the whole point, not a side benefit. A dual-era client decides which era a server speaks
from the shape of its replies — so a legacy answer that drifted even slightly would stop the
fallback every current client depends on today.
dispatchMcpRequestis untouched. The era branch lives at the boundary above it.dispatchToolMethods, extracted verbatim, so they cannot answer the same toolcall differently.
initialize/tools/list/pingresponses anddiffs them against the same responses built from
main. 46,310 bytes each, identical.HTTP status is contract, not decoration
Modern framing failures return
400(-32020header mismatch,-32021missing client capability,-32022unsupported version with thesupportedlist so a client can retry) and404for anunimplemented method.
That matters because a dual-era client reads the body of a
400to decide whether a server ismodern. Flattening those to
200— which is what the legacy path does with every error — would readas "not a modern server" and send the client straight back to the handshake. Half-migrating really is
worse than not migrating, and this is the specific mechanism.
Why it's smaller than "a transport rewrite"
Most of
2026-07-28needed no work here. It removed sessions,ping,logging/setLevel, SSEresumability and server-initiated requests, and added subscriptions and MRTR — none of which this
server implements. It declares one capability,
tools, and was already stateless (a comment inbusinesses.tshas said so since long before this). What remained: metadata validation, discovery,result shape.
server/discoveradvertises only modern revisions — listing the legacy one would invite a client to"choose" a version that has no per-request
_metato speak it with.Riding along
DELETE /mcpnow answers405, asGETalready did.Originis logged, not rejected. The spec says MUST reject with403, but thisserver has never read the header, so we do not know what our clients send — blind-enforcing a
403on a live connector is how you cause the outage you were preventing. Observe first, enforceagainst evidence. The same two-step that produced this migration.
Verification
912 tests pass (56 files), typecheck / eslint / prettier clean. New
dual-era.test.tscovers eraselection, discover shape, all four framing failures with their statuses, and
Mcp-Nameincludingthe Base64 sentinel.
🤖 Generated with Claude Code