ci(bump-callers): harden the main-tip lookup in the 5 sibling entrypoints (BE-4683) - #82
Conversation
…ints (BE-4683) `main_tip=$(git ls-remote origin refs/heads/main | cut -f1)` reports the exit status of `cut`, not of `ls-remote`, so the step's `bash -e` never sees a failed lookup. On an ls-remote failure (network blip, remote hiccup, rate limit) main_tip was set to the empty string, the staleness guard compared empty != github.sha, printed the stale-run message and `exit 0` — a green run that bumped nothing, indistinguishable from a green run that bumped everything unless someone read the log. That is precisely the silent-drift failure mode this machinery exists to prevent. Port the shape already landed in the auto-label entrypoint: run ls-remote on its own so its status is checked, reject empty output, and only then compare against github.sha. A lookup we could not perform is not evidence of staleness — fail loudly instead. Applied verbatim to all five siblings (cursor-review, groom, assign-reviewers, pr-size, agents-md); each file's surrounding comment is preserved and the stale-run branch is unchanged.
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: 42 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 (5)
✨ 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.
✅ No high-signal findings.
Panel: 8/8 reviewers contributed findings.
2 participants
ELI-5
Five of our "bump the pinned SHA in every caller repo" workflows looked up the current tip of
mainlike this:main_tip=$(git ls-remote origin refs/heads/main | cut -f1). A shell pipeline reports the exit status of its last command, sobash -eonly ever sawcutsucceed — a failedls-remote(network blip, remote hiccup, rate limit) quietly leftmain_tipempty. Empty then compared unequal togithub.sha, so the guard printed "stale run/re-run; nothing to bump" andexit 0. The workflow reported success while bumping nothing, and a green run that bumped nothing looks exactly like a green run that bumped everything unless a human reads the log. That is precisely the silent-drift failure this machinery exists to prevent (AGENTS.md records a pin that never moved causing a realci-groom.ymlstartup failure).This runs
ls-remoteon its own so its exit status is actually checked, rejects empty output, and only then compares againstgithub.sha. A lookup we couldn't perform is not evidence of staleness — so it now fails loudly instead of pretending the run was stale.What changed
The fix already landed in
bump-auto-label-callers.yml(commit7b360ed, branchci/auto-label-bump-fleet/ #81) is ported verbatim to the five sibling entrypoints that #81 left untouched:.github/workflows/bump-cursor-review-callers.yml.github/workflows/bump-groom-callers.yml.github/workflows/bump-assign-reviewers-callers.yml.github/workflows/bump-pr-size-callers.yml.github/workflows/bump-agents-md-callers.ymlEach file's surrounding explanatory comment is preserved and the stale-run branch itself is byte-for-byte unchanged — the only behavior change is that a failed or empty lookup now exits 1 with a
::error::annotation instead of falling through to the stale-runexit 0.Verification
bash .github/bump-callers/tests/test_bump_callers.sh→ 123 passed, 0 failed.shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh→ clean.run:blocks, so additionally: all five files parse as YAML, and each editedrun:block passesbash -nandshellcheck -s bashafter extraction.daf635a…); a bogus remote exits 1 with::error::Could not look up the current main tip; a non-existent ref (empty output) exits 1 with::error::git ls-remote returned no SHA. Confirmed under bothset -euo pipefailand plainset -e(GitHub's defaultrunshell), sinceif ! var=$(cmd)suspends-ein the condition and must not swallow the failure.Risk / falsification
The change converts a silent skip into a hard failure, so the one real risk is
git ls-remote originnot working in the runner at all — it runs afteractions/checkoutwithpersist-credentials: false. Falsified empirically against the real runner, not assumed: the log of run 30222872404 (bump-groom-callers, 2026-07-26,success) shows thels-remoteline executing, no stale-tip message, and the job reachinggroom bump complete for all callers. The repo is public, so anonymous HTTPSls-remotesucceeds; reproduced locally againsthttps://github.com/Comfy-Org/github-workflows.git.Judgment calls / notes
main_tip=${ls_remote%%$'\t'*}:$'\t'is a bash-ism, and under a non-bash shell it would strip nothing and reintroduce the exact silent no-op. That is safe here because the surrounding guard already uses[[ ]], so these blocks are bash-only by construction — and it is verified by the extraction checks above rather than by inspection.grep -rn ls-remoteover the repo returns only these five files, and there are no other unguardedx=$(… | cut)assignments under.github/workflows/.bump-auto-label-callers.ymlis not onmainyet — it lands with ci(bump-callers): add the cursor-review-auto-label fleet + repair frozen caller pins (BE-4682) #81 already carrying this fix.main, not stacked on ci(bump-callers): add the cursor-review-auto-label fleet + repair frozen caller pins (BE-4682) #81 — these five files are pre-existing and ci(bump-callers): add the cursor-review-auto-label fleet + repair frozen caller pins (BE-4682) #81 does not touch them, so there is no dependency in either direction.run:blocks, which the repo has no harness for; the extraction + branch-exercise checks above are the substitute. Adding such a harness would be a bigger, separate change than this mechanical port.Closes BE-4683.