diff --git a/.changeset/2104-foreign-prefix-sibling-commands.md b/.changeset/2104-foreign-prefix-sibling-commands.md new file mode 100644 index 000000000..5ad473b35 --- /dev/null +++ b/.changeset/2104-foreign-prefix-sibling-commands.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 0 +--- +**`init execute-phase`, `init verify-work`, and `init phase-op` no longer collapse foreign-prefixed task IDs to numeric phases** — `MEM-01` under `project_code: LKML` was silently stripped to `01` and resolved to the unrelated numeric Phase 01, because the #2056 guard was applied only to `init plan-phase`. The guard is now extracted into shared helpers (`guardedFindPhase` / `guardedGetRoadmapPhase`) that delegate to the canonical `isForeignPrefixedPhaseQuery` from `phase-id.cts`, and all four init commands route through them. (#2104) diff --git a/src/init.cts b/src/init.cts index 9b791316e..4b37d2968 100644 --- a/src/init.cts +++ b/src/init.cts @@ -73,7 +73,7 @@ const { extractCurrentMilestone, } = roadmapParser; const { pathExistsInternal, generateSlugInternal, toPosixPath } = coreUtils; -const { escapeRegex, normalizePhaseName, phaseTokenMatches, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE } = phaseId; +const { escapeRegex, normalizePhaseName, phaseTokenMatches, stripProjectCodePrefix, PHASE_NUMBER_TOKEN_SOURCE, isForeignPrefixedPhaseQuery } = phaseId; const { pruneOrphanedWorktrees } = worktreeSafety; const { @@ -96,25 +96,10 @@ void stripShippedMilestones; // Accept all bold/colon variants of the Requirements header (#2769) const REQUIREMENTS_HEADER_RE = /^\*\*Requirements:?\*\*[^\S\n]*:?[^\S\n]*([^\n]*)$/m; -// #2056: normalizePhaseName() strips ANY [A-Z][A-Z0-9_]*- prefix as a project -// code (e.g. 'MEM-01' → '01'), so a foreign-prefixed workstream/task id would -// collapse to its numeric suffix and resolve to an unrelated numeric phase via -// the dir/roadmap fallback. init plan-phase must require EXACT prefixed -// evidence — a phase dir whose token literally IS the prefixed query, or a -// roadmap entry literally headed with it — before accepting such a match. The -// configured project_code's own prefix (e.g. LKML-01 under project_code: LKML) -// is never foreign and always passes through. -function parsePhasePrefix(phase: unknown): string | null { - const match = String(phase).match(/^([A-Z][A-Z0-9_]*)-(?=\d)/i); - return match ? match[1] : null; -} - -function isForeignPrefixedPhaseQuery(phase: unknown, projectCode: unknown): boolean { - const prefix = parsePhasePrefix(phase); - if (!prefix) return false; - const configured = typeof projectCode === 'string' ? projectCode.trim() : ''; - return !configured || prefix.toUpperCase() !== configured.toUpperCase(); -} +// #2056/#2104: isForeignPrefixedPhaseQuery is imported from phase-id.cts +// (the canonical predicate). parsePhasePrefix is no longer needed locally. +// phaseInfoMatchesExactPrefix and roadmapPhaseMatchesExactPrefix are local +// helpers that post-filter the lookup results for foreign-prefix queries. function phaseInfoMatchesExactPrefix( phaseInfo: Record | null, @@ -134,6 +119,33 @@ function roadmapPhaseMatchesExactPrefix( return new RegExp(`^#{2,4}\\s*Phase\\s+${escapeRegex(phase)}(?:\\b|\\s|:)`, 'i').test(section); } +// #2104: shared helpers that wrap findPhaseInternal / getRoadmapPhaseInternal +// with the #2056 foreign-prefix guard, so every init command gets the same +// protection without duplicating the guard logic at each call site. +function guardedFindPhase( + cwd: string, + phase: string, + projectCode: unknown, +): Record | null { + let phaseInfo = findPhaseInternal(cwd, phase) as unknown as Record | null; + if (isForeignPrefixedPhaseQuery(phase, projectCode) && !phaseInfoMatchesExactPrefix(phaseInfo, phase)) { + phaseInfo = null; + } + return phaseInfo; +} + +function guardedGetRoadmapPhase( + cwd: string, + phase: string, + projectCode: unknown, +): Record | null { + let roadmapPhase = getRoadmapPhaseInternal(cwd, phase) as unknown as Record | null; + if (isForeignPrefixedPhaseQuery(phase, projectCode) && !roadmapPhaseMatchesExactPrefix(roadmapPhase, phase)) { + roadmapPhase = null; + } + return roadmapPhase; +} + function listPhaseSummaryFiles(phaseDir: string): string[] { return (scanPhasePlans(phaseDir) as unknown as Record)['summaryFiles']; } @@ -334,10 +346,10 @@ function cmdInitExecutePhase( } const config = loadConfig(cwd); - let phaseInfo = findPhaseInternal(cwd, phase) as unknown as Record | null; + let phaseInfo = guardedFindPhase(cwd, phase, config.project_code); const milestone = getMilestoneInfo(cwd) as unknown as Record; - const roadmapPhase = getRoadmapPhaseInternal(cwd, phase) as unknown as Record | null; + const roadmapPhase = guardedGetRoadmapPhase(cwd, phase, config.project_code); if (phaseInfo?.['archived'] && roadmapPhase?.['found']) { phaseInfo = null; @@ -468,19 +480,9 @@ function cmdInitPlanPhase( } const config = loadConfig(cwd); - const foreignPrefixedPhase = isForeignPrefixedPhaseQuery(phase, config.project_code); - let phaseInfo = findPhaseInternal(cwd, phase) as unknown as Record | null; - // #2056: a foreign-prefixed query must only match a phase whose token literally - // IS the prefixed query (e.g. a real 'MEM-01-*' dir); otherwise the numeric - // fallback ('MEM-01' → '01') would resolve to the unrelated numeric phase. - if (foreignPrefixedPhase && !phaseInfoMatchesExactPrefix(phaseInfo, phase)) { - phaseInfo = null; - } - - let roadmapPhase = getRoadmapPhaseInternal(cwd, phase) as unknown as Record | null; - if (foreignPrefixedPhase && !roadmapPhaseMatchesExactPrefix(roadmapPhase, phase)) { - roadmapPhase = null; - } + // #2056/#2104: foreign-prefixed queries must not collapse to numeric phases. + let phaseInfo = guardedFindPhase(cwd, phase, config.project_code); + const roadmapPhase = guardedGetRoadmapPhase(cwd, phase, config.project_code); if (phaseInfo?.['archived'] && roadmapPhase?.['found']) { phaseInfo = null; @@ -901,17 +903,17 @@ function cmdInitVerifyWork(cwd: string, phase: string, raw: boolean): void { const config = loadConfig(cwd); const _slashRuntime = resolveRuntime(cwd); - let phaseInfo = findPhaseInternal(cwd, phase) as unknown as Record | null; + let phaseInfo = guardedFindPhase(cwd, phase, config.project_code); if (phaseInfo?.['archived']) { - const roadmapPhase = getRoadmapPhaseInternal(cwd, phase) as unknown as Record | null; + const roadmapPhase = guardedGetRoadmapPhase(cwd, phase, config.project_code); if (roadmapPhase?.['found']) { phaseInfo = null; } } if (!phaseInfo) { - const roadmapPhase = getRoadmapPhaseInternal(cwd, phase) as unknown as Record | null; + const roadmapPhase = guardedGetRoadmapPhase(cwd, phase, config.project_code); if (roadmapPhase?.['found']) { const phaseName = roadmapPhase['phase_name'] as string | null; phaseInfo = { @@ -974,10 +976,10 @@ function cmdInitVerifyWork(cwd: string, phase: string, raw: boolean): void { function cmdInitPhaseOp(cwd: string, phase: string, raw: boolean): void { const config = loadConfig(cwd); - let phaseInfo = findPhaseInternal(cwd, phase) as unknown as Record | null; + let phaseInfo = guardedFindPhase(cwd, phase, config.project_code); if (phaseInfo?.['archived']) { - const roadmapPhase = getRoadmapPhaseInternal(cwd, phase) as unknown as Record | null; + const roadmapPhase = guardedGetRoadmapPhase(cwd, phase, config.project_code); if (roadmapPhase?.['found']) { const phaseName = roadmapPhase['phase_name'] as string | null; phaseInfo = { @@ -999,7 +1001,7 @@ function cmdInitPhaseOp(cwd: string, phase: string, raw: boolean): void { } if (!phaseInfo) { - const roadmapPhase = getRoadmapPhaseInternal(cwd, phase) as unknown as Record | null; + const roadmapPhase = guardedGetRoadmapPhase(cwd, phase, config.project_code); if (roadmapPhase?.['found']) { const phaseName = roadmapPhase['phase_name'] as string | null; phaseInfo = { diff --git a/tests/init.test.cjs b/tests/init.test.cjs index 99b6412c7..635050878 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -177,6 +177,106 @@ describe('init commands', () => { assert.strictEqual(output.phase_number, null); }); + // #2104: the #2056 guard must also cover init execute-phase, verify-work, + // and phase-op — all three had unguarded findPhaseInternal/getRoadmapPhaseInternal + // calls that collapsed foreign prefixes (MEM-01 → 01) to numeric phases. + test('#2104 — init execute-phase does not collapse foreign-prefixed task IDs', () => { + seedPhase(tmpDir, '01-stable-baseline-on-main', { + '01-CONTEXT.md': '# Phase Context', + }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '# Roadmap\n\n### Phase 1: Stable Baseline On Main\n**Goal:** Establish baseline\n**Plans:** 1 plan\n', + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ project_code: 'LKML' }, null, 2), + ); + + const result = runGsdTools('init execute-phase MEM-01', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, false, 'MEM-01 must NOT resolve to numeric Phase 01'); + assert.strictEqual(output.phase_number, null); + }); + + test('#2104 — init verify-work does not collapse foreign-prefixed task IDs', () => { + seedPhase(tmpDir, '01-stable-baseline-on-main', { + '01-CONTEXT.md': '# Phase Context', + }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '# Roadmap\n\n### Phase 1: Stable Baseline On Main\n**Goal:** Establish baseline\n**Plans:** 1 plan\n', + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ project_code: 'LKML' }, null, 2), + ); + + const result = runGsdTools('init verify-work MEM-01', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, false, 'MEM-01 must NOT resolve to numeric Phase 01'); + }); + + // #2104 accept-branch: a real foreign-prefixed phase dir MUST still resolve + // through the now-guarded commands, proving the guard narrows but does not block. + test('#2104 — init execute-phase resolves a real foreign-prefixed phase dir', () => { + seedPhase(tmpDir, 'MEM-01-integration', { + 'MEM-01-CONTEXT.md': '# Phase Context', + }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ project_code: 'LKML' }, null, 2), + ); + + const result = runGsdTools('init execute-phase MEM-01', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, true, 'a real MEM-01-* dir must resolve under its own prefix'); + assert.strictEqual(output.phase_dir, '.planning/phases/MEM-01-integration'); + }); + + test('#2104 — init verify-work resolves a real foreign-prefixed phase dir', () => { + seedPhase(tmpDir, 'MEM-01-integration', { + 'MEM-01-CONTEXT.md': '# Phase Context', + }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ project_code: 'LKML' }, null, 2), + ); + + const result = runGsdTools('init verify-work MEM-01', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, true, 'a real MEM-01-* dir must resolve under its own prefix'); + }); + + test('#2104 — init phase-op does not collapse foreign-prefixed task IDs', () => { + seedPhase(tmpDir, '01-stable-baseline-on-main', { + '01-CONTEXT.md': '# Phase Context', + }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + '# Roadmap\n\n### Phase 1: Stable Baseline On Main\n**Goal:** Establish baseline\n**Plans:** 1 plan\n', + ); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ project_code: 'LKML' }, null, 2), + ); + + const result = runGsdTools('init phase-op MEM-01', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, false, 'MEM-01 must NOT resolve to numeric Phase 01'); + assert.strictEqual(output.phase_number, null); + }); + test('init plan-phase exposes text_mode from config (defaults false)', () => { const result = runGsdTools('init plan-phase 03', tmpDir); assert.ok(result.success, `Command failed: ${result.error}`);