Files
app/docs/codebase-audit-2026-06-06.md
2026-06-06 02:41:57 +02:00

6.9 KiB
Raw Blame History

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

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