diff --git a/.changeset/proud-rams-greet.md b/.changeset/proud-rams-greet.md new file mode 100644 index 000000000..ac8930ac3 --- /dev/null +++ b/.changeset/proud-rams-greet.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1845 +--- +**Roadmap phase lookup now ignores fenced examples and the backlog sentinel lane** — `roadmap get-phase` and `init plan-phase` no longer return fenced sample headings as real phases or treat `999.x` backlog items as active milestone work. (#1845) diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 862920be3..882e3b6c7 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -209,19 +209,22 @@ interface RoadmapPhaseResult { } function findRoadmapPhaseInContent(content: string, phaseNum: unknown, phaseSource?: string): RoadmapPhaseResult | null { - const phasePattern = new RegExp( - `#{2,4}\\s*(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${phaseSource ?? phaseMarkdownRegexSource(phaseNum)}:\\s*([^\\n]+)`, + const headingPattern = new RegExp( + `^(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${phaseSource ?? phaseMarkdownRegexSource(phaseNum)}:\\s*(.+)$`, 'i' ); - const headerMatch = content.match(phasePattern); + const headings = tokenizeHeadings(content); + const headingIndex = headings.findIndex((heading) => headingPattern.test(heading.text)); + if (headingIndex === -1) return null; + + const heading = headings[headingIndex]; + const headerMatch = heading.text.match(headingPattern); if (!headerMatch) return null; const phaseName = headerMatch[1].trim(); - const headerIndex = headerMatch.index!; - const restOfContent = content.slice(headerIndex); - const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+(?:\[[^\]]+\]\s*)?Phase\s+[\w]/i); - const sectionEnd = nextHeaderMatch ? headerIndex + nextHeaderMatch.index! : content.length; - const section = content.slice(headerIndex, sectionEnd).trim(); + const nextHeading = headings.slice(headingIndex + 1).find((candidate) => candidate.level <= heading.level); + const sectionEnd = nextHeading ? nextHeading.offset : content.length; + const section = content.slice(heading.offset, sectionEnd).trim(); const goalMatch = section.match(/\*\*Goal(?:\*\*:|\*?\*?:\*\*)\s*([^\n]+)/i); const goal = goalMatch ? goalMatch[1].trim() : null; @@ -254,6 +257,8 @@ function roadmapPhaseLookupSources(phaseNum: unknown): string[] { function getRoadmapPhaseInternal(cwd: string, phaseNum: unknown): RoadmapPhaseResult | null { if (!phaseNum) return null; + const normalizedPhase = stripProjectCodePrefix(phaseNum); + if (/^999(?:\.|$)/.test(normalizedPhase)) return null; const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); if (!fs.existsSync(roadmapPath)) return null; diff --git a/src/roadmap.cts b/src/roadmap.cts index 2fbe15df2..02547f4a4 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -14,13 +14,14 @@ import ioMod = require('./io.cjs'); const { output, error } = ioMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseIdMod = require('./phase-id.cjs'); -const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, phaseTokenMatches } = phaseIdMod; +const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, phaseTokenMatches, stripProjectCodePrefix } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal } = phaseLocatorMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import roadmapParserModule = require('./roadmap-parser.cjs'); const { stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone } = roadmapParserModule; +import { tokenizeHeadings } from './markdown-sectionizer.cjs'; import { platformWriteSync } from './shell-command-projection.cjs'; // eslint-disable-next-line @typescript-eslint/no-require-imports import planningWorkspace = require('./planning-workspace.cjs'); @@ -117,12 +118,13 @@ function countPhasePlansAndSummaries(phaseDir: string): PhasePlansAndSummaries { * checklist-only match), or null if the phase is not present at all. */ function searchPhaseInContent(content: string, escapedPhase: string, phaseNum: string): PhaseSearchResult | null { - // Match "## Phase X:", "### Phase X:", or "#### Phase X:" with optional name - const phasePattern = new RegExp( - `#{2,4}\\s*(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${escapedPhase}:\\s*([^\\n]+)`, + const headingPattern = new RegExp( + `^(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${escapedPhase}:\\s*(.+)$`, 'i' ); - const headerMatch = content.match(phasePattern); + const headings = tokenizeHeadings(content); + const headingIndex = headings.findIndex((heading) => headingPattern.test(heading.text)); + const headerMatch = headingIndex === -1 ? null : headings[headingIndex].text.match(headingPattern); if (!headerMatch) { // Fallback: check if phase exists in summary list but missing detail section @@ -146,15 +148,13 @@ function searchPhaseInContent(content: string, escapedPhase: string, phaseNum: s } const phaseName = headerMatch[1].trim(); - const headerIndex = headerMatch.index!; + const headerIndex = headings[headingIndex].offset; - // Find the end of this section (next ## or ### phase header, or end of file). - // Also matches bracket-prefixed headings like ### [GSD] Phase 2-01:. - const restOfContent = content.slice(headerIndex); - const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+(?:\[[^\]]+\]\s*)?Phase\s+[\w][\w.-]*/i); - const sectionEnd = nextHeaderMatch - ? headerIndex + nextHeaderMatch.index! - : content.length; + const currentHeading = headings[headingIndex]; + const nextHeading = headings + .slice(headingIndex + 1) + .find((candidate) => candidate.level <= currentHeading.level); + const sectionEnd = nextHeading ? nextHeading.offset : content.length; const section = content.slice(headerIndex, sectionEnd).trim(); @@ -200,6 +200,7 @@ function searchPhaseInContent(content: string, escapedPhase: string, phaseNum: s * phase resolution as `roadmap.get-phase` — not a milestone-only subset. */ function getRoadmapPhaseWithFallback(cwd: string, phaseNum: string): string | null { + if (/^999(?:\.|$)/.test(stripProjectCodePrefix(phaseNum))) return null; const roadmapPath = planningPaths(cwd).roadmap; if (!fs.existsSync(roadmapPath)) return null; @@ -228,6 +229,10 @@ function getRoadmapPhaseWithFallback(cwd: string, phaseNum: string): string | nu // ─── cmdRoadmapGetPhase ─────────────────────────────────────────────────────── function cmdRoadmapGetPhase(cwd: string, phaseNum: string, raw: boolean): void { + if (/^999(?:\.|$)/.test(stripProjectCodePrefix(phaseNum))) { + output({ found: false, phase_number: phaseNum }, raw, ''); + return; + } const roadmapPath = planningPaths(cwd).roadmap; if (!fs.existsSync(roadmapPath)) { diff --git a/tests/feat-3594-parser-adversarial-roadmap.test.cjs b/tests/feat-3594-parser-adversarial-roadmap.test.cjs index d2d249775..d92eb4623 100644 --- a/tests/feat-3594-parser-adversarial-roadmap.test.cjs +++ b/tests/feat-3594-parser-adversarial-roadmap.test.cjs @@ -90,27 +90,69 @@ describe('feat-3594: roadmap parser and fenced-code-block headings (#2787)', () assert.match(result.parsed.phase_name, /real phase one/); }); - test('phase 999 inside a fenced block: CJS parser currently STILL matches it (open: needs fence-stripping)', (t) => { + test('phase 999 inside a fenced block is ignored', (t) => { const projectDir = projectWithFixture(t, 'phase-heading-inside-fenced-code.md'); const result = getPhase(projectDir, '999'); assert.equal(result.hasStackTrace, false, 'no stack trace'); - // The CJS regex parser does not strip fenced code blocks before - // matching. The SDK roadmap parser tracks fenced blocks (per #2787 - // comment in sdk/src/query/roadmap.ts) — the CJS path has not caught - // up. This test pins the current behavior so the day someone wires - // CJS fence-stripping, flipping `found: true` to `found: false` - // becomes the regression guard. assert.ok(result.parsed, `expected JSON payload, got: ${result.raw}`); - assert.equal(result.parsed.found, true, 'CJS parser currently matches inside fences (known open bug)'); - // The matched heading is the one INSIDE the fenced block. Match - // its distinctive substring so a future "fix" that strips fences - // and instead matches a different (real) phase 999 (which we don't - // have in this fixture, so impossible) still fails the right test. - assert.match( - result.parsed.phase_name, - /fenced code block/i, - 'currently-matched heading must be the one inside the fence', + assert.equal(result.parsed.found, false, 'phase headings inside fenced blocks must not be parsed'); + }); + + test('fenced example heading does not shadow the real phase details and backlog phase stays unresolved (#1588)', (t) => { + const projectDir = createTempProject('roadmap-1588-'); + t.after(() => cleanup(projectDir)); + fs.writeFileSync( + path.join(projectDir, '.planning', 'STATE.md'), + [ + '---', + 'gsd_state_version: 1.0', + 'milestone: v1.1', + 'status: planning', + '---', + '', + ].join('\n') ); + fs.writeFileSync( + path.join(projectDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '
', + 'v1.1 Current (Phases 8-9) - PLANNED', + '', + '- [ ] **Phase 9: Real Phase**', + '', + '
', + '', + '## Phase Details', + '', + '```markdown', + '### Phase 9: Fenced Example Phase', + '**Goal:** This example must not be treated as roadmap structure.', + '```', + '', + '### Phase 9: Real Phase', + '**Goal:** Use the real phase details outside the fenced block.', + '**Requirements:** REAL-01', + '', + '## Backlog', + '', + '### Phase 999.1: Backlog Thing', + '**Goal:** Future backlog item.', + '', + ].join('\n') + ); + + const phase9 = getPhase(projectDir, '9'); + assert.ok(phase9.parsed, `expected JSON payload, got: ${phase9.raw}`); + assert.equal(phase9.parsed.found, true, 'phase 9 must be found'); + assert.equal(phase9.parsed.phase_name, 'Real Phase'); + assert.equal(phase9.parsed.goal, 'Use the real phase details outside the fenced block.'); + assert.match(phase9.parsed.section, /REAL-01/, 'real phase section must be returned'); + + const backlog = getPhase(projectDir, '999.1'); + assert.ok(backlog.parsed, `expected JSON payload, got: ${backlog.raw}`); + assert.equal(backlog.parsed.found, false, 'backlog sentinel phase must not resolve as an active roadmap phase'); }); }); @@ -205,15 +247,12 @@ describe('feat-3594: roadmap parser and HTML-commented headings', () => { assert.equal(result.parsed.phase_name, 'real phase'); }); - test('phase 999 inside an HTML comment: CJS parser currently STILL matches it (open: needs comment-stripping)', (t) => { + test('phase 999 inside an HTML comment remains ignored because backlog sentinels never resolve', (t) => { const projectDir = projectWithFixture(t, 'markdown-headings-inside-html-comment.md'); const result = getPhase(projectDir, '999'); assert.equal(result.hasStackTrace, false, 'no stack trace'); - // Same shape as the fenced-code-block case: the CJS regex parser - // doesn't strip HTML comments before matching. assert.ok(result.parsed, `expected JSON payload, got: ${result.raw}`); - assert.equal(result.parsed.found, true, 'CJS parser currently matches inside HTML comments (known open bug)'); - assert.match(result.parsed.phase_name, /HTML comment/); + assert.equal(result.parsed.found, false, 'backlog sentinel phases must not resolve'); }); }); diff --git a/tests/init.test.cjs b/tests/init.test.cjs index f0c1ad693..924c7f5a2 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -8,6 +8,7 @@ const fs = require('fs'); const path = require('path'); const { runGsdTools, cleanup } = require('./helpers.cjs'); const { createFixture, seedPhase } = require('./fixtures/index.cjs'); +const { createTempProject } = require('./helpers.cjs'); describe('init commands', () => { let tmpDir; @@ -426,6 +427,66 @@ describe('init commands', () => { assert.strictEqual(output.phase_req_ids, 'REQ-02, REQ-03'); }); + test('init plan-phase prefers real phase details outside fenced examples and ignores backlog sentinels (#1588)', () => { + const projectDir = createTempProject('init-1588-'); + try { + fs.writeFileSync( + path.join(projectDir, '.planning', 'STATE.md'), + [ + '---', + 'gsd_state_version: 1.0', + 'milestone: v1.1', + 'status: planning', + '---', + '', + ].join('\n') + ); + fs.writeFileSync( + path.join(projectDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '
', + 'v1.1 Current (Phases 8-9) - PLANNED', + '', + '- [ ] **Phase 9: Real Phase**', + '', + '
', + '', + '## Phase Details', + '', + '```markdown', + '### Phase 9: Fenced Example Phase', + '**Goal:** This example must not be treated as roadmap structure.', + '```', + '', + '### Phase 9: Real Phase', + '**Goal:** Use the real phase details outside the fenced block.', + '**Requirements:** REAL-01', + '', + '## Backlog', + '', + '### Phase 999.1: Backlog Thing', + '**Goal:** Future backlog item.', + '', + ].join('\n') + ); + + const phase9 = runGsdTools('init plan-phase 9', projectDir); + assert.ok(phase9.success, `init plan-phase 9 failed: ${phase9.error}`); + const phase9Output = JSON.parse(phase9.output); + assert.equal(phase9Output.phase_name, 'Real Phase'); + assert.equal(phase9Output.phase_req_ids, 'REAL-01'); + + const backlog = runGsdTools('init plan-phase 999.1', projectDir); + assert.ok(backlog.success, `init plan-phase 999.1 failed: ${backlog.error}`); + const backlogOutput = JSON.parse(backlog.output); + assert.equal(backlogOutput.phase_found, false); + } finally { + cleanup(projectDir); + } + }); + test('init phase-op resolves a details-summary milestone phase from later flat Phase Details', () => { fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), [ 'milestone: v1.11', diff --git a/tests/roadmap-phase-fallback.test.cjs b/tests/roadmap-phase-fallback.test.cjs index de5653e86..287e7463d 100644 --- a/tests/roadmap-phase-fallback.test.cjs +++ b/tests/roadmap-phase-fallback.test.cjs @@ -84,7 +84,7 @@ describe('roadmap get-phase fallback to full ROADMAP.md (#1634)', () => { assert.equal(output.goal, 'Set up project infrastructure'); }); - test('backlog phase outside current milestone resolves via fallback', () => { + test('backlog sentinel outside current milestone does not resolve via fallback', () => { writeState(tmpDir, 'v1.0'); fs.writeFileSync( path.join(tmpDir, '.planning', 'ROADMAP.md'), @@ -106,10 +106,8 @@ describe('roadmap get-phase fallback to full ROADMAP.md (#1634)', () => { assert.ok(result.success, `Command failed: ${result.error}`); const output = JSON.parse(result.output); - assert.equal(output.found, true, 'backlog phase should be found via fallback'); + assert.equal(output.found, false, 'backlog sentinel should stay unresolved'); assert.equal(output.phase_number, '999.60'); - assert.equal(output.phase_name, 'Backlog Cleanup'); - assert.equal(output.goal, 'Clean up technical debt from backlog'); }); test('future planned milestone phase resolves via fallback', () => {