goat-review

name	goat-review
description	Use when reviewing a diff, PR, or set of code changes, or auditing a codebase area for quality issues. Triggers: 'review this', 'code review', 'audit X', 'look at these changes'.
goat-flow-skill-version	1.14.0

/goat-review

Shared Conventions

Read .goat-flow/skill-docs/skill-preamble.md; on full-depth also read .goat-flow/skill-docs/skill-conventions.md.

When to Use

Use for diff/PR review or codebase-area quality audits.

Boundary Commands

• NEVER: Auto-edit, perform security review, or run an unapproved refuter.
• ALWAYS: Reconstruct intent, run both passes, disprove suspicions, and emit the local verdict with Review Integrity.
• DEFER TO: Security, debug, QA, planning, or dispatcher routes for their named work.

Step 0 - Scope, Size, Spec

"Reviewing [X] -- diff review (quick), PR review against a base branch (quick by default), or area audit + DoD cross-checks (full)?"

• If user already says "quick", "PR", or "full", confirm and continue.
• If the dispatcher chose depth, skip the question.
• If vague, ask one follow-up covering files, concerns, and mode.
• Auto-detect: explicit input; otherwise a dirty worktree (combine staged and unstaged changes into one declared change set); otherwise PR-style branch ahead of base, then git diff.

PR mode: prefer URL/number; otherwise prompt or use local. Get metadata: gh pr view <ref> --json baseRefName,headRefName,headRefOid,url,number,title,body; diff: gh pr diff <ref>. Record URL/base SHA. Automated-review conclusions stay unread until both local passes finish; Step 0 fetches no review/comment bodies.

Base fallback: when no PR link or gh unavailable, resolve base from explicit user base, skills.goat-review.local_pr_base, remote HEAD, user prompt, then main with base-detection-failed. Prefer existing refs; only git fetch origin <base> --quiet after explicit network approval. Diff origin/<base>...HEAD if present, else local <base>...HEAD with base-fetch-skipped or base-fetch-failed. Record base/source/SHA in Review Integrity.

Scope sizing: Diff: measure files/changed lines; above 20 files OR 3000 lines, propose chunking and flag large-diff-unchunked if declined. Area: measure files/clusters; above 20 files, propose splitting and flag large-area-unchunked if declined.

Spec source (opt-in): if .goat-flow/plans/.active points to an in-progress/testing milestone, offer: "Include Spec Drift check against M[NN] exit criteria?" Default skip for quick, offer for full. Record checked/skipped/unavailable in Review Integrity; optional skip is not degradation.

Temporary artifacts: use .goat-flow/logs/review/goat-review-<artifact>.<random>.txt only.

Footgun check: use preamble learning-loop retrieval on .goat-flow/learning-loop/footguns/ for the target area. Present matches or retrieval miss; do not broad-load.

Review Scope Snapshot (mandatory)

Before Pass 1, record the review surface:

• Source: worktree | staged | unstaged | PR | branch diff | area | explicit path list
• Base/Head: <branch-or-sha> / <branch-or-sha> (n/a for area audit)
• Uncommitted included: yes | no | n/a
• Size: diff <files>/<changed-lines>; area <files>/<clusters>
• Chunking: no | proposed | accepted | skipped-by-user
• Scope degradation: <flags or "none">

For worktree, inspect both git diff --cached and git diff; record both path sets.

Unknown mode-applicable values add degradation. Required n/a is resolved, not degraded.

Step 0.5 - Intent Reconstruction (mandatory)

Before Pass 1, reconstruct intent. Diff/PR: PR/issues, HEAD, then active milestone; none means intent-unstated. Area: the user's audit brief plus responsibilities inferred from source/docs; change history is not required.

Output three-bullet reconstruction:

• Stated intent: change claim or area brief
• Implied intent: observed behavior/responsibility
• Gap: divergence or "none"

Anchor both passes to diff and stated intent, or the declared area and audit intent.

CHECKPOINT: Scope locked, intent reconstructed. Proceeding to Pass 1.

Diff Review (Quick) - Two-Pass Discipline

Run two sequential local passes; Pass 2 is authoritative and findings surface afterward.

Pass 1 - Blind Suspicion (diff only)

Read the diff without opening full files. The point is to see what the diff reveals before surrounding code anchors you.

Scan for severity cues (auth, secrets, SQL/shell/API calls, mutation, state transitions) and edge cases: boundary conditions, nullish/default branches, concurrency, error handling, contract changes, and observability/DDT testability. For opaque state transitions, background tasks, retries, or async flows, ask: "can a human tell if this succeeded without instrumenting it?" If no, consider [SHOULD:needs-signal] or [MUST:needs-signal] per risk.

Write raw suspicions with file + semantic anchor drawn from the diff. Do NOT verify, confirm, or dismiss in this pass. Over-capture is fine; Pass 2 filters.

CHECKPOINT: Pass 1 complete - [N] suspicions captured (no resolution yet). Proceeding to Pass 2 grounded verification.

Pass 2 - Grounded Verification (full files)

Now read full files. For each Pass-1 suspicion:

• Try to DISPROVE it by re-reading the anchor and looking for guards, upstream checks, framework mitigation, or contracts that remove the risk.
• Blast Radius Rule: for contract changes (signature, payload, exported type, event shape, error channel, status code), MUST run external call-site search before resolving. Prefer rg -n '<symbol>' -t ts -t js -t py -t php -t go -t rust; else use host search or grep -rniE '<symbol>' and record fallback. Verify at least one consumer. If skipped, stays UNRESOLVED with coverage-degraded.
• Mark each suspicion: CONFIRMED / REFUTED / UNRESOLVED.
• Refutation Ledger: write REFUTED suspicions to .goat-flow/logs/review/goat-review-refutations.<random>.txt with original suspicion, refuting evidence, and one-sentence rationale. Do not surface refuted items in final output.
• Add findings that only became visible with file context (integration breakage, call-site contract mismatch, regression in a sibling file).
• Re-verify every file + semantic anchor reference exists before writing the final output.

Automated-Review Overlap (PR mode, after local findings)

After Pass 2 records local findings, fetch inline comments with gh api --paginate 'repos/<owner>/<repo>/pulls/<number>/comments?per_page=100', then apply references/automated-review.md; never suppress a finding as overlap.

Full Excuse/Reality table: references/examples.md. Key entries:

Excuse	Reality
"Skip Pass 2 / CI is green / zero findings anyway"	Trust, CI, and empty results don't replace opening files. See full table.
"The symbol is unique enough that grep is overkill"	The bug is in the consumer, not the emitter. Run the grep.
"Refuted suspicions are noise - logging them wastes tokens"	The ledger is the integrity surface. Without it, REFUTED is indistinguishable from "didn't bother to check."

Severity + Action Tagging

Every surfaced finding gets severity and action tags. Severity: MUST blocks approval, SHOULD fixes before merge unless disputed, MAY is optional. Action: patch, needs-decision, pre-existing, intent-mismatch, or needs-signal.

Finding line prefix: [SEVERITY:ACTION]. Example: [MUST:needs-decision].

Proof Capsule: every finding includes a proof class per skill-preamble.md Proof Classification: RUNTIME | CONTRACT-GREP | STATIC | NOT-REPRODUCED. MUST/correctness-SHOULD should prefer RUNTIME or CONTRACT-GREP. NOT-REPRODUCED adds not-reproduced-findings to Review Integrity.

Systemic Patterns

When 3+ surfaced findings share the same root cause, report one parent entry under ## Systemic Patterns using the highest applicable severity and action tag. Include the affected file anchors, the repeated failure mode, and the concrete harm. Keep individual findings only when they have distinct harm or distinct fixes; otherwise the systemic pattern is the finding.

Pre-existing Separation

• Pre-existing Nearby (in-scope surface): a pre-existing bug in the same function or tightly-coupled call-site the diff touches. Surface as a one-line pointer under ## Pre-existing Nearby. Does not block.
• Pre-existing Issues (out-of-scope): pre-existing bugs outside the diff's surface. List under ## Pre-existing Issues without severity tags. Does not block.

Footgun Cross-Check

Check each finding with targeted INDEX-first retrieval against .goat-flow/learning-loop/footguns/INDEX.md. When a direct match exists, include it. Omit the footgun tag when no direct match is found after the one allowed reword.

BLOCKING GATE: Present findings plus Top 5 Risks and Review Integrity, then pause. If Pass 3 is pending, Ship Verdict must be PENDING REFUTER/HUMAN; after response/refuter, present final verdict.

Review DoD gate: for reporting-only review, verify findings, cross-references, and scope. No implementation tests unless a finding requires it. If user says "implement", switch to the instruction file's implementation DoD.

Proof Gate: per skill-preamble.md.

Area Audit (Full)

Audit the declared area, not a diff; pre-existing issues are in scope.

Area Pass 1 - Inventory and Risk Hypotheses

For each cluster, inventory responsibilities, interfaces, trust/state boundaries, and critical paths without using recent diff as scope. Record raw suspicions with file + semantic anchor; do not resolve them.

Area Pass 2 - Implementation and Consumer Verification

Open the implementation, relevant tests, and callers/consumers. Disprove suspicions using guards and call-site evidence; apply the Blast Radius Rule. Mark each suspicion CONFIRMED, REFUTED, or UNRESOLVED and retain the Refutation Ledger.

Without a release/merge question, emit N/A - AREA AUDIT ONLY.

BLOCKING GATE: Present findings and pause. If calibration is uncertain, consider /goat-critique.

Direction / Opportunity Audit

Only on explicit request, add an advisory opportunity output backed by repo-grounded evidence; it does not affect Ship Verdict. Categories, leverage ranking, and rejection routing: references/examples.md. Defects stay in normal findings.

Spec Drift (opt-in)

Only emitted when Step 0 prompt was accepted and a live milestone was found. Reads the milestone's Exit Criteria and Assumptions, splits by direction:

• Exit-criteria drift [advisory] under ## Spec Drift -- criterion marked done but diff doesn't support it. No severity tag.
• Assumption invalidation [MUST:needs-decision] under ## Findings -- diff makes an assumption false.
• Open criterion satisfied [ready-to-tick] under ## Spec Drift -- advisory, human ticks milestone.

If none detected, emit "No drift detected against M[NN]" so the reader knows the check ran.

Pass 3 - Cross-Model Refuter (explicit approval only)

Offer Pass 3 when ANY of: (1) user opts in at Step 0, (2) Review Integrity would be coverage-degraded or high-inference, (3) any [MUST:needs-decision] finding exists, (4) any INTENT-MISMATCH finding exists.

Approval gate: A trigger is not approval. Run only local installation and auth status checks first. Disclose the runtime and model, authentication state, findings-only payload, maximum of one refuter inference call, known cost or rate-limit impact (or unknown), and the local-only fallback. Wait for explicit current-session approval after that disclosure; generic instructions such as “keep going,” urgency, or a request for a definitive answer do not count. If approval is declined or unanswered, skip Pass 3, complete the local review, and record Refuter pass: skipped. Preserve only degradation flags already earned by Passes 1–2; do not add coverage-degraded or cross-model-refuter-failed solely because the user declined.

Method (after approval): Use an authenticated external refuter runtime, not the host model. Default host map: Claude -> codex exec; Codex/Copilot/Antigravity -> claude -p unless a verified stronger opposite runtime is documented. Pass FINDINGS LIST, not the diff. Template: references/refuter-spec.md.

Synthesis: REFUTER-CONFIRMED findings get [CONFIRMED-CROSS-MODEL] upgrade. REFUTER-REFUTED move to ## Refuted by Refuter with reasoning preserved verbatim. REFUTER-UNRESOLVED keep original severity; add cross-model-unresolved to Review Integrity. Refuter leads do not become findings unless host verifies via Pass 2 rules.

Constraints: Only the local availability and auth checks from references/refuter-spec.md may run before approval; version-only commands do not count. If no authenticated refuter exists for the current host, skip Pass 3 and emit cross-model-refuter-failed. REFUTER-REFUTED stays advisory.

Review Integrity (confidence signal)

Confidence signal for review coverage.

• Files opened in Pass 2: count / total; diff mode also lists paths.
• Evidence tags: N OBSERVED / M INFERRED.
• Size: diff lines or area files/clusters, plus chunking. PR mode: base, source, short SHA.
• Scope snapshot: source, base, head, uncommitted, chunking.
• Refutations logged: <N>
• Spec drift: checked M[NN] | skipped | unavailable. Optional skip is not degradation.
• PR-mode extension: record Automated-reviewer overlap: <K> overlap with <reviewer-list>, <M> net-new; use no-automated-review-present when absent and n/a outside PR mode.
• Pass-3 extension: when Pass 3 runs, is triggered, or is skipped after a trigger, add Refuter pass: yes | no | skipped; confirmed=<N>, refuted=<M>, unresolved=<K>, leads-verified=<N>, model=<id|n/a>.
• Degradation flags: chunked-partial, large-diff-unchunked, large-area-unchunked, high-inference-ratio, files-not-opened, unfamiliar-area, missing-types, footguns-unread, not-reproduced-findings, coverage-degraded, configured-base-unresolved=<base>, base-detection-failed, base-fetch-skipped, base-fetch-failed, intent-unstated, automated-review-uningested, cross-model-refuter-failed, cross-model-unresolved.
• Conclusion: confident | coverage-degraded | high-inference | partial.

Always emit it; minimum: "confident - no degradation flags".

Constraints

Diff review (quick):

• MUST run Pass 1 (diff only) before opening any full files in Pass 2
• MUST NOT surface Pass-1 suspicions that Pass 2 refuted
• MUST NOT flag pre-existing issues as blocking the change

Area audit (full):

• MUST scan the declared area regardless of recent changes
• Pre-existing issues ARE in scope

Both modes:

• MUST apply the Blast Radius Rule, severity/action tags, Footgun Cross-Check, systemic grouping, and Review Integrity in both modes
• MUST order findings by severity, not by file or discovery order
• MUST propose chunking above 20 files in either mode, or 3000 changed lines in diff mode
• Emit Spec Drift only when opted in. If skipped, record Spec drift: skipped without a degradation flag
• Route Spec Drift by direction
• MUST NOT edit files unless user says "implement"; MUST NOT frame Pass 1/Pass 2 as doer/verifier
• Consequence Gate: every MUST and SHOULD finding MUST state concrete harm (what breaks, leaks, regresses, silently fails, corrupts data, or blocks a workflow). If the reviewer cannot name harm, downgrade to MAY.
• Ship Verdict rules (diff/PR or explicit release/merge question): unresolved MUST -> NO. SHOULD-only -> YES WITH CONDITIONS. MAY-only -> YES. INTENT-MISMATCH -> NO until author confirms intent. Review Integrity coverage-degraded, high-inference, or partial -> downgrade verdict one step.
• Zero-findings HALT: If Pass 2 produces zero findings, state what was checked and why no issues surfaced. Zero findings must be defended.
• Universal constraints from skill-preamble.md apply.

Output Format

## TL;DR  <!-- what was reviewed, found, matters most -->

## Review Integrity
- Scope snapshot: source=<source>, base=<base>, head=<head>, uncommitted=<yes|no|n/a>, chunking=<state>
- Files opened in Pass 2: <k>/<n>  (diff paths: <list or "n/a">)
- Evidence: <N> OBSERVED / <M> INFERRED
- Refutations logged: <N>
- Size: <files> files, <changed lines | clusters>  (chunked: <group or "no">)
- Automated-reviewer overlap: <K> overlap with <reviewer-list>, <M> net-new | no-automated-review-present | n/a
- Refuter pass: yes | no | skipped; confirmed=<N>, refuted=<M>, unresolved=<K>, leads-verified=<N>, model=<id|n/a>
- Spec drift: <checked M[NN] | skipped | unavailable>
- Degradation flags: <list or "none">
- Conclusion: <confident | coverage-degraded | high-inference | partial>

## Findings

### MUST / SHOULD / MAY
- [SEVERITY:ACTION] **[title]** `file + semantic anchor` - [desc] | Footgun: [entry or none] | Evidence: OBSERVED/INFERRED | Proof: RUNTIME/CONTRACT-GREP/STATIC/NOT-REPRODUCED

## Systemic Patterns  <!-- only when 3+ findings share one root cause -->
- [SEVERITY:ACTION] **[pattern title]** - affected anchors: `<file + semantic anchor>`, `<file + semantic anchor>`; repeated failure: <one sentence>; harm: <one sentence>

## Spec Drift   <!-- only when opt-in triggered -->
<!-- advisory-only entries (exit-criteria drift, ready-to-tick); assumption invalidation goes under ## Findings as [MUST:needs-decision] -->
- [advisory] **[criterion title]** - claimed done in M[NN] but not supported by diff
- [ready-to-tick] **[criterion title]** - now satisfied by diff, milestone still shows `- [ ]`

## Pre-existing Nearby  <!-- in-function only; one-liners; no blocking tags -->

## Pre-existing Issues  <!-- out-of-scope pre-existing bugs -->

## Breaking Changes

## Top 5 Risks (cross-tier)
<!-- Five findings most likely to cause harm if unresolved, ranked regardless of tier. If <5 total, list all. If zero: "No surfaced risks." -->
1. [SEVERITY:ACTION] **[title]** `file + semantic anchor` - one-sentence why

## Ship Verdict
Decision: **YES** | **YES WITH CONDITIONS** | **NO** | **PARTIAL** | **PENDING REFUTER/HUMAN** | **N/A - AREA AUDIT ONLY**
Reasoning: <2-3 sentences anchored to Top 5 Risks and Review Integrity>
Conditions to ship: <numbered list, only when YES WITH CONDITIONS>
Confidence: HIGH | MEDIUM | LOW

## What's Good

## What I Didn't Examine