fix(#3807): advance-plan refuses an ambiguous multi-Phase Current Position (#4028)

* test(#3807): advance-plan must refuse an ambiguous multi-entry Current Position

* fix(#3807): refuse an ambiguous multi-Phase Current Position before advancing

* chore(#3807): changeset fragment (pr number backfilled after PR creation)

* chore(#3807): backfill changeset PR number (4028)

---------

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

.changeset/patient-sloths-glide.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 4028
+---
+**`state advance-plan` refuses an ambiguous Current Position instead of silently advancing the first entry** — when the section carries more than one `Phase:` line (the wave-log style), the command now returns a typed `ambiguous_position_phase` error naming every candidate and leaves STATE.md byte-identical, instead of silently advancing the first entry's plan counter (in the reporting incident, a hard-gated final plan 7→8 of 8) with `advanced: true` and no ambiguity signal. (#3807)

src/state-transition.cts

@@ -16,7 +16,7 @@
 
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import frontmatter = require('./frontmatter.cjs');
-import { stateReplaceField, stateExtractField, stateReplaceFieldIfTemplate, stateReplaceFieldWithFallback, stateReplaceFieldInSession } from './state-document.cjs';
+import { stateReplaceField, stateExtractField, stateReplaceFieldIfTemplate, stateReplaceFieldWithFallback, stateReplaceFieldInSession, stateCurrentPositionSlice } from './state-document.cjs';
 import { KNOWN_TEMPLATE_DEFAULTS, toFiniteNumber } from './state-document.cjs';
 import { tokenizeHeadings } from './markdown-sectionizer.cjs';
 import type { HeadingToken } from './markdown-sectionizer.cjs';
@@ -1382,6 +1382,34 @@ function advancePlanCore(content: string, deps: StateTransitionDeps): StateTrans
   const { body: initialBody, reassemble } = beginFrontmatterReassembly(content, deps.sourcePath);
   let body = initialBody;
 
+  // #3807: refuse a Current Position section carrying more than one `Phase:`
+  // entry BEFORE mutating. The plan fields below come from document-wide
+  // first-match extraction, so in a wave-log style section (one entry per
+  // completed wave) the FIRST entry's plan counter silently advanced — in the
+  // reporting incident, a hard-gated final plan 7→8 of 8 — while the entry
+  // the caller meant sat untouched below it, with advanced:true and no
+  // ambiguity signal. advance-plan now refuses before acting. Scoped via the
+  // #2956 canonical locator (stateCurrentPositionSlice — H2 or H3 heading,
+  // the same one cmdStateAdvancePlan's own milestone read uses); NO whole-body
+  // fallback — a legacy-format document with unrelated `Phase:` history lines
+  // elsewhere has no section to disambiguate and must keep its current
+  // behavior rather than be falsely refused.
+  const positionScope = stateCurrentPositionSlice(body);
+  if (positionScope !== null) {
+    const phaseCandidates = (positionScope.match(/^Phase:.*$/gm) || []);
+    if (phaseCandidates.length > 1) {
+      return {
+        content,
+        updated: [],
+        data: {
+          error: true,
+          reason: 'ambiguous_position_phase',
+          phase_candidates: phaseCandidates.map((l) => l.trim()),
+        },
+      };
+    }
+  }
+
   // Parse plan number — legacy first, then compound.
   const legacyPlan = stateExtractField(content, 'Current Plan');
   const legacyTotal = stateExtractField(content, 'Total Plans in Phase');

src/state.cts

@@ -894,6 +894,16 @@ function cmdStateAdvancePlan(cwd: string, raw: boolean): void {
   }, cwd, { divergedFields, preWriteState });
 
   if (!resultData || resultData['error']) {
+    // #3807: a multi-`Phase:` Current Position section carries its own cause
+    // and its own remedy (name the candidates; the caller resolves them).
+    if (resultData && resultData['reason'] === 'ambiguous_position_phase') {
+      output({
+        error: 'Current Position section contains more than one Phase: entry — refusing to silently advance the first. Resolve the section to a single current entry and re-run.',
+        reason: resultData['reason'],
+        phase_candidates: resultData['phase_candidates'],
+      }, raw, undefined);
+      return;
+    }
     output({ error: 'Cannot parse Current Plan or Total Plans in Phase from STATE.md' }, raw, undefined);
     return;
   }

tests/advance-plan-ambiguous-phase.test.cjs

@@ -0,0 +1,100 @@
+'use strict';
+
+// ─────────────────────────────────────────────────────────────────────────────
+// #3807 — advance-plan must refuse a Current Position section carrying
+// more than one `Phase:` entry instead of silently advancing the first.
+//
+// The #2956 fix scoped the milestone-conflict Phase read to the Current
+// Position section, but advancePlanCore's plan fields still came from
+// document-wide first-match stateExtractField — so a wave-log style section
+// (one Phase: entry per completed wave, all under Current Position) had its
+// FIRST entry's plan counter silently advanced — in the reporter's incident,
+// a hard-gated final plan 7→8 of 8 — with advanced:true and
+// milestone_conflict:null, no error, no ambiguity signal.
+// ─────────────────────────────────────────────────────────────────────────────
+
+const { test, describe } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+
+const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs');
+
+const TWO_ENTRY_BODY = [
+  '## Current Position',
+  '',
+  'Phase: 03.1 of 8 (some-phase)',
+  'Plan: 7 of 8 in current phase',
+  'Status: In progress',
+  'Last activity: 2026-08-24 — working',
+  '',
+  'Phase: 04 of 15 (other-phase)',
+  'Plan: 7 of 15 in current phase',
+  'Status: Phase complete',
+  'Last activity: 2026-08-24 — wave 4 done',
+  '',
+].join('\n');
+
+function writeState(tmpDir, positionBody) {
+  const content = [
+    '---',
+    'gsd_state_version: 1.0',
+    'current_phase: 03',
+    'status: executing',
+    'progress:',
+    '  total_phases: 2',
+    '---',
+    '',
+    positionBody,
+  ].join('\n');
+  fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), content);
+}
+
+function runAdvance(cwd) {
+  return runGsdTools(['state', 'advance-plan'], cwd);
+}
+
+describe('#3807: advance-plan refuses an ambiguous multi-entry Current Position', () => {
+  test('#3807: two Phase: entries under Current Position → ambiguous error, no mutation', (t) => {
+    const tmpDir = createTempProject('gsd-3807-amb-');
+    t.after(() => cleanup(tmpDir));
+    writeState(tmpDir, TWO_ENTRY_BODY);
+    const before = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf8');
+
+    const r = runAdvance(tmpDir);
+    // The CLI reports an error payload (exit success shape is the command's
+    // own convention for parse errors — assert on the payload, not the code).
+    const out = JSON.parse(r.output);
+    assert.ok(
+      out.error && out.reason === 'ambiguous_position_phase' && /more than one Phase/i.test(String(out.error)),
+      `#3807: the error must name the multi-Phase condition with the typed reason; got ${r.output}`,
+    );
+    assert.ok(
+      Array.isArray(out.phase_candidates) && out.phase_candidates.length === 2,
+      `#3807: both Phase: candidates must be named; got ${JSON.stringify(out.phase_candidates)}`,
+    );
+    const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf8');
+    assert.equal(after, before, '#3807: refusing must leave STATE.md byte-identical');
+    assert.ok(!/Plan: 8 of 8/.test(after), 'the first entry\'s plan counter must NOT advance');
+  });
+
+  test('#3807 control: a single-entry section advances exactly as before', (t) => {
+    const tmpDir = createTempProject('gsd-3807-ctl-');
+    t.after(() => cleanup(tmpDir));
+    writeState(tmpDir, [
+      '## Current Position',
+      '',
+      'Phase: 03 of 8 (some-phase)',
+      'Plan: 3 of 8 in current phase',
+      'Status: In progress',
+      'Last activity: 2026-08-24 — working',
+      '',
+    ].join('\n'));
+
+    const r = runAdvance(tmpDir);
+    const out = JSON.parse(r.output);
+    assert.equal(out.advanced, true, `single-entry advance still works; got ${r.output}`);
+    const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf8');
+    assert.match(after, /Plan: 4 of 8/, 'the plan counter advanced');
+  });
+});

tests/io.test.cjs

@@ -841,11 +841,11 @@ describe('#3912 A3-A5: output({error}) records DEGRADED — shape-exhaustive plu
       perFile,
       {
         'commands.cts': 5, 'frontmatter.cts': 7, 'gsd2-import.cts': 2, 'phase.cts': 4,
-        'roadmap.cts': 3, 'state.cts': 25, 'template.cts': 3, 'verify.cts': 8, 'workstream.cts': 7,
+        'roadmap.cts': 3, 'state.cts': 26, 'template.cts': 3, 'verify.cts': 8, 'workstream.cts': 7,  // +1 #3807: advance-plan's ambiguous-position error
       },
       `per-file output({error}) census drifted: ${JSON.stringify(perFile)}`,
     );
-    assert.strictEqual(total, 64, `enumerated output({error}) population drifted from the measured 64: got ${total}`);
+    assert.strictEqual(total, 65, `enumerated output({error}) population drifted from the measured 65 (64 + #3807's ambiguous-position error): got ${total}`);
   });
 });