Skip to content

feat(core): adopt spec#2907 error-code renumber; conformance referee → 0.2.0-alpha.5#2335

Merged
felixweinberger merged 2 commits into
v2-2026-07-28from
fweinberger/on-renumber
Jun 23, 2026
Merged

feat(core): adopt spec#2907 error-code renumber; conformance referee → 0.2.0-alpha.5#2335
felixweinberger merged 2 commits into
v2-2026-07-28from
fweinberger/on-renumber

Conversation

@felixweinberger

@felixweinberger felixweinberger commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Adopts the spec#2907 / conformance#353 draft error-code renumber and bumps the conformance referee to 0.2.0-alpha.5 (which carries #353), so SDK and referee are aligned in the same change. The streamableHttp store-first fix that was previously on this branch is split out to #2342.

Motivation and Context

The 2026-07-28 draft renumbers HeaderMismatch −32001→−32020, MissingRequiredClientCapability −32003→−32021, UnsupportedProtocolVersion −32004→−32022. This updates the ProtocolErrorCode enum and the emission/recognition sites that flow from it; 2025-era serving (Session-not-found −32001) and the −32000 literal are untouched. The second commit pins the alpha.5 referee and reconciles both expected-failures*.yaml: the renumber-mismatch entries and auth/scope-step-up (2025 leg, via conformance#359) burn; nothing new is added — alpha.5's tasks-* scenarios are extension-gated and don't run against a server that doesn't advertise the extension.

How Has This Been Tested?

  • typecheck:all, lint:all
  • conformance client:all 348p/12f, client:2026 231p/24f, server 42p/0f, server:draft 81p/0f, server:2026 105p/5f — all baseline-ok against alpha.5

Breaking Changes

None at the API surface. Wire-level: 2026-07-28 era emits −32020/−32021/−32022 (alpha-to-alpha; changeset + migration note included).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@felixweinberger felixweinberger requested a review from a team as a code owner June 19, 2026 14:18
@changeset-bot

changeset-bot Bot commented Jun 19, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b9bbcda

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@modelcontextprotocol/client Minor
@modelcontextprotocol/core Minor
@modelcontextprotocol/server Minor
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/express Major
@modelcontextprotocol/fastify Major
@modelcontextprotocol/hono Major
@modelcontextprotocol/node Major

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

@pkg-pr-new

pkg-pr-new Bot commented Jun 19, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@2335

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/codemod@2335

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@2335

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server-legacy@2335

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@2335

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/fastify@2335

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@2335

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@2335

commit: b9bbcda

Comment thread packages/server/src/server/streamableHttp.ts Outdated
Comment thread packages/server/src/server/streamableHttp.ts Outdated
Comment thread .changeset/pin-modern-rejection-codes.md Outdated
Comment thread test/conformance/package.json Outdated
Comment thread .changeset/spec-anchor-repin-2fb207da.md Outdated
Comment thread packages/server/src/server/streamableHttp.ts Outdated
Comment thread packages/server/src/server/streamableHttp.ts Outdated
Comment thread docs/migration-SKILL.md
Comment thread packages/server/src/server/streamableHttp.ts Outdated
@felixweinberger felixweinberger force-pushed the fweinberger/on-renumber branch 2 times, most recently from dd652fd to 56eeb2c Compare June 22, 2026 20:15
@felixweinberger felixweinberger force-pushed the fweinberger/on-renumber branch from 56eeb2c to d5453fd Compare June 22, 2026 20:21

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Additional findings (outside current diff — PR may have been updated during review):

  • 🔴 packages/server/src/server/streamableHttp.ts:1017-1040 — After a request completes while its per-request SSE stream is closed (the new poll-and-replay success path), the client's GET + Last-Event-ID reconnect replays the stored final response but replayEvents() then registers a resumed live stream under the same streamId that nothing ever closes — no request maps to it anymore, the resumed stream's cancel callback is a no-op, and there is no after-replay close — so each completed-while-disconnected poll cycle leaks an open controller plus a permanent _streamMapping entry until transport.close(), and the server never terminates the SSE response carrying the replayed final response. Fix by closing (or not registering) the resumed stream after replay when no in-flight request still maps to the resolved streamId.

    Extended reasoning...

    The bug. This PR's store-first change makes "response delivered via replay" the standard success path of the SEP-1699 poll-and-replay flow, but the replay side never terminates that path. closeSSEStream() (packages/server/src/server/streamableHttp.ts:939-962) closes the controller and installs a controller-less placeholder under the per-request streamId S. When the handler finishes before the client reconnects, send() for the final response stores it to the eventStore (the placeholder satisfies the new stream !== undefined gate at line 1031), then the response branch finds the placeholder, calls stream.cleanup() at line 1069 (deleting _streamMapping[S]), and the cleanup loop at lines 1072-1075 deletes _requestResponseMap / _requestToStreamMapping for the request. When the client then reconnects with GET + Last-Event-ID, replayEvents() (lines 502-582) passes the conflict check (the placeholder was just removed), replays the stored notification(s) and the final response, and unconditionally registers a resumed live stream under S at lines 564-575 before returning the 200 SSE Response.

    Why nothing ever closes it. In the pre-existing resume-before-response flow, a later transport.send(response) targets S, and its response branch runs stream.cleanup(), closing the controller and removing the entry. In the resume-after-response flow there will never be another send() for S: _requestToStreamMapping no longer maps any request to it (deleted in the step above, and new requests get fresh UUID streamIds). replayEvents() has no after-replay close, and the resumed stream's ReadableStream cancel callback is deliberately empty ("Cleanup will be handled by the mapping", lines 544-547), so even a client-side disconnect leaves the entry. Only transport.close() clears it.

    Impact. Two consequences per completed-while-disconnected poll cycle on a long-lived sessionful transport: (a) the server holds an open ReadableStream controller plus a permanent _streamMapping entry — an unbounded accumulation for a workload that polls repeatedly; (b) the SSE response that delivered the replayed final JSON-RPC response is never terminated by the server, contrary to the Streamable HTTP guidance that the server SHOULD close the per-request SSE stream once all responses for the request have been sent — a spec-following client (or an intermediary doing idle-connection accounting) is left holding a hanging GET that never ends.

    Step-by-step proof. (1) Client POSTs a tools/call (id 5) on a sessionful transport with an eventStore; _requestToStreamMapping[5] = S, _streamMapping[S] is live. (2) The handler calls ctx.http.closeSSE()closeSSEStream(5) closes the controller and installs the placeholder under S. (3) The handler finishes before the client's retry interval elapses; send(response) stores the response (line 1031-1032), the response branch runs stream.cleanup() (line 1069) deleting _streamMapping[S], and the cleanup loop deletes _requestToStreamMapping[5]. (4) The client reconnects with GET + Last-Event-ID; replayEvents() finds no conflict, replays the stored events including the final response, and registers a resumed stream under S (line 564). (5) From this point on, no code path can reach that entry: send() can never resolve S again, the cancel callback is a no-op, and the SSE response stays open. The entry and its controller persist until transport.close() (lines 924-931).

    Why this is in the PR's scope. replayEvents() itself is unchanged, but pre-PR this state was effectively unreachable as a designed flow: events and responses emitted while the per-request stream was disconnected were never stored, so a Last-Event-ID reconnect could never be the delivery vehicle for an already-settled response — any resumed per-request stream still had its live response coming through send(), which closed it. The store-first change plus the placeholder cleanup-on-response is exactly what creates the "response already delivered, nothing left to close the resumed stream" state, and the changeset advertises this as the new behavior, so the missing close is the natural completion of this PR's fix. It is also distinct from the previously-flagged issues: the 409-conflict comment covers reconnects attempted while the placeholder is still installed (mid-flight), and the cancelled-request placeholder leak covers the case where no response is ever sent — this finding is the success path after the response has been delivered via replay.

    How to fix. After replayEventsAfter() completes in replayEvents(), check whether any entry in _requestToStreamMapping still maps to the resolved replayedStreamId; if none does (the replay-after-completion case), close the controller and skip (or immediately remove) the _streamMapping registration instead of leaving the resumed stream open indefinitely. A regression test that performs the closeSSE() → complete-while-disconnected → Last-Event-ID GET sequence and asserts the replayed SSE stream is closed and _streamMapping does not retain the entry would lock this in — the PR's new vitest case stops at checking the eventStore contents and never issues the reconnect.

  • 🔴 packages/server/src/server/streamableHttp.ts:951-961 — When a client cancels an in-flight request (notifications/cancelled) after the handler has called closeSSE(), the placeholder _streamMapping entry installed by closeSSEStream() is never removed: the Protocol layer suppresses the final response on cancellation, and the response-branch stream.cleanup() is the placeholder's only normal removal path. The entry (plus the _requestToStreamMapping entry) leaks for the lifetime of the sessionful transport, and for eventStores implementing getStreamIdForEventId the leaked placeholder makes replayEvents()'s conflict check answer 409 for that streamId permanently. Pre-PR, closeSSEStream() deleted the entry, so cancellation left no residue — worth folding into the placeholder rework already requested in the other review comment: also clean up the placeholder (and request bookkeeping) when a request is torn down without a response, or key the intentional-close marker outside _streamMapping.

    Extended reasoning...

    The bug. closeSSEStream() (packages/server/src/server/streamableHttp.ts:951-961) now installs a controller-less placeholder under the per-request streamId instead of deleting the _streamMapping entry as the pre-PR stream.cleanup() did. The inline comment says the placeholder "is removed when the final response is sent (or overwritten when the client reconnects via Last-Event-ID)", but a final response is not guaranteed: in packages/core/src/shared/protocol.ts, _onrequest's completion arms both return early when the request was cancelled (if (abortController.signal.aborted) { /* Request was cancelled */ return; } at lines 1023-1026 and 1053-1056), so transport.send(response) never runs for a cancelled request and the response-branch cleanup loop — the only normal-operation path that calls the placeholder's cleanup() — never executes.\n\nThe code path. Entirely within the SEP-1699 polling flow this PR targets: (1) a sessionful 2025-era transport with an eventStore; (2) a long-running tool handler calls ctx.http.closeSSE()closeSSEStream(requestId) closes the controller and installs the placeholder; (3) the client cancels the in-flight request by POSTing notifications/cancelled (the normal 2025-era cancellation path); (4) the Protocol layer aborts the request and suppresses the response, so send(response) never runs for that request id; (5) nothing else removes the placeholder — the original per-request ReadableStream's cancel callback cannot fire because closeSSE already closed the controller gracefully, and replayEvents() only overwrites the entry if the cancelling client bothers to resume, which it has no reason to do. Only the transport-level close() clears it.\n\nImpact. The placeholder _streamMapping entry (plus the pre-existing _requestToStreamMapping entry) accumulates one per cancelled-after-closeSSE request for the lifetime of the long-lived sessionful transport. The sharper consequence: for eventStores implementing getStreamIdForEventId, replayEvents()'s conflict check at line 520 (this._streamMapping.get(streamId) !== undefined → 409) treats the leaked placeholder as an active connection, so any later Last-Event-ID resume keyed to events stored under that stream is permanently rejected with 409 instead of replaying the stored events.\n\nWhy this is a regression introduced by this PR. Pre-PR, closeSSEStream() called stream.cleanup(), which deleted the _streamMapping entry — cancellation after closeSSE left no _streamMapping residue and no 409 (only the pre-existing _requestToStreamMapping leak). The new vitest cases only cover the closeSSE-then-respond and hard-disconnect paths, both of which end in a response or a deleted entry; nothing cancels a request after closeSSE(), so no test would catch this.\n\nWhy it's distinct from the existing placeholder comment (inline comment at line 961). That comment's primary 409 issue is transient — it resolves when the final response is sent and cleans up the placeholder. This bug is the path where the response is never sent, so the leak and the 409-blocked streamId are permanent. Notably, even the fixes that comment proposes (treat a controller-less placeholder as not-active in the conflict check, or guard the install) would still leave the cancelled request's marker leaking unless teardown-without-response cleanup is added — which is exactly this report's point.\n\nStep-by-step proof. (1) Client POSTs tools/call id 5 on a sessionful transport with a getStreamIdForEventId-implementing eventStore: _requestToStreamMapping[5] = S, _streamMapping[S] is the live per-request stream. (2) The handler calls ctx.http.closeSSE(): the controller is closed and _streamMapping[S] becomes the placeholder. (3) The client decides to give up and POSTs notifications/cancelled for id 5; _oncancel aborts the per-request AbortController. (4) The handler eventually settles; both completion arms in _onrequest see abortController.signal.aborted and return without calling transport.send. (5) The placeholder's cleanup() is never invoked; _streamMapping[S] and _requestToStreamMapping[5] persist until the session is closed. (6) Any later GET with a Last-Event-ID resolving to stream S hits the line-520 conflict check and gets 409 forever.\n\nFix. Fold this into the placeholder rework the existing comment already requests: remove the placeholder and the per-request bookkeeping when a request is torn down without a response (e.g. on processing notifications/cancelled for a request whose stream entry is a controller-less placeholder), or key the intentional-close marker somewhere other than _streamMapping so neither the conflict check nor the lifetime of the map is affected. A regression test that cancels a request after closeSSE() and asserts the mappings are cleared (and a subsequent Last-Event-ID resume succeeds) would lock the behavior in.

Comment thread packages/core/src/types/spec.types.2026-07-28.ts
Comment thread packages/server/src/server/streamableHttp.ts Outdated
Comment thread packages/server/src/server/streamableHttp.ts Outdated
Comment thread packages/server/src/server/streamableHttp.ts Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The latest revision addresses the issues raised in earlier review rounds (store-first with post-await re-read, identity-guarded cancel callbacks, replayedEventIds dedup on both paths, the retire-then-reconnect close, the no-eventStore onerror surface, the commit-SHA conformance pin, the schema-twin/fixture refresh to dc105208, and the doc/changeset literal fixes), and I found no further issues — but the scope (wire-visible error-code renumber plus a substantial streamableHttp lifecycle restructure) still warrants a human maintainer's review before merge.

Extended reasoning...

Overview

This PR spans 73 files across three concerns: (1) the spec#2907 error-code renumber (-32001/-32003/-32004 → -32020/-32021/-32022) touching the ProtocolErrorCode enum, HEADER_MISMATCH_ERROR_CODE, the spec.types anchor, schema twins/fixtures, emission and recognition sites, ~28 test files, docs, and changesets; (2) the conformance referee repin from a vendored tarball to a pkg.pr.new commit-SHA build with both expected-failures baselines burned down; and (3) a significant restructure of WebStandardStreamableHTTPServerTransport.send()/replayEvents() to store request-related events first (poll-and-replay support), with new dedup, identity-guard, and retire/close logic plus seven new vitest cases.

Prior review rounds

Earlier runs flagged several issues on previous revisions of the streamableHttp change (store gate vs. closeSSE cleanup, the 409 placeholder, the stale-snapshot TOCTOU, unguarded cancel deletes, duplicate delivery, missing changeset/tests) and doc/changeset literal stragglers. The current diff shows each of these addressed: the store is keyed on request-in-flight with a post-await re-read, all three ReadableStream cancel callbacks are identity-guarded, replayEvents records replayedEventIds and send() dedups against it on both the standalone and per-request paths, the !stream response branch retires bookkeeping cleanly (with onerror when no eventStore exists), the resumed stream is closed when the request was already retired, the JSON-mode resolveJson arm now calls cleanup(), the conformance pin uses an immutable commit SHA, the schema twins/fixtures/manifests were refreshed to the post-renumber spec commit, and the migration-SKILL/changeset literals were corrected. The new test suite covers the closeSSE store, hard-disconnect, reconnect/replay, retire-then-reconnect, the storeEvent-await TOCTOU (with an exactly-once assertion), the stale-cancel identity guard, and the no-eventStore onerror path.

Security risks

No direct security exposure: no auth, crypto, or permission logic is touched. The wire-visible change is limited to draft 2026-07-28 error-code values; the -32000 and the SDK-conventional -32001 session-not-found paths are verified untouched. The transport changes affect availability/correctness of SSE resumability rather than data exposure.

Level of scrutiny

High. The renumber is mechanical but wire-visible for v2-alpha consumers, and the streamableHttp restructure changes concurrency-sensitive lifecycle behavior (event-store ordering, stream-mapping ownership, reconnect semantics) on a production transport. The history of this PR — several legitimate races found and fixed across review iterations — is itself evidence that this code path deserves a human maintainer's judgment on the final design (e.g. whether the placeholder-free closeSSE + retire-close approach is the intended SEP-1699 semantics) rather than shadow approval.

Other factors

The PR description reports full local gates including conformance suites at the new pin, and the changeset/migration docs accurately describe the shipped behavior. No outstanding unresolved review threads remain; the bug-hunting system found no new issues on this revision.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No new issues from the latest pass — the earlier rounds' findings on the streamableHttp store-first/replay paths appear addressed in this revision (identity-guarded cancels, post-await re-read with replayedEventIds dedup, retire/close handling, no-eventStore onerror, plus regression tests). Given the scope — a cross-package wire-visible error-code renumber, conformance repin, and non-trivial transport lifecycle changes — this still warrants a human maintainer's review rather than auto-approval.

Extended reasoning...

Overview

This PR spans 73 files across three concerns: (1) the spec#2907 error-code renumber (-32001/-32003/-32004 → -32020/-32021/-32022) touching the ProtocolErrorCode enum, HEADER_MISMATCH_ERROR_CODE, the spec.types anchor, emission sites in createMcpHandler/serveStdio/inboundClassification/mcpParamHeaders, client probe recognition, ~28 test files, docs, and changesets; (2) repointing the conformance dependency from a vendored tarball to a pkg.pr.new commit-pinned build and burning down the renumber-pending expected-failures entries; (3) a behavior fix in WebStandardStreamableHTTPServerTransport so request-related events and final responses are stored to the eventStore while the per-request SSE stream is disconnected, with substantial follow-on changes to replayEvents()/send() (identity-guarded cancel callbacks, post-await re-reads, replayedEventIds dedup, retire-and-close of resumed streams, onerror for the no-eventStore case) and six new vitest cases.

Security risks

No direct security exposure: no auth, crypto, or permission logic is touched. The renumber is wire-visible but draft-era only. The transport changes affect resource lifecycle (stream map entries, eventStore persistence) rather than access control; the main risks are correctness/availability ones (dropped or duplicated events, leaked stream entries), which earlier review rounds probed extensively.

Level of scrutiny

High. The renumber portion is mechanical and well covered by the flipped test assertions and the conformance baselines, but the streamableHttp send()/replayEvents() restructure is concurrency-sensitive transport code that required several review iterations to converge, and the schema-twin/fixture artifacts plus the conformance pin strategy involve maintainer-level judgment (the twins/fixtures were partially refreshed relative to the anchor commit, and the pkg.pr.new pin is an interim measure). This is well outside the simple/mechanical category eligible for shadow approval.

Other factors

The latest bug-hunting pass found no new issues, and the prior inline findings (placeholder/409 conflict, TOCTOU around the awaited storeEvent, stale-cancel deletes, missing changeset, doc literals) appear addressed in the current revision with corresponding regression tests. Existing coverage is strong (full unit/integration/e2e/conformance gates reported green). I am not approving because the change is large, touches critical transport lifecycle code, and includes process decisions (conformance pin form, deferred twin refresh) that a maintainer should sign off on.

@felixweinberger felixweinberger force-pushed the fweinberger/on-renumber branch 2 times, most recently from 3b4f61a to c3936a4 Compare June 23, 2026 10:56
Comment thread test/conformance/expected-failures.yaml Outdated
@felixweinberger felixweinberger force-pushed the fweinberger/on-renumber branch from c3936a4 to 619e4d4 Compare June 23, 2026 11:18
@felixweinberger felixweinberger changed the title feat(core): adopt spec#2907 error-code renumber; conformance pin to pkg.pr.new alpha.5; streamableHttp store-first fix feat(core): adopt spec#2907 error-code renumber; streamableHttp store-first fix (referee unchanged @ alpha.4) Jun 23, 2026
Comment on lines 203 to 209
conformance: ['server-stateless'],
input: post(bare('tools/list')),
strict: true,
reject: { rung: 'era-classification', httpStatus: 400, code: -32_004, settled: true },
reject: { rung: 'era-classification', httpStatus: 400, code: -32_022, settled: true },
rationale:
'A modern-only endpoint answers envelope-less requests with the unsupported-protocol-version error and its supported list. ' +
'This cell shares its numeric code with the disputed mismatch family but is itself settled.'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 The 'modern-only-missing-envelope' row's expected code was flipped from -32_004 to -32_022 in this PR, but the rationale string two lines below still says 'This cell shares its numeric code with the disputed mismatch family but is itself settled' — after the renumber this cell answers -32022 (UnsupportedProtocolVersion) while the mismatch family answers -32020 (HeaderMismatch), so the codes are no longer shared, and the file's own header (also edited here) describes the mismatch assignments as pinned, not disputed. One-line reword/drop of the clause.

Extended reasoning...

What's stale. In packages/core/test/shared/inboundLadderCellSheet.test.ts, the modern-only-missing-envelope sheet row has its expected code flipped by this PR from -32_004 to -32_022 (line 206), but the rationale string two lines below is left unchanged: "This cell shares its numeric code with the disputed mismatch family but is itself settled." That sentence is now wrong on both of its claims.

Why both claims are now false. First, the codes are no longer shared: after this PR the missing-envelope cell asserts -32022 (UnsupportedProtocolVersion), while the header/body mismatch family rows in the same sheet (header-body-version-mismatch, method-header-mismatch, the notification variants) assert -32020 (HeaderMismatch) — both literals edited in this same diff. Second, the mismatch family is no longer "disputed": the file's own header comment (lines 6–12, also rewritten by this PR) states the previously-parameterized cells are now pinned to specific assignments, and every sibling rationale in the mismatch family was systematically updated to name -32020. This one clause is the missed straggler.

Step-by-step. (1) Pre-renumber, the clause was written when the missing-envelope cell and the mismatch family both provisionally carried -32004-adjacent codes and the assignment was under upstream discussion. (2) The pin-modern-rejection-codes change split them (mismatch family → HeaderMismatch), and this PR's spec#2907 renumber moves them further apart: -32020 vs -32022. (3) This PR edits the literal on line 206 directly above the rationale and rewrites the file header's pinned/disputed framing, but the clause on line 209 still asserts a shared code with a 'disputed' family. (4) A reader auditing the cell sheet against the renumber is told the missing-envelope cell shares its code with the mismatch family — the row's own reject.code two lines up contradicts it.

Why nothing catches it. The rationale is documentation-only prose inside the data table; no assertion reads it, so the test stays green while contradicting the assignments it annotates.

Scope note. The staleness arguably predates this PR (the codes had already diverged after pin-modern-rejection-codes), but this PR rewrites this exact row's literal and updated every sibling rationale string in the file, so cleaning up this clause belongs in the same change.

Fix. One-line reword — e.g. drop the clause entirely, or replace it with something like "This cell carries the unsupported-protocol-version code (-32022), distinct from the mismatch family's -32020, and is settled." No code or test-behavior impact.

Comment thread packages/core/test/corpus/schema-twins/manifest.json
Base automatically changed from fweinberger/on-e2e to v2-2026-07-28 June 23, 2026 11:57
… 2026-07-28

HeaderMismatch -32001 -> -32020, MissingRequiredClientCapability -32003 ->
-32021, UnsupportedProtocolVersion -32004 -> -32022. These codes are part of
the draft 2026-07-28 protocol revision only and have never appeared on a
2025-era wire — the 2025 serving paths and the SDK-conventional -32001
('Session not found') on the stateful Streamable HTTP transport are unchanged.

- ProtocolErrorCode enum members and HEADER_MISMATCH_ERROR_CODE constant
  renumbered; spec.types.2026-07-28 anchor constants follow.
- Emission sites (createMcpHandler/serveStdio/inboundClassification/mcpParamHeaders),
  the LADDER_ERROR_HTTP_STATUS table and the multi-round-trip capability gate
  emit the renumbered codes via the renumbered enum members.
- Client recognition: the probe classifier recognizes -32022 for the
  corrective continuation, NOT_PROBE_RECOGNIZED keeps -32001 (deployed
  Session-not-found overload) and adds -32020/-32021; ProtocolError.fromError
  reconstructs the typed classes from the renumbered codes; the SEP-2243
  one-refresh-on-miss retry keys on -32020.
- encodeErrorCode pass-through pin: -32000 literal untouched; the
  pass-through list updated to -32020/-32021/-32022.
- Conformance: the 0.2.0-alpha.4 referee still asserts the pre-renumber
  codes, so request-metadata / server-stateless /
  http-custom-header-server-validation / http-header-validation are added to
  expected-failures with the "SDK ahead of referee" note; they burn when the
  conformance pin bumps to a published release carrying conformance#353.
- Migration docs note the alpha-to-alpha renumber (no v1.x->v2 impact).

The vendored schema-twins/2026-07-28.schema.json and
corpus/fixtures/2026-07-28/* are deliberately untouched — they are sha256-
locked upstream artifacts at spec commit 2fb207da (pre-#2907) and refresh
atomically with the next spec-anchor repin.
@felixweinberger felixweinberger force-pushed the fweinberger/on-renumber branch from d4de428 to d5affb9 Compare June 23, 2026 11:57
The alpha.5 referee carries conformance#353 (the spec#2907 error-code
renumber), so the SDK and referee are aligned on -32020/-32021/-32022 and
the renumber-mismatch expected-failures recorded in the previous commit
burn here:

- expected-failures.yaml: drop client request-metadata; drop the entire
  server section (http-custom-header-server-validation, http-header-validation,
  server-stateless) -> server: [].
- expected-failures.2026-07-28.yaml: drop client request-metadata; drop the
  three server renumber cells (json-schema-2020-12 stays).

alpha.5 also carries conformance#359 which gates the SEP-2350 scope-step-up
WARNING to 2026-07-28+, so auth/scope-step-up burns on the 2025 client leg
(it remains in the 2026-07-28 baseline where it still fails on the 2026
connection-lifecycle gap).

alpha.5 ships the conformance#262 SEP-2663 tasks-* scenarios, but they are
extension-sourced (source.extensionId io.modelcontextprotocol/tasks) and do
not run against a server that does not advertise the extension, so no new
expected-failures entries are needed; they will appear when ext-tasks lands.

Header NOTE in both files flipped from 'AHEAD of this referee' to 'aligned
with what this referee asserts'. README unchanged (does not reference the
pin version).
@felixweinberger felixweinberger force-pushed the fweinberger/on-renumber branch from d5affb9 to b9bbcda Compare June 23, 2026 12:15
@felixweinberger felixweinberger changed the title feat(core): adopt spec#2907 error-code renumber; streamableHttp store-first fix (referee unchanged @ alpha.4) feat(core): adopt spec#2907 error-code renumber; conformance referee → 0.2.0-alpha.5 Jun 23, 2026
Comment on lines +18 to +21
# NOTE: the SDK's modern-path rejection codes are aligned with what this
# referee asserts — both sides have adopted the spec#2907 / conformance#353
# renumber (-32020 / -32021 / -32022) on emission and recognition.
#

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 The waiver rationale at lines 46-48 still says the 2026-mode mock rejects the MCP POST with -32001, but at the 0.2.0-alpha.5 referee this PR pins, that missing-MCP-Protocol-Version-header rejection is emitted as -32020 (HeaderMismatch) — the alpha.5 dist contains no -32001 — so the literal now contradicts the NOTE this same diff adds at lines 18-21. Comment-only: change -32001 to -32020 (or reword to "HeaderMismatch").

Extended reasoning...

What's stale. This PR bumps the conformance referee pin to 0.2.0-alpha.5 and adds the NOTE at lines 18-21 of this file stating that both sides have adopted the spec#2907 / conformance#353 renumber (-32020 / -32021 / -32022) on emission and recognition. A few lines below, the pre-existing waiver rationale for the "Auth scenarios cut short by the 2026 connection lifecycle" group (lines 46-48) still reads: "the 2026-mode mock rejects the MCP POST (-32001, missing MCP-Protocol-Version header)". A missing required MCP-Protocol-Version header is exactly the HeaderMismatch family the renumber moves from -32001 to -32020, so at the new pin this parenthetical is the pre-renumber literal.

Why the literal is wrong at the new pin. Verifiers downloaded the @modelcontextprotocol/conformance@0.2.0-alpha.5 tarball this PR pins (the same version recorded in test/conformance/package.json and pnpm-lock.yaml) and inspected its dist: the 2026-mode mock's request gate now rejects a POST lacking the header with c(400, -32020, 'Missing MCP-Protocol-Version header'), and the literal -32001 does not appear anywhere in the alpha.5 dist (only -32020/-32021/-32022 and the unrelated -32002/-32602 etc.). The spec.types.2026-07-28.ts regenerated in this same PR likewise defines HEADER_MISMATCH = -32020 covering missing/malformed required headers.

Why it matters. The file is now internally inconsistent: the header NOTE asserts both sides have adopted the renumber, while a waiver rationale 25 lines later attributes a pre-renumber code to the very referee the NOTE describes. The file's own header (lines 14-16) states that newer referee releases are adopted by "bumping the pin and reconciling this file in the same change", so this reconciliation belongs in this PR. A reader auditing the auth waivers against the alpha.5 referee will look for a -32001 rejection that no longer exists.

Step-by-step. (1) The auth fixture drives the 2025 stateful lifecycle, so its MCP POST carries no MCP-Protocol-Version header. (2) Pre-PR, the alpha.4 2026-mode mock rejected that POST with -32001 — the rationale was accurate when written. (3) This PR pins alpha.5, whose mock emits -32020 for the same rejection. (4) The waiver entries themselves (auth/scope-step-up, auth/scope-retry-limit) remain correct — the substantive reason for the failure (the fixture speaks the 2025 lifecycle, so no authorization requests are observed) is unchanged — only the numeric literal in the rationale is stale.

Why nothing catches it. The rationale is a YAML comment; the conformance runner only validates the scenario-name entries against actual results, so prose staleness is invisible to CI.

Fix. One-word edit on line 47: -32001-32020, or reword the parenthetical to "HeaderMismatch, missing MCP-Protocol-Version header". No behavioral impact — comment-only, should not block the merge.

@felixweinberger felixweinberger merged commit 3d88907 into v2-2026-07-28 Jun 23, 2026
19 checks passed
@felixweinberger felixweinberger deleted the fweinberger/on-renumber branch June 23, 2026 12:53
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.

1 participant