diff --git a/src/roadmap.cts b/src/roadmap.cts index 740794042..27f0e36c3 100644 --- a/src/roadmap.cts +++ b/src/roadmap.cts @@ -14,7 +14,7 @@ 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, stripProjectCodePrefix, OPTIONAL_PHASE_TAG_SOURCE } = phaseIdMod; +const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, phaseTokenMatches, stripProjectCodePrefix, OPTIONAL_PHASE_TAG_SOURCE, roadmapPhaseLookupSources } = phaseIdMod; // eslint-disable-next-line @typescript-eslint/no-require-imports import phaseLocatorMod = require('./phase-locator.cjs'); const { findPhaseInternal } = phaseLocatorMod; @@ -215,22 +215,17 @@ function getRoadmapPhaseWithFallback(cwd: string, phaseNum: string): string | nu const milestoneContent = extractCurrentMilestone(rawContent, cwd); const fullContent = stripShippedMilestones(rawContent); - const exactSource = phaseMarkdownRegexSourceExact(phaseNum); - if (exactSource) { - const exactMilestone = searchPhaseInContent(milestoneContent, exactSource, phaseNum); - if (exactMilestone && !exactMilestone.error) return exactMilestone.section ?? null; - const exactFull = searchPhaseInContent(fullContent, exactSource, phaseNum); - if (exactFull && !exactFull.error) return exactFull.section ?? null; + // #2121/#2114: iterate the shared lookup-source list (exact → numeric → + // prefix-tolerant) so this resolver matches getRoadmapPhaseInternal and a + // bare-number query resolves a drifted project-code-prefixed heading. + for (const source of roadmapPhaseLookupSources(phaseNum)) { + const milestoneResult = searchPhaseInContent(milestoneContent, source, phaseNum); + if (milestoneResult && !milestoneResult.error) return milestoneResult.section ?? null; + const fullResult = searchPhaseInContent(fullContent, source, phaseNum); + if (fullResult && !fullResult.error) return fullResult.section ?? null; } - const escapedPhase = phaseMarkdownRegexSource(phaseNum); - const milestoneResult = searchPhaseInContent(milestoneContent, escapedPhase, phaseNum); - const result = (milestoneResult && !milestoneResult.error) - ? milestoneResult - : searchPhaseInContent(fullContent, escapedPhase, phaseNum) || milestoneResult; - - if (!result || result.error) return null; - return result.section ?? null; + return null; } // ─── cmdRoadmapGetPhase ─────────────────────────────────────────────────────── @@ -251,52 +246,37 @@ function cmdRoadmapGetPhase(cwd: string, phaseNum: string, raw: boolean): void { const rawContent = fs.readFileSync(roadmapPath, 'utf-8'); const milestoneContent = extractCurrentMilestone(rawContent, cwd); - // #3599 two-pass: when the caller passes a project-code-prefixed ID like - // `PROJ-42`, try the exact-prefixed heading first (`### Phase PROJ-42:`). - // If no match, fall back to the #3537 padding-tolerant numeric form so - // a `CK-01` query still resolves to `### Phase 1:`. Doing this at the - // call site (instead of inside phaseMarkdownRegexSource) avoids the - // alternation-order ambiguity where a bare `### Phase 42:` heading in - // the same document would intercept the match for a `PROJ-42` query. const fullContent = stripShippedMilestones(rawContent); - const exactSource = phaseMarkdownRegexSourceExact(phaseNum); - if (exactSource) { - const exactMilestone = searchPhaseInContent(milestoneContent, exactSource, phaseNum); - if (exactMilestone && !exactMilestone.error) { - output(exactMilestone, raw, exactMilestone.section); + // #2121/#2114: iterate the shared lookup-source list (exact → numeric → + // prefix-tolerant) so all three roadmap resolvers share one contract and a + // bare-number query resolves a drifted `### Phase AB-29:` heading. This + // preserves the #3599 exact-prefix-first and #3537 padding-tolerant behavior + // (both now encoded in roadmapPhaseLookupSources' ordering). A clean match + // (milestone or full, any source) wins immediately; a malformed_roadmap + // (checklist-only) candidate is surfaced only if no source finds a real + // heading — so a milestone checklist never blocks a full-roadmap header. + let malformed: PhaseSearchResult | null = null; + for (const source of roadmapPhaseLookupSources(phaseNum)) { + const milestoneResult = searchPhaseInContent(milestoneContent, source, phaseNum); + if (milestoneResult && !milestoneResult.error) { + output(milestoneResult, raw, milestoneResult.section); return; } - const exactFull = searchPhaseInContent(fullContent, exactSource, phaseNum); - if (exactFull && !exactFull.error) { - output(exactFull, raw, exactFull.section); + const fullResult = searchPhaseInContent(fullContent, source, phaseNum); + if (fullResult && !fullResult.error) { + output(fullResult, raw, fullResult.section); return; } + if (!malformed) malformed = (milestoneResult?.error ? milestoneResult : (fullResult?.error ? fullResult : null)); } - // #3537: padding-tolerant fragment so callers passing `02.7` still match - // un-padded ROADMAP prose (`### Phase 2.7:`). - const escapedPhase = phaseMarkdownRegexSource(phaseNum); - - // Search the current milestone slice first, then fall back to full roadmap. - // A malformed_roadmap result (checklist-only) from the milestone should not - // block finding a full header match in the wider roadmap content. - const milestoneResult = searchPhaseInContent(milestoneContent, escapedPhase, phaseNum); - const result = (milestoneResult && !milestoneResult.error) - ? milestoneResult - : searchPhaseInContent(fullContent, escapedPhase, phaseNum) || milestoneResult; - - if (!result) { - output({ found: false, phase_number: phaseNum }, raw, ''); + if (malformed) { + output(malformed, raw, ''); return; } - if (result.error) { - output(result, raw, ''); - return; - } - - output(result, raw, result.section); + output({ found: false, phase_number: phaseNum }, raw, ''); } catch (e) { error('Failed to read ROADMAP.md: ' + (e as Error).message); } diff --git a/tests/roadmap.test.cjs b/tests/roadmap.test.cjs index da0ebfe65..e3c1d850d 100644 --- a/tests/roadmap.test.cjs +++ b/tests/roadmap.test.cjs @@ -1918,11 +1918,13 @@ describe('bug #3599: roadmap get-phase preserves project-code prefix in lookup', assert.strictEqual(payload.goal, 'Verify project-code-prefixed lookup'); }); - test('does NOT cross-match: querying 42 must not match ### Phase PROJ-42:', () => { - // Counter-test: if the regex erroneously matches both forms in both - // directions, this catches it. `42` must only match `Phase 42:` — not - // `Phase PROJ-42:` — otherwise integer phase lookups silently steal - // matches from prefixed siblings. + test('bare numeric prefers a bare sibling over a prefixed one (#3599 anti-steal, updated for #2114)', () => { + // #3599's real guard is anti-STEALING: when BOTH a bare `Phase 42:` and a + // distinct prefixed `Phase PROJ-42:` exist, a bare `42` query must resolve + // the BARE one — the numeric source is tried before the prefix-tolerant + // fallback, so a bare query never steals a distinct prefixed sibling. + // (Since #2114/#2121, a bare query DOES resolve a *drifted-only* prefixed + // heading when no bare sibling exists — see the bug #2114 block below.) writeState(tmpDir, 'v1.0.0'); writeRoadmap( tmpDir, @@ -1931,8 +1933,11 @@ describe('bug #3599: roadmap get-phase preserves project-code prefix in lookup', '', '## Current Milestone: v1.0.0 - Test', '', - '### Phase PROJ-42: Should not be returned for `42`', - '**Goal:** Counter-test', + '### Phase 42: Bare', + '**Goal:** Canonical bare heading', + '', + '### Phase PROJ-42: Prefixed', + '**Goal:** Distinct prefixed sibling', '', ].join('\n'), ); @@ -1940,10 +1945,11 @@ describe('bug #3599: roadmap get-phase preserves project-code prefix in lookup', const result = runGsdTools('roadmap get-phase 42 --json', tmpDir); assert.ok(result.success); const payload = JSON.parse(result.output); + assert.strictEqual(payload.found, true, `expected found=true, got: ${result.output}`); assert.strictEqual( - payload.found, - false, - `bare numeric '42' must not match 'Phase PROJ-42:'; got ${result.output}`, + payload.phase_name, + 'Bare', + `bare '42' must resolve the bare 'Phase 42:', not steal 'Phase PROJ-42:'; got ${result.output}`, ); }); @@ -2071,6 +2077,52 @@ function makePlanProject(files = {}) { return dir; } +describe('bug #2114: roadmap get-phase resolves drifted prefixed headings by bare number', () => { + let tmpDir; + + beforeEach(() => { tmpDir = createTempProject('bug-2114-'); }); + afterEach(() => { cleanup(tmpDir); }); + + test('bare-number query resolves a drifted project-code-prefixed heading', () => { + // Before the fix, bare `29` did NOT match `### Phase AB-29:` from the CLI + // (2-source lookup), even though getRoadmapPhaseInternal (init.phase-op) did + // — the #2114 divergence. Now all three resolvers share the 3-source list. + fs.writeFileSync( + path.join(tmpDir, '.planning', 'STATE.md'), + '---\nmilestone: v1.0.0\n---\n# State\n\n**Status:** In progress\n', + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + [ + '# Roadmap', + '', + '## Current Milestone: v1.0.0 - Test', + '', + '### Phase 30: Plain', + '**Goal:** Canonical bare heading', + '', + '### Phase AB-29: Prefixed', + '**Goal:** Drifted prefixed heading', + '', + ].join('\n'), + ); + + const resultAB29 = runGsdTools('roadmap get-phase 29 --json', tmpDir); + assert.ok(resultAB29.success, `command failed: ${resultAB29.error || resultAB29.output}`); + const payloadAB29 = JSON.parse(resultAB29.output); + assert.strictEqual(payloadAB29.found, true, `expected found=true for drifted AB-29, got: ${resultAB29.output}`); + assert.strictEqual(payloadAB29.phase_name, 'Prefixed'); + assert.strictEqual(payloadAB29.goal, 'Drifted prefixed heading'); + + // The canonical bare heading still resolves. + const result30 = runGsdTools('roadmap get-phase 30 --json', tmpDir); + assert.ok(result30.success); + const payload30 = JSON.parse(result30.output); + assert.strictEqual(payload30.found, true); + assert.strictEqual(payload30.phase_name, 'Plain'); + }); +}); + describe('roadmap annotate-dependencies', () => { let tmpDir;