release-train: develop -> staging - #327
Merged
Merged
Conversation
…p commit (backend#2365) (#323) * fix(scripts): branch ownership comes from the PR author, never the tip commit (backend#2365) A "my branches" list built from `%(authorname)` on the ref answers "who touched it last", not "whose work is this". Pushing a review fixup onto someone else's branch rewrites the tip author and changes nothing about who owns it -- and the next command in that workflow is `push origin --delete`. Measured: `client` PRs #395 and #592 are @shujaatTracebloc's and both showed Lukas as tip author. backend#2001's per-person table is NOT affected -- checked first, and it attributed by PR author already (`inventory.py`: "merged/closed PR author wins, else the tip's git identity"). The defect is that the rule lived in a throwaway script, so every later query re-derives it and the obvious derivation is wrong. `branch_owner.py` is the one place that answers it: PR author first (exact head oid, then head name), the oldest commit not on the default branch for a branch with no PR, and `unattributable` otherwise. `attribute()` takes no tip-author parameter, so no path can return one. Fails closed -- an unread or truncated PR list, an ambiguous head name and a deleted PR author all refuse rather than guess -- and `origin/HEAD` is verified against the remote before the first-commit signal is used, because a stale default corrupts that signal exactly. `CreateEvent` is not used for the no-PR case despite the ticket proposing it: /repos/{repo}/events caps at 300 events, which on backend spans under four hours and holds two branch creations. 71 assertions and 26 mutations, wired into the already-required `selftests` context; three mutations were uncaught on the first run and the tests were strengthened rather than the mutations dropped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(scripts): a withheld check is not a finding, and an absent tip is not a mismatch (backend#2365) Both of Bugbot's findings on #323, and both are the same shape as the defect already fixed in the first commit: a message asserting a comparison that never ran. Reproduced by construction before changing anything. 1. The multi-author reuse refusal ended "and none is at the current tip" even with no tip in hand -- reachable for a branch named on the command line that this clone has not fetched, whose head name two people have used. The single -author `pr` arm already split those two sentences; this arm now does too. 2. When the default branch could not be confirmed, `main` withheld the first-commit signal by passing "" and `attribute` then reported "no commit on this branch that is not already on the default branch" -- a fact about the branch, from a check nobody ran. Appending the reason did not retract it. `attribute` takes `first_commit_problem` now, so a withheld signal is reported as unmeasured with its reason, and the measured-and-empty wording is pinned apart from it. 77 assertions, 29 mutations, 0 stale, 0 uncaught. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(scripts): a branch list that could not be read is not an empty clone (backend#2365) Bugbot's medium-severity finding on 9102520, and it is the sharpest one yet because it is the fail-closed rule turned on this module's own third seam. `remote_branches` collapsed a failed `git for-each-ref` into `[]` with no problem string, and `main` printed that as `0 branch(es)`, exit 0 -- a clean bill of health from a read that never happened, while the other two seams (`pull_requests`, `default_branch`) refuse explicitly. "I could not read the branch list" and "this clone has no branches" are different answers and only one of them is a fact about the clone. It returns `(refs, problem)` now and `main` refuses with exit 2, because without the ref list there is no tip for any branch and no way to tell whether a named one exists -- so every verdict below it would be weaker than it looked. A genuinely empty remote still reports no problem, pinned by its own case so the two cannot collapse back together. Bugbot's other comment on that head is stale: it restates the absent-tip finding already fixed in 9102520 and its own footer names commit 6f45cdc. Verified by construction on the current tree -- the refusal reads "and no tip was supplied to break the tie with". 80 assertions, 30 mutations, 0 stale, 0 uncaught. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(scripts): audit the other reads, and pin both empties apart at the seam and in main (backend#2365) Saqlain's requested change on #323 landed in 7a87b83, before his review arrived. This is the rest of what he asked for, and it found a fourth instance he had not seen. AUDITED ALL FOUR READS rather than fixing only the reported one. `pull_requests` and `default_branch` already refuse with a problem string. `remote_branches` was fixed in 7a87b83. `first_commit_author` had the SAME defect and worse: its condition was `if rc != 0 or not out`, conflating a FAILED `git log` with a branch that genuinely has no commits off the default branch, and `attribute` then reported the second one -- "no commit on this branch that is not already on the default branch" -- for both. A fact about the branch, from a read that never ran. It returns `(author, problem)` now, and `main` carries that reason through. THE TWO EMPTIES ARE PINNED APART IN BOTH DIRECTIONS, for both reads: collapse the failure into the success and the mutation reddens; report the genuine empty AS a failure and it reddens too. Asserting only "something empty came back" would have passed in a world where both refuse identically. AND `main` IS NOW TESTED, which is what made this worth doing. Dropping the one argument that carries a seam's problem into the rule reverted the whole fix while every isolated case stayed green -- the mutation run caught it as UNCAUGHT, so the entry point gets driven directly: a failed history read renders as unmeasured, a genuine empty renders as no-unique-commits, a failed enumeration exits 2 with no rows, and a genuinely empty remote exits 0. 92 assertions, 35 mutations, 0 stale, 0 uncaught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(scripts): a default branch the clone never fetched is one fact, not N failed ranges (backend#2365) Bugbot's `Failed first-commit read looks empty` names the seam already fixed in 6665d13, and two sub-cases with it. Both were verified by construction: an unfetched default branch and a CLI-named branch absent from the clone now report "the oldest-commit signal was not measured", naming the failed command, rather than "no commit not already on the default branch". The root cause behind the first one was still open, though. `default_branch` trusted `gh` for the NAME and never asked whether this clone has the ref, so a checkout that never fetched the remote's default produced one failed `default..branch` range per branch -- honest since 6665d13, but blaming N ranges for one missing ref, and unreadable on a repo with hundreds of branches. It is verified with `rev-parse --verify` now and reported once, as its own fact, with the fetch that fixes it. 95 assertions, 36 mutations, 0 stale, 0 uncaught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ured" (backend#2157) (#324) * fix(2157): scope the four kanban mints whose reason was "not yet measured" (backend#2157) Four of `mint-scope.py`'s ten exemptions said, in as many words, "scopes not yet measured". This measures them and drops the rows. Each scope set is derived from the workflow's OWN calls -- every `gh api`, `gh issue`, and GraphQL query/mutation the App token touches -- and from GitHub's documented per-endpoint requirement, not copied between files: advance-deploy-env issues:read pull-requests:read org-projects:write customer-priority-bump issues:write fr-pass-comment issues:write pull-requests:read org-projects:write kanban-closure-router issues:write pull-requests:read org-projects:write `contents:write` -- the grant that makes a leaked token able to push -- drops from all four, along with administration/actions/checks read. Two derivations are worth naming because they are the ones a reader would guess wrong: * labelling a PULL REQUEST (kanban-closure-router's sibling-merge marker) needs only `issues: write`. The label endpoints document "at least one of" Issues write / Pull requests write, and every shared action on a PR is served by the Issues endpoints. So pull-requests stays at READ. * the issue-comment REACTION endpoint (fr-pass-comment's thumbs) documents `Issues: write` ALONE -- no pull-requests alternative -- which is what forces a write grant in that file rather than a read one. `repositories:` is left unnarrowed everywhere: advance-deploy-env's closing-issue advancement and kanban-closure-router's closer lookup are deliberately cross-repo, and repo narrowing is measured in this org for a READ and not for a WRITE. Six exemptions remain, each for a stated reason rather than for lack of time, and the note above EXEMPT now says which is which. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2157): the board mappers read another repo over the API, so contents:read stays (backend#2157) Bugbot High, confirmed by saadqbal: the router mint dropped contents entirely while branch_status_map.py fetches the CALLER repo .kanban.yml with that token. The mapper refuses on any non-404 fetch failure and the call sites fall to the holding state, so a 403 would park every merged PR at override-unusable with a green job. advance-deploy-env.yml has the same three call sites and was flagged by nobody; scoping one and not the other would have shipped the same silent mis-routing through the other writer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(2157): drop the contents grant advance-deploy-env never uses (backend#2157) saadqbal on .github#324, and he is right. I added contents:read there by analogy with kanban-closure-router without checking the second instance. advance-deploy-env has ONE mapper call, at :94 in the env step, which runs BEFORE the mint and under github.token -- the comment at :169 already said so. It also maps $GITHUB_REPOSITORY, its own repo, where the router maps $REPO_FULL, the caller. Same script, different token, different repo: the scope is required there and inert here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…red (#325) `repo-inventory.yml` marked `advance-deploy-env.yml` and `kanban-closure-router.yml` as `exempt` under `rfcs`, while both callers exist on the branch the guard reads. `caller-drift` reports it as two stale-exemption findings, which fails `conformance-gate` -- a required check -- and blocks .github#318, the staging -> main promotion. Both exemptions were explicitly STAGED, not structural, with a written flip condition: "once rfcs#41 has reached rfcs' DEFAULT branch, which is `main`". That condition is met. rfcs#41 merged to `develop` 2026-08-23 and rfcs#45 reconciled it into `main` 2026-08-24, so both callers are on `main` -- which is the audited branch here, because `rfcs` is `release_train: false` and `audit_branch: develop-first-on-train` falls back to the default branch for a repo the train does not promote. The `kanban_closure_router_caller_missing` anchor is deleted in the same commit: this was its only citation, so the flip would otherwise leave it defined and uncited, and no check flags an orphaned YAML anchor. Both staging notes are deleted rather than reworded, per their own clause that a sequencing note outliving its landing "has stopped being a sequencing note and become a written excuse". No check logic changed -- caller-drift.py derives caller presence from each repo's `uses:` values, so this is data catching up to reality. Co-authored-by: Claude Opus 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 146dad7. Configure here.
Their exemption reason was that branch protection can return LESS to a narrower token rather than erroring -- a 200 with a field missing, which a checker reads as "not configured" on a green run. Right worry, so it was measured instead of argued, and it landed on the other workflow. bricked-prs: protection does NOT degrade silently. Without administration:read it answers 403 and read_protection() reports an error. Narrowed to five READ scopes; bricked-prs.py over four repos gives output identical to the full grant. actions:read is required and non-obvious -- it is what lets GraphQL resolve statusCheckRollup -> commit.status on a private repo. merge-settings-drift: reads no branch protection at all. It reads GET /repos, and that endpoint IS the silent one -- the four merge-setting fields are returned only to a caller with PUSH access, 200 either way. contents:write is the one grant it measurably cannot lose; five others go. Every dropped permission was re-added one at a time to prove it load-bearing. Refs tracebloc/backend#2157 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ckend#2449) (#329) Co-authored-by: Claude Opus 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 3b87f81. Configure here.
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
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.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Org-scoped workflow tokens and kanban advancement depend on the new scope matrix; under-scoping can fail silently on green runs until real pushes exercise each path, though the PR documents those failure modes and measured scopes.
Overview
This PR tightens GitHub App installation tokens across several release-train workflows by adding explicit
permission-*scopes derived from (or measured against) each job’s actual API calls, instead of minting the App’s full grant.advance-deploy-env,fr-pass-comment,kanban-closure-router, andcustomer-priority-bumpdrop broad rights such ascontents:write;bricked-prsnarrows to five read scopes with empirical verification;merge-settings-driftkeepscontents: writebecause merge-setting fields only appear to callers with push access—a silent 200-with-missing-keys failure mode documented and tested.It adds
scripts/reason-citations.pyplus CI wiring inselftests.yml(make reason-citationswith an org-wide read token) so exemption reason text inrepo-inventory.ymlthat cites closed issues or abandoned PRs fails the required selftests context, complementing entry-level caller-drift checks.scripts/branch_owner.pydefines branch ownership as PR author first (not tip-commit author), with selftests/mutations and anorg-standards.mdbullet pointing tooling at it.repo-inventory.ymlflipsrfcsadvance-deploy-envandkanban-closure-routerfrom staged exemptions torequiredand removes the orphanedkanban_closure_router_caller_missinganchor.mint-scope.py’s exemption list shrinks to reflect workflows already downscoped.Reviewed by Cursor Bugbot for commit 3b87f81. Bugbot is set up for automated code reviews on this repo. Configure here.