diff --git a/src/main/ipc/renderer-shutdown-checkpoint.test.ts b/src/main/ipc/renderer-shutdown-checkpoint.test.ts index 78accf8ab..76fc0509e 100644 --- a/src/main/ipc/renderer-shutdown-checkpoint.test.ts +++ b/src/main/ipc/renderer-shutdown-checkpoint.test.ts @@ -1,10 +1,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { syncHandlers } = vi.hoisted(() => ({ +const { syncHandlers, invokeHandlers } = vi.hoisted(() => ({ syncHandlers: new Map< string, (event: { returnValue?: unknown }, args: Record) => void - >() + >(), + invokeHandlers: new Map Promise<{ ok: boolean }>>() })) vi.mock('electron', () => ({ @@ -16,15 +17,25 @@ vi.mock('electron', () => ({ ) => { syncHandlers.set(channel, handler) } - ) + ), + handle: vi.fn((channel: string, handler: () => Promise<{ ok: boolean }>) => { + invokeHandlers.set(channel, handler) + }) } })) -import { registerRendererShutdownCheckpointHandler } from './renderer-shutdown-checkpoint' +import { + registerRendererShutdownCheckpointHandler, + SHUTDOWN_CHECKPOINT_FLUSH_DEADLINE_MS +} from './renderer-shutdown-checkpoint' + +const AWAIT_CHANNEL = 'app:await-before-unload-checkpoint' describe('registerRendererShutdownCheckpointHandler', () => { beforeEach(() => { syncHandlers.clear() + invokeHandlers.clear() + vi.restoreAllMocks() }) it('stages every shutdown mutation before queueing persistence', () => { @@ -34,7 +45,7 @@ describe('registerRendererShutdownCheckpointHandler', () => { callOrder.push(`session:${hostId ?? 'local'}`) }), updateUI: vi.fn(() => callOrder.push('ui')), - flushPendingAsync: vi.fn(() => { + flushPendingOrThrowAsync: vi.fn(() => { callOrder.push('persist') return Promise.resolve() }) @@ -62,7 +73,11 @@ describe('registerRendererShutdownCheckpointHandler', () => { 'runtime:host-1' ) expect(store.updateUI).toHaveBeenCalledWith({ activeView: 'settings' }) - expect(store.flushPendingAsync).toHaveBeenCalledTimes(1) + expect(store.flushPendingOrThrowAsync).toHaveBeenCalledTimes(1) + // Why: a live app keeps mutating state, so draining to a stable generation would livelock. + expect(store.flushPendingOrThrowAsync).toHaveBeenCalledWith( + expect.objectContaining({ drainToStableGeneration: false }) + ) expect(callOrder).toEqual(['session:local', 'session:runtime:host-1', 'ui', 'persist']) expect(event.returnValue).toEqual({ ok: true }) }) @@ -73,7 +88,7 @@ describe('registerRendererShutdownCheckpointHandler', () => { updateUI: vi.fn(() => { throw new Error('disk full') }), - flushPendingAsync: vi.fn(() => Promise.resolve()) + flushPendingOrThrowAsync: vi.fn(() => Promise.resolve()) } registerRendererShutdownCheckpointHandler(store as never) @@ -84,11 +99,11 @@ describe('registerRendererShutdownCheckpointHandler', () => { expect(event.returnValue).toEqual({ ok: false }) }) - it('does not queue persistence when staging is incomplete', () => { + it('does not queue persistence when staging is incomplete', async () => { const store = { stageWorkspaceSessionBeforeUnload: vi.fn(), updateUI: vi.fn(), - flushPendingAsync: vi.fn(() => Promise.resolve()) + flushPendingOrThrowAsync: vi.fn(() => Promise.resolve()) } registerRendererShutdownCheckpointHandler(store as never) @@ -99,19 +114,16 @@ describe('registerRendererShutdownCheckpointHandler', () => { const event: { returnValue?: unknown } = {} handler?.(event, { sessions: [], ui: { activeView: 'settings' } }) - expect(store.flushPendingAsync).not.toHaveBeenCalled() + expect(store.flushPendingOrThrowAsync).not.toHaveBeenCalled() expect(event.returnValue).toEqual({ ok: false }) + await expect(invokeHandlers.get(AWAIT_CHANNEL)?.()).resolves.toEqual({ ok: false }) }) - it('returns before the asynchronous persistence settles', () => { - let resolve!: () => void - const pending = new Promise((next) => { - resolve = next - }) + it('stages synchronously without waiting on the durable write', () => { const store = { stageWorkspaceSessionBeforeUnload: vi.fn(), updateUI: vi.fn(), - flushPendingAsync: vi.fn(() => pending) + flushPendingOrThrowAsync: vi.fn(() => new Promise(() => {})) } registerRendererShutdownCheckpointHandler(store as never) @@ -120,6 +132,84 @@ describe('registerRendererShutdownCheckpointHandler', () => { handler?.(event, { sessions: [], ui: { activeView: 'settings' } }) expect(event.returnValue).toEqual({ ok: true }) - resolve() + }) + + it('holds the checkpoint open until the durable write settles', async () => { + let resolveFlush!: () => void + const store = { + stageWorkspaceSessionBeforeUnload: vi.fn(), + updateUI: vi.fn(), + flushPendingOrThrowAsync: vi.fn( + () => + new Promise((next) => { + resolveFlush = next + }) + ) + } + registerRendererShutdownCheckpointHandler(store as never) + + syncHandlers.get('app:stage-before-unload-sync')?.({}, { sessions: [], ui: {} }) + const checkpoint = invokeHandlers.get(AWAIT_CHANNEL)?.() + let settled: unknown = 'pending' + void checkpoint?.then((result) => { + settled = result + }) + + await Promise.resolve() + expect(settled).toBe('pending') + + resolveFlush() + await expect(checkpoint).resolves.toEqual({ ok: true }) + }) + + it('reports a failed durable write instead of a successful checkpoint', async () => { + const store = { + stageWorkspaceSessionBeforeUnload: vi.fn(), + updateUI: vi.fn(), + flushPendingOrThrowAsync: vi.fn(() => Promise.reject(new Error('disk full'))) + } + vi.spyOn(console, 'error').mockImplementation(() => {}) + registerRendererShutdownCheckpointHandler(store as never) + + const event: { returnValue?: unknown } = {} + syncHandlers.get('app:stage-before-unload-sync')?.(event, { sessions: [], ui: {} }) + + expect(event.returnValue).toEqual({ ok: true }) + await expect(invokeHandlers.get(AWAIT_CHANNEL)?.()).resolves.toEqual({ ok: false }) + }) + + it('fails the checkpoint when the durable write outlives its deadline', async () => { + const store = { + stageWorkspaceSessionBeforeUnload: vi.fn(), + updateUI: vi.fn(), + flushPendingOrThrowAsync: vi.fn( + (_options: { signal: AbortSignal }) => new Promise(() => {}) + ) + } + vi.spyOn(console, 'error').mockImplementation(() => {}) + vi.useFakeTimers() + try { + registerRendererShutdownCheckpointHandler(store as never) + syncHandlers.get('app:stage-before-unload-sync')?.({}, { sessions: [], ui: {} }) + const checkpoint = invokeHandlers.get(AWAIT_CHANNEL)?.() + + await vi.advanceTimersByTimeAsync(SHUTDOWN_CHECKPOINT_FLUSH_DEADLINE_MS) + + await expect(checkpoint).resolves.toEqual({ ok: false }) + expect(store.flushPendingOrThrowAsync.mock.calls[0]?.[0]?.signal.aborted).toBe(true) + } finally { + vi.useRealTimers() + } + }) + + it('reports success before any checkpoint is staged', async () => { + const store = { + stageWorkspaceSessionBeforeUnload: vi.fn(), + updateUI: vi.fn(), + flushPendingOrThrowAsync: vi.fn(() => Promise.resolve()) + } + registerRendererShutdownCheckpointHandler(store as never) + + await expect(invokeHandlers.get(AWAIT_CHANNEL)?.()).resolves.toEqual({ ok: true }) }) }) diff --git a/src/main/ipc/renderer-shutdown-checkpoint.ts b/src/main/ipc/renderer-shutdown-checkpoint.ts index a1fafa166..e875f950d 100644 --- a/src/main/ipc/renderer-shutdown-checkpoint.ts +++ b/src/main/ipc/renderer-shutdown-checkpoint.ts @@ -8,7 +8,42 @@ type StageBeforeUnloadSyncArgs = { ui: Partial } +export type ShutdownCheckpointResult = { ok: boolean } + +/** Matches the will-quit teardown budget so a stalled disk can't strand a restart. */ +export const SHUTDOWN_CHECKPOINT_FLUSH_DEADLINE_MS = 20_000 + +function flushStagedStateWithDeadline(store: Store): Promise { + const controller = new AbortController() + let timer: ReturnType | null = null + const deadline = new Promise((resolve) => { + timer = setTimeout(() => { + controller.abort() + console.error('[app] Timed out persisting staged renderer state') + resolve({ ok: false }) + }, SHUTDOWN_CHECKPOINT_FLUSH_DEADLINE_MS) + }) + // Why not drain to a stable generation: the staged snapshot lands in the first + // write, and a live app keeps mutating state, which would livelock the drain. + const flush = store + .flushPendingOrThrowAsync({ signal: controller.signal, drainToStableGeneration: false }) + .then((): ShutdownCheckpointResult => ({ ok: true })) + .catch((error): ShutdownCheckpointResult => { + console.error('[app] Failed to persist staged renderer state:', error) + return { ok: false } + }) + return Promise.race([flush, deadline]).finally(() => { + if (timer) { + clearTimeout(timer) + } + }) +} + export function registerRendererShutdownCheckpointHandler(store: Store): void { + // Why: beforeunload cannot await, so the sync reply only reports staging. + // Durability is joined out-of-band by paths that are about to navigate. + let pendingCheckpoint: Promise = Promise.resolve({ ok: true }) + ipcMain.on('app:stage-before-unload-sync', (event, args: StageBeforeUnloadSyncArgs) => { let ok = true try { @@ -20,11 +55,12 @@ export function registerRendererShutdownCheckpointHandler(store: Store): void { console.error('[app] Failed to stage renderer state before unload:', error) ok = false } - if (ok) { - void store.flushPendingAsync().catch((error) => { - console.error('[app] Failed to persist staged renderer state:', error) - }) - } + pendingCheckpoint = ok ? flushStagedStateWithDeadline(store) : Promise.resolve({ ok: false }) event.returnValue = { ok } }) + + ipcMain.handle( + 'app:await-before-unload-checkpoint', + (): Promise => pendingCheckpoint + ) } diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 40660eb8a..077d9b871 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -7291,11 +7291,13 @@ export class Store { return this.flushCurrentStateAsync(false, undefined, false).catch(() => {}) } - flushPendingOrThrowAsync(options: { signal?: AbortSignal } = {}): Promise { + flushPendingOrThrowAsync( + options: { signal?: AbortSignal; drainToStableGeneration?: boolean } = {} + ): Promise { if (this.writesFrozen || this.quitFlushStarted) { return Promise.reject(new Error('Cannot flush while persistence is finalized')) } - return this.flushCurrentStateAsync(false, options.signal) + return this.flushCurrentStateAsync(false, options.signal, options.drainToStableGeneration) } // Async twin of flushOrThrow: durable state only. Active-view and GitHub sidecars are diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 2acb211c3..7f60af7e8 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -990,6 +990,9 @@ export type AppApi = { sessions: { state: WorkspaceSessionState; hostId?: ExecutionHostId }[] ui: Partial }) => void + /** Resolves once the last staged checkpoint is durably written; rejects if that + * write failed, so a reload/restart can abort instead of losing the snapshot. */ + awaitBeforeUnloadCheckpoint: () => Promise /** Resolves when the daemon PTY provider and hook receiver have either * started or failed open for the first BrowserWindow. */ awaitFirstWindowStartupServices: () => Promise diff --git a/src/preload/index.ts b/src/preload/index.ts index c6bbf3739..de829aafe 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -290,6 +290,17 @@ import { registerRendererRestartIpcRelays } from './renderer-restart-wiring' +// Why: the sync checkpoint only stages; this joins its durable write so a +// navigating path can abort instead of losing the staged session. +async function awaitBeforeUnloadCheckpoint(): Promise { + const result = (await ipcRenderer.invoke('app:await-before-unload-checkpoint')) as { + ok?: unknown + } + if (result?.ok !== true) { + throw new Error('Failed to persist renderer state before unload.') + } +} + type NativeFileDropCallback = (data: NativeFileDropPayload) => void const nativeFileDropCallbacks: NativeFileDropCallback[] = [] @@ -479,7 +490,8 @@ const api = { restart: async (): Promise => { await prepareRendererForAppRestart(window, { startedEventName: ORCA_APP_RESTART_STARTED_EVENT, - abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT + abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT, + awaitCheckpoint: awaitBeforeUnloadCheckpoint }) try { return await ipcRenderer.invoke('app:restart') @@ -2974,8 +2986,11 @@ const api = { showLinuxPackage: () => ipcRenderer.invoke('updater:showLinuxPackage'), listBuilds: (channel) => ipcRenderer.invoke('updater:listBuilds', channel), quitAndInstall: (): Promise => - prepareAndInvokeUpdaterInstall(window, updaterQuitAbortRelay, () => - ipcRenderer.invoke('updater:quitAndInstall') + prepareAndInvokeUpdaterInstall( + window, + updaterQuitAbortRelay, + () => ipcRenderer.invoke('updater:quitAndInstall'), + awaitBeforeUnloadCheckpoint ), onStatus: (callback) => { diff --git a/src/preload/renderer-restart-wiring.test.ts b/src/preload/renderer-restart-wiring.test.ts index 56cd5d3dd..0198c72a4 100644 --- a/src/preload/renderer-restart-wiring.test.ts +++ b/src/preload/renderer-restart-wiring.test.ts @@ -44,10 +44,27 @@ describe('renderer restart wiring', () => { throw new Error('IPC failed') }) - await expect(prepareAndInvokeUpdaterInstall(eventTarget, relay, invoke)).rejects.toThrow( - 'IPC failed' - ) + await expect( + prepareAndInvokeUpdaterInstall(eventTarget, relay, invoke, async () => { + calls.push('checkpoint-flushed') + }) + ).rejects.toThrow('IPC failed') - expect(calls).toEqual(['prepared', 'marked', 'invoked', 'aborted']) + expect(calls).toEqual(['prepared', 'checkpoint-flushed', 'marked', 'invoked', 'aborted']) + }) + + it('never installs the update when the shutdown checkpoint fails to persist', async () => { + const eventTarget = new EventTarget() + const invoke = vi.fn(() => Promise.resolve()) + const relay = { markPrepared: vi.fn(), abort: vi.fn() } + + await expect( + prepareAndInvokeUpdaterInstall(eventTarget, relay, invoke, () => + Promise.reject(new Error('Failed to persist renderer state before unload.')) + ) + ).rejects.toThrow('Failed to persist renderer state before unload.') + + expect(invoke).not.toHaveBeenCalled() + expect(relay.markPrepared).not.toHaveBeenCalled() }) }) diff --git a/src/preload/renderer-restart-wiring.ts b/src/preload/renderer-restart-wiring.ts index fbd3e26a7..254d38640 100644 --- a/src/preload/renderer-restart-wiring.ts +++ b/src/preload/renderer-restart-wiring.ts @@ -26,11 +26,13 @@ export function registerRendererRestartIpcRelays( export async function prepareAndInvokeUpdaterInstall( eventTarget: EventTarget, relay: Pick, - invoke: () => Promise + invoke: () => Promise, + awaitCheckpoint: () => Promise ): Promise { await prepareRendererForAppRestart(eventTarget, { startedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, - abortedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT + abortedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, + awaitCheckpoint }) relay.markPrepared() try { diff --git a/src/renderer/src/lib/lazy-chunk-recovery-reload.test.ts b/src/renderer/src/lib/lazy-chunk-recovery-reload.test.ts new file mode 100644 index 000000000..9cdf4fa72 --- /dev/null +++ b/src/renderer/src/lib/lazy-chunk-recovery-reload.test.ts @@ -0,0 +1,39 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../../../shared/renderer-shutdown-events' +import { requestLazyChunkRecoveryReload } from './lazy-chunk-recovery-reload' + +describe('requestLazyChunkRecoveryReload', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('refuses the reload when the staged checkpoint never reaches disk', async () => { + const reload = vi.spyOn(window.location, 'reload').mockImplementation(() => undefined) + + await expect( + requestLazyChunkRecoveryReload(window, () => + Promise.reject(new Error('Failed to persist renderer state before unload.')) + ) + ).resolves.toBe('checkpoint-refused') + + expect(reload).not.toHaveBeenCalled() + }) + + it('navigates only after the checkpoint is durably written', async () => { + const order: string[] = [] + vi.spyOn(window.location, 'reload').mockImplementation(() => { + order.push('reload') + // A real landed reload destroys the document; veto so the wait settles. + window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT)) + }) + + await expect( + requestLazyChunkRecoveryReload(window, async () => { + order.push('flushed') + }) + ).resolves.toBe('unload-vetoed') + + expect(order).toEqual(['flushed', 'reload']) + }) +}) diff --git a/src/renderer/src/lib/lazy-chunk-recovery-reload.ts b/src/renderer/src/lib/lazy-chunk-recovery-reload.ts index 5bc259a7a..64e78fc28 100644 --- a/src/renderer/src/lib/lazy-chunk-recovery-reload.ts +++ b/src/renderer/src/lib/lazy-chunk-recovery-reload.ts @@ -49,12 +49,16 @@ function waitForRefusedNavigation(win: Window): RefusedNavigationWait { /** Resolves only if the navigation is refused; a landed reload destroys this document. */ export async function requestLazyChunkRecoveryReload( - win: Window + win: Window, + // Hosts without a preload bridge stage durably in-process; nothing to join. + awaitCheckpoint: () => Promise = () => + window.api?.app?.awaitBeforeUnloadCheckpoint?.() ?? Promise.resolve() ): Promise { try { await prepareRendererForAppRestart(win, { startedEventName: ORCA_APP_RESTART_STARTED_EVENT, - abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT + abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT, + awaitCheckpoint }) } catch { // Never reload over editor buffers that could not be backed up. diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index dc9903bea..6199c35b8 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -556,6 +556,8 @@ function createWebPreloadApi(): Partial { } writeJson(UI_STORAGE_KEY, mergeWebUIState(readLocalWebUIState(), ui)) }, + // Staging already wrote through to browser storage, so there is nothing left to join. + awaitBeforeUnloadCheckpoint: () => Promise.resolve(), awaitFirstWindowStartupServices: () => Promise.resolve(), recoverLegacyWorkerTerminalsForRendererStartup: () => Promise.resolve(), startupDiagnostic: () => Promise.resolve(), diff --git a/src/shared/renderer-restart-preparation.test.ts b/src/shared/renderer-restart-preparation.test.ts index 026654426..925aedbf8 100644 --- a/src/shared/renderer-restart-preparation.test.ts +++ b/src/shared/renderer-restart-preparation.test.ts @@ -18,7 +18,8 @@ describe('prepareRendererForAppRestart', () => { await expect( prepareRendererForAppRestart(eventTarget, { startedEventName: 'restart-started', - abortedEventName: 'restart-aborted' + abortedEventName: 'restart-aborted', + awaitCheckpoint: () => Promise.resolve() }) ).rejects.toThrow('Renderer shutdown checkpoint was not completed.') @@ -26,6 +27,53 @@ describe('prepareRendererForAppRestart', () => { expect(checkpoint).toHaveBeenCalledTimes(1) expect(aborted).toHaveBeenCalledTimes(1) }) + + it('waits for the durable checkpoint write before the restart proceeds', async () => { + const eventTarget = new EventTarget() + const order: string[] = [] + let releaseCheckpoint!: () => void + eventTarget.addEventListener('beforeunload', () => order.push('staged')) + + const prepared = prepareRendererForAppRestart(eventTarget, { + startedEventName: 'restart-started', + abortedEventName: 'restart-aborted', + awaitCheckpoint: () => + new Promise((resolve) => { + order.push('awaiting-flush') + releaseCheckpoint = () => { + order.push('flushed') + resolve() + } + }) + }) + let settled = false + void prepared.then(() => { + settled = true + }) + + await Promise.resolve() + expect(settled).toBe(false) + + releaseCheckpoint() + await prepared + expect(order).toEqual(['staged', 'awaiting-flush', 'flushed']) + }) + + it('aborts the restart when the staged state cannot be persisted', async () => { + const eventTarget = new EventTarget() + const aborted = vi.fn() + eventTarget.addEventListener('restart-aborted', aborted) + + await expect( + prepareRendererForAppRestart(eventTarget, { + startedEventName: 'restart-started', + abortedEventName: 'restart-aborted', + awaitCheckpoint: () => Promise.reject(new Error('Failed to persist renderer state.')) + }) + ).rejects.toThrow('Failed to persist renderer state.') + + expect(aborted).toHaveBeenCalledTimes(1) + }) }) describe('createUpdaterQuitAbortRelay', () => { diff --git a/src/shared/renderer-restart-preparation.ts b/src/shared/renderer-restart-preparation.ts index 78560dd76..19aeb20ff 100644 --- a/src/shared/renderer-restart-preparation.ts +++ b/src/shared/renderer-restart-preparation.ts @@ -7,6 +7,8 @@ import type { UpdateStatus } from './types' export type AppRestartPrepOptions = { startedEventName: string abortedEventName: string + /** Joins the durable write of the state the checkpoint staged; rejects if it failed. */ + awaitCheckpoint: () => Promise } function requestEditorHotExitBackup(eventTarget: EventTarget): Promise { @@ -36,7 +38,7 @@ function requestEditorHotExitBackup(eventTarget: EventTarget): Promise { export async function prepareRendererForAppRestart( eventTarget: EventTarget, - { startedEventName, abortedEventName }: AppRestartPrepOptions + { startedEventName, abortedEventName, awaitCheckpoint }: AppRestartPrepOptions ): Promise { eventTarget.dispatchEvent(new Event(startedEventName)) @@ -48,6 +50,9 @@ export async function prepareRendererForAppRestart( if (!accepted) { throw new Error('Renderer shutdown checkpoint was not completed.') } + // Why: the checkpoint only stages synchronously. Navigating before that + // write lands loses the session snapshot to a crash or power loss. + await awaitCheckpoint() } catch (error) { eventTarget.dispatchEvent(new Event(abortedEventName)) throw error