DEV Community

Cover image for Every PR gets an agent review before a human sees it
Aviv Laufer
Aviv Laufer

Posted on Originally published at golemnotes.substack.com on

Every PR gets an agent review before a human sees it

#ai #codereview #devtools #productivity

Part 4 of My Agentic Setup. Start from part 1

About 50 PRs a week come to me for review. I don't open any of them cold.

Each PR gets read first by an agent, in its own pane. The agent can read the PR and nothing else: it can't comment, approve, push or merge. It writes a report on my machine, and I read the report before I open the diff.

I start all of it with one command:

fmreview https://github.com/acme/api/pull/123 acme/web:45 'acme/api#130'
Enter fullscreen mode Exit fullscreen mode

The idea: every PR gets a review from a model on a different vendor than the one that wrote the code, the reviewer is read-only, and its report comes to me before anything is posted on GitHub.

fmreview takes a list of PRs and hands them to the first mate. The first mate starts one read-only scout per PR, each in its own pane, on the reviewer model. Each scout writes report.md on my machine. The first mate relays the verdicts to me and stops. Only I post anything on GitHub.

What broke

The reviewer model didn't reach the reviewers

My main agent, OMP, has a reviewer role pinned to a model on another vendor. But the pin applies only to a subagent whose type is literally reviewer.

My review agents are Firstmate scouts, and a scout is not a reviewer. Neither are the subagents a scout starts on its own. None of them get the reviewer model by themselves.

The fix is to pin the model by hand, every time. The note in my captain's preferences:

- Dispatch every code-review task on `openai-codex/gpt-6.1-sol` at high effort,
  not the default model (same as omp's `task.agentModelOverrides.reviewer` and
  `FMREVIEW_MODEL`). When spawning the scout, pass
  `--model openai-codex/gpt-6.1-sol --effort high` explicitly (the omp override
  only auto-applies to a subagent literally typed `reviewer`, not to a
  `scout`-typed spawn or its own internal fan-out subagents)
Enter fullscreen mode Exit fullscreen mode

And fmreview puts the model into the message it sends, so the first mate doesn't have to remember:

# Keep FMREVIEW_MODEL/EFFORT in sync with
# task.agentModelOverrides.reviewer in ~/.omp/agent/config.yml.
: ${FMREVIEW_HARNESS:="omp"}
: ${FMREVIEW_MODEL:="openai-codex/gpt-6.1-sol"}
: ${FMREVIEW_EFFORT:="high"}
Enter fullscreen mode Exit fullscreen mode

There is also an option to start omp in the reviewer role from the command line, if your omp build supports it. I leave it empty. Pinning the model is the path I verified.

# is a glob in my shell

The short way to name a PR is acme/api#123. In zsh with extendedglob on, # is a glob operator, so that word has to be quoted, or zsh tries to match files with it.

So fmreview also takes a form that needs no quotes:

case "$t" in
  https://github.com/*/pull/*)
    t="${t#https://github.com/}"; printf '%s#%s\n' "${t%%/pull/*}" "${t##*/pull/}" ;;
  */*\#[0-9]*) printf '%s\n' "$t" ;;
  # owner/repo:123 needs no quoting under extendedglob, unlike the # form.
  */*:[0-9]*) printf '%s#%s\n' "${t%%:*}" "${t##*:}" ;;
  [0-9]*) [ -n "$dr" ] && printf '%s#%s\n' "$dr" "$t"; return 0 ;;
  *) return 1 ;;
esac
Enter fullscreen mode Exit fullscreen mode

A bare number works too, with --repo acme/api.

herdr's split ratio is the other way around

Before fmreview I had a simpler batch launcher: one pane per PR, each running a review. The first version was for cmux, which takes a whole pane layout as JSON in one call. When I moved to herdr, there was no such call. Panes are made one herdr pane split at a time.

To get equal panes, you split off one slice at a time. herdr's --ratio is the share the original pane keeps, not the share the new pane gets. Splitting a 20-row pane with --ratio 0.75 leaves it 15 rows and gives 5 away. So to carve n equal slices, the ratio at each step is 1/n:

split_off() {
  herdr pane split "$1" --direction "$2" \
    --ratio "$(awk "BEGIN { printf \"%.6f\", 1 / $3 }")" --no-focus |
    json_get result.pane.pane_id
}
Enter fullscreen mode Exit fullscreen mode

The panes are laid out as a grid, rows first, so the PRs read left to right and top to bottom, and the panes stay close to square. Five PRs make three panes on top and two below.

A running server that looked down

The launcher checks that a herdr server is running. The obvious check is:

herdr status server | grep -q "status: running"
Enter fullscreen mode Exit fullscreen mode

With set -o pipefail, this fails even when the server is up. grep -q exits on the first match and closes the pipe. herdr is still writing, gets SIGPIPE, and exits with an error. pipefail reports that error as the result of the whole pipeline.

The fix is to capture first and match after:

server_status="$(herdr status server 2>/dev/null || true)"
case "$server_status" in
*"status: running"*) ;;
*) echo "Error: no herdr server is running. Start one with: herdr" >&2; exit 1 ;;
esac
Enter fullscreen mode Exit fullscreen mode

The setup

One PR: review-pr

The smallest piece is review-pr. It takes a PR URL, goes to my clone of that repo, and starts Cursor's agent CLI with the /review-pr command:

if [[ "$PR_URL" =~ github\.com/([^/]+)/([^/]+)/pull/[0-9]+ ]]; then
    ORG="${BASH_REMATCH[1]}"
    REPO="${BASH_REMATCH[2]}"
fi

REPO_DIR="$HOME/GitHub/$ORG/$REPO"
cd "$REPO_DIR"
exec "$(dirname "$0")/wt-agent" agent "/review-pr $PR_URL"
Enter fullscreen mode Exit fullscreen mode

It needs the repo cloned at ~/GitHub/<org>/<repo>, and it stops if the folder isn't there. /review-pr is the command from the pr-review-toolkit plugin. wt-agent sets a marker in wt list while the agent runs, so I can see which worktrees have an agent in them.

The review stays in the pane. Nothing is posted.

Many PRs: one scout each

For a batch, I use fmreview. It does four things:

  1. Normalizes each PR to owner/repo#number.
  2. Creates a Firstmate home called reviews, the same way fm does, if it doesn't exist.
  3. Writes a review charter into that home.
  4. Opens the home in herdr, waits for omp to start, and sends the first mate this message: 
ahoy

Dispatch one SCOUT per pull request below. One scout each, in parallel, each in its own pane.

Pull requests:
- acme/api PR #123
- acme/web PR #45

Profile for every one of these scouts: harness omp, model openai-codex/gpt-6.1-sol, effort high.

Register each repo as local-only if it is not registered yet.

Every brief must carry the full contents of data/review-charter.md in its Firstmate spec.
The charter is binding: these scouts are read-only and must not comment, review, approve,
request changes, push, or merge anything on GitHub.

When a scout finishes, relay its verdict and findings to me and then STOP. Do not act on
any recommendation, do not open follow-up tasks, and do not tear the scout down until I say so.
Enter fullscreen mode Exit fullscreen mode

local-only means the first mate never opens a PR for that repo. A scout writes a report, never code.

fmreview --print shows the message without sending it.

The charter

Every scout gets the same charter, word for word, in its brief. The first part is about what it may not do:

## Read-only, without exception
- NEVER post a comment, review, approval, change request, or reaction on GitHub.
- NEVER push, close, reopen, label, assign, or merge anything.
- NEVER edit files in the project clone or the worktree.
- `gh` is for READING the PR only: diff, files, checks, linked issues.
- The single deliverable is `data/<id>/report.md`. Nothing leaves the machine.
- If the review implies an action, write the recommendation in the report and
  stop. The captain decides every next step.
Enter fullscreen mode Exit fullscreen mode

The second part is what the report has:

- Verdict in one line, then findings ordered by severity.
- Every finding cites `path/to/file:line` from the PR's own diff.
- Separate what the diff does from what it fails to do.
- Call out failing or missing CI checks and say what they imply.
- Flag anything touching auth, tenancy boundaries, data retention, migration
  ordering, or IAM as captain-escalation regardless of test status.
- End with a short list of open questions for the author.
Enter fullscreen mode Exit fullscreen mode

The third part says which review tools win when they disagree. First, the repo's own review rules, when a repo has them. A repo with compliance rules, for example, gets those checked before anything generic. Then pr-review-toolkit for the mechanics: correctness, edge cases, error handling, tests for the changed lines. Then a second mechanical pass, only on what the first two didn't cover.

Each finding goes in the report once, under the tool with the highest precedence that found it. The report also says which tools were there and which weren't. If none loaded, it says so and reviews without them, instead of guessing what their checklists say.

Five passes, one of them for the spec

Inside each review there are five review passes, all on the reviewer model. Four look at the code. The fifth is a spec reviewer, and it gets less context on purpose: only the diff, the commit list, and the linked ticket. When there's no ticket, it gets the intent from the brief.

It returns three lists, and every item quotes its source line:

  • asked but not built
  • built but not asked for
  • built wrong

The other four passes read the code the way the author meant it. This one checks the code against what was asked.

The reviewer is on a different vendor

The model that writes most of my code is Claude. The reviewer is a GPT model, so the reviewer is never the same model that wrote the code.

The profiles in my dotfiles set this per machine:

# Agent pins (task.agentModelOverrides). The pr-review-toolkit agents are the
# cheap first pass; reviewer and security-reviewer are the real gate.
reviewer = "openai-codex/gpt-6-sol:high"
security-reviewer = "anthropic/claude-sonnet-5-5:high"
code-reviewer = "@crew-light"
silent-failure-hunter = "@crew-light"
pr-test-analyzer = "@crew-light"
comment-analyzer = "@crew-light"
Enter fullscreen mode Exit fullscreen mode

@crew-light points at another role, a small model. The pr-review-toolkit agents run often, so they get the cheap model. The security reviewer is Claude, so only the main reviewer is on the other vendor.

What runs today:

Role What it does Work laptop Personal laptop
default, crew writes the code Claude Sonnet 5.5 Claude Sonnet 5.5 via Cursor
reviewer the real gate, fmreview scouts GPT-6.1 Sol, high GPT-5.6 Terra via Cursor, high
security-reviewer security pass Claude Sonnet 5.5, high Claude Sonnet 5.5 via Cursor, high
pr-review-toolkit agents cheap first pass Claude Haiku 5.5 Claude Haiku 5.5 via Cursor

When my own agents open the PR

The same review runs on the other side, when my agents write the code. These rules are in the instructions every agent gets (trimmed):

- Before opening: run the review skill (the repo's own, or
  `pr-review-toolkit:review-pr` otherwise) and `simplify`, and address the findings.
- Always draft (`gh pr create --draft`).
- Promote to ready only on the captain's explicit instruction for that PR. Green
  CI or self-review never authorizes promoting or merging.
  Humans approve and merge (squash); do not merge unless asked.
Enter fullscreen mode Exit fullscreen mode

So an agent's PR has been reviewed by an agent before it exists, and a human still decides when it's ready and when it merges.

A budget for review rounds

The newest change is a limit on how many times a PR goes back and forth with a reviewer. Until late October, nothing in my setup said when to stop. Now my captain's preferences have this:

### Review budget
- Default to 3 review rounds per PR. Round 1 is a full review. Later rounds
  check only the changed code and the findings that are still open.
- Extend to 5 rounds only when the change touches security, permissions, auth,
  data loss, or state and concurrency, and a blocker is still open with
  evidence. Also extend when the captain asks.
- Stop early when a round returns only nits.
- Keep a ledger of every finding: id, severity, and outcome (fixed, rejected
  with reason, or open). Never guess usage or token numbers. Report only what
  the tools show.
- Make the smallest change that satisfies a reviewer. Do not widen the scope
  to fix things nobody flagged.
Enter fullscreen mode Exit fullscreen mode

Round 1 is a full review. Rounds 2 and 3 check only the changed code and the open findings. A round with only nits stops early. Rounds 4 and 5 happen only when a blocker is still open with evidence on security, permissions, auth, data loss, or state and concurrency, or when I ask. Every finding goes into a ledger as fixed, rejected with a reason, or open.

With the ledger, every finding ends with an outcome, and "rejected" needs a reason. A finding can't just disappear.

Try it

The companion folder has sanitized copies of review-pr and the herdr batch launcher, the PR parser, a script that prints the fmreview message, and the charter. The tests use a fake agent CLI and a fake herdr that keeps the size and place of every pane, so they check the real grid for 1 to 7 PRs, not just the calls. They also check URL quoting, PR parsing, and the read-only lines in the message.

cd 04-pr-review
make test
Enter fullscreen mode Exit fullscreen mode

golem-notes-examples/04-pr-review

Who reviews your agents' code, and does the reviewer come from the same vendor as the author? Leave a comment.

This is part 4 of my series on running a real agentic dev setup. I publish each part first on Golem Notes, with the full write-up and what's coming next. Subscribe there to get the next one in your inbox: Golem Notes

Top comments (0)