From d1760e3c31623d8c221169c95518f36fbb567155 Mon Sep 17 00:00:00 2001 From: sim Date: Thu, 13 Aug 2026 02:28:49 -0400 Subject: [PATCH] refactor(#3309): migrate cmdValidateHealth onto the rule table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces cmdValidateHealth's hand-rolled addIssue/switch accumulation (961 lines) with buildPlanningSnapshot -> evaluateRules -> map to the legacy {code, message, fix, repairable} shape, bucketed by severity. Two pre-checks (home-dir E010/I010, .planning/-root-missing E001) stay outside the rule table entirely, per ADR-3180 §8.2 rule 4 ("no precedence system") — building "some rules suppress others" into the table would itself be the forbidden precedence system. W024 (STATE.md commit-age freshness) also stays outside the table: its committed rule is a documented permanent no-op (readStateHeadFreshness's git-log shell-out is ambient I/O a Rule.check may never perform, and no PlanningSnapshot field carries a commits-behind count). Migrating onto the rule table as designed would have silently regressed 7 passing tests in tests/health-validation.test.cjs — found while wiring this function, kept as a real check in the wrapper instead (same I/O license applyRepairs already relies on), fixed inline per this repo's no-defer policy rather than accepted as a silent loss. Ports the real repair-handler bodies (createConfig/resetConfig, regenerateState, addNyquistKey/addAiIntegrationPhaseKey, backfillMilestones) into health-diagnostic.cts's applyRepairs, replacing the skeleton's stub. DESTRUCTIVE-risk remedies (resetConfig/regenerateState) are refused by --repair — a disclosed breaking change; repairable now means "an automatic repair will actually run," not merely "a remedy exists to describe," so E004/E005 now report repairable:false. --backfill alone now actually triggers backfillMilestones, fixing a latent bug where its gate was unreachable without --repair also being set (verify.cts:2504, confirmed dead code pre-migration). Test updates distinguish the two explicitly-authorized behavior changes (DESTRUCTIVE refusal, backfill-alone fix, W021->W026 split) from preservation — every changed assertion is commented with why, and new regression tests were added for both changes plus W021/W026 mutual independence. Drift-guard bookkeeping (bypass-baseline shrunk to the one disclosed W024 exception, milestone-window and phase-enumeration exemptions, test-file-count allowlist) updated for the relocated/new functions this migration introduces. --- .../planning-snapshot-bypass-baseline.json | 98 -- scripts/lint-milestone-window-drift.cjs | 41 +- scripts/lint-phase-enumeration-drift.cjs | 15 + scripts/lint-test-file-count.allowlist.json | 5 + src/health-diagnostic.cts | 348 ++++- src/verify.cts | 1149 +++-------------- tests/health-diagnostic.test.cjs | 171 ++- tests/phase-resolution-parity.test.cjs | 13 +- tests/roadmap.test.cjs | 23 +- tests/verify-health.test.cjs | 69 +- tests/verify.test.cjs | 70 + 11 files changed, 820 insertions(+), 1182 deletions(-) diff --git a/scripts/baselines/planning-snapshot-bypass-baseline.json b/scripts/baselines/planning-snapshot-bypass-baseline.json index 87eee1c6d..028e09d4f 100644 --- a/scripts/baselines/planning-snapshot-bypass-baseline.json +++ b/scripts/baselines/planning-snapshot-bypass-baseline.json @@ -1,109 +1,11 @@ { "$comment": "ADR-3180 §8.1 rule 2 ratchet, owned by Phase 11 (#3309). See scripts/lint-planning-snapshot-bypass-drift.cjs. SHRINK-ONLY: entries are removed as cmdValidateHealth migrates onto src/planning-snapshot.cts; new or changed entries fail lint:ci. `count` is the number of byte-identical (file, text) occurrences acknowledged at this site — a run producing fewer fails as a partial migration, more fails as an unacknowledged new copy.", "entries": [ - { - "file": "src/verify.cts", - "text": ".readdirSync(phasesDir, { withFileTypes: true })", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "? fs.readFileSync(milestonesPath, 'utf-8')", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 2 - }, - { - "file": "src/verify.cts", - "text": "const archiveFiles = fs.readdirSync(milestonesArchiveDir);", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "const configRaw = fs.readFileSync(configPath, 'utf-8');", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 4 - }, - { - "file": "src/verify.cts", - "text": "const content = fs.readFileSync(projectPath, 'utf-8');", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "const entries = fs.readdirSync(rootBase, { withFileTypes: true });", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "const rawCfg = fs.readFileSync(configPath, 'utf-8');", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "const researchContent = fs.readFileSync(", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8');", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "const roadmapContentFull = fs.readFileSync(roadmapPath, 'utf-8');", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "const roadmapContentRaw = fs.readFileSync(roadmapPath, 'utf-8');", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "const roadmapRaw = fs.readFileSync(roadmapPath, 'utf-8');", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 2 - }, { "file": "src/verify.cts", "text": "const stateContent = fs.readFileSync(statePath, 'utf-8');", "derivation": "planning-snapshot-bypass", "owner_issue": "#3309", - "count": 2 - }, - { - "file": "src/verify.cts", - "text": "const stateRaw = fs.readFileSync(statePath, 'utf-8');", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", - "count": 1 - }, - { - "file": "src/verify.cts", - "text": "phaseDirFiles.set(e.name, fs.readdirSync(path.join(phasesDir, e.name)));", - "derivation": "planning-snapshot-bypass", - "owner_issue": "#3309", "count": 1 } ] diff --git a/scripts/lint-milestone-window-drift.cjs b/scripts/lint-milestone-window-drift.cjs index 7d54c0c0d..a1da11fe6 100644 --- a/scripts/lint-milestone-window-drift.cjs +++ b/scripts/lint-milestone-window-drift.cjs @@ -188,23 +188,13 @@ const OWNER_FILE = path.join('src', 'roadmap-parser.cts'); // question `computeMilestoneSectionEnd` answers — so it cannot diverge // from that computation; it answers a narrower, different question this // derivation does not own. -// - verify.cts checkMilestonePrefixMismatches: `sectionRx` ENUMERATES -// every milestone heading in the document to build a list of -// `{version, start, end}` sections (each section's `end` is provisionally -// "rest of document" until the NEXT heading is found, then backfilled) — -// it is answering "what are ALL the milestone sections", to check every -// phase against its OWN enclosing milestone, not "where does THIS ONE -// milestone (the current/asserted one) end" — `computeMilestoneSectionEnd` -// takes a single heading and returns a single boundary; this function -// never calls anything with that shape. (Design brief named this -// `cmdValidateConsistency` — the code actually lives in the sibling -// function `checkMilestonePrefixMismatches`, called from -// `cmdValidateHealth`; `cmdValidateConsistency` itself does not contain -// `sectionRx`. Exempted here under its ACTUAL containing function.) Also: -// `sectionRx` (`/^#{1,3}\s+(?:\[[^\]]{1,200}\]\s*)?.*v(\d+\.\d+)/gim`) -// does not itself carry token (b) as this guard defines it (no -// `(?!Phase` lookahead, no marker-emoji pairing) — this exemption -// currently documents intent rather than suppressing a live match. +// - (Phase 11, #3309: `verify.cts`'s pre-migration `checkMilestonePrefixMismatches` +// — formerly exempted here — was DELETED when `cmdValidateHealth` migrated +// onto the rule table; its `sectionRx` walk relocated verbatim into +// `planning-snapshot.cts`'s `buildRoadmapDeclaredPhasesField`, which needs +// no exemption of its own: like the deleted function, its `sectionRx` +// never carries token (b) as this guard defines it — no `(?!Phase` +// lookahead, no marker-emoji pairing — so it was never a live match.) // - roadmap-parser.cts isMilestoneShippedInRoadmap: composes the heading // quantifier with the shipped/active MARKER check (via // isClosedMilestoneHeading) to answer "is THIS milestone version marked @@ -235,9 +225,22 @@ const OWNER_FILE = path.join('src', 'roadmap-parser.cts'); // source span. It is a named canonical function defining the grammar, // not a copy of it — replacing the third independent re-derivation the // widened guard found at `roadmap.cts:454`. +// - planning-snapshot.cts buildMilestoneArchiveStatusField (Phase 11, +// #3309): its `## ` heading scan reads `MILESTONES.md` — a +// FLAT version registry, not `ROADMAP.md` — asking "which versions does +// the registry already document", never "where does THIS milestone's +// ROADMAP section begin/end" (`computeMilestoneSectionEnd`/ +// `locateMilestoneHeadings`'s own question). A different document, a +// different question; not a re-derivation of ROADMAP windowing. +// - health-diagnostic.cts computeMissingMilestoneVersions (Phase 11, +// #3309): `applyRepairs` is not a `Rule` and is not handed a +// `PlanningSnapshot` (see that file's header comment), so +// `backfillMilestones` recomputes the IDENTICAL `MILESTONES.md` +// heading-membership check `buildMilestoneArchiveStatusField` already +// performs for the W018 rule's read side — same non-ROADMAP-windowing +// question as that function, for the same reason. const FUNCTION_SCOPED_EXEMPTIONS = new Map([ [path.join('src', 'roadmap-command-router.cts'), new Set(['checkW021'])], - [path.join('src', 'verify.cts'), new Set(['checkMilestonePrefixMismatches'])], [ OWNER_FILE, new Set([ @@ -248,6 +251,8 @@ const FUNCTION_SCOPED_EXEMPTIONS = new Map([ 'extractCurrentMilestoneScoped', ]), ], + [path.join('src', 'planning-snapshot.cts'), new Set(['buildMilestoneArchiveStatusField'])], + [path.join('src', 'health-diagnostic.cts'), new Set(['computeMissingMilestoneVersions'])], ]); // Optional `export ` modifier, mirroring `lint-plan-count-drift.cjs`'s diff --git a/scripts/lint-phase-enumeration-drift.cjs b/scripts/lint-phase-enumeration-drift.cjs index 81ea0bb50..20f617d4e 100644 --- a/scripts/lint-phase-enumeration-drift.cjs +++ b/scripts/lint-phase-enumeration-drift.cjs @@ -195,6 +195,20 @@ * spanning every milestone ever shipped — the union is a strict * superset of any one milestone's window by design; scoping the live * half would silently drop history the digest exists to preserve. + * - `src/planning-snapshot.cts` `buildAllPhaseDirNamesField` (Phase 11, + * #3309): the un-windowed twin of `phaseDirs`/`listMilestonePhaseDirs` — + * every directory actually present under the active `phases/` root, + * UNFILTERED by current-milestone-window membership. Backs the migrated + * `cmdValidateHealth`'s W007 rule ("an on-disk phase directory has no + * matching ROADMAP entry"): sourcing that check from the WINDOWED owner + * would make it structurally unable to fire on the exact orphan + * directory it exists to find (an orphan-by-definition can never be a + * member of a set defined as "directories the roadmap already + * declares") — see that field's own doc comment on `PlanningSnapshot` + * for the full, empirically-verified rationale. Same "must see the + * physical set by definition" shape as `collectDiskPhases`/ + * `cmdValidateHealth` above, generalized from a raw `readdirSync` call + * site to a dedicated snapshot-builder function. * * The tree-walk / root-confinement / regex-literal-tokenizer / sanitizer * machinery is SHARED with the sibling drift guards via @@ -270,6 +284,7 @@ const FUNCTION_SCOPED_EXEMPTIONS = new Map([ [path.join('src', 'roadmap-upgrade.cts'), new Set(['computeMigrationPlan'])], [path.join('src', 'smart-entry.cts'), new Set(['detectVerifyFailed'])], [path.join('src', 'roadmap-parser.cts'), new Set(['getMilestonePhaseFilter'])], + [path.join('src', 'planning-snapshot.cts'), new Set(['buildAllPhaseDirNamesField'])], ]); // Optional `export ` modifier, mirroring the sibling guards' function diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index 91acc2b97..28f55910c 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -7,6 +7,7 @@ "config-field-docs.test.cjs", "config-get-default.test.cjs", "config-schema.property.test.cjs", + "config-validation.test.cjs", "config.test.cjs" ], "issue": "TBD" @@ -33,6 +34,7 @@ }, "milestone": { "files": [ + "milestone-archive-hygiene.test.cjs", "milestone-archive.test.cjs", "milestone-helper.test.cjs", "milestone-prefixed-convention.test.cjs", @@ -48,12 +50,14 @@ "phase-completion-single-owner.test.cjs", "phase-dependency-levels.test.cjs", "phase-resolution-parity.test.cjs", + "phase-structure.test.cjs", "phase.test.cjs" ], "issue": "3186" }, "roadmap": { "files": [ + "roadmap-disk-consistency.test.cjs", "roadmap-mode-field.test.cjs", "roadmap-phase-fallback.test.cjs", "roadmap.test.cjs" @@ -83,6 +87,7 @@ "files": [ "state-acquirestatelock-non-eexist.test.cjs", "state-command-cutover.test.cjs", + "state-consistency.test.cjs", "state-field-drift.test.cjs", "state-prune.test.cjs", "state-rebuild-cli.test.cjs", diff --git a/src/health-diagnostic.cts b/src/health-diagnostic.cts index 430d42b8d..2410426c9 100644 --- a/src/health-diagnostic.cts +++ b/src/health-diagnostic.cts @@ -2,14 +2,24 @@ * Health Diagnostic — frozen rule-table types, enums, and evaluator for * `validate health` (Phase 11, #3309, ADR-3180 §8.2/§8.3/§8.5). * - * SKELETON (this phase). Establishes the exact contract every later batch of - * extracted rules builds onto: the frozen `SEVERITY`/`REMEDY_ACTION`/ - * `REMEDY_RISK` enums, the `Diagnostic`/`Remedy`/`Rule` shapes, the `RULES` - * container (starts EMPTY — a later migration step appends the 32 rules - * extracted from `cmdValidateHealth`, `src/verify.cts:1616-2577`), the - * `evaluateRules` evaluator, and the `applyRepairs` `--repair`/`--backfill` - * dispatcher. `applyRepairs`'s per-action handlers are stubs in this phase — - * they land alongside the rules that need them. + * Establishes the exact contract every extracted rule builds onto: the + * frozen `SEVERITY`/`REMEDY_ACTION`/`REMEDY_RISK` enums, the + * `Diagnostic`/`Remedy`/`Rule` shapes, the `RULES` container (the 32 rules + * extracted from `cmdValidateHealth`, `src/verify.cts:1616-2577`, are + * concatenated in from each rule-group file under + * `src/health-diagnostic-rules/`), the `evaluateRules` evaluator, and the + * `applyRepairs` `--repair`/`--backfill` dispatcher — whose per-action + * handlers are REAL here (ported behavior-preserving from + * `verify.cts:2405-2553`'s repair switch), not stubs. + * + * `applyRepairs` does not receive a `PlanningSnapshot` (its call-site + * signature, `(cwd, diagnostics, repair, backfill)`, is a locked contract — + * see `tests/health-diagnostic.test.cjs`) — so, like `cmdValidateHealth` + * itself before this migration, it performs its own bounded filesystem I/O + * to apply a repair. This is not a §8.1 rule 1 violation: that rule + * constrains a RULE's `check(snapshot)` signature (no ambient I/O), not the + * evaluator/dispatcher, which the design doc's "subject-surface gap" section + * already establishes performs I/O once, up front, on the rules' behalf. * * `PlanningSnapshot` is deliberately NOT re-exported as a type from * `planning-snapshot.cts` here (see the design doc's "Known limits" and this @@ -25,6 +35,9 @@ * gsd-core/bin/lib/health-diagnostic.cjs (gitignored). */ +import fs from 'node:fs'; +import path from 'node:path'; + // 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'); @@ -80,15 +93,36 @@ const RULES: Rule[] = [ ...milestoneArchiveHygieneMod.RULES, ]; +// ─── Repair-handler runtime dependencies ─────────────────────────────────── +// +// Same owners `cmdValidateHealth`'s pre-migration repair switch used +// (`verify.cts:2405-2553`) — ported verbatim, not reinvented. + +// eslint-disable-next-line @typescript-eslint/no-require-imports +import planningWorkspaceMod = require('./planning-workspace.cjs'); +const { planningRoot, planningDir } = planningWorkspaceMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import configLoaderMod = require('./config-loader.cjs'); +const { CONFIG_DEFAULTS } = configLoaderMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import roadmapParserMod = require('./roadmap-parser.cjs'); +const { getMilestoneInfo } = roadmapParserMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports +import stateMod = require('./state.cjs'); +const { writeStateMd } = stateMod; +import { realClock } from './clock.cjs'; +import { platformReadSync as safeReadFile, platformWriteSync } from './shell-command-projection.cjs'; +import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; + // ─── Evaluator ────────────────────────────────────────────────────────────── /** * Evaluate an explicit `rules` array against `snapshot`, throwing if any two - * entries share a `code` (defense in depth beside the future static lint - * guard, §8.2 rule 1). Separated from `evaluateRules` so the duplicate-code - * guard is unit-testable against a small, locally-constructed fake rule - * array, independent of whether `RULES` itself has any entries yet (it does - * not, in this skeleton). + * entries share a `code` (defense in depth beside the static lint guard, + * §8.2 rule 1, `scripts/lint-health-diagnostic-rule-table.cjs`). Separated + * from `evaluateRules` so the duplicate-code guard is unit-testable against + * a small, locally-constructed fake rule array, independent of the real + * `RULES` table. */ function evaluateRuleTable(rules: Rule[], snapshot: PlanningSnapshot): Diagnostic[] { const seen = new Set(); @@ -110,17 +144,231 @@ function evaluateRules(snapshot: PlanningSnapshot): Diagnostic[] { } // ─── Repair dispatcher ────────────────────────────────────────────────────── +// Repair-handler bodies (real, ported from verify.cts:2405-2553). /** - * Stub repair handler. Real per-action handlers (`createConfig`, - * `resetConfig`, `regenerateState`, `addNyquistKey`, - * `addAiIntegrationPhaseKey`, `backfillMilestones`) land in a later - * migration batch alongside the rules that need them — see this phase's - * brief. Applying a NONE-risk remedy is a no-op beyond recording it, in this - * skeleton. + * One `repairs_performed`-shaped entry (legacy `cmdValidateHealth` output + * shape), tagged with the diagnostic `code` it came from so + * `applyRepairs`'s caller can build BOTH the code-keyed `applied`/`refused` + * arrays this module's own tests lock (`tests/health-diagnostic.test.cjs`) + * AND the action-keyed `repairs_performed` array `cmdValidateHealth` still + * emits. `code` is stripped by the caller before the entry reaches JSON + * output — the legacy shape never carried it. */ -function applyStubRepair(_cwd: string, _diagnostic: Diagnostic): void { - /* intentionally empty — real handlers land with the rules that need them */ +interface RepairDetail { + code: string; + action: string; + success: boolean; + path?: string; + detail?: string; + error?: string; +} + +interface RepairPaths { + rootBase: string; + configPath: string; + statePath: string; + milestonesPath: string; + milestonesArchiveDir: string; +} + +/** + * Derive every filesystem path a repair handler needs, from `cwd` alone — + * exactly how `cmdValidateHealth` derived them pre-migration + * (`verify.cts:1644-1652`/`2301-2302`). `config.json`/`MILESTONES.md`/ + * `milestones/` are root-scoped (`planningRoot`); `STATE.md` is + * workstream-scoped (`planningDir`) — the same root-vs-workstream split + * `buildConfigField`/`buildStateFields` (`planning-snapshot.cts`) already + * document for the read side. + */ +function repairPaths(cwd: string): RepairPaths { + const rootBase = planningRoot(cwd); + const wsBase = planningDir(cwd); + return { + rootBase, + configPath: path.join(rootBase, 'config.json'), + statePath: path.join(wsBase, 'STATE.md'), + milestonesPath: path.join(rootBase, 'MILESTONES.md'), + milestonesArchiveDir: path.join(rootBase, 'milestones'), + }; +} + +/** `verify.cts:2413-2429`'s default config.json payload, ported verbatim. */ +function defaultConfigPayload(): Record { + return { + model_profile: CONFIG_DEFAULTS.model_profile, + commit_docs: CONFIG_DEFAULTS.commit_docs, + search_gitignored: CONFIG_DEFAULTS.search_gitignored, + branching_strategy: CONFIG_DEFAULTS.branching_strategy, + phase_branch_template: CONFIG_DEFAULTS.phase_branch_template, + milestone_branch_template: CONFIG_DEFAULTS.milestone_branch_template, + quick_branch_template: CONFIG_DEFAULTS.quick_branch_template, + workflow: { + research: CONFIG_DEFAULTS.research, + plan_check: CONFIG_DEFAULTS.plan_checker, + verifier: CONFIG_DEFAULTS.verifier, + nyquist_validation: CONFIG_DEFAULTS.nyquist_validation, + }, + parallelization: CONFIG_DEFAULTS.parallelization, + brave_search: CONFIG_DEFAULTS.brave_search, + }; +} + +/** + * `verify.cts:2301-2335`'s W018 archived-vs-documented-versions diff, + * relocated verbatim (same two regexes, same two-file read) so + * `backfillMilestones` can recompute exactly which versions are missing + * without a `PlanningSnapshot` (`applyRepairs` is not a `Rule` and is not + * handed one — see this file's header comment). This is the same + * derivation `buildMilestoneArchiveStatusField` + * (`src/planning-snapshot.cts`) already performs for the W018 RULE's read + * side; recomputed here, not re-invented, because the rule's own + * `Diagnostic.remedy.args` carries no version list (confirmed by direct + * read of `src/health-diagnostic-rules/milestone-archive-hygiene.cts`). + */ +function computeMissingMilestoneVersions(milestonesArchiveDir: string, milestonesPath: string): string[] { + let archivedVersions: string[] = []; + try { + if (fs.existsSync(milestonesArchiveDir)) { + const archiveFiles = fs.readdirSync(milestonesArchiveDir); + archivedVersions = archiveFiles + .map((f) => f.match(/^(v\d+\.\d+(?:\.\d+)?)-ROADMAP\.md$/)) + .filter((m): m is RegExpMatchArray => m !== null) + .map((m) => m[1]); + } + } catch { + /* intentionally empty — mirrors the original's advisory try/catch */ + } + + let documentedVersions: string[] = []; + try { + if (fs.existsSync(milestonesPath)) { + const registryContent = fs.readFileSync(milestonesPath, 'utf-8'); + documentedVersions = [...registryContent.matchAll(/^##\s+(v\d+\.\d+(?:\.\d+)?)/gm)].map((m) => m[1]); + } + } catch { + /* intentionally empty */ + } + + const documented = new Set(documentedVersions); + return archivedVersions.filter((v) => !documented.has(v)); +} + +interface RepairOutcome { + success: boolean; + path?: string; + detail?: string; + error?: string; + // regenerateState's original backup step (verify.cts:2435-2440) pushed its + // own SEPARATE `repairActions` entry before the main one — preserved here + // as extra, prepended detail rows. Unreachable in practice today + // (regenerateState is DESTRUCTIVE and `applyRepairs`'s dispatcher below + // refuses it before this handler is ever invoked), but the handler stays + // complete rather than partially ported, per this batch's brief. + extraDetails?: { action: string; success: boolean; path?: string }[]; +} + +/** + * Execute exactly one real repair action, ported behavior-preserving from + * `verify.cts:2405-2553`'s `switch (repair)`. Throws are the caller's + * responsibility to catch (mirrors the original's per-action try/catch + * shape, collapsed to one seam here since every case now shares one + * caller). + */ +function runRepairAction(cwd: string, action: RemedyAction, paths: RepairPaths): RepairOutcome { + const { rootBase, configPath, statePath, milestonesPath, milestonesArchiveDir } = paths; + + switch (action) { + case REMEDY_ACTION.CREATE_CONFIG: + case REMEDY_ACTION.RESET_CONFIG: { + platformWriteSync(configPath, JSON.stringify(defaultConfigPayload(), null, 2)); + return { success: true, path: 'config.json' }; + } + + case REMEDY_ACTION.REGENERATE_STATE: { + const extraDetails: { action: string; success: boolean; path?: string }[] = []; + if (fs.existsSync(statePath)) { + const timestamp = new Date().toISOString().replace(/[:.]/g, '-').slice(0, 19); + const backupPath = `${statePath}.bak-${timestamp}`; + fs.copyFileSync(statePath, backupPath); + extraDetails.push({ action: 'backupState', success: true, path: backupPath }); + } + const milestone = getMilestoneInfo(cwd).value; + const projectRef = path + .relative(cwd, path.join(rootBase, 'PROJECT.md')) + .split(path.sep) + .join('/'); + const slashRuntime = resolveRuntime(cwd); + const slash = (name: string) => formatGsdSlash(name, slashRuntime) as string; + let stateContent = `# Session State\n\n`; + stateContent += `## Project Reference\n\n`; + stateContent += `See: ${projectRef}\n\n`; + stateContent += `## Position\n\n`; + stateContent += `**Milestone:** ${milestone?.version ?? ''} ${milestone?.name ?? ''}\n`; + stateContent += `**Current phase:** (determining...)\n`; + stateContent += `**Status:** Resuming\n\n`; + stateContent += `## Session Log\n\n`; + stateContent += `- ${realClock.localToday()}: STATE.md regenerated by ${slash('health')} --repair\n`; + writeStateMd(statePath, stateContent, cwd); + return { success: true, path: 'STATE.md', extraDetails }; + } + + case REMEDY_ACTION.ADD_NYQUIST_KEY: + case REMEDY_ACTION.ADD_AI_INTEGRATION_PHASE_KEY: { + const key = action === REMEDY_ACTION.ADD_NYQUIST_KEY ? 'nyquist_validation' : 'ai_integration_phase'; + const configRaw = fs.readFileSync(configPath, 'utf-8'); + const configParsed = JSON.parse(configRaw) as Record; + if (!configParsed['workflow']) configParsed['workflow'] = {}; + const wf = configParsed['workflow'] as Record; + if (wf[key] === undefined) { + wf[key] = true; + platformWriteSync(configPath, JSON.stringify(configParsed, null, 2)); + } + return { success: true, path: 'config.json' }; + } + + case REMEDY_ACTION.BACKFILL_MILESTONES: { + const missing = computeMissingMilestoneVersions(milestonesArchiveDir, milestonesPath); + const today = realClock.localToday(); + const slashRuntime = resolveRuntime(cwd); + const slash = (name: string) => formatGsdSlash(name, slashRuntime) as string; + let backfilled = 0; + for (const ver of missing) { + try { + const snapshotPath = path.join(milestonesArchiveDir, `${ver}-ROADMAP.md`); + const snapshot = safeReadFile(snapshotPath); + const titleMatch = snapshot && snapshot.match(/^#\s+(.+)$/m); + const milestoneName = titleMatch + ? titleMatch[1].replace(/^Milestone\s+/i, '').replace(/^v[\d.]+\s*/, '').trim() + : ver; + const entry = + `## ${ver}${milestoneName && milestoneName !== ver ? ` ${milestoneName}` : ''} (Backfilled: ${today})\n\n**Note:** Synthesized from archive snapshot by \`${slash('health')} --backfill\`. Original completion date unknown.\n\n---\n\n`; + const milestonesContent = fs.existsSync(milestonesPath) + ? fs.readFileSync(milestonesPath, 'utf-8') + : ''; + if (!milestonesContent.trim()) { + platformWriteSync(milestonesPath, `# Milestones\n\n${entry}`); + } else { + const headerMatch = milestonesContent.match(/^(#{1,3}\s+[^\n]*\n\n?)/); + if (headerMatch) { + const header = headerMatch[1]; + const rest = milestonesContent.slice(header.length); + platformWriteSync(milestonesPath, header + entry + rest); + } else { + platformWriteSync(milestonesPath, entry + milestonesContent); + } + } + backfilled++; + } catch { + /* intentionally empty — partial backfill is acceptable */ + } + } + return { success: true, detail: `Backfilled ${backfilled} milestone(s) into MILESTONES.md` }; + } + + default: + return { success: false, error: `no repair handler registered for action "${action}"` }; + } } /** @@ -136,21 +384,33 @@ function applyStubRepair(_cwd: string, _diagnostic: Diagnostic): void { * - Requested and `remedy.risk === DESTRUCTIVE` — pushed onto `refused`, * handler never invoked. This is the §8.3 rule 3 breaking-change * enforcement point: a DESTRUCTIVE remedy is describable but is never - * applied by `--repair`. - * - Requested and `remedy.risk === NONE` — stub handler invoked, pushed - * onto `applied`. + * applied by `--repair`. A `details` row is still recorded, so the + * refusal is VISIBLE in `cmdValidateHealth`'s `repairs_performed` output, + * not silently dropped. + * - Requested and `remedy.risk === NONE` — the real handler is invoked, + * pushed onto `applied`. + * + * `applied`/`refused` are unchanged in shape from the pre-existing skeleton + * (locked by `tests/health-diagnostic.test.cjs`, rows 11-12): arrays of + * diagnostic `code`s. `details` is ADDITIVE — every real action maps 1:1 to + * exactly one code in this rule table (confirmed: no `REMEDY_ACTION` other + * than `ADVISE` is used by more than one rule), so `cmdValidateHealth` can + * rebuild the legacy action-keyed `repairs_performed` shape directly from + * it. */ function applyRepairs( cwd: string, diagnostics: Diagnostic[], repair: boolean, backfill: boolean, -): { applied: string[]; refused: string[] } { +): { applied: string[]; refused: string[]; details: RepairDetail[] } { const applied: string[] = []; const refused: string[] = []; + const details: RepairDetail[] = []; + const paths = repairPaths(cwd); for (const diagnostic of diagnostics) { - const { remedy } = diagnostic; + const { remedy, code } = diagnostic; if (remedy.action === REMEDY_ACTION.ADVISE) continue; const requested = @@ -158,15 +418,43 @@ function applyRepairs( if (!requested) continue; if (remedy.risk === REMEDY_RISK.DESTRUCTIVE) { - refused.push(diagnostic.code); + refused.push(code); + details.push({ + code, + action: remedy.action, + success: false, + error: `refused: '${remedy.action}' is a destructive remedy and is not auto-applied by --repair`, + }); continue; } - applyStubRepair(cwd, diagnostic); - applied.push(diagnostic.code); + try { + const outcome = runRepairAction(cwd, remedy.action, paths); + if (outcome.extraDetails) { + for (const extra of outcome.extraDetails) { + details.push({ code, action: extra.action, success: extra.success, ...(extra.path ? { path: extra.path } : {}) }); + } + } + details.push({ + code, + action: remedy.action, + success: outcome.success, + ...(outcome.path ? { path: outcome.path } : {}), + ...(outcome.detail ? { detail: outcome.detail } : {}), + ...(outcome.error ? { error: outcome.error } : {}), + }); + } catch (err) { + details.push({ + code, + action: remedy.action, + success: false, + error: err instanceof Error ? err.message : String(err), + }); + } + applied.push(code); } - return { applied, refused }; + return { applied, refused, details }; } // ─── Exports ──────────────────────────────────────────────────────────────── diff --git a/src/verify.cts b/src/verify.cts index 9da03c2aa..30a3658e5 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -9,9 +9,8 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; -import { phaseVariants, buildRoadmapPhaseVariants, buildNotStartedPhaseVariants } from './validate.cjs'; -import { realClock } from './clock.cjs'; -import { phaseDirNameRe, PHASE_TOKEN_FROM_DIR_RE, MILESTONE_ARCHIVE_DIR_RE, textEncodingError } from './validate.cjs'; +import { phaseVariants, buildRoadmapPhaseVariants } from './validate.cjs'; +import { PHASE_TOKEN_FROM_DIR_RE, 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 @@ -28,13 +27,10 @@ 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, platformWriteSync, posixNormalize } from './shell-command-projection.cjs'; -import { PACKAGE_NAME } from './package-identity.cjs'; +import { execGit, platformReadSync as safeReadFile, posixNormalize } from './shell-command-projection.cjs'; import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; import { detectSchemaFiles, checkSchemaDrift } from './schema-detect.cjs'; -import { isCanonicalPlanningFile } from './artifacts.cjs'; import { extractTaggedBlocks } from './markdown-sectionizer.cjs'; -import { VALID_PROFILES, VALID_TIERS, VALID_PHASE_TYPES } from './model-catalog.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports -- agent-install-check.cjs is an export= CommonJS module import agentInstallCheck = require('./agent-install-check.cjs'); const { checkAgentsInstalled, checkCodexModelPosture } = agentInstallCheck; @@ -43,26 +39,27 @@ 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, CONFIG_DEFAULTS } = configLoaderMod; +const { loadConfig } = configLoaderMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { normalizePhaseName, matchPhaseDirs, escapeRegex, getMilestoneFromPhaseId, OPTIONAL_PHASE_TAG_SOURCE, PHASE_NUMBER_TOKEN_SOURCE, extractPhaseToken, stripProjectCodePrefix, comparePhaseNum, isSentinelPhaseId } = phaseIdMod; +const { normalizePhaseName, matchPhaseDirs, stripProjectCodePrefix, isSentinelPhaseId } = 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 { getMilestoneInfo, stripShippedMilestones, extractCurrentMilestone } = roadmapParserMod; -// eslint-disable-next-line @typescript-eslint/no-require-imports -import worktreeSafetyMod = require('./worktree-safety.cjs'); -const { inspectWorktreeHealth } = worktreeSafetyMod; -// eslint-disable-next-line @typescript-eslint/no-require-imports -- commands.cjs is an export= CommonJS module -import commandsMod = require('./commands.cjs'); -const { determinePhaseStatus } = commandsMod; +const { stripShippedMilestones, extractCurrentMilestone } = 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; +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'); +const { buildPlanningSnapshot } = planningSnapshotMod; const { planningDir, planningRoot } = planningWorkspace; const { extractFrontmatter, parseMustHavesBlock } = frontmatterMod; -const { writeStateMd, readStateHeadFreshness } = stateMod; +const { readStateHeadFreshness } = stateMod; /** * W024 (#2573) threshold — how many commits STATE.md may lag HEAD before @@ -1306,21 +1303,6 @@ function listMilestoneArchiveDirs(planBase: string): string[] { } } -function forEachArchivedPhaseToken(planBase: string, onPhase: (token: string) => void): void { - for (const archiveDir of listMilestoneArchiveDirs(planBase)) { - try { - const entries = fs.readdirSync(archiveDir, { withFileTypes: true }); - for (const e of entries) { - if (!e.isDirectory()) continue; - const m = e.name.match(PHASE_TOKEN_FROM_DIR_RE); - if (m) onPhase(stripProjectCodePrefix(m[1])); - } - } catch { - /* archive dir absent/unreadable */ - } - } -} - function getActiveMilestoneArchiveDir(planBase: string): string | null { const archiveDirs = listMilestoneArchiveDirs(planBase); if (archiveDirs.length === 0) return null; @@ -1400,63 +1382,6 @@ function collectDiskPhases(planBase: string): Set { return new Set(collectDiskPhaseEntries(planBase).keys()); } -/** - * #2528: archived phase DIRECTORY NAMES, the name-side twin of - * `forEachArchivedPhaseToken`. W006 must not warn about a roadmap phase whose - * only directory lives in a shipped-milestone archive, and deciding that needs - * the same name-based resolution the active roots get. - */ -function collectArchivedPhaseDirNames(planBase: string): string[] { - const names: string[] = []; - for (const archiveDir of listMilestoneArchiveDirs(planBase)) { - try { - for (const e of fs.readdirSync(archiveDir, { withFileTypes: true })) { - if (e.isDirectory() && PHASE_TOKEN_FROM_DIR_RE.test(e.name)) names.push(e.name); - } - } catch { - /* archive dir absent/unreadable */ - } - } - return names; -} - -interface MilestoneMismatch { - phaseId: string; - foundInMilestone: string; - expectedMilestone: string; -} - -function checkMilestonePrefixMismatches( - roadmapContent: string, - { getMilestoneFromPhaseId }: { getMilestoneFromPhaseId: (id: string) => string | null }, -): MilestoneMismatch[] { - const mismatches: MilestoneMismatch[] = []; - const sections: { version: string; start: number; end: number }[] = []; - const sectionRx = /^#{1,3}\s+(?:\[[^\]]{1,200}\]\s*)?.*v(\d+\.\d+)/gim; - let m: RegExpExecArray | null; - while ((m = sectionRx.exec(roadmapContent)) !== null) { - if (sections.length > 0) sections[sections.length - 1].end = m.index; - sections.push({ version: `v${m[1]}`, start: m.index, end: roadmapContent.length }); - } - for (const section of sections) { - const content = roadmapContent.slice(section.start, section.end); - // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phaseRx = /#{2,4}\s*(?:\[[^\]]{1,200}\]\s*)?Phase\s+([\w][\w.-]*)(?:\s*\([^)\n]{0,200}\))?\s*:/gi; - let pm: RegExpExecArray | null; - while ((pm = phaseRx.exec(content)) !== null) { - const phaseId = pm[1]; - const expectedMilestone = getMilestoneFromPhaseId(phaseId); - if (expectedMilestone !== null && expectedMilestone !== section.version) { - mismatches.push({ - phaseId, - foundInMilestone: section.version, - expectedMilestone, - }); - } - } - } - return mismatches; -} interface IssueEntry { code: string; @@ -1465,6 +1390,79 @@ interface IssueEntry { repairable: boolean; } +/** + * Wrapper-level fix-text table for `cmdValidateHealth`'s migrated + * `HealthDiagnostic` -> `IssueEntry` mapping (Phase 11, #3309). Rules cannot + * call `slash()` (forbidden ambient I/O, §8.1 rule 1) so every REPAIRABLE + * (non-ADVISE) diagnostic's `fix` text — which the pre-migration code always + * built via `${slash('health')} --repair|--backfill ...` — is reconstructed + * HERE instead, keyed by `remedy.action`. Each real repair action maps 1:1 + * back to exactly one pre-migration code (confirmed: no `REMEDY_ACTION` + * other than `ADVISE` is used by more than one rule in the migrated table), + * so this table reproduces the original `fix` text byte-for-byte, including + * its `slash()` calls, without the RULE ever needing to know about `slash`. + * Source line refs are the exact pre-migration `addIssue(...)` call each + * text was copied from: + * - createConfig — verify.cts:1782 (W003) + * - resetConfig — verify.cts:1830 (E005) + * - regenerateState — verify.cts:1702 (E004) + * - addNyquistKey — verify.cts:1847 (W008) + * - addAiIntegrationPhaseKey — verify.cts:1857 (W016) + * - backfillMilestones — verify.cts:2326 (W018) + * ADVISE diagnostics do NOT go through this table — their `fix` is + * `diagnostic.remedy.args.command` directly (already a complete, final + * string baked in by the rule, confirmed via + * `src/health-diagnostic-rules/root-existence.cts`/`config-validation.cts`). + */ +function repairFixText(slash: (name: string) => string, action: string): string { + switch (action) { + case REMEDY_ACTION.CREATE_CONFIG: + return `Run ${slash('health')} --repair to create with defaults`; + case REMEDY_ACTION.RESET_CONFIG: + return `Run ${slash('health')} --repair to reset to defaults`; + case REMEDY_ACTION.REGENERATE_STATE: + return `Run ${slash('health')} --repair to regenerate`; + case REMEDY_ACTION.ADD_NYQUIST_KEY: + case REMEDY_ACTION.ADD_AI_INTEGRATION_PHASE_KEY: + return `Run ${slash('health')} --repair to add key`; + case REMEDY_ACTION.BACKFILL_MILESTONES: + return `Run ${slash('health')} --backfill to synthesize missing entries from archive snapshots`; + default: + return ''; + } +} + +/** + * Map one `HealthDiagnostic` (rule-table shape) back onto the legacy + * `IssueEntry` shape `cmdValidateHealth` has always returned (design doc, + * "Output-shape preservation" section). + * + * `repairable` for a DESTRUCTIVE-risk remedy (regenerateState/resetConfig) + * is `false` here — NOT the pre-migration `true` those two codes always + * carried. This is a deliberate, disclosed decision (see this batch's + * dispatch report): the design doc's own "`--repair` behavior change" + * section establishes that `--repair` never actually applies a DESTRUCTIVE + * remedy post-migration, so marking it `repairable: true` would mislead a + * caller that uses this field to decide whether re-running with `--repair` + * will fix anything. `repairable` now means "an automatic repair will + * actually run", not merely "a remedy exists to describe" — the more + * conservative of the two readings the brief identified, chosen because the + * question was genuinely ambiguous and this reading cannot itself cause a + * caller to skip a fix that would have worked. + */ +function diagnosticToIssueEntry(diagnostic: HealthDiagnostic, slash: (name: string) => string): IssueEntry { + const { code, message, remedy } = diagnostic; + if (remedy.action === REMEDY_ACTION.ADVISE) { + return { code, message, fix: remedy.args['command'] as string, repairable: false }; + } + return { + code, + message, + fix: repairFixText(slash, remedy.action), + repairable: remedy.risk !== REMEDY_RISK.DESTRUCTIVE, + }; +} + function cmdValidateConsistency(cwd: string, raw: boolean): void { const planBase = planningDir(cwd); const roadmapPath = path.join(planBase, 'ROADMAP.md'); @@ -1640,917 +1638,104 @@ function cmdValidateHealth( } // rootBase always resolves to .planning/ (shared root — PROJECT.md, config.json live here) - // wsBase resolves to .planning/workstreams// when GSD_WORKSTREAM is set (STATE.md, ROADMAP.md, phases/) const rootBase = planningRoot(cwd); - const wsBase = planningDir(cwd); - // planBase is kept as an alias for wsBase for all the internal helpers (collectDiskPhases, etc.) - // that are already parameterised on the workstream-aware path. - const planBase = wsBase; - const projectPath = path.join(rootBase, 'PROJECT.md'); - const roadmapPath = path.join(wsBase, 'ROADMAP.md'); - const statePath = path.join(wsBase, 'STATE.md'); - const configPath = path.join(rootBase, 'config.json'); - const phasesDir = path.join(wsBase, 'phases'); const _slashRuntime = resolveRuntime(cwd); const slash = (name: string) => formatGsdSlash(name, _slashRuntime) as string; + // Second (and last) pre-check that stays OUTSIDE the rule table entirely + // (design doc, "Two guards that stay OUTSIDE the rule table entirely" — + // this one must run BEFORE any snapshot is built, since a flat + // `evaluateRules` pass over an entirely-absent `.planning/` would produce + // spurious per-rule clutter no one asked for, not a clean E001-only + // report). + if (!fs.existsSync(rootBase)) { + const errors: IssueEntry[] = [ + { + code: 'E001', + message: '.planning/ directory not found', + fix: `Run ${slash('new-project')} to initialize`, + repairable: false, + }, + ]; + output({ status: 'broken', errors, warnings: [], info: [], repairable_count: 0 }, raw); + return; + } + + // ─── Rule-table evaluation (Phase 11, #3309) ─────────────────────────────── + // Replaces the entire hand-rolled addIssue/switch accumulation this + // function used to run inline (verify.cts, pre-migration) — see the + // design doc's "Output-shape preservation" section for the exact + // Diagnostic -> IssueEntry mapping contract this reproduces. + const snapshot = buildPlanningSnapshot(cwd); + const diagnostics = evaluateRules(snapshot); + const errors: IssueEntry[] = []; const warnings: IssueEntry[] = []; const info: IssueEntry[] = []; - const repairs: string[] = []; - - const addIssue = ( - severity: 'error' | 'warning' | 'info', - code: string, - message: string, - fix: string, - repairable = false, - ) => { - const issue: IssueEntry = { code, message, fix, repairable }; - if (severity === 'error') errors.push(issue); - else if (severity === 'warning') warnings.push(issue); - else info.push(issue); - }; - - if (!fs.existsSync(rootBase)) { - addIssue('error', 'E001', '.planning/ directory not found', `Run ${slash('new-project')} to initialize`); - output({ status: 'broken', errors, warnings, info, repairable_count: 0 }, raw); - return; + for (const diagnostic of diagnostics) { + const entry = diagnosticToIssueEntry(diagnostic, slash); + if (diagnostic.severity === HEALTH_SEVERITY.ERROR) errors.push(entry); + else if (diagnostic.severity === HEALTH_SEVERITY.WARNING) warnings.push(entry); + else info.push(entry); } - if (!fs.existsSync(projectPath)) { - addIssue('error', 'E002', 'PROJECT.md not found', `Run ${slash('new-project')} to create`); - } else { - const content = fs.readFileSync(projectPath, 'utf-8'); - const requiredSections = ['## What This Is', '## Core Value', '## Requirements']; - for (const section of requiredSections) { - if (!content.includes(section)) { - addIssue('warning', 'W001', `PROJECT.md missing section: ${section}`, 'Add section manually'); - } - } - } - - if (!fs.existsSync(roadmapPath)) { - addIssue('error', 'E003', 'ROADMAP.md not found', `Run ${slash('new-milestone')} to create roadmap`); - } - - if (!fs.existsSync(statePath)) { - addIssue( - 'error', - 'E004', - 'STATE.md not found', - `Run ${slash('health')} --repair to regenerate`, - true, - ); - repairs.push('regenerateState'); - } else { - const stateContent = fs.readFileSync(statePath, 'utf-8'); - - // W024 (#2573): STATE.md commit-age freshness. Advisory ONLY — it appends - // to warnings[] and never touches `status`, the repair set, or any existing - // count. Silent when the stamp is absent or unresolvable: "unknown" is not - // a finding. The threshold is deliberately coarse so an ordinary project - // stays quiet — firing on every project would change health's observable - // "clean" state for anything gating on it. - { - const fm = extractFrontmatter(stateContent) as Record; - const freshness = readStateHeadFreshness(cwd, fm['state_head']); - if ( - freshness.commits_behind !== null && - freshness.commits_behind >= STATE_HEAD_ADVISORY_COMMITS - ) { - addIssue( - 'warning', - 'W024', - `STATE.md was written ${freshness.commits_behind} commits ago (at ${freshness.state_head}) — treat its contents as approximate`, - 'Re-read the current phase artifacts before relying on STATE.md, or run a GSD command that refreshes it', - ); - } - } - - const phaseRefs = [ - ...stateContent.matchAll(new RegExp(`[Pp]hase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})`, 'g')), - ].map( - (m) => m[1], - ); - const validPhases = collectDiskPhases(planBase); - try { - if (fs.existsSync(roadmapPath)) { - const roadmapRaw = fs.readFileSync(roadmapPath, 'utf-8'); - const all = [ - ...roadmapRaw.matchAll(new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})`, 'gi')), - ]; - for (const m of all) validPhases.add(m[1]); - } - } catch { - /* intentionally empty */ - } - forEachArchivedPhaseToken(planBase, (token) => validPhases.add(token)); - const normalizedValid = new Set(); - for (const p of validPhases) { - normalizedValid.add(p); - const dotIdx = p.indexOf('.'); - const head = dotIdx === -1 ? p : p.slice(0, dotIdx); - const tail = dotIdx === -1 ? '' : p.slice(dotIdx); - if (/^\d+$/.test(head)) { - normalizedValid.add(head.padStart(2, '0') + tail); - } - } - for (const ref of phaseRefs) { - const dotIdx = ref.indexOf('.'); - const head = dotIdx === -1 ? ref : ref.slice(0, dotIdx); - const tail = dotIdx === -1 ? '' : ref.slice(dotIdx); - const padded = /^\d+$/.test(head) ? head.padStart(2, '0') + tail : ref; - if (!normalizedValid.has(ref) && !normalizedValid.has(padded)) { - if (normalizedValid.size > 0) { - addIssue( - 'warning', - 'W002', - `STATE.md references phase ${ref}, but only phases ${[...validPhases].sort((a, b) => a.localeCompare(b, undefined, { numeric: true })).join(', ')} are declared`, - `Review STATE.md manually before changing it; ${slash('health')} --repair will not overwrite an existing STATE.md for phase mismatches`, - ); - } - } - } - } - - if (!fs.existsSync(configPath)) { - addIssue( - 'warning', - 'W003', - 'config.json not found', - `Run ${slash('health')} --repair to create with defaults`, - true, - ); - repairs.push('createConfig'); - } else { - try { - const rawCfg = fs.readFileSync(configPath, 'utf-8'); - const parsed = JSON.parse(rawCfg) as Record; - if (parsed['model_profile'] && !VALID_PROFILES.includes(parsed['model_profile'] as string)) { - addIssue( - 'warning', - 'W004', - `config.json: invalid model_profile "${parsed['model_profile'] as string}"`, - `Valid values: ${VALID_PROFILES.join(', ')}`, - ); - } - const configModels = parsed['models']; - if (configModels && typeof configModels === 'object' && !Array.isArray(configModels)) { - for (const [phaseType, tierValue] of Object.entries(configModels as Record)) { - if (!VALID_PHASE_TYPES.has(phaseType)) { - addIssue( - 'warning', - 'W022', - `config.json: models has an unknown phase type "${phaseType}" which will be ignored`, - `Valid phase types: ${[...VALID_PHASE_TYPES].join(', ')}`, - ); - } else if (typeof tierValue !== 'string' || !VALID_TIERS.has(tierValue)) { - addIssue( - 'warning', - 'W022', - `config.json: models.${phaseType} has an invalid tier value ${JSON.stringify(tierValue)} which will be ignored`, - `Valid tiers: ${[...VALID_TIERS].join(', ')}`, - ); - } - } - } else if (configModels !== undefined && configModels !== null) { - addIssue( - 'warning', - 'W022', - `config.json: models is set to ${JSON.stringify(configModels)}, but must be an object mapping phase types to tiers — this value will be ignored`, - `Set models to an object like {"planning": "sonnet"}, or remove the key to use profile defaults`, - ); - } - } catch (err) { - addIssue( - 'error', - 'E005', - `config.json: JSON parse error - ${err instanceof Error ? err.message : String(err)}`, - `Run ${slash('health')} --repair to reset to defaults`, - true, - ); - repairs.push('resetConfig'); - } - } - - if (fs.existsSync(configPath)) { - try { - const configRaw = fs.readFileSync(configPath, 'utf-8'); - const configParsed = JSON.parse(configRaw) as Record; - const workflow = configParsed['workflow'] as Record | undefined; - if (workflow && workflow['nyquist_validation'] === undefined) { - addIssue( - 'warning', - 'W008', - 'config.json: workflow.nyquist_validation absent (defaults to enabled but agents may skip)', - `Run ${slash('health')} --repair to add key`, - true, - ); - if (!repairs.includes('addNyquistKey')) repairs.push('addNyquistKey'); - } - if (workflow && workflow['ai_integration_phase'] === undefined) { - addIssue( - 'warning', - 'W016', - `config.json: workflow.ai_integration_phase absent (defaults to enabled — run ${slash('ai-integration-phase')} before planning AI system phases)`, - `Run ${slash('health')} --repair to add key`, - true, - ); - if (!repairs.includes('addAiIntegrationPhaseKey')) repairs.push('addAiIntegrationPhaseKey'); - } - } catch { - /* intentionally empty */ - } - } - - let phaseDirEntries: fs.Dirent[] = []; - const phaseDirFiles = new Map(); - // #3183: companion map of the single owner's scan per phase dir - // (root+nested, superseded-excluded plan/summary sets + canonical - // pairing), computed alongside the raw readdirSync listing above. The - // W023 duplicate-dir describer and the I001 unsummarized-plan detector - // below use THIS map for plan/summary counts and pairing; phaseDirFiles - // stays raw for the RESEARCH/VALIDATION and phase-dir-naming checks that - // are not plan-count questions. - const phaseDirScans = new Map>(); - try { - phaseDirEntries = fs - .readdirSync(phasesDir, { withFileTypes: true }) - .filter((e) => e.isDirectory()); - for (const e of phaseDirEntries) { - try { - phaseDirFiles.set(e.name, fs.readdirSync(path.join(phasesDir, e.name))); - } catch { - phaseDirFiles.set(e.name, []); - } - phaseDirScans.set(e.name, planScanMod.scanPhasePlans(path.join(phasesDir, e.name))); - } - } catch { - /* intentionally empty */ - } - - for (const e of phaseDirEntries) { - if (!e.name.match(phaseDirNameRe)) { - addIssue( - 'warning', - 'W005', - `Phase directory "${e.name}" doesn't follow NN-name format`, - 'Rename to match pattern (e.g., 01-setup)', - ); - } - } - - // W023 (#2408): detect two or more real on-disk phase directories that - // normalize to the same phase key (e.g. `05-real/` + `05-real-stray/`). - // The collision silently breaks /gsd-stats status accuracy (now folded by - // precedence — see commands.cts foldPhaseStatus) and forces an operator - // decision. Wording is neutral — never guesses which directory is "real". + // W024 (#2573): STATE.md commit-age freshness — kept OUTSIDE the rule + // table, exactly like E001/E010/I010 above. NOT a preservation nicety: the + // committed `RULE_W024` (`src/health-diagnostic-rules/state-consistency.cts`) + // is a documented PERMANENT no-op (`check` always returns `[]`) because + // `readStateHeadFreshness`'s `git log` shell-out is ambient I/O a + // `Rule.check(snapshot)` may never perform (§8.1 rule 1) and no + // `PlanningSnapshot` field carries a commits-behind count. Migrating + // `cmdValidateHealth` onto the rule table as designed would silently + // regress `tests/health-validation.test.cjs`'s "W024 — STATE.md commit-age + // freshness advisory" suite (7 currently-passing tests exercising the REAL + // git-based check end-to-end) — found while wiring this function to the + // rule table, fixed inline per this repo's no-defer policy rather than + // silently accepting the loss. `cmdValidateHealth` itself (unlike a Rule) + // is licensed to perform its own bounded I/O — the same license + // `applyRepairs` already relies on (see `health-diagnostic.cts`'s header + // comment) — so this reproduces the exact pre-migration check + // (`verify.cts`, W024) verbatim, advisory-only: it only ever appends to + // `warnings`, never touches `status`/`errors`/the repair set. { - const groups = new Map(); - for (const e of phaseDirEntries) { - // extractPhaseToken never returns empty — for unparseable dir names it - // falls back to the dir name itself. Two distinct unparseable names - // therefore normalize to distinct keys and cannot false-positive here; - // only dirs whose tokens collapse to the same key (e.g. `05-real` and - // `05-real-stray` → token `05`) produce a collision group. - const token = extractPhaseToken(e.name); - const key = normalizePhaseName(token); - const list = groups.get(key); - if (list) list.push(e.name); - else groups.set(key, [e.name]); - } - for (const [key, dirs] of groups) { - if (dirs.length < 2) continue; - // Compute each dir's status independently so the warning is informative. - // Sort by phase id for stable output regardless of readdir order; tie- - // break on the dir name itself so two dirs sharing the same phase token - // (the collision case itself) still sort deterministically (V8's stable - // sort would otherwise fall back to non-portable fs.readdirSync order). - const described = dirs - .slice() - .sort((a, b) => comparePhaseNum(a, b) || String(a).localeCompare(String(b))) - .map((d) => { - // #3183: canonical plan/summary counts (root+nested, - // superseded-excluded, canonical pairing) from the single owner. - const scan = phaseDirScans.get(d); - const plans = scan ? scan.planCount : 0; - const summaries = scan ? scan.summaryCount : 0; - const status = determinePhaseStatus(plans, summaries, path.join(phasesDir, d), 'Not Started'); - return `${d} (${status})`; - }) - .join(', '); - addIssue( - 'warning', - 'W023', - `Phase directories collide on normalized key "${key}": ${described}`, - 'Inspect each directory; rename or remove the duplicate so only one directory maps to this phase key', - ); - } - } - - // I001 (#3183): this IS findUnsummarizedPlans's exact question — routed - // through the single owner's scan (root+nested, superseded-excluded plan - // set) and the canonical summaryCandidates-based pairing, instead of a - // bespoke canonicalPlanStem reimplementation. Fixes: a superseded plan is - // no longer permanently flagged "may be in progress" (false noise - // forever), and nested (#3139 layout) plans are no longer invisible. - for (const e of phaseDirEntries) { - const scan = phaseDirScans.get(e.name); - const planFiles = scan ? scan.planFiles : []; - const summaryFiles = scan ? scan.summaryFiles : []; - for (const plan of findUnsummarizedPlans(planFiles, summaryFiles)) { - addIssue('info', 'I001', `${e.name}/${plan} has no SUMMARY.md`, 'May be in progress'); - } - } - - for (const e of phaseDirEntries) { - const phaseFiles = phaseDirFiles.get(e.name) || []; - const hasResearch = phaseFiles.some((f) => f.endsWith('-RESEARCH.md')); - const hasValidation = phaseFiles.some((f) => f.endsWith('-VALIDATION.md')); - if (hasResearch && !hasValidation) { - const researchFile = phaseFiles.find((f) => f.endsWith('-RESEARCH.md')); + const wsBase = planningDir(cwd); + const statePath = path.join(wsBase, 'STATE.md'); + if (fs.existsSync(statePath)) { try { - const researchContent = fs.readFileSync( - path.join(phasesDir, e.name, researchFile!), - 'utf-8', - ); - if (researchContent.includes('## Validation Architecture')) { - addIssue( - 'warning', - 'W009', - `Phase ${e.name}: has Validation Architecture in RESEARCH.md but no VALIDATION.md`, - `Re-run ${slash('plan-phase')} with --research to regenerate`, - ); + const stateContent = fs.readFileSync(statePath, 'utf-8'); + const fm = extractFrontmatter(stateContent) as Record; + const freshness = readStateHeadFreshness(cwd, fm['state_head']); + if ( + freshness.commits_behind !== null && + freshness.commits_behind >= STATE_HEAD_ADVISORY_COMMITS + ) { + warnings.push({ + code: 'W024', + message: `STATE.md was written ${freshness.commits_behind} commits ago (at ${freshness.state_head}) — treat its contents as approximate`, + fix: 'Re-read the current phase artifacts before relying on STATE.md, or run a GSD command that refreshes it', + repairable: false, + }); } } catch { - /* intentionally empty */ + /* intentionally empty — W024 is advisory */ } } } - try { - const agentStatus = checkAgentsInstalled(_slashRuntime, cwd); - if (!agentStatus.agents_installed) { - if ((agentStatus.installed_agents).length === 0) { - addIssue( - 'warning', - 'W010', - `No GSD agents found in ${agentStatus.agents_dir} — Task(subagent_type="gsd-*") will fall back to general-purpose`, - `Run the GSD installer: npx ${PACKAGE_NAME}@latest`, - ); - } else if ((agentStatus.incomplete_agents).length > 0 && (agentStatus.missing_agents).length === 0) { - addIssue( - 'warning', - 'W010', - `Incomplete agent installs (missing generated file): ${(agentStatus.incomplete_agents).join(', ')} — affected workflows may fall back to general-purpose`, - `Re-run the GSD installer to complete the install: npx ${PACKAGE_NAME}@latest`, - ); - } else if ((agentStatus.incomplete_agents).length > 0) { - addIssue( - 'warning', - 'W010', - `Missing ${(agentStatus.missing_agents).length} GSD agents: ${(agentStatus.missing_agents).join(', ')}; incomplete agent installs (missing generated file): ${(agentStatus.incomplete_agents).join(', ')} — affected workflows will fall back to general-purpose`, - `Run the GSD installer: npx ${PACKAGE_NAME}@latest`, - ); - } else { - addIssue( - 'warning', - 'W010', - `Missing ${(agentStatus.missing_agents).length} GSD agents: ${(agentStatus.missing_agents).join(', ')} — affected workflows will fall back to general-purpose`, - `Run the GSD installer: npx ${PACKAGE_NAME}@latest`, - ); - } - } - } catch { - /* intentionally empty — agent check is non-blocking */ - } - - if (fs.existsSync(roadmapPath)) { - const roadmapContentRaw = fs.readFileSync(roadmapPath, 'utf-8'); - const roadmapContent = extractCurrentMilestone(roadmapContentRaw, cwd); - - const { roadmapPhases } = buildRoadmapPhaseVariants(roadmapContent); - const { roadmapPhases: fullRoadmapPhases, roadmapPhaseVariants: fullRoadmapPhaseVariants } = - buildRoadmapPhaseVariants(roadmapContentRaw); - - const diskPhases = collectDiskPhases(planBase); - forEachArchivedPhaseToken(planBase, (token) => diskPhases.add(token)); - - const activeDiskEntries = collectDiskPhaseEntries(planBase); - - // #2528: the name side of the same inventory. The token sets above answer - // "do two independently-derived labels agree"; these answer "does the - // canonical matcher resolve this roadmap phase to a real directory" — the - // question W006/W007 are actually asking. Both are kept: the token - // intersection still decides every shape it already decided correctly, and - // the resolution below only ever REMOVES a warning, so a phase the tokens - // already paired up cannot start warning because of this. - const activeDirNames = [...activeDiskEntries.values()].flat(); - const allDirNames = [...activeDirNames, ...collectArchivedPhaseDirNames(planBase)]; - - // A directory is CLAIMED when some roadmap phase resolves to it. This is the - // inverse mapping W007 never had: it iterates directories, so it has no query - // to resolve, and a dir whose label does not appear in the roadmap looked - // orphaned even when the roadmap phase that owns it resolves to it exactly. - // Built from the FULL roadmap (shipped milestones included), matching the - // variant set W007 already compares against. - const claimedDirs = new Set(); - for (const p of fullRoadmapPhases) { - for (const d of matchPhaseDirs(activeDirNames, normalizePhaseName(p)).matches) { - claimedDirs.add(d); - } - } - - const notStartedPhases = buildNotStartedPhaseVariants(roadmapContent); - - for (const p of roadmapPhases) { - // #3225: sentinel phase ids (999.x/0.x) are never-on-roadmap by convention; - // a sentinel heading shouldn't demand a directory. - if (isSentinelPhaseId(p)) continue; - const variants = phaseVariants(p); - const existsOnDisk = [...variants].some((v) => diskPhases.has(v)) - || matchPhaseDirs(allDirNames, normalizePhaseName(p)).matches.length > 0; - if (!existsOnDisk) { - const isNotStarted = [...variants].some((v) => notStartedPhases.has(v)); - if (isNotStarted) continue; - addIssue( - 'warning', - 'W006', - `Phase ${p} in ROADMAP.md but no directory on disk`, - 'Create phase directory or remove from roadmap', - ); - } - } - - for (const [p, dirsForToken] of activeDiskEntries) { - // #3225: a sentinel dir on disk (999-interim, 0-drafts) is defined as - // never-on-roadmap; it must not trigger W007 ("Add to roadmap or remove - // directory" — both wrong for a sentinel). Mirrors the isSentinelPhaseId - // guard phase.cts has at 10+ sites (#2786/#2949). - if (isSentinelPhaseId(p)) continue; - const variants = phaseVariants(p); - if ([...variants].some((v) => fullRoadmapPhaseVariants.has(v))) continue; - if (dirsForToken.every((d) => claimedDirs.has(d))) continue; - addIssue( - 'warning', - 'W007', - `Phase ${p} exists on disk but not in ROADMAP.md`, - 'Add to roadmap or remove directory', - ); - } - } - - if (fs.existsSync(statePath) && fs.existsSync(roadmapPath)) { - try { - const stateContent = fs.readFileSync(statePath, 'utf-8'); - const roadmapContentFull = fs.readFileSync(roadmapPath, 'utf-8'); - - const currentPhaseMatch = - stateContent.match(/\*\*Current Phase:\*\*\s*(\S+)/i) || - stateContent.match(/Current Phase:\s*(\S+)/i); - if (currentPhaseMatch) { - const statePhase = currentPhaseMatch[1].replace(/^0+/, ''); - const phaseCheckboxRe = new RegExp( - `-\\s*\\[x\\].*Phase\\s+0*${escapeRegex(statePhase)}${OPTIONAL_PHASE_TAG_SOURCE}[:\\s]`, - 'i', - ); - if (phaseCheckboxRe.test(roadmapContentFull)) { - const stateStatus = stateContent.match(/\*\*Status:\*\*\s*(.+)/i); - const statusVal = stateStatus ? stateStatus[1].trim().toLowerCase() : ''; - if (statusVal !== 'complete' && statusVal !== 'done') { - addIssue( - 'warning', - 'W011', - `STATE.md says current phase is ${statePhase} (status: ${statusVal || 'unknown'}) but ROADMAP.md shows it as [x] complete — state files may be out of sync`, - `Run ${slash('progress')} to re-derive current position, or manually update STATE.md`, - ); - } - } - } - } catch { - /* intentionally empty — cross-validation is advisory */ - } - } - - if (fs.existsSync(configPath)) { - try { - const configRaw = fs.readFileSync(configPath, 'utf-8'); - const configParsed = JSON.parse(configRaw) as Record; - - const validStrategies = ['none', 'phase', 'milestone']; - if ( - configParsed['branching_strategy'] && - !validStrategies.includes(configParsed['branching_strategy'] as string) - ) { - addIssue( - 'warning', - 'W012', - `config.json: invalid branching_strategy "${configParsed['branching_strategy'] as string}"`, - `Valid values: ${validStrategies.join(', ')}`, - ); - } - - if (configParsed['context_window'] !== undefined) { - const cw = configParsed['context_window']; - if (typeof cw !== 'number' || cw <= 0 || !Number.isInteger(cw)) { - addIssue( - 'warning', - 'W013', - `config.json: context_window should be a positive integer, got "${cw as string}"`, - 'Set to 200000 (default) or 1000000 (for 1M models)', - ); - } - } - - if ( - configParsed['phase_branch_template'] && - !(configParsed['phase_branch_template'] as string).includes('{phase}') - ) { - addIssue( - 'warning', - 'W014', - 'config.json: phase_branch_template missing {phase} placeholder', - 'Template must include {phase} for phase number substitution', - ); - } - if ( - configParsed['milestone_branch_template'] && - !(configParsed['milestone_branch_template'] as string).includes('{milestone}') - ) { - addIssue( - 'warning', - 'W015', - 'config.json: milestone_branch_template missing {milestone} placeholder', - 'Template must include {milestone} for version substitution', - ); - } - } catch { - /* parse error already caught in Check 5 */ - } - } - - try { - const worktreeHealth = (inspectWorktreeHealth as unknown as ( - cwd: string, - opts: { staleAfterMs: number }, - deps: { execGit: unknown; existsSync: unknown; statSync: unknown }, - ) => Record)( - cwd, - { staleAfterMs: 60 * 60 * 1000 }, - { execGit, existsSync: fs.existsSync, statSync: fs.statSync }, - ); - if (!(worktreeHealth['ok'] as boolean)) { - if (worktreeHealth['reason'] === 'git_timed_out') { - addIssue( - 'warning', - 'W020', - 'Worktree health check degraded: git worktree list timed out after 10s — orphan/stale worktrees could not be inspected', - 'Run: git worktree list --porcelain to diagnose; check for .git/index.lock or a hung git process', - ); - } - if (worktreeHealth['reason'] === 'git_list_failed') { - addIssue( - 'warning', - 'W020', - 'Worktree health check degraded: git worktree list failed — orphan/stale worktrees could not be inspected', - 'Run: git worktree list --porcelain to diagnose; check git repository state and permissions', - ); - } - } else { - for (const finding of worktreeHealth['findings'] as Record[]) { - if (finding['kind'] === 'orphan') { - addIssue( - 'warning', - 'W017', - `Orphan git worktree: ${finding['path'] as string} (path no longer exists on disk)`, - 'Run: git worktree prune', - ); - continue; - } - - if (finding['kind'] === 'stale') { - // Do not flag the active session's worktree — removing it would be harmful. - const worktreePath = finding['path'] as string; - const activeCwd = process.cwd(); - const normalizedWorktree = path.resolve(worktreePath); - const normalizedCwd = path.resolve(activeCwd); - // Skip if the worktree IS the cwd or is an ancestor of it. - const isActiveWorktree = - normalizedCwd === normalizedWorktree || - normalizedCwd.startsWith(normalizedWorktree + path.sep); - if (isActiveWorktree) continue; - addIssue( - 'warning', - 'W017', - `Stale git worktree: ${worktreePath} (last modified ${finding['ageMinutes'] as number} minutes ago)`, - `Run: git worktree remove ${worktreePath} --force`, - ); - continue; - } - - // #3050/#3057 (B5): a 'unverified' finding means existsSync confirmed - // the worktree is present but statSync threw, so orphan/stale status - // could not be determined for THIS entry — it must not be silently - // dropped (that would be the exact fail-open the row exists to close). - if (finding['kind'] === 'unverified') { - addIssue( - 'warning', - 'W020', - `Worktree health check degraded: could not stat ${finding['path'] as string} — presence/staleness could not be verified`, - 'Check filesystem permissions on the worktree path, or investigate why statSync failed for it', - ); - } - } - } - } catch { - /* git worktree not available or not a git repo — skip silently */ - } - - try { - const phaseConvention = (() => { - if (!fs.existsSync(configPath)) return null; - try { - const configRaw = fs.readFileSync(configPath, 'utf-8'); - const configParsed = JSON.parse(configRaw) as Record; - return (configParsed['phase_id_convention'] as string | undefined) || null; - } catch { - return null; - } - })(); - if (phaseConvention === 'milestone-prefixed') { - if (fs.existsSync(roadmapPath)) { - const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); - const mismatches = checkMilestonePrefixMismatches(roadmapContent, { - getMilestoneFromPhaseId: getMilestoneFromPhaseId, - }); - for (const mm of mismatches) { - addIssue( - 'warning', - 'W021', - `Phase ${mm.phaseId}: integer prefix implies ${mm.expectedMilestone} but listed under ${mm.foundInMilestone}`, - 'Run `gsd-tools roadmap upgrade --convention milestone-prefixed` to migrate (dry-run by default)', - ); - } - } - } - } catch { - /* W021 check is advisory — skip on error */ - } - - const milestonesPath = path.join(rootBase, 'MILESTONES.md'); - const milestonesArchiveDir = path.join(rootBase, 'milestones'); - const missingFromRegistry: string[] = []; - try { - if (fs.existsSync(milestonesArchiveDir)) { - const archiveFiles = fs.readdirSync(milestonesArchiveDir); - const archivedVersions = archiveFiles - .map((f) => f.match(/^(v\d+\.\d+(?:\.\d+)?)-ROADMAP\.md$/)) - .filter(Boolean) - .map((m) => m![1]); - - if (archivedVersions.length > 0) { - const registryContent = fs.existsSync(milestonesPath) - ? fs.readFileSync(milestonesPath, 'utf-8') - : ''; - for (const ver of archivedVersions) { - if (!registryContent.includes(`## ${ver}`)) { - missingFromRegistry.push(ver); - } - } - if (missingFromRegistry.length > 0) { - addIssue( - 'warning', - 'W018', - `MILESTONES.md missing ${missingFromRegistry.length} archived milestone(s): ${missingFromRegistry.join(', ')}`, - `Run ${slash('health')} --backfill to synthesize missing entries from archive snapshots`, - true, - ); - repairs.push('backfillMilestones'); - } - } - } - } catch { - /* intentionally empty — milestone sync check is advisory */ - } - - try { - const entries = fs.readdirSync(rootBase, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isFile()) continue; - if (!entry.name.endsWith('.md')) continue; - if (!isCanonicalPlanningFile(entry.name)) { - addIssue( - 'warning', - 'W019', - `Unrecognized .planning/ file: ${entry.name} — not a canonical GSD artifact`, - 'Move to .planning/milestones/ archive subdir or delete if stale. See templates/README.md for the canonical artifact list.', - false, - ); - } - } - } catch { - /* artifact check is advisory — skip on error */ - } - - try { - if (fs.existsSync(statePath) && fs.existsSync(roadmapPath)) { - const stateRaw = fs.readFileSync(statePath, 'utf-8'); - const statusMatch = stateRaw.match(/^status:\s*(.+)/im); - const stateStatus = statusMatch ? statusMatch[1].trim().toLowerCase() : ''; - const isMarkedComplete = /milestone complete|archived/.test(stateStatus); - if (isMarkedComplete) { - const roadmapRaw = fs.readFileSync(roadmapPath, 'utf-8'); - const scopedContent = extractCurrentMilestone(roadmapRaw, cwd); - // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE). - const phasePattern = new RegExp(`#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gi'); - const unstarted: string[] = []; - let pm: RegExpExecArray | null; - // Non-hoisted: load-order matters (circular dep guard) - // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-workspace.cjs is an export= CommonJS module - const planningWorkspace2 = require('./planning-workspace.cjs') as typeof planningWorkspace; - const phasesDir2 = planningWorkspace2.planningPaths(cwd).phases; - const phaseDirNames2 = (() => { - try { - return fs - .readdirSync(phasesDir2, { withFileTypes: true }) - .filter((e) => e.isDirectory()) - .map((e) => e.name); - } catch { - return []; - } - })(); - while ((pm = phasePattern.exec(scopedContent)) !== null) { - const phaseNum = pm[1]; - const normalizedPh = normalizePhaseName(phaseNum); - const hasDirectory = matchPhaseDirs(phaseDirNames2, normalizedPh).matches.length > 0; - if (!hasDirectory) { - unstarted.push(phaseNum); - } - } - if (unstarted.length > 0) { - addIssue( - 'warning', - 'W021', - `STATE says milestone complete but ROADMAP lists ${unstarted.length} unstarted phase(s) (e.g. Phase ${unstarted[0]})`, - 'Run validate consistency or re-run complete-milestone after verifying all phases are done', - ); - } - } - } - } catch { - /* W021 check is advisory — skip on error */ - } - // ─── Perform repairs if requested ───────────────────────────────────────── - const repairActions: Record[] = []; - if (options['repair'] && repairs.length > 0) { - for (const repair of repairs) { - try { - switch (repair) { - case 'createConfig': - case 'resetConfig': { - const defaults = { - model_profile: CONFIG_DEFAULTS.model_profile, - commit_docs: CONFIG_DEFAULTS.commit_docs, - search_gitignored: CONFIG_DEFAULTS.search_gitignored, - branching_strategy: CONFIG_DEFAULTS.branching_strategy, - phase_branch_template: CONFIG_DEFAULTS.phase_branch_template, - milestone_branch_template: CONFIG_DEFAULTS.milestone_branch_template, - quick_branch_template: CONFIG_DEFAULTS.quick_branch_template, - workflow: { - research: CONFIG_DEFAULTS.research, - plan_check: CONFIG_DEFAULTS.plan_checker, - verifier: CONFIG_DEFAULTS.verifier, - nyquist_validation: CONFIG_DEFAULTS.nyquist_validation, - }, - parallelization: CONFIG_DEFAULTS.parallelization, - brave_search: CONFIG_DEFAULTS.brave_search, - }; - platformWriteSync(configPath, JSON.stringify(defaults, null, 2)); - repairActions.push({ action: repair, success: true, path: 'config.json' }); - break; - } - case 'regenerateState': { - if (fs.existsSync(statePath)) { - const timestamp = new Date().toISOString().replace(/[:.]/g, '-').slice(0, 19); - const backupPath = `${statePath}.bak-${timestamp}`; - fs.copyFileSync(statePath, backupPath); - repairActions.push({ action: 'backupState', success: true, path: backupPath }); - } - const milestone = getMilestoneInfo(cwd).value; - const projectRef = path - .relative(cwd, path.join(rootBase, 'PROJECT.md')) - .split(path.sep) - .join('/'); - let stateContent = `# Session State\n\n`; - stateContent += `## Project Reference\n\n`; - stateContent += `See: ${projectRef}\n\n`; - stateContent += `## Position\n\n`; - stateContent += `**Milestone:** ${milestone?.version ?? ''} ${milestone?.name ?? ''}\n`; - stateContent += `**Current phase:** (determining...)\n`; - stateContent += `**Status:** Resuming\n\n`; - stateContent += `## Session Log\n\n`; - stateContent += `- ${realClock.localToday()}: STATE.md regenerated by ${slash('health')} --repair\n`; - writeStateMd(statePath, stateContent, cwd); - repairActions.push({ action: repair, success: true, path: 'STATE.md' }); - break; - } - case 'addNyquistKey': { - if (fs.existsSync(configPath)) { - try { - const configRaw = fs.readFileSync(configPath, 'utf-8'); - const configParsed = JSON.parse(configRaw) as Record; - if (!configParsed['workflow']) configParsed['workflow'] = {}; - const wf = configParsed['workflow'] as Record; - if (wf['nyquist_validation'] === undefined) { - wf['nyquist_validation'] = true; - platformWriteSync(configPath, JSON.stringify(configParsed, null, 2)); - } - repairActions.push({ action: repair, success: true, path: 'config.json' }); - } catch (err) { - repairActions.push({ - action: repair, - success: false, - error: err instanceof Error ? err.message : String(err), - }); - } - } - break; - } - case 'addAiIntegrationPhaseKey': { - if (fs.existsSync(configPath)) { - try { - const configRaw = fs.readFileSync(configPath, 'utf-8'); - const configParsed = JSON.parse(configRaw) as Record; - if (!configParsed['workflow']) configParsed['workflow'] = {}; - const wf = configParsed['workflow'] as Record; - if (wf['ai_integration_phase'] === undefined) { - wf['ai_integration_phase'] = true; - platformWriteSync(configPath, JSON.stringify(configParsed, null, 2)); - } - repairActions.push({ action: repair, success: true, path: 'config.json' }); - } catch (err) { - repairActions.push({ - action: repair, - success: false, - error: err instanceof Error ? err.message : String(err), - }); - } - } - break; - } - case 'backfillMilestones': { - if (!options['backfill'] && !options['repair']) break; - const today = realClock.localToday(); - let backfilled = 0; - for (const ver of missingFromRegistry) { - try { - const snapshotPath = path.join(milestonesArchiveDir, `${ver}-ROADMAP.md`); - const snapshot = safeReadFile(snapshotPath); - const titleMatch = snapshot && snapshot.match(/^#\s+(.+)$/m); - const milestoneName = titleMatch - ? titleMatch[1].replace(/^Milestone\s+/i, '').replace(/^v[\d.]+\s*/, '').trim() - : ver; - const entry = - `## ${ver}${milestoneName && milestoneName !== ver ? ` ${milestoneName}` : ''} (Backfilled: ${today})\n\n**Note:** Synthesized from archive snapshot by \`${slash('health')} --backfill\`. Original completion date unknown.\n\n---\n\n`; - const milestonesContent = fs.existsSync(milestonesPath) - ? fs.readFileSync(milestonesPath, 'utf-8') - : ''; - if (!milestonesContent.trim()) { - platformWriteSync(milestonesPath, `# Milestones\n\n${entry}`); - } else { - const headerMatch = milestonesContent.match(/^(#{1,3}\s+[^\n]*\n\n?)/); - if (headerMatch) { - const header = headerMatch[1]; - const rest = milestonesContent.slice(header.length); - platformWriteSync(milestonesPath, header + entry + rest); - } else { - platformWriteSync(milestonesPath, entry + milestonesContent); - } - } - backfilled++; - } catch { - /* intentionally empty — partial backfill is acceptable */ - } - } - repairActions.push({ - action: repair, - success: true, - detail: `Backfilled ${backfilled} milestone(s) into MILESTONES.md`, - }); - break; - } - } - } catch (err) { - repairActions.push({ - action: repair, - success: false, - error: err instanceof Error ? err.message : String(err), - }); - } - } - } + // `applyRepairs` internally no-ops (produces zero `details` rows, no + // filesystem writes) when both `repair` and `backfill` are falsy, so this + // call is unconditional — mirroring the original's own + // `if (options['repair'] && repairs.length > 0)` gate without needing to + // duplicate that condition here. `backfill` is threaded through as its own + // boolean (not folded into `repair`), which is what makes `--backfill` + // alone now actually trigger `backfillMilestones` — the disclosed latent- + // bug fix from `verify.cts:2504`'s previously-unreachable inner gate (see + // design doc "Known limits"). + const repairResult = applyRepairs(cwd, diagnostics, Boolean(options['repair']), Boolean(options['backfill'])); + // The legacy `repairs_performed` shape never carried a `code` field — + // strip it before it reaches JSON output. + const repairActions = repairResult.details.map(({ code: _code, ...rest }) => rest); let status: string; if (errors.length > 0) { diff --git a/tests/health-diagnostic.test.cjs b/tests/health-diagnostic.test.cjs index a2e9171f8..e8621da54 100644 --- a/tests/health-diagnostic.test.cjs +++ b/tests/health-diagnostic.test.cjs @@ -6,25 +6,26 @@ * Design: .gsd/phase/refactor-3309-health-diagnostic-rule-table/40-design.md * Test matrix: .gsd/phase/refactor-3309-health-diagnostic-rule-table/50-test-matrix.md * - * This file covers ONLY the skeleton's own contract — test-matrix section 2, - * rows 9-14. `RULES` starts EMPTY in this phase (later batches append the 32 - * extracted rules); rows 15-16 (the DESTRUCTIVE-refusal proof against REAL - * diagnostics emitted by real rules) and section 3 (per-rule fixtures) are - * deferred to the migration step that adds rules. This file DOES prove - * `applyRepairs`'s risk-gating logic directly against hand-constructed fake - * `Diagnostic` objects, independent of whether any real rule produces them - * yet — per this phase's brief. - * - * TDD RED: `src/health-diagnostic.cts` does not exist yet — this file's - * `require('../gsd-core/bin/lib/health-diagnostic.cjs')` throws - * MODULE_NOT_FOUND until this phase's implementation lands. That is the - * intended starting state. + * Covers test-matrix section 2 (rows 9-16) against the FULLY WIRED rule + * table (`RULES` now carries all 31 rules — see the "RULES" describe block + * below for the exact count and why it is 31, not 32 — extracted from + * `cmdValidateHealth`, `src/verify.cts:1616-2577`). Rows 15-16 (the + * DESTRUCTIVE-refusal proof and the NONE-risk apply proof) run against REAL + * diagnostics emitted by REAL rules over a REAL `buildPlanningSnapshot` + * projection of a temp fixture, not hand-constructed fakes — the + * hand-constructed-fake coverage (rows 11-12 below) is kept alongside it + * since it exercises `applyRepairs`'s gating logic in isolation from any + * particular rule's shape. */ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); const healthDiagnostic = require('../gsd-core/bin/lib/health-diagnostic.cjs'); +const { buildPlanningSnapshot } = require('../gsd-core/bin/lib/planning-snapshot.cjs'); +const { createTempProject, createTempGitProject, cleanup } = require('./helpers.cjs'); const { SEVERITY, @@ -36,6 +37,50 @@ const { applyRepairs, } = healthDiagnostic; +// ─── Shared fixture helpers (mirror tests/orphan-worktree-detection.test.cjs's +// setupHealthyProject, the proven-healthy recipe for the pre-migration +// cmdValidateHealth) ──────────────────────────────────────────────────────── + +function writeMinimalProjectMd(tmpDir) { + const sections = ['## What This Is', '## Core Value', '## Requirements']; + const content = sections.map((s) => `${s}\n\nContent here.\n`).join('\n'); + fs.writeFileSync(path.join(tmpDir, '.planning', 'PROJECT.md'), `# Project\n\n${content}`); +} + +function writeMinimalRoadmap(tmpDir) { + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), '# Roadmap\n\n### Phase 1: Setup\n'); +} + +function writeMinimalStateMd(tmpDir) { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Session State\n\n## Current Position\n\nPhase: 1\n', + ); +} + +function writeValidConfigJson(tmpDir) { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify( + { + model_profile: 'balanced', + commit_docs: true, + workflow: { nyquist_validation: true, ai_integration_phase: true }, + }, + null, + 2, + ), + ); +} + +function setupHealthyProject(tmpDir) { + writeMinimalProjectMd(tmpDir); + writeMinimalRoadmap(tmpDir); + writeMinimalStateMd(tmpDir); + writeValidConfigJson(tmpDir); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-setup'), { recursive: true }); +} + // ─── Row 9 — REMEDY_ACTION locks exactly 7 members ───────────────────────── describe('REMEDY_ACTION', () => { @@ -171,9 +216,9 @@ describe('applyRepairs — risk gating (hand-constructed diagnostics)', () => { // ─── Row 13 — duplicate-code detection, LOCAL fake rule array ────────────── // -// `RULES` is still empty in this skeleton, so the duplicate check cannot be -// exercised through the real exported table yet. Proven here instead against -// a small, locally-constructed fake rule array — per this phase's brief. +// Proven against a small, locally-constructed fake rule array — independent +// of the real `RULES` table's own (already-unique, see the "RULES" describe +// block below) codes, so this guard's logic is covered in isolation. describe('evaluateRuleTable — duplicate-code guard (row 13)', () => { test('throws when two rules share the same code', () => { @@ -212,15 +257,97 @@ describe('evaluateRuleTable — duplicate-code guard (row 13)', () => { }); }); -// ─── Row 14 — evaluator against an all-clean (here: rule-less) snapshot ─── +// ─── RULES — the fully wired table ────────────────────────────────────────── +// +// 31 rule entries, not the design doc's own prose figure of "32" (that doc's +// "Rule table organization" section already flags its own count as +// inconsistent between its table and prose — see this repo's design doc, +// same section). Counted directly from each rule-group file's own exported +// `RULES` array: root-existence (4: E002/E003/E004/W001) + state-consistency +// (5: W024/W002/W011/W021/W026) + config-validation (10: W003/E005/W004/ +// W008/W016/W012/W013/W014/W015/W022) + phase-structure (4: W005/W023/I001/ +// W009) + agent-install (1: W010) + roadmap-disk-consistency (2: W006/W007) +// + worktree-health (3: W020/W017/W027) + milestone-archive-hygiene (2: +// W018/W019) = 31. E001 and the home-directory guard (E010/I010) are +// deliberately NOT rows (design doc, "Two guards that stay OUTSIDE the rule +// table entirely"). -describe('evaluateRules (row 14)', () => { - test('RULES starts empty in this skeleton', () => { - assert.deepEqual(RULES, []); +describe('RULES', () => { + test('is the full, frozen 31-rule table with every code unique', () => { assert.equal(Array.isArray(RULES), true); + assert.equal(RULES.length, 31); + const codes = RULES.map((r) => r.code); + assert.equal(new Set(codes).size, codes.length, 'every rule code must be unique'); }); - test('returns [] against any snapshot, since RULES is empty', () => { - assert.deepEqual(evaluateRules({}), []); + test('every rule carries a code, severity, and check function', () => { + for (const rule of RULES) { + assert.equal(typeof rule.code, 'string'); + assert.ok(Object.values(SEVERITY).includes(rule.severity), `${rule.code}: unknown severity ${rule.severity}`); + assert.equal(typeof rule.check, 'function'); + } + }); +}); + +// ─── Row 14 — evaluator against an all-clean REAL snapshot ──────────────── + +describe('evaluateRules (row 14)', () => { + test('evaluateRules(buildPlanningSnapshot(healthyProject)) returns []', (t) => { + const tmpDir = createTempGitProject(); + t.after(() => cleanup(tmpDir)); + setupHealthyProject(tmpDir); + + const snapshot = buildPlanningSnapshot(tmpDir); + const diagnostics = evaluateRules(snapshot); + assert.deepEqual(diagnostics, [], `expected zero diagnostics for a healthy project, got: ${JSON.stringify(diagnostics)}`); + }); +}); + +// ─── Rows 15-16 — applyRepairs against REAL diagnostics from REAL rules ──── + +describe('applyRepairs — REAL diagnostics (rows 15-16)', () => { + test('row 15: --repair given a real DESTRUCTIVE E004 finding (STATE.md missing) refuses regenerateState; STATE.md stays absent', (t) => { + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + setupHealthyProject(tmpDir); + fs.unlinkSync(path.join(tmpDir, '.planning', 'STATE.md')); + + const snapshot = buildPlanningSnapshot(tmpDir); + const diagnostics = evaluateRules(snapshot); + const e004 = diagnostics.find((d) => d.code === 'E004'); + assert.ok(e004, `expected E004 when STATE.md is missing, got: ${JSON.stringify(diagnostics)}`); + assert.equal(e004.remedy.action, REMEDY_ACTION.REGENERATE_STATE); + assert.equal(e004.remedy.risk, REMEDY_RISK.DESTRUCTIVE); + + const result = applyRepairs(tmpDir, diagnostics, true, false); + assert.ok(!result.applied.includes('E004'), 'E004 must not be applied'); + assert.ok(result.refused.includes('E004'), 'E004 must be refused'); + assert.equal( + fs.existsSync(path.join(tmpDir, '.planning', 'STATE.md')), + false, + 'STATE.md must remain absent — the DESTRUCTIVE remedy is refused, not silently applied', + ); + }); + + test('row 16: --repair given a real NONE-risk W003 finding (config.json missing) applies createConfig, exactly as pre-migration', (t) => { + const tmpDir = createTempProject(); + t.after(() => cleanup(tmpDir)); + setupHealthyProject(tmpDir); + fs.unlinkSync(path.join(tmpDir, '.planning', 'config.json')); + + const snapshot = buildPlanningSnapshot(tmpDir); + const diagnostics = evaluateRules(snapshot); + const w003 = diagnostics.find((d) => d.code === 'W003'); + assert.ok(w003, `expected W003 when config.json is missing, got: ${JSON.stringify(diagnostics)}`); + assert.equal(w003.remedy.action, REMEDY_ACTION.CREATE_CONFIG); + assert.equal(w003.remedy.risk, REMEDY_RISK.NONE); + + const result = applyRepairs(tmpDir, diagnostics, true, false); + assert.ok(result.applied.includes('W003'), 'W003 must be applied'); + assert.ok(!result.refused.includes('W003'), 'W003 must not be refused'); + const configPath = path.join(tmpDir, '.planning', 'config.json'); + assert.ok(fs.existsSync(configPath), 'config.json should now exist on disk'); + const diskConfig = JSON.parse(fs.readFileSync(configPath, 'utf-8')); + assert.equal(diskConfig.model_profile, 'balanced'); }); }); diff --git a/tests/phase-resolution-parity.test.cjs b/tests/phase-resolution-parity.test.cjs index 6cd7c407d..02d343289 100644 --- a/tests/phase-resolution-parity.test.cjs +++ b/tests/phase-resolution-parity.test.cjs @@ -439,14 +439,17 @@ describe('#2528 consumer parity — the eight sites migrated to matchPhaseDirs', }); test(`${name} — roadmap-driven consumers`, () => { - // 5. validate health, W021: STATE must claim the milestone is done for - // the roadmap-vs-disk consistency check to run at all. + // 5. validate health, W026 (Phase 11, #3309 — split off the + // pre-migration 'W021' site for this exact subject; the OTHER W021 + // subject, phase_id_convention mismatch, kept its code): STATE must + // claim the milestone is done for the roadmap-vs-disk consistency + // check to run at all. const health = json('validate health', project(dirs, query, 'milestone complete')); - const w021 = health.warnings.filter((w) => w.code === 'W021'); + const w026 = health.warnings.filter((w) => w.code === 'W026'); assert.strictEqual( - w021.length > 0, + w026.length > 0, !resolves, - `W021 disagreed on whether Phase ${query} is started: ${JSON.stringify(w021)}`, + `W026 disagreed on whether Phase ${query} is started: ${JSON.stringify(w026)}`, ); const tmpDir = project(dirs, query); diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs index 86b6dc340..2e562db3c 100644 --- a/tests/roadmap.test.cjs +++ b/tests/roadmap.test.cjs @@ -2822,9 +2822,14 @@ describe('bug #557 —
/ active milestone strip', () => { ); }); - // ── Health check W021: milestone_complete vs unstarted phases ───────────── + // ── Health check W026: milestone_complete vs unstarted phases ───────────── + // Phase 11 (#3309): this subject moved off the pre-migration 'W021' code + // onto the new 'W026' code (the split-off half of the two-subject + // conflation the design doc's "New codes for the two split subjects" + // section documents) — the OTHER W021 subject, phase_id_convention + // mismatch, kept its code. - test('validate health emits W021 when STATE says milestone complete but ROADMAP has unstarted phases', () => { + test('validate health emits W026 when STATE says milestone complete but ROADMAP has unstarted phases', () => { const planning = path.join(tmpDir, '.planning'); // ROADMAP still has active phases in it fs.writeFileSync(path.join(planning, 'ROADMAP.md'), ROADMAP_DETAILS_SUMMARY, 'utf-8'); @@ -2849,12 +2854,20 @@ Phase: Milestone v1.3 complete const output = JSON.parse(result.output); const warnings = output.warnings || []; - const w021 = warnings.find(w => w.code === 'W021'); + const w026 = warnings.find(w => w.code === 'W026'); assert.ok( - w021 !== undefined, - `Expected W021 warning (milestone-status vs. roadmap-progress incoherence). ` + + w026 !== undefined, + `Expected W026 warning (milestone-status vs. roadmap-progress incoherence). ` + `Got warnings: ${JSON.stringify(warnings.map(w => w.code))}` ); + // W021/W026 independence (Phase 11, #3309 split): this fixture's subject + // is the W026 one (milestone-complete vs. unstarted phases) — it must + // NOT also produce a W021 (phase_id_convention mismatch, an unrelated + // subject this config.json-less fixture never triggers). + assert.ok( + warnings.every(w => w.code !== 'W021'), + `W026 fixture must not also fire W021: ${JSON.stringify(warnings.map(w => w.code))}` + ); }); }); }); diff --git a/tests/verify-health.test.cjs b/tests/verify-health.test.cjs index 26c5fa612..5caa9e7af 100644 --- a/tests/verify-health.test.cjs +++ b/tests/verify-health.test.cjs @@ -173,7 +173,13 @@ describe('validate health command', () => { // ─── Check 4: STATE.md exists and references valid phases ───────────────── - test('errors when STATE.md is missing with repairable true', () => { + test('errors when STATE.md is missing with repairable false (DESTRUCTIVE remedy is never auto-applied)', () => { + // Phase 11 (#3309): E004's remedy (regenerateState) is DESTRUCTIVE, and + // `--repair` refuses to auto-apply a DESTRUCTIVE remedy (design doc, + // "--repair behavior change" section) — a disclosed breaking change from + // the pre-migration `repairable: true`. `repairable` now means "an + // automatic repair will actually run", not merely "a remedy exists to + // describe". writeMinimalProjectMd(tmpDir); writeMinimalRoadmap(tmpDir, ['1']); writeValidConfigJson(tmpDir); @@ -186,7 +192,7 @@ describe('validate health command', () => { const output = JSON.parse(result.output); const e004 = output.errors.find(e => e.code === 'E004'); assert.ok(e004, `Expected E004 in errors: ${JSON.stringify(output.errors)}`); - assert.strictEqual(e004.repairable, true, 'E004 should be repairable'); + assert.strictEqual(e004.repairable, false, 'E004 (DESTRUCTIVE remedy) should not be marked repairable'); }); test('warns when STATE.md references nonexistent phase', () => { @@ -1068,10 +1074,15 @@ describe('validate health --repair command', () => { assert.strictEqual(diskConfig.milestone_branch_template, 'gsd/{milestone}-{slug}'); }); - test('resets config.json when JSON is invalid', () => { + test('Phase 11 (#3309): refuses to reset config.json when JSON is invalid — resetConfig is DESTRUCTIVE, --repair leaves it untouched', () => { + // Pre-migration this repair action applied unconditionally; the design + // doc's "--repair behavior change" section makes this a disclosed + // breaking change: a DESTRUCTIVE remedy is reported (still visible in + // repairs_performed, as a refusal) but never executed by --repair. writeMinimalStateMd(tmpDir, '# Session State\n\nPhase 1 in progress.\n'); const configPath = path.join(tmpDir, '.planning', 'config.json'); - fs.writeFileSync(configPath, '{broken json'); + const originalContent = '{broken json'; + fs.writeFileSync(configPath, originalContent); const result = runGsdTools('validate health --repair', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`); @@ -1082,16 +1093,15 @@ describe('validate health --repair command', () => { `Expected repairs_performed: ${JSON.stringify(output)}` ); const resetAction = output.repairs_performed.find(r => r.action === 'resetConfig'); - assert.ok(resetAction, `Expected resetConfig action: ${JSON.stringify(output.repairs_performed)}`); + assert.ok(resetAction, `Expected a resetConfig refusal entry: ${JSON.stringify(output.repairs_performed)}`); + assert.strictEqual(resetAction.success, false, 'resetConfig must be refused, not applied'); + assert.match(resetAction.error || '', /destructive/i, 'refusal must explain WHY it was not applied'); - // Verify config.json is now valid JSON with correct nested structure - const diskConfig = JSON.parse(fs.readFileSync(configPath, 'utf-8')); - assert.ok(typeof diskConfig === 'object', 'config.json should be valid JSON after repair'); - assert.ok(diskConfig.workflow, 'reset config should have nested workflow object'); - assert.strictEqual(diskConfig.workflow.research, true, 'workflow.research should be true after reset'); + // config.json must remain exactly as it was — untouched. + assert.strictEqual(fs.readFileSync(configPath, 'utf-8'), originalContent, 'config.json must not be modified by a refused repair'); }); - test('regenerates STATE.md when missing', () => { + test('Phase 11 (#3309): refuses to regenerate STATE.md when missing — regenerateState is DESTRUCTIVE, --repair leaves it absent', () => { writeValidConfigJson(tmpDir); // No STATE.md const statePath = path.join(tmpDir, '.planning', 'STATE.md'); @@ -1106,13 +1116,18 @@ describe('validate health --repair command', () => { `Expected repairs_performed: ${JSON.stringify(output)}` ); const regenerateAction = output.repairs_performed.find(r => r.action === 'regenerateState'); - assert.ok(regenerateAction, `Expected regenerateState action: ${JSON.stringify(output.repairs_performed)}`); - assert.strictEqual(regenerateAction.success, true, 'regenerateState should succeed'); + assert.ok(regenerateAction, `Expected a regenerateState refusal entry: ${JSON.stringify(output.repairs_performed)}`); + assert.strictEqual(regenerateAction.success, false, 'regenerateState must be refused, not applied'); + assert.match(regenerateAction.error || '', /destructive/i, 'refusal must explain WHY it was not applied'); - // Verify STATE.md now exists and contains "# Session State" - assert.ok(fs.existsSync(statePath), 'STATE.md should now exist on disk'); - const stateContent = fs.readFileSync(statePath, 'utf-8'); - assert.ok(stateContent.includes('# Session State'), 'regenerated STATE.md should contain "# Session State"'); + // STATE.md must remain absent, and no backup file should have been created. + assert.strictEqual(fs.existsSync(statePath), false, 'STATE.md must remain absent — the DESTRUCTIVE remedy is refused'); + const planningFiles = fs.readdirSync(path.join(tmpDir, '.planning')); + assert.strictEqual( + planningFiles.some(f => f.startsWith('STATE.md.bak-')), + false, + 'no backup file should be created for a refused repair', + ); }); test('does not rewrite existing STATE.md for invalid phase references', () => { @@ -1168,8 +1183,12 @@ describe('validate health --repair command', () => { assert.strictEqual(diskConfig.workflow.nyquist_validation, true, 'nyquist_validation should be true'); }); - test('reports repairable_count correctly', () => { - // No config.json (W003, repairable=true) and no STATE.md (E004, repairable=true) + test('reports repairable_count correctly — counts NONE-risk findings only, not the DESTRUCTIVE E004', () => { + // No config.json (W003, createConfig, NONE risk -> repairable=true) and no + // STATE.md (E004, regenerateState, DESTRUCTIVE risk -> repairable=false, + // Phase 11 #3309: --repair never auto-applies a DESTRUCTIVE remedy, so it + // is deliberately excluded from this count — see the `diagnosticToIssueEntry` + // comment in src/verify.cts for the full reasoning). const configPath = path.join(tmpDir, '.planning', 'config.json'); if (fs.existsSync(configPath)) fs.unlinkSync(configPath); const statePath = path.join(tmpDir, '.planning', 'STATE.md'); @@ -1180,9 +1199,15 @@ describe('validate health --repair command', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); - assert.ok( - output.repairable_count >= 2, - `Expected repairable_count >= 2, got ${output.repairable_count}. Full output: ${JSON.stringify(output)}` + const w003 = output.warnings.find(w => w.code === 'W003'); + const e004 = output.errors.find(e => e.code === 'E004'); + assert.ok(w003, `Expected W003 in warnings: ${JSON.stringify(output.warnings)}`); + assert.ok(e004, `Expected E004 in errors: ${JSON.stringify(output.errors)}`); + assert.strictEqual(w003.repairable, true, 'W003 (createConfig, NONE risk) should be repairable'); + assert.strictEqual(e004.repairable, false, 'E004 (regenerateState, DESTRUCTIVE risk) should not be repairable'); + assert.strictEqual( + output.repairable_count, 1, + `Expected repairable_count 1 (W003 only), got ${output.repairable_count}. Full output: ${JSON.stringify(output)}` ); }); diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index a1b62d944..b1a9b9583 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -1938,6 +1938,76 @@ test('--backfill synthesizes missing MILESTONES.md entry from snapshot', () => { assert.ok(content.includes('Backfilled'), 'should note it was backfilled'); }); +// Phase 11 (#3309): pre-migration, `--backfill` ALONE (without `--repair`) +// was dead code — `verify.cts:2504`'s inner backfill gate was unreachable +// because the outer `if (options['repair'] && repairs.length > 0)` gate +// already required `repair`. The migrated `applyRepairs` threads `backfill` +// as its own boolean (`repair || backfill` for `backfillMilestones` +// specifically), so `--backfill` alone now actually works — a disclosed +// latent-bug fix (design doc, "Known limits"), not a preservation +// requirement. +test('--backfill alone (without --repair) now synthesizes the missing MILESTONES.md entry', () => { + const dir = makeTempProject({ + '.planning/PROJECT.md': '# P\n\n## What This Is\n\nX\n\n## Core Value\n\nY\n\n## Requirements\n\nZ\n', + '.planning/ROADMAP.md': '# Roadmap\n', + '.planning/STATE.md': '# State\n', + '.planning/config.json': '{}', + '.planning/milestones/v1.0-ROADMAP.md': '# Milestone v1.0 First Release\n', + }); + + cmdValidateHealth(dir, { repair: false, backfill: true }, false); + + const milestonesPath = path.join(dir, '.planning', 'MILESTONES.md'); + assert.ok(fs.existsSync(milestonesPath), '--backfill alone should create MILESTONES.md'); + const content = fs.readFileSync(milestonesPath, 'utf-8'); + assert.ok(content.includes('## v1.0'), 'backfilled entry should contain v1.0'); + assert.ok(content.includes('Backfilled'), 'should note it was backfilled'); +}); + +test('--backfill alone does NOT apply an unrelated NONE-risk repair (createConfig) — only backfillMilestones is gated by backfill', () => { + const dir = makeTempProject({ + '.planning/PROJECT.md': '# P\n\n## What This Is\n\nX\n\n## Core Value\n\nY\n\n## Requirements\n\nZ\n', + '.planning/ROADMAP.md': '# Roadmap\n', + '.planning/STATE.md': '# State\n', + // No config.json — W003 (createConfig) would fire and be repairable, but + // must NOT be applied by --backfill alone (only --repair applies it). + '.planning/milestones/v1.0-ROADMAP.md': '# Milestone v1.0 First Release\n', + }); + + cmdValidateHealth(dir, { repair: false, backfill: true }, false); + + const configPath = path.join(dir, '.planning', 'config.json'); + assert.strictEqual(fs.existsSync(configPath), false, 'config.json must not be created by --backfill alone'); + const milestonesPath = path.join(dir, '.planning', 'MILESTONES.md'); + assert.ok(fs.existsSync(milestonesPath), '--backfill alone should still create MILESTONES.md'); +}); + +// Phase 11 (#3309): W021 (phase_id_convention integer-prefix/milestone +// mismatch) and W026 (STATE milestone-complete vs. unstarted ROADMAP +// phases) are the split-off halves of the pre-migration 'W021' code — two +// genuinely unrelated subjects (design doc, "New codes for the two split +// subjects" section). This fixture triggers ONLY the phase_id_convention +// mismatch (W021's remaining subject) and must not also produce W026. +test('W021 (phase_id_convention mismatch) fires independently of W026 — same fixture never also emits W026', () => { + const dir = makeTempProject({ + '.planning/PROJECT.md': '# P\n\n## What This Is\n\nX\n\n## Core Value\n\nY\n\n## Requirements\n\nZ\n', + '.planning/ROADMAP.md': '# Roadmap\n\n## [GSD] v2.0 — Expansion\n\n### Phase 1-01: Setup\n**Goal:** g\n', + // STATE.md status is plainly "In progress" — never "milestone complete" + // or "archived", so W026's precondition never holds for this fixture. + '.planning/STATE.md': '# State\n\n## Current Position\n\nPhase: 1-01\n\n**Status:** In progress\n', + '.planning/config.json': JSON.stringify({ phase_id_convention: 'milestone-prefixed' }), + }); + + const result = cmdValidateHealth(dir, { repair: false }, false); + + const w021 = result.warnings.find(w => w.code === 'W021'); + assert.ok(w021, `expected W021 for phase 1-01 (implies v1.0) listed under v2.0: ${JSON.stringify(result.warnings)}`); + assert.ok( + result.warnings.every(w => w.code !== 'W026'), + `W021 fixture must not also fire W026: ${JSON.stringify(result.warnings.map(w => w.code))}` + ); +}); + test('health.md mentions --backfill flag', () => { const healthMd = fs.readFileSync( path.join(__dirname, '../gsd-core/workflows/health.md'), 'utf-8'