Fail loudly when a DO restart kills an in-flight MCP tool call - #1489
Draft
RhysSullivan wants to merge 1 commit into
Draft
Fail loudly when a DO restart kills an in-flight MCP tool call#1489RhysSullivan wants to merge 1 commit into
RhysSullivan wants to merge 1 commit into
Conversation
Contributor
Cloudflare preview
Sign-in is Cloudflare Access (one-time PIN to an allowed email). The preview has its own database and encryption key; it is destroyed when this PR closes. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | 888bba6 | Commit Preview URL Branch Preview URL |
Jul 28 2026, 11:49 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | 888bba6 | Jul 28 2026, 11:50 PM |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
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
1 participant
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.
When a deploy resets a Durable Object mid tool call, the running execution dies
with the isolate. The client's POST SSE stream closes cleanly, so the MCP client
neither errors nor reconnects — it hangs to its own timeout (~90s observed) with
no server-side exception and no exported span. The failure is invisible in
observability: production traces carry
Durable Object reset because its code was updatedonly on session-establishment spans, and everymcp.host.tool.executespan is complete, because killed executions never export a span at all.
Reproduced with overlapping deploys: a single deploy did not trigger it (30/30
clean), rapid successive deploy bursts did (4/4).
What this does
On every runtime start, scan for POST streams that still carry persisted
in-flight request ids. The transport writes that key when a request is
dispatched and deletes it the moment the final response is written, so a
surviving entry at start means "dispatched, never answered" — after a restart
that can only be work the reset destroyed. Each one gets a JSON-RPC error
appended to its event log at the next sequence number, and the request-id key
dropped. A reconnecting client replays from
last-event-idand gets a realerror in seconds instead of hanging.
The client half of the fix — the SSE priming event that makes the MCP SDK
willing to reconnect a dropped POST stream at all — already ships in
patches/agents@0.17.3.patch. Without it the error written here would sit instorage unread. Both halves are required; measured on a deployed worker:
unfixed hung 91.5s, priming alone hung 90.8s, priming + reaper failed in 6.3s.
Retry safety — the main judgment call
The prototype's message said "please retry". This ships something different, on
purpose.
executeruns arbitrary user code, which may already have charged acard, sent mail, or written to a third party before the isolate died. The server
cannot know how far it got — the only evidence was in the dead isolate — so it
cannot honestly promise a retry is safe, and an LLM client will act on whatever
the message implies.
The error therefore states what is known and hands the decision back:
with
data: { reason: "server_restart", retrySafe: false }carrying the samefact machine-readably. A test pins that the message never says "please retry".
Making retry genuinely safe needs idempotency keys or durable execution; this
change does not pretend to have them.
Scope and edge cases
the lost work — durable execution is separate and out of scope.
McpSessionDO.restart_reapspan as afailure, which the OTEL tracer renders as status ERROR with a recorded
exception, so a deploy that ate N tool calls becomes queryable rather than an
absence. Pinned with a recording tracer; no changes to shared telemetry wiring.
the transport only closes a stream once all of them have responded, so missing
one still hangs the client.
_GET_stream): explicitly skipped. It carries no POSTrequest ids so it cannot look orphaned today, but the skip is explicit so a
future SDK change cannot make the reaper cancel a live listen stream.
durably stored, so a reset during the reap leaves the stream reapable on the
next start rather than silently forgotten.
SDK seam
getOpenStreamRequestIdsis@internalinagents. Rather than repeat theprototype's raw storage-prefix manipulation, all SDK-shaped access is behind one
structurally-typed interface in
restart-reaper.tswith a runtime shape check.Nothing else in the module knows a storage key, prefix, or the seq zero-padding.
An
agentsupgrade that moves this surface breaks in exactly one place, and ifthe method disappears the reaper stands down instead of throwing on every cold
start.
Verification
bun run typecheck— 43/43 tasks passbun run lint— 0 warnings, 0 errorsbun run format:check— all files correctly formattedbun run test— 37/37 tasks passreaper call makes the reaping tests fail with empty replays, while the
no-false-positive test still passes.