Review Pr

review-pr

Read-only first-pass PR review, partitioned into 3 sequential stages. Gather PR context and the full diff (inline) → review the whole diff in cohesion as a single isolated-context subagent (the critical-judgement step) → post the review comment, apply the deterministic security and lane:* labels, apply the reviewed label last, write the local report (inline). Never checks out code, never fixes issues, never pushes commits — that's double-check's job.

Purpose

The monolithic review-pr ran the dedup gate, context gathering, diff read, analysis, confidence-scoring, convention check, comment post, label, and report write in one session. The critical-judgement step (analyze + confidence-score the diff) shared a context window already polluted by raw gh JSON, the full diff, CI output, and existing comments. This proc isolates that judgement into one clean-context subagent that sees only the curated PR context and diff.

The ICM win here is clean-context isolation of the review verdict — not parallelism. A PR is reviewed in COHESION: the entire diff, all dimensions (correctness, conventions, Closes-vs-Refs) together in ONE review stage. There is NO per-file fan-out and NO parallel subagents.

Arguments

Parse from $ARGUMENTS: PR=$1, REPO=$2. Both required.

Token: none needed. GitHub auth is ambient — the pod's git-credential-pylot helper and the gh shim mint short-lived App installation tokens per operation.

What it does

3-stage SEQUENTIAL ICM procedure:

There is exactly one subagent stage (01). It is NOT split per-file or per-dimension.

Handoff locations

All handoffs live in the repo working directory:

.procedure-output/review-pr/{stage}/handoff.md

Stage 00 writes the curated PR context + full diff. Stage 01 receives ONLY that handoff (never the orchestrator history). Stage 02 reads both handoffs.

Execution

Stage 00 (inline)

Run stage 00 yourself (orchestrator context). Read CONTEXT.md:

skills/review-pr/stages/00-context/CONTEXT.md

Run the dedup gate first. Short-circuit only when the PR has reviewed AND the latest review comment's **Head reviewed:** line names the current headRefOid. A missing, invalid, or stale receipt is not completion: record review_run: stale-refresh, continue through the normal review, and let stage 02 re-toggle reviewed after posting the current-head receipt.

Stage 01 (single subagent — NO fan-out)

Spawn exactly ONE Task. Pass only the stage's input handoff path and the stage CONTEXT.md path. Do NOT pass orchestrator history or the raw gh output you already saw. Do NOT split the diff across multiple Tasks — the review must see the whole diff in cohesion.

Task prompt template:

You are running stage 01-cohesive-review of the review-pr procedure.

Read your stage instructions:
  skills/review-pr/stages/01-cohesive-review/CONTEXT.md

Your inputs:
  .procedure-output/review-pr/00-context/handoff.md

Write your output to:
  .procedure-output/review-pr/01-cohesive-review/handoff.md

Execute all steps in CONTEXT.md. Review the ENTIRE diff together in cohesion — do not fragment it
per-file. Write handoff.md before exiting.

Await the task before proceeding to stage 02. If it fails, emit the failure outcome marker and stop.

Stage 02 (inline)

Run stage 02 yourself (orchestrator context). Read CONTEXT.md:

skills/review-pr/stages/02-post/CONTEXT.md

Post the comment, apply the security label if warranted (Step 2), apply the lane:* label (Step 2.5), apply the reviewed label LAST (Step 3), write the local report file, and emit the [pylot] outcome=... marker from the orchestrator (never from a subagent).

Stage handoff chain

00-context (inline: dedup gate + context + full diff + auth surface detection)
   │
   └─► 01-cohesive-review (single subagent, clean context, whole diff + security classification)
          │
          └─► 02-post (inline: comment → security label → lane label → reviewed label → report → outcome marker)
                         │
                         ├── lane:fast    ──► cto-review-on-reviewed-fast  (review + cto merge)
                         ├── lane:staging ──► review-pr-on-reviewed        (full pipeline)
                         └── no lane      ──► review-pr-on-reviewed        (pre-#2996 default)

Exit paths

• Already complete: stage 00 dedup gate finds a current-head receipt → [pylot] outcome="already complete — reviewed receipt matches current HEAD {sha}" status=success (orchestrator, inline)
• Success: stage 02 emits [pylot] outcome="review-pr complete — reviewed label applied, lane:{fast|staging|n/a}" status=success
• Failure: failing stage emits [pylot] outcome="review-pr failed at stage NN: {reason}" status=failed

Hard Rules

1. SEQUENTIAL ONLY — stages run one after another. NO parallel Task launches.
2. Exactly one subagent stage (01) — the whole-diff cohesive review. NO per-file fan-out, NO per-dimension split. The review must see the entire diff together for cross-file cohesion.
3. Stage 00 runs inline — dedup gate + context gathering happen in the orchestrator.
4. Stage 02 runs inline — the [pylot] outcome=... marker MUST come from the orchestrator.
5. Never pass full orchestrator context into the subagent — input handoff path only.
6. Each stage writes handoff.md before the next stage reads it.
7. Read-only — no git clone, no git checkout, no file modifications to the repo under review, no pushes. The diff comes from gh pr diff.
8. Confidence threshold is 80 — never surface findings below 80. Don't lower it.
9. NO QUEST — reporting is the local report file only. No Quest POST, no QUEST_TOKEN, no 127.0.0.1:4242. Operators surface the report via the mission report.
10. Never apply double-checked — only reviewed. The verdict is always "proceed to double-check"; this skill never blocks.
11. Do not skip stages — every stage executes (except stage 01/02 when the head-aware dedup gate proves the latest receipt matches current HEAD). A reviewed label by itself is never proof of completion.
12. Risk tier is mechanical and escalate-only (#2210) — stage 00 computes it from the rubric; stage 01 may raise it (recording why) but never lower it. LOW-tier review is bounded by design — do not "be thorough" past the tier; the saved depth is reallocated to HIGH-tier PRs.
13. The **Head reviewed:** receipt line is always posted with the full 40-hex head — it is what binds the review to the exact head for the dedup gate and downstream stages. Findings keep their R{n} IDs downstream; never renumber.
14. Security label is deterministic (#2918) — stage 02 applies security if any finding is security-class OR if auth_surface: new-auth-surface. No judgement: if the condition is met, the label is applied, period. The label is set in the same mission as the review.
15. Lane label is a SCRIPT's answer, not yours (#2996) — stage 02 runs scripts/classify-pr-surface.mts --lane and applies whatever it prints. Never reason about the lane, never override it, never talk yourself into fast because the diff "looks small". The classifier is the single source of truth; disagreeing with it is a PR against LANE_STAGING_GLOBS, not a decision in this mission. Anything other than the exact string fast — a crash, empty output, a missing classifier, a repo that is not lane-enabled — resolves to lane:staging / no label, i.e. the pre-#2996 pipeline. Fast is opt-in, always.
16. Label ORDER: security → lane:* → reviewed, and reviewed is LAST (#2996) — the automations that react to reviewed read the label set carried in that webhook payload, so a lane label applied after reviewed is invisible to them and the PR silently pays for the full pipeline. This ordering is the mechanism, not a style preference.
17. Review completion is head-bound — every posted review names and records the automatically fetched current HEAD SHA. When refreshing a stale receipt, remove and re-add reviewed only after the new comment and prerequisite labels land, so downstream stages receive a current trigger without restarting or blocking the review itself.

Reference files

• CONTEXT.md — architecture overview
• stages/NN-name/CONTEXT.md — per-stage inputs, task, steps, output contract