Skip to content

ci(bump-callers): add the cursor-review-auto-label fleet + repair frozen caller pins (BE-4682) - #81

Open
mattmillerai wants to merge 3 commits into
mainfrom
ci/auto-label-bump-fleet
Open

ci(bump-callers): add the cursor-review-auto-label fleet + repair frozen caller pins (BE-4682)#81
mattmillerai wants to merge 3 commits into
mainfrom
ci/auto-label-bump-fleet

Conversation

@mattmillerai

Copy link
Copy Markdown
Contributor

Enrolling a repo as a caller is two steps — merge the caller, and add the repo to its vars.*_CALLERS roster. 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 own ci-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-workflows reusable) across 112 active non-fork repos, against each bump roster:

Reusable Real callers Roster Frozen pins
cursor-review 19 14 6
detect-unreviewed-merge 12 none all 12 — deferred
agents-md-integrity 6 7 roster is ahead by 1
cursor-review-auto-label 5 none all 5
assign-reviewers 3 1 2
groom 3 2 1
pr-size 1 1 0
stale.yml 0 none
assign-prs-to-author.yml 0 none

This 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 in test-bump-callers.yml, the bump-callers README table, and AGENTS.md.

Two deliberate choices:

  • Separate fleet from cursor-review. Consumers pin the two reusables independently — many repos run cursor-review without auto-label — so one combined fleet would bump callers of one on a change to the other.
  • 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 stale and assign-prs-to-author have no fleet (zero callers — machinery with no users rots) and that detect-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_CALLERS 14 → 19 — ComfyUI-private, comfy-agent-testing-suite, comfy-cli, comfy-conventions, comfy-toolbox
  • ASSIGN_REVIEWERS_CALLERS 1 → 3 — website, github-workflows
  • GROOM_CALLERS 2 → 3 — github-workflows (ci-groom.yml)
  • AUTO_LABEL_CALLERS seeded with this fleet's 5

Adding this repo to GROOM_CALLERS self-heals the stale ci-groom.yml pin: the next groom.yml change bumps it automatically, and current main is 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_frontend at 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 code reported only 2 groom callers org-wide. It under-reports badly on private repos, so ground truth came from per-repo content reads.
  • My first scan over-credited this repo by matching its own reusables' header examples — each contains a uses: Comfy-Org/github-workflows/... line. It self-enrolls in exactly 4 fleets via ci-*.yml, not 7. Correcting that is what revealed stale and assign-prs-to-author have zero real callers; their only apparent caller was a comment.
  • Bulk content reads tripped GitHub's secondary rate limit, whose signature is worth knowing: /rate_limit showed core: 4969/5000 remaining — essentially untouched — while every content call returned "API rate limit exceeded". Secondary limits don't decrement the primary bucket, and /rate_limit is itself exempt, so bucket-remaining can't detect this.

Verification

  • 123 bump-callers functional tests pass; shellcheck -x clean
  • New entrypoint parses; VAR_NAME / TAG / WORKFLOW_FILE / ALLOW_EMPTY / concurrency group / path filter all asserted
  • All four variables re-read after writing and confirmed well-formed
  • check_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

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.
@mattmillerai mattmillerai added the cursor-review Multi-model cursor review label Jul 27, 2026
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 33 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dfc8a590-5da9-4c31-bef6-efd7ed1d67c1

📥 Commits

Reviewing files that changed from the base of the PR and between f9b0619 and 91022c1.

📒 Files selected for processing (3)
  • .github/bump-callers/README.md
  • .github/workflows/bump-auto-label-callers.yml
  • AGENTS.md
📝 Walkthrough

Walkthrough

Adds an automated cursor-review-auto-label.yml caller-bump workflow, wires it into bump-caller tests, and documents fleet enrollment, no-fleet workflows, deferred gaps, and groom’s double-pin synchronization.

Changes

Auto-label caller bumping

Layer / File(s) Summary
Auto-label bump workflow
.github/workflows/bump-auto-label-callers.yml
Adds dispatch and main push triggers, concurrency controls, GitHub App authentication, stale-run and decommission checks, and shared bumper execution using AUTO_LABEL_CALLERS.
Bump workflow validation wiring
.github/workflows/test-bump-callers.yml
Updates fan-out comments and path filters so auto-label caller-bump workflow changes trigger validation.
Fleet enrollment documentation
.github/bump-callers/README.md, AGENTS.md
Documents no-fleet workflows, the deferred unreviewed-merge gap, groom’s double pinning, and the required caller-plus-roster enrollment process.

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
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/auto-label-bump-fleet
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch ci/auto-label-bump-fleet

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between daf635a and f9b0619.

📒 Files selected for processing (4)
  • .github/bump-callers/README.md
  • .github/workflows/bump-auto-label-callers.yml
  • .github/workflows/test-bump-callers.yml
  • AGENTS.md

Comment thread .github/workflows/bump-auto-label-callers.yml Outdated
Comment thread AGENTS.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.
@mattmillerai

Copy link
Copy Markdown
Contributor Author

🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:

  • BE-4683 — Harden the main-tip lookup in the 5 remaining bump-caller entrypoints

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 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.

Comment thread .github/workflows/bump-auto-label-callers.yml
Comment thread .github/workflows/bump-auto-label-callers.yml
Comment thread .github/workflows/bump-auto-label-callers.yml Outdated
Comment thread .github/workflows/bump-auto-label-callers.yml
Comment thread .github/workflows/bump-auto-label-callers.yml
Comment thread .github/workflows/bump-auto-label-callers.yml
…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.
@mattmillerai

Copy link
Copy Markdown
Contributor Author

🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:

  • BE-4688 — Stop the bump-caller fleets leaking their private caller roster into public run logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cursor-review Multi-model cursor review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants