Skip to content

fix(server,client): per-request ctx.log delivery; 2026-era HTTP cancel via stream close; skip unused entry clone#2330

Open
felixweinberger wants to merge 20 commits into
fweinberger/sep2243-stdfrom
fweinberger/on-small-fixes
Open

fix(server,client): per-request ctx.log delivery; 2026-era HTTP cancel via stream close; skip unused entry clone#2330
felixweinberger wants to merge 20 commits into
fweinberger/sep2243-stdfrom
fweinberger/on-small-fixes

Conversation

@felixweinberger

Copy link
Copy Markdown
Contributor

Three small server/client correctness fixes for the 2026-07-28 per-request hosting path. Stacked on #2328.

Motivation and Context

  • ctx.mcpReq.log on per-request hosting — handler-emitted log notifications were sent via the session-scoped sendLoggingMessage path, which has no delivery channel under createMcpHandler's per-request model. Server.buildContext now wires log through the request context's ctx.mcpReq.notify (stamping relatedRequestId) so the notification rides the in-flight exchange the same way progress/notify already do. Level filtering branches by era: 2026-era reads the per-request _meta['io.modelcontextprotocol/logLevel'] envelope key (the modern equivalent now that logging/setLevel is removed); legacy-era keeps the existing _loggingLevels map lookup unchanged. The public sendLoggingMessage API is untouched.
  • 2026-era HTTP cancel = stream close — the spec wording at the 2026-07-28 revision is "no notifications/cancelled message is required or expected" when the transport carries a per-request stream. Protocol._requestWithSchemaViaCodec now threads a per-request AbortController and aborts it on cancel/timeout when codec.era === MODERN_WIRE_REVISION && transport.hasPerRequestStream; otherwise the legacy notifications/cancelled POST is byte-identical. New optional Transport.hasPerRequestStream flag, set true on StreamableHTTPClientTransport.
  • createMcpHandler clone nitclassifyEntryRequest grows a needsForward flag (default true) gating the body-preserving request.clone(); the isLegacyRequest path passes false so the unused forwarding tee is skipped.

How Has This Been Tested?

  • core unit tests cover modern×per-request-stream / modern×stdio / legacy×per-request-stream / modern timeout for the cancel path; client test pins hasPerRequestStream
  • e2e: protocol:cancel:http-stream-close supersedes the abort-signal / sends-cancellation rows (manifest-only); mcpserver:context:log-from-handler re-admitted to entryStateless + entryModern
  • Full gates at this tip: typecheck, lint, core (1207), client (535), server (320), integration (348), e2e (2535p/205xf)

Breaking Changes

None. New optional Transport.hasPerRequestStream flag; existing transports without it keep legacy cancel semantics.

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

Additional context

Changesets + migration.md / migration-SKILL.md entries included for the cancel and log changes.

…lidate

Pure module for the custom-header half of SEP-2243 (protocol revision
2026-07-28): scanning a tool inputSchema for x-mcp-header declarations
(RFC 9110 token + primitive-type + case-insensitive-uniqueness constraints),
the =?base64?…?= sentinel value encoding/decoding, building Mcp-Param-{Name}
headers from call arguments, and the server-side header/body comparison.

The -32001 (HeaderMismatch) rejection consumes the same shape the inbound
classifier already emits for the standard-header cross-checks (400 Bad
Request, data.mismatch, settled), with a new 'param-header-validation'
ladder rung evaluated pre-dispatch against the resolved tool's schema.

Internal-barrel only; no public-surface change in this commit.
Client.callTool() mirrors x-mcp-header-designated arguments into
Mcp-Param-{Name} HTTP headers on a modern (2026-07-28) connection over
Streamable HTTP, using the codec module from the previous commit.
Client.listTools() excludes constraint-violating tool definitions on a
modern HTTP connection (with a warning naming the tool and the reason).

Transport plumbing: TransportSendOptions.headers (additive, optional)
threads per-request HTTP headers from Protocol.request() into the
Streamable HTTP transport's POST; the transport also derives Mcp-Name
from params.name/uri alongside the existing body-derived
MCP-Protocol-Version/Mcp-Method. Single-channel transports (stdio,
in-memory) ignore the option, satisfying the spec's stdio MAY-ignore
exemption without an explicit branch.

Schema-knowledge policy: definitions are cached from tools/list with the
server's ttlMs (CacheableResult); on a -32001 (HeaderMismatch) rejection
the client refreshes once and retries. New
CallToolRequestOptions.toolDefinition lets callers supply the schema
directly. Browser environments skip mirroring (dynamically named headers
cannot be statically allow-listed for credentialed CORS) and rely on the
server's body-authoritative validation. Legacy-era callTool/listTools
paths are unchanged.
… entry

Pre-dispatch ladder rung on the modern (2026-07-28) serving path: a
tools/call whose Mcp-Param-{Name} headers disagree with the body
arguments (or are missing for a present body value, or carry an invalid
=?base64?…?= sentinel) is rejected 400/-32001 (HeaderMismatch) — the
same shape the inbound classifier emits for the standard-header
cross-checks. A null/absent body value passes regardless of the header.

McpServer.registerTool now warns at registration time when an
x-mcp-header declaration violates the spec's constraints (additive — it
does not throw). McpServer.toolInputSchemaJson() exposes the
JSON-serialized inputSchema for the entry's pre-dispatch lookup.

The 2025-era serving paths are unchanged; the validation only fires when
the factory returns an McpServer (the registry is the schema source).
…aders / http-invalid-tool-headers / http-custom-header-server-validation

- everythingServer: register a tool with an x-mcp-header annotation so
  http-custom-header-server-validation runs (was 0/0 SKIPPED → 9/0).
- everythingClient: handlers for http-custom-headers (18/0) and
  http-invalid-tool-headers (11/0); same withLocalDiscoverResponse shim
  the multi-round-trip scenario uses (the SEP-2243 mocks have no
  server/discover).
- Client: drop the freshness gate from the x-mcp-header scan lookup —
  the most recent tools/list result is the best schema available
  regardless of the server's ttlMs (a ttlMs:0 result was being treated
  as immediately unusable); the -32001 → refresh-and-retry path covers
  the stale-schema case.
- Streamable HTTP client: derive Mcp-Name from params.name/uri alongside
  the existing body-derived Mcp-Method (5-step algorithm step 2).
- expected-failures: remove http-custom-headers / http-invalid-tool-headers
  (both legs).
…ation

- client: encode Mcp-Name with the =?base64?...?= sentinel so non-ASCII
  names/URIs cannot make Headers.set() throw or diverge from the body
- client: skip reserved standard/auth header names in the per-request
  TransportSendOptions.headers loop so callers cannot override them
- client: surface an HTTP 400 carrying a JSON-RPC error response in-band
  via onmessage so the -32001 refresh-and-retry path is reachable over
  Streamable HTTP (not just InMemory)
- client: cacheToolMetadata only clears on the first (cursor-less)
  tools/list page and merges across subsequent pages
- client: clear _cachedToolDefinitions in _resetConnectionState
- client: remove dead TTL/expiresAt/tool plumbing on the definition
  cache; correct the field docstring
- core: carry per-request headers through buildRetryLegRequestOptions so
  Mcp-Param-* survives input_required retry legs
- core: add the param-header-validation rung to INBOUND_VALIDATION_LADDER
- core: fall back to string comparison in validateMcpParamHeaders when a
  number-declared param carries a non-numeric primitive (no false NaN
  mismatch for an identical pair)
- server: wrap the registration-time x-mcp-header scan in try/catch and
  memoize the JSON-converted inputSchema so toolInputSchemaJson() reuses
  it instead of re-converting per call
- docs: migration.md / client.md / server.md prose for Mcp-Param-*
  mirroring, the new CallToolRequestOptions.toolDefinition /
  TransportSendOptions.headers, the listTools exclusion, and the
  createMcpHandler 400/-32001 rejection
- changeset: correct the browser-skip prose to state the limitation
  against conforming SEP-2243 servers and document the reserved-header
  guard / 400 in-band delivery
… arm

Adds the sep-2243:param-header:roundtrip requirement (entryModern,
2026-07-28) and its scenario body: a tool with one x-mcp-header-declared
parameter is called through the wired client, and the Mcp-Param-{Name}
header is asserted on the recorded HTTP request alongside the encoded
value and the successful result.

RecordedHttpExchange grows a requestHeaders field so entry-arm scenarios
can assert on raw HTTP request headers.
…aJson lazy path

- callTool's one-refresh-on-HeaderMismatch now walks every tools/list
  page via a private _refreshAllToolDefinitions helper. A bare
  cursor-less listTools only fetched page 1 and (because page 1 clears
  the merged cache) wiped the page-≥2 scans the application had
  accumulated, so a page-≥2 x-mcp-header tool was never recovered and
  every other page-≥2 tool stopped mirroring until the app re-ran the
  full cursor loop. New mcpParamMirroring test pins the page-2 case.
- McpServer.toolInputSchemaJson() now returns undefined when the lazy
  standardSchemaToJsonSchema fallback throws (memo slot unset because
  registerTool's eager conversion was swallowed, or update/rename
  invalidated it). The pre-dispatch SEP-2243 caller in createMcpHandler
  was turning that throw into a 500 for a tools/call whose
  body-authoritative dispatch would otherwise succeed; the conversion
  failure now stays where it always surfaced (tools/list).
- _cachedToolDefinitions docstring corrected: only consumed (not
  populated) on a modern connection.
@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: e230a8e

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

This PR includes changesets to release 3 packages
Name Type
@modelcontextprotocol/core Minor
@modelcontextprotocol/client Minor
@modelcontextprotocol/server Patch

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@2330

@modelcontextprotocol/codemod

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

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

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

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: e230a8e

…resh

The one-refresh-on-HeaderMismatch recovery in callTool() passed the
caller's full CallToolRequestOptions to _refreshAllToolDefinitions(),
which forwards them to every internal listTools() page. Options like
toolDefinition, headers, and onprogress are tools/call-specific and do
not apply to tools/list — only the abort signal and timeout should
propagate.
…cp-Name presence and Mcp-Name cross-check

Adds `validateStandardRequestHeaders`, evaluated by the HTTP entry on a
modern-classified request immediately after `classifyInboundRequest`
returns a modern route. Rejects `400`/`-32001` (`HeaderMismatch`) — the
same shape and rung the classifier already emits for the
`MCP-Protocol-Version` and `Mcp-Method` mismatch cells — when the
required `Mcp-Method` header is absent, when the required `Mcp-Name`
header is absent on a `tools/call`/`prompts/get`/`resources/read`
request, when `Mcp-Name` carries an invalid Base64 sentinel, and when its
(decoded) value disagrees with `params.name`/`params.uri`.

Kept separate from `classifyInboundRequest` so a body-only call to the
classifier keeps routing a modern request unchanged: the classifier remains
a pure body-primary router, and this function is the presence/`Mcp-Name`
half of the standard-header rung the entry layers on top.

`InboundHttpRequest` gains an additive optional `mcpNameHeader` field.
…dler entry

`createMcpHandler` now reads `Mcp-Name` and runs
`validateStandardRequestHeaders` on a modern-classified request
immediately after the body-primary classifier returns a modern route.

The four hand-written test fixtures that build modern requests directly
(`postRequest`/`nodeRequestResponse` in createMcpHandler.test.ts,
`listenRequest` in createMcpHandlerListen.test.ts, `postEcho` in
createMcpHandlerCapabilityGate.test.ts, `call` in
mcpParamValidation.test.ts) now derive the SEP-2243 standard headers from
the body they send, exactly as a conformant client does — fixture-only,
no assertion changes. Legacy test cells stay byte-untouched (the
derivation only emits for a body carrying a modern envelope claim).
…ndard headers to raw-request fixtures

Removes `http-header-validation` from both server expected-failures
baselines (the scenario is now 13/0).

The four hand-written raw-request sites in test/integration and test/e2e
that POST a modern envelope directly at `createMcpHandler` (the
subscriptions/listen ack-first cell, the capacity-guard cell, the
honored-filter integration check, and the hosting-entry-session router
probe) now carry the SEP-2243 standard headers a conformant client sends
— fixture-only, no assertion changes.
The body method string is peer-controlled; a bare plain-object lookup
returns inherited Object.prototype members for names like 'constructor'
or 'toString', so the off-table early-return would not fire and an
invalid Mcp-Name sentinel on such a request was answered 400/-32001
instead of reaching dispatch's -32601. Match the established idiom in
requiredClientCapabilitiesForRequest.
…l on the entryModern arm

Adds the sep-2243:std-header:mismatch-rejected requirement (entryModern,
2026-07-28) and its scenario body: a raw envelope-carrying tools/call
POSTed with an Mcp-Method: tools/list header is rejected by the
createMcpHandler entry with HTTP 400 and the -32001 HeaderMismatch
JSON-RPC error code.
…ch ladder rung

The four validateStandardRequestHeaders rejection cells
(method-header-missing, name-header-missing,
name-header-invalid-encoding, name-header-mismatch) were stamped
'era-classification', whose ladder descriptor says evaluatedAt: 'edge'
— but they are evaluated by the HTTP entry's serveModern path after the
supported-revision gate, not by the edge classifier. Add a
'standard-header-validation' rung (order 8, evaluatedAt: 'pre-dispatch',
sitting between client-capabilities and the param-header rung, which
moves to order 9) and re-stamp the four cells with it. The classifier's
own header-mismatch cells (protocol-version, Mcp-Method mismatch) stay
on the edge era-classification rung. crossCheckMismatch grows an
optional rung parameter so the shared shape stays single-source.
…header; add precedence caveat

The serveModern call-site comment and the server stdHeaderValidation
test header still attributed the presence/Mcp-Name rejections to the
edge era-classification rung after 5779aab re-stamped them onto the
dedicated standard-header-validation rung. Both now name the rung
explicitly (and note the classifier's mismatch cells stay on
era-classification).

The new rung's documented order (8) is also not the observed
precedence — serveModern evaluates it immediately after the
supported-revision gate, before the dispatch rungs (5-6) and the
capability gate (7). Rather than renumber (wider blast across the
ladder table and the param-header rung), add a precedence caveat to
its rationale, mirroring the client-capabilities entry's caveat.
The legacy-era-untouched test title said it posts a 2025-era tools/list
without standard headers, but the body POSTs initialize (the 2025
handshake, which is the right body for era-gating). Aligned the title
to match the body.
@felixweinberger felixweinberger force-pushed the fweinberger/sep2243-std branch from 1e780e2 to 6e8a521 Compare June 19, 2026 14:24
… path

classifyEntryRequest grows a needsForward flag (default true) that gates the
body-preserving request.clone() it tees off for the legacy leg. isLegacyRequest
passes false: it already classifies a clone of the caller's request and never
reads forwardRequest, so the second clone buffered one extra body copy per
routed POST for nothing. The entry handler call site is unchanged (default
true) and the body-readability contract test stays.

Follow-up from the #2316 review (r3435103544); closes #36.
…tion timing-sensitive on CI

The 75% replay check depends on the request-related event-store path; on this stack the request-related routing change lands before the store-first fix, so the post-disconnect log is not reliably replayed on intermediate commits. Re-enable once the store-first fix is in the base.
…est stream, not POSTing notifications/cancelled

The 2026-07-28 spec (basic/patterns/cancellation §Transport-Specific) makes
closing the per-request SSE response stream the cancellation signal on
Streamable HTTP — no notifications/cancelled message is required or expected.
The general client request-abort path now routes accordingly: on a 2026-era
connection over a transport that opens a per-request stream, aborting the
caller signal or hitting the timeout aborts that request's
TransportSendOptions.requestSignal (the same per-request abort proven by the
listen driver) instead of POSTing notifications/cancelled. Legacy-era
connections, and stdio/in-memory at any era, keep the existing
notifications/cancelled POST path unchanged.

Adds the optional Transport.hasPerRequestStream capability flag (set on
StreamableHTTPClientTransport) so the protocol layer can route the
per-transport cancel path without inspecting transport type. e2e splits the
2025 cancel/timeout requirements via supersedes/supersededBy and adds the
2026 stream-close requirement on the entryModern arm.
…ing delivers it; 2026-era requests filter by _meta.logLevel
Comment on lines +267 to +274
/**
* Streamable HTTP opens one POST (and SSE response stream) per outbound
* request and honors `TransportSendOptions.requestSignal`. On a 2026-era
* connection the protocol layer aborts that per-request stream as the
* spec cancellation signal instead of POSTing `notifications/cancelled`.
*/
readonly hasPerRequestStream = true;

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 new 2026-era stream-close cancellation mechanism this PR introduces is a no-op for requests sent with RequestOptions.resumptionToken: _send's resumption branch calls _startOrAuthSse({ resumptionToken, replayMessageId }) without threading options.requestSignal (or onRequestStreamEnd), so the per-request abort the protocol layer now relies on is connected to nothing — the resumed stream stays open and no cancel signal of any kind reaches the server. Thread requestSignal/onRequestStreamEnd into that _startOrAuthSse call (or fall back to the legacy path when resumptionToken is set) so the hasPerRequestStream contract added here holds on the resume path too.

Extended reasoning...

What the bug is. This PR routes 2026-era cancellation entirely through TransportSendOptions.requestSignal: in Protocol._requestWithSchemaViaCodec, when codec.era === MODERN_WIRE_REVISION && transport.hasPerRequestStream === true, cancel()/timeout only call requestAbort.abort() and explicitly skip the notifications/cancelled POST (the requestAbort === undefined branch). The new hasPerRequestStream flag added at packages/client/src/client/streamableHttp.ts:267-274 documents the contract this depends on: the transport "honors TransportSendOptions.requestSignal". But StreamableHTTPClientTransport._send's resumption branch (the very first thing it does when options.resumptionToken is set) calls this._startOrAuthSse({ resumptionToken, replayMessageId }) and returns — options.requestSignal and options.onRequestStreamEnd are not passed, even though StartSSEOptions supports both and _startOrAuthSse would honor them (it folds requestSignal into the GET fetch signal and into the no-reconnect / no-onerror gate).\n\nConcrete walk-through. (1) A client on a 2026-07-28 Streamable HTTP connection re-issues a long-running request with callTool(..., { resumptionToken, onresumptiontoken, signal }) — the documented resumability flow. (2) Protocol._requestWithSchemaViaCodec computes streamCloseCancels === true, creates requestAbort, and calls transport.send(outbound, { resumptionToken, requestSignal: requestAbort.signal, ... }). (3) _send hits if (resumptionToken), opens the resumed GET stream via _startOrAuthSse({ resumptionToken, replayMessageId }) — with no requestSignal — and returns. (4) The caller aborts (or the timeout fires); cancel() sees requestAbort !== undefined, so it only calls requestAbort.abort() and never sends notifications/cancelled. (5) Nothing is listening on that signal: the resumed GET stream stays open (and keeps reconnecting on drops), the server keeps working on the request, and only the local promise rejects. The cancel signal is silently lost in every direction.\n\nWhy nothing else catches it. The unit tests added in this PR use a mock PerRequestStreamTransport that merely records requestSignal, and the e2e protocol:cancel:http-stream-close scenario taps tx.send and asserts requestSignal.aborted === true — neither exercises a send that carries a resumptionToken, so the un-wired resume branch is never observed actually tearing a stream down. The legacy notifications/cancelled fallback can't save it either, because the new code skips it whenever requestAbort exists.\n\nOn the refutation (acknowledged, and the framing here is adjusted for it). A verifier correctly points out that pre-PR behavior on this exact path was also broken: the old cancel path sent notifications/cancelled with { resumptionToken, ... } in its send options, and the same resume branch swallowed that message too (it opened another GET instead of POSTing), so the server was not informed before this PR either. That is true — this comment does not claim a behavioral regression in what the server observes. What is attributable to this PR is the contract and the mechanism: the PR (a) introduces hasPerRequestStream with a JSDoc guarantee that the transport honors requestSignal, which is false on the resume path, and (b) makes the protocol layer's only 2026-era cancel signal depend on that guarantee. The spec signal this PR is implementing — "closing the per-request stream IS the cancellation" — is therefore never emitted for resumed requests, which is squarely a gap in the feature being added, not merely a doc nit. The refutation's other point (that "fall back to notifications/cancelled" wouldn't help because the same branch swallows it) is also fair, and is why threading the signal is the right fix rather than the fallback.\n\nImpact and fix. Impact is scoped (2026-era connection + caller-supplied resumptionToken + cancel/timeout) but the failure is silent: the server keeps executing, the resumed SSE stream and its reconnect loop stay alive until transport close, and no error or notification surfaces. The fix is small and local: in the if (resumptionToken) branch of _send, pass requestSignal: options?.requestSignal and onRequestStreamEnd: options?.onRequestStreamEnd through to _startOrAuthSse_startOrAuthSse and _handleSseStream already do the right thing with both (abort the fetch, suppress the spurious onerror, skip reconnection). A unit test that sends with a resumptionToken on a modern-era connection and asserts the resumed stream is torn down on abort would pin the contract.

Comment on lines +308 to +330
log: (level, data, logger) => {
if (!this._capabilities.logging) {
return Promise.resolve();
}
// Level filter: on a 2026-era request the client declares its
// threshold per request via the `_meta.logLevel` envelope key
// (the modern equivalent of `logging/setLevel`, which is not a
// request method on that revision); on 2025-era connections the
// session-scoped level set via `logging/setLevel` applies
// exactly as before.
const threshold = this._servedModernEra()
? (ctx.mcpReq.envelope?.[LOG_LEVEL_META_KEY] as LoggingLevel | undefined)
: this._loggingLevels.get(undefined);
if (threshold !== undefined && this.LOG_LEVEL_SEVERITY.get(level)! < this.LOG_LEVEL_SEVERITY.get(threshold)!) {
return Promise.resolve();
}
// Emit request-related (like progress and `ctx.mcpReq.notify`)
// so the notification rides the in-flight exchange. Without the
// related-request stamp, per-request hosting (`createMcpHandler`,
// either era) silently drops the message because it has no
// session-wide stream to deliver it on.
return ctx.mcpReq.notify({ method: 'notifications/message', params: { level, data, logger } });
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 On a 2026-era request, ctx.mcpReq.log() sends notifications/message even when the client did not opt in: when the io.modelcontextprotocol/logLevel envelope key is absent, the new code skips the level filter and emits the notification, but the spec says an absent key means the server MUST NOT send any notifications/message for the request. Fix: in the modern-era branch, return early when the envelope carries no logLevel key (the legacy branch is correct as-is).

Extended reasoning...

The bug. The new log callback in Server.buildContext (packages/server/src/server/server.ts:308-330) reads the per-request threshold on a 2026-era request from ctx.mcpReq.envelope?.[LOG_LEVEL_META_KEY] and only suppresses the message when threshold !== undefined && severity < thresholdSeverity. When the envelope does not carry the io.modelcontextprotocol/logLevel key at all, threshold is undefined, the filter is skipped entirely, and ctx.mcpReq.notify({ method: 'notifications/message', ... }) is sent. In other words, absence of the key is treated as "no filter — send everything."\n\nWhy that is the inverse of the spec. The repo's own 2026-07-28 spec material is explicit that absence means opt-out, not opt-in. packages/core/src/types/spec.types.2026-07-28.ts (~lines 100-110) documents the io.modelcontextprotocol/logLevel key: "If absent, the server MUST NOT send any notifications/message notifications for this request. The client opts in to log messages by explicitly setting a level. Replaces the former logging/setLevel RPC." The same MUST-NOT wording is repeated on LOG_LEVEL_META_KEY in constants.ts and in the 2026 wire schemas (rev2026-07-28/schemas.ts), where the key is .optional() — so an absent key is the legal, common case.\n\nWhy the default path actually hits this. The SDK client never auto-attaches a logLevel envelope key: Client._outboundMetaEnvelope only adds protocolVersion/clientInfo/clientCapabilities. So an ordinary 2026-era SDK client that has not opted in will receive notifications/message from any handler that calls ctx.mcpReq.log. There is no other filter on this path: assertNotificationCapability only checks the server's own logging capability, and the session-scoped isMessageIgnored/sendLoggingMessage filtering is bypassed because the new code goes straight to ctx.mcpReq.notify.\n\nStep-by-step proof.\n1. A 2026-era client (e.g. one connected via createMcpHandler's modern path) calls tools/call with the required _meta envelope but no io.modelcontextprotocol/logLevel key — exactly what the SDK Client produces by default.\n2. The protocol layer lifts the envelope; ctx.mcpReq.envelope contains protocolVersion/clientInfo/clientCapabilities but no logLevel key.\n3. The tool handler calls ctx.mcpReq.log('debug', 'some message').\n4. In the new log callback, this._servedModernEra() is true, so threshold = ctx.mcpReq.envelope?.[LOG_LEVEL_META_KEY] evaluates to undefined.\n5. The guard if (threshold !== undefined && ...) is false, so the early return is skipped.\n6. ctx.mcpReq.notify({ method: 'notifications/message', ... }) rides the per-request stream to a client that never opted in — a direct violation of the spec's MUST NOT.\n\nNotably, the re-admitted e2e row mcpserver:context:log-from-handler on entryModern (handler-context.test.ts) uses a default Client that never sets a logLevel, so that test passes only by virtue of this violation — it pins the wrong behavior on the modern cell.\n\nWhy the existing code doesn't prevent it / why legacy is fine. On the 2025 era the "no level set ⇒ send" posture is correct (that was the logging/setLevel semantics, and the legacy branch reads this._loggingLevels), but the same posture transplanted onto the 2026 envelope key inverts the new opt-in model. The 2026-07-28 LoggingMessageNotification doc reiterates the opt-in semantics.\n\nFix. In the modern-era branch, return early (Promise.resolve()) when ctx.mcpReq.envelope?.[LOG_LEVEL_META_KEY] is undefined; only when a level is present should the severity comparison run. The e2e expectation for entryModern should then either set a logLevel in the request envelope or assert that no notifications/message is delivered without one.

@felixweinberger felixweinberger force-pushed the fweinberger/on-small-fixes branch from 761bd17 to e230a8e Compare June 19, 2026 14:37
Comment on lines +324 to +330
// Emit request-related (like progress and `ctx.mcpReq.notify`)
// so the notification rides the in-flight exchange. Without the
// related-request stamp, per-request hosting (`createMcpHandler`,
// either era) silently drops the message because it has no
// session-wide stream to deliver it on.
return ctx.mcpReq.notify({ method: 'notifications/message', params: { level, data, logger } });
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Routing every ctx.mcpReq.log() through ctx.mcpReq.notify() stamps relatedRequestId on legacy (2025-era) sessionful Streamable HTTP transports too, moving handler logs off the session-scoped standalone/eventStore channel onto the in-flight request's POST SSE stream — and after ctx.http.closeSSE() that stream is gone from _streamMapping, so a log emitted before the client reconnects is silently dropped instead of stored for replay (the SEP-1699 sse-polling flow). The only coverage of that flow is excluded from run:examples in this same PR, and the changeset/migration docs present legacy behavior as unchanged. Either keep the session-scoped sendLoggingMessage routing for legacy-era requests (use ctx.mcpReq.notify only on the modern/per-request path), or make the related-request branch store-while-disconnected and document the legacy delivery-channel change rather than excluding the example.

Extended reasoning...

What changes. Pre-PR, ctx.mcpReq.log() delegated to this.sendLoggingMessage()Protocol.notification(), which carries no relatedRequestId. Post-PR, the new log callback in Server.buildContext (packages/server/src/server/server.ts:324-330) returns ctx.mcpReq.notify(...), and the protocol layer always stamps relatedRequestId: request.id on related notifications — for both eras, not just the per-request hosting path the PR is motivated by.

Why that matters on a legacy sessionful transport. In WebStandardStreamableHTTPServerTransport.send() the routing is keyed entirely on relatedRequestId (packages/server/src/server/streamableHttp.ts:960-1014):

  • No relatedRequestId (the old behavior): the message takes the standalone-stream branch, where it is stored in the eventStore even while the stream is disconnected ("Store even if stream is disconnected so events can be replayed on reconnect", lines 976-988) and otherwise written to the standalone GET stream — which closeSSE() never touches.
  • With relatedRequestId (the new behavior): the message takes the per-request branch, where eventStore.storeEvent and the write happen only inside if (!this._enableJsonResponse && stream?.controller && stream?.encoder) (lines 1005-1014). closeSSEStream(requestId) calls stream.cleanup(), which removes the stream from _streamMapping (lines 939-947) — so once the request stream has been closed, a handler log is neither delivered nor stored. It is silently dropped.

Step-by-step proof (the SEP-1699 sse-polling example). examples/sse-polling/server.ts runs exactly this flow on a sessionful 2025-era transport with an eventStore: (1) the tool handler logs early progress, (2) calls ctx.http.closeSSE() to close the request's SSE stream (~400ms), (3) logs 'Progress: 75%' ~200ms later while the client is still waiting out its 300ms retryInterval, (4) the client reconnects with Last-Event-ID and client.ts asserts the 75% line was delivered/replayed. Pre-PR, step 3's notification went out without relatedRequestId, so it was stored under the standalone stream id regardless of connection state and replayed on reconnect — the assertion held deterministically. Post-PR, step 3's notification carries relatedRequestId, hits the per-request branch with the stream already cleaned up, and is dropped unless the client's reconnect happens to win the race. The example's outcome is now a timing coin-flip.

Why nothing in the PR catches it — and the documentation gap. This same PR adds "excluded": "replay assertion is timing-sensitive on CI; revisit" to examples/sse-polling/package.json, i.e. the only coverage of the affected behavior is disabled in the PR that changes it. The exclusion commit's own message (fd07235) is more candid than the package.json prose: "the request-related routing change lands before the store-first fix, so the post-disconnect log is not reliably replayed on intermediate commits" — the regression is acknowledged but neither fixed nor documented. Meanwhile the changeset (.changeset/server-ctx-log-request-related.md) frames the change as making handler logs delivered "instead of being silently dropped", the PR description says legacy keeps only "the existing _loggingLevels map lookup unchanged" and declares "Breaking Changes: None", and migration.md / migration-SKILL.md only cover the cancel change. None of them mention that on a legacy sessionful transport the delivery channel — and, with closeSSE/eventStore or enableJsonResponse, deliverability itself — changes.

Impact. Any existing legacy sessionful deployment that uses ctx.mcpReq.log() together with closeSSE()+eventStore (the documented SEP-1699 polling pattern) loses log notifications emitted while the request stream is closed; the same applies on enableJsonResponse transports, where the per-request branch never writes notifications at all. These are messages that previously reached the client or were buffered for replay, now dropped with no error.

Fix. Either (a) scope the new routing to the path that needs it: in the log callback, use ctx.mcpReq.notify only when this._servedModernEra() (or when there is no session-wide channel), and keep the previous sendLoggingMessage() session-scoped routing for legacy-era requests; or (b) land the store-while-disconnected fix for the related-request branch in send() together with this change, document the legacy delivery-channel change in the changeset and migration docs, and re-enable the sse-polling example instead of excluding it.

Comment on lines +312 to +323
// Level filter: on a 2026-era request the client declares its
// threshold per request via the `_meta.logLevel` envelope key
// (the modern equivalent of `logging/setLevel`, which is not a
// request method on that revision); on 2025-era connections the
// session-scoped level set via `logging/setLevel` applies
// exactly as before.
const threshold = this._servedModernEra()
? (ctx.mcpReq.envelope?.[LOG_LEVEL_META_KEY] as LoggingLevel | undefined)
: this._loggingLevels.get(undefined);
if (threshold !== undefined && this.LOG_LEVEL_SEVERITY.get(level)! < this.LOG_LEVEL_SEVERITY.get(threshold)!) {
return Promise.resolve();
}

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 legacy-era branch of the new ctx.mcpReq.log filter looks up the threshold with this._loggingLevels.get(undefined), but logging/setLevel stores the level keyed by the transport session id — so on a sessionful 2025-era connection a client's logging/setLevel('error') is never consulted and every ctx.mcpReq.log('debug', ...) message is delivered. The pre-PR sendLoggingMessage delegation had the same hole (no behavioral regression), but this PR rewrites the filter inline with ctx.sessionId in scope and adds a comment claiming the session-scoped level "applies exactly as before"; the lookup should be this._loggingLevels.get(ctx.sessionId) ?? this._loggingLevels.get(undefined) (or just reuse isMessageIgnored(level, ctx.sessionId)).

Extended reasoning...

What the bug is. The new inline log callback in Server.buildContext (packages/server/src/server/server.ts:308-330) branches the level filter by era. On the legacy (2025-era) branch it reads the threshold with this._loggingLevels.get(undefined). However, the logging/setLevel handler installed by _registerLoggingHandler (server.ts:286-293) stores the client's chosen level keyed by the transport session idctx.sessionId or the mcp-session-id header. On any sessionful 2025-era Streamable HTTP connection the level therefore lives under the session-id key, the get(undefined) lookup returns undefined, the threshold !== undefined guard is skipped, and the notification is always emitted.

Concrete walk-through.

  1. A client connects over a sessionful 2025-era Streamable HTTP transport to a server with the logging capability; the transport assigns session id abc.
  2. The client sends logging/setLevel { level: 'error' }. The handler computes transportSessionId = 'abc' and stores _loggingLevels.set('abc', 'error').
  3. A tool handler calls ctx.mcpReq.log('debug', 'verbose detail').
  4. In the new log callback, this._servedModernEra() is false (legacy connection), so threshold = this._loggingLevels.get(undefined)undefined (the only entry is keyed 'abc').
  5. The filter if (threshold !== undefined && severity < thresholdSeverity) is skipped, and ctx.mcpReq.notify({ method: 'notifications/message', params: { level: 'debug', ... } }) goes out — even though the client explicitly asked for error and above only.

Why nothing else prevents it. The only other check on this path is assertNotificationCapability('notifications/message'), which validates the server's logging capability, not the client's level. The session-keyed filtering logic that does the right thing — isMessageIgnored(level, sessionId) at server.ts:352-355 — is bypassed because the new code no longer routes through sendLoggingMessage.

Why this belongs in this PR. To be clear about scope: the pre-PR code (log: (level, data, logger) => this.sendLoggingMessage({ level, data, logger }) with no sessionId argument → isMessageIgnored(level, undefined)) had exactly the same lookup hole, so this is not a behavioral regression introduced here. But the PR rewrites this exact filter inline, has ctx.sessionId directly in scope, and adds a comment asserting that on 2025-era connections "the session-scoped level set via logging/setLevel applies exactly as before" — when the session-scoped level is in fact never read on this path. The ServerContext.log JSDoc in protocol.ts likewise promises "Respects the client's log level filter set via logging/setLevel". With the code being rewritten and re-documented here, this is the natural — and trivial — place to close the gap.

Impact. Sessionful legacy clients that lower their log verbosity via logging/setLevel still receive every handler-emitted log message (debug/info noise on the wire), contradicting both the spec semantics of logging/setLevel and the SDK's documented contract for ctx.mcpReq.log.

Fix. In the legacy branch, change the lookup to this._loggingLevels.get(ctx.sessionId) ?? this._loggingLevels.get(undefined) (the fallback covers stateless transports where setLevel was stored under undefined), or simply replace the inline comparison with the existing this.isMessageIgnored(level, ctx.sessionId) helper, which already performs the session-keyed lookup and severity comparison.

@felixweinberger felixweinberger force-pushed the fweinberger/sep2243-std branch 3 times, most recently from a271dc7 to 9e9ede2 Compare June 22, 2026 11:41
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