--- title: "Review Pr: References" description: "11 references for the Review Pr skill: Adversarial Refutation; Claude Code; Cross Model Refuter; Memory Persistence; Orchestration Mode Selection; Progressive And Partial Results; Review Report Template; Review Template; Rule Check Mode; Task Metrics Template; Ultrareview Gate" canonical: "https://orchestkit.dev/docs/reference/skills/review-pr/references" --- # Review Pr: References 11 references for the Review Pr skill: Adversarial Refutation; Claude Code; Cross Model Refuter; Memory Persistence; Orchestration Mode Selection; Progressive And Partial Results; Review Report Template; Review Template; Rule Check Mode; Task Metrics Template; Ultrareview Gate > Part of the [Review Pr](/docs/reference/skills/review-pr) skill reference. The main page carries the skill itself; this page holds material that used to sit at the bottom of it. ## References (11) ### Adversarial Refutation # Adversarial Refutation: review-pr bindings Thin adapter. Loads the shared engine, then binds it to review-pr's finding + verdict model. **Load the engine first:** `Read("${CLAUDE_PLUGIN_ROOT}/shared/rules/adversarial-refutation.md")` the blindness contract, independent-score-first, citation-verify, quorum, cross-file UPHELD-default, deterministic-exemption, no-auto-flip, spawn-ceiling, ledger schema, and isolated-spawn rules. This file only supplies what's review-pr-specific. **Default path:** `workflows/review-fanout.js` enforces the effort gate, the scope filter, blindness, the quorum, the deterministic exemption and the section 8 ceiling below, and returns the ledger. The shell still does section 3 (re-open each cited `file:line`) and section 7 (ask before a kill removes a blocker). The rest of this file is the contract the script follows and the procedure for the fallback path. ## Bindings | Engine concept | review-pr binding | |----------------|-------------------| | "finding" | a Phase 3 conventional comment classified `issue` (especially a **request-changes blocker**) or a decision-bearing `suggestion`, from the 6-agent JSON | | rubric | `code-review-playbook` + the producing agent's domain rubric (security→OWASP, perf→Core Web Vitals, tests→coverage-gap) | | refuter agent | the **same** `subagent_type` that produced the finding (code-quality-reviewer / security-auditor / test-generator / backend-system-architect / frontend-performance-engineer / accessibility-specialist), spawned blind | | code artifact | the `CHANGED_FILES` diff slice for the cited `file:line` (Phase 1 scope), the refuter re-reads the diff itself, never the producer's quoted snippet | | ledger | `refutation-ledger.json` in the review job dir (`$CLAUDE_JOB_DIR`) | | revised output | a `refuted` + `original_severity` field on each finding object + a "Refuted?" note in the Phase 5 report; the Phase 6 verdict honors **no-auto-flip** (§7) | ## Scope filter (which findings get a refuter) A finding qualifies only if decision-bearing, ANY of: - it is a **request-changes blocker** (the verdict flips on it) - **CRITICAL or HIGH** severity (security / correctness / data-loss) - it is the **sole** blocker standing between the PR and `approve` - a finding `/ork:implement` or the author will act on immediately (concrete code change demanded) Skip: `praise`, `nitpick`, and low/style `suggestion` comments; mid-severity advisory notes that cannot change the merge verdict. Dedup duplicate findings to root-cause BEFORE counting (engine §8). Bounds spawns to ~2-6 per review. ## Effort gate (review-pr-specific) - `low` / `medium` → **skip Phase 4.5 entirely** - `high` → up-to-6 **single** refuters, **advisory only**, an `OVERTURNED`-with-verified-citation is surfaced for the user (engine §7 no-auto-flip; a single refuter never demotes a blocker on its own) - `xhigh` → **quorum** per engine §4: 3-refuter majority for a request-changes blocker, 2 for a HIGH finding; a kill that would remove a blocker still requires explicit user confirmation before the verdict changes (§7) ## Verdict guardrail (review-pr-specific) Refutation MAY demote a finding's display bucket and drop its confidence, but it may **NOT** by itself flip the human-facing verdict from `request-changes` → `approve` (engine §7). Keep the producer-basis verdict AND a labeled "post-refutation" view; surface every killed CRITICAL/HIGH prominently. Ground truth (failing CI/tests/lint, npm-audit/CVSS matches) is exempt, never refuted (engine §6); only a *reachability claim* layered on a CVE is refutable. ## Isolation note Even when Phase 3 ran in Agent Teams mode, Phase 4.5 refuters are ALWAYS standalone `Agent(...)` Task spawns with **no `team_name`**, fed only the serialized claim + diff slice. Joining the mesh would leak producer reasoning via `SendMessage` history (engine §9). ### Claude Code # Claude Code adapter: review-pr Slash invoke: `/ork:review-pr` YAML `hooks:` / `command:` lines in this skill's frontmatter stay Claude-only. They use `${CLAUDE_PLUGIN_ROOT}/hooks/bin/run-hook.mjs` and are ignored by pi. Session chain files live at `.claude/chain/` (state.json, decisions.json, capabilities.json). Other hosts should persist equivalent run state in their own session store. Agent spawn form, when used: `Agent(ork:...)` is Claude Code. Portable body text uses the skill name. Lines that still mention a Claude-only path in the body: ``` Host-neutral workflow. Invoke by skill name (`review-pr`). Claude Code slash routing, YAML hook loaders, and `.claude/chain` live in `references/claude-code.md`. Write(".claude/chain/capabilities.json", { memory, timestamp }) ``` ### Cross Model Refuter # Cross-Model Adversarial Refuter (provenance + cost gate) Operational doc for the optional **cross-model** lane of the adversarial-refutation engine, used by `review-pr` (Phase 4.5) and `assess` (Phase 2.5). Loads on top of the engine (`${CLAUDE_PLUGIN_ROOT}/shared/rules/adversarial-refutation.md`) and the skill's bindings adapter, this file only adds the alternate-model wiring. > Pseudocode below is **illustrative prompt-doc**, not a real API, function names (`build_neutral_claim()`, `run()`, etc.) name the steps Claude performs, not symbols to import. ## Why a different model at all Same-model refuters (engine §1, §10) buy **variance reduction**, not independent bias correction, N Claude agents share blind spots ("Known residual bias"). A refuter on a *different* model family (Codex / GPT) has a **different failure surface**: it misses different things, so a finding that survives both a Claude producer AND a non-Claude refuter is structurally harder to fake. This lane changes *who* judges, not *how*. It is **off by default**. Diversity costs money, latency, and an egress hop; the homogeneous lane is correct for the overwhelming majority of reviews. ## Default-OFF precondition gate (ALL must hold) ``` cross_model_enabled = effort in {high, xhigh} # never on low/medium AND alt_model_cmd_present() # a configured non-Claude CLI AND finding.tier in {request-changes-blocker, CRITICAL, HIGH} # decision-bearing only AND ORK_CROSS_MODEL != "0" # explicit kill switch, checked first AND under_cross_model_budget() # see Cost gate ``` If any clause is false → run the **same-model** lane exactly as today (engine §4 quorum). Cross-model never runs ALONE, there is always ≥1 same-model refuter; it **substitutes one slot** of the existing quorum (total count unchanged, §8 ceiling intact), so a misconfigured or down alternate degrades to the pure same-model quorum, never to "no refutation". ### `alt_model_cmd_present()`: transport (no credentials in-process) ``` ORK_ALT_MODEL_CMD set → a shell command that takes a prompt on stdin and emits refuter JSON on stdout (e.g. `codex exec --json`, `llm -m gpt-...`). This is the ONLY transport. The skill never reads a provider key from the environment and never opens a socket itself — routing is delegated to a CLI the user already trusts and configured. else → cross-model UNAVAILABLE. Skip silently with one WARN: "cross-model refuter: no ORK_ALT_MODEL_CMD — same-model lane only". ``` House posture (mirrors the network-egress guard #2533): the skill must not own credentials or open egress. It shells out to a user-configured command, exactly like `gh`/`glab`; that command's egress is the user's explicit choice, surfaced once. ## Provenance: labeled, but **not** a trust boundary Every cross-model artifact is stamped so a wrong call is traceable: - each refuted finding gains `refuter_model` (e.g. `"gpt-5-codex"`) + `refuter_lane: "cross-model"` (same-model gets `"same-model"`); the report renders a `🔀 cross-model` tag and a footer (`Refuters: 4 same-model, 2 cross-model (gpt-5-codex)`); the engine §10 ledger carries `refuter_model`/`refuter_lane` per vote. **Honesty:** `refuter_model` is **best-effort attribution derived from the CLI's output, NOT a verified trust boundary**, a wrapper can emit any banner it likes. Do not treat the label as proof of which model ran. The actual gaming protection is the citation-verify gate + zero-weight-on-agreement + no-auto-flip (see Anti-gaming), which hold *regardless* of the label. A cross-model refuter is bound by the same blindness contract (engine §1): neutral claim + raw diff slice + rubric excerpt only, never the producer's score, identity, or prose. The transport difference grants no relaxation. ## Quorum interaction (a slot, not a veto) Cross-model adds ONE diverse refuter into the engine §4 quorum. It **substitutes** a same-model slot at the request-changes-blocker tier (total judges unchanged at 3), but at **CRITICAL / HIGH it is ADDED, not substituted**, the two same-model refuters are kept and the cross-model one is a third judge (3 total). Substitution there would drop same-model 2→1, leaving a CRITICAL judged by exactly two voters (one of them an untrusted-label alt model), thinner corroboration precisely where stakes are highest (#2556). The ADD costs one extra spawn at that tier by design; it is exempted from the §8 ceiling accounting the same way the blocker tier's substitution is (dedup-to-root-cause still runs first). | Finding tier | Same-model | + Cross-model | Total judges | Rule | |---|---|---|---|---| | request-changes blocker | 2 (was 3) | 1 (substitutes) | 3 | majority of 3; cross-model counts once, like any refuter | | CRITICAL / HIGH | 2 (kept) | 1 (**added**, #2556) | 3 | majority of 3 → no lone kill flips a verdict (engine §4/§7) | | advisory | 1 | 0 | 1 | cross-model never spawned for advisory findings | A 1-of-N cross-model dissent follows engine §4: records a caveat, drops the finding's confidence to "low", never revises alone. The no-auto-flip gate (engine §7) is untouched: a cross-model KILL of a `request-changes` blocker still requires explicit user confirmation before the verdict moves. ## Cost gate Cross-model is the only lane that can bill an external provider, so it is hard-capped: ``` under_cross_model_budget() = cross_model_spawns_this_run < ORK_CROSS_MODEL_MAX # default 4 AND finding ranked top-K by (severity-weight x distance-from-decision-boundary) ``` - Default cap **4** cross-model spawns/run (inside the engine §8 ceiling). `ORK_CROSS_MODEL_MAX=0` disables; `ORK_CROSS_MODEL=0` is the master kill switch (checked first). - Over the cap → refute the top-K cross-model, the rest **same-model only**, flagged `cross_model: "skipped — budget"` in the report (never silently truncated, engine §8). - **Prompt once before the first spawn** (consent, not just disclosure, aligns with `/ultrareview`, which asks first for the same external spend; #2556). Ask via AskUserQuestion and proceed only on approval; on decline, fall back to the same-model lane (never "no refutation"). The `ORK_ALT_MODEL_CMD` env var + high-effort gate express *configuration*, not per-run consent, a configured command shouldn't silently bill on every run. Prompt copy: `🔀 Cross-model refutation will make up to N calls to via ORK_ALT_MODEL_CMD (external, billed by your provider). Run it? [Yes / Same-model only]`. Skip the prompt only when `ORK_CROSS_MODEL_CONSENT=1` is set (a durable pre-authorization for unattended/`-p` runs), mirroring the standing-authorization carve-out. ## Run loop (per qualifying finding: illustrative) ``` for finding in qualifying_findings_ranked: # engine §8 dedup + rank first same = spawn_same_model_refuters(finding) # engine §4 quorum, minus 1 slot if X-model on cross = None if cross_model_enabled and under_cross_model_budget(): claim = build_neutral_claim(finding) # engine §1 blindness — no producer value diff = changed_files_slice(finding.file, finding.line) out = run(ORK_ALT_MODEL_CMD, stdin=render_refuter_prompt(claim, diff, rubric_excerpt)) cross = parse_refuter_json(out) # {verdict, independent_band, citation} cross.refuter_model = detect_model_label(out) or "alt-model" # best-effort, not trusted cross.refuter_lane = "cross-model" votes = same + ([cross] if cross else []) for v in votes: if v.verdict in {KILL, OVERTURN, DOWNGRADE} and not reopen_and_verify_citation(v.citation): v.verdict = UPHELD # engine §3: unverifiable cite → UPHELD outcome = engine_quorum_decision(finding, votes) # engine §4; no-auto-flip §7 ledger_append(finding, votes, outcome) # engine §10 + refuter_model/lane ``` A cross-model vote that fails to parse, times out, or returns no citation → treated as **UPHELD** (engine §3). A flaky alternate model can never *weaken* a finding. **Parser contract.** `ORK_ALT_MODEL_CMD` must emit exactly the `ork-cross-model-refuter/1.0` shape, `{verdict, independent_band, citation}`, pinned in [`cross-model-output.schema.json`](./cross-model-output.schema.json) so `parse_refuter_json` never improvises the shape (#2556). `verdict ∈ {UPHELD, DOWNGRADE, OVERTURN, KILL}`; `independent_band` is the model's own blind `lo-hi` band on 0-10 (engine §1); `citation` is a `file:line` or `null`. Anything off-contract (extra keys, bad verdict, non-JSON) is parse-failure → UPHELD. A stub implementation for tests lives at `tests/fixtures/cross-model-refuter/stub-alt-model.sh`. ## Decision / reset rules - **Disagreement** (Claude UPHELD, cross-model KILL or vice-versa): never auto-resolve toward the kill. Surface BOTH bands + citations; outcome = `survived`, `cross_model_dissent: true`, confidence "low". A Claude/Codex split is exactly the diversity you paid for, show it to the human. - **Reset** (CLI fails mid-run, rate limit, auth expiry): stop spawning cross-model, log `cross_model: degraded — same-model only from finding #k`, finish on the same-model lane. Never block a review on an external dependency. - **Until blocking findings clear:** cross-model only re-runs on findings still tagged `request-changes-blocker` / `CRITICAL` / `HIGH` after producer fixes; a finding that drops below HIGH or a verdict that reaches `approve` falls out of scope automatically (the precondition gate re-evaluates `finding.tier` each pass). ## Scope (v1) The engine names three consumers: `assess`, `review-pr`, `audit-full`. v1 wires the cross-model lane into **review-pr** (Phase 4.5) and **assess** (Phase 2.5). `audit-full` inherits the engine §11 *principle* but gets **no Phase pointer in v1** (out of scope), wire it in a follow-up if its single-pass audit grows a refutation phase. ## Anti-gaming guardrail **The one way it gets gamed:** route the "cross-model" refuter to a weak/sycophantic model (or a same-Claude command mislabeled as "gpt") so cross-model findings rubber-stamp the producer, diversity theater. **Why it can't work**: net, not by the label: (1) a cross-model refuter can only KILL/DOWNGRADE through the engine §3 citation-verify gate, which the **orchestrator (Claude, not the alt model)** runs, it re-opens the cited `file:line` and confirms support; an unverifiable cite → UPHELD, so a sycophantic "looks fine, KILL it" does nothing. (2) A cross-model *agreement* with the producer carries **zero** extra weight, it cannot raise a score or clear a blocker (engine §7 no-auto-flip). (3) `refuter_model` is best-effort attribution, **not** a trust check, but it doesn't need to be, because (1) and (2) bound a captured alt-model to discardable noise regardless of its self-reported label. ## Anti-patterns | Anti-pattern | Why it's wrong | Do instead | |---|---|---| | Read a provider key from env and POST to the API | skill owns credentials + opens egress (violates #2533) | shell `ORK_ALT_MODEL_CMD` only; key stays in the user's CLI | | Cross-model on by default when a key exists | cost/latency/egress on every review | gate on effort high/xhigh AND decision-bearing tier AND kill switch | | Let a cross-model KILL flip request-changes→approve | external model silently clears a real bug | engine §7 no-auto-flip; explicit user confirm | | Trust the alt model's self-reported name as a check | "gpt" label on a Claude command = fake diversity | label is best-effort only; rely on citation-verify + zero-weight-agreement | | Cross-model *adds* a refuter on top of the quorum | inflates spawn count + cost | it SUBSTITUTES one same-model slot; total unchanged | | Skip same-model when cross-model is on | one external model becomes a lone judge | cross-model is always additive to ≥1 same-model refuter | | Hard-fail the review when the CLI is down | external dependency blocks merge | degrade to same-model, log `degraded`, finish | | Forward the producer's score to the alt model "for context" | breaks blindness (engine §1) | neutral claim + raw diff slice only | ### Memory Persistence # Memory Persistence (manual fallback) The Phase 8c verdict writeback script (`scripts/verdict_writeback.py`) handles this automatically when `yg-mcp-core>=0.3.0` is installed. Use the manual pattern below when running an interactive review on a host that does NOT have yg-mcp-core (the script will skip cleanly in that case and you can fall back to direct memory MCP calls). ## Pattern ```python # Persist review findings for cross-session learning mcp__memory__create_entities(entities=[{ "name": "PR-{number}-Review", "entityType": "code-review", "observations": [ "", "", "", ], }]) # Update known-weaknesses entity if new patterns found mcp__memory__add_observations(observations=[{ "entityName": "review-known-weaknesses", "contents": [""], }]) ``` ## When to use this vs Phase 8c | Context | Use | |---------|-----| | HQ environment, `yg-mcp-core` installed | Phase 8c (automatic) | | Public fork, `yg-mcp-core` not installed | This manual pattern (interactive) | | Headless CI without HQ creds | Skip both, Phase 8c exits 0 | The two paths produce the same KG shape (entity name `PR-{number}-Review`, entityType `code-review`). Either path is safe; don't run both. ### Orchestration Mode Selection # Orchestration Mode Selection Choose **Agent Teams** (mesh, reviewers cross-reference findings) or **Agent tool** (star, all report to lead): 0. Workflow (`workflows/review-fanout.js`) -> **default**; the rules below apply when the Workflow tool is unavailable 1. Agent Teams mode (GA since CC 2.1.33) -> **recommended for full review with 6+ agents** 2. Agent tool mode -> **for quick/focused review** 3. `ORCHESTKIT_FORCE_TASK_TOOL=1` -> **Agent tool** (override) | Aspect | Agent Tool | Agent Teams | |--------|-----------|-------------| | Communication | All reviewers report to lead | Reviewers cross-reference findings | | Security + quality overlap | Lead deduplicates | security-auditor messages code-quality-reviewer directly | | Cost | ~200K tokens | ~500K tokens | | Best for | Quick/focused reviews | Full reviews with cross-cutting concerns | > **Fallback:** If Agent Teams encounters issues, fall back to Agent tool for remaining review. ### Progressive And Partial Results # Progressive Output and Partial Results (fallback modes) Applies when Phase 3 runs in the Agent tool or Agent Teams fallback. On the default Workflow path, `workflows/review-fanout.js` returns every reviewer's result in one object, and a reviewer that fails is reported as `NOT-REVIEWED` in `reviewers`. ## Progressive Output (CC 2.1.76+) Output each agent's findings **as they complete**. Don't batch until synthesis. > **Focus mode (CC 2.1.101):** In focus mode, the user only sees your final message. Include the full review verdict, all findings by severity, and the approve/request-changes recommendation. Don't assume they saw per-agent outputs. - **Security findings** → show blockers and critical issues first - **Code quality** → show pattern violations, complexity hotspots - **Test coverage gaps** → show missing test cases This lets the PR author start addressing blocking issues while remaining agents are still analyzing. Only the final synthesis (Phase 5) requires all agents to have completed. **Partial results (CC 2.1.98):** If a review agent fails mid-analysis, synthesize partial findings: ```python for agent_result in review_results: if "[PARTIAL RESULT]" in agent_result.output: # A security agent that found 2 issues before crashing > no security review for finding in parse_findings(agent_result.output): # may be empty finding["partial"] = True # Flag each one in synthesis findings.append(finding) # Do NOT re-spawn: partial findings are still valuable ``` A `maxTurns` stop is also partial since CC 2.1.246 (summary: "stopped at its N-turn limit (partial result; continue it with SendMessage to the task-id)"); continue that agent with `SendMessage` instead of re-spawning it. **Monitor for CI streaming (CC 2.1.98):** Stream CI check output in Phase 4: ```python Bash(command="gh pr checks $PR_NUMBER --watch 2>&1", run_in_background=true) Monitor(pid=ci_watch_id) # Each status change → notification ``` ### Review Report Template # Review Report Template Use this template when synthesizing agent feedback in Phase 5: ```markdown # PR Review: #$ARGUMENTS ## Summary [1-2 sentence overview] ## Code Quality | Area | Status | Notes | |------|--------|-------| | Readability | // | [notes] | | Type Safety | // | [notes] | ## Test Adequacy | Check | Status | Details | |-------|--------|---------| | Tests exist for changes | // | [X changed files have tests, Y do not] | | Test types match changes | // | [e.g., API changes have integration tests] | | Coverage gaps | // | [N untested paths] | | Test quality | // | [meaningful assertions, no flaky patterns] | **Verdict:** [ADEQUATE | GAPS (list) | MISSING (critical)] ## Security | Check | Status | |-------|--------| | Secrets | / | | Input Validation | / | | Dependencies | / | ## Blockers (Must Fix) - [if any] ## Suggestions (Non-Blocking) - [improvements] ``` ### Review Template # PR Review Template ## Review Output Format ```markdown # PR Review: #[NUMBER] **Title**: [PR Title] **Author**: [Author] **Files Changed**: X | **Lines**: +Y / -Z ## Summary [1-2 sentence overview of changes] ## ✅ Strengths - [What's done well - from praise comments] - [Good patterns observed] ## 🔍 Code Quality | Area | Status | Notes | |------|--------|-------| | Readability | ✅/⚠️/❌ | [notes] | | Type Safety | ✅/⚠️/❌ | [notes] | | Test Coverage | ✅/⚠️/❌ | [X% coverage] | | Error Handling | ✅/⚠️/❌ | [notes] | ## 🔒 Security | Check | Status | Issues | |-------|--------|--------| | Secrets Scan | ✅/❌ | [count] | | Input Validation | ✅/❌ | [issues] | | Dependencies | ✅/❌ | [vulnerabilities] | ## ⚠️ Suggestions (Non-Blocking) - [suggestion 1 with file:line reference] - [suggestion 2] ## 🔴 Blockers (Must Fix Before Merge) - [blocker 1 if any] - [blocker 2 if any] ## 📋 CI Status - Backend Lint: ✅/❌ - Backend Types: ✅/❌ - Backend Tests: ✅/❌ - Frontend Format: ✅/❌ - Frontend Lint: ✅/❌ - Frontend Types: ✅/❌ - Frontend Tests: ✅/❌ ``` ## Approval Message ```markdown ## ✅ Approved Great work! Code quality is solid, tests pass, and security looks good. ### Highlights - [specific positive feedback] ### Minor Suggestions (Non-Blocking) - [optional improvements] 🤖 Reviewed with Claude Code (6 parallel agents) ``` ## Request Changes Message ```markdown ## 🔄 Changes Requested Good progress, but a few items need addressing before merge. ### Must Fix 1. [blocker 1] 2. [blocker 2] ### Suggestions - [optional improvements] 🤖 Reviewed with Claude Code (6 parallel agents) ``` ## Conventional Comments | Prefix | Usage | |--------|-------| | `praise:` | Highlight good patterns | | `nitpick:` | Minor style preference | | `suggestion:` | Non-blocking improvement | | `issue:` | Must be addressed | | `question:` | Needs clarification | ## Example Comments ``` praise: Excellent use of the repository pattern here - clean separation of concerns. nitpick: Consider using a more descriptive variable name than `d` - maybe `data` or `response`. suggestion: This loop could be replaced with a list comprehension for better readability. issue: This SQL query is vulnerable to injection - use parameterized queries instead. question: Is there a reason we're not using the existing `UserService` here? ``` ### Rule Check Mode # Rule-check Mode (opt-in) Source: claude.dev, "A harness for every task". Rule adherence is a harness, not a reviewer's afterthought: one verifier agent per rule against the diff, then a skeptic agent that filters false positives, and only the survivors are reported. A single reviewer holding every CLAUDE.md rule at once skims them; a verifier holding one rule does not. Off by default. It runs when `/ork:review-pr` gets `--rules`, or when the user asks to check a change against their CLAUDE.md, rules files or "my rules". ## Pipeline | Step | Who | What | |------|-----|------| | Collect | `scripts/collect-rules.mjs` | Reads `CLAUDE.md`, `.claude/CLAUDE.md`, `.claude/rules/**/*.md` in the repo, then `~/.claude/CLAUDE.md` and `~/.claude/rules/**/*.md`, plus whole-line `@path` imports one level deep. Every path the reviewed PR controls is confined: the repo's own `CLAUDE.md`, `.claude/CLAUDE.md` and `.claude/rules/**` are read only when their realpath is a `.md` inside the repo root (a symlinked rules directory that leaves the repo is not even walked), and an import made from one of them only when its realpath is a `.md` inside the repo root (so a PR cannot import the operator's `~/.claude` memory into a review comment); an import made from `~/.claude/CLAUDE.md` or `~/.claude/rules/**` may also reach a `.md` inside `~/.claude`. Anything refused is listed in `skipped` (`missing`, `outside-root`, `symlink-escape`, `not-md`) and never opened, so a PR cannot pull a credential file from the home directory into a prompt. `~/.claude/CLAUDE.md` and `~/.claude/rules/**` are exempt: they are the operator's own files, often symlinked into a dotfiles repo. `--no-user` keeps it to the project. A file reached twice is read once. | | Split | `workflows/rule-check.js`, plain code | Top-level list items (nested items and continuation lines fold in), table data rows, directive paragraphs and quotes become rules. Code fences, YAML frontmatter, HTML comments and table headers never do. Text without a directive word (must, never, always, do not, avoid, prefer, should, only, required, ...) is skipped and listed in `skipped`. A rule copied into several files is checked once; `alsoIn` names the copies. | | Verify | one agent per rule, effort `low` | Reads the diff itself (`diffCommand`), answers `applies` and lists violations on ADDED or changed lines only, each with file, line and the quoted line. | | Skeptic | one agent per violation, session effort | Gets the rule, the location and the quoted line, not the verifier's explanation, and is told to refute it. | | Report | plain code | `survivors`, `refuted`, `outOfScope`, `unverified`, `unchecked`, `notApplicable`, `skipped`, `reasons`. | ## Ceilings - Verifiers: 12 by default (`maxVerifiers`, lowerable, never above 24). With more rules than that, rules share verifiers in contiguous batches of near-equal size, each rule still answered on its own row. The `reasons` line says so; no rule is dropped. - Skeptics: 12 by default (`maxSkeptics`, same bounds). Violations past the ceiling go to `unverified` with the reason, never silently into `survivors`. ## The survivor filter A violation is **refuted** only when the skeptic answers `refuted=true` AND cites a `file:line` in the violation's file or another changed file AND gives a reason. Anything less leaves it standing: | Skeptic answer | Outcome | |----------------|---------| | `refuted=true`, in-diff `file:line`, reason | refuted (listed with the citation) | | `refuted=true` without a citation, outside the diff, or no line | survives, `skeptic: "unbacked refutation"` | | `refuted=false` | survives, `skeptic: "upheld"` | | dead or throwing skeptic | survives, `confidence: "low"` | Before any skeptic runs: a violation in a file outside `changedFiles` is `outOfScope` (rule check is diff-scoped), and one with no file goes to `unverified`. A dead, BLOCKED or silent verifier leaves its rules in `unchecked`. ## In the report - Each survivor is an `issue (rule)` finding: the rule text with its `source:line`, the violating `file:line`, the quoted line. Any survivor floors the verdict at `comment`; rule findings never raise it to `request-changes` on their own. - `unverified` and `unchecked` are listed as "not checked, manual review required". - `skipped` is a count, plus the list on request. - `refuted` is listed in a collapsed section with each citation, so a wrong kill is auditable. Tests: `tests/unit/test-review-rule-check.mjs` (splitting fixtures, fan-out and batching, the survivor filter, the collector), no live agent runs. ## Standards pass (`--standards`) Two-pass review: the builder makes it work, then a separate pass checks the repo's standards. The source is only `.github/review-standards.md` (no CLAUDE.md, no `~/.claude`, no imports), a file Claude Code never loads into a builder session. At most 20 rules and 3 KB. A leading `[glob]` limits a rule to matching paths, and a violation outside it goes to `outOfScope`. - The file is read from the repo's default branch (`--default-branch`, as `refs/remotes/origin/`), never from the PR head or the PR's chosen base; a differing base gets one `notes` line. `--base-ref` accepts only `refs/remotes/origin/*`, `origin/*` or a full SHA, and is refused when it resolves to the checked-out HEAD. The ref is resolved to one SHA, and both git reads use it. A symlink committed there is refused. - No file at the rules ref, neither `--default-branch` nor `--base-ref`, or a tree or file git cannot read (a shallow clone): the collector returns no sources and one `skip` line, and the pass logs it and stops. - Every verifier row counts, also a second row for the same rule. A row naming a rule the agent was not given is reported in `unverified`, `unknownRuleIds` and `findingLines`. - Rules are numbered `S1..Sn` in file order. Each survivor prints as `S (.github/review-standards.md:) broken at :` (`findingLines`). - Default `strategy: "single"`: one agent holds every rule at session effort and refutes its own findings first, with no skeptics. `--standards --rules` sets `strategy: "fanout"` (one verifier per rule plus skeptics). Measured on #4667 at 03f6132d: single 107,832 tokens, fan-out 1,099,909, the same one finding. - The pass never gives LAND or HOLD and never commits. The conductor sends the findings to a separate fix lane, then the LAND reviewer reviews the final head. Tests: `tests/unit/test-review-standards-pass.mjs`. ### Task Metrics Template # Task Metrics Template (CC 2.1.30) Agent tool results now include efficiency metrics. After parallel agents complete, report: ```markdown ## Review Efficiency | Agent | Tokens | Tools | Duration | |-------|--------|-------|----------| | code-quality-reviewer | 450 | 8 | 12s | | security-auditor | 620 | 12 | 18s | | test-generator | 380 | 6 | 10s | **Total:** 1,450 tokens, 26 tool calls ``` Use metrics to: - Identify slow or expensive agents - Track review efficiency over time - Optimize agent prompts based on token usage ### Ultrareview Gate # /ultrareview Gate (CC 2.1.111+, optional) Claude Code 2.1.111 ships a built-in `/ultrareview`, parallel multi-agent deep review (Pro/Max users get 3 free per month). It overlaps this skill's Phase 3 but goes deeper. It's not free, so never fire it by default, offer it only when a trigger justifies the cost, and always ask the user before burning a quota. Reliability note (CC 2.1.212): `/ultrareview ` PR references parse correctly, branch fetch works for branches not checked out locally, and billing no longer misfires after `/clear`. ## Trigger evaluation (automatic, Phase 2.5, BEFORE the review call) The shell owns every question to the user, so the ask happens before the `workflows/review-fanout.js` call, from the PR metadata Phase 1 already collected: ```python triggers = [] if diff_loc_changed > 500: triggers.append("large_diff") if any(path.startswith(p) for path in changed_files for p in ["auth/", "migrations/", "hooks/", "crypto/", "security/", "payments/"]): triggers.append("sensitive_path") if any(label in pr_labels for label in ["release", "hotfix"]): triggers.append("high_stakes_label") ``` `reviewer_disagreement` needs the reviewers' verdicts, so it cannot fire before the call. The workflow returns it as `reviewerDisagreement`; if it is true and this gate did not ask earlier in the run, the shell may ask in Phase 5 with the same prompt and reason `reviewer_disagreement`. If `triggers` is empty → **skip** the gate entirely and proceed to Phase 3. Never mention `/ultrareview` to the user. ## When triggers fire: voice-friendly prompt Read session state: `Read(".claude/state/ultrareview-usage.json")` (may not exist). If `month == currentMonth()` and `skip_session == true`, **skip the prompt** and proceed to Phase 3. Otherwise: ```python AskUserQuestion(questions=[{ "question": f"This PR triggers /ultrareview (reason: {', '.join(triggers)}). Run it? (Pro/Max: 3 free per month.)", "header": "Ultrareview", "multiSelect": false, "options": [ {"label": "Yes, run ultrareview", "description": "Invoke built-in /ultrareview as a final deep pass. Adds 5–10 min."}, {"label": "No, skip it", "description": "Continue with the Phase 3 review without it."}, {"label": "Skip for this session", "description": "Don't ask again until this session ends."} ] }]) ``` Why `AskUserQuestion` and not a `--ultra` flag: the user relies on voice, so "yes"/"no"/"skip for session" is speakable whereas flags are not. ## After user response - **Yes** → ask the user to type `/code-review ultra ` themselves: it is user-triggered and billed, and the post gate denies it from inside this skill. Merge any findings they paste back with Phase 3 results in Phase 5 (label them "Ultrareview:"). - **No** → proceed to Phase 3 unchanged. - **Skip for this session** → write `.claude/state/ultrareview-usage.json`: ```json { "month": "2026-04", "session_skip": true, "last_asked": "" } ``` Then proceed to Phase 3. On every run where the user said "Yes", increment the month counter so we advise against a third ask in the same month: ```json { "month": "2026-04", "used_this_month": 2, "last_used": "" } ``` This is advisory only, we cannot query Anthropic's real quota. When `used_this_month >= 3`, the AskUserQuestion text changes the third option to warn: *"You may have exhausted the monthly free quota."* ## Opt-out Set `ORK_DISABLE_ULTRAREVIEW=1` or `.claude/settings.json` → `"ork.disableUltrareview": true` to skip the gate entirely regardless of triggers. Honored at the top of this phase.