feat(fr-gate): make the FR gate self-clearing and self-explanatory#68
Closed
LukasWodka wants to merge 4 commits into
Closed
feat(fr-gate): make the FR gate self-clearing and self-explanatory#68LukasWodka wants to merge 4 commits into
LukasWodka wants to merge 4 commits into
Conversation
The FR gate is correct about what it checks and awful about when it checks it. It fires the instant a promotion PR opens, which is before anyone has moved the kanban cards, so the first run is structurally doomed — and clearing that red then requires a human to notice it and find the "Re-run jobs" button. Over 60 days: 448 runs, 58% failed, 30% were retried by hand, and the retry passed 96% of the time. The gate alone accounts for 128 of the fleet's 151 same-commit fail→pass events; excluding it, org-wide CI flake drops from 1.26% to 0.20%. Three additions, no change to what counts as ready: 1. fr-gate.yml posts ONE sticky comment naming each blocking item, its Status now, and the Status it needs. Found and updated in place by a hidden marker, so N runs leave one comment, not N. Silent when the gate never failed. The marker also carries the blocking item numbers. 2. fr-pass-comment.yml re-runs the gate on the repo's open promotion PRs after it advances a card, so /fr-pass clears a stale red in a minute. 3. fr-gate-recheck.yml sweeps the org 4x/hour for the drag-on-the-board path, which fires no event at all. It reads the marker, re-checks only those items, and presses re-run only once they have actually moved. Re-running the existing run — not publishing a status — is what keeps the required check "gate / gate" satisfiable by name. The sweep is a trigger, never an authority: the re-run it presses is a full, normal, authoritative gate run that can still fail. Untouched: item discovery, the rank table, pass/fail semantics, the skip-fr-gate override, and the job name the branch protections require. Also guards fr-pass-comment.yml against its own sticky comment, which spells out "/fr-pass" as the remedy and is posted by an account whose author_association passes the collaborator check. Refs tracebloc/backend#930 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
Contributor
Author
|
👋 Heads-up — Code review queue is at 40 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Bugbot caught a real divergence. fr-gate.yml skips items that are not on
the kanban ("⚪ not on kanban, skipping") and can pass with them present.
The sweep counted the same items as blocking, so a card leaving the board
made the targeted path refuse to re-run — and because the age fallback
only ran when there was no marker at all, that PR stayed red until a
human pressed the button. Exactly the failure this change exists to end.
item_status() now separates the two cases it was conflating: "" means
genuinely not on the board (skip it, as the gate does) and "?" means the
lookup failed (conclude nothing, retry next sweep). Collapsing them the
other way — treating every API hiccup as "off the board" — would have
turned each hiccup into a re-run.
The blocked predicate now also carries the gate's exact-match clause, so
the two are the same test rather than merely similar ones.
Also generalises the age fallback into a backstop that applies whether or
not a marker was read: if the heuristic still says "blocked" but the gate
run is older than STALE_HOURS, stop guessing and let the gate answer. That
covers this bug, a stale marker, and any future drift in the duplicated
rank table. Self-throttling — a re-run resets updated_at, so it can fire
at most once per STALE_HOURS per PR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
Second Bugbot finding, also real. Both re-run paths selected the newest *completed* gate run and ignored the possibility of another gate run for the same head being queued or in progress. A label change, a push, or a second /fr-pass seconds after the first all start a fresh run — and both paths would then re-run the older completed one alongside it, producing two runs that answer the same question. Both selectors now report "INFLIGHT" when any fr-gate run for that head is not yet completed, and both skip on it. A verdict already coming is precisely the case where pressing the button buys nothing. Scoped to fr-gate runs by workflow path, so an unrelated workflow running on the same commit does not suppress a legitimate re-run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
Third Bugbot finding. The sweep pulled comments(first: 100) only. The gate posts its sticky on the first failure and thereafter edits it in place, so the marker sits wherever the thread was at that moment — near the top on an ordinary promotion PR, near the bottom on one that was already busy. A first-100 window misses the second case, and the sweep then falls back to the 4h blind path instead of clearing in ≤15 min. Now fetches head and tail as two aliased connections on the same query and looks in both. No extra round-trip. A thread long enough to bury the marker between the two windows still degrades to the age backstop, which is the right floor for a case that cannot be read cheaply. Theoretical today — the org's nine live promotion PRs carry 0-2 comments each — but the marker is the mechanism the fast path depends on, so it should not have a comment-count cliff in it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d7a08ca. Configure here.
This was referenced Jul 26, 2026
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
The FR gate is right about what it checks and wrong about when. It fires the instant a promotion PR opens — before anyone has moved the kanban cards — so the first run is structurally doomed. Clearing that red then requires a human to notice it, open the workflow log to find out which item is at fault, and press "Re-run jobs".
Measured across 60 days:
A check that is red 58% of the time, and that a human fixes by pressing a button that works 19 times out of 20, has stopped being a signal. Nothing here changes what the gate checks. It makes the gate say what it wants and then clear itself.
Live right now, for illustration:
cli#397andclient#383are both on attempt 4,data-ingestors#396on attempt 2 — all still red, all waiting on a human.What ships
1.
fr-gate.yml— one sticky comment that says what blocks itOn a blocked promotion the gate posts a single comment on the PR:
Found and updated in place via a hidden
<!-- fr-gate-sticky …>marker, so N runs leave one comment rather than N. When the gate goes green the comment is closed out; if the gate never failed, nothing is posted at all — the happy path stays silent.2.
fr-pass-comment.yml—/fr-passclears the red immediatelyAdvancing a card is exactly the input the gate reads, so after a successful advance this now re-runs the FR gate on the repo's open promotion PRs. This is the highest-value half: it turns the most common unblock action into a self-clearing one, in about a minute.
3.
fr-gate-recheck.yml(new) — a sweep for the drag-on-the-board pathDragging a card on the board fires no event anywhere: GitHub has no trigger for a Projects v2 field change, and a promotion PR has no event of its own. A cron is the only mechanism available. It runs centrally in this repo (same shape as
auto-classify.ymlandkanban-reconcile.yml), four times an hour at off-peak minutes.It is not wasteful, because it is not blind: it reads the blocking item numbers out of the sticky marker, re-checks only those, and presses re-run only once they have actually reached the required column. A genuinely-blocked PR costs one read per sweep and zero re-runs. There are ~9 open promotion PRs org-wide at any time, and this repo is public, so the Actions minutes are free.
Design decisions
Re-run the existing run; never publish a status. Branch protection requires the check by name (
gate / gate— verified oncli'smain). Only a real run of that job satisfies it; a synthetic commit status of the same name would sit alongside the failed check run, not replace it. Re-running is also precisely what humans do today — the automated behaviour is already understood and already known to work 96% of the time.The sweep is a trigger, not a second gate. Nothing in
fr-gate-recheck.ymldecides whether a promotion may merge; it only decides whether pressing re-run is worth a minute. The re-run it presses is a full, normal, authoritative gate run that can still fail. So the worst a bug in that file can do is re-run too often (noise) or too rarely (today's status quo). It cannot let an unready item through. That invariant is written at the top of the file in block capitals for whoever edits it next.Read the marker instead of re-deriving the item list. Duplicating the commit-subject discovery logic in a second workflow is the one change here that could actually diverge from the gate. So the gate publishes what it blocked on, and the sweep consumes it. The rank table is duplicated — but only as a "is it worth re-running yet" heuristic, annotated as such.
Predict the gate, don't re-implement it. The sweep's item check is the gate's own predicate: at-or-beyond by rank, with the same exact-match clause, and — importantly — the same treatment of items that are not on the kanban at all. The gate skips those (
⚪ not on kanban, skipping) and can pass with them present, so the sweep must not count them as blockers or it would strand a PR the gate would happily let through.item_status()therefore distinguishes "the query succeeded and this item is genuinely off the board" from "the lookup failed", which are not the same fact and must not produce the same decision.Never press the button when a verdict is already coming. Both re-run paths skip when any
fr-gaterun for that head is queued or in progress. A label change, a push, or a second/fr-passseconds after the first all start a fresh run, and re-running the older completed one alongside it would just produce two runs answering the same question. Scoped by workflow path, so an unrelated workflow on the same commit does not suppress a legitimate re-run.Backstop. Whenever the heuristic above still says "blocked" — including when there is no marker to read at all — and the red run is ≥4h old, the sweep stops guessing and re-runs the gate to let it answer for itself. That covers a stale marker, a repeatedly-failing lookup, and any future drift in the duplicated rank table, without this file needing to know every rule the gate has. Self-throttling: a re-run resets the run's
updated_at, so it fires at most once per 4h per PR. This is also what makes the activation gap below harmless.The PAT, not
GITHUB_TOKEN, posts the comment. A reusable workflow can only narrow the caller's permissions, never widen them. Relying onpull-requests: writewould mean editing the caller in all ~17 repos and would silently no-op wherever the org default is read-only.PROJECTS_KANBAN_TOKENis already inherited everywhere this gate runs. No caller repo needs any change.Loop guard. The sticky comment spells out
/fr-passas the remedy and is posted by an account whoseauthor_associationpasses the collaborator check — so without a guard, the gate would advance the promotion PR's own card the moment it explained why the promotion was blocked.fr-pass-comment.ymlnow skips comments starting with the sticky marker. Only the initial POST could ever reach it; later runs edit in place, and that workflow subscribes tocreatedonly.Considered and rejected
workflow_dispatchon the caller — the callers arepull_request-triggered; there is nothing to dispatch.pull_requestevent — the promotion PR does not change when a card moves. No event exists.Blast radius — deliberately untouched
This is consumed by ~17 repos, so the diff is purely additive: 557 insertions, 0 deletions.
git log --format='%s'extraction and its fallback) — not a character.rank()table and the at-or-beyond comparison — not a character.continue-on-errorand cannot turn a blocked promotion green, or a clean one red.skip-fr-gateoverride — byte-for-byte as it was; the sweep explicitly leaves labelled PRs alone.gate, i.e. the required check namegate / gate. No new job was added to the reusable workflow, so no new check appears anywhere.permissions:onfr-gate.yml— hardening it would change inherited permissions in 17 repos for no benefit here.Activation order (worth knowing)
The callers pin
tracebloc/.github/.github/workflows/…@main, but this repo's default branch isdevelop, and scheduled workflows only run from the default branch. So on merge todevelop:No action needed, just don't be surprised by the staggering.
actionlint
Run locally with
actionlint 1.7.12 -no-color -oneline -shellcheck shellcheck(shellcheck 0.11.0), matching #66's CI invocation.develop(baseline)fr-gate-recheck.yml(new file, 305 lines)Zero findings introduced. The two
# shellcheck disable=SC2016directives in the new file are per-line with reasons, matching #66's stated convention — both are GraphQL variables that must not expand in shell.Overlap with #66 / #67
fr-gate.ymlhunk is a shellcheck comment ~55 lines above my nearest change; itsfr-pass-comment.ymlhunk is an output-grouping change ~20 lines above my new step. Either merge order works. If ci(actionlint): lint this repo's own workflows + clear the 29 pre-existing findings #66 lands first I will rebase and re-verify rather than assume.kanban-closure-router.yml) — no file overlap. Test-merged clean.kanban-closure-router.yml; no overlap with this PR either.Testing
The shell was extracted from each
run:block and executed locally against a mockedgh, since this ships to ~17 repos. 12 sweeper scenarios plus the/fr-passpath, all green:$GITHUB_OUTPUT— confirmed the multi-line heredoc round-trips and is still written when the step exits 1.skip-fr-gate→ skipped; gate already green → skipped; no marker + 1h → wait; no marker + 9h → re-run; forged marker containing shell metacharacters → refused, no re-run, no injection (only digits are ever passed onward; anyone can comment on a public repo).MAX_RERUNSandDRY_RUNboth verified./fr-passstep — re-runs red gates, ignores green ones, ignoresdevelop-targeting PRs, falls back to the second credential when the first lacksactions:write.Also dry-run against the live org (read-only): it enumerated all 9 open promotion PRs, correctly identified which carry a red gate run, and applied the age rule per PR.
One real bug was found by the harness itself: the original age calculation used GNU
date -d, and a malformed value made the loop exit early while still reporting success. It now computes the age inside the samejqcall that selects the run, so it is one integer out of one command.Bugbot
Three findings, all genuine, all fixed and replied to in thread: gate-parity on not-on-kanban items (5519d35), duplicate re-runs while a verdict was already in flight (1731754), and the sticky marker's comment-window cliff (d7a08ca). Each fix came with regression fixtures.
Rollback
Delete
fr-gate-recheck.ymland revert the two additive hunks. The gate returns to exactly its current behaviour.Part of the CI/pipeline health work in tracebloc/backend#930.
🤖 Generated with Claude Code