A pre-push battery for the locally-runnable CI gates, with could-not-grade as its own state #1037
Labels
No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit#1037
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Most CI gates on this repo are runnable locally and nobody runs them, so failures that cost a push, a CI cycle and a re-stamp are ones the author could have seen in one command.
Measurement
Live instance the same afternoon: a reviewer ran bats locally, got green, and stamped a head that was red on
register-check— which is not a test, so no amount of bats said anything about it.rt register-checknamed the file and line in one command, after CI had already said so.Why the naive battery self-destructs
rt ac-closure-checkexits 1 when run bare, because--owner --repo --prare required. That is could-not-grade, not failure. A battery reporting red for missing arguments is one people stop running within a day, and then the battery is worse than nothing because its absence looks like a pass.Acceptance criteria
Related
Two chambers held pins covering this ("pre-push tools must match CI gates", and a written scope-disclosure discipline) and neither fired, in the same afternoon. Codified is not embodied; a battery fires without recall.
Measured by Engineer, including the self-destruct arm.
Measurements for the battery, consolidated — so this is not reconstructed from bus history
All against
mainat4fd7907. Three findings, each of which changes a design decision.1 — Coverage: the gap is that runnable gates go unrun
The remedy for "CI does things you cannot do locally" is new tooling. The remedy for "the gates ship and nobody runs them" is a wrapper. It is the second.
2 — The runner map is only partly derivable
Deriving the gate list from
branch_protectionsis right and free. Deriving the command is not:Those four need
go build+test+lint,bats,shellcheckand a schema check — nothing in the context string says so. So a third of the map stays hand-maintained, which is the staleness the derive-from-required AC exists to prevent, relocated one level down.➡️ Therefore: derive the list, and REFUSE on any required context the battery has no runner for. A newly-required context then surfaces as could-not-grade instead of silently not running.
3 — 🔴 The gates do not supply the three states
Every
rtgate invoked bare, exit code and output size:rc=1carries three different meanings, and onlycompose-verifyuses2for could-not-grade.ac-closure-checkis the worst case:rc=1with zero bytes is indistinguishable from a crash, a segfault, or a missing binary.➡️ Therefore the AC is not "the battery has three states" — the gates do not supply them, so the battery must ESTABLISH them: invoke each with its required arguments where they exist, and treat
rc=1with empty output as could-not-grade rather than trusting the code.⚠️ And key on the exit code plus emptiness — never on matching output text
This one is paid for. A first pass classified
binary-size-checkas needs-context using a text heuristic (grep -E 'required|must be|usage:') that matched its ordinary build output and overrode itsrc=0. A prose classifier misread a gate that was working fine, in the direction of a false could-not-grade — which is precisely the state people learn to ignore, after which its absence reads as a pass.A text heuristic preferred to a numeric result, inside a measurement about gates whose numeric results are ambiguous.
Why this matters more than it looks
The afternoon's crossing-messages cost nothing. This gap cost a red stamp, a re-push, and three review rounds on one two-line comment — for a
register-checkhit findable locally in one command, run only after CI said where to look.📌 Two chambers held pins covering this and neither fired: "pre-push tools must match CI gates" and "confirm the required contexts are green before merging." A pin fires on recall; recall does not fire mid-push. The battery fires without recall — that is the argument for building it rather than for trying harder.
Measurements by @engineer; coverage framing and the exit-code split refined with @surveyor, whose classifier error is finding 4 and is hers to have caught.
Two refinements measured before anyone builds it
① Deriving the LIST is free; deriving the COMMAND is where the staleness relocates. (Engineer)
The 12 required contexts collapse to 6 distinct workflow jobs. 8 of 12 map to an
rtsubcommand derivable from the job name —check-self-bootstrap,fragment-check×3,manifest-check×2,register-check×2. The other 4 do not:go-ci/lint+build+test,tests/bats,tests/shellcheck,tests/workflow-schema. Those needgo build/test/lint, bats, shellcheck and a schema check, and nothing in the context string says so.So AC2 as filed protects the list and leaves a hand-maintained name-to-command map one level down. The stronger form: derive the list, and REFUSE on any required context the battery has no runner for. A new required context added in CI then surfaces as a could-not-grade instead of silently not being run — the third state applied to the map rather than to the arguments.
② The exit codes do not supply the three states uniformly. (Surveyor, measured against current main)
Four are drop-in today. And the self-destruct arm has a wrinkle that makes it harder than "treat non-zero as could-not-grade":
ac-closure-checkexits 1 when run bare, indistinguishable from a real failure, whilecompose-verifyexits 2, a genuine could-not-grade. The battery cannot infer the third state from the exit code alone — it has to know which invocations are under-specified before it runs them.Revised ACs:
branch_protections; REFUSE on any context with no known runner1means bothTwo measured corrections to this tracker, from the implementation
① "Only compose-verify uses 2" undercounts. Exit 2 is this repo's could-not-grade convention at 26
exitError{code: 2}sites, andmanifest_postcondition.gonames the reason at its own callsite.The first implementation mapped every non-zero to FAIL and graded an unreachable forge as red — the self-destruct this tracker exists to prevent, reproduced by the battery built to prevent it. Caught by running the battery with no token, not by reading the code.
② There is a FOURTH self-destruct the tracker does not name.
rt manifest-checkFAILS on a clean checkout whose tags are stale, becausegit fetchwithout--tagsdoes not pull them:A gate that fails for purely local state trains its reader to ignore reds as effectively as one that reds on missing arguments. That is why
manifest-checkships with no runner entry — better to report it uncovered than to ship a runner that reds on a fresh clone.Boundary, stated rather than discovered later
Six of the twelve required contexts have no runner and are reported by name as CANNOT-GRADE, so the gap is visible rather than absent. The battery is
toolkit-self: the runner table maps this repo's context names, so an adopter would get CANNOT-GRADE for everything. That is the honest boundary, not an oversight.Both corrections measured by Shipwright while implementing; ② found by running against a fresh clone.
ACs dispositioned — all three DONE, ticked plain. Nothing here is deferred: the battery grades three states, derives its gate list from
branch_protectionsrather than a hand-written list, and reports what it could not grade with a two-state exit.Two follow-ups from the review are tracked on
#1046, neither an AC of this tracker:pre-push-summaryprefix is unpinned. Reproduced onb1c6d71: prefix mutant 0 red against a control at 1 red.Landed in
#1044atb1c6d719.Closing — all three ACs verified against merged
cmd/rt/pre_push.go, not the PR body.Landed in PR#1044, merged
b1c6d719.📌 The coverage gap is deliberately NOT an AC here. AC3 asks the battery to report what it could not grade, which it does — 6 of 12 required contexts named individually. Growing the covered set is #1046, which now carries three ACs: the go-ci runner, the uncapped-lint flags, and the summary-prefix arm.
Two corrections this work made to its own tracker, both measured during implementation and recorded above: exit 2 is this repo's could-not-grade convention at 26
exitError{code: 2}sites, not compose-verify alone; andrt manifest-checkfails on a clean checkout with stale tags — anrc=1that is a could-not-grade wearing a FAIL's exit code, which is why it ships with no runner entry.