From 383c2f6b348b9f944a0882e348faf0ad6cf16b3e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 2 Sep 2026 02:33:54 -0400 Subject: [PATCH] fix(#3982): strip closed-milestone details from the current window (#4177) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#3982): archived details must not leak into the current-milestone window Parser-level regression (newest-first layout, two archived details blocks) plus the issue's end-to-end phase.complete fixture: completing phase 20 must advance to 21, never backwards into the archived range. * fix(#3982): strip closed-milestone details from the current window The heading-located window stripped
archives from the preamble but not from currentSection; on newest-first roadmaps the archived titles sit in summary tags rather than headings, so the section walk reached end-of-document and the window swallowed every collapsed archive below the active milestone. The strip is gated on isClosedMilestoneHeading over each block's summary — the issue's prescribed narrow form — so the active milestone's own collapsed blocks (#1341) survive instead of trading this bug for the phase_count: 0 class (#557/#2947). * chore(#3982): backfill changeset pr number --------- Co-authored-by: sim --- .changeset/vivid-lynx-romp.md | 5 +++ src/roadmap-parser.cts | 27 +++++++++++++- tests/phase.test.cjs | 52 +++++++++++++++++++++++++++ tests/roadmap-parser.test.cjs | 66 +++++++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 .changeset/vivid-lynx-romp.md diff --git a/.changeset/vivid-lynx-romp.md b/.changeset/vivid-lynx-romp.md new file mode 100644 index 000000000..b464cd9d3 --- /dev/null +++ b/.changeset/vivid-lynx-romp.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4177 +--- +**Completing a phase no longer jumps backwards into an archived milestone** — on newest-milestone-first roadmaps the collapsed archive below the active milestone leaked into the current-milestone window, so an unchecked phase from a closed milestone could win the next-phase scan. (#3982) diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 8f71eae5d..c7daca942 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -91,6 +91,21 @@ function stripShippedMilestones(content: string): string { return stripTaggedBlocks(content, 'details'); } +/** + * #3982: strip only
blocks whose marks a CLOSED milestone + * (ARCHIVED/SHIPPED/✅/… without an active marker) — the narrow form of + * stripShippedMilestones the current-milestone window needs. A blanket strip + * would delete the ACTIVE milestone's own collapsed blocks (#1341) and + * reproduce the phase_count: 0 class of #557/#2947. + */ +function stripClosedMilestoneDetails(content: string): string { + return content.replace(/]*>[\s\S]*?<\/details>/gi, (block) => { + const summaryMatch = block.match(/]*>([^<]*)<\/summary>/i); + if (!summaryMatch) return block; + return isClosedMilestoneHeading(summaryMatch[1]) ? '' : block; + }); +} + /** * #2562: is the milestone `version` marked SHIPPED by the ROADMAP itself? * @@ -1177,7 +1192,17 @@ function extractCurrentMilestoneScoped(content: string, cwd?: string, ws?: strin ? earliestMilestoneIndex : firstMatch.index; const beforeMilestones = content.slice(0, preambleCutoff); - const currentSection = content.slice(sectionStart, sectionEnd); + // #3982: newest-first roadmaps collapse their archives into
+ // blocks BELOW the active milestone, whose titles live in tags — + // not headings — so no milestone-shaped heading bounds the section walk and + // the raw slice runs to end-of-document. Strip CLOSED milestone details + // blocks here so the window never feeds archived phases to the + // lowest-outstanding scan. Deliberately NOT a blanket strip: the active + // milestone may legitimately hold its own collapsed
(#1341), and + // removing that would trade this bug for the phase_count: 0 class (#557). + // Gated on the same isClosedMilestoneHeading the file already applies to + // lines, per the issue's prescribed narrower fix. + const currentSection = stripClosedMilestoneDetails(content.slice(sectionStart, sectionEnd)); // Multi-milestone roadmaps split each added milestone across two version-bearing // headings: a `## Phases` checklist subsection (early) and a dedicated diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index fe4996cf8..ea37b9167 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -14919,3 +14919,55 @@ describe('#3701 phase complete — next_phase follows roadmap order, not disk', ); }); }); + +// ─── #3982: phase.complete must not pick an archived details-block phase ───── + +describe('bug #3982: archived details leak into lowest-outstanding scan', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3982-')); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '20-first-thing'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), [ + '---', 'milestone: v0.3', 'current_phase: 20', '---', '', + ].join('\n')); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), [ + '# Roadmap', '', + '### 🚧 v0.3 — Third Milestone (Phases 20-22) — ACTIVE', '', + '- [x] **Phase 20: First Thing** - does the first thing.', + '- [ ] **Phase 21: Second Thing** - does the second thing.', + '- [ ] **Phase 22: Third Thing** - does the third thing.', + '', + '
', + '✅ v0.2 Second Milestone (Phases 10-12) — ARCHIVED', '', + '- [ ] **Phase 10: Never Finished** - was left unchecked when v0.2 closed.', + '- [x] **Phase 11: Done Thing** - completed.', + '- [x] **Phase 12: Other Done Thing** - completed.', + '', + '
', '', + ].join('\n')); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '20-first-thing'); + fs.writeFileSync(path.join(phaseDir, '20-01-PLAN.md'), [ + '---', 'phase: 20-first-thing', 'plan: 01', '---', '', + 'Do the first thing.', '', + ].join('\n')); + fs.writeFileSync(path.join(phaseDir, '20-01-SUMMARY.md'), [ + '---', 'phase: 20-first-thing', 'plan: 01', 'status: complete', '---', '', + 'Done.', '', + ].join('\n')); + }); + + afterEach(() => { cleanup(tmpDir); }); + + test('phase complete 20 advances to 21, not the archived range', () => { + const out = runPhaseComplete(tmpDir, { phase: '20', tolerateExit: true }); + const payload = JSON.parse(out.slice(out.indexOf('{'))); + assert.strictEqual(payload.next_phase, '21', + 'completing phase 20 must advance to the real next phase 21 (#3982)'); + assert.notStrictEqual(payload.next_phase, '10', + 'an archived milestone\'s unchecked phase must never win the lowest-outstanding scan (#3982)'); + const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); + assert.ok(!/current_phase:\s*10(\s|$)/m.test(state), + `STATE.md current_phase must not jump backwards into the archived range; got: ${state}`); + }); +}); diff --git a/tests/roadmap-parser.test.cjs b/tests/roadmap-parser.test.cjs index dc7989056..f3ab36fc0 100644 --- a/tests/roadmap-parser.test.cjs +++ b/tests/roadmap-parser.test.cjs @@ -111,6 +111,72 @@ describe('roadmap-parser: extractCurrentMilestone', () => { assert.ok(result.includes('v2.0'), 'version heading preserved'); }); + test('newest-first layout: archived details below the active milestone do not leak into the window (#3982)', () => { + // The archived milestone's title lives in the TAG, not a + // heading, so no milestone-shaped heading bounds the section walk and the + // raw currentSection used to swallow the whole
block — feeding + // archived phases to phase.complete's lowest-outstanding scan. + writeState(tmpDir, { milestone: 'v0.3' }); + const content = [ + '# Roadmap', + '', + '### 🚧 v0.3 — Third Milestone (Phases 20-22) — ACTIVE', + '', + '- [x] **Phase 20: First Thing** - does the first thing.', + '- [ ] **Phase 21: Second Thing** - does the second thing.', + '- [ ] **Phase 22: Third Thing** - does the third thing.', + '', + '
', + '✅ v0.2 Second Milestone (Phases 10-12) — ARCHIVED', + '', + '- [ ] **Phase 10: Never Finished** - was left unchecked when v0.2 closed.', + '- [x] **Phase 11: Done Thing** - completed.', + '', + '
', + '', + '
', + '✅ v0.1 First Milestone (Phases 1-2) — ARCHIVED', + '', + '- [x] **Phase 1: Done** - completed.', + '', + '
', + ].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 21'), 'the real next phase stays in the window'); + assert.ok(result.includes('Phase 22'), 'the last current phase stays in the window'); + assert.ok(!result.includes('Phase 10'), 'an archived milestone\'s unchecked phase must not leak into the current window (#3982)'); + assert.ok(!result.includes('Never Finished'), 'archived milestone content must not leak (#3982)'); + assert.ok(!result.includes('Phase 1: Done'), 'a second archived details block must not leak either'); + }); + + test('active milestone own collapsed details are preserved by the closed-only strip (#3982)', () => { + // The issue's adversarial fixture: the ACTIVE milestone holds its own + // collapsed
(deferred scope). A blanket strip would delete + // phases 21/22 and reproduce the phase_count: 0 class (#557/#2947). + writeState(tmpDir, { milestone: 'v0.3' }); + const content = [ + '# Roadmap', '', + '### 🚧 v0.3 — Third Milestone (Phases 20-22) — ACTIVE', '', + '- [x] **Phase 20: First Thing** - done.', + '', + '
', + 'Deferred scope for v0.3', '', + '- [ ] **Phase 21: Second Thing** - deferred.', + '- [ ] **Phase 22: Third Thing** - deferred.', + '', + '
', '', + ].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 21'), 'the active milestone\'s own collapsed phases must survive (#3982)'); + assert.ok(result.includes('Phase 22'), 'the active milestone\'s own collapsed phases must survive (#3982)'); + }); + test('reads milestone from STATE.md and extracts that section', () => { writeState(tmpDir, { milestone: 'v2.0' }); const content = [