From 4794be385473ebb9ad61a8bc5312feb36273032f Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Fri, 10 Apr 2026 20:00:00 +0530 Subject: [PATCH 1/4] fix(execution): fork sandbox as daemon so pause/resume survives runPromise boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The execution engine's `startPausableExecution` forked the sandbox fiber with `Effect.fork`, which attaches it to the current fiber's `Local` FiberScope. Every host that drives `executeWithPause` and `resume` from separate contexts (the HTTP API handlers, CLI `executor call` + `executor resume`, and any host that pauses across an async boundary) runs each call in its own top-level `Effect.runPromise`. When the first runPromise's root fiber exits to return the paused result, its `FiberRuntime.evaluateEffect` calls `interruptAllChildren()` and interrupts the sandbox before the caller ever sees the response. The subsequent `resume` then races `Fiber.join(paused.fiber)` — which returns the interrupt exit immediately (converted to a defect by `Effect.orDie` in `awaitCompletionOrPause`) — against `Deferred.await(nextSignal)`. `Effect.race`'s `onSelfDone` on failure waits for the loser, but nothing ever signals the next pause Deferred: the tool's continuation runs on a separate root fiber spawned by the quickjs sandbox bridge for each tool call, and once that completes with no further elicitations, `nextSignal` is never filled. The resume call hangs forever. Crucially, the underlying HTTP side effect of the tool can still succeed in this window — producing "phantom writes" where the upstream observes the mutation but the caller never sees the response. `Effect.forkDaemon` attaches the sandbox fiber to the global FiberScope instead of the parent's children set, so the parent's `interruptAllChildren` on exit does not touch it. FiberRefs and Context are copied at fork time via `unsafeMakeChildFiber`, so the daemon is fully independent of the driving runPromise's lifetime. Adds a regression test (`"resume returns across separate runPromise boundaries for a single-elicit tool (HTTP-like)"`) that uses plain `it(async () => ...)` so each engine call is its own top-level `runPromise`, matching the HTTP handler shape. The test runs against a new single-elicit `singleApproval` tool that mirrors the Gmail `labels.create`-style "one approval, one side effect, one response" shape. The existing `multiApproval`-based `it.effect` test masks the bug because both calls share one runPromise scope AND the second elicit eventually fills `nextSignal`, letting the race complete even with a dead sandbox fiber. --- packages/core/execution/src/engine.ts | 33 +++++- .../core/execution/src/tool-invoker.test.ts | 109 +++++++++++++++++- 2 files changed, 140 insertions(+), 2 deletions(-) diff --git a/packages/core/execution/src/engine.ts b/packages/core/execution/src/engine.ts index 20350b0da5..64286a4fd9 100644 --- a/packages/core/execution/src/engine.ts +++ b/packages/core/execution/src/engine.ts @@ -320,6 +320,37 @@ export const createExecutionEngine = (config: ExecutionEngineConfig): ExecutionE * Start an execution in the pause/resume mode. Forks the sandbox * onto its own fiber and waits for either completion or the first * elicitation pause. + * + * The sandbox fiber MUST outlive the outer `runPromise` that drives this + * effect. Over HTTP (and any host that drives `executeWithPause` and + * `resume` from separate contexts) each call runs in its own top-level + * `Effect.runPromise`. If we used `Effect.fork`, the sandbox fiber would + * attach to the first runPromise's root fiber via a `Local` `FiberScope`, + * and that root fiber's `FiberRuntime.evaluateEffect` would call + * `interruptAllChildren()` on exit — sending an interrupt signal to the + * sandbox before the pause result is even returned to the caller. + * + * The subsequent `resume` would then race `Fiber.join(paused.fiber)` + * against `Deferred.await(nextSignal)`. With a dead sandbox, join returns + * the interrupt exit (converted to a defect by `Effect.orDie` in + * `awaitCompletionOrPause`). `Effect.race`'s `onSelfDone` on failure + * waits for the loser, but nothing ever signals the next pause Deferred: + * the tool's continuation runs on a SEPARATE root fiber that the quickjs + * sandbox bridge spawns per tool call via its own `Effect.runPromise` + * (see `__executor_invokeTool` in `@executor/runtime-quickjs`), and once + * that completes with no further elicitations, `nextSignal` is never + * filled. The resume HTTP call hangs forever. The underlying HTTP side + * effect of the tool can still succeed in this window — producing + * "phantom writes" where the upstream observes the mutation but the + * caller never sees the response. + * + * `Effect.forkDaemon` attaches the sandbox fiber to the global + * `FiberScope` instead of the parent's children set, so the parent's + * `interruptAllChildren` on exit does not touch it. FiberRefs and + * Context are copied at fork time, so the daemon is fully independent + * of the driving runPromise's lifetime. Regression test: `"resume + * returns across separate runPromise boundaries for a single-elicit + * tool"` in `tool-invoker.test.ts`. */ const startPausableExecution = (code: string): Effect.Effect => Effect.gen(function* () { @@ -353,7 +384,7 @@ export const createExecutionEngine = (config: ExecutionEngineConfig): ExecutionE }); const invoker = makeFullInvoker(executor, { onElicitation: elicitationHandler }); - fiber = yield* Effect.fork(codeExecutor.execute(code, invoker)); + fiber = yield* Effect.forkDaemon(codeExecutor.execute(code, invoker)); const initialSignal = yield* Ref.get(pauseSignalRef); return yield* awaitCompletionOrPause(fiber, initialSignal); diff --git a/packages/core/execution/src/tool-invoker.test.ts b/packages/core/execution/src/tool-invoker.test.ts index 1dd0cb6d75..7870dbe477 100644 --- a/packages/core/execution/src/tool-invoker.test.ts +++ b/packages/core/execution/src/tool-invoker.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "@effect/vitest"; -import { Effect, Schema } from "effect"; +import { Effect, Fiber, Schema } from "effect"; import { ElicitationResponse, @@ -293,6 +293,22 @@ describe("pause/resume with multiple elicitations", () => { return { first: r1, second: r2 }; }), }), + tool({ + name: "singleApproval", + description: + "A tool that elicits exactly once and then returns a value. Mirrors the shape of a typical `gmail.users.labels.create` style operation: one approval, one side effect, one success response.", + inputSchema: EmptyInput, + handler: (_args, ctx) => + Effect.gen(function* () { + const r = yield* ctx.elicit( + new FormElicitation({ + message: "Only approval", + requestedSchema: {}, + }), + ); + return { ok: true, response: r }; + }), + }), ], }), ] as const, @@ -346,4 +362,95 @@ describe("pause/resume with multiple elicitations", () => { }), { timeout: 10000 }, ); + + // Regression test for a fiber-scoping bug that the `it.effect` test above + // does NOT catch, for two reasons stacked on each other: + // + // 1. `it.effect` wraps the whole body in a single `Effect.gen` → single + // `Effect.runPromise`, so the first call's root fiber is still alive + // when resume runs. The HTTP API (and any host that drives + // `executeWithPause` and `resume` from separate contexts) runs each + // call in its own top-level `runPromise`. + // + // 2. `multiApproval` elicits twice. If the sandbox fiber were attached + // to the first runPromise's scope (via `Effect.fork`), it would be + // interrupted between the two calls. The resume's + // `awaitCompletionOrPause` would then race a dead `Fiber.join` + // against `Deferred.await(nextSignal)`. With a double-elicit tool + // the second elicit eventually fills `nextSignal` (from the invoker + // fiber spawned on a separate root by the quickjs sandbox bridge) + // and the race completes — hiding the bug. + // + // A single-elicit tool (matching the Gmail shape) has nothing to produce + // a second pause signal, so with `Effect.fork` the resume hangs forever + // waiting on a Deferred that will never be filled. The fix is to use + // `Effect.forkDaemon` at the fork site in engine.ts; see the JSDoc on + // `startPausableExecution` for the full trace. + it( + "resume returns across separate runPromise boundaries for a single-elicit tool (HTTP-like)", + async () => { + const executor = await Effect.runPromise(makeElicitingExecutor()); + const engine = createExecutionEngine({ executor }); + + const code = "return await tools.api.singleApproval({});"; + + // First call — own runPromise. Sandbox fiber is forked here. + const outcome1 = await engine.executeWithPause(code); + expect(outcome1.status).toBe("paused"); + const paused1 = outcome1 as Extract; + expect(paused1.execution.elicitationContext.request.message).toBe("Only approval"); + + // Assert the sandbox fiber is still alive across the runPromise + // boundary. Under `Effect.fork`, it would already be `Done` with an + // `Interrupt` exit cause here (its parent's `interruptAllChildren()` + // ran on exit). Under `Effect.forkDaemon`, it is attached to the + // global FiberScope and remains suspended on the response Deferred + // inside the elicitation handler. + // + // `execution.fiber` is on the internal paused shape, not the public + // `PausedExecution` type exported from the engine — cast to read it. + const sandboxFiber = ( + paused1.execution as unknown as { + readonly fiber: Fiber.Fiber; + } + ).fiber; + const exitProbe = await Effect.runPromise( + Effect.race( + Fiber.await(sandboxFiber), + Effect.map(Effect.sleep("50 millis"), () => "still-running" as const), + ), + ); + expect(exitProbe).toBe("still-running"); + + // Second call — another top-level runPromise. With the tool only + // eliciting once, the only way this can return is for `Fiber.join` + // inside `awaitCompletionOrPause` to see a CLEAN completion from the + // sandbox fiber. That requires the sandbox fiber to still be alive + // going into resume. + const outcome2 = await Promise.race([ + engine.resume(paused1.execution.id, { action: "accept" }), + new Promise((_, reject) => + setTimeout( + () => + reject( + new Error( + "resume hung across runPromise boundaries — sandbox fiber was interrupted by the first runPromise's exit. Fix: use Effect.forkDaemon in startPausableExecution.", + ), + ), + 2000, + ), + ), + ]); + + expect(outcome2).not.toBeNull(); + const resumed = outcome2 as NonNullable; + // Single-elicit tool completes after one approval. + expect(resumed.status).toBe("completed"); + if (resumed.status === "completed") { + expect(resumed.result.error).toBeUndefined(); + expect(resumed.result.result).toMatchObject({ ok: true }); + } + }, + 10000, + ); }); From f4f333e7f8054d19f39f1093b9b6008651273434 Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Fri, 10 Apr 2026 20:27:43 +0530 Subject: [PATCH 2/4] fix(local): disable Bun.serve idleTimeout so interactive MCP elicitation round-trips can complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bun.serve's default idleTimeout is 10 seconds (see `src/bun.js/api/server/ServerConfig.zig:26`). That is too short for the executor's HTTP endpoints: - The MCP streamable-HTTP transport on `/mcp` keeps the TCP socket open for the duration of a tool call. When a non-SAFE operation trips the `requiresApproval` annotation in the google-discovery / openapi / graphql plugins, the executor's elicitation handler calls `server.server.elicitInput(...)`, which sends an `elicitation/create` request to the MCP client and awaits its response. For clients that surface an approval UI to a human operator, the full round-trip — render prompt, read it, click, serialize the response, send it back over the socket — routinely takes 5–10 seconds. Bun's 10s idle timeout was closing the socket mid-flight, either: (a) before the response returned — the tool call appeared to hang and the caller saw a transport error; or (b) after the sandbox had already received `{ action: "accept" }` and the underlying HTTP call to the upstream API had gone through — producing "phantom writes" where the upstream observed the mutation but the caller never saw the response. Both failure modes were reproduced against Gmail's `labels.create` during debugging. - The executor's own `/api/executions` + `/api/executions/:id/resume` pause/resume flow also benefits: long-running sandboxed computations (e.g. large batch operations driven from a `tools.search` loop) can legitimately idle the socket beyond 10 seconds while awaiting tool continuations. `idleTimeout: 0` disables the idle timeout entirely. This is a documented behavior of Bun's uSockets: `us_socket_timeout` in `packages/bun-usockets/src/socket.c:86-92` sets the internal `s->timeout` field to the sentinel `255` — which never fires — when `seconds == 0`. `idleTimeout: 0` is therefore the correct "no timeout" signal, not a "close immediately" bug. The executor's MCP host and HTTP API clients are responsible for enforcing their own per-request timeouts (MCP SDK's `DEFAULT_REQUEST_TIMEOUT_MSEC = 60000`, and any caller-side timeouts). Disabling the Bun-level idle timeout only removes a racing cutoff that was shorter than those logical timeouts and shorter than a realistic human approval cycle. --- apps/local/src/serve.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apps/local/src/serve.ts b/apps/local/src/serve.ts index 7ee956e455..ccc88e66a2 100644 --- a/apps/local/src/serve.ts +++ b/apps/local/src/serve.ts @@ -106,6 +106,17 @@ export async function startServer(opts: StartServerOptions = {}): Promise Date: Fri, 10 Apr 2026 20:52:50 +0530 Subject: [PATCH 3/4] chore(execution,local): trim verbose narrative comments from fix commits --- apps/local/src/serve.ts | 14 ++--- packages/core/execution/src/engine.ts | 41 +++----------- .../core/execution/src/tool-invoker.test.ts | 53 +++---------------- 3 files changed, 19 insertions(+), 89 deletions(-) diff --git a/apps/local/src/serve.ts b/apps/local/src/serve.ts index ccc88e66a2..913e9e4e29 100644 --- a/apps/local/src/serve.ts +++ b/apps/local/src/serve.ts @@ -106,16 +106,10 @@ export async function startServer(opts: StartServerOptions = {}): Promise => Effect.gen(function* () { diff --git a/packages/core/execution/src/tool-invoker.test.ts b/packages/core/execution/src/tool-invoker.test.ts index 7870dbe477..cd8106f9fb 100644 --- a/packages/core/execution/src/tool-invoker.test.ts +++ b/packages/core/execution/src/tool-invoker.test.ts @@ -363,29 +363,11 @@ describe("pause/resume with multiple elicitations", () => { { timeout: 10000 }, ); - // Regression test for a fiber-scoping bug that the `it.effect` test above - // does NOT catch, for two reasons stacked on each other: - // - // 1. `it.effect` wraps the whole body in a single `Effect.gen` → single - // `Effect.runPromise`, so the first call's root fiber is still alive - // when resume runs. The HTTP API (and any host that drives - // `executeWithPause` and `resume` from separate contexts) runs each - // call in its own top-level `runPromise`. - // - // 2. `multiApproval` elicits twice. If the sandbox fiber were attached - // to the first runPromise's scope (via `Effect.fork`), it would be - // interrupted between the two calls. The resume's - // `awaitCompletionOrPause` would then race a dead `Fiber.join` - // against `Deferred.await(nextSignal)`. With a double-elicit tool - // the second elicit eventually fills `nextSignal` (from the invoker - // fiber spawned on a separate root by the quickjs sandbox bridge) - // and the race completes — hiding the bug. - // - // A single-elicit tool (matching the Gmail shape) has nothing to produce - // a second pause signal, so with `Effect.fork` the resume hangs forever - // waiting on a Deferred that will never be filled. The fix is to use - // `Effect.forkDaemon` at the fork site in engine.ts; see the JSDoc on - // `startPausableExecution` for the full trace. + // Regression: each engine call must run in its own top-level + // `runPromise` to reproduce the HTTP shape. Uses a single-elicit tool so + // nothing can accidentally unstick a dead-fiber race via a second + // elicitation (which is what masks the bug in the `multiApproval` test + // above). it( "resume returns across separate runPromise boundaries for a single-elicit tool (HTTP-like)", async () => { @@ -394,21 +376,13 @@ describe("pause/resume with multiple elicitations", () => { const code = "return await tools.api.singleApproval({});"; - // First call — own runPromise. Sandbox fiber is forked here. const outcome1 = await engine.executeWithPause(code); expect(outcome1.status).toBe("paused"); const paused1 = outcome1 as Extract; expect(paused1.execution.elicitationContext.request.message).toBe("Only approval"); - // Assert the sandbox fiber is still alive across the runPromise - // boundary. Under `Effect.fork`, it would already be `Done` with an - // `Interrupt` exit cause here (its parent's `interruptAllChildren()` - // ran on exit). Under `Effect.forkDaemon`, it is attached to the - // global FiberScope and remains suspended on the response Deferred - // inside the elicitation handler. - // - // `execution.fiber` is on the internal paused shape, not the public - // `PausedExecution` type exported from the engine — cast to read it. + // `execution.fiber` is on `InternalPausedExecution`; the exported + // `PausedExecution` type doesn't carry it. Cast to read. const sandboxFiber = ( paused1.execution as unknown as { readonly fiber: Fiber.Fiber; @@ -422,21 +396,11 @@ describe("pause/resume with multiple elicitations", () => { ); expect(exitProbe).toBe("still-running"); - // Second call — another top-level runPromise. With the tool only - // eliciting once, the only way this can return is for `Fiber.join` - // inside `awaitCompletionOrPause` to see a CLEAN completion from the - // sandbox fiber. That requires the sandbox fiber to still be alive - // going into resume. const outcome2 = await Promise.race([ engine.resume(paused1.execution.id, { action: "accept" }), new Promise((_, reject) => setTimeout( - () => - reject( - new Error( - "resume hung across runPromise boundaries — sandbox fiber was interrupted by the first runPromise's exit. Fix: use Effect.forkDaemon in startPausableExecution.", - ), - ), + () => reject(new Error("resume hung across runPromise boundaries")), 2000, ), ), @@ -444,7 +408,6 @@ describe("pause/resume with multiple elicitations", () => { expect(outcome2).not.toBeNull(); const resumed = outcome2 as NonNullable; - // Single-elicit tool completes after one approval. expect(resumed.status).toBe("completed"); if (resumed.status === "completed") { expect(resumed.result.error).toBeUndefined(); From 2178343541819a07e55408b82f21dacb4dea8ac0 Mon Sep 17 00:00:00 2001 From: Saatvik Arya Date: Fri, 10 Apr 2026 21:06:52 +0530 Subject: [PATCH 4/4] docs: tighten fix rationale comments --- apps/local/src/serve.ts | 6 ++---- packages/core/execution/src/engine.ts | 11 ++++------- packages/core/execution/src/tool-invoker.test.ts | 8 +++----- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/apps/local/src/serve.ts b/apps/local/src/serve.ts index 913e9e4e29..c634a45f87 100644 --- a/apps/local/src/serve.ts +++ b/apps/local/src/serve.ts @@ -106,10 +106,8 @@ export async function startServer(opts: StartServerOptions = {}): Promise => Effect.gen(function* () { diff --git a/packages/core/execution/src/tool-invoker.test.ts b/packages/core/execution/src/tool-invoker.test.ts index cd8106f9fb..b9b752f243 100644 --- a/packages/core/execution/src/tool-invoker.test.ts +++ b/packages/core/execution/src/tool-invoker.test.ts @@ -363,11 +363,9 @@ describe("pause/resume with multiple elicitations", () => { { timeout: 10000 }, ); - // Regression: each engine call must run in its own top-level - // `runPromise` to reproduce the HTTP shape. Uses a single-elicit tool so - // nothing can accidentally unstick a dead-fiber race via a second - // elicitation (which is what masks the bug in the `multiApproval` test - // above). + // Regression: use separate top-level runPromise calls to match HTTP/CLI + // pause/resume, and a single-elicit tool so no later pause can mask a dead + // sandbox fiber. it( "resume returns across separate runPromise boundaries for a single-elicit tool (HTTP-like)", async () => {