From db41bfcb382ad02b16e266f5f2f3695c34e575c4 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 22:07:00 -0600 Subject: [PATCH] fix(recovery): let an accepted Retry finish after a killed copyback owner (#19721) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run saves the agent's workspace files after the provider turn (copyback). One controller owns copyback at a time and records a durable owner receipt on the run (`nativeWorkspaceFinalizationOwner`) > - When copyback of an accepted result fails, retained saved-work recovery exports the files from the kept sandbox. A task Retry waits for that recovery before it continues > - A control-plane restart can kill the controller while it owns copyback. The dead controller can never confirm its own stop, and nothing in the retained recovery path could release its receipt > - The recovery worker then spent one attempt on each recheck of that receipt until it was exhausted, and the accepted Retry kept saying "Paperclip is finishing the saved result" with no run. The task waited forever > - This pull request stops spending attempts on that recheck, names the real wait, and lets the board release the exact receipt so the same recovery and the same Retry continue > - The benefit is that an accepted Retry never waits silently: it finishes, or it shows one honest action that makes it finish ## Linked Issues or Issue Description Refs #19717 (same-host automatic proof for a killed copyback owner; this PR covers the retained-recovery path and the case where no automatic proof is possible) Refs #19713, #19609, #19694 (related restart-recovery fixes; no overlap in files except this doc) **What happened?** Two Daytona restart-chaos waves. In each, a native run's copyback failed and the run failed with `native_workspace_sync_out_retry_exhausted`. The agent's result was accepted. Retained recovery started. A SIGKILL of the control plane landed while a recovery attempt owned copyback: - Wave s13, task t06 (Claude, crash loop): Retry was accepted at 02:30:35. A SIGKILL landed 2 s later. All later attempts failed with `copyback_stop_unconfirmed`. After 6 attempts the recovery was exhausted. The Retry showed `retryWait.reason = finalizing_saved_result` for more than 15 minutes. No run was created. - Wave s12, task t01 (Claude): attempt 1 failed with "Workspace checkpoint payload mismatch". The SIGKILL at 01:43:54 killed the owner of attempt 2. Attempts 3 to 5 failed with `copyback_stop_unconfirmed`. The Retry was accepted with HTTP 2xx and produced no run. In both kept databases the run carries an owner receipt for the killed process. **Steps to reproduce** 1. Accept a native result and make its workspace copyback fail, so that retained recovery owns the export. 2. Press Retry on the task. 3. Kill the control plane while a recovery attempt owns copyback, so that its owner receipt stays on the run. Use another host, or a receipt without automatic stop proof. 4. Let recovery run. **Expected behavior** Rechecking the receipt does not spend attempts. The task names the wait and does not offer a Retry that cannot help. A board member can confirm that the previous server stopped. The recovery then finishes, and the waiting Retry continues by itself. **Actual behavior** `withNativeWorkspaceFinalizationOwnership` returned `owner_unverified` for a retained repair without creating any board action (`native-workspace-finalization-ownership.ts`, `requireStopVerification`). `recoverRetainedWorkspace` reserved an attempt for each recheck and failed it as `copyback_stop_unconfirmed` (`workspace-recovery.ts`), until the budget was exhausted. The Retry gate in `explicit-native-continuation.ts` returned `finalizing_saved_result` for an accepted result with no verified repair, with no check for this hold. `resumeNativeWorkspaceAfterOwnerStop` accepted only an `owner_unverified` action on a `running` run, so no route could release the receipt of a finished run. The exhausted handoff told the board to press Retry again, which repeats the same failure. ## What Changed - `server/src/services/workspace-recovery.ts`: when retained recovery finds an unverifiable copyback owner, it records the recheck without reserving an attempt. The 30-minute recovery window still bounds it. The retained action records the exact receipt as `workspaceOwnerRecovery: { state: "operator_required", owner }`. A newer receipt replaces the record and clears an older confirmation. - `server/src/services/explicit-native-continuation.ts`: a Retry that waits for an accepted saved result reports `workspace_owner_stop_required` when the run's active retained action has that hold. It no longer reports `finalizing_saved_result` for this case. - `server/src/services/task-interruption-presentation.ts`: the new wait has a fixed message, and Retry is not offered for it, because another click cannot release the receipt. - `server/src/services/native-runtime/native-workspace-owner-recovery.ts`: the existing board route `POST /issues/:id/recovery-actions/resume-workspace-finalization` (permission `runtime:confirm_owner_stop`) also accepts a retained recovery action with that hold. The confirmation must name the receipt that is still on the run. A live owner on this host still rejects it. It clears only that receipt, records the stop proof and an activity entry, and returns the same recovery to the worker. A spent budget gets one new generation, as a deliberate recovery request does. It accepts no result and starts no agent turn. The owner check is shared with the existing path. - `server/src/services/system-recovery.ts`: an exhausted `copyback_stop_unconfirmed` handoff names the board confirmation instead of another Retry. - `ui/src/api/issues.ts`: the route receipt's `resultId` can be `null` (a retained recovery has no live result binding). - `doc/native-workspace-finalization-recovery.md`: an additive paragraph describes this path. ## Verification - Red then green on embedded Postgres: `server/src/services/workspace-recovery-preparation-gates.test.ts`, "lets an accepted Retry finish after a killed copyback owner instead of waiting forever" with one case per wave (s13, s12). Each case uses a real accepted remote export, a Retry through the HTTP route, a killed-owner receipt, and 8 recovery passes. - Before the fix (both cases): `attempts: 6, state: "exhausted"`. That is the wave state. - After the fix: no attempt is spent on the recheck. The hold is recorded. The Retry wait is `workspace_owner_stop_required` with `canRetry: false`. The board confirmation through the route returns 202 and clears the receipt. The same recovery completes. The same accepted Retry then finishes the saved result (s13: the task is `done`) or continues in exactly one new run (s12). - `npx vitest run src/services/workspace-recovery-preparation-gates.test.ts -t "killed copyback owner"`: 2 passed. - `pnpm verify` (affected selection): see the comment below. Server `tsc --noEmit` is clean. - No live Daytona replay. Both waves ran on old signed images. The local proof uses the same accepted-export fixture as the existing Retry tests, with the receipt state copied from the kept databases. ## Risks - A copyback owner on another host that is still alive is now rechecked within the recovery window without spending attempts. Before this change it spent them. The window still bounds the wait. - The board confirmation for a retained action is an operator statement, the same as the existing owner-stop path. It is limited to roles with `runtime:confirm_owner_stop`. It never clears a receipt other than the one it names, and a live same-host owner still rejects it. - #19717 changes the same ownership area and adds a UI action for the running-run case. That UI does not show a retained action yet. After both merge, a small follow-up can show its button for this hold. Until then the API route and the task's wait message cover it. - Not addressed here: the "Workspace checkpoint payload mismatch" that started wave s12. The checkpoint outbound transfer extracts without `-p` (`sandbox-managed-runtime.ts` checkpoint sync-out, `extractHostTarball` in the Daytona provider), so the host umask can change file modes that the remote manifest recorded. That is a separate signature. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, test runs, embedded Postgres). ## 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 - [ ] 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 --- doc/native-workspace-finalization-recovery.md | 15 +++ .../services/explicit-native-continuation.ts | 16 ++- .../native-workspace-owner-recovery.ts | 103 +++++++++++++++--- server/src/services/system-recovery.ts | 5 + .../task-interruption-presentation.ts | 4 +- ...rkspace-recovery-preparation-gates.test.ts | 85 ++++++++++++++- server/src/services/workspace-recovery.ts | 40 ++++++- ui/src/api/issues.ts | 2 +- 8 files changed, 244 insertions(+), 26 deletions(-) diff --git a/doc/native-workspace-finalization-recovery.md b/doc/native-workspace-finalization-recovery.md index 995332f5a28..c8cb5ef959d 100644 --- a/doc/native-workspace-finalization-recovery.md +++ b/doc/native-workspace-finalization-recovery.md @@ -602,3 +602,18 @@ task's queue through current admission checks. Each saved request still advances its recovery cursor. Rejected attempts, changed source runs, and updated source evidence remain eligible, and the next pass checks current state again. Legacy per-request handling stays unchanged. Pending work is awaited before a source is marked checked. + +Retained saved-work recovery for a finished run uses the same owner receipt. +When the receipt names a controller that a restart killed and no automatic proof +can release it, the recovery worker does not spend an attempt on each recheck. +It records the exact receipt on the retained recovery action as +`workspaceOwnerRecovery.state = "operator_required"` and keeps rechecking within +the bounded recovery window. A task Retry waiting on that saved result reports +`workspace_owner_stop_required` and does not offer another Retry, because another +click cannot release the receipt. The board confirms the stop through the same +`resume-workspace-finalization` route. The confirmation must name the receipt that +is still on the run. It clears only that receipt and returns the existing recovery +to the worker (a spent budget gets one new generation). It accepts no result and +starts no agent turn. The waiting Retry then continues through its normal path. +If the recovery window ends first, the board handoff names the same confirmation +instead of advising another Retry. diff --git a/server/src/services/explicit-native-continuation.ts b/server/src/services/explicit-native-continuation.ts index d7b6fac001d..afd1f00b2d6 100644 --- a/server/src/services/explicit-native-continuation.ts +++ b/server/src/services/explicit-native-continuation.ts @@ -401,8 +401,20 @@ export async function admitExplicitNativeContinuation(input: { actionId: action.id, operationIds: recovery.preservationOperationIds }); const repairedTerminal = completedResult && coordinator.phase === "terminal_failure" && await hasVerifiedWorkspaceRepair(db, run); - if (completedResult && !repairedTerminal && !(freshIntent && snapshotSettled)) - return blocked("finalizing_saved_result", "Paperclip is finishing the saved result."); + if (completedResult && !repairedTerminal && !(freshIntent && snapshotSettled)) { + // Nothing finishes the saved result while a stopped copyback owner is + // unverified. The run's live retained recovery records that hold; name + // its board action instead of an endless finalizing wait (RC-50). + const [ownerHold] = await db.select({ id: issueRecoveryActions.id }).from(issueRecoveryActions).where(and( + eq(issueRecoveryActions.companyId, companyId), eq(issueRecoveryActions.sourceIssueId, issueId), + inArray(issueRecoveryActions.status, ["active", "escalated"]), + sql`${issueRecoveryActions.evidence}->>'runId' = ${run.id}`, + sql`${issueRecoveryActions.evidence}#>>'{workspaceOwnerRecovery,state}' = 'operator_required'`, + sql`${issueRecoveryActions.evidence}->'workspaceOwnerStop' is null`)).limit(1); + return ownerHold + ? blocked("workspace_owner_stop_required", "A board member must confirm that the previous server stopped copying files. Your Retry is saved.") + : blocked("finalizing_saved_result", "Paperclip is finishing the saved result."); + } } // A repaired export no longer blocks execution. Its machine receipt still // binds the saved user turn; finalized file state alone is insufficient. diff --git a/server/src/services/native-runtime/native-workspace-owner-recovery.ts b/server/src/services/native-runtime/native-workspace-owner-recovery.ts index 346ee79e885..357eb09d37d 100644 --- a/server/src/services/native-runtime/native-workspace-owner-recovery.ts +++ b/server/src/services/native-runtime/native-workspace-owner-recovery.ts @@ -11,10 +11,34 @@ import { conflict } from "../../errors.js"; import { persistActivity, publishActivity } from "../activity-log.js"; import { currentNativeWorkspaceRecoveryResult } from "./native-workspace-recovery-binding.js"; import { evaluateNativeControllerTakeover } from "./native-restart-recovery.js"; +import { readWorkspaceRecovery } from "../workspace-recovery-receipt.js"; +import { updateSystemRecovery } from "../system-recovery.js"; +import { createSystemRecovery, readSystemRecovery } from "../system-recovery-state.js"; const OWNER_KEY = "nativeWorkspaceFinalizationOwner"; const record = (value: unknown): Record => value && typeof value === "object" && !Array.isArray(value) ? value as Record : {}; const changed = () => conflict("The recorded workspace owner is no longer current. Refresh the task and inspect its run.", { code: "workspace_owner_recovery_stale" }); +const retainedExportCauses = ["native_workspace_sync_out_unrecoverable", "native_workspace_sync_out_retry_exhausted"]; + +/** The confirmation must name the exact receipt still on the run. A live owner + * observable on this host contradicts it, even after its advisory lock dropped. */ +async function assertRecordedOwnerStopped(prior: Record, recordedOwner: Record, ownerToken: string) { + if (typeof prior.hostname !== "string" || !prior.hostname + || !Number.isInteger(prior.pid) || Number(prior.pid) <= 0 + || typeof prior.processStartedAt !== "string" || !Number.isFinite(Date.parse(prior.processStartedAt)) + || prior.token !== ownerToken || recordedOwner.token !== ownerToken + || prior.hostname !== recordedOwner.hostname || prior.pid !== recordedOwner.pid + || prior.processStartedAt !== recordedOwner.processStartedAt + || prior.controllerBootId !== recordedOwner.controllerBootId) throw changed(); + if (prior.hostname === os.hostname()) { + const takeover = await evaluateNativeControllerTakeover({ owner: { + leaseOwner: ownerToken, leaseExpiresAt: new Date(0), + controllerPid: typeof prior.pid === "number" ? prior.pid : null, + controllerProcessStartedAt: typeof prior.processStartedAt === "string" ? new Date(prior.processStartedAt) : null, + }, now: new Date() }); + if (!takeover.allowed) throw conflict("The previous workspace controller is still active or its stop cannot be verified."); + } +} /** Board-only admission after deployment-platform verification. A missing DB * lock or an expired lease alone never proves physical copyback has stopped. */ @@ -35,6 +59,8 @@ export async function resumeNativeWorkspaceAfterOwnerStop(input: { const [issue] = await tx.select().from(issues).where(and(eq(issues.companyId, input.companyId), eq(issues.id, input.issueId))).for("update"); const [run] = await tx.select().from(heartbeatRuns).where(and(eq(heartbeatRuns.companyId, input.companyId), eq(heartbeatRuns.nativeIssueId, input.issueId), eq(heartbeatRuns.id, input.runId))).for("update"); const [action] = await tx.select().from(issueRecoveryActions).where(and(eq(issueRecoveryActions.companyId, input.companyId), eq(issueRecoveryActions.sourceIssueId, input.issueId), eq(issueRecoveryActions.id, input.actionId))).for("update"); + if (issue && run && action && readWorkspaceRecovery(action.evidence)) + return confirmRetainedCopybackOwnerStop(tx as unknown as Db, { ...input, confirmation, run, coordinator, action }); if (!issue || !run || !coordinator || !action || action.evidence?.runId !== run.id || action.cause !== "native_workspace_finalization_owner_unverified" || action.ownerType !== "board" || action.kind !== "active_run_watchdog" || !["active", "escalated"].includes(action.status) @@ -49,23 +75,7 @@ export async function resumeNativeWorkspaceAfterOwnerStop(input: { const receipt = { runId: run.id, resultId: result.id, status: "queued" as const }; if (run.runnerProfileJson?.[OWNER_KEY] == null && record(stop.owner).token === input.ownerToken && stop.resultId === result.id) return { receipt, publication: null }; - if (typeof prior.hostname !== "string" || !prior.hostname - || !Number.isInteger(prior.pid) || Number(prior.pid) <= 0 - || typeof prior.processStartedAt !== "string" || !Number.isFinite(Date.parse(prior.processStartedAt)) - || prior.token !== input.ownerToken || recordedOwner.token !== input.ownerToken - || prior.hostname !== recordedOwner.hostname || prior.pid !== recordedOwner.pid - || prior.processStartedAt !== recordedOwner.processStartedAt - || prior.controllerBootId !== recordedOwner.controllerBootId) throw changed(); - // Reject a confirmation contradicted by a locally observable live owner, - // even if that owner's advisory connection has already disconnected. - if (prior.hostname === os.hostname()) { - const takeover = await evaluateNativeControllerTakeover({ owner: { - leaseOwner: input.ownerToken, leaseExpiresAt: new Date(0), - controllerPid: typeof prior.pid === "number" ? prior.pid : null, - controllerProcessStartedAt: typeof prior.processStartedAt === "string" ? new Date(prior.processStartedAt) : null, - }, now: new Date() }); - if (!takeover.allowed) throw conflict("The previous workspace controller is still active or its stop cannot be verified."); - } + await assertRecordedOwnerStopped(prior, recordedOwner, input.ownerToken); const now = new Date(); const proof = { owner: prior, resultId: result.id, actorId: input.actorId, stopEvidence: confirmation.stopEvidence, confirmedAt: now.toISOString() }; @@ -93,3 +103,62 @@ export async function resumeNativeWorkspaceAfterOwnerStop(input: { if (admitted.publication) publishActivity(admitted.publication.publication); return admitted.receipt; } + +/** + * Retained saved-work recovery for a finished run (RC-50). A restart killed its + * copyback owner, so recovery and Retry wait on a receipt nothing can release. + * The board confirmation clears that exact receipt and returns the existing + * recovery to the system worker. It accepts no result, starts no agent turn and + * leaves the task status to the normal recovery and Retry paths. + */ +async function confirmRetainedCopybackOwnerStop(tx: Db, input: { + companyId: string; issueId: string; ownerToken: string; actorId: string; + confirmation: { stopEvidence: string }; + run: typeof heartbeatRuns.$inferSelect; + coordinator: typeof nativeRunFinalizations.$inferSelect | undefined; + action: typeof issueRecoveryActions.$inferSelect; +}) { + const { run, coordinator, action } = input; + const hold = record(action.evidence?.workspaceOwnerRecovery); + const stop = record(action.evidence?.workspaceOwnerStop); + const receipt = { runId: run.id, resultId: coordinator?.resultId ?? null, status: "queued" as const }; + if (action.evidence?.runId !== run.id || !retainedExportCauses.includes(action.cause) + || action.kind !== "active_run_watchdog" || !["active", "escalated"].includes(action.status) + || !["failed", "interrupted", "timed_out", "cancelled", "succeeded"].includes(run.status) + || (coordinator?.leaseOwner && (!coordinator.leaseExpiresAt || coordinator.leaseExpiresAt.getTime() > Date.now()))) throw changed(); + if (run.runnerProfileJson?.[OWNER_KEY] == null && record(stop.owner).token === input.ownerToken) + return { receipt, publication: null }; + if (hold.state !== "operator_required") throw changed(); + const prior = record(run.runnerProfileJson?.[OWNER_KEY]); + await assertRecordedOwnerStopped(prior, record(hold.owner), input.ownerToken); + const current = readSystemRecovery(action.evidence); + if (!current || current.sourceRunId !== run.id || current.state === "complete") throw changed(); + const now = new Date(); + // Return the same recovery to the worker now. A spent budget gets one new + // generation, as a deliberate recovery request would; saved receipts carry over. + const next = current.state === "exhausted" + ? { ...createSystemRecovery(run.id, current.intentId), revision: current.revision + 1, + preservationOperationIds: current.preservationOperationIds, providerCooldownAt: current.providerCooldownAt, + nextAttemptAt: current.providerCooldownAt } + : { ...current, revision: current.revision + 1, nextAttemptAt: Math.max(now.getTime(), current.providerCooldownAt ?? 0) }; + const source = { companyId: input.companyId, issueId: input.issueId, runId: run.id, actionId: action.id }; + if (!await updateSystemRecovery(tx, source, current, next)) throw changed(); + const proof = { owner: prior, resultId: receipt.resultId, actorId: input.actorId, + stopEvidence: input.confirmation.stopEvidence, confirmedAt: now.toISOString() }; + const cleared = await tx.update(heartbeatRuns).set({ + runnerProfileJson: sql`${heartbeatRuns.runnerProfileJson} - ${OWNER_KEY}`, updatedAt: now, + }).where(and(eq(heartbeatRuns.companyId, input.companyId), eq(heartbeatRuns.id, run.id), + sql`${heartbeatRuns.runnerProfileJson}->${OWNER_KEY}->>'token' = ${input.ownerToken}`)).returning({ id: heartbeatRuns.id }); + if (!cleared.length) throw changed(); + await tx.update(issueRecoveryActions).set({ + evidence: sql`${issueRecoveryActions.evidence} || ${JSON.stringify({ workspaceOwnerStop: proof, + workspaceOwnerRecovery: { ...hold, state: "resuming" } })}::jsonb`, + updatedAt: now, + }).where(and(eq(issueRecoveryActions.companyId, input.companyId), eq(issueRecoveryActions.id, action.id))); + const publication = await persistActivity(tx, { companyId: input.companyId, + actorType: "user", actorId: input.actorId, action: "issue.workspace_owner_stop_confirmed", + entityType: "issue", entityId: input.issueId, runId: run.id, + details: { recoveryActionId: action.id, ...proof }, + }); + return { receipt, publication }; +} diff --git a/server/src/services/system-recovery.ts b/server/src/services/system-recovery.ts index b32580e3392..3edd496a1d3 100644 --- a/server/src/services/system-recovery.ts +++ b/server/src/services/system-recovery.ts @@ -48,6 +48,11 @@ export async function adoptSystemRecovery(db: Db, source: WorkspaceRecoverySourc } export function exhaustedSystemRecoveryAction(state: SystemRecoveryState): string { + // Another Retry cannot release a copyback owner that stopped without reporting. + if (state.failure?.code === "copyback_stop_unconfirmed") + return "Automatic recovery of saved work is waiting for the previous server's file copy to stop, and Paperclip cannot verify " + + "that it stopped. A board member who manages this company's runtime can confirm that it stopped; recovery then " + + "resumes. No provider turn is submitted."; const cause = state.failure?.cause ? `, cause: ${state.failure.cause.label}` : ""; const reason = state.failure?.code ? ` (last failure: ${state.failure.code}${cause})` : ""; return `Automatic recovery of saved work stopped after ${state.attempts} attempts${reason}. ` diff --git a/server/src/services/task-interruption-presentation.ts b/server/src/services/task-interruption-presentation.ts index 40804d66f47..632b3a96843 100644 --- a/server/src/services/task-interruption-presentation.ts +++ b/server/src/services/task-interruption-presentation.ts @@ -82,6 +82,8 @@ const retryWaitMessages: Record = { decision_pending: "Retry is waiting for a pending approval or question to be resolved. Your message is saved.", "budget.blocked": "Retry is waiting for the budget to allow new work. Check Costs for unrecorded or unpriced usage.", finalizing_saved_result: "Paperclip is finishing the saved result before Retry can continue.", + // Another click cannot release a copyback owner that stopped without reporting. + workspace_owner_stop_required: "Retry is waiting for a board member to confirm that the previous server stopped copying files. The agent's result and your Retry are saved.", cleanup_quarantined: "Retry cannot continue this attempt. Send a new message after the previous execution has stopped.", cancelled_by_operator: "This attempt was stopped. Send a new message to continue from saved work.", // A new click replaces a Retry declined for its requester's access with a @@ -208,7 +210,7 @@ export function projectTaskInterruption(issue: ProjectionIssue, input: Projectio const reason = Object.hasOwn(retryWaitMessages, retryWait.executionWaitReason) ? retryWait.executionWaitReason : "execution_recovery"; // Keep the latest input receipt so message submission can settle independently. return { ...base, state: "failed", - canRetry: !["cleanup_quarantined", "cancelled_by_operator", "retry_source_changed"].includes(reason), retryWait: { reason, + canRetry: !["cleanup_quarantined", "cancelled_by_operator", "retry_source_changed", "workspace_owner_stop_required"].includes(reason), retryWait: { reason, message: retryWaitMessages[reason] ?? "Retry is waiting for execution checks. Your message is saved." } }; } if (wait && wait.requestedAt.getTime() > now - 10_000 && system?.state !== "exhausted") diff --git a/server/src/services/workspace-recovery-preparation-gates.test.ts b/server/src/services/workspace-recovery-preparation-gates.test.ts index f566c836f90..9e1f34187ac 100644 --- a/server/src/services/workspace-recovery-preparation-gates.test.ts +++ b/server/src/services/workspace-recovery-preparation-gates.test.ts @@ -3,7 +3,7 @@ import { heartbeatRunEvents, agents, approvals, costEvents, issueApprovals, envi import { terminalizeLegacyExecution } from "./legacy-execution-recovery.js"; import * as recoverySources from "./workspace-recovery-source.js"; import { requestedRunCancellation } from "./run-cancellation.js"; -import { readSystemRecovery } from "./system-recovery-state.js"; +import { failSystemRecovery, readSystemRecovery, reserveSystemRecoveryAttempt, SystemRecoveryFailure } from "./system-recovery-state.js"; import { getStorageService } from "../storage/index.js"; import { attachPreservedWorkspaceSnapshots } from "./workspace-recovery-snapshot.js"; import { requestTaskRetry } from "./task-retry.js"; @@ -26,6 +26,8 @@ import { createSavedWorkspaceLoader, readRemoteWorkspaceAuthority, saveRemoteWor import { createReusableLeaseHandoff } from "./environment-lease-handoff.js"; import { preserveRemoteWorkspacePreparationFailure } from "./native-runtime/native-workspace-finalization-state.js"; import { getExecutionBlocker } from "./execution-blocker.js"; +import { taskInterruptionPresentation } from "./task-interruption-presentation.js"; +import { resumePreservedResultFinalization } from "./preserved-result-finalization.js"; import { useWorkspaceRecoveryFixture, acceptedRemoteExport, action, boardRecoveryApp, db, interruptedRemotePreparation, makeRetryDue, root, run, seed, withheldPreparationRetry } from "./__tests__/workspace-recovery-fixture.js"; const targets = vi.hoisted(() => new Map()); vi.mock("./environment-execution-target.js", async importOriginal => { @@ -279,6 +281,87 @@ describe("automatic retained workspace recovery", () => { .toMatchObject({ status: "pending", result: null, resolvedAt: null }); }, 30_000); + // RC-50: a restart killed the controller that owned the saved-result copyback. + // s13/t06: the kill landed 2 s after Retry. s12/t01: it landed during a later + // attempt, after an earlier export failure had already spent one. + // "exhausted before upgrade": the wave databases, where earlier code already + // spent the whole budget on rechecks and recorded no hold. + it.each([ + { wave: "s13", spentAttempts: 0, expectedStrategy: "finalize_result", exhaustedBeforeUpgrade: false }, + { wave: "s12", spentAttempts: 1, expectedStrategy: "continue", exhaustedBeforeUpgrade: false }, + { wave: "exhausted before upgrade", spentAttempts: 0, expectedStrategy: "finalize_result", exhaustedBeforeUpgrade: true }, + ])("lets an accepted Retry finish after a killed copyback owner instead of waiting forever ($wave)", async ({ spentAttempts, expectedStrategy, exhaustedBeforeUpgrade }) => { + const f = await acceptedRemoteExport(); + const issue = (await db.select().from(issues).where(eq(issues.id, f.issueId)))[0]; + await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.source.agentId, status: "running" }); + if (spentAttempts) { + f.runtime.resumeRunLease.mockRejectedValueOnce(new Error("Workspace checkpoint payload mismatch")); + await recoverRetainedWorkspace(f.input); + await makeRetryDue(f.actionId); + } + const app = boardRecoveryApp(); + const retry = (await request(app).post(`/api/issues/${f.issueId}/retry`) + .send({ observedRunId: f.runId, idempotencyKey: randomUUID() }).expect(202)).body; + // The controller that claimed copyback was SIGKILLed on another host + // (no local proof is possible), leaving its durable receipt behind. + const dead = { token: randomUUID(), hostname: "killed-controller", pid: 91826, + controllerBootId: randomUUID(), processStartedAt: new Date().toISOString() }; + const source = await run(f.runId); + await db.update(heartbeatRuns).set({ runnerProfileJson: { ...source.runnerProfileJson, nativeWorkspaceFinalizationOwner: dead } }) + .where(eq(heartbeatRuns.id, f.runId)); + if (exhaustedBeforeUpgrade) { + const before = await action(f.actionId); + let spent = readSystemRecovery(before.evidence)!; + for (let attempt = 0; attempt < 6; attempt++) spent = failSystemRecovery(reserveSystemRecoveryAttempt({ ...spent, nextAttemptAt: null })!, + new SystemRecoveryFailure("copyback_stop_unconfirmed", "recheck_evidence"), { now: spent.firstFailureAt ?? Date.now() }); + expect(spent.state).toBe("exhausted"); + await db.update(issueRecoveryActions).set({ evidence: { ...before.evidence, systemRecovery: spent } }) + .where(eq(issueRecoveryActions.id, f.actionId)); + } + for (let pass = 0; pass < 8; pass++) { + await recoverRetainedWorkspace(f.input); + await heartbeatService(db).resumeTaskRetry(f.companyId, retry.requestId!); + if (!exhaustedBeforeUpgrade) await makeRetryDue(f.actionId); + } + expect(f.runtime.resumeRunLease).toHaveBeenCalledTimes(spentAttempts); + // Rechecking an unverifiable owner is not an attempt and never ends in a stale "Retry recovery" handoff. + const held = await action(f.actionId); + expect(readSystemRecovery(held.evidence)).toMatchObject(exhaustedBeforeUpgrade + ? { state: "exhausted", attempts: 6, failure: { code: "copyback_stop_unconfirmed" } } + : { state: "awaiting_evidence", attempts: spentAttempts, failure: { code: "copyback_stop_unconfirmed" } }); + if (exhaustedBeforeUpgrade) expect(held.nextAction).toContain("confirm that it stopped"); + expect(held.evidence.workspaceOwnerRecovery).toEqual({ state: "operator_required", owner: dead }); + // Retry names the one action that can progress it, instead of "finalizing" forever. + expect((await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, retry.requestId!)))[0]) + .toMatchObject({ status: "deferred_issue_execution", payload: { executionWait: { reason: "workspace_owner_stop_required" } } }); + expect(await taskInterruptionPresentation(db, issue)).toMatchObject({ state: "failed", canRetry: false, + retryWait: { reason: "workspace_owner_stop_required" } }); + await request(app).post(`/api/issues/${f.issueId}/recovery-actions/resume-workspace-finalization`).send({ + actionId: f.actionId, runId: f.runId, ownerToken: dead.token, controllerAndCopybackStopped: true, + stopEvidence: "The killed control-plane process exited and its host was replaced.", + }).expect(202); + expect((await run(f.runId)).runnerProfileJson?.nativeWorkspaceFinalizationOwner).toBeUndefined(); + // The same steps the periodic sweep runs for this action. + await recoverRetainedWorkspace(f.input); + expect(readSystemRecovery((await action(f.actionId)).evidence)?.state).toBe("complete"); + await resumePreservedResultFinalization(db, f.companyId, f.runId); + await heartbeatService(db).resumeTaskRetry(f.companyId, retry.requestId!); + // The one accepted click now does work: it finishes the saved result, or, + // after an earlier recovery handoff (s12), continues in exactly one new run. + const [settled] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, retry.requestId!)); + expect(settled).toMatchObject({ status: "coalesced", runId: expect.any(String) }); + if (expectedStrategy === "finalize_result") { + expect(settled).toMatchObject({ runId: f.runId, payload: { taskRetry: { strategy: "finalize_result" } } }); + await finalizeNativeRun({ db, runId: f.runId, workspaceFinalizeStatus: "succeeded", projectRunStatus: true }); + expect((await db.select().from(issues).where(eq(issues.id, f.issueId)))[0]).toMatchObject({ status: "done" }); + } else { + expect(settled.payload).toMatchObject({ taskRetry: { strategy: "continue" } }); + expect(await db.select({ id: heartbeatRuns.id, previousRunId: sql`${heartbeatRuns.contextSnapshot}->>'previousRunId'` }) + .from(heartbeatRuns).where(and(eq(heartbeatRuns.companyId, f.companyId), eq(heartbeatRuns.status, "queued")))) + .toEqual([{ id: settled.runId, previousRunId: f.runId }]); + } + }, 60_000); + it("preserves an explicitly requested superseded remote export without applying its result over newer work", async () => { const f = await acceptedRemoteExport(); const [later] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.source.agentId, diff --git a/server/src/services/workspace-recovery.ts b/server/src/services/workspace-recovery.ts index b4d9a9d6595..b4b07d55c9c 100644 --- a/server/src/services/workspace-recovery.ts +++ b/server/src/services/workspace-recovery.ts @@ -161,6 +161,9 @@ export async function recoverRetainedWorkspace(input: Source & { db: Db; environ await adoptSystemRecovery(db, input); const initial = await loadSystemRecovery(db, input); if (initial?.state === "exhausted" && !canAdoptNativeSourceRetention(initial)) { + // A budget spent before this hold existed still names its receipt, so the + // board confirmation in its handoff can release it (RC-50 wave databases). + if (initial.failure?.code === "copyback_stop_unconfirmed") await recordCopybackOwnerHold(db, input, initial.generationId); await handOffExhaustedSystemRecovery(db, input, initial); return; } @@ -439,14 +442,43 @@ export async function recoverRetainedWorkspace(input: Source & { db: Db; environ const owned = await withNativeWorkspaceFinalizationOwnership({ db, companyId: input.companyId, runId: input.runId, retainedRepairActionId: input.actionId }, runAttempt); if (!owned.acquired && owned.reason === "owner_unverified") { - const current = await loadSystemRecovery(db, input); - const reserved = current && reserveSystemRecoveryAttempt(current); - if (current && reserved) await updateSystemRecovery(db, input, current, - failSystemRecovery(reserved, new SystemRecoveryFailure("copyback_stop_unconfirmed", "recheck_evidence"))); + await holdForUnverifiedCopybackOwner(db, input); await recheckStoppedWorkspaceRecovery(db, input, false); } } +/** + * A copyback owner that a restart killed can never confirm its own stop, and + * Retry cannot release its receipt (RC-50). Rechecking that receipt is not a + * recovery attempt: the bounded recovery window still applies, but no attempt + * is spent. Record the exact receipt so the board can confirm the stop through + * `resumeNativeWorkspaceAfterOwnerStop`, and so Retry names this wait. + */ +async function holdForUnverifiedCopybackOwner(db: Db, input: Source) { + const current = await loadSystemRecovery(db, input); + if (!current || current.state === "complete" || current.state === "exhausted") return; + const next = failSystemRecovery(current, new SystemRecoveryFailure("copyback_stop_unconfirmed", "recheck_evidence")); + if (!await updateSystemRecovery(db, input, current, next)) return; + await recordCopybackOwnerHold(db, input, next.generationId); +} + +/** Bookkeeping only: names the exact receipt for the board. Grants no attempt. */ +async function recordCopybackOwnerHold(db: Db, input: Source, generationId: string) { + const [source] = await db.select({ owner: sql`${heartbeatRuns.runnerProfileJson}->'nativeWorkspaceFinalizationOwner'` }) + .from(heartbeatRuns).where(and(eq(heartbeatRuns.companyId, input.companyId), eq(heartbeatRuns.id, input.runId))); + const owner = source?.owner; + if (!owner || typeof owner !== "object" || typeof (owner as { token?: unknown }).token !== "string") return; + await db.update(issueRecoveryActions).set({ + // A confirmation names one receipt; a newer unverified owner needs its own. + evidence: sql`(${issueRecoveryActions.evidence} - 'workspaceOwnerStop') || jsonb_build_object('workspaceOwnerRecovery', + jsonb_build_object('state', 'operator_required', 'owner', ${JSON.stringify(owner)}::jsonb))`, + updatedAt: new Date(), + }).where(and(eq(issueRecoveryActions.companyId, input.companyId), eq(issueRecoveryActions.id, input.actionId), + eq(issueRecoveryActions.sourceIssueId, input.issueId), + sql`${issueRecoveryActions.evidence}#>>'{systemRecovery,generationId}' = ${generationId}`, + sql`${issueRecoveryActions.evidence}#>'{workspaceOwnerRecovery,owner}' is distinct from ${JSON.stringify(owner)}::jsonb`)); +} + const cursors = new WeakMap(); const workInFlight = new WeakMap>(); const unfinishedCursors = new WeakMap(); diff --git a/ui/src/api/issues.ts b/ui/src/api/issues.ts index 49e77870fb7..1e5a094076f 100644 --- a/ui/src/api/issues.ts +++ b/ui/src/api/issues.ts @@ -420,7 +420,7 @@ export const issuesApi = { data, ), resumeWorkspaceFinalization: (id: string, data: { actionId: string; runId: string; ownerToken: string; controllerAndCopybackStopped: true; stopEvidence: string }) => - api.post<{ runId: string; resultId: string; status: "queued" }>(`/issues/${id}/recovery-actions/resume-workspace-finalization`, data), + api.post<{ runId: string; resultId: string | null; status: "queued" }>(`/issues/${id}/recovery-actions/resume-workspace-finalization`, data), retryWorkspaceExport: (id: string, data: { actionId: string; runId: string; repairNote: string }) => api.post<{ runId: string; resultId: string; leaseId: string; status: "queued" }>(`/issues/${id}/recovery-actions/retry-workspace-export`, data), retryTask: (issueId: string, body: { idempotencyKey: string; observedRunId: string | null }) =>