feat(core): adopt spec#2907 error-code renumber; conformance pin to pkg.pr.new alpha.5; streamableHttp store-first fix#2335
Conversation
🦋 Changeset detectedLatest commit: fc38188 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 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/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
felixweinberger
force-pushed
the
fweinberger/on-renumber
branch
from
f61a634 to
308c912
Compare
felixweinberger
force-pushed
the
fweinberger/on-e2e
branch
from
d81cc51 to
5725817
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (!this._enableJsonResponse) { | ||
| // Store FIRST so request-related events emitted while the per-request | ||
| // stream is disconnected (e.g. after `closeSSE()`) are replayed on | ||
| // reconnect — same store-first semantics as the standalone path above. | ||
| let eventId: string | undefined; | ||
|
|
||
| if (this._eventStore) { | ||
| eventId = await this._eventStore.storeEvent(streamId, message); | ||
| } | ||
| // Write the event to the response stream | ||
| this.writeSSEEvent(stream.controller, stream.encoder, message, eventId); | ||
| if (stream?.controller && stream?.encoder) { | ||
| // Write the event to the response stream | ||
| this.writeSSEEvent(stream.controller, stream.encoder, message, eventId); | ||
| } | ||
| } | ||
|
|
||
| if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) { |
There was a problem hiding this comment.
🔴 The new store-first block now persists the final JSON-RPC response to the eventStore when the per-request SSE stream is disconnected (e.g. after closeSSEStream()), but the response branch below still hits if (!stream) throw new Error('No connection established for request ID...') — so the response is stored and replayable, yet send() reports a failure and the cleanup loop deleting the _requestResponseMap/_requestToStreamMapping entries is skipped. The disconnected-response case should mirror the standalone path (lines 984–988) and return cleanly after storing instead of store-then-throw, so the Protocol layer doesn't surface a spurious send error and the per-request bookkeeping doesn't leak.
Extended reasoning...
The bug. closeSSEStream(requestId) (streamableHttp.ts:939–947) calls stream.cleanup(), which removes the entry from _streamMapping but leaves _requestToStreamMapping intact. When the handler later returns and the final JSON-RPC result/error response is sent while the client has not yet reconnected, send() resolves streamId (line 998) but stream === undefined (line 1003). With this PR, the new store-first block (lines 1005–1017) correctly stores the response to the eventStore — good, it will be replayed on a Last-Event-ID reconnect via replayEvents(). But execution then falls into the response branch: _requestResponseMap.set() runs, allResponsesReady is true, and the unchanged if (!stream) throw new Error('No connection established for request ID...') at lines 1027–1028 fires.
Why this is half-applied state. The throw propagates out of transport.send(response); in core/protocol.ts the response send is wrapped and surfaced as onerror('Failed to send response: ...'), so the server reports a send failure for a response that is in fact stored and deliverable. Worse, the throw skips the cleanup loop at lines 1051–1054, leaking the _requestResponseMap and _requestToStreamMapping entries for that request for the lifetime of this long-lived sessionful transport — and a later resumed stream replaying that response is never cleanup()'d by this path either.
Why it's squarely in this PR's scope. The standalone path that the new comment claims parity with ('same store-first semantics as the standalone path above') handles the disconnected case by returning cleanly after storing (lines 984–988). The request-related path stores then throws. The throw itself is pre-existing, but the PR rewrites exactly this block and its stated purpose is the disconnected-emission scenario (the sse-polling example: ctx.http.closeSSE(), then emit while the client hasn't re-polled), so this is the natural completion of the fix rather than an unrelated pre-existing issue. Pre-PR the same scenario also threw, but the response was lost entirely; post-PR the response survives but the transport still misreports the send and leaks bookkeeping.
Step-by-step proof.
- Client POSTs a
tools/call(request id 5) on a sessionful transport with aneventStore;_requestToStreamMapping[5] = streamId,_streamMapping[streamId] = stream. - The tool handler calls
ctx.http.closeSSE()→closeSSEStream(5)→stream.cleanup()deletes_streamMapping[streamId]._requestToStreamMapping[5]remains. - The handler finishes before the client's retry interval elapses and Protocol sends the final result via
transport.send(response). - In
send():requestId = 5,streamIdresolves,stream = undefined. The new block stores the response to the eventStore (line 1011) — replayable. - The response branch sets
_requestResponseMap[5],allResponsesReady = true, then!stream→ throw (line 1027–1028). - Protocol catches the rejection and emits
onerror('Failed to send response: ...'); the cleanup loop (1051–1054) never runs, so_requestResponseMap[5]and_requestToStreamMapping[5]leak. - The client reconnects with
Last-Event-IDand receives the response via replay — so the client got the answer the server believes it failed to send.
Fix. In the response branch, when !stream and the event store has the response (the disconnected case this PR now supports), skip the stream end, run the cleanup loop, and return without throwing — mirroring the standalone path's return-after-store at lines 984–988. (Keep the throw for the no-event-store case where the response really is undeliverable, if desired.)
| if (!this._enableJsonResponse) { | ||
| // Store FIRST so request-related events emitted while the per-request | ||
| // stream is disconnected (e.g. after `closeSSE()`) are replayed on | ||
| // reconnect — same store-first semantics as the standalone path above. | ||
| let eventId: string | undefined; | ||
|
|
||
| if (this._eventStore) { | ||
| eventId = await this._eventStore.storeEvent(streamId, message); | ||
| } | ||
| // Write the event to the response stream | ||
| this.writeSSEEvent(stream.controller, stream.encoder, message, eventId); | ||
| if (stream?.controller && stream?.encoder) { | ||
| // Write the event to the response stream | ||
| this.writeSSEEvent(stream.controller, stream.encoder, message, eventId); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 This PR's streamableHttp store-first change is a user-visible bugfix in @modelcontextprotocol/server (request-related events emitted while the per-request SSE stream is disconnected are now stored to the eventStore and replayed on reconnect, instead of being silently dropped), but no changeset records it — the only new changeset (.changeset/spec-2907-error-code-renumber.md) covers the error-code renumber exclusively. Consider adding a one-line patch changeset for @modelcontextprotocol/server so the fix appears in the release changelog, matching the convention used for similar transport fixes.
Extended reasoning...
What's missing
The change at packages/server/src/server/streamableHttp.ts:1005-1017 is a genuine behavior fix in WebStandardStreamableHTTPServerTransport.send(): previously, request-related events were only stored to the configured eventStore when the per-request stream controller was live (stream?.controller && stream?.encoder), so any notification emitted after ctx.http.closeSSE() (e.g. progress, ctx.mcpReq.notify, ctx.mcpReq.log) was silently dropped. After this PR, the event is stored first and only then written if the stream is connected, so it can be replayed on reconnect — the same store-first semantics the standalone-stream path already uses. The PR description itself calls this out as a standalone, cherry-pickable bugfix commit.
Why a changeset is expected
The repository follows a changeset-per-behavior-change convention: small transport bugfixes consistently get their own patch changeset (e.g. fix-streamable-close-reentrant.md, fix-streamable-http-error-response.md, server-ctx-log-request-related.md, bound-resumability-version-gates.md), and the changeset-bot generates the release notes for @modelcontextprotocol/server from these files. This PR even adds a changeset for the alpha-to-alpha error-code renumber, so the convention clearly applies here too.
Verification that nothing covers it
- The only new changeset added by this PR,
.changeset/spec-2907-error-code-renumber.md, describes exclusively the-32020/-32021/-32022renumber. - The 8 retro-aligned changesets (envelope-auto-emission, missing-client-capability-error, mrtr-server-seam, pin-modern-rejection-codes, sep-2243-*, spec-anchor-repin, spec-reference-types) only flip code literals — none mentions storing events while the stream is disconnected.
- A search of the existing
.changeset/directory for eventStore / replay / closeSSE / streamable mentions finds no entry covering this behavior either (bound-resumability-version-gates.mdandserver-ctx-log-request-related.mddescribe unrelated behavior).
Concrete impact
Walk-through of what a consumer sees: (1) a server tool handler calls ctx.http.closeSSE(), then later emits a log/progress notification for that request; (2) on the previous release that event vanished, on this release it is persisted to the eventStore and replayed when the client reconnects with Last-Event-ID; (3) a consumer scanning the @modelcontextprotocol/server changelog after the next version bump (the changeset-bot already reports this PR will release 8 packages) finds no mention of this behavior change, even though it alters what events their event stores receive and what their clients see on reconnect.
Fix
Add a one-line patch changeset, e.g. .changeset/streamable-http-store-first.md:
---
'@modelcontextprotocol/server': patch
---
WebStandardStreamableHTTPServerTransport now stores request-related events to the eventStore even while the per-request SSE stream is disconnected (e.g. after closeSSE()), so they are replayed on reconnect instead of being dropped.This is a process/documentation gap only — the code change itself is correct and tested — so it should not block the merge.
… 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: request-metadata / server-stateless /
http-custom-header-server-validation / http-header-validation now pass; the
'renumber pending Felix ruling' entries are burned from both
expected-failures yamls and the header NOTE rewritten to reflect alignment.
- 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.
…build (#357) Replace the file:./vendor/*.tgz pin (built locally from conformance@d70d7ad) with the pkg.pr.new build of conformance#357 (commit 65fcd39 — the 0.2.0-alpha.5 version bump on top of main@d70d7ad, carrying #347 + #353). Content-identical to the previous referee; expected-failures baselines are unchanged. - test/conformance/package.json: @modelcontextprotocol/conformance → https://pkg.pr.new/@modelcontextprotocol/conformance@357 - test/conformance/vendor/: deleted (tarball + .gitignore override) - README.md: drop local-build/tarball/sha256 instructions; note pkg.pr.new pin pending the npm publish (switch to ^0.2.0-alpha.5 once published) - expected-failures.yaml: header pin → 0.2.0-alpha.5 (via pkg.pr.new #357) - pnpm-lock.yaml: regenerated
… is disconnected The standalone-SSE path stores to eventStore first, then writes if connected. The request-related path only stored when the stream was live, so a notification sent after closeSSE() (SEP-1699 polling) was silently dropped instead of being persisted for replay on reconnect. Exposed by the ctx.mcpReq.log request-related change against the new sse-polling example story; the gap pre-exists on main for any request-related notification (progress, ctx.mcpReq.notify) emitted after closeSSE().
felixweinberger
force-pushed
the
fweinberger/on-renumber
branch
from
308c912 to
fc38188
Compare
felixweinberger
force-pushed
the
fweinberger/on-e2e
branch
from
5725817 to
9fcdcc2
Compare
| --- | ||
|
|
||
| Pin the modern (2026-07-28) HTTP serving path's rejection codes to the assignments the published conformance suite asserts: a header/body cross-check mismatch (`MCP-Protocol-Version` or `Mcp-Method` disagreeing with the request body) is now rejected with `-32001` (HeaderMismatch), and a request whose protocol-version header names a modern revision but whose body is missing the `_meta` envelope (or its required protocol-version key) is rejected with `-32602` invalid params naming the missing key(s). Both keep HTTP 400. These cells previously emitted a provisional `-32004` while the upstream error-code discussion was open. The envelope-less rejection on a modern-only endpoint (`-32004` with the supported-versions list), the 2025-era serving paths, and the client-side probe handling are unchanged. | ||
| Pin the modern (2026-07-28) HTTP serving path's rejection codes to the assignments the published conformance suite asserts: a header/body cross-check mismatch (`MCP-Protocol-Version` or `Mcp-Method` disagreeing with the request body) is now rejected with `-32020` (HeaderMismatch), and a request whose protocol-version header names a modern revision but whose body is missing the `_meta` envelope (or its required protocol-version key) is rejected with `-32602` invalid params naming the missing key(s). Both keep HTTP 400. These cells previously emitted a provisional `-32022` while the upstream error-code discussion was open. The envelope-less rejection on a modern-only endpoint (`-32022` with the supported-versions list), the 2025-era serving paths, and the client-side probe handling are unchanged. |
There was a problem hiding this comment.
🟡 The blanket -32004→-32022 retro-edit in this changeset rewrote a backward-looking sentence: "These cells previously emitted a provisional -32022…" — but the header/body-mismatch cells previously emitted the provisional -32004; no build has ever emitted -32022 for them, and -32022 also appears in the next sentence as the current (unchanged) envelope-less rejection code, which makes the changelog entry confusing. Keep -32004 in the historical clause (or reword to "the provisional pre-renumber unsupported-protocol-version code").
Extended reasoning...
What's wrong. The PR retro-aligns the 8 pre-existing alpha changesets by replacing the old error-code literals with the renumbered ones. For most of those changesets that's correct, because the sentences describe what the release will ship. But in .changeset/pin-modern-rejection-codes.md (line 6) the replacement landed inside a sentence that describes history: the pre-PR text read "These cells previously emitted a provisional -32004 while the upstream error-code discussion was open", which was accurate — the header/body-mismatch cells provisionally borrowed the then-current UnsupportedProtocolVersion wire code -32004 in earlier v2 alpha builds before being pinned to HeaderMismatch. After the edit it claims those cells "previously emitted a provisional -32022", which is false: -32022 only exists as of this PR's spec#2907 renumber and was never emitted by any build for those cells.
Why it's confusing in context. Within the same paragraph, -32022 now appears twice with two different meanings: first as the code the mismatch cells used to emit (false), then in the very next sentence as the current, unchanged code for the envelope-less rejection on a modern-only endpoint (true). It also reads oddly next to the new spec-2907-error-code-renumber.md changeset in the same release, which correctly states the -32004 → -32022 transition for UnsupportedProtocolVersion.
Concrete walkthrough. (1) An earlier v2 alpha emitted -32004 for a header/body protocol-version mismatch on the modern path. (2) The pin-modern-rejection-codes change (already pending in this changeset) moved those cells to HeaderMismatch — at that time -32001, now -32020 after this PR. (3) A consumer who hard-coded -32004 for the mismatch cells reads the generated changelog after the next version bump: the entry tells them those cells "previously emitted a provisional -32022", a code their build never saw, while the migration note elsewhere in this PR correctly says -32004 → -32022 is the UnsupportedProtocolVersion renumber. The code-transition story for the mismatch cells comes out garbled.
On the counter-argument that the retro-alignment is deliberate and harmless. The retro-alignment of forward-looking statements is indeed deliberate and correct, and the changesets do publish atomically — but that argument applies to sentences describing what the release ships, not to a sentence whose subject is what earlier interim builds emitted. "Previously emitted a provisional -32022" is simply not true under any reading; referring to the historical interim code by its post-renumber identity would at minimum need rewording (e.g. "previously borrowed the provisional unsupported-protocol-version code") rather than a bare numeric literal that names a value that did not exist yet. The PR itself treats the renumber as wire-visible for v2-alpha consumers (the migration note tells them to update hard-coded literals), so changelog readers who care about these numbers are real.
Fix. One-word change: keep -32004 in that historical sentence, or reword it to avoid a numeric literal entirely ("These cells previously emitted the provisional pre-renumber unsupported-protocol-version code while the upstream error-code discussion was open"). All other claims in the changeset (the -32020 HeaderMismatch pin, the -32602 envelope rejection, the unchanged -32022 envelope-less cell) match the implementation in this diff.
This is changelog-prose accuracy only — code, tests, and conformance baselines are all consistent — so it's a nit and shouldn't block the merge.
| }, | ||
| "devDependencies": { | ||
| "@modelcontextprotocol/conformance": "file:./vendor/modelcontextprotocol-conformance-0.2.0-main.d70d7ad.tgz", | ||
| "@modelcontextprotocol/conformance": "https://pkg.pr.new/@modelcontextprotocol/conformance@357", |
There was a problem hiding this comment.
🟡 The conformance dependency is pinned to https://pkg.pr.new/@modelcontextprotocol/conformance@357, a PR-number ref that pkg.pr.new re-resolves to the latest build pushed to conformance#357 — which is why the README has to name commit 65fcd39 in prose. Since pnpm-lock.yaml records this URL with an integrity hash for the current alpha.5 tarball, any new commit on that upstream PR will break fresh/frozen-lockfile installs with a tarball-integrity mismatch (or, if the lockfile is regenerated, silently swap the referee under the existing expected-failures baselines). Use the immutable commit-build URL (e.g. ...@65fcd39) until the npm ^0.2.0-alpha.5 publish lands.
Extended reasoning...
The issue
test/conformance/package.json now resolves @modelcontextprotocol/conformance from https://pkg.pr.new/@modelcontextprotocol/conformance@357. pkg.pr.new supports two URL ref forms: @<commit-sha> (immutable, tied to one published build) and @<PR-number> (mutable — it always serves the latest build published for that upstream PR). The @357 form chosen here is the mutable one: the artifact behind the URL changes whenever a new commit is pushed to modelcontextprotocol/conformance#357, which is entirely outside this repo's control while that PR is still open awaiting the npm publish.
Evidence inside this PR
The new README block has to document "commit 65fcd39" out-of-band in prose precisely because the URL itself cannot encode it — the same README also states the pin philosophy is an exact pin ("pinned to an exact version (no ^ range)... adopted by deliberately bumping the pin"), which a PR-number ref does not satisfy. The vendored tarball this PR deletes existed for exactly this reason: the referee bytes were committed and sha256-documented. And the pkg-pr-new bot comment on this very PR shows the same behavior for this repo's own packages: the ...@2335 (PR-number) URLs stay constant across pushes while the bot separately annotates the current commit (fc38188), i.e. the same URL re-resolves to new content on every push.
Concrete failure walkthrough
pnpm-lock.yamlrecords the resolution ashttps://pkg.pr.new/@modelcontextprotocol/conformance@357withintegrity: sha512-lBpIgubk...computed from the current 0.2.0-alpha.5 tarball (commit65fcd39).- A maintainer pushes another commit to conformance#357 upstream (plausible while the PR is open). pkg.pr.new publishes a new build and the
@357URL now serves a different tarball. - Every fresh
pnpm install/--frozen-lockfileCI run in this repo that installs the conformance workspace now fails with a tarball-integrity mismatch — loud, but triggered entirely by an external event with no change in this repo. - Alternatively, if someone regenerates the lockfile to "fix" it, the conformance referee silently changes underneath the existing
expected-failures*.yamlbaselines without a deliberate repin — the exact property the deleted vendored-tarball pin was protecting against.
Why nothing prevents this
pnpm stores tarball integrity for URL dependencies and verifies it on install; there is no mechanism that re-pins the URL to the commit the README names. The integrity hash protects against silent swaps (good) but converts an upstream push into an install failure here.
Fix
One-line change: point the dependency at the immutable commit-build URL — https://pkg.pr.new/@modelcontextprotocol/conformance@65fcd39 — which is the same 0.2.0-alpha.5 build the README and PR description already name, and keep the planned switch to ^0.2.0-alpha.5 once the npm release exists. This is dev/CI tooling reproducibility only (no runtime/SDK impact) and the pin is explicitly temporary, so it should not block the merge.
| - `notifications/elicitation/complete` is no longer part of the 2026-07-28 wire registry (the draft removed the notification together with `elicitationId` on URL-mode elicitation). On connections negotiated at 2026-07-28, sending it — including via `Server.createElicitationCompletionNotifier()` — now fails locally with `SdkErrorCode.MethodNotSupportedByProtocolVersion`, and inbound copies are dropped as unknown notifications. Both remain fully supported on 2025-11-25. | ||
| - `notifications/cancelled` on 2026-era connections now parses with a revision-exact schema that requires `requestId` (the draft made it required); the notification `_meta` shape types the `io.modelcontextprotocol/subscriptionId` key. 2025-era parsing is unchanged. | ||
| - The error code `-32001` emitted for HTTP header/body mismatches is now defined by the draft schema (`HEADER_MISMATCH`); the emitted behavior is unchanged (documentation only). | ||
| - The error code `-32020` emitted for HTTP header/body mismatches is now defined by the draft schema (`HEADER_MISMATCH`); the emitted behavior is unchanged (documentation only). |
There was a problem hiding this comment.
🟡 The blanket -32001→-32020 retro-edit also rewrote this anchored sentence, but this changeset documents the re-pin to spec commit 2fb207da — a pre-spec#2907 anchor where the draft schema's HEADER_MISMATCH constant was -32001, and the vendored schema-twins/2026-07-28.schema.json this PR deliberately leaves at that anchor still defines -32001 (no in-repo schema artifact defines -32020). Keep -32001 in this anchored sentence or drop the numeric literal; the "emitted behavior is unchanged (documentation only)" parenthetical also reads contradictory next to the sibling renumber changeset once the literal is flipped. Same class of retro-edit issue as the already-flagged pin-modern-rejection-codes.md sentence, but a separate instance in a different file.
Extended reasoning...
What's wrong. This PR retro-aligns 8 pre-existing alpha changesets by replacing the old 2026-07-28 error-code literals with the renumbered ones. That is correct for forward-looking sentences describing what the release will ship, but line 11 of .changeset/spec-anchor-repin-2fb207da.md is anchored to a specific artifact and spec commit: it documents the re-pin of the 2026-07-28 draft references to spec commit 2fb207da, and says the header/body-mismatch error code "is now defined by the draft schema (HEADER_MISMATCH)". At that anchor — which predates spec#2907 — the draft schema defines HEADER_MISMATCH as -32001, not -32020.
Why the in-repo artifact contradicts the new wording. The vendored schema twin produced by this re-pin, packages/core/test/corpus/schema-twins/2026-07-28.schema.json, still defines HeaderMismatchError with "const": -32001 (line 1149), and -32020 does not appear anywhere in that file. The PR description explicitly states these sha256-locked artifacts are deliberately left at the pre-#2907 anchor, to be refreshed atomically with the next repin. So after the retro-edit, the changeset whose subject is precisely that anchor claims "-32020 ... is now defined by the draft schema", a statement not backed by the artifact the changeset is about.
The parenthetical becomes contradictory. The original sentence's "the emitted behavior is unchanged (documentation only)" was accurate for -32001: the SDK already emitted -32001 for header/body mismatches, and the re-pin merely gave it schema backing. Flipped to -32020, the same parenthetical now sits next to the new spec-2907-error-code-renumber.md changeset in the same release, which describes exactly the wire-visible -32001 → -32020 change. A changelog reader sees one entry calling -32020 a wire-visible renumber and another calling it documentation-only.
Step-by-step proof. (1) The re-pin commit this changeset documents pinned the draft references to spec commit 2fb207da428f... (the manifest commit for the schema twin). (2) At that commit the schema's HeaderMismatchError constant is -32001 — confirmed by the vendored twin at packages/core/test/corpus/schema-twins/2026-07-28.schema.json:1149, which this PR does not touch. (3) The retro-edit replaced the literal in line 11, so the changeset now attributes -32020 to a schema anchor that defines -32001. (4) When the changesets are consumed into the release changelog, this entry says -32020 was "documentation only" while the renumber entry says -32020 is the new wire code replacing -32001 — the code-history story for HeaderMismatch comes out garbled.
Why this is not a duplicate. The existing inline PR comment covers .changeset/pin-modern-rejection-codes.md line 6 (the "previously emitted a provisional -32022" sentence). This is the same class of issue — a backward-looking/anchored sentence renumbered as if it were forward-looking — but a distinct surviving instance in a different file and sentence.
Fix. One-line reword: keep -32001 in this anchored sentence ("The error code -32001 emitted for HTTP header/body mismatches is now defined by the draft schema (HEADER_MISMATCH)..."), or drop the numeric literal entirely ("the header-mismatch code is now defined by the draft schema (HEADER_MISMATCH)"). Changelog-prose accuracy only — no code, test, or conformance impact — so this should not block the merge.
| if (!this._enableJsonResponse) { | ||
| // Store FIRST so request-related events emitted while the per-request | ||
| // stream is disconnected (e.g. after `closeSSE()`) are replayed on | ||
| // reconnect — same store-first semantics as the standalone path above. | ||
| let eventId: string | undefined; | ||
|
|
||
| if (this._eventStore) { | ||
| eventId = await this._eventStore.storeEvent(streamId, message); | ||
| } | ||
| // Write the event to the response stream | ||
| this.writeSSEEvent(stream.controller, stream.encoder, message, eventId); | ||
| if (stream?.controller && stream?.encoder) { | ||
| // Write the event to the response stream | ||
| this.writeSSEEvent(stream.controller, stream.encoder, message, eventId); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 The store-first behavior change in WebStandardStreamableHTTPServerTransport.send() (storing request-related events to the eventStore while the per-request SSE stream is disconnected, e.g. after closeSSE()) ships with no vitest coverage — the only validation cited is the sse-polling example story under run:examples. Consider adding a streamableHttp.test.ts case that emits a request-related notification (and the final response) after closeSSEStream() and asserts it is persisted to the eventStore and replayed on a Last-Event-ID reconnect, per the repo checklist's "New behavior has vitest coverage including error paths".
Extended reasoning...
What's missing
The tip commit changes send() in packages/server/src/server/streamableHttp.ts:1005-1017 so that, on the request-related path, the event is stored to the configured eventStore before checking whether the per-request stream controller is live. Pre-PR, the store only happened inside if (!this._enableJsonResponse && stream?.controller && stream?.encoder), so any notification or final response emitted after ctx.http.closeSSE() (while the client had not yet re-polled) was silently dropped. That is a genuine new behavior — yet the PR's changed-files list contains no streamableHttp test file and no test anywhere in the diff exercises the disconnected-emission path.
Why the existing tests don't cover it
The "Event Store (Resumability)" suite in packages/server/test/server/streamableHttp.test.ts:607-708 has exactly two cases: "should store events when event store is configured" and "should include event ID in SSE events". Both run tools/list against a live per-request stream — neither test ever calls closeSSEStream() (or otherwise tears the stream down) before an event is emitted, and neither asserts replay on a Last-Event-ID reconnect. The closest e2e coverage (hosting:resume:close-stream) reconnects with last-event-id before the deferred event fires, so the response is delivered on the re-established stream — that scenario would have passed pre-fix and does not guard the new behavior either. The only validation the PR description cites for this change is the sse-polling example story under pnpm run:examples, which is an example smoke run, not a vitest regression guard.
Why it matters
The repository checklist requires that "New behavior has vitest coverage including error paths". Without a unit test, the exact gap this commit closes — store only when the stream is live — could silently regress in a future refactor of send() (the conditional structure there has already been reshuffled once by this PR). The related disconnected-response edge (the store-then-throw path flagged in a separate review comment) likewise has no error-path coverage, so a follow-up fix there would also land untested.
Step-by-step proof that the new path is untested
- The new code path is entered only when
requestIdresolves astreamIdbutthis._streamMapping.get(streamId)returnsundefined— i.e. the per-request stream was cleaned up (viacloseSSEStream()/ctx.http.closeSSE()) beforesend()runs. - In
streamableHttp.test.ts, the eventStore suite's tests (lines 687-707) issue a plaintools/listPOST and read the SSE response immediately; the stream stays connected for the entire request, sostream?.controller && stream?.encoderis truthy and the pre-existing connected-store branch is what executes. - No test in the file (or anywhere in the PR) calls
closeSSEStream, closes the per-request response stream mid-request, or issues aGETwithLast-Event-IDto assert replay of an event stored while disconnected. - Therefore reverting the production change (restoring the old
stream?.controller && stream?.encoderguard aroundstoreEvent) would leave the entire vitest suite green — the regression guard does not exist.
How to fix
Add one case to the Event Store suite: register a tool whose handler calls the transport's closeSSEStream(requestId) (or use the ctx.http.closeSSE() surface) and then emits a request-related logging/progress notification before returning its result; assert (a) the notification and the final response appear in the stub eventStore's storedEvents, and (b) a follow-up GET with Last-Event-ID set to the priming event replays them. That single test covers both halves of the new behavior (notification + final response) and turns the example-story validation into a durable vitest guard.
This is a test-coverage/process gap only — the production change itself looks correct — so it shouldn't block the merge.
1 participant
Adopts the modelcontextprotocol/modelcontextprotocol#2907 / modelcontextprotocol/conformance#353 draft error-code renumber, swaps the conformance pin to the
pkg.pr.newalpha.5 build, and fixes a baseline gap instreamableHttprequest-related event storage. Stacked onfweinberger/on-e2e.Motivation and Context
spec#2907 error-code renumber
HeaderMismatch−32001 → −32020,MissingRequiredClientCapability−32003 → −32021,UnsupportedProtocolVersion−32004 → −32022.ProtocolErrorCodeenum,HEADER_MISMATCH_ERROR_CODE, and thespec.types.2026-07-28anchor constants renumberedcreateMcpHandler,serveStdio,inboundClassification,mcpParamHeaders,LADDER_ERROR_HTTP_STATUS, the multi-round-trip capability gate) and client recognition (probeClassifierUNSUPPORTED_PROTOCOL_VERSION= −32022;NOT_PROBE_RECOGNIZEDkeeps the deployed −32001 Session-not-found overload and adds −32020/−32021;ProtocolError.fromErrortyped-class reconstruction; SEP-2243 one-refresh-on-miss keys on −32020) follow via the enumstreamableHttp/createMcpHandler/originValidation/hostHeaderValidation/inboundClassification); theencodeErrorCodepass-through pin still holds for −32000, list updated to −32020/−32021/−32022streamableHttp, examples, e2e, conformance fixtures) and v1RequestTimeout/ConnectionClosedhistory inmigration.mdleft as-isschema-twins/2026-07-28.schema.json+corpus/fixtures/2026-07-28/*deliberately not touched (sha256-locked at a pre-#2907 spec anchor; refresh atomically with the next repin)expected-failures*.yamlburned (the renumber-pending entries removed); header NOTE rewritten to "aligned";migration.md/migration-SKILL.mdalpha-to-alpha note addedConformance pin → pkg.pr.new alpha.5
@modelcontextprotocol/conformancerepointed from the vendoredfile:./vendor/...d70d7ad.tgztohttps://pkg.pr.new/@modelcontextprotocol/conformance@357(commit65fcd39= the0.2.0-alpha.5version bump on top ofmain@d70d7ad; carries #347 + #353).test/conformance/vendor/deleted; README local-pin block rewritten (drops the tarball sha256 / rebuild recipe; switch to^0.2.0-alpha.5once published on npm).expected-failures.yamlheader pin updated. Referee content-identical tod70d7ad— no baseline reconcile needed.streamableHttpstore-first for request-related eventsThe
sse-pollingexample (callctx.http.closeSSE()then log while disconnected) exposed a baseline gap:streamableHttp.tssend()only stored request-related events toeventStorewhen the stream controller was live, so events emitted aftercloseSSEwere dropped — unlike the standalone path which stores first. Fixed to store-first, then write-if-connected.How Has This Been Tested?
Full gates at tip: typecheck, lint, build:all, docs:check; core (1210), server (334), client (536), codemod (350), server-legacy (162), express (40), fastify (22), hono (12), node (88), examples-shared (2); integration (348); e2e (2490p/205xf);
pnpm run:examples24 stories / 63 legs / 0 failed; conformanceclient:all(348p/12xf baseline ok,request-metadatapasses),server(42/0),server:draft(81/0),client:2026(baseline ok,request-metadata7/0),server:2026(baseline ok,server-stateless26/0,http-header-validation13/0,http-custom-header-server-validation9/0).Breaking Changes
None at the API surface. Wire-level: the 2026-07-28 era now emits −32020/−32021/−32022 where it previously emitted −32001/−32003/−32004 (alpha-to-alpha; covered by changeset + migration note).
Types of changes
Checklist
Additional context
60 files, +258/−257 in the renumber commit. The
streamableHttpstore-first fix is a standalone tip commit so it can be cherry-picked independently if preferred.