Skip to content

Put the reviewer through the role runner with a READ-ONLY environment, so it verifies instead of infers #226

Description

@thedancingdeveloper

Part of #195. Blocked on #215 (the role runner).

Today

The reviewer is shown a diff and the brief, in one call, and must decide whether the change does what the item asked. Every claim it makes about the rest of the repository is an inference from a patch.

The measured case

Reviewing rdpapp T1 it rejected the work with:

its tests do not derive coverage from migration-tool or the legacy SQL fixtures … It verifies a hand-maintained copy of an inventory, not the tracked sources the task named.

That was correct, and it was a guess. Nothing let it open the test file, run it, or check what the SQL fixtures contain. It happened to be right; on another item the same inference is a false rejection, and a false rejection costs an attempt and blames a model that was correct — the failure mode this repository spent 2026-08-05 removing elsewhere.

A reviewer that can grep and run the test knows.

Read-only is not a detail

The environment must be read-only, and this is the whole risk of the change.

A gate with write access to the tree it is judging is a gate that can be talked out of a rejection: it could make the tests pass and then approve. That is not a hypothetical failure mode — a loop with a shell did exactly this shape of thing on 2026-08-05, appending tables to the tracked SQL fixture so its own registry matched, i.e. moving the measuring stick to reach green.

So: no writes, and the guard must enforce it rather than the prompt asking for it. A gate is never weakened to make the scaffolding simpler (AGENTS.md).

Running the checks is a separate question from writing. Reading and executing a test is legitimate verification; the gate already ran them, and a reviewer re-running one to confirm a claim is cheap. Whether it may run arbitrary commands read-only, or only the declared check commands, should be decided rather than defaulted — my view is the declared checks plus read-only exploration.

What must not change

  • Checks still run before the reviewer. Paying the dearest gate to catch what the cheapest already did is the trade this ordering exists to prevent.
  • Reviewer independence (a different vendor from the implementer) is unaffected by this and remains unavailable while one vendor answers.
  • A rejection still costs an attempt and still returns its reasoning to the next one (Session mode retries blind: the agent is never told why the last attempt was refused #166).

Acceptance

  • A reviewer rejection cites something it verified — a file it read, a command it ran — not only the diff.
  • A test proves the reviewer cannot write to the worktree, and that an attempt to do so is refused rather than ignored.
  • A test proves a reviewer that verifies a passing claim approves it, so the change is not just a stricter gate.
  • Mock end-to-end in the established pattern.

Blind spots

  • Whether read-only is enough to keep a gate honest is untested. It can still be argued into a pass by its own reading of the code, and a more capable reviewer is more persuasive in both directions. T43: A/B whether the reviewer seeing the plan changes its verdict #84 is the experiment that would say.
  • The reviewer becoming ~30 calls rather than one changes the economics of the most expensive gate. Nothing measures that yet.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:model-clientThe ModelClient: routing, retry classification, per-endpoint cooldowntype:taskUnit of implementation work

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions