From 2980f0ec485fc7f1ef481e9d0ff7760650c2a184 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 21 Apr 2026 10:41:35 -0400 Subject: [PATCH] fix(sdk): stripShippedMilestones handles inline SHIPPED headings; getMilestoneInfo prefers STATE.md (#2508) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(sdk): stripShippedMilestones handles inline SHIPPED headings; getMilestoneInfo prefers STATE.md Fixes two compounding bugs: - #2496: stripShippedMilestones only stripped
blocks, ignoring '## Heading — ✅ SHIPPED ...' inline markers. Shipped milestone sections were leaking into downstream parsers. - #2495: getMilestoneInfo checked STATE.md frontmatter only as a last-resort fallback, so it returned the first heading match (often a leaked shipped milestone) rather than the current milestone. Moved STATE.md check to priority 1, consistent with extractCurrentMilestone. Closes #2495 Closes #2496 Co-Authored-By: Claude Sonnet 4.6 * fix(roadmap): handle ### SHIPPED headings and STATE.md version-only case Two follow-up fixes from CodeRabbit review of #2508: 1. stripShippedMilestones only split on ## boundaries; ### headings marked ✅ SHIPPED were not stripped, leaking into fallback parsers. Expanded the split/filter regex to #{2,3} to align with extractCurrentMilestone. 2. getMilestoneInfo's early-return on parseMilestoneFromState discarded the real milestone name from ROADMAP.md when STATE.md had only `milestone:` (no `milestone_name:`), returning the placeholder name 'milestone'. Now only short-circuits when STATE.md provides a real name; otherwise falls through to ROADMAP for the name while using stateVersion to override the version in every ROADMAP-derived return path. Tests: +2 new cases (### SHIPPED heading, version-only STATE.md). Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- sdk/src/query/roadmap.test.ts | 117 ++++++++++++++++++++++++++++++++++ sdk/src/query/roadmap.ts | 35 ++++++---- 2 files changed, 140 insertions(+), 12 deletions(-) diff --git a/sdk/src/query/roadmap.test.ts b/sdk/src/query/roadmap.test.ts index e6f785828..dafcabdf0 100644 --- a/sdk/src/query/roadmap.test.ts +++ b/sdk/src/query/roadmap.test.ts @@ -100,6 +100,63 @@ describe('stripShippedMilestones', () => { it('returns content unchanged when no details blocks', () => { expect(stripShippedMilestones('no details here')).toBe('no details here'); }); + + // Bug #2496: inline ✅ SHIPPED heading sections must be stripped + it('strips ## heading sections marked ✅ SHIPPED', () => { + const content = [ + '## Milestone v1.0: MVP — ✅ SHIPPED 2026-01-15', + '', + 'Phase 1, Phase 2', + '', + '## Milestone v2.0: Current', + '', + 'Phase 3', + ].join('\n'); + const stripped = stripShippedMilestones(content); + expect(stripped).not.toContain('MVP'); + expect(stripped).not.toContain('v1.0'); + expect(stripped).toContain('v2.0'); + expect(stripped).toContain('Current'); + }); + + it('strips multiple inline SHIPPED sections and leaves non-shipped content', () => { + const content = [ + '## Milestone v1.0: Alpha — ✅ SHIPPED 2026-01-01', + '', + 'Old content', + '', + '## Milestone v1.5: Beta — ✅ SHIPPED 2026-02-01', + '', + 'More old content', + '', + '## Milestone v2.0: Gamma', + '', + 'Current content', + ].join('\n'); + const stripped = stripShippedMilestones(content); + expect(stripped).not.toContain('Alpha'); + expect(stripped).not.toContain('Beta'); + expect(stripped).toContain('Gamma'); + expect(stripped).toContain('Current content'); + }); + + // Bug #2508 follow-up: ### headings must be stripped too + it('strips ### heading sections marked ✅ SHIPPED', () => { + const content = [ + '### Milestone v1.0: MVP — ✅ SHIPPED 2026-01-15', + '', + 'Phase 1, Phase 2', + '', + '### Milestone v2.0: Current', + '', + 'Phase 3', + ].join('\n'); + const stripped = stripShippedMilestones(content); + expect(stripped).not.toContain('MVP'); + expect(stripped).not.toContain('v1.0'); + expect(stripped).toContain('v2.0'); + expect(stripped).toContain('Current'); + }); }); // ─── getMilestoneInfo ───────────────────────────────────────────────────── @@ -158,6 +215,66 @@ describe('getMilestoneInfo', () => { expect(info.version).toBe('v1.0'); expect(info.name).toBe('milestone'); }); + + // Bug #2495: STATE.md must take priority over ROADMAP heading matching + it('prefers STATE.md milestone over ROADMAP heading match', async () => { + const roadmap = [ + '## Milestone v1.0: Shipped — ✅ SHIPPED 2026-01-01', + '', + 'Phase 1', + '', + '## Milestone v2.0: Current Active', + '', + 'Phase 2', + ].join('\n'); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + await writeFile( + join(tmpDir, '.planning', 'STATE.md'), + '---\nmilestone: v2.0\nmilestone_name: Current Active\n---\n', + ); + const info = await getMilestoneInfo(tmpDir); + expect(info.version).toBe('v2.0'); + expect(info.name).toBe('Current Active'); + }); + + // Bug #2508 follow-up: STATE.md has milestone version but no milestone_name — + // should use ROADMAP for the real name, still prefer STATE.md for version. + it('uses ROADMAP name when STATE.md has milestone version but no milestone_name', async () => { + const roadmap = [ + '## Milestone v2.0: Real Name From Roadmap', + '', + 'Phase 2', + ].join('\n'); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + await writeFile( + join(tmpDir, '.planning', 'STATE.md'), + '---\nmilestone: v2.0\n---\n', // no milestone_name + ); + const info = await getMilestoneInfo(tmpDir); + expect(info.version).toBe('v2.0'); + expect(info.name).toBe('Real Name From Roadmap'); + }); + + it('returns correct milestone from STATE.md even when ROADMAP inline-SHIPPED stripping would fix it', async () => { + // ROADMAP with an unstripped shipped milestone heading (pre-fix state) + const roadmap = [ + '## Milestone v1.0: Old — ✅ SHIPPED 2026-01-01', + '', + 'Old phases', + '', + '## Milestone v2.0: New', + '', + 'New phases', + ].join('\n'); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + await writeFile( + join(tmpDir, '.planning', 'STATE.md'), + '---\nmilestone: v2.0\nmilestone_name: New\n---\n', + ); + const info = await getMilestoneInfo(tmpDir); + expect(info.version).toBe('v2.0'); + expect(info.name).toBe('New'); + }); }); // ─── extractCurrentMilestone ────────────────────────────────────────────── diff --git a/sdk/src/query/roadmap.ts b/sdk/src/query/roadmap.ts index 1c151096f..c18e5e0a5 100644 --- a/sdk/src/query/roadmap.ts +++ b/sdk/src/query/roadmap.ts @@ -50,7 +50,13 @@ interface PhaseSection { * Port of stripShippedMilestones from core.cjs line 1082-1084. */ export function stripShippedMilestones(content: string): string { - return content.replace(/
[\s\S]*?<\/details>/gi, ''); + // Pattern 1:
...
blocks (explicit collapse) + let result = content.replace(/
[\s\S]*?<\/details>/gi, ''); + // Pattern 2: inline milestone headings marked as shipped. + // Keep aligned with heading levels accepted by extractCurrentMilestone() (## and ###). + const sections = result.split(/(?=^#{2,3}\s)/m); + result = sections.filter(s => !/^#{2,3}\s[^\n]*✅\s*SHIPPED\b/im.test(s)).join(''); + return result; } /** @@ -84,25 +90,35 @@ async function parseMilestoneFromState(projectDir: string): Promise<{ version: s */ export async function getMilestoneInfo(projectDir: string): Promise<{ version: string; name: string }> { try { + // Priority 1: STATE.md frontmatter (authoritative for version; name only when real) + const fromState = await parseMilestoneFromState(projectDir); + const stateVersion = fromState?.version ?? null; + const stateName = fromState && fromState.name !== 'milestone' ? fromState.name : null; + if (stateVersion && stateName) { + return { version: stateVersion, name: stateName }; + } + // STATE.md has a version but no real name — fall through to ROADMAP for the name, + // then override the version with the authoritative STATE.md value. + const roadmap = await readFile(planningPaths(projectDir).roadmap, 'utf-8'); // List-format: construction / blocked (legacy emoji) const barricadeMatch = roadmap.match(/🚧\s*\*\*v(\d+(?:\.\d+)+)\s+([^*]+)\*\*/); if (barricadeMatch) { - return { version: 'v' + barricadeMatch[1], name: barricadeMatch[2].trim() }; + return { version: stateVersion ?? 'v' + barricadeMatch[1], name: barricadeMatch[2].trim() }; } // List-format: in flight / active (GSD ROADMAP template uses 🟡 for current milestone) const inFlightMatch = roadmap.match(/🟡\s*\*\*v(\d+(?:\.\d+)+)\s+([^*]+)\*\*/); if (inFlightMatch) { - return { version: 'v' + inFlightMatch[1], name: inFlightMatch[2].trim() }; + return { version: stateVersion ?? 'v' + inFlightMatch[1], name: inFlightMatch[2].trim() }; } // Heading-format — strip shipped
blocks first const cleaned = stripShippedMilestones(roadmap); const headingMatch = cleaned.match(/##\s+.*v(\d+(?:\.\d+)+)[:\s]+([^\n(]+)/); if (headingMatch) { - return { version: 'v' + headingMatch[1], name: headingMatch[2].trim() }; + return { version: stateVersion ?? 'v' + headingMatch[1], name: headingMatch[2].trim() }; } // Milestone bullet list (## Milestones … ## Phases): use last **vX.Y Title** — typically the current row @@ -110,21 +126,16 @@ export async function getMilestoneInfo(projectDir: string): Promise<{ version: s const boldMatches = [...beforePhases.matchAll(/\*\*v(\d+(?:\.\d+)+)\s+([^*]+)\*\*/g)]; if (boldMatches.length > 0) { const last = boldMatches[boldMatches.length - 1]; - return { version: 'v' + last[1], name: last[2].trim() }; - } - - const fromState = await parseMilestoneFromState(projectDir); - if (fromState) { - return fromState; + return { version: stateVersion ?? 'v' + last[1], name: last[2].trim() }; } const allBare = [...cleaned.matchAll(/\bv(\d+(?:\.\d+)+)\b/g)]; if (allBare.length > 0) { const lastBare = allBare[allBare.length - 1]; - return { version: lastBare[0], name: 'milestone' }; + return { version: stateVersion ?? lastBare[0], name: 'milestone' }; } - return { version: 'v1.0', name: 'milestone' }; + return { version: stateVersion ?? 'v1.0', name: 'milestone' }; } catch { const fromState = await parseMilestoneFromState(projectDir); if (fromState) return fromState;