ci(bump-callers): add the cursor-review-auto-label fleet + repair frozen caller pins (BE-4682) - #81
ci(bump-callers): add the cursor-review-auto-label fleet + repair frozen caller pins (BE-4682)#81mattmillerai wants to merge 3 commits into
Conversation
cursor-review-auto-label.yml had 5 callers and no bumper, so none of their pins had ever moved — they were frozen on whatever SHA each was first wired to. Measured across 112 active non-fork repos, comparing real callers (files whose CONTENT references a reusable) against each bump roster: cursor-review 19 callers / 14 rostered -> 6 frozen cursor-review-auto-label 5 callers / 0 rostered -> 5 frozen assign-reviewers 3 callers / 1 rostered -> 2 frozen groom 3 callers / 2 rostered -> 1 frozen detect-unreviewed-merge 12 callers / no fleet -> deferred stale, assign-prs-to-author 0 callers -> no fleet needed The roster halves were repaired in the Actions variables (no commit needed). This commit adds the missing fleet: a thin entrypoint over the shared bump-callers.sh — not a fork, that script stays one file — plus registration in test-bump-callers.yml, the bump-callers README table and AGENTS.md. Kept separate from the cursor-review fleet deliberately: consumers pin the two reusables independently (many run cursor-review without auto-label), so a single fleet would bump callers of one on a change to the other. ALLOW_EMPTY is false because this fleet is seeded non-empty, so an empty list means a clobbered variable rather than a genuinely unpopulated fleet. Also records why stale/assign-prs-to-author have no fleet (no callers) and that detect-unreviewed-merge's absence is a deferred gap, so neither reads as an oversight to whoever looks next.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 33 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 (3)
📝 WalkthroughWalkthroughAdds an automated ChangesAuto-label caller bumping
Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant GitHubApp
participant BumpCallers
participant CallerRepositories
GitHubActions->>GitHubApp: Generate app token
GitHubActions->>BumpCallers: Execute with AUTO_LABEL_CALLERS and bump parameters
BumpCallers->>CallerRepositories: Create SHA-bump PRs
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/bump-auto-label-callers.yml:
- Around line 120-123: Update the main-tip lookup in the workflow’s bump logic
to detect and propagate a failed git ls-remote operation instead of allowing the
cut pipeline to produce an empty main_tip. Make the lookup failure exit nonzero
with an appropriate error path, while preserving the existing stale-run
comparison for successful lookups.
In `@AGENTS.md`:
- Around line 81-88: The fleet-variable roster documentation is missing
AUTO_LABEL_CALLERS. Update the adjacent convention in AGENTS.md to include
AUTO_LABEL_CALLERS alongside CURSOR_REVIEW_CALLERS and AGENTS_MD_CALLERS,
keeping the enrollment guidance complete for the ALLOW_EMPTY=false bumper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b544df4e-17ff-41a7-9b0e-b1dd46092cf1
📒 Files selected for processing (4)
.github/bump-callers/README.md.github/workflows/bump-auto-label-callers.yml.github/workflows/test-bump-callers.ymlAGENTS.md
…4682) CodeRabbit review on #81, two findings: 1. `main_tip=$(git ls-remote origin refs/heads/main | cut -f1)` reports `cut`'s exit status, so a failed ls-remote leaves `main_tip` EMPTY. The stale-run guard then compares empty != github.sha, prints "not the current main tip", and exits 0 — a transient network blip silently skips the whole fleet bump while the run reports success. Silent pin drift is exactly the failure this fleet exists to prevent, so a lookup we could not perform now exits nonzero instead of masquerading as staleness. Verified against a real sibling run (groom fleet, run 30222872404): the step gets past the tip check and opens PRs, confirming `git ls-remote origin` succeeds under `persist-credentials: false` on a public repo — the new hard failure cannot fire spuriously. 2. AGENTS.md listed only two of the six per-fleet caller variables; rounded the roll call out and pointed at the README table as canonical. The five sibling bump entrypoints share defect (1); deferred to a follow-up rather than widening this PR.
|
🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:
|
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 6 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 2 |
| 🟡 Medium | 2 |
| 🟢 Low | 2 |
Panel: 8/8 reviewers contributed findings.
…out (BE-4682) Cursor review panel findings on the new auto-label bump fleet: - The `main_tip != github.sha` guard exited 0 on ANY newer commit on main. The push trigger is path-filtered to cursor-review-auto-label.yml, so an unrelated commit landing between trigger and check starts no run of its own — the only run for the change was silently discarded and callers stayed frozen, the exact pin-drift this fleet exists to prevent. Skip now only when the WATCHED file actually changed since (a later commit then did touch the path and has its own run); an unrelated move proceeds. Also treats the file being gone at main tip as decommissioned. - Scope the App token to contents/pull-requests/issues: write instead of every permission the app holds on every installed repo — mirrors the groom entrypoint, which already documents why `repositories:` cannot be narrowed. - Add timeout-minutes: 20 so a hung git/gh call can't hold a runner for 6 hours and block later bumps behind the serializing concurrency group. - Correct the header's masking claim: the roster is passed via the step `env:`, which Actions dumps before bump-callers.sh's ::add-mask:: ever runs, so it IS visible in the public log. Documented as a known fleet-wide gap here and in the bump-callers README rather than overclaimed. - README: registering a fleet now says to workflow_dispatch it once after merge — a fleet's own merge touches no path filter, so already-stale callers stay stale otherwise.
|
🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:
|
Enrolling a repo as a caller is two steps — merge the caller, and add the repo to its
vars.*_CALLERSroster. The second gets skipped, and when it does the pin never moves: the caller drifts behind the reusable indefinitely, then fails much later with no obvious cause. This repo did it to its ownci-groom.yml, whose pin sat unchanged from the day it was written until it failed at startup.I measured every reusable's real callers (files whose content references a
Comfy-Org/github-workflowsreusable) across 112 active non-fork repos, against each bump roster:stale.ymlassign-prs-to-author.ymlThis PR
Adds the missing cursor-review-auto-label fleet — 5 callers, no bumper, so not one of their pins had ever moved. A thin entrypoint over the shared
bump-callers.sh(not a fork — that script stays one file), plus registration intest-bump-callers.yml, the bump-callers README table, and AGENTS.md.Two deliberate choices:
ALLOW_EMPTY: "false". Unlike the fleets seeded[], this one starts non-empty. An empty list here means the variable got clobbered, not that the fleet is unpopulated, and failing loudly beats silently bumping nothing.The README now also records why
staleandassign-prs-to-authorhave no fleet (zero callers — machinery with no users rots) and thatdetect-unreviewed-merge's absence is a deferred gap, not an oversight. That table is the part I'd most like a second opinion on.Already applied outside this PR (Actions variables, no commit)
Prior values backed up first.
CURSOR_REVIEW_CALLERS14 → 19 —ComfyUI-private,comfy-agent-testing-suite,comfy-cli,comfy-conventions,comfy-toolboxASSIGN_REVIEWERS_CALLERS1 → 3 —website,github-workflowsGROOM_CALLERS2 → 3 —github-workflows(ci-groom.yml)AUTO_LABEL_CALLERSseeded with this fleet's 5Adding this repo to
GROOM_CALLERSself-heals the staleci-groom.ymlpin: the nextgroom.ymlchange bumps it automatically, and currentmainis at/after the BE-4214 CLI migration, so the shorter permission grant that caller declares stays valid.Deliberately excluded:
ComfyUI-ghsa-f328-9qc2-fjcf. It runs cursor-review, but it's an ephemeral security-advisory fork that exists to ship one embargoed fix — it doesn't need to stay current, and an automated PR arriving mid-embargo is noise. Trivial to add if you disagree.Left alone: the agents-md roster lists
ComfyUI_frontendat a caller path that doesn't exist. That belongs to the in-flight wave-2 program (BE-3921) and the right fix is landing that repo's caller, not editing the roster around it.Measurement notes
Three things that would have produced wrong answers if taken at face value:
gh search codereported only 2 groom callers org-wide. It under-reports badly on private repos, so ground truth came from per-repo content reads.uses: Comfy-Org/github-workflows/...line. It self-enrolls in exactly 4 fleets viaci-*.yml, not 7. Correcting that is what revealedstaleandassign-prs-to-authorhave zero real callers; their only apparent caller was a comment./rate_limitshowedcore: 4969/5000remaining — essentially untouched — while every content call returned "API rate limit exceeded". Secondary limits don't decrement the primary bucket, and/rate_limitis itself exempt, so bucket-remaining can't detect this.Verification
shellcheck -xcleanVAR_NAME/TAG/WORKFLOW_FILE/ALLOW_EMPTY/ concurrency group / path filter all assertedcheck_agents_md.py --root .passes (1 pre-existing CODEOWNERS warning)Heads-up: #80 also edits
AGENTS.md, so whichever lands second may want a trivial conflict resolution in the conventions list.BE-4682
🤖 Generated with Claude Code