From 9384641b2588fbfdc8fdd40423bfdcbc51ae7755 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Mon, 30 Mar 2026 16:49:03 -0400
Subject: [PATCH] fix: prevent data loss when git isolation default changes
 (#2625) (#3043)

When the default isolation mode flipped from "worktree" to "none" between
versions, mergeAndExit() returned early for mode "none" without checking
whether the session was physically inside an active worktree. This silently
skipped the merge, orphaning committed work on the milestone branch.

The fix moves the worktree-presence check (isInAutoWorktree + originalBasePath)
before the mode-none early return. If we are inside a worktree, mergeAndExit
proceeds with the worktree merge path regardless of the configured mode.

Also fixes the misleading JSDoc on GitPreferences.isolation that claimed
"worktree" was the default when the runtime default is actually "none".

Closes #2625

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/resources/extensions/gsd/git-service.ts   |  4 +-
 .../gsd/tests/worktree-resolver.test.ts       | 46 +++++++++++++++++++
 .../extensions/gsd/worktree-resolver.ts       | 11 +++--
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/src/resources/extensions/gsd/git-service.ts b/src/resources/extensions/gsd/git-service.ts
index d7c543182..88c07f50a 100644
--- a/src/resources/extensions/gsd/git-service.ts
+++ b/src/resources/extensions/gsd/git-service.ts
@@ -50,9 +50,9 @@ export interface GitPreferences {
   main_branch?: string;
   merge_strategy?: "squash" | "merge";
   /** Controls auto-mode git isolation strategy.
-   *  - "worktree": (default) creates a milestone worktree for isolated work
+   *  - "worktree": creates a milestone worktree for isolated work
    *  - "branch": works directly in the project root (for submodule-heavy repos)
-   *  - "none": no git isolation — commits land on the user's current branch directly
+   *  - "none": (default) no git isolation — commits land on the user's current branch directly
    */
   isolation?: "worktree" | "branch" | "none";
   /** When false, GSD will not modify .gitignore at all — no baseline patterns
diff --git a/src/resources/extensions/gsd/tests/worktree-resolver.test.ts b/src/resources/extensions/gsd/tests/worktree-resolver.test.ts
index d227d6efb..9cfda718d 100644
--- a/src/resources/extensions/gsd/tests/worktree-resolver.test.ts
+++ b/src/resources/extensions/gsd/tests/worktree-resolver.test.ts
@@ -914,3 +914,49 @@ test("isolationDegraded is reset by session.reset() (#2483)", () => {
 
   assert.equal(s.isolationDegraded, false);
 });
+
+// ─── #2625 — Default isolation mode change must not orphan worktree commits ──
+
+test("mergeAndExit still merges when mode is 'none' but session is in a worktree (#2625)", () => {
+  // Scenario: user upgraded from a version where default was "worktree" to one
+  // where default is "none". They have an active worktree with committed work.
+  // mergeAndExit must detect the active worktree and merge regardless of config.
+  const s = makeSession({
+    basePath: "/project/.gsd/worktrees/M001",
+    originalBasePath: "/project",
+  });
+  const deps = makeDeps({
+    isInAutoWorktree: () => true,
+    getIsolationMode: () => "none", // config says "none" — but we ARE in a worktree
+  });
+  const ctx = makeNotifyCtx();
+  const resolver = new WorktreeResolver(s, deps);
+
+  resolver.mergeAndExit("M001", ctx);
+
+  // Must still merge — not skip silently
+  assert.equal(findCalls(deps.calls, "mergeMilestoneToMain").length, 1,
+    "must call mergeMilestoneToMain even when isolation mode is 'none' but we are in a worktree");
+  assert.equal(s.basePath, "/project", "basePath must be restored to project root");
+  assert.ok(ctx.messages.some((m) => m.msg.includes("merged to main")),
+    "must notify about the merge");
+});
+
+test("mergeAndExit in none mode remains a no-op when NOT in a worktree (#2625)", () => {
+  // When mode is "none" and we are genuinely not in a worktree, it should still be a no-op.
+  const s = makeSession({
+    basePath: "/project",
+    originalBasePath: "/project",
+  });
+  const deps = makeDeps({
+    isInAutoWorktree: () => false,
+    getIsolationMode: () => "none",
+  });
+  const ctx = makeNotifyCtx();
+  const resolver = new WorktreeResolver(s, deps);
+
+  resolver.mergeAndExit("M001", ctx);
+
+  assert.equal(findCalls(deps.calls, "mergeMilestoneToMain").length, 0,
+    "must NOT merge when not in a worktree and mode is none");
+});
diff --git a/src/resources/extensions/gsd/worktree-resolver.ts b/src/resources/extensions/gsd/worktree-resolver.ts
index 6c459ba67..413096fe0 100644
--- a/src/resources/extensions/gsd/worktree-resolver.ts
+++ b/src/resources/extensions/gsd/worktree-resolver.ts
@@ -350,7 +350,13 @@ export class WorktreeResolver {
       data: { milestoneId, mode },
     });
 
-    if (mode === "none") {
+    // #2625: If we are physically inside an auto-worktree, we MUST merge
+    // regardless of the current isolation config. This prevents data loss when
+    // the default isolation mode changes between versions (e.g., "worktree" ->
+    // "none"): the worktree branch still holds real commits that need merging.
+    const inWorktree = this.deps.isInAutoWorktree(this.s.basePath) && this.s.originalBasePath;
+
+    if (mode === "none" && !inWorktree) {
       debugLog("WorktreeResolver", {
         action: "mergeAndExit",
         milestoneId,
@@ -361,8 +367,7 @@ export class WorktreeResolver {
     }
 
     if (
-      mode === "worktree" ||
-      (this.deps.isInAutoWorktree(this.s.basePath) && this.s.originalBasePath)
+      mode === "worktree" || inWorktree
     ) {
       this._mergeWorktreeMode(milestoneId, ctx);
     } else if (mode === "branch") {