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

fix(#2125): migrate state.cts prose parsing to canonical parser (drives #2111)

Browse Source 
Phase 2 of epic #2121. state.cts:parseProsePhaseField now delegates to the
anchored phase-id.cts:parsePhaseFromProse (built in Phase 1), removing this
module's independent prose phase-id regex.

Drives #2111: `milestone complete vX.Y` no longer corrupts current_phase. The
body line "Phase: Milestone v0.5 complete" previously had "5" mined from it by
the unanchored regex; the anchored parser returns { phase: null }, so
syncStateFrontmatter's #905 guard preserves the real current_phase. This also
fixes the broader family the review surfaced — every milestone completion
(e.g. v1.0 -> "0") was silently corrupting current_phase, not just .5-versions.

Regression (tests/milestone.test.cjs, in the milestone-complete suite, #2111):
`milestone complete v0.5` on a project with current_phase: "19" now preserves
"19". Demonstrated fail-first end-to-end: reverting the migration reproduces
current_phase = "5".

Closes #2125
Refs #2121

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-07-09 17:57:47 -04:00
parent 960c3eaa15
commit e7ff7be1a4
2 changed files with 37 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
src/state.cts
View File

@@ -16,7 +16,7 @@ import configLoaderMod = require('./config-loader.cjs');
const { loadConfig } = configLoaderMod;
// eslint-disable-next-line @typescript-eslint/no-require-imports
import phaseIdMod = require('./phase-id.cjs');
const { escapeRegex, normalizePhaseName, extractPhaseToken } = phaseIdMod;
const { escapeRegex, normalizePhaseName, extractPhaseToken, parsePhaseFromProse } = phaseIdMod;
// eslint-disable-next-line @typescript-eslint/no-require-imports
import roadmapParserMod = require('./roadmap-parser.cjs');
const { getMilestoneInfo, getMilestonePhaseFilter, extractCurrentMilestone } = roadmapParserMod;
@@ -1116,22 +1116,13 @@ function matchSessionSection(body: string): RegExpMatchArray | null {
}
function parseProsePhaseField(value: string | null): { phase: string | null; name: string | null } {
if (!value) return { phase: null, name: null };
const phaseMatch = value.match(/\b(\d+[A-Z]?(?:\.\d+)*)\b/i);
// #2124 review: length-bound the name quantifiers so a crafted long
// unterminated `(` / `—` run in an untrusted STATE.md field cannot drive
// O(n^2) backtracking (CPU DoS). (Phase 2 / #2125 supersedes this function
// by delegating to phase-id.cts:parsePhaseFromProse, which is bounded too.)
const parenName = value.match(/\(([^)]{1,200})\)/);
const dashName = value.match(/—\s*([^(\n]{1,200}?)(?:\s*\(|$)/);
const rawName = parenName?.[1] ?? dashName?.[1] ?? null;
const name = rawName && !/^(?:complete|executing|not started)$/i.test(rawName.trim())
? rawName.trim()
: null;
return {
phase: phaseMatch ? phaseMatch[1] : null,
name,
};
// #2121 Phase 2 (#2125): delegate to the canonical anchored parser so this
// module holds no independent prose phase-id regex. Drives #2111 — the
// anchored parser returns { phase: null } for a "Milestone vX.Y complete"
// body line (the old unanchored regex mined the minor-version digit, e.g.
// v0.5 -> "5"), so syncStateFrontmatter's #905 guard preserves the real
// current_phase instead of clobbering it.
return parsePhaseFromProse(value);
}
function parseProseLastActivityField(value: string | null): { date: string | null; description: string | null } {

30
tests/milestone.test.cjs
View File

@@ -16,7 +16,7 @@ const { test, describe, before, beforeEach, afterEach } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
const { runGsdTools, createTempProject, cleanup, parseFrontmatter } = require('./helpers.cjs');
// ─── helpers ─────────────────────────────────────────────────────────────────
@@ -54,6 +54,34 @@ describe('milestone complete command', () => {
beforeEach(() => { tmpDir = createTempProject(); });
afterEach(() => { cleanup(tmpDir); });
test('preserves current_phase frontmatter through milestone complete (#2111)', () => {
// Seed STATE.md mid-phase-19: the real current_phase lives in frontmatter
// and the only body phase source is the `Phase:` prose line (no explicit
// `Current Phase:` field — matching what milestoneCompleteCore writes).
fs.writeFileSync(
path.join(tmpDir, '.planning', 'STATE.md'),
`---\ncurrent_phase: "19"\n---\n# State\n\n**Status:** In progress\n` +
`**Last Activity:** 2025-01-01\n**Last Activity Description:** Working\n\n` +
`## Current Position\n\nPhase: 19 — EXECUTING\nPlan: 1 of 1\n` +
`Status: Executing\nLast activity: 2025-01-01 — Running phase\n`,
);
// No ROADMAP.md — mirrors 'handles missing ROADMAP.md gracefully' so the
// milestone-phase-filter guard never fires.
const result = runGsdTools('milestone complete v0.5 --name Test', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
const fm = parseFrontmatter(state);
// Fails pre-migration: the unanchored parser mined "5" from
// "Phase: Milestone v0.5 complete" and clobbered current_phase.
assert.strictEqual(
fm.current_phase, '19',
`current_phase must be preserved across milestone complete, not mined from ` +
`the version string (#2111); got ${JSON.stringify(fm.current_phase)}`,
);
});
test('archives roadmap, requirements, creates MILESTONES.md', () => {
writeRoadmap(tmpDir, `# Roadmap v1.0 MVP\n\n### Phase 1: Foundation\n**Goal:** Setup\n`);
fs.writeFileSync(