diff --git a/.changeset/tidy-bears-chatter.md b/.changeset/tidy-bears-chatter.md new file mode 100644 index 000000000..af65ceb84 --- /dev/null +++ b/.changeset/tidy-bears-chatter.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 3598 +--- +**GSD now warns when `.planning/` is gitignored but still tracked by git** — adding `.planning/` to `.gitignore` has no effect on files git already tracks, so planning docs kept landing in commits while `commit_docs` reported false. `validate health` now reports this as W029 with the `git rm -r --cached` remedy. (#3586) diff --git a/CONTEXT.md b/CONTEXT.md index f1846940f..ccdc99b12 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -119,10 +119,10 @@ Module owning the parsed projection of `.planning/` that a diagnostic rule may r Leaf module owning the `SEVERITY`/`REMEDY_ACTION`/`REMEDY_RISK` enums and `Diagnostic`/`Remedy`/`Rule` types shared between the Health Diagnostic Module (the evaluator) and the Health Diagnostic Rule Groups (the eight rule-group files it concatenates). Split out of `src/health-diagnostic.cts` (Phase 11, #3309, ADR-3180 §8.2/§8.3/§8.5) to break a CJS circular dependency: the evaluator must `require()` every rule-group file to populate `RULES`, and every rule-group file needs these enums/types — if the rule-group files required the evaluator back, the require cycle would resolve `module.exports` before it is assigned. This leaf has no runtime dependency on either side of that cycle. Source of truth: `gsd-core/bin/lib/health-diagnostic-types.cjs` (generated from `src/health-diagnostic-types.cts`). ### Health Diagnostic Module -Module owning the frozen rule-table contract for `validate health`, per ADR-3180 §8.2/§8.3/§8.5 (Phase 11, #3309). Exposes three frozen enums — `SEVERITY` (`error`/`warning`/`info`), `REMEDY_ACTION` (the six real repair actions harvested from `cmdValidateHealth`'s existing `--repair` implementation — `createConfig`, `resetConfig`, `regenerateState`, `addNyquistKey`, `addAiIntegrationPhaseKey`, `backfillMilestones` — plus `advise`, the non-repairable payload every non-actionable finding's fix text becomes), and `REMEDY_RISK` (`none`/`destructive`) — plus the `Diagnostic`/`Remedy`/`Rule` shapes every rule's `check(snapshot: PlanningSnapshot) → Diagnostic[]` signature and every finding's `remedy` conform to. `RULES: Rule[]` is the rule table, fully wired: the static concatenation of the 31 rules exported by the eight Health Diagnostic Rule Groups files below (extracted from `cmdValidateHealth`, `src/verify.cts:1616-2577` — 31, not the design doc's own inconsistent prose figure of 32; see the Rule Groups entry's own note). `evaluateRules(snapshot) → Diagnostic[]` runs every rule in `RULES` against one `PlanningSnapshot` and flattens the results, throwing on any two rules sharing a `code` — defense in depth beside the static 1:1 lint guard (§8.2 rule 1, `scripts/lint-health-diagnostic-rule-table.cjs`). `applyRepairs(cwd, diagnostics, repair, backfill) → {applied, refused, details}` is the `--repair`/`--backfill` dispatcher: a `DESTRUCTIVE` remedy (`resetConfig`/`regenerateState` — health.md's own published table: "loses custom settings" / "loses session history") is reported but never executed by `--repair`, a deliberate, disclosed breaking change (§8.3 rule 3) from `cmdValidateHealth`'s current unconditional application; `backfillMilestones` alone among the `NONE`-risk actions is requested by `--backfill` without `--repair`, mirroring `cmdValidateHealth`'s existing gate (`src/verify.cts:2504`). Per-action repair handlers (`runRepairAction`) are REAL, ported behavior-preserving from `verify.cts:2405-2553`'s repair switch — `createConfig`/`resetConfig` (write the default config.json payload), `regenerateState` (backs up and regenerates STATE.md), `addNyquistKey`/`addAiIntegrationPhaseKey` (add a missing `workflow.*` key), `backfillMilestones` (synthesize missing MILESTONES.md entries from archive snapshots) — not stubs. `applied` records only a repair that actually SUCCEEDED (`outcome.success === true`); a thrown or `{success: false}` attempt is recorded in `details` with `success: false` but is never pushed to `applied`. Source of truth: `gsd-core/bin/lib/health-diagnostic.cjs` (generated from `src/health-diagnostic.cts`). Design: `.gsd/phase/refactor-3309-health-diagnostic-rule-table/40-design.md`. +Module owning the frozen rule-table contract for `validate health`, per ADR-3180 §8.2/§8.3/§8.5 (Phase 11, #3309). Exposes three frozen enums — `SEVERITY` (`error`/`warning`/`info`), `REMEDY_ACTION` (the six real repair actions harvested from `cmdValidateHealth`'s existing `--repair` implementation — `createConfig`, `resetConfig`, `regenerateState`, `addNyquistKey`, `addAiIntegrationPhaseKey`, `backfillMilestones` — plus `advise`, the non-repairable payload every non-actionable finding's fix text becomes), and `REMEDY_RISK` (`none`/`destructive`) — plus the `Diagnostic`/`Remedy`/`Rule` shapes every rule's `check(snapshot: PlanningSnapshot) → Diagnostic[]` signature and every finding's `remedy` conform to. `RULES: Rule[]` is the rule table, fully wired: the static concatenation of the 33 rules exported by the eight Health Diagnostic Rule Groups files below (extracted from `cmdValidateHealth`, `src/verify.cts:1616-2577`; the count is locked by `tests/health-diagnostic.test.cjs`'s frozen RULES assertion and by `scripts/lint-health-diagnostic-rule-table.cjs`, so update all three together). `evaluateRules(snapshot) → Diagnostic[]` runs every rule in `RULES` against one `PlanningSnapshot` and flattens the results, throwing on any two rules sharing a `code` — defense in depth beside the static 1:1 lint guard (§8.2 rule 1, `scripts/lint-health-diagnostic-rule-table.cjs`). `applyRepairs(cwd, diagnostics, repair, backfill) → {applied, refused, details}` is the `--repair`/`--backfill` dispatcher: a `DESTRUCTIVE` remedy (`resetConfig`/`regenerateState` — health.md's own published table: "loses custom settings" / "loses session history") is reported but never executed by `--repair`, a deliberate, disclosed breaking change (§8.3 rule 3) from `cmdValidateHealth`'s current unconditional application; `backfillMilestones` alone among the `NONE`-risk actions is requested by `--backfill` without `--repair`, mirroring `cmdValidateHealth`'s existing gate (`src/verify.cts:2504`). Per-action repair handlers (`runRepairAction`) are REAL, ported behavior-preserving from `verify.cts:2405-2553`'s repair switch — `createConfig`/`resetConfig` (write the default config.json payload), `regenerateState` (backs up and regenerates STATE.md), `addNyquistKey`/`addAiIntegrationPhaseKey` (add a missing `workflow.*` key), `backfillMilestones` (synthesize missing MILESTONES.md entries from archive snapshots) — not stubs. `applied` records only a repair that actually SUCCEEDED (`outcome.success === true`); a thrown or `{success: false}` attempt is recorded in `details` with `success: false` but is never pushed to `applied`. Source of truth: `gsd-core/bin/lib/health-diagnostic.cjs` (generated from `src/health-diagnostic.cts`). Design: `.gsd/phase/refactor-3309-health-diagnostic-rule-table/40-design.md`. ### Health Diagnostic Rule Groups -Directory `src/health-diagnostic-rules/` (Phase 11, #3309, ADR-3180 §8.2/§8.3/§8.5) owning the 31 rules migrated off `cmdValidateHealth` (not the design doc's own inconsistent prose figure of 32 — 31 is the count actually summed from each group's exported `RULES` array and locked by `tests/health-diagnostic.test.cjs`'s "RULES" describe block), split into eight files — one per subject-area group from the design doc's "Rule table organization" table — each exporting a `RULES: Rule[]` conforming to the Health Diagnostic Module's frozen `Rule` shape. `src/health-diagnostic.cts` concatenates all eight into the single `RULES` table `evaluateRules` runs; no group re-derives its own `Diagnostic`/`Remedy` shapes. Groups: `root-existence.cts` (root `.planning/` + PROJECT.md existence, E002-E004/W001), `state-consistency.cts` (STATE.md vs config/ROADMAP/disk, W002/W011/W021/W026 — W024's state_head freshness check is a disclosed gap, deliberately not migrated), `config-validation.cts` (config.json shape, W003/W004/W022/E005/W008/W012-W016), `phase-structure.cts` (phase directory structure, W005/W023/I001/W009), `agent-install.cts` (agent-installation completeness, W010), `roadmap-disk-consistency.cts` (ROADMAP-vs-disk phase matching via the shared `matchPhaseDirs` matcher, W006/W007), `worktree-health.cts` (worktree health, W020/W017/W027), `milestone-archive-hygiene.cts` (milestone archive + root hygiene, W018/W019). Every rule is a behavior-preserving port of one `addIssue` call site in `cmdValidateHealth` (`src/verify.cts`), reading only the parsed `PlanningSnapshot` fields the Planning Snapshot Module already computes — never raw `.planning/` I/O. Source of truth: `gsd-core/bin/lib/health-diagnostic-rules/*.cjs` (generated from `src/health-diagnostic-rules/*.cts`). +Directory `src/health-diagnostic-rules/` (Phase 11, #3309, ADR-3180 §8.2/§8.3/§8.5) owning the 33 rules migrated off `cmdValidateHealth` (the count actually summed from each group's exported `RULES` array and locked by `tests/health-diagnostic.test.cjs`'s "RULES" describe block; it was stale at 31 through W028 and is corrected here alongside W029, #3586), split into eight files — one per subject-area group from the design doc's "Rule table organization" table — each exporting a `RULES: Rule[]` conforming to the Health Diagnostic Module's frozen `Rule` shape. `src/health-diagnostic.cts` concatenates all eight into the single `RULES` table `evaluateRules` runs; no group re-derives its own `Diagnostic`/`Remedy` shapes. Groups: `root-existence.cts` (root `.planning/` + PROJECT.md existence, E002-E004/W001), `state-consistency.cts` (STATE.md vs config/ROADMAP/disk, W002/W011/W021/W026 — W024's state_head freshness check is a disclosed gap, deliberately not migrated), `config-validation.cts` (config.json shape, W003/W004/W022/E005/W008/W012-W016, plus W029 — the tracked-but-gitignored `.planning/` contradiction, #3586), `phase-structure.cts` (phase directory structure, W005/W023/I001/W009), `agent-install.cts` (agent-installation completeness, W010), `roadmap-disk-consistency.cts` (ROADMAP-vs-disk phase matching via the shared `matchPhaseDirs` matcher, W006/W007), `worktree-health.cts` (worktree health, W020/W017/W027), `milestone-archive-hygiene.cts` (milestone archive + root hygiene, W018/W019). Every rule is a behavior-preserving port of one `addIssue` call site in `cmdValidateHealth` (`src/verify.cts`), reading only the parsed `PlanningSnapshot` fields the Planning Snapshot Module already computes — never raw `.planning/` I/O. Source of truth: `gsd-core/bin/lib/health-diagnostic-rules/*.cjs` (generated from `src/health-diagnostic-rules/*.cts`). ### Planning Workspace Module Module owning `.planning` path resolution, active workstream pointer policy (`session-scoped > shared`), pointer self-heal behavior, and planning lock semantics for workstream-aware execution. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 0c0d8b937..5183845b6 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -505,6 +505,31 @@ If none match, the starting directory is returned unchanged. Explicit `--project If `.planning/` is in `.gitignore`, `commit_docs` is automatically `false` regardless of config.json. This prevents git errors. +#### Caveat: `.gitignore` does not affect files git already tracks + +Adding `.planning/` to `.gitignore` stops git from picking up **new** files there. It has no effect +on files already committed — git keeps tracking those, so `git add -A` keeps staging them even +though `commit_docs` now resolves to `false`. Because GSD's default is `commit_docs: true`, most +existing projects have already committed `.planning/`, which makes this the common case rather than +the edge case. + +`/gsd-health` reports this contradiction as **`W029`**: + +``` +W029 .planning/ is gitignored but N file(s) are still tracked by git + Fix: git rm -r --cached .planning/ && git commit -m "chore: stop tracking planning docs" +``` + +The warning is advisory. GSD never untracks files for you — `--repair` deliberately will not act on +`W029`, because removing files from the index is destructive and the timing is yours to choose. + +Once you run the `git rm -r --cached` above, `.planning/` is untracked, the ignore rule takes full +effect, and the warning clears. + +Note: a file deliberately force-added under an otherwise-ignored `.planning/` (`git add -f +.planning/keep.md`) triggers this same warning — there is no reliable way to distinguish an +intentional force-add from the accidental case above, so `W029` is expected in that situation too. + --- ## Hook Settings @@ -523,7 +548,7 @@ The prompt injection guard hook (`gsd-prompt-guard.js`) is always active and can ### Private Planning Setup -When `planning.commit_docs` is `false` and `.planning/` is listed in `.gitignore`, GSD treats planning artefacts as local-only. `planning.search_gitignored: true` ensures broad searches still include the `.planning/` directory in this configuration. See [Configure private planning](how-to/configure-model-profiles.md) for setup steps. +When `planning.commit_docs` is `false` and `.planning/` is listed in `.gitignore`, GSD treats planning artifacts as local-only. `planning.search_gitignored: true` ensures broad searches still include the `.planning/` directory in this configuration. See [Keep planning docs out of a shared repo](how-to/keep-planning-docs-private.md) for the full setup, including untracking files git is already tracking. --- diff --git a/docs/README.md b/docs/README.md index 52a4c7057..6d061ee36 100644 --- a/docs/README.md +++ b/docs/README.md @@ -27,6 +27,7 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md) - [Resolve a contract-drift finding](how-to/resolve-contract-drift-findings.md) — bring an agent's completion contract, read-tag gate, or deleted-file test reference back into agreement with the registry - [Resolve unreachable-guard findings](how-to/resolve-unreachable-guard-findings.md) — fix shell guards whose fallback arm cannot run, and tell "nothing to report" apart from "could not look" - [Resolve an ESLint glob-coverage finding](how-to/resolve-eslint-coverage-findings.md) — bring a source file that matches no lint rule under coverage, or record a reasoned exemption +- [Keep planning docs out of a shared repo](how-to/keep-planning-docs-private.md) — make `.planning/` local-only, including untracking files git already tracks (the step `.gitignore` alone cannot do) - [Plan a phase](how-to/plan-a-phase.md) — run research, decompose work, and verify plan quality - [Execute a phase](how-to/execute-a-phase.md) — run plans in parallel waves with fresh-context subagents - [Verify and ship](how-to/verify-and-ship.md) — walk through completed work, diagnose failures, and create the PR diff --git a/docs/how-to/keep-planning-docs-private.md b/docs/how-to/keep-planning-docs-private.md new file mode 100644 index 000000000..293dd28c8 --- /dev/null +++ b/docs/how-to/keep-planning-docs-private.md @@ -0,0 +1,96 @@ +# Keep planning docs out of a shared repo + +You are working in a team repo and want GSD's `.planning/` artifacts — PLAN.md, SUMMARY.md, +ROADMAP.md, STATE.md — to stay on your machine instead of appearing in commits your teammates read. + +This takes four steps, and the third is the one people miss. + +## 1. Turn off doc commits + +```bash +gsd-tools config-set planning.commit_docs false +``` + +`gsd-tools query commit` now returns a `skipped` envelope instead of committing, and every workflow +that writes a planning artifact honors it. + +## 2. Ignore the directory + +Add to `.gitignore`: + +``` +.planning/ +``` + +This is also enough on its own: when `.planning/` is gitignored and `config.json` sets no explicit +value, GSD auto-resolves `commit_docs` to `false`. Setting it explicitly in step 1 is clearer, and +it survives someone later editing `.gitignore`. + +## 3. Untrack what git is already tracking + +**This is the step that catches people out.** `.gitignore` only stops git picking up *new* files. +It has no effect on files already committed — git keeps tracking those, so `git add -A` keeps +staging them even though steps 1 and 2 are both done. + +Because GSD's default is `commit_docs: true`, most existing projects already have `.planning/` +in history, which makes this the common case rather than an edge case. + +```bash +git rm -r --cached .planning/ +git commit -m "chore: stop tracking planning docs" +``` + +`--cached` removes the files from the index only — your files on disk are untouched. + +To check whether this applies to you before running it: + +```bash +git ls-files .planning +``` + +Any output means git is still tracking those paths. + +## 4. Keep search working + +With `.planning/` ignored, tools that respect `.gitignore` stop searching it — including GSD's own +broad searches, which is rarely what you want, since the planning docs are exactly what you want an +agent to read. + +```bash +gsd-tools config-set planning.search_gitignored true +``` + +This adds `--no-ignore` to broad searches so `.planning/` is still found locally. + +## Verify + +```bash +gsd-tools validate health +``` + +A clean result means you are done. If you skipped step 3, you will see: + +``` +W029 .planning/ is gitignored but N file(s) are still tracked by git + Fix: git rm -r --cached .planning/ && git commit -m "chore: stop tracking planning docs" +``` + +`W029` is advisory. GSD will not untrack files for you, and `--repair` deliberately does not act on +it — removing files from the index is destructive and the timing is yours. + +## Notes + +- **A deliberate force-add also raises `W029`.** If you intentionally keep one file tracked under an + otherwise-ignored `.planning/` (`git add -f .planning/decisions.md`), the warning still appears. + There is no reliable way to tell an intentional force-add from the accidental case, so the warning + is expected there too. +- **Teammates who have already pulled the tracked files** will see them deleted by your step-3 + commit. That is the intended effect — the files leave the repo, not their working copies of your + branch — but say so in the commit message or PR description so it is not a surprise. +- **Per-phase control** (committing docs for an architecture phase while keeping execution phases + local) is tracked separately; `commit_docs` is currently project-wide. + +## Related + +- [Configuration reference — `planning.commit_docs`](../CONFIGURATION.md#planning-settings) +- [Configuration reference — auto-detection and the tracked-files caveat](../CONFIGURATION.md#auto-detection) diff --git a/gsd-core/workflows/health.md b/gsd-core/workflows/health.md index da57196ff..c2b63b0cb 100644 --- a/gsd-core/workflows/health.md +++ b/gsd-core/workflows/health.md @@ -251,6 +251,7 @@ Report final status. | W026 | warning | STATE says milestone complete but ROADMAP lists an unstarted phase for that milestone | No | | W027 | warning | Stale git worktree (not modified in a long time) | No | | W028 | warning | A GSD-owned install scope shadows another on this machine | No | +| W029 | warning | .planning/ matches a gitignore rule but is still tracked by git (gitignore has no effect on already-tracked files) | No | | I001 | info | Plan without SUMMARY (may be in progress) | No | | I010 | info | Resolved CWD reported alongside the E010 home-directory guard | No | diff --git a/src/health-diagnostic-rules/config-validation.cts b/src/health-diagnostic-rules/config-validation.cts index 867b28440..dab4f4fbc 100644 --- a/src/health-diagnostic-rules/config-validation.cts +++ b/src/health-diagnostic-rules/config-validation.cts @@ -62,6 +62,10 @@ type Rule = healthDiagnosticMod.Rule; import { VALID_PROFILES, VALID_TIERS, VALID_PHASE_TYPES } from '../model-catalog.cjs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import planningScopeMod = require('../planning-scope.cjs'); +const { SCOPE } = planningScopeMod; + // verify.cts:2141 — inlined literal, not exported from anywhere; same list. const VALID_BRANCHING_STRATEGIES = ['none', 'phase', 'milestone']; @@ -273,6 +277,39 @@ function checkW022(snapshot: PlanningSnapshot): Diagnostic[] { return diagnostics; } +// ─── W029 — `.planning/` ignored but still tracked (#3586, epic #2292) ───── +// +// Not a config.json-sourced check (unlike every other rule in this file) — +// registered here per the phase brief rather than in a new file, reading +// only `snapshot.planningTracked` (`src/planning-snapshot.cts`'s +// `buildPlanningTrackedField`). Fires ONLY when the probe's scope is +// COMPLETE and both `ignored` and `tracked` are true: `.gitignore` has no +// effect on files git already tracks, so a project that committed +// `.planning/` before ignoring it keeps staging those files forever even +// though `commit_docs` correctly auto-resolves `false`. Any other COMPLETE +// combination (most importantly `ignored: false, tracked: true` — the +// default, healthy state of every normal project) is silent; a degraded +// (non-COMPLETE) scope is ALSO silent — a probe that could not run must +// never manufacture a finding. ADVISE-only: auto-untracking is destructive +// and outside this phase's stated boundary (design doc, "Rejected" #3). + +function checkW029(snapshot: PlanningSnapshot): Diagnostic[] { + const field = snapshot.planningTracked; + if (!field || field.scope !== SCOPE.COMPLETE) return []; + if (!field.value.ignored || !field.value.tracked) return []; + return [ + { + code: 'W029', + severity: SEVERITY.WARNING, + message: + '.planning/ matches a gitignore rule but is still tracked by git — .gitignore has no effect on already-tracked files, so staging keeps picking them up', + remedy: adviseRemedy( + 'Run `git rm -r --cached .planning/` to untrack it (the ignore rule then takes effect); this does not delete the files on disk', + ), + }, + ]; +} + // ─── Exports ──────────────────────────────────────────────────────────────── const RULES: Rule[] = [ @@ -329,6 +366,13 @@ const RULES: Rule[] = [ repairable: false, check: checkW022, }, + { + code: 'W029', + severity: SEVERITY.WARNING, + description: '.planning/ matches a gitignore rule but is still tracked by git (gitignore has no effect on already-tracked files)', + repairable: false, + check: checkW029, + }, ]; export = { RULES }; diff --git a/src/planning-snapshot.cts b/src/planning-snapshot.cts index e956f8359..66c52e780 100644 --- a/src/planning-snapshot.cts +++ b/src/planning-snapshot.cts @@ -57,6 +57,9 @@ const { checkAgentsInstalled } = agentInstallCheckMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- worktree-safety.cjs is an export= CommonJS module import worktreeSafetyMod = require('./worktree-safety.cjs'); const { inspectWorktreeHealth } = worktreeSafetyMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- config-loader.cjs is an export= CommonJS module +import configLoaderMod = require('./config-loader.cjs'); +const { isGitIgnored } = configLoaderMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); const { PHASE_NUMBER_TOKEN_SOURCE, OPTIONAL_PHASE_TAG_SOURCE, stripProjectCodePrefix, scopeToPhase } = phaseIdMod; @@ -121,6 +124,11 @@ interface PlanningSnapshot { config: { value: Record | null; scope: Scope; exists: boolean }; agentInstall: { value: ReturnType; scope: Scope }; worktreeHealth: { value: ReturnType['findings']; scope: Scope; reason: string }; + // ─── #3586 (Phase 2, epic #2292) addition ────────────────────────────────── + // Backs W029: detects `.planning/` matching an ignore rule while at least + // one path under it is still tracked by git — see `buildPlanningTrackedField`'s + // own doc comment for the full rationale. + planningTracked: { value: { tracked: boolean; ignored: boolean }; scope: Scope; reason: string }; // ─── Phase 11 (#3309) "Rule table organization" additions ───────────────── // The design doc's own "Rule table organization" table and prose disagree // on the count: the table lists EIGHT rows (through `planningRootFiles`, @@ -452,6 +460,104 @@ function buildWorktreeHealthField(cwd: string): { value: ReturnType 0; + const ignored = isGitIgnored(cwd, '.planning/'); + return { value: { tracked, ignored }, scope: SCOPE.COMPLETE, reason: 'ok' }; + } catch { + return { value: { tracked: false, ignored: false }, scope: SCOPE.UNREADABLE, reason: 'exception' }; + } +} + // ─── Phase 11 (#3309) "Rule table organization" builders ──────────────────── // Each relocates (not reinvents) an existing `verify.cts` derivation. See the // design doc's "Rule table organization" table for the exact source lines. @@ -952,6 +1058,7 @@ function buildPlanningSnapshot(cwd: string): PlanningSnapshot { config: buildConfigField(cwd), agentInstall: buildAgentInstallField(cwd), worktreeHealth: buildWorktreeHealthField(cwd), + planningTracked: buildPlanningTrackedField(cwd), projectSections: buildProjectSectionsField(cwd), statePhaseTokens: stateFields.statePhaseTokens, stateStatus: stateFields.stateStatus, diff --git a/tests/emitted-drift-acks/3309-health-docs-generated.json b/tests/emitted-drift-acks/3309-health-docs-generated.json index c72a3f51e..6d46e536e 100644 --- a/tests/emitted-drift-acks/3309-health-docs-generated.json +++ b/tests/emitted-drift-acks/3309-health-docs-generated.json @@ -2,7 +2,7 @@ "version": 1, "paths": { "health.md": { - "reason": "#3309 (epic #3180 Phase 11, ADR-3180): the ``/`` tables and their footnote are GENERATED by `scripts/gen-health-docs.cjs` from the live `RULES` table, replacing a hand-maintained 16-code table — growth here is a deliberate, expected consequence of that generator doing its job as `RULES` grows, not accidental bloat. #2873 (epic #2866 Phase 4a) adds health rule W028 (\"A GSD-owned install scope shadows another on this machine\"), growing `RULES` from 31 to 32 entries and the generated `` table from 34 to 35 rows (an incremental 84-byte growth on top of #3309's original generation). Regeneration is verified deterministic via `node scripts/gen-health-docs.cjs --check` (wired into `npm run lint:generated-sync`)." + "reason": "#3309 (epic #3180 Phase 11, ADR-3180): the ``/`` tables and their footnote are GENERATED by `scripts/gen-health-docs.cjs` from the live `RULES` table, replacing a hand-maintained 16-code table — growth here is a deliberate, expected consequence of that generator doing its job as `RULES` grows, not accidental bloat. #2873 (epic #2866 Phase 4a) adds health rule W028 (\"A GSD-owned install scope shadows another on this machine\"), growing `RULES` from 31 to 32 entries and the generated `` table from 34 to 35 rows (an incremental 84-byte growth on top of #3309's original generation). #3586 (epic #2292 Phase 2) adds health rule W029 (\"`.planning/` is gitignored but still tracked by git\"), growing `RULES` from 32 to 33 entries and the generated `` table from 35 to 36 rows (a further incremental 141-byte growth). Regeneration is verified deterministic via `node scripts/gen-health-docs.cjs --check` (wired into `npm run lint:generated-sync`). NOTE for the next code added: this file is amended IN PLACE rather than joined by a sibling fragment — `health.md` is generated and grows on every new rule, and two ack sources naming one path is a hard error (`mergeAckSources`), so a per-PR fragment is not an option here. The base-side copy is spent, so the amendment is what makes the ack live for your diff." } } } diff --git a/tests/gen-health-docs.test.cjs b/tests/gen-health-docs.test.cjs index 8370d24a2..24b6c474f 100644 --- a/tests/gen-health-docs.test.cjs +++ b/tests/gen-health-docs.test.cjs @@ -151,9 +151,9 @@ describe('gen-health-docs.cjs --check / --write (CLI, --target fixture)', () => describe('gen-health-docs.cjs row content (representative codes)', () => { const rules = loadRealRules(); - test('produces a 35-row table: 32 rules + 3 pre-checks (E001, E010, I010)', () => { + test('produces a 36-row table: 33 rules + 3 pre-checks (E001, E010, I010)', () => { const rows = buildErrorCodeRows(rules); - assert.equal(rows.length, 35); + assert.equal(rows.length, 36); const codes = rows.map((r) => r.code); for (const precheck of PRECHECK_CODES) { assert.ok(codes.includes(precheck.code), `missing pre-check code ${precheck.code}`); diff --git a/tests/health-diagnostic-rules/config-validation.test.cjs b/tests/health-diagnostic-rules/config-validation.test.cjs index 302defa29..e92bd4469 100644 --- a/tests/health-diagnostic-rules/config-validation.test.cjs +++ b/tests/health-diagnostic-rules/config-validation.test.cjs @@ -39,14 +39,17 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); +const childProcess = require('node:child_process'); -const { createTempDir, cleanup } = require('../helpers.cjs'); +const { createTempDir, createTempGitProject, cleanup } = require('../helpers.cjs'); +const { gitOrThrow } = require('../helpers/git-fixture.cjs'); const configValidation = require('../../gsd-core/bin/lib/health-diagnostic-rules/config-validation.cjs'); const { RULES } = configValidation; const { buildPlanningSnapshot } = require('../../gsd-core/bin/lib/planning-snapshot.cjs'); -const { SEVERITY, REMEDY_ACTION, REMEDY_RISK } = require('../../gsd-core/bin/lib/health-diagnostic.cjs'); +const { SEVERITY, REMEDY_ACTION, REMEDY_RISK, applyRepairs } = require('../../gsd-core/bin/lib/health-diagnostic.cjs'); +const { SCOPE } = require('../../gsd-core/bin/lib/planning-scope.cjs'); // Real shipped default, parsed once — the mutation base for every // MECHANICAL MUTATION fixture below (design doc Fixture provenance §4). @@ -85,16 +88,16 @@ function ruleFor(code) { // ─── RULES shape ──────────────────────────────────────────────────────────── describe('RULES (config-validation group)', () => { - test('exports exactly 10 rules: W003, E005, W004, W008, W016, W012, W013, W014, W015, W022', () => { + test('exports exactly 11 rules: W003, E005, W004, W008, W016, W012, W013, W014, W015, W022, W029', () => { assert.deepEqual( RULES.map((r) => r.code).sort(), - ['E005', 'W003', 'W004', 'W008', 'W012', 'W013', 'W014', 'W015', 'W016', 'W022'].sort(), + ['E005', 'W003', 'W004', 'W008', 'W012', 'W013', 'W014', 'W015', 'W016', 'W022', 'W029'].sort(), ); }); test('E005 is severity ERROR; the rest are severity WARNING', () => { assert.equal(ruleFor('E005').severity, SEVERITY.ERROR); - for (const code of ['W003', 'W004', 'W008', 'W012', 'W013', 'W014', 'W015', 'W016', 'W022']) { + for (const code of ['W003', 'W004', 'W008', 'W012', 'W013', 'W014', 'W015', 'W016', 'W022', 'W029']) { assert.equal(ruleFor(code).severity, SEVERITY.WARNING, `${code} should be WARNING`); } }); @@ -697,3 +700,160 @@ describe('W022 — config.json models malformed', () => { assert.deepEqual(ruleFor('W022').check(snapshot), []); }); }); + +// ─── W029 — .planning/ ignored but still tracked (#3586, epic #2292) ────── +// +// Group B of the test matrix (.gsd/phase/fix-3586-tracked-gitignored-planning/ +// 50-test-matrix.md): the rule is a pure predicate over +// `snapshot.planningTracked` — every fixture here is driven through the REAL +// `buildPlanningSnapshot(cwd)` against a REAL temp git repo, mirroring +// tests/planning-snapshot.test.cjs's own A1-A12 fixtures (same helpers: +// createTempGitProject / gitOrThrow, no hand-built partial snapshots except +// B4, which specifically tests the field-absent-entirely case). + +function writeGitignore(cwd, content) { + fs.writeFileSync(path.join(cwd, '.gitignore'), content); +} + +function commitAll(cwd, message) { + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-m', message], { cwd }); +} + +describe('W029 — .planning/ ignored but still tracked', () => { + test('B1: fires when ignored AND tracked, carrying the git rm -r --cached remedy', (t) => { + const cwd = createTempGitProject('gsd-3586-w029-b1-'); + t.after(() => cleanup(cwd)); + writeGitignore(cwd, '.planning/\n'); + commitAll(cwd, 'add gitignore'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('W029').check(snapshot); + + assert.equal(diagnostics.length, 1); + assert.equal(diagnostics[0].code, 'W029'); + assert.equal(diagnostics[0].severity, SEVERITY.WARNING); + assert.deepEqual(diagnostics[0].remedy.action, REMEDY_ACTION.ADVISE); + assert.deepEqual(diagnostics[0].remedy.risk, REMEDY_RISK.NONE); + assert.match(diagnostics[0].remedy.args.command, /git rm -r --cached \.planning\//); + }); + + test('B2a: tracked + NOT ignored (the default project) is silent', (t) => { + const cwd = createTempGitProject('gsd-3586-w029-b2a-'); + t.after(() => cleanup(cwd)); + // No .gitignore at all. + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(ruleFor('W029').check(snapshot), []); + }); + + test('B2b: ignored + untracked (the healthy ignored state) is silent', (t) => { + const cwd = createTempDir('gsd-3586-w029-b2b-'); + t.after(() => cleanup(cwd)); + gitOrThrow(['init'], { cwd }); + gitOrThrow(['config', 'user.email', 'test@test.com'], { cwd }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd }); + gitOrThrow(['config', 'commit.gpgsign', 'false'], { cwd }); + writeGitignore(cwd, '.planning/\n'); + commitAll(cwd, 'initial (gitignore only)'); + fs.mkdirSync(path.join(cwd, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.planning', 'PROJECT.md'), '# Project\n'); + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(ruleFor('W029').check(snapshot), []); + }); + + test('B2c: neither tracked nor ignored (fresh project) is silent', (t) => { + const cwd = createTempDir('gsd-3586-w029-b2c-'); + t.after(() => cleanup(cwd)); + gitOrThrow(['init'], { cwd }); + gitOrThrow(['config', 'user.email', 'test@test.com'], { cwd }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd }); + gitOrThrow(['config', 'commit.gpgsign', 'false'], { cwd }); + gitOrThrow(['commit', '--allow-empty', '-m', 'initial'], { cwd }); + fs.mkdirSync(path.join(cwd, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.planning', 'PROJECT.md'), '# Project\n'); + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(ruleFor('W029').check(snapshot), []); + }); + + test('B3: a degraded (UNREADABLE) probe produces zero findings — never a finding from a failed probe', (t) => { + const cwd = createTempDir('gsd-3586-w029-b3-'); + t.after(() => cleanup(cwd)); + fs.mkdirSync(path.join(cwd, '.planning'), { recursive: true }); + // No `git init` at all — the probe degrades to UNREADABLE/not_a_git_repo. + + const snapshot = buildPlanningSnapshot(cwd); + assert.notEqual(snapshot.planningTracked.scope, 'complete'); + assert.deepEqual(ruleFor('W029').check(snapshot), []); + }); + + test('B4: field absent from the snapshot entirely is silent, no throw (forward/backward compat)', () => { + const fakeSnapshot = {}; // deliberately missing `planningTracked` + assert.doesNotThrow(() => { + assert.deepEqual(ruleFor('W029').check(fakeSnapshot), []); + }); + }); + + test('B5: the RULES entry marks W029 non-repairable — ADVISE never auto-untracks', () => { + assert.equal(ruleFor('W029').repairable, false); + }); + + // ─── ENOBUFS overflow (#3586 review F2) — the field reads COMPLETE/tracked + // via the overflow-as-proof-of-tracking path, so the rule must still fire. + // Discriminates by argv (mirrors tests/planning-snapshot.test.cjs's own + // `mockGitSpawnEnobufs`) so `isGitIgnored`'s `check-ignore` call still runs + // against the real fixture `.gitignore`. + + test('B1-enobufs: an ENOBUFS-overflowed ls-files still fires W029 (tracked is proof, not a failure)', (t) => { + const cwd = createTempGitProject('gsd-3586-w029-b1-enobufs-'); + t.after(() => cleanup(cwd)); + writeGitignore(cwd, '.planning/\n'); + commitAll(cwd, 'add gitignore'); + + const originalSpawnSync = childProcess.spawnSync; + t.mock.method(childProcess, 'spawnSync', (cmd, args, opts) => { + if (Array.isArray(args) && args.includes('ls-files')) { + return { + status: null, + stdout: '', + stderr: '', + signal: null, + error: Object.assign(new Error('spawnSync git ENOBUFS'), { code: 'ENOBUFS' }), + }; + } + return originalSpawnSync.call(childProcess, cmd, args, opts); + }); + + const snapshot = buildPlanningSnapshot(cwd); + assert.strictEqual(snapshot.planningTracked.value.tracked, true); + assert.strictEqual(snapshot.planningTracked.scope, SCOPE.COMPLETE); + + const diagnostics = ruleFor('W029').check(snapshot); + assert.equal(diagnostics.length, 1); + assert.equal(diagnostics[0].code, 'W029'); + }); + + // ─── C3 — negative-proof: --repair must NOT untrack anything ───────────── + + test('C3: applyRepairs never applies the W029 remedy, and .planning/ stays tracked afterward', (t) => { + const cwd = createTempGitProject('gsd-3586-w029-c3-'); + t.after(() => cleanup(cwd)); + writeGitignore(cwd, '.planning/\n'); + commitAll(cwd, 'add gitignore'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('W029').check(snapshot); + assert.equal(diagnostics.length, 1); + + const result = applyRepairs(cwd, diagnostics, /* repair */ true, /* backfill */ false); + assert.deepEqual(result.applied, []); + assert.deepEqual(result.refused, []); + assert.deepEqual(result.details, []); + + // The tracked file must still be in the index — nothing was untracked. + const trackedAfter = gitOrThrow(['ls-files', '--', '.planning'], { cwd }).trim(); + assert.ok(trackedAfter.length > 0, '.planning/ must still be tracked after a --repair run'); + }); +}); diff --git a/tests/health-diagnostic.test.cjs b/tests/health-diagnostic.test.cjs index a5d0d6802..48ef4b892 100644 --- a/tests/health-diagnostic.test.cjs +++ b/tests/health-diagnostic.test.cjs @@ -274,21 +274,23 @@ describe('evaluateRuleTable — duplicate-code guard (row 13)', () => { // ─── RULES — the fully wired table ────────────────────────────────────────── // -// 32 rule entries. Counted directly from each rule-group file's own exported +// 33 rule entries. Counted directly from each rule-group file's own exported // `RULES` array: root-existence (4: E002/E003/E004/W001) + state-consistency -// (5: W024/W002/W011/W021/W026) + config-validation (10: W003/E005/W004/ -// W008/W016/W012/W013/W014/W015/W022) + phase-structure (4: W005/W023/I001/ -// W009) + agent-install (1: W010) + roadmap-disk-consistency (2: W006/W007) -// + worktree-health (3: W020/W017/W027) + milestone-archive-hygiene (2: -// W018/W019) + install-surface-shadowing (1: W028, #2873 epic #2866 Phase -// 4a) = 32. E001 and the home-directory guard (E010/I010) are deliberately -// NOT rows (design doc, "Two guards that stay OUTSIDE the rule table -// entirely"). +// (5: W024/W002/W011/W021/W026) + config-validation (11: W003/E005/W004/ +// W008/W016/W012/W013/W014/W015/W022/W029) + phase-structure (4: W005/W023/ +// I001/W009) + agent-install (1: W010) + roadmap-disk-consistency (2: W006/ +// W007) + worktree-health (3: W020/W017/W027) + milestone-archive-hygiene +// (2: W018/W019) + install-surface-shadowing (1: W028, #2873 epic #2866 +// Phase 4a) = 33. config-validation.cts's W029 (#3586, epic #2292) is +// registered there per this batch's brief even though its subject +// (`.planning/` tracked-but-ignored) is not config.json-sourced. E001 and +// the home-directory guard (E010/I010) are deliberately NOT rows (design +// doc, "Two guards that stay OUTSIDE the rule table entirely"). describe('RULES', () => { - test('is the full, frozen 32-rule table with every code unique', () => { + test('is the full, frozen 33-rule table with every code unique', () => { assert.equal(Array.isArray(RULES), true); - assert.equal(RULES.length, 32); + assert.equal(RULES.length, 33); const codes = RULES.map((r) => r.code); assert.equal(new Set(codes).size, codes.length, 'every rule code must be unique'); }); diff --git a/tests/planning-snapshot.test.cjs b/tests/planning-snapshot.test.cjs index 69bc8fec1..8740f89d6 100644 --- a/tests/planning-snapshot.test.cjs +++ b/tests/planning-snapshot.test.cjs @@ -29,7 +29,8 @@ const path = require('node:path'); const childProcess = require('node:child_process'); const fc = require('fast-check'); -const { createTempDir, cleanup } = require('./helpers.cjs'); +const { createTempDir, createTempGitProject, cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); // `export =` shape TBD by the implementing phase — this module is "the sole // export consumers reach for" per the design doc, with `worstScope` also @@ -1291,3 +1292,360 @@ describe('Phase-10/11 fields unchanged by the Phase-12 extension (matrix row 8)' assert.deepStrictEqual(snap.perPhaseWaveMissingPlans.value, []); }); }); + +// ═════════════════════════════════════════════════════════════════════════ +// #3586 (Phase 2, epic #2292) — `planningTracked` field +// +// Design: .gsd/phase/fix-3586-tracked-gitignored-planning/40-design.md +// Test matrix: .gsd/phase/fix-3586-tracked-gitignored-planning/50-test-matrix.md +// section A (rows A1-A12) +// +// `.gitignore` has no effect on files git already tracks — this field detects +// the resulting contradiction: `.planning/` matches an ignore rule AND at +// least one path under it is still tracked. Backs W029 +// (`src/health-diagnostic-rules/config-validation.cts`). +// ═════════════════════════════════════════════════════════════════════════ + +function initBareGitRepo(cwd) { + gitOrThrow(['init'], { cwd }); + gitOrThrow(['config', 'user.email', 'test@test.com'], { cwd }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd }); + gitOrThrow(['config', 'commit.gpgsign', 'false'], { cwd }); +} + +function commitAll(cwd, message) { + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-m', message], { cwd }); +} + +function writeGitignore(cwd, content) { + fs.writeFileSync(path.join(cwd, '.gitignore'), content); +} + +// Simulates a timed-out `git ls-files` at the spawnSync seam — mirrors this +// file's existing `mockGitWorktreeListTimeout` convention (t.mock.method, +// auto-restored by node:test, never chmod 0o000). +function mockGitSpawnTimeout(t) { + t.mock.method(childProcess, 'spawnSync', () => ({ + status: null, + stdout: '', + stderr: '', + signal: null, + error: Object.assign(new Error('spawnSync git ETIMEDOUT'), { code: 'ETIMEDOUT' }), + })); +} + +// Simulates `git ls-files` exiting non-zero while the structural +// `rev-parse --is-inside-work-tree` probe (the builder's OWN follow-up call +// on the failure path, #3586) still succeeds — i.e. a real repo, but the +// `ls-files` invocation failed for some other reason. Differentiates by argv +// rather than blanket-mocking every spawnSync call, since the builder now +// issues a second git subprocess on this path. +function mockGitSpawnFailure(t) { + t.mock.method(childProcess, 'spawnSync', (_cmd, args) => { + if (Array.isArray(args) && args.includes('rev-parse')) { + return { status: 0, stdout: 'true', stderr: '', signal: null, error: null }; + } + return { status: 128, stdout: '', stderr: 'fatal: some other git failure', signal: null, error: null }; + }); +} + +// Simulates `git ls-files` overflowing spawnSync's Node-default 1MB stdout +// buffer (#3586 review F2) — Node reports this as `error.code === 'ENOBUFS'`, +// `status: null`. Discriminates by argv (mirrors `mockGitSpawnFailure` above) +// so the `check-ignore` call `isGitIgnored` issues on the same happy path +// still reaches the REAL spawnSync against the fixture's real `.gitignore` — +// a blanket mock here would falsely report `ignored: false` too. +function mockGitSpawnEnobufs(t) { + const originalSpawnSync = childProcess.spawnSync; + t.mock.method(childProcess, 'spawnSync', (cmd, args, opts) => { + if (Array.isArray(args) && args.includes('ls-files')) { + return { + status: null, + stdout: '', + stderr: '', + signal: null, + error: Object.assign(new Error('spawnSync git ENOBUFS'), { code: 'ENOBUFS' }), + }; + } + return originalSpawnSync.call(childProcess, cmd, args, opts); + }); +} + +// Simulates a directory that is genuinely not inside any git work tree: both +// `ls-files` AND the structural `rev-parse --is-inside-work-tree` probe fail. +// Used to prove the `not_a_git_repo` classification does NOT depend on +// stderr content — both calls here carry EMPTY stderr. +function mockGitSpawnNotARepoNoStderr(t) { + t.mock.method(childProcess, 'spawnSync', () => ({ + status: 128, + stdout: '', + stderr: '', + signal: null, + error: null, + })); +} + +describe('planningTracked field (#3586, matrix rows A1-A12)', () => { + test('A1: ignored + tracked is detected', (t) => { + const cwd = createTempGitProject('gsd-3586-a1-'); + t.after(() => cleanup(cwd)); + writeGitignore(cwd, '.planning/\n'); + commitAll(cwd, 'add gitignore'); + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.planningTracked, { + value: { tracked: true, ignored: true }, + scope: SCOPE.COMPLETE, + reason: 'ok', + }); + }); + + test('A2: ignored + untracked is clean', (t) => { + const cwd = createTempDir('gsd-3586-a2-'); + t.after(() => cleanup(cwd)); + initBareGitRepo(cwd); + writeGitignore(cwd, '.planning/\n'); + commitAll(cwd, 'initial (gitignore only)'); + fs.mkdirSync(path.join(cwd, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.planning', 'PROJECT.md'), '# Project\n'); + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.planningTracked, { + value: { tracked: false, ignored: true }, + scope: SCOPE.COMPLETE, + reason: 'ok', + }); + }); + + test('A3: tracked + NOT ignored is the normal (default) project — must NOT read as detected', (t) => { + const cwd = createTempGitProject('gsd-3586-a3-'); + t.after(() => cleanup(cwd)); + // No .gitignore at all — the default state of essentially every project. + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.planningTracked, { + value: { tracked: true, ignored: false }, + scope: SCOPE.COMPLETE, + reason: 'ok', + }); + }); + + test('A4: fresh project (untracked, not ignored) is clean', (t) => { + const cwd = createTempDir('gsd-3586-a4-'); + t.after(() => cleanup(cwd)); + initBareGitRepo(cwd); + gitOrThrow(['commit', '--allow-empty', '-m', 'initial'], { cwd }); + fs.mkdirSync(path.join(cwd, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.planning', 'PROJECT.md'), '# Project\n'); + // Deliberately never `git add`. + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.planningTracked, { + value: { tracked: false, ignored: false }, + scope: SCOPE.COMPLETE, + reason: 'ok', + }); + }); + + test('A5: not a git repo degrades to UNREADABLE/not_a_git_repo, silently', (t) => { + const cwd = createTempDir('gsd-3586-a5-'); + t.after(() => cleanup(cwd)); + fs.mkdirSync(path.join(cwd, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.planning', 'PROJECT.md'), '# Project\n'); + // No `git init` at all. + + const snap = buildPlanningSnapshot(cwd); + assert.strictEqual(snap.planningTracked.scope, SCOPE.UNREADABLE); + assert.strictEqual(snap.planningTracked.reason, 'not_a_git_repo'); + }); + + test('A6: git ls-files timeout degrades quietly (no throw)', (t) => { + const cwd = createTempGitProject('gsd-3586-a6-'); + t.after(() => cleanup(cwd)); + mockGitSpawnTimeout(t); + + let snap; + assert.doesNotThrow(() => { + snap = buildPlanningSnapshot(cwd); + }); + assert.strictEqual(snap.planningTracked.scope, SCOPE.UNREADABLE); + assert.strictEqual(snap.planningTracked.reason, 'git_timed_out'); + }); + + test('A7: git ls-files non-zero exit degrades quietly (no throw)', (t) => { + const cwd = createTempGitProject('gsd-3586-a7-'); + t.after(() => cleanup(cwd)); + mockGitSpawnFailure(t); + + let snap; + assert.doesNotThrow(() => { + snap = buildPlanningSnapshot(cwd); + }); + assert.strictEqual(snap.planningTracked.scope, SCOPE.UNREADABLE); + assert.strictEqual(snap.planningTracked.reason, 'git_list_failed'); + }); + + test('A8: .planning/ absent from disk is silent (untracked, not ignored)', (t) => { + const cwd = createTempDir('gsd-3586-a8-'); + t.after(() => cleanup(cwd)); + initBareGitRepo(cwd); + gitOrThrow(['commit', '--allow-empty', '-m', 'initial'], { cwd }); + // .planning/ never created at all. + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.planningTracked, { + value: { tracked: false, ignored: false }, + scope: SCOPE.COMPLETE, + reason: 'ok', + }); + }); + + test('A9: exactly ONE tracked file under an ignored .planning/ is enough to detect', (t) => { + const cwd = createTempGitProject('gsd-3586-a9-'); + t.after(() => cleanup(cwd)); + // createTempGitProject seeds exactly .planning/PROJECT.md — one file. + writeGitignore(cwd, '.planning/\n'); + commitAll(cwd, 'add gitignore'); + + const snap = buildPlanningSnapshot(cwd); + assert.strictEqual(snap.planningTracked.value.tracked, true); + assert.strictEqual(snap.planningTracked.value.ignored, true); + }); + + test('A10: tracked in the index but deleted from the worktree is still detected', (t) => { + const cwd = createTempGitProject('gsd-3586-a10-'); + t.after(() => cleanup(cwd)); + writeGitignore(cwd, '.planning/\n'); + commitAll(cwd, 'add gitignore'); + // Delete the tracked file from disk WITHOUT `git rm` — it stays in the index. + // (fs.unlinkSync, not fs.rmSync — this is a single tracked file, not a + // directory teardown, so the Windows-EBUSY-retry rmSync rule does not apply.) + fs.unlinkSync(path.join(cwd, '.planning', 'PROJECT.md')); + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.planningTracked.value, { tracked: true, ignored: true }); + }); + + test('A11: ignored via .git/info/exclude counts the same as .gitignore', (t) => { + const cwd = createTempGitProject('gsd-3586-a11-'); + t.after(() => cleanup(cwd)); + // Local-only, never committed — check-ignore honors it all the same. + fs.appendFileSync(path.join(cwd, '.git', 'info', 'exclude'), '.planning/\n'); + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.planningTracked.value, { tracked: true, ignored: true }); + }); + + test('A12: a .gitignore covering only .planning/cache/ does NOT count as the root ignored (sub-path ignore is not the contradictory state)', (t) => { + const cwd = createTempGitProject('gsd-3586-a12-'); + t.after(() => cleanup(cwd)); + writeGitignore(cwd, '.planning/cache/\n'); + commitAll(cwd, 'add sub-path gitignore'); + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.planningTracked.value, { tracked: true, ignored: false }); + }); + + // ─── Locale-independence regression (#3586) ──────────────────────────────── + // The original defect: `not_a_git_repo` vs `git_list_failed` was + // distinguished by regex-matching git's (localized) stderr prose. Under a + // non-English `LANG`/`LC_ALL`, git prints the translated string and the + // regex misses, misclassifying a plain non-git directory as + // `git_list_failed`. The fix asks git a structural yes/no question + // (`rev-parse --is-inside-work-tree`) instead of parsing stderr. + + test('A5-locale: not-a-git-repo classification holds with a non-English LANG/LC_ALL set', (t) => { + const cwd = createTempDir('gsd-3586-a5-locale-'); + t.after(() => cleanup(cwd)); + fs.mkdirSync(path.join(cwd, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(cwd, '.planning', 'PROJECT.md'), '# Project\n'); + + const savedLang = process.env['LANG']; + const savedLcAll = process.env['LC_ALL']; + process.env['LANG'] = 'de_DE.UTF-8'; + process.env['LC_ALL'] = 'de_DE.UTF-8'; + t.after(() => { + if (savedLang === undefined) delete process.env['LANG']; + else process.env['LANG'] = savedLang; + if (savedLcAll === undefined) delete process.env['LC_ALL']; + else process.env['LC_ALL'] = savedLcAll; + }); + + const snap = buildPlanningSnapshot(cwd); + assert.strictEqual(snap.planningTracked.scope, SCOPE.UNREADABLE); + assert.strictEqual(snap.planningTracked.reason, 'not_a_git_repo'); + // NOTE: this machine's `git` (Apple Git 2.50.1) ships no German NLS + // catalog (verified: no `.mo` translation file under its install), so + // this run does not actually exercise TRANSLATED stderr text — git still + // prints English here even with the locale env vars set. This test + // therefore does not, by itself, prove locale-independence; the test + // below (A5-structural) is the real regression coverage: it fails + // against the old stderr-regex implementation regardless of what + // translations happen to be installed on the machine running the suite. + }); + + test('A5-structural: not_a_git_repo classification does not depend on stderr content (regression for #3586)', (t) => { + const cwd = createTempDir('gsd-3586-a5-structural-'); + t.after(() => cleanup(cwd)); + mockGitSpawnNotARepoNoStderr(t); + + const snap = buildPlanningSnapshot(cwd); + // Both `ls-files` and the structural `rev-parse` probe fail here with + // EMPTY stderr (no "not a git repository" string anywhere) — the old + // stderr-regex implementation would have misclassified this as + // `git_list_failed`. The fix classifies structurally from the probe's + // exit code alone, so this still correctly reads `not_a_git_repo`. + assert.strictEqual(snap.planningTracked.scope, SCOPE.UNREADABLE); + assert.strictEqual(snap.planningTracked.reason, 'not_a_git_repo'); + }); + + test('happy path issues exactly ONE git subprocess for planningTracked (no rev-parse probe when ls-files succeeds)', (t) => { + const cwd = createTempGitProject('gsd-3586-one-call-'); + t.after(() => cleanup(cwd)); + + const originalSpawnSync = childProcess.spawnSync; + let lsFilesCalls = 0; + let revParseProbeCalls = 0; + t.mock.method(childProcess, 'spawnSync', (cmd, args, opts) => { + if (cmd === 'git' && Array.isArray(args)) { + if (args.includes('ls-files')) lsFilesCalls += 1; + if (args.includes('rev-parse') && args.includes('--is-inside-work-tree')) revParseProbeCalls += 1; + } + return originalSpawnSync.call(childProcess, cmd, args, opts); + }); + + const snap = buildPlanningSnapshot(cwd); + assert.strictEqual(snap.planningTracked.scope, SCOPE.COMPLETE); + assert.strictEqual(lsFilesCalls, 1, 'ls-files must run exactly once on the happy path'); + assert.strictEqual( + revParseProbeCalls, + 0, + 'the rev-parse probe must NOT run when ls-files succeeds — the happy path stays a single subprocess', + ); + }); + + // ─── ENOBUFS overflow (#3586 review F2) ──────────────────────────────────── + // `execGit` sets no `maxBuffer`, so a `.planning/` tree with enough tracked + // paths to exceed spawnSync's Node-default 1MB stdout cap makes `ls-files` + // fail with `error.code === 'ENOBUFS'`. Treating that like any other + // non-zero-exit failure (scope UNREADABLE, silent downstream) would hide + // W029 in exactly the large-tracked-history case it exists to catch — the + // overflow itself is proof `ls-files` had non-empty output, so `tracked` + // must read `true`, not degrade. + + test('A13: git ls-files ENOBUFS overflow is treated as proof of tracking, not a failure', (t) => { + const cwd = createTempGitProject('gsd-3586-a13-enobufs-'); + t.after(() => cleanup(cwd)); + writeGitignore(cwd, '.planning/\n'); + commitAll(cwd, 'add gitignore'); + mockGitSpawnEnobufs(t); + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.planningTracked, { + value: { tracked: true, ignored: true }, + scope: SCOPE.COMPLETE, + reason: 'ok_truncated', + }); + }); +}); diff --git a/tests/verify-health.test.cjs b/tests/verify-health.test.cjs index 5caa9e7af..9fe381c7b 100644 --- a/tests/verify-health.test.cjs +++ b/tests/verify-health.test.cjs @@ -22,6 +22,7 @@ const fs = require('fs'); const path = require('path'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); const { VALID_PROFILES } = require('../gsd-core/bin/lib/model-catalog.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); // ─── Helpers for setting up minimal valid projects ──────────────────────────── @@ -1549,3 +1550,74 @@ describe('validate health — #1454 W017 excludes active worktree', () => { ); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// W029 — .planning/ tracked-and-gitignored (#3586, epic #2292) +// +// Test matrix: .gsd/phase/fix-3586-tracked-gitignored-planning/50-test-matrix.md +// section C (C1/C2) — CLI-LEVEL integration, through the real `gsd-tools +// validate health` dispatch path (unlike tests/health-diagnostic-rules/ +// config-validation.test.cjs's B-group, which drives `ruleFor('W029').check` +// directly against a hand-built snapshot). These two prove W029 is actually +// REACHABLE end-to-end — registered in RULES is not the same claim as wired +// into `cmdValidateHealth`'s JSON output. +// +// Asserted on the parsed JSON's `code` field only (never rendered text, per +// CONTRIBUTING's ban on raw-text output matching) — the real CLI response +// shape carries no per-diagnostic `severity` field; severity is expressed +// structurally by which top-level array (`errors` vs `warnings`) an entry +// lands in, so "W029 is in `warnings`" already encodes SEVERITY.WARNING. +// ───────────────────────────────────────────────────────────────────────────── + +describe('validate health — W029 .planning/ tracked-and-gitignored (#3586)', () => { + function initGitRepo(cwd) { + gitOrThrow(['init'], { cwd }); + gitOrThrow(['config', 'user.email', 'test@test.com'], { cwd }); + gitOrThrow(['config', 'user.name', 'Test'], { cwd }); + gitOrThrow(['config', 'commit.gpgsign', 'false'], { cwd }); + } + + function commitAll(cwd, message) { + gitOrThrow(['add', '-A'], { cwd }); + gitOrThrow(['commit', '-m', message], { cwd }); + } + + test('C1: healthReportsTrackedIgnoredState — a temp git project with .planning/ committed then gitignored reports W029', () => { + const cwd = createTempProject(); + // createTempProject() writes only an EMPTY .planning/phases/ dir — git + // never tracks an empty directory, so without a real file under + // .planning/ the initial commit would track nothing and this fixture + // could never reach the "tracked" half of the contradiction. + fs.writeFileSync(path.join(cwd, '.planning', 'PROJECT.md'), '# Project\n'); + initGitRepo(cwd); + commitAll(cwd, 'initial commit — .planning/ tracked'); + fs.writeFileSync(path.join(cwd, '.gitignore'), '.planning/\n'); + commitAll(cwd, 'add gitignore — .planning/ now contradictorily ignored-but-tracked'); + + const result = runGsdTools('validate health', cwd); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const w029 = output.warnings.find(w => w.code === 'W029'); + assert.ok(w029, `Expected W029 in warnings: ${JSON.stringify(output.warnings)}`); + assert.strictEqual(w029.code, 'W029'); + }); + + test('C2: healthSilentOnHealthyProject — the same project layout with .planning/ tracked and NOT ignored reports no W029', () => { + const cwd = createTempProject(); + fs.writeFileSync(path.join(cwd, '.planning', 'PROJECT.md'), '# Project\n'); + initGitRepo(cwd); + commitAll(cwd, 'initial commit — .planning/ tracked, no .gitignore at all'); + // No .gitignore written — the default, healthy state of essentially + // every GSD project (mirrors the B2a/A3 fixture convention). + + const result = runGsdTools('validate health', cwd); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.ok( + !output.warnings.some(w => w.code === 'W029'), + `Should not have W029 on a healthy tracked-not-ignored project: ${JSON.stringify(output.warnings)}` + ); + }); +});