mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
fix(workspaces): ignore killed-transfer scratch in sparse checkpoints (#19730)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run in a remote sandbox copies its workspace back to the host after the turn. A sparse checkpoint copies only changed entries, and the host validates each entry against the sandbox's own manifest > - Every Daytona transfer stages a `.paperclip-upload-<uuid>` scratch file at the sandbox workspace root and removes it only in the same process > - A control-plane kill during any transfer leaves that scratch behind. The sparse checkpoint then recorded it as a workspace file, but providers never export it. Every later copyback and every saved-work recovery attempt failed with "Workspace checkpoint payload mismatch" > - The failed recovery then left an accepted Retry showing "Paperclip is finishing the saved result" with no run > - This pull request stops recording transfer scratch, makes the checkpoint export an exact copy, names the failing entry, and gives a Retry behind a spent recovery an honest reason and a working action > - The benefit is that a restart during a transfer no longer makes a task's saved work permanently unrecoverable ## Linked Issues or Issue Description Refs #19721 (RC-50: the killed copyback owner; this PR covers the separate RC-50b cause) **What happened?** Restart-chaos waves s12/t01 and s14/t05 (Claude on Daytona). After a SIGKILL of the control plane, the native result was accepted, and then workspace copyback failed with "Workspace checkpoint payload mismatch". In s14 all 6 saved-work recovery attempts failed the same way. A Retry accepted during recovery kept `retryWait.reason = finalizing_saved_result` after the recovery budget was spent. No run was created. **Steps to reproduce** 1. Start a native Claude run in a Daytona sandbox. 2. SIGKILL the control plane while any provider transfer is in flight. 3. Let the run finish its turn and copy its workspace back. **Expected behavior** Copyback succeeds. Provider scratch is not workspace content. A Retry behind a spent recovery names the failure and can start a new recovery. **Actual behavior** The s14 sandbox (6f4a0b6a) still held `/home/daytona/paperclip-workspace/.paperclip-upload-d99fe36b-5acc-437b-96ad-85d5ff9e79c4.tar`. Its mtime was 03:29:23, the second of the K4 SIGKILL. The Daytona provider stages transfer scratch at `remoteDir`, which is the lease `remoteCwd` (the workspace root). The runtime workspace wipe deliberately preserves `.paperclip-upload-*`. The checkpoint helper (`workspace-checkpoint.ts`, `checkpointMain`) recorded the stale tar. The ordinary export excludes `.paperclip-upload*`, so the host received nothing for that entry. `validateCheckpointPayload` then threw "payload mismatch" on every attempt. The file-mode hypothesis was disproved: the sandbox umask is 022 and the scratch is mode 644. ## What Changed - `packages/adapter-utils/src/exclude-patterns.ts`: new `isSandboxTransferScratchPath`, which matches a root-level `.paperclip-upload-*` entry. This agrees with the wipe's preserve rule in `buildWorkspaceTarExtractCommand`. - `packages/adapter-utils/src/workspace-checkpoint.ts`: the sandbox checkpoint helper skips that scratch. A mismatch error now names the entry and its recorded, baseline and received forms (kind, octal mode, short hash; never contents). - `packages/adapter-utils/src/sandbox-managed-runtime.ts`: the checkpoint export requests `preserveSourceEntries: true`. The checkpoint output is an exact copy that the host validates entry by entry. An ordinary export applies the host umask and drops `._*` and `.paperclip-upload*` names at any depth. That gives the same permanent mismatch for a legitimate user file such as `._summary`, `notes/.paperclip-upload-plan.md`, or a `0775` script. - `server/src/services/explicit-native-continuation.ts` and `task-interruption-presentation.ts`: when the run's saved-work recovery is exhausted, a waiting Retry reports `saved_work_recovery_failed` with Retry available, instead of `finalizing_saved_result`. The existing task Retry path starts one new recovery generation. - `doc/execution-semantics.md`: a short additive note. ## Verification - `packages/adapter-utils/src/workspace-checkpoint-capture.test.ts` uses the real checkpoint helper and an export that behaves like Daytona's ordinary export: - Before the fix: `Workspace checkpoint payload mismatch at ".paperclip-upload-d99fe36b-….tar": sandbox recorded file mode=644 …; baseline absent; received absent`. That is the wave error. - After the fix: the scratch is not recorded, and validation passes. - A second case shows that reserved-looking user names and `0775` modes need the exact export. - `packages/adapter-utils/src/workspace-checkpoint.test.ts` is an end-to-end `prepareSandboxManagedRuntime` restore through a provider model. In that model, ordinary exports drop reserved names and apply the umask, and preserve exports copy exactly. Two red checks: - Without `preserveSourceEntries`, it fails with the mismatch on `._summary`. - Without the scratch skip, the stale tar leaks into the host workspace. - Green: the agent file, the `._summary` file and the `0775` mode arrive, and no scratch arrives. - `server/src/services/workspace-recovery-preparation-gates.test.ts`, "names a spent saved-work recovery behind an accepted Retry, and the next Retry finishes it" (s14 shape): - Red: `finalizing_saved_result` after 6 failed attempts. - Green: `saved_work_recovery_failed` with `canRetry: true`. The next Retry starts a new generation, recovery completes, and the Retry continues in exactly one new run. - Suites: adapter-utils checkpoint, capture, sandbox-managed-runtime, seed-reuse and managed-seed (168 passed). Server preparation-gates (47), explicit-native-continuation and task-interruption-presentation (455), workspace-recovery admission and retry-copyback, and native-workspace-finalizer. `tsc --noEmit` is clean for adapter-utils and server. - Live evidence: I started the stopped s14 sandbox read-only, listed its workspace, and stopped it again. No Daytona replay was run. The runner content ID of this checkout matches no built image. ## Risks - A user file named `.paperclip-upload-*` directly at the workspace root is no longer captured by a sparse checkpoint. Providers already drop that name from every ordinary export, and the runtime already reserves it at the root. - The checkpoint export now uses the provider's exact-copy mode. A workspace file with setuid, setgid or sticky bits is refused by that mode, as it is for the existing exact-copy transfers. The ordinary export would have stripped those bits and failed validation anyway. - The stale scratch tar stays in the sandbox. It is ignored, not deleted, because a concurrent transfer can own a file with the same prefix. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, through Claude Code with tool use (shell, file edits, test runs, embedded Postgres, a read-only Daytona sandbox listing). ## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
b21884bdcf
commit
c45c7cb854
9 files changed
+212
-11
No files matched your search
@@ -405,15 +405,23 @@ export async function admitExplicitNativeContinuation(input: {
|
||||
// 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(
|
||||
const live = await db.select({
|
||||
ownerHold: sql<boolean>`${issueRecoveryActions.evidence}#>>'{workspaceOwnerRecovery,state}' = 'operator_required'
|
||||
and ${issueRecoveryActions.evidence}->'workspaceOwnerStop' is null`,
|
||||
recoveryState: sql<string | null>`${issueRecoveryActions.evidence}#>>'{systemRecovery,state}'`,
|
||||
}).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.");
|
||||
sql`${issueRecoveryActions.evidence} ? 'systemRecovery'`));
|
||||
if (live.some(row => row.ownerHold))
|
||||
return blocked("workspace_owner_stop_required", "A board member must confirm that the previous server stopped copying files. Your Retry is saved.");
|
||||
// A spent recovery budget finishes nothing by itself. Another Retry
|
||||
// starts one new recovery generation (task-retry), so name that action
|
||||
// instead of an endless finalizing wait (RC-50b, s14).
|
||||
if (live.length && live.every(row => row.recoveryState === "exhausted"))
|
||||
return blocked("saved_work_recovery_failed", "Saving the agent's files failed after automatic retries. Retry to try again.");
|
||||
return blocked("finalizing_saved_result", "Paperclip is finishing the saved result.");
|
||||
}
|
||||
}
|
||||
// A repaired export no longer blocks execution. Its machine receipt still
|
||||
|
||||
@@ -83,6 +83,8 @@ const retryWaitMessages: Record<string, string> = {
|
||||
"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.
|
||||
// Retry starts a new recovery generation; the agent does not repeat its work.
|
||||
saved_work_recovery_failed: "Saving the agent's files failed after automatic retries. Retry to try again; the agent will not repeat its work.",
|
||||
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.",
|
||||
|
||||
@@ -362,6 +362,47 @@ describe("automatic retained workspace recovery", () => {
|
||||
}
|
||||
}, 60_000);
|
||||
|
||||
// RC-50b (wave s14/t05): Retry was accepted while saved-work recovery ran.
|
||||
// Every export attempt failed, the budget was spent, and the Retry still said
|
||||
// "finishing the saved result" with no run. It must name the failure and a
|
||||
// second Retry must start one new recovery that finishes the saved result.
|
||||
it("names a spent saved-work recovery behind an accepted Retry, and the next Retry finishes it", async () => {
|
||||
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" });
|
||||
const app = boardRecoveryApp();
|
||||
const first = (await request(app).post(`/api/issues/${f.issueId}/retry`)
|
||||
.send({ observedRunId: f.runId, idempotencyKey: randomUUID() }).expect(202)).body;
|
||||
f.runtime.resumeRunLease.mockRejectedValue(new Error("Workspace checkpoint payload mismatch"));
|
||||
for (let attempt = 0; attempt < 6; attempt++) {
|
||||
await recoverRetainedWorkspace(f.input);
|
||||
await heartbeatService(db).resumeTaskRetry(f.companyId, first.requestId!);
|
||||
await makeRetryDue(f.actionId);
|
||||
}
|
||||
expect(readSystemRecovery((await action(f.actionId)).evidence)).toMatchObject({ state: "exhausted", attempts: 6 });
|
||||
expect((await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, first.requestId!)))[0])
|
||||
.toMatchObject({ status: "deferred_issue_execution", payload: { executionWait: { reason: "saved_work_recovery_failed" } } });
|
||||
expect(await taskInterruptionPresentation(db, issue)).toMatchObject({ state: "failed", canRetry: true,
|
||||
retryWait: { reason: "saved_work_recovery_failed" } });
|
||||
// The working action: one more Retry starts one new recovery generation.
|
||||
f.runtime.resumeRunLease.mockReset();
|
||||
f.runtime.resumeRunLease.mockImplementation(async () => ({ providerLeaseId: f.providerLeaseId, metadata: { remoteCwd: await realpath(f.remote) } }));
|
||||
const second = (await request(app).post(`/api/issues/${f.issueId}/retry`)
|
||||
.send({ observedRunId: f.runId, idempotencyKey: randomUUID() }).expect(202)).body;
|
||||
expect(readSystemRecovery((await action(f.actionId)).evidence)).toMatchObject({ state: "scheduled", attempts: 0 });
|
||||
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, second.requestId!);
|
||||
// After a board handoff the Retry continues from the recovered files in
|
||||
// exactly one new run (as in the s12 case), never another wait.
|
||||
const [settled] = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.id, second.requestId!));
|
||||
expect(settled).toMatchObject({ status: "coalesced", runId: expect.any(String), payload: { taskRetry: { strategy: "continue" } } });
|
||||
expect(await db.select({ id: heartbeatRuns.id, previousRunId: sql<string>`${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,
|
||||
|
||||
Reference in new issue
Block a user