From cf0fe6c57172c8a97628b0615ad89557d6743b01 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 25 Mar 2026 00:36:06 -0400 Subject: [PATCH] fix: stop auto loop on real code merge conflicts (#2330) (#2428) 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) --- src/resources/extensions/gsd/auto/phases.ts | 42 +++++++++++- .../tests/merge-conflict-stops-loop.test.ts | 66 +++++++++++++++++++ .../extensions/gsd/worktree-resolver.ts | 7 ++ 3 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 src/resources/extensions/gsd/tests/merge-conflict-stops-loop.test.ts diff --git a/src/resources/extensions/gsd/auto/phases.ts b/src/resources/extensions/gsd/auto/phases.ts index 0b4e276ad..0008db09b 100644 --- a/src/resources/extensions/gsd/auto/phases.ts +++ b/src/resources/extensions/gsd/auto/phases.ts @@ -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) } diff --git a/src/resources/extensions/gsd/tests/merge-conflict-stops-loop.test.ts b/src/resources/extensions/gsd/tests/merge-conflict-stops-loop.test.ts new file mode 100644 index 000000000..5afca834c --- /dev/null +++ b/src/resources/extensions/gsd/tests/merge-conflict-stops-loop.test.ts @@ -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(); diff --git a/src/resources/extensions/gsd/worktree-resolver.ts b/src/resources/extensions/gsd/worktree-resolver.ts index dceb4ed26..093899297 100644 --- a/src/resources/extensions/gsd/worktree-resolver.ts +++ b/src/resources/extensions/gsd/worktree-resolver.ts @@ -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