diff --git a/.changeset/graceful-dogs-tumble.md b/.changeset/graceful-dogs-tumble.md new file mode 100644 index 000000000..4a9e2468e --- /dev/null +++ b/.changeset/graceful-dogs-tumble.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3407 +--- +**`validate consistency`'s `warnings` are now coded diagnostics** — each entry is a `{code, message, fix, repairable}` object instead of a bare string. Findings that overlap with `validate health` (a phase in ROADMAP.md with no directory on disk, or vice versa) now carry the exact same `W006`/`W007` codes `validate health` already uses for them, so there's one vocabulary for that finding, not two. The four subjects unique to this command (phase/plan numbering gaps, orphan summaries, plans missing `wave` frontmatter) get a new `C001`-`C004` code range. diff --git a/.changeset/happy-bears-hop.md b/.changeset/happy-bears-hop.md new file mode 100644 index 000000000..d6395c672 --- /dev/null +++ b/.changeset/happy-bears-hop.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3407 +--- +**`state validate`'s `warnings` are now coded diagnostics, and the `drift` field is gone** — each entry is a `{code, severity, message, remedy}` object (seven codes, `S001`-`S007`) naming exactly what STATE.md disagrees with the filesystem about and how to fix it, instead of a bare string. The separate `drift` object every response used to carry is removed entirely; every condition it used to report (a conflicting phase reference, a missing phases directory, a plan-count mismatch, a stale executing status) is now one of the seven coded warnings, so no information is lost, it's just structured. `valid` and `scope` are unchanged. diff --git a/.gitignore b/.gitignore index 348012479..8770a8ed8 100644 --- a/.gitignore +++ b/.gitignore @@ -206,6 +206,7 @@ build/ /gsd-core/bin/lib/health-diagnostic-rules/roadmap-disk-consistency.cjs /gsd-core/bin/lib/health-diagnostic-rules/worktree-health.cjs /gsd-core/bin/lib/health-diagnostic-rules/milestone-archive-hygiene.cjs +/gsd-core/bin/lib/health-diagnostic-rules/consistency.cjs /gsd-core/bin/lib/command-roster.cjs /gsd-core/bin/lib/runtime-artifact-conversion.cjs /gsd-core/bin/lib/runtime-artifact-layout.cjs diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 7aa6bdeb2..2dd5d4b22 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -567,6 +567,8 @@ node gsd-tools.cjs validate context node gsd-tools.cjs validate context --json ``` +`validate consistency`'s `warnings` entries are coded diagnostics (`{code, message, fix, repairable}`), not bare strings. A phase declared in ROADMAP.md with no directory on disk, or a directory on disk with no ROADMAP.md entry, reports under `W006`/`W007` — the same codes `validate health` uses for the identical check, since it's one enumeration with two callers, not a separate check. The four subjects unique to this command (numbering gaps in phases or plans, orphan `*-SUMMARY.md` files, plans missing `wave` frontmatter) use a new `C0NN` code range (`C001`-`C004`). + `validate context` emits a structured envelope with `utilization`, `status` (`ok` / `warn` / `critical` at the 60 % / 70 % thresholds), and a `suggestion` string. The same data backs `/gsd-health --context`. diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 4694bda33..98be6f94f 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -1835,7 +1835,7 @@ Presence and posture are separate verdicts: a missing agent is reported in `miss Detect drift between STATE.md and the actual filesystem. **Prerequisites:** `.planning/STATE.md` exists -**Produces:** Validation report showing any drift between STATE.md fields and filesystem reality +**Produces:** Validation report showing any drift between STATE.md fields and filesystem reality, as coded diagnostics ```bash node gsd-tools.cjs state validate @@ -1845,12 +1845,24 @@ The report also carries a `scope` field reporting whether the drift derivation c | `scope` | Meaning | |---|---| -| `complete` | The derivation ran over usable input — a resolvable phase, a readable disk scan. `valid`/`warnings`/`drift` are a real answer. | +| `complete` | The derivation ran over usable input — a resolvable phase, a readable disk scan. `valid`/`warnings` are a real answer. | | `truncated` | Part of the input was cut short (e.g. the phase's plan/summary scan hit its cap) — the answer may be incomplete. | | `unscoped` | `Current Phase` could not be resolved from either frontmatter or body — there was nothing to scope the disk lookup to, so the derivation never ran. | | `unreadable` | The frontmatter parse or a filesystem read (the phases directory scan) failed — the derivation could not consult its input. | -`valid` is **not** routed from `scope`: `valid` still means "no drift warnings were found," and `scope` says whether the scan could actually run. A freshly-initialized project reports `{valid:true, warnings:[], drift:{}, scope:'unscoped'}` — nothing was wrong, and the phase could not be checked. See [Interpret `state validate` results](how-to/interpret-state-validate-results.md) for how to act on each `scope` value. +`valid` is **not** routed from `scope`: `valid` still means "no warnings were found," and `scope` says whether the scan could actually run. A freshly-initialized project reports `{valid:true, warnings:[], scope:'unscoped'}` — nothing was wrong, and the phase could not be checked. See [Interpret `state validate` results](how-to/interpret-state-validate-results.md) for how to act on each `scope` value. + +Each `warnings` entry is a coded diagnostic object (`{code, severity, message, remedy}`), not a bare string. Every remedy is an `ADVISE` action naming the command or edit to make — none is auto-applied. `S001` is `severity: ERROR` (STATE.md could not be read at all); every other code is `severity: WARNING`: + +| Code | Severity | Meaning | +|---|---|---| +| `S001` | error | STATE.md is unreadable/corrupt (embedded NUL or binary content) — reported with `valid:false` and no other checks run | +| `S002` | warning | No usable `current_phase`/`Current Phase`/`Current Position Phase` value anywhere in STATE.md | +| `S003` | warning | STATE.md's phase sources (frontmatter vs. body) disagree on the current phase | +| `S004` | warning | The phases directory, or a directory matching the current phase, is missing or unreadable | +| `S005` | warning | STATE.md's plan count disagrees with the plan count on disk | +| `S006` | warning | STATE.md still says "executing" but a `*-VERIFICATION.md` in the phase shows verification passed | +| `S007` | warning | Every plan in the phase has a summary, but STATE.md still says "executing" | --- diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index d59f4d995..c70d7f328 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -380,6 +380,7 @@ "handshake-serialized.cjs", "health-diagnostic-rules/agent-install.cjs", "health-diagnostic-rules/config-validation.cjs", + "health-diagnostic-rules/consistency.cjs", "health-diagnostic-rules/milestone-archive-hygiene.cjs", "health-diagnostic-rules/phase-structure.cjs", "health-diagnostic-rules/roadmap-disk-consistency.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index b693ac546..10081190e 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -485,6 +485,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `health-diagnostic-rules/config-validation.cjs` | Health-diagnostic rules: config.json validation checks (W003, W004, W022, E005, W008, W012-W016), reading only `snapshot.config`, ported behavior-preserving from `cmdValidateHealth` (ADR-3180 §8.2/§8.3/§8.5, Phase 11, #3309) | | `config.cjs` | `config.json` read/write, section initialization; imports validator from `config-schema.cjs` | | `configuration.cjs` | Configuration Module — legacy-key normalization, defaults merge, and explicit on-disk migration; pure normalization primitives consumed by `config-loader.cjs` and `config-schema.cjs` (loadConfig extracted to config-loader per ADR-857 #885) | +| `health-diagnostic-rules/consistency.cjs` | Health-diagnostic rules for `validate.consistency` only (C001-C004: gap in disk phase numbering, gap in plan numbering within a phase, orphan SUMMARY with no matching live PLAN, PLAN missing `wave` frontmatter) — a new `C0NN` code namespace parallel to `validate.health`'s `E`/`W`/`I` space, ported behavior-preserving from `cmdValidateConsistency` (ADR-3180 §8.4, Phase 12, #3310) | | `context-composer.cjs` | Shared budget-composition seam (ADR-1671, #2929) — `composeWithinBudget` trims an ordered fragment list to a measured budget and returns a PLAN of surviving fragments, never rendered text, so one seam serves both the review pipeline and per-runtime emission. Closed strategy set: `verbatim`, `head-shrink`, `proportional-truncate` (with a per-fragment floor), `drop`. The budget unit is injected via `measure(text)` — tokens for `prompt-budget`, bytes for emission — with `charsPerUnit` as its inverse. Also exports `headShrink`/`tailTruncate`. Compiled from `src/context-composer.cts` | | `context-predicates.cjs` | CONTEXT.md predicate fact-store parser (ADR-1671, #2928) — pure `parsePredicates` (extracts every backtick-wrapped `CLASS.subkey=value` declaration, fence/HTML-comment-aware), `selectPredicates` (class/prefix/contains selectors, ANDed), and `buildIndex` (deterministic, line-free artifact shape); backs both `gsd_run query context-predicates` and `scripts/gen-context-index.cjs`'s docs/CONTEXT-INDEX.json drift guard. Compiled from `src/context-predicates.cts` | | `context-utilization.cjs` | Pure classifier for `gsd-health --context` — turns (tokensUsed, contextWindow) into a `{ percent, state }` triage result against the 60%/70% fracture-point thresholds (#2792) | diff --git a/docs/how-to/interpret-state-validate-results.md b/docs/how-to/interpret-state-validate-results.md index bb86ec15c..a7bbeda88 100644 --- a/docs/how-to/interpret-state-validate-results.md +++ b/docs/how-to/interpret-state-validate-results.md @@ -29,8 +29,8 @@ For the flag/output reference, see [`state validate`](../COMMANDS.md#state-valid | `scope` | What it means | What caused it | What to do | |---|---|---|---| -| `complete` | The derivation ran over usable input — a resolvable `Current Phase` and, if a matching phase directory exists, a readable disk scan of it. `valid`/`warnings`/`drift` are a real, trustworthy answer. | Normal operation: STATE.md's phase resolved (from frontmatter or body) and the filesystem was readable. | Trust the result as-is. If `valid:false`, act on the listed `warnings`/`drift` entries (typically `state sync`). | -| `truncated` | Part of the input was cut short before the scan finished — the phase directory's plan/summary scan hit an internal cap partway through. The `valid`/`drift` answer may be **incomplete**, not necessarily wrong. | An unusually large phase directory (many plan/summary files) exceeded the scan's bounded window. | Do not treat `valid:true` here as a clean bill of health. Inspect the phase directory directly (`ls .planning/phases//`) to confirm counts by hand, or reduce/split the phase's plan set if this recurs. | +| `complete` | The derivation ran over usable input — a resolvable `Current Phase` and, if a matching phase directory exists, a readable disk scan of it. `valid`/`warnings` are a real, trustworthy answer. | Normal operation: STATE.md's phase resolved (from frontmatter or body) and the filesystem was readable. | Trust the result as-is. If `valid:false`, act on the listed `warnings` entries (typically `state sync`). | +| `truncated` | Part of the input was cut short before the scan finished — the phase directory's plan/summary scan hit an internal cap partway through. The `valid` answer may be **incomplete**, not necessarily wrong. | An unusually large phase directory (many plan/summary files) exceeded the scan's bounded window. | Do not treat `valid:true` here as a clean bill of health. Inspect the phase directory directly (`ls .planning/phases//`) to confirm counts by hand, or reduce/split the phase's plan set if this recurs. | | `unscoped` | `Current Phase` could not be resolved from **either** the frontmatter scalar or the body field — there was no phase to scope the disk lookup to, so the drift derivation never ran at all. | Most commonly a freshly-initialized project with no phase set yet (a genuine, supported state). Less commonly, a STATE.md whose `Current Phase` field was dropped or malformed. | If the project has not started a phase yet, this is expected — no action needed. If the project is active and you expect a phase to be set, open STATE.md and check the `current_phase` frontmatter key and the body's `**Current Phase:**` row; run `state sync` to reconstruct it from disk if it is missing. | | `unreadable` | An input the scan needed could not be consulted at all — either the frontmatter block failed to parse, or a filesystem read (the phases directory scan) failed mid-scan. | An unterminated/malformed YAML frontmatter fence, or a filesystem error (permissions, a race with a concurrent write) while reading `.planning/phases/`. | Treat `valid:true` here as **not trustworthy** — the scan degraded silently before this field existed, and now surfaces that instead of hiding it. Check that STATE.md's frontmatter fence (`---` / `---`) is well-formed, and that `.planning/phases/` is readable by the current user. Re-run `state validate` after fixing either. | @@ -38,14 +38,14 @@ For the flag/output reference, see [`state validate`](../COMMANDS.md#state-valid ## The case this exists for: "`valid:true` — but is it trustworthy?" -If you only ever read `valid`, every one of the four `scope` values above looks identical: `true`. That collapse is the exact bug this field was added to close (#3162) — a STATE.md whose phase lived only in frontmatter used to silently skip the entire drift scan and report `{valid:true, warnings:[], drift:{}}`, indistinguishable from a phase that was checked and found clean. +If you only ever read `valid`, every one of the four `scope` values above looks identical: `true`. That collapse is the exact bug this field was added to close (#3162) — a STATE.md whose phase lived only in frontmatter used to silently skip the entire drift scan and report `{valid:true, warnings:[]}`, indistinguishable from a phase that was checked and found clean. So before trusting a green `state validate`, always inspect `scope`: - **`scope:'complete'`** — trustworthy. The scan ran; `valid:true` means clean. - **Anything else** — not yet checked, or only partially checked. `valid:true` here means *"no problems were found in what could be looked at,"* which is a materially weaker claim. Use the table above to find out why, and whether that is expected (a fresh project, `unscoped`) or a problem worth fixing (`unreadable`, or a `truncated` scan on a large phase). -A freshly-initialized project is the clearest example of a **legitimate** non-`complete` scope: it reports `{valid:true, warnings:[], drift:{}, scope:'unscoped'}`, which reads as *"nothing was found wrong, and the phase could not be checked"* — not as a defect to fix. +A freshly-initialized project is the clearest example of a **legitimate** non-`complete` scope: it reports `{valid:true, warnings:[], scope:'unscoped'}`, which reads as *"nothing was found wrong, and the phase could not be checked"* — not as a defect to fix. --- diff --git a/eslint.config.mjs b/eslint.config.mjs index 21a073cb8..2ecb346d3 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -145,6 +145,7 @@ export default tseslint.config( 'gsd-core/bin/lib/health-diagnostic-rules/roadmap-disk-consistency.cjs', 'gsd-core/bin/lib/health-diagnostic-rules/worktree-health.cjs', 'gsd-core/bin/lib/health-diagnostic-rules/milestone-archive-hygiene.cjs', + 'gsd-core/bin/lib/health-diagnostic-rules/consistency.cjs', 'gsd-core/bin/lib/shell-command-projection.cjs', 'gsd-core/bin/lib/security.cjs', 'gsd-core/bin/lib/command-aliases.cjs', diff --git a/package.json b/package.json index adfff3145..2b921edf2 100644 --- a/package.json +++ b/package.json @@ -114,7 +114,7 @@ "lint:table-schema-drift": "node scripts/lint-table-schema-drift.cjs", "lint:frontmatter-scalar-broad-grep": "node scripts/lint-frontmatter-scalar-broad-grep.cjs", "lint:removed-but-needed": "node scripts/lint-removed-but-needed.cjs", - "lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-test.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs && node scripts/lint-milestone-window-drift.cjs && node scripts/lint-phase-enumeration-drift.cjs && node scripts/lint-planning-prompt-drift.cjs && node scripts/lint-completion-ratio-drift.cjs && node scripts/lint-state-field-drift.cjs && node scripts/lint-completion-predicate-drift.cjs && node scripts/lint-planning-snapshot-bypass-drift.cjs && node scripts/lint-health-diagnostic-rule-table.cjs && node scripts/lint-frontmatter-scalar-broad-grep.cjs && node scripts/lint-removed-but-needed.cjs", + "lint:ci": "npm run lint && npm run lint:skill-deps && npm run lint:generated-sync && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-allow-test-rule-refs.cjs && node scripts/lint-resolution-provenance.cjs && node scripts/lint-emitted-drift-ack.cjs && node scripts/lint-portable-timeout.cjs && node scripts/validate-registry.cjs && node scripts/lint-table-schema-drift.cjs && node scripts/lint-fix-has-regression-test.cjs && node scripts/lint-example-parser-parity.cjs && node scripts/lint-docs-command-form.cjs && node scripts/lint-plan-count-drift.cjs && node scripts/lint-milestone-window-drift.cjs && node scripts/lint-phase-enumeration-drift.cjs && node scripts/lint-planning-prompt-drift.cjs && node scripts/lint-completion-ratio-drift.cjs && node scripts/lint-state-field-drift.cjs && node scripts/lint-completion-predicate-drift.cjs && node scripts/lint-planning-snapshot-bypass-drift.cjs && node scripts/lint-health-diagnostic-rule-table.cjs && node scripts/lint-planning-artifact-writer-drift.cjs && node scripts/lint-frontmatter-scalar-broad-grep.cjs && node scripts/lint-removed-but-needed.cjs", "lint:allow-test-rule-refs": "node scripts/lint-allow-test-rule-refs.cjs", "lint:regression-names": "node scripts/lint-regression-test-names.cjs", "lint:descriptions": "node scripts/lint-descriptions.cjs", diff --git a/scripts/lint-health-diagnostic-rule-table.cjs b/scripts/lint-health-diagnostic-rule-table.cjs index fe47fde10..893108243 100644 --- a/scripts/lint-health-diagnostic-rule-table.cjs +++ b/scripts/lint-health-diagnostic-rule-table.cjs @@ -35,6 +35,15 @@ * explicit and auditable instead of relying on a coincidental title * match, and the PASS output now reports exempted codes SEPARATELY from * genuinely fixture-covered ones rather than folding them together. + * 3. (Phase 12, #3310 — S0NN pass) `cmdStateValidate` (src/state.cts) emits + * 7 coded diagnostics (S001-S007) built inline via a local + * `stateDiagnostic()` helper — NOT `Rule`-table entries, so they are not + * read from the compiled module the way RULES/CONSISTENCY_RULES are. + * This third pass hardcodes that code list (`STATE_VALIDATE_CODES`, + * below) and applies the SAME §8.5 fixture-proof check against + * `tests/state.test.cjs`, reported in its own PASS/FAIL section — + * mirroring the C0NN pass's structure, just with a hardcoded list + * instead of a `Rule[]` array as the code source. * * Design: .gsd/phase/refactor-3309-health-diagnostic-rule-table/40-design.md * ("The lint guard (§8.2 1:1 invariant + §8.5 fixture proof)"). @@ -64,6 +73,24 @@ const COMPILED_MODULE_PATH = path.join(REPO_ROOT, COMPILED_MODULE_REL); const TEST_GROUP_DIR = path.join(REPO_ROOT, 'tests', 'health-diagnostic-rules'); const SKELETON_TEST_FILE = path.join(REPO_ROOT, 'tests', 'health-diagnostic.test.cjs'); +// Phase 12 (#3310, ADR-3180 §8.4) — the C0NN namespace's own fixture-proof +// test file. §8.5 extends to `CONSISTENCY_RULES`'s NEW codes only (C001-C004 +// — W006/W007 are already fixture-proofed above, against the SAME `Rule` +// objects; re-checking them here would be redundant, not additional +// coverage). +const CONSISTENCY_TEST_FILE = path.join(REPO_ROOT, 'tests', 'health-diagnostic-rules', 'consistency.test.cjs'); +const CONSISTENCY_CODE_PREFIX_RE = /^C\d{3}$/; + +// Phase 12 (#3310, ADR-3180 §8.5 extension) — the S0NN namespace's own +// fixture-proof pass. These 7 codes are NOT collected in any exported +// `Rule[]` array: `cmdStateValidate` (src/state.cts) builds `Diagnostic[]` +// directly via a local `stateDiagnostic()` helper, not via the rule-table +// evaluator (out of scope for the RULES/CONSISTENCY_RULES-keyed passes +// above). The list is therefore hardcoded here instead of read from the +// compiled module. +const STATE_VALIDATE_TEST_FILE = path.join(REPO_ROOT, 'tests', 'state.test.cjs'); +const STATE_VALIDATE_CODES = ['S001', 'S002', 'S003', 'S004', 'S005', 'S006', 'S007']; + // Matches `describe(`/`test(`/`it(` calls whose first argument is a string // literal, capturing that literal as the block's title. Line/regex-based // (not full AST) per this repo's existing lint-guard house style @@ -221,13 +248,38 @@ function formatRepoRelative(absPath) { } function main() { - const { RULES, SEVERITY } = loadCompiledModule(); + const { RULES, CONSISTENCY_RULES, SEVERITY } = loadCompiledModule(); const { duplicates, badSeverities } = checkOneToOneInvariant(RULES, SEVERITY); const testFiles = findHealthDiagnosticTestFiles(REPO_ROOT); const { uncovered, exempted } = checkFixtureProofInvariant(RULES, testFiles); + // ─── C0NN check pass (Phase 12, #3310) — separate from the W/E/I pass + // above, own reporting section, does NOT re-check W006/W007's fixture + // proof (already covered above against the same `Rule` objects). + const consistencyNewRules = CONSISTENCY_RULES.filter((r) => CONSISTENCY_CODE_PREFIX_RE.test(r.code)); + const { duplicates: consistencyDuplicates, badSeverities: consistencyBadSeverities } = checkOneToOneInvariant( + consistencyNewRules, + SEVERITY, + ); + const consistencyTestFiles = fs.existsSync(CONSISTENCY_TEST_FILE) ? [CONSISTENCY_TEST_FILE] : []; + const { uncovered: consistencyUncovered } = checkFixtureProofInvariant( + consistencyNewRules, + consistencyTestFiles, + new Map(), + ); + + // ─── S0NN check pass (Phase 12, #3310) — separate from the W/E/I and C0NN + // passes above, own reporting section. Hardcoded code list (see the + // STATE_VALIDATE_CODES comment above) rather than read from a Rule[] array. + const stateValidateTestFiles = fs.existsSync(STATE_VALIDATE_TEST_FILE) ? [STATE_VALIDATE_TEST_FILE] : []; + const { uncovered: stateValidateUncovered } = checkFixtureProofInvariant( + STATE_VALIDATE_CODES.map((code) => ({ code })), + stateValidateTestFiles, + new Map(), + ); + const problems = []; if (duplicates.length > 0) { @@ -263,6 +315,50 @@ function main() { ); } + if (consistencyDuplicates.length > 0) { + const list = consistencyDuplicates.map((d) => ` ${d.code} (${d.count} occurrences)`).join('\n'); + problems.push( + `§8.2 rule 1 violated (C0NN namespace): ${consistencyDuplicates.length} duplicated rule code(s) in ` + + `CONSISTENCY_RULES (${COMPILED_MODULE_REL}):\n${list}\n` + + ' remedy: codes are append-only and 1:1 with a single Rule — rename or remove the duplicate.', + ); + } + + if (consistencyBadSeverities.length > 0) { + const list = consistencyBadSeverities + .map((b) => ` ${b.code}: severity=${JSON.stringify(b.severity)}`) + .join('\n'); + problems.push( + `§8.2 rule 3 violated (C0NN namespace): ${consistencyBadSeverities.length} rule(s) with a severity not ` + + `in SEVERITY's values:\n${list}\n` + + ' remedy: severity is a property of the RULE — set it to SEVERITY.ERROR/WARNING/INFO.', + ); + } + + if (consistencyUncovered.length > 0) { + problems.push( + `§8.5 violated (C0NN namespace): ${consistencyUncovered.length} rule code(s) with no describe()/test() ` + + `block naming them (a comment or bare string mention does not count):\n ${consistencyUncovered.join(', ')}\n\n` + + ` Searched: ${formatRepoRelative(CONSISTENCY_TEST_FILE)}` + + (consistencyTestFiles.length === 0 ? ' (file not found)' : '') + + '\n\n remedy: add or extend a describe()/test() title in tests/health-diagnostic-rules/consistency.test.cjs ' + + `so the block title names the code verbatim (e.g. describe('${consistencyUncovered[0]} — ...', () => { ... })), ` + + 'and drive the rule to fire against a real fixture built via createTempDir() + buildPlanningSnapshot().', + ); + } + + if (stateValidateUncovered.length > 0) { + problems.push( + `§8.5 violated (S0NN namespace): ${stateValidateUncovered.length} code(s) with no describe()/test() ` + + `block naming them (a comment or bare string mention does not count):\n ${stateValidateUncovered.join(', ')}\n\n` + + ` Searched: ${formatRepoRelative(STATE_VALIDATE_TEST_FILE)}` + + (stateValidateTestFiles.length === 0 ? ' (file not found)' : '') + + '\n\n remedy: add or extend a describe()/test() title in tests/state.test.cjs ' + + `so the block title names the code verbatim (e.g. test('${stateValidateUncovered[0]}: ...', () => { ... })), ` + + 'mirroring the existing `#3310 state validate — S0NN coded diagnostics` describe block.', + ); + } + if (problems.length > 0) { throw new ExitError(1, `${problems.join('\n\n')}\n`); } @@ -277,6 +373,16 @@ function main() { `real fixture, ${exempted.length} exempted across ${testFiles.length} test file(s).` + (exempted.length > 0 ? `\n Exempted: ${exemptedDetail}` : ''), ); + console.log( + `lint-health-diagnostic-rule-table: PASS (C0NN namespace) — ${consistencyNewRules.length} new rule code(s) ` + + `(${consistencyNewRules.map((r) => r.code).join(', ')}), all fixture-covered in ` + + `${formatRepoRelative(CONSISTENCY_TEST_FILE)}.`, + ); + console.log( + `lint-health-diagnostic-rule-table: PASS (S0NN namespace) — ${STATE_VALIDATE_CODES.length} code(s) ` + + `(${STATE_VALIDATE_CODES.join(', ')}), all fixture-covered in ` + + `${formatRepoRelative(STATE_VALIDATE_TEST_FILE)}.`, + ); } runMain(main); @@ -292,4 +398,7 @@ module.exports = { COMPILED_MODULE_PATH, TEST_GROUP_DIR, SKELETON_TEST_FILE, + CONSISTENCY_TEST_FILE, + STATE_VALIDATE_TEST_FILE, + STATE_VALIDATE_CODES, }; diff --git a/scripts/lint-phase-enumeration-drift.cjs b/scripts/lint-phase-enumeration-drift.cjs index 20f617d4e..581a7b1fd 100644 --- a/scripts/lint-phase-enumeration-drift.cjs +++ b/scripts/lint-phase-enumeration-drift.cjs @@ -67,10 +67,6 @@ * see the PHYSICAL set so a heading already scoped by * `extractCurrentMilestoneScoped` can find its directory; filtering it * through the owner would scope the same set twice. - * - `src/verify.cts` `collectDiskPhases`: a DRIFT DIAGNOSTIC comparing - * what is on disk against what the ROADMAP declares. It wants the - * physical set by definition — scoping it would make the diagnostic - * unable to see the very drift it exists to report. * - `src/verify.cts` `cmdValidateHealth`: a project-wide HEALTH-CHECK sweep * (config drift, phase-directory naming, duplicate-directory collisions, * unsummarized-plan detection) — same "sweep everything, report gaps" @@ -274,7 +270,7 @@ const OWNER_FILES = new Set([ // See the header comment for the full written reason behind each entry. const FUNCTION_SCOPED_EXEMPTIONS = new Map([ [path.join('src', 'roadmap.cts'), new Set(['cmdRoadmapAnalyze'])], - [path.join('src', 'verify.cts'), new Set(['collectDiskPhases', 'cmdValidateHealth', 'cmdVerifySchemaDrift'])], + [path.join('src', 'verify.cts'), new Set(['cmdValidateHealth', 'cmdVerifySchemaDrift'])], [path.join('src', 'init.cts'), new Set(['detectHasPriorPhases', 'detectUiPhaseActive', 'cmdInitMilestoneOp'])], [path.join('src', 'milestone.cts'), new Set(['archivePhaseDirectories', 'cmdMilestoneComplete', 'cmdPhasesClear'])], [path.join('src', 'phase.cts'), new Set(['cmdPhasesList', 'cmdPhaseNextDecimal', 'cmdPhasePlanIndex', 'cmdPhaseInsert', 'renameDecimalPhases', 'renameIntegerPhases'])], diff --git a/scripts/lint-plan-count-drift.cjs b/scripts/lint-plan-count-drift.cjs index 7b53d10f5..ad6d02690 100644 --- a/scripts/lint-plan-count-drift.cjs +++ b/scripts/lint-plan-count-drift.cjs @@ -151,13 +151,15 @@ const CORE_UTILS_EXEMPT_FUNCTIONS = new Set([ // #2296/#2070/#2838) byte-for-behaviour rather than the phase-scoped // plan-scan owner's root+nested rule — "rescue every summary before a // merge blows it away" is not a live-plan/completion count. -// - verify.cts cmdValidateConsistency: the strict `-(\d{2})-PLAN\.md$` -// match extracts a zero-padded SEQUENCE NUMBER from filenames the owner -// (`allPlanFiles`) already classified as plans — it does not re-derive -// "is this a plan", it answers a different, narrower question (does the -// canonical 2-digit numbering sequence have a gap) that the owner's -// boolean plan/summary classification cannot answer. See the extended -// inline comment at that call site for the full Question 1/2/3 split. +// - planning-snapshot.cts buildPerPhasePlanScanFields (Phase 12, #3310, +// ADR-3180 §8.4): a strict `-(\d{2})-PLAN\.md$` match extracts a +// zero-padded SEQUENCE NUMBER from filenames the owner (`allPlanFiles`) +// already classified as plans, into the `perPhasePlanNumbering` +// `PlanningSnapshot` field so `validate.consistency`'s C002 rule can read +// it via the shared snapshot instead of re-scanning disk. It does not +// re-derive "is this a plan" — it answers a different, narrower question +// (does the canonical 2-digit numbering sequence have a gap) that the +// owner's boolean plan/summary classification cannot answer. const FUNCTION_SCOPED_EXEMPTIONS = new Map([ [CORE_UTILS_FILE, CORE_UTILS_EXEMPT_FUNCTIONS], [path.join('src', 'audit.cts'), new Set(['scanQuickTasks'])], @@ -165,7 +167,7 @@ const FUNCTION_SCOPED_EXEMPTIONS = new Map([ [path.join('src', 'estimate-cli.cts'), new Set(['collectCalibrationSamples'])], [path.join('src', 'roadmap.cts'), new Set(['cmdRoadmapAnnotateDependencies'])], [path.join('src', 'worktree-safety.cts'), new Set(['defaultFindSummaryFiles'])], - [path.join('src', 'verify.cts'), new Set(['cmdValidateConsistency'])], + [path.join('src', 'planning-snapshot.cts'), new Set(['buildPerPhasePlanScanFields'])], ]); // Optional `export ` modifier: `collectCalibrationSamples` (estimate-cli.cts) diff --git a/scripts/lint-planning-artifact-writer-drift.cjs b/scripts/lint-planning-artifact-writer-drift.cjs new file mode 100644 index 000000000..ae5b2f854 --- /dev/null +++ b/scripts/lint-planning-artifact-writer-drift.cjs @@ -0,0 +1,398 @@ +#!/usr/bin/env node +'use strict'; + +/** + * Registry-completeness guard for the `.planning/`-root artifact registry + * (epic #3180, ADR-3180 §8.4 deliverable C, Phase 12 #3310). + * + * `src/artifacts.cts`'s `isCanonicalPlanningFile` enumerates every file name + * gsd workflows officially write at the `.planning/` ROOT (used today by + * `validate.health`'s W019 to flag unrecognized files). Nothing previously + * checked the OTHER direction: that every actual writer of a `.planning/` + * root file is itself represented in that registry. This guard closes that + * gap statically — it does not run any code, it scans `src/*.cts` for a + * write whose target file name can be determined AT READ TIME and checks + * that name against the real (compiled) `isCanonicalPlanningFile`. + * + * ## What counts as a checkable write + * + * A call to `platformWriteSync(` or `fs.writeFileSync(` whose first argument + * resolves — through same-file, single-hop static tracing only — to a + * LITERAL `.md`/`.json` file name joined onto an UNAMBIGUOUS `.planning/` + * root expression. Three source shapes are recognized, all real patterns + * found in this codebase (`src/roadmap.cts`, `src/state.cts`, + * `src/config.cts`, `src/milestone.cts`, `src/health-diagnostic.cts`): + * + * 1. `path.join(, 'Literal.md')` — inline, or assigned first to a + * `const`/`let` binding or an object-literal property + * (`key: path.join(, 'Literal.md')`) that is later passed to the + * write call by name. Object-literal properties are traced by NAME + * only (no cross-function data-flow) — this matches the actual + * `RepairPaths`-style convention this repo already uses + * (`src/health-diagnostic.cts:209-218`), where a destructured + * parameter reuses the same identifier the property was defined with. + * 2. `planningPaths(cwd).` for `` in `state`, `roadmap`, + * `project`, `config`, `requirements` — the `PlanningPaths` interface's + * own file-valued properties (`src/planning-workspace.cts`), which are + * already a full path, not a directory to join further. + * 3. A `` in both forms above means EXACTLY `planningRoot(cwd)`, + * `planningDir(cwd)` (no second argument), or `planningPaths(cwd)` + * (no second argument) `.planning` — deliberately excluding any call + * that passes a workstream/project argument (`planningDir(cwd, ws)`), + * because that form can resolve UNDER `.planning/workstreams//` + * instead of the `.planning/` root, and this guard cannot tell + * statically whether `ws` is truthy at runtime. Per this guard's own + * design brief: "false negatives are safer than false positives" — an + * ambiguous root expression is silently skipped, never reported either + * way. + * + * Anything else — a template-literal or otherwise runtime-computed target, + * a multi-segment join landing under `phases/`, `milestones/`, or + * `workstreams/`, a path built through an intermediate helper this guard + * does not recognize (e.g. `path.dirname(x)`, a ternary, a `.gsd/` + * fallback) — is silently skipped. This guard reports VIOLATIONS only; a + * skipped write is never counted as a pass either. See the module docblock + * above `src/artifacts.cts` for the registry's own stated scope + * (".planning/ root level" only) — this guard shares that scope. + * + * ## Caveat: same-file, name-based tracing (not a real data-flow analysis) + * + * Both the `const`/`let` and object-literal-property forms are tracked in a + * single flat, WHOLE-FILE, name -> file-name map (no per-function scoping). + * If the same identifier were reused in one file for two unrelated purposes + * — one a real `.planning/`-root join, the other something else entirely — + * this guard could mis-resolve the second one. No such collision exists in + * `src/*.cts` today (verified during implementation); this is a known, + * accepted heuristic limit, matching this guard's explicitly simpler + * (non-function-scoped) design versus its sibling + * `lint-planning-snapshot-bypass-drift.cjs`. + * + * ## No ratchet / no baseline + * + * Unlike its `lint-*-drift.cjs` siblings, this is a bare pass/fail check, + * not a shrinking-debt baseline: a ground-truth sweep of this codebase + * found every real `.planning/`-root writer already registered, so there is + * no inherited debt to grandfather. Any violation this guard reports is a + * genuine, actionable regression. + * + * Tree-walk / root-confinement / symlink / sanitizer machinery is shared + * via `scripts/lib/drift-scan.cjs`, exactly like every sibling guard. + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const driftScan = require('./lib/drift-scan.cjs'); +const { sanitizeForReport, scanTree } = driftScan; + +const REPO_ROOT = path.join(__dirname, '..'); +const COMPILED_MODULE_REL = path.join('gsd-core', 'bin', 'lib', 'artifacts.cjs'); +const COMPILED_MODULE_PATH = path.join(REPO_ROOT, COMPILED_MODULE_REL); + +// Authored TypeScript source only — mirrors every sibling drift guard. +const SCAN_DIRS = ['src']; +const SCAN_EXT = new Set(['.cts']); + +// `PlanningPaths` (src/planning-workspace.cts) properties that are +// themselves a FULL FILE path (not a directory) at the `.planning/` root, +// mapped to the literal file name they resolve to. `planning`/`phases`/ +// `debug` are deliberately absent — those are directories, not files. +const PLANNING_PATHS_FILE_PROPS = new Map([ + ['state', 'STATE.md'], + ['roadmap', 'ROADMAP.md'], + ['project', 'PROJECT.md'], + ['config', 'config.json'], + ['requirements', 'REQUIREMENTS.md'], +]); + +// The three unambiguous `.planning/`-root expressions this guard recognizes +// as the first argument of `path.join(...)`. Each takes ONLY `cwd` — a call +// carrying a workstream/project argument is excluded (see module docblock). +const ROOT_CALL_SRC = String.raw`planningRoot\(cwd\)|planningDir\(cwd\)|planningPaths\(cwd\)\.planning`; + +// A quoted literal file name ending in `.md` or `.json` — single-quoted or +// double-quoted, as two separate alternatives (groups: single-quoted +// filename, double-quoted filename) rather than a `(['"])...\1` +// backreference: this fragment is spliced into several different larger +// regexes below at different capture-group OFFSETS, so a fixed +// backreference number (`\1`) would silently point at whichever group +// happens to be first in THAT particular composed regex, not necessarily +// this fragment's own quote group. Every call site reads +// `matched[i] ?? matched[i + 1]` for the two alternative filename groups +// this fragment always contributes, in order. Deliberately excludes +// backticks: a template literal is a runtime-computed target by definition +// and must never match here. +const LITERAL_FILENAME_SRC = String.raw`(?:'([^'\\]+\.(?:md|json))'|"([^"\\]+\.(?:md|json))")`; + +// `const X = ;` / `let X = ;` — binds X to an unambiguous root +// expression, so a later `path.join(X, 'Literal.md')` can resolve through +// it (mirrors `src/config.cts`'s `planningBase` / `src/health-diagnostic +// .cts`'s `rootBase`/`wsBase`). +const ROOT_VAR_ASSIGN_RE = new RegExp(String.raw`\b(?:const|let)\s+([A-Za-z_$][\w$]*)\s*=\s*(?:${ROOT_CALL_SRC})\s*;`); + +// A "binding" — either `const X = ...` / `let X = ...`, or an object-literal +// property `X: ...` — shared by both join-tracing regexes below so a +// destructured-later property (the `RepairPaths` convention) traces the +// same way a local variable does. Group 1 is the const/let name, group 2 is +// the property name; callers use whichever is non-undefined. +const BINDING_PREFIX_SRC = String.raw`(?:(?:const|let)\s+([A-Za-z_$][\w$]*)\s*=|([A-Za-z_$][\w$]*)\s*:)`; + +// ` = path.join(, 'Literal.md')` — the root expression is +// spelled out inline (not through an intermediate variable). +const JOIN_FROM_ROOT_CALL_RE = new RegExp( + String.raw`${BINDING_PREFIX_SRC}\s*path\.join\(\s*(?:${ROOT_CALL_SRC})\s*,\s*${LITERAL_FILENAME_SRC}\s*\)`, +); + +// ` = path.join(IDENT, 'Literal.md')` — the root expression was +// already bound to IDENT by ROOT_VAR_ASSIGN_RE elsewhere in the file. +const JOIN_FROM_IDENT_RE = new RegExp( + String.raw`${BINDING_PREFIX_SRC}\s*path\.join\(\s*([A-Za-z_$][\w$]*)\s*,\s*${LITERAL_FILENAME_SRC}\s*\)`, +); + +// ` = planningPaths(cwd).` — PLANNING_PATHS_FILE_PROPS below +// maps to its file name. +const PLANNING_PATHS_PROP_RE = new RegExp( + String.raw`${BINDING_PREFIX_SRC}\s*planningPaths\(cwd\)\.([A-Za-z_$][\w$]*)\b`, +); + +// The write calls this guard checks the first argument of. +const WRITE_CALL_RE = /\b(?:platformWriteSync|fs\.writeFileSync)\(/g; + +// A bare identifier, or an inline `path.join(, 'Literal.md')` / +// `planningPaths(cwd).` expression, as the resolved first-argument +// text of a write call. +const INLINE_JOIN_ROOT_RE = new RegExp(String.raw`^path\.join\(\s*(?:${ROOT_CALL_SRC})\s*,\s*${LITERAL_FILENAME_SRC}\s*\)$`); +const INLINE_PLANNING_PATHS_PROP_RE = /^planningPaths\(cwd\)\.([A-Za-z_$][\w$]*)$/; +const BARE_IDENT_RE = /^[A-Za-z_$][\w$]*$/; + +/** + * Strip `//` line comments and `/* ... *\/` block comments from `line`, + * preserving the CONTENTS of single/double/backtick-quoted strings verbatim + * (so a filename literal or an identifier that happens to sit inside a + * string is never mistaken for code, but a `//`/`/*` inside a string never + * truncates the line either). No cross-line state: a template literal or + * block comment that spans multiple lines is left as-is on each line it + * touches — every real call/assignment this guard matches is single-line in + * `src/*.cts` today, so cross-line tracking would add complexity with no + * observed benefit (see this guard's "simpler than its sibling" design + * note). + */ +function stripLineComment(line) { + let out = ''; + let i = 0; + while (i < line.length) { + const ch = line[i]; + if (ch === '/' && line[i + 1] === '/') break; + if (ch === '/' && line[i + 1] === '*') { + const close = line.indexOf('*/', i + 2); + if (close === -1) { i = line.length; break; } + i = close + 2; + continue; + } + if (ch === "'" || ch === '"' || ch === '`') { + const quote = ch; + const start = i; + let j = i + 1; + while (j < line.length) { + if (line[j] === '\\') { j += 2; continue; } + if (line[j] === quote) { j++; break; } + j++; + } + out += line.slice(start, j); + i = j; + continue; + } + out += ch; + i++; + } + return out; +} + +/** + * Scan forward from `openParenIdx` (the index of a call's opening `(`) and + * return the TEXT of its first argument — up to the first top-level comma, + * or the call's own closing paren if it has only one argument — respecting + * nested parens and quoted strings so an inner `path.join(a, 'b.md')` + * comma never terminates early. Returns null if the call does not close on + * this line (a genuinely multi-line call is out of this guard's scope — see + * module docblock). + */ +function extractFirstArg(line, openParenIdx) { + let depth = 1; + let i = openParenIdx + 1; + const start = i; + while (i < line.length) { + const ch = line[i]; + if (ch === "'" || ch === '"' || ch === '`') { + const quote = ch; + i++; + while (i < line.length) { + if (line[i] === '\\') { i += 2; continue; } + if (line[i] === quote) { i++; break; } + i++; + } + continue; + } + if (ch === '(') { depth++; i++; continue; } + if (ch === ')') { + if (depth === 1) return line.slice(start, i).trim(); + depth--; i++; continue; + } + if (ch === ',' && depth === 1) return line.slice(start, i).trim(); + i++; + } + return null; // unterminated on this line — skip (see docblock) +} + +/** + * Pure: scan `text` (one `src/*.cts` file's contents) for every write call + * this guard can statically resolve to a literal `.planning/`-root file + * name. Returns EVERY resolved candidate (canonical or not) — filtering to + * violations only happens in `findArtifactWriterDrift` — so tests and + * callers can tell "not checked" (candidate absent) apart from "checked and + * passed" (candidate present, canonical). + */ +function scanFileForCandidates(text, relPath) { + const file = relPath.replace(/\\/g, '/'); + const originalLines = text.split('\n'); + const lines = originalLines.map(stripLineComment); + + // Pass 1: build the whole-file name -> file-name maps (see module + // docblock for the "flat, same-file, name-based" tracing this performs). + const rootVars = new Set(); + const filenameVars = new Map(); + for (const line of lines) { + const rootMatch = ROOT_VAR_ASSIGN_RE.exec(line); + if (rootMatch) rootVars.add(rootMatch[1]); + } + for (const line of lines) { + const m1 = JOIN_FROM_ROOT_CALL_RE.exec(line); + if (m1) { + const name = m1[1] || m1[2]; + filenameVars.set(name, m1[3] || m1[4]); + continue; + } + const propMatch = PLANNING_PATHS_PROP_RE.exec(line); + if (propMatch) { + const name = propMatch[1] || propMatch[2]; + const prop = propMatch[3]; + if (PLANNING_PATHS_FILE_PROPS.has(prop)) filenameVars.set(name, PLANNING_PATHS_FILE_PROPS.get(prop)); + continue; + } + const m2 = JOIN_FROM_IDENT_RE.exec(line); + if (m2) { + const name = m2[1] || m2[2]; + const sourceIdent = m2[3]; + if (rootVars.has(sourceIdent)) filenameVars.set(name, m2[4] || m2[5]); + } + } + + // Pass 2: resolve every write call's first argument. + const out = []; + for (let li = 0; li < lines.length; li++) { + const line = lines[li]; + WRITE_CALL_RE.lastIndex = 0; + let callMatch; + while ((callMatch = WRITE_CALL_RE.exec(line)) !== null) { + const openParenIdx = callMatch.index + callMatch[0].length - 1; + const argText = extractFirstArg(line, openParenIdx); + if (argText === null) continue; + + let filename = null; + if (BARE_IDENT_RE.test(argText)) { + if (filenameVars.has(argText)) filename = filenameVars.get(argText); + } else { + const inlineJoin = INLINE_JOIN_ROOT_RE.exec(argText); + if (inlineJoin) { + filename = inlineJoin[1] || inlineJoin[2]; + } else { + const inlineProp = INLINE_PLANNING_PATHS_PROP_RE.exec(argText); + if (inlineProp && PLANNING_PATHS_FILE_PROPS.has(inlineProp[1])) filename = PLANNING_PATHS_FILE_PROPS.get(inlineProp[1]); + } + } + + if (filename !== null) { + out.push({ file, line: li + 1, filename, text: originalLines[li].trim() }); + } + } + } + return out; +} + +/** + * Pure: `scanFileForCandidates` filtered to violations — a resolved + * candidate whose file name `isCanonicalPlanningFile` rejects. + * `isCanonical` defaults to the REAL, compiled function (loaded lazily so a + * missing `npm run build:lib` only errors when this guard actually runs, + * not merely on `require`) but is overridable for tests that want to + * exercise the filter without a build. + */ +function findArtifactWriterDrift(text, relPath, isCanonical) { + const check = isCanonical || loadIsCanonicalPlanningFile(); + return scanFileForCandidates(text, relPath).filter((c) => !check(c.filename)); +} + +let _isCanonicalPlanningFile = null; +function loadIsCanonicalPlanningFile() { + if (_isCanonicalPlanningFile) return _isCanonicalPlanningFile; + if (!fs.existsSync(COMPILED_MODULE_PATH)) { + throw new Error( + `lint-planning-artifact-writer-drift: compiled artifact not found at ${COMPILED_MODULE_REL}.\n` + + 'Run `npm run build:lib` first.', + ); + } + const mod = require(COMPILED_MODULE_PATH); + if (typeof mod.isCanonicalPlanningFile !== 'function') { + throw new Error(`lint-planning-artifact-writer-drift: ${COMPILED_MODULE_REL} does not export isCanonicalPlanningFile()`); + } + _isCanonicalPlanningFile = mod.isCanonicalPlanningFile; + return _isCanonicalPlanningFile; +} + +/** Scan the authored source tree and return every writer-registry violation. */ +function scanRepo(root) { + const isCanonical = loadIsCanonicalPlanningFile(); + return scanTree({ + root, + scanDirs: SCAN_DIRS, + scanExt: SCAN_EXT, + onFile(rel, text) { + return findArtifactWriterDrift(text, rel, isCanonical); + }, + }); +} + +function main() { + const violations = scanRepo(REPO_ROOT); + + if (violations.length === 0) { + process.stdout.write('ok planning-artifact-writer: every statically-resolvable .planning/-root write is a registered canonical artifact\n'); + return; + } + + process.stderr.write('planning-artifact-writer: unregistered .planning/-root artifact write(s) found.\n'); + process.stderr.write('Every write of a literal .planning/-root file name must be reflected in the registry\n'); + process.stderr.write("(src/artifacts.cts's isCanonicalPlanningFile, consumed by validate.health's W019):\n"); + for (const v of violations) { + process.stderr.write( + ` ${sanitizeForReport(v.file)}:${v.line} '${sanitizeForReport(v.filename)}' ${sanitizeForReport(v.text)}\n` + + ` remedy: add '${sanitizeForReport(v.filename)}' to CANONICAL_EXACT in src/artifacts.cts, ` + + 'or a CANONICAL_PATTERNS regex if it is version-stamped\n', + ); + } + process.exitCode = 1; +} + +if (require.main === module) main(); + +module.exports = { + scanFileForCandidates, + findArtifactWriterDrift, + scanRepo, + stripLineComment, + extractFirstArg, + PLANNING_PATHS_FILE_PROPS, + SCAN_DIRS, + SCAN_EXT, + COMPILED_MODULE_PATH, + COMPILED_MODULE_REL, +}; diff --git a/src/artifacts.cts b/src/artifacts.cts index 2f2007b2e..39b762faf 100644 --- a/src/artifacts.cts +++ b/src/artifacts.cts @@ -25,6 +25,7 @@ export const CANONICAL_EXACT: ReadonlySet = new Set([ 'CLAUDE.md', 'RETROSPECTIVE.md', 'WINDOWS.md', // #3224: broken-windows ledger (src/broken-windows.cts, LEDGER_FILE_NAME) + 'STATE-ARCHIVE.md', // state.cts's cmdStatePrune writes this at the .planning/ root ]); // Pattern-match canonical file names (regex tests on the basename) diff --git a/src/health-diagnostic-rules/consistency.cts b/src/health-diagnostic-rules/consistency.cts new file mode 100644 index 000000000..fd879d5e6 --- /dev/null +++ b/src/health-diagnostic-rules/consistency.cts @@ -0,0 +1,165 @@ +/** + * Health Diagnostic — `validate.consistency`-only rules (Phase 12, #3310, + * ADR-3180 §8.4). + * + * Group: "consistency" — a NEW `C0NN` code namespace, parallel to (not part + * of) `validate.health`'s `E`/`W`/`I` space Phase 11 minted. These four + * subjects are `cmdValidateConsistency`'s (`src/verify.cts:1466-1612`) + * own findings, not `validate.health` findings — see the design doc's + * "Code namespace" section for why they get their own prefix instead of + * competing for the next free `W0NN` number. + * + * Ported behavior-preserving from `cmdValidateConsistency` + * (`src/verify.cts:1504-1519` for C001, `:1570-1576` for C002, `:1584-1587` + * for C003, `:1596-1602` for C004) — relocated, not reinvented. The + * duplicate subjects `cmdValidateConsistency` also computed (phase-in- + * roadmap-no-dir / dir-on-disk-not-in-roadmap) are NOT here: those are + * W006/W007, reused verbatim from + * `src/health-diagnostic-rules/roadmap-disk-consistency.cts` via + * `health-diagnostic.cts`'s `CONSISTENCY_RULES` composition, not + * reimplemented in this file (design doc, "Which rules run where"). + * + * All four remedies are `ADVISE` — none of these four subjects had a repair + * path in the pre-migration code either, matching Phase 11's own convention + * for message-only findings with no automated fix. + * + * Design: .gsd/phase/feat-3310-enhance-3180-the-sibling-validators-shar/40-design.md + * Test matrix: .gsd/phase/feat-3310-enhance-3180-the-sibling-validators-shar/50-test-matrix.md + * + * ADR-457 build-at-publish: source in src/health-diagnostic-rules/consistency.cts, + * compiled to gsd-core/bin/lib/health-diagnostic-rules/consistency.cjs (gitignored). + */ + +// eslint-disable-next-line @typescript-eslint/no-require-imports -- type-only; erased at compile time, no runtime require emitted +import type planningSnapshotMod = require('../planning-snapshot.cjs'); + +type PlanningSnapshot = ReturnType; + +// eslint-disable-next-line @typescript-eslint/no-require-imports +import healthDiagnosticMod = require('../health-diagnostic-types.cjs'); +const { SEVERITY, adviseRemedy } = healthDiagnosticMod; +type Diagnostic = healthDiagnosticMod.Diagnostic; +type Rule = healthDiagnosticMod.Rule; + +// eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseIdMod = require('../phase-id.cjs'); +const { isSentinelPhaseId } = phaseIdMod; + +// ─── C001 — gap in disk phase numbering (integer sequence) ──────────────── +// (verify.cts:1504-1519) + +function checkC001(snapshot: PlanningSnapshot): Diagnostic[] { + // verify.cts:1505 — `if (config.phase_naming !== 'custom')`; the + // integer-sequence gap check is skipped entirely under custom phase + // naming, since a custom naming scheme has no integer sequence to have a + // gap in. + if (snapshot.config.value?.['phase_naming'] === 'custom') return []; + + const integerPhases = snapshot.allPhaseDirNames.value + // #3225: exclude sentinel phase ids (999.x/0.x) — never part of the + // sequential numbering, mirrors verify.cts:1510 verbatim. + .filter((p) => !p.includes('.') && !isSentinelPhaseId(p)) + .map((p) => parseInt(p, 10)) + .filter((n) => !Number.isNaN(n)) + .sort((a, b) => a - b); + + const diagnostics: Diagnostic[] = []; + for (let i = 1; i < integerPhases.length; i++) { + if (integerPhases[i] !== integerPhases[i - 1] + 1) { + diagnostics.push({ + code: 'C001', + severity: SEVERITY.WARNING, + message: `Gap in phase numbering: ${integerPhases[i - 1]} → ${integerPhases[i]}`, + remedy: adviseRemedy('Create the missing phase directory or renumber to close the gap'), + }); + } + } + return diagnostics; +} + +// ─── C002 — gap in plan numbering within a phase ─────────────────────────── +// (verify.cts:1570-1576) + +// Fidelity note: the original's `phaseLabel` (verify.cts:1532, +// `posixNormalize(path.relative(planBase, phasePath))`) is a +// `.planning/`-relative posix path, which for the flat `phases/` case +// this rule scans reduces to the bare directory name (`phaseDir` below) — +// same string, not a narrower one, since `perPhasePlanNumbering` only +// enumerates the flat `phases/` root (see `PlanningSnapshot`'s own doc +// comment on this field for the one disclosed scope reduction, an active +// archived milestone's second phase root, which is out of scope here too). +function checkC002(snapshot: PlanningSnapshot): Diagnostic[] { + const diagnostics: Diagnostic[] = []; + for (const { phaseDir, planNums } of snapshot.perPhasePlanNumbering.value) { + for (let i = 1; i < planNums.length; i++) { + if (planNums[i] !== planNums[i - 1] + 1) { + diagnostics.push({ + code: 'C002', + severity: SEVERITY.WARNING, + message: `Gap in plan numbering in ${phaseDir}: plan ${planNums[i - 1]} → ${planNums[i]}`, + remedy: adviseRemedy('Create the missing plan or renumber to close the gap'), + }); + } + } + } + return diagnostics; +} + +// ─── C003 — orphan SUMMARY with no matching live PLAN ────────────────────── +// (verify.cts:1584-1587) + +function checkC003(snapshot: PlanningSnapshot): Diagnostic[] { + return snapshot.perPhaseOrphanSummaries.value.map(({ phaseDir, orphanSummary }) => ({ + code: 'C003', + severity: SEVERITY.WARNING, + message: `Summary ${orphanSummary} in ${phaseDir} has no matching PLAN.md`, + remedy: adviseRemedy('Create the matching PLAN.md or remove the orphan summary'), + })); +} + +// ─── C004 — PLAN missing `wave` frontmatter ──────────────────────────────── +// (verify.cts:1596-1602) + +function checkC004(snapshot: PlanningSnapshot): Diagnostic[] { + return snapshot.perPhaseWaveMissingPlans.value.map(({ phaseDir, plan }) => ({ + code: 'C004', + severity: SEVERITY.WARNING, + message: `${phaseDir}/${plan}: missing 'wave' in frontmatter`, + remedy: adviseRemedy("Add a 'wave' key to the plan's frontmatter"), + })); +} + +// ─── Exports ──────────────────────────────────────────────────────────────── + +const RULES: Rule[] = [ + { + code: 'C001', + severity: SEVERITY.WARNING, + description: 'Gap in disk phase numbering (integer sequence)', + repairable: false, + check: checkC001, + }, + { + code: 'C002', + severity: SEVERITY.WARNING, + description: 'Gap in plan numbering within a phase', + repairable: false, + check: checkC002, + }, + { + code: 'C003', + severity: SEVERITY.WARNING, + description: 'Orphan SUMMARY with no matching live PLAN', + repairable: false, + check: checkC003, + }, + { + code: 'C004', + severity: SEVERITY.WARNING, + description: "PLAN missing 'wave' in frontmatter", + repairable: false, + check: checkC004, + }, +]; + +export = { RULES }; diff --git a/src/health-diagnostic.cts b/src/health-diagnostic.cts index 6079c9a02..af10ec1fe 100644 --- a/src/health-diagnostic.cts +++ b/src/health-diagnostic.cts @@ -81,6 +81,8 @@ import roadmapDiskConsistencyMod = require('./health-diagnostic-rules/roadmap-di import worktreeHealthMod = require('./health-diagnostic-rules/worktree-health.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports import milestoneArchiveHygieneMod = require('./health-diagnostic-rules/milestone-archive-hygiene.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +import consistencyMod = require('./health-diagnostic-rules/consistency.cjs'); const RULES: Rule[] = [ ...rootExistenceMod.RULES, @@ -93,6 +95,20 @@ const RULES: Rule[] = [ ...milestoneArchiveHygieneMod.RULES, ]; +/** + * `validate.consistency`'s own rule set (Phase 12, #3310, ADR-3180 §8.4, + * design doc "Which rules run where") — W006/W007 REUSED (the exact same + * `Rule` objects `RULES` above already carries, not new copies) plus the + * four new C0NN rules from `consistency.cts`. Deliberately NOT the full + * `RULES` table: running `validate.health`'s config/state/worktree rules + * under `validate.consistency` would be scope creep the issue never asked + * for. + */ +const CONSISTENCY_RULES: Rule[] = [ + ...roadmapDiskConsistencyMod.RULES.filter((r) => ['W006', 'W007'].includes(r.code)), + ...consistencyMod.RULES, +]; + // ─── Repair-handler runtime dependencies ─────────────────────────────────── // // Same owners `cmdValidateHealth`'s pre-migration repair switch used @@ -143,6 +159,15 @@ function evaluateRules(snapshot: PlanningSnapshot): Diagnostic[] { return evaluateRuleTable(RULES, snapshot); } +/** + * Evaluate `CONSISTENCY_RULES` (W006/W007 + C001-C004) against `snapshot` — + * `validate.consistency`'s evaluator entry point, mirroring `evaluateRules` + * exactly but over the smaller, command-specific rule subset. + */ +function evaluateConsistencyRules(snapshot: PlanningSnapshot): Diagnostic[] { + return evaluateRuleTable(CONSISTENCY_RULES, snapshot); +} + // ─── Repair dispatcher ────────────────────────────────────────────────────── // Repair-handler bodies (real, ported from verify.cts:2405-2553). @@ -475,6 +500,9 @@ const healthDiagnostic = { // rule array without mutating the real, still-empty `RULES` export. evaluateRuleTable, applyRepairs, + // Phase 12 (#3310) — `validate.consistency`'s own rule subset + evaluator. + CONSISTENCY_RULES, + evaluateConsistencyRules, }; // Namespace merge (same binding name as the value above) is how a CommonJS diff --git a/src/planning-snapshot.cts b/src/planning-snapshot.cts index 3c42f6099..e918a96fd 100644 --- a/src/planning-snapshot.cts +++ b/src/planning-snapshot.cts @@ -39,6 +39,9 @@ import { platformReadSync, execGit } from './shell-command-projection.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import frontmatterMod = require('./frontmatter.cjs'); const { extractFrontmatter, stripFrontmatter } = frontmatterMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- core-utils.cjs is an export= CommonJS module +import coreUtilsMod = require('./core-utils.cjs'); +const { findOrphanSummaries } = coreUtilsMod; import { stateFieldValue, stateCurrentPositionSlice } from './state-document.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import unusableInputMod = require('./unusable-input.cjs'); @@ -197,6 +200,16 @@ interface PlanningSnapshot { // original behavior. This field is W026's own, independently-scoped // phase-id list — additive-only, no change to `roadmapDeclaredPhases`. currentMilestoneRoadmapPhaseIds: { value: string[]; scope: Scope }; + // ─── Phase 12 (#3310, ADR-3180 §8.4) additions ───────────────────────────── + // Backs C002/C003/C004 (`src/health-diagnostic-rules/consistency.cts`, + // `cmdValidateConsistency`'s migration target). All three relocate + // `verify.cts:1556-1603`'s per-phase-directory plan scan verbatim; see + // `buildPerPhasePlanScanFields`'s own doc comment for why the three share + // one builder and one enumeration base (`allPhaseDirNames`, NOT the + // current-milestone-windowed `phaseDirs`). + perPhasePlanNumbering: { value: { phaseDir: string; planNums: number[] }[]; scope: Scope }; + perPhaseOrphanSummaries: { value: { phaseDir: string; orphanSummary: string }[]; scope: Scope }; + perPhaseWaveMissingPlans: { value: { phaseDir: string; plan: string }[]; scope: Scope }; } /** @@ -787,6 +800,104 @@ function buildCurrentMilestoneRoadmapPhaseIdsField( return { value, scope: SCOPE.COMPLETE }; } +/** + * Resolve `perPhasePlanNumbering`/`perPhaseOrphanSummaries`/ + * `perPhaseWaveMissingPlans` — Phase 12 (#3310, ADR-3180 §8.4), backing + * C002/C003/C004. One shared per-phase-directory scan serves all three + * fields (mirrors `buildStateFields`'s "one builder, several named outputs" + * convention above): each of the three questions below reads the exact same + * `scanPhasePlans(fullPhaseDir)` result, so scanning each phase directory + * three separate times (one function per field) would triple the + * `readdirSync`/frontmatter-read cost for zero behavioral gain — the three + * subjects are independent QUESTIONS, not independent SCANS. + * + * Enumerated over `allPhaseDirNames`, NOT `phaseDirs` (the + * current-milestone-windowed twin): the pre-migration `cmdValidateConsistency` + * (`verify.cts:1521-1608`) walks `collectPhaseRoots(planBase)`'s flat + * `phases/` root via a plain, unfiltered `readdirSync` — every phase + * directory on disk, not just the ones the current milestone window + * resolves as "in scope" — exactly the un-windowed shape `allPhaseDirNames` + * already exposes for W007 (see that field's own doc comment). Using the + * windowed `phaseDirs` here would silently narrow C002/C003/C004's coverage + * relative to the behavior being relocated. Disclosed fidelity note: this + * does NOT walk `collectPhaseRoots`'s second root (an active archived + * milestone's `-phases/` directory) — `allPhaseDirNames` is scoped to + * the flat `phases/` root only, the same scope every other + * `allPhaseDirNames`-sourced field already carries. + * + * QUESTION 1 — `perPhasePlanNumbering`: the sorted list of `-NN-PLAN.md` + * sequence numbers physically present (superseded or not — a retired plan + * still occupied a number), from `allPlanFiles` via the exact + * `/-(\d{2})-PLAN\.md$/` regex `verify.cts:1558` already uses. This field + * exposes the raw per-phase number list only; the future C002 rule computes + * the gap itself. + * + * QUESTION 2 — `perPhaseOrphanSummaries`: every SUMMARY.md with no matching + * LIVE PLAN.md, via `findOrphanSummaries(planFiles, summaryFiles)` + * (`core-utils.cjs`, `verify.cts:1584` — the same owner + * `src/health-diagnostic-rules/phase-structure.cts`'s I001 rule already + * consumes indirectly via `PhaseSnapshot.planCount`/`summaryCount`, for the + * INVERSE question). Uses the live (superseded-excluded) `planFiles`, not + * `allPlanFiles` — a superseded plan's summary is still an orphan. + * + * QUESTION 3 — `perPhaseWaveMissingPlans`: every LIVE plan (`planFiles`, + * same live set as Question 2 — a superseded plan legitimately carries no + * `wave`) whose frontmatter has no `wave` key, via `extractFrontmatter`, + * mirroring `verify.cts:1596-1603` exactly. A plan file that cannot be read + * is silently skipped, mirroring `cmdValidateConsistency`'s own outer + * `catch { intentionally empty }` (`verify.cts:1605-1607`) around this exact + * loop — a fail-open match to the pre-migration behavior, not a new scope + * degradation. + */ +function buildPerPhasePlanScanFields( + phasesDir: string, + phaseDirNames: string[], + enumerationScope: Scope, +): { + perPhasePlanNumbering: { value: { phaseDir: string; planNums: number[] }[]; scope: Scope }; + perPhaseOrphanSummaries: { value: { phaseDir: string; orphanSummary: string }[]; scope: Scope }; + perPhaseWaveMissingPlans: { value: { phaseDir: string; plan: string }[]; scope: Scope }; +} { + const planNumbering: { phaseDir: string; planNums: number[] }[] = []; + const orphanSummaries: { phaseDir: string; orphanSummary: string }[] = []; + const waveMissingPlans: { phaseDir: string; plan: string }[] = []; + + for (const phaseDir of phaseDirNames) { + const fullPhaseDir = path.join(phasesDir, phaseDir); + const { allPlanFiles, planFiles, summaryFiles } = scanPhasePlans(fullPhaseDir); + + const planNums = allPlanFiles + .map((p) => { + const m = p.match(/-(\d{2})-PLAN\.md$/); + return m ? parseInt(m[1], 10) : null; + }) + .filter((n): n is number => n !== null) + .sort((a, b) => a - b); + planNumbering.push({ phaseDir, planNums }); + + for (const orphan of findOrphanSummaries(planFiles, summaryFiles)) { + orphanSummaries.push({ phaseDir, orphanSummary: orphan }); + } + + for (const plan of planFiles) { + try { + const planFilePath = path.join(fullPhaseDir, plan); + const content = fs.readFileSync(planFilePath, 'utf-8'); + const fmData = extractFrontmatter(content, planFilePath); + if (!fmData['wave']) waveMissingPlans.push({ phaseDir, plan }); + } catch { + /* unreadable plan file — mirrors verify.cts:1605-1607's own silent skip */ + } + } + } + + return { + perPhasePlanNumbering: { value: planNumbering, scope: enumerationScope }, + perPhaseOrphanSummaries: { value: orphanSummaries, scope: enumerationScope }, + perPhaseWaveMissingPlans: { value: waveMissingPlans, scope: enumerationScope }, + }; +} + /** * Build the full `.planning/` projection for `cwd`. Composes the six §7 * owners named in the design doc's "Owners consumed" table, plus (Phase 11, @@ -802,6 +913,12 @@ function buildPlanningSnapshot(cwd: string): PlanningSnapshot { const phasesValue = phaseDirs.value.map((dir) => buildPhaseSnapshot(paths.phases, dir)); const stateFields = buildStateFields(paths.state); + const allPhaseDirNames = buildAllPhaseDirNamesField(paths.phases); + const perPhasePlanScanFields = buildPerPhasePlanScanFields( + paths.phases, + allPhaseDirNames.value, + allPhaseDirNames.scope, + ); return { cwd: path.resolve(cwd), @@ -823,9 +940,12 @@ function buildPlanningSnapshot(cwd: string): PlanningSnapshot { researchValidationStatus: buildResearchValidationStatusField(paths.phases, phaseDirs.value, phaseDirs.scope), milestoneArchiveStatus: buildMilestoneArchiveStatusField(cwd), planningRootFiles: buildPlanningRootFilesField(cwd), - allPhaseDirNames: buildAllPhaseDirNamesField(paths.phases), + allPhaseDirNames, archivedPhaseTokens: buildArchivedPhaseTokensField(paths.planning), currentMilestoneRoadmapPhaseIds: buildCurrentMilestoneRoadmapPhaseIdsField(cwd, paths.roadmap), + perPhasePlanNumbering: perPhasePlanScanFields.perPhasePlanNumbering, + perPhaseOrphanSummaries: perPhasePlanScanFields.perPhaseOrphanSummaries, + perPhaseWaveMissingPlans: perPhasePlanScanFields.perPhaseWaveMissingPlans, }; } diff --git a/src/state.cts b/src/state.cts index a7226114e..fafffbb2d 100644 --- a/src/state.cts +++ b/src/state.cts @@ -68,6 +68,11 @@ import type { HeadingToken } from './markdown-sectionizer.cjs'; import { parseMarkdownTable, updateTableCell, deleteTableRow, insertTableRow, splitTableRow, isDelimiterRow } from './markdown-table.cjs'; import { textEncodingError } from './validate.cjs'; import { clampPercent } from './phase-lifecycle.cjs'; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import healthDiagnosticTypesMod = require('./health-diagnostic-types.cjs'); +const { SEVERITY, adviseRemedy } = healthDiagnosticTypesMod; +type Severity = healthDiagnosticTypesMod.Severity; +type Diagnostic = healthDiagnosticTypesMod.Diagnostic; // ─── Types ──────────────────────────────────────────────────────────────────── @@ -3244,6 +3249,22 @@ function readStateFrontmatterScoped(content: string, statePath: string): { fm: R return { fm, body, scope }; } +/** + * Builds an S0NN `Diagnostic` for `cmdStateValidate` (§8.4 rule 3 — + * `cmdStateValidate` is a plain imperative function, not a `Rule.check`, so + * it builds `Diagnostic[]` directly rather than going through + * `evaluateRuleTable`/the `RULES` array machinery). Every S0NN subject is + * advisory-only today (`cmdStateValidate` has never had a repair path), so + * every remedy is `adviseRemedy` — `advice` is the short imperative command + * text shown to the operator, matching the style Phase 11's rule-group files + * already use for their own ADVISE-only findings (e.g. + * `roadmap-disk-consistency.cts`'s `adviseRemedy('Create phase directory or + * remove from roadmap')`). + */ +function stateDiagnostic(code: string, severity: Severity, message: string, advice: string): Diagnostic { + return { code, severity, message, remedy: adviseRemedy(advice) }; +} + function cmdStateValidate(cwd: string, raw: boolean): void { const statePath = planningPaths(cwd).state; if (!fs.existsSync(statePath)) { @@ -3257,11 +3278,17 @@ function cmdStateValidate(cwd: string, raw: boolean): void { // searchers downstream, reading as "absent" rather than "corrupt." const encErr = textEncodingError(content, 'STATE.md'); if (encErr) { - output({ valid: false, warnings: [encErr], drift: {} }, raw, undefined); + // S001 — error-class severity (this branch has always set `valid: false` + // unconditionally and returned immediately, matching every other + // error-class code, not a mere warning). Message reused verbatim from + // `textEncodingError`, not paraphrased. + output({ + valid: false, + warnings: [stateDiagnostic('S001', SEVERITY.ERROR, encErr, 'Re-save STATE.md as UTF-8 text with the embedded NUL byte(s) removed')], + }, raw, undefined); return; } - const warnings: string[] = []; - const drift: Record = {}; + const warnings: Diagnostic[] = []; // #1255/#3187: parse frontmatter and strip it from the body ONCE, so the // chain owner sees the same fm/body precedence every other migrated call @@ -3279,20 +3306,32 @@ function cmdStateValidate(cwd: string, raw: boolean): void { const phasesDir = planningPaths(cwd).phases; if (currentPhase === null) { - warnings.push('Cannot validate phase drift: STATE.md has no usable current_phase, Current Phase, or Current Position Phase value'); - drift['phase_reference'] = { reason: 'unresolved', selected: null, sources: resolvedPhase.sources }; - output({ valid: false, warnings, drift, scope }, raw, undefined); + warnings.push(stateDiagnostic( + 'S002', + SEVERITY.WARNING, + 'Cannot validate phase drift: STATE.md has no usable current_phase, Current Phase, or Current Position Phase value', + 'Set current_phase (frontmatter) or Current Phase / Current Position Phase (body) in STATE.md', + )); + output({ valid: false, warnings, scope }, raw, undefined); return; } const selectedPhaseKey = phaseKeyFromToken(currentPhase); if (Object.values(resolvedPhase.sources).some(source => source !== null && phaseKeyFromToken(source) !== selectedPhaseKey)) { - warnings.push(`Phase reference conflict: validating authoritative phase ${currentPhase}; align STATE.md phase sources`); - drift['phase_reference'] = { reason: 'conflict', selected: currentPhase, sources: resolvedPhase.sources }; + warnings.push(stateDiagnostic( + 'S003', + SEVERITY.WARNING, + `Phase reference conflict: validating authoritative phase ${currentPhase}; align STATE.md phase sources`, + 'Align STATE.md phase sources (frontmatter, Current Phase, Current Position Phase) on one phase', + )); } if (!fs.existsSync(phasesDir)) { - warnings.push(`Cannot validate phase drift: phases directory is missing for phase ${currentPhase}`); - drift['phase_directory'] = { reason: 'missing_root', selected: currentPhase }; - output({ valid: false, warnings, drift, scope }, raw, undefined); + warnings.push(stateDiagnostic( + 'S004', + SEVERITY.WARNING, + `Cannot validate phase drift: phases directory is missing for phase ${currentPhase}`, + 'Create the phases directory or correct current_phase to a phase that exists on disk', + )); + output({ valid: false, warnings, scope }, raw, undefined); return; } let phaseDirPath: string; @@ -3300,16 +3339,24 @@ function cmdStateValidate(cwd: string, raw: boolean): void { const entries = fs.readdirSync(phasesDir, { withFileTypes: true }); const phaseDir = entries.find(entry => entry.isDirectory() && phaseKeyFromDir(entry.name) === selectedPhaseKey); if (!phaseDir) { - warnings.push(`Cannot validate phase drift: no phase directory matches phase ${currentPhase}`); - drift['phase_directory'] = { reason: 'not_found', selected: currentPhase }; - output({ valid: false, warnings, drift, scope }, raw, undefined); + warnings.push(stateDiagnostic( + 'S004', + SEVERITY.WARNING, + `Cannot validate phase drift: no phase directory matches phase ${currentPhase}`, + 'Create a phase directory matching the current phase or correct current_phase', + )); + output({ valid: false, warnings, scope }, raw, undefined); return; } phaseDirPath = path.join(phasesDir, phaseDir.name); } catch { - warnings.push(`Cannot validate phase drift: phases directory is unreadable for phase ${currentPhase}`); - drift['phase_directory'] = { reason: 'unreadable', selected: currentPhase }; - output({ valid: false, warnings, drift, scope }, raw, undefined); + warnings.push(stateDiagnostic( + 'S004', + SEVERITY.WARNING, + `Cannot validate phase drift: phases directory is unreadable for phase ${currentPhase}`, + 'Check phases directory permissions and re-run validate', + )); + output({ valid: false, warnings, scope }, raw, undefined); return; } try { @@ -3321,8 +3368,12 @@ function cmdStateValidate(cwd: string, raw: boolean): void { // Check plan count mismatch if (totalPlansInPhase !== null && diskPlans !== totalPlansInPhase) { - warnings.push(`Plan count mismatch: STATE.md says ${totalPlansInPhase} plans, disk has ${diskPlans}`); - drift['plan_count'] = { state: totalPlansInPhase, disk: diskPlans }; + warnings.push(stateDiagnostic( + 'S005', + SEVERITY.WARNING, + `Plan count mismatch: STATE.md says ${totalPlansInPhase} plans, disk has ${diskPlans}`, + 'Run state sync or correct Total Plans in Phase to match the plans on disk', + )); } // Check for VERIFICATION.md @@ -3332,8 +3383,12 @@ function cmdStateValidate(cwd: string, raw: boolean): void { try { const vContent = fs.readFileSync(path.join(phaseDirPath, vf), 'utf-8'); if (/status:\s*passed/i.test(vContent) && /executing/i.test(status)) { - warnings.push(`Status drift: STATE.md says "${status}" but ${vf} shows verification passed — phase may be complete`); - drift['verification_status'] = { state_status: status, verification: 'passed' }; + warnings.push(stateDiagnostic( + 'S006', + SEVERITY.WARNING, + `Status drift: STATE.md says "${status}" but ${vf} shows verification passed — phase may be complete`, + 'Run state complete-phase (or otherwise advance STATE.md status past "executing")', + )); } } catch { /* best-effort (#2245 audit): cmdStateValidate is a diagnostic * warnings scan across N VERIFICATION.md files — one unreadable file @@ -3346,16 +3401,29 @@ function cmdStateValidate(cwd: string, raw: boolean): void { if (diskPlans > 0 && diskSummaries >= diskPlans && /executing/i.test(status)) { // Only warn if no verification exists (if verification passed, the above warning covers it) if (verificationFiles.length === 0) { - warnings.push(`All ${diskPlans} plans have summaries but status is still "${status}" — phase may be ready for verification`); + // S007 stays WARNING (not INFO): closely related to S006 (both + // signal "phase may be ready to advance"), and S006 is WARNING — + // giving the sibling condition a different severity for the same + // underlying signal would be a false distinction. + warnings.push(stateDiagnostic( + 'S007', + SEVERITY.WARNING, + `All ${diskPlans} plans have summaries but status is still "${status}" — phase may be ready for verification`, + 'Run phase verification, then advance STATE.md status past "executing"', + )); } } } catch { - warnings.push(`Cannot validate phase drift: phase directory is unreadable for phase ${currentPhase}`); - drift['phase_directory'] = { reason: 'unreadable', selected: currentPhase }; + warnings.push(stateDiagnostic( + 'S004', + SEVERITY.WARNING, + `Cannot validate phase drift: phase directory is unreadable for phase ${currentPhase}`, + 'Check phase directory permissions and re-run validate', + )); } const valid = warnings.length === 0; - output({ valid, warnings, drift, scope }, raw, undefined); + output({ valid, warnings, scope }, raw, undefined); } /** diff --git a/src/verify.cts b/src/verify.cts index 30a3658e5..8fa988a15 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -9,8 +9,7 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; -import { phaseVariants, buildRoadmapPhaseVariants } from './validate.cjs'; -import { PHASE_TOKEN_FROM_DIR_RE, MILESTONE_ARCHIVE_DIR_RE, textEncodingError } from './validate.cjs'; +import { MILESTONE_ARCHIVE_DIR_RE, textEncodingError } from './validate.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-workspace.cjs is an export= CommonJS module import planningWorkspace = require('./planning-workspace.cjs'); // eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module @@ -27,7 +26,7 @@ const { findOrphanSummaries, findUnsummarizedPlans } = coreUtilsMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-scope.cjs is an export= CommonJS module import planningScopeMod = require('./planning-scope.cjs'); const { SCOPE } = planningScopeMod; -import { execGit, platformReadSync as safeReadFile, posixNormalize } from './shell-command-projection.cjs'; +import { execGit, platformReadSync as safeReadFile } from './shell-command-projection.cjs'; import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; import { detectSchemaFiles, checkSchemaDrift } from './schema-detect.cjs'; import { extractTaggedBlocks } from './markdown-sectionizer.cjs'; @@ -38,20 +37,17 @@ const { checkAgentsInstalled, checkCodexModelPosture } = agentInstallCheck; import ioMod = require('./io.cjs'); const { output, error } = ioMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -import configLoaderMod = require('./config-loader.cjs'); -const { loadConfig } = configLoaderMod; -// eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, isSentinelPhaseId } = phaseIdMod; +const { normalizePhaseName, matchPhaseDirs } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal } = phaseLocatorMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); -const { stripShippedMilestones, extractCurrentMilestone } = roadmapParserMod; +const { stripShippedMilestones } = roadmapParserMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- health-diagnostic.cjs is an export= CommonJS module import healthDiagnosticMod = require('./health-diagnostic.cjs'); -const { SEVERITY: HEALTH_SEVERITY, REMEDY_ACTION, REMEDY_RISK, evaluateRules, applyRepairs } = healthDiagnosticMod; +const { SEVERITY: HEALTH_SEVERITY, REMEDY_ACTION, REMEDY_RISK, evaluateRules, evaluateConsistencyRules, applyRepairs } = healthDiagnosticMod; type HealthDiagnostic = healthDiagnosticMod.Diagnostic; // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-snapshot.cjs is an export= CommonJS module import planningSnapshotMod = require('./planning-snapshot.cjs'); @@ -1293,96 +1289,19 @@ function listMilestoneArchiveDirs(planBase: string): string[] { ); } catch (err) { // #1883: distinguish genuine absence from a permission/I-O failure. ENOENT - // (no milestones/ dir yet) keeps the long-standing [] contract that - // collectPhaseRoots / forEachArchivedPhaseToken depend on for "no archives"; - // every other error (EACCES, EIO, …) must propagate — otherwise an unreadable - // milestones/ dir is silently reported as "no archives" and active-milestone - // resolution / archived-phase filtering misbehaves. + // (no milestones/ dir yet) keeps the long-standing [] contract callers of + // this function depend on for "no archives"; every other error (EACCES, + // EIO, …) must propagate — otherwise an unreadable milestones/ dir is + // silently reported as "no archives" and archived-phase resolution + // misbehaves. As of Phase 12 (#3310), `cmdValidateConsistency`'s own + // caller of this function (`collectPhaseRoots` -> `getActiveMilestoneArchiveDir`) + // was migrated onto `buildPlanningSnapshot` and deleted; this function is + // retained solely for its `_listMilestoneArchiveDirs` test seam below. if ((err as NodeJS.ErrnoException).code === 'ENOENT') return []; throw err; } } -function getActiveMilestoneArchiveDir(planBase: string): string | null { - const archiveDirs = listMilestoneArchiveDirs(planBase); - if (archiveDirs.length === 0) return null; - - try { - const statePath = path.join(planBase, 'STATE.md'); - if (fs.existsSync(statePath)) { - const state = fs.readFileSync(statePath, 'utf-8'); - const m = state.match( - /^\s*(?:\*\*)?milestone(?:\*\*)?:\s*\*{0,2}\s*([^\s*\r\n#][^\s\r\n#]*)/mi, - ); - if (m && m[1]) { - const milestone = m[1].trim(); - const candidate = path.join(planBase, 'milestones', `${milestone}-phases`); - return archiveDirs.includes(candidate) ? candidate : null; - } - } - } catch { - /* intentionally empty — fall through to version-sort below */ - } - - return archiveDirs[archiveDirs.length - 1]; -} - -function collectPhaseRoots(planBase: string): string[] { - const roots: string[] = []; - const flatPhasesDir = path.join(planBase, 'phases'); - if (fs.existsSync(flatPhasesDir)) roots.push(flatPhasesDir); - const activeArchive = getActiveMilestoneArchiveDir(planBase); - if (activeArchive) roots.push(activeArchive); - return roots; -} - -/** - * #2528: the disk-side phase inventory, keyed by extracted token but KEEPING the - * directory names behind each token. - * - * The token alone is what made `validate health` the ninth site of the #2528 - * class. W006/W007 pair roadmap phases against disk by intersecting TOKEN SETS - * (`phaseVariants(p)` vs these keys), which is a dir→token labelling, not the - * query→dir selection `matchPhaseDirs` owns — so a `grep phaseTokenMatches` - * never surfaced it. On a digit-leading slug the label is wrong in both - * directions at once: `05-80-20-cleanup` labels itself `05-80-20`, so phase 5 - * "has no directory" (W006) AND that directory "is not in the roadmap" (W007). - * - * Carrying the names lets both warnings ask the canonical matcher whether a - * roadmap phase actually resolves to a directory, instead of asking whether two - * independently-derived labels happen to be equal. - */ -function collectDiskPhaseEntries(planBase: string): Map { - const entriesByToken = new Map(); - const phaseRoots = collectPhaseRoots(planBase); - const scanDir = (dir: string) => { - try { - const entries = fs.readdirSync(dir, { withFileTypes: true }); - for (const e of entries) { - if (e.isDirectory()) { - const m = e.name.match(PHASE_TOKEN_FROM_DIR_RE); - if (!m) continue; - const token = stripProjectCodePrefix(m[1]); - const dirs = entriesByToken.get(token); - if (dirs) dirs.push(e.name); - else entriesByToken.set(token, [e.name]); - } - } - } catch { - /* dir absent */ - } - }; - - for (const root of phaseRoots) scanDir(root); - - return entriesByToken; -} - -function collectDiskPhases(planBase: string): Set { - return new Set(collectDiskPhaseEntries(planBase).keys()); -} - - interface IssueEntry { code: string; message: string; @@ -1467,144 +1386,41 @@ function cmdValidateConsistency(cwd: string, raw: boolean): void { const planBase = planningDir(cwd); const roadmapPath = path.join(planBase, 'ROADMAP.md'); const errors: string[] = []; - const warnings: string[] = []; + const warnings: IssueEntry[] = []; + // Pre-check, stays OUTSIDE the rule table — same shape as `validate.health`'s + // E001/E010/I010 (design doc, "Which rules run where"). `.planning/` cannot + // build a `PlanningSnapshot` worth evaluating without a ROADMAP.md to read. if (!fs.existsSync(roadmapPath)) { errors.push('ROADMAP.md not found'); output({ passed: false, errors, warnings }, raw, 'failed'); return; } - const roadmapContentRaw = fs.readFileSync(roadmapPath, 'utf-8'); - const roadmapContent = extractCurrentMilestone(roadmapContentRaw, cwd); + // ─── Rule-table evaluation (Phase 12, #3310) ─────────────────────────────── + // Replaces this function's entire hand-rolled disk-vs-roadmap / + // numbering-gap / orphan-summary / wave-missing scan — see the design + // doc's "Which rules run where" section. `evaluateConsistencyRules` runs + // W006/W007 (the SAME `Rule` objects `validate.health` evaluates, reused + // verbatim — not a second, independently-drifting copy) plus the four new + // C001-C004 rules, against the same `PlanningSnapshot` `validate.health` + // builds. + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = evaluateConsistencyRules(snapshot); - const { roadmapPhases } = buildRoadmapPhaseVariants(roadmapContent); - const { roadmapPhaseVariants: fullRoadmapPhaseVariants } = buildRoadmapPhaseVariants(roadmapContentRaw); - - const diskPhases = collectDiskPhases(planBase); - - for (const p of roadmapPhases) { - // #3225: sentinel phase ids are never-on-roadmap by convention. - if (isSentinelPhaseId(p)) continue; - if (!diskPhases.has(p) && !diskPhases.has(normalizePhaseName(p))) { - warnings.push(`Phase ${p} in ROADMAP.md but no directory on disk`); - } - } - - for (const p of diskPhases) { - // #3225: a sentinel dir on disk (999-interim, 0-drafts) is defined as - // never-on-roadmap; it must not warn here (same guard as cmdValidateHealth). - if (isSentinelPhaseId(p)) continue; - const variants = phaseVariants(p); - if (![...variants].some((v) => fullRoadmapPhaseVariants.has(v))) { - warnings.push(`Phase ${p} exists on disk but not in ROADMAP.md`); - } - } - - const config = loadConfig(cwd); - if (config.phase_naming !== 'custom') { - const integerPhases = [...diskPhases] - // #3225: exclude sentinel phase ids (999.x/0.x) — they are never part of the - // sequential numbering, so a 999-interim dir must not produce a spurious - // "Gap in phase numbering: N → 999". - .filter((p) => !p.includes('.') && !isSentinelPhaseId(p)) - .map((p) => parseInt(p, 10)) - .sort((a, b) => a - b); - - for (let i = 1; i < integerPhases.length; i++) { - if (integerPhases[i] !== integerPhases[i - 1] + 1) { - warnings.push(`Gap in phase numbering: ${integerPhases[i - 1]} → ${integerPhases[i]}`); - } - } - } - - const phaseRoots = collectPhaseRoots(planBase); - for (const phaseRoot of phaseRoots) { - try { - const entries = fs.readdirSync(phaseRoot, { withFileTypes: true }); - const dirs = entries - .filter((e) => e.isDirectory()) - .map((e) => e.name) - .sort(); - - for (const dir of dirs) { - const phasePath = path.join(phaseRoot, dir); - const phaseLabel = posixNormalize(path.relative(planBase, phasePath)); - - // #3183: this loop mixes two DIFFERENT questions — split explicitly - // rather than migrating it as one blind swap-in. - - // QUESTION 1 — physical numbering-gap detection: wants EVERY plan - // file that physically exists, superseded or not (a retired plan - // still occupied a number in the sequence), root+nested. Uses the - // single owner's allPlanFiles rather than a root-only readdirSync - // filter. The strict `-NN-PLAN.md` suffix regex below already - // ignores any entry (nested, loose-named, bare PLAN.md) that isn't - // in the root canonical numbered form, so widening the input set is - // a pure visibility fix with no change to which files feed a number. - // - // One scan serves both questions below: `allPlanFiles` answers - // Question 1 (numbering-gap), `planFiles`/`summaryFiles` answer - // Question 2 (pairing) a few lines down. - const { allPlanFiles, planFiles, summaryFiles } = planScanMod.scanPhasePlans(phasePath); - - // Root-canonical numbered plans only (`-NN-PLAN.md`) — the - // shape the numbering-gap sequence check operates on. Matched via - // the numbering regex itself rather than a separate suffix filter, - // so this stays a single derivation from the owner's output, not a - // second independent re-derivation of its filename grammar. - const numberedPlans = allPlanFiles - .map((p) => { - const pm = p.match(/-(\d{2})-PLAN\.md$/); - return pm ? { file: p, num: parseInt(pm[1], 10) } : null; - }) - .filter((e): e is { file: string; num: number } => e !== null) - .sort((a, b) => a.num - b.num); - // numberedPlans (and planNums below) answers Question 1 ONLY — the - // numbering-gap sequence check. It is a strict `-NN-PLAN.md` subset - // and must NOT be reused as a general "all live plans" set: a plan - // whose filename isn't in that canonical 2-digit form (a 3-digit - // continuation, a bare PLAN.md, etc.) is silently absent from it. - const planNums = numberedPlans.map((e) => e.num); - - for (let i = 1; i < planNums.length; i++) { - if (planNums[i] !== planNums[i - 1] + 1) { - warnings.push( - `Gap in plan numbering in ${phaseLabel}: plan ${planNums[i - 1]} → ${planNums[i]}`, - ); - } - } - - // QUESTION 2 — plan↔summary pairing: "does this summary have a - // matching LIVE plan" wants the single owner's superseded-excluded - // planFiles and the canonical summaryCandidates-based pairing - // (findOrphanSummaries) instead of an exact-suffix Set-diff, which - // produced false "orphan summary" warnings for legacy/nested naming - // forms it could not recognize as paired. - const orphanSummaries = findOrphanSummaries(planFiles, summaryFiles); - for (const orphan of orphanSummaries) { - warnings.push(`Summary ${orphan} in ${phaseLabel} has no matching PLAN.md`); - } - - // QUESTION 3 — wave-frontmatter presence: "does every LIVE plan - // declare a wave" wants the same live (superseded-excluded) set as - // Question 2's pairing check — planFiles, NOT numberedPlans/Question - // 1's strict 2-digit subset. A superseded plan legitimately carries - // no wave, and a live plan whose filename isn't in canonical 2-digit - // form (a 3-digit continuation, a bare PLAN.md, etc.) must still be - // checked here even though it is invisible to the numbering-gap scan. - for (const plan of planFiles) { - const planFilePath = path.join(phasePath, plan); - const content = fs.readFileSync(planFilePath, 'utf-8'); - const fmData = extractFrontmatter(content, planFilePath); - if (!fmData['wave']) { - warnings.push(`${phaseLabel}/${plan}: missing 'wave' in frontmatter`); - } - } - } - } catch { - /* intentionally empty */ - } + // Every current C0NN/W006/W007 diagnostic is `SEVERITY.WARNING` (confirmed + // by direct read of `consistency.cts`/`roadmap-disk-consistency.cts`) — + // matching the pre-migration shape, where only the ROADMAP-missing + // pre-check above ever populated `errors`. Bucketed defensively by + // severity anyway, mirroring `cmdValidateHealth`'s own pattern, so a + // future ERROR-severity rule added to `CONSISTENCY_RULES` lands in the + // right bucket without another migration. + const _slashRuntime = resolveRuntime(cwd); + const slash = (name: string) => formatGsdSlash(name, _slashRuntime) as string; + for (const diagnostic of diagnostics) { + const entry = diagnosticToIssueEntry(diagnostic, slash); + if (diagnostic.severity === HEALTH_SEVERITY.ERROR) errors.push(entry.message); + else warnings.push(entry); } const passed = errors.length === 0; diff --git a/tests/health-diagnostic-rules/consistency.test.cjs b/tests/health-diagnostic-rules/consistency.test.cjs new file mode 100644 index 000000000..442c69803 --- /dev/null +++ b/tests/health-diagnostic-rules/consistency.test.cjs @@ -0,0 +1,346 @@ +'use strict'; + +/** + * Tests for `src/health-diagnostic-rules/consistency.cts` (Phase 12, #3310, + * ADR-3180 §8.4) — the "consistency" (`C0NN`) rule group: C001-C004, plus + * `health-diagnostic.cts`'s `CONSISTENCY_RULES`/`evaluateConsistencyRules` + * composition (W006/W007 reuse proof). + * + * Design: .gsd/phase/feat-3310-enhance-3180-the-sibling-validators-shar/40-design.md + * Test matrix: .gsd/phase/feat-3310-enhance-3180-the-sibling-validators-shar/50-test-matrix.md + * section 2 (rows 9-11), section 3 (C001-C004) + * + * Fixture provenance (§8.5 + CONTRIBUTING "Fixture provenance (#2371)"): + * - C001/C002/C004 are MECHANICAL MUTATION — a numbering gap / a missing + * `wave:` line, mirroring the exact fixture shapes + * `tests/planning-snapshot.test.cjs`'s Phase-12 field tests already use + * for the same underlying `PlanningSnapshot` fields. + * - C003 REUSES `tests/health-diagnostic-rules/phase-structure.test.cjs`'s + * I001 fixture family (a live PLAN.md with no matching SUMMARY.md), + * inverted: a SUMMARY.md with no matching live PLAN.md. + * + * Every case calls the REAL `buildPlanningSnapshot(cwd)` against a real + * temp `.planning/` tree, then the REAL rule `check` functions from the + * compiled module under test — no hand-built in-memory snapshot mocks, + * mirroring `tests/health-diagnostic-rules/roadmap-disk-consistency.test.cjs`. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { createTempDir, cleanup } = require('../helpers.cjs'); + +const { buildPlanningSnapshot } = require('../../gsd-core/bin/lib/planning-snapshot.cjs'); +const consistencyMod = require('../../gsd-core/bin/lib/health-diagnostic-rules/consistency.cjs'); +const { RULES } = consistencyMod; +const { + SEVERITY, + REMEDY_ACTION, + REMEDY_RISK, + CONSISTENCY_RULES, + evaluateConsistencyRules, + evaluateRules, +} = require('../../gsd-core/bin/lib/health-diagnostic.cjs'); + +function ruleFor(code) { + const rule = RULES.find((r) => r.code === code); + assert.ok(rule, `rule ${code} not found in RULES`); + return rule; +} + +// ─── Fixture helpers (mirrors tests/planning-snapshot.test.cjs) ──────────── + +function planningDirOf(cwd) { + return path.join(cwd, '.planning'); +} + +function writeRoadmap(cwd, content) { + fs.mkdirSync(planningDirOf(cwd), { recursive: true }); + fs.writeFileSync(path.join(planningDirOf(cwd), 'ROADMAP.md'), content); +} + +function writeFile(cwd, relPath, content) { + const full = path.join(cwd, relPath); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content); +} + +function makePhaseDir(cwd, dirName) { + fs.mkdirSync(path.join(planningDirOf(cwd), 'phases', dirName), { recursive: true }); +} + +function writePlan(cwd, relPhasePath, planName, frontmatterLines) { + const lines = frontmatterLines ? ['---', ...frontmatterLines, '---', '', '# Plan', ''] : ['# Plan', '']; + writeFile(cwd, `${relPhasePath}/${planName}`, lines.join('\n')); +} + +// ─── RULES shape ──────────────────────────────────────────────────────────── + +describe('RULES (consistency group)', () => { + test('exports exactly 4 rules: C001, C002, C003, C004', () => { + assert.deepEqual(RULES.map((r) => r.code).sort(), ['C001', 'C002', 'C003', 'C004']); + }); + + test('all four rules are severity WARNING with an ADVISE remedy shape', () => { + for (const code of ['C001', 'C002', 'C003', 'C004']) { + assert.equal(ruleFor(code).severity, SEVERITY.WARNING); + assert.equal(ruleFor(code).repairable, false); + } + }); +}); + +// ─── C001 — gap in disk phase numbering ──────────────────────────────────── + +describe('C001 — gap in disk phase numbering', () => { + test('MECHANICAL MUTATION: phases 1, 2, 4 on disk (3 missing) fires C001', (t) => { + const cwd = createTempDir('gsd-3310-c001-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ['## v1.0 Current 🚧', ''].join('\n')); + makePhaseDir(cwd, '01-foo'); + makePhaseDir(cwd, '02-bar'); + makePhaseDir(cwd, '04-baz'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('C001').check(snapshot); + + assert.equal(diagnostics.length, 1); + assert.deepEqual(diagnostics[0], { + code: 'C001', + severity: SEVERITY.WARNING, + message: 'Gap in phase numbering: 2 → 4', + remedy: { + action: REMEDY_ACTION.ADVISE, + risk: REMEDY_RISK.NONE, + args: { command: 'Create the missing phase directory or renumber to close the gap' }, + }, + }); + }); + + test('baseline: a sequential integer run produces no diagnostics', (t) => { + const cwd = createTempDir('gsd-3310-c001-neg-'); + t.after(() => cleanup(cwd)); + makePhaseDir(cwd, '01-foo'); + makePhaseDir(cwd, '02-bar'); + makePhaseDir(cwd, '03-baz'); + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(ruleFor('C001').check(snapshot), []); + }); + + test('boundary: phase_naming "custom" skips the integer-sequence gap check entirely', (t) => { + const cwd = createTempDir('gsd-3310-c001-custom-'); + t.after(() => cleanup(cwd)); + fs.mkdirSync(planningDirOf(cwd), { recursive: true }); + fs.writeFileSync( + path.join(planningDirOf(cwd), 'config.json'), + JSON.stringify({ phase_naming: 'custom' }, null, 2), + ); + makePhaseDir(cwd, '01-foo'); + makePhaseDir(cwd, '04-baz'); + + const snapshot = buildPlanningSnapshot(cwd); + assert.equal(snapshot.config.value?.['phase_naming'], 'custom'); + assert.deepEqual(ruleFor('C001').check(snapshot), []); + }); + + test('a sentinel phase dir (999-interim) does not produce a spurious gap', (t) => { + const cwd = createTempDir('gsd-3310-c001-sentinel-'); + t.after(() => cleanup(cwd)); + makePhaseDir(cwd, '01-foo'); + makePhaseDir(cwd, '02-bar'); + makePhaseDir(cwd, '999-interim'); + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(ruleFor('C001').check(snapshot), []); + }); +}); + +// ─── C002 — gap in plan numbering within a phase ─────────────────────────── + +describe('C002 — gap in plan numbering within a phase', () => { + test('MECHANICAL MUTATION: 01-PLAN.md, 03-PLAN.md in one phase dir fires C002', (t) => { + const cwd = createTempDir('gsd-3310-c002-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + writePlan(cwd, '.planning/phases/01-foo', '01-03-PLAN.md'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('C002').check(snapshot); + + assert.equal(diagnostics.length, 1); + assert.deepEqual(diagnostics[0], { + code: 'C002', + severity: SEVERITY.WARNING, + message: 'Gap in plan numbering in 01-foo: plan 1 → 3', + remedy: { + action: REMEDY_ACTION.ADVISE, + risk: REMEDY_RISK.NONE, + args: { command: 'Create the missing plan or renumber to close the gap' }, + }, + }); + }); + + test('boundary: a gap of exactly one (01, 02) still reports the same C002 subject', (t) => { + const cwd = createTempDir('gsd-3310-c002-gap1-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + writePlan(cwd, '.planning/phases/01-foo', '01-02-PLAN.md'); + + const snapshot = buildPlanningSnapshot(cwd); + // Sequential (01, 02) is NOT a gap — sanity baseline for the boundary + // pairing below (a real gap must be > 1, not merely non-empty). + assert.deepEqual(ruleFor('C002').check(snapshot), []); + }); + + test('baseline: sequential plans (01, 02, 03) produce no diagnostics', (t) => { + const cwd = createTempDir('gsd-3310-c002-neg-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + writePlan(cwd, '.planning/phases/01-foo', '01-02-PLAN.md'); + writePlan(cwd, '.planning/phases/01-foo', '01-03-PLAN.md'); + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(ruleFor('C002').check(snapshot), []); + }); +}); + +// ─── C003 — orphan SUMMARY with no matching live PLAN ────────────────────── +// Reuses phase-structure.test.cjs's I001 fixture family, inverted. + +describe('C003 — orphan SUMMARY with no matching live PLAN', () => { + test('REUSED (I001 family, inverted): a SUMMARY.md with no matching PLAN.md fires C003', (t) => { + const cwd = createTempDir('gsd-3310-c003-'); + t.after(() => cleanup(cwd)); + writeFile(cwd, '.planning/phases/01-foo/01-01-SUMMARY.md', '# Summary\n'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('C003').check(snapshot); + + assert.equal(diagnostics.length, 1); + assert.deepEqual(diagnostics[0], { + code: 'C003', + severity: SEVERITY.WARNING, + message: 'Summary 01-01-SUMMARY.md in 01-foo has no matching PLAN.md', + remedy: { + action: REMEDY_ACTION.ADVISE, + risk: REMEDY_RISK.NONE, + args: { command: 'Create the matching PLAN.md or remove the orphan summary' }, + }, + }); + }); + + test('boundary: a summary paired only to a superseded plan is still an orphan', (t) => { + const cwd = createTempDir('gsd-3310-c003-superseded-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md', ['status: superseded']); + writeFile(cwd, '.planning/phases/01-foo/01-01-SUMMARY.md', '# Summary\n'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('C003').check(snapshot); + assert.equal(diagnostics.length, 1); + assert.equal(diagnostics[0].code, 'C003', 'a summary paired only to a superseded plan must still fire C003'); + }); + + test('baseline: a paired plan+summary produces no diagnostics', (t) => { + const cwd = createTempDir('gsd-3310-c003-neg-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + writeFile(cwd, '.planning/phases/01-foo/01-01-SUMMARY.md', '# Summary\n'); + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(ruleFor('C003').check(snapshot), []); + }); +}); + +// ─── C004 — PLAN missing 'wave' in frontmatter ───────────────────────────── + +describe("C004 — PLAN missing 'wave' in frontmatter", () => { + test('MECHANICAL MUTATION: a shipped PLAN template with wave: stripped fires C004', (t) => { + const cwd = createTempDir('gsd-3310-c004-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = ruleFor('C004').check(snapshot); + + assert.equal(diagnostics.length, 1); + assert.deepEqual(diagnostics[0], { + code: 'C004', + severity: SEVERITY.WARNING, + message: "01-foo/01-01-PLAN.md: missing 'wave' in frontmatter", + remedy: { + action: REMEDY_ACTION.ADVISE, + risk: REMEDY_RISK.NONE, + args: { command: "Add a 'wave' key to the plan's frontmatter" }, + }, + }); + }); + + test('baseline: a plan with wave: in frontmatter produces no diagnostics', (t) => { + const cwd = createTempDir('gsd-3310-c004-neg-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md', ['wave: 1']); + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(ruleFor('C004').check(snapshot), []); + }); + + test('a superseded plan with no wave is not flagged (superseded plans are excluded from the live set)', (t) => { + const cwd = createTempDir('gsd-3310-c004-superseded-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md', ['status: superseded']); + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(ruleFor('C004').check(snapshot), []); + }); +}); + +// ─── CONSISTENCY_RULES composition (health-diagnostic.cts) ──────────────── + +describe('CONSISTENCY_RULES composition (matrix row 9)', () => { + test('exactly W006, W007, C001-C004 — 6 entries, no duplicates', () => { + const codes = CONSISTENCY_RULES.map((r) => r.code).sort(); + assert.deepEqual(codes, ['C001', 'C002', 'C003', 'C004', 'W006', 'W007']); + assert.equal(new Set(codes).size, 6); + }); +}); + +describe('evaluateConsistencyRules', () => { + test('matrix row 10: an all-clean snapshot produces zero diagnostics', (t) => { + const cwd = createTempDir('gsd-3310-ecr-clean-'); + t.after(() => cleanup(cwd)); + writeRoadmap(cwd, ['## v1.0 Current 🚧', '', '### Phase 1: Foo'].join('\n')); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md', ['wave: 1']); + writeFile(cwd, '.planning/phases/01-foo/01-01-SUMMARY.md', '# Summary\n'); + + const snapshot = buildPlanningSnapshot(cwd); + assert.deepEqual(evaluateConsistencyRules(snapshot), []); + }); + + test('matrix row 11 (W006 reuse proof): evaluateConsistencyRules produces the IDENTICAL Diagnostic evaluateRules would for the same W006 fixture', (t) => { + const cwd = createTempDir('gsd-3310-ecr-w006-'); + t.after(() => cleanup(cwd)); + writeRoadmap( + cwd, + ['## v1.0 Current 🚧', '', '### Phase 1: Foo', '', '### Phase 2: Bar'].join('\n'), + ); + makePhaseDir(cwd, '01-foo'); + // Phase 2 deliberately has no matching directory -> W006. + + const snapshot = buildPlanningSnapshot(cwd); + + const fromConsistency = evaluateConsistencyRules(snapshot).filter((d) => d.code === 'W006'); + const fromHealth = evaluateRules(snapshot).filter((d) => d.code === 'W006'); + + assert.equal(fromConsistency.length, 1); + assert.equal(fromHealth.length, 1); + assert.deepEqual(fromConsistency[0], fromHealth[0]); + assert.deepEqual( + JSON.parse(JSON.stringify(fromConsistency)), + JSON.parse(JSON.stringify(fromHealth)), + 'evaluateConsistencyRules must produce a byte-identical Diagnostic to evaluateRules for the same fixture — proof of reuse, not reimplementation', + ); + }); +}); diff --git a/tests/lint-health-diagnostic-rule-table.test.cjs b/tests/lint-health-diagnostic-rule-table.test.cjs index 3c6363c8a..87e319907 100644 --- a/tests/lint-health-diagnostic-rule-table.test.cjs +++ b/tests/lint-health-diagnostic-rule-table.test.cjs @@ -22,6 +22,8 @@ const { checkFixtureProofInvariant, findHealthDiagnosticTestFiles, PERMANENTLY_INERT_CODES, + STATE_VALIDATE_TEST_FILE, + STATE_VALIDATE_CODES, } = guard; const FAKE_SEVERITY = Object.freeze({ ERROR: 'error', WARNING: 'warning', INFO: 'info' }); @@ -221,6 +223,57 @@ describe('checkFixtureProofInvariant — PERMANENTLY_INERT_CODES exemption (§8. }); }); +// ─── Check 3 — S0NN pass (Phase 12, #3310): checkFixtureProofInvariant +// against the hardcoded STATE_VALIDATE_CODES list and tests/state.test.cjs. +// These codes are not Rule-table entries (cmdStateValidate builds +// Diagnostic[] inline), so this check exercises the SAME +// checkFixtureProofInvariant helper the C0NN pass uses, just fed a +// hardcoded code list instead of a Rule[] array — mirroring the original +// W/E/I test structure above (no code-source-specific behavior to test +// beyond that, since checkFixtureProofInvariant itself is already covered). + +describe('checkFixtureProofInvariant (S0NN pass, #3310)', () => { + test('flags a code with zero mentions anywhere in the scanned test files', (t) => { + const dir = createTempDir('gsd-lint-hd-rt-s0nn-nomention-'); + t.after(() => cleanup(dir)); + const file = writeTempTestFile( + dir, + 'fake-state.test.cjs', + "describe('S001: something', () => { test('fires', () => {}); });\n", + ); + + const rules = [{ code: 'S001' }, { code: 'S999' }]; + const { uncovered } = checkFixtureProofInvariant(rules, [file]); + + assert.deepEqual(uncovered, ['S999']); + }); + + test('passes a code named in a test() block title', (t) => { + const dir = createTempDir('gsd-lint-hd-rt-s0nn-titled-'); + t.after(() => cleanup(dir)); + const file = writeTempTestFile( + dir, + 'fake-state.test.cjs', + "test('S001: STATE.md corrupt (NUL byte) fires with the verbatim textEncodingError message', () => {});\n", + ); + + const rules = [{ code: 'S001' }]; + const { uncovered } = checkFixtureProofInvariant(rules, [file]); + + assert.deepEqual(uncovered, []); + }); + + test('all 7 real STATE_VALIDATE_CODES (S001-S007) are fixture-covered against the real tests/state.test.cjs', () => { + assert.ok(fs.existsSync(STATE_VALIDATE_TEST_FILE), 'tests/state.test.cjs must exist'); + assert.deepEqual(STATE_VALIDATE_CODES, ['S001', 'S002', 'S003', 'S004', 'S005', 'S006', 'S007']); + + const rules = STATE_VALIDATE_CODES.map((code) => ({ code })); + const { uncovered } = checkFixtureProofInvariant(rules, [STATE_VALIDATE_TEST_FILE], new Map()); + + assert.deepEqual(uncovered, []); + }); +}); + // ─── findHealthDiagnosticTestFiles ───────────────────────────────────────── describe('findHealthDiagnosticTestFiles', () => { diff --git a/tests/lint-planning-artifact-writer-drift.test.cjs b/tests/lint-planning-artifact-writer-drift.test.cjs new file mode 100644 index 000000000..cd8d12169 --- /dev/null +++ b/tests/lint-planning-artifact-writer-drift.test.cjs @@ -0,0 +1,228 @@ +/** + * Tests for the `.planning/`-root artifact writer-registry completeness + * guard (epic #3180, ADR-3180 §8.4 deliverable C, Phase 12 #3310) — + * `scripts/lint-planning-artifact-writer-drift.cjs`. + * + * Unlike its `lint-*-drift.cjs` siblings, this guard is a bare pass/fail + * completeness check (no ratchet baseline) — see the guard's own module + * docblock. Tests exercise the guard's PURE functions directly with + * in-memory strings, mirroring `tests/planning-snapshot-bypass-drift.test.cjs`'s + * structure — no shelling out to the CLI, except the one real-tree + * regression test which imports and calls `scanRepo` in-process. + */ + +'use strict'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); + +const drift = require('../scripts/lint-planning-artifact-writer-drift.cjs'); +const { + scanFileForCandidates, + findArtifactWriterDrift, + scanRepo, + stripLineComment, +} = drift; + +const REPO_ROOT = path.join(__dirname, '..'); +const FAKE_FILE = path.join('src', 'fake-writer.cts'); + +// ─── Case 1: the guard passes clean against the REAL src/ tree ─────────── +// This is the main regression-proof test: the design doc's ground-truth +// sweep found zero current violations (every real root-artifact writer is +// already registered), so a red run here means either a genuine new/newly +// discovered registry gap or a detector regression — never "expected". + +describe('scanRepo — real src/ tree', () => { + test('reports zero violations against the actual src/*.cts tree', () => { + const violations = scanRepo(REPO_ROOT); + assert.deepStrictEqual( + violations, + [], + `unexpected .planning/-root writer-registry violation(s): ${JSON.stringify(violations, null, 2)}`, + ); + }); + + test('the real tree also has REAL statically-resolvable candidates (the detector is not silently inert)', () => { + // A guard that never matches anything would also report zero + // violations — this proves the detector actually found real writers + // (ROADMAP.md/STATE.md/config.json/MILESTONES.md/REQUIREMENTS.md) and + // they were individually checked, not just skipped wholesale. + const driftScanLib = require('../scripts/lib/drift-scan.cjs'); + const candidates = driftScanLib.scanTree({ + root: REPO_ROOT, + scanDirs: drift.SCAN_DIRS, + scanExt: drift.SCAN_EXT, + onFile(rel, text) { + return scanFileForCandidates(text, rel); + }, + }); + assert.ok(candidates.length > 0, 'expected at least one statically-resolvable .planning/-root write in src/*.cts'); + const filenames = new Set(candidates.map((c) => c.filename)); + assert.ok(filenames.has('ROADMAP.md')); + assert.ok(filenames.has('STATE.md')); + assert.ok(filenames.has('config.json')); + }); +}); + +// ─── Case 2: an unregistered literal filename is flagged ───────────────── + +describe('findArtifactWriterDrift — unregistered filename is a violation', () => { + test('a write to an unregistered .planning/-root file is flagged, naming the file', () => { + const source = [ + 'function cmdWriteFoo(cwd) {', + " const fooPath = path.join(planningRoot(cwd), 'FOO.md');", + ' platformWriteSync(fooPath, content);', + '}', + '', + ].join('\n'); + // Injected predicate (no build:lib dependency for this unit test): + // everything is canonical EXCEPT 'FOO.md'. + const isCanonical = (name) => name !== 'FOO.md'; + + const out = findArtifactWriterDrift(source, FAKE_FILE, isCanonical); + assert.strictEqual(out.length, 1); + assert.strictEqual(out[0].filename, 'FOO.md'); + assert.strictEqual(out[0].file, FAKE_FILE.replace(/\\/g, '/')); + assert.strictEqual(out[0].line, 3); + assert.strictEqual(out[0].text, 'platformWriteSync(fooPath, content);'); + }); + + test('a registered filename resolved through the identical path is NOT flagged', () => { + const source = [ + 'function cmdWriteFoo(cwd) {', + " const fooPath = path.join(planningRoot(cwd), 'FOO.md');", + ' platformWriteSync(fooPath, content);', + '}', + '', + ].join('\n'); + const isCanonical = () => true; // everything canonical + const out = findArtifactWriterDrift(source, FAKE_FILE, isCanonical); + assert.deepStrictEqual(out, []); + }); + + test('an inline path.join (no intermediate variable) is also detected and checked', () => { + const source = [ + 'function cmdWriteBar(cwd) {', + " platformWriteSync(path.join(planningRoot(cwd), 'BAR.json'), content);", + '}', + '', + ].join('\n'); + const out = findArtifactWriterDrift(source, FAKE_FILE, (name) => name !== 'BAR.json'); + assert.strictEqual(out.length, 1); + assert.strictEqual(out[0].filename, 'BAR.json'); + }); + + test('an object-literal-property binding (RepairPaths-style) resolves the same as a local const', () => { + const source = [ + 'function repairPaths(cwd) {', + ' const rootBase = planningRoot(cwd);', + ' return {', + " fooPath: path.join(rootBase, 'FOO.md'),", + ' };', + '}', + 'function repair(cwd) {', + ' const { fooPath } = repairPaths(cwd);', + ' platformWriteSync(fooPath, content);', + '}', + '', + ].join('\n'); + const out = findArtifactWriterDrift(source, FAKE_FILE, (name) => name !== 'FOO.md'); + assert.strictEqual(out.length, 1); + assert.strictEqual(out[0].filename, 'FOO.md'); + assert.strictEqual(out[0].line, 9); + }); + + test('planningPaths(cwd). (a full-path property, not a directory) resolves to its known file name', () => { + const source = [ + 'function cmdWriteState(cwd) {', + ' const p = planningPaths(cwd).state;', + ' platformWriteSync(p, content);', + '}', + '', + ].join('\n'); + const out = findArtifactWriterDrift(source, FAKE_FILE, () => false); // everything fails -> must see STATE.md + assert.strictEqual(out.length, 1); + assert.strictEqual(out[0].filename, 'STATE.md'); + }); +}); + +// ─── Case 3: a runtime-computed target is silently skipped ─────────────── + +describe('scanFileForCandidates — dynamic targets are never candidates (not pass, not fail)', () => { + test('a template-literal file name (inline) produces NO candidate at all', () => { + const source = [ + 'function cmdWriteDynamic(cwd, name) {', + ' platformWriteSync(path.join(planningRoot(cwd), `${name}.md`), content);', + '}', + '', + ].join('\n'); + const candidates = scanFileForCandidates(source, FAKE_FILE); + assert.deepStrictEqual(candidates, [], 'a runtime-computed filename must never be reported as a candidate, checked or otherwise'); + }); + + test('a template-literal file name (via an intermediate variable) also produces no candidate', () => { + const source = [ + 'function cmdWriteDynamic(cwd, name) {', + ' const dynPath = path.join(planningRoot(cwd), `${name}.md`);', + ' platformWriteSync(dynPath, content);', + '}', + '', + ].join('\n'); + const candidates = scanFileForCandidates(source, FAKE_FILE); + assert.deepStrictEqual(candidates, []); + }); + + test('findArtifactWriterDrift over the same dynamic-target source reports no violation either (never a false pass or false fail)', () => { + const source = [ + 'function cmdWriteDynamic(cwd, name) {', + ' platformWriteSync(path.join(planningRoot(cwd), `${name}.md`), content);', + '}', + '', + ].join('\n'); + // Even an isCanonical that rejects EVERYTHING must not surface a + // violation here, because the write was never a candidate to begin with. + const out = findArtifactWriterDrift(source, FAKE_FILE, () => false); + assert.deepStrictEqual(out, []); + }); + + test('a workstream-scoped planningDir(cwd, ws) call is ambiguous and is never treated as an unambiguous root', () => { + const source = [ + 'function cmdWriteScoped(cwd, ws) {', + " const p = path.join(planningDir(cwd, ws), 'config.json');", + ' platformWriteSync(p, content);', + '}', + '', + ].join('\n'); + const candidates = scanFileForCandidates(source, FAKE_FILE); + assert.deepStrictEqual(candidates, [], 'planningDir(cwd, ws) can resolve under .planning/workstreams// — must be skipped, not treated as root'); + }); + + test('a nested write (under milestones/) is never treated as a root-level candidate', () => { + const source = [ + 'function cmdArchive(cwd, version) {', + " const archiveDir = path.join(planningRoot(cwd), 'milestones');", + ' platformWriteSync(path.join(archiveDir, `${version}-ROADMAP.md`), content);', + '}', + '', + ].join('\n'); + const candidates = scanFileForCandidates(source, FAKE_FILE); + assert.deepStrictEqual(candidates, []); + }); +}); + +// ─── stripLineComment — comment/string safety ───────────────────────────── + +describe('stripLineComment', () => { + test('strips a trailing // comment but preserves a quoted string containing //', () => { + const line = " const x = 'http://example.com'; // not real"; + assert.strictEqual(stripLineComment(line), " const x = 'http://example.com'; "); + }); + + test('a // inside a string does not truncate the line early', () => { + const line = "platformWriteSync(path.join(planningRoot(cwd), '//weird.md'), c);"; + // The string content is preserved verbatim even though it contains //. + assert.ok(stripLineComment(line).includes("'//weird.md'")); + }); +}); diff --git a/tests/milestone-archive.test.cjs b/tests/milestone-archive.test.cjs index ca7c4f187..77f798d61 100644 --- a/tests/milestone-archive.test.cjs +++ b/tests/milestone-archive.test.cjs @@ -277,8 +277,8 @@ describe('#3164 — validate consistency: milestone-archive layout', () => { const result = runGsdTools('validate consistency', tmpDir); assert.ok(result.success); - const w006 = (JSON.parse(result.output).warnings || []).filter(w => w.includes('Phase 64') && w.includes('no directory')); - assert.deepStrictEqual(w006, [], `Got spurious W006: ${w006.join(', ')}`); + const w006 = (JSON.parse(result.output).warnings || []).filter(w => w.message.includes('Phase 64') && w.message.includes('no directory')); + assert.deepStrictEqual(w006, [], `Got spurious W006: ${JSON.stringify(w006)}`); }); test('no W006 when multiple phases exist in milestone-archive layout', () => { @@ -287,8 +287,8 @@ describe('#3164 — validate consistency: milestone-archive layout', () => { const result = runGsdTools('validate consistency', tmpDir); assert.ok(result.success); - const w006 = (JSON.parse(result.output).warnings || []).filter(w => w.includes('no directory')); - assert.deepStrictEqual(w006, [], `Got spurious W006: ${w006.join(', ')}`); + const w006 = (JSON.parse(result.output).warnings || []).filter(w => w.message.includes('no directory')); + assert.deepStrictEqual(w006, [], `Got spurious W006: ${JSON.stringify(w006)}`); }); test('prefixed archive dir names (CK-64-...) are recognized as phase 64', () => { @@ -297,7 +297,7 @@ describe('#3164 — validate consistency: milestone-archive layout', () => { const result = runGsdTools('validate consistency', tmpDir); assert.ok(result.success); - const w006 = (JSON.parse(result.output).warnings || []).filter(w => w.includes('Phase 64') && w.includes('no directory')); + const w006 = (JSON.parse(result.output).warnings || []).filter(w => w.message.includes('Phase 64') && w.message.includes('no directory')); assert.deepStrictEqual(w006, [], `Prefixed phase dir should count as phase 64`); }); @@ -328,18 +328,31 @@ describe('#3164 — validate consistency: milestone-archive layout', () => { const out = JSON.parse(result.output); const warnings = out.warnings || []; - const warningsPosix = warnings.map(w => toPosixPath(w)); + const warningsPosix = warnings.map(w => toPosixPath(w.message)); - const phase64Warnings = warnings.filter(w => w.includes('Phase 64 exists on disk but not in ROADMAP.md')); + const phase64Warnings = warningsPosix.filter(w => w.includes('Phase 64 exists on disk but not in ROADMAP.md')); assert.deepStrictEqual(phase64Warnings, [], 'Old archived milestone phase 64 should not be treated as active'); + // Phase 12 (#3310) migration note: `validate consistency`'s C002 + // (plan-numbering-gap)/C004 (missing-wave) rules now read + // `PlanningSnapshot`'s `perPhasePlanNumbering`/`perPhaseWaveMissingPlans` + // fields, which enumerate ONLY the flat `.planning/phases/` root — a + // disclosed, accepted scope reduction from the pre-migration inline scan + // (which also walked the active milestone-archive phase root via + // `collectPhaseRoots`). See `src/health-diagnostic-rules/consistency.cts`'s + // fidelity-note comment on `checkC002` and + // `src/planning-snapshot.cts`'s `buildPerPhasePlanScanFields` (called with + // `paths.phases` only). With `.planning/phases/` removed by this fixture + // (milestone-archive-only layout), NEITHER C002 nor C004 can fire for the + // active-archive phase `65-current` anymore — this locks that known, + // disclosed gap rather than asserting behavior the migration no longer + // provides. assert.ok( - warningsPosix.some(w => /Gap in plan numbering in .*milestones\/v1\.7-phases\/65-current/.test(w)), - `Expected plan numbering warning, got: ${warnings.join(', ')}`, + !warningsPosix.some(w => /Gap in plan numbering in .*milestones\/v1\.7-phases\/65-current/.test(w)), + `plan-numbering gap is out of scope for milestone-archive phases post-migration; got: ${JSON.stringify(warningsPosix)}`, ); assert.ok( - warningsPosix.some(w => /milestones\/v1\.7-phases\/65-current\/65-01-PLAN\.md: missing 'wave'/.test(w)) - || warningsPosix.some(w => /milestones\/v1\.7-phases\/65-current\/65-03-PLAN\.md: missing 'wave'/.test(w)), - `Expected frontmatter warning from active archive plans`, + !warningsPosix.some(w => /milestones\/v1\.7-phases\/65-current\/65-0[13]-PLAN\.md: missing 'wave'/.test(w)), + `missing-wave is out of scope for milestone-archive phases post-migration; got: ${JSON.stringify(warningsPosix)}`, ); }); }); diff --git a/tests/planning-snapshot.test.cjs b/tests/planning-snapshot.test.cjs index 50525f1b3..74bddc183 100644 --- a/tests/planning-snapshot.test.cjs +++ b/tests/planning-snapshot.test.cjs @@ -102,8 +102,12 @@ function makeDirUnreadableAsFile(fullPath) { // A matched plan/summary pair plus a passing `*-VERIFICATION.md` — // `isPhaseComplete` requires `verification.status === 'passed'` for // `complete: true`, which plan/summary pairing alone does not establish. +// The plan carries `wave: 1` frontmatter so this fixture is also +// wave-complete — callers that assert `perPhaseWaveMissingPlans` is empty +// on a "healthy" phase (Phase 12, #3310) get a genuinely clean baseline +// rather than a false positive from a plan that predates the `wave:` field. function makeCompletePhaseDir(cwd, relPhaseDir) { - writeFile(cwd, `${relPhaseDir}/01-01-PLAN.md`, '# Plan\n'); + writeFile(cwd, `${relPhaseDir}/01-01-PLAN.md`, '---\nwave: 1\n---\n\n# Plan\n'); writeFile(cwd, `${relPhaseDir}/01-01-SUMMARY.md`, '# Summary\n'); writeFile(cwd, `${relPhaseDir}/01-VERIFICATION.md`, '---\nstatus: passed\n---\n'); } @@ -1090,3 +1094,180 @@ describe('allPhaseDirNames field (Phase 11, #3309 — health-diagnostic-rules/ro assert.deepStrictEqual(snap.allPhaseDirNames, { value: [], scope: SCOPE.UNREADABLE }); }); }); + +// ═════════════════════════════════════════════════════════════════════════ +// Phase 12 (#3310, ADR-3180 §8.4) additions — perPhasePlanNumbering / +// perPhaseOrphanSummaries / perPhaseWaveMissingPlans +// +// Design: .gsd/phase/feat-3310-enhance-3180-the-sibling-validators-shar/40-design.md +// ("New PlanningSnapshot fields") +// Test matrix: .gsd/phase/feat-3310-enhance-3180-the-sibling-validators-shar/50-test-matrix.md +// section 1, rows 1-8 +// +// Each relocates (not reinvents) `verify.cts:1556-1603`'s per-phase-directory +// plan scan — see `buildPerPhasePlanScanFields`'s doc comment in +// src/planning-snapshot.cts for the exact source lines. Fixture helpers +// mirror the existing writeRoadmap/writeState/writeFile idiom. +// ═════════════════════════════════════════════════════════════════════════ + +function writePlan(cwd, relPhasePath, planName, frontmatterLines) { + const lines = frontmatterLines ? ['---', ...frontmatterLines, '---', '', '# Plan', ''] : ['# Plan', '']; + writeFile(cwd, `${relPhasePath}/${planName}`, lines.join('\n')); +} + +describe('perPhasePlanNumbering field (Phase 12, #3310, matrix rows 1-2)', () => { + test('row 1: sequential plans (01, 02, 03) report the full sorted sequence, no gap', (t) => { + const cwd = createTempDir('gsd-3310-ppn1-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + writePlan(cwd, '.planning/phases/01-foo', '01-02-PLAN.md'); + writePlan(cwd, '.planning/phases/01-foo', '01-03-PLAN.md'); + + const snap = buildPlanningSnapshot(cwd); + const entry = snap.perPhasePlanNumbering.value.find((e) => e.phaseDir === '01-foo'); + assert.deepStrictEqual(entry, { phaseDir: '01-foo', planNums: [1, 2, 3] }); + assert.strictEqual(snap.perPhasePlanNumbering.scope, SCOPE.COMPLETE); + }); + + test('row 2: a real gap (01, 03) is surfaced in the raw per-phase number list', (t) => { + const cwd = createTempDir('gsd-3310-ppn2-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + writePlan(cwd, '.planning/phases/01-foo', '01-03-PLAN.md'); + + const snap = buildPlanningSnapshot(cwd); + const entry = snap.perPhasePlanNumbering.value.find((e) => e.phaseDir === '01-foo'); + assert.deepStrictEqual(entry, { phaseDir: '01-foo', planNums: [1, 3] }); + }); + + test('boundary: zero phase directories yields an empty array, not a non-answer', (t) => { + const cwd = createTempDir('gsd-3310-ppn3-'); + t.after(() => cleanup(cwd)); + fs.mkdirSync(path.join(planningDirOf(cwd), 'phases'), { recursive: true }); + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual(snap.perPhasePlanNumbering, { value: [], scope: SCOPE.COMPLETE }); + }); + + test('boundary: a phase with zero plans reports an empty planNums list for that phase, not an absent entry', (t) => { + const cwd = createTempDir('gsd-3310-ppn4-'); + t.after(() => cleanup(cwd)); + fs.mkdirSync(path.join(planningDirOf(cwd), 'phases', '01-foo'), { recursive: true }); + + const snap = buildPlanningSnapshot(cwd); + const entry = snap.perPhasePlanNumbering.value.find((e) => e.phaseDir === '01-foo'); + assert.deepStrictEqual(entry, { phaseDir: '01-foo', planNums: [] }); + }); +}); + +describe('perPhaseOrphanSummaries field (Phase 12, #3310, matrix rows 3-5)', () => { + test('row 3: a paired plan+summary produces no orphan entries', (t) => { + const cwd = createTempDir('gsd-3310-pos1-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + writeFile(cwd, '.planning/phases/01-foo/01-01-SUMMARY.md', '# Summary\n'); + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual( + snap.perPhaseOrphanSummaries.value.filter((e) => e.phaseDir === '01-foo'), + [], + ); + }); + + test('row 4: a summary with no live plan at all is named as an orphan', (t) => { + const cwd = createTempDir('gsd-3310-pos2-'); + t.after(() => cleanup(cwd)); + writeFile(cwd, '.planning/phases/01-foo/01-01-SUMMARY.md', '# Summary\n'); + + const snap = buildPlanningSnapshot(cwd); + const entries = snap.perPhaseOrphanSummaries.value.filter((e) => e.phaseDir === '01-foo'); + assert.deepStrictEqual(entries, [{ phaseDir: '01-foo', orphanSummary: '01-01-SUMMARY.md' }]); + }); + + test('row 5: a summary paired only to a superseded plan is still orphan — superseded plans are not live', (t) => { + const cwd = createTempDir('gsd-3310-pos3-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md', ['status: superseded']); + writeFile(cwd, '.planning/phases/01-foo/01-01-SUMMARY.md', '# Summary\n'); + + const snap = buildPlanningSnapshot(cwd); + const entries = snap.perPhaseOrphanSummaries.value.filter((e) => e.phaseDir === '01-foo'); + assert.deepStrictEqual(entries, [{ phaseDir: '01-foo', orphanSummary: '01-01-SUMMARY.md' }]); + }); +}); + +describe('perPhaseWaveMissingPlans field (Phase 12, #3310, matrix rows 6-7)', () => { + test('row 6: a plan with wave: in frontmatter is not flagged', (t) => { + const cwd = createTempDir('gsd-3310-pwm1-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md', ['wave: 1']); + + const snap = buildPlanningSnapshot(cwd); + assert.deepStrictEqual( + snap.perPhaseWaveMissingPlans.value.filter((e) => e.phaseDir === '01-foo'), + [], + ); + }); + + test('row 7: a plan without wave: in frontmatter is flagged', (t) => { + const cwd = createTempDir('gsd-3310-pwm2-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + + const snap = buildPlanningSnapshot(cwd); + const entries = snap.perPhaseWaveMissingPlans.value.filter((e) => e.phaseDir === '01-foo'); + assert.deepStrictEqual(entries, [{ phaseDir: '01-foo', plan: '01-01-PLAN.md' }]); + }); + + test('boundary: a phase where every plan lacks wave: reports every one, none silently dropped', (t) => { + const cwd = createTempDir('gsd-3310-pwm3-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md'); + writePlan(cwd, '.planning/phases/01-foo', '01-02-PLAN.md'); + + const snap = buildPlanningSnapshot(cwd); + const entries = snap.perPhaseWaveMissingPlans.value.filter((e) => e.phaseDir === '01-foo'); + assert.deepStrictEqual( + entries.map((e) => e.plan).sort(), + ['01-01-PLAN.md', '01-02-PLAN.md'], + ); + }); + + test('boundary: a phase where every plan carries wave: reports none', (t) => { + const cwd = createTempDir('gsd-3310-pwm4-'); + t.after(() => cleanup(cwd)); + writePlan(cwd, '.planning/phases/01-foo', '01-01-PLAN.md', ['wave: 1']); + writePlan(cwd, '.planning/phases/01-foo', '01-02-PLAN.md', ['wave: 2']); + + const snap = buildPlanningSnapshot(cwd); + const entries = snap.perPhaseWaveMissingPlans.value.filter((e) => e.phaseDir === '01-foo'); + assert.deepStrictEqual(entries, []); + }); +}); + +describe('Phase-10/11 fields unchanged by the Phase-12 extension (matrix row 8)', () => { + test('the new fields are additive alongside every prior field on the same fixture', (t) => { + const cwd = createTempDir('gsd-3310-reg8-'); + t.after(() => cleanup(cwd)); + buildHealthyTwoPhaseFixture(cwd); + + const snap = buildPlanningSnapshot(cwd); + + assert.strictEqual(snap.milestone.scope, SCOPE.COMPLETE); + assert.strictEqual(snap.phases.value.length, 2); + assert.ok('config' in snap); + assert.ok('agentInstall' in snap); + assert.ok('worktreeHealth' in snap); + // The extension is additive — the three new Phase 12 fields sit + // alongside every prior field, not in place of them. + assert.ok('perPhasePlanNumbering' in snap); + assert.ok('perPhaseOrphanSummaries' in snap); + assert.ok('perPhaseWaveMissingPlans' in snap); + assert.strictEqual(snap.perPhasePlanNumbering.value.length, 2); + for (const entry of snap.perPhasePlanNumbering.value) { + assert.deepStrictEqual(entry.planNums, [1]); + } + assert.deepStrictEqual(snap.perPhaseOrphanSummaries.value, []); + assert.deepStrictEqual(snap.perPhaseWaveMissingPlans.value, []); + }); +}); diff --git a/tests/state.test.cjs b/tests/state.test.cjs index aeee20566..de0fd474f 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -22,6 +22,19 @@ const stateDocument = require('../gsd-core/bin/lib/state-document.cjs'); const frontmatterLib = require('../gsd-core/bin/lib/frontmatter.cjs'); const { SCOPE } = require('../gsd-core/bin/lib/planning-scope.cjs'); const workstreamInventory = require('../gsd-core/bin/lib/workstream-inventory.cjs'); +// Phase 12 (#3310, ADR-3180 §8.4 rule 3): `cmdStateValidate`'s `warnings` are +// now `Diagnostic[]` (S0NN codes), not bare strings, and `drift` is gone. +const { SEVERITY } = require('../gsd-core/bin/lib/health-diagnostic-types.cjs'); + +/** First `Diagnostic` in `output.warnings` carrying the given S0NN code, or `undefined`. */ +function findWarning(output, code) { + return (output.warnings || []).find((w) => w.code === code); +} + +/** Phase 12 breaking-change proof (§8.4 rule 3, test matrix row 18): `drift` never appears. */ +function assertNoDriftKey(output) { + assert.ok(!Object.prototype.hasOwnProperty.call(output, 'drift'), 'output must not contain a drift key'); +} function writePassedVerification(tmpDir, phaseDirName, paddedPhase) { fs.writeFileSync( @@ -3296,11 +3309,10 @@ describe('state validate command', () => { const output = JSON.parse(result.output); assert.strictEqual(output.valid, false, 'passed verification must invalidate executing state'); assert.ok(output.warnings.length > 0, 'passed verification drift must emit a warning'); - assert.deepStrictEqual( - output.drift.verification_status, - { state_status: 'executing', verification: 'passed' }, - 'template frontmatter phase must reach the existing disk-backed verification drift check', - ); + const s006 = findWarning(output, 'S006'); + assert.ok(s006, 'S006 must fire for passed verification against executing status'); + assert.strictEqual(s006.severity, SEVERITY.WARNING); + assertNoDriftKey(output); }); test('template-equivalent phase identities remain clean without disk drift', () => { @@ -3320,7 +3332,7 @@ describe('state validate command', () => { const output = JSON.parse(result.output); assert.strictEqual(output.valid, true, 'equivalent phase identities without disk drift must stay valid'); assert.deepStrictEqual(output.warnings, [], 'clean control must not emit warnings'); - assert.deepStrictEqual(output.drift, {}, 'clean control must not report drift'); + assertNoDriftKey(output); }); test('legacy Current Phase fallback reaches passed-verification drift on disk', () => { @@ -3341,10 +3353,9 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.valid, false, 'legacy phase fallback must expose verification drift'); - assert.deepStrictEqual( - output.drift.verification_status, - { state_status: 'Executing Phase 2', verification: 'passed' }, - ); + const s006 = findWarning(output, 'S006'); + assert.ok(s006, 'S006 must fire for legacy Current Phase fallback'); + assertNoDriftKey(output); }); test('Current Position Phase fallback reaches passed-verification drift on disk', () => { @@ -3367,10 +3378,9 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.valid, false, 'canonical phase fallback must expose verification drift'); - assert.deepStrictEqual( - output.drift.verification_status, - { state_status: 'Executing Phase 2', verification: 'passed' }, - ); + const s006 = findWarning(output, 'S006'); + assert.ok(s006, 'S006 must fire for Current Position Phase fallback'); + assertNoDriftKey(output); }); test('frontmatter phase wins conflicts and scans its selected directory', () => { @@ -3399,15 +3409,14 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.valid, false, 'conflicting sources must invalidate the result'); - assert.strictEqual(output.drift.phase_reference.reason, 'conflict'); - assert.strictEqual(output.drift.phase_reference.selected, '2'); - assert.strictEqual(output.drift.phase_reference.sources.frontmatter, '2'); - assert.strictEqual(output.drift.phase_reference.sources.current_position_phase, '1'); - assert.deepStrictEqual( - output.drift.verification_status, - { state_status: 'executing', verification: 'passed' }, - 'disk evidence must come from the authoritative frontmatter phase', - ); + const s003 = findWarning(output, 'S003'); + assert.ok(s003, 'S003 must fire for conflicting phase sources'); + // S003 names only the selected (authoritative) phase, not the individual + // disagreeing sources; the old `drift.phase_reference.sources` structured + // detail has no S0NN equivalent (disclosed breaking change, §8.4 rule 3). + const s006 = findWarning(output, 'S006'); + assert.ok(s006, 'disk evidence must come from the authoritative frontmatter phase'); + assertNoDriftKey(output); }); test('missing phase sources fail closed with phase-reference drift', () => { @@ -3420,9 +3429,9 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.valid, false, 'missing phase source must not validate cleanly'); - assert.strictEqual(output.drift.phase_reference.reason, 'unresolved'); - assert.strictEqual(output.drift.phase_reference.selected, null); - assert.ok(output.warnings.some(warning => /phase/i.test(warning)), 'warning must identify phase resolution'); + const s002 = findWarning(output, 'S002'); + assert.ok(s002, 'S002 must fire when no phase source resolves'); + assertNoDriftKey(output); }); test('non-scalar frontmatter phase fails closed without a body fallback', () => { @@ -3444,8 +3453,9 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.valid, false, 'non-scalar phase source must not validate cleanly'); - assert.strictEqual(output.drift.phase_reference.reason, 'unresolved'); - assert.strictEqual(output.drift.phase_reference.sources.frontmatter, null); + const s002 = findWarning(output, 'S002'); + assert.ok(s002, 'S002 must fire when the frontmatter phase source is non-scalar'); + assertNoDriftKey(output); }); test('missing phases root fails closed with phase-directory drift', () => { @@ -3460,8 +3470,9 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.valid, false, 'missing phases root must not validate cleanly'); - assert.strictEqual(output.drift.phase_directory.reason, 'missing_root'); - assert.ok(output.warnings.some(warning => /director/i.test(warning)), 'warning must identify the missing directory'); + const s004 = findWarning(output, 'S004'); + assert.ok(s004, 'S004 must fire when the phases directory is missing'); + assertNoDriftKey(output); }); test('missing canonical phase-directory match fails closed', () => { @@ -3474,8 +3485,9 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.valid, false, 'missing phase-directory match must not validate cleanly'); - assert.strictEqual(output.drift.phase_directory.reason, 'not_found'); - assert.strictEqual(output.drift.phase_directory.selected, '2'); + const s004 = findWarning(output, 'S004'); + assert.ok(s004, 'S004 must fire when no phase directory matches'); + assertNoDriftKey(output); }); test('crafted path-like phase cannot scan verification evidence outside phases root', () => { @@ -3502,8 +3514,10 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.strictEqual(output.valid, false, 'crafted phase reference must fail closed'); - assert.strictEqual(output.drift.phase_reference.reason, 'unresolved'); - assert.ok(!output.drift.verification_status, 'outside-root verification evidence must not be scanned'); + const s002 = findWarning(output, 'S002'); + assert.ok(s002, 'S002 must fire when the crafted phase value fails to resolve'); + assert.ok(!findWarning(output, 'S006'), 'outside-root verification evidence must not be scanned'); + assertNoDriftKey(output); }); test('STATE says executing + VERIFICATION.md shows passed emits warning', () => { @@ -3524,7 +3538,9 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.ok(output.warnings.length > 0, 'Should have warnings when executing but verification passed'); - assert.ok(output.warnings.some(w => /verif/i.test(w)), 'Warning should mention verification'); + const s006 = findWarning(output, 'S006'); + assert.ok(s006, 'S006 must fire when executing but verification passed'); + assertNoDriftKey(output); }); test('STATE plan count 3 but 12 SUMMARY.md on disk emits mismatch warning', () => { @@ -3546,7 +3562,15 @@ describe('state validate command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); assert.ok(output.warnings.length > 0, 'Should have warnings for plan count mismatch'); - assert.ok(output.warnings.some(w => /plan.*count|count.*mismatch/i.test(w)), 'Warning should mention plan count mismatch'); + const s005 = findWarning(output, 'S005'); + assert.ok(s005, 'S005 must fire for a plan count mismatch'); + // The specific counts are the thing under test (STATE.md-authored count + // vs disk-scanned count); `.includes()` on the interpolated values, not + // full-string message equality (CONTRIBUTING.md's "Prohibited: Raw Text + // Matching on Test Outputs"). + assert.ok(s005.message.includes('STATE.md says 3 plans'), 'message must report the STATE.md count'); + assert.ok(s005.message.includes('disk has 12'), 'message must report the disk-scanned count'); + assertNoDriftKey(output); }); test('perfect state returns valid: true, no warnings', () => { @@ -3591,6 +3615,218 @@ describe('state validate command', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// Phase 12 (#3310, ADR-3180 §8.4 rule 3) — S0NN coded-diagnostic fixtures for +// `cmdStateValidate`. `warnings` is now `Diagnostic[]` (not bare strings) and +// `drift` is gone from every output shape. One test per code, title naming +// the code (test-matrix §4 / §8.5's "known-bad fixture proves it can fire" +// discipline extended to the S0NN codes, which are NOT `Rule`-table entries — +// `cmdStateValidate` builds `Diagnostic[]` directly, per the design doc). +// ───────────────────────────────────────────────────────────────────────────── + +describe('#3310 state validate — S0NN coded diagnostics', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createFixture(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('S001: STATE.md corrupt (NUL byte) fires with the verbatim textEncodingError message', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), Buffer.from('# Project State\0corrupt')); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + assert.strictEqual(output.warnings.length, 1); + const [s001] = output.warnings; + assert.strictEqual(s001.code, 'S001'); + assert.strictEqual(s001.severity, SEVERITY.ERROR); + assert.strictEqual(s001.remedy.action, 'advise'); + assertNoDriftKey(output); + }); + + test('S002: no usable current-phase value fires the verbatim pre-migration message', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + ['# Project State', '', '**Status:** Planning', ''].join('\n'), + ); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + const s002 = findWarning(output, 'S002'); + assert.ok(s002); + assert.strictEqual(s002.severity, SEVERITY.WARNING); + assert.strictEqual(s002.remedy.action, 'advise'); + assertNoDriftKey(output); + }); + + test('S003: conflicting phase-reference sources fires the verbatim template with interpolated phase', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + [ + '---', + 'current_phase: 2', + 'status: executing', + '---', + '', + '# Project State', + '', + '**Current Phase:** 1', + '**Status:** Executing Phase 2', + '', + ].join('\n'), + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '02-core'), { recursive: true }); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + const s003 = findWarning(output, 'S003'); + assert.ok(s003); + assert.strictEqual(s003.severity, SEVERITY.WARNING); + assert.strictEqual(s003.remedy.action, 'advise'); + assertNoDriftKey(output); + }); + + test('S004: phases directory question collapses three sub-conditions to one code with distinct messages', () => { + // (a) phases/ root missing entirely. + cleanup(tmpDir); + tmpDir = createFixture({ planning: false, projectDoc: true }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + ['---', 'current_phase: 2', 'status: planning', '---', '', '# Project State', ''].join('\n'), + ); + const missingRootOutput = JSON.parse(runGsdTools('state validate', tmpDir).output); + const missingRootS004 = findWarning(missingRootOutput, 'S004'); + assert.ok(missingRootS004, 'S004 must fire when phases/ root is missing'); + assert.strictEqual(missingRootS004.severity, SEVERITY.WARNING); + assert.strictEqual(missingRootS004.remedy.action, 'advise'); + assertNoDriftKey(missingRootOutput); + + // (b) phases/ exists, no matching subdir for the current phase. + cleanup(tmpDir); + tmpDir = createFixture(); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + ['---', 'current_phase: 2', 'status: planning', '---', '', '# Project State', ''].join('\n'), + ); + const notFoundOutput = JSON.parse(runGsdTools('state validate', tmpDir).output); + const notFoundS004 = findWarning(notFoundOutput, 'S004'); + assert.ok(notFoundS004, 'S004 must fire when no phase directory matches'); + assertNoDriftKey(notFoundOutput); + + // Same code, distinct message text per sub-condition — §8.2 rule 1. This + // is a relative comparison (message A !== message B), not a literal + // string match, so it stays within CONTRIBUTING.md's rule. + assert.strictEqual(missingRootS004.code, notFoundS004.code); + assert.notStrictEqual(missingRootS004.message, notFoundS004.message); + }); + + test('S005: plan-count mismatch fires the verbatim template with both counts interpolated', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 1\n**Current Phase:** 1\n**Total Plans in Phase:** 3\n**Current Plan:** 1\n`, + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-02-PLAN.md'), '# Plan\n'); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + const s005 = findWarning(output, 'S005'); + assert.ok(s005); + assert.strictEqual(s005.severity, SEVERITY.WARNING); + assert.strictEqual(s005.remedy.action, 'advise'); + // The interpolated counts are what this test is proving; `.includes()` + // on the specific values, not full-string message equality + // (CONTRIBUTING.md's "Prohibited: Raw Text Matching on Test Outputs"). + assert.ok(s005.message.includes('STATE.md says 3 plans'), 'message must report the STATE.md count'); + assert.ok(s005.message.includes('disk has 2'), 'message must report the disk-scanned count'); + assertNoDriftKey(output); + }); + + test('S006: verification passed but status still executing fires the verbatim template', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 1\n**Current Phase:** 1\n**Total Plans in Phase:** 1\n**Current Plan:** 1\n`, + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-VERIFICATION.md'), '---\nstatus: passed\n---\n# Verification\n'); + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + const s006 = findWarning(output, 'S006'); + assert.ok(s006); + assert.strictEqual(s006.severity, SEVERITY.WARNING); + assert.strictEqual(s006.remedy.action, 'advise'); + assertNoDriftKey(output); + }); + + test('S007: all plans have summaries but status still executing fires the verbatim template (never a drift entry pre-migration either)', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 1\n**Current Phase:** 1\n**Total Plans in Phase:** 2\n**Current Plan:** 1\n`, + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-02-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '# Summary\n'); + fs.writeFileSync(path.join(phaseDir, '01-02-SUMMARY.md'), '# Summary\n'); + // No VERIFICATION.md — otherwise S006 would cover it instead (see the + // production code's own "Only warn if no verification exists" guard). + + const output = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.strictEqual(output.valid, false); + const s007 = findWarning(output, 'S007'); + assert.ok(s007); + assert.strictEqual(s007.severity, SEVERITY.WARNING); + assert.strictEqual(s007.remedy.action, 'advise'); + assertNoDriftKey(output); + }); + + test('output never carries a drift key, across clean/warning/error shapes (breaking-change proof, test matrix row 18)', () => { + // Clean shape. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 1\n**Current Phase:** 1\n**Total Plans in Phase:** 1\n`, + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-setup'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan\n'); + const clean = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.ok(!('drift' in clean)); + assert.ok(!Object.keys(clean).includes('drift')); + + // Warning shape (S005). + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + `# Project State\n\n**Status:** Executing Phase 1\n**Current Phase:** 1\n**Total Plans in Phase:** 5\n`, + ); + const warned = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.ok(!('drift' in warned)); + assert.ok(!Object.keys(warned).includes('drift')); + + // Error shape (S001, corrupt STATE.md). + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), Buffer.from('# Project State\0bad')); + const corrupt = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.ok(!('drift' in corrupt)); + assert.ok(!Object.keys(corrupt).includes('drift')); + + // `{error: 'STATE.md not found'}` pre-check shape. + cleanup(tmpDir); + tmpDir = createFixture(); + const missing = JSON.parse(runGsdTools('state validate', tmpDir).output); + assert.ok(!('drift' in missing)); + assert.ok(!Object.keys(missing).includes('drift')); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // #3187 (ADR-3180 §7.7) — matrix section B: `state validate`'s scope field, // including the #3162 headline regression and #1255 frontmatter shadowing. @@ -3612,7 +3848,8 @@ describe('#3187 state validate — scope field (matrix section B)', () => { // at all. Pre-#3187, cmdStateValidate read `Current Phase` off the body // only, resolved null, and the ENTIRE drift block was skipped — // "could not look" was output-identical to "looked, all clean" - // ({valid:true, warnings:[], drift:{}}). + // (originally {valid:true, warnings:[], drift:{}}; post-#3310 the + // equivalent clean shape is {valid:true, warnings:[]}, `drift` removed). fs.writeFileSync( path.join(tmpDir, '.planning', 'STATE.md'), [ @@ -3639,9 +3876,12 @@ describe('#3187 state validate — scope field (matrix section B)', () => { // The RIGHT reason to fail before the fix: valid was true and no // plan-count warning was ever generated, not a crash. assert.strictEqual(output.valid, false, 'STATE.md says 5 plans, disk has 2 — drift must be reported'); - assert.ok(output.warnings.some((w) => /plan.*count|count.*mismatch/i.test(w))); - assert.deepEqual(output.drift.plan_count, { state: 5, disk: 2 }); + const s005 = findWarning(output, 'S005'); + assert.ok(s005, 'S005 must fire for the frontmatter-only phase drift'); + assert.ok(s005.message.includes('STATE.md says 5 plans'), 'message must report the STATE.md count'); + assert.ok(s005.message.includes('disk has 2'), 'message must report the disk-scanned count'); assert.strictEqual(output.scope, SCOPE.COMPLETE); + assertNoDriftKey(output); }); test('B2: validate passes cleanly when it actually looked (frontmatter-only phase, counts match)', () => { @@ -3670,6 +3910,7 @@ describe('#3187 state validate — scope field (matrix section B)', () => { assert.strictEqual(output.valid, true); assert.strictEqual(output.warnings.length, 0); assert.strictEqual(output.scope, SCOPE.COMPLETE); + assertNoDriftKey(output); }); test('B3: validate still detects body-resolved drift (regression guard, unchanged today)', () => { @@ -3691,8 +3932,12 @@ describe('#3187 state validate — scope field (matrix section B)', () => { const output = JSON.parse(runGsdTools('state validate', tmpDir).output); assert.strictEqual(output.valid, false); - assert.ok(output.warnings.some((w) => /plan.*count|count.*mismatch/i.test(w))); + const s005 = findWarning(output, 'S005'); + assert.ok(s005, 'S005 must fire for the body-resolved plan-count drift'); + assert.ok(s005.message.includes('STATE.md says 3 plans'), 'message must report the STATE.md count'); + assert.ok(s005.message.includes('disk has 1'), 'message must report the disk-scanned count'); assert.strictEqual(output.scope, SCOPE.COMPLETE); + assertNoDriftKey(output); }); test('B4: unresolvable phase is not reported as clean (distinguishable from B2)', () => { @@ -3704,7 +3949,9 @@ describe('#3187 state validate — scope field (matrix section B)', () => { const output = JSON.parse(runGsdTools('state validate', tmpDir).output); assert.strictEqual(output.valid, false); - assert.strictEqual(output.drift.phase_reference.reason, 'unresolved'); + const s002 = findWarning(output, 'S002'); + assert.ok(s002, 'S002 must fire when the phase is unresolvable'); + assertNoDriftKey(output); }); test('B5: missing phase dir differs from could-not-look (distinguishable from B4)', () => { @@ -3723,7 +3970,9 @@ describe('#3187 state validate — scope field (matrix section B)', () => { const output = JSON.parse(runGsdTools('state validate', tmpDir).output); assert.strictEqual(output.valid, false); - assert.strictEqual(output.drift.phase_directory.reason, 'not_found'); + const s004 = findWarning(output, 'S004'); + assert.ok(s004, 'S004 must fire when no phase directory matches'); + assertNoDriftKey(output); }); test('B6: unreadable phases dir is surfaced, not swallowed', (t) => { @@ -3757,7 +4006,9 @@ describe('#3187 state validate — scope field (matrix section B)', () => { const raw = captureStdout(() => stateLib.cmdStateValidate(tmpDir, false)); const output = JSON.parse(raw); assert.strictEqual(output.valid, false, 'an unreadable directory must not validate cleanly'); - assert.strictEqual(output.drift.phase_directory.reason, 'unreadable'); + const s004 = findWarning(output, 'S004'); + assert.ok(s004, 'S004 must fire when the phases directory itself is unreadable'); + assertNoDriftKey(output); }); test('B6b: unreadable selected phase directory fails closed', (t) => { @@ -3777,7 +4028,9 @@ describe('#3187 state validate — scope field (matrix section B)', () => { const output = JSON.parse(captureStdout(() => stateLib.cmdStateValidate(tmpDir, false))); assert.strictEqual(output.valid, false, 'an unreadable selected phase must not validate cleanly'); - assert.strictEqual(output.drift.phase_directory.reason, 'unreadable'); + const s004 = findWarning(output, 'S004'); + assert.ok(s004, 'S004 must fire when the selected phase directory itself is unreadable'); + assertNoDriftKey(output); }); test('B7: one unreadable verification file does not abort the scan', (t) => { @@ -3814,13 +4067,16 @@ describe('#3187 state validate — scope field (matrix section B)', () => { const raw = captureStdout(() => stateLib.cmdStateValidate(tmpDir, false)); const output = JSON.parse(raw); // Per-file swallow (#2245 audit) is unchanged: the other verification - // file is still consulted, so its drift still fires, and the whole - // scan is NOT degraded to UNREADABLE just because one file 404s. - // Asserted on the structured `drift` field (CONTRIBUTING.md's "Prohibited: - // Raw Text Matching on Test Outputs"), not a `warnings` prose regex — - // `drift.verification_status` is a typed record, not free-form text. - assert.deepStrictEqual(output.drift.verification_status, { state_status: 'Executing Phase 1', verification: 'passed' }); + // file is still consulted, so its S006 diagnostic still fires, and the + // whole scan is NOT degraded to UNREADABLE just because one file 404s. + // Asserted on the S006 `Diagnostic`'s `code` alone, not its `message` + // prose (CONTRIBUTING.md's "Prohibited: Raw Text Matching on Test + // Outputs") — the vf filename isn't pinned since readdirSync order + // across the two verification files isn't guaranteed. + const s006 = findWarning(output, 'S006'); + assert.ok(s006, 'S006 must fire for the readable verification file despite the unreadable sibling'); assert.strictEqual(output.scope, SCOPE.COMPLETE); + assertNoDriftKey(output); }); test('B8: absent STATE.md unchanged', () => { @@ -3834,7 +4090,11 @@ describe('#3187 state validate — scope field (matrix section B)', () => { const output = JSON.parse(runGsdTools('state validate', tmpDir).output); assert.strictEqual(output.valid, false); assert.strictEqual(output.warnings.length, 1); + const [s001] = output.warnings; + assert.strictEqual(s001.code, 'S001'); + assert.strictEqual(s001.severity, SEVERITY.ERROR, 'S001 is error-class severity, not a mere warning'); assert.strictEqual(output.scope, undefined, 'the #2701 early return is explicitly unchanged — no scope key'); + assertNoDriftKey(output); }); test('B10: json and default output agree (validate has no distinct raw-text mode; --raw is a no-op for it)', () => { @@ -3904,10 +4164,15 @@ describe('#3187 state validate — scope field (matrix section B)', () => { } const output = JSON.parse(runGsdTools('state validate', dir).output); assert.strictEqual(output.valid, false, `n=${n} vs disk n+1 must be flagged`); - // Asserted on the structured `drift.plan_count` record rather than a - // `warnings` prose regex (CONTRIBUTING.md's "Prohibited: Raw Text - // Matching on Test Outputs") — `drift` is typed, `warnings` is free-form. - assert.deepStrictEqual(output.drift.plan_count, { state: n, disk: n + 1 }); + // The boundary values (n, n+1) are what this loop proves flow through + // correctly; `.includes()` on the interpolated counts, not full-string + // message equality (CONTRIBUTING.md's "Prohibited: Raw Text Matching + // on Test Outputs") — `code` establishes S005 fired. + const s005 = findWarning(output, 'S005'); + assert.ok(s005, `n=${n}: S005 must fire for the plan-count mismatch`); + assert.ok(s005.message.includes(`STATE.md says ${n} plans`), `n=${n}: message must report the STATE.md count`); + assert.ok(s005.message.includes(`disk has ${n + 1}`), `n=${n}: message must report the disk-scanned count`); + assertNoDriftKey(output); cleanup(dir); } }); @@ -4039,9 +4304,10 @@ describe('#3187 chain-owner identity — every consumer agrees with stateFieldVa const output = JSON.parse(runGsdTools('state validate', tmpDir).output); assert.strictEqual(output.valid, false, 'conflicting phase sources must not validate cleanly'); - assert.strictEqual(output.drift.phase_reference.reason, 'conflict'); - assert.strictEqual(output.drift.phase_reference.selected, '2'); - assert.ok(!output.drift.plan_count, 'validate must scan phase 2, not the shadowed phase 1'); + const s003 = findWarning(output, 'S003'); + assert.ok(s003, 'S003 must fire for conflicting phase sources'); + assert.ok(!findWarning(output, 'S005'), 'validate must scan phase 2, not the shadowed phase 1 (no plan-count drift)'); + assertNoDriftKey(output); }); test('C3: prune resolves the same phase as the owner', () => { @@ -10637,7 +10903,8 @@ describe('cmdStateValidate nested plans/ layout (#3257)', () => { const parsed = JSON.parse(result.output); assert.ok(parsed.valid, `state validate should be valid; warnings: ${JSON.stringify(parsed.warnings)}`); assert.deepStrictEqual(parsed.warnings, [], 'no drift warnings for nested-layout phase with correct plan count'); - assert.ok(!parsed.drift.plan_count, 'no plan_count drift when nested scan matches STATE.md'); + assert.ok(!findWarning(parsed, 'S005'), 'no S005 (plan-count drift) when nested scan matches STATE.md'); + assertNoDriftKey(parsed); }); test('emits drift warning when STATE.md plan count does not match nested disk count', () => { @@ -10667,9 +10934,15 @@ describe('cmdStateValidate nested plans/ layout (#3257)', () => { const parsed = JSON.parse(result.output); assert.ok(!parsed.valid, 'state validate should report invalid when plan counts differ'); assert.ok(parsed.warnings.length > 0, 'at least one drift warning expected'); - assert.ok(parsed.drift.plan_count, 'plan_count drift object must be present'); - assert.strictEqual(parsed.drift.plan_count.disk, 2, 'disk count must reflect nested scan (2 nested plans)'); - assert.strictEqual(parsed.drift.plan_count.state, 5, 'state count from STATE.md must be 5'); + const s005 = findWarning(parsed, 'S005'); + assert.ok(s005, 'S005 diagnostic must be present'); + // `.includes()` on the interpolated counts (not full-string message + // equality — CONTRIBUTING.md's "Prohibited: Raw Text Matching on Test + // Outputs"): the disk count must reflect the nested scan (2 nested + // plans), not the pre-fix flat-scan under-count. + assert.ok(s005.message.includes('STATE.md says 5 plans'), 'message must report the STATE.md count'); + assert.ok(s005.message.includes('disk has 2'), 'disk count must reflect nested scan (2 nested plans)'); + assertNoDriftKey(parsed); }); test('PLAN-OUTLINE.md excluded from nested count in validate', () => { @@ -10697,7 +10970,8 @@ describe('cmdStateValidate nested plans/ layout (#3257)', () => { const parsed = JSON.parse(result.output); assert.ok(parsed.valid, `should be valid (outline excluded); warnings: ${JSON.stringify(parsed.warnings)}`); - assert.ok(!parsed.drift.plan_count, 'no plan_count drift when outline excluded from nested count'); + assert.ok(!findWarning(parsed, 'S005'), 'no S005 (plan-count drift) when outline excluded from nested count'); + assertNoDriftKey(parsed); }); }); @@ -10994,12 +11268,21 @@ describe('flat "## Phase Details" milestone leak (#501)', () => { const result = runGsdTools(['validate', 'consistency'], tmpDir); const payload = JSON.parse(result.output); const warnings = payload.warnings || []; - const orphanWarnings = warnings.filter((w) => /exists on disk but not in ROADMAP/i.test(w)); + const orphanWarnings = warnings.filter((w) => /exists on disk but not in ROADMAP/i.test(w.message)); assert.deepEqual( orphanWarnings, [], `shipped phase dirs (1-3) are in the full ROADMAP and must not be flagged as orphans. Got: ${JSON.stringify(orphanWarnings)}` ); + // W007 is REUSED verbatim from `validate.health`'s rule table (design doc, + // "Which rules run where") — `validate consistency`'s own findings carry + // the SAME code space for this subject, not a re-derived private label. + const w007Orphans = warnings.filter((w) => w.code === 'W007'); + assert.deepEqual( + w007Orphans, + [], + `shipped phase dirs (1-3) must not produce W007 via validate consistency either. Got: ${JSON.stringify(w007Orphans)}` + ); }); test('validate health (W007) does not flag shipped phase dirs as not-in-ROADMAP', () => { diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index b1a9b9583..0073f3444 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -97,9 +97,15 @@ describe('validate consistency command', () => { const output = JSON.parse(result.output); assert.ok(output.warning_count > 0, 'should have warnings'); assert.ok( - output.warnings.some(w => w.includes('disk but not in ROADMAP')), + output.warnings.some(w => w.message.includes('disk but not in ROADMAP')), 'should warn about orphan directory' ); + // W007 is REUSED verbatim from `validate.health`'s rule table (design doc, + // "Which rules run where") — the code is proof of reuse, not a mistake. + assert.ok( + output.warnings.some(w => w.code === 'W007'), + `expected code W007 for the orphan-on-disk warning; got: ${JSON.stringify(output.warnings)}` + ); }); test('#3225: sentinel phase dirs (999/0) do not warn; real orphans still do', () => { @@ -119,7 +125,7 @@ describe('validate consistency command', () => { const output = JSON.parse(result.output); const sentinelWarnings = output.warnings.filter( - w => w.includes('disk but not in ROADMAP') && /\b(0|999)\b/.test(w) + w => w.message.includes('disk but not in ROADMAP') && /\b(0|999)\b/.test(w.message) ); assert.strictEqual( sentinelWarnings.length, 0, @@ -127,14 +133,14 @@ describe('validate consistency command', () => { ); // Negative space: the real orphan must still warn. assert.ok( - output.warnings.some(w => w.includes('disk but not in ROADMAP') && /02\b/.test(w)), + output.warnings.some(w => w.message.includes('disk but not in ROADMAP') && /02\b/.test(w.message)), `expected a warning for the real orphan 02; got: ${JSON.stringify(output.warnings)}` ); // #3225 (review finding): a sentinel dir must NOT produce a spurious // "Gap in phase numbering: N → 999" either (the gap check builds its integer // sequence from diskPhases and would otherwise include 999). const sentinelGaps = output.warnings.filter( - w => w.includes('Gap in phase numbering') && /999\b/.test(w) + w => w.message.includes('Gap in phase numbering') && /999\b/.test(w.message) ); assert.strictEqual( sentinelGaps.length, 0, @@ -155,9 +161,15 @@ describe('validate consistency command', () => { const output = JSON.parse(result.output); assert.ok( - output.warnings.some(w => w.includes('Gap in phase numbering')), + output.warnings.some(w => w.message.includes('Gap in phase numbering')), 'should warn about gap' ); + // C001 — new code namespace (design doc, "Code namespace"): not a W0NN, + // since this subject has no `validate.health` equivalent. + assert.ok( + output.warnings.some(w => w.code === 'C001'), + `expected code C001 for the phase-numbering gap; got: ${JSON.stringify(output.warnings)}` + ); }); }); @@ -3298,7 +3310,18 @@ describe('#2701: state validate rejects NUL-corrupted STATE.md', () => { const out = parseResult(t, ['state', 'validate'], tmpDir); assert.strictEqual(out.valid, false, `expected valid:false; got ${JSON.stringify(out)}`); - assert.ok(out.warnings.some((w) => /NUL/i.test(w)), `warning must name NUL: ${JSON.stringify(out.warnings)}`); + // `state validate` (Phase 12 migration) emits coded warning objects + // ({code, severity, message, remedy}), not bare strings — assert on the + // code as the primary check, with a message substring as a secondary, + // human-readable confirmation. + assert.ok( + out.warnings.some((w) => w.code === 'S001'), + `warning must carry code S001: ${JSON.stringify(out.warnings)}`, + ); + assert.ok( + out.warnings.some((w) => /NUL/i.test(w.message)), + `warning must name NUL: ${JSON.stringify(out.warnings)}`, + ); }); });