mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
fix(recovery): let preservation owners reopen retained sources (#17053)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Recovery must save retained workspace files before ordinary execution can resume. > - The environment runtime blocks access to retained sources to prevent unsafe reuse or deletion. > - The recovery worker also used that ordinary resume path. > - The guard returned no lease, so recovery reported an invalid source binding before it could save files. > - This pull request gives the physical preservation owner a separate, source-bound resume path. > - Ordinary execution and destructive cleanup keep their existing guards. ## Linked Issues or Issue Description Refs #16965. **What happened?** A retained workspace with a workspaceRestoreRecovery marker could not recover through the real environment runtime. The runtime returned null before calling the provider. Recovery classified this as source_binding_invalid. Earlier recovery fixtures replaced the runtime facade, so they did not exercise this guard. **Expected behavior** The verified preservation owner can reopen its exact stopped allocation to save files. Other callers remain blocked. Recovery preserves newer local files and does not start another agent or repeat a completed external action. **Steps to reproduce** 1. Seed a terminal source run with a retained sandbox, a recovery action, and the workspaceRestoreRecovery lease marker. 2. Give the source a verified stop receipt. 3. Call recoverRetainedWorkspace through environmentRuntimeService. 4. Before this change, the recovery action returns to blocked with source_binding_invalid. No files are copied. **Paperclip version or commit** Reproduced on master at65f59e339a. The same failure was observed in a served build containing #16965. ## What Changed - Add an internal preservation resume method. It validates the exact company, task, run, action, lease, environment, current physical owner, and stop-only cleanup claim before provider access. - Keep ordinary resume and destructive cleanup blocked for protected sources. - Run all retained-workspace fixtures through the assembled environment runtime. Add a failing-before/passing-after protected-source regression and four rejection cases. - Use the same runtime path and protection marker in the opt-in Daytona interruption probe. Allow a verified immutable snapshot UUID as its input. - Document the authority boundary and verification recipes. ## Verification - Red: the protected-source regression failed with expected recovered, received blocked. It recorded source_binding_invalid before provider resume. - Green: 235 tests passed across workspace recovery, durable task Retry, and environment runtime. These include saved files, newer local files, bounded retries, competing owners, and no extra run or wake. - pnpm -r typecheck and pnpm build passed. The initial sandboxed typecheck could not open a tsx IPC pipe; the same command passed with local process access. - node scripts/check-maps.mjs and git diff --check passed. - Live Daytona qualification passed using the same immutable snapshot ID as the affected deployment. Export was interrupted after a real download; a second controller restored the files and verified one original agent start, one simulated external action, one saved snapshot, and a stopped retained source. The disposable sandbox was deleted after verification. - Greptile reviewed804dfe24efat 5/5 with no actionable findings or unresolved review threads. - Full current-commit CI passed: https://github.com/paperclipai/paperclip/actions/runs/39540771747. Required ci / verify and ci / e2e passed. Superagent, Socket, and Snyk checks passed. - Full local pnpm test:run did not pass: 22,992 tests passed, one managed-run identity test timed out at 60 seconds, and 100 tests skipped across the first general-server phase. The command stopped before its later phases. The unchanged agent-identity.test.ts suite then passed all 11 tests in isolation in 15.53 seconds. The original timeout remains recorded; it is not reclassified as a full local pass. The same commit's complete CI test matrix passed. - The optional Paperclip Review completed with a neutral advisory 5/5 on the same head and no actionable findings. It inspected all eight changed files and connected code. It did not independently run tests or the provider probe. - Hosted ordinary Retry recorded one durable intent and no successor run while file preservation remained blocked. The existing published review stayed unchanged. Hosted recovery remains unverified until this follow-up is deployed. A Retry acknowledgement alone does not prove recovery. ## Risks - Recovery can reopen a retained sandbox. The dedicated path requires the current physical ownership claim and the exact source's stop-only intent. It does not clear the ordinary protection marker. - Provider failures retain the source and use the existing bounded recovery budget. The provider cannot substitute another allocation. - The live probe uses a disposable sandbox and simulated agent/external-effect counters. It does not prove a live LLM or GitHub review journey. - No schema, public API, permission, budget, pause, or approval changes. ## Model Used OpenAI GPT-6 through Codex, with repository inspection, code execution, browser verification, and test tools. The exact served model variant and context window are not exposed by this session. ## 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 (focused suites; full suite still running) - [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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
30b753f614
commit
eaacadeba3
8 files changed
+131
-9
No files matched your search
@@ -15,6 +15,7 @@ import { NOOP_PLUGIN_TRACER } from "@paperclipai/plugin-sdk";
|
||||
import { startEmbeddedPostgresTestDatabase } from "../src/__tests__/helpers/embedded-postgres.js";
|
||||
import { readSystemRecovery } from "../src/services/system-recovery-state.js";
|
||||
import { recoverRetainedWorkspace } from "../src/services/workspace-recovery.js";
|
||||
import { classifyEnvironmentCapabilities, environmentRuntimeService } from "../src/services/environment-runtime.js";
|
||||
import { attachPreservedWorkspaceSnapshots } from "../src/services/workspace-recovery-snapshot.js";
|
||||
import { performSyncOut } from "../../packages/plugins/sandbox-providers/daytona/src/file-sync.js";
|
||||
|
||||
@@ -68,7 +69,11 @@ it.skipIf(!live || !worker)("provider controller", async () => {
|
||||
},
|
||||
syncOut: operations => performSyncOut({ sandbox, operations, remoteDir: f.remoteCwd, timeoutSeconds: 120 }),
|
||||
} };
|
||||
const environmentRuntime = {
|
||||
const driver = {
|
||||
driver: "sandbox",
|
||||
acquireRunLease: async () => { throw new Error("Recovery must not acquire another sandbox"); },
|
||||
releaseRunLease: async () => { throw new Error("Recovery must not destroy its retained source"); },
|
||||
resolveCapabilities: async () => classifyEnvironmentCapabilities({ verifiedMethods: [] }),
|
||||
resumeRunLease: async () => {
|
||||
await sandbox.start(120); await sandbox.refreshData();
|
||||
if (sandbox.state !== "started") throw new Error("Provider did not confirm start");
|
||||
@@ -80,6 +85,7 @@ it.skipIf(!live || !worker)("provider controller", async () => {
|
||||
return { providerLeaseId: sandbox.id, state: "stopped" };
|
||||
},
|
||||
};
|
||||
const environmentRuntime = environmentRuntimeService(db, { drivers: [driver] });
|
||||
await recoverRetainedWorkspace({ db, ...f, environmentRuntime });
|
||||
const [action] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.id, f.actionId));
|
||||
const state = readSystemRecovery(action.evidence);
|
||||
@@ -90,7 +96,9 @@ it.skipIf(!live || !worker)("provider controller", async () => {
|
||||
|
||||
it.skipIf(!live || Boolean(worker))("preserves provider files across an interrupted export and controller restart without restarting the old agent", async () => {
|
||||
const image = process.env.PAPERCLIP_RECOVERY_IMAGE;
|
||||
if (!image?.includes("@sha256:")) throw new Error("An immutable qualification image is required");
|
||||
const snapshotId = process.env.PAPERCLIP_RECOVERY_SNAPSHOT_ID;
|
||||
if (!image?.includes("@sha256:") && !/^[0-9a-f-]{36}$/i.test(snapshotId ?? ""))
|
||||
throw new Error("An immutable qualification image or snapshot ID is required");
|
||||
const directory = await mkdtemp(path.join(os.tmpdir(), "paperclip-provider-recovery-"));
|
||||
const priorHome = process.env.PAPERCLIP_HOME;
|
||||
process.env.PAPERCLIP_HOME = path.join(directory, "home");
|
||||
@@ -100,11 +108,16 @@ it.skipIf(!live || Boolean(worker))("preserves provider files across an interrup
|
||||
const db = createDb(database.connectionString);
|
||||
const daytona = await client();
|
||||
let sandbox;
|
||||
const evidence = { image, interrupted: false, restored: false, oldAgentStarts: null, externalOperations: null,
|
||||
const evidence = { image, snapshotId, interrupted: false, restored: false, oldAgentStarts: null, externalOperations: null,
|
||||
prematureCleanup: false, controllerProcesses: [], snapshotCount: 0 };
|
||||
try {
|
||||
sandbox = await daytona.create({ image, autoStopInterval: 10, autoArchiveInterval: 60,
|
||||
resources: { cpu: 2, memory: 4, disk: 10 }, labels: { purpose: "paperclip-recovery-qualification" } }, { timeout: 300 });
|
||||
if (snapshotId) {
|
||||
const snapshot = await daytona.snapshot.get(snapshotId);
|
||||
expect(snapshot.id).toBe(snapshotId);
|
||||
expect(snapshot.state).toBe("active");
|
||||
}
|
||||
sandbox = await daytona.create({ ...(snapshotId ? { snapshot: snapshotId } : { image, resources: { cpu: 2, memory: 4, disk: 10 } }),
|
||||
autoStopInterval: 10, autoArchiveInterval: 60, labels: { purpose: "paperclip-recovery-qualification" } }, { timeout: 300 });
|
||||
const remoteCwd = "/tmp/paperclip-recovery-fixture";
|
||||
await sandbox.process.executeCommand(`mkdir -p ${remoteCwd}`, undefined, undefined, 30);
|
||||
await sandbox.process.createSession("old-agent");
|
||||
@@ -124,7 +137,8 @@ it.skipIf(!live || Boolean(worker))("preserves provider files across an interrup
|
||||
const [lease] = await db.insert(environmentLeases).values({ companyId, issueId, heartbeatRunId: runId,
|
||||
environmentId: environment.id, provider: "daytona", providerLeaseId: sandbox.id,
|
||||
leasePolicy: "retain_on_failure", status: "released", releasedAt: new Date(),
|
||||
metadata: { remoteCwd, driver: "sandbox", pluginId: randomUUID(), sandboxProviderPlugin: true } }).returning();
|
||||
metadata: { remoteCwd, driver: "sandbox", pluginId: randomUUID(), sandboxProviderPlugin: true,
|
||||
workspaceRestoreRecovery: { schema: "paperclip.workspace-restore-recovery.v1", runId } } }).returning();
|
||||
const recovery = { schema: "paperclip.workspace-restore-recovery.v1", leaseIds: [lease.id] };
|
||||
await db.update(heartbeatRuns).set({ resultJson: { workspaceRestoreRecovery: recovery, workspaceRestoreFailure: "restore_failed" } })
|
||||
.where(eq(heartbeatRuns.id, runId));
|
||||
|
||||
Reference in new issue
Block a user