5 Commits

Author SHA1 Message Date
Jakub Zych
a9a7a328e6 refactor: hard-fork GSD -> MSD (Make Software Done)
Mechanical rename produced by scripts/msd-rename.cjs: gsd/Gsd/GSD -> msd/Msd/MSD
across contents and paths, upstream package/repo coordinates -> @golem15/msd-core
and golem15com/msd-core. Deep links into upstream history, sibling upstream
packages, the GSD-2 import feature, CHANGELOG.md and .changeset/ are kept as-is.

Hand edits on top: MSD block-letter banner and logos, LICENSE copyright line,
package/plugin identity, regenerated lockfile, install-tree fixtures, derived
registries and benchmark baseline; migration checksum baseline re-locked
(MSD keeps its own install state, so no install had applied the old sums);
sort-order and regex-escaped expectations in tests adjusted.
2026-10-06 01:47:40 +02:00
Rezolv
85026f6a05 feat(#4668): add StateWriteIntent type surface and opaque-transform guard recognition (ADR-4629 C1) (#4676)
* feat(#4668): add StateWriteIntent type surface and opaque-transform guard recognition (ADR-4629 C1)

Child C1 of epic #4629 — migration-order step (1) of ADR-4629: the guard/type
scaffolding, with NO behavior change and no caller migrated.

1. StateWriteIntent (src/state-transition.cts) extends StateTransaction with the
   ADR-4629 section 8.1 concepts: field/section assertions marked required vs
   best-effort, plus a declared mutation scope (narrow | broad). Frozen like its
   base. createStateWriteIntent builds one from an existing transaction. Nothing
   in production constructs it yet — section 8.1's caller-side rule is Required in
   Phase 2; C2 (the verifying executor) and C3+ (caller migration) consume it.

2. findOpaqueStateTransforms (scripts/lint-state-write-path-drift.cjs) recognizes
   a residual readModifyWriteStateMd(path, (content) => ...) write whose transform
   is an inline anonymous arrow/function — the opaque shape section 8.1 replaces
   with a declared StateWriteIntent. readModifyWriteStateMd goes THROUGH the seam
   (it is not a raw-write bypass, Axis 2's concern), but its opaque body transform
   is neither verified (section 8.2) nor bounded (section 8.3).

   This ships recognition as a CAPABILITY: exported and unit-tested (positive
   control on a seeded fixture) but DELIBERATELY NOT wired into collect()'s
   failing scan. Wiring it now would turn the ~16 residual callers red at once,
   and ADR-3473 section 8.6 retired the ratchet that would otherwise absorb them.
   C2 wires it terminal as the verifying executor lands and callers migrate under
   ADR-3408 section 6 phasing.

No behavior change: the guard is green on the tree (detection not wired), every
state verb's output is unchanged, and the relevant suites (1763 tests) plus
lint:ci pass. Regression tests are failing-first: positive/negative controls for
the guard capability and a shape test for the type, plus a pin that collect() has
no opaque-transform findings (C1 must not enforce; that is C2).

Closes #4668

* chore(#4668): backfill changeset pr field to the real PR number (#4676)

pr: 0 is rejected by parseFragment as invalid_pr (it is not a valid placeholder);
the fragment must carry the real PR number, which fixes both changeset-lint and
docs-lint (fail_invalid_fragment / fail_malformed_fragment).

---------

Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
2026-09-14 21:43:49 -04:00
Tom Boucher
1863f5569c enhance(#3871): the state transaction — mandatory snapshot, open()/rebuild() (#3874)
* 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>
2026-08-25 20:38:12 -04:00
Tom Boucher
e2f4c16d9e 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>
2026-08-14 16:04:09 -04:00
Tom Boucher
1218d76d62 refactor(#3468): dispatch state preservation on the declared policy, not the field (#3495)
* test(#3468): add write-path drift guard, ratcheted at its measured baseline

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

Measured, not assumed:

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

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

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

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

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

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

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

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

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

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

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

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

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

Implements ADR-3408 sections 8.1, 8.2 and 8.6.

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

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

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

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

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

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

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

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

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

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

Also adds the changeset fragment.

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

Standards axis, both hard violations:

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

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

Contract tension, resolved rather than left:

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

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

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

Isolated security review, MEDIUM, confidence 8/10.

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

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

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

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

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

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

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

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

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

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

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

  actual   'current_phase_name'
  expected 'current_plan'

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

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

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

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

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

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

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

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

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

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

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

---------

Co-authored-by: sim <sim@local>
2026-08-14 13:25:30 -04:00