Skip to content

fix: use --admin bypass merge instead of --auto on approval - #13

Closed
don-petry wants to merge 3 commits into
mainfrom
claude/dazzling-cerf
Closed

don-petry wants to merge 3 commits into
mainfrom
claude/dazzling-cerf

Conversation

@don-petry

Copy link
Copy Markdown
Collaborator

Summary

  • Replaces gh pr merge --auto --squash with gh pr merge --squash --admin in both cascade-action.md and single-review.md
  • Adds a 5-second sleep after the update-branch rebase call to let GitHub process the branch update before merging
  • Non-approval (escalate) paths are unchanged — --admin is only invoked when the agent's decision is approve

Why

--auto queues a merge gated on further required approvals, which the bot account couldn't satisfy. --admin bypasses branch protection rules immediately — don-petry has bypass permissions configured on these repos.

Test plan

  • Trigger a manual workflow_dispatch run against a known-approvable PR with dry_run=false
  • Verify the PR shows an Approved review event followed by a merged state
  • Trigger against a PR the agent would escalate — confirm no merge occurs

🤖 Generated with Claude Code

Replace `--auto --squash` with `--squash --admin` in both cascade-action
and single-review prompts. Approval decisions now immediately bypass branch
protection rules (don-petry has bypass permissions) rather than queuing
for auto-merge. Non-approval (escalate) paths are unchanged — no bypass
merge is triggered.

Also adds a 5-second sleep after the rebase/update-branch call to give
GitHub time to process the branch update before the merge is attempted.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 17, 2026 02:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the agent action prompts used by the PR-review automation to merge approved PRs via an admin bypass path (instead of GitHub auto-merge), and adds a short wait after branch updates to reduce merge races.

Changes:

  • Switch approval-path merge command from gh pr merge --auto --squash to gh pr merge --squash --admin.
  • Add a 5-second sleep after the update-branch (rebase) API call before merging.
  • Document why --admin is used and explicitly forbid --auto in these approval flows.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
prompts/single-review.md Updates single-review approval actions to use admin bypass merge and adds a post-rebase delay.
prompts/cascade-action.md Updates cascade approval actions to use admin bypass merge and adds a post-rebase delay.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread prompts/cascade-action.md Outdated
Comment thread prompts/single-review.md
Comment thread prompts/single-review.md Outdated
Comment thread prompts/cascade-action.md
…ompts

- Replace fixed `sleep 5` with a bounded poll loop (6×5s) that exits as
  soon as `mergeStateStatus` is no longer BEHIND, avoiding the race
  between update-branch and the subsequent merge call
- Update synthesize.md to use `--squash --admin` instead of `--auto --squash`
  so all three action prompts (cascade-action, single-review, synthesize)
  are consistent
- Remove stale "same logic as synthesize.md" / "same as synthesizer"
  cross-references that caused documentation drift

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@don-petry

Copy link
Copy Markdown
Collaborator Author

Automated review — NEEDS HUMAN REVIEW

Risk: HIGH
Reviewed commit: 2eb448cd2f0d914d3f568c549106f8fba53778f8
Cascade: triage → audit (see triage: haiku 4.5 → deep: sonnet 4.6 + duck: gpt-5.4 → audit: opus 4.6 for models)

Summary

This PR replaces --auto --squash with --squash --admin across all three AI review agent prompts, enabling the bot to bypass ALL branch protection rules on approval. While the stated problem (bot can't satisfy required-approvals gate) is legitimate, --admin is an overbroad fix that also bypasses status checks, signed commits, and linear history requirements. Combined with the self-modifying nature of this repo (the AI agent reviews changes to its own prompts) and the autonomous review-approve-merge loop, this removes every automated safety net and warrants explicit human sign-off.

Findings

Critical

  • [critical] prompts/cascade-action.md:81 — gh pr merge --squash --admin bypasses ALL branch protection rules — required status checks, required reviews, signed commits, linear history, and branch restrictions. The PR body states the problem was specifically that --auto gated on required approvals the bot can't satisfy. --admin is a sledgehammer: it bypasses protections that were NOT blocking (e.g., status checks). A more targeted fix would be to add the bot account as a bypass actor in branch protection settings, or to configure the required-reviews rule to exclude bot-initiated merges.
  • [critical] This repo contains the prompt files that control the AI review agent's behavior. The AI agent can review and approve changes to its own prompts. With --admin, a successfully manipulated approval of prompt changes would be merged immediately with no external gate — creating a recursive trust loop. An attacker (or a bug in the AI's judgment) could modify the prompts to weaken review criteria, and the weakened criteria would then govern future reviews.

Major

  • [major] The architecture creates a fully autonomous loop: AI reviews PR → AI approves → AI merges (with --admin). Previously, --auto preserved status checks as a fallback gate. With --admin, there is zero automated safety net between AI approval and code landing on the default branch. The claude.yml workflow then picks up fix-request comments, making the entire pipeline self-sustaining with no mandatory human touchpoint.
  • [major] The AI review agent reads PR diffs (user-controlled content) and makes approve/escalate decisions. Prompt injection via crafted diff content, commit messages, or PR body could manipulate the agent into approving. With --auto, branch protections (status checks, required reviews) served as a defense-in-depth layer. With --admin, a successful prompt injection leads directly to merge with no fallback.
  • [major] prompts/synthesize.md:77 — These prompt files are used by pr-review.yml which enumerates PRs across repos via scripts/list-prs.sh. The GH_PAT secret grants the bot permissions across multiple repos. Any repo where the PAT has admin/bypass permissions will now have its branch protections bypassed on AI-approved merges — not just this repo. The --admin change affects every repo in the bot's scope.

Minor

  • [minor] prompts/synthesize.md:72 — All merge and rebase operations swallow errors. Combined with --admin (which is an immediate merge, not queued), a failed merge after a successful --approve review could leave the PR in an approved-but-unmerged state with no notification. The poll loop also has no hard abort — after 30s it proceeds to merge regardless of branch state.

Info

  • [info] CI is green. The change is consistent across all three prompt files. The poll loop replacing sleep 5 is a correctness improvement. The PR body clearly documents the rationale and includes a test plan. The escalation path (non-approve decisions) is unchanged and unaffected by this PR.

CI status

CI is passing (mergeStateStatus: CLEAN). All checks green.


Reviewed by the don-petry PR-review cascade (triage: haiku 4.5 → deep: sonnet 4.6 + duck: gpt-5.4 → audit: opus 4.6). Reply with @don-petry if you need a human.

@don-petry don-petry added the needs-human-review Flagged by automated PR review agent label Apr 17, 2026
Add a hard CI gate in review-one-pr.sh (step 1b) that runs immediately
after the head SHA fetch, before triage or any LLM calls. PRs with
failing/errored checks (FAILURE, ACTION_REQUIRED, TIMED_OUT, CANCELLED)
or still-running checks (IN_PROGRESS, QUEUED, WAITING) exit with code
100 (no-op sentinel) so they don't count against the MAX_PRS budget.
PRs with no CI checks at all are treated as passing.

Also remove the redundant CI-failing escalation signal from triage.md —
failing-CI PRs can no longer reach the triage tier.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@don-petry

Copy link
Copy Markdown
Collaborator Author

Abandoning --admin approach — too broad a bypass. Starting over with a dedicated bot account so the approval gate works correctly without bypassing CI checks.

@don-petry don-petry closed this Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-human-review Flagged by automated PR review agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants