Align with spec #3002: optional clientInfo, serverInfo in result _meta#3143
Conversation
Spec PR #3002 (merged upstream 07-16) made io.modelcontextprotocol/clientInfo optional on request _meta, removed the top-level serverInfo field from DiscoverResult, and introduced ResultMetaObject: every result's _meta now carries an optional io.modelcontextprotocol/serverInfo Implementation. ResultMetaObject joins OPEN_CLASSES: the spec defines it as an open key-value bag (all MetaObject key rules apply), so it must keep extra="allow" or the serialize sieve would silently drop custom result _meta keys.
…3002) Spec PR #3002 made io.modelcontextprotocol/clientInfo optional (SHOULD-include): the required per-request envelope is now the protocolVersion + clientCapabilities pair. - classify_inbound_request rung 1 demands the pair and reads absent clientInfo as None; a missing required key rejects -32602 with a message naming the key(s), per basic/index.mdx. - Era evidence on the stdio dual-era loop is now presence of the reserved protocolVersion _meta key alone: the io.modelcontextprotocol/ prefix is spec-reserved, so legacy traffic never mints it, and a half-built envelope (version without capabilities) routes modern to get the classifier's named rejection instead of the legacy path's generic one. Failed classification still locks no era. - The regenerated surface types (previous commit) already carry the optional clientInfo on RequestMetaObject. - Connection records client_capabilities as its own fact so capability checks (check_capability, sampling tools validation, extension and apps gates) work for pair-only requests; the client_params setter keeps the two in lockstep on the handshake path. ServerSession exposes client_capabilities and all capability consumers read it. - The Mcp-Param schema-resolving tools/list walk omits the clientInfo key from its synthetic envelope when the caller sent none.
…(spec #3002) The 2026-07-28 draft removed the serverInfo body field from DiscoverResult: servers now report identity by stamping io.modelcontextprotocol/serverInfo into every result's _meta (new ResultMetaObject type), and clients treat it as optional, display-only metadata. Server side: - DiscoverResult loses server_info (monolith and generated surface); the default discover handler no longer passes identity. - Every 2026-era result is stamped at the runner's single exit point, after the middleware chain, so custom methods, empty results, and middleware short-circuits are covered by construction. Stamping builds a shallow copy rather than mutating a dict the handler may retain, a handler-authored value wins, and the dumped stamp is cached per server. Notifications, error responses, and handshake-era results are never stamped. - Opt-out per the spec's 'unless specifically configured not to do so': Server(include_server_info=False), also exposed on MCPServer. Client side: - ClientSession.server_info reads the discover result's _meta stamp, parsed once at adopt time; absent or malformed reads as None (the spec forbids acting on the value, so a bad stamp must not fail the connection). The wire surface keeps the field untyped for the same reason, via a shape-driven transform in the generator that stays lenient for future result-meta definitions. - Client.server_info is now Implementation | None. This also fixes a live interop break: servers that already shipped the new shape omit the body field, which previously failed our discover validation and silently downgraded auto mode to the legacy handshake. Tests: the interaction matrix runs with stamping off (the stamp would bake the commit-dependent package version into every snapshot); stamping has dedicated unit and wire-level coverage in tests/server/, and the tests that exercise identity assert the new _meta form.
Migration guide gets a grouped section for the spec #3002 reshape (field removal, stamping default and opt-out, Optional server_info, pair-only requests and the client_capabilities accessor). Whats-new, protocol-versions, client, session-groups, media, testing, and low-level pages updated to the new shapes.
The last published referee (0.2.0-alpha.9) still enforces the pre-#3002 shape, so implementing the reshape at the old pin fails three checks. Conformance main@da56f663 includes #403 (checks flipped to the final revision) and #406 (per-check expected-failures granularity); the sha256- verified tarball pin mechanism is unchanged. All server scenarios pass with zero baseline entries. The bump window adds client auth scenarios for features the OAuth client does not implement (DPoP per SEP-1932, and the jwt-bearer grant); those are baselined per-check so their passing checks stay live. Riding the same window: requiredCapabilities error data moves from the old array shape to the schema's object shape (referee #376), and the everything-server capability gate reads client_capabilities so it works for clients that omit the now-optional clientInfo.
📚 Documentation preview
|
There was a problem hiding this comment.
All reported issues were addressed across 77 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Beyond the inline doc nit, a few other candidates were examined and ruled out this run: the server_info_stamp cached_property going stale vs. live identity attributes (identity is fixed at construction, and the include_server_info opt-out is read live at the stamping site, so the interaction conftest's post-construction toggle works); the docs/client/index.md intro contradicting the new Implementation | None contract (the intro line was updated in this PR); and docs/get-started/testing.md's snippet drifting from its tests/docs_src copy (the result.meta = None line keeps them equivalent).
Extended reasoning...
One nit-level finding is posted inline (doc fence shows a serverInfo version the tutorial server does not produce). This note records the additional candidates that finder agents raised and verifiers refuted, so a later pass does not re-explore them from scratch: the cached_property staleness concern, the docs/client/index.md intro consistency, and the testing.md snippet/test drift — each held up under checking against the diff. The PR itself is a large spec-alignment change with author-flagged design decisions (public API break, null-clientInfo tolerance, empty-result stamping), so it needs human review either way; the inline comment already signals that.
Cache-hint filling and the serverInfo stamp were two separate if-blocks in ServerRunner._on_request, one of them applied after the middleware chain by patching the returned dict. _serialize now owns the whole outbound envelope (hints, dump, per-version sieve, serverInfo stamp) as the counterpart of the inbound classification ladder, and _dump_result copies dict results so a handler-retained object can never be mutated by wire shaping. Behavior change: a middleware that short-circuits without call_next now owns its result envelope entirely, per the existing trust contract; it is no longer stamped after the fact.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Beyond the inline finding, this pass also examined and ruled out two candidates:
- The
tests/interaction/conftest.pywrapper passingspec_versiontwice (also flagged by cubic): latent only — no test passesspec_version=through theconnectfixture; the value comes exclusively from the matrix parametrization inpytest_generate_tests, so no current caller can hit theTypeError. cubic's suggestedkwargs.popcleanup would still be a harmless hardening. - The
tests/docs_src/test_testing.pydocstring claim that "the import path is the only difference" vs the page's test (the test now also strips the serverInfo stamp): cosmetic docstring drift, no behavioral impact.
Extended reasoning...
Bugs were found this run (posted inline), so no approval verdict is offered. This note records candidates that finder agents raised and verification refuted, so the author and later review passes have a record of what was already examined — in particular the conftest spec_version concern, which overlaps with cubic-dev-ai's open P3 comment. I independently confirmed the refutation: a repo-wide search shows no interaction test invokes the connect fixture with an explicit spec_version keyword; the fixture's cells are generated from @requirement marks, so the wrapper's fixed keyword is never in conflict in practice.
A short-circuiting Extension.intercept_tool_call previously returned from the middleware chain, above the runner's outbound envelope pass, so its result skipped the per-version result sieve and the 2026-era serverInfo _meta stamp - and extension authors had no way to add the stamp themselves. Compose the interceptor chain around the tools/call handler instead of appending a middleware: a short-circuited result is now serialized exactly like a handler result, and call_next hands interceptors the handler's domain result rather than the wire dict. Also: pin version="2.0.0" on the lowlevel structured-output tutorial server so the page's fenced result is reproducible (and prove the fence verbatim in its test), document middleware envelope ownership in the middleware and migration pages, and update two stale docstrings from the envelope refactor.
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Moving extension interception to the handler layer put interceptor results through the per-version result sieve, which strips fields the surface doesn't know - breaking claimed extension shapes (a resultType outside the core vocabulary carrying vendor fields). The sieve's job is per-version surface enforcement for core results; an extension resultType marks a shape the extension owns, so it now applies to core-vocabulary results only. Claimed shapes still get the 2026-era serverInfo stamp: they are results like any other.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The spec's opt-out language ("unless specifically configured not to do
so") is already expressible at the layer that owns response envelopes:
a middleware that strips the key from the results it passes along. A
dedicated constructor flag duplicated that mechanism, so it is removed;
neither the go nor the typescript SDK shipped one. The interaction
matrix now pins a server version and strips the stamp at comparison
sites (tests/_stamp.unstamped) instead of running a non-default config.
Also from review:
- server_info_stamp returns a deep copy per access: the cached dump's
nested values (icons) previously aliased into every stamped response,
so mutating one response's stamp corrupted all later ones.
- The claimed-shape sieve bypass is now modern-only: a resultType
outside the core vocabulary on a legacy session fails the per-version
surface (INTERNAL_ERROR) instead of leaking a shape legacy clients
cannot resolve.
- The validated params argument of Extension.intercept_tool_call is
documented as authoritative: a context rewrite through call_next
adjusts what the handler observes, not which tool call runs (pinned
by test).
There was a problem hiding this comment.
All reported issues were addressed across 33 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The server-identity section described 2026-07-28 behavior only: server/discover does not exist in v1.x, so no v1 code path reaches it. The migration guide covers exactly what breaks when v1.x code moves to v2; the identity stamp and the Implementation|None client surface are documented on the low-level server, middleware, and client pages instead. The era-neutral accessors section keeps its corrected server_info type without the removed cross-reference. No-Verification-Needed: docs-only edit
The shared strip helper was if-present, so a modern cell whose result lost its serverInfo stamp would still pass. The helper is now strict (stamp must be present and well-formed before it is stripped), and the interaction matrix resolves 'unstamped' as a fixture keyed on the cell's spec_version: modern cells assert-and-strip, handshake-era cells assert the result was never stamped. Every existing comparison site now enforces the stamp contract in both directions. The connect fixture returns a CellConnect callable that names its cell's spec_version, so sibling fixtures can key on the era without re-deriving it. No-Verification-Needed: test-only change
There was a problem hiding this comment.
Beyond the inline docs nit, two candidates were examined and ruled out this run: (1) whether a mis-shaped (present) optional clientInfo on spec methods contradicts the degrade-to-absent contract — it does not: the lenient coercion is scoped to connection identity in Connection.from_envelope, while the per-version params surface rejecting a malformed present value is deliberate (the custom-method test in tests/server/test_runner.py documents the asymmetry); (2) whether tests/docs_src/test_testing.py's "import path is the only difference" docstring went stale — both the page's shown test and the repo test normalize the _meta stamp before the snapshot, so the claim holds in substance.
Extended reasoning...
This run's only new finding is the docs/whats-new.md line 47 nit, posted inline. The two refuted candidates above are recorded so a later pass doesn't re-explore them from scratch; this note is informational only and makes no claim about the rest of the PR, which as a 97-file protocol-alignment change still warrants human review.
The version fallback to the installed mcp package dated to the SDK's first initialization-options helper and reported the SDK's version as the server's own. No spec text supports that: the schema requires the field but says only 'the version of this implementation', and the ecosystem expectation (spec discussion #1176) is the server's distributed version. typescript and kotlin require an explicit identity; go serializes an empty string for an unset version. Align with go: version='' unless the constructor sets one, and delete the _package_version helper. The stamp is now fully deterministic for unversioned servers, so the unit suites carry it literally in their expectations instead of stripping it (tests._stamp stays for the dual-era interaction matrix, whose era split is unrelated), and the matrix conftest's version pin is gone. Small migration note added: this is a v1-visible behavior change for servers that never set a version.
None and the empty string were never distinguished (the old fallback used a falsy check, and version, unlike title or description, can never be omitted from the wire), so the str|None type encoded the same state twice. The constructor takes version: str = "" on both server tiers, MCPServer.version returns str, and the two 'or empty' patches at the identity sites disappear. No-Verification-Needed: type-narrowing refactor, zero behavior change, full suite green
Unversioned servers now report an empty version string, and the conformance referee's identity check (sep-2575-server-identifies-in- result-meta) treats an empty version as an unidentified server. A conformance fixture should fully identify itself; use the example package's own version. No-Verification-Needed: one-line example fixture config; conformance CI is the verification
- extensions.md interception section: call_next returns the handler's result, short-circuits are serialized and stamped like handler results, the validated params argument decides the tool invocation, and no middleware is involved either way. - whats-new client recap: server_info is Implementation | None. - Server.server_info and a transport-test helper docstring still described the removed package-version fallback. - _stamp_server_info docstring states the explicit-null posture (null reads as absent, mirroring clientInfo). No-Verification-Needed: docs and docstring-only edits
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/advanced/extensions.md">
<violation number="1" location="docs/advanced/extensions.md:143">
P2: Interceptor docs promise core-schema enforcement that the response pipeline does not provide for modern extension `resultType` values. Keep the stamp guarantee, but remove the “off-schema” claim so extension authors do not rely on validation that `_serialize` deliberately bypasses.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| result, including the 2026-era `serverInfo` identity stamp, so a | ||
| short-circuiting interceptor never produces an anonymous or off-schema | ||
| response. |
There was a problem hiding this comment.
P2: Interceptor docs promise core-schema enforcement that the response pipeline does not provide for modern extension resultType values. Keep the stamp guarantee, but remove the “off-schema” claim so extension authors do not rely on validation that _serialize deliberately bypasses.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/advanced/extensions.md, line 143:
<comment>Interceptor docs promise core-schema enforcement that the response pipeline does not provide for modern extension `resultType` values. Keep the stamp guarantee, but remove the “off-schema” claim so extension authors do not rely on validation that `_serialize` deliberately bypasses.</comment>
<file context>
@@ -133,14 +133,21 @@ or veto a tool call:
+* `call_next(ctx)` runs the rest of the chain and returns the handler's result.
+ Return it unchanged (observe), return something else (replace), or raise an
+ `MCPError` (refuse). Whatever you return is serialized like any handler
+ result, including the 2026-era `serverInfo` identity stamp, so a
+ short-circuiting interceptor never produces an anonymous or off-schema
+ response.
</file context>
| result, including the 2026-era `serverInfo` identity stamp, so a | |
| short-circuiting interceptor never produces an anonymous or off-schema | |
| response. | |
| result, including the 2026-era `serverInfo` identity stamp. |
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/mcp/server/runner.py:484-506— The optional-clientInfo docstring sweep missedmodern_on_request(runner.py:784-787): it still describes the modern per-request envelope as carrying "protocol version, client info, and capabilities" inparams._meta, but after this PR (spec #3002) clientInfo is optional and thehandle()closure directly below serves pair-only requests. A one-line rewording — e.g. "each request carries the protocol version and client capabilities (and optionally client info) inparams._meta" — keeps this public docstring consistent with the sweep applied everywhere else.Extended reasoning...
The stale claim. The
modern_on_requestdocstring (src/mcp/server/runner.py:784-787, just below this hunk) still reads: "drive an in-process server on the modern per-request-envelope path (each request carries protocol version, client info, and capabilities inparams._meta; noinitializehandshake)". That sentence presentsclientInfoas part of what every modern request carries — exactly the pre-#3002 required-triple framing this PR retires.Why it is wrong post-PR. Spec #3002 (and this PR) make the request-side
io.modelcontextprotocol/clientInfo_metakey optional (SHOULD-include); the required pair isprotocolVersion+clientCapabilities. The code directly under this docstring already implements that: thehandle()closure readsmeta.get(CLIENT_INFO_META_KEY)with aNonedefault, andConnection.from_envelopedegrades an absent or mis-shaped value to not-supplied while recordingclient_capabilitieson its own. A pair-only request is a complete, valid modern request served by this very entry point — the PR's own tests pin it (test_from_envelope_records_capabilities_without_client_info,test_dual_era_loop_pair_only_envelope_serves_modern_and_locks).This is a missed sweep spot, not a deliberate wording choice. The PR systematically rewrote the identical required-triple claim everywhere else in this changeset:
_has_modern_envelope(runner.py:487-498, now "presence of the version key alone is the rule, not the full required pair"),classify_inbound_request's rung-1 docs (shared/inbound.py:380-384, "client info is optional (SHOULD-include, spec PR #3002)"),Connection.from_envelope's docstring,serve_dual_era_loop's prose ("envelope triple" → "envelope keys"),tests/interaction/_requirements.py(lifecycle:stateless:request-envelope: "/clientInfo is optional"), anddocs/whats-new.md. Only this one sentence kept the old framing.Why it matters.
modern_on_requestis public surface — it is listed in runner's__all__(runner.py:76) — and this docstring is the documented recipe for wiring aDirectDispatcherpeer-pair onto the modern path. An integrator reading it comes away believing client info belongs to the required envelope shape, the exact requirement the spec and this PR removed. AGENTS.md requires accurate docstrings on public APIs.Step-by-step proof. (1) Build a modern request whose
_metacarries onlyprotocolVersion+clientCapabilities(noclientInfo) and dispatch it through theOnRequestreturned bymodern_on_request. (2)handle()extractsmeta.get(CLIENT_INFO_META_KEY)→Noneandmeta[CLIENT_CAPABILITIES_META_KEY]→ the declared caps. (3)Connection.from_enveloperecordsclient_capabilitiesand leavesclient_paramsasNone; the request is served normally — as the PR's pair-only tests demonstrate. (4) Yet the docstring told the reader this request would be incomplete, since "each request carries ... client info". The doc contradicts the code it sits on.Fix. One-line rewording, e.g.: "each request carries the protocol version and client capabilities (and optionally client info) in
params._meta; noinitializehandshake".Severity. Docs-only staleness — no runtime behavior is wrong, so nit. Same missed-sweep class as the already-flagged
Server.server_infopackage-version-fallback docstring, but a distinct location; no other finding covers it.
There was a problem hiding this comment.
No new findings this pass, and the docs/docstring items from my earlier comments are all addressed in 3d5f673 — but this PR reshapes protocol identity across the runner, client, and generated types with a public API break (Client.server_info is now Implementation | None), so the design decisions listed in the description deserve a human look.
What was reviewed this pass:
- The outbound envelope pass in
runner.py: stamp precedence (handler-authored wins, explicit null re-stamped), the legacy-gated sieve bypass for claimedresultTypeshapes, and stamp aliasing (fresh deep copy per access). - The relaxed inbound ladder (
shared/inbound.py) and the dual-era loop's version-key-only era evidence — rejections name the missing required key, and legacy traffic cannot mint the reserved prefix. - Ruled out: the extensions.md "never off-schema" wording (a claimed
resultTypeis vocabulary the extension owns, not a validation gap), and mis-shaped optionalclientInforejecting spec-method requests (verified: permitted strictness on a present-but-invalid value; the spec's -32602 MUST-reject scope covers only the required pair, andnullstill reads as absent).
Extended reasoning...
Overview
This PR aligns the SDK with spec #3002: clientInfo becomes an optional (SHOULD-include) request _meta key, and serverInfo moves from the server/discover result body into a per-result _meta stamp. It touches 98 files: the vendored schema and regenerated wire types (mcp-types), the server runner's new single outbound envelope pass (cache hints + per-version sieve + identity stamp), the inbound classification ladder, connection capability bookkeeping (client_capabilities decoupled from client_params), a rework of extension tools/call interceptors from middleware to handler-layer composition, the client's identity read path, conformance pins, docs, and a large test sweep.
Security risks
Low direct risk, but not zero: the inbound classifier's envelope rung was relaxed (era evidence is now the protocol-version key alone), and the sieve gained a bypass for claimed extension resultType shapes. Both were examined — the relaxation still rejects missing required keys with INVALID_PARAMS naming the key and locks no era on failure, and the sieve bypass is gated to modern protocol versions after cubic's P1 (fixed in fb9b64c), so legacy sessions cannot receive shapes they cannot resolve. The stamp itself is display-only per spec and is deep-copied per response, with a regression test covering cache-aliasing corruption. No auth or crypto surface is touched; the conformance workflow bump is pinned by commit SHA plus a SHA-256 digest check.
Level of scrutiny
This is production-critical protocol-core code with a deliberate public API break (Client.server_info → Implementation | None) and roughly ten judgment calls the author explicitly lists for review (null-clientInfo tolerance, empty-result stamping, no constructor opt-out, interceptor params authority, unversioned servers reporting an empty version). Those are exactly the kinds of design decisions a maintainer should sign off on; shadow-approval is not appropriate regardless of bug count.
Other factors
The review history on this PR is healthy: cubic and my prior passes surfaced one real P1 (legacy leak of claimed shapes), a stamp-aliasing hazard, and several docs staleness items, and every one was either fixed (fb9b64c, debec20, 3d5f673) or explicitly pinned as contract with tests and docs. This pass found no new bugs; two finder candidates were adversarially examined and refuted, including one I re-verified against the live code (mis-shaped clientInfo on spec methods rejects at the surface, which is permitted strictness, while null reads as absent — matching the PR's stated posture). Coverage stays at 100% branch, and the six conformance legs are reported green against the new referee. Remaining open thread: cubic's latest P2 on the extensions.md "off-schema" wording, which overlaps the candidate refuted here — a wording call for the author to settle.
The 2026-07-28 schema requires Result.resultType: servers implementing the revision MUST include it; the absent-means-complete bridge exists for clients of older servers only. The per-version sieve guaranteed it for core spec methods but custom methods, extension methods, and empty results shipped without it, which the typescript client rejects as an invalid result. Fill 'complete' at the envelope pass when the field is absent (or explicitly null) on a modern-era result, mirroring the typescript server's stampResultType. Handshake-era results are untouched, and claimed extension shapes keep their own tag.
A server that answers server/discover but advertises no modern version in supportedVersions (go-sdk's default stateful streamable server does this) previously made adopt() raise 'No mutually supported modern protocol version'. On the old schema this path was unreachable by accident: the reply lacked the then-required body serverInfo, failed validation, and fell into the unparseable-result fallback. Making serverInfo optional exposed the raise. Treat the reply as what it is, an explicit legacy advertisement, and run the initialize handshake, matching the -32022-with-legacy-versions branch directly above and the behavior of both the typescript and go clients. adopt() itself stays strict for direct prior_discover use.
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/mcp/client/_probe.py">
<violation number="1" location="src/mcp/client/_probe.py:104">
P1: A discover response that advertises only an unsupported future/modern version is now downgraded to `initialize()` rather than reported as a version incompatibility. The no-mutual-version test also matches a legacy-only advertisement, so it needs to verify that the advertised list includes a handshake-era version before falling back; otherwise preserve `adopt()`'s incompatibility error (as the `-32022` branch already does).</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| except ValidationError: | ||
| await session.initialize() # unparseable result → not modern evidence | ||
| return | ||
| if not any(v in result.supported_versions for v in MODERN_PROTOCOL_VERSIONS): |
There was a problem hiding this comment.
P1: A discover response that advertises only an unsupported future/modern version is now downgraded to initialize() rather than reported as a version incompatibility. The no-mutual-version test also matches a legacy-only advertisement, so it needs to verify that the advertised list includes a handshake-era version before falling back; otherwise preserve adopt()'s incompatibility error (as the -32022 branch already does).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/client/_probe.py, line 104:
<comment>A discover response that advertises only an unsupported future/modern version is now downgraded to `initialize()` rather than reported as a version incompatibility. The no-mutual-version test also matches a legacy-only advertisement, so it needs to verify that the advertised list includes a handshake-era version before falling back; otherwise preserve `adopt()`'s incompatibility error (as the `-32022` branch already does).</comment>
<file context>
@@ -89,13 +94,21 @@ async def negotiate_auto(session: ClientSession) -> None:
except ValidationError:
await session.initialize() # unparseable result → not modern evidence
return
+ if not any(v in result.supported_versions for v in MODERN_PROTOCOL_VERSIONS):
+ # A discover-answering server that advertises no modern version
+ # (go-sdk's stateful streamable default does this) is an explicit
</file context>
| if not any(v in result.supported_versions for v in MODERN_PROTOCOL_VERSIONS): | |
| if not any(v in result.supported_versions for v in MODERN_PROTOCOL_VERSIONS): | |
| if not any(v in result.supported_versions for v in HANDSHAKE_PROTOCOL_VERSIONS): | |
| session.adopt(result) | |
| return |
There was a problem hiding this comment.
I didn't find any bugs in this run, but this is a 99-file spec-alignment PR with public API breaks (Client.server_info becoming Implementation | None, the empty-version default) and changes to the inbound classification ladder and outbound envelope pass — it needs a human maintainer's judgment on the design decisions the description flags.
What was reviewed:
- The outbound envelope pass in
runner.py(stamp precedence,resultTypefill, the legacy gate on the claimed-shape sieve bypass) and the interceptor rework inextension.py, including the fixes for the earlier cubic/claude findings. - The probe's new legacy-advertisement fallback in
_probe.py— the disjoint-supportedVersions concern was examined and ruled out (it matches the ts/go client behavior, and a modern-only disjoint server still surfaces -32022 as a real incompatibility). - Candidates examined and ruled out:
Server(version=None)lazy failure (a type violation on the v2 branch, with the behavior change documented in migration.md); dual-eraserverInfoconsistency in the runner stamp; the -32602 asymmetry for malformedclientInfo; and the extensions.md off-schema wording cubic flagged (the sieve does apply to short-circuited core-shape interceptor results). - Checked the malformed-stamp-reads-as-absent client parse in
session.py— it's confined to the display-only identity and never fails a connection.
Extended reasoning...
Overview
This PR aligns the SDK with spec #3002 (2026-07-28 draft identity reshape): clientInfo becomes an optional request _meta key, and serverInfo moves from the server/discover result body into a per-result _meta stamp. It re-vendors the draft schema and regenerates the surface models, reworks the runner's outbound serialization into a single envelope pass (cache hints → per-version sieve → resultType fill → serverInfo stamp), moves extension tools/call interceptors from the middleware tier to the handler tier, adds a client_capabilities connection fact so capability checks work for pair-only requests, changes the auto-mode probe's fallback behavior, and bumps the conformance referee pin. 99 files changed across types, server, client, transports, docs, and tests.
Security risks
Low direct risk: the serverInfo/clientInfo values are explicitly display-only and the schema patch plus lenient read-edge parse keep malformed identities from affecting behavior. The relevant surfaces are the inbound classification ladder (shared/inbound.py now requires only the version+capabilities pair, with INVALID_PARAMS naming missing keys) and the era-evidence relaxation in _has_modern_envelope (protocol-version key alone instead of the full triple) — both are request-routing changes on a spec-reserved key prefix, well-tested, and neither weakens an auth or capability check. The stamp-aliasing fix (deep-copied server_info_stamp) closes the one real cross-response mutation hazard, with a regression test.
Level of scrutiny
This is production-critical protocol machinery in the SDK's dispatch paths, with deliberate public API breaks (Client.server_info nullable, empty-string default version, the interceptor params-is-authoritative contract) that the author explicitly asks reviewers to weigh in on in the description. That combination — large, core, and carrying design decisions — rules out shadow-approval regardless of bug count. The right reviewer is a human maintainer who can ratify the flagged decisions (null-tolerant clientInfo, no stamping opt-out flag, claimed-shape sieve bypass being modern-only).
Other factors
All previously raised findings (mine and cubic's) were addressed in debec20f, fb9b64cf, 476d758, and 3d5f6730, each with a pinned regression test; the remaining open cubic thread on extensions.md wording is a docs nit. Test coverage is strong: the interaction matrix gained an era-aware stamp fixture, the runner gained targeted tests for every stamp edge case (handler-authored value, null, non-mapping _meta, retained-dict mutation, legacy sieve), and the conformance suite passes with zero server skips. The bug-hunting pass this run raised five candidates and refuted all five; my spot-checks of the probe fallback and the version default agree with those verdicts.
Aligns the SDK with spec #3002 (merged 2026-07-16), the 2026-07-28 draft's identity reshape. Reference implementations: go#1097, ts#2513 (shipped in
2.0.0-beta.5).What the spec changed
_metakeyio.modelcontextprotocol/clientInfois now optional (SHOULD-include); the required pair isprotocolVersion+clientCapabilities, and the-32602/HTTP 400 MUST-reject applies only to those.DiscoverResultno longer has a top-levelserverInfofield. Servers SHOULD stampio.modelcontextprotocol/serverInfointo every result's_metainstead (newResultMetaObjecttype), "unless specifically configured not to do so".What this PR does
Types: re-vendors the draft schema at spec commit
71e30695and regenerates the surface models. The newResultMetaObjectjoins the generator's open-classes list (the spec defines it as an open key-value bag; without this the serialize sieve would drop custom result_metakeys).Server: accepts requests that omit
clientInfo(rejecting a missing required key with-32602naming the key); recordsclient_capabilitiesas its own connection fact so capability checks work for pair-only requests (ServerSession.client_capabilitiesis the new preferred accessor); era evidence on the stdio dual-era loop is now the reservedprotocolVersionkey alone. The response envelope is assembled in a single outbound pass in the runner (cache hints, the per-version sieve, and now theserverInfo_metastamp), the outbound counterpart of the inbound classification ladder: every 2026-era result is stamped there, custom methods and empty results included. A handler-authored value wins; notifications, error responses, and handshake-era results are never stamped, and a middleware that short-circuits withoutcall_nextowns its result envelope entirely (the pipeline never patches it after the fact). Two consequences of that pass, from review: extensiontools/callinterceptors now compose around the handler instead of running as middleware, so a short-circuitingExtension.intercept_tool_callresult is sieved and stamped exactly like a handler result (extension authors deal in domain results and cannot author wire envelopes); and the sieve applies to core-vocabulary results only - a claimed extension shape (aresultTypeoutsidecomplete/input_required) is the extension's to shape, though it is still stamped.Client: reads server identity from the discover result's
_metastamp;Client.server_info/ClientSession.server_infoare nowImplementation | None. This also fixes a live interop break: typescript-sdk2.0.0-beta.5already omits the bodyserverInfo, which made our auto-mode probe treat a spec-conformant discover reply as unparseable and silently fall back to the legacy handshake. The probe also gained the complementary fallback that removing the required field exposed: a discover reply advertising only handshake-era versions (go-sdk's default stateful streamable server) is treated as an explicit legacy advertisement and runsinitialize, matching the typescript and go clients.Conformance: bumps the pinned referee to conformance
da56f663(includes conformance#403, the post-#3002 checks, and #406, per-check baselines). All server legs pass with zero skip entries. The bump window adds new client auth scenarios (DPoP per SEP-1932, and a jwt-bearer grant check) for features the OAuth client does not implement; those are baselined per-check. TherequiredCapabilitiesfixtures move from the old array shape to the schema's object shape, and the everything-server capability gate now readsclient_capabilitiesso it works for pair-only clients.Decisions to review
Client.server_infoisImplementation | None. Identity is optional wire metadata now; an anonymous server (or a pinned connection withoutprior_discover=) reads asNone, where it previously was a placeholderImplementation(name="", version=""). Not in the migration guide by design: that file covers only v1.x code that breaks on v2, andserver/discovernever existed in v1; the client and low-level server pages carry the details.serverInfodisplay-only (clients SHOULD NOT act on it), so a malformed value must never reject an otherwise-valid response. The wire surface keeps the field opaque (schema patch) and the typed, lenient parse happens at the read edge. typescript-sdk does the same with a schema-level catch."clientInfo": nullis accepted as absent. go rejects an explicit null with-32602; this SDK treats it like a missing key. The field is optional and display-only, so we lean tolerant; flagging it in case reviewers prefer strictness.resultType(schema-required: servers implementing 2026-07-28 MUST include it). The per-version sieve guarantees it for core methods; the envelope pass fills"complete"for everything else (custom methods, extension methods, empty results), mirroring the typescript server'sstampResultType. An empty result becomes{"resultType": "complete", "_meta": {...}}on the 2026 wire.include_server_info=False; it was removed as redundant with the middleware mechanism.)resultTypeoutside the core vocabulary marks a shape the extension owns; on a legacy session it still goes through the per-version surface and fails as INTERNAL_ERROR, since legacy clients cannot resolve claims.paramsargument ofExtension.intercept_tool_callis authoritative: a context rewrite throughcall_nextadjusts what the handler observes onctx, not which tool call runs. Wire-level rewriting belongs toServer.middleware, above params validation.version: "". The v1 fallback to the installedmcppackage version reported the SDK's version as the server's own; no spec text supports that, and the ecosystem expectation (spec discussion Query-Awarelist_toolsfor Scalable Tool Selection #1176) is the server's distributed version. go serializes an empty string for an unset version; typescript and kotlin require an explicit identity. Aligned with go, with a migration note since v1 servers that never set a version change what they report.unstamped(result)asserts the stamp is present and well-formed, then strips it so one expected payload stays valid across eras; on a handshake-era cell it asserts the result was never stamped. Unit suites carry the (now deterministic) stamp literally in their expectations instead.Testing
clientInforequests, malformed-envelope rejections, discover shape, stamp presence/opt-out, legacy handshake untouched, and auto-mode negotiation against both an identifying and an anonymous server.AI Disclaimer