mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 02:26:12 +02:00
fix: validate linked worktrees before reuse
This commit is contained in:
1 parent
3638036230
commit
2ce37a4f94
2 files changed
+170
-8
No files matched your search
@@ -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";
|
||||
|
||||
@@ -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<Set<string>> {
|
||||
const output = await runGit(["worktree", "list", "--porcelain"], repoRoot);
|
||||
const paths = new Set<string>();
|
||||
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 ?? "<detached>"}" 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 {
|
||||
|
||||
Reference in new issue
Block a user