From 302ada90dfda0e8fbcd8a514d506d310c9843bcb Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 17:05:17 -0600 Subject: [PATCH] fix(workspace): refresh source admission proofs on import resume (#20394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip preserves agent work and recovery obligations across server replacement. > - Online workspace imports keep durable operation and snapshot journals for interrupted work. > - A recovery admission fingerprint hashes complete run, finalizer, and lease rows. > - Accounting updates can change that hash while every saved recovery reference remains identical. > - Resume currently treats that expired admission observation as immutable source identity and refuses the existing journal. > - This pull request refreshes only that observation after exact identity checks and finalization fencing, preserving the original archive and journal. ## Linked Issues or Issue Description **What happened?** An interrupted import whose saved recovery sources have no changing online recovery hold can fail on its next attempt with `remote_workspace_migration_source_changed`. A change to a run's accounting fields changes `onlineProof.fingerprint`, even when all eight saved recovery records have the same typed references and ownership fields. **Expected behavior** For source-only admission drift, resume must compare every saved recovery source field exactly, except the supported ephemeral admission fingerprint. It must inspect the current source again, acquire the existing finalization fence, and retain strict full-proof checks through publication. It must keep the original operation, snapshot, archive, nested-child scope, and cohort identity. **Steps to reproduce** 1. Save a journal for an online import with eight recovery sources and no active online recovery holds. Interrupt after real authenticated restore of its uploaded snapshot. 2. Change only accounting fields on source 6's saved run. 3. Resume the same journal. The unchanged importer refuses it. The fixed importer preserves the original identities and commits after fresh guarded inspection. **Paperclip version or commit** Reproduced on master `9c634e42e449e91f3935cf59036981852f94a65e`. **Deployment mode** Online administrative imports with PostgreSQL. Related preservation work: https://github.com/paperclipai/paperclip/pull/20311 and https://github.com/paperclipai/paperclip/pull/20370. The independent readonly parallelism work in https://github.com/paperclipai/paperclip/pull/20387 changes different loops. ## What Changed - Compare the complete `legacyRecoverySources` records at resume, neutralizing only a valid fingerprint under the supported proof schema. Preserve every other field and refuse unsupported proof versions by name. - Reuse the current canonical full proofs. Persist them with an operation metadata compare-and-swap only after the existing finalization lock and strict reinspection succeed. - Keep strict full-proof checks within each attempt and before publication. Preserve all journal, snapshot, archive, nested-child, and cohort identities. - Add focused real PostgreSQL regressions in the existing online-source fixture, including exactly eight saved sources and repeat-apply idempotence. ## Verification - Genuine baseline RED: singleton and cohort resume both fail after accounting-only changes on the unchanged product. - `pnpm exec vitest run server/src/services/remote-workspace-online-source.test.ts -t 'eight saved recovery|changed saved recovery|recovery accounting changes|saved recovery proof'`: 8 passed. - Controls refuse changed valid references, a matched run/lease allocation change, competing ownership, and unsupported proof versions. They also prove that bookkeeping changes within an admitted attempt still refuse and a busy finalization fence prevents proof refresh. - Successful resumes preserve the operation/snapshot IDs, cipher SHA, manifest digest, byte size, nested scope, and cohort. Repeat apply creates no duplicate operation or snapshot. - Server layered typecheck, module boundaries, and whitespace checks passed. All applicable exact-head CI passed (including typecheck, build, all test shards and aggregate verify); hosted acceptance remains owned by the deployment workstream. Greptile re-reviewed the narrowed scope at 5/5. Ponytail scored 4/5, recommending a regression for the unchanged hold-bearing refusal; the maintainer explicitly kept that separate behavior outside this source-only fix. ## Risks This changes only source-proof resume admission. `onlineRecoveryHolds` and their action/intent fingerprints remain compared exactly, as on the baseline: a hold-bearing journal whose hold proof changes still refuses. Supporting that separate case would require preserving and validating the hold owner’s complete intent facts; this PR deliberately retains that safety check. Bookkeeping that changes during an active attempt still causes the existing strict publication checks to refuse that attempt. Older unsupported proof shapes remain refused. The metadata update uses the existing company/workspace/operation identity, running/unfinished status, and metadata compare-and-swap. No schema, provider, or storage format changes are included. ## Model Used OpenAI Codex, based on GPT-6, with tool use and code execution. The exact deployment identifier and context-window size are not exposed in 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 - [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 --- .../services/remote-workspace-migration.ts | 25 +++- .../remote-workspace-online-source.test.ts | 113 +++++++++++++++++- 2 files changed, 135 insertions(+), 3 deletions(-) diff --git a/server/src/services/remote-workspace-migration.ts b/server/src/services/remote-workspace-migration.ts index bb69c0b91f7..6819091b8ac 100644 --- a/server/src/services/remote-workspace-migration.ts +++ b/server/src/services/remote-workspace-migration.ts @@ -27,7 +27,7 @@ import { createNestedWorkspaceImportVerifier, onlineOrdinarySourceBlockers, offl import { inspectOnlineWorkspaceSource, requiresOnlineWorkspaceSource, type OnlineWorkspaceSource } from "./online-workspace-source.js"; import { withWorkspaceSourceLocks, withIndependentWorkspaceSourceLocks, IsolatedRepositorySourceBusy } from "./workspace-source-lock.js"; -import { inspectOnlineLegacyWorkspaceRecoverySource, inspectOnlineLegacyWorkspaceRecoveryHold, lockOnlineLegacyWorkspaceRecoverySources, type OnlineLegacyWorkspaceRecoveryHold } from "./online-legacy-workspace-recovery.js"; +import { inspectOnlineLegacyWorkspaceRecoverySource, inspectOnlineLegacyWorkspaceRecoveryHold, lockOnlineLegacyWorkspaceRecoverySources, ONLINE_LEGACY_WORKSPACE_PRESERVATION, type OnlineLegacyWorkspaceRecoveryHold } from "./online-legacy-workspace-recovery.js"; import { inspectNativeWorkspaceMigration, nativeWorkspaceMigrationMarker } from "./native-runtime/native-workspace-migration.js"; const SCHEMA = "paperclip.remote-workspace-migration.v1"; @@ -341,6 +341,17 @@ async function resolveMigrationSource(input: MigrationInput) { return onlineSource; } +function recoverySourcesResumeIdentity(value: unknown): unknown { + if (!Array.isArray(value)) return value; + return value.map(source => { + if (!source || typeof source !== "object" || source.onlineProof == null) return source; + const proof = object(source.onlineProof); + if (proof.schema !== ONLINE_LEGACY_WORKSPACE_PRESERVATION || typeof proof.fingerprint !== "string" + || !/^[a-f0-9]{64}$/.test(proof.fingerprint)) throw new Error("remote_workspace_migration_recovery_proof_invalid"); + return { ...source, onlineProof: { ...proof, fingerprint: null } }; + }); +} + async function reserveMigration(input: MigrationInput, onlineSource?: OnlineWorkspaceSource, cohort?: WorkspaceMigrationCohort, expectedSourceFingerprint?: string) { return input.db.transaction(async tx => { @@ -374,7 +385,8 @@ async function reserveMigration(input: MigrationInput, onlineSource?: OnlineWork if (Boolean(operation.metadata?.retainedDirectory) !== Boolean(current.retainedDirectory)) throw new Error("remote_workspace_migration_source_changed"); if (canonical(operation.metadata?.archivedGitSource ?? null) !== canonical(gitSourceProof(current.archivedGitSource))) throw new Error("remote_workspace_migration_source_changed"); if (canonical(operation.metadata?.finalizedRecoveryHolds ?? []) !== canonical(current.finalizedRecoveryHolds)) throw new Error("remote_workspace_migration_source_changed"); - if (canonical(operation.metadata?.legacyRecoverySources ?? []) !== canonical(current.legacyRecoverySources)) throw new Error("remote_workspace_migration_source_changed"); + if (canonical(recoverySourcesResumeIdentity(operation.metadata?.legacyRecoverySources ?? [])) + !== canonical(recoverySourcesResumeIdentity(current.legacyRecoverySources))) throw new Error("remote_workspace_migration_source_changed"); if (canonical(operation.metadata?.onlineRecoveryHolds ?? []) !== canonical(current.onlineRecoveryHolds)) throw new Error("remote_workspace_migration_source_changed"); return { workspace, authority: null, operation, source, sourceFingerprint, issueBinding, retainedDirectory: current.retainedDirectory, archivedGitSource: current.archivedGitSource, legacyRecoverySources: current.legacyRecoverySources, finalizedRecoveryHolds: current.finalizedRecoveryHolds, onlineRecoveryHolds: current.onlineRecoveryHolds }; } @@ -434,6 +446,15 @@ function createMigrationChildVerifier(input: MigrationInput, onlineSource: Onlin async function captureMigration(input: MigrationInput, reserved: ReservedMigration, onlineSource?: OnlineWorkspaceSource, childVerifier?: ReturnType) { let operation = reserved.operation!; + if (canonical(operation.metadata?.legacyRecoverySources ?? []) !== canonical(reserved.legacyRecoverySources)) { + const [refreshed] = await input.db.update(workspaceOperations).set({ metadata: { ...operation.metadata, + legacyRecoverySources: reserved.legacyRecoverySources }, updatedAt: new Date() }).where(and( + eq(workspaceOperations.companyId, input.companyId), eq(workspaceOperations.executionWorkspaceId, input.workspaceId), + eq(workspaceOperations.id, operation.id), eq(workspaceOperations.phase, "remote_workspace_migration"), eq(workspaceOperations.status, "running"), + sql`${workspaceOperations.finishedAt} is null`, eq(workspaceOperations.metadata, operation.metadata!))).returning(); + if (!refreshed) throw new Error("remote_workspace_migration_operation_changed"); + operation = refreshed; + } const snapshot = await beginWorkspaceSnapshot(input.db, { companyId: input.companyId, workspaceId: input.workspaceId, operationId: operation.id, generation: 1, epoch: 1 }); const scratch = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "pc-workspace-migration-"))); diff --git a/server/src/services/remote-workspace-online-source.test.ts b/server/src/services/remote-workspace-online-source.test.ts index 85527595038..c8af7667b7f 100644 --- a/server/src/services/remote-workspace-online-source.test.ts +++ b/server/src/services/remote-workspace-online-source.test.ts @@ -13,9 +13,10 @@ import { resolveManagedProjectWorkspaceDir, resolvePaperclipInstanceRoot } from import { createLocalDiskStorageProvider } from "../storage/local-disk-provider.js"; import { createStorageService } from "../storage/service.js"; import type { StorageService } from "../storage/types.js"; -import { inventoryRemoteWorkspaceMigration, migrateRemoteWorkspace } from "./remote-workspace-migration.js"; +import { inventoryRemoteWorkspaceMigration, migrateRemoteWorkspace, migrateRemoteWorkspaceCohort } from "./remote-workspace-migration.js"; import { getWorkspaceSnapshot, loadWorkspaceSnapshotArchive } from "./remote-workspace-snapshot-storage.js"; import { restoreRemoteWorkspaceArchive } from "./remote-workspace-snapshot-archive.js"; +import * as workspaceArchive from "./remote-workspace-snapshot-archive.js"; import { materializeManagedProjectWorkspace } from "./managed-repository-checkout.js"; import { withExecutionWorkspaceLifecycleLock } from "./execution-workspace-lifecycle-lock.js"; import { IsolatedRepositorySourceBusy, withIsolatedRepositorySourceLock, withWorkspaceSourceLocks } from "./workspace-source-lock.js"; @@ -528,6 +529,116 @@ describe.skipIf(!support.supported)("online ordinary physical source fence", () status: "active", evidence: { runId } }); return { ...f, run, owner, lease, warning }; } + async function interruptedRecoveryFixture(cohort = false) { + const f = await recoveryFixture(); + await db.delete(issueRecoveryActions).where(eq(issueRecoveryActions.id, f.warning.id)); + const reference = f.run.runnerProfileJson!.nativeWorkspaceSync as Record; + const extra = Array.from({ length: 7 }, () => ({ runId: randomUUID(), leaseId: randomUUID(), providerLeaseId: randomUUID() })); + await db.insert(heartbeatRuns).values(extra.map(row => ({ id: row.runId, companyId: f.companyId, agentId: f.run.agentId, + nativeIssueId: f.run.nativeIssueId, status: "failed", runtimeMode: "native", finishedAt: new Date(), + runnerProfileJson: { nativeWorkspaceSync: { ...reference, leaseId: row.leaseId, providerLeaseId: row.providerLeaseId } } }))); + await db.insert(nativeRunFinalizations).values(extra.map(row => ({ companyId: f.companyId, + issueId: f.run.nativeIssueId!, runId: row.runId, phase: "terminal_failure" }))); + await db.insert(environmentLeases).values(extra.map(row => ({ id: row.leaseId, companyId: f.companyId, + environmentId: f.lease.environmentId, executionWorkspaceId: f.workspaceId, issueId: f.run.nativeIssueId, + heartbeatRunId: row.runId, provider: "daytona", providerLeaseId: row.providerLeaseId, + status: "retained", leasePolicy: "retain_on_failure", metadata: { remoteCwd: reference.remoteCwd } }))); + const apply = () => cohort ? migrateRemoteWorkspaceCohort({ db, storage, companyId: f.companyId, + workspaceIds: [f.workspaceId], apply: true, signal: AbortSignal.timeout(30_000) }) + : migrateRemoteWorkspace({ db, storage, companyId: f.companyId, workspaceId: f.workspaceId, apply: true }); + const restore = workspaceArchive.restoreRemoteWorkspaceArchive; + const interrupt = vi.spyOn(workspaceArchive, "restoreRemoteWorkspaceArchive").mockImplementationOnce(async input => { + await restore(input); + expect(await db.select().from(executionWorkspaceSnapshots)).toEqual([expect.objectContaining({ status: "uploaded" })]); + throw new Error("interrupted after verified upload"); + }); + try { await expect(apply()).rejects.toThrow("interrupted after verified upload"); } + finally { interrupt.mockRestore(); } + const [operation] = await db.select().from(workspaceOperations), [snapshot] = await db.select().from(executionWorkspaceSnapshots); + const sources = operation.metadata!.legacyRecoverySources as Array<{ runId: string; issueId: string; + reference: Record; onlineProof: { schema: string; fingerprint: string } }>; + expect(sources).toHaveLength(8); + expect(operation.status).toBe("running"); expect(snapshot.status).toBe("uploaded"); + return { ...f, apply, operation, snapshot, sources, changed: sources[6] }; + } + it.each([false, true])("resumes eight saved recovery sources after accounting changes (cohort=%s)", async cohort => { + const f = await interruptedRecoveryFixture(cohort); + await db.update(heartbeatRuns).set({ accountingAttemptCount: 1, accountingLastAttemptAt: new Date(), + accountingLastError: "retry scheduled", updatedAt: new Date() }).where(eq(heartbeatRuns.id, f.changed.runId)); + await expect(f.apply()).resolves.toMatchObject({ applied: true }); + const operations = await db.select().from(workspaceOperations), snapshots = await db.select().from(executionWorkspaceSnapshots); + expect(operations).toEqual([expect.objectContaining({ id: f.operation.id, status: "succeeded" })]); + expect(snapshots).toEqual([expect.objectContaining({ id: f.snapshot.id, sourceOperationId: f.operation.id, status: "committed", + objectKey: f.snapshot.objectKey, sha256: f.snapshot.sha256, manifestDigest: f.snapshot.manifestDigest, byteSize: f.snapshot.byteSize })]); + expect(operations[0].metadata!.nestedWorkspaceScope).toEqual(f.operation.metadata!.nestedWorkspaceScope); + expect(operations[0].metadata!.cohortMigration).toEqual(f.operation.metadata!.cohortMigration); + const sources = operations[0].metadata!.legacyRecoverySources as typeof f.sources; + expect(sources.map(source => ({ ...source, onlineProof: { ...source.onlineProof, fingerprint: null } }))) + .toEqual(f.sources.map(source => ({ ...source, onlineProof: { ...source.onlineProof, fingerprint: null } }))); + expect(sources[6].onlineProof.fingerprint).not.toBe(f.changed.onlineProof.fingerprint); + expect(sources.filter((_, index) => index !== 6)).toEqual(f.sources.filter((_, index) => index !== 6)); + expect(readRemoteWorkspaceAuthority((await stored(f.workspaceId)).metadata)?.snapshotId).toBe(f.snapshot.id); + expect(await fs.readFile(path.join(f.cwd, "saved.txt"), "utf8")).toBe("saved legacy edits\n"); + await expect(f.apply()).resolves.toMatchObject({ applied: true, replayed: true }); + expect(await db.select().from(workspaceOperations)).toHaveLength(1); + expect(await db.select().from(executionWorkspaceSnapshots)).toHaveLength(1); + }); + it.each(["reference", "allocation", "owner", "proof-version"] as const)("refuses a changed saved recovery %s on resume", async change => { + const f = await interruptedRecoveryFixture(); + const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.changed.runId)); + if (change === "reference" || change === "allocation") { + const reference: Record = { ...f.changed.reference, ...(change === "reference" ? { baselineSha256: "56".repeat(32) } : { providerLeaseId: randomUUID() }) }; + await db.update(heartbeatRuns).set({ runnerProfileJson: { ...run.runnerProfileJson, nativeWorkspaceSync: reference } }) + .where(eq(heartbeatRuns.id, run.id)); + if (change === "allocation") await db.update(environmentLeases).set({ providerLeaseId: reference.providerLeaseId as string }) + .where(eq(environmentLeases.id, reference.leaseId as string)); + expect(await inspectOnlineLegacyWorkspaceRecoverySource(db, { workspace: await stored(f.workspaceId), runId: run.id })).not.toBeNull(); + } + if (change === "owner") await db.update(nativeRunFinalizations).set({ leaseOwner: "competing-finalizer", + leaseExpiresAt: new Date(Date.now() + 60_000) }).where(eq(nativeRunFinalizations.runId, run.id)); + if (change === "proof-version") { + const sources = f.sources.map((source, index) => index === 6 ? { ...source, onlineProof: { ...source.onlineProof, schema: "unsupported.v2" } } : source); + await db.update(workspaceOperations).set({ metadata: { ...f.operation.metadata, legacyRecoverySources: sources } }) + .where(eq(workspaceOperations.id, f.operation.id)); + } + const [before] = await db.select().from(workspaceOperations); + await expect(f.apply()).rejects.toThrow(change === "proof-version" ? "remote_workspace_migration_recovery_proof_invalid" + : change === "owner" ? "remote_copyback_unresolved" : "remote_workspace_migration_source_changed"); + expect(await db.select().from(workspaceOperations)).toEqual([before]); + expect(await db.select().from(executionWorkspaceSnapshots)).toEqual([f.snapshot]); + expect(readRemoteWorkspaceAuthority((await stored(f.workspaceId)).metadata)).toBeNull(); + expect(await fs.readFile(path.join(f.cwd, "saved.txt"), "utf8")).toBe("saved legacy edits\n"); + }); + it("still refuses recovery accounting changes within one admitted attempt", async () => { + const f = await recoveryFixture(), put = storage.putFile.bind(storage); + await db.delete(issueRecoveryActions).where(eq(issueRecoveryActions.id, f.warning.id)); + vi.spyOn(storage, "putFile").mockImplementationOnce(async input => { + const result = await put(input); + await db.update(heartbeatRuns).set({ accountingAttemptCount: 1 }).where(eq(heartbeatRuns.id, f.run.id)); + return result; + }); + await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true })).rejects.toThrow("remote_workspace_migration_source_changed"); + expect(readRemoteWorkspaceAuthority((await stored(f.workspaceId)).metadata)).toBeNull(); + expect((await db.select().from(workspaceOperations))[0].status).toBe("running"); + }); + it("does not refresh a saved recovery proof before its finalization fence is acquired", async () => { + const f = await interruptedRecoveryFixture(), other = createDb(database.connectionString); + await db.update(heartbeatRuns).set({ accountingAttemptCount: 1 }).where(eq(heartbeatRuns.id, f.changed.runId)); + let entered!: () => void, release!: () => void; + const ready = new Promise(resolve => { entered = resolve; }); + const pending = new Promise(resolve => { release = resolve; }); + const held = other.transaction(async tx => { + expect(await tryLockNativeWorkspaceFinalization(tx as unknown as Db, { companyId: f.companyId, runId: f.changed.runId })).toBe(true); + entered(); await pending; + }); + await ready; + try { + await expect(f.apply()).rejects.toThrow("remote_copyback_owned"); + expect(await db.select().from(workspaceOperations)).toEqual([f.operation]); + expect(await db.select().from(executionWorkspaceSnapshots)).toEqual([f.snapshot]); + expect(readRemoteWorkspaceAuthority((await stored(f.workspaceId)).metadata)).toBeNull(); + } finally { release(); await held; await other.$client.end({ timeout: 1 }); } + }); async function artifactFailureFixture(descendant = false, cause: "runner_remote_provider_artifact_incompatible" | "native_session_interrupted" = "runner_remote_provider_artifact_incompatible") { const f = await recoveryFixture(descendant), statusVersion = 2; const profile = { ...f.run.runnerProfileJson, sessionCheckpoint: cause === "native_session_interrupted"