Replaces the bare "+ New document" Button on the documents hub with a
NewDocumentMenu dropdown so reps explicitly pick between:
- "Upload file" → opens a Dialog with FileUploadZone scoped to the
current folder + entity context. No signing flow attached.
- "Generate document for signing" → navigates to /documents/new wizard.
Avoids the prior ambiguity where reps clicked "+ New document" intending
to attach a file and were dropped into the Documenso signer wizard.
Also adds FolderDropZone wrapping FlatFolderListing and EntityFolderView.
Dragging files from the OS over the current folder shows a drop overlay;
drop fires N parallel uploads carrying the folder + entity context.
Mirrors the per-entity Files tab UX but works in-place on the hub.
Both surfaces hit /api/v1/files/upload with folderId + entityType/Id +
the legacy clientId/companyId/yachtId FKs so files land on the right
entity AND inside the correct folder.
Also includes the in-flight prettier reformat from lint-staged on a
few previously-touched files (create-document-wizard, file-upload-zone,
admin/documenso/page) and adds the standalone prod-readiness audit
report to docs/superpowers/audits/ for permanent reference.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
**Test posture at audit time:** 1287/1287 unit + integration pass. TypeScript clean (4 pre-existing errors: 1 stale `.next/` build artifact, 3 in a Wave 11.B-era `InMemoryBackend` test stub).
## Headline
**~28 Critical, ~38 Important, ~36 Minor findings across 17 domains.** (Original 16-domain count was 23/32/30; Audit 17 added 5/6/6.)
A handful of the Criticals are real bugs in this session's work that need to be fixed on this branch before merging to `main`. A few are long-standing gaps that survived multiple iterations (storage migration script, `.env.example` URL) and should be fixed independently of this branch but before any prod cutover. Several are mobile/a11y issues that were never going to be caught without a running dev server, which the implementation pass didn't have.
**Recommendation:** fix the 23 Criticals before merging this branch. Triage Importants into "fix-before-prod" vs "follow-up-on-main". Minors → backlog.
Estimated effort to clear Criticals: 6-10 hours of focused work.
---
## Critical findings
Grouped by remediation domain. Each entry: brief rationale + file:line ref + fix sketch.
### A. Core feature regressions in this session's work
**A1. `handleDocumentCompleted` is not idempotent — Documenso retries duplicate `files` rows + orphan blobs**
`src/lib/services/documents.service.ts:1115`
`resolveWebhookDocument` returns the doc regardless of `status`. Two webhook deliveries (Documenso retries on 5xx) can both pass through and both insert `files` rows; the second `UPDATE documents SET signedFileId` clobbers the first and the first blob is permanently orphaned in storage with no DB row.
**Fix:**`if (doc.status === 'completed' && doc.signedFileId) return;` immediately after `resolveWebhookDocument`. Standard idempotency gate for this pattern.
**A2. Realtime hookup dropped by hub rebuild — multi-rep stale data**
The pre-rebuild hub consumed `document:*` and `file:*` Socket.IO events via `useRealtimeInvalidation`. After the rebuild, both `HubRootView` and `EntityFolderView` have no realtime subscription at all. The remaining hook lives inside `FlatFolderListing`, which is torn down when navigating away. Result: rep A on `Clients/Smith/` will not see rep B's upload until manual refresh; webhook-completed signatures don't appear in the Signing-in-progress section.
**Fix:** lift `useRealtimeInvalidation` up to `DocumentsHub` with both `document:*` and `file:*` events targeting the prefix keys `['files']` and `['documents']`. TanStack Query prefix matching will invalidate the aggregated keys.
**A3. LEFT JOIN port_id in ON clause defeats `idx_docs_signed_file_id`**
Planner picks `idx_docs_port` and applies `signed_file_id = f.id` as a residual filter. At scale this is 20 × N comparisons per page load instead of 20 point lookups. Same pattern in `documents.service.ts:1915` for the workflow projection.
**Fix:** drop `AND d.port_id = portId` from the ON clause and add `AND (d.port_id = portId OR d.id IS NULL)` to the outer WHERE. Or add a composite `(signed_file_id, port_id)` index. `files.port_id` is already scoped, so cross-port leak risk is zero.
**A4. Importer doesn't set `files.folder_id` — imported files invisible to folder queries**
`scripts/import-organized-documents.ts:196-208`
The `documents` row gets `folderId` correctly (line 216) but the companion `files` row does not. `files.folder_id` is a separate column. The backfill won't rescue these — it only acts on files with entity FKs set, and the importer sets none of those either.
**Fix:** copy `folderId` into the `files.values(...)` block alongside the document insert.
**A5. `chk_system_folder_shape` has NULL escape — corrupted system rows persist**
`NOT system_managed OR entity_type = 'root' OR (...)` evaluates to `NULL` (not `false`) when `entity_type IS NULL` and `system_managed = true`. Postgres treats NULL as "not false" so the constraint passes. Confirmed by direct insert test.
**Fix:** add `entity_type IS NOT NULL` to the constraint, or restructure as `CHECK (NOT system_managed OR (entity_type IS NOT NULL AND (entity_type = 'root' OR (entity_type = ANY(...) AND entity_id IS NOT NULL))))`.
**A6. `document-folders.service.ts` has zero log lines — silent failures across the entire folder service**
`src/lib/services/document-folders.service.ts` (no `logger` import)
Orphan rows in `listTree` are silently dropped (line 83-84). The 50-attempt suffix-loop exhaustion throws `ConflictError` with no log. `ensureSystemRoots` "missing root after upsert" throws raw `Error`. At 3am you would have no diagnostic for folder-related failures.
**Fix:**`import { logger } from '@/lib/logger'`. Add `logger.warn` on orphan-detection, retry-exhaustion (both `ensureEntityFolder` and `syncEntityFolderName`), and the missing-root invariant in `ensureSystemRoots`.
**A7. `demoteSystemFolderOnEntityDelete` is not wired into `client-hard-delete.service.ts`**
`src/lib/services/document-folders.service.ts:650` (exported but zero callers)
`client-hard-delete.service.ts` exists. It clears entity FKs on `files` and `documents` inside its transaction but never demotes the system folder. After hard-delete: folder retains `system_managed=true` + the dead `entity_id`. The partial unique index `uniq_document_folders_entity` permanently blocks any future client folder that would get the same display name. Also a GDPR right-to-be-forgotten gap.
**Fix:** call `demoteSystemFolderOnEntityDelete(portId, 'client', clientId)` inside `hardDeleteClient`'s transaction (or as a post-commit hook with audit log). Confirm whether `companies`/`yachts` have analogous hard-delete services that also need wiring.
### B. Accessibility blockers (WCAG 2.1 AA failures)
**B1. Unlabeled search input**
`src/components/documents/documents-hub.tsx:265`
`<Input placeholder="Search by title..." />` — placeholder is not a label. Fails WCAG 1.3.1 / 4.1.2.
The expand button has `aria-label="Collapse"` / `"Expand"` with no folder name, so SR users hear "Expand button, Expand button…" with no differentiation. And it lacks `aria-expanded` so the open/closed state is invisible.
**Fix:**`aria-expanded={open}`, `aria-label={\`${open ? 'Collapse' : 'Expand'} ${node.name}\`}`. Same pattern in `documents-hub.tsx:210-217` for the per-row signer expand.
**B4. `aria-label` on Lock SVG becomes part of button's accessible name**
`<Lock aria-label="System folder" />` inside the folder-select `<button>` produces accessible name "Smith System folder" rather than a separate badge announcement.
**Fix:** `aria-hidden="true"` on the SVG + `<span className="sr-only"> (system folder)</span>` after the folder name.
### C. Mobile blockers
**C1. FolderTreeSidebar stacks above main panel with no collapse toggle**
On mobile the entire folder tree renders above the document list. With any non-trivial tree, reps scroll past it to reach content. Every other secondary-nav page uses a Sheet or Collapsible.
**Fix:** wrap in a Sheet drawer (default closed on mobile) with a "Show folders" trigger button.
**C2. `border-r` on wrong axis at mobile breakpoint**
**Fix:** `min-h-[44px]` + `py-2` (or `py-1.5`) on each. Or wrap in `<Button size="sm">` where the visual change is acceptable.
### D. Long-standing infra gaps (independent of this branch, must fix before prod)
**D1. `migrate-storage.ts` migrates zero files — silent footgun**
`src/lib/storage/migrate.ts:40-43`
`TABLES_WITH_STORAGE_KEYS` is an empty array. The comment says "Phase 6a ships an empty list" — never followed up. Running `pnpm tsx scripts/migrate-storage.ts` flips the active backend but migrates nothing. Existing blobs in `files`, `berth_pdf_versions`, `brochure_versions`, `gdpr_exports`, `report_snapshots` become unreachable.
**Fix:** populate the table list with all five tables + their `storagePath`/`storageKey` columns. The `copyAndVerify` SHA-256 round-trip already works; it just needs entries to act on.
**D2. `.env.example` DOCUMENSO_API_URL has `/api/v1` baked in → double-path URLs**
`.env.example`
Current value: `DOCUMENSO_API_URL=https://documenso.example.com/api/v1`. The client appends `/api/v1/documents` etc., producing `https://documenso.example.com/api/v1/api/v1/documents`. Anyone copying the example file gets 404s from Documenso with no diagnostic. Applies to both v1 and v2 deployments.
**Fix:** change to `DOCUMENSO_API_URL=https://documenso.example.com` (bare host). Update the admin UI placeholder to match.
When the API setup step (client create, file upload, file list) returns non-2xx, the test calls `test.skip(true, ...)` and proceeds no further. Playwright reports skipped tests as passed — a green CI run hides whether the actual assertion would have succeeded.
**Fix:** convert skip-on-non-ok to `expect.fail()` so a 401 on setup becomes a real test failure. Skip should only fire when the precondition is genuinely "this scenario doesn't apply", not "the infrastructure broke".
### F. Webhook event coverage gap (with v1 + v2 support in scope)
**F1. `DOCUMENT_DECLINED` has no handler**
`src/app/api/webhooks/documenso/route.ts:146-214`
v2 distinguishes Decline (recipient refuses) from Reject (admin cancels). The switch handles `DOCUMENT_REJECTED` only. A v2-declined document leaves the CRM document in `sent` status indefinitely; the poller doesn't catch it either (only checks `COMPLETED` and `EXPIRED`).
**Fix:** add a `DOCUMENT_DECLINED` case to the switch. Behaviorally mirror `DOCUMENT_REJECTED` initially; product can refine if Decline vs Reject should differentiate downstream.
---
## Important findings (fix before prod, or as follow-up on `main`)
Listed by audit domain. Each has a file:line ref in its source audit; I'll quote the highlights here for triage.
### Security
- **`storagePath` + `storageBucket` exposed via aggregated files API** (`files.ts:533-534`) — internal storage paths reach authenticated rep clients via `GET /api/v1/files?entityType=X`. Auditors flagged this from both Security and Integration angles. Sanitize at service layer.
- **Missing `portId` on UPDATE in folder-move route** (`api/v1/documents/[id]/folder/route.ts:41-44`) — pre-flight read scopes by portId so no current exploit, but defense-in-depth gap that breaks if pre-flight is ever refactored.
- **Signer emails exposed to all `documents.view` holders** — confirm with product whether read-only roles should see signatory email addresses or get them redacted.
### Database / Migration
- **`uniq_document_folders_entity` doesn't cover `entity_type = NULL`** — rows with NULL entity_type but non-NULL entity_id can duplicate. Closes when CHECK constraint is tightened (A5 above).
- **Backfill transaction holds advisory lock across N `ensureEntityFolder` calls** — at 10k files the lock is held for minutes. Batch in chunks of 500.
- **`CREATE INDEX` without `CONCURRENTLY`** in migration 0051 — blocks writes briefly. Quantify: short-duration on small tables, moderate on prod-sized. Split for zero-downtime if needed.
### Concurrency / Error Paths
- **Storage blob orphaned on DB-insert failure** in `handleDocumentCompleted` — `storage.put` before `db.insert(files)`. No janitor. Long-standing tradeoff; document explicitly.
- **`ensureSystemRoots`/`ensureEntityFolder` outside backfill transaction** — folder rows persist if the wrapping tx rolls back. Idempotent so re-run heals.
- **`syncEntityFolderName` 50-attempt cap with concurrent renames to same target** — silent log + stale folder name. Accepted divergence.
### Performance
- **N+1 grows with linked entities** — leasing company with 50 yachts = 110 queries per page load. Worst case (5 companies + 100 yachts) = 216. Acceptable for now; future optimization: single CTE with grouping.
- **Count queries can collapse via window function** — `count(*) OVER ()` halves round-trip count at scale.
- **Missing composite indexes `(port_id, client_id)` / `(port_id, company_id)` / `(port_id, yacht_id)` on `files`** — same for `documents`. Add before prod backfill at scale.
- **`listDocuments` calls `listTree()` twice when `includeDescendants=true`** — pass already-fetched tree into `hydrateDocumentsWithDownloadUrl`.
### Data migration (importer)
- **System-root collision risk** — bucket folders named `Clients`/`Companies`/`Yachts` silently merge into auto-created system roots. Add a pre-flight check that warns when any top-level segment matches a system root name.
### Observability
- **Archive/restore hooks missing `portId` in log context** (`companies.service.ts:215`, `yachts.service.ts:193`) — clients has it; companies and yachts don't.
- **Backfill CLI has no row-count telemetry** — only "Backfill complete" on success. Want files-processed / folders-created / FKs-propagated counts.
- **No log on empty aggregated projection** — `assertEntityInPort` returning false produces a silent empty result. Log warn with `portId + entityType + entityId`.
- **Em-dash in `SigningDetailsDialog` description** (line 62) — user-facing copy.
- **Em-dashes baked into aggregated group labels** (`FROM COMPANY — ACME CORP`) — rendered on every entity folder view. `files.ts:335`, `documents.service.ts:1877`. Replace with colon or slash.
- **Mixed `Loading...` (ASCII) and `Loading…` (Unicode ellipsis)** across components. Normalize.
- **Raw `partially_signed` status in `HubRootView`** — no StatusPill or underscore replacement. Apply `StatusPill` or at minimum `replace(/_/g, ' ')`.
- **"view signing details" button too subtle** — inline-text in a tight muted cluster, blends into the date. Consider `<Button variant="ghost" size="sm">`.
- **Documenso poll worker double-fire of `handleDocumentCompleted`** writes a second blob + second `files` row and overwrites `signedFileId`. Confirmed by both concurrency and integration audits. Resolved by A1's idempotency gate.
- **MinIO operations have no socket timeout** — TCP blackhole stalls workers indefinitely. `fetchWithTimeout` doesn't cover the minio client's `putObject`/`getObject`. Wrap with an external timeout (`AbortController` or `Promise.race`).
- **No 0-byte check on `downloadSignedPdf` result** — a 0-byte response from Documenso writes a permanent corrupt `signedFileId` with no recovery path.
- **`DOCUMENSO_API_VERSION` env defaults to `v1`** with no documentation in `.env.example` that v2 is supported. A v2-pointed deployment that misses the env var fires v1 code paths against a v2 instance.
- **`DOCUMENT_DECLINED` event handler** — already listed as Critical F1; mentioned again here because the integration audit captured it under v2-specific gaps.
- **`RECIPIENT_VIEWED` / `RECIPIENT_SIGNED`** v2 event aliases — currently silently dropped. Confirm whether v2 actually fires these or maps to `DOCUMENT_OPENED` / `DOCUMENT_SIGNED` like v1. If v2 fires them, add handlers.
### Realtime / Socket.IO
- **`useRealtimeInvalidation` is inside `FlatFolderListing`, not `DocumentsHub`** — torn down when navigating away. Lifting to DocumentsHub closes this and unblocks A2 cleanly.
- **`['document-folders']` query key has no realtime invalidation path** — rep B renaming a folder takes up to 30s `staleTime` to surface for rep A. Add a folder-rename socket emit + invalidate.
### Audit log completeness
- **`createFolder` has no audit log** (line 102-136) — inconsistent with rename/move/delete which all audit.
- **`handleDocumentCompleted` file insert has no audit** (line 1163-1180) — signed PDFs created with no audit trail.
- **`syncEntityFolderName` ignores `_userId`** — folder renames driven by entity rename leave no audit trail.
- **Archive/restore suffix helpers no audit** — parent entity action audits, but folder mutation doesn't.
### Type-safety
- **`entityType as 'client'|'company'|'yacht'`** in `documents-hub.tsx:134` — no runtime guard. Fix with `ENTITY_TYPES.has()`.
- **`INFLIGHT_STATUSES as unknown as string[]`** — replace with `[...INFLIGHT_STATUSES]`.
- **Loose `files?/workflows?` union + unconstrained `T`** in `AggregatedSection` — refactor to discriminated union + `T extends { id: string }`.
### Test quality
- **`mapWorkflowStatus` `partially_signed` fix has no regression test**.
- **`applyEntityRestoredSuffix` "restore without prior archive" path not tested**.
- **`folderId="" → null` validator transform has zero test coverage**.
- **`syncEntityFolderName` collision beyond `(2)` untested** — if `isSiblingNameConflict` ever mis-classifies the error shape, retries never fire and the test wouldn't notice.
### Mobile
- **DocumentsHub sets no `useMobileChrome`/`setChrome` title** — falls back to URL-segment title-casing.
- **FolderActionsMenu trigger overrides to 28×28px** — should use default `size="icon"` (44×44).
- **SigningDetailsDialog signer email no `truncate`** — long emails overflow on narrow viewports.
- **Breadcrumb tap targets too small** (`folder-breadcrumb.tsx:41-60`) — no padding.
---
## Minor (backlog)
Approximately 30 minor findings across all domains. Highlights:
- **Em-dashes in `CLAUDE.md`** (29 in prose bullets, all in pre-existing content; no new em-dashes added in commit `ab79894`) — backlog cleanup pass.
- **`@radix-ui/react-icons` unused** — safe to remove from `package.json`.
- **`@hookform/resolvers`, `zod`, `tailwindcss`** all have major-version updates available — DO NOT upgrade pre-cutover (breaking changes).
- **Sonnet color contrast on `muted-foreground/70` opacity variant** (`aggregated-section.tsx:94`) — ~3.2:1 fails WCAG AA for normal text. Drop the `/70` tint.
- **`<header>` element inside `<div>` not under a sectioning element** (`aggregated-section.tsx:92`) — wrong landmark scope; use `<div>` or `<h6>`.
- **`h3` → `h5` jump in SigningDetailsDialog** (skipped heading level).
- **`renameFolder` `updatedAt` test uses 10ms `setTimeout`** — fragile but `toBeGreaterThan` is OK; can drop the sleep entirely.
- **`MINIO_AUTO_CREATE_BUCKET`** bypasses zod env schema; undocumented in `.env.example`.
- **`DOCUMENSO_TEMPLATE_ID_EOI` + recipient ID vars absent from `.env.example`** with Port-Nimara-specific hardcoded defaults.
- **`voidDocument` raw `FetchTimeoutError` propagation** — no `CodedError('DOCUMENSO_TIMEOUT')` wrap. Both call sites handle gracefully; cosmetic.
---
## Audit-by-audit completion log
| # | Audit | Status | Critical | Important | Minor |
- Important UI/UX (em-dashes, loading state consistency, status pill on HubRootView).
- Important audit log completeness.
- Important type-safety tightening.
- All Minor.
---
## Notes on session vs. pre-existing findings
Several Criticals (D1 storage migration script, D2 .env.example, A3 LEFT JOIN port_id, parts of the audit-log gaps and observability gaps) are long-standing — they survived multiple iterations of the codebase, sometimes since Phase 6a. Fixing them on this branch is fine but they're not regressions introduced by this session.
The session's actual regressions are: A1 (idempotency), A2 (realtime), A5 (CHECK NULL), A6 (folder service has no logger), A7 (demote not wired), B1-B4 (a11y missed during the UI rebuild), C1-C7 (mobile never tested), E1 (test theatre).
The dependency, integration-conformance (Context7), and type-safety audits are clean of Critical findings — your dep posture is solid and the implementation follows published specs.
---
## Audit 17 — Data structure & sales process completeness
**5 Critical, 6 Important, 6 Minor.** This audit walked the entire entity graph and the sales-process pipeline end-to-end. Most findings are not regressions from this session — they are gaps in the sales-process plumbing that pre-date the documents-hub-split work but matter for prod cutover. C-1 and C-3 are session-introduced; C-2, C-4, C-5 are long-standing.
### Critical (data graph + sales pipeline)
**G-C1. `deleteFolderSoftRescue` re-parents documents but not files — split delete behavior**
The soft-rescue transaction `UPDATE`s `documents.folderId = newParent`, then deletes the folder row. The schema cascade on `files.folderId` is `ON DELETE SET NULL` (not `SET DEFAULT newParent`) — so any files in the deleted folder land at **root**, while documents in the same folder correctly land at the deleted folder's **parent**. A folder containing both will scatter on delete.
Fix: inside the transaction, between the documents UPDATE and the folder DELETE:
`scratchpadNotes.linkedClientId references clients.id` with no `onDelete` → defaults to RESTRICT. The hard-delete service nullifies six nullable FKs (files, documents, formSubmissions, emailThreads, reminders, documentSends) but skips `scratchpadNotes`. Any rep who scratchpad-linked a note to a client → hard-delete throws an FK violation and aborts the transaction.
The unique index `uniq_document_folders_entity` on `(portId, entityType, entityId)` enforces a singleton system folder per entity. Hard-delete removes the client row but does not call `demoteSystemFolderOnEntityDelete`. The folder persists with `systemManaged=true, entityType='client', entityId=<deleted-id>` — invisible in the sidebar but holding the unique slot.
Fix: after the client delete, fire-and-forget the demote:
(This is the same wire-up A7 in the main report flagged — confirmed missing on the hard-delete pathway specifically.)
**G-C4. Five of seven berth-rule triggers are defined but never called**
`src/lib/services/berth-rules-engine.ts:37-44` vs `src/lib/services/documents.service.ts:798,894,1234`
`DEFAULT_RULES` defines triggers for `eoi_sent`, `eoi_signed`, `deposit_received`, `contract_signed`, `interest_archived`, `interest_completed`, `berth_unlinked`. Only `eoi_sent` and `eoi_signed` are passed to `evaluateRule` anywhere in the codebase.
Concrete consequences:
- Deposit received (invoice paid) → no berth state change. Should auto-mark berth as Sold.
- Contract signed → no berth state change.
- Interest archived → no "berth available" suggestion fires.
- Interest marked Won/Lost → no rule trigger.
- Interest unlinked from berth → no rule trigger (off-by-default, but configurable and silently dead).
- `interests.service.ts:archiveInterest` after `softDelete`: fetch primary berth via `getPrimaryBerth`, then `void evaluateRule('interest_archived', ...)`.
- `interests.service.ts:setInterestOutcome` after the outcome write: `void evaluateRule('interest_completed', ...)`.
- `interest-berths.service.ts:removeInterestBerth` after delete: `void evaluateRule('berth_unlinked', ...)`.
**G-C5. `contract_sent` and `contract_signed` pipeline stages have zero auto-advancement triggers**
`src/lib/services/documents.service.ts` (absent)
`STAGE_TRANSITIONS` defines `contract_sent` and `contract_signed` and they render in the Kanban/funnel UI, but no code path calls `advanceStageIfBehind(..., 'contract_sent')` or `advanceStageIfBehind(..., 'contract_signed')`. Sending a reservation agreement → no stage advance. Completing one (signed PDF arrives, `contractFileId` set in `handleDocumentCompleted` ~line 887) → no stage advance.
Effect: deals stall at whatever stage they hit when the reservation agreement was sent, until a rep manually drags them in the Kanban.
Fix: in `documents.service.ts`:
- `sendDocument` pathway (~line 798): if `doc.documentType === 'reservation_agreement'`, fire `advanceStageIfBehind(..., 'contract_sent', meta, 'Reservation agreement sent')`.
- `handleDocumentCompleted` (~line 887, where `contractFileId` is set): fire `advanceStageIfBehind(..., 'contract_signed', meta, 'Reservation agreement signed')` and `evaluateRule('contract_signed', ...)`.
### Important (cross-entity gaps)
**G-I1. Portal email uniqueness is global, not per-port**
A client who has dealt with two ports under this deployment can only ever have one portal account. The second `createPortalUser` will throw a unique-constraint violation. Make per-port (`.on(table.email, table.portId)`) if multi-port is a real deployment scenario, or document as single-port-only.
**G-I2. `archiveInterest` skips `interest_archived` rule and `notifyNextInLine`**
`src/lib/services/interests.service.ts:985-1014`
Archive does the audit log + socket emit but does not (a) trigger the berth-availability rule, (b) notify the waiting list for the primary berth. The waiting-list code is only fired when the **client** is archived, not the **interest**.
Archive sides call `applyEntityArchivedSuffix`. Restore paths do not exist for yachts/companies at all today — but when they are added (or if the entity-restoration logic moves to the `clients/archive` parity routes), `applyEntityRestoredSuffix` must be wired. `clients.service.ts:596` already does this correctly.
**G-I4. `berthRecommendations.interestId` has no FK constraint**
`src/lib/db/schema/berths.ts:134` — column comment says "references interests.id" but `.references()` is omitted.
If an interest is hard-deleted (currently only possible via `db:studio` or future migrations), stale `berthRecommendations` rows persist and skew the recommender's tier aggregates. Add `.references(() => interests.id, { onDelete: 'cascade' })` and generate a migration.
**G-I5. Portal invoices invisible for company-billed deals**
`src/lib/services/portal.service.ts:232`
`getClientInvoices` matches on `billingEmail in client.emails`. Invoices with `billingEntityType='company'` (the most common B2B pattern: client is an individual buying through their company) are not surfaced even when the client is the company's director. Extend the query to OR-in invoices where `billingEntityType='company' AND company.directorClientId = portalUser.clientId`.
**G-I6. `hub-counts` API endpoint is orphaned**
`src/app/api/v1/documents/hub-counts/route.ts:5-10` + `getHubTabCounts` in `documents.service.ts:397`
The hub rebuild on this branch removed the component that called this endpoint. Service function + route are dead code. Either wire a KPI strip back into `HubRootView` (the spec does call for this) or delete the route + service function.
### Minor
- **G-M1.** Website inquiry → client conversion is fully manual; `prefill_*` query params are hints only. `inquiry-inbox.tsx:119`.
- **G-M2.** Polymorphic array columns (`photoFileIds`, `attachmentFileIds`) have no FK protection. Files deleted via any future hard-purge path silently orphan these arrays.
- **G-M3.** `berthReservations.interestId` RESTRICT default (notNull, no `onDelete`) — intent (preserve history vs oversight) undocumented.
- **G-M4.** `setInterestOutcome` to `won` does not fire berth-sold; downstream of G-C4.
- **G-M5.** `advanceStageIfBehind` silently no-ops when `yachtId` is null at `open` stage. Walk-in EOIs (vessel not yet identified) stall invisibly at `open`.
- **G-M6.** `removeInterestBerth` emits socket + webhook but skips `evaluateRule('berth_unlinked')`. Downstream of G-C4.
### Impact on cutover gate
- **G-C2** is the most pressing for cutover: it is a hard error on a foreseeable action (any rep deleting a client with a linked scratchpad note → 500). Fix before any team testing.
- **G-C4 + G-C5** mean the berth-map status and Kanban columns will drift visually for every deal that progresses past EOI. This is not data corruption, but it will erode rep trust quickly during initial team testing. Fix before cutover.
- **G-C1** is a UX correctness issue; will surprise reps but won't lose data. Same-branch fix.
- **G-C3** is data-integrity hygiene; no immediate user-visible effect but pollutes the unique-folder slot. Same-branch fix.
### Updated headline
With Audit 17 folded in, the corrected count is **~28 Critical, ~38 Important, ~36 Minor** across 17 domains. The new Criticals (G-C2, G-C4, G-C5) are long-standing pre-existing gaps in the sales pipeline — they don't block this branch's merge to `main`, but they block prod cutover. G-C1 and G-C3 are this-branch issues and should be folded into the same fix pass as A1-A7.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
