fix(#5320): record planned-phase plan count on Plan:/Current Plan: documents

.changeset/gallant-hawks-dance.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 5424
+---
+**`state planned-phase`** — records the `--plans` count on STATE.md documents that use the `Plan:` / `Current Plan:` Current Position line, keeps replan progress, and now exits non-zero (writing nothing) when no such line or `Total Plans in Phase:` exists. `--plans` is now validated as a non-negative integer, so `state begin-phase --plans` rejects non-integers too (it used to truncate `3.5` to 3). Fixes #5320.

gsd-core/workflows/plan-phase.md

@@ -1423,7 +1423,7 @@ After plans pass all gates, record that planning is complete so STATE.md reflect
 gsd_run query state.planned-phase --phase "${PHASE_NUMBER}" --name "${PHASE_NAME}" --plans "${PLAN_COUNT}"
 ```
 
-This updates STATUS to "Ready to execute", sets the correct plan count, and timestamps Last Activity.
+This updates STATUS to "Ready to execute", sets the correct plan count, and timestamps Last Activity. Fails non-zero, writing nothing, when STATE.md `## Current Position` has no `Plan:`, `Current Plan:` or `Total Plans in Phase:` line.
 
 ## 13c. Annotate ROADMAP with Wave Dependencies and Cross-cutting Constraints
 

src/state-command-router.cts

@@ -67,9 +67,9 @@ interface RouteStateCommandOptions {
 
 function routeStateCommand({ state, args, cwd, raw, error }: RouteStateCommandOptions): void {
   const parsePlans = (plans: string | null | undefined): number | null => {
-    const parsedPlans = plans == null ? null : Number.parseInt(plans, 10);
-    if (plans != null && Number.isNaN(parsedPlans)) {
-      error('Invalid --plans value. Expected an integer.');
+    const parsedPlans = plans == null ? null : Number(plans);
+    if (plans != null && !(/^\d+$/.test(plans) && Number.isSafeInteger(parsedPlans))) {
+      error('Invalid --plans value. Expected a non-negative integer.');
       return null;
     }
     return parsedPlans;

src/state-transition.cts

@@ -22,6 +22,8 @@ import { tokenizeHeadings, withSection } from './markdown-sectionizer.cjs';
 import type { HeadingToken } from './markdown-sectionizer.cjs';
 import { deriveProgressFromRoadmap, clampPercent, clampPercentFromFraction, renderProgressBar } from './phase-lifecycle.cjs';
 import { escapeRegex } from './pattern.cjs';
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+import phaseIdMod = require('./phase-id.cjs');
 // #4129: the completion-ratio kernel for the resync-arm ratchet's percent
 // (planning-scope's SCOPE — state-document's own dependency, no cycle here:
 // state-document never imports this module).
@@ -2437,7 +2439,9 @@ function plannedPhaseCore(
   const { existingFm, hasFrontmatter, body: initialBody, reassemble } = beginFrontmatterReassembly(content, deps.sourcePath);
   let body = initialBody;
 
-  const statusDefaults = KNOWN_TEMPLATE_DEFAULTS['Status'];
+  // #5320: a bare pre-plan `Ready` is moved to `Ready to execute` here; nothing
+  // else writes bare Ready, so it is local to this transition.
+  const statusDefaults = [...(KNOWN_TEMPLATE_DEFAULTS['Status'] ?? []), 'Ready'];
   const lastActivityDefaults = KNOWN_TEMPLATE_DEFAULTS['Last Activity'];
 
   // Status — template-aware (preserve executor-authored values).
@@ -2448,10 +2452,12 @@ function plannedPhaseCore(
   }
 
   // Total Plans in Phase — system-derived; always replaced when a count is given.
+  let totalRecorded = false;
   if (intent.planCount !== null && intent.planCount !== undefined) {
     const result = stateReplaceField(body, 'Total Plans in Phase', String(intent.planCount));
     if (result) {
       body = result;
+      totalRecorded = true;
       updated.push('Total Plans in Phase');
     }
   }
@@ -2467,7 +2473,7 @@ function plannedPhaseCore(
   const ladResult = stateReplaceField(
     body,
     'Last Activity Description',
-    `Phase ${intent.phaseNumber} planning complete — ${intent.planCount || '?'} plans ready`,
+    `Phase ${intent.phaseNumber} planning complete — ${intent.planCount ?? '?'} plans ready`,
   );
   if (ladResult) {
     body = ladResult;
@@ -2486,17 +2492,48 @@ function plannedPhaseCore(
   // Phase is system-derived, always replaced (Knuth invariant does not apply);
   // Status / Last activity stay template-aware.
   const beforePos = body;
+  const posSlice = (b: string): string => {
+    const sp = locateCurrentPosition(b);
+    return sp ? b.slice(sp.start, sp.end) : '';
+  };
+  const posBefore = posSlice(beforePos);
+  // #5320: `Plan: A of B in current phase` template shape; advance-plan parses
+  // `N of M` from either spelling (#3784 hybrid for `Current Plan:`). A replan
+  // keeps the recorded leading N and moves only M; anything else starts at 0.
+  // The recorded N carries over only for the SAME phase (`Phase:` leading token).
+  const samePhase = phaseIdMod.phaseKeyFromProse(stateExtractField(posBefore, 'Phase')) === phaseIdMod.phaseKeyFromToken(intent.phaseNumber);
+  const below: { n?: number } = {};
+  const planValue = (name: string): string | undefined => {
+    if (intent.planCount == null) return undefined;
+    const m = samePhase ? PLAN_SHAPE_N_OF_M.exec(stateExtractField(posBefore, name) ?? '') : null;
+    if (m && Number(m[1]) > intent.planCount) below.n = Number(m[1]);
+    return `${m ? m[1] : 0} of ${intent.planCount} in current phase`;
+  };
   body = mutateCurrentPositionForAdvance(
     body,
     {
       phase: `${intent.phaseNumber}${intent.phaseName ? ` (${intent.phaseName})` : ''} — READY TO EXECUTE`,
       status: 'Ready to execute',
       lastActivity: `${today} — Phase ${intent.phaseNumber} planning complete`,
+      plan: planValue('Plan'),
+      currentPlan: planValue('Current Plan'),
     },
     statusDefaults,
     lastActivityDefaults,
   );
   if (body !== beforePos) updated.push('Current Position');
+  const posAfter = posSlice(body);
+  for (const name of ['Plan', 'Current Plan']) {
+    if (stateExtractField(posAfter, name) !== stateExtractField(posBefore, name)) updated.push(name);
+  }
+  // #5320: --plans must land somewhere; the caller fails before any write.
+  if (intent.planCount != null && !totalRecorded
+    && stateExtractField(posBefore, 'Plan') === null && stateExtractField(posBefore, 'Current Plan') === null) {
+    return { content, updated: [], data: { planError: 'state planned-phase: --plans has no line to land on. Add `Plan: 0 TBD in current phase` under `## Current Position` (or a `Total Plans in Phase:` line) and retry.' } };
+  }
+  if (below.n !== undefined) {
+    return { content, updated: [], data: { planError: `state planned-phase: --plans ${intent.planCount} is below the executed plan count ${below.n}.` } };
+  }
 
   // #2400 Bug B: sync progress.total_plans to the frontmatter when a plan count
   // is given. This writes the explicitly-provided count — it is NOT a re-derivation

src/state.cts

@@ -6118,6 +6118,8 @@ function cmdStatePlannedPhase(cwd: string, phaseNumber: string | number | null |
       }
     }
     const result = transitionCore(content, intent, deps);
+    // #5320: nothing to record --plans on; exit before the write.
+    if (result.data?.['planError']) error(result.data['planError'] as string, ioMod.ERROR_REASON.USAGE);
     precomputedUpdated = result.updated;
     return result.content;
   }, cwd, rmwOptions);
@@ -6131,7 +6133,7 @@ function cmdStatePlannedPhase(cwd: string, phaseNumber: string | number | null |
   const updated = reconcileReportedFields(statePath, preWriteState, precomputedUpdated, divergedFields);
 
   const result = updated.length === 0
-    ? { updated, phase: phaseNumber, plan_count: planCount, warning: 'STATE.md Current Position has no recognized labels — transition was a no-op. Verify STATE.md uses the canonical labeled format (Status:, Total Plans in Phase:, etc.).' }
+    ? { updated, phase: phaseNumber, plan_count: planCount, warning: 'No fields changed — STATE.md is already up to date, or its Current Position has no recognized labels. If the latter, verify the canonical labeled format (Status:, Total Plans in Phase:, etc.).' }
     : { updated, phase: phaseNumber, plan_count: planCount };
   output(result, raw, updated.length > 0 ? 'true' : 'false');
   // #3227 (design doc §40 row 26 / "Not-corruption" rule): gate on

tests/fixtures/compact-content-benchmark-baseline.json

@@ -28,9 +28,9 @@
       "reductionPct": 13.37
     },
     "plan-phase": {
-      "offTokens": 28447,
-      "onTokens": 25179,
-      "reductionPct": 11.49
+      "offTokens": 28482,
+      "onTokens": 25214,
+      "reductionPct": 11.47
     },
     "verify-work": {
       "offTokens": 15378,
@@ -39,8 +39,8 @@
     }
   },
   "aggregate": {
-    "offTokens": 112429,
-    "onTokens": 94808,
+    "offTokens": 112464,
+    "onTokens": 94843,
     "reductionPct": 15.67
   }
 }

tests/state-contract.test.cjs

@@ -345,10 +345,10 @@ describe('state contract — call-site wiring (integration, real CLI)', () => {
       '(no recognized labels here)',
     ]);
     const result = runGsdTools(['state', 'planned-phase', '--phase', '3', '--name', 'Polish', '--plans', '1'], tmpDir);
-    assert.ok(result.success, `planned-phase fixture must genuinely succeed: ${result.error}`);
-    const output = JSON.parse(result.output);
-    assert.deepStrictEqual(output.updated, [], 'expected a genuine no-op (zero recognized Current Position labels)');
-    assert.strictEqual(fs.existsSync(statePathOf(tmpDir)), false, 'a no-op transition must not publish state.json');
+    // #5320: with `--plans` and no Plan / Current Plan / Total Plans line to
+    // land on, planned-phase now fails non-zero before any write.
+    assert.strictEqual(result.success, false, 'planned-phase must fail when --plans has no line to land on');
+    assert.strictEqual(fs.existsSync(statePathOf(tmpDir)), false, 'a failed transition must not publish state.json');
   });
 
   test('beginPhaseNoOpDoesNotPublish', (t) => {