golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#2947): preserve preamble phase details when milestone section has none (#3084)

Browse Source 
* test(#2947): milestone anchor must prefer heading with Phase details

Row 1 of the test matrix: the failing-first regression test. When the
phase-listing heading (## Phases) is NOT version-bearing but a later
version-bearing progress heading (### v9.0 phase progress) exists,
extractCurrentMilestone latches onto the progress heading and silently drops
the phases (phase_count: 0, exit 0). Five cases: the regression, the
version-bearing control (must keep working), the no-phase-details fallback,
the closed-vs-open preference, and an end-to-end roadmap.analyze check.

Reproduced locally against built lib + confirmed by maintainer triage (trek-e).
The one-word control (## Phases -> ## v9.0 Phases) restores phase_count: 2.

* fix(#2947): preserve preamble phase details when the milestone section has none

Root cause was one layer deeper than the issue title: the anchor selection
(selected = first non-closed version-bearing heading) is fine — the real
drop happens in the preamble strip. When the phase list lives under a
non-version-bearing ## Phases heading (the shipped greenfield template's own
shape) and the selected version-bearing heading is a LATER progress/notes
sub-heading with no ### Phase N: details of its own, the preamble strip
removed every phase-detail heading from the pre-milestone region (intended
to avoid duplication with a Phase Details section that does not exist here)
— silently dropping all phases (phase_count: 0, exit 0, empty stderr).

Fix: only strip preamble ### Phase N: headings when the selected milestone
section (currentSection) actually contains its own phase details. When it
does not, the preamble phases ARE this milestone's phases and must be
preserved. Falls back to today's behavior (strip) whenever the selected
section has phase details, so multi-milestone roadmaps with a dedicated
Phase Details section (#730) are unaffected.

Surgical: one conditional on the existing strip, no signature change, no
change to computeSectionEnd or the #730 Phase Details append. Blast radius
CRITICAL (84 upstream symbols) — the change is gated on currentSection's
content so every existing roadmap that currently resolves phases correctly
keeps doing so byte-identically.

* chore(#2947): add changeset fragment

* chore(#2947): backfill changeset PR number 3084

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-05 13:10:52 -04:00
committed by GitHub
parent 5719efbc6b
commit 2bc53baa03
3 changed files with 173 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/quick-birds-jump.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3084
---
**`roadmap.analyze` no longer silently drops phases when the phase-listing heading isn't version-bearing** — if the phase list lives under a plain `## Phases` heading (the shipped greenfield template's own shape) and a later version-bearing progress/notes heading exists, the milestone scope previously latched onto the later heading and stripped every `### Phase N:` detail from the preamble, returning `phase_count: 0` with exit 0 and empty stderr. Phase details in the preamble are now preserved when the selected milestone section has none of its own. (#2947)

12
src/roadmap-parser.cts
View File

@@ -235,9 +235,19 @@ function extractCurrentMilestone(content: string, cwd?: string, ws?: string | nu
);
}
// #2947: the preamble strip removes `### Phase N:` detail headings from the
// pre-milestone region so they don't duplicate the ones inside the selected
// milestone section. But when the phase list lives under a non-version-bearing
// `## Phases` heading (the shipped greenfield template's own shape) and the
// selected version-bearing heading is a LATER progress/notes sub-heading with
// NO phase details of its own, stripping the preamble phases silently drops
// every phase (phase_count: 0, exit 0). Only strip preamble phase details when
// the selected milestone section actually contains its own — otherwise the
// preamble phases ARE this milestone's phases and must be preserved.
const currentSectionHasPhaseDetails = /^#{2,4}\s*Phase\s+\S/im.test(currentSection);
const preamble = stripTaggedBlocks(beforeMilestones, 'details')
// #1729: `(?:\s*\([^)\n]{0,200}\))?` tolerates a pre-colon ( ) tag (literal mirror of OPTIONAL_PHASE_TAG_SOURCE).
.replace(/^#{2,4}\s*Phase\s+[\w][\w.-]*(?:\s*\([^)\n]{0,200}\))?\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim, '')
.replace(currentSectionHasPhaseDetails ? /^#{2,4}\s*Phase\s+[\w][\w.-]*(?:\s*\([^)\n]{0,200}\))?\s*:[^\n]*(?:\n(?!#{1,6}\s)[^\n]*)*\n?/gim : /$/, '')
.replace(/^#{1,4}\s*Phase Details\b[^\n]*\n?/gim, '');
return detailsSection

158
tests/roadmap-parser.test.cjs
View File

@@ -23,7 +23,7 @@ const fs = require('node:fs');
const path = require('node:path');
const roadmapParser = require('../gsd-core/bin/lib/roadmap-parser.cjs');
const { createTempProject, cleanup } = require('./helpers.cjs');
const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs');
const {
stripShippedMilestones,
@@ -201,6 +201,162 @@ describe('roadmap-parser: extractCurrentMilestone', () => {
assert.ok(result.includes('real goal'), 'phase 1 content included');
assert.ok(result.includes('Also Real'), 'phase 2 content also included');
});
// ─── #2947: milestone anchor must prefer the heading whose section contains
// Phase details, not just the first version-bearing heading anywhere. ────────
test('#2947 — prefers the heading whose section contains Phase details over a later version-bearing progress heading', () => {
// The shipped greenfield template's `## Phases` is NOT version-bearing,
// but a later `### v9.0 phase progress` heading (under `## Progress`) is.
// The anchor must not latch onto the progress heading and drop the phases.
writeState(tmpDir, { milestone: 'v9.0' });
const content = [
'# ROADMAP',
'',
'## Milestones',
'',
'- 🚧 **v9.0 Test Milestone** — Phases 1-2 (in progress)',
'',
'## Phases',
'',
'### Phase 1: Alpha',
'',
'**Goal:** do alpha',
'',
'### Phase 2: Beta',
'',
'**Goal:** do beta',
'',
'## Progress',
'',
'### v9.0 phase progress',
'',
'| Phase | Status |',
'|-------|--------|',
'| 1 | Planned |',
'| 2 | Planned |',
].join('\n');
writeRoadmap(tmpDir, content);
const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
const result = extractCurrentMilestone(roadmap, tmpDir);
// The Phase detail headings must be inside the extracted section.
assert.ok(result.includes('Phase 1: Alpha'), 'Phase 1 detail must be in scope (got dropped — #2947)');
assert.ok(result.includes('Phase 2: Beta'), 'Phase 2 detail must be in scope (got dropped — #2947)');
// The progress heading should NOT be the anchor (it has no phase details).
assert.ok(!result.startsWith('### v9.0 phase progress'), 'progress heading must not be the anchor');
});
test('#2947 — version-bearing phase-listing heading still resolves (control, no regression)', () => {
// The one-word control from the issue: rename `## Phases` → `## v9.0 Phases`.
// This already works today and must keep working after the fix.
writeState(tmpDir, { milestone: 'v9.0' });
const content = [
'# ROADMAP',
'',
'## v9.0 Phases',
'',
'### Phase 1: Alpha',
'',
'**Goal:** do alpha',
'',
'### Phase 2: Beta',
'',
'**Goal:** do beta',
'',
'## Progress',
'',
'### v9.0 phase progress',
'',
'| Phase | Status |',
].join('\n');
writeRoadmap(tmpDir, content);
const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
const result = extractCurrentMilestone(roadmap, tmpDir);
assert.ok(result.includes('Phase 1: Alpha'), 'control: Phase 1 in scope');
assert.ok(result.includes('Phase 2: Beta'), 'control: Phase 2 in scope');
});
test('#2947 — falls back to first non-closed when no candidate section has Phase details', () => {
// No `### Phase N:` details anywhere — the fix's fallback must preserve
// today's behavior (no crash, returns a section).
writeState(tmpDir, { milestone: 'v9.0' });
const content = [
'# ROADMAP',
'',
'## v9.0 Milestone',
'',
'Some prose, no phase detail headings.',
'',
'### v9.0 notes',
'',
'More prose.',
].join('\n');
writeRoadmap(tmpDir, content);
const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
const result = extractCurrentMilestone(roadmap, tmpDir);
assert.ok(typeof result === 'string', 'fallback returns a string section without throwing');
assert.ok(result.includes('v9.0'), 'fallback still includes the milestone content');
});
test('#2947 — closed milestone heading is not preferred over an open one with phase details', () => {
// A closed (✅) version-bearing heading must not win over an open one
// whose section contains the phase details.
writeState(tmpDir, { milestone: 'v2.0' });
const content = [
'# ROADMAP',
'',
'## ✅ v1.0 Shipped',
'',
'### Phase 1: Old',
'',
'## 🚧 v2.0 Current',
'',
'### Phase 2: New',
'',
'**Goal:** new work',
].join('\n');
writeRoadmap(tmpDir, content);
const roadmap = fs.readFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'utf-8');
const result = extractCurrentMilestone(roadmap, tmpDir);
assert.ok(result.includes('Phase 2: New'), 'open milestone phase selected');
assert.ok(!result.includes('Phase 1: Old'), 'closed milestone phase excluded');
});
test('#2947 — roadmap.analyze on the issue fixture reports phase_count 2 (end-to-end)', () => {
writeState(tmpDir, { milestone: 'v9.0', gsd_state_version: '1.0' });
const content = [
'# ROADMAP',
'',
'## Milestones',
'',
'- 🚧 **v9.0 Test Milestone** — Phases 1-2 (in progress)',
'',
'## Phases',
'',
'### Phase 1: Alpha',
'',
'**Goal:** do alpha',
'',
'### Phase 2: Beta',
'',
'**Goal:** do beta',
'',
'## Progress',
'',
'### v9.0 phase progress',
'',
'| Phase | Status |',
'|-------|--------|',
'| 1 | Planned |',
'| 2 | Planned |',
].join('\n');
writeRoadmap(tmpDir, content);
const result = runGsdTools(['query', 'roadmap.analyze', '--raw'], tmpDir);
assert.ok(result.success, `roadmap.analyze should succeed; got: ${result.error}`);
const payload = JSON.parse(result.output);
assert.strictEqual(payload.phase_count, 2, `expected phase_count 2, got ${payload.phase_count} (phases dropped — #2947)`);
});
});
// ─── replaceInCurrentMilestone ────────────────────────────────────────────────