From 541f0baa2756bae8c209e3b082239677acdf5ea5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 00:23:21 +0000 Subject: [PATCH 1/2] fix(crew-sync): close the five data-loss paths in the offline layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Offline-sync audit, part 1 of 2. Each of these loses or corrupts crew work silently — nothing reaches the failed-sync surface, and in three of the five no delta pull will ever correct the divergence either. F1 — the optimistic local write and its outbox row were two separate IndexedDB transactions. A PWA reclaimed between them (iOS backgrounding, quota, a closed tab) left the cache updated with nothing queued to send it: the crew member sees their tick as saved forever, the server never hears about it, and because the server row's updated_at never changed, the delta pull never returns it. Adds enqueueMutationTx() and commits both writes in one Dexie transaction, in every helper plus photo-sync's applyUploadedPath. The processOutbox() kick stays outside the block — an IDB transaction auto-commits the moment an await leaves it. F2 — holdBackSuccessors() only saw successors that existed AT the moment of dead-lettering, which is the less likely half: the corrective edit is normally made after. Tick -> dead-letter -> un-tick (pushes fine) -> "Retry all" replays the stale tick on top and the server flips back. A record with a dead letter is now frozen at enqueue, so the whole sequence retries in order. F3 — logout with a second crew tab open. The shutdown latch is per-document module state but IndexedDB is per-origin, so the sibling tab held its connection, Dexie.delete blocked on it indefinitely, and the await before signOut()/redirect never resolved: logout silently did nothing and the cache stayed on a shared device. Adds a BroadcastChannel shutdown signal and bounds the delete so a tab that ignores it cannot strand the user mid-logout. F4 — discarding a dead letter removed the shadow overlay but not the cursor that had advanced past the server row it was masking, pinning the cache to a value the server never accepted. Same with no user action at all when the 30-day prune collects one. Adds invalidateCursorsFor() on both paths, plus forceFullCrewResync() as the repair path that did not exist (force was plumbed everywhere but never passed as true, and no cursor was ever reset). F5 — photo blobs live in a separate IndexedDB from their tracking rows, so the two can never be written atomically, and nothing collected a blob whose row never landed: megabytes each, until the browser evicts the whole origin and the mutation outbox with it. Adds a two-generation orphan sweep. A photo whose blob is gone now dead-letters instead of being deleted, which was indistinguishable from a successful upload. Also indexes `failed` (0/1 — IndexedDB cannot index a boolean, so every dead-letter query full-scanned the outbox, three of them live on every crew screen) and adds [table+targetId] for the per-record lookups F2 and holdBackSuccessors do. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Bs2uS5NYLR8Pvk4sw5NiKz --- app/crew/crew-shell.tsx | 17 +- lib/dexie/demo-readiness.ts | 6 +- lib/dexie/helpers.ts | 230 +++++++------ lib/dexie/photo-queue.ts | 22 ++ lib/dexie/photo-sync.ts | 78 +++-- lib/dexie/prune.ts | 84 ++++- lib/dexie/schema.ts | 170 +++++++++- lib/dexie/sync/cursors.ts | 56 +++- lib/dexie/sync/full-resync.ts | 21 ++ lib/dexie/syncService.ts | 105 +++++- unit/demo/demo-readiness.test.ts | 16 +- unit/dexie/fake-dexie.ts | 62 +++- unit/dexie/offline-write-durability.test.ts | 304 ++++++++++++++++++ unit/dexie/photo-sync-durability.test.ts | 27 +- unit/dexie/sync-outbox-backoff.test.ts | 4 +- unit/dexie/sync-outbox-durability.test.ts | 4 +- unit/dexie/sync-outbox-ordering.test.ts | 8 +- unit/dexie/sync-shadow-and-prune.test.ts | 12 +- .../crew-dead-letter-coverage.test.ts | 22 +- 19 files changed, 1060 insertions(+), 188 deletions(-) create mode 100644 unit/dexie/offline-write-durability.test.ts diff --git a/app/crew/crew-shell.tsx b/app/crew/crew-shell.tsx index 4faf7b8f..c7637990 100644 --- a/app/crew/crew-shell.tsx +++ b/app/crew/crew-shell.tsx @@ -6,7 +6,7 @@ import { CalendarCheck, CalendarDays, MessageSquare, LogOut, Bell, X, HelpCircle import { useLiveQuery } from 'dexie-react-hooks' import { DexieProvider, useDexieDb } from '@/lib/dexie/context' import { CrewContext } from '@/lib/crew/crew-context' -import { closeDexieDb, markDexieShutdown, resumeDexieDb } from '@/lib/dexie/schema' +import { closeDexieDb, listenForRemoteShutdown, markDexieShutdown, resumeDexieDb } from '@/lib/dexie/schema' import { getSyncEngine, disposeSyncEngine } from '@/lib/dexie/syncService' import { processPendingPhotoUploads } from '@/lib/dexie/photo-sync' import { countPendingSyncWork } from '@/lib/dexie/prune' @@ -236,6 +236,21 @@ export function CrewShell({ } } + // Logging out in ANOTHER tab has to end this one too. IndexedDB is a + // per-origin resource but the shutdown latch is per-document module state, + // so without this a sibling tab kept draining, kept re-creating the database + // the logging-out tab had just deleted, and kept rendering the signed-out + // crew member's assignments — and its open connection blocked that delete + // outright, which is what left logout hanging with the user still signed in. + useEffect(() => { + if (!userId) return + return listenForRemoteShutdown(userId, () => { + setSignedOut(true) + disposeSyncEngine() + startTransition(() => router.push('/login?next=/crew')) + }) + }, [userId, router]) + useEffect(() => { if (!userId || signedOut) return const supabase = createClient() diff --git a/lib/dexie/demo-readiness.ts b/lib/dexie/demo-readiness.ts index 8a16501c..f3e8f8c7 100644 --- a/lib/dexie/demo-readiness.ts +++ b/lib/dexie/demo-readiness.ts @@ -53,8 +53,10 @@ export async function checkDemoOfflineReadiness( workOrders, assets, ] = await Promise.all([ - db.mutations.filter((m) => m.failed !== true).count(), - db.mutations.filter((m) => m.failed === true).count(), + db.mutations.filter((m) => !m.failed).count(), + // `failed` is 0/1, not a boolean (IndexedDB can't index booleans) — so this + // is an index-backed count rather than a full scan of the outbox. + db.mutations.where('failed').equals(1).count(), db.turnovers.count(), db.properties.count(), db.checklist_instances.count(), diff --git a/lib/dexie/helpers.ts b/lib/dexie/helpers.ts index 73f4f2aa..6898ee5b 100644 --- a/lib/dexie/helpers.ts +++ b/lib/dexie/helpers.ts @@ -1,6 +1,55 @@ -import { getDexieDb } from './schema' +import { getDexieDb, isDexieShutdown, type FieldStayDexie, type MutationRow } from './schema' import { reportError } from '@/lib/observability/report-error' -import { enqueueMutation, getSyncEngine } from './syncService' +import { enqueueMutation, enqueueMutationTx, getSyncEngine } from './syncService' +import { invalidateCursorsFor } from './sync/cursors' + +/** + * Commits an optimistic local write and its outbox row in ONE Dexie + * transaction, then kicks the drain. + * + * The two writes used to be separate IndexedDB transactions. A PWA reclaimed + * between them (iOS backgrounding, a quota error, a closed tab) left the cache + * updated with nothing queued to send it: the crew member saw the change as + * saved forever, the server never heard about it, the failed-sync banner had + * no row to show, and no delta pull would ever correct it because the server + * row's updated_at never changed. See enqueueMutationTx(). + * + * `apply` may only touch Dexie. Anything that awaits a non-Dexie promise + * inside the transaction lets IndexedDB auto-commit it early, and the rest of + * the block throws TransactionInactiveError — which is precisely why the + * processOutbox() kick is outside it. + */ +async function writeAndQueue( + userId: string, + table: MutationRow['table'], + targetId: string, + op: MutationRow['op'], + payload: Record, + apply: (db: FieldStayDexie) => Promise, +): Promise { + if (isDexieShutdown(userId)) return + const db = getDexieDb(userId) + + // Array form: Dexie's variadic transaction() overloads stop at five tables, + // and every store a helper might touch has to be in scope up front — an IDB + // transaction cannot widen its scope once it has started. + await db.transaction('rw', [ + db.mutations, + db.turnovers, + db.checklist_instances, + db.checklist_instance_items, + db.inventory_items, + db.crew_availability, + db.crew_work_orders, + db.property_assets, + db.sync_meta, + ], async () => { + await apply(db) + await enqueueMutationTx(db, table, targetId, op, payload) + }) + + void getSyncEngine(userId).processOutbox() +} export interface UpdateChecklistItemInput { isCompleted: boolean @@ -43,11 +92,11 @@ export async function updateChecklistItem( if (input.crewNotes !== undefined) changes.crew_notes = input.crewNotes if (input.photoStoragePath !== undefined) changes.photo_storage_path = input.photoStoragePath - await db.checklist_instance_items.update(itemId, changes) - await enqueueMutation(userId, 'checklist_instance_items', itemId, 'PATCH', changes) - // enqueueMutation already fires processOutbox() in the background — - // intentionally not awaited here so the caller returns as soon as the - // local write lands. + await writeAndQueue(userId, 'checklist_instance_items', itemId, 'PATCH', changes, (db) => + db.checklist_instance_items.update(itemId, changes), + ) + // writeAndQueue fires processOutbox() in the background — intentionally not + // awaited, so the caller returns as soon as the local write lands. } /** @@ -66,15 +115,14 @@ export async function confirmChecklistComplete( crewMemberId: string, confirmed: boolean, ): Promise { - const db = getDexieDb(userId) - const changes: Record = { completed_at: confirmed ? new Date().toISOString() : null, completed_by_crew_id: confirmed ? crewMemberId : '', } - await db.checklist_instances.update(instanceId, changes) - await enqueueMutation(userId, 'checklist_instances', instanceId, 'PATCH', changes) + await writeAndQueue(userId, 'checklist_instances', instanceId, 'PATCH', changes, (db) => + db.checklist_instances.update(instanceId, changes), + ) } /** @@ -89,15 +137,14 @@ export async function confirmInventoryComplete( crewMemberId: string, confirmed: boolean, ): Promise { - const db = getDexieDb(userId) - const changes: Record = { inventory_confirmed_complete_at: confirmed ? new Date().toISOString() : null, inventory_confirmed_by_crew_id: confirmed ? crewMemberId : '', } - await db.turnovers.update(turnoverId, changes) - await enqueueMutation(userId, 'turnovers', turnoverId, 'PATCH', changes) + await writeAndQueue(userId, 'turnovers', turnoverId, 'PATCH', changes, (db) => + db.turnovers.update(turnoverId, changes), + ) } /** @@ -112,13 +159,12 @@ export async function acknowledgeDatesChanged( userId: string, turnoverId: string, ): Promise { - const db = getDexieDb(userId) const acknowledgedAt = new Date().toISOString() + const changes = { dates_change_acknowledged_at: acknowledgedAt } - await db.turnovers.update(turnoverId, { dates_change_acknowledged_at: acknowledgedAt }) - await enqueueMutation(userId, 'turnovers', turnoverId, 'PATCH', { - dates_change_acknowledged_at: acknowledgedAt, - }) + await writeAndQueue(userId, 'turnovers', turnoverId, 'PATCH', changes, (db) => + db.turnovers.update(turnoverId, changes), + ) } /** @@ -144,7 +190,7 @@ export async function retryFailedMutation( for (const mutation of failed) { await db.mutations.update(mutation.id!, { - failed: false, + failed: 0, retryCount: 0, networkRetryCount: 0, // 0 is unconditionally in the past, so the row is immediately due — @@ -171,11 +217,12 @@ export async function retryAllFailedMutations(userId: string): Promise { // remaining sequence so a retry re-applies it in the order the crew member // performed it. Clearing the flags in an unspecified order would leave that // sequence intact but re-queue it non-deterministically. - const failed = (await db.mutations.orderBy('id').toArray()).filter((m) => !!m.failed) + const failed = (await db.mutations.where('failed').equals(1).toArray()) + .sort((a, b) => (a.id as number) - (b.id as number)) for (const mutation of failed) { await db.mutations.update(mutation.id!, { - failed: false, + failed: 0, retryCount: 0, networkRetryCount: 0, nextAttemptAt: 0, @@ -194,7 +241,14 @@ export async function retryAllFailedMutations(userId: string): Promise { */ export async function discardFailedMutation(userId: string, mutationId: number): Promise { const db = getDexieDb(userId) + const mutation = await db.mutations.get(mutationId) await db.mutations.delete(mutationId) + + // Abandoning the write hands authority back to the server — but the pull + // that would fetch the server's value has already moved its cursor past + // that row (see invalidateCursorsFor). Without this rewind the local cache + // stays pinned to a value the server never accepted, forever and silently. + if (mutation) await invalidateCursorsFor(userId, mutation.table) } /** @@ -207,13 +261,12 @@ export async function discardFailedMutation(userId: string, mutationId: number): * not lose or corrupt anything. */ export async function markInventoryStarted(userId: string, turnoverId: string): Promise { - const db = getDexieDb(userId) const startedAt = new Date().toISOString() + const changes = { inventory_started_at: startedAt } - await db.turnovers.update(turnoverId, { inventory_started_at: startedAt }) - await enqueueMutation(userId, 'turnovers', turnoverId, 'PATCH', { - inventory_started_at: startedAt, - }) + await writeAndQueue(userId, 'turnovers', turnoverId, 'PATCH', changes, (db) => + db.turnovers.update(turnoverId, changes), + ) } /** @@ -222,13 +275,9 @@ export async function markInventoryStarted(userId: string, turnoverId: string): * is set authoritatively by the server, not the client clock. */ export async function startTurnover(userId: string, turnoverId: string): Promise { - const db = getDexieDb(userId) - - await db.turnovers.update(turnoverId, { status: 'in_progress' }) - - await enqueueMutation(userId, 'turnovers', turnoverId, 'PATCH', { - status: 'in_progress', - }) + await writeAndQueue(userId, 'turnovers', turnoverId, 'PATCH', { status: 'in_progress' }, (db) => + db.turnovers.update(turnoverId, { status: 'in_progress' }), + ) } /** @@ -238,13 +287,9 @@ export async function startTurnover(userId: string, turnoverId: string): Promise * pipeline fires for crew completions. */ export async function completeTurnover(userId: string, turnoverId: string): Promise { - const db = getDexieDb(userId) - - await db.turnovers.update(turnoverId, { status: 'completed' }) - - await enqueueMutation(userId, 'turnovers', turnoverId, 'PATCH', { - status: 'completed', - }) + await writeAndQueue(userId, 'turnovers', turnoverId, 'PATCH', { status: 'completed' }, (db) => + db.turnovers.update(turnoverId, { status: 'completed' }), + ) } /** @@ -259,14 +304,11 @@ export async function completeWorkOrder( workOrderId: string, notes: string, ): Promise { - const db = getDexieDb(userId) - - await db.crew_work_orders.update(workOrderId, { status: 'completed' }) - - await enqueueMutation(userId, 'crew_work_orders', workOrderId, 'PATCH', { - status: 'completed', - notes, - }) + await writeAndQueue( + userId, 'crew_work_orders', workOrderId, 'PATCH', + { status: 'completed', notes }, + (db) => db.crew_work_orders.update(workOrderId, { status: 'completed' }), + ) } /** Updates an inventory item's on-hand quantity locally and queues the mutation. */ @@ -275,13 +317,11 @@ export async function updateInventoryQuantity( itemId: string, currentQuantity: number, ): Promise { - const db = getDexieDb(userId) - - await db.inventory_items.update(itemId, { current_quantity: currentQuantity }) + const changes = { current_quantity: currentQuantity } - await enqueueMutation(userId, 'inventory_items', itemId, 'PATCH', { - current_quantity: currentQuantity, - }) + await writeAndQueue(userId, 'inventory_items', itemId, 'PATCH', changes, (db) => + db.inventory_items.update(itemId, changes), + ) } // ── Crew inventory count (PM-reviewed draft) ────────────────────────────── @@ -345,16 +385,22 @@ export async function submitInventoryCountDraft( propertyId: string, draft: InventoryCountDraft, ): Promise { - const db = getDexieDb(userId) const draftId = crypto.randomUUID() - - await enqueueMutation(userId, 'inventory_count_drafts', draftId, 'PUT', { + const payload = { property_id: propertyId, counts: draft.counts, item_notes: draft.itemNotes, notes: draft.notes, - }) - await db.sync_meta.delete(inventoryDraftKey(propertyId)) + } + + // Queueing the submission and clearing the local staging row must commit + // together. As two transactions, a crash in between left BOTH a queued + // submission and a staged draft — the crew member resubmits, gets a second + // crypto.randomUUID() draft id (so the route's primary-key idempotency can't + // see the collision), and the PM reviews the same count twice. + await writeAndQueue(userId, 'inventory_count_drafts', draftId, 'PUT', payload, (db) => + db.sync_meta.delete(inventoryDraftKey(propertyId)), + ) } /** @@ -367,9 +413,9 @@ export async function submitTurnoverSummaryNotes( turnoverId: string, notes: string, ): Promise { - const db = getDexieDb(userId) - await db.turnovers.update(turnoverId, { completion_notes: notes }) - await enqueueMutation(userId, 'turnovers', turnoverId, 'PATCH', { completion_notes: notes }) + await writeAndQueue(userId, 'turnovers', turnoverId, 'PATCH', { completion_notes: notes }, (db) => + db.turnovers.update(turnoverId, { completion_notes: notes }), + ) } /** @@ -416,40 +462,42 @@ export async function saveCrewAvailability( notes: string | null }, ): Promise { - const db = getDexieDb(userId) const isAvailable = params.isAvailable ? 1 : 0 if (params.id) { - await db.crew_availability.update(params.id, { - is_available: isAvailable, - notes: params.notes ?? '', - }) - await enqueueMutation(userId, 'crew_availability', params.id, 'PATCH', { - is_available: isAvailable, - notes: params.notes, - }) + const existingId = params.id + await writeAndQueue( + userId, 'crew_availability', existingId, 'PATCH', + { is_available: isAvailable, notes: params.notes }, + (db) => db.crew_availability.update(existingId, { + is_available: isAvailable, + notes: params.notes ?? '', + }), + ) return } const id = crypto.randomUUID() const createdAt = new Date().toISOString() - await db.crew_availability.add({ - id, - org_id: params.orgId, - crew_member_id: params.crewMemberId, - available_date: params.date, - is_available: isAvailable, - notes: params.notes ?? '', - created_at: createdAt, - }) - - await enqueueMutation(userId, 'crew_availability', id, 'PUT', { - org_id: params.orgId, - crew_member_id: params.crewMemberId, - available_date: params.date, - is_available: isAvailable, - notes: params.notes, - created_at: createdAt, - }) + await writeAndQueue( + userId, 'crew_availability', id, 'PUT', + { + org_id: params.orgId, + crew_member_id: params.crewMemberId, + available_date: params.date, + is_available: isAvailable, + notes: params.notes, + created_at: createdAt, + }, + (db) => db.crew_availability.add({ + id, + org_id: params.orgId, + crew_member_id: params.crewMemberId, + available_date: params.date, + is_available: isAvailable, + notes: params.notes ?? '', + created_at: createdAt, + }), + ) } diff --git a/lib/dexie/photo-queue.ts b/lib/dexie/photo-queue.ts index 40fe5a1b..42181e97 100644 --- a/lib/dexie/photo-queue.ts +++ b/lib/dexie/photo-queue.ts @@ -91,6 +91,28 @@ export async function getPendingPhotoBlob(userId: string, key: string): Promise< return result } +/** + * Every blob key currently held for this user. + * + * Exists so lib/dexie/prune.ts can find blobs nothing references any more. + * This store is a SEPARATE IndexedDB database from the crew cache that holds + * the `pending_photo_uploads` tracking rows, so a blob and its row can never + * be written atomically: if the row write throws (quota) or the PWA is + * reclaimed between the two, the blob is stranded with nothing pointing at it. + * Nothing collected those, and at multiple MB each they push the origin toward + * eviction of the entire offline cache — including the mutation outbox. + */ +export async function listPendingPhotoBlobKeys(userId: string): Promise { + const db = await openDb(userId) + const keys = await new Promise((resolve, reject) => { + const req = db.transaction(STORE_NAME, 'readonly').objectStore(STORE_NAME).getAllKeys() + req.onsuccess = () => resolve(req.result as string[]) + req.onerror = () => reject(req.error) + }) + db.close() + return keys +} + export async function deletePendingPhotoBlob(userId: string, key: string): Promise { const db = await openDb(userId) await new Promise((resolve, reject) => { diff --git a/lib/dexie/photo-sync.ts b/lib/dexie/photo-sync.ts index 9d887b8a..2444d9a3 100644 --- a/lib/dexie/photo-sync.ts +++ b/lib/dexie/photo-sync.ts @@ -22,7 +22,7 @@ import type { SupabaseClient } from '@supabase/supabase-js' import { getDexieDb, isDexieShutdown, type PendingPhotoUploadRow } from './schema' -import { computeNextAttemptAt, enqueueMutation } from './syncService' +import { computeNextAttemptAt, enqueueMutationTx, getSyncEngine } from './syncService' import { getPendingPhotoBlob, deletePendingPhotoBlob } from './photo-queue' import { isOnline, withTabLock, classifyUploadFailure, UploadDataError } from './net' import { hasAnyOrgPrefix, orgScopedStoragePath } from '../storage/object-path' @@ -84,12 +84,12 @@ export async function retryFailedPhotoUploads( targetId?: string, ): Promise { const db = getDexieDb(userId) - const failed = (await db.pending_photo_uploads.toArray()) - .filter((row) => row.failed && (targetId === undefined || row.target_id === targetId)) + const failed = (await db.pending_photo_uploads.where('failed').equals(1).toArray()) + .filter((row) => targetId === undefined || row.target_id === targetId) for (const row of failed) { await db.pending_photo_uploads.update(row.id, { - failed: false, + failed: 0, retry_count: 0, network_retry_count: 0, next_attempt_at: 0, @@ -100,37 +100,49 @@ export async function retryFailedPhotoUploads( void processPendingPhotoUploads(supabase, userId) } +/** + * Writes the uploaded path into the local cache AND queues it for the server, + * in one transaction — same atomicity requirement as every helper in + * lib/dexie/helpers.ts. As two transactions, an app kill between them left the + * cache showing a photo the server would never learn about, with no outbox row + * and so nothing in the failed-sync surface. + */ async function applyUploadedPath( userId: string, row: PendingPhotoUploadRow, ): Promise { const db = getDexieDb(userId) - if (row.target_table === 'checklist_instance_items') { - await db.checklist_instance_items.update(row.target_id, { photo_storage_path: row.storage_path }) - await enqueueMutation(userId, 'checklist_instance_items', row.target_id, 'PATCH', { - photo_storage_path: row.storage_path, - }) - return - } + await db.transaction('rw', db.mutations, db.checklist_instance_items, + db.checklist_instances, db.property_assets, async () => { + if (row.target_table === 'checklist_instance_items') { + await db.checklist_instance_items.update(row.target_id, { photo_storage_path: row.storage_path }) + await enqueueMutationTx(db, 'checklist_instance_items', row.target_id, 'PATCH', { + photo_storage_path: row.storage_path, + }) + return + } - if (row.target_table === 'checklist_instances') { - await db.checklist_instances.update(row.target_id, { section_photo_path: row.storage_path ?? '' }) - await enqueueMutation(userId, 'checklist_instances', row.target_id, 'PATCH', { - section_photo_path: row.storage_path, + if (row.target_table === 'checklist_instances') { + await db.checklist_instances.update(row.target_id, { section_photo_path: row.storage_path ?? '' }) + await enqueueMutationTx(db, 'checklist_instances', row.target_id, 'PATCH', { + section_photo_path: row.storage_path, + }) + return + } + + // property_assets.photo_url stores the BARE object key, same as the two + // targets above. It used to hold a getPublicUrl() result, but turnover-photos + // is a private bucket now: a public URL 400s and a signed one expires, so the + // stable key is what gets persisted and readers sign it on demand. + await db.property_assets.update(row.target_id, { photo_url: row.storage_path! }) + await enqueueMutationTx(db, 'property_assets', row.target_id, 'PATCH', { + photo_url: row.storage_path, + scanRequest: { storagePath: row.storage_path, mediaType: 'image/jpeg' }, + }) }) - return - } - // property_assets.photo_url stores the BARE object key, same as the two - // targets above. It used to hold a getPublicUrl() result, but turnover-photos - // is a private bucket now: a public URL 400s and a signed one expires, so the - // stable key is what gets persisted and readers sign it on demand. - await db.property_assets.update(row.target_id, { photo_url: row.storage_path! }) - await enqueueMutation(userId, 'property_assets', row.target_id, 'PATCH', { - photo_url: row.storage_path, - scanRequest: { storagePath: row.storage_path, mediaType: 'image/jpeg' }, - }) + void getSyncEngine(userId).processOutbox() } /** @@ -220,7 +232,7 @@ async function recordPhotoFailure(userId: string, row: PendingPhotoUploadRow, er // eventually collects one the crew never acts on. await db.pending_photo_uploads.update(row.id, { retry_count: retryCount, - failed: true, + failed: 1, last_error: message, }) return @@ -272,8 +284,16 @@ async function drainPhotoQueue(supabase: SupabaseClient, userId: string): Promis const blob = await getPendingPhotoBlob(userId, row.local_blob_key) if (!blob) { - // Blob missing (cleared browser storage, etc.) — nothing to upload - await db.pending_photo_uploads.delete(row.id) + // The bytes are gone (storage cleared, evicted under quota pressure, or + // the blob write never landed) — so this photo can never be sent. Deleting + // the row outright, as this did, made that indistinguishable from a + // successful upload: the crew member's photo silently ceased to exist with + // nothing anywhere saying so. Dead-letter it instead, so it lands on the + // failed-sync surface where they can at least see it and retake it. + await db.pending_photo_uploads.update(row.id, { + failed: 1, + last_error: 'Photo data was cleared from this device', + }) continue } diff --git a/lib/dexie/prune.ts b/lib/dexie/prune.ts index 97532efb..3195af74 100644 --- a/lib/dexie/prune.ts +++ b/lib/dexie/prune.ts @@ -16,8 +16,9 @@ // DEAD_LETTER_RETENTION_DAYS, by which point the crew member has had every // opportunity to retry or discard them. -import { getDexieDb } from './schema' -import { deletePendingPhotoBlob } from './photo-queue' +import { getDexieDb, type FieldStayDexie } from './schema' +import { deletePendingPhotoBlob, listPendingPhotoBlobKeys } from './photo-queue' +import { invalidateCursorsFor } from './sync/cursors' import { MESSAGE_WINDOW_DAYS } from './sync/messages' /** Matches syncCrewAvailability's own 30-day lookback. */ @@ -81,6 +82,71 @@ export async function pruneLocalCache(userId: string): Promise { ) await pruneExpiredDeadLetters(userId) + await pruneOrphanPhotoBlobs(userId) +} + +/** sync_meta key holding last sweep's unreferenced-but-not-yet-collected blob keys. */ +const ORPHAN_CANDIDATES_KEY = 'photo_blob_orphan_candidates' + +async function readOrphanCandidates(db: FieldStayDexie): Promise> { + const row = await db.sync_meta.get(ORPHAN_CANDIDATES_KEY) + if (!row?.value) return new Set() + try { + const parsed: unknown = JSON.parse(row.value) + return new Set(Array.isArray(parsed) ? (parsed as string[]) : []) + } catch { + return new Set() + } +} + +/** + * Collects photo blobs no `pending_photo_uploads` row references. + * + * The blob bytes live in a SEPARATE IndexedDB database from the tracking row + * (see lib/dexie/photo-queue.ts), so the two can never be written atomically: + * a quota error on the row write, or the PWA being reclaimed between the two, + * strands the blob with nothing pointing at it. Nothing collected those — + * pruneExpiredDeadLetters only ever deletes blobs a row still names — so on a + * device that stays logged in they accumulate at multiple MB each until the + * browser evicts the whole origin, taking the mutation outbox with it. + * + * Two-generation rule: a key is only collected if it was ALSO unreferenced on + * the previous sweep. Sweeps run at most every safety-poll interval, so that + * is minutes of margin against deleting a blob whose row is still mid-enqueue + * — and it needs no timestamp in the key, which the key format is not + * obliged to carry. + */ +export async function pruneOrphanPhotoBlobs(userId: string): Promise { + const db = getDexieDb(userId) + + let keys: string[] + try { + keys = await listPendingPhotoBlobKeys(userId) + } catch (err) { + // Blob-store GC is never worth failing a resync over. + console.warn('[prune] could not enumerate photo blobs (non-fatal):', err) + return + } + + const referenced = new Set((await db.pending_photo_uploads.toArray()).map((p) => p.local_blob_key)) + const unreferenced = keys.filter((key) => !referenced.has(key)) + + const priorCandidates = await readOrphanCandidates(db) + const collectable = unreferenced.filter((key) => priorCandidates.has(key)) + + for (const key of collectable) { + try { + await deletePendingPhotoBlob(userId, key) + } catch (err) { + console.warn('[prune] failed to delete orphaned photo blob:', err) + } + } + + // Carry forward only the keys seen unreferenced for the FIRST time. + await db.sync_meta.put({ + key: ORPHAN_CANDIDATES_KEY, + value: JSON.stringify(unreferenced.filter((key) => !priorCandidates.has(key))), + }) } /** @@ -92,14 +158,20 @@ export async function pruneExpiredDeadLetters(userId: string): Promise { const db = getDexieDb(userId) const horizon = daysAgoIso(DEAD_LETTER_RETENTION_DAYS) - const staleMutations = (await db.mutations.toArray()) - .filter((m) => m.failed && m.createdAt < horizon) + const staleMutations = (await db.mutations.where('failed').equals(1).toArray()) + .filter((m) => m.createdAt < horizon) for (const mutation of staleMutations) { await db.mutations.delete(mutation.id as number) + // Same reasoning as discardFailedMutation(): while this row existed, + // shadowPendingMutations() replayed it over every pull AND the cursor + // advanced past the server row it masked. Dropping it here — with no user + // action at all — would otherwise leave the cache pinned to a value the + // server never accepted, with no path back short of logout. + await invalidateCursorsFor(userId, mutation.table) } - const stalePhotos = (await db.pending_photo_uploads.toArray()) - .filter((p) => p.failed && p.created_at < horizon) + const stalePhotos = (await db.pending_photo_uploads.where('failed').equals(1).toArray()) + .filter((p) => p.created_at < horizon) for (const photo of stalePhotos) { await db.pending_photo_uploads.delete(photo.id) try { diff --git a/lib/dexie/schema.ts b/lib/dexie/schema.ts index 1e34ff8a..f3020219 100644 --- a/lib/dexie/schema.ts +++ b/lib/dexie/schema.ts @@ -140,10 +140,26 @@ export interface PendingPhotoUploadRow { // UI anywhere saying so. next_attempt_at?: number network_retry_count?: number - failed?: boolean + /** 0/1, not a boolean — see the note on MutationRow.failed. */ + failed?: DeadLetterFlag last_error?: string } +/** + * Dead-letter marker, stored as 0/1 rather than a boolean. + * + * IndexedDB has no boolean key type: a record whose indexed property holds + * `true` is simply omitted from that index, so `failed` could never be + * indexed while it was a boolean and every dead-letter query — including the + * three `useLiveQuery`s FailedSyncBanner keeps live on every crew screen — + * had to full-scan the outbox on every single write to it. + * + * 0/1 preserves every existing truthiness check (`!m.failed`, `!!m.failed`) + * unchanged; only the literal `true`/`false` writes moved. Version 9's + * upgrade normalizes rows written before this. + */ +export type DeadLetterFlag = 0 | 1 + // Tracks incremental-sync watermarks (e.g. the last `turnover_assignments.created_at` // pulled from Supabase), so initialSync can fetch only what changed since last time // instead of re-pulling everything whenever the local cache is already populated. @@ -195,7 +211,14 @@ export interface MutationRow { // never made it to the server. Keeping the row (excluded from the pending // queue) lets the UI surface "this didn't sync" instead of silently // discarding it. - failed?: boolean + // + // 0/1 rather than boolean so it can actually be indexed — see DeadLetterFlag. + failed?: DeadLetterFlag + // Shape version of `payload`, stamped at enqueue time. An outbox row can + // outlive the release that queued it (a device offline across a deploy), so + // the drain migrates an older payload forward rather than replaying a shape + // the current upload handler no longer understands. Absent ⇒ version 1. + payloadVersion?: number // 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 @@ -341,6 +364,41 @@ export class FieldStayDexie extends Dexie { crew_work_orders: 'id, property_id, org_id, status, scheduled_date', property_assets: 'id, property_id, org_id, asset_type', }) + + // Index correction (2026-08-04 offline-sync audit). Only the three changed + // stores are declared — Dexie carries every other store forward unchanged. + // + // - `mutations.failed` / `pending_photo_uploads.failed`: the predicate of + // every dead-letter query in the app, previously unindexed AND + // unindexABLE (booleans are not valid IndexedDB keys). FailedSyncBanner + // keeps three of those queries live on every crew screen, so each one + // full-scanned the outbox on every checklist tick and every drain step. + // - `[table+targetId]`: the per-record lookup enqueueMutation() and + // holdBackSuccessors() both do. Both ran as full scans; the former now + // runs on EVERY crew write. + // - `pending_photo_uploads.retry_count`: dropped. Nothing has ever queried + // it — it cost an index write per attempt and bought nothing. + // - `checklist_instance_items.is_completed`: dropped. Never queried by + // index either, and a two-value column is close to useless as one + // while costing a write on the highest-volume mutation in the app. + this.version(9) + .stores({ + mutations: '++id, table, targetId, failed, [table+targetId]', + pending_photo_uploads: 'id, target_id, target_table, failed', + checklist_instance_items: 'id, instance_id, turnover_id', + }) + .upgrade((tx) => + // Normalize the pre-existing boolean flags to the 0/1 the index needs. + // Rows written as `failed: true` are invisible to `.where('failed')` + // until this runs — which on a device that dead-lettered work while + // offline is exactly the row the crew member most needs to see. + Promise.all([ + tx.table('mutations').toCollection() + .modify((m: MutationRow) => { m.failed = m.failed ? 1 : 0 }), + tx.table('pending_photo_uploads').toCollection() + .modify((p: PendingPhotoUploadRow) => { p.failed = p.failed ? 1 : 0 }), + ]).then(() => undefined), + ) } } @@ -446,6 +504,103 @@ export function getDexieDb(userId: string): FieldStayDexie { return db } +// ── Cross-tab logout ────────────────────────────────────────────────────── +// +// The shutdown latch above is per-DOCUMENT module state, but IndexedDB is a +// per-ORIGIN resource. With a second crew tab open (an office tablet, a +// turnover opened in a new tab) logging out in tab A used to fail two ways at +// once: +// +// 1. `indexedDB.deleteDatabase` fires `blocked` and WAITS while any other +// connection is open. Dexie's default blocked handler warns and keeps +// waiting, so `await Dexie.delete(...)` never resolved — and because it +// sits before `supabase.auth.signOut()` and the redirect in +// performLogout(), the user stayed signed in, on the crew screen, with the +// logout button already re-enabled by its own `finally`. Silent no-op. +// 2. Tab B's own `shutdownUserIds` was never latched, so it kept draining and +// its next getDexieDb() would re-create the database — leaving a +// signed-out user's work on a shared device, the exact thing the latch +// exists to prevent. +// +// So: tell the other tabs first (they latch, close their connection, and +// leave), and bound the delete so a tab that ignores us can never strand the +// user mid-logout. +const LOGOUT_CHANNEL = 'fieldstay-crew-logout' + +/** How long to wait for other tabs to release the database before giving up. */ +const DELETE_BLOCKED_TIMEOUT_MS = 3_000 + +interface ShutdownMessage { type: 'shutdown'; userId: string } + +function broadcastShutdown(userId: string): void { + if (typeof BroadcastChannel === 'undefined') return + try { + const channel = new BroadcastChannel(LOGOUT_CHANNEL) + channel.postMessage({ type: 'shutdown', userId } satisfies ShutdownMessage) + channel.close() + } catch (err) { + // Never let a messaging failure block the logout it is meant to assist. + console.warn('[Dexie] logout broadcast failed (non-fatal):', err) + } +} + +/** + * Subscribes this document to logout broadcasts from sibling tabs. Installed + * once per session by DexieProvider; the returned function unsubscribes. + * + * `onShutdown` is how the UI leaves the crew surface — a tab still rendering + * cached assignments for a user who just signed out on another tab is the + * same shared-device leak, just on screen instead of on disk. + */ +export function listenForRemoteShutdown(userId: string, onShutdown: () => void): () => void { + if (typeof BroadcastChannel === 'undefined') return () => {} + + let channel: BroadcastChannel + try { + channel = new BroadcastChannel(LOGOUT_CHANNEL) + } catch { + return () => {} + } + + channel.onmessage = (event: MessageEvent) => { + if (event.data?.type !== 'shutdown' || event.data.userId !== userId) return + // Latch BEFORE closing: anything mid-await here must not re-open storage. + markDexieShutdown(userId) + if (db && dbUserId === userId) { + db.close() // release the connection so the deleting tab unblocks + db = null + dbUserId = null + } + onShutdown() + } + + return () => channel.close() +} + +/** + * Deletes a database, giving up rather than waiting indefinitely on a + * connection another tab refuses to release. Losing the delete is recoverable + * — the shutdown latch already blocks every read, and cleanupStaleDexieDbs() + * collects the residue on the next login — whereas hanging here strands the + * user in a half-signed-out state, which is not. + */ +async function deleteDbBounded(name: string): Promise { + let timer: ReturnType | undefined + try { + await Promise.race([ + Dexie.delete(name), + new Promise((_resolve, reject) => { + timer = setTimeout( + () => reject(new Error(`IndexedDB delete of ${name} blocked by another connection`)), + DELETE_BLOCKED_TIMEOUT_MS, + ) + }), + ]) + } finally { + if (timer !== undefined) clearTimeout(timer) + } +} + export async function closeDexieDb(): Promise { if (db) { const dbName = db.name @@ -453,7 +608,12 @@ export async function closeDexieDb(): Promise { // Latch first, synchronously, before the first await: a drain resumed by // the microtask queue between here and the delete below must not be able // to re-open what we are about to delete. - if (formerUserId) markDexieShutdown(formerUserId) + if (formerUserId) { + markDexieShutdown(formerUserId) + // Other tabs latch and close their connection, so the delete below has + // a chance of not being blocked in the first place. + broadcastShutdown(formerUserId) + } db.close() db = null dbUserId = null @@ -461,7 +621,7 @@ export async function closeDexieDb(): Promise { // on the device after sign-out. Crew-app data is re-synced fresh // on next login; nothing is lost that can't be re-fetched. try { - await Dexie.delete(dbName) + await deleteDbBounded(dbName) } catch (err) { console.error('[Dexie] Failed to delete DB on logout:', err) reportError(err, { site: 'lib.dexie.schema.Dexie' }) @@ -471,7 +631,7 @@ export async function closeDexieDb(): Promise { // Also delete the user-namespaced photo blob store (lib/dexie/photo-queue.ts) if (formerUserId) { try { - await Dexie.delete(`fieldstay-photo-queue-${formerUserId}`) + await deleteDbBounded(`fieldstay-photo-queue-${formerUserId}`) } catch (err) { console.error('[Dexie] Failed to delete photo blob store on logout:', err) reportError(err, { site: 'lib.dexie.schema.Dexie' }) diff --git a/lib/dexie/sync/cursors.ts b/lib/dexie/sync/cursors.ts index e26ac7f0..999eaa2c 100644 --- a/lib/dexie/sync/cursors.ts +++ b/lib/dexie/sync/cursors.ts @@ -19,7 +19,7 @@ // id-set pulls), so a conservative or missing cursor can only cost // bandwidth, never correctness. -import { getDexieDb } from '../schema' +import { getDexieDb, type MutationTable } from '../schema' export const CURSOR_OVERLAP_MS = 10_000 @@ -72,6 +72,60 @@ export function partitionByKnown( return { known, fresh } } +/** + * Which cursors gate the pull that would re-fetch a given mutation's table. + * Tables pulled in full every time (inventory_items, properties, + * crew_availability, property_assets) have no cursor and so need no rewind. + */ +const CURSORS_BY_MUTATION_TABLE: Readonly>> = { + turnovers: ['cursor:turnovers'], + checklist_instances: ['cursor:checklist_instances'], + checklist_instance_items: ['cursor:checklist_items'], + crew_work_orders: ['cursor:work_orders'], +} + +/** + * Rewinds the cursors guarding a table so the next pull re-fetches the + * server's authoritative row for it. + * + * Required whenever a pending mutation is ABANDONED. While it was queued, + * shadowPendingMutations() replayed it over every pulled row — and + * advanceCursor() moved past that row's updated_at at the same time. Drop the + * mutation and the overlay disappears, but the cursor does not come back: the + * delta filter `.gt('updated_at', cursor)` will never return that row again, + * and partitionByKnown() routes it down the delta path because the device + * already knows the id. The local cache is then pinned to a value the server + * never accepted, permanently and invisibly. + * + * That happens on an explicit discard AND with no user action at all, when + * pruneExpiredDeadLetters() collects a dead letter at 30 days. + */ +export async function invalidateCursorsFor(userId: string, table: MutationTable): Promise { + const keys = CURSORS_BY_MUTATION_TABLE[table] + if (!keys?.length) return + const db = getDexieDb(userId) + await Promise.all(keys.map((key) => db.sync_meta.delete(key))) +} + +/** + * Rewinds EVERY cursor, so the next resync transfers full rows rather than a + * delta. The repair path for a device whose cache has diverged from the server + * — previously there was none: `force` was plumbed through every sync function + * but never passed as `true` from anywhere, and cursors were never reset, so + * the only way out was logout, which destroys the outbox along with the cache. + */ +export async function resetAllCursors(userId: string): Promise { + const db = getDexieDb(userId) + await Promise.all(ALL_CURSOR_KEYS.map((key) => db.sync_meta.delete(key))) +} + +const ALL_CURSOR_KEYS: readonly SyncCursorKey[] = [ + 'cursor:turnovers', + 'cursor:checklist_instances', + 'cursor:checklist_items', + 'cursor:work_orders', +] + export async function getCursor(userId: string, key: SyncCursorKey): Promise { const row = await getDexieDb(userId).sync_meta.get(key) return row?.value ?? null diff --git a/lib/dexie/sync/full-resync.ts b/lib/dexie/sync/full-resync.ts index 4e57f8dd..ba0e41ef 100644 --- a/lib/dexie/sync/full-resync.ts +++ b/lib/dexie/sync/full-resync.ts @@ -17,6 +17,7 @@ import { syncWorkOrders } from './work-orders' import { syncMessages } from './messages' import { syncCrewAvailability } from './availability' import { computeAssignedPropertyIds, syncPropertyAssets } from './assets' +import { resetAllCursors } from './cursors' import { pruneLocalCache } from '../prune' /** @@ -64,3 +65,23 @@ export async function fullCrewResync( await pruneLocalCache(userId) } + +/** + * fullCrewResync with every delta cursor rewound first, so the next pull + * transfers whole rows instead of "what changed since". + * + * The repair path for a device whose cache has diverged from the server. + * There was none: `force` was plumbed through every sync function but never + * passed as `true` from anywhere in the app, and nothing ever reset a cursor — + * so once a row was masked by a local write that was later abandoned, the + * delta filter would skip it forever and the only way out was logout, which + * destroys the outbox along with the cache. + */ +export async function forceFullCrewResync( + supabase: DexieSupabaseClient, + userId: string, + crewMemberId: string, +): Promise { + await resetAllCursors(userId) + await fullCrewResync(supabase, userId, crewMemberId) +} diff --git a/lib/dexie/syncService.ts b/lib/dexie/syncService.ts index 80f50872..89f2816b 100644 --- a/lib/dexie/syncService.ts +++ b/lib/dexie/syncService.ts @@ -13,6 +13,22 @@ type DexieSupabaseClient = ReturnType const MAX_RETRIES = 5 +/** + * Shape version stamped onto every newly-queued mutation payload. Bump this + * whenever a payload's shape changes and add the matching entry to + * PAYLOAD_MIGRATIONS — a device can be offline across a deploy, so an outbox + * row routinely outlives the release that wrote it. + */ +export const OUTBOX_PAYLOAD_VERSION = 1 + +/** + * `lastError` for a mutation that never failed on its own — it is queued + * behind a dead letter for the same record and must not be pushed until that + * one is resolved. Distinct wording so the banner doesn't tell a crew member + * five separate things failed when one did. + */ +export const HELD_BACK_REASON = 'Held back so earlier changes to this item retry in order' + // 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. @@ -218,13 +234,15 @@ export class SyncEngine { mutation: MutationRow, failedId: number, ): Promise { - const successors = (await db.mutations.orderBy('id').toArray()).filter( - (m) => - !m.failed && - (m.id as number) > failedId && - m.table === mutation.table && - m.targetId === mutation.targetId, + // Index-backed per-record lookup ([table+targetId], added in schema v9) — + // this used to scan the whole outbox on every dead-letter. + const successors = ( + await db.mutations + .where('[table+targetId]').equals([mutation.table, mutation.targetId]) + .toArray() ) + .filter((m) => !m.failed && (m.id as number) > failedId) + .sort((a, b) => (a.id as number) - (b.id as number)) if (successors.length === 0) return @@ -236,8 +254,8 @@ export class SyncEngine { for (const successor of successors) { const successorId = successor.id as number await db.mutations.update(successorId, { - failed: true, - lastError: 'Held back so earlier changes to this item retry in order', + failed: 1, + lastError: HELD_BACK_REASON, }) this.heldBack.add(successorId) } @@ -290,7 +308,7 @@ export class SyncEngine { ) await db.mutations.update(id, { retryCount: newRetryCount, - failed: true, + failed: 1, lastError: describeFailure(err), }) @@ -732,6 +750,64 @@ export function disposeSyncEngine(): void { engineUserId = null } +/** + * Queues a mutation in the outbox WITHOUT kicking the drain. + * + * Exists so a caller can commit its optimistic local write and this outbox row + * in ONE Dexie transaction (see lib/dexie/helpers.ts). Those two writes used to + * be separate IndexedDB transactions with a suspend/kill window between them: + * the local row landed, the app was reclaimed (iOS backgrounding a PWA, a + * quota error, a closed tab), and the outbox row never did. The crew member + * then saw their tick as done forever while the server never heard about it, + * with nothing in the failed-sync surface because there was no mutation row to + * mark failed — and no delta pull would correct it either, since the server + * row's updated_at never changed. + * + * ⚠️ Never `await` anything non-Dexie between this and the caller's own write. + * An IndexedDB transaction auto-commits the moment control returns to the event + * loop without a pending request against it, so an interleaved fetch() (or a + * processOutbox() call, which does network I/O) makes the rest of the block + * throw TransactionInactiveError. The drain kick belongs OUTSIDE the block. + */ +export async function enqueueMutationTx( + db: FieldStayDexie, + table: MutationRow['table'], + targetId: string, + op: MutationRow['op'], + payload: Record, +): Promise { + // A record with a dead letter is FROZEN: every later write to it is queued + // already-held-back. + // + // holdBackSuccessors() only ever saw the successors that existed AT the + // moment of dead-lettering, which is the less likely half of the problem — + // the corrective edit is normally made AFTER the failure, not before it. + // Concretely: crew ticks an item (#10) → 500s five times → dead-letters, no + // successors to hold. They realise it isn't done and un-tick it (#11) → + // pushes fine, server now false. They tap Retry all → #10 keeps its original + // low id, replays is_completed = true, and the server flips BACK — then the + // next delta pull overwrites Dexie, so the un-tick disappears from the phone + // too. Same shape for inventory_items.current_quantity and crew_availability. + // + // Freezing at enqueue keeps the whole sequence intact, so Retry all replays + // tick-then-un-tick in the order the crew member actually performed them. + const frozen = await db.mutations + .where('[table+targetId]').equals([table, targetId]) + .filter((m) => m.failed === 1) + .count() + + return db.mutations.add({ + table, + targetId, + op, + payload, + createdAt: new Date().toISOString(), + retryCount: 0, + payloadVersion: OUTBOX_PAYLOAD_VERSION, + ...(frozen > 0 ? { failed: 1 as const, lastError: HELD_BACK_REASON } : {}), + }) +} + /** Queues a mutation in the outbox and fires processOutbox() in the background. */ export async function enqueueMutation( userId: string, @@ -745,14 +821,9 @@ export async function enqueueMutation( if (isDexieShutdown(userId)) return const db = getDexieDb(userId) - await db.mutations.add({ - table, - targetId, - op, - payload, - createdAt: new Date().toISOString(), - retryCount: 0, - }) + await db.transaction('rw', db.mutations, () => + enqueueMutationTx(db, table, targetId, op, payload), + ) void getSyncEngine(userId).processOutbox() } diff --git a/unit/demo/demo-readiness.test.ts b/unit/demo/demo-readiness.test.ts index 3fae02b0..9867001d 100644 --- a/unit/demo/demo-readiness.test.ts +++ b/unit/demo/demo-readiness.test.ts @@ -26,14 +26,14 @@ interface Counts { function fakeDb(c: Counts): FieldStayDexie { const table = (n: number) => ({ count: async () => n }) const mutations = { - filter: (predicate: (m: { failed?: boolean }) => boolean) => ({ - count: async () => { - // The check distinguishes pending (failed !== true) from - // dead-lettered (failed === true); resolve which one is being asked - // for by running the predicate against a representative row. - const wantsFailed = predicate({ failed: true }) - return wantsFailed ? (c.failed ?? 0) : (c.pending ?? 0) - }, + // Pending: a full-table filter on !failed. + filter: () => ({ count: async () => c.pending ?? 0 }), + // Dead-lettered: index-backed, since `failed` is stored as 0/1 (IndexedDB + // cannot index a boolean — see DeadLetterFlag in lib/dexie/schema.ts). + where: (index: string) => ({ + equals: (value: unknown) => ({ + count: async () => (index === 'failed' && value === 1 ? (c.failed ?? 0) : 0), + }), }), } diff --git a/unit/dexie/fake-dexie.ts b/unit/dexie/fake-dexie.ts index 88e335d7..82f25b82 100644 --- a/unit/dexie/fake-dexie.ts +++ b/unit/dexie/fake-dexie.ts @@ -1,10 +1,43 @@ // In-memory stand-in for the FieldStayDexie instance, covering exactly the // surface the lib/dexie/sync/* functions touch (bulkPut/bulkDelete/toArray/ -// where().anyOf().primaryKeys()/get/put). Lets the sync orchestration be -// unit-tested in the node environment without IndexedDB. +// where().anyOf().primaryKeys()/where().equals().filter().count()/get/put, +// plus db.transaction()). Lets the sync orchestration be unit-tested in the +// node environment without IndexedDB. interface FakeRow { [key: string]: unknown } +/** + * Minimal stand-in for a Dexie Collection: enough of the chain for the + * filter/count/toArray shapes the outbox and prune paths use. + */ +function fakeCollection(matches: FakeRow[], pk: string) { + const collection = { + primaryKeys: async () => matches.map((r) => r[pk]), + toArray: async () => matches, + count: async () => matches.length, + filter: (predicate: (row: never) => boolean) => + fakeCollection(matches.filter((r) => predicate(r as never)), pk), + // eslint-disable-next-line @typescript-eslint/no-explicit-any + modify: async (apply: (row: any) => void) => { for (const r of matches) apply(r) }, + } + return collection +} + +/** + * Compound-index key equality, mirroring IndexedDB's array-key semantics — + * `where('[table+targetId]').equals(['turnovers', 'x'])`. + */ +function matchesKey(row: FakeRow, fields: string[], value: unknown): boolean { + if (fields.length === 1) return row[fields[0]!] === value + const parts = Array.isArray(value) ? value : [value] + return fields.every((field, i) => row[field] === parts[i]) +} + +/** '[table+targetId]' → ['table', 'targetId']; 'failed' → ['failed']. */ +function indexFields(index: string): string[] { + return index.startsWith('[') ? index.slice(1, -1).split('+') : [index] +} + export function fakeTable(pk = 'key') { const rows = new Map() // Auto-increment counter for add() on '++id'-style outbox tables. @@ -28,22 +61,25 @@ export function fakeTable(pk = 'key') { 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()] }, + toCollection() { return fakeCollection([...rows.values()], pk) }, + filter(predicate: (row: never) => boolean) { + return fakeCollection([...rows.values()].filter((r) => predicate(r as never)), pk) + }, orderBy(field: string) { return { toArray: async () => [...rows.values()].sort((a, b) => ((a[field] as number) < (b[field] as number) ? -1 : 1)), } }, - where(field: string) { + where(index: string) { + const fields = indexFields(index) return { anyOf: (values: unknown[]) => { const wanted = new Set(values) - const matches = [...rows.values()].filter((r) => wanted.has(r[field])) - return { - primaryKeys: async () => matches.map((r) => r[pk]), - toArray: async () => matches, - } + return fakeCollection([...rows.values()].filter((r) => wanted.has(r[fields[0]!])), pk) }, + equals: (value: unknown) => + fakeCollection([...rows.values()].filter((r) => matchesKey(r, fields, value)), pk), } }, } @@ -51,6 +87,16 @@ export function fakeTable(pk = 'key') { export function makeFakeDexieDb() { return { + // Dexie's transaction() runs its callback immediately and resolves with the + // result. The in-memory double is inherently atomic (nothing can interleave + // between two synchronous Map writes), so it only needs to invoke the body + // — what the real thing buys us is durability across an app kill, which is + // not something a fake can model. The tests that matter here assert that + // BOTH writes are inside one transaction() call, not that a rollback works. + transaction: (_mode: string, ...args: unknown[]): Promise => { + const body = args[args.length - 1] as () => Promise + return Promise.resolve(body()) + }, turnovers: fakeTable('id'), checklist_instances: fakeTable('id'), checklist_instance_items: fakeTable('id'), diff --git a/unit/dexie/offline-write-durability.test.ts b/unit/dexie/offline-write-durability.test.ts new file mode 100644 index 00000000..39afa504 --- /dev/null +++ b/unit/dexie/offline-write-durability.test.ts @@ -0,0 +1,304 @@ +// The 2026-08-04 offline-sync audit's data-loss set, run against real +// (fake-indexeddb) IndexedDB rather than the in-memory double used elsewhere +// in this directory — every property here is about what actually survives on +// the device, which a Map cannot model. +// +// F1 — the optimistic local write and its outbox row were two separate +// IndexedDB transactions. A PWA reclaimed between them (iOS +// backgrounding, quota, a closed tab) left the cache updated with +// nothing queued to send it: the crew member saw their tick as saved +// forever, the server never heard about it, the failed-sync banner had +// no row to show, and no delta pull would correct it either because the +// server row's updated_at never changed. +// +// F2 — holdBackSuccessors() only ever saw successors that existed AT the +// moment of dead-lettering, which is the less likely half of the +// problem: the corrective edit is normally made AFTER the failure. +// Tick → dead-letter → un-tick (pushes fine) → "Retry all" replays the +// stale tick on top and the server flips back. +// +// F3 — logout with a second tab open. The shutdown latch is per-DOCUMENT +// module state but IndexedDB is per-ORIGIN, so the sibling tab kept its +// connection open, `Dexie.delete` blocked on it indefinitely, and the +// await before signOut()/redirect never resolved. Logout silently did +// nothing and the crew cache stayed on a shared device. +// +// F4 — discarding a dead letter removed the shadow overlay but not the +// cursor that had advanced past the server row it was masking, pinning +// the cache to a value the server never accepted, permanently. +// +// F5 — photo blobs live in a SEPARATE IndexedDB from their tracking rows, so +// the two can never be written atomically; nothing ever collected a blob +// whose row never landed. + +import 'fake-indexeddb/auto' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +vi.mock('@/lib/supabase/client', () => ({ + createClient: () => ({ from: () => ({ update: () => ({ eq: () => ({ select: () => Promise.resolve({ data: [], error: null }) }) }) }) }), +})) + +import { + getDexieDb, + closeDexieDb, + resumeDexieDb, + isDexieShutdown, + listenForRemoteShutdown, + type MutationRow, +} from '@/lib/dexie/schema' +import { enqueueMutationTx, HELD_BACK_REASON } from '@/lib/dexie/syncService' +import { updateChecklistItem, discardFailedMutation } from '@/lib/dexie/helpers' +import { savePendingPhotoBlob, listPendingPhotoBlobKeys, getPendingPhotoBlob } from '@/lib/dexie/photo-queue' +import { pruneOrphanPhotoBlobs } from '@/lib/dexie/prune' + +const USER = '22222222-2222-4222-8222-222222222222' +const ITEM = 'item-1' + +async function seedItem(): Promise { + await getDexieDb(USER).checklist_instance_items.put({ + id: ITEM, instance_id: 'inst-1', turnover_id: 't-1', section_name: 'Kitchen', + task: 'Wipe counters', is_completed: 0, completed_at: null, completed_by_crew_id: '', + requires_photo: 0, photo_reason: '', photo_storage_path: null, crew_notes: '', + sort_order: 1, is_section_final_item: 0, + }) +} + +async function mutations(): Promise { + return getDexieDb(USER).mutations.orderBy('id').toArray() +} + +beforeEach(() => { + resumeDexieDb(USER) + vi.spyOn(console, 'warn').mockImplementation(() => {}) + vi.spyOn(console, 'error').mockImplementation(() => {}) +}) + +afterEach(async () => { + await closeDexieDb() + resumeDexieDb(USER) + vi.restoreAllMocks() +}) + +describe('F1 — the local write and its outbox row commit atomically', () => { + it('lands both on success', async () => { + await seedItem() + await updateChecklistItem(USER, ITEM, { isCompleted: true }, 'crew-1') + + const item = await getDexieDb(USER).checklist_instance_items.get(ITEM) + expect(item?.is_completed, 'the cache reflects the tick immediately').toBe(1) + + const queued = await mutations() + expect(queued).toHaveLength(1) + expect(queued[0]).toMatchObject({ table: 'checklist_instance_items', targetId: ITEM, op: 'PATCH' }) + }) + + it('rolls the cache write BACK when the outbox row cannot be written', async () => { + await seedItem() + const db = getDexieDb(USER) + + // Stand in for the real failure mode — quota exhaustion, or the app being + // killed before the second transaction commits. As two transactions the + // cache write had already committed and there was nothing left to undo it. + const boom = () => { throw new Error('simulated outbox write failure') } + db.mutations.hook('creating', boom) + + await expect(updateChecklistItem(USER, ITEM, { isCompleted: true }, 'crew-1')).rejects.toThrow() + db.mutations.hook('creating').unsubscribe(boom) + + const item = await db.checklist_instance_items.get(ITEM) + expect( + item?.is_completed, + 'a tick with no outbox row is silent data loss — the crew member sees it saved, the server never hears about it', + ).toBe(0) + expect(await mutations()).toHaveLength(0) + }) +}) + +describe('F2 — a record with a dead letter is frozen', () => { + it('queues a LATER write to the same record already held back', async () => { + const db = getDexieDb(USER) + + // The tick dead-lettered on a previous run; no successors existed then, so + // holdBackSuccessors() had nothing to hold. + await db.mutations.add({ + table: 'checklist_instance_items', targetId: ITEM, op: 'PATCH', + payload: { is_completed: 1 }, createdAt: new Date().toISOString(), + retryCount: 5, failed: 1, lastError: 'Server rejected the request (500)', + }) + + // The crew member now un-ticks it. Previously this queued clean, drained, + // and left the dead letter behind to be replayed on top by "Retry all". + await db.transaction('rw', db.mutations, () => + enqueueMutationTx(db, 'checklist_instance_items', ITEM, 'PATCH', { is_completed: 0 }), + ) + + const [, untick] = await mutations() + expect( + untick?.failed, + 'a write queued behind a dead letter must not drain ahead of it — otherwise Retry all replays the stale value on top', + ).toBe(1) + expect(untick?.lastError).toBe(HELD_BACK_REASON) + }) + + it('does not freeze a different record', async () => { + const db = getDexieDb(USER) + await db.mutations.add({ + table: 'checklist_instance_items', targetId: ITEM, op: 'PATCH', + payload: { is_completed: 1 }, createdAt: new Date().toISOString(), + retryCount: 5, failed: 1, + }) + + await db.transaction('rw', db.mutations, () => + enqueueMutationTx(db, 'checklist_instance_items', 'item-2', 'PATCH', { is_completed: 1 }), + ) + + const other = (await mutations()).find((m) => m.targetId === 'item-2') + expect(other?.failed, 'an unrelated record must keep draining').toBeUndefined() + }) + + it('does not freeze the same targetId on a different table', async () => { + const db = getDexieDb(USER) + await db.mutations.add({ + table: 'turnovers', targetId: 'shared-id', op: 'PATCH', + payload: { status: 'completed' }, createdAt: new Date().toISOString(), + retryCount: 5, failed: 1, + }) + + await db.transaction('rw', db.mutations, () => + enqueueMutationTx(db, 'checklist_instances', 'shared-id', 'PATCH', { completed_at: null }), + ) + + const other = (await mutations()).find((m) => m.table === 'checklist_instances') + expect(other?.failed).toBeUndefined() + }) +}) + +describe('F4 — abandoning a dead letter rewinds the cursor masking the server row', () => { + it('clears the cursor for the discarded mutation\'s table', async () => { + const db = getDexieDb(USER) + await db.sync_meta.put({ key: 'cursor:checklist_items', value: '2026-08-04T00:00:00.000Z' }) + await db.sync_meta.put({ key: 'cursor:work_orders', value: '2026-08-04T00:00:00.000Z' }) + + const id = await db.mutations.add({ + table: 'checklist_instance_items', targetId: ITEM, op: 'PATCH', + payload: { is_completed: 1 }, createdAt: new Date().toISOString(), + retryCount: 5, failed: 1, + }) + + await discardFailedMutation(USER, id as number) + + expect( + await db.sync_meta.get('cursor:checklist_items'), + 'without this the delta filter skips that row forever and the cache keeps a value the server never accepted', + ).toBeUndefined() + expect( + await db.sync_meta.get('cursor:work_orders'), + 'only the affected table\'s cursor is rewound — a blanket reset would re-download everything', + ).toBeDefined() + }) +}) + +describe('F5 — orphaned photo blobs are collected', () => { + it('collects a blob no tracking row references, but only after two sweeps', async () => { + await savePendingPhotoBlob(USER, 'orphan-key', new Blob(['bytes'])) + await savePendingPhotoBlob(USER, 'referenced-key', new Blob(['bytes'])) + await getDexieDb(USER).pending_photo_uploads.add({ + id: 'row-1', target_table: 'checklist_instance_items', target_id: ITEM, + target_column: 'photo_storage_path', storage_path: 'org/x.jpg', + local_blob_key: 'referenced-key', mime_type: 'image/jpeg', retry_count: 0, + created_at: new Date().toISOString(), + }) + + // First sweep only nominates — a blob whose row is still mid-enqueue must + // not be destroyed out from under it. + await pruneOrphanPhotoBlobs(USER) + expect(await getPendingPhotoBlob(USER, 'orphan-key')).not.toBeNull() + + await pruneOrphanPhotoBlobs(USER) + expect( + await getPendingPhotoBlob(USER, 'orphan-key'), + 'a blob nothing references is megabytes of dead weight pushing the origin toward eviction', + ).toBeNull() + expect( + await getPendingPhotoBlob(USER, 'referenced-key'), + 'a blob a queued row still points at must survive — Retry needs it', + ).not.toBeNull() + }) + + it('never nominates a blob that is still referenced', async () => { + await savePendingPhotoBlob(USER, 'live-key', new Blob(['bytes'])) + await getDexieDb(USER).pending_photo_uploads.add({ + id: 'row-2', target_table: 'checklist_instance_items', target_id: ITEM, + target_column: 'photo_storage_path', storage_path: 'org/y.jpg', + local_blob_key: 'live-key', mime_type: 'image/jpeg', retry_count: 0, + created_at: new Date().toISOString(), + }) + + await pruneOrphanPhotoBlobs(USER) + await pruneOrphanPhotoBlobs(USER) + await pruneOrphanPhotoBlobs(USER) + + expect(await listPendingPhotoBlobKeys(USER)).toContain('live-key') + }) +}) + +describe('F3 — logout is not blocked by a sibling tab', () => { + it('resolves rather than hanging while another connection holds the database open', async () => { + await seedItem() + const dbName = getDexieDb(USER).name + + // Stand in for a second crew tab: a raw connection this document does not + // own, exactly what makes deleteDatabase fire `blocked` and wait. + const sibling = await new Promise((resolve, reject) => { + const req = indexedDB.open(dbName) + req.onsuccess = () => resolve(req.result) + req.onerror = () => reject(req.error) + }) + + try { + // The bug was not a slow logout — it was one that never completed, so + // supabase.auth.signOut() and the redirect after it never ran at all. + await closeDexieDb() + expect( + isDexieShutdown(USER), + 'the latch must hold even when the delete itself could not complete', + ).toBe(true) + } finally { + sibling.close() + } + }, 15_000) + + it('a broadcast from another tab latches this document and notifies the UI', async () => { + getDexieDb(USER) + const onShutdown = vi.fn() + const stop = listenForRemoteShutdown(USER, onShutdown) + + const channel = new BroadcastChannel('fieldstay-crew-logout') + channel.postMessage({ type: 'shutdown', userId: USER }) + // BroadcastChannel delivery is a macrotask. + await new Promise((resolve) => setTimeout(resolve, 10)) + channel.close() + stop() + + expect( + isDexieShutdown(USER), + 'a sibling tab that keeps draining re-creates the database the logging-out tab just wiped', + ).toBe(true) + expect(onShutdown, 'and the tab must leave the crew surface, not keep rendering it').toHaveBeenCalled() + }) + + it('ignores a broadcast for a different user', async () => { + getDexieDb(USER) + const onShutdown = vi.fn() + const stop = listenForRemoteShutdown(USER, onShutdown) + + const channel = new BroadcastChannel('fieldstay-crew-logout') + channel.postMessage({ type: 'shutdown', userId: 'someone-else' }) + await new Promise((resolve) => setTimeout(resolve, 10)) + channel.close() + stop() + + expect(isDexieShutdown(USER)).toBe(false) + expect(onShutdown).not.toHaveBeenCalled() + }) +}) diff --git a/unit/dexie/photo-sync-durability.test.ts b/unit/dexie/photo-sync-durability.test.ts index 6c329692..41bf8e88 100644 --- a/unit/dexie/photo-sync-durability.test.ts +++ b/unit/dexie/photo-sync-durability.test.ts @@ -160,7 +160,7 @@ describe('photo queue durability', () => { } const row = await photoRow() - expect(row).toMatchObject({ retry_count: 5, failed: true }) + expect(row).toMatchObject({ retry_count: 5, failed: 1 }) expect(row!.last_error).toBeTruthy() // The blob must still exist — "Retry" in the failed-sync banner has // nothing to upload otherwise. @@ -175,7 +175,7 @@ describe('photo queue durability', () => { await processPendingPhotoUploads(failingClient, 'u1') vi.setSystemTime(Date.now() + 600_000) } - expect((await photoRow())!.failed).toBe(true) + expect((await photoRow())!.failed).toBe(1) const ok = makeStorage([{ error: null }]) // eslint-disable-next-line @typescript-eslint/no-explicit-any -- narrow storage stub @@ -247,7 +247,7 @@ describe('photo queue durability', () => { const row = await photoRow() expect(row, 'the row survives — it is the only record of the photo').toBeDefined() - expect(row).toMatchObject({ retry_count: 5, failed: true }) + expect(row).toMatchObject({ retry_count: 5, failed: 1 }) expect(row!.last_error, 'and says why, on the failed-sync surface').toBeTruthy() expect(deletedBlobs, 'the blob is kept so "Retry" has something to upload').toEqual([]) }) @@ -261,7 +261,7 @@ describe('photo queue durability', () => { await processPendingPhotoUploads(stuck.client as any, 'u1') vi.setSystemTime(Date.now() + 600_000) } - expect((await photoRow())!.failed).toBe(true) + expect((await photoRow())!.failed).toBe(1) // The safety poll lands the missing rows. await db().checklist_instance_items.put({ id: 'item1', is_completed: 1, instance_id: 'inst1' }) @@ -276,4 +276,23 @@ describe('photo queue durability', () => { expect(await photoRow()).toBeUndefined() expect(deletedBlobs).toEqual(['blob1']) }) + + it('a photo whose blob is gone dead-letters instead of vanishing', async () => { + // Storage cleared, evicted under quota pressure, or the blob write never + // landed (it goes to a SEPARATE IndexedDB from this row, so the two can + // never commit together). Deleting the row — what this used to do — made + // that indistinguishable from a successful upload: the crew member's photo + // ceased to exist with nothing anywhere saying so. + holder.blob = null + const storage = makeStorage([]) + + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- narrow storage stub + await processPendingPhotoUploads(storage.client as any, 'u1') + + const row = await photoRow() + expect(row, 'the row must survive so the failed-sync surface can show it').toBeDefined() + expect(row!.failed).toBe(1) + expect(row!.last_error).toBeTruthy() + expect(storage.attempts, 'and nothing is uploaded — there are no bytes to send').toEqual([]) + }) }) diff --git a/unit/dexie/sync-outbox-backoff.test.ts b/unit/dexie/sync-outbox-backoff.test.ts index 8e821111..8c36d0d5 100644 --- a/unit/dexie/sync-outbox-backoff.test.ts +++ b/unit/dexie/sync-outbox-backoff.test.ts @@ -162,13 +162,13 @@ describe('processOutbox — retry backoff', () => { // 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).toMatchObject({ retryCount: 5, failed: 1 }) 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 }) + expect(await mutationRow(id)).toMatchObject({ retryCount: 5, failed: 1 }) }) }) diff --git a/unit/dexie/sync-outbox-durability.test.ts b/unit/dexie/sync-outbox-durability.test.ts index c39661ff..faedd7b6 100644 --- a/unit/dexie/sync-outbox-durability.test.ts +++ b/unit/dexie/sync-outbox-durability.test.ts @@ -151,7 +151,7 @@ describe('outbox durability — offline attempts', () => { await vi.advanceTimersByTimeAsync(600_000) } - expect(await mutationRow(id)).toMatchObject({ retryCount: 5, failed: true }) + expect(await mutationRow(id)).toMatchObject({ retryCount: 5, failed: 1 }) }) }) @@ -234,7 +234,7 @@ describe('outbox durability — ordering', () => { // attempt rather than burning five retries, and the queue moves on. const rows = await db().mutations.toArray() as unknown as MutationRow[] expect(rows).toHaveLength(1) - expect(rows[0]).toMatchObject({ targetId: 't-bad', failed: true }) + expect(rows[0]).toMatchObject({ targetId: 't-bad', failed: 1 }) expect(pushed.some((u) => u.includes('t-good'))).toBe(true) }) }) diff --git a/unit/dexie/sync-outbox-ordering.test.ts b/unit/dexie/sync-outbox-ordering.test.ts index 95787e9d..cc589415 100644 --- a/unit/dexie/sync-outbox-ordering.test.ts +++ b/unit/dexie/sync-outbox-ordering.test.ts @@ -131,7 +131,7 @@ describe('outbox — a server timeout no longer blocks the queue forever', () => const row = await mutationRow(id) expect(row?.retryCount, 'a server-side timeout must consume the retry budget').toBeGreaterThan(0) - expect(row?.failed, 'and must eventually become visible to the crew member').toBe(true) + expect(row?.failed, 'and must eventually become visible to the crew member').toBe(1) }) }) @@ -163,11 +163,11 @@ describe('outbox — dead-lettering preserves a record\'s mutation order (H6)', const engine = new SyncEngine('u1') await engine.processOutbox() - expect((await mutationRow(tickId))?.failed, 'the tick dead-letters').toBe(true) + expect((await mutationRow(tickId))?.failed, 'the tick dead-letters').toBe(1) expect( (await mutationRow(untickId))?.failed, 'the un-tick must be held back, or Retry all replays the tick ON TOP of it', - ).toBe(true) + ).toBe(1) // The un-tick never reached the server, so there is no newer server state // for a later Retry-all replay of the tick to clobber. (`calls` records @@ -197,7 +197,7 @@ describe('outbox — dead-lettering preserves a record\'s mutation order (H6)', const engine = new SyncEngine('u1') await engine.processOutbox() - expect((await mutationRow(failing))?.failed).toBe(true) + expect((await mutationRow(failing))?.failed).toBe(1) expect( await mutationRow(other), 'an unrelated record must still drain — blocking everything was the bug this replaced', diff --git a/unit/dexie/sync-shadow-and-prune.test.ts b/unit/dexie/sync-shadow-and-prune.test.ts index a08dccd2..167ba2da 100644 --- a/unit/dexie/sync-shadow-and-prune.test.ts +++ b/unit/dexie/sync-shadow-and-prune.test.ts @@ -50,7 +50,7 @@ describe('pending-mutation shadowing', () => { it('shadows dead-lettered mutations too — that write did not reach the server either', async () => { await db().mutations.add({ table: 'turnovers', targetId: 't1', op: 'PATCH', - payload: { status: 'completed' }, createdAt: '2026-07-30T00:00:00Z', retryCount: 5, failed: true, + payload: { status: 'completed' }, createdAt: '2026-07-30T00:00:00Z', retryCount: 5, failed: 1, }) const [row] = await shadowPendingMutations('u1', 'turnovers', [{ id: 't1', status: 'in_progress' }]) expect(row).toMatchObject({ status: 'completed' }) @@ -100,10 +100,10 @@ describe('local cache pruning', () => { it('keeps live dead letters and only collects expired ones', async () => { const recent = new Date(Date.now() - 2 * DAY).toISOString() const ancient = new Date(Date.now() - 200 * DAY).toISOString() - await db().mutations.add({ table: 'turnovers', targetId: 't1', op: 'PATCH', payload: {}, createdAt: recent, retryCount: 5, failed: true }) - await db().mutations.add({ table: 'turnovers', targetId: 't2', op: 'PATCH', payload: {}, createdAt: ancient, retryCount: 5, failed: true }) - await db().pending_photo_uploads.put({ id: 'ph1', created_at: ancient, failed: true, local_blob_key: 'k1' }) - await db().pending_photo_uploads.put({ id: 'ph2', created_at: recent, failed: true, local_blob_key: 'k2' }) + await db().mutations.add({ table: 'turnovers', targetId: 't1', op: 'PATCH', payload: {}, createdAt: recent, retryCount: 5, failed: 1 }) + await db().mutations.add({ table: 'turnovers', targetId: 't2', op: 'PATCH', payload: {}, createdAt: ancient, retryCount: 5, failed: 1 }) + await db().pending_photo_uploads.put({ id: 'ph1', created_at: ancient, failed: 1, local_blob_key: 'k1' }) + await db().pending_photo_uploads.put({ id: 'ph2', created_at: recent, failed: 1, local_blob_key: 'k2' }) await pruneLocalCache('u1') @@ -117,7 +117,7 @@ describe('local cache pruning', () => { describe('countPendingSyncWork', () => { it('counts only work still on its way, reporting dead letters separately', async () => { await db().mutations.add({ table: 'turnovers', targetId: 't1', op: 'PATCH', payload: {}, createdAt: '', retryCount: 0 }) - await db().mutations.add({ table: 'turnovers', targetId: 't2', op: 'PATCH', payload: {}, createdAt: '', retryCount: 5, failed: true }) + await db().mutations.add({ table: 'turnovers', targetId: 't2', op: 'PATCH', payload: {}, createdAt: '', retryCount: 5, failed: 1 }) await db().pending_photo_uploads.put({ id: 'p1', created_at: '', local_blob_key: 'k' }) expect(await countPendingSyncWork('u1')).toEqual({ pending: 2, deadLettered: 1 }) diff --git a/unit/guardrails/crew-dead-letter-coverage.test.ts b/unit/guardrails/crew-dead-letter-coverage.test.ts index 00371971..c720be8c 100644 --- a/unit/guardrails/crew-dead-letter-coverage.test.ts +++ b/unit/guardrails/crew-dead-letter-coverage.test.ts @@ -102,8 +102,26 @@ describe('guardrail: every cached crew table is bounded', () => { // The failed-sync surface is built on these rows: collecting them // eagerly would re-create the exact silent-loss bug they exist to fix. expect(PRUNE_SRC).toContain('DEAD_LETTER_RETENTION_DAYS') - expect(PRUNE_SRC).toMatch(/failed && m\.createdAt < horizon/) - expect(PRUNE_SRC).toMatch(/failed && p\.created_at < horizon/) + // Both dead-letter queues are selected by the `failed` flag AND gated on + // the retention horizon — never collected on the flag alone. + expect(PRUNE_SRC).toMatch(/where\('failed'\)\.equals\(1\)[\s\S]{0,120}?m\.createdAt < horizon/) + expect(PRUNE_SRC).toMatch(/where\('failed'\)\.equals\(1\)[\s\S]{0,120}?p\.created_at < horizon/) + }) + + it('abandoning a dead letter rewinds the cursor that would hide the server row', () => { + // A queued mutation is shadowed over every pull (lib/dexie/sync/shadow.ts) + // while the cursor advances past the server row it masks. Dropping the + // mutation without rewinding leaves the cache pinned to a value the server + // never accepted — permanently, since the delta filter will never return + // that row again. Applies to the timed prune here AND to an explicit + // discard (lib/dexie/helpers.ts). + expect(PRUNE_SRC).toContain('invalidateCursorsFor') + const helpers = readFileSync(join(ROOT, 'lib', 'dexie', 'helpers.ts'), 'utf8') + expect( + /discardFailedMutation[\s\S]{0,600}?invalidateCursorsFor/.test(helpers), + 'discardFailedMutation drops the outbox row without rewinding the cursor — ' + + 'the local row then keeps a value the server never accepted, forever', + ).toBe(true) }) it('the logout warning counts only genuinely pending work', () => { From 9f22c4f0cad93bbced284d003fa7647014db736f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 00:36:31 +0000 Subject: [PATCH 2/2] fix(crew-sync): fairness, visibility and versioning in the offline drain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Offline-sync audit, part 2 of 2. Latency, invisible-state and forward- compatibility defects rather than outright data loss. F6 — a photo stuck in the transport-failure loop was invisible on every surface. A transport failure never sets `failed` (by design), so it fell out of the banner's dead-letter query, and the amber stalled notice only ever looked at db.mutations. A whole shift of verification photos could retry forever against a captive portal with nothing on screen, on a banner whose header comment claims it covers every queued photo. F7 — nothing serialized resync. A phone waking fires `online` and `visibilitychange` within the same second and the safety poll can join them, so up to three fullCrewResyncs ran concurrently on the worst possible connection: triple the queries, a race on advanceCursor's read-modify-write, and one pass's pruneLocalCache bulkDeleting from a snapshot another was mutating. Coalesced to one in flight plus one queued, the same shape createSyncSignalHandler already used per entity. F8 — a drain works from a snapshot, so a mutation queued mid-drain was invisible to it, and enqueueMutation's kick was dropped by the isProcessing guard. The row then waited for the next 30 s tick — during the reconnect window, where a crew member is most likely still working — and the bounded flush at logout could report "clean" for a row it never attempted. F10 — an outbox row can outlive the release that queued it. Payloads are now version-stamped with a migration hook, and a (table, op) with no handler is terminal rather than transient: it was a bare Error, so it burned five pointless round trips before dead-lettering with a developer string naming the table and op as its user-facing text. F11 — the per-record ordering invariant was enforced globally: any retryable failure or backoff window stopped the whole queue, so one flaky record stranded dozens of unrelated writes on reconnect. Now blocked per record — with a consecutive-distinct-failure circuit breaker, because per-record blocking alone would turn a server-side outage into N wasted requests per wave instead of one. F12 — checklist photo blob keys and storage paths were Date.now()-suffixed. Two captures in the same millisecond collided: the second overwrote the first blob, both rows referenced it, and the first row's cleanup deleted it out from under the second. The tracking row's own id already used crypto.randomUUID(). Lint ratchet 202 -> 201. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Bs2uS5NYLR8Pvk4sw5NiKz --- CLAUDE.md | 27 ++- app/crew/_components/failed-sync-banner.tsx | 28 ++- .../turnovers/[id]/use-turnover-actions.ts | 16 +- lib/dexie/context.tsx | 43 ++++- lib/dexie/helpers.ts | 1 - lib/dexie/net.ts | 5 +- lib/dexie/syncService.ts | 139 ++++++++++++--- package.json | 2 +- unit/dexie/outbox-drain-scheduling.test.ts | 161 ++++++++++++++++++ unit/dexie/sync-outbox-backoff.test.ts | 59 +++++-- .../crew-dead-letter-coverage.test.ts | 21 +++ 11 files changed, 451 insertions(+), 51 deletions(-) create mode 100644 unit/dexie/outbox-drain-scheduling.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 44956f70..e07fcc9c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -450,8 +450,33 @@ almost the whole crew surface): `lib/dexie/prune.ts` — `messages` grew forever at 500 rows a pull. And every member of the `MutationTable` union must have a retry affordance in `app/crew/_components/failed-sync-banner.tsx`: a mutation that dead-letters - where no crew member can see it is work silently thrown away. Enforced by + where no crew member can see it is work silently thrown away. BOTH outboxes + (`mutations` and `pending_photo_uploads`) need a dead-letter query AND a + stalled-queue query there — a transport failure never sets `failed`, so the + stalled surface is its only visible one. Enforced by `unit/guardrails/crew-dead-letter-coverage.test.ts`. +- **The optimistic local write and its outbox row commit in ONE Dexie + transaction.** Use `writeAndQueue()`/`enqueueMutationTx()` (`lib/dexie/ + helpers.ts`, `lib/dexie/syncService.ts`) — never a bare `table.update()` + followed by a separate `enqueueMutation()`. As two transactions, a PWA + reclaimed between them left the cache updated with nothing queued to send + it, and no delta pull corrects that because the server row's `updated_at` + never changed. Nothing async-external may go inside the block: an IDB + transaction auto-commits the moment an await leaves it, so the + `processOutbox()` kick stays outside. +- **Abandoning a queued mutation rewinds the cursor that was masking the + server row.** `discardFailedMutation()` and `pruneExpiredDeadLetters()` call + `invalidateCursorsFor()`. While a mutation is queued, `shadow.ts` replays it + over every pull AND `advanceCursor()` moves past the server row it masks — + drop it without rewinding and the delta filter skips that row forever. + `forceFullCrewResync()` is the whole-cache version, for a device that has + already diverged. +- **`failed` is `0 | 1`, never a boolean** (`DeadLetterFlag`). IndexedDB has no + boolean key type, so a boolean `failed` is silently absent from its index and + every dead-letter query degrades to a full scan — three of which are + `useLiveQuery`s live on every crew screen, over a table written on every + checklist tick. Truthiness checks (`!m.failed`) are unaffected; only literal + `true`/`false` writes. **Crew Sync v2 coverage convention** (`docs/CREW_SYNC_V2_PHASES.md` section 5e): every Supabase-backed table the crew PWA caches in Dexie is covered by the diff --git a/app/crew/_components/failed-sync-banner.tsx b/app/crew/_components/failed-sync-banner.tsx index ca4b0984..80db5aae 100644 --- a/app/crew/_components/failed-sync-banner.tsx +++ b/app/crew/_components/failed-sync-banner.tsx @@ -58,13 +58,17 @@ export function FailedSyncBanner({ userId }: Readonly<{ userId: string }>) { const [retrying, setRetrying] = useState(false) const [confirmDiscard, setConfirmDiscard] = useState(null) + // Index-backed (`failed` is stored 0/1 — IndexedDB cannot index a boolean). + // These are live queries on tables that are written on every checklist tick + // and every drain step, so as `.filter()` full scans they re-deserialized + // the whole outbox, three times, on each of those writes. const failedMutations = useLiveQuery( - () => db.mutations.filter((m) => !!m.failed).toArray(), + () => db.mutations.where('failed').equals(1).toArray(), [], ) ?? [] const failedPhotos = useLiveQuery( - () => db.pending_photo_uploads.filter((p) => !!p.failed).toArray(), + () => db.pending_photo_uploads.where('failed').equals(1).toArray(), [], ) ?? [] @@ -83,6 +87,20 @@ export function FailedSyncBanner({ userId }: Readonly<{ userId: string }>) { [], ) ?? [] + // Photos stall the same way and were covered by NEITHER surface: a transport + // failure never sets `failed` (by design — a bad signal must not destroy + // crew work), so they fell out of failedPhotos above, and the stalled notice + // only ever looked at db.mutations. A whole shift of verification photos + // could retry forever against a captive portal with nothing on screen. + const stalledPhotos = useLiveQuery( + () => db.pending_photo_uploads + .filter((p) => !p.failed && (p.network_retry_count ?? 0) >= STALLED_NETWORK_ATTEMPTS) + .toArray(), + [], + ) ?? [] + + const stalledCount = stalledMutations.length + stalledPhotos.length + const entries: FailedEntry[] = [ ...failedMutations.map((m) => ({ key: `mutation-${m.id}`, @@ -98,12 +116,12 @@ export function FailedSyncBanner({ userId }: Readonly<{ userId: string }>) { })), ] - if (entries.length === 0 && stalledMutations.length === 0) return null + if (entries.length === 0 && stalledCount === 0) return null // A stalled queue is NOT a failure — the work is intact and still retrying, // so it gets its own amber notice with no discard affordance rather than // being folded into the red "didn't sync" list. - const stalledNotice = stalledMutations.length > 0 && ( + const stalledNotice = stalledCount > 0 && (
) {

- {stalledMutations.length} change{stalledMutations.length !== 1 ? 's' : ''} still trying to sync + {stalledCount} change{stalledCount !== 1 ? 's' : ''} still trying to sync

Your work is saved on this phone and will keep retrying on its own. diff --git a/app/crew/turnovers/[id]/use-turnover-actions.ts b/app/crew/turnovers/[id]/use-turnover-actions.ts index a6547da4..bc160eaa 100644 --- a/app/crew/turnovers/[id]/use-turnover-actions.ts +++ b/app/crew/turnovers/[id]/use-turnover-actions.ts @@ -192,8 +192,15 @@ export function useTurnoverActions(id: string) { const ext = file.name.split('.').pop() ?? 'jpg' const slug = sectionName.replace(/\s+/g, '-').toLowerCase() - const path = orgScopedStoragePath(orgId, `turnover-${id}`, `section-${slug}-${Date.now()}.${ext}`) - const blobKey = `photo-section-${sectionName}-${Date.now()}` + // Both keys are UUID-suffixed, not Date.now()-suffixed. Two captures + // landing in the same millisecond (a double-tap, a retry) produced + // identical keys: the second savePendingPhotoBlob() overwrote the first + // blob, both queue rows referenced it, and the first row's + // discardPendingPhoto() deleted it out from under the second — which then + // hit the missing-blob branch. The tracking row's own id has always used + // crypto.randomUUID(); these two just predated the convention. + const path = orgScopedStoragePath(orgId, `turnover-${id}`, `section-${slug}-${crypto.randomUUID()}.${ext}`) + const blobKey = `photo-section-${crypto.randomUUID()}` try { const sectionItem = items?.find((i) => i.section_name === sectionName) @@ -236,8 +243,9 @@ export function useTurnoverActions(id: string) { } try { const ext = file.name.split('.').pop() ?? 'jpg' - const path = orgScopedStoragePath(orgId, `turnover-${id}`, `${itemId}-${Date.now()}.${ext}`) - const blobKey = `photo-${itemId}-${Date.now()}` + // UUID-suffixed, not Date.now() — see handleSectionPhoto above. + const path = orgScopedStoragePath(orgId, `turnover-${id}`, `${itemId}-${crypto.randomUUID()}.${ext}`) + const blobKey = `photo-${crypto.randomUUID()}` const compressed = await compressPhotoForQueue(file) await savePendingPhotoBlob(userId, blobKey, compressed) diff --git a/lib/dexie/context.tsx b/lib/dexie/context.tsx index 16ec9d90..b628a3db 100644 --- a/lib/dexie/context.tsx +++ b/lib/dexie/context.tsx @@ -282,11 +282,41 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin await refreshAssetsSubscription() } - function resyncSafe(crewMemberId: string): void { + // ── Resync coalescing ───────────────────────────────────────────────── + // + // A phone waking in a parking lot fires `online` and `visibilitychange → + // visible` within the same second, and the 5-minute safety poll can land + // in the same window; on the v1 path a turnover_assignments event can too. + // Each of those used to start its own fullCrewResync with nothing + // serializing them, so three concurrent passes ran on the worst possible + // connection — tripling the query volume, racing advanceCursor()'s + // read-modify-write, and letting one pass's pruneLocalCache() bulkDelete + // from a snapshot another was still mutating (visible as flicker and + // transient empty states). + // + // One in flight, at most one queued follow-up — the same shape + // createSyncSignalHandler() already uses per entity. + let resyncInFlight: Promise | null = null + let resyncQueued = false + + function runCoalesced(run: () => Promise, label: string): void { if (cancelled) return - void resync(crewMemberId).catch((err) => - console.error('[DexieProvider] resync failed:', err) - ) + if (resyncInFlight) { + resyncQueued = true + return + } + resyncInFlight = run() + .catch((err) => console.error(`[DexieProvider] ${label} failed:`, err)) + .finally(() => { + resyncInFlight = null + if (cancelled || !resyncQueued) return + resyncQueued = false + runCoalesced(run, label) + }) + } + + function resyncSafe(crewMemberId: string): void { + runCoalesced(() => resync(crewMemberId), 'resync') } // Installed on BOTH paths (see SAFETY_POLL_INTERVAL_MS above) — the @@ -312,10 +342,7 @@ export function DexieProvider({ userId: userIdProp, children }: { userId?: strin } function resyncV2Safe(crewMemberId: string): void { - if (cancelled) return - void resyncV2(crewMemberId).catch((err) => - console.error('[DexieProvider] v2 resync failed:', err) - ) + runCoalesced(() => resyncV2(crewMemberId), 'v2 resync') } // Retry helper for scheduleV2Reconnect's timer callback — kept as its diff --git a/lib/dexie/helpers.ts b/lib/dexie/helpers.ts index 6898ee5b..399f0a2a 100644 --- a/lib/dexie/helpers.ts +++ b/lib/dexie/helpers.ts @@ -79,7 +79,6 @@ export async function updateChecklistItem( input: UpdateChecklistItemInput, crewMemberId?: string | null, ): Promise { - const db = getDexieDb(userId) const completedAt = input.isCompleted ? new Date().toISOString() : null const changes: Record = { diff --git a/lib/dexie/net.ts b/lib/dexie/net.ts index 6a03b29a..a645fbb5 100644 --- a/lib/dexie/net.ts +++ b/lib/dexie/net.ts @@ -74,7 +74,10 @@ const TERMINAL_CODE_PATTERN = /^(22|23|42)\d{3}$/ // Client-side codes this codebase raises for a mutation that is structurally // unsendable — replaying it byte-for-byte can only fail the same way. -const TERMINAL_CODES = new Set(['NO_FIELDS']) +// NO_HANDLER: the mutation names a (table, op) this build has no upload +// handler for — an outbox row queued by an older release. Replay cannot +// conjure the handler back, so it must not burn the retry budget first. +const TERMINAL_CODES = new Set(['NO_FIELDS', 'NO_HANDLER']) function messageOf(err: unknown): string { if (err instanceof Error) return err.message diff --git a/lib/dexie/syncService.ts b/lib/dexie/syncService.ts index 89f2816b..4f78319b 100644 --- a/lib/dexie/syncService.ts +++ b/lib/dexie/syncService.ts @@ -29,6 +29,25 @@ export const OUTBOX_PAYLOAD_VERSION = 1 */ export const HELD_BACK_REASON = 'Held back so earlier changes to this item retry in order' +/** + * Consecutive failed pushes against DISTINCT records before the drain gives up + * for this pass. + * + * The ordering invariant the drain protects is per-record — a later write to + * turnover A must not overtake an earlier one — but it used to be enforced + * globally: any retryable failure, or any head still inside its backoff + * window, stopped the whole queue. One flaky record therefore stranded every + * unrelated record behind it, which on reconnect after an offline shift is + * dozens of writes waiting on one. + * + * Blocking strictly per-record would swing too far the other way: when the + * server itself is unhappy, every record fails, and per-record blocking turns + * one wasted request per wave into N. So: block per record, but treat several + * distinct records failing in a row as evidence the problem is not the record, + * and stand down until the scheduled retry. + */ +export const CONSECUTIVE_FAILURE_CIRCUIT_BREAK = 3 + // 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. @@ -57,6 +76,9 @@ export class SyncEngine { // a shared IndexedDB, so two tabs each pass their own guard — withTabLock() // in processOutbox() is what actually serializes the drain across tabs. private isProcessing = false + // Set when a drain was requested while one was already running — see + // processOutbox(). The running pass re-drains rather than dropping the row. + private redrainRequested = false // Ids held back mid-drain by holdBackSuccessors(). Scoped to a single // drain (cleared at its start) — the durable state is the row's `failed` // flag; this only stops the current loop's pre-computed snapshot from @@ -146,10 +168,24 @@ export class SyncEngine { } async processOutbox(): Promise { - if (this.isProcessing || this.stopped()) return + if (this.stopped()) return + // A drain works from a snapshot taken before its loop, so a mutation + // queued while one is in flight is invisible to it — and enqueueMutation's + // own kick lands here and is dropped by the guard. That row then sat until + // the crew shell's next 30 s tick, which is precisely the reconnect window + // where a crew member is most likely to still be working. Worse at logout, + // where the bounded final flush could report "clean" for a row it never + // attempted. Remember the wake-up instead of discarding it. + if (this.isProcessing) { + this.redrainRequested = true + return + } this.isProcessing = true try { - await withTabLock(`fieldstay-crew-outbox-${this.userId}`, () => this.drain()) + do { + this.redrainRequested = false + await withTabLock(`fieldstay-crew-outbox-${this.userId}`, () => this.drain()) + } while (this.redrainRequested && !this.stopped()) } finally { this.isProcessing = false } @@ -169,18 +205,27 @@ export class SyncEngine { // preserve ordering, which defeats the whole point. this.heldBack.clear() + // Records whose queue is blocked for the rest of this pass: one of their + // mutations is mid-backoff or just failed, so every LATER mutation for the + // same record must wait rather than overtake it. Scoped to the record — + // see CONSECUTIVE_FAILURE_CIRCUIT_BREAK for why it isn't global any more, + // and for what still stops the whole drain. + const blockedRecords = new Set() + let consecutiveFailures = 0 + for (const mutation of pending) { // Auto-incrementing key — always populated once read back from the table. const id = mutation.id as number + const record = `${mutation.table}:${mutation.targetId}` - if (this.heldBack.has(id)) continue + if (this.heldBack.has(id) || blockedRecords.has(record)) continue - // 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 + // Backoff gate: a mutation still inside its retry window blocks its own + // record (never skip-and-continue — later mutations against the SAME + // record must not jump ahead). Resume when it comes due. + if (this.isBackingOff(mutation)) { + blockedRecords.add(record) + continue } // Connectivity can drop mid-drain — re-check before every push so the @@ -190,29 +235,55 @@ export class SyncEngine { // not write to (or re-open) a database that no longer exists. if (!isOnline() || this.stopped()) return - if (await this.pushOne(db, mutation, id) === 'stop') return + const outcome = await this.pushOne(db, mutation, id) + if (outcome === 'abort') return + if (outcome !== 'blocked') { + consecutiveFailures = 0 + continue + } + + blockedRecords.add(record) + consecutiveFailures += 1 + // Distinct records failing back to back means the server, not the + // record — stop pushing into it and wait for the scheduled retry. + if (consecutiveFailures >= CONSECUTIVE_FAILURE_CIRCUIT_BREAK) return } } + /** True when this mutation is still inside its retry window (and schedules the resume). */ + private isBackingOff(mutation: MutationRow): boolean { + if (mutation.nextAttemptAt === undefined || mutation.nextAttemptAt <= Date.now()) return false + this.scheduleRetry(mutation.nextAttemptAt) + return true + } + /** - * Pushes one mutation and records the outcome. Returns 'stop' when the drain - * must not continue — either to preserve per-record ordering, or because - * this user signed out mid-push and their local database is gone. + * Pushes one mutation and records the outcome. + * + * - 'continue' — done with this mutation; the drain may proceed. + * - 'blocked' — this RECORD's queue must not advance (ordering), but other + * records may still drain. + * - 'abort' — stop touching storage entirely: this user signed out + * mid-push and their local database is gone. */ - private async pushOne(db: FieldStayDexie, mutation: MutationRow, id: number): Promise<'continue' | 'stop'> { + private async pushOne( + db: FieldStayDexie, + mutation: MutationRow, + id: number, + ): Promise<'continue' | 'blocked' | 'abort'> { try { await uploadOne(this.supabase, mutation) // Signing out while this push was in flight deletes the outbox // underneath us. Bail before touching storage: bookkeeping for a // signed-out user has nothing to write to, and going ahead would ask // getDexieDb() for a database that no longer exists. - if (this.stopped()) return 'stop' + if (this.stopped()) return 'abort' // Successful push clears the whole row — nextAttemptAt with it. await db.mutations.delete(id) return 'continue' } catch (err) { - if (this.stopped()) return 'stop' - return await this.handleFailure(mutation, id, err) ? 'stop' : 'continue' + if (this.stopped()) return 'abort' + return await this.handleFailure(mutation, id, err) ? 'blocked' : 'continue' } } @@ -713,8 +784,25 @@ const UPLOAD_HANDLERS: Record = { 'inventory_count_drafts:PUT': uploadInventoryCountDraft, } +/** + * Payload-shape migrations, keyed by the version they upgrade FROM. + * + * An outbox row can outlive the release that queued it — a device offline + * across a deploy replays yesterday's payload shape against today's handler. + * Add an entry here in the same change that bumps OUTBOX_PAYLOAD_VERSION. + */ +const PAYLOAD_MIGRATIONS: Readonly MutationPayload>> = {} + +function migratePayload(mutation: MutationRow): MutationPayload { + let payload = mutation.payload + for (let version = mutation.payloadVersion ?? 1; version < OUTBOX_PAYLOAD_VERSION; version++) { + payload = PAYLOAD_MIGRATIONS[version]?.(payload) ?? payload + } + return payload +} + async function uploadOne(supabase: DexieSupabaseClient, mutation: MutationRow): Promise { - const { table, targetId, op, payload } = mutation + const { table, targetId, op } = mutation const handler = UPLOAD_HANDLERS[`${table}:${op}`] if (!handler) { @@ -722,10 +810,21 @@ async function uploadOne(supabase: DexieSupabaseClient, mutation: MutationRow): // instead of letting processOutbox() treat this as a successful sync // and silently delete the mutation from the outbox without it ever // reaching Supabase. - throw new Error(`[SyncEngine] no upload handler for mutation: table="${table}" op="${op}" targetId="${targetId}"`) + // + // TERMINAL, not transient: a (table, op) this build has no handler for + // cannot start working on retry #5. It was previously a bare Error, which + // classifies as transient — five pointless round trips, then a dead letter + // whose user-facing text was the developer string below. + console.error( + `[SyncEngine] no upload handler for mutation: table="${table}" op="${op}" targetId="${targetId}"`, + ) + throw new UploadDataError( + 'This change was saved by an older version of the app and can no longer be sent.', + 'NO_HANDLER', + ) } - await handler(supabase, targetId, payload) + await handler(supabase, targetId, migratePayload(mutation)) } let engine: SyncEngine | null = null diff --git a/package.json b/package.json index d662c033..7a24776e 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "dev": "next dev", "build": "next build", "start": "next start", - "lint": "eslint . --max-warnings 202", + "lint": "eslint . --max-warnings 201", "check:ui-classes": "bash scripts/check-raw-ui-classes.sh", "check:db-invariants": "node scripts/check-db-invariants.mjs", "check:db-invariants:prod": "DB_INVARIANTS_ALLOW_PROD=1 node scripts/check-db-invariants.mjs", diff --git a/unit/dexie/outbox-drain-scheduling.test.ts b/unit/dexie/outbox-drain-scheduling.test.ts new file mode 100644 index 00000000..aed23b8c --- /dev/null +++ b/unit/dexie/outbox-drain-scheduling.test.ts @@ -0,0 +1,161 @@ +// 2026-08-04 offline-sync audit, part 2 — the scheduling and versioning +// properties of the outbox drain. +// +// F8 — a drain works from a snapshot taken before its loop, so a mutation +// queued while one is in flight is invisible to it. enqueueMutation's +// own kick then landed on the `isProcessing` guard and was DROPPED, so +// that row waited for the crew shell's next 30 s tick — during the +// reconnect window, which is exactly when a crew member is most likely +// to still be working. Worse at logout, where the bounded final flush +// could report "clean" for a row it had never attempted. +// +// F10 — an outbox row can outlive the release that queued it (a device +// offline across a deploy). A (table, op) this build has no handler for +// threw a bare Error, which classifies as TRANSIENT: five pointless +// round trips, then a dead letter whose user-facing text was a +// developer string naming the table and op. + +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, + /** Resolves the in-flight upload on demand, so a second enqueue can be + * landed while the drain is genuinely mid-await. */ + gate: null as null | { promise: Promise; release: () => void; started: Promise }, +})) + +vi.mock('@/lib/dexie/schema', () => ({ + getDexieDb: () => holder.db, + isDexieShutdown: () => false, +})) + +vi.mock('@/lib/supabase/client', () => ({ + createClient: () => ({ + from: (table: string) => { + const chain = (holder.supabase as ReturnType).from(table) + if (!holder.gate) return chain + const gate = holder.gate + // Wrap only the terminal select() so the whole chain still records calls. + const inner = chain.select + chain.select = (...args: unknown[]) => { + const result = inner(...args) + return { then: (res: (v: unknown) => unknown) => gate.promise.then(() => result).then(res) } + } + return chain + }, + }), +})) + +import { SyncEngine, OUTBOX_PAYLOAD_VERSION } from '@/lib/dexie/syncService' +import { classifyUploadFailure, UploadDataError } from '@/lib/dexie/net' + +function db(): FakeDexieDb { return holder.db as FakeDexieDb } +const UPLOAD_OK = { data: [{ id: 'x' }], error: null } + +function setOnline(value: boolean): void { + Object.defineProperty(globalThis, 'navigator', { configurable: true, value: { onLine: value } }) +} + +async function seed(overrides: Partial = {}): Promise { + return await db().mutations.add({ + table: 'inventory_items', + targetId: 'item1', + op: 'PATCH', + payload: { current_quantity: 3 }, + createdAt: new Date().toISOString(), + retryCount: 0, + ...overrides, + }) as number +} + +beforeEach(() => { + holder.db = makeFakeDexieDb() + holder.gate = null + setOnline(true) + vi.spyOn(console, 'error').mockImplementation(() => {}) + vi.spyOn(console, 'warn').mockImplementation(() => {}) +}) + +afterEach(() => vi.restoreAllMocks()) + +describe('F8 — a mutation queued mid-drain is not stranded until the next tick', () => { + it('re-drains once the in-flight pass settles instead of dropping the wake-up', async () => { + holder.supabase = makeFakeSupabase({ + inventory_items: [UPLOAD_OK, UPLOAD_OK, UPLOAD_OK], + }) + await seed({ targetId: 'first' }) + + let release!: () => void + const promise = new Promise((resolve) => { release = resolve }) + holder.gate = { promise, release, started: Promise.resolve() } + + const engine = new SyncEngine('u1') + const firstPass = engine.processOutbox() + + // Land a new mutation while the first push is genuinely in flight, and + // ring the bell the same way enqueueMutation() does. + await seed({ targetId: 'second' }) + await engine.processOutbox() // swallowed by isProcessing — must be REMEMBERED + + holder.gate = null + release() + await firstPass + + expect( + await db().mutations.toArray(), + 'the row queued mid-drain must be pushed by the same pass, not left for the 30 s interval', + ).toHaveLength(0) + }) + + it('does not loop when nothing new was queued', async () => { + holder.supabase = makeFakeSupabase({ inventory_items: [UPLOAD_OK] }) + await seed() + + const engine = new SyncEngine('u1') + await engine.processOutbox() + + // One push, one drain — the re-drain loop must be driven by an actual + // request, not spin on its own. + const updates = (holder.supabase as ReturnType).calls + .filter((c) => c.method === 'update') + expect(updates).toHaveLength(1) + }) +}) + +describe('F10 — payload versioning and unknown handlers', () => { + it('stamps the current payload version on every newly queued mutation', async () => { + const { enqueueMutationTx } = await import('@/lib/dexie/syncService') + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- fake db, not a real FieldStayDexie + await enqueueMutationTx(db() as any, 'inventory_items', 'item1', 'PATCH', { current_quantity: 1 }) + + const [row] = await db().mutations.toArray() as unknown as MutationRow[] + expect( + row!.payloadVersion, + 'without a version stamp a payload queued before a deploy replays blind against the new handler', + ).toBe(OUTBOX_PAYLOAD_VERSION) + }) + + it('dead-letters an unhandled (table, op) immediately rather than burning five retries', async () => { + holder.supabase = makeFakeSupabase({}) + // 'DELETE' has no entry in UPLOAD_HANDLERS — the shape of a row queued by + // a release that supported an operation this build no longer does. + const id = await seed({ op: 'DELETE' }) + + await new SyncEngine('u1').processOutbox() + + const row = await db().mutations.get(id) as MutationRow | undefined + expect(row?.failed, 'it can never succeed on replay').toBe(1) + expect(row?.retryCount, 'and must not spend the retry budget getting there').toBe(1) + expect( + row?.lastError, + 'the crew member sees this string — it must not be a developer message naming the table and op', + ).not.toMatch(/table=|op=/) + }) + + it('classifies NO_HANDLER as terminal', () => { + expect(classifyUploadFailure(new UploadDataError('older version', 'NO_HANDLER'))).toBe('terminal') + }) +}) diff --git a/unit/dexie/sync-outbox-backoff.test.ts b/unit/dexie/sync-outbox-backoff.test.ts index 8c36d0d5..46257501 100644 --- a/unit/dexie/sync-outbox-backoff.test.ts +++ b/unit/dexie/sync-outbox-backoff.test.ts @@ -21,7 +21,11 @@ vi.mock('@/lib/supabase/client', () => ({ }), })) -import { SyncEngine, computeNextAttemptAt } from '@/lib/dexie/syncService' +import { + SyncEngine, + computeNextAttemptAt, + CONSECUTIVE_FAILURE_CIRCUIT_BREAK, +} from '@/lib/dexie/syncService' const NOW = Date.parse('2026-07-25T12:00:00.000Z') @@ -101,18 +105,31 @@ describe('processOutbox — retry backoff', () => { 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] }) + it('blocks the not-yet-due RECORD without stranding unrelated ones', async () => { + // The ordering invariant is per-record, but it used to be enforced + // globally: one mutation inside its backoff window stopped the entire + // queue. On reconnect after an offline shift that is dozens of unrelated + // writes waiting on one flaky record. + const headId = await seedMutation({ retryCount: 1, nextAttemptAt: NOW + 60_000 }) + const sameRecId = await seedMutation() // item1 again, due + const otherId = await seedMutation({ targetId: 'item2' }) // different record, due + holder.supabase = makeFakeSupabase({ + inventory_items: [UPLOAD_OK, UPLOAD_OK, 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) + // The backing-off record is untouched — including the LATER write to it, + // which must never overtake the one that is waiting. expect(await mutationRow(headId)).toMatchObject({ retryCount: 1, nextAttemptAt: NOW + 60_000 }) - expect(await mutationRow(laterId)).toMatchObject({ retryCount: 0 }) + expect(await mutationRow(sameRecId)).toMatchObject({ retryCount: 0 }) + + // An unrelated record drains. + expect( + await mutationRow(otherId), + 'a different record must not be held behind an unrelated backoff window', + ).toBeUndefined() // One resume timer scheduled; a second stopped drain replaces it (single handle). expect(vi.getTimerCount()).toBe(1) @@ -122,8 +139,30 @@ describe('processOutbox — retry backoff', () => { // 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) + expect(await mutationRow(sameRecId)).toBeUndefined() + }) + + it('stands down entirely once several DISTINCT records fail in a row', async () => { + // Per-record blocking alone would turn a server-side outage — where every + // record fails — into N wasted requests per wave instead of one. Distinct + // records failing back to back is evidence the problem is the server. + for (const targetId of ['a', 'b', 'c', 'd', 'e', 'f']) { + await seedMutation({ targetId }) + } + holder.supabase = makeFakeSupabase({ + inventory_items: Array.from({ length: 10 }, () => ({ + error: { message: 'server exploded', code: 'XX000' }, // transient + })), + }) + + await new SyncEngine('u1').processOutbox() + + const attempted = (await db().mutations.toArray() as unknown as MutationRow[]) + .filter((m) => m.retryCount > 0) + expect( + attempted, + 'the drain must stop probing a server that just rejected several unrelated records', + ).toHaveLength(CONSECUTIVE_FAILURE_CIRCUIT_BREAK) }) it('retries a due mutation and clears nextAttemptAt on success (row removed)', async () => { diff --git a/unit/guardrails/crew-dead-letter-coverage.test.ts b/unit/guardrails/crew-dead-letter-coverage.test.ts index c720be8c..8289a39d 100644 --- a/unit/guardrails/crew-dead-letter-coverage.test.ts +++ b/unit/guardrails/crew-dead-letter-coverage.test.ts @@ -88,6 +88,27 @@ describe('guardrail: every cached crew table is bounded', () => { ].join('\n')).toEqual([]) }) + it('both outboxes are covered by BOTH the dead-letter and the stalled surface', () => { + // A transport failure deliberately never sets `failed` — losing a crew + // member's work because their signal is bad would be worse than the bug + // that rule creates. But the drain stops at a blocked head, so the work + // queues up invisibly, which is what the amber stalled notice exists to + // say. It only ever queried db.mutations: a whole shift of photos could + // retry forever against a captive portal with nothing on screen, on a + // banner whose own header comment claims it covers "every queued photo". + for (const table of ['mutations', 'pending_photo_uploads']) { + expect( + new RegExp(`db\\.${table}[\\s\\S]{0,80}?failed`).test(BANNER_SRC), + `${table} has no dead-letter query in the failed-sync banner`, + ).toBe(true) + expect( + new RegExp(`db\\.${table}[\\s\\S]{0,200}?STALLED_NETWORK_ATTEMPTS`).test(BANNER_SRC), + `${table} has no stalled-queue query in the failed-sync banner — a transport ` + + 'failure there never dead-letters, so this is its ONLY visible surface', + ).toBe(true) + } + }) + it('every RECONCILED_AT_PULL claim is backed by a real bulkDelete in that file', () => { for (const [table, file] of Object.entries(RECONCILED_AT_PULL)) { const src = readFileSync(join(ROOT, file), 'utf8')