fix(crew-sync): unblock the outbox on server timeouts; keep retry ord…

app/crew/_components/failed-sync-banner.tsx

@@ -24,6 +24,7 @@
 import type { MutationTable } from '@/lib/dexie/schema'
 import { retryAllFailedMutations, discardFailedMutation } from '@/lib/dexie/helpers'
 import { retryFailedPhotoUploads, discardPendingPhoto } from '@/lib/dexie/photo-sync'
+import { STALLED_NETWORK_ATTEMPTS } from '@/lib/dexie/net'
 import { Button } from '@/components/ui/Button'
 import { Badge } from '@/components/ui/Badge'
 import { Dialog } from '@/components/ui/Dialog'
@@ -67,6 +68,21 @@
     [],
   ) ?? []
 
+  // Transport failures deliberately never dead-letter — losing a crew
+  // member's work because their signal is bad would be worse than the bug
+  // this surfaces. But the drain STOPS at a blocked head, so every later
+  // change on the device queues behind it. Previously that state was
+  // completely invisible: `failed` is never set on the network path, this
+  // banner filters on `failed`, and the only trace anywhere was the pending
+  // count in the logout dialog. A crew member could work a whole shift, sync
+  // nothing, and find out at logout.
+  const stalledMutations = useLiveQuery(
+    () => db.mutations
+      .filter((m) => !m.failed && (m.networkRetryCount ?? 0) >= STALLED_NETWORK_ATTEMPTS)
+      .toArray(),
+    [],
+  ) ?? []
+
   const entries: FailedEntry[] = [
     ...failedMutations.map((m) => ({
       key:     `mutation-${m.id}`,
@@ -82,7 +98,34 @@
     })),
   ]
 
-  if (entries.length === 0) return null
+  if (entries.length === 0 && stalledMutations.length === 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 && (
+    <div
+      className="mx-4 mt-3 rounded-xl p-4"
+      style={{ background: 'var(--accent-amber-dim)', border: '1px solid var(--accent-amber-dim)' }}
+      role="status"
+    >
+      <div className="flex items-start gap-3">
+        <AlertTriangle className="w-5 h-5 shrink-0 mt-0.5" style={{ color: 'var(--accent-amber)' }} />
+        <div className="flex-1 min-w-0">
+          <p className="text-sm font-bold" style={{ color: 'var(--accent-amber)' }}>
+            {stalledMutations.length} change{stalledMutations.length !== 1 ? 's' : ''} still trying to sync
+          </p>
+          <p className="text-xs mt-0.5" style={{ color: 'var(--text-secondary)' }}>
+            Your work is saved on this phone and will keep retrying on its own.
+            If this stays here, move somewhere with better signal before you
+            finish for the day.
+          </p>
+        </div>
+      </div>
+    </div>
+  )
+
+  if (entries.length === 0) return <>{stalledNotice}</>
 
   const retryAll = async () => {
     setRetrying(true)
@@ -96,6 +139,7 @@
 
   return (
     <>
+      {stalledNotice}
       <div
         className="mx-4 mt-3 rounded-xl p-4"
         style={{ background: 'var(--accent-red-dim)', border: '1px solid var(--accent-red-dim)' }}

lib/dexie/helpers.ts

@@ -166,7 +166,12 @@ export async function retryFailedMutation(
  */
 export async function retryAllFailedMutations(userId: string): Promise<void> {
   const db = getDexieDb(userId)
-  const failed = (await db.mutations.toArray()).filter((m) => !!m.failed)
+  // orderBy('id') — NOT a bare toArray(). The drain replays in id order, and
+  // SyncEngine.holdBackSuccessors() deliberately dead-letters a record's whole
+  // 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)
 
   for (const mutation of failed) {
     await db.mutations.update(mutation.id!, {

lib/dexie/net.ts

@@ -110,12 +110,46 @@ function isTerminalDataCode(code: string): boolean {
 export function classifyUploadFailure(err: unknown): UploadFailureKind {
   if (!isOnline()) return 'network'
   if (err instanceof UploadHttpError) return classifyHttpStatus(err.status)
-  if (err instanceof UploadDataError && err.code && isTerminalDataCode(err.code)) return 'terminal'
+
+  // A DATA error carrying a Postgres/PostgREST code demonstrably REACHED the
+  // server — the server is what produced the code. It is therefore terminal or
+  // transient, and must never fall through to the transport-message test
+  // below.
+  //
+  // That fall-through was a real trap. A Postgres statement timeout arrives as
+  // UploadDataError('… canceling statement due to statement timeout', '57014').
+  // 57014 is not 22/23/42 and not PGRST, so it was not terminal; the message
+  // then matched \btimeout\b and it was classified 'network'. The network
+  // branch in SyncEngine.handleFailure never consumes a retry, never sets
+  // `failed`, and stops the drain — so one server-side timeout pinned the head
+  // of the outbox forever and blocked every later write on the device, while
+  // FailedSyncBanner (which filters on `failed`) showed nothing at all.
+  //
+  // Classified 'transient': it may well succeed on replay, but it now spends
+  // the retry budget and eventually dead-letters into a surface a crew member
+  // can see.
+  if (err instanceof UploadDataError && err.code) {
+    return isTerminalDataCode(err.code) ? 'terminal' : 'transient'
+  }
+
   if (err instanceof TypeError) return 'network'
   if (NETWORK_MESSAGE_PATTERN.test(messageOf(err))) return 'network'
   return 'transient'
 }
 
+/**
+ * How many consecutive transport failures — all while the device reported
+ * itself ONLINE — before the outbox is treated as stalled and surfaced.
+ *
+ * Transport failures deliberately never dead-letter: discarding a crew
+ * member's work because their connection is bad would be far worse than the
+ * bug this bounds. But an unbounded, silent retry loop is its own failure —
+ * the drain stops at the blocked head, so every later write on the device
+ * queues behind it invisibly. Past this threshold the mutation stays queued
+ * and keeps retrying; it simply stops being invisible.
+ */
+export const STALLED_NETWORK_ATTEMPTS = 5
+
 // navigator.locks is unavailable in non-secure contexts, in workers on some
 // engines, and in the jsdom/node test environment. Declared narrowly here
 // rather than relying on the ambient DOM lib so the fallback path is

lib/dexie/syncService.ts

@@ -41,6 +41,11 @@ 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
+  // 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
+  // pushing a row that was marked failed after the snapshot was taken.
+  private readonly heldBack = new Set<number>()
   private disposed = false
   // Single pending wake-up for a drain that stopped on a not-yet-due
   // mutation. One handle only — scheduleRetry() clears any previous timer
@@ -142,10 +147,18 @@ export class SyncEngine {
     const db = getDexieDb(this.userId)
     const pending = (await db.mutations.orderBy('id').toArray()).filter((m) => !m.failed)
 
+    // `pending` is a SNAPSHOT taken before the loop. holdBackSuccessors() can
+    // mark rows failed part-way through it, and the loop would otherwise push
+    // them anyway — sending exactly the write that was just held back to
+    // preserve ordering, which defeats the whole point.
+    this.heldBack.clear()
+
     for (const mutation of pending) {
       // Auto-incrementing key — always populated once read back from the table.
       const id = mutation.id as number
 
+      if (this.heldBack.has(id)) 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.
@@ -187,6 +200,49 @@ export class SyncEngine {
     }
   }
 
+  /**
+   * Marks every still-queued mutation for the SAME record as failed, once one
+   * of them dead-letters.
+   *
+   * Without this, dead-lettering silently drops one write out of the middle of
+   * a record's sequence: the drain moves on, later writes for that record land
+   * on the server, and a subsequent "Retry all" replays the stale one on top
+   * of them. Holding the successors back keeps the sequence intact so a retry
+   * re-applies it in the order the crew member performed it.
+   *
+   * They carry a distinct lastError so the banner does not tell a crew member
+   * that five separate things failed when one did.
+   */
+  private async holdBackSuccessors(
+    db: FieldStayDexie,
+    mutation: MutationRow,
+    failedId: number,
+  ): Promise<void> {
+    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,
+    )
+
+    if (successors.length === 0) return
+
+    console.warn(
+      `[SyncEngine] holding back ${successors.length} later change(s) to ` +
+      `${mutation.table}/${mutation.targetId} so the retry order is preserved`
+    )
+
+    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',
+      })
+      this.heldBack.add(successorId)
+    }
+  }
+
   /**
    * Records one push failure. Returns true when the drain must stop (so
    * later mutations against the same record can't jump ahead of this one),
@@ -237,8 +293,27 @@ export class SyncEngine {
         failed: true,
         lastError: describeFailure(err),
       })
-      // A mutation that will never succeed must not block every later write
-      // against other records — it is finished, so the drain continues.
+
+      // Dead-lettering breaks this record's mutation ORDER, and nothing used
+      // to put it back. The drain continues past the dead letter (correctly —
+      // other records must not be blocked), so later mutations for the SAME
+      // record push successfully on top of a gap. Then "Retry all" clears the
+      // failed flag in place, the row keeps its original low id, and the drain
+      // replays the stale payload as though it were the newest write.
+      //
+      // Concretely: crew ticks a checklist item (#10) → 500s five times →
+      // dead-letters. They realise it isn't done and un-tick it (#11) → pushes
+      // fine, server now false. They tap Retry all → #10 replays
+      // is_completed = true → the server flips BACK to complete, and the next
+      // delta pull overwrites Dexie too, so the un-tick disappears from the
+      // phone as well. Same shape for inventory_items.current_quantity (an old
+      // count overwriting a corrected one) and crew_availability.
+      //
+      // So: hold back this record's remaining queued mutations too. They keep
+      // their ids, so the sequence is preserved and a retry replays
+      // tick-then-un-tick in the order the crew member actually performed
+      // them. Scoped to (table, targetId) — every other record keeps draining.
+      await this.holdBackSuccessors(db, mutation, id)
       return false
     }