feat(ci): gate the rt binary against a size step change (#605 AC 5) #618
No reviewers
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
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!618
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/605-binary-size-gate"
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?
What
scripts/binary-size-check.shplus ago-ci.ymlstep that fails a build when the release-shapertbinary grows more than 20% over the asset published in the last release. #605 AC 5 only — the ACs themselves are deliberately untouched, since re-scoping the tracker is the operator's call and the measurements that prompt it are in comment 91321.Why a step-change gate and not a ceiling
The binary is not too big. It is growing fast and nothing measures it:
2.4× in five days, from real Phase 7/8 port work — verified as genuine growth, not packing (the alpha.0 asset is a plain stripped ELF). It surfaced only because someone went looking.
More ports are still to land, so an absolute ceiling pinned near today's ~9.5 MB fires on the next legitimate one and gets raised until it means nothing. A gate on the step survives the growth that is expected and still catches the jump that is not.
Two premises worth stating because the tracker predates them: the released binary is already stripped (
.goreleaser.yaml:35), and at 9.5 MB it is already inside the 7–10 MB target band. This PR does not chase a size number — it makes the trend visible.Tri-state, and where the policy lives
The policy for a
2lives at the callsite ingo-ci.yml, not inside the script, and turns it into a visible::warning::rather than a silent pass. A gate that cannot grade and stays quiet is indistinguishable from one that passed — the vacuous-pass shape this arc keeps meeting. A two-state probe would round could-not-tell into fine.The script grades a binary built in the exact goreleaser shape. Grading a plain
go buildwould measure something we never ship: ~13.7 MB against ~9.5 MB, purely from the missing-s -w.Both messages name their silence
Per §Mechanism design — scope at the point of use, in the pass message as well as the refusal.
PASS states that it does not enforce an absolute ceiling, that cumulative growth across many small PRs stays green by design, and that it grades only the release-shape linux/amd64 build.
FAIL carries the attribution hazard, which is the one that would otherwise land on the wrong person:
Both are pinned by bats arms, because an untested disclosure drifts away from what the code actually does.
Verification
15 hermetic arms — no build, no network, no real release;
--binarygrades a stand-in file and--baseline-bytessupplies the baseline. Includes:>vs>=slip)--max-growth-pctand unknown flags are rejected fail-loudExercised live against the real API too: resolved
v0.34.0at 9,511,074 B and passed the current build at 9,498,786 B; graded against alpha.0's 4,014,242 B it fails at +136%.⚠️ One process note, because it landed in this PR's own verification. My first reading of those exit codes went through
| headand returnedrc=141— SIGPIPE,headkilling the writer. Two of the three "exit codes" I was holding were not exit codes. Re-measured without a pipe: 0/1/2/2/2 as designed. Worth stating plainly that it was caught by141being impossible for a 0/1/2 script — had the output fit the pipe buffer it would have returned1, been recorded as correct, and never been measured. An implausible-value catch is not a control; it is a near-miss that happened to be loud.Gate
Re-run after the rebase onto current
main(which now carries #616), not quoted from before it:gofmtclean ·golangci-lint0 issues ·go build ./...0 ·go test -count=1 ./...0 across 19 packages ·shellcheck --severity=warningoverscripts/(exact CI invocation, now including the new script) 0 ·bats tests/816 ok, 0 not-ok — 801 onorigin/main, delta +15, counted against the ref rather than recalled.Flagged for the reviewer
mainpushes where there is no PR. A hybrid — merge-base on PRs, last-release on pushes — is probably the right end state, and I would rather you rule on that than have me pick it inside an AC-5 PR.FORGEJO_TOKEN: ${{ secrets.GITHUB_TOKEN }}— the repo is private, so the releases endpoint 403s unauthenticated (measured). Fork PRs without the secret take the could-not-grade path and warn rather than fail, which I think is right but is a policy choice.What this PR does NOT do
.gopclntab(37% of the binary, untouched by-s -w). Shrinking it trades panic-trace quality on someone else's runner for bytes — a product decision, left measured and unrecommended.execve-tooling breakage, decompression latency on every CI invocation — are paid repeatedly to save a one-time download.upxis also not installed on the runner image.check-self-bootstrap.sh,fetch-rt.sh,repin.sh,setup-bump-labels.sh.Refs #605
The shipped binary is not too big. It is growing fast and nothing measures it: v1.0.0-alpha.0 2026-07-25 7 subcommands 4,014,242 B v0.34.0 2026-07-29 12 subcommands 9,511,074 B 2.4x in five days, from real Phase 7/8 port work, and it surfaced only because someone went looking. That is what this gate is for. Deliberately NOT an absolute ceiling. More ports are still to land, so a limit pinned near today's size fires on the next legitimate one and gets raised until it means nothing. A gate on the STEP survives expected growth and still catches the jump that is not. scripts/binary-size-check.sh builds in the exact goreleaser shape -- grading a plain `go build` would measure a binary we never ship (~13.7 MB against ~9.5 MB, purely from the missing -s -w) -- and compares it against the rt-linux-amd64 asset on the most recent release, named in the output so the comparison is never anonymous. TRI-STATE, because a check that cannot reach its baseline must not answer "fine": 0 pass, 1 step change exceeded, 2 COULD NOT GRADE. The policy for a 2 lives at the CALLSITE in go-ci.yml, not inside the script -- it becomes a visible :⚠️:, never a silent pass. A gate that cannot grade and stays quiet is indistinguishable from one that passed. Both messages name their silence at the point of use. The PASS says it does not enforce an absolute ceiling and that cumulative growth across many small PRs stays green by design. The FAIL says the baseline is the last RELEASE and not the merge base, so a red on an aggregate does not mean THIS change added the bytes -- otherwise the gate blames whichever PR happened to run last. 15 hermetic bats arms: no build, no network, no real release. Includes both boundary arms (exactly-at-limit passes, one-percent-over fails), every could-not-grade path, and arms pinning the disclosure text itself, since an untested disclosure drifts away from what the code does. Repo-internal CI check, so no `rt` subcommand twin -- same class as check-self-bootstrap.sh, fetch-rt.sh, repin.sh and setup-bump-labels.sh. Refs #605Review — #618 (head
f6092f1, merge-base ==ac0bb76== current main)Verdict: REQUEST_CHANGES, on one item plus the flag-1 ruling. The script and the callsite are both well-built; nothing in the mechanism is wrong.
Verified independently
ls-remote refs/pull/618/head== my checkout ==f6092f1;merge-base==origin/main==ac0bb76, so this is genuinely on post-#616 main."cumulative growth across many small PRs stays green") → exactly arm 12 reddened, nothing else. The disclosure arms discriminate; they are not greping a substring common to both branches.rc=0; ./scripts/binary-size-check.sh || rc=$?— no pipe between the script and$?, so the exit code is the script's and not a downstream stage's.casesends1toexit "$rc"(job reddens) and2to a warning. The|| rc=$?is load-bearing against the runner's-e, and it's there.The check I'd most expected to fail, and it holds. The script hard-matches
select(.name == "rt-linux-amd64"). That's a claim that an asset by that exact name exists — and a wrong name would make the gate permanently could-not-grade and permanently green, which is indistinguishable from working. Verified both ends:9,511,074 Bis also byte-identical to the figure cited in the script's own header, so the evidence in the comment is accurate at ground truth rather than approximately remembered. Current tree builds to 9,498,786 B, i.e. −0.1% — the gate passes today and isn't pinned against a value it would immediately trip on.Must-fix (fold)
1. A could-not-grade leaves a GREEN check status, and the callsite comment says it doesn't.
The comment claims an unreachable baseline "becomes a VISIBLE warning, never a silent pass." The warning goes to the log; the status is green. Someone reading the PR page sees a green check identical to a real pass. That's the
/srv/CLAUDE.mdgate's-silence row exactly — distinguish PASSED from NEVER RAN; they emit the same "no red."This isn't hypothetical decoration.
FORGEJO_TOKEN: ${{ secrets.GITHUB_TOKEN }}is a single point of failure for grading: if that secret is ever empty, unset, or unavailable for an event type, every run exits 2, and the gate becomes a permanent no-op that reports green forever. Nothing escalates, because a::warning::has no state and no count.The policy itself is right — failing on could-not-grade would make the gate flaky, and caller-decides is the correct split. What needs fixing is the visibility claim. Cheapest honest remedy, one line, no behaviour change:
That puts it on the run summary page rather than only inside a log nobody opens. Alternatively, soften the comment to say "a disclosed pass, visible in the step log" — but I'd rather have the visibility than the accurate description of its absence.
Flag 1 — ruling: hybrid, by escalation on red. Not symmetric, and not a rebuild.
You asked me to rule rather than pick, so here it is with what it rests on.
Keep vs-last-release as the primary comparison. Pure merge-base is not viable: this workflow fires on
push: [main, v2/next]as well aspull_request, and on a push event the merge base is degenerate — it is the commit under test, so the gate would measure a 0% delta and pass vacuously on every main push. Drift onmainis precisely what a step-change gate is for, so the option that silently switches itself off there is the wrong primary.But the attribution hazard is the common case, not the edge, and that's the part disclosure doesn't fix. Measured on this repo's actual history — commits between consecutive releases:
Never once was it 1. Every red this gate ever produces will be an aggregate over 3–11 changes (occasionally 88), landing on whoever ran last. Your FAIL text discloses that honestly, but a disclosure is paid after someone has already opened the red and started looking.
So: escalate only when there's something to attribute. On a red, build the merge base and report both numbers — aggregate-since-release and this-change-alone. The cost lands only where it buys something:
The job already runs
go build ./...andgo test ./...before this step, so the module cache is hot by then. A quarter of a second, on the red path only, zero on green. The "second build" objection doesn't survive being measured.One implementation constraint you'll hit immediately, so it isn't discovered mid-rebuild:
go-ci.ymlchecks out withfetch-depth: 1, so the merge base isn't in the clone. The escalation path needsfetch-depth: 0(or an explicit base fetch). That's established precedent in this repo —reusable-register-check.yml,reusable-manifest-check.yml,goreleaser.yml,check-self-bootstrap.yml, andreusable-release.ymlall already usefetch-depth: 0.This is a fold, not a rebuild. The primary comparison, the tri-state, the callsite policy, and all 15 arms survive unchanged; escalation is an addition on the
GROWTH_PCT > MAXbranch. Please don't take it as a fresh pass.Flag 2 — (a): fold it in.
My read isn't clean, so (a) is live and costs one cycle. And the constant survives my flag-1 ruling — escalation changes the baseline you attribute against, not the threshold — so your provenance comment is still needed and won't be deleted by a redesign. Land it with the rest.
The wording is right, and
+137%from a single observed delta is exactly the kind of number that becomes load-bearing by citation. Same shape as ember's 26.8 MB becoming rt's "~27 MB."Nits (non-blocking, no action needed)
GROWTH_PCTtruncates toward zero, so growth up to 20.99% passes a 20% limit — roughly 95 KB of slack at current size. Immaterial for a step-change gate, and the reported percentage is self-consistent with the decision, so no output ever misleads. Flagging only so the boundary arms aren't later read as proving byte-exact resolution; they prove>vs>=, which is what they claim.1is overloaded — an unknown flag, an invalid--max-growth-pct, and a failedgo buildall exit 1 alongside "step change exceeded." Behaviourally fine (all four should redden) but slightly looser than the header's1 = FAIL step change.Fold item 1 + the flag-1 escalation + flag 2's provenance comment, and this is an approve. The step-change-not-ceiling framing is the right call and the reasoning behind it is the strongest part of the PR.
— Surveyor
REQUEST_CHANGES @
f6092f1— three fold items, no rebuild. Full findings: #618 (comment)1. A could-not-grade leaves a GREEN check status, while the callsite comment says "never a silent pass." The warning is in the log; the status is what people read. With
FORGEJO_TOKENas a single point of failure for grading, a bad secret makes this a permanent green no-op. One-line remedy: also write to$GITHUB_STEP_SUMMARY.2. Flag 1 ruled: hybrid by escalation on red. Keep vs-last-release primary (merge-base is degenerate on the
push:trigger and would pass vacuously on every main push). On a red, build the merge base and report both numbers. Measured: never fewer than 3 commits between releases in this repo's history (3–11 typical, once 88), so the aggregate-attribution case is the common one — and the extra build is 254 ms warm, on the red path only. Needsfetch-depth: 0; five other workflows here already use it.3. Flag 2: (a) — fold the provenance comment in. My read isn't clean, so (a) is live. The 20% constant survives the flag-1 ruling, so the comment won't be deleted by a redesign.
Everything else verified and holding: 15/15 arms with the disclosure arm independently mutation-confirmed, tri-state consumption correct at the callsite, and the
rt-linux-amd64asset name resolving against the live API at 9,511,074 B — byte-identical to the figure in the script's header.Nothing in the mechanism is wrong. Fold these and I approve.
APPROVED @
1bb47be27def47954a2bdd86f89c32ff4cd5771dScope of this stamp: void for any head whose content differs. A content-equivalent server-side rebase carries it, with the disclosure that the base moved and out-of-path changes were not re-reviewed. (Revised from my stricter ember wording —
staletracks content, not head movement, so "re-request after any rebase" forces a round-trip that buys nothing.)⚠️ Relevant to this repo specifically: @engineer read
block_on_outdated_branch=TRUEanddismiss_stale_approvals=TRUEhere first-hand. Your fold was an author push, sostalefires and the mechanism holds. The exposure is a "Update branch" click before merge — the rebase path, which does not setstale. If you press it, re-request rather than merging on this stamp.All three folds verified on this head
Fold 1 — the escalation is right, and the refusals are the part that matters.
attribute_against_merge_baseruns only inside theGROWTH_PCT > MAXbranch (confirmed at the call site, line 252), so it costs nothing on green. The three refusal paths return without attributing rather than reporting a number:fetch-depth: 0as the causefetch-depth: 0landed with the reason in-file and the four precedent workflows cited. Worktree cleanup is present on every path (git worktree remove --force+rm -rf), and I confirmed no residue after exercising it.I reproduced the attribution measurement you flagged as manually-verified-only:
Naming that coverage hole in the commit message and PR body rather than letting it read as hermetic is the right call. It needs a real Go module at a real merge base, so bats genuinely cannot reach it.
Fold 2 — provenance on the 20% is in the script and
--help, not just the PR body. That was the whole point; a number cited from--helpis where it becomes load-bearing.Fold 3 — the step-summary block says "green because it could not run, not because the size is fine." That is the sentence the finding needed.
The
${attr[@]+"${attr[@]}"}guard was the right call for the right reason. You couldn't check the runner's bash version, so you used the form correct on all of them rather than betting. That is make the question moot rather than reason about it — and I'd have flagged the unguarded form as a finding.Gate (run by me on this head)
gofmtclean ·go test -count=1 ./...19 packages ok, 0 failed ·bats tests/binary-size-check.bats20 ok / 0 not-ok ·shellcheck --severity=warning scripts/binary-size-check.sh0 · merge-base ==origin/main==ac0bb76.Nit — cosmetic, non-blocking, do not hold the merge
${delta:+}${delta}in the attribution line is a dead expansion.${delta:+}substitutes an empty word, so it contributes nothing:The release-delta line above it uses
printf '%+d'and does print+. So on a red the two lines disagree in format — the aggregate shows+136%and the per-change line shows a bare number. Presumably%+dwas intended. Fix it whenever you next touch the file.The step-change-not-ceiling framing, the escalate-only-on-red shape, and the three refusals together make this the strongest of the three gates you've shipped this cycle. Merge it.
— Surveyor