Merge pull request #3616 from gsd-build/fix/3599-bug-roadmap-get-phase-no-longer-matches-
fix(3599): preserve project-code prefix when looking up roadmap phases
This commit is contained in:
5
.changeset/3599-roadmap-get-phase-project-code-prefix.md
Normal file
5
.changeset/3599-roadmap-get-phase-project-code-prefix.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
issue: 3599
|
||||
---
|
||||
**`roadmap get-phase PROJ-42` now matches `### Phase PROJ-42:` instead of returning `found: false`** — `cmdRoadmapGetPhase` now does a two-pass search when the caller passes a project-code-prefixed ID. The exact-escaped form (`### Phase PROJ-42:`) is tried first, and only falls back to the #3537 padding-tolerant numeric form (`### Phase 42:`) when the exact heading is not present. New helper `phaseMarkdownRegexSourceExact()` is added alongside the existing `phaseMarkdownRegexSource()` so callers that need the prefix-preserving path can opt in.
|
||||
@@ -687,6 +687,25 @@ function phaseMarkdownRegexSource(phaseNum) {
|
||||
return `0*${escapeRegex(integer)}${letter}${decimal}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* #3599: when the caller passed a project-code-prefixed ID like `PROJ-42`,
|
||||
* return the exact-escaped form so the caller can search the ROADMAP for
|
||||
* `### Phase PROJ-42:` BEFORE falling back to the padding-tolerant numeric
|
||||
* form. Returns null when the input has no project-code prefix — in that
|
||||
* case the numeric form (`phaseMarkdownRegexSource`) is the only thing the
|
||||
* caller needs.
|
||||
*
|
||||
* Two-pass at the call site preserves the #3537 contract (`CK-01` directory
|
||||
* names mapping to `Phase 1:` prose) while letting `PROJ-42` resolve to its
|
||||
* own prefixed heading without cross-matching a bare `### Phase 42:` that
|
||||
* happens to share the trailing integer.
|
||||
*/
|
||||
function phaseMarkdownRegexSourceExact(phaseNum) {
|
||||
const raw = String(phaseNum);
|
||||
if (!/^[A-Z]{1,6}-(?=\d)/i.test(raw)) return null;
|
||||
return escapeRegex(raw);
|
||||
}
|
||||
|
||||
function comparePhaseNum(a, b) {
|
||||
// Strip optional project_code prefix before comparing (e.g., 'CK-01-name' → '01-name')
|
||||
const sa = String(a).replace(/^[A-Z]{1,6}-/, '');
|
||||
@@ -1848,6 +1867,7 @@ module.exports = {
|
||||
escapeRegex,
|
||||
normalizePhaseName,
|
||||
phaseMarkdownRegexSource,
|
||||
phaseMarkdownRegexSourceExact,
|
||||
comparePhaseNum,
|
||||
searchPhaseInDir,
|
||||
extractPhaseToken,
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, phaseTokenMatches } = require('./core.cjs');
|
||||
const { escapeRegex, normalizePhaseName, phaseMarkdownRegexSource, phaseMarkdownRegexSourceExact, output, error, findPhaseInternal, stripShippedMilestones, extractCurrentMilestone, replaceInCurrentMilestone, phaseTokenMatches } = require('./core.cjs');
|
||||
const { platformWriteSync } = require('./shell-command-projection.cjs');
|
||||
const { planningPaths, withPlanningLock } = require('./planning-workspace.cjs');
|
||||
const scanPhasePlans = require('./plan-scan.cjs');
|
||||
@@ -94,7 +94,7 @@ function searchPhaseInContent(content, escapedPhase, phaseNum) {
|
||||
|
||||
// Find the end of this section (next ## or ### phase header, or end of file)
|
||||
const restOfContent = content.slice(headerIndex);
|
||||
const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+Phase\s+\d/i);
|
||||
const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+Phase\s+[\w][\w.-]*/i);
|
||||
const sectionEnd = nextHeaderMatch
|
||||
? headerIndex + nextHeaderMatch.index
|
||||
: content.length;
|
||||
@@ -139,6 +139,29 @@ function cmdRoadmapGetPhase(cwd, phaseNum, raw) {
|
||||
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);
|
||||
return;
|
||||
}
|
||||
const exactFull = searchPhaseInContent(fullContent, exactSource, phaseNum);
|
||||
if (exactFull && !exactFull.error) {
|
||||
output(exactFull, raw, exactFull.section);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// #3537: padding-tolerant fragment so callers passing `02.7` still match
|
||||
// un-padded ROADMAP prose (`### Phase 2.7:`).
|
||||
const escapedPhase = phaseMarkdownRegexSource(phaseNum);
|
||||
@@ -146,7 +169,6 @@ function cmdRoadmapGetPhase(cwd, phaseNum, raw) {
|
||||
// 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 fullContent = stripShippedMilestones(rawContent);
|
||||
const milestoneResult = searchPhaseInContent(milestoneContent, escapedPhase, phaseNum);
|
||||
const result = (milestoneResult && !milestoneResult.error)
|
||||
? milestoneResult
|
||||
|
||||
167
tests/bug-3599-roadmap-get-phase-project-code-prefix.test.cjs
Normal file
167
tests/bug-3599-roadmap-get-phase-project-code-prefix.test.cjs
Normal file
@@ -0,0 +1,167 @@
|
||||
/**
|
||||
* Bug #3599: roadmap.get-phase no longer matches custom phase IDs with
|
||||
* project-code prefixes like `PROJ-42`.
|
||||
*
|
||||
* `phaseMarkdownRegexSource(phaseNum)` in get-shit-done/bin/lib/core.cjs
|
||||
* (and its SDK twin in sdk/src/query/roadmap-update-plan-progress.ts) strips
|
||||
* the `PROJ-` prefix before building the padding-tolerant numeric regex.
|
||||
* Result: `roadmap get-phase PROJ-42` produces a regex of `0*42`, which
|
||||
* matches `### Phase 42:` instead of (or in addition to) the intended
|
||||
* `### Phase PROJ-42:`. The function's own docstring promises a fallback to
|
||||
* `escapeRegex(phaseNum)` for non-numeric custom IDs, but that branch is
|
||||
* unreachable for project-code-prefixed numeric IDs.
|
||||
*
|
||||
* Fix: the emitted regex must match BOTH the stripped numeric form (so
|
||||
* `CK-01-name` directory inputs still resolve to `Phase 1:` in prose, the
|
||||
* #3537 contract) AND the full prefixed form (so `PROJ-42` resolves to
|
||||
* `Phase PROJ-42:`).
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { describe, test, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||
|
||||
function writeRoadmap(tmpDir, body) {
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), body);
|
||||
}
|
||||
|
||||
function writeState(tmpDir, version) {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'STATE.md'),
|
||||
`---\nmilestone: ${version}\n---\n`,
|
||||
);
|
||||
}
|
||||
|
||||
describe('bug #3599: roadmap get-phase preserves project-code prefix in lookup', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject('bug-3599-');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
});
|
||||
|
||||
test('finds ### Phase PROJ-42: when queried as PROJ-42', () => {
|
||||
writeState(tmpDir, 'v1.0.0');
|
||||
writeRoadmap(
|
||||
tmpDir,
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0.0 - Test',
|
||||
'',
|
||||
'### Phase PROJ-42: Custom phase',
|
||||
'**Goal:** Verify project-code-prefixed lookup',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
|
||||
const result = runGsdTools('roadmap get-phase PROJ-42 --json', tmpDir);
|
||||
assert.ok(result.success, `command failed: ${result.error || result.output}`);
|
||||
|
||||
const payload = JSON.parse(result.output);
|
||||
assert.strictEqual(payload.found, true, `expected found=true, got: ${result.output}`);
|
||||
assert.strictEqual(payload.phase_name, 'Custom phase');
|
||||
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.
|
||||
writeState(tmpDir, 'v1.0.0');
|
||||
writeRoadmap(
|
||||
tmpDir,
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0.0 - Test',
|
||||
'',
|
||||
'### Phase PROJ-42: Should not be returned for `42`',
|
||||
'**Goal:** Counter-test',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
|
||||
const result = runGsdTools('roadmap get-phase 42 --json', tmpDir);
|
||||
assert.ok(result.success);
|
||||
const payload = JSON.parse(result.output);
|
||||
assert.strictEqual(
|
||||
payload.found,
|
||||
false,
|
||||
`bare numeric '42' must not match 'Phase PROJ-42:'; got ${result.output}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('preserves #3537 contract: CK-01 directory form resolves to Phase 1 prose', () => {
|
||||
// Existing contract: phase directory names like `CK-01-name` carry the
|
||||
// project_code prefix and a zero-padded number, but ROADMAP prose is
|
||||
// typically un-padded (`### Phase 1:`). The padding-tolerant lookup must
|
||||
// still bridge those two surfaces.
|
||||
writeState(tmpDir, 'v1.0.0');
|
||||
writeRoadmap(
|
||||
tmpDir,
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0.0 - Test',
|
||||
'',
|
||||
'### Phase 1: Numeric prose',
|
||||
'**Goal:** #3537 contract — CK-01 dir → Phase 1 prose',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
|
||||
const result = runGsdTools('roadmap get-phase CK-01 --json', tmpDir);
|
||||
assert.ok(result.success);
|
||||
const payload = JSON.parse(result.output);
|
||||
assert.strictEqual(
|
||||
payload.found,
|
||||
true,
|
||||
`CK-01 must still resolve to 'Phase 1:' prose (#3537 contract); got ${result.output}`,
|
||||
);
|
||||
assert.strictEqual(payload.phase_name, 'Numeric prose');
|
||||
});
|
||||
|
||||
test('finds the right phase when both prefixed and bare forms coexist', () => {
|
||||
// Disambiguation test: a roadmap that contains BOTH `### Phase 42:` and
|
||||
// `### Phase PROJ-42:` must resolve each query to its specific match.
|
||||
writeState(tmpDir, 'v1.0.0');
|
||||
writeRoadmap(
|
||||
tmpDir,
|
||||
[
|
||||
'# Roadmap',
|
||||
'',
|
||||
'## Current Milestone: v1.0.0 - Test',
|
||||
'',
|
||||
'### Phase 42: Bare numeric',
|
||||
'**Goal:** Bare',
|
||||
'',
|
||||
'### Phase PROJ-42: Prefixed',
|
||||
'**Goal:** Prefixed',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
|
||||
const r42 = runGsdTools('roadmap get-phase 42 --json', tmpDir);
|
||||
const rProj = runGsdTools('roadmap get-phase PROJ-42 --json', tmpDir);
|
||||
|
||||
const p42 = JSON.parse(r42.output);
|
||||
const pProj = JSON.parse(rProj.output);
|
||||
|
||||
assert.strictEqual(p42.found, true);
|
||||
assert.strictEqual(p42.phase_name, 'Bare numeric');
|
||||
assert.strictEqual(p42.goal, 'Bare');
|
||||
|
||||
assert.strictEqual(pProj.found, true);
|
||||
assert.strictEqual(pProj.phase_name, 'Prefixed');
|
||||
assert.strictEqual(pProj.goal, 'Prefixed');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user