Align 2026-07-28 wire with spec PR #3002: serverInfo in result _meta, clientInfo optional#2513
Conversation
🦋 Changeset detectedLatest commit: d1040b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
There was a problem hiding this comment.
Beyond the inline doc-prose nits, two candidate issues were examined and ruled out: (1) whether Client.discover() re-probing leaves getServerVersion() and the cache partition stale — it only ever updated _discoverResult, a pre-existing behavior this PR does not change (the PR only alters how _serverVersion is derived at connect time); (2) the stampServerInfoMeta / probe-classifier interplay (malformed _meta stamp, handler-authored value, 2025 never-stamp) is pinned by the new encodeContract and probeFixtureCorpus tests and behaves as documented.
Extended reasoning...
Bugs were found (three doc/prose nits, posted inline), so per policy this is an informational note only, not a verdict. The core wire changes (envelope clientInfo optionality, DiscoverResult serverInfo relocation to _meta, the encode-seam stamp, and the transitional dual emission) are extensively test-pinned in this diff; the ruled-out items above were re-checked against packages/client/src/client/client.ts and the new test suites. This PR is large and touches the protocol wire layer with breaking public-type changes, so human maintainer review is still expected.
1f79483 to
ea05888
Compare
d2da01e to
6eb7046
Compare
There was a problem hiding this comment.
Beyond the previously posted inline findings (which still stand — no new commits since), this run also examined the anonymous-server cache-identity path in Client._deriveServerIdentity and ruled it out: the fallback order (serverInfo → transport sessionId → per-connection surrogate) matches the documented contract, the sessionId fallback is pre-existing behavior, and partition collision-safety comes from the cache's JSON-array encoding rather than the identity value itself.
Extended reasoning...
This run's bug hunters surfaced one candidate — that the anonymous-server cache identity uses the server-controlled sessionId instead of the promised per-connection surrogate — and verification refuted it: the JSDoc on _deriveServerIdentity (packages/client/src/client/client.ts:1301-1319) explicitly documents sessionId as the intermediate fallback before the surrogate, that ordering predates this PR, and the surrogate is only needed for the sessionId-less anonymous case introduced by #3002. Not approving: this is a large breaking wire-protocol change and the prior run's finding on ResultMetaObjectSchema in packages/core/src/schemas.ts (missing .catch(undefined) receive-side leniency, affecting message classification) is still present in the current code with no commits since it was posted.
… clientInfo optional The final 2026-07-28 revision (spec PR #3002) moved serverInfo from the DiscoverResult body to _meta['io.modelcontextprotocol/serverInfo'] and demoted the request envelope's clientInfo from required to SHOULD. We shipped the pre-#3002 shape in both directions: the client hard-rejected a conforming server's discover response (misclassifying it as legacy and attempting initialize), and the server rejected requests without clientInfo. This models the final revision exactly — no pre-#3002 shape support in either direction. - Regenerate the 2026-07-28 anchor types, schema.json twin, and example corpus at spec commit 71e30695. - Wire schemas: envelope clientInfo optional (malformed still rejected); DiscoverResult has no body serverInfo; result _meta typed as the anchor's ResultMetaObject. A malformed _meta serverInfo is treated as absent on receive — the spec marks the field self-reported, unverified, and display-only, so a bad identity stamp never invalidates a result. The leniency lives in both the 2026 wire schema and the public ResultMetaObjectSchema: the latter sits inside JSONRPCResultResponseSchema, whose guard classifies every inbound message on both eras, so a strict key there would drop whole responses (probe failures, hung requests). - Server: stamp _meta serverInfo on every 2026-era result (handler-authored value wins; the 2025-era wire is untouched) — including the entry-built subscriptions/listen graceful-close results (SubscriptionsListenResultMeta extends ResultMetaObject). - Public schemas (@modelcontextprotocol/core): new ResultMetaObjectSchema; result _meta validates against it and SubscriptionsListenResultMetaSchema extends it, with the matching ResultMetaObject type. - Client: still sends clientInfo; reads identity from _meta only. An identity-less server is anonymous: getServerVersion() is undefined and the response cache partitions under a per-connection surrogate so anonymous servers never share cache entries. - Conformance: the pinned referee (0.2.0-alpha.9) still asserts the old shape, so server-stateless is expected-failed on exactly three checks until the pin bumps to a #3002-aware release. - Tests: the exact go-sdk v1.7.0-pre.3 discover response now classifies modern (was legacy); new e2e requirements for the identity stamp and clientInfo optionality; a stale persisted blob with a body serverInfo connects with anonymous identity.
6eb7046 to
d1040b8
Compare
There was a problem hiding this comment.
No bugs found this pass, and all findings from my earlier review rounds are resolved in the latest revision — but this is a 61-file breaking change to the 2026-07-28 wire (schemas, codec encode seam, probe classification, public API), so it needs a human maintainer's look rather than a bot approval.
What was reviewed:
- The previously flagged classification-guard hazard is fixed: the neutral
ResultMetaObjectSchemaserverInfo key now carries.catch(undefined)(packages/core/src/schemas.ts), so a malformed identity stamp can no longer drop whole responses atisJSONRPCResultResponse; regression tests pin it on both the neutral layer and the real transport connect path. - Checked the encode seam:
stampServerInfoMetaruns only on the 2026 codec (2025 never-stamp pinned), handler-authored values win, non-object_metais left untouched, and both entry-builtsubscriptions/listenclose paths (HTTP + stdio) carry the stamp consistently with the schema (SubscriptionsListenResultMetakeeps subscriptionId strict, serverInfo lenient). - Checked the anonymous-identity cache path: the per-connection surrogate in
_deriveServerIdentityprevents two identity-less servers on sessionId-less transports from sharing the pre-connect''partition. - Prior doc/changeset staleness findings are resolved: the
_ondiscoverJSDoc now says capabilities are advertised as-is, the changeset scopes fail-on-malformed toclientInfo, and the PR description now matches the shipped no-dual-emission design and the three expected-failed conformance checks.
Extended reasoning...
Overview
This PR aligns the 2026-07-28 wire implementation with spec PR #3002: serverInfo moves from the DiscoverResult body to the result _meta key io.modelcontextprotocol/serverInfo, and the request envelope's clientInfo demotes from required to SHOULD. It touches 61 files across the spec anchor types, vendored schema twins and example corpus, the 2026 wire schemas and codec (a new stampServerInfoMeta encode step and an encodeResult signature extension threaded through Protocol._outboundServerInfo()), the neutral public schemas in @modelcontextprotocol/core (new ResultMetaObjectSchema inside ResultSchema._meta), client identity resolution and response-cache partitioning, both listen-router entries, conformance baselines, and migration docs. It is explicitly breaking (DiscoverResult loses serverInfo; RequestMetaEnvelope.clientInfo becomes optional).
Security risks
Low. serverInfo/clientInfo are self-reported display-only metadata and the diff consistently documents that neither side should use them for behavior or security decisions. The one identity-adjacent surface is response-cache partitioning: the new per-connection surrogate (anonymous:Date.now()-Math.random()) closes the hole where two identity-less servers over sessionId-less transports would have shared the pre-connect '' partition — an improvement, and the partition's collision safety comes from the cache's JSON-array encoding rather than the identity string's contents. Making clientInfo optional removes a validation requirement but only for a field servers were already told not to trust.
Level of scrutiny
High — this is production-critical protocol code. The most dangerous part of the design is the interaction between the newly typed _meta key and JSONRPCResultResponseSchema, whose guard classifies every inbound message on both eras: a strict key there silently drops whole responses (hung requests, probe misclassification to legacy). A prior review round flagged exactly that as a release blocker; the current revision fixes it with .catch(undefined) on the neutral schema and pins it with tests exercising the full transport path (discover.test.ts), the guard directly (types.test.ts), and the probe classifier (probeFixtureCorpus.test.ts, including the byte-real go-sdk v1.7.0-pre.3 fixture). I verified the fix is present in the checkout at the PR head. Even with that resolved, the breadth (public API additions, breaking type changes, cross-SDK interop claims, conformance-baseline expected-failure entries keyed to a future referee bump) is the kind of judgment call a maintainer should own.
Other factors
Test coverage is thorough and adversarial: leniency vs strictness is pinned on both layers (malformed clientInfo still rejects; malformed serverInfo drops to absent; subscriptionId stays strict), the 2025 never-stamp guarantee is pinned, and stale-blob/body-serverInfo paths are covered. All findings from my earlier rounds — the 🔴 classification-guard bug, the _ondiscover JSDoc contradiction, the changeset over-generalization, and the stale PR-description claims (dual emission, body fallback, two-vs-three conformance checks, the 'unstamped exception' clause) — are resolved in the latest commit; the description, changeset, migration doc, and code now tell one consistent story. The remaining open question for a human is the design ruling itself (no pre-#3002 compatibility shape in either direction, which hard-breaks pre-#3002 peers), which the PR states was decided deliberately.
Spec PR modelcontextprotocol/modelcontextprotocol#3002 (part of the final 2026-07-28
revision) moved
serverInfofrom theDiscoverResultbody to the result-_metakeyio.modelcontextprotocol/serverInfo, and demoted the request envelope'sclientInfofrom required to SHOULD. This PR updates the 2026-07-28 wire implementation to match —
the final revision is modeled exactly, with no pre-#3002 shape support in either
direction.
Motivation and Context
We shipped the pre-#3002 shape in both directions, which breaks interop with SDKs that
follow the final revision (go-sdk v1.7.0-pre.3+, csharp):
serverInfowhen parsing aDiscoverResult, so a conformingserver's discover response failed validation, the version-negotiation probe classified
the server as legacy, and the client attempted an
initializehandshake against it —a hard connect failure against a modern-only server.
_metaenvelope omitsclientInfo, which the finalrevision explicitly allows.
What changed
71e30695(the #3002 merge): the2026-07-28 anchor types, the vendored schema.json twin, and the example corpus.
clientInfois optional (present-but-malformed still fails);DiscoverResulthas nobody
serverInfo; result_metais typed as the anchor'sResultMetaObject. Amalformed
_metaserverInfo value is treated as absent on receive: the spec marks thefield self-reported, unverified, and display-only ("clients SHOULD NOT use it to
change their behavior"), so a bad identity stamp never invalidates a result. The
leniency lives in both the 2026 wire schema and the public
ResultMetaObjectSchema—the latter sits inside
JSONRPCResultResponseSchema, whose guard classifies everyinbound message on both eras, so a strict key there would drop whole responses.
_metaserverInfo stamp (spec SHOULD;a handler-authored value wins; the 2025-era wire is untouched) — including the
entry-built
subscriptions/listengraceful-close results, whose_metacarries theidentity next to the subscription id (
SubscriptionsListenResultMetaextendsResultMetaObject).clientInfoon every request (SHOULD). Reads server identityfrom the discover result's
_metaonly. A server that stamps no identity isanonymous: the connection works,
getServerVersion()isundefined, and the responsecache partitions under a per-connection surrogate so identity-less servers can never
share cache entries.
DiscoverResultno longer declaresserverInfo;RequestMetaEnvelope['io.modelcontextprotocol/clientInfo']is optional; new publicconstant
SERVER_INFO_META_KEY; newResultMetaObjectSchemain@modelcontextprotocol/core(result_metanow validates against it, andSubscriptionsListenResultMetaSchemaextends it), with the matchingResultMetaObjecttype.0.2.0-alpha.9) still asserts the OLDshape, so
server-statelessis expected-failed on exactly three checks(
sep-2575-request-meta-invalid-missing-client-info, the missing-client-infoiteration of
sep-2575-http-server-meta-invalid-400, andsep-2575-server-implements-discover, which requires body serverInfo). The entriesburn when the pin bumps to a referee release that incorporates #3002.
How Has This Been Tested?
(serverInfo in
_meta, no body field) — classifiesmodernnow, misclassifiedlegacybefore this change. A malformed_metaserverInfo also staysmodern._metaand no body field; requests withoutclientInfoare served; every modern response is stamped; 2025-era responses neverare.
_meta; a stray bodyserverInfois ignored(anonymous connect); a persisted stale blob carrying a body
serverInfoconnectswith anonymous identity; a MALFORMED
_metaserverInfo still connects modern throughthe real transport path and a post-connect call carrying one still resolves (the
neutral-layer classification guard must not drop such frames — pinned on both eras).
legs) / docs:check / docs:examples — all green.
Breaking Changes
DiscoverResultno longer declaresserverInfo, andRequestMetaEnvelope'sclientInfois optional. Code that readdiscover.serverInfomoves toclient.getServerVersion()(or the_metakey directly). Seedocs/migration/support-2026-07-28.md.Types of changes
Checklist