From 34399eed7093128d842e05d7a5635472bb450d42 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 27 Aug 2026 14:06:29 -0400 Subject: [PATCH] =?UTF-8?q?docs(#3942):=20ADR=20=E2=80=94=20the=20emitted-?= =?UTF-8?q?drift=20ack=20belongs=20in=20a=20commit=20trailer=20(#3943)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The acknowledgment explains one PR's unattributable emitted-artifact delta. The moment that PR merges the delta is in the base, so the acknowledgment can never clear anything again — its useful life is exactly the PR's open window. It is stored in permanent, shared, merge-path state. That single lifetime mismatch is the origin of six consecutive rounds of defect-and-fix (#2789, #2914, #3078, #3823, #3842, #3875), each fix generating the next defect, ending in a scheduled sweeper whose own PR (#3927) could not merge itself. ADR-3942 supersedes ADR-2719 section 3 only, and its #2789 Amendment. Sections 1, 2 and 4-7 are retained and depended upon: the conservation law itself is not in question, only where its escape hatch is stored. Also lands the root-cause research note behind the decision, and regenerates the ADR index. Two corrections to earlier framing, recorded here rather than dropped: - Per-commit trailers do NOT reliably survive squash-merge; the repo's own ship.md:306 reconstructs a gate trail precisely because squash discards it. The ADR therefore claims no durable audit record on next. Non-survival is the property being bought, not a cost. - The test job checks out at depth 1 (test.yml:107-110, no fetch-depth key). A depth-1 checkout cannot see the PR's commit range and fails VACUOUSLY rather than erroring, so fetch-depth: 0 is a named decision in the ADR and the acceptance criterion requires a test that fails under depth 1. Docs-only; no code, no changeset. Refs #3942 Co-authored-by: sim --- .../3942-emitted-drift-ack-commit-trailer.md | 122 +++++++++++++++++ docs/adr/README.md | 1 + .../3875-ack-sweep-automation-failure.md | 123 ++++++++++++++++++ 3 files changed, 246 insertions(+) create mode 100644 docs/adr/3942-emitted-drift-ack-commit-trailer.md create mode 100644 docs/research/3875-ack-sweep-automation-failure.md diff --git a/docs/adr/3942-emitted-drift-ack-commit-trailer.md b/docs/adr/3942-emitted-drift-ack-commit-trailer.md new file mode 100644 index 000000000..bb1435822 --- /dev/null +++ b/docs/adr/3942-emitted-drift-ack-commit-trailer.md @@ -0,0 +1,122 @@ +# ADR-3942: The emitted-drift acknowledgment is PR-lifetime data — it belongs in a commit trailer, not the working tree + +| | | +|---|---| +| **Status** | Proposed | +| **Date** | 2026-08-27 | +| **Issue** | [#3942](https://github.com/open-gsd/gsd-core/issues/3942) | +| **Supersedes** | [ADR-2719](2719-emitted-artifact-attribution.md) **§3 only** ("The escape hatch is a committed acknowledgment, not a flag"), including its #2789 Amendment. §1, §2, §4–§7 are retained and depended upon. | +| **Amends** | — | +| **Constrained by** | [ADR-2719](2719-emitted-artifact-attribution.md) §1 (the invariant is relative, stated as attribution), §4 (the size ratchet folds into the same machine), §6 (it is a test, not a CI job) | + +> **Evidence note.** Line citations below were read from the worktree at `origin/next` `11cdc19e3`. CI verdicts are from the live GitHub API for [PR #3927](https://github.com/open-gsd/gsd-core/pull/3927), not from re-derivation. The full root-cause trace is `docs/research/3875-ack-sweep-automation-failure.md`. + +## Context + +ADR-2719 §3 established that the attribution law needs an escape hatch, and chose a committed document for it. That choice was right about the *shape* of the escape hatch — a prose declaration, not a flag — and wrong about its *storage*. This ADR changes only the storage. + +### The acknowledgment's lifetime does not match its storage + +An acknowledgment explains one PR's unattributable delta. The moment that PR merges, the delta it explained is in the base, and the acknowledgment can never clear anything again. The directory's own README says so at `tests/emitted-drift-acks/README.md:6`: + +> This directory being empty is the healthy steady state. A fragment appearing in a diff *is* the alarm; a fragment sitting here on `next` is spent cruft. + +So the data has PR lifetime and is stored in permanent, shared, merge-path state. **Every defect in this family descends from that one mismatch**, and each fix has generated the next: + +| # | Fix | Defect it created | +|---|---|---| +| — | Single `tests/emitted-drift-ack.json` | One shared mutable file per PR. "5 of 6 conflicting PRs in one open queue collided on this file and nothing else" (`CONTRIBUTING.md:1086`) | +| #2914 | Split into per-PR fragments, modeled on `.changeset/` | Fragments do not share a *file*, but they do share a *path-key namespace* | +| #3078 | Guard reds `next` on spent fragments | 45 fragments owning 403 paths; each spent fragment walls off the next PR that grows one of its keys | +| #3842 | Sweep spent fragments | Handed three in-flight external PRs (#3330, #3774, #3648) a `modify/delete` conflict each; needed `--defer-to-open-prs` | +| #3823 | Hand-authored sweep | Computed at branch time, guard evaluates at merge time; lost the race to #3809 and left `next` red for **24 consecutive pushes** | +| #3875 | Timed sweeper (`ack-fragment-sweep.yml`) | Its own PR cannot merge itself (below) | + +### Why `.changeset/` was the wrong analogy + +#2914 reasoned that independently-named fragments cannot conflict, by analogy to `.changeset/`. Changeset fragments are genuinely independent: two of them never name the same entity. Ack fragments key into a shared namespace, and two sources declaring the same key is a hard duplicate-key error (`scripts/lint-emitted-drift-ack.cjs:872-885`). The analogy held at the filesystem layer and failed at the semantic layer. #3078 measured the cost. + +### The automation could not close the loop + +[PR #3927](https://github.com/open-gsd/gsd-core/pull/3927), the sweeper's first production run, was merged by hand at 12:16Z — 2h38m after opening — **with `validate-title` and `Required tests` still red**. Three independent, deterministic defects, none of them flaky: + +1. `.github/workflows/ack-fragment-sweep.yml:266` hardcodes `chore: sweep spent ack fragments from next (${SHORT_SHA})`. `scripts/release-notes/conventional-title.cjs:28,92` requires `(#)` immediately after the type. The parenthetical is at the end (never parsed as scope) and a git sha contains no `#`. Fails on every run. +2. The sweep's diff is, by construction, deletions under `tests/emitted-drift-acks/`. No rule in `scripts/ci-test-scope.cjs` matches that path, so `classify()` falls through the #408 fallback to the `'unit'` suite sentinel on the **unsharded, 15-minute-capped** lane. Run log: `suite="all" files=827`, killed at chunk 13/14, 15m18s against `timeout-minutes: 15` (`.github/workflows/test.yml:155`). +3. No auto-merge path exists. The workflow ends at `gh pr create` plus labels. + +These are fixable in isolation. They are listed here not as the problem but as evidence of its shape: **a garbage collector that needs its own CI lane, its own title convention, and its own merge story is a large amount of machinery to hold up an artifact whose correct steady state is "absent."** + +### A latent defect in the current design + +The two key spaces are convention-only. A hash ripple keys on the emitted path (`ackEntries.has(rel)`, `tests/helpers/emitted-diff.cjs:597`); growth keys on a bare filename (`ackEntries.has(name)`, `:632`). Both read the same `paths` map with no schema difference — `tests/emitted-drift-acks/README.md:20-23` documents the split, nothing enforces it. A key intended for one space silently satisfies a lookup in the other. + +## Decision + +### 1. The acknowledgment moves to a commit trailer on the PR's own commits + + Emitted-Drift-Ack-Hash: — + Emitted-Drift-Ack-Growth: — + +Read from `git log ..` — the PR's own commits and no others. + +This preserves what ADR-2719 §3 actually cared about. Its stated design property is *"the acknowledgment file appears in the changed-files list **only when something rippled unexpectedly** … touching the acknowledgment *is* the alarm."* A trailer is still a conspicuous, reviewable, prose-carrying declaration that appears in the PR's diff — it is not the `UPDATE_GOLDEN=1` flag §3 rejected. What changes is that the declaration stops outliving the thing it declares. + +### 2. "Spent" stops existing + +The #2789 Amendment built spent-detection because the document persisted at the base, so `staleAcks` could not distinguish "never explained anything" from "its ripple was absorbed into the base." Scoping the trailer to `base..head` makes that distinction structural rather than computed: a trailer in the PR's commit range is by definition this PR's, and there is no base-side copy to compare against. + +This is a strictly stronger form of what #2789 wanted. `readAckFileAtRef`, `listAckFragmentFilesAtRef`, `readAckSourcesAtRef`, the spent/re-arm prose normalization, and `assertNoAllSpentFragments` all become unreachable. + +`staleAcks` itself is **retained** — a trailer declaring a key that no delta consumed is still an error (`tests/helpers/emitted-diff.cjs:648`). That check is per-PR and does not depend on persistence. + +### 3. The two key spaces become structurally distinct + +Two trailer keys instead of one map. A growth acknowledgment can no longer satisfy a hash lookup by coincidence of naming. This closes the latent defect above rather than carrying it forward. + +### 4. The pure law does not change + +`diffEmitted` already receives `ackEntries` as a plain `Map`. The storage medium lives entirely behind the IO shell in `tests/helpers/emitted-runtime.cjs`. Replacing `readAckSources` with a trailer reader is an adapter swap; `tests/helpers/emitted-diff.cjs` — the law — is untouched apart from the key-space split in §3. + +There is in-repo precedent for the mechanism: `gsd-core/workflows/ship.md:312` already parses a `gate_status:` trailer with `git log --format='...%(trailers:key=gate_status,valueonly,separator=%x2c)...'`. + +### 5. The PR test lane must fetch the commit range + +`.github/workflows/test.yml:107-110` — the `test` job — has no `fetch-depth` key and therefore checks out at depth 1. A depth-1 checkout cannot see the PR's commit range, and the failure mode is a **vacuous pass**, not an error. `test.yml:882-885` already documents this exact hazard for the `guard-no-ack-on-next` job. + +`fetch-depth: 0` is required on that job, and the gate must fail closed when the range is unavailable — never `return` on a missing base, per ADR-2719 §6 ("A baseline-unavailable path must never be a bare `return`. In `node:test` that is a **pass**"). + +### 6. What gets deleted + +- `.github/workflows/ack-fragment-sweep.yml` (237 lines) +- `guard-no-ack-on-next` (`.github/workflows/test.yml:865-920`) +- `scripts/lint-emitted-drift-ack.cjs` (937 lines) and its `package.json:124` `lint:ci` invocation +- `tests/emitted-drift-acks/` and its README +- The at-ref/spent halves of `tests/helpers/emitted-runtime.cjs` + +## Consequences + +**The conflict surface goes to zero.** Not "smaller" — the acknowledgment stops being a tree object, so it cannot conflict on a file, a key namespace, or a modify/delete. `--defer-to-open-prs` becomes unnecessary rather than merely correct. + +**`next` can no longer be reddened by paperwork.** The guard that reds it is deleted along with the state it guards. + +**The acknowledgment does not survive to `next`, and that is the point.** The repo allows squash, merge, and rebase (`allow_squash_merge`, `allow_merge_commit`, `allow_rebase_merge` all true; `squash_merge_commit_message: COMMIT_MESSAGES`), and `gsd-core/workflows/ship.md:306` treats per-commit trailers as not reliably surviving squash-merge. Under `COMMIT_MESSAGES` the text does concatenate into the squash body, but that is a mutable repo setting and a merger can edit the body, so **this ADR claims no durable audit record on `next`**. The acknowledgment is read during the PR, which is the only window in which it is meaningful. An earlier framing of this design claimed trailers were "permanent and auditable, same as today"; that claim was wrong and is withdrawn here rather than shipped. + +**Amending an acknowledgment means amending a commit.** Editing a file is cheaper than rewriting history. This is a real ergonomic cost. It is also a correctness property: the acknowledgment cannot drift out of sync with the diff it explains, because changing either changes the sha and re-runs the gate. + +**Review ergonomics change.** A reviewer reads the acknowledgment in the commit message rather than in a file diff. GitHub renders commit messages in the Commits tab, not inline in the Files tab — less prominent than a changed file. Mitigation: the gate's failure output already names its own remedy (`CONTRIBUTING.md:1074-1076`), and the trailer text appears in the PR's own commit list. + +**Local runs work with no network.** `git log base..head` needs no API call, unlike a PR-label or PR-body scheme. This is why label-based and body-based designs were rejected: both are mutable after CI has run, both need an authenticated API call from the test, and a label is coarser than per-key prose — a blanket "excuse this PR" lets a genuine regression ride along. + +## Revisit if + +- Squash-merge stops preserving commit bodies **and** a durable on-`next` audit trail of acknowledgments turns out to be needed for something concrete. Nothing consumes one today. +- The trailer key space needs more than two members, which would suggest the attribution table (ADR-2719 §2) has a gap the escape hatch is absorbing. +- `fetch-depth: 0` on the `test` job measurably slows the PR lane. + +## References + +- ADR-2719 §3 and its #2789 Amendment — the design this supersedes +- #2789, #2914, #3078, #3823, #3842, #3875 — the six prior rounds +- [PR #3927](https://github.com/open-gsd/gsd-core/pull/3927) — the sweeper's first production run +- `docs/research/3875-ack-sweep-automation-failure.md` — full root-cause trace with line citations +- `gsd-core/workflows/ship.md:306-348` — in-repo precedent for trailer parsing diff --git a/docs/adr/README.md b/docs/adr/README.md index ff0c1fd9e..b0e31eb81 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -276,6 +276,7 @@ Decided in principle, not yet ratified. Do not cite as settled architecture. | [ADR-1953](1953-complexity-triggered-refactor.md) | Complexity-triggered refactor — the loop measures the entropy it just added | Proposed | — | | [ADR-3128](3128-adaptive-runtime-evidence.md) | Adaptive runtime evidence for GSD Debug | Proposed | — | | [ADR-3889](3889-process-exit-contract.md) | One exit-code registry — 0 and 1 are free, everything else is allocated | Proposed | — | +| [ADR-3942](3942-emitted-drift-ack-commit-trailer.md) | The emitted-drift acknowledgment is PR-lifetime data — it belongs in a commit trailer, not the working tree | Proposed | — | ### Superseded, Retired, and Legacy diff --git a/docs/research/3875-ack-sweep-automation-failure.md b/docs/research/3875-ack-sweep-automation-failure.md new file mode 100644 index 000000000..bd5d94e1c --- /dev/null +++ b/docs/research/3875-ack-sweep-automation-failure.md @@ -0,0 +1,123 @@ +# Why the ack-fragment sweep (#3875) is not "fully automatic": PR #3927 case study + +**Status:** research note — grounded in the repo's own source, workflow YAML, ADRs, and live `gh`/GitHub API data for [PR #3927](https://github.com/open-gsd/gsd-core/pull/3927), pulled 2026-08-27. + +## Summary + +`ack-fragment-sweep.yml` (cron `30 */6 * * *`) correctly *detects* and *plans* the deletion of spent `tests/emitted-drift-acks/*.json` fragments, and correctly *opens* a PR. It does not, and structurally cannot, get that PR merged unattended: + +1. Its hardcoded PR title (`ack-fragment-sweep.yml:266`) never satisfies the repo's own title-validator regex (`scripts/release-notes/conventional-title.cjs:28,84-93`) — a self-inflicted, deterministic failure on every run. +2. Its freeform PR body fails the typed-template check, but that check only **warns** (not fails) because the sweep authenticates as a `MEMBER` account, not because the body is compliant (`scripts/pr-template-policy.cjs:167-240`). +3. Its diff — one or more `tests/emitted-drift-acks/*.json` deletions — matches **no** rule in `scripts/ci-test-scope.cjs`'s `RULES` table, so `classify()` falls through the `#408` empty-match fallback and selects the `'unit'` suite sentinel for the **unsharded, 15-minute-capped** "targeted/fast-signal" matrix lane (`scripts/ci-test-scope.cjs:520-563`, `.github/workflows/test.yml:155,166-172`). That lane then runs effectively the whole 827-file corpus and is killed by its own job timeout — verified directly in the run log (below). This is not flaky; it will recur on every future sweep PR whose diff is exactly the kind the sweep workflow always produces. +4. There is no auto-merge path for this PR anywhere in the workflow set. It sat open 2h38m and was merged by a human maintainer (`trek-e`, `MEMBER`) via a merge that landed **while `validate-title` and `Required tests` were still red** — an override, not an automatic pass. + +## A. The model itself + +### A1. What an emitted-drift-ack fragment is, and its lifecycle + +Read in full: `scripts/lint-emitted-drift-ack.cjs` (937 lines) and `tests/emitted-drift-acks/README.md`. + +- **Purpose** (`scripts/lint-emitted-drift-ack.cjs:1-31`): the differential-attribution gate (`tests/emitted-attribution.test.cjs`, ADR-2719) requires every emitted-artifact byte or size delta between `next` and a PR's HEAD to be attributable to that PR's own diff. When it legitimately cannot be (a converter change, a deliberate size grow), the escape hatch is a committed acknowledgment fragment under `tests/emitted-drift-acks/.json`, one file per PR (`#2914`), never the legacy single `tests/emitted-drift-ack.json`. +- **Two key spaces** (`tests/emitted-drift-acks/README.md:20-23`): an unattributable hash ripple is keyed on the emitted path; workflow/agent-file growth is keyed on the bare filename. +- **Creation**: the differential check's own failure output prints the exact fragment name/shape to paste (`CONTRIBUTING.md:1074-1076`, `docs/TESTING-SUITES.md:139-145`) — a human authors it as part of the PR that needs it. +- **"Spent"** (`scripts/lint-emitted-drift-ack.cjs:304-322`, function `assertNoAllSpentFragments`): every entry's prose in the fragment is compared, byte-for-byte after invisible-character/whitespace normalization (`ackProse`, line 261), against the copy of the same fragment at the **pre-push tip of `next`** (`readFragmentAtRef`, line 538). If every surviving entry matches its base-ref copy, the whole fragment is "all-spent" — its ripple is now baked into `next`, so it can no longer clear anything, yet it still **owns its path keys** (a second source declaring the same key is a hard duplicate-key error, lines 872-885). +- **Red on `next`**: the `guard-no-ack-on-next` job (`.github/workflows/test.yml:865-920`) runs on every push to `next` and calls `--guard-next --base-ref --defer-to-open-prs`. It fails when (a) the legacy `tests/emitted-drift-ack.json` is present at all (`assertAbsentOnNext`, line 449), or (b) any fragment is fully spent and not held by an open PR that still touches it (`#3842` deferral, line 363). +- **Remedy**: `git rm` the fragment. Since `#3875`, `ack-fragment-sweep.yml` automates exactly this `git rm`, computed via `--sweep-plan` (lines 815-833), which asks the guard for the identical set it reasoned about rather than re-deriving it. + +### A2. Governing ADRs/docs (verbatim quotes) + +- **ADR-2719** (`docs/adr/2719-emitted-artifact-attribution.md:1-11`): "Emitted-artifact attribution — replace the committed parity fixtures with a computed conservation law." Accepted 2026-07-27, supersedes ADR-2264 §2-4. +- **`CONTEXT.md:625`** (`RULESET.EMITTED_ATTRIBUTION`): "...`#2914` replaced the single shared ack file with per-PR fragments...exactly the shape `.changeset/` already uses for the identical 'every PR rewrites one shared document' conflict problem...`#3078` REVERSED that [premise] — fragments do not share a FILE but they DO share a PATH KEY SPACE...`#3875` automated the REMEDY that alert asks for, because detection without an executable remedy is what actually failed: `#3823` shipped the guard together with a static 45-fragment sweep computed at its own branch point, `#3809`'s fragment merged to `next` while it was in flight, and the guard reddened on its own merge commit and stayed red for 24 consecutive pushes over two days." +- **`CONTRIBUTING.md:1086-1094`**: "every PR needing an acknowledgment used to rewrite `tests/emitted-drift-ack.json`'s `paths` map wholesale — a single shared mutable file every such PR touches guarantees a merge conflict between any two of them (5 of 6 conflicting PRs in one open queue collided on this file and nothing else)." +- **`tests/emitted-drift-acks/README.md:37-41`**: "The sweep is automated because the manual remedy could not keep up. The guard evaluates at MERGE time; a hand-authored `git rm` is fixed at BRANCH time...`#3823` lost exactly that race to `#3809` on its own merge commit and left `next` red for 24 consecutive pushes." + +### A3. Every CI surface referencing the ack model + +- `.github/workflows/test.yml:865-920` — `guard-no-ack-on-next` job (push-to-`next` trigger; the "next-lane guard"). Also referenced at `test.yml:842-864` (comment block explaining scope/limits) and `test.yml:127` (the `lint-tests` step runs `node scripts/lint-emitted-drift-ack.cjs` as part of `lint:ci`, see `CONTRIBUTING.md` cross-ref and the raw log line captured in this session: `lint-tests` step at 2026-08-27T09:38:55Z runs the full chained lint including `node scripts/lint-emitted-drift-ack.cjs`). +- `.github/workflows/ack-fragment-sweep.yml` — the entire file (237 lines); scheduled sweep, `sweep` job, calls `--guard-next --sweep-plan --defer-to-open-prs` (line 90-92) and `--guard-next --sweep-plan` unheld variant (line 115-116). +- `scripts/lint-emitted-drift-ack.cjs` — the validator/guard/sweep-plan CLI itself (937 lines; exports `validateAckText`, `assertAbsentOnNext`, `assertNoAllSpentFragments`, `runGuardNext`, `main`, `fetchOpenPrTouchedAckPaths`, etc.). +- `tests/helpers/emitted-diff.cjs` and `tests/helpers/emitted-runtime.cjs` — the "ships"-side duplicate of the same constants/logic (`ACK_INVISIBLE`, `MAX_ACK_FRAGMENTS`, `RESERVED_ACK_KEYS`, `readAckFileAtRef`), held to parity by `tests/emitted-attribution.test.cjs` (per `scripts/lint-emitted-drift-ack.cjs:19-23,58-63,77-81,94-99`). +- No `eslint-rules/*` or `.claude/hooks/*` file references `emitted-drift`/ack fragments (grepped both directories; zero hits). +- `docs/TESTING-SUITES.md:130-160` and `CONTRIBUTING.md:1055-1124` — the human-facing "Editing shipped content" / TESTING-SUITES how-to sections. + +### A4. Design-history issues (via `gh issue view`) + +- **#2789** — root-caused why merging an ack reddened `next`: the ack set was the one input read *absolutely* (working tree only) in an otherwise base-relative differential machine, so a spent ack (already at base) still looked live to `staleAcks`. Fixed by scoping every ack to the diff that introduced it. +- **#3078** — proved #2914's premise wrong. #2914 exempted the fragment directory from the next-lane guard on the theory that independently-named fragments "cannot conflict with any other PR." #3078 measured 45 fragments owning 403 paths on `next` and showed fragments share a *path key space* even without sharing a file, so a spent fragment left in place still walls off the next PR that grows the same path. Extended `guard-no-ack-on-next` to cover fragments (implemented in PR **#3823**, "Closes #3078," merge commit `a84f75630`). +- **#3842** — the #3078 sweep, once it started deleting fragments, handed three outside-contributor open PRs (#3330, #3774, #3648) a `modify/delete` git conflict apiece, each on the exact fragment it was the PR's only conflicting path. Added the `--defer-to-open-prs` hold (`fetchOpenPrTouchedAckPaths`). +- **#3823** (a PR, not a separate issue — `gh issue view 3823` resolves to the PR body) — implemented the fragment half of `guard-no-ack-on-next`, closing #3078. +- **#3875** — root-caused the 24-consecutive-push red streak: `#3823` computed its sweep as a *static* list at branch time; `#3809`'s fragment merged to `next` *while #3823 was in flight* and was therefore never in that list. Proposed the scheduled sweeper (`ack-fragment-sweep.yml`) that recomputes the plan from the guard itself, on a timer, "honouring the existing `--defer-to-open-prs` hold." This issue is exactly what PR #3927 is discharging. + +### A5. Blast radius of removing the model entirely + +Files/jobs/tests that would need to change if `emitted-drift-ack` fragments were removed outright: + +- `scripts/lint-emitted-drift-ack.cjs` — deleted (or gutted to a no-op). +- `.github/workflows/test.yml:865-920` (`guard-no-ack-on-next` job) and its two checkout/fetch steps (`test.yml:875-895`) — deleted. +- `.github/workflows/ack-fragment-sweep.yml` — deleted entirely (237 lines). +- `tests/emitted-drift-acks/` directory and its `README.md` — deleted. +- `tests/helpers/emitted-diff.cjs`, `tests/helpers/emitted-runtime.cjs` — the ack-reading halves (`readAckFileAtRef`, `mergeAckSources`, `readAckSources`, `readAckSourcesAtRef`, `isSpent`, `staleAcks`, `spentAcks`) removed; `tests/emitted-attribution.test.cjs`'s differential check would then need a different escape hatch for legitimate unattributable ripples, or none at all (making every ripple a hard failure). +- `tests/emitted-attribution.test.cjs` — its ack-consuming assertions and the "#2914 migration pins" mentioned in `tests/emitted-drift-acks/README.md:64-69` removed. +- `tests/pr-template-policy.test.cjs`, `scripts/pr-template-policy.cjs` — unaffected directly, but the sweep PR's body text (which cites the ack model) would need rewriting if the workflow itself survives in some other form. +- `CONTEXT.md`'s `RULESET.EMITTED_ATTRIBUTION` entry (`CONTEXT.md:625`) and the `### Emitted Artifact Provenance` section it cross-references — rewritten. +- `CONTRIBUTING.md:1055-1124` ("Editing shipped content" ack sections) — rewritten. +- `docs/TESTING-SUITES.md:130-160` (the "workflow or agent grew" how-to) — rewritten. +- `docs/adr/2719-emitted-artifact-attribution.md` — would need a superseding ADR documenting the new escape hatch (or its removal), per this repo's own convention that architectural reversals get a new ADR rather than a silent edit. +- Any lint/test referencing `tests/emitted-drift-ack.json` or `tests/emitted-drift-acks/` by path (`grep -rl` across `.github/workflows/`, `scripts/`, `tests/` returns `test.yml`, `ack-fragment-sweep.yml`, `lint-emitted-drift-ack.cjs`, `docs/TESTING-SUITES.md`, `CONTRIBUTING.md`, `CONTEXT.md`, and the `tests/emitted-drift-acks/README.md` itself — all of the above). +- Issues #2789, #2914, #3078, #3842, #3875, #3823 would all become historical/moot context requiring a note that the model they describe no longer exists. + +## B. Why #3927 hung — one root cause per red signal + +### B6. `validate-title` — the sweep's own hardcoded title cannot pass its own gate + +The sweep hardcodes: `--title "chore: sweep spent ack fragments from next (${SHORT_SHA})"` (`ack-fragment-sweep.yml:266`), producing the literal title `chore: sweep spent ack fragments from next (929e02cb)` for PR #3927. + +The gate (`.github/workflows/pr-title-validator.yml`) delegates to `evaluatePrTitle()` in `scripts/release-notes/conventional-title.cjs:81-97`, which requires a header matching `HEADER_RE = /^([a-z]+)(\([^)]*\))?(!)?:/i` (line 28) **and** a scope group containing `#\d+` (`ISSUE_REF_IN_SCOPE_RE`, line 31; checked at line 92). + +Against `chore: sweep spent ack fragments from next (929e02cb)`: `HEADER_RE` matches with `type="chore"` and scope-group `undefined` (there is no `(...)` immediately after `chore` — the colon follows directly). Since `!scope` is true, `evaluatePrTitle` returns `{ valid: false, reason: 'missing-issue-ref' }` (line 92-93) — confirmed as the actual CI failure (`gh pr checks 3927`: `validate-title fail 8s`). + +Two independent defects in the hardcoded string, either one sufficient to fail: (1) the parenthetical `(${SHORT_SHA})` sits at the *end* of the title, not immediately after the type, so it is never read as the "scope" group at all; (2) even if repositioned, a short git SHA (`929e02cb`) contains no `#`, so it would still fail `ISSUE_REF_IN_SCOPE_RE`. + +A title that **would** pass: `chore(#3875): sweep spent ack fragments from next (929e02cb)` — matches the repo's own worked example in `conventional-title.cjs:71` (`enhance(#1549): add PR-title validator`) and correctly attributes the sweep to the issue it closes toward (#3875), which the current PR body already does in prose but the title does not. + +### B7. `Pull request template format` — passed, but only because the account is trusted, not because the body complies + +Contrary to the initial framing, this check **passed** (`gh pr checks 3927`: `Pull request template format pass 12s`). The mechanism, read in full from `scripts/pr-template-policy.cjs`: + +- The sweep's PR body (`ack-fragment-sweep.yml:243-261`) is freeform prose — no `## Fix PR` / `## Enhancement PR` / `## Feature PR` heading, none of the required headings in `TEMPLATES` (`pr-template-policy.cjs:55-97`). +- The changed-files carve-out does not apply: `tests/emitted-drift-acks/*.json` is not in `TOOLING_PATH_ALLOWLIST` (`pr-template-policy.cjs:24-43`), so `allPathsAreTooling` returns `false`. +- `matchingTemplate()` finds no heading match → `template: null` → falls to `reason = 'PR body does not match the fix, enhancement, or feature template.'`, `valid: false` (`pr-template-policy.cjs:217-223`). +- The consequence is gated on **author trust**, not template compliance (`pr-template-policy.cjs:226-229`): `action = trusted ? 'warn' : 'close'`. `TRUSTED_AUTHOR_ASSOCIATIONS` includes `MEMBER` (line 6-11). The PR's actual author association is `MEMBER` (`gh api repos/open-gsd/gsd-core/pulls/3927 --jq '.author_association'` → `MEMBER`), because the workflow authenticates `gh pr create` with `GSD_BOT_PR_TOKEN` — a personal-access token belonging to a real maintainer account (`trek-e`), not a GitHub App/bot identity. So `action: warn`: the `gsd-pr-template-policy` bot comment fires (`core.warning`, `pr-template-format.yml`'s "Warn trusted contributor" step), but the workflow only `core.setFailed`s on `action == 'close'` — never reached. The template body is objectively wrong; the check is objectively green. + +### B8. `Required tests` / `test (ubuntu-latest, 24)` — the fast-signal lane ran the WHOLE suite and hit its own 15-minute cap + +`gh run view 33059472019 --repo open-gsd/gsd-core --log-failed` and the per-job API (`jobs/98474440239`, `jobs/98478252101`) show: + +- `test (ubuntu-latest, 24)` — the **unsharded** matrix entry (`.github/workflows/test.yml:198-200`: `{os: ubuntu-latest, node-version: 24, scope: targeted}`, no `shard` key, hence the bare job name per the `name:` template at `test.yml:130`) — started `09:38:38Z`, its "Run scoped tests" step ran from `09:39:10Z` to `09:53:51Z` (14m41s), and the whole job was marked `cancelled` at `09:53:56Z`, total **15m18s** — exactly the job's `timeout-minutes: 15` cap (`test.yml:155`). +- The raw log for that job shows `run-tests: suite="all" files=827` at `09:39:11Z`, and the job reached only `chunk 13/14` before being killed (`chunk 13/14 — 61 files` at `09:52:37Z`) — i.e. it was running virtually the entire test corpus (827 files) in one unsharded process, not a narrow "targeted/fast-signal" subset. The three actually-sharded ubuntu `scope: full` jobs (which exist precisely to spread this same corpus across 3 runners, per the `#2952`/`#3057` comments at `test.yml:141-164`) all finished successfully in 6-9 minutes each. +- Root cause, traced through the scope classifier: `scripts/ci-test-scope.cjs`'s `RULES` array (lines 71-324) has **no entry** matching `tests/emitted-drift-acks/*.json`. In `classify()` (lines 483-591), the changed path starts with `tests/` so `productOrPipelineChanged = true` (line 502), but it is not a `.test.cjs` file so it is not added to `targeted`/`windows` (line 520 requires `endsWith('.test.cjs')`), and no `RULES[].match` fires for it. The `#408` fallback then fires: `if (codeChanged && targetedTests.length === 0) targetedTests.push('unit')` (lines 561-563). `scripts/ci-prepare-test-scope.cjs`'s `resolveSelection()` (lines 49-71) passes that non-empty `['unit']` list through **verbatim** to `.ci-selected-tests.txt`, and `'unit'` is a `SUITE_SENTINEL` (`ci-prepare-test-scope.cjs:22`) that `run-tests.cjs` resolves live to (effectively) the whole non-integration/security/install/slow corpus — 827 files, matching the observed log. +- `Required tests` (`.github/workflows/test.yml:702-703`) then reads `TEST_RESULT: cancelled` (its own job log, captured verbatim above) and fails with `##[error]test matrix did not pass` — a mechanical propagation of the canceled test job, not an independent finding. +- **This is deterministic, not flaky.** Every sweep PR's diff is, by construction, one or more deletions under `tests/emitted-drift-acks/` — a path class the scope-classification table has never had a rule for. Any future run of `ack-fragment-sweep.yml` will hit the identical fallback and the identical 15-minute cliff, for the same reason PR #3094 (referenced in `test.yml:151-153`) hit it on the `scope: windows` lane before that lane was sharded — except the `scope: targeted` lane was never sharded because nothing was expected to widen it to "all." + +### B9. How #3927 was actually merged despite red required checks + +`gh api repos/open-gsd/gsd-core/pulls/3927 --jq '.merged_by.login, .merge_commit_sha, .author_association'` → `trek-e`, `ad6abc896...`, `MEMBER`. The PR was opened `09:38:15Z` and merged `12:16:49Z` (2h38m later) by the same account, `trek-e` (a human maintainer, `is_bot: false` per `gh pr list ... --json mergedBy`). The timeline (`gh api .../issues/3927/timeline`) shows only `review_requested` → two bot `commented` events → `merged`/`closed`/`head_ref_deleted`, all attributed to `trek-e` or `github-actions[bot]`; no re-run or re-triggered check event appears between open and merge. `gh pr checks 3927` (queried post-merge) still reports `validate-title fail` and `Required tests fail` as the latest, final state of those checks — they were never turned green. The merge therefore landed with required checks still red: a maintainer override (admin merge / branch-protection bypass), not an automatic pass triggered by the automation. + +### B10. No auto-merge path exists for the sweep PR + +`ack-fragment-sweep.yml` ends at `gh pr create` + `gh pr edit --add-label automation --add-label no-changelog` (lines 263-280) — no `gh pr merge`, no `enableAutoMerge` call, nothing. Grepping every workflow for `automerge`/`auto-merge` finds only `auto-backmerge.yml` (an unrelated `main`→`next` release-engineering workflow, gated on `push: branches: [main]`, using `-s ours` merges — nothing about ack fragments) and a comment in `release.yml:729` about release→main PRs. `pr-mergeable-preflight.yml` is a reusable merge-conflict early-exit gate shared by eight caller workflows; it applies no label and performs no merge (`pr-mergeable-preflight.yml:1-40`). There is no maintainer-facing auto-merge wired to `ack-fragment-sweep.yml`'s PRs at all — every one requires a human to notice it, review it, and merge it by hand (or override), exactly as `ack-fragment-sweep.yml:14-15`'s own top-of-file comment states: "a human still approves the deletion." + +### B11. `GSD_BOT_PR_TOKEN` fallback warning did NOT fire — ruled out as a cause + +`gh run view 33059445125 --repo open-gsd/gsd-core --log` (the sweep workflow's own run) shows the "Open the sweep PR" step's env block: `HAS_BOT_TOKEN: 1`. The step's guard is `if [ -z "${HAS_BOT_TOKEN:-}" ]; then echo '::warning::GSD_BOT_PR_TOKEN is unset...'; fi` (`ack-fragment-sweep.yml:191-193`) — with `HAS_BOT_TOKEN=1` this branch is skipped, and indeed no such warning line appears anywhere in the run log. `GSD_BOT_PR_TOKEN` **was** configured and used. This is corroborated independently by B7/B9: the PR's `author_association` is `MEMBER` (not the unauthenticated-fallback shape) and the title/template *required* checks did run and report real verdicts (`validate-title`, `Pull request template format` both executed) — under the `GITHUB_TOKEN` fallback the workflow's own comment (`ack-fragment-sweep.yml:189`) states "no required checks will run on it," which is not what happened. **The fallback-token hypothesis is not the cause of this hang.** (Git commit authorship on the branch itself still reads `github-actions[bot]` per `git config user.name` at `ack-fragment-sweep.yml:204-205` — a separate, cosmetic identity from the `gh` API actor, which is `trek-e` via the PAT.) + +## C. Track record — is #3927 representative or a one-off? + +`gh pr list --repo open-gsd/gsd-core --search "head:chore/ack-sweep" --state all --limit 30 --json number,title,state,createdAt,mergedAt,mergedBy` returns **exactly one PR**: #3927 itself (opened `09:38:15Z`, merged `12:16:49Z`, `mergedBy: trek-e`). There is no prior sweep PR to compare against — this is the automation's first (and so far only) production run that produced a non-empty plan and opened a PR. It cannot yet be called "systematically failing" by volume, but the failure mechanism identified in B8 is a property of the scope-classification table and the sweep's title-generation code, not of this specific run's data — so it will reproduce on the *next* sweep PR with high confidence, absent a fix to either `ack-fragment-sweep.yml`'s title format or `scripts/ci-test-scope.cjs`'s `RULES` table (or both). + +## Open questions + +- Should `scripts/ci-test-scope.cjs`'s `RULES` gain an explicit entry for `tests/emitted-drift-acks/` (mapping to a narrow, cheap test list — e.g. `tests/emitted-attribution.test.cjs`, `tests/lint-emitted-drift-ack.cjs`'s own test file if any) so a fragment-only diff no longer falls through the `#408` "unit" fallback onto the unsharded lane? This is the most direct fix for B8, and does not require sharding the `scope: targeted` lane. +- Should `ack-fragment-sweep.yml:266` be changed to `--title "chore(#3875): sweep ${COUNT} spent ack fragment(s) from next (${SHORT_SHA})"` (or similarly scoped) to satisfy `conventional-title.cjs` outright? This is a one-line fix for B6. +- Should the sweep PR body be reshaped to match one of the three typed templates (likely "Fix" or a new "chore" carve-out) so `Pull request template format` passes on its merits rather than on the author's trust level — closing the latent gap where an untrusted-token run of this same workflow (if `GSD_BOT_PR_TOKEN` ever lapses to a non-`MEMBER` identity) would hit `action: close` instead of `warn`? +- Given B10/B11, should the workflow (or a separate follow-up automation) add a bounded auto-merge step once its own required checks are green, so a correctly-titled, correctly-scoped sweep PR does not still require a human to notice and merge it by hand every six hours?