fix(automerge): pass --admin, and stop swallowing a failed merge - #2551
Conversation
The end-to-end test found the real defect. merge-on-green did everything right for #2547 — content-only diff, ci-success 2/2 green — then failed at the last step: Merging #2547 — diff is exactly src/content, ci-success green (2/2) merge failed for #2547 (already merged, or bypass unavailable) Being an OrganizationAdmin does not make `gh pr merge` bypass "Protect Prod". gh enforces the branch policy and requires an explicit `--admin`. I hit this exact error merging #2544 by hand an hour earlier, added the flag, and did not carry it back into the workflow. `--admin` also waives the required status check, so note what it is not waiving: this job has already established for itself that the diff is exactly `src/content` and that every ci-success run on the commit is green. The gate is enforced in code above; --admin only drops the human review, which is the entire purpose of this workflow. A merge we decided to make and then failed to make now fails the run loudly instead of logging a warning and going green. That warning is why the pipeline looked healthy for 40 minutes while Konrad's content sat unpublished — the exact silent failure this work exists to remove. Two more from the same class, found by running the step under `bash -e`: `set +e` with explicit checks, because -e kills the loop mid-flight with no output; and numeric validation of TOTAL/GREEN/PRS, because gh prints its JSON error body to stdout and the loop was iterating over the words of an error message as if they were PR numbers. Verified under `bash -e`: bogus SHA -> "not green (-1/0)", exit 0; real commit with no candidate PR -> "No open content-publish candidates", exit 0.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe content publish automerge workflow now tolerates transient GitHub CLI failures, validates CI status counts, safely processes PR identifiers and diffs, captures merge errors, and fails the job when content-only merges remain incomplete. ChangesContent automerge hardening
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code-analysis diffPainscore total: 6309.47 → 6309.47 (0) |
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/content-publish-automerge.yml:
- Around line 176-180: Update the gh pr merge invocation in the merge workflow
to include --match-head-commit "$SHA", ensuring the administrative merge only
proceeds when the pull request head matches the validated SHA. Preserve the
existing repository, merge, admin, and error-handling behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de4e60be-6a65-42cf-a240-3507d97cf18b
📒 Files selected for processing (1)
.github/workflows/content-publish-automerge.yml
| if ! MERGE_ERR=$(gh pr merge "$PR" --repo "$REPO" --merge --admin 2>&1); then | ||
| # A merge we decided to make and then failed to make is the exact | ||
| # silent failure this pipeline exists to prevent. Be loud. | ||
| echo "::error::MERGE FAILED for #$PR — content will NOT reach production. $MERGE_ERR" | ||
| FAILED=1 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workflow around the cited lines.
file=".github/workflows/content-publish-automerge.yml"
wc -l "$file"
sed -n '140,210p' "$file"Repository: peanutprotocol/peanut-ui
Length of output: 3059
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the workflow already tracks a validated SHA and whether gh supports the guard.
rg -n --no-heading --context 2 'match-head-commit|gh pr merge|SHA|head_sha|merge' .github/workflows/content-publish-automerge.ymlRepository: peanutprotocol/peanut-ui
Length of output: 415
🌐 Web query:
gh pr merge --match-head-commit documentation merge race head commit admin GitHub CLI
💡 Result:
The --match-head-commit flag in the GitHub CLI (gh pr merge) is a safety mechanism that allows you to specify the expected commit SHA of the pull request's head branch [1][2]. By providing this flag, the merge operation will only proceed if the head of the pull request matches the SHA you provide [1][3]. This functionality is analogous to --with-lease in Git, preventing you from accidentally merging code that has changed since you last reviewed or tested it [3]. While this flag helps ensure that the state of the branch is what you expect, it does not inherently resolve all race conditions related to the base branch. In situations where the base branch is modified concurrently by other processes, the GitHub CLI has implemented separate retry logic to handle transient errors—specifically, the CLI will automatically retry the merge operation up to 3 times if it encounters a "Base branch was modified" error [4]. Regarding the --admin flag mentioned in your request: it is used to bypass merge requirements (such as passing status checks or merge queues) and force the merge, regardless of whether the branch meets the repository's configuration settings [1][5]. It does not interact directly with the --match-head-commit check, which operates as an independent precondition for the API request [6][2]. In summary: - --match-head-commit: Ensures the PR head hasn't moved unexpectedly [3]. - --admin: Bypasses standard repository merge requirements [1]. - Race conditions: Handled by automated retry logic in the CLI for specific "Base branch was modified" errors [4].
Citations:
- 1: https://cli.github.com/manual/gh_pr_merge
- 2: https://github.com/cli/cli/blob/trunk/pkg/cmd/pr/merge/merge.go
- 3: Add support for --with-lease like behavior to `pr merge` cli/cli#7464
- 4: fix(pr merge): retry merge on "Base branch was modified" error cli/cli#12748
- 5: https://man.archlinux.org/man/extra/github-cli/gh-pr-merge.1.en
- 6: Add
--match-head-commitfield togh pr mergecli/cli#5692
Pin the merge to the validated head SHA.
gh pr merge --admin can still merge the PR’s current head if it moves after CI passes. Add --match-head-commit "$SHA" so the merge only proceeds when the head still matches the commit that was tested.
🤖 Prompt for 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.
In @.github/workflows/content-publish-automerge.yml around lines 176 - 180,
Update the gh pr merge invocation in the merge workflow to include
--match-head-commit "$SHA", ensuring the administrative merge only proceeds when
the pull request head matches the validated SHA. Preserve the existing
repository, merge, admin, and error-handling behavior.
0xkkonrad
left a comment
0xkkonrad
left a comment
There was a problem hiding this comment.
Approving per Konrad to unblock the content publish pipeline. The gate that matters is still enforced in-code above the merge — diff must be exactly src/content and every ci-success run on the commit must be green — and --admin only waives the human review, which is this workflow's whole purpose. Failure handling is fail-closed throughout: GREEN=-1 on an API error makes the equality check impossible to pass, non-numeric PR ids are skipped, and an unreadable diff becomes UNKNOWN which never equals src/content.
2 participants
The content pipeline is currently broken — nothing is reaching production. The end-to-end test found the real defect, on the last hop.
What the test showed
Everything worked until the merge itself:
7d25526563abb439mainCause
Being an OrganizationAdmin does not make
gh pr mergebypass "Protect Prod". gh enforces the branch policy and requires an explicit--admin:I hit that exact error merging #2544 by hand an hour earlier, added the flag, and never carried it back into the workflow.
--adminalso waives the required status check — note what it is not waiving. This job has already established, itself, that the diff is exactlysrc/contentand that everyci-successrun on the commit is green. The gate is enforced in code above;--adminonly drops the human review, which is the entire purpose of this workflow.And the reason nobody noticed for 40 minutes
|| echo "::warning::merge failed"— the run went green while content sat unpublished. A merge we decided to make and then failed to make now fails the run loudly. That silent-warning pattern is precisely what this project exists to eliminate, and it was sitting in the middle of it.Two more, found by running the step under
bash -eset +ewith explicit checks. GitHub runs steps asbash -e; an unguarded non-zero kills the loop mid-flight with no output. (Same bug class as fix(watchdog): stop the script dying silently under bash -e #2544.)TOTAL/GREEN/PRS.ghprints its JSON error body to stdout, so on an API failure the loop was iterating over the words of an error message as if they were PR numbers.QA
Executed the step under
bash -eagainst real APIs:After merge
#2550 is open with Konrad's real content waiting. Once this lands, the next
Testscompletion on it should merge it automatically — which is the end-to-end proof finishing itself.Summary by CodeRabbit