* test(#3871): failing-first regressions for the dropped curated progress block Pins ADR-3473 §8.6 / #3756 at the consumer's output: state record-session and state add-decision on an archived-milestone project drop the curated progress frontmatter entirely, exit 0, and report nothing. Reproduced against the real CLI before writing the tests, not inferred from the issue text. Also adds the unit-level probe that applyStatePreservation's preserve-always row is inert on a resyncing write, and an over-preservation guard that an empty project is never inflated. Refs #3871 * feat(#3871): make the STATE.md pre-write snapshot mandatory via open()/rebuild() ADR-3473 §8.6. StatePreservationInput's nullable preFm and the always-present preFmSnapshot were the same extractFrontmatter call, one of them nulled on resync — a policy flag baked into a snapshot. Both collapse into a single StateTransaction whose snapshot cannot be absent: openStateTransaction() applies preservation, rebuildStateTransaction() does not, and both carry the snapshot because the reporting phase needs it either way. An absent snapshot is now a construction failure; an empty one stays legal, because that is what a document with no parseable frontmatter honestly has. writeStateMd requires a rebuild transaction, which types ADR-3408 §8.3's closed exception list at both call sites (state sync, health --repair) instead of matching them as strings in a ratcheted baseline. Fixes the dropped curated progress block: an all-zero or absent derived total set is an unmeasured scan, not a measurement, so the curated block stands. Also fixes two defects surfaced while building — preserve-always reported a mutation even when it restored an identical value, and it re-entered the curated object by reference, which would alias the snapshot the next phase diffs against. Refs #3871 * fix(#3871): close the three remaining subsumed defects and restore the arm the type does not replace Review of the first two commits found four things. The guard shrink deleted the seam-bypass axis whole, but only its writeStateMd( arm became redundant. Its other arm catches a call site re-assembling syncStateFrontmatter + applyPostSyncPreservation instead of the owned composition, which the transaction type does not make unrepresentable and which #3469 found live. Restored as findCompositionBypasses, terminal rather than ratcheted. Three of the four issues this phase claims were untouched. All three are the epic's own shape and are fixed at the seam: current_phase_name is reasserted from the curated value when the caller names none, and cmdStateJson stops carrying a hand-maintained list parallel to FIELD_CLASSIFICATION and projects it instead. The construction failure that is the point of this phase had no test. Every enumerated matrix row now has one, including the measured-versus-unmeasured coercion boundary and a seeded property that no curated key is ever dropped. ADR-3473 §8.6 said the guard 'keeps only its raw-write check'. Verified against next: there was no raw-write check, and four other checks it does not name. Amended in place with the evidence. ARCHITECTURE.md separately advertised a preservation policy the code had deleted. Refs #3871 * fix(#3871): do not let the unmeasured-scan rule block an explicitly-requested resync The remote matrix caught over-preservation, the failure this phase's own negative space says must not happen. state update Progress re-derives the block from the body the caller just rewrote; on a project with no phase dirs the derivation yields zero totals, the unmeasured rule read that as 'the scan measured nothing', and the stale curated percent was restored over the resync the user asked for. preserve-always already said what the missing condition was: never overwrite unless the caller explicitly names this field. explicitProgressField carries it and is derived from shouldResyncStateProgress, not set by hand at a call site, so it cannot drift from what the caller asked for. Two defects found in the same mechanism and fixed with it. readModifyWriteStateMd enumerates its option keys, so a new option was silently dropped rather than rejected. And the raw-write axis captured its first argument up to the first comma, which lands inside a nested path.join, so a write to a STATE.md literal was invisible to it — the prove-it-can-fail test caught that one immediately. No test assertion was weakened; all three frontmatter rows encode #3242, #1969 B3 and #1972 and stand unchanged. Refs #3871 * docs(#3871): record why the raw-write check is kept, not why it was named The amendment justified findRawStateWrites as 'written because §8.6 requires it to exist', which is cargo-culting the contract and would have been the wrong reason to keep anything. The real reason is that writeStateMd acquires the STATE.md lockfile and a raw fs.writeFileSync acquires nothing, so this is a lock bypass and lost-update is the #500/#905/#1230 family — and after this phase it is the one reachable path into the file that nothing else covers. Also records why ADR-3408 §8.6's deletion of the 'clear' policy is not the precedent it looks like: 'clear' was dead vocabulary in a closed enum, this is coverage of a reachable path. Refs #3871 * chore(#3871): backfill changeset PR number --------- Co-authored-by: sim <sim@local>
This commit is contained in:
@@ -551,7 +551,11 @@ Locking decides *who* writes. A separate contract decides *what survives the wri
|
||||
|
||||
STATE.md carries the same fact in two places — YAML frontmatter and the document body — and the body is authoritative. Every write therefore re-derives frontmatter from the body, which raises the question the write path exists to answer: when a re-derived value disagrees with the one already in frontmatter, which wins?
|
||||
|
||||
`FIELD_CLASSIFICATION` (`src/state-transition.cts`) answers it per field, declaring a `preservation` policy — `preserve-when-unchanged`, `preserve-always`, `preserve-if-placeholder`, `derive`, `clear` — that `applyStatePreservation` executes after `syncStateFrontmatter` re-derives.
|
||||
`FIELD_CLASSIFICATION` (`src/state-transition.cts`) answers it per field, declaring a `preservation` policy — `preserve-when-unchanged`, `preserve-always`, `preserve-if-placeholder`, `derive` — that `applyStatePreservation` executes after `syncStateFrontmatter` re-derives. (A fifth policy, `clear`, was listed here until ADR-3408 §8.6's amendment removed it: no row used it and no executor existed for it.)
|
||||
|
||||
**The pipeline's precondition is a type, not a convention (ADR-3473 §8.6).** A policy row can only be honored if the pre-write frontmatter snapshot it compares against is actually present. That snapshot now travels as a `StateTransaction`, built by `openStateTransaction()` — preservation applies — or `rebuildStateTransaction()` — it does not. Both carry the snapshot, and a transaction cannot be constructed without one: an absent snapshot is a *construction failure*, not a runtime skip. That distinction is the whole point. Previously the snapshot was nulled to signal "re-derive from disk", so a declared `preserve-always` row and a silently-skipped one were indistinguishable at runtime, which is how a curated `progress:` block was erased by verbs that had nothing to do with progress.
|
||||
|
||||
`rebuildStateTransaction()` is the typed form of ADR-3408 §8.3's closed exception list: `state sync`, which exists to let the body win, and `/gsd-health --repair`'s factory reset. Both are deliberate and permanent, not debt — and because the type names them, the write-path drift guard no longer has to track them as strings in a ratcheted baseline.
|
||||
|
||||
**[ADR-3408](adr/3408-state-write-path-preservation.md) is the normative contract** for that path: one executor per declared policy, one write seam, and reports computed from what was actually persisted rather than from what the caller intended to write. Where the contract and the code disagree, the code is the defect. It is the write-side counterpart of [ADR-3180](adr/3180-planning-semantic-model-single-owner.md), which gave each read-side derivation a single owner.
|
||||
|
||||
|
||||
@@ -48,7 +48,7 @@ Two `/m`-anchored patterns in the same module use `\s` at a `^` position. Per EC
|
||||
|
||||
This family is the reason Phases 1–3 exist and is recorded here rather than inferred later.
|
||||
|
||||
ADR-3408 shipped completely: all five phases closed, `scripts/lint-state-write-path-drift.cjs` is live, and `scripts/state-write-path-drift-baseline.json` stands at **two entries, both `owner: sanctioned-permanent`** — zero debt. The seam is real and it held.
|
||||
ADR-3408 shipped completely: all five phases closed, `scripts/lint-state-write-path-drift.cjs` is live, and its write-path drift baseline stood at **two entries, both `owner: sanctioned-permanent`** — zero debt. The seam is real and it held. *(That baseline was retired, file and all, by Phase 1 — see §8.6's amendment. This paragraph records the state at filing.)*
|
||||
|
||||
Eleven state defects were nonetheless filed between 2026-08-21 and 2026-08-25 (#3853, #3836, #3835, #3834, #3830, #3818, #3812, #3807, #3784, #3756, #3743). **None is a bypass of that seam.** They sit at four places it does not reach:
|
||||
|
||||
@@ -200,7 +200,15 @@ Both close #3349 and #3360, which are **read-side** defects a real parser fixes
|
||||
|
||||
**Rule — `rebuild()` is the typed expression of the sanctioned exceptions.** ADR-3408 §8.3's closed list of commands whose contract is to let the body win — `cmdStateSync` (#905: `state sync` re-derives frontmatter *from* the body) and `/gsd-health --repair`'s `REGENERATE_STATE` (a factory reset) — call `rebuild()`. They are **not** debt: a guard reporting them is reporting correctly, and a change that removes one is a regression. Adding to the list is an amendment to ADR-3408 §8.3.
|
||||
|
||||
**Consequence for the guard.** `scripts/state-write-path-drift-baseline.json`'s two `sanctioned-permanent` entries are retired: the exception becomes a constructor the type system names, not a ratcheted string match. `scripts/lint-state-write-path-drift.cjs` keeps only its raw-write check (`fs.writeFileSync` against the state path), which the type cannot make unrepresentable.
|
||||
**Consequence for the guard.** The write-path drift baseline's two `sanctioned-permanent` entries are retired, and the baseline file with them: the exception becomes a constructor the type system names, not a ratcheted string match. `scripts/lint-state-write-path-drift.cjs` keeps every check the type does **not** make unrepresentable.
|
||||
|
||||
> **Amendment, 2026-08-25 (Phase 1, #3871) — this paragraph originally read "keeps only its raw-write check (`fs.writeFileSync` against the state path)", and that sentence was wrong on both halves.** Verified against `next` while implementing: the guard contained **no raw-write check at all**, and it contained **four** checks besides the one §8.6 retires — policy-dispatch drift, unimplemented policies, unstripped content writes, and prompt-layer state writes. None of those is named by §8.6 and none is made unrepresentable by the transaction type, so all four are retained; the raw-write check is **net-new**.
|
||||
|
||||
> It is kept — rather than dropped along with the sentence that mis-described it — because of what it covers, not because §8.6 named it. `writeStateMd` acquires the STATE.md lockfile before it writes; a raw `fs.writeFileSync` against the state path acquires nothing. That is not a preservation bypass, it is a **lock** bypass, and lost-update is the #500/#905/#1230 family. Every other route into the file is now closed by construction — an `open()` transaction preserves, a `rebuild()` transaction is the typed exception, and a re-assembled composition is caught by the axis above — so the raw write is the one remaining reachable path that nothing covers. Zero occurrences to date is not the test; reachability and blast radius are.
|
||||
>
|
||||
> The tempting counter-precedent does **not** apply. ADR-3408 §8.6's amendment deleted the `clear` preservation policy when it turned out no row used it and no executor existed — contract names X, X does not exist, delete the naming. `clear` was dead *vocabulary* in a closed enum: deleting it removed a way to express something meaningless. This is coverage of a *reachable path*. The two look alike and are not the same shape.
|
||||
>
|
||||
> The retired axis was the **seam-bypass** scan, and only half of it was redundant. Its `writeStateMd(` arm is genuinely replaced by the type and is gone with its ratchet. Its **composition-bypass** arm — a new call site re-assembling `syncStateFrontmatter` + `applyPostSyncPreservation` instead of routing through the owned composition, which is ADR-3408 §8.3's rule and the exact shape #3469 found live in `cmdPhaseComplete` — is **not** replaced by the type, which gates one parameter of one function and nothing more. It is retained, made terminal rather than ratcheted, and carries its own reason code. Decision 6 sanctions retiring a guard the change makes **redundant**; deleting this arm would have been a silent coverage regression dressed as a guard-count win, which is precisely the Goodhart outcome Decision 6 exists to prevent.
|
||||
|
||||
#### 8.7 What a command reports it wrote — *Required — Phase 2*
|
||||
|
||||
@@ -288,7 +296,7 @@ Net across the set: one guard retired, one increase recorded honestly. The incre
|
||||
|
||||
| Guard | Status under this ADR |
|
||||
|---|---|
|
||||
| `scripts/lint-state-write-path-drift.cjs` | retained, shrunk (§8.6) |
|
||||
| `scripts/lint-state-write-path-drift.cjs` | retained, shrunk (§8.6) — seam-bypass `writeStateMd(` arm and its ratchet retired at Phase 1; composition-bypass arm retained and made terminal; raw-write check added net-new. See §8.6's amendment. |
|
||||
| `scripts/lint-state-field-drift.cjs` | **retired at Phase 3** (§8.8) |
|
||||
| `scripts/lint-vendored-deps.cjs` | reused as-is for §8.1's vendoring rule |
|
||||
| `local/no-external-require-in-bin` | reused as-is; enforces §8.1's packaging rule |
|
||||
|
||||
Reference in New Issue
Block a user