fix(bump-callers): commit all of a repo's files onto one bump branch (BE-3896)#45
Merged
mattmillerai merged 2 commits intoJul 21, 2026
Conversation
…(BE-3896) Group CALLERS entries by repo before the bump loop and reset the stable branch ONCE per repo, then commit each of that repo's files onto it with successive PUTs (each carrying that file's own blob SHA). The old per-entry loop reset the branch before every file, so a second same-repo entry's reset discarded the first entry's commit and the PR shipped only the last file while every entry reported success — a silent partial bump. Only affects repos that appear more than once in a caller list; single-file repos are unchanged. The test stub now models the one bump branch's committed file set (a ref reset truncates it, a PUT appends), and a new same-repo two-file case asserts BOTH files land on the branch — it fails against the pre-fix script.
mattmillerai
added
agent-coded
mattmillerai
marked this pull request as ready for review
July 21, 2026 07:00
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 | 1 |
| 🟡 Medium | 4 |
| 🟢 Low | 3 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
…bumps (BE-3896) Address cursor-review panel findings on the multi-file-per-repo bumper: - Distinguish a genuine 404 (per-file skip) from any other fetch error (auth/rate-limit/5xx/network); a transient error now fails the whole repo instead of silently shipping a PR that omits the un-fetched file. - Resolve the default-branch tip (MAIN_SHA) up front and pin every Pass-1 blob fetch to that immutable SHA, closing a TOCTOU race with a moving default branch. - De-duplicate staged files by path so a repo listed twice for the same file commits it once (a second PUT with a now-stale blob sha would 409 the repo). - Skip an already-pinned file by comparing rewritten-vs-original content instead of grepping for NEW_SHA anywhere, which also repairs a half-bumped file. - Anchor the 40-hex SHA rewrite to the github-workflows / workflows_ref pin contexts so a full-SHA pin of another action (actions/checkout@<sha>) is not clobbered. - Collect a caller entry's label only once its file is confirmed staged, so a skipped entry's label never lands on the real bump PR. - Exact-match label de-dup (GitHub label names may contain the `|` sentinel). Adds functional tests for each: 404-skip vs transient-fail, same-file de-dup, non-github-workflows pin preservation, half-bump repair, and already-pinned skip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:
|
mattmillerai
merged commit Jul 21, 2026
f08a97e
into
matt/be-3882-stable-bump-branch
4 checks passed
mattmillerai
added a commit
that referenced
this pull request
Jul 21, 2026
…-3882) (#44) * fix(bump-callers): reuse one open PR per repo via a stable branch (BE-3882) The dispatcher embedded the new short-SHA in the branch name (ci/bump-<tag>-<short>), so every bump minted a unique branch and thus a brand-new PR in each caller repo, leaving the prior bump PRs open. The stack made it unclear which pin was current. Make the branch stable per (repo, TAG) — ci/bump-<tag> — and update it in place: each run rebuilds the branch from the caller's current default-branch tip (a clean single-commit "bump to @short" diff), then, if a bump PR is already open for that branch, refreshes its title/body to the new SHA instead of opening another; a fresh PR is opened only when none is open (first bump, or the prior one merged/closed since the last run). Result: at most one open bump PR per (repo, workflow) at any time. Applies to both the cursor-review and agents-md fleets via the shared script. Adds test coverage for the update-in-place path (open PR is edited, not re-opened) and the create path (no edit when no PR is open). * fix(bump-callers): guard existing-PR lookup + fail loud on create error (BE-3882) Harden the stable-branch bump PR reuse from the review panel: - Existing-PR lookup: use `.[0].number // empty` (a bare `.[0].number` prints the literal `null` on an empty list, so the first bump for a repo — and every bump after the prior PR merged/closed — ran `gh pr edit null` and failed the caller). Also exclude cross-repository PRs (`isCrossRepository == false`): `--head` matches by branch name across forks, so with the now-predictable branch name an attacker could pre-open a fork PR the bot would stamp instead of bumping the real caller. - Create path: `return 1` on a genuine `gh pr create` failure. The update-in-place path already handles an open PR, so reaching create means none exists and a failure is a real miss — record it in FAILED instead of reporting success. - Serialize each fleet with a `concurrency:` group (cancel-in-progress: false) so overlapping runs can't race the shared stable branch; the newest pending run wins so the latest SHA is committed. - Tests: the gh stub now faithfully models `gh pr list --json --jq` by running the real jq over the JSON gh would return, so the no-open-PR case reproduces production instead of masking it; add a fork-PR decoy regression case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(bump-callers): commit all of a repo's files onto one bump branch (BE-3896) (#45) * fix(bump-callers): commit all of a repo's files onto one bump branch (BE-3896) Group CALLERS entries by repo before the bump loop and reset the stable branch ONCE per repo, then commit each of that repo's files onto it with successive PUTs (each carrying that file's own blob SHA). The old per-entry loop reset the branch before every file, so a second same-repo entry's reset discarded the first entry's commit and the PR shipped only the last file while every entry reported success — a silent partial bump. Only affects repos that appear more than once in a caller list; single-file repos are unchanged. The test stub now models the one bump branch's committed file set (a ref reset truncates it, a PUT appends), and a new same-repo two-file case asserts BOTH files land on the branch — it fails against the pre-fix script. * fix(bump-callers): harden per-file staging against partial/erroneous bumps (BE-3896) Address cursor-review panel findings on the multi-file-per-repo bumper: - Distinguish a genuine 404 (per-file skip) from any other fetch error (auth/rate-limit/5xx/network); a transient error now fails the whole repo instead of silently shipping a PR that omits the un-fetched file. - Resolve the default-branch tip (MAIN_SHA) up front and pin every Pass-1 blob fetch to that immutable SHA, closing a TOCTOU race with a moving default branch. - De-duplicate staged files by path so a repo listed twice for the same file commits it once (a second PUT with a now-stale blob sha would 409 the repo). - Skip an already-pinned file by comparing rewritten-vs-original content instead of grepping for NEW_SHA anywhere, which also repairs a half-bumped file. - Anchor the 40-hex SHA rewrite to the github-workflows / workflows_ref pin contexts so a full-SHA pin of another action (actions/checkout@<sha>) is not clobbered. - Collect a caller entry's label only once its file is confirmed staged, so a skipped entry's label never lands on the real bump PR. - Exact-match label de-dup (GitHub label names may contain the `|` sentinel). Adds functional tests for each: 404-skip vs transient-fail, same-file de-dup, non-github-workflows pin preservation, half-bump repair, and already-pinned skip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI-5
The SHA-bumper opens one PR per repo on a fixed branch (
ci/bump-<tag>). To keep that PR a clean "bump to @short" diff, it wipes the branch back to the repo's default-branch tip before committing. That wipe ran once per file. So when one repo pins the reusable workflow from two files (a monorepo), the second file's wipe erased the first file's commit — the PR ended up with only the last file bumped, even though every file "succeeded." This groups the work by repo: wipe the branch once, then commit each of that repo's files onto it, then open/update the single PR. Now both files ride along.What changed
bump-callers.sh:bump_one()(per-entry) →bump_repo()(per-repo). The driving loop now groups the flatrepo|file|labelentry list by repo (first-seen order preserved) and callsbump_repoonce per repo with all its entries.tests/test_bump_callers.sh: theghstub now models the one bump branch's committed file set (agit/refscreate/reset truncates it; a contents PUT appends the file). A new monorepo case drives two entries for the SAME repo and asserts both files land on the branch and exactly one PR is opened.Only triggers on the multi-file path
Single-file-per-repo callers are structurally unchanged — the 37 pre-existing test assertions (single caller, PR reuse, decoy-fork rejection, two distinct repos, empty/malformed handling) all pass untouched.
Testing
bash .github/bump-callers/tests/test_bump_callers.sh→ 45 passed, 0 failed (37 existing + 8 new).shellcheck -x .github/bump-callers/bump-callers.sh .github/bump-callers/tests/test_bump_callers.sh→ clean.ci-bsurvives, two PRs opened), exactly matching the bug. Against this fix they pass.Deferred from a review thread on #44 (BE-3896).