* fix(#1580): exclude 0/999 sentinels from milestone-complete guard and roadmap analyze Closed #1445 added the `^999` backlog-sentinel exclusion to the progress denominators but missed two other resolvers, leaving two user-facing failures live on a milestone whose only directory-less ROADMAP heading is a backlog sentinel: (A) `milestone complete` was blocked by the unstarted-phase guard in src/milestone.cts — it flagged `### Phase 999: Backlog` as an unstarted phase and refused to close a fully-shipped milestone without --force. (B) `roadmap analyze` (src/roadmap.cts) counted the sentinel in phase_count and routed `next_phase` straight into Phase 999. Both now skip Phase 0 (pre-milestone) and Phase 999 (backlog) sentinels, mirroring the engine-wide convention (phase-id getMilestoneFromPhaseId, roadmap-command-router SENTINELS, the #1445 progress filters). Symptom (C) (state.cts total_phases) was already fixed inline by #1445/#1514 and is out of scope here. Regression coverage folded into tests/fix-1445-*.test.cjs (scenarios C and D); updated tests/bug-978 fixture to use a real unstarted phase (Phase 2) instead of a 999 sentinel, since the sentinel is now correctly excluded from that guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(#1580): add changeset for 0/999 sentinel exclusion fix Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
b2a7a3980d
commit
6414249d25
@@ -19,10 +19,13 @@ const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||
* Build a fixture where the guard will fire:
|
||||
* - STATE.md has `milestone: <version>` 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',
|
||||
|
||||
@@ -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',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user