Skip to content

Commit ed1e178

Browse files
authored
fix(worktree): clean error when restoring/promoting onto an occupied branch
createWorktreeForExistingBranch only checked that the branch existed and that the worktree name/path was free -- never whether the branch was already checked out in another worktree. When it was, git failed with a raw "git worktree add exited with code 128" error that escaped through the restore and promote paths into error tracking. The workspace creation path already caught the "is already used by worktree" message and surfaced a friendly error, but the restore (suspension/archive) and promote paths did not. Detect the occupied branch up front in createWorktreeForExistingBranch and throw a clean, actionable message instead. The wording mirrors git's own so existing consumers that key on the "is already used by worktree" substring keep recognising the case. Fixing it at this shared chokepoint covers all three callers (create, restore, promote). Generated-By: PostHog Code Task-Id: c5eefa8e-bed0-4490-bb76-948ba7260998
1 parent d9f9204 commit ed1e178

2 files changed

Lines changed: 61 additions & 0 deletions

File tree

packages/git/src/worktree.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,30 @@ describe("WorktreeManager lifecycle (add / exists / list / remove / prune)", ()
229229
expect(await dirExists(info.worktreePath)).toBe(false);
230230
expect(await manager.listWorktrees()).toEqual([]);
231231
});
232+
233+
it("throws a clean occupied-branch error when the branch is already checked out", async () => {
234+
await createGitClient(localDir).branch(["feature"]);
235+
236+
const manager = new WorktreeManager({
237+
mainRepoPath: localDir,
238+
worktreeBasePath: worktreeBaseDir,
239+
});
240+
241+
// First checkout succeeds and claims the branch.
242+
const first = await manager.createWorktreeForExistingBranch("feature");
243+
expect(await dirExists(first.worktreePath)).toBe(true);
244+
245+
// Second checkout must fail with a clean, actionable message rather than the
246+
// raw "git worktree add exited with code 128" error. The message keeps the
247+
// "is already used by worktree" substring the workspace and restore paths
248+
// match on.
249+
await expect(
250+
manager.createWorktreeForExistingBranch("feature"),
251+
).rejects.toThrow(/is already used by worktree/);
252+
await expect(
253+
manager.createWorktreeForExistingBranch("feature"),
254+
).rejects.not.toThrow(/exited with code 128/);
255+
});
232256
});
233257

234258
describe("WorktreeManager worktree link/include processing", () => {

packages/git/src/worktree.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,19 @@ export class WorktreeManager {
250250
throw new Error(`Branch '${branch}' does not exist`);
251251
}
252252

253+
// Git refuses to check a branch out in a second worktree, failing with an
254+
// opaque "git worktree add exited with code 128" error. Detect that up
255+
// front and throw a clean, actionable message. The wording mirrors git's
256+
// own ("is already used by worktree at ...") so callers that key on that
257+
// substring — the workspace creation path and the restore toast — keep
258+
// recognising the occupied-branch case.
259+
const occupyingWorktreePath = await this.findWorktreePathForBranch(branch);
260+
if (occupyingWorktreePath) {
261+
throw new Error(
262+
`Branch '${branch}' is already used by worktree at '${occupyingWorktreePath}'`,
263+
);
264+
}
265+
253266
const worktreeName = await this.resolveAvailableWorktreeName(preferredName);
254267
const { worktreePath, targetPath } =
255268
await this.prepareWorktreePath(worktreeName);
@@ -345,6 +358,30 @@ export class WorktreeManager {
345358
};
346359
}
347360

361+
/**
362+
* Returns the path of an existing worktree that already has `branch` checked
363+
* out (including the main checkout), or null when the branch is free. Uses
364+
* git's own worktree list, so it covers worktrees anywhere on disk, not just
365+
* managed ones under the base path.
366+
*/
367+
private async findWorktreePathForBranch(
368+
branch: string,
369+
): Promise<string | null> {
370+
try {
371+
const worktrees = await listWorktreesRaw(this.mainRepoPath);
372+
const match = worktrees.find((wt) => wt.branch === branch);
373+
return match ? match.path : null;
374+
} catch (error) {
375+
// Degrade to the prior behaviour: if the check can't run, let the git
376+
// command proceed rather than blocking creation on a listing failure.
377+
this.log.warn("Failed to check for existing worktree on branch", {
378+
branch,
379+
error: error instanceof Error ? error.message : String(error),
380+
});
381+
return null;
382+
}
383+
}
384+
348385
/**
349386
* Resolves a worktree name that does not collide with an existing worktree,
350387
* falling back to a freshly generated unique name when the preferred (or

0 commit comments

Comments
 (0)