Files
motovaultpro/.claude/skills/mvp-architecture-contract/SKILL.md
T
Eric GullicksonandClaude Fable 5 c239bb9347
Deploy to Staging / Build Images (push) Successful in 5m32s
Deploy to Staging / Deploy to Staging (push) Successful in 44s
Deploy to Staging / Verify Staging (push) Successful in 5s
Deploy to Staging / Notify Staging Ready (push) Successful in 4s
Deploy to Staging / Notify Staging Failure (push) Has been skipped
chore: replace AI skill library with 16 verified mvp-* skills
Removes the old planner/decision-critic/role-agents/domain-agents system
(step-injector scripts, stale scopes, sprint-era workflow) and replaces it
with 16 ground-truth-verified skills under .claude/skills/: change control,
debugging playbook, failure archaeology, architecture contract, domain
reference, OCR/Gemini pipeline, config and secrets, build and env, run and
operate, diagnostics (with tested scripts), validation and QA, docs and
writing, launch readiness, deploy-safety campaign, proof and analysis
toolkit, and research frontier. RULE 0/1/2, the temporal-contamination rule,
and the decision stress-test protocol are carried forward into the new
skills; the retired content remains in git history.

Co-Authored-By: Claude Fable 5 <[email protected]>
2026-07-09 20:21:42 -05:00

170 lines
21 KiB
Markdown

---
name: mvp-architecture-contract
description: Load this before designing, extending, or reviewing any MotoVaultPro feature or refactor. It states the load-bearing architecture decisions (and WHY), the invariants that must hold, and the known-weak points. Symptoms that mean you need this skill; adding a new backend feature capsule or API route; a new route returns 401 everywhere or, worse, works WITHOUT a token; a migration "ran" but the table does not exist; a route registered fine but is missing the /api prefix; a new page works on desktop but is unreachable on mobile (or vice versa); a tier-gated feature is accessible to free users; wondering why this is Docker Compose and not Kubernetes; wondering whether userContext.userId is the Auth0 sub (it is not).
---
# MotoVaultPro Architecture Contract
Authored 2026-07-07. Every code claim below was verified against the repo on that date. Where a doc contradicts this file, re-verify against code: code wins.
## When to use / When NOT to use
Use this skill when you are about to add or modify a backend feature capsule, an API route, a repository, a migration, a frontend page, or tier gating — or when a change "works" but violates one of the failure symptoms in the description above.
Do NOT use this skill for:
- How to branch/commit/PR or what review rules apply: `mvp-change-control`
- Triage of a live failure: `mvp-debugging-playbook`
- Why a past decision was reverted, incident history: `mvp-failure-archaeology`
- Config/secret axes and how to add one: `mvp-config-and-secrets`
- Deploy, blue-green, rollback, backups: `mvp-run-and-operate`
- VIN/fuel/date domain math: `mvp-vehicle-domain-reference`
- Which docs to trust in full detail: `mvp-docs-and-writing`
## Doc-trust rule (architecture facts)
For architecture facts: **code > docs/CICD-DEPLOY.md and docs/LOGGING.md > everything else.**
`docs/ARCHITECTURE-OVERVIEW.md` and `docs/DATABASE-SCHEMA.md` are known-stale (DATABASE-SCHEMA.md still says "15 feature capsules" and a migration order starting with `admin`; reality is 21 capsules and an order starting with `features/vehicles`). Route to `mvp-docs-and-writing` for the full trust map. When this skill and the code disagree, the code changed after 2026-07-07 — update this skill.
## Part 1: Load-bearing decisions and WHY
### 1.1 9-container single-tenant Docker Compose (Kubernetes was tried and abandoned)
9 containers: Traefik, Frontend (React/nginx), Backend (Fastify), OCR (Python), PostgreSQL, Redis + Loki, Alloy, Grafana. Base file `docker-compose.yml`; staging/prod/blue-green are overlay files.
WHY compose, not k8s: a Kubernetes-style redesign was planned and executed as a "Kubernetes-like Docker Compose" restructure — commit `040da4c` ("k8s redesign complete", 2025-09-18) added `K8S-STATUS.md` (titled "Kubernetes-like Docker Compose Migration Status") plus compose/Makefile changes; no actual k8s manifests were ever committed anywhere in history (only Markdown planning docs). The status doc was demoted out of the repo root into `docs/changes/` four days later (`8fd7973`, 2025-09-22) and deleted entirely on 2025-10-16 (`5638d39`). The product is single-tenant on VPS-class hardware; the k8s direction bought operational complexity with no scaling need. This is a settled battle — do not propose k8s again without new facts (see `mvp-failure-archaeology`).
Single-tenant means: one deployment per customer base is NOT the model — there is one user population in one database, all rows scoped per user (section 2.5).
### 1.2 Backend feature capsules
All application features are self-contained modules under `backend/src/features/` — 21 capsule directories (the 22nd entry is `CLAUDE.md`). Canonical anatomy (fuel-logs is the reference):
```
backend/src/features/{name}/
index.ts # barrel export: routes + anything other capsules may import
api/ # {name}.routes.ts (FastifyPluginAsync, per-route preHandlers),
# {name}.controller.ts, {name}.validators.ts
domain/ # {name}.service.ts, {name}.types.ts (camelCase TS types)
data/ # {name}.repository.ts (class taking pool; private mapRow())
external/ # cross-feature or third-party clients
migrations/ # NNN_name.sql, feature-owned schema
tests/ # unit/ and integration/
```
Wiring: `backend/src/app.ts` imports each capsule's routes and registers every one with `{ prefix: '/api' }` (lines ~136-159). Plugin registration order before routes: helmet, cors, logging, error, multipart, auth, admin-guard, tier-guard. Shared infra lives in `backend/src/core/` (plugins, config, logging, middleware, scheduler, storage).
WHY: capsules keep each feature's schema, logic, and HTTP surface in one directory so an AI session can load one capsule instead of the whole backend — this is the project's stated AI-context-efficiency principle.
### 1.3 YAML config + file secrets, NOT env vars
`backend/src/core/config/config-loader.ts` loads a zod-validated YAML file (`CONFIG_PATH`, default `/app/config/production.yml`) plus file-based secrets from `SECRETS_DIR` (default `/run/secrets`). DB, Redis, Auth0, and CORS settings live in YAML. Env vars are the exception (Stripe price IDs, `OCR_SERVICE_URL`, `MIGRATIONS_DIR`, `LOG_LEVEL`, `CONFIG_PATH`/`SECRETS_DIR` themselves). Missing required config or secret = startup crash by design.
WHY: one schema-validated config document beats scattered env vars for auditability, and file secrets match Docker secret mounts. Do not add `process.env.X` for app config — add it to the zod schema and YAML. Full catalog: `mvp-config-and-secrets`.
### 1.4 Repository mapRow: snake_case to camelCase, MUST coerce numerics
Every repository exposes data only through a private mapper (`mapRow()`), converting DB snake_case to TS camelCase. Critically: node-postgres returns `NUMERIC`/`DECIMAL` (OID 1700) as **strings** and this project does NOT override that parser globally — every mapper must coerce (`parseFloat`/`Number`) or the API leaks strings where numbers are typed. This caused a multi-incident bug train (issues #239, #241, #244). New repository with decimal columns = coerce in the mapper, no exceptions.
### 1.5 DATE columns are plain strings end-to-end
`backend/src/core/config/database.ts` line 12:
```typescript
types.setTypeParser(1082, (val: string) => val);
```
DATE (OID 1082) values stay `"YYYY-MM-DD"` strings through repository, API, and frontend. WHY: pg's default returns a `Date` at local midnight, which shifts a day when serialized to UTC (issue #237). The full DATE-handling rules (dayjs display, lexicographic sort, the three historical traps) are canonical in `mvp-vehicle-domain-reference` section 3.
### 1.6 Auth: per-route preHandler; userId is the internal UUID, not the Auth0 sub
`backend/src/core/plugins/auth.plugin.ts` decorates `fastify.authenticate` (line 120): validates the Auth0 JWT (JWKS, issuer, audience), then loads/creates the `user_profiles` row and hydrates `request.userContext`.
**`userContext.userId` is the internal `user_profiles.id` UUID, NOT the Auth0 `sub`** (`auth.plugin.ts` line ~130 defaults to auth0Sub, overwritten with `profile.id` at line ~165). This is the post-#206 identity migration state (merged in PR #219). All repositories scope queries by this UUID.
There is NO global auth hook. Every protected route must list `preHandler: [fastify.authenticate]` itself (see section 2.1).
### 1.7 Migrations: feature-owned, hard-coded order, run at container start, NO rollback
- SQL files live in each capsule's `migrations/` dir, executed in lexical order within a feature.
- Cross-feature order is the hard-coded `MIGRATION_ORDER` array in `backend/src/_system/migrations/run-all.ts` (17 entries; `features/vehicles` first because it defines `update_updated_at_column()` which later features depend on; `core/identity-migration` last).
- Execution is tracked in the `_migrations` table (`UNIQUE(feature, file)`); already-run files are skipped.
- Migrations run automatically on every backend container start (`backend/Dockerfile` CMD: `node dist/_system/migrations/run-all.js && npm start`), and manually via `make migrate` or `cd backend && npm run migrate`.
- **There is no rollback.** No down migrations. Recovery is restore-from-export (`scripts/import-database.sh`). WHY: single-tenant, small blast radius, and honest acknowledgment that untested down-migrations are worse than a restore path.
OWNER NON-NEGOTIABLE: no destructive database operation without a fresh backup — that includes schema migrations on staging/prod (`./scripts/export-database.sh --env <env>` first), `make clean` (destroys volumes), and `import-database.sh --drop-existing`. See `mvp-change-control`.
### 1.8 Redis: cache-aside with mvp: prefix; cache failure never breaks a request
`backend/src/core/config/redis.ts`: singleton ioredis client; `CacheService` prefixes every key with `mvp:` and wraps every operation in try/catch that logs and returns null/continues. A dead Redis degrades to cache-miss behavior, never a 500. `DistributedLockService` (prefix `mvp:lock:`, SET NX EX + Lua release) backs scheduled jobs. Follow this pattern: read cache, on miss hit DB and set with TTL, invalidate on write — and never let a cache error propagate.
### 1.9 Frontend dual navigation — deliberate but costly (the registration-checklist tax)
`frontend/src/App.tsx` forks the entire app at `window.innerWidth <= 768` plus a UA regex (~lines 360-407):
- **Desktop**: react-router `<Routes>` under `/garage/*`, lazy-loaded `features/*/pages/*Page`.
- **Mobile**: NO router — a Zustand screen switcher (`useNavigationStore().activeScreen`) renders `features/*/mobile/*MobileScreen` components, with two-way URL sync via the `routeToScreen` / `screenToRoute` maps in `frontend/src/core/store/navigation.ts`.
WHY: the mobile experience is a purpose-built app shell (bottom nav, gesture transitions, error boundaries) rather than responsive pages. The cost is real and accepted: every new page must run the full registration checklist in invariant 2.7, and mobile+desktop are separate implementations that must BOTH be built and tested (hard project rule).
All frontend HTTP goes through the shared `apiClient` (`frontend/src/core/api/client.ts`), a queued axios wrapper that holds requests until the Auth0 gate reports ready — bypassing it with raw axios/fetch reintroduces the auth race it exists to solve.
## Part 2: INVARIANTS — break these and here is what happens
| # | Invariant | If you break it | Enforcing code path |
|---|-----------|-----------------|---------------------|
| 2.1 | Every protected route lists `preHandler: [fastify.authenticate]` explicitly | **The route is PUBLIC.** No global hook exists; nothing fails loudly. An unlisted route ships as an unauthenticated endpoint on the open internet | `backend/src/core/plugins/auth.plugin.ts:120` (decorator only); routes files per capsule. Intentional public routes: canonical list lives in the header comment of `.claude/skills/mvp-diagnostics-and-logging/scripts/check-route-auth.sh` (webhooks + pre-auth signup routes in its allowlist, plus core-registered routes outside its scan scope: `/health`, `/api/health`, `/auth/verify`, `GET /api/config/feature-tiers`) |
| 2.2 | Every repository method returning rows goes through a mapper that coerces NUMERIC/DECIMAL | API returns `"42.50"` where the type says `number`; frontend `.toFixed()` crashes or renders garbage. This has shipped as a bug at least 3 times (#239, #241, #244) | No global parser for OID 1700 in `backend/src/core/config/database.ts` — coercion is per-mapper, by convention and review only |
| 2.3 | The DATE parser override stays | Dates shift one day for any user west of UTC (issue #237 regression) | `backend/src/core/config/database.ts:12` `types.setTypeParser(1082, ...)` |
| 2.4 | A new capsule with migrations gets appended to `MIGRATION_ORDER` | Its migrations **silently never run** — no error, the runner only walks listed directories. First symptom is "relation does not exist" on staging | `backend/src/_system/migrations/run-all.ts:17-35`; also requires an image rebuild so the SQL is packaged into `/app/migrations` |
| 2.5 | All user data scoped by the `user_profiles` UUID (`userContext.userId`) | Cross-user data leak, or zero rows returned if you scope by the Auth0 `sub`. Post-#206, feature tables' `user_id` column is **UUID type** FK to `user_profiles(id) ON DELETE CASCADE` (the migration added `user_profile_id`, backfilled, dropped old VARCHAR `user_id`, then RENAMED `user_profile_id` back to `user_id`); `admin_users` keeps a separate `user_profile_id` column | `backend/src/core/identity-migration/migrations/001_migrate_user_id_to_uuid.sql`; `auth.plugin.ts` userContext hydration |
| 2.6 | Frontend API calls only via the shared queued `apiClient` | Requests fire before the Auth0 token interceptor is installed: intermittent 401s on first load, spurious error toasts | `frontend/src/core/api/client.ts` (`createQueuedAxios`, `authReady` gate); interceptor installed in `frontend/src/core/auth/Auth0Provider.tsx` |
| 2.7 | New pages registered at EVERY registration point. This is the CANONICAL checklist (other skills point here; do not trust any "4 places" shorthand — the full union is 7 steps): (1) desktop `<Route>` in App.tsx; (2) `MobileScreen` union type in navigation.ts; (3) `routeToScreen` map; (4) `screenToRoute` map; (5) lazy import of the `*MobileScreen` component in App.tsx; (6) mobile render block `{activeScreen === "X" && ...}` in App.tsx; (7) a navigation entry point (`BottomNavigation.tsx` or `HamburgerDrawer.tsx` item, or navigation from another screen) | Page unreachable on one platform; registered-but-unreachable on mobile (missing entry point or import); or URL/back-button desync (screen renders but URL lies, deep links break) | `frontend/src/App.tsx` (lazy imports + both render paths), `frontend/src/core/store/navigation.ts` (union line ~5; maps lines ~9, ~29), `frontend/src/shared-minimal/components/mobile/` |
| 2.8 | Tier-gated features registered in `FEATURE_TIERS`**unknown keys FAIL OPEN** | A typo'd or unregistered `featureKey` on a plugin-guarded route silently grants access to ALL tiers. No error, no log at gate time | `backend/src/core/config/feature-tiers.ts:57-62` (`canAccessFeature` returns `true` for unknown keys, by design). Registered-key and vehicle-limit literals: canonical catalog in `mvp-config-and-secrets` section 3 |
| 2.9 | Capsule routes registered in `app.ts` with `{ prefix: '/api' }` using the plain (non-fp) export | Route mounts at `/thing` instead of `/api/thing`; Traefik and the frontend both expect `/api` — the endpoint 404s in every deployed environment | `backend/src/app.ts:136-159`; see weak point 3.7 for the platform dual-export trap |
New-capsule checklist (all must hold): barrel `index.ts`; routes registered in `app.ts` with `/api` prefix; every route lists its auth preHandler; repository mapper coerces numerics; migrations dir appended to `MIGRATION_ORDER`; queries scoped by the profile UUID; if tier-gated, key added to `FEATURE_TIERS` AND spelled identically at the gate site.
## Part 3: Known-weak points (stated plainly — these are debt, not patterns to copy)
3.1 **Tier gating fails open AND has two parallel mechanisms with different failure behavior** — the middleware 500s on unknown keys, the plugin decorator fails open (full mechanics and the current key catalog are canonical in `mvp-config-and-secrets` section 3). Both are used, even mixed within one file (`backend/src/features/ocr/api/ocr.routes.ts`). The plugin's dependency check on the auth plugin was deliberately removed for testability — registration order is convention-only. When gating something new, prefer the plugin decorator with a key you have verified exists in `FEATURE_TIERS`.
3.2 **Controllers string-match error messages for status codes.** No typed error classes: catch blocks do `error.message.includes('not found')` -> 404, `includes('Unauthorized')` -> 403, else 500 (e.g. `backend/src/features/fuel-logs/api/fuel-logs.controller.ts:30,36`). Rewording a service error message silently changes an HTTP status. If you touch a service's thrown messages, grep the controller for `includes(` first.
3.3 **Auth plugin is a per-request hot path to Auth0.** Worst case per authenticated request: profile `getOrCreate` DB roundtrip, plus up to two Auth0 Management API `getUser` calls (email backfill when the JWT lacks email; verification re-check when the profile is unverified) plus the SDK's token grant — up to ~3 Auth0 HTTP calls. Failures fall back silently to JWT claims (`auth.plugin.ts` ~line 201). Latency and rate-limit exposure live here; do not add more per-request external calls to this plugin.
3.4 **Dual fuel-logs schema, with a documented convention break.** `backend/src/features/fuel-logs/data/fuel-logs.repository.ts` carries a legacy API (`create`/`mapRow`, gallons columns) and an enhanced API (`createEnhanced`/`mapEnhancedRow`, fuel_units/cost_per_unit, dual-writing legacy columns). `mapEnhancedRow` (line ~252) deliberately returns **snake_case** numeric-coerced rows, and the service's `toEnhancedResponse` (`fuel-logs.service.ts:273`) does the camelCase mapping — a known break from the mapRow convention, typed `any` throughout. Do not copy this pattern into new capsules; do not "fix" it casually either (dual-write compatibility is load-bearing).
3.5 **App.tsx is a 1192-line god component** (`frontend/src/App.tsx`): inline screen components, all mobile switching, auth routing, and global error suppression in one file. It violates the repo's own RULE 2. Changes here have wide blast radius; keep diffs surgical.
3.6 **DB pool max is hard-coded 10** (`backend/src/core/config/database.ts:16`) while the config schema defines `database.pool_size` with default 20 (`config-loader.ts:28`) — the config value is not consumed by the pool. YAML pool tuning silently does nothing.
3.7 **platform.routes.ts dual-export trap.** `backend/src/features/platform/api/platform.routes.ts:45-46` exports both a `fastify-plugin`-wrapped default AND a raw named export. `app.ts` imports the NAMED export, so the `/api` prefix applies. Switching the import to the fp-wrapped default would silently drop the prefix (fastify-plugin breaks encapsulation, so `prefix` is ignored) — every platform dropdown endpoint would move off `/api` with no error.
3.8 **Two shared frontend trees with no boundary rule:** `frontend/src/shared/` and `frontend/src/shared-minimal/` (theme, GlassCard, BottomNavigation live in shared-minimal). Check both before creating a "new" shared component.
3.9 **`(fastify as any).requireAdmin`** in `backend/src/features/backup/api/backup.routes.ts` (every route) — the admin-guard decorator is used through an `any` cast, so TypeScript would not catch the decorator being renamed or unregistered; the routes would throw at runtime instead.
Severity calibration: this is a pre-launch product heading toward paying users. 3.1 and invariant 2.1/2.8 are the security-shaped items — treat regressions there as RULE 0. The rest are RULE 1/2 debt: known, tolerated, but never to be silently extended.
## Provenance and maintenance
Authored 2026-07-07 from direct repo inspection (file reads, greps, git history). Re-verify volatile facts before relying on them:
| Fact (as of 2026-07-07) | Re-verify with |
|---|---|
| 21 feature capsules | `ls backend/src/features/ \| grep -v CLAUDE.md \| wc -l` |
| Routes registered with `/api` prefix; platform uses named export | `grep -n "prefix: '/api'" backend/src/app.ts \| wc -l` and `grep -n "platformRoutes" backend/src/app.ts` |
| DATE parser override present | `grep -n "setTypeParser(1082" backend/src/core/config/database.ts` |
| Pool max hard-coded 10; schema pool_size default 20 | `grep -n "max:" backend/src/core/config/database.ts; grep -n "pool_size" backend/src/core/config/config-loader.ts` |
| MIGRATION_ORDER has 17 entries, vehicles first, identity-migration last | `sed -n '17,36p' backend/src/_system/migrations/run-all.ts` |
| Tier fail-open (key/limit catalog: `mvp-config-and-secrets` section 3) | `grep -n "return true" backend/src/core/config/feature-tiers.ts; grep -n "minTier" backend/src/core/config/feature-tiers.ts` |
| userContext.userId = profile UUID, not Auth0 sub | `grep -n "userId = profile.id" backend/src/core/plugins/auth.plugin.ts` |
| Feature tables' user_id is UUID (renamed from user_profile_id) | `grep -n "RENAME COLUMN user_profile_id TO user_id" backend/src/core/identity-migration/migrations/001_migrate_user_id_to_uuid.sql` |
| Redis `mvp:` prefix, errors swallowed | `grep -n "prefix = 'mvp:'" backend/src/core/config/redis.ts` |
| App.tsx 1192 lines; 768px fork | `wc -l frontend/src/App.tsx; grep -n "768" frontend/src/App.tsx` |
| Nav registration checklist (2.7 union) | `grep -n "routeToScreen\|screenToRoute\|MobileScreen" frontend/src/core/store/navigation.ts; grep -n "activeScreen ===" frontend/src/App.tsx \| head -3` |
| Queued apiClient auth gate | `grep -n "createQueuedAxios\|authReady" frontend/src/core/api/client.ts` |
| platform dual export | `grep -n "export" backend/src/features/platform/api/platform.routes.ts \| tail -2` |
| requireAdmin any-cast in backup routes | `grep -c "(fastify as any).requireAdmin" backend/src/features/backup/api/backup.routes.ts` |
| k8s-style redesign tried/abandoned Sept-Oct 2025 | `git log --all --oneline -i --grep=k8s` and `git log --all --oneline --follow -- docs/changes/K8S-STATUS.md` |
| Controller string-matching | `grep -rn "includes('not found')" backend/src/features/*/api/*.controller.ts \| head` |