From df9e06cfa599d6be6bc648410606abf8f4cef046 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 16:43:56 -0400 Subject: [PATCH] fix: respect .gitignore for .gsd/ in rethink prompt (#3059) * fix: respect .gitignore for .gsd/ in rethink prompt (#2570) The rethink.md prompt template hardcoded `git add .gsd/` which caused the executing agent to force-add .gsd/ files (via `git add -f`) when .gsd was listed in .gitignore. This silently overrode the user's gitignore configuration, tracking planning artifacts they explicitly excluded. - Add `isGsdGitignored()` utility that uses `git check-ignore` to detect when .gsd is covered by .gitignore rules - Replace hardcoded `git add .gsd/` in rethink.md with the `{{commitInstruction}}` template variable (consistent with all other prompt templates) - Pass gitignore-aware commit instruction from rethink.ts: skip commit when .gsd is gitignored, include git add only when it is not Closes #2570 Co-Authored-By: Claude Opus 4.6 * ci: re-trigger checks --------- Co-authored-by: Claude Opus 4.6 --- src/resources/extensions/gsd/gitignore.ts | 32 ++++ .../extensions/gsd/prompts/rethink.md | 2 +- src/resources/extensions/gsd/rethink.ts | 6 + .../gitignore-staging-2570.test.ts | 150 ++++++++++++++++++ 4 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 src/resources/extensions/gsd/tests/integration/gitignore-staging-2570.test.ts diff --git a/src/resources/extensions/gsd/gitignore.ts b/src/resources/extensions/gsd/gitignore.ts index 93cddca4b..c3c2f66b8 100644 --- a/src/resources/extensions/gsd/gitignore.ts +++ b/src/resources/extensions/gsd/gitignore.ts @@ -85,6 +85,38 @@ const BASELINE_PATTERNS = [ "tmp/", ]; +/** + * Check whether `.gsd` is covered by the project's `.gitignore`. + * + * Uses `git check-ignore` for accurate evaluation — this respects nested + * .gitignore files, global gitignore, and negation patterns. Returns true + * only when git would actually ignore `.gsd/`. + * + * Returns false (not ignored) if: + * - No `.gitignore` exists + * - `.gsd` is not listed in any active ignore rule + * - Not a git repo or git is unavailable + */ +export function isGsdGitignored(basePath: string): boolean { + // Check both `.gsd` and `.gsd/` because `.gsd/` in .gitignore (trailing + // slash = directory-only pattern) only matches the directory form. Using + // both paths covers all gitignore pattern variants. + for (const path of [".gsd", ".gsd/"]) { + try { + // git check-ignore exits 0 when the path IS ignored, 1 when it is NOT. + execFileSync("git", ["check-ignore", "-q", path], { + cwd: basePath, + stdio: "pipe", + env: GIT_NO_PROMPT_ENV, + }); + return true; // exit 0 → .gsd is ignored + } catch { + // exit 1 → this form is NOT ignored, try the other + } + } + return false; // neither form is ignored (or git unavailable) +} + /** * Check whether `.gsd/` contains files tracked by git. * If so, the project intentionally keeps `.gsd/` in version control diff --git a/src/resources/extensions/gsd/prompts/rethink.md b/src/resources/extensions/gsd/prompts/rethink.md index da2a91495..e1222b9d0 100644 --- a/src/resources/extensions/gsd/prompts/rethink.md +++ b/src/resources/extensions/gsd/prompts/rethink.md @@ -80,4 +80,4 @@ If a proposed order would violate constraints, explain the issue and suggest alt - Do NOT park completed milestones — it would corrupt dependency satisfaction - Park is preferred over discard when a milestone has any completed work - Always persist queue order changes to `.gsd/QUEUE-ORDER.json` -- After changes, run `git add .gsd/ && git commit -m "docs(gsd): rethink milestone plan"` to persist (rethink runs interactively outside auto-mode, so no system auto-commit) +- {{commitInstruction}} diff --git a/src/resources/extensions/gsd/rethink.ts b/src/resources/extensions/gsd/rethink.ts index a6f049b77..599cbc32c 100644 --- a/src/resources/extensions/gsd/rethink.ts +++ b/src/resources/extensions/gsd/rethink.ts @@ -19,6 +19,7 @@ import { isParked, getParkedReason } from "./milestone-actions.js"; import { getMilestoneSlices, isDbAvailable } from "./gsd-db.js"; import { buildExistingMilestonesContext } from "./guided-flow-queue.js"; import { loadPrompt } from "./prompt-loader.js"; +import { isGsdGitignored } from "./gitignore.js"; // ─── Entry Point ────────────────────────────────────────────────────────────── @@ -53,9 +54,14 @@ export async function handleRethink( const rethinkData = buildRethinkData(basePath, milestoneIds, state, queueOrder); const existingMilestonesContext = await buildExistingMilestonesContext(basePath, milestoneIds, state); + const commitInstruction = isGsdGitignored(basePath) + ? "Do not commit planning artifacts — .gsd/ is gitignored in this project." + : 'After changes, run `git add .gsd/ && git commit -m "docs(gsd): rethink milestone plan"` to persist (rethink runs interactively outside auto-mode, so no system auto-commit)'; + const content = loadPrompt("rethink", { rethinkData, existingMilestonesContext, + commitInstruction, }); pi.sendMessage( diff --git a/src/resources/extensions/gsd/tests/integration/gitignore-staging-2570.test.ts b/src/resources/extensions/gsd/tests/integration/gitignore-staging-2570.test.ts new file mode 100644 index 000000000..b32f046a9 --- /dev/null +++ b/src/resources/extensions/gsd/tests/integration/gitignore-staging-2570.test.ts @@ -0,0 +1,150 @@ +/** + * gitignore-staging-2570.test.ts — Regression tests for #2570. + * + * Verifies that: + * 1. isGsdGitignored() detects when .gsd is covered by .gitignore + * 2. The rethink prompt uses {{commitInstruction}} instead of hardcoded git add .gsd/ + * 3. rethink.ts passes the correct commitInstruction based on gitignore state + * + * Uses real temporary git repos — no mocks. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +// Dynamic import — isGsdGitignored is the function under test (may not exist yet during TDD red phase) +const { isGsdGitignored } = await import("../../gitignore.ts"); + +// ─── Helpers ───────────────────────────────────────────────────────── + +function git(dir: string, ...args: string[]): string { + return execFileSync("git", args, { cwd: dir, stdio: "pipe", encoding: "utf-8" }).trim(); +} + +function makeTempRepo(): string { + const dir = mkdtempSync(join(tmpdir(), "gsd-staging-2570-")); + git(dir, "init"); + git(dir, "config", "user.email", "test@test.com"); + git(dir, "config", "user.name", "Test"); + writeFileSync(join(dir, "README.md"), "# init\n"); + git(dir, "add", "-A"); + git(dir, "commit", "-m", "init"); + git(dir, "branch", "-M", "main"); + return dir; +} + +function cleanup(dir: string): void { + try { + rmSync(dir, { recursive: true, force: true }); + } catch { + // ignore + } +} + +// ─── isGsdGitignored ───────────────────────────────────────────────── + +test("isGsdGitignored returns true when .gsd is in .gitignore (#2570)", (t) => { + const dir = makeTempRepo(); + t.after(() => { cleanup(dir); }); + + writeFileSync(join(dir, ".gitignore"), ".gsd\n"); + assert.equal(isGsdGitignored(dir), true); +}); + +test("isGsdGitignored returns true when .gsd/ (with slash) is in .gitignore", (t) => { + const dir = makeTempRepo(); + t.after(() => { cleanup(dir); }); + + writeFileSync(join(dir, ".gitignore"), ".gsd/\n"); + // Create .gsd directory so git check-ignore can match the directory-only pattern + mkdirSync(join(dir, ".gsd"), { recursive: true }); + assert.equal(isGsdGitignored(dir), true); +}); + +test("isGsdGitignored returns false when .gsd is NOT in .gitignore", (t) => { + const dir = makeTempRepo(); + t.after(() => { cleanup(dir); }); + + writeFileSync(join(dir, ".gitignore"), "node_modules/\n"); + assert.equal(isGsdGitignored(dir), false); +}); + +test("isGsdGitignored returns false when no .gitignore exists", (t) => { + const dir = makeTempRepo(); + t.after(() => { cleanup(dir); }); + + // No .gitignore — default + assert.equal(isGsdGitignored(dir), false); +}); + +// ─── rethink.md prompt template ───────────────────────────────────── + +test("rethink.md prompt uses {{commitInstruction}} not hardcoded git add .gsd/ (#2570)", () => { + const promptPath = join( + import.meta.dirname!, + "..", + "..", + "prompts", + "rethink.md", + ); + const content = readFileSync(promptPath, "utf-8"); + + // Must NOT contain hardcoded `git add .gsd/` + assert.ok( + !content.includes("git add .gsd/"), + `rethink.md must not contain hardcoded "git add .gsd/" — use {{commitInstruction}} instead.\nFound: ${content.match(/.*git add .gsd\/.*/)?.[0]}`, + ); + + // Must contain the {{commitInstruction}} placeholder + assert.ok( + content.includes("{{commitInstruction}}"), + "rethink.md must use {{commitInstruction}} template variable for commit step", + ); +}); + +// ─── smartStage respects .gitignore for .gsd/ (#2570) ─────────────── + +test("smartStage does not stage .gsd/ files when .gsd is gitignored (#2570)", async (t) => { + // This imports GitServiceImpl to test through the public commit() method + // which calls smartStage() internally. + const { GitServiceImpl } = await import("../../git-service.ts"); + + const dir = makeTempRepo(); + t.after(() => { cleanup(dir); }); + + // Add .gsd to .gitignore + writeFileSync(join(dir, ".gitignore"), ".gsd\nnode_modules/\n"); + git(dir, "add", ".gitignore"); + git(dir, "commit", "-m", "add gitignore with .gsd"); + + // Create .gsd/ milestone artifacts (NOT tracked, NOT symlinked) + mkdirSync(join(dir, ".gsd", "milestones", "M001", "slices", "S01"), { recursive: true }); + writeFileSync(join(dir, ".gsd", "milestones", "M001", "slices", "S01", "S01-PLAN.md"), "# Plan"); + writeFileSync(join(dir, ".gsd", "DECISIONS.md"), "# Decisions"); + + // Create a normal source file + writeFileSync(join(dir, "src.ts"), "export const x = 1;"); + + // Commit through GitServiceImpl (uses smartStage internally) + const svc = new GitServiceImpl(dir); + const msg = svc.commit({ message: "test: should not include .gsd files" }); + assert.ok(msg !== null, "commit should succeed"); + + // Check what was committed + const committed = git(dir, "show", "--name-only", "HEAD"); + assert.ok(committed.includes("src.ts"), "source files ARE committed"); + assert.ok( + !committed.includes(".gsd/"), + `gitignored .gsd/ files must NOT be staged by smartStage.\nCommitted files: ${committed}`, + ); +});