fix(ci): gate required-context drift instead of discovering it on a stuck PR - #377
fix(ci): gate required-context drift instead of discovering it on a stuck PR#377avrabe wants to merge 2 commits into
Conversation
…tuck PR
Branch protection's required-check list and the workflows that produce
those checks are edited in different places and drift apart silently.
Three distinct ways, each with a different failure:
- a job runs on every PR and is NOT required -> it enforces nothing.
`osate-corpus` was this shape.
- a required context names a job that no longer exists, or one behind a
workflow-level path filter -> the PR sits at "Expected - waiting for
status" forever. Unmergeable, with nothing red to point at.
- a `continue-on-error` job is required -> the tick is green whatever
happened, so requiring it is decoration.
tools/check_required_contexts.py sorts every PR-triggered job into
GATEABLE / ADVISORY / UNDELIVERABLE from the workflow YAML alone, and
asserts .github/required-contexts.txt is exactly the GATEABLE set.
The distinction the whole gate turns on is invisible in the YAML unless
you look for it: a job skipped by a job-level `if:` reports "skipped",
which SATISFIES a required context. A job in a workflow that never
triggered reports *nothing at all*. Job-level filtering is therefore
gateable and workflow-level `on.pull_request.paths` is not - and getting
that backwards is what produces the unmergeable PR.
Evidence it models reality rather than itself: `--print` emits the
GATEABLE set derived purely from YAML, and it is byte-identical to the 18
contexts branch protection actually enforces today.
Two follow-on fixes it demanded on its first live run: `bazel-test` and
`kani` are both `continue-on-error` but named as if they were verdicts.
"Kani Bounded Model Checking" with a green tick is a claim the run does
not support. Both now carry "(advisory)" in the name - the only place a
PR reader sees it. Neither is a required context, so the rename cannot
strand one.
Why a committed list rather than the live API: reading branch protection
needs an admin token, which the Actions GITHUB_TOKEN is not.
Why steps of rivet-validate rather than a job of its own: a new job would
itself be gateable, so it would have to be added to the required list -
and syncing branch protection needs that same admin token. It would ship
permanently red.
Stdlib-only, following the constraint already documented above the
human-scoped guardrail: no workflow here installs a Python package, so
importing PyYAML would make a *required* check depend on an unverified
runner package. The reader is fail-closed - unrecognised syntax at the
depths it reads exits 2 rather than scanning past it - and `--cross-check`
validates it against PyYAML on the real corpus wherever PyYAML exists.
11 self-tests, each recording the failure it proves is still caught,
including both halves of the path-filter trap: required-and-undeliverable
must fail, and not-required-and-undeliverable must pass. A mechanical
"every job must be required" rule gets that second one wrong.
Closes #372
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A cold subagent was asked to refute this gate's own claims. Five of ten
held; the rest are fixed here. Two of the defects were the gate's stated
failure mode reproduced inside the gate itself.
VERDICT-FLIPPING (the reader classified a job wrongly, silently):
* `pull_request: {branches: [main], paths: ["src/**"]}` — a flow mapping
read as an opaque string. `classify_workflow` does
`if not isinstance(pr, dict): pr = {}`, so the paths filter evaporated,
the job was ruled GATEABLE, and the gate emitted an ::error:: ordering
the author to require a context GitHub will never deliver. The gate
handed you the exact deadlock it exists to prevent.
* four-space indentation — every job id lands at indent 4, where the
reader is not looking, and the indent-2 fail-closed check never fires.
Result: zero jobs, zero violations, PASS. That is fail-OPEN, which the
docstring asserted could not happen.
LOSS-OF-INFORMATION (wrong name, so the real context goes ungated):
* a folded/literal block scalar as a job `name:` read as the literal `>-`;
* YAML anchors and aliases read verbatim.
All four now raise via `_reject_unsupported` plus an empty-`jobs:` check.
Refusing is the only safe answer: `--cross-check` catches these, but it
needs PyYAML and so deliberately does not run in CI — the one place the
gate is load-bearing. Safety there cannot rest on agreeing with a reference
parser; it has to rest on refusing what cannot be represented.
DISCOVERY:
* the glob was `*.yml`. Actions loads `.yaml` too, so a `sneaky.yaml`
holding a red-capable job was invisible and the gate reported PASS on a
directory it never fully read. Now `*.y*ml`, asserted on the constant
itself — the fixtures build their own glob and cannot see it.
TESTS:
* self-test 9 ("job-level if: is required") was provably vacuous:
the reader never reads `if:`, so its fixture parses byte-identically to
the clean one and no mutant can distinguish them. It claimed to prove
"the distinction the whole gate turns on" — a claim about GitHub's
runtime behaviour that no fixture over a YAML string can establish.
Replaced with the `paths-ignore` case, whose guard survived the entire
previous suite (deleting it reddened nothing), refuting the comment
"each fixture kills exactly one guard".
* fixtures added for all four refusals. 15 self-tests; each new guard
verified by mutation — no-op it, and a named fixture goes red.
DOCSTRING:
* `osate-corpus` was cited as a live instance of failure mode 1. It has
the symptom, but `aadl-interop.yml` has a workflow-level paths filter,
so this gate buckets it UNDELIVERABLE, rules it must NOT be required,
and reports PASS. The flagship motivating example is one the gate
declines to fix. Said plainly, with the remedy.
Live corpus unchanged: 21 PR-triggered jobs -> 18 gateable, 2 advisory,
1 undeliverable; --cross-check still agrees with PyYAML across 10
workflows; --against-api still matches branch protection exactly (18).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clean-room audit: 3 of 5 claims about this gate were refutedI asked a fresh-context subagent to refute this PR's claims, not confirm
The rest were wrong. Fixed in 00393b2:
Docstring correction. Verification. 15 self-tests. Each new guard mutation-tested — no-op it and a Not claimed: that the reader now handles arbitrary YAML. It handles less than 🤖 Generated with Claude Code |
Rivet verification gate✅ 20/20 passed
Filter: Failed artifacts(none) Updated automatically by |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Superseded by #387, merged as This branch was not merged directly — the six guardrail PRs were rebuilt onto one batch branch ( Verified on Closing as superseded, not as unmerged work. Reopen if anything here is missing from |
1 participant
Closes #372.
The failure
Branch protection's required-check list and the workflows that produce those
checks live in different places and drift apart with nothing watching. Three
distinct drifts, three different failures:
osate-corpuswas this shapecontinue-on-errorjob is requiredThe distinction the gate turns on
This is the part that is invisible in workflow YAML unless you go looking for
it, and getting it backwards is what produces the unmergeable PR:
if:reportsskipped— whichsatisfies a required context.
on.pull_request.paths) reports nothing at all.So job-level filtering is gateable and workflow-level filtering is not. A
mechanical "every job must be required" rule gets this exactly wrong, and
would have deadlocked every PR that misses those paths.
tools/check_required_contexts.pytherefore sorts every PR-triggered job intoGATEABLE / ADVISORY / UNDELIVERABLE from the YAML alone, and asserts
.github/required-contexts.txtis exactly the GATEABLE set.Evidence it models reality, not just itself
--printemits the GATEABLE set derived purely from workflow YAML. It isbyte-identical to the 18 contexts branch protection actually enforces today
(
diff→ no output). The classifier and GitHub agree on the live repo.On its first live run it flagged exactly two violations and correctly did
not flag
osate-corpus— the deadlock trap avoided on the real repo, notonly in a fixture.
The two fixes it demanded
bazel-testandkaniare bothcontinue-on-errorbut named as if they wereverdicts. "Kani Bounded Model Checking ✓" is precisely the claim a reader
should not draw from a tick that is green regardless of outcome. Both now carry
(advisory)in the name — the only place a PR reader sees it. Neither is arequired context, so the rename cannot strand one (guard 2 would fail if it
could).
Two design constraints, recorded so they aren't re-litigated
Why a committed list rather than the live API. Reading branch protection
needs an admin token; the Actions
GITHUB_TOKENis not one.--against-apiexists for a human with credentials.
Why steps of
rivet-validaterather than a job of its own. A new job woulditself be GATEABLE, so it would have to be added to the required list — and
syncing branch protection needs that same admin token. It would ship
permanently red.
Stdlib-only, fail-closed
Following the constraint already documented above the human-scoped guardrail:
no workflow here installs a Python package, so importing PyYAML would make a
required check depend on an unverified runner package — and a guardrail that
cannot run reads as approval. The reader is fail-closed (unrecognised syntax at
the depths it reads exits 2, rather than scanning past it), narrowed to fire
only where a misread would change a verdict.
--cross-checkvalidates itagainst PyYAML on the real corpus wherever PyYAML exists: agrees on every
field it reads, across all 10 workflows.
Self-tests
11 cases, each recording the failure it proves is still caught — including
both halves of the path-filter trap: required-and-undeliverable must fail,
and not-required-and-undeliverable must pass.
Scope
CI plumbing, no rivet artifact — matching the #353/#363/#364/#366 precedent;
this repo carries no
REQ-CI-*ids. Flagged so the omission reads as a choice.🤖 Generated with Claude Code