fix(client): abort legacy SSE reconnect chain when the originating request times out

[claude]

Requested by Felix Weinberger · Slack thread

Fixes #2615.

The issue reporter explicitly asked for maintainer guidance on where the ownership boundary for this fix should sit (request-lifecycle abort signal vs. per-request reconnect disposal vs. a transport teardown hook). This PR is a concrete proposal for that discussion: it uses the request-lifecycle abort signal that already exists for the modern era, since the transport already threads and honors it end-to-end.

Root cause

On a legacy (2025-11-25) Streamable HTTP session, request lifecycle ownership is split:

• Protocol._requestWithSchemaViaCodec (packages/core-internal/src/shared/protocol.ts) owns timeout/cancellation and response-handler cleanup, and on the legacy era cancels by POSTing notifications/cancelled.
• StreamableHTTPClientTransport (packages/client/src/client/streamableHttp.ts) owns the request's SSE reconnect chain (GET + Last-Event-ID resumption) and already guards every leg of it with options.requestSignal — the fetch signal, the scheduled-reconnect callback, and the "should I reschedule" checks.

But the per-request AbortController was only created when streamCloseCancels was true (modern era × per-request-stream transport). On the legacy era requestSignal was never threaded, so those transport guards were permanently inert: after the request settled via timeout or caller abort, the reconnect chain kept resuming — each successful resume re-enters _scheduleReconnection with attemptCount 0, so maxRetries never binds — until a late resumed GET delivered the original response, which then surfaced through Protocol._onresponse as Received a response for an unknown message ID.

Fix

Create the request-scoped AbortController for every per-request-stream transport regardless of era, and abort it whenever the request settles through cancel():

• Modern era (2026-07-28): unchanged — aborting the stream is the spec cancel signal; no notifications/cancelled is sent.
• Legacy era (2025-11-25) on a per-request-stream transport: the notifications/cancelled POST stays the wire signal, exactly as before; the abort is purely local teardown that stops the transport's reconnect chain for that request. The cancelled POST does not carry the signal, so the abort cannot cut it off.
• Single-channel transports (stdio / in-memory): unchanged — no requestSignal is threaded (hasPerRequestStream is not set).

No transport changes were needed: the requestSignal plumbing (including the intentional-abort guards that suppress spurious onerror/reconnects) already existed for the modern era and is era-agnostic.

Tests

Written failing-first against main (cc4b416):

• packages/client/test/client/streamableHttp.test.ts — end-to-end regression: a real Client over a mocked-fetch legacy session sends a request with a short timeout, the server primes and closes the SSE stream so the resume chain runs, the request times out, and the test asserts exactly one notifications/cancelled POST, zero resumed GETs after settlement, and no "unknown message ID" error when the late response would have arrived. Fails on main (the post-settlement resume fires and delivers the late response), passes with the fix. Plus a transport-level test that an abort landing while a reconnect is scheduled prevents the pending GET.
• packages/core-internal/test/shared/protocol.test.ts — the (era × transport) cancellation matrix: legacy + per-request-stream now asserts notifications/cancelled and an aborted requestSignal for both caller abort and timeout; legacy + single-channel asserts no requestSignal is threaded; the existing modern-era tests pin that stream-close-cancels behavior is unchanged.

pnpm --filter test runs: core-internal 1435 passed, client 799 passed, server 468 passed; lint and typecheck clean on touched packages (pre-push hook also ran workspace-wide build/typecheck/lint).

A changeset (patch, @modelcontextprotocol/client) is included.

Not addressed here (possible follow-up, related to #2098): the retry counter resetting to 0 on every successful resume, which makes maxRetries a per-gap rather than per-request budget.

---

Generated by Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: 87ebece

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

This PR includes changesets to release 6 packages

Name	Type
@modelcontextprotocol/client	Patch
@modelcontextprotocol/server	Patch
@modelcontextprotocol/core	Patch
@modelcontextprotocol/server-legacy	Patch
@modelcontextprotocol/codemod	Patch
@modelcontextprotocol/core-internal	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]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2616

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2616

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2616

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2616

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2616

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2616

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2616

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2616

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2616

commit: 87ebece

[claude]

Beyond the inline finding on the resumptionToken short-circuit's unguarded .catch, this pass also verified that 29c4e86 fully resolves both earlier findings: the .finally() requestAbort?.abort() covers the maxTotalTimeout and success settlement paths (with era-matrix regression tests), and the cancellation POST no longer inherits resumption options. I also checked the other consumers of the newly-always-aborted requestSignal in streamableHttp.ts — _handleSseStream and _scheduleReconnection's reconnect callback both gate onerror/reconnect on isIntentionalAbort(), so the short-circuit .catch flagged inline is the only unguarded site.

Extended reasoning...

Bugs were found this run, so the inline comments carry the substance; this note records what was additionally examined and ruled out. I independently confirmed the inline finding (streamableHttp.ts ~961: the outer catch fires onerror unconditionally while the inner catch at ~632 suppresses-then-rethrows on intentional abort), verified the prior round's two red findings are genuinely fixed in 29c4e86 with pinned tests, and grepped the remaining requestSignal consumers in the transport for the same missing-guard pattern — all sibling sites are guarded. The two other findings are prose-only nits (changeset scoping, one stale doc site), so the code change itself is close to done, but the unguarded onerror path warrants the author's attention before merge.

[claude]

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

• 🔴 packages/core-internal/src/shared/protocol.ts:1590-1592 — Widening timeoutHandler to (error?: Error) => cancel(error ?? new SdkError(...)) introduces a Firefox regression: the handler is passed unwrapped to setTimeout in _setupTimeout (line 750) and _resetTimeout (line 773), and Gecko invokes timer callbacks with an extra 'lateness' Number argument — since ?? only filters null/undefined, that number becomes the cancel reason, so every per-leg timeout in Firefox rejects with message '0' instead of 'Request timed out' (losing the { timeout } data) and, on a legacy (2025-11-25) session, POSTs notifications/cancelled with reason: '0'. One-line fix: wrap the timer callback (setTimeout(() => onTimeout(), timeout) at both sites) or guard with error instanceof Error ? error : new SdkError(...) in timeoutHandler.

  Extended reasoning...

  What the bug is

  This PR widens the per-request timeout callback from () => void to (error?: Error) => void so _onprogress can route the maxTotalTimeout settlement through cancel(). The new handler at packages/core-internal/src/shared/protocol.ts:1591-1592 is:

  const timeoutHandler = (error?: Error) =>
      cancel(error ?? new SdkError(SdkErrorCode.RequestTimeout, 'Request timed out', { timeout }));

  But the function is still handed directly to the host timer in two places: _setupTimeout (timeoutId: setTimeout(onTimeout, timeout), line 750) and _resetTimeout's re-arm (info.timeoutId = setTimeout(info.onTimeout, info.timeout), line 773).

  In Firefox (Gecko), setTimeout invokes its callback with an extra argument: the lateness of the timeout in milliseconds — a Number, typically 0–3, occasionally negative. This is long-standing, MDN/BCD-documented, spec-nonconforming Gecko behavior (whatwg/html#4108) — the well-known reason new Promise(r => setTimeout(r, 100)) resolves to a number in Firefox. So on Firefox, every plain per-leg timeout calls timeoutHandler(lateness) with a number, and because ?? filters only null/undefined (0 and other numbers pass through), the SdkError(RequestTimeout, 'Request timed out', { timeout }) branch is skipped and cancel(lateness) runs with a bare number as the reason.

  Code path and impact

  Inside cancel() (protocol.ts ~1465–1520):

  1. reason instanceof SdkError is false, so the caller's rejection becomes new SdkError(SdkErrorCode.RequestTimeout, String(lateness)) — message '0'/'2' instead of 'Request timed out', with the { timeout } data lost. The SDK's own tests pin the message contract (rejects.toThrow('Request timed out')), and host code matching that documented message breaks on Firefox clients. (The error code survives, so code-based matching still works — but the message and data do not.)
  2. On a legacy (2025-11-25) connection the notifications/cancelled POST goes out with params.reason: String(reason) — i.e. reason: '0' — a meaningless cancellation reason on the wire, where it previously read 'MCP error -32001: Request timed out ...'.

  Why nothing catches it

  • Type checking: TypeScript's setTimeout callback type is (...args: any[]) => void, so passing the one-arg handler type-checks cleanly.
  • Tests: Node, Workers, and Deno pass no extra timer arguments, so no CI leg can ever exercise this — only Firefox at runtime. Browsers are an explicitly supported client target of this SDK (CLAUDE.md's runtime-neutral root-entry / browser-bundler constraint; StreamableHTTPClientTransport in browsers).
  • Regression, not pre-existing: before this PR the handler was the zero-arg closure () => cancel(new SdkError(...)), so Gecko's extra argument was silently ignored. The widening is exactly this PR's maxTotalTimeout-reroute change (c1b55eb).

  Step-by-step proof

  1. A browser client in Firefox calls client.callTool(..., { timeout: 5000 }) over a legacy (2025-11-25) Streamable HTTP session.
  2. The server is slow; after 5000ms Gecko fires the timer as onTimeout(0) (lateness 0ms).
  3. timeoutHandler(0) evaluates 0 ?? new SdkError(...) → 0 (?? passes 0 through), so cancel(0) runs.
  4. 0 instanceof SdkError is false → the caller's promise rejects with SdkError(RequestTimeout, '0') — no 'Request timed out' message, no { timeout: 5000 } data.
  5. streamCloseCancels is false on the legacy era, so the notifications/cancelled POST is sent with reason: String(0) = '0'.
  6. In Chrome/Node the same code produces timeoutHandler(undefined) → the correct 'Request timed out' error — the divergence is Firefox-only and deterministic on every per-leg timeout there.

  This needs no maxTotalTimeout involvement — it is the mainline timeout path of every Firefox client.

  How to fix

  Either wrap the timer callback so host-supplied arguments never reach the handler:

  // _setupTimeout
  timeoutId: setTimeout(() => onTimeout(), timeout),
  // _resetTimeout
  info.timeoutId = setTimeout(() => info.onTimeout(), info.timeout);

  or harden the handler itself:

  const timeoutHandler = (error?: Error) =>
      cancel(error instanceof Error ? error : new SdkError(SdkErrorCode.RequestTimeout, 'Request timed out', { timeout }));

  Either is a one-line change per site; the instanceof guard has the advantage of protecting any future caller of onTimeout as well.

• 🟣 packages/client/src/client/streamableHttp.ts:687-699 — Pre-existing issue (not introduced by this PR): both reconnect-rebuild sites in _handleSseStream (the graceful-close branch at ~801 and the stream-error branch at ~834) rebuild StartSSEOptions with resumptionToken: lastEventId — a leg-local variable assigned only when the current stream leg delivers an event with an id — with no fallback to options.resumptionToken, the token the leg was opened with. If a resume GET opens and then drops (or closes gracefully) before its first event, the next leg is scheduled with resumptionToken: undefined, degrading the resume into a token-less standalone GET: on a server with no standalone stream (the 405 shape both new tests in this PR model) the chain terminates as non-resumable and the request hangs to timeout, and on a server that does offer one the client opens a spurious extra subscription still carrying replayMessageId while the real replay is lost. One-line fix at both sites: resumptionToken: lastEventId ?? options.resumptionToken.

  Extended reasoning...

  What the bug is

  In _handleSseStream (packages/client/src/client/streamableHttp.ts), the leg-local let lastEventId: string | undefined (~line 731) is assigned only when the current stream leg delivers an SSE event carrying an id (~line 763). Both reconnect-scheduling sites — the graceful-close branch (~line 801) and the stream-error branch (~line 834) — rebuild the next leg's StartSSEOptions with resumptionToken: lastEventId and nothing else. Crucially, _handleSseStream never reads options.resumptionToken at all: its destructuring at ~line 723 pulls only { onresumptiontoken, replayMessageId, requestSignal, onRequestStreamEnd }. So the token a resume leg was opened with is discarded the moment that leg ends, unless the leg happened to deliver a fresh event id first.

  This comment is anchored on the in-diff reconnect scheduler (_scheduleReconnection's reconnect() callback, lines ~687-699), which is the direct consumer that re-runs the flawed options; the two offending rebuild sites sit just below it, outside the diff.

  The code path that triggers it

  For a resume GET leg, _startOrAuthSse calls _handleSseStream(response.body, options, /* isReconnectable */ true), so canResume = true regardless of hasPrimingEvent. If that leg ends — a graceful server close or a network drop (e.g. a proxy cutting an idle stream during a long quiet gap, exactly when long-running requests emit nothing between the priming event and the response) — before any event arrives, then lastEventId is still undefined, receivedResponse is false, and no intentional abort occurred, so needsReconnect is true and the next leg is scheduled with resumptionToken: undefined. _startOrAuthSse sets the last-event-id header only under if (resumptionToken), so the next GET goes out with no Last-Event-ID header — the wire shape of a standalone notification stream, not a resume. (Note the SSE contract itself — EventSource semantics — persists Last-Event-ID across reconnects even when a leg delivers no events; this code does not.)

  Impact

  • Server with no standalone GET (405) — the shape both new tests in this PR model: _startOrAuthSse's 405 branch fires options.onRequestStreamEnd?.() and returns. The chain terminates as "non-resumable" even though the request was resumable. And for a plain protocol-layer request, _requestWithSchemaViaCodec's transport.send options never thread onRequestStreamEnd at all — so nothing fires, the buffered response is never replayed, and the request silently hangs until timeout.
  • Server that does offer a standalone stream: the client opens a spurious extra standalone subscription that still carries the original replayMessageId — any response delivered on that stream gets remapped onto the pending request's id — while the actual replay from the real token never happens; the original response is lost and the request times out.

  Either way, one empty resume leg permanently destroys resumability for that request.

  Step-by-step proof

  Legacy (2025-11-25) session, per-request-stream transport:

  1. A request's POST SSE stream is primed with id: evt-1 and closes gracefully without the response → _handleSseStream schedules a reconnect with resumptionToken: 'evt-1'. Correct so far.
  2. The resume GET goes out with Last-Event-ID: evt-1, gets 200/text/event-stream, and _handleSseStream(body, options, true) starts a fresh leg — with a fresh lastEventId = undefined.
  3. Before any event arrives, the connection drops (proxy idle-timeout) or the server closes gracefully. isReconnectable = true, receivedResponse = false, no intentional abort → the reschedule fires with resumptionToken: lastEventId = undefined. The token evt-1 — still in options.resumptionToken — is never consulted.
  4. _startOrAuthSse sends a GET with no Last-Event-ID header. On a 405 server the chain dies as non-resumable; on a standalone-offering server a stray subscription with a stale replayMessageId is opened. In both cases the request hangs to its timeout and the buffered response at evt-1 is never replayed.

  Why the PR's tests mask it

  Every leg in this PR's new tests is primed with id: evt-N before closing, so lastEventId is always populated when the reschedule runs and the empty-leg path is never exercised. The window between resume-GET response headers and the first event is precisely where this hides.

  Why it's in scope despite pre-dating the PR

  The two lines are outside the diff and predate it — hence pre_existing severity, not blocking. But the PR's entire subject is the correctness of this exact reconnect chain: it edits _scheduleReconnection's reconnect callback (the direct consumer of these rebuilt options), threads requestSignal/onRequestStreamEnd through these very option-rebuild sites, and adds tests over the chain. This repo's REVIEW.md Completeness guidance (flag sibling code paths carrying the bug class a PR is fixing) applies.

  Fix

  One line at each site (~801 and ~834):

  resumptionToken: lastEventId ?? options.resumptionToken,

  This preserves the invariant "resume from the last event id in hand": a leg that saw newer events advances the token; a leg that saw none falls back to the token it was opened with.

[claude]

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

• 🟡 packages/core-internal/src/shared/protocol.ts:1505-1533 — Two sites still violate the clean-shutdown onerror contract this PR's sweep establishes (intentional aborts must never surface through onerror): (1) cancel()'s cancellation-send catch here (.catch(error => this._onerror(new Error('Failed to send cancellation: ...')))) reports unconditionally, so the ordinary await client.close() right after a timeout rejection on a legacy session surfaces a spurious transport-shaped AbortError through client.onerror — the transport's new guard correctly stays silent and rethrows, but this protocol-layer catch re-reports it (and the c1b55eb maxTotalTimeout reroute adds a brand-new emission path through it); (2) the same unguarded catch-report-rethrow shape survives, pre-existing, in the legacy SSE transport at packages/client/src/client/sse.ts:409-411, whose POST runs on the transport-lifetime signal alone. A transport-liveness/intentional-abort guard at each site (mirroring the terminateSession fix) keeps genuine send failures reporting while silencing the deliberate-shutdown case.

  Extended reasoning...

  What the bug is

  This PR's onerror-discipline sweep (e00d771 → 248cb69) establishes, documents (docs/advanced/custom-transports.md: an intentional abort "should be treated as a clean shutdown — swallow it rather than surfacing it through onerror"), and regression-tests ('notification POST: transport.close() while the POST is in flight surfaces no spurious onerror') the contract that a deliberate abort must never reach onerror. Two sibling sites of the swept catch shape remain.

  Site 1 — in this PR's hunk: protocol.ts cancel()'s cancellation-send catch. The notifications/cancelled send at the lines this diff edits ends in an unconditional .catch(error => this._onerror(new Error(\Failed to send cancellation: ${error}`)))`. The transport-level guard this PR added at the POST-path catch (streamableHttp.ts:1224) deliberately RETHROWS ("still rethrow so ... the protocol layer settle their state machines"), and this catch is the protocol-layer consumer of that rethrow — with no intentional-abort or connection-closed discrimination.

  Site 2 — pre-existing: packages/client/src/client/sse.ts:409-411. The legacy SSE transport's _send catch is byte-for-byte the shape the sweep replaced: this.onerror?.(error as Error); throw error;. Its POST (sse.ts:358-364) runs on signal: this._abortController?.signal alone (no requestSignal exists on this transport), and close() (sse.ts:340-344) calls this._abortController?.abort() unconditionally — the identical one-signal failure mode this PR fixed at streamableHttp.ts's POST catch and at terminateSession in 248cb69. SSEClientTransport is deprecated but still exported from packages/client/src/index.ts, and Protocol.connect wires transport.onerror through to client.onerror, so the spurious AbortError reaches applications.

  Step-by-step proof (site 1, deterministic — no race)

  1. Legacy-era (2025-11-25) Streamable HTTP session (any !streamCloseCancels connection). A request times out → cancel() fire-and-forget-initiates the notifications/cancelled POST (async: it first awaits _commonHeaders(), then fetch) and synchronously rejects the caller.
  2. The caller's rejection handler does the ordinary thing after a timeout: await client.close(). This runs within microtasks while the cancelled POST is still in flight (its network round trip takes real time).
  3. close() → _abortController.abort(); Protocol._onclose() runs (this._transport becomes undefined).
  4. The POST's fetch rejects with AbortError. The transport's outer catch — the very guard this PR added — correctly stays silent (transport signal aborted; the cancellation send passes only { relatedRequestId }, no requestSignal) and rethrows.
  5. The rejection lands in the unconditional protocol-layer catch → this._onerror(new Error('Failed to send cancellation: AbortError...')) → client.onerror fires a transport-shaped error during a deliberate clean shutdown — exactly the failure mode the PR's transport-level tests pin as fixed.

  For site 2, the same sequence is even simpler: any close() landing while an SSE-transport POST is in flight rejects the fetch with AbortError, and the catch reports it unconditionally.

  Why existing code doesn't prevent it

  The transport-level suppression is one layer down and intentionally rethrows; nothing in cancel()'s catch (or sse.ts's) discriminates an intentional abort from a genuine send failure. Protocol._onerror is a bare this.onerror?.(error) with no post-close guard, and the onerror callback survives _onclose.

  In-PR relevance

  The cancellation-send options hunk containing this catch is edited by this diff (the resumptionToken-drop fix), and the c1b55eb maxTotalTimeout reroute adds a brand-new emission path through this same catch (maxTotalTimeout settlements previously never POSTed anything, so the spurious report is newly reachable there). Pre-PR, the close-after-timeout sequence produced this message PLUS the transport's unguarded AbortError; the PR silenced the transport half and left this sibling. REVIEW.md's Completeness catch applies: "when a PR replaces a pattern (catch shape), grep the package for surviving instances of the old form — flag every leftover site" — the same basis on which the terminateSession leftover was already confirmed and fixed on this PR.

  Impact

  Consumers that log or alert on client.onerror see a spurious "Failed to send cancellation: AbortError" (or a raw AbortError, for the SSE transport) on every clean shutdown that races an in-flight POST. Nothing functionally breaks — the caller's promise settles correctly and the rethrow is preserved — which is why this is a nit rather than blocking.

  How to fix

  • protocol.ts: guard the report in cancel()'s send-catch on transport liveness or intentional abort — e.g. skip _onerror when this._transport === undefined or when the error is a ConnectionClosed/abort. By the time the rejection lands, _onclose has already cleared _transport, so the guard is reliable, and genuine cancellation-send failures on a live connection still report.
  • sse.ts (pre-existing; non-blocking mention): mirror the two sites this PR already fixed — if (this._abortController?.signal.aborted !== true) { this.onerror?.(error as Error); } with the rethrow kept.

• 🟣 packages/core-internal/src/shared/protocol.ts:1441-1458 — Pre-existing issue (the operative lines pre-date this PR): RequestOptions is declared as {...} & TransportSendOptions, so requestSignal and onRequestStreamEnd type-check on every high-level request API — but the funnel never forwards them: the destructure at protocol.ts:1394 drops onRequestStreamEnd, and the send at :1617 unconditionally replaces any caller-supplied requestSignal with the internal requestAbort?.signal, making both dead API surface through client.request(). This matters more after this PR's transport-side work: the resumptionToken short-circuit now fires onRequestStreamEnd on every terminal non-resumable outcome "so the caller can settle" — but through the protocol funnel there is never a callback to fire, so a client.request(req, schema, { resumptionToken }) whose resume GET dead-ends (405/null body/fetch rejection) dangles until the full request timeout while an identical direct transport.send caller settles immediately. Suggested: have the funnel supply its own onRequestStreamEnd that settles the pending request promptly, and/or narrow the type with Omit<TransportSendOptions, 'requestSignal' | 'onRequestStreamEnd'> (or document that these members are protocol-owned on this path).

  Extended reasoning...

  The gap

  RequestOptions is declared as {...} & TransportSendOptions (packages/core-internal/src/shared/protocol.ts:158), so requestSignal and onRequestStreamEnd are publicly-typed, compile-clean members of the options accepted by every high-level request API — client.request, client.callTool, client.ping, ctx.mcpReq.send. But _requestWithSchemaViaCodec forwards only a subset:

  • protocol.ts:1394: const { relatedRequestId, resumptionToken, onresumptiontoken, headers } = options ?? {}; — onRequestStreamEnd is never destructured, so a caller-supplied callback never fires.
  • protocol.ts:1617: this._transport.send(outbound, { relatedRequestId, resumptionToken, onresumptiontoken, headers, requestSignal: requestAbort?.signal }) — a caller-supplied options.requestSignal is unconditionally replaced by the internal per-request controller's signal (or by undefined on single-channel transports). Aborting a caller-supplied requestSignal does nothing, despite its TransportSendOptions JSDoc promising "Aborting it cancels the underlying request".

  Both members are dead API surface at the protocol layer. Grep confirms no other site forwards a caller-supplied onRequestStreamEnd from RequestOptions — the only live consumers are direct transport.send callers (the subscriptions/listen driver at client.ts:2110-2113, and this PR's transport-level tests).

  Why it matters more after this PR

  This PR's transport-side work (c1b55eb) made the resumptionToken short-circuit forward onresumptiontoken/onRequestStreamEnd/requestSignal into _startOrAuthSse and fire the stream-end callback on every terminal non-resumable outcome, with the in-diff comment: "this send() already resolved fire-and-forget — fire the stream-end callback so the caller can settle." But through the protocol funnel there is never a callback to fire — and the PR's own e2e regression test drives resumption through exactly this path (client.ping({ timeout: 100, resumptionToken: 'evt-0' })), so re-issuing a request with a resumption token via client.request is a supported, tested pattern. Additionally, the PR's rewritten JSDoc on Transport.hasPerRequestStream / TransportSendOptions.requestSignal (transport.ts:130-143) now describes requestSignal as protocol-layer-owned, which sharpens the footgun: the same field name means "protocol-internal, will be replaced" at one layer and "caller-suppliable" at the other, with nothing in the RequestOptions docs saying so.

  Step-by-step proof

  1. Legacy or modern Streamable HTTP session. Caller follows the documented resume pattern through the Client API: client.request(req, schema, { resumptionToken: persistedToken, onRequestStreamEnd: () => settle(), timeout: 60_000 }) — compiles cleanly; RequestOptions admits both members.
  2. The funnel's destructure at :1394 drops onRequestStreamEnd; the send at :1617 passes requestSignal: requestAbort?.signal and no onRequestStreamEnd.
  3. The transport sees the truthy resumptionToken and short-circuits into GET + Last-Event-ID; the server answers 405 (does not offer GET resume). _startOrAuthSse fires options.onRequestStreamEnd?.() — which is undefined, because the funnel never passed one.
  4. send() already resolved fire-and-forget; no response will ever arrive. The caller's callback never fires; the request settles only via the 60s default timeout. A direct transport.send caller with the identical options settles immediately — the PR's own new transport tests pin exactly that behavior ("resumptionToken send: forwards onRequestStreamEnd so a terminal non-resumable outcome settles the caller").

  Why nothing else prevents it

  The transport-side forwarding fixes made on this PR cannot resolve this: they operate one layer down, on options the funnel never supplies. The caller's documented cancellation channel (RequestOptions.signal) does still work fully — it routes through cancel() which aborts requestAbort — and the request does eventually settle via timeout (which post-PR now cleanly tears down the reconnect chain). Those mitigations are why this is delayed settlement plus a type-surface footgun, not a hang or correctness failure.

  Why pre_existing

  The operative lines — the :1394 destructure and the :1617 send-options shape — pre-date this PR unchanged; the PR only changed which internal signal populates requestSignal. Nothing this PR ships breaks; the PR actually improved the timeout-settlement teardown on this path. Flagged because the PR's transport work built and documented the very settlement machinery (onRequestStreamEnd "so the caller can settle") that remains unreachable through client.request(), and its own e2e tests exercise the affected path.

  Suggested fix (either or both)

  • Have the funnel supply its own onRequestStreamEnd that settles the pending request promptly (e.g. cancel(new SdkError(..., 'request stream ended without a response'))) — fixing dangling-until-timeout for every terminal stream-end, not just resumes; and/or
  • Narrow the type with Omit<TransportSendOptions, 'requestSignal' | 'onRequestStreamEnd'> on RequestOptions, or document that these two members are protocol-layer-owned on this path and caller-supplied values are ignored.

[claude]

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

• 🟣 packages/core-internal/src/shared/protocol.ts:738-748 — Pre-existing issue — the last surviving sibling of the falsy-request-id pattern this PR fixes at _oncancel: the debounce gate in _notificationViaCodec (protocol.ts:1698) uses !options?.relatedRequestId, so a notification sent with relatedRequestId: 0 (every peer's FIRST outbound request id, as this PR's own _oncancel comment and requestId-0 tests establish) is wrongly classified as 'simple' and debounced — violating the adjacent comment's own invariant ('no related request ID that could be lost'). With debouncedNotificationMethods configured, a same-tick pair coalesces into one send carrying the wrong (absent) relatedRequestId, which the Streamable HTTP server then routes to the standalone GET stream — dropped entirely if none is connected. Same one-line fix as the _oncancel hunk: options?.relatedRequestId === undefined.

  Extended reasoning...

  What the bug is

  This PR replaces the falsy request-id guard in Protocol._oncancel with an explicit === undefined check, with a comment and two regression tests establishing that JSON-RPC id 0 is a first-class value: _requestMessageId starts at 0 and post-increments, so every SDK peer's first outbound request carries id 0, and RequestIdSchema (z.union([z.string(), z.number().int()])) accepts it. One sibling of the exact same falsy-id bug survives in the same file, in _notificationViaCodec's debounce gate (protocol.ts:1698):

  const canDebounce = debouncedMethods.includes(notification.method) && !notification.params && !options?.relatedRequestId;

  Since !0 === true, a notification sent with relatedRequestId: 0 is classified as 'simple' and enters the debounce branch — directly contradicting the adjacent comment's stated invariant that debounced notifications have no related request ID that could be lost. (!notification.params is fine — params is an object or absent.)

  How relatedRequestId 0 arises

  • ctx.mcpReq.notify hardwires relatedRequestId: request.id (the sendNotification closure in _onrequest). An inbound request with id 0 occurs whenever the peer's first outbound request is not initialize: a client reconnecting with a preserved sessionId skips the handshake, so its first tools/call carries id 0; and server→client, the server's first createMessage/elicitation request is id 0 (servers never send initialize) — the Client owns this same code path.
  • The public NotificationOptions.relatedRequestId also accepts 0 directly.

  Notably, this PR's own new test.each maxTotalTimeout test pins requestId: 0 as exactly this kind of first-request value.

  Step-by-step proof

  Using the ProtocolOptions JSDoc's own example config debouncedNotificationMethods: ['notifications/tools/list_changed'] on a Streamable HTTP server:

  1. In one tick, sendToolListChanged() (no relatedRequestId) takes the debounce branch and queues the pending microtask send.
  2. A tool handler for inbound request id 0 calls ctx.mcpReq.notify({ method: 'notifications/tools/list_changed' }) → relatedRequestId: 0 → !0 === true → canDebounce is true → the method is already pending, so the call silently returns (dropped).
  3. The one send that fires carries the FIRST call's options (no relatedRequestId). The server transport's send() routes requestId === undefined messages to the standalone GET stream — and if no standalone stream is connected and no eventStore is configured, the notification is dropped entirely, even though request 0's SSE stream was live and would have delivered it.
  4. In the reverse arrival order, the queued send carries relatedRequestId: 0 and the subsequent unrelated list_changed is dropped, misrouting the coalesced notification onto request 0's per-request stream instead of the standalone stream.

  With any non-zero request id, canDebounce is false and both notifications are sent immediately with correct routing — id 0 is the only value that misbehaves, precisely the bug class this PR fixes at _oncancel.

  Why nothing else prevents it

  The existing test 'should NOT debounce a notification that has a relatedRequestId' pins the intended contract but only exercises string ids ('req-1', 'req-2'), so the falsy-zero hole is uncovered. Nothing upstream normalizes or rejects id 0.

  Why pre_existing, and the fix

  The line is untouched by this PR and the PR adds no new callers of the debounce path; triggering additionally requires the opt-in debouncedNotificationMethods config with a paramless listed notification from an id-0 request context — real but narrow, and not introduced here. It's flagged under the repo's Completeness rule: when a PR replaces a pattern, surviving instances of the old form in the same package should be caught. The fix is the same one-liner as the _oncancel hunk:

  const canDebounce = debouncedMethods.includes(notification.method) && !notification.params && options?.relatedRequestId === undefined;

  A regression test mirroring the existing relatedRequestId debounce test but with relatedRequestId: 0 would pin it, exactly as the PR's new id-0 _oncancel test does.

• 🟡 packages/core-internal/src/shared/protocol.ts:1451-1468 — Because RequestOptions is declared as { ... } & TransportSendOptions, the public request API (client.request(), callTool(), server.createMessage(), …) type-accepts requestSignal and onRequestStreamEnd — but _requestWithSchemaViaCodec destructures only { relatedRequestId, resumptionToken, onresumptiontoken, headers } and sends with requestSignal: requestAbort?.signal, so a caller-supplied requestSignal is silently overridden (now unconditionally, on every per-request-stream transport at either era) and onRequestStreamEnd is silently dropped. This is a pre-existing gap, but this PR deepens the trap: it expands the public docs of exactly these two options and its own comments call onRequestStreamEnd "the caller's only per-request settlement signal" on the resume path — yet client.request(req, schema, { resumptionToken, onRequestStreamEnd: cb }) type-checks and never fires cb. Either forward/compose these options in the funnel, or Omit them from the TransportSendOptions intersection in RequestOptions and document them as transport-level only.

  Extended reasoning...

  The gap

  RequestOptions (packages/core-internal/src/shared/protocol.ts:101-158) is declared as an intersection:

  export type RequestOptions = { onprogress?; signal?; timeout?; ... } & TransportSendOptions;

  so every public request method — client.request(), callTool(), server.createMessage(), ctx.mcpReq.send() — type-accepts all five TransportSendOptions fields, including requestSignal?: AbortSignal and onRequestStreamEnd?: () => void. But the request funnel forwards only four of them. _requestWithSchemaViaCodec destructures { relatedRequestId, resumptionToken, onresumptiontoken, headers } (protocol.ts:1404) and the transport send is:

  this._transport.send(outbound, { relatedRequestId, resumptionToken, onresumptiontoken, headers, requestSignal: requestAbort?.signal })

  A caller-supplied options.requestSignal is silently overridden by the protocol-owned per-request controller, and options.onRequestStreamEnd is silently dropped — it never reaches the transport on any era or transport.

  Why this PR makes the trap worse

  The drop itself pre-dates this PR (the send line was identical before; on legacy requestAbort was undefined, so the caller's option was equally discarded). But this PR interacts with it on three fronts. First, post-PR requestAbort exists on every per-request-stream transport at either era (the change at protocol.ts:1451-1468), so the requestSignal override is now unconditional wherever the option could matter. Second, the PR substantially expands the public documentation of exactly these two options — the TransportSendOptions.requestSignal/onRequestStreamEnd JSDoc in transport.ts, docs/advanced/custom-transports.md, support-2026-07-28.md, and upgrade-to-v2.md all describe rich per-request semantics, with nothing noting the options are transport-level only and inert when passed through RequestOptions. Third, the PR's own new comments on the transport's resumptionToken short-circuit call onRequestStreamEnd "the caller's only per-request settlement signal" — and the PR adds forwarding of options?.onRequestStreamEnd there — but only direct transport.send() callers benefit; the high-level funnel never threads it.

  Step-by-step proof

  1. A caller re-issues an interrupted request the documented way: client.request(req, schema, { resumptionToken: token, onRequestStreamEnd: cb, timeout: 60_000 }). This type-checks — both keys are on RequestOptions via the intersection.
  2. _requestWithSchemaViaCodec destructures resumptionToken (forwarded) but not onRequestStreamEnd (dropped) and sends with requestSignal: requestAbort?.signal (caller's requestSignal, if any, discarded).
  3. The transport's _send sees the truthy resumptionToken and short-circuits into a GET + Last-Event-ID resume, forwarding options?.onRequestStreamEnd — which is undefined, because the funnel never passed it.
  4. The resume GET dead-ends (405, null body, or a genuine open failure — exactly the terminal outcomes this PR's new short-circuit code fires the callback for). The transport invokes options?.onRequestStreamEnd?.() … on undefined. cb never runs.
  5. The caller learns of the terminal outcome only when the full request timeout expires — despite the PR's comments describing this callback as the caller's only per-request settlement signal on that path.

  Similarly, { requestSignal: mySignal } passed to any request method is silently ignored; callers must know to use options.signal instead — a differently-named field on the same options object with different semantics.

  Why nothing else prevents it

  The internal subscriptions/listen driver (client.ts ~2111-2112) confirms these two options are protocol/driver-owned plumbing: it bypasses the funnel and calls transport.send() directly with its own requestSignal/onRequestStreamEnd. The intersection type leaks that plumbing into the public API surface, and no runtime check or doc note warns the caller.

  Fix

  Either (a) forward onRequestStreamEnd from options into the transport send (composing with any future protocol-internal use) and compose the caller's requestSignal into requestAbort (abort on either), or — cheaper and probably right given the protocol layer owns per-request settlement — (b) change the intersection to Omit<TransportSendOptions, 'requestSignal' | 'onRequestStreamEnd'> so the public type stops advertising options the funnel discards, and note in the TransportSendOptions JSDoc that these two fields are transport-level only (honored on direct transport.send() calls, supplied by the protocol layer on the request path).

  This is a pre-existing type-surface issue and nothing breaks for callers using the documented options.signal path, so it is not blocking — but the PR's doc expansion of these exact options makes it materially more likely to bite, and the Omit fix is one line.

• 🟡 docs/migration/upgrade-to-v2.md:1825-1835 — The "Unchanged, for re-baselining relief" bullet (docs/migration/upgrade-to-v2.md:1504-1509) still says "The cancelled-on-timeout signal is unchanged on legacy-era connections and on stdio/in-memory at any era" — but this PR's maxTotalTimeout reroute makes exactly those combinations now POST notifications/cancelled when the total budget trips (previously nothing went on the wire), as the PR's own changeset and era-matrix test pin. Suggest scoping the sentence to per-leg timeouts / caller aborts and adding a clause noting that maxTotalTimeout settlements now emit the era's cancel signal where v1 emitted none. (This is a separate claim from the adjacent "SSE reconnection exhaustion" bullet at 1510-1514 — fixing that one leaves this one wrong.)

  Extended reasoning...

  What the guide now gets wrong

  The Behavioral changes › Error-shape changes (every era) section of docs/migration/upgrade-to-v2.md opens with a bullet whose explicit purpose is telling v1 migrators what they do not need to re-baseline (lines 1504-1509):

  Unchanged, for re-baselining relief: timeout rejections still carry data.timeout / data.maxTotalTimeout exactly as v1 McpError did — v1 assertions on those survive verbatim. The cancelled-on-timeout signal is unchanged on legacy-era connections and on stdio/in-memory at any era; …

  The bullet's first sentence explicitly names data.maxTotalTimeout, so maxTotalTimeout settlements are squarely within its stated scope. After this PR, the "unchanged" claim is false for exactly that flavor of timeout on exactly the combinations the sentence names.

  The code path that changed

  Pre-PR (and in v1), when a progress notification arrived after the total budget elapsed, _onprogress's over-budget catch settled the request via responseHandler(error) directly — nothing was put on the wire. Post-PR, the catch calls timeoutInfo.onTimeout(error) → cancel(error) (packages/core-internal/src/shared/protocol.ts, the _onprogress catch), and cancel() POSTs notifications/cancelled on every !streamCloseCancels combination: legacy-era connections on any transport, and stdio/in-memory at any era — precisely the combinations the bullet declares unchanged.

  The diff documents its own contradiction

  The PR's changeset (.changeset/legacy-sse-reconnect-after-timeout.md) states verbatim that maxTotalTimeout settlements "previously settled without any wire cancel signal at either era" and "now route through the request's cancel path and emit the era's signal (the notifications/cancelled POST on legacy connections and modern single-channel transports…)". The new test.each era-matrix test in packages/core-internal/test/shared/protocol.test.ts pins it: cancelledSent has length 1 with requestId: 0 on 2025-11-25. So the changeset and the test say the wire signal changed, while the migration guide's re-baselining bullet says it didn't.

  Step-by-step proof of the consumer impact

  1. A v1 consumer runs a wire-level test or server-side monitor pinning "no notifications/cancelled is emitted when maxTotalTimeout trips" — e.g. a client test on a stdio connection asserting sent-message counts after a maxTotalTimeout rejection, or a server assertion that a budget-exceeded client goes silent.
  2. They issue a request with { maxTotalTimeout, resetTimeoutOnProgress: true, onprogress } (all public RequestOptions); a progress notification arrives after the budget elapses.
  3. On v1, nothing goes on the wire; the assertion holds. On v2 with this PR, cancel() POSTs (or writes, on stdio) notifications/cancelled — the assertion breaks.
  4. They consult this exact section — the guide's designated place for timeout-signal re-baselining — and it affirmatively tells them the cancelled-on-timeout signal is unchanged for their combination.

  Why nothing else covers it

  Two adjacent findings were already raised and fixed on this PR, but neither touches this sentence: the changeset was updated (now accurate) and the adjacent "Also unchanged: SSE reconnection exhaustion" bullet at lines 1510-1514 was flagged separately for the removed onerror wrapper — that is a different bullet making a different claim. The latest commit (3b486e8) did not touch lines 1504-1509; the sentence survives verbatim at HEAD.

  How to fix

  Scope the sentence to plain per-leg timeouts and caller aborts, and add one clause mirroring the changeset's wording, e.g.:

  The cancelled-on-timeout signal for per-leg timeouts and caller aborts is unchanged on legacy-era connections and on stdio/in-memory at any era; maxTotalTimeout settlements — which previously emitted no wire cancel at all — now send notifications/cancelled on those combinations (and cancel via the per-request stream close on 2026-era Streamable HTTP).

  Severity

  Nit: doc-only, nothing in shipped code misbehaves, and the trigger requires the maxTotalTimeout + resetTimeoutOnProgress combination — but per REVIEW.md's Documentation catch ("flag prose that now contradicts the implementation"), the guide's exact re-baselining bullet should not affirmatively promise a wire signal is unchanged when the same diff's changeset says it changed.

[claude]

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

• 🟡 packages/client/src/client/streamableHttp.ts:975-989 — The new _pendingReconnectCancels bookkeeping (adefd71) has two lifecycle gaps in close() that bite custom ReconnectionScheduler cancels (public API; the default clearTimeout path is unaffected): (1) the disarm loop at streamableHttp.ts:982-984 invokes each cancel unguarded, so one throwing cancel skips every remaining chain's cancel (the finally only clears/aborts/fires onclose), leaving their timers or platform tasks armed after a clean close() — the same shape exists in the requestSignal abort listener (cancelPending(); disarmBookkeeping(); unguarded, ~line 755); and (2) close() stores only the raw cancels, never the per-chain listenerCleanup, so every pending chain's settlement listener survives close() armed — and this PR's own .finally() requestAbort.abort() then deterministically fires it (close → onclose → Protocol._onclose rejects the request → abort), invoking the user's cancel a SECOND time against the JSDoc's once-on-close reading. Converging fix: store the per-chain disarm alongside the cancel (e.g. Set entries of { cancel, release: () => listenerCleanup.abort() }), have close() invoke both with per-entry try/finally, and use try { cancelPending(); } finally { disarmBookkeeping(); } in the listener.

  Extended reasoning...

  Two lifecycle gaps in the Set-based reconnect-disarm bookkeeping

  Both defects live in code this PR added in adefd71 (the _pendingReconnectCancels Set replacing the single _cancelReconnection slot), and both are invisible with the default setTimeout scheduler — they only manifest through the public, documented ReconnectionScheduler API (whose JSDoc example is platformBackgroundTask.cancel(id), i.e. exactly the kind of platform call that can throw or be non-idempotent).

  Facet 1: a throwing cancel skips disarming every remaining chain

  close() (packages/client/src/client/streamableHttp.ts:975-989) iterates the Set with an unguarded call:

  for (const cancel of this._pendingReconnectCancels) {
      cancel();
  }

  If one user-supplied cancel throws, the for loop aborts. The finally clears the Set, aborts the transport controller, and fires onclose — but it never invokes the skipped entries. Every later-registered chain's timer or platform task stays armed after a clean close(), which is precisely the leaked-armed-timer defect the Set was introduced to eliminate, and it breaks the ReconnectionScheduler JSDoc promise that a returned cancel "will be called on transport.close()". The same unguarded shape exists in the requestSignal abort listener registered by _scheduleReconnection (~line 755): cancelPending(); disarmBookkeeping(); — a throwing cancelPending skips disarmBookkeeping, leaving a stale Set entry (a later close() re-invokes the same throwing cancel and again skips its siblings) and an un-aborted listenerCleanup. This is REVIEW.md's Async/Lifecycle recurring catch verbatim (#1735/#1763: wrap user-supplied cancel fns in close paths in try/finally so a throw can't skip the remaining teardown), applied to code this PR itself added. The pinned test at streamableHttp.test.ts:3211 ("still aborts and fires onclose if the cancel function throws") covers only a single chain and pins propagation+abort+onclose — a per-entry-try fix that rethrows the first error after the loop is fully compatible with it.

  Facet 2: close() cannot release the settlement listeners, so every pending chain's cancel runs a second time — deterministically, with no throw

  _scheduleReconnection registers, per pending attempt: (a) the raw cancelPending in the Set, and (b) an abort listener on options.requestSignal with { once: true, signal: listenerCleanup.signal }. The only thing that releases that listener is disarmBookkeeping()'s listenerCleanup.abort(), reachable from exactly two places: the attempt firing (reconnect()) or the settlement listener itself. close() takes a third path the bookkeeping never anticipated — it invokes the raw cancels and merely clear()s the Set; the per-chain disarm closures are unreachable from it, so every pending chain's settlement listener survives close() fully armed.

  Step-by-step proof (no throw, no race):

  1. Legacy (2025-11-25) session, a request in flight through Protocol.request(), its per-request SSE reconnect chain in a backoff window. The Set holds the chain's cancel; the settlement listener is armed on the request's requestSignal (the protocol layer's requestAbort.signal).
  2. The app calls client.close() → transport.close(): the loop invokes cancelPending() — first invocation — then the finally runs Set.clear(), transport abort, this.onclose?.().
  3. onclose → Protocol._onclose() rejects the pending request's response handler with ConnectionClosed → the request promise settles → its .finally() runs requestAbort?.abort() (protocol.ts, this PR's every-settlement-path release).
  4. That abort dispatches on exactly the signal the armed listener sits on: cancelPending() runs a second time, then disarmBookkeeping() (both now no-ops on the cleared Set / aborted controller).

  Step 3 is guaranteed by this very PR (requestAbort is aborted on every settlement path), so the double-invocation happens on every clean close() that catches a per-request chain in its backoff window. The { once: true } on the listener prevents double-fire of the listener, not double-invocation of the cancel (the first call comes from close()'s loop); the fired guard only covers synchronous scheduler invocation at schedule time; and the close()-disarms-every-chain test cannot observe it because a double clearTimeout is harmless.

  Impact and why the fixes converge

  With the default scheduler both facets are benign (clearTimeout never throws and is idempotent), which is why nothing in the test suite sees either. With a custom ReconnectionScheduler: facet 1 leaves un-cancelled platform tasks/timers behind after close() (wire-silent — the fire-time guards prevent any GET — but a leaked background task on the platforms the JSDoc targets); facet 2 runs the user's cancel twice against the natural once-on-close reading of the contract, and a throw from that second call escapes an abort-event listener dispatched from inside Protocol's .finally() during a clean shutdown. Note that facet 1's fix (per-entry try/catch in close(); try { cancelPending(); } finally { disarmBookkeeping(); } in the listener) does not fix facet 2 — close() still can't reach listenerCleanup — so the right shape addresses both at once: store the per-chain disarm alongside the cancel, e.g. Set entries of { cancel: cancelPending, release: () => listenerCleanup.abort() } (or a single wrapper running cancel-then-disarmBookkeeping), and have close() invoke both per entry with per-entry try/finally. Then close() and the settlement path converge on the same disarm-once bookkeeping: every user cancel runs at most once, and one throwing cancel can't strand its siblings.

  Severity: nit rather than blocking — the trigger requires a custom scheduler whose cancel throws or is non-idempotent; the default configuration cannot hit either facet, and the fallout is a leaked-but-inert task or a duplicate cancel call, not a protocol failure. Still worth fixing here since it's a contract wrinkle in this PR's own new teardown machinery and the fix is mechanical.

• 🟡 packages/client/src/client/streamableHttp.ts:847-859 — On Node 20.0–20.2, the anySignal fallback (streamableHttp.ts:283–311) removes its onA/onB listener pair only inside cleanup(), which runs solely when one input signal fires — so every SSE reconnect leg that completes with neither signal aborting strands one closure pair on both the transport-lifetime signal and the requestSignal until the request settles, and this hunk newly exercises that path on the legacy era by threading requestSignal into every rebuilt leg (pre-PR, legacy sessions short-circuited past anySignal entirely). On the documented SSE polling pattern this accrues one pair per resume cycle, hitting MaxListenersExceededWarning after ~11 cycles; fix by building the composite once per request chain and threading it through StartSSEOptions, or having anySignal return a disposer invoked in a finally around the guarded fetch.

  Extended reasoning...

  What the bug is

  The anySignal() fallback for Node 20.0–20.2 (packages/client/src/client/streamableHttp.ts:283–311; the engines floor is >=20, and the fallback exists solely for those three releases where AbortSignal.any is unavailable) registers one abort listener on EACH input signal and removes the pair only inside cleanup() — which is invoked exclusively from onA/onB, i.e. when one of the two inputs FIRES. The function returns only controller.signal; there is no disposer. A guarded fetch that completes with neither signal aborting leaves both listeners (and the closures they pin) registered for as long as the input signals live.

  The code path that triggers it

  _startOrAuthSse builds a fresh composite per GET leg (anySignal(transportSignal, requestSignal) at ~line 563), and both _scheduleReconnection rebuild sites in _handleSseStream (the anchored hunks at :847–859 and :886) forward requestSignal into every rebuilt leg's StartSSEOptions. So on the fallback path, each resume leg of a per-request chain that ends gracefully (server closes without either signal aborting) adds one closure pair to BOTH the transport-lifetime AbortSignal and the request's AbortSignal. Unlike native AbortSignal.any (which holds dependent signals weakly, so a completed leg's composite is GC-collectable), the fallback's closures are strongly held by the input signals while registered.

  Why this is a regression introduced by this PR (legacy era)

  Before this PR, requestAbort was created only when streamCloseCancels was true (modern 2026-07-28 era × per-request-stream transport). On a legacy (2025-11-25) session, requestSignal was undefined, the signal expression short-circuited to (requestSignal ?? transportSignal), and anySignal never ran. This PR's core change in protocol.ts — requestAbort = this._transport.hasPerRequestStream === true ? new AbortController() : undefined — plus the reconnect-leg threading in this hunk routes every legacy-era POST and every reconnect GET leg through anySignal on Node 20.0–20.2. The legacy era is the entire current install base of Streamable HTTP servers, so the fallback's leak surface goes from "modern-era only" to "everything".

  Step-by-step proof

  1. A client on Node 20.0/20.1/20.2 connects to a legacy (2025-11-25) Streamable HTTP server and issues a long-running tools/call.
  2. The server uses the documented SSE polling pattern: priming event id, then graceful stream close with a retry hint — exactly the shape this PR's own e2e regression test mocks.
  3. Each polling cycle re-enters _startOrAuthSse via _scheduleReconnection with requestSignal forwarded (this hunk). anySignal builds a fresh composite, registering onA on the transport-lifetime signal and onB on the requestSignal.
  4. The leg completes gracefully — neither signal fired — so cleanup() never runs. The pair stays registered. One pair accrues per cycle.
  5. After 11 cycles Node prints MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal] — once for the transport signal, once for the requestSignal. An hour-long request polling every 10s pins ~360 closure pairs, multiplied across concurrent in-flight requests, all sharing the one transport-lifetime signal.
  6. Everything releases in a burst only at settlement: the request funnel's .finally() requestAbort.abort() fires every accumulated composite's onB, whose cleanup() also removes each sibling onA.

  Why existing safeguards miss it

  The fallback's own comment ("{once:true} alone leaks the sibling listener") addresses only the one-input-fires case — it says nothing about legs that complete with neither input firing. The new "successful completion releases the request-scoped signal" test and the changeset cover only the settlement-time release (a distinct, already-fixed issue: the .finally() abort). No test runs more than a couple of reconnect legs, and none inspects listener counts, so per-leg growth before settlement is unpinned.

  Impact and how to fix

  Impact is bounded: only three 2023-era Node patch releases (Node 20 is past EOL as of April 2026), growth is confined to a request's lifetime and fully released at settlement, and the visible symptom is warning noise plus intra-request memory growth — no incorrect behavior. That said, the SDK deliberately maintains this fallback (it exists solely for 20.0–20.2), so the leak is real and actionable on the supported floor. Fix options, either of which resolves it: (a) build the composite once per request chain and thread the composed signal through StartSSEOptions so every leg reuses it (also saves an allocation per leg on the native path), or (b) have anySignal return a disposer and invoke it in a finally around the guarded fetch in _startOrAuthSse (:563) and _send (:1102).

[claude]

Re the two "Additional findings" in review 4872890061 — both confirmed and fixed in e0ab79e.

1. _pendingReconnectCancels lifecycle gaps (close() disarm loop / settlement-listener release) — confirmed, both facets. close() invoked each cancel unguarded (one throwing custom ReconnectionScheduler cancel skipped every remaining chain), and it stored only the raw cancels, so pending chains' settlement listeners survived close() armed and the request funnel's .finally() requestAbort.abort() deterministically invoked the user's cancel a second time. Fixed with the converging shape suggested: Set entries are now { cancel, release } (release aborts the chain's listenerCleanup), close() invokes both per entry with per-entry try/finally and rethrows the first cancel error only after every chain is disarmed (the pinned throwing-cancel test still passes unchanged), and the settlement listener runs try { cancelPending(); } finally { disarmBookkeeping(); }. Regression tests added (failing-first): a throwing cancel no longer skips sibling chains on close(), and a chain's cancel runs at most once across close() + settlement. The ReconnectionScheduler JSDoc now states the at-most-once contract explicitly.

2. anySignal fallback listener accrual per reconnect leg (Node 20.0–20.2) — confirmed; the failing-first test measured 12 stranded abort listeners on the request signal after ~5 polling cycles. Fixed with option (a): the composite is built once per request chain and threaded through every rebuilt leg via an internal SseLegOptions extension of StartSSEOptions (fetchSignal) — _send's POST passes its composed signal into _handleSseStream, both _scheduleReconnection rebuild sites forward it, and _startOrAuthSse composes only when no chain signal exists yet (first leg of a resumption-token chain / standalone GET). Option (b) (disposer in a finally around the fetch) was rejected: _handleSseStream consumes the response body after _startOrAuthSse returns, so disposing at fetch scope would detach the abort path from an in-flight SSE read. Listener-count regression test added with AbortSignal.any stubbed absent; the native path also saves one composite allocation per leg. Chosen public surface: none — StartSSEOptions is unchanged.

Changeset updated with both hardening notes. Client tests 819/819, full e2e suite 44 files green, lint + typecheck + snippet sync clean.

---

Generated by Claude Code

[claude]

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

• 🟡 packages/client/src/client/streamableHttp.ts:349-359 — On Node 20.0–20.2 (where the anySignal fallback is used instead of native AbortSignal.any), every SSE reconnect/resume leg re-enters _startOrAuthSse and builds a fresh anySignal(transportSignal, requestSignal) whose listener pair is only released when a parent aborts — and since this PR now threads requestSignal on the legacy era too, every leg of every legacy-era request pins +1 abort listener/closure/controller on BOTH the transport-lifetime and request-scoped signals until the request settles. Against a polling-pattern server (the shape this PR's own e2e test mocks), a long-running request crosses Node's MaxListenersExceededWarning threshold at leg 11 and memory grows linearly in legs; all pairs are released at settlement, so this is bounded growth plus warning spam, not a permanent leak. Fix: give anySignal a dispose handle released when the leg settles, or combine the two signals once per chain and thread the combined signal through StartSSEOptions.

  Extended reasoning...

  What the bug is

  The anySignal Node 20.0–20.2 fallback (packages/client/src/client/streamableHttp.ts:283-311) registers one {once: true} abort listener on EACH parent signal, and its cleanup() runs only inside onA/onB — i.e. only when one of the parents actually aborts. Nothing releases the pair when the combined signal's fetch completes or its stream ends. _startOrAuthSse builds a FRESH combined signal on every invocation:

  const signal =
      requestSignal !== undefined && transportSignal !== undefined
          ? anySignal(transportSignal, requestSignal)
          : (requestSignal ?? transportSignal);

  Every SSE reconnect/resume leg re-enters _startOrAuthSse (via _scheduleReconnection → reconnect()), so on Node 20.0–20.2 each leg permanently (until request settlement) pins +1 listener + closure + fallback AbortController on both the transport-lifetime signal and the request-scoped signal.

  Why this is new exposure introduced by this diff

  Pre-PR, on a legacy (2025-11-25) session — the DEFAULT era — requestSignal was never threaded (requestAbort was created only when streamCloseCancels, i.e. modern era × per-request-stream), so every legacy-era leg took the bare (requestSignal ?? transportSignal) branch: zero anySignal calls, zero accumulation. Post-PR, requestAbort = this._transport.hasPerRequestStream === true ? new AbortController() : undefined threads requestSignal into every legacy-era request, so every leg of every request's reconnect chain now takes the combinator branch.

  Why existing safeguards don't cover it

  The PR explicitly treats this fallback's listener hygiene as in-scope: the changeset and the new "successful completion releases (aborts) the request-scoped signal" test exist precisely to avoid pinning "one abort listener per request on the transport-lifetime signal via the Node 20.0–20.2 anySignal fallback". But that fix addresses the per-REQUEST pair created by _send's POST — the per-LEG pairs created by _startOrAuthSse for the same request were not considered. One pair per request was deemed worth releasing, yet a single request can now create hundreds of pairs before settlement.

  Step-by-step proof

  1. Client on Node 20.0–20.2 (AbortSignal.any absent; the package's engines floor is >=20, and the fallback exists precisely to serve these versions) connects to a legacy server using the documented polling pattern: priming event id, graceful close, retry: hint — exactly the server shape this PR's own e2e regression test mocks.
  2. tools/call with a generous timeout (say 10 minutes). The server closes the primed stream every retry: 1000 ms.
  3. Each gap → _scheduleReconnection → reconnect() → _startOrAuthSse → a new anySignal(transportSignal, requestSignal) pair. Each leg's GET succeeds and its stream closes gracefully — neither parent aborts, so cleanup() never runs. ~60 pairs/minute.
  4. At leg 11, Node prints MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 abort listeners added to [AbortSignal] for the transport-lifetime signal — spurious leak warnings during perfectly normal operation. Memory grows linearly in legs (closure + fallback controller per leg) until the request settles; with several concurrent long-running requests the transport-lifetime signal accumulates pairs from all of them at once.
  5. When the request settles, the funnel's .finally() runs requestAbort?.abort() → every leg's onB fires → cleanup() removes both listeners of every pair. So the pairs ARE fully released at settlement — the defect is unbounded growth DURING the request plus the warning spam, not a permanent leak.

  Impact

  Confined to Node 20.0–20.2 (three 2023 patch releases; native AbortSignal.any from 20.3 holds dependent signals weakly, so they're collectable once the leg's fetch drops them). Symptoms: MaxListenersExceededWarning spam within seconds against a polling server, and transient linear memory growth bounded by request lifetime. Nothing functionally breaks, which is why this is a nit rather than a blocker.

  How to fix

  Either (a) make anySignal return a dispose handle (or accept a cleanup signal, the same idiom _scheduleReconnection's new listenerCleanup controller already uses) and have _startOrAuthSse release the pair when the leg's fetch settles / its stream ends, or (b) combine the two signals ONCE per request chain and thread the combined signal through StartSSEOptions so resume legs reuse it instead of recombining per leg.

• 🟡 packages/client/src/client/streamableHttp.ts:981-995 — Two residual gaps in the close() teardown of _pendingReconnectCancels (added in adefd71): (1) the disarm loop is bare, so one throwing cancel (possible only for a custom ReconnectionScheduler) skips all remaining entries and the finally clears the set without invoking them — leaving sibling chains' scheduler tasks armed, and the standalone GET chain has no requestSignal rescue; (2) close() invokes each entry's cancel but never releases its listenerCleanup, so the surviving requestSignal abort listener deterministically re-invokes an already-invoked cancel when the settled request's funnel aborts requestAbort. Fix both at the entry shape: store per-entry {cancel, release} records and have close() run each with per-entry try isolation.

  Extended reasoning...

  What the bug is

  The _pendingReconnectCancels Set (introduced on this PR in adefd71 to fix the single-slot _cancelReconnection leak) has two residual gaps in how close() tears its entries down, both in packages/client/src/client/streamableHttp.ts.

  Facet 1 — a throwing cancel skips sibling chains. close() (lines ~981-995) runs for (const cancel of this._pendingReconnectCancels) cancel(); bare inside one try. If an entry's cancel throws, the loop aborts and the finally clears the set without invoking the remaining cancels. The inline comment acknowledges only that "a throwing cancel still propagates to the caller" — not that siblings are skipped. This matches REVIEW.md's Async/Lifecycle recurring catch (#1735/#1763), which names "cancel fns" in close() paths verbatim: teardown must complete even when a chained callback throws.

  Facet 2 — the settlement listener survives close() and re-invokes the cancel. Each entry registers a { once: true } abort listener on options.requestSignal (lines ~756-765) that calls cancelPending(), released only via that entry's private listenerCleanup controller. The Set stores only the bare cancelPending, so close() can invoke the cancel but can never run disarmBookkeeping/abort listenerCleanup — the listener stays armed after close().

  Step-by-step proof (facet 2, deterministic)

  1. A legacy-session request is in flight with a scheduled reconnect pending; its entry's abort listener is armed on requestSignal.
  2. transport.close() runs: the loop invokes the entry's cancelPending — first invocation of the user-supplied cancel.
  3. The finally fires onclose → Protocol._onclose settles the pending request with ConnectionClosed → the request funnel's .finally() in protocol.ts (added by this PR on every settlement path) runs requestAbort?.abort().
  4. The surviving abort listener fires and calls cancelPending() a second time. For the built-in path a double clearTimeout is a no-op; for a custom ReconnectionScheduler (public, documented API) the user cancel is invoked twice — the JSDoc ("If returned, it will be called on transport.close()") reads as at-most-once — and a platform cancel API that throws on an already-cancelled task id throws inside an abort-event listener, where nothing catches it.

  Facet 1 proof

  1. Two chains are pending under a custom scheduler at close(): the standalone GET chain and a per-request chain (the exact multi-chain scenario the Set was added for).
  2. The first entry's cancel throws (platform quota / invalid-id — the scheduler-throws class was already accepted as reachable on this PR: the scheduleError fix in 3f66066).
  3. The loop exits; finally clears the set. The other chain's scheduler task is never cancelled. Per-request chains can still be rescued post-close by the surviving requestSignal listener (facet 2's mechanism, ironically), but the standalone GET chain has no requestSignal — its task stays armed with no rescue, breaking the ReconnectionScheduler JSDoc contract and leaving the un-cancelled platform task to wake the app for a no-op (the fire-time abort guard keeps it wire-silent).

  Addressing the refutation

  One verifier argued the facets are speculative. On facet 1: entries are homogeneous per transport (all clearTimeout wrappers or all custom-scheduler cancels), so the harm is never a pinned setTimeout coexisting with a throw — but that narrows rather than refutes: with a custom scheduler, the skipped siblings are un-cancelled platform tasks, exactly the leak class adefd71 was written to eliminate, and the finally's abort()/onclose does not cancel them. On facet 2: the refutation concedes the double-invocation sequence is reachable and disputes only the consequence; the JSDoc's "will be called on transport.close()" phrasing plus the deterministic second invocation from a code path the same PR added (requestAbort?.abort() in the funnel's .finally()) makes this a genuine gap in teardown machinery this PR authored, not a hypothetical. Both points are why this is a nit, not a blocker: nothing breaks under the default setTimeout scheduler, and concrete harm requires a custom scheduler whose cancel throws or is non-idempotent.

  How to fix

  Change the Set's entry shape from bare () => void to per-entry records { cancel, release } (where release runs disarmBookkeeping, aborting listenerCleanup), and have close() iterate with per-entry try isolation — invoke both members for every entry, collect the first error, and rethrow it after the loop. That disarms every chain even when one cancel throws, and guarantees no settlement listener can re-invoke a cancel after close().

[claude]

Re the two "Additional findings" in review 4873086583: that review ran against the stale head 3f66066 — both findings restate the previous round's body findings and were already fixed in e0ab79e (CI green, 18/18 checks), including the sub-facets this round adds:

1. Per-leg anySignal listener accrual (Node 20.0–20.2) — fixed via the suggested option (b)/(a)-hybrid: the transport+request composite is built once per request chain and threaded through every rebuilt reconnect leg (internal fetchSignal extension of StartSSEOptions; _send's POST passes its composed signal into _handleSseStream, both _scheduleReconnection rebuild sites forward it, _startOrAuthSse composes only when the chain has no signal yet). A dispose-at-leg-settlement handle was deliberately NOT used: _handleSseStream consumes the response body after _startOrAuthSse returns, so releasing the pair at fetch scope would detach the abort path from an in-flight SSE read. Listener-count regression test (with AbortSignal.any stubbed absent) went from 12 accrued listeners after ~5 polling cycles to a flat count.

2. _pendingReconnectCancels close() gaps — fixed exactly at the entry shape suggested: Set entries are { cancel, release } (release aborts the chain's listenerCleanup), close() runs both per entry with per-entry try/finally and rethrows the first error only after every chain is disarmed — which also covers this round's standalone-GET-chain point (the loop itself now always completes, so chains with no requestSignal rescue are disarmed regardless of a sibling's throw) — and the settlement listener runs cancel-then-disarm under try/finally. Regression tests: throwing cancel doesn't skip siblings; a chain's cancel runs at most once across close() + settlement.

The two anchored comments from this round (maxTotalTimeout: 0 falsy gate; sse.ts 401-recovery continuation after close()) are fixed in 2728d04 — replied inline.

---

Generated by Claude Code

[claude]

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

• 🟡 packages/client/src/client/streamableHttp.ts:893-910 — In _handleSseStream's processStream, the graceful-close settlement tail (the needsReconnect check, the unguarded _scheduleReconnection call at streamableHttp.ts:894, and the else-branch onRequestStreamEnd?.() at :915) runs inside the same try as the stream read loop, so a synchronous throw from the tail lands in the catch at :917, gets mislabeled as SSE stream disconnected, and re-drives the tail. A custom ReconnectionScheduler that throws on the FIRST schedule of a gap is invoked twice and produces two onerror reports (violating the exactly-once contract this PR documents), and a throwing caller-supplied onRequestStreamEnd is re-invoked from inside the catch, whose second throw escapes into the fire-and-forget processStream() promise as an unhandledRejection. Fix by moving the settlement tail out of the read-loop try (or wrapping its _scheduleReconnection/onRequestStreamEnd calls like the error path's guarded twin at :933-947).

  Extended reasoning...

  What the bug is

  In _handleSseStream's processStream (packages/client/src/client/streamableHttp.ts), the post-loop graceful-close settlement tail — the needsReconnect computation, the _scheduleReconnection(...) call at line 894, and the else-branch onRequestStreamEnd?.() at line 915 — executes inside the same try block as the stream read loop. Any synchronous throw from that tail falls into the generic catch at line 917, which (a) reports it as SSE stream disconnected: ... — a mislabel, since the stream actually ended gracefully — and (b) re-computes needsReconnect from unchanged inputs and re-drives the whole settlement tail a second time.

  Two concrete triggers, both through public API surface this PR touches:

  1. A custom ReconnectionScheduler that throws synchronously on the FIRST schedule of a gap. _scheduleReconnection invokes this._reconnectionScheduler(reconnect, delay, attemptCount) bare — no try/catch at that call site — so a first-schedule throw propagates straight out to the tail. This is the exact "platform denied background task" scenario the PR's own test models, but that test only covers the reschedule site (schedulerCalls === 1 succeeds; only the second call throws), leaving the first-schedule path untested and unhandled.

  2. A caller-supplied onRequestStreamEnd that throws (public TransportSendOptions callback, for direct transport.send() users).

  Step-by-step proof (trigger 1)

  1. A client configures reconnectionScheduler whose schedule call can throw (e.g. the platform denies the background task).
  2. A primed POST SSE stream closes gracefully without the response → needsReconnect is true → line 894 calls _scheduleReconnection(..., 0) → the scheduler throws.
  3. The throw propagates out of _scheduleReconnection (no guard, unlike the error path's twin at lines 933-947) and is caught at line 917 → onerror('SSE stream disconnected: Error: platform denied background task') — mislabeled as a network disconnect.
  4. The catch re-computes needsReconnect (still true) and calls _scheduleReconnection a SECOND time at line 934 with attemptCount 0 — the scheduler is invoked twice for one gap. If it throws again, the inner catch fires a second onerror (Failed to reconnect: ...).

  Net: one scheduler failure → two onerror reports, the first mislabeled — directly violating the exactly-once contract this PR newly documents in its changeset ("each failed SSE reconnect leg now reports exactly once with the underlying error") and in docs/migration/upgrade-to-v2.md.

  Step-by-step proof (trigger 2)

  1. A POST stream with no priming event ends gracefully → needsReconnect is false → line 915 invokes onRequestStreamEnd?.() inside the try; it throws.
  2. Caught at line 917 → mislabeled SSE stream disconnected onerror → needsReconnect is false in the catch too → line 955 invokes the same throwing callback a second time.
  3. That second throw is inside the catch block with nothing above it — it escapes, rejecting the processStream() promise, which is invoked fire-and-forget at line 959 with no .catch → an unhandledRejection, which terminates a Node process by default.

  Why existing code doesn't prevent it

  The error path's schedule call IS wrapped (lines 933-947 report the raw scheduleError once and fire onRequestStreamEnd), but the graceful path's twin at 893-915 is not — this is the surviving unswept instance of exactly the pattern the PR's scheduler-throw sweep fixed everywhere else (close() per-entry try/finally, the reschedule-throw settle, the sync-fire guard, the settlement listener). It also matches two of this repo's REVIEW.md recurring catches: user-supplied callbacks in teardown paths must be guarded so a throw can't skip/corrupt remaining teardown, and pattern replacements must not leave sibling instances behind.

  How to fix

  Move the settlement tail out of the read-loop try — only the reader loop belongs there — or mirror the error path: wrap the graceful-path _scheduleReconnection in its own try/catch (report the raw scheduleError once + fire onRequestStreamEnd), and guard the tail's onRequestStreamEnd invocations so a throwing callback can neither re-enter the tail nor escape into the un-awaited processStream() promise.

  Severity

  Nit rather than blocking: both triggers require a user-supplied ReconnectionScheduler or onRequestStreamEnd callback that throws synchronously; the try-encapsulation of the tail predates this PR, and default configurations (setTimeout scheduler, the SDK's own non-throwing callbacks) are unaffected. It's worth fixing here because this PR edits both call sites in this exact hunk, establishes the exactly-once onerror contract this path violates, and already fixed the identical pattern at every sibling site.

[claude]

Re the "Additional findings" item in review 4873290202 (graceful-close settlement tail inside the read-loop try) — confirmed and fixed in 9636485.

Both triggers verified: a first-schedule scheduler throw landed in the generic catch as a mislabeled SSE stream disconnected, then the catch re-drove the tail for a second onerror (violating the exactly-once contract this PR documents); a throwing caller onRequestStreamEnd was re-invoked from the catch, whose second throw escaped the fire-and-forget processStream() as an unhandledRejection. Fix takes the "move the tail out of the try" shape: the settlement tail now runs after the try/catch (the catch returns), the graceful-path _scheduleReconnection is guarded like its error-path twin (raw scheduler error reported once, then stream-end), and every stream-end invocation inside processStream goes through a guarded fireStreamEnd that routes callback throws to onerror so nothing can escape the un-awaited promise. Failing-first regression tests for both triggers: first-schedule throw → scheduler invoked once, one correctly-labeled onerror, stream-end fired once; throwing onRequestStreamEnd → invoked exactly once, its error reported via onerror, no unhandledRejection.

---

Generated by Claude Code

[claude]

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

• 🔴 packages/client/src/client/streamableHttp.ts:1125-1144 — Three sites in this same reconnect machinery still invoke potentially-throwing user code bare, while this PR deliberately contained every sibling (the scheduler cancel at close() and at settlement in 0e65433, the throwing eventSource.close() in sse.ts): (1) onRequestStreamEnd is called inside floating promise chains with no rejection handler — this resumptionToken short-circuit's .catch (~line 1142), the reconnect closure's scheduleError branch (~771), and the fire-and-forget processStream() (~969) — so a throwing callback becomes an unhandledRejection that terminates the Node process by default (and the graceful-end path also double-invokes the callback); (2) onresumptiontoken is invoked bare in _handleSseStream's read loop (~871), so a throw (e.g. QuotaExceededError from the storage write it's documented for) surfaces a misattributed 'SSE stream disconnected' onerror, loses the event's payload, and re-enters reconnection at attempt 0 where maxRetries never binds; (3) the graceful-close _scheduleReconnection call (~904) is uncontained while its error-path sibling ~40 lines below is wrapped, so a synchronously-throwing scheduler cascades into a misattributed onerror plus a second attempt-0 scheduler invocation for the same gap. Wrapping each site in try/catch routed through this.onerror (mirroring the settlement-listener fix in 0e65433) closes all three; sites (2) and (3) predate the PR but are the surviving instances of the containment sweep it performs, while two of the three onRequestStreamEnd sites are new in this PR.

  Extended reasoning...

  What the bug is

  This PR performs a deliberate sweep containing throwing user callbacks in the SSE reconnect machinery — the ReconnectionScheduler cancel is wrapped at both close() (per-entry try/finally) and the settlement listener (catch → onerror, added in 0e65433 with the explicit 'process exit by default' rationale), and sse.ts's close() got a try/finally around the throwing eventSource.close(). Three sites in packages/client/src/client/streamableHttp.ts survive that sweep uncontained:

  1. onRequestStreamEnd inside floating promise chains. The resumptionToken short-circuit in send() (lines ~1119–1145, new in this PR) runs this._startOrAuthSse({...}).catch(() => { ... options?.onRequestStreamEnd?.(); }) — the chain's result is discarded and send() returns immediately. The reconnect closure's scheduleError branch (~771, new in this PR) invokes the same callback inside the floating this._startOrAuthSse(options).catch(...) at ~755. And processStream() is fired-and-forgotten at ~969 with no .catch: a throw from the graceful-end invocation at ~925 is caught at ~927, mis-reported as SSE stream disconnected: ... through onerror (violating the one-report-per-failure contract this PR pins in upgrade-to-v2.md), and then — because needsReconnect recomputes to the same false — the same callback is invoked a second time at ~965, whose throw escapes the catch and rejects the floating promise.

  2. onresumptiontoken bare in the read loop (~871): if (event.id) { lastEventId = event.id; hasPrimingEvent = true; onresumptiontoken?.(event.id); } — the adjacent message-dispatch block (~880+) has its own per-event try/catch, but the id-handling block does not.

  3. The graceful-close _scheduleReconnection call (~904) sits bare inside processStream's try, while the structurally identical error-path call (~943) is wrapped in its own try/catch (Failed to reconnect: ... + onRequestStreamEnd). _scheduleReconnection invokes the user scheduler synchronously (~778), and the throw happens before the if (!fired) bookkeeping at ~786 — so a scheduler that arms a platform task and then throws leaves it with no Set entry and no settlement listener (close() cannot cancel it).

  Step-by-step proof (site 1, single throw)

  1. A caller drives transport.send(msg, { resumptionToken: 'evt-42', onRequestStreamEnd }) directly — the exact pattern the new RequestOptions JSDoc recommends ('To observe the per-request stream's lifecycle directly, call transport.send() yourself') and the listen driver uses.
  2. The resume GET fails to open genuinely (fetch TypeError, or a non-405 HTTP status). _startOrAuthSse's own catch reports once via onerror and rethrows.
  3. The floating .catch at ~1131 runs: the intentional-abort guard passes (nothing aborted), so options?.onRequestStreamEnd?.() is invoked at ~1142. The callback throws (same plausibility class as the 'platform denied background task' throwing-cancel this PR already fixed).
  4. The .catch()-returned promise rejects with nothing attached — send() already returned at ~1145. Node (default --unhandled-rejections=throw since v15) raises unhandledRejection and terminates the process. No SDK or application catch can observe it. A verifier empirically confirmed this shape on Node 22 (exit 1).

  For site 2: a QuotaExceededError from the localStorage/IndexedDB write the callback is documented for ('persist the latest token for potential reconnection') exits the read loop into processStream's catch → misattributed SSE stream disconnected onerror. Because lastEventId/hasPrimingEvent are assigned before the call (~868–870) and parse+onmessage delivery sits after it (~875+), a throwing response-bearing event never sets receivedResponse and never fires onmessage — and the reschedule uses resumptionToken = that event's own id, so per Last-Event-ID semantics the server never replays it: the response is permanently lost and the request starves to its timeout. Worse, the catch reschedules at hard-coded attemptCount 0, every resumed leg opens successfully, and the hook throws again — maxRetries never binds (the same 'retry counter never binds' shape as #2615 itself), each cycle abandoning one locked, undrained reader. For the standalone GET chain (no requestSignal), the loop is bounded only by transport.close().

  For site 3: read loop finishes (done), needsReconnect is true, ~904 calls _scheduleReconnection(..., 0); the custom scheduler throws synchronously. The throw propagates into processStream's catch (~927) → misattributed SSE stream disconnected: Error: platform denied background task onerror → the catch recomputes needsReconnect (still true) and invokes the scheduler a second time for the same gap, again at attempt 0 (~943). One scheduler failure = two onerror reports + two scheduler invocations; a partial-effect scheduler (arms, then throws) additionally leaves an armed task that neither close() nor settlement can disarm, risking two concurrent attempt-0 legs (duplicate GET + Last-Event-ID resumes).

  Why existing code doesn't prevent it

  None of these invocation sites has a wrapper, and every enclosing promise is deliberately fire-and-forget with no terminal rejection handler: the .catch chains at ~755 and ~1119 discard their results, and processStream() at ~969 has no .catch. The containment this PR added sits around scheduling (cancel at close(), cancel at settlement, the retry closure's catch(scheduleError)) — not around these callback invocations. The per-event try/catch in the read loop covers only the message-dispatch block, not the id/onresumptiontoken block. The SDK's own consumer (the listen driver's settle()) happens not to throw, which is why tests never hit any of this. The exhaustion path also falsifies the PR's own inline comment 'No double-fire is possible': a throw from onRequestStreamEnd at ~726 propagates into the scheduleError catch and re-invokes the callback at ~771.

  Impact and fix

  Impact: an uncatchable process termination on reachable terminal-stream paths (site 1 — the identical failure class this PR fixed as blocking in 0e65433), double invocation breaking the at-most-once settle semantics the listen driver's settle-once machine assumes, a permanently lost JSON-RPC response plus an unbounded attempt-0 reconnect loop (site 2), and misattributed/duplicated onerror with a duplicate scheduler invocation (site 3).

  Fix: wrap each site in the same small guard already applied to the settlement listener — try { cb() } catch (e) { this.onerror?.(e instanceof Error ? e : new Error(String(e))) } — at the onRequestStreamEnd invocation sites (or equivalently give the floating chains at ~969/~755/~1119 a terminal .catch routing to onerror), around onresumptiontoken?.(event.id) (mirroring the adjacent dispatch containment, so the event's data is still delivered and no bogus reconnect starts), and around the graceful-close _scheduleReconnection call (mirroring the error path's existing try/catch at ~943). This matches the repo's Async/Lifecycle recurring catch ('wrap user-supplied or chained callbacks ... so a throw can't skip the remaining teardown') and Completeness catch ('grep the package for surviving instances of the old form') — these are the last uncontained user-callback sites of the sweep this PR performs, and two of the onRequestStreamEnd sites are introduced by this PR itself. (Sites 2 and 3's invocation lines predate the PR; they're included because the PR edits these exact hunks and establishes the containment contract they violate.)

[claude]

Re the red finding in review 4873635714 (three uncontained user-callback sites) — the review ran against the stale head 0e65433; per-site outcomes against the current code:

(1) onRequestStreamEnd in floating promise chains — confirmed for the reconnect closure's scheduleError branch and the resumptionToken short-circuit's catch (both still bare at 9636485; the fire-and-forget processStream() site was already contained by 9636485's guarded fireStreamEnd, which also removed the graceful-path double-invoke). Fixed in 6bf58a3: every onRequestStreamEnd invocation in the transport now routes through a guarded _fireRequestStreamEnd helper (throws report via onerror) — the two flagged sites plus the maxRetries-exhaustion branch, the 405 terminal outcome, and the null-body terminal outcome, so no bare invocation survives anywhere in the file. Failing-first tests for both flagged sites reproduced the unhandledRejection pre-fix and pin containment post-fix.

(2) onresumptiontoken bare in the read loop — confirmed and fixed in 6bf58a3: guarded in place, so a throwing persistence hook (the QuotaExceededError shape) reports once via onerror, correctly attributed, while the event still dispatches — the regression test pins that a response-bearing event's payload is delivered, no SSE stream disconnected misattribution occurs, and no reconnect chain starts (the response marks the request complete).

(3) graceful-close _scheduleReconnection — already fixed in 9636485 (the settlement tail moved out of the read-loop try; the graceful-path schedule is wrapped like its error-path twin, raw scheduler error reported once + stream-end). On the partial-effect sub-facet (a scheduler that arms a platform task and then throws before returning): there is nothing for the SDK to release at that point — the cancel handle IS the scheduler's return value, and a throw means no handle ever existed. The Set entry would hold a no-op. The fire-time guards make such an orphaned task wire-silent (per-request chains: requestSignal is aborted at settlement, which the schedule-throw path triggers via the stream-end settle; any chain: the transport-abort check after close()), and the at-most-once cancel contract is unaffected since no cancel was returned. Happy to revisit if you see a stronger containment shape.

---

Generated by Claude Code

[claude]

No bugs found in this round against head 87ebece — every finding from the earlier review rounds is confirmed fixed — but this PR needs a human maintainer pass: it rewrites request-settlement teardown in the shared Protocol base and the client transport reconnect machinery, changes observable behavior (maxTotalTimeout settlements now emit the era's wire cancel signal; the onerror contract is re-baselined), and the description itself asks for maintainer guidance on where the ownership boundary for the fix should sit.

What was reviewed this round:

• The (era × transport) cancellation matrix and the settled/idempotence path in protocol.ts — the maxTotalTimeout reroute disarms the per-leg timer before settling, pinned by the matrix tests.
• Swept streamableHttp.ts/sse.ts for surviving unguarded onerror/onRequestStreamEnd/onresumptiontoken/scheduler-cancel sites after the containment sweep — none remain.
• Checked the changeset and migration-doc claims against the implementation, including the era-scoped cancel-signal prose.

Extended reasoning...

Overview

This PR fixes #2615 (legacy-era Streamable HTTP SSE reconnect chains surviving their originating request's settlement) and, across ~10 review-driven commits, grew into a broader lifecycle hardening pass: request-scoped AbortController threading for every per-request-stream transport at both protocol eras, a _pendingReconnectCancels set replacing the single reconnect-cancel slot, guarded invocation of every user-supplied callback (onRequestStreamEnd, onresumptiontoken, scheduler cancels), the maxTotalTimeout settlement rerouted through the request cancel path, maxTotalTimeout: 0 and falsy-id (requestId: 0, relatedRequestId: 0) fixes, SSE-transport 401-recovery close guards, probe-exchange requestSignal threading, and matching changeset/migration/docs updates. It touches the shared Protocol base (so server-initiated requests are affected too), both client HTTP transports, and the Transport interface docs.

Security risks

No direct security surface: no auth logic, crypto, or permission checks are changed. The sse.ts edits touch the 401-recovery path but only add closed-state guards around existing onUnauthorized continuations (preventing post-close EventSource resurrection and spurious onerror); no token handling changes. Main risk class is availability/correctness — lifecycle races, double-fires, and unhandled rejections — which is exactly what the added test matrix pins.

Level of scrutiny

Highest tier. This is production-critical shared protocol code: every SDK consumer's request lifecycle flows through _requestWithSchemaViaCodec and the Streamable HTTP transport. There are also deliberate observable-behavior changes documented as re-baselining guidance (the Failed to reconnect SSE stream: wrapper removal, maxTotalTimeout now emitting a wire cancel signal, no AbortError on deliberate teardown). The issue reporter explicitly requested maintainer guidance on the design boundary (request-lifecycle abort signal vs. per-request reconnect disposal vs. a transport teardown hook), so a design sign-off is required regardless of implementation correctness — this alone rules out shadow approval.

Other factors

Eight prior bug-hunt rounds each produced findings, and each was fixed with failing-first regression tests; all inline threads are resolved and this final round against 87ebece found nothing new. Test coverage is unusually strong — the (era × transport) cancel matrix, close/settlement race tests, Node 20.0-20.2 anySignal listener-leak test, Gecko-timer-argument test, and an end-to-end #2615 reproduction. The stated test runs (core-internal 1435, client 799, server 468 passing) plus updated e2e assertions support the diff. The remaining questions are design-level (SDK minimalism, whether the review-sweep scope should have been decomposed per REVIEW.md's decompose-by-default principle) — calls for a human maintainer, not further automated review.