feat: Review every PR a session opens with two opus agents before it merges

[ecv]

Summary

A Claude Code session that writes a change is the worst judge of it, and today nothing reviews the pull request it opens until a human finds time.

Running two read-only opus reviewers with deliberately different angles on every new PR caught five real defects in one week, including one that had already merged and taken staging cert-manager down.

This proposes shipping that habit as two reviewer agents, a fixer agent, and a skill that drives them, so every session on the team runs the review without anyone remembering to ask.

The problem

The review happens by hand, one session at a time, only when someone thinks of it. When it does not happen, the PR merges on a green CI run that never checked the thing that broke.

Five cases from a single week:

• https://github.com/datum-cloud/infra/pull/4634 merged green and broke staging cert-manager. A pre-review had flagged the namespace ownership conflict as something to confirm, and nobody acted on the flag.
• https://github.com/datum-cloud/infra/pull/4708 shipped an owner-resolution change to production with a pruning gap the reviewers found before merge.
• https://github.com/datum-cloud/infra/pull/4709 pointed alert fallbacks at a check that the change itself switched off.
• https://github.com/datum-cloud/infra/pull/4710 routed staging alerts to a team route that does not exist.
• https://github.com/datum-cloud/infra/pull/4702 rendered the edge fleet from the wrong set of files, so the check passed against manifests the cluster never sees.

Four of the five were caught before merge, by two reviewers who disagreed with the PR body rather than with each other. The one that was not caught is the reason for the rule.

The second half of the rule matters as much. When both reviewers land on the same verdict with compatible fixes, waiting for a human to say "yes, do that" costs a round trip and buys nothing. The session should fix, push, and set the PR up to merge on its own, and stop only where a person genuinely has to choose.

What the two reviewers do

Two angles, run at the same time, on the same PR, with no knowledge of each other.

Adversarial correctness. Reproduce the failure the PR claims to fix. Run the queries, builds, renders, and gates the body cites, and check the output says what the body says it says. Attack every claim in the description. Ask what would have to be true for this change to be wrong, then go look.

Conventions and blast radius. Where does this land: staging only, or a shared base that reaches production and the edge sites on the next reconcile? Does the Flux render actually produce what the diff suggests? Do the patch targets anchor to real object names? Does it collide with another open PR touching the same artifacts? Does the commit message and the body meet the pr-conventions bar?

Both are read-only. Neither posts anything to GitHub. Both return the same shape, one line per finding, no praise, and a verdict:

<path>:<line>: <severity>: <problem>. <fix>.
VERDICT: merge|hold. <the single most important reason>

Severity is one of blocker, warning, nit, decision. The decision tag is what routes a finding to a human rather than to the fixer.

Design

Four mechanisms are available in a plugin, and they enforce different amounts.

Mechanism	Enforces	Cannot do
Agent definition	The model pin, the tool allowlist, and the read-only posture of the reviewer, every time it runs	Make itself run. An agent exists only when the main session decides to launch it
Skill	Nothing. It is instructions the model reads and follows	Guarantee it loads, or that the session obeys it once loaded
Command hook	Fires on the tool event whether the model wants it to or not. Can block a call, or inject text into the session with additionalContext, which PostToolUse supports	Spawn an agent. It is a shell command, so the most it can do is return context telling the session to launch one
Agent hook	Spawns a subagent on the event, with no session cooperation at all	Nothing relevant, except that it is marked experimental and is the newest surface here
Command	An explicit, repeatable entry point the user types	Fire on its own
Plugin default settings	Sets a default main-thread agent for every session	Carry a general instruction. It accepts only the agent and subagentStatusLine keys

No single one of these is enough, so the design uses three together, each doing the part it can actually do.

Primary: two agents plus a skill. The agents carry the identity, the opus pin, the read-only tool allowlist, and the output contract in their prompts, so a reviewer is the same reviewer in every repo and every session. The skill carries the protocol: launch both on any PR this session opens, wait for both, compare the verdicts, and run the fix path when they agree. The agents are the enforceable half, the skill is the advisory half, and this split is already how code-reviewer and /review work in the plugin today.

Backstop: a PostToolUse command hook on gh pr create. The skill only fires when the session recognises it should. The hook fires because a PR was created, which is the actual trigger. A command hook cannot launch the agents itself, so it returns additionalContext naming the PR that was just opened and telling the session to run the review. The PR number comes free from the gh output the hook receives. This is the same shape as the existing pr-op-gate, which self-filters inside the script on a broad Bash matcher.

The fix step: a third agent. Applying findings is a write operation on a branch another session may be sharing, so it gets its own definition with the worktree rule, the signing rule, and the body rewrite rules built in, rather than being improvised by whoever spawns it. A subagent can spawn subagents, so a PR opened by a fan-out writer can start its own review without coming back to the main session first.

The fixer does not use isolation: worktree. That setting cuts a fresh worktree from the session's current HEAD, not from the PR branch, and checking the PR branch out there fails whenever the worktree that opened the PR still has it checked out. The fixer finds the branch's existing worktree with git worktree list and works there, creating one only when none exists.

Read-only has to be enforced, not declared. Scoped entries such as Bash(git diff *) in a subagent tool list do not scope anything. A probe of the shipped code-reviewer, which carries that shape, reported plain Read, Bash, dropped Grep and Glob, and ran unrelated commands freely. The docs show that syntax for skill and command allowlists only. So every reviewer built this way holds unrestricted Bash, and the tool list is documentation of intent, nothing more.

The reviewers therefore declare the documented form, Read, Grep, Glob, Bash, and the guarantee moves entirely to a PreToolUse hook. It refuses the GitHub write surface (gh pr and gh issue mutations, releases, workflow runs), git mutations (push, commit, tag, checkout), network clients (curl, wget, inline interpreters), gh api with a non-GET method or a request body, and shell wrappers (eval, sh -c, xargs, aliases, variable indirection) for the reviewer agents only. It is a backstop against an accidental write, not a control against a determined agent. The reviewer prompt carries the rule and the branch ruleset's human approval is the last gate.

That hook cannot live in the agent frontmatter. The subagent docs say plugin subagents ignore the hooks, mcpServers, and permissionMode frontmatter fields, so a plugin agent carrying them ships dead config, and the existing code-reviewer already does. The hook registers in the plugin's own hook file on a broad Bash matcher and self-filters on the agent_type field the hook input carries inside a subagent, firing only for the two reviewers. That is the same shape as pr-op-gate and it ships in 1.13.0, since it fires for nobody else.

Fallbacks if the primary is rejected. Replace the command hook with an agent hook, which spawns the reviewers on the event with no session cooperation at all and would make the rule genuinely unskippable. That is the strongest version of this and the least proven, since the feature is experimental, so it is the natural second version rather than the first. Failing that, fold the protocol into pr-conventions instead of a new skill, which costs a reader nothing to load but buries a workflow inside a style document. Or ship only the /pr-review command and drop the automatic trigger, which is honest but reverts to remembering.

The autonomy boundary

The session acts without asking only when the two reviews agree cleanly. All five conditions have to hold:

1. Both reviewers finished. A timeout or a truncated report is not an opinion.
2. Same verdict. Both merge, or both hold.
3. No finding tagged decision from either reviewer.
4. The fixes are compatible. Neither reviewer asks to remove what the other asks to add, and neither wants a file the other wants left alone.
5. Neither reviewer questions the premise of the PR, meaning it should be closed, split, or rewritten to a different goal.

When all five hold, spawn the fixer and say in one line what it is doing. When any fails, put the choice to the human with a recommendation, then act on the answer.

Always escalate, whatever the verdicts say:

• A finding about ownership: which team or repo owns the thing being changed.
• A naming choice that outlives the PR.
• Whether a change that reaches a shared base ships now or waits behind a staging soak.
• Splitting a PR, closing it, or reopening a question the issue already settled.
• A reviewer that could not run the check it needed, so hedged. A hedge is not agreement.

The fix and ready path, once the boundary is clear:

1. Fixer applies the findings in the worktree that already holds the PR branch, never in a shared checkout.
2. Commits signed, one commit per coherent change, message wrapped at 80 and shaped by commit-conventions. Never pass a flag that skips signing.
3. Rewrites the body if a reviewer said to, which sends it back through pr-op-gate on the way out.
4. Pushes and waits for CI green on the new head.
5. The session runs both reviewers once more on the new head. Two merge verdicts, or two hold with nothing left but nit, continue. A second hold goes to the human. One re-review, never a loop.
6. gh pr ready, which is what makes GitHub request the CODEOWNERS teams.
7. Requests the configured human reviewer, if one is configured.
8. Enables auto-merge with the merge method the repo allows, falling back to a merge commit when no ruleset names one.
9. Posts one comment on the PR recording both verdicts and what each reviewer ran, so the human approver sees what was already checked.

Auto-merge cannot fire unaided on infra. The main branch ruleset requires one approving review, a code-owner review, and approval of the last push, and it dismisses stale reviews on every push. So the fixer's push always lands in front of a human, and the comment in step 9 is what that human reads first. A repository without that ruleset does not get the same guard. Auto-merge still gets enabled there, and the comment says in one line that no human approval stands between the push and the merge.

How the reviewer is configured. Team review requests already happen without help. A repository carrying a CODEOWNERS file gets its owning teams requested the moment the PR goes ready, and the infra repository maps every directory to a team that way.

A named human is different, and the skill must never guess a handle. Read it from the repository's own Claude settings under an agreed key, treat its absence as "request nobody beyond the code owners", and never invent a login. The merge method belongs next to it, because the org ruleset on the infra repository accepts merge commits only while the repository API cheerfully reports that squash is allowed.

Cost, and where to cap it

Measured over the sessions that produced the five findings above, each opus reviewer consumed roughly 100k to 160k tokens, and the pair plus the fixer added about ten to fifteen minutes of wall time per PR. Both reviewers run at once, so the wall time is one review, not two.

That is cheap next to one broken staging reconcile, and expensive to spend on a typo. Suggested caps, all of them arguable:

• Skip when nothing deployable changed. A diff confined to Markdown, CHANGELOG, or comments gets the conventions reviewer alone.
• Skip trivial diffs. Below a small threshold of changed lines across a single file, run one reviewer.
• Never skip on reach. Any diff that touches a shared base, a production overlay, or an alert rule gets both reviewers regardless of size. Blast radius, not diff size, is what decides.
• Run in the background. The session should keep working while the reviews run, and pick them up when they land.

Rollout

The staging-first analogue for a plugin is opt-in first, default second.

1.13.0 ships the agents, the skill, the command, and the read-only guard hook, with no automatic trigger hook. The skill fires when the session opens a PR or a user names one, and /pr-review runs it by hand. Nothing fires on gh pr create from outside the session's own reasoning yet. This is the soak.

1.14.0 adds the PostToolUse command hook, so the review fires on every gh pr create whether the session remembered or not, with an environment variable to switch it off for a session that has a reason.

A later version can swap that for an agent hook, which spawns the reviewers itself rather than asking the session to. That removes the last place the rule can be skipped, and it should wait until the command hook has run long enough to show the protocol is right.

Pickup is the normal path for this marketplace. Plugins track main, and a user on a stale cache refreshes with claude plugin update or by updating the marketplace. The version has to move in the plugin manifest, the marketplace catalogue, the README table, and the changelog together, which is the drift already recorded in issue #26. That drift is live today, with the marketplace catalogue still at 1.0.0 while the manifest says 1.12.0, so the plugin PR bumps both.

Collision check. No open PR or issue on this repository touches the reviewer agents or a review skill. Pull requests #19, #11, and #10 all edit the changelog, the README, and the plugin manifest, so whichever lands second rebases the version line. Pull request #10 also adds an agent, so it is the closest neighbour. Pull request #15 touches only the plan and api-dev agents and the platform-knowledge skill, so it does not collide.

Open questions

• Which model does the fixer get? Opus applies subtle findings more reliably. Sonnet costs a fraction. The draft says opus and invites the argument.
• Two reviewers or three? A third angle, security and secret handling, is the obvious candidate, at another full review's cost.
• Does this belong in every repository, or only the ones that deploy? A documentation repository gains little from an adversarial correctness pass.

Files for the plugin PR

plugins/datum-platform/agents/pr-adversary.md               new
plugins/datum-platform/agents/pr-conventions-reviewer.md    new
plugins/datum-platform/agents/pr-review-fixer.md            new
plugins/datum-platform/skills/pr-review-loop/SKILL.md       new
plugins/datum-platform/skills/pr-review-loop/protocol.md    new
plugins/datum-platform/commands/pr-review.md                new
plugins/datum-platform/hooks/deny-gh-api-write              new
plugins/datum-platform/hooks/pr-review-nudge                new, 1.14.0
plugins/datum-platform/hooks/hooks.json                     add PreToolUse deny entry now, PostToolUse nudge 1.14.0
plugins/datum-platform/.claude-plugin/plugin.json           version
.claude-plugin/marketplace.json                             version
README.md                                                   version table, feature list
CHANGELOG.md                                                entry

pr-adversary

---
name: pr-adversary
description: >
  Adversarial correctness review of an open pull request. Reproduces the
  failure the change claims to fix, runs the queries, renders, and gates the
  body cites, and attacks every claim the description makes. Read-only and
  posts nothing to GitHub. Launch it on every PR a session opens, at the same
  time as pr-conventions-reviewer.
tools: Read, Grep, Glob, Bash(git diff *), Bash(git log *), Bash(git show *), Bash(git fetch *), Bash(git worktree list), Bash(gh pr view:*), Bash(gh pr diff:*), Bash(gh pr checks:*), Bash(gh run view:*), Bash(gh api:*), Bash(kubectl get:*), Bash(kubectl describe:*), Bash(kustomize build:*), Bash(flux get:*), Bash(task validate-kustomizations)
disallowedTools: Write, Edit, NotebookEdit
model: opus
background: true
---

The hook reads the command from stdin and exits 2 when it sees gh api with -X, --method, -f, -F, --field, --raw-field, or --input. Everything else passes.

pr-conventions-reviewer

---
name: pr-conventions-reviewer
description: >
  Conventions and blast-radius review of an open pull request. Checks whether
  the change starts in staging, what it reaches once merged, how Flux renders
  it, whether patch targets anchor to real names, whether it collides with
  another open PR, and whether the commits and body meet the pr-conventions
  bar. Read-only and posts nothing to GitHub. Launch it on every PR a session
  opens, at the same time as pr-adversary.
tools: Read, Grep, Glob, Bash(git diff *), Bash(git log *), Bash(git show *), Bash(git fetch *), Bash(git worktree list), Bash(gh pr view:*), Bash(gh pr diff:*), Bash(gh pr list:*), Bash(gh api:*), Bash(kustomize build:*), Bash(task validate-kustomizations)
disallowedTools: Write, Edit, NotebookEdit
model: opus
background: true
---

background: true on both reviewers is what lets the session keep working while they run, so the ten to fifteen minutes below is not dead time.

pr-review-fixer

---
name: pr-review-fixer
description: >
  Applies agreed review findings to an open pull request. Works in the
  worktree that already holds the PR branch, commits signed, pushes, rewrites
  the body to the pr-conventions bar, waits for CI to go green, then marks the
  PR ready, requests the configured reviewer, enables auto-merge, and posts
  one comment recording both verdicts. Use only when two reviewers returned
  the same verdict with compatible fixes, never on a split verdict and never
  on a finding a person has to decide.
tools: Read, Write, Edit, Grep, Glob, Bash
model: opus
---

pr-review-loop skill

Triggers on: "review this PR", "double review", "two reviewers", "run the review agents", "I just opened a PR", "check this before it merges", and on the session opening any pull request itself, including one opened by a subagent.

The skill body carries the protocol: launch both reviewers at once with the PR number and the base SHA, wait for both, apply the five agreement conditions, and either spawn the fixer or put the choice to the human. It states the output contract once, so the agents and the comparison step read the same definition.

https://claude.ai/code/session_01FLHBSueNDbSoQvM45Jq3TN

[ecv]

Shipped as datum-platform 1.13.0 in #43: the three agents, the pr-review-loop skill, /pr-review, and the reviewer write guard. The 1.14.0 nudge hook continues in #45, the security reviewer and further angles in #42, and the tech-writer cleanup in #44.