fix(pwa): flush pending state before a forced SW-update reload (DA-02)

README.md

@@ -14,7 +14,7 @@
   <img src="https://img.shields.io/badge/Storage-IndexedDB_v8-F59E0B" alt="IndexedDB v8">
   <img src="https://img.shields.io/badge/PWA-v3.0-5BB974?logo=pwa" alt="PWA v3.0">
   <img src="https://img.shields.io/badge/i18n-19_locales-2925_keys-0EA5E9" alt="i18n 19 locales — 2925 keys">
-  <img src="https://img.shields.io/badge/Tests-7138%2B_%2F_583_files-22C55E" alt="7138+ tests / 583 files">
+  <img src="https://img.shields.io/badge/Tests-7157%2B_%2F_587_files-22C55E" alt="7157+ tests / 587 files">
   <img src="https://img.shields.io/codecov/c/github/qnbs/WorldScript-Studio?logo=codecov&label=Coverage" alt="Codecov Coverage">
   <img src="https://img.shields.io/badge/License-MIT-22C55E" alt="License MIT">
   <img src="https://img.shields.io/github/actions/workflow/status/qnbs/WorldScript-Studio/.github/workflows/ci.yml?branch=main&logo=github" alt="CI Status">
@@ -512,7 +512,7 @@ The Settings → AI panel shows a live GPU status badge with adapter details and
 | **Document Export**  | docx + jszip                                              | Word-compatible `.docx` generation (lazy-loaded)                     |
 | **PWA**              | Service Worker + Web App Manifest v3                     | Offline support, installability, Workbox chunking                    |
 | **i18n**             | Custom React Context (`I18nContext.tsx`)                  | 2925 keys × 19 locales (de/en/es/fr/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta); EN fallback; `localStorage` persistence |
-| **Testing**          | Vitest 4.x (7138+ tests / 583 files) + Playwright E2E     | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow)          |
+| **Testing**          | Vitest 4.x (7157+ tests / 587 files) + Playwright E2E     | Unit/integration + cross-browser E2E; Stryker mutation (manual workflow)          |
 | **Code Quality**     | Biome (lint + format) + TypeScript 7 (tsgo) strict       | `--error-on-warnings` in CI; zero `any` policy                      |
 | **Visualization**    | Force-directed graph                                      | Interactive character relationship network                           |
 | **Desktop**          | Tauri v2                                                  | Cross-platform installer; auto-updater via `latest.json`             |
@@ -550,7 +550,7 @@ WorldScript-Studio/
 │   ├── sw.js             # PWA Service Worker
 │   └── manifest.json     # PWA Web App Manifest v3
 ├── tests/
-│   ├── unit/             # Vitest unit tests (7138+ tests, 583 files) — count spans tests/, components/, packages/*/tests/, not just this folder
+│   ├── unit/             # Vitest unit tests (7157+ tests, 587 files) — count spans tests/, components/, packages/*/tests/, not just this folder
 │   │   ├── ai/           # aiSmallModules, aiCoreFallbackPaths
 │   │   └── settings/     # WebLlmPanel, AiSections
 │   └── e2e/              # Playwright specs + helpers.ts
@@ -712,7 +712,7 @@ The main pipeline is [`.github/workflows/ci.yml`](.github/workflows/ci.yml). Opt
 | `scorecard`  | weekly + `main` push | OpenSSF Scorecard — SARIF uploaded to GitHub Code Scanning |
 
 **Current test metrics (2026-08-21, source-synchronized; CI remains authoritative for pass/fail):**
-- **7138+ unit tests** across **583 test files** — CI is authoritative for pass/fail
+- **7157+ unit tests** across **587 test files** — CI is authoritative for pass/fail
 - Coverage thresholds: lines ≥ 80 · branches ≥ 66 · functions ≥ 72 · statements ≥ 78 — enforced in CI (see Codecov badge for live metrics)
 - i18n: **2925 keys × 19 locales** (en/de/fr/es/it + ar/he/fa RTL Beta + ja/zh/pt/el/fi/sv/hu/is/eu/ru/ko Beta)
 

app/persistedStateFlush.ts

@@ -32,5 +32,12 @@ export async function flushPersistedState(state: RootState): Promise<void> {
       ),
     );
   }
-  await Promise.all(saves);
+  // QNBS-v3: allSettled, not Promise.all — its fail-fast let a caller reload before the other save finished; both must settle first, still failing closed if either rejected.
+  const results = await Promise.allSettled(saves);
+  // QNBS-v3: a coordinator that rejected can already be running a superseding queued save it never told us about — wait for both to genuinely drain before returning or throwing.
+  await Promise.all([settingsPersistenceCoordinator.idle(), projectPersistenceCoordinator.idle()]);
+  const rejected = results.find(
+    (result): result is PromiseRejectedResult => result.status === 'rejected',
+  );
+  if (rejected) throw rejected.reason;
 }

app/persistenceCoordinator.ts

@@ -16,6 +16,13 @@ export class PersistenceCoordinator {
   private active: PendingOperation | null = null;
   private queued: PendingOperation | null = null;
   private waiters: Waiter[] = [];
+  private idleWaiters: Array<() => void> = [];
+
+  // QNBS-v3: rejectThrough fires immediately on failure without waiting for a superseding queued operation — idle() lets a caller wait for the coordinator to genuinely finish before doing something destructive (e.g. reload).
+  idle(): Promise<void> {
+    if (!this.active && !this.queued) return Promise.resolve();
+    return new Promise((resolve) => this.idleWaiters.push(resolve));
+  }
 
   enqueue(operation: SaveOperation): Promise<PersistenceResult> {
     const generation = ++this.nextGeneration;
@@ -57,6 +64,9 @@ export class PersistenceCoordinator {
       this.resolveThrough(current.generation);
       this.active = null;
     }
+    const idleWaiters = this.idleWaiters;
+    this.idleWaiters = [];
+    for (const resolve of idleWaiters) resolve();
   }
 
   private resolveThrough(generation: number): void {

public/sw.js

@@ -111,9 +111,7 @@ async function offlineFallback(request) {
 // INSTALL — Precache shell
 // ════════════════════════════════════════════════════════════
 self.addEventListener('install', (event) => {
-  // QNBS-v3: skipWaiting immediately so a new SW never sits in "waiting" state behind a stale
-  // active SW that serves cached v.old assets. The app auto-saves to IDB so a mid-session
-  // reload is safe. Paired with clients.claim() in activate this ensures all tabs get new code.
+  // QNBS-v3: activates immediately, no waiting — register-sw.ts owns the bounded pre-reload flush mitigation, residual risk tracked in #518.
   self.skipWaiting();
   // QNBS-v3: Never precache inside Tauri — the desktop app serves its shell from the bundle.
   if (IS_TAURI) return;

register-sw.ts

@@ -1,3 +1,6 @@
+import type { RootState } from './app/store';
+import { appStoreRef } from './app/storeRef';
+import { flushPersistedState } from './app/persistedStateFlush';
 import { logger as appLogger } from './services/logger';
 
 // ============================================================
@@ -183,13 +186,22 @@ const registerServiceWorker = async (): Promise<void> => {
       announceUpdateAvailable(registration.waiting);
     }
 
-    // QNBS-v3: Reload on any SW controller change — install now calls skipWaiting() automatically,
-    // so controllerchange fires whenever a new SW activates (not just on user-initiated updates).
-    // The app auto-saves to IDB so a mid-session reload is safe and always serves fresh assets.
+    // QNBS-v3: only the visible tab flushes — a hidden tab's stale write could race a fresher one (residual multi-window gap: #518).
     let refreshing = false;
+    let reloadPendingWhileHidden = false;
     navigator.serviceWorker.addEventListener('controllerchange', () => {
-      if (!refreshing) {
+      if (refreshing) return;
+      if (document.visibilityState !== 'visible') {
+        reloadPendingWhileHidden = true;
+        return;
+      }
+      refreshing = true;
+      void flushLatestStateThenReload();
+    });
+    document.addEventListener('visibilitychange', () => {
+      if (reloadPendingWhileHidden && document.visibilityState === 'visible' && !refreshing) {
         refreshing = true;
+        // QNBS-v3: no flush here — index.tsx's best-effort hide-time flush may have failed, but re-flushing risks clobbering a fresher write from another tab, the worse failure mode of the two (#518).
         window.location.reload();
       }
     });
@@ -225,6 +237,68 @@ const registerServiceWorker = async (): Promise<void> => {
   }
 };
 
+// QNBS-v3: loops until a flush completes against state that provably hasn't changed since — a single snapshot could miss edits made while the async write was still in flight.
+const MAX_FLUSH_ATTEMPTS = 5;
+
+// QNBS-v3: mirrors exactly what flushPersistedState reads/persists, field by field — versionControl also carries isPanelOpen (UI-only), so comparing the whole slice would retry on that too.
+function persistedSlices(state: RootState) {
+  return {
+    project: state.project.present,
+    branches: state.versionControl.branches,
+    snapshots: state.versionControl.snapshots,
+    currentBranchId: state.versionControl.currentBranchId,
+    settings: state.settings,
+  };
+}
+
+function persistedSlicesUnchanged(
+  a: ReturnType<typeof persistedSlices>,
+  b: ReturnType<typeof persistedSlices>,
+): boolean {
+  return (
+    a.project === b.project &&
+    a.branches === b.branches &&
+    a.snapshots === b.snapshots &&
+    a.currentBranchId === b.currentBranchId &&
+    a.settings === b.settings
+  );
+}
+
+async function flushLatestState(): Promise<void> {
+  const store = appStoreRef.current;
+  if (!store) return;
+  let snapshot = store.getState() as RootState;
+  let snapshotSlices = persistedSlices(snapshot);
+  for (let attempt = 0; attempt < MAX_FLUSH_ATTEMPTS; attempt++) {
+    await flushPersistedState(snapshot);
+    const latest = store.getState() as RootState;
+    const latestSlices = persistedSlices(latest);
+    if (persistedSlicesUnchanged(snapshotSlices, latestSlices)) return;
+    snapshot = latest;
+    snapshotSlices = latestSlices;
+  }
+  // QNBS-v3: narrows but doesn't eliminate the race — a keystroke during this final await is still possible to lose (#518).
+  await flushPersistedState(store.getState() as RootState);
+}
+
+// QNBS-v3: bounds the flush — an unbounded wait (e.g. queued behind another tab's exclusive Web Lock) would hang forever on an already-cache-pruned bundle, defeating the always-reload policy below.
+const FLUSH_TIMEOUT_MS = 8000;
+
+// QNBS-v3: the reload always proceeds — activation already pruned old-version caches by the time controllerchange fires, so staying on the old bundle risks missing-chunk failures too.
+async function flushLatestStateThenReload(): Promise<void> {
+  try {
+    await Promise.race([
+      flushLatestState(),
+      new Promise<never>((_, reject) =>
+        setTimeout(() => reject(new Error(`Pre-reload flush timed out after ${FLUSH_TIMEOUT_MS}ms`)), FLUSH_TIMEOUT_MS),
+      ),
+    ]);
+  } catch (error) {
+    appLogger.error('[SW] Pre-reload state flush failed (reloading anyway):', error);
+  }
+  window.location.reload();
+}
+
 if (typeof window !== 'undefined') {
   window.addEventListener('load', registerServiceWorker);
 }

services/storage/idbProjectStore.ts

@@ -268,23 +268,26 @@ export class IdbProjectStore extends IdbAssetStore {
       const store = await this.getObjectStore(APP_DATA_STORE, 'readwrite');
       return new Promise<void>((resolve, reject) => {
         const request = store.put(payload, sliceName);
-        request.onsuccess = () => resolve();
+        const transaction = store.transaction;
+        // QNBS-v3: resolve on transaction commit, not request success — onsuccess fires before the write is durable, which now matters since a caller can immediately reload.
         request.onerror = () => reject(request.error);
+        transaction.oncomplete = () => resolve();
+        transaction.onerror = () => reject(transaction.error);
+        transaction.onabort = () => reject(transaction.error ?? new Error('IDB transaction aborted'));
       });
     });
   }
 
   async saveProject(data: SaveProjectInput): Promise<void> {
-    // Check auto-snapshot condition during save
-    if (Date.now() - this.lastAutoSnapshotTime > this.AUTO_SNAPSHOT_INTERVAL) {
+    // QNBS-v3: autoSnapshotInFlight prevents a saveProject() call arriving before the first snapshot's success callback runs from starting a duplicate concurrent snapshot.
+    if (!this.autoSnapshotInFlight && Date.now() - this.lastAutoSnapshotTime > this.AUTO_SNAPSHOT_INTERVAL) {
       // data may arrive as a Redux-undo envelope (PersistedProjectState) or plain StoryProject
       const persisted = data as PersistedProjectState;
       const projectData = persisted.present ? persisted.present.data : persisted.data;
       if (projectData?.manuscript) {
-        // QNBS-v3: Only commit the timestamp after a successful snapshot — an unhandled rejection
-        //          here (e.g. the expected locked-write error) previously suppressed the next
-        //          automatic snapshot for a full interval even though none was actually taken.
+        // QNBS-v3: Only commit the timestamp after a successful snapshot — an unhandled rejection here previously suppressed the next automatic snapshot for a full interval even though none was actually taken.
         const snapshotTime = Date.now();
+        this.autoSnapshotInFlight = true;
         // Fire and forget snapshot to not block UI
         this.createSnapshot(projectData)
           .then(() => {
@@ -293,6 +296,9 @@ export class IdbProjectStore extends IdbAssetStore {
           })
           .catch((error: unknown) => {
             logger.warn('Automatic snapshot failed', { error: String(error) });
+          })
+          .finally(() => {
+            this.autoSnapshotInFlight = false;
           });
       }
     }

services/storage/idbSnapshotStore.ts

@@ -22,6 +22,8 @@ import {
 
 export class IdbSnapshotStore extends IdbCodexStore {
   protected lastAutoSnapshotTime = Date.now();
+  // QNBS-v3: guards against concurrent saveProject() calls each starting their own auto-snapshot before the first one's success callback updates lastAutoSnapshotTime.
+  protected autoSnapshotInFlight = false;
   protected readonly AUTO_SNAPSHOT_INTERVAL = 5 * 60 * 1000; // 5 minutes
   protected readonly MAX_AUTO_SNAPSHOTS = 20;
 

tests/unit/dbServiceAutoSnapshotRace.test.ts

@@ -0,0 +1,94 @@
+// QNBS-v3: proves a saveProject() call arriving before the first snapshot's success callback runs never starts a duplicate concurrent auto-snapshot.
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+vi.mock('../../services/logger', () => {
+  const noopLogger = { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() };
+  return {
+    logger: noopLogger,
+    createLogger: () => ({ ...noopLogger, withContext: () => ({ ...noopLogger }) }),
+  };
+});
+
+const fakeStore = {
+  put: vi.fn().mockImplementation(() => {
+    const r: Record<string, unknown> = {};
+    Promise.resolve().then(() => {
+      if (typeof r['onsuccess'] === 'function') (r['onsuccess'] as () => void)();
+    });
+    return r;
+  }),
+  count: vi.fn().mockImplementation(() => {
+    const r = { result: 0 } as Record<string, unknown>;
+    Promise.resolve().then(() => {
+      if (typeof r['onsuccess'] === 'function') (r['onsuccess'] as () => void)();
+    });
+    return r;
+  }),
+};
+
+const fakeDb = {
+  objectStoreNames: { contains: () => true },
+  transaction: vi.fn().mockReturnValue({ objectStore: () => fakeStore }),
+};
+
+async function flushMicrotasks(): Promise<void> {
+  await new Promise((resolve) => setTimeout(resolve, 0));
+}
+
+describe('dbService — saveProject auto-snapshot in-flight guard (DA-02, codex)', () => {
+  let saveSliceMock: ReturnType<typeof vi.fn>;
+  let createSnapshotMock: ReturnType<typeof vi.fn>;
+
+  beforeEach(() => {
+    vi.resetModules();
+    saveSliceMock = vi.fn().mockResolvedValue(undefined);
+    createSnapshotMock = vi.fn();
+  });
+
+  async function getService() {
+    const mod = await import('../../services/dbService');
+    const svc = mod.dbService as unknown as Record<string, unknown>;
+    svc['stateDb'] = fakeDb;
+    svc['dataDb'] = fakeDb;
+    svc['saveSlice'] = saveSliceMock;
+    svc['createSnapshot'] = createSnapshotMock;
+    // Force the 5-minute interval to already have elapsed.
+    svc['lastAutoSnapshotTime'] = 0;
+    return mod.dbService;
+  }
+
+  const project = { title: 'T', manuscript: [{ id: '1', title: 'S', content: 'x' }] };
+
+  it('does not start a second concurrent auto-snapshot while the first is still pending', async () => {
+    let resolveFirstSnapshot: (id: number) => void = () => {};
+    createSnapshotMock.mockImplementationOnce(
+      () => new Promise<number>((resolve) => { resolveFirstSnapshot = resolve; }),
+    );
+
+    const svc = await getService();
+    const firstSave = svc.saveProject({ present: { data: project } } as never);
+    const secondSave = svc.saveProject({ present: { data: project } } as never);
+
+    await Promise.all([firstSave, secondSave]);
+    await flushMicrotasks();
+    expect(createSnapshotMock).toHaveBeenCalledTimes(1);
+
+    resolveFirstSnapshot(1);
+    await flushMicrotasks();
+  });
+
+  it('allows a new auto-snapshot once the prior one has settled and the interval has elapsed again', async () => {
+    createSnapshotMock.mockResolvedValueOnce(1);
+    const svc = await getService();
+
+    await svc.saveProject({ present: { data: project } } as never);
+    await flushMicrotasks();
+    expect(createSnapshotMock).toHaveBeenCalledTimes(1);
+
+    (svc as unknown as Record<string, unknown>)['lastAutoSnapshotTime'] = 0;
+    createSnapshotMock.mockResolvedValueOnce(2);
+    await svc.saveProject({ present: { data: project } } as never);
+    await flushMicrotasks();
+    expect(createSnapshotMock).toHaveBeenCalledTimes(2);
+  });
+});

tests/unit/persistedStateFlush.test.ts

@@ -3,7 +3,9 @@
  * QNBS-v3 (#332/D3): shared flush helper used by both index.tsx's visibilitychange handler and the
  * desktop close-to-tray quit flush — verifies it saves project+settings via storageService, always
  * saves settings even with no project data yet (fresh/new-user state), and fails closed on any
- * rejected save (Promise.all, not Promise.allSettled) so a failed write is never silently ignored.
+ * rejected save (Promise.allSettled, waiting for both to settle before rejecting) so a failed
+ * write is never silently ignored and a caller that reloads immediately after never tears down
+ * the page while the other save is still in flight.
  */
 
 import { beforeEach, describe, expect, it, vi } from 'vitest';
@@ -81,4 +83,36 @@ describe('flushPersistedState', () => {
     h.saveSettings.mockRejectedValueOnce(new Error('disk full'));
     await expect(flushPersistedState(buildState())).rejects.toThrow('disk full');
   });
+
+  // QNBS-v3: an immediate-reload caller must never tear down the page while the other save is still in flight.
+  it('waits for the other save to settle before rejecting, instead of rejecting as soon as one fails', async () => {
+    const order: string[] = [];
+    h.saveProject.mockImplementation(async () => {
+      order.push('project-rejected');
+      throw new Error('project save failed');
+    });
+    let resolveSettings: () => void = () => {};
+    h.saveSettings.mockImplementation(
+      () =>
+        new Promise<void>((resolve) => {
+          resolveSettings = () => {
+            order.push('settings-resolved');
+            resolve();
+          };
+        }),
+    );
+
+    const flushPromise = flushPersistedState(buildState()).catch((err: unknown) => {
+      order.push('flush-rejected');
+      throw err;
+    });
+
+    // QNBS-v3: a macrotask boundary drains every microtask the real coordinator's drain loop schedules, however many ticks deep.
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    expect(order).toEqual(['project-rejected']);
+
+    resolveSettings();
+    await expect(flushPromise).rejects.toThrow('project save failed');
+    expect(order).toEqual(['project-rejected', 'settings-resolved', 'flush-rejected']);
+  });
 });