diff --git a/.changeset/fierce-pumas-dance.md b/.changeset/fierce-pumas-dance.md new file mode 100644 index 000000000..db5686c22 --- /dev/null +++ b/.changeset/fierce-pumas-dance.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4578 +--- +**`roadmap analyze` no longer mints a phantom phase from a mid-line mention** — a sentence, blockquote, or inline-code-span reference to a `### Phase N:`-shaped heading anywhere in the ROADMAP was previously counted as a real phase, inflating `phase_count` and able to collide on a phase number with a real heading nearby. The phase-heading extraction is now anchored to line start, matching this repo's other heading parsers. diff --git a/src/roadmap.cts b/src/roadmap.cts index de59dae3e..d3ff3e77f 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -465,8 +465,17 @@ function collectAnalyzePhases( // #3036: widen the id capture to accept non-numeric-leading ids (e.g. B7, P0.3-2) // that get-phase/execute-phase already resolve. An optional leading letter prefix // ([A-Za-z]?) covers letter-prefixed ids without breaking numeric-leading ones. + // #4478: line-anchored (`^ {0,3}`, `/m`) — unanchored, `#{2,4}` matched a + // `### Phase N:`-shaped mention ANYWHERE `exec()`'s scan reached: mid-sentence + // prose, inside a blockquote, inside an inline code span (backtick-quoted on + // the same line, not a fenced block `tokenizeHeadings` would exclude). Any + // such line minted a phantom phase entry, inflating phase_count and able to + // collide on a phase NUMBER with a real heading nearby. `{0,3}` leading + // spaces mirrors `tokenizeHeadings`'s own CommonMark ATX-heading tolerance + // (src/markdown-sectionizer.cts:453) so a legitimately-indented heading that + // matched before this fix still matches after it. // phase-id-owner: uses the [.-] (dot-or-dash) separator variant, not the canonical dot-only token; a swap to PHASE_NUMBER_TOKEN_SOURCE would drop hyphenated phase-id matches. - const phasePattern = new RegExp(`#{2,4}\\s*${phaseHeadingPrefixSrcFor(PHASE_HEADING_BASELINE.ANY_BRACKET, convention, true)}([A-Za-z]?\\d+[A-Z]?(?:[.-]\\d+)*)(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gi'); + const phasePattern = new RegExp(`^ {0,3}#{2,4}\\s*${phaseHeadingPrefixSrcFor(PHASE_HEADING_BASELINE.ANY_BRACKET, convention, true)}([A-Za-z]?\\d+[A-Z]?(?:[.-]\\d+)*)(?:\\s*\\([^)\\n]{0,200}\\))?\\s*:\\s*([^\\n]+)`, 'gim'); // The capturing intro inserts the bracket id at group 1 only under the // bracket convention; the token and name shift by the same offset. const G = convention === 'bracket' ? 1 : 0; @@ -488,7 +497,13 @@ function collectAnalyzePhases( // #3691: `\d` → `\d[\d.]*` so decimal phase headings (e.g. `### Phase 02.3:`) are // recognised as section boundaries. #3036: `[A-Za-z]?\d` so non-numeric-leading ids // (e.g. B7) are also recognised. - const nextHeader = restOfContent.match(new RegExp(`\\n#{2,4}\\s+${phaseHeadingPrefixSrcFor(PHASE_HEADING_BASELINE.ANY_BRACKET, convention)}[A-Za-z]?\\d[\\d.-]*`, 'i')); + // #4478 follow-up (independent code review on this same fix): ` {0,3}` after + // the literal `\n` mirrors phasePattern's own new leading-space tolerance + // above -- without it, a legitimately-indented (1-3 space) NEXT phase + // heading was invisible to this boundary lookup, letting the PRIOR phase's + // goal/mode/depends_on extraction bleed across the section boundary into + // the next phase's own body. + const nextHeader = restOfContent.match(new RegExp(`\\n {0,3}#{2,4}\\s+${phaseHeadingPrefixSrcFor(PHASE_HEADING_BASELINE.ANY_BRACKET, convention)}[A-Za-z]?\\d[\\d.-]*`, 'i')); const sectionEnd = nextHeader ? sectionStart + nextHeader.index! : content.length; const section = content.slice(sectionStart, sectionEnd); diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs index de1ecf3fc..03d544685 100644 --- a/tests/roadmap.test.cjs +++ b/tests/roadmap.test.cjs @@ -363,6 +363,127 @@ describe('roadmap analyze command', () => { assert.strictEqual(output.phases[1].goal, 'Colon outside bold', 'colon-outside goal works'); assert.strictEqual(output.phases[1].depends_on, 'Phase 1', 'colon-outside depends_on works'); }); + + // #4478: collectAnalyzePhases's phase-heading regex had no line anchor — + // #{2,4} could match a `### Phase N:`-shaped mention ANYWHERE the global + // regex scan reached: mid-sentence prose, inside a blockquote, inside an + // inline code span (backtick-quoted on the same line, not a fenced code + // block tokenizeHeadings would exclude). Any such line minted a phantom + // phase entry. Rows mirror the issue's own reproduction table exactly. + const PHANTOM_PHASE_LOOKALIKE_ROWS = [ + ['blockquote + inline code span', '> A note mentioning `### Phase 2:` in a blockquote.'], + ['blockquote, no code span', '> A note mentioning ### Phase 2: in a blockquote.'], + ['plain prose + inline code span', 'A note mentioning `### Phase 2:` in plain prose.'], + ['plain prose, no code span', 'A note mentioning ### Phase 2: in plain prose.'], + ['list item + inline code span', '- list item mentioning `### Phase 2:` here.'], + ]; + + for (const [label, lookalikeLine] of PHANTOM_PHASE_LOOKALIKE_ROWS) { + test(`#4478: does not mint a phantom phase — ${label}`, () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## Milestone v1.0 — Example + +${lookalikeLine} + +### Phase 1: Real First Phase + +**Goal**: Real. +` + ); + const result = runGsdTools('roadmap analyze', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_count, 1, `expected only the real phase, got: ${JSON.stringify(output.phases)}`); + assert.deepStrictEqual(output.phases.map((p) => p.name), ['Real First Phase']); + }); + } + + test('#4478: a phantom does not shadow or collide with a real heading sharing its number', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## Milestone v1.0 — Example + +### Phase 1: Real First Phase + +**Goal**: Real. + +A note mentioning ### Phase 2: in plain prose. + +### Phase 2: Real Second Phase + +**Goal**: Also real. +` + ); + const result = runGsdTools('roadmap analyze', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_count, 2, `expected exactly the two real phases, got: ${JSON.stringify(output.phases)}`); + assert.deepStrictEqual( + output.phases.map((p) => p.name).sort(), + ['Real First Phase', 'Real Second Phase'].sort(), + ); + }); + + test('#4478: a legitimately-indented (1-3 space) real heading is still counted', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## Milestone v1.0 — Example + + ### Phase 1: Indented but real + +**Goal**: Real. +` + ); + const result = runGsdTools('roadmap analyze', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_count, 1, `expected the indented heading to still count, got: ${JSON.stringify(output.phases)}`); + assert.deepStrictEqual(output.phases.map((p) => p.name), ['Indented but real']); + }); + + // Follow-up (found by this fix's own independent code review): the "next + // heading" section-boundary lookup a few lines below phasePattern lacked + // the SAME {0,3} leading-space tolerance, so an indented NEXT phase heading + // was invisible to it, letting the prior phase's field extraction bleed + // across the section boundary. Uses **Depends on:** specifically because + // it is a field only the SECOND phase has — Goal alone can't demonstrate + // the bleed since both phases have one and the first (non-global) match + // still finds phase 1's own occurrence first either way. + test('#4478: an indented NEXT phase heading still bounds the prior phase\'s field extraction', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap + +## Milestone v1.0 — Example + +### Phase 1: Real First Phase + +**Goal**: First goal. + + ### Phase 2: Indented Next Phase + +**Goal**: Second goal. +**Depends on:** Phase 1 +` + ); + const result = runGsdTools('roadmap analyze', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const output = JSON.parse(result.output); + const byNumber = Object.fromEntries(output.phases.map((p) => [p.number, p])); + assert.strictEqual( + byNumber['1'].depends_on, + null, + `phase 1 has no Depends on field of its own — it must not inherit phase 2's, got: ${JSON.stringify(byNumber['1'])}`, + ); + assert.strictEqual(byNumber['2'].depends_on, 'Phase 1'); + }); }); // ─────────────────────────────────────────────────────────────────────────────