Review

<skill id="review"> <purpose> Review code changes for quality, correctness, security. Only report issues with >80% confidence.

Load: skills/testing/SKILL.md, skills/security/SKILL.md Reference: _meta/research/ai-code-anti-patterns.md </purpose>

<context> ai_code_stats: buggier: 1.7x more issues than human code security: 40-62% contain vulnerabilities findings: 10.83 per AI PR vs 6.45 human

priority: check Big 5 first (what AI actually breaks) </context>

<on_start>

agentdb read-start

<identify_scope>

gh pr diff {number}        # For PRs
git diff --staged          # For staged
git diff HEAD~1            # For recent

</identify_scope>

<deterministic_lane> Machines first, then judgment. Run before reading a single diff hunk:

"${CLAUDE_PLUGIN_ROOT:-.}/scripts/deterministic-review.sh" <repo-dir> [base-ref]

Parallel gitleaks / semgrep(p/security-audit) / eslint / ruff / shellcheck / actionlint / zizmor / osv-scanner — whatever is installed; diff-scoped when base-ref given; exit 1 on HIGH (secrets, security SAST, high CVEs). Feed findings.tsv into the review as ground truth. Lanes marked SKIPPED are reported as NOT CHECKED, never as clean. </deterministic_lane> </on_start>

<confidence_threshold>

Confidence	Category	Report?
95%+	Definite bug	YES
85-95%	Likely issue	YES
70-85%	Possible issue	MAYBE
<70%	Style preference	NO
</confidence_threshold>

<big5 name="BIG 5: AI-SPECIFIC CONCERNS"> Check these FIRST - what AI actually breaks: <check id="1_input_validation"> - Zod/Pydantic schema for every API endpoint? - Parameterized queries (no string concat)? - File uploads validated (size, type, extension)? detection: grep -r "req\.body" | grep -v "parse\|validate\|z\." </check> <check id="2_edge_cases"> - Null/undefined handling present? - Empty arrays handled (length check)? - Zero-length strings rejected? - Timeout handling for external calls? </check> <check id="3_error_handling"> - No empty catch blocks? - Errors logged with context? - User-facing messages generic? detection: grep -r "catch.*{}" (empty catch) </check> <check id="4_duplication"> - Same logic repeated in multiple places? - Should be extracted to shared utility? </check> <check id="5_complexity"> - Functions > 30 lines? - Nested ternaries > 2 levels? </check> </big5> <checklist> <section name="Logic & Correctness"> - [ ] Edge cases handled - [ ] Error paths covered - [ ] Null checks present - [ ] Type safety </section> <section name="Security"> - [ ] Input validation (Zod schema) - [ ] No hardcoded secrets - [ ] SQL injection prevented (parameterized queries) - [ ] XSS prevented (DOMPurify) - [ ] Auth tokens in httpOnly cookies </section> <section name="Performance"> - [ ] No N+1 queries - [ ] Appropriate caching </section> </checklist>

<refute_before_report> Before a finding enters the report, check it against reference/refutation-patterns.md (11 false-alarm families + 5 refutation moves). A finding matching a family needs evidence distinguishing it from the refuted precedent. Severity gates from the same file apply: falsification check before CRITICAL, consensus deflation, authority stripping on CVEs, and the judge's own additions get refuted too. Refuted candidates go in the report WITH their refutations; if the project keeps a refutation-corpus file, append new refuted false alarms to it in the same session. </refute_before_report>

<output_format> CODE REVIEW

Files: X changed Findings: Y (Z critical)

CRITICAL

[file:line] Issue (confidence: XX%) → Fix: suggestion

HIGH

[file:line] Issue (confidence: XX%) → Fix: suggestion

Summary: APPROVE | REQUEST CHANGES | COMMENT </output_format>

<ask_user> Use AskUserQuestion when: a finding is between 70-85% confidence (ambiguous) Ask: "Found {issue} at {file:line} (confidence {X}%). Intentional, or should I flag it?" Options: intentional — skip, flag it, investigate deeper </ask_user>

<verdict_rules> Review is finite because its purpose is AUTHORIZATION, not exhaustion. Approve once the change improves the health of the codebase, not once nothing else can be found. Nothing else can ever be found. (#204)

• APPROVE: no finding clears the block bar. Open non-blocking comments do NOT prevent APPROVE — say the issue number out loud and approve. Forcing another round to re-check a nit costs more than the nit.
• REQUEST CHANGES: at least one finding clears the block bar below.
• COMMENT: findings worth saying, none blocking, and the author asked for a read rather than a decision.

<block_bar> A finding blocks only with ALL of:

1. pasted output, not a described procedure
2. a distance proof threshold cleared — distance sets how much evidence is needed, never whether a finding may block: d0 changed code fails · d1 violates a declared invariant · d2 pre-existing defect this change exposed, needs a user-visible consequence · d3+ needs outside assumptions, blocks only on an executed demonstration or a cited prior failure. Never auto-close d3; taste never clears it, a real outcome does.
3. a named observable failure: predict what breaks and how we would see it
4. violates the acceptance profile: tag the finding's dimension, and let the profile's blocks_at map decide the threshold for that dimension. A production-hardening finding on a demo is not a blocker however real it is, and a privacy finding on a demo IS one when the profile says so. The stage label never decides; the dimensions do.

Everything else is filed with its distance:N label under the quarantine milestone. Recurrence is signal, silence is a verdict. </block_bar>

Read the acceptance record before reviewing: claims, declared invariants, and tradeoffs already accepted. If the commit carries one, it is FROZEN and adjudication will refuse a FAIL. Reopening takes new_failing_input, changed_dependency, missed_requirement, disproven_assumption, profile_changed, or owner_promotion. A new reviewer is not a reopen event. Never ask a reviewer whether criticism is complete; it cannot answer and will always say no. </verdict_rules>

<on_complete> Emit findings as a kernel.verdict/v1 document and let the acceptance function decide. The reviewer never grades its own completeness.

python3 scripts/adjudicate.py findings.json --text --strict   # 0 PASS · 1 FAIL · 2 INVALID
agentdb write-end '{"command":"review","verdict":"X","critical":N,"high":N,"big5_violations":N,"quarantined":N}'

cannot_falsify is mandatory and non-empty. State what this review structurally could not see (no real device, conformance only, no live data, a suite that did not finish). Silence about coverage reads as coverage. </on_complete>

</skill>