fix: send SSE keep-alive comment frames from WebStandardStreamableHTTPServerTransport#2541
fix: send SSE keep-alive comment frames from WebStandardStreamableHTTPServerTransport#2541mattzcarey wants to merge 2 commits into
Conversation
…PServerTransport Idle SSE streams (the standalone GET stream in particular, but also POST response streams during long-running tool calls) are killed by intermediaries and server idle timeouts, which clients observe as "SSE stream disconnected: TypeError: terminated" followed by a reconnect loop. The transport now writes an SSE comment frame (`: keepalive`) to every open SSE stream every keepAliveMs milliseconds (default 15000, per the WHATWG SSE spec recommendation; set 0 to disable). Comment frames are dropped by SSE parsers and never surface as protocol messages. The timer is unref'd so it never holds the process open, and is cleared on stream cleanup/cancel and transport close. Naming matches the existing keepAliveMs on createMcpHandler's subscriptions/listen streams. v2 port of #2538 (v1.x); refs #1211
🦋 Changeset detectedLatest commit: 9505f93 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the candidate concern that the keepAliveMs: 0 disable test is vacuous (Promise.race against an already-resolved promise) — it is not: race subscribes to reader.read() first, so when a frame is buffered its already-resolved promise's reaction is queued ahead of Promise.resolve('pending') and the test would correctly fail if frames were written.
Extended reasoning...
Bugs were found and posted as inline comments (keep-alive timer armed before the fallible writePrimingEvent await, and the close-during-replayEventsAfter race), so human review is already signalled. This note only records the one additional candidate examined and ruled out this run — the supposed vacuity of the keepAliveMs: 0 test's Promise.race — which was verified to be a sound test via microtask reaction ordering. Nothing here should be read as a guarantee of correctness of the rest of the diff.
- startKeepAlive is a no-op after transport close: a deferred arm (e.g. resuming after an event-store replay await that straddled close()) would otherwise create a timer that close()'s sweep can never clear. - The POST SSE path arms keep-alive after the fallible awaits (priming event write, message dispatch) instead of before them, so an error path that discards the Response cannot leak a permanently-firing timer against a stream nothing can cancel. - createMcpHandler's keepAliveMs now also reaches the legacy stateless fallback's per-request transport, instead of governing only subscriptions/listen streams while the legacy leg silently used the transport default. - Documents the keep-alive behavior and the 'SSE stream disconnected: TypeError: terminated' symptom in docs/troubleshooting.md.
|
All four review findings addressed in 6d20a1f:
|
| ## `SSE stream disconnected: TypeError: terminated` | ||
|
|
||
| An idle SSE stream was killed by an intermediary or an idle-connection timeout — Node's `server.requestTimeout` defaults to 300 seconds, and reverse proxies and cloud load balancers have similar watchdogs. The client observes the dropped socket as this error (typically every ~5 minutes) and reconnects in a loop. | ||
|
|
||
| `WebStandardStreamableHTTPServerTransport` prevents this by writing an SSE comment frame (`: keepalive`) to every open SSE stream every 15 seconds by default. Comment frames are dropped by SSE parsers before event dispatch, so they never surface as protocol messages. Tune or disable the interval with the transport's `keepAliveMs` option (`0` disables); `createMcpHandler`'s `keepAliveMs` option covers both its `subscriptions/listen` streams and the legacy fallback's per-request transport. | ||
|
|
||
| If you still see this error, either keep-alive is disabled (`keepAliveMs: 0`) or an intermediary between client and server buffers or strips SSE data — check for proxies that buffer streaming responses (e.g. nginx without `proxy_buffering off`). |
There was a problem hiding this comment.
🔴 createMcpHandler's modern (2026-07-28) leg serves request exchanges via PerRequestHTTPServerTransport, which has no keep-alive at all — writeCommentFrame() (perRequestTransport.ts:326) has zero production callers — so a long-running modern tools/call with responseMode 'sse' (or 'auto' upgraded by a mid-call notification) still idles past Node's 300s requestTimeout and dies with 'SSE stream disconnected: TypeError: terminated'. The new troubleshooting entry then affirmatively rules out this cause ('either keep-alive is disabled (keepAliveMs: 0) or an intermediary... buffers or strips SSE data'), sending modern-leg users to chase proxy buffering that isn't the problem. Either drive writeCommentFrame() from a keepAliveMs interval in PerRequestHTTPServerTransport, or scope the troubleshooting prose to the streams that actually have keep-alive.
Extended reasoning...
The remaining gap. This PR (with follow-up 6d20a1f) adds keep-alive to WebStandardStreamableHTTPServerTransport's SSE streams and threads keepAliveMs through createMcpHandler's subscriptions/listen router and legacy stateless fallback. But createMcpHandler's PRIMARY modern (2026-07-28) leg serves every request exchange through invoke() → PerRequestHTTPServerTransport (invoke.ts:59, constructed with only {classification, responseMode}), and that transport has no keep-alive whatsoever. It even ships a purpose-built heartbeat method — writeCommentFrame() at packages/server/src/server/perRequestTransport.ts:326, whose JSDoc calls it "a keep-alive heartbeat" — but a grep across packages/server/src finds zero production callers (only two calls in test/server/perRequestStreaming.test.ts). No setInterval/keep-alive wiring exists in invoke.ts or perRequestTransport.ts.\n\nThe idle scenario is real on this leg. With responseMode: 'sse', the transport eagerly opens the SSE stream immediately after dispatch (perRequestTransport.ts ~316: "Forced-SSE exchanges open their stream as soon as the request has passed the pre-dispatch gates"), so a long-running tools/call sits on an open, frameless SSE stream until the result arrives. With the default 'auto', a single mid-call notification (e.g. one progress update early in a long tool call) upgrades the exchange to SSE, after which the stream idles the same way. Either way, this is exactly the failure mode the PR is titled to fix — an idle SSE stream reaped by Node's 300s server.requestTimeout or a proxy/LB watchdog, surfacing client-side as SSE stream disconnected: TypeError: terminated. This matches the repo's Completeness recurring catch: a sibling code path is left with the very bug the PR claims to fix.\n\nThe doc misstatement introduced by this diff. The new docs/troubleshooting.md entry closes with: "If you still see this error, either keep-alive is disabled (keepAliveMs: 0) or an intermediary between client and server buffers or strips SSE data." For the modern createMcpHandler leg, neither is true: keep-alive simply does not exist there, and the handler-level keepAliveMs option — which the same paragraph correctly scopes to listen streams and the legacy fallback — does not govern per-request exchange streams. The sentence affirmatively rules out the actual remaining cause, matching the Documentation recurring catch (prose promising behavior the diff doesn't back).\n\nStep-by-step proof.\n1. Deploy createMcpHandler(factory, { responseMode: 'sse' }) behind Node's default server.requestTimeout (300s).\n2. A modern (2026-07-28) client calls a tool that takes >5 minutes and emits no mid-call messages.\n3. serveModern → invoke() constructs PerRequestHTTPServerTransport (no keep-alive option exists on it); forced-SSE settles the 200 SSE Response immediately after dispatch, before the handler result.\n4. The stream carries zero bytes for 300s; Node reaps the connection; the client throws SSE stream disconnected: TypeError: terminated and enters a reconnect loop.\n5. The user opens docs/troubleshooting.md, matches the heading, confirms they never set keepAliveMs: 0, and — per the entry's closing sentence — goes hunting for a buffering proxy that does not exist.\n\nWhy nothing prevents it. keepAliveMs is wired into streamableHttp.ts, listenRouter.ts, and (post-6d20a1f) legacyStatelessFallback — but invoke() has no options plumbing for it, and PerRequestHTTPServerTransport accepts no such option, so nothing fails at compile time; the gap is silent.\n\nHow to fix. The full fix: give PerRequestHTTPServerTransport a keepAliveMs option that drives writeCommentFrame() from an interval while an SSE exchange is open (cleared on close/settle), and thread createMcpHandler's keepAliveMs through invoke() — this also gives the orphaned heartbeat method its intended caller. The minimal pre-merge fix: scope the troubleshooting entry's closing sentence to the streams that actually have keep-alive (WebStandard transport streams, listen streams, legacy fallback) and note that modern per-request exchange streams do not yet emit keep-alive frames, so the entry stops ruling out the real cause. Severity is driven by the doc half: the runtime gap is pre-existing in perRequestTransport, but the troubleshooting prose ships in this diff and actively misdirects users on the SDK's flagship modern serving path.
| ): void { | ||
| // A deferred arm (e.g. after an event-store await) must not outlive the | ||
| // transport: close()'s timer sweep has already run and never runs again. | ||
| if (this._keepAliveMs <= 0 || this._closed) { | ||
| return; | ||
| } | ||
| this.stopKeepAlive(streamId); | ||
| const timer = setInterval(() => { | ||
| try { | ||
| controller.enqueue(encoder.encode(': keepalive\n\n')); | ||
| } catch { | ||
| this.stopKeepAlive(streamId); | ||
| } | ||
| }, this._keepAliveMs); | ||
| // Don't let the keep-alive timer hold the process open (Node.js only) | ||
| (timer as { unref?: () => void }).unref?.(); |
There was a problem hiding this comment.
🟡 The guard 'if (this._keepAliveMs <= 0 || this._closed) return;' passes NaN (NaN <= 0 is false), and Node clamps setInterval delays of NaN/Infinity/>2^31-1 to ~1ms — so a non-finite keepAliveMs (e.g. parseInt of an unset env var, or Infinity as a "never" sentinel) floods every SSE stream with ~1000 ': keepalive' frames/sec, while listenRouter.ts:221 uses the opposite polarity ('if (keepAliveMs > 0)') and safely disables for the same value — one createMcpHandler option, opposite behaviors on the two legs it now covers. One-line fix: match listenRouter's polarity ('if (!(this._keepAliveMs > 0) || this._closed)') and/or normalize with Number.isFinite in the constructor.
Extended reasoning...
What the bug is. startKeepAlive() guards with if (this._keepAliveMs <= 0 || this._closed) return;. NaN <= 0 evaluates false, so a NaN value passes the guard and reaches setInterval(fn, NaN) — which Node clamps to a ~1ms delay (empirically verified on this repo's Node: ~40 fires in 50ms; only a process-level warning is emitted for the out-of-range case). Values above 2^31 - 1 (Infinity, Number.MAX_SAFE_INTEGER, or any "effectively never" sentinel) hit Node's TimeoutOverflowWarning and are likewise clamped to 1ms. The option is stored unvalidated in the constructor (this._keepAliveMs = options.keepAliveMs ?? DEFAULT_KEEP_ALIVE_MS), so nothing upstream catches it.\n\nThe intra-PR divergence. listenRouter.ts:221 guards the same-named option with the opposite polarity — if (keepAliveMs > 0) — under which NaN > 0 is false and keep-alive is safely disabled. This PR now threads one keepAliveMs value from createMcpHandler to both the listen router and the legacy-leg transport (createMcpHandler.ts:645–648, added in 6d20a1f to unify the option's scope, with JSDoc claiming uniform coverage). So createMcpHandler(factory, { keepAliveMs: NaN }) silently disables keep-alive on subscriptions/listen streams while flooding legacy-leg SSE streams at ~1kHz — same option, same value, opposite behaviors on the two legs the option's own JSDoc says it covers.\n\nStep-by-step proof.\n1. Operator configures new WebStandardStreamableHTTPServerTransport({ keepAliveMs: parseInt(process.env.MCP_KEEPALIVE_MS!) }) with the env var unset → parseInt(undefined) is NaN.\n2. A client opens the standalone GET stream. startKeepAlive('_GET_stream', ...) runs; NaN <= 0 is false, this._closed is false → guard passes.\n3. setInterval(fn, NaN) arms; Node coerces the invalid delay to 1ms.\n4. Every ~1ms the timer enqueues ': keepalive\n\n' — ~13 KB/s of junk per open stream, constant timer churn, and unbounded controller-queue growth whenever the consumer stalls. Compliant SSE parsers drop comment frames before event dispatch, so the flood is invisible client-side and hard to diagnose.\n5. Meanwhile the same NaN fed through createMcpHandler reaches listenRouter.ts:221, where NaN > 0 is false → listen streams get no keep-alive at all. The operator intended one thing and got two different wrong things.\n\nWhy existing code doesn't prevent it. TypeScript's number type admits NaN/Infinity; the <= 0 polarity was chosen to disable on 0/negative but NaN fails all comparisons, so it slips through only this polarity; and Node clamps out-of-range setInterval delays to 1 instead of throwing.\n\nAddressing the refutation. One verifier argued this is garbage-in-garbage-out on operator-controlled config: the documented domain is 0 or a positive finite number, sibling options (retryInterval, maxSubscriptions) are equally unvalidated per repo convention, and both legs agree for every in-domain value. All of that is fair — which is why this is a nit, not blocking: no valid configuration is affected, the input is never attacker-controlled, and the trigger is a caller error. But the finding doesn't dissolve: the polarity divergence is within code this PR touches, against the sibling implementation the option was explicitly modeled on (PR description: "naming matches createMcpHandler's existing keepAliveMs"), and the two failure modes for the same garbage value point in opposite directions (silent flood vs. silent disable). The listenRouter polarity makes garbage fail safe; the new transport polarity makes it fail loud-but-invisible. Matching the safer polarity costs one character of logic and requires no new validation convention.\n\nHow to fix. Invert the guard to if (!(this._keepAliveMs > 0) || this._closed) return; so NaN (and any non-comparable value) disables rather than floods, matching listenRouter.ts:221. Optionally also normalize in the constructor — this._keepAliveMs = Number.isFinite(options.keepAliveMs) ? options.keepAliveMs! : DEFAULT_KEEP_ALIVE_MS — and cap at 2^31 - 1 if Infinity-as-never should mean "disabled" rather than "default".
| releaseReplay = resolve; | ||
| }); | ||
| return 'stream-1'; | ||
| } | ||
| }; | ||
| const transport = new WebStandardStreamableHTTPServerTransport({ sessionIdGenerator: () => randomUUID(), eventStore }); | ||
| await new McpServer({ name: 'test-server', version: '1.0.0' }).connect(transport); | ||
| const initResponse = await transport.handleRequest(createRequest('POST', TEST_MESSAGES.initialize)); | ||
| const sessionId = initResponse.headers.get('mcp-session-id') as string; | ||
|
|
||
| // Enter replayEvents and park on the replayEventsAfter await | ||
| const pendingGet = transport.handleRequest( | ||
| createRequest('GET', undefined, { sessionId, extraHeaders: { 'Last-Event-ID': 'evt-1' } }) | ||
| ); | ||
| await vi.advanceTimersByTimeAsync(0); | ||
| expect(releaseReplay).toBeDefined(); | ||
|
|
||
| // Close the transport mid-await, then let the replay continuation run | ||
| await transport.close(); | ||
| releaseReplay?.(); | ||
| await pendingGet; | ||
|
|
||
| // The deferred continuation must not have armed a timer close() can never sweep | ||
| expect(vi.getTimerCount()).toBe(0); | ||
| }); |
There was a problem hiding this comment.
🟡 The regression test 'should not arm keep-alive when the transport closes during an event-store replay await' never actually reaches startKeepAlive: its mock replayEventsAfter returns 'stream-1', which can never appear in _requestToStreamMapping, so the no-in-flight early-close branch closes the stream and the assertion vi.getTimerCount() === 0 holds vacuously — deleting || this._closed from the guard it was added to protect leaves this test green. Have the mock return '_GET_stream' (the standalone stream id) instead; mutation-verified that this makes the test fail when the guard is removed, turning it into a real regression test.
Extended reasoning...
What the bug is. This test (streamableHttp.test.ts:1543-1567) was added in 6d20a1f as the regression test for the confirmed close-during-replayEventsAfter race, and the follow-up PR comment explicitly cites it as such ('Regression test: close mid-replay-await, assert zero armed timers after the continuation runs'). But the test's assertion passes for a reason unrelated to the this._closed guard it was written to protect: with the test's inputs, startKeepAlive is never invoked at all, so vi.getTimerCount() === 0 holds vacuously.
The code path. The mock replayEventsAfter returns 'stream-1'. In replayEvents (streamableHttp.ts:653-692), after the mapping registration, the no-in-flight-request branch runs: 'stream-1' !== '_GET_stream', and hasInFlightRequest = [...this._requestToStreamMapping.values()].includes('stream-1') is necessarily false — the values in _requestToStreamMapping are only ever crypto.randomUUID() streamIds minted in handlePostRequest, never a fabricated 'stream-1' (and the init request 'init-1' was already retired by send()'s all-responses-ready cleanup anyway). So the early-close path deletes _streamMapping.get('stream-1') and closes the controller. The arm guard at line 688 — this._streamMapping.get(replayedStreamId)?.controller === streamController — then evaluates undefined === controller → false, and startKeepAlive is skipped entirely.
Step-by-step proof (mutation experiment, empirically verified by three independent verifiers).
- Delete
|| this._closedfrom thestartKeepAliveguard at streamableHttp.ts:303 — i.e., reintroduce the exact bug this test claims to prevent. - Run the test: it still passes (timer count is 0 because
startKeepAlivewas never called, not because the guard blocked it). - Now change the mock's return value to
'_GET_stream'(the standalone stream id, for which the early-close branch is skipped unconditionally) and keep the guard removed: the test fails withexpected 1 to be +0—startKeepAliveis reached post-close and a timer leaks, exactly the confirmed race. - Restore the guard with the fixed mock: the test passes. The fixed variant is a real regression test; the shipped one is inert.
Why existing code doesn't prevent it. Nothing in the test constructs either condition that lets the arm attempt survive to startKeepAlive: the replayed id is neither '_GET_stream' (which skips the early-close unconditionally) nor a streamId with a request still in flight (close() clears _streamMapping/_keepAliveTimers/_requestResponseMap but not _requestToStreamMapping, so a parked tool call would keep hasInFlightRequest true).
Impact. The production fix itself is correct and present — nothing breaks at merge time. The gap is that the confirmed close-race (an unclearable interval writing : keepalive into a dead session forever, since close() is re-entrancy-guarded and its timer sweep never runs again) has no real regression coverage: a future refactor that drops the _closed guard ships the bug back with green CI. This also matches the repo's recurring catch about prose claims the diff doesn't back — the PR comment claims regression coverage that doesn't exist. (Repo checklist: 'New behavior has vitest coverage including error paths'.)
How to fix. One-line change: have the test's replayEventsAfter return '_GET_stream' instead of 'stream-1'. The early-close branch is then skipped, the line-688 identity check passes, startKeepAlive is reached, and only the _closed guard keeps the timer count at 0. (Alternative: keep a gated tool call in flight on the replayed streamId across the close.)
| * The entry passes its own `onerror` here when expanding the default, so | ||
| * legacy-leg failures are never silently swallowed. | ||
| */ | ||
| export function legacyStatelessFallback(factory: McpServerFactory, onerror?: (error: Error) => void): LegacyHttpHandler { | ||
| export function legacyStatelessFallback( | ||
| factory: McpServerFactory, | ||
| onerror?: (error: Error) => void, | ||
| transportOptions?: { keepAliveMs?: number } | ||
| ): LegacyHttpHandler { | ||
| return async (request, options) => { | ||
| if (request.method.toUpperCase() !== 'POST') { | ||
| return jsonRpcErrorResponse(405, -32_000, 'Method not allowed.'); |
There was a problem hiding this comment.
🟡 Two doc blocks in this file still claim the legacy fallback transport is constructed "with only sessionIdGenerator: undefined" — the CreateMcpHandlerOptions.legacy JSDoc (line 148) and legacyStatelessFallback's own doc block (lines 298–300, which also says "unchanged" directly above the signature this PR modified). Both now contradict the updated keepAliveMs JSDoc in the same interface, and the new transportOptions parameter on this public export is undocumented; reword both sites and document the parameter.
Extended reasoning...
What's stale. Commit 6d20a1f (addressing the earlier review round) threads keepAliveMs into the legacy fallback: legacyStatelessFallback gained a third transportOptions?: { keepAliveMs?: number } parameter, and its per-request transport is now constructed as new WebStandardStreamableHTTPServerTransport({ sessionIdGenerator: undefined, ...(transportOptions?.keepAliveMs !== undefined && { keepAliveMs: ... }) }) (lines 325–328), with createMcpHandler forwarding options.keepAliveMs into it (lines 643–648). Two prose sites in the same file were left describing the pre-fix construction:
- Line 148 (
CreateMcpHandlerOptions.legacyJSDoc): "…over a streamable HTTP transport constructed with onlysessionIdGenerator: undefined(the established stateless idiom)". - Lines 298–300 (
legacyStatelessFallback's doc block): "…constructed with onlysessionIdGenerator: undefined— the established stateless idiom, unchanged" — directly above the signature this PR changed.
Why it's wrong now. The keepAliveMs option JSDoc updated by this very PR (lines 196–201) promises the option is "applied to subscriptions/listen streams and to the SSE streams of the legacy stateless fallback's per-request transport", and docs/troubleshooting.md (also added in this PR) repeats that promise. So the options interface now contradicts itself: the legacy option's JSDoc says the transport gets only sessionIdGenerator, while the keepAliveMs JSDoc two options down says it reaches that same transport. The word "unchanged" at line 300 is falsified by the diff immediately below it. Additionally, legacyStatelessFallback is a public export ("Exported as a standalone building block for hand-wired compositions"), and its new third parameter has no JSDoc at all — a consumer hand-wiring it has no documented way to discover the keep-alive plumbing.
Concrete walk-through. (1) A consumer deploys createMcpHandler(factory, { keepAliveMs: 0 }) to silence comment frames. (2) They read the legacy option's JSDoc at line 148 to understand the legacy leg and conclude the fallback transport is built with only sessionIdGenerator: undefined — i.e. the transport-level default of 15 s applies and their 0 can't reach it. (3) They rewrap or fork the legacy leg to suppress the frames — exactly the confusion 6d20a1f was made to eliminate, since the code actually does thread their 0 through. Conversely, a hand-wirer calling legacyStatelessFallback(factory, onerror) directly sees no documented transportOptions parameter and assumes the leg is untunable.
Why nothing catches it. JSDoc is prose; nothing at compile or test time checks it against the construction site, and the repo's sync:snippets machinery only covers fenced @example regions, not free prose.
This matches the repo's Documentation recurring catch (REVIEW.md #1718, #1838: prose in the same diff promising behavior the code no longer ships), making it a repo-requested check rather than a style preference — but that rule flags the finding; it doesn't make it blocking, and there is no runtime effect.
Fix. Reword both sites, e.g. "constructed with sessionIdGenerator: undefined (stateless) plus the handler's keepAliveMs when provided", drop "unchanged" from the legacyStatelessFallback doc block, and add a short JSDoc line documenting the transportOptions parameter (transport options threaded into the per-request transport; currently just keepAliveMs).
1 participant
v2 port of #2538 (v1.x). Refs #1211.
Problem
Idle SSE streams from
WebStandardStreamableHTTPServerTransportare killed by idle-connection timeouts (Node'sserver.requestTimeoutdefault of 300s, reverse proxies, cloud LBs). The standalone GET stream is the worst case, but POST response streams during long-running tool calls hit the same thing. Clients observeSSE stream disconnected: TypeError: terminatedevery ~5 minutes and enter a reconnect loop. v2 already keep-alivessubscriptions/listenstreams vialistenRouter, but the transport's own SSE streams have no equivalent — downstream users (e.g. Cloudflare'sagentsSDK v2 integration) currently rewrap the transport's SSE response bodies just to inject keep-alive frames.Fix
keepAliveMs(naming matchescreateMcpHandler's existingkeepAliveMs), defaulting to 15000 (the WHATWG SSE spec recommends a comment line roughly every 15 seconds). Set0to disable.NodeStreamableHTTPServerTransportinherits it via the aliased options type.: keepalivecomment frame. Comment frames are dropped by SSE parsers before event dispatch, so default-on is wire-transparent to compliant clients.unref'd where supported, cleared on stream cleanup/cancel and transportclose(), self-clear if a write hits a closed controller, and a resumed stream re-registered under the same stream id supersedes its predecessor's timer. The replay path only arms keep-alive if the stream survived the no-in-flight-request early close.Tests
Added an
SSE keep-alivesuite (fake timers): frames on idle GET stream, custom interval,keepAliveMs: 0disables, timers fully cleared on close, frames on a POST SSE stream while a tool call is pending. Server package tests (453), typecheck, and lint pass; fulltest:allincluding e2e passes with the change (the twoprotocol:timeout:max-total [sse]flakes reproduce on cleanmainwithout it). Changeset included (patch).