# Post-Audit Fix Plan

Generated 2026-05-14 from two rounds of deep Playwright + API audit on `feat/documents-folders` → `main`.

**Total findings:** 24 fixes + 1 new feature. Grouped by priority. Each entry has impact, file pointer, and effort estimate.

---

## TIER 0 — Already Applied in Working Tree (uncommitted)

Status: **fixed in code, not yet committed**. Commit + push to ship.

### F1. `/api/v1/bootstrap/*` proxy allow-list (task #22)

- **Impact:** Cold-start VPS deploy can't bootstrap its first super-admin. `/setup` page calls `/api/v1/bootstrap/status` which 401s; setup form never renders.
- **File:** `src/proxy.ts` — added to `PUBLIC_PATHS`.
- **Effort:** XS.

### F2. Interest detail page 500s on every visit (task #25)

- **Impact:** Sales workflow non-functional. Raw `Date` passed to postgres-js `sql\`${col} >= ${dateVar}\`` template crashes the Bind step.
- **File:** `src/lib/services/interests.service.ts:566` — switched to `gte(col, date)`.
- **Effort:** XS.

---

## TIER 1 — Pre-Deploy Blockers (P1)

Ship before any real client touches the system.

### F3. GDPR export 500s — BullMQ rejects job IDs with colons (task #51)

- **Impact:** GDPR Article 15 right-to-access non-functional. Legal/compliance gate.
- **File:** `src/lib/services/gdpr-export.service.ts:113` — change `jobId: \`gdpr-export:${row.id}\`` → `jobId: \`gdpr-export-${row.id}\``.
- **Effort:** XS (one char).

### F4. Redis eviction policy is `allkeys-lru` but BullMQ requires `noeviction` (companion to F3)

- **Impact:** Under memory pressure, Redis will evict BullMQ keys; jobs disappear silently.
- **File:** production Redis config (`maxmemory-policy noeviction`) + the docker-compose redis service.
- **Effort:** XS (config).

### F5. `deleteBerth()` hard-deletes rows instead of soft-archiving (task #65)

- **Impact:** Permanent data loss on accidental delete. Junction tables CASCADE-vanish. Audit log points to non-existent rows. Public feed could 404 mid-customer-inquiry.
- **Files:**
  - `src/lib/services/berths.service.ts:673-685` — replace `db.delete()` with `set archivedAt = now(), archivedBy = userId, archiveReason = input.reason`.
  - Add filter `isNull(berths.archivedAt)` to all default berth queries (recommender, public feed, list, dashboard heat).
  - Add restore endpoint `POST /api/v1/berths/[id]/restore` mirroring the interests pattern.
  - Require `reason` (min 5 chars) before destructive call.
- **Effort:** M.

### F6. Weak input validation on `/api/v1/clients` (task #50)

- **Impact:** Email format not validated (bounces silently); whitespace-only names accepted (blank chips everywhere); XSS payload stored verbatim (depends on every render path being safe).
- **Files:**
  - `src/lib/validators/clients.ts` — add `.email()` refinement on contacts where `channel === 'email'`; trim+min(1) on `fullName`; regex-strip control chars + zero-width chars.
  - Audit every fullName render path for `dangerouslySetInnerHTML` / pdfme / react-pdf / email template merges and ensure escaping.
  - Apply similar hardening to yachts, companies, interests, notes, berths, reminders (audit all string fields).
- **Effort:** S for the obvious zod tweaks, M for the full audit.

### F7. No rate limiting on login (task #68)

- **Impact:** Brute force is wide open. 20 wrong-password attempts in a row all returned 401 with no lockout.
- **Files:**
  - `src/lib/auth/` — add a `rateLimit` block to the better-auth config: `{ window: 60, max: 5 }` per IP+email.
  - Optionally: Redis sliding window via existing ioredis client.
  - Optionally: per-user lockout table (`auth_lockouts`) after 5 failures, locked 15min.
- **Effort:** S.

### F8. postgres-js pool corruption causes CONNECT_TIMEOUT (task #46)

- **Impact:** During the audit the dev server twice entered a stuck state where every query 500'd with `CONNECT_TIMEOUT` while the DB was healthy (1/100 connections used). Production VPS will hit this under load.
- **Files:**
  - `src/lib/db/index.ts` — add `connect_timeout: 5`, `max_lifetime: 60 * 60`, `idle_timeout: 30`.
  - Wrap critical-path queries in retry-on-CONNECT_TIMEOUT logic (one retry, then 503).
  - Consider pgbouncer in front of postgres for production multi-process deployments.
- **Effort:** S for the postgres-js options, M for full pgbouncer.

---

## TIER 2 — High Impact Architectural / UX

Not strictly deploy-blocking, but each one breaks the UX in observable ways every day.

### F9. Layout-wide duplicate mobile/desktop DOM rendering (task #26)

- **Impact:** Single highest leverage UX bug. EVERY page mounts BOTH responsive layouts; both Radix Tabs providers are concurrently active with `data-state="active"`. Half my click attempts on tabs/filters/popovers went to the wrong layer. Doubled network requests, doubled component state, doubled a11y landmarks.
- **Files:** the responsive shell (likely `src/components/layout/*-shell.tsx` and detail-page wrappers).
- **Fix options:** use `useMediaQuery` to mount only one tree; or hoist `<Tabs>` to a single provider and let both layouts consume context.
- **Effort:** L (architectural refactor across multiple pages).

### F10. Archiving a client doesn't cascade-archive their interests (task #66)

- **Impact:** Orphan refs. Archived clients have active interests; active queries surface them with broken breadcrumbs / silent 404s on drill-in.
- **Files:** `src/lib/services/clients.service.ts:archiveClient()` — wrap in transaction, archive open interests too. OR extend `activeInterestsWhere()` to filter on `client.archived_at IS NULL`.
- **Effort:** S.

---

## TIER 3 — Standard Fixes (P3)

UX polish + missing entry points. Each is small, but the sum matters.

### F11. "Mark as won" dialog still says "moves to Completed" (task #27)

- **Impact:** Stale copy from before the 7-stage refactor. Misleads users.
- **File:** `src/components/interests/won-dialog.tsx` (or similar) — update copy to "marks Won; stage stays at <current>".
- **Effort:** XS.

### F12. Activity feed + tab count concatenation (task #23)

- **Impact:** "Test Person 1interest", "Interests0", "Click Test Co.company" — unprofessional.
- **Files:** `src/components/dashboard/activity-feed.tsx` (entity name + type), every detail-page tab count render. Audit log FTS `search_text` should also include entity names.
- **Effort:** S.

### F13. Bulk-add berths wizard has no UI entry point (task #28)

- **Impact:** Feature built for new-port setup, but invisible. Operator must know the URL.
- **Files:** Add a "Bulk add" button next to "New berth" on `/[portSlug]/berths`. Add link on `/admin` landing card.
- **Effort:** S.

### F14. Audit Log page has no UI entry point (task #49)

- **Impact:** Feature built, no nav link. Discovery requires URL knowledge.
- **Files:** Sidebar Admin section — add "Audit Log" entry under `documents` settings or as its own item, gated by `audit_log.view` permission.
- **Effort:** S.

### F15. New Yacht dialog only lists clients in owner picker (task #44)

- **Impact:** Data model supports `'client' | 'company'` ownership; UI only lets you pick clients. Cannot create company-owned yacht via UI.
- **Files:** `src/components/yachts/new-yacht-dialog.tsx` — add owner-type segmented control (Client / Company) above the owner picker; switch data source.
- **Effort:** S.

### F16. InlineTagEditor "Add tag" focus + create flow (task #45)

- **Impact:** Typing in the tag widget set the CONTACT LABEL instead. Plus no "Create new tag" affordance for new tag names.
- **Files:** `src/components/shared/inline-tag-editor.tsx`. Fix focus target; surface "Create new: X" as a popover item; orchestrate POST /api/v1/tags then PUT .../tags.
- **Effort:** S.

### F17. Cross-port (and 404) detail URLs silently render list shell (task #48)

- **Impact:** User pastes a wrong-port URL → API 404s correctly but UI silently shows the list shell. No explicit "not found" message.
- **Files:** every entity-detail client component — render `<EmptyState title="Not found" />` when GET returns 404. Apply to clients, interests, yachts, companies, berths.
- **Effort:** M (apply pattern to each detail page).

### F18. Recommender `limit` param ignored (task #69)

- **Impact:** Request with `{"limit": 3}` returned 8 berths. Either param name mismatch or no clamp.
- **Files:** `src/lib/services/berth-recommender.service.ts` + the recommend-berths validator.
- **Effort:** XS.

---

## TIER 4 — Polish & UX Reductions (P4)

The `UX EFFICIENCY` list (task #24). Each is small, mostly copy/flow improvements.

### F19. New Client form — primary contact default trap

- Default-checked "Primary contact" with empty email silently rejects on submit. Either don't pre-add OR drop empty contacts on save.

### F20. New Interest dialog — redirect to detail page on create

- Currently returns to the list. Add `router.push('/interests/' + newId)` to land on the workflow page immediately.

### F21. Stage-transition error toast leaks developer language

- "yachtId is required before leaving stage=enquiry" → "Yacht is required before leaving the Enquiry stage."
- Audit ALL ValidationError + ConflictError + service error messages for user-readable copy.

### F22. Stage menu uses unicode emoji `⚑` as prereq-blocked indicator

- Per user preference (memory: avoid decorative emoji), replace with a Lucide icon (`Lock`, `AlertCircle`, or `FlagOff`).

### F23. Blocked-stage UX — show prereq picker inline

- Clicking a blocked stage currently dismisses with a toast. Better: open the prereq picker inline ("Pick a yacht to leave Enquiry" with combobox right there).

### F24. New Client form — "Country" optional but prominent

- Drop from quick-path OR move to a "More details" disclosure.

### F25. Documents Hub — folder navigation doesn't update URL

- Drilling into a folder updates "Current location" but doesn't change `location.search`. Can't deep-link, browser-back broken, refresh resets to root.

### F26. "Reopen" outcome action silent — no toast

- After clicking Reopen, no feedback. Add `toast.success('Outcome cleared')` or similar.

### F27. Same-stage write returns full body — should be 204

- PATCH /stage with same stage = current stage returns 200 + full interest. Should be 204 No Content (no-op).

### F28. Recommender empty-result UI

- 300ft yacht returns `data: []` — UI Recommendations tab silently shows blank. Should render "No berths match — try relaxing constraints."

### F29. Inbox first-load "Loading..." stuck

- First navigation to /inbox shows "Loading..." indefinitely; subsequent reload renders fine. TanStack Query cache initialization issue.

### F30. Berths in default queries should filter `archivedAt IS NULL`

- Companion to F5 — once soft-delete lands, every default list query must filter archived rows.

---

## NEW FEATURE — Manual Berth Status Catch-Up Workflow (task #67)

User-requested. Foundation already exists (column `berths.status_override_mode` is in schema but never written).

### Phase 1 — Wire the status_override_mode field

- `updateBerthStatus()` sets `status_override_mode = 'manual'` when called via the user-facing API.
- `berth-rules-engine.ts` triggers set `status_override_mode = 'automated'`.
- When a backing interest is successfully created and links the berth, clear `status_override_mode` back to null in the same transaction; set `status_last_changed_reason` to "Reconciled via interest [id]".
- **Effort:** S.

### Phase 2 — Visual indicator

- On berth list rows: small chip "Manual" next to the status badge when `status_override_mode = 'manual'` AND no active interest is linked.
- On berth detail page header: badge + tooltip showing last reason, user, when.
- On dashboard "Berth Heat" widget: filter or annotate the manual rows.
- **Effort:** S.

### Phase 3 — Reconciliation Queue page

- New page `/[portSlug]/admin/berths/reconcile`.
- Lists every berth where `status_override_mode = 'manual'` and no active interest. Sortable by `status_last_modified DESC`.
- Each row links to the catch-up wizard.
- Sidebar Admin section gets a link with the queue count badge.
- **Effort:** S.

### Phase 4 — Catch-Up Wizard (the core piece)

- Multi-step modal. Steps:
  1. **Pick or create client** — combobox + inline quick-create (name + email only).
  2. **Pick or create yacht** — optional if pre-EOI; quick-create with name + dimensions.
  3. **Pick the matching stage** — based on current berth status:
     - `under_offer` → enquiry / qualified / nurturing / eoi (default eoi)
     - `sold` → contract + outcome=won
     - Allow override.
  4. **Upload existing docs** — EOI PDF, contract PDF, reservation form. Each auto-filed to the right entity folder.
  5. **Optional payments** — if status=sold, prompt for deposit/full amount.
  6. **Review + submit.** On submit, transaction:
     - Create/select client + yacht
     - Create interest at chosen stage with `assigned_to = current user`
     - Upsert `interest_berths(is_primary=true, is_specific_interest=true, is_in_eoi_bundle=true)`
     - Upload + attach files
     - Insert payments
     - Set `berth.status_override_mode = null` + `status_last_changed_reason = 'Reconciled via interest [id]'`
     - Audit log single "reconcile" event linking berth + new interest.
- **Effort:** M (wizard) + S (transaction service) + S (API endpoint). Total M-L.

### Phase 5 — Entry points

- Berth list row menu → "Catch up..."
- Berth detail page next to manual badge → "Catch up"
- Dashboard widget "Manual statuses awaiting reconciliation" (count + link)
- Sidebar link
- **Effort:** S.

### Total feature effort: M-L (2-3 dev days).

---

## What I Tested in Round 2 (15 deep journeys, all passed structural validation)

| Journey                                      | Result                                                                                      |
| -------------------------------------------- | ------------------------------------------------------------------------------------------- |
| State machine — stage skipping               | ✓ Rejects forward/backward jumps with friendly copy + override path                         |
| Double outcome write                         | ⚠ Allowed (won→lost flips freely); audit log just says "update" — should tag outcome change |
| Cascade — delete with dependents             | ✗ Inconsistent: clients soft-archive, **berths HARD-delete**, companies soft-archive        |
| Manual berth status without backing interest | ✗ Foundation column exists, never written                                                   |
| Unicode (emoji/RTL/zero-width)               | ⚠ Emoji + RTL OK; zero-width chars NOT stripped (search blind spot)                         |
| Storage / file upload magic-byte             | ✓ Rejects JPEG/HTML disguised as PDF                                                        |
| Documenso webhook idempotency                | ✓ Timing-safe + rate-limited bad-secret check                                               |
| Berth recommender edge cases                 | ⚠ Empty dims OK; extreme dims return empty; **limit param ignored**                         |
| Email body XSS via markdown                  | ✓ Escape-first-then-rules, javascript: URLs stripped                                        |
| Public berth feed correctness                | ✓ Port allow-list, archive filter, status enum validation                                   |
| Rate limiting / abuse                        | ✗ Login: no rate limit; public feed: CDN-cached                                             |
| Health check + dependency probes             | ✓ Anonymous minimal payload, secret-mode for website-intake                                 |
| Direct ID enumeration                        | ✓ Uniform 404 — no leak                                                                     |
| Cross-port API access                        | ✓ 404 at API; **silent at UI**                                                              |
| CSRF — fake Origin                           | ✓ Prod-only protection — dev intentionally skips                                            |

---

## Recommended Commit Sequence

1. **Squash-commit T0 fixes** (F1 + F2) — these are deploy-blockers already applied. Push to main.
2. **T1 batch commit** (F3, F4, F5, F6, F7, F8) — pre-deploy blockers. Single commit per fix for clean review.
3. **T2** (F9, F10) — schedule for next sprint (F9 is architectural).
4. **T3** (F11-F18) — knock out in a few hours. Quick polish wave.
5. **T4** (F19-F30) — UX list. Bundle into a single PR over a few sessions.
6. **NEW FEATURE — Catch-Up Workflow** — 2-3 dev days. Higher business value than T2; prioritize after T1.

---

## Risk Notes

- The audit polluted the dev DB with test entities: `Smoke Test Client (renamed)`, `Aurora Marine Holdings Ltd`, `Bad Email Test`, `Phone Test`, `Robert'; DROP TABLE clients`, `François 🏄 المعتمد`, `محمد عبد الله`, `CSRF Test`, etc. Also **hard-deleted berth A1 in port-amador** + soft-archived Test Person 1. Consider `pnpm db:reseed:synthetic` before the next clean run.
- The Smoke Test Client interest had `outcome=lost_other` set during the won-then-lost test (R2-B). Audit log preserved both transitions but with action="update" not action="outcome_change".