Skip to content

Commit adc046f

Browse files
Claude Code Botclaude
andcommitted
feat(code-critic): adversarial-reviewer v1.4.0 — production patterns, cross-file awareness, severity recalibration
4A: Added "Production Failure Patterns" to failure mode checklist — platform guard testing gaps, silent env var failures, test isolation (afterEach cleanup), selector/ID collisions, feature discoverability, async error boundary crossings, RLS policy changes (auto-Critical). 4B: Added "Cross-File Awareness" section with trigger-based review questions for removed user-facing strings, platform guards, shared state/ID modifications, and error handling changes. 4C: Severity recalibration — promoted missing afterEach cleanup, untested platform paths, unguarded env vars, and RLS changes to Critical. Promoted removed UI entry points and testID collisions to Concern. Part of dev-env Phase 1: Enhanced Local Review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 463caf5 commit adc046f

File tree

3 files changed

+28
-3
lines changed

3 files changed

+28
-3
lines changed

plugins/code-critic/.claude-plugin/plugin.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "code-critic",
33
"description": "Skeptical code review agent that assumes code is wrong until proven otherwise. Challenges architectural decisions, identifies failure modes systematically, and prioritizes long-term maintainability over validation.",
4-
"version": "1.3.0",
4+
"version": "1.4.0",
55
"author": {
66
"name": "Andrew Rich",
77
"email": "andrew.rich@gmail.com"

plugins/code-critic/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## v1.4.0
4+
5+
- **Production failure patterns** (4A): New failure mode category covering platform guard testing gaps, silent env var failures, test isolation (`afterEach` cleanup), selector/ID collisions, feature discoverability, async error boundary crossings, and RLS policy changes (auto-Critical)
6+
- **Cross-file awareness prompts** (4B): New section with trigger-based review questions — fires when diff removes user-facing strings, adds platform guards, modifies shared state/IDs, or changes error handling. Framed as questions, not assumptions
7+
- **Severity recalibration** (4C): Promoted to Critical: missing afterEach cleanup, untested platform paths, unguarded env vars, RLS changes. Promoted to Concern: removed UI entry points, testID collisions
8+
39
## v1.3.0
410

511
- **"Lead with verdict" resolved**: Automated pipelines state

plugins/code-critic/agents/adversarial-reviewer.md

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,16 @@ Do not rely on general skepticism. Systematically check for these categories whe
106106
- Missing correlation IDs or request tracing across service boundaries
107107
- Silent degradation — code that fails without anyone noticing
108108

109+
**Production Failure Patterns**
110+
111+
- Platform guards without dual-path testing (e.g., `Platform.OS === 'ios'` guard exists but only one path has test coverage)
112+
- Silent env var failures — code reads `process.env.X` without fallback or validation, fails silently when unset
113+
- Test isolation — missing `afterEach`/`teardown` cleanup that causes test pollution or flaky ordering dependencies
114+
- Selector/ID uniqueness collisions — `testID`, CSS selectors, or keys that collide across components or screens
115+
- Feature discoverability — removed or relocated entry points (buttons, navigation links, menu items) that make features unreachable
116+
- Async error boundaries across file boundaries — errors thrown in async code that cross module boundaries without being caught by any error boundary
117+
- RLS policy changes — any modification to Row-Level Security policies, permission checks, or access control rules (auto-Critical: these gate data access)
118+
109119
Skip categories that are clearly irrelevant to the code under review. Apply the relevant ones thoroughly.
110120

111121
## Context and Scope
@@ -120,6 +130,15 @@ Before forming opinions, understand the context:
120130
- **If reviewing a full file**: You have more context but may lack knowledge of callers and system integration. State this.
121131
- **If context is insufficient**: Use the Insufficient Context verdict rather than guessing.
122132

133+
## Cross-File Awareness
134+
135+
When reviewing diffs, apply these trigger-based questions. Frame as questions, not assumptions — you may not have full context.
136+
137+
- **When diff removes user-facing strings, buttons, links, or navigation targets**: "Is this feature still discoverable? What was the entry point, and does an alternative exist?"
138+
- **When diff adds platform guards** (`Platform.OS`, `navigator.userAgent`, feature flags): "Is the other platform/branch path tested?"
139+
- **When diff modifies shared state, IDs, keys, or selectors**: "What else uses this identifier? Could this cause a collision or stale reference?"
140+
- **When diff changes error handling** (try/catch, `.catch()`, error boundaries, fallback logic): "Where does this error propagate to? Is the caller prepared for the new error shape?"
141+
123142
## Behavioral Rules
124143

125144
- Ask "why" before "how." Challenge the premise before reviewing the solution.
@@ -193,9 +212,9 @@ The verdict tells the developer exactly what needs to happen next. Every review
193212

194213
Apply these standards consistently:
195214

196-
**Critical** (blocks merge): The code will break in production under realistic conditions. Data loss, security vulnerabilities, correctness bugs that affect users, unhandled failure modes that will cause outages.
215+
**Critical** (blocks merge): The code will break in production under realistic conditions. Data loss, security vulnerabilities, correctness bugs that affect users, unhandled failure modes that will cause outages. Also Critical: missing `afterEach`/teardown cleanup (causes flaky tests that mask real failures), untested platform paths (code ships with dead branches), unguarded env var reads that silently produce wrong behavior, and any RLS/permission policy changes.
197216

198-
**Concern** (should fix, doesn't block): Technical debt that will compound. Missing observability. Coupling that will make the next change painful. Performance issues that don't matter now but will at 10x scale.
217+
**Concern** (should fix, doesn't block): Technical debt that will compound. Missing observability. Coupling that will make the next change painful. Performance issues that don't matter now but will at 10x scale. Also Concern: removed UI entry points that may make features unreachable, and unchecked `testID`/selector collisions across components.
199218

200219
**Question** (needs justification): Design decisions that seem undermotivated. Trade-offs that weren't explained. Patterns that differ from the rest of the codebase without obvious reason.
201220

0 commit comments

Comments
 (0)