fix(gsd): close residual #1364 data-loss vectors on v2.36.0+ (#1637)

Two targeted fixes that close the three remaining paths where .gsd/
tracked files can still be silently deleted after the v2.36.0 fix.

--- Path 1: hasGitTrackedGsdFiles fails open on git error (gitignore.ts)

nativeLsFiles() swallows git failures via allowFailure=true and returns
[], making hasGitTrackedGsdFiles() indistinguishable between "nothing
tracked" and "git failed". On any transient git failure (locked index,
binary not on PATH, corrupted .git/index), the function returned false
and .gsd was added to .gitignore, deleting all tracked state.

Fix: after nativeLsFiles returns [], verify git is reachable with a
cheap rev-parse call. If git is unavailable, return true (fail safe —
assume tracked). The outer catch also returns true instead of false.

--- Path 2: migration never cleans git index (migrate-external.ts)

migrateToExternalState() correctly creates the .gsd symlink/junction but
never ran `git rm -r --cached .gsd/`. All previously tracked .gsd/* files
remained in the git index pointing through the new symlink, which git
cannot follow — causing PROJECT.md, milestones/, REQUIREMENTS.md etc. to
appear as deleted in git status immediately after every migration.

Fix: after the symlink is verified, run:
  git rm -r --cached --ignore-unmatch .gsd
--ignore-unmatch makes this a no-op on fresh/untracked projects.

--- Path 3: race between migration and ensureGitignore

Resolved by Path 2. If migration always cleans the index, the race
window (another process converting .gsd/ to a symlink between the
migrateToExternalState() and ensureGitignore() calls) is harmless —
the index is already clean and there is nothing to lose.

--- Tests added (gitignore-tracked-gsd.test.ts)

- hasGitTrackedGsdFiles returns true (fail-safe) when git is unavailable
  (simulated via .git/index.lock to force git ls-files failure)
- migrateToExternalState cleans git index so tracked files don't show
  as deleted after successful migration

Fixes residual vectors from #1364 (original fix: #1367, v2.36.0)
This commit is contained in:
Jeremy McSpadden 2026-03-20 14:15:11 -05:00 committed by GitHub
parent 8e2d403179
commit 6f15ddcbf7
3 changed files with 85 additions and 4 deletions

View file

@ -7,9 +7,11 @@
*/
import { join } from "node:path";
import { execFileSync } from "node:child_process";
import { existsSync, lstatSync, readFileSync, writeFileSync } from "node:fs";
import { nativeRmCached, nativeLsFiles } from "./native-git-bridge.js";
import { gsdRoot } from "./paths.js";
import { GIT_NO_PROMPT_ENV } from "./git-constants.js";
/**
* GSD runtime patterns for git index cleanup.
@ -104,10 +106,22 @@ export function hasGitTrackedGsdFiles(basePath: string): boolean {
// Check if git tracks any files under .gsd/
try {
const tracked = nativeLsFiles(basePath, ".gsd");
return tracked.length > 0;
} catch {
// Not a git repo or git not available — safe to proceed
if (tracked.length > 0) return true;
// nativeLsFiles swallows git failures and returns []. An empty result
// could mean "nothing tracked" OR "git failed silently". Verify git is
// reachable before trusting the empty result — if it isn't, fail safe
// by assuming files ARE tracked to prevent data loss.
execFileSync("git", ["rev-parse", "--git-dir"], {
cwd: basePath,
stdio: "pipe",
env: GIT_NO_PROMPT_ENV,
});
return false;
} catch {
// git unavailable, index locked, or repo corrupt — fail safe
return true;
}
}

View file

@ -6,11 +6,13 @@
* symlink replaces the original directory so all paths remain valid.
*/
import { execFileSync } from "node:child_process";
import { existsSync, lstatSync, mkdirSync, readdirSync, realpathSync, renameSync, cpSync, rmSync, symlinkSync } from "node:fs";
import { join } from "node:path";
import { externalGsdRoot } from "./repo-identity.js";
import { getErrorMessage } from "./error-utils.js";
import { hasGitTrackedGsdFiles } from "./gitignore.js";
import { GIT_NO_PROMPT_ENV } from "./git-constants.js";
export interface MigrationResult {
migrated: boolean;
@ -144,7 +146,22 @@ export function migrateToExternalState(basePath: string): MigrationResult {
return { migrated: false, error: `Migration verification failed: ${getErrorMessage(verifyErr)}` };
}
// Remove .gsd.migrating only after symlink is verified
// Clean the git index — any .gsd/* files tracked before migration now
// sit behind the symlink and git can't follow it, causing them to show
// as deleted. Remove them from the index so the working tree stays clean.
// --ignore-unmatch makes this a no-op on fresh projects with no tracked .gsd/.
try {
execFileSync("git", ["rm", "-r", "--cached", "--ignore-unmatch", ".gsd"], {
cwd: basePath,
stdio: ["ignore", "pipe", "ignore"],
env: GIT_NO_PROMPT_ENV,
timeout: 10_000,
});
} catch {
// Non-fatal — git may be unavailable or nothing was tracked
}
// Remove .gsd.migrating only after symlink is verified and index is clean
rmSync(migratingPath, { recursive: true, force: true });
return { migrated: true };

View file

@ -183,6 +183,28 @@ test("ensureGitignore with tracked .gsd/ does not cause git to see files as dele
}
});
test("hasGitTrackedGsdFiles returns true (fail-safe) when git is not available", () => {
const dir = makeTempRepo();
try {
// Create and track .gsd/ files
mkdirSync(join(dir, ".gsd"), { recursive: true });
writeFileSync(join(dir, ".gsd", "PROJECT.md"), "# Project\n");
git(dir, "add", ".gsd/");
git(dir, "commit", "-m", "track gsd");
// Corrupt the git index to simulate git failure
const indexPath = join(dir, ".git", "index.lock");
writeFileSync(indexPath, "locked");
// Should fail safe — assume tracked rather than silently returning false
// (The index lock causes git ls-files to fail; rev-parse also fails → true)
const result = hasGitTrackedGsdFiles(dir);
assert.equal(result, true, "Should return true (fail-safe) when git is unavailable");
} finally {
cleanup(dir);
}
});
// ─── migrateToExternalState — tracked .gsd/ protection ──────────────
test("migrateToExternalState aborts when .gsd/ has tracked files (#1364)", () => {
@ -212,3 +234,31 @@ test("migrateToExternalState aborts when .gsd/ has tracked files (#1364)", () =>
cleanup(dir);
}
});
test("migrateToExternalState cleans git index so tracked files don't show as deleted (#1364 path 2)", () => {
const dir = makeTempRepo();
try {
// Track .gsd/ files, then untrack them so migration proceeds
mkdirSync(join(dir, ".gsd", "milestones", "M001"), { recursive: true });
writeFileSync(join(dir, ".gsd", "PROJECT.md"), "# Project\n");
writeFileSync(join(dir, ".gsd", "milestones", "M001", "PLAN.md"), "# Plan\n");
git(dir, "add", ".gsd/");
git(dir, "commit", "-m", "track gsd state");
git(dir, "rm", "-r", "--cached", ".gsd/");
git(dir, "commit", "-m", "untrack gsd (simulates pre-migration project)");
const result = migrateToExternalState(dir);
assert.equal(result.migrated, true, "Migration should succeed");
// git status must show NO deleted files after migration
const status = git(dir, "status", "--porcelain");
const deletions = status.split("\n").filter((l) => /^\s*D\s/.test(l) || /^D\s/.test(l));
assert.equal(
deletions.length,
0,
`Expected no deleted files after migration, but found:\n${deletions.join("\n")}`,
);
} finally {
cleanup(dir);
}
});