From 4d7ebd9818abd00b5056b0a1fb0dd2180028ffd0 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:38:00 -0700 Subject: [PATCH] fix(quit): harden async persistence finalization --- ...ve-view-preference-sync-flush-veto.test.ts | 147 +++++++- src/main/active-view-preference.ts | 93 ++++- .../agent-auth-restart-preservation.test.ts | 41 ++- src/main/agent-auth-restart-preservation.ts | 24 +- src/main/durable-file-write.ts | 23 +- src/main/index.ts | 6 +- src/main/ipc/orca-profiles.test.ts | 30 +- src/main/ipc/orca-profiles.ts | 10 +- .../ipc/renderer-shutdown-checkpoint.test.ts | 83 ++--- src/main/ipc/renderer-shutdown-checkpoint.ts | 25 +- .../profile-persistence-deadline.ts | 21 ++ .../persistence-async-write-syscalls.test.ts | 152 +++++++- src/main/persistence.ts | 327 ++++++++++++++---- .../quit-path-durable-write-blocking.test.ts | 134 ++++++- src/main/quit-teardown-start-gate.test.ts | 15 + src/main/quit-teardown-start-gate.ts | 12 + src/main/stats/collector-async-save.test.ts | 87 ++++- src/main/stats/collector.ts | 160 ++------- src/main/stats/stats-file-loader.ts | 38 ++ src/main/stats/stats-snapshot-writer.ts | 134 +++++++ ...nal-scrollback-snapshot-async-migration.ts | 54 +++ src/main/terminal-scrollback-snapshots.ts | 46 +++ .../attach-main-window-services.test.ts | 10 +- .../window/attach-main-window-services.ts | 2 +- src/preload/api-types.ts | 5 +- src/preload/index.ts | 8 +- src/renderer/src/App.tsx | 4 +- src/renderer/src/app-startup-routing.test.ts | 2 +- src/renderer/src/web/web-preload-api.test.ts | 2 +- src/renderer/src/web/web-preload-api.ts | 2 +- 30 files changed, 1351 insertions(+), 346 deletions(-) create mode 100644 src/main/orca-profiles/profile-persistence-deadline.ts create mode 100644 src/main/quit-teardown-start-gate.test.ts create mode 100644 src/main/quit-teardown-start-gate.ts create mode 100644 src/main/stats/stats-file-loader.ts create mode 100644 src/main/stats/stats-snapshot-writer.ts create mode 100644 src/main/terminal-scrollback-snapshot-async-migration.ts diff --git a/src/main/active-view-preference-sync-flush-veto.test.ts b/src/main/active-view-preference-sync-flush-veto.test.ts index f02c212a73a..343f6fb9f2f 100644 --- a/src/main/active-view-preference-sync-flush-veto.test.ts +++ b/src/main/active-view-preference-sync-flush-veto.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { mkdtempSync, readdirSync, readFileSync, rmSync } from 'node:fs' import type * as FsPromises from 'node:fs/promises' +import type * as Fs from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -11,10 +12,25 @@ import { join } from 'node:path' const gate = vi.hoisted(() => ({ blockRename: false, + blockAfterRename: false, waiters: [] as (() => void)[], - renameCalls: 0 + afterRenameWaiters: [] as (() => void)[], + renameCompleted: [] as (() => void)[], + renameCalls: 0, + failUnlink: false })) +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs') + const unlinkSync = ((...args: Parameters) => { + if (gate.failUnlink) { + throw Object.assign(new Error('busy'), { code: 'EBUSY' }) + } + return actual.unlinkSync(...args) + }) as typeof actual.unlinkSync + return { ...actual, unlinkSync } +}) + vi.mock('node:fs/promises', async () => { const actual = await vi.importActual('node:fs/promises') const rename = (async (...args: Parameters) => { @@ -22,7 +38,12 @@ vi.mock('node:fs/promises', async () => { if (gate.blockRename) { await new Promise((resolve) => gate.waiters.push(resolve)) } - return actual.rename(...args) + const result = await actual.rename(...args) + gate.renameCompleted.splice(0).forEach((resolve) => resolve()) + if (gate.blockAfterRename) { + await new Promise((resolve) => gate.afterRenameWaiters.push(resolve)) + } + return result }) as typeof actual.rename return { ...actual, rename } }) @@ -34,8 +55,12 @@ describe('ActiveViewPreference sync flush vs. parked async rename', () => { beforeEach(() => { dir = mkdtempSync(join(tmpdir(), 'orca-active-view-')) gate.blockRename = false + gate.blockAfterRename = false gate.waiters = [] + gate.afterRenameWaiters = [] + gate.renameCompleted = [] gate.renameCalls = 0 + gate.failUnlink = false }) afterEach(() => { @@ -65,4 +90,122 @@ describe('ActiveViewPreference sync flush vs. parked async rename', () => { expect(JSON.parse(readFileSync(viewFile(), 'utf-8')).activeView).toBe('tasks') expect(readdirSync(dir).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) }) + + it('an unlink veto failure queues a newest-value correction', async () => { + const { ActiveViewPreference } = await import('./active-view-preference') + const pref = new ActiveViewPreference(join(dir, 'orca-data.json'), 'terminal') + + gate.blockRename = true + pref.set('activity') + await vi.waitFor(() => expect(gate.renameCalls).toBeGreaterThan(0)) + + pref.set('tasks') + gate.failUnlink = true + expect(() => pref.flushOrThrow()).toThrow('busy') + + gate.failUnlink = false + gate.blockRename = false + gate.waiters.splice(0).forEach((resolve) => resolve()) + await pref.waitForPendingWrite() + + expect(JSON.parse(readFileSync(viewFile(), 'utf-8')).activeView).toBe('tasks') + expect(readdirSync(dir).filter((f) => f.endsWith('.tmp'))).toHaveLength(0) + }) + + it('a completed stale rename cannot regress the persisted marker', async () => { + const { ActiveViewPreference } = await import('./active-view-preference') + const pref = new ActiveViewPreference(join(dir, 'orca-data.json'), 'terminal') + const renameCompleted = new Promise((resolve) => gate.renameCompleted.push(resolve)) + + gate.blockAfterRename = true + pref.set('activity') + await renameCompleted + + pref.set('tasks') + pref.flushOrThrow() + gate.blockAfterRename = false + gate.afterRenameWaiters.splice(0).forEach((resolve) => resolve()) + await pref.waitForPendingWrite() + + pref.set('activity') + await vi.waitFor(() => { + expect(JSON.parse(readFileSync(viewFile(), 'utf-8')).activeView).toBe('activity') + }) + await pref.waitForPendingWrite() + }) + + it('a pending flush drains a view changed during its rename', async () => { + const { ActiveViewPreference } = await import('./active-view-preference') + const pref = new ActiveViewPreference(join(dir, 'orca-data.json'), 'terminal') + + gate.blockRename = true + pref.set('activity') + const flush = pref.flushPendingAsync() + await vi.waitFor(() => expect(gate.renameCalls).toBeGreaterThan(0)) + + pref.set('tasks') + gate.blockRename = false + gate.waiters.splice(0).forEach((resolve) => resolve()) + await flush + + expect(JSON.parse(readFileSync(viewFile(), 'utf-8')).activeView).toBe('tasks') + expect(gate.renameCalls).toBe(2) + }) + + it('coalesces concurrent pending flushes while draining the newest view', async () => { + const { ActiveViewPreference } = await import('./active-view-preference') + const pref = new ActiveViewPreference(join(dir, 'orca-data.json'), 'terminal') + + gate.blockRename = true + pref.set('activity') + const firstFlush = pref.flushPendingAsync() + await vi.waitFor(() => expect(gate.renameCalls).toBeGreaterThan(0)) + + pref.set('tasks') + const secondFlush = pref.flushPendingAsync() + expect(secondFlush).toBe(firstFlush) + gate.blockRename = false + gate.waiters.splice(0).forEach((resolve) => resolve()) + await Promise.all([firstFlush, secondFlush]) + + expect(JSON.parse(readFileSync(viewFile(), 'utf-8')).activeView).toBe('tasks') + expect(gate.renameCalls).toBe(2) + }) + + it('keeps an unabortable shared drain alive when an attached signal aborts', async () => { + const { ActiveViewPreference } = await import('./active-view-preference') + const pref = new ActiveViewPreference(join(dir, 'orca-data.json'), 'terminal') + const controller = new AbortController() + + gate.blockRename = true + pref.set('activity') + const flush = pref.flushPendingAsync() + await vi.waitFor(() => expect(gate.renameCalls).toBeGreaterThan(0)) + expect(pref.flushPendingAsync(controller.signal)).toBe(flush) + + pref.set('tasks') + controller.abort() + gate.blockRename = false + gate.waiters.splice(0).forEach((resolve) => resolve()) + await expect(flush).resolves.toBeUndefined() + expect(JSON.parse(readFileSync(viewFile(), 'utf-8')).activeView).toBe('tasks') + expect(gate.renameCalls).toBe(2) + }) + + it('stops an abortable-only drain after its signal aborts', async () => { + const { ActiveViewPreference } = await import('./active-view-preference') + const pref = new ActiveViewPreference(join(dir, 'orca-data.json'), 'terminal') + const controller = new AbortController() + + gate.blockRename = true + pref.set('activity') + const flush = pref.flushPendingAsync(controller.signal) + await vi.waitFor(() => expect(gate.renameCalls).toBeGreaterThan(0)) + + controller.abort() + gate.blockRename = false + gate.waiters.splice(0).forEach((resolve) => resolve()) + await expect(flush).rejects.toThrow('Active-view flush aborted') + expect(gate.renameCalls).toBe(1) + }) }) diff --git a/src/main/active-view-preference.ts b/src/main/active-view-preference.ts index 425fe6e1ec6..5bd78b14713 100644 --- a/src/main/active-view-preference.ts +++ b/src/main/active-view-preference.ts @@ -34,11 +34,15 @@ export class ActiveViewPreference { private persistedActiveView: TopLevelView | null private writeTimer: ReturnType | null = null private pendingWrite: Promise | null = null + private pendingFlush: Promise | null = null + private flushSignals = new Set() + private unabortableFlushConsumers = 0 private writeGeneration = 0 /** Temp path of the write awaiting its rename; flushOrThrow removes it to veto that rename. */ private inFlightTmpFile: string | null = null /** Set by flushAsync so the quit flush is the final write; see scheduleSave. */ private quitFlushStarted = false + private quitFlushPromise: Promise | null = null constructor(dataFile: string, legacyActiveView: unknown) { this.file = getActiveViewPreferenceFile(dataFile) @@ -115,10 +119,15 @@ export class ActiveViewPreference { try { await rename(tmpFile, this.file) renamed = true - this.persistedActiveView = activeView + if (generation === this.writeGeneration) { + this.persistedActiveView = activeView + } } catch (err) { // ENOENT: flushOrThrow removed this temp file because it already wrote a fresher view. - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { + if ( + (err as NodeJS.ErrnoException).code !== 'ENOENT' || + generation === this.writeGeneration + ) { throw err } } finally { @@ -134,13 +143,15 @@ export class ActiveViewPreference { } flushOrThrow(): void { + if (this.quitFlushStarted) { + throw new Error('Cannot synchronously flush active view after final persistence has started') + } if (this.writeTimer) { clearTimeout(this.writeTimer) this.writeTimer = null } const asyncWriteWasInFlight = this.pendingWrite !== null this.writeGeneration += 1 - this.pendingWrite = null if (!asyncWriteWasInFlight && this.activeView === this.persistedActiveView) { return } @@ -150,10 +161,13 @@ export class ActiveViewPreference { if (this.inFlightTmpFile) { try { unlinkSync(this.inFlightTmpFile) - } catch { - // Already renamed or never created. + this.inFlightTmpFile = null + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + void this.enqueueWrite(this.activeView, this.writeGeneration) + throw error + } } - this.inFlightTmpFile = null } mkdirSync(dirname(this.file), { recursive: true }) const tmpFile = `${this.file}.${process.pid}.${this.writeGeneration}.tmp` @@ -164,18 +178,67 @@ export class ActiveViewPreference { /** Async twin of flushOrThrow() for the quit path; never throws so it can join the teardown barrier. */ flushAsync(): Promise { + if (this.quitFlushPromise) { + return this.quitFlushPromise + } this.quitFlushStarted = true - if (this.writeTimer) { - clearTimeout(this.writeTimer) - this.writeTimer = null + this.quitFlushPromise = this.flushPendingAsync() + return this.quitFlushPromise + } + + flushPendingAsync(signal?: AbortSignal): Promise { + if (signal?.aborted) { + return Promise.reject(new Error('Active-view flush aborted')) } - // Why still write when a write is already in flight: that one may carry a stale view, - // and its generation guard would drop it — matching flushOrThrow's `force` reasoning. - if (this.pendingWrite === null && this.activeView === this.persistedActiveView) { - return Promise.resolve() + if (signal) { + this.flushSignals.add(signal) + } else { + this.unabortableFlushConsumers += 1 } - this.writeGeneration += 1 - return this.enqueueWrite(this.activeView, this.writeGeneration) + if (this.pendingFlush) { + return this.pendingFlush + } + const run = this.drainPendingWrites() + const tracked = run.finally(() => { + if (this.pendingFlush === tracked) { + this.pendingFlush = null + this.flushSignals.clear() + this.unabortableFlushConsumers = 0 + } + }) + this.pendingFlush = tracked + return tracked + } + + private async drainPendingWrites(): Promise { + for (;;) { + if (this.allFlushConsumersAborted()) { + throw new Error('Active-view flush aborted') + } + if (this.writeTimer) { + clearTimeout(this.writeTimer) + this.writeTimer = null + } + if (this.pendingWrite === null && this.activeView === this.persistedActiveView) { + return + } + const generation = ++this.writeGeneration + await this.enqueueWrite(this.activeView, generation) + if (this.allFlushConsumersAborted()) { + throw new Error('Active-view flush aborted') + } + if (generation === this.writeGeneration) { + return + } + } + } + + private allFlushConsumersAborted(): boolean { + return ( + this.unabortableFlushConsumers === 0 && + this.flushSignals.size > 0 && + [...this.flushSignals].every((signal) => signal.aborted) + ) } async waitForPendingWrite(): Promise { diff --git a/src/main/agent-auth-restart-preservation.test.ts b/src/main/agent-auth-restart-preservation.test.ts index 057e3d876f7..441ec609624 100644 --- a/src/main/agent-auth-restart-preservation.test.ts +++ b/src/main/agent-auth-restart-preservation.test.ts @@ -24,7 +24,7 @@ describe('preserveAgentAuthBeforeRestart', () => { }) }, store: { - flush: vi.fn(() => { + flushPendingOrThrowAsync: vi.fn(async () => { calls.push('flush') }) } @@ -43,7 +43,7 @@ describe('preserveAgentAuthBeforeRestart', () => { syncActiveWslSelectionsBeforeRestart }, store: { - flush: vi.fn() + flushPendingOrThrowAsync: vi.fn() } }) @@ -70,7 +70,7 @@ describe('preserveAgentAuthBeforeRestart', () => { }) }, store: { - flush: vi.fn(() => { + flushPendingOrThrowAsync: vi.fn(async () => { calls.push('flush') }) } @@ -81,7 +81,7 @@ describe('preserveAgentAuthBeforeRestart', () => { it('continues after WSL Codex preservation fails', async () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const flush = vi.fn() + const flushPendingOrThrowAsync = vi.fn() await preserveAgentAuthBeforeRestart({ codexRuntimeHome: { @@ -91,25 +91,25 @@ describe('preserveAgentAuthBeforeRestart', () => { }) }, store: { - flush + flushPendingOrThrowAsync } }) - expect(flush).toHaveBeenCalledTimes(1) + expect(flushPendingOrThrowAsync).toHaveBeenCalledTimes(1) expect(JSON.stringify(warn.mock.calls)).not.toContain('token-secret') }) it('flushes the store when auth services are missing', async () => { - const flush = vi.fn() + const flushPendingOrThrowAsync = vi.fn() - await preserveAgentAuthBeforeRestart({ store: { flush } }) + await preserveAgentAuthBeforeRestart({ store: { flushPendingOrThrowAsync } }) - expect(flush).toHaveBeenCalledTimes(1) + expect(flushPendingOrThrowAsync).toHaveBeenCalledTimes(1) }) it('logs secret-free warnings and does not throw when sync fails', async () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const flush = vi.fn() + const flushPendingOrThrowAsync = vi.fn() await expect( preserveAgentAuthBeforeRestart({ @@ -124,11 +124,11 @@ describe('preserveAgentAuthBeforeRestart', () => { throw new Error('claude-token-secret') }) }, - store: { flush } + store: { flushPendingOrThrowAsync } }) ).resolves.toBeUndefined() - expect(flush).toHaveBeenCalledTimes(1) + expect(flushPendingOrThrowAsync).toHaveBeenCalledTimes(1) expect(warn).toHaveBeenCalledTimes(2) expect(JSON.stringify(warn.mock.calls)).not.toContain('token-secret') }) @@ -150,7 +150,7 @@ describe('preserveAgentAuthBeforeRestart', () => { }) }, store: { - flush: vi.fn(() => { + flushPendingOrThrowAsync: vi.fn(async () => { calls.push('flush') }) } @@ -169,4 +169,19 @@ describe('preserveAgentAuthBeforeRestart', () => { expect(calls).toEqual(['claude-start', 'flush', 'claude-finish']) }) + + it('bounds a store flush that never settles', async () => { + vi.useFakeTimers() + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const preservation = preserveAgentAuthBeforeRestart({ + store: { flushPendingOrThrowAsync: vi.fn(() => new Promise(() => {})) } + }) + + await vi.advanceTimersByTimeAsync(2_000) + await preservation + + expect(warn).toHaveBeenCalledWith( + '[agent-auth-restart] Store persistence exceeded 2000ms; continuing restart/update' + ) + }) }) diff --git a/src/main/agent-auth-restart-preservation.ts b/src/main/agent-auth-restart-preservation.ts index cb3fe94921a..eaae45f331b 100644 --- a/src/main/agent-auth-restart-preservation.ts +++ b/src/main/agent-auth-restart-preservation.ts @@ -9,9 +9,12 @@ type CodexRuntimeAuthSync = Pick< 'syncForCurrentSelection' | 'syncActiveWslSelectionsBeforeRestart' > type ClaudeRuntimeAuthSync = Pick -type ShutdownStore = Pick +type ShutdownStore = Pick -type AuthPreservationStep = 'Codex auth preservation' | 'Claude auth preservation' +type AuthPreservationStep = + | 'Codex auth preservation' + | 'Claude auth preservation' + | 'Store persistence' export type AgentAuthRestartPreservationOptions = { codexRuntimeHome?: CodexRuntimeAuthSync | null @@ -45,10 +48,13 @@ export async function preserveAgentAuthBeforeRestart({ logStepTimeout('Codex auth preservation', 0) } - try { - store?.flush() - } catch (error) { - logStoreFlushFailure(error) + if (store) { + const storeRemainingMs = Math.max(0, AUTH_PRESERVATION_TIMEOUT_MS - (Date.now() - startedAt)) + await runWithinLifecycleTimeout( + 'Store persistence', + () => store.flushPendingOrThrowAsync(), + storeRemainingMs + ) } } @@ -109,12 +115,6 @@ function logStepTimeout(step: AuthPreservationStep, timeoutMs: number): void { console.warn(`[agent-auth-restart] ${step} exceeded ${timeoutMs}ms; continuing restart/update`) } -function logStoreFlushFailure(error: unknown): void { - console.warn( - `[agent-auth-restart] Store flush failed (${describeErrorKind(error)}); continuing restart/update` - ) -} - function describeErrorKind(error: unknown): string { if (error instanceof Error) { return error.name || 'Error' diff --git a/src/main/durable-file-write.ts b/src/main/durable-file-write.ts index ac57e7d465d..9de0ec28109 100644 --- a/src/main/durable-file-write.ts +++ b/src/main/durable-file-write.ts @@ -4,7 +4,7 @@ // hour's loss; fsync stops it from happening. import { closeSync, fsyncSync, openSync, renameSync, writeFileSync } from 'node:fs' -import { open, readdir, rename, rm } from 'node:fs/promises' +import { open, readdir, rename, rm, stat } from 'node:fs/promises' import { basename, dirname, join } from 'node:path' /** @@ -103,11 +103,13 @@ export function durableWriteTempPath(finalPath: string): string { /** * Sweep temp files orphaned by a death between write and rename — for multi-MB payloads they would - * otherwise accumulate forever. Racing another instance's in-flight save at worst loses that save, - * the trade already accepted for rename-based atomicity. This process's own temps are skipped: a - * `..*.tmp` seen during a sweep is a live write, and deleting it fails its rename. + * otherwise accumulate forever. Callers can require a minimum age to spare another live instance's + * write. This process's own temps are always skipped because deleting one would fail its rename. */ -export async function removeStaleDurableWriteTempFiles(finalPath: string): Promise { +export async function removeStaleDurableWriteTempFiles( + finalPath: string, + options: { minimumAgeMs?: number } = {} +): Promise { const directory = dirname(finalPath) const prefix = `${basename(finalPath)}.` const ownPrefix = `${prefix}${process.pid}.` @@ -118,7 +120,16 @@ export async function removeStaleDurableWriteTempFiles(finalPath: string): Promi .filter( (name) => name.startsWith(prefix) && name.endsWith('.tmp') && !name.startsWith(ownPrefix) ) - .map((name) => rm(join(directory, name), { force: true }).catch(() => {})) + .map(async (name) => { + const path = join(directory, name) + if (options.minimumAgeMs) { + const info = await stat(path).catch(() => null) + if (!info || Date.now() - info.mtimeMs < options.minimumAgeMs) { + return + } + } + await rm(path, { force: true }).catch(() => {}) + }) ) } catch { // Directory missing or unreadable — nothing to sweep. diff --git a/src/main/index.ts b/src/main/index.ts index 7aceeb942b2..2c94c61488c 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -250,6 +250,7 @@ import { buildHeadlessAutomationWorktreeCreateArgs } from './automations/headles import { AgentAwakeService } from './agent-awake-service' import { registerSystemResumeBroadcast } from './system-resume-broadcast' import { settleTeardownWithinDeadline } from './quit-teardown-deadline' +import { QuitTeardownStartGate } from './quit-teardown-start-gate' import { PluginService } from './plugins/plugin-service' import { PluginKillListService } from './plugins/plugin-kill-list-service' import { getPluginsDataDir } from './plugins/plugin-discovery' @@ -2936,6 +2937,7 @@ app.on('before-quit', () => { // Why: will-quit fires twice — first pass preventDefaults and runs teardown; second pass exits. let daemonDisconnectDone = false +const quitTeardownStartGate = new QuitTeardownStartGate() app.on('will-quit', (e) => { // Why return instead of re-running teardown: the second pass is Electron re-firing after // our own app.quit(), so every step below already ran and every durable write already @@ -2948,7 +2950,9 @@ app.on('will-quit', (e) => { // network profile mount. The teardown deadline cannot rescue that, because its timer // lives on the same thread it would need to bound (#9447 covers the wedged-transport // half; this covers the blocked-syscall half). - e.preventDefault() + if (!quitTeardownStartGate.tryStart(e)) { + return + } // Why: renderer guards can still cancel before this committed phase; `log stream` must survive those vetoes. stopTccPromptNotice() const updateQuitInProgress = isQuittingForUpdate() diff --git a/src/main/ipc/orca-profiles.test.ts b/src/main/ipc/orca-profiles.test.ts index 3cad49f00ff..20aef7e0582 100644 --- a/src/main/ipc/orca-profiles.test.ts +++ b/src/main/ipc/orca-profiles.test.ts @@ -55,12 +55,12 @@ vi.mock('../orca-profiles/profile-index-store', () => ({ setActiveOrcaProfile: setActiveOrcaProfileMock })) -function makeStoreMock(flush = vi.fn()): { - flush: typeof flush +function makeStoreMock(flushPendingOrThrowAsync = vi.fn()): { + flushPendingOrThrowAsync: typeof flushPendingOrThrowAsync freezeWrites: ReturnType getSettings: () => Record } { - return { flush, freezeWrites: vi.fn(), getSettings: () => ({}) } + return { flushPendingOrThrowAsync, freezeWrites: vi.fn(), getSettings: () => ({}) } } vi.mock('../orca-profiles/profile-project-transfer', () => ({ @@ -162,6 +162,9 @@ describe('registerOrcaProfileHandlers', () => { expect(flush.mock.invocationCallOrder[0]).toBeLessThan( setActiveOrcaProfileMock.mock.invocationCallOrder[0] ?? Number.POSITIVE_INFINITY ) + expect(flush.mock.invocationCallOrder[0]).toBeLessThan( + onBeforeRelaunch.mock.invocationCallOrder[0] ?? Number.POSITIVE_INFINITY + ) expect(appRelaunchMock).not.toHaveBeenCalled() await vi.advanceTimersByTimeAsync(150) @@ -192,6 +195,27 @@ describe('registerOrcaProfileHandlers', () => { expect(appRelaunchMock).not.toHaveBeenCalled() }) + it('does not switch profiles when persistence cannot reach quiescence', async () => { + const flush = vi.fn(() => new Promise(() => {})) + const onBeforeRelaunch = vi.fn() + getOrcaProfileListStateMock.mockReturnValue({ + activeProfileId: 'local-default', + profiles: [] + }) + registerOrcaProfileHandlers(makeStoreMock(flush) as never, { onBeforeRelaunch }) + + const switchProfile = Promise.resolve( + handlers.get('orcaProfiles:switch')?.(null, { profileId: 'local-work' }) + ) + const rejection = expect(switchProfile).rejects.toThrow('orca_profile_persistence_timeout') + await vi.advanceTimersByTimeAsync(20_000) + await rejection + + expect(setActiveOrcaProfileMock).not.toHaveBeenCalled() + expect(appRelaunchMock).not.toHaveBeenCalled() + expect(onBeforeRelaunch).not.toHaveBeenCalled() + }) + it('does not relaunch when switching to the active profile', async () => { getOrcaProfileListStateMock.mockReturnValue({ activeProfileId: 'local-default', diff --git a/src/main/ipc/orca-profiles.ts b/src/main/ipc/orca-profiles.ts index b11478bdf89..497da481ae0 100644 --- a/src/main/ipc/orca-profiles.ts +++ b/src/main/ipc/orca-profiles.ts @@ -34,6 +34,7 @@ import { getProfileUserDataPath } from '../orca-profiles/profile-storage-paths' import { isMultiProfileUiEnabled } from '../orca-profiles/profile-ui-scope' import { transferOrcaProfileProject } from '../orca-profiles/profile-project-transfer' import { findOrcaProfileProjectsByPath } from '../orca-profiles/profile-project-presence' +import { flushActiveProfileBeforeFileMutation } from '../orca-profiles/profile-persistence-deadline' import { normalizeExecutionHostId } from '../../shared/execution-host' import { createCloudLinkedOrcaProfile, @@ -212,8 +213,8 @@ export function registerOrcaProfileHandlers( } // Why: the current profile must be persisted before the global index // points startup at the target profile. + await flushActiveProfileBeforeFileMutation(store) await runBeforeProfileRelaunch(options.onBeforeRelaunch) - store.flush() setActiveOrcaProfile(profileId) scheduleProfileRelaunch('profile-switch') @@ -236,10 +237,7 @@ export function registerOrcaProfileHandlers( if (args.mode === 'move' && args.sourceProfileId === current.activeProfileId) { // Why: transfer before any relaunch side effect so a duplicate-target // or validation failure cannot strand the app in a quitting state. - // flush→transfer→freeze runs synchronously with no interleaving, and - // the freeze keeps late sync saves from resurrecting the moved - // project from stale memory before the relaunch. - store.flush() + await flushActiveProfileBeforeFileMutation(store) const result = transferOrcaProfileProject(args, getProfileUserDataPath()) if (result.status === 'transferred') { store.freezeWrites() @@ -250,7 +248,7 @@ export function registerOrcaProfileHandlers( } return result } - store.flush() + await flushActiveProfileBeforeFileMutation(store) return transferOrcaProfileProject(args, getProfileUserDataPath()) } ) diff --git a/src/main/ipc/renderer-shutdown-checkpoint.test.ts b/src/main/ipc/renderer-shutdown-checkpoint.test.ts index ca6e335f055..78accf8abe1 100644 --- a/src/main/ipc/renderer-shutdown-checkpoint.test.ts +++ b/src/main/ipc/renderer-shutdown-checkpoint.test.ts @@ -27,19 +27,21 @@ describe('registerRendererShutdownCheckpointHandler', () => { syncHandlers.clear() }) - it('commits every shutdown state mutation before flushing both stores', () => { + it('stages every shutdown mutation before queueing persistence', () => { const callOrder: string[] = [] const store = { - setWorkspaceSession: vi.fn((_state, hostId?: string) => { + stageWorkspaceSessionBeforeUnload: vi.fn((_state, hostId?: string) => { callOrder.push(`session:${hostId ?? 'local'}`) }), updateUI: vi.fn(() => callOrder.push('ui')), - flushOrThrow: vi.fn(() => callOrder.push('flush')), - flushActiveViewPreferenceOrThrow: vi.fn(() => callOrder.push('active-view')) + flushPendingAsync: vi.fn(() => { + callOrder.push('persist') + return Promise.resolve() + }) } registerRendererShutdownCheckpointHandler(store as never) - const handler = syncHandlers.get('app:persist-before-unload-sync') + const handler = syncHandlers.get('app:stage-before-unload-sync') expect(handler).toBeDefined() const event: { returnValue?: unknown } = {} const localSession = { activeWorktreeId: 'local-worktree' } @@ -49,76 +51,75 @@ describe('registerRendererShutdownCheckpointHandler', () => { ui: { activeView: 'settings' } }) - expect(store.setWorkspaceSession).toHaveBeenNthCalledWith(1, localSession, undefined) - expect(store.setWorkspaceSession).toHaveBeenNthCalledWith(2, remoteSession, 'runtime:host-1') + expect(store.stageWorkspaceSessionBeforeUnload).toHaveBeenNthCalledWith( + 1, + localSession, + undefined + ) + expect(store.stageWorkspaceSessionBeforeUnload).toHaveBeenNthCalledWith( + 2, + remoteSession, + 'runtime:host-1' + ) expect(store.updateUI).toHaveBeenCalledWith({ activeView: 'settings' }) - expect(store.flushOrThrow).toHaveBeenCalledTimes(1) - expect(store.flushActiveViewPreferenceOrThrow).toHaveBeenCalledTimes(1) - expect(callOrder).toEqual([ - 'session:local', - 'session:runtime:host-1', - 'ui', - 'flush', - 'active-view' - ]) + expect(store.flushPendingAsync).toHaveBeenCalledTimes(1) + expect(callOrder).toEqual(['session:local', 'session:runtime:host-1', 'ui', 'persist']) expect(event.returnValue).toEqual({ ok: true }) }) - it('reports a failed durable checkpoint so the renderer can retry', () => { + it('reports a staging failure so the renderer can retry', () => { const store = { - setWorkspaceSession: vi.fn(), - updateUI: vi.fn(), - flushOrThrow: vi.fn(() => { + stageWorkspaceSessionBeforeUnload: vi.fn(), + updateUI: vi.fn(() => { throw new Error('disk full') }), - flushActiveViewPreferenceOrThrow: vi.fn() + flushPendingAsync: vi.fn(() => Promise.resolve()) } registerRendererShutdownCheckpointHandler(store as never) - const handler = syncHandlers.get('app:persist-before-unload-sync') + const handler = syncHandlers.get('app:stage-before-unload-sync') const event: { returnValue?: unknown } = {} handler?.(event, { sessions: [], ui: { activeView: 'settings' } }) expect(event.returnValue).toEqual({ ok: false }) }) - it('still flushes the active-view sidecar when the durable flush throws', () => { - // Why: the two stores are independent; a durable-state failure must not drop - // the tiny active-view checkpoint (and vice versa). + it('does not queue persistence when staging is incomplete', () => { const store = { - setWorkspaceSession: vi.fn(), + stageWorkspaceSessionBeforeUnload: vi.fn(), updateUI: vi.fn(), - flushOrThrow: vi.fn(() => { - throw new Error('disk full') - }), - flushActiveViewPreferenceOrThrow: vi.fn() + flushPendingAsync: vi.fn(() => Promise.resolve()) } registerRendererShutdownCheckpointHandler(store as never) - const handler = syncHandlers.get('app:persist-before-unload-sync') + store.updateUI.mockImplementation(() => { + throw new Error('invalid state') + }) + const handler = syncHandlers.get('app:stage-before-unload-sync') const event: { returnValue?: unknown } = {} handler?.(event, { sessions: [], ui: { activeView: 'settings' } }) - expect(store.flushActiveViewPreferenceOrThrow).toHaveBeenCalledTimes(1) + expect(store.flushPendingAsync).not.toHaveBeenCalled() expect(event.returnValue).toEqual({ ok: false }) }) - it('flushes the durable store even when the active-view flush throws', () => { + it('returns before the asynchronous persistence settles', () => { + let resolve!: () => void + const pending = new Promise((next) => { + resolve = next + }) const store = { - setWorkspaceSession: vi.fn(), + stageWorkspaceSessionBeforeUnload: vi.fn(), updateUI: vi.fn(), - flushOrThrow: vi.fn(), - flushActiveViewPreferenceOrThrow: vi.fn(() => { - throw new Error('disk full') - }) + flushPendingAsync: vi.fn(() => pending) } registerRendererShutdownCheckpointHandler(store as never) - const handler = syncHandlers.get('app:persist-before-unload-sync') + const handler = syncHandlers.get('app:stage-before-unload-sync') const event: { returnValue?: unknown } = {} handler?.(event, { sessions: [], ui: { activeView: 'settings' } }) - expect(store.flushOrThrow).toHaveBeenCalledTimes(1) - expect(event.returnValue).toEqual({ ok: false }) + expect(event.returnValue).toEqual({ ok: true }) + resolve() }) }) diff --git a/src/main/ipc/renderer-shutdown-checkpoint.ts b/src/main/ipc/renderer-shutdown-checkpoint.ts index d268d388dd0..a1fafa16634 100644 --- a/src/main/ipc/renderer-shutdown-checkpoint.ts +++ b/src/main/ipc/renderer-shutdown-checkpoint.ts @@ -3,38 +3,27 @@ import type { ExecutionHostId } from '../../shared/execution-host' import type { PersistedUIState, WorkspaceSessionState } from '../../shared/types' import type { Store } from '../persistence' -type PersistBeforeUnloadSyncArgs = { +type StageBeforeUnloadSyncArgs = { sessions: { state: WorkspaceSessionState; hostId?: ExecutionHostId }[] ui: Partial } export function registerRendererShutdownCheckpointHandler(store: Store): void { - ipcMain.on('app:persist-before-unload-sync', (event, args: PersistBeforeUnloadSyncArgs) => { + ipcMain.on('app:stage-before-unload-sync', (event, args: StageBeforeUnloadSyncArgs) => { let ok = true - // Why: apply both renderer-owned snapshots before synchronously flushing - // each owning store, so an immediate exit cannot outrun either update. try { for (const { state, hostId } of args.sessions) { - store.setWorkspaceSession(state, hostId) + store.stageWorkspaceSessionBeforeUnload(state, hostId) } store.updateUI(args.ui) } catch (error) { console.error('[app] Failed to stage renderer state before unload:', error) ok = false } - // Why: the durable snapshot and the active-view sidecar are independent stores; - // flush each on its own so one store's failure can't skip the other's checkpoint. - try { - store.flushOrThrow() - } catch (error) { - console.error('[app] Failed to flush durable state before unload:', error) - ok = false - } - try { - store.flushActiveViewPreferenceOrThrow() - } catch (error) { - console.error('[app] Failed to flush active-view preference before unload:', error) - ok = false + if (ok) { + void store.flushPendingAsync().catch((error) => { + console.error('[app] Failed to persist staged renderer state:', error) + }) } event.returnValue = { ok } }) diff --git a/src/main/orca-profiles/profile-persistence-deadline.ts b/src/main/orca-profiles/profile-persistence-deadline.ts new file mode 100644 index 00000000000..084a53c40fe --- /dev/null +++ b/src/main/orca-profiles/profile-persistence-deadline.ts @@ -0,0 +1,21 @@ +import type { Store } from '../persistence' + +const PROFILE_PERSISTENCE_TIMEOUT_MS = 20_000 + +export async function flushActiveProfileBeforeFileMutation(store: Store): Promise { + const controller = new AbortController() + let timeout: ReturnType | null = null + const deadline = new Promise((_resolve, reject) => { + timeout = setTimeout(() => { + controller.abort() + reject(new Error('orca_profile_persistence_timeout')) + }, PROFILE_PERSISTENCE_TIMEOUT_MS) + }) + try { + await Promise.race([store.flushPendingOrThrowAsync({ signal: controller.signal }), deadline]) + } finally { + if (timeout) { + clearTimeout(timeout) + } + } +} diff --git a/src/main/persistence-async-write-syscalls.test.ts b/src/main/persistence-async-write-syscalls.test.ts index 6a4491e8b38..9299d1a1149 100644 --- a/src/main/persistence-async-write-syscalls.test.ts +++ b/src/main/persistence-async-write-syscalls.test.ts @@ -1,5 +1,13 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' -import { mkdtempSync, readFileSync, readdirSync, rmSync, utimesSync, writeFileSync } from 'node:fs' +import { + existsSync, + mkdtempSync, + readFileSync, + readdirSync, + rmSync, + utimesSync, + writeFileSync +} from 'node:fs' import type * as NodeFs from 'node:fs' import type * as NodeFsPromises from 'node:fs/promises' import { join } from 'node:path' @@ -130,8 +138,11 @@ const ROTATION_INTERLEAVE_CASES = [ type TestStore = { updateUI(updates: { sidebarWidth: number }): void + setGitHubCache(cache: { pr: Record; issue: Record }): void waitForPendingWrite(): Promise flushOrThrow(): void + flushPendingAsync(): Promise + flushPendingOrThrowAsync(): Promise } async function createStore(dir: string): Promise { @@ -309,7 +320,136 @@ describe('async persistence write path avoids synchronous fs syscalls', () => { expect(ring['orca-data.json.bak.2']).toBeUndefined() }) - it('keeps the due check inside ownership when a second writer starts', async () => { + it('a sync checkpoint vetoes an async write already parked on rename', async () => { + const dir = makeDir() + const store = await createStore(dir) + let releaseRename!: () => void + const renameRelease = new Promise((resolve) => { + releaseRename = resolve + }) + let signalRename!: () => void + const renameStarted = new Promise((resolve) => { + signalRename = resolve + }) + fsCalls.waitAsync = (fn, target) => { + if (fn !== 'rename' || target === dataFile(dir) || !target.startsWith(dataFile(dir))) { + return null + } + signalRename() + return renameRelease + } + + fsCalls.dirPrefix = dir + fsCalls.recording = true + store.updateUI({ sidebarWidth: 501 }) + vi.advanceTimersByTime(SAVE_DEBOUNCE_MS) + const pending = store.waitForPendingWrite() + await renameStarted + + store.updateUI({ sidebarWidth: 502 }) + store.flushOrThrow() + expect(JSON.parse(readFileSync(dataFile(dir), 'utf-8')).ui.sidebarWidth).toBe(502) + + releaseRename() + await pending + fsCalls.recording = false + fsCalls.waitAsync = null + + expect(JSON.parse(readFileSync(dataFile(dir), 'utf-8')).ui.sidebarWidth).toBe(502) + expect(readdirSync(dir).filter((name) => name.endsWith('.tmp'))).toHaveLength(0) + }) + + it('retries a genuine ENOENT instead of marking the state persisted', async () => { + const dir = makeDir() + const store = await createStore(dir) + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}) + fsCalls.dirPrefix = dir + fsCalls.recording = true + fsCalls.failAsync = (fn, target) => + fn === 'rename' && target.startsWith(dataFile(dir)) && !target.includes('.bak.') + ? Object.assign(new Error('mount disappeared'), { code: 'ENOENT' }) + : null + + store.updateUI({ sidebarWidth: 511 }) + await store.flushPendingAsync() + expect(existsSync(dataFile(dir))).toBe(false) + + fsCalls.failAsync = null + await store.flushPendingAsync() + fsCalls.recording = false + errors.mockRestore() + + expect(JSON.parse(readFileSync(dataFile(dir), 'utf-8')).ui.sidebarWidth).toBe(511) + }) + + it('the throwing async barrier drains mutations made during its write', async () => { + const dir = makeDir() + const store = await createStore(dir) + let releaseRename!: () => void + const renameRelease = new Promise((resolve) => { + releaseRename = resolve + }) + let signalRename!: () => void + const renameStarted = new Promise((resolve) => { + signalRename = resolve + }) + let held = false + fsCalls.waitAsync = (fn, target) => { + if (held || fn !== 'rename' || !target.startsWith(dataFile(dir))) { + return null + } + held = true + signalRename() + return renameRelease + } + fsCalls.dirPrefix = dir + fsCalls.recording = true + + store.updateUI({ sidebarWidth: 601 }) + const barrier = store.flushPendingOrThrowAsync() + await renameStarted + store.updateUI({ sidebarWidth: 602 }) + releaseRename() + await barrier + fsCalls.recording = false + + expect(JSON.parse(readFileSync(dataFile(dir), 'utf-8')).ui.sidebarWidth).toBe(602) + }) + + it('the throwing async barrier drains mutations made during sidecar I/O', async () => { + const dir = makeDir() + const store = await createStore(dir) + let releaseRename!: () => void + const renameRelease = new Promise((resolve) => { + releaseRename = resolve + }) + let signalRename!: () => void + const renameStarted = new Promise((resolve) => { + signalRename = resolve + }) + fsCalls.waitAsync = (fn, target) => { + if (fn !== 'rename' || !target.includes('orca-github-cache.json.')) { + return null + } + signalRename() + return renameRelease + } + fsCalls.dirPrefix = dir + fsCalls.recording = true + + store.updateUI({ sidebarWidth: 611 }) + store.setGitHubCache({ pr: {}, issue: {} }) + const barrier = store.flushPendingOrThrowAsync() + await renameStarted + store.updateUI({ sidebarWidth: 612 }) + releaseRename() + await barrier + fsCalls.recording = false + + expect(JSON.parse(readFileSync(dataFile(dir), 'utf-8')).ui.sidebarWidth).toBe(612) + }) + + it('serializes a second writer behind the owned rotation', async () => { const dir = makeDir() const store = await createStore(dir) seedStaleBackup(dir) @@ -340,22 +480,24 @@ describe('async persistence write path avoids synchronous fs syscalls', () => { store.updateUI({ sidebarWidth: 371 }) vi.advanceTimersByTime(SAVE_DEBOUNCE_MS) const firstWrite = store.waitForPendingWrite() + let allWrites = firstWrite try { await rotationStarted store.updateUI({ sidebarWidth: 372 }) store.flushOrThrow() store.updateUI({ sidebarWidth: 373 }) vi.advanceTimersByTime(SAVE_DEBOUNCE_MS) - await store.waitForPendingWrite() + allWrites = store.waitForPendingWrite() expect(fsCalls.asyncCalls.filter((call) => call === statCall)).toHaveLength(1) } finally { releaseRotation() - await firstWrite + await allWrites fsCalls.recording = false fsCalls.waitAsync = null } const ring = ringSnapshot(dir) - expect(ring['orca-data.json.bak.0']).toBe(ring['orca-data.json']) + expect(JSON.parse(ring['orca-data.json']).ui.sidebarWidth).toBe(373) + expect(JSON.parse(ring['orca-data.json.bak.0']).ui.sidebarWidth).toBe(372) expect(ring['orca-data.json.bak.1']).toBe(staleBackup) expect(ring['orca-data.json.bak.2']).toBeUndefined() }) diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 61d2ed2f40b..7e0ff22f18a 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -12,7 +12,12 @@ import { realpathSync } from 'node:fs' import { rename, mkdir, rm, copyFile, open, stat, access, writeFile } from 'node:fs/promises' -import { renameDurable, writeFileDurableSync } from './durable-file-write' +import { + durableWriteTempPath, + removeStaleDurableWriteTempFiles, + renameDurable, + writeFileDurableSync +} from './durable-file-write' import { join, dirname, isAbsolute, resolve, sep } from 'node:path' import { homedir } from 'node:os' import { createHash, randomUUID } from 'node:crypto' @@ -267,6 +272,10 @@ import { readTerminalScrollbackSnapshotSync, type TerminalScrollbackSnapshotStorage } from './terminal-scrollback-snapshots' +import { + deleteRemovedTerminalScrollbackSnapshotsAsync, + migrateWorkspaceSessionTerminalScrollbackSnapshotsAsync +} from './terminal-scrollback-snapshot-async-migration' import { track } from './telemetry/client' import { getCohortAtEmit } from './telemetry/cohort-classifier' import { isStartupDiagnosticsEnabled, logStartupDiagnostic } from './startup/startup-diagnostics' @@ -372,6 +381,7 @@ function getGithubCacheFile(dataFile = getDataFile()): string { // Why: worktrees deleted outside Orca orphan their worktreeMeta, so the map grew monotonically (63% dead on a heavy install). // GC stays narrow: local-host entries only (a local existsSync would falsely condemn SSH/WSL remote paths) and only after a 30-day idle grace. const WORKTREE_META_GC_GRACE_MS = 30 * 24 * 60 * 60 * 1000 +const STALE_DURABLE_WRITE_TEMP_AGE_MS = 24 * 60 * 60 * 1000 function gcStaleWorktreeMeta(state: PersistedState): number { // Why: a hand-corrupted "worktreeMeta": null overrides the defaults merge; normalize here instead of throwing. @@ -2749,17 +2759,24 @@ export class Store { private readonly terminalScrollbackSnapshotStorage: TerminalScrollbackSnapshotStorage private writeTimer: ReturnType | null = null private pendingWrite: Promise | null = null + private pendingSnapshotFileWork: Promise | null = null + private readonly staleTempCleanup: Promise private writeGeneration = 0 + private inFlightAsyncTmpFile: string | null = null // Prevent a sync flush from interleaving a second rotation with awaited ring mutations. private backupRotationInFlight = false // Why: after a profile transfer rewrites this file on disk, a late flush of stale in-memory state would resurrect the moved project. private writesFrozen = false /** Set by flushAsync so the quit flush is the final write; see scheduleSave. */ private quitFlushStarted = false + private quitFlushPromise: Promise | null = null // Content hash at last write, to skip no-op writes; derived from the payload with encrypted blobs normalized back to plaintext (see buildStateToSave), since encrypt() uses a random IV per call. private lastWrittenStateHash: string | null = null private firstPendingSaveAt: number | null = null private githubCacheDirty = false + private githubCacheGeneration = 0 + private pendingGithubCacheWrite: Promise | null = null + private readonly staleGithubCacheTempCleanup: Promise private gitUsernameCache = new Map() private loadNeedsSave = false private settingsChangeListeners = new Set< @@ -2774,6 +2791,13 @@ export class Store { constructor(options: StoreOptions = {}) { // Why: profile switching yields multiple state paths; capture per Store so late async writes can't follow a global path. this.dataFile = options.dataFile ?? getDataFile() + this.staleTempCleanup = removeStaleDurableWriteTempFiles(this.dataFile, { + minimumAgeMs: STALE_DURABLE_WRITE_TEMP_AGE_MS + }) + this.staleGithubCacheTempCleanup = removeStaleDurableWriteTempFiles( + getGithubCacheFile(this.dataFile), + { minimumAgeMs: STALE_DURABLE_WRITE_TEMP_AGE_MS } + ) const profileSnapshotRoot = getProfileTerminalScrollbackSnapshotRoot(this.dataFile) const legacySnapshotRoot = getProfileTerminalScrollbackSnapshotRoot(getDataFile()) this.terminalScrollbackSnapshotStorage = { @@ -3788,6 +3812,7 @@ export class Store { if (this.quitFlushStarted) { return } + this.writeGeneration += 1 const now = Date.now() this.firstPendingSaveAt ??= now if (this.writeTimer) { @@ -3798,22 +3823,29 @@ export class Store { this.writeTimer = setTimeout(() => { this.writeTimer = null this.firstPendingSaveAt = null - // Why (issue #1158): serialize async writes so backup rotation can't race two callers over the same paths. - const prev = this.pendingWrite ?? Promise.resolve() - const next = prev - .then(() => this.writeToDiskAsync()) - .catch((err) => { - console.error('[persistence] Failed to write state:', err) - }) - .finally(() => { - if (this.pendingWrite === next) { - this.pendingWrite = null - } - }) - this.pendingWrite = next + void this.enqueueWrite() }, delay) } + private enqueueWrite(): Promise { + const previousWrite = Promise.all([ + this.pendingWrite ?? this.staleTempCleanup, + this.pendingSnapshotFileWork ?? Promise.resolve() + ]).then(() => {}) + const write = previousWrite.then(() => this.writeToDiskAsync()) + const trackedWrite = write + .catch((err) => { + console.error('[persistence] Failed to write state:', err) + }) + .finally(() => { + if (this.pendingWrite === trackedWrite) { + this.pendingWrite = null + } + }) + this.pendingWrite = trackedWrite + return write + } + /** Wait for any in-flight async disk write to complete. Used in tests. */ async waitForPendingWrite(): Promise { await Promise.all([this.pendingWrite, this.activeViewPreference.waitForPendingWrite()]) @@ -3900,7 +3932,7 @@ export class Store { const dataFile = this.dataFile const dir = dirname(dataFile) await mkdir(dir, { recursive: true }).catch(() => {}) - const tmpFile = `${dataFile}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp` + const tmpFile = durableWriteTempPath(dataFile) // Why: on any write/rename failure, remove the tmp file so it doesn't leave a multi-MB orphan. let renamed = false @@ -3917,10 +3949,21 @@ export class Store { if (this.writeGeneration !== gen) { return } - await renameDurable(tmpFile, dataFile) - renamed = true + this.inFlightAsyncTmpFile = tmpFile + try { + await renameDurable(tmpFile, dataFile) + renamed = true + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT' || this.writeGeneration === gen) { + throw error + } + } finally { + if (this.inFlightAsyncTmpFile === tmpFile) { + this.inFlightAsyncTmpFile = null + } + } // Why re-check gen: a sync flush during the rename await may have written fresher state; don't record a stale hash over it. - if (this.writeGeneration === gen) { + if (renamed && this.writeGeneration === gen) { this.lastWrittenStateHash = stateHash } } finally { @@ -3928,6 +3971,9 @@ export class Store { await rm(tmpFile).catch(() => {}) } } + if (!renamed) { + return + } // Why (#1158): rotate only after the primary rename while this write still owns its generation. if (this.writeGeneration !== gen) { return @@ -3976,6 +4022,9 @@ export class Store { } flushOrThrow(): void { + if (this.quitFlushStarted) { + throw new Error('Cannot synchronously flush after final persistence has started') + } if (this.writeTimer) { clearTimeout(this.writeTimer) this.writeTimer = null @@ -3984,7 +4033,17 @@ export class Store { const asyncWriteWasInFlight = this.pendingWrite !== null // Why: bump writeGeneration so an in-flight async write skips its rename and can't overwrite this sync write. this.writeGeneration++ - this.pendingWrite = null + if (this.inFlightAsyncTmpFile) { + try { + unlinkSync(this.inFlightAsyncTmpFile) + this.inFlightAsyncTmpFile = null + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + void this.enqueueWrite().catch(() => {}) + throw error + } + } + } this.writeToDiskSync({ force: asyncWriteWasInFlight, skipBackupRotation: this.backupRotationInFlight @@ -6029,6 +6088,7 @@ export class Store { // Why no scheduleSave: cache is memory-only and snapshotted to a sidecar at flush; persisting here rewrote the whole state file every poll cycle. this.state.githubCache = cache this.githubCacheDirty = true + this.githubCacheGeneration += 1 } // ── Workspace Session ───────────────────────────────────────────── @@ -6075,6 +6135,18 @@ export class Store { this.setHostWorkspaceSession(resolved, session) } + stageWorkspaceSessionBeforeUnload( + session: PersistedState['workspaceSession'], + hostId?: string | null + ): void { + const resolved = this.resolveHostId(hostId) + if (resolved === LOCAL_EXECUTION_HOST_ID) { + this.setLocalWorkspaceSession(session, true) + return + } + this.setHostWorkspaceSession(resolved, session) + } + /** Persist a non-'local' host partition; remote hosts skip setLocalWorkspaceSession's local-daemon PTY-binding race guards. */ private setHostWorkspaceSession(hostId: ExecutionHostId, session: WorkspaceSessionState): void { // Why: each partition owns its topology fence; renderer writes omit it and must rebase locally. @@ -6092,7 +6164,10 @@ export class Store { this.scheduleSave() } - private setLocalWorkspaceSession(session: PersistedState['workspaceSession']): void { + private setLocalWorkspaceSession( + session: PersistedState['workspaceSession'], + deferSnapshotFiles = false + ): void { const prior = this.state.workspaceSession session = sanitizeWorkspaceSessionTerminalRetirements(session, prior) session = pruneWorkspaceSessionBrowserHistory( @@ -6221,16 +6296,76 @@ export class Store { } } session = pruneLocalTerminalScrollbackBuffers(session, this.state.repos) - const migratedScrollback = migrateWorkspaceSessionTerminalScrollbackSnapshots( - session, - this.terminalScrollbackSnapshotStorage - ) - session = migratedScrollback.session - deleteRemovedTerminalScrollbackSnapshots(prior, session, this.terminalScrollbackSnapshotStorage) + if (!deferSnapshotFiles) { + const migratedScrollback = migrateWorkspaceSessionTerminalScrollbackSnapshots( + session, + this.terminalScrollbackSnapshotStorage + ) + session = migratedScrollback.session + deleteRemovedTerminalScrollbackSnapshots( + prior, + session, + this.terminalScrollbackSnapshotStorage + ) + } this.state.workspaceSession = session + if (deferSnapshotFiles) { + this.enqueueTerminalScrollbackSnapshotWork(prior, session) + } this.scheduleSave() } + private enqueueTerminalScrollbackSnapshotWork( + prior: WorkspaceSessionState | undefined, + staged: WorkspaceSessionState + ): void { + const previous = this.pendingSnapshotFileWork ?? Promise.resolve() + const work = previous + .then(async () => { + if (this.state.workspaceSession !== staged) { + if (this.state.workspaceSession) { + await deleteRemovedTerminalScrollbackSnapshotsAsync( + prior, + this.state.workspaceSession, + this.terminalScrollbackSnapshotStorage + ) + } + return + } + const migrated = await migrateWorkspaceSessionTerminalScrollbackSnapshotsAsync( + staged, + this.terminalScrollbackSnapshotStorage + ) + const current = + this.state.workspaceSession === staged ? migrated : this.state.workspaceSession + if (this.state.workspaceSession === staged) { + this.state.workspaceSession = migrated + } else if (current) { + await deleteRemovedTerminalScrollbackSnapshotsAsync( + migrated, + current, + this.terminalScrollbackSnapshotStorage + ) + } + if (current) { + await deleteRemovedTerminalScrollbackSnapshotsAsync( + prior, + current, + this.terminalScrollbackSnapshotStorage + ) + } + }) + .catch((error) => { + console.error('[terminal-scrollback] Failed to prepare unload snapshots:', error) + }) + .finally(() => { + if (this.pendingSnapshotFileWork === work) { + this.pendingSnapshotFileWork = null + } + }) + this.pendingSnapshotFileWork = work + } + patchWorkspaceSession(patch: WorkspaceSessionPatch, hostId?: string | null): void { const resolved = this.resolveHostId(hostId) // Why: the debounced hot path sends only changed slices; scalar/UI patches skip terminal normalization, topology patches keep stale-PTY protections. @@ -6992,6 +7127,9 @@ export class Store { // ── Flush (for shutdown) ─────────────────────────────────────────── flush(): void { + if (this.quitFlushStarted) { + return + } try { this.flushOrThrow() } catch (err) { @@ -7015,44 +7153,110 @@ export class Store { * * Never throws — it joins the quit teardown barrier, where a rejection is noise. */ - async flushAsync(): Promise { + flushAsync(): Promise { + if (this.quitFlushPromise) { + return this.quitFlushPromise + } this.quitFlushStarted = true - if (this.writeTimer) { - clearTimeout(this.writeTimer) - this.writeTimer = null + this.quitFlushPromise = this.flushCurrentStateAsync(true).catch(() => {}) + return this.quitFlushPromise + } + + flushPendingAsync(): Promise { + return this.flushCurrentStateAsync(false).catch(() => {}) + } + + flushPendingOrThrowAsync(options: { signal?: AbortSignal } = {}): Promise { + if (this.writesFrozen || this.quitFlushStarted) { + return Promise.reject(new Error('Cannot flush while persistence is finalized')) + } + return this.flushCurrentStateAsync(false, options.signal) + } + + private async flushCurrentStateAsync(final: boolean, signal?: AbortSignal): Promise { + for (;;) { + if (signal?.aborted) { + throw new Error('Persistence flush aborted') + } + if (this.writeTimer) { + clearTimeout(this.writeTimer) + this.writeTimer = null + } + this.firstPendingSaveAt = null + const generation = this.writeGeneration + try { + await this.enqueueWrite() + } catch (error) { + await (final + ? this.activeViewPreference.flushAsync() + : this.activeViewPreference.flushPendingAsync(signal)) + await this.writeGithubCacheSnapshotAsync(final, signal) + throw error + } + await (final + ? this.activeViewPreference.flushAsync() + : this.activeViewPreference.flushPendingAsync(signal)) + await this.writeGithubCacheSnapshotAsync(final, signal) + if (signal?.aborted) { + throw new Error('Persistence flush aborted') + } + if (generation === this.writeGeneration) { + break + } } - this.firstPendingSaveAt = null - // Why drain instead of vetoing by generation like flushOrThrow: an in-flight write - // owns the tmp path and the backup ring, and letting it finish is what makes the - // follow-up write's dirty check meaningful rather than a forced duplicate write. - await (this.pendingWrite ?? Promise.resolve()).catch(() => {}) - await this.writeToDiskAsync().catch((err) => { - console.error('[persistence] Failed to flush state:', err) - }) - await this.activeViewPreference.flushAsync() - await this.writeGithubCacheSnapshotAsync() } // Why best-effort: the sidecar is a refetchable cache; a failed write only costs a cold badge paint next launch, never data. - private async writeGithubCacheSnapshotAsync(): Promise { + private async writeGithubCacheSnapshotAsync( + drainToStableGeneration = true, + signal?: AbortSignal + ): Promise { if (!this.githubCacheDirty) { return } - const cacheFile = getGithubCacheFile(this.dataFile) - const tmpFile = `${cacheFile}.${process.pid}.tmp` - let renamed = false - try { - await writeFile(tmpFile, JSON.stringify(this.state.githubCache), 'utf-8') - await rename(tmpFile, cacheFile) - renamed = true - this.githubCacheDirty = false - } catch (err) { - console.warn('[persistence] Failed to write github cache snapshot:', err) - } finally { - if (!renamed) { - await rm(tmpFile).catch(() => {}) - } - } + const previousWrite = this.pendingGithubCacheWrite ?? this.staleGithubCacheTempCleanup + const nextWrite = previousWrite + .then(async () => { + while (this.githubCacheDirty) { + if (signal?.aborted) { + throw new Error('GitHub cache flush aborted') + } + const generation = this.githubCacheGeneration + const cacheFile = getGithubCacheFile(this.dataFile) + const tmpFile = durableWriteTempPath(cacheFile) + let renamed = false + try { + await writeFile(tmpFile, JSON.stringify(this.state.githubCache), 'utf-8') + if (generation === this.githubCacheGeneration) { + await rename(tmpFile, cacheFile) + renamed = true + if (generation === this.githubCacheGeneration) { + this.githubCacheDirty = false + } + } + } finally { + if (!renamed) { + await rm(tmpFile).catch(() => {}) + } + } + if (signal?.aborted) { + throw new Error('GitHub cache flush aborted') + } + if (!drainToStableGeneration) { + break + } + } + }) + .catch((err) => { + console.warn('[persistence] Failed to write github cache snapshot:', err) + }) + .finally(() => { + if (this.pendingGithubCacheWrite === nextWrite) { + this.pendingGithubCacheWrite = null + } + }) + this.pendingGithubCacheWrite = nextWrite + await nextWrite } // Why: a project move rewrote the data file directly; in-memory state is now stale and any write would undo the transfer. @@ -7069,12 +7273,19 @@ export class Store { if (!this.githubCacheDirty) { return } + if (this.pendingGithubCacheWrite) { + void this.writeGithubCacheSnapshotAsync() + return + } const cacheFile = getGithubCacheFile(this.dataFile) - const tmpFile = `${cacheFile}.${process.pid}.tmp` + const generation = this.githubCacheGeneration + const tmpFile = durableWriteTempPath(cacheFile) try { writeFileSync(tmpFile, JSON.stringify(this.state.githubCache), 'utf-8') renameSync(tmpFile, cacheFile) - this.githubCacheDirty = false + if (generation === this.githubCacheGeneration) { + this.githubCacheDirty = false + } } catch (err) { try { unlinkSync(tmpFile) diff --git a/src/main/quit-path-durable-write-blocking.test.ts b/src/main/quit-path-durable-write-blocking.test.ts index 7bd98aadd4b..35ebe7460c3 100644 --- a/src/main/quit-path-durable-write-blocking.test.ts +++ b/src/main/quit-path-durable-write-blocking.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' -import { mkdtempSync, readFileSync, existsSync, rmSync } from 'node:fs' +import { mkdtempSync, readFileSync, existsSync, rmSync, utimesSync, writeFileSync } from 'node:fs' import type * as NodeFs from 'node:fs' import type * as NodeFsPromises from 'node:fs/promises' import { join } from 'node:path' @@ -20,9 +20,11 @@ const fsCalls = vi.hoisted(() => { syncCalls: [] as string[], /** Set to park every in-scope async fs call — a stalled mount, without stalling the thread. */ holdAsync: null as Promise | null, + waitAsync: null as ((fn: string, target: string) => Promise | null) | null, reset(): void { calls.syncCalls.length = 0 calls.holdAsync = null + calls.waitAsync = null }, inScope(target: unknown): target is string { return ( @@ -65,6 +67,9 @@ vi.mock('node:fs/promises', async (importOriginal) => { if (fsCalls.inScope(args[0]) && fsCalls.holdAsync) { await fsCalls.holdAsync } + if (fsCalls.inScope(args[0]) && typeof args[0] === 'string') { + await fsCalls.waitAsync?.(name, args[0]) + } return fn(...args) } } @@ -96,6 +101,8 @@ type TestStore = { setGitHubCache(cache: unknown): void waitForPendingWrite(): Promise flushAsync(): Promise + flushOrThrow(): void + stageWorkspaceSessionBeforeUnload(session: Record): void } type TestStatsCollector = { @@ -165,8 +172,57 @@ describe('quit-path durable writes never park the main thread', () => { expect(JSON.parse(readFileSync(dataFile(dir), 'utf-8')).ui.sidebarWidth).toBe(321) }) + it('renderer unload staging issues no synchronous fs syscalls', async () => { + const dir = makeDir() + const store = await createStore(dir) + + fsCalls.dirPrefix = dir + fsCalls.recording = true + const leafId = '11111111-1111-4111-8111-111111111111' + store.stageWorkspaceSessionBeforeUnload({ + tabsByWorktree: { + 'remote-repo::/remote': [ + { + id: 'remote-tab', + ptyId: 'remote-pty', + worktreeId: 'remote-repo::/remote', + title: 'Remote', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + terminalLayoutsByTabId: { + 'remote-tab': { + root: { type: 'leaf', leafId }, + activeLeafId: leafId, + expandedLeafId: null, + buffersByLeafId: { [leafId]: 'remote-scrollback' } + } + }, + openFilesByWorktree: {} + }) + fsCalls.recording = false + + expect(fsCalls.syncCalls).toEqual([]) + await store.flushAsync() + const layout = JSON.parse(readFileSync(dataFile(dir), 'utf-8')).workspaceSession + .terminalLayoutsByTabId['remote-tab'] + const ref = layout.scrollbackRefsByLeafId[leafId] + expect(layout.buffersByLeafId).toBeUndefined() + expect(readFileSync(join(dir, 'terminal-scrollback', `${ref}.bin`), 'utf-8')).toBe( + 'remote-scrollback' + ) + }) + it('store.flushAsync() also moves the active-view and github-cache sidecars off the thread', async () => { const dir = makeDir() + const staleCacheTemp = join(dir, 'orca-github-cache.json.999999.1.orphan.tmp') + writeFileSync(staleCacheTemp, 'stale', 'utf-8') + const staleSeconds = (Date.now() - 25 * 60 * 60 * 1000) / 1000 + utimesSync(staleCacheTemp, staleSeconds, staleSeconds) const store = await createStore(dir) store.updateUI({ activeView: 'activity' }) store.setGitHubCache({ pullRequestsByWorktree: {} }) @@ -179,6 +235,7 @@ describe('quit-path durable writes never park the main thread', () => { expect(fsCalls.syncCalls).toEqual([]) expect(JSON.parse(readFileSync(activeViewFile(dir), 'utf-8')).activeView).toBe('activity') expect(existsSync(join(dir, 'orca-github-cache.json'))).toBe(true) + expect(existsSync(staleCacheTemp)).toBe(false) }) it('stats.flushAsync() issues no synchronous fs syscalls', async () => { @@ -262,6 +319,57 @@ describe('quit-path durable writes never park the main thread', () => { expect(JSON.parse(readFileSync(dataFile(dir), 'utf-8')).ui.sidebarWidth).toBe(200) }) + it('store.flushAsync() is one idempotent final barrier', async () => { + const dir = makeDir() + const store = await createStore(dir) + store.updateUI({ sidebarWidth: 777 }) + + const first = store.flushAsync() + const second = store.flushAsync() + + expect(second).toBe(first) + expect(() => store.flushOrThrow()).toThrow('final persistence') + await first + expect(JSON.parse(readFileSync(dataFile(dir), 'utf-8')).ui.sidebarWidth).toBe(777) + }) + + it('serializes github-cache snapshots and keeps the newest generation', async () => { + const dir = makeDir() + const store = await createStore(dir) + let releaseWrite!: () => void + const writeRelease = new Promise((resolve) => { + releaseWrite = resolve + }) + let signalWrite!: () => void + const writeStarted = new Promise((resolve) => { + signalWrite = resolve + }) + let held = false + fsCalls.dirPrefix = dir + fsCalls.recording = true + fsCalls.waitAsync = (fn, target) => { + if (held || fn !== 'writeFile' || !target.includes('orca-github-cache.json')) { + return null + } + held = true + signalWrite() + return writeRelease + } + + store.setGitHubCache({ version: 1 }) + const finalFlush = store.flushAsync() + await writeStarted + store.setGitHubCache({ version: 2 }) + + releaseWrite() + await finalFlush + fsCalls.recording = false + + expect(JSON.parse(readFileSync(join(dir, 'orca-github-cache.json'), 'utf-8'))).toEqual({ + version: 2 + }) + }) + it('stats.flushAsync() wins over a debounced write already in flight', async () => { const dir = makeDir() const stats = await createStatsCollector(dir) @@ -297,6 +405,30 @@ describe('quit-path durable writes never park the main thread', () => { expect(fsCalls.syncCalls).toEqual([]) expect(JSON.parse(readFileSync(dataFile(dir), 'utf-8')).ui.sidebarWidth).toBe(10) + expect(JSON.parse(readFileSync(activeViewFile(dir), 'utf-8')).activeView).toBe('terminal') + }) + + it('sweeps orphaned state and stats temp files before the final write', async () => { + const dir = makeDir() + const stateTemp = `${dataFile(dir)}.999999.1.orphan.tmp` + const statsTemp = `${statsFile(dir)}.999999.1.orphan.tmp` + const freshOtherProcessTemp = `${dataFile(dir)}.999998.1.live.tmp` + writeFileSync(stateTemp, 'stale', 'utf-8') + writeFileSync(statsTemp, 'stale', 'utf-8') + writeFileSync(freshOtherProcessTemp, 'in-flight', 'utf-8') + const staleSeconds = (Date.now() - 25 * 60 * 60 * 1000) / 1000 + utimesSync(stateTemp, staleSeconds, staleSeconds) + utimesSync(statsTemp, staleSeconds, staleSeconds) + + const store = await createStore(dir) + const stats = await createStatsCollector(dir) + store.updateUI({ sidebarWidth: 123 }) + stats.onAgentStart('pty-sweep', Date.now()) + await Promise.all([store.flushAsync(), stats.flushAsync()]) + + expect(existsSync(stateTemp)).toBe(false) + expect(existsSync(statsTemp)).toBe(false) + expect(existsSync(freshOtherProcessTemp)).toBe(true) }) it('store.flushAsync() resolves instead of throwing when the profile mount rejects writes', async () => { diff --git a/src/main/quit-teardown-start-gate.test.ts b/src/main/quit-teardown-start-gate.test.ts new file mode 100644 index 00000000000..7c42a3979f6 --- /dev/null +++ b/src/main/quit-teardown-start-gate.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it, vi } from 'vitest' +import { QuitTeardownStartGate } from './quit-teardown-start-gate' + +describe('QuitTeardownStartGate', () => { + it('starts teardown once while vetoing every overlapping quit', () => { + const gate = new QuitTeardownStartGate() + const first = { preventDefault: vi.fn() } + const second = { preventDefault: vi.fn() } + + expect(gate.tryStart(first)).toBe(true) + expect(gate.tryStart(second)).toBe(false) + expect(first.preventDefault).toHaveBeenCalledTimes(1) + expect(second.preventDefault).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/quit-teardown-start-gate.ts b/src/main/quit-teardown-start-gate.ts new file mode 100644 index 00000000000..be970495a4b --- /dev/null +++ b/src/main/quit-teardown-start-gate.ts @@ -0,0 +1,12 @@ +export class QuitTeardownStartGate { + private started = false + + tryStart(event: { preventDefault(): void }): boolean { + event.preventDefault() + if (this.started) { + return false + } + this.started = true + return true + } +} diff --git a/src/main/stats/collector-async-save.test.ts b/src/main/stats/collector-async-save.test.ts index e7976ffcb5e..80d6c8b5497 100644 --- a/src/main/stats/collector-async-save.test.ts +++ b/src/main/stats/collector-async-save.test.ts @@ -25,7 +25,9 @@ const gate = vi.hoisted(() => ({ waiters: [] as (() => void)[], renameWaiters: [] as (() => void)[], writeFileCalls: 0, - renameCalls: 0 + renameCalls: 0, + failNextWrite: false, + failNextRenameEnoent: false })) vi.mock('node:fs/promises', async () => { @@ -35,10 +37,18 @@ vi.mock('node:fs/promises', async () => { if (gate.blocked) { await new Promise((resolve) => gate.waiters.push(resolve)) } + if (gate.failNextWrite) { + gate.failNextWrite = false + throw new Error('transient write failure') + } return actual.writeFile(...args) }) as typeof actual.writeFile const rename = (async (...args: Parameters) => { gate.renameCalls += 1 + if (gate.failNextRenameEnoent) { + gate.failNextRenameEnoent = false + throw Object.assign(new Error('missing temp'), { code: 'ENOENT' }) + } if (gate.blockRename) { await new Promise((resolve) => gate.renameWaiters.push(resolve)) } @@ -60,6 +70,8 @@ describe('StatsCollector async debounced save', () => { gate.renameWaiters = [] gate.writeFileCalls = 0 gate.renameCalls = 0 + gate.failNextWrite = false + gate.failNextRenameEnoent = false vi.resetModules() }) @@ -92,6 +104,70 @@ describe('StatsCollector async debounced save', () => { expect(Array.isArray(parsed.events)).toBe(true) }) + it('coalesces timer saves while one write is stalled', async () => { + vi.useFakeTimers() + const { StatsCollector, initStatsPath } = await importCollector() + initStatsPath() + const collector = new StatsCollector() + + gate.blocked = true + collector.onAgentStart('pty-1', 1_000) + await vi.advanceTimersByTimeAsync(5_000) + await vi.waitFor(() => expect(gate.writeFileCalls).toBe(1)) + + for (let index = 2; index <= 5; index += 1) { + collector.onAgentStart(`pty-${index}`, index * 1_000) + await vi.advanceTimersByTimeAsync(5_000) + } + expect(gate.writeFileCalls).toBe(1) + + gate.blocked = false + gate.waiters.splice(0).forEach((resolve) => resolve()) + await vi.waitFor(() => expect(gate.writeFileCalls).toBe(2)) + await ( + collector as unknown as { snapshotWriter: { waitForPendingWrite(): Promise } } + ).snapshotWriter.waitForPendingWrite() + + expect(JSON.parse(readFileSync(statsPath(), 'utf-8')).aggregates.totalAgentsSpawned).toBe(5) + }) + + it('retries a queued final snapshot after the active write fails', async () => { + const { StatsCollector, initStatsPath } = await importCollector() + initStatsPath() + const collector = new StatsCollector() + const internals = collector as unknown as { enqueueWrite(): Promise } + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}) + + collector.onAgentStart('pty-final', 1_000) + gate.blocked = true + void internals.enqueueWrite() + await vi.waitFor(() => expect(gate.writeFileCalls).toBe(1)) + const finalWrite = collector.flushAsync() + + gate.failNextWrite = true + gate.blocked = false + gate.waiters.splice(0).forEach((resolve) => resolve()) + await finalWrite + errors.mockRestore() + + expect(gate.writeFileCalls).toBe(2) + expect(JSON.parse(readFileSync(statsPath(), 'utf-8')).aggregates.totalAgentsSpawned).toBe(1) + }) + + it('does not treat a genuine rename ENOENT as a committed snapshot', async () => { + const { StatsCollector, initStatsPath } = await importCollector() + initStatsPath() + const collector = new StatsCollector() + const internals = collector as unknown as { enqueueWrite(): Promise } + collector.onAgentStart('pty-enoent', 1_000) + + gate.failNextRenameEnoent = true + await expect(internals.enqueueWrite()).rejects.toMatchObject({ code: 'ENOENT' }) + await expect(internals.enqueueWrite()).resolves.toBeUndefined() + + expect(JSON.parse(readFileSync(statsPath(), 'utf-8')).aggregates.totalAgentsSpawned).toBe(1) + }) + it('flush() writes synchronously (no timer, immediately on disk)', async () => { const { StatsCollector, initStatsPath } = await importCollector() initStatsPath() @@ -110,8 +186,7 @@ describe('StatsCollector async debounced save', () => { initStatsPath() const collector = new StatsCollector() // White-box: drive the two writers directly so the race is deterministic - // (the debounce path calls writeToDiskAsync; flush() calls writeToDiskSync). - const internals = collector as unknown as { writeToDiskAsync: () => Promise } + const internals = collector as unknown as { enqueueWrite: () => Promise } collector.onAgentStart('pty-a', 1_000) collector.onAgentStart('pty-b', 1_000) @@ -120,7 +195,7 @@ describe('StatsCollector async debounced save', () => { // Why waitFor and not one tick: the writer awaits mkdir first, so the number of // microtask turns before writeFile is reached is an implementation detail. gate.blocked = true - const inflight = internals.writeToDiskAsync() + const inflight = internals.enqueueWrite() await vi.waitFor(() => expect(gate.writeFileCalls).toBeGreaterThan(0)) // App quits: close out both agents and flush the COMPLETE state synchronously @@ -147,13 +222,13 @@ describe('StatsCollector async debounced save', () => { const { StatsCollector, initStatsPath } = await importCollector() initStatsPath() const collector = new StatsCollector() - const internals = collector as unknown as { writeToDiskAsync: () => Promise } + const internals = collector as unknown as { enqueueWrite: () => Promise } collector.onAgentStart('pty-a', 1_000) collector.onAgentStart('pty-b', 1_000) gate.blockRename = true - const inflight = internals.writeToDiskAsync() + const inflight = internals.enqueueWrite() await vi.waitFor(() => expect(gate.renameCalls).toBeGreaterThan(0)) collector.onAgentStop('pty-a', 5_000) diff --git a/src/main/stats/collector.ts b/src/main/stats/collector.ts index f91742e4a7f..9c0889d830c 100644 --- a/src/main/stats/collector.ts +++ b/src/main/stats/collector.ts @@ -1,9 +1,9 @@ import { app } from 'electron' -import { readFileSync, writeFileSync, mkdirSync, existsSync, renameSync, unlinkSync } from 'node:fs' -import { writeFile, mkdir, rm, rename } from 'node:fs/promises' -import { join, dirname } from 'node:path' +import { join } from 'node:path' import type { StatsSummary } from '../../shared/types' -import type { StatsEvent, StatsAggregates, StatsFile } from './types' +import type { StatsEvent, StatsAggregates } from './types' +import { loadStatsFile } from './stats-file-loader' +import { StatsSnapshotWriter } from './stats-snapshot-writer' const STATS_SCHEMA_VERSION = 1 const MAX_EVENTS = 10_000 @@ -33,45 +33,22 @@ function getStatsFile(): string { return _statsFile } -function getDefaultAggregates(): StatsAggregates { - return { - totalAgentsSpawned: 0, - totalPRsCreated: 0, - totalAgentTimeMs: 0, - countedPRs: [], - firstEventAt: null - } -} - -function getDefaultStatsFile(): StatsFile { - return { - schemaVersion: STATS_SCHEMA_VERSION, - events: [], - aggregates: getDefaultAggregates() - } -} - export class StatsCollector { private events: StatsEvent[] private aggregates: StatsAggregates private liveAgents = new Map() // ptyId → startTimestamp private writeTimer: ReturnType | null = null - // Monotonic id stamped on each prepared payload; the highest committed one - // wins so a slow in-flight async write can't clobber a newer sync flush. - private writeGeneration = 0 - private lastCommittedGeneration = 0 - private writeChain: Promise = Promise.resolve() - /** Temp path of the async write awaiting its rename; writeToDiskSync removes it to veto that rename. */ - private inFlightAsyncTmpFile: string | null = null + private readonly snapshotWriter = new StatsSnapshotWriter(getStatsFile) /** Set by flushAsync so the quit flush is the final write; see scheduleSave. */ private quitFlushStarted = false + private quitFlushPromise: Promise | null = null // Why: star-nag lives in its own service but needs to observe the running // agent-spawned counter. A lightweight listener avoids cyclic imports and // keeps StatsCollector unaware of how the counter is consumed. private agentStartListeners: ((totalAgentsSpawned: number) => void)[] = [] constructor() { - const data = this.load() + const data = loadStatsFile(getStatsFile()) this.events = data.events this.aggregates = data.aggregates } @@ -151,9 +128,12 @@ export class StatsCollector { * a second flush() after resumed activity works correctly. */ flush(): void { + if (this.quitFlushStarted) { + return + } this.closeOutLiveAgents() this.cancelPendingSave() - this.writeToDiskSync() + this.snapshotWriter.writeSync(() => this.serialize()) } /** @@ -170,12 +150,16 @@ export class StatsCollector { * Never throws — it joins the quit teardown barrier, where a rejection is noise. */ flushAsync(): Promise { + if (this.quitFlushPromise) { + return this.quitFlushPromise + } this.quitFlushStarted = true this.closeOutLiveAgents() this.cancelPendingSave() - return this.enqueueWrite().catch((err) => { + this.quitFlushPromise = this.enqueueWrite().catch((err) => { console.error('[stats] Failed to flush stats:', err) }) + return this.quitFlushPromise } private closeOutLiveAgents(): void { @@ -190,32 +174,6 @@ export class StatsCollector { // ── Persistence ─────────────────────────────────────────────────── - private load(): StatsFile { - try { - const statsFile = getStatsFile() - if (existsSync(statsFile)) { - const raw = readFileSync(statsFile, 'utf-8') - const parsed = JSON.parse(raw) as StatsFile - // Merge with defaults for forward compatibility - return { - ...getDefaultStatsFile(), - ...parsed, - aggregates: { - ...getDefaultAggregates(), - ...parsed.aggregates - } - } - } - } catch (err) { - // Why "start fresh" instead of crashing: lifetime aggregates are lost - // on corruption, which is unfortunate but not critical — this is a - // "fun stats" feature, not billing data. The corrupt file is left on - // disk so it can be inspected for debugging. - console.error('[stats] Failed to load stats, starting fresh:', err) - } - return getDefaultStatsFile() - } - private updateAggregates(event: StatsEvent): void { if (this.aggregates.firstEventAt === null) { this.aggregates.firstEventAt = event.at @@ -287,96 +245,18 @@ export class StatsCollector { } } - // Serialize the current state and pick a unique temp path. Trimming mutates - // in-memory state and JSON.stringify must see a consistent snapshot, so both - // writers call this synchronously before any await to avoid a torn snapshot. - // The monotonic generation lets a later write veto an earlier, still-in-flight - // one so a stale rename can never win (see writeToDiskAsync). - private prepareWritePayload(): { - statsFile: string - tmpFile: string - json: string - generation: number - } { - const statsFile = getStatsFile() - - // Trim events to bounded size before writing + private serialize(): string { if (this.events.length > MAX_EVENTS) { this.events = this.events.slice(-MAX_EVENTS) } - - const data: StatsFile = { + return JSON.stringify({ schemaVersion: STATS_SCHEMA_VERSION, events: this.events, aggregates: this.aggregates - } - - const generation = ++this.writeGeneration - // Unique temp file so the async debounced writer and the sync shutdown - // flush never write the same temp path (same pattern as persistence.ts). - const tmpFile = `${statsFile}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp` - return { statsFile, tmpFile, json: JSON.stringify(data), generation } + }) } - private writeToDiskSync(): void { - const dir = dirname(getStatsFile()) - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }) - } - // Why remove the async writer's temp file: once it is parked on `await rename` it has - // already cleared the generation guard, so nothing downstream can stop it clobbering the - // fuller state below. Deleting what it would rename turns that swap into a swallowed ENOENT. - if (this.inFlightAsyncTmpFile) { - try { - unlinkSync(this.inFlightAsyncTmpFile) - } catch { - // Already renamed or never created. - } - this.inFlightAsyncTmpFile = null - } - const { statsFile, tmpFile, json, generation } = this.prepareWritePayload() - writeFileSync(tmpFile, json, 'utf-8') - renameSync(tmpFile, statsFile) - this.lastCommittedGeneration = Math.max(this.lastCommittedGeneration, generation) - } - - // Why serialize: the rename below is async, so two overlapping writes could land in - // either order and let an older payload win. One writer at a time makes the generation - // guard sufficient again — it only has to veto payloads staler than what was committed. private enqueueWrite(): Promise { - const next = this.writeChain.then(() => this.writeToDiskAsync()) - this.writeChain = next.catch(() => {}) - return next - } - - private async writeToDiskAsync(): Promise { - const dir = dirname(getStatsFile()) - // Why not existsSync first: that probe parks the main thread on a stalled network - // profile mount, and recursive mkdir is already a no-op when the directory exists. - await mkdir(dir, { recursive: true }).catch(() => {}) - const { statsFile, tmpFile, json, generation } = this.prepareWritePayload() - // Stringify stayed sync above (torn-snapshot constraint); every syscall from here is - // async so the main thread never parks on the profile mount. The generation guard - // vetoes this write if a newer one already committed while we were writing. - await writeFile(tmpFile, json, 'utf-8') - if (this.lastCommittedGeneration >= generation) { - await rm(tmpFile, { force: true }).catch(() => {}) - return - } - this.inFlightAsyncTmpFile = tmpFile - try { - await rename(tmpFile, statsFile) - this.lastCommittedGeneration = generation - } catch (err) { - // Why swallow ENOENT: writeToDiskSync removed our temp file to keep this stale swap - // from overwriting the shutdown state it just committed. Anything else is a real error. - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { - throw err - } - } finally { - if (this.inFlightAsyncTmpFile === tmpFile) { - this.inFlightAsyncTmpFile = null - } - } + return this.snapshotWriter.write(() => this.serialize()) } } diff --git a/src/main/stats/stats-file-loader.ts b/src/main/stats/stats-file-loader.ts new file mode 100644 index 00000000000..302be3285fc --- /dev/null +++ b/src/main/stats/stats-file-loader.ts @@ -0,0 +1,38 @@ +import { existsSync, readFileSync } from 'node:fs' +import type { StatsAggregates, StatsFile } from './types' + +const STATS_SCHEMA_VERSION = 1 + +function defaultAggregates(): StatsAggregates { + return { + totalAgentsSpawned: 0, + totalPRsCreated: 0, + totalAgentTimeMs: 0, + countedPRs: [], + firstEventAt: null + } +} + +function defaultStatsFile(): StatsFile { + return { + schemaVersion: STATS_SCHEMA_VERSION, + events: [], + aggregates: defaultAggregates() + } +} + +export function loadStatsFile(statsFile: string): StatsFile { + try { + if (existsSync(statsFile)) { + const parsed = JSON.parse(readFileSync(statsFile, 'utf-8')) as StatsFile + return { + ...defaultStatsFile(), + ...parsed, + aggregates: { ...defaultAggregates(), ...parsed.aggregates } + } + } + } catch (err) { + console.error('[stats] Failed to load stats, starting fresh:', err) + } + return defaultStatsFile() +} diff --git a/src/main/stats/stats-snapshot-writer.ts b/src/main/stats/stats-snapshot-writer.ts new file mode 100644 index 00000000000..b9b1d911840 --- /dev/null +++ b/src/main/stats/stats-snapshot-writer.ts @@ -0,0 +1,134 @@ +import { existsSync, mkdirSync, renameSync, unlinkSync, writeFileSync } from 'node:fs' +import { mkdir, rename, rm, writeFile } from 'node:fs/promises' +import { dirname } from 'node:path' +import { durableWriteTempPath, removeStaleDurableWriteTempFiles } from '../durable-file-write' + +const STALE_WRITE_TEMP_AGE_MS = 24 * 60 * 60 * 1000 + +export class StatsSnapshotWriter { + private writeGeneration = 0 + private lastCommittedGeneration = 0 + private writeRequested = false + private pendingWrite: Promise | null = null + private pendingSerialize: (() => string) | null = null + private readonly staleTempCleanup: Promise + private inFlightAsyncTmpFile: string | null = null + + constructor(private readonly resolveFile: () => string) { + this.staleTempCleanup = removeStaleDurableWriteTempFiles(resolveFile(), { + minimumAgeMs: STALE_WRITE_TEMP_AGE_MS + }) + } + + write(serialize: () => string): Promise { + this.writeRequested = true + this.pendingSerialize = serialize + if (this.pendingWrite) { + return this.pendingWrite + } + const run = this.drainWrites() + const tracked = run.finally(() => { + if (this.pendingWrite === tracked) { + this.pendingWrite = null + } + }) + this.pendingWrite = tracked + return tracked + } + + writeSync(serialize: () => string): void { + const statsFile = this.resolveFile() + const dir = dirname(statsFile) + if (!existsSync(dir)) { + mkdirSync(dir, { recursive: true }) + } + if (this.inFlightAsyncTmpFile) { + try { + unlinkSync(this.inFlightAsyncTmpFile) + this.inFlightAsyncTmpFile = null + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + void this.write(serialize).catch((writeError) => { + console.error('[stats] Failed to write stats:', writeError) + }) + throw error + } + } + } + const { tmpFile, json, generation } = this.preparePayload(serialize) + writeFileSync(tmpFile, json, 'utf-8') + renameSync(tmpFile, statsFile) + this.lastCommittedGeneration = Math.max(this.lastCommittedGeneration, generation) + } + + waitForPendingWrite(): Promise { + return this.pendingWrite ?? Promise.resolve() + } + + private preparePayload(serialize: () => string): { + tmpFile: string + json: string + generation: number + } { + const generation = ++this.writeGeneration + return { + tmpFile: durableWriteTempPath(this.resolveFile()), + json: serialize(), + generation + } + } + + private async drainWrites(): Promise { + await this.staleTempCleanup + let firstError: unknown = null + while (this.writeRequested) { + this.writeRequested = false + const serialize = this.pendingSerialize! + try { + await this.writeToDiskAsync(serialize) + } catch (error) { + firstError ??= error + if (!this.writeRequested) { + throw error + } + } + } + if (firstError) { + throw firstError + } + } + + private async writeToDiskAsync(serialize: () => string): Promise { + const statsFile = this.resolveFile() + await mkdir(dirname(statsFile), { recursive: true }).catch(() => {}) + const { tmpFile, json, generation } = this.preparePayload(serialize) + let renamed = false + try { + await writeFile(tmpFile, json, 'utf-8') + if (this.lastCommittedGeneration >= generation) { + return + } + this.inFlightAsyncTmpFile = tmpFile + try { + await rename(tmpFile, statsFile) + renamed = true + this.lastCommittedGeneration = Math.max(this.lastCommittedGeneration, generation) + } catch (err) { + if ( + (err as NodeJS.ErrnoException).code !== 'ENOENT' || + this.lastCommittedGeneration < generation + ) { + throw err + } + } finally { + if (this.inFlightAsyncTmpFile === tmpFile) { + this.inFlightAsyncTmpFile = null + } + } + } finally { + if (!renamed) { + await rm(tmpFile, { force: true }).catch(() => {}) + } + } + } +} diff --git a/src/main/terminal-scrollback-snapshot-async-migration.ts b/src/main/terminal-scrollback-snapshot-async-migration.ts new file mode 100644 index 00000000000..b1a064657c1 --- /dev/null +++ b/src/main/terminal-scrollback-snapshot-async-migration.ts @@ -0,0 +1,54 @@ +import type { WorkspaceSessionState } from '../shared/types' +import { + collectTerminalScrollbackSnapshotRefs, + deleteTerminalScrollbackSnapshot, + type TerminalScrollbackSnapshotStorage, + writeTerminalScrollbackSnapshot +} from './terminal-scrollback-snapshots' + +export async function migrateWorkspaceSessionTerminalScrollbackSnapshotsAsync( + session: WorkspaceSessionState, + storage?: TerminalScrollbackSnapshotStorage +): Promise { + let terminalLayoutsByTabId: WorkspaceSessionState['terminalLayoutsByTabId'] | null = null + for (const [tabId, layout] of Object.entries(session.terminalLayoutsByTabId ?? {})) { + const buffers = layout.buffersByLeafId + if (!buffers || Object.keys(buffers).length === 0) { + continue + } + const refs = { ...layout.scrollbackRefsByLeafId } + const remainingBuffers: Record = {} + for (const [leafId, buffer] of Object.entries(buffers)) { + const ref = await writeTerminalScrollbackSnapshot({ tabId, leafId, buffer, storage }) + if (ref) { + refs[leafId] = ref + } else { + remainingBuffers[leafId] = buffer + delete refs[leafId] + } + } + terminalLayoutsByTabId ??= { ...session.terminalLayoutsByTabId } + terminalLayoutsByTabId[tabId] = { + ...layout, + buffersByLeafId: Object.keys(remainingBuffers).length > 0 ? remainingBuffers : undefined, + scrollbackRefsByLeafId: Object.keys(refs).length > 0 ? refs : undefined + } + } + return terminalLayoutsByTabId ? { ...session, terminalLayoutsByTabId } : session +} + +export async function deleteRemovedTerminalScrollbackSnapshotsAsync( + prior: WorkspaceSessionState | undefined, + next: WorkspaceSessionState, + storage?: TerminalScrollbackSnapshotStorage +): Promise { + if (!prior) { + return + } + const nextRefs = collectTerminalScrollbackSnapshotRefs(next) + await Promise.all( + [...collectTerminalScrollbackSnapshotRefs(prior)] + .filter((ref) => !nextRefs.has(ref)) + .map((ref) => deleteTerminalScrollbackSnapshot(ref, storage)) + ) +} diff --git a/src/main/terminal-scrollback-snapshots.ts b/src/main/terminal-scrollback-snapshots.ts index 3929288d588..de387c356c9 100644 --- a/src/main/terminal-scrollback-snapshots.ts +++ b/src/main/terminal-scrollback-snapshots.ts @@ -9,6 +9,7 @@ import { statSync, writeFileSync } from 'node:fs' +import { mkdir, rename, rm, writeFile } from 'node:fs/promises' import { dirname, join } from 'node:path' import { app } from 'electron' import type { WorkspaceSessionState } from '../shared/types' @@ -133,6 +134,42 @@ export function writeTerminalScrollbackSnapshotSync(args: { } } +export async function writeTerminalScrollbackSnapshot(args: { + tabId: string + leafId: string + buffer: string + storage?: TerminalScrollbackSnapshotStorage +}): Promise { + if (!args.buffer) { + return null + } + const ref = makeTerminalScrollbackSnapshotRef(args.tabId, args.leafId) + const snapshotRoot = getSnapshotRoot(args.storage) + const path = snapshotPath(ref, snapshotRoot) + if (!path) { + return null + } + const tmpPath = `${path}.${process.pid}.${Date.now()}.${Math.random().toString(16).slice(2)}.tmp` + let renamed = false + try { + await mkdir(snapshotRoot, { recursive: true, mode: 0o700 }) + const bytes = trailingUtf8Bytes(args.buffer, TERMINAL_SCROLLBACK_STORE_BYTE_LIMIT) + await writeFile(tmpPath, bytes, { mode: 0o600 }) + await rename(tmpPath, path) + renamed = true + return ref + } catch (err) { + console.warn( + `[terminal-scrollback] Failed to write snapshot: ${err instanceof Error ? err.message : String(err)}` + ) + return null + } finally { + if (!renamed) { + await rm(tmpPath, { force: true }).catch(() => {}) + } + } +} + export function readTerminalScrollbackSnapshotSync( ref: string, storage?: TerminalScrollbackSnapshotStorage @@ -160,6 +197,15 @@ export function deleteTerminalScrollbackSnapshotSync( } } +export async function deleteTerminalScrollbackSnapshot( + ref: string, + storage?: TerminalScrollbackSnapshotStorage +): Promise { + await Promise.all( + snapshotReadPaths(ref, storage).map((path) => rm(path, { force: true }).catch(() => {})) + ) +} + export function collectTerminalScrollbackSnapshotRefs(session: WorkspaceSessionState): Set { const refs = new Set() for (const layout of Object.values(session.terminalLayoutsByTabId ?? {})) { diff --git a/src/main/window/attach-main-window-services.test.ts b/src/main/window/attach-main-window-services.test.ts index bc1e7ae258d..9bb9e35f9e3 100644 --- a/src/main/window/attach-main-window-services.test.ts +++ b/src/main/window/attach-main-window-services.test.ts @@ -160,8 +160,10 @@ function createMainWindow( } } -function createStore(): Store & { flush: MockFn } { - return { flush: vi.fn() } as Store & { flush: MockFn } +function createStore(): Store & { flushPendingAsync: MockFn } { + return { + flushPendingAsync: vi.fn(() => Promise.resolve()) + } as Store & { flushPendingAsync: MockFn } } function createRuntime(): RuntimeStub { @@ -299,7 +301,7 @@ describe('attachMainWindowServices', () => { await setupAutoUpdaterMock.mock.calls[0][1].onBeforeQuit() expect(onBeforeUpdateQuit).toHaveBeenCalledTimes(1) - expect(store.flush).toHaveBeenCalledTimes(1) + expect(store.flushPendingAsync).toHaveBeenCalledTimes(1) }) it('flushes the store before update quit when no cleanup is injected', async () => { @@ -311,7 +313,7 @@ describe('attachMainWindowServices', () => { await fireReadyToShow(mainWindow) await setupAutoUpdaterMock.mock.calls[0][1].onBeforeQuit() - expect(store.flush).toHaveBeenCalledTimes(1) + expect(store.flushPendingAsync).toHaveBeenCalledTimes(1) }) it('replaces the TCC handlers when the main window is reattached', () => { diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index 5984fe715af..b580b97e456 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -159,7 +159,7 @@ export function attachMainWindowServices( try { await options?.onBeforeUpdateQuit?.() } finally { - store.flush() + await store.flushPendingAsync() } }, setLastUpdateCheckAt: (timestamp) => { diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 368c4df4ff7..692b00e6d2d 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -968,9 +968,8 @@ export type AppApi = { /** Reloads the current app renderer through main so expected renderer * teardown can be classified before Electron emits process-gone events. */ reload: () => Promise - /** Commits the renderer's final locally durable state before unload and - * throws when the blocking durable write fails. */ - persistBeforeUnloadSync: (args: { + /** Stages the renderer's final state synchronously before unload. */ + stageBeforeUnloadSync: (args: { sessions: { state: WorkspaceSessionState; hostId?: ExecutionHostId }[] ui: Partial }) => void diff --git a/src/preload/index.ts b/src/preload/index.ts index e2ca5656d00..b190bf13a37 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -484,14 +484,12 @@ const api = { } }, reload: (): Promise => ipcRenderer.invoke('app:reload'), - persistBeforeUnloadSync: ( - args: Parameters[0] - ) => { - const result = ipcRenderer.sendSync('app:persist-before-unload-sync', args) as { + stageBeforeUnloadSync: (args: Parameters[0]) => { + const result = ipcRenderer.sendSync('app:stage-before-unload-sync', args) as { ok?: unknown } if (result?.ok !== true) { - throw new Error('Failed to persist renderer state before unload.') + throw new Error('Failed to stage renderer state before unload.') } }, awaitFirstWindowStartupServices: (): Promise => diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index eca968e1b05..179108b194b 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -1317,9 +1317,7 @@ function App(): React.JSX.Element { const sessionSnapshots = shouldCaptureSession ? buildWorkspaceSessionHostSnapshots(buildWorkspaceSessionPayload(freshState), freshState) : [] - // Why: one blocking checkpoint closes the immediate-quit race for both - // the narrow view preference and the larger session recovery snapshots. - window.api.app.persistBeforeUnloadSync({ + window.api.app.stageBeforeUnloadSync({ sessions: sessionSnapshots, ui: buildActiveViewUnloadPatch(freshState) }) diff --git a/src/renderer/src/app-startup-routing.test.ts b/src/renderer/src/app-startup-routing.test.ts index 4b41790f87f..bfe17737970 100644 --- a/src/renderer/src/app-startup-routing.test.ts +++ b/src/renderer/src/app-startup-routing.test.ts @@ -443,7 +443,7 @@ describe('renderer startup runtime routing', () => { expect(checkpointBlock).toContain( 'buildWorkspaceSessionHostSnapshots(buildWorkspaceSessionPayload(freshState), freshState)' ) - expect(checkpointBlock).toContain('window.api.app.persistBeforeUnloadSync({') + expect(checkpointBlock).toContain('window.api.app.stageBeforeUnloadSync({') expect(checkpointBlock).toContain('sessions: sessionSnapshots') expect(checkpointBlock).toContain('ui: buildActiveViewUnloadPatch(freshState)') expect(source).toContain( diff --git a/src/renderer/src/web/web-preload-api.test.ts b/src/renderer/src/web/web-preload-api.test.ts index 6b5b3a093e3..a8aec38fc92 100644 --- a/src/renderer/src/web/web-preload-api.test.ts +++ b/src/renderer/src/web/web-preload-api.test.ts @@ -207,7 +207,7 @@ describe('web before-unload persistence', () => { it('persists final UI and host-partitioned sessions synchronously', async () => { const { api, storage } = await installApi('Linux') - api.app.persistBeforeUnloadSync({ + api.app.stageBeforeUnloadSync({ sessions: [ { state: { activeWorktreeId: 'local-worktree' } as never }, { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index b9df0d8f5a5..962a9440ca8 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -548,7 +548,7 @@ function createWebPreloadApi(): Partial { relaunch: () => Promise.resolve(window.location.reload()), restart: () => Promise.resolve(window.location.reload()), reload: () => Promise.resolve(window.location.reload()), - persistBeforeUnloadSync: ({ sessions, ui }) => { + stageBeforeUnloadSync: ({ sessions, ui }) => { // Why: beforeunload cannot await the paired runtime, so the web adapter // guarantees immediate browser-local durability for the final snapshot. for (const { state, hostId } of sessions) {