fix(bump-callers): reuse one open PR per repo via a stable branch (BE-3882)#44
Merged
Merged
Conversation
…-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).
mattmillerai
added
agent-coded
mattmillerai
marked this pull request as ready for review
July 21, 2026 03:15
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe caller bump process now operates per repository, reuses stable branches and open pull requests, distinguishes missing files from fetch failures, and serializes each caller fleet’s workflow runs. ChangesCaller bump consolidation
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant bump_repo
participant ContentsAPI
participant PullRequestAPI
Workflow->>bump_repo: Group caller entries by repository
bump_repo->>ContentsAPI: Fetch files at default-branch tip
ContentsAPI-->>bump_repo: Return contents or fetch status
bump_repo->>PullRequestAPI: Reset branch and commit updated files
bump_repo->>PullRequestAPI: Update existing PR or create one
✨ 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 7 finding(s).
| Severity | Count |
|---|---|
| 🔴 Critical | 1 |
| 🟡 Medium | 5 |
| 🟢 Low | 1 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
…or (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>
Contributor
Author
|
🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:
|
…(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>
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
When we update a shared workflow here, a bot goes to every repo that uses it and opens a PR to point at the new version. The bug: it named each PR's branch after the new version's ID, so every update opened a brand-new PR and left the old ones sitting open — a growing pile where you couldn't tell which pin was current.
Now the bot uses one fixed branch name per repo (
ci/bump-<tag>). On the next update it reuses that same branch — rebuilding it fresh from the repo's current main and refreshing the existing PR's diff/title/body — instead of opening another. So there's at most one open bump PR per repo per workflow at any time, always showing the latest pin.What changed
.github/bump-callers/bump-callers.sh(the one shared script behind both thebump-cursor-review-callersandbump-agents-md-callersfleets):BRANCH="ci/bump-${TAG}"(dropped the${SHORT}suffix). The SHA now lives only in the commit/title/body.PATCH-force it back to the tip if it already exists, then apply the single file-pin change. The PR diff is always a clean single "bump to @short" commit — no accumulated stale commits, no drift if the caller's main moved.gh pr list --head "$BRANCH" --state opendecides: if a bump PR is already open,gh pr editrefreshes its title/body (they embed${SHORT}) — no second PR. If none is open (first bump, or the prior one merged/closed since last run),gh pr createopens a fresh single one.Acceptance criteria
--state opencheck falls through to create; a residual branch is force-reset first).bump-callers.sh, so it applies to both the cursor-review and agents-md dispatchers.Tests
bash .github/bump-callers/tests/test_bump_callers.sh+shellcheck -x— 33/33 pass. Added coverage for the new update-in-place path (open PR is edited, not re-opened) and asserted the create path does no edit when no PR is open. Theghstub gainedpr list/pr edit/PATCHhandling and aSTUB_OPEN_PRtoggle.Judgment calls / notes
ci/bump-*branch, so the only thing discarded is the prior bump commit — exactly what we're replacing. It does refresh an open PR's diff out from under any in-progress review, but that's the desired "update to latest SHA" behavior for an automated bump PR.--field force=truerelies on gh's typed--fieldform sending a JSON boolean (same--fieldform the script already uses); confirmed against the refsPATCHAPI.