From 9f00f2b83eccb91fba029a83a27f4f293c3dcea9 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 21:11:54 -0600 Subject: [PATCH] fix(workspaces): recognize committed imports in linked source scope (#20444) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Online workspace migration fences every local Git source that a capture can reach. > - A linked worktree can remain on disk after its workspace is migrated to cloud storage. > - The source inspector lists local workspace rows, so it then treats that linked worktree as unregistered. > - This pull request uses the existing canonical import journal and snapshot proof to recognize its exact original physical root. > - The benefit is that a later linked workspace can migrate without weakening source ownership or stopping agents. ## Linked Issues or Issue Description **What happened?** After one linked Git workspace commits its canonical migration, its local files and Git registration can remain. The workspace row now uses `cloud_sandbox`. A later migration of a related local workspace fails with `online_workspace_source_scope_unproven` because the original linked root is no longer in the local-row list. **Expected behavior** The inspector can recognize the exact original physical root from the same-company successful migration journal and its committed or superseded snapshot. Unknown or changed provenance must still fail. **Steps to reproduce** 1. Create two local Git workspaces with a shared worktree registration. 2. Complete canonical migration for one workspace while retaining its original local files. 3. Inspect or migrate the other workspace. Before this fix, the inspector returns no source scope. **Paperclip version or commit** Reproduced on `f73ff4ace9` with real PostgreSQL, Git, filesystem, and canonical snapshot storage. **Deployment mode** Self-hosted server. This is a core workspace migration bug. **Relevant logs or output** `remote_workspace_migration_blocked:online_workspace_source_scope_unproven` Refs #20418, #20436, #20441. This fix changes source registration only. ## What Changed - Let the existing imported-source owner match optional exact linked physical roots in addition to its existing nested-child paths. - Require each added linked root to have a finished successful same-company journal, an exact committed or superseded snapshot binding, and the same manifest digest. - Include those proven physical roots in the online scope and its source fingerprint checks. - Retain Git root, backlink, pointer, ownership, company, foreign writer, source lock, and publication reinspection guards. - Keep existing nested-child behavior and all persisted schemas unchanged. Previous-release canonical imports remain readable. No runtime authority comes from an arbitrary provider path or remote authority. ## Verification - Genuine baseline RED: both committed and superseded imported sibling cases return a null source scope after a real successful import. - GREEN: 17 focused real-PostgreSQL/FS/Git tests pass. They cover both imported states, the original issue source fence, changed journal/snapshot/manifest/unfinished provenance, a foreign local alias, unregistered links, backlink/pointer corruption, and a listing change during upload. - Command: `pnpm exec vitest run server/src/services/remote-workspace-migration.test.ts -t 'imported linked Git source|foreign local writer|unproven linked|revalidates the linked Git listing|nested Git wrapper.*shared|nested Git wrapper blocked'`. - Server layered typecheck, module boundaries, and `git diff --check` pass. - Actual controlled staging canary: the canonical API committed both targeted workspace snapshots in 310.249 seconds. The owned job closed with exit 0 and all owned processes joined. A separate read-only inventory confirmed that both workspaces left the legacy provider set. This used a reviewed private administrative artifact; the serving app image and normal release path stayed unchanged. - Final head `1759ac6f40a1b35f79587b246de8b36e7a371537`: 56 successful checks, 4 intentional skips, and 1 neutral status. Greptile and Ponytail both report 5/5 with no findings or unresolved threads. - Initial CI had unchanged Runner-support and workspace-recovery timeouts, plus an unrelated heartbeat plugin-environment timing/fixture failure. The same owning inputs passed on the sibling PR. Each failed job then passed one targeted retry on unchanged product bytes. The original failures remain recorded in the CI history. ## Risks - This recognizes a retained original root only from the existing committed import ledger. It does not treat arbitrary cloud workspace paths as local ownership. - Each physical root still passes the existing Git identity and foreign-scope checks. Publication re-inspects the source scope under the existing source locks. - This adds no object reads, provider calls, recurring work, capture algorithm changes, or data exclusion. The existing import query keeps its current 10,240-row limit and runs during explicit migration inspection only. ## Model Used OpenAI GPT-6.1 Codex, with reasoning, repository tools, and code execution. The host does not expose an exact context-window size. ## 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 --- .../src/services/online-workspace-source.ts | 8 +-- .../remote-workspace-migration-tree.ts | 10 ++-- .../remote-workspace-migration.test.ts | 51 +++++++++++++++++++ 3 files changed, 62 insertions(+), 7 deletions(-) diff --git a/server/src/services/online-workspace-source.ts b/server/src/services/online-workspace-source.ts index 2c8d4a64cdb..3ba9b3b8576 100644 --- a/server/src/services/online-workspace-source.ts +++ b/server/src/services/online-workspace-source.ts @@ -140,7 +140,8 @@ export async function inspectOnlineWorkspaceSource(db: ReadDb, workspace: Worksp const projectIds = new Set(selected.projectIds), physicalIssueIds = new Set(selected.issueIds), aliases = new Set(); const importedChildren: NonNullable = []; try { - for (const child of await inspectImportedWorkspaceSourcePaths(db, workspace)) { + for (const child of await inspectImportedWorkspaceSourcePaths(db, workspace, + [...linkedSources.values()].flatMap(proof => proof.links.map(link => link.root.path)))) { const identities = await workspaceSourceIdentities(workspace.companyId, child.sourcePath); if (identities.unownedPaths.length) return null; for (const id of identities.projectIds) projectIds.add(id); @@ -168,7 +169,8 @@ export async function inspectOnlineWorkspaceSource(db: ReadDb, workspace: Worksp } if (!aliases.has(workspace.id) || aliases.size > 256) return null; const included = nodes.filter(node => aliases.has(node.row.id)); - const registeredRoots = await Promise.all(included.flatMap(node => localWorkspaceSourcePaths(node.row)).map(physicalPath)); + const registeredRoots = await Promise.all([...included.flatMap(node => localWorkspaceSourcePaths(node.row)), + ...importedChildren.map(child => child.sourcePath)].map(physicalPath)); if ([...linkedSources.values()].some(proof => proof.links.some(link => !registeredRoots.some(root => link.root.path === root || link.root.path.startsWith(root + path.sep))))) return null; const issueIds = [...new Set([...physicalIssueIds, ...included.map(node => node.row.sourceIssueId!)])].sort(); @@ -224,7 +226,7 @@ export async function inspectOnlineWorkspaceSource(db: ReadDb, workspace: Worksp } return { kind: "git", root: captureRoot, projectId: null, projectIds: [...projectIds].sort(), issueIds, workspaceIds: [...aliases].sort(), device: entry.dev, inode: entry.ino, gitSourceRoots, companions, - physicalRoots: included.flatMap(node => localWorkspaceSourcePaths(node.row)).sort(), + physicalRoots: [...new Set([...included.flatMap(node => localWorkspaceSourcePaths(node.row)), ...importedChildren.map(child => child.sourcePath)])].sort(), settledPreparedWorkspaceIds: settledPreparedWorkspaceIds.sort(), settledRecoveryIssueIds: [...settledRecoveryIssueIds].sort(), settledFinalizationRunIds: [...new Set(companions.flatMap(proof => proof.sources.map(source => source.runId)))].sort(), gitSourceFingerprints: included.map(node => ({ workspaceId: node.row.id, fingerprint: node.fingerprint })), importedChildren, diff --git a/server/src/services/remote-workspace-migration-tree.ts b/server/src/services/remote-workspace-migration-tree.ts index d7e3880c22a..03e2f4fa19b 100644 --- a/server/src/services/remote-workspace-migration-tree.ts +++ b/server/src/services/remote-workspace-migration-tree.ts @@ -269,7 +269,7 @@ export async function workspaceMigrationLayoutBlockers(db: ReadDb, workspace: Wo return []; } -async function importedWorkspaceCandidates(db: ReadDb, workspace: Workspace) { +async function importedWorkspaceCandidates(db: ReadDb, workspace: Workspace, linkedRoots: readonly string[] = []) { const sourceRoot = rootOf(workspace); if (!sourceRoot) return []; const root = await physicalPath(sourceRoot); @@ -289,12 +289,14 @@ async function importedWorkspaceCandidates(db: ReadDb, workspace: Workspace) { if (!row.operation.cwd) continue; located.push({ ...row, sourcePath: await physicalPath(row.operation.cwd) }); } - return located.filter(row => row.sourcePath.startsWith(`${root}/.paperclip/worktrees/`)) + const linked = new Set(linkedRoots); + return located.filter(row => row.sourcePath.startsWith(`${root}/.paperclip/worktrees/`) || linked.has(row.sourcePath) + && row.operation.finishedAt !== null && row.operation.metadata?.manifestDigest === row.snapshot.manifestDigest) .sort((a, b) => a.sourcePath.length - b.sourcePath.length || a.operation.id.localeCompare(b.operation.id)); } -export async function inspectImportedWorkspaceSourcePaths(db: ReadDb, workspace: Workspace) { - return (await importedWorkspaceCandidates(db, workspace)).map(({ operation, snapshot, sourcePath }) => ({ +export async function inspectImportedWorkspaceSourcePaths(db: ReadDb, workspace: Workspace, linkedRoots: readonly string[] = []) { + return (await importedWorkspaceCandidates(db, workspace, linkedRoots)).map(({ operation, snapshot, sourcePath }) => ({ workspaceId: snapshot.executionWorkspaceId, operationId: operation.id, snapshotId: snapshot.id, sourcePath, manifestDigest: snapshot.manifestDigest, operationManifestDigest: operation.metadata?.manifestDigest, })); diff --git a/server/src/services/remote-workspace-migration.test.ts b/server/src/services/remote-workspace-migration.test.ts index 5f5f643027d..f25df01355c 100644 --- a/server/src/services/remote-workspace-migration.test.ts +++ b/server/src/services/remote-workspace-migration.test.ts @@ -1671,6 +1671,57 @@ describe.skipIf(!support.supported)("remote workspace migration", () => { expect(readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)).toBeNull(); if (!foreign) await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true })).resolves.toMatchObject({ applied: true }); }); + async function importedLinkedFixture() { + const { f, nested } = await nestedGitWrapperFixture(), issueId = randomUUID(), workspaceId = randomUUID(); + await db.insert(issues).values({ id: issueId, companyId: f.companyId, title: "Imported linked workspace" }); + const cwd = path.join(resolvePaperclipInstanceRoot(), "isolated-workspaces", f.companyId, issueId); + await git(nested, "worktree", "add", "-b", "imported-linked", cwd); + await db.insert(executionWorkspaces).values({ id: workspaceId, companyId: f.companyId, sourceIssueId: issueId, + name: "Imported linked workspace", mode: "isolated_workspace", strategyType: "git_worktree", providerType: "git_worktree", cwd, providerRef: cwd }); + await db.update(issues).set({ executionWorkspaceId: workspaceId }).where(eq(issues.id, issueId)); + await migrateRemoteWorkspace({ db, storage, companyId: f.companyId, workspaceId, apply: true }); + const [operation] = await db.select().from(workspaceOperations).where(eq(workspaceOperations.executionWorkspaceId, workspaceId)); + const imported = await workspace(workspaceId), authority = readRemoteWorkspaceAuthority(imported.metadata)!; + return { f, nested, issueId, workspaceId, cwd, operation: operation!, imported, authority }; + } + it.each(["committed", "superseded"] as const)("recognizes a canonically imported linked Git source (%s)", async status => { + const { f, nested, issueId, workspaceId, cwd, operation, imported, authority } = await importedLinkedFixture(); + if (status === "superseded") await db.update(executionWorkspaceSnapshots).set({ status }).where(eq(executionWorkspaceSnapshots.id, authority.snapshotId!)); + const scope = await inspectOnlineWorkspaceSource(db, await workspace(f.workspaceId)); + expect(scope).toMatchObject({ kind: "git", workspaceIds: [f.workspaceId], + importedChildren: [expect.objectContaining({ workspaceId, operationId: operation.id, sourcePath: cwd })] }); + expect(scope!.physicalRoots).toContain(cwd); expect(scope!.issueIds).toContain(issueId); + let acquire!: () => void, release!: () => void; + const acquired = new Promise(resolve => { acquire = resolve; }), released = new Promise(resolve => { release = resolve; }); + const holder = withWorkspaceSourceLocks(db, { companyId: f.companyId, issueIds: [issueId] }, async () => { acquire(); await released; }); + await acquired; + try { await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true })).rejects.toThrow("workspace_source_busy"); } + finally { release(); await holder; } + await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true })).resolves.toMatchObject({ applied: true }); + expect(await workspace(workspaceId)).toEqual(imported); + expect(await fs.readFile(path.join(cwd, "tracked.txt"), "utf8")).toBe("committed\n"); + expect(await fs.readFile(path.join(nested, "tracked.txt"), "utf8")).toBe("unstaged\n"); + }); + it.each(["journal", "snapshot", "manifest", "unfinished"] as const)("refuses an imported linked Git source with changed %s provenance", async change => { + const { f, operation, authority } = await importedLinkedFixture(); + if (change === "journal") await db.update(workspaceOperations).set({ status: "failed" }).where(eq(workspaceOperations.id, operation.id)); + if (change === "snapshot") await db.update(executionWorkspaceSnapshots).set({ status: "uploaded" }).where(eq(executionWorkspaceSnapshots.id, authority.snapshotId!)); + if (change === "manifest") await db.update(workspaceOperations).set({ metadata: { ...operation.metadata, manifestDigest: "a".repeat(64) } }).where(eq(workspaceOperations.id, operation.id)); + if (change === "unfinished") await db.update(workspaceOperations).set({ finishedAt: null }).where(eq(workspaceOperations.id, operation.id)); + expect(await inspectOnlineWorkspaceSource(db, await workspace(f.workspaceId))).toBeNull(); + await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true })).rejects.toThrow("online_workspace_source_scope_unproven"); + expect(readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)).toBeNull(); + }); + it("keeps a foreign local writer blocked after a linked Git source is imported", async () => { + const { f, cwd } = await importedLinkedFixture(), companyId = randomUUID(), issueId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Foreign owner", issuePrefix: "FOREIGN" }); + await db.insert(issues).values({ id: issueId, companyId, title: "Foreign source alias" }); + await db.insert(executionWorkspaces).values({ companyId, sourceIssueId: issueId, name: "Foreign source alias", mode: "isolated_workspace", + strategyType: "git_worktree", providerType: "git_worktree", cwd, providerRef: cwd }); + expect(await inspectOnlineWorkspaceSource(db, await workspace(f.workspaceId))).toBeNull(); + await expect(migrateRemoteWorkspace({ db, storage, ...f, apply: true })).rejects.toThrow("online_workspace_source_scope_unproven"); + expect(readRemoteWorkspaceAuthority((await workspace(f.workspaceId)).metadata)).toBeNull(); + }); it.each(["unregistered", "backlink", "pointer", "symlink-backlink"])("refuses a nested Git wrapper with an unproven linked %s", async mode => { const { f, nested } = await nestedGitWrapperFixture(), issueId = randomUUID(); await db.insert(issues).values({ id: issueId, companyId: f.companyId, title: "Linked wrapper" });