fix(mcp): SSE resumability for McpAgent + correct keepalive for stateless WorkerTransport (#1583)#1602
Merged
mattzcarey merged 6 commits intoMay 28, 2026
Conversation
🦋 Changeset detectedLatest commit: f85177d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
468
to
471
| cleanup: () => { | ||
| clearInterval(keepAlive); | ||
| this.streamMapping.delete(streamId); | ||
| writer.close().catch(() => {}); | ||
| } |
Contributor
There was a problem hiding this comment.
🟡 Remapped stream cleanup omits stopKeepalive() — incomplete mechanical transformation
When the GET handler remaps the stream ID after replay (line 462-472), the replacement cleanup function does not call stopKeepalive(). The original cleanup at line 438-442 correctly calls it. In the base code, both cleanup functions called clearInterval(keepAlive), so this is an incomplete transformation from the old clearInterval pattern to the new stopKeepalive() pattern.
Currently this is harmless because the remap only happens when this.eventStore is truthy (line 451), and armKeepalive returns a no-op () => {} in that case (worker-transport.ts:299). However, the inconsistency breaks the invariant that every stream's cleanup tears down the keepalive, which could become a real interval leak if armKeepalive is later modified.
Suggested change
| cleanup: () => { | |
| clearInterval(keepAlive); | |
| this.streamMapping.delete(streamId); | |
| writer.close().catch(() => {}); | |
| } | |
| cleanup: () => { | |
| stopKeepalive(); | |
| this.streamMapping.delete(streamId); | |
| writer.close().catch(() => {}); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
added 2 commits
May 28, 2026 12:17
McpAgent.serve() opens an SSE stream from the Worker to the client that
the Cloudflare edge closes after ~5 minutes of inactivity. The existing
workaround was to write keepalive pings, which would have prevented the
Durable Object from hibernating and contradict the serverless model.
This switches McpAgent to the spec-sanctioned recovery path instead:
- New `DurableObjectEventStore` that backs the MCP SDK's `EventStore`
interface with the agent's own DO storage. Bounded at 256 events per
stream (configurable), namespaced under `__mcp_event__:` to avoid
user-state collisions, and rehydrates its sequence counter from
storage after hibernation so reconnects never issue duplicate IDs.
- `McpAgent` exposes a `getEventStore()` hook (override to disable or
swap) and `initTransport()` wires the result into
`StreamableHTTPServerTransport`. Resumability is now on by default.
- `StreamableHTTPServerTransport.handleGetRequest` now tags the
reconnected connection as the standalone SSE stream *before* replay.
Previously the early `return` after `replayEvents` meant a resumed
stream received only the backlog and then sat idle — server-initiated
notifications had no connection to land on. Per the 2025-03-26 spec
the server replays missed messages on the disconnected stream *and*
continues delivering subsequent messages on the same stream.
Verified end-to-end against a deployed worker: idle GET still drops at
~270s (DO is free to hibernate), and a follow-up GET with Last-Event-ID
returns 200 with no spurious `event: ping` frames.
…1583) The MCP transports had a `event: ping\ndata: \n\n` keepalive frame that the SSE parser dispatched as a `MessageEvent` with `type="ping"` and empty data, firing any `addEventListener("ping", ...)` listener the client had registered. Per the WHATWG SSE spec the keepalive form is the comment frame `: ...\n\n`, which the parser drops outright ("If the line starts with a U+003A COLON character (:), ignore the line"). Beyond the frame format, the previous code armed a keepalive on every SSE response stream. That made sense for POST tool-call responses (scoped to a single request id, cannot be resumed), but for the standalone GET listen stream it forced the server to stay alive indefinitely instead of letting the edge close idle connections and relying on `Last-Event-ID` resumption \u2014 which `McpAgent` now defaults to via `DurableObjectEventStore`. This commit splits the policy by stream direction: - GET (standalone listen stream): never keepalive. Idle drops are recovered by clients reconnecting with `Last-Event-ID` against the configured `EventStore`. `McpAgent` ships one by default; `WorkerTransport` callers bring their own. - POST (tool response stream): always keepalive. The transport writes `: keepalive\n\n` every 25s so long-running tool calls survive the ~5min Cloudflare edge idle-stream watchdog. The two transport implementations (`utils.ts` for `McpAgent`, `worker-transport.ts` for `createMcpHandler`) now share the keepalive helper in `sse-keepalive.ts`.
mattzcarey
force-pushed
the
fix/mcp-sse-keepalive-issue-1583
branch
from
May 28, 2026 11:17
149546b to
97f238d
Compare
…are#1583) In the GET handler, the standalone SSE stream may be re-mapped under a different streamId after the eventStore replays missed events. Both the initial `streamMapping.set` and the remapped one previously inlined identical cleanup closures, which made it easy to drift if teardown later grew a new step (e.g. tearing down a keepalive). Hoist the cleanup into a single `const cleanup` closure used by both mappings. The closure reads `streamId` lazily so it stays correct after the remap rebinding. Behaviour unchanged; the invariant that every mapping shares the same teardown is now structural rather than copy-paste. Addresses devin-ai-integration review feedback on PR cloudflare#1602.
whoiskatrin
reviewed
May 28, 2026
| case "streamable-http": { | ||
| const transport = new StreamableHTTPServerTransport({}); | ||
| const transport = new StreamableHTTPServerTransport({ | ||
| eventStore: this.getEventStore() |
Contributor
There was a problem hiding this comment.
wondering if this could keep adding stuff to storage over time
the cap is per stream, but it looks like every POST gets a new connection id. so most tool calls would add a couple events to a new stream, never hit the cap, and i can't see anything removing them after the response is done
now that this is on by default for McpAgent, feels like a busy session could just keep building up old POST response events in DO storage
should finished POST streams be cleared, or should there be some cap for the whole session?
Contributor
Author
There was a problem hiding this comment.
POST streams shouldnt add to storage. Should just be GETs. POST needs the keep alive for this reason. Will double check
Contributor
Author
There was a problem hiding this comment.
Adding a ttl or some type of cleanup. good shout.
| writer.close().catch(() => {}); | ||
| } | ||
| }); | ||
| // No keepalive on the standalone GET stream. Idle drops are recovered |
Contributor
There was a problem hiding this comment.
could this break WorkerTransport setups that don't pass an eventStore?
before this, the GET stream stayed open because of the keepalive. now it can get dropped when idle, but without an eventStore there aren't any ids or replay to recover from that
i think the mcp elicitation example might hit this too since it has a persistent WorkerTransport with storage but no eventStore
feels like GET should keep the new comment keepalive when there isn't an eventStore, and only rely on reconnecting once one is set
Contributor
Author
There was a problem hiding this comment.
the keepalive on the GET is just burning cash tho. it should only be for notifications as is being removed real soon. Elicitations go down the POST for the tool call that caused them. If they dont thats a bug that users need to fix.
Contributor
Author
There was a problem hiding this comment.
feels like GET should keep the new comment keepalive when there isn't an eventStore, and only rely on reconnecting once one is set
will add this this tho. Nice to keep it a patch change.
| * edge idle watchdog (~5 min) and never carries a server-side | ||
| * keepalive. | ||
| * | ||
| * POST response streams are scoped to a single request id and can't be |
Contributor
There was a problem hiding this comment.
not sure about saying POST streams can't be resumed here
i think with streamable HTTP, if the POST SSE connection drops, the client can reconnect with GET and Last Event ID and get events from that original stream replayed. this code also seems to do that already since it stores the POST events
would be clearer to say the keepalive stops an in progress tool call from getting dropped, instead of saying POST can't be resumed
Contributor
Author
There was a problem hiding this comment.
will check this. I'm not sure you can resume POST messaged down the reconnected GET cause now they would be orphaned with respect to the tool call they were for.
The default event store is an implementation detail of McpAgent.getEventStore() and not part of the public API. Drop the public re-export from agents/mcp, mark the class @internal, and remove the recommendation to construct it directly from the WorkerTransportOptions.eventStore JSDoc. Callers who want resumability on a WorkerTransport bring their own EventStore implementation.
whoiskatrin
self-requested a review
May 28, 2026 11:57
whoiskatrin
approved these changes
May 28, 2026
whoiskatrin
left a comment
whoiskatrin
left a comment
Contributor
There was a problem hiding this comment.
Overall looks pretty solid, just a few comments above
added 2 commits
May 28, 2026 13:14
Three fixes following review on cloudflare/agents PR cloudflare#1602: **1. POST stream resumption (transport.ts) — Kate concern 3.** The previous implementation stored POST events but couldn't actually resume them: events were keyed by the POST WS connection.id, and the send() routing looked up connections by their state.requestIds. When the POST WS died, both were lost; a reconnecting GET created a fresh WS with a fresh connection.id and no requestIds. Replay reached the new connection but in-flight tool messages never could. Fix: introduce a stable streamId on the connection state (seeded with connection.id for fresh POSTs), persist a streamId -> requestIds mapping in DO storage, and on resumed GET-with-Last-Event-ID restore those requestIds onto the new WS. Subsequent send() calls keyed by requestId now find the resumed connection. New McpAgent helpers (set/get/deleteStreamRequestIds) wrap the persistence so the transport doesn't reach into protected ctx.storage directly. **2. Bounded event-store storage (event-store.ts, index.ts) — Kate concern 1.** The cap was per-stream, so busy sessions with many short-lived POST streams under the cap could accumulate events forever in DO storage. Fix: clear streams cleanly on the final POST response (eventStore clearStream + deleteStreamRequestIds), and add a TTL safety net for the unhappy path. DurableObjectEventStore now wraps stored values with a write timestamp and exposes sweep(maxAgeMs). McpAgent schedules a recurring sweep via the existing Agent scheduler (default cron */5 minutes, default 1 hr TTL), gated on idempotent: true so duplicate schedules don't pile up across hibernation/restart. getEventStoreMaxAgeMs() and getEventStoreSweepCron() expose both knobs. **3. WorkerTransport GET keepalive when no eventStore (worker-transport.ts) — Kate concern 2.** Previous behaviour kept GET streams alive via setInterval. New code relied entirely on resumability, which is a regression for callers who don't configure an eventStore. Restore the policy: keepalive when no eventStore (preserve pre-fix behaviour), skip keepalive when one is configured (resumability is the recovery path). Keeps this PR a true patch with no behaviour regression. **4. Re-export DurableObjectEventStore + wire into elicitation example.** The elicitation example demonstrates the stateful createMcpHandler pattern (Agent-hosted WorkerTransport with persistent storage). It needs a concrete EventStore to opt into resumability without writing its own. Removed @internal, exported from agents/mcp, added `eventStore: new DurableObjectEventStore(this.ctx.storage)` to the example. **5. Fixed stale JSDoc on WorkerTransportOptions.eventStore** that claimed POST streams can't be resumed. They can. Tests: 434/434 pass. Added 4 new tests covering sweep behaviour (deletes old events, no-op on bad input, respects batchSize, restarts seq after wiping a stream).
…re#1583) Five-minute sweeps with a one-hour TTL were too aggressive \u2014 cron tick cost on a busy DO and a tight window for client reconnects. Move to hourly sweeps (cron `0 * * * *`) and a 24-hour event TTL so abandoned streams still get garbage-collected but clients have all day to reconnect with `Last-Event-ID`. Knobs (`getEventStoreSweepCron`, `getEventStoreMaxAgeMs`) unchanged \u2014 override either to tighten or relax.
threepointone
approved these changes
May 28, 2026
threepointone
left a comment
threepointone
left a comment
Contributor
There was a problem hiding this comment.
feel free to land after CI passes
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
Merged
mattzcarey
pushed a commit
to mattzcarey/agents
that referenced
this pull request
May 28, 2026
cloudflare#1583 followup) Reworking the resumability sweep so quiescent McpAgent DOs do no periodic work, and so the sweep scales to sessions that emit millions of events. **The clearStream-before-send race (was: Devin Review on cloudflare#1602).** The previous code wrote the final POST response into the event store via storeEvent() and then immediately called clearStream(streamId), wiping every event for that stream including the one just written. Clients that lost the connection right as the response was in flight could reconnect with Last-Event-ID and find an empty store. Drop the clearStream() call entirely; events persist until either the per-stream cap evicts them or the on-demand sweep does. **Unconditional event-store writes on the request-bound send() path.** The standalone branch already stored events regardless of whether a live WS was attached. The request-bound branch only stored when a live connection existed via state.requestIds, dropping mid-flight tool messages whenever the client lost the WS. Now we fall back to a persistent requestId -> streamId reverse lookup (McpAgent.getStreamIdForRequestId, scanning __mcp_stream_reqs__:*) so the event is recorded even after the originating connection died. writeSSEEvent only fires when a live connection is still attached; reconnecting with Last-Event-ID replays everything that was missed. **Resumed connection no longer plays standalone double-duty.** A GET reconnecting from a completed POST stream used to get tagged _standaloneSse: true so it kept receiving notifications, but that caused future standalone events to store under the dead POST's streamId and fragment the standalone log. Now the resumed connection registers under the source streamId and only inherits requestIds if the source is an active POST. Standalone-stream resumption still gets the _standaloneSse tag. Matches the SDK reference. **Stream-level lifetime tracking + on-demand sweep schedule.** Every storeEvent writes a __mcp_stream_evt_meta__:<streamId> key alongside the event itself, holding lastWriteAt for that stream. The sweep scans only this metadata index (O(active streams), not O(total events)) and deletes entire streams whose last write is past the configured TTL. At "millions of events per session" scale this is 1000x cheaper than scanning the event log. The sweep itself is no longer a recurring cron. The store fires an onStoreEvent hook after every write; the agent uses it to arm an idempotent one-shot schedule maxAgeMs into the future. The sweep callback, after running, peeks the oldest remaining stream and either reschedules at exactly oldest.lastWriteAt + maxAgeMs OR (if nothing remains) doesn't reschedule at all. Quiescent McpAgents stop ticking entirely; the first write after a quiet period re-arms. Knobs: getEventStoreMaxAgeMs (default 24h), getEventStoreSweepMaxIterations (default 10 pages * batchSize=256 streams per invocation). Removed getEventStoreSweepCron — no longer applicable. **DurableObjectEventStore is exported.** Callers embedding WorkerTransport inside an Agent (e.g. the elicitation example) can opt into resumability with new DurableObjectEventStore(this.ctx.storage). Tests: 457/457 MCP tests pass. New coverage for the sweep return shape, getOldestStreamWriteAt, onStoreEvent hook, batchSize pagination, and the replay-after-stream-expired path. npm run check exit 0 across all 88 projects.
threepointone
pushed a commit
that referenced
this pull request
May 28, 2026
…less WorkerTransport (#1583) (#1602) * fix(mcp): default-enable SSE resumability for McpAgent (#1583) McpAgent.serve() opens an SSE stream from the Worker to the client that the Cloudflare edge closes after ~5 minutes of inactivity. The existing workaround was to write keepalive pings, which would have prevented the Durable Object from hibernating and contradict the serverless model. This switches McpAgent to the spec-sanctioned recovery path instead: - New `DurableObjectEventStore` that backs the MCP SDK's `EventStore` interface with the agent's own DO storage. Bounded at 256 events per stream (configurable), namespaced under `__mcp_event__:` to avoid user-state collisions, and rehydrates its sequence counter from storage after hibernation so reconnects never issue duplicate IDs. - `McpAgent` exposes a `getEventStore()` hook (override to disable or swap) and `initTransport()` wires the result into `StreamableHTTPServerTransport`. Resumability is now on by default. - `StreamableHTTPServerTransport.handleGetRequest` now tags the reconnected connection as the standalone SSE stream *before* replay. Previously the early `return` after `replayEvents` meant a resumed stream received only the backlog and then sat idle — server-initiated notifications had no connection to land on. Per the 2025-03-26 spec the server replays missed messages on the disconnected stream *and* continues delivering subsequent messages on the same stream. Verified end-to-end against a deployed worker: idle GET still drops at ~270s (DO is free to hibernate), and a follow-up GET with Last-Event-ID returns 200 with no spurious `event: ping` frames. * fix(mcp): correct SSE keepalive on POST response streams (#1583) The MCP transports had a `event: ping\ndata: \n\n` keepalive frame that the SSE parser dispatched as a `MessageEvent` with `type="ping"` and empty data, firing any `addEventListener("ping", ...)` listener the client had registered. Per the WHATWG SSE spec the keepalive form is the comment frame `: ...\n\n`, which the parser drops outright ("If the line starts with a U+003A COLON character (:), ignore the line"). Beyond the frame format, the previous code armed a keepalive on every SSE response stream. That made sense for POST tool-call responses (scoped to a single request id, cannot be resumed), but for the standalone GET listen stream it forced the server to stay alive indefinitely instead of letting the edge close idle connections and relying on `Last-Event-ID` resumption \u2014 which `McpAgent` now defaults to via `DurableObjectEventStore`. This commit splits the policy by stream direction: - GET (standalone listen stream): never keepalive. Idle drops are recovered by clients reconnecting with `Last-Event-ID` against the configured `EventStore`. `McpAgent` ships one by default; `WorkerTransport` callers bring their own. - POST (tool response stream): always keepalive. The transport writes `: keepalive\n\n` every 25s so long-running tool calls survive the ~5min Cloudflare edge idle-stream watchdog. The two transport implementations (`utils.ts` for `McpAgent`, `worker-transport.ts` for `createMcpHandler`) now share the keepalive helper in `sse-keepalive.ts`. * refactor(mcp): share cleanup closure across GET stream remap (#1583) In the GET handler, the standalone SSE stream may be re-mapped under a different streamId after the eventStore replays missed events. Both the initial `streamMapping.set` and the remapped one previously inlined identical cleanup closures, which made it easy to drift if teardown later grew a new step (e.g. tearing down a keepalive). Hoist the cleanup into a single `const cleanup` closure used by both mappings. The closure reads `streamId` lazily so it stays correct after the remap rebinding. Behaviour unchanged; the invariant that every mapping shares the same teardown is now structural rather than copy-paste. Addresses devin-ai-integration review feedback on PR #1602. * chore(mcp): mark DurableObjectEventStore as internal (#1583) The default event store is an implementation detail of McpAgent.getEventStore() and not part of the public API. Drop the public re-export from agents/mcp, mark the class @internal, and remove the recommendation to construct it directly from the WorkerTransportOptions.eventStore JSDoc. Callers who want resumability on a WorkerTransport bring their own EventStore implementation. * fix(mcp): address Kate's review concerns on PR #1602 Three fixes following review on cloudflare/agents PR #1602: **1. POST stream resumption (transport.ts) — Kate concern 3.** The previous implementation stored POST events but couldn't actually resume them: events were keyed by the POST WS connection.id, and the send() routing looked up connections by their state.requestIds. When the POST WS died, both were lost; a reconnecting GET created a fresh WS with a fresh connection.id and no requestIds. Replay reached the new connection but in-flight tool messages never could. Fix: introduce a stable streamId on the connection state (seeded with connection.id for fresh POSTs), persist a streamId -> requestIds mapping in DO storage, and on resumed GET-with-Last-Event-ID restore those requestIds onto the new WS. Subsequent send() calls keyed by requestId now find the resumed connection. New McpAgent helpers (set/get/deleteStreamRequestIds) wrap the persistence so the transport doesn't reach into protected ctx.storage directly. **2. Bounded event-store storage (event-store.ts, index.ts) — Kate concern 1.** The cap was per-stream, so busy sessions with many short-lived POST streams under the cap could accumulate events forever in DO storage. Fix: clear streams cleanly on the final POST response (eventStore clearStream + deleteStreamRequestIds), and add a TTL safety net for the unhappy path. DurableObjectEventStore now wraps stored values with a write timestamp and exposes sweep(maxAgeMs). McpAgent schedules a recurring sweep via the existing Agent scheduler (default cron */5 minutes, default 1 hr TTL), gated on idempotent: true so duplicate schedules don't pile up across hibernation/restart. getEventStoreMaxAgeMs() and getEventStoreSweepCron() expose both knobs. **3. WorkerTransport GET keepalive when no eventStore (worker-transport.ts) — Kate concern 2.** Previous behaviour kept GET streams alive via setInterval. New code relied entirely on resumability, which is a regression for callers who don't configure an eventStore. Restore the policy: keepalive when no eventStore (preserve pre-fix behaviour), skip keepalive when one is configured (resumability is the recovery path). Keeps this PR a true patch with no behaviour regression. **4. Re-export DurableObjectEventStore + wire into elicitation example.** The elicitation example demonstrates the stateful createMcpHandler pattern (Agent-hosted WorkerTransport with persistent storage). It needs a concrete EventStore to opt into resumability without writing its own. Removed @internal, exported from agents/mcp, added `eventStore: new DurableObjectEventStore(this.ctx.storage)` to the example. **5. Fixed stale JSDoc on WorkerTransportOptions.eventStore** that claimed POST streams can't be resumed. They can. Tests: 434/434 pass. Added 4 new tests covering sweep behaviour (deletes old events, no-op on bad input, respects batchSize, restarts seq after wiping a stream). * tune(mcp): default event-store sweep to hourly with 24h TTL (#1583) Five-minute sweeps with a one-hour TTL were too aggressive \u2014 cron tick cost on a busy DO and a tight window for client reconnects. Move to hourly sweeps (cron `0 * * * *`) and a 24-hour event TTL so abandoned streams still get garbage-collected but clients have all day to reconnect with `Last-Event-ID`. Knobs (`getEventStoreSweepCron`, `getEventStoreMaxAgeMs`) unchanged \u2014 override either to tighten or relax. --------- Co-authored-by: Matt Carey <matt@cloudflare.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1583.
Summary
Two-commit fix for SSE keepalive + resumability across both MCP transports.
The policy is now split by stream direction, consistently across
McpAgent(utils.ts) andcreateMcpHandler(worker-transport.ts):Last-Event-IDagainst a configuredEventStore: keepalive\n\nevery 25s8b0c30e2McpAgent: newDurableObjectEventStore(storage-backed, hibernation-safe),McpAgent.getEventStore()hook, wired intoStreamableHTTPServerTransport(theDurableObjectEventStoreitself is@internal, callers overridegetEventStore()to swap or disable). Also fixes a pre-existing transport bug where resumed GET streams were never re-tagged as the standalone SSE connection.97f238daevent: pingkeepalive in both transports with a spec-compliant: keepalive\n\ncomment frame, armed only on POST response streams. GET streams now always rely on resumability. Shared helper insse-keepalive.ts.What's actually a bug vs. correctness vs. cleanup
Real bugs (user-visible)
McpAgent.serve()standalone GET SSE drops at ~270s with no recovery. Reproduced onagents@0.13.2against a deployed worker — the stream dies at the Cloudflare edge idle watchdog and there is no resumability wired in. Fixed by defaultingEventStoreon forMcpAgent. (Commit 1)McpAgent'shandleGetRequestreturned early afterreplayEvents()without tagging the reconnected connection as the standalone SSE stream. Resumed streams received the backlog and then sat idle — server-initiated notifications had no connection to land on. Pre-existing bug, not in the original report. Fixed by tagging before replay. (Commit 1)McpAgent's POST tool-response SSE had no keepalive. A long-running tool that didn't emit progress would let the response stream sit silent past the ~5min edge watchdog and the client would never get the result. POST streams are scoped to a single request id and can't be replayed, so resumability isn't an option — the only fix is to keepalive them. Fixed in commit 2.event: ping\ndata: \n\nis a named SSE event. Per the WHATWG SSE algorithm adata:line with empty value still appends a line feed to the data buffer, so the buffer is non-empty at dispatch time and an event withtype="ping"and empty data fires on the client.onmessagedoesn't fire (wrong type), but anyaddEventListener("ping", …)listener does. The MCP TS SDK client skips these (it discards events with empty data) but other SSE clients see them. Fixed by switching to the comment frame: keepalive\n\n. (Commit 2)Correctness (spec alignment, no behaviour change measured)
createMcpHandlerkeepalive cancellation. The reporter argued the Workers runtime cancels the baresetInterval~30s after the fetch handler returns. I could not reproduce this — a 330sslowtool call survived end-to-end against an unfixedagents@0.13.2baseline. The open SSE response itself counts as foreground I/O and keeps the worker (and therefore the interval) alive. The frame format and interval period fixes are still correct on their own merits.Cleanup
Shared
sse-keepalive.tshelper. Both transports were duplicating the same keepalive logic. Now exported once.JSDoc on
WorkerTransportOptions.eventStorenow explains the GET-vs-POST split and the BYO recipe.What's deliberately out of scope
eventStorefor statelesscreateMcpHandler. Stateless deployments can't ship a sensible default — there's no per-session storage handle available without the caller bringing one. Callers who want resumability supply their own store viaWorkerTransportOptions.eventStore. Stateless callers get the corrected POST keepalive by default.ctx.waitUntilthreading. Considered briefly to anchor the keepalive against post-handler background-work cancellation, but I couldn't reproduce that scenario empirically and the threading muddied the public API. The streaming SSE response itself keeps the worker alive in practice.Last-Event-IDentirely). All of these additions delete cleanly together.Architecture
This sticks to serverless principles:
Last-Event-IDand theEventStore(backed bythis.ctx.storage) replays missed messages. No artificial keepalive holding the DO awake.Compatibility
All changes are additive — patch-level, no breaking changes:
McpAgent.getEventStore()is overridable (return undefinedto opt out of the default store).Verification
McpAgent: GET drops at 271s withINTERNAL_ERROR, zero bytes received.McpAgent: GET still drops at 271s (DO is free to hibernate), and a follow-up GET withLast-Event-IDreturns 200 OK. Tool-call POST frames now carryid: <streamId>:<seqHex>.createMcpHandler330s tool call: completed, but emitted 10 spuriousevent: pingframes.createMcpHandler330s tool call: completed, emitted ~12: keepalivecomment frames the SSE parser discards.New tests
mcp/event-store.test.ts— 10 unit tests forDurableObjectEventStoreagainst a mockDurableObjectStorage(id format, per-stream isolation, eviction, hibernation rehydration, no timers).mcp/resumability.test.ts— 5 e2e tests through the worker entry exercising theMcpAgentresumability path.mcp/transports/post-keepalive.test.tsrewritten — asserts GET never arms a keepalive (both with and withouteventStore) and POST always does, in both modes.