fix(timeouts): raise sandbox wall-clock backstop to 4h and make acpx_local timeouts self-describing (#9232)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent runs execute through adapters (e.g. `acpx_local`), which can run locally, over SSH, or inside sandbox execution targets, each with a wall-clock execution timeout > - Sandbox-backed runs defaulted to a 30-minute wall-clock backstop (`DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC = 1800`), which kills healthy long agent runs that are still making progress — long before the recovery watchdog's 4h critical threshold would even consider them stuck > - On top of that, `acpx_local` resolved its timeout directly from `adapterConfig.timeoutSec` instead of the shared execution-target resolver, and its timeout failures surfaced as a bare `Timed out after Ns` — giving operators no clue which timer fired or which knob raises it > - This pull request raises the sandbox backstop to 4h (aligned with the recovery watchdog), routes `acpx_local` through the shared timeout resolver, logs the effective timeout and its source at run start, and makes every timeout error message self-describing > - The benefit is that long-running sandbox agent runs no longer die at 30 minutes, and when a wall-clock timeout does fire, the run log states exactly which timer fired and how to configure it ## Linked Issues or Issue Description Refs #4535 (related: wall-clock execution timeouts killing agent runs that are still making progress — that issue covers a different hardcoded 600s timer, but the operator pain is the same). No exact public issue exists for this one, so describing it in-PR: **Bug:** A long sandbox-backed `acpx_local` agent run was killed with a bare `Timed out after 1800s` even though the agent was actively working. - **What happened:** The run hit the 30-minute `DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC` backstop. `acpx_local` never consulted the shared execution-target timeout resolution (it read `adapterConfig.timeoutSec` directly, default 0), so on sandbox targets the sandbox-provider default applied with no adapter-level say. The resulting error named neither the timer that fired nor the knob that controls it. - **Expected:** Healthy long runs should not be killed by a 30-minute wall-clock backstop when the recovery watchdog only treats runs as critically stuck after 4h of output silence; and any timeout error should say which timeout fired and how to raise it. - **Impact:** Long, legitimate agent runs in sandboxes fail mid-work; operators waste time reverse-engineering which of several timers produced "Timed out after Ns". ## What Changed - `packages/adapter-utils/src/execution-target.ts` - `DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC` raised from `1_800` to `14_400` (4h), with a comment explaining it intentionally matches the recovery watchdog's `ACTIVE_RUN_OUTPUT_CRITICAL_THRESHOLD_MS` (4h) so the adapter backstop never fires before the watchdog path. Output-inactivity monitors remain the primary hang detectors. - New `resolveAdapterExecutionTargetTimeout(target, configuredTimeoutSec)` returns `{ timeoutSec, source }` where `source` is `configured` / `sandbox_default` / `unlimited`. The existing `resolveAdapterExecutionTargetTimeoutSec` is preserved as a thin wrapper, so current callers are unaffected. - New `formatAdapterExecutionTimeoutErrorMessage(resolution)` and `formatAdapterExecutionTimeoutStartLogLine(resolution)` produce self-describing messages that name the timer that fired and the `adapterConfig.timeoutSec` knob that controls it. - `packages/adapters/acpx-local/src/server/execute.ts` - `buildRuntime` now resolves the wall-clock timeout through the shared resolver: sandbox targets default to the 4h backstop, local/SSH keep the historical "0 = no adapter timeout", and a configured `adapterConfig.timeoutSec` always wins. - The executor logs the effective timeout and its source at run start (`[paperclip] Adapter execution timeout: …`), so a later timeout is diagnosable from the run log alone. - All three bare timeout messages (timer cancel reason, turn result `errorMessage`, catch-path `messageOverride`) now use the self-describing format. - `packages/adapters/acpx-local/src/index.ts` — the adapter configuration doc for `timeoutSec` states the sandbox default and that the output-inactivity monitor remains the primary hang detector. - Tests: `packages/adapter-utils/src/execution-target-sandbox.test.ts` and `packages/adapters/acpx-local/src/server/execute.test.ts` (see Verification). ## Verification - `pnpm --filter @paperclipai/adapter-utils typecheck` — passes - `pnpm --filter @paperclipai/adapter-acpx-local typecheck` — passes - `npx vitest run packages/adapter-utils/src/execution-target-sandbox.test.ts packages/adapters/acpx-local/src/server/execute.test.ts` — 2 files, 40 tests, all pass - New/updated test coverage: - sandbox default resolves to 4h (and the constant is asserted to be `4 * 60 * 60`) - `resolveAdapterExecutionTargetTimeout` reports `configured` / `sandbox_default` / `unlimited` sources with the correct precedence (configured > sandbox default; local/SSH stay unlimited) - exact wording of the self-describing error message and the start-of-run log line - `acpx_local` runtime picks up the sandbox default into `timeoutMs`, keeps the unlimited local default, honors configured-over-default precedence, emits the start-of-run log line, and surfaces the self-describing `errorMessage`/cancel reason when the wall-clock timer kills a turn ## Risks - **Behavioral shift:** sandbox-backed adapter runs that previously hit the 30-minute backstop now run up to 4h before the adapter kills them. Genuinely hung runs are still caught much earlier by the adapters' output-inactivity monitors and by the recovery watchdog; the wall-clock timer is a last-resort kill switch. Operators who relied on the 30-minute default can restore it explicitly via `adapterConfig.timeoutSec`. - **Error-message consumers:** any tooling that pattern-matched the exact `Timed out after Ns` string from `acpx_local` will see the new self-describing message instead. - No API or schema changes; `resolveAdapterExecutionTargetTimeoutSec` keeps its exact signature and behavior (modulo the raised sandbox default). > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude (Anthropic) via Claude Code CLI — model ID `claude-fable-5`, extended thinking enabled, agentic tool use (file edits, shell, test execution) ## 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 - [ ] All Paperclip CI gates are green - [ ] 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 <noreply@paperclip.ing>
This commit is contained in:
Devin Foley
GitHub
parent
4a7a732476
commit
cc17f29e7e
|
|
@ -66,7 +66,7 @@ In agent runtime settings, configure heartbeat policy:
|
|||
For local adapters, set:
|
||||
|
||||
- `cwd` (working directory)
|
||||
- `timeoutSec` (max runtime per heartbeat)
|
||||
- `timeoutSec` (max runtime per heartbeat; `0` uses the target default — no adapter timeout on local/SSH, a 4-hour backstop on sandbox targets — and a negative value disables the adapter timeout everywhere, including sandboxes)
|
||||
- `graceSec` (time before force-kill after timeout/cancel)
|
||||
- optional env vars and extra CLI args
|
||||
- use **Test environment** in agent configuration to run adapter-specific diagnostics before saving
|
||||
|
|
|
|||
|
|
@ -10,6 +10,9 @@ import {
|
|||
adapterExecutionTargetToRemoteSpec,
|
||||
adapterExecutionTargetUsesPaperclipBridge,
|
||||
ensureAdapterExecutionTargetCommandResolvable,
|
||||
formatAdapterExecutionTimeoutErrorMessage,
|
||||
formatAdapterExecutionTimeoutStartLogLine,
|
||||
resolveAdapterExecutionTargetTimeout,
|
||||
resolveAdapterExecutionTargetTimeoutSec,
|
||||
runAdapterExecutionTargetProcess,
|
||||
runAdapterExecutionTargetShellCommand,
|
||||
|
|
@ -163,6 +166,10 @@ 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.
|
||||
expect(DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC).toBe(4 * 60 * 60);
|
||||
expect(resolveAdapterExecutionTargetTimeoutSec(sandboxTarget, 0)).toBe(
|
||||
DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC,
|
||||
);
|
||||
|
|
@ -182,6 +189,121 @@ describe("sandbox adapter execution targets", () => {
|
|||
strictHostKeyChecking: true,
|
||||
},
|
||||
}, 0)).toBe(0);
|
||||
expect(resolveAdapterExecutionTargetTimeoutSec({ kind: "local" }, 0)).toBe(0);
|
||||
});
|
||||
|
||||
it("reports which knob produced the resolved timeout", () => {
|
||||
const sandboxTarget: AdapterSandboxExecutionTarget = {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
remoteCwd: "/workspace",
|
||||
runner: createLocalSandboxRunner(),
|
||||
};
|
||||
|
||||
expect(resolveAdapterExecutionTargetTimeout(sandboxTarget, 0)).toEqual({
|
||||
timeoutSec: DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC,
|
||||
source: "sandbox_default",
|
||||
});
|
||||
expect(resolveAdapterExecutionTargetTimeout(sandboxTarget, 90)).toEqual({
|
||||
timeoutSec: 90,
|
||||
source: "configured",
|
||||
});
|
||||
expect(resolveAdapterExecutionTargetTimeout({ kind: "local" }, 0)).toEqual({
|
||||
timeoutSec: 0,
|
||||
source: "unlimited",
|
||||
});
|
||||
// Fractional (sub-second) configured timeouts are preserved rather than
|
||||
// floored to 0, which would silently mean "no timeout".
|
||||
expect(resolveAdapterExecutionTargetTimeout({ kind: "local" }, 0.01)).toEqual({
|
||||
timeoutSec: 0.01,
|
||||
source: "configured",
|
||||
});
|
||||
expect(resolveAdapterExecutionTargetTimeout(sandboxTarget, 0.5)).toEqual({
|
||||
timeoutSec: 0.5,
|
||||
source: "configured",
|
||||
});
|
||||
});
|
||||
|
||||
it("treats a negative timeoutSec as the explicit no-timeout opt-out, even on sandbox targets", () => {
|
||||
const sandboxTarget: AdapterSandboxExecutionTarget = {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
remoteCwd: "/workspace",
|
||||
runner: createLocalSandboxRunner(),
|
||||
};
|
||||
|
||||
expect(resolveAdapterExecutionTargetTimeout(sandboxTarget, -1)).toEqual({
|
||||
timeoutSec: 0,
|
||||
source: "configured",
|
||||
});
|
||||
expect(resolveAdapterExecutionTargetTimeout({ kind: "local" }, -1)).toEqual({
|
||||
timeoutSec: 0,
|
||||
source: "configured",
|
||||
});
|
||||
expect(resolveAdapterExecutionTargetTimeoutSec(sandboxTarget, -1)).toBe(0);
|
||||
|
||||
// Explicit zero intentionally does NOT opt out: the adapter config UI
|
||||
// persists the schema default of 0 for untouched fields, so a stored
|
||||
// timeoutSec=0 cannot be read as operator intent. It keeps the sandbox
|
||||
// backstop; the documented opt-out is a negative value.
|
||||
expect(resolveAdapterExecutionTargetTimeout(sandboxTarget, 0)).toEqual({
|
||||
timeoutSec: DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC,
|
||||
source: "sandbox_default",
|
||||
});
|
||||
// Unset behaves like zero.
|
||||
expect(resolveAdapterExecutionTargetTimeout(sandboxTarget, undefined)).toEqual({
|
||||
timeoutSec: DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC,
|
||||
source: "sandbox_default",
|
||||
});
|
||||
expect(resolveAdapterExecutionTargetTimeout({ kind: "local" }, undefined)).toEqual({
|
||||
timeoutSec: 0,
|
||||
source: "unlimited",
|
||||
});
|
||||
});
|
||||
|
||||
it("formats self-describing timeout errors naming the timer and knob", () => {
|
||||
expect(
|
||||
formatAdapterExecutionTimeoutErrorMessage({
|
||||
timeoutSec: DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC,
|
||||
source: "sandbox_default",
|
||||
}),
|
||||
).toBe(
|
||||
"Run exceeded the adapter execution timeout (timeoutSec=14400, sandbox default). " +
|
||||
"Set adapterConfig.timeoutSec to raise it.",
|
||||
);
|
||||
expect(
|
||||
formatAdapterExecutionTimeoutErrorMessage({ timeoutSec: 1800, source: "configured" }),
|
||||
).toBe(
|
||||
"Run exceeded the adapter execution timeout (timeoutSec=1800, configured via adapterConfig.timeoutSec). " +
|
||||
"Set adapterConfig.timeoutSec to raise it.",
|
||||
);
|
||||
});
|
||||
|
||||
it("formats the start-of-run timeout log line with the resolved value and source", () => {
|
||||
expect(
|
||||
formatAdapterExecutionTimeoutStartLogLine({
|
||||
timeoutSec: DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC,
|
||||
source: "sandbox_default",
|
||||
}),
|
||||
).toBe(
|
||||
"Adapter execution timeout: timeoutSec=14400 (sandbox default; set adapterConfig.timeoutSec to override).",
|
||||
);
|
||||
expect(
|
||||
formatAdapterExecutionTimeoutStartLogLine({ timeoutSec: 900, source: "configured" }),
|
||||
).toBe(
|
||||
"Adapter execution timeout: timeoutSec=900 (configured via adapterConfig.timeoutSec; set adapterConfig.timeoutSec to override).",
|
||||
);
|
||||
expect(
|
||||
formatAdapterExecutionTimeoutStartLogLine({ timeoutSec: 0, source: "unlimited" }),
|
||||
).toBe(
|
||||
"Adapter execution timeout: none (no adapter wall-clock timeout for this target; set adapterConfig.timeoutSec to add one).",
|
||||
);
|
||||
// Negative opt-out resolves to { timeoutSec: 0, source: "configured" }.
|
||||
expect(
|
||||
formatAdapterExecutionTimeoutStartLogLine({ timeoutSec: 0, source: "configured" }),
|
||||
).toBe(
|
||||
"Adapter execution timeout: none (explicitly disabled via adapterConfig.timeoutSec; set it to a positive value to add one).",
|
||||
);
|
||||
});
|
||||
|
||||
it("uses the caller timeout override when installing a missing sandbox command", async () => {
|
||||
|
|
@ -892,7 +1014,9 @@ describe("sandbox adapter execution targets", () => {
|
|||
try {
|
||||
expect(bridge).not.toBeNull();
|
||||
expect(runner.execute).toHaveBeenCalled();
|
||||
expect(runner.execute.mock.calls.some(([input]) => input.timeoutMs === 1_800_000)).toBe(true);
|
||||
expect(
|
||||
runner.execute.mock.calls.some(([input]) => input.timeoutMs === DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC * 1000),
|
||||
).toBe(true);
|
||||
} finally {
|
||||
await bridge?.stop();
|
||||
await new Promise<void>((resolve) => apiServer.close(() => resolve()));
|
||||
|
|
|
|||
|
|
@ -127,7 +127,14 @@ export interface AdapterExecutionTargetPaperclipBridgeHandle {
|
|||
|
||||
export { sanitizeRemoteExecutionEnv } from "./remote-execution-env.js";
|
||||
|
||||
export const DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC = 1_800;
|
||||
// 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.
|
||||
export const DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC = 14_400;
|
||||
|
||||
function parseObject(value: unknown): Record<string, unknown> {
|
||||
return value && typeof value === "object" && !Array.isArray(value)
|
||||
|
|
@ -252,24 +259,107 @@ export function describeAdapterExecutionTarget(
|
|||
return `sandbox environment${target.providerKey ? ` (${target.providerKey})` : ""}`;
|
||||
}
|
||||
|
||||
export function resolveAdapterExecutionTargetTimeoutSec(
|
||||
export type AdapterExecutionTargetTimeoutSource =
|
||||
| "configured"
|
||||
| "sandbox_default"
|
||||
| "unlimited";
|
||||
|
||||
export interface AdapterExecutionTargetTimeoutResolution {
|
||||
/** Resolved wall-clock timeout in seconds; 0 means no adapter timeout. */
|
||||
timeoutSec: number;
|
||||
/** Which knob produced the resolved value, for logs and error messages. */
|
||||
source: AdapterExecutionTargetTimeoutSource;
|
||||
}
|
||||
|
||||
export function resolveAdapterExecutionTargetTimeout(
|
||||
target: AdapterExecutionTarget | null | undefined,
|
||||
configuredTimeoutSec: number | null | undefined,
|
||||
): number {
|
||||
const normalizedConfiguredTimeoutSec =
|
||||
typeof configuredTimeoutSec === "number" && Number.isFinite(configuredTimeoutSec) && configuredTimeoutSec > 0
|
||||
? Math.floor(configuredTimeoutSec)
|
||||
: 0;
|
||||
if (normalizedConfiguredTimeoutSec > 0) return normalizedConfiguredTimeoutSec;
|
||||
): AdapterExecutionTargetTimeoutResolution {
|
||||
if (typeof configuredTimeoutSec === "number" && Number.isFinite(configuredTimeoutSec)) {
|
||||
// Preserve fractional (sub-second) configured values instead of flooring:
|
||||
// adapters historically honored e.g. timeoutSec=0.5, and flooring would
|
||||
// silently turn it into "no timeout".
|
||||
if (configuredTimeoutSec > 0) {
|
||||
return { timeoutSec: configuredTimeoutSec, source: "configured" };
|
||||
}
|
||||
// A negative timeoutSec is the explicit "no adapter wall-clock timeout"
|
||||
// opt-out, honored even on sandbox targets. Zero cannot carry that
|
||||
// meaning: the adapter config UI persists the schema default of 0 for
|
||||
// untouched fields, so timeoutSec=0 in stored config does not signal
|
||||
// operator intent and falls through to target defaults below.
|
||||
if (configuredTimeoutSec < 0) {
|
||||
return { timeoutSec: 0, source: "configured" };
|
||||
}
|
||||
}
|
||||
// Local and SSH adapters preserve the historical "0 means no adapter
|
||||
// timeout" behavior. Sandbox-backed runs execute through provider RPCs
|
||||
// that usually apply their own shorter command defaults, so request an
|
||||
// explicit longer timeout for full adapter runs when the adapter leaves
|
||||
// timeoutSec unset.
|
||||
if (target?.kind === "remote" && target.transport === "sandbox") {
|
||||
return DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC;
|
||||
return { timeoutSec: DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC, source: "sandbox_default" };
|
||||
}
|
||||
return 0;
|
||||
return { timeoutSec: 0, source: "unlimited" };
|
||||
}
|
||||
|
||||
export function resolveAdapterExecutionTargetTimeoutSec(
|
||||
target: AdapterExecutionTarget | null | undefined,
|
||||
configuredTimeoutSec: number | null | undefined,
|
||||
): number {
|
||||
return resolveAdapterExecutionTargetTimeout(target, configuredTimeoutSec).timeoutSec;
|
||||
}
|
||||
|
||||
function describeAdapterExecutionTimeoutSource(
|
||||
source: AdapterExecutionTargetTimeoutSource,
|
||||
): string {
|
||||
switch (source) {
|
||||
case "configured":
|
||||
return "configured via adapterConfig.timeoutSec";
|
||||
case "sandbox_default":
|
||||
return "sandbox default";
|
||||
case "unlimited":
|
||||
return "no adapter wall-clock timeout";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Self-describing error message for when the adapter wall-clock execution
|
||||
* timeout kills a run. Names the timer that fired and the knob that controls
|
||||
* it so run failures never surface as a bare "Timed out".
|
||||
*/
|
||||
export function formatAdapterExecutionTimeoutErrorMessage(
|
||||
resolution: AdapterExecutionTargetTimeoutResolution,
|
||||
): string {
|
||||
return (
|
||||
`Run exceeded the adapter execution timeout ` +
|
||||
`(timeoutSec=${resolution.timeoutSec}, ${describeAdapterExecutionTimeoutSource(resolution.source)}). ` +
|
||||
`Set adapterConfig.timeoutSec to raise it.`
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* One-line start-of-run statement of the effective wall-clock timeout and its
|
||||
* source. Callers prefix with `[paperclip] ` and append a newline.
|
||||
*/
|
||||
export function formatAdapterExecutionTimeoutStartLogLine(
|
||||
resolution: AdapterExecutionTargetTimeoutResolution,
|
||||
): string {
|
||||
if (resolution.timeoutSec <= 0) {
|
||||
if (resolution.source === "configured") {
|
||||
return (
|
||||
"Adapter execution timeout: none " +
|
||||
"(explicitly disabled via adapterConfig.timeoutSec; set it to a positive value to add one)."
|
||||
);
|
||||
}
|
||||
return (
|
||||
"Adapter execution timeout: none " +
|
||||
"(no adapter wall-clock timeout for this target; set adapterConfig.timeoutSec to add one)."
|
||||
);
|
||||
}
|
||||
return (
|
||||
`Adapter execution timeout: timeoutSec=${resolution.timeoutSec} ` +
|
||||
`(${describeAdapterExecutionTimeoutSource(resolution.source)}; set adapterConfig.timeoutSec to override).`
|
||||
);
|
||||
}
|
||||
|
||||
function requireSandboxRunner(target: AdapterSandboxExecutionTarget): CommandManagedRuntimeRunner {
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ Core fields:
|
|||
- model (string, optional): requested ACP model. Claude and Codex ACP agents both receive this through ACP session config.
|
||||
- effort/modelReasoningEffort (string, optional): requested thinking effort. Claude uses effort; Codex uses modelReasoningEffort/reasoning_effort.
|
||||
- fastMode (boolean, optional): for ACPX Codex, request Codex fast mode through ACP session config.
|
||||
- timeoutSec (number, optional): run timeout in seconds. Defaults to 0, meaning no adapter timeout.
|
||||
- timeoutSec (number, optional): run timeout in seconds. Defaults to 0, meaning no adapter timeout for local/SSH execution. Sandbox execution targets default to a 4h wall-clock backstop when timeoutSec is unset; the output-inactivity monitor remains the primary hang detector.
|
||||
- warmHandleIdleMs (number, optional): live ACPX process idle window after a successful persistent run. Defaults to 0, meaning Paperclip shuts the process down after each run while retaining ACPX session state.
|
||||
- env (object, optional): KEY=VALUE environment variables or secret bindings.
|
||||
|
||||
|
|
|
|||
|
|
@ -61,6 +61,7 @@ export function getConfigSchema(): AdapterConfigSchema {
|
|||
label: "Timeout seconds",
|
||||
type: "number",
|
||||
default: DEFAULT_ACPX_LOCAL_TIMEOUT_SEC,
|
||||
hint: "Wall-clock timeout for a run. 0 uses the target default: no adapter timeout on local/SSH, 4 hours on sandbox targets. Set a negative value (e.g. -1) to disable the adapter timeout everywhere, including sandboxes.",
|
||||
},
|
||||
{
|
||||
key: "warmHandleIdleMs",
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import os from "node:os";
|
|||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import type { AcpRuntimeOptions } from "acpx/runtime";
|
||||
import { DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC } from "@paperclipai/adapter-utils/execution-target";
|
||||
import { createAcpxLocalExecutor } from "./execute.js";
|
||||
|
||||
const tempRoots: string[] = [];
|
||||
|
|
@ -62,6 +63,7 @@ async function runExecutor(
|
|||
options: {
|
||||
context?: Record<string, unknown>;
|
||||
executionTransport?: Record<string, unknown>;
|
||||
executionTarget?: Record<string, unknown>;
|
||||
} = {},
|
||||
) {
|
||||
const runtimeOptions: Record<string, unknown>[] = [];
|
||||
|
|
@ -84,6 +86,7 @@ async function runExecutor(
|
|||
config,
|
||||
context: options.context ?? {},
|
||||
executionTransport: options.executionTransport,
|
||||
executionTarget: options.executionTarget,
|
||||
onLog: async (stream: "stdout" | "stderr", text: string) => {
|
||||
logs.push({ stream, text });
|
||||
},
|
||||
|
|
@ -700,3 +703,185 @@ describe("acpx_local runtime skill isolation", () => {
|
|||
expect(await pathExists(path.join(cwd, ".claude", "settings.local.json"))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("acpx_local execution timeouts", () => {
|
||||
it("applies the 4h sandbox backstop when timeoutSec is unset on a sandbox execution target", async () => {
|
||||
const root = await makeTempRoot();
|
||||
const stateDir = path.join(root, "state");
|
||||
const cwd = path.join(root, "worktree");
|
||||
await fs.mkdir(cwd, { recursive: true });
|
||||
|
||||
const { logs, runtimeOptions } = await runExecutor(
|
||||
{ agent: "custom", agentCommand: "node ./fake-acp.js", stateDir, cwd },
|
||||
{
|
||||
executionTarget: {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
providerKey: "acme-sandbox",
|
||||
environmentId: "env-1",
|
||||
leaseId: "lease-1",
|
||||
remoteCwd: cwd,
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
// The sandbox default flows into the ACPX runtime wall-clock timer.
|
||||
expect(runtimeOptions[0]?.timeoutMs).toBe(DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC * 1000);
|
||||
// The effective timeout and its source are stated at run start so a later
|
||||
// timeout is diagnosable from the run log alone.
|
||||
const startLine = logs.find(
|
||||
(entry) => entry.stream === "stderr" && entry.text.includes("Adapter execution timeout:"),
|
||||
);
|
||||
expect(startLine).toBeTruthy();
|
||||
expect(startLine!.text).toContain(
|
||||
`[paperclip] Adapter execution timeout: timeoutSec=${DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC} ` +
|
||||
"(sandbox default; set adapterConfig.timeoutSec to override).",
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps local execution unlimited by default and logs the unlimited timeout", async () => {
|
||||
const root = await makeTempRoot();
|
||||
const stateDir = path.join(root, "state");
|
||||
const cwd = path.join(root, "worktree");
|
||||
await fs.mkdir(cwd, { recursive: true });
|
||||
|
||||
const { logs, runtimeOptions } = await runExecutor({
|
||||
agent: "custom",
|
||||
agentCommand: "node ./fake-acp.js",
|
||||
stateDir,
|
||||
cwd,
|
||||
});
|
||||
|
||||
expect(runtimeOptions[0]?.timeoutMs).toBeUndefined();
|
||||
const startLine = logs.find(
|
||||
(entry) => entry.stream === "stderr" && entry.text.includes("Adapter execution timeout:"),
|
||||
);
|
||||
expect(startLine).toBeTruthy();
|
||||
expect(startLine!.text).toContain("Adapter execution timeout: none");
|
||||
});
|
||||
|
||||
it("prefers a configured timeoutSec over the sandbox default", async () => {
|
||||
const root = await makeTempRoot();
|
||||
const stateDir = path.join(root, "state");
|
||||
const cwd = path.join(root, "worktree");
|
||||
await fs.mkdir(cwd, { recursive: true });
|
||||
|
||||
const { logs, runtimeOptions } = await runExecutor(
|
||||
{ agent: "custom", agentCommand: "node ./fake-acp.js", stateDir, cwd, timeoutSec: 90 },
|
||||
{
|
||||
executionTarget: {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
remoteCwd: cwd,
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
expect(runtimeOptions[0]?.timeoutMs).toBe(90 * 1000);
|
||||
const startLine = logs.find(
|
||||
(entry) => entry.stream === "stderr" && entry.text.includes("Adapter execution timeout:"),
|
||||
);
|
||||
expect(startLine!.text).toContain(
|
||||
"Adapter execution timeout: timeoutSec=90 (configured via adapterConfig.timeoutSec; set adapterConfig.timeoutSec to override).",
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps the sandbox backstop for an explicit timeoutSec of 0 but honors a negative opt-out", async () => {
|
||||
const root = await makeTempRoot();
|
||||
const stateDir = path.join(root, "state");
|
||||
const cwd = path.join(root, "worktree");
|
||||
await fs.mkdir(cwd, { recursive: true });
|
||||
const sandboxContext = {
|
||||
executionTarget: {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
remoteCwd: cwd,
|
||||
},
|
||||
};
|
||||
|
||||
// The config UI persists the schema default of 0 for untouched fields, so
|
||||
// an explicit 0 cannot mean "no timeout" — it keeps the 4h backstop.
|
||||
const explicitZero = await runExecutor(
|
||||
{ agent: "custom", agentCommand: "node ./fake-acp.js", stateDir, cwd, timeoutSec: 0 },
|
||||
sandboxContext,
|
||||
);
|
||||
expect(explicitZero.runtimeOptions[0]?.timeoutMs).toBe(
|
||||
DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC * 1000,
|
||||
);
|
||||
|
||||
// A negative timeoutSec is the documented opt-out from any adapter
|
||||
// wall-clock timeout, sandbox targets included.
|
||||
const negativeOptOut = await runExecutor(
|
||||
{ agent: "custom", agentCommand: "node ./fake-acp.js", stateDir, cwd, timeoutSec: -1 },
|
||||
sandboxContext,
|
||||
);
|
||||
expect(negativeOptOut.runtimeOptions[0]?.timeoutMs).toBeUndefined();
|
||||
const startLine = negativeOptOut.logs.find(
|
||||
(entry) => entry.stream === "stderr" && entry.text.includes("Adapter execution timeout:"),
|
||||
);
|
||||
expect(startLine!.text).toContain(
|
||||
"Adapter execution timeout: none (explicitly disabled via adapterConfig.timeoutSec; " +
|
||||
"set it to a positive value to add one).",
|
||||
);
|
||||
});
|
||||
|
||||
it("reports a self-describing timeout error when the wall-clock timer kills a turn", async () => {
|
||||
const root = await makeTempRoot();
|
||||
const stateDir = path.join(root, "state");
|
||||
const cwd = path.join(root, "worktree");
|
||||
await fs.mkdir(cwd, { recursive: true });
|
||||
|
||||
const cancelReasons: string[] = [];
|
||||
let releaseTurn: (() => void) | null = null;
|
||||
const turnCancelled = new Promise<void>((resolve) => {
|
||||
releaseTurn = resolve;
|
||||
});
|
||||
|
||||
const execute = createAcpxLocalExecutor({
|
||||
createRuntime: () => ({
|
||||
ensureSession: async () => ({
|
||||
backendSessionId: "backend-session",
|
||||
agentSessionId: "agent-session",
|
||||
runtimeSessionName: "runtime-session",
|
||||
}),
|
||||
startTurn: () => ({
|
||||
// Never yields on its own: only the Paperclip wall-clock timer's
|
||||
// cancel unblocks the turn, simulating a hung run.
|
||||
events: (async function* () {
|
||||
await turnCancelled;
|
||||
})(),
|
||||
result: turnCancelled.then(() => ({ status: "cancelled", stopReason: "cancelled" })),
|
||||
cancel: async ({ reason }: { reason: string }) => {
|
||||
cancelReasons.push(reason);
|
||||
releaseTurn?.();
|
||||
},
|
||||
}),
|
||||
close: async () => {},
|
||||
}) as never,
|
||||
});
|
||||
|
||||
const result = await execute({
|
||||
runId: "run-timeout-1",
|
||||
agent: { id: "agent-1", companyId: "company-1" },
|
||||
runtime: {},
|
||||
config: {
|
||||
agent: "custom",
|
||||
agentCommand: "node ./fake-acp.js",
|
||||
stateDir,
|
||||
cwd,
|
||||
timeoutSec: 1,
|
||||
},
|
||||
context: {},
|
||||
onLog: async () => {},
|
||||
onMeta: async () => {},
|
||||
} as never);
|
||||
|
||||
const expectedMessage =
|
||||
"Run exceeded the adapter execution timeout (timeoutSec=1, configured via adapterConfig.timeoutSec). " +
|
||||
"Set adapterConfig.timeoutSec to raise it.";
|
||||
expect(result.timedOut).toBe(true);
|
||||
expect(result.errorCode).toBe("acpx_timeout");
|
||||
expect(result.errorMessage).toBe(expectedMessage);
|
||||
expect(cancelReasons).toContain(expectedMessage);
|
||||
}, 15_000);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -4,7 +4,14 @@ import path from "node:path";
|
|||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import type { AdapterExecutionContext, AdapterExecutionResult } from "@paperclipai/adapter-utils";
|
||||
import { readAdapterExecutionTarget, adapterExecutionTargetSessionIdentity } from "@paperclipai/adapter-utils/execution-target";
|
||||
import {
|
||||
adapterExecutionTargetSessionIdentity,
|
||||
formatAdapterExecutionTimeoutErrorMessage,
|
||||
formatAdapterExecutionTimeoutStartLogLine,
|
||||
readAdapterExecutionTarget,
|
||||
resolveAdapterExecutionTargetTimeout,
|
||||
type AdapterExecutionTargetTimeoutResolution,
|
||||
} from "@paperclipai/adapter-utils/execution-target";
|
||||
import {
|
||||
DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE,
|
||||
applyPaperclipWorkspaceEnv,
|
||||
|
|
@ -87,6 +94,7 @@ interface AcpxPreparedRuntime {
|
|||
requestedThinkingEffort: string;
|
||||
fastMode: boolean;
|
||||
timeoutSec: number;
|
||||
timeoutResolution: AdapterExecutionTargetTimeoutResolution;
|
||||
sessionKey: string;
|
||||
fingerprint: string;
|
||||
agentCommand: string | null;
|
||||
|
|
@ -772,7 +780,14 @@ async function buildRuntime(input: {
|
|||
const requestedModel = asString(config.model, "").trim();
|
||||
const requestedThinkingEffort = normalizeRequestedThinkingEffort(config);
|
||||
const fastMode = acpxAgent === "codex" && config.fastMode === true;
|
||||
const timeoutSec = asNumber(config.timeoutSec, DEFAULT_ACPX_LOCAL_TIMEOUT_SEC);
|
||||
// Resolve the wall-clock timeout through the shared execution-target
|
||||
// resolver so sandbox-backed runs pick up the 4h backstop default while
|
||||
// local/SSH runs keep the historical "0 = no adapter timeout" behavior.
|
||||
const timeoutResolution = resolveAdapterExecutionTargetTimeout(
|
||||
executionTarget,
|
||||
asNumber(config.timeoutSec, DEFAULT_ACPX_LOCAL_TIMEOUT_SEC),
|
||||
);
|
||||
const timeoutSec = timeoutResolution.timeoutSec;
|
||||
const stateDir = path.resolve(asString(config.stateDir, "") || defaultStateDir(agent.companyId, agent.id));
|
||||
await fs.mkdir(stateDir, { recursive: true });
|
||||
|
||||
|
|
@ -940,6 +955,7 @@ async function buildRuntime(input: {
|
|||
requestedThinkingEffort,
|
||||
fastMode,
|
||||
timeoutSec,
|
||||
timeoutResolution,
|
||||
sessionKey,
|
||||
fingerprint,
|
||||
agentCommand,
|
||||
|
|
@ -1258,8 +1274,8 @@ async function emitAcpxFailure(input: {
|
|||
err: unknown;
|
||||
phase: AcpxExecutionPhase;
|
||||
// Replace the err-derived message in both the stderr-tail log header and the
|
||||
// acpx.error payload. Used by the turn path to surface "Timed out after Ns"
|
||||
// instead of the raw underlying error message.
|
||||
// acpx.error payload. Used by the turn path to surface the self-describing
|
||||
// adapter execution timeout message instead of the raw underlying error.
|
||||
messageOverride?: string;
|
||||
}): Promise<{
|
||||
classified: Pick<AdapterExecutionResult, "errorCode" | "errorMeta">;
|
||||
|
|
@ -1383,6 +1399,14 @@ export function createAcpxLocalExecutor(deps: ExecuteDeps = {}) {
|
|||
|
||||
return async function executeAcpxLocal(ctx: AdapterExecutionContext): Promise<AdapterExecutionResult> {
|
||||
const prepared = await buildRuntime({ ctx });
|
||||
// State the effective wall-clock timeout and its source up front so a
|
||||
// later timeout is diagnosable from the run log alone. Goes to stderr:
|
||||
// the acpx stdout log stream carries JSON acpx.* event payloads and must
|
||||
// stay machine-parseable line by line.
|
||||
await ctx.onLog(
|
||||
"stderr",
|
||||
`[paperclip] ${formatAdapterExecutionTimeoutStartLogLine(prepared.timeoutResolution)}\n`,
|
||||
);
|
||||
const warmIdleMs = asNumber(ctx.config.warmHandleIdleMs, DEFAULT_ACPX_LOCAL_WARM_HANDLE_IDLE_MS);
|
||||
await cleanupIdleHandles({ handles: warmHandles, now: now(), idleMs: warmIdleMs });
|
||||
|
||||
|
|
@ -1576,7 +1600,7 @@ export function createAcpxLocalExecutor(deps: ExecuteDeps = {}) {
|
|||
timeout = setTimeout(() => {
|
||||
timedOut = true;
|
||||
controller?.abort();
|
||||
void cancelActiveTurn?.(`Timed out after ${prepared.timeoutSec}s`).catch(() => {});
|
||||
void cancelActiveTurn?.(formatAdapterExecutionTimeoutErrorMessage(prepared.timeoutResolution)).catch(() => {});
|
||||
}, timeoutMs);
|
||||
}
|
||||
const turn = runtime.startTurn({
|
||||
|
|
@ -1656,7 +1680,7 @@ export function createAcpxLocalExecutor(deps: ExecuteDeps = {}) {
|
|||
}
|
||||
|
||||
const errorMessage = timedOut
|
||||
? `Timed out after ${prepared.timeoutSec}s`
|
||||
? formatAdapterExecutionTimeoutErrorMessage(prepared.timeoutResolution)
|
||||
: resultErrorMessage(terminal);
|
||||
const terminalStopReason = terminal.status === "failed" ? terminal.error.message : terminal.stopReason;
|
||||
await emitAcpxLog(ctx, {
|
||||
|
|
@ -1692,7 +1716,9 @@ export function createAcpxLocalExecutor(deps: ExecuteDeps = {}) {
|
|||
};
|
||||
} catch (err) {
|
||||
if (timeout) clearTimeout(timeout);
|
||||
const messageOverride = timedOut ? `Timed out after ${prepared.timeoutSec}s` : undefined;
|
||||
const messageOverride = timedOut
|
||||
? formatAdapterExecutionTimeoutErrorMessage(prepared.timeoutResolution)
|
||||
: undefined;
|
||||
const cancel = cancelActiveTurn as ((reason: string) => Promise<void>) | null;
|
||||
const preEmitMessage =
|
||||
messageOverride ?? (err instanceof Error ? err.message : String(err));
|
||||
|
|
|
|||
Loading…
Reference in New Issue