| 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.11.0 |
/goat-review
Shared Conventions
Read .goat-flow/skill-docs/skill-preamble.md for shared conventions.
On full-depth, also read .goat-flow/skill-docs/skill-conventions.md.
When to Use
Use when reviewing a diff, PR, or set of changes. Also for quality audits of a codebase area.
Boundary: goat-review owns quality, style, correctness. goat-security owns threat models, compliance, CVEs, auth boundaries. Security issues: flag and suggest /goat-security.
NOT this skill: OWASP assessment → /goat-security. Understanding code → /goat-debug. Generating tests → /goat-qa. Planning milestones → /goat-plan. Feature briefs → dispatcher Route Map.
Step 0 - Scope, Size, Spec
"Reviewing [X] -- diff review (quick), PR review against a base branch, or area audit + DoD cross-checks (full)?"
- If user already says "quick", "PR", or "full", confirm and continue.
- If arriving from the dispatcher with depth already chosen, skip the depth question.
- If vague, ask one follow-up covering files, concerns, and diff / PR / audit.
- Auto-detect scope: (1) explicit input, (2) staged changes, (3) unstaged changes, (4) PR-style when HEAD is on a non-default branch with commits ahead of the detected review base, (5) git diff.
PR mode (prefer PR link): ask for PR URL/number first; it collapses base, head, description, and linked issues. Prompt: "PR URL or number? -- or say 'local' if not pushed." Resolve with gh pr view <ref> --json baseRefName,headRefName,headRefOid,url,title,body,reviews,comments; diff via gh pr diff <ref>. Record PR URL and base SHA. See references/automated-review.md for overlap-tagging protocol.
PR mode (base fallback): when no PR link or gh unavailable, resolve base: explicit user base, config skills.goat-review.local_pr_base (record configured-base or configured-base-unresolved), remote HEAD, ask user, then main with base-detection-failed. Prefer existing refs; only run git fetch origin <base> --quiet after explicit network approval. Diff via origin/<base>...HEAD if present, else local <base>...HEAD with base-fetch-skipped or base-fetch-failed. Record base/source/SHA in Review Integrity.
Size sizing (before Pass 1): measure the diff. If it exceeds 20 files OR 3000 changed lines, propose chunking by file group and ask. If the user proceeds un-chunked, record as large-diff-unchunked for Review Integrity.
Spec source (opt-in): if .goat-flow/plans/.active exists, read it to find the active plan subdir and scan for a milestone file with Status: in-progress or testing-gate. If found, offer: "Include Spec Drift check against M[NN] exit criteria?" Default: skip for quick, offer for full. Note the choice in Review Integrity.
Temporary review artifacts: write under .goat-flow/logs/review/ only with a random suffix (goat-review-<artifact>.<random>.txt). Never write to repo root.
Footgun check: Use the preamble's learning-loop retrieval on .goat-flow/learning-loop/footguns/ for the target area. Present matches or an explicit retrieval miss; do not broad-load the bucket.
Review Scope Snapshot (mandatory)
Before Pass 1, record the exact review surface:
- Source: staged | unstaged | PR | branch diff | explicit path list
- Base/Head:
<branch-or-sha> / <branch-or-sha> (or n/a)
- Uncommitted included: yes | no | n/a
- Size:
<files> files, <changed-lines> changed lines
- Chunking: no | proposed | accepted | skipped-by-user
- Scope degradation:
<flags or "none">
If any value is undetermined, write unknown and add a degradation flag.
Step 0.5 - Intent Reconstruction (mandatory)
Before Pass 1, reconstruct WHY this change exists. Read in priority order: (1) PR description and linked issues via gh pr view <ref> --json body,title and gh issue view <n>, (2) commit message of HEAD, (3) active milestone exit criteria from .goat-flow/plans/.active. If none exist, flag intent-unstated in Review Integrity.
Output three-bullet reconstruction:
- Stated intent: what the change claims to do
- Implied intent: what the diff actually appears to do
- Gap: divergence between stated and implied, or "none"
Pass 1 and Pass 2 anchor to BOTH the diff and the stated intent.
CHECKPOINT: Scope locked, intent reconstructed. Proceeding to Pass 1.
Diff Review (Quick) - Two-Pass Discipline
The review runs two sequential passes. This is a deliberate reading discipline, not a doer-verifier split: you are the reviewer throughout, Pass 2 is the source of truth, and findings are only surfaced after Pass 2.
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/permission checks, secret handling, SQL/shell/API calls, data mutation, state transitions
- Edge-case sweep - 6 meta-categories, specifics bubble up as the diff warrants:
- Boundary conditions - off-by-one, pagination/index bounds, empty collections, integer overflow
- Nullish values - null / undefined / default branches, missing optional fields
- Concurrency - race windows, shared state, concurrent access
- Error handling - timeouts, retries/backoff, silent exception swallowing
- Contract changes - signature, return type, error channel, status code, event shape
- Observability & DDT testability - state transitions, background tasks, retries, or async flows lacking logs, telemetry, or signals. Ask: "can a human tell if this succeeded without instrumenting it?" If no:
[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 context. For each Pass-1 suspicion:
- Try to DISPROVE it (negative verification). Re-read the
file + semantic anchor, look for a guard, an upstream check, a framework mitigation, or a contract that removes the risk.
- Blast Radius Rule: if a suspicion involves a contract change (signature, payload shape, exported type, event shape, error channel, status code), MUST run an external call-site search before resolving. Prefer
rg -n '<symbol>' -t ts -t js -t py -t php -t go -t rust; if shell rg is unavailable, use the host search tool or grep -rniE '<symbol>' and record the fallback. Verify at least one consumer. If skipped, stays UNRESOLVED and gets coverage-degraded.
- Mark each suspicion: CONFIRMED / REFUTED / UNRESOLVED.
- Refutation Ledger: REFUTED suspicions are not silently dropped. Write a ledger to
.goat-flow/logs/review/goat-review-refutations.<random>.txt. Each entry: original suspicion (verbatim), refuting evidence (file + semantic anchor), one-sentence rationale. Refuted suspicions do not appear in final output; the ledger is the audit trail.
- 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.
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 two orthogonal tags:
| Severity | Meaning |
|---|
| MUST | fix before merge; blocks approval |
| SHOULD | fix before merge unless disputed |
| MAY | nice-to-have |
| Action | Meaning |
|---|
| patch | fix direction is unambiguous - a coding agent can apply it |
| needs-decision | correct fix requires human input (policy, product call, trade-off) |
| pre-existing | bug exists in unchanged code (see separation below) |
| intent-mismatch | code is correct but does not match stated intent - needs author confirmation |
| needs-signal | code is a black box that degrades manual testability - needs emitted signal, log, or observable return value |
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.
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)
When the target is a codebase area (not a diff). For >20 files, recommend splitting. Two-pass discipline still applies per file cluster: skim the surface for suspicions, then open files for verification. Pre-existing issues ARE in scope (they are the point of an area audit).
BLOCKING GATE: Present findings and pause. If calibration is uncertain, consider /goat-critique.
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 (opt-in or auto-triggered)
Triggers 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.
Method: 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: Run the target auth check from references/refuter-spec.md first; 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)
Anti-hallucination surface -- tells the reader at a glance how confident the review is.
- Files opened in Pass 2: count / total. Paths read diff-only.
- Evidence tags: N OBSERVED / M INFERRED.
- Size: lines changed, files changed, chunking state. PR mode: resolved base, source annotation, short SHA.
- Scope snapshot: source, base, head, uncommitted, chunking.
- Refutations logged:
<N>
- Degradation flags:
chunked-partial, large-diff-unchunked, high-inference-ratio, files-not-opened, unfamiliar-area, missing-types, spec-drift-skipped, footguns-unread, not-reproduced-findings, coverage-degraded, configured-base-unresolved=<base>, base-detection-failed, base-fetch-skipped, base-fetch-failed, intent-unstated, cross-model-refuter-failed.
- Conclusion:
confident | coverage-degraded | high-inference | partial.
Never leave this section empty. "confident - no degradation flags" is the minimum.
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 run external call-site search for any contract-change suspicion before resolving (Blast Radius Rule); prefer
rg, fall back to host search or grep -rniE, and flag coverage-degraded if skipped
- MUST tag every surfaced finding with
[SEVERITY:ACTION]
- MUST grep
.goat-flow/learning-loop/footguns/ per finding; omit the tag on no direct match after the allowed reword
- MUST order findings by severity, not by file or discovery order
- MUST emit Review Integrity on every run
- MUST propose chunking when the diff exceeds 20 files OR 3000 changed lines
- MUST emit Spec Drift only when opt-in triggered; if skipped, log
spec-drift-skipped in Review Integrity
- MUST split Spec Drift output by direction: exit-criteria drift as
[advisory] (no severity tag), assumption invalidation as [MUST:needs-decision] under ## Findings, open-criterion satisfaction as [ready-to-tick]
- MUST store temporary review artifacts under
.goat-flow/logs/review/ with random suffix
- MUST attempt to disprove each Pass-1 suspicion during Pass 2
- MUST group 3+ related findings as systemic patterns
- 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: 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-only: <list or "none">)
- Evidence: <N> OBSERVED / <M> INFERRED
- Refutations logged: <N>
- Size: <files> files, <lines> lines (chunked: <group or "no">)
- 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
## Spec Drift <!-- only when opt-in triggered; otherwise omit and log spec-drift-skipped -->
<!-- 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 merged, 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**
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
