code-review - SKILL.md Agent Skill

name: code-review description: Run a full-codebase audit of CachiBot using 8 parallel review agents (DRY, Security, API, Data, Frontend, Tests, State, Observability). Each agent reports findings with file paths and line numbers, compiled into a dated report.

Code Review — 8-Agent Sweep

Run a full-codebase audit of CachiBot using 8 parallel review agents. Each agent focuses on a specific quality concern and reports findings with file paths and line numbers. Results are compiled into a single dated report.

Instructions

Note today's date for the report header.
Spawn all 8 review agents in parallel using the Task tool (subagent_type: Explore). Each agent scans its focus area and returns findings rated Fix, Improve, or Note.
After all agents complete, compile their findings into a single report file at .reviews/YYYY-MM-DD-review.md (create the .reviews/ directory if it doesn't exist).
Print a summary to the user with totals per severity.

Agent Definitions

1. Tina — The DRY Enforcer

Scope: cachibot/ and frontend/src/ Looks for: Duplicated logic, copy-pasted code, missed shared abstractions

Functions or blocks that appear in 2+ files with identical or near-identical logic (same body, different name counts)
Pydantic models in models/ that share 80%+ of their fields — candidates for a base class
Repeated inline patterns: .env read/write, response formatting, validation checks, error response construction
React components that duplicate rendering logic instead of extracting a shared component
Zustand stores with duplicated fetch/loading/error patterns that could use a shared factory or helper
Constants or config values hardcoded in multiple places instead of a single source of truth

2. Marcus — The Security Auditor

Scope: Full codebase Looks for: Auth gaps, injection vectors, data leaks, OWASP top 10

Endpoints missing auth dependencies (Depends(get_current_user) or role checks) — every route should require auth unless explicitly public
Route guards on the frontend — can a non-admin navigate to admin pages by typing the URL directly?
Injection risks in any string interpolation touching .env files, database queries, shell commands, or file paths
API keys or secrets that could leak in response payloads, error messages, or logs
CORS configuration — is it locked to expected origins or wide open?
WebSocket auth — does the WS handshake validate tokens the same way REST endpoints do?
Hardcoded secrets, tokens, or passwords anywhere in the codebase
File operation paths — can user input escape the workspace sandbox?

3. Priya — The API Consistency Reviewer

Scope: cachibot/api/routes/ Looks for: Inconsistent endpoint patterns, missing validation, API design issues

Inconsistent response shapes — do all endpoints follow the same success/error response pattern?
Missing input validation — are request bodies validated with Pydantic models, or do some endpoints accept raw dicts?
HTTP status code correctness — 200 vs 201 for creation, 404 vs 400 for bad input, proper use of 403 vs 401
Missing or inconsistent error handling — do all endpoints catch expected failures and return structured error responses?
Endpoint naming conventions — RESTful consistency across all routers
Missing response_model declarations on endpoints that return data
Deprecated endpoints still active without migration path or warnings

4. Carlos — The Data Layer Inspector

Scope: cachibot/storage/ and any database-touching code Looks for: Database issues, unsafe queries, missing error handling in storage

Raw SQL without parameterized queries (SQL injection risk)
Missing error handling around database operations — are connection failures, constraint violations, and timeouts caught?
Transactions that should be atomic but aren't wrapped properly
Missing indexes on columns used in WHERE/ORDER BY clauses
Schema inconsistencies between what the code expects and what migrations create
Repository methods that duplicate query logic instead of composing
Connection pool or lifecycle issues — are connections properly closed on error paths?

5. Zoe — The Frontend Quality Checker

Scope: frontend/src/ Looks for: Component issues, accessibility, React anti-patterns

Components that are too large (>200 lines) and should be split
Missing error boundaries — what happens when a component throws?
Accessibility gaps — missing aria labels, keyboard navigation, focus management in modals/dialogs
Hardcoded strings that should be in constants or i18n
Inline styles that should be in CSS/Tailwind classes
Props being drilled through 3+ levels instead of using context or stores
useEffect with missing or incorrect dependency arrays
Components re-rendering unnecessarily — missing memo, unstable references in deps

6. Derek — The Test Coverage Analyst

Scope: tests/ and full codebase Looks for: Missing tests, weak assertions, untested paths

Which endpoints in api/routes/ have zero test coverage?
Which utility functions and service methods lack unit tests?
Are error paths tested, or only happy paths?
Do integration tests exist for critical flows (auth, chat, WebSocket)?
Are there tests that assert on implementation details instead of behavior (brittle tests)?
Frontend: are there any component or store tests? What's missing?
Are test fixtures/factories available, or does every test build its own data from scratch?

7. Luna — The State & Integration Reviewer

Scope: frontend/src/stores/, frontend/src/api/ Looks for: State management issues, frontend-backend contract mismatches

Zustand stores with stale state bugs — data fetched but never refreshed, or refreshed too aggressively
API client methods that don't handle errors consistently — some throw, some return null, some swallow
Type mismatches between frontend TypeScript interfaces and backend Pydantic models — fields that exist on one side but not the other
localStorage persistence of sensitive data (tokens, keys, admin state)
WebSocket reconnection logic — does the client recover from disconnects gracefully?
Race conditions — multiple concurrent requests updating the same store slice
Loading/error states that are missing or inconsistent across views

8. Oscar — The Observability Reviewer

Scope: Full codebase Looks for: Silent failures, missing logs, poor error messages, bare excepts

Bare except Exception or except: blocks that swallow errors silently — every catch should log or re-raise
Important operations with no logging at all (user creation, config changes, provider key updates)
Log messages that are too vague to be useful ("error occurred" with no context)
Error responses that expose internal details (tracebacks, file paths, SQL) to the client
Missing logger = logging.getLogger(...) in modules that should have one
Inconsistent log levels — warnings that should be errors, debug that should be info
Health check gaps — what breaks silently with no way to detect it?

Report Format

After all agents finish, write .reviews/YYYY-MM-DD-review.md with this structure:

# CachiBot Code Review — YYYY-MM-DD

**Reviewers:** Tina, Marcus, Priya, Carlos, Zoe, Derek, Luna, Oscar
**Scope:** cachibot/, frontend/src/, desktop/

## Summary

| Severity | Count |
|----------|-------|
| Fix      | N     |
| Improve  | N     |
| Note     | N     |

## Tina — DRY

### Fix
- `path/to/file.py:42` — Description

### Improve
- `path/to/file.ts:108` — Description

### Note
- `path/to/file.py:15` — Description

## Marcus — Security

(same structure)

... (all 8 agents)

## Previous Reviews

(Link to or diff against the last review file in .reviews/ if one exists,
noting which prior findings were fixed and which are still open.)

Severity Definitions

Fix — Real bug, security issue, or maintenance risk. Should be fixed now.
Improve — Worth doing, not urgent. Better patterns, cleaner code.
Note — Awareness item. No action needed yet, but worth knowing about.