mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
fix(recovery): keep a stopped copy-back's files instead of holding the workspace (#19385)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A native run can have its result accepted before the copy-back of its sandbox files finishes. If the copy-back fails, it retries. > - A board Stop during those retries ends the run. It sets the run's finalization to `terminal_failure` / `native_retry_cancelled`, and the result stays accepted. > - After that, nothing exports the files. The finalization worker skips terminal rows, and cancelled-workspace preservation skipped any run with an accepted result. > - The task workspace stays dirty and bound to the stopped run. Every later run on the task waits for the workspace, then fails setup with the unfinished-writer error. > - This pull request lets cancelled-workspace preservation also admit an accepted result whose copy-back a Stop ended. > - The benefit is that stopping a stuck copy-back saves its files and frees the task, like stopping any other run. ## Linked Issues or Issue Description Refs #16700, #17677, #18820, #19304 and #19305 (cancelled-workspace preservation). This pull request adds the accepted-result case to that path and keeps its other checks unchanged. **What happened?** On a hosted instance, a native run's result was accepted, but its copy-back kept failing and retrying. A board operator pressed Stop to unblock the task. The run became `cancelled`, its sandbox was stopped and kept, and its finalization moved to `terminal_failure` / `native_retry_cancelled` with the result still accepted. The next run on the task was cancelled with "Shared execution workspace is busy: run … ended, but its workspace save has not settled". About 24 automatic retries followed over an hour. The last one failed with "The workspace still has an unfinished writer or save operation." After that nothing ran: the task stayed `in_progress` with no run and no recovery action. A Retry or a new comment would hit the same block. The workspace stayed `dirty: true` with the stopped run as its writer, and no `workspaceRestoreRecovery` was ever recorded. The reason: `preserveCancelledNativeWorkspaces` requires `resultId IS NULL`, and `registerCancelledWorkspacePreservationWork` and `preserveNativeFailedWorkspace` both return early when `resultId` is set. **Expected behavior** Stopping a run during its copy-back retries keeps its files, as stopping any other run does. Paperclip's workspace recovery saves them and releases the workspace. Nothing commits or replays the stopped result. **Steps to reproduce** 1. Accept a native result for a run whose copy-back is in `retryable_failure`. 2. Stop the run with the board Stop. The finalization becomes `terminal_failure` / `native_retry_cancelled`. 3. Start another run on the task. It waits for the workspace and then fails setup. No save of the stopped run's files is ever recorded. The new database tests run steps 1 and 2 against PostgreSQL and check that the save is recorded. **Paperclip version or commit** `master` at `9ceaec6f97e`. **Deployment mode** Any deployment that uses remote (sandbox) task workspaces. Seen on a cloud-managed instance. ## What Changed - `server/src/services/native-runtime/native-failed-workspace-recovery.ts`: cancelled-workspace preservation now also admits an accepted result when the finalization is `terminal_failure` with `native_retry_cancelled`. That is the state a Stop leaves after ending copy-back retries. All three entry points use the same rule: - the startup scan `preserveCancelledNativeWorkspaces`; - the worker `registerCancelledWorkspacePreservationWork`; - `preserveNativeFailedWorkspace`, only when it is called with `requireCancelledStop`. Preservation still requires everything it required before: a cancelled run, an acknowledged Stop, no live finalization owner or controller lease, a prepared workspace reference, and the exact retained source. - An accepted result with any other failure code is still skipped; those runs keep their own export repair path. Failed (not cancelled) runs are unchanged. - The accepted result stays recorded and uncommitted. Preservation does not change the task's status or queue a run. - `doc/execution-semantics.md`: describe the stopped copy-back case in the paragraph on writers stopped mid-run. - `native-cancelled-workspace-preservation.test.ts`: move the accepted-result fixture into an `acceptResult` helper, and add a test with three cases. ## Verification - Failing first. The new test "preserves files of an accepted result whose copy-back was stopped" has three cases: worker, startup scan, and a sandbox that the Stop already stopped and kept, through the real `prepareSandboxStopAndRetain` and `settleStopOnlyCleanup`. All three fail on `master`, with no `workspaceRestoreFailure` recorded, and pass with this change. - Each case checks that: - the save obligation and recovery action are recorded for the exact lease; - `inspectWorkspaceRecoverySource` finds one source; - the finalization keeps its `resultId`, `terminal_failure` and `native_retry_cancelled`; - the workspace stays held by the stopped writer until the save publishes; - no run or wake is created, and the task status is unchanged. - The existing "does not admit preservation for accepted-result" case still passes. It uses an accepted result with a different failure code. - `cd server && npx vitest run src/services/native-runtime/native-cancelled-workspace-preservation.test.ts`: 44 passed. - Related suites, 10 files and 854 passed: - `native-failed-workspace-recovery`, `workspace-recovery`, `heartbeat/run-control` - `native-cancellation-request` (unit and integration) - `heartbeat-remote-workspace-busy`, `native-workspace-export-retry`, `native-workspace-reconstruction` - `native-stop-accounting`, `task-workspace-binding` - `cd server && npx tsc --noEmit -p tsconfig.json`: pass. - Not run end to end on a live instance. Diagnosed from that instance's run, finalization, lease, workspace and recovery-action rows. ## Risks - On deploy, the startup scan and the worker will pick up existing cancelled runs in this state and record their saves. That is the intended repair, and it uses the same retention checks as other stopped writers. - A copy-back interrupted by a server restart can leave a `remote_workspace_snapshot` operation marked `running` after its process is gone. This change does not settle such rows. If one blocks the later save, the save follows its existing retry and board hand-off. - No schema migration. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code, with tool use (shell, file edits, test runs, read-only database queries). The runtime does not expose the context window or reasoning configuration. ## 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 Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
39096a55a4
commit
11bc490cda
3 files changed
+79
-18
No files matched your search
@@ -535,10 +535,13 @@ in its stopped sandbox. When the run used a reusable (warm) sandbox, the stop
|
||||
leaves that lease `released` under its reusable policy with a stop-and-retain
|
||||
receipt. Cancelled-workspace preservation admits that stopped source, as it does
|
||||
an active or ephemeral one. It records the save obligation for the exact writer
|
||||
run while the workspace is still bound to that writer. Paperclip's workspace
|
||||
recovery then saves those files, which releases the hold. It never replays the
|
||||
stopped run. While that save is in progress, its last progress counts as the
|
||||
ended writer's activity for the holder cutoff. A waiter keeps waiting instead of
|
||||
run while the workspace is still bound to that writer. A Stop during an
|
||||
accepted result's copy-back retries ends that copy-back, and no finalization
|
||||
exports those files afterwards. Preservation admits that writer as well and
|
||||
leaves the accepted result uncommitted. Other accepted results keep their own
|
||||
export repair path. Paperclip's workspace recovery then saves those files, which
|
||||
releases the hold. It never replays the stopped run. While that save is in
|
||||
progress, its last progress counts as the ended writer's activity for the holder cutoff. A waiter keeps waiting instead of
|
||||
failing setup. The save's retry budget bounds that wait. A save that exhausts
|
||||
its retries is handed to the board as a named recovery action. After that hand-off,
|
||||
the cutoff no longer advances.
|
||||
|
||||
+60
-11
@@ -75,6 +75,19 @@ import { readRemoteWorkspaceAuthority } from "@paperclipai/shared";
|
||||
expect(await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, f.companyId))).toHaveLength(0);
|
||||
}
|
||||
|
||||
async function acceptResult(f: Awaited<ReturnType<typeof seed>>) {
|
||||
const [contract] = await db.insert(completionContracts).values({ companyId: f.companyId, issueId: f.issueId,
|
||||
revision: 1, schemaVersion: "v1", policyVersion: "v1", risk: "low", completionAuthority: "agent",
|
||||
incompleteCriteriaPolicy: "block", contractJson: {}, canonicalSha256: "a".repeat(64),
|
||||
createdByActorType: "system", createdByActorId: "test" }).returning();
|
||||
await db.update(heartbeatRuns).set({ completionContractId: contract.id }).where(eq(heartbeatRuns.id, f.runId));
|
||||
const [result] = await db.insert(nativeRunResults).values({ companyId: f.companyId, issueId: f.issueId, runId: f.runId,
|
||||
completionContractId: contract.id, serverFingerprint: "b".repeat(64), schemaStatus: "accepted",
|
||||
resultJson: {}, canonicalSha256: "c".repeat(64) }).returning();
|
||||
await db.update(nativeRunFinalizations).set({ resultId: result.id }).where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
return result;
|
||||
}
|
||||
|
||||
it("reconstructs prepared remote files after an audited lifecycle Stop without a board actor", async () => {
|
||||
const f = await seed();
|
||||
const [historical] = await db.insert(issueRecoveryActions).values({ companyId: f.companyId, sourceIssueId: f.issueId,
|
||||
@@ -263,17 +276,7 @@ import { readRemoteWorkspaceAuthority } from "@paperclipai/shared";
|
||||
}).where(eq(heartbeatRuns.id, f.runId));
|
||||
if (invalid === "live-owner") await db.update(nativeRunFinalizations).set({ leaseOwner: randomUUID(),
|
||||
leaseExpiresAt: new Date(Date.now() + 60_000) }).where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
if (invalid === "accepted-result") {
|
||||
const [contract] = await db.insert(completionContracts).values({ companyId: f.companyId, issueId: f.issueId,
|
||||
revision: 1, schemaVersion: "v1", policyVersion: "v1", risk: "low", completionAuthority: "agent",
|
||||
incompleteCriteriaPolicy: "block", contractJson: {}, canonicalSha256: "a".repeat(64),
|
||||
createdByActorType: "system", createdByActorId: "test" }).returning();
|
||||
await db.update(heartbeatRuns).set({ completionContractId: contract.id }).where(eq(heartbeatRuns.id, f.runId));
|
||||
const [result] = await db.insert(nativeRunResults).values({ companyId: f.companyId, issueId: f.issueId, runId: f.runId,
|
||||
completionContractId: contract.id, serverFingerprint: "b".repeat(64), schemaStatus: "accepted",
|
||||
resultJson: {}, canonicalSha256: "c".repeat(64) }).returning();
|
||||
await db.update(nativeRunFinalizations).set({ resultId: result.id }).where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
}
|
||||
if (invalid === "accepted-result") await acceptResult(f);
|
||||
const errors: unknown[] = [];
|
||||
const worker = createDeliveryWorkCoordinator({ owner: db, canRun: () => true, onError: error => { errors.push(error); } });
|
||||
try {
|
||||
@@ -345,6 +348,52 @@ import { readRemoteWorkspaceAuthority } from "@paperclipai/shared";
|
||||
} finally { await worker.stop(); }
|
||||
}, 120_000);
|
||||
|
||||
// Stop during an accepted result's copy-back retries ends the copy-back. No
|
||||
// finalization exports those files afterwards, so preservation keeps them.
|
||||
it.each(["worker", "startup-scan", "stopped-sandbox"] as const)("preserves files of an accepted result whose copy-back was stopped (%s)", async kind => {
|
||||
const f = await seed();
|
||||
const result = await acceptResult(f);
|
||||
const nativeCancellation = { ...f.nativeCancellation, dispatched: false };
|
||||
await db.update(heartbeatRuns).set({ nativePhase: "retryable_failure", error: "Cancelled by a board operator",
|
||||
resultJson: { nativeCancellation, cancelledByActorType: "user", cancelledByUserId: "operator" } })
|
||||
.where(eq(heartbeatRuns.id, f.runId));
|
||||
await db.update(nativeRunFinalizations).set({ failureCode: "native_retry_cancelled",
|
||||
failureDetail: { originalFailureCode: "native_workspace_sync_out_failed", workspaceFinalizeAttempt: 1 } })
|
||||
.where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
if (kind === "stopped-sandbox") {
|
||||
// The Stop's resource release already stopped and kept the sandbox.
|
||||
const [lease] = await db.select().from(environmentLeases).where(eq(environmentLeases.id, f.leaseId));
|
||||
const stopping = (await prepareSandboxStopAndRetain(db, lease))!;
|
||||
expect(await settleStopOnlyCleanup(db, stopping, { attemptId: String(stopping.metadata!.pendingCleanupAttemptId),
|
||||
receipt: { providerLeaseId: f.reference.providerLeaseId, state: "stopped" } }))
|
||||
.toMatchObject({ status: "released", leasePolicy: "ephemeral", cleanupStatus: "success" });
|
||||
}
|
||||
const worker = createDeliveryWorkCoordinator({ owner: db, canRun: () => true, onError: error => { throw error; } });
|
||||
try {
|
||||
if (kind === "startup-scan") await preserveCancelledNativeWorkspaces(db);
|
||||
else await registerCancelledWorkspacePreservationWork(db, worker, () => true, async () => true).ready;
|
||||
const [retained] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId));
|
||||
expect(retained.resultJson).toMatchObject({ nativeCancellation, workspaceRestoreFailure: "restore_failed",
|
||||
workspaceRestoreRecovery: { schema: "paperclip.workspace-restore-recovery.v1", leaseIds: [f.leaseId] } });
|
||||
expect(retained).toMatchObject({ status: "cancelled", finishedAt: f.run.finishedAt });
|
||||
const actions = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.companyId, f.companyId));
|
||||
expect(actions).toHaveLength(1);
|
||||
expect(actions[0].evidence).toMatchObject({ runId: f.runId, workspaceRestoreRecovery: { leaseIds: [f.leaseId] } });
|
||||
expect((await inspectWorkspaceRecoverySource(db, { companyId: f.companyId, issueId: f.issueId,
|
||||
runId: f.runId, actionId: actions[0].id })).sources).toHaveLength(1);
|
||||
// The accepted result stays recorded and is never committed or replayed.
|
||||
expect((await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.runId)))[0])
|
||||
.toMatchObject({ phase: "terminal_failure", resultId: result.id, failureCode: "native_retry_cancelled", leaseOwner: null });
|
||||
const [workspace] = await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, f.workspaceId));
|
||||
expect(await readRemoteWorkspaceWriterHold(db, { companyId: f.companyId,
|
||||
authority: readRemoteWorkspaceAuthority(workspace.metadata)! })).toMatchObject({ kind: "held", writer: { runId: f.runId, ended: true } });
|
||||
expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, f.companyId))).toHaveLength(1);
|
||||
expect(await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, f.companyId))).toHaveLength(0);
|
||||
expect((await db.select().from(issues).where(eq(issues.id, f.issueId)))[0].status).toBe("in_progress");
|
||||
if (kind !== "startup-scan") expect(worker.nextWakeAt()).toBeNull();
|
||||
} finally { await worker.stop(); }
|
||||
}, 120_000);
|
||||
|
||||
it("fences recovery of a stopped reusable source behind later allocation owners and live writers", async () => {
|
||||
const f = await seed();
|
||||
await stopAndRetainReusableSource(f);
|
||||
|
||||
@@ -68,6 +68,14 @@ export async function preserveTaskRemoteWorkspaceWriter(db: Db, companyId: strin
|
||||
catch (error) { if (!(error instanceof NativeWorkspaceFinalizationBusyError)) throw error; }
|
||||
}
|
||||
|
||||
/** A Stop during an accepted result's copy-back retries ends that copy-back.
|
||||
* No finalization exports those files afterwards, so they are kept like the
|
||||
* files of any other stopped run. Other accepted results keep their own export
|
||||
* repair path. */
|
||||
function stoppedAcceptedCopyback(coordinator: Pick<typeof nativeRunFinalizations.$inferSelect, "phase" | "failureCode">) {
|
||||
return coordinator.phase === "terminal_failure" && coordinator.failureCode === "native_retry_cancelled";
|
||||
}
|
||||
|
||||
export async function preserveNativeFailedWorkspace(db: Db, selected: Pick<typeof heartbeatRuns.$inferSelect,
|
||||
"id" | "companyId" | "nativeIssueId" | "issueId" | "contextSnapshot" | "runnerProfileJson">,
|
||||
options: { allowPreterminal?: boolean; requireCancelledStop?: boolean; restartRecovery?: NativeRestartRecoveryClaim } = {}) {
|
||||
@@ -94,7 +102,7 @@ export async function preserveNativeFailedWorkspace(db: Db, selected: Pick<typeo
|
||||
&& (claim.kind !== "reattach_remote_runner" || (claim.remote.kind !== "plugin_task" && claim.remote.providerLeaseId === reference.providerLeaseId
|
||||
&& claim.remote.remoteCwd === reference.remoteCwd)));
|
||||
if ((coordinator?.leaseOwner || (claim && coordinator?.phase === "observed")) && !ownsClaim) throw new NativeWorkspaceFinalizationBusyError();
|
||||
if (!coordinator || coordinator.resultId
|
||||
if (!coordinator || (coordinator.resultId && !(options.requireCancelledStop && stoppedAcceptedCopyback(coordinator)))
|
||||
|| !(options.allowPreterminal ? ["observed", "terminal_failure"] : ["terminal_failure"]).includes(coordinator.phase)) return;
|
||||
const [run] = await tx.select().from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.id, selected.id), eq(heartbeatRuns.companyId, selected.companyId),
|
||||
@@ -217,7 +225,8 @@ export async function preserveCancelledNativeWorkspaces(db: Db) {
|
||||
eq(nativeRunFinalizations.companyId, heartbeatRuns.companyId), eq(nativeRunFinalizations.issueId, heartbeatRuns.nativeIssueId)))
|
||||
.where(and(cancelledWorkspaceSourceLease(), eq(heartbeatRuns.runtimeMode, "native"),
|
||||
eq(heartbeatRuns.status, "cancelled"), eq(nativeRunFinalizations.phase, "terminal_failure"),
|
||||
isNull(nativeRunFinalizations.resultId), isNull(nativeRunFinalizations.leaseOwner),
|
||||
or(isNull(nativeRunFinalizations.resultId), eq(nativeRunFinalizations.failureCode, "native_retry_cancelled")),
|
||||
isNull(nativeRunFinalizations.leaseOwner),
|
||||
sql`(${heartbeatRuns.runnerProfileJson}->'nativeWorkspaceSync'->>'state' = 'prepared' OR ${heartbeatRuns.runnerProfileJson}->'remoteWorkspace'->>'state' = 'prepared')`,
|
||||
sql`${heartbeatRuns.resultJson}->'nativeCancellation'->>'dispatchState' = 'acknowledged'`,
|
||||
cursor ? gt(heartbeatRuns.id, cursor) : undefined,
|
||||
@@ -289,7 +298,7 @@ export function registerCancelledWorkspacePreservationWork(db: Db,
|
||||
eq(nativeRunFinalizations.runId, run.id), eq(nativeRunFinalizations.companyId, run.companyId),
|
||||
eq(nativeRunFinalizations.issueId, run.nativeIssueId!),
|
||||
));
|
||||
if (finalization?.resultId) return null;
|
||||
if (finalization?.resultId && !stoppedAcceptedCopyback(finalization)) return null;
|
||||
if (finalization?.phase !== "terminal_failure" || finalization.leaseOwner)
|
||||
return { ...entry, dueAt: Date.now() + 30_000 };
|
||||
if (await cancelledWorkspaceSource(db, run)) await preserveNativeFailedWorkspace(db, run, { allowPreterminal: true, requireCancelledStop: true });
|
||||
|
||||
Reference in new issue
Block a user