fix(web): replay offline highlights; reading-session queue race

CHANGELOG.md

@@ -26,6 +26,8 @@ the archive; if it broke production, it belongs in `docs/incidents/`. See
 - **Ops** — a deploy waits for any SSG rebuild in progress (≤40 min, only jobs active in the last hour) before the frontend build empties `dist/`; the post-deploy health gate no longer treats ssg-worker's "last rebuild failed" as a broken deploy — infra
 - **Ops** — deploys no longer run a full SSG rebuild (~25 min of CPU, held the only runner): it runs nightly after the backup; `rebuild_ssg` input on a manual deploy for releases that change SEO rendering; health check alarms at 36h stale (was 72h) — infra
 - **Sync** — catalog progress last-write-wins compares the client clock only with itself (new `reading_progresses.client_updated_at`, client stamps clamped to server now + 5 min) instead of the server's `UpdatedAt`, so a device whose clock runs behind no longer has its newer writes silently refused; mobile stamps the time a position was recorded, like web; mobile queues reading sessions in AsyncStorage (cap 50, flushed after each session, on sign-in/app start and on reconnect) instead of dropping a failed submit; queue rules shared with web; edition sessions dedup on resend like user-book ones — backend, web, mobile
+- **Reader** — highlights made while the server is unreachable are no longer dropped: kept visible through the next server load and replayed (create/update/delete) on load and on `online`; a lost-response create is matched by anchor, not re-POSTed; offline deletes leave a tombstone. Reading-session queue flush removes only the sessions it settled, so sessions queued mid-flush survive — web
+
 - **Security** — hardening from the architecture review: stored files are served with a sandbox CSP + nosniff (nginx and the API), `/internal/*` is refused at nginx and its network check is one tested helper, two copyrighted test PDFs removed (image-only test now uses a generated PDF) — backend, infra
 - **Security** — connect keys get the same account-route scope as OAuth tokens; per-IP rate limits group IPv6 by /64; nginx rate-limit zones key on the client address — api, infra
 - **Cleanup** — podcast feature deleted: worker, script builder, ffmpeg audio assembly, `/books/{slug}/podcast` + `/admin/podcasts` endpoints, `podcast.script` route + eval, web book-page player, admin Generate button; `podcast_generation_jobs` dropped (`DropPodcast`), ffmpeg removed from the Worker image; stored mp3s left on disk — backend, admin, web

apps/web/src/hooks/__tests__/useReadingSession.flush.test.ts

@@ -94,4 +94,39 @@ describe('useReadingSession — flushPendingSessions pruning', () => {
     expect(submitSession.mock.calls[0][0].durationSeconds).toBe(27)
     await waitFor(() => expect(localStorage.getItem(KEY) ?? '[]').toBe('[]'))
   })
+
+  it('keeps a session enqueued while the flush is in flight (no clobber on write-back)', async () => {
+    const old = makePendingSession()
+    localStorage.setItem(KEY, JSON.stringify([old]))
+    let resolveSubmit!: () => void
+    submitSession.mockReturnValue(new Promise<void>(r => { resolveSubmit = r }))
+
+    renderHook(() => useReadingSession(opts))
+    await waitFor(() => expect(submitSession).toHaveBeenCalledTimes(1))
+
+    // Another tab / the unload path enqueues while the request is outstanding.
+    const fresh = { ...makePendingSession(), editionId: 'fresh-book' }
+    localStorage.setItem(KEY, JSON.stringify([...JSON.parse(localStorage.getItem(KEY) ?? '[]'), fresh]))
+    resolveSubmit()
+
+    await waitFor(() => expect(JSON.parse(localStorage.getItem(KEY) ?? '[]')).toEqual([fresh]))
+  })
+
+  it('keeps a failed session and one enqueued during the flush', async () => {
+    localStorage.setItem(KEY, JSON.stringify([makePendingSession()]))
+    let rejectSubmit!: (e: Error) => void
+    submitSession.mockReturnValue(new Promise<void>((_, r) => { rejectSubmit = r }))
+
+    renderHook(() => useReadingSession(opts))
+    await waitFor(() => expect(submitSession).toHaveBeenCalledTimes(1))
+
+    const fresh = { ...makePendingSession(), editionId: 'fresh-book' }
+    localStorage.setItem(KEY, JSON.stringify([...JSON.parse(localStorage.getItem(KEY) ?? '[]'), fresh]))
+    rejectSubmit(new Error('network down'))
+
+    await waitFor(() => {
+      const q = JSON.parse(localStorage.getItem(KEY) ?? '[]')
+      expect(q.map((s: { editionId: string }) => s.editionId)).toEqual(['dead-book-id', 'fresh-book'])
+    })
+  })
 })

apps/web/src/hooks/useHighlights.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest'
-import { renderHook, waitFor } from '@testing-library/react'
+import { renderHook, waitFor, act } from '@testing-library/react'
 import type { StoredHighlight } from '../lib/offlineDb'
 
 // Mock the persistence + network layers so the hook mounts without IndexedDB /
@@ -21,6 +21,7 @@ vi.mock('../lib/dataEvents', () => ({ emitDataChange: vi.fn() }))
 
 import { useHighlights } from './useHighlights'
 import * as offlineDb from '../lib/offlineDb'
+import * as userData from '../api/userData'
 
 function pdfHighlight(): StoredHighlight {
   return {
@@ -80,3 +81,65 @@ describe('useHighlights.getHighlightsForRange', () => {
     expect(result.current.getHighlightsForRange(50, 60)).toHaveLength(0) // no overlap
   })
 })
+
+describe('useHighlights offline replay', () => {
+  beforeEach(() => vi.clearAllMocks())
+
+  const serverRow = {
+    id: '3f2b8c1e-1111-4a2b-9c3d-000000000001',
+    editionId: 'ed-1',
+    chapterId: 'ch-1',
+    userBookId: null,
+    userChapterId: null,
+    anchorJson: JSON.stringify(reflowHighlight(10, 20).anchor),
+    color: 'green',
+    selectedText: 'b',
+    noteText: null,
+    version: 1,
+    createdAt: '2026-10-04T00:00:00Z',
+    updatedAt: '2026-10-04T00:00:00Z',
+  }
+  const pending = (): StoredHighlight => ({ ...reflowHighlight(10, 20), id: '1759581234567-abc12de', syncStatus: 'pending' })
+
+  it('keeps a pending offline highlight after server load and POSTs it', async () => {
+    vi.mocked(offlineDb.getHighlightsForEdition).mockResolvedValue([pending()])
+    vi.mocked(userData.getPublicHighlights).mockResolvedValue([])
+    vi.mocked(userData.createPublicHighlight).mockResolvedValue(serverRow)
+
+    const { result } = renderHook(() => useHighlights('ed-1', undefined, { isAuthenticated: true }))
+
+    await waitFor(() => expect(result.current.highlights.map((x) => x.id)).toEqual([serverRow.id]))
+    expect(userData.createPublicHighlight).toHaveBeenCalledTimes(1)
+    expect(offlineDb.saveHighlight).toHaveBeenCalledWith(expect.objectContaining({ id: serverRow.id, syncStatus: 'synced' }))
+    expect(offlineDb.deleteHighlight).toHaveBeenCalledWith(pending().id)
+  })
+
+  it('a failed replay keeps the highlight pending and visible', async () => {
+    vi.mocked(offlineDb.getHighlightsForEdition).mockResolvedValue([pending()])
+    vi.mocked(userData.getPublicHighlights).mockResolvedValue([])
+    vi.mocked(userData.createPublicHighlight).mockRejectedValue(new Error('offline'))
+
+    const { result } = renderHook(() => useHighlights('ed-1', undefined, { isAuthenticated: true }))
+
+    await waitFor(() => expect(result.current.loading).toBe(false))
+    expect(userData.createPublicHighlight).toHaveBeenCalledTimes(1)
+    expect(result.current.highlights.map((x) => x.id)).toEqual([pending().id])
+    expect(offlineDb.deleteHighlight).not.toHaveBeenCalled()
+  })
+
+  it('a delete that cannot reach the server leaves a tombstone, not a resurrection', async () => {
+    vi.mocked(offlineDb.getHighlightsForEdition).mockResolvedValue([{ ...reflowHighlight(10, 20), id: serverRow.id }])
+    vi.mocked(userData.getPublicHighlights).mockResolvedValue([serverRow])
+    vi.mocked(userData.deletePublicHighlight).mockRejectedValue(new Error('offline'))
+
+    const { result } = renderHook(() => useHighlights('ed-1', undefined, { isAuthenticated: true }))
+    await waitFor(() => expect(result.current.loading).toBe(false))
+
+    await act(() => result.current.removeHighlight(serverRow.id))
+
+    expect(result.current.highlights).toEqual([])
+    expect(offlineDb.saveHighlight).toHaveBeenLastCalledWith(
+      expect.objectContaining({ id: serverRow.id, deleted: true, syncStatus: 'pending' })
+    )
+  })
+})

apps/web/src/hooks/useHighlights.ts

@@ -1,4 +1,4 @@
-import { useState, useEffect, useCallback, useRef } from 'react'
+import { useState, useEffect, useCallback, useRef, useMemo } from 'react'
 import { isPdfAnchor } from '@textstack/shared'
 import {
   type StoredHighlight,
@@ -16,7 +16,10 @@ import {
   updatePublicHighlight,
   deletePublicHighlight,
 } from '../api/userData'
+import { ApiError } from '../api/client'
 import { emitDataChange } from '../lib/dataEvents'
+import { planHighlightSync, fromServerHighlight, isLocalHighlightId } from '../lib/highlightSync'
+import { useNetworkRecovery } from './useNetworkRecovery'
 
 function generateId(): string {
   return `${Date.now()}-${Math.random().toString(36).slice(2, 9)}`
@@ -35,6 +38,101 @@ export function useHighlights(editionId?: string, userBookId?: string, options?:
   const bookId = userBookId || editionId || ''
   const isUserBook = !!userBookId
 
+  // Fetch server list, merge with local pending rows (local pending wins and
+  // stays visible), then replay pending creates/updates/deletes. A failed replay
+  // leaves the row pending for the next load / `online` event — never discarded.
+  // Runs are serialized (each starts after the previous finished and re-reads
+  // IndexedDB), so overlapping triggers can't POST the same pending row twice.
+  const syncQueueRef = useRef<Promise<void>>(Promise.resolve())
+  const syncOnce = useCallback(
+    async (isCancelled: () => boolean) => {
+      try {
+        const serverHighlights = isUserBook
+          ? await getUserBookHighlights(bookId)
+          : await getPublicHighlights(bookId)
+        if (isCancelled()) return
+        serverSyncedRef.current = true
+
+        const local = isUserBook
+          ? await getHighlightsForUserBook(bookId)
+          : await getHighlightsForEdition(bookId)
+        const plan = planHighlightSync(serverHighlights.map(fromServerHighlight), local)
+
+        // Reconcile, don't wipe-and-rebuild (see git history: a wipe window
+        // could leave IndexedDB empty on navigation).
+        for (const h of plan.store) await saveHighlight(h)
+        for (const id of plan.drop) await deleteHighlightFromDB(id)
+        if (isCancelled()) return
+        setHighlights(plan.visible)
+
+        let changed = false
+        for (const h of plan.create) {
+          try {
+            const created = fromServerHighlight(
+              await createPublicHighlight(
+                h.userBookId
+                  ? {
+                      userBookId: h.userBookId,
+                      userChapterId: h.userChapterId,
+                      anchorJson: JSON.stringify(h.anchor),
+                      color: h.color,
+                      selectedText: h.selectedText,
+                      noteText: h.noteText,
+                    }
+                  : {
+                      editionId: h.editionId,
+                      chapterId: h.chapterId,
+                      anchorJson: JSON.stringify(h.anchor),
+                      color: h.color,
+                      selectedText: h.selectedText,
+                      noteText: h.noteText,
+                    }
+              )
+            )
+            await saveHighlight(created)
+            await deleteHighlightFromDB(h.id)
+            if (!isCancelled()) setHighlights((prev) => prev.map((p) => (p.id === h.id ? created : p)))
+            changed = true
+          } catch {
+            // stays pending
+          }
+        }
+        for (const h of plan.update) {
+          try {
+            // No `version`: the offline edit is the reader's latest intent (last write wins).
+            const saved = fromServerHighlight(await updatePublicHighlight(h.id, { color: h.color, noteText: h.noteText }))
+            await saveHighlight(saved)
+            if (!isCancelled()) setHighlights((prev) => prev.map((p) => (p.id === h.id ? saved : p)))
+            changed = true
+          } catch (err) {
+            if (err instanceof ApiError && err.status === 404) await deleteHighlightFromDB(h.id)
+          }
+        }
+        for (const { serverId, localId } of plan.remove) {
+          try {
+            await deletePublicHighlight(serverId)
+          } catch (err) {
+            if (!(err instanceof ApiError && err.status === 404)) continue // stays pending
+          }
+          await deleteHighlightFromDB(localId)
+          changed = true
+        }
+        if (changed) emitDataChange('highlights')
+      } catch {
+        // Server unavailable, keep local data.
+      }
+    },
+    [bookId, isUserBook]
+  )
+  const syncWithServer = useCallback(
+    (isCancelled: () => boolean) => {
+      const run = syncQueueRef.current.then(() => syncOnce(isCancelled))
+      syncQueueRef.current = run
+      return run
+    },
+    [syncOnce]
+  )
+
   // Load highlights: IndexedDB first, then server if authenticated.
   // We intentionally do NOT filter by chapter — the scroll reader mounts
   // multiple chapters and every visible chapter needs its highlights.
@@ -51,59 +149,17 @@ export function useHighlights(editionId?: string, userBookId?: string, options?:
       ? getHighlightsForUserBook(bookId)
       : getHighlightsForEdition(bookId)
 
-    loadLocal
+    const localLoaded = loadLocal
       .then((localHighlights) => {
         if (cancelled) return
-        setHighlights(localHighlights)
+        setHighlights(localHighlights.filter((h) => !h.deleted))
       })
       .catch(() => {})
 
     if (isAuthenticated) {
-      const fetchServer = isUserBook
-        ? getUserBookHighlights(bookId)
-        : getPublicHighlights(bookId)
-
-      fetchServer
-        .then(async (serverHighlights) => {
-          if (cancelled) return
-          serverSyncedRef.current = true
-
-          const converted: StoredHighlight[] = serverHighlights.map((sh) => ({
-            id: sh.id,
-            editionId: sh.editionId || '',
-            chapterId: sh.chapterId || '',
-            userBookId: sh.userBookId || undefined,
-            userChapterId: sh.userChapterId || undefined,
-            anchor: JSON.parse(sh.anchorJson) as HighlightAnchor,
-            color: sh.color as HighlightColor,
-            selectedText: sh.selectedText,
-            noteText: sh.noteText ?? undefined,
-            syncStatus: 'synced' as const,
-            version: sh.version,
-            createdAt: new Date(sh.createdAt).getTime(),
-            updatedAt: new Date(sh.updatedAt).getTime(),
-          }))
-
-          // Reconcile, don't wipe-and-rebuild. A prior transient window
-          // between deleteByEdition and saveAll could leave IndexedDB empty
-          // if the user navigated during sync — next cold start would see
-          // zero highlights.
-          const serverIds = new Set(converted.map((h) => h.id))
-          for (const h of converted) await saveHighlight(h)
-          const existingLocal = isUserBook
-            ? await getHighlightsForUserBook(bookId)
-            : await getHighlightsForEdition(bookId)
-          for (const local of existingLocal) {
-            if (!serverIds.has(local.id) && local.syncStatus === 'synced') {
-              await deleteHighlightFromDB(local.id)
-            }
-          }
-
-          if (!cancelled) setHighlights(converted)
-        })
-        .catch(() => {
-          // Server unavailable, keep local data.
-        })
+      // After the local paint, so a slow IndexedDB read can't overwrite the merged list.
+      localLoaded
+        .then(() => syncWithServer(() => cancelled))
         .finally(() => {
           if (!cancelled) setLoading(false)
         })
@@ -114,7 +170,14 @@ export function useHighlights(editionId?: string, userBookId?: string, options?:
     return () => {
       cancelled = true
     }
-  }, [bookId, isAuthenticated, isUserBook])
+  }, [bookId, isAuthenticated, isUserBook, syncWithServer])
+
+  // Back online → replay whatever is still pending.
+  const recoveryOptions = useMemo(
+    () => ({ onOnline: () => { if (isAuthenticated && bookId) void syncWithServer(() => false) } }),
+    [isAuthenticated, bookId, syncWithServer]
+  )
+  useNetworkRecovery(recoveryOptions)
 
   const addHighlight = useCallback(
     async (
@@ -228,20 +291,27 @@ export function useHighlights(editionId?: string, userBookId?: string, options?:
 
   const removeHighlight = useCallback(
     async (id: string) => {
-      // If authenticated, delete from server
-      if (isAuthenticated) {
+      let confirmed = !isAuthenticated
+      // A client-id row never reached the server (as far as we know) — leave a
+      // tombstone so sync can remove its twin if a lost-response create did land.
+      if (isAuthenticated && !isLocalHighlightId(id)) {
         try {
           await deletePublicHighlight(id)
-        } catch {
-          // Server unavailable, continue with local delete
+          confirmed = true
+        } catch (err) {
+          confirmed = err instanceof ApiError && err.status === 404
         }
       }
 
-      await deleteHighlightFromDB(id)
+      const existing = highlights.find((h) => h.id === id)
+      if (confirmed || !existing) await deleteHighlightFromDB(id)
+      // Server unreachable: keep a tombstone so the next sync replays the delete
+      // instead of the server list resurrecting it.
+      else await saveHighlight({ ...existing, deleted: true, syncStatus: 'pending' })
       setHighlights((prev) => prev.filter((h) => h.id !== id))
       emitDataChange('highlights')
     },
-    [isAuthenticated]
+    [highlights, isAuthenticated]
   )
 
   const getHighlightsForRange = useCallback(