/aos-review
Select the files to review (or paste code), then run /aos-review.
For targeted reviews: /aos-review architecture · /aos-review security · /aos-review tests · /aos-review naming
For deeper specialist reviews, chain with:
/aos-audit— full security audit/aos-qa— test suite quality/aos-frontend— UI component standards/aos-codereview— full PR decision with BLOCK/REQUEST/NIT
What gets checked
Architecture
- Business logic in services — not in controllers, components, or route handlers
- Controllers delegate in one line — no if/else, no DB calls, no calculations
- No direct DB access in controllers or UI components
- DTOs used for all API inputs/outputs — no raw entity objects exposed
- Feature-based folder structure — grouped by domain, not technical layer
- No circular dependencies between modules
- Domain errors thrown by services, not HTTP exceptions
API design
- Routes use nouns for resources, HTTP verbs for actions (
GET /orders, notGET /getOrders) - Collections return paginated responses — no unbounded list endpoints
- Consistent response envelope across all endpoints
- Breaking changes versioned — new behaviour behind
/v2/, old/v1/still works - Error responses include a code, message, and (where safe) details
Type safety
- No
anyin TypeScript — useunknownor a proper type - All function parameters and return types declared
- Domain IDs use branded types to prevent ID mixup
- No unsafe type assertions (
as X) without a comment explaining why
Security
- Inputs validated at the boundary (Zod / class-validator / Pydantic)
- Parameterised queries — no SQL string concatenation
- No hardcoded secrets, tokens, or API keys
- Auth guard on every state-changing or sensitive endpoint
- Sensitive fields stripped before response (password hashes, internal flags)
- Tokens in
httpOnlycookies, notlocalStorage - Resource ownership verified before read or mutation (IDOR check)
Naming and readability
- Names are accurate —
getUseronly gets, does not also mutate - No unexplained abbreviations:
usr,mgr,tmp,d,ts— spell them out - Booleans are positive and assertive:
isActive,hasPermission— notnotDisabled - No dead code — unreachable branches, unused exports, unused imports
- No
console.log/printdebug statements - No commented-out code blocks — delete or put behind a feature flag
Testing
- Unit tests for all service / use-case logic
- Test names describe behaviour:
should throw when email is already registered - Mocks only at system boundaries (HTTP, DB, external APIs)
- Error paths and edge cases covered — not just the happy path
- New behaviour has new tests — no uncovered code paths
Performance
- No N+1 query patterns
- No unbounded queries — pagination on all collections
- No synchronous I/O in async request handlers
- Expensive computations memoised or cached where appropriate
Dependencies
- New packages are actively maintained (last release < 12 months)
- No packages that duplicate something already in the project
- No
npm audithigh/critical vulnerabilities introduced
Output format
[FAIL] Architecture — business logic in controller
File: src/users/users.controller.ts:34
Fix: move validation and duplicate check to UserService.create()
Standard: playbooks/nestjs/architecture.md — Thin Controllers
[FAIL] API design — GET /getUsers is RPC-style, not RESTful
File: src/users/users.controller.ts:12
Fix: rename to GET /users
[WARN] Testing — no test for the duplicate email case
File: src/users/user.service.spec.ts
Fix: add it('should throw ConflictError when email is already registered')
[WARN] Naming — variable 'd' on line 47 holds a discount percentage
File: src/orders/order.service.ts:47
Fix: rename d → discountPercent
Summary: 2 FAIL · 2 WARN · 11 PASS
Full standards reference
See REFERENCE.md for annotated code examples for each rule.