# Nfs Review Project > Review an existing project scaffolded with nextjs-fullstack-starter against the architectural invariants and patterns. Use this whenever the user wants to audit project conventions, asks 'does this follow our patterns', wants a pre-PR architecture check, suspects drift, is onboarding a new contributor, or invokes /nfs-review-project. Runs a mechanical check script for fast objective violations (missing server-only, DB queries in src/app/, missing requirePermission on mutations, missing audit, wrong cache primitives, tRPC imports from the wrong stack), then samples files for judgment calls, then produces a categorized report grouped by severity (must fix / should fix / notes / passing). - Skill: `juncoding/nfs-review-project` (Agent Skill, multi-file: 3 files) - Install (CLI): `npx skillmds@latest add juncoding/nfs-review-project` - Raw SKILL.md: https://api.skillmd.com/api/skills/juncoding/nfs-review-project/raw - Safety review: pending - Works with: Claude Code, Claude.ai, OpenAI Codex - Category: Security - Author: juncoding (https://skillmd.com/u/juncoding) - Updated: 2026-09-17 - Page: https://skillmd.com/skills/juncoding/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 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: ```markdown # Project review: **Mode detected:** Next.js fullstack (Server Components + Server Actions) ✓ **Files scanned:** N TS/TSX files across src/ **Date:** ## 🔴 Must fix (N) Severity rule: missing security/correctness primitive that the rest of the codebase depends on. ### - `:` — ## 🟡 Should fix (N) Severity rule: pattern violation that doesn't break correctness today but will erode invariants if not addressed. ### - `:` — ## 🟢 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 0:> Tackle 🔴 first — those are real holes. Pattern X appears in N files; consider a codemod or a project-wide refactor PR. 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.