diff --git a/.changeset/soft-jade-quartz.md b/.changeset/soft-jade-quartz.md new file mode 100644 index 000000000..4be7e3bce --- /dev/null +++ b/.changeset/soft-jade-quartz.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3371 +--- +**`gsd-tools validate health` and `validate consistency` no longer flag sentinel phase directories (999.x backlog/interim, 0.x drafts)** — the disk-vs-roadmap comparison now applies the `isSentinelPhaseId` guard that the phase commands already had. Sentinel ids are defined as never-on-roadmap, so a `999-interim` directory previously produced a permanent spurious W007 ("Phase 999 exists on disk but not in ROADMAP.md", advice to add it to the roadmap or delete it — both wrong) and a spurious "Gap in phase numbering: N → 999". Real (non-sentinel) orphans and genuine numbering gaps still warn. (#3225) diff --git a/src/verify.cts b/src/verify.cts index 011b9fcf3..a98ef09ce 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -46,7 +46,7 @@ import configLoaderMod = require('./config-loader.cjs'); const { loadConfig, CONFIG_DEFAULTS } = configLoaderMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { normalizePhaseName, phaseTokenMatches, escapeRegex, getMilestoneFromPhaseId, OPTIONAL_PHASE_TAG_SOURCE, PHASE_NUMBER_TOKEN_SOURCE, extractPhaseToken, comparePhaseNum } = phaseIdMod; +const { normalizePhaseName, phaseTokenMatches, escapeRegex, getMilestoneFromPhaseId, OPTIONAL_PHASE_TAG_SOURCE, PHASE_NUMBER_TOKEN_SOURCE, extractPhaseToken, comparePhaseNum, isSentinelPhaseId } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal } = phaseLocatorMod; @@ -1442,12 +1442,17 @@ function cmdValidateConsistency(cwd: string, raw: boolean): void { const diskPhases = collectDiskPhases(planBase); for (const p of roadmapPhases) { + // #3225: sentinel phase ids are never-on-roadmap by convention. + if (isSentinelPhaseId(p)) continue; if (!diskPhases.has(p) && !diskPhases.has(normalizePhaseName(p))) { warnings.push(`Phase ${p} in ROADMAP.md but no directory on disk`); } } for (const p of diskPhases) { + // #3225: a sentinel dir on disk (999-interim, 0-drafts) is defined as + // never-on-roadmap; it must not warn here (same guard as cmdValidateHealth). + if (isSentinelPhaseId(p)) continue; const variants = phaseVariants(p); if (![...variants].some((v) => fullRoadmapPhaseVariants.has(v))) { warnings.push(`Phase ${p} exists on disk but not in ROADMAP.md`); @@ -1457,7 +1462,10 @@ function cmdValidateConsistency(cwd: string, raw: boolean): void { const config = loadConfig(cwd); if (config.phase_naming !== 'custom') { const integerPhases = [...diskPhases] - .filter((p) => !p.includes('.')) + // #3225: exclude sentinel phase ids (999.x/0.x) — they are never part of the + // sequential numbering, so a 999-interim dir must not produce a spurious + // "Gap in phase numbering: N → 999". + .filter((p) => !p.includes('.') && !isSentinelPhaseId(p)) .map((p) => parseInt(p, 10)) .sort((a, b) => a - b); @@ -1990,6 +1998,9 @@ function cmdValidateHealth( 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)); if (!existsOnDisk) { @@ -2005,6 +2016,11 @@ function cmdValidateHealth( } for (const p of activeDiskPhases) { + // #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))) { addIssue( diff --git a/tests/verify-health.test.cjs b/tests/verify-health.test.cjs index b09f492a4..26c5fa612 100644 --- a/tests/verify-health.test.cjs +++ b/tests/verify-health.test.cjs @@ -977,6 +977,41 @@ describe('validate health command', () => { assert.strictEqual(output.errors.length, 0, 'should have no errors'); assert.ok(output.warnings.length > 0, 'should have warnings'); }); + + // #3225: sentinel phase dirs (999.x backlog/interim, 0.x drafts) are defined as + // never-on-roadmap (SENTINEL_RANGES=[0,999]). The W006/W007 disk↔roadmap loops + // never had the isSentinelPhaseId guard that phase.cts has, so every sentinel dir + // produced a spurious W007 with wrong fix advice ("Add to roadmap or remove + // directory"). Sentinels must be excluded; a real (non-sentinel) orphan must still warn. + test('#3225: sentinel phase dirs (999/0) do not trigger W007; real orphans still do', () => { + writeMinimalRoadmap(tmpDir, ['1']); + writeMinimalStateMd(tmpDir); + writeValidConfigJson(tmpDir); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-a'), { recursive: true }); + // Sentinel dirs — never-on-roadmap by convention. + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '999-interim'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '0-drafts'), { recursive: true }); + // A real orphan (non-sentinel) that SHOULD still trigger W007. + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '77-orphan'), { recursive: true }); + + const result = runGsdTools('validate health', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + + const sentinelW007s = output.warnings.filter( + w => w.code === 'W007' && /\b(0|999)\b/.test(w.message) + ); + assert.strictEqual( + sentinelW007s.length, 0, + `sentinel phase dirs must not trigger W007; got: ${JSON.stringify(sentinelW007s)}` + ); + + // Negative space: the non-sentinel orphan must still be flagged. + assert.ok( + output.warnings.some(w => w.code === 'W007' && /77\b/.test(w.message)), + `expected W007 for the real orphan 77; got: ${JSON.stringify(output.warnings.filter(w => w.code === 'W007'))}` + ); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index d6e33bd89..0e45d74a0 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -102,6 +102,46 @@ describe('validate consistency command', () => { ); }); + test('#3225: sentinel phase dirs (999/0) do not warn; real orphans still do', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n### Phase 1: A\n` + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-a'), { recursive: true }); + // Sentinel dirs — never-on-roadmap by convention (SENTINEL_RANGES=[0,999]). + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '999-interim'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '0-drafts'), { recursive: true }); + // A real orphan (non-sentinel) that SHOULD still warn. + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '02-orphan'), { recursive: true }); + + const result = runGsdTools('validate consistency', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + + const sentinelWarnings = output.warnings.filter( + w => w.includes('disk but not in ROADMAP') && /\b(0|999)\b/.test(w) + ); + assert.strictEqual( + sentinelWarnings.length, 0, + `sentinel phase dirs must not warn; got: ${JSON.stringify(sentinelWarnings)}` + ); + // Negative space: the real orphan must still warn. + assert.ok( + output.warnings.some(w => w.includes('disk but not in ROADMAP') && /02\b/.test(w)), + `expected a warning for the real orphan 02; got: ${JSON.stringify(output.warnings)}` + ); + // #3225 (review finding): a sentinel dir must NOT produce a spurious + // "Gap in phase numbering: N → 999" either (the gap check builds its integer + // sequence from diskPhases and would otherwise include 999). + const sentinelGaps = output.warnings.filter( + w => w.includes('Gap in phase numbering') && /999\b/.test(w) + ); + assert.strictEqual( + sentinelGaps.length, 0, + `sentinel 999 must not create a spurious numbering gap; got: ${JSON.stringify(sentinelGaps)}` + ); + }); + test('warns about gaps in phase numbering', () => { fs.writeFileSync( path.join(tmpDir, '.planning', 'ROADMAP.md'),