diff --git a/packages/ui/src/lib/persistence.test.ts b/packages/ui/src/lib/persistence.test.ts index 38057e9c..5a29f976 100644 --- a/packages/ui/src/lib/persistence.test.ts +++ b/packages/ui/src/lib/persistence.test.ts @@ -568,3 +568,87 @@ describe('updateDesktopSettings', () => { expect(saveCalls.some((changes) => changes.autoSaveEnabled === true)).toBe(true); }); }); + +describe('unload lifecycle flush (#2197)', () => { + beforeEach(() => { + getWindow(); + registerRuntimeAPIs(null); + invalidateSettingsCache(); + }); + + test('flushes a pending debounced settings save on pagehide without a double write', async () => { + const saveCalls: Array> = []; + registerSettingsSave(async (changes) => { + saveCalls.push(changes); + return {}; + }); + + const update = updateDesktopSettings({ showDeletionDialog: false }); + expect(saveCalls).toEqual([]); + + getWindow().dispatchEvent(new Event('pagehide')); + + // The flush must hand the pending changes to the settings backend + // synchronously inside the lifecycle listener — an unloading window has + // no later turn for the debounce timer. + expect(saveCalls).toEqual([{ showDeletionDialog: false }]); + + await update; + await delay(300); + // The canceled debounce timer must not replay the same write. + expect(saveCalls).toHaveLength(1); + }); + + test('flushes a pending debounced settings save on beforeunload without a double write', async () => { + const saveCalls: Array> = []; + registerSettingsSave(async (changes) => { + saveCalls.push(changes); + return {}; + }); + + const update = updateDesktopSettings({ gitChangesViewMode: 'tree' }); + expect(saveCalls).toEqual([]); + + getWindow().dispatchEvent(new Event('beforeunload')); + + expect(saveCalls).toEqual([{ gitChangesViewMode: 'tree' }]); + + await update; + await delay(300); + expect(saveCalls).toHaveLength(1); + }); + + test('persists a showDeletionDialog toggle followed by an immediate unload', async () => { + const saveCalls: Array> = []; + registerSettingsSave(async (changes) => { + saveCalls.push(changes); + return {}; + }); + startAppearanceAutoSave(); + + try { + useUIStore.getState().setShowDeletionDialog(false); + getWindow().dispatchEvent(new Event('pagehide')); + + expect(saveCalls.some((changes) => changes.showDeletionDialog === false)).toBe(true); + } finally { + useUIStore.getState().setShowDeletionDialog(true); + // Let the restore write drain so it cannot leak into other tests. + await delay(300); + } + }); + + test('ignores lifecycle events when no settings write is pending', async () => { + const saveCalls: Array> = []; + registerSettingsSave(async (changes) => { + saveCalls.push(changes); + return {}; + }); + + getWindow().dispatchEvent(new Event('pagehide')); + getWindow().dispatchEvent(new Event('beforeunload')); + await delay(50); + + expect(saveCalls).toEqual([]); + }); +}); diff --git a/packages/ui/src/lib/persistence.ts b/packages/ui/src/lib/persistence.ts index 5dcc40ab..9375a468 100644 --- a/packages/ui/src/lib/persistence.ts +++ b/packages/ui/src/lib/persistence.ts @@ -1625,6 +1625,21 @@ const isSettingsRuntimeContextCurrent = (context: SettingsRuntimeContext): boole context.generation === _settingsRuntimeGeneration && context.runtimeKey === getRuntimeKey() ); +// Best-effort flush of the pending debounced settings write at a lifecycle +// boundary. Clearing the timer before flushing means the write happens exactly +// once — the flush consumes the pending changes, so a timer that already fired +// cannot double-write. A hard process kill (crash, task-manager kill) can +// still lose the in-flight request; this narrows the loss window to the +// request itself instead of the whole debounce interval (#2197). +const flushPendingSettingsBeforeSuspend = (): void => { + if (!_pendingSettingsChanges) return; + if (_settingsFlushTimer) { + clearTimeout(_settingsFlushTimer); + _settingsFlushTimer = null; + } + void _flushSettingsUpdate(); +}; + const ensureSettingsRuntimeLifecycle = (): void => { if (_settingsLifecycleInitialized || typeof window === 'undefined') return; _settingsLifecycleInitialized = true; @@ -1640,6 +1655,22 @@ const ensureSettingsRuntimeLifecycle = (): void => { _settingsCache = null; _settingsInflight = null; }); + + // Mirror the deferred safe-storage lifecycle: without these listeners, a + // settings change made within SETTINGS_DEBOUNCE_MS of closing the window is + // silently dropped, and the stale server snapshot wins on next startup. + try { + window.addEventListener('pagehide', flushPendingSettingsBeforeSuspend, { capture: true }); + window.addEventListener('beforeunload', flushPendingSettingsBeforeSuspend, { capture: true }); + if (typeof document !== 'undefined') { + document.addEventListener('visibilitychange', () => { + if (document.visibilityState === 'hidden') flushPendingSettingsBeforeSuspend(); + }); + document.addEventListener('freeze', flushPendingSettingsBeforeSuspend); + } + } catch { + // Restricted environments can reject listeners; the debounce timer still flushes. + } }; const fetchWebSettings = async (context = captureSettingsRuntimeContext()): Promise => { diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index a578d61f..eccda5ea 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -71,7 +71,7 @@ Permission auto-accept policy is authoritative in the active Web server or VS Co Shared safe storage treats durable failures per key. A quota or access failure creates an ephemeral override or tombstone for that key without disabling reads and writes for unrelated keys; later writes retry the durable backend. Deferred adapters retain failed operations for a later flush, and malformed Zustand JSON is removed and treated as missing so hydration can recover. -Project and UI settings use successful settings synchronization as authority. Omitted fields in a complete snapshot reset to canonical client defaults, including an omitted project list becoming empty; transport or settings-load failure dispatches no synchronization event and preserves current state. Settings save responses are partial patches and must not clear unrelated in-memory preferences or local mirrors. +Project and UI settings use successful settings synchronization as authority. Omitted fields in a complete snapshot reset to canonical client defaults, including an omitted project list becoming empty; transport or settings-load failure dispatches no synchronization event and preserves current state. Settings save responses are partial patches and must not clear unrelated in-memory preferences or local mirrors. Debounced settings writes flush best-effort on page hide, document hidden, app freeze, and unload — canceling the pending timer so the write happens exactly once — because a write lost inside the debounce window lets the stale server snapshot override the change on next startup; a hard process kill can still lose the in-flight request. Project ordering defaults to manual. Session display persistence v3 migrates the previously shipped `recent` project order to `manual` while preserving every other explicit sort mode.