mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 15:55:46 +01:00
fix(workspaces): hand on a proven-unchanged preparation epoch instead of abandoning it (#20736)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native run on a remote sandbox first restores its workspace's saved version, publishes that root as its dirty epoch, and only then starts work > - A restart that kills the run in that window leaves the epoch dirty. The ownership settle (`settlePreparationOnlyRemoteWorkspaceEpoch`) always abandons it: the root is forgotten and the retry restores the saved version into a new directory > - The saved provider session still names the old directory, which is what #20712 and #20723 now have to handle > - In the common case the killed preparation only restored the saved version and changed nothing. A durable proof for exactly that already exists (`proveRemoteWorkspaceUnchanged`, used when the next admission finds an ended writer) > - This pull request makes the settle use that proof first, and keeps abandoning whenever the proof fails > - The benefit is a stable workspace root across a restart during preparation, with no change for anything unproven ## Linked Issues or Issue Description Refs #20712 **What happened?** In restart-chaos waves, a server kill during a follow-up's preparation (right after the workspace restore published) made the ownership settle abandon the epoch. The retry restored into `<workspace>-<epoch>-<lease>` instead of the published root, and the inherited session checkpoint named a root that no longer existed. **Expected behavior** When durable records prove the killed preparation only restored the saved version, the workspace is handed on at that saved version and keeps its root. Anything unproven is abandoned exactly as before. **Steps to reproduce** 1. Run a remote native task to a saved version, then start a follow-up or continuation run. 2. Kill the server after the follow-up's restore publishes (crash point `remote_workspace.prepare.after_publish_before_provision`). 3. Let ownership reconciliation settle the dirty epoch, then let the retry prepare. ## What Changed - `recovery/remote-workspace-unsaved-files-settlement.ts`: before it abandons a preparation-only epoch, the settle calls the existing `releaseUnchangedRemoteWorkspaceWriterById` (lock the workspace, check the writer, then `releaseUnchangedRemoteWorkspaceWriter`). That releases the writer at its saved version (dirty false, writer cleared, location and root kept, baseline marked released, run reference set to `saved`) only when `proveRemoteWorkspaceUnchanged` holds. The proof requires all of these durable records: - the unchanged-baseline marker for this exact epoch, run and saved version. It is written in the same transaction that publishes the restored root, so a kill mid-restore leaves no marker; - no recorded process, process group, process start or provider session; - no execution-start run event, no native result and no native runner checkpoint (the controller writes one before every runner command); - no workspace operation other than the file-neutral restore, finalize and recovery receipts (provisioning and saves record theirs before they touch files); - every workspace repository already in the saved version. - If the proof fails for any reason, the settle abandons the epoch exactly as before. The existing settle preconditions (run terminal, still `preparing`, no finalization, no work-started event, no save in flight) are unchanged and run first. - `packages/shared/src/crash-point-registry.ts` and `remote-workspaces.ts`: add the crash point `remote_workspace.prepare.after_publish_before_provision`, so the chaos harness can kill exactly at this boundary. It counts only a preparation that published an unchanged baseline (a restore of a saved version), and it is a no-op unless a test arms it. `tests/runner-e2e/source-sdk-public-inputs.json` is refreshed because the registry file is a pinned SDK input. - Test `ownership-reconciliation-report.test.ts`: a proven-clean epoch is handed on with its root kept, with no abandonment receipt, and the writer hold is free for the retry. These cases are still abandoned exactly as before: no baseline (an epoch saved by the previous release), a baseline for another epoch, an already-released baseline, a provisioning operation. A run with a recorded process is still neither handed on nor abandoned by this settle. Previous-release story: an epoch saved by a release that did not write the baseline marker has no proof, so it is abandoned as before. No persisted shape changes; the marker already exists on master. ## Verification - Red: with the release branch disabled, the new test fails on the clean case (the epoch is abandoned). Green with the change. - `cd server && npx vitest run src/__tests__/ownership-reconciliation-report.test.ts src/services/remote-workspace-snapshot-reuse.test.ts src/__tests__/crash-point-call-sites.test.ts` passes. `packages/shared` `crash-points.test.ts` passes. `tsc --noEmit -p server` passes. - Live Daytona waves (claude, workload S1, the new crash point at the continuation's preparation (`:2` before the point was gated to saved restores; `:1` now), image built from master `e039be9202`, one task each): | | before (release disabled) | after | | --- | --- | --- | | Killed epoch 2 | abandoned (`workspace.remote_epoch_abandoned`) | handed on (`workspace.snapshot_skipped_unchanged`, epoch 2) | | Retry root | new root `…-4-ecf1c81e…` | same published root `…-1-33e93b90…` | | Retry `task.prepare` | 4598 ms | 4731 ms | | Retry restore / environment acquire | 1141 / 1381 ms | 1106 / 1478 ms | | Task result | done, no Retry | done, no Retry | | Stopped or orphan sandbox at rest | none | none | Preparation time does not change in this setup. The Chaos Lab environment uses ephemeral leases, so every run gets a fresh sandbox and a full restore with or without this change, and the killed run's sandbox is destroyed either way. The gain is the stable root, which keeps the saved session's working directory valid. ## Risks - Low. The release path is the same one admission already uses for an ended writer, under the same proof. The settle now tries it first. - A proof false positive would keep a root whose files changed. The proof refuses on every record that precedes a write, and on any missing or older-release evidence. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit, test execution). ## 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 - [x] All Paperclip CI gates are green - [x] 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
62b7733161
commit
9574065a28
5 files changed
+86
-1
No files matched your search
@@ -28,6 +28,13 @@ export type CrashPointDefinition = {
|
||||
export const CRASH_POINT_NAME_PATTERN = /^[a-z][a-z0-9_]*\.[a-z][a-z0-9_]*\.(before|after)_[a-z0-9_]+$/;
|
||||
|
||||
export const CRASH_POINTS = {
|
||||
"remote_workspace.prepare.after_publish_before_provision": {
|
||||
description: "A run restored its remote workspace's saved version and published the root as its dirty epoch, with the unchanged baseline; no provisioning, runner command or agent process has started. Recovery must hand the workspace on at its saved version and keep the root, not abandon the epoch.",
|
||||
environments: ["daytona"],
|
||||
kinds: ["remote_workspace_unsaved_files"],
|
||||
invariants: ["I2", "I7"],
|
||||
prerequisite: "a previously saved remote workspace: the point counts only a preparation that restores a saved version (a follow-up or continuation run, for example workload S1), never a workspace's first preparation",
|
||||
},
|
||||
"sandbox.create.after_intent_before_provider": {
|
||||
description: "The sandbox_creation_operations intent is committed; the provider create call has not been made. Recovery must settle the intent without a sandbox to find.",
|
||||
environments: ["daytona"],
|
||||
|
||||
@@ -527,6 +527,66 @@ describe("report-only ownership reconciliation", () => {
|
||||
.toEqual({ kind: "free" });
|
||||
});
|
||||
|
||||
it("hands on a preparation-only epoch at its saved version only when the unchanged baseline is proven", async () => {
|
||||
// Each writer restored saved version `snapshotId` and was killed in
|
||||
// preparation. Only `clean` has nothing after the restore; every other
|
||||
// writer lacks the proof and keeps today's abandonment.
|
||||
const cases = ["clean", "noBaseline", "otherEpochBaseline", "releasedBaseline", "provisioned", "processStarted"] as const;
|
||||
const snapshotId = randomUUID();
|
||||
const setup = Object.fromEntries(cases.map(name => [name, {
|
||||
runId: randomUUID(), workspaceId: randomUUID(), leaseId: randomUUID(), providerLeaseId: `sandbox-${randomUUID()}`,
|
||||
}])) as Record<(typeof cases)[number], { runId: string; workspaceId: string; leaseId: string; providerLeaseId: string }>;
|
||||
const remoteCwd = (name: string) => `/home/daytona/paperclip-workspace/.paperclip-workspaces/${setup[name as keyof typeof setup].workspaceId}`;
|
||||
await db.insert(heartbeatRuns).values(cases.map(name => {
|
||||
const s = setup[name];
|
||||
return { id: s.runId, companyId, agentId, status: "failed", invocationSource: "manual", executionStage: "preparing",
|
||||
finishedAt: minutesAgo(2), errorCode: "process_lost",
|
||||
...(name === "processStarted" ? { processPid: 4_194_303 } : {}),
|
||||
runnerProfileJson: { remoteWorkspace: { schema: "paperclip.remote-workspace-run.v1", workspaceId: s.workspaceId,
|
||||
leaseId: s.leaseId, providerLeaseId: s.providerLeaseId, remoteCwd: remoteCwd(name), epoch: 2, state: "prepared", snapshotId } } };
|
||||
}));
|
||||
await db.insert(executionWorkspaces).values(cases.map(name => {
|
||||
const s = setup[name];
|
||||
const baseline = name === "noBaseline" ? {}
|
||||
: { remoteWorkspaceUnchangedBaseline: { schema: "paperclip.remote-workspace-unchanged-baseline.v1",
|
||||
epoch: name === "otherEpochBaseline" ? 1 : 2, runId: s.runId, snapshotId,
|
||||
...(name === "releasedBaseline" ? { releasedAt: minutesAgo(5).toISOString() } : {}) } };
|
||||
return { id: s.workspaceId, companyId, mode: "isolated_workspace", strategyType: "test", name,
|
||||
metadata: { ...baseline, remoteWorkspace: { schema: "paperclip.remote-workspace.v1", epoch: 2, snapshotId,
|
||||
source: { kind: "git", repoUrl: "https://example.invalid/repo.git", ref: null }, dirty: true, writerRunId: s.runId,
|
||||
location: { environmentId: randomUUID(), leaseId: s.leaseId, providerLeaseId: s.providerLeaseId, remoteCwd: remoteCwd(name) } } } };
|
||||
}));
|
||||
// The restore itself is file-neutral; a provisioning command is not.
|
||||
await db.insert(workspaceOperations).values(cases.map(name => ({ companyId, executionWorkspaceId: setup[name].workspaceId,
|
||||
heartbeatRunId: setup[name].runId, phase: "remote_workspace_restore", status: "succeeded", metadata: { epoch: 2, snapshotId } })));
|
||||
await db.insert(workspaceOperations).values({ companyId, executionWorkspaceId: setup.provisioned.workspaceId,
|
||||
heartbeatRunId: setup.provisioned.runId, phase: "remote_workspace_provision", status: "failed", metadata: { epoch: 2 } });
|
||||
const read = async (name: (typeof cases)[number]) => (await db.select().from(executionWorkspaces)
|
||||
.where(eq(executionWorkspaces.id, setup[name].workspaceId)))[0]!;
|
||||
|
||||
await runOwnershipReconciliationPass(db, { state: createOwnershipReportState() });
|
||||
|
||||
const clean = await read("clean");
|
||||
expect(readRemoteWorkspaceAuthority(clean.metadata)).toMatchObject({ epoch: 2, dirty: false, writerRunId: null, snapshotId,
|
||||
location: { leaseId: setup.clean.leaseId, providerLeaseId: setup.clean.providerLeaseId, remoteCwd: remoteCwd("clean") } });
|
||||
expect(clean.metadata?.remoteWorkspaceAbandonedEpochs).toBeUndefined();
|
||||
expect(clean.metadata?.remoteWorkspaceUnchangedBaseline).toMatchObject({ releasedAt: expect.any(String) });
|
||||
const [cleanRun] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, setup.clean.runId));
|
||||
expect(cleanRun.runnerProfileJson?.remoteWorkspace).toMatchObject({ state: "saved", epoch: 2 });
|
||||
// The retry is admitted at once, and the killed run's sandbox is no longer a prepared source holding it.
|
||||
expect(await readRemoteWorkspaceWriterHold(db, { companyId, authority: readRemoteWorkspaceAuthority(clean.metadata)!,
|
||||
runId: randomUUID() })).toEqual({ kind: "free" });
|
||||
// A recorded process is runtime work: neither handed on nor abandoned here.
|
||||
expect(readRemoteWorkspaceAuthority((await read("processStarted")).metadata))
|
||||
.toMatchObject({ epoch: 2, dirty: true, writerRunId: setup.processStarted.runId });
|
||||
for (const name of cases.filter(name => name !== "clean" && name !== "processStarted")) {
|
||||
const row = await read(name);
|
||||
expect(readRemoteWorkspaceAuthority(row.metadata), name)
|
||||
.toMatchObject({ epoch: 3, dirty: false, writerRunId: null, location: null });
|
||||
expect(row.metadata?.remoteWorkspaceAbandonedEpochs, name).toEqual([expect.objectContaining({ epoch: 2, runId: setup[name].runId })]);
|
||||
}
|
||||
});
|
||||
|
||||
it("settles a workspace command whose process died mid-command, and keeps live or unprovable ones", async () => {
|
||||
const run = (id: string, status: string) => ({ id, companyId, agentId, status, invocationSource: "manual",
|
||||
...(status === "running" ? {} : { finishedAt: minutesAgo(2) }) });
|
||||
|
||||
@@ -3,6 +3,7 @@ import { executionWorkspaces, heartbeatRunEvents, heartbeatRuns, nativeRunFinali
|
||||
import { readRemoteWorkspaceAuthority } from "@paperclipai/shared";
|
||||
import type { OwnerProof } from "../owner-proof.js";
|
||||
import { abandonUnverifiedRemoteWorkspaceEpoch } from "../remote-workspace-epoch-abandonment.js";
|
||||
import { releaseUnchangedRemoteWorkspaceWriterById } from "../remote-workspaces.js";
|
||||
import { notifyTerminalWorkspaceWork } from "../terminal-workspace-work.js";
|
||||
import type { InProgressRecord, SettleOutcome } from "./ownership-registry.js";
|
||||
|
||||
@@ -43,6 +44,16 @@ export async function settlePreparationOnlyRemoteWorkspaceEpoch(db: Db, record:
|
||||
eq(workspaceOperations.phase, "remote_workspace_snapshot"), inArray(workspaceOperations.status, ["running", "pending"]),
|
||||
sql`${workspaceOperations.metadata}->>'epoch' = ${String(authority.epoch)}`)).limit(1);
|
||||
if (save) return { settled: false, reason: "save_in_flight" };
|
||||
// A preparation that restored the saved version and then did nothing else
|
||||
// left exactly that version in its root. When the durable unchanged-baseline
|
||||
// proof holds, hand the workspace on at its saved version and keep the root,
|
||||
// so the retry reuses the same directory and the saved session still names
|
||||
// it. Anything unproven (no baseline from an older release, a provisioning
|
||||
// command, a runner command, a process, a kill before the restore published)
|
||||
// is abandoned exactly as before.
|
||||
if (await releaseUnchangedRemoteWorkspaceWriterById(transaction, { companyId, workspaceId: record.recordId, runId })) {
|
||||
return { settled: true, reason: context.proof.reason };
|
||||
}
|
||||
await notifyTerminalWorkspaceWork(tx, `workspace:${record.recordId}`);
|
||||
try {
|
||||
await abandonUnverifiedRemoteWorkspaceEpoch(transaction, {
|
||||
|
||||
@@ -8,6 +8,7 @@ import { mkdtemp, readFile, rm, statfs } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { and, eq, inArray, isNull, or, sql } from "drizzle-orm";
|
||||
import { crashPoint } from "@paperclipai/shared/crash-points";
|
||||
import {
|
||||
companies, environmentLeases, executionWorkspaces, executionWorkspaceSnapshots, heartbeatRuns, issues, workspaceOperations, type Db,
|
||||
} from "@paperclipai/db";
|
||||
@@ -618,6 +619,12 @@ export async function prepareRemoteWorkspace(input: {
|
||||
return { workspace: savedWorkspace!, lease: savedLease! as EnvironmentLease, remoteRoot, reference: runReference };
|
||||
});
|
||||
target.remoteCwd = remoteRoot;
|
||||
// Counted only for a preparation that published an unchanged baseline (a
|
||||
// restore of a saved version), so the point exercises the saved-root handoff.
|
||||
const publishedBaseline = readUnchangedBaseline(prepared.workspace.metadata);
|
||||
if (publishedBaseline?.runId === input.runId && publishedBaseline.epoch === prepared.reference.epoch && !publishedBaseline.releasedAt) {
|
||||
crashPoint("remote_workspace.prepare.after_publish_before_provision");
|
||||
}
|
||||
if (restoredExport?.cleanupPending) await cleanupSavedExports({ db: input.db, companyId: input.workspace.companyId,
|
||||
workspaceId: input.workspace.id, location: readRemoteWorkspaceAuthority(prepared.workspace.metadata)!.location!, target,
|
||||
assertOwned: input.assertOwned ?? (async () => undefined) });
|
||||
|
||||
@@ -226,7 +226,7 @@
|
||||
"packages/shared/src/connector-identity.test.ts": "d58357707faefa7c98bb906cdaeb4db39d31081a",
|
||||
"packages/shared/src/connector-identity.ts": "5ede83c9c5225dbe8af9cf3a49c74e477ef5b955",
|
||||
"packages/shared/src/constants.ts": "3149ee1cc5ffa34b50ecd493276b3a70fc46adc2",
|
||||
"packages/shared/src/crash-point-registry.ts": "708b1e431032ecf370fd259ab180d11e4c0b5cf3",
|
||||
"packages/shared/src/crash-point-registry.ts": "8d6f60e8ad8f90338fb3047955bb32160f50b9ae",
|
||||
"packages/shared/src/crash-points.test.ts": "804a3acfd7a51010f31dbc3fb9a4dca771562c99",
|
||||
"packages/shared/src/crash-points.ts": "d9f31d95c78f32290190d7728f42cd9573681fc8",
|
||||
"packages/shared/src/customer-success.ts": "a2d02aaa8223164c0eae4267b2a6da82242eaa6f",
|
||||
|
||||
Reference in new issue
Block a user