diff --git a/src/resources/extensions/gsd/auto-worktree.ts b/src/resources/extensions/gsd/auto-worktree.ts index e20b2a80c..33dc2c514 100644 --- a/src/resources/extensions/gsd/auto-worktree.ts +++ b/src/resources/extensions/gsd/auto-worktree.ts @@ -984,20 +984,32 @@ export function mergeMilestoneToMain( } // 10. Remove worktree directory first (must happen before branch deletion) - try { - removeWorktree(originalBasePath_, milestoneId, { - branch: null as unknown as string, - deleteBranch: false, - }); - } catch { - // Best-effort -- worktree dir may already be gone - } + // ONLY when a commit was actually produced — if nativeCommit returned null + // (nothing to commit), tearing down the worktree would destroy source code + // that was never merged (#1672). + if (nothingToCommit) { + // eslint-disable-next-line no-console + console.warn( + `[GSD] Warning: squash merge of ${milestoneBranch} produced nothing to commit. ` + + "Worktree and branch preserved to prevent data loss. " + + "Inspect the worktree manually and retry.", + ); + } else { + try { + removeWorktree(originalBasePath_, milestoneId, { + branch: null as unknown as string, + deleteBranch: false, + }); + } catch { + // Best-effort -- worktree dir may already be gone + } - // 11. Delete milestone branch (after worktree removal so ref is unlocked) - try { - nativeBranchDelete(originalBasePath_, milestoneBranch); - } catch { - // Best-effort + // 11. Delete milestone branch (after worktree removal so ref is unlocked) + try { + nativeBranchDelete(originalBasePath_, milestoneBranch); + } catch { + // Best-effort + } } // 12. Clear module state diff --git a/src/resources/extensions/gsd/native-git-bridge.ts b/src/resources/extensions/gsd/native-git-bridge.ts index d091da965..bd4ae4b68 100644 --- a/src/resources/extensions/gsd/native-git-bridge.ts +++ b/src/resources/extensions/gsd/native-git-bridge.ts @@ -835,11 +835,18 @@ export function nativeMergeSquash(basePath: string, branch: string): GitMergeRes encoding: "utf-8", }); return { success: true, conflicts: [] }; - } catch { - // Check for conflicts + } catch (err: unknown) { + // Check for conflicts — only treat as recoverable if actual conflict + // markers are present. Other failures (bad ref, corrupt repo, etc.) + // must propagate so callers don't assume the merge succeeded (#1672). const conflictOutput = gitExec(basePath, ["diff", "--name-only", "--diff-filter=U"], true); const conflicts = conflictOutput ? conflictOutput.split("\n").filter(Boolean) : []; - return { success: conflicts.length === 0, conflicts }; + if (conflicts.length > 0) { + return { success: false, conflicts }; + } + // No conflicts detected — this is a non-conflict failure; re-throw + // so the caller knows the merge did not succeed. + throw err; } } diff --git a/src/resources/extensions/gsd/tests/auto-worktree-milestone-merge.test.ts b/src/resources/extensions/gsd/tests/auto-worktree-milestone-merge.test.ts index 30fd9a7e4..218750e62 100644 --- a/src/resources/extensions/gsd/tests/auto-worktree-milestone-merge.test.ts +++ b/src/resources/extensions/gsd/tests/auto-worktree-milestone-merge.test.ts @@ -17,6 +17,7 @@ import { getAutoWorktreeOriginalBase, } from "../auto-worktree.ts"; import { getSliceBranchName } from "../worktree.ts"; +import { nativeMergeSquash } from "../native-git-bridge.ts"; import { createTestContext } from "./test-helpers.ts"; @@ -389,6 +390,90 @@ async function main(): Promise { assertTrue(!branches.includes("milestone/M070"), "milestone branch deleted after merge"); } + // ─── Test 8: Worktree preserved when commit is empty (#1672) ────── + console.log("\n=== worktree preserved when commit is empty (#1672) ==="); + { + const repo = freshRepo(); + const wtPath = createAutoWorktree(repo, "M080"); + + // Do NOT add any slices/changes — milestone branch is identical to main. + // This simulates the WSL stat-cache bug where autoCommitCurrentBranch + // skips commits, leaving the milestone branch identical to main. + const roadmap = makeRoadmap("M080", "Empty milestone", []); + + // Capture console.warn to verify the warning is emitted + const warnings: string[] = []; + const origWarn = console.warn; + console.warn = (...args: unknown[]) => { + warnings.push(args.map(String).join(" ")); + }; + + try { + mergeMilestoneToMain(repo, "M080", roadmap); + } finally { + console.warn = origWarn; + } + + // Milestone branch must still exist (not deleted) + const branches = run("git branch", repo); + assertTrue( + branches.includes("milestone/M080"), + "milestone branch preserved when nothing was committed (#1672)", + ); + + // A warning should have been emitted + assertTrue( + warnings.some((w) => w.includes("nothing to commit")), + "emits warning about empty merge (#1672)", + ); + } + + // ─── Test 9: Worktree removed when commit succeeds (#1672) ────── + console.log("\n=== worktree removed when commit succeeds (#1672) ==="); + { + const repo = freshRepo(); + const wtPath = createAutoWorktree(repo, "M090"); + + addSliceToMilestone(repo, wtPath, "M090", "S01", "Teardown test", [ + { file: "teardown.ts", content: "export const teardown = true;\n", message: "add teardown file" }, + ]); + + const roadmap = makeRoadmap("M090", "Teardown verification", [ + { id: "S01", title: "Teardown test" }, + ]); + + mergeMilestoneToMain(repo, "M090", roadmap); + + // Milestone branch must be deleted + const branches = run("git branch", repo); + assertTrue( + !branches.includes("milestone/M090"), + "milestone branch deleted after successful commit (#1672)", + ); + + // Worktree directory must be removed + const worktreeDir = join(repo, ".gsd", "worktrees", "M090"); + assertTrue(!existsSync(worktreeDir), "worktree directory removed after successful commit (#1672)"); + + // File should be on main + assertTrue(existsSync(join(repo, "teardown.ts")), "teardown.ts merged to main (#1672)"); + } + + // ─── Test 10: nativeMergeSquash throws on non-conflict failures (#1672) ─ + console.log("\n=== nativeMergeSquash throws on non-conflict failures (#1672) ==="); + { + const repo = freshRepo(); + + // Merge a nonexistent branch — a non-conflict failure that must throw + let threw = false; + try { + nativeMergeSquash(repo, "nonexistent-branch"); + } catch { + threw = true; + } + assertTrue(threw, "nativeMergeSquash throws on nonexistent branch (#1672)"); + } + } finally { process.chdir(savedCwd); for (const d of tempDirs) {