From 33309e22e1eba3798643b74f767eb85e5d8a3657 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 19:21:07 +0000 Subject: [PATCH 01/10] Add notifications retention cron (Tier item 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The notifications table (in-app bell events, added 2026-07-15) had no retention job and grew forever — every other append-heavy table already has one. New cron-notifications-retention runs daily at 9:30am CT (15 min after dailyGuestPiiRetention, continuing the retention-cron stagger) and enforces: - read rows (read_at IS NOT NULL) older than 90 days: deleted - all rows, read or unread, older than 180 days: deleted Deletes run in bounded batches (select up to 500 ids, delete by id, repeat, max 20 batches per policy per run) — never one unbounded DELETE — so a large backlog degrades to "finish tomorrow" rather than one giant transaction. Both steps are pure deletes and safe for Inngest to replay. Service client created inside step.run() only, with the { system: 'inngest:notifications-retention' } context. Pure cron trigger like the sibling retention crons — no new FieldStayEvents entry needed. Log lines carry row counts only, never notification content. Registered in the single serve() call in the Inngest route. Unit test mirrors the queue-based .from() mock convention of cron-comms-retention.test.ts: policy filters, bounded id-batch deletes, batch-ceiling behavior, cutoff date math, and error-throw-for-retry. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- app/api/inngest/route.ts | 2 + .../functions/cron/notifications-retention.ts | 105 +++++++++ .../cron-notifications-retention.test.ts | 218 ++++++++++++++++++ 3 files changed, 325 insertions(+) create mode 100644 lib/inngest/functions/cron/notifications-retention.ts create mode 100644 unit/inngest/cron-notifications-retention.test.ts diff --git a/app/api/inngest/route.ts b/app/api/inngest/route.ts index 69fa4b8f..2bc2f5f2 100644 --- a/app/api/inngest/route.ts +++ b/app/api/inngest/route.ts @@ -18,6 +18,7 @@ import { dailyAssetHealth } from '@/lib/inngest/functions/cron/asse import { dailyCommsRetention } from '@/lib/inngest/functions/cron/comms-retention' import { dailyGuestPiiRetention } from '@/lib/inngest/functions/cron/guest-pii-retention' import { auditRetentionCron } from '@/lib/inngest/functions/cron/audit-retention' +import { notificationsRetentionCron } from '@/lib/inngest/functions/cron/notifications-retention' import { staleFeedAlert } from '@/lib/inngest/functions/cron/stale-feed-alert' import { turnoverPriorityDecay } from '@/lib/inngest/functions/cron/turnover-priority-decay' import { notificationDigest } from '@/lib/inngest/functions/cron/notification-digest' @@ -180,6 +181,7 @@ export const { GET, POST, PUT } = serve({ dailyCommsRetention, dailyGuestPiiRetention, auditRetentionCron, + notificationsRetentionCron, staleFeedAlert, turnoverPriorityDecay, notificationDigest, diff --git a/lib/inngest/functions/cron/notifications-retention.ts b/lib/inngest/functions/cron/notifications-retention.ts new file mode 100644 index 00000000..9d12c83d --- /dev/null +++ b/lib/inngest/functions/cron/notifications-retention.ts @@ -0,0 +1,105 @@ +import { inngest } from '@/lib/inngest/client' +import { createServiceClient } from '@/lib/supabase/server' +import type { DBClient } from '@/lib/supabase/server' + +/** + * SCHEDULED: runs daily at 9:30am CT — 15 min after dailyGuestPiiRetention, + * continuing the retention-cron stagger to avoid Supabase contention. + * + * Retention policy for the `notifications` table (in-app bell events — + * append-heavy, previously grew forever): + * • read rows (`read_at IS NOT NULL`) older than 90 days are deleted + * • ALL rows (read or unread) older than 180 days are deleted + * + * Deletes run in bounded batches (select up to BATCH_SIZE ids, delete by id, + * repeat) — never one unbounded DELETE — with a per-run batch ceiling so a + * huge backlog degrades to "finish tomorrow" instead of one giant + * transaction. Deletes are naturally idempotent, so every step is safe for + * Inngest to replay. Only row counts are logged — never notification + * title/subtitle content. + */ + +const READ_RETENTION_DAYS = 90 // read rows older than this are purged +const MAX_RETENTION_DAYS = 180 // all rows older than this are purged, read or not +const BATCH_SIZE = 500 +const MAX_BATCHES_PER_RUN = 20 // hard ceiling: 10k rows per policy per run + +interface PurgeResult { + deleted: number + exhausted: boolean // true = no rows left past the cutoff; false = hit the batch ceiling +} + +/** + * Deletes rows past `cutoffIso` in bounded batches. `onlyRead` restricts the + * purge to rows that have been read (`read_at IS NOT NULL`). + */ +async function purgeNotificationsBefore( + supabase: DBClient, + cutoffIso: string, + onlyRead: boolean, +): Promise { + let deleted = 0 + + for (let batch = 0; batch < MAX_BATCHES_PER_RUN; batch++) { + let query = supabase + .from('notifications') + .select('id') + .lt('created_at', cutoffIso) + if (onlyRead) query = query.not('read_at', 'is', null) + + const { data: stale, error: selectError } = await query.limit(BATCH_SIZE) + if (selectError) throw new Error(`notifications retention select failed: ${selectError.message}`) + if (!stale || stale.length === 0) return { deleted, exhausted: true } + + const ids = stale.map((row: { id: string }) => row.id) + const { error: deleteError } = await supabase + .from('notifications') + .delete() + .in('id', ids) + if (deleteError) throw new Error(`notifications retention delete failed: ${deleteError.message}`) + + deleted += ids.length + if (ids.length < BATCH_SIZE) return { deleted, exhausted: true } + } + + return { deleted, exhausted: false } +} + +export const notificationsRetentionCron = inngest.createFunction( + { + id: 'cron-notifications-retention', + name: 'Cron: Notifications Retention Purge', + retries: 1, + }, + { cron: '30 14 * * *' }, // 15 min after dailyGuestPiiRetention + async ({ step, logger }) => { + // Read rows: purged once 90 days old. Idempotent — re-running deletes + // nothing new once the window is clear. + const readPurge = await step.run('purge-read-notifications-90d', async () => { + const supabase = createServiceClient({ system: 'inngest:notifications-retention' }) + const cutoff = new Date(Date.now() - READ_RETENTION_DAYS * 86_400_000).toISOString() + return purgeNotificationsBefore(supabase, cutoff, true) + }) + + // All rows (read or unread): purged once 180 days old. + const maxAgePurge = await step.run('purge-all-notifications-180d', async () => { + const supabase = createServiceClient({ system: 'inngest:notifications-retention' }) + const cutoff = new Date(Date.now() - MAX_RETENTION_DAYS * 86_400_000).toISOString() + return purgeNotificationsBefore(supabase, cutoff, false) + }) + + logger.info( + `Notifications retention — read>90d deleted: ${readPurge.deleted}, ` + + `any>180d deleted: ${maxAgePurge.deleted}` + + (readPurge.exhausted && maxAgePurge.exhausted + ? '' + : ' (batch ceiling hit — remainder purges on the next run)') + ) + + return { + read_deleted: readPurge.deleted, + max_age_deleted: maxAgePurge.deleted, + exhausted: readPurge.exhausted && maxAgePurge.exhausted, + } + } +) diff --git a/unit/inngest/cron-notifications-retention.test.ts b/unit/inngest/cron-notifications-retention.test.ts new file mode 100644 index 00000000..2b0fa6f5 --- /dev/null +++ b/unit/inngest/cron-notifications-retention.test.ts @@ -0,0 +1,218 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +vi.mock('@/lib/supabase/server', () => ({ + createServiceClient: vi.fn(), +})) + +import { notificationsRetentionCron } from '@/lib/inngest/functions/cron/notifications-retention' +import { createServiceClient } from '@/lib/supabase/server' +import { invokeHandler } from './test-helpers' + +// Cron function — no meaningful `data` on the real event (only wall-clock +// date driven), so `event` is `{}`, mirroring cron-comms-retention. +// +// Queue-based `.from(table)` mock, same convention as the other retention +// crons. `notifications` is hit in select/delete pairs (select a bounded +// batch of ids, delete by id) per retention policy, so order matters: +// [read>90d select, read>90d delete, all>180d select, all>180d delete, ...] +// A select that returns [] is NOT followed by a delete (the batch loop +// stops), so queue entries must account for that. +function makeSupabase(queued: Record) { + const counters: Record = {} + const calls: { table: string; method: string; args: unknown[] }[] = [] + + const from = vi.fn((table: string) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const chain: any = {} + const record = (method: string, args: unknown[]) => { + calls.push({ table, method, args }) + return chain + } + chain.select = (...a: unknown[]) => record('select', a) + chain.not = (...a: unknown[]) => record('not', a) + chain.lt = (...a: unknown[]) => record('lt', a) + chain.limit = (...a: unknown[]) => record('limit', a) + chain.delete = (...a: unknown[]) => record('delete', a) + chain.in = (...a: unknown[]) => record('in', a) + + const resolveNext = () => { + const idx = counters[table] ?? 0 + counters[table] = idx + 1 + return Promise.resolve(queued[table]?.[idx] ?? { data: null, error: null }) + } + + chain.then = (resolve: (v: unknown) => unknown, reject?: (e: unknown) => unknown) => + resolveNext().then(resolve, reject) + return chain + }) + + return { from, calls } +} + +function makeStep() { + return { run: vi.fn((_name: string, cb: () => unknown) => cb()) } +} + +function ids(n: number, prefix: string) { + return Array.from({ length: n }, (_, i) => ({ id: `${prefix}_${i}` })) +} + +describe('notificationsRetentionCron', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('purges read rows past 90 days and all rows past 180 days, deleting by bounded id batches', async () => { + const supabase = makeSupabase({ + notifications: [ + { data: [{ id: 'n_1' }, { id: 'n_2' }], error: null }, // read>90d select + { data: null, error: null }, // read>90d delete + { data: [{ id: 'n_3' }], error: null }, // all>180d select + { data: null, error: null }, // all>180d delete + ], + }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + + const logger = { info: vi.fn(), error: vi.fn() } + const result = await invokeHandler(notificationsRetentionCron, { + event: {}, + step: makeStep(), + logger, + }) + + expect(result).toEqual({ read_deleted: 2, max_age_deleted: 1, exhausted: true }) + + // Every delete is bounded by an explicit id list — never an open-ended filter. + const inCalls = supabase.calls.filter((c) => c.method === 'in') + expect(inCalls).toHaveLength(2) + expect(inCalls[0].args).toEqual(['id', ['n_1', 'n_2']]) + expect(inCalls[1].args).toEqual(['id', ['n_3']]) + + // The 90-day pass targets read rows only; the 180-day pass has no read_at filter. + const notCalls = supabase.calls.filter((c) => c.method === 'not') + expect(notCalls).toHaveLength(1) + expect(notCalls[0].args).toEqual(['read_at', 'is', null]) + + // Counts only in the log line — never notification content. + expect(logger.info).toHaveBeenCalledWith( + expect.stringContaining('read>90d deleted: 2') as unknown as string, + ) + }) + + it('is a no-op when nothing has aged past either retention window', async () => { + const supabase = makeSupabase({ + notifications: [ + { data: [], error: null }, // read>90d select — empty, no delete follows + { data: [], error: null }, // all>180d select — empty, no delete follows + ], + }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + + const result = await invokeHandler(notificationsRetentionCron, { + event: {}, + step: makeStep(), + logger: { info: vi.fn(), error: vi.fn() }, + }) + + expect(result).toEqual({ read_deleted: 0, max_age_deleted: 0, exhausted: true }) + expect(supabase.calls.filter((c) => c.method === 'delete')).toHaveLength(0) + }) + + it('keeps deleting in batches until a short batch signals the backlog is exhausted', async () => { + const supabase = makeSupabase({ + notifications: [ + { data: ids(500, 'a'), error: null }, // read>90d batch 1 — full, loop again + { data: null, error: null }, + { data: ids(120, 'b'), error: null }, // read>90d batch 2 — short, stop + { data: null, error: null }, + { data: [], error: null }, // all>180d — nothing + ], + }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + + const result = await invokeHandler(notificationsRetentionCron, { + event: {}, + step: makeStep(), + logger: { info: vi.fn(), error: vi.fn() }, + }) + + expect(result).toEqual({ read_deleted: 620, max_age_deleted: 0, exhausted: true }) + + const limitCalls = supabase.calls.filter((c) => c.method === 'limit') + expect(limitCalls.every((c) => c.args[0] === 500)).toBe(true) + }) + + it('reports exhausted: false when a run hits the per-run batch ceiling', async () => { + // 20 full batches for the read>90d pass (the MAX_BATCHES_PER_RUN ceiling), + // then an empty all>180d pass. + const queue: { data?: unknown; error?: unknown }[] = [] + for (let i = 0; i < 20; i++) { + queue.push({ data: ids(500, `batch${i}`), error: null }) + queue.push({ data: null, error: null }) + } + queue.push({ data: [], error: null }) + + const supabase = makeSupabase({ notifications: queue }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + + const logger = { info: vi.fn(), error: vi.fn() } + const result = await invokeHandler(notificationsRetentionCron, { + event: {}, + step: makeStep(), + logger, + }) + + expect(result).toEqual({ read_deleted: 10_000, max_age_deleted: 0, exhausted: false }) + expect(logger.info).toHaveBeenCalledWith( + expect.stringContaining('batch ceiling hit') as unknown as string, + ) + }) + + it('throws when a query errors, so Inngest retries the step', async () => { + const supabase = makeSupabase({ + notifications: [ + { data: null, error: { message: 'connection reset' } }, + ], + }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + + await expect( + invokeHandler(notificationsRetentionCron, { + event: {}, + step: makeStep(), + logger: { info: vi.fn(), error: vi.fn() }, + }), + ).rejects.toThrow('notifications retention select failed: connection reset') + }) + + describe('retention-window cutoff date math', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-07-25T14:30:00.000Z')) + }) + afterEach(() => { + vi.useRealTimers() + }) + + it('uses a 90-day cutoff for read rows and a 180-day cutoff for all rows', async () => { + const supabase = makeSupabase({ + notifications: [ + { data: [], error: null }, + { data: [], error: null }, + ], + }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + + await invokeHandler(notificationsRetentionCron, { + event: {}, + step: makeStep(), + logger: { info: vi.fn(), error: vi.fn() }, + }) + + const ltCalls = supabase.calls.filter((c) => c.method === 'lt') + expect(ltCalls).toHaveLength(2) + expect(ltCalls[0].args).toEqual(['created_at', '2026-04-26T14:30:00.000Z']) // 90 days + expect(ltCalls[1].args).toEqual(['created_at', '2026-01-26T14:30:00.000Z']) // 180 days + }) + }) +}) From 11309a629243b0ae3dc3812c108159ff729983f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 19:25:12 +0000 Subject: [PATCH 02/10] Crew Sync v2 Phase 2: broadcast wake-up triggers (deploys dark) Adds the database-side broadcast infrastructure for the crew PWA realtime redesign (Option B: broadcast signal + delta pull), per docs/CREW_SYNC_V2_PHASES.md section 2: - notify_crew_sync(uuid[], text): shared realtime.send() helper on topic 'crew:{user_id}', event 'sync', payload {entity} only (signal, never row data). Per-user exception-safe so a broadcast failure can never break the write that triggered it. - Statement-level AFTER triggers with transition tables (one broadcast per statement, not per row): turnover_assignments (INS/UPD/DEL) and turnovers (UPD) -> 'turnovers'; checklist_instances and checklist_instance_items (INS/UPD) -> 'checklists'; work_orders (INS/UPD/DEL) -> 'work_orders'. UPDATE triggers notify old AND new assignees so reassignment signals both sides. - All functions SECURITY DEFINER with pinned empty search_path; EXECUTE revoked from PUBLIC/anon/authenticated on the helper AND on all five trigger functions (the latter added beyond the spec SQL: Supabase security advisors flagged the trigger functions as anon/authenticated-executable via PostgREST RPC, lints 0028/0029 -- confirmed cleared on re-run). - RLS policy on realtime.messages authorizing each authenticated user to receive broadcasts only on their own 'crew:{auth.uid()}' topic (negative subscription test verified: foreign topic -> Unauthorized). Applied to production (vpmznjktllhmmbfnxuvk) and e2e (syhthijeqlnltufdawyb). Touch tests verified correct realtime.messages rows for all three entities; scratch client received its own-topic broadcast and was rejected from a foreign topic. No client subscribes to these topics until Phase 3, so this ships with zero user-facing behavior change. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- ...725191358_crew_sync_broadcast_triggers.sql | 269 ++++++++++++++++++ 1 file changed, 269 insertions(+) create mode 100644 supabase/migrations/20260725191358_crew_sync_broadcast_triggers.sql diff --git a/supabase/migrations/20260725191358_crew_sync_broadcast_triggers.sql b/supabase/migrations/20260725191358_crew_sync_broadcast_triggers.sql new file mode 100644 index 00000000..44023054 --- /dev/null +++ b/supabase/migrations/20260725191358_crew_sync_broadcast_triggers.sql @@ -0,0 +1,269 @@ +-- Crew Sync v2 Phase 2: broadcast wake-up signals for the crew PWA. +-- Statement-level triggers call realtime.send() on topic 'crew:{user_id}' +-- with a minimal {entity} payload. Signal-only: no row data, no PII. +-- Deploys dark — no client subscribes until the Phase 3 cutover. + +-- ── Shared send helper ────────────────────────────────────────────────── +CREATE OR REPLACE FUNCTION public.notify_crew_sync(p_user_ids uuid[], p_entity text) +RETURNS void +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_id uuid; +BEGIN + IF p_user_ids IS NULL THEN + RETURN; + END IF; + FOR v_user_id IN SELECT DISTINCT u FROM unnest(p_user_ids) AS u WHERE u IS NOT NULL + LOOP + BEGIN + PERFORM realtime.send( + jsonb_build_object('entity', p_entity), -- payload: signal only, never row data + 'sync', -- event + 'crew:' || v_user_id::text, -- topic + true -- private channel + ); + EXCEPTION WHEN OTHERS THEN + -- A broadcast failure must never break the write that triggered it. + RAISE WARNING 'notify_crew_sync: send failed for user % (%): %', + v_user_id, p_entity, SQLERRM; + END; + END LOOP; +END; +$$; + +-- Not callable by clients — trigger-context only. An authenticated user +-- must not be able to spam arbitrary crew topics through this definer fn. +REVOKE EXECUTE ON FUNCTION public.notify_crew_sync(uuid[], text) FROM PUBLIC, anon, authenticated; + +-- ── turnover_assignments → 'turnovers' ───────────────────────────────── +CREATE OR REPLACE FUNCTION public.crew_sync_on_turnover_assignments() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + IF TG_OP = 'INSERT' THEN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.crew_members cm ON cm.id = r.crew_member_id + WHERE cm.user_id IS NOT NULL; + ELSIF TG_OP = 'DELETE' THEN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM old_rows r + JOIN public.crew_members cm ON cm.id = r.crew_member_id + WHERE cm.user_id IS NOT NULL; + ELSE -- UPDATE: notify both the previous and the new crew member + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM ( + SELECT crew_member_id FROM new_rows + UNION + SELECT crew_member_id FROM old_rows + ) r + JOIN public.crew_members cm ON cm.id = r.crew_member_id + WHERE cm.user_id IS NOT NULL; + END IF; + + PERFORM public.notify_crew_sync(v_user_ids, 'turnovers'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_turnover_assignments_ins ON public.turnover_assignments; +CREATE TRIGGER crew_sync_turnover_assignments_ins + AFTER INSERT ON public.turnover_assignments + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_turnover_assignments(); + +DROP TRIGGER IF EXISTS crew_sync_turnover_assignments_upd ON public.turnover_assignments; +CREATE TRIGGER crew_sync_turnover_assignments_upd + AFTER UPDATE ON public.turnover_assignments + REFERENCING OLD TABLE AS old_rows NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_turnover_assignments(); + +DROP TRIGGER IF EXISTS crew_sync_turnover_assignments_del ON public.turnover_assignments; +CREATE TRIGGER crew_sync_turnover_assignments_del + AFTER DELETE ON public.turnover_assignments + REFERENCING OLD TABLE AS old_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_turnover_assignments(); + +-- ── turnovers (UPDATE only) → 'turnovers' ────────────────────────────── +-- INSERT is pointless (a brand-new turnover has no assignments yet — the +-- assignment INSERT is the signal). DELETE is covered by the FK cascade +-- firing crew_sync_turnover_assignments_del. +CREATE OR REPLACE FUNCTION public.crew_sync_on_turnovers() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.turnover_assignments ta ON ta.turnover_id = r.id + JOIN public.crew_members cm ON cm.id = ta.crew_member_id + WHERE cm.user_id IS NOT NULL; + + PERFORM public.notify_crew_sync(v_user_ids, 'turnovers'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_turnovers_upd ON public.turnovers; +CREATE TRIGGER crew_sync_turnovers_upd + AFTER UPDATE ON public.turnovers + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_turnovers(); + +-- ── checklist_instances (INSERT, UPDATE) → 'checklists' ──────────────── +CREATE OR REPLACE FUNCTION public.crew_sync_on_checklist_instances() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.turnover_assignments ta ON ta.turnover_id = r.turnover_id + JOIN public.crew_members cm ON cm.id = ta.crew_member_id + WHERE cm.user_id IS NOT NULL; + + PERFORM public.notify_crew_sync(v_user_ids, 'checklists'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_checklist_instances_ins ON public.checklist_instances; +CREATE TRIGGER crew_sync_checklist_instances_ins + AFTER INSERT ON public.checklist_instances + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_checklist_instances(); + +DROP TRIGGER IF EXISTS crew_sync_checklist_instances_upd ON public.checklist_instances; +CREATE TRIGGER crew_sync_checklist_instances_upd + AFTER UPDATE ON public.checklist_instances + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_checklist_instances(); + +-- ── checklist_instance_items (INSERT, UPDATE) → 'checklists' ─────────── +CREATE OR REPLACE FUNCTION public.crew_sync_on_checklist_items() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.checklist_instances ci ON ci.id = r.instance_id + JOIN public.turnover_assignments ta ON ta.turnover_id = ci.turnover_id + JOIN public.crew_members cm ON cm.id = ta.crew_member_id + WHERE cm.user_id IS NOT NULL; + + PERFORM public.notify_crew_sync(v_user_ids, 'checklists'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_checklist_items_ins ON public.checklist_instance_items; +CREATE TRIGGER crew_sync_checklist_items_ins + AFTER INSERT ON public.checklist_instance_items + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_checklist_items(); + +DROP TRIGGER IF EXISTS crew_sync_checklist_items_upd ON public.checklist_instance_items; +CREATE TRIGGER crew_sync_checklist_items_upd + AFTER UPDATE ON public.checklist_instance_items + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_checklist_items(); + +-- ── work_orders (INSERT, UPDATE, DELETE) → 'work_orders' ─────────────── +CREATE OR REPLACE FUNCTION public.crew_sync_on_work_orders() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_user_ids uuid[]; +BEGIN + IF TG_OP = 'INSERT' THEN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM new_rows r + JOIN public.crew_members cm ON cm.id = r.assigned_crew_member_id + WHERE r.assigned_crew_member_id IS NOT NULL AND cm.user_id IS NOT NULL; + ELSIF TG_OP = 'DELETE' THEN + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM old_rows r + JOIN public.crew_members cm ON cm.id = r.assigned_crew_member_id + WHERE r.assigned_crew_member_id IS NOT NULL AND cm.user_id IS NOT NULL; + ELSE -- UPDATE: notify previous and new assignee (covers reassignment) + SELECT array_agg(DISTINCT cm.user_id) INTO v_user_ids + FROM ( + SELECT assigned_crew_member_id FROM new_rows + UNION + SELECT assigned_crew_member_id FROM old_rows + ) r + JOIN public.crew_members cm ON cm.id = r.assigned_crew_member_id + WHERE r.assigned_crew_member_id IS NOT NULL AND cm.user_id IS NOT NULL; + END IF; + + PERFORM public.notify_crew_sync(v_user_ids, 'work_orders'); + RETURN NULL; +END; +$$; + +DROP TRIGGER IF EXISTS crew_sync_work_orders_ins ON public.work_orders; +CREATE TRIGGER crew_sync_work_orders_ins + AFTER INSERT ON public.work_orders + REFERENCING NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_work_orders(); + +DROP TRIGGER IF EXISTS crew_sync_work_orders_upd ON public.work_orders; +CREATE TRIGGER crew_sync_work_orders_upd + AFTER UPDATE ON public.work_orders + REFERENCING OLD TABLE AS old_rows NEW TABLE AS new_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_work_orders(); + +DROP TRIGGER IF EXISTS crew_sync_work_orders_del ON public.work_orders; +CREATE TRIGGER crew_sync_work_orders_del + AFTER DELETE ON public.work_orders + REFERENCING OLD TABLE AS old_rows + FOR EACH STATEMENT EXECUTE FUNCTION public.crew_sync_on_work_orders(); + +-- ── Lock down the trigger functions from PostgREST RPC ───────────────── +-- (Added after the initial spec SQL: Supabase security advisors flagged the +-- five SECURITY DEFINER trigger functions as executable by anon/authenticated +-- via /rest/v1/rpc/* — lints 0028/0029. Trigger functions are invoked by the +-- trigger machinery (EXECUTE is checked at trigger creation, not at fire +-- time), so client roles never need EXECUTE on them.) +REVOKE EXECUTE ON FUNCTION public.crew_sync_on_turnover_assignments() FROM PUBLIC, anon, authenticated; +REVOKE EXECUTE ON FUNCTION public.crew_sync_on_turnovers() FROM PUBLIC, anon, authenticated; +REVOKE EXECUTE ON FUNCTION public.crew_sync_on_checklist_instances() FROM PUBLIC, anon, authenticated; +REVOKE EXECUTE ON FUNCTION public.crew_sync_on_checklist_items() FROM PUBLIC, anon, authenticated; +REVOKE EXECUTE ON FUNCTION public.crew_sync_on_work_orders() FROM PUBLIC, anon, authenticated; + +-- ── Authorize crew clients to receive their own private-topic broadcasts ─ +-- Private Realtime channels authorize against RLS on realtime.messages. +-- A crew user may join exactly one topic: crew:{their own auth.uid()}. +DROP POLICY IF EXISTS "crew_receive_own_sync_broadcasts" ON realtime.messages; +CREATE POLICY "crew_receive_own_sync_broadcasts" + ON realtime.messages + FOR SELECT + TO authenticated + USING ( + realtime.messages.extension = 'broadcast' + AND realtime.topic() = 'crew:' || (SELECT auth.uid())::text + ); From 8421b2b03a28be3e6abb78a7f3c58d9fc925a5ee Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 21:52:10 +0000 Subject: [PATCH 03/10] Add exponential retry backoff to the crew outbox drain (Crew Sync v2 Phase 4) Failed outbox mutations previously re-pushed on every drain trigger (online event, every enqueueMutation), hammering the server during an outage. Now a push failure sets nextAttemptAt on the mutation row: Date.now() + min(2^(retryCount-1) * 5s, 5min) scaled by a uniform 0.5-1.5x jitter factor, so delays grow 5s -> 10s -> 20s ... capped at 5 minutes and a fleet of crew devices recovering from the same outage doesn't retry in lockstep. - lib/dexie/schema.ts: Dexie version(8); MutationRow gains nextAttemptAt?: number (epoch ms). Non-indexed - the drain scans in insertion order - so index strings are unchanged; the full store map is repeated as a complete snapshot per the file's v1-v3 pattern. - lib/dexie/syncService.ts processOutbox(): a head mutation still inside its backoff window stops the drain entirely (never skip-and-continue - later mutations against the same record must not jump ahead; "not due yet" joins the existing stop-on-first-error semantics) and schedules a single one-shot resume timer on the SyncEngine instance (previous timer cleared first). Success deletes the row, clearing nextAttemptAt with it. The MAX_RETRIES=5 dead-letter (failed: true) classification is preserved exactly. - unit/dexie/sync-outbox-backoff.test.ts (new, with additive fake-dexie.ts extensions: add/update/delete/orderBy on fakeTable, mutations table, update in the supabase chain): backoff growth, cap, and jitter bounds; drain stops at a not-yet-due head touching nothing behind it and resumes via the timer; due mutation retries and clears nextAttemptAt on success; permanent-failure path unchanged and excluded from later drains. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- lib/dexie/schema.ts | 27 ++++ lib/dexie/syncService.ts | 62 ++++++++- unit/dexie/fake-dexie.ts | 23 +++- unit/dexie/sync-outbox-backoff.test.ts | 172 +++++++++++++++++++++++++ 4 files changed, 280 insertions(+), 4 deletions(-) create mode 100644 unit/dexie/sync-outbox-backoff.test.ts diff --git a/lib/dexie/schema.ts b/lib/dexie/schema.ts index 1c0fbd83..49652be2 100644 --- a/lib/dexie/schema.ts +++ b/lib/dexie/schema.ts @@ -186,6 +186,11 @@ export interface MutationRow { // queue) lets the UI surface "this didn't sync" instead of silently // discarding it. failed?: boolean + // Retry backoff: epoch ms before which processOutbox() must not re-push + // this mutation. Set on push failure (exponential backoff with jitter), + // cleared by the row's deletion on successful push. Not indexed — the + // drain scans in insertion order and checks this in memory. + nextAttemptAt?: number } export class FieldStayDexie extends Dexie { @@ -294,6 +299,28 @@ export class FieldStayDexie extends Dexie { this.version(7).stores({ messages: 'id, org_id, turnover_id, recipient_id, sender_id, created_at', }) + + // Outbox retry backoff (Crew Sync v2 Phase 4): MutationRow gains + // nextAttemptAt (epoch ms). Non-indexed — processOutbox() drains in + // insertion order and checks due-ness in memory — so the index strings + // are unchanged; the full store map is repeated here to keep this block + // a complete snapshot of the live schema. + this.version(8).stores({ + turnovers: 'id, property_id, org_id, status', + checklist_instances: 'id, turnover_id, org_id, status', + checklist_instance_items: 'id, instance_id, turnover_id, is_completed', + inventory_items: 'id, property_id, org_id', + properties: 'id, org_id', + crew_availability: 'id, org_id, crew_member_id, available_date', + messages: 'id, org_id, turnover_id, recipient_id, sender_id, created_at', + pending_photo_uploads: 'id, target_id, target_table, retry_count', + // ++id = auto-incrementing outbox key; table/targetId are indexed so + // processOutbox() can replay mutations in insertion order per record. + mutations: '++id, table, targetId', + sync_meta: 'key', + crew_work_orders: 'id, property_id, org_id, status, scheduled_date', + property_assets: 'id, property_id, org_id, asset_type', + }) } } diff --git a/lib/dexie/syncService.ts b/lib/dexie/syncService.ts index 9b9b621e..7242931d 100644 --- a/lib/dexie/syncService.ts +++ b/lib/dexie/syncService.ts @@ -3,17 +3,53 @@ import { getDexieDb, type MutationRow } from './schema' type DexieSupabaseClient = ReturnType +const MAX_RETRIES = 5 + +// Retry backoff: 5 s base doubling per retry, capped at 5 min, each delay +// scaled by a uniform 0.5–1.5× jitter factor so a fleet of crew devices +// coming back from the same outage doesn't retry in lockstep. +const BASE_RETRY_DELAY_MS = 5_000 +const MAX_RETRY_DELAY_MS = 300_000 + +/** + * Computes the epoch-ms timestamp before which a failed mutation must not be + * re-pushed. `retryCount` is the ALREADY-incremented count for the failure + * being handled — the `- 1` keeps the first retry at the 5 s base (growth: + * 5 s → 10 s → 20 s … capped at 5 min, each scaled 0.5–1.5×). + */ +export function computeNextAttemptAt(retryCount: number, now: number): number { + const baseDelay = Math.min(2 ** (retryCount - 1) * BASE_RETRY_DELAY_MS, MAX_RETRY_DELAY_MS) + // eslint-disable-next-line no-restricted-properties -- retry backoff jitter to spread outbox retry storms after an outage, not id/token generation + const jitter = Math.random() + return now + baseDelay * (0.5 + jitter) +} + // Mirrors the upstream sync logic in lib/powersync/client.ts (SupabaseConnector.uploadData), // replacing PowerSync's CRUD transaction queue with the local `mutations` outbox table. export class SyncEngine { private supabase = createClient() private userId: string private isProcessing = false + // Single pending wake-up for a drain that stopped on a not-yet-due + // mutation. One handle only — scheduleRetry() clears any previous timer + // before setting a new one; the existing `online` listener and + // enqueueMutation()'s fire-and-forget drains remain additional entry + // points, so an overwritten (later) wake-up self-corrects on the next + // drain, which stops on the still-not-due head and reschedules. + private retryTimer: ReturnType | null = null constructor(userId: string) { this.userId = userId } + private scheduleRetry(nextAttemptAt: number): void { + if (this.retryTimer !== null) clearTimeout(this.retryTimer) + this.retryTimer = setTimeout(() => { + this.retryTimer = null + void this.processOutbox() + }, Math.max(0, nextAttemptAt - Date.now())) + } + /** * Drains the local `mutations` outbox in chronological (insertion) order. * Each mutation is removed from the outbox only after it is successfully @@ -22,6 +58,14 @@ export class SyncEngine { * marked `failed` (dead-lettered on a prior run) are excluded rather than * retried forever — retryFailedMutation() in helpers.ts is the only way * back into the queue for those. + * + * Failed (non-dead-lettered) mutations back off exponentially via + * nextAttemptAt: a mutation whose backoff window hasn't elapsed stops the + * drain entirely — never skip-and-continue, because later mutations against + * the same record must not jump ahead (the same ordering rule as the + * stop-on-first-error semantics below; "not due yet" is just an additional + * stop reason). A stopped drain schedules a one-shot timer to resume at + * nextAttemptAt. */ async processOutbox(): Promise { if (this.isProcessing) return @@ -34,8 +78,18 @@ export class SyncEngine { for (const mutation of pending) { // Auto-incrementing key — always populated once read back from the table. const id = mutation.id as number + + // Backoff gate: a mutation still inside its retry window stops the + // drain entirely (never skip-and-continue — later mutations against + // the same record must not jump ahead). Resume when it comes due. + if (mutation.nextAttemptAt !== undefined && mutation.nextAttemptAt > Date.now()) { + this.scheduleRetry(mutation.nextAttemptAt) + return + } + try { await uploadOne(this.supabase, mutation) + // Successful push clears the whole row — nextAttemptAt with it. await db.mutations.delete(id) } catch (err) { const newRetryCount = mutation.retryCount + 1 @@ -44,7 +98,6 @@ export class SyncEngine { `(attempt ${newRetryCount}):`, err ) - const MAX_RETRIES = 5 if (newRetryCount >= MAX_RETRIES) { // Dead-letter: keep the row (marked failed) rather than deleting // it, so a write that never reached the server leaves a durable, @@ -57,13 +110,16 @@ export class SyncEngine { ) await db.mutations.update(id, { retryCount: newRetryCount, failed: true }) } else { - await db.mutations.update(id, { retryCount: newRetryCount }) + const nextAttemptAt = computeNextAttemptAt(newRetryCount, Date.now()) + await db.mutations.update(id, { retryCount: newRetryCount, nextAttemptAt }) // Only block the queue on transient failures, not permanent ones. // If we've retried >= 3 times, skip this mutation and continue // draining so later mutations (which may be independent) still go through. if (newRetryCount >= 3) continue // Stop draining on first/second failure so later mutations against - // the same record aren't applied out of order. + // the same record aren't applied out of order; wake up again when + // the backoff window elapses. + this.scheduleRetry(nextAttemptAt) break } } diff --git a/unit/dexie/fake-dexie.ts b/unit/dexie/fake-dexie.ts index 08f841f2..ed6da646 100644 --- a/unit/dexie/fake-dexie.ts +++ b/unit/dexie/fake-dexie.ts @@ -7,13 +7,33 @@ interface FakeRow { [key: string]: unknown } export function fakeTable(pk = 'key') { const rows = new Map() + // Auto-increment counter for add() on '++id'-style outbox tables. + let nextAutoId = 0 return { rows, async get(id: unknown) { return rows.get(id) }, async put(row: FakeRow) { rows.set(row[pk], row) }, + async add(row: FakeRow) { + const id = row[pk] ?? ++nextAutoId + rows.set(id, { ...row, [pk]: id }) + return id + }, + async update(id: unknown, changes: FakeRow) { + const existing = rows.get(id) + if (!existing) return 0 + rows.set(id, { ...existing, ...changes }) + return 1 + }, + async delete(id: unknown) { rows.delete(id) }, async bulkPut(list: FakeRow[]) { for (const r of list) rows.set(r[pk], r) }, async bulkDelete(ids: unknown[]) { for (const id of ids) rows.delete(id) }, async toArray() { return [...rows.values()] }, + orderBy(field: string) { + return { + toArray: async () => + [...rows.values()].sort((a, b) => ((a[field] as number) < (b[field] as number) ? -1 : 1)), + } + }, where(field: string) { return { anyOf: (values: unknown[]) => { @@ -38,6 +58,7 @@ export function makeFakeDexieDb() { inventory_items: fakeTable('id'), crew_work_orders: fakeTable('id'), sync_meta: fakeTable('key'), + mutations: fakeTable('id'), } } @@ -58,7 +79,7 @@ export function makeFakeSupabase(queued: Record record(m, a) } const resolveNext = () => { diff --git a/unit/dexie/sync-outbox-backoff.test.ts b/unit/dexie/sync-outbox-backoff.test.ts new file mode 100644 index 00000000..0c1e62b5 --- /dev/null +++ b/unit/dexie/sync-outbox-backoff.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { makeFakeDexieDb, makeFakeSupabase, type FakeDexieDb } from './fake-dexie' +import type { MutationRow } from '@/lib/dexie/schema' + +const holder = vi.hoisted(() => ({ + db: null as unknown, + supabase: null as unknown, +})) + +vi.mock('@/lib/dexie/schema', () => ({ + getDexieDb: () => holder.db, +})) + +// SyncEngine captures createClient() at construction — delegate through the +// holder so each test can swap in its own queued fake supabase. +vi.mock('@/lib/supabase/client', () => ({ + createClient: () => ({ + from: (table: string) => (holder.supabase as ReturnType).from(table), + }), +})) + +import { SyncEngine, computeNextAttemptAt } from '@/lib/dexie/syncService' + +const NOW = Date.parse('2026-07-25T12:00:00.000Z') + +function db(): FakeDexieDb { return holder.db as FakeDexieDb } +function supabaseCalls() { return (holder.supabase as ReturnType).calls } + +// inventory_items:PATCH is the simplest pure-supabase upload handler +// (update → eq → select) — no fetch()-routed side effects to stub. +async function seedMutation(overrides: Partial = {}): Promise { + const id = await db().mutations.add({ + table: 'inventory_items', + targetId: 'item1', + op: 'PATCH', + payload: { current_quantity: 3 }, + createdAt: new Date(NOW).toISOString(), + retryCount: 0, + ...overrides, + }) + return id as number +} + +async function mutationRow(id: number): Promise { + return (await db().mutations.get(id)) as MutationRow | undefined +} + +const UPLOAD_OK = { data: [{ id: 'item1' }], error: null } +const UPLOAD_FAIL = { error: { message: 'network down' } } + +describe('computeNextAttemptAt — backoff delay math', () => { + afterEach(() => vi.restoreAllMocks()) + + it('grows 5s → 10s → 20s → 40s (jitter factor pinned to 1.0)', () => { + vi.spyOn(Math, 'random').mockReturnValue(0.5) // factor = 0.5 + 0.5 = 1.0 + expect(computeNextAttemptAt(1, NOW)).toBe(NOW + 5_000) + expect(computeNextAttemptAt(2, NOW)).toBe(NOW + 10_000) + expect(computeNextAttemptAt(3, NOW)).toBe(NOW + 20_000) + expect(computeNextAttemptAt(4, NOW)).toBe(NOW + 40_000) + }) + + it('caps the base delay at 5 minutes', () => { + vi.spyOn(Math, 'random').mockReturnValue(0.5) + // 2^6 * 5s = 320s > 300s cap + expect(computeNextAttemptAt(7, NOW)).toBe(NOW + 300_000) + expect(computeNextAttemptAt(20, NOW)).toBe(NOW + 300_000) + }) + + it('jitter scales each delay by 0.5–1.5x (bounds of the uniform factor)', () => { + const random = vi.spyOn(Math, 'random') + random.mockReturnValue(0) // factor 0.5 + expect(computeNextAttemptAt(1, NOW)).toBe(NOW + 2_500) + random.mockReturnValue(0.999_999) // factor → just under 1.5 + expect(computeNextAttemptAt(1, NOW)).toBeLessThan(NOW + 7_500) + expect(computeNextAttemptAt(1, NOW)).toBeGreaterThanOrEqual(NOW + 2_500) + }) + + it('stays inside [0.5x, 1.5x) of the base delay with real randomness', () => { + for (let i = 0; i < 100; i++) { + const delay = computeNextAttemptAt(2, 0) // base 10s + expect(delay).toBeGreaterThanOrEqual(5_000) + expect(delay).toBeLessThan(15_000) + } + }) +}) + +describe('processOutbox — retry backoff', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(NOW) + holder.db = makeFakeDexieDb() + holder.supabase = makeFakeSupabase({}) + // Failure paths log deliberately — keep test output clean. + vi.spyOn(console, 'error').mockImplementation(() => {}) + }) + + afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() + }) + + it('stops the drain at a not-yet-due head mutation and touches nothing behind it', async () => { + const headId = await seedMutation({ retryCount: 1, nextAttemptAt: NOW + 60_000 }) + const laterId = await seedMutation({ targetId: 'item2' }) + holder.supabase = makeFakeSupabase({ inventory_items: [UPLOAD_OK, UPLOAD_OK] }) + + const engine = new SyncEngine('u1') + await engine.processOutbox() + + // Nothing pushed — not even the due mutation behind the head. + expect(supabaseCalls()).toHaveLength(0) + expect(await mutationRow(headId)).toMatchObject({ retryCount: 1, nextAttemptAt: NOW + 60_000 }) + expect(await mutationRow(laterId)).toMatchObject({ retryCount: 0 }) + + // One resume timer scheduled; a second stopped drain replaces it (single handle). + expect(vi.getTimerCount()).toBe(1) + await engine.processOutbox() + expect(vi.getTimerCount()).toBe(1) + + // When the head comes due, the timer re-runs the drain and both flush in order. + await vi.advanceTimersByTimeAsync(60_000) + expect(await mutationRow(headId)).toBeUndefined() + expect(await mutationRow(laterId)).toBeUndefined() + expect(supabaseCalls().filter((c) => c.method === 'update')).toHaveLength(2) + }) + + it('retries a due mutation and clears nextAttemptAt on success (row removed)', async () => { + const id = await seedMutation({ retryCount: 2, nextAttemptAt: NOW - 1_000 }) + holder.supabase = makeFakeSupabase({ inventory_items: [UPLOAD_OK] }) + + await new SyncEngine('u1').processOutbox() + + expect(await mutationRow(id)).toBeUndefined() + expect(supabaseCalls().filter((c) => c.method === 'update')).toHaveLength(1) + expect(vi.getTimerCount()).toBe(0) + }) + + it('sets nextAttemptAt with backoff on push failure and resumes via the timer', async () => { + vi.spyOn(Math, 'random').mockReturnValue(0.5) // jitter factor 1.0 + const id = await seedMutation() + holder.supabase = makeFakeSupabase({ inventory_items: [UPLOAD_FAIL, UPLOAD_OK] }) + + await new SyncEngine('u1').processOutbox() + + // First failure: retryCount 1, due again in 5s (base delay, factor 1.0). + expect(await mutationRow(id)).toMatchObject({ retryCount: 1, nextAttemptAt: NOW + 5_000 }) + expect(vi.getTimerCount()).toBe(1) + + // The scheduled retry fires once due, succeeds, and removes the row. + await vi.advanceTimersByTimeAsync(5_000) + expect(await mutationRow(id)).toBeUndefined() + }) + + it('keeps the permanent-failure (dead-letter) path unchanged', async () => { + const id = await seedMutation({ retryCount: 4 }) + holder.supabase = makeFakeSupabase({ inventory_items: [UPLOAD_FAIL, UPLOAD_OK] }) + + const engine = new SyncEngine('u1') + await engine.processOutbox() + + // Fifth failure dead-letters: row kept, marked failed, no backoff window. + const row = await mutationRow(id) + expect(row).toMatchObject({ retryCount: 5, failed: true }) + expect(row?.nextAttemptAt).toBeUndefined() + expect(vi.getTimerCount()).toBe(0) + + // Dead-lettered rows are excluded from later drains, not retried forever. + await engine.processOutbox() + expect(supabaseCalls().filter((c) => c.method === 'update')).toHaveLength(1) + expect(await mutationRow(id)).toMatchObject({ retryCount: 5, failed: true }) + }) +}) From a508248bc64af8a0405126994caecef395145e23 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 22:08:18 +0000 Subject: [PATCH 04/10] Crew Sync v2 Phase 3: client cutover behind NEXT_PUBLIC_CREW_SYNC_V2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the crew PWA's broadcast-signal client path per docs/CREW_SYNC_V2_PHASES.md section 3, gated on NEXT_PUBLIC_CREW_SYNC_V2 so it ships dormant — the flag defaults off and the existing three postgres_changes channels + generation-token machinery in lib/dexie/context.tsx are untouched (pure additions: one early-return in run() when the flag is on, new teardown lines appended after the existing ones, no modified lines in the v1 path). Flag on: one private broadcast channel (crew:{userId}), subscribed after supabase.realtime.setAuth() and re-authed on TOKEN_REFRESHED; signals route through a per-entity 1s trailing debounce with in-flight serialization (exactly one queued follow-up, never stacked); each entity resolves to a full-scope pull so cursor advancement stays safe per the doc's cursor invariants. Safety poll every 5 minutes (also the only freshness path for property_assets, which has no trigger by design) plus resyncs on mount/online/visibilitychange/(re)subscribe. Reconnect on CHANNEL_ERROR/TIMED_OUT/CLOSED after 5s base + uniform [0,30s] jitter to avoid a rejoin thundering herd. The per-entity debouncer, entity->action mapping, and reconnect-jitter math are extracted into lib/dexie/sync/signals.ts as pure/injectable functions, unit-tested with fake timers in unit/dexie/sync-signals.test.ts and unit/dexie/sync-reconnect-jitter.test.ts (burst coalescing, in-flight serialization with a single queued follow-up, unknown entities ignored, each entity invoking its own sync fn, jitter always within [5s, 35s]). Documents NEXT_PUBLIC_CREW_SYNC_V2 in .env.example. Deviation: the doc's Phase 1 field-soak precondition is waived per the project owner — pre-launch, no field crew traffic exists yet to soak against. Manual smoke (doc 3d): the disposable e2e crew user/turnover were seeded and torn down on the syhthijeqlnltufdawyb project, and the app was built+started against that project with the flag on. The browser-driven half of the smoke (Playwright/Chromium hitting Supabase auth through the sandbox's required HTTPS proxy) could not be completed — confirmed via a minimal repro that an unrelated bare page.goto() to Supabase's own endpoint fails the same way through Chromium's network stack over this proxy, while curl through the identical proxy succeeds, ruling out a TLS/cert issue and pointing at a sandbox/Chromium proxy-tunneling limitation rather than anything in this change. Phase 2's already-completed pure-Node scratch-client test independently proved the wire-level mechanism this phase's UI consumes (private channel subscribe via realtime.setAuth(), broadcast delivery, cross-user-topic rejection). Full verification pass (tsc, lint, vitest, check:ui-classes) is green. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- .env.example | 7 + lib/dexie/context.tsx | 180 +++++++++++++++++++ lib/dexie/sync/signals.ts | 136 ++++++++++++++ unit/dexie/sync-reconnect-jitter.test.ts | 50 ++++++ unit/dexie/sync-signals.test.ts | 218 +++++++++++++++++++++++ 5 files changed, 591 insertions(+) create mode 100644 lib/dexie/sync/signals.ts create mode 100644 unit/dexie/sync-reconnect-jitter.test.ts create mode 100644 unit/dexie/sync-signals.test.ts diff --git a/.env.example b/.env.example index 2dda7e69..4754759d 100644 --- a/.env.example +++ b/.env.example @@ -78,6 +78,13 @@ NEXT_PUBLIC_APP_URL=https://app.fieldstay.app # Dev override (set to http://localhost:3000 in .env.local) # NEXT_PUBLIC_APP_URL=http://localhost:3000 +# Crew Sync v2 (docs/CREW_SYNC_V2_PHASES.md Phase 3): when 'true', the crew +# PWA replaces its postgres_changes subscriptions with one private Realtime +# Broadcast channel (crew:{user_id}) + debounced delta pulls + a 5-minute +# safety poll. Ships dormant — leave unset/false until the Phase 5 rollout. +# Build-time flag (NEXT_PUBLIC_ = inlined at build), not a runtime toggle. +# NEXT_PUBLIC_CREW_SYNC_V2=true + # ------------------------------------------------------------ # KROGER (Grocery Cart Integration) # From: https://developer.kroger.com/manage/ diff --git a/lib/dexie/context.tsx b/lib/dexie/context.tsx index 2ebb8e84..4be1ce82 100644 --- a/lib/dexie/context.tsx +++ b/lib/dexie/context.tsx @@ -16,6 +16,23 @@ import { syncWorkOrders } from './sync/work-orders' import { syncMessages } from './sync/messages' import { syncCrewAvailability } from './sync/availability' import { computeAssignedPropertyIds, syncPropertyAssets } from './sync/assets' +import { + createSyncSignalHandler, + reconnectDelayWithJitterMs, + type SyncSignalHandler, +} from './sync/signals' + +// Crew Sync v2 (docs/CREW_SYNC_V2_PHASES.md Phase 3): broadcast signal + +// delta pull instead of postgres_changes. Ships dormant — the flag defaults +// off and the v1 path below stays the production behavior until Phase 5. +// NEXT_PUBLIC_ vars are inlined at build time, so this is a build-time +// constant, not a runtime toggle. +const CREW_SYNC_V2 = process.env.NEXT_PUBLIC_CREW_SYNC_V2 === 'true' + +/** How often the v2 safety poll runs a full resync — the correctness + * backstop for missed broadcasts and the only freshness path for + * property_assets, which deliberately has no broadcast trigger. */ +const SAFETY_POLL_INTERVAL_MS = 5 * 60_000 interface DexieContextValue { db: FieldStayDexie | null @@ -226,6 +243,14 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin let checklistRefreshGeneration = 0 let assetsRefreshGeneration = 0 + // ── Crew Sync v2 state (all stays null/untouched when the flag is off) ─ + let v2Channel: ReturnType | null = null + let v2ReconnectTimer: ReturnType | null = null + let v2SafetyPollTimer: ReturnType | null = null + let v2VisibilityHandler: (() => void) | null = null + let v2AuthSubscription: { unsubscribe: () => void } | null = null + let v2SignalHandler: SyncSignalHandler | null = null + // Realtime's postgres_changes subscriptions never replay events fired // while the socket was disconnected — a crew member offline for a // stretch (a reassignment, a co-crew-member's checklist completion) @@ -254,6 +279,145 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin await refreshAssetsSubscription() } + // ── Crew Sync v2 (flag on): broadcast signal + delta pull ────────────── + // No postgres_changes channels at all. One private broadcast channel + // (`crew:{userId}`) delivers tiny `{ entity }` wake-up signals; the + // client answers each with a debounced full-scope delta pull. A 5-minute + // safety poll plus resyncs on mount/online/visible/(re)subscribe are the + // correctness backstops — a missed broadcast only ever costs latency. + + // Full v2 resync: all entities + reconciliation. Unlike v1's resync() + // this never touches channel subscriptions — the single broadcast + // channel's scope is the user id, which never changes mid-session — and + // it also covers property_assets (no broadcast trigger; see the doc's + // section 1) directly instead of via refreshAssetsSubscription. + async function resyncV2(crewMemberId: string): Promise { + await Promise.all([ + syncAssignedTurnovers(supabase, userId!, crewMemberId), + syncWorkOrders(supabase, userId!, crewMemberId), + syncMessages(supabase, userId!), + syncCrewAvailability(supabase, userId!, crewMemberId), + ]) + if (cancelled) return + const propertyIds = await computeAssignedPropertyIds(userId!) + if (cancelled) return + await syncPropertyAssets(supabase, userId!, propertyIds) + } + + function resyncV2Safe(crewMemberId: string): void { + if (cancelled) return + void resyncV2(crewMemberId).catch((err) => + console.error('[DexieProvider] v2 resync failed:', err) + ) + } + + // Rejoin after base + uniform jitter (5–35 s) so a Realtime node restart + // doesn't stampede every crew device back at the same instant. Tearing + // down the old channel sets v2Channel to null FIRST, so the stale + // channel's own CLOSED status callback (fired by removeChannel) can't + // schedule a second, competing reconnect loop. + function scheduleV2Reconnect(crewMemberId: string): void { + if (cancelled || v2ReconnectTimer !== null) return + v2ReconnectTimer = setTimeout(() => { + v2ReconnectTimer = null + if (cancelled) return + if (v2Channel) { + const stale = v2Channel + v2Channel = null + supabase.removeChannel(stale) + } + void subscribeV2(crewMemberId).catch((err) => { + console.error('[DexieProvider] v2 resubscribe failed:', err) + scheduleV2Reconnect(crewMemberId) + }) + }, reconnectDelayWithJitterMs()) + } + + async function subscribeV2(crewMemberId: string): Promise { + if (cancelled) return + // Private channels authorize against RLS on realtime.messages — the + // realtime socket needs the user's JWT attached before joining. + // Newer supabase-js versions refresh realtime auth automatically, but + // the explicit call is harmless and version-proof. + await supabase.realtime.setAuth() + if (cancelled) return + + const ch = supabase + .channel(`crew:${userId}`, { config: { private: true } }) + .on('broadcast', { event: 'sync' }, (message: { payload?: { entity?: unknown } }) => { + v2SignalHandler?.handleSignal(message.payload?.entity) + }) + .subscribe((status: string) => { + // Ignore callbacks from a channel that's been superseded (or a + // deliberate unmount teardown) — only the current channel may + // trigger resyncs/reconnects. + if (cancelled || ch !== v2Channel) return + if (status === 'SUBSCRIBED') { + // The gap while disconnected may have swallowed signals. + resyncV2Safe(crewMemberId) + } else if (status === 'CHANNEL_ERROR' || status === 'TIMED_OUT' || status === 'CLOSED') { + scheduleV2Reconnect(crewMemberId) + } + }) + v2Channel = ch + } + + async function runV2(crewMemberId: string): Promise { + // Signal → action map. Every action is a FULL-scope pull for its + // entity, so cursor advancement is safe (cursor invariant: cursors + // advance only from full-scope pulls). + v2SignalHandler = createSyncSignalHandler({ + turnovers: () => syncAssignedTurnovers(supabase, userId!, crewMemberId), + checklists: async () => { + // Full CURRENT assigned-turnover set from the local cache (kept + // reconciled by syncAssignedTurnovers) — full scope, so + // advanceCursors is allowed. All ids are already cached locally, + // so none qualify as fresh. + const db = getDexieDb(userId!) + const turnoverIds = (await db.turnovers.toArray()).map((t) => t.id) + await pullChecklistsForTurnovers(supabase, userId!, turnoverIds, crewMemberId, { + advanceCursors: true, + }) + }, + work_orders: () => syncWorkOrders(supabase, userId!, crewMemberId), + }) + + // Delta on mount, same as v1: cursors make this cheap, scope + // reconciliation makes it correct. + await resyncV2(crewMemberId) + if (cancelled) return + + // Re-attach the realtime JWT whenever Supabase refreshes the session + // token, so the private channel doesn't die with the old token. (The + // provider's other onAuthStateChange listener is skipped entirely + // when userIdProp is supplied — the crew layout's case — so the v2 + // path registers its own.) + const { data: authListener } = supabase.auth.onAuthStateChange( + (event: AuthChangeEvent) => { + if (event !== 'TOKEN_REFRESHED' || cancelled) return + void Promise.resolve(supabase.realtime.setAuth()).catch((err: unknown) => + console.error('[DexieProvider] v2 realtime setAuth refresh failed:', err) + ) + } + ) + v2AuthSubscription = authListener.subscription + + onlineHandler = () => resyncV2Safe(crewMemberId) + globalThis.addEventListener('online', onlineHandler) + + // PWA returning from background has likely missed broadcasts. + v2VisibilityHandler = () => { + if (globalThis.document?.visibilityState === 'visible') resyncV2Safe(crewMemberId) + } + globalThis.document?.addEventListener('visibilitychange', v2VisibilityHandler) + + // Safety poll: correctness backstop for missed broadcasts and the + // freshness path for property_assets. + v2SafetyPollTimer = setInterval(() => resyncV2Safe(crewMemberId), SAFETY_POLL_INTERVAL_MS) + + await subscribeV2(crewMemberId) + } + async function run() { const { data: crewMember } = await supabase .from('crew_members') @@ -265,6 +429,11 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin setCrewMemberId(crewMember.id as string) + if (CREW_SYNC_V2) { + await runV2(crewMember.id) + return + } + // Delta on mount too: a device with cursors only fetches what changed // since it last synced; a fresh device (no cursors) naturally does a // full pull. Scope reconciliation inside syncAssignedTurnovers guards @@ -311,6 +480,17 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin if (checklistChannel) supabase.removeChannel(checklistChannel) if (assetsChannel) supabase.removeChannel(assetsChannel) if (onlineHandler) globalThis.removeEventListener('online', onlineHandler) + // Crew Sync v2 teardown — everything here is null when the flag is off. + if (v2Channel) { + const ch = v2Channel + v2Channel = null // deliberate unmount: the CLOSED callback must not reconnect + supabase.removeChannel(ch) + } + if (v2ReconnectTimer !== null) clearTimeout(v2ReconnectTimer) + if (v2SafetyPollTimer !== null) clearInterval(v2SafetyPollTimer) + if (v2VisibilityHandler) globalThis.document?.removeEventListener('visibilitychange', v2VisibilityHandler) + v2AuthSubscription?.unsubscribe() + v2SignalHandler?.dispose() } }, [userId]) diff --git a/lib/dexie/sync/signals.ts b/lib/dexie/sync/signals.ts new file mode 100644 index 00000000..9d0b4a6c --- /dev/null +++ b/lib/dexie/sync/signals.ts @@ -0,0 +1,136 @@ +// lib/dexie/sync/signals.ts +// +// Pure/injectable pieces of the Crew Sync v2 broadcast-signal client path +// (docs/CREW_SYNC_V2_PHASES.md section 3), extracted from DexieProvider +// (lib/dexie/context.tsx) so they're unit-testable with fake timers and no +// DOM/Supabase dependency: +// +// - the entity vocabulary + validation (broadcast payloads carry +// `{ entity }` only — anything not in SYNC_SIGNAL_ENTITIES is ignored), +// - createSyncSignalHandler(): the signal → action map with a trailing +// per-entity debounce and per-entity serialization (an in-flight pull +// plus at most ONE queued follow-up — bursts never stack), +// - the reconnect-jitter computation (base 5 s + uniform [0, 30 s]). + +/** The only entity values a crew-sync broadcast may carry — see the + * entity → signal mapping table in docs/CREW_SYNC_V2_PHASES.md section 1. */ +export const SYNC_SIGNAL_ENTITIES = ['turnovers', 'checklists', 'work_orders'] as const + +export type SyncSignalEntity = (typeof SYNC_SIGNAL_ENTITIES)[number] + +export function isSyncSignalEntity(value: unknown): value is SyncSignalEntity { + return typeof value === 'string' && (SYNC_SIGNAL_ENTITIES as readonly string[]).includes(value) +} + +/** One async pull per entity. Broadcasts are wake-up signals only, so each + * action re-pulls its entity's FULL current scope — never a partial pull + * that could advance a global cursor (cursor invariant #2). */ +export type SyncSignalActions = Readonly Promise>> + +export interface SyncSignalHandler { + /** Feed a raw broadcast payload entity value. Invalid/unknown values are + * ignored; valid ones (re)start that entity's trailing debounce timer. */ + handleSignal(value: unknown): void + /** Cancel all pending timers and queued follow-ups. In-flight pulls run + * to completion but trigger nothing further. */ + dispose(): void +} + +export const SIGNAL_DEBOUNCE_MS = 1_000 + +interface EntityState { + timer: ReturnType | null + running: boolean + queued: boolean +} + +/** + * Builds the broadcast signal → delta-pull dispatcher. + * + * Per entity, independently: + * - trailing debounce: a burst of N signals inside `debounceMs` collapses + * into one action run, `debounceMs` after the last signal; + * - serialization: if the debounce fires while a previous run of the same + * action is still in flight, exactly one follow-up run is queued (a + * second, third, … fire while still in flight queues nothing more) and + * starts immediately after the in-flight run settles. + * + * Action rejections are logged and never break the handler — the safety + * poll in DexieProvider is the correctness backstop for a failed pull. + */ +export function createSyncSignalHandler( + actions: SyncSignalActions, + debounceMs: number = SIGNAL_DEBOUNCE_MS, +): SyncSignalHandler { + const states = new Map() + let disposed = false + + function stateFor(entity: SyncSignalEntity): EntityState { + let state = states.get(entity) + if (!state) { + state = { timer: null, running: false, queued: false } + states.set(entity, state) + } + return state + } + + function runAction(entity: SyncSignalEntity): void { + const state = stateFor(entity) + if (state.running) { + state.queued = true // exactly one follow-up, no matter how many fires land meanwhile + return + } + state.running = true + void actions[entity]() + .catch((err) => console.error(`[crewSyncSignals] ${entity} pull failed:`, err)) + .finally(() => { + state.running = false + if (disposed || !state.queued) return + state.queued = false + runAction(entity) + }) + } + + function handleSignal(value: unknown): void { + if (disposed || !isSyncSignalEntity(value)) return + const state = stateFor(value) + if (state.timer !== null) clearTimeout(state.timer) + state.timer = setTimeout(() => { + state.timer = null + runAction(value) + }, debounceMs) + } + + function dispose(): void { + disposed = true + for (const state of states.values()) { + if (state.timer !== null) clearTimeout(state.timer) + state.timer = null + state.queued = false + } + } + + return { handleSignal, dispose } +} + +// ── Reconnect jitter ─────────────────────────────────────────────────────── +// On CHANNEL_ERROR/TIMED_OUT/CLOSED the client rejoins after base + jitter, +// spreading the rejoin herd when a Realtime node restarts and every crew +// device loses its channel at the same instant. + +export const RECONNECT_BASE_DELAY_MS = 5_000 +export const RECONNECT_JITTER_MAX_MS = 30_000 + +/** Pure jitter math: `random` ∈ [0, 1) (clamped defensively) maps to a + * total delay uniform in [RECONNECT_BASE_DELAY_MS, + * RECONNECT_BASE_DELAY_MS + RECONNECT_JITTER_MAX_MS]. */ +export function computeReconnectDelayMs(random: number): number { + const clamped = Math.min(Math.max(random, 0), 1) + return RECONNECT_BASE_DELAY_MS + clamped * RECONNECT_JITTER_MAX_MS +} + +/** The one impure call site: draws the jitter sample. */ +export function reconnectDelayWithJitterMs(): number { + // eslint-disable-next-line no-restricted-properties -- reconnect jitter to spread realtime rejoins, not id/token generation + return computeReconnectDelayMs(Math.random()) +} diff --git a/unit/dexie/sync-reconnect-jitter.test.ts b/unit/dexie/sync-reconnect-jitter.test.ts new file mode 100644 index 00000000..995e87bb --- /dev/null +++ b/unit/dexie/sync-reconnect-jitter.test.ts @@ -0,0 +1,50 @@ +// Crew Sync v2 Phase 3 (docs/CREW_SYNC_V2_PHASES.md section 3c): +// reconnect jitter bounds — the rejoin delay is always within +// [base, base + 30 s] so a Realtime node restart never stampedes every +// crew device back at the same instant. + +import { describe, it, expect } from 'vitest' +import { + computeReconnectDelayMs, + reconnectDelayWithJitterMs, + RECONNECT_BASE_DELAY_MS, + RECONNECT_JITTER_MAX_MS, +} from '@/lib/dexie/sync/signals' + +describe('computeReconnectDelayMs', () => { + it('maps random=0 to exactly the base delay (5 s)', () => { + expect(computeReconnectDelayMs(0)).toBe(RECONNECT_BASE_DELAY_MS) + expect(RECONNECT_BASE_DELAY_MS).toBe(5_000) + }) + + it('maps random=1 to base + full jitter (35 s)', () => { + expect(computeReconnectDelayMs(1)).toBe(RECONNECT_BASE_DELAY_MS + RECONNECT_JITTER_MAX_MS) + expect(RECONNECT_JITTER_MAX_MS).toBe(30_000) + }) + + it('is linear in between (uniform jitter, not skewed)', () => { + expect(computeReconnectDelayMs(0.5)).toBe(RECONNECT_BASE_DELAY_MS + RECONNECT_JITTER_MAX_MS / 2) + expect(computeReconnectDelayMs(0.1)).toBeCloseTo(RECONNECT_BASE_DELAY_MS + RECONNECT_JITTER_MAX_MS * 0.1) + }) + + it('clamps out-of-range random inputs instead of over/undershooting', () => { + expect(computeReconnectDelayMs(-0.5)).toBe(RECONNECT_BASE_DELAY_MS) + expect(computeReconnectDelayMs(1.5)).toBe(RECONNECT_BASE_DELAY_MS + RECONNECT_JITTER_MAX_MS) + }) +}) + +describe('reconnectDelayWithJitterMs', () => { + it('always lands within [5 s, 35 s]', () => { + for (let i = 0; i < 1_000; i++) { + const delay = reconnectDelayWithJitterMs() + expect(delay).toBeGreaterThanOrEqual(RECONNECT_BASE_DELAY_MS) + expect(delay).toBeLessThanOrEqual(RECONNECT_BASE_DELAY_MS + RECONNECT_JITTER_MAX_MS) + } + }) + + it('actually varies (it is jitter, not a constant)', () => { + const samples = new Set() + for (let i = 0; i < 100; i++) samples.add(reconnectDelayWithJitterMs()) + expect(samples.size).toBeGreaterThan(1) + }) +}) diff --git a/unit/dexie/sync-signals.test.ts b/unit/dexie/sync-signals.test.ts new file mode 100644 index 00000000..7bc73655 --- /dev/null +++ b/unit/dexie/sync-signals.test.ts @@ -0,0 +1,218 @@ +// Crew Sync v2 Phase 3 (docs/CREW_SYNC_V2_PHASES.md section 3c): +// the broadcast signal → delta-pull dispatcher — entity validation, the +// per-entity trailing debounce, and in-flight serialization with exactly +// one queued follow-up. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { + createSyncSignalHandler, + isSyncSignalEntity, + SYNC_SIGNAL_ENTITIES, + SIGNAL_DEBOUNCE_MS, + type SyncSignalActions, +} from '@/lib/dexie/sync/signals' + +function makeActions() { + const turnovers = vi.fn(() => Promise.resolve()) + const checklists = vi.fn(() => Promise.resolve()) + const workOrders = vi.fn(() => Promise.resolve()) + const actions: SyncSignalActions = { + turnovers, + checklists, + work_orders: workOrders, + } + return { actions, turnovers, checklists, workOrders } +} + +describe('isSyncSignalEntity', () => { + it('accepts exactly the three known entities', () => { + for (const entity of SYNC_SIGNAL_ENTITIES) { + expect(isSyncSignalEntity(entity)).toBe(true) + } + }) + + it('rejects unknown strings and non-strings', () => { + expect(isSyncSignalEntity('property_assets')).toBe(false) + expect(isSyncSignalEntity('TURNOVERS')).toBe(false) + expect(isSyncSignalEntity('')).toBe(false) + expect(isSyncSignalEntity(undefined)).toBe(false) + expect(isSyncSignalEntity(null)).toBe(false) + expect(isSyncSignalEntity(42)).toBe(false) + expect(isSyncSignalEntity({ entity: 'turnovers' })).toBe(false) + }) +}) + +describe('createSyncSignalHandler', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + afterEach(() => { + vi.useRealTimers() + }) + + it('runs nothing before the debounce window elapses', async () => { + const { actions, turnovers } = makeActions() + const handler = createSyncSignalHandler(actions) + + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS - 1) + expect(turnovers).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(1) + expect(turnovers).toHaveBeenCalledTimes(1) + handler.dispose() + }) + + it('coalesces a burst of signals into one pull (trailing debounce)', async () => { + const { actions, turnovers } = makeActions() + const handler = createSyncSignalHandler(actions) + + // Five signals inside the window; the timer restarts on each. + for (let i = 0; i < 5; i++) { + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(100) + } + expect(turnovers).not.toHaveBeenCalled() + + // Fires SIGNAL_DEBOUNCE_MS after the LAST signal. + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS - 100) + expect(turnovers).toHaveBeenCalledTimes(1) + + // Quiet afterwards: nothing further fires. + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS * 5) + expect(turnovers).toHaveBeenCalledTimes(1) + handler.dispose() + }) + + it('ignores unknown entities entirely', async () => { + const { actions, turnovers, checklists, workOrders } = makeActions() + const handler = createSyncSignalHandler(actions) + + handler.handleSignal('bogus') + handler.handleSignal(undefined) + handler.handleSignal(null) + handler.handleSignal(7) + handler.handleSignal('property_assets') + + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS * 3) + expect(turnovers).not.toHaveBeenCalled() + expect(checklists).not.toHaveBeenCalled() + expect(workOrders).not.toHaveBeenCalled() + handler.dispose() + }) + + it('maps each entity to its own action, debounced independently', async () => { + const { actions, turnovers, checklists, workOrders } = makeActions() + const handler = createSyncSignalHandler(actions) + + handler.handleSignal('turnovers') + handler.handleSignal('checklists') + handler.handleSignal('work_orders') + + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS) + expect(turnovers).toHaveBeenCalledTimes(1) + expect(checklists).toHaveBeenCalledTimes(1) + expect(workOrders).toHaveBeenCalledTimes(1) + + // A signal for one entity never restarts another entity's timer. + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS / 2) + handler.handleSignal('checklists') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS / 2) + expect(turnovers).toHaveBeenCalledTimes(2) + expect(checklists).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS / 2) + expect(checklists).toHaveBeenCalledTimes(2) + handler.dispose() + }) + + it('serializes per entity: fires during an in-flight pull queue exactly one follow-up', async () => { + vi.useFakeTimers() + const resolvers: Array<() => void> = [] + const turnovers = vi.fn( + () => new Promise((resolve) => { resolvers.push(resolve) }) + ) + const { actions: base } = makeActions() + const handler = createSyncSignalHandler({ ...base, turnovers }) + + // First pull starts and stays in flight. + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS) + expect(turnovers).toHaveBeenCalledTimes(1) + + // THREE separate debounce firings land while it's still in flight … + for (let i = 0; i < 3; i++) { + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS) + } + // … none of them start a concurrent pull. + expect(turnovers).toHaveBeenCalledTimes(1) + + // Completing the in-flight pull releases exactly ONE queued follow-up. + resolvers[0]!() + await vi.advanceTimersByTimeAsync(0) + expect(turnovers).toHaveBeenCalledTimes(2) + + // Completing the follow-up releases nothing more — no stacking. + resolvers[1]!() + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS * 5) + expect(turnovers).toHaveBeenCalledTimes(2) + handler.dispose() + }) + + it('a rejected pull is contained and does not block later signals', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const turnovers = vi.fn() + .mockRejectedValueOnce(new Error('network down')) + .mockResolvedValue(undefined) + const { actions: base } = makeActions() + const handler = createSyncSignalHandler({ ...base, turnovers }) + + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS) + expect(turnovers).toHaveBeenCalledTimes(1) + expect(consoleError).toHaveBeenCalled() + + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS) + expect(turnovers).toHaveBeenCalledTimes(2) + + handler.dispose() + consoleError.mockRestore() + }) + + it('dispose cancels pending timers, queued follow-ups, and future signals', async () => { + const resolvers: Array<() => void> = [] + const turnovers = vi.fn( + () => new Promise((resolve) => { resolvers.push(resolve) }) + ) + const { actions: base, checklists } = makeActions() + const handler = createSyncSignalHandler({ ...base, turnovers }) + + // An in-flight pull with a queued follow-up … + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS) + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS) + expect(turnovers).toHaveBeenCalledTimes(1) + // … and a pending (not yet fired) debounce timer. + handler.handleSignal('checklists') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS / 2) + + handler.dispose() + + // Pending checklist timer never fires. + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS * 5) + expect(checklists).not.toHaveBeenCalled() + + // Queued turnovers follow-up is dropped when the in-flight pull settles. + resolvers[0]!() + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS * 5) + expect(turnovers).toHaveBeenCalledTimes(1) + + // Post-dispose signals are no-ops. + handler.handleSignal('turnovers') + await vi.advanceTimersByTimeAsync(SIGNAL_DEBOUNCE_MS * 5) + expect(turnovers).toHaveBeenCalledTimes(1) + }) +}) From 314e7c1b17ee6af1e6c654b385a52ca64f794edc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 22:12:02 +0000 Subject: [PATCH 05/10] Add types/database.ts drift check to CI (Tier 3 enforcement leftover) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the last item from PR #505's db-invariants job: check 4 (types vs. live schema) was deferred. Adds public.db_type_shape_report() (mirrors db_invariant_report()'s SECURITY DEFINER/service-role-only pattern) and scripts/check-type-drift.mjs, which diffs it against a mechanical parse of types/database.ts — every enum's labels, every table's presence, and column presence for every table wired into Database.public.Tables. Wired into the existing db-invariants CI job as a second step. First real run against both projects surfaced three genuine drift incidents beyond the wo_status one that motivated this check: - wo_source was missing 'vacancy_gap_suggestion', which advanceScheduleAfterCompletion() already branches on (20260725201000_add_vacancy_gap_suggestion_to_wo_source.sql). - crew_feedback's timestamp column was renamed created_at -> submitted_at on both projects by an earlier drift-capture migration, but support-inbox/page.tsx and its client still queried/displayed the old name — a live "column does not exist" bug in the support inbox feedback list, now fixed alongside the stale CrewFeedback interface. - inventory_count_drafts never actually had the reviewed_at/reviewed_by columns approveInventoryCount()/rejectInventoryCount() write to (an earlier migration defined them but no-op'd against an already-existing table) — every PM approve/reject of a pending count was failing. Added the columns for real, with their FK covering index (20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql). Also reconciled a long tail of types/database.ts fields that had drifted from the live schema (BookingSource.ownerrez, SupportMessageRole.human, Organization/Property/Booking/Turnover/WorkOrder/MaintenanceSchedule columns, TurnoverAssignment/PushSubscription/SupportConversation/ SupportMessage/InventoryCountDraft(Item)/InventoryTemplateItem shapes, new OrgSmsTemplate interface) and wired a dozen previously-unmapped but already-interfaced tables into Database.public.Tables. Intentional mismatches (the deprecated work_orders.assigned_crew_id, join-only relationship fields, DB-internal-only tables like platform_admins) are allowlisted in the new script, same shrink-only ratchet as SERVICE_ROLE_ONLY_TABLES. Migrations applied to both vpmznjktllhmmbfnxuvk and syhthijeqlnltufdawyb. get_advisors on production shows no new findings from db_type_shape_report. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- .github/workflows/ci.yml | 16 +- CLAUDE.md | 19 +- .../bookings/bookings-calendar.tsx | 9 + app/(dashboard)/bookings/bookings-client.tsx | 5 + app/(dashboard)/support-inbox/page.tsx | 4 +- .../support-inbox/support-inbox-client.tsx | 4 +- scripts/check-type-drift.mjs | 340 ++++++++++++++++++ .../20260725200500_db_type_shape_report.sql | 69 ++++ ...dd_vacancy_gap_suggestion_to_wo_source.sql | 10 + ...ewed_columns_to_inventory_count_drafts.sql | 27 ++ types/database.ts | 177 ++++++--- 11 files changed, 627 insertions(+), 53 deletions(-) create mode 100644 scripts/check-type-drift.mjs create mode 100644 supabase/migrations/20260725200500_db_type_shape_report.sql create mode 100644 supabase/migrations/20260725201000_add_vacancy_gap_suggestion_to_wo_source.sql create mode 100644 supabase/migrations/20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2215f2bb..43a8b3e2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,12 +120,15 @@ jobs: # ── Database invariants (structural enforcement Tier 3) ────────────────── # Checks the live schema for what no code-side check can see: RLS enabled # on every public table, no unexpected policy-less (deny-all) tables, a - # covering index on every FK column, zero anon table grants. Runs against - # the DEDICATED E2E project (same secrets as the e2e job — CI never holds - # production credentials); both projects receive every migration, so - # schema invariants verified there hold for production by construction. - # Self-disarming like the e2e job: secrets absent → warning annotation, - # job passes. See scripts/check-db-invariants.mjs for the check details. + # covering index on every FK column, zero anon table grants, and (the + # former enforcement leftover, closed by check-type-drift.mjs) that + # types/database.ts's enums/tables/columns still match the live schema. + # Runs against the DEDICATED E2E project (same secrets as the e2e job — + # CI never holds production credentials); both projects receive every + # migration, so invariants verified there hold for production by + # construction. Self-disarming like the e2e job: secrets absent -> + # warning annotation, job passes. See scripts/check-db-invariants.mjs and + # scripts/check-type-drift.mjs for the check details. db-invariants: runs-on: ubuntu-latest env: @@ -141,3 +144,4 @@ jobs: with: node-version: 22 - run: node scripts/check-db-invariants.mjs + - run: node scripts/check-type-drift.mjs diff --git a/CLAUDE.md b/CLAUDE.md index 22729937..8552de89 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1058,7 +1058,8 @@ item below" as part of the definition of done for any non-trivial change. Conventions in this file are enforced in code wherever they can be, so following them stops being a memory test. Four layers, checked in CI via -`npm run lint` and `vitest run` (plus the `db-invariants` CI job for layer 4): +`npm run lint` and `vitest run` (plus the `db-invariants` CI job for layer +4, which runs two scripts): 1. **ESLint rules** (`eslint.config.mjs`, the "Structural enforcement" config block) — AST-level bans scoped to `app/`, `lib/`, `components/`: @@ -1097,6 +1098,22 @@ following them stops being a memory test. Four layers, checked in CI via unauthenticated). Self-disarms with a warning when the E2E secrets are absent, same as the e2e job. + **Type drift gate** (`scripts/check-type-drift.mjs`, same + `db-invariants` CI job, run as its own step after + `check-db-invariants.mjs`) — diffs `types/database.ts` against the live + schema via `public.db_type_shape_report()`: every Postgres enum's labels + vs. its TS union (`ENUM_MAP`), every `public` table vs. + `Database.public.Tables` (`TABLE_ALLOWLIST` for the deliberately + unmodeled — `platform_admins`, `system_job_runs`, + `wo_number_counters`), and column presence for every mapped table + (`COLUMN_ALLOWLIST` for deliberate mismatches — e.g. the deprecated + `work_orders.assigned_crew_id`). Closes the exact class of bug that cost + real debugging time when the E2E project's `wo_status` enum silently + lacked `quote_requested` — see + `20260725043000_add_quote_requested_to_wo_status.sql`. Both allowlists + are shrink-only, same ratchet as `SERVICE_ROLE_ONLY_TABLES`. Self-disarms + the same way as the other two checks. + **The meta-rule: a new convention ships WITH its guardrail.** If a rule is worth adding to this file, add its ESLint rule or `unit/guardrails/` test in the same PR — and the CLAUDE.md prose for mechanically-checkable rules diff --git a/app/(dashboard)/bookings/bookings-calendar.tsx b/app/(dashboard)/bookings/bookings-calendar.tsx index 30fed2f7..cc8fcdd0 100644 --- a/app/(dashboard)/bookings/bookings-calendar.tsx +++ b/app/(dashboard)/bookings/bookings-calendar.tsx @@ -74,6 +74,14 @@ const SOURCE_LABELS: Record = { booking_com: 'Booking.com', direct: 'Direct', manual: 'Manual', + // 'ownerrez' on the booking_source enum predates + // mapOwnerRezChannelToSource() (lib/integrations/providers/ownerrez.ts), + // which normalizes every OwnerRez-synced booking into airbnb/vrbo/ + // booking_com/direct/other by channel name — no current write path sets + // this value, but the enum label is live on both projects (see + // 20260616141406_add_ownerrez_booking_source.sql) so it must stay + // handled here rather than silently falling through to `undefined`. + ownerrez: 'OwnerRez', other: 'Other', } @@ -83,6 +91,7 @@ const SOURCE_STYLE: Record = { booking_com: 'Booking.com', direct: 'Direct', manual: 'Manual', + // See bookings-calendar.tsx's SOURCE_LABELS comment — 'ownerrez' is a live + // enum label (20260616141406_add_ownerrez_booking_source.sql) with no + // current write path, kept here so it doesn't fall through to `undefined`. + ownerrez: 'OwnerRez', other: 'Other', } @@ -102,6 +106,7 @@ const SOURCE_COLORS: Record = { booking_com: 'blue', direct: 'green', manual: 'gold', + ownerrez: 'slate', other: 'slate', } diff --git a/app/(dashboard)/support-inbox/page.tsx b/app/(dashboard)/support-inbox/page.tsx index 1b9f07a6..a40679ba 100644 --- a/app/(dashboard)/support-inbox/page.tsx +++ b/app/(dashboard)/support-inbox/page.tsx @@ -31,11 +31,11 @@ export default async function SupportInboxPage() { supabase .from('crew_feedback') .select(` - id, feedback_text, created_at, + id, feedback_text, submitted_at, crew_members ( name ), organizations ( name ) `) - .order('created_at', { ascending: false }) + .order('submitted_at', { ascending: false }) .limit(50), ]) diff --git a/app/(dashboard)/support-inbox/support-inbox-client.tsx b/app/(dashboard)/support-inbox/support-inbox-client.tsx index edea5e53..db7378b3 100644 --- a/app/(dashboard)/support-inbox/support-inbox-client.tsx +++ b/app/(dashboard)/support-inbox/support-inbox-client.tsx @@ -29,7 +29,7 @@ interface MessageRow { interface FeedbackRow { id: string feedback_text: string - created_at: string + submitted_at: string crew_members: { name: string } | { name: string }[] | null organizations: { name: string } | { name: string }[] | null } @@ -379,7 +379,7 @@ export function SupportInboxClient({ · {orgName(f.organizations)} - {new Date(f.created_at).toLocaleDateString()} + {new Date(f.submitted_at).toLocaleDateString()}

diff --git a/scripts/check-type-drift.mjs b/scripts/check-type-drift.mjs new file mode 100644 index 00000000..14d6ff05 --- /dev/null +++ b/scripts/check-type-drift.mjs @@ -0,0 +1,340 @@ +#!/usr/bin/env node +/** + * FieldStay — types/database.ts drift check (structural enforcement, Tier 3 + * check 4 — the enforcement leftover from PR #505's db-invariants job). + * + * check-db-invariants.mjs polices schema-level SECURITY invariants + * (RLS/grants/FK indexes); this script polices SHAPE drift between the live + * schema and the hand-maintained types/database.ts — the class of bug that + * cost real debugging time when the E2E project's wo_status enum silently + * lacked 'quote_requested' (present on production, never captured in a + * tracked migration), making every /maintenance board query fail invisibly + * there (see supabase/migrations/20260725043000_add_quote_requested_to_wo_status.sql). + * This check makes that class of drift a CI failure instead of a mystery. + * + * It calls public.db_type_shape_report() (see + * supabase/migrations/20260725200500_db_type_shape_report.sql) against the + * E2E project and diffs it against a mechanical parse of types/database.ts. + * + * ── What this DOES check ───────────────────────────────────────────────── + * 1. Enum drift: every Postgres enum type's label set vs. the matching + * hand-written TS union type (see ENUM_MAP below for the name + * mapping — Postgres snake_case type name -> TS PascalCase type name). + * This is the check the wo_status incident needed and didn't have. + * 2. Table presence: every `public` BASE TABLE vs. every entry in + * `Database.public.Tables` in types/database.ts, both directions. + * 3. Column presence (bonus, best-effort): for any table wired into the + * `Database.public.Tables` map, the columns of its `Row` interface vs. + * the live table's columns, both directions. + * + * ── What this does NOT check (explicitly out of scope) ────────────────── + * - Column nullability strictness, precision/scale, or exact Postgres + * type vs. TS type compatibility — only column PRESENCE is diffed. + * - Views (`vendor_compliance_status`) — db_type_shape_report() only + * covers BASE TABLEs; types/database.ts models views separately under + * `Database.public.Views`, which this script does not parse. + * - CHECK-constraint-based unions on plain `text` columns (e.g. + * `InventoryCountDraft.status`, `AutoAssignMode`) — only real Postgres + * `CREATE TYPE ... AS ENUM` types are compared, since those are the only + * ones db_type_shape_report() can see via pg_enum. + * - Anything about `auth`/`storage`/`vault` schemas — public schema only. + * + * types/database.ts is parsed with regexes, not a TS compiler — it is a + * hand-written file with a consistent-enough shape + * (`export type Foo = 'a' | 'b' | ...`, `export interface Foo { field: T }`, + * `table_name: { Row: Foo; ... }`) for this to be reliable, but a + * sufficiently unusual edit (e.g. a union spread across a `type` alias + * built from other aliases) could parse as "field/value not found" rather + * than a true positive — false negatives (missed real drift) are more + * likely than false positives here, so treat a clean run as "no drift found + * in the parseable subset," not an absolute guarantee. + * + * Self-disarms with a CI warning annotation when the E2E secrets are + * absent, mirroring check-db-invariants.mjs and the e2e job. + */ + +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import path from 'node:path' + +const __dirname = path.dirname(fileURLToPath(import.meta.url)) +const TYPES_PATH = path.join(__dirname, '..', 'types', 'database.ts') + +const url = process.env.NEXT_PUBLIC_SUPABASE_URL +const key = process.env.SUPABASE_SERVICE_ROLE_KEY + +if (!url || !key) { + console.log( + '::warning title=Type drift gate UNARMED::NEXT_PUBLIC_SUPABASE_URL / ' + + 'SUPABASE_SERVICE_ROLE_KEY are not configured, so types/database.ts ' + + 'was NOT diffed against the live schema. Follow docs/E2E_SETUP.md to ' + + 'arm the gate.' + ) + process.exit(0) +} + +const PROD_PROJECT_REF = 'vpmznjktllhmmbfnxuvk' +if (url.includes(PROD_PROJECT_REF)) { + console.error( + 'Refusing to run: NEXT_PUBLIC_SUPABASE_URL points at the PRODUCTION ' + + 'Supabase project. CI must use the dedicated E2E project — see ' + + 'docs/E2E_SETUP.md.' + ) + process.exit(1) +} + +// ── Postgres enum type name -> TS union type name ────────────────────────── +// Every enum currently in the public schema (34, verified against both +// live projects on 2026-07-25). Add the pair here the same commit a new +// `CREATE TYPE ... AS ENUM` migration ships alongside its TS union. +const ENUM_MAP = { + asset_scan_status: 'AssetScanStatus', + asset_type: 'AssetType', + booking_source: 'BookingSource', + booking_status: 'BookingStatus', + checklist_status: 'ChecklistStatus', + comm_channel: 'CommChannel', + comm_recipient_type: 'CommRecipientType', + comm_source: 'CommSource', + compliance_doc_type: 'ComplianceDocType', + contact_pref: 'ContactPref', + crew_role: 'CrewRole', + ical_source: 'IcalSource', + inventory_category: 'InventoryCategory', + line_item_type: 'LineItemType', + macrs_class: 'MacrsClass', + member_role: 'MemberRole', + org_plan: 'OrgPlan', + org_plan_status: 'OrgPlanStatus', + po_status: 'PoStatus', + priority_level: 'PriorityLevel', + property_type: 'PropertyType', + quote_request_status: 'QuoteRequestStatus', + schedule_frequency: 'ScheduleFrequency', + schedule_type: 'ScheduleType', + support_category: 'SupportCategory', + support_message_role: 'SupportMessageRole', + sync_status: 'SyncStatus', + turnover_status: 'TurnoverStatus', + txn_category: 'TxnCategory', + txn_type: 'TxnType', + vendor_specialty: 'VendorSpecialty', + wo_category: 'WoCategory', + wo_source: 'WoSource', + wo_status: 'WoStatus', +} + +// ── Known, deliberate table/column mismatches ─────────────────────────────── +// Shrink-only, same convention as SERVICE_ROLE_ONLY_TABLES in +// check-db-invariants.mjs — a stale entry (table/column now modeled, or +// dropped) is itself a failure this script will report. +const TABLE_ALLOWLIST = new Set([ + // Never queried via .from() anywhere in app code — read only through + // SECURITY DEFINER RPCs (is_platform_staff_admin) or Postgres triggers + // (next_wo_number()), so there is no typed call site that needs a Row + // interface. See 20260622121938_observability_platform_admin_tables.sql. + 'platform_admins', + 'system_job_runs', + // Internal sequence table for work_orders.wo_number, mutated only inside + // the next_wo_number() Postgres function — never selected/inserted + // directly from application code. + 'wo_number_counters', +]) + +// column allowlist entries are `${table}.${column}` +const COLUMN_ALLOWLIST = new Set([ + // Deprecated, superseded by assigned_crew_member_id — CLAUDE.md's + // "Things That Will Break If You Do Them" table calls this out + // explicitly; the column must never be reintroduced into app code, so it + // deliberately has no place in WorkOrder. + 'work_orders.assigned_crew_id', + // Relationship/join fields populated only by a nested Supabase select + // (`turnovers(*, turnover_assignments(*, crew_members(...)))`), not real + // columns on the underlying table — db_type_shape_report() only reports + // physical columns, so these will always show as "TS-only". + 'turnovers.turnover_assignments', + 'turnover_assignments.crew_members', +]) + +// ── Fetch live shape ──────────────────────────────────────────────────────── + +const res = await fetch(new URL('/rest/v1/rpc/db_type_shape_report', url), { + method: 'POST', + headers: { + apikey: key, + authorization: `Bearer ${key}`, + 'content-type': 'application/json', + }, + body: '{}', +}) + +if (!res.ok) { + console.error(`db_type_shape_report RPC failed: HTTP ${res.status}`) + console.error( + 'Has supabase/migrations/20260725200500_db_type_shape_report.sql been applied to the E2E project?' + ) + process.exit(1) +} + +const report = await res.json() +const dbTables = report.tables ?? {} +const dbEnums = report.enums ?? {} + +// ── Parse types/database.ts ───────────────────────────────────────────────── + +const src = readFileSync(TYPES_PATH, 'utf8') + +// 1. Union type declarations: `export type Foo = 'a' | 'b' | ...` — may span +// multiple lines when the union is long (e.g. AssetType, WoCategory). +// Stops at the first line that doesn't continue the union (blank line, +// comment, or a new declaration). +function parseUnionTypes(text) { + const unions = {} + const re = /^export type (\w+)\s*=\s*([\s\S]*?)(?=\n(?:export |\/\/|$))/gm + for (const m of text.matchAll(re)) { + const [, name, body] = m + const values = [...body.matchAll(/'([^']+)'/g)].map((v) => v[1]) + if (values.length > 0) unions[name] = values + } + return unions +} + +// 2. `export interface Foo { field: Type | null; ... }` — one level, no +// nested braces other than Record etc. (which don't contain +// a bare `{` on their own line so the naive brace-match below is fine). +function parseInterfaces(text) { + const ifaces = {} + const re = /^export interface (\w+)\s*\{\n([\s\S]*?)^\}/gm + for (const m of text.matchAll(re)) { + const [, name, body] = m + const fields = {} + for (const line of body.split('\n')) { + const f = line.match(/^\s{2}(\w+)\??:\s*(.+?)\s*$/) + if (f) fields[f[1]] = f[2] + } + ifaces[name] = fields + } + return ifaces +} + +// 3. `Database.public.Tables` map: `table_name: { Row: InterfaceName; ...` +function parseTableMap(text) { + const tablesBlockMatch = text.match(/Tables:\s*\{([\s\S]*?)\n\s{4}\}\n\s{4}Views:/) + const block = tablesBlockMatch ? tablesBlockMatch[1] : text + const map = {} + const re = /^\s+(\w+):\s*\{\s*Row:\s*(\w+);/gm + for (const m of block.matchAll(re)) map[m[1]] = m[2] + return map +} + +const tsUnions = parseUnionTypes(src) +const tsInterfaces = parseInterfaces(src) +const tsTableMap = parseTableMap(src) + +// ── Compare ────────────────────────────────────────────────────────────────── + +const failures = [] + +// 1. Enum drift +for (const [pgName, tsName] of Object.entries(ENUM_MAP)) { + const dbValues = dbEnums[pgName] + const tsValues = tsUnions[tsName] + + if (dbValues === undefined) { + failures.push(`Enum '${pgName}' is in ENUM_MAP but no longer exists in the DB — remove the mapping.`) + continue + } + if (tsValues === undefined) { + failures.push(`Enum '${pgName}' -> TS union '${tsName}' not found in types/database.ts (parse miss, or the union was renamed/removed).`) + continue + } + + const dbSet = new Set(dbValues) + const tsSet = new Set(tsValues) + const dbOnly = dbValues.filter((v) => !tsSet.has(v)) + const tsOnly = tsValues.filter((v) => !dbSet.has(v)) + + if (dbOnly.length > 0 || tsOnly.length > 0) { + const parts = [] + if (dbOnly.length > 0) parts.push(`DB has, TS missing: ${dbOnly.join(', ')}`) + if (tsOnly.length > 0) parts.push(`TS has, DB missing: ${tsOnly.join(', ')}`) + failures.push(`Enum drift on '${pgName}' / TS '${tsName}': ${parts.join(' | ')}`) + } +} + +// Enums present in DB but never mapped at all (new enum types nobody wired up) +for (const pgName of Object.keys(dbEnums)) { + if (!(pgName in ENUM_MAP)) { + failures.push(`Enum '${pgName}' exists in the DB but has no entry in ENUM_MAP in scripts/check-type-drift.mjs — add the TS union mapping (or confirm it's intentionally unmodeled).`) + } +} + +// 2. Table presence, both directions +const dbTableNames = new Set(Object.keys(dbTables)) +const tsTableNames = new Set(Object.keys(tsTableMap)) + +for (const t of dbTableNames) { + if (!tsTableNames.has(t) && !TABLE_ALLOWLIST.has(t)) { + failures.push(`Table '${t}' exists in the DB but has no entry in Database.public.Tables in types/database.ts (and is not in TABLE_ALLOWLIST).`) + } +} +for (const t of tsTableNames) { + if (!dbTableNames.has(t)) { + failures.push(`Table '${t}' is modeled in types/database.ts's Tables map but no longer exists in the DB.`) + } +} +// Shrink-only allowlist hygiene — same ratchet as SERVICE_ROLE_ONLY_TABLES +for (const t of TABLE_ALLOWLIST) { + if (!dbTableNames.has(t)) { + failures.push(`Stale TABLE_ALLOWLIST entry '${t}' — table no longer exists in the DB. Remove it from scripts/check-type-drift.mjs.`) + } else if (tsTableNames.has(t)) { + failures.push(`Stale TABLE_ALLOWLIST entry '${t}' — it's now modeled in Database.public.Tables. Remove it from scripts/check-type-drift.mjs.`) + } +} + +// 3. Column presence (bonus), for every table wired into the Tables map +for (const [table, ifaceName] of Object.entries(tsTableMap)) { + const dbCols = dbTables[table] + const tsFields = tsInterfaces[ifaceName] + if (!dbCols || !tsFields) continue // table not in DB (already reported above), or interface parse miss + + const dbColNames = new Set(Object.keys(dbCols)) + const tsColNames = new Set(Object.keys(tsFields)) + + const dbOnly = [...dbColNames].filter((c) => !tsColNames.has(c) && !COLUMN_ALLOWLIST.has(`${table}.${c}`)) + const tsOnly = [...tsColNames].filter((c) => !dbColNames.has(c) && !COLUMN_ALLOWLIST.has(`${table}.${c}`)) + + if (dbOnly.length > 0) { + failures.push(`Table '${table}': DB has column(s) not in ${ifaceName}: ${dbOnly.join(', ')}`) + } + if (tsOnly.length > 0) { + failures.push(`Table '${table}': ${ifaceName} has field(s) not in the DB: ${tsOnly.join(', ')}`) + } +} +// Column allowlist hygiene +for (const entry of COLUMN_ALLOWLIST) { + const [table, col] = entry.split('.') + const dbCols = dbTables[table] + if (!dbCols) continue // table itself already reported/allowlisted above + const inDb = col in dbCols + const ifaceName = tsTableMap[table] + const inTs = ifaceName && tsInterfaces[ifaceName] && col in tsInterfaces[ifaceName] + if (!inDb && !inTs) { + failures.push(`Stale COLUMN_ALLOWLIST entry '${entry}' — column no longer exists anywhere. Remove it from scripts/check-type-drift.mjs.`) + } else if (inDb && inTs) { + failures.push(`Stale COLUMN_ALLOWLIST entry '${entry}' — column is now modeled on both sides. Remove it from scripts/check-type-drift.mjs.`) + } +} + +// ── Verdict ────────────────────────────────────────────────────────────────── + +if (failures.length > 0) { + console.error(`Type drift check FAILED (${failures.length} finding${failures.length === 1 ? '' : 's'}):\n`) + for (const f of failures) console.error(`✗ ${f}\n`) + process.exit(1) +} + +console.log( + 'Type drift check OK — every DB enum matches its TS union, every table is ' + + 'modeled or allowlisted, and column presence matches for every mapped table.' +) diff --git a/supabase/migrations/20260725200500_db_type_shape_report.sql b/supabase/migrations/20260725200500_db_type_shape_report.sql new file mode 100644 index 00000000..a20ddec0 --- /dev/null +++ b/supabase/migrations/20260725200500_db_type_shape_report.sql @@ -0,0 +1,69 @@ +-- db_type_shape_report(): structural-enforcement Tier 3, check 4 — the +-- types/database.ts drift gate's DB-side half. +-- +-- Returns a jsonb snapshot of the public schema's shape so +-- scripts/check-type-drift.mjs (CI db-invariants job) can diff it against the +-- committed types/database.ts. CI only holds the E2E project's service-role +-- key — no Supabase management token — so schema introspection has to go +-- through a service-role-only RPC like db_invariant_report(), not the +-- Supabase CLI type generator. +-- +-- Why this exists: the E2E project's wo_status enum silently lacked +-- 'quote_requested' (present in production, never captured in a migration), +-- which made every /maintenance board query fail invisibly there and cost +-- significant debugging time — see +-- 20260725043000_add_quote_requested_to_wo_status.sql. This report makes +-- schema-vs-types drift a CI failure instead of a mystery. +-- +-- Shape: +-- tables — { table_name: { column_name: { data_type, udt_name, is_nullable } } } +-- BASE TABLEs in public only (views excluded — types/database.ts +-- models views separately under Database.public.Views) +-- enums — { enum_name: [labels in enumsortorder] } for public enum types + +CREATE OR REPLACE FUNCTION public.db_type_shape_report() +RETURNS jsonb +LANGUAGE sql +STABLE +SECURITY DEFINER +SET search_path = '' +AS $$ + SELECT jsonb_build_object( + 'tables', ( + SELECT coalesce(jsonb_object_agg(x.table_name, x.cols), '{}'::jsonb) + FROM ( + SELECT c.table_name, + jsonb_object_agg(c.column_name, jsonb_build_object( + 'data_type', c.data_type, + 'udt_name', c.udt_name, + 'is_nullable', (c.is_nullable = 'YES') + )) AS cols + FROM information_schema.columns c + WHERE c.table_schema = 'public' + AND EXISTS ( + SELECT 1 FROM information_schema.tables t + WHERE t.table_schema = 'public' + AND t.table_name = c.table_name + AND t.table_type = 'BASE TABLE' + ) + GROUP BY c.table_name + ) x + ), + 'enums', ( + SELECT coalesce(jsonb_object_agg(e.enum_name, e.labels), '{}'::jsonb) + FROM ( + SELECT t.typname AS enum_name, + jsonb_agg(en.enumlabel ORDER BY en.enumsortorder) AS labels + FROM pg_catalog.pg_type t + JOIN pg_catalog.pg_enum en ON en.enumtypid = t.oid + JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace + WHERE n.nspname = 'public' + GROUP BY t.typname + ) e + ) + ); +$$; + +-- Introspection-only, but there's no reason clients should ever call it. +REVOKE EXECUTE ON FUNCTION public.db_type_shape_report() FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION public.db_type_shape_report() TO service_role; diff --git a/supabase/migrations/20260725201000_add_vacancy_gap_suggestion_to_wo_source.sql b/supabase/migrations/20260725201000_add_vacancy_gap_suggestion_to_wo_source.sql new file mode 100644 index 00000000..0963e21e --- /dev/null +++ b/supabase/migrations/20260725201000_add_vacancy_gap_suggestion_to_wo_source.sql @@ -0,0 +1,10 @@ +-- First-run reconciliation finding from the types/database.ts drift gate +-- (scripts/check-type-drift.mjs): WoSource in types/database.ts declares +-- 'vacancy_gap_suggestion' — and app/(dashboard)/maintenance/actions.ts's +-- advanceScheduleAfterCompletion() branches on it — but the wo_source enum +-- on BOTH projects lacks the label, so any future write of that source +-- value would throw "invalid input value for enum wo_source" at runtime. +-- Same failure class as the wo_status.quote_requested incident +-- (20260725043000_add_quote_requested_to_wo_status.sql), caught before it +-- shipped this time. Align the DB with the compile-time type. +ALTER TYPE wo_source ADD VALUE IF NOT EXISTS 'vacancy_gap_suggestion'; diff --git a/supabase/migrations/20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql b/supabase/migrations/20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql new file mode 100644 index 00000000..a43f5d6e --- /dev/null +++ b/supabase/migrations/20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql @@ -0,0 +1,27 @@ +-- Second first-run reconciliation finding from the types/database.ts drift +-- gate (scripts/check-type-drift.mjs), and the more serious of the two: +-- app/(dashboard)/inventory/actions.ts's approveInventoryCount() and +-- rejectInventoryCount() both write +-- .update({ status: ..., reviewed_at: , reviewed_by: user.id }) +-- to inventory_count_drafts, but that table has never had reviewed_at or +-- reviewed_by columns on either live project — the table that actually got +-- created was 20260604223326_add_inventory_count_drafts.sql (submitted_by, +-- status, notes only). A later migration +-- (20260609000003/20260609111810_schema_history_gaps.sql) DID define +-- reviewed_at/reviewed_by, but used CREATE TABLE IF NOT EXISTS against a +-- table that already existed by then, so it silently no-opped — the columns +-- were never actually added. Net effect: every PM approve/reject of a +-- pending inventory count throws "Could not find the 'reviewed_at' column" +-- and fails, on both projects, today. Same silent-drift failure mode as the +-- wo_status.quote_requested and crew_feedback.submitted_at incidents, just +-- undiscovered until this check went looking. Add the columns the app +-- already depends on rather than rip out the review audit trail. +ALTER TABLE public.inventory_count_drafts + ADD COLUMN IF NOT EXISTS reviewed_at timestamptz, + ADD COLUMN IF NOT EXISTS reviewed_by uuid REFERENCES auth.users(id) ON DELETE SET NULL; + +-- Covering index for the new FK, matching this repo's convention for every +-- other *_by/*_user_id → auth.users(id) column (see +-- idx_work_orders_completion_verified_by, idx_work_order_updates_updated_by_user_id). +CREATE INDEX IF NOT EXISTS idx_inventory_count_drafts_reviewed_by + ON public.inventory_count_drafts (reviewed_by); diff --git a/types/database.ts b/types/database.ts index 60cf63c3..c78a4b40 100644 --- a/types/database.ts +++ b/types/database.ts @@ -21,7 +21,7 @@ export type PropertyType = 'house' | 'condo' | 'cabin' | 'cottage' | 'tow export type IcalSource = 'airbnb' | 'vrbo' | 'booking_com' | 'direct' | 'other' export type SyncStatus = 'pending' | 'success' | 'error' export type BookingStatus = 'confirmed' | 'cancelled' | 'blocked' | 'tentative' -export type BookingSource = 'airbnb' | 'vrbo' | 'booking_com' | 'direct' | 'manual' | 'other' +export type BookingSource = 'airbnb' | 'vrbo' | 'booking_com' | 'direct' | 'manual' | 'ownerrez' | 'other' export type TurnoverStatus = 'pending_assignment' | 'assigned' | 'in_progress' | 'completed' | 'flagged' | 'cancelled' export type PriorityLevel = 'low' | 'medium' | 'high' | 'urgent' export type ContactPref = 'email' | 'sms' | 'both' @@ -74,7 +74,9 @@ export type IntegrationStatus = 'active' | 'revoked' | 'error' | 'disconnected // Support bot export type SupportCategory = 'faq' | 'technical' | 'account_specific' -export type SupportMessageRole = 'user' | 'assistant' +// 'human' = a platform staff member replying from the support inbox +// (app/api/support-inbox/reply/route.ts) — distinct from the bot 'assistant'. +export type SupportMessageRole = 'user' | 'assistant' | 'human' // ───────────────────────────────────────────────────────────── // Row interfaces — one per Supabase table @@ -106,7 +108,9 @@ export interface Organization { bathroom_room_template_id: string | null default_room_templates_seeded_at: string | null preferred_retailer: string | null + kroger_location_id: string | null kroger_location_name: string | null + auto_assign_enabled: boolean auto_assign_mode: AutoAssignMode vendor_auto_assign_mode: VendorAutoAssignMode comms_log_retention_days: number @@ -137,7 +141,6 @@ export interface Property { org_id: string name: string address: string | null - address_line1: string | null city: string | null state: string | null zip: string | null @@ -172,6 +175,8 @@ export interface Property { max_pets: number | null events_allowed: boolean | null min_renter_age: number | null + external_id: string | null + external_source: string | null created_at: string updated_at: string } @@ -242,6 +247,8 @@ export interface Booking { guidebook_pre_arrival_email_sent_at: string | null actual_total_amount: number | null door_code_secret_id: string | null + door_code_lock: string | null + door_code_synced_at: string | null guest_pii_anonymized_at: string | null created_at: string updated_at: string @@ -290,7 +297,12 @@ export interface CrewFeedback { crew_member_id: string property_id: string | null feedback_text: string - created_at: string + // Renamed from created_at → submitted_at on prod out-of-band, then + // replicated to e2e by 20260724160000_capture_prod_drift_functions_columns_seed.sql. + // types/database.ts and app/(dashboard)/support-inbox/{page,support-inbox-client}.tsx + // still referenced the old name until the drift check caught it — see + // 20260725200500_db_type_shape_report.sql / scripts/check-type-drift.mjs. + submitted_at: string } export interface CrewAvailabilityEntry { @@ -502,6 +514,11 @@ export interface Turnover { suggestion_reasoning: string | null suggestion_status: SuggestionStatus | null is_archived: boolean + // Booking date-change reconciliation (pending new window until PM acks) + dates_changed_at: string | null + dates_change_acknowledged_at: string | null + pending_checkin_datetime: string | null + pending_checkout_datetime: string | null created_at: string updated_at: string turnover_assignments: TurnoverAssignment[] @@ -509,8 +526,11 @@ export interface Turnover { export interface TurnoverAssignment { id: string + org_id: string | null turnover_id: string crew_member_id: string + user_id: string | null + property_id: string | null assigned_at: string notified_at: string | null notification_type: ContactPref | null @@ -627,27 +647,41 @@ export interface InventoryCountItem { created_at: string } +// Field names match the live schema (submitted_by, item_id, counted_qty) — +// not the never-applied crew_member_id/submitted_at/inventory_item_id/ +// submitted_quantity names from the superseded schema_history_gaps +// migrations (20260609000003/20260609111810), whose CREATE TABLE IF NOT +// EXISTS no-op'd against the table 20260604223326_add_inventory_count_drafts.sql +// had already created. reviewed_at/reviewed_by were added for real by +// 20260725201500_add_reviewed_columns_to_inventory_count_drafts.sql, after +// the drift check caught app/(dashboard)/inventory/actions.ts writing to +// them against columns that didn't exist yet. export interface InventoryCountDraft { - id: string - org_id: string - property_id: string - crew_member_id: string | null - status: 'pending_review' | 'approved' | 'rejected' - submitted_at: string | null - reviewed_at: string | null - reviewed_by: string | null - notes: string | null - created_at: string - updated_at: string + id: string + org_id: string + property_id: string + submitted_by: string | null + status: 'pending_review' | 'approved' | 'rejected' + reviewed_at: string | null + reviewed_by: string | null + notes: string | null + created_at: string + updated_at: string } export interface InventoryCountDraftItem { - id: string - draft_id: string - inventory_item_id: string - previous_quantity: number - submitted_quantity: number - created_at: string + id: string + draft_id: string + item_id: string + previous_quantity: number + counted_qty: number + // `note` (singular) is a legacy duplicate column nobody writes or reads — + // app/api/crew/inventory-count/route.ts and app/(dashboard)/inventory/ + // page.tsx both use `notes` (plural) exclusively. Kept here only so the + // interface doesn't silently drop a real live column (see CLAUDE.md's + // "Two inventory tables with different column names" section). + note: string | null + notes: string | null } export interface PurchaseOrder { @@ -746,10 +780,8 @@ export interface WorkOrder { completion_notes: string | null completed_by_name: string | null invoice_reference: string | null - quote_token: string | null - quote_token_expires_at: string | null - quoted_amount: number | null - quote_notes: string | null + // Quote fields live on the quote_requests table — work_orders itself has + // no quote_token/quoted_amount columns (removed 2026-07-25 drift fix). vendor_acknowledged_at: string | null vendor_acknowledged_by: string | null completion_verified_at: string | null @@ -768,6 +800,7 @@ export interface WorkOrder { suggested_vendor_ids: string[] | null suggestion_reasoning: string | null suggestion_status: SuggestionStatus | null + client_report_id: string | null created_at: string updated_at: string } @@ -876,6 +909,7 @@ export interface MaintenanceSchedule { property_id: string org_id: string assigned_vendor_id: string | null + vendor_specialty_hint: VendorSpecialty | null name: string description: string | null schedule_type: ScheduleType @@ -1148,18 +1182,30 @@ export interface InventoryTemplateItem { template_id: string catalog_item_id: string | null name: string - category: InventoryCategory - unit: string + // category/unit are nullable at the DB level, but every write path + // (app/(dashboard)/templates/inventory/actions.ts) always copies both + // from the source catalog row, so a null in practice means a bug, not an + // expected state. + category: InventoryCategory | null + unit: string | null par_level: number + // Legacy column from the original 20260604223335_add_inventory_templates.sql + // schema, superseded by par_level (added later) — never read or written + // by current app code (see actions.ts's "par_qty (unused, see Pass 1/3 + // self-audit)" comment). No created_at column exists on this table. + par_qty: number sort_order: number notes: string | null preferred_brand: string | null - created_at: string } +// Two disjoint subscriber shapes share this table: crew (crew_member_id set, +// user_id null — app/api/crew/push-subscribe/route.ts) and PM dashboard +// users (user_id set, crew_member_id null — app/api/dashboard/push-subscribe/route.ts). export interface PushSubscription { id: string - crew_member_id: string + crew_member_id: string | null + user_id: string | null org_id: string endpoint: string p256dh: string @@ -1510,22 +1556,31 @@ export interface SupportKbChunk { } export interface SupportConversation { - id: string - org_id: string - user_id: string - status: string - created_at: string - last_message_at: string + id: string + org_id: string + user_id: string + status: string + needs_human: boolean + escalation_reason: string | null + escalated_at: string | null + resolved_at: string | null + assigned_staff_id: string | null + staff_notified_at: string | null + created_at: string + last_message_at: string } export interface SupportMessage { - id: string - conversation_id: string - role: SupportMessageRole - content: string - category: SupportCategory | null - model_used: string | null - created_at: string + id: string + conversation_id: string + role: SupportMessageRole + content: string + category: SupportCategory | null + model_used: string | null + // Set only for role='human' (a platform staff reply via + // app/api/support-inbox/reply/route.ts) — null for bot/guest messages. + sent_by_user_id: string | null + created_at: string } // ── In-app notifications (bell) ───────────────────────────────────────────── @@ -1550,6 +1605,19 @@ export interface NotificationDigestState { updated_at: string } +// ── Org-level SMS template overrides ──────────────────────────────────────── +// Per-org customization of the default guest SMS copy in lib/sms/templates.ts. +// UNIQUE(org_id, key) — an org may override any subset of template keys; +// resetOrgSmsTemplate() deletes the row to fall back to the built-in default. +export interface OrgSmsTemplate { + id: string + org_id: string + key: string + body: string + created_at: string + updated_at: string +} + // ───────────────────────────────────────────────────────────── // Supabase Database interface — used by createClient() // @@ -1613,6 +1681,31 @@ export interface Database { communication_logs: { Row: CommunicationLog; Insert: Partial; Update: Partial; Relationships: [] } messages: { Row: Message; Insert: Partial; Update: Partial; Relationships: [] } push_subscriptions: { Row: PushSubscription; Insert: Partial; Update: Partial; Relationships: [] } + org_sms_templates: { Row: OrgSmsTemplate; Insert: Partial; Update: Partial; Relationships: [] } + + // ── Crew learning loop / feedback ─────────────────────── + assignment_outcomes: { Row: AssignmentOutcome; Insert: Partial; Update: Partial; Relationships: [] } + vendor_assignment_outcomes: { Row: VendorAssignmentOutcome; Insert: Partial; Update: Partial; Relationships: [] } + crew_feedback: { Row: CrewFeedback; Insert: Partial; Update: Partial; Relationships: [] } + checklist_item_signals: { Row: ChecklistItemSignal; Insert: Partial; Update: Partial; Relationships: [] } + + // ── Inventory templates ───────────────────────────────── + inventory_templates: { Row: InventoryTemplate; Insert: Partial; Update: Partial; Relationships: [] } + inventory_template_items: { Row: InventoryTemplateItem; Insert: Partial; Update: Partial; Relationships: [] } + + // ── Maintenance ────────────────────────────────────────── + maintenance_catalog_items: { Row: MaintenanceCatalogItem; Insert: Partial; Update: Partial; Relationships: [] } + maintenance_completions: { Row: MaintenanceCompletion; Insert: Partial; Update: Partial; Relationships: [] } + + // ── Work order billing ────────────────────────────────── + work_order_invoices: { Row: WorkOrderInvoice; Insert: Partial; Update: Partial; Relationships: [] } + + // ── Guest messaging ────────────────────────────────────── + reservation_messages: { Row: ReservationMessage; Insert: Partial; Update: Partial; Relationships: [] } + + // ── RepuGuard ──────────────────────────────────────────── + reviews: { Row: Review; Insert: Partial; Update: Partial; Relationships: [] } + review_responses: { Row: ReviewResponse; Insert: Partial; Update: Partial; Relationships: [] } // ── Asset Health ─────────────────────────────────────── property_assets: { Row: PropertyAsset; Insert: Partial; Update: Partial; Relationships: [] } From e9426241e21bf9b93ab3c38066d9f5cbe212f46e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 23:25:29 +0000 Subject: [PATCH 06/10] =?UTF-8?q?Fully=20disable=20Hostaway=20integration?= =?UTF-8?q?=20=E2=80=94=20not=20ready=20to=20be=20live=20yet?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Product decision: Hostaway must not be reachable at all right now, but must stay easy to re-enable later. The connect UI/server-actions were already disabled in 4c239e3 (revenue-posting gap), but the two real chokepoints were still live — this closes them: - lib/integrations/registry.ts: comment out the hostawayProvider import and its map entry, so getProvider('hostaway') now throws (webhook route at app/api/webhooks/[provider]/route.ts already 404s on that, and disconnectIntegration's revoke path already no-ops on failure) - app/api/inngest/route.ts: comment out the hostawayInitialSync import and its serve() array entry, so the job can never be invoked by Inngest even if something (a stray manual trigger, a resurrected send()) tried to fire integration/hostaway.sync.requested Both lib/integrations/providers/hostaway.ts and lib/inngest/functions/hostaway/initial-sync.ts are left functionally untouched — only a top-of-file comment was added pointing at exactly what's commented out and where, so re-enabling is a matter of uncommenting four spots rather than reconstructing anything. Verified no test asserts on registry contents or the Inngest functions array (all mock the registry module wholesale), so the existing Hostaway-specific tests still validate the adapter/job in isolation unchanged. ops/page.tsx and lib/support/account-tools.ts's Hostaway mentions are pre-existing generic/example references unrelated to the connect flow — left alone so support tooling still surfaces any historical connection state. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- app/api/inngest/route.ts | 9 +++++++-- lib/inngest/functions/hostaway/initial-sync.ts | 14 ++++++++++++++ lib/integrations/providers/hostaway.ts | 14 ++++++++++++++ lib/integrations/registry.ts | 10 ++++++++-- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/app/api/inngest/route.ts b/app/api/inngest/route.ts index 2bc2f5f2..5914231e 100644 --- a/app/api/inngest/route.ts +++ b/app/api/inngest/route.ts @@ -35,7 +35,10 @@ import { ownerRezReconciliationCron } from '@/lib/inngest/functions/ownerrez/ import { ownerRezReconciliationHandler } from '@/lib/inngest/functions/ownerrez/reconciliation-handler' // Hostaway integration -import { hostawayInitialSync } from '@/lib/inngest/functions/hostaway/initial-sync' +// Disabled — not ready for launch. Re-enable by uncommenting this import and +// the hostawayInitialSync entry in the serve() functions array below. Do not +// delete lib/inngest/functions/hostaway/initial-sync.ts. +// import { hostawayInitialSync } from '@/lib/inngest/functions/hostaway/initial-sync' // Hospitable integration import { hospInitialSync } from '@/lib/inngest/functions/hospitable/initial-sync' @@ -202,7 +205,9 @@ export const { GET, POST, PUT } = serve({ ownerRezReconciliationHandler, // Hostaway sync - hostawayInitialSync, + // Disabled — not ready for launch. Re-enable by uncommenting this line + // and the import above. + // hostawayInitialSync, // Hospitable sync hospInitialSync, diff --git a/lib/inngest/functions/hostaway/initial-sync.ts b/lib/inngest/functions/hostaway/initial-sync.ts index 1c9580c0..c429ee49 100644 --- a/lib/inngest/functions/hostaway/initial-sync.ts +++ b/lib/inngest/functions/hostaway/initial-sync.ts @@ -1,4 +1,18 @@ /** + * DISABLED — not ready for launch (product decision, 2026-07-25). This + * function is intact and functional; it is simply unregistered so it can + * never run: + * - Not registered in app/api/inngest/route.ts's serve() functions array + * (the `hostawayInitialSync` import + array entry are commented out there) + * - Its provider adapter (lib/integrations/providers/hostaway.ts) is not + * registered in lib/integrations/registry.ts either + * - Nothing sends the triggering event — connectWithApiKey() in + * app/(dashboard)/settings/integrations/actions.ts (the only place that + * used to send it) has its Hostaway credential-exchange path commented out + * To re-enable: uncomment the registry entry, the Inngest route + * registration, and the connect entry points (see hostaway.ts's top-of-file + * comment for the full list). + * * Hostaway Initial Sync * * Triggered by: integration/hostaway.sync.requested diff --git a/lib/integrations/providers/hostaway.ts b/lib/integrations/providers/hostaway.ts index 6ffcd13b..abb00f9a 100644 --- a/lib/integrations/providers/hostaway.ts +++ b/lib/integrations/providers/hostaway.ts @@ -1,5 +1,19 @@ // lib/integrations/providers/hostaway.ts // ============================================================ +// DISABLED — not ready for launch (product decision, 2026-07-25). This +// implementation is intact and functional; it is simply unreachable: +// - Not registered in lib/integrations/registry.ts (hostawayProvider +// import + map entry are commented out there) +// - Its Inngest sync job (lib/inngest/functions/hostaway/initial-sync.ts) +// is not registered in app/api/inngest/route.ts's serve() call +// - Every UI/server-action connect entry point (settings/integrations, +// setup/pms) already excludes 'hostaway' from its provider list +// To re-enable: uncomment the registry entry and the Inngest route +// registration, then re-add the connect UI/actions (see the "Hostaway is +// not fully implemented yet" comments in +// app/(dashboard)/settings/integrations/actions.ts and +// app/(dashboard)/settings/integrations/integrations-client.tsx). +// ============================================================ // Hostaway API-key provider adapter. // // Hostaway specifics: diff --git a/lib/integrations/registry.ts b/lib/integrations/registry.ts index 71c50a5e..2ac3dfec 100644 --- a/lib/integrations/registry.ts +++ b/lib/integrations/registry.ts @@ -13,14 +13,20 @@ import type { IntegrationProvider } from './types' import { ownerRezProvider } from './providers/ownerrez' import { krogerProvider } from './providers/kroger' -import { hostawayProvider } from './providers/hostaway' +// Hostaway disabled — not ready for launch. Re-enable by uncommenting this +// import and the 'hostaway' map entry below. Do not delete +// lib/integrations/providers/hostaway.ts. +// import { hostawayProvider } from './providers/hostaway' import { hospitableProvider } from './providers/hospitable' // Future: import { guestyProvider } from './providers/guesty' const providers = new Map([ ['ownerrez', ownerRezProvider], ['kroger', krogerProvider], - ['hostaway', hostawayProvider], + // Hostaway disabled — not ready for launch. Re-enable by uncommenting this + // line and the import above. Do not delete + // lib/integrations/providers/hostaway.ts. + // ['hostaway', hostawayProvider], ['hospitable', hospitableProvider], // ['guesty', guestyProvider], ]) From a44c229c743ff7fa876e6c5fd1d81fbb610fba09 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 23:27:04 +0000 Subject: [PATCH 07/10] Report SMS nudge-budget Redis outages to Sentry, document fail-closed spend gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daily nudge budget check in lib/sms/telnyx.ts already fails closed on a Redis error (shipped in 268beb5) — this only adds reportError alongside the existing console.error so the outage surfaces in Sentry, not just logs, and documents the fail-closed/fail-open split in CLAUDE.md's SMS section per docs/SCALABILITY_TIERS_REMAINING.md section 4. Audited every other Redis- backed limiter in the codebase (lib/rate-limit.ts consumers, proxy.ts, OwnerRez/Hospitable API budgets) — none of them gate real spend, so none needed a fail-open -> fail-closed change; see session report for the full classification. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- CLAUDE.md | 5 +++++ lib/sms/telnyx.ts | 2 ++ unit/sms/send-sms-gate.test.ts | 17 +++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 8552de89..ebde5b4c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,6 +129,11 @@ This flag is `false` until 10DLC campaign verification clears. Never send to guests without this gate in place. The flag lives in `lib/sms/telnyx.ts` — check that any new SMS-sending code respects it. +The daily nudge budget check in `lib/sms/telnyx.ts` (`claimNudgeBudgetSlot`) +fails CLOSED on a Redis error — the nudge is skipped, not sent — unlike the +abuse-rate limiters in `lib/rate-limit.ts`/`proxy.ts`, which deliberately +fail open; a spend ceiling must not disappear during an outage. + --- ## The Table That Breaks Everything If Wrong diff --git a/lib/sms/telnyx.ts b/lib/sms/telnyx.ts index 70bf887e..7a1fc747 100644 --- a/lib/sms/telnyx.ts +++ b/lib/sms/telnyx.ts @@ -1,4 +1,5 @@ import { Redis } from '@upstash/redis' +import { reportError } from '@/lib/observability/report-error' import type { GuidebookOfferType } from '@/types/database' const TELNYX_API_URL = 'https://api.telnyx.com/v2/messages' @@ -167,6 +168,7 @@ export async function sendSMS( console.error('[sms:nudge-budget] Redis unavailable — skipping nudge send', { error: err instanceof Error ? err.message : String(err), }) + reportError(err, { site: 'sms.telnyx.nudge_budget_unavailable' }) return { sent: false, reason: 'nudge budget check unavailable' } } if (!claimed) { diff --git a/unit/sms/send-sms-gate.test.ts b/unit/sms/send-sms-gate.test.ts index 524de2ac..a7680cc8 100644 --- a/unit/sms/send-sms-gate.test.ts +++ b/unit/sms/send-sms-gate.test.ts @@ -12,8 +12,12 @@ vi.mock('@upstash/redis', () => ({ expire = mockExpire }, })) +vi.mock('@/lib/observability/report-error', () => ({ + reportError: vi.fn(), +})) import { sendSMS } from '@/lib/sms/telnyx' +import { reportError } from '@/lib/observability/report-error' // CLAUDE.md: SMS_ENABLED is the single most safety-critical flag in this // codebase — every SMS send must be gated on it. These tests prove sendSMS() @@ -191,6 +195,19 @@ describe('sendSMS — daily nudge budget', () => { expect(errorSpy).toHaveBeenCalled() }) + it('reports the Redis outage to Sentry via reportError, not just console.error', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + const redisError = new Error('redis down') + mockIncr.mockRejectedValue(redisError) + + await sendSMS('+15551234567', 'Good morning!', { category: 'nudge' }) + + expect(reportError).toHaveBeenCalledWith( + redisError, + expect.objectContaining({ site: 'sms.telnyx.nudge_budget_unavailable' }), + ) + }) + it('transactional sends never consult the budget — door codes go out even if Redis is down', async () => { const fetchSpy = vi.spyOn(global, 'fetch').mockResolvedValue( new Response(JSON.stringify({ data: { id: 'msg_1' } }), { status: 200 }) From f626d1946129677f1d8ea24ec9c4490befaf4632 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 23:31:54 +0000 Subject: [PATCH 08/10] Add shared rate limiter + 429 handling for the Kroger API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cart automation fanning out across orgs shares one app-level Kroger client credential (one Vercel deployment IP/token), same class of problem as OwnerRez/Hospitable's shared-IP budgets — but Kroger had no limiter at all before this, so a busy cart-build tick could burn the whole platform's daily quota and 429 every other org silently. lib/rate-limit.ts gains four endpoint-class Upstash sliding-window limiters, each a fixed shared identifier (not per-org): products (9,000/day, 90% headroom off Kroger's confirmed 10,000/day) and locations (1,440/day, off the confirmed 1,600/day) are sourced from developer.kroger.com; cart and auth/identity limits aren't published there, so those two use clearly-commented conservative defaults. lib/kroger/client.ts routes every outbound call through a new krogerFetch() wrapper (same shape as hospitableFetch in providers/hospitable.ts): checks the relevant limiter first, reacts to a real 429 by parsing Retry-After, and fails open on a Redis error since this is an abuse/quota limiter, not a spend-budget one. Both throw the existing shared RateLimitError. Inside Inngest steps (build-shopping-cart.ts, kroger-connected.ts, integration-token-refresh-handler.ts) a RateLimitError now propagates so Inngest's own backoff retries the step — build-shopping-cart.ts's get-customer-token step previously swallowed every error, including a rate limit, into a silent list-only fallback; it now rethrows RateLimitError specifically before that fallback. The two OAuth callback routes run outside any Inngest step, so a rate limit there gets a distinct rate_limited reason on /connect/error instead of the generic token_exchange_failed/restart-connect-flow path, since neither route has a retry mechanism to lean on. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- .../integrations/[provider]/callback/route.ts | 9 ++ app/connect/error/page.tsx | 1 + app/connect/finish/route.ts | 12 ++ docs/SCALABILITY_TIERS_REMAINING.md | 23 ++- lib/inngest/functions/build-shopping-cart.ts | 11 ++ lib/kroger/client.ts | 98 ++++++++++-- lib/rate-limit.ts | 68 ++++++++ unit/inngest/build-shopping-cart.test.ts | 24 +++ unit/lib/kroger-client-rate-limit.test.ts | 147 ++++++++++++++++++ .../integrations-callback.test.ts | 21 ++- 10 files changed, 396 insertions(+), 18 deletions(-) create mode 100644 unit/lib/kroger-client-rate-limit.test.ts diff --git a/app/api/integrations/[provider]/callback/route.ts b/app/api/integrations/[provider]/callback/route.ts index 8654f06e..7c3bc507 100644 --- a/app/api/integrations/[provider]/callback/route.ts +++ b/app/api/integrations/[provider]/callback/route.ts @@ -43,6 +43,7 @@ import { getProvider } from '@/lib/integrations/registry' import { holdPendingOAuthCode } from '@/lib/integrations/vault' import { finalizeIntegrationConnection } from '@/lib/integrations/finalize-connection' import { logAuditEvent } from '@/lib/audit' +import { RateLimitError } from '@/lib/integrations/types' export async function GET( request: NextRequest, @@ -214,6 +215,14 @@ export async function GET( try { tokenData = await providerAdapter.exchangeCodeForToken({ code, redirectUri }) } catch (err) { + // This runs outside any Inngest step — there's no retry mechanism to + // lean on here, so a rate limit gets its own clear reason instead of + // the generic failure message, telling the PM it's transient and to + // just try again shortly rather than suggesting something is broken. + if (err instanceof RateLimitError) { + console.warn(`[OAuth:${providerId}] Token exchange rate limited (retry after ${err.retryAfter}s)`) + return errorRedirect('rate_limited') + } console.error(`[OAuth:${providerId}] Token exchange failed:`, err) return errorRedirect('token_exchange_failed') } diff --git a/app/connect/error/page.tsx b/app/connect/error/page.tsx index cb2b5c88..37f1895f 100644 --- a/app/connect/error/page.tsx +++ b/app/connect/error/page.tsx @@ -12,6 +12,7 @@ const REASON_MESSAGES: Record = { token_exchange_failed: 'We couldn’t complete the connection with the provider. Please try again.', storage_failed: 'We connected successfully but couldn’t save the connection securely. Please try again.', claim_failed: 'We couldn’t finish linking your connection to your new account. Please reconnect from Settings.', + rate_limited: 'This integration is temporarily rate-limited. Please wait a few minutes and try connecting again.', } export default async function ConnectErrorPage({ diff --git a/app/connect/finish/route.ts b/app/connect/finish/route.ts index b3e1e3e3..e806a477 100644 --- a/app/connect/finish/route.ts +++ b/app/connect/finish/route.ts @@ -34,6 +34,7 @@ import { claimPendingOAuthCode, cleanupExpiredPendingIntegrationArtifacts } from import { finalizeIntegrationConnection } from '@/lib/integrations/finalize-connection' import { logAuditEvent } from '@/lib/audit' import { revalidatePath } from 'next/cache' +import { RateLimitError } from '@/lib/integrations/types' export async function GET(request: NextRequest) { const appUrl = process.env.NEXT_PUBLIC_APP_URL! @@ -98,6 +99,17 @@ export async function GET(request: NextRequest) { try { tokenData = await providerAdapter.exchangeCodeForToken({ code, redirectUri }) } catch (err) { + // This runs outside any Inngest step — there's no retry mechanism to + // lean on, and unlike the "code expired" case below, immediately + // bouncing through a fresh /connect flow would likely just hit the + // same rate limit again. Show a clear, actionable error instead. + if (err instanceof RateLimitError) { + console.warn(`[connect/finish] Token exchange rate limited for ${providerId} (retry after ${err.retryAfter}s)`) + const url = new URL('/connect/error', appUrl) + url.searchParams.set('provider', providerId) + url.searchParams.set('error', 'rate_limited') + return NextResponse.redirect(url) + } // The code expired or was already used on the provider's side (signup — // especially with email confirmation — can outlive a provider code's // ~10 min lifetime). Restart the standard connect flow: the user is diff --git a/docs/SCALABILITY_TIERS_REMAINING.md b/docs/SCALABILITY_TIERS_REMAINING.md index b4ea2be0..adfa63a3 100644 --- a/docs/SCALABILITY_TIERS_REMAINING.md +++ b/docs/SCALABILITY_TIERS_REMAINING.md @@ -23,7 +23,7 @@ status (checked against the live codebase, not assumed): | 7. Dexie delta sync + outbox backoff | 2 | 🔶 Half done — delta sync shipped as Crew Sync v2 Phase 1; **outbox backoff = Phase 4, still open** | | 8. Bound the unbounded queries | 2 | ✅ Done — `checklist-signals` has a 180-day rolling window, reviews/owners pages are `.limit()`-bounded | | 9. Enforcement Tiers 1–3 (ESLint/guardrails → typed ServiceRoleContext → DB invariant CI gate) | — | ✅ Done — Tier 3 is PR #505 | -| 10. Tier 3 hygiene list | 3 | ⬜ **All open — sections below** | +| 10. Tier 3 hygiene list | 3 | 🔶 Section 3 (Kroger rate limiter) done; sections 1, 2, 4, 5 remain — see below | So the actual remaining work is: **Crew Sync v2 Phases 2–5** (the other document), plus the four Tier 3 hygiene items and one enforcement leftover @@ -79,7 +79,26 @@ incremental cursor, unlike OwnerRez which has ## 3. Kroger API rate limiter -**Problem:** `lib/integrations/providers/kroger.ts` calls the Kroger API +**Status: ✅ Done.** `lib/rate-limit.ts` gained four endpoint-class limiters +(`krogerProductsApiLimiter` 9,000/day, `krogerLocationsApiLimiter` 1,440/day — +both 90%-headroom off Kroger's confirmed published daily limits; +`krogerCartApiLimiter` and `krogerAuthApiLimiter` use conservative defaults +where Kroger's own docs don't publish a figure — see the comments in that +file for sourcing). `lib/kroger/client.ts` now routes every outbound Kroger +call through a shared `krogerFetch()` wrapper (same shape as +`hospitableFetch` in `lib/integrations/providers/hospitable.ts`) that +consults the relevant limiter first and reacts to a real 429 by parsing +`Retry-After`, fails open on a Redis error, and throws the shared +`RateLimitError` either way. `build-shopping-cart.ts`'s `get-customer-token` +step now rethrows `RateLimitError` instead of swallowing it into the +list-only fallback; the two OAuth callback routes (which run outside any +Inngest step) now redirect to a distinct `rate_limited` reason instead of +the generic `token_exchange_failed`/restart-connect-flow path. See +`unit/lib/kroger-client-rate-limit.test.ts` and the added cases in +`unit/inngest/build-shopping-cart.test.ts` / +`unit/route-handlers/integrations-callback.test.ts`. + +**Original problem:** `lib/integrations/providers/kroger.ts` calls the Kroger API with no rate limiting or 429 handling — cart automation fanning out across orgs shares one IP/token budget, same class of problem as OwnerRez was. diff --git a/lib/inngest/functions/build-shopping-cart.ts b/lib/inngest/functions/build-shopping-cart.ts index 931847d0..147ba463 100644 --- a/lib/inngest/functions/build-shopping-cart.ts +++ b/lib/inngest/functions/build-shopping-cart.ts @@ -10,6 +10,7 @@ import { } from '@/lib/kroger/client' import { getValidKrogerToken } from '@/lib/integrations/providers/kroger-token' import { reportError } from '@/lib/observability/report-error' +import { RateLimitError } from '@/lib/integrations/types' import { NonRetriableError } from 'inngest' import { resend, FROM } from '@/lib/resend/client' import { renderShoppingCartReadyEmail } from '@/lib/resend/emails/shopping-cart-ready' @@ -145,6 +146,16 @@ export const buildShoppingCart = inngest.createFunction( try { return await getValidKrogerToken(connection.user_id) } catch (err) { + if (err instanceof RateLimitError) { + // Kroger's own API quota (or our proactive guard in front of it, + // see lib/kroger/client.ts's krogerFetch) is exhausted. Rethrow + // so Inngest retries this step with backoff instead of silently + // degrading to the list-only fallback below — that would mask a + // transient condition as a permanent one and skip a retry that + // would likely succeed once the window resets. + throw err + } + if (err instanceof NonRetriableError) { // Refresh token itself is revoked/expired — mark the connection so // the PM sees a reconnect prompt instead of the cart silently diff --git a/lib/kroger/client.ts b/lib/kroger/client.ts index 7a289977..a4e4a9ed 100644 --- a/lib/kroger/client.ts +++ b/lib/kroger/client.ts @@ -1,6 +1,14 @@ // lib/kroger/client.ts // Place at: lib/kroger/client.ts +import type { Ratelimit } from '@upstash/ratelimit' +import { + krogerAuthApiLimiter, + krogerProductsApiLimiter, + krogerLocationsApiLimiter, + krogerCartApiLimiter, +} from '@/lib/rate-limit' +import { RateLimitError } from '@/lib/integrations/types' import type { KrogerTokenResponse, KrogerProductSearchResponse, @@ -12,6 +20,56 @@ import type { const KROGER_API_BASE = 'https://api.kroger.com/v1' const KROGER_AUTH_BASE = 'https://api.kroger.com/v1/connect/oauth2' +// ── Rate limiting ──────────────────────────────────────────────── +// +// Every outbound Kroger call in this file goes through krogerFetch instead +// of calling fetch() directly, so the shared platform-wide budget (see +// lib/rate-limit.ts — one app-level Kroger client credential, not a +// per-org allocation, same rationale as hospitableFetch in +// lib/integrations/providers/hospitable.ts and OwnerRez's per-IP tracker +// in lib/integrations/providers/ownerrez-api.ts) is enforced uniformly: +// +// 1. Proactively check the relevant endpoint-class limiter BEFORE the +// call. Throws RateLimitError before Kroger would actually 429 us. +// 2. If Kroger 429s anyway, parse Retry-After and throw RateLimitError +// with that exact wait time. +// +// Fails OPEN if the limiter check itself errors (Redis unavailable) — +// this is an abuse/external-quota limiter guarding Kroger's own rate +// limit, not a spend-budget limiter (see +// docs/SCALABILITY_TIERS_REMAINING.md item 4, which reserves fail-closed +// behavior for money-spending paths only). Matches proxy.ts's +// rateLimiterForPathname() convention: log and proceed rather than +// blocking every Kroger call over an infrastructure outage. +async function krogerFetch( + limiter: Ratelimit, + identifier: string, + input: string, + init: RequestInit, +): Promise { + try { + const { success, reset } = await limiter.limit(identifier) + if (!success) { + const retryAfterSeconds = Math.max(1, Math.ceil((reset - Date.now()) / 1000)) + throw new RateLimitError(retryAfterSeconds) + } + } catch (err) { + if (err instanceof RateLimitError) throw err + // Redis unavailable or otherwise erroring — fail open. Don't block + // every Kroger call over an infrastructure issue on our side. + console.error('[Kroger] rate limit check failed — proceeding without it', err) + } + + const res = await fetch(input, init) + + if (res.status === 429) { + const retryAfter = parseInt(res.headers.get('Retry-After') ?? '60', 10) + throw new RateLimitError(retryAfter) + } + + return res +} + // ── Token Management ──────────────────────────────────────────── /** @@ -29,7 +87,7 @@ export async function getClientToken(): Promise { const credentials = Buffer.from(`${clientId}:${clientSecret}`).toString('base64') - const res = await fetch(`${KROGER_AUTH_BASE}/token`, { + const res = await krogerFetch(krogerAuthApiLimiter, 'kroger-auth', `${KROGER_AUTH_BASE}/token`, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', @@ -62,7 +120,7 @@ export async function exchangeCodeForCustomerToken( const clientSecret = process.env.KROGER_CLIENT_SECRET! const credentials = Buffer.from(`${clientId}:${clientSecret}`).toString('base64') - const res = await fetch(`${KROGER_AUTH_BASE}/token`, { + const res = await krogerFetch(krogerAuthApiLimiter, 'kroger-auth', `${KROGER_AUTH_BASE}/token`, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', @@ -93,7 +151,7 @@ export async function refreshCustomerToken( const clientSecret = process.env.KROGER_CLIENT_SECRET! const credentials = Buffer.from(`${clientId}:${clientSecret}`).toString('base64') - const res = await fetch(`${KROGER_AUTH_BASE}/token`, { + const res = await krogerFetch(krogerAuthApiLimiter, 'kroger-auth', `${KROGER_AUTH_BASE}/token`, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', @@ -135,7 +193,7 @@ export function buildKrogerAuthUrl(state: string, redirectUri: string): string { export async function getKrogerProfile( customerToken: string, ): Promise<{ id: string } | null> { - const res = await fetch(`${KROGER_API_BASE}/identity/profile`, { + const res = await krogerFetch(krogerAuthApiLimiter, 'kroger-auth', `${KROGER_API_BASE}/identity/profile`, { headers: { 'Authorization': `Bearer ${customerToken}`, 'Accept': 'application/json', @@ -163,12 +221,17 @@ export async function searchProducts( 'filter.fulfillment': 'ais', }) - const res = await fetch(`${KROGER_API_BASE}/products?${params.toString()}`, { - headers: { - 'Authorization': `Bearer ${token}`, - 'Accept': 'application/json', + const res = await krogerFetch( + krogerProductsApiLimiter, + 'kroger-products', + `${KROGER_API_BASE}/products?${params.toString()}`, + { + headers: { + 'Authorization': `Bearer ${token}`, + 'Accept': 'application/json', + }, }, - }) + ) if (!res.ok) { console.error(`Kroger product search failed for "${query}": ${res.status}`) @@ -191,12 +254,17 @@ export async function findNearestKrogerStore( 'filter.limit': '1', }) - const res = await fetch(`${KROGER_API_BASE}/locations?${params.toString()}`, { - headers: { - 'Authorization': `Bearer ${token}`, - 'Accept': 'application/json', + const res = await krogerFetch( + krogerLocationsApiLimiter, + 'kroger-locations', + `${KROGER_API_BASE}/locations?${params.toString()}`, + { + headers: { + 'Authorization': `Bearer ${token}`, + 'Accept': 'application/json', + }, }, - }) + ) if (!res.ok) return null @@ -216,7 +284,7 @@ export async function addItemsToKrogerCart( ): Promise { if (!items.length) return true - const res = await fetch(`${KROGER_API_BASE}/cart/add`, { + const res = await krogerFetch(krogerCartApiLimiter, 'kroger-cart', `${KROGER_API_BASE}/cart/add`, { method: 'PUT', headers: { 'Authorization': `Bearer ${customerToken}`, diff --git a/lib/rate-limit.ts b/lib/rate-limit.ts index 305f0de6..e9ab8e41 100644 --- a/lib/rate-limit.ts +++ b/lib/rate-limit.ts @@ -49,6 +49,74 @@ export const hospitableApiLimiter = new Ratelimit({ prefix: 'hospitable-api', }) +// Proactive outbound budget for our own calls TO Kroger's API — same +// rationale as hospitableApiLimiter/OwnerRez's per-IP tracker above: all +// FieldStay tenants share one Vercel deployment's outbound identity (one +// app-level Kroger client credential), so cart automation fanning out +// across orgs shares a single external quota, not a per-org one. Kroger +// publishes separate daily limits per endpoint class (confirmed against +// developer.kroger.com 2026-07-25 — Products API: 10,000 calls/day; +// Locations API: 1,600 calls/day per endpoint; Identity/profile: 5,000 +// calls/day), so each endpoint class gets its own limiter here rather than +// one shared bucket, mirroring how Kroger itself enforces it. Each slides +// at 90% of the documented ceiling (10% headroom) so FieldStay throws its +// own RateLimitError before Kroger would actually 429 it — same headroom +// convention as hospitableApiLimiter (54/60) and OwnerRez (270/300) above. +// All four are called with a FIXED identifier string (not per-org) so the +// budget is genuinely shared platform-wide, same as +// hospitableApiLimiter.limit('hospitable-api'). + +// Products API (product search, called per below-par item during cart +// building) — 10,000/day confirmed at +// developer.kroger.com/reference/api/product-api-public. +export const krogerProductsApiLimiter = new Ratelimit({ + redis, + limiter: Ratelimit.slidingWindow(9_000, '1 d'), + analytics: true, + prefix: 'kroger-products', +}) + +// Locations API (nearest-store lookup on Kroger connect) — 1,600/day per +// endpoint confirmed at +// developer.kroger.com/reference/api/location-api-public. +export const krogerLocationsApiLimiter = new Ratelimit({ + redis, + limiter: Ratelimit.slidingWindow(1_440, '1 d'), + analytics: true, + prefix: 'kroger-locations', +}) + +// Cart API (adding matched items to the customer's cart) — ⚠️ Kroger's +// published Cart API rate limit could not be confirmed: developer.kroger.com's +// Cart API reference page is JS-rendered and didn't return limit figures via +// search/fetch at implementation time (2026-07-25); Kroger's own support +// contact (APISupport@kroger.com) would be the way to confirm it directly. +// Treating this as unverifiable for now — conservatively assumes the same +// order of magnitude as the lowest CONFIRMED figure above (Locations' +// 1,600/day) rather than guessing something higher. Revisit if real 429s +// in production logs suggest the true ceiling is different. +export const krogerCartApiLimiter = new Ratelimit({ + redis, + limiter: Ratelimit.slidingWindow(1_440, '1 d'), + analytics: true, + prefix: 'kroger-cart', +}) + +// OAuth token endpoint (client-credentials + customer code exchange + +// refresh) and Identity/profile lookup, combined into one budget — both are +// inherently low-volume (tokens are cached ~30min per getClientToken's own +// doc comment; profile is fetched once per connect). Kroger's Identity API +// is confirmed at 5,000 calls/day; the token endpoint itself has no +// separately published figure, so this combined bucket conservatively +// reuses the Identity figure as its basis rather than assuming an +// unconfirmed higher number for token calls specifically. +export const krogerAuthApiLimiter = new Ratelimit({ + redis, + limiter: Ratelimit.slidingWindow(4_500, '1 d'), + analytics: true, + prefix: 'kroger-auth', +}) + // Public work order page — 20 requests per minute per IP // Allows a contractor to refresh and interact normally, blocks enumeration export const workOrderRatelimit = new Ratelimit({ diff --git a/unit/inngest/build-shopping-cart.test.ts b/unit/inngest/build-shopping-cart.test.ts index 4bf290ea..c090514a 100644 --- a/unit/inngest/build-shopping-cart.test.ts +++ b/unit/inngest/build-shopping-cart.test.ts @@ -36,6 +36,7 @@ import { } from '@/lib/kroger/client' import { getValidKrogerToken } from '@/lib/integrations/providers/kroger-token' import { reportError } from '@/lib/observability/report-error' +import { RateLimitError } from '@/lib/integrations/types' import { resend } from '@/lib/resend/client' import { renderShoppingCartReadyEmail } from '@/lib/resend/emails/shopping-cart-ready' import { invokeHandler } from './test-helpers' @@ -332,6 +333,29 @@ describe('buildShoppingCart', () => { expect(addItemsToKrogerCart).not.toHaveBeenCalled() }) + it('rethrows a Kroger rate limit from token refresh so Inngest retries the step, instead of degrading to list-only', async () => { + const supabase = makeSupabase({ + organizations: [{ data: { id: 'org_1', preferred_retailer: 'kroger' }, error: null }], + inventory_items: [{ data: [belowParInventoryItem], error: null }], + integration_connections: [{ data: activeKrogerConnection, error: null }], + }) + ;(createServiceClient as ReturnType).mockReturnValue(supabase) + ;(getValidKrogerToken as ReturnType).mockRejectedValue(new RateLimitError(30)) + + await expect(invokeHandler(buildShoppingCart, baseCtx())).rejects.toBeInstanceOf(RateLimitError) + + // Rate limiting is a transient, retriable condition — must NOT be + // reported as an error or silently degrade to a list-only cart the + // way a genuinely revoked/expired token does. + expect(reportError).not.toHaveBeenCalled() + expect(getClientToken).not.toHaveBeenCalled() + expect(searchProducts).not.toHaveBeenCalled() + expect(addItemsToKrogerCart).not.toHaveBeenCalled() + + const revokeUpdate = supabase.calls.find((c) => c.table === 'integration_connections' && c.method === 'update') + expect(revokeUpdate).toBeUndefined() + }) + it('only scopes the inventory query to the requested properties when property_ids is provided', async () => { const supabase = makeSupabase({ organizations: [{ data: { id: 'org_1', preferred_retailer: 'kroger' }, error: null }], diff --git a/unit/lib/kroger-client-rate-limit.test.ts b/unit/lib/kroger-client-rate-limit.test.ts new file mode 100644 index 00000000..64a9bc77 --- /dev/null +++ b/unit/lib/kroger-client-rate-limit.test.ts @@ -0,0 +1,147 @@ +// Tests krogerFetch — the shared rate-limit wrapper in lib/kroger/client.ts +// used by every outbound Kroger API call (product search, cart add, +// location lookup, token/identity calls). See +// docs/SCALABILITY_TIERS_REMAINING.md item 3 and lib/rate-limit.ts's +// kroger*ApiLimiter exports. +// +// Unlike unit/inngest/kroger-connected.test.ts and +// unit/inngest/build-shopping-cart.test.ts (which mock '@/lib/kroger/client' +// wholesale to isolate the Inngest function under test), this file mocks +// '@/lib/rate-limit' instead and exercises the REAL lib/kroger/client.ts +// implementation, so the limiter-consultation / 429 / fail-open behavior +// itself is what's under test. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +vi.mock('@/lib/rate-limit', () => ({ + krogerAuthApiLimiter: { limit: vi.fn() }, + krogerProductsApiLimiter: { limit: vi.fn() }, + krogerLocationsApiLimiter: { limit: vi.fn() }, + krogerCartApiLimiter: { limit: vi.fn() }, +})) + +import { searchProducts, addItemsToKrogerCart, findNearestKrogerStore } from '@/lib/kroger/client' +import { RateLimitError } from '@/lib/integrations/types' +import { + krogerProductsApiLimiter, + krogerCartApiLimiter, + krogerLocationsApiLimiter, +} from '@/lib/rate-limit' + +function okJson(body: unknown) { + return { + ok: true, + status: 200, + headers: new Headers(), + json: async () => body, + text: async () => JSON.stringify(body), + } +} + +function rateLimited(retryAfterSeconds: string) { + return { + ok: false, + status: 429, + headers: new Headers({ 'Retry-After': retryAfterSeconds }), + json: async () => ({}), + text: async () => '', + } +} + +describe('lib/kroger/client — krogerFetch rate limiting', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('consults the endpoint-class limiter BEFORE making the outbound request', async () => { + const callOrder: string[] = [] + ;(krogerProductsApiLimiter.limit as ReturnType).mockImplementation(async () => { + callOrder.push('limiter') + return { success: true, reset: Date.now() + 1_000 } + }) + const fetchMock = vi.fn().mockImplementation(async () => { + callOrder.push('fetch') + return okJson({ data: [] }) + }) + vi.stubGlobal('fetch', fetchMock) + + await searchProducts('paper towels', 'loc_1', 'token_x') + + expect(krogerProductsApiLimiter.limit).toHaveBeenCalledWith('kroger-products') + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(callOrder).toEqual(['limiter', 'fetch']) + }) + + it('throws RateLimitError proactively — without ever calling fetch — once the shared budget reports exhausted', async () => { + const resetAt = Date.now() + 5_000 + ;(krogerProductsApiLimiter.limit as ReturnType).mockResolvedValue({ success: false, reset: resetAt }) + const fetchMock = vi.fn() + vi.stubGlobal('fetch', fetchMock) + + await expect(searchProducts('paper towels', 'loc_1', 'token_x')).rejects.toBeInstanceOf(RateLimitError) + expect(fetchMock).not.toHaveBeenCalled() + }) + + it('reacts to a genuine 429 from Kroger by throwing RateLimitError with the exact Retry-After wait time', async () => { + ;(krogerCartApiLimiter.limit as ReturnType).mockResolvedValue({ success: true, reset: Date.now() + 1_000 }) + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(rateLimited('42'))) + + let caught: unknown + try { + await addItemsToKrogerCart([{ upc: '0001111041700', quantity: 1, modality: 'PICKUP' }], 'customer_token') + } catch (err) { + caught = err + } + + expect(caught).toBeInstanceOf(RateLimitError) + expect((caught as RateLimitError).retryAfter).toBe(42) + }) + + it('falls back to a 60s default Retry-After when Kroger 429s without the header', async () => { + ;(krogerLocationsApiLimiter.limit as ReturnType).mockResolvedValue({ success: true, reset: Date.now() + 1_000 }) + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, status: 429, headers: new Headers(), text: async () => '', + })) + + let caught: unknown + try { + await findNearestKrogerStore('35007', 'token_x') + } catch (err) { + caught = err + } + + expect(caught).toBeInstanceOf(RateLimitError) + expect((caught as RateLimitError).retryAfter).toBe(60) + }) + + it('fails OPEN — still makes the real request — when the limiter check itself errors (Redis unavailable)', async () => { + ;(krogerProductsApiLimiter.limit as ReturnType).mockRejectedValue(new Error('ECONNREFUSED')) + const fetchMock = vi.fn().mockResolvedValue(okJson({ data: [] })) + vi.stubGlobal('fetch', fetchMock) + + const result = await searchProducts('paper towels', 'loc_1', 'token_x') + + // Proceeded to the real call instead of throwing — an abuse/quota + // limiter fails open on infra errors, unlike a spend-budget limiter. + expect(result).toEqual([]) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it('does not fail open for a genuine RateLimitError — only for the limiter check erroring', async () => { + // Guards against a regression that would swallow the proactive + // RateLimitError itself into the fail-open branch. + ;(krogerProductsApiLimiter.limit as ReturnType).mockResolvedValue({ + success: false, + reset: Date.now() + 2_000, + }) + const fetchMock = vi.fn() + vi.stubGlobal('fetch', fetchMock) + + await expect(searchProducts('paper towels', 'loc_1', 'token_x')).rejects.toBeInstanceOf(RateLimitError) + expect(fetchMock).not.toHaveBeenCalled() + }) +}) diff --git a/unit/route-handlers/integrations-callback.test.ts b/unit/route-handlers/integrations-callback.test.ts index 4eca92fe..1f3d4b5f 100644 --- a/unit/route-handlers/integrations-callback.test.ts +++ b/unit/route-handlers/integrations-callback.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { NextRequest } from 'next/server' -import type { IntegrationProvider } from '@/lib/integrations/types' +import { RateLimitError, type IntegrationProvider } from '@/lib/integrations/types' vi.mock('server-only', () => ({})) vi.mock('@supabase/ssr', () => ({ @@ -232,6 +232,25 @@ describe('GET /api/integrations/[provider]/callback (OAuth CSRF state validation expect(storeIntegrationToken).not.toHaveBeenCalled() }) + it('redirects to rate_limited (not the generic token_exchange_failed) when the provider adapter throws a RateLimitError during exchange — this route runs outside any Inngest step, so there is no retry to lean on and the UI needs a clear, distinct reason', async () => { + const admin = makeAdmin({ + oauth_states: [{ data: { state: 's1', provider_id: 'kroger', user_id: 'state_user', return_to: null }, error: null }], + }) + vi.mocked(createServiceClient).mockReturnValue(admin as never) + vi.mocked(getProvider).mockReturnValue( + oauthProvider({ + id: 'kroger', + exchangeCodeForToken: vi.fn(async () => { throw new RateLimitError(30) }), + }), + ) + + const res = await callGet('kroger', '?code=abc123&state=s1') + + expect(locationOf(res)).toContain('error=rate_limited') + expect(locationOf(res)).not.toContain('token_exchange_failed') + expect(storeIntegrationToken).not.toHaveBeenCalled() + }) + it('deferred exchange: holds the UNEXCHANGED code for post-signup claim (never exchanges, never attaches) when neither an active session nor the state row carries a user id', async () => { // Exchanging pre-signup registers the connection on the provider's side // ("Connected" with no FieldStay account behind it) — the marketplace From 81e6da8e17507f8621273f53be3de73b8c794b1a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 23:37:43 +0000 Subject: [PATCH 09/10] docs: close out tiers 2-4 status in scalability tracking doc Hostaway deliberately disabled (not built) per product decision; Kroger rate limiter, fail-closed spend budgets, and drift check all shipped. Only Crew Sync v2 Phase 5 remains open. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- docs/SCALABILITY_TIERS_REMAINING.md | 79 +++++++++++++++++++++++++---- 1 file changed, 68 insertions(+), 11 deletions(-) diff --git a/docs/SCALABILITY_TIERS_REMAINING.md b/docs/SCALABILITY_TIERS_REMAINING.md index adfa63a3..c45e19d0 100644 --- a/docs/SCALABILITY_TIERS_REMAINING.md +++ b/docs/SCALABILITY_TIERS_REMAINING.md @@ -23,18 +23,22 @@ status (checked against the live codebase, not assumed): | 7. Dexie delta sync + outbox backoff | 2 | 🔶 Half done — delta sync shipped as Crew Sync v2 Phase 1; **outbox backoff = Phase 4, still open** | | 8. Bound the unbounded queries | 2 | ✅ Done — `checklist-signals` has a 180-day rolling window, reviews/owners pages are `.limit()`-bounded | | 9. Enforcement Tiers 1–3 (ESLint/guardrails → typed ServiceRoleContext → DB invariant CI gate) | — | ✅ Done — Tier 3 is PR #505 | -| 10. Tier 3 hygiene list | 3 | 🔶 Section 3 (Kroger rate limiter) done; sections 1, 2, 4, 5 remain — see below | +| 10. Tier 3 hygiene list | 3 | ✅ Done — sections 1, 3, 4, 5 shipped; section 2 (Hostaway) deliberately disabled instead of built, see below | -So the actual remaining work is: **Crew Sync v2 Phases 2–5** (the other -document), plus the four Tier 3 hygiene items and one enforcement leftover -below. Each section here is independent — they can be separate small PRs -in any order. +So the only work still open is **Crew Sync v2 Phase 5** (the other +document) — everything in this doc is closed out. --- ## 1. `notifications` retention cron -**Problem:** the `notifications` table (in-app bell events, added +**Status: ✅ Done.** `lib/inngest/functions/cron/notifications-retention.ts` +runs daily, deleting read notifications older than 90 days and all +notifications older than 180 days, in bounded id-list batches (capped at +20 batches/10k rows per run, resuming the remainder the next day). See +`unit/inngest/cron-notifications-retention.test.ts`. + +**Original problem:** the `notifications` table (in-app bell events, added 2026-07-15) has no retention job — it grows forever. Every other append-heavy table already has one (`audit-retention.ts`, `comms-retention.ts`, `guest-pii-retention.ts` in @@ -57,11 +61,31 @@ append-heavy table already has one (`audit-retention.ts`, ## 2. Hostaway incremental sync -**Problem:** Hostaway sync is initial-import only / full-refetch — no -incremental cursor, unlike OwnerRez which has +**Status: ⏸️ Deliberately NOT built — Hostaway fully disabled instead, +per product decision (2026-07-25).** Hostaway isn't ready to be live yet. +Rather than build incremental sync on top of an integration that +shouldn't be reachable, the whole integration was commented out (not +deleted) at its two chokepoints: +- `lib/integrations/registry.ts` — `hostawayProvider` import + its + `['hostaway', hostawayProvider]` map entry. +- `app/api/inngest/route.ts` — `hostawayInitialSync` import + its + `serve()` array entry. + +The connect UI (`setup/pms/page.tsx`, `settings/integrations/ +integrations-client.tsx`/`actions.ts`) was already disabled by an earlier, +unrelated commit (no revenue-posting yet). The provider implementation +(`lib/integrations/providers/hostaway.ts`) and the sync job +(`lib/inngest/functions/hostaway/initial-sync.ts`) are untouched +internally — each has a top-of-file note naming the exact lines to +uncomment to re-enable. To resume this item once Hostaway is ready to +launch: uncomment those two chokepoints, re-enable the connect UI, THEN +come back to the original instructions below for incremental sync. + +**Original problem:** Hostaway sync is initial-import only / full-refetch — +no incremental cursor, unlike OwnerRez which has `ownerrez/incremental-sync.ts`. -**Instructions:** +**Instructions (for when Hostaway is re-enabled):** 1. Read `lib/integrations/providers/` for the Hostaway provider and the OwnerRez incremental sync function as the reference implementation. @@ -117,7 +141,25 @@ orgs shares one IP/token budget, same class of problem as OwnerRez was. ## 4. Fail-closed outbound budgets on Redis outage -**Problem:** budget/spend limiters (SMS budget chokepoint in +**Status: ✅ Done.** The SMS nudge-budget chokepoint +(`claimNudgeBudgetSlot` in `lib/sms/telnyx.ts`) already failed closed on a +Redis error (shipped alongside item 1/5's SMS spend guard) — this pass +added a `reportError` call alongside the existing `console.error` for +observability, plus a runtime test and a one-line CLAUDE.md note. Every +other limiter in `lib/rate-limit.ts`/`proxy.ts` was classified and +confirmed to be an abuse/API-quota limiter (correctly fail-open, matching +the Kroger limiter's own fail-open choice in section 3) — none needed to +change. Three limiters (`repuguardLimiter`, `scanLimiter`, +`supportChatLimiter`/`supportChatDailyLimiter`) gate paid Anthropic API +calls but are framed as per-user abuse quotas rather than spend budgets; +left alone as ambiguous rather than guessed on a money path — revisit if +that framing changes. A PM notification on a skipped SMS send was +considered and explicitly deferred (no `org_id` in scope at the call +site; wiring it through three cron call sites was judged more than a +small addition) — worth a follow-up if silent-skip visibility becomes a +real problem in practice. + +**Original problem:** budget/spend limiters (SMS budget chokepoint in `lib/sms/telnyx.ts`, retailer/cart spend) currently follow the same fail-open-on-Redis-error convention as the abuse rate limiters in `proxy.ts`. For token-enumeration throttles fail-open is correct (an @@ -143,7 +185,22 @@ nothing is watching. ## 5. Enforcement leftover: `types/database.ts` drift check -**Problem:** PR #505's `db-invariants` CI job implemented checks 1–3 of +**Status: ✅ Done.** `scripts/check-type-drift.mjs` (new) diffs a +service-role-only `public.db_type_shape_report()` RPC (new migration, +applied to both projects) against a parse of `types/database.ts` — enum +labels both directions, table presence both directions, and column +presence for every mapped table. Wired into the `db-invariants` CI job +as a second step. The first real run found and fixed three genuinely +live drift bugs beyond the original `wo_status` incident: `wo_source` +missing `vacancy_gap_suggestion` (migration), `inventory_count_drafts` +missing `reviewed_at`/`reviewed_by` entirely (migration — this was +actively breaking every PM approve/reject of a pending inventory count), +and `crew_feedback.created_at` vs. the real `submitted_at` column +(breaking the support inbox feedback list — types/code fix, no +migration). See the script's own header for exactly what it does and +doesn't check. + +**Original problem:** PR #505's `db-invariants` CI job implemented checks 1–3 of the Tier 3 outline (RLS everywhere, FK covering indexes, zero anon grants). Check 4 — generating types from the e2e project and diffing the table/column shape against the committed `types/database.ts` — was From d1b4af44321d66a89ed178341b86d9e32d3cb69e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 01:51:14 +0000 Subject: [PATCH 10/10] Fix SonarCloud findings on PR #508 and close two CI gate gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SonarCloud (13 findings on this branch): - lib/dexie/context.tsx: extract 3 deeply-nested closures (scheduleV2Reconnect's retry, runV2's checklist sync, the v2 auth-state-change handler) into named sibling functions to bring nesting depth back under the project's limit - lib/dexie/sync/signals.ts, lib/dexie/syncService.ts: NOSONAR the two Math.random() jitter sites (already eslint-disable-justified as timing jitter, not id/token generation) - lib/kroger/client.ts: Number.parseInt over parseInt - lib/sms/telnyx.ts: extract formatOfferPrice/formatPercentageOffer/ formatFixedAmountOffer to remove the nested ternaries and bring formatOffer's cognitive complexity back under 15 - scripts/check-type-drift.mjs: NOSONAR the three regex findings — this script only ever parses our own committed types/database.ts, never attacker-controlled input, so ReDoS isn't a real risk here - supabase/migrations/20260725200500_db_type_shape_report.sql: dedupe the 3x-repeated 'public' literal via a target_schema CTE; verified byte-identical md5 output on both projects before/after re-applying CI gate gaps surfaced by PR #508's own db-invariants job, both from PR #507 (guidebook v2) never having its migration pushed to the E2E project: - Applied 20260726100000_guidebook_v2_foundation.sql and 20260726120000_guidebook_property_photos_storage_policies.sql to E2E (already live on production) — clears the type-drift findings for guidebook_offer_redemptions and hero_photo_storage_path - New migration: covering index for guidebook_offer_redemptions.booking_id, the one FK column that migration's own indexes didn't cover — clears the FK-coverage invariant finding that surfaced once the table existed on E2E Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01EgNNY4XjLYRHZ3w4RHocLp --- lib/dexie/context.tsx | 59 +++++++++++-------- lib/dexie/sync/signals.ts | 2 +- lib/dexie/syncService.ts | 2 +- lib/kroger/client.ts | 2 +- lib/sms/telnyx.ts | 29 ++++++--- scripts/check-type-drift.mjs | 12 +++- .../20260725200500_db_type_shape_report.sql | 9 ++- ...ook_offer_redemptions_booking_id_index.sql | 11 ++++ 8 files changed, 86 insertions(+), 40 deletions(-) create mode 100644 supabase/migrations/20260726130000_guidebook_offer_redemptions_booking_id_index.sql diff --git a/lib/dexie/context.tsx b/lib/dexie/context.tsx index 4be1ce82..57a1996b 100644 --- a/lib/dexie/context.tsx +++ b/lib/dexie/context.tsx @@ -311,6 +311,16 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin ) } + // Retry helper for scheduleV2Reconnect's timer callback — kept as its + // own named function (a sibling, not nested inside the timer callback) + // to keep that callback's nesting depth within the project's ≤4 limit. + function retrySubscribeV2(crewMemberId: string): void { + void subscribeV2(crewMemberId).catch((err) => { + console.error('[DexieProvider] v2 resubscribe failed:', err) + scheduleV2Reconnect(crewMemberId) + }) + } + // Rejoin after base + uniform jitter (5–35 s) so a Realtime node restart // doesn't stampede every crew device back at the same instant. Tearing // down the old channel sets v2Channel to null FIRST, so the stale @@ -326,10 +336,7 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin v2Channel = null supabase.removeChannel(stale) } - void subscribeV2(crewMemberId).catch((err) => { - console.error('[DexieProvider] v2 resubscribe failed:', err) - scheduleV2Reconnect(crewMemberId) - }) + retrySubscribeV2(crewMemberId) }, reconnectDelayWithJitterMs()) } @@ -362,23 +369,36 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin v2Channel = ch } + // Full CURRENT assigned-turnover set from the local cache (kept + // reconciled by syncAssignedTurnovers) — full scope, so advanceCursors + // is allowed. All ids are already cached locally, so none qualify as + // fresh. Kept as its own named function (a sibling, not nested inside + // runV2's signal-handler map) to keep nesting depth within the + // project's ≤4 limit. + async function syncChecklistsV2(crewMemberId: string): Promise { + const db = getDexieDb(userId!) + const turnoverIds = (await db.turnovers.toArray()).map((t) => t.id) + await pullChecklistsForTurnovers(supabase, userId!, turnoverIds, crewMemberId, { + advanceCursors: true, + }) + } + + // Kept as its own named function (a sibling, not nested inside runV2) + // to keep nesting depth within the project's ≤4 limit. + function handleV2AuthStateChange(event: AuthChangeEvent): void { + if (event !== 'TOKEN_REFRESHED' || cancelled) return + void Promise.resolve(supabase.realtime.setAuth()).catch((err: unknown) => + console.error('[DexieProvider] v2 realtime setAuth refresh failed:', err) + ) + } + async function runV2(crewMemberId: string): Promise { // Signal → action map. Every action is a FULL-scope pull for its // entity, so cursor advancement is safe (cursor invariant: cursors // advance only from full-scope pulls). v2SignalHandler = createSyncSignalHandler({ turnovers: () => syncAssignedTurnovers(supabase, userId!, crewMemberId), - checklists: async () => { - // Full CURRENT assigned-turnover set from the local cache (kept - // reconciled by syncAssignedTurnovers) — full scope, so - // advanceCursors is allowed. All ids are already cached locally, - // so none qualify as fresh. - const db = getDexieDb(userId!) - const turnoverIds = (await db.turnovers.toArray()).map((t) => t.id) - await pullChecklistsForTurnovers(supabase, userId!, turnoverIds, crewMemberId, { - advanceCursors: true, - }) - }, + checklists: () => syncChecklistsV2(crewMemberId), work_orders: () => syncWorkOrders(supabase, userId!, crewMemberId), }) @@ -392,14 +412,7 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin // provider's other onAuthStateChange listener is skipped entirely // when userIdProp is supplied — the crew layout's case — so the v2 // path registers its own.) - const { data: authListener } = supabase.auth.onAuthStateChange( - (event: AuthChangeEvent) => { - if (event !== 'TOKEN_REFRESHED' || cancelled) return - void Promise.resolve(supabase.realtime.setAuth()).catch((err: unknown) => - console.error('[DexieProvider] v2 realtime setAuth refresh failed:', err) - ) - } - ) + const { data: authListener } = supabase.auth.onAuthStateChange(handleV2AuthStateChange) v2AuthSubscription = authListener.subscription onlineHandler = () => resyncV2Safe(crewMemberId) diff --git a/lib/dexie/sync/signals.ts b/lib/dexie/sync/signals.ts index 9d0b4a6c..fde94f8e 100644 --- a/lib/dexie/sync/signals.ts +++ b/lib/dexie/sync/signals.ts @@ -132,5 +132,5 @@ export function computeReconnectDelayMs(random: number): number { /** The one impure call site: draws the jitter sample. */ export function reconnectDelayWithJitterMs(): number { // eslint-disable-next-line no-restricted-properties -- reconnect jitter to spread realtime rejoins, not id/token generation - return computeReconnectDelayMs(Math.random()) + return computeReconnectDelayMs(Math.random()) // NOSONAR -- timing jitter only, not security-sensitive (see eslint-disable justification above) } diff --git a/lib/dexie/syncService.ts b/lib/dexie/syncService.ts index 7242931d..49727619 100644 --- a/lib/dexie/syncService.ts +++ b/lib/dexie/syncService.ts @@ -20,7 +20,7 @@ const MAX_RETRY_DELAY_MS = 300_000 export function computeNextAttemptAt(retryCount: number, now: number): number { const baseDelay = Math.min(2 ** (retryCount - 1) * BASE_RETRY_DELAY_MS, MAX_RETRY_DELAY_MS) // eslint-disable-next-line no-restricted-properties -- retry backoff jitter to spread outbox retry storms after an outage, not id/token generation - const jitter = Math.random() + const jitter = Math.random() // NOSONAR -- timing jitter only, not security-sensitive (see eslint-disable justification above) return now + baseDelay * (0.5 + jitter) } diff --git a/lib/kroger/client.ts b/lib/kroger/client.ts index a4e4a9ed..d787ca08 100644 --- a/lib/kroger/client.ts +++ b/lib/kroger/client.ts @@ -63,7 +63,7 @@ async function krogerFetch( const res = await fetch(input, init) if (res.status === 429) { - const retryAfter = parseInt(res.headers.get('Retry-After') ?? '60', 10) + const retryAfter = Number.parseInt(res.headers.get('Retry-After') ?? '60', 10) throw new RateLimitError(retryAfter) } diff --git a/lib/sms/telnyx.ts b/lib/sms/telnyx.ts index 7a1fc747..d9fbd23c 100644 --- a/lib/sms/telnyx.ts +++ b/lib/sms/telnyx.ts @@ -74,6 +74,25 @@ export function normalizePhoneToE164(raw: string): string | null { return null } +function formatOfferPrice(value: number): string { + return value % 1 === 0 ? String(value) : value.toFixed(2) +} + +function formatPercentageOffer(offerValue: number | null, offerItem: string | null): string | null { + if (!offerValue) return null + return offerItem + ? `${offerValue}% off ${offerItem} — just show this screen` + : `${offerValue}% off — just show this screen` +} + +function formatFixedAmountOffer(offerValue: number | null, offerItem: string | null): string | null { + if (!offerValue) return null + const price = formatOfferPrice(offerValue) + return offerItem + ? `$${price} off ${offerItem} — just show this screen` + : `$${price} off — just show this screen` +} + export function formatOffer( offerType: GuidebookOfferType, offerValue: number | null, @@ -82,16 +101,10 @@ export function formatOffer( ): string | null { switch (offerType) { case 'percentage': - if (!offerValue) return null - return offerItem - ? `${offerValue}% off ${offerItem} — just show this screen` - : `${offerValue}% off — just show this screen` + return formatPercentageOffer(offerValue, offerItem) case 'fixed_amount': - if (!offerValue) return null - return offerItem - ? `$${offerValue % 1 === 0 ? offerValue : offerValue.toFixed(2)} off ${offerItem} — just show this screen` - : `$${offerValue % 1 === 0 ? offerValue : offerValue.toFixed(2)} off — just show this screen` + return formatFixedAmountOffer(offerValue, offerItem) case 'item': return offerItem ? `Free ${offerItem} — just show this screen` : null diff --git a/scripts/check-type-drift.mjs b/scripts/check-type-drift.mjs index 14d6ff05..6a06c122 100644 --- a/scripts/check-type-drift.mjs +++ b/scripts/check-type-drift.mjs @@ -190,7 +190,9 @@ const src = readFileSync(TYPES_PATH, 'utf8') // comment, or a new declaration). function parseUnionTypes(text) { const unions = {} - const re = /^export type (\w+)\s*=\s*([\s\S]*?)(?=\n(?:export |\/\/|$))/gm + // `text` is always this repo's own committed types/database.ts, never + // attacker-controlled input, so ReDoS is not a real risk here. + const re = /^export type (\w+)\s*=\s*([\s\S]*?)(?=\n(?:export |\/\/|$))/gm // NOSONAR for (const m of text.matchAll(re)) { const [, name, body] = m const values = [...body.matchAll(/'([^']+)'/g)].map((v) => v[1]) @@ -208,8 +210,10 @@ function parseInterfaces(text) { for (const m of text.matchAll(re)) { const [, name, body] = m const fields = {} + // `line` comes from this repo's own committed types/database.ts, never + // attacker-controlled input, so ReDoS is not a real risk here. for (const line of body.split('\n')) { - const f = line.match(/^\s{2}(\w+)\??:\s*(.+?)\s*$/) + const f = line.match(/^\s{2}(\w+)\??:\s*(.+?)\s*$/) // NOSONAR if (f) fields[f[1]] = f[2] } ifaces[name] = fields @@ -222,7 +226,9 @@ function parseTableMap(text) { const tablesBlockMatch = text.match(/Tables:\s*\{([\s\S]*?)\n\s{4}\}\n\s{4}Views:/) const block = tablesBlockMatch ? tablesBlockMatch[1] : text const map = {} - const re = /^\s+(\w+):\s*\{\s*Row:\s*(\w+);/gm + // `block` comes from this repo's own committed types/database.ts, never + // attacker-controlled input, so ReDoS is not a real risk here. + const re = /^\s+(\w+):\s*\{\s*Row:\s*(\w+);/gm // NOSONAR for (const m of block.matchAll(re)) map[m[1]] = m[2] return map } diff --git a/supabase/migrations/20260725200500_db_type_shape_report.sql b/supabase/migrations/20260725200500_db_type_shape_report.sql index a20ddec0..8d48c34d 100644 --- a/supabase/migrations/20260725200500_db_type_shape_report.sql +++ b/supabase/migrations/20260725200500_db_type_shape_report.sql @@ -28,6 +28,9 @@ STABLE SECURITY DEFINER SET search_path = '' AS $$ + WITH target_schema AS ( + SELECT 'public'::name AS schema_name + ) SELECT jsonb_build_object( 'tables', ( SELECT coalesce(jsonb_object_agg(x.table_name, x.cols), '{}'::jsonb) @@ -39,10 +42,10 @@ AS $$ 'is_nullable', (c.is_nullable = 'YES') )) AS cols FROM information_schema.columns c - WHERE c.table_schema = 'public' + WHERE c.table_schema = (SELECT schema_name FROM target_schema) AND EXISTS ( SELECT 1 FROM information_schema.tables t - WHERE t.table_schema = 'public' + WHERE t.table_schema = (SELECT schema_name FROM target_schema) AND t.table_name = c.table_name AND t.table_type = 'BASE TABLE' ) @@ -57,7 +60,7 @@ AS $$ FROM pg_catalog.pg_type t JOIN pg_catalog.pg_enum en ON en.enumtypid = t.oid JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace - WHERE n.nspname = 'public' + WHERE n.nspname = (SELECT schema_name FROM target_schema) GROUP BY t.typname ) e ) diff --git a/supabase/migrations/20260726130000_guidebook_offer_redemptions_booking_id_index.sql b/supabase/migrations/20260726130000_guidebook_offer_redemptions_booking_id_index.sql new file mode 100644 index 00000000..0b69e3c4 --- /dev/null +++ b/supabase/migrations/20260726130000_guidebook_offer_redemptions_booking_id_index.sql @@ -0,0 +1,11 @@ +-- guidebook_offer_redemptions.booking_id (FK to bookings, ON DELETE SET NULL, +-- added by 20260726100000_guidebook_v2_foundation.sql) had no covering index — +-- the two indexes that migration added cover (sponsor_id, opened_at) and +-- (org_id, opened_at) only, neither of which covers a bare booking_id lookup. +-- An unindexed FK sequential-scans guidebook_offer_redemptions on every +-- bookings DELETE/UPDATE that touches booking_id (per the db-invariants +-- check-db-invariants.mjs FK-coverage gate, CLAUDE.md's structural +-- enforcement Tier 4). + +CREATE INDEX IF NOT EXISTS idx_guidebook_offer_redemptions_booking_id + ON guidebook_offer_redemptions (booking_id);