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;