Review Pr

Deep PR Review (review-pr)

Runs this repo's review agents against a remote PR in nrwl/nx. The PR is checked out inside an isolated sandbox (gVisor on Linux, the Docker VM on macOS), the agents are dispatched with the PR's scope passed to them explicitly (Step 5), and their output is collected into a draft suitable for posting on GitHub.

Drafts only. This skill never posts to GitHub. The draft is reading material for the reviewer; if they want any of it on the PR, they post it themselves (or ask in the session, e.g. via gh pr review --body-file).

Trust model — why the sandbox

A PR is untrusted code. The dividing line is execution, not reading: the host may freely read public PR/issue information, but must never run PR-authored code (install scripts, builds, tests, the linked-issue reproduction). This skill enforces that with a strict split:

• Host (Claude + its credentials): reads GitHub metadata and the diff (gh pr view / gh pr diff / gh issue view), orchestrates the agents, and reads the checked-out code only through .claude/tools/sandbox read/grep/find. Claude's auth token never enters the sandbox.
• The sandbox: holds the PR checkout and is the only place any PR code executes — dependency installs, builds, tests, and the issue reproduction all run via sandbox exec.

The CLI owns isolation, and nothing above it names a runtime. sandbox start probes the available backends, picks the boundary (gVisor on Linux, the VM on macOS), and refuses to start at all when it cannot get a real one. This is the one thing that used to be a variable here, and its failure mode was "no isolation, reported as success" — an unset RUNTIME_FLAG expanded to nothing, which is byte-identical to the correct macOS value. Do not reintroduce a runtime flag anywhere in this skill.

Consequences that the rest of this skill depends on:

• Never check the PR out into the host working tree. The checkout lives only inside the sandbox and is destroyed by sandbox stop.
• The review agents cannot use native Read/Grep/Glob for PR source (those only see the host FS). They read it through the CLI, which presents identical commands whether the checkout is isolated or local — so no agent is ever told a native source read is an option. Read is still fine for host-side files this skill writes (the charter, the dumped diff).
• If you ever catch yourself about to run npm/pnpm/nx/a test/the repro on the host, stop — route it through sandbox exec instead. See Step 3.

Inputs

• <NUMBER> — the PR number in nrwl/nx. Required.

Configuration (env-overridable)

• SANDBOX_IMAGE — the toolchain image the checkout runs in. Default: nx-review-sandbox:latest (built by the setup-review-sandbox skill). Claude runs on the host, not in this image.
• SANDBOX — the sandbox id, returned by sandbox start in Step 3. There is no default and no name to guess: it is minted per run.
• TRIAGE_DIR — where drafts live. Default: ~/.nx-pr-reviews (outside the repo — so git clean never touches drafts and re-review history survives — and outside ~/.claude, so the skill never writes into Claude Code's own config dir)
• REVIEW_NONINTERACTIVE — set by headless callers (review-prs, the review cron) to skip Step 8.5's grill. Unset in a normal session. Default: unset.
• NX_REPO_PATH — path to the local clone of nrwl/nx this skill ships inside. Default: git rev-parse --show-toplevel. Used only by the Step 4.5 close-signal checks, which may run before the sandbox exists, and always with a fresh git fetch first. It is never used for the PR checkout and is never passed to an agent — agents read base state with sandbox read --ref base, which is fetched fresh every run and cannot be stale.

Step 1: Pre-flight

gh auth status
mkdir -p "$TRIAGE_DIR"

# Probe the backends and report what is usable. This subsumes the old uname/docker
# info/runsc checks: the CLI owns backend selection, so asking it is the only
# answer that matches what `sandbox start` will actually do.
.claude/tools/sandbox doctor

# Bring the image up to date. Do NOT probe whether it exists and skip on a hit: an image
# built from ANY older revision passes an existence check identically, so a missing
# capability is invisible and shows up only as a review that is slower or quietly weaker.
# Observed: an image predating the pnpm-store warming went unnoticed for two weeks and cost
# ~25 min of package downloads on every review.
#
# Just build. Docker's layer cache makes this the right default rather than an expensive one:
#   - nothing changed        -> ~0.6 s, every layer cached (measured)
#   - Dockerfile/mise.toml   -> rebuilds from the changed instruction
#   - pnpm-lock.yaml moved   -> re-runs `pnpm fetch`, which is the point: it keeps the warm
#                               store matching the lockfile reviews actually install from
# Concurrent runs are safe with no lock of our own — review-prs drives up to five parallel
# `/review-pr` panes, and BuildKit deduplicates identical concurrent builds (measured: a 20 s
# step ran ONCE across 5 simultaneous builds, all finishing in ~21 s rather than 100 s).
bash "$(git rev-parse --show-toplevel)/tools/review-sandbox/build-image.sh"

doctor reports each backend and whether it can isolate. You do not act on the detail and you never pass a runtime flag anywhere — sandbox start re-derives it and refuses if it cannot get a real boundary. Read doctor only to give the user a useful message before that refusal happens.

Fail fast with a clear message if: gh isn't authed; doctor reports no usable backend; or the image build fails. For the last two, point the user at the setup-review-sandbox skill — it installs Docker + gVisor, which the build above deliberately does not.

The build prints one line on the fast path (sandbox image up to date), so a slow first run after a lockfile change is expected and self-explanatory rather than a mystery.

Step 2: Fetch the PR metadata

gh pr view <NUMBER> \
  --repo nrwl/nx \
  --json number,title,author,headRefOid,headRefName,baseRefName,url,isDraft,additions,deletions,changedFiles \
  > /tmp/pr-<NUMBER>.json

Parse out:

• title, author.login, headRefOid (the head SHA), headRefName, baseRefName, url
• isDraft — if true, exit early (don't review drafts)
• Local dedup: if $TRIAGE_DIR/<NUMBER>.md exists, its frontmatter head_sha equals headRefOid, its pipeline_version equals the current PIPELINE_VERSION (see below), and its verdict is not failed, this PR was already reviewed at this commit — exit with no draft change; log "ALREADY_REVIEWED". A failed draft never blocks a retry. To deliberately re-review an unchanged PR, delete the draft file or just say so in the session.
• PIPELINE_VERSION: 9 — the current review-criteria generation. A draft whose frontmatter has an older pipeline_version (or none) was produced by a weaker pipeline: re-review even at an unchanged head_sha, treating the old draft as a prior review (Step 4). Bump this constant whenever the review criteria change materially (new agents, new calibrations, new required sections) so stale drafts age out instead of being pinned forever by the SHA dedup.

Fetch the tracking ticket

Much of the work in this repo is tracked in Linear, not in GitHub issues. A PR whose only reference is NXC-1234 is not an unlinked PR — it is a PR whose bug report lives somewhere you have to go and read. Treating "no Fixes #N" as "no grounding available" throws away the problem statement, the acceptance criteria, and usually the reproduction, and it silently degrades the reproduce-verifier to guessing from the PR body.

Extract every NXC-\d+ from the PR body and commit messages (also accept a linear.app/... link), then fetch each one:

mcp__plugin_linear_linear__get_issue with id "NXC-1234"

Also pull its comments when the description is thin — a repro often arrives in a follow-up comment rather than the original report.

From each ticket, keep:

• The problem statement — what is broken, for whom, under what conditions.
• The reproduction, if it has one. This is the highest-value field on the ticket: it is what Step 5a.5's Level 1 should actually run, and it is usually more precise than anything reconstructable from the diff.
• Acceptance criteria / definition of done, if stated.

Then classify the reproduction once, here, and carry it to Step 5a.5 as REPRO_CLASSIFICATION:

• RUNNABLE — the ticket (or a linked GitHub issue) carries a concrete command or a repro repo.
• MANUAL_ONLY — the trigger needs a live second Nx process, an interactive terminal, a real connected workspace, or network the sandbox lacks.
• NONE — no ticket, tracker unreachable, or the ticket has no reproduction.

Deriving it here rather than in the agent is the point: it is one read of material you already have open, and the verifier otherwise spends its opening tool calls rediscovering the same answer.

Fails open. No Linear tools configured, not authenticated (headless and cron runs often are neither), or the ticket is unreadable ⇒ continue exactly as before and note it. Never block a review on the tracker.

Two boundaries, both load-bearing:

• Internal content never reaches $REVIEW_BODY. nrwl/nx is public and tickets routinely carry customer names, embargoed detail, and internal planning. The ticket informs what you check; anything in the posted draft must stand on public evidence — the diff, the PR body, a linked GitHub issue, the repo's docs, or something this review executed. Same rule Step 5c applies to Polygraph sessions, and for the same reason.
• Carry the problem, not the verdict. The bug report and its reproduction are grounding, and every agent may have them. A maintainer's comment concluding "the right fix is X" is a rationale, and it belongs with the Polygraph session in Step 5c — handing it to alternative-approach up front is what destroys that agent's independence.

Step 3: Check the PR out inside the sandbox

Start a long-lived, locked-down sandbox and check the PR out inside it — the fetch and everything after run there; nothing lands on the host working tree. One call does the whole thing, and it picks the isolation runtime itself.

# Clear host artifacts left by any EARLIER run of this PR. Several later steps
# gate on these files merely existing, so a leftover silently changes this run's
# behaviour (see Step 4) — and a stale /tmp/repro-<NUMBER>.cmd would be executed
# in the sandbox and its result attributed to this review.
# NOTE: /tmp/pr-<NUMBER>.json is deliberately NOT cleared here — Step 2 wrote it
# one step ago and Step 8 still needs it for the draft frontmatter.
# /tmp/pr-<NUMBER>.session.json IS cleared: Step 5c writes it later in this run, so
# anything present now is a previous run's session record for this PR — and a stale
# one would be read as this run's, downgrading findings against an outdated record.
rm -f /tmp/pr-<NUMBER>.diff /tmp/pr-<NUMBER>.diff.tmp /tmp/pr-<NUMBER>.files \
      /tmp/pr-<NUMBER>.review-charter.md /tmp/pr-<NUMBER>.review-context.md \
      /tmp/pr-<NUMBER>-incremental.diff /tmp/pr-<NUMBER>.evidence /tmp/repro-<NUMBER>.cmd \
      /tmp/pr-<NUMBER>.session.json

# One call: starts a locked-down sandbox (caps dropped, no privilege escalation,
# bounded memory/cpu/pids, correct isolation runtime — chosen by the CLI, and
# refused outright if it cannot get a real one), shallow-fetches this PR's head,
# and adds the base ref as a second checkout. Both sides exist before any agent
# is dispatched. Capture the id: it is minted per run and there is no name to guess.
SANDBOX=$(.claude/tools/sandbox start \
  --image "$SANDBOX_IMAGE" \
  --checkout https://github.com/nrwl/nx \
  --ref pull/<NUMBER>/head \
  --base <BASE_REF_NAME> | head -1)

.claude/tools/sandbox exec "$SANDBOX" -- git rev-parse HEAD    # HEAD_SHA

# Install HEAD once, here, before any agent is dispatched. `exec` would install it
# on first use anyway, so this is not what makes it correct — it is what stops
# several agents from racing the same install, which can corrupt node_modules.
# The base side is deliberately NOT installed here: `read --ref base` answers the
# usual base question without running anything, and most reviews never run
# base-side at all. The first `exec --base` pays for it if one does.
.claude/tools/sandbox install "$SANDBOX"

This is the slowest step in the skill, but the image ships a warm pnpm store, so the install mostly links rather than downloads. It buys correctness as much as speed: a deterministic tree at the versions the PR pins. Do not skip it, even for docs-only changes.

How slow depends on whether you are the first review in the shared host: the first install in a container copies the store out of the read-only image layer (2.3 GB, minutes), and every install after it in that container hardlinks to what is already there (12 s). So a cold host is slow once, not every time. If it is slow on a warm host, the image predates the warm store — rebuild it via setup-review-sandbox.

Notes:

• No host mounts — the checkout lives only inside the sandbox. All caps dropped, no privilege escalation, resources bounded. The CLI applies all of this; none of it is yours to pass.
• Reviews share the container, deliberately. start does not create a container per review — it attaches to one shared host per image and gives this review /work/<id>/{nx,base,mutations}, worktrees of one shared repo. That is a disk and time optimisation, not a weakening: the boundary is still host vs. container, and isolation between reviews was never claimed. It is why a review costs ~0.5 GB rather than ~4.9 GB — the pnpm store's copy-up out of the read-only image layer is ~2.3 GB, and a shared container pays it once instead of once per PR. Because the caps are now one shared budget rather than one per review, a runaway test in your PR can starve the reviews running beside it.
• Efficiency: the gh-only close-without-merge signals (Step 4.5, signals 1–4 and 6–8) need no sandbox. For a first review, you may run those cheap signals first and only start the sandbox if no strong close signal fired — a superseded/unnecessary PR then costs no sandbox. For a re-review, Step 4's incremental diff needs it, so start it before Step 4. Either way, once created it must be torn down in Step 9.
• The image carries the repo toolchain (node/java/dotnet/rust/bun via mise) baked from mise.toml, and mise auto-installs the PR's pinned toolchain on first exec. It bakes no node_modules — that is what the install step above is for.
• tsc and eslint come from that install, so agents get the versions the PR pins rather than an arbitrary latest. Report the install outcome in the charter (Step 5).
• exec puts the mise shims on PATH and lands in the right checkout for you. Do not add a cd or a PATH export of your own — written out per call site, that export was the thing that got forgotten, and a bash -lc without it fails with No version is set for shim: npm, an error with nothing to do with the PR.
• The --depth 1 fetch gives full working trees at HEAD and base — enough for reading every changed and surrounding file.
• Read base state with --ref base, never from a host clone. It is fetched fresh from the remote on every run, so it is always the PR's actual base. A maintainer's local clone can be weeks stale, which would silently answer "was this behavior already there?" against the wrong tree — the question calibration 6 exists to settle.

The sandbox reading protocol (used by every agent below)

Each agent already carries this protocol in its own definition; what follows is here so you can check a dispatch prompt against it, not to be pasted into the charter. The PR source is not on the host — it is reached only through the CLI, which presents identical commands whether the checkout is isolated or local:

.claude/tools/sandbox read <SANDBOX> <path> [--range a,b] [--ref base]
.claude/tools/sandbox grep <SANDBOX> <pattern> [subdir] [--ref base]
.claude/tools/sandbox find <SANDBOX> <glob> [subdir] [--ref base]
.claude/tools/sandbox diff <SANDBOX> [--name-only] [-- <path>...]
.claude/tools/sandbox exec <SANDBOX> [--base] -- <CMD>

Those verbs each read a single side. To answer "what differs between base and HEAD?", use diff, which compares tree hashes instead of walking files:

.claude/tools/sandbox diff <SANDBOX> --name-only
.claude/tools/sandbox diff <SANDBOX> -- <path>

Do not spell the comparison out as exec -- git diff origin/<BASE_REF_NAME>..HEAD. Reviews share one repo inside the container, so origin/<BASE_REF_NAME> is a single ref that the next review's fetch fast-forwards — a review still running would silently start diffing against a base that moved, inflating its scope with commits its PR never touched. diff resolves the review's own immovable base ref, and being read-only it works from a view id that cannot exec at all.

Never compare the two sides with a recursive filesystem diff. The HEAD side is fully installed, so diff -r walks a complete node_modules tree for minutes — measured at ~148s of CPU on a live review — and piping through grep -v node_modules does not help, because the walk is the cost. A single file's base version is read <path> --ref base, which needs no install and no comparison.

Hand the read-only lanes a narrowed id. sandbox view mints a second id onto the same checkout at a lower exec tier, so "this agent may read but not run things" is enforced by the sandbox rather than by instructions — agent frontmatter grants bare tool names (Bash), never per-verb patterns, so it cannot be expressed there:

READONLY_SANDBOX=$(.claude/tools/sandbox view "$SANDBOX" --exec none | head -1)

Give $READONLY_SANDBOX to alternative-approach, and $SANDBOX to the lanes that may need to run something.

If an agent must edit tracked files, apply a patch, or run a command known to rewrite sources, it creates its own tree — never mutating the shared checkout:

.claude/tools/sandbox worktree <SANDBOX> <AGENT> head

That returns a new sandbox id, already installed, which the agent uses in place of its original. Pass base instead of head only when the experiment must mutate the baseline. One agent owns one tree; never share or reuse another agent's. If it is refused, the dynamic check is unavailable — report that rather than working around it. Build output and ignored caches from ordinary non-rewriting commands are fine in the shared checkout; the prohibition is against changes to tracked source or refs.

The diff — the primary review surface — is fetched host-side (it's public PR info) and written to a host file the agents can Read directly:

gh pr diff <NUMBER> --repo nrwl/nx > /tmp/pr-<NUMBER>.diff.tmp \
  || { echo "FATAL: gh pr diff failed"; exit 1; }
test -s /tmp/pr-<NUMBER>.diff.tmp \
  || { echo "FATAL: empty diff for a PR reporting <CHANGED_FILES> changed files"; exit 1; }
mv /tmp/pr-<NUMBER>.diff.tmp /tmp/pr-<NUMBER>.diff

Write-then-verify-then-move, rather than redirecting straight onto the final path. A bare > truncates the target before gh runs, so a token expiry or a transient 5xx leaves a 0-byte file that every agent is then told is "the complete PR diff" — and because the changed-file list is fetched by a separate gh call, agents can end up with a populated file list and an empty diff, which is exactly the shape the Step 5 verification is least able to catch. Cross-check wc -l < /tmp/pr-<NUMBER>.files against the changedFiles count already parsed in Step 2 before dispatching anyone.

Hard rule for every agent: never execute PR code on the host. Any command that runs the checkout — npm/pnpm install, nx …, a build, a test, the linked-issue reproduction — goes through .claude/tools/sandbox exec "$SANDBOX" -- <cmd>, never bare on the host.

Step 4: Gather incremental-review context (only if a prior review exists)

If $TRIAGE_DIR/<NUMBER>.md already exists and its verdict is not failed, this is a re-review triggered by new commits. Build context for the toolkit so it can be conversational instead of starting fresh.

(If the existing draft's verdict is failed and its ## Review draft body is empty or has no findings, the prior attempt produced nothing usable — skip this step and review fresh. Do NOT discard it merely because the token says failed: since Step 7 now sets failed when any single agent fails its EVIDENCE check, a failed draft can still contain other reviewers' real findings, and throwing those away loses the reconciliation this step exists for. The file's history is preserved by Step 8 either way.)

1. Read the existing triage file in full — the whole ## Review draft plus every entry under ## Prior reviews. This is for you, the orchestrator: Step 5b reconciliation is explicitly yours to do ("don't dispatch another agent — you already have all the context"), so you need the complete history to sort findings into Addressed / Still concerning / New. Extract:

   • The frontmatter head_sha (call it $PRIOR_SHA) and verdict.
   • The ## Review draft section (the most recent review). This becomes "the prior review."
   • The full ## Prior reviews section (older reviews, if any). All of them — no cap on history.

   What you pass to the agents is a different, much smaller artifact — see step 4. Keep the two straight: full history in your head, distilled carry-forward on disk.

2. Compute the incremental diff inside the sandbox, writing it to a host file the agents can Read. $PRIOR_SHA isn't in the shallow checkout, so fetch it first — and branch on whether that fetch succeeded:

   if .claude/tools/sandbox exec "$SANDBOX" -- git fetch -q --depth 1 origin "$PRIOR_SHA"; then
     .claude/tools/sandbox exec "$SANDBOX" -- git diff "$PRIOR_SHA".."<HEAD_REF_OID>" \
       > /tmp/pr-<NUMBER>-incremental.diff \
       || { echo "FATAL: failed to build incremental diff"; exit 1; }
   else
     echo "PRIOR_SHA <PRIOR_SHA> no longer on the remote — force-pushed; reviewing fresh"
   fi
   

   A failed fetch means the author force-pushed and orphaned $PRIOR_SHA. Treat that as a fresh review: set HAS_PRIOR_CONTEXT=false, skip the incremental diff, skip the remaining steps below entirely, and note the force-push in the draft. Do not fall through with an empty incremental diff — an empty diff reads as "nothing changed since the last review" when in fact the entire branch was rewritten. (GitHub keeps force-pushed head SHAs fetchable for a long time, so this branch is rare — the common rebase case lands in step 3.)

   Set HAS_PRIOR_CONTEXT=true only on the success path. Step 5 gates on that variable, never on the context file existing — file existence is not a safe signal, because a prior review of the same PR leaves one behind and it would silently narrow this run's scope to a stale delta. (Step 3 of the skill also clears these paths up front, so the two defenses are independent.)

3. Base-movement guard: do not trust the raw range after the merge base changes. Resolve both merge bases on the host because credentials never enter the sandbox:

   OLD_MB=$(gh api "repos/nrwl/nx/compare/<BASE_REF_NAME>...$PRIOR_SHA" \
     --jq .merge_base_commit.sha) \
     || { echo "FATAL: failed to resolve prior merge base"; exit 1; }
   NEW_MB=$(gh api "repos/nrwl/nx/compare/<BASE_REF_NAME>...<HEAD_REF_OID>" \
     --jq .merge_base_commit.sha) \
     || { echo "FATAL: failed to resolve current merge base"; exit 1; }
   test -n "$OLD_MB" && test -n "$NEW_MB" \
     || { echo "FATAL: empty merge-base SHA"; exit 1; }
   REPLAY_FALLBACK=false
   

   If OLD_MB == NEW_MB, the raw $PRIOR_SHA..HEAD range contains only the branch endpoint delta. Count its changed paths and keep it as the incremental surface:

   if [ "$OLD_MB" = "$NEW_MB" ]; then
     PATCH_CHANGES=$(awk '/^diff --git / { count++ } END { print count + 0 }' \
       /tmp/pr-<NUMBER>-incremental.diff) \
       || { echo "FATAL: failed to count incremental changes"; exit 1; }
   else
     .claude/tools/sandbox exec "$SANDBOX" -- \
       git fetch -q --depth 1 origin "$OLD_MB" "$NEW_MB" \
       || { echo "FATAL: failed to fetch merge bases"; exit 1; }
   fi
   

   If the merge bases differ, the raw range includes base-branch commits. Rebuild the incremental surface by replaying the prior PR patch onto the current merge base in a temporary Git index, then compare that expected tree with HEAD. Stream the trusted helper from the host into the sandbox; never execute a helper from the PR-controlled checkout. ${CLAUDE_SKILL_DIR} is substituted when the skill loads, so the input path does not depend on the current working directory:

   if [ "$OLD_MB" != "$NEW_MB" ]; then
     if .claude/tools/sandbox exec "$SANDBOX" -- bash -s -- \
       "$OLD_MB" "$PRIOR_SHA" "$NEW_MB" \
       < "${CLAUDE_SKILL_DIR}/scripts/replay-prior-patch.sh" \
       > /tmp/pr-<NUMBER>-incremental.diff
     then
       PATCH_CHANGES=$(awk '/^diff --git / { count++ } END { print count + 0 }' \
         /tmp/pr-<NUMBER>-incremental.diff) \
         || { echo "FATAL: failed to count replayed changes"; exit 1; }
       case "$PATCH_CHANGES" in
         ''|*[!0-9]*) echo "FATAL: invalid replayed-change count"; exit 1 ;;
       esac
     else
       REPLAY_STATUS=$?
       if [ "$REPLAY_STATUS" -eq 10 ]; then
         REPLAY_FALLBACK=true
         echo "Prior patch did not replay cleanly; reviewing the full PR diff"
       else
         echo "FATAL: failed to rebuild incremental diff (exit $REPLAY_STATUS)"
         exit 1
       fi
     fi
   fi
   

   The helper exits 10 only when --3way cannot replay the prior patch; every other nonzero exit is fatal. The temporary index preserves the prior author patch across non-overlapping base churn, including binary changes, modes, symlinks, unusual pathnames, and file-to-directory transitions. If the replay conflicts, the author may have resolved overlapping base changes manually. In that case, REPLAY_FALLBACK=true selects the full PR diff in Step 5 and keeps the prior review context; it never treats an uncomparable patch as an empty delta or assigns a numeric change count. If this block reports FATAL, stop the review because its evidence is invalid.

   If REPLAY_FALLBACK=false and PATCH_CHANGES is zero, this was a base-movement-only push or an equivalent tree rewrite. Skip the remaining context-building and agent steps, re-verify the carry-forward yourself at HEAD, update the review body with "no author delta", then continue at Step 8 so history and cleanup still run. Any positive count continues to Step 5 regardless of line count; the existing evidence fallback handles a small but real author delta.

4. Write a context file at /tmp/pr-<NUMBER>.review-context.md (host-side — the agents Read it directly; it is our file, not PR code).

   Distill; do not paste. Every byte here is read by every agent you dispatch, so its cost is multiplied by the whole fleet — on a PR with several prior attempts, pasting full bodies makes the carry-forward the single largest fixed charge in the run, larger for most agents than the diff they are meant to review. Worse, it is mostly inert: the bulk of a prior draft is that round's Reproduction / Approach / Performance / Security prose, which describes work already done and re-verified from scratch this round by the agents that own those dimensions. What an agent genuinely needs from history is short: what is still open, what was already fixed, and which trade-offs are settled so it does not re-litigate them.

   Write this shape instead, and keep the whole file under ~80 lines:

   # Re-review context
   
   Attempt <N-1> reviewed `$PRIOR_SHA` and returned **<PRIOR_VERDICT>**. This is attempt <N>.
   Earlier attempts: <one line per attempt, oldest first — "attempt 2 (1046ace) lgtm — daemon now
   rejects foreign-workspace messages">.
   
   ## Open items — I re-checked these at HEAD; cite them, do not re-verify
   
   <Every unresolved Critical/Important finding from ANY prior attempt, one bullet each.
   Quote the finding's own one-line summary verbatim where it has one; add the file:line and
   the specific ask. These are load-bearing — see the budget rule below.
   
   Mark each one with what YOU observed at HEAD before dispatching — "still present at
   performance-report.ts:41, unchanged by this delta" or "now fixed by <commit>". An agent that
   reads a bare open item will go and re-open the same three files to check it; an agent that
   reads your verified status will cite it and move on.>
   
   ## Already fixed — do not re-raise
   
   <One line per finding a prior attempt raised and a later attempt confirmed closed, with what
   closed it. Agents need these so they neither re-report them nor mistake the fix for new code.>
   
   ## Settled maintainer calls — do not re-litigate
   
   <One line each: the decision, and that it was reviewed and accepted. An agent that does not
   know a trade-off is settled will re-report it every single round; this section is the cheapest
   part of the file and prevents the most repeat noise.>
   
   ## Diff since last review (`$PRIOR_SHA..<HEAD>`)
   
   <When `REPLAY_FALLBACK=false`: See /tmp/pr-<NUMBER>-incremental.diff for the author delta since the prior review.>
   <When `REPLAY_FALLBACK=true`: The prior patch did not replay cleanly on the current base, so this attempt reviews the full PR diff. No narrower author delta is safe.>
   
   ## Review focus
   
   Focus on the named review target. The open items above are already re-checked; carry
   their status into your report if your dimension owns one, but do not go and re-derive it. Do not
   re-analyze unchanged code from scratch.
   
   <Optionally: 2-4 specific questions this round should settle, phrased neutrally.>
   

   Rules for the distillation:

   • Re-check open items once; move fixed ones to Already fixed.
   • When OLD_MB != NEW_MB, re-verify Already fixed items at HEAD too. Base movement can silently drop a landed fix. A dropped one goes back to Open items.
   • Never omit an unresolved finding; trim narrative first.
   • Preserve each finding's wording, location, and ask; omit old reproduction/approach/performance/security prose.
   • Carry facts, not a prior verdict's reasoning; keep focus questions neutral.
   • Full history remains in $TRIAGE_DIR/<NUMBER>.md; only this agent-facing digest is trimmed.

Step 4.5: Close-without-merge check

Before running the toolkit, do a cheap pass to answer: "Should this PR be closed without merging?" Two flavors:

• Superseded — master or another PR already addressed the goal.
• Unnecessary — the change shouldn't be merged at all (no real bug, abandoned, out of scope, duplicate of rejected work).

Both save the toolkit's effort on PRs that won't merge anyway. Signals 1–4 detect supersession; signals 6–8 detect unnecessary; signal 5 detects an unconfirmed bug (it can push to blocked, never to a close). Run the gh-only signals here. Signal 5 depends on the reproduce-verifier and is finalized after Step 5a.5.

These signals close other people's work, so bias every judgment call toward the contributor: when a signal is ambiguous, treat it as not fired.

Supersession signals (gh-only, run now)

1. Mergeability. If master moved in the same files, the PR is stale.

gh pr view <NUMBER> --repo nrwl/nx --json mergeable,mergeStateStatus

Flag if mergeable == "CONFLICTING" or mergeStateStatus == "DIRTY".

2. Cross-references on linked issues. Has another merged PR referenced the same issue? Parse closingIssuesReferences from the PR body + gh pr view (look for Fixes #N, Closes #N, Resolves #N). For each linked issue:

gh issue view <ISSUE> --repo nrwl/nx --json timelineItems --jq '.timelineItems[] | select(.__typename == "CrossReferencedEvent") | select(.source.__typename == "PullRequest") | {pr: .source.number, state: .source.state, merged: .source.merged, mergedAt: .source.mergedAt, title: .source.title}'

Flag any other PR with merged: true — that PR may have fixed the same issue.

3. Same-file merged PRs since this PR opened. Identify possibly-competing work. Get the PR's createdAt and files[].path, then:

gh pr list --repo nrwl/nx --state merged --search "<FILE_PATH> merged:><PR_CREATED_AT>" --json number,title,mergedAt --limit 5

Pick the 2-3 most-touched distinctive files — skip monorepo hot files (package.json, lockfiles, migrations.json, versions.ts) that unrelated PRs touch constantly. Only flag a hit when the merged PR's title suggests the same goal as this one; same-file overlap alone is not competing work.

4. Target-state check. For small PRs (< 50 lines changed OR touches only package.json / versions.ts / migrations.json), peek at master to see if the target state is already there.

Confirm NX_REPO_PATH really is an nrwl/nx clone before trusting it — its default is git rev-parse --show-toplevel, so invoking the skill from some other repo would silently point this signal at that repo's master. Then refresh the remote-tracking ref and read each changed file:

if git -C "$NX_REPO_PATH" remote get-url origin 2>/dev/null | grep -q 'nrwl/nx'; then
  git -C "$NX_REPO_PATH" fetch -q origin <BASE_REF_NAME>
  git -C "$NX_REPO_PATH" show origin/<BASE_REF_NAME>:<path>
else
  echo "NX_REPO_PATH is not an nrwl/nx clone — skipping signal 4 (would read the wrong repo's master)"
fi

The if/else must actually gate the fetch+show. A … || { echo "skip"; } form prints the warning and then runs them anyway — and signal 4 can recommend closing a contributor's PR, so reading the target state from the wrong repo's master is a confident wrong closure. (Verified: the ||-only form reaches both commands.)

(If the sandbox already exists at this point, prefer sandbox read "$SANDBOX" <path> --ref base and skip the host clone entirely — it needs neither the origin check nor the fetch.)

Compare key lines against what the PR is trying to set. Example: if the PR changes "@foo/bar": "^1.0.0" → "^2.0.0" but master already has "^2.3.3", flag it. The fetch is not optional — this signal can recommend closing someone's PR, and a local clone that is weeks stale would answer "is the target state already on master?" from the wrong tree. (If the sandbox already exists at this point, read --ref base is equivalent and needs no fetch.)

For larger PRs, skip this — the toolkit will catch subtler issues.

Unnecessary signals

5. Bug not confirmable. Finalized after Step 5a.5. If the reproduce-verifier returns BUG_NOT_REPRODUCED_ON_BASELINE, treat that as inconclusive, not proof of a non-bug — many nx bugs are environment-specific (package manager, OS, node version), so a local non-repro proves little. Look for corroboration in the linked issue instead:

# Has a maintainer engaged with the issue?
gh issue view <ISSUE> --repo nrwl/nx --json comments --jq '[.comments[].author.login]'

A Linear ticket is corroboration, and usually stronger than a GitHub comment — it means the team tracked the work deliberately. Step 2 has already fetched it, so check it here before concluding the bug is unconfirmed. Firing this signal on a NXC-… PR purely because it has no GitHub issue would push a tracked, triaged piece of work toward blocked for the sole reason that its tracker is not GitHub.

If no nrwl-org member has confirmed the bug, no tracking ticket describes it, AND the PR body offers no rationale of its own (no root-cause explanation, no design-doc link), the right outcome is a question, not a closure: flag it, push the verdict toward blocked, and have the draft ask the author for a runnable reproduction. This signal never forces unnecessary.

6. Stale + abandoned + conflicted. All three together:

• Last commit on the PR branch > 90 days ago: parse commits[-1].committedDate from gh pr view ... --json commits.
• Has merge conflicts (signal 1 fired).
• Has unanswered reviewer questions: most recent non-author comment is unanswered. Check via gh pr view <NUMBER> --json comments --jq '.comments | map({author: .author.login, at: .createdAt}) | last' — if the last commenter is not the author and the timestamp is > 30 days old, it's unanswered.

If all three fire, the PR is abandoned and unlikely to land. Any sign of recent author engagement (a comment within the last 30 days, even without new commits) resets this signal — prefer the stale-branch advisory instead.

7. Duplicate of recently-closed-without-merge PR. Search closed-but-not-merged PRs touching the same primary file in the last 6 months:

gh pr list --repo nrwl/nx --state closed --search "<MAIN_FILE_PATH> closed:>$(date -d '6 months ago' +%Y-%m-%d 2>/dev/null || date -v-6m +%Y-%m-%d)" --json number,title,closedAt,state,mergedAt --limit 10

Filter to entries where mergedAt is null (closed without merging). Only flag when a closed PR has a clearly similar title or approach — not merely the same file — and note that the prior close may have been for fixable reasons (stale, author gave up), which weakens the signal.

8. No linked issue + speculative scope. All of:

• No Fixes #N / Closes #N / Resolves #N reference in body or commits. A Linear reference (NXC-XXXX, or a linear.app/... link, whether phrased "Fixes" or "Relates to") counts as a linked issue — do NOT treat a Linear-only PR as unlinked; many nx PRs track work in Linear rather than GitHub. Step 2 has already fetched that ticket, so judge this signal on what the ticket actually says: a NXC-… whose ticket states a real problem satisfies the linked-issue check outright. Only a reference that resolves to nothing readable leaves the PR effectively unlinked.
• The PR body doesn't explain why the change is needed — no motivation, no linked discussion. Judge the substance, not the length.
• PR modifies > 100 lines OR touches public-API surface (packages/*/src/index.ts, files matching *.public.ts, anything under packages/*/index.ts).

Speculative refactors without a stated reason are usually closed. Advisory-strength signal — flag in the section, but don't on its own force a verdict.

Emit

If any signal fires, prepend a ### Close-without-merge check section to $REVIEW_BODY (above ### Reproduction verification):

### Close-without-merge check

<pick the strongest line — only one verdict-line, but multiple advisory lines OK:>

- 🛑 **Likely superseded.** <reason, with linked PR numbers / file evidence>
- 🛑 **Likely unnecessary.** <reason — name the signal(s) that fired: abandoned, duplicate of #N, etc.>
- ⚠️ **Bug unconfirmed.** Couldn't reproduce the linked issue on master and found no maintainer confirmation — the draft should ask the author for a runnable repro.
- ⚠️ **Stale branch.** Merge conflicts with master on <N> files; author should rebase before review lands.
- ⚠️ **Speculative scope.** No linked issue and no stated motivation for a large change.
- ✅ No close signals — PR is current and well-scoped.

Verdict influence (Step 7):

• Superseded (strong) → verdict superseded. "Strong" means ANY of: signal 2 fires (another merged PR closes the same issue), OR signals 3+4 both fire (same-file merged PR AND master already at/past the PR's target state). The section should include the specific superseding PR number(s) so whoe

…(truncated)