Diff Review — Independent Diff-First Code Review

Classification: Degraded — independence has three rungs. Strongest is a reviewer whose model differs from the diff's author (context- and model-independent — a different model has different blind spots); next is a fresh independent sub-agent in the same runtime (context-independent); weakest is an in-context deliberate reset (read the diff as an outsider, ignoring the rationale that produced it). Rung 1 launches the other runtime — or this runtime's own CLI on a backend running a different model — under a bundled Python 3 supervisor (review_runner.py); where Python 3 or that CLI is absent, the skill falls open to rung 2. Coverage is always preserved down this ladder; only the strength of independence varies with what the host offers.

Progress: observable — rungs 2–3 return in one shot; rung 1's background reviewer streams into an append-only display log a human may tail -f while it runs. The log is read by nothing on the correctness path: the supervisor (review_runner.py) judges liveness from the child's live stream in-process (never by re-reading the log), the authoritative verdict comes from a separate findings file, and termination is the supervisor's own (child exit / idle timeout / wall-clock deadline) — so the review completes identically whether or not anyone watches the log.

Overview

Provide a second, independent pass over a change set — the adversarial reviewer, not the author. It is deliberately different from author self-review (cyw): where self-review is broad and benefits from knowing why the code was written this way, this review is stronger precisely because it does not know or trust that rationale. It reads the diff on its own terms and asks whether the code is correct.

Strongest with a different model available. When the host has another runtime's CLI available (e.g. Claude reviewing Codex-authored code, or the reverse), the review runs in the runtime that did not write the code, so a different model — different training, different blind spots — examines it, catching classes of defect the authoring model is systematically likely to miss. With only one runtime, a backend that runs its CLI on a different model gives the same independence; with neither, it falls open to a fresh same-model reviewer (independent context, same model), so coverage is preserved either way — see the independence ladder in Step 2.

Two properties are non-negotiable:

• Diff-first, fresh context. Review the diff as an outsider would, without the implementation conversation coloring judgment.
• No tree edits — bounded by flags, not merely requested. The reviewer reports; it never fixes. On rung 1 per-instance flags set that bound, and neither is a lock. Codex -s read-only -c approval_policy="never" has the operating system refuse the shell's writes on Linux and macOS, less reliably on native Windows, while its own edit tool is checked only inside Codex. Claude --permission-mode plan is Claude Code's own permission check, which the user's settings can widen: an allow rule for the shell lets a shell write through. Neither flag constrains user-configured hooks, plugins or MCP servers. For an airtight boundary, run the reviewer on a read-only copy or mount of the tree. Findings go back to the author/gate.

Step 1 — Select the diff

Establish exactly what is under review, read-only:

• Uncommitted work: the staged and unstaged changes, and untracked files — a file authored during this work is outside every git diff range and reaches the commit unread. List them with git status --porcelain -uall and read them directly. Never git add -A -N to pull them into a diff: it writes to the index, changes what git status shows the user afterwards, and is a modification by a skill whose whole contract is that it makes none.
• A branch/PR: the diff against its base (e.g. git diff <base>...<head>).
• A single commit or range when asked.

If the scope is ambiguous, ask the user which range to review. If operating autonomously (no user available), resolve the base as the merge-base against the repository's default branch; use @{upstream} only where it tracks a different branch, never this branch's own remote counterpart — on a pushed branch that base is HEAD, so the selected diff is empty, and with unpushed commits on top it is a silent subset of the branch. State the resolved base in the noted assumption.

An empty selection is a stop, not a clean result. If the selected diff is empty, review the staged (then unstaged, then intent-added untracked) changes; if those are empty too, report no diff under review and stop. Reporting "nothing substantive was found" with blocking_count: 0 is how a gate passes code nobody read.

Step 2 — Establish an independent review context

Review with as little contamination from the authoring context as possible. Independence has two dimensions: context (the reviewer does not see the authoring conversation) and model (a different model has different blind spots than the author's). Prefer the strongest rung the host offers, and fall open down the ladder — an unavailable reviewer must never block the review:

1. Different model (strongest — context- and model-independent). If a runtime other than the author's is available, or the user names a backend that runs this runtime's own CLI on a model other than the author's, dispatch the review to it over the selected diff and collect its findings. A different model catches classes of defect the author's model is systematically blind to. Bound it and fail open (below).
2. Same-model fresh reviewer (context-independent). No different model available → spawn a fresh independent reviewer in the current runtime that reads only the selected diff and the minimal surrounding code, not this conversation.
3. In-context deliberate reset (weakest). Cannot spawn any independent reviewer → perform a deliberate reset: set aside the implementation rationale, re-read the diff from scratch as though seeing it for the first time, and judge it on the code alone. Note in the report that the review ran in-context (Degraded).

Because a spawned or rung-1 reviewer does not inherit this skill, the prompt handed to it must carry the Step 3 correctness checklist, the Step 4 report format, and the "report only, never edit the tree" constraint.

Rung 1 runs the different-model reviewer under a bundled supervisor. Launch it through review_runner.py (shipped beside this skill) — a short-lived Python 3 supervisor that owns the reviewer process for exactly the review's lifetime, then reports and exits, so a single blocking call is self-bounding and safe to wait on. Invoke it by its absolute path with a resolved Python 3 interpreter (it lives next to this SKILL.md), and pass --cwd <repo> so the reviewer runs in the repository — a runtime whose diff flag doesn't change the working directory needs this. It tees the reviewer's stream to an optional display log (a human may tail -f it in a separate pane), routes the authoritative verdict to a separate findings file, and bounds the reviewer three ways: the child's exit, an idle / heartbeat timeout (kills on a full window of silence, judged from the child's live stream — so a long-but-active review runs to completion while a genuine hang dies early), and an absolute wall-clock deadline backstop. It prints a one-line JSON status; on a status that means the review did not happen — idle-timeout, deadline, launch error — or when Python 3 or the rung-1 reviewer's CLI is unavailable, fall open to rung 2 (or 3), recording the attempted rung, the fallback, and the reason. A missing verdict is not one of those. The Codex adapter deliberately ships no schema flag (see the adapter note below), so a review that returns good prose and no parseable object is the expected outcome there, and it is a successful review — read the findings as prose, as the enforcement paragraph below says. Falling open on it would throw away a completed cross-model pass and redo it same-model, losing the one property rung 1 exists for. Keep the display, findings and verdict files outside the repo worktree (or gitignored) and delete them after; never stage them. Liveness is judged from the child's live stream in-process, so the display log stays read-by-nothing on the correctness path — the caller consumes the reviewer's full review transcript (all its message text, assembled by the supervisor from the stream into the findings file), not the raw event log, not its tool calls, and not the tree.

The verdict is machine-read, so its shape is pinned where that is free — the narrative always wins where it is not. The object a gate keys on is review-schema.json (shipped beside this SKILL.md): findings[] with file / line / severity / summary / failure_scenario, plus overall and blocking_count. Pass --verdict-json <v> and the supervisor writes that object to its own file; the one-line JSON status then reports verdict (the path, or null) and verdict_reason. --findings still receives the reviewer's full narrative — the verdict is written alongside the reasoning, never in place of it, because the reasoning is what a human reads and what makes a finding actionable.

Adapter note — why only one rung-1 command carries a schema flag. The two runtimes put a schema-validated object in different places, and only one of them keeps the prose: Claude returns the object on its terminal result event (structured_output) while its assistant messages stay ordinary prose — two separate channels, so enforcement costs nothing and its command carries --json-schema ⟪schema_json⟫. Codex coerces every agent_message to the schema, so under --output-schema its running narration is replaced by a series of stub verdict objects with empty findings arrays — the reasoning is destroyed, not relocated. Its command therefore omits the flag and the prompt asks for a closing verdict object instead, which the supervisor extracts from the transcript. Unenforced there, and that is the correct trade: losing the reasoning is a regression, not a simplification. (plan-duel's judge has no such conflict — it captures only the final message, so nothing narrative exists to lose, and both of its judge adapters enforce the schema.)

Where a flag is used, one schema file serves every runtime: the supervisor substitutes ⟪schema_path⟫ (an absolute path, for a flag taking a file) or ⟪schema_json⟫ (the same document inline, for a flag taking the schema as text) into the reviewer argv, so neither the schema nor the review prompt is forked per runtime.

Enforcement is degrade-only everywhere. Rung 2 dispatches in-harness with no CLI flag at all, and a reviewer that returns a good narrative without a parseable object is still a successful review — the supervisor reports the missing verdict and the caller reads the findings as prose. Never require the object.

findings[] is the authority and blocking_count is derived from it, so when the two disagree the supervisor corrects the count and says so in verdict_reason — publishing a known-wrong number with only a warning attached would leave the trap armed for any gate that reads the count without reading the note. It corrects it only as far as the findings support: a positive claim is never lowered to 0, and an unrecognized severity floors it at 1, because 0 is what a gate reads as clean and an off-enum spelling would otherwise erase a real finding. The number can therefore be higher than the entries listed — gate on findings[].

The heartbeat is only honest if the output flows unbuffered end to end: the reviewer runs in a per-event-flushed streaming mode (the adapters pass --json / --output-format stream-json --include-partial-messages), the supervisor reads it in raw chunks (os.read) and flushes each chunk to the display log as it arrives — so the heartbeat ticks per chunk, not only on newline. Without the CLI's streaming mode, output block-buffers into the pipe and a working review can look silent — so the streaming flag is not optional on rung 1.

Every path in the two lines below is absolute — --cwd, --display, --findings, --verdict-json. With --cwd set the supervisor refuses a relative output path, since it and the reviewer would resolve it against different directories, and this skill reads that refusal as grounds to fall back.

Claude adapter (Claude is running this review): rung 2 — run the review as an independent sub-agent (Agent tool, subagent_type general-purpose). Rung 1 — launch Codex through the supervisor, read-only-enforced (-s read-only -c approval_policy="never" — the sandbox rejects writes and the approval policy forbids escalation) with --json for a live heartbeat, returning its full transcript (--result-mode stream-transcript): <python> <skill-dir>/review_runner.py --idle 900 --deadline 1800 --cwd <dir> --display <cap> --findings <f> --result-mode stream-transcript --verdict-json <v> -- codex exec --json -s read-only -c approval_policy="never" --skip-git-repo-check -C <dir> "<review prompt>". Deliberately no --output-schema here — see the asymmetry note above; ask for the verdict object in the prompt instead and let the supervisor extract it. Codex adapter (Codex is running this review): rung 2 — use the native /review, or a fresh codex exec --skip-git-repo-check -s read-only -c approval_policy="never" -C <dir> "<review prompt>" reviewer over the diff. Both flags, on the fallback rung too — -s read-only bounds the shell and the approval policy bounds the built-in edit tool, and a reviewer whose edit tools are unbounded is not a review. Rung 1 — launch Claude through the supervisor with its edit tools blocked (--permission-mode plan, bounded as the rule above says), returning its full transcript: <python> <skill-dir>/review_runner.py --idle 900 --deadline 1800 --cwd <dir> --display <cap> --findings <f> --result-mode stream-transcript --schema <skill-dir>/review-schema.json --verdict-json <v> -- claude -p "<review prompt>" --add-dir <dir> --permission-mode plan --json-schema ⟪schema_json⟫ --output-format stream-json --include-partial-messages --verbose. Controller vs author / probe: rung 1 wants a reviewer whose model differs from the diff's author. If you — the runtime running this skill — already differ from the author, review directly: you are the cross-model reviewer. Otherwise a named backend picks the CLI by its harness: the other runtime's harness launches that CLI; this runtime's own harness launches this CLI, and only when the backend's model differs from yours, compared as text — an equal model is refused for rung 1, recorded, and falls to rung 2. The two rung-1 commands above belong to the CLI launched, not the host: a Claude host reviewing on Claude uses the Claude line, a Codex host on Codex the Codex line. With no backend, launch the other runtime. Launch only when the CLI is on PATH (probe command -v <cli>; native Windows where <cli> / Get-Command <cli>) and Python 3 is present; else fall open to rung 2.

A backend chooses rung 1's model. A user may keep a backends file: a JSON object at ~/.portable-agent-skills/backends.json (or where PORTABLE_AGENT_SKILLS_BACKENDS points) mapping each name to harness (the CLI it is written for), model, and optional args, env and env_from_parent. When the user names one, pass --backend <name> to the supervisor and write ⟪model⟫ and ⟪backend_args⟫ into the reviewer argv; the supervisor fills them, checks the backend fits the CLI launched, and hands the reviewer its settings and key. With no backend named, rung 1 runs on the CLI's own sign-in. Rung 2 is the host's in-process sub-agent, which runs the host's own model, so no backend applies there; rung 3 starts no agent. The report says which ran: rung 1 with the backend's name and model, or on the CLI's own sign-in, and which CLI; rung 2; or rung 3. On a backend for this runtime's own CLI it names your own model beside the backend's and states their independence as unverified: the two were compared as text, and two spellings of one model compare as different. A refused backend is a launch error: fall open and record why.

Adapter note — a backend on rung 1. Add --backend <name> before the -- and the markers after the program: codex exec --json -s read-only -c approval_policy="never" -m ⟪model⟫ ⟪backend_args⟫ …, or claude -p "<review prompt>" --model ⟪model⟫ ⟪backend_args⟫ ….

Waiting for the supervisor to finish — never poll for its process. The completion signal is the one-line JSON status the supervisor prints to stdout — that is the authoritative result. The display log's final [review_runner] end status=… exit=… line marks the same moment, but it is written before the verdict is routed, so its status= can still be revised: a reviewer that exits cleanly without a successful terminal event logs end status=ok and is then reported as error on stdout. Treat the marker as "the supervisor has stopped", and the JSON line as "here is what happened". In order of preference:

1. Run it in the foreground and read stdout. Correct whenever the host's own command timeout comfortably exceeds --deadline. It often does not: a real cross-model review of a substantial diff runs 7–10 minutes, and some hosts cap a single command well below --deadline's default of 1800s. A capped host will cut the call while the supervisor keeps running — which looks exactly like a hang.
2. Run it with your runtime's background-execution facility, which reports when the process exits. This is the reliable choice on a capped host, and it is what plan-duel already does for the same reason. Do not shorten --deadline to fit a cap instead: that kills legitimate long reviews rather than the hung ones.
3. If you must poll, wait on the status artifact — never on a process pattern. Redirect the status to a file, then poll until it holds parseable JSON (a shell redirect creates the file empty at launch, so existence alone is not completion). Bound the loop, and check the supervisor is still alive, since a supervisor that is signaled before it can write leaves the file empty forever.

Never wait with a command-line pattern match (pgrep -f, pkill -f, ps | grep). Those match full command lines, and an agent harness typically runs each shell command as bash -c '<the entire command text>' — so the pattern you are searching for is present in the polling shell's own argv and matches itself. The loop can never observe its exit condition, and the result is a multi-hour phantom "the review is hung" report about a supervisor that finished cleanly. If you have no alternative, capture the PID at launch and poll kill -0 "$PID": a PID cannot self-match.

Before concluding a review is hung, check its artifacts — the status file, the display log's end marker, the mtime of --findings — not whether a process appears to be alive.

Step 3 — Read the diff for correctness

Go hunk by hunk. For each change, ask what could make it wrong — do not assume it works because it was just written:

• Correctness / logic: wrong conditions, off-by-one, inverted checks, mishandled return values, incorrect API/contract usage.
• Edge cases: empty/null/large inputs, error and failure paths, concurrency and ordering, boundary values.
• Contract & integration: callers/consumers updated to match — open the unchanged files that reference the changed code to confirm this, since an un-updated caller lives outside the diff; interface, schema, or serialization changes are backward-safe; no dangling references after moves.
• Security: untrusted input handling, authz/authn, injection, secret exposure.
• Tests: does new behavior have tests that would actually fail if the behavior regressed? Are the assertions anchored, not silent?
• Consistency: matches surrounding conventions and patterns.

Prefer confirmed, reproducible findings over speculation; when a concern is uncertain, say so and state what would confirm it. Where the change ships with tests you can run, run the one that would confirm a finding before filing it, and say which findings were confirmed that way. Stating a failure scenario and demonstrating one are different claims, and the report cannot be told apart from the outside.

Step 4 — Report findings (no edits)

Emit a findings list, most-severe first. For each finding:

• Severity, by this rubric (a phase gate keys on it, so label accurately):
  • blocker — wrong in a way that breaks correctness, security, or data integrity; must not merge.
  • major — a real correctness or robustness defect that should be fixed before merge, but not catastrophic.
  • minor — a small correctness or clarity issue that is safe to defer.
  • nit — style or preference; no correctness impact.
• Location (file:line).
• What is wrong and the concrete failure it causes (inputs → wrong result).
• A suggested direction — not an applied edit.

If nothing substantive is found, say so plainly rather than inventing nits. Do not modify the working tree under any circumstance; the author or the phase gate applies fixes.

Where the review is dispatched to a rung-1 reviewer, the reviewer also returns this list as a JSON object matching review-schema.json: findings (an array of file / line / severity / summary / failure_scenario), overall, and blocking_count — the number of blocker plus major findings, which is what a phase gate acts on. The prose above is what the object's fields carry, not a second report: write the reasoning, and the same content lands in the structured fields. When the reviewer runs in-harness (rung 2) there is no flag to enforce the shape, so ask for the object in the prompt and fall back to reading the prose list if it does not arrive.

Scope note

The single, bounded rung-1 different-model pass above is a normal part of this skill and runs by default when a different model is available. What stays separate and user-triggered is a heavier review — the whole-tree, blind, multi-agent sweep the review-panel skill runs, or a cloud-based deep audit: recommend that when the change warrants deeper scrutiny, but do not launch it automatically from this skill. Where the review-panel skill is not installed, the recommendation still stands as a sentence to the user — the tree wants a whole-tree sweep and this diff-first pass is not one — and this review is never widened to stand in for it.