* feat(#3586): warn when .planning/ is gitignored but still tracked git ignore rules have no effect on files git already tracks, so a project that committed .planning/ before ignoring it keeps staging those files -- while commit_docs correctly resolves to false, which is exactly what makes the contradiction invisible. The probe lives in the SNAPSHOT BUILDER, not the rule: Rule.check may perform no ambient I/O (ADR-3180 8.1 rule 1, enforced by lint-planning-snapshot-bypass). buildPlanningTrackedField follows buildWorktreeHealthField's precedent -- injected execGit, bounded, degrading to UNREADABLE with a typed reason rather than throwing. W024 went inline instead only because no snapshot field carried its fact; that precondition does not apply here. W029 fires only on COMPLETE scope with ignored and tracked both true, so a degraded probe yields neither a finding nor a false all-clear, and the default project (tracked, not ignored) stays silent. The remedy is ADVISE-only -- --repair never untracks anything. * docs(#3586): document W029 and correct the health rule count CONFIGURATION.md documented the gitignore auto-detect without the caveat that ignore rules do not affect already-tracked files -- the very gap W029 exists to surface. Adds the caveat, the warning, its remedy, and why --repair will not act on it. CONTEXT.md's rule count was stale at 31 before this change (actual 32 through W028); corrected to 33 and pointed at the two other places the count is locked, so the next editor updates all three together. * fix(#3586): treat ls-files overflow as tracked, add CLI-level W029 tests Review findings. Security (minor, confirmed): execGit sets no maxBuffer, so Node's 1MB default applies to git ls-files. A .planning/ tree large enough to overflow it failed into git_list_failed and silenced W029 -- a false negative in exactly the large-history case most likely to have the real bug. Overflow is now treated as PROOF of tracking (the output was non-empty by definition) and resolves to tracked:true, scope COMPLETE, reason ok_truncated. Spec (major): test-matrix rows C1 and C2 were never implemented -- there was no CLI-level integration test at all, only rule-level ones. Both now drive the real validate-health dispatch and confirm W029 is reachable end-to-end. Known limit documented, not papered over: a deliberate git add -f under an otherwise-ignored .planning/ raises the same signal as the accidental case. There is no reliable way to tell them apart, the finding is advisory-only, and a heuristic that cannot actually distinguish them would be worse than the honest caveat. * test(#3586): update frozen health-doc counts and acknowledge health.md growth The remote matrix caught three gates that lint:ci does not cover. gen-health-docs.test.cjs froze a 35-row / 32-rule assertion; W029 makes it 36/33. Updated both the assertion and the test NAME, which embeds the counts -- a stale name is a lie even when the assertion passes. The second reported failure was the same assertion surfacing at describe-rollup granularity, not a distinct bug. emitted-attribution's growth arm needed an ack for the generated health.md. health.md was already named in 3309-health-docs-generated.json, and two ack sources naming one path is a hard error -- so a new fragment was not an option. That fragment's own history shows the pattern: #3309 created it, #2873 amended it in place for W028. Amended again for W029, with a note recording why this one file is amended rather than joined by a sibling. * docs(#3586): add the private-planning how-to and fix a wrong link docs/CONFIGURATION.md pointed 'Configure private planning' at how-to/configure-model-profiles.md -- an unrelated page -- and no private-planning how-to existed at all. Found while editing that section. The how-to test genuinely fires here: going private is four steps and crosses planning.search_gitignored, a setting owned by another concern, so a reference table structurally cannot carry it. The new page walks the whole sequence and leads with the step people miss -- .gitignore does not untrack what git already tracks -- which is the exact state W029 now detects. Also corrects 'artefacts' to 'artifacts' (repo house style is American). * chore(#3586): backfill changeset pr number to 3598 --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/tidy-bears-chatter.md
Normal file
5
.changeset/tidy-bears-chatter.md
Normal file
@@ -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)
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
96
docs/how-to/keep-planning-docs-private.md
Normal file
96
docs/how-to/keep-planning-docs-private.md
Normal file
@@ -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)
|
||||
@@ -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 |
|
||||
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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<string, unknown> | null; scope: Scope; exists: boolean };
|
||||
agentInstall: { value: ReturnType<typeof checkAgentsInstalled>; scope: Scope };
|
||||
worktreeHealth: { value: ReturnType<typeof inspectWorktreeHealth>['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<typeof inspe
|
||||
}
|
||||
}
|
||||
|
||||
// #3586 (Phase 2, epic #2292): matches `execGit`'s own default timeout
|
||||
// (`shell-command-projection.cts:628`, also `10_000`) — kept as an explicit
|
||||
// named constant here (rather than omitting `timeout` and relying on that
|
||||
// default silently) so this call site's bound is self-documenting; generous
|
||||
// enough for a normal repo, bounded enough to degrade rather than stall
|
||||
// `buildPlanningSnapshot`, which every health path calls.
|
||||
const PLANNING_TRACKED_GIT_TIMEOUT_MS = 10_000;
|
||||
|
||||
/**
|
||||
* Resolve `planningTracked` — whether `.planning/` matches a gitignore rule
|
||||
* AND whether at least one path under it is still tracked by git (#3586,
|
||||
* Phase 2 of epic #2292). `.gitignore` has no effect on files git already
|
||||
* tracks, so a project that committed `.planning/` before ignoring it keeps
|
||||
* staging those files forever — while `commit_docs` auto-resolves `false`
|
||||
* (`isGitIgnored`, reused below, is exactly what that resolution consults),
|
||||
* which is what makes the contradiction invisible. Backs W029
|
||||
* (`src/health-diagnostic-rules/config-validation.cts`).
|
||||
*
|
||||
* Modeled directly on `buildWorktreeHealthField` above (ADR-3180 §8.1 rule 1:
|
||||
* a `Rule.check(snapshot)` may perform no ambient I/O, so this `git ls-files`
|
||||
* probe lives here, in the snapshot builder, not the rule).
|
||||
*
|
||||
* `tracked` runs `git ls-files -- .planning` through the module's own
|
||||
* injected `execGit` seam: non-empty stdout means at least one path under
|
||||
* `.planning/` is in the INDEX — worktree presence is irrelevant (a path
|
||||
* tracked in the index but deleted on disk still counts; that is what "the
|
||||
* index is what matters" means for this probe).
|
||||
*
|
||||
* `ignored` reuses `isGitIgnored` (`config-loader.cjs`) — the SAME
|
||||
* `git check-ignore -q --no-index` resolution `commit_docs` auto-resolution
|
||||
* already calls (`config-loader.cts:824`) — rather than a second,
|
||||
* independently-drifting `check-ignore` invocation. Only computed once the
|
||||
* `ls-files` probe itself succeeded; a probe that could not run has no
|
||||
* grounds to ask a second question either.
|
||||
*
|
||||
* Degradation mirrors `buildWorktreeHealthField` exactly: not a git repo →
|
||||
* `SCOPE.UNREADABLE` + reason `not_a_git_repo` (silent downstream — matches
|
||||
* the sibling builder's deliberate treatment of a `.planning/`-only
|
||||
* fixture/tmp dir with no git repo at all); a timed-out `ls-files` →
|
||||
* `git_timed_out`; any other non-zero exit → `git_list_failed`; a thrown
|
||||
* exception → `exception`. Never throws out of the builder.
|
||||
*
|
||||
* Repo-presence is determined STRUCTURALLY, not by reading `ls-files`'s
|
||||
* stderr prose (#3586): git localizes its error text (`LANG`/`LC_ALL`), so a
|
||||
* regex matching the English "not a git repository" string silently
|
||||
* misclassifies `not_a_git_repo` as `git_list_failed` under any non-English
|
||||
* locale — this is exactly the "raw text matching on subprocess output"
|
||||
* `CONTRIBUTING.md` bans, applied to production code rather than a test.
|
||||
* Instead, on the `ls-files` failure path ONLY (never on the happy path —
|
||||
* `buildPlanningSnapshot` runs on every health invocation, and the happy
|
||||
* path must stay a single subprocess call), this asks git a structural
|
||||
* yes/no question via `git rev-parse --is-inside-work-tree`: exit 0 means we
|
||||
* ARE inside a work tree, so the `ls-files` failure was something else →
|
||||
* `git_list_failed`; a non-zero exit means we are NOT → `not_a_git_repo`.
|
||||
* If that probe itself times out, `result.timedOut` (the shared
|
||||
* `isSpawnTimeout` predicate) reports `git_timed_out` — not a hand-rolled
|
||||
* timeout check.
|
||||
*
|
||||
* `ENOBUFS` overflow (#3586 review F2): `execGit` sets no `maxBuffer`, so
|
||||
* `spawnSync`'s Node-default 1MB cap applies to `ls-files`' stdout. A
|
||||
* `.planning/` tree with enough tracked paths to exceed 1MB makes
|
||||
* `spawnSync` report `error.code === 'ENOBUFS'` — exactly the large-tracked-
|
||||
* history case this probe exists to catch, and exactly the case most likely
|
||||
* to legitimately overflow the buffer. Falling into the generic
|
||||
* non-zero-exit path here would misclassify it as `git_list_failed` →
|
||||
* `SCOPE.UNREADABLE`, silently dropping the finding in precisely the
|
||||
* scenario where it matters most. Overflow is therefore treated as PROOF OF
|
||||
* TRACKING, not as a degraded read: `ls-files` only overflows because it had
|
||||
* non-empty output to begin with, so `tracked` is unconditionally `true` —
|
||||
* detected BEFORE the generic `exitCode !== 0` branch below, so this case
|
||||
* never reaches (and never pays for) the `rev-parse` structural probe.
|
||||
*/
|
||||
function buildPlanningTrackedField(cwd: string): { value: { tracked: boolean; ignored: boolean }; scope: Scope; reason: string } {
|
||||
try {
|
||||
const result = execGit(['ls-files', '--', '.planning'], { cwd, timeout: PLANNING_TRACKED_GIT_TIMEOUT_MS });
|
||||
if (result.timedOut) {
|
||||
return { value: { tracked: false, ignored: false }, scope: SCOPE.UNREADABLE, reason: 'git_timed_out' };
|
||||
}
|
||||
if ((result.error as NodeJS.ErrnoException | null | undefined)?.code === 'ENOBUFS') {
|
||||
const ignored = isGitIgnored(cwd, '.planning/');
|
||||
return { value: { tracked: true, ignored }, scope: SCOPE.COMPLETE, reason: 'ok_truncated' };
|
||||
}
|
||||
if (result.exitCode !== 0) {
|
||||
const probe = execGit(['rev-parse', '--is-inside-work-tree'], { cwd, timeout: PLANNING_TRACKED_GIT_TIMEOUT_MS });
|
||||
if (probe.timedOut) {
|
||||
return { value: { tracked: false, ignored: false }, scope: SCOPE.UNREADABLE, reason: 'git_timed_out' };
|
||||
}
|
||||
const reason = probe.exitCode === 0 ? 'git_list_failed' : 'not_a_git_repo';
|
||||
return { value: { tracked: false, ignored: false }, scope: SCOPE.UNREADABLE, reason };
|
||||
}
|
||||
const tracked = result.stdout.trim().length > 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,
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"health.md": {
|
||||
"reason": "#3309 (epic #3180 Phase 11, ADR-3180): the `<error_codes>`/`<repair_actions>` 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 `<error_codes>` 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 `<error_codes>`/`<repair_actions>` 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 `<error_codes>` 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 `<error_codes>` 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."
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 <error_codes> table: 32 rules + 3 pre-checks (E001, E010, I010)', () => {
|
||||
test('produces a 36-row <error_codes> 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}`);
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
|
||||
@@ -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',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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)}`
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user