Skip to content

Commit f92dc14

Browse files
recuu-pfegclaude
andcommitted
feat: git merge conflict prevention — auto-rebase + escalation
- Rebase agent branch onto base before merge (reduces conflicts) - Conflict detection: extract conflicted files, abort safely - Escalation: task → blocked, review_comment with conflict details, WS notify - Cleanup consolidation into reusable cleanupBranch() - 9 new tests, 106 total pass Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 9088e0e commit f92dc14

2 files changed

Lines changed: 263 additions & 17 deletions

File tree

src/__tests__/git-merge.test.ts

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest";
2+
import { rebaseOntoBase, escalateConflict } from "../handlers/git-merge.js";
3+
import type { ActionContext } from "../agent-templates.js";
4+
5+
describe("rebaseOntoBase", () => {
6+
it("returns success when rebase completes without conflict", () => {
7+
const exec = vi.fn();
8+
const result = rebaseOntoBase("/repo", "agent/builder-abc", "main", exec as never);
9+
10+
expect(result.success).toBe(true);
11+
expect(result.conflictedFiles).toEqual([]);
12+
// Should checkout the branch then rebase
13+
expect(exec).toHaveBeenCalledTimes(2);
14+
expect(exec).toHaveBeenCalledWith(
15+
'git checkout "agent/builder-abc"',
16+
{ cwd: "/repo", stdio: "pipe" }
17+
);
18+
expect(exec).toHaveBeenCalledWith(
19+
'git rebase "main"',
20+
{ cwd: "/repo", stdio: "pipe" }
21+
);
22+
});
23+
24+
it("returns failure with conflicted files when rebase fails", () => {
25+
const exec = vi.fn()
26+
.mockImplementationOnce(() => {}) // checkout succeeds
27+
.mockImplementationOnce(() => { throw new Error("CONFLICT"); }) // rebase fails
28+
.mockReturnValueOnce(Buffer.from("src/file-a.ts\nsrc/file-b.ts\n")) // diff --name-only
29+
.mockImplementationOnce(() => {}); // rebase --abort
30+
31+
const result = rebaseOntoBase("/repo", "agent/builder-abc", "main", exec as never);
32+
33+
expect(result.success).toBe(false);
34+
expect(result.conflictedFiles).toEqual(["src/file-a.ts", "src/file-b.ts"]);
35+
// Should have called rebase --abort
36+
expect(exec).toHaveBeenCalledWith(
37+
"git rebase --abort",
38+
{ cwd: "/repo", stdio: "pipe" }
39+
);
40+
});
41+
42+
it("returns failure with empty conflictedFiles when diff also fails", () => {
43+
const exec = vi.fn()
44+
.mockImplementationOnce(() => {}) // checkout
45+
.mockImplementationOnce(() => { throw new Error("CONFLICT"); }) // rebase
46+
.mockImplementationOnce(() => { throw new Error("diff failed"); }) // diff fails
47+
.mockImplementationOnce(() => {}); // rebase --abort
48+
49+
const result = rebaseOntoBase("/repo", "agent/builder-abc", "main", exec as never);
50+
51+
expect(result.success).toBe(false);
52+
expect(result.conflictedFiles).toEqual([]);
53+
});
54+
55+
it("handles rebase --abort failure gracefully", () => {
56+
const exec = vi.fn()
57+
.mockImplementationOnce(() => {}) // checkout
58+
.mockImplementationOnce(() => { throw new Error("CONFLICT"); }) // rebase
59+
.mockReturnValueOnce(Buffer.from("file.ts\n")) // diff
60+
.mockImplementationOnce(() => { throw new Error("abort failed"); }); // abort fails
61+
62+
const result = rebaseOntoBase("/repo", "agent/builder-abc", "main", exec as never);
63+
64+
// Should still return the conflict info even if abort fails
65+
expect(result.success).toBe(false);
66+
expect(result.conflictedFiles).toEqual(["file.ts"]);
67+
});
68+
});
69+
70+
describe("escalateConflict", () => {
71+
let ctx: ActionContext;
72+
let mockUpdateTask: ReturnType<typeof vi.fn>;
73+
let mockOnDataUpdate: ReturnType<typeof vi.fn>;
74+
75+
beforeEach(() => {
76+
mockUpdateTask = vi.fn().mockResolvedValue(undefined);
77+
mockOnDataUpdate = vi.fn();
78+
ctx = {
79+
api: { updateTask: mockUpdateTask } as never,
80+
task: { id: "task-123", title: "Test task", description: "desc", status: "in_progress", priority: "p2" },
81+
agentName: "builder",
82+
config: {
83+
apiUrl: "http://localhost:8787",
84+
apiKey: "tb_test",
85+
workingDir: "/repo",
86+
baseBranch: "main",
87+
},
88+
onDataUpdate: mockOnDataUpdate as (entity: string, id: string, changes: Record<string, unknown>) => void,
89+
};
90+
});
91+
92+
it("updates task to blocked with conflict file list", async () => {
93+
await escalateConflict(ctx, ["src/a.ts", "src/b.ts"], "agent/builder-abc");
94+
95+
expect(mockUpdateTask).toHaveBeenCalledWith("task-123", {
96+
status: "blocked",
97+
review_comment: "Merge conflict on agent/builder-abc: src/a.ts, src/b.ts",
98+
});
99+
});
100+
101+
it("broadcasts data update via WS", async () => {
102+
await escalateConflict(ctx, ["src/a.ts"], "agent/builder-abc");
103+
104+
expect(mockOnDataUpdate).toHaveBeenCalledWith("task", "task-123", {
105+
status: "blocked",
106+
review_comment: "Merge conflict on agent/builder-abc: src/a.ts",
107+
});
108+
});
109+
110+
it("handles unknown files when conflict list is empty", async () => {
111+
await escalateConflict(ctx, [], "agent/builder-abc");
112+
113+
expect(mockUpdateTask).toHaveBeenCalledWith("task-123", {
114+
status: "blocked",
115+
review_comment: "Merge conflict on agent/builder-abc: (unknown files)",
116+
});
117+
});
118+
119+
it("does not throw when API update fails", async () => {
120+
mockUpdateTask.mockRejectedValue(new Error("network error"));
121+
122+
// Should not throw
123+
await escalateConflict(ctx, ["file.ts"], "agent/builder-abc");
124+
125+
// WS broadcast should still happen
126+
expect(mockOnDataUpdate).toHaveBeenCalled();
127+
});
128+
129+
it("works without onDataUpdate callback", async () => {
130+
ctx.onDataUpdate = undefined;
131+
132+
// Should not throw
133+
await escalateConflict(ctx, ["file.ts"], "agent/builder-abc");
134+
135+
expect(mockUpdateTask).toHaveBeenCalled();
136+
});
137+
});

src/handlers/git-merge.ts

Lines changed: 126 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
/**
22
* Handler for the git_merge template action.
33
* Merges the agent's worktree branch into the base branch.
4+
*
5+
* Before merging, attempts a rebase onto the base branch to incorporate
6+
* any commits merged by other agents while this agent was working.
7+
* If the rebase encounters conflicts, the merge is aborted and the
8+
* task is escalated to "blocked" status with conflict details.
49
*/
510

611
import { execSync } from "node:child_process";
@@ -15,14 +20,111 @@ import { resolveRepoRoot } from "../git-ops.js";
1520
/** Module-level mutex to serialize concurrent merge operations */
1621
const mergeLock = new Mutex();
1722

23+
/** Result of attempting to rebase a branch onto the base */
24+
export interface RebaseResult {
25+
success: boolean;
26+
/** Files that had conflicts (only populated on failure) */
27+
conflictedFiles: string[];
28+
}
29+
30+
/**
31+
* Attempt to rebase the worktree branch onto the base branch.
32+
* If conflicts are detected, aborts the rebase and returns the conflicted file list.
33+
*/
34+
export function rebaseOntoBase(
35+
repoDir: string,
36+
worktreeBranch: string,
37+
baseBranch: string,
38+
exec: typeof execSync = execSync
39+
): RebaseResult {
40+
try {
41+
// Switch to the worktree branch for rebasing
42+
exec(`git checkout "${worktreeBranch}"`, { cwd: repoDir, stdio: "pipe" });
43+
exec(`git rebase "${baseBranch}"`, { cwd: repoDir, stdio: "pipe" });
44+
return { success: true, conflictedFiles: [] };
45+
} catch (err) {
46+
// Rebase failed — extract conflicted files before aborting
47+
let conflictedFiles: string[] = [];
48+
try {
49+
const diffOutput = exec(
50+
"git diff --name-only --diff-filter=U",
51+
{ cwd: repoDir, stdio: "pipe" }
52+
).toString().trim();
53+
conflictedFiles = diffOutput.split("\n").filter(Boolean);
54+
} catch {
55+
// Could not list conflicts — still need to abort
56+
}
57+
58+
// Abort the rebase to leave the repo in a clean state
59+
try {
60+
exec("git rebase --abort", { cwd: repoDir, stdio: "pipe" });
61+
} catch {
62+
// Already clean or no rebase in progress
63+
}
64+
65+
return { success: false, conflictedFiles };
66+
}
67+
}
68+
69+
/**
70+
* Escalate a merge conflict: update task to blocked, record conflict details,
71+
* and notify connected clients via WebSocket.
72+
*/
73+
export async function escalateConflict(
74+
ctx: ActionContext,
75+
conflictedFiles: string[],
76+
worktreeBranch: string
77+
): Promise<void> {
78+
const fileList = conflictedFiles.length > 0
79+
? conflictedFiles.join(", ")
80+
: "(unknown files)";
81+
const comment = `Merge conflict on ${worktreeBranch}: ${fileList}`;
82+
83+
ui.warn(`[git_merge] Conflict detected — ${comment}`);
84+
85+
// Update task status to blocked with conflict info
86+
try {
87+
await ctx.api.updateTask(ctx.task.id, {
88+
status: "blocked",
89+
review_comment: comment,
90+
} as unknown as Parameters<typeof ctx.api.updateTask>[1]);
91+
} catch {
92+
// Non-fatal — log but continue
93+
ui.warn("[git_merge] Failed to update task status to blocked");
94+
}
95+
96+
// Broadcast via WS so dashboard shows the conflict immediately
97+
if (ctx.onDataUpdate) {
98+
ctx.onDataUpdate("task", ctx.task.id, {
99+
status: "blocked",
100+
review_comment: comment,
101+
});
102+
}
103+
}
104+
105+
/**
106+
* Clean up worktree directory, prune stale worktree refs, and delete the branch.
107+
*/
108+
function cleanupBranch(
109+
repoDir: string,
110+
worktreeDir: string,
111+
worktreeBranch: string,
112+
exec: typeof execSync = execSync
113+
): void {
114+
if (existsSync(worktreeDir)) {
115+
try { rmSync(worktreeDir, { recursive: true, force: true }); } catch { /* non-fatal */ }
116+
}
117+
try { exec("git worktree prune", { cwd: repoDir, stdio: "pipe" }); } catch { /* non-fatal */ }
118+
try { exec(`git branch -D "${worktreeBranch}"`, { cwd: repoDir, stdio: "pipe" }); } catch { /* non-fatal */ }
119+
}
120+
18121
export async function handleGitMerge(
19122
action: TemplateAction,
20123
ctx: ActionContext,
21124
phase: "pre" | "post"
22125
): Promise<void> {
23126
const label = action.label ?? "git_merge";
24127
const gitExec = execSync;
25-
const gitExists = existsSync;
26128
const gitJoin = join;
27129
// workingDir is the worktree path — we need the main repo root
28130
const worktreePath = ctx.config.workingDir;
@@ -55,12 +157,7 @@ export async function handleGitMerge(
55157
if (!agentCommits) {
56158
ui.warn(`[${phase}] ${label}: no agent commits on ${worktreeBranch} — skipping merge`);
57159
ctx.mergeSkipped = true;
58-
// Clean up the empty branch
59-
if (gitExists(worktreeDir)) {
60-
try { rmSync(worktreeDir, { recursive: true, force: true }); } catch { /* non-fatal */ }
61-
}
62-
try { gitExec("git worktree prune", { cwd: repoDir, stdio: "pipe" }); } catch { /* non-fatal */ }
63-
try { gitExec(`git branch -D "${worktreeBranch}"`, { cwd: repoDir, stdio: "pipe" }); } catch { /* non-fatal */ }
160+
cleanupBranch(repoDir, worktreeDir, worktreeBranch);
64161
return;
65162
}
66163

@@ -78,27 +175,36 @@ export async function handleGitMerge(
78175
if (meaningfulFiles.length === 0) {
79176
ui.warn(`[${phase}] ${label}: only metadata files changed (${diffFiles.join(", ")}) — skipping merge`);
80177
ctx.mergeSkipped = true;
81-
if (gitExists(worktreeDir)) {
82-
try { rmSync(worktreeDir, { recursive: true, force: true }); } catch { /* non-fatal */ }
83-
}
84-
try { gitExec("git worktree prune", { cwd: repoDir, stdio: "pipe" }); } catch { /* non-fatal */ }
85-
try { gitExec(`git branch -D "${worktreeBranch}"`, { cwd: repoDir, stdio: "pipe" }); } catch { /* non-fatal */ }
178+
cleanupBranch(repoDir, worktreeDir, worktreeBranch);
86179
return;
87180
}
88181

89182
ui.info(`[${phase}] ${label}: ${agentCommits.split("\n").length} commit(s), ${meaningfulFiles.length} file(s)`);
183+
184+
// ── Rebase onto base branch before merging ──
185+
// This incorporates any commits merged by other agents while this one was working.
186+
// If rebase encounters conflicts, we abort and escalate.
187+
const rebaseResult = rebaseOntoBase(repoDir, worktreeBranch, baseBranch);
188+
if (!rebaseResult.success) {
189+
await escalateConflict(ctx, rebaseResult.conflictedFiles, worktreeBranch);
190+
logError(
191+
CLI_ERR.GIT_MERGE_FAILED,
192+
`Rebase conflict on ${worktreeBranch}: ${rebaseResult.conflictedFiles.join(", ")}`,
193+
{ taskId: ctx.task.id }
194+
);
195+
// Clean up — the branch is back to pre-rebase state (rebase --abort was called)
196+
cleanupBranch(repoDir, worktreeDir, worktreeBranch);
197+
return;
198+
}
199+
90200
// Record pre-merge hash for accurate diff in reviewer
91201
try { ctx.preMergeHash = gitExec("git rev-parse HEAD", { cwd: repoDir, stdio: "pipe" }).toString().trim(); } catch { /* non-fatal */ }
92202
gitExec(`git checkout "${baseBranch}"`, { cwd: repoDir, stdio: "pipe" });
93203
gitExec(`git merge --no-ff "${worktreeBranch}" -m "merge: ${worktreeBranch}"`, { cwd: repoDir, stdio: "pipe" });
94204
ui.info( `[${phase}] ${label}: merged ${worktreeBranch}`);
95205

96206
// Clean up worktree
97-
if (gitExists(worktreeDir)) {
98-
try { rmSync(worktreeDir, { recursive: true, force: true }); } catch { /* non-fatal */ }
99-
}
100-
try { gitExec("git worktree prune", { cwd: repoDir, stdio: "pipe" }); } catch { /* non-fatal */ }
101-
try { gitExec(`git branch -D "${worktreeBranch}"`, { cwd: repoDir, stdio: "pipe" }); } catch { /* non-fatal */ }
207+
cleanupBranch(repoDir, worktreeDir, worktreeBranch);
102208
} else {
103209
ui.info( `[${phase}] ${label}: no agent branch found, skipping`);
104210
}
@@ -107,6 +213,9 @@ export async function handleGitMerge(
107213
logError(CLI_ERR.GIT_MERGE_FAILED, `git_merge failed: ${msg}`, { taskId: ctx.task.id }, mergeErr);
108214
ui.warn(`[template] git_merge failed: ${msg}`);
109215
try { gitExec("git merge --abort", { cwd: repoDir, stdio: "pipe" }); } catch { /* already clean */ }
216+
217+
// Escalate merge failure to blocked status
218+
await escalateConflict(ctx, [], ctx.agentBranch ?? "unknown");
110219
} finally {
111220
release();
112221
}

0 commit comments

Comments
 (0)