Skip to content

The engine seam

The one place a different coding agent plugs in. Everything else โ€” polling, allowlisting, dedupe, leasing, the budget windows, publishing โ€” is agent-agnostic, so only "run a review" is swappable โ€” and, since 1.12.0, "describe a pull request", which is the same call with a different schema.

This page documents the seam and the one adapter that implements it. The adapter can spend money, and since the review worker landed it has a caller: the worker claims a row, checks the code out and runs the engine the configuration names. For why the design has a seam at all, see DESIGN.md.

๐Ÿ”Œ The protocol

1
2
3
4
5
6
7
class ReviewEngine(Protocol):
    name: str

    @property
    def capabilities(self) -> Capabilities: ...

    async def review(self, request: ReviewRequest) -> ReviewResult: ...

Async, because every adapter will be waiting on a subprocess and the daemon and workspace already are.

name is recorded on the ledger row, so a posted comment is traceable to what produced it.

๐Ÿ“ฅ What an engine is given

Field Type Why it is there
checkout Checkout The tree on disk, its head_sha, the merge base, and the diff.
facts PullRequestFacts Number, base ref and the size counts the caps were measured against.
trigger Trigger What asked for this run โ€” an opened pull request or a mention โ€” and its command, review or describe.
mode Mode The rung of the degradation ladder the run was admitted under.
prior tuple[Finding, ...] What the last completed round on this pull request found. Empty on a first round.

prior is what lets a re-review say "still" truthfully instead of re-deriving everything from scratch. It arrives stripped of finding bodies โ€” only number, path, severity and title travel.

That stripping is a control, not an economy. A body is the longest and least constrained field a reviewer emits over an untrusted tree; carrying it forward would let text that reached one review reach every later review of the same pull request, which is a foothold that outlives its own run. A test pins the absence rather than a comment asserting it. The block is fenced by the same fencing the diff gets, because a title is untrusted text too.

The diff is not passed separately. It is already on the Checkout, and two copies of one string are two things that can disagree.

mode is passed rather than inferred so an engine can spend less under mention_only โ€” fewer turns, a tighter prompt โ€” instead of the governor's only lever being to refuse the run outright.

The tree and the diff are untrusted input. An engine may read them and must not let them widen what it is allowed to do. The rule is the same one that governs the allowlist; see DESIGN.md.

๐Ÿ“ค What an engine returns

ReviewResult is findings, usage and the outcome, plus the assessment every completed review carries and, on a @claude describe run, the description in place of both โ€” see DESCRIBE.md. The adapter reads the trigger's command to choose the schema and the system prompt; the rest of the argv, the sandbox included, is the same.

A Finding is path, line, severity, title, body and an optional number.

title is the one-sentence headline the report renders in bold, and it states the consequence rather than the mechanism โ€” it is read first and often instead of the body. The remedy is the last paragraph of body rather than a field of its own: the schema is kept small on purpose, because every required field is another way for a run to end in a validation failure that spent tokens and produced nothing.

number is the finding's identity across review rounds, and it is the one field an engine may leave unset. An engine that recognises a finding it was shown from an earlier round sets that finding's number; anything else is new, and numbering.assign gives it the next free one before the run is recorded. A number an engine invents is discarded: it is output over an untrusted tree, so it is honoured only if this pull request actually issued it.

Severity is advisory in the strongest sense: the publisher posts a plain issue comment, with no review event, whatever a review concludes, so not even blocker can block a merge. See PUBLISHER.md for which heading each level renders under.

outcome is how the run ended: completed, truncated or failed. It exists because a run that was cut off and a run that cleanly found nothing produce the same empty findings tuple, and only one of them reviewed the pull request. ReviewResult refuses to carry findings on any outcome but completed, so "the publisher may post these" is a property of the seam rather than a rule each adapter is trusted to follow.

Usage is carried on every outcome, including the failed ones. The money is spent whatever the run concluded, so it still has to settle.

usage is budget.Usage itself, not a parallel type. That is what makes "a result carries everything Governor.settle needs" true by construction rather than by review โ€” settle takes exactly that object. Two invariants are enforced in __post_init__:

  • usage_confidence: unavailable may not also report a token count. An unknown cost and a number are different answers, and a ledger row claiming both would let a run that measured nothing still draw down a window.
  • usage.engine must be set. An unattributable cost is not settleable.

๐ŸŽš Capabilities

Five booleans, none with a default, because a default would be a claim no adapter actually made.

Flag Consumed by Meaning
structured_output the prompt contract Whether the engine can be schema-constrained. If not, the adapter needs a fenced-JSON contract with a budgeted validation retry.
usage_reporting the governor Whether the engine reports what a run cost.
read_only_sandbox โ€” Whether the engine can be confined to reading.
subagents โ€” Whether the engine can fan out.
prompt_caching โ€” Whether repeated context is billed once.

The last three have no consumer yet. They are declared because DESIGN.md names them and because an adapter's answer is knowable when the adapter is written rather than months later.

usage_reporting: false is the interesting one

An engine that reports no tokens forces the governor onto proxy controls only โ€” run count, wall clock, turn caps. That is a materially weaker guarantee than a token count: the windows stop measuring spend and start measuring activity. It is why every ledger row already records a usage_confidence of exact / estimated / unavailable, and why the capability is declared in advance instead of being discovered from the first run that reports nothing.

The conformance suite enforces the pairing: an engine whose capabilities say usage_reporting: false must settle as unavailable, and one that says true must not.

๐Ÿงช FakeEngine

Canned findings, a canned Usage, a recorded list of every request it was handed, and no I/O of any kind. It is what lets the review step be tested without a network, an API key or a token โ€” the seam's most immediate return, before any adapter exists.

tests/test_engine.py holds a conformance suite parametrised over a list of engines. ClaudeCliEngine joins it with its subprocess stubbed out, because every rule in the suite is about what an engine returns. A second adapter joins the same list rather than growing its own copy of the rules.

๐Ÿ–ฅ CliEngine: the subprocess boundary

The part that is the same for claude, codex and opencode: launching the tool with cwd set to the checkout, an environment built rather than inherited, a wall clock it cannot outlive, and the error vocabulary โ€” EngineUnavailable, EngineTimeout, EngineProtocolError. A subclass supplies the argv, the prompt, the parser and the capabilities.

The environment is an allowlist โ€” PATH, HOME, and whatever name prefixes an adapter declares. os.environ minus a denylist fails open the moment a new variable appears, and the variable that must never reach this process is the agent's GitHub credential: a process reading an attacker's tree and feeding a public comment is the wrong place for it. A test asserts GH_TOKEN is absent by value.

The prompt goes on stdin, not in the argv. It carries the diff, and a diff-sized argv hits the platform limit on exactly the pull requests that most need reviewing. It also keeps the argv small enough to log whole, which is what makes a posted review traceable to the invocation that produced it.

Untrusted text inside the prompt is fenced, and the fence is sized to its contents: the builder finds the longest run of backticks in the diff and opens with one more than that. A fixed three-backtick fence is closable by any diff that contains one, which would let the "this is data" boundary be ended by the data.

A timeout kills the process before it prints anything, so EngineTimeout carries no usage while the governor is still holding a reservation. The honest settlement for such a run is the full reservation at unavailable confidence; that is the worker's call, and the adapter only guarantees the failure is distinguishable.

๐Ÿค– ClaudeCliEngine

1
2
3
4
claude -p --output-format json --json-schema <schema> --model <configured>
  --system-prompt <agent-authored> --tools Read,Grep,Glob --restricted
  --setting-sources '' --strict-mcp-config --disable-slash-commands
  --no-session-persistence --permission-mode dontAsk --permission-prompts none

Three of those are the injection defence, and they are argv a test asserts rather than prompt wording a model can be talked out of:

  • --tools names the whole tool set, and it is read-only. No Write, no Edit, no Bash.
  • --restricted removes the command-running tools a second time and ignores user, project and local settings files โ€” so a CLAUDE.md in the tree under review is not instructions to the reviewer.
  • --permission-prompts none means nothing can escalate by asking.

--bare would give the same isolation and was rejected: it forces ANTHROPIC_API_KEY authentication, which would silently settle the billing question DESIGN.md leaves open.

Capability Value Because
structured_output true --json-schema validates the output and re-prompts on mismatch
usage_reporting true the envelope reports tokens
read_only_sandbox true --tools plus --restricted
subagents false the CLI can fan out; this tool set does not let it
prompt_caching true the envelope reports cache tokens

Preflight refuses an unconfined run

The three flags above are the whole sandbox, and argv is only a control while the binary still has the flag. A future claude that renamed or dropped --restricted would take the argument and ignore it, and the adapter would review an attacker's tree unconfined while the log said "output parsing may fail".

So preflight does two things with two different endings:

Probe On a surprise
claude --version against engine.expected_version WARNING, and proceed
claude --help against REQUIRED_FLAGS EngineUnavailable โ€” refuse

The version only warns because the parse is what protects against output changing shape and it already fails loudly; refusing would take the reviewer offline on a routine upgrade that changed nothing we read. A missing containment flag is the opposite: offline is the correct failure, and EngineUnavailable is the right shape for it because nothing was executed, so the worker settles the run at a provable zero.

REQUIRED_FLAGS is spelled out rather than derived from a call to argv, which would need a ReviewRequest it has no use for โ€” and a test asserts the constant is exactly the set of long flags argv passes, so the two cannot drift.

Both probes run under PROBE_TIMEOUT_SECONDS (30 s). They are awaited inside review() before run()'s own wall clock starts, so without one a claude --version that hangs on a network update check holds the worker until the lease lapses โ€” and nothing claims that pull request again until a restart.

This is roadmap item A3.

Reading the envelope

Stdout is one JSON object, and the parse is strict:

Envelope Result
subtype: success with structured_output findings, completed
subtype: success without it no findings, failed
subtype: error_max_structured_output_retries no findings, failed
subtype: error_max_turns no findings, truncated
any other subtype, or is_error no findings, failed
not JSON, or not type: result EngineProtocolError

An unreadable envelope raises rather than being reported as an empty review. Parsing what a CLI prints is the cost of the subprocess boundary, and a format change that silently became "no findings" would look exactly like a clean review.

usage.tokens is every *_tokens field the envelope reports, cache reads included: cheaper than fresh input, not free, and the windows measure what a run consumed. total_cost_usd is on the envelope and deliberately not recorded โ€” the windows are token-denominated.

The schema retry is inside the CLI, and it spends

--json-schema does not merely validate. When the model's output does not fit the schema the CLI re-prompts by itself, and only gives up with error_max_structured_output_retries. That retry is not a lever this adapter holds: there is no flag to disable it and no callback before it fires.

Two consequences worth stating rather than discovering from a ledger row:

  • A single review() call can cost several model turns. The reported usage covers all of them, so the ledger stays honest โ€” but a run's cost is not bounded by one turn's worth of tokens, and the pre-flight estimate should not be read as though it were.
  • error_max_structured_output_retries is the expensive failure: it is the outcome that spent the most and produced the least. It maps to failed rather than truncated deliberately โ€” retrying it costs the same again with no reason to expect a different answer.

Keeping FINDINGS_SCHEMA small and flat is therefore a spending decision, not a style one. Every required field is another way for a run to end in the retry path. The one required addition since is assessment (issue #126): effort, risk, recommendation and priority files, all enums or bounded values, which constrained decoding rarely gets wrong. It is required by decision, and the adapter refuses output that leaves it out.

The sizes in the prompt are the reviewable ones

The prompt quotes checkout.reviewed โ€” what survived budget.excluded_paths โ€” and not PullRequestFacts' API totals. The diff the engine is shown has already had the excluded paths removed, so quoting the API's figures would tell the reviewer it is missing files that were withheld on purpose, and invite it to go looking for them with the tools it does have.

Standards come from the base ref

The engine is generic; the standards are per-repository. They are read at the checkout's merge base rather than its head, so opening a pull request cannot rewrite the reviewer's instructions, and read through the run worktree's shared object database, so it costs no second fetch. What that trusts is stated in DESIGN.md: anyone who can merge to the base branch. Configured in CONFIG.md's engine.standards_paths.

Version

claude --version runs once before the first review. A mismatch against engine.expected_version warns and proceeds: the parse is what actually protects the run and it already fails loudly, while refusing would take the reviewer offline on a routine upgrade that changed nothing we read.

๐Ÿงต Who calls it

The review worker, and nothing else. It builds the ReviewRequest from a claim, hands it over, and treats any exception the engine raises as one retryable fact: the run produced no review. An adapter therefore does not need to classify its own failures for the caller's benefit โ€” it needs to fail rather than return an empty review.

What the worker does need is an honest usage: it settles the ledger with exactly what ReviewResult.usage reports, and a run that fails after the engine started is charged its full reservation.

The exception is EngineUnavailable, and it is a contract on the adapter rather than a courtesy: raise it only when nothing was executed. The worker settles it at zero, so an adapter raising it after doing work would record a real spend as nothing.

๐Ÿšง What lands next

The publisher has landed, and so has the circuit breaker: an adapter now recognises the account's own usage limit and raises UsageLimited, which trips the breaker rather than being retried into the same wall. The worker has landed too, and with it layer 3's wall clock โ€” engine.timeout_seconds is validated at startup as strictly below the queue lease, which is the ceiling the subprocess boundary can actually enforce.

What layer 3 did not build is a token-denominated run ceiling, and this page is why. --max-budget-usd is denominated in dollars while every window here is denominated in tokens; converting between them needs a price that goes stale silently and is wrong the moment --fallback-model fires. Deriving a clock from a tokens-per-second rate fails on the relationship rather than the arithmetic โ€” a run that greps twenty files burns a minute and almost no tokens, while one that writes a long analysis burns seconds and thousands. And watching usage stream past, which is the honest version, assumes an engine that streams usage at all โ€” while usage_reporting exists precisely because one will not. A ceiling only the claude adapter could honour is the opposite of a seam. It went to the breaker instead, which is the right shape for a bound nobody can enforce mid-run.

Layer 2 is not among them: path exclusions and the pre-flight estimate needed only a diff, so they landed alongside the adapter rather than behind it. An engine is handed a diff with excluded paths already absent, and a run predicted to cost more than max_run_tokens never reaches one. The adapter's part is to report usage honestly, which is what keeps the estimate's fit calibrated โ€” an engine declaring usage_reporting: false produces rows the fit deliberately ignores.