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

fix(#1761): skip conflated progress in state json read-path when milestone unbounded (#1818)

Browse Source 
* fix(#1761): skip conflated progress in state json read-path when milestone unbounded

ADR-1769 Phase 7 (#1794) closed the state sync WRITE path — when a milestone
version is asserted in frontmatter but the ROADMAP has no versioned heading
for it, sync leaves Progress untouched. But the state json READ path rebuilds
progress via buildStateFrontmatter, whose roadmapPhaseCount loop counts phase
headings across the WHOLE document when extractCurrentMilestone can't bound
the milestone. state json therefore reported a conflated total_phases (sum of
sibling milestones) + a derived percent — exactly the value the sync guard
was added to prevent. (Repro from the issue: total_phases 8 = 4+4, percent 13.)

Mirror the cmdStateSync guard inside buildStateFrontmatter: when the asserted
milestone cannot be bounded to a versioned ROADMAP heading (the same
versionedHeading test the sync path uses), fall back to the on-disk
phase-dir count for total_phases and skip percent. Bounded milestones
(versioned ROADMAP, or no milestone asserted) are unchanged. The signal rides
on the existing _diskScanCache (new milestoneBounded field) so neither
extractCurrentMilestone's return contract nor its other callers change.

Regression: extend tests/bug-1761-state-sync-wrong-progress.test.cjs with the
read-path case (unbounded → no percent, no conflated total_phases) and a
bounded control (versioned ROADMAP → unchanged percent + total_phases).

* docs(#1761): add changeset fragment for state json read-path fix
This commit is contained in:
Tom Boucher
2026-06-28 22:41:38 -04:00
committed by GitHub
parent a47979bb92
commit 38c2c1805e
3 changed files with 155 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/1761-state-json-unbounded-milestone-read-path.md Normal file
View File

@@ -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.

47
src/state.cts
View File

@@ -144,6 +144,7 @@ const _diskScanCache = new Map<string, {
completedPhases: number;
totalPlans: number;
completedPlans: number;
milestoneBounded: boolean;
}>();
// 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<string>();
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);
}

113
tests/bug-1761-state-sync-wrong-progress.test.cjs
View File

@@ -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)',
);
});
});