fix(sdk): stripShippedMilestones handles inline SHIPPED headings; getMilestoneInfo prefers STATE.md (#2508)

* fix(sdk): stripShippedMilestones handles inline SHIPPED headings; getMilestoneInfo prefers STATE.md

Fixes two compounding bugs:

- #2496: stripShippedMilestones only stripped <details> 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-04-21 10:41:35 -04:00
committed by GitHub
parent 8789211038
commit 2980f0ec48
2 changed files with 140 additions and 12 deletions

View File

@@ -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 ──────────────────────────────────────────────

View File

@@ -50,7 +50,13 @@ interface PhaseSection {
* Port of stripShippedMilestones from core.cjs line 1082-1084.
*/
export function stripShippedMilestones(content: string): string {
return content.replace(/<details>[\s\S]*?<\/details>/gi, '');
// Pattern 1: <details>...</details> blocks (explicit collapse)
let result = content.replace(/<details>[\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 <details> 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;