Skip to content

Commit 4370066

Browse files
wshallwshallwshallwshallclaude
authored
ci(gates): re-run the commit-time gates in CI, which a replayed commit skips (BACKLOG #1395) (#823)
* ci(gates): re-run the commit-time gates in CI, which a replayed commit skips (BACKLOG #1395) git does not invoke the pre-commit hook for a commit created by the sequencer, so a rebase or cherry-pick lands a commit with none of the eleven gates having run and nothing reports it. #1395 prefers a CI-side re-run over a second local hook, because a local hook is advisory by construction and that row is the proof. precommit-replay.yml runs `pre-commit run --from-ref --to-ref` over the pull request's diff. It skips exactly two hooks, and the rule is narrow: skip only where running it on a runner gives a WRONG answer, not merely an inconvenient one. ledger-gate its hook entry omits --ci, so the ownership arm would read an allocation registry living in .git/mefor-coord/ that never reaches a runner. ci.yml already runs ledger_check.py --ci, the half CI can enforce. forbidden-content fails closed on a git-ignored token file no runner has. Reproducing security.yml's fork/secret branching in a NON-REQUIRED leg would either red every fork pull request or return a structural-only green that reads as a leak-gate pass. security.yml's REQUIRED job already scans the whole tree. Three guards, because the leg's failure modes are all silent. The skip list is pinned as a SET: it grows one convenient entry at a time until the leg runs nothing and still reports green, and pre-commit does not validate SKIP, so a typo skips nothing while reading as deliberate coverage. The invocation is asserted over the resolved run block rather than the file, because the header prose names --from-ref/--to-ref and a file-wide check would survive deleting the step. And the job is asserted absent from .github/required-contexts.txt. Also pins the VERSION axis _MIRRORS does not reach. It anchors on the invocation, so it proved the right tool ran with the right discriminating flag and said nothing about which BUILD ran. gitleaks (v8.18.4) and actionlint (v1.7.12) were held only by prose comments saying to keep the version in step, and a comment cannot fail. Anchored on each repo's own download URL so a stray VER= elsewhere cannot satisfy the comparison. Deliberately NOT a required context: it builds every pinned hook environment per run. Promoting it needs branch protection moved first, then required-contexts.txt, then the count in tests/test_required_contexts.py, and a merge_group trigger before any of that. Every guard was mutation-tested. Rev drift, SKIP growth, a SKIP typo, the run step losing --from-ref while the header kept it, a bare `pre-commit run`, and the job acquiring a required context all go red, and the tree returns to green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(gates): hold the bandit hook rev against the version CI installs (BACKLOG #1395) The two bandit arms in this file have pinned SCOPE since the hook and CI first drifted apart, and nothing pinned VERSION. That is the same divergence one level down: `--skip B101,...` means different findings under different bandit releases, so two halves that agree on every skip and every exclude can still enforce different standards with nothing saying so. It is not hypothetical here. The ci-scanners group's own comment records an unpinned 1.9.x upgrade silently changing `# nosec` parsing and breaking a green branch, which is why that pin is exact. It matters most on a commit nobody gated. Under #1395 git does not run pre-commit for a commit created by the sequencer, so after a rebase the CI build is the only bandit that ever looked, and a developer whose commit passed locally has learned nothing about the version that will judge it. `pre-commit autoupdate` is the likely author: .pre-commit-config.yaml already warns that a bare run walks the ruff rev past its cap, and it walks this one too. Lives here rather than in tests/test_gate_ci_mirror_parity.py deliberately. That file owns the eight hooks this one does not, and bandit is one of the three it defers here. One hook, one place, so the two files cannot grow a second silently different definition of the same rule. The sibling gitleaks and actionlint revs are held there in the same change. The `==` is asserted rather than borrowed from test_ci_venv_pinning's EXACT_GROUP_PINS: `uv export` writes a fully pinned lock from a `>=` spec just as readily, so a floor would leave this comparison holding the rev against a version nobody promised to install, and nothing downstream would reveal it. Mutation-tested: moving the hook rev to 1.9.3 against the group's 1.9.4 goes red, and restoring it returns the file to green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(backlog): record the #1395 CI leg, and correct a claim in the row that was never true Two changes to the #1395 row. SHIPPED: the CI-side re-run the row prefers, plus the version axis nothing held. Records which two hooks the leg skips and why each would give a WRONG answer on a runner rather than merely an inconvenient one, that the skip list is pinned as a set, that the leg is deliberately not a required context and what promoting it would cost, and which mirror still covers each skipped hook. It also records what the leg does NOT buy, so the row is not read as closure of the whole finding. The leg is late in exactly the way the 2026-09-03 scoring note says, a changed-file set against the checked-out tree still cannot see content that lives only in an intermediate commit, and the ownership arm still has no enforcement path on a replayed commit and cannot be given one. CORRECTION: the row ended a paragraph with "A PARITY TEST FOR THOSE EIGHT IS THE DURABLE FIX AND IS NOT BUILT", and that was already false when it was written. tests/test_gate_ci_mirror_parity.py covers exactly those eight and landed in 389168a, the very commit that filed this row. The scoring note above repeats the claim, so it over-counted the remaining work. Corrected in place rather than annotated, because a reader who reaches that sentence decides whether to BUILD that module, and two of them would then exist. A second silently different definition of one rule is the defect that whole family of tests exists to prevent. The correction is recorded rather than the sentence quietly deleted, so the next reader knows which half is real instead of wondering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: wshallwshall <mefordev@messagefoundry.org> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 99887f5 commit 4370066

4 files changed

Lines changed: 497 additions & 2 deletions

File tree

‎.github/workflows/precommit-replay.yml‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
name: pre-commit replay
2+
3+
# Re-runs the commit-time gates in CI, because git does not run them for a replayed commit
4+
# (BACKLOG #1395).
5+
#
6+
# THE DEFECT THIS ANSWERS, measured with a positive control. `git` never invokes the `pre-commit`
7+
# hook for a commit created by the sequencer. An ordinary `git commit` printed 11 hook result lines;
8+
# a `git cherry-pick` and a `git rebase --continue` each printed 0 and still created a commit. So a
9+
# rebase -- which every branch behind `main` needs, and which is an entirely innocent daily operation
10+
# -- lands a commit with NONE of the eleven gates having run, and nothing anywhere reports it. The
11+
# commit is indistinguishable from a gated one afterwards: `git log` cannot show which commits were
12+
# gated, and the only evidence is the terminal output of the run that created it.
13+
#
14+
# WHY A CI RE-RUN RATHER THAN A `post-rewrite` / `pre-merge-commit` HOOK PAIR. #1395 names both and
15+
# prefers this one, for a reason that is structural rather than a preference: a local hook is
16+
# advisory by construction, and this row is the proof. A second local hook can be skipped by the same
17+
# class of workflow that skips the first.
18+
#
19+
# WHAT THIS LEG ACTUALLY ADDS, STATED HONESTLY, BECAUSE OVERSTATING IT WOULD MAKE IT THE
20+
# "COMPENSATING CONTROL RESTING ON A FALSE PREMISE" THAT CLAUDE.md SECTION 11 FORBIDS.
21+
#
22+
# * IT IS NOT THE FIRST LINE, AND IT IS NOT EVEN EARLY. Ten of the eleven hooks already have a
23+
# hand-written CI mirror that runs on the tip tree, so on a replayed commit the leak guard and
24+
# the secret detector are LATE, not absent. This leg is late in the same way -- a push to this
25+
# public repository publishes before any runner starts. It buys defence in depth, not earliness.
26+
# * IT DOES NOT INSPECT INDIVIDUAL COMMITS. `--from-ref/--to-ref` resolves a CHANGED-FILE set
27+
# between two refs and runs the hooks against the checked-out tree. Content that exists only in
28+
# an intermediate commit and never reaches the tip is out of its reach, exactly as it is out of
29+
# the tip-tree mirrors' reach.
30+
# * WHAT IT DOES BUY IS THAT IT CANNOT DRIFT. Every other CI mirror is a separate hand-maintained
31+
# re-implementation of a hook, and that drift has already happened once in this repository (see
32+
# the "again" in security.yml's bandit comment). This leg runs THE HOOKS THEMSELVES, with their
33+
# own args, from their own pinned environments -- so for the nine it runs, hook-versus-CI
34+
# equivalence is true by construction rather than by a test. It also covers a future hook that is
35+
# added with no mirror at all.
36+
#
37+
# TWO HOOKS ARE SKIPPED, AND THE RULE IS NARROW: skip only where running it here would produce a
38+
# WRONG answer -- a green that means nothing, or a red that is not the pull request's fault.
39+
# Everything else runs.
40+
#
41+
# ledger-gate Its hook entry carries no `--ci`, so the OWNERSHIP arm would run. That arm
42+
# reads an allocation registry living in `.git/mefor-coord/`, which never
43+
# reaches a runner, so it would red every pull request on a check no author can
44+
# satisfy. ci.yml already runs `ledger_check.py --ci`, which is the
45+
# duplicate-number half -- the half CI can enforce. #1395 records that the
46+
# ownership arm is unavoidably CI-skipped rather than an oversight, and this
47+
# leg must not "fix" that by teaching CI to read allocations.
48+
# forbidden-content The hook fails closed via `--require-tokens`, and the real token list is a
49+
# git-ignored file that no runner has. Reproducing security.yml's fork/secret
50+
# branching in a NON-REQUIRED leg would either red every fork pull request, or
51+
# hand back a structural-only green that reads as a leak-gate pass. Both are
52+
# worse than not running it. security.yml's `forbidden-content` job is REQUIRED
53+
# and scans the whole tree with the token secret and a per-section detector
54+
# floor; branch-leak-scan.yml runs the same scanner on every push. Nothing is
55+
# lost by skipping it here.
56+
#
57+
# tests/test_gate_ci_mirror_parity.py pins that skip list to exactly those two ids. Without that, the
58+
# list grows one convenient entry at a time until the leg runs nothing and still reports green.
59+
#
60+
# NOT A REQUIRED CHECK, and it must not become one without the owner's decision. Branch protection's
61+
# required set is read from the server, and `.github/required-contexts.txt` plus the count pinned in
62+
# tests/test_required_contexts.py have to move in the same pull request as any change to it. This job
63+
# is deliberately absent from that file. It is non-required for a second reason too: it builds every
64+
# pinned hook environment from scratch on each run, so it is the slowest way to learn any of these
65+
# facts and belongs off the critical path.
66+
#
67+
# NO `paths:` FILTER, DELIBERATELY. The subject is "which gates ran on the commits in this pull
68+
# request", and that question is live for any diff. A paths filter would answer it only for pull
69+
# requests that happen to touch the files it names.
70+
#
71+
# NO `merge_group:` TRIGGER, DELIBERATELY. A non-required context cannot gate a queue entry, so
72+
# running it there would spend queue time and change no outcome. (For a REQUIRED check the omission
73+
# would be the total failure codeql.yml's header records under BACKLOG #340 -- that hazard is the
74+
# reason this line says why it is absent rather than leaving it to be re-derived.)
75+
#
76+
# NO HOOK-ENVIRONMENT CACHE. `actions/cache` is used by no workflow in this repository, so adding the
77+
# first one means choosing and pinning a new third-party action -- its own decision, not a side
78+
# effect of this leg. The cost is a few minutes on a job that blocks nothing.
79+
on:
80+
pull_request:
81+
workflow_dispatch:
82+
83+
permissions:
84+
contents: read
85+
86+
concurrency:
87+
group: precommit-replay-${{ github.ref }}
88+
cancel-in-progress: true
89+
90+
jobs:
91+
replay:
92+
name: pre-commit re-run over the diff
93+
runs-on: ubuntu-latest
94+
timeout-minutes: 20
95+
steps:
96+
# fetch-depth: 0 is required, not defensive. `--from-ref/--to-ref` resolves the changed-file
97+
# set between two refs, which needs a merge base; the default depth-1 checkout gives the head
98+
# no history to find one in, so a shallow fetch of the base alone would not help.
99+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
100+
with:
101+
fetch-depth: 0
102+
persist-credentials: false # read-only lint job; do not persist the token (zizmor: artipacked)
103+
104+
# setup-python is LOAD-BEARING HERE, not boilerplate. Four of the hooks are `language: system`
105+
# with `entry: python ...`, and .pre-commit-config.yaml records the measurement that `python`
106+
# (as distinct from `python3`) is not on a stock Ubuntu PATH. setup-python puts both on PATH,
107+
# which is what lets those four resolve at all on this runner.
108+
- uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
109+
with:
110+
python-version: '3.14'
111+
112+
# Pinned to the version this repository has actually measured against: the comments in
113+
# tests/test_lint_scope_parity.py and .pre-commit-config.yaml both record behaviour verified
114+
# against pre-commit 4.6.1. It is version-pinned but NOT hash-pinned, which is a real residual:
115+
# pre-commit is in no dependency group, and adding one means a `uv lock`/`uv export` pass.
116+
# Routing it through ci/locks/ the way the scanners are is the correct end state.
117+
- name: Install pre-commit
118+
run: python -m pip install --disable-pip-version-check "pre-commit==4.6.1"
119+
120+
# The base branch, resolved by NAME rather than by SHA so a re-targeted pull request still
121+
# compares against the branch it will actually merge into. On workflow_dispatch there is no
122+
# base_ref, so it falls back to the repository default branch.
123+
- name: Fetch the base branch
124+
env:
125+
BASE_REF: ${{ github.base_ref || github.event.repository.default_branch }}
126+
run: git fetch --no-tags origin "+refs/heads/${BASE_REF}:refs/remotes/origin/${BASE_REF}"
127+
128+
# SKIP is pre-commit's own comma-separated hook-id list. The two ids and the reasoning are in
129+
# the header; tests/test_gate_ci_mirror_parity.py asserts this value stays exactly those two.
130+
#
131+
# This step may go red because a hook REWROTE a file rather than because it found something --
132+
# `ruff-check` carries `args: [--fix]`. That is the same signal a developer gets locally, and
133+
# --show-diff-on-failure prints the rewrite so the report says what to apply.
134+
- name: Re-run every commit-time gate over the pull request diff
135+
env:
136+
BASE_REF: ${{ github.base_ref || github.event.repository.default_branch }}
137+
SKIP: ledger-gate,forbidden-content
138+
run: |
139+
pre-commit run \
140+
--show-diff-on-failure \
141+
--from-ref "refs/remotes/origin/${BASE_REF}" \
142+
--to-ref HEAD

‎docs/BACKLOG.md‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19745,8 +19745,17 @@ against a ledger that has work in it, and it does so by discarding the best-writ
1974519745
> **PARITY BETWEEN HOOK AND CI IS ITSELF ENFORCED FOR ONLY 3 OF THE 11**, by
1974619746
> `tests/test_lint_scope_parity.py` (14 tests): `ruff-format`, `ruff-check` and `bandit`. **The other
1974719747
> eight mirrors are hand-maintained with nothing comparing them** -- all eight MATCH today, checked
19748-
> individually rather than assumed. ***A PARITY TEST FOR THOSE EIGHT IS THE DURABLE FIX AND IS NOT
19749-
> BUILT.***
19748+
> individually rather than assumed. ***A PARITY TEST FOR THOSE EIGHT IS THE DURABLE FIX.***
19749+
19750+
> ***CORRECTION 2026-09-03: THE PRECEDING PARAGRAPH ENDED "AND IS NOT BUILT", AND THAT WAS ALREADY
19751+
> FALSE WHEN IT WAS WRITTEN.*** `tests/test_gate_ci_mirror_parity.py` covers exactly those eight and
19752+
> **landed in `389168a79`, the very commit that filed this row** -- so the claim has been wrong for
19753+
> the whole life of the item, and the 2026-09-03 scoring note above repeats it ("the durable half is
19754+
> a parity module for the eight ... plus a CI leg"). **The sentence is corrected in place rather than
19755+
> annotated, because a reader who reaches it decides whether to BUILD that module**, and two of them
19756+
> would then exist: a second, silently different definition of one rule, which is the defect that
19757+
> family of tests exists to prevent. Recorded here rather than deleted so the next reader knows the
19758+
> scoring note over-counted the remaining work rather than wondering which half is real.
1975019759

1975119760
> **Full working, per-gate, is anchored at `refs/builder2/dup-hunt-2026-08-29`
1975219761
> (`BUILDER2-2026-08-29-CI-TWIN-CENSUS.md`), including the census's own correction: it first claimed
@@ -19761,6 +19770,41 @@ against a ledger that has work in it, and it does so by discarding the best-writ
1976119770
> not reproduce in another. **The allocation answer is "replay versus fresh authorship". THE REASON IS
1976219771
> THAT NO GATE RUNS AT ALL, which is a larger finding than the question that produced it.**
1976319772

19773+
> ***SHIPPED 2026-09-03 -- THE PREFERRED FIX, PLUS THE VERSION AXIS NOTHING HELD.***
19774+
> `.github/workflows/precommit-replay.yml` runs `pre-commit run --from-ref --to-ref` over the pull
19775+
> request's diff, which is the CI-side re-run this row prefers over a second local hook. **It skips
19776+
> exactly two of the eleven, and both would give a WRONG answer on a runner rather than merely an
19777+
> inconvenient one:** `ledger-gate`, whose hook entry omits `--ci` and whose ownership arm reads a
19778+
> registry living in `.git/mefor-coord/` that no runner has, and `forbidden-content`, which fails
19779+
> closed on a git-ignored token file -- reproducing `security.yml`'s fork/secret branching in a
19780+
> NON-REQUIRED leg would either red every fork pull request or hand back a structural-only green
19781+
> reading as a leak-gate pass. Each keeps the mirror CI can actually run (`ledger_check.py --ci`;
19782+
> the REQUIRED `forbidden-content` job). **The skip list is pinned as a SET**, because its failure
19783+
> mode is growth until the leg runs nothing and still reports green.
19784+
>
19785+
> **NOT A REQUIRED CONTEXT, deliberately** -- it builds every pinned hook environment per run, and
19786+
> promoting it needs branch protection moved first, then `.github/required-contexts.txt`, then the
19787+
> count in `tests/test_required_contexts.py`, and a `merge_group:` trigger before any of that.
19788+
>
19789+
> ***AND THE GAP THE EXISTING PARITY MODULE DID NOT REACH: VERSION.*** `_MIRRORS` anchors on the
19790+
> INVOCATION, so it proved a mirror still ran the right tool with the right discriminating flag and
19791+
> said nothing about WHICH BUILD ran. For the three third-party hooks whose CI half is a downloaded
19792+
> release or a lock pin, that was held **only by prose** -- `.pre-commit-config.yaml`'s "Keep the
19793+
> version in step" and `zizmor.yml`'s matching line. **A comment cannot fail.** `gitleaks` (rev
19794+
> `v8.18.4` / `security.yml`) and `actionlint` (rev `v1.7.12` / `zizmor.yml`) are now held by
19795+
> `test_the_hook_rev_matches_the_version_ci_installs`, anchored on each repo's OWN download URL so a
19796+
> stray `VER=` elsewhere cannot satisfy it; `bandit` (rev `1.9.4` versus the `ci-scanners` group) by
19797+
> `test_bandit_hook_rev_matches_the_version_ci_installs` in the sibling, beside its scope arms.
19798+
> **All four halves matched already** -- this closes the axis, it did not find a live drift.
19799+
>
19800+
> **WHAT IT DOES NOT BUY, so the row is not read as closure of the whole finding.** The leg is **LATE
19801+
> in exactly the way the scoring note says** -- a push to this public repo publishes before any runner
19802+
> starts -- and `--from-ref/--to-ref` resolves a CHANGED-FILE set against the checked-out tree, so
19803+
> **content living only in an intermediate commit is still out of reach**, as it is for every
19804+
> tip-tree mirror. What it does buy is that the nine hooks it runs **cannot drift from CI**, because
19805+
> they ARE the hooks. **The ownership arm still has no enforcement path on a replayed commit and
19806+
> cannot be given one** -- unchanged, and unavoidable, per the amendment above.
19807+
1976419808
**Cluster:** Quality gates / CI. **Priority:** P2. **Verdict:** build.
1976519809
**Severity:** no engine effect and no PHI axis today (sec. 0, zero deployments) -- **but this repo is
1976619810
public**, and the bypassed set includes the guard whose job is to stop customer or PHI content

0 commit comments

Comments
 (0)