diff --git a/src/state.cts b/src/state.cts index 37b859091..a4778f1c1 100644 --- a/src/state.cts +++ b/src/state.cts @@ -2713,9 +2713,13 @@ function resolvePhaseIdForCompletePhase(content: string, overridePhase: string | stateExtractField(content, 'Phase') || ''; - // Accept canonical phase token only (e.g. 3, 03, 3A, 3.3, 10.2) - const phaseMatch = String(candidate).match(/(\d+[A-Z]?(?:\.\d+)*)/i); - return phaseMatch ? phaseMatch[1] : null; + // #2125: parse via the canonical anchored parser so a narrative `Phase:` + // body line (e.g. "Milestone v0.5 complete") does not mine a bogus token — + // the old unanchored regex yielded "0.5" and rewrote STATE.md as + // "Phase 0.5 complete". A canonical token at the start of the value + // (3, 03, 3A, 3.3, 10.2, "3 of 5", "1 — Setup") is preserved; a milestone + // closure line yields null, so the caller's "unable to resolve" guard fires. + return parsePhaseFromProse(candidate).phase; } function cmdStateCompletePhase(cwd: string, raw: boolean, overridePhase?: string): void { @@ -2742,8 +2746,9 @@ function cmdStateCompletePhase(cwd: string, raw: boolean, overridePhase?: string // The handler is now a no-op in that case so re-invocation from downstream // workflows cannot regress the project state. const existingCurrentPhaseRaw = stateExtractField(content, 'Current Phase') || ''; - const existingCurrentPhaseMatch = String(existingCurrentPhaseRaw).match(/(\d+[A-Z]?(?:\.\d+)*)/i); - const existingCurrentPhase = existingCurrentPhaseMatch ? existingCurrentPhaseMatch[1] : null; + // #2125: same canonical parser as resolvePhaseIdForCompletePhase so the two + // sites cannot diverge on the token they extract. + const existingCurrentPhase = parsePhaseFromProse(existingCurrentPhaseRaw).phase; if (existingCurrentPhase && existingCurrentPhase !== resolvedPhase) { output( { updated: [], phase: resolvedPhase, idempotent: true, note: 'phase already superseded; no-op' }, diff --git a/tests/state.test.cjs b/tests/state.test.cjs index 7167579e6..94163ec52 100644 --- a/tests/state.test.cjs +++ b/tests/state.test.cjs @@ -3055,6 +3055,39 @@ describe('state complete-phase: decorated Phase fallback (#2761 nitpick)', () => assert.ok(!after.includes('Status: Phase Phase complete')); }); + test('rejects a milestone-closure Phase line, never mines the version token (#2111 / #2125)', () => { + // After `milestone complete v0.5`, the only phase signal is the narrative + // `Phase: Milestone v0.5 complete`. The old unanchored resolver mined "0.5" + // and rewrote Status as "Phase 0.5 complete"; the anchored parser yields no + // token, so complete-phase must reject rather than corrupt STATE.md. + const stateMd = [ + '---', + 'milestone: v0.5', + '---', + '', + '# State', + '', + '**Status:** Awaiting next milestone', + '**Last Activity:** 2024-01-15', + '', + '## Current Position', + '', + 'Phase: Milestone v0.5 complete', + '', + ].join('\n'); + const statePath = path.join(tmpDir, '.planning', 'STATE.md'); + fs.writeFileSync(statePath, stateMd); + + const result = runGsdTools('state complete-phase', tmpDir); + assert.ok(result.success, 'command should return JSON error payload, not crash'); + const output = JSON.parse(result.output); + assert.ok(output.error, 'expected a resolution error, not a phase mined from the version string'); + + const after = fs.readFileSync(statePath, 'utf-8'); + assert.ok(!after.includes('Phase 0.5 complete'), `must not mine "0.5" from the version: ${after}`); + assert.ok(!after.includes('Phase: 0.5'), `must not rewrite Current Position to Phase 0.5: ${after}`); + }); + test('supports explicit phase override for complete-phase disambiguation (#3063)', () => { const stateMd = [ '---',