golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#3374): phase.complete stops harvesting stale body stopped_at (#3491)

Browse Source 
* fix(#3374): phase.complete stops harvesting stale body stopped_at

Variant A: cmdPhaseComplete's adapter calls syncStateFrontmatter directly
(deliberately - STATE.md commits atomically with ROADMAP/REQUIREMENTS),
which also bypassed the #948/#1230 preservation pass every RMW write gets.
A stale body 'Stopped at:' line then silently clobbered a fresher
frontmatter stopped_at on every phase completion, with warnings: [].

Three layers close it without reversing #3517's refresh expectation:
- completePhaseCore now refreshes the body continuity line it implies
  ('Phase N complete, ready to plan Phase N+1'; ADR-2207 phrasing on the
  last phase), session-scoped via the new stateReplaceFieldInSession seam
  so a decoy bold Stopped-at line in an unrelated section cannot absorb
  the refresh. Replace-only - a layout with no session line keeps its
  shape and its frontmatter value survives via the preservation delta.
- the RMW post-sync preservation chunk (snapshots + table-driven
  applyStatePreservation + #2736 re-assert, full bodyDeltas wired) is
  extracted into the shared applyPostSyncPreservation helper; the
  phase.complete adapter and writeStateMd (milestone complete / state
  sync - the gap the closed PR #3442 review flagged) now run it too.
- cmdStateRecordSession pushed 'Stopped At' onto updated[] on any label
  MATCH, including a value already on disk - reporting a write that never
  changed a byte. It now reports only on real change, and the match is
  tracked separately so an identical value does not arm the #944 DWIM
  section rewrite (which would reset an executor-authored resume file to
  None).

* docs(#3374): backfill changeset pr field to 3491

* fix(#3374): drop the writeStateMd preservation pass - state sync's #905 contract is body-wins

CI on this PR caught what the closed PR #3442 review's MAJOR remediation
option (a) would have broken: state sync's #905 contract ('body annotation
beats existing frontmatter when both are present') is the opposite by
design - sync exists to re-derive frontmatter from the body. A blanket
applyStatePreservation pass on writeStateMd re-locked stale frontmatter
(current_phase 3 over the body's 5) on every sync.

Take the review's sanctioned option (b) instead: the scope claim is
accurate (phase.complete only) and the milestone complete / state sync
exposure is tracked as follow-up issue #3492.

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-14 12:38:23 -04:00
committed by GitHub
parent 27cad971d0
commit be9329b10b
7 changed files with 551 additions and 134 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/plucky-wolves-leap.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 3491
---
phase.complete no longer rewrites STATE.md frontmatter stopped_at with a stale body 'Stopped at:' line: the completion now refreshes the session continuity line it implies ('Phase N complete, ready to plan Phase N+1') and applies the standard field-preservation policy on its atomic commit path. state record-session no longer reports 'Stopped At' as updated when the value is already current.

30
src/phase.cts
View File

@@ -91,6 +91,7 @@ const {
stateExtractField,
stateReplaceField,
syncStateFrontmatter,
applyPostSyncPreservation,
withStateLock,
updatePerformanceMetricsSection,
} = stateMod;
@@ -2964,7 +2965,9 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
// this adapter: they are section-table / disk-scan concerns, not
// classified fields, and `syncStateFrontmatter` is the post-sync this
// transaction needs (it does NOT go through readModifyWriteStateMd
// because STATE.md is committed atomically with ROADMAP/REQUIREMENTS).
// because STATE.md is committed atomically with ROADMAP/REQUIREMENTS —
// the post-sync preservation pass runs via applyPostSyncPreservation
// instead, #3374).
const nextPhaseDisplayName =
phaseDisplayNameFromRoadmap(roadmapContent, nextPhaseNum) ??
phaseDisplayNameFromSlug(nextPhaseName);
@@ -3018,7 +3021,30 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
current_phase_name: nextPhaseDisplayName,
}
: undefined;
stateContent = syncStateFrontmatter(stateContent, cwd, authoritativeFm);
const synced = syncStateFrontmatter(stateContent, cwd, authoritativeFm);
// #3374: the direct sync above deliberately bypasses
// readModifyWriteStateMd (STATE.md is committed atomically with
// ROADMAP/REQUIREMENTS), which also bypassed the #948/#1230
// preservation pass every RMW write gets — so a stale body
// `Stopped at:` line silently clobbered a fresher frontmatter
// stopped_at on every completion. Run the shared post-sync pass:
// snapshots from the on-disk pre-image (originalStateContent) and the
// transformed content, table-driven applyStatePreservation, then the
// #2736 authoritative re-assert (which restores the #3350 pairing
// override the preserve-always restore may have reverted). resync=true
// is the lifecycle-transition posture (progress recomputed from disk;
// only the preserve-when-unchanged deltas apply). Fields the
// transition legitimately rewrote (Status, Phase, Stopped At via
// completePhaseCore's #3374 continuity line) have changed body
// sources, so their deltas do not fire.
stateContent = applyPostSyncPreservation(
originalStateContent,
stateContent,
synced,
statePath,
true,
authoritativeFm,
);
writes.push({ filePath: statePath, before: originalStateContent, after: stateContent });
}

30
src/state-document.cts
View File

@@ -9,7 +9,7 @@
import { splitTableRow } from './markdown-table.cjs';
import { clampPercentFromFraction } from './phase-lifecycle.cjs';
import { collectSection } from './markdown-sectionizer.cjs';
import { collectSection, withSection } from './markdown-sectionizer.cjs';
import type { HeadingToken } from './markdown-sectionizer.cjs';
import { escapeRegex } from './pattern.cjs';
// eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-scope.cjs is an export= CommonJS module
@@ -373,6 +373,34 @@ export function stateReplaceFieldWithFallback(content: string, primary: string,
return content;
}
/**
* #3374: session-scoped variant of stateReplaceFieldWithFallback for the
* `## Session` continuity fields. The post-sync harvest (state.cts's
* matchSessionSection → buildStateFrontmatter) reads these fields ONLY from
* the session section, so a writer that refreshes one must target the same
* scope — a whole-body replace lets a decoy `**Stopped at:**` line in an
* unrelated (e.g. archive) section absorb the refresh while the harvested
* session value stays stale.
*
* Section preference mirrors the reader exactly: the normalized `## Session`
* block wins over the bootstrap `## Session Continuity` heading when both
* exist (legacy duplicate files); the continuity heading is only consulted
* when no canonical `## Session` section exists. `levelBounded` heading
* matching also excludes `## Session Continuity Archive` (the #2444 scoping).
*
* Replace-only (no insertion): returns `content` unchanged when no session
* section exists or the field is absent from it, so a STATE.md layout without
* the line keeps its shape and the post-sync preservation pass decides the
* frontmatter value (see #3374).
*/
export function stateReplaceFieldInSession(content: string, primary: string, fallback: string | null | undefined, value: string): string {
const isSession = (h: HeadingToken): boolean => h.level === 2 && h.text.trim().toLowerCase() === 'session';
const isSessionContinuity = (h: HeadingToken): boolean => h.level === 2 && h.text.trim().toLowerCase() === 'session continuity';
const hasCanonicalSession = collectSection(content, isSession, { levelBounded: true }) !== null;
const target = hasCanonicalSession ? isSession : isSessionContinuity;
return withSection(content, target, (sectionBody) => stateReplaceFieldWithFallback(sectionBody, primary, fallback, value));
}
export function normalizeStateStatus(status: string | null | undefined, pausedAt: unknown): string {
let normalizedStatus = status || 'unknown';
const statusLower = (status || '').toLowerCase();

26
src/state-transition.cts
View File

@@ -16,7 +16,7 @@
// eslint-disable-next-line @typescript-eslint/no-require-imports
import frontmatter = require('./frontmatter.cjs');
import { stateReplaceField, stateExtractField, stateReplaceFieldIfTemplate, stateReplaceFieldWithFallback } from './state-document.cjs';
import { stateReplaceField, stateExtractField, stateReplaceFieldIfTemplate, stateReplaceFieldWithFallback, stateReplaceFieldInSession } from './state-document.cjs';
import { KNOWN_TEMPLATE_DEFAULTS } from './state-document.cjs';
import { tokenizeHeadings } from './markdown-sectionizer.cjs';
import type { HeadingToken } from './markdown-sectionizer.cjs';
@@ -1057,6 +1057,7 @@ function completePhaseCore(
'current_plan',
'last_activity',
'last_activity_desc',
'stopped_at',
'progress',
]) {
const cls = getFieldClassification(fmKey);
@@ -1157,6 +1158,29 @@ function completePhaseCore(
updated.push('Last Activity Description');
}
// Stopped At — #3374: write the continuity line this transition implies.
// The frontmatter `stopped_at` is a projection of this body line
// (source: 'body' in FIELD_CLASSIFICATION), and phase completion is exactly
// the event the line describes — leaving it stale made the post-sync harvest
// overwrite a fresher frontmatter value with pre-completion prose on every
// completion (#3374), and left the workflow's later prose refresh as a
// divergence source. Session-SCOPED replace (stateReplaceFieldInSession):
// the harvest reads only the session section, so the write must target the
// same scope — a whole-body replace let a decoy `**Stopped at:**` line in an
// unrelated section absorb the refresh. Replace-only (no insertion): a
// STATE.md with no session continuity line keeps its shape, and the
// unchanged body source then lets the preservation delta keep an existing
// frontmatter value. Last-phase wording reuses the ADR-2207 status phrase;
// milestone termination wording stays owned by milestoneCompleteCore.
const stoppedAtLine = intent.isLastPhase
? `Phase ${intent.phaseNum} complete — all phases complete`
: `Phase ${intent.phaseNum} complete${intent.nextPhaseNum ? `, ready to plan Phase ${intent.nextPhaseNum}` : ''}`;
const stoppedAfter = stateReplaceFieldInSession(body, 'Stopped At', 'Stopped at', stoppedAtLine);
if (stoppedAfter !== body) {
body = stoppedAfter;
updated.push('Stopped At');
}
// Progress block — re-derive completed/total phases from the roadmap when
// available (milestone-wide source of truth), then recompute the percent.
// Only runs when a Completed Phases field exists (the existing guard).

337
src/state.cts
View File

@@ -1254,10 +1254,22 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
if (result) { content = result; updated.push('Last Date'); }
// Update Stopped at
// #3374 Variant B: stateReplaceField returns the replaced string on any
// label MATCH, including when the value is already the target. Pushing
// 'Stopped At' on match alone reported a write that never changed a byte
// (and that the #948 no-op guard may then discard entirely), leaving a
// stale frontmatter stopped_at undetectable to the caller. Report only on
// real change — and track the match separately so an identical value does
// not read as "label missing" to the #944 DWIM insertion below (whose
// section rewrite would reset an executor-authored resume file to None).
let stoppedAtMatched = false;
if (options.stopped_at) {
result = stateReplaceField(content, 'Stopped At', options.stopped_at);
if (!result) result = stateReplaceField(content, 'Stopped at', options.stopped_at);
if (result) { content = result; updated.push('Stopped At'); }
if (result) {
stoppedAtMatched = true;
if (result !== content) { content = result; updated.push('Stopped At'); }
}
}
// Update Resume File — only when the caller explicitly passed a value OR the
@@ -1308,7 +1320,10 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
// missing canonical fields are inserted while the heading and any prose are
// preserved (#1101). Only append a brand-new section when NEITHER heading exists.
const callerSuppliedValues = !!(options.stopped_at || (options.resume_file !== undefined && options.resume_file !== null));
const needsStoppedAt = options.stopped_at && !updated.includes('Stopped At');
// #3374: keyed on the label MATCH, not on updated[] — a matched-but-
// identical value is already persisted on disk and must not trigger the
// insertion rewrite below.
const needsStoppedAt = options.stopped_at && !stoppedAtMatched;
const needsResumeFile = options.resume_file !== undefined && options.resume_file !== null && !updated.includes('Resume File');
const needsLastSession = !updated.includes('Last session') && !updated.includes('Last Date');
@@ -2408,13 +2423,20 @@ function syncStateFrontmatter(content: string, cwd: string | undefined, authorit
// survive every writeStateMd call.
//
// For stopped_at / paused_at: the original #905 "fall back when derived is
// absent" rule is preserved here. The stale-body-overwrites-frontmatter
// scenario from #948 is prevented by the no-op guard in
// readModifyWriteStateMd: when the transform produces no change the file is
// never written, so syncStateFrontmatter never even runs. Attempting to
// "always prefer frontmatter" here breaks legitimate callers like phase.complete
// that intentionally write a new stopped_at value to the body and expect
// syncStateFrontmatter to pick it up.
// absent" rule is preserved here — this block handles the EMPTY case only.
// The disagreeing case (a present-but-stale body value vs a fresher
// frontmatter value, #948/#3374) is NOT handled here: it is governed by
// applyStatePreservation's preserve-when-unchanged delta, applied post-sync
// by the shared applyPostSyncPreservation pass — run by
// readModifyWriteStateMd and by cmdPhaseComplete's adapter (the one caller
// that deliberately bypasses the RMW wrapper for the atomic
// ROADMAP/REQUIREMENTS/STATE commit; #3374). The writeStateMd path
// (state sync) intentionally derives from the body instead — its #905
// contract is body-beats-frontmatter. "Always prefer frontmatter" here
// would still be wrong: it would break transforms that legitimately write a
// new body value and expect this sync to project it — the #1230 delta
// ("did THIS write change the body source?") is what distinguishes those
// from a stale harvest.
if (!derivedFm['stopped_at'] && existingFm['stopped_at']) {
derivedFm['stopped_at'] = existingFm['stopped_at'];
}
@@ -2748,6 +2770,165 @@ function writeStateMd(statePath: string, content: string, cwd?: string, clock?:
}
}
/**
* #3374: the shared post-sync preservation pass — the pre/post body-source
* snapshot + table-driven `applyStatePreservation` + #2736 authoritative
* re-assert sequence. Extracted from readModifyWriteStateMd so
* `cmdPhaseComplete`'s atomic-commit adapter (phase.cts) — which syncs
* STATE.md directly because it is committed atomically with
* ROADMAP/REQUIREMENTS and so cannot go through the RMW wrapper — applies the
* identical policy instead of a second, weaker encoding. Previously the
* adapter had no preservation at all, letting a stale body `Stopped at:` line
* silently clobber a fresher frontmatter `stopped_at` on every phase
* completion (#3374 Variant A).
*
* NOT applied on the writeStateMd path: `state sync`'s contract is the
* opposite by design (#905 — "body annotation beats existing frontmatter when
* both are present": sync exists to re-derive frontmatter from the body), so a
* blanket preservation pass there re-locks stale frontmatter. The
* milestone-complete equivalent of the #3374 exposure is tracked as a
* follow-up (see PR #3491 / the closed PR #3442 review's MAJOR finding).
*
* `originalContent` is the pre-write on-disk content (drives the #1230
* pre-snapshots), `transformedContent` is the post-transform content (the
* sync only rewrites the frontmatter block, so its body IS the post-write
* body), and `syncedContent` is what `syncStateFrontmatter` produced.
*/
function applyPostSyncPreservation(
originalContent: string,
transformedContent: string,
syncedContent: string,
statePath: string,
resync: boolean,
authoritativeFm?: Record<string, unknown>,
deriveProgressKeys?: boolean,
): string {
// Snapshot the existing progress block BEFORE the transform so we can
// restore it when resync is false.
const preFm = resync ? null : extractFrontmatter(originalContent, statePath) as Record<string, unknown>;
// Bug #1230: delta heuristic — snapshot pre-transform body source fields so
// we can detect whether THIS write changed them. syncStateFrontmatter
// re-derives frontmatter status/stopped_at from the body on every write;
// when the body's source field was NOT changed by the transform, the
// existing frontmatter value (e.g. a hand-set 'completed') must win over
// the body-derived value (e.g. 'verifying' from a stale "Status: Verifying
// Phase 3" line that an earlier tool wrote). We do NOT disturb `preFm`
// above (null when resync:true) — these are independent snapshots.
// Strip frontmatter before calling stateExtractField so the YAML `status:`
// key in the frontmatter block cannot shadow the body field we are tracking.
const preBody = stripFrontmatter(originalContent);
const preFmSnapshot = extractFrontmatter(originalContent, statePath) as Record<string, unknown>;
const preBodyStatus = stateExtractField(preBody, 'Status');
// Bug #1230 / Change B: scope stopped_at delta to the ## Session section,
// mirroring buildStateFrontmatter's sessionBodyScope logic.
// A stale "Stopped at:" in a non-Session section (e.g. Session Continuity
// Archive prose) must not interfere with the delta comparison.
const preSessionMatch = matchSessionSection(preBody);
const preSessionScope = preSessionMatch ?? preBody;
const preBodyStoppedAt = stateExtractField(preSessionScope, 'Stopped At') || stateExtractField(preSessionScope, 'Stopped at');
// ADR-1769 Phase 6 / #1743 / #1695: snapshot the body source for the curated
// current_phase_name (the `Phase:` line parseProsePhaseField harvests). When
// this write does NOT change that line, the curated frontmatter value must
// win over syncStateFrontmatter's body re-derivation (which can harvest a
// wrong parenthetical aside — #1695). Gated by the field-classification
// table's preserve-always row so the rule lives in one place.
const preBodyPhaseSource = stateExtractField(preBody, 'Phase');
// #3258: snapshot the body sources for the additional preserve-when-unchanged
// rows applyStatePreservation now honors (last_activity_desc, paused_at,
// current_phase, current_plan). Each mirrors buildStateFrontmatter's
// derivation so the #1230 delta ("did THIS write change the source?") is
// accurate: current_phase combines `Current Phase` with the prose `Phase:`
// fallback (parseProsePhaseField, scoped to ## Current Position); paused_at
// is session-scoped (mirrors stopped_at); last_activity_desc combines the
// `Last Activity Description` field with the prose desc fallback.
const preCurrentPositionScope = matchCurrentPositionSection(preBody) ?? preBody;
const preBodyCurrentPlan = stateExtractField(preBody, 'Current Plan');
const preBodyCurrentPhase = stateExtractField(preBody, 'Current Phase')
?? parseProsePhaseField(stateExtractField(preCurrentPositionScope, 'Phase')).phase;
const preBodyPausedAt = stateExtractField(preSessionScope, 'Paused At');
const preBodyLastActivityRaw = stateExtractField(preBody, 'Last Activity')
?? stateExtractField(preBody, 'Last activity');
const preBodyLastActivityDesc = stateExtractField(preBody, 'Last Activity Description')
?? parseProseLastActivityField(preBodyLastActivityRaw).description;
// Post-transform body source fields used for the delta comparison (#1230).
// Use `transformedContent` (not `syncedContent`): 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 we are tracking.
const postBody = stripFrontmatter(transformedContent);
const postBodyStatus = stateExtractField(postBody, 'Status');
// Bug #1230 / Change B: scope stopped_at delta to the ## Session section,
// consistent with the pre-transform snapshot above and buildStateFrontmatter.
const postSessionMatch = matchSessionSection(postBody);
const postSessionScope = postSessionMatch ?? postBody;
const postBodyStoppedAt = stateExtractField(postSessionScope, 'Stopped At') || stateExtractField(postSessionScope, 'Stopped at');
// ADR-1769 Phase 6 / #1695: post-transform body Phase source for the
// current_phase_name delta comparison.
const postBodyPhaseSource = stateExtractField(postBody, 'Phase');
// #3258: post-transform body sources for the preserve-when-unchanged rows
// added in #3258 (mirrors the pre-transform block above).
const postCurrentPositionScope = matchCurrentPositionSection(postBody) ?? postBody;
const postBodyCurrentPlan = stateExtractField(postBody, 'Current Plan');
const postBodyCurrentPhase = stateExtractField(postBody, 'Current Phase')
?? parseProsePhaseField(stateExtractField(postCurrentPositionScope, 'Phase')).phase;
const postBodyPausedAt = stateExtractField(postSessionScope, 'Paused At');
const postBodyLastActivityRaw = stateExtractField(postBody, 'Last Activity')
?? stateExtractField(postBody, 'Last activity');
const postBodyLastActivityDesc = stateExtractField(postBody, 'Last Activity Description')
?? parseProseLastActivityField(postBodyLastActivityRaw).description;
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 },
};
// 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. #3258 extends the same
// pass to last_activity_desc / paused_at / current_phase / current_plan
// (preserve-when-unchanged) and milestone / milestone_name (preserve-if-
// placeholder). Behavior-identical to the pre-#1796 inline block for the
// original four fields; this is the absorption ADR-1769 / CONTEXT.md
// already claimed shipped.
const postFm = extractFrontmatter(syncedContent, statePath) as Record<string, unknown>;
const preservation = applyStatePreservation({
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
// (equal), so the #1695 restore fires and would put the stale pre-transition
// name back over the authoritative one. Intent beats both the prose
// re-derivation and the curated restore — the transition just resolved it.
let authoritativeReasserted = false;
if (authoritativeFm) {
for (const [key, value] of Object.entries(authoritativeFm)) {
if (typeof value === 'string' && value.trim().length > 0 && preservation.postFm[key] !== value) {
preservation.postFm[key] = value;
authoritativeReasserted = true;
}
}
}
if (preservation.mutated || authoritativeReasserted) {
const yamlStr = reconstructFrontmatter(preservation.postFm as unknown as Frontmatter);
const body = stripFrontmatter(syncedContent);
return `---\n${yamlStr}\n---\n\n${body}`;
}
return syncedContent;
}
/**
* Atomic read-modify-write for STATE.md.
* Holds the lock across the entire read -> transform -> write cycle,
@@ -2773,56 +2954,6 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
const lockPath = acquireStateLock(statePath, clock);
try {
const content = platformReadSync(statePath) || '';
// Snapshot the existing progress block BEFORE the transform so we can
// restore it when resync is false.
const preFm = resync ? null : extractFrontmatter(content, statePath) as Record<string, unknown>;
// Bug #1230: delta heuristic — snapshot pre-transform body source fields so
// we can detect whether THIS write changed them. syncStateFrontmatter
// re-derives frontmatter status/stopped_at from the body on every write;
// when the body's source field was NOT changed by the transform, the
// existing frontmatter value (e.g. a hand-set 'completed') must win over
// the body-derived value (e.g. 'verifying' from a stale "Status: Verifying
// Phase 3" line that an earlier tool wrote). We do NOT disturb `preFm`
// above (null when resync:true) — these are independent snapshots.
// Strip frontmatter before calling stateExtractField so the YAML `status:`
// key in the frontmatter block cannot shadow the body field we are tracking.
const preBody = stripFrontmatter(content);
const preFmSnapshot = extractFrontmatter(content, statePath) as Record<string, unknown>;
const preBodyStatus = stateExtractField(preBody, 'Status');
// Bug #1230 / Change B: scope stopped_at delta to the ## Session section,
// mirroring buildStateFrontmatter's sessionBodyScope logic (line ~1172).
// A stale "Stopped at:" in a non-Session section (e.g. Session Continuity
// Archive prose) must not interfere with the delta comparison.
const preSessionMatch = matchSessionSection(preBody);
const preSessionScope = preSessionMatch ?? preBody;
const preBodyStoppedAt = stateExtractField(preSessionScope, 'Stopped At') || stateExtractField(preSessionScope, 'Stopped at');
// ADR-1769 Phase 6 / #1743 / #1695: snapshot the body source for the curated
// current_phase_name (the `Phase:` line parseProsePhaseField harvests). When
// this write does NOT change that line, the curated frontmatter value must
// win over syncStateFrontmatter's body re-derivation (which can harvest a
// wrong parenthetical aside — #1695). Gated by the field-classification
// table's preserve-always row so the rule lives in one place.
const preBodyPhaseSource = stateExtractField(preBody, 'Phase');
// #3258: snapshot the body sources for the additional preserve-when-unchanged
// rows applyStatePreservation now honors (last_activity_desc, paused_at,
// current_phase, current_plan). Each mirrors buildStateFrontmatter's
// derivation so the #1230 delta ("did THIS write change the source?") is
// accurate: current_phase combines `Current Phase` with the prose `Phase:`
// fallback (parseProsePhaseField, scoped to ## Current Position); paused_at
// is session-scoped (mirrors stopped_at); last_activity_desc combines the
// `Last Activity Description` field with the prose desc fallback.
const preCurrentPositionScope = matchCurrentPositionSection(preBody) ?? preBody;
const preBodyCurrentPlan = stateExtractField(preBody, 'Current Plan');
const preBodyCurrentPhase = stateExtractField(preBody, 'Current Phase')
?? parseProsePhaseField(stateExtractField(preCurrentPositionScope, 'Phase')).phase;
const preBodyPausedAt = stateExtractField(preSessionScope, 'Paused At');
const preBodyLastActivityRaw = stateExtractField(preBody, 'Last Activity')
?? stateExtractField(preBody, 'Last activity');
const preBodyLastActivityDesc = stateExtractField(preBody, 'Last Activity Description')
?? parseProseLastActivityField(preBodyLastActivityRaw).description;
const modified = transformFn(content);
@@ -2838,77 +2969,17 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
}
let synced = syncStateFrontmatter(modified, cwd, options?.authoritativeFm);
// 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 we are tracking.
const postBody = stripFrontmatter(modified);
const postBodyStatus = stateExtractField(postBody, 'Status');
// Bug #1230 / Change B: scope stopped_at delta to the ## Session section,
// consistent with the pre-transform snapshot above and buildStateFrontmatter.
const postSessionMatch = matchSessionSection(postBody);
const postSessionScope = postSessionMatch ?? postBody;
const postBodyStoppedAt = stateExtractField(postSessionScope, 'Stopped At') || stateExtractField(postSessionScope, 'Stopped at');
// ADR-1769 Phase 6 / #1695: post-transform body Phase source for the
// current_phase_name delta comparison.
const postBodyPhaseSource = stateExtractField(postBody, 'Phase');
// #3258: post-transform body sources for the preserve-when-unchanged rows
// added in #3258 (mirrors the pre-transform block above).
const postCurrentPositionScope = matchCurrentPositionSection(postBody) ?? postBody;
const postBodyCurrentPlan = stateExtractField(postBody, 'Current Plan');
const postBodyCurrentPhase = stateExtractField(postBody, 'Current Phase')
?? parseProsePhaseField(stateExtractField(postCurrentPositionScope, 'Phase')).phase;
const postBodyPausedAt = stateExtractField(postSessionScope, 'Paused At');
const postBodyLastActivityRaw = stateExtractField(postBody, 'Last Activity')
?? stateExtractField(postBody, 'Last activity');
const postBodyLastActivityDesc = stateExtractField(postBody, 'Last Activity Description')
?? parseProseLastActivityField(postBodyLastActivityRaw).description;
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 },
};
// 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. #3258 extends the same
// pass to last_activity_desc / paused_at / current_phase / current_plan
// (preserve-when-unchanged) and milestone / milestone_name (preserve-if-
// placeholder). Behavior-identical to the pre-#1796 inline block for the
// original four fields; this is the absorption ADR-1769 / CONTEXT.md
// already claimed shipped.
const postFm = extractFrontmatter(synced, statePath) as Record<string, unknown>;
const preservation = applyStatePreservation({
preFm, postFm, preFmSnapshot, resync,
deriveProgressKeys: options?.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
// (equal), so the #1695 restore fires and would put the stale pre-transition
// name back over the authoritative one. Intent beats both the prose
// re-derivation and the curated restore — the transition just resolved it.
let authoritativeReasserted = false;
if (options?.authoritativeFm) {
for (const [key, value] of Object.entries(options.authoritativeFm)) {
if (typeof value === 'string' && value.trim().length > 0 && preservation.postFm[key] !== value) {
preservation.postFm[key] = value;
authoritativeReasserted = true;
}
}
}
if (preservation.mutated || authoritativeReasserted) {
const yamlStr = reconstructFrontmatter(preservation.postFm as unknown as Frontmatter);
const body = stripFrontmatter(synced);
synced = `---\n${yamlStr}\n---\n\n${body}`;
}
// #3374: the post-sync preservation pass (snapshots, table-driven
// applyStatePreservation, #2736 re-assert) — see applyPostSyncPreservation.
synced = applyPostSyncPreservation(
content,
modified,
synced,
statePath,
resync,
options?.authoritativeFm,
options?.deriveProgressKeys === true,
);
platformWriteSync(statePath, synced);
return true;
@@ -4267,6 +4338,12 @@ export = {
writeStateMd,
readModifyWriteStateMd,
syncStateFrontmatter,
// #3374: the shared post-sync preservation pass (snapshots + table-driven
// applyStatePreservation + #2736 re-assert). Exported for cmdPhaseComplete's
// atomic-commit adapter in phase.cts, which syncs STATE.md directly (it is
// committed atomically with ROADMAP/REQUIREMENTS) and must apply the same
// preservation policy the RMW path applies.
applyPostSyncPreservation,
readStateHeadFreshness,
withStateLock,
updatePerformanceMetricsSection,

177
tests/frontmatter.test.cjs
View File

@@ -2218,6 +2218,183 @@ test('extractFrontmatter handles large frontmatter blocks without body bleed', (
}
// ────────────────────────────────────────────────────────────────────────
// #3374 — `phase complete`'s adapter calls syncStateFrontmatter directly
// (deliberately bypassing readModifyWriteStateMd for the atomic
// ROADMAP/REQUIREMENTS/STATE commit), which also bypassed the #948/#1230
// preservation pass every RMW write gets. A stale body `Stopped at:` line then
// silently clobbered a fresher frontmatter `stopped_at` on every phase
// completion. Placed beside the #2736 suite (the same defect family: the
// adapter's post-sync policy diverging from the RMW path's). The fix is
// two-layered: completePhaseCore now refreshes the body continuity line it
// implies (`Phase N complete, ready to plan Phase N+1`) — session-scoped, so a
// decoy `**Stopped at:**` line in an unrelated section cannot absorb the
// refresh — so the harvest projects a value this very completion produced
// (keeping #3517's refresh expectation), and the adapter runs the RMW post-sync
// preservation pass (applyPostSyncPreservation) so a body source this write did
// not refresh cannot beat a fresher frontmatter value.
// ────────────────────────────────────────────────────────────────────────
{
const { describe: __d3374, test: __t3374, beforeEach: __be3374, afterEach: __ae3374 } = require('node:test');
const __assert3374 = require('node:assert/strict');
const __fs3374 = require('node:fs');
const __path3374 = require('node:path');
const { runGsdTools: __run3374, createTempProject: __mk3374, cleanup: __rm3374 } = require('./helpers.cjs');
const { extractFrontmatter: __extractFm3374 } = require('../gsd-core/bin/lib/frontmatter.cjs');
const { stateExtractField: __extractField3374 } = require('../gsd-core/bin/lib/state-document.cjs');
const FRESH_3374 = 'Phase 2 gap closure executed — FRESH frontmatter value';
const STALE_3374 = 'Phase 1 complete, ready to plan Phase 2';
const COMPLETION_LINE_3374 = 'Phase 2 complete, ready to plan Phase 3';
// Mirrors the issue's repro: a 3-phase roadmap completing phase 2 (not-last),
// body `## Session Continuity` holding a stale plain-label `Stopped at:` line
// that phase.complete's transition previously never touched.
function writeCompleteFixture3374(tmpDir, { fmStoppedAt = null, sessionStoppedAt = STALE_3374, decoy = false } = {}) {
const planningDir = __path3374.join(tmpDir, '.planning');
const phase2Dir = __path3374.join(planningDir, 'phases', '02-second-phase');
__fs3374.mkdirSync(phase2Dir, { recursive: true });
__fs3374.writeFileSync(
__path3374.join(planningDir, 'ROADMAP.md'),
[
'# Roadmap',
'',
'### Phase 1: First phase',
'**Plans:** 1 plans',
'',
'### Phase 2: Second phase',
'**Plans:** 1 plans',
'',
'### Phase 3: Third phase',
'**Plans:** 1 plans',
'',
'## Progress',
'',
'- [x] **Phase 1: First phase** - done',
'- [ ] **Phase 2: Second phase** - pending',
'- [ ] **Phase 3: Third phase** - pending',
'',
].join('\n'),
);
const sessionLines = [
'Last session: 2026-08-10',
...(sessionStoppedAt === null ? [] : [`Stopped at: ${sessionStoppedAt}`]),
'Resume file: None',
];
__fs3374.writeFileSync(
__path3374.join(planningDir, 'STATE.md'),
[
'---',
"gsd_state_version: '1.0'",
'milestone: v1.0',
'current_phase: 2',
'current_phase_name: Second phase',
'status: executing',
...(fmStoppedAt ? [`stopped_at: "${fmStoppedAt}"`] : []),
'---',
'',
'# Project State',
...(decoy ? ['', '## Archive notes', '', '**Stopped at:** old prose from June'] : []),
'',
'## Session Continuity',
'',
...sessionLines,
'',
].join('\n'),
);
__fs3374.writeFileSync(__path3374.join(phase2Dir, '02-01-PLAN.md'), '# Plan\n');
__fs3374.writeFileSync(__path3374.join(phase2Dir, '02-01-SUMMARY.md'), '# Summary\n');
__fs3374.writeFileSync(
__path3374.join(phase2Dir, '02-VERIFICATION.md'),
['---', 'status: passed', '---', '', '# Verification', ''].join('\n'),
);
}
__d3374('#3374: phase complete must not harvest a stale body Stopped at over fresher frontmatter', () => {
let tmpDir;
const statePath = () => __path3374.join(tmpDir, '.planning', 'STATE.md');
__be3374(() => { tmpDir = __mk3374(); });
__ae3374(() => { __rm3374(tmpDir); });
__t3374('AC1: the stale body Stopped at never reaches the frontmatter — the transition refreshes the line it implies', () => {
writeCompleteFixture3374(tmpDir, { fmStoppedAt: FRESH_3374 });
const result = __run3374(['phase', 'complete', '2'], tmpDir);
__assert3374.ok(result.success, `phase complete failed: ${result.error}`);
const stateContent = __fs3374.readFileSync(statePath(), 'utf-8');
const fm = __extractFm3374(stateContent);
__assert3374.notStrictEqual(
fm.stopped_at,
STALE_3374,
'phase.complete harvested the stale pre-completion body value into the frontmatter (#3374 Variant A)',
);
__assert3374.strictEqual(
fm.stopped_at,
COMPLETION_LINE_3374,
`the frontmatter must project the continuity line this completion wrote, never the stale value; got ${JSON.stringify(fm.stopped_at)}`,
);
__assert3374.strictEqual(
__extractField3374(stateContent, 'Stopped at'),
COMPLETION_LINE_3374,
'the body continuity line must be refreshed by the transition itself, not left for a later prose step',
);
});
__t3374('AC1 scoping: a decoy Stopped at in a non-session section cannot absorb the continuity refresh', () => {
writeCompleteFixture3374(tmpDir, { fmStoppedAt: FRESH_3374, decoy: true });
const result = __run3374(['phase', 'complete', '2'], tmpDir);
__assert3374.ok(result.success, `phase complete failed: ${result.error}`);
// The harvest reads ONLY the session scope, so the projected frontmatter
// value proves the session line (not the decoy) was the one refreshed.
const fm = __extractFm3374(__fs3374.readFileSync(statePath(), 'utf-8'));
__assert3374.strictEqual(
fm.stopped_at,
COMPLETION_LINE_3374,
`the session-scoped continuity write must win over the whole-body decoy; got ${JSON.stringify(fm.stopped_at)}`,
);
});
__t3374('AC1 preservation leg: with no session Stopped at line to refresh, the fresher frontmatter value survives', () => {
writeCompleteFixture3374(tmpDir, { fmStoppedAt: FRESH_3374, sessionStoppedAt: null });
const result = __run3374(['phase', 'complete', '2'], tmpDir);
__assert3374.ok(result.success, `phase complete failed: ${result.error}`);
// Replace-only continuity write missed → nothing to harvest → the
// pre-existing (fresher) frontmatter value must be preserved, not
// dropped or replaced with pre-completion prose.
const fm = __extractFm3374(__fs3374.readFileSync(statePath(), 'utf-8'));
__assert3374.strictEqual(
fm.stopped_at,
FRESH_3374,
`with no body source refreshed by this write, the existing frontmatter value must survive; got ${JSON.stringify(fm.stopped_at)}`,
);
});
__t3374('AC2 (no-regress): with no pre-existing frontmatter stopped_at, the body line populates it', () => {
writeCompleteFixture3374(tmpDir, {});
const result = __run3374(['phase', 'complete', '2'], tmpDir);
__assert3374.ok(result.success, `phase complete failed: ${result.error}`);
const fm = __extractFm3374(__fs3374.readFileSync(statePath(), 'utf-8'));
__assert3374.strictEqual(
fm.stopped_at,
COMPLETION_LINE_3374,
`first-write population from the (refreshed) body line must keep working; got ${JSON.stringify(fm.stopped_at)}`,
);
});
});
}
// ────────────────────────────────────────────────────────────────────────
// Folded from tests/fix-2847-gap-closure-frontmatter.test.cjs — test-hygiene sweep #3335 (H3 Wave 3)
// ────────────────────────────────────────────────────────────────────────

80
tests/state.test.cjs
View File

@@ -2207,6 +2207,86 @@ describe('cmdStateRecordSession (state record-session)', () => {
assert.strictEqual(output.recorded, false, 'recorded should be false when no session fields found');
assert.ok(output.reason !== undefined, 'should have a reason');
});
// #3374 Variant B: stateReplaceField returns the replaced string on any label
// MATCH, including when the value is already the target — so `updated` used
// to report 'Stopped At' for a write that never changed a byte (and that the
// #948 no-op guard then discarded entirely), leaving a stale frontmatter
// stopped_at undetectable to the caller. The report must reflect real change,
// and a matched-but-identical value must NOT arm the #944 DWIM insertion
// branch (which wholesale-rewrites the session section and would reset an
// executor-authored resume file to 'None').
test('#3374: --stopped-at with the value already in the body is not reported updated and writes nothing', () => {
const PINNED_MS = Date.parse('2020-09-01T09:00:00.000Z');
const PINNED_ISO = '2020-09-01T09:00:00.000Z';
const executorResume = '.planning/phases/02/02-01-PLAN.md';
const fixture = [
'# Project State',
'',
'## Session Continuity',
'',
`**Last session:** ${PINNED_ISO}`,
'**Stopped at:** Phase 2, Plan 1',
`**Resume file:** ${executorResume}`,
].join('\n') + '\n';
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
fs.writeFileSync(statePath, fixture);
const result = runGsdTools('state record-session --stopped-at "Phase 2, Plan 1"', tmpDir, {
GSD_TEST_MODE: '1',
GSD_NOW_MS: String(PINNED_MS),
});
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.ok(
!((output.updated || []).includes('Stopped At')),
`updated must not report a Stopped At write that changed nothing; got ${JSON.stringify(output.updated)} (#3374 Variant B)`,
);
// The pinned clock makes the Last-session replacement an identity too, so
// the whole transform is a no-op and the #948 no-op guard must skip the
// write entirely — the file must be byte-identical.
const after = fs.readFileSync(statePath, 'utf-8');
assert.strictEqual(
after,
fixture,
'no field changed, so no write may occur (#3374 Variant B)',
);
assert.strictEqual(
stateDocument.stateExtractField(after, 'Resume file'),
executorResume,
'an identical --stopped-at must not arm the #944 DWIM section rewrite (executor-authored resume file reset to None)',
);
});
test('#3374: --stopped-at with a new value still reports Stopped At and syncs the frontmatter', () => {
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), sessionFixture);
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
const newValue = 'Phase 3 complete, ready to plan Phase 4';
const result = runGsdTools(['state', 'record-session', '--stopped-at', newValue], tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.ok(
(output.updated || []).includes('Stopped At'),
`a real change must keep reporting Stopped At; got ${JSON.stringify(output.updated)}`,
);
const after = fs.readFileSync(statePath, 'utf-8');
assert.strictEqual(
stateDocument.stateExtractField(after, 'Stopped at'),
newValue,
'the body Stopped at line should carry the new value',
);
const fm = frontmatterLib.extractFrontmatter(after);
assert.strictEqual(
fm.stopped_at,
newValue,
`the RMW sync must reflect the new value in frontmatter; got ${JSON.stringify(fm.stopped_at)}`,
);
});
});
// ─────────────────────────────────────────────────────────────────────────────