Nfs Review Project

Review project against architecture invariants

For projects scaffolded with nextjs-fullstack-starter. Detects drift from the patterns established by the scaffolder and the nfs-architecture-patterns skill.

Use this skill when

• User asks "does this project follow our conventions?"
• Before a PR review or merge.
• After a substantial feature lands, to catch newly-introduced drift.
• During onboarding — quick health check for an unfamiliar codebase.
• User invokes /nfs-review-project.
• User suspects something feels off architecturally.

Do NOT use this skill on:

• Projects NOT scaffolded with this plugin (the rules are tuned for the Server Components + Server Actions stack). If structure detection fails, refuse and explain.
• Projects in the middle of a partial migration — the noise drowns the signal.

The flow

1. Verify project structure. Confirm we're in a project this skill knows how to review.
2. Run scripts/check-conventions.sh. Mechanical, deterministic checks that take ~1s.
3. Parse the script's findings. Group by check type.
4. Sample-read flagged files. For checks that need judgment (e.g. "is this src/app/ page genuinely thin?"), read 5-10 of the flagged files and verify.
5. Sample-read passing files too. Spot-check 3-5 files that the script didn't flag, to catch issues the script can't see (silent permission bypasses, drift in module organization, smuggled state libs).
6. Produce the report. Markdown, grouped by severity.

Step 1 — Verify project structure

Refuse if any of these are missing:

src/server/modules/                          # service layer
src/server/actions/                          # Server Actions live here
src/app/                                     # delivery layer
src/server/auth/permissions.ts               # requirePermission helper
CLAUDE.md                                    # the project's contract

If src/server/api/trpc.ts is present, this is probably the tRPC variant — point the user at the sibling plugin instead:

This project has src/server/api/trpc.ts — it looks like it was scaffolded with nextjs-trpc-prisma-starter, not nextjs-fullstack-starter. The two have different invariants. Use /nts-review-project from the tRPC plugin instead.

If the layout is something else entirely, output:

This project doesn't match the layout nextjs-fullstack-starter expects (src/server/modules/, src/server/actions/, and CLAUDE.md are required). I won't review it with these rules — they'd produce noise.

Step 2 — Run the script

bash scripts/check-conventions.sh

The script outputs tagged lines like:

MISSING_SERVER_ONLY: src/server/modules/order/order.service.ts
DB_IN_APP: src/app/(dashboard)/orders/page.tsx:8
TRPC_IMPORT: src/server/modules/order/order.service.ts:3
WRONG_CACHE_PRIMITIVE: src/server/modules/customer/customer.service.ts:42:revalidateTag
SERVICE_NO_PERMISSION: src/server/modules/order/order.service.ts:markPaid
MUTATION_NO_AUDIT: src/server/modules/order/order.service.ts:markPaid
NO_TRANSACTION: src/server/modules/order/order.service.ts:bulkCreate
USE_SERVER_IN_PAGE: src/app/(dashboard)/orders/page.tsx:1
ACTION_NO_REVALIDATE: src/server/actions/customer.actions.ts:createCustomerAction

Plus a === Summary === block with counts.

Parse the output by tag. Don't trust the tags blindly — verify each finding by reading the actual file (especially for the heuristic ones like MUTATION_NO_AUDIT, which is a regex-based guess).

Step 3 — Sample-read flagged files

For each unique file the script flagged, read it and confirm the finding is real. Some checks (especially MUTATION_NO_AUDIT and SERVICE_NO_PERMISSION) use simple regex that can have false positives — e.g. a service method that's a pure read but follows a write-shaped name.

Use the nfs-architecture-patterns skill's references/ to remind yourself of the canonical shape before judging.

Step 4 — Sample-read passing files

The script can't catch:

• Silent permission bypasses (// @ts-ignore near a requirePermission call, or commented-out checks).
• Drift in module organization (a new "utils" folder at the top level instead of inside a module).
• Domain errors being thrown as raw Error everywhere instead of NotFoundError etc.
• Tests that test mocks instead of real behavior.
• A new dependency in package.json that doesn't fit the stack (a smuggled state library, an alternative ORM, etc.).
• A page that calls a service correctly but does additional DB work outside the service.

Spot-read:

• 2–3 service files that the script said are clean.
• 2–3 page files that look complex enough to hide business logic.
• The most-recently-modified file (whatever it is — ls -t to find).
• package.json for added deps that hint at architectural drift.

Step 5 — Produce the report

Use this exact format:

# Project review: <project-name>

**Mode detected:** Next.js fullstack (Server Components + Server Actions) ✓
**Files scanned:** N TS/TSX files across src/
**Date:** <YYYY-MM-DD>

## 🔴 Must fix (N)

Severity rule: missing security/correctness primitive that the rest of the codebase depends on.

### <Finding category>

- `<path>:<line>` — <one-line explanation>

## 🟡 Should fix (N)

Severity rule: pattern violation that doesn't break correctness today but will erode invariants if not addressed.

### <Finding category>

- `<path>:<line>` — <one-line explanation>

## 🟢 Notes (N)

Severity rule: a rule-break with a comment explaining intent, or a borderline case worth flagging.

## ✅ Passing

- N/N `src/server/` files have `import "server-only";`
- 0 tRPC imports (correct stack)
- 0 DB calls in `src/app/`
- All M Server Actions invalidate after mutation
- No smuggled state-management libraries

## Recommendations

<If "must fix" > 0:> Tackle 🔴 first — those are real holes.
<If patterns are repeated:> Pattern X appears in N files; consider a codemod or a project-wide refactor PR.
<If everything passes:> Healthy. Re-run after the next major feature.

Severity guide

See references/severity-guide.md for the full rubric. Quick reference:

• 🔴 Must fix — missing import "server-only";, missing requirePermission on mutation, missing auditLog on mutation, DB call in src/app/, tRPC imports (wrong stack), 'use server' at top of a page/component file.
• 🟡 Should fix — mutation outside transaction, raw Error instead of domain error, missing cache invalidation after mutation, page with non-trivial business logic, action that does DB work directly.
• 🟢 Notes — rule break with adjacent justification comment, intentional exception documented in CLAUDE.md.

What this skill explicitly does NOT do

• Lint / format / typecheck. That's pnpm verify. This skill is about architecture.
• Test coverage. Different concern — covered by nfs-testing-patterns.
• Performance review. N+1 queries, missing indexes — out of scope.
• Security audit beyond the boundaries. Doesn't check for SQL injection, XSS, CSRF. Trusts that the framework's defaults plus the boundary rules cover the basics.
• Code review of business logic. Doesn't comment on whether approve should fire before markPaid or whatever. Architecture only.

If the user wants any of those, point them at the right tool.