From c7f4bc1300d542803aff2935affb3110095026ea Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sun, 23 Aug 2026 16:55:23 -0700 Subject: [PATCH] fix: survive transient sandbox exec failures in the callback bridge worker (#12052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents on remote sandbox targets reach the Paperclip API through the sandbox callback bridge: a loopback gateway inside the sandbox writes request files, and a host-side worker polls them over the provider's exec channel and forwards them to the server > - The worker's poll loop had one terminal catch: a single reset or slow exec ended the relay for the rest of the run > - The in-sandbox gateway kept queueing requests against the dead worker, so every later API call from the agent stranded, including its final status write > - A relay that dies on one transient fault turns a routine provider hiccup into a lost issue disposition > - This pull request restructures the loop so transient faults back off and retry, while the watchdog remains the escalation path for sustained outages > - The benefit is that one flaky exec no longer severs an agent from the control plane mid-run ## Linked Issues or Issue Description Refs #9904. Refs #8977. Both touch adjacent bridge behavior (curl shim, header forwarding); neither addresses worker-loop lifetime. No public issue exists for this defect. The description below follows the bug report template. **What happened?** During a staging run, the host-side bridge worker hit one failed sandbox exec while relaying requests. The poll loop's only catch is terminal: it failed the pending requests and set the worker to settled, with no restart. The agent's later API calls saw connection-level failures or bridge errors until the run ended. Its final `PATCH status: done` was lost, and the missing-disposition recovery had to repair the issue in a corrective run. **Expected behavior** One transient exec failure must not end the relay for the rest of the run. The worker must back off and retry. A sustained outage must still fail queued requests fast through the watchdog. In-flight request semantics (abort plus 504 backstop, retry-safe 503) must not change. **Steps to reproduce** 1. Start a remote-sandbox run and let the provider exec channel reject or stall one call while the bridge worker polls. 2. The worker hits one `listJsonFiles` failure or one request-attempt timeout, and the loop exits through its terminal catch. 3. Every later bridge request strands. The loopback gateway keeps accepting requests that never complete, and after 64 queued files it answers every request with 503 until run end. ## What Changed - `startSandboxCallbackBridgeWorker`'s poll loop now separates three failure domains: - A failed poll backs off exponentially (capped at 5 s) and retries instead of dying. The first failure of a streak still lands on the run trace through the workerFailed span; later repeats only warn. - A failed or hung request attempt runs the same recovery pass the loop previously died on — abort the in-flight handler (its 504 backstop keeps the caller from stranding) and 503 the unclaimed queued requests — and the loop then continues and serves the caller's retry. - A listing where every file already has an in-flight attempt sleeps one poll interval, like an empty listing. The previous immediate re-list was a hot spin: an exec storm against a real channel, and a pure-microtask loop that starved every timer in the process when the queue client resolves synchronously. - The watchdog, the claim/finalize fences, and the stop/drain semantics are unchanged. ## Verification - `npx vitest run packages/adapter-utils/src/sandbox-callback-bridge.test.ts` — 38 passed. - New regression test: a request queued behind three consecutive poll failures is still delivered. - Updated tests: the stalled-poll test now expects recovery (the handler's real 200) instead of a terminal 503; the sustained-outage 503 path remains proven by the dedicated watchdog test; the recovery-503-write-retry test triggers the recovery pass through a hung request read, because a hung poll no longer runs that pass. - `pnpm --filter @paperclipai/adapter-utils typecheck`. ## Risks - Behavior change: a transiently failing poll no longer mass-fails queued requests on the first error. Callers wait through the backoff window, bounded by the existing in-sandbox 30 s response deadline, or the watchdog fails them after 20 s of no successful iteration. This trades fast-but-terminal degradation for recovery. - A hard-down channel now retries every ≤5 s for the rest of the run instead of stopping. Each retry is one exec attempt against a channel that already fails. - In-flight mutation safety is unchanged: the guard map and the claim protocol still prevent a double-applied host mutation, and a guarded file is always finalized by its own attempt or by its 504 backstop. ## Model Used - Claude Fable 5 (Anthropic), model id `claude-fable-5`, extended thinking enabled, agentic tool use via Claude Code (CLI harness), 200k context window. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- .../src/sandbox-callback-bridge.test.ts | 114 ++++++++++++++---- .../src/sandbox-callback-bridge.ts | 83 +++++++++++-- 2 files changed, 164 insertions(+), 33 deletions(-) diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts index b0300d6d7d..a3140e9b64 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts @@ -475,6 +475,68 @@ describe("sandbox callback bridge", () => { } }); + it("recovers from transient queue polling failures and keeps relaying requests", async () => { + // A single reset or slow sandbox exec used to unwind the poll loop into its + // terminal catch, killing the relay for the rest of the run. The loop must + // instead back off, retry, and still deliver requests queued after the + // failure window. + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-transient-")); + cleanupDirs.push(rootDir); + + const queueDir = path.posix.join(rootDir, "queue"); + const directories = sandboxCallbackBridgeDirectories(queueDir); + const baseClient = createFileSystemSandboxCallbackBridgeQueueClient(); + let listCalls = 0; + const client: SandboxCallbackBridgeQueueClient = { + ...baseClient, + listJsonFiles: async (dirPath: string) => { + listCalls += 1; + if (listCalls <= 3) { + throw new Error("list requests failed: kex_exchange_identification: read: Connection reset by peer"); + } + return baseClient.listJsonFiles(dirPath); + }, + }; + + const seenPaths: string[] = []; + const worker = await startSandboxCallbackBridgeWorker({ + client, + queueDir, + authorizeRequest: async () => null, + handleRequest: async (request) => { + seenPaths.push(request.path); + return { + status: 200, + headers: { "content-type": "application/json" }, + body: JSON.stringify({ ok: true }), + }; + }, + }); + cleanupFns.push(async () => { + await worker.stop(); + }); + + const requestId = "transient-recovery-1"; + await writeFile( + path.join(directories.requestsDir, `${requestId}.json`), + JSON.stringify({ + id: requestId, + method: "GET", + path: "/api/agents/me", + query: "", + headers: {}, + body: "", + }), + "utf8", + ); + + const responseFile = await waitForJsonFile(directories.responsesDir, 10_000); + const raw = await readFile(path.join(directories.responsesDir, responseFile), "utf8"); + expect(JSON.parse(raw)).toMatchObject({ id: requestId, status: 200 }); + expect(seenPaths).toEqual(["/api/agents/me"]); + expect(listCalls).toBeGreaterThan(3); + }); + it("keeps the queue-directory setup on the startup step but resets the poll loop store", async () => { // The worker starts inside the measured `bridge.paperclip` step. Its awaited // queue-directory setup is startup work, so a `makeDir` `sandbox.exec` span @@ -1510,7 +1572,7 @@ describe("sandbox callback bridge", () => { })}\n`; } - it("times out a stalled poll, writes a 503, and surfaces a run-level error", async () => { + it("times out a stalled poll, surfaces a run-level error, and recovers to deliver the request", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-hang-")); cleanupDirs.push(rootDir); @@ -1526,9 +1588,11 @@ describe("sandbox callback bridge", () => { const client: SandboxCallbackBridgeQueueClient = { ...base, // The first poll never resolves — a silently unresponsive sandbox channel. - // The per-iteration timeout must convert the hang into a caught error. The - // request never reaches the handler, so the recovery path can safely 503 - // it. Later calls (the recovery's failPendingRequests) resolve. + // The per-iteration timeout must convert the hang into a caught error, and + // the loop must then back off and retry rather than die: the request never + // reached the handler, so the retry delivers the real response. A + // sustained outage is the watchdog's job (proven separately below), not a + // reason to fail a request one transient hang could still serve. listJsonFiles: async (dir) => { listCalls += 1; if (listCalls === 1) { @@ -1550,7 +1614,7 @@ describe("sandbox callback bridge", () => { const responseFile = await waitForJsonFile(directories.responsesDir, 3_000); const responseBody = await readFile(path.posix.join(directories.responsesDir, responseFile), "utf8"); - expect(JSON.parse(responseBody).status).toBe(503); + expect(JSON.parse(responseBody).status).toBe(200); expect(workerErrors.length).toBeGreaterThan(0); expect(workerErrors[0]).toContain("timed out"); @@ -2261,10 +2325,13 @@ describe("sandbox callback bridge", () => { }); it("retries a recovery 503 write that fails transiently, delivers the 503, and removes the request", async () => { - // The poll times out, so the recovery path aborts the queued request with a - // 503. The first 503 write fails, so the recovery must retry it inside the - // same pass. It then delivers the 503 and removes the request. The request is - // unclaimed, so its host mutation never ran; the 503 stays retry-safe. + // A request attempt times out (its first read hangs), so the loop's request + // catch runs the recovery pass, which aborts the queued request with a 503. + // The first 503 write fails, so the recovery must retry it inside the same + // pass. It then delivers the 503 and removes the request. The request is + // unclaimed, so its host mutation never ran; the 503 stays retry-safe. (A + // hung poll no longer triggers this pass — the loop backs off and retries + // the poll instead, and a sustained hang is the watchdog's job.) const waitFor = async (predicate: () => boolean, timeoutMs: number) => { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { @@ -2286,27 +2353,26 @@ describe("sandbox callback bridge", () => { requestBodies.set(requestPath, bridgeRequestJson("req-503-retry")); const responseWrites: Array<{ path: string; status: number; body: string }> = []; const requestRemovals: string[] = []; - let listCalls = 0; + let readCalls = 0; let writeAttempts = 0; const client: SandboxCallbackBridgeQueueClient = { makeDir: async () => {}, makeDirs: async () => {}, - // The first poll never resolves — a silently unresponsive sandbox channel. - // The per-iteration timeout converts the hang into a caught error, so the - // loop `catch` runs the recovery path. Later listings resolve, so the - // recovery enumerates and aborts the request. - listJsonFiles: async (dir) => { - if (dir !== directories.requestsDir) { - return []; - } - listCalls += 1; - if (listCalls === 1) { - return await new Promise(() => {}); - } - return [...requestBodies.keys()].map((entry) => path.posix.basename(entry)).sort(); - }, + listJsonFiles: async (dir) => + dir === directories.requestsDir + ? [...requestBodies.keys()].map((entry) => path.posix.basename(entry)).sort() + : [], + // The first read never resolves — a silently unresponsive sandbox channel + // hit mid-request, before the handler claim. The per-iteration timeout + // converts the hang into a caught error, so the loop's request catch runs + // the recovery pass. The recovery's own read resolves, so it can build and + // deliver the 503. readTextFile: async (remotePath) => { + readCalls += 1; + if (readCalls === 1) { + return await new Promise(() => {}); + } const body = requestBodies.get(remotePath); if (body === undefined) { throw new Error(`missing request ${remotePath}`); diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.ts b/packages/adapter-utils/src/sandbox-callback-bridge.ts index 5d8907ce04..b198d6d9cb 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.ts @@ -34,7 +34,8 @@ const DEFAULT_BRIDGE_MAX_DUPLEX_DECODER_BYTES = 8 * 1024 * 1024; // (PAPERCLIP_BRIDGE_RESPONSE_TIMEOUT_MS), so the host loop fails fast and writes // 503 responses before the in-sandbox client gives up. A silently unresponsive // sandbox channel makes a client call hang with no reject; this timeout turns -// that hang into a caught error, so the loop `catch` runs `failPendingRequests`. +// that hang into a caught error, so the poll loop can back off and retry while +// the watchdog below decides when to fail the queued requests. const DEFAULT_BRIDGE_ITERATION_TIMEOUT_MS = 10_000; // Watchdog backstop for a hang that the per-iteration timeout does not catch // (for example many slow-but-under-timeout calls, or a stall outside the awaited @@ -58,6 +59,12 @@ const MAX_BACKSTOP_WRITE_ATTEMPTS = 3; // The delay between two 504 backstop write attempts. It is short, so all retries // finish well under the in-sandbox 30s response deadline. const BACKSTOP_WRITE_RETRY_MS = 50; +// Backoff cap between poll-loop retries after a transient iteration failure. +// The cap keeps a recovering loop probing often enough to resume before the +// in-sandbox 30s response deadline strands queued callers, while the +// exponential ramp below it keeps a hard-down channel from burning an exec +// call every poll interval. +const MAX_TRANSIENT_ITERATION_BACKOFF_MS = 5_000; const REMOTE_WRITE_BASE64_CHUNK_SIZE = 32 * 1024; export const SANDBOX_CALLBACK_BRIDGE_ENTRYPOINT = "paperclip-bridge-server.mjs"; const SANDBOX_EXEC_CHANNEL_ENV = "PAPERCLIP_SANDBOX_EXEC_CHANNEL"; @@ -1380,13 +1387,54 @@ export async function startSandboxCallbackBridgeWorker(input: { watchdogTimer.unref(); } try { + // Consecutive transient poll failures. A single failed list call — one + // reset or slow sandbox exec — must not end the relay for the rest of the + // run: the in-sandbox gateway keeps queueing requests, so a dead loop + // strands every later API call from the agent (its status writes then look + // like connection failures and the issue loses its disposition). Back off + // and retry the poll instead. The watchdog stays the escalation path for a + // sustained outage — it fires after `watchdogTimeoutMs` without a + // successful iteration and fails the queued requests fast, while this loop + // keeps probing for recovery. + let consecutivePollFailures = 0; while (true) { - const fileNames = await withTimeout( - input.client.listJsonFiles(directories.requestsDir), - iterationTimeoutMs, - "Sandbox callback bridge list requests", - ); - if (fileNames.length === 0) { + let fileNames: string[]; + try { + fileNames = await withTimeout( + input.client.listJsonFiles(directories.requestsDir), + iterationTimeoutMs, + "Sandbox callback bridge list requests", + ); + consecutivePollFailures = 0; + } catch (error) { + if (stopping) { + break; + } + consecutivePollFailures += 1; + const message = `${buildWorkerFailureMessage(error)} (transient poll failure ${consecutivePollFailures}; retrying)`; + if (consecutivePollFailures === 1) { + // Put the first failure of a streak on the run trace; later repeats + // only warn, so a flapping channel does not spam failed spans. + await surfaceRunError(new Error(message)); + } else { + console.warn(`[paperclip] ${message}`); + } + const backoffMs = Math.min( + pollIntervalMs * 2 ** consecutivePollFailures, + MAX_TRANSIENT_ITERATION_BACKOFF_MS, + ); + await new Promise((resolve) => setTimeout(resolve, backoffMs)); + continue; + } + // A file whose attempt is still in flight (or waiting on its 504 + // backstop) is not actionable: `processRequestFile` would skip it via + // the guard map. Treat an all-guarded listing like an empty one and + // sleep a poll interval. Re-listing immediately would spin the loop — + // an exec storm against a real sandbox channel, and with an in-memory + // client a pure-microtask loop that starves every timer in the process + // (including the guard's own backstop and abort timers). + const actionableFileNames = fileNames.filter((fileName) => !inFlightRequestGuards.has(fileName)); + if (actionableFileNames.length === 0) { lastSuccessfulIterationAt = Date.now(); if (stopping) { break; @@ -1394,7 +1442,7 @@ export async function startSandboxCallbackBridgeWorker(input: { await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); continue; } - for (const fileName of fileNames) { + for (const fileName of actionableFileNames) { if (stopping && Date.now() >= stopDeadline) break; inFlight += 1; try { @@ -1406,7 +1454,7 @@ export async function startSandboxCallbackBridgeWorker(input: { // `task.run` after it. Without a runner, the request runs under the // run parent with no wrapper span, exactly like the earlier behavior. // The per-iteration timeout wraps the whole request, so a hung - // request rejects and the loop `catch` runs `failPendingRequests`. + // request rejects and the catch below runs the recovery pass. await withTimeout( input.runtimeSpan ? input.runtimeSpan(CALLBACK_BRIDGE_RELAY_REQUEST_SPAN, () => @@ -1419,6 +1467,23 @@ export async function startSandboxCallbackBridgeWorker(input: { `Sandbox callback bridge process request ${fileName}`, ); lastSuccessfulIterationAt = Date.now(); + } catch (error) { + // A single request attempt failed or hung. Run the same recovery + // pass the loop previously died on — abort the in-flight handler + // (its 504 backstop keeps the caller from stranding) and 503 the + // unclaimed queued requests — but keep the loop alive afterward. A + // caller that sees the retry-safe 503 re-queues, and the recovered + // loop serves the retry; the old terminal catch left every later + // request to strand instead. + const message = buildWorkerFailureMessage(error); + await surfaceRunError(new Error(message)); + try { + await failPendingRequests(message, { abandonInFlight: true }); + } catch (failPendingError) { + console.warn( + `[paperclip] sandbox callback bridge failed to abort queued requests after a request failure: ${failPendingError instanceof Error ? failPendingError.message : String(failPendingError)}`, + ); + } } finally { inFlight -= 1; }