From 2ce37a4f94d962c0eabba67cb43d2f0ed975a194 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 11 Apr 2026 21:14:58 -0500 Subject: [PATCH] fix: validate linked worktrees before reuse --- .../src/__tests__/workspace-runtime.test.ts | 98 +++++++++++++++++++ server/src/services/workspace-runtime.ts | 80 +++++++++++++-- 2 files changed, 170 insertions(+), 8 deletions(-) diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index cf0c144a1a3..19bb57088f2 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -367,6 +367,42 @@ describe("realizeExecutionWorkspace", () => { expect(second.branchName).toBe(first.branchName); }); + it("rejects reusing an empty directory that only looks like a worktree because it sits inside the repo", async () => { + const repoRoot = await createTempRepo(); + const branchName = "PAP-447-add-worktree-support"; + const poisonedPath = path.join(repoRoot, ".paperclip", "worktrees", branchName); + await fs.mkdir(poisonedPath, { recursive: true }); + + await expect( + realizeExecutionWorkspace({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + config: { + workspaceStrategy: { + type: "git_worktree", + branchTemplate: "{{issue.identifier}}-{{slug}}", + }, + }, + issue: { + id: "issue-1", + identifier: "PAP-447", + title: "Add Worktree Support", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + }), + ).rejects.toThrow(/not a reusable git worktree \(path is not registered in `git worktree list`\)\./); + }); + it("reuses the current linked worktree instead of nesting another worktree inside it", async () => { const repoRoot = await createTempRepo(); const branchName = "PAP-1355-worktree-reuse"; @@ -408,6 +444,68 @@ describe("realizeExecutionWorkspace", () => { await expect(fs.realpath(realized.worktreePath ?? "")).resolves.toBe(expectedWorktreePath); }); + it("rejects reusing a linked worktree whose branch drifted from the expected issue branch", async () => { + const repoRoot = await createTempRepo(); + + const initial = await realizeExecutionWorkspace({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + config: { + workspaceStrategy: { + type: "git_worktree", + branchTemplate: "{{issue.identifier}}-{{slug}}", + }, + }, + issue: { + id: "issue-1", + identifier: "PAP-447", + title: "Add Worktree Support", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + }); + + await runGit(initial.cwd, ["checkout", "-b", "unexpected-branch"]); + + await expect( + realizeExecutionWorkspace({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + config: { + workspaceStrategy: { + type: "git_worktree", + branchTemplate: "{{issue.identifier}}-{{slug}}", + }, + }, + issue: { + id: "issue-1", + identifier: "PAP-447", + title: "Add Worktree Support", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + }), + ).rejects.toThrow(/not a reusable git worktree \(worktree HEAD is on "unexpected-branch" instead of "PAP-447-add-worktree-support"\)\./); + }); + it("reuses an already checked out branch from git worktree metadata even when the target path differs", async () => { const repoRoot = await createTempRepo(); const branchName = "PAP-1355-worktree-reuse"; diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index dce5a258ba7..bcfb4908a65 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -608,6 +608,56 @@ async function directoryExists(value: string) { return fs.stat(value).then((stats) => stats.isDirectory()).catch(() => false); } +async function listLinkedGitWorktreePaths(repoRoot: string): Promise> { + const output = await runGit(["worktree", "list", "--porcelain"], repoRoot); + const paths = new Set(); + for (const line of output.split("\n")) { + if (!line.startsWith("worktree ")) continue; + const worktree = line.slice("worktree ".length).trim(); + if (!worktree) continue; + paths.add(path.resolve(worktree)); + } + return paths; +} + +async function validateLinkedGitWorktree(input: { + repoRoot: string; + worktreePath: string; + expectedBranchName: string | null; +}): Promise<{ valid: true } | { valid: false; reason: string }> { + const resolvedWorktreePath = path.resolve(input.worktreePath); + const listedWorktrees = await listLinkedGitWorktreePaths(input.repoRoot); + if (!listedWorktrees.has(resolvedWorktreePath)) { + return { + valid: false, + reason: "path is not registered in `git worktree list`", + }; + } + + const worktreeTopLevel = await runGit(["rev-parse", "--show-toplevel"], resolvedWorktreePath).catch(() => null); + if (!worktreeTopLevel || path.resolve(worktreeTopLevel) !== resolvedWorktreePath) { + return { + valid: false, + reason: "git resolves this path to a different repository root", + }; + } + + if (input.expectedBranchName) { + const currentBranch = await runGit( + ["symbolic-ref", "--quiet", "--short", "HEAD"], + resolvedWorktreePath, + ).catch(() => null); + if (currentBranch !== input.expectedBranchName) { + return { + valid: false, + reason: `worktree HEAD is on "${currentBranch ?? ""}" instead of "${input.expectedBranchName}"`, + }; + } + } + + return { valid: true }; +} + function terminateChildProcess(child: ChildProcess) { if (!child.pid) return; if (process.platform !== "win32") { @@ -1008,18 +1058,32 @@ export async function realizeExecutionWorkspace(input: { }; } + async function validateReusableWorktree(reusablePath: string) { + return await validateLinkedGitWorktree({ + repoRoot, + worktreePath: reusablePath, + expectedBranchName: branchName, + }).catch(() => null); + } + const existingWorktree = await directoryExists(worktreePath); - if (existingWorktree && await isGitCheckout(worktreePath)) { - return await reuseExistingWorktree(worktreePath); + if (existingWorktree) { + const validation = await validateReusableWorktree(worktreePath); + if (validation?.valid) { + return await reuseExistingWorktree(worktreePath); + } + const reason = validation && !validation.valid ? ` (${validation.reason})` : ""; + throw new Error(`Configured worktree path "${worktreePath}" already exists and is not a reusable git worktree${reason}.`); } const registeredBranchWorktree = await findRegisteredGitWorktreeByBranch(repoRoot, branchName); - if (registeredBranchWorktree && await isGitCheckout(registeredBranchWorktree)) { - return await reuseExistingWorktree(registeredBranchWorktree); - } - - if (existingWorktree) { - throw new Error(`Configured worktree path "${worktreePath}" already exists and is not a git worktree.`); + if (registeredBranchWorktree) { + const validation = await validateReusableWorktree(registeredBranchWorktree); + if (validation?.valid) { + return await reuseExistingWorktree(registeredBranchWorktree); + } + const reason = validation && !validation.valid ? ` (${validation.reason})` : ""; + throw new Error(`Registered worktree for branch "${branchName}" at "${registeredBranchWorktree}" is not reusable${reason}.`); } try {