refactor(#3469): one composition for the STATE.md write seam (#3501)

* docs(#3469): amend ADR-3408 section 8.3 — the pipeline has sanctioned exceptions

Section 8.3 read 'Every STATE.md write applies the pipeline.' That is false by
design for two commands, and acting on it would have inverted a shipped
feature.

Preservation makes curated frontmatter win over a re-derived body value.
state sync exists to do the opposite — #905's 'body annotation beats existing
frontmatter when both are present'; it re-derives frontmatter FROM the body.
REGENERATE_STATE is a factory reset that rebuilds STATE.md from scratch.
Applying the pipeline to either would re-lock exactly what the command was
invoked to replace.

This issue's own scope line, inherited from the epic, said to route the direct
writeStateMd callers through the pipeline. For cmdStateSync that would have
shipped silently, with every gate green, because no test asserts that sync
LETS the body win. Caught by reading the helper's docstring and then verifying
the claim against the code — a stale comment had already misdirected this epic
once.

Both commands are now named in a closed exception list and are permanent
ratchet entries.

Consequence recorded rather than left to bite Phase 4: the 'drive the ratchet
to 0 and delete the file' target in this ADR and in #3471 is wrong. Two
entries are permanent, so the correct end state is 2, and the honest report is
'0 removable bypasses, 2 sanctioned'. A guard reaching 0 here would only do so
by having stopped looking at two real writers.

* refactor(#3469): one composition for the write seam, not one per caller

Implements ADR-3408 section 8.3 as amended.

syncAndPreserveStateMd is now the single composition of syncStateFrontmatter
and applyPostSyncPreservation. readModifyWriteStateMd and cmdPhaseComplete
both CALL it instead of each assembling the two steps themselves.
cmdPhaseComplete keeps its own writePlanningFileSet envelope — the
composition returns content, it does not take over the write, so STATE.md
still commits atomically with ROADMAP and REQUIREMENTS.

Assembling the stages at a call site is a re-derivation even when every step
calls an owner. Upstream's fix(#3374) routed cmdPhaseComplete through
applyPostSyncPreservation but left it calling syncStateFrontmatter directly
first, so the composition was duplicated and free to diverge with both guards
green. That is ADR-3180 Amendment 2's finding repeating on the write side.

cmdMilestoneComplete gains preservation. It wrote through writeStateMd, so it
got sync and no preservation — the identical shape #3374 reported for
phase.complete, and flagged upstream as a follow-up in the helper's own
docstring. This is that follow-up.

Divergence is now visible: preservation_warnings names each field restored
over a disagreeing derived value. Deliberately NOT named warnings —
cmdPhaseComplete already exposes warnings as a prose string array, and two
sibling commands carrying that name with different element types is
Generative Fix Divergence, the class this epic exists to remove.

patchCore stops running stateReplaceField over the whole document. One
observable consequence, intended per design row 9: a frontmatter-shaped patch
key with no body counterpart now reports failed instead of silently
succeeding, because the old whole-document match was literally hitting the
YAML line case-insensitively.

The guard closes Phase 1's DECLARED KNOWN GAP as promised rather than
re-deferring it: section 8.3(b) detection is tractable now the composition
exists. Scoped by two factors to avoid Phase 1's measured 29-to-1 false
positive rate — a variable field-name argument AND a content argument whose
nearest preceding assignment is not stripFrontmatter. Verified 0 findings and
0 false positives across all 33 call sites, plus 5 synthetic shapes. It also
detects the re-assembly shape above.

Ratchet: 4 entries to 2, both sanctioned-permanent. cmdStateSync's owner
changes from #3471 to sanctioned-permanent per Amendment 2 — routing it
through preservation would invert the #905 contract.

Also fixed inline rather than deferred: cmdMilestoneComplete's STATE.md read
now happens inside withStateLock. It previously read outside any lock before
writeStateMd took its own, leaving a TOCTOU window under concurrent writers.

* test(#3469): characterization coverage for the single write seam

Matrix sections A-E. Criterion 6 was amended by maintainer decision — all five
instances closed by point fixes while Phase 1 was in flight — so these are
characterization tests at the consumer's output per ADR-3180 Decision 4(b)/(c),
paired with the drift guard's count, never either alone.

Section C is the one that earns its keep. cmdStateSync is a sanctioned
permanent exception: state sync exists to re-derive frontmatter FROM the body,
so preservation there re-locks exactly what the command was invoked to
replace. C1 pins that the body wins; C4 pins that this phase left the command
byte-identical. Nothing else in the suite would notice if a future change made
sync start preserving, and the natural reading of 'one write seam' is to make
precisely that change.

Section E pins the guard's false-positive scoping. E4 (updateCore's
strip-then-replace) and E5 (sectionBody-scoped calls) must NOT be reported —
the naive detector measured 29 false positives to 1 true positive in Phase 1.
E7 is the inverse: a sanctioned-permanent entry disappearing must FAIL,
because a guard reaching zero here would only do so by having stopped looking
at two real writers.

Also corrects a stale test that asserted patchCore's old whole-document
behavior, which this phase deliberately changes.

One honest limitation, flagged rather than papered over: A1's 'byte-identical
to pre-refactor' cannot be diffed against real pre-refactor bytes from inside
the suite. It is implemented as the seeded fast-check property that
cmdPhaseComplete's composed output equals readModifyWriteStateMd's for the
same inputs — the strongest available proxy, not the literal claim.

* docs(#3469): refresh the seam glossary entry and add the changeset

Two spec-review gaps, both real.

CONTEXT.md's STATE.md Transition Module entry named three direct writeStateMd
callers including cmdMilestoneComplete. This phase routed that one through the
composition, so the line was false the moment the refactor landed.

Worth recording plainly: I wrote that sentence in Phase 0, correcting an
older stale pointer in it, and my own Phase 2 change invalidated it again
within the same epic. That is the exact drift this epic exists to remove,
demonstrated on the epic's own documentation — and it is why the entry now
ends by saying the whole-repo drift guard, not this line, is the authoritative
count.

The entry now records the composition (syncAndPreserveStateMd) and states that
exactly two direct callers remain, both SANCTIONED PERMANENT rather than debt.

Changeset: type Changed, because milestone complete's observable output moves.
Tier-2 per ADR-3180 Decision 3 — a stale body line no longer wins over fresher
frontmatter, and the command gains preservation_warnings. Docs requirement is
met by the ADR amendment already in this diff.

* test(#3469): register property-test temp-dir cleanup at creation time

Standards review, minor but real: the new fast-check property cleaned up its
temp dirs in a loop AFTER fc.assert returned. A genuine property failure
throws, so that line never ran and every dir from the failing run — including
all of fast-check's shrinking iterations — leaked.

The failure path is exactly when a littered machine hurts most, and a failing
property test is the case the test exists for.

Cleanup is now registered with t.after() at dir-creation time, so teardown
happens however the test exits. Not try/finally — CONTRIBUTING.md:356 bans it
inside test bodies, which is why the after-the-assertion shape existed in the
first place.

Swept the rest of the branch's test diff for the same shape; phase.test.cjs
already uses registered teardown and nothing else matched.

* fix(#3469): patchCore routes frontmatter writes instead of dropping them

Checkpoint returned 10 failures of 33880. One implementation defect, three
test defects, one stale test — all fixed, and the implementation defect is the
one that matters.

patchCore stripped frontmatter and then reconstructed it VERBATIM, applying no
patches to it. An arbitrary custom frontmatter key with no body counterpart and
no FIELD_CLASSIFICATION row — risk_level in the upstream fix(#3351) test —
therefore always reported failed and silently never wrote. It worked before,
via the old whole-document match on the raw YAML line.

That is a regression against this phase's own design row 9, which requires
frontmatter changes to ROUTE THROUGH the seam — still work, policy-governed —
not to stop working. Removing a capability is not routing it. An upstream test
caught it, which is the argument for running the checkpoint before believing
the refactor.

patchCore now partitions by frontmatter shape, decided structurally from the
parsed frontmatter's own keys rather than a naming heuristic:
  - classified keys still report failed — policy owns them and a raw patch may
    not bypass it;
  - unclassified keys apply to the frontmatter object and report updated —
    Phase 1's behavior-table row 19, a field with no row is not this contract's
    business;
  - body-shaped keys are unchanged.

The property 'failure' was my own test breaking the repo's Clock Seams rule.
The two paths agree byte-for-byte; the only difference was last_updated,
stamped from the wall clock on two invocations milliseconds apart, so it could
never pass. Time is now frozen with mock.timers across both — not by excluding
last_updated from the comparison, which would have silently stopped comparing
a field the composition writes.

B4's fixture could not discriminate: normalizeStateStatus maps any text
containing 'complete' to 'completed', and milestone complete's own new body
value derives to exactly that — which was also the fixture's stale value. The
stale value is now 'executing' so the assertion can tell 'body correctly won'
from 'stale survived'.

B5's fixture tripped a pre-existing unstarted-phase guard before reaching any
write-seam code; it now has the matching phase directory.

D9 asserted the old exempt set. readModifyWriteStateMd now calls one symbol
rather than assembling two, so it needs no exemption; syncAndPreserveStateMd
is the sole legitimate composition site.

* fix(#3469): patchCore resolves body-first, so the body wins a name collision

Re-verification returned 2 failures of 33880, both D4 — the hostile row for a
key that exists as BOTH a frontmatter key and a body field.

The partition checked frontmatter first, so 'status' — classified in
FIELD_CLASSIFICATION and also present as a body 'Status:' line — routed to the
frontmatter branch, was rejected as classified, and reported failed.

Wrong order. Patching 'status' means the body field, and upstream fix(#3351)
says so in its own comment: 'the legitimate working case for state.patch is
display-cased BODY fields — Status, Current Plan, Phase.' The body is
authoritative in this model; frontmatter is the projection. D4 asserted
exactly that and was right.

Resolution order is now body, then frontmatter:
  1. resolves to a body field -> apply to body, updated
  2. else an own key of the frontmatter:
       classified   -> failed  (policy owns it)
       unclassified -> apply to frontmatter, updated
  3. else -> failed

Verified by probe against the compiled lib for all four cases rather than
asserted: risk_level (frontmatter-only, unclassified) still lands;
current_phase still fails; display-cased Status unchanged; D4's lower-cased
status now lands via the body with the frontmatter untouched.

The current_phase case was the one that could have regressed silently, so its
fixture was read rather than assumed — D1's body carries 'Phase: 3 (alpha)'
and no 'Current Phase:' line, so body-first cannot reach it.

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

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-08-14 16:04:09 -04:00
committed by GitHub
parent 71180983a0
commit e2f4c16d9e
15 changed files with 1443 additions and 152 deletions

View File

@@ -0,0 +1,5 @@
---
type: Changed
pr: 3501
---
**milestone complete no longer lets a stale STATE.md body line overwrite fresher frontmatter** — it wrote through a path that re-derived frontmatter from the body with no preservation pass, so a stale Stopped-at line silently replaced a newer curated value, exactly as phase complete did before it was fixed. It now runs the same preservation the rest of the write path uses, and reports each field it protected in a new preservation_warnings array instead of staying silent about the divergence. (#3469)

View File

@@ -59,7 +59,7 @@ Module owning projection from dispatch results/errors to CLI `{ exitCode, stdout
Module owning STATE.md parse, field extraction, field replacement, status normalization, frontmatter reconstruction, and `## Current Position` section scoping (`stateCurrentPositionSlice`, #1956 — the one owner of that scope for the read path; `state.cts`'s `matchCurrentPositionSection` is a thin alias over it, and the `drift-guard phase-status` seam consumes it, so the #2956 archive-shadowing fix cannot be re-derived into a second copy — the byte-exact mutation path served by `state-transition.cts`'s `locateCurrentPosition`/`sliceCurrentPositionSection` is a deliberately separate, un-consolidated locator, #3187). `stateFieldValue` (#3187) is the single owner of the #1760 frontmatter-then-body field fallback chain, consolidating the 14 re-derivations of that ladder onto one scope-carrying (`complete`/`truncated`/`unscoped`/`unreadable`) primitive. It does not scan `.planning/phases` and does not own persistence or locking; phase/plan/summary counts arrive from inventory/progress Modules as inputs, and read-modify-write paths remain Adapters. Source of truth: `gsd-core/bin/lib/state-document.cjs`. **Commit provenance (#2573):** `state_head` records the full sha STATE.md was written against, stamped by `syncStateFrontmatter` and omitted entirely outside a git repo. `readStateHeadFreshness(cwd, stateHead)` (`src/state.cts`) is the single derivation consumed by both `validate.health` (W024) and smart-entry — it returns `{ state_head, current_commit, commits_behind, commit_stale }` with **tri-state** `commit_stale`: `null` = unknown (no stamp, no git, or a stamp that is not an ancestor of HEAD after a history rewrite), `false` = known fresh, `true` = the codebase has moved. Mirrors the graphify commit-staleness contract deliberately. It is a freshness PROXY, never a drift measurement: `rev-list` counts unrelated commits and the stamp restamps on every state write, so a low count means STATE.md was written recently, not that its contents are accurate — it must never gate. Module owning STATE.md parse, field extraction, field replacement, status normalization, frontmatter reconstruction, and `## Current Position` section scoping (`stateCurrentPositionSlice`, #1956 — the one owner of that scope for the read path; `state.cts`'s `matchCurrentPositionSection` is a thin alias over it, and the `drift-guard phase-status` seam consumes it, so the #2956 archive-shadowing fix cannot be re-derived into a second copy — the byte-exact mutation path served by `state-transition.cts`'s `locateCurrentPosition`/`sliceCurrentPositionSection` is a deliberately separate, un-consolidated locator, #3187). `stateFieldValue` (#3187) is the single owner of the #1760 frontmatter-then-body field fallback chain, consolidating the 14 re-derivations of that ladder onto one scope-carrying (`complete`/`truncated`/`unscoped`/`unreadable`) primitive. It does not scan `.planning/phases` and does not own persistence or locking; phase/plan/summary counts arrive from inventory/progress Modules as inputs, and read-modify-write paths remain Adapters. Source of truth: `gsd-core/bin/lib/state-document.cjs`. **Commit provenance (#2573):** `state_head` records the full sha STATE.md was written against, stamped by `syncStateFrontmatter` and omitted entirely outside a git repo. `readStateHeadFreshness(cwd, stateHead)` (`src/state.cts`) is the single derivation consumed by both `validate.health` (W024) and smart-entry — it returns `{ state_head, current_commit, commits_behind, commit_stale }` with **tri-state** `commit_stale`: `null` = unknown (no stamp, no git, or a stamp that is not an ancestor of HEAD after a history rewrite), `false` = known fresh, `true` = the codebase has moved. Mirrors the graphify commit-staleness contract deliberately. It is a freshness PROXY, never a drift measurement: `rev-list` counts unrelated commits and the stamp restamps on every state write, so a low count means STATE.md was written recently, not that its contents are accurate — it must never gate.
### STATE.md Transition Module ### STATE.md Transition Module
Module owning STATE.md lifecycle/maintenance transitions as intent-based methods (`beginPhase`, `advancePlan`, `completePhase`, `plannedPhase`, `milestoneSwitch`, `milestoneComplete`, `patch`, `sync`, `prune`, `update`, `rebuild`). Pure core `(content, intent, deps) → newContent` with injected I/O (file read/write, lock, disk scan); consults a field-classification table that names each STATE.md field's class (`derived-from-body` | `derived-from-disk` | `derived-from-external` | `curated` | `free`) and its preservation policy. Supersedes the 14 scattered RMW callbacks in `state.cts` (phase.cts's former direct caller has since been migrated away). **Three direct `writeStateMd` callers remain as of `next` @ `5452f1a70`: `cmdStateSync` (`state.cts:3682`), `cmdMilestoneComplete` (`milestone.cts:865`), and the `REGENERATE_STATE` remedy (`health-diagnostic.cts:337`)** — the last is the factory-reset primitive, which rebuilds STATE.md from scratch and therefore deliberately wants no preservation. ADR-3408 §8.3 governs all three; Phase 1's whole-repo drift guard, not this line, is the authoritative count. *(Location correction: this entry previously placed the factory-reset primitive at `verify.cts:1925`. It moved to `health-diagnostic.cts` when `cmdValidateHealth` migrated onto the rule table (#3309); `verify.cts` now contains no `writeStateMd` call. The design intent was unchanged — only the address was stale.)* Absorbs `syncStateFrontmatter` + `readModifyWriteStateMd`'s post-sync preservation block; Encoding 3 (`cmdStateBuildFrontmatter`) stays separate — read path concern. Sibling/super-module of the STATE.md Document Module; consumes its `stateReplaceField`/`stateExtractField` primitives. Body section structure (`## Current Position`, `## Session`, etc.) lives as a constants block inside the Module. Append-only transitions (`addDecision`, `addBlocker`, etc.) stay on today's RMW seam for now. Targets the #1760/#1761/#1743/#1695/#1264/#1255/#1257/#3242 bug cluster. Migration per ADR-1372 §T6 sequenced as substrate + `beginPhase` first (PR1), then transition-by-transition with characterization tests first per transition. **ADR-1817 adds `rebuild` as the capstone 11th transition — the body-structure derivability contract.** Re-derives `## Current Position` prose from frontmatter and `## By-Phase Progress` table from phase dirs on disk; preserves `## Session` / `## Decisions` / unknown sections verbatim; de-duplicates `## Session Continuity Archive` (keep most-recent N, default 3); appends a structured audit entry to `## Rebuild Log` (`timestamp`, `kind`, `section`, `before`, `after`, `reason`) for every mutation. Hard idempotency guarantee: a no-mutation rebuild appends no log entry, so two successive invocations on a clean file are byte-identical. Non-overlapping with `sync` (3 lightweight frontmatter fields, auto-triggered) and orthogonal to `auto_prune_state` (age-based removal) — `rebuild` reconciles with current canonical sources, `prune` removes by retention policy, the two compose (rebuild first, then prune). Section ordering is invariant: rebuild rewrites content in place, never reorders. Targets the #1776/#1761/#1591 body-drift cluster that survived ADR-1769's per-field transitions. Phased per ADR-1817: Phase 0 = this ADR + predicates (closes #1817), Phase 1 = `rebuildCore` body + `rebuild` dispatch case + drift-class unit tests (#1827), Phase 2 = `cmdStateRebuild` CLI + `--dry-run`/`--verbose` + integration tests + docs + changeset (#1826). Source of truth: `gsd-core/bin/lib/state-transition.cjs` (generated from `src/state-transition.cts`). `state_head` (#2573) is classified `{ source: 'free', preservation: 'derive' }` — an ambient git read recomputed on every write, like `last_updated`; never preserved, because a stale stamp would claim STATE.md was written against a commit it wasn't. Module owning STATE.md lifecycle/maintenance transitions as intent-based methods (`beginPhase`, `advancePlan`, `completePhase`, `plannedPhase`, `milestoneSwitch`, `milestoneComplete`, `patch`, `sync`, `prune`, `update`, `rebuild`). Pure core `(content, intent, deps) → newContent` with injected I/O (file read/write, lock, disk scan); consults a field-classification table that names each STATE.md field's class (`derived-from-body` | `derived-from-disk` | `derived-from-external` | `curated` | `free`) and its preservation policy. Supersedes the 14 scattered RMW callbacks in `state.cts` (phase.cts's former direct caller has since been migrated away). `syncAndPreserveStateMd` (`state.cts`, #3469) is the single composition of `syncStateFrontmatter` + `applyPostSyncPreservation`; `readModifyWriteStateMd` and `cmdPhaseComplete` both **call** it rather than assembling the two steps, because assembling them at a call site is a re-derivation even when every step calls an owner. **Exactly two direct `writeStateMd` callers remain, and both are SANCTIONED PERMANENT exceptions under ADR-3408 §8.3 (as amended): `cmdStateSync` and the `REGENERATE_STATE` remedy (`health-diagnostic.cts`).** Neither is debt — `state sync` exists to re-derive frontmatter *from* the body (#905), and `REGENERATE_STATE` is a factory reset; preservation on either would re-lock precisely what the command was invoked to replace. `cmdMilestoneComplete` was the third and is now routed through the composition (#3469). Phase 1's whole-repo drift guard, not this line, is the authoritative count. *(Location correction: this entry previously placed the factory-reset primitive at `verify.cts:1925`. It moved to `health-diagnostic.cts` when `cmdValidateHealth` migrated onto the rule table (#3309); `verify.cts` now contains no `writeStateMd` call. The design intent was unchanged — only the address was stale.)* Absorbs `syncStateFrontmatter` + `readModifyWriteStateMd`'s post-sync preservation block; Encoding 3 (`cmdStateBuildFrontmatter`) stays separate — read path concern. Sibling/super-module of the STATE.md Document Module; consumes its `stateReplaceField`/`stateExtractField` primitives. Body section structure (`## Current Position`, `## Session`, etc.) lives as a constants block inside the Module. Append-only transitions (`addDecision`, `addBlocker`, etc.) stay on today's RMW seam for now. Targets the #1760/#1761/#1743/#1695/#1264/#1255/#1257/#3242 bug cluster. Migration per ADR-1372 §T6 sequenced as substrate + `beginPhase` first (PR1), then transition-by-transition with characterization tests first per transition. **ADR-1817 adds `rebuild` as the capstone 11th transition — the body-structure derivability contract.** Re-derives `## Current Position` prose from frontmatter and `## By-Phase Progress` table from phase dirs on disk; preserves `## Session` / `## Decisions` / unknown sections verbatim; de-duplicates `## Session Continuity Archive` (keep most-recent N, default 3); appends a structured audit entry to `## Rebuild Log` (`timestamp`, `kind`, `section`, `before`, `after`, `reason`) for every mutation. Hard idempotency guarantee: a no-mutation rebuild appends no log entry, so two successive invocations on a clean file are byte-identical. Non-overlapping with `sync` (3 lightweight frontmatter fields, auto-triggered) and orthogonal to `auto_prune_state` (age-based removal) — `rebuild` reconciles with current canonical sources, `prune` removes by retention policy, the two compose (rebuild first, then prune). Section ordering is invariant: rebuild rewrites content in place, never reorders. Targets the #1776/#1761/#1591 body-drift cluster that survived ADR-1769's per-field transitions. Phased per ADR-1817: Phase 0 = this ADR + predicates (closes #1817), Phase 1 = `rebuildCore` body + `rebuild` dispatch case + drift-class unit tests (#1827), Phase 2 = `cmdStateRebuild` CLI + `--dry-run`/`--verbose` + integration tests + docs + changeset (#1826). Source of truth: `gsd-core/bin/lib/state-transition.cjs` (generated from `src/state-transition.cts`). `state_head` (#2573) is classified `{ source: 'free', preservation: 'derive' }` — an ambient git read recomputed on every write, like `last_updated`; never preserved, because a stale stamp would claim STATE.md was written against a commit it wasn't.
### STATE.md Status Lifecycle (ADR-2207) ### STATE.md Status Lifecycle (ADR-2207)
The `Status` field in STATE.md follows a strict lifecycle: `Ready to plan` → `All phases complete` (all phases done, milestone awaiting formal close) → `<version> milestone complete` (terminal, written only by the milestone-close verb `milestoneCompleteCore`) → `Awaiting next milestone` (archived). Phase-completion verbs write `All phases complete` on the last phase — never `Milestone complete` (the overloaded bare value was removed in #2204 per ADR-2207 to decouple phase-level writes from milestone termination). `normalizeStateStatus` maps any status containing "complete" → `completed`, so consumers using the normalized projection (workstream inventory's `status` field, statusline) recognize `All phases complete` without code changes. Note: `isCompletedInventory` (workstream-inventory-builder.cts) intentionally checks only for the terminal `\bmilestone\s+complete\b` / `\barchived\b` — `All phases complete` returns `false` (intermediate, not terminal). The `Status` field in STATE.md follows a strict lifecycle: `Ready to plan` → `All phases complete` (all phases done, milestone awaiting formal close) → `<version> milestone complete` (terminal, written only by the milestone-close verb `milestoneCompleteCore`) → `Awaiting next milestone` (archived). Phase-completion verbs write `All phases complete` on the last phase — never `Milestone complete` (the overloaded bare value was removed in #2204 per ADR-2207 to decouple phase-level writes from milestone termination). `normalizeStateStatus` maps any status containing "complete" → `completed`, so consumers using the normalized projection (workstream inventory's `status` field, statusline) recognize `All phases complete` without code changes. Note: `isCompletedInventory` (workstream-inventory-builder.cts) intentionally checks only for the terminal `\bmilestone\s+complete\b` / `\barchived\b` — `All phases complete` returns `false` (intermediate, not terminal).

View File

@@ -176,7 +176,18 @@ Decisions 1–7 answer *how* the write path is organized. This section says *wha
**Owner.** `src/state.cts` · the pure sync + preservation pipeline, and `readModifyWriteStateMd` as its I/O wrapper. **Owner.** `src/state.cts` · the pure sync + preservation pipeline, and `readModifyWriteStateMd` as its I/O wrapper.
**Rule.** Every STATE.md write applies the pipeline. A caller needing a different I/O envelope — `cmdPhaseComplete`'s atomic three-file commit via `writePlanningFileSet` — calls the pipeline and supplies its own envelope. It does not re-assemble the stages, and it does not skip them. Assembling the stages at a call site is a re-derivation even when every step calls the owner. **Rule.** Every STATE.md write applies the pipeline **unless the command's own contract is to let the body win** (see the exception list below). A caller needing a different I/O envelope — `cmdPhaseComplete`'s atomic three-file commit via `writePlanningFileSet` — calls the pipeline and supplies its own envelope. It does not re-assemble the stages, and it does not skip them. Assembling the stages at a call site is a re-derivation even when every step calls the owner.
**Sanctioned permanent exceptions — a closed list; adding to it is an amendment.** Preservation makes curated frontmatter win over a re-derived body value. Two commands exist precisely to do the opposite, and applying the pipeline to them would invert the feature rather than fix a bug:
| Command | Why preservation must NOT apply |
|---|---|
| `state sync` (`cmdStateSync`) | Its contract is #905's *"body annotation beats existing frontmatter when both are present"* — `sync` exists to re-derive frontmatter **from** the body. A preservation pass re-locks the stale frontmatter the command was invoked to replace. |
| `/gsd-health --repair`'s `REGENERATE_STATE` | A factory reset that rebuilds STATE.md from scratch. Preservation would restore exactly the values it was invoked to discard. |
Both are **permanent entries in the ratchet with `owner: sanctioned-permanent`**, never debt. A guard reporting them is reporting correctly; a change that removes one is a regression, not progress.
*This paragraph is Amendment 2. The rule previously read "Every STATE.md write applies the pipeline", which is false by design for both rows and would have had Phase 2 route `cmdStateSync` through preservation — inverting a shipped feature with every gate green.*
**Rule.** `current_phase` and `current_phase_name` are written as a **pair**. A transaction that computes one from a phase number writes both from that same number, so the two cannot describe different phases (#3350). **Rule.** `current_phase` and `current_phase_name` are written as a **pair**. A transaction that computes one from a phase number writes both from that same number, so the two cannot describe different phases (#3350).
@@ -304,4 +315,22 @@ The four are `cmdStateSync`, `cmdMilestoneComplete`, `cmdPhaseComplete`, and `RE
**Complexity, measured before and after.** `applyStatePreservation` was 177 lines, cyclomatic 61, cognitive 107, `risk_level: critical`. It is now a 27-line dispatch loop over four executors of 28, 44, 26 and 3 lines. This was Kernighan's Law's contribution to the design — the justification for the refactor was debuggability, not line count, and the largest remaining unit is the one holding the genuinely intricate #2440/#2969 ratchet. **Complexity, measured before and after.** `applyStatePreservation` was 177 lines, cyclomatic 61, cognitive 107, `risk_level: critical`. It is now a 27-line dispatch loop over four executors of 28, 44, 26 and 3 lines. This was Kernighan's Law's contribution to the design — the justification for the refactor was debuggability, not line count, and the largest remaining unit is the one holding the genuinely intricate #2440/#2969 ratchet.
*(Phase 3 records the §8.4 bucket decision here.)* ### Amendment 2 — Phase 2 (#3469): §8.3 was over-broad, and the ratchet's target was wrong
Decisions 1–5 held. §8.3's **rule** did not.
**"Every STATE.md write applies the pipeline" is false by design.** `state sync` and `REGENERATE_STATE` exist to let the body win; preservation exists to stop the body winning. §8.3 now carries the closed exception list above, and both are permanent `owner: sanctioned-permanent` ratchet entries.
This was not a theoretical over-reach. #3469's own scope line, inherited from the epic, said to route the direct `writeStateMd` callers through the pipeline — which for `cmdStateSync` would have inverted the command, silently, with every gate green. The correction came from reading `applyPostSyncPreservation`'s docstring and then **verifying the claim against the code**, because a stale comment had already misdirected this epic once (`CONTEXT.md` pointed at a `verify.cts:1925` call that had moved to `health-diagnostic.cts`).
**Consequence: Decision 5's and Phase 4's "ratchet to 0" target is wrong.** This ADR's guard roster and #3471 both say Phase 4 drives the baseline to empty and deletes the file. It cannot — two entries are permanent. **The correct end state is 2 permanent entries, not 0**, and the honest report is *"0 removable bypasses, 2 sanctioned"*. A guard that could reach 0 here would only do so by having stopped looking at two real writers.
**What Phase 2 actually found in the tree**, after `fix(#3374)` (#3491) landed part of §8.3 upstream mid-epic:
1. **`cmdPhaseComplete` re-assembles the pipeline.** Upstream routed it through `applyPostSyncPreservation`, but it still calls `syncStateFrontmatter` directly first. Every step calls an owner, so Axis 2 and an owner-level test both stay green while the *composition* is duplicated between the adapter and `readModifyWriteStateMd`, free to diverge. This is ADR-3180 Amendment 2's composition-level re-derivation, repeating on the write side, and §8.3 already forbade it by name.
2. **`cmdMilestoneComplete` is the remaining real exposure** — it writes through `writeStateMd`, so it gets sync and no preservation, the identical shape #3374 reported for `phase.complete`. Upstream flagged it as a follow-up in the same docstring; Phase 2 is that follow-up.
3. **Phase 1's declared known gap closes here**, as promised rather than re-deferred: §8.3(b)'s frontmatter-write detection becomes tractable once the composition exists, because the invariant simplifies to "no transition core calls `stateReplaceField` on unstripped content".
**Criterion 6 context.** All five of the epic's named instances were closed by point fixes while Phase 1 was in flight, so Phase 2 and Phase 4 are driven by **characterization tests at the consumer's output** (Decision 4(b)/(c)) rather than fail-first tests. Weaker, and stated as such.
*(Phase 3 records the §8.4 bucket decision here — though see the epic: `fix(#3351)` (#3487) appears to have subsumed most of Phase 3.)*

View File

@@ -62,35 +62,33 @@
* to prevent. `stripComments` does not track quoted strings for exactly * to prevent. `stripComments` does not track quoted strings for exactly
* this reason — see its own header. * this reason — see its own header.
* *
* DECLARED KNOWN GAP — §8.3(b) `patchCore` frontmatter-write shape is NOT * AXIS 3 — FRONTMATTER-SHAPED WRITE (§8.3(b)), CLOSED IN PHASE 2 (#3469).
* detected by this guard. `patchCore` runs `stateReplaceField(` over the * Phase 1 left this as a DECLARED KNOWN GAP: `patchCore` ran
* WHOLE document (body + frontmatter) instead of stripping frontmatter * `stateReplaceField(` over the WHOLE document (body + frontmatter) instead
* first, the way `updateCore` does — a real defect, but this guard does not * of stripping frontmatter first, the way `updateCore` does, and a naive
* catch it. * co-occurrence approximation ("does the enclosing function also call
* `stripFrontmatter(`?") measured at 33 occurrences of `stateReplaceField(`,
* of which only 4 were genuine write-seam bypasses and 29 were noise — the
* definition of `stateReplaceField` itself, ~20 calls on `sectionBody` (a
* body slice that is frontmatter-free by construction), and several calls
* inside `readModifyWriteStateMd` callbacks. 29 false positives to 1 true
* positive would have buried the signal.
* *
* Why: catching it needs genuine DATAFLOW ("is this argument a variable * Phase 2 fixes `patchCore` (it now strips frontmatter first, matching
* holding the full document, or a body slice?"), not function-scoped * `updateCore`) AND closes the gap, using a narrower, two-factor shape that
* co-occurrence. A co-occurrence approximation (does the enclosing function * does not reproduce that ratio: `findUnstrippedContentWrites` below flags a
* also call `stripFrontmatter(`?) was implemented and measured directly * `stateReplaceField(` call only when BOTH (a) its field-name argument is a
* against this repo: 33 occurrences of `stateReplaceField(`, of which only * VARIABLE, not a fixed string literal — every OTHER call site in
* 4 are genuine write-seam bypasses and 29 are noise — the definition of * `EXECUTOR_FILE` passes a fixed Title-Case literal (`'Phase'`, `'Total
* `stateReplaceField` itself (matched as a call), ~20 calls on `sectionBody` * Plans in Phase'`, ...) that can never collide with a lowercase/snake_case
* (a body slice that is frontmatter-free by construction), and several * YAML frontmatter key, so a literal field name is never a candidate
* calls inside `readModifyWriteStateMd` callbacks (correct, because the RMW * regardless of whether its content argument is stripped — and (b) its
* envelope applies preservation after the callback returns). 29 false * content argument has not been run through `stripFrontmatter` first,
* positives to 1 true positive buries the signal and makes the ratchet's * checked by a simple backward scan (within the same function) for the
* shrink-rate meaningless as a Phase 2 progress indicator — recorded here, * nearest preceding assignment to that argument's variable name. This is
* with these numbers, so the next reader does not re-attempt the same * deliberately NOT full alias/dataflow tracking — see the function's own
* approximation. * docstring for the narrow, documented limitation this trades for
* * tractability.
* Who owns closing it: Phase 2 (#3469), which also FIXES the defect by
* consolidating on the single write seam — after which detection becomes
* tractable, because once the pure pipeline exists the invariant simplifies
* to "no transition core calls `stateReplaceField` on unstripped content".
*
* This is a DECLARED gap with a named owner, not a silent omission — a
* guard that quietly does not look somewhere is the failure ADR-3180
* Decision 4(d) records.
*/ */
const fs = require('node:fs'); const fs = require('node:fs');
@@ -108,6 +106,10 @@ const BASELINE_PATH = path.join(__dirname, 'state-write-path-drift-baseline.json
const REASON = Object.freeze({ const REASON = Object.freeze({
FIELD_NAME_DISPATCH: 'field_name_dispatch', FIELD_NAME_DISPATCH: 'field_name_dispatch',
UNIMPLEMENTED_POLICY: 'unimplemented_policy', UNIMPLEMENTED_POLICY: 'unimplemented_policy',
// Axis 3 (§8.3(b), closed Phase 2 / #3469): a `stateReplaceField(` call
// with a variable field-name argument whose content argument was not run
// through `stripFrontmatter` first — see `findUnstrippedContentWrites`.
UNSTRIPPED_CONTENT_WRITE: 'unstripped_content_write',
SEAM_BYPASS_UNRECORDED: 'seam_bypass_unrecorded', SEAM_BYPASS_UNRECORDED: 'seam_bypass_unrecorded',
SEAM_BYPASS_COUNT_GREW: 'seam_bypass_count_grew', SEAM_BYPASS_COUNT_GREW: 'seam_bypass_count_grew',
SEAM_BYPASS_COUNT_SHRANK: 'seam_bypass_count_shrank', SEAM_BYPASS_COUNT_SHRANK: 'seam_bypass_count_shrank',
@@ -132,12 +134,22 @@ const EXECUTOR_FILE = 'src/state-transition.cts';
const SEAM_OWNER_FILE = 'src/state.cts'; const SEAM_OWNER_FILE = 'src/state.cts';
// Per Decision 4(d)'s "owner FILE is not exempt, only its named canonical // Per Decision 4(d)'s "owner FILE is not exempt, only its named canonical
// FUNCTIONS are": a `writeStateMd(`/`syncStateFrontmatter(` call inside one // FUNCTIONS are": a `writeStateMd(`/`syncStateFrontmatter(`/
// of these two functions, in `SEAM_OWNER_FILE` only, is the seam's own // `applyPostSyncPreservation(` call inside one of these two functions, in
// internal plumbing (the I/O wrapper calling the pure sync stage), not a // `SEAM_OWNER_FILE` only, is the seam's own internal plumbing, not a bypass.
// bypass. Every OTHER function in `state.cts` — and every function in every // `writeStateMd` is the `cmdStateSync`/`REGENERATE_STATE` path's own I/O
// OTHER file — is still scanned and still flagged. // wrapper calling `syncStateFrontmatter` directly (no preservation, by
const SEAM_OWNER_EXEMPT_FUNCTIONS = ['writeStateMd', 'readModifyWriteStateMd']; // design — §8.3's closed exception list). `syncAndPreserveStateMd` (#3469)
// is the ONE write-seam composition — `syncStateFrontmatter` then
// `applyPostSyncPreservation` — every OTHER caller needing a non-standard
// I/O envelope routes through. Every OTHER function in `state.cts` — and
// every function in every OTHER file — is still scanned and still flagged;
// in particular, `readModifyWriteStateMd` is NOT exempt: after #3469 it no
// longer contains a direct `syncStateFrontmatter(`/`applyPostSyncPreservation(`
// call at all (it calls `syncAndPreserveStateMd` like everyone else), so if
// one reappeared there it would be exactly the re-assembly shape this axis
// exists to catch.
const SEAM_OWNER_EXEMPT_FUNCTIONS = ['writeStateMd', 'syncAndPreserveStateMd'];
// Unconditional path-separator normalization (never gated on // Unconditional path-separator normalization (never gated on
// `process.platform` — a Windows-authored fork PR must be judged by the // `process.platform` — a Windows-authored fork PR must be judged by the
@@ -395,20 +407,128 @@ function findUnimplementedPolicies(text, rel) {
return out; return out;
} }
// The two write-seam functions, matched only as CALLS (`\(` immediately // AXIS 3 (§8.3(b), closed Phase 2 / #3469): `stateReplaceField(<contentArg>,
// after, modulo whitespace) — never as bare mentions of the name. // <fieldArg>, ...)` on a single line, capturing both argument expressions.
const SEAM_CALL_RE = /\b(writeStateMd|syncStateFrontmatter)\s*\(/g; // `contentArg` must be a bare identifier (a call expression or property
// A line that IS one of the two seam functions' own definitions — skipped // access as the first argument is not matched — silently out of scope, per
// outright, never counted as a call to itself. // this axis's own narrow-limitation note below) so its assignments can be
const SEAM_DEF_LINE_RE = /^\s*(?:export\s+)?(?:async\s+)?function\s+(?:writeStateMd|syncStateFrontmatter)\b/; // tracked; `fieldArg` is everything up to the next comma, trimmed, so its
// literal-vs-variable shape can be read off directly.
const STATE_REPLACE_FIELD_CALL_RE = /\bstateReplaceField\s*\(\s*([A-Za-z_$][\w$]*)\s*,\s*([^,()]+),/g;
// True when `arg` (already trimmed) is a fixed string/template literal —
// the safe shape, since every literal field name this codebase actually
// uses is a Title-Case body label that cannot collide with a lowercase/
// snake_case YAML frontmatter key.
function isQuotedLiteralArg(arg) {
const t = arg.trim();
return t.startsWith("'") || t.startsWith('"') || t.startsWith('`');
}
/** /**
* AXIS 2a: every direct `writeStateMd(`/`syncStateFrontmatter(` call in * The nearest assignment to `varName` (`varName = <expr>` or
* `text`, outside the two functions' own definitions and (only inside * `const|let|var varName = <expr>`), scanning `lines` BACKWARD from `index`
* `SEAM_OWNER_FILE`) outside `SEAM_OWNER_EXEMPT_FUNCTIONS`'s own bodies. No * (inclusive) and stopping at the nearest preceding named-function
* `reason` on these findings — `applyRatchet` assigns one, since the same * declaration (mirrors `enclosingFunction`'s own boundary, so the scan
* observed call site is a different failure shape depending on whether the * cannot walk into an unrelated function above the one containing the
* baseline already knows about it. * call). Returns the assigned expression's trimmed text, or `null` when no
* such assignment is found before the boundary — meaning `varName` is the
* enclosing function's own untouched parameter.
*
* Deliberately single-hop: this reports whatever the NEAREST assignment's
* right-hand side literally is, and does not itself follow a further alias
* (`let body = someOtherVar;` is reported as `"someOtherVar"`, not resolved
* further). Every real call site in this file assigns its body variable
* directly from `stripFrontmatter(content)` with no intermediate alias
* (`updateCore`, `patchCore`, `beginPhaseCore`'s `tryField` helper) — a
* future call site that introduces one extra hop of aliasing would evade
* this check. A declared, narrow limitation, not a silent one — mirrors
* this file's existing precedent (`FIELD_VAR_EQ_LITERAL_RE`'s own
* documented scope) of accepting a bounded risk in trade for not chasing
* full dataflow, which is exactly what made the Phase 1 approximation
* unusable (29 false positives to 1 true positive).
*/
function nearestPrecedingAssignment(lines, index, varName) {
const assignRe = new RegExp(`(?:^|[^.\\w$])(?:const|let|var)?\\s*${escapeRegex(varName)}\\s*=\\s*([^=].*)$`);
for (let i = index; i >= 0; i--) {
if (FUNCTION_DECL_LINE_RE.test(lines[i])) return null;
const m = assignRe.exec(lines[i]);
if (m) return m[1].trim();
}
return null;
}
/**
* AXIS 3: every `stateReplaceField(` call in `EXECUTOR_FILE` whose field-name
* argument is a VARIABLE (not a quoted literal) — the only shape that can
* ever rewrite YAML frontmatter, since `stateReplaceField`'s `^field:` line
* pattern is case-insensitive and matches any line starting with that name,
* literal or not — AND whose content argument was not assigned from
* `stripFrontmatter(` at the nearest preceding assignment. A literal
* field-name argument is never flagged regardless of stripping: every fixed
* string this file's `stateReplaceField` calls use is a Title-Case body
* label (`'Phase'`, `'Total Plans in Phase'`, ...) that cannot collide with
* a lowercase/snake_case frontmatter key by construction, so checking its
* content argument would only add false positives on the ~20 already-safe
* `sectionBody`-scoped calls this axis must NOT report (mirrors
* `updateCore`'s strip-then-replace shape, and `beginPhaseCore`'s
* `stateReplaceField(body, name, value)`, both legitimately unflagged).
*/
function findUnstrippedContentWrites(rel, text) {
const rawLines = text.split('\n');
const stripped = stripComments(text);
const out = [];
for (let i = 0; i < stripped.length; i++) {
const line = stripped[i];
if (!line.trim()) continue;
STATE_REPLACE_FIELD_CALL_RE.lastIndex = 0;
let m;
while ((m = STATE_REPLACE_FIELD_CALL_RE.exec(line)) !== null) {
const contentArg = m[1];
const fieldArg = m[2];
if (isQuotedLiteralArg(fieldArg)) continue;
const assignment = nearestPrecedingAssignment(stripped, i - 1, contentArg);
const isStripped = assignment !== null && /^stripFrontmatter\s*\(/.test(assignment);
if (isStripped) continue;
// `file`/`source` sanitized for the same fork-PR reason as every other
// finding in this guard; `contentArg` is captured out of repo source
// (an identifier name), attacker-controlled on the same basis.
out.push({
reason: REASON.UNSTRIPPED_CONTENT_WRITE,
axis: 'frontmatter-write',
file: sanitizeForReport(rel),
line: i + 1,
field: sanitizeForReport(contentArg),
source: sanitizeForReport(rawLines[i].trim()),
});
}
}
return out;
}
// The three write-seam functions, matched only as CALLS (`\(` immediately
// after, modulo whitespace) — never as bare mentions of the name.
// `applyPostSyncPreservation` (#3469) is included alongside
// `writeStateMd`/`syncStateFrontmatter`: after Phase 2, it is ONLY ever
// legitimately called from inside `syncAndPreserveStateMd` (the seam
// composition), so any OTHER call to it is either a re-assembly of the pair
// (Phase 2's Finding 3 shape — a call site invoking both
// `syncStateFrontmatter` and `applyPostSyncPreservation` itself instead of
// the composition) or a bypass calling it alone; either way it belongs on
// this axis.
const SEAM_CALL_RE = /\b(writeStateMd|syncStateFrontmatter|applyPostSyncPreservation)\s*\(/g;
// A line that IS one of the three seam functions' own definitions — skipped
// outright, never counted as a call to itself.
const SEAM_DEF_LINE_RE = /^\s*(?:export\s+)?(?:async\s+)?function\s+(?:writeStateMd|syncStateFrontmatter|applyPostSyncPreservation)\b/;
/**
* AXIS 2a: every direct `writeStateMd(`/`syncStateFrontmatter(`/
* `applyPostSyncPreservation(` call in `text`, outside the three functions'
* own definitions and (only inside `SEAM_OWNER_FILE`) outside
* `SEAM_OWNER_EXEMPT_FUNCTIONS`'s own bodies. No `reason` on these
* findings — `applyRatchet` assigns one, since the same observed call site
* is a different failure shape depending on whether the baseline already
* knows about it.
*/ */
function findSeamBypasses(rel, text) { function findSeamBypasses(rel, text) {
const rawLines = text.split('\n'); const rawLines = text.split('\n');
@@ -654,11 +774,13 @@ function applyRatchet(observed, baseline) {
} }
/** /**
* Run both scan passes (the `src/` tree for Axis 1 + Axis 2a, the prompt * Run both scan passes (the `src/` tree for Axis 1 + Axis 2a + Axis 3, the
* layer for Axis 2b) and split the combined findings by `axis` into * prompt layer for Axis 2b) and split the combined findings by `axis` into
* `{ policyFindings, seamFindings }`. `policyFindings` are already terminal * `{ policyFindings, seamFindings }`. `policyFindings` are already terminal
* (each carries its own `reason`); `seamFindings` are raw observations — * (each carries its own `reason`) — this bucket is every axis EXCEPT
* `applyRatchet` is what turns them into (or clears them of) a finding. * `write-seam` (Axis 2), which alone is ratcheted; `seamFindings` are raw
* write-seam observations — `applyRatchet` is what turns them into (or
* clears them of) a finding.
*/ */
function collect() { function collect() {
const srcFindings = scanTree({ const srcFindings = scanTree({
@@ -671,6 +793,7 @@ function collect() {
if (relPosix === EXECUTOR_FILE) { if (relPosix === EXECUTOR_FILE) {
found.push(...findPolicyDispatchDrift(relPosix, text)); found.push(...findPolicyDispatchDrift(relPosix, text));
found.push(...findUnimplementedPolicies(text, relPosix)); found.push(...findUnimplementedPolicies(text, relPosix));
found.push(...findUnstrippedContentWrites(relPosix, text));
} }
found.push(...findSeamBypasses(relPosix, text)); found.push(...findSeamBypasses(relPosix, text));
return found; return found;
@@ -688,7 +811,7 @@ function collect() {
const all = [...srcFindings, ...promptFindings]; const all = [...srcFindings, ...promptFindings];
return { return {
policyFindings: all.filter((f) => f.axis === 'policy-dispatch'), policyFindings: all.filter((f) => f.axis !== 'write-seam'),
seamFindings: all.filter((f) => f.axis === 'write-seam'), seamFindings: all.filter((f) => f.axis === 'write-seam'),
}; };
} }
@@ -745,18 +868,23 @@ function buildBaselineEntries(seamFindings, existingEntries) {
} }
const BASELINE_COMMENT = const BASELINE_COMMENT =
'ADR-3408 Decision 5 write-seam ratchet baseline (issue #3468, Phase 1). Every entry here is a ' + 'ADR-3408 Decision 5 write-seam ratchet baseline (issue #3468, Phase 1; Phase 2 / #3469 lands the ' +
'`writeStateMd(`/`syncStateFrontmatter(` bypass this guard found by a whole-repo scan (Decision ' + 'single write seam and Amendment 2). Every entry here is a `writeStateMd(`/`syncStateFrontmatter(`/' +
'4(a)) — it is ACKNOWLEDGED, not endorsed: acknowledgment is in writing (this file), with the ' + '`applyPostSyncPreservation(` bypass this guard found by a whole-repo scan (Decision 4(a)) — it is ' +
'issue owning its removal recorded in the entry\'s "owner" field. This baseline is SHRINK-ONLY — ' + 'ACKNOWLEDGED, not endorsed: acknowledgment is in writing (this file), with the issue owning its ' +
'an entry that stops firing goes STALE and fails the plain run until `--baseline` is re-run to ' + 'removal recorded in the entry\'s "owner" field. This baseline is SHRINK-ONLY — an entry that stops ' +
'drop it (ADR-3180 Decision 4(e)\'s "the baseline may only shrink", adopted verbatim by ADR-3408). ' + 'firing goes STALE and fails the plain run until `--baseline` is re-run to drop it (ADR-3180 ' +
'Phase 2 (#3469) removes the `cmdPhaseComplete` and `patchCore` entries when it lands the single ' + 'Decision 4(e)\'s "the baseline may only shrink", adopted verbatim by ADR-3408). Phase 2 (#3469) ' +
'write seam. Phase 4 (#3471) drives this baseline to empty and deletes this file. ' + 'removed the `cmdPhaseComplete` (`src/phase.cts`) and `cmdMilestoneComplete` (`src/milestone.cts`) ' +
'`REGENERATE_STATE` (`src/health-diagnostic.cts`) is a SANCTIONED PERMANENT exception, not debt — ' + 'entries by routing both through the single write-seam composition (`syncAndPreserveStateMd`, ' +
'it is `/gsd-health --repair`\'s factory reset, which rebuilds STATE.md from scratch, so ' + '`src/state.cts`). ADR-3408 Amendment 2: "0 bypasses" was never this baseline\'s target — TWO ' +
'preservation would restore exactly the values it was invoked to discard; do not "consolidate" ' + 'entries are SANCTIONED PERMANENT, not debt, and Phase 4 (#3471) does NOT drive this file to empty: ' +
'its entry away.'; '`cmdStateSync` (`src/state.cts`) exists precisely to let the body win (#905 — `state sync` ' +
're-derives frontmatter FROM the body), so routing it through preservation would invert the command ' +
'rather than fix a bug; `REGENERATE_STATE` (`src/health-diagnostic.cts`) is `/gsd-health --repair`\'s ' +
'factory reset, which rebuilds STATE.md from scratch, so preservation would restore exactly the ' +
'values it was invoked to discard. Neither entry may be "consolidated" away — a guard reporting them ' +
'is reporting correctly, and a change that removes one is a regression, not progress.';
function writeBaseline(seamFindings) { function writeBaseline(seamFindings) {
const priorBaseline = loadBaseline(); const priorBaseline = loadBaseline();
@@ -902,6 +1030,9 @@ module.exports = {
readPolicyUnion, readPolicyUnion,
findPolicyDispatchDrift, findPolicyDispatchDrift,
findUnimplementedPolicies, findUnimplementedPolicies,
findUnstrippedContentWrites,
isQuotedLiteralArg,
nearestPrecedingAssignment,
findSeamBypasses, findSeamBypasses,
findPromptSeamUses, findPromptSeamUses,
isInsideCodeSpan, isInsideCodeSpan,

View File

@@ -1,5 +1,5 @@
{ {
"_comment": "ADR-3408 Decision 5 write-seam ratchet baseline (issue #3468, Phase 1). Every entry here is a `writeStateMd(`/`syncStateFrontmatter(` bypass this guard found by a whole-repo scan (Decision 4(a)) — it is ACKNOWLEDGED, not endorsed: acknowledgment is in writing (this file), with the issue owning its removal recorded in the entry's \"owner\" field. This baseline is SHRINK-ONLY — an entry that stops firing goes STALE and fails the plain run until `--baseline` is re-run to drop it (ADR-3180 Decision 4(e)'s \"the baseline may only shrink\", adopted verbatim by ADR-3408). Phase 2 (#3469) removes the `cmdPhaseComplete` and `patchCore` entries when it lands the single write seam. Phase 4 (#3471) drives this baseline to empty and deletes this file. `REGENERATE_STATE` (`src/health-diagnostic.cts`) is a SANCTIONED PERMANENT exception, not debt — it is `/gsd-health --repair`'s factory reset, which rebuilds STATE.md from scratch, so preservation would restore exactly the values it was invoked to discard; do not \"consolidate\" its entry away.", "_comment": "ADR-3408 Decision 5 write-seam ratchet baseline (issue #3468, Phase 1; Phase 2 / #3469 lands the single write seam and Amendment 2). Every entry here is a `writeStateMd(`/`syncStateFrontmatter(`/`applyPostSyncPreservation(` bypass this guard found by a whole-repo scan (Decision 4(a)) — it is ACKNOWLEDGED, not endorsed: acknowledgment is in writing (this file), with the issue owning its removal recorded in the entry's \"owner\" field. This baseline is SHRINK-ONLY — an entry that stops firing goes STALE and fails the plain run until `--baseline` is re-run to drop it (ADR-3180 Decision 4(e)'s \"the baseline may only shrink\", adopted verbatim by ADR-3408). Phase 2 (#3469) removed the `cmdPhaseComplete` (`src/phase.cts`) and `cmdMilestoneComplete` (`src/milestone.cts`) entries by routing both through the single write-seam composition (`syncAndPreserveStateMd`, `src/state.cts`). ADR-3408 Amendment 2: \"0 bypasses\" was never this baseline's target — TWO entries are SANCTIONED PERMANENT, not debt, and Phase 4 (#3471) does NOT drive this file to empty: `cmdStateSync` (`src/state.cts`) exists precisely to let the body win (#905 — `state sync` re-derives frontmatter FROM the body), so routing it through preservation would invert the command rather than fix a bug; `REGENERATE_STATE` (`src/health-diagnostic.cts`) is `/gsd-health --repair`'s factory reset, which rebuilds STATE.md from scratch, so preservation would restore exactly the values it was invoked to discard. Neither entry may be \"consolidated\" away — a guard reporting them is reporting correctly, and a change that removes one is a regression, not progress.",
"entries": [ "entries": [
{ {
"file": "src/health-diagnostic.cts", "file": "src/health-diagnostic.cts",
@@ -8,26 +8,12 @@
"count": 1, "count": 1,
"owner": "sanctioned-permanent" "owner": "sanctioned-permanent"
}, },
{
"file": "src/milestone.cts",
"source": "writeStateMd(statePath, result.content, cwd);",
"symbol": "writeStateMd",
"count": 1,
"owner": "#3471"
},
{
"file": "src/phase.cts",
"source": "const synced = syncStateFrontmatter(stateContent, cwd, authoritativeFm);",
"symbol": "syncStateFrontmatter",
"count": 1,
"owner": "#3469"
},
{ {
"file": "src/state.cts", "file": "src/state.cts",
"source": "writeStateMd(statePath, modified, cwd);", "source": "writeStateMd(statePath, modified, cwd);",
"symbol": "writeStateMd", "symbol": "writeStateMd",
"count": 1, "count": 1,
"owner": "#3471" "owner": "sanctioned-permanent"
} }
] ]
} }

View File

@@ -14,7 +14,7 @@ import planningWorkspace = require('./planning-workspace.cjs');
import frontmatterMod = require('./frontmatter.cjs'); import frontmatterMod = require('./frontmatter.cjs');
// eslint-disable-next-line @typescript-eslint/no-require-imports -- state.cjs is an export= CommonJS module // eslint-disable-next-line @typescript-eslint/no-require-imports -- state.cjs is an export= CommonJS module
import stateMod = require('./state.cjs'); import stateMod = require('./state.cjs');
import { platformWriteSync, platformEnsureDir, execGit, retryRenameSync } from './shell-command-projection.cjs'; import { platformWriteSync, platformReadSync, platformEnsureDir, execGit, retryRenameSync } from './shell-command-projection.cjs';
import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs';
import { realClock } from './clock.cjs'; import { realClock } from './clock.cjs';
import { transitionCore } from './state-transition.cjs'; import { transitionCore } from './state-transition.cjs';
@@ -50,7 +50,13 @@ import phaseLocatorMod = require('./phase-locator.cjs');
const { listMilestonePhaseDirs } = phaseLocatorMod; const { listMilestonePhaseDirs } = phaseLocatorMod;
const { planningPaths } = planningWorkspace; const { planningPaths } = planningWorkspace;
const { extractFrontmatter } = frontmatterMod; const { extractFrontmatter } = frontmatterMod;
const { writeStateMd } = stateMod; // ADR-3408 §8.3 / #3469: `writeStateMd` gets sync and NO preservation — the
// same #3374-shaped exposure the milestone-complete write used to carry (a
// stale body value silently clobbering fresher frontmatter, with no
// divergence signal). Routed through the single write-seam composition
// (`syncAndPreserveStateMd`) instead, under `withStateLock` — see
// `cmdMilestoneComplete`'s own STATE.md-update block for the full rationale.
const { syncAndPreserveStateMd, withStateLock } = stateMod;
// #2288 security: a milestone version label becomes a filesystem directory // #2288 security: a milestone version label becomes a filesystem directory
// component (`milestones/<label>-phases/`) into which phase directories are // component (`milestones/<label>-phases/`) into which phase directories are
@@ -531,6 +537,24 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo
const phasesDir = planningPaths(cwd).phases; const phasesDir = planningPaths(cwd).phases;
const today = realClock.localToday(); const today = realClock.localToday();
const milestoneName = options.name || version; const milestoneName = options.name || version;
// ADR-3408 §8.5 / #3469: "liberal but visible" — when the write-seam
// composition's preservation stage restores a curated frontmatter value
// over a disagreeing freshly-derived one, that divergence is surfaced
// here rather than silently absorbed (the direct answer to #3374's
// `warnings: []`). Structured (field + reason), not prose, so a caller can
// assert on the value rather than regex a rendered message.
//
// Named `preservation_warnings`, NOT `warnings`: `cmdPhaseComplete` already
// exposes a sibling field called `warnings` typed as prose `string[]`. Reusing
// that name here for a structured `{field, reason}[]` shape would be the
// "Generative Fix Divergence" anti-pattern — two sibling state commands
// sharing one field name with different element types. `warnings` stays
// one meaning (prose) repo-wide; this is a distinct, machine-assertable
// signal for ADR-3408 §8.5's "preservation is visible" rule. Check
// `preservation_warnings.length` rather than a companion `has_warnings`
// flag — that flag existed only to mirror `cmdPhaseComplete`'s channel,
// which this field intentionally does not claim to be.
const preservationWarnings: Array<{ field: string; reason: string }> = [];
// Scope stats and accomplishments to only the phases belonging to the // Scope stats and accomplishments to only the phases belonging to the
// current milestone's ROADMAP. Uses the shared filter from roadmap-parser.cjs // current milestone's ROADMAP. Uses the shared filter from roadmap-parser.cjs
@@ -850,19 +874,49 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo
// reset, Operator Next Steps reset) is the pure `milestoneCompleteCore` in // reset, Operator Next Steps reset) is the pure `milestoneCompleteCore` in
// src/state-transition.cts, backed by the field-classification table. The // src/state-transition.cts, backed by the field-classification table. The
// runtime-specific next-milestone slash command is resolved here and injected // runtime-specific next-milestone slash command is resolved here and injected
// via the intent so the core stays pure. writeStateMd still owns the lock and // via the intent so the core stays pure.
// the steady-state syncStateFrontmatter post-sync. //
// ADR-3408 §8.3 / #3469: this used to write via `writeStateMd`, which gets
// sync and NO preservation — the identical shape #3374 reported for
// `phase.complete` (a stale body value silently clobbering fresher
// frontmatter). Routed through the single write-seam composition
// (`syncAndPreserveStateMd`) instead, under the same lock discipline
// `cmdPhaseComplete`'s atomic-commit adapter already uses: `withStateLock`
// wraps read + transform + sync + preserve + write so the read this
// transaction bases its transform on cannot be raced by a concurrent
// writer (closing a pre-existing TOCTOU gap `writeStateMd`'s own internal
// lock never covered, since the read used to happen before any lock was
// taken). `resync: true` mirrors `cmdPhaseComplete`'s posture (progress
// recomputed from disk; only the preserve-when-unchanged deltas apply) —
// milestone completion is the same kind of lifecycle transition.
if (fs.existsSync(statePath)) { if (fs.existsSync(statePath)) {
const result = transitionCore( withStateLock(statePath, () => {
fs.readFileSync(statePath, 'utf-8'), const originalStateContent = platformReadSync(statePath) || '';
{ const result = transitionCore(
kind: 'milestoneComplete', originalStateContent,
version, {
nextMilestoneCommand: formatGsdSlash('new-milestone', resolveRuntime(cwd)) as string, kind: 'milestoneComplete',
}, version,
{ clock: realClock, sourcePath: statePath }, nextMilestoneCommand: formatGsdSlash('new-milestone', resolveRuntime(cwd)) as string,
); },
writeStateMd(statePath, result.content, cwd); { clock: realClock, sourcePath: statePath },
);
const divergedFields: string[] = [];
const finalContent = syncAndPreserveStateMd(
originalStateContent,
result.content,
statePath,
cwd,
true,
undefined,
undefined,
divergedFields,
);
platformWriteSync(statePath, finalContent);
for (const field of divergedFields) {
preservationWarnings.push({ field, reason: 'preserved-over-disagreeing-derived' });
}
});
} }
// Archive phase directories if requested // Archive phase directories if requested
@@ -915,6 +969,7 @@ function cmdMilestoneComplete(cwd: string, version: string, options: MilestoneCo
}, },
milestones_updated: true, milestones_updated: true,
state_updated: fs.existsSync(statePath), state_updated: fs.existsSync(statePath),
preservation_warnings: preservationWarnings,
}; };
output(result, raw); output(result, raw);

View File

@@ -90,8 +90,7 @@ const {
readModifyWriteStateMd, readModifyWriteStateMd,
stateExtractField, stateExtractField,
stateReplaceField, stateReplaceField,
syncStateFrontmatter, syncAndPreserveStateMd,
applyPostSyncPreservation,
withStateLock, withStateLock,
updatePerformanceMetricsSection, updatePerformanceMetricsSection,
} = stateMod; } = stateMod;
@@ -2961,13 +2960,13 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
// to the STATE.md Transition Module. The ~90-line inline RMW callback // to the STATE.md Transition Module. The ~90-line inline RMW callback
// that lived here is the pure `completePhaseCore` in // that lived here is the pure `completePhaseCore` in
// src/state-transition.cts, backed by the field-classification table. // src/state-transition.cts, backed by the field-classification table.
// `updatePerformanceMetricsSection` + `syncStateFrontmatter` stay in // `updatePerformanceMetricsSection` stays in this adapter: it is a
// this adapter: they are section-table / disk-scan concerns, not // section-table / disk-scan concern, not a classified field. The
// classified fields, and `syncStateFrontmatter` is the post-sync this // sync + post-sync preservation this transaction needs runs via the
// transaction needs (it does NOT go through readModifyWriteStateMd // single write-seam composition, `syncAndPreserveStateMd` (it does
// because STATE.md is committed atomically with ROADMAP/REQUIREMENTS — // NOT go through readModifyWriteStateMd because STATE.md is
// the post-sync preservation pass runs via applyPostSyncPreservation // committed atomically with ROADMAP/REQUIREMENTS, ADR-3408 §8.3 /
// instead, #3374). // #3374 / #3469).
const nextPhaseDisplayName = const nextPhaseDisplayName =
phaseDisplayNameFromRoadmap(roadmapContent, nextPhaseNum) ?? phaseDisplayNameFromRoadmap(roadmapContent, nextPhaseNum) ??
phaseDisplayNameFromSlug(nextPhaseName); phaseDisplayNameFromSlug(nextPhaseName);
@@ -3021,27 +3020,28 @@ function cmdPhaseComplete(cwd: string, phaseNum: string, raw: boolean): void {
current_phase_name: nextPhaseDisplayName, current_phase_name: nextPhaseDisplayName,
} }
: undefined; : undefined;
const synced = syncStateFrontmatter(stateContent, cwd, authoritativeFm); // ADR-3408 §8.3 / #3469: this deliberately bypasses
// #3374: the direct sync above deliberately bypasses
// readModifyWriteStateMd (STATE.md is committed atomically with // readModifyWriteStateMd (STATE.md is committed atomically with
// ROADMAP/REQUIREMENTS), which also bypassed the #948/#1230 // ROADMAP/REQUIREMENTS), so it calls the single write-seam
// preservation pass every RMW write gets — so a stale body // composition (`syncAndPreserveStateMd`) directly instead of
// `Stopped at:` line silently clobbered a fresher frontmatter // assembling `syncStateFrontmatter` + `applyPostSyncPreservation`
// stopped_at on every completion. Run the shared post-sync pass: // itself — a call site re-assembling the pair, even with every step
// snapshots from the on-disk pre-image (originalStateContent) and the // calling an owner, is the exact re-derivation §8.3 forbids by name
// transformed content, table-driven applyStatePreservation, then the // (Phase 2 found this shape live here). The composition runs
// #2736 authoritative re-assert (which restores the #3350 pairing // snapshots from the on-disk pre-image (originalStateContent) and
// override the preserve-always restore may have reverted). resync=true // the transformed content, table-driven applyStatePreservation, then
// is the lifecycle-transition posture (progress recomputed from disk; // the #2736 authoritative re-assert (which restores the #3350
// only the preserve-when-unchanged deltas apply). Fields the // pairing override the preserve-always restore may have reverted).
// transition legitimately rewrote (Status, Phase, Stopped At via // resync=true is the lifecycle-transition posture (progress
// completePhaseCore's #3374 continuity line) have changed body // recomputed from disk; only the preserve-when-unchanged deltas
// sources, so their deltas do not fire. // apply). Fields the transition legitimately rewrote (Status, Phase,
stateContent = applyPostSyncPreservation( // Stopped At via completePhaseCore's #3374 continuity line) have
// changed body sources, so their deltas do not fire.
stateContent = syncAndPreserveStateMd(
originalStateContent, originalStateContent,
stateContent, stateContent,
synced,
statePath, statePath,
cwd,
true, true,
authoritativeFm, authoritativeFm,
); );

View File

@@ -1694,12 +1694,46 @@ function milestoneCompleteCore(
* Apply a `patch` transition to STATE.md content. * Apply a `patch` transition to STATE.md content.
* *
* Migrates `cmdStatePatch` (state.cts) onto the substrate. Applies each * Migrates `cmdStatePatch` (state.cts) onto the substrate. Applies each
* caller-supplied `{field: value}` pair via `stateReplaceField` over the full * caller-supplied `{field: value}` pair, resolved BODY-FIRST:
* content (body + frontmatter — patch can target either), tracking which fields *
* were updated vs. not found. * - A key that resolves against the STRIPPED body (via `stateReplaceField`,
* case-insensitive on the field name) is applied there and reported
* `updated` — this is the legitimate, documented case (display-cased body
* fields — Status, Current Plan, Phase — which are never frontmatter
* keys). It wins deterministically even when the same key also happens to
* exist as a parsed frontmatter key (e.g. `status` matches both the
* frontmatter key and a `Status:` body line) — frontmatter is inert for
* that key.
* - Only when the body has no match is the key checked against parsed
* frontmatter (determined structurally, never by a naming heuristic), and
* routed through the seam: `FIELD_CLASSIFICATION` governs it. A CLASSIFIED
* key (has a row, e.g. `current_phase`, `current_phase_name`) is NOT
* writable by an arbitrary patch — policy owns it — and is reported
* `failed`. An UNCLASSIFIED key (no row, e.g. a custom `risk_level`) is a
* pass-through per Phase 1 behavior-table row 19 ("field absent from
* FIELD_CLASSIFICATION → untouched pass-through"): it is applied directly
* to the frontmatter object before reassembly and reported `updated`.
* - A key matching neither the body nor the frontmatter is reported `failed`.
*
* ADR-3408 §8.3(b): this used to run `stateReplaceField` over the FULL
* document (body + frontmatter), which — because `field` is an arbitrary,
* caller-supplied string, unlike every other `stateReplaceField` call site in
* this file, which passes a fixed Title-Case string literal that can never
* collide with a lowercase/snake_case YAML key — let a frontmatter-shaped
* patch key (e.g. `status`, `current_phase`) match and rewrite the YAML
* frontmatter block directly via `stateReplaceField`'s case-insensitive
* `^field:` line pattern, entirely outside `FIELD_CLASSIFICATION` and the
* write-seam preservation policy: a second, undeclared writer. The fix is
* that a CLASSIFIED frontmatter key no longer writes outside the declared
* policy table — not that every frontmatter-shaped key stops working.
* `.gsd/phase/refactor-3469-one-write-seam/40-design.md` row 9 requires
* frontmatter changes to route through the seam (still work, governed by
* FIELD_CLASSIFICATION), not to stop working outright. Body-shaped keys
* (`Status`, `Current Plan`, `Phase`, ...) are the LEGITIMATE case and are
* unaffected — they were always matched against the body text, and still are.
* *
* The curated-field preservation that fixes #1743/#1695 is NOT in this core — * The curated-field preservation that fixes #1743/#1695 is NOT in this core —
* it lives in `readModifyWriteStateMd`'s post-sync delta (table-driven via * it lives in the write seam's post-sync delta (table-driven via
* `current_phase_name`'s `preserve-when-unchanged` row, ADR-3408 §8.1 — * `current_phase_name`'s `preserve-when-unchanged` row, ADR-3408 §8.1 —
* reclassified from `preserve-always` in #3468 to match its long-standing, * reclassified from `preserve-always` in #3468 to match its long-standing,
* delta-gated behavior). * delta-gated behavior).
@@ -1714,20 +1748,61 @@ function patchCore(
content: string, content: string,
intent: { kind: 'patch'; patches: Record<string, string> }, intent: { kind: 'patch'; patches: Record<string, string> },
): StateTransitionResult { ): StateTransitionResult {
const existingFm = extractFrontmatter(content) as Record<string, unknown>;
const hasFrontmatter = Object.keys(existingFm).length > 0;
let body = stripFrontmatter(content);
const fm: Record<string, unknown> = { ...existingFm };
const updated: string[] = []; const updated: string[] = [];
const failed: string[] = []; const failed: string[] = [];
let result = content;
for (const [field, value] of Object.entries(intent.patches)) { for (const [field, value] of Object.entries(intent.patches)) {
const replaced = stateReplaceField(result, field, value); // Body-first: a key that resolves against a body field is the
// legitimate, documented case (display-cased body fields — Status,
// Current Plan, Phase — are never frontmatter keys) and wins
// deterministically even when the same key also happens to exist as a
// frontmatter key (case-insensitively, via stateReplaceField's
// `^field:` pattern — e.g. `status` matching both the frontmatter key
// and a `Status:` body line). Frontmatter is only consulted when the
// body has no match for this key.
const replaced = stateReplaceField(body, field, value);
if (replaced !== null) { if (replaced !== null) {
result = replaced; body = replaced;
updated.push(field); updated.push(field);
} else { continue;
failed.push(field);
} }
if (Object.prototype.hasOwnProperty.call(existingFm, field)) {
// Frontmatter-shaped key: route through the seam. A classified field
// is policy-owned — a raw patch may not bypass it. An unclassified
// field is an untouched pass-through (behavior-table row 19).
if (getFieldClassification(field) !== null) {
failed.push(field);
} else {
fm[field] = value;
updated.push(field);
}
continue;
}
failed.push(field);
} }
if (updated.length === 0) {
// No field matched — return `content` VERBATIM (mirrors `updateCore`'s
// null-result branch): reassembling via stripFrontmatter/
// reconstructFrontmatter even when nothing changed can round-trip the
// frontmatter block to different bytes than the original (key order,
// formatting), which would falsely defeat `readModifyWriteStateMd`'s
// #948 no-op write guard for every patch that updates nothing, not just
// a frontmatter-shaped one.
return { content, updated, data: { updated, failed } };
}
const result = hasFrontmatter
? `---\n${reconstructFrontmatter(fm as unknown as Frontmatter)}\n---\n\n${body}`
: body;
return { content: result, updated, data: { updated, failed } }; return { content: result, updated, data: { updated, failed } };
} }

View File

@@ -2802,6 +2802,7 @@ function applyPostSyncPreservation(
resync: boolean, resync: boolean,
authoritativeFm?: Record<string, unknown>, authoritativeFm?: Record<string, unknown>,
deriveProgressKeys?: boolean, deriveProgressKeys?: boolean,
divergedFields?: string[],
): string { ): string {
// Snapshot the existing progress block BEFORE the transform so we can // Snapshot the existing progress block BEFORE the transform so we can
// restore it when resync is false. // restore it when resync is false.
@@ -2914,11 +2915,35 @@ function applyPostSyncPreservation(
// original four fields; this is the absorption ADR-1769 / CONTEXT.md // original four fields; this is the absorption ADR-1769 / CONTEXT.md
// already claimed shipped. // already claimed shipped.
const postFm = extractFrontmatter(syncedContent, statePath) as Record<string, unknown>; const postFm = extractFrontmatter(syncedContent, statePath) as Record<string, unknown>;
// #3469 (ADR-3408 §8.5): snapshot the freshly-synced (pre-preservation)
// frontmatter so a caller that wants visibility into "did preservation
// restore a curated value over a disagreeing derived one" can diff against
// it via the optional `divergedFields` out-param below. Additive only:
// callers that omit it (readModifyWriteStateMd, cmdPhaseComplete) pay
// nothing extra and see no change to `synced`/the returned content.
const preservationInputSnapshot = divergedFields ? { ...postFm } : null;
const preservation = applyStatePreservation({ const preservation = applyStatePreservation({
preFm, postFm, preFmSnapshot, resync, preFm, postFm, preFmSnapshot, resync,
deriveProgressKeys: deriveProgressKeys === true, deriveProgressKeys: deriveProgressKeys === true,
bodyDeltas, bodyDeltas,
}); });
if (divergedFields && preservationInputSnapshot) {
// §8.5's "liberal but visible": every field whose value actually
// differs before vs after `applyStatePreservation` is a field where the
// curated (frontmatter) value won over a disagreeing freshly-derived
// one — regardless of which policy executor fired. Diffing the object
// (rather than special-casing which executor mutated it) is intentional:
// it stays correct if a future FIELD_CLASSIFICATION row adds a new
// preservation policy without this function needing to know about it.
for (const key of Object.keys(preservation.postFm)) {
const before = preservationInputSnapshot[key];
const after = preservation.postFm[key];
const changed = (typeof before === 'object' || typeof after === 'object')
? JSON.stringify(before) !== JSON.stringify(after)
: before !== after;
if (changed) divergedFields.push(key);
}
}
// #2736: re-assert the intent-first values AFTER preservation. On STATE.md // #2736: re-assert the intent-first values AFTER preservation. On STATE.md
// layouts with no body `Phase:` line, both phase-source snapshots are null // 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 // (equal), so the #1695 restore fires and would put the stale pre-transition
@@ -2942,6 +2967,56 @@ function applyPostSyncPreservation(
return syncedContent; return syncedContent;
} }
/**
* ADR-3408 §8.3 — the ONE write-seam composition: `syncStateFrontmatter` then
* `applyPostSyncPreservation`, as a single named `content -> content`
* function. Every STATE.md write that (a) is not one of the two sanctioned-
* permanent exceptions (`cmdStateSync`, `REGENERATE_STATE` — §8.3's closed
* exception list, ADR Amendment 2) and (b) needs a non-standard I/O envelope
* calls THIS — never `syncStateFrontmatter` + `applyPostSyncPreservation`
* assembled locally. §8.3: "Assembling the stages at a call site is a
* re-derivation even when every step calls the owner." Phase 2 (#3469) found
* exactly that shape live in `cmdPhaseComplete`'s atomic-commit adapter
* (phase.cts) — every step called an owner, so the drift guard and an
* owner-level test both stayed green while the composition itself was free
* to diverge from `readModifyWriteStateMd`'s.
*
* Both current non-RMW callers of the pair — `readModifyWriteStateMd` and
* `cmdPhaseComplete`'s atomic 3-file commit adapter — now call this instead
* of assembling the two stages themselves. `cmdMilestoneComplete` (the
* #3374-shaped exposure `applyPostSyncPreservation`'s own docstring flagged
* as a follow-up) is the third.
*
* Returns CONTENT ONLY — a caller that needs its own I/O envelope (a lock,
* an atomic multi-file commit) supplies it around this call; this function
* never takes over the write.
*
* `divergedFields` is passed straight through to `applyPostSyncPreservation`
* — see its own docstring.
*/
function syncAndPreserveStateMd(
originalContent: string,
transformedContent: string,
statePath: string,
cwd: string | undefined,
resync: boolean,
authoritativeFm?: Record<string, unknown>,
deriveProgressKeys?: boolean,
divergedFields?: string[],
): string {
const synced = syncStateFrontmatter(transformedContent, cwd, authoritativeFm);
return applyPostSyncPreservation(
originalContent,
transformedContent,
synced,
statePath,
resync,
authoritativeFm,
deriveProgressKeys,
divergedFields,
);
}
/** /**
* Atomic read-modify-write for STATE.md. * Atomic read-modify-write for STATE.md.
* Holds the lock across the entire read -> transform -> write cycle, * Holds the lock across the entire read -> transform -> write cycle,
@@ -2981,14 +3056,16 @@ function readModifyWriteStateMd(statePath: string, transformFn: (content: string
return false; return false;
} }
let synced = syncStateFrontmatter(modified, cwd, options?.authoritativeFm); // #3469 (ADR-3408 §8.3): sync + post-sync preservation is the single
// #3374: the post-sync preservation pass (snapshots, table-driven // owned composition (`syncAndPreserveStateMd`), not assembled here — this
// applyStatePreservation, #2736 re-assert) — see applyPostSyncPreservation. // call site and `cmdPhaseComplete`'s atomic-commit adapter both route
synced = applyPostSyncPreservation( // through the same function so the composition cannot diverge between
// the two.
const synced = syncAndPreserveStateMd(
content, content,
modified, modified,
synced,
statePath, statePath,
cwd,
resync, resync,
options?.authoritativeFm, options?.authoritativeFm,
options?.deriveProgressKeys === true, options?.deriveProgressKeys === true,
@@ -4352,11 +4429,15 @@ export = {
readModifyWriteStateMd, readModifyWriteStateMd,
syncStateFrontmatter, syncStateFrontmatter,
// #3374: the shared post-sync preservation pass (snapshots + table-driven // #3374: the shared post-sync preservation pass (snapshots + table-driven
// applyStatePreservation + #2736 re-assert). Exported for cmdPhaseComplete's // applyStatePreservation + #2736 re-assert).
// 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, applyPostSyncPreservation,
// #3469 (ADR-3408 §8.3): the ONE write-seam composition (sync +
// preservation) as content -> content. Exported for cmdPhaseComplete's
// atomic-commit adapter (phase.cts, syncs STATE.md directly because it is
// committed atomically with ROADMAP/REQUIREMENTS) and for
// cmdMilestoneComplete (milestone.cts) — both need the composition's
// output but supply their own I/O envelope around it.
syncAndPreserveStateMd,
readStateHeadFreshness, readStateHeadFreshness,
withStateLock, withStateLock,
updatePerformanceMetricsSection, updatePerformanceMetricsSection,

View File

@@ -2094,6 +2094,81 @@ test('extractFrontmatter handles large frontmatter blocks without body bleed', (
// The body prose the transition wrote stays as designed. // The body prose the transition wrote stays as designed.
__assert2736.match(stateContent, /Phase: 2 — Closer-ruling measurement \(D1a\)/); __assert2736.match(stateContent, /Phase: 2 — Closer-ruling measurement \(D1a\)/);
}); });
// ADR-3408 §8.3 Matrix A5 (#3469, regression-critical): the OTHER branch
// of phase.cts's #3350 pairing decision — when the body carries NO
// Phase:/Current Phase field at all (narrative prose), authoritativeFm
// pairs BOTH current_phase and current_phase_name so the two frontmatter
// fields never describe different phases. The #2736 re-assertion (applied
// after preservation, via the shared syncAndPreserveStateMd composition
// this phase introduced) must still win over any restore for BOTH keys.
__t2736('A5 (#3469): with no body Phase field, the paired authoritativeFm override wins for BOTH current_phase and current_phase_name', () => {
const planningDir = __path2736.join(tmpDir, '.planning');
const phase1Dir = __path2736.join(planningDir, 'phases', '01-foundation');
__fs2736.mkdirSync(phase1Dir, { recursive: true });
__fs2736.writeFileSync(
__path2736.join(planningDir, 'ROADMAP.md'),
[
'# Roadmap',
'',
'- [ ] Phase 1: Foundation',
`- [ ] Phase 2: ${PAREN_NAME_2736}`,
'',
'### Phase 1: Foundation',
'**Goal:** Setup',
'**Plans:** 1 plans',
'',
`### Phase 2: ${PAREN_NAME_2736}`,
'**Goal:** Measure closer rulings',
'',
].join('\n'),
);
// Deliberately NO `Phase:` / `Current Phase:` line in the body.
__fs2736.writeFileSync(
__path2736.join(planningDir, 'STATE.md'),
[
'---',
'gsd_state_version: 1.0',
'current_phase: 1',
'current_phase_name: Foundation',
'status: executing',
'---',
'',
'# Project State',
'',
'## Current Position',
'',
'Currently working through Phase 1 setup tasks.',
'',
].join('\n'),
);
__fs2736.writeFileSync(__path2736.join(phase1Dir, '01-01-PLAN.md'), '# Plan\n');
__fs2736.writeFileSync(__path2736.join(phase1Dir, '01-01-SUMMARY.md'), '# Summary\n');
__fs2736.writeFileSync(
__path2736.join(phase1Dir, '01-VERIFICATION.md'),
['---', 'status: passed', '---', '', '# Verification', ''].join('\n'),
);
const result = __run2736(['phase', 'complete', '1'], tmpDir);
__assert2736.ok(result.success, `phase complete failed: ${result.error}`);
const stateContent = __fs2736.readFileSync(__path2736.join(planningDir, 'STATE.md'), 'utf-8');
const fm = extractFrontmatter(stateContent);
__assert2736.strictEqual(
fm.current_phase_name,
PAREN_NAME_2736,
`current_phase_name must be the exact next-phase display name; got ${JSON.stringify(fm.current_phase_name)}`,
);
__assert2736.strictEqual(
String(fm.current_phase),
'2',
'#3350: current_phase must be PAIRED with current_phase_name when the body carries no Phase field, ' +
`so the two frontmatter fields never describe different phases; got ${JSON.stringify(fm.current_phase)}`,
);
});
}); });
__d2736('#2736 sibling: state begin-phase preserves a paren-containing name (e2e)', () => { __d2736('#2736 sibling: state begin-phase preserves a paren-containing name (e2e)', () => {
@@ -2391,8 +2466,16 @@ test('extractFrontmatter handles large frontmatter blocks without body bleed', (
`first-write population from the (refreshed) body line must keep working; got ${JSON.stringify(fm.stopped_at)}`, `first-write population from the (refreshed) body line must keep working; got ${JSON.stringify(fm.stopped_at)}`,
); );
}); });
}); });
} }
// ADR-3408 §8.3 Matrix A4 (#3469): satisfied by the "AC1 preservation leg"
// test above ('with no session Stopped at line to refresh, the fresher
// frontmatter value survives') — that scenario now runs through the ONE
// write-seam composition (`syncAndPreserveStateMd`) instead of the
// hand-assembled sync+preserve pair `applyPostSyncPreservation`'s docstring
// used to describe. No new test added here: duplicating an already-passing,
// identically-shaped assertion adds no coverage.
// ──────────────────────────────────────────────────────────────────────── // ────────────────────────────────────────────────────────────────────────

View File

@@ -111,6 +111,9 @@ describe('milestone complete command', () => {
const milestones = fs.readFileSync(path.join(tmpDir, '.planning', 'MILESTONES.md'), 'utf-8'); const milestones = fs.readFileSync(path.join(tmpDir, '.planning', 'MILESTONES.md'), 'utf-8');
assert.ok(milestones.includes('v1.0 MVP Foundation')); assert.ok(milestones.includes('v1.0 MVP Foundation'));
assert.ok(milestones.includes('Set up project infrastructure')); assert.ok(milestones.includes('Set up project infrastructure'));
// B6 (ADR-3408 §8.3/#3469, independence): the new preservation-warnings
// channel must not disturb archival/roadmap behavior — it is additive.
assert.ok(Array.isArray(output.preservation_warnings), 'preservation_warnings must be an array');
}); });
test('#2118 — --dry-run does NOT mutate: no archive, no STATE.md rewrite, no phase move', () => { test('#2118 — --dry-run does NOT mutate: no archive, no STATE.md rewrite, no phase move', () => {
@@ -514,6 +517,142 @@ describe('milestone complete command', () => {
}); });
}); });
// ─────────────────────────────────────────────────────────────────────────────
// ADR-3408 §8.3 Matrix B (#3469): cmdMilestoneComplete now routes through the
// shared write-seam composition (syncAndPreserveStateMd) instead of the bare
// writeStateMd — the "real exposure" Finding 2 identified (the #3374 shape,
// applied to milestone.complete). Test matrix:
// .gsd/phase/refactor-3469-one-write-seam/50-test-matrix.md
// ─────────────────────────────────────────────────────────────────────────────
describe('ADR-3408 §8.3 Matrix B: cmdMilestoneComplete preserves + warns (#3469)', () => {
let tmpDir;
beforeEach(() => { tmpDir = createTempProject(); });
afterEach(() => { cleanup(tmpDir); });
// Frontmatter + a ## Session Stopped at body line milestoneCompleteCore
// never touches — the delta is therefore always "unchanged" for this
// field, exactly the #3374 shape (a stale body value vs. a curated
// frontmatter value) that Finding 2 says was silently clobbered pre-#3469.
function writeStateWithSession(dir, { fmStoppedAt, sessionStoppedAt, fmStatus = 'executing' } = {}) {
const fmLines = ['---', 'gsd_state_version: 1.0'];
if (fmStatus !== null) fmLines.push(`status: ${fmStatus}`);
if (fmStoppedAt !== undefined && fmStoppedAt !== null) fmLines.push(`stopped_at: "${fmStoppedAt}"`);
fmLines.push('---', '');
const bodyLines = [
'# State',
'',
'**Status:** In progress',
'**Last Activity:** 2025-01-01',
'**Last Activity Description:** Working',
'',
'## Session',
'',
'**Last session:** 2025-01-01T00:00:00.000Z',
];
if (sessionStoppedAt !== undefined && sessionStoppedAt !== null) {
bodyLines.push(`**Stopped at:** ${sessionStoppedAt}`);
}
bodyLines.push('');
fs.writeFileSync(path.join(dir, '.planning', 'STATE.md'), fmLines.concat(bodyLines).join('\n'));
}
// B1: stale body stopped_at, fresher frontmatter — frontmatter preserved.
test('B1: stale body Stopped at does not clobber a fresher curated frontmatter stopped_at', () => {
writeStateWithSession(tmpDir, { fmStoppedAt: 'Phase 7 verified PASS', sessionStoppedAt: 'Phase 3 work' });
const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
const fm = parseFrontmatter(state);
assert.strictEqual(
fm.stopped_at,
'Phase 7 verified PASS',
`frontmatter stopped_at must be preserved over the stale body value; got ${JSON.stringify(fm.stopped_at)}`,
);
});
// B2 (consumer-level, the criterion-6 substitute): the preserved value is
// observable through a SEPARATE subsequent CLI call reading the persisted
// file — not just this test's own fs.readFileSync of the writer's output.
test('B2: the preserved frontmatter value is observable via a separate `state get` call', () => {
writeStateWithSession(tmpDir, { fmStoppedAt: 'Phase 7 verified PASS', sessionStoppedAt: 'Phase 3 work' });
const complete = runGsdTools('milestone complete v1.0 --name Test', tmpDir);
assert.ok(complete.success, `Command failed: ${complete.error}`);
const got = runGsdTools('state get stopped_at', tmpDir);
assert.ok(got.success, `state get failed: ${got.error}`);
const gotOutput = JSON.parse(got.output);
assert.ok(
typeof gotOutput.stopped_at === 'string' && gotOutput.stopped_at.includes('Phase 7 verified PASS'),
`a second, independent CLI call must observe the preserved value; got ${JSON.stringify(gotOutput)}`,
);
});
// B3: divergence emits a structured, typed warning — never a rendered
// message. `preservation_warnings` (NOT `warnings`) is a distinct field
// shape from cmdPhaseComplete's prose `warnings: string[]` — see
// milestone.cts's own comment on Generative Fix Divergence.
test('B3: a preserved divergence emits preservation_warnings[0].field === "stopped_at"', () => {
writeStateWithSession(tmpDir, { fmStoppedAt: 'Phase 7 verified PASS', sessionStoppedAt: 'Phase 3 work' });
const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.ok(Array.isArray(output.preservation_warnings));
assert.ok(output.preservation_warnings.length > 0, 'a divergence must produce at least one warning');
assert.strictEqual(output.preservation_warnings[0].field, 'stopped_at');
assert.strictEqual(output.preservation_warnings[0].reason, 'preserved-over-disagreeing-derived');
});
// B4 (the false-positive guard): the body is genuinely newer THIS write —
// milestoneCompleteCore unconditionally rewrites body Status, so the delta
// always fires "changed" for `status`; the stale curated frontmatter value
// must NOT be restored, and no warning is emitted for it.
test('B4: a body field genuinely changed by this write is not preserved, and emits no warning', () => {
// #3469: `normalizeStateStatus` keyword-maps any "complete"-containing
// text to 'completed', and this write's own new body value ("v1.0
// milestone complete") legitimately derives to 'completed' too — so a
// stale curated value of 'completed' cannot discriminate "body won" from
// "stale value survived". Use a stale value that cannot collide with the
// derived result.
writeStateWithSession(tmpDir, { fmStatus: 'executing', fmStoppedAt: undefined, sessionStoppedAt: undefined });
const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
const statusWarning = output.preservation_warnings.find((w) => w.field === 'status');
assert.strictEqual(statusWarning, undefined, 'status changed this write — must not be reported preserved');
const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
const fm = parseFrontmatter(state);
assert.notStrictEqual(fm.status, 'executing', 'the stale curated status must not survive — body won');
assert.strictEqual(fm.status, 'completed', 'body-derived status must land');
});
// B5 (boundary): no pre-existing curated frontmatter to diverge from — no
// warning is emitted at all, and output is otherwise unaffected.
test('B5: no divergence at all — preservation_warnings is empty', () => {
writeRoadmap(tmpDir, `# Roadmap v1.0 MVP\n\n### Phase 1: Foundation\n**Goal:** Setup\n`);
// #3469: give Phase 1 a matching phase directory so the pre-existing
// unstarted-phase guard (src/milestone.cts) does not trip before any
// write-seam code runs — keeps this test exercising the real happy path.
mkPhaseDir(tmpDir, '01-foundation', { oneLiner: 'Setup' });
writeState(tmpDir); // no frontmatter at all — nothing curated to diverge from
const result = runGsdTools('milestone complete v1.0 --name Test', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.deepStrictEqual(output.preservation_warnings, []);
});
});
// ───────────────────────────────────────────────────────────────────────────── // ─────────────────────────────────────────────────────────────────────────────
// phases clear command // phases clear command
// ───────────────────────────────────────────────────────────────────────────── // ─────────────────────────────────────────────────────────────────────────────

View File

@@ -6546,6 +6546,137 @@ describe('bug-3287 — init plan-phase exposes expected_phase_dir with project_c
); );
}); });
}); });
// ─────────────────────────────────────────────────────────────────────────
// ADR-3408 §8.3 Matrix A (#3469): cmdPhaseComplete now calls the ONE
// write-seam composition (syncAndPreserveStateMd) directly instead of
// hand-assembling syncStateFrontmatter + applyPostSyncPreservation itself
// (Finding 3's re-derivation). Rows A2/A3's general identity claim (the
// composition agrees with itself regardless of caller) is covered by the
// required fast-check property test in tests/state.test.cjs; this block
// covers the ones that need a REAL cmdPhaseComplete run — A6/A7's atomic
// 3-file commit, plus a concrete consumer-level demonstration that BOTH
// sync (a body-derived field advances) and preservation (an untouched
// curated field survives) fire together through the real CLI path.
// ─────────────────────────────────────────────────────────────────────────
describe('ADR-3408 §8.3 Matrix A: cmdPhaseComplete write-seam composition (#3469)', () => {
let tmpDir;
beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3469-a-'));
});
afterEach(() => {
cleanup(tmpDir);
});
// A2/A3 (concrete, consumer-level): the same real cmdPhaseComplete call
// both advances a body-derived field (progress.completed_phases, via the
// transition) AND restores a CLASSIFIED preserve-when-unchanged field
// (`paused_at`) completePhaseCore's transform never touches — proof the
// seam applies sync AND `applyPostSyncPreservation`'s policy TOGETHER
// through the real adapter, not just the transition alone.
test('A2/A3: phase.complete advances the body-derived phase AND restores an untouched preserve-when-unchanged field, in one write', () => {
setupPhase3517Project(tmpDir);
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
// Seed a curated `paused_at` — completePhaseCore never writes the
// `## Session` `Paused At` body line, so its pre/post body source is
// unchanged (both absent) and the classified preserve-when-unchanged
// row must restore this curated value over the sync's empty derive.
const before = fs.readFileSync(statePath, 'utf8');
fs.writeFileSync(statePath, before.replace(/^gsd_state_version: 1\.0$/m, 'gsd_state_version: 1.0\npaused_at: "curated pause note — must survive"'));
const r = runSdkQuery(['phase.complete', '5'], tmpDir);
assert.ok(r.success, `call failed: ${r.error}`);
const state = fs.readFileSync(statePath, 'utf8');
const fm = extractFrontmatter(state);
// A2/A3: the transition genuinely advanced the body-derived phase...
assert.equal(Number(fm.progress && fm.progress.completed_phases), 2);
// ...while `applyPostSyncPreservation`'s classified restore fired in
// the SAME write — the composition's preservation stage ran, not just
// its sync stage.
assert.equal(
fm.paused_at,
'curated pause note — must survive',
'a classified preserve-when-unchanged field completePhaseCore never touches must survive the same write',
);
});
// A6 (independence): the composition returns content; the adapter's own
// atomic 3-file envelope is unaffected — ROADMAP, REQUIREMENTS, and
// STATE.md all change together as one unit.
test('A6: STATE.md, ROADMAP.md, and REQUIREMENTS.md commit atomically as one unit', () => {
setupPhase3517Project(tmpDir);
const paths = {
state: path.join(tmpDir, '.planning', 'STATE.md'),
roadmap: path.join(tmpDir, '.planning', 'ROADMAP.md'),
};
const before = {
state: fs.readFileSync(paths.state, 'utf8'),
roadmap: fs.readFileSync(paths.roadmap, 'utf8'),
};
const r = runSdkQuery(['phase.complete', '5'], tmpDir);
assert.ok(r.success, `call failed: ${r.error}`);
const after = {
state: fs.readFileSync(paths.state, 'utf8'),
roadmap: fs.readFileSync(paths.roadmap, 'utf8'),
};
assert.notStrictEqual(after.state, before.state, 'STATE.md must change');
assert.notStrictEqual(after.roadmap, before.roadmap, 'ROADMAP.md must change');
});
// A7 (IO failure, filesystem-failure category): a mid-commit failure on
// the FIRST file in the atomic set (ROADMAP.md) leaves NONE of the three
// partially written — including STATE.md, which now flows through the
// shared syncAndPreserveStateMd composition before writePlanningFileSet
// ever sees it. `t.mock.method` auto-restores at test end — never
// chmod 0o000, which root bypasses under Docker/CI.
test('A7: a failure writing ROADMAP.md (first in the atomic set) leaves STATE.md and REQUIREMENTS.md untouched', (t) => {
setupPhase3517Project(tmpDir);
const roadmapPath = path.join(tmpDir, '.planning', 'ROADMAP.md');
const reqPath = path.join(tmpDir, '.planning', 'REQUIREMENTS.md');
const statePath = path.join(tmpDir, '.planning', 'STATE.md');
const before = {
roadmap: fs.readFileSync(roadmapPath, 'utf8'),
req: fs.existsSync(reqPath) ? fs.readFileSync(reqPath, 'utf8') : null,
state: fs.readFileSync(statePath, 'utf8'),
};
writePassedVerificationForPhase(tmpDir, '5');
const phaseModule = require('../gsd-core/bin/lib/phase.cjs');
const originalWriteFileSync = fs.writeFileSync;
t.mock.method(fs, 'writeFileSync', function injectedRoadmapWriteFailure(target, ...args) {
const targetPath = String(target);
if (targetPath === roadmapPath || targetPath === `${roadmapPath}.tmp.${process.pid}`) {
const err = new Error('injected ROADMAP.md write failure');
err.code = 'EIO';
throw err;
}
return originalWriteFileSync.call(this, target, ...args);
});
assert.throws(
() => phaseModule.cmdPhaseComplete(tmpDir, '5', false),
/injected ROADMAP\.md write failure/,
);
const after = {
roadmap: fs.readFileSync(roadmapPath, 'utf8'),
req: fs.existsSync(reqPath) ? fs.readFileSync(reqPath, 'utf8') : null,
state: fs.readFileSync(statePath, 'utf8'),
};
assert.strictEqual(after.roadmap, before.roadmap, 'ROADMAP.md must be unchanged');
assert.strictEqual(after.req, before.req, 'REQUIREMENTS.md must be unchanged');
assert.strictEqual(after.state, before.state, 'STATE.md must be unchanged — none of the three partially written');
});
});
} }
// ───────────────────────────────────────────────────────────────────────────── // ─────────────────────────────────────────────────────────────────────────────

View File

@@ -1145,13 +1145,148 @@ describe('ADR-1769 Phase 6: patch transition — field updates', () => {
assert.deepStrictEqual(result.data && result.data.failed, ['Nonexistent']); assert.deepStrictEqual(result.data && result.data.failed, ['Nonexistent']);
}); });
test('patching a frontmatter YAML key directly updates the YAML line', () => { // ADR-3408 §8.3(b) / #3469: STALE as of this phase — patch now strips
// patch operates on the full content (body + frontmatter), so a lowercase // frontmatter FIRST (matching updateCore), so a lowercase frontmatter key
// frontmatter key like `stopped_at` is matched and replaced. // like `stopped_at` can no longer reach the YAML block through this path.
// The old assertion here ("patch operates on the full content... a
// lowercase frontmatter key is matched and replaced") is the exact bypass
// ADR-3408 §8.3(b) closes: it let an arbitrary caller-supplied patch key
// rewrite YAML frontmatter outside FIELD_CLASSIFICATION, undetected by the
// write-seam guard's Axis 2 (every step called an owner). See the Matrix D
// section below for the corrected contract.
test('patching a frontmatter-shaped key no longer reaches the YAML line (ADR-3408 §8.3(b) / #3469)', () => {
const input = ['---', 'status: executing', 'stopped_at: 2026-01-01', '---', '', '# State', ''].join('\n'); const input = ['---', 'status: executing', 'stopped_at: 2026-01-01', '---', '', '# State', ''].join('\n');
const result = transitionCore(input, { kind: 'patch', patches: { stopped_at: '2026-06-27' } }, deps); const result = transitionCore(input, { kind: 'patch', patches: { stopped_at: '2026-06-27' } }, deps);
assert.ok(/^stopped_at: 2026-06-27$/m.test(result.content), 'YAML stopped_at must be patched'); assert.ok(!/^stopped_at: 2026-06-27$/m.test(result.content), 'YAML stopped_at must NOT be patched directly');
assert.deepStrictEqual(result.data && result.data.updated, ['stopped_at']); assert.ok(/^stopped_at: 2026-01-01$/m.test(result.content), 'YAML stopped_at must survive unchanged');
assert.deepStrictEqual(result.data && result.data.updated, []);
assert.deepStrictEqual(result.data && result.data.failed, ['stopped_at']);
});
});
// ─────────────────────────────────────────────────────────────────────────────
// ADR-3408 §8.3(b) Matrix D (#3469): patchCore strips frontmatter first,
// matching updateCore — closes Phase 1's declared known gap (a
// frontmatter-shaped patch key could rewrite the YAML block outside
// FIELD_CLASSIFICATION). Test matrix: .gsd/phase/refactor-3469-one-write-seam/50-test-matrix.md
// ─────────────────────────────────────────────────────────────────────────────
describe('ADR-3408 §8.3(b) Matrix D: patchCore strips frontmatter first (#3469)', () => {
const deps = { clock: fixedClock };
// D1: a frontmatter-shaped key (snake_case, no Title-Case body counterpart)
// can never match — patch is body-only now, so the write "routes through
// the seam": a frontmatter change can only happen via FIELD_CLASSIFICATION's
// own preservation/sync machinery, never via a direct patch bypass.
test('D1: a frontmatter-shaped key (current_phase) is reported failed, and the frontmatter is untouched', () => {
const input = [
'---',
'current_phase: "3"',
'---',
'',
'# State',
'',
'## Current Position',
'',
'Phase: 3 (alpha)',
'',
].join('\n');
const result = transitionCore(input, { kind: 'patch', patches: { current_phase: '9' } }, deps);
assert.deepStrictEqual(result.data && result.data.updated, []);
assert.deepStrictEqual(result.data && result.data.failed, ['current_phase']);
assert.ok(/^current_phase: "3"$/m.test(result.content), 'frontmatter current_phase must be unchanged');
assert.strictEqual(result.content, input, 'no-op: content returned verbatim when nothing in the body matched');
});
// D2: a body-shaped key (Title-Case) is the legitimate case and is
// unaffected by the strip-first fix — it was always matched against the
// body, and still is.
test('D2: a body-shaped key (Status) still updates the body — the legitimate, unaffected case', () => {
const input = ['---', 'status: executing', '---', '', '# State', '', '**Status:** Planning', ''].join('\n');
const result = transitionCore(input, { kind: 'patch', patches: { Status: 'Paused' } }, deps);
assert.deepStrictEqual(result.data && result.data.updated, ['Status']);
assert.deepStrictEqual(result.data && result.data.failed, []);
assert.ok(result.content.includes('**Status:** Paused'), 'body Status must be updated');
assert.ok(/^status: executing$/m.test(result.content), 'frontmatter status is untouched by patchCore itself');
});
// D3 (boundary, extend #3351 variants A/B): a display-cased key matching
// the body field succeeds; the SAME field's lower-cased/frontmatter-shaped
// spelling fails — same content, two spellings, two outcomes.
test('D3: display-cased "Current Phase" succeeds; lower-cased "current_phase" fails, on the same content', () => {
const input = [
'---',
'current_phase: "1"',
'---',
'',
'# State',
'',
'**Current Phase:** 1',
'',
].join('\n');
const displayCased = transitionCore(input, { kind: 'patch', patches: { 'Current Phase': '2' } }, deps);
assert.deepStrictEqual(displayCased.data && displayCased.data.updated, ['Current Phase']);
assert.ok(displayCased.content.includes('**Current Phase:** 2'));
const lowerCased = transitionCore(input, { kind: 'patch', patches: { current_phase: '2' } }, deps);
assert.deepStrictEqual(lowerCased.data && lowerCased.data.updated, []);
assert.deepStrictEqual(lowerCased.data && lowerCased.data.failed, ['current_phase']);
});
// D4 (hostile): a key that is simultaneously frontmatter-shaped AND
// case-insensitively matches a body field (stateReplaceField's `^field:`
// pattern is case-insensitive) — the body is the ONE deterministic winner,
// asserted explicitly, because frontmatter is stripped out of the matching
// surface before any pattern ever runs.
test('D4: a key matching both a frontmatter key and a body field — body wins deterministically, frontmatter inert', () => {
const input = ['---', 'status: executing', '---', '', '# State', '', '**Status:** In progress', ''].join('\n');
const result = transitionCore(input, { kind: 'patch', patches: { status: 'Aborted' } }, deps);
assert.deepStrictEqual(result.data && result.data.updated, ['status']);
assert.ok(result.content.includes('**Status:** Aborted'), 'body Status must be the one that changed');
assert.ok(/^status: executing$/m.test(result.content), 'frontmatter status key must never be touched by patchCore');
});
// D5 (boundary, extend): an empty patch is a true no-op — no write, both
// report arrays empty.
test('D5: an empty patch {} is a no-op — content returned verbatim, both arrays empty', () => {
const input = '# State\n\n**Status:** Planning\n';
const result = transitionCore(input, { kind: 'patch', patches: {} }, deps);
assert.strictEqual(result.content, input);
assert.deepStrictEqual(result.data && result.data.updated, []);
assert.deepStrictEqual(result.data && result.data.failed, []);
});
// D6 (hostile): __proto__ / constructor as patch keys must not pollute
// Object.prototype. `intent.patches` is built via JSON.parse (the shape a
// real `state.patch` JSON payload takes) specifically because object-
// literal syntax special-cases `__proto__` — JSON.parse does not, and is
// the classic prototype-pollution vector this test must exercise for real.
test('D6: __proto__ / constructor patch keys do not pollute Object.prototype', () => {
const patches = JSON.parse('{"__proto__":"evil","constructor":"evil2"}');
const input = '# State\n\n**Status:** Planning\n';
const result = transitionCore(input, { kind: 'patch', patches }, deps);
// Object.prototype itself must be untouched.
assert.strictEqual(Object.getPrototypeOf({}), Object.prototype);
assert.strictEqual(({}).polluted, undefined);
assert.strictEqual(typeof ({}).constructor, 'function');
// Behaves like any other unmatched field — no body line named
// `__proto__` or `constructor` exists, so both are reported failed.
assert.deepStrictEqual((result.data && result.data.updated) || [], []);
assert.deepStrictEqual((result.data && result.data.failed || []).sort(), ['__proto__', 'constructor']);
assert.strictEqual(result.content, input);
});
// D7 (independence, extend): updateCore is unchanged — it already strips
// frontmatter first, the correct shape patchCore now matches. A
// frontmatter-shaped `field` still cannot reach the YAML block through it.
test('D7: updateCore is unchanged — a frontmatter-shaped field still cannot reach the YAML block', () => {
const input = ['---', 'current_phase: "3"', '---', '', '# State', '', '**Status:** Planning', ''].join('\n');
const result = transitionCore(input, { kind: 'update', field: 'current_phase', value: '9' }, deps);
assert.strictEqual(result.content, input);
assert.strictEqual(result.data && result.data.updated, false);
assert.ok(/^current_phase: "3"$/m.test(result.content));
}); });
}); });

View File

@@ -50,9 +50,11 @@ const {
findSeamBypasses, findSeamBypasses,
findPromptSeamUses, findPromptSeamUses,
findPolicyDispatchDrift, findPolicyDispatchDrift,
findUnstrippedContentWrites,
applyRatchet, applyRatchet,
loadBaseline, loadBaseline,
buildBaselineEntries, buildBaselineEntries,
collect,
SEAM_OWNER_FILE, SEAM_OWNER_FILE,
SEAM_OWNER_EXEMPT_FUNCTIONS, SEAM_OWNER_EXEMPT_FUNCTIONS,
EXECUTOR_FILE, EXECUTOR_FILE,
@@ -255,18 +257,23 @@ describe('D8 — comments are not drift', () => {
describe('D9 — owner functions are exempt', () => { describe('D9 — owner functions are exempt', () => {
test('guard: owner functions are exempt', () => { test('guard: owner functions are exempt', () => {
assert.ok(SEAM_OWNER_EXEMPT_FUNCTIONS.includes('readModifyWriteStateMd')); // #3469: `readModifyWriteStateMd` now calls the single
// `syncAndPreserveStateMd` symbol rather than assembling the two seam
// calls itself, so it needs no exemption — `syncAndPreserveStateMd` is
// the sole legitimate place `syncStateFrontmatter(` and
// `applyPostSyncPreservation(` appear together (the composition every
// OTHER caller, including `readModifyWriteStateMd`, now routes through).
assert.ok(SEAM_OWNER_EXEMPT_FUNCTIONS.includes('syncAndPreserveStateMd'));
const text = [ const text = [
'function readModifyWriteStateMd(cwd) {', 'function syncAndPreserveStateMd(originalContent, transformedContent, statePath, cwd, resync) {',
' const modified = compute();', ' const synced = syncStateFrontmatter(transformedContent, cwd);',
' writeStateMd(statePath, modified, cwd);', ' return applyPostSyncPreservation(originalContent, transformedContent, synced, statePath, resync);',
' return modified;',
'}', '}',
].join('\n'); ].join('\n');
// The seam's own internal plumbing (the I/O wrapper calling the pure // The seam's own internal plumbing (the one owned composition —
// sync stage) is not a bypass. // sync then post-sync preservation) is not a bypass.
assert.deepStrictEqual(findSeamBypasses(SEAM_OWNER_FILE, text), []); assert.deepStrictEqual(findSeamBypasses(SEAM_OWNER_FILE, text), []);
}); });
}); });
@@ -564,3 +571,190 @@ describe('D15 — file (and field) values are sanitized at construction', () =>
assert.ok(!out[0].field.includes(C1_CSI)); assert.ok(!out[0].field.includes(C1_CSI));
}); });
}); });
// ─────────────────────────────────────────────────────────────────────────
// Phase 2 (#3469) — ADR-3408 §8.3 Matrix section E: guard rows closing
// Phase 1's declared known gap (Axis 3, §8.3(b)) and pinning the ratchet's
// new 2-permanent-entry shape (Amendment 2). Test matrix:
// .gsd/phase/refactor-3469-one-write-seam/50-test-matrix.md
//
// E4/E5 are the false-positive guards — the exact shape that measured 29
// false positives to 1 true positive in Phase 1's naive co-occurrence
// approximation (see this guard's own header, Axis 3). E7 is the inverse: a
// sanctioned-permanent entry vanishing from the observed tree must FAIL, not
// silently reach zero — a guard reaching zero here would only do so by
// having stopped looking at a real writer.
// ─────────────────────────────────────────────────────────────────────────
describe('E1 — a re-assembled composition at a new call site is detected', () => {
test('guard: a call site invoking syncStateFrontmatter and applyPostSyncPreservation directly (bypassing syncAndPreserveStateMd) is caught on BOTH calls', () => {
// Finding 3's exact shape (ADR-3408 Amendment 2): every step calls an
// owner, so neither call alone is undeclared — but assembling the PAIR
// at a call site outside the seam composition is the re-derivation §8.3
// forbids by name. Synthetic: the real instance of this shape
// (cmdPhaseComplete's pre-#3469 adapter) was fixed by this same phase.
const text = [
'function cmdReassembledAdapter(cwd, statePath, stateContent) {',
' let synced = syncStateFrontmatter(stateContent, cwd, authoritativeFm);',
' synced = applyPostSyncPreservation(originalStateContent, stateContent, synced, statePath, true, authoritativeFm);',
' return synced;',
'}',
].join('\n');
const observed = findSeamBypasses(OTHER_FILE, text);
assert.strictEqual(observed.length, 2, 'both re-assembled stages must be caught, not just one');
assert.deepStrictEqual(observed.map((f) => f.symbol).sort(), ['applyPostSyncPreservation', 'syncStateFrontmatter']);
const findings = applyRatchet(observed, { entries: [] });
assert.strictEqual(findings.length, 2);
assert.ok(findings.every((f) => f.reason === REASON.SEAM_BYPASS_UNRECORDED));
});
});
describe('E2 — a legitimate single call to the composition is not detected', () => {
test('guard: calling syncAndPreserveStateMd (the ONE write-seam composition) is not a bypass', () => {
// Verbatim from src/milestone.cts's real cmdMilestoneComplete call site
// (ADR-3408 Amendment 2's third caller).
const text = [
' const finalContent = syncAndPreserveStateMd(',
' originalStateContent,',
' result.content,',
' statePath,',
' cwd,',
' true,',
' undefined,',
' undefined,',
' divergedFields,',
' );',
].join('\n');
assert.deepStrictEqual(findSeamBypasses(OTHER_FILE, text), []);
});
});
describe('E3 — a patchCore-style frontmatter write is detected (closes the Phase 1 declared gap)', () => {
test('guard: stateReplaceField over unstripped content with a variable field name is caught', () => {
// The pre-Phase-2 shape #3469 fixed: patchCore ran stateReplaceField
// over content that was never stripped of frontmatter, letting a
// lowercase/frontmatter-shaped patch key rewrite the YAML block
// directly, outside FIELD_CLASSIFICATION.
const text = [
'function patchCoreOld(content, intent) {',
' let modified = content;',
' for (const [field, value] of Object.entries(intent.patches)) {',
' const replaced = stateReplaceField(modified, field, value);',
' if (replaced !== null) modified = replaced;',
' }',
' return { content: modified };',
'}',
].join('\n');
const out = findUnstrippedContentWrites(EXECUTOR_FILE, text);
assert.strictEqual(out.length, 1);
assert.strictEqual(out[0].reason, REASON.UNSTRIPPED_CONTENT_WRITE);
assert.strictEqual(out[0].line, 4);
});
});
describe('E4 — updateCore\'s strip-then-replace is NOT detected', () => {
test('guard: the real updateCore call site (content stripped first) is not flagged', () => {
// Verbatim from src/state-transition.cts's real updateCore — the shape
// Phase 1 measured a naive co-occurrence detector at 29 false positives
// to 1 true positive against; this is one of the 29.
const text = [
'function updateCore(content, intent) {',
' const existingFm = extractFrontmatter(content) as Record<string, unknown>;',
' const hasFrontmatter = Object.keys(existingFm).length > 0;',
' const body = stripFrontmatter(content);',
' const result = stateReplaceField(body, intent.field, intent.value);',
' if (result === null) {',
' return { content, updated: [], data: { updated: false } };',
' }',
'}',
].join('\n');
assert.deepStrictEqual(findUnstrippedContentWrites(EXECUTOR_FILE, text), []);
});
});
describe('E5 — sectionBody-scoped stateReplaceField calls are NOT detected', () => {
test('guard: a literal field name against a non-stripFrontmatter-derived section slice is not flagged', () => {
// Verbatim from src/state-transition.cts's real mutateCurrentPositionFirstTime:
// sectionBody is a Current-Position section slice (frontmatter-free by
// construction — it comes from body.slice(...), never from raw content),
// and the field name is a fixed Title-Case literal that can never
// collide with a lowercase/snake_case YAML key. One of the ~20 calls
// Phase 1's naive detector over-reported.
const text = [
'function mutateCurrentPositionFirstTime(body, intent, today, updated) {',
' const span = locateCurrentPosition(body);',
' if (span === null) return body;',
' let sectionBody = body.slice(span.start, span.end);',
' const phaseLabel = `${intent.phaseNumber} — EXECUTING`;',
' if (/^Phase:/m.test(sectionBody)) {',
' sectionBody = sectionBody.replace(/^Phase:.*$/m, `Phase: ${phaseLabel}`);',
' } else {',
" const replaced = stateReplaceField(sectionBody, 'Phase', phaseLabel);",
' if (replaced !== null) sectionBody = replaced;',
' }',
'}',
].join('\n');
assert.deepStrictEqual(findUnstrippedContentWrites(EXECUTOR_FILE, text), []);
});
});
describe('E6 — ratchet: exactly 2 sanctioned-permanent entries remain (limit)', () => {
test('guard: the real baseline has exactly 2 permanent entries, and the real tree matches it with zero findings', () => {
const baseline = loadBaseline();
assert.strictEqual(
baseline.entries.length,
2,
'ADR-3408 Amendment 2: the ratchet holds exactly 2 sanctioned-permanent entries, not 0 — ' +
'Phase 4 does not drive this baseline to empty',
);
for (const entry of baseline.entries) {
assert.strictEqual(entry.owner, 'sanctioned-permanent');
}
const { seamFindings } = collect();
const findings = applyRatchet(seamFindings, baseline);
assert.deepStrictEqual(findings, [], 'the real tree must match the 2-entry baseline exactly');
});
});
describe('E7 — ratchet: a sanctioned-permanent entry disappearing fails (limit-1)', () => {
test('guard: removing one of the two permanent entries from the observed tree is reported STALE, not silently accepted', () => {
const baseline = loadBaseline();
assert.strictEqual(baseline.entries.length, 2);
// Simulate one sanctioned entry (cmdStateSync's writeStateMd call)
// vanishing from the observed tree — exactly the shape §8.3's closed
// exception list forbids: a sanctioned exception may not silently
// disappear (a guard reaching zero here would only do so by having
// stopped looking at a real writer).
const vanished = baseline.entries[0];
const stillPresent = baseline.entries[1];
const observed = [{ file: stillPresent.file, source: stillPresent.source, symbol: stillPresent.symbol, line: 1 }];
const findings = applyRatchet(observed, baseline);
assert.strictEqual(findings.length, 1);
assert.strictEqual(findings[0].reason, REASON.BASELINE_ENTRY_STALE);
assert.strictEqual(findings[0].file, vanished.file);
assert.strictEqual(findings[0].observed, 0);
assert.strictEqual(findings[0].acknowledged, 1);
});
});
describe('E8 — ratchet: a 3rd bypass beside the 2 sanctioned entries fails as unrecorded (limit+1)', () => {
test('guard: a new, unacknowledged writeStateMd call alongside the 2 sanctioned entries fails', () => {
const baseline = loadBaseline();
assert.strictEqual(baseline.entries.length, 2);
const matchingObserved = baseline.entries.map((e) => ({ file: e.file, source: e.source, symbol: e.symbol, line: 1 }));
const newBypass = { file: OTHER_FILE, source: 'writeStateMd(statePath, modified, cwd);', symbol: 'writeStateMd', line: 42 };
const observed = [...matchingObserved, newBypass];
const findings = applyRatchet(observed, baseline);
assert.strictEqual(findings.length, 1);
assert.strictEqual(findings[0].reason, REASON.SEAM_BYPASS_UNRECORDED);
assert.strictEqual(findings[0].file, OTHER_FILE);
});
});

View File

@@ -13,6 +13,9 @@ const os = require('os');
const path = require('path'); const path = require('path');
const { runGsdTools, createTempDir, createTempProject, cleanup } = require('./helpers.cjs'); const { runGsdTools, createTempDir, createTempProject, cleanup } = require('./helpers.cjs');
const { createFixture, seedWorkstream } = require('./fixtures/index.cjs'); const { createFixture, seedWorkstream } = require('./fixtures/index.cjs');
// ADR-3408 §8.3 Matrix A2/A3 (#3469): required fast-check property test — the
// composed cmdPhaseComplete/readModifyWriteStateMd write-seam identity.
const fc = require('fast-check');
// #3187 (ADR-3180 §7.7) matrix sections B/C: in-process access to the chain // #3187 (ADR-3180 §7.7) matrix sections B/C: in-process access to the chain
// owner and its raw inputs, needed to compute the "owner's answer" a // owner and its raw inputs, needed to compute the "owner's answer" a
// consumer's OBSERVABLE output is compared against (Decision 4c) — never the // consumer's OBSERVABLE output is compared against (Decision 4c) — never the
@@ -4907,6 +4910,151 @@ describe('state sync command', () => {
const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8'); const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.strictEqual(before, after, 'File should not be modified in verify mode'); assert.strictEqual(before, after, 'File should not be modified in verify mode');
}); });
// ADR-3408 §8.3 Matrix C3 (#3469, extend): `--verify` stays a true dry run
// for the sanctioned-exception path too — no write happens even though the
// (unwritten) sync would have let the body win over a curated frontmatter
// value.
test('C3 (#3469): --verify does not write even when the body would win over a curated frontmatter value', () => {
const content = [
'---',
'gsd_state_version: 1.0',
'stopped_at: "curated stale value"',
'---',
'',
'# Project State',
'',
'## Session',
'',
'**Stopped at:** fresh body value',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), content);
const result = runGsdTools('state sync --verify', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.strictEqual(output.dry_run, true);
const after = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
assert.strictEqual(after, content, '--verify must not write, regardless of what a real sync would change');
});
});
// ─────────────────────────────────────────────────────────────────────────────
// ADR-3408 §8.3 Matrix C (#3469): cmdStateSync is a SANCTIONED EXCEPTION.
// The most important section of this phase's matrix — a regression here
// silently INVERTS a shipped feature (#905: "body annotation beats existing
// frontmatter when both are present") with every OTHER gate green. state
// sync exists to re-derive frontmatter FROM the body; routing it through
// preservation would defeat the command. Test matrix:
// .gsd/phase/refactor-3469-one-write-seam/50-test-matrix.md
// ─────────────────────────────────────────────────────────────────────────────
describe('ADR-3408 §8.3 Matrix C: cmdStateSync — the sanctioned exception (#3469)', () => {
let tmpDir;
beforeEach(() => { tmpDir = createFixture(); });
afterEach(() => { cleanup(tmpDir); });
// C1 — the whole point: body annotation vs existing frontmatter, both
// present, differing. `state sync` re-derives frontmatter FROM the body
// (#905) — the body must win, never the curated frontmatter.
test('C1: body annotation wins over existing (differing) frontmatter — the whole point of `state sync`', () => {
const content = [
'---',
'gsd_state_version: 1.0',
'stopped_at: "curated stale value — must NOT survive"',
'---',
'',
'# Project State',
'',
'## Session',
'',
'**Stopped at:** fresh body value — must win',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), content);
const result = runGsdTools('state sync', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
const fm = frontmatterLib.extractFrontmatter(state);
assert.strictEqual(
fm.stopped_at,
'fresh body value — must win',
`body must win over the curated frontmatter value; a preservation regression here would ` +
`silently invert #905 with every other gate green; got ${JSON.stringify(fm.stopped_at)}`,
);
});
// C2: a stale-LOOKING frontmatter value (a different field than C1, to pin
// the contract on a second, independent field) loses to a fresh body value.
test('C2: a stale-looking frontmatter current_phase_name loses to a fresh body Phase: name', () => {
const content = [
'---',
'gsd_state_version: 1.0',
'current_phase: "1"',
'current_phase_name: Old Stale Name',
'---',
'',
'# Project State',
'',
'## Current Position',
'',
'Phase: 1 (Fresh Correct Name)',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), content);
const result = runGsdTools('state sync', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
const fm = frontmatterLib.extractFrontmatter(state);
assert.strictEqual(
fm.current_phase_name,
'Fresh Correct Name',
`body must win over the stale-looking curated name; got ${JSON.stringify(fm.current_phase_name)}`,
);
});
// C4 — identity: cmdStateSync's output is byte-identical to pre-refactor —
// no preservation artifact of any kind reaches it. Proven two ways: (a) the
// command's own JSON report carries no `preservation_warnings` key (the
// field ONLY cmdPhaseComplete/cmdMilestoneComplete now expose), and (b) the
// SAME divergence C1 exercises resolves the SAME way (body wins, nothing
// restored) — proving this phase's refactor did not quietly wire the seam
// in here.
test('C4: cmdStateSync carries no preservation_warnings key and never restores a curated value (unchanged by #3469)', () => {
const content = [
'---',
'gsd_state_version: 1.0',
'stopped_at: "curated stale value — must NOT survive"',
'---',
'',
'# Project State',
'',
'## Session',
'',
'**Stopped at:** fresh body value — must win',
'',
].join('\n');
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), content);
const result = runGsdTools('state sync', tmpDir);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
assert.ok(
!Object.prototype.hasOwnProperty.call(output, 'preservation_warnings'),
'cmdStateSync must not gain the preservation_warnings channel this phase added to phase.complete/milestone.complete',
);
const state = fs.readFileSync(path.join(tmpDir, '.planning', 'STATE.md'), 'utf-8');
const fm = frontmatterLib.extractFrontmatter(state);
assert.strictEqual(fm.stopped_at, 'fresh body value — must win', 'no curated value may be restored');
});
}); });
// ───────────────────────────────────────────────────────────────────────────── // ─────────────────────────────────────────────────────────────────────────────
@@ -9927,6 +10075,105 @@ describe('bug #1230: readModifyWriteStateMd preserves frontmatter status/stopped
}); });
} }
// ─────────────────────────────────────────────────────────────────────────────
// ADR-3408 §8.3 Matrix A1/A2/A3 (#3469): the ONE write-seam composition
// (`syncAndPreserveStateMd`) is now the single owner both `readModifyWriteStateMd`
// and `cmdPhaseComplete`'s atomic-commit adapter call — "the two compositions
// agree because they are one." Required fast-check property, per the matrix's
// closing bullet: for any (content, transform, resync, authoritativeFm),
// cmdPhaseComplete's composed output equals readModifyWriteStateMd's for the
// same inputs. Both sides persist to a REAL file and are compared by reading
// the file back (ADR-3180 Decision 4(c) — the consumer's output, never the
// owner's in-memory return value compared directly). Seed pinned, runs
// bounded, full replay data printed on failure (mirrors tests/state-transition
// .test.cjs's #3468 matrix C3 property).
// ─────────────────────────────────────────────────────────────────────────────
describe('ADR-3408 §8.3 Matrix A1/A2/A3 (property): the shared write-seam composition', () => {
function baseStateMd(phaseNum, phaseName) {
return [
'---',
'gsd_state_version: 1.0',
`current_phase: "${phaseNum}"`,
`current_phase_name: ${phaseName}`,
'status: executing',
'---',
'',
'# Project State',
'',
'## Current Position',
'',
`Phase: ${phaseNum} (${phaseName})`,
'Plan: 1 of 1',
'Status: Executing',
'',
].join('\n');
}
test('property: syncAndPreserveStateMd persists the same bytes whether reached via cmdPhaseComplete\'s shape or readModifyWriteStateMd', (t) => {
// Clock Seam: both paths stamp `last_updated` from `realClock.nowIso()`
// (Date.now() under the hood — src/clock.cts). Without freezing `Date`,
// the two invocations inside each property run happen milliseconds
// apart and `last_updated` genuinely differs, defeating the byte-for-byte
// comparison regardless of the composition itself. Both paths run
// in-process (required directly, not subprocessed), so mocking the
// global `Date` reaches `realClock` without any production change —
// same pattern as tests/commands.test.cjs's HTTP-date pin.
const PINNED_MS = 1_700_000_000_000; // 2023-11-14T22:13:20.000Z
t.mock.timers.enable(['Date']);
t.mock.timers.setTime(PINNED_MS);
fc.assert(
fc.property(
fc.integer({ min: 1, max: 99 }),
fc.constantFrom('Foundation', 'Execution', 'Wrap-up (final)'),
fc.boolean(), // touchPhaseLine — A2 (false, Phase: unchanged) vs A3 (true, Phase: changed)
fc.boolean(), // resync
fc.boolean(), // withAuthoritativeFm
(phaseNum, phaseName, touchPhaseLine, resync, withAuthoritativeFm) => {
const tmp = createTempDir('gsd-a1a2a3-');
t.after(() => cleanup(tmp));
const original = baseStateMd(phaseNum, phaseName);
// Always change SOMETHING (Status) so neither path's own no-op
// guard short-circuits the comparison — the property is about the
// shared COMPOSITION, not the two different no-op-skip policies
// each adapter legitimately layers around it.
let transformed = original.replace(/^Status: Executing$/m, `Status: Executing phase ${phaseNum}`);
if (touchPhaseLine) {
transformed = transformed.replace(/^Phase: .*/m, `Phase: ${phaseNum} (${phaseName}) — COMPLETE`);
}
const authoritativeFm = withAuthoritativeFm ? { current_phase_name: phaseName } : undefined;
// Path A: cmdPhaseComplete's real adapter shape (phase.cts) —
// syncAndPreserveStateMd, then the adapter's own write.
const pathA = path.join(tmp, 'A.md');
fs.writeFileSync(pathA, original);
const composed = stateLib.syncAndPreserveStateMd(original, transformed, pathA, tmp, resync, authoritativeFm);
fs.writeFileSync(pathA, composed);
// Path B: readModifyWriteStateMd's owner shape — same composition,
// reached through the RMW wrapper every OTHER caller uses.
const pathB = path.join(tmp, 'B.md');
fs.writeFileSync(pathB, original);
stateLib.readModifyWriteStateMd(pathB, () => transformed, tmp, { resync, authoritativeFm });
const bytesA = fs.readFileSync(pathA, 'utf8');
const bytesB = fs.readFileSync(pathB, 'utf8');
if (bytesA !== bytesB) {
throw new Error(
`composition diverged: phaseNum=${phaseNum} phaseName=${JSON.stringify(phaseName)} ` +
`touchPhaseLine=${touchPhaseLine} resync=${resync} withAuthoritativeFm=${withAuthoritativeFm}\n` +
`--- pathA (cmdPhaseComplete shape) ---\n${bytesA}\n--- pathB (readModifyWriteStateMd) ---\n${bytesB}`,
);
}
return true;
},
),
{ seed: 3469, numRuns: 50 },
);
});
});
// ──────────────────────────────────────────────────────────────────────── // ────────────────────────────────────────────────────────────────────────
// Folded from tests/bug-948-state-noop-write-guard.test.cjs — consolidation epic #1969 (B2 #1971) // Folded from tests/bug-948-state-noop-write-guard.test.cjs — consolidation epic #1969 (B2 #1971)