Lint-staged reformat after the previous commit added the file. Same content, prettier's preferred line wrap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
34 KiB
Prod-Readiness Audit — feat/documents-folders
Date: 2026-05-11
Branch: feat/documents-folders (67 commits ahead of main; 34 from this session's documents-hub-split work + 33 from Wave 11.B)
Scope: 17 parallel domain audits (data-structure & sales-process completeness appended at bottom)
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
src/components/documents/hub-root-view.tsx, src/components/documents/entity-folder-view.tsx
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
src/lib/services/files.ts:544
LEFT JOIN documents d ON d.signed_file_id = f.id AND d.port_id = $portId
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
src/lib/db/migrations/0051_documents_hub_split.sql:22-28
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.
Fix: aria-label="Search documents by title".
B2. No aria-pressed on type-filter chips
src/components/documents/documents-hub.tsx:276-299
Active state is purely visual. Screen readers can't tell which chip is selected. Fails WCAG 4.1.2.
Fix: aria-pressed={typeFilter === t} on each chip.
B3. No aria-expanded on tree chevrons; folder-row labels lack context
src/components/documents/folder-tree-sidebar.tsx:125, 135-155
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
src/components/documents/folder-tree-sidebar.tsx:150-154
<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
src/components/documents/folder-tree-sidebar.tsx:32 — w-full sm:w-60
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
src/components/documents/folder-tree-sidebar.tsx:32
Right border draws on full-width-stacked element instead of bottom separator.
Fix: border-b sm:border-r border-r-0.
C3-C7. 5 tap-target violations below WCAG 44×44px minimum
- C3: chevron expand button (
folder-tree-sidebar.tsx:125) — 20×20px - C4: row expand chevron (
documents-hub.tsx:210-216) — no sizing - C5: "view signing details" (
entity-folder-view.tsx:82-89) — ~20px tall - C6: "Show all (N)" (
aggregated-section.tsx:101-108) — ~18px tall - C7: type-filter chips (
documents-hub.tsx:277-297) —py-0.5gives ~24px
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.
E. Test theatre — assertions never run
E1. Smoke spec test.skip() guards mask infrastructure failures
tests/e2e/smoke/04-documents-hub-aggregated.spec.ts:99-104
tests/e2e/smoke/04-documents-hub-upload-into-entity.spec.ts:41, 129, 153, 165
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+storageBucketexposed via aggregated files API (files.ts:533-534) — internal storage paths reach authenticated rep clients viaGET /api/v1/files?entityType=X. Auditors flagged this from both Security and Integration angles. Sanitize at service layer.- Missing
portIdon 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.viewholders — confirm with product whether read-only roles should see signatory email addresses or get them redacted.
Database / Migration
uniq_document_folders_entitydoesn't coverentity_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
ensureEntityFoldercalls — at 10k files the lock is held for minutes. Batch in chunks of 500. CREATE INDEXwithoutCONCURRENTLYin 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.putbeforedb.insert(files). No janitor. Long-standing tradeoff; document explicitly. ensureSystemRoots/ensureEntityFolderoutside backfill transaction — folder rows persist if the wrapping tx rolls back. Idempotent so re-run heals.syncEntityFolderName50-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)onfiles— same fordocuments. Add before prod backfill at scale. listDocumentscallslistTree()twice whenincludeDescendants=true— pass already-fetched tree intohydrateDocumentsWithDownloadUrl.
Data migration (importer)
- System-root collision risk — bucket folders named
Clients/Companies/Yachtssilently 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
portIdin 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 —
assertEntityInPortreturning false produces a silent empty result. Log warn withportId + entityType + entityId. handleDocumentCompletedouter catch losesportId(line 1197).
UI/UX
- Em-dash in
SigningDetailsDialogdescription (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) andLoading…(Unicode ellipsis) across components. Normalize. - Raw
partially_signedstatus inHubRootView— no StatusPill or underscore replacement. ApplyStatusPillor at minimumreplace(/_/g, ' '). - "view signing details" button too subtle — inline-text in a tight muted cluster, blends into the date. Consider
<Button variant="ghost" size="sm">.
Integration conformance (with v1 + v2 support)
- Documenso poll worker double-fire of
handleDocumentCompletedwrites a second blob + secondfilesrow and overwritessignedFileId. Confirmed by both concurrency and integration audits. Resolved by A1's idempotency gate. - Poll worker omits
portIdwhen callinghandleRecipientSigned/handleDocumentCompleted— multi-port correctness risk. - MinIO operations have no socket timeout — TCP blackhole stalls workers indefinitely.
fetchWithTimeoutdoesn't cover the minio client'sputObject/getObject. Wrap with an external timeout (AbortControllerorPromise.race). - No 0-byte check on
downloadSignedPdfresult — a 0-byte response from Documenso writes a permanent corruptsignedFileIdwith no recovery path. DOCUMENSO_API_VERSIONenv defaults tov1with no documentation in.env.examplethat v2 is supported. A v2-pointed deployment that misses the env var fires v1 code paths against a v2 instance.DOCUMENT_DECLINEDevent handler — already listed as Critical F1; mentioned again here because the integration audit captured it under v2-specific gaps.RECIPIENT_VIEWED/RECIPIENT_SIGNEDv2 event aliases — currently silently dropped. Confirm whether v2 actually fires these or maps toDOCUMENT_OPENED/DOCUMENT_SIGNEDlike v1. If v2 fires them, add handlers.
Realtime / Socket.IO
useRealtimeInvalidationis insideFlatFolderListing, notDocumentsHub— 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 30sstaleTimeto surface for rep A. Add a folder-rename socket emit + invalidate.
Audit log completeness
createFolderhas no audit log (line 102-136) — inconsistent with rename/move/delete which all audit.handleDocumentCompletedfile insert has no audit (line 1163-1180) — signed PDFs created with no audit trail.syncEntityFolderNameignores_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'indocuments-hub.tsx:134— no runtime guard. Fix withENTITY_TYPES.has().INFLIGHT_STATUSES as unknown as string[]— replace with[...INFLIGHT_STATUSES].- Loose
files?/workflows?union + unconstrainedTinAggregatedSection— refactor to discriminated union +T extends { id: string }.
Test quality
mapWorkflowStatuspartially_signedfix has no regression test.applyEntityRestoredSuffix"restore without prior archive" path not tested.folderId="" → nullvalidator transform has zero test coverage.syncEntityFolderNamecollision beyond(2)untested — ifisSiblingNameConflictever mis-classifies the error shape, retries never fire and the test wouldn't notice.
Mobile
- DocumentsHub sets no
useMobileChrome/setChrometitle — 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 commitab79894) — backlog cleanup pass. @radix-ui/react-iconsunused — safe to remove frompackage.json.@hookform/resolvers,zod,tailwindcssall have major-version updates available — DO NOT upgrade pre-cutover (breaking changes).- Sonnet color contrast on
muted-foreground/70opacity variant (aggregated-section.tsx:94) — ~3.2:1 fails WCAG AA for normal text. Drop the/70tint. <header>element inside<div>not under a sectioning element (aggregated-section.tsx:92) — wrong landmark scope; use<div>or<h6>.h3→h5jump in SigningDetailsDialog (skipped heading level).renameFolderupdatedAttest uses 10mssetTimeout— fragile buttoBeGreaterThanis OK; can drop the sleep entirely.MINIO_AUTO_CREATE_BUCKETbypasses zod env schema; undocumented in.env.example.DOCUMENSO_TEMPLATE_ID_EOI+ recipient ID vars absent from.env.examplewith Port-Nimara-specific hardcoded defaults.voidDocumentrawFetchTimeoutErrorpropagation — noCodedError('DOCUMENSO_TIMEOUT')wrap. Both call sites handle gracefully; cosmetic.
Audit-by-audit completion log
| # | Audit | Status | Critical | Important | Minor |
|---|---|---|---|---|---|
| 1 | Security & multi-tenant isolation | ✓ | 0 | 3 | 0 |
| 2 | Database & migration safety | ✓ | 1 | 3 | 3 |
| 3 | Concurrency, idempotency, error paths | ✓ | 1 | 3 | 3 |
| 4 | Performance & query plans | ✓ | 1 | 3 | 3 |
| 5 | Data migration from old system | ✓ | 1 | 1 | 3 |
| 6 | Production observability | ✓ | 2 | 4 | 3 |
| 7 | UI/UX | ✓ | 0 | 5 | 4 |
| 8 | Integration conformance (Context7) | ✓ | 0 | 0 | 3 |
| 9 | Dependency audit | ✓ | 0 | 0 | ~10 |
| 10 | Accessibility (WCAG 2.1 AA) | ✓ | 4 | 5 | 4 |
| 11 | Test quality & coverage | ✓ | 2 | 6 | 3 |
| 12 | Realtime / Socket.IO | ✓ | 3 | 2 | 1 |
| 13 | Audit log completeness | ✓ | 0 | 4 | 4 |
| 14 | Type-safety | ✓ | 0 | 3 | 3 |
| 15 | Mobile / responsive | ✓ | 6 | 5 | 3 |
| 16 | Integration holes (MinIO + Documenso) | ✓ | 2 | 5 | 5 |
| 17 | Data structure & sales process completeness | ✓ | 5 | 6 | 6 |
Suggested remediation order
Pre-merge (block this branch):
- A1 (concurrency idempotency) — 1 line, 5 minutes.
- A2 (realtime hookup) — ~30 min: lift one hook up two layers in component tree.
- A4 (importer folder_id) — 1 line in scripts/import-organized-documents.ts.
- A5 (CHECK NULL escape) — 1-line migration patch + re-apply.
- A6 (folder service logger) — add
import { logger }+ 3 warn calls. - A7 (demote on hard-delete) — 1 line in client-hard-delete.service.ts.
- B1-B4 (a11y) — ~30 min combined: aria attributes only.
- C1-C7 (mobile) — ~1-2 hours: Sheet wrap + tap-target padding.
- E1 (test theatre) — convert skips to fails.
- F1 (DOCUMENT_DECLINED) — add case to switch.
Pre-prod cutover (independent of branch):
- A3 (LEFT JOIN port_id) — performance fix.
- D1 (storage migration table list) — populate TABLES_WITH_STORAGE_KEYS.
- D2 (.env.example URL) — strip
/api/v1. - All Important security findings.
- 0-byte signed PDF check.
- MinIO socket timeout wrapper.
- DOCUMENSO_API_VERSION documentation + v2 event audit.
Post-prod (backlog on main):
- 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
src/lib/services/document-folders.service.ts:268-282
The soft-rescue transaction UPDATEs 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:
await tx
.update(files)
.set({ folderId: newParent })
.where(and(eq(files.folderId, folderId), eq(files.portId, portId)));
G-C2. Client hard-delete blocked by scratchpadNotes.linkedClientId RESTRICT FK
src/lib/services/client-hard-delete.service.ts:190-218 + src/lib/db/schema/system.ts:180
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.
Fix: add to the nullification block:
await tx
.update(scratchpadNotes)
.set({ linkedClientId: null })
.where(eq(scratchpadNotes.linkedClientId, args.clientId));
G-C3. Client hard-delete leaves ghost system folder with stale entityId
src/lib/services/client-hard-delete.service.ts:214-218
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:
void demoteSystemFolderOnEntityDelete(args.portId, 'client', args.clientId).catch(logger.error);
(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).
Fix sketches:
invoices.ts:741(afteradvanceStageIfBehind('deposit_10pct')):const { evaluateRule } = await import('@/lib/services/berth-rules-engine'); void evaluateRule('deposit_received', updated.interestId, portId, meta);interests.service.ts:archiveInterestaftersoftDelete: fetch primary berth viagetPrimaryBerth, thenvoid evaluateRule('interest_archived', ...).interests.service.ts:setInterestOutcomeafter the outcome write:void evaluateRule('interest_completed', ...).interest-berths.service.ts:removeInterestBerthafter 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:
sendDocumentpathway (~line 798): ifdoc.documentType === 'reservation_agreement', fireadvanceStageIfBehind(..., 'contract_sent', meta, 'Reservation agreement sent').handleDocumentCompleted(~line 887, wherecontractFileIdis set): fireadvanceStageIfBehind(..., 'contract_signed', meta, 'Reservation agreement signed')andevaluateRule('contract_signed', ...).
Important (cross-entity gaps)
G-I1. Portal email uniqueness is global, not per-port
src/lib/db/schema/portal.ts:40 — uniqueIndex('idx_portal_users_email_unique').on(table.email)
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.
Fix after softDelete: fetch primary berth → evaluateRule('interest_archived', ...) + notifyNextInLine(primaryBerth.berthId, portId, meta.userId).
G-I3. Yacht/company restore paths missing applyEntityRestoredSuffix
src/lib/services/yachts.service.ts:178 + src/lib/services/companies.service.ts:200
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.interestIdRESTRICT default (notNull, noonDelete) — intent (preserve history vs oversight) undocumented. - G-M4.
setInterestOutcometowondoes not fire berth-sold; downstream of G-C4. - G-M5.
advanceStageIfBehindsilently no-ops whenyachtIdis null atopenstage. Walk-in EOIs (vessel not yet identified) stall invisibly atopen. - G-M6.
removeInterestBerthemits socket + webhook but skipsevaluateRule('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.
Suggested remediation order — addendum
After the A/B/C/D/E/F block from the main report:
- G-C1 — files folder UPDATE in
deleteFolderSoftRescuetransaction (1-line addition). - G-C2 — nullify
scratchpadNotes.linkedClientIdinclientHardDelete(1-line addition). - G-C3 — call
demoteSystemFolderOnEntityDeleteafter client hard-delete (1-line addition). - G-C4 + G-C5 — wire 6 missing berth-rule + pipeline-advance triggers (~30 min total, spread across invoices.ts, interests.service.ts, interest-berths.service.ts, documents.service.ts).
Total addendum effort: ~1 hour for G-C1/G-C2/G-C3, ~30 min for G-C4/G-C5, plus 1 migration regen for I-4 if you choose to fix it now.