When should an agent be allowed to fix code?

A reviewer identifies a bug and suggests a patch. Letting an agent apply it seems like an obvious next step.

Sometimes it is. But changing a branch crosses a different authority boundary from leaving a comment. The workflow now needs to justify which files it may change, what it may execute, and how it will establish that the resulting commit is the one it intended to produce.

The reviewer repository contains fixer primitives. The documented webhook entrypoint remains reviewer-only, and the fixer is disabled by default. This post examines those building blocks and the additional policy I would want before enabling automatic edits. It is not an announcement that unattended repair is deployed.

A finding is input, not authorization

The fixer prompt scopes work to listed blocking findings. It tells the agent not to broaden the change, refactor unrelated code, alter secrets or sensitive behavior, perform destructive data changes, or merge the PR. If a safe patch is unclear, the agent should stop.

Those are useful instructions, but a reviewer calling something “blocking” does not make the proposed fix safe.

I would start with changes whose behavior is unambiguous and whose impact is narrow: a focused correction to a reproducible defect, accompanied by a regression test. “Improve error handling” is too broad. “Change this branch so this known input returns the specified error, and preserve existing behavior for other inputs” is a better candidate.

Authentication, payments, migrations, destructive operations, and disputed requirements should need explicit human judgment. A small diff can still make a consequential change.

The policy also needs enforcement outside the prompt. Before committing, compare the actual changed paths and patch with the permitted scope. An agent’s statement that it only fixed the listed issue is not a substitute for inspecting what it changed.

An isolated worktree is not a sandbox

The checkout helper creates a detached Git worktree at the reviewed head commit. That separates the candidate patch from unrelated edits in another checkout.

It does not isolate processes, network access, credentials, or the host filesystem. Tests can execute arbitrary repository code, including code changed by the PR. Running them with privileged credentials can turn a verification step into an access problem.

For an enabled fixer, I would separate editing and test execution from the credentialed publication step. The execution environment should have limited access, and a narrow publishing component should receive only the verified candidate.

The current scaffolding passes a prompt to a supplied model runner; the surrounding integration must ensure that runner actually edits the intended worktree. Merely creating a directory is not proof that the agent used it.

Verify the patch, not its JSON claim

The fixer can return PATCH_APPLIED, but the controller does not use a model-supplied commit SHA as its final evidence. It runs configured verification commands and, when they pass, commits the worktree changes and obtains the resulting SHA through Git.

That is a useful separation between a claim and an observed result. Verification failures change the outcome to human attention rather than proceeding to commit and push in that branch of the helper.

Passing commands still establish only what those commands check. A test suite can miss a regression, and a patch can weaken its own tests. A safe publication gate should inspect the diff, including test changes, and use trusted verification configuration rather than blindly execute commands suggested by the agent.

The current commit helper stages all worktree changes with git add -A. That makes an enforced scope check especially important: otherwise unrelated files can enter the same commit even when the prompt said not to touch them.

Keep branch identity and budgets explicit

The implementation rejects fork PRs for automatic push and validates branch-reference syntax. It also keeps cumulative fixer budgets at the PR level, so moving to a new head does not automatically reset the recorded allowance.

Those checks reduce ambiguity. They do not establish that every concurrent execution respects the same budget atomically. The inspected loop checks counters and records attempts later; a rollout still needs a concurrency policy for overlapping attempts.

A branch can also move while the fixer works. The push helper uses an ordinary push rather than a force push, which normally rejects a non-fast-forward update. That is helpful, but it is not an explicit check that the branch still equals the revision originally reviewed.

I would recheck the expected head and use a publication mechanism that enforces that precondition, then read back the remote revision. If the branch changed, stop and reassess instead of silently expanding the fix to new code.

Fixing and merging stay separate

For this post, I ran the repository’s fixer-related tests:

python3 -m pytest tests/test_review_orchestrator.py -q -k 'fixer'

The result was 10 passed, 52 deselected. These tests use injected runners and temporary state; this was not an enabled production repair or a real push to a PR. They cover useful behavior such as default disablement, target restrictions, and budgets, without proving the whole editing environment safe.

My preferred first rollout would produce a bounded, reviewable candidate patch, with humans authorizing publication. Automatic branch updates can come later for a narrow class of changes with enforced scope, isolated execution, and verified revision identity.

Even then, a passing test is not permission to merge. The human reviewing the PR still owns the decision about whether this change should become part of the system.