End a request's sandbox fibers before its database connection closes - #1799
Merged
Conversation
executeWithPause forks the sandbox as a daemon so a pause can outlive the caller that observed it. That fiber closes over the executor, and the FumaDB client captures its handle at construction rather than resolving it per operation, so the fiber keeps a live reference to the connection the host opened for the scope that built the engine. On the HTTP executor plane that scope is the request, and its finalizer closes the pool as soon as the response is written. A sandbox still parked at that moment issues its next query against a closed pool. Add ExecutionEngine.shutdown, which interrupts the engine's in-flight sandbox fibers and waits for them to unwind, and run it from the shared execution-stack middleware so it happens before the request scope tears the connection down. Nothing is lost by ending it there: on a per-request engine the pause is already unreachable once the response is written, since a resume lands on a different engine and replays the call. The MCP session Durable Object does not use this middleware and keeps its session-lifetime pauses. shutdown is a required member so a decorator that rebuilds the engine object cannot drop it silently.
Contributor
Cloudflare previewTorn down — the PR is closed. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-cloud | 0a47f3a | Aug 28 2026, 03:52 AM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
executor-marketing | 0a47f3a | Commit Preview URL Branch Preview URL |
Aug 28 2026, 03:52 AM |
@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: |
RhysSullivan
marked this pull request as ready for review
August 28, 2026 04:10
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.
Problem
Reads and writes through the storage layer intermittently fail on the cloud executor plane with driver-level connection faults — the pool has already been closed, or the socket belongs to a different request. They land on ordinary operations (tool and plugin-storage reads, connection health writes), and the requests they happen inside otherwise succeed, so the failures are swallowed and the caller silently gets a degraded answer.
The cause is a lifetime mismatch.
executeWithPauseforks the sandbox as a daemon fiber on purpose, so a paused execution can outlive the caller that observed the pause. What that fiber closes over is the problem: its tool invoker is built from the request's executor, and the storage client captures its database handle at construction rather than resolving it per operation. The fiber therefore holds the connection that belongs to whichever scope built the engine.On the HTTP executor plane that scope is the request. Its finalizer closes the connection as soon as the response is written, while the forked sandbox is still parked. The next query that fiber makes runs against a closed pool.
Fix
Give the engine an explicit end-of-life seam and use it where the engine is per-request:
ExecutionEngine.shutdowninterrupts the engine's in-flight sandbox fibers and waits for them to finish unwinding. Awaiting matters — returning early would leave the same race open, just narrower.Effect.ensuring, so the fibers are gone before the request scope closes the connection.Nothing is lost by ending them there. On a per-request engine a pause is already unreachable once the response is written: a resume lands on a different engine and replays the call instead of resolving the original fiber. The MCP session Durable Object does not go through this middleware — it builds its stack over a session-lifetime handle — so its pauses still survive between requests, which is the point of that plane.
shutdownis a required member rather than an optional one. Two production decorators rebuild the engine as a fresh object literal, and an optional member would have let them drop it silently, reopening the race with nothing to show for it. Making it required turned that into a compile error and surfaced both.This does not touch the storage error classification added earlier. These faults should stop happening, not stop being reported.
Testing
bun run typecheckgreen across all packages;lintandformat:checkclean.shutdownends it, and thatshutdowndoes not block the request it runs in. Verified red before the fix — with the interrupt removed it fails on "shutdown interrupted the parked sandbox fiber: expected false to be true" — and green after. An earlier version of this test passed against the broken code because it only asserted bookkeeping; it was rewritten to observe the fiber itself.