refactor(#1796): ADR-1769 Path A — finish STATE.md preservation consolidation (#1799)

Extract readModifyWriteStateMd's post-sync preservation block into a pure,
field-classification-table-driven applyStatePreservation in the STATE.md
Transition Module. progress / status / stopped_at now join current_phase_name
as table-governed (getFieldClassification), so a preservation-policy change is
a one-row table edit instead of a per-call-site patch.

This realizes the consolidation ADR-1769 / CONTEXT.md already claimed shipped
('Absorbs readModifyWriteStateMd post-sync preservation block') and routes the
#1264 preservation policy through the single field-classification table — the
bug class is now structurally guarded by the table, not just the call-site
shouldResync flag.

Behavior is byte-identical to the pre-amendment inline block (Hyrum-safe — the
15 readModifyWriteStateMd callers' observable preservation is unchanged):
- state/frontmatter/transition + bug regression suite: 847 pass
- phase/milestone/verify (other RMW consumers): 582 pass
- codex (gpt-5.5/high) adversarial review: CLEAN (58,564-case equiv sweep)

ADR-1769 amendment appended documenting #1796.

Closes #1796
This commit is contained in:
Tom Boucher
2026-06-27 22:49:52 -04:00
committed by GitHub
parent a194f3f9a7
commit 21f0b4316f
5 changed files with 322 additions and 68 deletions

View File

@@ -117,6 +117,114 @@ export function getFieldClassification(field: string): FieldClassification | nul
return FIELD_CLASSIFICATION[field];
}
// ----------------------------------------------------------------------------
// applyStatePreservation — table-driven post-sync preservation (ADR-1769 #1796)
// ----------------------------------------------------------------------------
//
// Absorbs the post-sync preservation block that previously lived inline in
// `readModifyWriteStateMd` (state.cts). One pure, field-classification-table-
// driven implementation replaces the three drifting encodings (the RMW post-sync
// block, `syncStateFrontmatter`, and `cmdStateBuildFrontmatter`'s read-path
// copy). Every preserved field — progress, status, stopped_at, current_phase_name
// — is governed by its FIELD_CLASSIFICATION row, so a policy change is a one-row
// table edit rather than a per-call-site patch. Behavior is byte-identical to
// the pre-#1796 inline block; this is the consolidation ADR-1769 / CONTEXT.md
// already claimed shipped. See issue #1796 (Path A: finish the consolidation).
export type StatePreservationInput = {
/** Pre-transform frontmatter; `null` when the transition re-derives from disk (resync=true). */
preFm: Record<string, unknown> | null;
/** Post-`syncStateFrontmatter` frontmatter (the freshly recomputed one). */
postFm: Record<string, unknown>;
/** Always-present pre-transform frontmatter snapshot (drives the #1230 deltas). */
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;
};
export type StatePreservationResult = {
postFm: Record<string, unknown>;
mutated: boolean;
};
/**
* 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.
*/
export function applyStatePreservation(input: StatePreservationInput): StatePreservationResult {
const { preFm, postFm, preFmSnapshot, resync } = input;
let mutated = false;
// 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']
) {
postFm['progress'] = preFm['progress'];
mutated = true;
}
// status — #1230 body-delta heuristic. Table: preserve-when-unchanged.
const statusCls = getFieldClassification('status');
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']
) {
postFm['status'] = preFmSnapshot['status'];
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;
}
// 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;
}
return { postFm, mutated };
}
// ----------------------------------------------------------------------------
// Body section constants (ADR-1769 §6 — single writer after migration)
// ----------------------------------------------------------------------------

View File

@@ -32,7 +32,7 @@ const { extractFrontmatter, reconstructFrontmatter } = frontmatter;
import scanPhasePlans = require('./plan-scan.cjs');
// eslint-disable-next-line @typescript-eslint/no-require-imports
import stateTransitionMod = require('./state-transition.cjs');
const { transitionCore, getFieldClassification } = stateTransitionMod;
const { transitionCore, applyStatePreservation } = stateTransitionMod;
type StateTransitionIntent = stateTransitionMod.StateTransitionIntent;
type StateTransitionDeps = stateTransitionMod.StateTransitionDeps;
import {
@@ -1913,14 +1913,9 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
let synced = syncStateFrontmatter(modified, cwd);
// Compute postFm once and apply BOTH the progress-restore (when !resync)
// AND the status/stopped_at preservation (#1230) before reconstructing.
// This avoids double-wrapping the frontmatter block.
const needsProgressRestore = !resync && preFm && preFm['progress'];
// Post-transform body source fields used for the delta comparison (#1230).
// Use `modified` (not `synced`): syncStateFrontmatter only rewrites the frontmatter block, so the body is identical in both — and we need the body the transform produced.
// Strip frontmatter so the YAML status key cannot shadow the body field.
// Strip frontmatter so the YAML status key cannot shadow the body field we are tracking.
const postBody = stripFrontmatter(modified);
const postBodyStatus = stateExtractField(postBody, 'Status');
// Bug #1230 / Change B: scope stopped_at delta to the ## Session section,
@@ -1932,68 +1927,22 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
// current_phase_name delta comparison.
const postBodyPhaseSource = stateExtractField(postBody, 'Phase');
let mutated = false;
// ADR-1769 #1796 (Path A — finish the consolidation): the post-sync
// preservation block is now the pure, table-driven `applyStatePreservation`
// in the STATE.md Transition Module. progress / status / stopped_at /
// current_phase_name are all governed by their FIELD_CLASSIFICATION row —
// one policy source, not three drifting encodings. Behavior-identical to
// the pre-#1796 inline block; this is the absorption ADR-1769 / CONTEXT.md
// already claimed shipped.
const postFm = extractFrontmatter(synced) as Record<string, unknown>;
if (needsProgressRestore) {
// Re-apply the curated progress block that syncStateFrontmatter just
// overwrote with disk-derived values. Only restore keys that were present
// in the snapshot — this preserves any new non-progress frontmatter fields
// (e.g., status, current_phase) that syncStateFrontmatter legitimately
// derived from the updated body.
postFm['progress'] = preFm['progress'];
mutated = true;
}
// Bug #1230: preserve existing frontmatter status when this write did NOT
// change the body's Status field. A write that doesn't touch Status must
// not silently revert a hand-set frontmatter status (e.g. 'completed') to
// whatever the stale body Status happens to derive (e.g. 'verifying').
// Only apply when the existing frontmatter held a real, non-unknown status.
if (
postBodyStatus === preBodyStatus &&
typeof preFmSnapshot['status'] === 'string' &&
preFmSnapshot['status'].length > 0 &&
preFmSnapshot['status'] !== 'unknown' &&
postFm['status'] !== preFmSnapshot['status']
) {
postFm['status'] = preFmSnapshot['status'];
mutated = true;
}
// Bug #1230: same delta heuristic for stopped_at.
if (
postBodyStoppedAt === 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;
}
// ADR-1769 Phase 6 / #1743 / #1695: same delta heuristic for the curated
// current_phase_name. Gated by the field-classification table (preserve-always).
// When this write did NOT change the body `Phase:` source line, the curated
// frontmatter current_phase_name wins over syncStateFrontmatter's body
// re-derivation (parseProsePhaseField can harvest a wrong parenthetical
// aside — #1695). begin/planned/complete-phase rewrite their body Phase line,
// so the delta does not fire for them and current_phase_name still advances.
const phaseNameCls = getFieldClassification('current_phase_name');
if (
phaseNameCls !== null &&
phaseNameCls.preservation === 'preserve-always' &&
postBodyPhaseSource === 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;
}
if (mutated) {
const yamlStr = reconstructFrontmatter(postFm as unknown as Frontmatter);
const preservation = applyStatePreservation({
preFm, postFm, preFmSnapshot, resync,
preBodyStatus, postBodyStatus,
preBodyStoppedAt, postBodyStoppedAt,
preBodyPhaseSource, postBodyPhaseSource,
});
if (preservation.mutated) {
const yamlStr = reconstructFrontmatter(preservation.postFm as unknown as Frontmatter);
const body = stripFrontmatter(synced);
synced = `---\n${yamlStr}\n---\n\n${body}`;
}