A pre-push battery for the locally-runnable CI gates, with could-not-grade as its own state #1037

Closed
opened 2026-08-28 16:43:10 +02:00 by bosun · 5 comments
Owner

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

27  distinct CI contexts on one head
 5  covered by a local `go test` + `bats` run
22  not covered — and most are rt subcommands that run locally:
    register-check · fragment-check · changelog-body-check · manifest-check
    check-self-bootstrap · compose-verify · binary-size-check
    manifest-precheck · ac-closure-check

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-check named the file and line in one command, after CI had already said so.

Why the naive battery self-destructs

rt ac-closure-check exits 1 when run bare, because --owner --repo --pr are 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

  • A pre-push battery covering the locally-runnable gates, distinguishing three states: PASS · FAIL · COULD-NOT-GRADE (missing context, not a red)
  • The gate list is derived from the required-context set, not hand-maintained — a hand-written list goes stale silently and its omissions are invisible
  • Running it with no arguments reports which gates it could not grade and why, and exits non-zero only on a real FAIL

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.

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 ``` 27 distinct CI contexts on one head 5 covered by a local `go test` + `bats` run 22 not covered — and most are rt subcommands that run locally: register-check · fragment-check · changelog-body-check · manifest-check check-self-bootstrap · compose-verify · binary-size-check manifest-precheck · ac-closure-check ``` 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-check` named the file and line in one command, after CI had already said so. ## Why the naive battery self-destructs `rt ac-closure-check` exits 1 when run bare, because `--owner --repo --pr` are 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 - [x] A pre-push battery covering the locally-runnable gates, distinguishing three states: PASS · FAIL · COULD-NOT-GRADE (missing context, not a red) - [x] The gate list is derived from the required-context set, not hand-maintained — a hand-written list goes stale silently and its omissions are invisible - [x] Running it with no arguments reports which gates it could not grade and why, and exits non-zero only on a real FAIL ## 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.
Owner

Measurements for the battery, consolidated — so this is not reconstructed from bus history

All against main at 4fd7907. Three findings, each of which changes a design decision.

1 — Coverage: the gap is that runnable gates go unrun

27 distinct CI contexts
 5 covered by a local `go test` + `bats` run
22 not covered — and most are `rt` subcommands that run locally today

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_protections is right and free. Deriving the command is not:

12 required contexts -> 6 distinct jobs
 8 of 12  derivable   check-self-bootstrap · fragment-check ×3
                      manifest-check ×2 · register-check ×2
 4 of 12  NOT         go-ci/lint+build+test · tests/bats
                      tests/shellcheck · tests/workflow-schema

Those four need go build+test+lint, bats, shellcheck and 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 rt gate invoked bare, exit code and output size:

GATE                   rc   bytes   first line
register-check          0     ...   scanning: scripts .forgejo tests …
fragment-check          0     ...   all fragments carry a supported kind …
changelog-body-check    0     ...   PASS: check 1 (header) …
check-self-bootstrap    0     ...   HEAD is a rolling-PR merge …
binary-size-check       0     786   building release-shape binary …
manifest-check          1     701   a REAL verdict
manifest-precheck       1      35   Error: accepts 2 arg(s), received 0
ac-closure-check        1       0   ← SILENT
compose-verify          2      42   FATAL: --version required

rc=1 carries three different meanings, and only compose-verify uses 2 for could-not-grade. ac-closure-check is the worst case: rc=1 with 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=1 with 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-check as needs-context using a text heuristic (grep -E 'required|must be|usage:') that matched its ordinary build output and overrode its rc=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-check hit 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.

## Measurements for the battery, consolidated — so this is not reconstructed from bus history All against `main` at `4fd7907`. Three findings, each of which changes a design decision. ### 1 — Coverage: the gap is that runnable gates go unrun ``` 27 distinct CI contexts 5 covered by a local `go test` + `bats` run 22 not covered — and most are `rt` subcommands that run locally today ``` 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_protections` is right and free. Deriving the **command** is not: ``` 12 required contexts -> 6 distinct jobs 8 of 12 derivable check-self-bootstrap · fragment-check ×3 manifest-check ×2 · register-check ×2 4 of 12 NOT go-ci/lint+build+test · tests/bats tests/shellcheck · tests/workflow-schema ``` Those four need `go build`+`test`+lint, `bats`, `shellcheck` and 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 `rt` gate invoked bare, exit code and output size: ``` GATE rc bytes first line register-check 0 ... scanning: scripts .forgejo tests … fragment-check 0 ... all fragments carry a supported kind … changelog-body-check 0 ... PASS: check 1 (header) … check-self-bootstrap 0 ... HEAD is a rolling-PR merge … binary-size-check 0 786 building release-shape binary … manifest-check 1 701 a REAL verdict manifest-precheck 1 35 Error: accepts 2 arg(s), received 0 ac-closure-check 1 0 ← SILENT compose-verify 2 42 FATAL: --version required ``` **`rc=1` carries three different meanings**, and only `compose-verify` uses `2` for could-not-grade. `ac-closure-check` is the worst case: **`rc=1` with 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=1` with 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-check` as needs-context using a text heuristic (`grep -E 'required|must be|usage:'`) that matched its ordinary **build** output and overrode its `rc=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-check` hit 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.*
Author
Owner

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 rt subcommand derivable from the job namecheck-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 need go 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)

ran clean bare       register-check · fragment-check · changelog-body-check · check-self-bootstrap
real verdict bare    manifest-check(1) · manifest-precheck(1) · ac-closure-check(1)
NEEDS CONTEXT        compose-verify(2) · binary-size-check

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-check exits 1 when run bare, indistinguishable from a real failure, while compose-verify exits 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:

  • Derive the required-context list from branch_protections; REFUSE on any context with no known runner
  • Three states — PASS · FAIL · COULD-NOT-GRADE — determined by the battery's own knowledge of required arguments, NOT by exit code, since 1 means both
  • Start with the four drop-in gates; the rest land as their invocations are specified
## 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 `rt` subcommand 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 need `go 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) ``` ran clean bare register-check · fragment-check · changelog-body-check · check-self-bootstrap real verdict bare manifest-check(1) · manifest-precheck(1) · ac-closure-check(1) NEEDS CONTEXT compose-verify(2) · binary-size-check ``` **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-check` exits **1** when run bare, **indistinguishable from a real failure**, while `compose-verify` exits **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: - [ ] Derive the required-context list from `branch_protections`; REFUSE on any context with no known runner - [ ] Three states — PASS · FAIL · COULD-NOT-GRADE — determined by the battery's own knowledge of required arguments, NOT by exit code, since `1` means both - [ ] Start with the four drop-in gates; the rest land as their invocations are specified
Author
Owner

Two 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, and manifest_postcondition.go names 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-check FAILS on a clean checkout whose tags are stale, because git fetch without --tags does not pull them:

rc=1  local, stale tags
rc=0  after fetching tags

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-check ships 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.

## Two 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**, and `manifest_postcondition.go` names 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-check` FAILS on a clean checkout whose tags are stale, because `git fetch` without `--tags` does not pull them: ``` rc=1 local, stale tags rc=0 after fetching tags ``` **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-check` ships 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.*
Owner

ACs dispositioned — all three DONE, ticked plain. Nothing here is deferred: the battery grades three states, derives its gate list from branch_protections rather 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:

  • go-ci as a runner entry, plus the golangci-lint cap finding — CI's capped invocation under-reports repeated findings, so a runner inheriting the defaults would hand the battery a partial set.
  • One gap in the summary arm — it skips index 0, so the pre-push-summary prefix is unpinned. Reproduced on b1c6d71: prefix mutant 0 red against a control at 1 red.

Landed in #1044 at b1c6d719.

ACs dispositioned — all three DONE, ticked plain. Nothing here is deferred: the battery grades three states, derives its gate list from `branch_protections` rather 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: - go-ci as a runner entry, plus the golangci-lint cap finding — CI's capped invocation under-reports repeated findings, so a runner inheriting the defaults would hand the battery a partial set. - One gap in the summary arm — it skips index 0, so the `pre-push-summary` prefix is unpinned. Reproduced on `b1c6d71`: prefix mutant 0 red against a control at 1 red. Landed in `#1044` at `b1c6d719`.
Author
Owner

Closing — all three ACs verified against merged cmd/rt/pre_push.go, not the PR body.

AC1  three states                pre_push.go — CANNOT-GRADE present, 5 sites
AC2  list DERIVED, not written    :149 requiredContexts() reads branch protection;
                                  :145 --branch flag names the protected branch
AC3  reports what it could not    :345 summaryLine(pass, fail, cannotGrade,
     grade, exits non-zero only        required, covered)
     on a real FAIL

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; and rt manifest-check fails on a clean checkout with stale tags — an rc=1 that is a could-not-grade wearing a FAIL's exit code, which is why it ships with no runner entry.

**Closing — all three ACs verified against merged `cmd/rt/pre_push.go`, not the PR body.** ``` AC1 three states pre_push.go — CANNOT-GRADE present, 5 sites AC2 list DERIVED, not written :149 requiredContexts() reads branch protection; :145 --branch flag names the protected branch AC3 reports what it could not :345 summaryLine(pass, fail, cannotGrade, grade, exits non-zero only required, covered) on a real FAIL ``` 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; and `rt manifest-check` fails on a clean checkout with stale tags — **an `rc=1` that is a could-not-grade wearing a FAIL's exit code**, which is why it ships with no runner entry.
bosun closed this issue 2026-08-28 18:09:13 +02:00
Sign in to join this conversation.
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
frankenbit/release-toolkit#1037
No description provided.