# Codebase Consistency & Best-Practices Audit — 2026-06-06 (overnight)

**Branch:** `chore/codebase-consistency-audit` (off `master` @ v1.9.0). Nothing here is pushed or deployed; for user review in the morning.

**Mandate (from user):** scan the whole codebase, find inconsistencies, unify manners/conventions, apply best practices (use context7 for library docs). Edit CLAUDE.md to codify rules. Work only in the project dir. Use many agents.

**Operating rules I'm following (self-imposed for safe autonomous work):**

- Isolated branch only. Never touch `master`, never push, never deploy, never touch prod/DB.
- Keep `tsc` (server+client) at 0 and `vitest` green after every change set.
- Prefer documenting risky/large refactors over auto-applying them. Auto-apply only safe, verified, mechanical unifications.
- Don't `git add -A`; stage explicit paths.

---

## 🔴 CRITICAL FINDINGS (need user action)

### C1 — v1.9.0 production regression: plan edit-modal delete button missing

- **Cause:** `FormModal`'s `footerLeft` / `hideCloseButton` props + modal animations lived only in the user's _uncommitted_ working tree. Committed `PlanCellModal.tsx:225` (and the modal system) depend on `footerLeft`. Releases build from committed code (WIP stashed), so v1.9.0 shipped a `FormModal` without the footer-left slot → the **delete button in the plan entry/override edit modal does not render in production**. Also caused a committed-code `tsc` error (TS2322).
- **Impact:** plan entry/override deletion via the edit modal is broken in prod v1.9.0. Category management is NOT affected (it uses `hideFooter` + in-body delete). Minor cosmetic: modal entry animation, `hideCloseButton`.
- **Fix:** committed the modal enhancements (`86e4cbf` on this branch). **Needs a patch release (v1.9.1) to fix prod.**
- **Status:** fixed on branch; prod patch pending user.

### C2 — test suite was red on master/v1.9.0 (2 failing auth tests)

- **Cause:** the JWT log-noise fix (`88c9b7c`) stopped logging expected invalid/expired tokens, but `auth.service.test.ts` still asserted it logged. The wrong auth test file (`auth.test.ts`) was run when that change landed, so 2 failing tests shipped on master.
- **Fix:** updated the tests to assert null + no-log for `JsonWebTokenError` cases (`55a2c50`). Suite back to **134/134**.
- **Status:** fixed on branch. (No runtime impact; CI/test hygiene + would also be cleared by the v1.9.1 patch.)

---

## Findings log

(Discovery sweep results appended below as agents report. Severity: critical/high/medium/low. Fix class: safe-auto / needs-judgment / manual.)

## Changes applied on this branch (all verified: server+client tsc 0, vitest 134, build 0)

Correctness / bugs:

- `86e4cbf` **C1** modal `footerLeft`/`hideCloseButton` enhancements committed (fixes v1.9.0 regression + tsc error).
- `55a2c50` **C2** fix 2 failing `verifyAccessToken` tests (red on master).
- `379725a` **H1/H4** `isAdminLike` reads `roleName` not `role` (admins were self-scoped on /entries,/overrides); dropped 4 stale `as any` category casts.
- `c5f986a` **H5** corrected + unified audit `entity_type` label map (was showing raw English for invoice/order/quotation/trip/vehicle + all warehouse\_\*).
- `1f05ef6` date: `todayLocalStr()` replaces 11 UTC-`today` defaults (off-by-one in post-midnight Prague).
- `744baec` security: audit-log session termination (single + all-others); new `session` EntityType.

Consistency / cleanup:

- `9ae49c0` parseId (trips/sessions), broad `["offers"]` invalidation, Czech messages (attendance/orders/offers/project-files), Zod 4 `z.strictObject`.
- `522afef` undefined `--danger-color`→`--danger`, dedup `formatDate` + `require2FAOptions`, removed dead `systemSettingsOptions`, dropped double `plan.css` import.
- `9c582e1` **H2/H3** log the 30 silent NAS filesystem catches (kept 3 ENOENT-expected as deliberate no-logs).

Docs:

- `94a9fb8` CLAUDE.md: codified "Conventions (enforced)" section; corrected stale known-issues #4 (NAS) and #5 (sanitization is in place, not a gap).
- `58bf034`/`0559664` this report + full 84-finding catalog (`codebase-audit-findings-2026-06-06.md`).

## Remaining recommendations (NOT auto-applied — for your review)

Deliberately left for review (too broad/behavior-risky to apply unattended, or low value):

- **`reply.send` → helpers** (medium): ~1/3 of route files use raw `reply.send({success:true,...})` instead of `success()/paginated()`. Mechanical but touches many files + response-shape sensitive — convert per-file when touched. (List in findings catalog, dimension `routes`.)
- **Zod coercion helpers** (medium): the number/nullable-number/boolean-from-form coercions are copy-pasted ~150×; extract to `src/schemas/common.ts` (with a `NaN` guard — current coercion silently yields `NaN`). Big mechanical sweep — do with review.
- **console.\* → request logger** (medium): services log via `console.*` (bypasses pino/log-level). Architectural; needs a service-logging strategy.
- **`markOverdueInvoices`** (medium): swallows the DB-update failure and returns void, so the route treats a failed update as success — surface the error. Also it compares a `@db.Date` due_date against full `new Date()` (flips "overdue" on the due day) — compare date-only.
- **Dead exports** (low): `previewPattern()` (numbering.service), `usersQuery`/`planKeys.users()` (lib/queries/plan) — unused API surface; remove or wire up.
- **`ShiftFormModal`** (low): inline styles use `--danger-color`/`--success-color`/etc. (undefined; rely on fallbacks) — switch to the `--danger`/`--success` design tokens.
- **`.env.example` drift** (low): out of sync with vars read in `config/env.ts` — regenerate.
- **`@/*` path alias** (low): declared in tsconfig but not wired into Vite/vitest — either wire it up or remove.
- **warehouse soft-delete test** (low): a test named "soft delete" asserts a hard delete its own comment flags as buggy — clarify intent.
- **TOTP verify** doesn't increment failed-login counter (relies on rate limit); **HSTS** lacks `preload` — security hardening, low.

Full detail + file:line for every item: `docs/codebase-audit-findings-2026-06-06.md`.

## Progress tracker

- [x] Branch + clean baseline (tsc server 0 / client 0; vitest 134).
- [x] C1 modal regression + C2 red tests found & fixed (134/134).
- [x] Discovery sweep (16 parallel dimensions, 84 findings).
- [x] Synthesis + prioritization (high + safe-auto).
- [x] Safe unifications applied (12 commits, each verified green).
- [x] CLAUDE.md updated with codified conventions.
- [x] Remaining items documented as a review roadmap.
- [x] Final verification.

## ⚠️ Action for you in the morning

- **v1.9.1 patch:** C1 (plan edit-modal delete button) + C2 (red tests) affect production v1.9.0. Merge this branch (or cherry-pick `86e4cbf`,`55a2c50`,`379725a`,`c5f986a`) and cut a patch.
- Review this branch (`chore/codebase-consistency-audit`, 12 commits) and merge what you like. Nothing here is pushed or deployed.