From 81ecaf0654fe79a2a2dbda6588f033e39ee610e4 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 30 Oct 2026 15:29:43 -0700 Subject: [PATCH] fix(server): bound failed lifecycle verification (#18383) Failed lifecycle verification could allocate another test environment every minute indefinitely. Count failed host verification tests separately from claims, persist a repair wait for confirmed configuration failures or three generic failed tests, and clear it through authorized retry or execution configuration changes. Preserve typed temporary execution-target failures through adapter checks so timeouts, transport failures and interrupted probes keep automatic recovery. Retain existing readiness, authorization, termination and cleanup gates. Document the policy and add real PostgreSQL persistence and actual OpenCode adapter regressions. Verified 398 focused tests (one Windows-only test skipped on macOS), repository-wide typecheck and build, structural checks, fully green current-head CI and Greptile 5/5 with no unresolved comments. Co-Authored-By: Paperclip --- doc/DEVELOPING.md | 26 +++ maps/agent-access-map/agents.md | 4 + maps/feature-map/agents.md | 4 +- .../src/execution-target.test.ts | 55 ++++++ .../adapter-utils/src/execution-target.ts | 86 ++++++--- packages/adapter-utils/src/types.ts | 2 + .../adapters/claude-local/src/server/test.ts | 3 + .../adapters/codex-local/src/server/test.ts | 3 + .../adapters/cursor-local/src/server/test.ts | 3 + .../adapters/gemini-local/src/server/test.ts | 3 + .../adapters/grok-local/src/server/test.ts | 3 + .../adapters/kimi-local/src/server/test.ts | 3 + .../opencode-local/src/server/test.ts | 3 + packages/adapters/pi-local/src/server/test.ts | 3 + packages/shared/src/types/agent-lifecycle.ts | 4 + packages/shared/src/types/agent.ts | 2 + .../agent-harness-verification-remote.test.ts | 67 +++++++ .../agent-harness-verification.test.ts | 7 + .../__tests__/diagnostics.test.ts | 19 +- .../__tests__/verification-retry.test.ts | 182 ++++++++++++++++++ .../agent-lifecycle/adapters/configuration.ts | 3 +- .../agent-lifecycle/adapters/postgres.ts | 15 +- .../agent-lifecycle/application/failure.ts | 13 +- .../agent-lifecycle/application/ports.ts | 4 +- .../agent-lifecycle/application/worker.ts | 15 +- .../services/agent-harness-verification.ts | 3 +- .../components/AgentLifecycleStatus.test.tsx | 6 + ui/src/components/AgentLifecycleStatus.tsx | 4 +- 28 files changed, 502 insertions(+), 43 deletions(-) create mode 100644 server/src/__tests__/agent-harness-verification-remote.test.ts create mode 100644 server/src/modules/agent-lifecycle/__tests__/verification-retry.test.ts diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index d8652f0eaa3..188567eb963 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -903,6 +903,32 @@ npx paperclipai allowed-hostname dotta-macbook-pro ## Agent status recovery +Lifecycle verification stops automatic attempts after missing authentication, +an unavailable model or command, an invalid working directory, or unexpected +probe output. A generic failed test gets up to three failures before automatic +checks stop. The saved `verificationFailures` count only advances when the host +verification test fails; claims deferred before the test do not spend that budget. +An explicit retry or execution configuration change resets that count. +The agent stays in `verifying` +with execution paused and a safe error message. Its saved lifecycle operation +records `retryRequired`; process restarts and queue scans retain that wait. +Changing execution configuration or using **Retry now** clears it and runs the +normal verification again. Name and title edits do not restart verification. +Timeouts, unavailable test environments, plugin outages and unclassified runtime +failures retain automatic retry. Pause, termination, resource cleanup and idle +sleep checks keep their existing safety gates. + +Remote directory and command checks preserve timeout, interruption and transport +failures as typed execution-target failures. Adapters carry that signal in the +optional `AdapterEnvironmentCheck.retryable` field. A retryable error becomes +`test_environment_unavailable` before configuration classification, so it neither +parks verification nor consumes the generic failed-test budget. A completed check +that confirms a missing directory or command still waits for configuration repair. + +The retry persistence tests can use a dedicated local PostgreSQL database ending +in `_test` through `PAPERCLIP_LIFECYCLE_VERIFICATION_RETRY_TEST_DATABASE_URL` when +embedded PostgreSQL is unavailable. The test validates that URL before use. + Heartbeat completion locks the agent row before it reads remaining running work and writes the agent status. A concurrent execution start cannot be overwritten by a completion that read older run state. Run terminalization and agent status diff --git a/maps/agent-access-map/agents.md b/maps/agent-access-map/agents.md index 7ab0cc3db53..ebb8329bee9 100644 --- a/maps/agent-access-map/agents.md +++ b/maps/agent-access-map/agents.md @@ -270,6 +270,10 @@ Policy: Agent creation/configuration uses action grants, resource policies and c consent; some controls require a board actor. Permanent deletion is board-only. Retained case handoff references return `409 agent_case_history_retained` before dependency cleanup; keep the agent terminated. +Failed configuration verification waits for a changed execution configuration +or an authorized explicit retry. That wait does not grant an agent permission +to retry itself, resume execution, or change configuration. Temporary verification +outages retain automatic recovery. Evidence: `source-only` — [implementation](../../server/src/routes/agents.ts#L1395) at the reviewed commit above. diff --git a/maps/feature-map/agents.md b/maps/feature-map/agents.md index f939667b581..c8873671720 100644 --- a/maps/feature-map/agents.md +++ b/maps/feature-map/agents.md @@ -215,7 +215,9 @@ Manual: Save a harmless instruction edit, reload, and verify its revision and ef For failed setup, use an unavailable model or invalid command on a disposable agent. Confirm the profile names the failed verification step and gives a relevant action. -Check that it explains automatic retry and offers **Retry now**. Fix the configuration, +Check that a failed configuration test explains that automatic verification is paused +and offers **Retry now**. Repeated refreshes must not start another test. Timeouts +and temporary test-environment failures still retry automatically. Fix the configuration, retry, and reload; the failure must disappear only after readiness commits. Provider output and credentials must not appear in the lifecycle message. diff --git a/packages/adapter-utils/src/execution-target.test.ts b/packages/adapter-utils/src/execution-target.test.ts index be9dfde46e3..046f2cc80f5 100644 --- a/packages/adapter-utils/src/execution-target.test.ts +++ b/packages/adapter-utils/src/execution-target.test.ts @@ -8,6 +8,9 @@ import { prepareGitHubOperationLaunchers, adapterExecutionTargetUsesManagedHome, ensureAdapterExecutionTargetRuntimeCommandInstalled, + ensureAdapterExecutionTargetDirectory, + ensureAdapterExecutionTargetCommandResolvable, + isAdapterExecutionTargetUnavailableError, resolveAdapterExecutionTargetCwd, runAdapterExecutionTargetProcess, runAdapterExecutionTargetShellCommand, @@ -32,6 +35,58 @@ function sshExecutionTarget(): AdapterSshExecutionTarget { }; } +describe("execution target validation failures", () => { + afterEach(() => vi.restoreAllMocks()); + const result = { exitCode: 0, signal: null, timedOut: false, stdout: "", stderr: "", pid: null, startedAt: "" }; + + it.each(["directory", "command"] as const)("preserves temporary %s probe failures", async check => { + for (const failure of ["timeout", "transport", "interrupted"] as const) { + const target = { kind: "remote" as const, transport: "sandbox" as const, remoteCwd: "/workspace", runner: { + execute: vi.fn(async () => { + if (failure === "transport") throw Object.assign(new Error("private transport detail"), { code: "ECONNRESET" }); + return { ...result, exitCode: null, timedOut: failure === "timeout", signal: failure === "interrupted" ? "SIGTERM" : null }; + }), + } }; + const work = check === "directory" + ? ensureAdapterExecutionTargetDirectory("test", target, "/workspace", { cwd: "/workspace", env: {} }) + : ensureAdapterExecutionTargetCommandResolvable("opencode", target, "/workspace", {}); + const error = await work.catch(error => error); + expect(isAdapterExecutionTargetUnavailableError(error)).toBe(true); + } + }); + + it("preserves SSH disconnections instead of reporting a missing directory", async () => { + vi.spyOn(ssh, "runSshCommand").mockRejectedValue(Object.assign(new Error("private SSH detail"), { code: 255 })); + const error = await ensureAdapterExecutionTargetDirectory("test", sshExecutionTarget(), "/workspace", { + cwd: "/workspace", env: {}, + }).catch(error => error); + expect(isAdapterExecutionTargetUnavailableError(error)).toBe(true); + }); + + it.each(["directory", "command"] as const)("retains a confirmed missing %s as a configuration failure", async check => { + const target = { kind: "remote" as const, transport: "sandbox" as const, remoteCwd: "/workspace", runner: { + execute: vi.fn(async () => ({ ...result, exitCode: 1 })), + } }; + const error = await (check === "directory" + ? ensureAdapterExecutionTargetDirectory("test", target, "/workspace", { cwd: "/workspace", env: {} }) + : ensureAdapterExecutionTargetCommandResolvable("opencode", target, "/workspace", {})).catch(error => error); + expect(error).toBeInstanceOf(Error); + expect(isAdapterExecutionTargetUnavailableError(error)).toBe(false); + }); + + it("does not turn an installation timeout into a permanent missing command", async () => { + const execute = vi.fn().mockResolvedValueOnce({ ...result, exitCode: 1 }) + .mockResolvedValueOnce({ ...result, exitCode: null, timedOut: true }) + .mockResolvedValueOnce({ ...result, exitCode: 1 }); + const target = { kind: "remote" as const, transport: "sandbox" as const, remoteCwd: "/workspace", runner: { execute } }; + const error = await ensureAdapterExecutionTargetCommandResolvable("opencode", target, "/workspace", {}, { + installCommand: "install-fixture", + }).catch(error => error); + expect(execute).toHaveBeenCalledTimes(3); + expect(isAdapterExecutionTargetUnavailableError(error)).toBe(true); + }); +}); + describe("runAdapterExecutionTargetShellCommand", () => { afterEach(() => { vi.restoreAllMocks(); diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index da99bc82917..d94db9f0585 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -735,23 +735,53 @@ export async function ensureAdapterExecutionTargetCommandResolvable( }); } +function executionTargetCheckUnavailable(message: string, cause?: unknown): Error { + return Object.assign(new Error(message, { cause }), { code: "adapter_execution_target_unavailable" }); +} + +/** Preserve target failures before an adapter translates them into a failed check. */ +export function isAdapterExecutionTargetUnavailableError(error: unknown): boolean { + return typeof error === "object" && error !== null && "code" in error + && error.code === "adapter_execution_target_unavailable"; +} + +async function runRemoteExecutionTargetCheck( + target: AdapterExecutionTarget, + description: string, + check: () => Promise, +): Promise { + let result: RunProcessResult; + try { + result = await check(); + } catch (error) { + throw executionTargetCheckUnavailable(`Could not complete ${description}`, error); + } + if (result.timedOut || result.signal || result.exitCode === null + || (target.kind === "remote" && target.transport === "ssh" && result.exitCode === 255)) { + throw executionTargetCheckUnavailable(`${result.timedOut ? "Timed out" : "Could not complete"} ${description}`); + } + return result; +} + async function probeSandboxCommandResolvable( command: string, target: AdapterSandboxExecutionTarget, env: Record, -): Promise<{ resolved: boolean; timedOut: boolean; stderr: string }> { +): Promise<{ resolved: boolean; stderr: string }> { const runner = requireSandboxRunner(target); const probeScript = `command -v ${shellQuote(command)}`; - const result = await runner.execute({ - command: "sh", - args: ["-c", probeScript], - cwd: target.remoteCwd, - env, - timeoutMs: target.timeoutMs ?? 15_000, - }); + const result = await runRemoteExecutionTargetCheck( + target, `checking command "${command}" on sandbox target.`, + () => runner.execute({ + command: "sh", + args: ["-c", probeScript], + cwd: target.remoteCwd, + env, + timeoutMs: target.timeoutMs ?? 15_000, + }), + ); return { - resolved: !result.timedOut && (result.exitCode ?? 1) === 0, - timedOut: result.timedOut, + resolved: result.exitCode === 0, stderr: result.stderr.trim(), }; } @@ -772,15 +802,13 @@ async function ensureSandboxCommandResolvable( // SSH's buildSshSpawnTarget) so this and the hello probe agree on PATH. let probe = await probeSandboxCommandResolvable(command, target, env); if (probe.resolved) return; - if (probe.timedOut) { - throw new Error(`Timed out checking command "${command}" on sandbox target.`); - } // If the caller supplied an install command, attempt the install once via // the sandbox runner (which the sandbox provider wraps in a login shell) // and re-probe before reporting failure. This lets fresh sandbox leases // bring up the CLI before the resolvability gate, mirroring the test path. let installFailureDetail: string | null = null; + let installUnavailable: Error | null = null; if (installCommand) { const runner = requireSandboxRunner(target); const installTimeoutMs = @@ -797,6 +825,9 @@ async function ensureSandboxCommandResolvable( }); if (installResult.timedOut) { installFailureDetail = `install command timed out: ${installCommand}`; + installUnavailable = executionTargetCheckUnavailable("The runtime installation timed out."); + } else if (installResult.signal || installResult.exitCode === null) { + installUnavailable = executionTargetCheckUnavailable("The runtime installation did not complete."); } else if ((installResult.exitCode ?? 0) !== 0) { const tail = (text: string) => text.split(/\r?\n/).filter((line) => line.trim().length > 0).slice(-2).join(" | ").slice(0, 240); @@ -805,12 +836,11 @@ async function ensureSandboxCommandResolvable( } } catch (err) { installFailureDetail = `install command threw: ${err instanceof Error ? err.message : String(err)}`; + installUnavailable = executionTargetCheckUnavailable("The runtime installation was unavailable.", err); } probe = await probeSandboxCommandResolvable(command, target, env); if (probe.resolved) return; - if (probe.timedOut) { - throw new Error(`Timed out checking command "${command}" on sandbox target.`); - } + if (installUnavailable) throw installUnavailable; } const probeStderr = probe.stderr.length > 0 ? ` probe stderr: ${probe.stderr}` : ""; @@ -1270,7 +1300,8 @@ export async function ensureAdapterExecutionTargetFile( * `mkdir -p` (when allowed) followed by a `[ -d ]` check so the result reflects * the directory state inside the environment, not on the Paperclip host. * - * Throws an Error with a human-readable message on failure. + * Throws an Error with a human-readable message on failure. Temporary target + * failures carry a typed code so adapters can retain them as retryable checks. */ export async function ensureAdapterExecutionTargetDirectory( runId: string, @@ -1296,17 +1327,16 @@ export async function ensureAdapterExecutionTargetDirectory( ? `mkdir -p ${quoted} && [ -d ${quoted} ]` : `[ -d ${quoted} ]`; - const result = await runAdapterExecutionTargetShellCommand(runId, target, script, { - cwd: target.kind === "remote" ? target.remoteCwd : cwd, - env: options.env, - timeoutSec: options.timeoutSec ?? 15, - graceSec: options.graceSec ?? 5, - onLog: options.onLog, - }); - - if (result.timedOut) { - throw new Error(`Timed out checking working directory on remote target: "${cwd}"`); - } + const result = await runRemoteExecutionTargetCheck( + target, `checking working directory on remote target: "${cwd}"`, + () => runAdapterExecutionTargetShellCommand(runId, target, script, { + cwd: target.remoteCwd, + env: options.env, + timeoutSec: options.timeoutSec ?? 15, + graceSec: options.graceSec ?? 5, + onLog: options.onLog, + }), + ); if ((result.exitCode ?? 1) !== 0) { const detail = (result.stderr || result.stdout || "").trim(); if (createIfMissing) { diff --git a/packages/adapter-utils/src/types.ts b/packages/adapter-utils/src/types.ts index 37e6fc45a08..5d3557da6e3 100644 --- a/packages/adapter-utils/src/types.ts +++ b/packages/adapter-utils/src/types.ts @@ -308,6 +308,8 @@ export interface AdapterEnvironmentCheck { message: string; detail?: string | null; hint?: string | null; + /** The check could not complete because its execution target was unavailable. */ + retryable?: boolean; } export type AdapterEnvironmentTestStatus = "pass" | "warn" | "fail"; diff --git a/packages/adapters/claude-local/src/server/test.ts b/packages/adapters/claude-local/src/server/test.ts index 40859fa6455..0002cd96060 100644 --- a/packages/adapters/claude-local/src/server/test.ts +++ b/packages/adapters/claude-local/src/server/test.ts @@ -14,6 +14,7 @@ import { import { ensureAdapterExecutionTargetCommandResolvable, ensureAdapterExecutionTargetDirectory, + isAdapterExecutionTargetUnavailableError, runAdapterExecutionTargetProcess, resolveAdapterExecutionTargetCwd, resolveAdapterExecutionTargetCommandForLogs, @@ -114,6 +115,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "claude_cwd_invalid", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Invalid working directory", detail: cwd, @@ -168,6 +170,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "claude_command_unresolvable", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Command is not executable", detail: command, diff --git a/packages/adapters/codex-local/src/server/test.ts b/packages/adapters/codex-local/src/server/test.ts index da22fbd0b13..e4a87070424 100644 --- a/packages/adapters/codex-local/src/server/test.ts +++ b/packages/adapters/codex-local/src/server/test.ts @@ -11,6 +11,7 @@ import { import { ensureAdapterExecutionTargetCommandResolvable, ensureAdapterExecutionTargetDirectory, + isAdapterExecutionTargetUnavailableError, maybeRunSandboxInstallCommand, runAdapterExecutionTargetProcess, describeAdapterExecutionTarget, @@ -308,6 +309,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "codex_cwd_invalid", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Invalid working directory", detail: cwd, @@ -339,6 +341,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "codex_command_unresolvable", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Command is not executable", detail: command, diff --git a/packages/adapters/cursor-local/src/server/test.ts b/packages/adapters/cursor-local/src/server/test.ts index 762177e7d1e..acf138dd659 100644 --- a/packages/adapters/cursor-local/src/server/test.ts +++ b/packages/adapters/cursor-local/src/server/test.ts @@ -13,6 +13,7 @@ import { import { ensureAdapterExecutionTargetCommandResolvable, ensureAdapterExecutionTargetDirectory, + isAdapterExecutionTargetUnavailableError, maybeRunSandboxInstallCommand, runAdapterExecutionTargetProcess, describeAdapterExecutionTarget, @@ -119,6 +120,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "cursor_cwd_invalid", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Invalid working directory", detail: cwd, @@ -173,6 +175,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "cursor_command_unresolvable", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Command is not executable", detail: command, diff --git a/packages/adapters/gemini-local/src/server/test.ts b/packages/adapters/gemini-local/src/server/test.ts index 43572d76840..3e46c55da1b 100644 --- a/packages/adapters/gemini-local/src/server/test.ts +++ b/packages/adapters/gemini-local/src/server/test.ts @@ -16,6 +16,7 @@ import { ensureAdapterExecutionTargetCommandResolvable, maybeRunSandboxInstallCommand, ensureAdapterExecutionTargetDirectory, + isAdapterExecutionTargetUnavailableError, runAdapterExecutionTargetProcess, describeAdapterExecutionTarget, resolveAdapterExecutionTargetCwd, @@ -107,6 +108,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "gemini_cwd_invalid", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Invalid working directory", detail: cwd, @@ -141,6 +143,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "gemini_command_unresolvable", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Command is not executable", detail: command, diff --git a/packages/adapters/grok-local/src/server/test.ts b/packages/adapters/grok-local/src/server/test.ts index 22f163f662b..4df40080bba 100644 --- a/packages/adapters/grok-local/src/server/test.ts +++ b/packages/adapters/grok-local/src/server/test.ts @@ -15,6 +15,7 @@ import { prepareAdapterExecutionTargetRuntime, ensureAdapterExecutionTargetCommandResolvable, ensureAdapterExecutionTargetDirectory, + isAdapterExecutionTargetUnavailableError, resolveAdapterExecutionTargetCwd, runAdapterExecutionTargetProcess, } from "@paperclipai/adapter-utils/execution-target"; @@ -144,6 +145,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "grok_cwd_invalid", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Invalid working directory", detail: cwd, @@ -204,6 +206,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "grok_command_unresolvable", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Command is not executable", detail: command, diff --git a/packages/adapters/kimi-local/src/server/test.ts b/packages/adapters/kimi-local/src/server/test.ts index 5900f9d7546..5f660e4a6ad 100644 --- a/packages/adapters/kimi-local/src/server/test.ts +++ b/packages/adapters/kimi-local/src/server/test.ts @@ -17,6 +17,7 @@ import { ensureAdapterExecutionTargetCommandResolvable, maybeRunSandboxInstallCommand, ensureAdapterExecutionTargetDirectory, + isAdapterExecutionTargetUnavailableError, runAdapterExecutionTargetProcess, describeAdapterExecutionTarget, resolveAdapterExecutionTargetCwd, @@ -156,6 +157,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "kimi_cwd_invalid", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Invalid working directory", detail: cwd, @@ -187,6 +189,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "kimi_command_unresolvable", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Command is not executable", detail: command, diff --git a/packages/adapters/opencode-local/src/server/test.ts b/packages/adapters/opencode-local/src/server/test.ts index fa0823cdc61..7b33a957827 100644 --- a/packages/adapters/opencode-local/src/server/test.ts +++ b/packages/adapters/opencode-local/src/server/test.ts @@ -19,6 +19,7 @@ import { ensureAdapterExecutionTargetCommandResolvable, maybeRunSandboxInstallCommand, ensureAdapterExecutionTargetDirectory, + isAdapterExecutionTargetUnavailableError, runAdapterExecutionTargetProcess, describeAdapterExecutionTarget, resolveAdapterExecutionTargetCwd, @@ -102,6 +103,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "opencode_cwd_invalid", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Invalid working directory", detail: cwd, @@ -219,6 +221,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "opencode_command_unresolvable", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Command is not executable", detail: command, diff --git a/packages/adapters/pi-local/src/server/test.ts b/packages/adapters/pi-local/src/server/test.ts index 370a8c3d51d..fd965241e7e 100644 --- a/packages/adapters/pi-local/src/server/test.ts +++ b/packages/adapters/pi-local/src/server/test.ts @@ -16,6 +16,7 @@ import { ensureAdapterExecutionTargetCommandResolvable, maybeRunSandboxInstallCommand, ensureAdapterExecutionTargetDirectory, + isAdapterExecutionTargetUnavailableError, runAdapterExecutionTargetProcess, describeAdapterExecutionTarget, resolveAdapterExecutionTargetCwd, @@ -115,6 +116,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "pi_cwd_invalid", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Invalid working directory", detail: cwd, @@ -156,6 +158,7 @@ export async function testEnvironment( } catch (err) { checks.push({ code: "pi_command_unresolvable", + ...(isAdapterExecutionTargetUnavailableError(err) ? { retryable: true } : {}), level: "error", message: err instanceof Error ? err.message : "Command is not executable", detail: command, diff --git a/packages/shared/src/types/agent-lifecycle.ts b/packages/shared/src/types/agent-lifecycle.ts index 13a7a829c80..bfb9b6533a9 100644 --- a/packages/shared/src/types/agent-lifecycle.ts +++ b/packages/shared/src/types/agent-lifecycle.ts @@ -13,6 +13,10 @@ export interface AgentLifecycleOperation { leaseOwner?: string; leaseUntil?: string; retryAt?: string; + /** A failed configuration check waits for configuration repair or an explicit retry. */ + retryRequired?: boolean; + /** Failed generic host verification tests for this saved configuration. */ + verificationFailures?: number; attempts: number; responsibleUserId?: string | null; } diff --git a/packages/shared/src/types/agent.ts b/packages/shared/src/types/agent.ts index ba4b594389b..06a6fe90c20 100644 --- a/packages/shared/src/types/agent.ts +++ b/packages/shared/src/types/agent.ts @@ -333,6 +333,8 @@ export interface AdapterEnvironmentCheck { message: string; detail?: string | null; hint?: string | null; + /** The check could not complete because its execution target was unavailable. */ + retryable?: boolean; } export interface AdapterEnvironmentTestResult { diff --git a/server/src/__tests__/agent-harness-verification-remote.test.ts b/server/src/__tests__/agent-harness-verification-remote.test.ts new file mode 100644 index 00000000000..db5c5c01ed1 --- /dev/null +++ b/server/src/__tests__/agent-harness-verification-remote.test.ts @@ -0,0 +1,67 @@ +import { mkdtemp, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import type { AdapterExecutionTarget } from "@paperclipai/adapter-utils/execution-target"; + +// Keep the real directory, command and adapter checks. Runtime staging is a +// separate operation and must not copy files or contact a provider in this test. +vi.mock("@paperclipai/adapter-utils/execution-target", async importOriginal => ({ + ...await importOriginal(), + prepareAdapterExecutionTargetRuntime: async () => ({ + workspaceRemoteDir: "/workspace", runtimeRootDir: "/workspace/.paperclip-runtime", + assetDirs: {}, restoreWorkspace: async () => {}, + }), +})); + +import { testEnvironment } from "@paperclipai/adapter-opencode-local/server"; +import { assertHarnessTestPassed } from "../services/agent-harness-verification.js"; +import { lifecycleFailureMessage, lifecycleVerificationNeedsRetry } from "../modules/agent-lifecycle/application/failure.js"; + +describe("remote harness verification recovery", () => { + let configHome: string; + beforeAll(async () => { configHome = await mkdtemp(path.join(os.tmpdir(), "paperclip-remote-verification-")); }); + afterAll(async () => { if (configHome) await rm(configHome, { recursive: true, force: true }); }); + const context = { stage: "host" as const, phase: "verifying" as const, verificationFailures: 100 }; + + it.each([ + ["directory", "timeout"], ["directory", "transport"], + ["command", "timeout"], ["command", "transport"], + ["directory", "missing"], ["command", "missing"], + ] as const)("retains %s %s through the real adapter check and recovers when repaired", async (check, fault) => { + let failing = true; + const target: AdapterExecutionTarget = { + kind: "remote", transport: "sandbox", remoteCwd: "/workspace", runner: { + execute: async input => { + const script = input.args?.join(" ") ?? ""; + const selected = check === "directory" ? script.includes("[ -d ") : script.includes("command -v "); + if (failing && selected && fault === "transport") { + throw Object.assign(new Error("private provider output"), { code: "ECONNRESET" }); + } + return { + exitCode: failing && selected ? fault === "timeout" ? null : 1 : 0, + timedOut: failing && selected && fault === "timeout", signal: null, stderr: "", pid: null, + startedAt: new Date().toISOString(), + stdout: input.command === "opencode" ? JSON.stringify({ type: "text", part: { text: "hello" } }) : "", + }; + }, + }, + }; + const run = () => testEnvironment({ companyId: "company", adapterType: "opencode_local", executionTarget: target, + config: { command: "opencode", model: "provider/model", env: { XDG_CONFIG_HOME: configHome } } }); + const result = await run(); + const failed = result.checks.find(entry => entry.code === (check === "directory" ? "opencode_cwd_invalid" : "opencode_command_unresolvable")); + expect(failed?.level).toBe("error"); + expect(failed?.retryable === true).toBe(fault !== "missing"); + let error: unknown; + try { assertHarnessTestPassed(result); } catch (caught) { error = caught; } + expect(error).toMatchObject({ code: fault !== "missing" ? "test_environment_unavailable" + : check === "directory" ? "harness_cwd_invalid" : "harness_command_unavailable" }); + expect(lifecycleVerificationNeedsRetry(context, error)).toBe(fault === "missing"); + expect(lifecycleFailureMessage(context, error)).not.toContain("private provider output"); + if (fault !== "missing") expect(lifecycleFailureMessage(context, error)).toContain("retry automatically"); + failing = false; + const recovered = await run(); + expect(() => assertHarnessTestPassed(recovered)).not.toThrow(); + }); +}); diff --git a/server/src/__tests__/agent-harness-verification.test.ts b/server/src/__tests__/agent-harness-verification.test.ts index 6d97edb732c..8d5a68cca20 100644 --- a/server/src/__tests__/agent-harness-verification.test.ts +++ b/server/src/__tests__/agent-harness-verification.test.ts @@ -24,6 +24,13 @@ describe("saved harness verification", () => { expect(error).toMatchObject({ code, message: "The harness test failed" }); } }); + it("preserves a retryable failed check before classifying configuration failures", () => { + const temporary = result("fail", ["opencode_cwd_invalid", "opencode_command_unresolvable"]); + temporary.checks[0] = { ...temporary.checks[0], level: "error", retryable: true }; + expect(() => assertHarnessTestPassed(temporary)).toThrow(expect.objectContaining({ code: "test_environment_unavailable" })); + temporary.checks[0].level = "info"; + expect(() => assertHarnessTestPassed(temporary)).toThrow(expect.objectContaining({ code: "harness_command_unavailable" })); + }); it.each([ ["fail", []], ["warn", ["claude_hello_probe_auth_required"]], ["warn", ["gemini_hello_probe_timed_out"]], ["pass", ["claude_hello_probe_skipped_custom_command"]], diff --git a/server/src/modules/agent-lifecycle/__tests__/diagnostics.test.ts b/server/src/modules/agent-lifecycle/__tests__/diagnostics.test.ts index bcc786dd979..3f59f82d6ff 100644 --- a/server/src/modules/agent-lifecycle/__tests__/diagnostics.test.ts +++ b/server/src/modules/agent-lifecycle/__tests__/diagnostics.test.ts @@ -5,7 +5,7 @@ import type { LifecycleAgent, LifecycleStore, LifecycleDriver } from "../applica import { startAgentLifecycle } from "../index.js"; import { createAgentLifecycleEffects } from "../../../services/agent-lifecycle.js"; import { logger } from "../../../middleware/logger.js"; -import { lifecycleFailureMessage } from "../application/failure.js"; +import { lifecycleFailureMessage, lifecycleVerificationNeedsRetry } from "../application/failure.js"; const agent: LifecycleAgent = { id: "agent", companyId: "company", lifecycleState: "verifying", lifecycleVersion: 3, lifecycleRequiredPluginIds: null, lifecycleError: null, @@ -67,6 +67,20 @@ describe("lifecycle readiness handoff", () => { }); describe("lifecycle failure diagnostics", () => { + it.each(["harness_auth_failed", "harness_model_unavailable", "harness_command_unavailable", "harness_cwd_invalid", "harness_probe_unexpected_output"])("waits for repair only for a host verification failure: %s", code => { + const error = { code }; + expect(lifecycleVerificationNeedsRetry({ stage: "host", phase: "verifying" }, error)).toBe(true); + expect(lifecycleVerificationNeedsRetry({ stage: "plugin", phase: "verifying" }, error)).toBe(false); + expect(lifecycleVerificationNeedsRetry({ stage: "host", phase: "cleaning_up" }, error)).toBe(false); + }); + + it("allows two automatic retries of a generic failed test before waiting for repair", () => { + for (const verificationFailures of [1, 2, 3, 8_000]) { + const context = { stage: "host" as const, phase: "verifying" as const, attempts: 8_000, verificationFailures }; + expect(lifecycleVerificationNeedsRetry(context, { code: "harness_test_failed" })).toBe(verificationFailures >= 3); + } + }); + it.each([ ["host", "harness_model_unavailable", "Execution configuration check failed. The selected model is unavailable."], ["plugin", "required_plugin_unavailable", "Execution configuration check: required plugin step failed. A required plugin is unavailable."], @@ -80,7 +94,8 @@ describe("lifecycle failure diagnostics", () => { if (stage === "select_plugins") driver.requiredPluginIds.mockRejectedValue(error); if (stage === "transition") store.change.mockRejectedValue(error); await worker.process(agent.id); - expect(store.defer).toHaveBeenCalledWith(agent, expect.any(String), expect.stringContaining(expected), expect.any(Date)); + expect(store.defer).toHaveBeenCalledWith(agent, expect.any(String), expect.stringContaining(expected), expect.any(Date), + ...(stage === "host" ? [{ retryRequired: true }] : [])); expect(JSON.stringify(store.defer.mock.calls)).not.toContain("secret provider output"); expect(JSON.stringify(store.defer.mock.calls)).not.toContain("unknown private code"); await worker.stop(); diff --git a/server/src/modules/agent-lifecycle/__tests__/verification-retry.test.ts b/server/src/modules/agent-lifecycle/__tests__/verification-retry.test.ts new file mode 100644 index 00000000000..cc76c705d32 --- /dev/null +++ b/server/src/modules/agent-lifecycle/__tests__/verification-retry.test.ts @@ -0,0 +1,182 @@ +import { randomUUID } from "node:crypto"; +import { agents, companies, createDb, applyPendingMigrations, closeRegisteredClients, type Db } from "@paperclipai/db"; +import { eq, sql } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../../../__tests__/helpers/embedded-postgres.js"; +import { createAgentLifecycle, configureAgentLifecycle } from "../../../services/agent-lifecycle.js"; +import { createAgentLifecycleEffects } from "../../../services/agent-lifecycle.js"; +import { startAgentLifecycle } from "../index.js"; +import { createLifecycleStore } from "../adapters/postgres.js"; +import { createLifecycleWorker } from "../application/worker.js"; +import type { LifecycleAgent } from "../application/ports.js"; +import { resolveExternalTestDatabaseUrl } from "../../../__tests__/helpers/external-test-database.js"; + +const external = resolveExternalTestDatabaseUrl("PAPERCLIP_LIFECYCLE_VERIFICATION_RETRY_TEST_DATABASE_URL"); +const support = external ? { supported: true } : await getEmbeddedPostgresTestSupport(); +(support.supported ? describe : describe.skip)("failed lifecycle verification retries", () => { + let database: Awaited>; + let db: Db; + let companyId: string; + const workers: Array<{ stop(): Promise }> = []; + beforeAll(async () => { + if (external) { await applyPendingMigrations(external); db = createDb(external); } + else { database = await startEmbeddedPostgresTestDatabase("paperclip-verification-retry-"); db = createDb(database.connectionString); } + }, 90_000); + afterAll(async () => { if (external) await closeRegisteredClients(external); else await database?.cleanup(); }); + beforeEach(async () => { + companyId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Verification retry", issuePrefix: `V${companyId.slice(0, 7)}` }); + }); + afterEach(async () => { + for (const worker of workers.splice(0)) await worker.stop(); + // Retire only this test's synthetic records so later coordinator reads stay isolated. + await db.update(agents).set({ lifecycleState: "terminated", status: "terminated" }).where(eq(agents.companyId, companyId)); + }); + const current = async (id: string) => (await db.select().from(agents).where(eq(agents.id, id)))[0]!; + async function failed(code = "harness_test_failed") { + const runHost = vi.fn(async (agent: LifecycleAgent) => { + if (agent.lifecycleState === "verifying") throw Object.assign(new Error("private provider detail"), { code }); + return "complete" as const; + }); + const driver = { requiredPluginIds: async () => [], runHost, runPlugin: async () => "complete" as const }; + const work = configureAgentLifecycle(db, driver); workers.push(work); + const agent = await createAgentLifecycle(db).requestHire(companyId, { name: "Retry fixture", adapterType: "process", adapterConfig: { command: "missing-command" } }); + await work.process(agent.id); + if (code === "harness_test_failed") { + for (let retry = 0; retry < 2; retry++) { + expect((await current(agent.id)).lifecycleOperation?.retryRequired).toBeUndefined(); + await db.update(agents).set({ lifecycleOperation: sql`${agents.lifecycleOperation} - 'retryAt'` }).where(eq(agents.id, agent.id)); + await work.process(agent.id); + } + } + return { agent: await current(agent.id), work, driver, runHost }; + } + + it("persists a failed test across worker restarts without scanning, claiming, or probing it again", async () => { + const { agent, work, driver, runHost } = await failed(); + expect(agent).toMatchObject({ lifecycleState: "verifying", status: "paused", lifecycleOperation: { retryRequired: true } }); + expect(agent.lifecycleOperation).not.toHaveProperty("retryAt"); + expect(agent.lifecycleOperation).not.toHaveProperty("leaseOwner"); + expect(agent.lifecycleError).toContain("Automatic verification is paused"); + expect(agent.lifecycleError).not.toContain("private provider detail"); + await work.stop(); + const restarted = startAgentLifecycle(db, createAgentLifecycleEffects(), driver, () => true); workers.push(restarted); + const calls = runHost.mock.calls.length; + expect(await restarted.hasPending()).toBe(false); + expect(restarted.nextRunAt()).toBeNull(); + await restarted.sweep(); + const store = createLifecycleStore(db, createAgentLifecycleEffects()); + const tomorrow = new Date(Date.now() + 86_400_000); + expect(await store.pending(100, tomorrow)).not.toContain(agent.id); + expect(await store.claim(agent.id, "new-owner", tomorrow)).toBeNull(); + await work.process(agent.id); + expect(runHost).toHaveBeenCalledTimes(calls); + expect((await current(agent.id)).lifecycleOperation).toEqual(agent.lifecycleOperation); + }); + + it("permits an explicit retry of the same operation and completes verification before readiness", async () => { + const { agent, work, runHost } = await failed(); + runHost.mockResolvedValue("complete"); + await createAgentLifecycle(db).retry(agent.id); + expect((await current(agent.id)).lifecycleOperation).not.toHaveProperty("retryRequired"); + expect((await current(agent.id)).lifecycleOperation).not.toHaveProperty("verificationFailures"); + expect((await current(agent.id)).lifecycleOperation!.id).toBe(agent.lifecycleOperation!.id); + await work.process(agent.id); + expect(await current(agent.id)).toMatchObject({ lifecycleState: "ready", status: "idle", lifecycleError: null }); + }); + + it("does not spend verification retries on claims deferred before the host test", async () => { + const [agent] = await db.insert(agents).values({ companyId, name: "Deferred verification", status: "paused", + lifecycleState: "verifying", lifecycleRequiredPluginIds: [], + lifecycleOperation: { id: randomUUID(), hostComplete: false, completedPluginIds: [], attempts: 0 } }).returning(); + const store = createLifecycleStore(db, createAgentLifecycleEffects()); + let allowed = true; + let deferBeforeHost = true; + const runHost = vi.fn(async () => { throw Object.assign(new Error("test failed"), { code: "harness_test_failed" }); }); + const work = createLifecycleWorker({ ...store, async claim(...args) { + const claimed = await store.claim(...args); + if (claimed && deferBeforeHost) allowed = false; + return claimed; + } }, { requiredPluginIds: async () => [], runPlugin: async () => "complete", runHost }, () => {}, () => allowed); + workers.push(work); + const due = async () => { + allowed = true; + await db.update(agents).set({ lifecycleOperation: sql`${agents.lifecycleOperation} - 'retryAt'` }).where(eq(agents.id, agent.id)); + await work.process(agent.id); + }; + await due(); await due(); + expect(runHost).not.toHaveBeenCalled(); + expect((await current(agent.id)).lifecycleOperation).toMatchObject({ attempts: 2 }); + expect((await current(agent.id)).lifecycleOperation).not.toHaveProperty("verificationFailures"); + deferBeforeHost = false; + await due(); + expect((await current(agent.id)).lifecycleOperation).toMatchObject({ attempts: 3, verificationFailures: 1 }); + expect((await current(agent.id)).lifecycleOperation?.retryRequired).toBeUndefined(); + // Another deferred claim preserves the already recorded failed test. + deferBeforeHost = true; await due(); + expect((await current(agent.id)).lifecycleOperation?.verificationFailures).toBe(1); + deferBeforeHost = false; + await due(); + expect((await current(agent.id)).lifecycleOperation?.retryRequired).toBeUndefined(); + await due(); + expect(runHost).toHaveBeenCalledTimes(3); + expect((await current(agent.id)).lifecycleOperation).toMatchObject({ verificationFailures: 3, retryRequired: true }); + }); + + it("recovers from a generic failed test before the automatic retry budget is exhausted", async () => { + const { agent, work, runHost } = await failed("harness_probe_timed_out"); + runHost.mockRejectedValueOnce(Object.assign(new Error("temporary test failure"), { code: "harness_test_failed" })); + await db.update(agents).set({ lifecycleOperation: sql`${agents.lifecycleOperation} - 'retryAt'` }).where(eq(agents.id, agent.id)); + await work.process(agent.id); + expect((await current(agent.id)).lifecycleOperation?.retryRequired).toBeUndefined(); + runHost.mockResolvedValue("complete"); + await db.update(agents).set({ lifecycleOperation: sql`${agents.lifecycleOperation} - 'retryAt'` }).where(eq(agents.id, agent.id)); + await work.process(agent.id); + expect((await current(agent.id)).lifecycleState).toBe("ready"); + }); + + it("rechecks changed execution configuration but leaves a name edit waiting", async () => { + const { agent, work, runHost } = await failed(); + const lifecycle = createAgentLifecycle(db); + await lifecycle.updateConfiguration(agent.id, { name: "Renamed fixture" }); + expect((await current(agent.id)).lifecycleOperation!.retryRequired).toBe(true); + runHost.mockResolvedValue("complete"); + await lifecycle.updateConfiguration(agent.id, { adapterConfig: { command: process.execPath } }); + const changed = await current(agent.id); + expect(changed.lifecycleOperation).not.toHaveProperty("retryRequired"); + expect(changed.lifecycleOperation).not.toHaveProperty("verificationFailures"); + expect(changed.lifecycleOperation!.id).not.toBe(agent.lifecycleOperation!.id); + await work.process(agent.id); + expect((await current(agent.id)).lifecycleState).toBe("ready"); + }); + + it("does not let a stale verification failure park newly saved configuration", async () => { + const { agent, work } = await failed("harness_auth_failed"); + await work.stop(); + const store = createLifecycleStore(db, createAgentLifecycleEffects()); + await store.change(agent.id, "retry"); + const claimed = await store.claim(agent.id, "old-verification", new Date()); + expect(claimed).not.toBeNull(); + await createAgentLifecycle(db).updateConfiguration(agent.id, { adapterConfig: { command: process.execPath } }); + expect(await store.defer(claimed!, "old-verification", "Old failure", new Date(), { retryRequired: true })).toBe(false); + expect((await current(agent.id)).lifecycleOperation?.retryRequired).toBeUndefined(); + }); + + it.each(["harness_probe_timed_out", "test_environment_unavailable", "ECONNRESET", "unclassified"])("keeps automatic recovery for %s", async code => { + const { agent } = await failed(code); + expect(agent.lifecycleOperation).not.toHaveProperty("retryRequired"); + expect(Date.parse(agent.lifecycleOperation!.retryAt!)).toBeGreaterThan(Date.now()); + const store = createLifecycleStore(db, createAgentLifecycleEffects()); + expect(await store.pending(100, new Date(Date.now() + 120_000))).toContain(agent.id); + expect(await store.claim(agent.id, "automatic-retry", new Date(Date.now() + 120_000))).not.toBeNull(); + }); + + it("still completes termination and cleanup when verification needs repair", async () => { + const { agent, work, runHost } = await failed(); + runHost.mockResolvedValue("complete"); + await createAgentLifecycle(db).terminateAgent(agent.id); + await work.process(agent.id); + expect((await current(agent.id)).lifecycleState).toBe("terminated"); + expect(runHost.mock.calls.map(([a]) => a.lifecycleState)).toContain("cleaning_up"); + }); +}); diff --git a/server/src/modules/agent-lifecycle/adapters/configuration.ts b/server/src/modules/agent-lifecycle/adapters/configuration.ts index 7359ddfacf1..80e90dbd71d 100644 --- a/server/src/modules/agent-lifecycle/adapters/configuration.ts +++ b/server/src/modules/agent-lifecycle/adapters/configuration.ts @@ -37,7 +37,8 @@ export async function updateAgentConfiguration(tx: Db, effects: LifecycleEffects ...(!prepareSetup && current.lifecycleHolds.includes("setup") && ["preparing", "verifying"].includes(current.lifecycleState) ? { lifecycleState: "pausing" as const } : {}), lifecycleOperation: { ...current.lifecycleOperation!, id: randomUUID(), hostComplete: false, - completedPluginIds: [], leaseOwner: undefined, leaseUntil: undefined, retryAt: undefined }, + completedPluginIds: [], attempts: 0, leaseOwner: undefined, leaseUntil: undefined, + retryAt: undefined, retryRequired: undefined, verificationFailures: undefined }, } : {}) }).where(eq(agents.id, id)).returning(); await effects.completeConfiguration(tx, current, updated, patch, options, publications); return agentRecordQueries(tx).getById(id); diff --git a/server/src/modules/agent-lifecycle/adapters/postgres.ts b/server/src/modules/agent-lifecycle/adapters/postgres.ts index 0e618f2ae5f..d7f8e77c553 100644 --- a/server/src/modules/agent-lifecycle/adapters/postgres.ts +++ b/server/src/modules/agent-lifecycle/adapters/postgres.ts @@ -17,6 +17,7 @@ type ChangeOptions = { version?: number; owner?: string; reason?: string }; export const pendingLifecycleWork = and( inArray(agents.lifecycleState, ["preparing", "verifying", "pausing", "resuming", "terminating", "cleaning_up"]), sql`not (${agents.lifecycleState} = 'verifying' and ${agents.lifecycleHolds} @> '["setup"]'::jsonb)`, + sql`not (${agents.lifecycleState} = 'verifying' and coalesce(${agents.lifecycleOperation}->>'retryRequired', 'false') = 'true')`, ); /** The caller owns the accounting transaction and has locked this agent. */ @@ -33,7 +34,7 @@ async function changeLockedAgent(tx: Db, effects: LifecycleEffects, agent: Lifec if (agent.lifecycleOperation?.leaseUntil && Date.parse(agent.lifecycleOperation.leaseUntil) > Date.now()) return agent; await effects.notifyWork(tx); const [updated] = await tx.update(agents).set({ lifecycleError: null, - lifecycleOperation: sql`${agents.lifecycleOperation} - 'retryAt' - 'leaseOwner' - 'leaseUntil'`, updatedAt: new Date() }).where(eq(agents.id, agent.id)).returning(); + lifecycleOperation: sql`${agents.lifecycleOperation} - 'retryAt' - 'retryRequired' - 'verificationFailures' - 'leaseOwner' - 'leaseUntil'`, updatedAt: new Date() }).where(eq(agents.id, agent.id)).returning(); return updated; } if (command === "resume" && !["ready", "paused", "pausing", "resuming"].includes(agent.lifecycleState)) transition(agent.lifecycleState, command); @@ -177,11 +178,17 @@ export function createLifecycleStore(db: Db, effects: LifecycleEffects): Lifecyc sql`${agents.lifecycleOperation}->>'leaseOwner' = ${owner}`)).returning({ id: agents.id }); return rows.length > 0; }, - async defer(agent, owner, error, now) { + async defer(agent, owner, error, now, options) { + const retry: Partial = options?.retryRequired && agent.lifecycleState === "verifying" + ? { retryRequired: true } + : { retryAt: new Date(now.getTime() + (error ? 60_000 : 2_000)).toISOString() }; + if (options?.verificationFailures !== undefined && agent.lifecycleState === "verifying") { + retry.verificationFailures = options.verificationFailures; + } const rows = await db.update(agents).set({ lifecycleError: error, - lifecycleOperation: sql`(${agents.lifecycleOperation} - 'leaseOwner' - 'leaseUntil') - || jsonb_build_object('retryAt', ${new Date(now.getTime() + (error ? 60_000 : 2_000)).toISOString()}::text)`, + lifecycleOperation: sql`(${agents.lifecycleOperation} - 'leaseOwner' - 'leaseUntil' - 'retryAt' - 'retryRequired') + || ${JSON.stringify(retry)}::jsonb`, updatedAt: now, }).where(and(eq(agents.id, agent.id), eq(agents.lifecycleVersion, agent.lifecycleVersion), sql`${agents.lifecycleOperation}->>'leaseOwner' = ${owner}`)).returning({ id: agents.id }); diff --git a/server/src/modules/agent-lifecycle/application/failure.ts b/server/src/modules/agent-lifecycle/application/failure.ts index 0c5da80e34b..55b6e5dc7c6 100644 --- a/server/src/modules/agent-lifecycle/application/failure.ts +++ b/server/src/modules/agent-lifecycle/application/failure.ts @@ -15,6 +15,16 @@ const reasons = { } as const; const transientCodes = new Set(["ECONNREFUSED", "ECONNRESET", "ETIMEDOUT", "40001", "40P01", "53300", "57P01"]); +const configurationFailures = new Set([ + "harness_auth_failed", "harness_model_unavailable", + "harness_command_unavailable", "harness_cwd_invalid", "harness_probe_unexpected_output", +]); + +export function lifecycleVerificationNeedsRetry(context: LifecycleFailure, error: unknown): boolean { + const code = lifecycleFailureCode(error) ?? ""; + return context.phase === "verifying" && context.stage === "host" + && (configurationFailures.has(code) || (code === "harness_test_failed" && (context.verificationFailures ?? 0) >= 3)); +} export function lifecycleFailureCode(error: unknown): string | undefined { const candidate = error as { code?: unknown; cause?: { code?: unknown } } | null; @@ -36,5 +46,6 @@ export function lifecycleFailureMessage(context: LifecycleFailure, error: unknow const code = lifecycleFailureCode(error); const reason = code && Object.hasOwn(reasons, code) ? reasons[code as keyof typeof reasons] : "Paperclip could not complete this step. It will retry automatically. If this continues, contact your instance administrator."; - return `${step} failed. ${reason}`; + return `${step} failed. ${reason}${lifecycleVerificationNeedsRetry(context, error) + ? " Automatic verification is paused. Update Configuration or choose Retry now." : ""}`; } diff --git a/server/src/modules/agent-lifecycle/application/ports.ts b/server/src/modules/agent-lifecycle/application/ports.ts index 6ff3d170109..2fbcd33c8cc 100644 --- a/server/src/modules/agent-lifecycle/application/ports.ts +++ b/server/src/modules/agent-lifecycle/application/ports.ts @@ -20,7 +20,7 @@ export interface LifecycleStore { setRequiredPlugins(agent: LifecycleAgent, owner: string, pluginIds: string[]): Promise; completeHost(agent: LifecycleAgent, owner: string): Promise; completePlugin(agent: LifecycleAgent, owner: string, pluginId: string): Promise; - defer(agent: LifecycleAgent, owner: string, error: string | null, now: Date): Promise; + defer(agent: LifecycleAgent, owner: string, error: string | null, now: Date, options?: { retryRequired: boolean; verificationFailures?: number }): Promise; pending(limit: number, now: Date): Promise; } @@ -40,5 +40,7 @@ export interface LifecycleFailure { phase?: AgentLifecycleState; operationId?: string; version?: number; + attempts?: number; + verificationFailures?: number; pluginId?: string; } diff --git a/server/src/modules/agent-lifecycle/application/worker.ts b/server/src/modules/agent-lifecycle/application/worker.ts index 3c362dcbb11..02d2804f5d6 100644 --- a/server/src/modules/agent-lifecycle/application/worker.ts +++ b/server/src/modules/agent-lifecycle/application/worker.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import type { LifecycleDriver, LifecycleStore, LifecycleFailure } from "./ports.js"; import { isLifecycleWorkPending } from "../domain/policy.js"; -import { lifecycleFailureMessage } from "./failure.js"; +import { lifecycleFailureCode, lifecycleFailureMessage, lifecycleVerificationNeedsRetry } from "./failure.js"; /** Database leases protect each attempt across server processes. */ export function createLifecycleWorker(store: LifecycleStore, driver: LifecycleDriver, reportFailure: (context: LifecycleFailure, error: unknown) => void, canRun = () => true) { @@ -20,7 +20,8 @@ export function createLifecycleWorker(store: LifecycleStore, driver: LifecycleDr const agent = await attempt({ stage: "claim", agentId: id }, () => store.claim(id, owner, new Date())); if (!agent || !isLifecycleWorkPending(agent.lifecycleState)) return; const context = { agentId: agent.id, companyId: agent.companyId, phase: agent.lifecycleState, - operationId: agent.lifecycleOperation?.id, version: agent.lifecycleVersion }; + operationId: agent.lifecycleOperation?.id, version: agent.lifecycleVersion, + attempts: agent.lifecycleOperation?.attempts }; let failureContext: LifecycleFailure = { ...context, stage: "host" }; const runStep = (stage: LifecycleFailure["stage"], work: () => Promise, pluginId?: string) => { const stepContext = { ...context, stage, ...(pluginId ? { pluginId } : {}) }; @@ -70,8 +71,16 @@ export function createLifecycleWorker(store: LifecycleStore, driver: LifecycleDr } if (changed && !isLifecycleWorkPending(changed.lifecycleState)) return; } catch (error) { + // Claims can defer before runHost. Only a failed host test spends this budget. + const verificationFailures = failureContext.stage === "host" && agent.lifecycleState === "verifying" + && lifecycleFailureCode(error) === "harness_test_failed" + ? (agent.lifecycleOperation?.verificationFailures ?? 0) + 1 : undefined; + failureContext = { ...failureContext, verificationFailures }; const message = lifecycleFailureMessage(failureContext, error); - if (!await runStep("defer", () => store.defer(agent, owner, message, new Date()))) continue; + const retryRequired = lifecycleVerificationNeedsRetry(failureContext, error); + if (!await runStep("defer", () => retryRequired || verificationFailures !== undefined + ? store.defer(agent, owner, message, new Date(), { retryRequired, ...(verificationFailures !== undefined ? { verificationFailures } : {}) }) + : store.defer(agent, owner, message, new Date()))) continue; return; } finally { clearInterval(renewal); diff --git a/server/src/services/agent-harness-verification.ts b/server/src/services/agent-harness-verification.ts index c84194a6c78..ed585faf18d 100644 --- a/server/src/services/agent-harness-verification.ts +++ b/server/src/services/agent-harness-verification.ts @@ -23,7 +23,8 @@ export function assertHarnessTestPassed(result: AdapterEnvironmentTestResult) { const incomplete = !verified && (result.status === "warn" || result.checks.some(check => check.code.includes("hello_probe"))); if (result.status === "fail" || authenticationFailed || incomplete) { const has = (suffix: string) => result.checks.some(check => check.code.endsWith(suffix)); - const code = authenticationFailed ? "harness_auth_failed" + const code = result.checks.some(check => check.level === "error" && check.retryable === true) ? "test_environment_unavailable" + : authenticationFailed ? "harness_auth_failed" : has("_hello_probe_model_unavailable") || has("_model_invalid") ? "harness_model_unavailable" : has("_command_unresolvable") ? "harness_command_unavailable" : has("_cwd_invalid") ? "harness_cwd_invalid" diff --git a/ui/src/components/AgentLifecycleStatus.test.tsx b/ui/src/components/AgentLifecycleStatus.test.tsx index e2fe7baf497..0162f4c62ff 100644 --- a/ui/src/components/AgentLifecycleStatus.test.tsx +++ b/ui/src/components/AgentLifecycleStatus.test.tsx @@ -26,6 +26,12 @@ it.each([undefined, "ready", "paused", "pending_approval", "terminated", "reject expect(agentLifecycleRefetchInterval(lifecycleState)).toBe(30_000); }); +it("does not promise automatic retry of a failed configuration check", () => { + const html = renderToStaticMarkup( {}} retryPending={false} />); + expect(html).toContain("Retry now"); + expect(html).not.toContain("Paperclip retries automatically"); +}); + it("retries a failed step once while pending and removes the failure after recovery", async () => { const container = document.createElement("div"); const root = createRoot(container); diff --git a/ui/src/components/AgentLifecycleStatus.tsx b/ui/src/components/AgentLifecycleStatus.tsx index 65998141514..622281a8ca7 100644 --- a/ui/src/components/AgentLifecycleStatus.tsx +++ b/ui/src/components/AgentLifecycleStatus.tsx @@ -64,6 +64,8 @@ export function AgentLifecycleStatus({ agent, onRetry, retryPending, refreshErro {retryPending ? "Retrying…" : "Retry now"} } - {phase && agent.lifecycleError &&

Paperclip retries automatically; Retry now checks again immediately.

} + {phase && agent.lifecycleError &&

{agent.lifecycleState === "verifying" + ? "Retry now checks the saved configuration again immediately." + : "Paperclip retries automatically; Retry now checks again immediately."}

} ; }