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.

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<void> {
     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<number, number>()
     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<string, unknown>)
               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<number> {
+    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
   },
 }