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 <noreply@anthropic.com> * ci: re-trigger checks --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
e71de432ab
commit
df9e06cfa5
4 changed files with 189 additions and 1 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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}}
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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}`,
|
||||
);
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue