mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(recovery): settle a drained run's hold once its controller is proven gone (#20496)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When the control plane restarts, an interrupted run must continue on its own. A user must not have to press Retry because a deploy happened > - A graceful drain (SIGTERM) marks a preparing legacy run `interrupted` and exits, but the run keeps its legacy controller lease for up to 60 s more > - After the restart, `settleUnrecoverableExecutions` skips the run's recovery hold while that lease is live, in case a living controller is still unwinding a Stop > - The controller is the drained process, which is gone. For up to a minute, the task shows "Couldn't finish this attempt" with a Retry button, before the automatic continuation starts > - This pull request lets the disposition proceed when the shared owner prover proves the lease's recorded controller process gone, and expires that exact lease, so the held-preparation continuation also starts at once > - The benefit is that a run drained during workspace preparation continues automatically right after the restart, without a Retry click ## Linked Issues or Issue Description **Subsystem affected** Server execution recovery: the disposition of `legacy_execution_requires_reconciliation` holds (`server/src/services/execution-recovery-resolution.ts`). **Problem or motivation** The restart-chaos harness can now restart the server while it is in an S3 transfer (#20451). The wave ran on Daytona with `--storage s3 --storage-trigger snapshot-download`. A K2 drain interrupted a C1 follow-up run while its workspace snapshot was downloading, then a second K2 drain interrupted the automatic retry during its restore. The task then showed the board-owned "Couldn't finish this attempt. Your message is saved. Retry" state, and the harness had to click Retry (`R3:retry_needed:C1:K2`). A `--keep` replay reproduced it. The kept database shows a run drained at 07:08:05 with `controller_lease_expires_at` 07:09:04.98. The server was ready again at 07:08:45. Its disposition did not settle the hold until 07:09:14.8. The automatic continuation was then delegated, because the user's Retry had already started a run. **Proposed solution** Before the disposition transaction, ask the shared owner prover (`legacyControllerProvenGone`) whether the live lease's recorded controller process is gone. For that exact controller boot id only, skip the wait and expire the lease in the same transaction (compare-and-swap on run, company and boot id). The later checks (`isInterruptedNativePreparation`, `isFailedNativePreparation`, the held-preparation settlement) then see an expired lease and schedule the `interrupted_preparation` successor. A lease held by a live process, including this one, still blocks as before. **Alternatives considered** Expiring the lease in the drain itself. The drain runs before the process exits, so the owner proof after the restart is the safer evidence. ## What Changed - `legacy-controller-lease.ts`: new `provenGoneLegacyControllerBootId(db, companyId, runId)`. It returns the boot id of a legacy run's unexpired controller lease when the shared prover proves that controller's recorded process gone, else null. - `execution-recovery-resolution.ts`: `settleUnrecoverableExecutions` computes that boot id before its transaction. When the live lease's boot id matches, it expires that lease and settles the hold instead of waiting. - New `execution-recovery-dead-controller.test.ts` (embedded Postgres): a drained run whose controller process has exited is settled at once, its lease is expired, and `isInterruptedNativePreparation` turns true (continuation eligible) while the original lease would still run for about 55 s. A lease held by the current process still waits and keeps its expiry. ## Verification - Red/green: `npx vitest run src/services/execution-recovery-dead-controller.test.ts` fails on master (the hold stays `active`) and passes with the fix. - Related suites pass: `task-retry`, `execution-control-work`, `heartbeat-process-recovery` and `native-startup-retry.integration` with the new file (5 files, 573 tests). - Live: the Daytona `--storage s3 --storage-trigger snapshot-download` rerun on a build with this fix will be posted as a comment. ## Risks - Low. The change only removes a wait when the shared owner prover returns `gone` for the exact recorded controller. An unprovable or alive controller keeps the old behaviour. The prover runs outside the row-locked transaction, and only for candidates whose lease is still live. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
be34a2d9d2
commit
93faca77ef
4 files changed
+119
-6
No files matched your search
@@ -7,7 +7,7 @@ import { isUuidLike } from "@paperclipai/shared";
|
||||
import { claimedAdapterType } from "./conversation-continuation.js";
|
||||
import { hasRemoteTerminationReceipt } from "./remote-execution-termination.js";
|
||||
import { canContinueCancelledRun, readRunCancellation } from "./run-cancellation.js";
|
||||
import { legacyControllerBootId } from "./legacy-controller-lease.js";
|
||||
import { legacyControllerBootId, legacyControllerProvenGone } from "./legacy-controller-lease.js";
|
||||
import { hasRequiredWorkspaceRecovery } from "./workspace-restore-recovery-state.js";
|
||||
import { hasVerifiedWorkspaceRepair } from "./workspace-recovery-receipt.js";
|
||||
import { readFailedPreparationSettlement } from "./failed-preparation-settlement.js";
|
||||
@@ -23,6 +23,19 @@ type Run = Pick<typeof heartbeatRuns.$inferSelect,
|
||||
type Coordinator = typeof nativeRunFinalizations.$inferSelect;
|
||||
const EMPTY_LOG_SHA256 = createHash("sha256").digest("hex");
|
||||
|
||||
/**
|
||||
* An unexpired legacy controller lease means the old preparer may still be
|
||||
* unwinding, unless the shared owner prover proves that recorded controller
|
||||
* process gone (a drained or killed server whose replacement is now running).
|
||||
*/
|
||||
async function preparerMayStillUnwind(run: Run & { controllerProcess?: Record<string, unknown> | null }): Promise<boolean> {
|
||||
if (!run.controllerLeaseExpiresAt || run.controllerLeaseExpiresAt <= new Date()) return false;
|
||||
if (run.runtimeMode !== "legacy" || !run.controllerBootId || run.controllerBootId === legacyControllerBootId
|
||||
|| !run.controllerProcess) return true;
|
||||
return !(await legacyControllerProvenGone({ runtimeMode: run.runtimeMode, controllerBootId: run.controllerBootId,
|
||||
controllerProcess: run.controllerProcess }));
|
||||
}
|
||||
|
||||
export async function isFailedNativePreparation(db: Db, run: Run, coordinator: Coordinator | undefined) {
|
||||
return failedNativePreparation(db, run, coordinator, false);
|
||||
}
|
||||
@@ -42,7 +55,7 @@ async function failedNativePreparation(db: Db, run: Run, coordinator: Coordinato
|
||||
|| run.processPid || run.processGroupId || run.processStartedAt || run.sessionIdAfter
|
||||
|| run.nativeIssueId || run.nativeSessionId || run.runnerProfileJson?.nativeExecutionInput != null
|
||||
|| run.runnerProfileJson?.nativeWorkspaceSync != null
|
||||
|| run.controllerLeaseExpiresAt && run.controllerLeaseExpiresAt > new Date()) return false;
|
||||
|| await preparerMayStillUnwind(run)) return false;
|
||||
const leases = await db.select().from(environmentLeases).where(and(
|
||||
eq(environmentLeases.companyId, run.companyId), eq(environmentLeases.heartbeatRunId, run.id),
|
||||
));
|
||||
@@ -135,7 +148,7 @@ export async function isInterruptedNativePreparation(db: Db, run: Run): Promise<
|
||||
const recovery = run.resultJson?.executionRecovery as Record<string, unknown> | undefined;
|
||||
const settled = recovery?.kind === "bootstrap" && recovery.providerWorkStarted === false;
|
||||
if (!isInterruptedNativePreparationCandidate(run)
|
||||
|| run.controllerLeaseExpiresAt && run.controllerLeaseExpiresAt > new Date()
|
||||
|| await preparerMayStillUnwind(run)
|
||||
|| !settled && (!run.controllerBootId || run.controllerBootId === legacyControllerBootId)) return false;
|
||||
const [coordinator] = await db.select({ id: nativeRunFinalizations.runId }).from(nativeRunFinalizations).where(and(
|
||||
eq(nativeRunFinalizations.companyId, run.companyId), eq(nativeRunFinalizations.runId, run.id),
|
||||
@@ -279,7 +292,7 @@ export async function isCancelledNativeStartup(db: Db, run: Run, coordinator: Co
|
||||
if (!beforeSelection && !neverClaimed) return false;
|
||||
const settled = beforeDispatch || unclaimedWorkspaceRetry || typeof run.resultJson?.startupPreparationSettledAt === "string";
|
||||
// The old preparer can still be unwinding even though the run is terminal.
|
||||
if (run.controllerLeaseExpiresAt && run.controllerLeaseExpiresAt > new Date()) return false;
|
||||
if (await preparerMayStillUnwind(run)) return false;
|
||||
const leases = await db.select().from(environmentLeases).where(and(
|
||||
eq(environmentLeases.companyId, run.companyId), eq(environmentLeases.heartbeatRunId, run.id),
|
||||
));
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { agents, companies, createDb, heartbeatRuns, issueRecoveryActions, issues } from "@paperclipai/db";
|
||||
import { startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js";
|
||||
import { currentControllerProcessStamp } from "./controller-process-identity.js";
|
||||
import { isInterruptedNativePreparation } from "./cancelled-native-startup.js";
|
||||
import { settleUnrecoverableExecutions } from "./execution-recovery-resolution.js";
|
||||
import { legacyControllerBootId } from "./legacy-controller-lease.js";
|
||||
|
||||
describe("automatic recovery after a drained legacy controller", () => {
|
||||
let postgres: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
||||
let db: ReturnType<typeof createDb>;
|
||||
beforeAll(async () => {
|
||||
postgres = await startEmbeddedPostgresTestDatabase("dead-controller-");
|
||||
db = createDb(postgres.connectionString);
|
||||
}, 30_000);
|
||||
afterAll(async () => { await db?.$client.end(); await postgres?.cleanup(); });
|
||||
|
||||
function exitedPid(): number {
|
||||
const child = spawnSync(process.execPath, ["-e", "process.stdout.write(String(process.pid))"], { encoding: "utf8" });
|
||||
return Number(child.stdout);
|
||||
}
|
||||
|
||||
async function seedDrainedRun(controller: "exited" | "current", ending: "drained" | "killed" = "drained") {
|
||||
const companyId = randomUUID(), issueId = randomUUID(), agentId = randomUUID(), runId = randomUUID();
|
||||
const live = currentControllerProcessStamp();
|
||||
const bootId = controller === "current" ? legacyControllerBootId : randomUUID();
|
||||
const controllerProcess = controller === "current" ? live
|
||||
: { bootId, pid: exitedPid(), hostHash: live.hostHash, processStartedAt: new Date(Date.now() - 120_000).toISOString() };
|
||||
await db.insert(companies).values({ id: companyId, name: "Drain company", issuePrefix: `D${companyId.slice(0, 6)}` });
|
||||
await db.insert(agents).values({ id: agentId, companyId, name: "Worker", adapterType: "paperclip_runner", status: "idle" });
|
||||
await db.insert(issues).values({ id: issueId, companyId, title: "Follow up", status: "in_progress", assigneeAgentId: agentId });
|
||||
await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, issueId, status: ending === "drained" ? "interrupted" : "failed",
|
||||
errorCode: ending === "drained" ? "server_shutdown_interrupted" : "process_lost", runtimeMode: "legacy", executionStage: "preparing",
|
||||
contextSnapshot: { issueId }, finishedAt: new Date(), runnerProfileJson: { adapterDispatch: { adapterType: "paperclip_runner" } }, controllerBootId: bootId, controllerProcess,
|
||||
controllerLeaseExpiresAt: new Date(Date.now() + 55_000),
|
||||
resultJson: { stopReason: "interrupted", executionRecovery: { kind: "bootstrap", providerWorkStarted: false } } });
|
||||
const [action] = await db.insert(issueRecoveryActions).values({ companyId, sourceIssueId: issueId, kind: "active_run_watchdog",
|
||||
status: "active", ownerType: "board", returnOwnerAgentId: agentId, cause: "legacy_execution_requires_reconciliation",
|
||||
fingerprint: `legacy-execution:${runId}`, nextAction: "Reconcile the interrupted execution.",
|
||||
evidence: { runId, originalFailureCode: "server_shutdown_interrupted" } }).returning();
|
||||
return { actionId: action!.id, runId };
|
||||
}
|
||||
|
||||
it("settles the hold at once when the drained controller process is proven gone", async () => {
|
||||
const { actionId, runId } = await seedDrainedRun("exited");
|
||||
const [before] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId));
|
||||
expect(await isInterruptedNativePreparation(db, before!)).toBe(true);
|
||||
await settleUnrecoverableExecutions(db);
|
||||
const [action] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.id, actionId));
|
||||
expect(action).toMatchObject({ status: "resolved", outcome: "blocked" });
|
||||
expect(action!.evidence.automaticRecovery).toMatchObject({ policy: "preserve_without_replay_v1", replay: "blocked" });
|
||||
const [after] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId));
|
||||
expect(after!.controllerLeaseExpiresAt!.getTime()).toBeLessThanOrEqual(Date.now());
|
||||
expect(await isInterruptedNativePreparation(db, after!)).toBe(true);
|
||||
});
|
||||
|
||||
it("still waits for a live controller lease held by this process", async () => {
|
||||
const { actionId, runId } = await seedDrainedRun("current");
|
||||
await settleUnrecoverableExecutions(db);
|
||||
const [action] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.id, actionId));
|
||||
expect(action).toMatchObject({ status: "active", resolvedAt: null });
|
||||
const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId));
|
||||
expect(run!.controllerLeaseExpiresAt!.getTime()).toBeGreaterThan(Date.now());
|
||||
});
|
||||
|
||||
it.each(["exited", "current"] as const)("treats a killed preparation with a %s controller's live lease as interrupted only when that controller is gone", async controller => {
|
||||
const { runId } = await seedDrainedRun(controller, "killed");
|
||||
const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, runId));
|
||||
expect(run!.controllerLeaseExpiresAt!.getTime()).toBeGreaterThan(Date.now());
|
||||
expect(await isInterruptedNativePreparation(db, run!)).toBe(controller === "exited");
|
||||
});
|
||||
});
|
||||
@@ -35,6 +35,7 @@ import { isSupersededConversationRun } from "./agent-conversations.js";
|
||||
import { hasRequiredWorkspaceRecovery } from "./workspace-restore-recovery-state.js";
|
||||
import { hasRemoteTerminationReceipt } from "./remote-execution-termination.js";
|
||||
import { readSystemRecovery } from "./system-recovery-state.js";
|
||||
import { provenGoneLegacyControllerBootId } from "./legacy-controller-lease.js";
|
||||
import { alias } from "drizzle-orm/pg-core";
|
||||
|
||||
/** An operator records observed outcomes; this is not permission to blindly retry. */
|
||||
@@ -583,6 +584,7 @@ export async function settleUnrecoverableExecutions(
|
||||
for (const { action: candidate } of candidates) {
|
||||
const runId = candidate.evidence.runId;
|
||||
if (typeof runId !== "string") continue;
|
||||
const goneControllerBootId = await provenGoneLegacyControllerBootId(db, candidate.companyId, runId).catch(() => null);
|
||||
try {
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.execute(
|
||||
@@ -660,8 +662,16 @@ export async function settleUnrecoverableExecutions(
|
||||
return;
|
||||
}
|
||||
// A legacy executor still holding its controller lease may be unwinding
|
||||
// a Stop and can prove that no adapter started. Let it settle first.
|
||||
if (run.runtimeMode === "legacy" && run.controllerLeaseExpiresAt && run.controllerLeaseExpiresAt > now) return;
|
||||
// a Stop and can prove that no adapter started. Let it settle first,
|
||||
// unless its recorded process is proven gone (a drained server): then
|
||||
// expire that exact lease so every later preparation check sees it.
|
||||
if (run.runtimeMode === "legacy" && run.controllerLeaseExpiresAt && run.controllerLeaseExpiresAt > now) {
|
||||
if (goneControllerBootId === null || run.controllerBootId !== goneControllerBootId) return;
|
||||
await tx.update(heartbeatRuns).set({ controllerLeaseExpiresAt: now }).where(and(
|
||||
eq(heartbeatRuns.id, run.id), eq(heartbeatRuns.companyId, run.companyId),
|
||||
eq(heartbeatRuns.controllerBootId, goneControllerBootId)));
|
||||
run.controllerLeaseExpiresAt = now;
|
||||
}
|
||||
// Adoption transfers maintenance to the retained-workspace owner.
|
||||
// This compatibility resolver must not close its hold or relabel it
|
||||
// as a user repair merely because old return-owner metadata is absent.
|
||||
|
||||
@@ -177,6 +177,21 @@ export async function legacyControllerProvenGone(
|
||||
return stamp !== null && (await proveOwnerGone(stamp)).verdict === "gone";
|
||||
}
|
||||
|
||||
/**
|
||||
* The boot id of a run's legacy controller whose lease has not expired yet but
|
||||
* whose recorded process is proven gone, or null. A drained server terminalizes
|
||||
* its runs and exits with their leases unexpired; nothing of that controller is
|
||||
* left to unwind a Stop.
|
||||
*/
|
||||
export async function provenGoneLegacyControllerBootId(db: Db, companyId: string, runId: string): Promise<string | null> {
|
||||
const [run] = await db.select({ runtimeMode: heartbeatRuns.runtimeMode, controllerBootId: heartbeatRuns.controllerBootId,
|
||||
controllerProcess: heartbeatRuns.controllerProcess }).from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.id, runId), eq(heartbeatRuns.companyId, companyId), eq(heartbeatRuns.runtimeMode, "legacy"),
|
||||
gt(heartbeatRuns.controllerLeaseExpiresAt, sql`clock_timestamp()`)));
|
||||
if (!run?.controllerBootId) return null;
|
||||
return await legacyControllerProvenGone(run) ? run.controllerBootId : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Settle of the `legacy_controller_lease` ownership kind. The shared
|
||||
* reconciler calls it only after `proveOwnerGone` proved the recorded
|
||||
|
||||
Reference in new issue
Block a user