* 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 <details> 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 <sim@local>
This commit is contained in:
5
.changeset/vivid-lynx-romp.md
Normal file
5
.changeset/vivid-lynx-romp.md
Normal file
@@ -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)
|
||||
@@ -91,6 +91,21 @@ function stripShippedMilestones(content: string): string {
|
||||
return stripTaggedBlocks(content, 'details');
|
||||
}
|
||||
|
||||
/**
|
||||
* #3982: strip only <details> blocks whose <summary> 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(/<details\b[^>]*>[\s\S]*?<\/details>/gi, (block) => {
|
||||
const summaryMatch = block.match(/<summary[^>]*>([^<]*)<\/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 <details>
|
||||
// blocks BELOW the active milestone, whose titles live in <summary> 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 <details> (#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
|
||||
// <summary> 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
|
||||
|
||||
@@ -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.',
|
||||
'',
|
||||
'<details>',
|
||||
'<summary>✅ v0.2 Second Milestone (Phases 10-12) — ARCHIVED</summary>', '',
|
||||
'- [ ] **Phase 10: Never Finished** - was left unchecked when v0.2 closed.',
|
||||
'- [x] **Phase 11: Done Thing** - completed.',
|
||||
'- [x] **Phase 12: Other Done Thing** - completed.',
|
||||
'',
|
||||
'</details>', '',
|
||||
].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', '---', '',
|
||||
'<objective>Do the first thing.</objective>', '',
|
||||
].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}`);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 <summary> TAG, not a
|
||||
// heading, so no milestone-shaped heading bounds the section walk and the
|
||||
// raw currentSection used to swallow the whole <details> 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.',
|
||||
'',
|
||||
'<details>',
|
||||
'<summary>✅ v0.2 Second Milestone (Phases 10-12) — ARCHIVED</summary>',
|
||||
'',
|
||||
'- [ ] **Phase 10: Never Finished** - was left unchecked when v0.2 closed.',
|
||||
'- [x] **Phase 11: Done Thing** - completed.',
|
||||
'',
|
||||
'</details>',
|
||||
'',
|
||||
'<details>',
|
||||
'<summary>✅ v0.1 First Milestone (Phases 1-2) — ARCHIVED</summary>',
|
||||
'',
|
||||
'- [x] **Phase 1: Done** - completed.',
|
||||
'',
|
||||
'</details>',
|
||||
].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 <details> (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.',
|
||||
'',
|
||||
'<details>',
|
||||
'<summary>Deferred scope for v0.3</summary>', '',
|
||||
'- [ ] **Phase 21: Second Thing** - deferred.',
|
||||
'- [ ] **Phase 22: Third Thing** - deferred.',
|
||||
'',
|
||||
'</details>', '',
|
||||
].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 = [
|
||||
|
||||
Reference in New Issue
Block a user