diff --git a/src/health-diagnostic-rules/roadmap-disk-consistency.cts b/src/health-diagnostic-rules/roadmap-disk-consistency.cts index 83ec18898..5b92de5bd 100644 --- a/src/health-diagnostic-rules/roadmap-disk-consistency.cts +++ b/src/health-diagnostic-rules/roadmap-disk-consistency.cts @@ -41,11 +41,26 @@ * (`src/planning-snapshot.cts`) instead: every directory actually present * under the active `phases/` root, unfiltered by roadmap declaration. * Archived-milestone directory names (`verify.cts:2050`, - * `collectArchivedPhaseDirNames`) are still not part of `PlanningSnapshot` - * and remain a disclosed fidelity reduction (a phase whose only directory - * lives in a shipped-milestone archive can read as W006-missing; a shipped - * archived dir is never scanned so it cannot spuriously read as - * W007-orphaned either) — unchanged by this fix. + * `collectArchivedPhaseDirNames`) are still not exposed as directory NAMES + * on `PlanningSnapshot`, but the equivalent TOKEN set is: `checkW006` below + * additionally consults `snapshot.archivedPhaseTokens` (added for the + * W002/state-consistency group's #3652 fix, reused verbatim here — see that + * field's own doc comment) so a phase whose only directory lives in a + * milestone archive (shipped OR the current milestone's own archive layout) + * no longer reads as W006-missing (Bug 1, found post-migration: the archived + * fixtures under `tests/milestone-archive.test.cjs` and + * `tests/verify-health.test.cjs` regressed against the pre-migration + * `verify.cts` behavior). W007 still never scans archived dirs (its loop is + * `allPhaseDirNames.value`, the active `phases/` root only), so an archived + * dir still cannot spuriously read as W007-orphaned either — unchanged. + * + * Bug 2 (found alongside Bug 1): `dirsForPhase` below also runs a + * `phaseVariants()`-based fallback when `matchPhaseDirs` finds nothing — see + * its own doc comment. Pre-migration, `verify.cts:2071-2073`/`2092-2093` ran + * this as a SECOND, independent check the migrated matchPhaseDirs-only path + * had dropped, causing a false W006/W007 whenever ROADMAP and disk spelled + * the same phase with a different zero-padding (e.g. ROADMAP "01A" vs disk + * "1A-..."). * * Not-started exclusion (verify.cts:2065/2075-2076, * `buildNotStartedPhaseVariants`, `src/validate.cts:160`): the design doc's @@ -114,7 +129,34 @@ const { phaseVariants } = validateMod; * file-level comment. */ function dirsForPhase(dirs: string[], phaseId: string): string[] { - return matchPhaseDirs(dirs, normalizePhaseName(phaseId)).matches; + const canonical = matchPhaseDirs(dirs, normalizePhaseName(phaseId)).matches; + if (canonical.length > 0) return canonical; + + // Bug 2 (#3309 W006/W007 migration cluster, found while fixing the + // originally-reported archived-directory gap): `matchPhaseDirs`'s + // `phaseTokenMatches` compares `extractPhaseToken(dir)` (the directory's + // LITERAL, un-normalized digit run — e.g. "1A" for `1A-suffix-phase`) + // against `normalizePhaseName(phaseId)` (which PADS — "01A") case- + // insensitively, but never unifies a padding/letter-suffix mismatch + // BETWEEN the two sides: "1A" !== "01A" even though they name the same + // phase. Pre-migration, `verify.cts:2071-2073`/`2092-2093` ran a SECOND, + // independent check here — `[...phaseVariants(p)].some((v) => + // diskPhases.has(v))` — that the migrated matchPhaseDirs-only path + // dropped. `phaseVariants` (`validate.cts:101`) is symmetric + // (padded<->unpadded, letter-suffix preserved both ways), so generating + // variants from `phaseId` and checking raw-disk-token membership is + // equivalent to intersecting `phaseVariants(phaseId)` with + // `phaseVariants(diskToken)` — variants always include their own input + // verbatim, so this fallback catches exactly the cases `matchPhaseDirs` + // alone misses without re-deriving a second matcher. + const variants = phaseVariants(phaseId); + return dirs.filter((d) => { + const token = extractPhaseToken(d).toUpperCase(); + for (const variant of variants) { + if (token === variant.toUpperCase()) return true; + } + return false; + }); } /** @@ -145,6 +187,7 @@ function checkW006(snapshot: PlanningSnapshot): Diagnostic[] { if (snapshot.roadmapDeclaredPhases.scope !== SCOPE.COMPLETE) return []; const dirs = snapshot.allPhaseDirNames.value; + const archivedTokens = new Set(snapshot.archivedPhaseTokens.value); const checkboxes = snapshot.roadmapPhaseCheckboxes.value; const diagnostics: Diagnostic[] = []; @@ -153,6 +196,17 @@ function checkW006(snapshot: PlanningSnapshot): Diagnostic[] { // convention; a sentinel heading shouldn't demand a directory. if (isSentinelPhaseId(phaseId)) continue; if (dirsForPhase(dirs, phaseId).length > 0) continue; + // Bug 1 (#3309 W006/W007 migration cluster): a phase whose ONLY + // directory lives under a milestone archive + // (`.planning/milestones/vX.Y-phases//`, shipped OR the current + // milestone's own archive layout) must not read as "no directory on + // disk" — mirrors `verify.cts:2038`'s + // `forEachArchivedPhaseToken(planBase, (token) => diskPhases.add(token))` + // feeding the archived-phase token set into this exact existence check. + // `snapshot.archivedPhaseTokens` (`src/planning-snapshot.cts`) is the + // same token set, reused verbatim from the W002/state-consistency + // group's own fix for the analogous gap — not a re-derivation. + if ([...phaseVariants(phaseId)].some((v) => archivedTokens.has(v))) continue; if (isPhaseNotStarted(phaseId, checkboxes)) continue; diagnostics.push({ code: 'W006', diff --git a/src/planning-snapshot.cts b/src/planning-snapshot.cts index b8be41e8d..3c42f6099 100644 --- a/src/planning-snapshot.cts +++ b/src/planning-snapshot.cts @@ -23,7 +23,7 @@ import fs from 'node:fs'; import path from 'node:path'; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserMod = require('./roadmap-parser.cjs'); -const { getMilestoneInfo } = roadmapParserMod; +const { getMilestoneInfo, extractCurrentMilestone } = roadmapParserMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { listMilestonePhaseDirs } = phaseLocatorMod; @@ -56,8 +56,8 @@ import worktreeSafetyMod = require('./worktree-safety.cjs'); const { inspectWorktreeHealth } = worktreeSafetyMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { PHASE_NUMBER_TOKEN_SOURCE, OPTIONAL_PHASE_TAG_SOURCE } = phaseIdMod; -import { buildRoadmapPhaseVariants } from './validate.cjs'; +const { PHASE_NUMBER_TOKEN_SOURCE, OPTIONAL_PHASE_TAG_SOURCE, stripProjectCodePrefix } = phaseIdMod; +import { buildRoadmapPhaseVariants, PHASE_TOKEN_FROM_DIR_RE, MILESTONE_ARCHIVE_DIR_RE } from './validate.cjs'; // ─── worstScope — the one new piece of coordination logic ─────────────────── @@ -117,7 +117,7 @@ interface PlanningSnapshot { // names, "the snapshot". config: { value: Record | null; scope: Scope; exists: boolean }; agentInstall: { value: ReturnType; scope: Scope }; - worktreeHealth: { value: ReturnType['findings']; scope: Scope }; + worktreeHealth: { value: ReturnType['findings']; scope: Scope; reason: string }; // ─── Phase 11 (#3309) "Rule table organization" additions ───────────────── // The design doc's own "Rule table organization" table and prose disagree // on the count: the table lists EIGHT rows (through `planningRootFiles`, @@ -163,6 +163,40 @@ interface PlanningSnapshot { // they already are for `phaseDirs` (see this batch's own disclosed // fidelity reduction for that). allPhaseDirNames: { value: string[]; scope: Scope }; + // W002 (STATE.md-consistency group) fidelity fix, found while implementing + // `src/health-diagnostic-rules/state-consistency.cts`. The original + // `cmdValidateHealth` W002 check unions THREE sources into its "valid + // phase" set — disk dirs, ROADMAP headings, and + // `forEachArchivedPhaseToken(planBase, ...)` (`verify.cts:1748`, every + // phase-token-shaped subdirectory under `.planning/milestones/*-phases/`, + // via the same `MILESTONE_ARCHIVE_DIR_RE`/`PHASE_TOKEN_FROM_DIR_RE` + // `listMilestoneArchiveDirs`/`forEachArchivedPhaseToken` use, both already + // exported from `validate.cjs` — no new regex derivation here). Without the + // third source, a STATE.md reference to a phase whose only directory lives + // in a shipped-milestone archive reads as an undeclared phase (#3652). + // Additive-only per this batch's own field-table constraint. Also now reused + // by `src/health-diagnostic-rules/roadmap-disk-consistency.cts`'s `checkW006` + // (Bug 1, #3309 W006/W007 migration cluster) for the same "was this token + // archived" question a ROADMAP *entry* needs answered, not just a STATE.md + // *reference* — same token set, two independent consumers, no re-derivation. + archivedPhaseTokens: { value: string[]; scope: Scope }; + // W026 (STATE.md-consistency group) fidelity fix, found while implementing + // `src/health-diagnostic-rules/state-consistency.cts`. W026's original + // logic (`verify.cts:2356-2399`, the second `addIssue('warning', 'W021', + // ...)` call site before the #3309 code split) scopes ROADMAP.md to the + // CURRENT milestone via `extractCurrentMilestone(roadmapRaw, cwd)` — the + // same shared, `
`/``-tolerant scoping owner every other + // milestone-aware consumer uses (`roadmap-parser.cts`) — then scans + // `#{2,4}\s*Phase\s+(TOKEN)...` headings within that scoped slice. + // `roadmapDeclaredPhases`'s `milestone` attribution (above) is NOT a fit + // here even though it looks adjacent: it exists to relocate + // `checkMilestonePrefixMismatches`'s OWN narrower `sectionRx` + // (`verify.cts:1429-1459`, `^#{1,3}\s+...vX.Y`, no `
` support) — + // faithful for W021 (which never supported `
` either), but + // reusing it for W026 would regress W026's ALREADY-`
`-tolerant + // original behavior. This field is W026's own, independently-scoped + // phase-id list — additive-only, no change to `roadmapDeclaredPhases`. + currentMilestoneRoadmapPhaseIds: { value: string[]; scope: Scope }; } /** @@ -260,9 +294,23 @@ function buildStateFields(statePath: string): StateFields { const section = stateCurrentPositionSlice(body); const currentPositionScope = section === null ? SCOPE.TRUNCATED : SCOPE.COMPLETE; - const currentPhaseLabel = stateFieldValue(frontmatter, section ?? body, null, 'Phase', { + // #1760 fallback ladder (mirrors `state.cts:1499-1500`'s `resolveStatePhase` + // exactly, same `section ?? body` scope for both reads): the legacy bold + // `**Current Phase:**` field (what `verify.cts:2109-2111` originally + // matched, and what pre-template-migration STATE.md fixtures still use) + // takes priority over the current template's bare `Phase: [X] of [Y]` + // field — a document carrying both is read the same way `resolveStatePhase` + // reads it elsewhere. + const legacyCurrentPhaseLabel = stateFieldValue(frontmatter, section ?? body, null, 'Current Phase', { scope: currentPositionScope, }); + const templateCurrentPhaseLabel = stateFieldValue(frontmatter, section ?? body, null, 'Phase', { + scope: currentPositionScope, + }); + const currentPhaseLabel = { + value: legacyCurrentPhaseLabel.value ?? templateCurrentPhaseLabel.value, + scope: legacyCurrentPhaseLabel.value !== null ? legacyCurrentPhaseLabel.scope : templateCurrentPhaseLabel.scope, + }; const stateStatus = stateFieldValue(frontmatter, section ?? body, 'status', 'Status', { scope: currentPositionScope, }); @@ -350,9 +398,17 @@ function buildAgentInstallField(cwd: string): { value: ReturnType['findings']; scope: Scope } { +function buildWorktreeHealthField(cwd: string): { value: ReturnType['findings']; scope: Scope; reason: string } { try { const result = inspectWorktreeHealth( cwd, @@ -360,11 +416,11 @@ function buildWorktreeHealthField(cwd: string): { value: ReturnType e.isDirectory() && MILESTONE_ARCHIVE_DIR_RE.test(e.name)) + .map((e) => path.join(milestonesDir, e.name)); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return { value: [], scope: SCOPE.COMPLETE }; + return { value: [], scope: SCOPE.UNREADABLE }; + } + + const value: string[] = []; + for (const archiveDir of archiveDirs) { + 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) value.push(stripProjectCodePrefix(m[1])); + } + } catch { + /* archive dir absent/unreadable — mirrors forEachArchivedPhaseToken */ + } + } + return { value, scope: SCOPE.COMPLETE }; +} + +/** + * Resolve `currentMilestoneRoadmapPhaseIds` — every phase-number token found + * in ROADMAP.md's content once scoped to the CURRENT milestone via + * `extractCurrentMilestone(content, cwd)`. Backs W026's archive-tolerant + * unstarted-phase scan; see the field's own doc comment on `PlanningSnapshot` + * for why `roadmapDeclaredPhases` cannot serve this. An absent/unreadable + * ROADMAP.md degrades to an empty list, mirroring every other + * ROADMAP-sourced field's absent-file handling. + */ +function buildCurrentMilestoneRoadmapPhaseIdsField( + cwd: string, + roadmapPath: string, +): { value: string[]; scope: Scope } { + if (!fs.existsSync(roadmapPath)) return { value: [], scope: SCOPE.UNREADABLE }; + let content: string; + try { + content = fs.readFileSync(roadmapPath, 'utf-8'); + } catch { + return { value: [], scope: SCOPE.UNREADABLE }; + } + const scoped = extractCurrentMilestone(content, cwd); + // #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal + // mirror of OPTIONAL_PHASE_TAG_SOURCE) — verbatim from `verify.cts:2366`. + const phasePattern = new RegExp( + `#{2,4}\\s*Phase\\s+(${PHASE_NUMBER_TOKEN_SOURCE})(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:`, + 'gi', + ); + const value = [...scoped.matchAll(phasePattern)].map((m) => m[1]); + return { value, scope: SCOPE.COMPLETE }; +} + /** * Build the full `.planning/` projection for `cwd`. Composes the six §7 * owners named in the design doc's "Owners consumed" table, plus (Phase 11, @@ -695,6 +824,8 @@ function buildPlanningSnapshot(cwd: string): PlanningSnapshot { milestoneArchiveStatus: buildMilestoneArchiveStatusField(cwd), planningRootFiles: buildPlanningRootFilesField(cwd), allPhaseDirNames: buildAllPhaseDirNamesField(paths.phases), + archivedPhaseTokens: buildArchivedPhaseTokensField(paths.planning), + currentMilestoneRoadmapPhaseIds: buildCurrentMilestoneRoadmapPhaseIdsField(cwd, paths.roadmap), }; }