Reviewing

The boundary the local reviewer runs inside. Back to the README.

Local reviewer boundary

The factory never gives a reviewer the implementation worktree directly. review_runner.py stages the frozen base and candidate commits into a fresh, detached, zero-remote Git repository and verifies its identity before and after the review. Docker mounts that repository at /workspace read-only. The container also has a read-only root filesystem, no Linux capabilities, no privilege escalation, bounded memory/CPU, a pids limit of 4096 tasks (PIDS_LIMIT; on Linux it counts threads as well as processes), and no Docker socket or host home.

Codex runs with danger-full-access inside this container because Ubuntu's AppArmor policy blocks its nested Bubblewrap sandbox when the previous maintainer's agent runs it as a service. The outer container is the enforcement boundary: an actual write probe under /workspace must fail before the model is called. Only a disposable copy of ~/.codex/auth.json and the installed Codex release binaries are mounted; the copy and all reviewer state are removed afterward. Outbound network remains enabled because Codex uses remote inference, but no GitHub, SSH, Linear, Docker, or unrelated host credentials are exposed.

Two directories matter inside the container. /workspace is the read-only staged checkout: the identity check, the write probe and the PREFLIGHT_OK line all run there, and nothing ever writes to it. Once preflight passes the script copies the whole tree (including .git, so refs/review/base and refs/review/candidate resolve) to /home/reviewer/candidate and runs Codex from that writable copy, so a package install or a go build beside the sources can succeed and the ticket's criteria can actually be witnessed. The copy is discarded with the reviewer home at the end of the round; the merge takes the host worktree's SHA, never the container's files.

What the reviewer can run is what that copy holds. The container has no network route to a package registry, so [worktree] setup is not re-run inside it; instead the stage carries read-only copies of the ignored install directories the task worktree already holds, those the project lists in [worktree] carry (see config.md), at the same paths. With console/node_modules carried, a console ticket's bun --cwd=console test runs in the container against the packages the candidate was verified with. A verify command that needs an install the worktree does not hold, or a directory the project does not list, cannot be run there and the reviewer should say so rather than report the gate unverified. When the ticket's verify commands and the project's baseline passed at the candidate, the reviewer's brief says so and tells it not to rerun the full suite, which runs as a pull request check, but to run only the focused tests a specific concern needs.

A ticket whose mock-up the factory fetched gives every reviewer two more read-only directories, whatever the route: .holophyte/mockup/, the approved snapshots and board sources, and .holophyte/captured/, the screenshots the [merge] ui_capture run took at the reviewed head (empty when that capture failed or made nothing). The container stage carries them after the [worktree] carry entries, a seat reviewer's scratch checkout gets copies with their write bits cleared, and a wrapper reviewer already runs in the task worktree that holds them. The review prompt lists each file and asks for a design-match check: a visible difference between the candidate's UI and the approved design is a finding unless the ticket text asks for it, and a board no screenshot shows is compared with its .dc.html source.

Every UI capture's copies in .holophyte/captured/ are listed for the reviewer beside the Evidence section, whether or not the ticket has a mock-up. The factory fetches each Evidence image under the media bucket's public_base before the review, as the user agent holophyte, and names any it cannot fetch as an infrastructure problem, not the candidate's. A state marked not captured, or a failed capture, is a finding; an image the reviewer cannot fetch is not: it reads the local copy, or tries curl, and otherwise writes could not fetch URL: REASON before its CRITERION lines.

How many review rounds a run gets is decided per run, before its first review, from the size of the candidate's diff and the [loop] review keys (review_rounds, review_rounds_per_lines, review_rounds_max; see config.md). The cap is printed and recorded in the run's ledger, and the terminal adjudication follows the last round it allows.

The first review builds holophyte-reviewer:ubuntu24.04-v13 automatically from the digest-pinned Ubuntu image; it carries git, python3 with its venv module, ripgrep, a pinned Bun (checksum-verified, on PATH under /opt/bun/bin) so console bun criteria can be witnessed inside the container, a pinned Node.js 24 (checksum-verified against the release's SHASUMS256.txt, node, npm and npx on PATH under /opt/node/bin), so a Node project's commands find the tools inside it (the image carries the tools, not the install: a review does not re-run [worktree] setup, so npm ci is not run there, as above, and an npx verify command runs against the carried node_modules), Chromium and its headless shell for Playwright 1.62.1 (installed with their system libraries by that version's own playwright install --with-deps chromium, world-readable under the directory PLAYWRIGHT_BROWSERS_PATH names, /opt/ms-playwright) so a project's own Playwright 1.62.1 launches a browser for a [merge] ui_capture run in the container, whose home is an empty tmpfs (a project that moves to another Playwright version needs a new image, as a new Bun does), and a pinned Go 1.26.9 (checksum-verified, under /usr/local/go, GOTOOLCHAIN=local so no other toolchain is ever downloaded, caches under the writable /home/reviewer, CGO_ENABLED=1 with gcc and the C library headers so go test -race runs) so a Go project's go test criteria can be witnessed too. It also installs tomlkit and mcp at the versions requirements.txt pins, so the factory's own suite runs inside the container on its dependencies, and a pinned Claude CLI (the native linux-x64 build, checksum-verified against its release manifest, on PATH under /opt/claude/bin, DISABLE_AUTOUPDATER=1) whose managed settings at /etc/claude-code/managed-settings.json make bypassPermissions the default permission mode, so a Claude implementer turn under implementer_isolation = "container" runs its tools without asking; the container is its boundary. Only an adversary pass on the Claude family (below) gives it a credential; in any other review the CLI is inert. A change to the Dockerfile moves the tag so the next review rebuilds instead of reusing the cached image. The image follows the candidate: the runner reads the tag and the Dockerfile out of the candidate commit, so a candidate that changed either is reviewed in an image built from the candidate's Dockerfile under the candidate's tag before its review starts, and a candidate that changed neither runs in the image main names. That build runs on the host, with network, and a build that fails is an infra failure of the run naming the candidate; pruning old tags is the operator's docker image prune. /tmp is a noexec tmpfs so the reviewer cannot run what a candidate drops there; because go test executes its test binaries from the temp directory, the image sets TMPDIR and GOTMPDIR to /home/reviewer/tmp, which the container script creates on the writable reviewer home before preflight. A run fails closed if preflight identity or write rejection fails, the Codex tool host cannot execute a local command, the container times out, or the staged repository fingerprint changes.

A Claude turn, the adversary's alternate family, runs in the same container: the same staging, preflight, write probe, cp -a copy, read-only workspace mount and hardening flags, with no Codex release and no Codex auth file mounted. From /home/reviewer/candidate it executes /opt/claude/bin/claude -p --model MODEL --effort EFFORT --output-format json PROMPT, model, effort and prompt passed as arguments to the script. Its credential is the variable [agents] adversary_credential names, handed over as --env=NAME. The reply is the result field of the CLI's JSON; output that is not JSON, or whose is_error is true, fails the turn with the output's tail, and an outage line (You've hit your limit, Credit balance is too low) goes through the outage path below.

A review prompt whose range from the frozen base holds commits from the trim step gets a trim section, read from git on every round, resumed runs included. It lists each trim: pass commit by short sha and subject and quotes every Trade-off: line of their bodies. It tells the reviewer that a trim commit must not change behavior, that a test a trim: tests commit deleted needs a Proof: line, and that a trade-off on a trust boundary, an auth check, a data-loss path or a money path is a blocker. A range with no pass commit, such as an implementer's trim whitespace commit, gets no section, and the prompt is unchanged.

Configured [agents] reviewer and adjudicator commands heartbeat the run while they execute, like the implementer, every half heartbeat stale interval.

Witness pre-check

Before the round 1 review, after its verify passes, the factory checks each acceptance criterion's named test file: it exists, the branch adds or changes it, the verify commands run a test from it, and its tests, and the tests the branch adds there, did not run only skipped. Each gap it finds goes back to the implementer as a witness turn, on the run's implement time box rather than as a review round; the run stays in verifying. A turn that commits is verified again and checked again on the new head.

The turns stop at two a run, counted from the run's witness_turn events, so a resumed run does not get two more. A turn that makes no commit ends them (witness_turn_idle), as does a verify that goes red after a turn; a turn that times out fails the run. A criterion that names no test file never starts a turn, and neither does a file whose runner the factory cannot parse; its gaps go to the reviewer only.

When gaps or criteria naming no test file remain, the round 1 review prompt lists them after the criteria, for the reviewer to judge; the list is not a finding. When a turn turned verify red, the list is the last check's, named by the commit it ran at; when verify was red before any check, it holds only the criteria naming no test file.

Adversarial pass

With [review] adversary = true (see Config) a second seat, adversary, tries to break the candidate while the primary reviewer checks it against the ticket. Its turn starts on its own thread just before the primary review turn, on the same base and candidate, and the round waits for both; neither brief holds the other's output. Its default route is the review container on the reviewer's model pair with Codex multi-agent on, so it can start subagents; the primary reviewer's container keeps --disable multi_agent. [agents] adversary and adversary_fallback route it like the reviewer.

With [agents] adversary_credential set, the default route alternates between two model families so they can be compared: a run's passes are numbered from 0 in the order they run, counted from its adversary_round events (a resumed run counts from the run it continues), and pass k of run R runs Claude Opus at effort high in the review container when R + k is odd, Codex otherwise. Without the key every pass runs Codex. A Claude turn whose output, on either stream, carries a Claude outage line switches to a configured adversary_fallback, or the reviewer's container fallback pair, probed and recorded as a route_fallback event; with neither, the run fails as review_route and no Codex turn stands in, even when the reply otherwise ends with ADVERSARY: DONE. When the adversary has a fallback (adversary_fallback, or the reviewer's container fallback pair while [review] adversary is on), startup probes the Claude route after the seat's own probe, and a failed Claude probe starts the adversary on its fallback.

The round's tier is the one its blast_radius event records. The first round of a run whose tier is medium or high gets a pass over the whole candidate, BASE..SHA (scope candidate); a low round gets none. After a pass, a later round runs it again only when its fix range, from the previous round's recorded sha to this round's, changes a path matching BASE_HIGH_PATHS or [review] high_paths. That pass is light and has scope fix: it attacks only the fix range and re-checks each concern earlier passes in the run recorded.

Depth	When	Attack subagents	Time box
full	high	one per surface the diff touches (browser UI, HTTP or API, CLI, data and migrations) plus one per risky module	1800 seconds
light	medium, and every fix pass	one per surface the diff touches, no per-module subagents	900 seconds

Both briefs cap the pass at five subagents, give mechanical checks to a light model and defect reasoning and reproduction to a strong one, and keep a reproduction read-only against the candidate.

Each finding is one list item, PATH:LINE [p0|p1|p2] what goes wrong, then a line EVIDENCE: reproduced (the input and the observed bad result), EVIDENCE: traced (file:line for each step from the input to the harm) or EVIDENCE: concern (the scenario and why it cannot be shown yet). A finding with no EVIDENCE: line, or an unknown level, is a concern. The reply ends with exactly one line, ADVERSARY: DONE; a reply without it is asked again once, and a second miss fails the run as review_route with the candidate preserved. A reply without it that carries the provider's refusal line, "This content was flagged for possible cybersecurity risk", is not asked again: the pass records outcome refused with no findings or concerns, writes a ledger note, and the round stands on the primary review alone. A refusal is about the content, not the route, so no fallback is switched. This covers a Codex turn that exits non-zero on the refusal: when the Codex CLI's own error or turn.failed event carries the refusal line, the exit comes back as the adversary's reply and is recorded the same way. The line counts only from those CLI events, never from a message or command output that quotes it.

A reproduced or traced finding blocks the round: it is not approved, its reviewRounds row has verdict changes_requested and holds the finding with reviewer adversary and its evidence, and the fix turn gets it in the consolidated list below. Concerns never block and stay out of reviewRounds; each pass's concerns go into one ledger note. Each pass records an adversary_round detail event with round, tier, depth, scope, range, family (claude, codex, configured, or fallback once the route switched), model, effort, family_reason when the family was forced (no adversary_credential, or for fallback the reason its switch recorded; a pass on the container fallback pair records that pair's model and effort), outcome (clear, blocked, refused or malformed), seconds, findings (the blocking ones) and concerns. The pass's agent_turn event labels a Claude turn claude opus.

Consolidation and concerns

On a round with an adversarial pass, the primary's and the adversary's findings become one list before the fix turn sees them; a round without one hands the fix turn the primary's verdict text. The primary's items are the findings of a reply that asked for changes plus its unwitnessed criteria, or only the unwitnessed criteria when it approved; the adversary's are its blocking findings and its concerns. Each item carries found_by (primary, adversary) and an evidence rank: reproduced, traced, review (a primary finding), concern.

A round whose verify failed for a reason other than a timeout also stores a finding naming the failing command, so it reads changes_requested even when the reviewer approved, and the fix turn is also handed the report of a failing verify after the verdict text or the merged list. A fix turn that still makes no progress fails with a reason naming that command.

Pass 1 is factory code. Two items with the same path, the same line (or neither) and the same message, whitespace collapsed and case folded once the bullet, location, severity tag and EVIDENCE: line are set aside, become one item with the higher severity (p0, p1, p2, nit), the stronger evidence and both reviewers. Different findings at one location all stay. The list puts blocking items before concerns, each by severity, then primary before adversary, then the order reported, items at one location kept together.

Pass 2 runs only when the round goes to a fix turn and pass 1 left at least two items. The consolidate seat, [agents] consolidator (see Config), defaults to the review container on the reviewer's model pair. It sees the items as F1, F2 and so on and replies with MERGE: Fa INTO Fb — reason lines, one ORDER: Fx, Fy, ... line and the final line CONSOLIDATED. The factory applies the reply itself: a merge keeps the higher severity, the stronger evidence, both reviewers and both messages, a message merged from another location prefixed with its PATH:LINE; ids missing from ORDER follow in their pass-1 order; a line naming an unknown id, merging an item into itself, closing a cycle, or a second ORDER line is set aside. A failed route or a reply without CONSOLIDATED hands on the pass-1 list and the run goes on; the seat has no fallback, no startup probe and no startup check.

Every blocking item goes to the fix turn, and of the concerns the first three in the final order; the rest are held, and on a round that approves every concern is held. The fix turn gets one numbered list, each entry giving its location, severity, evidence and reviewers, then its messages and any EVIDENCE: lines the adversary wrote (kept out of the pass-1 match), a concern marked to be answered ADDRESS, FOLLOW_UP or DECLINE and never blocking. Each such round records a consolidation detail event with round, pass1_in, pass1_out, pass2 (merged when pass 2 applied a merge, unchanged when it applied none, skipped, unavailable or malformed), merges (from, into, reason), ignored, sent_concerns and held_concerns, and the held concerns go into one ledger note. The adversary_round event and the reviewRounds row keep each reviewer's raw findings.

Under [merge] mode = "pr" the body the factory opens with ends, after its Linear: line, with ## Other concerns (N): one bullet per distinct held concern of the run, PATH:LINE [severity] message (round R). A run that held none gets no section. A description refresh keeps it; a babysit round does not update it. Under mode = "local" the ledger note is the record.

A concern, sent or held, whose path matches BASE_HIGH_PATHS or [review] high_paths is raised once per run, however many passes report it again: a ledger note starting Concern on a high-blast-radius path: and a concern_raised detail event. The rule reads each concern as the adversary reported it, before any merge.

PR rounds

Under [merge] mode = "pr" (see Config) the reviewer's approval is not the last word: the candidate is pushed and opened as a pull request, and the repository's own review bots and people leave threads on it. The loop answers those the way an operator would by hand -- read, judge, fix, reply, wait, repeat -- and passes other than ask-only passes are review rounds of the run, so FINDINGS shows the GitHub rounds beside the Codex ones.

A mention of the configured handle accepts three explicit forms (case and surrounding spaces are ignored): @holophyte ? Can an existing guest be renamed? and @holophyte ask: Can an existing guest be renamed? ask for an answer; @holophyte fix: Change the title to Rename Guest requests a change. A bare mention, such as @holophyte Change the title to Rename Guest, keeps the existing fix behavior. An ask never changes code: the read-only adjudicator answers with file and line citations and advice about whether a change is warranted. It never commits, pushes, or requests review. A pass containing only asks records the replies as instruction events, consumes no review round or work strike, and restores the previous park reason.

A human mention in the pull request conversation follows the same forms. Its reply quotes the request; there is no review thread to resolve. Unmentioned conversation comments are ignored. A bot review's "Outside diff range comments" are treated as bot threads; each is answered with one pull request comment carrying a fingerprint marker, so a later pass does not act on it again.

A merge under human approval requires an explicit approve recorded on the run: --approve KO-n records approvedAt and approvedBy. A --babysit resume or an independent review approval alone does not grant that permission.

The pull request description is rewritten after each approved fix round, using the current diff, ticket and repository conventions, so it describes the candidate now under review. If the writing turn fails, the existing description is preserved.

One pass:

1. Read. One GraphQL query returns the PR's unresolved review threads, the head commit's check rollup, and whether the PR is merged or closed; the threads are paged, and every page is read before anything is decided, so a PR is never "quiet" because its open thread was past the first page. Each thread is read whole: the opening comment and every follow-up, paged to the last one, since a reply can withdraw a finding or turn it into a question the opener alone does not show. A PR someone merged by hand lands the run as merged with that sha; one closed without merging fails the run, branch preserved. A head that is not the candidate this run pushed -- someone else pushed to the branch -- parks the run naming both shas: the checks and threads are about their commit, and the babysitter judges and merges only its own.
2. Verdict. A thread whose latest comment mentions @holophyte (case-insensitive; configurable with [merge] mention_handle) is an instruction. Explicit asks receive read-only answers as described above. Other mentions bypass adjudication, are fixed using that comment's request, then receive an Addressed in SHA reply and are resolved. This also applies to a bot thread with a human mention reply. Otherwise, a thread a person opened -- its opening author is a GitHub User, or an account GitHub no longer names -- is HUMAN, "opened by a person", before the adjudicator is asked: bots get replies, people do not, and no reviewer is named to tell them apart. A project that sets [merge] human_threads = "act" has a person's thread judged with the bots': one asking for a concrete change is addressed -- fixed, answered with the sha, and left unresolved for its author to close, the run parking after the fix with it listed -- and anything else is handed to the operator as HUMAN; a person's thread is never declined. The adjudicator route ([agents] adjudicator, or the default container) is given the ticket, the candidate as the same frozen refs/review/base and refs/review/candidate pair a review round gets, and the bots' threads numbered with their whole conversation, and answers one line per thread: THREAD n: ADDRESS (a concrete defect -- a thread naming an existing function, helper or constant the diff duplicates is one, the fix being reuse), DECLINE (a thread asking for nothing specific, or for what the ticket puts out of scope) or HUMAN (a genuine question, a rejection of the approach, anything it would not answer on the operator's behalf). The repository's AGENTS.md or CLAUDE.md, when it has one, is quoted in the brief as the reviewer's standard. A thread with no verdict line is HUMAN. The pass is recorded as a reviewRounds row -- route github:LOGIN, the threads' authors; github:ci for a pass that found none -- before anything is posted, so an interrupted pass has its row.
3. Fix. The addressed threads go to one fix round on the branch (the implementer, under the ticket's budget). The fix must be one clean commit -- the tree, HEAD and the branch all on it -- before the ticket's verify commands run over it, so what verified is what is pushed; then the branch is pushed. A fix round that commits nothing, leaves edits uncommitted, or one the verify commands fail on, fails the run with the branch preserved, nothing pushed, nothing posted.
4. Reply. Each addressed thread gets a reply opening ---- Comment by MODEL ---- (the adjudicator's route, never a constant), then what changed and the sha it changed in, and is resolved. Each declined thread gets a reply with the reason. If its opening author's login ends in [bot] or is listed in [merge] bot_authors (default devin-ai-integration, coderabbitai, greptile-apps, github-actions), it is then resolved; otherwise it stays open for its author to close. A HUMAN thread gets no reply at all. Every reply and every resolve is a runEvents row.
5. Park or go on. A HUMAN verdict ends the pass with the run parked and the thread quoted in the ticket's question; only declines left open for their authors park the run with those threads listed. Resolved bot declines allow the next pass to proceed toward merge if no other gate holds it. The next pass reads the PR again -- new threads, the checks the fix restarted. A pass with no thread waits for pending checks (github.CHECK_POLL_S between reads, at most github.CHECK_WAIT_S). The checks are read beside the rollup: the head's check runs and the contexts main's rules require. A check still running, or a required check not yet reported, is pending even when the rollup already says success -- seconds after a PR opens only the instant checks have reported -- and so is a read of the runs or the rules the babysitter could not make. Red Actions checks are rerun once a pass; still red, they get a check fix round, an implementer turn handed the failure lines of each job log and its tail. A red read on the fix's commit gets another round only when its failure is new and [merge] check_fix_rounds is not spent; a repeated failure, spent rounds, a red status that is no Actions job or a round that makes no commit park the run. Green checks are "ready to merge": the PR is merged through its merge API, pinned to the candidate's sha so a push that races the pass is refused rather than landed, under [merge] approve = "auto" or after the operator's --approve, and parks for the human under approve = "human".
6. Review the fix. A fix round moved the candidate past the sha the reviewer approved, and the fix is the implementer's work nobody independent has judged. So before the merge API is called on a moved candidate, the reviewer route reviews it at its fixed sha -- verify first, the same read-only brief and frozen refs/review/* pair as a review round, one more reviewRounds row -- and only an APPROVE that accounts for every acceptance criterion merges; REQUEST_CHANGES, or an approval that leaves a criterion not met or unwitnessed, parks the run with the findings in the ticket's question, with no further fix round. The merge API is never called on a sha this process has not verified: a resumed run's candidate goes through the merge gate (the ticket's verify commands, then the drift check) first, and a failure stops the run there as under mode = "local". The operator's --approve was of the sha it released: a candidate moved since parks for the human again under approve = "human". The sha the last approval covered survives the park as runs.approvedSha, so a --babysit re-entry merges the candidate only at that sha and reviews it again at any other -- a fix the reviewer rejected has none on record, and is reviewed again before anything merges it.

That review reads what changed since the approval, not the whole candidate again. The reviewer is told to review the range from the approved sha to the candidate, those commits and whatever they touch; the rest stands on the earlier approval. When the range holds a merge of main, the reviewer is told to review it as git log -p --remerge-diff of the range less the merged main's history: the candidate's own commits and each merge's conflict resolution, with none of the hunks a merge took cleanly from main, which were reviewed on their own pull requests. The reviewer still answers for every criterion, and for one the range does not touch it may cite that approval as approval at SHA; tests/file.py::TestClass::test_name, where SHA is the approved sha (a test that is not Python is named path::"test name" or path::TestName). The prompt lists the test files the range changed, a merged main's included, and a citation of any of them is void: that criterion must be witnessed afresh. The gate holds the reply to the same rule -- a citation that does not name the approved sha, names no test, or names a test file changed in the range leaves its criterion unwitnessed, and the run parks. So a covering approval is only as good as the earlier review of each test it cites, and only while that test is unchanged. When the last review of the candidate asked for changes there is no approval to cite, and the candidate is read whole, the fixes included.

After [merge] pr_rounds passes the run parks naming the cap, whatever the PR looks like. A pass that only merges main into the branch (GitHub reported a conflict, or refused the merge as behind main) does not count toward pr_rounds; [merge] pr_main_refreshes (default 10) caps those passes instead, and the run parks naming that cap when it is reached.

Every park is the awaiting_merge_approval park of approve = "human": the ticket asks PR open: URL with why and the open threads listed, the run keeps runs.prUrl, runs.candidateSha and runs.approvedSha, branch and worktree stay, the lease is released. --approve KO-n answers "merge": the resumed run babysits once more and merges when green and quiet. --babysit KO-n answers "look again": the same resume, parking again rather than merging under approve = "human". It is refused on a run parked with no pull request (parked under mode = "local"): there are no threads to look at, and the local gate merges on release, so that answer is --approve's only. Merging is GitHub's; local main is never moved by the factory.