diff --git a/README.md b/README.md index 75801bee3..2a43d4fc3 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ IndexedDB v8 PWA v3.0 i18n 19 locales — 2925 keys - 7138+ tests / 583 files + 7157+ tests / 587 files Codecov Coverage License MIT 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) diff --git a/app/persistedStateFlush.ts b/app/persistedStateFlush.ts index 5a991d7cb..a6b5b5253 100644 --- a/app/persistedStateFlush.ts +++ b/app/persistedStateFlush.ts @@ -32,5 +32,12 @@ export async function flushPersistedState(state: RootState): Promise { ), ); } - 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; } \ No newline at end of file diff --git a/app/persistenceCoordinator.ts b/app/persistenceCoordinator.ts index 943584e4f..09e3d8f3b 100644 --- a/app/persistenceCoordinator.ts +++ b/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 { + if (!this.active && !this.queued) return Promise.resolve(); + return new Promise((resolve) => this.idleWaiters.push(resolve)); + } enqueue(operation: SaveOperation): Promise { 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 { diff --git a/public/sw.js b/public/sw.js index 6a63bbb87..af7150382 100644 --- a/public/sw.js +++ b/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; diff --git a/register-sw.ts b/register-sw.ts index 8e95326c0..2dc197fd8 100644 --- a/register-sw.ts +++ b/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 => { 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 => { } }; +// 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, + b: ReturnType, +): 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 { + 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 { + try { + await Promise.race([ + flushLatestState(), + new Promise((_, 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); } diff --git a/services/storage/idbProjectStore.ts b/services/storage/idbProjectStore.ts index 2df6fdce8..a6ed2172e 100644 --- a/services/storage/idbProjectStore.ts +++ b/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((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 { - // 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; }); } } diff --git a/services/storage/idbSnapshotStore.ts b/services/storage/idbSnapshotStore.ts index 0da8306fb..6c1302d66 100644 --- a/services/storage/idbSnapshotStore.ts +++ b/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; diff --git a/tests/unit/dbServiceAutoSnapshotRace.test.ts b/tests/unit/dbServiceAutoSnapshotRace.test.ts new file mode 100644 index 000000000..2799297bc --- /dev/null +++ b/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 = {}; + Promise.resolve().then(() => { + if (typeof r['onsuccess'] === 'function') (r['onsuccess'] as () => void)(); + }); + return r; + }), + count: vi.fn().mockImplementation(() => { + const r = { result: 0 } as Record; + 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 { + await new Promise((resolve) => setTimeout(resolve, 0)); +} + +describe('dbService — saveProject auto-snapshot in-flight guard (DA-02, codex)', () => { + let saveSliceMock: ReturnType; + let createSnapshotMock: ReturnType; + + 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; + 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((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)['lastAutoSnapshotTime'] = 0; + createSnapshotMock.mockResolvedValueOnce(2); + await svc.saveProject({ present: { data: project } } as never); + await flushMicrotasks(); + expect(createSnapshotMock).toHaveBeenCalledTimes(2); + }); +}); diff --git a/tests/unit/persistedStateFlush.test.ts b/tests/unit/persistedStateFlush.test.ts index 88158c8dd..e445d60ee 100644 --- a/tests/unit/persistedStateFlush.test.ts +++ b/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((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']); + }); }); diff --git a/tests/unit/persistenceCoordinator.test.ts b/tests/unit/persistenceCoordinator.test.ts index 2d49ac635..1ccd831ff 100644 --- a/tests/unit/persistenceCoordinator.test.ts +++ b/tests/unit/persistenceCoordinator.test.ts @@ -78,4 +78,50 @@ describe('PersistenceCoordinator', () => { await expect(second).resolves.toEqual({ superseded: false }); expect(saved).toEqual(['second']); }); + + // QNBS-v3: rejectThrough settles the failed generation's own promise immediately, but the coordinator keeps running a superseding queued operation in the background — idle() must wait for that too. + it('idle() waits for a superseding queued operation to finish even after the current one rejects', async () => { + const coordinator = new PersistenceCoordinator(); + const failure = new Error('disk full'); + const gate = deferred(); + const secondGate = deferred(); + const saved: string[] = []; + + const first = coordinator.enqueue(async () => { + await gate.promise; + throw failure; + }); + coordinator.enqueue(async () => { + saved.push('second:start'); + await secondGate.promise; + saved.push('second:end'); + }); + + gate.resolve(); + await expect(first).rejects.toBe(failure); + // The failed generation's own promise has already settled, but the superseding second + // generation is now running in the background — idle() must not resolve until it finishes too. + expect(saved).toEqual(['second:start']); + + let idleResolved = false; + const idlePromise = coordinator.idle().then(() => { + idleResolved = true; + }); + await Promise.resolve(); + expect(idleResolved).toBe(false); + + secondGate.resolve(); + await idlePromise; + expect(idleResolved).toBe(true); + expect(saved).toEqual(['second:start', 'second:end']); + }); + + it('idle() resolves immediately when nothing is active or queued', async () => { + const coordinator = new PersistenceCoordinator(); + let resolved = false; + await coordinator.idle().then(() => { + resolved = true; + }); + expect(resolved).toBe(true); + }); }); diff --git a/tests/unit/registerSwCacheOwnership.test.ts b/tests/unit/registerSwCacheOwnership.test.ts index 0392643e1..47a94a26f 100644 --- a/tests/unit/registerSwCacheOwnership.test.ts +++ b/tests/unit/registerSwCacheOwnership.test.ts @@ -1,9 +1,10 @@ // QNBS-v3: proves the Tauri-teardown cache cleanup in register-sw.ts never deletes an unowned cache. import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -vi.mock('../../services/logger', () => ({ - logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, -})); +vi.mock('../../services/logger', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } }; +}); import { isWorldScriptOwnedCacheName, registerServiceWorker } from '../../register-sw'; diff --git a/tests/unit/registerSwUpdateFlush.test.ts b/tests/unit/registerSwUpdateFlush.test.ts new file mode 100644 index 000000000..73a96d99f --- /dev/null +++ b/tests/unit/registerSwUpdateFlush.test.ts @@ -0,0 +1,259 @@ +// QNBS-v3: proves controllerchange flushes the latest visible-tab state before reloading. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../services/logger', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } }; +}); + +const { mockFlushPersistedState } = vi.hoisted(() => ({ mockFlushPersistedState: vi.fn() })); +vi.mock('../../app/persistedStateFlush', () => ({ flushPersistedState: mockFlushPersistedState })); + +import { appStoreRef } from '../../app/storeRef'; +import { registerServiceWorker } from '../../register-sw'; + +async function flushMicrotasks(): Promise { + await new Promise((resolve) => setTimeout(resolve, 0)); +} + +function setVisibility(value: 'visible' | 'hidden'): void { + Object.defineProperty(document, 'visibilityState', { value, configurable: true }); +} + +describe('register-sw — controllerchange flush-then-reload (DA-02)', () => { + let controllerChangeHandler: (() => void) | undefined; + let reloadSpy: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + controllerChangeHandler = undefined; + setVisibility('visible'); + + for (const key of ['__TAURI_INTERNALS__', '__TAURI__', '__TAURI_METADATA__']) { + delete (window as unknown as Record)[key]; + } + + reloadSpy = vi.fn(); + Object.defineProperty(window, 'location', { + value: { ...window.location, reload: reloadSpy }, + writable: true, + configurable: true, + }); + + const fakeRegistration = { + scope: '/', + installing: null, + waiting: null, + addEventListener: vi.fn(), + }; + + Object.defineProperty(navigator, 'serviceWorker', { + value: { + register: vi.fn().mockResolvedValue(fakeRegistration), + controller: {}, + addEventListener: vi.fn((type: string, handler: () => void) => { + if (type === 'controllerchange') controllerChangeHandler = handler; + }), + }, + writable: true, + configurable: true, + }); + + // QNBS-v3: a stable reference with real RootState shape — persistedSlices() reads project.present, matching production where project is never undefined. + const stableState = { project: { present: { fake: 'state' } }, versionControl: {}, settings: {} }; + appStoreRef.current = { + getState: () => stableState as never, + dispatch: vi.fn() as never, + }; + }); + + afterEach(() => { + appStoreRef.current = null; + // @ts-expect-error — test-only cleanup of a property this suite defines itself. + delete navigator.serviceWorker; + setVisibility('visible'); + }); + + it('flushes pending state and reloads, in that order, when the visible tab takes control', async () => { + mockFlushPersistedState.mockResolvedValue(undefined); + await registerServiceWorker(); + expect(controllerChangeHandler).toBeTypeOf('function'); + + controllerChangeHandler?.(); + await flushMicrotasks(); + + expect(mockFlushPersistedState).toHaveBeenCalledTimes(1); + expect(reloadSpy).toHaveBeenCalledTimes(1); + const flushOrder = mockFlushPersistedState.mock.invocationCallOrder[0] as number; + const reloadOrder = reloadSpy.mock.invocationCallOrder[0] as number; + expect(flushOrder).toBeLessThan(reloadOrder); + }); + + it('reloads even when the flush fails, rather than staying on a bundle whose old cache is already pruned', async () => { + mockFlushPersistedState.mockRejectedValue(new Error('IDB write failed')); + await registerServiceWorker(); + + controllerChangeHandler?.(); + await flushMicrotasks(); + + expect(mockFlushPersistedState).toHaveBeenCalledTimes(1); + expect(reloadSpy).toHaveBeenCalledTimes(1); + }); + + it('ignores a second controllerchange event (single-flight)', async () => { + mockFlushPersistedState.mockResolvedValue(undefined); + await registerServiceWorker(); + + controllerChangeHandler?.(); + controllerChangeHandler?.(); + await flushMicrotasks(); + + expect(mockFlushPersistedState).toHaveBeenCalledTimes(1); + expect(reloadSpy).toHaveBeenCalledTimes(1); + }); + + it('still reloads when no store is mounted yet (defensive null-guard, nothing to flush)', async () => { + appStoreRef.current = null; + await registerServiceWorker(); + + controllerChangeHandler?.(); + await flushMicrotasks(); + + expect(mockFlushPersistedState).not.toHaveBeenCalled(); + expect(reloadSpy).toHaveBeenCalledTimes(1); + }); + + // QNBS-v3: only the visible tab flushes — a hidden tab's state can't safely be assumed fresher than what's already persisted. + it('defers reload while the tab is hidden, then reloads once visible without flushing again', async () => { + mockFlushPersistedState.mockResolvedValue(undefined); + await registerServiceWorker(); + setVisibility('hidden'); + + controllerChangeHandler?.(); + await flushMicrotasks(); + + expect(mockFlushPersistedState).not.toHaveBeenCalled(); + expect(reloadSpy).not.toHaveBeenCalled(); + + setVisibility('visible'); + document.dispatchEvent(new Event('visibilitychange')); + await flushMicrotasks(); + + // QNBS-v3: index.tsx's own visibilitychange listener already attempted a flush when this tab went hidden — flushing its possibly-stale copy again could clobber a fresher write from another tab. + expect(mockFlushPersistedState).not.toHaveBeenCalled(); + expect(reloadSpy).toHaveBeenCalledTimes(1); + }); + + // QNBS-v3: a single snapshot could miss an edit made while the async write is still in flight. + it('re-flushes with the latest state when it changes during the pending flush, before reloading', async () => { + const stateA = { project: { present: { v: 'a' } }, versionControl: {}, settings: {} }; + const stateB = { project: { present: { v: 'b' } }, versionControl: {}, settings: {} }; + // 1st getState(): stateA. 2nd (after flush #1): stateB (changed — retry). 3rd (after flush #2): stateB (stable — stop). + const getStateMock = vi.fn().mockReturnValueOnce(stateA).mockReturnValueOnce(stateB).mockReturnValue(stateB); + appStoreRef.current = { getState: getStateMock, dispatch: vi.fn() as never }; + mockFlushPersistedState.mockResolvedValue(undefined); + + await registerServiceWorker(); + controllerChangeHandler?.(); + await flushMicrotasks(); + + expect(mockFlushPersistedState).toHaveBeenCalledTimes(2); + expect(mockFlushPersistedState).toHaveBeenNthCalledWith(1, stateA); + expect(mockFlushPersistedState).toHaveBeenNthCalledWith(2, stateB); + expect(reloadSpy).toHaveBeenCalledTimes(1); + const lastFlushOrder = mockFlushPersistedState.mock.invocationCallOrder[1] as number; + const reloadOrder = reloadSpy.mock.invocationCallOrder[0] as number; + expect(lastFlushOrder).toBeLessThan(reloadOrder); + }); + + // QNBS-v3: the retry loop can exhaust its budget while state keeps changing — one guaranteed final flush must still capture whatever's freshest, not silently drop it. + it('performs one final guaranteed flush of the freshest state after exhausting the retry budget', async () => { + const states = Array.from({ length: 7 }, (_, i) => ({ + project: { present: { v: i } }, + versionControl: {}, + settings: {}, + })); + const getStateMock = vi.fn(); + for (const s of states) getStateMock.mockReturnValueOnce(s); + appStoreRef.current = { getState: getStateMock, dispatch: vi.fn() as never }; + mockFlushPersistedState.mockResolvedValue(undefined); + + await registerServiceWorker(); + controllerChangeHandler?.(); + await flushMicrotasks(); + + // 5 in-loop attempts (states[0..4]) + 1 guaranteed final flush of the freshest state (states[6]). + expect(mockFlushPersistedState).toHaveBeenCalledTimes(6); + expect(mockFlushPersistedState).toHaveBeenNthCalledWith(6, states[6]); + expect(reloadSpy).toHaveBeenCalledTimes(1); + }); + + // QNBS-v3: comparing the whole root state retried on unrelated non-persisted churn (e.g. status.saving), wasting the retry budget on noise instead of real edits. + it('does not retry when only a non-persisted slice changes between getState() calls', async () => { + const project = { present: { v: 'a' } }; + const versionControl = {}; + const settings = {}; + // Same persisted slices every call — only the non-persisted `status` field differs. + const getStateMock = vi + .fn() + .mockReturnValueOnce({ project, versionControl, settings, status: { saving: 'saving' } }) + .mockReturnValue({ project, versionControl, settings, status: { saving: 'saved' } }); + appStoreRef.current = { getState: getStateMock, dispatch: vi.fn() as never }; + mockFlushPersistedState.mockResolvedValue(undefined); + + await registerServiceWorker(); + controllerChangeHandler?.(); + await flushMicrotasks(); + + expect(mockFlushPersistedState).toHaveBeenCalledTimes(1); + expect(reloadSpy).toHaveBeenCalledTimes(1); + }); + + // QNBS-v3: versionControl mixes persisted fields (branches/snapshots/currentBranchId) with a UI-only isPanelOpen toggle — must compare only the former. + it('does not retry when only versionControl.isPanelOpen changes, not the persisted version-control fields', async () => { + const project = { present: { v: 'a' } }; + const settings = {}; + const branches = [{ id: 'main' }]; + const snapshots: unknown[] = []; + const currentBranchId = 'main'; + const getStateMock = vi + .fn() + .mockReturnValueOnce({ + project, + versionControl: { branches, snapshots, currentBranchId, isPanelOpen: false }, + settings, + }) + .mockReturnValue({ + project, + versionControl: { branches, snapshots, currentBranchId, isPanelOpen: true }, + settings, + }); + appStoreRef.current = { getState: getStateMock, dispatch: vi.fn() as never }; + mockFlushPersistedState.mockResolvedValue(undefined); + + await registerServiceWorker(); + controllerChangeHandler?.(); + await flushMicrotasks(); + + expect(mockFlushPersistedState).toHaveBeenCalledTimes(1); + expect(reloadSpy).toHaveBeenCalledTimes(1); + }); + + // QNBS-v3: an unbounded wait (e.g. queued behind another tab's exclusive Web Lock) must not hang the reload forever on an already-cache-pruned bundle. + it('reloads once the flush timeout elapses if the flush never settles', async () => { + vi.useFakeTimers(); + try { + mockFlushPersistedState.mockImplementation(() => new Promise(() => {})); + await registerServiceWorker(); + controllerChangeHandler?.(); + + await vi.advanceTimersByTimeAsync(0); + expect(reloadSpy).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(8000); + expect(reloadSpy).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/tests/unit/services/storage/idbProjectStoreSaveSlice.test.ts b/tests/unit/services/storage/idbProjectStoreSaveSlice.test.ts new file mode 100644 index 000000000..498618779 --- /dev/null +++ b/tests/unit/services/storage/idbProjectStoreSaveSlice.test.ts @@ -0,0 +1,104 @@ +/** + * Tests for IdbProjectStore#saveSlice — DA-02 review-wave fix (codex): the returned promise must + * resolve only once the underlying IndexedDB transaction actually commits (transaction.oncomplete), + * not merely once the individual put() request succeeds (request.onsuccess) — a caller that reloads + * immediately after resolution must never be able to tear down the page mid-commit. + */ +import { describe, expect, it, vi } from 'vitest'; + +vi.mock('../../../../services/storage/storageEncryptionService', () => ({ + resolveProtectedWriteKey: vi.fn().mockResolvedValue(null), + assertNoActiveEncryptionMigration: vi.fn().mockResolvedValue(undefined), + idbEncryptWithKey: vi.fn(), + idbReadSecure: vi.fn(), + assertIdbProtectedWriteAllowed: vi.fn().mockResolvedValue(undefined), + assertSecureStorageReadable: vi.fn().mockResolvedValue(undefined), +})); + +import { IdbProjectStore } from '../../../../services/storage/idbProjectStore'; + +interface FakeIdbRequest { + onsuccess: (() => void) | null; + onerror: (() => void) | null; + error: unknown; +} + +interface FakeIdbTransaction { + oncomplete: (() => void) | null; + onerror: (() => void) | null; + onabort: (() => void) | null; + error: unknown; +} + +function makeFakeStore(): { store: { put: () => FakeIdbRequest; transaction: FakeIdbTransaction }; request: FakeIdbRequest; transaction: FakeIdbTransaction } { + const transaction: FakeIdbTransaction = { oncomplete: null, onerror: null, onabort: null, error: null }; + const request: FakeIdbRequest = { onsuccess: null, onerror: null, error: null }; + const store = { put: () => request, transaction }; + return { store, request, transaction }; +} + +describe('IdbProjectStore#saveSlice — resolves on transaction commit, not request success', () => { + it('does not resolve when only request.onsuccess has fired', async () => { + const projectStore = new IdbProjectStore(); + const { store, request, transaction } = makeFakeStore(); + vi.spyOn( + projectStore as unknown as { getObjectStore: () => Promise }, + 'getObjectStore', + ).mockResolvedValue(store as never); + + let resolved = false; + const savePromise = projectStore.saveSlice('settings', { theme: 'dark' } as never).then(() => { + resolved = true; + }); + + // Give the async setup (key resolution, migration guard, getObjectStore) time to run and call put(). + await new Promise((resolve) => setTimeout(resolve, 0)); + request.onsuccess?.(); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(resolved).toBe(false); + transaction.oncomplete?.(); // settle the promise so it doesn't leak past this test + await savePromise; + }); + + it('resolves once transaction.oncomplete fires, after request.onsuccess', async () => { + const projectStore = new IdbProjectStore(); + const { store, request, transaction } = makeFakeStore(); + vi.spyOn( + projectStore as unknown as { getObjectStore: () => Promise }, + 'getObjectStore', + ).mockResolvedValue(store as never); + + const order: string[] = []; + const savePromise = projectStore + .saveSlice('settings', { theme: 'dark' } as never) + .then(() => order.push('resolved')); + + await new Promise((resolve) => setTimeout(resolve, 0)); + request.onsuccess?.(); + order.push('request-succeeded'); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(order).toEqual(['request-succeeded']); + + transaction.oncomplete?.(); + await savePromise; + expect(order).toEqual(['request-succeeded', 'resolved']); + }); + + it('rejects if the transaction aborts even though the request itself succeeded', async () => { + const projectStore = new IdbProjectStore(); + const { store, request, transaction } = makeFakeStore(); + vi.spyOn( + projectStore as unknown as { getObjectStore: () => Promise }, + 'getObjectStore', + ).mockResolvedValue(store as never); + + const savePromise = projectStore.saveSlice('settings', { theme: 'dark' } as never); + await new Promise((resolve) => setTimeout(resolve, 0)); + request.onsuccess?.(); + transaction.error = new Error('QuotaExceededError'); + transaction.onabort?.(); + + await expect(savePromise).rejects.toThrow('QuotaExceededError'); + }); +}); diff --git a/tests/unit/services/storage/idbProjectStoreSaveSliceRealIdb.test.ts b/tests/unit/services/storage/idbProjectStoreSaveSliceRealIdb.test.ts new file mode 100644 index 000000000..e6fae12ee --- /dev/null +++ b/tests/unit/services/storage/idbProjectStoreSaveSliceRealIdb.test.ts @@ -0,0 +1,46 @@ +// @vitest-environment node +// QNBS-v3: complements the hand-built-mock ordering test with a real fake-indexeddb round trip proving the write genuinely persists. +import { IDBFactory } from 'fake-indexeddb'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../../../../services/storage/storageEncryptionService', () => ({ + resolveProtectedWriteKey: vi.fn().mockResolvedValue(null), + assertNoActiveEncryptionMigration: vi.fn().mockResolvedValue(undefined), + idbEncryptWithKey: vi.fn(), + idbReadSecure: vi.fn(), + assertIdbProtectedWriteAllowed: vi.fn().mockResolvedValue(undefined), + assertSecureStorageReadable: vi.fn().mockResolvedValue(undefined), +})); + +import { APP_DATA_STORE, STATE_DB_NAME } from '../../../../services/dbConstants'; +import { IdbProjectStore } from '../../../../services/storage/idbProjectStore'; + +beforeEach(() => { + global.indexedDB = new IDBFactory(); +}); + +describe('IdbProjectStore#saveSlice — real IndexedDB round trip', () => { + it('persists the write durably against a real IDBFactory, readable back afterward', async () => { + const projectStore = new IdbProjectStore(); + await projectStore.saveSlice('settings', { theme: 'dark' } as never); + + const readBack = await new Promise((resolve, reject) => { + const request = indexedDB.open(STATE_DB_NAME); + request.onsuccess = () => { + const database = request.result; + const transaction = database.transaction(APP_DATA_STORE, 'readonly'); + const getRequest = transaction.objectStore(APP_DATA_STORE).get('settings'); + getRequest.onsuccess = () => { + database.close(); + resolve(getRequest.result); + }; + getRequest.onerror = () => reject(getRequest.error); + }; + request.onerror = () => reject(request.error); + }); + + // Plaintext (no encryption key configured) is compressed JSON — decoding it back proves the + // real transaction actually committed, not just that some request fired successfully. + expect(readBack).toBeDefined(); + }); +});