Skip to content

feat(mcp-server): detect clients speaking a newer protocol era - #4310

Open
gilgardosh wants to merge 2 commits into
mainfrom
feat/mcp-modern-era-detector
Open

feat(mcp-server): detect clients speaking a newer protocol era#4310
gilgardosh wants to merge 2 commits into
mainfrom
feat/mcp-modern-era-detector

Conversation

@gilgardosh

Copy link
Copy Markdown
Collaborator

Closes a blind spot in #4299 that I only spotted afterwards.

The gap

I said handshake logging would show us a client changing protocol era. It won't.

This connector implements a handshake-based revision. The current revision removed initialize
entirely — version, identity and capabilities travel per-request in _meta. A client that moved
there completely would simply stop handshaking, so mcp_initialize would go quiet rather than
report a changed version. The first real symptom would be failing calls.

That's the same shape as the incident that started all this, and worse: the spec's compatibility
matrix says a modern-only client against a legacy-only server fails outright, rather than
returning partial results the way August did.

Why it's detectable anyway

A dual-era client on Streamable HTTP tries a modern request first and falls back based on the
response. That attempt is the warning — it arrives before anything breaks, and nothing was looking
for it.

What this does

Any request carrying one of these emits a single event: "mcp_modern_probe" line at warn, with the
method, the client identity it advertised, and the revision it named:

Signal Why
MCP-Protocol-Version disagreeing with what we serve A header this server has never read — half of a real conformance gap in our own revision. We observe it; we still don't enforce it, because enforcement changes behavior
per-request _meta protocol version / clientInfo / capabilities The defining marker of a modern request
server/discover Modern-only method; calling it means the client thinks we might be modern

Quiet otherwise. MCP-Protocol-Version is required by the revision we already implement, so it
may well be on every call. Recording its presence would make this event mean "a request happened"
rather than "something changed" — and mcp_initialize already reports the negotiated version. So the
header is logged only when its value disagrees.

The load-bearing property

Observation only — no response byte changes. This is not incidental caution. Era detection keys
off exactly what a server returns: a dual-era client decides we are legacy from the shape of our
reply. Answering server/discover, or anything else that makes us look modern, would stop the
fallback that is currently keeping every client working — causing the exact failure this is meant to
warn about.

So server/discover still returns method-not-found, and there's a test for the invariant directly:

it('changes no response byte when a probe is detected', ...)
  expect(JSON.stringify(probed)).toBe(JSON.stringify(quiet));

describeModernEraProbe is pure and total in the same way as describeInitializeParams — everything
it reads is caller-supplied and unvalidated, and a probe this server cannot parse is precisely the
event worth seeing rather than throwing on. Covered with params as null, a string, an array, and
a _meta holding junk.

Verified it actually fires

A realistic dual-era probe:

method: "server/discover", protocolVersionHeader: "2026-07-28",
metaProtocolVersion: "2026-07-28", metaClientName: "claude-ai", modernMethod: true

A current-revision request with a matching header: null — silent.

Docs

Runbook §3.2 covers the fields, jq recipes, and — since the whole point is to be a trigger — what
to do when it fires: the dual-era migration stops being hypothetical, and the hazard to avoid is a
partial one. Renumbered tool-call logs to §3.3 and updated the README cross-reference.

What this is not

It catches dual-era probes, not a client that goes straight to modern-only with no fallback. That
residual case is what the migration is for; this buys the warning, not the cure.

Verification

892 tests pass (55 files), typecheck / eslint / prettier clean.

🤖 Generated with Claude Code

This server implements a handshake-based revision. The current revision
removed `initialize` entirely, so a client that moved there would stop
handshaking — meaning the handshake logging added earlier cannot warn us:
mcp_initialize would go quiet rather than report a changed version, and the
first symptom would be failing calls.

A dual-era client tries a modern request first and falls back on the
response. That attempt is the available warning, and nothing was looking for
it. Requests carrying a disagreeing MCP-Protocol-Version header, a
per-request `_meta` protocol version/identity, or the modern-only
server/discover method now emit one `mcp_modern_probe` line at warn.

Quiet otherwise: the header is also required by our own revision, so logging
its presence would make the event mean "a request happened".

Observation only, and that is load-bearing — era detection keys off what we
return, so answering server/discover would stop the fallback currently
keeping clients working. A test asserts responses are byte-identical whether
or not a probe was detected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gilgardosh
gilgardosh deployed to accounter-fullstack August 27, 2026 14:42 — with GitHub Actions Active
@gilgardosh
gilgardosh deployed to accounter-fullstack August 27, 2026 14:43 — with GitHub Actions Active
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@accounter/client 0.1.0-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎
@accounter/green-invoice-graphql 0.8.7-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎
@accounter/hashavshevet-mesh 0.2.13-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎
@accounter/israeli-vat-scraper 0.1.13-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎
@accounter/modern-poalim-scraper 0.11.0-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎
@accounter/payper-mesh 0.2.13-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎
@accounter/scraper-app 0.0.3-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎
@accounter/server 0.2.0-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎
@accounter/shaam-uniform-format-generator 0.2.7-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎
@accounter/shaam6111-generator 0.1.9-alpha-20260827145748-55ab2c103b80da8a69c252fc2c9473c5e53404a3 npm ↗︎ unpkg ↗︎

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new warn log payload can still include unbounded caller-supplied capability key lists, and the runbook field reference should be aligned with the actual emitted fields.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds “modern-era probe” observation to the MCP server so operators get an early warning when a dual-era client attempts a modern-protocol request against this legacy/handshake-based connector (without changing any response bytes, preserving fallback behavior).

Changes:

  • Detect modern-era signals (MCP-Protocol-Version mismatch, modern _meta, server/discover) and emit a single event: "mcp_modern_probe" warn log line per detected request.
  • Add unit tests asserting probe detection is observation-only (byte-identical responses) and robust to junk _meta.
  • Update MCP server docs/runbook and add a changeset entry for the patch release.
File summaries
File Description
packages/mcp-server/src/mcp/handler.ts Adds modern-era probe detection + structured warn logging; threads protocol version header from HTTP boundary into dispatch context.
packages/mcp-server/src/mcp/tests/handler.test.ts Adds tests for probe detection, method-not-found invariant for server/discover, and byte-identical response behavior.
packages/mcp-server/README.md Documents the new probe detection signal and links to updated runbook sections.
packages/mcp-server/docs/operations-runbook.md Adds runbook section describing mcp_modern_probe fields and operational response.
.changeset/mcp-modern-era-detector.md Declares a patch release and summarizes behavior/constraints of probe detection.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +143 to +145
| `metaClientCapabilities` | Capability names only, sorted |
| `modernMethod` | The method itself exists only in the modern protocol |
| `servedEra` | Always `legacy` — what we actually answered, which this event never changes |

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — the row is in the table now.

Worth having: protocolVersionHeader and metaProtocolVersion are only meaningful against what we actually serve, and having both on the same line means a reader can see the mismatch without knowing MCP_PROTOCOL_VERSION by heart or grepping for it.

I also updated the metaClientCapabilities row and the matching one in §3.1 to state the clip and the 20-name cap, since the bound is now part of the contract an operator reads rather than an implementation detail.

Comment on lines +214 to +219
protocolVersionHeader: headerDisagrees ? clipClientLabel(protocolVersionHeader) : null,
metaProtocolVersion: clipClientLabel(meta[META_PROTOCOL_VERSION_KEY]),
metaClientName: clipClientLabel(metaClientInfo.name),
metaClientVersion: clipClientLabel(metaClientInfo.version),
metaClientCapabilities: Object.keys(asRecord(meta[META_CLIENT_CAPABILITIES_KEY])).sort(),
modernMethod: method === SERVER_DISCOVER_METHOD,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, and thanks — this was a real hole, not a theoretical one. Fixed.

Both how many capability keys a caller sends and how long each one is are the caller's choice, bounded only by the 1 MB body cap. Measured with a ~600KB _meta payload:

caller input:                  618,948 bytes
before (verbatim key copy):    612,891 bytes   <- the log line
after  (whole log line):         1,447 bytes

Names are now clipped individually and the set capped at 20, with a trailing +N more entry rather than a silent truncation — a list that is quietly short reads as a client declaring fewer capabilities, which is worse than saying it was cut.

Two things worth noting:

describeInitializeParams had the identical unbounded copy and is already merged in #4299, so this fixes both through one shared capabilityNames helper rather than only the code this PR adds.

The precedent was already here and I did not follow it. MAX_CLIENT_LABEL_LENGTH clips clientName/clientVersion in the same file, and the metrics registry has MAX_COUNTER_LABELS with the rationale spelled out — "their cardinality is bounded by the caller, not by us". I applied that reasoning to the scalar fields and skipped the collection.

Covered by tests for the cap, the clip, the +N more marker, the untouched normal case, and one asserting the whole serialized line stays under 2KB for a ~600KB input.

Review follow-ups on the modern-era detector.

Capability names were copied out of caller input verbatim. Both the number of
keys and their length are the caller's choice, bounded only by the 1 MB body
cap, so this was caller-controlled log amplification: a ~600KB payload
produced a ~613KB log line. Now 1.4KB — each name clipped, the set capped at
20 with a trailing `+N more` so a truncated list is visibly truncated rather
than reading as a client that declared fewer capabilities.

describeInitializeParams had the identical unbounded copy and is already
released; the shared helper fixes both. The repo already had the precedent I
should have followed — MAX_CLIENT_LABEL_LENGTH and the metrics registry's
MAX_COUNTER_LABELS exist for exactly this.

Also documents `servedProtocolVersion` in the runbook field table, which the
event emits but the reference omitted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gilgardosh
gilgardosh deployed to accounter-fullstack August 27, 2026 14:56 — with GitHub Actions Active
@gilgardosh
gilgardosh deployed to accounter-fullstack August 27, 2026 14:56 — with GitHub Actions Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants