From 16a646ba7a658aa84807d6ca496b9493bbd3d3e4 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 4 Nov 2026 21:56:13 -0600 Subject: [PATCH] fix(workspaces): preserve detached work and journaled Git sources Reject retained worktree paths and orphan indexes before branch-only import. Revalidate the source path from interrupted migration journals. Use the cleanup producer command formatter for clean user-owned removal receipts. Co-Authored-By: Paperclip --- .../remote-workspace-migration-git.ts | 51 +++++++++++++++++-- .../remote-workspace-migration-tree.ts | 2 +- .../remote-workspace-migration.test.ts | 44 ++++++++++++---- .../services/remote-workspace-migration.ts | 2 +- .../src/services/workspace-command-display.ts | 5 ++ server/src/services/workspace-runtime.ts | 7 +-- 6 files changed, 88 insertions(+), 23 deletions(-) create mode 100644 server/src/services/workspace-command-display.ts diff --git a/server/src/services/remote-workspace-migration-git.ts b/server/src/services/remote-workspace-migration-git.ts index 5ee238a5526..4ab4bbbd99c 100644 --- a/server/src/services/remote-workspace-migration-git.ts +++ b/server/src/services/remote-workspace-migration-git.ts @@ -15,6 +15,8 @@ import { ensureSavedGitCommit, gitRestorePointExpired, managedSourcePaths, readG import { localRepositoryPath } from "./local-repository-path.js"; import { materializeManagedProjectWorkspace } from "./managed-repository-checkout.js"; import { isRuntimeOwnedGitBranch } from "./execution-workspace-branch-ownership.js"; +import { physicalPath } from "./remote-workspace-migration-tree.js"; +import { formatCommandForDisplay } from "./workspace-command-display.js"; import { recordedIsolatedRemoteSource } from "./isolated-remote-workspace-source.js"; type Workspace = typeof executionWorkspaces.$inferSelect; @@ -39,10 +41,42 @@ async function git(cwd: string, args: string[]) { })).stdout.trim(); } +async function assertNoRetainedWorktree(sourcePath: string, expected: string, branch: string) { + const target = await physicalPath(expected); + const common = await git(sourcePath, ["rev-parse", "--path-format=absolute", "--git-common-dir"]); + const admin = path.join(common, "worktrees"); + const entries = await fs.readdir(admin, { withFileTypes: true }).catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return []; + throw error; + }); + for (const entry of entries) { + if (entry.isSymbolicLink()) throw unavailable(); + if (!entry.isDirectory()) continue; + const root = path.join(admin, entry.name); + const pointer = await fs.readFile(path.join(root, "gitdir"), "utf8").catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return null; + throw error; + }); + if (pointer && await physicalPath(pointer.trim()) === path.join(target, ".git")) throw unavailable(); + // An index without its registration can still own staged work. It has no + // reliable checkout identity, so a branch-only import cannot discard it. + if (!pointer && await fs.lstat(path.join(root, "index")).catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT") return null; + throw error; + })) throw unavailable(); + } + const records = (await git(sourcePath, ["worktree", "list", "--porcelain", "-z"])).split("\0\0"); + for (const record of records) { + const fields = record.split("\0"); + const worktree = fields.find(field => field.startsWith("worktree "))?.slice(9); + if (fields.includes(`branch refs/heads/${branch}`) || worktree && await physicalPath(worktree) === target) throw unavailable(); + } +} + /** A removed branch must be reconstructed from its saved commit or its exact * successful deletion receipt. Today's default branch is never a substitute. */ export async function inspectArchivedGitMigrationSource(db: Pick, workspace: Workspace, - options: { allowActiveRetainedBranch?: boolean } = {}): Promise { + options: { allowActiveRetainedBranch?: boolean; operationId?: string } = {}): Promise { const archived = workspace.status === "archived"; if ((!archived && !options.allowActiveRetainedBranch) || !workspace.repoUrl || !workspace.branchName || !workspace.sourceIssueId) return null; const recipe = readGitCacheRestoration(workspace); @@ -60,7 +94,14 @@ export async function inspectArchivedGitMigrationSource(db: Pick, if (error.code === "ENOENT") return null; throw error; }) : null; - const sourcePath = privatePresent && privateSource ? privateSource : projectWorkspace?.cwd ?? managedSourcePaths(workspace)[0]; + const [journal] = options.operationId ? await db.select({ metadata: workspaceOperations.metadata }).from(workspaceOperations).where(and( + eq(workspaceOperations.companyId, workspace.companyId), eq(workspaceOperations.executionWorkspaceId, workspace.id), + eq(workspaceOperations.id, options.operationId), eq(workspaceOperations.phase, "remote_workspace_migration"), + )) : []; + const saved = journal?.metadata?.archivedGitSource; + const savedPath = saved && typeof saved === "object" && !Array.isArray(saved) + && typeof (saved as { sourcePath?: unknown }).sourcePath === "string" ? (saved as { sourcePath: string }).sourcePath : null; + const sourcePath = savedPath ?? (privatePresent && privateSource ? privateSource : projectWorkspace?.cwd ?? managedSourcePaths(workspace)[0]); if (!sourcePath || !privatePresent && projectWorkspace?.repoUrl && projectWorkspace.repoUrl !== workspace.repoUrl) throw unavailable(); const [operation] = archived && workspace.closedAt ? await db.select().from(workspaceOperations).where(and( eq(workspaceOperations.companyId, workspace.companyId), eq(workspaceOperations.executionWorkspaceId, workspace.id), @@ -89,7 +130,7 @@ export async function inspectArchivedGitMigrationSource(db: Pick, const [removal] = await db.select({ id: workspaceOperations.id }).from(workspaceOperations).where(and( eq(workspaceOperations.companyId, workspace.companyId), eq(workspaceOperations.executionWorkspaceId, workspace.id), eq(workspaceOperations.phase, "worktree_cleanup"), eq(workspaceOperations.status, "succeeded"), - eq(workspaceOperations.command, `git worktree remove ${expected}`), isNotNull(workspaceOperations.finishedAt), + eq(workspaceOperations.command, formatCommandForDisplay("git", ["worktree", "remove", expected])), isNotNull(workspaceOperations.finishedAt), gte(workspaceOperations.startedAt, workspace.closedAt), sql`${workspaceOperations.metadata}->>'cleanupAction' = 'worktree_remove'`, sql`${workspaceOperations.metadata}->>'workspaceId' = ${workspace.id}`, @@ -105,8 +146,8 @@ export async function inspectArchivedGitMigrationSource(db: Pick, eq(executionWorkspaces.repoUrl, workspace.repoUrl), eq(executionWorkspaces.branchName, workspace.branchName), ne(executionWorkspaces.id, workspace.id), )).limit(1); - if (other || (await git(sourcePath, ["worktree", "list", "--porcelain", "-z"])) - .split("\0").includes(`branch refs/heads/${workspace.branchName}`)) throw unavailable(); + if (other) throw unavailable(); + await assertNoRetainedWorktree(sourcePath, expected, workspace.branchName); const commit = await git(sourcePath, ["rev-parse", "--verify", `refs/heads/${workspace.branchName}^{commit}`]); if (!/^[a-f0-9]{40}$/.test(commit)) throw unavailable(); return { commit, sourcePath, cleanupOperationId: removalId, recipe: null, retainedBranch: workspace.branchName }; diff --git a/server/src/services/remote-workspace-migration-tree.ts b/server/src/services/remote-workspace-migration-tree.ts index 4685a231f62..c634e41b9fb 100644 --- a/server/src/services/remote-workspace-migration-tree.ts +++ b/server/src/services/remote-workspace-migration-tree.ts @@ -18,7 +18,7 @@ const rootOf = (workspace: Pick) => workspace. // Missing historical paths still resolve through their nearest surviving // parent, so a symlinked ancestor cannot hide an alias from the writer fence. -async function physicalPath(value: string): Promise { +export async function physicalPath(value: string): Promise { let current = path.resolve(value); const missing: string[] = []; for (;;) { try { return path.join(await fs.realpath(current), ...missing.reverse()); } diff --git a/server/src/services/remote-workspace-migration.test.ts b/server/src/services/remote-workspace-migration.test.ts index d0676a3f5ca..d4c5be8d31f 100644 --- a/server/src/services/remote-workspace-migration.test.ts +++ b/server/src/services/remote-workspace-migration.test.ts @@ -452,9 +452,9 @@ describe.skipIf(!support.supported)("remote workspace migration", () => { await expect(migrateRemoteWorkspace({ db, storage, ...parent, apply: true, writersStopped: true })).rejects.toThrow("nested_workspace_source_changed"); expect(readRemoteWorkspaceAuthority((await workspace(parent.workspaceId)).metadata)).toBeNull(); }); - async function gitArchiveFixture(repoUrl = "https://example.invalid/repository.git") { + async function gitArchiveFixture(repoUrl = "https://example.invalid/repository.git", home = root) { const f = await seed(), projectId = randomUUID(), projectWorkspaceId = randomUUID(), branchName = "saved-task"; - vi.stubEnv("PAPERCLIP_HOME", root); vi.stubEnv("PAPERCLIP_INSTANCE_ID", "default"); + vi.stubEnv("PAPERCLIP_HOME", home); vi.stubEnv("PAPERCLIP_INSTANCE_ID", "default"); const base = resolveManagedProjectWorkspaceDir({ companyId: f.companyId, projectId, repoName: "repository" }); await fs.mkdir(base, { recursive: true }); await git(base, "init", "-q", "-b", "main"); await fs.writeFile(path.join(base, "base.txt"), "base\n"); await git(base, "add", "."); await git(base, "commit", "-qm", "base"); @@ -504,7 +504,7 @@ describe.skipIf(!support.supported)("remote workspace migration", () => { expect(await fs.readFile(path.join(targetDir, "base.txt"), "utf8")).toBe("base\n"); expect(await fs.readFile(path.join(targetDir, "saved.txt"), "utf8")).toBe("original saved work\n"); }); - it.each(["valid", "foreign-cache", "wrong-origin", "missing-receipt", "revoked-receipt", "private-cache"])("recovers deleted isolated history only from an authorized receipt and project object cache (%s)", async state => { + it.each(["valid", "foreign-cache", "wrong-origin", "missing-receipt", "revoked-receipt", "private-cache", "journaled-shared", "journaled-source-changed"])("recovers deleted isolated history only from an authorized receipt and project object cache (%s)", async state => { const f = await gitArchiveFixture(), privateBase = isolatedRemoteSourcePath(f.companyId, f.issueId); const privateCwd = path.join(privateBase, ".paperclip", "worktrees", f.branchName); await fs.mkdir(path.dirname(privateBase), { recursive: true }); @@ -516,7 +516,7 @@ describe.skipIf(!support.supported)("remote workspace migration", () => { const receipt = await git(privateBase, "branch", "-d", f.branchName); await git(f.base, "merge", "--ff-only", f.branchName); await git(f.base, "worktree", "remove", f.cwd); await git(f.base, "branch", "-d", f.branchName); - if (state !== "private-cache") await fs.rm(privateBase, { recursive: true }); + if (!["private-cache", "journaled-shared", "journaled-source-changed"].includes(state)) await fs.rm(privateBase, { recursive: true }); const closedAt = new Date(); await db.update(executionWorkspaces).set({ cwd: privateCwd, providerRef: privateCwd, status: "archived", closedAt, metadata: { createdByRuntime: true, gitBranchOwnershipVersion: 1, isolatedRemoteSource: { @@ -548,7 +548,16 @@ describe.skipIf(!support.supported)("remote workspace migration", () => { return result; }; } - if (state !== "valid" && state !== "private-cache") { + if (state.startsWith("journaled-")) { + const put = storage.putFile.bind(storage); let fail = true; + storage.putFile = async input => { const saved = await put(input); if (fail) { fail = false; throw new Error("upload reply lost"); } return saved; }; + await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true })).rejects.toThrow("upload reply lost"); + const [operation] = await db.select().from(workspaceOperations).where(and(eq(workspaceOperations.executionWorkspaceId, f.workspaceId), eq(workspaceOperations.phase, "remote_workspace_migration"))); + await db.update(workspaceOperations).set({ metadata: { ...operation!.metadata, + archivedGitSource: { ...(operation!.metadata!.archivedGitSource as object), sourcePath: f.base } } }).where(eq(workspaceOperations.id, operation!.id)); + if (state === "journaled-source-changed") await git(f.base, "remote", "set-url", "origin", "https://example.invalid/foreign.git"); + } + if (!["valid", "private-cache", "journaled-shared"].includes(state)) { await expect(migrateRemoteWorkspace({ db, storage, companyId: f.companyId, workspaceId: f.workspaceId, apply: true })).rejects.toThrow(); expect(readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)).toBeNull(); return; } @@ -566,14 +575,29 @@ describe.skipIf(!support.supported)("remote workspace migration", () => { expect(await fs.readFile(path.join(targetDir, "base.txt"), "utf8")).toBe("base\n"); expect(await fs.stat(path.join(targetDir, "unrelated.txt")).catch(() => null)).toBeNull(); }); - it.each(["valid", "changed-branch", "competing-owner", "registered-branch", "unowned", "missing-branch", "active-online", "active-offline", "user-owned-receipt", "user-owned-force-receipt", "user-owned-revoked-receipt"])("preserves a missing checkout from its retained owned branch (%s)", async mode => { - const f = await gitArchiveFixture(), closedAt = new Date(); + it.each(["valid", "changed-branch", "competing-owner", "registered-branch", "unowned", "missing-branch", "active-online", "active-offline", "user-owned-receipt", "user-owned-force-receipt", "user-owned-revoked-receipt", "user-owned-quoted-receipt", "detached-registration", "orphan-index"])("preserves a missing checkout from its retained owned branch (%s)", async mode => { + const f = await gitArchiveFixture(undefined, mode === "user-owned-quoted-receipt" ? path.join(root, "home with spaces") : root), closedAt = new Date(); const active = mode.startsWith("active-"); await db.update(executionWorkspaces).set({ status: active ? "active" : "archived", closedAt: active ? null : closedAt }) .where(eq(executionWorkspaces.id, f.workspaceId)); - await git(f.base, "worktree", "remove", f.cwd); + if (mode === "user-owned-quoted-receipt") { + await db.update(executionWorkspaces).set({ metadata: { createdByRuntime: false, gitBranchOwnershipVersion: 1 } }).where(eq(executionWorkspaces.id, f.workspaceId)); + const cleaned = await cleanupExecutionWorkspaceArtifacts({ workspace: await workspace(f.workspaceId), + projectWorkspace: { cwd: f.base, cleanupCommand: null }, runCleanupCommands: false, forceWorktreeRemoval: false, + recorder: workspaceOperationService(db).createRecorder({ companyId: f.companyId, executionWorkspaceId: f.workspaceId, issueId: f.issueId }) }); + expect(cleaned.warnings).toEqual([]); + } else if (mode === "detached-registration") { + await git(f.cwd, "checkout", "--detach"); + await fs.writeFile(path.join(f.cwd, "newer.txt"), "recoverable detached work"); + await git(f.cwd, "add", "."); await git(f.cwd, "commit", "-qm", "newer detached work"); + await fs.rm(f.cwd, { recursive: true }); + } else await git(f.base, "worktree", "remove", f.cwd); await fs.writeFile(path.join(f.base, "base.txt"), "new shared default branch\n"); await git(f.base, "add", "."); await git(f.base, "commit", "-qm", "unrelated shared work"); + if (mode === "orphan-index") { + const admin = path.join(f.base, ".git", "worktrees", "orphan"); + await fs.mkdir(admin, { recursive: true }); await fs.copyFile(path.join(f.base, ".git", "index"), path.join(admin, "index")); + } if (mode === "changed-branch") { const put = storage.putFile.bind(storage); storage.putFile = async input => { const saved = await put(input); await git(f.base, "update-ref", `refs/heads/${f.branchName}`, await git(f.base, "rev-parse", "HEAD")); return saved; }; @@ -585,7 +609,7 @@ describe.skipIf(!support.supported)("remote workspace migration", () => { repoUrl: f.repoUrl, branchName: f.branchName, cwd: path.join(root, "other-owner") }); } if (mode === "registered-branch") await git(f.base, "worktree", "add", path.join(root, "registered"), f.branchName); - if (mode.startsWith("user-owned-")) { + if (mode.startsWith("user-owned-") && mode !== "user-owned-quoted-receipt") { await db.update(executionWorkspaces).set({ metadata: { createdByRuntime: false, gitBranchOwnershipVersion: 1 } }).where(eq(executionWorkspaces.id, f.workspaceId)); const [receipt] = await db.insert(workspaceOperations).values({ companyId: f.companyId, executionWorkspaceId: f.workspaceId, phase: "worktree_cleanup", status: "succeeded", startedAt: new Date(), finishedAt: new Date(), @@ -602,7 +626,7 @@ describe.skipIf(!support.supported)("remote workspace migration", () => { const offline = mode === "active-offline"; if (offline) vi.stubEnv("PAPERCLIP_EPHEMERAL_MIGRATION", "true"); const migrate = () => migrateRemoteWorkspace({ db, storage, ...f, apply: true, writersStopped: offline }); - if (!["valid", "active-offline", "user-owned-receipt"].includes(mode)) { + if (!["valid", "active-offline", "user-owned-receipt", "user-owned-quoted-receipt"].includes(mode)) { await expect(migrate()).rejects.toThrow(); expect(readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)).toBeNull(); return; } diff --git a/server/src/services/remote-workspace-migration.ts b/server/src/services/remote-workspace-migration.ts index 1c3837bd5b5..afa8455e195 100644 --- a/server/src/services/remote-workspace-migration.ts +++ b/server/src/services/remote-workspace-migration.ts @@ -87,7 +87,7 @@ async function inspect(db: ReadDb, input: { companyId: string; workspaceId: stri } } if (!entry && !retainedDirectory && (workspace.status === "archived" || input.writersStopped)) { - try { archivedGitSource = await inspectArchivedGitMigrationSource(db, workspace, { allowActiveRetainedBranch: input.writersStopped }); } + try { archivedGitSource = await inspectArchivedGitMigrationSource(db, workspace, { allowActiveRetainedBranch: input.writersStopped, operationId: allowedOperation }); } catch (error) { reasons.push(error instanceof Error && error.message === "remote_workspace_migration_restore_point_expired" ? "restore_point_expired" : "archived_git_source_unavailable"); } } diff --git a/server/src/services/workspace-command-display.ts b/server/src/services/workspace-command-display.ts new file mode 100644 index 00000000000..a609774744b --- /dev/null +++ b/server/src/services/workspace-command-display.ts @@ -0,0 +1,5 @@ +export function formatCommandForDisplay(command: string, args: string[]) { + return [command, ...args] + .map((part) => (/^[A-Za-z0-9_./:-]+$/.test(part) ? part : JSON.stringify(part))) + .join(" "); +} diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index 8a6211968a6..b9bdfcfdda3 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -1,3 +1,4 @@ +import { formatCommandForDisplay } from "./workspace-command-display.js"; import { notifyStatusCardIssueChange } from "./status-card-notifications.js"; import { insertIssueComment } from "./issue-comment-writes.js"; import { restoreGitWorkspaceContents } from "./git-workspace-contents.js"; @@ -834,12 +835,6 @@ function resolveConfiguredPath(value: string, baseDir: string): string { return path.resolve(baseDir, value); } -function formatCommandForDisplay(command: string, args: string[]) { - return [command, ...args] - .map((part) => (/^[A-Za-z0-9_./:-]+$/.test(part) ? part : JSON.stringify(part))) - .join(" "); -} - function trimToLastBytes(value: string, limit: number) { const byteLength = Buffer.byteLength(value, "utf8"); if (byteLength <= limit) return value;