From 808df9110c46defc95c286f16abda68c5e3814cc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 8 Jun 2026 22:52:19 -0400 Subject: [PATCH] fix(#892): parse checklist-style roadmap phases in validate/verify (#908) buildRoadmapPhaseVariants() only matched heading-style phases (## Phase N:), silently skipping the supported checklist format (- [x] **Phase N: name**). This caused W007 false-positives for every on-disk phase dir when the project uses a checklist ROADMAP. Fix adds a second regex pass (mirroring the existing buildNotStartedPhaseVariants() approach). Also refactors the duplicate inline heading-only regex in cmdValidateConsistency() to delegate to buildRoadmapPhaseVariants() (DRY). Regression test in tests/bug-892-validate-checklist-roadmap-phases.test.cjs covers both paths. Closes #892 Co-authored-by: Claude Opus 4.8 --- .../892-validate-checklist-roadmap-phases.md | 5 + scripts/lint-test-file-count.allowlist.json | 8 + src/validate.cts | 9 + src/verify.cts | 26 +- ...validate-checklist-roadmap-phases.test.cjs | 290 ++++++++++++++++++ 5 files changed, 316 insertions(+), 22 deletions(-) create mode 100644 .changeset/892-validate-checklist-roadmap-phases.md create mode 100644 tests/bug-892-validate-checklist-roadmap-phases.test.cjs diff --git a/.changeset/892-validate-checklist-roadmap-phases.md b/.changeset/892-validate-checklist-roadmap-phases.md new file mode 100644 index 000000000..0b1c62292 --- /dev/null +++ b/.changeset/892-validate-checklist-roadmap-phases.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 893 +--- +**`validate health` and `validate consistency` no longer emit false-positive W007 warnings for projects using checklist-style ROADMAP.md phases.** `buildRoadmapPhaseVariants()` in `src/validate.cts` previously used only a heading-style regex (`## Phase N: name`), silently ignoring the supported checklist format (`- [x] **Phase N: name**`). This caused every on-disk phase directory to trigger W007 ("exists on disk but not in ROADMAP.md") when the project's ROADMAP used checklist-only notation. The fix adds a second regex pass mirroring the existing `buildNotStartedPhaseVariants()` approach. Additionally, `cmdValidateConsistency()` in `src/verify.cts` had a duplicate inline heading-only regex with the same gap — refactored to delegate to `buildRoadmapPhaseVariants()` (DRY). (#892) diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index 47aed538e..52ddc1270 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -131,6 +131,14 @@ "install.test.cjs" ], "issue": "TBD" + }, + "validate": { + "files": [ + "bug-3129-validate-commit-git-bypass.test.cjs", + "bug-892-validate-checklist-roadmap-phases.test.cjs", + "validate-context.test.cjs" + ], + "issue": "892" } } } diff --git a/src/validate.cts b/src/validate.cts index 0a6c30ece..c7b727d07 100644 --- a/src/validate.cts +++ b/src/validate.cts @@ -100,6 +100,15 @@ export function buildRoadmapPhaseVariants(roadmapContent: string): RoadmapPhaseV roadmapPhases.add(m[1]); for (const variant of phaseVariants(m[1])) roadmapPhaseVariants.add(variant); } + // Also matches checklist-style entries (checked or unchecked): + // - [x] **Phase 01: name** - [X] **Phase 2-01: name** - [ ] **Phase 3: name** + // This is a supported ROADMAP format (parallel to buildNotStartedPhaseVariants). + const checklistPattern = /-\s*\[[ xX]\]\s*\*{0,2}Phase\s+([\w][\w.-]*)\s*:/gi; + let cm: RegExpExecArray | null; + while ((cm = checklistPattern.exec(roadmapContent)) !== null) { + roadmapPhases.add(cm[1]); + for (const variant of phaseVariants(cm[1])) roadmapPhaseVariants.add(variant); + } return { roadmapPhases, roadmapPhaseVariants }; } diff --git a/src/verify.cts b/src/verify.cts index 8aa3d6016..b68a1b814 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -641,21 +641,8 @@ function cmdValidateConsistency(cwd: string, raw: boolean): void { const roadmapContentRaw = fs.readFileSync(roadmapPath, 'utf-8'); const roadmapContent = extractCurrentMilestone(roadmapContentRaw, cwd); - const roadmapPhases = new Set(); - const phasePattern = - /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)\s*:/gi; - let m: RegExpExecArray | null; - while ((m = phasePattern.exec(roadmapContent)) !== null) { - roadmapPhases.add(m[1]); - } - - const fullRoadmapPhases = new Set(); - const fullPhasePattern = - /#{2,4}\s*(?:\[[^\]]+\]\s*)?Phase\s+([\w][\w.-]*)\s*:/gi; - let fm: RegExpExecArray | null; - while ((fm = fullPhasePattern.exec(roadmapContentRaw)) !== null) { - fullRoadmapPhases.add(fm[1]); - } + const { roadmapPhases } = buildRoadmapPhaseVariants(roadmapContent); + const { roadmapPhaseVariants: fullRoadmapPhaseVariants } = buildRoadmapPhaseVariants(roadmapContentRaw); const diskPhases = collectDiskPhases(planBase); @@ -666,13 +653,8 @@ function cmdValidateConsistency(cwd: string, raw: boolean): void { } for (const p of diskPhases) { - const normalized = normalizePhaseName(p); - const unpadded = String(parseInt(p, 10)); - if ( - !fullRoadmapPhases.has(p) && - !fullRoadmapPhases.has(normalized) && - !fullRoadmapPhases.has(unpadded) - ) { + const variants = phaseVariants(p); + if (![...variants].some((v) => fullRoadmapPhaseVariants.has(v))) { warnings.push(`Phase ${p} exists on disk but not in ROADMAP.md`); } } diff --git a/tests/bug-892-validate-checklist-roadmap-phases.test.cjs b/tests/bug-892-validate-checklist-roadmap-phases.test.cjs new file mode 100644 index 000000000..4577d4419 --- /dev/null +++ b/tests/bug-892-validate-checklist-roadmap-phases.test.cjs @@ -0,0 +1,290 @@ +/** + * Regression test for #892: checklist-style roadmap phases (`- [x] **Phase NN: name**`) + * were silently skipped by `buildRoadmapPhaseVariants()`, causing W007 false positives + * on every on-disk phase dir when the project uses the checklist ROADMAP format. + * + * Covers: + * A. Unit-level: `buildRoadmapPhaseVariants()` in validate.cts recognises both + * checked (`- [x]`) and unchecked (`- [ ]`) checklist items. + * B. Integration: `validate health` emits NO W007 for a checklist-only roadmap + * whose phase dirs all appear in the checklist. + * C. Integration: `validate consistency` emits NO "exists on disk but not in + * ROADMAP.md" warning for the same checklist-only roadmap. + * + * Requirements: BUG-892 + */ +'use strict'; + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const { + buildRoadmapPhaseVariants, +} = require('../gsd-core/bin/lib/validate.cjs'); + +// ─── A: Unit-level ──────────────────────────────────────────────────────────── + +describe('buildRoadmapPhaseVariants — checklist format support (#892)', () => { + test('matches checked checklist item `- [x] **Phase 01: name**`', () => { + const content = [ + '# Roadmap', + '', + '- [x] **Phase 01: infrastructure-hardening**', + ].join('\n'); + + const { roadmapPhases } = buildRoadmapPhaseVariants(content); + assert.ok( + roadmapPhases.has('01') || roadmapPhases.has('1'), + `roadmapPhases should contain a variant of "01", got: ${JSON.stringify([...roadmapPhases])}` + ); + }); + + test('matches checked checklist item with uppercase X `- [X] **Phase 02: foo**`', () => { + const content = [ + '# Roadmap', + '', + '- [X] **Phase 02: feature-work**', + ].join('\n'); + + const { roadmapPhases } = buildRoadmapPhaseVariants(content); + assert.ok( + roadmapPhases.has('02') || roadmapPhases.has('2'), + `roadmapPhases should contain a variant of "02", got: ${JSON.stringify([...roadmapPhases])}` + ); + }); + + test('matches unchecked checklist item `- [ ] **Phase 03: name**`', () => { + // unchecked items are also phases — they just have not been started + const content = [ + '# Roadmap', + '', + '- [ ] **Phase 03: future-work**', + ].join('\n'); + + const { roadmapPhases } = buildRoadmapPhaseVariants(content); + assert.ok( + roadmapPhases.has('03') || roadmapPhases.has('3'), + `roadmapPhases should contain a variant of "03", got: ${JSON.stringify([...roadmapPhases])}` + ); + }); + + test('collects all phases from a pure checklist roadmap (no ## headings)', () => { + const content = [ + '# Roadmap', + '', + '- [x] **Phase 01: alpha**', + '- [x] **Phase 02: beta**', + '- [ ] **Phase 03: gamma**', + ].join('\n'); + + const { roadmapPhases } = buildRoadmapPhaseVariants(content); + const has = (id) => roadmapPhases.has(id) || roadmapPhases.has(id.replace(/^0+/, '')) || roadmapPhases.has(String(parseInt(id, 10)).padStart(2, '0')); + assert.ok(has('01'), 'should contain phase 01'); + assert.ok(has('02'), 'should contain phase 02'); + assert.ok(has('03'), 'should contain phase 03'); + }); + + test('populates roadmapPhaseVariants with padding-normalised forms for checklist phases', () => { + const content = [ + '# Roadmap', + '', + '- [x] **Phase 01: something**', + ].join('\n'); + + const { roadmapPhaseVariants } = buildRoadmapPhaseVariants(content); + // phaseVariants() adds both '1' and '01' forms + assert.ok( + roadmapPhaseVariants.has('1') || roadmapPhaseVariants.has('01'), + `roadmapPhaseVariants should contain at least one padding form, got: ${JSON.stringify([...roadmapPhaseVariants])}` + ); + }); + + test('mixed roadmap (headings + checklist) collects phases from both styles', () => { + const content = [ + '# Roadmap', + '', + '## Phase 1: heading-style', + '', + '- [x] **Phase 02: checklist-style**', + ].join('\n'); + + const { roadmapPhases } = buildRoadmapPhaseVariants(content); + assert.ok( + roadmapPhases.has('1') || roadmapPhases.has('01'), + 'should contain heading-style phase 1' + ); + assert.ok( + roadmapPhases.has('02') || roadmapPhases.has('2'), + 'should contain checklist-style phase 02' + ); + }); +}); + +// ─── B: validate health — no W007 for checklist-only roadmaps ──────────────── + +describe('validate health — checklist-style roadmap phases must not emit W007 (#892)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('no W007 when phase dirs match checked checklist entries in ROADMAP.md', () => { + // Write PROJECT.md, STATE.md, config.json, and a checklist-only ROADMAP.md + fs.writeFileSync( + path.join(tmpDir, '.planning', 'PROJECT.md'), + '# Project\n\n## What This Is\n\nTest.\n\n## Core Value\n\nValue.\n\n## Requirements\n\nRequirements.\n' + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Session State\n\n## Current Position\n\nPhase 1 in progress.\n' + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ model_profile: 'balanced', commit_docs: true }, null, 2) + ); + + // Checklist-only ROADMAP: no ## Phase headings, only checklist items + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '- [x] **Phase 01: infrastructure-hardening**', + '- [x] **Phase 02: feature-work**', + '', + ].join('\n') + ); + + // Create matching phase directories on disk + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-infrastructure-hardening'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '02-feature-work'), { recursive: true }); + + const result = runGsdTools('validate health', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const w007s = output.warnings.filter((w) => w.code === 'W007'); + assert.strictEqual( + w007s.length, + 0, + `W007 must not fire for phases whose dirs are listed in a checklist-style ROADMAP.md, got: ${JSON.stringify(w007s)}` + ); + }); + + test('W007 still fires when a phase dir is genuinely absent from a checklist roadmap', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'PROJECT.md'), + '# Project\n\n## What This Is\n\nTest.\n\n## Core Value\n\nValue.\n\n## Requirements\n\nRequirements.\n' + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Session State\n\n## Current Position\n\nPhase 1 in progress.\n' + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ model_profile: 'balanced', commit_docs: true }, null, 2) + ); + + // ROADMAP only lists phase 01, not phase 99 + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '- [x] **Phase 01: known-phase**', + '', + ].join('\n') + ); + + // Phase 01 dir is present but also an orphan phase 99 that is NOT in ROADMAP + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-known-phase'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '99-orphan'), { recursive: true }); + + const result = runGsdTools('validate health', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + const w007s = output.warnings.filter((w) => w.code === 'W007'); + assert.ok( + w007s.length > 0, + `W007 must still fire for a phase dir genuinely not listed in ROADMAP.md, got warnings: ${JSON.stringify(output.warnings)}` + ); + // Should only flag phase 99, not phase 01 + assert.ok( + w007s.some((w) => w.message.includes('99')), + `W007 should reference orphan phase 99, got: ${JSON.stringify(w007s)}` + ); + assert.ok( + !w007s.some((w) => w.message.includes('01') || w.message.includes('1')), + `W007 must NOT flag phase 01 which is in the checklist, got: ${JSON.stringify(w007s)}` + ); + }); +}); + +// ─── C: validate consistency — no false positive for checklist-only roadmaps ─ + +describe('validate consistency — checklist-style roadmap phases must not emit false warnings (#892)', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('no "exists on disk but not in ROADMAP.md" warning for checklist-matched phases', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'PROJECT.md'), + '# Project\n\n## What This Is\n\nTest.\n\n## Core Value\n\nValue.\n\n## Requirements\n\nRequirements.\n' + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '# Session State\n\n## Current Position\n\nPhase 1 in progress.\n' + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ model_profile: 'balanced', commit_docs: true }, null, 2) + ); + + // Checklist-only ROADMAP + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '- [x] **Phase 01: infrastructure-hardening**', + '- [x] **Phase 02: feature-work**', + '', + ].join('\n') + ); + + // Create matching phase directories + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-infrastructure-hardening'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '02-feature-work'), { recursive: true }); + + const result = runGsdTools('validate consistency', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + // No "exists on disk but not in ROADMAP" warnings for checklist-listed phases + const diskNotInRoadmapWarnings = (output.warnings || []).filter( + (w) => typeof w === 'string' + ? w.includes('exists on disk but not in ROADMAP') + : (w.message || '').includes('exists on disk but not in ROADMAP') + ); + assert.strictEqual( + diskNotInRoadmapWarnings.length, + 0, + `No "exists on disk but not in ROADMAP.md" warnings should fire for checklist-listed phases, got: ${JSON.stringify(diskNotInRoadmapWarnings)}` + ); + }); +});