Uh oh!
There was an error while loading. Please reload this page.
sec(2157): make "no unscoped App-token mint" a checkable property - #287
Conversation
The 19 add-to-kanban copies are scoped and the audit is clean (run 32322964366,
0 findings). Nothing stops the twentieth mint arriving the way the first nineteen
did -- nine workflows acquired a full-permission mint in one day because the first
one did and the rest were copied from it. Least privilege applied by hand is a
STATE; this is the part that makes it a property.
WHAT THE GUARD DECIDES, AND WHAT IT REFUSES TO GUESS. A mint with no
`permission-*` input carries the App's FULL installation grant -- contents +
pull-requests + issues + org-projects write across every installed repo, and the
App holds `bypass_reviews` on staging and prod fleet-wide, so the blast radius is
merge-past-review rather than merely write. Whether the scopes a workflow DOES
name are the right ones is a question only its own API calls can answer, and a
guard that claimed to know would be restating rather than deriving. "Asked for
nothing at all" is decidable, and is what was actually going wrong.
DERIVED, NOT RESTATED. The workflow list is the directory; mint steps are found by
parsing YAML and comparing the `uses:` value up to the `@`. Add a workflow and it
is covered. Bump the action's pin and it is still checked -- there is a test and a
mutation for exactly that, because an exact-string match including the version is
the obvious wrong implementation.
FAIL CLOSED, INCLUDING ON ZERO. An unparseable workflow, a missing directory and
ZERO MINTS FOUND are all hard errors. The last one matters most: this file's
premise is that mints exist, so finding none means the matcher broke, not that the
fleet got clean.
ARMED GREEN OVER 12 PRE-EXISTING OFFENDERS. Each is exempted BY NAME WITH A
REASON, so this lands green instead of as a red gate that trains people to skip
the tier. `fr-gate` is a required check on every promotion branch fleet-wide and
three others read branch protection, where a narrower token is known to return
LESS data rather than an error -- that is why caller-drift keeps a PAT at all
(ruleset `bypass_actors` is returned only to a write-access caller). Downscoping
those needs measurement per workflow; it is not a thing to guess at across 13 files
in one commit.
AND A STALE EXEMPTION IS ALSO A FINDING. Without that half the list stops being a
burn-down and becomes cover: a full-grant mint re-introduced into an exempted file
would be admitted by a row written years earlier for a different reason.
13 cases. 8 mutations, each verified applied AND caught. TWO OF THOSE MUTATIONS
WERE UNCAUGHT ON THE FIRST RUN, and the run is how I found out rather than review:
* "a missing `with:` counts as scoped" survived the whole suite, because every
fixture happened to supply a `with:` block;
* "the action match becomes a substring test" survived, because the look-alike
fixture (`evil/actions-create-github-app-token`) does not contain the real
name -- the hyphen breaks it -- so it could not distinguish `in` from
`split("@")[0] ==`. Replaced with `myorg/actions/create-github-app-token`,
which a substring test does wrongly match.
Both gaps are now cases. Also: two suite cases initially failed by measuring
PRODUCTION state -- without an explicit exemption override the live 12 rows all
read as stale against a fixture directory. That failure was the stale-exemption
check working on the wrong input, and the helper now says so where someone would
otherwise repeat it.
ALSO IN THIS COMMIT: repo-inventory.yml's `divergent` reason for release-train's
add-to-kanban copy said its header "is now the ONLY difference". That was false in
two ways -- the copy also lacked the least-privilege scoping until
release-train#93, and it carries a second wanted comment (the Dependabot note) the
reason never mentioned. Rewritten to name both, and to state the claim in a form
someone can check: strip every comment and blank line from that copy and from the
canonical one and they are byte-identical.
make check green: lint (incl. the new gate), 13 mint-scope cases, all selftests,
coverage gate, actionlint.
Refs backend#2157.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Uh oh!
There was an error while loading. Please reload this page.
… contamination
Two things this branch needed before it could go out.
1. THE DIFF CARRIED SOMEONE ELSE'S WORK. This branch was cut from the #2114
branch, so `scripts/bricked-prs.py` and its selftest showed as modified. #2114
has since merged (.github#282) with three rounds of review changes on top of the
snapshot here. Resolved by taking develop's version of both files outright --
verified `git diff --cached origin/develop` on them is EMPTY -- so the diff is
now #1979's own six files and nothing else.
2. IT ADDED A NEW UNSCOPED APP MINT. `stale-backlog.yml` minted with `owner:` and
no `permission-*`, i.e. the App's full installation grant, in the same week
backend#2157 swept 19 files to remove exactly that. Shipping a new one while a
guard against them is in review (.github#287) would have been the twentieth mint
arriving the way the first nineteen did.
Scoped from what the sweep actually CALLS, not from what the App holds:
issues: write `issue edit --add-label`, `issue comment`,
`issue close` (stale-backlog.py:188-192)
organization-projects: read ISSUES_Q reads `projectItems .. Status`; the
sweep never writes a board field
No `pull-requests` -- this workflow does not touch PRs by design (backend#1408).
No `contents` -- the App token is only ever `GH_TOKEN` for the sweep; the
`.github` checkout uses the workflow token with `persist-credentials: false`.
`repositories:` narrows the issue writes to the caller's own repo, which is
precisely the sweep's scope (`--repo "${{ github.repository }}"`). Without it a
DESTRUCTIVE, UNATTENDED sweep would hold issue-close rights across all 19 repos
while only ever closing in one.
Org-projects being unaffected by repo scoping is MEASURED rather than assumed:
backend#2181's verification run 32255581084 wrote the org project with
`repositories:` set. That was the load-bearing unknown and it is settled.
PROVED AGAINST THE GUARD ITSELF, not against my reading of it. Running #287's
mint-scope.py over this tree: 14 mint steps, 12 unscoped (all pre-existing and
exempted), 0 findings. So this branch passes the gate that is still in review --
which is the whole point of adding the gate before the burn-down rather than after.
make check green: 41 selftests, coverage gate, action-pins 34/0, actionlint 0.
Refs backend#1979, backend#1597 item 1, backend#2157.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Bugbot, High, and it is the failure mode this repo's own rule 2 names -- shipped in
the PR that added the guard.
`mint-scope` was wired only into `make lint`, which CI does not run. The required
contexts on develop are actionlint, gate, quality/{gitleaks,house-rules,ruff,
shellcheck,action-pins} and selftests -- and `selftests` runs the FIXTURE suite,
never the production audit. So a new unscoped `create-github-app-token` mint could
merge with everything green. A guard in a non-required job is advice; this one was
not even in a job.
`action-pins` was cited in the code as the peer that asks a structurally identical
question, and Bugbot's sharpest point is that action-pins IS enforced in CI, which
is exactly what made the comparison misleading rather than merely incomplete.
ADDED TO THE EXISTING `selftests` JOB rather than as a new context, and that is the
deliberate half: `selftests` is already required on develop/staging/main, so this
arms the gate with NO branch-protection edit. A new context would sit unrequired
until somebody armed it -- which is precisely how a guard ends up advisory, and
"never land a red gate" is the other half of the same rule.
BOTH HALVES NOW RUN IN THAT CONTEXT, and they answer different questions:
make selftests the rule CATCHES 13 fixture cases, 8 mutations
make mint-scope the fleet COMPLIES 13 mints, 12 exempted, 0 findings
Either can pass while the other fails, in both directions, so neither substitutes
for the other. The Makefile's CI-job map now carries that line, since that map is
what a reader uses to conclude a local pass predicts CI.
make check green.
Refs backend#2157.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>LukasWodka
commented
Aug 20, 2026
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 b9a7629. Configure here.
Bugbot, High, and it makes the previous commit worse than a no-op. `read_repo` read
`meta.get("release_train")`, but production `meta` comes from `list_active_repos`,
which returns ONLY:
{"visibility": ..., "default_branch": ...}
No `release_train`. So the flag was always None -> False, EVERY repo was audited on
its default branch, and develop-first was silently deleted for the train repos it
exists for. Today that happens to be harmless -- every train repo currently defaults
to `develop`, measured -- but the policy was gone, and the next repo to default to
main/master would have under-reported work in flight with nothing saying so.
THE SELFTEST PASSED BECAUSE THE FIXTURE CARRIED A KEY PRODUCTION NEVER SETS. I added
`release_train: True` to META in the same commit that started reading it from there.
A fixture richer than the real payload is a test asserting its author's assumption,
and this is the THIRD time that shape has appeared in this epic -- `is_bot` on #282,
the look-alike action name on #287, this.
THE FIX IS A PARAMETER, NOT A LOOKUP. `on_train` is required and positional, so a
caller that forgets it raises TypeError; defaulting it to False would reproduce the
bug with better manners. `main()` passes `bool(entry.get("release_train"))` from the
INVENTORY row -- schema-required, and cross-checked against release-train/repos.yml
by load_release_train -- so the ref this guard reads is derived from a fact it
independently verifies.
META IS NOW EXACTLY THE PRODUCER'S SHAPE, and a case asserts that by reading
`list_active_repos`'s own source rather than a list written in the test: it writes
`visibility` and `default_branch` and nothing else. That case reddens if the producer
starts setting `release_train`, which is the only honest way to keep the two in step.
AND THE WIRING IS PINNED SEPARATELY, because the behavioural cases could not see it.
They call read_repo directly, so mutations hardcoding the argument to True or False
left all 196 green -- and #289's bug WAS the wiring, not the resolver. A source
assertion on main() closes it. Weaker than behavioural and said so in the comment:
driving main() needs the org listing, the inventory and the train file stubbed
together, which this suite has no harness for.
That assertion was ALSO wrong on its first attempt: a paren-matching regex allowing
one level of nesting could not match `bool(entry.get(...))` across two lines, so it
found nothing and failed on the correct code while every mutation "passed". An
extractor that cannot find the thing reports the same as a defect. Line-based now.
7 mutations, all applied and caught:
on_train read from meta again 194/2 the train-branch case
call site passes False 196/1 the wiring case
call site passes True 196/1 the wiring case
call site reverts to meta 196/1 the wiring case
plain develop-first restored 193/1 the non-train case
always the default branch 192/2 the train case
list_active_repos sets release_train 194/2 the producer-shape case
197 cases green. make check green.
Addresses Bugbot on .github#289.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>* feat(2114): a missing Bugbot review is a finding, not a clean PR (backend#2114)
Cursor Bugbot's auto-trigger silently drops PRs. Measured 2026-08-17: five open PRs
across cli, release-train and .github carried no `Cursor Bugbot` check at all, while
PRs opened minutes before and hours after were reviewed normally.
No discriminator survived. Not diff size -- a 1-file/66-line PR was reviewed and a
1-file/29-line one was not. Not file type: two 3-file Go diffs in the same repo, one
reviewed, one not. Not repo, not time (10:54 absent, 10:55 present), not quota (PRs
hours later were fine), not author or draft state. Posting `bugbot run` started a
review within a minute and both came back clean -- the reviews were not failing,
they were never starting.
WHY IT BELONGS IN THIS WATCHER. bricked-prs.py exists because a required check that
never reports "is the one CI failure mode with NO red signal at all -- nobody is
notified, and no reviewer sees a problem, because there is no failure, only an
absence. So it needs a watcher; nothing inside a PR can detect it." That paragraph
describes this bug exactly, one class over: checks then, the reviewer now. A per-PR
required check would be wrong here -- it would sit pending for the minutes Bugbot
legitimately takes, which is the thing this repo refuses to ship.
Four rules, each earned from the measurement:
- a SKIPPED Bugbot counts as PRESENT. It ran and decided; that is a verdict. Only
total absence is the silent drop.
- bot authors are exempt, keyed on GitHub's `[bot]` suffix rather than a list of
names that would go stale. dependabot[bot]'s absence was the one legitimate case
in the sample.
- the same young-head rule as the required-check case, and an unreadable age reads
as young: a false finding is what makes a report ignorable.
- reported EVEN WHEN every required check is present, because that is precisely
the case that renders as a completely clean PR.
AND ITS OWN LABEL. The renderer sent anything not `conflicted` to "BRICKED", so a
missing review would have been announced as a bricked PR -- sending the reader to
branch protection for something one comment fixes. It renders as UNREVIEWED, names
`bugbot run` as the remedy, and the summary now says the quiet part: a bricked PR
cannot merge, an unreviewed one merges perfectly well, which is the worse of the two.
That rendering is asserted from the source, because it lives in main() and no
decision-table case reaches it.
Six mutations, all caught: the check removed, the bot exemption disabled, the
young-head guard dropped, the cause collapsed into `never-reported`, the label
collapsed to BRICKED, and the remedy text removed.
Verified: bricked-prs selftest 21 passed (was 14); `make selftests` 41 passed;
actionlint clean.
Closes backend#2114
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(1979): stale-backlog becomes a board-aware reusable (backend#1979, #1597 item 1)
THE BUG. `actions/stale` exempts on LABELS ONLY. It has no concept of the board, so
an item in `North Stars`, `Ready for prod` or `In progress` went stale and auto-closed
after 8 weeks of silence exactly like a Backlog item -- a strategic priority archived
because nobody commented on it. `kanban-reconcile.yml` already carries a "Shield
North Stars from the stale sweep" step, which is the workaround admitting it.
WHY THE COPY MODEL HAD TO GO FIRST. Board awareness cannot be expressed as a label,
so it cannot be a config knob on an action -- it needs a script, and a script cannot
be maintained as sixteen byte-identical copies. Fixing the copy model was the
prerequisite, not the goal. That is the whole content of #1979.
THE RULE IS AN ALLOW-LIST OF ONE: eligible only when the board Status is exactly
`Backlog`. Every other column, an issue with no card, a Status that is not a string,
and a card on a different project are all skipped. A deny-list of the columns we
happened to think of would make a column added next year eligible by default, and
nobody would find out until something was closed.
FAIL CLOSED HERE MEANS SKIP, NOT ACT. Every other guard in this org fails closed by
refusing to report clean. This one is destructive, so the direction reverses: an
unreadable board, an unparseable response, an absent card and an unreadable
updatedAt all mean "leave it alone". `--strict` additionally exits non-zero so an
outage is visible rather than merely harmless -- silence that means "I could not
tell" must not look like silence that means "nothing was due".
A MAX_ACTIONS ceiling refuses rather than executes: the only ways to reach 40 due
issues are a bug in the eligibility rule or a board outage that made everything read
as Backlog, and mass-closing is the wrong response to either.
App-native from the start (backend#2036) -- it never had a PAT to migrate off. The
selftest runs as a GATE inside the reusable, before the sweep: a destructive
unattended job whose rule has regressed must not run at all.
STAGED INVENTORY, per this ticket's own sequencing. `stale-backlog.yml` moves from
`copies:` to `reusables:`, and all 17 repos get a caller entry declared `exempt`
with a written reason while the caller PRs are in flight. Declaring it `required`
now would redden the org audit for every repo whose caller has not landed -- the
drift window made to look permanent. The final PR of the sweep flips them.
TWO THINGS THE MIGRATION SURFACED, both fixed here rather than carried:
- `cli` was recorded `divergent` for pinning actions/stale@v11 where everyone else
pinned v9. A thin caller has no action pin, so the divergence is resolved by
construction rather than by a realignment PR.
- `release-train`'s exemption reason was rendered FALSE by this change. It read
"stale-backlog.yml is column-blind and would close live pipeline work". The
column-blindness is what this fixes. The exemption is RETAINED -- enrolling the
repo that drives promotions into an unattended closing sweep is a decision
someone should take deliberately -- but it is now recorded as UNDECIDED, citing
the absence of a basis rather than a defect that has been fixed.
Seven mutations, all caught: board-awareness removed (20 cases fail), the allow-list
turned into a deny-list of known columns (19), an unknown Status defaulted to
Backlog (2), the project number ignored (1), exempt labels dropped (2), an
unreadable date treated as ancient (crashes -- fail-closed by construction), and the
close grace period removed (1).
Verified: 30-case decision table; `make selftests` runs it and `selftests-cover`
confirms all 9 suites are wired; inventory schema validates; actionlint clean.
Refs backend#1979
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(1979): scope the new App mint before it ships, and drop the merge contamination
Two things this branch needed before it could go out.
1. THE DIFF CARRIED SOMEONE ELSE'S WORK. This branch was cut from the #2114
branch, so `scripts/bricked-prs.py` and its selftest showed as modified. #2114
has since merged (.github#282) with three rounds of review changes on top of the
snapshot here. Resolved by taking develop's version of both files outright --
verified `git diff --cached origin/develop` on them is EMPTY -- so the diff is
now #1979's own six files and nothing else.
2. IT ADDED A NEW UNSCOPED APP MINT. `stale-backlog.yml` minted with `owner:` and
no `permission-*`, i.e. the App's full installation grant, in the same week
backend#2157 swept 19 files to remove exactly that. Shipping a new one while a
guard against them is in review (.github#287) would have been the twentieth mint
arriving the way the first nineteen did.
Scoped from what the sweep actually CALLS, not from what the App holds:
issues: write `issue edit --add-label`, `issue comment`,
`issue close` (stale-backlog.py:188-192)
organization-projects: read ISSUES_Q reads `projectItems .. Status`; the
sweep never writes a board field
No `pull-requests` -- this workflow does not touch PRs by design (backend#1408).
No `contents` -- the App token is only ever `GH_TOKEN` for the sweep; the
`.github` checkout uses the workflow token with `persist-credentials: false`.
`repositories:` narrows the issue writes to the caller's own repo, which is
precisely the sweep's scope (`--repo "${{ github.repository }}"`). Without it a
DESTRUCTIVE, UNATTENDED sweep would hold issue-close rights across all 19 repos
while only ever closing in one.
Org-projects being unaffected by repo scoping is MEASURED rather than assumed:
backend#2181's verification run 32255581084 wrote the org project with
`repositories:` set. That was the load-bearing unknown and it is settled.
PROVED AGAINST THE GUARD ITSELF, not against my reading of it. Running #287's
mint-scope.py over this tree: 14 mint steps, 12 unscoped (all pre-existing and
exempted), 0 findings. So this branch passes the gate that is still in review --
which is the whole point of adding the gate before the burn-down rather than after.
make check green: 41 selftests, coverage gate, action-pins 34/0, actionlint 0.
Refs backend#1979, backend#1597 item 1, backend#2157.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(1979): truncation must not close an exempt issue, and .github joins the wave
Two Bugbot findings, both correct.
HIGH -- `exempt` PLUS A CALLER IS ITSELF A FINDING. This PR added
stale-backlog-caller.yml to .github while marking .github's stale-backlog.yml
`exempt`. caller-drift.py:2260 reports exactly that pair ("the exemption is stale").
It went green pre-merge only because .github's OWN callers are read from develop
over the API rather than from the PR checkout -- the awkwardness caller-drift's
docstring already documents -- so it would have merged green and reddened the very
next audit. That is the outage the in-flight exemption exists to prevent, produced
by the exemption's own PR.
The caller is removed. .github joins the caller wave like every other repo, and the
wave's final PR flips every `exempt` to `required` -- which is what the exemption
anchor already says the plan is. Recorded there, with the cost stated: .github is
unswept until its caller lands. The sweep is weekly and the reusable has never run,
so nothing regresses.
The alternative -- flip .github to `required` here -- is the direction caller-drift
explicitly refuses: the caller is not on develop yet, so the PR would go red.
"Land the caller first, flip the entry in a follow-up" is its own instruction.
MEDIUM, AND THE WORSE ONE IN CONSEQUENCE -- `labels(first:40)` never read
`totalCount`, so a `keep-open` label past the page read as ABSENT and the issue was
labelled `stale` and later closed. Every other unreadable input in this script
causes a SKIP; this one PROCEEDED. Both connections now request `totalCount` and a
truncated read refuses.
The shape of the fix is what reddened when `totalCount` was added to the fixture:
28 passed, 2 failed
FAIL an idle Backlog issue past 42d is warned
FAIL a warned issue silent for 14d+ is closed
Exactly the two cases that WRITE. Every skip case stayed green.
projectItems(first:10) is handled by the same rule. It fails SAFE -- no card lands
on "not Backlog" -- so it was never destructive; it is checked so both connections
obey one rule rather than one by accident.
5 mutations, all applied and caught. Two of them found my own errors:
* `total >= len(nodes)` would call an exactly-full page truncated and silently
leave every such issue unswept -- the opposite failure, equally invisible. Now
pinned by a case.
* the comment claimed the truncation test is "checked FIRST" and nothing pinned
it: OR-ing it with the exempt test left all 35 cases green, because both paths
skip. The order only changes the REASON -- but a `keep-open` issue reported as
"label list truncated" sends someone to fix the wrong thing.
AND THAT ORDER CASE WAS INERT ON ITS FIRST ATTEMPT. I asserted `"exempt label" in
why`, and the truncation message CONTAINS those words ("cannot rule out an exempt
label"), so it passed on the wrong reason. Now
`why.startswith("exempt label") and "truncated" not in why`. Same substring-
collision shape as an earlier inert assertion in this repo, which is why the
mutation run is the thing that decides, not review.
36 cases green. make check green.
Refs backend#1979, backend#1597 item 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(1979): a skip nobody counts is a clean-looking sweep
Two Bugbot findings, both consequences of the truncation fix in the previous
commit. Closing the fail-open created a silent-nothing, which is the failure this
repo names most often.
HIGH -- TRUNCATION SKIPPED AND REPORTED SUCCESS. The skip was real; nothing counted
it. `--strict` tested only `unknown` (no readable Status), and truncation set no
counter at all. So if ISSUES_Q ever stopped asking for `totalCount`, EVERY issue
would read as cut, every issue would skip, and the sweep would print `0 due` and
exit 0 -- the same signal as a genuinely empty backlog, on an unattended weekly
cron that nobody watches when it is green.
Three changes, in increasing order of how loud they are:
* decide() returns a TRUNCATED sentinel rather than folding into None. None means
"no card", which is ordinary; TRUNCATED means "could not tell", which is a
defect in the read. An object(), not a string, so it can never equal
ELIGIBLE_STATUS or a column somebody adds later.
* main() counts them separately and names them in the summary line.
* `::warning::` unconditionally, and `--strict` now FAILS on them. Truncation was
outside the strict test entirely -- the one mode that exists to make an
incomplete sweep visible could not see the most complete way to be incomplete.
MEDIUM -- THE SUITE DID NOT PIN THE QUERY. Every case built its own `totalCount`
payload, so dropping the field from ISSUES_Q left all fixtures green while
production treated every label list as cut. Now asserted against the query TEXT --
`totalCount` present, and present on the labels and projectItems connections
specifically. Same restate-instead-of-derive shape as the meta fixture on
.github#289, in the other direction: there the fixture was richer than production,
here it was richer than the query.
AND main() IS NOW ACTUALLY DRIVEN, because two mutations lived where no case looked.
Stopping the `cut` increment, and removing truncation from `--strict`, both left
every one of the 42 decide()-level cases green -- the same wiring-versus-function
gap as .github#289's High, in the same epic, two PRs apart. fetch_issues and apply
are the only network seams, so stubbing them runs the real parsing, the real
counters and the real exit codes. Behavioural, not a source assertion.
8 mutations, all applied and caught:
truncation reports None again 42/4
the cut counter never increments 43/3 <- was uncaught before main() cases
--strict stops failing on truncation 45/1 <- was uncaught before main() cases
the warning is removed 45/1
TRUNCATED becomes None 45/1
ISSUES_Q drops totalCount 45/1 <- was uncaught before query cases
the truncation check is removed 39/7
truncated() uses >= 37/9
46 cases green. `make check` exit 0 -- captured, not piped, after the last commit
went out red because `make check | tail -3 && git push` returns tail's status.
Refs backend#1979.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(1979): create the `stale` label -- actions/stale did, gh issue edit does not
Bugbot, Medium, and it would have broken the sweep on the first repo it ran in.
`gh issue edit --add-label stale` FAILS when the repo has never defined that label.
`actions/stale` created it; this script replaced actions/stale and did not. Measured
across the org 2026-08-20:
`stale` label: 11/19 repos have it
MISSING: .github, claude-skills, data-ingestors, design-system, docs,
release-train, rfcs, start-training
The eight without include `.github`, where this reusable lives. The eleven WITH it
have it only because actions/stale happened to create it there -- so the sweep would
have worked in the repos that had already been swept and failed in every repo that
had not, which is the least useful possible distribution of a bug.
Without the fix: every warn fails, the run goes red, and no issue ever enters the
14-day close window. A sweep that cannot pass its own first stage.
ensure_label() probes and creates once per run, and only when a warn is actually due
-- a close-only run needs nothing created, and a repo with nothing due should not
acquire a label it never uses. Idempotent by ASKING rather than by `--force`, because
`--force` rewrites colour and description on every weekly run: eleven repos would
take a no-op edit forever to spare eight a one-line create.
FAIL CLOSED, at the run level rather than per issue: if the label cannot be had, the
warn loop is refused with a message, instead of every `--add-label` failing
separately and reddening the run while warning nobody.
FOUR OF MY OWN ERRORS IN THIS COMMIT, each found by the suite or a mutation:
* ensure_label was an UNSTUBBED NETWORK SEAM in run_main, so the clean-sweep case
started returning 2 by shelling out to a real `gh` against the fixture repo. A
stub list one short is a test measuring the machine it runs on.
* a mutation making ensure_label report success on a failed create left all 49
green, because every case stubbed the function and pinned only WHEN it is called.
Third instance of the function-versus-wiring split in this epic (.github#289's
High, the `cut` counter earlier on this PR, this). The real function is now driven
with subprocess.run stubbed.
* that same mutation's first anchor -- `if dry_run:\n return True,` --
matched apply()'s early return instead, a different function that run_main stubs.
So it landed on code no case drives and reported "uncaught" for the wrong reason.
An imprecise anchor lies in both directions.
* `make check` was piped again on the previous push; exit status is captured now.
7 mutations across the label work, all applied and caught:
the label is never ensured (the original bug) 47/2
a failed ensure is ignored and warns anyway 48/1
the label is ensured on every run 48/1
ensure_label reports success on a failed create 52/1
dry-run creates the label anyway 52/1
the probe result is ignored 52/1
...plus the 8 truncation mutations, still green
53 cases green. make check exit 0.
Refs backend#1979.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
saadqbal
left a comment
There was a problem hiding this comment.
Ran the suite (13/13) and the live audit, then mutated the three properties this stands or falls on:
- Exact-string match including the
@pin (uses.strip() == MINT_ACTION) → 7 failures, including the case named for it:a DIFFERENT version pin of the mint action is still matched. That's the obvious wrong implementation and it's genuinely pinned. - Zero mints stops being a finding (
if minted == 0:→ dead) → 3 failures, led byZERO mints found is a hard error, not a clean run. - Look-alike and path-containing action names both have their own cases and both redden under the matcher mutations.
Live audit on the real tree:
mint-scope: 13 `actions/create-github-app-token` step(s) across 33 workflow file(s)
12 unscoped, 12 exempted, 0 finding(s)
I agree with the scope decision. "Are the named scopes the right ones" is only answerable by the workflow's own API calls, and a guard claiming to know would be restating rather than deriving — whereas "asked for nothing at all" is decidable and is what actually went wrong nine times in one day. Reporting stale exemptions as findings too is the part that keeps the list from calcifying, and it's the bit most guards of this shape omit.
One fix: the count in the prose is off, and it's off in the direction this file argues against. Two places say 13 —
- the docstring: "The 13 reusables that already carry a full grant are exempted by name"
- the comment above
EXEMPT: "These 13 are the pre-existing state this guard was written to stop growing"
— but EXEMPT has 12 entries, and the PR description says 12. I think the 13 came from the mint step count (the audit reports 13 steps, 12 unscoped + 1 already scoped), which isn't the same population. Given the file's own "DERIVED, NEVER RESTATED" section, the right fix is probably not to write 12 but to let it say len(EXEMPT) — a hardcoded tally sitting directly above the list it counts is the exact pattern backend#1729 is cited for.
Also worth noting for whoever picks up the burn-down: the reasoning that a scoped canonical here means scoped fleet-wide rests on add-to-kanban.yml being the only copied workflow that mints, and the docstring says where to re-check that (copies: in repo-inventory.yml). Good that the dependency is written down rather than assumed.
…evelop (#289) * fix(2214): audit the branch a repo ships from, not any branch named develop `caller-drift` preferred `develop` wherever that branch existed. `rfcs` is the only repo in the org whose default is `main`, so the backend#2157 sweep landed there -- where rfcs actually ships -- and the audit read a `develop` that lags, then reported drift against a change that was correctly in place: rfcs/main 2a3a432 the default branch, where the change landed rfcs/develop 07283e3 what the audit read IT AGREED BY COINCIDENCE FOR MONTHS, and that is the part worth recording. Both refs held the same blob because nothing had ever changed that file, so a wrong ref and a right ref were indistinguishable. A resolver pointing at the wrong branch produces a correct-looking result for exactly as long as the two branches match, then emits a false finding the first time real work lands. THE RULE IS NOW DERIVED, NOT LISTED. Develop-first exists because TRAIN repos default to main/master while work lands on develop, so a default-branch audit under-reports work in flight. A NON-TRAIN repo has no promotion pipeline: its default branch IS where it ships, and preferring a stray `develop` audits a branch nobody merges to. The discriminator is `release_train`, which the inventory already carries and which `load_release_train` already verifies against release-train/repos.yml -- so the audited ref is derived from a fact this guard independently checks, rather than from a hand-maintained `audit_ref` field or an exception row for rfcs. A second place to be wrong is what an override would buy. Measured across all 19 repos before writing it: the new rule changes exactly ONE answer, rfcs develop -> main. Nothing else moves. `audit_branch` is renamed develop-first -> develop-first-on-train in the inventory AND in SUPPORTED_AUDIT_BRANCH, because the schema check compares them and the semantics changed. The one-line description above the key said "develop where that branch exists, else the default" -- exactly the behaviour being removed -- and now states the rule and names the discriminator. BOTH DIRECTIONS ARE PINNED, because neither case can fail alone: assert only the train side and the non-train path is untested; assert only the non-train side and a resolver that always uses the default branch passes. Each reddens a different mutation. 4 mutations, all applied and caught: reverts to plain develop-first 193/1 the non-train case always the default branch 192/2 the train case the train flag read inverted 191/3 the train case + two more audit_branch no longer enforced 193/1 the inventory positive control AND ONE FIX TO THE SUITE ITSELF. The `_good` positive control RAISED AssertionError when the tree was fetched off develop, which aborts the whole run -- so two of those mutations first reported "CRASH" and hid every other case they also broke. It now returns an empty tree, failing cleanly. A suite that dies on the first surprise cannot tell you the shape of a regression. The fixture META also gained `release_train: True`, which is now load-bearing: without it every existing case would have silently exercised the non-train path while being written about develop-first. NOT FIXED HERE, because it is not in this repo: the workspace CLAUDE.md states "Verified 2026-08-06: every active repo's default branch is now `develop` (all 20)". That is false for rfcs and is the line a reader would use to conclude this guard reads the right branch. It lives outside any git repo, so it cannot ride a PR; flagged on backend#2214. make check green: 194 caller-drift cases, all selftests, coverage gate, actionlint. Closes backend#2214. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2214): the train flag never reached the resolver Bugbot, High, and it makes the previous commit worse than a no-op. `read_repo` read `meta.get("release_train")`, but production `meta` comes from `list_active_repos`, which returns ONLY: {"visibility": ..., "default_branch": ...} No `release_train`. So the flag was always None -> False, EVERY repo was audited on its default branch, and develop-first was silently deleted for the train repos it exists for. Today that happens to be harmless -- every train repo currently defaults to `develop`, measured -- but the policy was gone, and the next repo to default to main/master would have under-reported work in flight with nothing saying so. THE SELFTEST PASSED BECAUSE THE FIXTURE CARRIED A KEY PRODUCTION NEVER SETS. I added `release_train: True` to META in the same commit that started reading it from there. A fixture richer than the real payload is a test asserting its author's assumption, and this is the THIRD time that shape has appeared in this epic -- `is_bot` on #282, the look-alike action name on #287, this. THE FIX IS A PARAMETER, NOT A LOOKUP. `on_train` is required and positional, so a caller that forgets it raises TypeError; defaulting it to False would reproduce the bug with better manners. `main()` passes `bool(entry.get("release_train"))` from the INVENTORY row -- schema-required, and cross-checked against release-train/repos.yml by load_release_train -- so the ref this guard reads is derived from a fact it independently verifies. META IS NOW EXACTLY THE PRODUCER'S SHAPE, and a case asserts that by reading `list_active_repos`'s own source rather than a list written in the test: it writes `visibility` and `default_branch` and nothing else. That case reddens if the producer starts setting `release_train`, which is the only honest way to keep the two in step. AND THE WIRING IS PINNED SEPARATELY, because the behavioural cases could not see it. They call read_repo directly, so mutations hardcoding the argument to True or False left all 196 green -- and #289's bug WAS the wiring, not the resolver. A source assertion on main() closes it. Weaker than behavioural and said so in the comment: driving main() needs the org listing, the inventory and the train file stubbed together, which this suite has no harness for. That assertion was ALSO wrong on its first attempt: a paren-matching regex allowing one level of nesting could not match `bool(entry.get(...))` across two lines, so it found nothing and failed on the correct code while every mutation "passed". An extractor that cannot find the thing reports the same as a defect. Line-based now. 7 mutations, all applied and caught: on_train read from meta again 194/2 the train-branch case call site passes False 196/1 the wiring case call site passes True 196/1 the wiring case call site reverts to meta 196/1 the wiring case plain develop-first restored 193/1 the non-train case always the default branch 192/2 the train case list_active_repos sets release_train 194/2 the producer-shape case 197 cases green. make check green. Addresses Bugbot on .github#289. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2214): ruff — the previous push was red, and I masked it Three ruff errors (E402 import-not-at-top, two E741 ambiguous `l`) shipped in the commit before this one. They shipped because I ran make check 2>&1 | tail -3 && git commit && git push and a pipeline exits with the status of its LAST command. `tail` returned 0, so the `&&` chain treated a failed `make check` as a pass and pushed anyway. The output was even visible -- "make: *** [ruff] Error 1" was in the three lines I printed -- and the chain ran on regardless. Recorded rather than quietly fixed, because it is the same defect this repo keeps finding in its own guards: a check whose result nothing actually reads. Mine was in the shell, one layer out from the code. `make check` now run with its exit status captured (exit=0), not piped. 197 selftest cases green, 41 suites green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
saadqbal approved with one fix, and it is the file's own rule turned on itself. THE PROSE SAID 13 IN TWO PLACES while `EXEMPT` holds 12. His diagnosis is right: 13 is the mint STEP count the audit reports -- 12 unscoped plus one already scoped -- so the two are different populations, and the smaller one drifts the moment a row is burnt down. Which is the point of the guard. And his recommended fix is the right one: not "write 12", but let the number come from `len(_exempt())`, which the run already prints. A hardcoded tally sitting directly above the list it counts is the exact pattern backend#1729 is cited for -- in the file that cites it. Both prose sites now carry no number, and the docstring says why, so the next person does not helpfully add one back. MERGE CONFLICT resolved against develop, which moved three times underneath this branch (#288, #289, #291). All three hunks wanted BOTH sides, not one: Makefile `lint` mint-scope AND mutation-house-rules-dry SELFTEST_TARGETS selftest-mint-scope AND selftest-house-rules selftests.yml the mint-scope audit step AND the house-rules mutation step Makefile CI map one line naming all three, since the required `selftests` context now runs all of them repo-inventory.yml kept OURS deliberately: develop still carries the pre-release-train#93 wording ("the header alone"), which is now false -- #93 added the scoping, so the divergence really is two comment blocks and nothing else, and ours is the version with the checkable claim in it. Two comments in selftests.yml also lost their case counts on the way through, for the same reason as the docstring: `mint-scope` prints its own totals. make check exit 0. mint-scope: 13 steps, 12 exempted, 0 findings. 41 house-rules cases. mint-scope selftest 13/13. Refs backend#2157. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 20, 2026
Fixed in the push just now, and the finding is this file's own rule turned on itself — thank you for catching it. You're right about the population, which is the part that makes it a real defect rather than a typo. 13 is the mint step count the audit reports: 12 unscoped plus the one already-scoped And your recommended fix is the right one rather than the obvious one. Writing
Two comments in Also in this push: the conflict against developdevelop moved three times underneath this branch (#288, #289, #291). All three hunks wanted both sides, not one:
The Makefile's CI-job map is now one line naming all three, since the required
On your closing noteThe fleet-wide inference does rest on
|
Uh oh!
There was an error while loading. Please reload this page.
Bugbot High, and CI agreed: `audit` and `gate` both red. The conformance audit could not load the contract at all -- exit 2 before auditing anything -- so this PR was disabling the guard it exists to add. WHAT I DID WRONG. Resolving this branch against develop, I took OURS wholesale for repo-inventory.yml because our side had the corrected `divergent` reason for release-train's add-to-kanban copy. But develop's side also carried a STRUCTURAL change from #288: `stale-backlog.yml` moved from top-level `copies:` to `reusables:`. Keeping ours preserved a `repos.release-train.copies.stale-backlog.yml` key whose family no longer lists that file, and caller-drift.py rejects any extra copies key as a schema error. "Keep ours" is only ever right about the hunk you looked at. I read the reason text, saw it was the version I wanted, and did not ask what else that side of the file had stopped saying. THE FIX is the narrow one: drop the orphaned `copies:` entry. release-train's stale-backlog exemption already lives under `callers:` on develop, with the reason #288 rewrote -- the one that says the original basis (column-blindness) no longer holds and the exemption is retained as an UNDECIDED question rather than a justification. Bugbot's second sentence is what pointed at that: the callers cell still referenced the newer anchor, so the reason I was preserving applied to nothing. The corrected `divergent` reason for add-to-kanban.yml -- the part I actually wanted from our side -- is untouched and still says the checkable thing: strip every comment and blank line from both copies and they are byte-identical. Verified: the inventory loads, `make check` exit 0, mint-scope 12 exempted 0 findings, 41 house-rules cases, 13 mint-scope cases. Refs backend#2157. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 20, 2026
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 05ab193. Configure here.
saadqbal
left a comment
There was a problem hiding this comment.
Re-approving — my earlier approval was dismissed by the three commits since, so I re-checked rather than assuming they were cosmetic. All three are good, and one is more than cosmetic.
My finding is fixed the way I hoped.9f75177a drops the hardcoded tally entirely rather than correcting 13 → 12, and the runtime figure now comes from len(_exempt()) at :215. The prose that replaced it explains why the 13 was wrong — it was the mint STEP count, of which 12 are unscoped and one already scoped — so the next reader doesn't rediscover the confusion. That's the right resolution for a file whose own thesis is "derived, never restated".
b9a76291 is the substantive one, and I'd missed it. The guard was wired only into make lint, which CI doesn't run — so on my last pass I verified a guard that could not have blocked anything. It now runs as make mint-scope inside selftests.yml, and the comment says exactly that. Worth calling out because a correct guard in a context nobody executes is the same as no guard, and it's the sort of gap a careful review of the script sails straight past.
One caveat on that: I can confirm selftests runs and passes on this PR, but I can't verify it's a required context — repos/tracebloc/.github/branches/develop/protection returns 404 for my token. Worth an admin eye if "required" is load-bearing for you; the commit message claims it and I have no reason to doubt it, I just can't see it from here.
05ab193c removes a stale-backlog.yml exempt block that the conflict resolution had reinstated after #288 deleted it. Checked the consequences rather than the diff: the real inventory still parses at schema_version 2 with 19 repos, every one carrying release_train, and the release-train entry's keys are clean. caller-drift selftest 197/0, so nothing downstream depends on the key that went.
Re-ran the guard itself: selftest 13/13, and the live audit is clean with the tally now derived —
mint-scope: 14 `actions/create-github-app-token` step(s) across 33 workflow file(s)
12 unscoped, 12 exempted, 0 finding(s)
14 steps rather than 13 because the develop merge brought in #288's scoped mint, which is the system working as intended.
Everything from my first pass stands: the version-agnostic uses: match and the zero-mints-is-an-error rule are both mutation-proved, and reporting stale exemptions keeps the list shrinking.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
commented
Aug 21, 2026
/fr-pass |
The conflict is in `mint-scope.py`'s EXEMPT map, and it is a real one rather than a textual collision: #352 scoped fr-gate and left standards-sync as the last row, this branch scopes standards-sync and left fr-gate as the last row. Each side is correct about its own workflow and wrong about the other's only because it was written before the other landed. Resolved to the state both sides were walking toward: EXEMPT is now EMPTY. The two were split into separate PRs precisely because each needed its own window -- fr-gate is a REQUIRED check on every promotion branch fleet-wide, and standards-sync is a contents:write sweep whose risky mode cannot be rehearsed without opening 19 real PRs -- so both landing is what completes the burn-down, not an accident of merge order. Taking either side's row verbatim would have re-exempted a workflow that is now scoped, which `stale_exemptions` would then correctly report as a finding. The prose above the map is rewritten to say the burn-down is finished and, more importantly, that an empty map is not an inert one: `stale_exemptions` compares against the live unscoped set, so a re-introduced full-grant mint has no row to hide behind. It also says outright not to add a row back to green a red run -- scope the workflow instead, which is what every row here was eventually replaced by. No tally was introduced; the file's own header forbids one (saadqbal, #287) and the count is printed from `len(_exempt())`. Verified on the merged tree: `mint-scope.py` reports 17 mint steps across 37 workflow files, 0 unscoped / 0 exempted / 0 findings -- so the empty map is measured, not asserted. `make check` green (ruff, shellcheck, house-rules, action-pins, mint-scope, actionlint, mutations-dry, and all selftests: 29 passed, 0 failed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Refs backend#2157. The 19
add-to-kanbancopies are scoped and the audit is clean (0 findings). Nothing stops the twentieth mint arriving the way the first nineteen did — nine workflows acquired a full-permission mint in one day because the first one did and the rest were copied from it.Least privilege applied by hand is a state. This is the part that makes it a property.
What it decides, and what it refuses to guess
A mint with no
permission-*input carries the App's full installation grant — contents + pull-requests + issues + org-projects write across every installed repo. The App holdsbypass_reviewson staging and prod fleet-wide, so the blast radius is merge past review, not merely write.Whether the scopes a workflow does name are the right ones is a question only its own API calls can answer, and a guard that claimed to know would be restating rather than deriving. "Asked for nothing at all" is decidable, and is what was actually going wrong.
Derived, not restated
The workflow list is the directory. Mint steps are found by parsing YAML and comparing
uses:up to the@— so a new workflow is covered automatically, and a pin bump does not silently stop the check. There's a case and a mutation for exactly that, because exact-string-including-version is the obvious wrong implementation.Fail closed, including on zero
An unparseable workflow, a missing directory, and zero mints found are all hard errors. The last matters most: this file's premise is that mints exist, so finding none means the matcher broke — not that the fleet got clean.
Armed green over 12 pre-existing offenders
Each is exempted by name with a reason, so this lands green rather than as a red gate that trains people to skip the tier.
fr-gateis a required check on every promotion branch fleet-wide; three others read branch protection, where a narrower token is known to return less data rather than an error — that's whycaller-driftkeeps a PAT at all (rulesetbypass_actorsis returned only to a write-access caller). Downscoping those needs measurement per workflow. It is not a thing to guess at across 13 files in one commit.And a stale exemption is also a finding. Without that half the list stops being a burn-down and becomes cover: a full-grant mint re-introduced into an exempted file would be admitted by a row written earlier for a different reason.
Verification — and two gaps the mutation run found
13 cases, 8 mutations, each verified applied and caught. Two were uncaught on the first run, and the run is how I found out rather than review:
a missing with: counts as scopedwith:blockthe action match becomes a substring testevil/actions-create-github-app-tokendoesn't contain the real name — the hyphen breaks it — so it couldn't distinguishinfromsplit("@")[0] ==Both are now cases. The second fixture became
myorg/actions/create-github-app-token, which a substring test does wrongly match.Separately, two cases initially failed by measuring production state: without an explicit exemption override, the live 12 rows all read as stale against a fixture directory. That failure was the stale-exemption check working on the wrong input, and the helper now says so where someone would otherwise repeat it.
Also in this PR
repo-inventory.yml'sdivergentreason for release-train's copy said its header "is now the ONLY difference". False in two ways — the copy also lacked the scoping until release-train#93, and it carries a second wanted comment (the Dependabot note) the reason never mentioned. Rewritten to name both, and to state the claim in a checkable form: strip every comment and blank line from that copy and from the canonical one and they are byte-identical.What this does not do
The 12 exemptions are the remaining work, in two risk tiers — the board-writing set (loud failures, local blast radius) and
fr-gate+ the three protection readers (a wrong scope there blocks the train or silently returns less). Those are follow-up PRs, one tier at a time, with the exemption rows coming out as each is measured.Note
Medium Risk
Changes enforce CI gates on workflow token scoping (security-sensitive), but behavior is additive with exemptions so merges stay green; wrong matcher or exemption drift could block CI or miss regressions.
Overview
Adds
mint-scope, a YAML audit that flagscreate-github-app-tokensteps with nopermission-*inputs (full installation grant). Twelve existing workflows stay on a namedEXEMPTburn-down list; stale exemptions fail too.scripts/tests/mint-scope-selftest.pyexercises the matcher on fixtures (fail-closed, version-pin matching, no false substring hits).Wiring:
make mint-scopejoinslintandmake check; the fixture selftest joinsmake selftests. The requiredselftestsCI job now runsmake mint-scopeon real workflows so a new unscoped mint cannot merge while only fixture tests pass.Docs:
repo-inventory.ymlupdates release-trainadd-to-kanban.ymldivergent text (checkable byte-identity after comments) and recordsstale-backlog.ymlas deliberately absent on release-train.Reviewed by Cursor Bugbot for commit 9f75177. Bugbot is set up for automated code reviews on this repo. Configure here.