fix(KB-170): handle worktree cleanup after merge retries exhausted
- Add mergeRetries counter to track per-task retry attempts - Auto-resolve lock files and generated files during retry attempts - Clean up worktree when all 3 merge retry attempts are exhausted - Add comprehensive tests for retry cleanup logic in executor - Add changeset for the worktree retry cleanup fix
This commit is contained in:
5
.changeset/fix-worktree-retry-cleanup.md
Normal file
5
.changeset/fix-worktree-retry-cleanup.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@dustinbyrne/kb": patch
|
||||
---
|
||||
|
||||
Automatically clean up a conflicting git worktree and retry creation when rerunning a failed task hits the "branch is already used by worktree" error.
|
||||
@@ -680,6 +680,80 @@ describe("TaskExecutor dependency-based worktree creation", () => {
|
||||
expect(logCalls[0][1]).not.toContain("based on");
|
||||
});
|
||||
|
||||
it("retries worktree creation after cleaning up conflicting worktree", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
const conflictingPath = "/tmp/test/.worktrees/sharp-stone";
|
||||
|
||||
let firstAttempt = true;
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
if (cmd === 'git worktree add -b "kb/kb-064" "/tmp/test/.worktrees/swift-falcon"' && firstAttempt) {
|
||||
firstAttempt = false;
|
||||
const err: any = new Error(
|
||||
`fatal: 'kb/kb-064' is already used by worktree at '${conflictingPath}'`,
|
||||
);
|
||||
err.stderr = Buffer.from(
|
||||
`fatal: 'kb/kb-064' is already used by worktree at '${conflictingPath}'`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
await executor.execute(makeTask({ id: "KB-064" }));
|
||||
|
||||
expect(mockedExecSync).toHaveBeenCalledWith(
|
||||
`git worktree remove "${conflictingPath}" --force`,
|
||||
expect.objectContaining({ cwd: "/tmp/test", stdio: "pipe" }),
|
||||
);
|
||||
expect(mockedExecSync).toHaveBeenCalledWith(
|
||||
'git branch -D "kb/kb-064"',
|
||||
expect.objectContaining({ cwd: "/tmp/test", stdio: "pipe" }),
|
||||
);
|
||||
|
||||
const worktreeCreateCalls = mockedExecSync.mock.calls.filter(
|
||||
(call) => call[0] === 'git worktree add -b "kb/kb-064" "/tmp/test/.worktrees/swift-falcon"',
|
||||
);
|
||||
expect(worktreeCreateCalls).toHaveLength(2);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"KB-064",
|
||||
expect.stringContaining("Worktree created at /tmp/test/.worktrees/swift-falcon"),
|
||||
);
|
||||
});
|
||||
|
||||
it("throws original error if cleanup also fails", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
const conflictingPath = "/tmp/test/.worktrees/sharp-stone";
|
||||
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
if (cmd === 'git worktree add -b "kb/kb-065" "/tmp/test/.worktrees/swift-falcon"') {
|
||||
const err: any = new Error(
|
||||
`fatal: 'kb/kb-065' is already used by worktree at '${conflictingPath}'`,
|
||||
);
|
||||
err.stderr = Buffer.from(
|
||||
`fatal: 'kb/kb-065' is already used by worktree at '${conflictingPath}'`,
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
if (cmd === `git worktree remove "${conflictingPath}" --force`) {
|
||||
throw new Error("remove failed");
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
await executor.execute(makeTask({ id: "KB-065" }));
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-065", {
|
||||
status: "failed",
|
||||
error: expect.stringContaining("already used by worktree"),
|
||||
});
|
||||
expect(store.updateTask).toHaveBeenCalledWith("KB-065", {
|
||||
status: "failed",
|
||||
error: expect.stringContaining("automatic cleanup failed: remove failed"),
|
||||
});
|
||||
});
|
||||
|
||||
it("passes baseBranch to pool prepareForTask when using pooled worktree", async () => {
|
||||
const pool = new WorktreePool();
|
||||
pool.release("/tmp/test/.worktrees/idle-wt");
|
||||
|
||||
@@ -963,21 +963,60 @@ export class TaskExecutor {
|
||||
executorLog.log(`Worktree already exists: ${path}`);
|
||||
return;
|
||||
}
|
||||
try {
|
||||
|
||||
const createWithBranch = () => {
|
||||
const cmd = startPoint
|
||||
? `git worktree add -b "${branch}" "${path}" "${startPoint}"`
|
||||
: `git worktree add -b "${branch}" "${path}"`;
|
||||
execSync(cmd, { cwd: this.rootDir, stdio: "pipe" });
|
||||
} catch {
|
||||
try {
|
||||
execSync(`git worktree add "${path}" "${branch}"`, { cwd: this.rootDir, stdio: "pipe" });
|
||||
} catch (e: any) {
|
||||
throw new Error(`Failed to create worktree: ${e.message}`);
|
||||
};
|
||||
|
||||
const createFromExistingBranch = () => {
|
||||
execSync(`git worktree add "${path}" "${branch}"`, { cwd: this.rootDir, stdio: "pipe" });
|
||||
};
|
||||
|
||||
try {
|
||||
createWithBranch();
|
||||
} catch (initialError: any) {
|
||||
const conflictPath = this.extractWorktreeConflictPath(initialError);
|
||||
|
||||
if (conflictPath) {
|
||||
try {
|
||||
execSync(`git worktree remove "${conflictPath}" --force`, {
|
||||
cwd: this.rootDir,
|
||||
stdio: "pipe",
|
||||
});
|
||||
execSync(`git branch -D "${branch}"`, {
|
||||
cwd: this.rootDir,
|
||||
stdio: "pipe",
|
||||
});
|
||||
createWithBranch();
|
||||
} catch (cleanupError: any) {
|
||||
throw new Error(
|
||||
`Failed to create worktree: ${initialError.message} ` +
|
||||
`(automatic cleanup failed: ${cleanupError.message})`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
try {
|
||||
createFromExistingBranch();
|
||||
} catch (e: any) {
|
||||
throw new Error(`Failed to create worktree: ${e.message}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
executorLog.log(`Worktree created: ${path}${startPoint ? ` (from ${startPoint})` : ""}`);
|
||||
}
|
||||
|
||||
private extractWorktreeConflictPath(error: any): string | null {
|
||||
const output = [error?.message, error?.stderr?.toString?.(), error?.stdout?.toString?.()]
|
||||
.filter(Boolean)
|
||||
.join("\n");
|
||||
const match = output.match(/already used by worktree at '([^']+)'/);
|
||||
return match?.[1] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a task's worktree, but only if no other in-progress or todo task
|
||||
* shares the same worktree path (dependency-chain reuse). The branch is
|
||||
|
||||
Reference in New Issue
Block a user