fix(#3573): keep the stored total_phases when the roadmap is absent at state-write time (#3595)

* test(#3573): pin stored-total retention when the roadmap is absent at state-write time

Failing-first regression for #3573: with ROADMAP.md absent and a milestone
asserted, every state.* write persisted the phase-directory count as
progress.total_phases (5 -> 1 in the issue) — only STARTED phases count,
quietly defeating #549's single source of truth. Rows pin the stored-value
outcome + stderr warning across record-session and begin-phase, the
fresh-project doctrine (no milestone asserted -> dir count stays), and the
roadmap-present control.

* fix(#3573): keep the stored total_phases when the roadmap is absent at state-write time

The #3354 withhold covered milestoned-but-unbounded roadmaps but not the
roadmap-absent shape: with ROADMAP.md unreadable the #549 heading counter
never runs, milestoneBounded is vacuously true, and every state.* write
persisted the phase-directory count as progress.total_phases — counting
only STARTED phases (5 -> 1 in the issue). When the STATE asserts a
milestone (storedMilestone), the stored frontmatter total now wins and a
(#3353)-style stderr warning names the condition; with no asserted
milestone the disk count stays authoritative (fresh-project doctrine).

* fix(#3573): thread stored milestone into the state json read for write/read parity; discriminate the doctrine row; pin planned-phase

Review findings: cmdStateJson passed storedMilestone=undefined so the new
withhold never fired on the read surface — state json reported the dir
count while the persisted file preserved the stored total (exactly the
divergence #3354 closed for its shape). The fresh-project doctrine row now
uses stored 5 vs dirs 2 so a milestone-gate-less withhold mutant cannot
survive it; the third issue-named verb (planned-phase) is pinned.

* chore(#3573): add changeset fragment

* chore(#3573): backfill changeset pr number

---------

Co-authored-by: sim <sim@local>

.changeset/quick-moles-caper.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3595
+---
+**`state` writes no longer shrink progress.total_phases to the started-phase count when ROADMAP.md is absent** — with no readable roadmap, every state command persisted the on-disk phase-directory count as the declared total (only phases that had started counted, so a 5-phase project read 50-100% complete with 3-4 phases unstarted); the stored frontmatter total now wins, with a warning, and `state json` reports the same preserved value. (#3573)

src/state.cts

@@ -2198,10 +2198,35 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined, sto
                 `gsd: warning — milestone '${String(assertedMilestoneVersion ?? '').trim()}' is asserted in STATE.md but matches no ROADMAP heading, and the ROADMAP carries multiple milestone sections; the on-disk phase-directory count would understate the declared total, so progress.total_phases is left at its stored value. (#3354)\n`
               );
             }
+            // #3573: the roadmap-absent sibling of the #3354 shape. With ROADMAP.md
+            // absent/unreadable the #549 heading counter never ran (roadmapScope
+            // stayed null), `milestoneBounded` is vacuously true (its gate requires
+            // roadmapRaw), and the dir count — which only ever counts phases that
+            // have STARTED — would be persisted as progress.total_phases by every
+            // state.* write. A STATE that asserts a milestone (storedMilestone —
+            // getMilestoneInfo is useless here, it reads the roadmap that is
+            // absent) declared a total somewhere; keep the stored frontmatter
+            // value instead. Without an asserted milestone (fresh project,
+            // pre-roadmap) the disk count is still the only source and stays
+            // authoritative (the #3354 doctrine's degenerate case).
+            const roadmapAbsentWithAssertedMilestone =
+              roadmapRaw === null &&
+              typeof storedMilestone === 'string' &&
+              storedMilestone.trim() !== '';
+            if (roadmapAbsentWithAssertedMilestone) {
+              process.stderr.write(
+                `gsd: warning — milestone '${storedMilestone.trim()}' is asserted in STATE.md but ROADMAP.md is absent or unreadable, so the phase-heading total cannot be derived; the on-disk phase-directory count would understate the declared total, so progress.total_phases is left at its stored value. (#3573)\n`
+              );
+            }
             return {
-              totalPhases: safeToUseRoadmapCount
-                ? Math.max(phaseDirs.length, roadmapPhaseCount)
-                : (milestonedButUnbounded ? null : phaseDirs.length),
+              // The two WITHHOLD shapes (#3354 milestoned-but-unbounded, #3573
+              // roadmap-absent-with-asserted-milestone) must be evaluated BEFORE
+              // safeToUseRoadmapCount — in the #3573 shape milestoneBounded is
+              // vacuously true (its gate requires roadmapRaw), so the safe-count
+              // arm would otherwise swallow the withhold.
+              totalPhases: (milestonedButUnbounded || roadmapAbsentWithAssertedMilestone)
+                ? null
+                : (safeToUseRoadmapCount ? Math.max(phaseDirs.length, roadmapPhaseCount) : phaseDirs.length),
               milestoneBounded,
               completedPhases: diskCompletedPhases,
               totalPlans: diskTotalPlans,
@@ -3510,7 +3535,12 @@ function cmdStateJson(cwd: string, raw: boolean): void {
   // when SUMMARY files were added after the last STATE.md write (#1589).
   // #3354: pass the stored total so the milestoned-but-unbounded withhold can
   // report the preserved value instead of omitting the key.
-  const built = buildStateFrontmatter(body, cwd, undefined, readStoredTotalPhases(existingFm));
+  // #3573: pass the STORED MILESTONE too (same parity reasoning) — otherwise the
+  // roadmap-absent withhold never fires on this read surface and `state json`
+  // reports the phase-directory count while the persisted file preserves the
+  // stored total, exactly the write/read divergence #3354 closed for its shape.
+  const storedMilestoneJson = typeof existingFm['milestone'] === 'string' ? existingFm['milestone'] : null;
+  const built = buildStateFrontmatter(body, cwd, storedMilestoneJson, readStoredTotalPhases(existingFm));
 
   // ADR-3408 §8.5 / D3: route stopped_at / paused_at / status / current_phase /
   // current_phase_name / current_plan through the SAME `preserve-when-unchanged`

tests/state-document.test.cjs

@@ -1438,5 +1438,162 @@ describe('#3354 buildStateFrontmatter total_phases — milestoned-but-unbounded
     );
   });
 });
+
+// #3573 — roadmap-absent sibling of the #3354 shape: when ROADMAP.md is
+// absent/unreadable while STATE.md asserts a milestone, the #549 heading counter
+// never runs (roadmapScope === null) and every state.* write persisted the on-disk
+// phase-directory count as progress.total_phases — counting only phases that have
+// STARTED, quietly defeating #549's single-source-of-truth (5 → 1 in the issue's
+// report). The stored frontmatter total must win, with a stderr warning.
+describe('#3573 total_phases — roadmap absent with an asserted milestone', () => {
+  const { runNode } = require('./helpers/process-seam.cjs');
+  const { TOOLS_PATH, TEST_ENV_BASE } = require('./helpers.cjs');
+
+  let tmpDir;
+
+  beforeEach(() => {
+    tmpDir = createTempProject();
+  });
+
+  afterEach(() => {
+    cleanup(tmpDir);
+  });
+
+  /** Read the PERSISTED progress.total_phases straight out of STATE.md (the corruption is a file write, not a read derivation). */
+  function persistedTotalPhases(dir) {
+    const raw = fs.readFileSync(path.join(dir, '.planning', 'STATE.md'), 'utf8');
+    const m = raw.match(/^\s{2}total_phases:\s*(\d+)\s*$/m);
+    return m ? Number(m[1]) : null;
+  }
+
+  function recordSession(dir, stoppedAt = 'Phase 1, Plan 1') {
+    return runNode(
+      [TOOLS_PATH, 'state', 'record-session', '--stopped-at', stoppedAt, '--resume-file', 'none'],
+      { cwd: dir, env: { ...process.env, ...TEST_ENV_BASE }, timeoutMs: 60000 },
+    );
+  }
+
+  test('#3573: roadmap-absent write keeps the stored total_phases instead of the phase-directory count', () => {
+    // No ROADMAP.md at all — the issue's "scoping fails" endpoint. STATE asserts
+    // milestone v1.0 with a stored total of 5; one phase directory exists.
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      buildStateMd({ milestone: 'v1.0', totalPhases: 5 }),
+    );
+    seedPhaseDirs(tmpDir, [1]);
+
+    const rec = recordSession(tmpDir);
+    assert.ok(rec.exitCode === 0, `state record-session failed: ${rec.stderr}`);
+    assert.match(
+      rec.stderr || '',
+      /\(#3573\)/,
+      `a stderr warning must name the roadmap-absent condition; got stderr=${JSON.stringify(rec.stderr)}`,
+    );
+    assert.strictEqual(
+      persistedTotalPhases(tmpDir),
+      5,
+      `total_phases must stay at the stored 5, not the phase-directory count of 1`,
+    );
+  });
+
+  test('#3573: begin-phase write keeps the stored total_phases (second issue-named verb)', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      buildStateMd({ milestone: 'v1.0', totalPhases: 5, currentPhase: '02' }),
+    );
+    seedPhaseDirs(tmpDir, [1]);
+
+    const rec = runNode(
+      [TOOLS_PATH, 'state', 'begin-phase', '2'],
+      { cwd: tmpDir, env: { ...process.env, ...TEST_ENV_BASE }, timeoutMs: 60000 },
+    );
+    assert.ok(rec.exitCode === 0, `state begin-phase failed: ${rec.stderr}`);
+    assert.strictEqual(
+      persistedTotalPhases(tmpDir),
+      5,
+      `total_phases must stay at the stored 5 across begin-phase's frontmatter resync`,
+    );
+  });
+
+  test('#3573: no milestone asserted + roadmap absent keeps the directory count (fresh-project doctrine)', () => {
+    // The #3354 doctrine: with nothing declared anywhere else, the disk count is
+    // the only source and stays authoritative. A milestone-less STATE must keep
+    // deriving total_phases from the directories — stored 5, dirs 2, expect 2,
+    // so the row DISCRIMINATES: a withhold that fires without the milestone
+    // gate would leave 5 and fail here (mutation-kill guard).
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      buildStateMd({ milestone: 'none', totalPhases: 5 }).replace(/^milestone: none$/m, ''),
+    );
+    seedPhaseDirs(tmpDir, [1, 2]);
+
+    const rec = recordSession(tmpDir);
+    assert.ok(rec.exitCode === 0, `state record-session failed: ${rec.stderr}`);
+    assert.strictEqual(
+      persistedTotalPhases(tmpDir),
+      2,
+      `without an asserted milestone, the directory count (2) remains the source — not the stored 5`,
+    );
+  });
+
+  test('#3573: state json read surface agrees with the persisted file (write/read parity)', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      buildStateMd({ milestone: 'v1.0', totalPhases: 5 }),
+    );
+    seedPhaseDirs(tmpDir, [1]);
+
+    const rec = recordSession(tmpDir);
+    assert.ok(rec.exitCode === 0, `state record-session failed: ${rec.stderr}`);
+
+    const jsonResult = runGsdTools(['state', 'json', '--raw'], tmpDir);
+    assert.ok(jsonResult.success, `state json --raw failed: ${jsonResult.error}`);
+    const out = JSON.parse(jsonResult.output);
+    assert.strictEqual(
+      Number(out.progress && out.progress.total_phases),
+      5,
+      `#3573 read parity: state json must report the preserved stored 5, not the dir count of 1. Got ${out.progress && out.progress.total_phases}`,
+    );
+  });
+
+  test('#3573: planned-phase write keeps the stored total_phases (third issue-named verb)', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      buildStateMd({ milestone: 'v1.0', totalPhases: 5, currentPhase: '02' }),
+    );
+    seedPhaseDirs(tmpDir, [1]);
+
+    const rec = runNode(
+      [TOOLS_PATH, 'state', 'planned-phase', '2', '--name', 'Core'],
+      { cwd: tmpDir, env: { ...process.env, ...TEST_ENV_BASE }, timeoutMs: 60000 },
+    );
+    assert.ok(rec.exitCode === 0, `state planned-phase failed: ${rec.stderr}`);
+    assert.strictEqual(
+      persistedTotalPhases(tmpDir),
+      5,
+      `total_phases must stay at the stored 5 across planned-phase's frontmatter resync`,
+    );
+  });
+
+  test('#3573 (control): roadmap-present scoped write derives from headings, unchanged', () => {
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'ROADMAP.md'),
+      ['# Roadmap', '', '## Milestone v1.0', '', ...[1, 2, 3, 4, 5].map((i) => `### Phase ${i}: p${i}`), ''].join('\n'),
+    );
+    fs.writeFileSync(
+      path.join(tmpDir, '.planning', 'STATE.md'),
+      buildStateMd({ milestone: 'v1.0', totalPhases: 5 }),
+    );
+    seedPhaseDirs(tmpDir, [1]);
+
+    const rec = recordSession(tmpDir);
+    assert.ok(rec.exitCode === 0, `state record-session failed: ${rec.stderr}`);
+    assert.strictEqual(
+      persistedTotalPhases(tmpDir),
+      5,
+      `scoped roadmap keeps the heading-derived total of 5`,
+    );
+  });
+});
   });
 }