From 22cea6b2e6aeb54b484ef095eab1378ecd0b4d79 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 2 Oct 2026 15:29:41 -0700 Subject: [PATCH] fix: bound sandbox bridge waits and flag silent runs sooner (#14979) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Sandbox agents exchange input and output through bridge control commands. > - A provider can stop responding to a command even when it receives a timeout. > - These small commands can inherit a four-hour agent lifetime and block input or teardown. > - The board also calls a silent run healthy for the first hour. > - This pull request bounds bridge control waits and surfaces silence sooner. ## Linked Issues or Issue Description **What happened?** A sandbox run can remain active when a bridge control command never returns. The shared helper passes a timeout to the provider but does not enforce it on the host. It also accepts the agent's hours-long timeout. Output silence remains `ok` for an hour and becomes `critical` only after four hours. **Expected behavior** Bound short bridge operations even if the provider never settles. Report failed input delivery through the existing shutdown path. Warn after five silent minutes and escalate after fifteen. Keep normal agent command limits and require verified termination before releasing execution ownership. **Steps to reproduce** 1. Use a sandbox runner whose bridge read or input-upload promise never settles. 2. Set its configured timeout to four hours. 3. Observe that the old queue client never returns or rejects. 4. Inspect a running task with 35 minutes of output silence. The old summary still reports `ok`. **Paperclip version or commit** Base commit `d6d88b9de2`. **Deployment mode** Self-hosted server with sandbox execution. **Agent adapter(s) involved** Shared command-managed sandbox bridge, including Codex ACP sessions. The informational silence thresholds apply to active runs across adapters. Related: #14889 recovers stalled Daytona output streams; #14485 retries explicit gateway failures during input delivery. This change bounds short control operations whose provider promises never settle. It does not add tool replay or automatic cancellation for output silence. #6297 proposes configurable per-agent silence thresholds; this patch only changes the existing defaults. ## What Changed - Enforce at most 30 seconds per bridge control shell command on the host and provider, including callback startup and shutdown, process-session launch, and payload setup. Preserve shorter configured deadlines and launch environments. - Keep the long-lived agent command outside this deadline. Use a fixed timeout diagnostic without command payloads. - Surface suspicious output silence after five minutes and critical silence after fifteen minutes. - Decouple the shared-workspace holder cutoff from warning thresholds and preserve its existing one-hour value. - Add regressions for hung reads, a late upload response, failed input delivery, exact warning boundaries, and fresh output clearing warnings. - Update the adapter guide and execution contract. ## Verification - The three new queue-client regressions fail on the unchanged base and pass with this patch. - Final callback bridge and sandbox session suites: 214 passed. These cover hung reads, writes, startup, shutdown, process-session launch, payload setup, and the separate long-running agent limit. - Stdin ordering and shutdown suite: 56 passed after the lifecycle change. - Daytona and watchdog coverage passed in the earlier focused runs. Across the focused suites, 602 distinct tests pass. - `pnpm -r typecheck` and `pnpm build`: passed. Server and adapter typecheck/build also passed after their respective follow-up changes. - `pnpm test:run`: attempted and stopped after known local failures. Four chat/email cases used an external ancestor skill path, three skill-cache cases failed on macOS, and one wakeup case timed out. The wakeup case passes alone (1 passed, 27 skipped). This run spanned the workspace-cutoff follow-up and also failed its new holder case; a fresh final-head workspace suite passes all 19 tests. The interrupted run is not a full local-suite pass or final-head verification. - A filesystem queue-drain test failed once during the lifecycle rerun and passed on the complete two-suite rerun. It uses the filesystem client, outside the changed command-runner path. - Complete CI on `ff2212c235`: 53 successful checks and two expected skips, including the full test suite and canary packaging dry run. No failed or pending checks. - Greptile reviewed `ff2212c235` at 5/5. All review findings are addressed, no threads remain unresolved, and the branch has no merge conflicts with `master`. - `git diff --check` and a scan of added text for secrets and private identifiers passed. ## Risks - A bridge control operation that needs more than 30 seconds now fails, even if the caller selected a longer run lifetime. Agent commands retain their own limits. - Timing out a provider promise does not cancel the remote operation or prove it stopped. Existing execution settlement still owns termination verification. No uncertain tool action is replayed. - Quiet healthy runs display warnings sooner. Existing snooze, continue, and false-positive dismissal controls still apply. Silence alone does not cancel a run, create review work, or change assignments. - No schema or API shape change. ## Model Used OpenAI GPT-6 through Codex, with tool use and code execution. The exact serving model ID and context window are not exposed in this session. ## 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 change-specific 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 --------- Co-authored-by: Paperclip --- doc/SPEC-implementation.md | 2 +- doc/execution-semantics.md | 2 +- packages/adapter-utils/README.md | 13 +++ .../src/execution-target-sandbox.test.ts | 60 +++++++++-- .../src/execution-target-stdin-race.test.ts | 3 + .../adapter-utils/src/execution-target.ts | 15 ++- .../src/sandbox-callback-bridge.test.ts | 99 +++++++++++++++++++ .../src/sandbox-callback-bridge.ts | 23 ++++- ...artbeat-active-run-output-watchdog.test.ts | 21 ++++ .../heartbeat-workspace-busy.test.ts | 4 +- server/src/services/heartbeat.ts | 23 ++--- server/src/services/recovery/service.ts | 4 +- 12 files changed, 225 insertions(+), 44 deletions(-) diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index e79ebaa327..f6bda971fa 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -548,7 +548,7 @@ V1 non-terminal liveness rule: - recovery-action ownership is separate from source-task ownership: automatic repair and board escalation preserve both source assignee fields; reassignment requires an explicit board decision or a policy-defined serious failure - source-scoped recovery routing is cause-keyed: bounded continuity and disposition repair may retry only the original agent; provider-quota failures create/reuse a scheduled wait-recovery monitor; every other exhausted or unsafe path creates/reuses a board-owned recovery action with `routingPolicy: board_escalation_no_takeover_v1` and no substitute-agent wake - legacy active agent-owned recovery actions remain readable, resolvable, and API-compatible after upgrade, but reconciliation does not enqueue another takeover wake for them -- active-run output silence is an informational board UI signal at one hour (`suspicious`) and four hours (`critical`); it does not create or update issues or recovery actions, comment on or block source work, change assignments, or wake an agent +- active-run output silence is an informational board UI signal at five minutes (`suspicious`) and fifteen minutes (`critical`); it does not create or update issues or recovery actions, comment on or block source work, change assignments, or wake an agent - board snooze and continue decisions suppress the run signal until their stored re-arm time; a false-positive decision suppresses it permanently for that run; open legacy evaluation issues remain readable and manually resolvable without automatic refresh Detailed ownership, execution, blocker, active-run watchdog, crash-recovery, and non-terminal liveness semantics are documented in `doc/execution-semantics.md`. diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index e6bd5fcf92..05b50745ec 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -841,7 +841,7 @@ An active run can still be unhealthy even when its process is `running`. Papercl The recovery service owns this contract: -- classify active-run output silence as `ok`, `suspicious`, `critical`, `snoozed`, or `not_applicable` +- classify active-run output silence as `ok`, `suspicious` after five minutes, `critical` after fifteen minutes, `snoozed`, or `not_applicable`; measure silence from the latest output, falling back to the run start when no output exists - honor active snooze and continue decisions on the run - permanently suppress the signal for a run after a `dismissed_false_positive` decision - build the `outputSilence` summary shown by live-run and active-run API responses diff --git a/packages/adapter-utils/README.md b/packages/adapter-utils/README.md index a51b3ebef9..b1e77d207d 100644 --- a/packages/adapter-utils/README.md +++ b/packages/adapter-utils/README.md @@ -9,6 +9,19 @@ For the adapter-author guide see [`docs/adapters/creating-an-adapter.md`](../../docs/adapters/creating-an-adapter.md) and the in-repo notes at [`packages/adapters/AUTHORING.md`](../adapters/AUTHORING.md). +## Sandbox bridge deadlines + +Command-managed bridge control operations (queue reads, input delivery, setup, +and cleanup) have a host-enforced deadline of at most 30 seconds per shell +command. A shorter configured timeout still applies. These operations cannot +inherit the agent run's hours-long lifetime or wait forever on a provider that +ignores its timeout. Long-lived agent session commands keep their own limits. + +A control timeout reports a transport failure. It does not prove that the +remote command stopped and does not replay an uncertain write. The process +session closes failed input delivery and uses its existing shutdown path; +normal execution settlement must still verify termination. + ## No-remote-git contract The local execution-workspace cwd is the only persistence boundary across diff --git a/packages/adapter-utils/src/execution-target-sandbox.test.ts b/packages/adapter-utils/src/execution-target-sandbox.test.ts index bf600fdd74..c1599a25d8 100644 --- a/packages/adapter-utils/src/execution-target-sandbox.test.ts +++ b/packages/adapter-utils/src/execution-target-sandbox.test.ts @@ -50,7 +50,7 @@ import { type StartupTracer, } from "./acpx-engine/startup-timing.js"; import { createSandboxRunLogTailFactory, type SandboxRunLogTailFactory } from "./sandbox-run-log-stream.js"; -import { runChildProcess } from "./server-utils.js"; +import { runChildProcess, type RunProcessResult } from "./server-utils.js"; import { shellQuote } from "./ssh.js"; import type { CommandManagedDuplexChannel } from "./command-managed-runtime.js"; import { @@ -1815,7 +1815,7 @@ describe("sandbox adapter execution targets", () => { ); const delegate = createLocalSandboxRunner(); - const execs: Array<{ useSession?: boolean; bypassSession?: boolean; script: string }> = []; + const execs: Array<{ useSession?: boolean; bypassSession?: boolean; timeoutMs?: number; script: string }> = []; const runner = { execute: vi.fn( async ( @@ -1827,6 +1827,7 @@ describe("sandbox adapter execution targets", () => { execs.push({ useSession: input.useSession, bypassSession: input.bypassSession, + timeoutMs: input.timeoutMs, script: input.args?.[1] ?? "", }); return delegate.execute(input); @@ -1851,7 +1852,7 @@ describe("sandbox adapter execution targets", () => { args: [childPath], cwd: rootDir, env: {}, - timeoutSec: 5, + timeoutSec: 4 * 60 * 60, onLog: async () => {}, streamOutputViaSession: true, }); @@ -1872,6 +1873,7 @@ describe("sandbox adapter execution targets", () => { const sessionExecs = execs.filter((exec) => exec.useSession === true); expect(sessionExecs).toHaveLength(1); expect(sessionExecs[0]!.bypassSession).not.toBe(true); + expect(sessionExecs[0]!.timeoutMs).toBe(4 * 60 * 60 * 1000); expect(sessionExecs[0]!.script).toContain("node "); // Every other exec is bridge control-plane plumbing. Each must force @@ -1881,6 +1883,7 @@ describe("sandbox adapter execution targets", () => { expect(controlExecs.length).toBeGreaterThan(0); for (const exec of controlExecs) { expect(exec.bypassSession).toBe(true); + expect(exec.timeoutMs).toBe(30_000); } } finally { await bridge?.stop(); @@ -1888,6 +1891,43 @@ describe("sandbox adapter execution targets", () => { }); }); + it.each(["launch", "payload setup"])("bounds a hung process-session %s", async (stage) => { + vi.useFakeTimers(); + try { + const runner = { + execute: vi.fn(async (input: { args?: string[] }) => { + const script = input.args?.[1] ?? ""; + if ((stage === "launch" && script.includes("nohup")) || + (stage === "payload setup" && script.startsWith("chmod 600"))) { + return new Promise(() => {}); + } + return { + exitCode: 0, signal: null, timedOut: false, stdout: '{"uploaded":true}', stderr: "", + pid: null, startedAt: new Date().toISOString(), + }; + }), + }; + let error: unknown; + const operation = startAdapterExecutionTargetProcessSessionBridge({ + runId: "run-hung-setup", + runtimeRootDir: "/workspace/runtime", + target: { kind: "remote", transport: "sandbox", remoteCwd: "/workspace", runner }, + adapterKey: "acpx", command: "cat", args: [], cwd: "/workspace", + env: stage === "payload setup" ? { LARGE_VALUE: "x".repeat(70_000) } : {}, + timeoutSec: 4 * 60 * 60, + }).catch((caught) => { error = caught; }); + await vi.advanceTimersByTimeAsync(30_000); + expect(error).toEqual(new Error("Sandbox bridge control command timed out after 30000ms.")); + await operation; + expect(runner.execute.mock.calls.filter(([input]) => input.args?.[1]?.includes("nohup"))) + .toHaveLength(stage === "launch" ? 1 : 0); + expect(runner.execute).toHaveBeenLastCalledWith(expect.objectContaining({ timeoutMs: 30_000 })); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + it("applies the remote sandbox fallback when adapter timeoutSec is unset", () => { const sandboxTarget: AdapterSandboxExecutionTarget = { kind: "remote", @@ -1896,9 +1936,8 @@ describe("sandbox adapter execution targets", () => { runner: createLocalSandboxRunner(), }; - // The sandbox default is a 4h wall-clock backstop matching the recovery - // watchdog critical threshold (ACTIVE_RUN_OUTPUT_CRITICAL_THRESHOLD_MS); - // the output-inactivity monitor remains the primary hang detector. + // The sandbox default stays at four hours independently of the earlier + // informational output-silence warnings and bridge control deadlines. expect(DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC).toBe(4 * 60 * 60); expect(resolveAdapterExecutionTargetTimeoutSec(sandboxTarget, 0)).toBe( DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC, @@ -2697,7 +2736,7 @@ describe("sandbox adapter execution targets", () => { } }); - it("uses the effective adapter timeout when starting the sandbox callback bridge", async () => { + it("bounds callback bridge operations independently of the adapter run timeout", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-execution-target-bridge-timeout-")); cleanupDirs.push(rootDir); const remoteCwd = path.join(rootDir, "workspace"); @@ -2744,9 +2783,10 @@ describe("sandbox adapter execution targets", () => { try { expect(bridge).not.toBeNull(); expect(runner.execute).toHaveBeenCalled(); - expect( - runner.execute.mock.calls.some(([input]) => input.timeoutMs === DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC * 1000), - ).toBe(true); + for (const [input] of runner.execute.mock.calls) { + expect(input.timeoutMs).toBeGreaterThan(0); + expect(input.timeoutMs).toBeLessThanOrEqual(30_000); + } } finally { await bridge?.stop(); await new Promise((resolve) => apiServer.close(() => resolve())); diff --git a/packages/adapter-utils/src/execution-target-stdin-race.test.ts b/packages/adapter-utils/src/execution-target-stdin-race.test.ts index a87deb0e89..b92e4eb7d2 100644 --- a/packages/adapter-utils/src/execution-target-stdin-race.test.ts +++ b/packages/adapter-utils/src/execution-target-stdin-race.test.ts @@ -591,6 +591,7 @@ describe("stdin file race (parent PAP-4037)", () => { ["Remote command failed: Request failed with status code 502", 1], ["Cloudflare sandbox bridge request failed with HTTP 502. sensitive-input", 1], ["Remote command failed: sensitive-input", 1], + ["Provider never responds", 1], ] as const)("bounds input failure %s to %i attempts and stops later writes", async (failure, expectedAttempts) => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-stdin-failed-")); cleanupDirs.push(rootDir); @@ -602,11 +603,13 @@ describe("stdin file race (parent PAP-4037)", () => { if (script.includes("/stdin/000000000002.json")) laterWrite = true; if (script.includes("/stdin/000000000001.json")) { attempts += 1; + if (failure === "Provider never responds") await new Promise(() => {}); throw new Error(failure); } }); const bridge = await startAdapterExecutionTargetProcessSessionBridge({ runId: "run-stdin-failed", + timeoutSec: failure === "Provider never responds" ? 5 : undefined, target: { kind: "remote", transport: "sandbox", remoteCwd: rootDir, runner }, runtimeRootDir: path.join(rootDir, "runtime"), adapterKey: "acpx", command: "cat", args: [], cwd: rootDir, env: {}, diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index 6b118011d4..aa66d00ddb 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -38,6 +38,7 @@ import { createSandboxCallbackBridgeToken, DEFAULT_SANDBOX_CALLBACK_BRIDGE_MAX_BODY_BYTES, HTTP2_SANDBOX_CALLBACK_BRIDGE_ROUTE_ALLOWLIST, + runSandboxBridgeControlCommand, SANDBOX_CALLBACK_BRIDGE_ENTRYPOINT, SANDBOX_CALLBACK_BRIDGE_HTTP2_MODE, sandboxCallbackBridgeDirectories, @@ -372,13 +373,9 @@ export interface AdapterExecutionTargetProcessSessionBridgeHandle { export { sanitizeRemoteExecutionEnv } from "./remote-execution-env.js"; -// 4-hour wall-clock backstop for sandbox-backed adapter runs. This is a -// last-resort kill switch, not the primary hang detector: genuinely hung runs -// are caught much earlier by the adapters' output-inactivity monitors (e.g. -// codex-local's 7-minute monitor). The value intentionally matches the -// recovery watchdog's ACTIVE_RUN_OUTPUT_CRITICAL_THRESHOLD_MS (4h) in -// server/src/services/recovery/service.ts so healthy long runs are never -// killed by the adapter before the watchdog would even consider them stuck. +// Four-hour wall-clock backstop for sandbox-backed adapter runs. Keep this +// execution limit independent of the earlier informational silence warnings +// and the short deadlines for bridge control operations. export const DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC = 14_400; function parseObject(value: unknown): Record { @@ -2068,7 +2065,7 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { } else { const payloadPath = path.posix.join(sessionDir, "command.b64"); const runPayloadSetup = async (script: string) => { - const result = await runner.execute({ + const result = await runSandboxBridgeControlCommand(runner, { command: shellCommand, args: shellCommandArgs(script), cwd: target.remoteCwd, @@ -2097,7 +2094,7 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { // as one foreground session command further down instead, so skip this. if (!streamOutput) { await onLog("stdout", `[paperclip] Starting ACP process session bridge in sandbox (${target.providerKey ?? "provider"}).\n`); - const startResult = await runner.execute({ + const startResult = await runSandboxBridgeControlCommand(runner, { command: shellCommand, args: shellCommandArgs( [ diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts index 4419bee265..d5d157ef59 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts @@ -1536,6 +1536,105 @@ describe("sandbox callback bridge", () => { } }); + it.each([ + [4 * 60 * 60 * 1000, 30_000], + [250, 250], + ])("bounds a hung bridge read configured for %i ms to %i ms", async (configuredMs, expectedMs) => { + vi.useFakeTimers(); + try { + const runner = { execute: vi.fn(() => new Promise(() => {})) }; + const client = createCommandManagedSandboxCallbackBridgeQueueClient({ + runner, remoteCwd: "/workspace", timeoutMs: configuredMs, + }); + let error: unknown; + const read = client.readTextFile("/workspace/events/1.json").catch((caught) => { error = caught; }); + await vi.advanceTimersByTimeAsync(expectedMs - 1); + expect(error).toBeUndefined(); + await vi.advanceTimersByTimeAsync(1); + expect(error).toEqual(new Error(`Sandbox bridge control command timed out after ${expectedMs}ms.`)); + await read; + expect(runner.execute).toHaveBeenCalledExactlyOnceWith(expect.objectContaining({ + timeoutMs: expectedMs, bypassSession: true, + })); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + it("abandons a timed-out upload without replaying it or continuing after a late response", async () => { + vi.useFakeTimers(); + try { + const success: RunProcessResult = { + exitCode: 0, signal: null, timedOut: false, stdout: "", stderr: "", pid: null, + startedAt: new Date().toISOString(), + }; + let finishAppend!: (result: RunProcessResult) => void; + const runner = { + execute: vi.fn(async (input: { args?: string[] }) => { + if (input.args?.[1]?.startsWith("printf")) { + return new Promise((resolve) => { finishAppend = resolve; }); + } + return success; + }), + }; + const client = createCommandManagedSandboxCallbackBridgeQueueClient({ + runner, remoteCwd: "/workspace", timeoutMs: 4 * 60 * 60 * 1000, + }); + let error: unknown; + const write = client.writeTextFile("/workspace/stdin/1.json", "sensitive-input") + .catch((caught) => { error = caught; }); + await vi.advanceTimersByTimeAsync(30_000); + expect(error).toEqual(new Error("Sandbox bridge control command timed out after 30000ms.")); + await write; + finishAppend(success); + await vi.advanceTimersByTimeAsync(0); + const scripts = runner.execute.mock.calls.map(([input]) => input.args?.[1] ?? ""); + expect(scripts.filter((script) => script.startsWith("printf"))).toHaveLength(1); + expect(scripts.some((script) => script.startsWith("base64 -d"))).toBe(false); + expect(scripts.at(-1)).toMatch(/^rm -f .*paperclip-upload/); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + it.each(["start", "stop"])("bounds a hung callback bridge %s while preserving its launch environment", async (stage) => { + vi.useFakeTimers(); + try { + const runner = { + execute: vi.fn(async (input: { args?: string[]; env?: Record }) => { + const script = input.args?.[1] ?? ""; + if ((stage === "start" && script.includes("nohup")) || + (stage === "stop" && script.includes('kill "$pid"'))) { + return new Promise(() => {}); + } + return { + exitCode: 0, signal: null, timedOut: false, stderr: "", pid: null, + startedAt: new Date().toISOString(), stdout: JSON.stringify({ port: 3101 }), + }; + }), + }; + let error: unknown; + const operation = startSandboxCallbackBridgeServer({ + runner, remoteCwd: "/workspace", assetRemoteDir: "/workspace/assets", + queueDir: "/workspace/queue", bridgeToken: "private-bridge-token", timeoutMs: 4 * 60 * 60 * 1000, + }).then(async (bridge) => { if (stage === "stop") await bridge.stop(); }) + .catch((caught) => { error = caught; }); + await vi.advanceTimersByTimeAsync(30_000); + expect(error).toEqual(new Error("Sandbox bridge control command timed out after 30000ms.")); + await operation; + expect(runner.execute.mock.calls[0]?.[0].env).toMatchObject({ + PAPERCLIP_BRIDGE_TOKEN: "private-bridge-token", + PAPERCLIP_SANDBOX_EXEC_CHANNEL: "bridge", + }); + expect(runner.execute).toHaveBeenLastCalledWith(expect.objectContaining({ timeoutMs: 30_000 })); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + it("marks command-managed bridge operations with the bridge execution channel", async () => { const runner = { execute: vi.fn(async () => ({ diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.ts b/packages/adapter-utils/src/sandbox-callback-bridge.ts index 40776a3da5..ff56a94a70 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.ts @@ -26,6 +26,7 @@ import type { RunProcessResult } from "./server-utils.js"; const DEFAULT_BRIDGE_TOKEN_BYTES = 24; const DEFAULT_BRIDGE_POLL_INTERVAL_MS = 100; const DEFAULT_BRIDGE_RESPONSE_TIMEOUT_MS = 30_000; +const MAX_BRIDGE_CONTROL_COMMAND_TIMEOUT_MS = 30_000; const DEFAULT_BRIDGE_STOP_TIMEOUT_MS = 2_000; const DEFAULT_BRIDGE_MAX_QUEUE_DEPTH = 64; // A `BridgeBodyReservation` owner (`http2-bridge-server.ts`) now bounds the @@ -350,6 +351,22 @@ function buildRunnerFailureMessage(action: string, result: RunProcessResult): st return `${action} failed with exit code ${result.exitCode ?? "null"}${detail ? `: ${detail}` : ""}`; } +export function runSandboxBridgeControlCommand( + runner: CommandManagedRuntimeRunner, + input: Parameters[0], +): Promise { + // These short file/control operations must not inherit an hours-long agent + // lifetime. Enforce the deadline on the host too: a provider may never settle + // its promise even when it receives timeoutMs. This does not prove the remote + // operation stopped; callers must not replay an uncertain write on timeout. + const controlTimeoutMs = Math.min( + normalizeTimeoutMs(input.timeoutMs, MAX_BRIDGE_CONTROL_COMMAND_TIMEOUT_MS), + MAX_BRIDGE_CONTROL_COMMAND_TIMEOUT_MS, + ); + return withTimeout(runner.execute({ ...input, timeoutMs: controlTimeoutMs }), + controlTimeoutMs, "Sandbox bridge control command"); +} + async function runShell( runner: CommandManagedRuntimeRunner, cwd: string, @@ -358,7 +375,7 @@ async function runShell( shellCommand: "bash" | "sh" = "sh", stdin?: string, ): Promise { - return await runner.execute({ + return await runSandboxBridgeControlCommand(runner, { command: shellCommand, args: shellCommandArgs(script), cwd, @@ -1833,7 +1850,7 @@ export async function startSandboxCallbackBridgeServer(input: { maxBodyBytes: input.maxBodyBytes, }); const nodeCommand = input.nodeCommand?.trim() || "node"; - const startResult = await input.runner.execute({ + const startResult = await runSandboxBridgeControlCommand(input.runner, { command: shellCommand, args: shellCommandArgs( [ @@ -1909,7 +1926,7 @@ export async function startSandboxCallbackBridgeServer(input: { pid: typeof readyData.pid === "number" && Number.isFinite(readyData.pid) ? readyData.pid : 0, directories, stop: async () => { - const stopResult = await input.runner.execute({ + const stopResult = await runSandboxBridgeControlCommand(input.runner, { command: shellCommand, args: shellCommandArgs( [ diff --git a/server/src/__tests__/heartbeat-active-run-output-watchdog.test.ts b/server/src/__tests__/heartbeat-active-run-output-watchdog.test.ts index ce42e31fb6..eb04d057a4 100644 --- a/server/src/__tests__/heartbeat-active-run-output-watchdog.test.ts +++ b/server/src/__tests__/heartbeat-active-run-output-watchdog.test.ts @@ -230,6 +230,27 @@ describeEmbeddedPostgres("active-run output watchdog", () => { expect(manager?.status).toBe("idle"); } + it("warns after five silent minutes and escalates after fifteen without changing active work", async () => { + const now = new Date("2026-04-22T20:00:00.000Z"); + const seeded = await seedRunningRun({ now, ageMs: 0 }); + const summaryAt = (elapsedMs: number) => buildSummary(seeded.runId, new Date(now.getTime() + elapsedMs)); + await expect(summaryAt(5 * 60_000 - 1)).resolves.toMatchObject({ level: "ok" }); + await expect(summaryAt(5 * 60_000)).resolves.toMatchObject({ level: "suspicious" }); + await expect(summaryAt(15 * 60_000 - 1)).resolves.toMatchObject({ level: "suspicious" }); + await expect(summaryAt(15 * 60_000)).resolves.toMatchObject({ level: "critical" }); + const { recovery, enqueueWakeup } = createRecovery(); + await recovery.scanSilentActiveRuns({ now: new Date(now.getTime() + 35 * 60_000), companyId: seeded.companyId }); + const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, seeded.runId)); + expect(run?.status).toBe("running"); + expect(enqueueWakeup).not.toHaveBeenCalled(); + await expectNoReviewArtifacts(seeded); + + // Fresh output clears the warning even on a long-running task. + await db.update(heartbeatRuns).set({ lastOutputAt: new Date(now.getTime() + 35 * 60_000) }) + .where(eq(heartbeatRuns.id, seeded.runId)); + await expect(summaryAt(36 * 60_000)).resolves.toMatchObject({ level: "ok" }); + }); + it.each(["stale_active_run_evaluation", "issue_productivity_review"])("keeps blocked and %s sources artifact-free", async (originKind) => { const now = new Date("2026-04-22T20:00:00.000Z"); const blocked = await seedRunningRun({ diff --git a/server/src/__tests__/heartbeat-workspace-busy.test.ts b/server/src/__tests__/heartbeat-workspace-busy.test.ts index 44cff68cfb..b62f6f061c 100644 --- a/server/src/__tests__/heartbeat-workspace-busy.test.ts +++ b/server/src/__tests__/heartbeat-workspace-busy.test.ts @@ -523,8 +523,8 @@ describeEmbeddedPostgres("shared-workspace run serialization", () => { expect(retryRuns).toHaveLength(0); }); - it("defers a run whose issue targets a busy shared workspace and schedules a bounded retry", async () => { - const fixture = await seedWorkspaceFixture(); + it.each([0, 35 * 60_000])("defers a run in a busy shared workspace after %i ms of holder silence", async (silenceMs) => { + const fixture = await seedWorkspaceFixture({ holderActivityAt: new Date(Date.now() - silenceMs) }); const run = await heartbeat.invoke( fixture.agentId, diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 5d8e295559..a1c57d00f2 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -527,10 +527,7 @@ import { type StrandedRecoveryNoticeSeed, } from "./recovery/stranded-notice.js"; import { withRecoveryContext } from "./recovery/status-only-context.js"; -import { - ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS as RECOVERY_ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS, - recoveryService, -} from "./recovery/service.js"; +import { recoveryService } from "./recovery/service.js"; import { createRunDispatch, type PostCommitEffect, @@ -914,16 +911,10 @@ export const WORKSPACE_BUSY_RETRY_WAKE_REASON = "workspace_busy_retry"; export const WORKSPACE_BUSY_ERROR_CODE = "workspace_busy"; export const WORKSPACE_BUSY_RETRY_BASE_DELAY_MS = 60 * 1000; export const WORKSPACE_BUSY_RETRY_JITTER_MS = 60 * 1000; -// A running run stops counting as a shared-workspace holder once it has been -// silent this long. This is recovery's own "suspicious silence" bar for active -// runs (scanSilentActiveRuns escalates such runs), so a zombie holder cannot -// park other work on the workspace forever: it stops blocking here at the same -// moment the recovery machinery starts treating it as stuck. A LIVE holder, in -// contrast, never gets overtaken — a deferred run keeps rescheduling until the -// workspace frees, because dispatching alongside a live holder is exactly the -// concurrent-mutation failure this gate exists to prevent. -export const WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS = - RECOVERY_ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS; +// Preserve the one-hour shared-workspace holder cutoff independently of the +// informational output-silence warnings. Warning sooner must not let another +// run overtake a quiet holder and mutate its shared workspace. +export const WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS = 60 * 60 * 1000; // Issue-level executionWorkspaceSettings.mode values that unambiguously opt an // issue's runs out of the shared project workspace, and therefore out of // shared-workspace serialization ("isolated" is the legacy alias @@ -16277,8 +16268,8 @@ export function heartbeatService( // issue shares the same project workspace, i.e. the run that currently // "holds" the shared working tree. Runs that have been silent past // WORKSPACE_BUSY_HOLDER_STALE_AFTER_MS do not count — a zombie holder must - // not park other work forever, and recovery's silent-run escalation is - // already reaping it. When isolated workspaces are enabled, holders whose + // not park other work forever. This cutoff is independent of informational + // silence warnings. When isolated workspaces are enabled, holders whose // issue explicitly opted into an isolated workspace never touch the shared // tree, so they are excluded; a NULL/agent_default mode may resolve to the // shared tree and counts as a holder (over-serializing is the safe diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 6f7ac9d9ca..113ea5ce1c 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -163,8 +163,8 @@ const UNSUCCESSFUL_HEARTBEAT_RUN_TERMINAL_STATUSES = [ "cancelled", "timed_out", ] as const; -export const ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS = 60 * 60 * 1000; -export const ACTIVE_RUN_OUTPUT_CRITICAL_THRESHOLD_MS = 4 * 60 * 60 * 1000; +export const ACTIVE_RUN_OUTPUT_SUSPICION_THRESHOLD_MS = 5 * 60 * 1000; +export const ACTIVE_RUN_OUTPUT_CRITICAL_THRESHOLD_MS = 15 * 60 * 1000; export const ACTIVE_RUN_OUTPUT_CONTINUE_REARM_MS = 30 * 60 * 1000; const STRANDED_ISSUE_RECOVERY_ORIGIN_KIND = RECOVERY_ORIGIN_KINDS.strandedIssueRecovery;