From a3e002852bf0ced7859c8cf354cc8ab83ac061c2 Mon Sep 17 00:00:00 2001 From: Matt Ciaccio Date: Tue, 5 May 2026 04:20:38 +0200 Subject: [PATCH] fix(audit-2): integration regressions + data-integrity from second-pass review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviewer agents did a second-pass deep audit of the 21-commit refactor. Eight findings; four fixed here (one was deferred with a schema comment, three were 🟡 nice-to-haves left for follow-up). Integration regressions (🟠 high): - Outbound webhook `interest.berth_linked` now fires from the new junction-add handler. Was emitting a socket-only event, leaving external integrations silent post-refactor. - Two new webhook events `interest.berth_unlinked` and `interest.berth_link_updated` added to WEBHOOK_EVENTS + INTERNAL_TO_WEBHOOK_MAP. PATCH and DELETE handlers now dispatch them alongside the existing socket emits — lifecycle parity restored. - BerthInterestPulse adds useRealtimeInvalidation for berth-link events. The query key was berth-scoped while the linked-berths dialog invalidates interest-scoped keys (no prefix match), so the pulse went stale. Bridges via the realtime hook now. Recommender semantic fix (🟠 medium-high): - aggregates CTE: active_interest_count now filters on `ib.is_specific_interest = true`, matching the public-map "Under Offer" derivation. EOI-bundle-only links no longer demote a berth to Tier C for other reps. Smoke test confirms previously-all-Tier-C results now correctly classify as Tier A. - Same CTE: `total_interest_count` uses COUNT(ib.berth_id) instead of COUNT(*) so a berth with no junction rows reports 0 (not 1 from the LEFT JOIN's NULL-right-side row). Prevents heat over-counting. Data integrity (🟠): - AcroForm tier rejects negative numerics in coerceFieldValue (was letting through `length_ft="-50"` which would poison the recommender feasibility filter on apply). - FilesystemBackend.resolveHmacSecret throws in production when storage_proxy_hmac_secret_encrypted is null. Dev still derives from BETTER_AUTH_SECRET for ergonomics; prod must explicitly configure. - Documented the circular FK between berths.current_pdf_version_id and berth_pdf_versions.id. Drizzle's `.references()` can't express the cycle so the schema column is plain text + a comment; the FK is authoritatively maintained by migration 0030. Tests still 1163/1163. tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../interests/[id]/berths/[berthId]/handlers.ts | 12 ++++++++++++ .../api/v1/interests/[id]/berths/handlers.ts | 9 +++++++++ src/components/berths/berth-interest-pulse.tsx | 17 ++++++++++++++++- src/lib/db/schema/berths.ts | 11 +++++++++++ src/lib/services/berth-pdf-parser.ts | 8 ++++++-- src/lib/services/berth-recommender.service.ts | 16 ++++++++++++++-- src/lib/services/webhook-event-map.ts | 4 ++++ src/lib/storage/filesystem.ts | 16 +++++++++++++--- 8 files changed, 85 insertions(+), 8 deletions(-) diff --git a/src/app/api/v1/interests/[id]/berths/[berthId]/handlers.ts b/src/app/api/v1/interests/[id]/berths/[berthId]/handlers.ts index 6325bbe..4c5c3bd 100644 --- a/src/app/api/v1/interests/[id]/berths/[berthId]/handlers.ts +++ b/src/app/api/v1/interests/[id]/berths/[berthId]/handlers.ts @@ -103,6 +103,12 @@ export const patchHandler: RouteHandler = async (req, ctx, params) => { interestId, berthId, }); + void import('@/lib/services/webhook-dispatch').then(({ dispatchWebhookEvent }) => + dispatchWebhookEvent(ctx.portId, 'interest:berthLinkUpdated', { + interestId, + berthId, + }), + ); return NextResponse.json({ data: updated }); } catch (error) { @@ -136,6 +142,12 @@ export const deleteHandler: RouteHandler = async (_req, ctx, params) => { interestId, berthId, }); + void import('@/lib/services/webhook-dispatch').then(({ dispatchWebhookEvent }) => + dispatchWebhookEvent(ctx.portId, 'interest:berthUnlinked', { + interestId, + berthId, + }), + ); return new NextResponse(null, { status: 204 }); } catch (error) { diff --git a/src/app/api/v1/interests/[id]/berths/handlers.ts b/src/app/api/v1/interests/[id]/berths/handlers.ts index 35c640d..1ca32d7 100644 --- a/src/app/api/v1/interests/[id]/berths/handlers.ts +++ b/src/app/api/v1/interests/[id]/berths/handlers.ts @@ -92,6 +92,15 @@ export const addHandler: RouteHandler = async (req, ctx, params) => { interestId, berthId: body.berthId, }); + // Outbound webhook: the legacy /link-berth path dispatched + // `interest.berth_linked` and external integrations subscribe to it. + // The new junction-add path must keep that contract. + void import('@/lib/services/webhook-dispatch').then(({ dispatchWebhookEvent }) => + dispatchWebhookEvent(ctx.portId, 'interest:berthLinked', { + interestId, + berthId: body.berthId, + }), + ); return NextResponse.json({ data: link }, { status: 201 }); } catch (error) { diff --git a/src/components/berths/berth-interest-pulse.tsx b/src/components/berths/berth-interest-pulse.tsx index 733c1b8..97df770 100644 --- a/src/components/berths/berth-interest-pulse.tsx +++ b/src/components/berths/berth-interest-pulse.tsx @@ -11,6 +11,7 @@ import { apiFetch } from '@/lib/api/client'; import { stageBadgeClass, stageLabel } from '@/lib/constants'; import { computeUrgencyBadges } from '@/components/interests/urgency'; import type { InterestRow } from '@/components/interests/interest-columns'; +import { useRealtimeInvalidation } from '@/hooks/use-realtime-invalidation'; import { cn } from '@/lib/utils'; interface InterestsResponse { @@ -33,8 +34,9 @@ export function BerthInterestPulse({ berthId }: { berthId: string }) { const params = useParams<{ portSlug: string }>(); const portSlug = params?.portSlug ?? ''; + const queryKey = ['interests', { berthId, sort: 'dateLastContact', order: 'desc' }]; const { data, isLoading } = useQuery({ - queryKey: ['interests', { berthId, sort: 'dateLastContact', order: 'desc' }], + queryKey, queryFn: () => apiFetch( `/api/v1/interests?berthId=${berthId}&limit=10&sort=dateLastContact&order=desc`, @@ -42,6 +44,19 @@ export function BerthInterestPulse({ berthId }: { berthId: string }) { staleTime: 30_000, }); + // Stay in sync with the linked-berths list + add-to-interest dialog. + // Each of those flows emits a realtime socket event but does NOT + // invalidate this exact query key (it's berth-scoped, theirs are + // interest-scoped) — bridge via the invalidation hook. + useRealtimeInvalidation({ + 'interest:berthLinked': [queryKey], + 'interest:berthUnlinked': [queryKey], + 'interest:berthLinkUpdated': [queryKey], + 'interest:created': [queryKey], + 'interest:stageChanged': [queryKey], + 'interest:archived': [queryKey], + }); + const all = data?.data ?? []; const active = all.filter((i) => !i.archivedAt && !i.outcome); const preview = active.slice(0, PREVIEW_LIMIT); diff --git a/src/lib/db/schema/berths.ts b/src/lib/db/schema/berths.ts index 1f89a6a..58ba6a8 100644 --- a/src/lib/db/schema/berths.ts +++ b/src/lib/db/schema/berths.ts @@ -92,6 +92,17 @@ export const berths = pgTable( ], ); +// Note: `berths.current_pdf_version_id` has an `ON DELETE SET NULL` FK to +// `berth_pdf_versions.id` installed by migration 0030. The column is left +// without a `.references()` / `foreignKey()` declaration in the Drizzle +// schema because the two tables form a circular FK (berth_pdf_versions → +// berths), and Drizzle's relation inference doesn't tolerate the cycle +// when both sides are declared via column-level `.references()`. The +// migration chain authoritatively maintains the constraint; a fresh +// `db:push` against an empty DB would skip the FK and require a follow-up +// generated migration to add it back. This is acceptable because we +// always apply migrations in order in dev/CI/prod. + export const berthMapData = pgTable( 'berth_map_data', { diff --git a/src/lib/services/berth-pdf-parser.ts b/src/lib/services/berth-pdf-parser.ts index d7d49c8..4d5c746 100644 --- a/src/lib/services/berth-pdf-parser.ts +++ b/src/lib/services/berth-pdf-parser.ts @@ -470,9 +470,13 @@ function coerceFieldValue(key: keyof ExtractedBerthFields, raw: string): string } return raw; } - // Numeric columns: strip currency / unit suffixes and commas. + // Numeric columns: strip currency / unit suffixes and commas. Berth + // dimensions / capacities / prices are all non-negative — reject + // negatives outright so an AcroForm with `length_ft="-50"` doesn't + // poison the recommender feasibility filter when applied. const numeric = Number(raw.replace(/[^0-9.\-]/g, '')); - return Number.isFinite(numeric) ? numeric : null; + if (!Number.isFinite(numeric)) return null; + return numeric < 0 ? null : numeric; } /** Parse a human date like "September 15 2025" → "2025-09-15". */ diff --git a/src/lib/services/berth-recommender.service.ts b/src/lib/services/berth-recommender.service.ts index ebc7c6e..197ad92 100644 --- a/src/lib/services/berth-recommender.service.ts +++ b/src/lib/services/berth-recommender.service.ts @@ -448,7 +448,15 @@ export async function recommendBerths(args: RecommendBerthsArgs): Promise = { 'interest:created': 'interest.created', 'interest:stageChanged': 'interest.stage_changed', 'interest:berthLinked': 'interest.berth_linked', + 'interest:berthUnlinked': 'interest.berth_unlinked', + 'interest:berthLinkUpdated': 'interest.berth_link_updated', 'berth:statusChanged': 'berth.status_changed', 'berth:updated': 'berth.updated', 'document:sent': 'document.sent', diff --git a/src/lib/storage/filesystem.ts b/src/lib/storage/filesystem.ts index f4bf45d..5d22475 100644 --- a/src/lib/storage/filesystem.ts +++ b/src/lib/storage/filesystem.ts @@ -314,9 +314,19 @@ function resolveHmacSecret(encryptedSecret: string | null): string { logger.error({ err }, 'Failed to decrypt storage_proxy_hmac_secret_encrypted'); } } - // Derive a stable per-process secret from BETTER_AUTH_SECRET so dev mode - // works without explicit configuration. In production the admin UI writes - // an encrypted random secret. + // Production refuses to derive: an admin must have explicitly configured + // `storage_proxy_hmac_secret_encrypted` before flipping the storage + // backend to filesystem. Conflating this trust domain with the auth + // cookie HMAC (BETTER_AUTH_SECRET) is acceptable in dev for ergonomics + // but a deployment-time misconfig in prod. + if (process.env.NODE_ENV === 'production') { + throw new Error( + 'FilesystemBackend: storage_proxy_hmac_secret_encrypted must be set in production. ' + + 'Generate a random secret in admin > storage and persist it before flipping the backend.', + ); + } + // Dev fallback: derive a stable per-process secret so the filesystem + // backend works without explicit configuration during local development. const seed = process.env.BETTER_AUTH_SECRET ?? env.BETTER_AUTH_SECRET ?? 'storage-default'; return createHash('sha256').update(`storage-proxy:${seed}`).digest('hex'); }