| name | review-service |
| description | Deep code review for a backend service and/or the frontend. Checks security vulnerabilities, performance, architecture compliance, code cleanliness, DB access patterns, messaging, auth, tests, and production readiness. Triggers on "review service", "review frontend", "audit", "check quality", or "/review-service". |
Service / Frontend Review
Deep review of a backend service and/or the frontend. Analyzes code against project conventions (CLAUDE.md + .claude/rules/), security best practices, and architectural rules.
Output: HTML report per target. Each reviewing agent writes a structured docs/reviews/YYYY-MM-DD-{Target}.json (findings list, schema below), then renders it via the shared generator:
python3 ~/.claude/skills/_shared/render-report.py docs/reviews/YYYY-MM-DD-{Target}.json
In chat, the agent prints only an executive summary (top 3 blockers, counts by severity, HTML path). The full review lives in the browser with filters by severity / category / text search. Markdown dumps in chat are noise for any non-trivial review (>10 findings).
Severity vocabulary (use exactly these strings in JSON):
blocker — must fix before merge (security holes, data loss, broken contracts)high — should fix before merge (architectural violations, N+1 on hot paths, missing entitlement checks)medium — fix this sprint (code quality, missing tests on non-critical paths)low — nice to have (style, docs, micro-perf)info — observation only, no action required
Required JSON fields per finding: severity, category (e.g. security, architecture, database, performance, messaging, logging, tests, production), title, file, line, summary, recommendation. Optional: snippet, risk. Full schema: ~/.claude/skills/_shared/render-report.py docstring.
Usage
/review-service ServiceName # Review one backend service
/review-service frontend # Review frontend
/review-service --all # Review all backend services + frontend in parallel
/review-service --backend # Review all backend services only
/review-service Svc1 Svc2 frontend # Review specific targets
Service Discovery
Do NOT hardcode a service list. Discover backend services dynamically by your project's convention. For a typical backend/<Service>/... layout:
find backend -maxdepth 3 -type d -name "*.Web"
find backend -maxdepth 2 -type d
Always pass the service path (not just the name) so the agent can locate sources regardless of internal layout (src/ vs flat).
Frontend path: frontend/ (or per your project's convention).
Workflow
digraph review {
"Parse args" -> "Discover services";
"Discover services" -> "Resolve targets";
"Resolve targets" -> "Launch agents\n(parallel, one per target)";
"Launch agents\n(parallel, one per target)" -> "Collect results";
"Collect results" -> "Print summary\n+ report paths";
}
Execution
Backend Service Review
For each backend service, launch a code-reviewer (or microservice-reviewer if your project has the dedicated agent) with the Backend Agent Prompt below.
Frontend Review
For the frontend, launch a code-reviewer agent with the Frontend Agent Prompt below.
Parallelism
Launch ALL target agents in a single message with multiple Agent tool calls, so reviews run concurrently. Do not wait for one review to finish before kicking off the next.
After Agent(s) Complete
Print a summary table with HTML paths:
| Target | Blocker | High | Med | Low | Report |
|--------------------------|---------|------|-----|-----|-------------------------------------------------|
| AuthService | 1 | 4 | 7 | 2 | docs/reviews/YYYY-MM-DD-AuthService.html |
| frontend | 0 | 3 | 5 | 4 | docs/reviews/YYYY-MM-DD-frontend.html |
For each row, list the top blocker (file:line + title) so the user can decide what to fix without opening the HTML. If Blocker > 0, flag the row for immediate attention.
Selective Fix
After reviewing, the user can ask to fix specific items. Read the report, find the referenced items, apply fixes.
Backend Agent Prompt
Include this full prompt when launching the agent. Replace {ServiceName} and {ServicePath}:
Review {ServiceName}. Service path: {ServicePath}.
Read the project's CLAUDE.md, backend/CLAUDE.md, and the service's own CLAUDE.md (if it exists) before starting.
## Output
Write findings as JSON to `docs/reviews/YYYY-MM-DD-{ServiceName}.json` (one object, schema below), then render the HTML report:
```bash
python3 ~/.claude/skills/_shared/render-report.py docs/reviews/YYYY-MM-DD-{ServiceName}.json
```
## JSON schema
```json
{
"title": "{ServiceName} review",
"scope": "{ServicePath}",
"generatedAt": "<ISO 8601>",
"findings": [
{
"id": "F-001",
"severity": "blocker | high | medium | low | info",
"category": "security | architecture | database | performance | messaging | logging | tests | production",
"title": "<short imperative>",
"file": "<relative path from repo root>",
"line": <int, optional>,
"summary": "<one paragraph: what + why>",
"snippet": "<optional code excerpt>",
"recommendation": "<concrete fix>",
"risk": "low | medium | high (optional)"
}
]
}
```
In chat, after the HTML is written, reply with:
1. Counts by severity (one line)
2. Top 3 blockers (file:line + title)
3. Absolute HTML path
## Review Checklist
Review EVERY item below. Report violations as findings. Skip a category in the report only if it has no issues.
### 1. Security & Vulnerabilities
- [ ] **SQL injection** — All raw SQL uses parameterized queries, never string interpolation/concatenation with user input
- [ ] **Authorization on every endpoint** — Every endpoint has an explicit permission attribute or an explicit `AllowAnonymous`. No unprotected endpoints that should be protected
- [ ] **Permission constants** — Uses centralized permission/role constants, never hardcoded role/permission strings
- [ ] **IDOR** — Endpoints that operate on user-owned resources verify ownership (user id from claims matches resource owner)
- [ ] **Mass assignment** — DTOs / commands don't expose fields that shouldn't be user-settable (Role, IsAdmin, CreatedBy)
- [ ] **Secrets in code** — No hardcoded passwords, API keys, connection strings, or tokens in source files
- [ ] **Input validation** — Every command / query has a validator. Validators check string lengths, ranges, required fields, format constraints
- [ ] **File upload safety** — File uploads validate content type, size limits, and file extension. No path traversal in filenames
- [ ] **Rate limiting** — Auth-sensitive endpoints (login, OTP, password reset) have rate limiting or are behind one
- [ ] **Content access on list endpoints** — List endpoints that don't call the per-item entitlement checker MUST filter by access level in SQL. Anonymous → only public. Authenticated → public + registered. Never include gated tiers without server-verified entitlement
- [ ] **No client-supplied entitlement IDs** — Endpoints must NEVER accept enrollment IDs or course IDs from query params to determine content access. Resolve server-side
- [ ] **Detail endpoints check entitlements** — Endpoints returning full content (body, markdown, video) must call the entitlement checker. Admin bypasses automatically
### 2. Architecture & Code Structure
- [ ] **Vertical slice pattern** — Each use case file has Command / Query + Validator + Endpoint + Handler. No logic leaking between slices
- [ ] **Clean Architecture layers** — Domain has no infrastructure dependencies. Core defines interfaces, Infrastructure implements them. Web only does DI and pipeline config
- [ ] **No business logic in endpoints** — Endpoints only parse request, dispatch command / query, return result
- [ ] **No business logic in DbContext / migrations** — DbContext for configuration only. Migrations for schema changes only
- [ ] **Repository pattern for data access** — ALL database calls during business operations go through repository interfaces defined in Core and implemented in Infrastructure. Handlers never use DbContext / IDbConnection directly. Exception: TransactionManager / UnitOfWork. CRITICAL if broken
- [ ] **No service-to-service DB access** — Services access other services' data only via typed HTTP clients (Contracts), never by querying another service's schema directly
- [ ] **DI registration centralized** — One `Registration.cs` per layer. No service registration scattered across files
- [ ] **Domain entity encapsulation** — Entities use private constructors + static `Create()` factories returning `Result<T, Error>` (or your project's monad). State changes through methods that validate
### 3. Database & Data Access
- [ ] **Repository interface in Core** — Every repository has an interface in Core and implementation in Infrastructure
- [ ] **CQRS compliance** — Writes via ORM (through repositories), complex reads via raw SQL helper (through read-only services). No mixing in a single operation
- [ ] **Time-ordered UUIDs** — Production code uses time-ordered UUID (UUIDv7 / `Guid.CreateVersion7()`), not random UUIDs (tests are exempt)
- [ ] **Naming convention** — Tables / columns follow project convention (snake_case for custom tables, etc.)
- [ ] **Missing indexes** — Frequently queried columns (foreign keys, status fields, lookup fields) have appropriate indexes
- [ ] **N+1 queries** — No loops executing a query per iteration. Use batch fetching, joins, or eager loading
- [ ] **Unbounded queries** — All list queries have pagination (skip/take or cursor). No `ToList()` without limits on potentially large tables
- [ ] **Connection / transaction management** — No manual open/close. Let ORM manage. Transactions scoped via TransactionManager
- [ ] **Migration immutability** — Existing migrations not modified or deleted. New migrations only
### 4. Performance & Optimization
- [ ] **Caching where appropriate** — Frequently accessed, rarely changing data uses a tiered cache (Redis + local). Invalidation correct
- [ ] **Cache decorator parity** — If a typed HTTP client has a cache decorator, every method on the interface is in the decorator too
- [ ] **Async all the way** — No `.Result`, `.Wait()`, `.GetAwaiter().GetResult()` on async calls (deadlock risk)
- [ ] **Streaming for large data** — File downloads / uploads use streaming, not loading entire content into memory
- [ ] **Select only needed columns** — Raw SQL queries select specific columns, not `SELECT *`. ORM uses projections where appropriate
- [ ] **GET query optimization** — Read endpoints use `const string` SQL (not interpolated) for query plan caching. Complex queries with conditional WHERE/ORDER use pre-built const variants
- [ ] **Feed / list queries minimize JOINs** — UI feed queries avoid unnecessary JOINs; use `DISTINCT ON` or subqueries when an entity belongs to multiple parents
### 5. Messaging & Integration
- [ ] **Outbox pattern** — Integration events published via outbox within the same transaction as the domain change. No direct broker publishing
- [ ] **Routing key conventions** — Keys follow `{entity}.{action}[.{target}]` pattern. Match `MESSAGING_CONVENTIONS.md`
- [ ] **Idempotent consumers** — Handlers safe to replay. Use deduplication or upsert
- [ ] **No domain events from integration handlers** — Wrong transaction context
- [ ] **Transport isolation in tests** — Tests stub broker transports and create outbox schema tables
### 6. Error Handling & Logging
- [ ] **Result pattern everywhere** — Business operations return `Result<T, Error>` (or your project's monad). No `throw` for expected business errors
- [ ] **User-facing error messages** — In the project's user language (e.g. Russian); error code in dot-separated lowercase English
- [ ] **Structured logging** — Uses `ILogger<T>` with structured parameters `{ParamName}`, not string interpolation. Log levels appropriate (Information for business events, Warning for recoverable issues, Error for failures)
- [ ] **No swallowed exceptions** — Catch blocks either log + rethrow, log + return error, or handle meaningfully. No empty catches
- [ ] **Sensitive data not logged** — No passwords, tokens, PII
### 7. Test Coverage
- [ ] **Integration tests exist** — Every public endpoint / major use case has at least one integration test
- [ ] **Test isolation** — Each test resets DB via Respawn (or equivalent). No test-to-test dependencies
- [ ] **Auth testing** — Tests cover both authorized and unauthorized access. Tests use `AuthenticateAs()` helper
- [ ] **Edge cases** — Tests cover validation failures, not-found, unauthorized, concurrent access
- [ ] **Testcontainers setup** — Tests use real Postgres via Testcontainers, not in-memory DB or mocks for data access
### 8. Production Readiness
- [ ] **Health checks** — Service has health check endpoints for DB and external dependencies
- [ ] **Configuration** — Secrets via env vars / secrets manager, non-secrets in appsettings. No hardcoded config
- [ ] **Docker** — Dockerfile follows conventions (healthcheck, multi-stage build)
- [ ] **OpenAPI** — All endpoints have proper OpenAPI metadata (tags, response types)
- [ ] **Observability** — OTEL tracing configured. Important operations have custom spans / metrics where needed
Frontend Agent Prompt
Include this full prompt when launching the frontend agent:
Review the frontend. Path: frontend/.
Read frontend/CLAUDE.md and the project root CLAUDE.md before starting.
## Output
Write findings as JSON to `docs/reviews/YYYY-MM-DD-frontend.json`, then render the HTML report:
```bash
python3 ~/.claude/skills/_shared/render-report.py docs/reviews/YYYY-MM-DD-frontend.json
```
## JSON schema
```json
{
"title": "Frontend review",
"scope": "frontend/",
"generatedAt": "<ISO 8601>",
"findings": [
{
"id": "F-001",
"severity": "blocker | high | medium | low | info",
"category": "security | architecture | performance | code-quality | ux | auth | tests | production",
"title": "<short imperative>",
"file": "<relative path from repo root>",
"line": <int, optional>,
"summary": "<one paragraph>",
"snippet": "<optional code excerpt>",
"recommendation": "<concrete fix>",
"risk": "low | medium | high (optional)"
}
]
}
```
Severity vocabulary matches the backend prompt: `blocker | high | medium | low | info`. Layer-hierarchy violations and upward imports = `blocker` (mechanically prevented by ESLint, so any occurrence means a rule was bypassed). Manual memoization (`useCallback` / `useMemo` / `React.memo`) under React Compiler 19 = `blocker` — it actively confuses the compiler.
In chat after the HTML is written, reply with:
1. Counts by severity (one line)
2. Top 3 blockers (file:line + title)
3. Absolute HTML path
## Review Checklist
Review EVERY item below. Report violations as findings.
### 1. Security & Vulnerabilities
- [ ] **XSS** — No unsanitized user input rendered as raw HTML. Markdown uses a safe renderer with sanitization
- [ ] **Token exposure** — Auth tokens not stored in localStorage. No tokens in URLs or logs
- [ ] **CSRF** — Forms use proper CSRF protection where needed
- [ ] **Sensitive data in client bundle** — No secrets, API keys, or internal URLs in `NEXT_PUBLIC_*` (or equivalent public vars) that shouldn't be public
- [ ] **Open redirects** — Login / callback redirect URLs validated against allowlist, not taken raw from query params
- [ ] **Dependency vulnerabilities** — No known vulnerable packages
### 2. Architecture (project convention)
- [ ] **Layer hierarchy** — Imports only flow downward per your architecture (e.g. FSD: app → pages → widgets → features → entities → shared). No upward imports. CRITICAL if broken
- [ ] **No cross-slice imports** — `features/A` never imports from `features/B`; shared logic in `entities` or `shared`
- [ ] **Public API via `index.ts`** — External consumers import from slice barrel, not internal files
- [ ] **Route layout structure** — Pages use composition / slot pattern. Layouts don't import feature components directly
- [ ] **Entity layer pattern** — Each entity has `api.ts` (query options factories), `types.ts`, `index.ts`
- [ ] **Mutation hooks** — One file per mutation: `use-{action}-{entity}.ts` in the feature's `model/`. Handles toast + cache invalidation
### 3. Performance & Optimization
- [ ] **No manual memoization** — React Compiler enabled: no `useCallback`, `useMemo`, `React.memo`. CRITICAL if used
- [ ] **No object / array mutation** — Always return new references; in-place mutation breaks React Compiler assumptions
- [ ] **Bundle size** — No large libraries imported in client components that should be code-split. Heavy components use `dynamic()` or `React.lazy()`
- [ ] **Image optimization** — Uses `next/image`, not raw `<img>`. Proper width / height or fill mode
- [ ] **Query optimization** — TanStack Query stale times appropriate. No redundant refetches. Proper `queryKey` for cache isolation
- [ ] **Unbounded lists** — Lists with potentially many items use pagination or virtualization
### 4. Code Quality & Patterns
- [ ] **Route constants** — All routes use `routes.*` from `shared/config/routes.ts`, never hardcoded path strings
- [ ] **Trailing slashes in API URLs** — All API URLs include trailing slash if your gateway 301s without it
- [ ] **Error display** — Uses central error formatter `getErrorMessage(error, "fallback")`, never raw `error.message`
- [ ] **Toast style** — Consistent voice (short success, "Error {action}" template)
- [ ] **Consistent naming** — Files: kebab-case. Components: PascalCase. Hooks: `use-{name}.ts`. Types: PascalCase
- [ ] **No dead code** — No unused imports, components, hooks, or commented-out code blocks
- [ ] **TypeScript strictness** — No `any`, no `@ts-ignore` without justification, no type assertions where proper typing is possible
### 5. Error Handling & UX
- [ ] **Loading states** — Async operations show loading indicators. Pages use `loading.tsx` Suspense boundaries
- [ ] **Error boundaries** — Route segments have `error.tsx` for graceful recovery
- [ ] **Form validation** — Forms use `react-hook-form` + Zod (or equivalent). Server errors mapped via `setServerErrors()`
- [ ] **Optimistic updates** — Where appropriate, mutations use optimistic updates with rollback
- [ ] **Empty states** — Lists handle empty state with a meaningful message
### 6. Auth & Access Control
- [ ] **Protected routes** — Middleware redirects unauthenticated users. No auth-required pages accessible without login
- [ ] **Role-based UI** — Uses central `Can` component or `useRoles()` hook. No hardcoded role string literals
- [ ] **Token sync** — Token sync runs before session guard and authenticated queries. No race conditions
- [ ] **Session handling** — Logout clears all state. Session expiry redirects to login
- [ ] **Feed components filter by access level** — Frontend lists don't display content the user cannot access
- [ ] **No client-side access decisions** — Access decisions come from the backend. Frontend only controls UI visibility based on backend-provided flags
### 7. Test Coverage
- [ ] **Unit tests exist** — Critical utilities, hooks, and business logic have tests (Vitest / Jest)
- [ ] **Component tests** — Key interactive components have render tests
- [ ] **API mocking** — Tests mock API calls properly, not implementation details
### 8. Production Readiness
- [ ] **Build succeeds** — `npm run build` passes without errors
- [ ] **Lint clean** — `npm run lint` has no errors
- [ ] **Environment config** — All env vars documented. No missing public vars in production
- [ ] **Hydration safety** — No client-only APIs (`window`, `localStorage`) accessed during SSR without guards
- [ ] **Middleware** — Auth middleware covers all protected routes. Public routes explicitly excluded
