docs: codify audit rules in CLAUDE.md + README + audit records
CLAUDE.md: new "2026-06-09 audit" enforced-rules section (mandatory coercion helpers, deterministic id-tiebreak ordering, $queryRaw FOR-UPDATE locking discipline, uniqueness-in-transaction, guard reads, Rules-of-Hooks, seed prod guard), lint/typecheck/format commands, and the app_test/.env.test testing section with the hard-throw guard. README rewritten from the placeholder stub (setup/run/build/test/gates/migrations). REVIEW.md / REVIEW_FINDINGS.md / REVIEW_FIXES.md: the full audit record — 277 files reviewed, ~362 findings, every one fixed and verified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
BOHA
51
CLAUDE.md
51
CLAUDE.md
@@ -76,9 +76,14 @@ npm run build:client # vite build → dist-client/
|
||||
npm start # node dist/server.js
|
||||
|
||||
# Tests
|
||||
npm test # vitest run (single pass)
|
||||
npm test # vitest run (single pass) — runs against the app_test DB via .env.test
|
||||
npm run test:watch # vitest watch
|
||||
|
||||
# Quality gates
|
||||
npm run typecheck # tsc -b --noEmit (also type-checks the tests via tsconfig.test.json)
|
||||
npm run lint # eslint . — react-hooks/rules-of-hooks is an ERROR; keep at 0 errors
|
||||
npm run format # prettier --write .
|
||||
|
||||
# Database
|
||||
npx prisma migrate dev # Create migration from schema changes + apply to dev
|
||||
npx prisma migrate dev --name <descriptive_name> # Named migration
|
||||
@@ -239,11 +244,13 @@ if ("error" in body) return error(reply, body.error, 400);
|
||||
|
||||
## Testing
|
||||
|
||||
Tests live in `src/__tests__/`. They use Vitest + Supertest against a real test database (`.env.test`).
|
||||
Tests live in `src/__tests__/`. They use Vitest + Supertest against a **real, throwaway test database** (`app_test`).
|
||||
|
||||
- The suite spans 17 files / 195 tests — auth, numbering, warehouse, plan, invoices, exchange-rates, schema coercion, NAS file manager, env, manual-create, AI/Odin (conversation CRUD, isolation, auto-title, cascade, budget), received-invoices VAT. Server-side only (no component tests yet).
|
||||
- **Isolated test DB (`app_test`):** the suite MUTATES a real MySQL DB, so it runs against `app_test` (NOT dev `app`), configured in **`.env.test`** (gitignored). `src/__tests__/setup.ts` **hard-throws** if `DATABASE_URL` doesn't name a `*test*` database — a stray URL can never corrupt dev/prod data. To (re)create it: `CREATE DATABASE app_test;` → copy `.env` to `.env.test` with the DB name swapped + `ANTHROPIC_API_KEY=` blanked → `DATABASE_URL=<app_test-url> npx prisma migrate deploy` → `DATABASE_URL=<app_test-url> npx tsx prisma/seed.ts`.
|
||||
- The suite spans 18 files / 247 tests — auth (incl. happy-path login/refresh-rotation), numbering, warehouse (incl. FIFO oldest-first), plan, invoices, exchange-rates, schema coercion, NAS file manager, env, manual-create, AI/Odin, received-invoices VAT. Server-side only (no component tests yet).
|
||||
- Tests are now **type-checked** by `tsc -b` (via `tsconfig.test.json`) — keep them compiling.
|
||||
- Use `buildApp()` helper to spin up the Fastify instance for tests.
|
||||
- Tests use `vitest.config.ts` with `environment: 'node'` and 15s timeout.
|
||||
- Tests use `vitest.config.ts` with `environment: 'node'` and 15s timeout; `fileParallelism: false` (serialized) to avoid `number_sequences` deadlocks.
|
||||
- **Do not mock Prisma** — tests hit a real database to catch schema/query bugs.
|
||||
|
||||
When adding new features, add tests in `src/__tests__/`. Name test files `<feature>.test.ts`.
|
||||
@@ -302,7 +309,7 @@ The admin frontend is **fully on Material UI v7** (Emotion). The old ~7,100 line
|
||||
- **Theme is single-source:** `src/context/ThemeContext.tsx` owns the `<html data-theme>` attribute + the View-Transitions cross-fade, and persists under MUI's **`mui-mode`** localStorage key (the same key MUI reads on mount). Do NOT add a second theme key — that desyncs page vs toggle on refresh.
|
||||
- **Dialogs** lock `<html>` via `useDialogScrollLock` (MUI only locks `<body>`, and `html{overflow-x:hidden}` makes `<html>` the scroller); Modal/ConfirmDialog freeze title/label/loading through the close fade so nothing flashes. Login renders OUTSIDE AppShell.
|
||||
|
||||
**Gates every change:** `npx tsc -b --noEmit` (NOT `-p tsconfig.json` — a vacuous solution file that checks nothing), `npm run build`, `npx vitest run`.
|
||||
**Gates every change:** `npx tsc -b --noEmit` (NOT `-p tsconfig.json` — a vacuous solution file that checks nothing), `npm run build`, `npx vitest run`, `npm run lint`. The solution `tsconfig.json` now references `tsconfig.test.json` too, so `tsc -b` **type-checks the test files** (previously skipped). ESLint (flat config in `eslint.config.mjs`) enforces `react-hooks/rules-of-hooks` as an **error** — that rule catches the "hook after an early `return`" bug class; keep `npm run lint` at zero errors (warnings are advisory). Prettier config is `.prettierrc.json` (`npm run format`).
|
||||
|
||||
---
|
||||
|
||||
@@ -445,6 +452,40 @@ audit found and fixed the deviations. Full report: `docs/codebase-audit-2026-06-
|
||||
- **Attendance/leave** writes `@db.Date` at **local noon** (`new Date(y, m, d, 12, 0, 0)`).
|
||||
- **Frontend "today" / date-string round-trips:** use `utils/date.ts` `localDateStr` (server) / `normalizeDateStr` (client). **Never** use `new Date().toISOString().split("T")[0]` for "today" — it's the UTC date and is a day early during the late-evening Prague window.
|
||||
|
||||
## Conventions (enforced — added by the 2026-06-09 full audit)
|
||||
|
||||
The 2026-06-09 file-by-file audit traced most bugs to a handful of patterns. These are now rules — `npm run lint` + `tsc -b` (which now type-checks tests) catch some automatically.
|
||||
|
||||
**Zod validation — shared coercion helpers are MANDATORY**
|
||||
|
||||
- All numeric/boolean/date/email form fields MUST use the helpers in **`src/schemas/common.ts`**: `numberFromForm`, `numberInRange(min,max)`, `nonNegativeNumberFromForm`, `positiveNumberFromForm`, `intIdFromForm`, `nullableNumberFromForm`/`nullableIntIdFromForm`, `booleanFromForm`, `isoDateString`, `timeString`, `emailOrEmpty`.
|
||||
- **NEVER** the raw `z.union([z.number(), z.string()]).transform(Number)` idiom — it silently yields `NaN` that flows into Prisma/business math (this was the single biggest bug class). FK ids → `intIdFromForm`/`nullableIntIdFromForm`; quantities/prices → `nonNegative`/`positive`; bounded values (VAT `0–100`, month `1–12`) → `numberInRange`.
|
||||
- `isoDateString`/`timeString` are **lenient** (they strip a trailing time/seconds component) because an edit form re-submits a `@db.Date` that the `toJSON` override serialised as `"YYYY-MM-DDT00:00:00"`. Use them for date/time fields, not a bare regex.
|
||||
- An optional email a form may submit as `""` → **`emailOrEmpty.nullish()`** (a bare `.email()` 400s the whole form, blocking unrelated saves).
|
||||
|
||||
**Deterministic ordering — always tiebreak a timestamp sort with `id`**
|
||||
|
||||
- `created_at`/`received_at` are **second-precision** (`@db.Timestamp(0)`/`DateTime(0)`), so `orderBy: { created_at: "desc" }` alone is non-deterministic for same-second rows. ALWAYS add `{ id: "desc" }` (or `asc`) as the secondary sort. (Bit `plan.service.resolveCell`/`resolveGrid` and warehouse FIFO `selectFifoBatches`.)
|
||||
|
||||
**Concurrency — locking discipline for stock / balance / sequence mutations**
|
||||
|
||||
- Any service op that read-modify-writes shared rows (stock, batches, reservations, balances, number sequences) MUST: run inside a `prisma.$transaction`; take the row lock with `SELECT … FOR UPDATE` via **`tx.$queryRaw`** (NOT `$executeRaw` — it does not reliably hold the lock); and lock rows in one global **ascending-id order** (parent → items → batches) so concurrent paths can't deadlock. See `warehouse.service.ts` `lockParentRow`/`lockRowsForUpdate` and `attendance.service.ts` `lockUserRow`.
|
||||
- **Uniqueness checks** (username/email/document number) go INSIDE the create transaction (re-check immediately before insert) and catch `P2002` → 409. A pre-transaction check is a TOCTOU race that surfaces as a 500.
|
||||
|
||||
**Permissions — guard reads, not just writes**
|
||||
|
||||
- GET list/detail endpoints need a `requirePermission`/`requireAnyPermission` guard, NOT bare `requireAuth` (bare-auth reads leak data to any logged-in user). Role-management writes are admin-only and re-checked (no privilege escalation; no creating/rewriting the `admin` role).
|
||||
|
||||
**Frontend — Rules of Hooks (lint-enforced) + no UTC-today**
|
||||
|
||||
- ALL hooks (`useState`, `useApiMutation`, `useMemo`, …) run BEFORE any early `return` (the `<Forbidden/>`/`<Navigate/>` permission guard goes AFTER every hook). `eslint.config.mjs` sets `react-hooks/rules-of-hooks` to **error** — `npm run lint` must stay at 0 errors. This was a systemic crash bug across ~14 pages.
|
||||
- Mutations invalidate the **broad domain key**; a mutation that writes via raw `apiFetch` must still `invalidateQueries` the domain it touched (several dashboard widgets were missing this).
|
||||
|
||||
**Safety**
|
||||
|
||||
- `prisma/seed.ts` **refuses to run when `APP_ENV=production`** (it wipes/reseeds permissions). Never seed prod.
|
||||
- Every catch logs (never silently swallow) — the NAS managers were the worst offenders.
|
||||
|
||||
---
|
||||
|
||||
## Known Issues & Gotchas
|
||||
|
||||
Reference in New Issue
Block a user