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>
This commit is contained in:
Tom Boucher 2026-03-30 16:49:03 -04:00 committed by GitHub
parent 893c525578
commit 9384641b25
3 changed files with 56 additions and 5 deletions

View file

@ -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

View file

@ -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");
});

View file

@ -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") {