From 4188f67ab73b874d6fa9a44057dd9451176aa7cd Mon Sep 17 00:00:00 2001 From: jubnl <66769052+jubnl@users.noreply.github.com> Date: Mon, 15 Jun 2026 07:51:52 +0200 Subject: [PATCH] fix(sync): remap temp ids, prevent id collisions, surface failed mutations (#1175) Closes three offline BLOCKERs from the PWA audit: - B1: offline edits/deletes of an offline-created entity were lost. The negative temp id was baked into the PUT/DELETE url and never rewritten after the CREATE returned a real id, so dependents 404'd and were dropped. Dependents now carry a {id} placeholder + tempEntityId; flush builds a tempId->realId map and durably rewrites still-queued dependents on CREATE success (survives flush boundaries / reloads). - B2: tempId = -(Date.now()) collided within a millisecond, overwriting an optimistic row. Replaced with a monotonic nextTempId() minter. - B3: any 4xx marked the mutation failed with no rollback and no signal, and the badge ignored failed rows. Terminal failures now roll back the phantom optimistic CREATE; 401/408/425/429 are treated as retryable; failedCount() is surfaced in OfflineBanner (red pill) and OfflineTab. --- .../components/Layout/OfflineBanner.test.tsx | 42 +++++ .../src/components/Layout/OfflineBanner.tsx | 40 ++-- client/src/components/Settings/OfflineTab.tsx | 11 +- client/src/db/offlineDb.ts | 6 + client/src/repo/packingRepo.ts | 13 +- client/src/repo/placeRepo.ts | 17 +- client/src/sync/mutationQueue.ts | 95 +++++++++- client/tests/unit/sync/mutationQueue.test.ts | 177 +++++++++++++++++- 8 files changed, 366 insertions(+), 35 deletions(-) create mode 100644 client/src/components/Layout/OfflineBanner.test.tsx diff --git a/client/src/components/Layout/OfflineBanner.test.tsx b/client/src/components/Layout/OfflineBanner.test.tsx new file mode 100644 index 00000000..3df45ba3 --- /dev/null +++ b/client/src/components/Layout/OfflineBanner.test.tsx @@ -0,0 +1,42 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { screen, waitFor } from '@testing-library/react' +import { render } from '../../../tests/helpers/render' +import OfflineBanner from './OfflineBanner' + +vi.mock('../../sync/mutationQueue', () => ({ + mutationQueue: { + pendingCount: vi.fn(), + failedCount: vi.fn(), + }, +})) + +import { mutationQueue } from '../../sync/mutationQueue' + +const pendingCount = mutationQueue.pendingCount as ReturnType +const failedCount = mutationQueue.failedCount as ReturnType + +afterEach(() => { + vi.clearAllMocks() + Object.defineProperty(navigator, 'onLine', { value: true, writable: true, configurable: true }) +}) + +describe('OfflineBanner (B3 surface)', () => { + it('shows the failed pill when failedCount > 0 while online', async () => { + pendingCount.mockResolvedValue(0) + failedCount.mockResolvedValue(2) + + render() + + expect(await screen.findByText(/2 changes failed to sync/i)).toBeInTheDocument() + }) + + it('stays hidden when online with nothing pending or failed', async () => { + pendingCount.mockResolvedValue(0) + failedCount.mockResolvedValue(0) + + const { container } = render() + // Give the async poll a tick to resolve. + await waitFor(() => expect(failedCount).toHaveBeenCalled()) + expect(container.querySelector('[role="status"]')).toBeNull() + }) +}) diff --git a/client/src/components/Layout/OfflineBanner.tsx b/client/src/components/Layout/OfflineBanner.tsx index 34b57e18..c70db013 100644 --- a/client/src/components/Layout/OfflineBanner.tsx +++ b/client/src/components/Layout/OfflineBanner.tsx @@ -2,6 +2,7 @@ * OfflineBanner — connectivity + sync state indicator. * * States: + * N failed → red pill "N changes failed to sync" (takes priority) * offline + N queued → amber pill "Offline · N queued" * offline + 0 queued → amber pill "Offline" * online + N pending → blue pill "Syncing N…" @@ -12,7 +13,7 @@ * headers. On mobile it hovers just above the bottom tab bar. */ import React, { useState, useEffect } from 'react' -import { WifiOff, RefreshCw } from 'lucide-react' +import { WifiOff, RefreshCw, AlertTriangle } from 'lucide-react' import { mutationQueue } from '../../sync/mutationQueue' const POLL_MS = 3_000 @@ -20,6 +21,7 @@ const POLL_MS = 3_000 export default function OfflineBanner(): React.ReactElement | null { const [isOnline, setIsOnline] = useState(navigator.onLine) const [pendingCount, setPendingCount] = useState(0) + const [failedCount, setFailedCount] = useState(0) useEffect(() => { const onOnline = () => setIsOnline(true) @@ -35,26 +37,36 @@ export default function OfflineBanner(): React.ReactElement | null { useEffect(() => { let cancelled = false async function poll() { - const n = await mutationQueue.pendingCount() - if (!cancelled) setPendingCount(n) + const [n, failed] = await Promise.all([ + mutationQueue.pendingCount(), + mutationQueue.failedCount(), + ]) + if (!cancelled) { + setPendingCount(n) + setFailedCount(failed) + } } poll() const id = setInterval(poll, POLL_MS) return () => { cancelled = true; clearInterval(id) } }, []) - const hidden = isOnline && pendingCount === 0 + const hidden = isOnline && pendingCount === 0 && failedCount === 0 if (hidden) return null const offline = !isOnline - const bg = offline ? '#92400e' : '#1e40af' + // Failed mutations are the most important signal — they mean data was dropped. + const failed = failedCount > 0 + const bg = failed ? '#b91c1c' : offline ? '#92400e' : '#1e40af' const text = '#fff' - const label = offline - ? pendingCount > 0 - ? `Offline · ${pendingCount} queued` - : 'Offline' - : `Syncing ${pendingCount}…` + const label = failed + ? `${failedCount} change${failedCount !== 1 ? 's' : ''} failed to sync` + : offline + ? pendingCount > 0 + ? `Offline · ${pendingCount} queued` + : 'Offline' + : `Syncing ${pendingCount}…` return (
- {offline - ? - : + {failed + ? + : offline + ? + : } {label}
diff --git a/client/src/components/Settings/OfflineTab.tsx b/client/src/components/Settings/OfflineTab.tsx index e935fea9..baa49ea2 100644 --- a/client/src/components/Settings/OfflineTab.tsx +++ b/client/src/components/Settings/OfflineTab.tsx @@ -21,6 +21,7 @@ interface CachedTripRow { export default function OfflineTab(): React.ReactElement { const [rows, setRows] = useState([]) const [pendingCount, setPendingCount] = useState(0) + const [failedCount, setFailedCount] = useState(0) const [syncing, setSyncing] = useState(false) const [clearing, setClearing] = useState(false) const [loading, setLoading] = useState(true) @@ -28,11 +29,13 @@ export default function OfflineTab(): React.ReactElement { const load = useCallback(async () => { setLoading(true) try { - const [metas, pending] = await Promise.all([ + const [metas, pending, failed] = await Promise.all([ offlineDb.syncMeta.toArray(), mutationQueue.pendingCount(), + mutationQueue.failedCount(), ]) setPendingCount(pending) + setFailedCount(failed) const result: CachedTripRow[] = [] for (const meta of metas) { @@ -85,6 +88,7 @@ export default function OfflineTab(): React.ReactElement {
+ {failedCount > 0 && }
{/* Actions */} @@ -165,13 +169,14 @@ export default function OfflineTab(): React.ReactElement { ) } -function Stat({ label, value }: { label: string; value: number }) { +function Stat({ label, value, danger }: { label: string; value: number; danger?: boolean }) { return (
-
{value}
+
{value}
{label}
) diff --git a/client/src/db/offlineDb.ts b/client/src/db/offlineDb.ts index 308fcb85..e94ceba1 100644 --- a/client/src/db/offlineDb.ts +++ b/client/src/db/offlineDb.ts @@ -27,6 +27,12 @@ export interface QueuedMutation { tempId?: number; /** For DELETE mutations: the entity id to remove from Dexie on flush */ entityId?: number; + /** + * For PUT/DELETE enqueued offline against a still-unsynced (negative-id) entity: + * the temp id of the target. The url carries an `{id}` placeholder that the + * mutation queue rewrites to the real server id once the dependent CREATE flushes. + */ + tempEntityId?: number; } export interface SyncMeta { diff --git a/client/src/repo/packingRepo.ts b/client/src/repo/packingRepo.ts index 42051b7b..955f7dae 100644 --- a/client/src/repo/packingRepo.ts +++ b/client/src/repo/packingRepo.ts @@ -1,6 +1,6 @@ import { packingApi } from '../api/client' import { offlineDb, upsertPackingItems } from '../db/offlineDb' -import { mutationQueue, generateUUID } from '../sync/mutationQueue' +import { mutationQueue, generateUUID, nextTempId } from '../sync/mutationQueue' import type { PackingItem } from '../types' export const packingRepo = { @@ -19,7 +19,7 @@ export const packingRepo = { async create(tripId: number | string, data: Record & { name: string }): Promise<{ item: PackingItem }> { if (!navigator.onLine) { - const tempId = -(Date.now()) + const tempId = nextTempId() const tempItem: PackingItem = { ...(data as Partial), id: tempId, @@ -51,13 +51,16 @@ export const packingRepo = { const optimistic: PackingItem = { ...(existing ?? {} as PackingItem), ...(data as Partial), id } await offlineDb.packingItems.put(optimistic) const mutId = generateUUID() + const isTemp = id < 0 await mutationQueue.enqueue({ id: mutId, tripId: Number(tripId), method: 'PUT', - url: `/trips/${tripId}/packing/${id}`, + url: isTemp ? `/trips/${tripId}/packing/{id}` : `/trips/${tripId}/packing/${id}`, body: data, resource: 'packingItems', + entityId: id, + ...(isTemp ? { tempEntityId: id } : {}), }) return { item: optimistic } } @@ -70,14 +73,16 @@ export const packingRepo = { if (!navigator.onLine) { await offlineDb.packingItems.delete(id) const mutId = generateUUID() + const isTemp = id < 0 await mutationQueue.enqueue({ id: mutId, tripId: Number(tripId), method: 'DELETE', - url: `/trips/${tripId}/packing/${id}`, + url: isTemp ? `/trips/${tripId}/packing/{id}` : `/trips/${tripId}/packing/${id}`, body: undefined, resource: 'packingItems', entityId: id, + ...(isTemp ? { tempEntityId: id } : {}), }) return { success: true } } diff --git a/client/src/repo/placeRepo.ts b/client/src/repo/placeRepo.ts index 8f4ee28a..a81088a6 100644 --- a/client/src/repo/placeRepo.ts +++ b/client/src/repo/placeRepo.ts @@ -1,6 +1,6 @@ import { placesApi } from '../api/client' import { offlineDb, upsertPlaces } from '../db/offlineDb' -import { mutationQueue, generateUUID } from '../sync/mutationQueue' +import { mutationQueue, generateUUID, nextTempId } from '../sync/mutationQueue' import type { Place } from '../types' export const placeRepo = { @@ -19,7 +19,7 @@ export const placeRepo = { async create(tripId: number | string, data: Record & { name: string }): Promise<{ place: Place }> { if (!navigator.onLine) { - const tempId = -(Date.now()) + const tempId = nextTempId() const tempPlace: Place = { ...(data as Partial), id: tempId, @@ -50,13 +50,16 @@ export const placeRepo = { const optimistic: Place = { ...(existing ?? {} as Place), ...(data as Partial), id: Number(id) } await offlineDb.places.put(optimistic) const mutId = generateUUID() + const isTemp = Number(id) < 0 await mutationQueue.enqueue({ id: mutId, tripId: Number(tripId), method: 'PUT', - url: `/trips/${tripId}/places/${id}`, + url: isTemp ? `/trips/${tripId}/places/{id}` : `/trips/${tripId}/places/${id}`, body: data, resource: 'places', + entityId: Number(id), + ...(isTemp ? { tempEntityId: Number(id) } : {}), }) return { place: optimistic } } @@ -69,14 +72,16 @@ export const placeRepo = { if (!navigator.onLine) { await offlineDb.places.delete(Number(id)) const mutId = generateUUID() + const isTemp = Number(id) < 0 await mutationQueue.enqueue({ id: mutId, tripId: Number(tripId), method: 'DELETE', - url: `/trips/${tripId}/places/${id}`, + url: isTemp ? `/trips/${tripId}/places/{id}` : `/trips/${tripId}/places/${id}`, body: undefined, resource: 'places', entityId: Number(id), + ...(isTemp ? { tempEntityId: Number(id) } : {}), }) return { success: true } } @@ -90,14 +95,16 @@ export const placeRepo = { await offlineDb.places.bulkDelete(ids) for (const id of ids) { const mutId = generateUUID() + const isTemp = id < 0 await mutationQueue.enqueue({ id: mutId, tripId: Number(tripId), method: 'DELETE', - url: `/trips/${tripId}/places/${id}`, + url: isTemp ? `/trips/${tripId}/places/{id}` : `/trips/${tripId}/places/${id}`, body: undefined, resource: 'places', entityId: id, + ...(isTemp ? { tempEntityId: id } : {}), }) } return { deleted: ids, count: ids.length } diff --git a/client/src/sync/mutationQueue.ts b/client/src/sync/mutationQueue.ts index 0b68826c..34cc80a5 100644 --- a/client/src/sync/mutationQueue.ts +++ b/client/src/sync/mutationQueue.ts @@ -39,6 +39,27 @@ let _flushing = false // Monotonically increasing timestamp so same-millisecond enqueues // still get a deterministic FIFO order when sorted by createdAt. let _lastTs = 0 +// Monotonic counter for offline temp ids. Date.now() alone collides when two +// creates land in the same millisecond (bulk import, rapid tapping), which would +// overwrite one optimistic Dexie row. This guarantees distinct negative ids. +let _lastTempId = 0 + +/** + * Mint a collision-free temporary (negative) id for an offline-created entity. + * Monotonic across the session so same-millisecond creates never collide. + */ +export function nextTempId(): number { + const now = Date.now() + _lastTempId = now > _lastTempId ? now : _lastTempId + 1 + return -_lastTempId +} + +/** HTTP statuses that should be retried later rather than treated as terminal. */ +function isRetryableStatus(status: number | undefined): boolean { + // 401: token expired mid-flush (offline window) — retry after re-auth. + // 408/425/429: timeout / too-early / rate-limited — transient. + return status === 401 || status === 408 || status === 425 || status === 429 +} export const mutationQueue = { /** @@ -69,6 +90,10 @@ export const mutationQueue = { async flush(): Promise { if (_flushing || !navigator.onLine) return _flushing = true + // tempId → realId learned during this flush, so a dependent edit/delete + // queued against an offline-created entity (still holding the negative id) + // can be rewritten to the server id before it is replayed. + const idMap = new Map() try { const pending = await offlineDb.mutationQueue .where('status') @@ -79,10 +104,32 @@ export const mutationQueue = { // Mark as syncing so UI can show progress await offlineDb.mutationQueue.update(mutation.id, { status: 'syncing' }) + // Resolve a temp-id reference now that earlier CREATEs in this flush + // may have completed (FIFO order guarantees the CREATE ran first). + let reqUrl = mutation.url + let reqEntityId = mutation.entityId + if (mutation.tempEntityId !== undefined) { + const realId = idMap.get(mutation.tempEntityId) + if (realId !== undefined) { + reqUrl = reqUrl.replace('{id}', String(realId)) + reqEntityId = realId + } + } + // Placeholder still unresolved → the create it depended on is gone + // (failed or missing). Surface it as failed rather than firing a 404. + if (reqUrl.includes('{id}')) { + await offlineDb.mutationQueue.update(mutation.id, { + status: 'failed', + attempts: mutation.attempts + 1, + lastError: 'unresolved temp id (dependent create did not sync)', + }) + continue + } + try { const response = await apiClient.request({ method: mutation.method, - url: mutation.url, + url: reqUrl, data: mutation.body, headers: { 'X-Idempotency-Key': mutation.id }, }) @@ -95,31 +142,51 @@ export const mutationQueue = { const values = Object.values(response.data as Record) const entity = values[0] if (entity && typeof entity === 'object' && 'id' in entity) { - // Remove temp optimistic entry if id changed (CREATE case) - if (mutation.tempId !== undefined && mutation.tempId !== (entity as { id: number }).id) { + const realId = (entity as { id: number }).id + // Remove temp optimistic entry if id changed (CREATE case) and + // remap any queued mutations that still target the negative id. + if (mutation.tempId !== undefined && mutation.tempId !== realId) { await table.delete(mutation.tempId) + idMap.set(mutation.tempId, realId) + // Durable rewrite so dependents survive a flush boundary / reload. + await offlineDb.mutationQueue + .where('tripId') + .equals(mutation.tripId) + .filter(m => m.tempEntityId === mutation.tempId) + .modify(m => { + m.url = m.url.replace('{id}', String(realId)) + m.entityId = realId + m.tempEntityId = undefined + }) } await table.put(entity) } } - } else if (mutation.method === 'DELETE' && mutation.resource && mutation.entityId !== undefined) { + } else if (mutation.method === 'DELETE' && mutation.resource && reqEntityId !== undefined) { // DELETE was already applied optimistically; ensure it's gone const table = getTable(mutation.resource) - if (table) await table.delete(mutation.entityId) + if (table) await table.delete(reqEntityId) } await offlineDb.mutationQueue.delete(mutation.id) } catch (err: unknown) { const httpStatus = (err as { response?: { status: number } })?.response?.status - if (httpStatus !== undefined && httpStatus >= 400 && httpStatus < 500) { - // Permanent client error — mark failed, continue with next + const isTerminal = + httpStatus !== undefined && httpStatus >= 400 && httpStatus < 500 && !isRetryableStatus(httpStatus) + if (isTerminal) { + // Permanent client error — roll back the phantom optimistic CREATE so + // it can't masquerade as synced, then mark failed and continue. + if (mutation.method !== 'DELETE' && mutation.tempId !== undefined && mutation.resource) { + const table = getTable(mutation.resource) + if (table) await table.delete(mutation.tempId) + } await offlineDb.mutationQueue.update(mutation.id, { status: 'failed', attempts: mutation.attempts + 1, lastError: String(err), }) } else { - // Network error — reset to pending, abort flush (retry on next trigger) + // Network / transient error — reset to pending, abort flush (retry next trigger) await offlineDb.mutationQueue.update(mutation.id, { status: 'pending', attempts: mutation.attempts + 1, @@ -160,9 +227,19 @@ export const mutationQueue = { .count() }, - /** Reset internal flushing flag and timestamp counter — useful in tests. */ + /** Count permanently-failed mutations (surfaced separately so the user knows + * changes were dropped — they are NOT folded into pendingCount). */ + async failedCount(): Promise { + return offlineDb.mutationQueue + .where('status') + .equals('failed') + .count() + }, + + /** Reset internal flushing flag and timestamp counters — useful in tests. */ _resetFlushing(): void { _flushing = false _lastTs = 0 + _lastTempId = 0 }, } diff --git a/client/tests/unit/sync/mutationQueue.test.ts b/client/tests/unit/sync/mutationQueue.test.ts index 6a473bb7..ff4b5041 100644 --- a/client/tests/unit/sync/mutationQueue.test.ts +++ b/client/tests/unit/sync/mutationQueue.test.ts @@ -8,8 +8,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import 'fake-indexeddb/auto'; import { server } from '../../helpers/msw/server'; import { http, HttpResponse } from 'msw'; -import { mutationQueue, generateUUID } from '../../../src/sync/mutationQueue'; +import { mutationQueue, generateUUID, nextTempId } from '../../../src/sync/mutationQueue'; import { offlineDb, clearAll } from '../../../src/db/offlineDb'; +import { placeRepo } from '../../../src/repo/placeRepo'; import { buildPlace, buildPackingItem } from '../../helpers/factories'; beforeEach(async () => { @@ -265,3 +266,177 @@ describe('mutationQueue.pendingCount', () => { expect(await mutationQueue.pendingCount()).toBe(2); }); }); + +describe('mutationQueue.failedCount', () => { + it('counts only failed mutations (not pending/syncing)', async () => { + const id1 = generateUUID(); + const id2 = generateUUID(); + await mutationQueue.enqueue(makeMutation({ id: id1 })); + await mutationQueue.enqueue(makeMutation({ id: id2 })); + await offlineDb.mutationQueue.update(id2, { status: 'failed' }); + + expect(await mutationQueue.failedCount()).toBe(1); + expect(await mutationQueue.pendingCount()).toBe(1); + }); +}); + +// ── B2: collision-free temp ids ──────────────────────────────────────────────── + +describe('nextTempId (B2)', () => { + it('returns distinct negative ids even within the same millisecond', () => { + mutationQueue._resetFlushing(); + const a = nextTempId(); + const b = nextTempId(); + const c = nextTempId(); + expect(a).toBeLessThan(0); + expect(new Set([a, b, c]).size).toBe(3); + }); + + it('two tight offline creates produce two distinct Dexie rows', async () => { + Object.defineProperty(navigator, 'onLine', { value: false }); + await placeRepo.create(1, { name: 'First' }); + await placeRepo.create(1, { name: 'Second' }); + + const rows = await offlineDb.places.where('trip_id').equals(1).toArray(); + expect(rows).toHaveLength(2); + expect(rows.map(r => r.name).sort()).toEqual(['First', 'Second']); + }); +}); + +// ── B1: temp-id → real-id remapping ───────────────────────────────────────────── + +describe('mutationQueue.flush — temp-id remapping (B1)', () => { + it('rewrites a dependent PUT/DELETE to the real id within one flush', async () => { + const tempId = -1; + await offlineDb.places.put({ ...buildPlace({ trip_id: 1 }), id: tempId }); + + const createId = generateUUID(); + const putId = generateUUID(); + const deleteId = generateUUID(); + + await mutationQueue.enqueue({ + id: createId, tripId: 1, method: 'POST', url: '/trips/1/places', + body: { name: 'Temp' }, resource: 'places', tempId, + }); + await mutationQueue.enqueue({ + id: putId, tripId: 1, method: 'PUT', url: '/trips/1/places/{id}', + body: { name: 'Edited' }, resource: 'places', entityId: tempId, tempEntityId: tempId, + }); + await mutationQueue.enqueue({ + id: deleteId, tripId: 1, method: 'DELETE', url: '/trips/1/places/{id}', + body: undefined, resource: 'places', entityId: tempId, tempEntityId: tempId, + }); + + const putUrls: string[] = []; + const deleteUrls: string[] = []; + server.use( + http.post('/api/trips/1/places', () => HttpResponse.json({ place: buildPlace({ trip_id: 1, id: 42 }) })), + http.put('/api/trips/1/places/:id', ({ params }) => { putUrls.push(String(params.id)); return HttpResponse.json({ place: buildPlace({ trip_id: 1, id: 42, name: 'Edited' }) }); }), + http.delete('/api/trips/1/places/:id', ({ params }) => { deleteUrls.push(String(params.id)); return HttpResponse.json({ success: true }); }), + ); + + await mutationQueue.flush(); + + expect(putUrls).toEqual(['42']); + expect(deleteUrls).toEqual(['42']); + expect(await mutationQueue.pendingCount()).toBe(0); + expect(await mutationQueue.failedCount()).toBe(0); + }); + + it('durably rewrites a still-queued dependent after the CREATE flushes alone', async () => { + const tempId = -7; + await offlineDb.places.put({ ...buildPlace({ trip_id: 1 }), id: tempId }); + + const createId = generateUUID(); + const putId = generateUUID(); + await mutationQueue.enqueue({ + id: createId, tripId: 1, method: 'POST', url: '/trips/1/places', + body: { name: 'Temp' }, resource: 'places', tempId, + }); + await mutationQueue.enqueue({ + id: putId, tripId: 1, method: 'PUT', url: '/trips/1/places/{id}', + body: { name: 'Edited' }, resource: 'places', entityId: tempId, tempEntityId: tempId, + }); + + // Only the CREATE succeeds this round; the PUT errors out (network) and stays queued. + let putAttempts = 0; + server.use( + http.post('/api/trips/1/places', () => HttpResponse.json({ place: buildPlace({ trip_id: 1, id: 88 }) })), + http.put('/api/trips/1/places/:id', () => { putAttempts++; return HttpResponse.error(); }), + ); + + await mutationQueue.flush(); + + const queuedPut = await offlineDb.mutationQueue.get(putId); + expect(queuedPut).toBeDefined(); + expect(queuedPut!.url).toBe('/trips/1/places/88'); + expect(queuedPut!.entityId).toBe(88); + expect(queuedPut!.tempEntityId).toBeUndefined(); + expect(putAttempts).toBeGreaterThanOrEqual(1); + }); + + it('marks an orphaned dependent (placeholder never resolved) as failed', async () => { + const putId = generateUUID(); + await mutationQueue.enqueue({ + id: putId, tripId: 1, method: 'PUT', url: '/trips/1/places/{id}', + body: { name: 'Edited' }, resource: 'places', entityId: -999, tempEntityId: -999, + }); + + await mutationQueue.flush(); + + const m = await offlineDb.mutationQueue.get(putId); + expect(m!.status).toBe('failed'); + }); +}); + +// ── B3: terminal rollback + retryable classification ──────────────────────────── + +describe('mutationQueue.flush — failure handling (B3)', () => { + it('rolls back the phantom optimistic row on a terminal 400 CREATE', async () => { + const tempId = -3; + await offlineDb.places.put({ ...buildPlace({ trip_id: 1 }), id: tempId }); + + const id = generateUUID(); + await mutationQueue.enqueue(makeMutation({ id, tempId })); + + server.use( + http.post('/api/trips/1/places', () => HttpResponse.json({ error: 'Bad' }, { status: 400 })), + ); + + await mutationQueue.flush(); + + expect(await offlineDb.places.get(tempId)).toBeUndefined(); + const m = await offlineDb.mutationQueue.get(id); + expect(m!.status).toBe('failed'); + }); + + it('treats 429 as retryable: resets to pending and stops the flush', async () => { + const id = generateUUID(); + await mutationQueue.enqueue(makeMutation({ id })); + + server.use( + http.post('/api/trips/1/places', () => HttpResponse.json({ error: 'slow down' }, { status: 429 })), + ); + + await mutationQueue.flush(); + + const m = await offlineDb.mutationQueue.get(id); + expect(m!.status).toBe('pending'); + expect(m!.attempts).toBe(1); + expect(await mutationQueue.failedCount()).toBe(0); + }); + + it('treats 401 as retryable rather than dropping the change', async () => { + const id = generateUUID(); + await mutationQueue.enqueue(makeMutation({ id })); + + server.use( + http.post('/api/trips/1/places', () => HttpResponse.json({ error: 'AUTH_REQUIRED' }, { status: 401 })), + ); + + await mutationQueue.flush(); + + const m = await offlineDb.mutationQueue.get(id); + expect(m!.status).toBe('pending'); + }); +});