diff --git a/.changeset/1580-999-sentinel-milestone-roadmap.md b/.changeset/1580-999-sentinel-milestone-roadmap.md new file mode 100644 index 000000000..2ddf49e5f --- /dev/null +++ b/.changeset/1580-999-sentinel-milestone-roadmap.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1691 +--- +`milestone complete` and `roadmap analyze` now exclude the Phase 0 / Phase 999 backlog sentinels. A milestone whose only directory-less ROADMAP heading is a backlog sentinel can be completed without `--force`, and `roadmap analyze` no longer counts the sentinel in `phase_count` or routes `next_phase` into it. Completes the `^999` exclusion #1445 added to the progress denominators. diff --git a/src/milestone.cts b/src/milestone.cts index f15c9baad..9bf81cc05 100644 --- a/src/milestone.cts +++ b/src/milestone.cts @@ -184,6 +184,13 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo })(); while ((pm = phasePattern.exec(scopedContent)) !== null) { const phaseNum = pm[1]; + // Phase 0 (pre-milestone) and Phase 999 (backlog) are sentinels, not + // real phases — they legitimately have no directory and must not block + // milestone completion. Mirrors the engine-wide sentinel convention + // (phase-id getMilestoneFromPhaseId, roadmap-command-router SENTINELS, + // the #1445 /^999/ progress filters). (#1580) + const major = parseInt(phaseNum, 10); + if (major === 0 || major === 999) continue; const normalized = normalizePhaseName(phaseNum); // A phase has disk_status: 'no_directory' when no phase directory // with a matching token exists on disk. Use the same phaseTokenMatches diff --git a/src/roadmap.cts b/src/roadmap.cts index 1c442c502..510edf647 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -318,6 +318,16 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { }> = []; let match: RegExpExecArray | null; + // Phase 0 (pre-milestone) and Phase 999 (backlog) are sentinels, not real + // phases. They legitimately have no directory and must never be surfaced as + // current/next phase or counted in phase_count. Mirrors the engine-wide + // sentinel convention (phase-id getMilestoneFromPhaseId, roadmap-command-router + // SENTINELS, the #1445 /^999/ progress filters). (#1580) + const isSentinelPhase = (num: string): boolean => { + const major = parseInt(num, 10); + return major === 0 || major === 999; + }; + // Build phase directory lookup once (O(1) readdir instead of O(N) per phase) const _phaseDirNames = (() => { try { @@ -329,6 +339,7 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { while ((match = phasePattern.exec(content)) !== null) { const phaseNum = match[1]; + if (isSentinelPhase(phaseNum)) continue; const phaseName = match[2].replace(/\(INSERTED\)/i, '').trim(); // Extract goal from the section @@ -437,7 +448,7 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void { checklistPhases.add(checklistMatch[1]); } const detailPhases = new Set(phases.map(p => p.number)); - const missingDetails = [...checklistPhases].filter(p => !detailPhases.has(p)); + const missingDetails = [...checklistPhases].filter(p => !detailPhases.has(p) && !isSentinelPhase(p)); const result = { milestones, diff --git a/tests/bug-978-milestone-complete-force.test.cjs b/tests/bug-978-milestone-complete-force.test.cjs index 8cd74c883..0ff7d5612 100644 --- a/tests/bug-978-milestone-complete-force.test.cjs +++ b/tests/bug-978-milestone-complete-force.test.cjs @@ -19,10 +19,13 @@ const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); * Build a fixture where the guard will fire: * - STATE.md has `milestone: ` so the guard's version-match check is * satisfied. - * - ROADMAP.md lists a `### Phase 999.1: Backlog Work` heading for that - * milestone, but there is NO on-disk phase directory for it. + * - ROADMAP.md lists a `### Phase 2: Real Work` heading for that milestone, but + * there is NO on-disk phase directory for it. * * This guarantees "unstarted phase" detection without touching any real phases. + * NOTE: the unstarted phase must be a REAL phase number — Phase 0 and Phase 999 + * are backlog/pre-milestone sentinels that are intentionally excluded from this + * guard (#1580), so they would not fire it. */ function makeGuardFixture(tmpDir, version) { // STATE.md with frontmatter milestone field matching the version @@ -32,10 +35,10 @@ function makeGuardFixture(tmpDir, version) { ); // ROADMAP.md — the heading must include the version so getMilestonePhaseFilter - // does not return missingExplicitVersion. Phase 999.1 has no on-disk dir. + // does not return missingExplicitVersion. Phase 2 has no on-disk dir. fs.writeFileSync( path.join(tmpDir, '.planning', 'ROADMAP.md'), - `# Roadmap ${version}\n\n### Phase 999.1: Backlog Work\n**Goal:** Not started\n`, + `# Roadmap ${version}\n\n### Phase 2: Real Work\n**Goal:** Not started\n`, ); } @@ -80,7 +83,7 @@ describe('bug-978: milestone complete --force overrides unstarted-phase guard', const output = JSON.parse(result.output); assert.strictEqual(output.version, 'v1.0'); - // Milestone entry should have been created even though phase 999.1 has no dir + // Milestone entry should have been created even though phase 2 has no dir assert.ok( fs.existsSync(path.join(tmpDir, '.planning', 'MILESTONES.md')), 'MILESTONES.md should have been created', diff --git a/tests/fix-1445-999x-backlog-excluded-from-total-phases.test.cjs b/tests/fix-1445-999x-backlog-excluded-from-total-phases.test.cjs index 1490db251..00b4d169a 100644 --- a/tests/fix-1445-999x-backlog-excluded-from-total-phases.test.cjs +++ b/tests/fix-1445-999x-backlog-excluded-from-total-phases.test.cjs @@ -16,6 +16,14 @@ * Scenarios: * A. deriveProgressFromRoadmap with a progress table containing a 999.x row. * B. state json total_phases via extractCurrentMilestone / roadmapPhaseCount. + * + * Follow-up #1580: the same `^999` (and Phase 0) sentinel exclusion was missing + * in two more code paths — `milestone complete`'s unstarted-phase guard + * (src/milestone.cts) and `roadmap analyze`'s next_phase routing + phase_count + * (src/roadmap.cts). Scenarios C and D below cover those. + * + * C. milestone complete is NOT blocked by a Phase 999 backlog heading. + * D. roadmap analyze never routes next_phase to 999 / never counts it. */ const { describe, test, beforeEach, afterEach } = require('node:test'); @@ -172,3 +180,116 @@ describe('bug #1445 — state json excludes 999.x phase headings from total_phas ); }); }); + +// ─── Scenario C: milestone complete not blocked by a 999 backlog heading ───── + +describe('fix #1580 — milestone complete ignores the 999 backlog sentinel', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject('fix-1580-mc-'); + const planning = path.join(tmpDir, '.planning'); + // One real, on-disk phase + a directory-less Phase 999 backlog heading. + fs.writeFileSync( + path.join(planning, 'ROADMAP.md'), + [ + '# Roadmap v1.0', + '## v1.0 Milestone', + '## Phases', + '- [x] **Phase 1: Foundation**', + '## Phase Details', + '### Phase 1: Foundation', + '**Goal:** build it', + '### Phase 999: Backlog / Someday', + '**Goal:** deferred, never executed', + ].join('\n'), + 'utf-8', + ); + fs.writeFileSync( + path.join(planning, 'STATE.md'), + `---\nmilestone: v1.0\n---\n# State\n\n**Status:** In progress\n**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n`, + 'utf-8', + ); + const dir = path.join(planning, 'phases', '01-foundation'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'PLAN.md'), '# Plan\n', 'utf-8'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('completes WITHOUT --force despite a Phase 999 backlog heading', () => { + const result = runGsdTools( + ['milestone', 'complete', 'v1.0', '--name', 'Regression'], + tmpDir, + ); + assert.ok( + result.success, + `milestone complete must not be blocked by the 999 sentinel; got error: ${result.error}`, + ); + assert.ok( + !/Cannot mark milestone complete/.test(result.error || ''), + `the unstarted-phase guard must not fire on Phase 999. Got: ${result.error}`, + ); + }); +}); + +// ─── Scenario D: roadmap analyze never routes/ counts the 999 sentinel ──────── + +describe('fix #1580 — roadmap analyze excludes the 999 backlog sentinel', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject('fix-1580-ra-'); + const planning = path.join(tmpDir, '.planning'); + fs.writeFileSync( + path.join(planning, 'ROADMAP.md'), + [ + '# Roadmap v1.0', + '## v1.0 Milestone', + '## Phases', + '- [x] **Phase 1: Foundation**', + '## Phase Details', + '### Phase 1: Foundation', + '**Goal:** build it', + '### Phase 999: Backlog / Someday', + '**Goal:** deferred, never executed', + ].join('\n'), + 'utf-8', + ); + fs.writeFileSync( + path.join(planning, 'STATE.md'), + `---\nmilestone: v1.0\n---\n# State\n`, + 'utf-8', + ); + const dir = path.join(planning, 'phases', '01-foundation'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'PLAN.md'), '# Plan\n', 'utf-8'); + fs.writeFileSync(path.join(dir, 'SUMMARY.md'), '# Summary\n', 'utf-8'); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('next_phase is never 999 and phase_count excludes the sentinel', () => { + const result = runGsdTools(['roadmap', 'analyze', '--raw'], tmpDir); + assert.ok(result.success, `roadmap analyze failed: ${result.error}`); + const analysis = JSON.parse(result.output); + assert.notEqual( + String(analysis.next_phase), + '999', + `next_phase must never route to the 999 backlog sentinel. Got ${analysis.next_phase}`, + ); + assert.equal( + analysis.phase_count, + 1, + `phase_count must exclude the 999 sentinel (expected 1). Got ${analysis.phase_count}`, + ); + assert.ok( + !(analysis.phases || []).some(p => String(p.number) === '999'), + 'the phases array must not include the 999 backlog sentinel', + ); + }); +});