From fa8e5500ac44c29eee500bc17048a518cf9da16f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 25 Mar 2026 02:05:39 -0400 Subject: [PATCH] fix(auto-start): handle survivor branch recovery in phase=complete (#2358) (#2427) When bootstrapAutoSession finds a survivor milestone branch and the derived state phase is "complete", recovery was skipped entirely because the survivor branch detection only triggered for phase === "pre-planning". This left the milestone worktree/branch alive and routed bootstrap into showSmartEntry instead of running finalization (merge, cleanup). Changes: - Broaden survivor branch detection to also check phase === "complete" - Add explicit finalization path: when hasSurvivorBranch && phase === "complete", call resolver.mergeAndExit() to run the pending merge and worktree cleanup, then re-derive state so the normal flow continues - After finalization, clear hasSurvivorBranch so the "all milestones complete" or "next milestone" path runs correctly Fixes #2358 Co-authored-by: Claude Opus 4.6 (1M context) --- src/resources/extensions/gsd/auto-start.ts | 27 ++++- .../tests/survivor-branch-complete.test.ts | 108 ++++++++++++++++++ 2 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 src/resources/extensions/gsd/tests/survivor-branch-complete.test.ts diff --git a/src/resources/extensions/gsd/auto-start.ts b/src/resources/extensions/gsd/auto-start.ts index c63f0c5cb..4963f962c 100644 --- a/src/resources/extensions/gsd/auto-start.ts +++ b/src/resources/extensions/gsd/auto-start.ts @@ -297,11 +297,14 @@ export async function bootstrapAutoSession( } } - // Milestone branch recovery (#601) + // Milestone branch recovery (#601, #2358) + // Detect survivor milestone branches in both pre-planning and complete phases. + // In phase=complete, the milestone artifacts exist but finalization (merge, + // worktree cleanup) was never run — the survivor branch must be merged. let hasSurvivorBranch = false; if ( state.activeMilestone && - state.phase === "pre-planning" && + (state.phase === "pre-planning" || state.phase === "complete") && shouldUseWorktreeIsolation() && !detectWorktreeName(base) && !base.includes(`${pathSep}.gsd${pathSep}worktrees${pathSep}`) @@ -343,6 +346,26 @@ export async function bootstrapAutoSession( } } + // Survivor branch exists and milestone is complete (#2358): + // The milestone artifacts were written but finalization (merge, worktree + // cleanup) never ran. Run mergeAndExit to finalize, then re-derive state + // so the normal "all milestones complete" or "next milestone" path runs. + if (hasSurvivorBranch && state.phase === "complete") { + const mid = state.activeMilestone!.id; + ctx.ui.notify( + `Milestone ${mid} is complete but branch/worktree was not finalized. Running merge now.`, + "info", + ); + const resolver = buildResolver(); + resolver.mergeAndExit(mid, { + notify: ctx.ui.notify.bind(ctx.ui), + }); + invalidateAllCaches(); + state = await deriveState(base); + // Clear survivor flag — finalization is done + hasSurvivorBranch = false; + } + if (!hasSurvivorBranch) { // No active work — start a new milestone via discuss flow if (!state.activeMilestone || state.phase === "complete") { diff --git a/src/resources/extensions/gsd/tests/survivor-branch-complete.test.ts b/src/resources/extensions/gsd/tests/survivor-branch-complete.test.ts new file mode 100644 index 000000000..0d6fe66a4 --- /dev/null +++ b/src/resources/extensions/gsd/tests/survivor-branch-complete.test.ts @@ -0,0 +1,108 @@ +/** + * Regression test for #2358: Survivor branch recovery skipped in phase=complete. + * + * When bootstrapAutoSession finds a survivor milestone branch and the derived + * state phase is "complete", recovery/finalization is skipped entirely because + * the survivor branch detection only triggers when phase === "pre-planning". + * The milestone finalization (merge, cleanup) never runs, leaving the worktree + * and branch alive. + * + * The fix broadens the survivor branch detection to also check phase === "complete", + * and adds a finalization path that runs mergeAndExit before falling through to + * the normal "complete" handling. + */ + +import { createTestContext } from "./test-helpers.ts"; + +const { assertTrue, assertEq, report } = createTestContext(); + +// ═══ Test: survivor branch detection conditions ══════════════════════════════ + +// The survivor branch detection block in auto-start.ts checks: +// state.activeMilestone && +// state.phase === "pre-planning" && // <-- BUG: too restrictive +// shouldUseWorktreeIsolation() && +// !detectWorktreeName(base) && +// !base.includes(...) +// +// The fix should also include state.phase === "complete". + +{ + console.log("\n=== #2358: survivor branch should be detected in phase=complete ==="); + + // Simulate the condition check before the fix (only pre-planning) + const phasesBeforeFix = ["pre-planning"]; + const phasesAfterFix = ["pre-planning", "complete"]; + + const testPhase = "complete"; + + const detectedBefore = phasesBeforeFix.includes(testPhase); + assertEq(detectedBefore, false, "before fix: phase=complete should NOT trigger survivor detection"); + + const detectedAfter = phasesAfterFix.includes(testPhase); + assertEq(detectedAfter, true, "after fix: phase=complete SHOULD trigger survivor detection"); +} + +// ═══ Test: pre-planning survivor detection still works ═══════════════════════ + +{ + console.log("\n=== #2358: pre-planning survivor detection is not broken ==="); + + const phasesAfterFix = ["pre-planning", "complete"]; + const testPhase = "pre-planning"; + + const detected = phasesAfterFix.includes(testPhase); + assertEq(detected, true, "pre-planning should still trigger survivor detection after fix"); +} + +// ═══ Test: other phases do NOT trigger survivor detection ════════════════════ + +{ + console.log("\n=== #2358: other phases should NOT trigger survivor detection ==="); + + const phasesAfterFix = ["pre-planning", "complete"]; + + for (const phase of ["planning", "executing", "blocked", "needs-discussion"]) { + const detected = phasesAfterFix.includes(phase); + assertEq(detected, false, `phase=${phase} should NOT trigger survivor detection`); + } +} + +// ═══ Test: phase=complete + hasSurvivorBranch should trigger finalization ═════ + +{ + console.log("\n=== #2358: phase=complete + survivor branch triggers finalization path ==="); + + // Simulate the decision logic after the fix: + // if (hasSurvivorBranch && state.phase === "complete") -> finalize + // if (hasSurvivorBranch && state.phase === "needs-discussion") -> discuss + // if (!hasSurvivorBranch && state.phase === "complete") -> showSmartEntry + + const scenarios = [ + { hasSurvivorBranch: true, phase: "complete", expected: "finalize" }, + { hasSurvivorBranch: true, phase: "needs-discussion", expected: "discuss" }, + { hasSurvivorBranch: true, phase: "pre-planning", expected: "continue" }, + { hasSurvivorBranch: false, phase: "complete", expected: "showSmartEntry" }, + ]; + + for (const { hasSurvivorBranch, phase, expected } of scenarios) { + let result: string; + if (hasSurvivorBranch && phase === "complete") { + result = "finalize"; + } else if (hasSurvivorBranch && phase === "needs-discussion") { + result = "discuss"; + } else if (!hasSurvivorBranch && (!phase || phase === "complete")) { + result = "showSmartEntry"; + } else { + result = "continue"; + } + + assertEq( + result, + expected, + `hasSurvivorBranch=${hasSurvivorBranch}, phase=${phase} -> expected ${expected}, got ${result}`, + ); + } +} + +report();