2026-07-06 14:28:05 -05:00
|
|
|
|
# Code Standards
|
|
|
|
|
|
|
|
|
|
|
|
The living rulebook for this codebase. One canonical way to do each thing, enforced in
|
|
|
|
|
|
CI so it can't drift. Sections marked **(enforced)** fail CI; **(target)** are being
|
|
|
|
|
|
rolled out. Update this doc when a standard changes.
|
|
|
|
|
|
|
|
|
|
|
|
## Tooling & enforcement (enforced)
|
|
|
|
|
|
|
|
|
|
|
|
- **Formatting:** Prettier (`.prettierrc.json`) — single quotes, semicolons, 2-space,
|
|
|
|
|
|
`printWidth: 100`, trailing commas. Run `npm run format`; CI runs `format:check`.
|
|
|
|
|
|
- **Lint:** `npm run lint` (`eslint .`) covers **client and server**. Focused rule set
|
|
|
|
|
|
(react-hooks on the client; `js/recommended` + TS-unused on the server). Errors block CI;
|
|
|
|
|
|
warnings are advisory (ratchet down over time).
|
|
|
|
|
|
- **Typecheck:** `npm run typecheck` (client, strict + `noUncheckedIndexedAccess`) and
|
|
|
|
|
|
`npm run typecheck:server` (server). Both run in CI.
|
|
|
|
|
|
- **Local hooks (lefthook):** pre-commit auto-formats staged files + lints; pre-push
|
|
|
|
|
|
typechecks. Installed by `npm install` (the `prepare` script).
|
|
|
|
|
|
- **Dead code:** `npm run knip` (advisory) for unused exports/deps.
|
|
|
|
|
|
- **Full gate:** `npm run ci` = format:check + lint + typecheck (×2) + check:server +
|
|
|
|
|
|
tests (server + client) + build. The CI pipeline runs the same.
|
|
|
|
|
|
|
|
|
|
|
|
## Modules & files
|
|
|
|
|
|
|
|
|
|
|
|
- Server is **CommonJS TypeScript `.cts`**, run by Node via type-stripping (no build step).
|
|
|
|
|
|
Every migrated module is `require`d **with the explicit `.cts`/`.mts` extension**
|
|
|
|
|
|
(extensionless does not resolve). Each `.cts` leads with an `import type …` line so
|
|
|
|
|
|
type-stripping activates.
|
|
|
|
|
|
- Client is ESM `.ts`/`.tsx` with the `@/*` path alias.
|
|
|
|
|
|
- Money is **integer cents** in the DB and on the wire; convert to dollars only at the edge
|
|
|
|
|
|
via the money helpers. Branded `Cents`/`Dollars` types where practical.
|
|
|
|
|
|
|
|
|
|
|
|
## HTTP API conventions
|
|
|
|
|
|
|
|
|
|
|
|
- **Wire format:** JSON, **snake_case** field names (mirrors DB columns), money as integer
|
|
|
|
|
|
cents.
|
2026-07-10 17:20:42 -05:00
|
|
|
|
- **Error responses (enforced):** every error body is `{ error, message, code, field? }` with
|
2026-07-06 14:28:05 -05:00
|
|
|
|
the correct HTTP status. Throw the factories from `utils/apiError.cts`
|
2026-07-10 17:20:42 -05:00
|
|
|
|
(`ValidationError`, `NotFoundError`, `ConflictError`, `AuthError`, `ForbiddenError`, or
|
|
|
|
|
|
`new ApiError(code, message, status, { field })` for anything else) and let the single
|
|
|
|
|
|
terminal error handler format them — do not hand-roll `res.status().json({ error })`.
|
|
|
|
|
|
Express 5 forwards rejected async handlers automatically, so route bodies do not wrap
|
|
|
|
|
|
everything in `try/catch`; a thrown `ApiError` keeps its message on the wire while any
|
|
|
|
|
|
other 5xx is masked + logged by the terminal handler (that masking is the whitelist:
|
|
|
|
|
|
wrap a message in `ApiError` only when it is deliberately user-facing). Clients read
|
|
|
|
|
|
`message`/`code` + status. **Canonical example:** `routes/categories.cts`.
|
|
|
|
|
|
**Enforced by:** the eslint `no-restricted-imports` ratchet on `routes/**` (all 29 route
|
|
|
|
|
|
files converted 2026-07). **Documented exceptions** (each commented at the site):
|
|
|
|
|
|
`routes/transactions.cts` (internal `{ error }` parse-result convention, rethrown at the
|
|
|
|
|
|
boundary via `throwStandardized`), `routes/import.cts` (`sendImportError` error-id
|
|
|
|
|
|
envelope), `routes/admin.cts` `sendError` (backup download can fail mid-stream after
|
|
|
|
|
|
headers are sent), payments' `DUPLICATE_SUSPECTED` 409 and transactions' bulk-unmatch 500
|
|
|
|
|
|
(both carry extra payload fields the client renders), `routes/calendarFeed.cts`
|
|
|
|
|
|
(text/plain for ICS clients).
|
|
|
|
|
|
- **Client error shape:** `client/api.ts` builds every thrown error through `toApiError()` —
|
|
|
|
|
|
one construction site; on a 401 outside `/auth/*` it dispatches `auth:expired` so
|
|
|
|
|
|
`AuthProvider`/`RequireAuth` redirect to login. Mutations follow the React Query patterns
|
|
|
|
|
|
in `client/hooks/useQueries.ts` / `usePaymentActions.ts` (invalidate + toast on error).
|
2026-07-06 14:28:05 -05:00
|
|
|
|
- **Success envelope (target):** reads return the bare resource/list; mutations return
|
|
|
|
|
|
`{ success: true, … }`.
|
|
|
|
|
|
- **Validation (target):** validate inputs with the shared validators (throwing
|
|
|
|
|
|
`ValidationError`); do not scatter ad-hoc `parseInt`/range checks.
|
|
|
|
|
|
|
|
|
|
|
|
## Security (target; see also SECURITY.md)
|
|
|
|
|
|
|
|
|
|
|
|
- Every **mutating** route carries CSRF + auth + an appropriate rate-limit + input
|
|
|
|
|
|
validation. Admin routes require `requireAdmin`.
|
|
|
|
|
|
- Every user-scoped query filters `user_id` **and** `deleted_at IS NULL`. Never interpolate
|
|
|
|
|
|
user input into SQL — use bound parameters.
|
|
|
|
|
|
- Never log secrets, tokens, or raw financial values.
|
|
|
|
|
|
- Fail fast at boot on missing required config (e.g. `TOKEN_ENCRYPTION_KEY` in production).
|
|
|
|
|
|
|
2026-07-10 17:20:42 -05:00
|
|
|
|
## Logging (enforced)
|
2026-07-06 14:28:05 -05:00
|
|
|
|
|
2026-07-06 16:05:18 -05:00
|
|
|
|
- Use `utils/logger.cts` (`const { log } = require('.../utils/logger.cts')`) — `log.debug/info/warn/error`,
|
|
|
|
|
|
console-compatible. Level via `LOG_LEVEL` (default: debug in dev, info in prod). It **redacts**
|
|
|
|
|
|
secrets/PII (sensitive keys, bearer/JWT) so tokens/passwords never reach logs. All server
|
|
|
|
|
|
runtime `console.*` has been migrated. Do **not** use raw `console.*` in server code.
|
2026-07-06 14:28:05 -05:00
|
|
|
|
|
|
|
|
|
|
## Data integrity
|
|
|
|
|
|
|
|
|
|
|
|
- Every **multi-statement write** runs inside `db.transaction()`.
|
|
|
|
|
|
- Migrations are append-only versioned entries in `db/migrations/versionedMigrations.cts`
|
|
|
|
|
|
(idempotent; guard column adds with `PRAGMA table_info`).
|
|
|
|
|
|
|
|
|
|
|
|
## Testing
|
|
|
|
|
|
|
|
|
|
|
|
- Server: `node:test` in `tests/*.test.js`. Client: Vitest (`*.test.ts[x]`). E2E: Playwright
|
|
|
|
|
|
(`npm run test:e2e`, probe subset `test:e2e:probe`). Add coverage for new handlers; prove
|
|
|
|
|
|
money invariants with property tests where practical.
|
|
|
|
|
|
|
2026-07-06 16:23:50 -05:00
|
|
|
|
## Typed SQL (Kysely — available, adopt incrementally)
|
|
|
|
|
|
|
|
|
|
|
|
`db/kysely.cts` provides Kysely as a **type-safe query builder executed synchronously**.
|
|
|
|
|
|
The app is sync (better-sqlite3) and Kysely's `.execute()` is async, so we never execute
|
|
|
|
|
|
through Kysely — we build the query (columns + `user_id`/`deleted_at` scoping + types all
|
|
|
|
|
|
checked at compile time), then run it via the sync helpers:
|
|
|
|
|
|
|
|
|
|
|
|
```ts
|
|
|
|
|
|
const { kysely, all, get, run } = require('../db/kysely.cts');
|
|
|
|
|
|
const cats = all(
|
|
|
|
|
|
kysely()
|
|
|
|
|
|
.selectFrom('categories')
|
|
|
|
|
|
.select(['id', 'name'])
|
|
|
|
|
|
.where('user_id', '=', userId)
|
|
|
|
|
|
.where('deleted_at', 'is', null),
|
|
|
|
|
|
);
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
Compiled SQL is always parameterized (injection-safe). Prefer this for **new/refactored**
|
|
|
|
|
|
user-scoped queries; extend the `Database` schema in `db/kysely.cts` as tables are adopted.
|
|
|
|
|
|
Proven byte-identical to raw SQL in `tests/kyselyPoc.test.js`.
|
|
|
|
|
|
|
|
|
|
|
|
## Future / modernization (tracked)
|
|
|
|
|
|
|
|
|
|
|
|
Zod (validation) · `node:sqlite` (drop the native dep) · ts-rest + OpenAPI · Vite 6/7.
|
|
|
|
|
|
Adopt where each makes the code more uniform; see the standardization plan.
|