refactor(#3187): give state field extraction one canonical owner (#3283)

* refactor(#3187): give state field extraction one canonical owner stateFieldValue in state-document.cts becomes the single owner of the #1760 frontmatter-then-body fallback chain. The new whole-repo guard found 14 independent re-derivations where the epic scoped 5, all now routed through it: cmdStateSnapshot (11), cmdStatePrune (2) and smart-entry fmScalar (1). state validate was a gate that could not fail. Every warning it could emit sat behind a phase resolved without the frontmatter tier, so a STATE.md whose phase lives only in frontmatter skipped the drift scan entirely and returned valid:true. It also read unstripped content, letting a frontmatter status: key shadow the body field (#1255 class). Both fixed; output gains a scope field so could-not-look stops being output-identical to looked-and-clean. Verified on the remote runner. * docs(#3187): document the state validate scope field and its reason codes Adds docs/how-to/interpret-state-validate-results.md so a reader can tell nothing-to-report from could-not-look, updates the COMMANDS.md and USER-GUIDE.md entries, corrects the CONTEXT.md glossary overstatement about Current Position sole ownership, and drops the changeset fragment. * fix(#3187): close three drift-guard evasion shapes and test the refuse path The isolated adversarial review found the ladder detector was evadable by ordinary reformatting, not just deliberately: a member or computed operand (fm.key / fm[key]) missed the bare-identifier backreference, a swapped tier order missed a hardcoded number-then-boolean sequence, and a ladder wrapped across lines missed single-line detection. All three now caught, each with its own test plus a proven boundary control. The frontmatter-parse refuse path on the destructive complete-phase route was unreachable and therefore untested. It is now driven by an injected parse failure and asserts STATE.md is byte-identical after the refusal, rather than shipping untested defensive code on a path that rewrites user state. Verified on the remote runner. * fix(#3187): widen the drift guard to the prompt layer and disclose tier-2 changes The code-review spec axis found the guard's scan surface was src/ only, which is Decision 4(d)'s forbidden allowlist one directory wide - and it had a live miss: gsd-core/workflows/smart-entry.md tells an agent to read status from frontmatter or the body, a prose expression of this same chain. The surface now covers the prompt layer. That one site carries a permanent written exemption rather than a ratchet: it is the gsd-tools-is-down fallback, so it cannot call the owner by construction, and a ratchet would imply removable debt that does not exist. Two tier-2 output changes shipped undisclosed and are now named in the changeset and docs: complete-phase's idempotency guard consulting frontmatter, and the workstream inventory resolving frontmatter-only fields. docs/COMMANDS.md gains a state complete-phase entry, which it never had. Also records Amendment 5 on ADR-3180, extracts the duplicated frontmatter-parse block the epic's own thesis forbids, and re-points two assertions from free-form warning prose onto the structured drift object. Verified on the remote runner. * chore(#3187): backfill changeset PR number pr:0 placeholder replaced with the real PR number now that #3283 exists. --------- Co-authored-by: sim <sim@local>
2026-08-09 22:49:39 -04:00
parent cf6de5e1c0
commit 693f12ad56
16 changed files with 2616 additions and 80 deletions
--- a/src/smart-entry.cts
+++ b/src/smart-entry.cts
@@ -47,7 +47,7 @@ import phaseLifecycle = require('./phase-lifecycle.cjs');
 const { deriveProgressFromRoadmap } = phaseLifecycle;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import stateDocument = require('./state-document.cjs');
-const { stateExtractField } = stateDocument;
+const { stateFieldValue } = stateDocument;
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import phaseId = require('./phase-id.cjs');
 const { comparePhaseNum, extractPhaseToken, normalizePhaseName, phaseTokenMatches } = phaseId;
@@ -140,14 +140,6 @@ export const SITUATIONS: readonly Situation[] = Object.freeze([

 // ─── Detection ─────────────────────────────────────────────────────────────────

-/** Frontmatter scalar helper: prefer YAML frontmatter, fall back to body field. */
-function fmScalar(fm: Record<string, unknown>, body: string, key: string, bodyField: string): string | null {
-  const v = fm[key];
-  if (typeof v === 'string' && v.trim()) return v.trim();
-  if (typeof v === 'number' || typeof v === 'boolean') return String(v);
-  return stateExtractField(body, bodyField);
-}
-
 /** Read a scalar value from a nested frontmatter object (e.g. progress.total_phases). */
 function fmScalarKey(obj: unknown, key: string): string | null {
  if (!obj || typeof obj !== 'object') return null;
@@ -324,26 +316,34 @@ export function detectSignals(cwd: string, now: () => number = Date.now): SmartE
  //   - body prose:         `Phase: 3`, `**Status:** verifying`, `Total Phases: 5`
  // Read each field across every form, scalar-first then nested then body, so
  // the classifier works on real STATE.md files written by current GSD.
-  const statusRaw = fmScalar(fm, body, 'status', 'Status');
-  const pausedAtRaw = fmScalar(fm, body, 'paused_at', 'Paused At');
-  const lastActivityRaw = fmScalar(fm, body, 'last_activity', 'Last Activity');
+  const statusRaw = stateFieldValue(fm, body, 'status', 'Status').value;
+  const pausedAtRaw = stateFieldValue(fm, body, 'paused_at', 'Paused At').value;
+  const lastActivityRaw = stateFieldValue(fm, body, 'last_activity', 'Last Activity').value;

  // current_phase: scalar fm → nested (none) → body "Current Phase" → body "Phase".
  // The body `Phase:` field is the canonical location in prose-form STATE.md
  // (e.g. "Phase: 3" or "Phase: 3 — ui-review"); parse the leading number.
+  // #3187 / ADR-3180 Amendment 3 ("0.x split"): this read is DELIBERATELY
+  // unscoped — smart-entry classifies `gsd next` routing over the whole body,
+  // whereas state.cts's copies of this same field scope it to `## Current
+  // Position` (#1776/#2956). Those are two different questions sharing a
+  // name; folding the scoped read in here would silently change smart-entry's
+  // routing, an undisclosed Tier-2 change (design's Rejected #3). Both owner
+  // calls below intentionally pass the unscoped `body`, never a Current-
+  // Position slice.
  const currentPhaseRaw =
-    fmScalar(fm, body, 'current_phase', 'Current Phase') ??
-    stateExtractField(body, 'Phase');
+    stateFieldValue(fm, body, 'current_phase', 'Current Phase').value ??
+    stateFieldValue(fm, body, null, 'Phase').value;

  // total_phases & percent: nested `progress:` object takes precedence in the
  // nested schema; scalar fm / body fields cover the flat schema.
  const progressFm = typeof fm.progress === 'object' ? fm.progress : null;
  const totalPhasesRaw: string | null =
    fmScalarKey(progressFm, 'total_phases') ??
-    fmScalar(fm, body, 'total_phases', 'Total Phases');
+    stateFieldValue(fm, body, 'total_phases', 'Total Phases').value;
  const progressRaw: string | null =
    fmScalarKey(progressFm, 'percent') ??
-    fmScalar(fm, body, 'progress', 'Progress');
+    stateFieldValue(fm, body, 'progress', 'Progress').value;

  // Blockers list: `- <text>` items under a `## Blockers` heading.
  const blockers: string[] = [];
--- a/src/state-document.cts
+++ b/src/state-document.cts
@@ -11,6 +11,10 @@ import { splitTableRow } from './markdown-table.cjs';
 import { clampPercentFromFraction } from './phase-lifecycle.cjs';
 import { collectSection } from './markdown-sectionizer.cjs';
 import type { HeadingToken } from './markdown-sectionizer.cjs';
+// eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-scope.cjs is an export= CommonJS module
+import planningScopeMod = require('./planning-scope.cjs');
+const { SCOPE } = planningScopeMod;
+type Scope = planningScopeMod.Scope;

 // Internal helpers
 function escapeRegex(str: string): string {
@@ -234,6 +238,80 @@ export function stateExtractField(content: string, fieldName: string): string |
  return null;
 }

+/**
+ * Single owner of the #1760 STATE.md field-extraction fallback chain: "read
+ * field F, preferring the YAML frontmatter scalar, falling back to the body
+ * field." Added for #3187 (epic #3180, ADR-3180 §7.7) to collapse three
+ * independent re-derivations of this chain — `src/smart-entry.cts`'s
+ * `fmScalar` closure, and `src/state.cts`'s `cmdStateSnapshot` and
+ * `cmdStatePrune` — onto one function, per ADR-3180 Decision 1 ("keep N
+ * copies with a parity test" is rejected: a parity test proves today's
+ * agreement, not that copy N+1 won't happen).
+ *
+ * Takes ALREADY-PARSED `fm` and `body` rather than raw STATE.md content: the
+ * heaviest caller, `cmdStateSnapshot`, reads roughly ten fields off one parse
+ * and must not re-parse frontmatter per field.
+ *
+ * `stateExtractField` (above) is deliberately left untouched — it has 20
+ * direct callers and a CRITICAL blast radius (ADR-3180 §7.7's Rejected #1) —
+ * so this function is additive: it calls `stateExtractField` rather than
+ * replacing it or changing its signature.
+ *
+ * Fallback ladder (unchanged from every prior copy this replaces):
+ *   1. `fm[fmKey]` is a non-empty (post-`.trim()`) string → that trimmed
+ *      string.
+ *   2. `fm[fmKey]` is a `number` or `boolean` → `String(fm[fmKey])`, so `0`
+ *      and `false` are VALUES, not absence.
+ *   3. Anything else (`null`, `undefined`, an object, an array, or an
+ *      empty/whitespace-only string) → fall through to
+ *      `stateExtractField(body, bodyField)`.
+ *
+ * `fmKey === null` skips steps 1–2 outright: for a caller whose chain has no
+ * frontmatter side for this particular field (e.g. `state.cts`'s body-only
+ * `Last Activity` / `Last activity` case-variant pair, which sits inside a
+ * function that DOES own a ladder for its other fields, so per this phase's
+ * function-scoped guard it must still route through this owner).
+ * `bodyField === null` skips step 3: for a caller whose "no frontmatter
+ * value" case falls through to an already-computed value instead of a fresh
+ * extractor call (e.g. `cmdStateSnapshot`'s `last_activity`, which falls to
+ * its already-parsed prose date rather than re-extracting the body).
+ *
+ * `scope` reports whether the chain ran over inputs it could actually
+ * consult (ADR-3180 Decision 2/§7.7 — mirrors `scanPhasePlans`'s
+ * scope-carrying result in `plan-scan.cts`; see `planning-scope.cjs`). This
+ * function's own ladder always runs to completion on whatever `fm`/`body` it
+ * is given, INCLUDING when the answer is `null` — a genuinely absent field is
+ * a real answer, not a failure to look (§7.7 behavior table row 4). So
+ * `scope` defaults to `SCOPE.COMPLETE` and is only ever something else when
+ * the CALLER passes `opts.scope`, because only the caller knows whether an
+ * input it handed in was itself degraded — e.g. `fm` came back `{}` from an
+ * unterminated frontmatter fence (`extractFrontmatter` swallows that parse
+ * failure), or `body` is an unscoped whole-document fallback because a
+ * required `## Current Position` section was not found (#2956). This
+ * function never invents a new `SCOPE` member — the enum is frozen at
+ * COMPLETE/TRUNCATED/UNSCOPED/UNREADABLE (`planning-scope.cjs`).
+ *
+ * #1760 is the fallback chain's origin.
+ */
+export function stateFieldValue(
+  fm: Record<string, unknown>,
+  body: string,
+  fmKey: string | null,
+  bodyField: string | null,
+  opts?: { scope?: Scope },
+): { value: string | null; scope: Scope } {
+  const v = fmKey === null ? undefined : fm[fmKey];
+  let value: string | null;
+  if (typeof v === 'string' && v.trim()) {
+    value = v.trim();
+  } else if (typeof v === 'number' || typeof v === 'boolean') {
+    value = String(v);
+  } else {
+    value = bodyField === null ? null : stateExtractField(body, bodyField);
+  }
+  return { value, scope: opts?.scope ?? SCOPE.COMPLETE };
+}
+
 /**
 * Match the "Current Position" section body from a STATE.md body. #2956: this
 * is the Phase analogue of state.cts's matchSessionSection. `Phase` canonically
--- a/src/state.cts
+++ b/src/state.cts
@@ -49,6 +49,7 @@ import {
  normalizeStateStatus,
  shouldPreserveExistingProgress,
  stateExtractField,
+  stateFieldValue,
  stateReplaceField,
  KNOWN_TEMPLATE_DEFAULTS,
  stateReplaceFieldIfTemplate,
@@ -1456,18 +1457,9 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void {
  const fm = extractFrontmatter(content, statePath) as Record<string, unknown>;
  const body = stripFrontmatter(content);

-  // Helper: return frontmatter scalar value when present and non-empty.
-  // Accepts strings, numbers, and booleans — coercing non-string primitives to
-  // their string representation so callers always receive string | null.
-  // Returns null for missing, null/undefined, or empty-after-trim values so
-  // the caller falls back to body extraction.
-  const fmScalar = (key: string): string | null => {
-    const v = fm[key];
-    if (v === null || v === undefined) return null;
-    if (typeof v === 'string') return v.trim() || null;
-    if (typeof v === 'number' || typeof v === 'boolean') return String(v);
-    return null;
-  };
+  // #3187: frontmatter-scalar-then-body-field precedence is owned by
+  // state-document.cjs's `stateFieldValue` (ADR-3180 §7.7) — this function no
+  // longer holds its own fmScalar ladder.

  // Extract basic fields — frontmatter keys take precedence over body
  // #2956: scope `Phase` extraction to ## Current Position so a historical
@@ -1477,24 +1469,24 @@ function cmdStateSnapshot(cwd: string, raw: boolean): void {
  // full-body search only when no ## Current Position section exists, so files
  // with no section heading keep their current behaviour.
  const currentPositionScope = matchCurrentPositionSection(body) ?? body;
-  const prosePhase = parseProsePhaseField(stateExtractField(currentPositionScope, 'Phase'));
-  const currentPhase = fmScalar('current_phase') ?? stateExtractField(body, 'Current Phase') ?? prosePhase.phase;
-  const currentPhaseName = fmScalar('current_phase_name') ?? stateExtractField(body, 'Current Phase Name') ?? prosePhase.name;
-  const totalPhasesRaw = fmScalar('total_phases') ?? stateExtractField(body, 'Total Phases');
-  const currentPlan = fmScalar('current_plan') ?? stateExtractField(body, 'Current Plan');
-  const totalPlansRaw = fmScalar('total_plans_in_phase') ?? stateExtractField(body, 'Total Plans in Phase');
-  const status = fmScalar('status') ?? stateExtractField(body, 'Status');
-  const progressRaw = fmScalar('progress') ?? stateExtractField(body, 'Progress');
-  const rawLastActivity = stateExtractField(body, 'Last Activity') ?? stateExtractField(body, 'Last activity');
+  const prosePhase = parseProsePhaseField(stateFieldValue(fm, currentPositionScope, null, 'Phase').value);
+  const currentPhase = stateFieldValue(fm, body, 'current_phase', 'Current Phase').value ?? prosePhase.phase;
+  const currentPhaseName = stateFieldValue(fm, body, 'current_phase_name', 'Current Phase Name').value ?? prosePhase.name;
+  const totalPhasesRaw = stateFieldValue(fm, body, 'total_phases', 'Total Phases').value;
+  const currentPlan = stateFieldValue(fm, body, 'current_plan', 'Current Plan').value;
+  const totalPlansRaw = stateFieldValue(fm, body, 'total_plans_in_phase', 'Total Plans in Phase').value;
+  const status = stateFieldValue(fm, body, 'status', 'Status').value;
+  const progressRaw = stateFieldValue(fm, body, 'progress', 'Progress').value;
+  const rawLastActivity = stateFieldValue(fm, body, null, 'Last Activity').value ?? stateFieldValue(fm, body, null, 'Last activity').value;
  const proseLastActivity = parseProseLastActivityField(rawLastActivity);
-  const lastActivity = fmScalar('last_activity') ?? proseLastActivity.date ?? rawLastActivity;
-  const lastActivityDesc = fmScalar('last_activity_desc') ?? stateExtractField(body, 'Last Activity Description') ?? proseLastActivity.description;
+  const lastActivity = stateFieldValue(fm, body, 'last_activity', null).value ?? proseLastActivity.date ?? rawLastActivity;
+  const lastActivityDesc = stateFieldValue(fm, body, 'last_activity_desc', 'Last Activity Description').value ?? proseLastActivity.description;
  // #2956: Paused At canonically lives in ## Session (see the comment above
  // preferNewerLastActivity and the write seam in buildStateFrontmatter). The
  // write seam already scopes it to ## Session; this read seam must agree, so a
  // stale "Paused At:" in a Session Continuity Archive cannot win here either.
  const sessionScope = matchSessionSection(body) ?? body;
-  const pausedAt = fmScalar('paused_at') ?? stateExtractField(sessionScope, 'Paused At');
+  const pausedAt = stateFieldValue(fm, sessionScope, 'paused_at', 'Paused At').value;

  // Parse numeric fields
  const totalPhases = totalPhasesRaw ? parseInt(totalPhasesRaw, 10) : null;
@@ -2843,8 +2835,84 @@ function cmdStateMilestoneSwitch(cwd: string, version: string | undefined, name:

 /**
 * Gate 1: Validate STATE.md against filesystem.
- * Returns { valid, warnings, drift } JSON.
+ * Returns { valid, warnings, drift, scope } JSON.
+ *
+ * #3187 (ADR-3180 §7.7, Decisions 2-4): two defects fixed here.
+ *
+ * (1) #3162 THE HEADLINE. Every warning this function can emit used to be
+ * gated behind `if (currentPhase && fs.existsSync(phasesDir))`, and
+ * `currentPhase` came from a body-only `stateExtractField(content, 'Current
+ * Phase')` call with no frontmatter fallback. A STATE.md whose phase lives
+ * ONLY in frontmatter therefore resolved `currentPhase` to `null`, the whole
+ * drift block was skipped, and the function returned
+ * `{valid:true, warnings:[], drift:{}}` — "could not look" was
+ * output-identical to "looked, all clean." Current Phase / Status / Total
+ * Plans in Phase now route through `stateFieldValue` (the single owner of the
+ * #1760 frontmatter-then-body fallback chain), so the frontmatter tier is
+ * actually consulted.
+ *
+ * (2) #1255 FRONTMATTER SHADOWING. The old code passed UNSTRIPPED `content`
+ * to the extractor. `stateExtractField`'s plain-format branch is
+ * `^Field:` with the `i` flag, so a frontmatter `status:` key matched the
+ * pattern for the body field `Status` and won, because the frontmatter block
+ * precedes the body. Parsed once now — `extractFrontmatter` +
+ * `stripFrontmatter` — and `fm`/`body` are handed to the chain owner, exactly
+ * as `advancePlanCore`/`beginPhaseCore`/`completePhaseCore`/
+ * `readModifyWriteStateMd` already guard against this class of defect.
+ *
+ * `scope` (ADR-3180 Decision 2) reports whether the derivation actually ran:
+ *   - `COMPLETE`  — the phase-vs-disk derivation ran over usable input,
+ *     including when it legitimately finds no VERIFICATION.md / no matching
+ *     phase directory (a real answer, not a non-answer).
+ *   - `UNSCOPED`  — Current Phase could not be resolved by ANY chain step (no
+ *     frontmatter scalar, no body field), so the drift derivation had no
+ *     phase to scope its disk lookup to and could not run at all. Reporting
+ *     this as COMPLETE would recreate the #3162 collapse this phase closes,
+ *     one layer out.
+ *   - `UNREADABLE` — the frontmatter parse or the phases-dir scan itself
+ *     could not be consulted (an existing `catch` block used to swallow this
+ *     silently; the degrade stays, but is now visible).
+ *
+ * ⛔ Rejected (ADR-3180 §7.7 Rejected #2): a non-`COMPLETE` scope is never
+ * routed to `valid:false`. `valid` keeps meaning "no drift warnings were
+ * found"; `scope` says whether the derivation could actually run. A caller
+ * branches on both — folding them into one boolean recreates the exact
+ * collapse this epic removes, in the opposite direction (a legacy STATE.md
+ * with no resolvable phase is a supported degrade, not an invalid document).
 */
+
+/**
+ * #1255/#3187: parse frontmatter and strip it from the body ONCE, shared by
+ * `cmdStateValidate` and `cmdStateCompletePhase` so both consult the identical
+ * fm/body precedence and degrade identically when the frontmatter half of the
+ * chain cannot be consulted. Extracted (code-review finding, epic #3180): the
+ * two call sites previously carried a byte-identical try/catch, comments
+ * included — an epic whose own thesis is "one canonical owner per
+ * derivation" must not ship a duplicated derivation in its own diff.
+ *
+ * Returns `scope: SCOPE.COMPLETE` unless the frontmatter parse itself threw,
+ * in which case `fm` degrades to `{}` and `scope` becomes `SCOPE.UNREADABLE`
+ * — callers that mutate `scope` further (e.g. `cmdStateValidate`'s later
+ * UNSCOPED/disk-scan degrades) start from this returned value rather than a
+ * fresh `SCOPE.COMPLETE`.
+ */
+function readStateFrontmatterScoped(content: string, statePath: string): { fm: Record<string, unknown>; body: string; scope: planningScopeMod.Scope } {
+  let fm: Record<string, unknown>;
+  let scope: planningScopeMod.Scope = SCOPE.COMPLETE;
+  try {
+    fm = extractFrontmatter(content, statePath);
+  } catch {
+    // extractFrontmatter is documented never to throw, but this mirrors the
+    // defensive try/catch already used around it elsewhere in this file
+    // (e.g. spliceFrontmatter) — a parse hiccup here means the frontmatter
+    // half of the chain could not be consulted; degrade visibly.
+    fm = {};
+    scope = SCOPE.UNREADABLE;
+  }
+  const body = stripFrontmatter(content);
+  return { fm, body, scope };
+}
+
 function cmdStateValidate(cwd: string, raw: boolean): void {
  const statePath = planningPaths(cwd).state;
  if (!fs.existsSync(statePath)) {
@@ -2864,22 +2932,36 @@ function cmdStateValidate(cwd: string, raw: boolean): void {
  const warnings: string[] = [];
  const drift: Record<string, unknown> = {};

-  const status = stateExtractField(content, 'Status') || '';
-  const currentPhase = stateExtractField(content, 'Current Phase');
-  const totalPlansRaw = stateExtractField(content, 'Total Plans in Phase');
+  // #1255/#3187: parse frontmatter and strip it from the body ONCE, so the
+  // chain owner sees the same fm/body precedence every other migrated call
+  // site sees. Pass statePath so a truncated STATE.md is named in the #1882
+  // diagnostic rather than reported under a content digest.
+  const { fm, body, scope: initialScope } = readStateFrontmatterScoped(content, statePath);
+  let scope: planningScopeMod.Scope = initialScope;
+
+  const status = stateFieldValue(fm, body, 'status', 'Status').value || '';
+  const currentPhase = stateFieldValue(fm, body, 'current_phase', 'Current Phase').value;
+  const totalPlansRaw = stateFieldValue(fm, body, 'total_plans_in_phase', 'Total Plans in Phase').value;
  const totalPlansInPhase = totalPlansRaw ? parseInt(totalPlansRaw, 10) : null;

  const phasesDir = planningPaths(cwd).phases;

-  // Scan disk for current phase
-  if (currentPhase && fs.existsSync(phasesDir)) {
+  if (currentPhase === null) {
+    // #3162: nothing to scope the disk lookup to — the derivation cannot run
+    // at all. Distinct from "ran, found nothing to warn about" (scope stays
+    // COMPLETE elsewhere in this function).
+    if (scope === SCOPE.COMPLETE) scope = SCOPE.UNSCOPED;
+  } else if (fs.existsSync(phasesDir)) {
    const normalized = currentPhase.replace(/\s+of\s+\d+.*/, '').trim();
    try {
      const entries = fs.readdirSync(phasesDir, { withFileTypes: true });
      const phaseDir = entries.find(e => e.isDirectory() && e.name.startsWith(normalized.replace(/^0+/, '').padStart(2, '0')));
      if (phaseDir) {
        const phaseDirPath = path.join(phasesDir, phaseDir.name);
-        const { planCount: diskPlans, summaryCount: diskSummaries } = scanPhasePlans(phaseDirPath);
+        const { planCount: diskPlans, summaryCount: diskSummaries, scope: planScanScope } = scanPhasePlans(phaseDirPath);
+        if (planScanScope !== SCOPE.COMPLETE && scope === SCOPE.COMPLETE) {
+          scope = planScanScope;
+        }

        // Check plan count mismatch
        if (totalPlansInPhase !== null && diskPlans !== totalPlansInPhase) {
@@ -2900,7 +2982,8 @@ function cmdStateValidate(cwd: string, raw: boolean): void {
          } catch { /* best-effort (#2245 audit): cmdStateValidate is a diagnostic
             * warnings scan across N VERIFICATION.md files — one unreadable file
             * (permission/race) must not abort the scan of the rest; it's simply
-             * excluded from drift detection. */ }
+             * excluded from drift detection. Does not degrade `scope` — the other
+             * N-1 files were consulted fine. */ }
        }

        // Check if all plans have summaries but status still says executing
@@ -2911,17 +2994,22 @@ function cmdStateValidate(cwd: string, raw: boolean): void {
          }
        }
      }
-    } catch { /* best-effort (#2245 audit): cmdStateValidate is a read-only
-       * diagnostic scan of the current phase's directory (readdirSync +
-       * scanPhasePlans). A disk-scan failure here means drift detection for
-       * this phase is skipped for this run, degrading to "no warnings from
-       * that scan" rather than crashing the validate command — the same
-       * degrade-on-scan-failure pattern buildStateFrontmatter's own disk scan
-       * already uses. */ }
+      // else: phase resolved, but no matching directory on disk — a real
+      // answer (row 22), not a look failure. scope stays COMPLETE.
+    } catch {
+      // #3162/#2245: previously a silent best-effort swallow. The disk-scan
+      // failure (readdirSync/scanPhasePlans) means drift detection for this
+      // phase could not run this pass — that degrade is kept (validate still
+      // does not crash), but is now visible via `scope` instead of being
+      // indistinguishable from a clean pass.
+      scope = SCOPE.UNREADABLE;
+    }
  }
+  // else: phasesDir itself does not exist — a real answer (no phases on disk
+  // yet), not a look failure. scope stays COMPLETE.

  const valid = warnings.length === 0;
-  output({ valid, warnings, drift }, raw, undefined);
+  output({ valid, warnings, drift, scope }, raw, undefined);
 }

 /**
@@ -3113,18 +3201,16 @@ function cmdStatePrune(cwd: string, options: StatePruneOptions, raw: boolean): v
  const rawState = fs.readFileSync(statePath, 'utf-8');
  const fm = extractFrontmatter(rawState, statePath) as Record<string, unknown>;
  const body = stripFrontmatter(rawState);
-  // Mirror buildStateFrontmatter's fmScalar: only string/number/boolean
-  // frontmatter scalars are usable (an object/array `current_phase` is ignored,
-  // which also avoids a base-to-string on a non-primitive).
-  const fmRawPhase = fm.current_phase;
-  const fmCurrentPhase =
-    typeof fmRawPhase === 'string' ? (fmRawPhase.trim() || null)
-      : typeof fmRawPhase === 'number' || typeof fmRawPhase === 'boolean' ? String(fmRawPhase)
-        : null;
+  // #3187: frontmatter-scalar-then-body-field precedence is owned by
+  // state-document.cjs's `stateFieldValue` (ADR-3180 §7.7). This comment
+  // previously claimed to mirror `buildStateFrontmatter`'s fmScalar — that
+  // attribution was stale: buildStateFrontmatter (state.cts:1620) reads body
+  // only and never consults frontmatter at all; this actually mirrored
+  // cmdStateSnapshot's now-removed closure instead.
  const positionSection = sliceCurrentPositionSection(body);
  const prosePhase =
-    positionSection !== null ? parseProsePhaseField(stateExtractField(positionSection, 'Phase')).phase : null;
-  const currentPhaseRaw = fmCurrentPhase ?? stateExtractField(body, 'Current Phase') ?? prosePhase;
+    positionSection !== null ? parseProsePhaseField(stateFieldValue(fm, positionSection, null, 'Phase').value).phase : null;
+  const currentPhaseRaw = stateFieldValue(fm, body, 'current_phase', 'Current Phase').value ?? prosePhase;
  const currentPhase = parseInt(String(currentPhaseRaw), 10) || 0;
  const cutoff = currentPhase - keepRecent;

@@ -3366,10 +3452,17 @@ function cmdStateRebuild(cwd: string, options: StateRebuildOptions, raw: boolean
 * that the phase execution is finished and the project is ready for the next phase.
 * Implements the `gsd state complete-phase` subcommand (issue #2735).
 */
-function resolvePhaseIdForCompletePhase(content: string, overridePhase: string | undefined): string | null {
+function resolvePhaseIdForCompletePhase(fm: Record<string, unknown>, body: string, overridePhase: string | undefined): string | null {
+  // #3187: route through the single #1760 fallback-chain owner (fm scalar
+  // then body field) instead of two raw stateExtractField calls on
+  // frontmatter-blind content — a STATE.md whose phase lives only in
+  // frontmatter no longer resolves to null here. `Phase` (the historical
+  // second-choice field name) has no frontmatter counterpart, so its fmKey
+  // is null — same shape as cmdStateSnapshot's `stateFieldValue(fm,
+  // currentPositionScope, null, 'Phase')` fallback.
  const candidate = overridePhase ||
-    stateExtractField(content, 'Current Phase') ||
-    stateExtractField(content, 'Phase') ||
+    stateFieldValue(fm, body, 'current_phase', 'Current Phase').value ||
+    stateFieldValue(fm, body, null, 'Phase').value ||
    '';

  // #2125: parse via the canonical anchored parser so a narrative `Phase:`
@@ -3389,7 +3482,36 @@ function cmdStateCompletePhase(cwd: string, raw: boolean, overridePhase?: string
  }

  const content = fs.readFileSync(statePath, 'utf-8');
-  const resolvedPhase = resolvePhaseIdForCompletePhase(content, overridePhase);
+  // #1255/#3187: parse frontmatter and strip it from the body ONCE, mirroring
+  // cmdStateValidate/cmdStateSnapshot, so resolvePhaseIdForCompletePhase and
+  // the idempotency guard below consult the identical fm/body precedence —
+  // the two sites cannot drift onto different chains, extending the #2125
+  // "same canonical parser" guarantee one layer earlier.
+  const { fm, body, scope } = readStateFrontmatterScoped(content, statePath);
+
+  // #3187 Postel/visibility (design doc's sharpest case): this whole handler
+  // is the DESTRUCTIVE path the #3489 idempotency guard below protects — it
+  // decides whether a re-run of `state complete-phase --phase N` is allowed
+  // to roll STATE.md back to N's moment-of-completion. If the frontmatter
+  // half of the chain could not be consulted (`scope` UNREADABLE),
+  // `existingCurrentPhase` below could read as null even though the
+  // project's true current phase lives only in that unreadable frontmatter —
+  // silently treating a non-COMPLETE scope as "not complete" would let the
+  // guard's `existingCurrentPhase &&` check fail OPEN and re-run an
+  // already-completed phase. Refuse outright instead of guessing; this
+  // applies even when `--phase` is explicit, because the guard's job is to
+  // protect against exactly that already-completed-phase case regardless of
+  // how the target phase was named.
+  if (scope !== SCOPE.COMPLETE) {
+    output(
+      { error: 'Unable to read STATE.md frontmatter; refusing to run complete-phase to avoid a destructive rollback (#3489). Fix or remove the malformed frontmatter and retry.' },
+      raw,
+      undefined,
+    );
+    return;
+  }
+
+  const resolvedPhase = resolvePhaseIdForCompletePhase(fm, body, overridePhase);
  if (!resolvedPhase || /^phase$/i.test(resolvedPhase)) {
    output({ error: 'Unable to resolve current phase. Pass an explicit phase: state complete-phase --phase <N>' }, raw, undefined);
    return;
@@ -3404,7 +3526,7 @@ function cmdStateCompletePhase(cwd: string, raw: boolean, overridePhase?: string
  // Last Activity, Last Activity Description, and the Current Position body.
  // The handler is now a no-op in that case so re-invocation from downstream
  // workflows cannot regress the project state.
-  const existingCurrentPhaseRaw = stateExtractField(content, 'Current Phase') || '';
+  const existingCurrentPhaseRaw = stateFieldValue(fm, body, 'current_phase', 'Current Phase').value || '';
  // #2125: same canonical parser as resolvePhaseIdForCompletePhase so the two
  // sites cannot diverge on the token they extract.
  const existingCurrentPhase = parsePhaseFromProse(existingCurrentPhaseRaw).phase;
--- a/src/workstream-inventory.cts
+++ b/src/workstream-inventory.cts
@@ -23,7 +23,10 @@ import planScan = require('./plan-scan.cjs');
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import planningWorkspace = require('./planning-workspace.cjs');
 const { planningPaths, planningRoot, getActiveWorkstream } = planningWorkspace;
-import { stateExtractField } from './state-document.cjs';
+import { stateFieldValue } from './state-document.cjs';
+// eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module
+import frontmatterMod = require('./frontmatter.cjs');
+const { extractFrontmatter, stripFrontmatter } = frontmatterMod;
 import { findTableWithColumns } from './markdown-table.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports -- verification.cjs is an export= CommonJS module
 import verificationMod = require('./verification.cjs');
@@ -435,12 +438,29 @@ function writeVerificationLedger(wsDir: string, ledger: VerificationLedger): voi
 function readStateProjection(statePath: string): StateProjection {
  try {
    const stateContent = fs.readFileSync(statePath, 'utf-8');
+    // #3187: route Status/Current Phase/Last Activity through the single
+    // #1760 fallback-chain owner (state-document.cjs's stateFieldValue)
+    // instead of a frontmatter-blind stateExtractField(stateContent, …) call,
+    // mirroring cmdStateValidate/cmdStateSnapshot — a STATE.md whose fields
+    // live only in frontmatter is no longer projected as absent here.
+    const fm = extractFrontmatter(stateContent, statePath) as Record<string, unknown>;
+    const body = stripFrontmatter(stateContent);
    return {
-      status: stateExtractField(stateContent, 'Status') || 'unknown',
-      current_phase: stateExtractField(stateContent, 'Current Phase'),
-      last_activity: stateExtractField(stateContent, 'Last Activity'),
+      status: stateFieldValue(fm, body, 'status', 'Status').value || 'unknown',
+      current_phase: stateFieldValue(fm, body, 'current_phase', 'Current Phase').value,
+      last_activity: stateFieldValue(fm, body, 'last_activity', 'Last Activity').value,
    };
  } catch {
+    // Read/parse failure (missing file, permission fault, etc.) degrades to
+    // an all-unknown projection — unchanged. Not widened to also carry a
+    // `scope` here: doing so would ripple `StateProjection`
+    // (workstream-inventory-builder.cjs) and every consumer of this
+    // read-only rollup — the design doc's blast-radius table rates
+    // `readStateProjection` "low"/Tier-2, and this call site's migration is
+    // scoped to routing the fallback chain, not to widening the return type.
+    // The existing all-unknown degrade already distinguishes "could not
+    // read" from any real field value; only its scope-vs-absence *reason*
+    // stays uncaptured, same as before this change.
    return {
      status: 'unknown',
      current_phase: null,