feat(server): createRequestStateCodec; pin conformance to main@d70d7ad; toNodeHandler onerror + ctx.log spec-align#2332
Conversation
🦋 Changeset detectedLatest commit: f4e4654 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| * | ||
| * Verification is fail-closed and constant-time (WebCrypto `subtle.verify`). | ||
| * See `examples/server/src/multiRoundTrip.ts` for a worked end-to-end example. | ||
| */ |
There was a problem hiding this comment.
🟡 Both the new JSDoc on createRequestStateCodec and the rewritten requestState section in docs/migration.md point readers to examples/server/src/multiRoundTrip.ts, but that file (and the examples/server/ directory) does not exist in the repo. The actual worked end-to-end example — which this PR itself updates to use the codec — is examples/mrtr/server.ts, so both references should point there instead.
Extended reasoning...
What the bug is. The JSDoc for createRequestStateCodec (packages/server/src/server/requestStateCodec.ts:116-119) ends with "See examples/server/src/multiRoundTrip.ts for a worked end-to-end example." The rewritten requestState paragraph in docs/migration.md (~line 1246) ends with the same sentence. Neither path resolves: there is no examples/server/ directory in the repo at all, so the pointer is dead in both places.
How it got here. The JSDoc reference is brand-new in this PR (the entire requestStateCodec.ts file is added by this diff). The migration.md sentence is also re-authored by this PR — the old text said "worked HMAC example", the new text says "worked end-to-end example" — but the dangling path was carried over unchanged. The examples tree was previously reorganized into per-story directories, and the multi-round-trip example now lives at examples/mrtr/server.ts (with its client at examples/mrtr/client.ts).
Step-by-step proof.
ls examples/showsmrtr/,tools/,sampling/, etc. — there is noserver/subdirectory, soexamples/server/src/multiRoundTrip.tscannot exist.grep -r multiRoundTripacross the repo matches only the two referencing locations themselves:packages/server/src/server/requestStateCodec.tsanddocs/migration.md. No file by that name exists anywhere.examples/mrtr/server.tsis the worked multi-round-trip example, and this very PR converts it from hand-rollednode:cryptoHMAC tocreateRequestStateCodec— making it exactly the "worked end-to-end example" the docs intend to point at.
Why nothing prevents it. Doc/JSDoc path references are not checked by the build, lint, or tests, so a stale path passes every gate silently.
Impact. Readers following either pointer (a developer reading the codec's API docs, or someone working through the migration guide's requestState section) hit a dead path and have to hunt for the example themselves. Low severity — purely a documentation accuracy issue with no runtime effect.
Fix. Update both references to examples/mrtr/server.ts:
- in
packages/server/src/server/requestStateCodec.ts, the closing JSDoc line ofcreateRequestStateCodec - in
docs/migration.md, the final sentence of the rewrittenrequestStateparagraph
…he multi-round-trip requestState
`createRequestStateCodec({ key, ttlSeconds?, bind? })` returns `{ mint, verify }`:
`mint` HMAC-SHA256-seals a JSON-serializable payload (with TTL, default 600 s,
and optional context binding) into the opaque `requestState` wire string;
`verify` is the function consumers drop directly into
`ServerOptions.requestState.verify` (and call again in the handler to read the
payload back). WebCrypto-based and runtime-neutral; verification is fail-closed
and constant-time (`subtle.verify`). Thrown reasons are fixed opaque codes
(`malformed`/`mac`/`expired`/`bind`) — never decoded payload or binding values
— so the seam's `onerror` relay cannot leak principal identifiers.
The `ServerOptions.requestState.verify` return type is widened to
`unknown | Promise<unknown>` (the seam already awaited-and-discarded) so the
codec's payload-returning `verify` is directly assignable.
The hand-rolled HMAC in `examples/server/src/multiRoundTrip.ts` and the
conformance fixture switch to the helper (the
`input-required-result-tampered-state` scenario is now its conformance-side
proof).
…d7ad Vendors a tarball built from modelcontextprotocol/conformance@main (d70d7ad, post-0.2.0-alpha.4) and points the package.json devDependency at it via a file: spec, so #347 (client-scenario mocks serve server/discover) is available without waiting for the next published release. README documents the pin and how to revert to a published version. Baseline read against d6008bcd7 (no fixture changes in this commit): client:all 345p / 14f / 1w - stale baseline: auth/scope-step-up now passes (#337 gates the SEP-2350 scope-union WARNING to 2026-07-28+) - new unexpected: request-metadata (2f) — mock now rejects with -32020/-32022 (renumbered per spec#2907 in #353); SDK still emits and recognises -32001/-32004 - sep-2322-client-request-state, http-custom-headers, http-invalid-tool-headers all pass with the withLocalDiscoverResponse shim still in place; #347 makes those mocks serve server/discover natively, so the shim is now redundant (removal is a separate change) server:draft 68p / 4f - server-stateless 23p/1f, http-custom-header-server-validation 6p/3f, http-header-validation flagged — all #353 error-code renumbering (referee now asserts -32020/-32021/-32022; SDK emits -32001/-32004) - new on main: SEP-2663 tasks lifecycle scenario (#262), pending-list only — not in the draft suite - spec-types/draft.ts re-synced (#348)
…vel; toNodeHandler gains onerror
server: on a 2026-07-28 request, ctx.mcpReq.log() now sends nothing when the
per-request _meta io.modelcontextprotocol/logLevel envelope key is absent (the
spec at the pin says absent → server MUST NOT send notifications/message).
Previously absent meant "send everything". The 2025-era branch (session-scoped
logging/setLevel) is unchanged. The e2e log-from-handler scenario now sets the
key explicitly so the entryModern cell still observes the log; the changeset
notes that 2025-stateful + JSON-mode + standalone-GET deployments lose handler
logs they previously received via the GET stream.
node: toNodeHandler(handler, { onerror? }) — the optional onerror receives the
adapter-level error fallback (request conversion / handler.fetch throw) before
the 500 response is written, restoring observability parity with the removed
.node face. New ToNodeHandlerOptions type exported; migration entries updated;
unit test added.
docs: align the { fetch, close, notify } shape wording with the actual
McpHttpHandler type (also has bus) in the create-mcp-handler changeset, the
McpHttpHandler JSDoc, and migration-SKILL.md.
Also: prettier-format test/conformance/README.md (drift from the previous
commit on this branch) so lint:all passes.
felixweinberger
force-pushed
the
fweinberger/on-codec-conformance
branch
from
f2a3ec6 to
f0848e2
Compare
felixweinberger
force-pushed
the
fweinberger/on-tonodehandler
branch
from
f9069fa to
c8779f8
Compare
…oNodeHandler onerror; reconcile conformance expected-failures to local main pin
createRequestStateCodec: the optional bind(ctx) value is now stored in the
envelope as b64url(HMAC(key, "mcp.requestState.bind:" + bind(ctx))[:16])
instead of the raw string, so a principal identifier in the binding is not
client-readable in the signed-not-encrypted body. Verify recomputes the tag.
JSDoc + migration.md call out that the codec is signed-not-encrypted (payload
readable; use AEAD for confidentiality) and that handlers re-calling verify is
the intended decode pattern. New unit test asserts the raw bind string is
absent from the decoded body.
toNodeHandler: wrap the opts.onerror call in a try/catch so a throwing user
callback cannot escape past the adapter-level 500 fallback. New unit test
covers a throwing onerror still answering 500/-32603.
test/conformance: reconcile expected-failures{,.2026-07-28}.yaml to the
vendored 0.2.0-main.d70d7ad pin — drop the now-passing auth/scope-step-up
(conformance#337) and add request-metadata / server-stateless /
http-custom-header-server-validation / http-header-validation as
conformance#353-renumber fallout (renumber pending a ruling on spec#2907;
SDK error codes are unchanged). Header line updated to name the local pin.
README gains the vendored tarball's sha256 and rebuild command.
felixweinberger
force-pushed
the
fweinberger/on-codec-conformance
branch
from
f0848e2 to
f4e4654
Compare
| // request method on that revision). The spec at 2026-07-28 | ||
| // says an absent key means the server MUST NOT send | ||
| // `notifications/message` for the request — so an absent key | ||
| // suppresses, it does not mean "send everything". On | ||
| // 2025-era connections the session-scoped level set via | ||
| // `logging/setLevel` applies exactly as before (an absent | ||
| // session level there continues to mean no filter). | ||
| let threshold: LoggingLevel | undefined; | ||
| if (this._servedModernEra()) { | ||
| threshold = ctx.mcpReq.envelope?.[LOG_LEVEL_META_KEY] as LoggingLevel | undefined; | ||
| if (threshold === undefined) { | ||
| return Promise.resolve(); | ||
| } | ||
| } else { | ||
| threshold = this._loggingLevels.get(undefined); | ||
| } | ||
| if (threshold !== undefined && this.LOG_LEVEL_SEVERITY.get(level)! < this.LOG_LEVEL_SEVERITY.get(threshold)!) { | ||
| return Promise.resolve(); | ||
| } |
There was a problem hiding this comment.
🟡 Two follow-ups on the rewritten ctx.mcpReq.log() level filter: (1) the legacy branch reads the threshold via this._loggingLevels.get(undefined), but logging/setLevel stores the level under the transport session id, so on a sessionful 2025-era connection handler-emitted logs ignore the client-set level (this lookup pre-dates the PR, but these exact lines are rewritten and re-documented here — consult ctx.sessionId with a fallback to the undefined key); (2) the new modern-era behavior — an absent _meta['io.modelcontextprotocol/logLevel'] key now suppresses the notification entirely — is documented in the changeset and e2e test, but docs/server.md's Logging section still tells authors to call ctx.mcpReq.log() inside any handler with no mention that on 2026-07-28 requests delivery requires the per-request logLevel envelope key, so the documented pattern silently does nothing for the common case.
Extended reasoning...
Part 1 — legacy branch ignores the session-scoped logging/setLevel level (pre-existing).
The logging/setLevel handler (_registerLoggingHandler, server.ts ~290) stores the client's chosen level keyed by the transport session id:
const transportSessionId = ctx.sessionId || (ctx.http?.req?.headers.get('mcp-session-id') as string) || undefined;
this._loggingLevels.set(transportSessionId, parseResult.data);So on a sessionful Streamable HTTP connection the level lives under the session-id key. The legacy branch of the rewritten filter in buildContext (server.ts:336) reads only this._loggingLevels.get(undefined) — it never consults ctx.sessionId — so it finds nothing and applies no filter.
Step-by-step: (1) a 2025-era client on a sessionful Streamable HTTP transport sends logging/setLevel with level: 'warning'; the level is stored under the session id (e.g. 'abc-123'). (2) A tool handler calls ctx.mcpReq.log('debug', ...). (3) _servedModernEra() is false, so threshold = this._loggingLevels.get(undefined) → undefined. (4) The severity comparison is skipped (threshold !== undefined is false) and the debug notification is sent — even though the client asked for warning and above. By contrast, Server.sendLoggingMessage(params, sessionId) honors the per-session level via isMessageIgnored(level, sessionId), and the changeset/migration prose says ctx.mcpReq.log "respects the client's log level filter", so the behavior contradicts both the sibling API and the docs.
This lookup-by-undefined is byte-identical to the pre-PR ternary, so the PR did not introduce it — but the PR rewrites these exact lines into the new if/else and re-documents the legacy semantics in the comment ("the session-scoped level set via logging/setLevel applies exactly as before"), so this is the natural place to fix it. ctx.sessionId is in scope in buildContext; the fix is this._loggingLevels.get(ctx.sessionId) ?? this._loggingLevels.get(undefined) (or just reuse isMessageIgnored(level, ctx.sessionId)).
Part 2 — docs/server.md Logging prose now contradicts the modern-era behavior (introduced by this PR).
The new branch (server.ts:330-334) makes an absent _meta['io.modelcontextprotocol/logLevel'] key on a 2026-07-28-served request return early and emit nothing, where previously an absent key meant "no filter, send". The change is recorded in the changeset, and the e2e scenario (handler-context.test.ts) had to be updated to set the _meta key explicitly to keep the log delivered — which is itself evidence the default path now suppresses.
However, docs/server.md's "Logging" section (lines 345-376) — untouched by this PR — still tells server authors to declare the logging capability and "call ctx.mcpReq.log(level, data) inside any handler", with examples that just call ctx.mcpReq.log('info', ...). There is no mention of the per-request io.modelcontextprotocol/logLevel envelope key anywhere in docs/server.md or docs/client.md. After this change, the documented pattern silently does nothing for any 2026-era request whose client did not set that key — the common case, since nothing in the docs tells clients to set it. Per the repo checklist ("Bugfix or behavior change: check whether docs/**/*.md describes the old behavior and needs updating"), the Logging section should add a sentence noting that on 2026-07-28 requests delivery requires the client to set the per-request io.modelcontextprotocol/logLevel _meta key.
Impact and fix. Part 1: real correctness gap on the legacy sessionful path, but pre-existing — fix or note, not blocking. Part 2: documentation-only; one or two sentences in docs/server.md's Logging section close the gap. Neither blocks the PR.
| GET/DELETE session operations, all-legacy batches, posted responses, non-JSON bodies). Returns `false` for envelope-claiming requests AND for malformed/incomplete modern claims (the modern path answers those with `-32602`/`-32001`) — route `false` traffic to the modern handler, | ||
| never to a legacy handler. The predicate classifies a clone (the body stays readable); pass the parsed body as the second argument when the stream was already consumed. | ||
| - `legacyStatelessFallback(factory)` is exported as a standalone fetch-shaped handler producing the same stateless legacy serving as the default. | ||
| - The handler is web-standards-only (`{ fetch, close, notify }`). On Workers / Bun / Deno, `export default handler` works directly. On Node frameworks (Express, Fastify, plain `node:http`), wrap once with `toNodeHandler(handler)` from `@modelcontextprotocol/node`: | ||
| `app.all('/mcp', toNodeHandler(handler))`, or `const node = toNodeHandler(handler); app.all('/mcp', (req, res) => void node(req, res, req.body))` when a body parser already consumed the stream. Earlier 2.x alphas exposed this as `handler.node(req, res, req.body)` — replace with | ||
| the `toNodeHandler` wrap and add the `@modelcontextprotocol/node` import. `NodeIncomingMessageLike` / `NodeServerResponseLike` are now exported from `@modelcontextprotocol/node`, not `@modelcontextprotocol/server`. | ||
| - The handler is web-standards-only (`{ fetch, close, notify, bus }`). On Workers / Bun / Deno, `export default handler` works directly. On Node frameworks (Express, Fastify, plain `node:http`), wrap once with `toNodeHandler(handler, { onerror? })` from | ||
| `@modelcontextprotocol/node`: `app.all('/mcp', toNodeHandler(handler))`, or `const node = toNodeHandler(handler); app.all('/mcp', (req, res) => void node(req, res, req.body))` when a body parser already consumed the stream. The optional `onerror` receives the adapter-level | ||
| error fallback (request conversion / `handler.fetch` throw) before the `500` response is written. Earlier 2.x alphas exposed this as `handler.node(req, res, req.body)` — replace with the `toNodeHandler` wrap and add the `@modelcontextprotocol/node` import. | ||
| `NodeIncomingMessageLike` / `NodeServerResponseLike` are now exported from `@modelcontextprotocol/node`, not `@modelcontextprotocol/server`. |
There was a problem hiding this comment.
🟡 docs/migration-SKILL.md's multi-round-trip section (line 554) still tells readers to integrity-protect requestState themselves but never mentions the createRequestStateCodec helper or the ServerOptions.requestState.verify hook that this PR adds and documents in the sibling docs/migration.md. Consider adding a one-sentence pointer so the LLM-applied SKILL doc steers migrations to the new helper instead of hand-rolled HMAC.
Extended reasoning...
What's missing. This PR introduces createRequestStateCodec({ key, ttlSeconds?, bind? }) in @modelcontextprotocol/server and rewrites the requestState paragraph in docs/migration.md (~line 1238) to present the codec plus ServerOptions.requestState.verify as the intended drop-in. It also converts the SDK's own worked examples (examples/mrtr/server.ts, test/conformance/src/everythingServer.ts) from hand-rolled node:crypto HMAC to the codec. However docs/migration-SKILL.md — which describes itself as the LLM-optimized mirror of migration.md and IS touched by this PR (the toNodeHandler { onerror } lines at 596–602) — still carries only the old guidance in its multi-round-trip section.\n\nThe exact gap. Line 554 of migration-SKILL.md reads: "requestState round-trips as an opaque string and comes back as attacker-controlled input — integrity-protect (HMAC/AEAD) and verify it yourself when relying on it." A grep of the whole file finds zero occurrences of createRequestStateCodec or requestState.verify (the only other requestState mentions, lines 514/525/528, are about the wire-lift, not integrity protection).\n\nWhy it matters. The SKILL file exists specifically so tools like Claude Code can mechanically apply the migration. With the sentence as-is, an automated migration of a multi-round-trip server will conclude the consumer must hand-roll HMAC — exactly the boilerplate this PR's helper was added to eliminate, and exactly what this PR removed from its own examples. The two migration docs are otherwise kept in step by this PR (the { fetch, close, notify, bus } and toNodeHandler onerror updates land in both), so the requestState section is the one place they now diverge.\n\nStep-by-step proof.\n1. grep -n createRequestStateCodec docs/migration-SKILL.md → no matches; grep -n requestState docs/migration-SKILL.md → lines 514, 525, 528 (wire-lift) and 554 (the verify-it-yourself sentence).\n2. docs/migration.md in this PR's diff now says: "…and an opt-in helper to drop into it: createRequestStateCodec({ key, ttlSeconds?, bind? }) returns { mint, verify } … verify is exactly the function you assign to the hook."\n3. An LLM following migration-SKILL.md §12d for a server using requestState therefore writes createHmac/timingSafeEqual boilerplate, while the same migration applied from migration.md (or by reading the updated examples) would use the codec.\n\nOn the refutation. It is true that the hard "document in both files" rule in CLAUDE.md is scoped to breaking changes, that the existing sentence at line 554 remains technically accurate (the SDK still applies no sealing by default), and that the codec has no v1 API to map from — so this is not a correctness error and should not block the PR. But the SKILL doc is not purely a v1→v2 mapping table: its §12d already carries this exact security-guidance prose, which is the SKILL-side counterpart of the migration.md paragraph this PR rewrote, and the PR edits the SKILL file anyway. Leaving the two docs divergent on the one point where the SDK now ships a first-class answer is an avoidable inconsistency, hence filed as a non-blocking nit rather than a normal finding.\n\nFix. Append one sentence to the line-554 paragraph along the lines of: "…or drop the SDK's createRequestStateCodec({ key, ttlSeconds?, bind? }) into ServerOptions.requestState.verify (mint with codec.mint, decode on re-entry with codec.verify)."
| # NOTE: the SDK's modern-path rejection codes are aligned with what this | ||
| # referee asserts: header/body mismatches answer -32001 (HeaderMismatch) and a |
There was a problem hiding this comment.
🟡 The header NOTE in test/conformance/expected-failures.yaml (lines 9-13) still claims the SDK's modern-path rejection codes "are aligned with what this referee asserts" (-32001 / -32602), but the conformance#353 entries this same PR adds further down in this file (and in expected-failures.2026-07-28.yaml) state the new pin asserts -32020/-32021/-32022 while the SDK still emits -32001/-32003/-32004 — which is exactly why the new expected-failure entries exist. Rewrite or drop the NOTE (e.g. point at the #353 sections / the fweinberger/on-renumber follow-up) so the file's header doesn't contradict its own entries.
Extended reasoning...
What the inconsistency is. The PR repoints the conformance referee from the published 0.2.0-alpha.4 to a vendored build of main @ d70d7ad and rewrites the adjacent "Baseline established against..." paragraph (lines 4-7) of test/conformance/expected-failures.yaml to name the new pin. However, the NOTE immediately below it (lines 9-13) is carried forward unchanged: it still states that "the SDK's modern-path rejection codes are aligned with what this referee asserts: header/body mismatches answer -32001 (HeaderMismatch) and a missing _meta envelope (or missing protocolVersion key) answers -32602", and that mismatched cells would only be re-derived "if a future published conformance release changes those assignments".
Why it is now false at this pin. "This referee" now refers to the vendored 0.2.0-main.d70d7ad build, which carries the conformance#353 error-code renumber (spec#2907). The very entries this PR adds further down in the same file say so explicitly: under the new # --- conformance#353 error-code renumber (spec#2907) --- sections, the comments state the referee at this pin asserts -32020/-32021/-32022 for header/body mismatch, missing-required-client-capability, and unsupported-protocol-version, while the SDK "still emits -32001/-32003/-32004". The same wording appears in the new entries added to expected-failures.2026-07-28.yaml. The four #353-fallout expected failures (request-metadata, http-custom-header-server-validation, http-header-validation, server-stateless) exist precisely because the SDK's codes are no longer aligned with the referee — the future-tense escape hatch in the NOTE ("if a future ... release changes those assignments") has already happened at this pin, just via a vendored main build rather than a published release.
Step-by-step walk-through of the contradiction as the file stands after this PR:
- Lines 4-7 (rewritten by this PR): baseline is established against
0.2.0-main.d70d7ad. - Lines 9-10 (untouched): "the SDK's modern-path rejection codes are aligned with what this referee asserts: header/body mismatches answer -32001 (HeaderMismatch)...".
- The new server-section entry added by this PR for
http-custom-header-server-validation: "SDK still emits −32001/−32003/−32004; the referee at this pin asserts −32020/−32021/−32022 for header/body mismatch, ..." — i.e. the referee does not assert -32001 for header/body mismatch, directly contradicting step 2. - A reader (or the next person bumping the pin) following the header NOTE would conclude no error-code reconciliation is pending, when the file's own Notification on Capabilities #353 sections — and the
fweinberger/on-renumberbranch later in this stack, per the PR description — say the opposite.
Why nothing catches it. The expected-failures evaluator only checks scenario names against actual results (and stale entries); comment prose in the YAML is never validated, so the contradiction passes every gate silently.
Impact. Documentation/comment accuracy only — no runtime or CI behaviour is affected. The cost is reader confusion in the file that is supposed to be the authoritative record of why each conformance cell is expected to fail, which matters most when the next pin bump or the renumber follow-up reconciles this file.
Fix. Rewrite (or drop) the NOTE so it reflects the d70d7ad pin — e.g. note that at this pin the referee asserts the conformance#353-renumbered codes (-32020/-32021/-32022), the SDK still emits -32001/-32003/-32004, the affected cells are tracked in the #353 sections below (and in expected-failures.2026-07-28.yaml), and the SDK-side flip lands in fweinberger/on-renumber. The -32602 missing-envelope statement can be kept if it still holds, but the headline alignment claim should go.
| /** Options for {@linkcode toNodeHandler}. */ | ||
| export interface ToNodeHandlerOptions { | ||
| /** | ||
| * Called when the adapter answers `500` because request conversion or | ||
| * `handler.fetch` itself threw (e.g. a closed handler). Restores the | ||
| * observability the removed `.node` face had via the entry's own | ||
| * `onerror` — entry-internal failures are still reported through | ||
| * `handler.fetch` and surface via the entry's `onerror` option as before. | ||
| */ | ||
| onerror?: (error: Error) => void; | ||
| } |
There was a problem hiding this comment.
🟡 The module-level JSDoc at the top of toNodeHandler.ts still describes the handler shape as { fetch, close, notify }, while this PR aligns that same shape string to { fetch, close, notify, bus } in the changeset, the McpHttpHandler JSDoc, and migration-SKILL.md. Since this file is already touched by the PR (the new onerror option), update the comment here too for consistency.
Extended reasoning...
What the issue is. The module-level JSDoc at the top of packages/middleware/node/src/toNodeHandler.ts (line 5) still reads: "The handler itself is web-standards-only ({ fetch, close, notify } — the shape Workers/Bun/Deno expect from export default)". The handler shape it describes is missing bus.
Why it matters in this PR. One of the explicit follow-up fixes folded into this PR is aligning the documented handler shape from { fetch, close, notify } to { fetch, close, notify, bus }. The diff does this in three places: .changeset/create-mcp-handler.md, the McpHttpHandler JSDoc in createMcpHandler.ts, and docs/migration-SKILL.md. The one remaining occurrence of the old shape sits in toNodeHandler.ts — a file this same PR also edits (it adds ToNodeHandlerOptions and the onerror plumbing at lines 67–77 and 88–120). So the doc-alignment fix is partially applied, leaving a sibling site with the stale form.
Step-by-step proof.
McpHttpHandlerinpackages/server/src/server/createMcpHandler.tsdeclaresfetch,close,notify, andbus— and its JSDoc was updated by this PR to say{ fetch, close, notify, bus }..changeset/create-mcp-handler.mdanddocs/migration-SKILL.mdwere likewise updated in this diff to the four-member shape.grep -n 'fetch, close, notify' packages/middleware/node/src/toNodeHandler.tsstill matches line 5: "web-standards-only ({ fetch, close, notify }— …)" with nobus.- A reader of the Node adapter's module docs therefore sees a handler shape that contradicts both the handler's own type and every other doc the PR just aligned.
Why nothing catches it. Comment text is not checked by typecheck, lint, or tests, so the stale shape passes every gate silently. The runtime is unaffected — toNodeHandler only consumes handler.fetch (the structural FetchLikeMcpHandler face), so this is purely a documentation-consistency item.
Fix. Change the line-5 comment to { fetch, close, notify, bus }, matching the McpHttpHandler JSDoc, the changeset, and migration-SKILL.md. One-word change, non-blocking.
| const subtle = globalThis.crypto?.subtle; | ||
| if (subtle === undefined) { | ||
| throw new TypeError( | ||
| 'createRequestStateCodec requires the Web Crypto API (globalThis.crypto.subtle); ' + | ||
| 'see https://github.com/modelcontextprotocol/typescript-sdk#nodejs-web-crypto-globalthiscrypto-compatibility' | ||
| ); | ||
| } |
There was a problem hiding this comment.
🟡 The TypeError thrown by createRequestStateCodec when globalThis.crypto.subtle is unavailable points users to https://github.com/modelcontextprotocol/typescript-sdk#nodejs-web-crypto-globalthiscrypto-compatibility, but README.md has no Web Crypto section, so the anchor resolves to nothing — the actual guidance lives in docs/faq.md ("How do I enable Web Crypto (globalThis.crypto) for client authentication in older Node.js versions?"). Point the message at the FAQ entry or add the referenced README section. (The same dead link exists pre-PR in packages/client/src/client/authExtensions.ts:40, but this PR copies it into brand-new code.)
Extended reasoning...
What the bug is. The new createRequestStateCodec (packages/server/src/server/requestStateCodec.ts:121-127) guards on WebCrypto availability and, when globalThis.crypto?.subtle is undefined, throws a TypeError whose message tells the user to "see https://github.com/modelcontextprotocol/typescript-sdk#nodejs-web-crypto-globalthiscrypto-compatibility". That URL fragment targets a README heading named "Node.js Web Crypto (globalThis.crypto) compatibility" — but no such heading exists in README.md.
The code path that triggers it. A consumer calls createRequestStateCodec({ key }) on a runtime where globalThis.crypto.subtle is not defined (e.g. Node 18 without --experimental-global-webcrypto or a polyfill, or an unusual embedded runtime). The very first statement of the factory reads globalThis.crypto?.subtle, finds undefined, and throws the TypeError carrying the dead link. The user, following the SDK's own pointer, lands on the repo README with no matching anchor and no Web Crypto content at all.
Step-by-step proof.
- README.md's headings are: MCP TypeScript SDK / Overview / Packages / Installation / Getting Started / Documentation / Building docs locally / v1 (legacy) documentation and fixes / Contributing / License. A grep for "Web Crypto" or "globalThis.crypto" in README.md returns no matches, so
#nodejs-web-crypto-globalthiscrypto-compatibilitycannot resolve to any heading. - The actual guidance exists, but in docs/faq.md line 41: "How do I enable Web Crypto (
globalThis.crypto) for client authentication in older Node.js versions?" — including the polyfill snippet and the Node-version matrix the error message presumably means to point at. - The only other occurrence of this exact link in the repo is packages/client/src/client/authExtensions.ts:40, where it is equally dead today; the new message in requestStateCodec.ts was evidently copied from there. The pre-existing occurrence is not this PR's fault, but the PR introduces a brand-new occurrence in brand-new public-API code, so a user of the new helper hitting the missing-WebCrypto error is steered to a dead anchor.
Why nothing prevents it. URLs embedded in error-message strings are not checked by typecheck, lint, or any test (the codec's own test suite never exercises the missing-WebCrypto branch's message content), so the dead anchor passes every gate silently.
Impact. Documentation/error-message accuracy only — the error itself is correct, fail-fast, and its prose ("requires the Web Crypto API (globalThis.crypto.subtle)") is actionable on its own, so there is no runtime or security impact. The cost is a confusing dead end for the exact user the link is meant to help: someone on an older runtime who needs the polyfill instructions.
How to fix. Either point the message at the FAQ entry — e.g. "see docs/faq.md, 'How do I enable Web Crypto (globalThis.crypto) for client authentication in older Node.js versions?' (https://github.com/modelcontextprotocol/typescript-sdk/blob/main/docs/faq.md)" — or add the referenced "Node.js Web Crypto (globalThis.crypto) compatibility" section to README.md so both this link and the pre-existing one in authExtensions.ts:40 start resolving. If the README section is added, no code change is needed at all; otherwise, updating the pre-existing client-side occurrence in the same sweep would be a nice (optional) cleanup.
1 participant
Adds
createRequestStateCodec(opt-in HMAC sealing for the multi-round-triprequestState), pins the conformance referee to a local build ofmain @ d70d7ad, and folds in two follow-up fix rounds for earlier PRs in this stack. Stacked onfweinberger/on-tonodehandler.Motivation and Context
createRequestStateCodecThe multi-round-trip
requestStatehook expects callers to seal opaque state across the client round-trip; today every consumer hand-rollsnode:cryptoHMAC. NewcreateRequestStateCodec({ key, ttlSeconds?, bind? })in@modelcontextprotocol/serverreturns{ mint, verify }:subtle.importKey/sign/verify), runtime-neutral, constant-time MAC check"v1." b64url({"p":payload,"exp":unix,"b":bind?}) "." b64url(mac); TTL default 600 s; key ≥32 bytes elseRangeErrorbind?: (ctx) => stringfor principal/method binding — stored as a domain-separated keyed tag (b64url(HMAC(key, "mcp.requestState.bind:" + bind(ctx))[:16])), so the signed-not-encrypted body never carries a client-readable principal identifiermalformed/mac/expired/bind) — never decoded payload or binding valuesServerOptions.requestState.verifyreturn widenedvoid|Promise<void>→unknown|Promise<unknown>socodec.verifyis directly assignable (seam already awaited-and-discarded)examples/mrtr/server.tsandtest/conformance/src/everythingServer.tsswitch from hand-rolled HMAC to the helper; theinput-required-result-tampered-stateconformance scenario is now its proof. JSDoc andmigration.md§requestState carry the signed-not-encrypted note (payloadpis client-readable; use AEAD for confidentiality; the bind tag is keyed so it is not).Conformance referee pin
@modelcontextprotocol/conformancerepointed from0.2.0-alpha.4to a vendored tarball built fromorigin/main @ d70d7ad(file:./vendor/...tgz, version0.2.0-main.d70d7ad). README documents the rebuild recipe + tarball sha256. Whatmainbrings vs alpha.4: modelcontextprotocol/conformance#337 (gates the SEP-2350 scope-union WARNING), #347 (client-scenario mocks serveserver/discovernatively), #348 (re-syncs draft spec types), #353 (renumbers draft error codes per modelcontextprotocol/modelcontextprotocol#2907), #262 (SEP-2663 tasks-lifecycle scenario).expected-failures*.yamlreconciled so all gate legs exit 0; the four #353-fallout entries are annotated as renumber-pending (the SDK error codes are flipped infweinberger/on-renumber, later in this stack).Follow-up fixes folded in
ctx.mcpReq.log: absent_meta['io.modelcontextprotocol/logLevel']on a 2026-era request now suppresses (spec: absent → MUST NOT sendnotifications/message); legacy-era branch unchanged. The e2e scenario body sets the_metakey explicitly.toNodeHandler(handler, opts?): newopts.onerror(error)is called before the adapter-level 500 fallback when request-conversion /handler.fetchthrows; wrapped intry { … } catch {}so a throwing user callback never escapes past the 500/−32603 fallback. NewToNodeHandlerOptionsexported from@modelcontextprotocol/node.{ fetch, close, notify }→{ fetch, close, notify, bus }aligned in.changeset/create-mcp-handler.md,McpHttpHandlerJSDoc, andmigration-SKILL.md.How Has This Been Tested?
packages/server/test/server/requestStateCodec.test.ts(round-trip, body/MAC tamper, cross-key, malformed×4, expired, bind ok/mismatch-opaque/mint-requires-ctx, key-too-short, hook-assignability, decoded-body-never-carries-raw-bind)onerrorcalled; throwingonerrorswallowed)server:draft77p/0f at the codec commit (all 14input-required-result-*scenarios pass incl.tampered-state); after the pin reconcile all gate legs (client:all,client:2026,server,server:draft,server:2026) exit 0Breaking Changes
None.
createRequestStateCodecis opt-in;ServerOptions.requestState.verifywidening is backward-compatible.Types of changes
Checklist
Additional context
server:allremains red on the pre-existingjson-schema-2020-12/server-sse-pollingpair plus the new SEP-2663tasks-*pending-only scenarios from conformance#262 — not a gate leg, untouched. Reference shape: signed mode only; AEAD/HKDF deferred.