diff --git a/sdk/src/query/roadmap.test.ts b/sdk/src/query/roadmap.test.ts index 63bbc3a2f..7ff5e3bcf 100644 --- a/sdk/src/query/roadmap.test.ts +++ b/sdk/src/query/roadmap.test.ts @@ -495,6 +495,133 @@ describe('extractCurrentMilestone', () => { expect(result).toMatch(/^##\s+v0\.9 Local-First Bus/m); }); + // ─── Bug #2641 (review hardening): substring-version trap ─── + it('bug-2641: v0.1 must not substring-match v0.10 …', async () => { + // The fallback regex anchors on `escapedVersion` inside `` text. + // Without a non-version-character lookahead, `v0.1` matches inside `v0.10`, + // and the function returns the v0.10 block's body as the active milestone + // — confidently-wrong content (worse than the pre-fix fall-through, which + // returned known-incomplete content). The synthesized `## v0.10 …` heading + // would then mask the bug from downstream debugging. Lock the boundary. + const roadmap = `# Roadmap + +
+v0.10 Future Milestone — Phase Details + +### Phase 7: Wrong Phase +**Goal:** This is from v0.10, not v0.1. +
+ +
+v0.1 Active — Phase Details + +### Phase 1: Right Phase +**Goal:** This is the active milestone. +
+`; + const state = `---\nmilestone: v0.1\n---\n# State\n`; + await writeFile(join(tmpDir, '.planning', 'STATE.md'), state); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + + const result = await extractCurrentMilestone(roadmap, tmpDir); + + expect(result).toContain('### Phase 1: Right Phase'); + expect(result).toContain('This is the active milestone'); + expect(result).not.toContain('Phase 7: Wrong Phase'); + expect(result).not.toContain('This is from v0.10'); + }); + + // ─── Bug #2641 (review hardening): nested
guard ─── + it('bug-2641: nested
falls through (does not silently truncate)', async () => { + // The lazy [\s\S]*?
terminates on the FIRST
, which + // is the inner closer when nesting is present. Without a guard, the + // function returns truncated body and silently loses everything after the + // inner . Detect nesting and fall through to the existing + // stripShippedMilestones path so the failure mode is loud (no match) not + // silent (truncated content). + const roadmap = `# Roadmap + +
+v0.9 Local-First Bus — Phase Details + +### Phase 1: Library +
+Implementation notes +Detail +
+ +### Phase 2: Polish — would be silently lost without the guard +**Goal:** Add polish. +
+`; + const state = `---\nmilestone: v0.9\n---\n# State\n`; + await writeFile(join(tmpDir, '.planning', 'STATE.md'), state); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + + const result = await extractCurrentMilestone(roadmap, tmpDir); + + // The critical contract: must NOT return a synthesized `## v0.9` heading + // anchored to truncated body. The truncation case (without the nested- + // guard) would emit `## v0.9 Local-First Bus\n\n### Phase 1: Library\n + //
Implementation notes\nDetail` and silently + // lose Phase 2 — confidently-wrong content. Falling through to + // stripShippedMilestones() may leak unrelated content but doesn't claim + // to be the active milestone. Loud failure > silent truncation. + expect(result).not.toMatch(/^##\s+v0\.9 Local-First Bus/m); + // The Phase 1 detail block (which sits between the outer
open + // and the inner
) must not appear under a v0.9 heading. + expect(result).not.toMatch(/##\s+v0\.9[\s\S]*Phase 1: Library/); + }); + + // ─── Bug #2641 (review hardening): empty
body guard ─── + it('bug-2641: empty
body falls through (no phantom milestone)', async () => { + //
v0.9
with no body would synthesize + // `## v0.9\n` — a phantom milestone with zero phases. roadmapAnalyze would + // then return {phases: []} with no error signal. Treat as no-match. + const roadmap = `# Roadmap + +
+v0.9 Empty +
+`; + const state = `---\nmilestone: v0.9\n---\n# State\n`; + await writeFile(join(tmpDir, '.planning', 'STATE.md'), state); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + + const result = await extractCurrentMilestone(roadmap, tmpDir); + + // Must not synthesize a phantom heading + expect(result).not.toMatch(/^##\s+v0\.9/m); + }); + + // ─── Bug #2641 (review hardening): inline HTML in + leading # ─── + it('bug-2641: tolerates inline HTML in and strips it from synthesized heading', async () => { + // GitHub-rendered summaries commonly contain inline tags like + // (active) or v0.9. The summary capture must allow + // them through and the synthesized `## ` heading must strip the tags so + // the result is clean markdown (no `## ...`). + const roadmap = `# Roadmap + +
+v0.9 Local-First Bus (active) + +### Phase 3: Polish +**Goal:** Add polish. +
+`; + const state = `---\nmilestone: v0.9\n---\n# State\n`; + await writeFile(join(tmpDir, '.planning', 'STATE.md'), state); + await writeFile(join(tmpDir, '.planning', 'ROADMAP.md'), roadmap); + + const result = await extractCurrentMilestone(roadmap, tmpDir); + + expect(result).toContain('### Phase 3: Polish'); + expect(result).toMatch(/^##\s+v0\.9 Local-First Bus\s+\(active\)/m); + // Tags must be stripped from the synthesized heading + expect(result).not.toMatch(/^##.*/m); + expect(result).not.toMatch(/^##.*/m); + }); + // ─── Bug #2422: same-version sub-heading truncation ─────────────────── it('bug-2422: does not truncate at same-version sub-heading (## v2.0 Phase Details)', async () => { const roadmapWithDetails = `# ROADMAP diff --git a/sdk/src/query/roadmap.ts b/sdk/src/query/roadmap.ts index f4d3c0898..cce4f0967 100644 --- a/sdk/src/query/roadmap.ts +++ b/sdk/src/query/roadmap.ts @@ -146,7 +146,21 @@ export async function getMilestoneInfo(projectDir: string, workstream?: string): /** * Extract the current milestone section from ROADMAP.md. * - * Port of extractCurrentMilestone from core.cjs lines 1102-1170. + * Two anchoring strategies, tried in order: + * 1. Markdown heading containing the active version (`^#{1,3}\s+.*vX.Y…`). + * 2. `
vX.Y……
` block (the GitHub-friendly + * collapse pattern; see #2641). When this fallback fires, the captured + * `` text is synthesized as a `##` heading prepended to the + * returned slice so downstream consumers that scan for milestone headings + * (e.g. the `data.milestones` loop in `roadmapAnalyze`) still see an + * active-milestone anchor. + * + * If neither strategy matches the active version, falls through to + * `stripShippedMilestones(content)`. + * + * Originally ported from core.cjs lines 1102-1170; the TS implementation has + * since diverged (Backlog-leak fix #2422, phase-vX.Y truncation fix #2619, + * fenced-code-block tracking #2787, `
` fallback #2641). * * @param content - Full ROADMAP.md content * @param projectDir - Working directory for reading STATE.md @@ -207,26 +221,47 @@ export async function extractCurrentMilestone(content: string, projectDir: strin // active ROADMAP. The init.phase-op safety guard then misfires and can // route phase lookups into archived milestones. // - // ]*> tolerates attributes like
and - //
. ]*> tolerates the same on the - // tag. The lazy [\s\S]*? terminates on the first
; - // nested
inside the active milestone are not expected and would - // mis-anchor (acceptable; FAMP-style ROADMAPs do not nest, and any project - // that does will fall through to the existing stripShippedMilestones path - // with no regression vs. today's behavior). + // Regex anatomy: + // ]*> tolerate attributes (e.g.
) + // \s*]*> tolerate attributes on + // ((?:(?!).)*? non-greedy summary capture; tolerates + // ${escapedVersion} inline HTML in the summary text + // (?![\d.]) non-version-character lookahead — prevents + // `v0.1` from substring-matching `v0.10` + // (?:(?!
).)*) + //
end of summary + // ([\s\S]*?)
lazy body capture to the FIRST
// - // We capture the text and prepend it as a normalized `##` - // milestone heading on the returned slice. This keeps downstream consumers - // that scan for `##` milestone headings (e.g. roadmapAnalyze's - // data.milestones loop later in this file) producing a meaningful entry - // for the active milestone instead of seeing an unanchored body. + // Contract: any consumer that scans the returned slice for milestone + // headings (e.g. /##\s*.*vX.Y/) sees the active milestone's anchor. We + // synthesize that heading from the captured text rather than + // returning the body alone. + // + // Hardening guards: + // - Nested
: the lazy quantifier truncates at the inner + //
, silently losing trailing phases. Detect and fall through + // to stripShippedMilestones() instead of returning truncated content. + // - Empty body: a
block with no body would synthesize a heading + // with nothing under it. Treat as no-match. + // - Summary sanitization: strip inline HTML (e.g. active) and + // leading `#` tokens before promoting to a `##` heading, so the result + // is a single well-formed markdown heading. const detailsPattern = new RegExp( - `]*>\\s*]*>([^<]*${escapedVersion}[^<]*)
([\\s\\S]*?)
`, + `]*>\\s*]*>` + + `((?:(?!
).)*?${escapedVersion}(?![\\d.])(?:(?!).)*)` + + `([\\s\\S]*?)`, 'i' ); const detailsMatch = content.match(detailsPattern); - if (detailsMatch) { - const summary = detailsMatch[1].trim(); + if ( + detailsMatch && + detailsMatch[2].trim() && // empty-body guard + !detailsMatch[2].includes(' guard + ) { + const summary = detailsMatch[1] + .replace(/<[^>]+>/g, '') // strip inline HTML + .replace(/^#+\s*/, '') // strip leading `#` + .trim(); const body = detailsMatch[2]; return `## ${summary}\n${body}`; }