fix: guard worktree teardown on empty merge to prevent data loss (#1672) (#1755)

When nativeCommit returns null (nothing to commit), the worktree directory
and milestone branch are now preserved instead of unconditionally deleted.
This prevents data loss on WSL where git's stat cache can cause
autoCommitCurrentBranch to skip commits.

Additionally, nativeMergeSquash now re-throws non-conflict git failures
(bad ref, corrupt repo) instead of masking them as { success: true }.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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

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;
   }
 }
 

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<void> {
       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) {