Files

Tom Boucher fa41bfec5c enhance(#3942 ): the emitted-drift ack is PR-lifetime data — move it to a commit trailer (#3954 )

* test(#3942): failing-first suite for the emitted-drift ack commit trailer

Binds 37 input classes from the phase test matrix to the behavior ADR-3942
specifies, before any of it exists. Stubs return benign empty values rather
than throwing, deliberately: several rows assert that something DOES throw
(cap overflow, uncomputable commit range), and a throwing stub would turn
those green for the wrong reason and destroy the red.

The two rows that carry the design's load:

- merge-base semantics. The range is $(git merge-base base HEAD)..HEAD, not
  base..HEAD, because changedPaths comes from `git diff base...HEAD` (three
  dot). Two-dot would let the ack set and the change set disagree about which
  commits are this PR's. The fixture forks a topic branch, puts a trailer on
  each side, and asserts only the topic-side trailer is in range.

- fail-closed on an uncomputable range. With fragments a depth-1 checkout
  passes VACUOUSLY, every fragment reading as brand-new. With trailers the
  range cannot be computed at all, and returning an empty set would silently
  disarm the gate, so it must throw. The fixture builds a genuine shallow
  clone rather than simulating one.

Also covers the self-inflicted case: this change's own documentation quotes
the trailer syntax, so an example landing at the end of a commit message would
arm a live acknowledgment keyed on the literal placeholder text. Keys carrying
angle brackets or whitespace are rejected.

Authored per the phase artifacts 40-design.md and 50-test-matrix.md.
Not yet run on the remote runner — this commit exists to be tested.

Refs #3942

* chore(#3942): move the emitted-drift ack to a commit trailer

Implements ADR-3942, superseding ADR-2719 section 3 and its #2789 amendment.
Sections 1, 2 and 4-7 are retained: the conservation law is unchanged, only the
storage of its escape hatch moved off the working tree.

An acknowledgment explains one PR's ripple, and the moment that PR merges the
ripple is in the base, so it can never clear anything again. It was stored in
permanent shared state anyway, and every consequence of that mismatch had to be
built and then maintained. The chain is #2789 -> #2914 -> #3078 -> #3842 ->
#3823 -> #3875, each fix generating the next defect, ending in a scheduled
sweeper whose own first PR could not merge itself.

Added
  parseAckTrailers + renderAckTrailer (pure) and readAckTrailers (IO shell),
  reading Emitted-Drift-Ack-Hash: / Emitted-Drift-Ack-Growth: trailers over
  the merge-base range. tests/emitted-ack-trailer.test.cjs, 37 cases, written
  failing-first and confirmed red before any of this existed.

Changed
  diffEmitted takes two structurally distinct key-space maps instead of one
  shared paths map. That closes a latent defect: the spaces were separated by
  convention only, so a growth key satisfied a hash lookup by naming
  coincidence. staleAcks now reports which space a key was declared in.
  REMEDIATION teaches the trailer, per space, with its example rendered through
  renderAckTrailer so the taught grammar cannot drift from what the parser
  accepts.

Removed
  the sweep workflow, the guard-no-ack-on-next job, the standalone linter and
  its lint:ci entry, the fragment directory and its three spent fragments, the
  legacy single-file union, and the baseAck/spentAcks mechanism -- spentness is
  now structural, not computed.

Two range properties carry the design and are pinned by tests rather than
asserted: the range is merge-base scoped, matching git diff base...HEAD, so an
already-merged trailer is out of range by construction; and an uncomputable
range throws instead of reading as zero acknowledgments, which is the inverse
of the fragment guard's vacuous pass.

Three deliberate observable changes, each disclosed in the changeset: the
unread runtime field is gone, the legacy file is no longer read, and cross-space
excusal no longer works.

Ten open PRs carry fragments and will meet a modify/delete conflict. Measured
before landing and accepted deliberately; the one-line migration is in the PR
body.

Verified: lint:ci exit 0. Remote runner to follow on this exact sha.

Refs #3942

* fix(#3942): silent trailer collapse, lost coverage, and an unbounded cap

Six findings from the orthogonal review round, all fixed in place.

BLOCKER -- two trailers of the same name on one commit collapsed silently.
readAckTrailers built `separator=1d` where git needs `separator=%x1d`: the
`separator=` value inside a %(trailers:...) placeholder is itself a
pretty-format string, so the bare hex was emitted as two literal characters
and the split on \x1d never matched. Two same-name trailers therefore joined
into one value with errors empty -- the first reason absorbing the second
entry's key. Silent truncation, the exact class MAX_ACK_TRAILERS throws to
prevent. Confirmed with od -c against real git output before and after.

The failing-first matrix did not catch it because its "both spaces coexist"
row uses Hash plus Growth -- different trailer NAMES -- so the value separator
was never exercised. Two regression tests now cover same-name trailers
directly.

Coverage recovered: normalizeAckReason and INVISIBLE stayed on the live path
via parseAckTrailers but lost every test when the old suite was pruned. Back
under test against the current surface -- all six invisible codepoints
individually, whitespace collapse, trim, CRLF, and two seeded fast-check
properties. Dropping any single codepoint now fails.

MAX_ACK_TRAILERS counted raw trailers before de-duplication, so one trailer
carried forward across rebased commits counted once per commit and could throw
on a legitimate branch. Now counts distinct entries; 100 identical repeats
dedupe to one.

diffEmitted validated baseline, current and changedPaths but not the new
ackHash/ackGrowth, so a bad shape raised an unhandled TypeError instead of an
error verdict -- the same defect shape this file documents for #2778.

Docs: CONTRIBUTING and TESTING-SUITES were rewritten only in their first
sections; the later passages still taught fragments, git rm and the deleted
guard, contradicting the new text directly above them. Finished.

Also extends lint-removed-but-needed to exempt docs/adr and docs/research.
That gate fails on any docs mention of a file deleted in the same diff, which
makes it impossible to document a deletion in the PR performing it -- an ADR's
whole job is naming what it retired. Exemption is narrow and comes with a test
proving the gate still fires for a live consumer elsewhere under docs/. A
guard that cannot fail is worse than no guard. Maintainer-approved.

CONTEXT.md names the retired machinery by role rather than by filename: its
generated projection lands in docs/, which that gate does scan.

Adds docs/how-to/acknowledge-emitted-drift.md. The required docs set is
Reference and Explanation, so the task quadrant can be empty with every gate
green -- and this change has a real multi-step journey, including the fragment
migration ten open PRs now need.

lint:ci exit 0.

Refs #3942

* docs(#3942): correct the duplicate-trailer rule in CONTRIBUTING

Both axes of the code review independently flagged the same passage, without
seeing each other's output.

It claimed two declarations of the same key are always "a hard, loudly-reported
error, not a silent last-wins". That is only half true, and the missing half is
the one contributors hit: identical declarations -- same key, same reason --
dedupe silently, because a trailer legitimately survives a rebase and reappears
on every rebased commit. Failing there would red a branch for doing nothing
wrong, which is exactly why the dedup exists.

Only a same-key/different-reason pair errors, and that one is a genuine
ambiguity about which explanation holds.

As written, the paragraph told a contributor that a rebase-carried trailer
breaks the gate -- the opposite of the behavior. CONTEXT.md's parallel entry
already stated it correctly; this brings CONTRIBUTING into line.

Doc-only, root-level markdown.

Refs #3942

* chore(#3942): backfill changeset PR number to 3954

---------

Co-authored-by: sim <sim@local>

2026-08-27 17:28:39 -04:00

17 KiB

Raw Blame History

ADR-2719: Emitted-artifact attribution — replace the committed parity fixtures with a computed conservation law

Status: Accepted; Decision §3 superseded by ADR-3942 (The emitted-drift acknowledgment is PR-lifetime data — it belongs in a commit trailer) (2026-08-27), which replaces §3 and its #2789 Amendment. §1, §2 and §4–§7 are retained and depended upon — the conservation law itself is unchanged; only the storage of its escape hatch moved off the working tree.
Date: 2026-07-27
Issue: #2719 (epic); Phase 0 tracked by #2720
Supersedes: ADR-2264 (Redesign golden-install-parity) — replaces its Decision §2–§4 and its 2026-07-14 Amendment. ADR-2264 Phase 1 is retained and depended upon: the single-source buildParityManifest and the four exclusion constants in tests/helpers/install-shared.cjs are the foundation this design builds on, not something being reverted.
Relationship to prior work: Evolves the ADR-1239 Phase-B installer byte-parity harness. Related: ADR-3660 / ADR-1508 (Runtime Artifact Layout / Conversion / Install Plan), #2086, #2100, #2117, #2266, #2267, #2268.

Context

Three families of committed artifacts are pure functions of the source tree: tests/fixtures/golden-install-parity/*.json (19 path→hash manifests), tests/workflow-size-baseline.json, and tests/agent-size-baseline.json. Every edit to shipped content requires regenerating them by hand.

Measured against origin/next @ 1c93df04 with git merge-tree:

7 of 7 conflicting open PRs collide on the identical 20-file set.
140 of 143 conflicted-file instances across those PRs are these artifacts. Three other conflicts exist in total.
16 of 32 open PRs touch these paths at all.

The conflicts split into two mechanisms, and neither is what a reviewer would guess.

Adjacency (3 of 7). #2662, #2531 and #2301 change manifest keys entirely disjoint from what next changed — zero overlap. They conflict only because the map is sorted and unrelated keys fall inside git's three-line diff context. The union of the two edits is the correct merge. These are false conflicts.

Same key (4 of 7). Both sides re-hashed the same emitted file. Git offers ours or theirs; both are wrong, because the correct value is neither — it is recomputed from the merged tree. Git's merge interface cannot express the resolution this artifact always needs.

Amplification makes it worse: content under gsd-core/workflows/, references/, templates/ and contexts/ is copied to every host, so one source edit rewrites the same hash in all 19 manifests. The conflict surface is 19× the change. 281 of claude's 440 emitted paths (64%) are plain identity copies.

Why the current design cannot be patched into shape

The premise that this is an absolute invariant does not survive inspection. When a contributor changes emitted bytes they run npm run gen:golden, and the anchor is overwritten. Every PR re-blesses the snapshot. What makes it feel absolute is manual friction, not a fixed reference point — the golden is already a relative check wearing an absolute costume.

Its stated purpose is detecting a change that propagated further than the author intended. That detection path currently runs entirely through a human noticing an anomaly in 7,500 lines of hex:

The fixtures carry no linguist-generated marker, so GitHub renders them expanded.
.github/CODEOWNERS:1-2 is advisory — required_approving_review_count: 0.
.github/PULL_REQUEST_TEMPLATE.md never mentions fixtures, baselines, or regeneration.
CONTEXT.md has no entry for this artifact family under any name, so a contributor who sees "conflicts" has nothing to look up.
No single command regenerates it. npm run build covers five of eleven generators; gen:golden, size:baseline, gen-inventory-manifest, gen:registry, gen-adr-index and gen-capability-matrix each need separate invocation.

ADR-2264 diagnosed the churn correctly and shipped Phase 1 cleanly. Its Amendment then fixed silent staleness and correctly rejected the copy/transform split on measurement — but it walked back the churn fix without replacing it. That ADR's own 2026-07-17 audit records AC1, "editing the content of a verbatim/path-injected copied shipped file requires zero manual fixture regeneration," as literally unmet. This ADR satisfies AC1 rather than rewording it away.

Decision

Stop committing the derived state. Replace it with a conservation law, checked on every PR.

1. The invariant is relative, and stated as attribution

Build the parity manifest at next HEAD and at PR HEAD. Every emitted path whose hash moved must be attributable — through a declarative provenance table — to a path the pull request actually changed. Unattributable deltas are a hard failure that names them:

39 emitted paths changed that nothing in this diff explains:
  gsd-core/workflows/execute-phase.md
  agents/gsd-planner.md
  ...

This is strictly stronger than the current mechanism for the failure it exists to catch. "You touched one thing and it rippled" stops being an anomaly a reviewer must notice and becomes a computed statement.

This is not the §3 property test ADR-2264's Amendment rejected. That design asserted emitted == transform(source) by calling the installer's own transform, which is tautological. This design never re-derives a byte; it constrains which keys are permitted to move.

2. Provenance comes from a declarative table with a totality guard

The mapping surface is small and stable. Per host, 13 families; of those, roughly nine are identity or prefix rewrites, one is a stem rewrite (skills/ ← commands/gsd/*.md), and four are synthesized files with no repo source (.gsd-profile, package.json, VERSION, .gsd-runtime) that get their own exempt category. Roughly 15–20 rules total, changing only when a new shipped family or host appears.

Totality is enforced. Any emitted path matching no rule is a hard failure, not a skipped entry. A hand-maintained table's characteristic risk is becoming a silent gap; totality converts that into a loud one. If the installer starts emitting something new, the build fails rather than passing it through unattributed.

Extending the Runtime Artifact Install Plan module to emit file-level provenance is the architecturally correct long-term home and is deliberately not a prerequisite. PlanItem is { kind, sourceDir, destDir } — directory-granular, covering only commands, agents, skills and kimi-agents, so the 281 bulk copies bypass it entirely. Making it a prerequisite turns one epic into two. It remains available later as a refactor with behavior already pinned.

3. The escape hatch is a committed acknowledgment, not a flag

Legitimate unattributable deltas exist: change a converter and emitted bytes move for files whose sources nobody touched. That is ADR-2264's "~5% git cannot review," and it must have a way through.

The way through is tests/emitted-drift-ack.json, which names the affected paths and states why. It is deliberately not an environment variable or a CLI flag — a contributor facing a red gate sets a flag, which is what UPDATE_GOLDEN=1 is today.

The design property that matters: the acknowledgment file appears in the changed-files list only when something rippled unexpectedly. Today 100% of emitted-byte changes touch fixtures, so touching them signals nothing. Under this design, touching the acknowledgment is the alarm.

This does not make a bad change impossible. It converts a silent regeneration into a conspicuous declaration. That is the intended strength, stated plainly rather than overclaimed.

Amendment (#2789): touching the acknowledgment is still the alarm, and is now strictly harder to fake. Mere presence is not, and treating it as such was a real defect. The document was read only from the working tree while every other input to the law is base-relative, so staleAcks — "acks no delta consumed" — could not tell an ack that never explained anything from one whose ripple had been absorbed into the base, which is the ack's success condition. Merging an acknowledgment therefore reddened next and every PR branching off it (#2768).

An ack is now scoped to the diff that introduced it: diffEmitted also takes the document at the base ref, and an entry already present there is spent — it can no longer consume a delta and is never reported stale, only listed for tidying. New or reworded entries stay live, and re-arming costs actual prose (internal whitespace, invisible characters and the unread runtime field are all normalized away), so the conspicuous declaration this section asks for cannot be forged with a zero-information edit. That also closes a hazard the implementation named but could not prevent — a leftover ack silently pre-clearing the next ripple on its path; note this ADR's own residual-risk list never covered it. scripts/lint-emitted-drift-ack.cjs refuses the merge if a malformed document would reach the base, where the base-side reader's deliberate hard failure is expensive.

4. The size ratchet folds into the same machine

workflow-size-baseline.json conflicts on 7 of 7 — deleting only the golden fixtures would leave every affected PR still blocked. The attribution law does not transfer to it (growth is trivially attributable to the edit that caused it), so instead the same differential machine reports growth with exact byte deltas, and growth requires the same acknowledgment entry.

The committed number was only ever a means to "growth must be noticed and justified." That function survives, stated more legibly — verify-work.md grew 1,247 bytes beats a number changing inside a 93-line map — while pinning nothing and conflicting never.

Bucketing the sizes was rejected: rounding to the nearest 2KB lets a PR add 1.9KB invisibly, and invisible growth is exactly what the ratchet defends against.

5. The baseline is cached, not committed

The push-to-next run publishes the manifest and size maps; the PR lane restores them from cache, keyed on the next sha the PR was merged with. That key discipline is load-bearing — a stale baseline mis-attributes silently. Cache miss falls back to an in-job build at origin/next.

Bot-committing the baseline to next post-merge was considered and rejected. It is close to ADR-2264 Phase 3 (#2268), and notably that ADR's objection does not apply here: the Amendment deferred CI-owned regen because it "needs a write-token workflow running on PR code (an injection surface)," whereas committing post-merge runs already-reviewed code. It was rejected on a different cost — next carries strict: true, so a bot commit after every emitted-byte-changing merge doubles the rate next advances and taxes all ~30 open PRs. Its one unique benefit is an absolute anchor, which §1 establishes is not required.

6. It is a test, not a CI job

The check is a node:test file in the unit suite. Required tests already gates next, so it is enforced the day it lands; a separate job would need an eighth required context, and adding one under strict: true invalidates the status of every open PR at once — a self-inflicted instance of the problem being solved.

A baseline-unavailable path must never be a bare return. In node:test that is a pass, and it would make the gate fail open with nothing in CI to say so.

7. `install-tree` stays committed

tests/fixtures/install-tree/*.json conflicts on 0 of 7. The file set genuinely changes rarely, its diffs are readable, and keeping it preserves "the installer stopped shipping X" as a hard, absolute failure with no attribution reasoning involved. It is the one artifact in this family already behaving correctly.

Phases

Phase 0 — this ADR (#2720, docs-only).
Phase 1 — interim relief (#2721). merge=gsd-regen driver, npm run regen:derived, and naming: RULESET.EMITTED_ATTRIBUTION= under ## Test rules and lint plus ### Emitted Artifact Provenance in the glossary. Touches no fixtures, so it breaks none of the 16 exposed PRs.
Phase 2 — provenance table + totality guard (#2722).
Phase 3 — differential check, dual-run (#2723). Runs beside golden-install-parity.test.cjs, both green, fixtures untouched.
Phase 4 — cutover (#2724). Delete the fixtures, generators and bridge driver; flip this ADR to Accepted.

Acceptance criteria (must-haves)

Editing the content of a copied shipped file (for example a gsd-core/workflows/*.md) requires zero manual fixture regeneration and the parity gate still passes. (This is ADR-2264 AC1, satisfied rather than reworded.)
A simulated ripple — edit one source file, corrupt an unrelated emitted file — fails with the unattributable paths named.
A simulated converter change fails without an acknowledgment entry and passes with one.
Every emitted path across all 19 runtime manifests matches exactly one provenance rule; removing a rule fails the totality guard naming the unmatched paths.
A stale cache key is detected rather than silently used as the baseline.
Growth in a workflow or agent file is reported with its exact byte delta and requires an acknowledgment entry.
After Phase 4, no committed path→hash manifest or per-file size baseline remains, and the two checks agreed throughout the Phase 3 dual-run window.

Consequences

Positive. The conflict class ends rather than being automated around: 140 of 143 conflicted-file instances disappear. The propagation catch becomes a computed statement instead of a reviewer noticing an anomaly in hex. ~520 KB of committed derived state is deleted, along with the duplicate generator, UPDATE_GOLDEN, and npm run gen:golden. The artifact family finally has a name in CONTEXT.md.
Negative — one-time migration. Deleting the fixtures converts existing conflicts into delete/modify conflicts on the same 20 files, affecting 16 PRs. Verified by simulating the deletion with git commit-tree and re-running git merge-tree. The resolution starts with one identical command per PR — git rm tests/fixtures/golden-install-parity/*.json tests/workflow-size-baseline.json tests/agent-size-baseline.json — and for a PR that changes no shipped file's size, that is the whole of it. It is not terminal for a PR that grows a gsd-core/workflows/*.md or agents/gsd-*.md file, which is the common case for a feature change: those need a second step, creating tests/emitted-drift-ack.json with an entry keyed on the bare filename (§3, §4). This ADR originally described the migration as terminal, full stop; that claim was corrected by #2778 after the Phase 4 cutover met it in the field on #2543. The failure output now states the second step itself, so the correction is discoverable where the contributor actually is rather than only here. Either way the cost is bounded and one-time; today's is regenerate-rebase-repush, recurring on every merge to next.
Negative — new residual risks, accepted. A provenance rule can map to the wrong source and still pass the totality guard: false attribution, not a false alarm. The Phase 3 dual-run window exists to surface exactly this, and spot-check tests on known pairs reduce it further. Separately, the relative chain holds only while the check runs on every merging PR — that is CI configuration rather than design, and if the selection rules narrow later it breaks silently.
Neutral. During Phase 1–3, github.com still reports CONFLICTING. Merge drivers live in .git/config, so forks lack them and GitHub's own merge never runs them. The driver removes the labour, not the label, and is retired in Phase 4.

References

tests/helpers/install-shared.cjs — buildParityManifest and the exclusion constants (ADR-2264 Phase 1, retained)
tests/golden-install-parity.test.cjs, scripts/gen-golden-install-parity-zcode.cjs — removed in Phase 4
src/runtime-artifact-install-plan.cts — createRuntimeArtifactInstallPlan; the long-term provenance home
scripts/ci-test-scope.cjs:148-178 — the anti-staleness selection rule from ADR-2264 Phase 2
scripts/ci-rebase-check.cjs:108 — merges base into HEAD before tests, which is why the baseline is next HEAD rather than the merge-base
scripts/lib/allowlist-ratchet.cjs:157-230 — assertFileBaseline, the size ratchet being replaced
ADR-1239 (Phase-B safety net), ADR-2264 (superseded by this ADR), ADR-3660 / ADR-1508 (Runtime Artifact family)

17 KiB Raw Blame History Unescape Escape