MergeConflictError from squash merge was caught silently in worktree-resolver's mergeAndExit, so the auto loop retried the merge forever. Now: 1. worktree-resolver re-throws MergeConflictError after cleanup 2. auto/phases.ts catches it at all 3 mergeAndExit call sites 3. On conflict, stops the loop with a clear error message Fixes #2330 Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
df269b3b00
commit
cf0fe6c571
3 changed files with 112 additions and 3 deletions
|
|
@ -27,6 +27,7 @@ import { debugLog } from "../debug-logger.js";
|
|||
import { gsdRoot } from "../paths.js";
|
||||
import { atomicWriteSync } from "../atomic-write.js";
|
||||
import { PROJECT_FILES } from "../detection.js";
|
||||
import { MergeConflictError } from "../git-service.js";
|
||||
import { join } from "node:path";
|
||||
import { existsSync, cpSync } from "node:fs";
|
||||
|
||||
|
|
@ -234,7 +235,20 @@ export async function runPreDispatch(
|
|||
loopState.stuckRecoveryAttempts = 0;
|
||||
|
||||
// Worktree lifecycle on milestone transition — merge current, enter next
|
||||
deps.resolver.mergeAndExit(s.currentMilestoneId!, ctx.ui);
|
||||
try {
|
||||
deps.resolver.mergeAndExit(s.currentMilestoneId!, ctx.ui);
|
||||
} catch (mergeErr) {
|
||||
if (mergeErr instanceof MergeConflictError) {
|
||||
// Real code conflicts — stop the loop instead of retrying forever (#2330)
|
||||
ctx.ui.notify(
|
||||
`Merge conflict: ${mergeErr.conflictedFiles.join(", ")}. Resolve conflicts manually and run /gsd auto to resume.`,
|
||||
"error",
|
||||
);
|
||||
await deps.stopAuto(ctx, pi, `Merge conflict on milestone ${s.currentMilestoneId}`);
|
||||
return { action: "break", reason: "merge-conflict" };
|
||||
}
|
||||
// Non-conflict errors — log and continue
|
||||
}
|
||||
|
||||
// PR creation (auto_pr) is handled inside mergeMilestoneToMain (#2302)
|
||||
|
||||
|
|
@ -315,7 +329,18 @@ export async function runPreDispatch(
|
|||
if (incomplete.length === 0 && state.registry.length > 0) {
|
||||
// All milestones complete — merge milestone branch before stopping
|
||||
if (s.currentMilestoneId) {
|
||||
deps.resolver.mergeAndExit(s.currentMilestoneId, ctx.ui);
|
||||
try {
|
||||
deps.resolver.mergeAndExit(s.currentMilestoneId, ctx.ui);
|
||||
} catch (mergeErr) {
|
||||
if (mergeErr instanceof MergeConflictError) {
|
||||
ctx.ui.notify(
|
||||
`Merge conflict: ${mergeErr.conflictedFiles.join(", ")}. Resolve conflicts manually and run /gsd auto to resume.`,
|
||||
"error",
|
||||
);
|
||||
await deps.stopAuto(ctx, pi, `Merge conflict on milestone ${s.currentMilestoneId}`);
|
||||
return { action: "break", reason: "merge-conflict" };
|
||||
}
|
||||
}
|
||||
|
||||
// PR creation (auto_pr) is handled inside mergeMilestoneToMain (#2302)
|
||||
}
|
||||
|
|
@ -399,7 +424,18 @@ export async function runPreDispatch(
|
|||
if (state.phase === "complete") {
|
||||
// Milestone merge on complete (before closeout so branch state is clean)
|
||||
if (s.currentMilestoneId) {
|
||||
deps.resolver.mergeAndExit(s.currentMilestoneId, ctx.ui);
|
||||
try {
|
||||
deps.resolver.mergeAndExit(s.currentMilestoneId, ctx.ui);
|
||||
} catch (mergeErr) {
|
||||
if (mergeErr instanceof MergeConflictError) {
|
||||
ctx.ui.notify(
|
||||
`Merge conflict: ${mergeErr.conflictedFiles.join(", ")}. Resolve conflicts manually and run /gsd auto to resume.`,
|
||||
"error",
|
||||
);
|
||||
await deps.stopAuto(ctx, pi, `Merge conflict on milestone ${s.currentMilestoneId}`);
|
||||
return { action: "break", reason: "merge-conflict" };
|
||||
}
|
||||
}
|
||||
|
||||
// PR creation (auto_pr) is handled inside mergeMilestoneToMain (#2302)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,66 @@
|
|||
/**
|
||||
* merge-conflict-stops-loop.test.ts — #2330
|
||||
*
|
||||
* When a squash merge has real code conflicts (not just .gsd/ files),
|
||||
* the merge retries forever because MergeConflictError is caught
|
||||
* silently in mergeAndExit. This test verifies that:
|
||||
* 1. worktree-resolver re-throws MergeConflictError for code conflicts
|
||||
* 2. auto/phases.ts wraps mergeAndExit calls to stop the loop on conflict
|
||||
*/
|
||||
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { createTestContext } from "./test-helpers.ts";
|
||||
|
||||
const { assertTrue, report } = createTestContext();
|
||||
|
||||
const resolverPath = join(import.meta.dirname, "..", "worktree-resolver.ts");
|
||||
const resolverSrc = readFileSync(resolverPath, "utf-8");
|
||||
|
||||
const phasesPath = join(import.meta.dirname, "..", "auto", "phases.ts");
|
||||
const phasesSrc = readFileSync(phasesPath, "utf-8");
|
||||
|
||||
console.log("\n=== #2330: Merge conflict stops auto loop ===");
|
||||
|
||||
// ── Test 1: worktree-resolver re-throws MergeConflictError ──────────────
|
||||
|
||||
const methodStart = resolverSrc.indexOf("Worktree-mode merge:");
|
||||
assertTrue(methodStart > 0, "worktree-resolver has _mergeWorktreeMode method");
|
||||
|
||||
const methodBody = resolverSrc.slice(methodStart, methodStart + 5000);
|
||||
const rethrowsConflict =
|
||||
methodBody.includes("MergeConflictError") &&
|
||||
methodBody.includes("throw err");
|
||||
|
||||
assertTrue(
|
||||
rethrowsConflict,
|
||||
"worktree-resolver._mergeWorktreeMode re-throws MergeConflictError (#2330)",
|
||||
);
|
||||
|
||||
// ── Test 2: auto/phases.ts imports and uses MergeConflictError ──────────
|
||||
|
||||
assertTrue(
|
||||
phasesSrc.includes("MergeConflictError") && phasesSrc.includes("mergeAndExit"),
|
||||
"auto/phases.ts handles MergeConflictError from mergeAndExit (#2330)",
|
||||
);
|
||||
|
||||
// ── Test 3: The handler stops the loop (doesn't just warn) ──────────────
|
||||
|
||||
// Find the instanceof MergeConflictError check (not the import line)
|
||||
const instanceofIdx = phasesSrc.indexOf("instanceof MergeConflictError");
|
||||
assertTrue(instanceofIdx > 0, "auto/phases.ts has instanceof MergeConflictError check");
|
||||
|
||||
if (instanceofIdx > 0) {
|
||||
const afterHandler = phasesSrc.slice(instanceofIdx, instanceofIdx + 500);
|
||||
const stopsLoop =
|
||||
afterHandler.includes("stopAuto") ||
|
||||
afterHandler.includes('action: "break"') ||
|
||||
afterHandler.includes("reason: \"merge-conflict\"");
|
||||
|
||||
assertTrue(
|
||||
stopsLoop,
|
||||
"auto/phases.ts stops the loop when merge conflict is detected (#2330)",
|
||||
);
|
||||
}
|
||||
|
||||
report();
|
||||
|
|
@ -17,6 +17,7 @@ import { existsSync, unlinkSync } from "node:fs";
|
|||
import { join } from "node:path";
|
||||
import type { AutoSession } from "./auto/session.js";
|
||||
import { debugLog } from "./debug-logger.js";
|
||||
import { MergeConflictError } from "./git-service.js";
|
||||
|
||||
// ─── Dependency Interface ──────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -433,6 +434,12 @@ export class WorktreeResolver {
|
|||
/* best-effort */
|
||||
}
|
||||
}
|
||||
|
||||
// Re-throw MergeConflictError so the auto loop can detect real code
|
||||
// conflicts and stop instead of retrying forever (#2330).
|
||||
if (err instanceof MergeConflictError) {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
// Always restore basePath and rebuild — whether merge succeeded or failed
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue