diff --git a/src/resources/extensions/gsd/git-service.ts b/src/resources/extensions/gsd/git-service.ts index e264170e3..9d3f46674 100644 --- a/src/resources/extensions/gsd/git-service.ts +++ b/src/resources/extensions/gsd/git-service.ts @@ -63,6 +63,7 @@ export const RUNTIME_EXCLUSION_PATHS: readonly string[] = [ ".gsd/worktrees/", ".gsd/auto.lock", ".gsd/metrics.json", + ".gsd/completed-units.json", ".gsd/STATE.md", ]; @@ -130,16 +131,42 @@ export class GitServiceImpl { */ private smartStage(extraExclusions: readonly string[] = []): void { const allExclusions = [...RUNTIME_EXCLUSION_PATHS, ...extraExclusions]; - const excludes = allExclusions.map(p => `':(exclude)${p}'`); - const args = ["add", "-A", "--", ".", ...excludes]; - try { - this.git(args); - } catch { - console.error("GitService: smart staging failed, falling back to git add -A"); - this.git(["add", "-A"]); + + // One-time cleanup: if runtime files are already tracked in the index + // (from older versions where the fallback bug staged them), untrack them + // in a dedicated commit. This must happen as a separate commit because + // the git reset HEAD step below would otherwise undo the rm --cached. + if (!this._runtimeFilesCleanedUp) { + let cleaned = false; + for (const exclusion of RUNTIME_EXCLUSION_PATHS) { + const result = this.git(["rm", "--cached", "-r", "--ignore-unmatch", exclusion], { allowFailure: true }); + if (result && result.includes("rm '")) cleaned = true; + } + if (cleaned) { + this.git(["commit", "-F", "-"], { input: "chore: untrack .gsd/ runtime files from git index" }); + } + this._runtimeFilesCleanedUp = true; + } + + // Stage everything, then unstage excluded paths. + // + // Previous approach used pathspec excludes (:(exclude)...) with git add -A, + // but that fails when .gsd/ is in .gitignore — git exits non-zero before + // evaluating the excludes. The catch fallback ran plain `git add -A`, + // staging all tracked runtime files unconditionally and defeating the + // exclusion list entirely. + // + // git reset HEAD silently succeeds when the path isn't staged, so no + // error handling is needed per-path. + this.git(["add", "-A"]); + for (const exclusion of allExclusions) { + this.git(["reset", "HEAD", "--", exclusion], { allowFailure: true }); } } + /** Tracks whether runtime file cleanup has run this session. */ + private _runtimeFilesCleanedUp = false; + /** * Stage files (smart staging) and commit. * Returns the commit message string on success, or null if nothing to commit. @@ -312,6 +339,12 @@ export class GitServiceImpl { // Exclude .gsd/ to prevent merge conflicts when both branches modify planning artifacts. this.autoCommit("pre-switch", current, [".gsd/"]); + // Discard uncommitted .gsd/ changes so checkout doesn't fail. + // These are runtime files (metrics, completed-units, STATE) that were + // intentionally excluded from the commit above. If they remain dirty, + // git checkout refuses when the target branch has different versions. + this.git(["checkout", "--", ".gsd/"], { allowFailure: true }); + this.git(["checkout", branch]); return created; } @@ -327,6 +360,9 @@ export class GitServiceImpl { // Exclude .gsd/ to prevent merge conflicts when both branches modify planning artifacts. this.autoCommit("pre-switch", current, [".gsd/"]); + // Discard uncommitted .gsd/ changes so checkout doesn't fail. + this.git(["checkout", "--", ".gsd/"], { allowFailure: true }); + this.git(["checkout", mainBranch]); } diff --git a/src/resources/extensions/gsd/gitignore.ts b/src/resources/extensions/gsd/gitignore.ts index db9d40d91..2886519f6 100644 --- a/src/resources/extensions/gsd/gitignore.ts +++ b/src/resources/extensions/gsd/gitignore.ts @@ -20,6 +20,7 @@ const BASELINE_PATTERNS = [ ".gsd/worktrees/", ".gsd/auto.lock", ".gsd/metrics.json", + ".gsd/completed-units.json", ".gsd/STATE.md", // ── OS junk ── diff --git a/src/resources/extensions/gsd/tests/git-service.test.ts b/src/resources/extensions/gsd/tests/git-service.test.ts index f0096084f..994e7e410 100644 --- a/src/resources/extensions/gsd/tests/git-service.test.ts +++ b/src/resources/extensions/gsd/tests/git-service.test.ts @@ -210,8 +210,8 @@ async function main(): Promise { assertEq( RUNTIME_EXCLUSION_PATHS.length, - 6, - "exactly 6 runtime exclusion paths" + 7, + "exactly 7 runtime exclusion paths" ); const expectedPaths = [ @@ -220,6 +220,7 @@ async function main(): Promise { ".gsd/worktrees/", ".gsd/auto.lock", ".gsd/metrics.json", + ".gsd/completed-units.json", ".gsd/STATE.md", ]; @@ -347,52 +348,76 @@ async function main(): Promise { rmSync(repo, { recursive: true, force: true }); } - // ─── GitServiceImpl: smart staging fallback ──────────────────────────── + // ─── GitServiceImpl: smart staging excludes tracked runtime files ────── - console.log("\n=== GitServiceImpl: smart staging fallback ==="); + console.log("\n=== GitServiceImpl: smart staging excludes tracked runtime files ==="); { - // We can't easily make the pathspec fail in a real repo, but we can test - // the fallback behavior by verifying that if smart staging somehow fails, - // everything gets staged. We do this by checking that a commit with both - // runtime and real files works when pathspec would fail. + // Reproduces the real bug: .gsd/ runtime files that are already tracked + // (in the git index) must be excluded from staging even when .gsd/ is + // in .gitignore. The old pathspec-exclude approach failed silently in + // this case and fell back to `git add -A`, staging everything. // - // To force the fallback: temporarily override RUNTIME_EXCLUSION_PATHS - // with an invalid pathspec. Since we can't modify a readonly array, - // we'll test the actual fallback by creating a custom subclass. + // The fix has three layers: + // 1. Auto-cleanup: git rm --cached removes tracked runtime files from index + // 2. Stage-then-unstage: git add -A + git reset HEAD replaces pathspec excludes + // 3. Pre-checkout discard: git checkout -- .gsd/ clears dirty runtime files const repo = initTempRepo(); + const svc = new GitServiceImpl(repo); - // Create a subclass that overrides smartStage to simulate failure + fallback - class FallbackTestService extends GitServiceImpl { - fallbackUsed = false; - smartStageWithBadPathspec(): void { - // Simulate: try bad pathspec, catch, fallback - try { - runGit(this.basePath, ["add", "-A", "--", ".", ":(exclude)__NONEXISTENT_PATHSPEC_SYNTAX_ERROR__["]); - // If the above doesn't throw, git accepted it (some versions do). - // That's fine — the point is testing the fallback path. - throw new Error("force fallback for test"); - } catch { - console.error("GitService: smart staging failed, falling back to git add -A"); - this.fallbackUsed = true; - runGit(this.basePath, ["add", "-A"]); - } - } - } - - const svc = new FallbackTestService(repo); + // Simulate a repo where .gsd/ files were previously force-added + createFile(repo, ".gsd/metrics.json", '{"version":1}'); + createFile(repo, ".gsd/completed-units.json", '["unit1"]'); + createFile(repo, ".gsd/activity/log.jsonl", '{"ts":1}'); createFile(repo, "src/real.ts", "real code"); - createFile(repo, ".gsd/activity/log.jsonl", "log"); + // Force-add .gsd/ files to simulate historical tracking + runGit(repo, ["add", "-f", ".gsd/metrics.json", ".gsd/completed-units.json", ".gsd/activity/log.jsonl", "src/real.ts"]); + runGit(repo, ["commit", "-F", "-"], { input: "init with tracked runtime files" }); - // Call the fallback path manually - svc.smartStageWithBadPathspec(); + // Add .gitignore with .gsd/ (matches real-world setup from ensureGitignore) + createFile(repo, ".gitignore", ".gsd/\n"); + runGit(repo, ["add", ".gitignore"]); + runGit(repo, ["commit", "-F", "-"], { input: "add gitignore" }); - // Check that everything was staged (fallback stages all) - const staged = run("git diff --cached --name-only", repo); - assert(staged.includes("src/real.ts"), "fallback stages real files"); - assert(staged.includes(".gsd/activity/log.jsonl"), "fallback stages runtime files too (no exclusion)"); - assert(svc.fallbackUsed, "fallback path was actually used"); + // Verify runtime files are tracked (precondition) + const tracked = run("git ls-files .gsd/", repo); + assert(tracked.includes("metrics.json"), "precondition: metrics.json tracked"); + assert(tracked.includes("completed-units.json"), "precondition: completed-units.json tracked"); + assert(tracked.includes("activity/log.jsonl"), "precondition: activity log tracked"); + + // Now modify both runtime and real files + createFile(repo, ".gsd/metrics.json", '{"version":2}'); + createFile(repo, ".gsd/completed-units.json", '["unit1","unit2"]'); + createFile(repo, ".gsd/activity/log.jsonl", '{"ts":2}'); + createFile(repo, "src/real.ts", "updated code"); + + // autoCommit should commit real.ts. The first call also runs auto-cleanup + // which removes runtime files from the index via a dedicated commit. + const msg = svc.autoCommit("execute-task", "M001/S01/T01"); + assert(msg !== null, "autoCommit produces a commit"); + + const show = run("git show --stat HEAD", repo); + assert(show.includes("src/real.ts"), "real files are committed"); + + // After the commit, runtime files must no longer be in the git index. + // They remain on disk but are untracked (protected by .gitignore). + const trackedAfter = run("git ls-files .gsd/", repo); + assertEq(trackedAfter, "", "no .gsd/ runtime files remain in the index"); + + // Verify a second autoCommit with changed runtime files does NOT stage them + createFile(repo, ".gsd/metrics.json", '{"version":3}'); + createFile(repo, ".gsd/completed-units.json", '["unit1","unit2","unit3"]'); + createFile(repo, "src/real.ts", "third version"); + + const msg2 = svc.autoCommit("execute-task", "M001/S01/T02"); + assert(msg2 !== null, "second autoCommit produces a commit"); + + const show2 = run("git show --stat HEAD", repo); + assert(show2.includes("src/real.ts"), "real files committed in second commit"); + assert(!show2.includes("metrics"), "metrics.json not in second commit"); + assert(!show2.includes("completed-units"), "completed-units.json not in second commit"); + assert(!show2.includes("activity"), "activity not in second commit"); rmSync(repo, { recursive: true, force: true }); }