From 4a83ba4050f6e6d334fe4cb6e3ace050097530ce Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 3 Nov 2026 22:30:48 -0600 Subject: [PATCH] fix(workspaces): isolate unpublished preparation directories (#18922) Bind new remote preparation directories to the writer epoch and environment lease. Preserve earlier unpublished files and the recorded path for published warm workspaces. Run the interrupted reused-allocation and saved-file lifecycle regressions with owned Postgres by default, and retain the warm-path recovery checks. Co-Authored-By: Paperclip --- doc/execution-semantics.md | 7 +++ .../remote-workspaces.integration.test.ts | 51 ++++++++++++++----- server/src/services/remote-workspaces.ts | 3 +- .../src/services/workspace-recovery.test.ts | 4 +- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 2f22aac559f..95d333ac571 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -937,6 +937,13 @@ identity, unknown failures, active leases, and incomplete cleanup still require their normal reconciliation and termination proof. Unknown historical setup errors remain outside this policy, and generic recovery cannot renew its exhausted budget. +When an admitted continuation reuses a sandbox after verified cleanup, an +unpublished preparation uses a directory bound to its writer epoch and lease. +Earlier unpublished directories and their receipts stay intact. Repeating the +same preparation retains its directory and receipt check. A published workspace +on the same allocation retains its recorded directory. Automatic bootstrap +timeout retries still require a fresh allocation under the policy above. + ### Runner authentication and ACP startup handshake bounds The native Runner waits up to the ten-second connection setup budget for the diff --git a/server/src/services/remote-workspaces.integration.test.ts b/server/src/services/remote-workspaces.integration.test.ts index d87caddcc77..0ef9e7ccdf3 100644 --- a/server/src/services/remote-workspaces.integration.test.ts +++ b/server/src/services/remote-workspaces.integration.test.ts @@ -38,19 +38,30 @@ import { issueService } from "./issues.js"; import { instanceSettingsService } from "./instance-settings.js"; import { resolveRemoteWorkspaceSource } from "./heartbeat/remote-workspace-source.js"; import { toExecutionWorkspace } from "./execution-workspaces.js"; +import { recoveryPostgresSupport, startRecoveryPostgres } from "../__tests__/helpers/recovery-postgres.js"; const exec = promisify(execFile); -const databaseUrl = process.env.PAPERCLIP_REMOTE_WORKSPACE_TEST_DATABASE_URL; +let databaseUrl = process.env.PAPERCLIP_REMOTE_WORKSPACE_TEST_DATABASE_URL; if (databaseUrl && !/^remote_workspace_[a-z_]*test$/.test(new URL(databaseUrl).pathname.slice(1))) throw new Error("Disposable workspace database required"); -const suite = databaseUrl ? describe : describe.skip; +const suite = databaseUrl || recoveryPostgresSupport.supported ? describe : describe.skip; const s3Bucket = process.env.PAPERCLIP_REMOTE_WORKSPACE_TEST_S3_BUCKET; if (s3Bucket && !s3Bucket.startsWith("paperclip-workspace-")) throw new Error("Disposable workspace S3 bucket required"); suite("remote workspace lifecycle with real Postgres and independent filesystem execution", () => { let db: ReturnType, root: string, storage: StorageService; + let temporary: Awaited> | undefined; let ids: { company: string; agent: string; issue: string; run: string; environment: string; lease: string }; - beforeAll(async () => { await applyPendingMigrations(databaseUrl!); db = createDb(databaseUrl!); }, 120_000); - afterAll(async () => { await closeRegisteredClients(databaseUrl!); }); + beforeAll(async () => { + if (!databaseUrl) { + temporary = await startRecoveryPostgres("remote-workspace-lifecycle-"); + databaseUrl = temporary.connectionString; + } + await applyPendingMigrations(databaseUrl); db = createDb(databaseUrl); + }, 120_000); + afterAll(async () => { + try { if (databaseUrl) await closeRegisteredClients(databaseUrl); } + finally { await temporary?.cleanup(); } + }); beforeEach(async () => { vi.stubEnv("DATABASE_URL", databaseUrl!); vi.stubEnv("PAPERCLIP_SECRETS_MASTER_KEY", "62".repeat(32)); root = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "pc-remote-workspace-lifecycle-"))); @@ -173,40 +184,51 @@ suite("remote workspace lifecycle with real Postgres and independent filesystem expect(Boolean((await lease()).metadata?.remoteExecutionTermination)).toBe(receipt); } - it.each(["receipt", "void"] as const)("recovers a lost preparation reply after %s cleanup and retains the resulting work", async cleanup => { + it.each([["receipt", "fresh"], ["void", "fresh"], ["receipt", "reused"]] as const)( + "continues unpublished preparation after %s cleanup on a %s allocation and retains the resulting work", async (cleanup, allocation) => { await db.update(heartbeatRuns).set({ runtimeMode: "legacy", nativeIssueId: null, issueId: ids.issue }) .where(eq(heartbeatRuns.id, ids.run)); const reserved = await reserve({ kind: "empty" }), first = await target("lost-bootstrap-reply"); + const interrupted = allocation === "reused"; const actual = first.runner!.execute.bind(first.runner!); const execute = vi.spyOn(first.runner!, "execute").mockImplementationOnce(async input => { const result = await actual(input); expect(result.exitCode).toBe(0); - return { ...result, timedOut: true }; + return { ...result, timedOut: !interrupted }; }); - await expect(prepareRemoteWorkspace({ db, storage, workspace: reserved, runId: ids.run, lease: await lease(), target: first })) - .rejects.toMatchObject({ code: "remote_workspace_prepare_timeout" }); - const oldRoot = path.join(first.remoteCwd, ".paperclip-workspaces", reserved.id); + let ownershipChecks = 0; + const preparation = prepareRemoteWorkspace({ db, storage, workspace: reserved, runId: ids.run, lease: await lease(), target: first, + assertOwned: async () => { if (interrupted && ++ownershipChecks === 2) throw new Error("lease_released_before_terminal"); } }); + if (interrupted) await expect(preparation).rejects.toThrow("lease_released_before_terminal"); + else await expect(preparation).rejects.toMatchObject({ code: "remote_workspace_prepare_timeout" }); + const oldRoot = execute.mock.calls[0][0].args?.[2]; + if (!oldRoot) throw new Error("Preparation did not record its directory"); const oldReceipt = await fs.readFile(path.join(oldRoot, ".paperclip-runtime", "workspace-ready.json"), "utf8"); const [failed] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, ids.run)); - await terminalizeLegacyExecution({ db, run: failed!, status: "failed", patch: { - errorCode: "remote_workspace_prepare_timeout", resultJson: { executionRecovery: { kind: "bootstrap", providerWorkStarted: false } }, + await terminalizeLegacyExecution({ db, run: failed!, status: interrupted ? "interrupted" : "failed", patch: { + errorCode: interrupted ? "lease_released_before_terminal" : "remote_workspace_prepare_timeout", + resultJson: { executionRecovery: { kind: "bootstrap", providerWorkStarted: false } }, } }); await releaseBootstrapLease(cleanup === "receipt"); const continueOn = async (name: string) => { const runId = randomUUID(), leaseId = randomUUID(); + const reused = allocation === "reused" && name === "fresh-bootstrap-retry"; + const providerLeaseId = reused ? `allocation-${ids.lease}` : `allocation-${leaseId}`; await db.insert(heartbeatRuns).values({ id: runId, companyId: ids.company, agentId: ids.agent, nativeIssueId: ids.issue, runtimeMode: "native", status: "running" }); const [nextLease] = await db.insert(environmentLeases).values({ id: leaseId, companyId: ids.company, environmentId: ids.environment, heartbeatRunId: runId, issueId: ids.issue, provider: "daytona", - providerLeaseId: `allocation-${leaseId}`, status: "active" }).returning(); + providerLeaseId, status: "active" }).returning(); const next = await reserveRemoteWorkspace({ db, storage, companyId: ids.company, issueId: ids.issue, runId, agentId: ids.agent, environmentId: ids.environment, projectId: null, projectWorkspaceId: null, existingWorkspaceId: reserved.id, source: { kind: "empty" } }); - const remote = await target(name); + const remote = reused ? { ...first, leaseId, sandboxId: providerLeaseId, + runner: { ...first.runner!, execute: actual } } : await target(name); return prepareRemoteWorkspace({ db, storage, workspace: next, runId, lease: nextLease as EnvironmentLease, target: remote }).then(prepared => ({ runId, prepared, remote })); }; const { runId, prepared, remote } = await continueOn("fresh-bootstrap-retry"); + expect(prepared.remoteRoot).not.toBe(oldRoot); await fs.writeFile(path.join(prepared.remoteRoot, "report.md"), "useful recovered result\n"); const { snapshotId } = await saveRemoteWorkspace({ db, storage, workspace: prepared.workspace, runId, lease: prepared.lease, target: remote, assertOwned: async () => undefined }); @@ -597,7 +619,8 @@ suite("remote workspace lifecycle with real Postgres and independent filesystem existingWorkspaceId: reserved.id, source: { kind: "empty" } }); const nextTarget = await target("restore-cleanup-destination"); const sandbox = nextTarget as Extract; - const expectedRemoteRoot = path.join(sandbox.remoteCwd, ".paperclip-workspaces", reserved.id); + const expectedRemoteRoot = path.join(sandbox.remoteCwd, ".paperclip-workspaces", + `${reserved.id}-${readRemoteWorkspaceAuthority(nextWorkspace.metadata)!.epoch}-${nextLease}`); const execute = sandbox.runner!.execute.bind(sandbox.runner!); let failCleanup = true, cleanupAttempts = 0; sandbox.runner!.execute = async input => { diff --git a/server/src/services/remote-workspaces.ts b/server/src/services/remote-workspaces.ts index 18a31f71f5a..4712ae33568 100644 --- a/server/src/services/remote-workspaces.ts +++ b/server/src/services/remote-workspaces.ts @@ -344,7 +344,8 @@ export async function prepareRemoteWorkspace(input: { const sameAllocation = initial.location?.providerLeaseId === input.lease.providerLeaseId && initial.location?.environmentId === input.lease.environmentId; if (initial.location && !sameAllocation && initial.dirty) throw conflict("The prior workspace has unsaved work; recover it before replacing its allocation."); - const remoteRoot = sameAllocation ? initial.location!.remoteCwd : path.posix.join(target.remoteCwd, ".paperclip-workspaces", input.workspace.id); + const remoteRoot = sameAllocation ? initial.location!.remoteCwd : path.posix.join(target.remoteCwd, ".paperclip-workspaces", + `${input.workspace.id}-${initial.epoch}-${input.lease.id}`); const preparationIdentity = createHash("sha256").update(JSON.stringify([input.workspace.companyId, input.workspace.id, input.runId, initial.epoch, input.lease.id, input.lease.providerLeaseId, remoteRoot, initial.source])).digest("hex"); let restoredExport: Awaited> | null = null; diff --git a/server/src/services/workspace-recovery.test.ts b/server/src/services/workspace-recovery.test.ts index f6b2177ab78..c93da025b86 100644 --- a/server/src/services/workspace-recovery.test.ts +++ b/server/src/services/workspace-recovery.test.ts @@ -1815,7 +1815,9 @@ describe("automatic retained workspace recovery", () => { existingWorkspaceId: kind === "same" ? first.workspace.id : null }); const prepared = await prepareRemoteWorkspace({ db, storage, workspace: nextWorkspace, runId: nextRun.id, lease: nextLease as EnvironmentLease, target: { ...target, remoteCwd: resumed!.metadata!.remoteCwd as string } }); - expect(prepared.remoteRoot).toBe(path.join(allocationRoot, ".paperclip-workspaces", nextWorkspace.id)); + const expectedRemoteRoot = kind === "same" ? first.remoteRoot : path.join(allocationRoot, ".paperclip-workspaces", + `${nextWorkspace.id}-${readRemoteWorkspaceAuthority(nextWorkspace.metadata)!.epoch}-${nextLease.id}`); + expect(prepared.remoteRoot).toBe(expectedRemoteRoot); if (kind === "same") expect(await readFile(path.join(prepared.remoteRoot, "saved.txt"), "utf8")).toBe("prior task work\n"); else expect(prepared.remoteRoot).not.toBe(first.remoteRoot); await saveRemoteWorkspace({ db, storage, workspace: prepared.workspace, runId: nextRun.id, lease: prepared.lease,