Review model
xiv's review architecture follows one design decision: code review happens locally, before anything is pushed. A review agent reads the diff on disk and judges it against the issue's acceptance criteria while the implement loop is still running — so by the time a PR exists, the only signals left that GitHub alone can supply are CI status and comments from human reviewers. Nothing in xiv waits on, polls for, or classifies a remote review bot.
Four surfaces implement that decision:
| Surface | Where it runs | Reads | Allowed to do |
|---|---|---|---|
| Local acceptance review | impl:review step inside linear-implement | working-tree diff vs. acceptance criteria | withhold the loop's "done" — never grant it |
| PR review loop | pr-review-loop — xiv pr refine, and the pr step of xiv ship | CI + human comments on one PR | fix findings, push; never merge |
| Stack review | stack-review — xiv stack review | CI + human comments across every open PR in the stack | fix in the owning branch via jj, cascade up, push; never merge |
| Inbound review | xiv pr review (interactive CLI) | someone else's PR, in an isolated worktree | draft a review you edit and submit; never merge |
Local review inside the implement loop
Each pass of the implement loop runs implement → validate → review. The reviewer reads the working-tree diff against the Linear issue's acceptance criteria (the judgment it applies is the xiv-review-core skill) and returns a LocalReview — the one review vocabulary in the system, defined in pack/components/LocalReview.ts:
verdict: "approve" | "request_changes" | "comment" // no schema default, on purpose
summary: string // markdown; the review body
findings: [{ id, severity, title, body, path, startLine, line, inlineable }]
// severity: "blocker" | "high" | "medium" | "low" | "nit"Two properties of this step are load-bearing:
- Validation is the arbiter; review is advisory. The loop's done-gate is
validationPassed && !requestedChanges. A review may only ever withhold done —request_changessends the work back for another pass — never grant it:approveandcommentboth defer to whether tests/lint/typecheck actually passed. - A broken reviewer cannot wedge the build.
verdictdeliberately has no default: a reviewer that cannot produce a real verdict fails its Task rather than silently degrading into a fake one. The Task iscontinueOnFail, so a crashed, timed-out, or unparseable review simply leaves that iteration with no review, and validation decides alone. Demanding approval from a review that does not exist would burn every iteration with nothing real gating.
Non-approving findings — including from a non-blocking comment verdict — are fed into the next iteration's implement prompt as previous-attempt feedback, so the implementer sees them even when it was failed validation that triggered the retry. Review state is read per-iteration, never across iterations: a request_changes from pass 1 cannot block pass 3, and a stale approve cannot wave a later broken pass through.
What --skip-acceptance-review changes
The flag drops the impl:review step from the loop entirely. Validation (tests, lint, typecheck) still gates every iteration, and since an absent review already means "gate on validation alone", skipping cannot wedge the loop. It is usually the right trade when issues are fully spec'd — the review step is one of the pricier agents per pass (see model tiers).
Scope rules:
- On
xiv stack plan, the flag is recorded in the stack map (skipAcceptanceReview: true) as the feature-wide default. - On
xiv implement,xiv ship, andxiv stack build, it applies to that run and ORs with the map default — a per-run flag can force skipping on, but can never re-enable review for a map that opted out.
Where unfixed findings go: Known gaps
The loop may end with findings still open — it hit its iteration cap, or the final review raised things validation did not force it to fix. Those findings are not dropped; they become the PR's honesty section.
Mechanically: linear-implement's compose-output step reads the final review and copies every finding it still reported into the workflow output's knownGaps — anything the last review still listed is by definition unfixed, because the loop ended without addressing it. xiv stack build persists them on the stack entry; xiv ship forwards them in-process. When the PR is described later — a separate command, in a separate run — the describer receives them verbatim instead of re-reviewing the diff to rediscover them, and renders them under ## Known gaps in the PR body. See stack map & triage for where they are stored.
PR-level review: xiv pr refine and xiv pr fix
Once a PR exists, pr-review-loop reads exactly two kinds of signal (pack/components/PrReview.ts):
- CI — aggregate check state:
passing,failing,pending, ornone, plus the names of the failing checks. - Human comments — unresolved review comments, each a finding with
source: "human"and the reviewer's GitHub login as its origin.
Each round takes one snapshot of those signals — read-only, and explicitly not a poll — then fixes what they raise and pushes. A round that finds checks still pending and nothing else simply ends: pending CI alone leaves nothing to do, so the fix step is skipped rather than run against nothing. clean means checks green (or absent) and no unaddressed human comment.
maxRounds defaults to a deliberately low 2: code review already happened locally before the PR existed, so the only things left to chase here are a red build and a human's comment. Two rounds fixes the build and confirms the fix; anything beyond that is a person's call, not a loop's.
xiv pr fix is the loop-less variant: address the existing findings once, push, and post nothing back.
Stack-wide review: xiv stack review
stack-review runs the same signal sweep across every open PR in the stack (or one repo's substack, with --repo). The difference is where fixes land: each finding is fixed in its owning branch — the entry the problem actually belongs to, not the tip — and jj cascades the edit up through every descendant; every touched branch is then force-pushed. Same 2-round cap, and when the stack is not clean at the end, it escalates to a human rather than continuing to loop.
Reviewing others' PRs: xiv pr review
The inbound direction: reviewing a PR someone else wrote. xiv pr review [number] checks the PR out into an isolated worktree under SMITHERS_HOME/review/, runs a review agent against it (same LocalReview schema; prompt template overridable with --prompt-file), then hands you the result:
- Pick the verdict — the agent's suggestion is pre-selected, or choose "don't submit" to keep the notes only.
- Toggle findings in or out (nits are deselected by default).
- Edit the top-level body in your editor, then confirm.
Findings whose file and line verifiably sit on the PR diff's new side post as inline comments; if GitHub rejects them anyway, the submission retries once with those findings folded into the body, so a review is never lost. --auto-submit skips the interaction and submits the agent's verdict with all inline-able findings as-is. Findings and the draft body persist in a per-PR scratch directory either way.
What no stage may do
No review stage merges — ever. Every autonomous agent carries a hard system-prompt rule: never run gh pr merge, never enable auto-merge; when a PR looks ready, stop and report that it is ready for a human to review and merge. The terminal report steps of both review loops repeat the instruction. xiv drives a PR to mergeable and hands it over — merging is a human's job.