Posted in Software Development tagged AI

Observations From Building a PR Review Loop

Building my own agent has been fun. Sure, I could have opted for an off-the-shelf solution, but I wanted control. The ironic part is that I'm using that control to offload more control to the agent.

Recent experiments have included letting the agent work on itself. Super meta, but didn't quite go as planned considering the guardrails I have in place around the size of PRs. Agent work tends to be heavy lifting, whereas most project work is small and fits nicely inside the constraints.

Part of the issue there is that the PR reviewer would constantly request changes on those larger PRs. Changes would happen, PR message body would diverge. Infinite loop death spiral until eventually I was called.

Around this same time, I was also tracking some issues that started to creep in after I changed some models from Opus 4.8 to Opus 5. Approval rate dropped by about half.

I like to think that each new model is going to be better than the last, so I thought perhaps it was time to revisit the reviewer to see if there was something we were missing.

The review mechanic

By design, the review persona is pretty dumb. Not dumb in the sense that I give it a lesser model or anything. Dumb in that it just doesn't have the full picture of things.

It gets the PR diff as a blob of text and that's it. No git worktree, no way to read files or grep around. No commit messages or recent changes to compare against.

I don't review PRs this way. I click around and look at adjacent code. I'll git checkout the branch locally and poke around. I try to gather as much as I can to give feedback with more substance than nit: stray line break or the like.

Because of this, a lot of the feedback from the reviewer was effectively LGTM, but I can't really confirm anything so... lol good luck. Then I'd get pinged to take a closer look.

Safety first

It's probably not fair to say the reviewer is dumb. The reviewer is gated by security checks. I'm the only user on my personal Forgejo instance, but that doesn't mean I'd prefer to be flighty with things.

Any changes made and the PR content can be considered untrusted input. Because of this, the reviewer shouldn't be poking around with tools on the command-line. The diff-only review blindness is a security measure.

Raising everything the agent couldn't rule out, that's a feature, not a bug.

Review, rework, repeat

At this point, the dutiful worker agent grabs the feedback with the goal of addressing the requested changes. The worker can verify the feedback against the working tree and attempt to remediate it.

And if the feedback was already implemented or worse, just plain wrong? The worker would either do its very best to remedy something, or it would bail on the task. Work typically resulted in PR body divergence, which is an underlying bug since the reviewer takes those messages very seriously.

If there's nothing to action on, I get the escalation. I also get pinged if the loop hits a certain number of iterations. The assumption being they aren't going to figure it out on their own.

Giving the worker permission

Accounting for the security concerns above, the fix wasn't to give the reviewer more access. Fun fact, I tried but my own security gates did their job and were like "lolnope".

The remedy was to give the worker permission to push back. Giving explicit permission is always one of my favorite prompt changes. It always feels like a cheat code.

This one wasn't even as much about giving permission as explaining the situation a bit better. "Hey, the robot that reviewed this only looked at the diff," and outlining a specific set of options to dismiss, fix or escalate.

To spell out things further, I explained that it could mix and match things as well. Since the reviewer may mention a few things, no reason to be committed to a single outcome for everything.

The most permissive part looks something like this:

Dismissing is a real option, not a loophole. But the burden is on you: "I could not confirm the reviewer's concern" is not a dismissal, it is a shrug. Go check, then either fix it or state what you found. A dismissal a human reads and disagrees with costs more than the fix would have.

Also worth mentioning, Forgejo v16 (released in July 2026) has expanded the PR review system greatly amongst other things. Before this, the only way to review was to leave feedback in a single input. Now they have inline commenting which will greatly improve how my agent reviews code.

The end of the no-op

With the worker in a state of always communicating, the reviewer has a lot more context to go on. The initial failure loop was a no-op that would turn into an escalation.

The new working model allows for additional context to be added for the reviewer. Said information could be pushed back on by the reviewer, but seems like most of the time it allows the agents to actually work through things.

At this point the game loop, still on Opus 5, is more likely to play out to an approval from the reviewer. Many of my repos will auto-merge in this scenario, allowing for more autonomy and less frequent phoning home.

:wq


Like this drivel? There's a whole RSS feed of it, or subscribe via email.