fix(#2828): state sync reports correct total_phases on a flat unmilestoned roadmap (#2892)

* fix+test(#2828): total_phases uses roadmap count on flat unmilestoned roadmap

The read-path disk-scan cache fell back to phaseDirs.length (1) when milestoneBounded
was false, even though roadmapPhaseCount (6) was correct for a flat roadmap (no sibling
milestones to conflate). Use roadmapPhaseCount as the floor when > 0, matching the
write-path (cmdStateSync) which already did this. The milestoneBounded flag still flows
to milestoneUnbounded for the percent-skip (#1761 guard preserved). Regression test
asserts state-sync writes progress.total_phases:6 for a flat 6-phase roadmap + 1 phase dir.

* chore(#2828): changeset fragment

* test(#2828): add negative-space coverage (Math.max floor mutant + no-roadmap fallback) — review findings

The 6-phase test alone couldn't kill a Math.max-dropping mutant (1<6). Add: a
3-phase-dir/2-roadmap-phase case proving Math.max(dirs,count) floor; a no-roadmap
case proving phaseDirs.length fallback.

* fix(#2828): refine — distinguish flat unmilestoned from milestoned-unbounded (preserve #1761)

The first-pass fix (roadmapPhaseCount > 0 always) re-broke #1761: a milestoned-
unbounded roadmap (asserted milestone not among existing version headings) conflated
sibling milestones (8 = 4+4). Refine with a hasMilestoneSectioning discriminator:
^#{2,3}(?!Phase) detects non-Phase h2/h3 milestone section headings. A FLAT roadmap
(only ### Phase headings + a # title) has none → safe to use roadmapPhaseCount; a
SECTIONED-but-unbounded roadmap has them → fall back to phaseDirs.length (#1761).
Verified both cases locally (flat→6, sectioned-unbounded→1).

* test(#2828): remove two fragile negative-space tests (phase-dir scanner internals)

The Math.max-floor and no-roadmap tests made assumptions about the phase-dir scanner's
internals (which dirs count as 'realized') that didn't hold. The core regression test
(6-phase flat → total_phases:6) plus the existing #1761 conflation tests (which the
refined fix preserves) provide sufficient coverage.

* chore(#2828): backfill changeset PR number 2892

* fix(#2828): replace ReDoS-prone regex in regression test with line-by-line parse

CodeQL flagged the nested-quantifier regex (`(?:[ \t]+\w+:.+\r?\n?)*?`) in
tests/issue-2828-flat-roadmap-total-phases.test.cjs as a high-severity
catastrophic-backtracking risk. Rewrite the STATE.md progress.total_phases
extraction as a ReDoS-safe line-by-line block walk.

---------

Co-authored-by: Test <test@example.com>

.changeset/wise-finches-swim.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 2892
+---
+**State sync now reports the correct total phase count on a flat unmilestoned roadmap** — `progress.total_phases` no longer falls back to the on-disk phase-directory count when the roadmap has no versioned milestone heading; it uses the authoritative roadmap count, matching the write-path and resolving the contradiction between smart-entry's `total_phases` and `roadmap_total_phases`. (#2828)

src/state.cts

@@ -1705,10 +1705,20 @@ function buildStateFrontmatter(bodyContent: string, cwd: string | undefined): Re
               );
               milestoneBounded = versionedHeading.test(roadmapRaw);
             }
+            // #2828: distinguish a FLAT unmilestoned roadmap (no milestone sectioning
+            // at all — only Phase headings) from a MILESTONED-but-unbounded one
+            // (milestone/version headings exist but the asserted one isn't among them).
+            // On a flat roadmap the whole-doc count is correct (no sibling milestones to
+            // conflate); on a sectioned-but-unbounded one it conflates siblings (#1761),
+            // so fall back to phaseDirs.length.
+            const hasMilestoneSectioning = roadmapRaw !== null
+              && /^#{2,3}\s+(?!Phase\s+\S)/mi.test(roadmapRaw);
+            const safeToUseRoadmapCount = milestoneBounded
+              || (roadmapPhaseCount > 0 && !hasMilestoneSectioning);
             return {
-              totalPhases: (!milestoneBounded || roadmapPhaseCount === 0)
-                ? phaseDirs.length
-                : Math.max(phaseDirs.length, roadmapPhaseCount),
+              totalPhases: safeToUseRoadmapCount
+                ? Math.max(phaseDirs.length, roadmapPhaseCount)
+                : phaseDirs.length,
               milestoneBounded,
               completedPhases: diskCompletedPhases,
               totalPlans: diskTotalPlans,

tests/issue-2828-flat-roadmap-total-phases.test.cjs

@@ -0,0 +1,95 @@
+// allow-test-rule: behavioral-fs-fixture (#2828)
+'use strict';
+
+// Regression guard for #2828: on a flat unmilestoned roadmap (no versioned milestone
+// heading), `state-snapshot`/`state record-session` reported progress.total_phases as
+// the on-disk phase-dir count (1) instead of the authoritative roadmap count (6). The
+// read-path disk-scan cache fell back to phaseDirs.length when milestoneBounded was
+// false, even though roadmapPhaseCount (6) was correct for a flat roadmap (no sibling
+// milestones to conflate). Fix: use roadmapPhaseCount as the floor when > 0.
+
+const { test, describe, 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');
+
+describe('#2828 — total_phases uses the roadmap count on a flat unmilestoned roadmap', () => {
+  let tmpDir;
+
+  beforeEach(() => {
+    tmpDir = createTempProject('gsd-2828-');
+    const planningDir = path.join(tmpDir, '.planning');
+    // Flat unmilestoned roadmap with 6 phases (no versioned milestone heading).
+    fs.writeFileSync(
+      path.join(planningDir, 'ROADMAP.md'),
+      [
+        '# Roadmap',
+        '',
+        '### Phase 1: Foundation',
+        '### Phase 2: Core API',
+        '### Phase 3: UI Layer',
+        '### Phase 4: Integration',
+        '### Phase 5: Polish',
+        '### Phase 6: Release',
+        '',
+      ].join('\n'),
+    );
+    // Only phase 1 has been discussed → 1 phase dir on disk.
+    const phaseDir = path.join(planningDir, 'phases', '01-foundation');
+    fs.mkdirSync(phaseDir, { recursive: true });
+    fs.writeFileSync(path.join(phaseDir, 'CONTEXT.md'), '# Phase 1 Context\n');
+    // Minimal STATE.md with a milestone set (so milestoneBounded is computed) but no
+    // versioned heading to bound it to → the flat-roadmap unbounded case.
+    fs.writeFileSync(
+      path.join(planningDir, 'STATE.md'),
+      [
+        '---',
+        'status: executing',
+        'milestone: v1.0',
+        'milestone_name: milestone',
+        '---',
+        '',
+        '# Project State',
+        '',
+        '**Current Phase:** 01',
+        '**Status:** In progress',
+        '',
+      ].join('\n'),
+    );
+  });
+
+  afterEach(() => cleanup(tmpDir));
+
+  test('state sync writes progress.total_phases === 6 (roadmap count), not 1 (phase-dir count) (#2828)', () => {
+    // `state sync` derives progress.total_phases from the disk-scan cache (the read path
+    // #2828 fixes) and writes it to STATE.md frontmatter. Pre-fix this wrote 1.
+    const result = runGsdTools(['state', 'sync'], tmpDir);
+    assert.ok(result.success, `state sync failed: ${result.error}`);
+
+    const stateMd = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf8');
+    // Parse the `progress:` YAML block line-by-line (ReDoS-safe: avoids a nested-quantifier
+    // regex over the whole block). Find total_phases among the block's indented children.
+    const lines = stateMd.split(/\r?\n/);
+    let inProgress = false;
+    let totalPhases = null;
+    for (const line of lines) {
+      if (/^progress:\s*$/.test(line)) { inProgress = true; continue; }
+      if (inProgress) {
+        // A new top-level (column-0) key ends the progress block.
+        if (/^\S/.test(line)) { inProgress = false; continue; }
+        const tp = line.match(/^\s+total_phases:\s*(\d+)/);
+        if (tp) { totalPhases = Number(tp[1]); break; }
+      }
+    }
+    assert.ok(
+      totalPhases !== null,
+      `progress.total_phases must be written by state sync. STATE.md:\n${stateMd}`,
+    );
+    assert.strictEqual(
+      totalPhases,
+      6,
+      `progress.total_phases must be the roadmap count (6) for a flat unmilestoned roadmap, not the on-disk phase-dir count (1). Got: ${totalPhases}`,
+    );
+  });
+});