golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#2126): route roadmap.cts CLI resolvers through shared lookup sources (drives #2114)

Browse Source 
Phase 3 of epic #2121. cmdRoadmapGetPhase and getRoadmapPhaseWithFallback now
iterate the shared roadmapPhaseLookupSources (exact -> numeric -> prefix-tolerant,
owned by phase-id.cts since Phase 1) instead of a hand-rolled 2-source lookup, so
all three roadmap resolvers share one resolution contract.

Drives #2114: `roadmap get-phase <bare-N>` now resolves a drifted
`### Phase AB-29:` heading (matching getRoadmapPhaseInternal / init.phase-op),
previously EMPTY from the CLI. The malformed_roadmap checklist-fallback and the
milestone-then-full precedence are preserved (a milestone checklist never blocks
a full-roadmap header match).

Behavior reversal (approved in-session): a bare query now also resolves a
*drifted-only* prefixed heading when no bare sibling exists, reversing the #3599
counter-test's expectation. #3599's real anti-steal intent (a bare sibling wins
over a distinct prefixed one) is preserved by the exact->numeric->prefix-tolerant
ordering and re-asserted in the updated test; a new #2114 block covers the
drifted-only case. Fail-first demonstrated.

Closes #2126
Refs #2121

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-07-09 19:56:10 -04:00
parent b9c8ea143b
commit 61f3cafc70
2 changed files with 92 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

80
src/roadmap.cts
View File

@@ -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);
}

72
tests/roadmap.test.cjs
View File

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