ci: opt-in (label-gated) Review Auto-Fix for bot reviews (⑤ aggressive variant) - #1796
Conversation
The aggressive counterpart to review-comment-monitor.yml (#1789): on a trusted-bot review of a PR carrying the human-applied 'pr-trim:auto' label, runs a conservative pr-trim pass that applies clearly-actionable fixes, pushes to the PR branch, and resolves the addressed threads. Safety: HUMAN label gate (never fires unsolicited); changes land as reviewable commits; the pass is instructed to skip P3/judgment/behaviour-changing items and touch only files in the bot's findings. contents:write is scoped to this opt-in path only (the default monitor stays contents:read). allowed_bots set per the #1789 lesson. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 18 minutes and 12 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7084ed4f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addresses Codex P1 on #1796: actions/checkout's ref resolves against the base repo, so a fork PR with a base-named head branch (e.g. main) could make the autofix job check out + push its commit to the wrong repo. Restrict the job to same-repo PRs (head.repo.full_name == github.repository) so head==base and the ref is safe. PMOVES PRs are internal org branches, so this loses no coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
POWERFULMOVES
left a comment
POWERFULMOVES
left a comment
There was a problem hiding this comment.
Pair-review pass from Z890-CLAUDE
The safety architecture here is genuinely well thought out and the inline comments are the real product. Splitting always-on triage (review-comment-monitor.yml, contents:read) from opt-in mutation (this, contents:write) is the right boundary, and the triple-gate (same-repo AND human pr-trim:auto label AND trusted-bot login) is appropriately paranoid for a write-capable, bot-triggered job. The action pins match the repo's sha_pinning_required invariant and are identical to the sibling workflow (no drift), allowed_bots is set per the #1789 lesson, and the PR body's honest "can't be e2e-confirmed until a labelled bot review lands" caveat is the right disclosure. I verified the label exists and that there's no recursive-trigger risk (GITHUB_TOKEN pushes don't re-fire workflows, and the trigger is pull_request_review not push). This is mergeable.
3 observations surfaced for follow-up commit (non-blocking):
1. checkout ref is the branch tip, not the reviewed commit (TOCTOU window)
The review fires on github.event.review.commit_id, but ref: ${{ github.event.pull_request.head.ref }} (line 49-52) checks out the branch tip at run time. If a human pushes between the bot review and this job (per-PR concurrency with cancel-in-progress: false means jobs can queue and run late), the fix is computed from findings against commit A but pushed on top of commit B. The read-only monitor has no such exposure; a writer does. Suggest checking out github.event.review.commit_id, or having the prompt verify the head SHA still matches review.commit_id before pushing and bail otherwise.
2. same-repo gate comment conflates "head==base repo" with "head==base branch"
Lines 23-26 say restricting to same-repo PRs "makes head==base, so the ref below is safe." The real property is head==base repo — head and base branches are still different (that's what a PR is). The gate and the fork-attack reasoning are correct; only the head==base shorthand is imprecise and could mislead a future maintainer into thinking a branch comparison is the guard. Reword to "...head.ref resolves against a branch that actually exists in this repo, so checkout cannot be redirected to a fork."
3. concurrency group dropped review.id vs the sibling monitor
review-comment-monitor.yml keys on ...-${{ github.event.review.id }}; this keys on PR number only (line 12-14). For a writer that's the safer choice (serializes pushes), so likely intentional — but it's undocumented drift from the established sibling, and the per-PR serialization is exactly what widens the TOCTOU window in #1. A one-line comment ("per-PR concurrency intentionally serializes pushes so two bot reviews can't race the head branch") prevents a future "make it match the monitor" regression.
Nit (skip if scope creep)
See observation #2 — that's the nit; the head==base wording is a learning about being precise in security-rationale comments that future maintainers lean on.
Disposition
All three are follow-up commits, not blockers. The triple-gate plus human-reviewed-before-merge invariant bounds the blast radius even with the TOCTOU window open, and no contract/security correctness issue is unaddressed. Recommend ready — fold #1 into the next touch of this file.
agent_signature (advisory unsigned-local): ACK::Z890-CLAUDE::PAIR-REVIEW-2026-06-11
1 participant
The aggressive counterpart to #1789's triage-only monitor — built opt-in so it's safe. Closes item ⑤.
What
On a trusted-bot (
coderabbitai/chatgpt-codex-connector) review of a PR carrying the human-appliedpr-trim:autolabel, runs a conservative pr-trim pass: applies clearly-actionable fixes, pushes to the PR branch, replies + resolves the addressed threads, and posts a summary of what it fixed vs deliberately skipped.Safety (why "aggressive" is OK here)
if:requirespr-trim:autoon the PR. It never fires unsolicited; remove the label to disable per-PR. (Label created.)contents: writeis scoped to this opt-in workflow only — the defaultreview-comment-monitor.ymlstayscontents: read.allowed_botsset (per the ci: triage-only Review Comment Monitor for bot reviews (auto-monitor) #1789 lesson — the action default-denies bot triggers).Relationship
review-comment-monitor.yml(ci: triage-only Review Comment Monitor for bot reviews (auto-monitor) #1789): always-on, triage-only (read, summarize P0/P1).review-autofix.yml(this): opt-in, applies fixes (label-gated).Verification caveat (honest): like the monitor, this can't be end-to-end-confirmed until a bot reviews a labelled post-merge PR. YAML/gate/permissions validated; the label gate bounds blast radius to PRs a human explicitly opted in.
🤖 Generated with Claude Code