fix: detect and preserve submodule state during worktree teardown (#2337) (#2425)

Worktree teardown with --force destroyed uncommitted changes in
submodule directories. Now detects .gitmodules, checks submodule
status for uncommitted changes, and stashes them before removal.
When submodules have dirty state, attempts non-force removal first.

Fixes #2337

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

src/resources/extensions/gsd/tests/worktree-submodule-safety.test.ts

@@ -0,0 +1,65 @@
+/**
+ * worktree-submodule-safety.test.ts — #2337
+ *
+ * Worktree teardown (removeWorktree) uses --force which destroys
+ * uncommitted changes in submodule directories. This test verifies
+ * that the removal logic detects submodules and preserves their state.
+ */
+
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+import { createTestContext } from "./test-helpers.ts";
+
+const { assertTrue, report } = createTestContext();
+
+const srcPath = join(import.meta.dirname, "..", "worktree-manager.ts");
+const src = readFileSync(srcPath, "utf-8");
+
+console.log("\n=== #2337: Worktree teardown preserves submodule state ===");
+
+// ── Test 1: removeWorktree function exists ──────────────────────────────
+
+const removeWorktreeIdx = src.indexOf("export function removeWorktree");
+assertTrue(removeWorktreeIdx > 0, "worktree-manager.ts exports removeWorktree");
+
+const fnBody = src.slice(removeWorktreeIdx, removeWorktreeIdx + 3000);
+
+// ── Test 2: The function checks for submodules before force removal ─────
+
+const checksSubmodules =
+  fnBody.includes("submodule") ||
+  fnBody.includes(".gitmodules");
+
+assertTrue(
+  checksSubmodules,
+  "removeWorktree checks for submodules before force removal (#2337)",
+);
+
+// ── Test 3: Submodule changes are stashed or warned about ───────────────
+
+const preservesSubmoduleState =
+  fnBody.includes("stash") ||
+  fnBody.includes("uncommitted") ||
+  fnBody.includes("dirty") ||
+  fnBody.includes("submodule") && (fnBody.includes("warn") || fnBody.includes("preserv"));
+
+assertTrue(
+  preservesSubmoduleState,
+  "removeWorktree preserves or warns about submodule uncommitted changes (#2337)",
+);
+
+// ── Test 4: Force removal is skipped when submodules have changes ───────
+
+// The key fix: when submodules have dirty state, we should NOT use force
+// removal. Instead, use non-force first and fall back to force only after
+// submodule state is preserved.
+const hasConditionalForce =
+  fnBody.includes("submodule") &&
+  (fnBody.includes("force") || fnBody.includes("--force"));
+
+assertTrue(
+  hasConditionalForce,
+  "removeWorktree has conditional force logic around submodules (#2337)",
+);
+
+report();

src/resources/extensions/gsd/worktree-manager.ts

@@ -16,6 +16,7 @@
  */
 
 import { existsSync, mkdirSync, readFileSync, realpathSync, rmSync } from "node:fs";
+import { execFileSync } from "node:child_process";
 import { join, resolve, sep } from "node:path";
 import { GSDError, GSD_PARSE_ERROR, GSD_STALE_STATE, GSD_LOCK_HELD, GSD_GIT_ERROR, GSD_MERGE_CONFLICT } from "./errors.js";
 import {
@@ -321,8 +322,48 @@ export function removeWorktree(
     return;
   }
 
-  // Remove worktree using the resolved path (force if requested, to handle dirty worktrees)
-  try { nativeWorktreeRemove(basePath, resolvedWtPath, force); } catch { /* may fail */ }
+  // Submodule safety (#2337): detect submodules with uncommitted changes
+  // before force-removing the worktree. Force removal destroys all uncommitted
+  // state, which is especially destructive for submodule directories.
+  let hasSubmoduleChanges = false;
+  const gitmodulesPath = join(resolvedWtPath, ".gitmodules");
+  if (existsSync(gitmodulesPath)) {
+    try {
+      const submoduleStatus = execFileSync(
+        "git", ["submodule", "status"], 
+        { cwd: resolvedWtPath, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" },
+      ).trim();
+      // Lines starting with '+' indicate uncommitted submodule changes
+      hasSubmoduleChanges = submoduleStatus.split("\n").some(
+        (line: string) => line.startsWith("+") || line.startsWith("-"),
+      );
+      if (hasSubmoduleChanges) {
+        // Stash submodule changes so they are not lost during force removal.
+        // The stash is created in the worktree before it's torn down.
+        try {
+          execFileSync(
+            "git", ["stash", "push", "-m", "gsd: auto-stash submodule changes before worktree teardown"],
+            { cwd: resolvedWtPath, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" },
+          );
+          process.stderr.write(
+            `[GSD] WARNING: Stashed uncommitted submodule changes in ${resolvedWtPath} before worktree teardown.\n`,
+          );
+        } catch {
+          // Stash failed — warn the user that submodule changes may be lost
+          process.stderr.write(
+            `[GSD] WARNING: Submodule changes detected in ${resolvedWtPath} — stash failed, changes may be lost during force removal.\n`,
+          );
+        }
+      }
+    } catch {
+      // submodule status failed — proceed with normal removal
+    }
+  }
+
+  // Remove worktree: try non-force first when submodules have changes,
+  // falling back to force only after submodule state has been preserved.
+  const useForce = hasSubmoduleChanges ? false : force;
+  try { nativeWorktreeRemove(basePath, resolvedWtPath, useForce); } catch { /* may fail */ }
 
   // If the directory is still there (e.g. locked), try harder with force
   if (existsSync(resolvedWtPath)) {