# Aicr Cross Review > Multi-agent PR review using Claude Code, Codex, and CodeRabbit. Runs parallel reviews with integration impact analysis, then one cross-review round to a 2-of-3 consensus, with every confirmed finding adversarially verified by a fresh agent. Never runs the reviewed commit's code, and never posts unless explicitly asked. Use when asked for a thorough cross-review or multi-reviewer analysis. Requires the Codex plugin; CodeRabbit is best-effort. Claude Code only — uses the Workflow and Agent tools, which are not available in other agents. - Skill: `nvidia/aicr-cross-review` (Agent Skill, multi-file: 2 files) - Install (CLI): `npx skillmds@latest add nvidia/aicr-cross-review` - Raw SKILL.md: https://api.skillmd.com/api/skills/nvidia/aicr-cross-review/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: AI & ML - Author: NVIDIA (https://skillmd.com/u/nvidia) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/nvidia/aicr-cross-review --- # AICR Cross-Review: Multi-Agent PR Review with Consensus Three reviewers (Claude Code, Codex, CodeRabbit) plus a targeted integration impact analysis, cross-reviewed to 2-of-3 consensus, with every confirmed finding adversarially verified by a fresh agent. Orchestration runs as a **Workflow** (`scripts/workflow.mjs`). **Claude Code only.** If the `Workflow` tool is unavailable, stop and say why — do not fall back to another review command. `/code-review` in particular posts its result to the PR (see Phase 2), which this skill never does without an explicit request. Named `aicr-cross-review` so it does not shadow a contributor's global `cross-review` skill. **When in doubt, stop.** Every check below either passes or ends the review with an explanation. The skill never executes the reviewed commit's code, and never posts to the PR unless you explicitly ask (Phase 5). ## Input Raw arguments: `$ARGUMENTS` `$ARGUMENTS` must be a PR number or a URL; both are normalized in Phase 0. There is no no-argument mode: a fork PR cannot be found from the local branch name alone, since the branch lives on the contributor's fork while the PR lives on `NVIDIA/aicr`. Stop and ask for a PR reference rather than guessing. Do not write a parser; `gh` accepts both forms. ## Phase 0: Pre-flight Only the required lanes are hard requirements. Claude, Codex and integration analysis must work — if one fails at runtime the review reports `incomplete` and stops. CodeRabbit is best-effort: a missing or unauthenticated CLI is not an error, its vote slot just records `NONE`. ```bash for tool in gh git; do which "$tool" >/dev/null || { echo "$tool not found — install it and retry."; exit 1; } done ls ~/.claude/plugins/cache/openai-codex/codex/*/scripts/codex-companion.mjs >/dev/null 2>&1 \ || { echo "Codex companion not found. Install the Codex plugin (Settings → Extensions → Codex)."; exit 1; } echo "Pre-flight OK." ``` If either check fails, stop and report which tool is missing. Do not fall back to another review command. **Resolve the PR number — before anything else needs it.** Every `gh` call and the ref fetch are scoped to `NVIDIA/aicr` **literally**, written out in each command. Two reasons: in GitHub's standard fork layout the local repository is the contributor's fork, which has neither the PR nor `refs/pull/*`; and a shell variable would not survive anyway, since each Bash call is a fresh shell. ```bash # $ARGUMENTS must be a PR number or URL — gh accepts either. test -n "$ARGUMENTS" || { echo "usage: /aicr-cross-review "; exit 1; } gh pr view "$ARGUMENTS" --repo NVIDIA/aicr \ --json number,title,body,baseRefName,headRefName,headRefOid,files ``` Take `` = `.number` and use that numeric value for every later temp path, scoped ref name and `gh` call — never the raw argument. Keep the rest of the response; Phase 1 does not re-fetch it. **Self-review guard.** From the `files` list just fetched: if any changed path is under `.agents/skills/aicr-cross-review/`, **stop** — the scripts you would execute are the ones under review. Ask for a trusted checkout. This catches the accidental case only; `SKILL.md` lives inside the reviewed repo, so it is not a security boundary. ## Phase 1: Setup **Batch A — one parallel message:** 1. From the Phase 0 response, pin `HEAD_SHA` = `headRefOid`. Every reviewer reviews this exact commit. `` is already resolved in Phase 0; do not re-fetch. 2. Worktree hygiene: `git worktree prune`, then `git worktree list | wc -l`. If the count still exceeds ~15, **stop** and ask the user to clean up before retrying. Do not remove worktrees yourself — a clean detached-HEAD worktree may be another session's active review. (Each worktree adds sandbox deny-list paths; at ~70 the profile exceeded the OS spawn-arg limit and every sandboxed Bash call failed with `E2BIG`. Recovery needs a fresh session.) 3. Reap dead runs' pinned inputs. A session killed between Batch B and Phase 5 leaks its two `refs/cr/*` and its temp diff file permanently — nothing else reclaims them, and they accumulate in the same slow way the worktrees above do. ```bash RUNS="$(git -C "" rev-parse --path-format=absolute --git-common-dir)/cr-runs" find "$RUNS" -maxdepth 1 -type f -mmin +1440 -delete 2>/dev/null || true git -C "" for-each-ref --format='%(refname)' 'refs/cr/pr*' 'refs/cr/base*' | while read -r REF; do KEY=${REF#refs/cr/} # want - case "$KEY" in pr*) KEY=${KEY#pr};; base*) KEY=${KEY#base};; esac case "$KEY" in *-*) ;; *) continue;; esac # must have both components case "${KEY%-*}" in ''|*[!0-9]*) continue;; esac # is a PR number case "${KEY##*-}" in ??????) ;; *) continue;; esac # is mktemp's six chars [ -e "$RUNS/$KEY" ] || git -C "" update-ref -d "$REF" done find "${TMPDIR:-/tmp}" -maxdepth 1 -type f -name 'cross-review-pr*.??????' -mmin +1440 -delete 2>/dev/null || true ``` Liveness comes from the per-run marker Batch B drops in `cr-runs/`, not from the ref and not from the diff file. Both alternatives are broken: - **Not the ref.** `git gc` packs `refs/cr/*` into `packed-refs`, after which the per-ref file under `.git/refs/cr/` no longer exists and an mtime gate silently stops reaping — and `git fetch`, which Batch B runs, triggers `gc --auto`. The two substitutes fail too: `refs/cr/*` has no reflog (`core.logAllRefUpdates` covers only `refs/heads`, `refs/remotes`, `refs/notes`, and `HEAD`), and `%(creatordate)` is the *commit's* date, so a ref created a minute ago on yesterday's `main` reports "21 hours ago". - **Not the diff file.** `TMPDIR` is not stable across sessions, or even within one: under Claude Code's sandbox it is `/tmp/claude-`, and with the sandbox bypassed it is the shell default (`/var/folders/…/T/` on macOS). A reaper that tested for the diff file would miss a live session's file whenever the two disagree and delete that session's pinned refs — the one thing this skill must never do. `cr-runs/` sits next to the refs it guards, under the **common** git dir, so every worktree of a clone shares one view, exactly as `refs/cr/*` are shared. Git ignores unknown entries there, so nothing packs or prunes it and the marker keeps a real creation timestamp. The last `find` is only a temp-file janitor: it reclaims diff files in whatever `TMPDIR` this session sees, and reclaiming none is harmless. **The three `case` guards are the safety boundary, and all three are load-bearing.** A candidate must have both components, a numeric ``, and a six-character `` before it can be deleted. Prefix-stripping alone is not enough: `refs/cr/pr*` also matches `refs/cr/private-ABC123`, which strips to `ivate-ABC123` and would pass a suffix-only check. Hand-made bookmarks (`refs/cr/2183-r5`, `refs/cr/2187-test`) are deliberate, often pin active work, and must survive — the only names that now collide are a literal `pr-` or `base-`, since the loop accepts both prefixes, so do not use either shape for one. `find … -delete` rather than `rm` for the same reason as everywhere else in this skill: managed permission policies gate `rm:*` behind a prompt. The janitor is `-type f` so a directory that happens to match the diff-file pattern is never removed. **The marker key is `-`, never `` alone.** `mktemp` guarantees the full filename it returns is unique; it does not reserve the suffix. Two concurrent reviews of *different* PRs pass different templates, so both can be handed the same six characters — their refs stay distinct, but a ``-keyed marker would be one shared file, and whichever run finished first would delete the other's protection. Reviews of the *same* PR are safe whenever they share a `TMPDIR`, since `mktemp` guarantees distinct names within one directory. It guarantees nothing across directories, so same-PR runs under different `TMPDIR` roots can still collide — but on `$SID` itself, which makes `PRREF` and `BASEREF` collide first. That is a property of how Batch B derives `$SID`, not of this reaper, and is left to a follow-up. **The gate is 24 hours, and it must stay far above any real run.** Nothing enforces an end-to-end limit on a review: Codex gets a five-wait, ~45-minute budget in the Review phase and again in Cross-review, with Verify on top. A gate near the expected duration would let a later Phase 1 reap a *live* run's marker, then its refs, and the temp-file janitor would take its `DIFFPATH` with them — the run would destroy itself. The gate measures **age since Batch B stamped the marker, not inactivity** — the marker is written once and never refreshed — so a run still alive a day later is outside every documented budget and is treated as dead. The temp-file janitor is gated independently, on each diff file's own mtime, so refreshing the marker would not cover `DIFFPATH` either way. Since this reaper exists for leaks that accumulate over days, waiting a day to collect one costs nothing. Do not tune it down toward the expected runtime. **Batch B — after A** (needs `HEAD_SHA` and `baseRefName`). `gh pr diff` takes no SHA argument, so pin the diff with `git fetch`. Refs and the diff file are **session-scoped**: two sessions reviewing the same PR must not share, overwrite, or delete each other's pinned input. ```bash set -euo pipefail # a failed fetch or diff must abort, not leave an empty diff file BASE="" # from step 1 — never hardcode "main" DIFFPATH=$(mktemp "${TMPDIR:-/tmp}/cross-review-pr.XXXXXX") # must end in X on macOS SID=${DIFFPATH##*.} # reuse mktemp's unique suffix to scope the refs PRREF="refs/cr/pr-$SID"; BASEREF="refs/cr/base-$SID" # Liveness marker for Batch A step 3's reaper, written BEFORE the refs exist so no # concurrent reaper can ever see a ref without its marker. Keyed by -$SID — mktemp # guarantees the full filename is unique, not the suffix, so a $SID-only key would # collide with a concurrent review of a DIFFERENT PR. Under the common git dir, so # every worktree of this clone shares one view. RUNS="$(git -C "" rev-parse --path-format=absolute --git-common-dir)/cr-runs" RUNMARK="$RUNS/-$SID"; mkdir -p "$RUNS"; : > "$RUNMARK" # Fetch from the canonical repo by URL, not from `origin`: in GitHub's standard fork # layout `origin` is the contributor's fork, and refs/pull/* exist only on the canonical # repository. git -C "" fetch "https://github.com/NVIDIA/aicr.git" \ "+refs/pull//head:$PRREF" "+refs/heads/$BASE:$BASEREF" # Head moved → stop. Clean the refs we just created before exiting; set -e would # otherwise abort before the names are ever printed, leaving them unreclaimable. if [ "$(git -C "" rev-parse "$PRREF")" != "" ]; then git -C "" update-ref -d "$PRREF"; git -C "" update-ref -d "$BASEREF" find "$DIFFPATH" "$RUNMARK" -maxdepth 0 -delete echo "HEAD moved since setup — restart the review"; exit 1 fi # Echo the names FIRST: under `set -e` an empty or failing diff aborts, and any # echo below it would never run — leaking the refs and the temp file with a random # suffix nobody recorded, which Phase 5 then cannot clean up. echo "DIFFPATH=$DIFFPATH"; echo "PRREF=$PRREF"; echo "BASEREF=$BASEREF"; echo "RUNMARK=$RUNMARK" git -C "" diff "$BASEREF...$PRREF" > "$DIFFPATH" test -s "$DIFFPATH" # a real PR diff is never empty # repoNotes source, pinned to the BASE ref — a fork PR must not be able to rewrite # the instructions fed to the reviewer. Absent on some repos; that is fine. git -C "" show "$BASEREF":.claude/CLAUDE.md 2>/dev/null || echo "(no tracked CLAUDE.md)" # BASE_SHA is the base branch tip. Its only consumer is CodeRabbit's --base-commit, # and the CLI resolves the merge-base itself, so this stays consistent with the # three-dot diff above without a second baseline to keep in sync. echo "BASE_SHA=$(git -C "" rev-parse "$BASEREF")" ``` Capture `DIFFPATH`, `BASE_SHA`, `PRREF`, `BASEREF`, `RUNMARK` — shell variables do not persist between Bash calls and Phase 5 needs the ref names. Then build `repoNotes` for the Claude reviewer only (never fed to Codex — lean-context rule): distill the base-pinned `CLAUDE.md` plus the local overlay into 3–6 lines of the rules most likely to catch defects in the changed paths. The check below reduces accidental exposure, but it is **not a trust boundary**: reviewer subagents load the checkout's `CLAUDE.md` hierarchy automatically, before any guard here runs. Treat `repoNotes` as a relevance digest, not a sanitiser. **For an untrusted or fork PR, run this skill from a session started in a trusted checkout** — the same operational remedy as the self-review guard in Phase 0. Git overwrites *ignored* files during checkout without complaint, so checking out a fork that force-added an ignored overlay silently replaces yours. ```bash for f in AGENTS.local.md CLAUDE.local.md; do [ -e "/$f" ] || continue # Skip symlinks first. The tracked-status check applies to the link, not its target, # so an untracked symlink pointing at a PR-tracked file would otherwise be reported # TRUSTED while resolving to PR-controlled instructions. [ -L "/$f" ] && { echo "SKIP $f — symlink"; continue; } if git -C "" ls-files --error-unmatch -- "$f" >/dev/null 2>&1; then echo "SKIP $f — tracked by this PR, not a trusted local overlay" else echo "TRUSTED $f" # regular untracked file: safe to read fi done ``` Read only the paths reported `TRUSTED`. `AGENTS.local.md` is normally a symlink to `CLAUDE.local.md`, so it is skipped and the overlay is read through the real file — no content is lost. **Verify the workflow script version before Phase 2.** The script about to be passed as `scriptPath` must contain the sentinel identifier `codexResumeJobId`: `grep -c codexResumeJobId "/scripts/workflow.mjs"` — expect a non-zero count. If it is absent, STOP: the file is a stale or reverted copy, and running it silently restores the old semantics (observed live: a concurrent session's git operation reverted uncommitted skill files in a shared checkout, and a full review round ran the old script unnoticed). The sentinel detects staleness relative to this revision only — if that identifier is ever renamed, update this check in the same change. ## Phase 1.5: Classify and extract the change list **Classify** the PR: `code-change` | `adr` | `config-change` | `documentation-only`. **Extract a bounded change list** so integration analysis verifies specific items instead of fishing across the repo: - Exported functions/types/constants added, removed, or modified - Config keys added or changed (`.yaml`, `.toml`, `.json`) - Workflow inputs/triggers added or changed - File/manifest paths renamed or restructured - Behaviorally significant defaults changed (timeouts, versions, namespaces) > **This skill never runs the PR's code.** No build, test, or coverage step; every > reviewer prompt forbids it. Only trusted tools run (`git`, `gh`, the CodeRabbit CLI, > the Codex companion). Coverage is CI's job — see Phase 3. ## Phase 2: Run the review workflow ``` Workflow({ scriptPath: "/scripts/workflow.mjs", args: { pr: , repo: "/", repoPath: "", headSha: "", baseSha: "", diffPath: "", prType: "", changeList: ["", ""], repoNotes: "<3-6 line digest, optional>" } }) ``` Pass `changeList` as a real JSON array, not a stringified one. Every lane is `general-purpose` and inherits the session model, so there is no model argument to pass. **What the workflow does** (`scripts/workflow.mjs` is the single source of truth for the consensus mechanics): - **Review** — Claude Code (reviews the pinned diff directly; it deliberately does *not* delegate to the `code-review` command, whose step 8 instructs its agent to `gh pr comment` the result back to the PR), Codex (two chained agents: a dispatch agent starts the remote background job and hands back its id, which the workflow immediately writes to the progress log — `Codex job dispatched — review running remotely` — then a wait agent runs a 9-min bounded wait plus up to four continuation waits when the job is still running — about 45 min for a live job), CodeRabbit (CLI against a detached worktree at `HEAD_SHA`, explicit 600000 ms timeout — the Bash tool caps any single call at 10 minutes, which is why Codex exceeds it by waiting across several calls rather than waiting longer), and integration analysis (bounded to `changeList`). Every lane is a `general-purpose` agent. All parallel, schema-validated, and none may execute the reviewed commit's code. - **Merge** — dedupe by `path:line:normalized-summary:consumerPath:consumerLine`; duplicates merge to the highest severity and union their sources; a finding citing a file the reporter never listed in `filesChecked` is flagged for extra scrutiny. **Two lanes wording one defect differently stay separate candidates, by design.** Keying on location alone was tried and reverted: it did merge those duplicates, but the evaluation schema permits exactly one verdict per candidate id, so a merged pair of *distinct* same-line defects has no correct verdict — confirming the real one also confirms the false one, and refuting the false one dismisses the real one. Retaining both summaries prevented data loss but not mis-adjudication, which is the worse failure. Instead, candidates sharing a location — `path:line` **and** the same `consumerPath`/`consumerLine` — are **flagged** as possible duplicates. The consumer half matters: one changed declaration breaking two callers is deliberately two candidates, and hinting that they might be duplicates would push reviewers to collapse a distinction the key exists to preserve. The flag reaches the cross-review candidate list and the refuter prompt, so reviewers decide whether the two are one defect and evaluate them consistently. Equivalence stays an explicit judgement rather than an assumption from a shared line number. Merging also stops once candidates are presented: a late finding that merged into an already-evaluated id would inherit votes cast before it existed. Late findings always become their own candidate and, being unpresented, stay contested for the human. - **Cross-review (one round, Claude + Codex only)** — each re-reviews independently first (anti-anchoring), then returns AGREE/DISAGREE/OPEN_QUESTION per candidate. CodeRabbit does *not* take part: its CLI is a slow blocking cloud call and it reviews Git changes generically, so it cannot adjudicate our candidate ids and a second run over the same commit adds no signal. Its round-1 findings still stand as its AGREE votes, so it can still corroborate a split it independently reported. Anything still split afterwards is reported as contested for you to settle. - **Consensus rule** — confirmed = 2 of the 3 reviewer slots AGREE **with evidence**; integration analysis is never a reviewer slot. A round-1 finding whose evidence is blank or whitespace-only is dropped at intake, so it never registers its reporter as a source. In the cross-review round an unevidenced AGREE/DISAGREE instead aborts the run (`incomplete`) — dropping it would leave the reviewer's round-1 source vote to decide the tally. - **Verify** — every confirmed finding goes to a fresh adversarial refuter (REFUTED → dismissed; UNVERIFIABLE, no result, or a verdict without a citation → the `unresolved` array). `consensusReached` is true only when both `contested` and `unresolved` are empty — a finding that reached consensus but failed verification is an open question, not a settled one. **Read `adjudication` on every contested entry.** The bucket holds two different states and they need different things from you. `evaluated` means the finding was presented, the reviewers voted, and they did not reach 2-of-3 — a genuine split, so break the tie. `raised-late` means it was raised *during* the cross-review round, after candidates were presented, so nobody cross-evaluated it and its only position is its reporter's — it just needs reading. Measured on a real run: 8 of 8 contested findings were `raised-late`, each with a single AGREE and NONE elsewhere, so the count read as eight disagreements when there were none. `consensusReached` counts both, deliberately: a late finding is unadjudicated, and letting it report consensus would be the same overstatement this skill exists to avoid. - **Report incomplete and stop** — Claude, Codex and integration analysis are required in round 1, and Claude and Codex must each return exactly one evaluation per candidate in the cross-review round. A missing lane, a missing evaluation, a duplicate, or an unknown candidate id returns `status: "incomplete"` with the reason and raw unverified findings. There is no degraded-consensus mode. CodeRabbit is the only best-effort lane: when it does not run, its vote slot records `NONE`, which raises the bar (Claude and Codex must then agree) rather than lowering it. One deliberate exception, at the level of a **finding** rather than a lane. An integration finding claims a specific consumer breaks, so one lacking `consumerPath`/`consumerLine` cannot be verified *as an integration claim* — but if it still locates a defect (its own path/line) with evidence, it is a perfectly reviewable ordinary finding. It is therefore **demoted** — consumer fields stripped, flagged in the candidate list, excluded from the integration severity escalation — rather than dropped, with a `log()` naming what was demoted. Dropping was tried twice and cost a whole run each time: first when the lane returned several findings and one legitimately consumer-less observation failed the run; then, after per-finding dropping replaced that, when the lane's ONLY finding was such an observation and the zero-survivor rule stopped the run with all four lanes `ok` and no report produced (observed on PR 2097). The run still stops when every integration finding lacks even a locatable defect or evidence — that is the case the fail-closed rule exists for: silently dropping the lane's only finding once yielded `consensusReached: true` while a required lane had contributed nothing. "Contributed nothing" is measured on what survives `intake()`: a demoted finding passes through the same coordinate and evidence gates as every other candidate, so a consumer-less, whitespace-evidence finding cannot slip through demotion into a false-clean. One rule covers the stop: a non-empty integration result that yields no accepted finding — neither as an integration claim nor as a demoted ordinary finding — stops the run, and the message says how many went for each reason. **Coordinates are validated by a single shared rule**, `hasCoords`, applied to a finding's own `path`/`line` in `intake()` — every lane, not just integration — and to `consumerPath`/`consumerLine` for the integration pair. The response schema **requires** `path` and `line` and leaves `consumerPath`/`consumerLine` optional — deliberately, since only integration findings carry a consumer — but it constrains none of the four, so `""`, `" "`, `0` and `-1` all satisfy it and all passed a truthiness/null check. Tightening the schema instead would fail a whole lane on one bad field, which is the all-or-nothing behavior this section exists to remove. It is one helper rather than two call-site conditions for a specific reason: every earlier version of this guard fixed the pair it was shown and left the other, and a shared rule is what stops the next field pair from repeating that. **The zero-survivor rule applies to every required round-1 batch**, not just integration. Claude's and Codex's round-1 findings go through `intakeBatch`, which reports what survived. A required lane whose round-1 findings are *all* malformed contributed nothing, and letting them vanish silently is the same false-clean one lane over. Mixed batches still proceed. CodeRabbit is exempt: a total loss there records `NONE` and raises the bar, exactly as a lane that never ran. Rejection reasons are counted separately (`unlocatable` vs `unevidenced`) rather than inferred from a subtraction, so the message names the defect the reader should go looking for. **It deliberately does NOT apply to cross-review `newFindings`.** Those go through the same `intakeBatch` gate — a malformed late finding still never enters the tally — but a total loss there is not fatal. The rule tests "this lane contributed nothing", which round 1 can assert because findings are the lane's whole output. In the cross-review round the lane's output is its *evaluations*, and the completeness gate has already returned `incomplete` unless the lane evaluated every presented candidate; `newFindings` are supplementary and volunteered. Aborting there would throw away a full set of adjudications and the verification round over one imprecise extra finding — the same disproportionate total loss seen on PR 1908, one round over. Malformed late findings are dropped individually and each drop is logged with its reason. **Operational notes:** - The workflow runs in the background — wait for its completion notification. - If it dies mid-run, **resume, don't restart**: `Workflow({scriptPath: ..., resumeFromRunId: ""})` — completed lanes replay from cache. Empty or odd result → read `/journal.jsonl` first. - **The Codex round-1 lane is two agents, deliberately.** A dispatch agent composes the lean Codex task, starts the background job (and owns the fast-transient retry-once rule, decided inside a brief ~90-second launch watch that exactly covers the under-60s retry window), and returns `{jobId, dispatchNote}`; the workflow then logs `Codex job dispatched — review running remotely` and hands the id to a wait agent that runs the continuation-wait protocol unchanged and translates the result. The split exists for progress visibility: a single opaque agent call shows "running" from spawn, which cannot distinguish "remote job dispatched and working" from "dispatch never happened" — a real run sat silent for 19 minutes with no way to tell which. The logged job id is the visible "started" signal, and it doubles as the recovery handle when everything after dispatch dies: a dispatch-agent failure or a wait-agent loss surfaces exactly like any Codex-lane unavailability (`incomplete`, with `codexJobId` whenever a live job id exists). The wait agent never dispatches. - The Codex lane fails in three distinct ways, and the dispatch and wait protocols treat them differently: - **Lookup miss** — the status call exits 1 with empty stdout and `No job found` on stderr. Companion state is keyed by workspace root and each Bash call is a fresh shell, so an unpinned lookup resolves to a different workspace and reports a live job as unknown; the miss is **not** evidence the job died. **Always recheck exactly once** with `--cwd` pinned, whether or not the missing call already carried it — two causes produce the identical message and only one is settled by adding the flag. The other is transient: in companion v1.0.2 `saveState` writes `state.json` with a plain `fs.writeFileSync` (truncate-then-write, no temp-and-rename) while `loadState` wraps `JSON.parse` in a bare `catch` returning the **default** state, whose `jobs` list is empty. A read landing inside that write window yields a well-formed `No job found` rather than an error, and the background worker is rewriting that file precisely while the status call runs. So an identical repeat need not return an identical answer. Once a pinned recheck has also missed, return `unavailable` saying the job could not be located — never re-dispatch (the original may still be running) and never record it as exhausted budget. - **Fast transient failure** — retried **once**, in the dispatch agent's launch watch (the wait agent never dispatches), and only when all three hold: `.job.status` is `failed` (never `cancelled`), the job died in **under 60 seconds** by its own timestamps, and the error names a known-retryable cause such as an upstream capacity rejection (`Selected model is at capacity`) or a transient dispatch fault. The threshold is a number rather than a judgment because the observed cases are far apart — a capacity rejection at ~10s against a genuine timeout at 10m19s — and an undefined "quickly" is how a one-retry budget erodes. A retry then costs seconds and these clear on their own. **`cancelled` is never retried** — the companion emits it only for explicit cancellation, so a retry would restart work someone deliberately stopped. Nor is a late failure: `.waitTimedOut` false only means the job became terminal before the inner deadline, which a failure at 8:59 also satisfies while having burned the whole window. An unrecognised error is not assumed retryable either. - **Wait elapsed, job still alive** (`.waitTimedOut` true with `.job.status` still `queued`/`running`) — not the end of the lane. The job is dispatched in the background and outlives the Bash call waiting on it, so it needs more **time**, not another attempt — and the protocol now gives it exactly that: **up to four continuation waits** on the *same* job id in fresh calls. Those are not retries; nothing is re-dispatched and no work is duplicated. A fifth `.waitTimedOut` is then genuinely exhausted budget, and the lane returns `unavailable` with the job id in the structured `jobId` field — required, not prose — so the result can be fetched or the run resumed later. Measured on a real run: the lane timed out at the full 540000 ms while the job was demonstrably mid-work, the job was **still** `running` long after the review was abandoned, and its result stayed retrievable — so reporting exhausted budget there discarded a required lane, and the whole review with it, over a job that had merely not finished. - **Exhausted budget** — a fifth `.waitTimedOut`, or no parseable JSON at all because the outer timeout killed the call (a dead broker). No retry, no further wait. The ceiling is not tunable — the wait runs inside a Bash call, and that tool silently kills any foreground command at 600000 ms. Exceeding 10 minutes requires polling across several calls, which is exactly what the continuation wait above does: a live job gets five waits, roughly forty-five minutes, without a longer single call. The budget was three waits until a real ~53-minute job on a +1951/−153 PR outlasted even that — hence five waits, and a resumable `jobId` on exhaustion instead of lost work. The inner wait is therefore 540000 ms, deliberately **below** the outer cap: were the two equal, Bash could kill the command before it printed its JSON, leaving no `.waitTimedOut` to classify on. That is why an unclassifiable kill counts as exhausted-budget rather than a fast failure — guessing wrong there costs another full window for nothing. The lane still reports the job id it was waiting on (`jobId`), so even that kill stays resumable. - Codex is required, so a lane that is still unavailable after its retry makes the run report `incomplete`; re-run rather than interpreting a partial result. - **`incomplete` with a `codexJobId` usually means the review is NOT lost.** That field is the Codex job the run was waiting on, surfaced top-level (alongside `reviewerStatus`) precisely so recovery is mechanical, not improvised. The lane attaches it to every unavailable wait result deliberately — losing a live id costs a whole review, a wasted resume costs minutes — so it can also reference a job that already failed or was cancelled: a poll that shows a terminal non-completed state, or a resume that comes back unavailable again, confirms the job is dead and the review must be re-run rather than resumed. For a live job: poll it with the companion status command until it is terminal (a background 60-second loop is fine — polling is cheap once the workflow is no longer holding a lane open for it). Resolve the companion the same way the lane does, with `-t` — the lane's messages refer to `$comp` but cannot export it across Bash calls: ```bash comp=$(ls -t ~/.claude/plugins/cache/openai-codex/codex/*/scripts/codex-companion.mjs | head -1) node "$comp" status --cwd "" --json ``` `ls -t` picks the most recently installed companion, which is the version the plugin system has active. Dropping the `-t` sorts by version *name* instead and can select a stale cached copy — polling a job with a different companion version than dispatched it returns misleading results (`status` finding a job that `result` then reports as unknown), which reads exactly like a dead job and is not one. Then resume: `Workflow({scriptPath, resumeFromRunId: "", args: {...prevArgs, codexResumeJobId: ""}})` — `prevArgs` is the previous run's args object, unchanged. The three completed lanes replay from cache, the Codex dispatch agent is skipped entirely (the workflow logs `Codex resume: waiting on existing job `), the wait agent collects the existing job without dispatching a second one, and the run proceeds to cross-review and verification normally. A run interrupted *between* dispatch and result needs no `codexResumeJobId` at all: on `resumeFromRunId` the dispatch agent's cached `{jobId}` replays instantly, so the wait prompt is byte-identical to the original run's and the resume lands in the same wait with no re-dispatch. Proven live on PR 2097: a ~53-minute job outlasted the then-three-wait budget, and the resumed run recovered it with zero re-dispatched work. - **A dead job from broker teardown needs a private-broker resume, not a plain one.** When a Codex lane reports the job killed by concurrent-session broker teardown (`statusNote` names infra-kill-by-concurrent-session-teardown — the confirmed `sessionRuntime` fingerprint, which W2 and W4 both run; a late `failed` with a reaper/null error is a symptom, not the gate, and an UNCONFIRMED cause gets an ordinary re-run under the shared broker instead), the job is terminal — there is nothing to poll and `codexResumeJobId` does not apply. A plain `resumeFromRunId` does not help either: the lane *completed* with its unavailable result, so the cache replays the failure verbatim. And a re-dispatch under the same shared broker (keyed to the repo path) faces the same teardown risk while the concurrent sessions that caused it are still running. The remedy: copy `scripts/workflow.mjs` to a scratch path (never edit the checked-in file), add a one-line nonce to the affected lane's prompt, and in the scratch copy replace `${repoPath}` with the session-private worktree path in that lane's `--cwd` interpolations — every companion command: dispatch, status, result. Edit the interpolation itself, not an appended prompt override: the generated prompt's literal commands pin `--cwd "${repoPath}"` and insist on it for every call, so an override that contradicts them may lose, and any call that keeps the shared path lands the recovered job back under the broker being torn down. Then `Workflow({scriptPath: "", resumeFromRunId: "", args: prevArgs})`. Every other lane replays from cache; only the edited lane re-runs, and its job lives under a private workspace broker that no concurrent session's SessionEnd hook will tear down. The task prompt's pinned `git -C` reads still name the original repo path, so the review context is unchanged. Proven live 2026-08-08 on PR 2097: two evaluation jobs died to teardown under the shared broker; the third, dispatched under a private broker, completed and the run reached consensus with zero re-reviewed lanes. - **Execute the CodeRabbit lane's STEP blocks verbatim** — same commands and paths, no substitutions; in particular never swap the `find … -delete` cleanup for `rm` (an invented `rm -rf` cleanup once blocked a run for hours on a managed-policy confirmation prompt). The no-`rm` rule covers **every** command composed in the lane, ad-hoc diagnostics included — a self-written `.git/worktrees` writability probe with `rm -f` cleanup once blocked a round the same way. `rmdir` for empty dirs, `find -maxdepth 0 -delete` for files, always. - CodeRabbit slow runs: check the newest file in `~/.coderabbit/logs/` (429/queue lines mean cloud-side queueing) and confirm `which -a coderabbit` resolves to the brew-managed binary — a stale `~/.local/bin` copy shadows it. - **A sandboxed CodeRabbit run hangs instead of failing.** The sandbox can deny the CLI two independent ways, and both stall at `connecting_to_review_service` until the timebox kills it: `~/.coderabbit` outside the write allowlist (the CLI cannot create its log or review store), or the `coderabbit.ai` hosts outside the allowed-hosts list. The lane therefore **probes both** in step 1 — writability of `~/.coderabbit` *and* reachability of both CLI hosts, `cli.coderabbit.ai` (startup config fetch) and `ide.coderabbit.ai` (the review session's WebSocket) — and runs the `coderabbit` command (and only that command) with sandbox bypass when any check fails. The hosts are probed separately because the allowlist is per-host: an entry naming only the config host passes the first check and still hangs step 2 on the WebSocket connect. **Why probe-gated bypass rather than an allowlist assumption.** Adding `~/.coderabbit` to the sandbox write allowlist is the narrower grant, but this skill is checked into the repo and has to work on a contributor's machine as written: an allowlist entry lives in each person's local settings, so a lane that *assumed* it would hand anyone who has not made the edit the silent ten-minute hang above rather than a usable lane. And the hang means "try sandboxed, fall back on failure" without a probe costs a full timebox per wrong guess. The step-1 probe settles it in milliseconds: machines with the allowlist entry run step 2 fully sandboxed and pay **no bypass prompt at all**; every other machine gets the bypass from the start, portable and self-documenting at the call site. If you run this often, add **both** grants — `~/.coderabbit` (and `~/.claude/plugins/data`, for the Codex companion's job log) to your local sandbox `filesystem.allowWrite`, *and* the `coderabbit.ai` hosts to the network allowlist — since that pair is what removes the per-round approval prompts. The skill must not depend on either. Granting only the filesystem half is worse than granting neither: it satisfies the write probe while the network stays blocked, and an earlier writability-only probe read that state as sandbox-clean and hung the lane for a full ten-minute timebox. The probe now tests both for exactly this reason. **Diagnose it from the log directory**, since the two denials differ there. A stall at `connecting` *with no new file in `~/.coderabbit/logs/`* is **filesyst …(truncated)