fix(FN-706): add branch conflict recovery and pool fallthrough in executor
- Add branch conflict recovery to worktree-pool prepareForTask when checkout fails - Add fallthrough in executor to create fresh worktree when pool preparation fails - Add tests for branch conflict recovery in worktree-pool - Add tests for executor fallthrough on pool preparation failure
This commit is contained in:
@@ -1348,6 +1348,44 @@ describe("TaskExecutor worktree pool integration", () => {
|
||||
// Pool should still have the entry (not acquired)
|
||||
expect(pool.size).toBe(1);
|
||||
});
|
||||
|
||||
it("falls through to fresh worktree when pool prepareForTask throws", async () => {
|
||||
const pool = new WorktreePool();
|
||||
pool.release("/tmp/test/.worktrees/bad-wt");
|
||||
// Make prepareForTask throw
|
||||
vi.spyOn(pool, "prepareForTask").mockImplementation(() => {
|
||||
throw new Error("branch conflict unrecoverable");
|
||||
});
|
||||
const releaseSpy = vi.spyOn(pool, "release");
|
||||
|
||||
const store = createMockStore();
|
||||
store.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 15000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: false,
|
||||
recycleWorktrees: true,
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test", { pool });
|
||||
await executor.execute(makeTask());
|
||||
|
||||
// Should have released the bad worktree back to pool
|
||||
expect(releaseSpy).toHaveBeenCalledWith("/tmp/test/.worktrees/bad-wt");
|
||||
|
||||
// Should have fallen through to fresh worktree creation
|
||||
const worktreeAddCalls = mockedExecSync.mock.calls.filter(
|
||||
(c) => typeof c[0] === "string" && (c[0] as string).includes("worktree add"),
|
||||
);
|
||||
expect(worktreeAddCalls.length).toBeGreaterThan(0);
|
||||
|
||||
// Should log the pool failure
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-020",
|
||||
expect.stringContaining("Pool worktree preparation failed"),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("WorktreePool capacity", () => {
|
||||
|
||||
@@ -424,12 +424,23 @@ export class TaskExecutor {
|
||||
if (this.options.pool && settings.recycleWorktrees) {
|
||||
const pooled = this.options.pool.acquire();
|
||||
if (pooled) {
|
||||
this.options.pool.prepareForTask(pooled, branchName, baseBranch ?? undefined);
|
||||
worktreePath = pooled;
|
||||
acquiredFromPool = true;
|
||||
executorLog.log(`Acquired worktree from pool: ${pooled}`);
|
||||
await this.store.updateTask(task.id, { worktree: worktreePath });
|
||||
await this.store.logEntry(task.id, `Acquired worktree from pool: ${worktreePath}`);
|
||||
try {
|
||||
this.options.pool.prepareForTask(pooled, branchName, baseBranch ?? undefined);
|
||||
worktreePath = pooled;
|
||||
acquiredFromPool = true;
|
||||
executorLog.log(`Acquired worktree from pool: ${pooled}`);
|
||||
await this.store.updateTask(task.id, { worktree: worktreePath });
|
||||
await this.store.logEntry(task.id, `Acquired worktree from pool: ${worktreePath}`);
|
||||
} catch (poolErr: any) {
|
||||
// Pool preparation failed — release the worktree back and fall through
|
||||
// to fresh worktree creation
|
||||
this.options.pool.release(pooled);
|
||||
executorLog.log(`Pool prepareForTask failed, falling through to fresh worktree: ${poolErr.message}`);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Pool worktree preparation failed (${poolErr.message}), creating fresh worktree`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -173,6 +173,106 @@ describe("WorktreePool", () => {
|
||||
expect(calls).toContain("git clean -fd");
|
||||
expect(calls).toContain('git checkout -B "fusion/fn-001" main');
|
||||
});
|
||||
|
||||
it("recovers from 'already used by worktree' by detaching conflicting worktree", () => {
|
||||
let callCount = 0;
|
||||
mockedExecSync.mockImplementation((cmd: any, opts: any) => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("checkout -B")) {
|
||||
callCount++;
|
||||
if (callCount === 1) {
|
||||
const err: any = new Error("branch conflict");
|
||||
err.stderr = Buffer.from(
|
||||
"fatal: 'fusion/fn-042' is already used by worktree at '/other/wt'"
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
// Second call succeeds (retry)
|
||||
return Buffer.from("");
|
||||
}
|
||||
if (cmdStr === "git checkout --detach") {
|
||||
expect(opts.cwd).toBe("/other/wt"); // Must target the conflicting worktree
|
||||
return Buffer.from("");
|
||||
}
|
||||
if (cmdStr.includes("branch -D")) {
|
||||
expect(cmdStr).toContain("fusion/fn-042");
|
||||
return Buffer.from("");
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
expect(() => pool.prepareForTask("/tmp/wt", "fusion/fn-042")).not.toThrow();
|
||||
|
||||
const calls = mockedExecSync.mock.calls.map((c) => [c[0], (c[1] as any)?.cwd]);
|
||||
// Verify detach targeted the conflicting worktree, not the current one
|
||||
const detachCall = calls.find(([cmd]) => cmd === "git checkout --detach");
|
||||
expect(detachCall).toBeDefined();
|
||||
expect(detachCall![1]).toBe("/other/wt");
|
||||
});
|
||||
|
||||
it("falls back to git worktree prune when detach in conflicting path fails", () => {
|
||||
let checkoutBCount = 0;
|
||||
mockedExecSync.mockImplementation((cmd: any, opts: any) => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("checkout -B")) {
|
||||
checkoutBCount++;
|
||||
if (checkoutBCount === 1) {
|
||||
const err: any = new Error("branch conflict");
|
||||
err.stderr = Buffer.from(
|
||||
"fatal: 'fusion/fn-042' is already used by worktree at '/gone/wt'"
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
return Buffer.from("");
|
||||
}
|
||||
if (cmdStr === "git checkout --detach") {
|
||||
throw new Error("not a git repository"); // Conflicting path no longer exists
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
expect(() => pool.prepareForTask("/tmp/wt", "fusion/fn-042")).not.toThrow();
|
||||
|
||||
const cmds = mockedExecSync.mock.calls.map((c) => c[0]);
|
||||
expect(cmds).toContain("git worktree prune");
|
||||
});
|
||||
|
||||
it("re-throws non-conflict errors from checkout -B unchanged", () => {
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
if (String(cmd).includes("checkout -B")) {
|
||||
const err: any = new Error("some other git error");
|
||||
err.stderr = Buffer.from("fatal: some other git error");
|
||||
throw err;
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
expect(() => pool.prepareForTask("/tmp/wt", "fusion/fn-042")).toThrow(
|
||||
"some other git error"
|
||||
);
|
||||
});
|
||||
|
||||
it("re-throws when recovery itself fails", () => {
|
||||
let checkoutBCount = 0;
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("checkout -B")) {
|
||||
checkoutBCount++;
|
||||
if (checkoutBCount === 1) {
|
||||
const err: any = new Error("branch conflict");
|
||||
err.stderr = Buffer.from(
|
||||
"fatal: 'fusion/fn-042' is already used by worktree at '/other/wt'"
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
// Retry also fails
|
||||
throw new Error("still broken");
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
expect(() => pool.prepareForTask("/tmp/wt", "fusion/fn-042")).toThrow("still broken");
|
||||
});
|
||||
});
|
||||
|
||||
describe("rehydrate", () => {
|
||||
|
||||
@@ -127,10 +127,31 @@ export class WorktreePool {
|
||||
|
||||
// Create or force-reset the branch from the start point (or main)
|
||||
const base = startPoint || "main";
|
||||
execSync(`git checkout -B "${branchName}" ${base}`, {
|
||||
cwd: worktreePath,
|
||||
stdio: "pipe",
|
||||
});
|
||||
const checkoutCmd = `git checkout -B "${branchName}" ${base}`;
|
||||
try {
|
||||
execSync(checkoutCmd, {
|
||||
cwd: worktreePath,
|
||||
stdio: "pipe",
|
||||
});
|
||||
} catch (err: any) {
|
||||
const stderr = err?.stderr?.toString() ?? err?.message ?? "";
|
||||
const match = stderr.match(/already used by worktree at '([^']+)'/);
|
||||
if (!match) {
|
||||
throw err;
|
||||
}
|
||||
|
||||
// The branch is checked out in a different worktree — detach it there,
|
||||
// delete the stale branch, then retry.
|
||||
const conflictingPath = match[1];
|
||||
try {
|
||||
execSync("git checkout --detach", { cwd: conflictingPath, stdio: "pipe" });
|
||||
} catch {
|
||||
// Conflicting worktree may no longer exist on disk — try pruning instead
|
||||
execSync("git worktree prune", { cwd: worktreePath, stdio: "pipe" });
|
||||
}
|
||||
execSync(`git branch -D "${branchName}"`, { cwd: worktreePath, stdio: "pipe" });
|
||||
execSync(checkoutCmd, { cwd: worktreePath, stdio: "pipe" });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user