Merge pull request #2139 from open-gsd/chore/2126-migrate-roadmap-lookup

chore(#2121): route roadmap.cts through shared lookup sources (drives #2114) — Phase 3
This commit is contained in:
Tom Boucher
2026-07-10 07:55:00 -04:00
committed by GitHub
10 changed files with 339 additions and 6854 deletions

View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 2139
---
**`roadmap get-phase` resolves project-code-prefixed headings by bare number** — a bare-number query (e.g. `29`) now resolves a drifted `### Phase AB-29:` heading, matching the internal resolver used by `init.phase-op`; previously the CLI returned empty. A bare sibling (`### Phase 29:`) still takes precedence. A project-code-prefixed heading present only as a summary/checklist line (no matching detail section) now reports a `malformed_roadmap` diagnostic — for both prefixed and bare-number queries — instead of a silent empty result. (#2114)

View File

@@ -399,4 +399,17 @@ export default tseslint.config(
languageOptions: { sourceType: 'commonjs', globals: { ...globals.node } },
rules: { 'local/no-source-grep': 'error' },
},
// ── #2126 lint-rule CLEAN fixture ───────────────────────────────────────────
// `tests/_ff_lint_clean.cjs` is the KNOWN-CLEAN companion to the violation fixture: the
// prohibition-enforcement real-runner tests lint it as their non-vacuous "clean target" instead of
// a type-aware `src/**/*.cts` file, so each eslint spawn is ~0.8s (non-type-aware) not ~2s
// (whole-tsconfig-program load) — removing the CPU starvation that blew the 60s bound under
// --test-concurrency. Rule enabled (as error) so the pass is non-vacuous; the file is clean so it
// greens. PLAIN `.cjs`, kept OFF the `*.test.cjs` runner glob. (#2126)
{
files: ['tests/_ff_lint_clean.cjs'],
plugins: { local: localPlugin },
languageOptions: { sourceType: 'commonjs', globals: { ...globals.node } },
rules: { 'local/no-source-grep': 'error' },
},
);

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

View File

@@ -2522,10 +2522,12 @@ function rewriteStagedSkillBodies(stagedDir, opts) {
* attribution from opts, then delegates to applyRuntimeContentRewritesForCommandsInPlace
* (single copy+rewrite owner).
*
* @internal — symmetric companion to rewriteStagedSkillBodies; retained as the deep-seam
* API for command bodies. No production caller today (install rewrites commands via
* copyWithPathReplacement → applyRuntimeContentRewritesForCommandsInPlace). Kept for
* API symmetry + test coverage.
* @internal — symmetric companion to rewriteStagedSkillBodies; the deep-seam API for
* command bodies. Production callers: applySurface (surface.cts) and the install path
* in createRuntimeArtifactInstallPlan (runtime-artifact-install-plan.cts) — both keep
* the returned temp dir alive until they have copied its contents out, then clean it up
* in their own finally. (A test that treats this as a throwaway shared-tmp path will
* race those live temp dirs under --test-concurrency; see #1575/#2090.)
*
* @returns {string} path to the temp dir (caller is responsible for cleanup)
*/

19
tests/_ff_lint_clean.cjs Normal file
View File

@@ -0,0 +1,19 @@
// PERMANENT LOAD-BEARING FIXTURE for #1259 / #2126 — DO NOT delete or rename to `*.test.cjs`.
//
// A KNOWN-CLEAN, lint-scoped `.cjs` companion to `_ff_lint_violation.cjs`. It has NO
// `local/no-source-grep` violation, so the prohibition-enforcement real-runner tests can use it as
// the "clean target" for a NON-VACUOUS pass — the rule RUNS on it (enabled via the flat-config
// block below) and finds nothing.
//
// Why a `.cjs` and not `src/clock.cts`: linting a `src/**/*.cts` file is type-aware
// (`recommendedTypeChecked` + `parserOptions.project`), which loads the whole `tsconfig.build.json`
// program on every eslint spawn (~2s, CPU-heavy). The real-runner tests spawn eslint repeatedly and,
// under `--test-concurrency`, those full-program type-checks oversubscribe the bench CPU and blow the
// 60s subprocess bound (#2126). A plain `.cjs` is linted non-type-aware (~0.8s) — same coverage of
// the AST-only `no-source-grep` rule, no starvation.
//
// PLAIN `.cjs` (NOT `*.test.cjs`) on purpose — same reason as the violation fixture: keep it OFF the
// `node --test` runner glob so it is only ever linted, never executed.
'use strict';
module.exports = {};

View File

@@ -13,11 +13,95 @@ const { describe, test, beforeEach, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const { execFileSync } = require('child_process');
const os = require('os');
const { cleanup } = require('./helpers.cjs');
const GSD_TOOLS = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs');
// In-process invocation, not execFileSync: cmdConfigGet is a pure CJS
// function reachable without spawning `node` as a child. The prior
// execFileSync(..., { timeout: 5000 }) raced a real subprocess's startup
// (full node boot + gsd-tools.cjs's large eager require graph — capability
// registry, phase/roadmap/agent/check/task routers, verify.cjs,
// cli-skew-check, findProjectRoot, etc.) against a fixed 5s wall clock, with
// no retry. Under Docker host contention that wall clock loses
// nondeterministically (ETIMEDOUT) — a test-harness race, not a product
// defect. bin/lib/config.cjs requires none of that dispatcher machinery, so
// calling cmdConfigGet directly removes the subprocess-spawn cost and the
// wall-clock race entirely: no timeout of any size can flake this.
const config = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'config.cjs'));
// io.cjs owns error()/output() and the JSON-error-mode toggle. cmdConfigGet's `error`
// is bound to io.error at load, so we drive io directly to (a) get structured stderr
// we can assert a typed `reason` on, and (b) restore the mode after each error probe.
const io = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'io.cjs'));
/**
* cmdConfigGet's error() path (gsd-core/bin/lib/io.cjs) calls process.exit(1)
* directly (it predates the ExitError/runMain seam used by the CLI
* entrypoint's non-error paths). Intercepting process.exit with a throwable
* sentinel lets the error path be exercised in-process without killing the
* test worker.
*
* The sentinel carries the ORIGINAL error message (not a generic "process.exit(1)").
* That matters for cmdConfigGet's "no config.json" branch, whose `error()` sits inside
* a try/catch that reclassifies any throw NOT starting with "No config.json" as a parse
* failure (a guard that is dead in production, where process.exit terminates first, but
* becomes live once process.exit is a throwing seam). Carrying the real message makes
* that guard re-throw — modeling the single, faithful production termination instead of
* a spurious second error() call with the wrong reason.
*/
class _ExitSignal extends Error {
constructor(code, message) {
super(message ?? `process.exit(${code})`);
this.code = code;
}
}
/**
* bin/lib/io.cjs's output()/error() write directly to the raw fd (1 or 2)
* via fs.writeSync — they bypass console.log entirely, so
* tests/helpers.cjs's captureConsole() cannot observe them (see
* tests/io.test.cjs: "output() writes directly to fd 1"). Monkeypatch
* fs.writeSync itself — save the original, override, restore in a finally,
* the project's standard IO capture/fault-injection seam — to capture what
* would have hit the fd.
*/
function captureFdWrite(fd, fn) {
const orig = fs.writeSync;
let captured = Buffer.alloc(0);
fs.writeSync = (writeFd, ...rest) => {
if (writeFd !== fd) return orig.call(fs, writeFd, ...rest);
const [data, offset = 0, length] = rest;
const chunk = Buffer.isBuffer(data)
? data.subarray(offset, offset + (length ?? data.length - offset))
: Buffer.from(String(data), 'utf8');
captured = Buffer.concat([captured, chunk]);
return chunk.length;
};
try {
fn();
} finally {
fs.writeSync = orig;
}
return captured.toString('utf-8');
}
/**
* Parse a CLI-style config-get argv (mirrors gsd-core/bin/gsd-tools.cjs's
* 'config-get' case: key is args[1], optional --default <value>, optional
* --raw) into cmdConfigGet's positional params. Keeps the test bodies below
* expressed in the same CLI-args vocabulary they always were.
*/
function parseConfigGetArgs(args) {
const rest = args.slice(1); // drop the leading 'config-get'
let raw = false;
let defaultValue;
const positional = [];
for (let i = 0; i < rest.length; i++) {
if (rest[i] === '--raw') { raw = true; continue; }
if (rest[i] === '--default') { defaultValue = rest[i + 1] ?? ''; i++; continue; }
positional.push(rest[i]);
}
return { keyPath: positional[0], raw, defaultValue };
}
describe('config-get --default flag (#1893)', () => {
let tmpDir;
@@ -34,10 +118,11 @@ describe('config-get --default flag (#1893)', () => {
});
function run(...args) {
return execFileSync('node', [GSD_TOOLS, ...args, '--cwd', tmpDir], {
encoding: 'utf-8',
timeout: 5000,
}).trim();
const { keyPath, raw, defaultValue } = parseConfigGetArgs(args);
const out = captureFdWrite(1, () => {
config.cmdConfigGet(tmpDir, keyPath, raw, defaultValue);
});
return out.trim();
}
function runRaw(...args) {
@@ -45,22 +130,55 @@ describe('config-get --default flag (#1893)', () => {
}
function runExpectError(...args) {
const { keyPath, raw, defaultValue } = parseConfigGetArgs(args);
const origExit = process.exit;
const origWriteSync = fs.writeSync;
io.setJsonErrorMode(true); // structured stderr line lets the sentinel carry the message + assert reason
let exitCount = 0;
let exitCode;
let stderr = '';
fs.writeSync = (fd, ...rest) => {
if (fd !== 2) return origWriteSync.call(fs, fd, ...rest);
const [data, offset = 0, length] = rest;
const chunk = Buffer.isBuffer(data)
? data.subarray(offset, offset + (length ?? data.length - offset)).toString('utf8')
: String(data);
stderr += chunk;
return Buffer.byteLength(chunk);
};
const lastError = () => {
const parts = stderr.split('\n').filter(Boolean);
try { return JSON.parse(parts[parts.length - 1]); } catch { return {}; }
};
process.exit = (code) => {
exitCount++;
exitCode = code;
// Carry the just-emitted error message so cmdConfigGet's seam guard re-throws
// (single, faithful fire) instead of catching + reclassifying into a 2nd error().
throw new _ExitSignal(code, lastError().message);
};
try {
execFileSync('node', [GSD_TOOLS, ...args, '--cwd', tmpDir], {
encoding: 'utf-8',
timeout: 5000,
stdio: ['pipe', 'pipe', 'pipe'],
});
assert.fail('Expected command to exit non-zero');
} catch (err) {
assert.ok(err.status !== 0, 'Expected non-zero exit code');
return err;
config.cmdConfigGet(tmpDir, keyPath, raw, defaultValue);
} catch (e) {
if (!(e instanceof _ExitSignal)) throw e;
} finally {
process.exit = origExit;
fs.writeSync = origWriteSync;
io.setJsonErrorMode(false);
}
assert.ok(exitCode !== 0 && exitCode !== undefined, 'Expected non-zero exit code');
// Faithfulness guard: production process.exit terminates, so error() fires exactly
// once. A count of 2 means the throwing-exit seam was caught + reclassified (the bug
// this harness redesign fixes) — fail loudly rather than report a wrong reason.
assert.equal(exitCount, 1, 'error() must fire exactly once (production process.exit terminates)');
const payload = lastError();
return { status: exitCode, reason: payload.reason, message: payload.message, stderr };
}
test('absent key without --default errors', () => {
fs.writeFileSync(path.join(planningDir, 'config.json'), '{}');
runExpectError('config-get', 'nonexistent.key', '--raw');
const { reason } = runExpectError('config-get', 'nonexistent.key', '--raw');
assert.equal(reason, io.ERROR_REASON.CONFIG_KEY_NOT_FOUND, 'absent key must report CONFIG_KEY_NOT_FOUND');
});
test('absent key with --default returns default value', () => {
@@ -99,7 +217,8 @@ describe('config-get --default flag (#1893)', () => {
test('missing config.json without --default errors', () => {
// No config.json written
runExpectError('config-get', 'any.key', '--raw');
const { reason } = runExpectError('config-get', 'any.key', '--raw');
assert.equal(reason, io.ERROR_REASON.CONFIG_NO_FILE, 'missing config.json must report CONFIG_NO_FILE');
});
test('--default works with JSON output (no --raw)', () => {

File diff suppressed because it is too large Load Diff

View File

@@ -98,6 +98,8 @@ describe('#1575 — golden-parity: surface path matches install path for descrip
installContent,
`${runtime}/${fileName}: surface content must be byte-identical to install content`,
);
}
});
}
test('cursor with non-undefined attribution: surface agents byte-identical to install agents (M2 coverage)', (t) => {
@@ -124,8 +126,6 @@ describe('#1575 — golden-parity: surface path matches install path for descrip
`cursor/${fileName}: content must be byte-identical with non-undefined attribution`);
}
});
});
}
test('copilot: agents installed as .agent.md (filename rename parity)', (t) => {
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1575-copilot-rename-'));

View File

@@ -716,13 +716,13 @@ describe('prohibition-enforcement REAL runner end-to-end (#1259)', () => {
const enforce = require(ENFORCEMENT_LIB);
// Migrated to the SHIPPING prover (#1279): the default real prover lints the committed
// `_ff_lint_violation.cjs` violationFixture (the rule fires -> fail-first proven) while the
// clean runCheck lints src/clock.cts (no violation -> non-vacuous pass). Both via real eslint.
// clean runCheck lints tests/_ff_lint_clean.cjs (no violation -> non-vacuous pass). Both via real eslint.
const result = enforce.runProhibitionEnforcement(
TEST_TIER,
{
kind: 'lint-rule',
rule: 'local/no-source-grep',
target: 'src/clock.cts',
target: 'tests/_ff_lint_clean.cjs',
failFirst: true,
violationFixture: path.join('tests', '_ff_lint_violation.cjs'),
},
@@ -759,13 +759,13 @@ describe('prohibition-enforcement REAL runner end-to-end (#1259)', () => {
const enforce = require(ENFORCEMENT_LIB);
// No injected runCheck/proveFailFirst: the default prover lints the committed
// `_ff_lint_violation.cjs` (the rule fires -> fail-first proven) AND the default runner lints
// the clean `src/clock.cts` (no violation -> non-vacuous pass). BOTH directions via real eslint.
// the clean `tests/_ff_lint_clean.cjs` (no violation -> non-vacuous pass). BOTH directions via real eslint.
const result = enforce.runProhibitionEnforcement(
TEST_TIER,
{
kind: 'lint-rule',
rule: 'local/no-source-grep',
target: 'src/clock.cts',
target: 'tests/_ff_lint_clean.cjs',
violationFixture: path.join('tests', '_ff_lint_violation.cjs'),
},
{ cwd: process.cwd() },
@@ -780,7 +780,7 @@ describe('prohibition-enforcement REAL runner end-to-end (#1259)', () => {
test('FULL producer (real): lint-rule hard-gates on a TOOTHLESS violationFixture (rule does not flag it) (FF-02 wrong-direction)', () => {
const enforce = require(ENFORCEMENT_LIB);
// The "violation fixture" is a CLEAN in-tree file (src/clock.cts) the rule does NOT flag, so the
// The "violation fixture" is a CLEAN in-tree file (tests/_ff_lint_clean.cjs) the rule does NOT flag, so the
// default prover cannot prove fail-first -> the producer must hard-gate (never green), even though
// the clean target itself would pass the runner. A toothless guard is not a guard.
const result = enforce.runProhibitionEnforcement(
@@ -788,8 +788,8 @@ describe('prohibition-enforcement REAL runner end-to-end (#1259)', () => {
{
kind: 'lint-rule',
rule: 'local/no-source-grep',
target: 'src/clock.cts',
violationFixture: 'src/clock.cts',
target: 'tests/_ff_lint_clean.cjs',
violationFixture: 'tests/_ff_lint_clean.cjs',
},
{ cwd: process.cwd() },
);
@@ -807,8 +807,8 @@ describe('prohibition-enforcement REAL runner end-to-end (#1259)', () => {
{
kind: 'lint-rule',
rule: 'local/no-source-grep',
target: 'src/clock.cts',
violationFixture: 'src/clock.cts',
target: 'tests/_ff_lint_clean.cjs',
violationFixture: 'tests/_ff_lint_clean.cjs',
},
{ cwd: process.cwd(), mode },
);
@@ -1009,11 +1009,11 @@ describe('prohibition-enforcement defaultProveFailFirst REAL prover (#1279)', ()
test('lint-rule: a CLEAN violationFixture (rule does not flag) is NOT proven (FF-02 toothless direction)', () => {
const enforce = require(ENFORCEMENT_LIB);
// src/clock.cts is a clean in-tree source with no no-source-grep violation. If a "violation
// tests/_ff_lint_clean.cjs is a clean in-tree source with no no-source-grep violation. If a "violation
// fixture" does not actually trigger the rule, the rule is toothless on it → not a guard → not
// proven → must hard-gate.
const proof = enforce.defaultProveFailFirst(
{ kind: 'lint-rule', rule: 'local/no-source-grep', target: 'src/clock.cts', violationFixture: 'src/clock.cts' },
{ kind: 'lint-rule', rule: 'local/no-source-grep', target: 'tests/_ff_lint_clean.cjs', violationFixture: 'tests/_ff_lint_clean.cjs' },
process.cwd(),
);
assert.equal(proof.provenFailFirst, false,
@@ -1023,7 +1023,7 @@ describe('prohibition-enforcement defaultProveFailFirst REAL prover (#1279)', ()
test('lint-rule: no violationFixture -> not proven (FF-05 fail-closed)', () => {
const enforce = require(ENFORCEMENT_LIB);
const proof = enforce.defaultProveFailFirst(
{ kind: 'lint-rule', rule: 'local/no-source-grep', target: 'src/clock.cts' }, // no violationFixture
{ kind: 'lint-rule', rule: 'local/no-source-grep', target: 'tests/_ff_lint_clean.cjs' }, // no violationFixture
process.cwd(),
);
assert.equal(proof.provenFailFirst, false, 'no violationFixture -> cannot prove -> hard-gate');

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,85 @@ 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');
});
test('project-code-prefixed checklist-only entry surfaces malformed_roadmap for both query forms', () => {
// #2121/#2114 route all three resolvers through the shared 3-source lookup. A
// `**Phase PROJ-42:**` summary line with no matching `### Phase PROJ-42:` detail heading
// is a malformed ROADMAP. Before the consolidation this project-code-prefixed checklist
// was reported as a silent `{found:false}` for BOTH query forms — the prefixed pass
// discarded its malformed candidate, and the bare pass could not match the `PROJ-` prefix
// at all. The unified lookup newly surfaces the malformed_roadmap diagnostic for both, so
// this test fails on the prior silent-empty behavior for the prefixed AND the bare form.
fs.writeFileSync(
path.join(tmpDir, '.planning', 'ROADMAP.md'),
[
'# Roadmap v1.0',
'',
'## Phases',
'',
'- [ ] **Phase PROJ-42: Checklist only, no header**',
'',
].join('\n'),
);
const prefixed = runGsdTools('roadmap get-phase PROJ-42 --json', tmpDir);
assert.ok(prefixed.success, `command failed: ${prefixed.error || prefixed.output}`);
const pPayload = JSON.parse(prefixed.output);
assert.strictEqual(pPayload.found, false, 'malformed roadmap: phase must not be found');
assert.strictEqual(pPayload.error, 'malformed_roadmap', 'prefixed query must surface malformed_roadmap');
assert.ok(pPayload.message.includes('missing'), 'message must explain the missing detail section');
// Parity: the bare numeric form yields the same diagnostic against the same fixture.
const bare = runGsdTools('roadmap get-phase 42 --json', tmpDir);
assert.ok(bare.success, `command failed: ${bare.error || bare.output}`);
assert.strictEqual(JSON.parse(bare.output).error, 'malformed_roadmap', 'bare query surfaces the same diagnostic');
});
});
describe('roadmap annotate-dependencies', () => {
let tmpDir;