diff --git a/.changeset/1761-state-json-unbounded-milestone-read-path.md b/.changeset/1761-state-json-unbounded-milestone-read-path.md new file mode 100644 index 000000000..bd1c4f043 --- /dev/null +++ b/.changeset/1761-state-json-unbounded-milestone-read-path.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1818 +--- +**`gsd-tools state json` no longer reports conflated progress for an unversioned milestone (#1761)** — the ADR-1769 Phase 7 fix (#1794) taught `state sync` to leave Progress untouched when a milestone version is asserted but the ROADMAP has no versioned heading for it, but the `state json` **read** path still rebuilt progress via `buildStateFrontmatter`, whose phase-heading count fell back to the whole document and summed sibling milestones. `state json` therefore reported a conflated `total_phases` (e.g. 8 = 4+4 across two milestones) plus a derived `percent`, contradicting the sync guard on the very same project. The read path now mirrors the sync guard: when the asserted milestone cannot be bounded to a versioned ROADMAP heading, `total_phases` falls back to the on-disk phase-dir count and `percent` is omitted. Bounded milestones (versioned ROADMAP, or no milestone asserted) are unchanged; the signal rides on the existing `_diskScanCache` so `extractCurrentMilestone`'s return contract and its other callers are untouched. diff --git a/src/state.cts b/src/state.cts index ac9df0f5e..5f26d8dcb 100644 --- a/src/state.cts +++ b/src/state.cts @@ -144,6 +144,7 @@ const _diskScanCache = new Map(); // Track all lock files held by this process so they can be removed on exit. @@ -1359,6 +1360,9 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re let completedPhases: number | null = null; let totalPlans: number | null = totalPlansRaw ? parseInt(totalPlansRaw, 10) : null; let completedPlans: number | null = null; + // #1761 read-path: set from cached.milestoneBounded inside the disk-scan + // block; consumed at the percent computation to mirror the cmdStateSync guard. + let milestoneUnbounded = false; if (cwd) { try { @@ -1373,10 +1377,11 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re // exclusion (#1514). Computed before the disk scan so retired phases // can be dropped from the dir set too. let roadmapScope: string | null = null; + let roadmapRaw: string | null = null; let retiredPhaseNums = new Set(); try { const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); - const roadmapRaw = platformReadSync(roadmapPath); + roadmapRaw = platformReadSync(roadmapPath); if (roadmapRaw !== null) { roadmapScope = extractCurrentMilestone(roadmapRaw, cwd); retiredPhaseNums = extractRetiredPhaseNumbers(roadmapScope); @@ -1449,20 +1454,39 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re } } - cached = { - totalPhases: roadmapPhaseCount > 0 - ? Math.max(phaseDirs.length, roadmapPhaseCount) - : phaseDirs.length, - completedPhases: diskCompletedPhases, - totalPlans: diskTotalPlans, - completedPlans: diskTotalSummaries, - }; + cached = (() => { + // #1761 read-path: mirror the cmdStateSync guard (#1794). When the + // asserted milestone version can't be bounded to a versioned ROADMAP + // heading, extractCurrentMilestone falls back to the whole document + // and roadmapPhaseCount conflates sibling milestones. In that case + // don't substitute the whole-doc count — fall back to the on-disk + // phase-dir count only, and mark unbounded so percent is skipped + // downstream (mirrors the sync write-path guard). + let milestoneBounded = true; + if (milestone && roadmapRaw !== null) { + const versionedHeading = new RegExp( + `^#{1,3}\\s+(?!Phase\\s+\\S).*${escapeRegex(String(milestone).trim())}`, + 'mi', + ); + milestoneBounded = versionedHeading.test(roadmapRaw); + } + return { + totalPhases: (!milestoneBounded || roadmapPhaseCount === 0) + ? phaseDirs.length + : Math.max(phaseDirs.length, roadmapPhaseCount), + milestoneBounded, + completedPhases: diskCompletedPhases, + totalPlans: diskTotalPlans, + completedPlans: diskTotalSummaries, + }; + })(); _diskScanCache.set(cwd, cached); } totalPhases = cached.totalPhases; completedPhases = cached.completedPhases; totalPlans = cached.totalPlans; completedPlans = cached.completedPlans; + milestoneUnbounded = cached.milestoneBounded === false; } } catch { /* intentionally empty */ } } @@ -1473,7 +1497,10 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re // instead of a false 100% from plan-only coverage (#3242 Bug B). // Falls back to the body Progress: field only when no plan files exist on disk. let progressPercent = computeProgressPercent(completedPlans, totalPlans, completedPhases, totalPhases); - if (progressPercent === null && progressRaw) { + // #1761 read-path: when the milestone can't be bounded, percent would be + // derived from a conflated/understated total — skip it (mirror cmdStateSync). + if (milestoneUnbounded) progressPercent = null; + if (progressPercent === null && progressRaw && !milestoneUnbounded) { const pctMatch = progressRaw.match(/(\d+)%/); if (pctMatch) progressPercent = parseInt(pctMatch[1], 10); } diff --git a/tests/bug-1761-state-sync-wrong-progress.test.cjs b/tests/bug-1761-state-sync-wrong-progress.test.cjs index a5ebbe412..c5cb1eec5 100644 --- a/tests/bug-1761-state-sync-wrong-progress.test.cjs +++ b/tests/bug-1761-state-sync-wrong-progress.test.cjs @@ -87,3 +87,116 @@ describe('#1761: state sync leaves Progress untouched when milestone is unbounde `Progress must be left untouched when the milestone is unbounded; before=${JSON.stringify(before)} after=${JSON.stringify(after)} (#1761)`); }); }); + +// #1761 read-path: the ADR-1769 Phase 7 fix (#1794) closed the `state sync` +// WRITE path, but `state json` (the READ path) rebuilds progress via +// buildStateFrontmatter, whose roadmapPhaseCount loop counts phase headings +// across the WHOLE document when extractCurrentMilestone can't bound the +// asserted milestone. Result: state json reported a conflated total_phases +// (sum of sibling milestones) + a derived percent, contradicting the sync +// guard. This block mirrors the write-path guard on the read path. +describe('#1761 read-path: state json does not conflate progress when milestone is unbounded', () => { + let tmpDir; + beforeEach(() => { tmpDir = createTempProject(); }); + afterEach(() => { cleanup(tmpDir); }); + + test('state json omits percent and does NOT report the conflated whole-doc total_phases', () => { + // Repro from the issue: STATE.md asserts milestone: v2.0; ROADMAP has two + // UNVERSIONED sibling milestones (4 + 4 phases) — neither matches v2.0, so + // the milestone is unbounded. One summarized phase dir on disk. + const statePath = path.join(tmpDir, '.planning', 'STATE.md'); + fs.writeFileSync(statePath, [ + '---', + 'gsd_state_version: 1.0', + 'milestone: v2.0', + 'milestone_name: Second', + 'current_phase: "2"', + 'status: executing', + '---', + '', + '# GSD State', + '**Current Phase:** 2', + '**Status:** Executing Phase 2', + '', + ].join('\n')); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), [ + '# ROADMAP', + '## Milestone 1: First Milestone', + '### Phase 1: a', + '### Phase 2: b', + '### Phase 3: c', + '### Phase 4: d', + '## Milestone 2: Second Milestone', + '### Phase 5: e', + '### Phase 6: f', + '### Phase 7: g', + '### Phase 8: h', + '', + ].join('\n')); + // One summarized phase dir on disk. + const dir01 = path.join(tmpDir, '.planning', 'phases', '01'); + fs.mkdirSync(dir01, { recursive: true }); + fs.writeFileSync(path.join(dir01, '01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(dir01, '01-SUMMARY.md'), '# Summary\n'); + + const result = runGsdTools('state json --raw', tmpDir); + assert.ok(result.success, `state json failed: ${result.error}`); + const out = JSON.parse(result.output); + + // BEFORE the fix this printed progress.total_phases: 8 (4+4 sibling + // milestones) and percent: 13 — exactly the conflated read-path the sync + // guard was added to prevent. + assert.ok( + out.progress === undefined || out.progress.percent === undefined, + `state json must omit percent when the milestone is unbounded; got progress=${JSON.stringify(out.progress)}`, + ); + assert.ok( + !(out.progress && out.progress.total_phases === 8), + `state json must NOT report the conflated whole-doc total_phases (8 = 4+4 sibling milestones); got total_phases=${out.progress && out.progress.total_phases}`, + ); + }); + + test('state json still reports percent + total_phases when the milestone IS bounded (versioned ROADMAP)', () => { + // Control: a versioned ROADMAP heading matching the asserted milestone + // keeps the read path unchanged — the guard only fires when unbounded. + const statePath = path.join(tmpDir, '.planning', 'STATE.md'); + fs.writeFileSync(statePath, [ + '---', + 'gsd_state_version: 1.0', + 'milestone: v1.0', + 'milestone_name: First', + 'current_phase: "1"', + 'status: executing', + '---', + '', + '# GSD State', + '**Current Phase:** 1', + '**Status:** Executing Phase 1', + '', + ].join('\n')); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), [ + '# ROADMAP', + '## Milestone 1: First Milestone v1.0', + '### Phase 1: a', + '### Phase 2: b', + '', + ].join('\n')); + const dir01 = path.join(tmpDir, '.planning', 'phases', '01'); + fs.mkdirSync(dir01, { recursive: true }); + fs.writeFileSync(path.join(dir01, '01-PLAN.md'), '# Plan\n'); + fs.writeFileSync(path.join(dir01, '01-SUMMARY.md'), '# Summary\n'); + + const result = runGsdTools('state json --raw', tmpDir); + assert.ok(result.success, `state json failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.ok( + out.progress && typeof out.progress.percent === 'number', + `state json must report a numeric percent when the milestone is bounded; got progress=${JSON.stringify(out.progress)}`, + ); + assert.strictEqual( + out.progress.total_phases, + 2, + 'bounded read path must report the versioned milestone phase count (2)', + ); + }); +});