ci(bump-callers): bring sibling entrypoints to parity with hardened auto-label bumper (BE-4058)#62
Conversation
…uto-label bumper (BE-4058) Mirror the three hardenings from 89e3110 (bump-cursor-review-auto-label-callers.yml, PR #60) onto the four pre-existing bump-*-callers entrypoints: - SHA-pin actions/checkout (v6.0.3) per the repo's pin-everything policy - main-only job guard: skip workflow_dispatch runs from non-main refs, which would pin every caller to a stale SHA and run bump-callers.sh from an unreviewed commit with the freshly minted App token - deletion no-op guard: a push that DELETES the reusable workflow matches the path filter; exit 0 instead of opening caller PRs pinned to a SHA where the referenced file no longer exists
mattmillerai
added
cursor-review
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 27 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 4 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 1 |
| 🟢 Low | 2 |
Panel: 8/8 reviewers contributed findings.
…commission (BE-4058) Review follow-ups from the cursor-review panel on PR #62: - A manual re-run of an older main run passes the `github.ref == 'refs/heads/main'` guard but executes with that run's stale github.sha, force-repinning every caller backwards. Verify github.sha is still the tip of main (git ls-remote) before bumping; a newer push always has its own queued run that does the bump. - The decommission guard only checked the reusable .yml, but the push path filter also fires on the run-time asset dir callers load from the pinned SHA (.github/cursor-review/**, .github/agents-md-integrity/**, scripts/check-pr-size/**). Deleting that dir while leaving the yml would repin callers to a SHA whose scripts are gone — check both. assign-reviewers has no asset dir; noted inline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:
|
Two conflicts, one textual and one semantic. Both sides' intents preserved. TEXTUAL — .github/bump-callers/bump-callers.sh, the pin-rewrite block. Main's BE-4523 (#75) added the multi-reusable attribution guard (GW_USES / SHA_ADDR tightening / the `main (<short>)` marker refresh); this branch added the groom callers' `# main @ <short>` rewrite. Resolved by keeping main's structure whole and placing the `# main @` rules in the SAME sed as the 40-hex rule, sharing `${SHA_ADDR}` verbatim. That placement — rather than folding them under the `GW_ONLY_OURS` guard — is what preserves both intents at once. `SHA_ADDR` is exactly the set of lines whose pin the 40-hex rule rewrites, and the `# main @` annotation rides ON that line, so the address is honest attribution for it: in the sibling regime the tightened address skips a sibling's `uses:` line, so neither its pin nor its annotation is stamped with our SHA. The `main (<short>)` form cannot use the same trick — it also appears in prose ABOVE the `uses:` line, naming no pin context — which is why it keeps the caller-level guard. Gating `# main @` on that guard instead would have manufactured the exact stale comment BE-4346 removed: a `workflows_ref:` pin is bumped even in a multi-reusable file (that context stays broad by main's own reasoning), so its annotation must move with it. Three assertions added for the merged interaction, which neither branch's suite covered. Mutation-checked both ways: dropping the `# main @` rules fails the groom fixture; widening them back to the pre-BE-4523 broad address fails the new sibling assertion. SEMANTIC — .github/workflows/bump-groom-callers.yml. No textual conflict (a new file against modified siblings), but main's BE-4058 (#62) hardened all four existing bump entrypoints while this branch adds a fifth written from the pre-hardening template, so the merged result shipped one unguarded fleet. Brought to parity: the main-only ref guard, the stale re-run tip check, the decommission guard (checking `.github/groom/` as well as `groom.yml`, since a groom caller pins both), and the SHA-pinned checkout the sweep moved to v6.0.3. The ref guard here is main's accidental-stale-dispatch guard, not a security control — the workflow_dispatch token-escalation path stays open and stays tracked as BE-4661, which needs the server-side environment gate. 123 bump-callers assertions pass, shellcheck clean, groom/agents-md/cursor-review suites green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 participants
ELI-5
We have five little robots that, whenever one of our shared workflow files changes, go tell every repo that uses it "hey, update to the new version." The newest robot (PR #60) got three safety upgrades after its review; the four older robots never got them. This PR gives the same three upgrades to the older four, copied word-for-word from the new one: (1) they now fetch their tools from an exact, tamper-proof address instead of a movable label; (2) they refuse to run if someone accidentally starts them from an old copy of the code (which would push everyone backwards); (3) if the file they announce got deleted, they say "nothing to announce" and stop instead of telling every repo to use a file that no longer exists.
What
Mirrors the three hardenings from commit 89e3110 (
bump-cursor-review-auto-label-callers.yml, PR #60's review panel) onto the four pre-existing sibling entrypoints —bump-cursor-review-callers.yml,bump-agents-md-callers.yml,bump-pr-size-callers.yml,bump-assign-reviewers-callers.yml— with the exact comments/idiom from the reference:actions/checkout@v6→actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3, per the repo's own AGENTS.md pin-by-SHA policy. Tag→commit mapping independently verified via the GitHub API (v6.0.3annotated tag dereferences todf4cb1c...); same pin already used bypr-size.ymland PR ci(bump-callers): add cursor-review-auto-label caller fleet (BE-4005) #60.if: github.ref == 'refs/heads/main'on thebumpjob. All siblings gainedworkflow_dispatchin ci(bump-callers): add workflow_dispatch for on-demand runs #15 and setNEW_SHA: ${{ github.sha }}; a manual dispatch from a non-main/older ref would pin every caller to that stale SHA (the bumper force-resets a stable sharedci/bump-*branch, so an old-ref run can overwrite a newer bump and downgrade consumers) and would runbump-callers.shfrom an unreviewed ref with the freshly minted App token. The push trigger is already main-only, so this only skips stray manual runs.WORKFLOW_FILEat the checked-out SHA and exits 0 ("absent at this SHA — decommissioned; nothing to bump") before invokingbump-callers.sh.bump-callers.shand its tests are untouched — workflow-YAML only, as the ticket scopes.Verification
shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh— cleanbash .github/bump-callers/tests/test_bump_callers.sh— 88 passed, 0 failedyaml.safe_load)v6.0.3tag→commit mapping verified live againstactions/checkout(annotated tag9f69817→ commitdf4cb1c...)Judgment calls / notes
main— no file overlap, so it's based onmainand merges independently in either order..github/cursor-review/**,scripts/check-pr-size/**), an asset-dir-only deletion with the workflow file still present still bumps — correct, since callers reference the workflow file, and this matches the ticket's "if the fleet's WORKFLOW_FILE is absent" spec and the 89e3110 idiom.cursor-review.yml,agents-md-integrity.yml, severaltest-*.yml) still use bareactions/checkout@v6/setup-python@v6. The ticket scopes this PR to the four bump entrypoints; the rest would be a separate sweep.