Skip to content

Commit 6be535d

Browse files
henrikjeclaude
andcommitted
fix(rebase): consolidate conflict recovery guidance into single clear block
The conflict output duplicated and contradicted itself — mentioning both git and arb continue/abort per-repo, then repeating arb at the bottom. Redesign: per-repo sections show only conflict files, trailing block provides one set of aligned instructions with a muted git reassurance. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 981373d commit 6be535d

7 files changed

Lines changed: 95 additions & 52 deletions

File tree

src/commands/pull.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ export async function runPull(
391391
repo: c.assessment.repo,
392392
stdout: c.stdout,
393393
stderr: c.stderr,
394-
subcommand: c.assessment.pullMode === "rebase" ? ("rebase" as const) : ("merge" as const),
394+
mode: "pull",
395395
})),
396396
);
397397

@@ -411,7 +411,7 @@ export async function runPull(
411411
record.status = "completed";
412412
record.completedAt = new Date().toISOString();
413413
writeOperationRecord(wsDir, record);
414-
} else {
414+
} else if (conflicted.length === 0 && failed.length > 0) {
415415
info("Use 'arb pull --continue' to resume or 'arb pull --abort' to cancel");
416416
}
417417

src/commands/retarget.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ export function registerRetargetCommand(program: Command): void {
360360
repo: c.assessment.repo,
361361
stdout: c.stdout,
362362
stderr: c.stderr,
363-
subcommand: "rebase" as const,
363+
mode: "retarget",
364364
})),
365365
);
366366
const reportCtx = { tty: shouldColor() };
@@ -380,7 +380,6 @@ export function registerRetargetCommand(program: Command): void {
380380
}
381381
} else {
382382
// Conflicts — config NOT updated (this is the bug fix)
383-
info("Use 'arb retarget --continue' to resume or 'arb retarget --abort' to cancel");
384383
}
385384

386385
// Phase 9: summary

src/lib/render/conflict-report.test.ts

Lines changed: 59 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,26 +7,34 @@ describe("buildConflictReport", () => {
77
expect(buildConflictReport([])).toEqual([]);
88
});
99

10-
test("produces gap + message + gap + section for a single entry", () => {
10+
test("produces gap + message + gap + section + trailing guidance for a single entry", () => {
1111
const entries: ConflictEntry[] = [
12-
{ repo: "api", stdout: "CONFLICT (content): Merge conflict in index.ts\n", stderr: "", subcommand: "rebase" },
12+
{ repo: "api", stdout: "CONFLICT (content): Merge conflict in index.ts\n", stderr: "", mode: "rebase" },
1313
];
1414
const nodes = buildConflictReport(entries);
1515

1616
expect(nodes[0]).toEqual({ kind: "gap" });
1717
const msg = nodes[1] as MessageNode;
1818
expect(msg.kind).toBe("message");
19-
expect(msg.text).toBe("1 repo(s) have conflicts:");
19+
expect(msg.text).toBe("1 repo has conflicts:");
2020

2121
expect(nodes[2]).toEqual({ kind: "gap" });
2222
const section = nodes[3] as SectionNode;
2323
expect(section.kind).toBe("section");
2424
expect(section.header.plain).toBe("api");
25+
expect(section.items).toHaveLength(1);
2526
expect(section.items[0]?.spans[0]?.attention).toBe("muted");
2627
expect(section.items[0]?.plain).toBe("CONFLICT (content): Merge conflict in index.ts");
27-
expect(section.items[1]?.plain).toBe("cd api");
28-
expect(section.items[2]?.plain).toBe("# fix conflicts, then: git rebase --continue");
29-
expect(section.items[3]?.plain).toBe("# or to undo: git rebase --abort");
28+
29+
// Trailing guidance
30+
expect(nodes[4]).toEqual({ kind: "gap" });
31+
const cont = nodes[5] as MessageNode;
32+
expect(cont.text).toBe("Fix conflicts, then: arb rebase --continue");
33+
const abort = nodes[6] as MessageNode;
34+
expect(abort.text).toBe("Or to abort: arb rebase --abort");
35+
const git = nodes[7] as MessageNode;
36+
expect(git.level).toBe("muted");
37+
expect(git.text).toContain("git rebase --continue/--abort per repo");
3038
});
3139

3240
test("filters only CONFLICT lines from combined stdout/stderr", () => {
@@ -35,38 +43,67 @@ describe("buildConflictReport", () => {
3543
repo: "web",
3644
stdout: "Applying: abc\nCONFLICT (content): file.ts\nFailed to merge\n",
3745
stderr: "error: could not apply\nCONFLICT (modify/delete): old.ts\n",
38-
subcommand: "merge",
46+
mode: "merge",
3947
},
4048
];
4149
const nodes = buildConflictReport(entries);
4250
const section = nodes[3] as SectionNode;
43-
const conflictItems = section.items.filter((i) => i.spans[0]?.attention === "muted");
44-
expect(conflictItems).toHaveLength(3); // 2 CONFLICT lines + 1 arb guidance line
45-
expect(conflictItems[0]?.plain).toBe("CONFLICT (content): file.ts");
46-
expect(conflictItems[1]?.plain).toBe("CONFLICT (modify/delete): old.ts");
51+
const mutedItems = section.items.filter((i) => i.spans[0]?.attention === "muted");
52+
expect(mutedItems).toHaveLength(2);
53+
expect(mutedItems[0]?.plain).toBe("CONFLICT (content): file.ts");
54+
expect(mutedItems[1]?.plain).toBe("CONFLICT (modify/delete): old.ts");
4755
});
4856

49-
test("uses correct subcommand in recovery instructions", () => {
50-
const mergeEntries: ConflictEntry[] = [{ repo: "lib", stdout: "", stderr: "", subcommand: "merge" }];
57+
test("uses mode for arb command and derives git subcommand", () => {
58+
const mergeEntries: ConflictEntry[] = [{ repo: "lib", stdout: "", stderr: "", mode: "merge" }];
5159
const mergeNodes = buildConflictReport(mergeEntries);
52-
const section = mergeNodes[3] as SectionNode;
53-
expect(section.items.some((i) => i.plain.includes("git merge --continue"))).toBe(true);
54-
expect(section.items.some((i) => i.plain.includes("git merge --abort"))).toBe(true);
60+
const cont = mergeNodes[5] as MessageNode;
61+
expect(cont.text).toContain("arb merge --continue");
62+
const abort = mergeNodes[6] as MessageNode;
63+
expect(abort.text).toContain("arb merge --abort");
64+
const git = mergeNodes[7] as MessageNode;
65+
expect(git.text).toContain("git merge --continue/--abort");
66+
67+
// pull and retarget use git rebase under the hood
68+
const pullEntries: ConflictEntry[] = [{ repo: "lib", stdout: "", stderr: "", mode: "pull" }];
69+
const pullNodes = buildConflictReport(pullEntries);
70+
const pullCont = pullNodes[5] as MessageNode;
71+
expect(pullCont.text).toContain("arb pull --continue");
72+
const pullGit = pullNodes[7] as MessageNode;
73+
expect(pullGit.text).toContain("git rebase --continue/--abort");
74+
75+
const retargetEntries: ConflictEntry[] = [{ repo: "lib", stdout: "", stderr: "", mode: "retarget" }];
76+
const retargetNodes = buildConflictReport(retargetEntries);
77+
const retargetCont = retargetNodes[5] as MessageNode;
78+
expect(retargetCont.text).toContain("arb retarget --continue");
79+
const retargetGit = retargetNodes[7] as MessageNode;
80+
expect(retargetGit.text).toContain("git rebase --continue/--abort");
5581
});
5682

5783
test("produces gap between multiple repo sections", () => {
5884
const entries: ConflictEntry[] = [
59-
{ repo: "api", stdout: "", stderr: "", subcommand: "rebase" },
60-
{ repo: "web", stdout: "", stderr: "", subcommand: "rebase" },
85+
{ repo: "api", stdout: "", stderr: "", mode: "rebase" },
86+
{ repo: "web", stdout: "", stderr: "", mode: "rebase" },
6187
];
6288
const nodes = buildConflictReport(entries);
6389

6490
const msg = nodes[1] as MessageNode;
65-
expect(msg.text).toBe("2 repo(s) have conflicts:");
91+
expect(msg.text).toBe("2 repos have conflicts:");
6692

67-
// Structure: gap, message, gap, section(api), gap, section(web)
93+
// Structure: gap, message, gap, section(api), gap, section(web), gap, message, message, message
6894
const kinds = nodes.map((n) => n.kind);
69-
expect(kinds).toEqual(["gap", "message", "gap", "section", "gap", "section"]);
95+
expect(kinds).toEqual([
96+
"gap",
97+
"message",
98+
"gap",
99+
"section",
100+
"gap",
101+
"section",
102+
"gap",
103+
"message",
104+
"message",
105+
"message",
106+
]);
70107
});
71108
});
72109

@@ -80,7 +117,7 @@ describe("buildStashPopFailureReport", () => {
80117

81118
expect(nodes[0]).toEqual({ kind: "gap" });
82119
const msg = nodes[1] as MessageNode;
83-
expect(msg.text).toBe("1 repo(s) need manual stash application:");
120+
expect(msg.text).toBe("1 repo needs manual stash application:");
84121

85122
expect(nodes[2]).toEqual({ kind: "gap" });
86123
const section = nodes[3] as SectionNode;

src/lib/render/conflict-report.ts

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,14 @@ export interface ConflictEntry {
55
repo: string;
66
stdout: string;
77
stderr: string;
8-
subcommand: "rebase" | "merge";
8+
/** The arb command name: "rebase", "merge", "pull", "retarget" */
9+
mode: string;
910
}
1011

1112
export function buildConflictReport(entries: ConflictEntry[]): OutputNode[] {
1213
if (entries.length === 0) return [];
13-
const nodes: OutputNode[] = [
14-
{ kind: "gap" },
15-
{ kind: "message", level: "default", text: `${entries.length} repo(s) have conflicts:` },
16-
];
14+
const header = entries.length === 1 ? "1 repo has conflicts:" : `${entries.length} repos have conflicts:`;
15+
const nodes: OutputNode[] = [{ kind: "gap" }, { kind: "message", level: "default", text: header }];
1716
for (const e of entries) {
1817
const combined = `${e.stdout}\n${e.stderr}`;
1918
const conflictLines = combined.split("\n").filter((l) => l.startsWith("CONFLICT"));
@@ -22,24 +21,35 @@ export function buildConflictReport(entries: ConflictEntry[]): OutputNode[] {
2221
{
2322
kind: "section",
2423
header: cell(e.repo),
25-
items: [
26-
...conflictLines.map((line) => cell(line, "muted")),
27-
cell(`cd ${e.repo}`),
28-
cell(`# fix conflicts, then: git ${e.subcommand} --continue`),
29-
cell(`# or to undo: git ${e.subcommand} --abort`),
30-
cell(`# or from workspace root: arb ${e.subcommand} --continue / arb ${e.subcommand} --abort`, "muted"),
31-
],
24+
items: conflictLines.map((line) => cell(line, "muted")),
3225
},
3326
);
3427
}
28+
// Safe: guarded by early return when entries is empty
29+
const mode = entries[0]?.mode ?? "rebase";
30+
const gitSub = mode === "merge" ? "merge" : "rebase";
31+
nodes.push(
32+
{ kind: "gap" },
33+
{ kind: "message", level: "default", text: `Fix conflicts, then: arb ${mode} --continue` },
34+
{ kind: "message", level: "default", text: `Or to abort: arb ${mode} --abort` },
35+
{
36+
kind: "message",
37+
level: "muted",
38+
text: ` (or use git ${gitSub} --continue/--abort per repo)`,
39+
},
40+
);
3541
return nodes;
3642
}
3743

3844
export function buildStashPopFailureReport(repos: { repo: string }[], verb: string): OutputNode[] {
3945
if (repos.length === 0) return [];
4046
const nodes: OutputNode[] = [
4147
{ kind: "gap" },
42-
{ kind: "message", level: "default", text: `${repos.length} repo(s) need manual stash application:` },
48+
{
49+
kind: "message",
50+
level: "default",
51+
text: `${repos.length === 1 ? "1 repo needs" : `${repos.length} repos need`} manual stash application:`,
52+
},
4353
];
4454
for (const r of repos) {
4555
nodes.push(

src/lib/sync/continue-flow.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,14 @@ export async function runContinueFlow(params: ContinueFlowParams): Promise<void>
200200

201201
// Step 9: Show conflict details for failures (B2 fix)
202202
if (newConflicts.length > 0) {
203-
const subcommand = mode === "merge" ? ("merge" as const) : ("rebase" as const);
204203
const conflictNodes = buildConflictReport(
205-
newConflicts.map((c) => ({ repo: c.repo, stdout: c.stdout, stderr: c.stderr, subcommand })),
204+
newConflicts.map((c) => ({ repo: c.repo, stdout: c.stdout, stderr: c.stderr, mode })),
206205
);
207206
if (conflictNodes.length > 0) {
208207
process.stderr.write(render(conflictNodes, rCtx));
209208
}
209+
} else if (stillConflicting.length > 0) {
210+
info(`Fix conflicts, then: arb ${mode} --continue`);
210211
}
211212

212213
// Step 10: Finalize
@@ -219,9 +220,6 @@ export async function runContinueFlow(params: ContinueFlowParams): Promise<void>
219220
writeOperationRecord(wsDir, record);
220221
} else {
221222
writeOperationRecord(wsDir, record);
222-
if (newConflicts.length > 0 || stillConflicting.length > 0) {
223-
info(`Use 'arb ${mode} --continue' to resume or 'arb ${mode} --abort' to cancel`);
224-
}
225223
}
226224

227225
// Step 11: Summary

src/lib/sync/integrate.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -304,13 +304,12 @@ export async function integrate(
304304
});
305305

306306
// Consolidated conflict report
307-
const subcommand = mode === "rebase" ? ("rebase" as const) : ("merge" as const);
308307
const conflictNodes = buildConflictReport(
309308
conflicted.map((c) => ({
310309
repo: c.assessment.repo,
311310
stdout: c.stdout,
312311
stderr: c.stderr,
313-
subcommand,
312+
mode,
314313
})),
315314
);
316315

@@ -326,8 +325,6 @@ export async function integrate(
326325
record.status = "completed";
327326
record.completedAt = new Date().toISOString();
328327
writeOperationRecord(wsDir, record);
329-
} else {
330-
info(`Use 'arb ${mode} --continue' to resume or 'arb ${mode} --abort' to cancel`);
331328
}
332329

333330
// Update config after stale base fallback

test/integration/integrate.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,9 @@ describe("rebase", () => {
6969
expect(result.output).toContain("CONFLICT");
7070
expect(result.output).toContain("conflict.txt");
7171
expect(result.output).toContain("conflict");
72-
expect(result.output).toContain("git rebase --continue");
73-
expect(result.output).toContain("git rebase --abort");
72+
expect(result.output).toContain("arb rebase --continue");
73+
expect(result.output).toContain("arb rebase --abort");
74+
expect(result.output).toContain("git rebase --continue/--abort per repo");
7475
expect(result.output).toContain("Rebased 1 repo, 1 conflicted");
7576
}));
7677

@@ -207,8 +208,9 @@ describe("merge", () => {
207208
expect(result.output).toContain("CONFLICT");
208209
expect(result.output).toContain("conflict.txt");
209210
expect(result.output).toContain("conflict");
210-
expect(result.output).toContain("git merge --continue");
211-
expect(result.output).toContain("git merge --abort");
211+
expect(result.output).toContain("arb merge --continue");
212+
expect(result.output).toContain("arb merge --abort");
213+
expect(result.output).toContain("git merge --continue/--abort per repo");
212214
expect(result.output).toContain("Merged 1 repo, 1 conflicted");
213215
}));
214216
});

0 commit comments

Comments
 (0)