From b67101c51bfadd58f8b7f31b1372cbc2ea9f4e5a Mon Sep 17 00:00:00 2001 From: dan bachelder <325706+dbachelder@users.noreply.github.com> Date: Thu, 19 Mar 2026 06:37:25 -0700 Subject: [PATCH] fix: keep external GSD state stable in worktrees (#1334) --- src/resources/extensions/gsd/auto-worktree.ts | 5 +- src/resources/extensions/gsd/repo-identity.ts | 86 +++++++++++++------ .../gsd/tests/loop-regression.test.ts | 5 +- .../gsd/tests/repo-identity-worktree.test.ts | 71 +++++++++++++++ 4 files changed, 135 insertions(+), 32 deletions(-) create mode 100644 src/resources/extensions/gsd/tests/repo-identity-worktree.test.ts diff --git a/src/resources/extensions/gsd/auto-worktree.ts b/src/resources/extensions/gsd/auto-worktree.ts index fb1c9332d..3fdbc8444 100644 --- a/src/resources/extensions/gsd/auto-worktree.ts +++ b/src/resources/extensions/gsd/auto-worktree.ts @@ -6,7 +6,7 @@ * manages create, enter, detect, and teardown for auto-mode worktrees. */ -import { existsSync, readFileSync, realpathSync, unlinkSync, statSync, rmSync, readdirSync, cpSync, lstatSync as lstatSyncFn } from "node:fs"; +import { existsSync, readFileSync, realpathSync, unlinkSync, statSync, rmSync, readdirSync, cpSync, mkdirSync, lstatSync as lstatSyncFn } from "node:fs"; import { isAbsolute, join, sep } from "node:path"; import { GSDError, GSD_IO_ERROR, GSD_GIT_ERROR } from "./errors.js"; import { execSync, execFileSync } from "node:child_process"; @@ -93,8 +93,9 @@ export function syncGsdStateToWorktree(mainBasePath: string, worktreePath_: stri // Sync milestones: copy entire milestone directories that are missing const mainMilestonesDir = join(mainGsd, "milestones"); const wtMilestonesDir = join(wtGsd, "milestones"); - if (existsSync(mainMilestonesDir) && existsSync(wtMilestonesDir)) { + if (existsSync(mainMilestonesDir)) { try { + mkdirSync(wtMilestonesDir, { recursive: true }); const mainMilestones = readdirSync(mainMilestonesDir, { withFileTypes: true }) .filter(d => d.isDirectory() && /^M\d{3}/.test(d.name)) .map(d => d.name); diff --git a/src/resources/extensions/gsd/repo-identity.ts b/src/resources/extensions/gsd/repo-identity.ts index 326f591d9..f9429458e 100644 --- a/src/resources/extensions/gsd/repo-identity.ts +++ b/src/resources/extensions/gsd/repo-identity.ts @@ -8,9 +8,9 @@ import { createHash } from "node:crypto"; import { execFileSync } from "node:child_process"; -import { existsSync, lstatSync, mkdirSync, readFileSync, realpathSync, symlinkSync } from "node:fs"; +import { existsSync, lstatSync, mkdirSync, readFileSync, realpathSync, rmSync, symlinkSync } from "node:fs"; import { homedir } from "node:os"; -import { join, resolve, sep } from "node:path"; +import { join, resolve } from "node:path"; // ─── Repo Identity ────────────────────────────────────────────────────────── @@ -35,35 +35,56 @@ function getRemoteUrl(basePath: string): string { * Resolve the git toplevel (real root) for the given path. * For worktrees this returns the main repo root, not the worktree path. */ +function canonicalizeExistingPath(path: string): string { + try { + return realpathSync(path); + } catch { + return resolve(path); + } +} + +function resolveGitCommonDir(basePath: string): string { + try { + return execFileSync("git", ["rev-parse", "--path-format=absolute", "--git-common-dir"], { + cwd: basePath, + encoding: "utf-8", + stdio: ["ignore", "pipe", "ignore"], + timeout: 5_000, + }).trim(); + } catch { + const raw = execFileSync("git", ["rev-parse", "--git-common-dir"], { + cwd: basePath, + encoding: "utf-8", + stdio: ["ignore", "pipe", "ignore"], + timeout: 5_000, + }).trim(); + return resolve(basePath, raw); + } +} + function resolveGitRoot(basePath: string): string { try { - // In a worktree, --show-toplevel returns the worktree path, not the main - // repo root. Use --git-common-dir to find the shared .git directory, - // then derive the main repo root from it (#1288). - const commonDir = execFileSync("git", ["rev-parse", "--git-common-dir"], { - cwd: basePath, - encoding: "utf-8", - stdio: ["ignore", "pipe", "ignore"], - timeout: 5_000, - }).trim(); + const commonDir = resolveGitCommonDir(basePath); + const normalizedCommonDir = commonDir.replaceAll("\\", "/"); - // If commonDir ends with .git/worktrees/, the main repo is two - // levels up from the worktrees dir. If it's just .git, resolve normally. - if (commonDir.includes(`${sep}worktrees${sep}`) || commonDir.includes("/worktrees/")) { - // e.g., /path/to/project/.gsd/worktrees/M001/.git → /path/to/project - // or /path/to/project/.git/worktrees/M001 → /path/to/project - const gitDir = commonDir.replace(/[/\\]worktrees[/\\][^/\\]+$/, ""); - const mainRoot = resolve(gitDir, ".."); - return mainRoot; + // Normal repo or worktree with shared common dir pointing at /.git. + if (normalizedCommonDir.endsWith("/.git")) { + return canonicalizeExistingPath(resolve(commonDir, "..")); } - // Not in a worktree — use --show-toplevel as usual - return execFileSync("git", ["rev-parse", "--show-toplevel"], { + // Some git setups may still expose /.git/worktrees/. + const worktreeMarker = "/.git/worktrees/"; + if (normalizedCommonDir.includes(worktreeMarker)) { + return canonicalizeExistingPath(resolve(commonDir, "..", "..")); + } + + // Fallback for unusual layouts. + return canonicalizeExistingPath(execFileSync("git", ["rev-parse", "--show-toplevel"], { cwd: basePath, encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 5_000, - }).trim(); + }).trim()); } catch { return resolve(basePath); } @@ -111,10 +132,17 @@ export function externalGsdRoot(basePath: string): string { export function ensureGsdSymlink(projectPath: string): string { const externalPath = externalGsdRoot(projectPath); const localGsd = join(projectPath, ".gsd"); + const inWorktree = isInsideWorktree(projectPath); // Ensure external directory exists mkdirSync(externalPath, { recursive: true }); + const replaceWithSymlink = (): string => { + rmSync(localGsd, { recursive: true, force: true }); + symlinkSync(externalPath, localGsd, "junction"); + return externalPath; + }; + if (!existsSync(localGsd)) { // Nothing exists yet — create symlink symlinkSync(externalPath, localGsd, "junction"); @@ -130,14 +158,20 @@ export function ensureGsdSymlink(projectPath: string): string { if (target === externalPath) { return externalPath; // correct symlink, no-op } - // Symlink exists but points elsewhere — leave it for now - // (could be a custom override or stale symlink) + // In a worktree, mismatched symlinks are always stale. Heal them so + // the worktree points at the same external state dir as the main repo. + if (inWorktree) { + return replaceWithSymlink(); + } + // Outside worktrees, preserve custom overrides or legacy symlinks. return target; } if (stat.isDirectory()) { - // Real directory — migration will handle this later. - // Return the local path so existing code still works. + // Real directory in the main repo — migration will handle this later. + // In worktrees, keep the directory in place and let syncGsdStateToWorktree + // refresh its contents. Replacing a git-tracked .gsd directory with a + // symlink makes git think tracked planning files were deleted. return localGsd; } } catch { diff --git a/src/resources/extensions/gsd/tests/loop-regression.test.ts b/src/resources/extensions/gsd/tests/loop-regression.test.ts index 3fbf223d4..fbb39d1c5 100644 --- a/src/resources/extensions/gsd/tests/loop-regression.test.ts +++ b/src/resources/extensions/gsd/tests/loop-regression.test.ts @@ -510,11 +510,8 @@ test("dispatch returns stop when phase=executing but activeSlice is null (corrup // ─── Phase 6: Worktree & Lock Consistency ──────────────────────────────── -test("repoIdentity returns same hash for main repo and worktree", async () => { - // This test verifies the fix for #1288 — identity hash must be stable - // across worktree and non-worktree contexts. +test("repoIdentity returns a 12-char hex hash", async () => { const { repoIdentity } = await import("../repo-identity.ts"); - // Call from the current directory (main repo) const hash = repoIdentity(process.cwd()); assert.ok(hash.length === 12, `hash should be 12 hex chars, got: ${hash}`); assert.match(hash, /^[a-f0-9]{12}$/, `hash should be hex, got: ${hash}`); diff --git a/src/resources/extensions/gsd/tests/repo-identity-worktree.test.ts b/src/resources/extensions/gsd/tests/repo-identity-worktree.test.ts new file mode 100644 index 000000000..1719fe264 --- /dev/null +++ b/src/resources/extensions/gsd/tests/repo-identity-worktree.test.ts @@ -0,0 +1,71 @@ +import { mkdtempSync, rmSync, writeFileSync, existsSync, lstatSync, realpathSync, mkdirSync, symlinkSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { execSync } from "node:child_process"; + +import { externalGsdRoot, ensureGsdSymlink } from "../repo-identity.ts"; +import { createTestContext } from "./test-helpers.ts"; + +const { assertEq, assertTrue, report } = createTestContext(); + +function run(command: string, cwd: string): string { + return execSync(command, { cwd, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8" }).trim(); +} + +async function main(): Promise { + const base = mkdtempSync(join(tmpdir(), "gsd-repo-identity-")); + const stateDir = mkdtempSync(join(tmpdir(), "gsd-state-")); + + try { + process.env.GSD_STATE_DIR = stateDir; + + run("git init -b main", base); + run('git config user.name "Pi Test"', base); + run('git config user.email "pi@example.com"', base); + run('git remote add origin git@github.com:example/repo.git', base); + writeFileSync(join(base, "README.md"), "# Test Repo\n", "utf-8"); + run("git add README.md", base); + run('git commit -m "chore: init"', base); + + const worktreePath = join(base, ".gsd", "worktrees", "M001"); + run(`git worktree add -b milestone/M001 ${worktreePath}`, base); + + console.log("\n=== ensureGsdSymlink points worktree at main repo external state dir ==="); + const expectedExternalState = externalGsdRoot(base); + const mainState = ensureGsdSymlink(base); + assertEq(mainState, realpathSync(join(base, ".gsd")), "ensureGsdSymlink(base) returns the current main repo .gsd target"); + const worktreeState = ensureGsdSymlink(worktreePath); + assertEq(worktreeState, expectedExternalState, "worktree symlink target matches main repo external state dir"); + assertTrue(existsSync(join(worktreePath, ".gsd")), "worktree .gsd exists"); + assertTrue(lstatSync(join(worktreePath, ".gsd")).isSymbolicLink(), "worktree .gsd is a symlink"); + assertEq(realpathSync(join(worktreePath, ".gsd")), expectedExternalState, "worktree .gsd symlink resolves to main repo external state dir"); + + console.log("\n=== ensureGsdSymlink heals stale worktree symlinks ==="); + const staleState = join(stateDir, "projects", "stale-worktree-state"); + mkdirSync(staleState, { recursive: true }); + rmSync(join(worktreePath, ".gsd"), { recursive: true, force: true }); + symlinkSync(staleState, join(worktreePath, ".gsd"), "junction"); + const healedState = ensureGsdSymlink(worktreePath); + assertEq(healedState, expectedExternalState, "stale worktree symlink is repaired to canonical external state dir"); + assertEq(realpathSync(join(worktreePath, ".gsd")), expectedExternalState, "healed worktree symlink resolves to canonical external state dir"); + + console.log("\n=== ensureGsdSymlink preserves worktree .gsd directories ==="); + rmSync(join(worktreePath, ".gsd"), { recursive: true, force: true }); + mkdirSync(join(worktreePath, ".gsd", "milestones"), { recursive: true }); + writeFileSync(join(worktreePath, ".gsd", "milestones", "stale.txt"), "stale\n", "utf-8"); + const preservedDirState = ensureGsdSymlink(worktreePath); + assertEq(preservedDirState, join(worktreePath, ".gsd"), "worktree .gsd directory is left in place for sync-based refresh"); + assertTrue(lstatSync(join(worktreePath, ".gsd")).isDirectory(), "worktree .gsd directory remains a directory"); + assertTrue(existsSync(join(worktreePath, ".gsd", "milestones", "stale.txt")), "existing worktree .gsd directory contents remain available for sync logic"); + } finally { + delete process.env.GSD_STATE_DIR; + rmSync(base, { recursive: true, force: true }); + rmSync(stateDir, { recursive: true, force: true }); + report(); + } +} + +main().catch((error) => { + console.error(error); + process.exit(1); +});