refactor(#3468): dispatch state preservation on the declared policy, not the field (#3495)

* test(#3468): add write-path drift guard, ratcheted at its measured baseline

Guard-first, per ADR-3180 Amendment 3's standing rule that a phase builds
and runs its guard BEFORE its scope is fixed, and states its copy count as
'N found by the guard', never 'N per the epic'.

Measured, not assumed:

  Axis 1 (policy dispatch, ADR-3408 section 8.1) — 7 violations, RED by
  design. 5 field-name-keyed getFieldClassification('literal') branches
  plus 2 declared FieldPreservation members with no executor at all
  (derive, clear). This is the fail-first evidence for the refactor.

  Axis 2 (write seam, section 8.3) — 4 bypasses, ratcheted. Epic #3408
  scoped this at two writers; the whole-repo scan found four, and one the
  epic named (patchCore) is not among them because it bypasses via
  stateReplaceField rather than the seam calls. Fourth consecutive time an
  epic's copy count proved a lower bound.

Two detectors were written and removed again before this commit, both
recorded in the file header rather than silently dropped:

  - A prompt-layer detector that reported 5 backticked prose mentions as
    drift. That is ADR-3180 Amendment 3's recorded false-positive class,
    and CONTRIBUTING.md already settles it: a backticked command reference
    is a mention. Now gated on inline-code spans.

  - A stateReplaceField co-occurrence detector for section 8.3(b). Measured
    at 29 false positives to 1 true positive — it matched the function's own
    definition and ~20 calls on frontmatter-free body slices. Banking 29
    non-defects to catch one is the 'ratchet as a parking lot' gaming route
    Decision 5 names, so it is a DECLARED KNOWN GAP owned by Phase 2
    (#3469), which both fixes it and makes its detection tractable.

* test(#3468): failing-first coverage for policy dispatch and the loud failure

Matrix sections A, B and C from 50-test-matrix.md.

Expected RED against this tree, confirmed by static trace rather than
assumed:

  B1, B2, B3 — an unwired declared preserve-when-unchanged row must throw
  with code STATE_PRESERVATION_UNWIRED_ROW and a structured .field. Today
  src/state-transition.cts:314 silently continues.

  A4 — a whitespace-only snapshot is restored today, because the guard is
  .length > 0. Required behavior is skip.

Everything else is characterization, locking in behavior the refactor must
preserve. C1 is table-driven over every FIELD_CLASSIFICATION key; C2 pins
current_phase_name's exact outputs as literals, because its row is being
reclassified preserve-always to preserve-when-unchanged as a
behavior-preserving change and nothing else would catch a drift. C3 is a
seeded fast-check property (seed 3468, 200 runs, replay data on failure).

A22 is deliberately NOT a behavioral test. Whether 'derive' has an explicit
executor is not observable through applyStatePreservation's public API — it
is a structural property, and the drift guard's unimplemented_policy axis is
what enforces it. That split is ADR-3408 Decision 5's own pairing: the lint
is the structural metric, the test is the outcome metric, and neither is
reported alone.

* refactor(#3468): dispatch preservation on the declared policy, not the field

Implements ADR-3408 sections 8.1, 8.2 and 8.6.

applyStatePreservation is now one loop over FIELD_CLASSIFICATION dispatching
on the row's preservation value, with four small executors — one per
FieldPreservation member. No branch is selected by field name. Zero
literal-argument getFieldClassification calls remain.

Behavior-preserving for 16 of 20 input classes. The four that change:

  - An unwired declared preserve-when-unchanged row now THROWS
    (code STATE_PRESERVATION_UNWIRED_ROW, structured .field) instead of
    silently continuing. This fires only on an internal invariant violation
    with both ends in our own source; a drifted, malformed or unparseable
    user STATE.md must never reach it, which is section 8.2's bright line
    and what test B8 proves through the real CLI.
  - derive gained an explicit no-op executor. That is what makes the throw
    decidable: 'policy says do nothing' is now distinguishable from 'nobody
    wired this'.
  - current_phase_name's row is corrected from preserve-always to
    preserve-when-unchanged. The row was wrong, not the code — it has always
    been delta-gated on the body Phase line, so preserve-always had two
    divergent implementations. Behavior is unchanged and test C2 pins it.
  - A whitespace-only snapshot is no longer restored; the check is trimmed.

clear is deleted from the FieldPreservation union — no row used it and no
executor existed. Speculative Generality: a policy invented for a need that
never arrived. Verified zero dependents.

The caller folds six dedicated pre/post parameters into one bodyDeltas map
keyed by field, so all seven preserve-when-unchanged rows travel one channel
instead of two. Two shapes for one kind of data is why the executor needed
per-field branches at all.

Also fixed, found while reviewing the refactor rather than deferred:

  - applyPreserveIfPlaceholder opened with a field-name literal test, which
    section 8.1 forbids outright. The executor is idempotent, so the test
    bought nothing. The drift guard could not see it, so Axis 1 is widened
    to catch field-variable comparisons against literals — the guard
    reported zero while a violation sat in the file it polices, which is
    Goodhart's gaming-by-indirection.
  - loadBaseline conflated an unreadable baseline with an absent one. A
    guard whose own diagnostic collapses two states into one identical
    result reproduces the exact failure shape this epic exists to remove.

* docs(#3468): record Phase 1 validation as ADR-3408 Amendment 1

Amendment 1 records what Phase 1 found, per ADR-3408 section 8's rule that a
behavior it does not state is not decided:

- preserve-always had TWO divergent implementations; current_phase_name's
  row was wrong and is reclassified, behavior unchanged.
- section 8.6 resolved: clear is deleted, zero dependents.
- the closed guard vocabulary is real and has exactly one true member,
  because stopped_at's scoping turned out to be caller-side extraction.
- copy count found by the guard: 4 write-seam bypasses where the epic
  scoped 2, and patchCore — one of the two it named — is not among them.
- two detectors built and removed again, with their measured false-positive
  rates, so nobody re-attempts them.
- a DECLARED KNOWN GAP for section 8.3(b), owned by Phase 2.
- Decision 5's anti-gaming list earned itself twice in one phase.

Also adds the changeset fragment.

* test(#3468): fix review findings — try/finally, stale clear allowlist, ratchet owners

Standards axis, both hard violations:

  - tests/state-write-path-drift-guard.test.cjs wrapped stdout/argv/exitCode
    restoration in try/finally inside the test body. CONTRIBUTING.md:356
    forbids it outright, and the correct t.after() pattern was already in
    use two lines up in the same test.

  - tests/state-transition.test.cjs still listed 'clear' as an allowed
    FieldPreservation value in the row-enumeration test AND the
    getFieldClassification property test, after this PR deleted it. A stale
    allowlist weakens the property's negative space — it would accept a
    resurrected clear row as valid.

Contract tension, resolved rather than left:

  ADR-3408 section 8.3 requires each ratchet entry carry the issue owning
  its removal. All four shipped with owner: null. The guard was right not to
  INVENT one, but the owners are known from the phase plan, so recording
  them is not inventing: phase.cts -> #3469, state.cts and milestone.cts ->
  #3471, health-diagnostic.cts -> sanctioned-permanent.

  Rather than a JSDoc caveat, --baseline now MERGES prior owner values on
  the (file, source) key, so a mechanical regeneration can no longer
  silently discard curated provenance. Verified by regenerating twice.

* fix(#3468): sanitize attacker-controlled fields on every guard output path

Isolated security review, MEDIUM, confidence 8/10.

findSeamBypasses and findPromptSeamUses built findings with an UNSANITIZED
`file`, while the co-located `source` on the same object was correctly
wrapped in sanitizeForReport. On a fork PR a filename is exactly as
attacker-controlled as a source fragment — a repo can legally track a
filename carrying C1 control bytes or bidi overrides.

The raw value reached two paths: --json stdout, and the COMMITTED baseline
JSON via buildBaselineEntries. JSON.stringify neutralizes C0 controls but
does NOT escape C1 (0x7f-0x9f) nor the bidi/zero-width range
sanitizeForReport exists to strip — which is the precise threat the guard's
own header names. Only the human formatter was safe.

Sanitization now happens at CONSTRUCTION, so every consumer inherits it
rather than each output path having to remember. The same defect was present
on `field` and `policy` and is fixed alongside. Double-sanitization in the
formatter is left in place, verified idempotent: escaped output is ASCII and
cannot re-match the control/bidi classes.

Also: the guard was not referenced anywhere in package.json, so nothing ran
it. A drift guard nobody runs is not a guard, and ADR-3408 Decision 5 assumes
it runs. Wired into lint:ci beside its sibling drift guards; it was already
green on this tree, so the chain stays green.

* chore(#3468): re-curate ratchet after an upstream rewording of a tracked bypass

The rebase onto origin/next turned the guard red on its first real day, which
is the ratchet working rather than a defect.

c90ae479f fix(#3350) reworded cmdPhaseComplete's syncStateFrontmatter call
onto one line and changed its third argument. Because entries are keyed on
(file, trimmed source text) rather than a line number, that single upstream
edit registered as BOTH a stale acknowledgment and an unrecorded site — the
two-sided signal the design intends, forcing a human to look rather than
letting a tracked bypass drift out of view.

The owner-preserving merge behaved exactly as designed: three owners survived
because their keys were unchanged, and phase.cts's dropped to null because its
source text is genuinely a different key. Re-curated to #3469, the phase that
owns its removal.

Note for Phase 2: c90ae479f is #3350's fix landing independently on next —
one of the two instances Phase 2 was scoped to drive fail-first. Surfaced to
the epic rather than absorbed silently.

* test(#3468): derive B1's fixture from the table so it cannot go stale

Checkpoint 2 came back with 2 failures of 33803, both B1:

  actual   'current_phase_name'
  expected 'current_plan'

The implementation was right and the test was stale. B1 hand-built a
bodyDeltas literal intending current_plan to be the ONLY unwired row, but it
also omitted status, stopped_at and current_phase_name — all three of which
became preserve-when-unchanged rows in THIS PR. Table order puts
current_phase_name first, so the throw correctly named it.

B1 now builds from neutralBodyDeltas() and deletes exactly one key, which is
what its own comment always claimed it did. A future table change can no
longer silently make it assert the wrong field.

Audited every other bodyDeltas literal in the file: four exist, all correct —
two enumerate all seven rows explicitly, two pass {} where the emptiness is
the point of the test. Roughly thirty other sites already derive from the
helper.

Also renames the local unchchangedChanged to lastActivityDescChangedDeltas.
A typo'd identifier that happens to work is still a Mysterious Name; noted
during research and fixed now that this change touches the file.

* chore(#3468): re-curate ratchet and fold the seam channel into the shared helper

The rebase onto be9329b10 fix(#3374) was a true semantic conflict, resolved
rather than handed back, because the resolution was determinable:

That PR extracted the post-sync preservation pass into a shared
applyPostSyncPreservation helper — which is ADR-3408 section 8.3, i.e. a
piece of Phase 2's own deliverable, landing upstream. Its structure is kept
wholesale; this branch's contribution is applied INSIDE it.

That combination had to be checked rather than assumed. Upstream's helper
wires only FOUR bodyDeltas keys and still passes status / stopped_at /
current_phase_name through six dedicated parameters. This branch reclassifies
current_phase_name to preserve-when-unchanged, deletes those six parameters
from StatePreservationInput, and makes an unwired declared row THROW. Taking
upstream's file as-is would therefore have thrown on EVERY STATE.md write.

The helper now wires all seven rows through the single channel. Verified
7-to-7 against FIELD_CLASSIFICATION, with a clean tsc — which is the real
proof the dedicated parameters are gone, since they no longer exist on the
input type.

The ratchet also caught the same phase.cts call being reworded a second time,
reporting it as both a stale acknowledgment and an unrecorded site. Re-curated
to #3469. Recording the tradeoff plainly: keying on (file, source text) means
an upstream reword of a tracked line needs re-curation, where keying on line
numbers would churn on every unrelated edit. ADR-3180 Decision 4(e) chose
source text deliberately, and the owner-preserving merge added earlier covers
the common case where the text is unchanged.

* chore(#3468): backfill pr number in changeset fragment

---------

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

src/state-transition.cts

@@ -51,10 +51,31 @@ export type FieldPreservation =
   | 'derive' // always re-derive from source
   | 'preserve-when-unchanged' // #1230 delta heuristic: keep existing if body source field unchanged
   | 'preserve-always' // never overwrite unless the caller explicitly names this field
-  | 'preserve-if-placeholder' // overwrite only when derived value is a known placeholder (#948)
-  | 'clear'; // remove the field entirely
+  | 'preserve-if-placeholder'; // overwrite only when derived value is a known placeholder (#948)
+// ADR-3408 §8.6 amendment: 'clear' was deleted (no row used it, no executor
+// existed) rather than implemented — Speculative Generality, a policy
+// invented for a need that never arrived.
 
-export type FieldClassification = { source: FieldSource; preservation: FieldPreservation };
+/**
+ * ADR-3408 Decision 1 (Greenspun's Tenth Rule): guards and merge strategies
+ * are named members of a CLOSED vocabulary, never an open predicate slot. The
+ * table has already accreted five times (`preserve-always` #1743/#1695,
+ * `preserve-if-placeholder` #948/#2135, `state_head` #2573, `deriveProgressKeys`
+ * #2440, `bodyDeltas` #3258) — an open per-row predicate is what would turn
+ * this table into an interpreter. Adding a member to either union below is an
+ * amendment to ADR-3408, not a table edit.
+ */
+export type FieldGuard = 'non-sentinel-unknown';
+export type FieldMergeStrategy = 'progress-ratchet';
+
+export type FieldClassification = {
+  source: FieldSource;
+  preservation: FieldPreservation;
+  /** Closed vocabulary (see the comment above). Adding a member is an ADR-3408 amendment, not a table edit. */
+  guard?: FieldGuard;
+  /** Closed vocabulary. Same rule. */
+  mergeStrategy?: FieldMergeStrategy;
+};
 
 /**
  * Single source of truth for "which fields win when frontmatter and body
@@ -83,11 +104,17 @@ export const FIELD_CLASSIFICATION: Readonly<Record<string, FieldClassification>>
 
       // Phase / plan position (body-derived)
       current_phase: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification,
-      current_phase_name: { source: 'curated', preservation: 'preserve-always' } as FieldClassification, // #1743, #1695
+      // #1743, #1695. #3468: row corrected to match its long-standing behavior
+      // — was declared preserve-always, has always been delta-gated (only
+      // restores when the body `Phase:` source is unchanged this write).
+      current_phase_name: { source: 'curated', preservation: 'preserve-when-unchanged' } as FieldClassification,
       current_plan: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification,
 
       // Status / lifecycle (body-derived; #1230 delta heuristic applies)
-      status: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification,
+      // guard: the 'unknown' sentinel is the ONLY true executor-side guard in
+      // this table (stopped_at's `## Session` scoping is caller-side delta
+      // extraction, not an executor condition) — ADR-3408 Decision 1.
+      status: { source: 'body', preservation: 'preserve-when-unchanged', guard: 'non-sentinel-unknown' } as FieldClassification,
       stopped_at: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification,
       paused_at: { source: 'body', preservation: 'preserve-when-unchanged' } as FieldClassification,
 
@@ -102,7 +129,10 @@ export const FIELD_CLASSIFICATION: Readonly<Record<string, FieldClassification>>
       state_head: { source: 'free', preservation: 'derive' } as FieldClassification, // #2573
 
       // Progress block (disk-derived, except the curated progress ratchet)
-      progress: { source: 'curated', preservation: 'preserve-always' } as FieldClassification, // #3242, #1446
+      // mergeStrategy: 'progress-ratchet' — completed_plans/completed_phases
+      // only ever ratchet UP toward the derived value (#2969); everything
+      // else in the merge is either always-derived (#2440) or always-curated.
+      progress: { source: 'curated', preservation: 'preserve-always', mergeStrategy: 'progress-ratchet' } as FieldClassification, // #3242, #1446
       'progress.total_phases': { source: 'disk', preservation: 'derive' } as FieldClassification,
       'progress.completed_phases': { source: 'disk', preservation: 'derive' } as FieldClassification,
       'progress.total_plans': { source: 'disk', preservation: 'derive' } as FieldClassification,
@@ -144,12 +174,6 @@ export type StatePreservationInput = {
   preFmSnapshot: Record<string, unknown>;
   /** True when the caller asked for a full disk re-derivation (sync / advancePlan / completePhase). */
   resync: boolean;
-  preBodyStatus: string | null;
-  postBodyStatus: string | null;
-  preBodyStoppedAt: string | null;
-  postBodyStoppedAt: string | null;
-  preBodyPhaseSource: string | null;
-  postBodyPhaseSource: string | null;
   /**
    * #2440: when true, total_plans and total_phases take the derived (post-sync)
    * value even under !resync, instead of the wholesale curated restore. Used
@@ -159,16 +183,18 @@ export type StatePreservationInput = {
    */
   deriveProgressKeys?: boolean;
   /**
-   * #3258: pre/post body-source values for the preserve-when-unchanged (#1230
-   * delta heuristic) family, keyed by frontmatter field. The caller snapshots
-   * each field's body source before/after the transform (mirroring how
-   * buildStateFrontmatter derives it); applyStatePreservation consults
-   * FIELD_CLASSIFICATION and, for every row whose policy is preserve-when-
-   * unchanged that is NOT already on a dedicated channel (status, stopped_at),
+   * #3258 / #3468: pre/post body-source values for every preserve-when-unchanged
+   * (#1230 delta heuristic) row, keyed by the FRONTMATTER field the policy
+   * guards — one channel for all seven rows (status, stopped_at,
+   * current_phase_name, current_phase, current_plan, paused_at,
+   * last_activity_desc), folded from the two channels (a bodyDeltas map plus
+   * three dedicated pre/post parameter pairs) #3468 replaced. The caller
+   * snapshots each field's body source before/after the transform (mirroring
+   * how buildStateFrontmatter derives it); `applyPreserveWhenUnchanged`
    * restores the pre-write frontmatter value when that body source was left
-   * unchanged by this write. Adding a preserve-when-unchanged row is a one-row
-   * table edit PLUS a bodyDeltas entry from the caller; the invariant test in
-   * tests/state-transition.test.cjs fails if either is missing.
+   * unchanged by this write. Adding a preserve-when-unchanged row is a
+   * one-row table edit PLUS a bodyDeltas entry from the caller — omitting the
+   * entry is not a silent no-op, it THROWS (ADR-3408 §8.2, `applyPreserveWhenUnchanged`).
    */
   bodyDeltas?: Record<string, { pre: string | null; post: string | null }>;
 };
@@ -179,187 +205,225 @@ export type StatePreservationResult = {
 };
 
 /**
- * Pure, table-driven post-sync preservation. Mutates `postFm` in place to
- * mirror the pre-consolidation inline block (which also mutated in place) and
- * returns whether any field was restored.
+ * Mutable, single-write dispatch context threaded through every policy
+ * executor below. `mutated` accumulates across the whole field loop (ADR-3408
+ * §8.1 — one executor per policy, sharing one result).
  */
-export function applyStatePreservation(input: StatePreservationInput): StatePreservationResult {
-  const { preFm, postFm, preFmSnapshot, resync } = input;
-  let mutated = false;
+type PreservationCtx = {
+  preFm: Record<string, unknown> | null;
+  postFm: Record<string, unknown>;
+  preFmSnapshot: Record<string, unknown>;
+  resync: boolean;
+  deriveProgressKeys: boolean;
+  bodyDeltas: Record<string, { pre: string | null; post: string | null }> | undefined;
+  mutated: boolean;
+};
 
-  // Curated progress ratchet (#3242/#1446; closes the #1264 class by routing
-  // the policy through the table). Restored only when the table says preserve-
-  // always AND this transition is not re-deriving from disk (!resync). sync and
-  // the lifecycle transitions pass resync=true and recompute; patch/update and
-  // body-only writes pass resync=false and keep the curated counters.
-  const progressCls = getFieldClassification('progress');
-  if (
-    progressCls !== null &&
-    progressCls.preservation === 'preserve-always' &&
-    !resync &&
-    preFm &&
-    preFm['progress']
-  ) {
-    // #2440: when the caller opts in (deriveProgressKeys), total_plans and
-    // total_phases always take the derived (post-sync) value even under !resync.
-    // This is used by cmdStatePlannedPhase where total_plans must correct upward
-    // after plans are added. For body-only writes (state.update/patch without
-    // the flag), the wholesale restore preserves everything as before — the
-    // #3242 Bug A protection stays fully in force.
-    if (input.deriveProgressKeys && postFm['progress']) {
-      const curated = preFm['progress'] as Record<string, unknown> | null;
-      const derived = (postFm['progress'] ?? {}) as Record<string, unknown>;
-      const merged: Record<string, unknown> = { ...derived };
-      if (curated) {
-        // #2440: total_plans and total_phases always take the derived value.
-        // #2969: completed_plans and completed_phases take the derived value
-        // when it is GREATER than the curated value (gap-closure plans that
-        // completed after the plan count grew) — ratcheting UP only, never
-        // deriving downward (preserves the #3242 curated-progress protection
-        // for cases unrelated to plan-count growth, e.g. a deleted SUMMARY).
-        // percent also takes the derived value — the resync recomputed it from
-        // disk counts, and a stale curated percent would be incoherent against
-        // the ratcheted-up completed counts (e.g. 54/54 at 93%).
-        const ratchetUpKeys = new Set(['completed_plans', 'completed_phases']);
-        for (const [key, value] of Object.entries(curated)) {
-          if (key === 'total_plans' || key === 'total_phases' || key === 'percent') continue;
-          if (ratchetUpKeys.has(key)) {
-            const derivedNum = typeof derived[key] === 'number' ? derived[key] : -Infinity;
-            const curatedNum = typeof value === 'number' ? value : -Infinity;
-            // Take the derived value only when it ratchets up; else keep curated.
-            if (derivedNum > curatedNum) continue;
-            merged[key] = value;
-          } else {
-            merged[key] = value;
-          }
+/**
+ * ADR-3408 §8.2: an unenforced `preserve-when-unchanged` row throws. Both
+ * ends of this invariant are gsd-core's own source — a declared row the
+ * *caller code* forgot to wire via `bodyDeltas` — so it is a programming
+ * error, unreachable from any user document. The bright line, stated because
+ * conflating its two sides would be severe: a drifted, malformed, or
+ * unparseable user STATE.md NEVER reaches this throw (§8.5 governs that case
+ * with preserve-and-warn); this fires only when the *caller* omitted a
+ * `bodyDeltas` entry for a row the table itself declares. Getting this
+ * backwards turns every desynced project's `phase.complete` into a hard
+ * failure.
+ */
+function throwUnwiredRow(field: string): never {
+  const err = new Error(
+    `applyStatePreservation: preserve-when-unchanged row ${JSON.stringify(field)} reached the ` +
+    'executor with no wired ctx.bodyDeltas entry. This is an internal invariant violation (ADR-3408 ' +
+    '§8.2) — the caller (readModifyWriteStateMd) forgot to supply this field\'s body-source delta. ' +
+    'Add a bodyDeltas entry for this field per ADR-3408 §8.3, or remove the row from ' +
+    'FIELD_CLASSIFICATION if the field no longer needs this policy.',
+  ) as Error & { code: string; field: string };
+  err.code = 'STATE_PRESERVATION_UNWIRED_ROW';
+  err.field = field;
+  throw err;
+}
+
+/**
+ * Executor for `preservation: 'preserve-when-unchanged'` (ADR-3408 §8.1). The
+ * #1230 delta heuristic: restore the pre-write frontmatter snapshot when this
+ * write did not change the field's body source, and the snapshot is a real
+ * (non-empty-after-trim) curated value the derived value should not clobber.
+ *
+ * Every row carrying this policy — status, stopped_at, current_phase_name,
+ * current_phase, current_plan, paused_at, last_activity_desc — is honored by
+ * this ONE executor; `cls.guard` is the only field-specific variation (the
+ * closed vocabulary of ADR-3408 Decision 1).
+ */
+function applyPreserveWhenUnchanged(field: string, cls: FieldClassification, ctx: PreservationCtx): void {
+  // 1. A declared row with no wired delta is an internal invariant violation
+  // — throw (ADR-3408 §8.2). Never reached for a user-document defect: the
+  // production caller (readModifyWriteStateMd) wires every preserve-when-
+  // unchanged row unconditionally.
+  const delta = ctx.bodyDeltas ? ctx.bodyDeltas[field] : undefined;
+  if (!delta) throwUnwiredRow(field);
+
+  // 2. Only a real, non-whitespace-only curated string is worth restoring
+  // (#3468: tightened from `.length > 0` to a trimmed check — a whitespace-
+  // only snapshot is not a real curated value).
+  const snapshot = ctx.preFmSnapshot[field];
+  if (typeof snapshot !== 'string' || snapshot.trim().length === 0) return;
+
+  // 3. Closed-vocabulary guard: status's 'unknown' sentinel is never restored.
+  // Exact-match, case-sensitive — 'Unknown' is a real value and IS restored.
+  if (cls.guard === 'non-sentinel-unknown' && snapshot === 'unknown') return;
+
+  // 4. The body source changed this write → the freshly-derived value wins.
+  if (delta.pre !== delta.post) return;
+
+  // 5. Already correct → no-op (avoid a spurious `mutated=true`).
+  if (ctx.postFm[field] === snapshot) return;
+
+  // 6. Restore.
+  ctx.postFm[field] = snapshot;
+  ctx.mutated = true;
+}
+
+/**
+ * Executor for `preservation: 'preserve-always'` (ADR-3408 §8.1). Only
+ * `progress` carries this policy today. Preserves #3242/#1446/#2440/#2969
+ * semantics byte-for-byte: gated on `!resync` and a truthy `preFm[field]`;
+ * the `mergeStrategy: 'progress-ratchet'` per-key merge only fires when the
+ * caller opts in via `deriveProgressKeys`, else the whole curated block wins
+ * wholesale.
+ */
+function applyPreserveAlways(field: string, cls: FieldClassification, ctx: PreservationCtx): void {
+  if (ctx.resync || !ctx.preFm || !ctx.preFm[field]) return;
+
+  if (cls.mergeStrategy === 'progress-ratchet' && ctx.deriveProgressKeys && ctx.postFm[field]) {
+    // #2440: total_plans and total_phases always take the derived (post-sync)
+    // value even under !resync. This is used by cmdStatePlannedPhase where
+    // total_plans must correct upward after plans are added. For body-only
+    // writes (state.update/patch without the flag), the wholesale restore
+    // below preserves everything as before — the #3242 Bug A protection
+    // stays fully in force.
+    const curated = ctx.preFm[field] as Record<string, unknown> | null;
+    const derived = (ctx.postFm[field] ?? {}) as Record<string, unknown>;
+    const merged: Record<string, unknown> = { ...derived };
+    if (curated) {
+      // #2440: total_plans and total_phases always take the derived value.
+      // #2969: completed_plans and completed_phases take the derived value
+      // when it is GREATER than the curated value (gap-closure plans that
+      // completed after the plan count grew) — ratcheting UP only, never
+      // deriving downward (preserves the #3242 curated-progress protection
+      // for cases unrelated to plan-count growth, e.g. a deleted SUMMARY).
+      // percent also takes the derived value — the resync recomputed it from
+      // disk counts, and a stale curated percent would be incoherent against
+      // the ratcheted-up completed counts (e.g. 54/54 at 93%).
+      const ratchetUpKeys = new Set(['completed_plans', 'completed_phases']);
+      for (const [key, value] of Object.entries(curated)) {
+        if (key === 'total_plans' || key === 'total_phases' || key === 'percent') continue;
+        if (ratchetUpKeys.has(key)) {
+          const derivedNum = typeof derived[key] === 'number' ? derived[key] : -Infinity;
+          const curatedNum = typeof value === 'number' ? value : -Infinity;
+          // Take the derived value only when it ratchets up (strictly
+          // greater — #2969's `>` not `>=`); else keep curated.
+          if (derivedNum > curatedNum) continue;
+          merged[key] = value;
+        } else {
+          merged[key] = value;
         }
       }
-      postFm['progress'] = merged;
-    } else {
-      postFm['progress'] = preFm['progress'];
     }
-    mutated = true;
+    ctx.postFm[field] = merged;
+  } else {
+    ctx.postFm[field] = ctx.preFm[field];
   }
+  ctx.mutated = true;
+}
 
-  // status — #1230 body-delta heuristic. Table: preserve-when-unchanged.
-  const statusCls = getFieldClassification('status');
+/**
+ * Executor for `preservation: 'preserve-if-placeholder'` (ADR-3408 §8.1).
+ * `milestone` and `milestone_name` both carry this policy in the table, and
+ * both rows dispatch into this SAME executor body — no branch is selected by
+ * field name (ADR-3408 §8.1). The body always restores the name+version pair
+ * together (#948/#2135), ignoring which of the two rows triggered the call;
+ * this is deliberately safe because the executor is idempotent: whichever
+ * row fires first either performs the restore (after which the second row's
+ * call recomputes against already-restored state and finds nothing left to
+ * do) or finds no placeholder to restore (in which case the second row's
+ * call, seeing the same unchanged inputs, reaches the same conclusion). Two
+ * dispatches per write converge to the identical single-pass result, so the
+ * field argument itself is unused here — it exists only to satisfy the
+ * shared executor signature every policy branch in the dispatch loop shares.
+ */
+function applyPreserveIfPlaceholder(_field: string, _cls: FieldClassification, ctx: PreservationCtx): void {
+  const MILESTONE_PLACEHOLDER = 'milestone';
+  const derivedName = ctx.postFm['milestone_name'];
+  const derivedLooksLikeName = typeof derivedName === 'string'
+    && derivedName.length > 0
+    && derivedName !== MILESTONE_PLACEHOLDER
+    && !/^[\s—–:-]/.test(derivedName);
+  const snapshotName = ctx.preFmSnapshot['milestone_name'];
+  const snapshotNameIsReal = typeof snapshotName === 'string'
+    && snapshotName.length > 0
+    && snapshotName !== MILESTONE_PLACEHOLDER;
+  if (derivedLooksLikeName || !snapshotNameIsReal) return;
+
+  if (ctx.postFm['milestone_name'] !== snapshotName) {
+    ctx.postFm['milestone_name'] = snapshotName;
+    ctx.mutated = true;
+  }
+  const snapshotVersion = ctx.preFmSnapshot['milestone'];
   if (
-    statusCls !== null &&
-    statusCls.preservation === 'preserve-when-unchanged' &&
-    input.postBodyStatus === input.preBodyStatus &&
-    typeof preFmSnapshot['status'] === 'string' &&
-    preFmSnapshot['status'].length > 0 &&
-    preFmSnapshot['status'] !== 'unknown' &&
-    postFm['status'] !== preFmSnapshot['status']
+    typeof snapshotVersion === 'string' && snapshotVersion.length > 0 &&
+    ctx.postFm['milestone'] !== snapshotVersion
   ) {
-    postFm['status'] = preFmSnapshot['status'];
-    mutated = true;
+    ctx.postFm['milestone'] = snapshotVersion;
+    ctx.mutated = true;
   }
+}
 
-  // stopped_at — same #1230 body-delta heuristic. Table: preserve-when-unchanged.
-  const stoppedCls = getFieldClassification('stopped_at');
-  if (
-    stoppedCls !== null &&
-    stoppedCls.preservation === 'preserve-when-unchanged' &&
-    input.postBodyStoppedAt === input.preBodyStoppedAt &&
-    typeof preFmSnapshot['stopped_at'] === 'string' &&
-    preFmSnapshot['stopped_at'].length > 0 &&
-    postFm['stopped_at'] !== preFmSnapshot['stopped_at']
-  ) {
-    postFm['stopped_at'] = preFmSnapshot['stopped_at'];
-    mutated = true;
-  }
+/**
+ * Executor for `preservation: 'derive'`. Explicit no-op — the sync's
+ * freshly-derived value stands untouched. Naming this executor (rather than
+ * skipping `derive` rows by omission) is what makes ADR-3408 §8.2's throw
+ * decidable: "policy says do nothing" is now distinguishable from "nobody
+ * wired this", because every member of `FieldPreservation` reaches an
+ * executor.
+ */
+function applyDerive(_field: string, _cls: FieldClassification, _ctx: PreservationCtx): void {
+  // No-op by design — see docstring.
+}
 
-  // current_phase_name — curated (#1743/#1695). Table: preserve-always.
-  const phaseNameCls = getFieldClassification('current_phase_name');
-  if (
-    phaseNameCls !== null &&
-    phaseNameCls.preservation === 'preserve-always' &&
-    input.postBodyPhaseSource === input.preBodyPhaseSource &&
-    typeof preFmSnapshot['current_phase_name'] === 'string' &&
-    preFmSnapshot['current_phase_name'].length > 0 &&
-    postFm['current_phase_name'] !== preFmSnapshot['current_phase_name']
-  ) {
-    postFm['current_phase_name'] = preFmSnapshot['current_phase_name'];
-    mutated = true;
-  }
+/**
+ * Pure, table-driven post-sync preservation (ADR-3408 §8.1). One loop over
+ * `FIELD_CLASSIFICATION`, dispatching on the row's `preservation` value —
+ * never on the field name. Mutates `postFm` in place to mirror the
+ * pre-#3468 inline block (which also mutated in place) and returns whether
+ * any field was restored.
+ */
+export function applyStatePreservation(input: StatePreservationInput): StatePreservationResult {
+  const ctx: PreservationCtx = {
+    preFm: input.preFm,
+    postFm: input.postFm,
+    preFmSnapshot: input.preFmSnapshot,
+    resync: input.resync,
+    deriveProgressKeys: input.deriveProgressKeys === true,
+    bodyDeltas: input.bodyDeltas,
+    mutated: false,
+  };
 
-  // ─── #3258: the remaining preserve-when-unchanged rows ────────────────────
-  //
-  // status and stopped_at (above) carry field-specific guards (the 'unknown'
-  // sentinel; ## Session scoping done by the caller) and stay on their
-  // dedicated blocks. The other preserve-when-unchanged rows —
-  // last_activity_desc, paused_at, current_phase, current_plan — share a
-  // uniform #1230 delta heuristic with no extra sentinel, so they are driven
-  // from the table in one loop. This is the reconciliation the maintainer
-  // rescope to #3258 asked for: the row is honored by the table-consuming pass
-  // itself, not only by a weaker absent-value fallback elsewhere.
-  //
-  // For last_activity_desc this replaces preferNewerLastActivity's date-based
-  // desc writes (removed from syncStateFrontmatter + cmdStateJson): the field
-  // is now governed by exactly one rule, this one. For paused_at / current_phase
-  // / current_plan the declared delta heuristic is strictly stronger than the
-  // #905 absent-fallback that syncStateFrontmatter still applies on the
-  // writeStateMd / cmdStateJson paths this pass does not reach — same policy,
-  // two enforcement points, they agree by construction (both restore the
-  // pre-write snapshot value when they fire).
-  const bodyDeltas = input.bodyDeltas;
-  if (bodyDeltas) {
-    for (const field of Object.keys(FIELD_CLASSIFICATION)) {
-      const cls = getFieldClassification(field);
-      if (!cls || cls.preservation !== 'preserve-when-unchanged') continue;
-      // status / stopped_at stay on their dedicated blocks above.
-      if (field === 'status' || field === 'stopped_at') continue;
-      const delta = bodyDeltas[field];
-      if (!delta) continue; // caller did not wire this field's body source → cannot decide
-      const snapshot = preFmSnapshot[field];
-      if (typeof snapshot !== 'string' || snapshot.length === 0) continue;
-      if (delta.pre !== delta.post) continue; // body source changed this write → derived wins
-      if (postFm[field] === snapshot) continue; // already correct, no-op
-      postFm[field] = snapshot;
-      mutated = true;
+  for (const field of Object.keys(FIELD_CLASSIFICATION)) {
+    const cls = getFieldClassification(field);
+    if (!cls) continue;
+    if (cls.preservation === 'preserve-when-unchanged') {
+      applyPreserveWhenUnchanged(field, cls, ctx);
+    } else if (cls.preservation === 'preserve-always') {
+      applyPreserveAlways(field, cls, ctx);
+    } else if (cls.preservation === 'preserve-if-placeholder') {
+      applyPreserveIfPlaceholder(field, cls, ctx);
+    } else if (cls.preservation === 'derive') {
+      applyDerive(field, cls, ctx);
     }
   }
 
-  // ─── #3258: preserve-if-placeholder — milestone / milestone_name ──────────
-  //
-  // The derived milestone_name (from ROADMAP.md via buildStateFrontmatter) must
-  // not clobber a curated name when it resolves to the template placeholder
-  // 'milestone' or a punctuation-led fragment. Mirrors the #948/#2135 guard in
-  // syncStateFrontmatter so the table pass enforces the same policy on the RMW
-  // path; syncStateFrontmatter remains the enforcement point on the writeStateMd
-  // / cmdStateJson paths this pass does not reach (same policy, two points).
-  // The milestone VERSION is restored alongside the name, exactly as the sync
-  // guard does, so the two stay consistent.
-  const milestoneNameCls = getFieldClassification('milestone_name');
-  if (milestoneNameCls !== null && milestoneNameCls.preservation === 'preserve-if-placeholder') {
-    const MILESTONE_PLACEHOLDER = 'milestone';
-    const derivedName = postFm['milestone_name'];
-    const derivedLooksLikeName = typeof derivedName === 'string'
-      && derivedName.length > 0
-      && derivedName !== MILESTONE_PLACEHOLDER
-      && !/^[\s—–:-]/.test(derivedName);
-    const snapshotName = preFmSnapshot['milestone_name'];
-    const snapshotNameIsReal = typeof snapshotName === 'string'
-      && snapshotName.length > 0
-      && snapshotName !== MILESTONE_PLACEHOLDER;
-    if (!derivedLooksLikeName && snapshotNameIsReal) {
-      if (postFm['milestone_name'] !== snapshotName) {
-        postFm['milestone_name'] = snapshotName;
-        mutated = true;
-      }
-      const snapshotVersion = preFmSnapshot['milestone'];
-      if (
-        typeof snapshotVersion === 'string' && snapshotVersion.length > 0 &&
-        postFm['milestone'] !== snapshotVersion
-      ) {
-        postFm['milestone'] = snapshotVersion;
-        mutated = true;
-      }
-    }
-  }
-
-  return { postFm, mutated };
+  return { postFm: ctx.postFm, mutated: ctx.mutated };
 }
 
 // ----------------------------------------------------------------------------
@@ -1108,8 +1172,8 @@ function completePhaseCore(
   }
 
   // Current Phase Name — only written when a next-phase display name is known
-  // (#1743/#1695: classified curated/preserve-always, so an absent name does
-  // NOT clear an existing curated value).
+  // (#1743/#1695: classified curated/preserve-when-unchanged, so an absent
+  // name does NOT clear an existing curated value).
   if (nextPhaseDisplayName) {
     const after = stateReplaceField(body, 'Current Phase Name', nextPhaseDisplayName);
     if (after) {
@@ -1369,10 +1433,11 @@ function plannedPhaseCore(
  * preserved.
  *
  * This is a destructive reset intent: it intentionally overwrites the curated
- * `progress` / `current_phase_name` fields (classified preserve-always) because
- * a new milestone starts from zero. That is the intent's contract, not a
- * violation of the field-classification table — the table governs the steady-
- * state RMW transitions; a milestone boundary is an explicit reset.
+ * `progress` (preserve-always) / `current_phase_name` (preserve-when-unchanged)
+ * fields because a new milestone starts from zero. That is the intent's
+ * contract, not a violation of the field-classification table — the table
+ * governs the steady-state RMW transitions; a milestone boundary is an
+ * explicit reset.
  *
  * The adapter wraps this in `acquireStateLock` + `platformWriteSync` (NOT
  * `readModifyWriteStateMd`) because milestoneSwitch rebuilds frontmatter
@@ -1635,7 +1700,9 @@ function milestoneCompleteCore(
  *
  * The curated-field preservation that fixes #1743/#1695 is NOT in this core —
  * it lives in `readModifyWriteStateMd`'s post-sync delta (table-driven via
- * `getFieldClassification('current_phase_name').preservation === 'preserve-always'`).
+ * `current_phase_name`'s `preserve-when-unchanged` row, ADR-3408 §8.1 —
+ * reclassified from `preserve-always` in #3468 to match its long-standing,
+ * delta-gated behavior).
  * `patch` consulting the table "refuses to overwrite" curated fields implicitly:
  * when the patch does not change a curated field's body source line, the
  * existing frontmatter value wins over the sync re-derivation. The adapter

src/state.cts

@@ -2880,11 +2880,27 @@ function applyPostSyncPreservation(
     ?? stateExtractField(postBody, 'Last activity');
   const postBodyLastActivityDesc = stateExtractField(postBody, 'Last Activity Description')
     ?? parseProseLastActivityField(postBodyLastActivityRaw).description;
+  // #3468: single channel for every preserve-when-unchanged row. Before this
+  // change, seven body-source pre/post pairs travelled in two different
+  // shapes — this map for four fields, six dedicated parameters
+  // (preBodyStatus/postBodyStatus, preBodyStoppedAt/postBodyStoppedAt,
+  // preBodyPhaseSource/postBodyPhaseSource) for the other three — same data,
+  // same purpose, which is exactly why applyStatePreservation needed a
+  // hand-written branch per field instead of one loop over the table. Every
+  // row FIELD_CLASSIFICATION declares preserve-when-unchanged MUST appear
+  // here — an omission now throws (STATE_PRESERVATION_UNWIRED_ROW, ADR-3408
+  // §8.2) at the first write rather than becoming a quiet preservation bug.
+  // Note current_phase_name's source is the body `Phase:` line, deliberately
+  // a DIFFERENT source from current_phase's: the key names the field the
+  // policy GUARDS, not the body field it reads.
   const bodyDeltas = {
     last_activity_desc: { pre: preBodyLastActivityDesc, post: postBodyLastActivityDesc },
     paused_at: { pre: preBodyPausedAt, post: postBodyPausedAt },
     current_phase: { pre: preBodyCurrentPhase, post: postBodyCurrentPhase },
     current_plan: { pre: preBodyCurrentPlan, post: postBodyCurrentPlan },
+    status: { pre: preBodyStatus, post: postBodyStatus },
+    stopped_at: { pre: preBodyStoppedAt, post: postBodyStoppedAt },
+    current_phase_name: { pre: preBodyPhaseSource, post: postBodyPhaseSource },
   };
 
   // ADR-1769 #1796 (Path A — finish the consolidation): the post-sync
@@ -2902,9 +2918,6 @@ function applyPostSyncPreservation(
     preFm, postFm, preFmSnapshot, resync,
     deriveProgressKeys: deriveProgressKeys === true,
     bodyDeltas,
-    preBodyStatus, postBodyStatus,
-    preBodyStoppedAt, postBodyStoppedAt,
-    preBodyPhaseSource, postBodyPhaseSource,
   });
   // #2736: re-assert the intent-first values AFTER preservation. On STATE.md
   // layouts with no body `Phase:` line, both phase-source snapshots are null