root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(FN-709): store branch name on tasks for reliable merge recovery

Browse Source 
- Add 'branch' field to Task and ArchivedTaskEntry types with DB migration
- Executor stores branch name on task after worktree assignment
- Merger reads branch from task metadata instead of relying on worktree state
- Implement non-destructive conflict recovery in prepareForTask with reset/clean
- Add tests for branch storage, merger branch reading, and worktree recovery
This commit is contained in:
gsxdsm
2026-04-02 14:08:31 -07:00
parent 98dd0fad01
commit 0258c21ff6
10 changed files with 241 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
packages/engine/src/executor.test.ts
View File

@@ -426,6 +426,7 @@ describe("TaskExecutor worktree naming", () => {
// The worktree path stored should use the generated name, not the task ID
expect(store.updateTask).toHaveBeenCalledWith("FN-030", {
worktree: "/tmp/test/.worktrees/swift-falcon",
branch: "kb/fn-030",
});
expect(mockedGenerateWorktreeName).toHaveBeenCalledWith("/tmp/test");
});
@@ -477,6 +478,7 @@ describe("TaskExecutor worktree naming", () => {
// Should use task ID (lowercase) as worktree name
expect(store.updateTask).toHaveBeenCalledWith("FN-042", {
worktree: "/tmp/test/.worktrees/fn-042",
branch: "kb/fn-042",
});
// Should NOT call generateWorktreeName when using task-id
expect(mockedGenerateWorktreeName).not.toHaveBeenCalled();
@@ -503,6 +505,7 @@ describe("TaskExecutor worktree naming", () => {
const expectedSlug = slugify("Fix login bug with OAuth");
expect(store.updateTask).toHaveBeenCalledWith("FN-043", {
worktree: `/tmp/test/.worktrees/${expectedSlug}`,
branch: "kb/fn-043",
});
expect(mockedGenerateWorktreeName).not.toHaveBeenCalled();
});
@@ -530,6 +533,7 @@ describe("TaskExecutor worktree naming", () => {
const expectedSlug = slugify(taskDescription.slice(0, 60));
expect(store.updateTask).toHaveBeenCalledWith("FN-044", {
worktree: `/tmp/test/.worktrees/${expectedSlug}`,
branch: "kb/fn-044",
});
});
@@ -550,6 +554,7 @@ describe("TaskExecutor worktree naming", () => {
// Should use generateWorktreeName for random mode
expect(store.updateTask).toHaveBeenCalledWith("FN-045", {
worktree: "/tmp/test/.worktrees/swift-falcon",
branch: "kb/fn-045",
});
expect(mockedGenerateWorktreeName).toHaveBeenCalledWith("/tmp/test");
});
@@ -571,6 +576,7 @@ describe("TaskExecutor worktree naming", () => {
// Should default to random naming
expect(store.updateTask).toHaveBeenCalledWith("FN-046", {
worktree: "/tmp/test/.worktrees/swift-falcon",
branch: "kb/fn-046",
});
expect(mockedGenerateWorktreeName).toHaveBeenCalledWith("/tmp/test");
});
@@ -600,6 +606,7 @@ describe("TaskExecutor worktree naming", () => {
// Should acquire from pool, ignoring the task-id naming preference
expect(store.updateTask).toHaveBeenCalledWith("FN-047", {
worktree: "/tmp/test/.worktrees/pooled-warm-wt",
branch: "kb/fn-047",
});
// Should NOT call generateWorktreeName when using pooled worktree
expect(mockedGenerateWorktreeName).not.toHaveBeenCalled();
@@ -1148,7 +1155,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
(p) => p === "/tmp/test/.worktrees/idle-wt",
);
const prepareSpy = vi.spyOn(pool, "prepareForTask");
const prepareSpy = vi.spyOn(pool, "prepareForTask").mockReturnValue("kb/fn-064");
const store = createMockStore();
store.getSettings.mockResolvedValue({
@@ -1181,7 +1188,7 @@ describe("TaskExecutor dependency-based worktree creation", () => {
(p) => p === "/tmp/test/.worktrees/idle-wt",
);
const prepareSpy = vi.spyOn(pool, "prepareForTask");
const prepareSpy = vi.spyOn(pool, "prepareForTask").mockReturnValue("kb/fn-065");
const store = createMockStore();
store.getSettings.mockResolvedValue({
@@ -1205,6 +1212,39 @@ describe("TaskExecutor dependency-based worktree creation", () => {
undefined,
);
});
it("stores suffixed branch name when pool returns a different name", async () => {
const pool = new WorktreePool();
pool.release("/tmp/test/.worktrees/idle-wt");
mockedExistsSync.mockImplementation(
(p) => p === "/tmp/test/.worktrees/idle-wt",
);
// Pool returns a suffixed branch name due to conflict
vi.spyOn(pool, "prepareForTask").mockReturnValue("kb/fn-066-2");
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({
id: "FN-066",
}));
// Should store the suffixed branch name
expect(store.updateTask).toHaveBeenCalledWith("FN-066", {
worktree: "/tmp/test/.worktrees/idle-wt",
branch: "kb/fn-066-2",
});
});
});
describe("TaskExecutor worktree pool integration", () => {
@@ -1352,6 +1392,10 @@ describe("TaskExecutor worktree pool integration", () => {
it("falls through to fresh worktree when pool prepareForTask throws", async () => {
const pool = new WorktreePool();
pool.release("/tmp/test/.worktrees/bad-wt");
// Pool path must exist on disk for acquire() to return it
mockedExistsSync.mockImplementation(
(p) => p === "/tmp/test/.worktrees/bad-wt",
);
// Make prepareForTask throw
vi.spyOn(pool, "prepareForTask").mockImplementation(() => {
throw new Error("branch conflict unrecoverable");

18
packages/engine/src/executor.ts
View File

@@ -425,12 +425,17 @@ export class TaskExecutor {
const pooled = this.options.pool.acquire();
if (pooled) {
try {
this.options.pool.prepareForTask(pooled, branchName, baseBranch ?? undefined);
const actualBranch = 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}`);
await this.store.updateTask(task.id, { worktree: worktreePath, branch: actualBranch });
if (actualBranch !== branchName) {
executorLog.log(`Branch conflict resolved: using ${actualBranch} instead of ${branchName}`);
await this.store.logEntry(task.id, `Acquired worktree from pool: ${worktreePath} (branch conflict: using ${actualBranch})`);
} else {
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
@@ -447,7 +452,7 @@ export class TaskExecutor {
// Fall through to fresh worktree creation if pool had nothing
if (!acquiredFromPool) {
worktreePath = await this.createWorktree(branchName, worktreePath, task.id, baseBranch ?? undefined);
await this.store.updateTask(task.id, { worktree: worktreePath });
await this.store.updateTask(task.id, { worktree: worktreePath, branch: branchName });
if (baseBranch) {
await this.store.logEntry(task.id, `Worktree created at ${worktreePath} (based on ${baseBranch})`);
@@ -1130,8 +1135,9 @@ export class TaskExecutor {
// Worktree may already be gone
}
// Delete the branch
const branch = `kb/${taskId.toLowerCase()}`;
// Delete the branch — use stored branch name if available, fall back to convention
const task = await this.store.getTask(taskId);
const branch = task.branch || `kb/${taskId.toLowerCase()}`;
try {
execSync(`git branch -D "${branch}"`, { cwd: this.rootDir, stdio: "pipe" });
} catch {

48
packages/engine/src/merger.test.ts
View File

@@ -211,6 +211,54 @@ describe("aiMergeTask — conditional worktree cleanup", () => {
});
});
describe("aiMergeTask — task.branch field", () => {
beforeEach(() => {
vi.clearAllMocks();
mockedExistsSync.mockReturnValue(true);
setupHappyPathExecSync();
mockedCreateHaiAgent.mockResolvedValue({
session: {
prompt: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn(),
},
} as any);
});
it("uses task.branch when set instead of deriving from task ID", async () => {
const store = createMockStore(
{ id: "FN-050", branch: "kb/fn-050-2", worktree: "/tmp/root/.worktrees/KB-050" },
[{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
);
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
// Should use kb/fn-050-2, not kb/fn-050
expect(result.branch).toBe("kb/fn-050-2");
// Verify the suffixed branch was verified and deleted
const revParseCall = mockedExecSync.mock.calls.find(
(call) => String(call[0]).includes("rev-parse --verify") && String(call[0]).includes("kb/fn-050-2"),
);
expect(revParseCall).toBeDefined();
const branchDeleteCall = mockedExecSync.mock.calls.find(
(call) => String(call[0]).includes("branch -d") && String(call[0]).includes("kb/fn-050-2"),
);
expect(branchDeleteCall).toBeDefined();
});
it("falls back to conventional branch name when task.branch is not set", async () => {
const store = createMockStore(
{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050" },
[{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
);
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
expect(result.branch).toBe("kb/fn-050");
});
});
describe("aiMergeTask — empty squash merge (branch already merged via dep)", () => {
beforeEach(() => {
vi.clearAllMocks();

2
packages/engine/src/merger.ts
View File

@@ -557,7 +557,7 @@ export async function aiMergeTask(
);
}
const branch = `kb/${taskId.toLowerCase()}`;
const branch = task.branch || `kb/${taskId.toLowerCase()}`;
const worktreePath = task.worktree;
const result: MergeResult = {
task,

119
packages/engine/src/worktree-pool.test.ts
View File

@@ -122,6 +122,11 @@ describe("WorktreePool", () => {
});
describe("prepareForTask", () => {
it("returns the original branch name on success", () => {
const result = pool.prepareForTask("/tmp/wt", "fusion/fn-042");
expect(result).toBe("fusion/fn-042");
});
it("cleans dirty working tree before checkout", () => {
pool.prepareForTask("/tmp/wt", "fusion/fn-042");
@@ -165,8 +170,8 @@ describe("WorktreePool", () => {
return Buffer.from("");
});
// Should not throw
expect(() => pool.prepareForTask("/tmp/wt", "fusion/fn-001")).not.toThrow();
const result = pool.prepareForTask("/tmp/wt", "fusion/fn-001");
expect(result).toBe("fusion/fn-001");
// Should still run clean and branch creation
const calls = mockedExecSync.mock.calls.map((c) => c[0]);
@@ -174,45 +179,65 @@ describe("WorktreePool", () => {
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) => {
it("uses suffixed branch name when original is in use by an active worktree", () => {
mockedExistsSync.mockImplementation((p) => {
// The conflicting worktree exists on disk
if (p === "/other/wt") return true;
return true;
});
mockedExecSync.mockImplementation((cmd: 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("");
if (cmdStr === 'git checkout -B "fusion/fn-042" main') {
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;
}
return Buffer.from("");
});
expect(() => pool.prepareForTask("/tmp/wt", "fusion/fn-042")).not.toThrow();
const result = pool.prepareForTask("/tmp/wt", "fusion/fn-042");
expect(result).toBe("fusion/fn-042-2");
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");
// Verify the suffixed checkout was called
const checkoutCalls = mockedExecSync.mock.calls
.map((c) => c[0])
.filter((c) => typeof c === "string" && c.includes("checkout -B"));
expect(checkoutCalls).toContain('git checkout -B "fusion/fn-042-2" main');
});
it("falls back to git worktree prune when detach in conflicting path fails", () => {
it("increments suffix when lower suffixes are also in use", () => {
mockedExistsSync.mockReturnValue(true);
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
// Original and -2 are both in use
if (cmdStr === 'git checkout -B "fusion/fn-042" main' ||
cmdStr === 'git checkout -B "fusion/fn-042-2" main') {
const err: any = new Error("branch conflict");
err.stderr = Buffer.from(
`fatal: 'x' is already used by worktree at '/other/wt'`
);
throw err;
}
return Buffer.from("");
});
const result = pool.prepareForTask("/tmp/wt", "fusion/fn-042");
expect(result).toBe("fusion/fn-042-3");
});
it("falls back to git worktree prune when conflicting worktree no longer exists on disk", () => {
mockedExistsSync.mockImplementation((p) => {
// The conflicting worktree does NOT exist
if (p === "/gone/wt") return false;
return true;
});
let checkoutBCount = 0;
mockedExecSync.mockImplementation((cmd: any, opts: any) => {
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("checkout -B")) {
checkoutBCount++;
@@ -225,13 +250,11 @@ describe("WorktreePool", () => {
}
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 result = pool.prepareForTask("/tmp/wt", "fusion/fn-042");
expect(result).toBe("fusion/fn-042");
const cmds = mockedExecSync.mock.calls.map((c) => c[0]);
expect(cmds).toContain("git worktree prune");
@@ -252,26 +275,24 @@ describe("WorktreePool", () => {
);
});
it("re-throws when recovery itself fails", () => {
let checkoutBCount = 0;
it("throws when all suffixed names are exhausted", () => {
mockedExistsSync.mockReturnValue(true);
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");
const err: any = new Error("branch conflict");
err.stderr = Buffer.from(
`fatal: 'x' is already used by worktree at '/other/wt'`
);
throw err;
}
return Buffer.from("");
});
expect(() => pool.prepareForTask("/tmp/wt", "fusion/fn-042")).toThrow("still broken");
expect(() => pool.prepareForTask("/tmp/wt", "fusion/fn-042")).toThrow(
/suffixes -2 through -6 are all in use/
);
});
});

43
packages/engine/src/worktree-pool.ts
View File

@@ -110,11 +110,15 @@ export class WorktreePool {
* 2. `git clean -fd` — remove untracked files (but not .gitignore'd caches)
* 3. `git checkout -B <branchName> <startPoint>` — create/reset branch from start point
*
* Returns the actual branch name used. This may differ from `branchName`
* when conflict recovery generates a suffixed name (e.g., `kb/fn-042-2`).
*
* @param worktreePath — Absolute path to the recycled worktree
* @param branchName — Branch name for the new task (e.g., `kb/kb-042`)
* @param startPoint — Git ref to branch from (e.g., `kb/kb-041`). Defaults to `main`.
* @returns The actual branch name checked out in the worktree
*/
prepareForTask(worktreePath: string, branchName: string, startPoint?: string): void {
prepareForTask(worktreePath: string, branchName: string, startPoint?: string): string {
// Clean tracked modifications
try {
execSync("git checkout -- .", { cwd: worktreePath, stdio: "pipe" });
@@ -133,6 +137,7 @@ export class WorktreePool {
cwd: worktreePath,
stdio: "pipe",
});
return branchName;
} catch (err: any) {
const stderr = err?.stderr?.toString() ?? err?.message ?? "";
const match = stderr.match(/already used by worktree at '([^']+)'/);
@@ -140,17 +145,37 @@ export class WorktreePool {
throw err;
}
// The branch is checked out in a different worktree — detach it there,
// delete the stale branch, then retry.
// The branch is checked out in a different worktree.
// First check if the conflicting worktree still exists on disk.
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
if (!existsSync(conflictingPath)) {
// Conflicting worktree no longer exists — prune and retry with original name
execSync("git worktree prune", { cwd: worktreePath, stdio: "pipe" });
execSync(checkoutCmd, { cwd: worktreePath, stdio: "pipe" });
return branchName;
}
execSync(`git branch -D "${branchName}"`, { cwd: worktreePath, stdio: "pipe" });
execSync(checkoutCmd, { cwd: worktreePath, stdio: "pipe" });
// Conflicting worktree exists and is active — use a suffixed branch name
// to avoid disrupting the other worktree
for (let suffix = 2; suffix <= 6; suffix++) {
const suffixedName = `${branchName}-${suffix}`;
const suffixedCmd = `git checkout -B "${suffixedName}" ${base}`;
try {
execSync(suffixedCmd, { cwd: worktreePath, stdio: "pipe" });
return suffixedName;
} catch (suffixErr: any) {
const suffixStderr = suffixErr?.stderr?.toString() ?? "";
if (!suffixStderr.includes("already used by worktree")) {
throw suffixErr;
}
// This suffixed name is also in use — try the next one
}
}
// All suffixed names exhausted — should not happen in practice
throw new Error(
`Cannot create branch for task: "${branchName}" and suffixes -2 through -6 are all in use by other worktrees`,
);
}
}
}