ci(bump-callers): add bump-assign-reviewers-callers.yml dispatch flow (BE-3924) - #54
Conversation
… (BE-3924) assign-reviewers.yml was the one reusable with SHA-pinned callers but no auto-bump flow (pr-size, cursor-review, agents-md all have one), so pinned callers silently drift onto stale behavior when it changes on main. Add a thin entrypoint over the shared bump-callers.sh, mirroring the ALLOW_EMPTY siblings (bump-pr-size/agents-md): reads a new repo-level ASSIGN_REVIEWERS_CALLERS variable (empty/unset is a clean no-op), triggers on push to assign-reviewers.yml + workflow_dispatch, serializes via a concurrency group, and opens/refreshes one sticky bump PR per caller. Paths trigger is the workflow file alone since assign-reviewers.yml runs entirely inline via actions/github-script (no external asset dir). Register it in the bump-callers README table, AGENTS.md catalog, and the test-bump-callers.yml path filters.
mattmillerai
added
agent-coded
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 8 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 3 |
| 🟡 Medium | 3 |
| 🟢 Low | 2 |
Panel: 8/8 reviewers contributed findings.
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
🟠 High — actions/checkout is pinned to the mutable @v6 tag instead of a full commit SHA, violating this repo's SHA-pin-everything policy (AGENTS.md) and inconsistent with the SHA-pinned create-github-app-token on line 72 of the same job. If that tag were retargeted it would run attacker-controlled checkout code in the very job that mints an org-scoped Cloud Code Bot token, and the bare tag also fails the pinact/zizmor pin-validation. Raised by 5 of 8 reviewers (claude-opus-4-8-thinking-max adversarial, gemini-3.1-pro adversarial, gpt-5.6-sol-max adversarial, kimi-k2.7-code adversarial, gemini-3.1-pro edge-case).
| # specific home repo is intentionally not named here — this file is public.) | ||
|
|
||
| on: | ||
| workflow_dispatch: {} # allow on-demand runs (e.g. to re-bump callers) |
There was a problem hiding this comment.
🟠 High — workflow_dispatch here accepts any branch or tag ref, and the dispatched ref supplies both this workflow definition and the checked-out bump-callers.sh. A collaborator can dispatch a branch with a modified bumper and have it run with the org-scoped Cloud Code Bot token, bypassing main-branch review; add a job guard that fails unless github.ref == 'refs/heads/main'. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).
| # JSON array of {"repo","file","label"} — see the header comment for the | ||
| # update flow. Kept in a variable (not the file) so private caller names | ||
| # never land in this public repo or its logs. | ||
| CALLERS_JSON: ${{ vars.ASSIGN_REVIEWERS_CALLERS }} |
There was a problem hiding this comment.
🟠 High — CALLERS_JSON is populated from a non-secret Actions variable and set as step-level env, so the runner prints its value in the step's env: preamble before bump-callers.sh can ::add-mask:: the names, leaking the entire private caller list into this public repo's logs once the variable is populated. Register the masks in an earlier step, or feed the list via a file/stdin instead of a logged env var. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).
| - name: Bump SHA in caller repos | ||
| env: | ||
| GH_TOKEN: ${{ steps.token.outputs.token }} | ||
| NEW_SHA: ${{ github.sha }} |
There was a problem hiding this comment.
🟡 Medium — NEW_SHA uses github.sha unconditionally, but that is not guaranteed to be the current main tip: a workflow_dispatch on a branch, a re-run of an old push (which keeps its original SHA), or an out-of-order concurrency run can each supply a stale/unmerged commit and pin callers to code that isn't on main — contradicting the 'latest SHA always wins' claim in the concurrency comment. Resolve the current main tip at run time (e.g. git rev-parse origin/main) instead of trusting github.sha. Raised by 3 of 8 reviewers (gpt-5.6-sol-max edge-case, kimi-k2.7-code edge-case, gpt-5.6-sol-max adversarial).
| # CURSOR_REVIEW_CALLERS. Every repo name is `::add-mask::`ed out of the (public) | ||
| # run logs before it is ever echoed. The variable is seeded EMPTY (`[]`) — an | ||
| # empty list is a clean no-op here (ALLOW_EMPTY below); the rollout's per-repo | ||
| # caller tickets add entries as callers land (today only Comfy-Org/cloud pins a |
There was a problem hiding this comment.
🟡 Medium — This comment hard-codes the caller repo name Comfy-Org/cloud, permanently disclosing a private consumer in this public repo's source history — the exact leak the surrounding design (variable-stored, ::add-mask::ed caller list) exists to prevent. Drop the repo name from the comment. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).
| with: | ||
| app-id: ${{ vars.APP_ID }} | ||
| private-key: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }} | ||
| owner: Comfy-Org |
There was a problem hiding this comment.
🟡 Medium — The token is requested with only owner: Comfy-Org and no repositories: scope, so it grants every repository and permission in the App's org installation rather than just the caller repos the bumper touches. Scope it to the specific caller repositories to limit blast radius if the bumper or its token is misdirected or compromised. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).
| TAG: assign-reviewers | ||
| WORKFLOW_FILE: assign-reviewers.yml | ||
| # Seeded empty; an empty list is a clean no-op, not an error. | ||
| ALLOW_EMPTY: "true" |
There was a problem hiding this comment.
🟢 Low — ALLOW_EMPTY: "true" is correct for the seeded-empty phase, but once callers are added it turns an accidentally-cleared or missing ASSIGN_REVIEWERS_CALLERS variable into a silent no-op, so SHA bumps would quietly stop with no failure signal. Consider requiring non-empty once the fleet has live callers, or at least warning when the list is empty. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).
| # runs inline via actions/github-script, with no external prompt/script | ||
| # asset dir (unlike cursor-review/** or scripts/check-pr-size/**). So the | ||
| # workflow file itself is the only path that can change its behavior. | ||
| - .github/workflows/assign-reviewers.yml |
There was a problem hiding this comment.
🟢 Low — The push path filter also fires when assign-reviewers.yml is deleted or renamed; the bumper would then open PRs pinning callers to a commit where the referenced reusable workflow no longer exists, breaking those callers. Raised by 1 of 8 reviewers (gpt-5.6-sol-max edge-case).
1 participant
ELI-5
Three of our reusable workflows (
pr-size,cursor-review,agents-md) have a little robot that, whenever the workflow changes, goes around to every repo that "pins" it and opens a PR to update the pin.assign-reviewerswas the odd one out — it had pinned users but no robot, so those repos silently fell behind. This adds the missing robot forassign-reviewers.What & why
assign-reviewers.ymlis a reusable workflow that downstream repos SHA-pin (today onlyComfy-Org/cloud). Unlikepr-size/cursor-review/agents-md, it had nobump-*-callersdispatch flow, so when it changed onmain, pinned callers drifted onto stale behavior and had to be re-pinned by hand.This adds
.github/workflows/bump-assign-reviewers-callers.yml, a thin entrypoint over the shared.github/bump-callers/bump-callers.sh(the same one-implementation script every other fleet uses — no forked logic). It mirrors theALLOW_EMPTYsiblings (bump-pr-size/bump-agents-md):pushtomaintouching.github/workflows/assign-reviewers.yml, plusworkflow_dispatchfor manual re-bumps.ASSIGN_REVIEWERS_CALLERS— a JSON array of{repo,file,label}, same shape asCURSOR_REVIEW_CALLERS/PR_SIZE_CALLERS.ALLOW_EMPTY: "true"makes an empty or unset variable a clean no-op (run stays green, dispatches to nobody).assign-reviewers.ymluses:SHA to the new main tip; pushes to the stableci/bump-assign-reviewersbranch; serialized viaconcurrency: bump-assign-reviewers-callers.Judgment call: paths trigger is the workflow file only
The siblings also watch an asset dir (
cursor-review/**,scripts/check-pr-size/**,agents-md-integrity/**).assign-reviewers.ymlruns its entire selection logic inline viaactions/github-scriptwith no external prompt/script directory (confirmed:git ls-treeshows noassign-reviewers/asset dir; the only scripts are inlinescript:blocks). So the workflow file itself is the only path whose change can alter behavior — documented in a comment on the trigger.Out of scope (per ticket)
ASSIGN_REVIEWERS_CALLERS— a manual admin op (Actions variables aren't committed), done after this lands. Initial seed is cloud-only. Until then the flow is a documented clean no-op.assign-reviewersout to more repos (per-repo caller work, tracked separately).Tests
test-bump-callers.ymlpath filters (bothpull_requestandpush) so a change to it runs the shared-script suite — analogous to the coverage the sibling bumpers already have.shellcheckclean;bash .github/bump-callers/tests/test_bump_callers.sh→ 88 passed, 0 failed; both edited/new workflows parse as valid YAML; AGENTS.md integrity checker passes (123 lines, under the 200 gate).bump-callersREADME fleet table and the AGENTS.md catalog to keep docs in sync.BE-3924