Skip to content

Make pr-reviewer agent iterate to an ideal merge candidate #706

@aram356

Description

@aram356

Description

Today the pr-reviewer agent (.claude/agents/pr-reviewer.md) does one read-only pass: gather context, read changed files, classify findings, present them to the user, and submit a GitHub review with inline comments. The findings live as text on the PR; the author then has to interpret and implement them.

This issue tracks teaching the agent to iterate to an ideal merge candidate:

  1. Each review pass starts by re-resolving the PR's current head so the diff being reviewed is the diff that would actually merge. PRs move between passes (force-pushes, rebases, base merges); a review against a stale head wastes everyone's time.
  2. The user triages each finding twice: include it in the GitHub review, and implement it as code in a stacked fix-up PR. Findings with a concrete actionable fix (🔧 ♻️ ⛏ 🏕) are eligible; questions, thinking-aloud, seedlings, out-of-scope, and praise stay comment-only.
  3. The agent opens one fix-up PR per review engagement, stacked on top of the PR under review and reused across passes:
    • branch: review/<timestamp>-<pr-number> (UTC YYYYMMDD-HHMMSS, fixed at engagement start)
    • title: <timestamp> Review fixes for #<pr-number>
    • base: the PR's head ref (not main)
    • later passes rebase the branch onto the PR's current head and force-push-with-lease; new commits are appended, the PR body's "Findings addressed" list grows
  4. The GitHub review references the fix-up PR in the body summary and in each inline comment for findings that were implemented; non-implemented findings keep the existing suggestion format.
  5. The verdict logic is updated so 🔧 findings that were addressed in the fix-up PR no longer force REQUEST_CHANGES.
  6. After each pass the agent loops back to step 1 and re-reviews the combined (PR + fix-up) state. It stops when:
    • a full pass surfaces no new actionable findings (ideal merge candidate), or
    • the only remaining items are blocked on the author (open ❓, decisions, required rebase), or
    • the user says so.
  7. Hard rules forbid pushing fix-up commits or submitting reviews without explicit user approval of which findings to implement, force-pushing to main, or letting a fix-up PR target main instead of the PR's head branch.

Motivation

End state we want: a stacked fix-up PR that, once merged into the PR under review, leaves nothing further a reviewer would need to change. Today the author bears the full burden of translating review text into commits, and a second review pass against a moved head is manual. With the iterative workflow the agent shortens the review cycle from "review → discuss → author implements → re-review" to "review → implement → re-review until clean".

This was exercised end-to-end on #621 across two passes — a stale-PR situation, several upstream fixes between passes, and a final stacked fix-up PR — and the workflow held up.

Acceptance criteria

  • .claude/agents/pr-reviewer.md documents the iterative workflow as the agent's default behavior, including the goal statement up front, the review/<timestamp>-<pr#> / <timestamp> Review fixes for #<pr#> naming, the re-fetch-head step, the per-pass loop, and the stop conditions.
  • The verdict matrix accounts for findings fixed in the stacked PR.
  • The rules section forbids skipping user approval, force-pushing without --force-with-lease after a rebase, and targeting main from a fix-up PR.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions