diff --git a/src/main/active-view-persistence-boundary.test.ts b/src/main/active-view-persistence-boundary.test.ts new file mode 100644 index 000000000..26bb25ac7 --- /dev/null +++ b/src/main/active-view-persistence-boundary.test.ts @@ -0,0 +1,58 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { getActiveViewPreferenceFile } from './active-view-preference' + +const testState = { dir: '' } + +vi.mock('electron', () => ({ + app: { getPath: () => testState.dir }, + safeStorage: { + isEncryptionAvailable: () => true, + encryptString: (plaintext: string) => Buffer.from(`encrypted:${plaintext}`, 'utf-8'), + decryptString: (ciphertext: Buffer) => ciphertext.toString('utf-8').replace('encrypted:', '') + } +})) + +vi.mock('./telemetry/client', () => ({ track: vi.fn() })) +vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn() })) +vi.mock('./ssh/ssh-config-parser', () => ({ + loadUserSshConfig: vi.fn(), + sshConfigHostsToTargets: vi.fn() +})) + +describe('active-view persistence boundary', () => { + beforeEach(() => { + testState.dir = mkdtempSync(join(tmpdir(), 'orca-active-view-boundary-')) + }) + + afterEach(() => { + vi.useRealTimers() + rmSync(testState.dir, { recursive: true, force: true }) + }) + + it('persists a view switch without changing the global durable snapshot', async () => { + vi.useFakeTimers() + vi.resetModules() + const { Store } = await import('./persistence') + const dataFile = join(testState.dir, 'orca-data.json') + const store = new Store({ dataFile }) + store.flush() + const durableBefore = readFileSync(dataFile, 'utf-8') + + store.updateUI({ activeView: 'settings' }) + vi.advanceTimersByTime(1_000) + await store.waitForPendingWrite() + + const preferenceFile = getActiveViewPreferenceFile(dataFile) + const preferencePayload = readFileSync(preferenceFile, 'utf-8') + expect(Buffer.byteLength(preferencePayload)).toBeLessThan(64) + expect(JSON.parse(preferencePayload)).toEqual({ activeView: 'settings' }) + expect(readFileSync(dataFile, 'utf-8')).toBe(durableBefore) + expect(store.getUI().activeView).toBe('settings') + + const reloaded = new Store({ dataFile }) + expect(reloaded.getUI().activeView).toBe('settings') + }) +}) diff --git a/src/main/active-view-preference.test.ts b/src/main/active-view-preference.test.ts new file mode 100644 index 000000000..972009f72 --- /dev/null +++ b/src/main/active-view-preference.test.ts @@ -0,0 +1,91 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { ActiveViewPreference, getActiveViewPreferenceFile } from './active-view-preference' + +describe('ActiveViewPreference', () => { + let dir: string + let dataFile: string + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'orca-active-view-')) + dataFile = join(dir, 'orca-data.json') + }) + + afterEach(() => { + vi.useRealTimers() + rmSync(dir, { recursive: true, force: true }) + }) + + it('migrates the legacy durable-state value into the profile sidecar', async () => { + vi.useFakeTimers() + const preference = new ActiveViewPreference(dataFile, 'tasks') + + expect(preference.get()).toBe('tasks') + expect(existsSync(getActiveViewPreferenceFile(dataFile))).toBe(false) + + // The renderer reasserts its hydrated value once persistence is ready. + expect(preference.set('tasks')).toBe(false) + vi.advanceTimersByTime(100) + await preference.waitForPendingWrite() + + expect(JSON.parse(readFileSync(getActiveViewPreferenceFile(dataFile), 'utf-8'))).toEqual({ + activeView: 'tasks' + }) + }) + + it('coalesces rapid switches into one tiny preference write', async () => { + vi.useFakeTimers() + const preference = new ActiveViewPreference(dataFile, 'terminal') + + preference.set('settings') + vi.advanceTimersByTime(50) + preference.set('automations') + vi.advanceTimersByTime(50) + expect(existsSync(getActiveViewPreferenceFile(dataFile))).toBe(false) + + vi.advanceTimersByTime(50) + await preference.waitForPendingWrite() + + expect(JSON.parse(readFileSync(getActiveViewPreferenceFile(dataFile), 'utf-8'))).toEqual({ + activeView: 'automations' + }) + }) + + it('flushes synchronously for an immediate graceful exit', () => { + vi.useFakeTimers() + const preference = new ActiveViewPreference(dataFile, 'terminal') + + preference.set('settings') + preference.flushOrThrow() + + expect(JSON.parse(readFileSync(getActiveViewPreferenceFile(dataFile), 'utf-8'))).toEqual({ + activeView: 'settings' + }) + vi.advanceTimersByTime(100) + expect(JSON.parse(readFileSync(getActiveViewPreferenceFile(dataFile), 'utf-8'))).toEqual({ + activeView: 'settings' + }) + }) + + it('ignores an invalid sidecar and invalid updates', () => { + writeFileSync(getActiveViewPreferenceFile(dataFile), '{"activeView":"unknown"}', 'utf-8') + const preference = new ActiveViewPreference(dataFile, 'not-a-view') + + expect(preference.get()).toBe('terminal') + expect(preference.set('also-not-a-view')).toBe(false) + expect(preference.get()).toBe('terminal') + }) + + it('rejects inherited object keys from a corrupt sidecar', () => { + // Why: `constructor`/`__proto__` are truthy under `in`; the sidecar must not + // treat them as a valid view and leave the main surface blank. + writeFileSync(getActiveViewPreferenceFile(dataFile), '{"activeView":"constructor"}', 'utf-8') + const preference = new ActiveViewPreference(dataFile, 'tasks') + + expect(preference.get()).toBe('tasks') + expect(preference.set('__proto__')).toBe(false) + expect(preference.get()).toBe('tasks') + }) +}) diff --git a/src/main/active-view-preference.ts b/src/main/active-view-preference.ts new file mode 100644 index 000000000..beed983ae --- /dev/null +++ b/src/main/active-view-preference.ts @@ -0,0 +1,136 @@ +import { mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs' +import { mkdir, rm, writeFile } from 'node:fs/promises' +import { dirname, join } from 'node:path' +import type { TopLevelView } from '../shared/types' +import { isTopLevelView } from '../shared/top-level-view' + +const ACTIVE_VIEW_FILE_NAME = 'active-view.json' +const SAVE_DEBOUNCE_MS = 100 + +type ActiveViewFile = { + activeView: TopLevelView +} + +export function getActiveViewPreferenceFile(dataFile: string): string { + return join(dirname(dataFile), ACTIVE_VIEW_FILE_NAME) +} + +function readActiveView(file: string): TopLevelView | null { + try { + const parsed = JSON.parse(readFileSync(file, 'utf-8')) as Partial + return isTopLevelView(parsed.activeView) ? parsed.activeView : null + } catch { + return null + } +} + +function serializeActiveView(activeView: TopLevelView): string { + return `${JSON.stringify({ activeView } satisfies ActiveViewFile)}\n` +} + +export class ActiveViewPreference { + private readonly file: string + private activeView: TopLevelView + private persistedActiveView: TopLevelView | null + private writeTimer: ReturnType | null = null + private pendingWrite: Promise | null = null + private writeGeneration = 0 + + constructor(dataFile: string, legacyActiveView: unknown) { + this.file = getActiveViewPreferenceFile(dataFile) + const storedActiveView = readActiveView(this.file) + const fallbackActiveView = isTopLevelView(legacyActiveView) ? legacyActiveView : 'terminal' + this.activeView = storedActiveView ?? fallbackActiveView + this.persistedActiveView = storedActiveView + } + + get(): TopLevelView { + return this.activeView + } + + set(value: unknown): boolean { + if (!isTopLevelView(value)) { + return false + } + const changed = value !== this.activeView + this.activeView = value + if ( + value !== this.persistedActiveView || + this.writeTimer !== null || + this.pendingWrite !== null + ) { + this.scheduleSave() + } + return changed + } + + private scheduleSave(): void { + this.writeGeneration += 1 + if (this.writeTimer) { + clearTimeout(this.writeTimer) + } + this.writeTimer = setTimeout(() => { + this.writeTimer = null + const generation = this.writeGeneration + const activeView = this.activeView + const previousWrite = this.pendingWrite ?? Promise.resolve() + const nextWrite = previousWrite + .then(() => this.writeAsync(activeView, generation)) + .catch((error) => { + console.error('[active-view] Failed to persist preference:', error) + }) + .finally(() => { + if (this.pendingWrite === nextWrite) { + this.pendingWrite = null + } + }) + this.pendingWrite = nextWrite + }, SAVE_DEBOUNCE_MS) + } + + private async writeAsync(activeView: TopLevelView, generation: number): Promise { + const tmpFile = `${this.file}.${process.pid}.${generation}.tmp` + let renamed = false + try { + await mkdir(dirname(this.file), { recursive: true }) + await writeFile(tmpFile, serializeActiveView(activeView), 'utf-8') + // Why: keep the generation guard and the swap synchronous (no await between), + // or a concurrent flushOrThrow could rename a newer view that this stale write + // then clobbers, restoring the prior view on next launch. + if (generation !== this.writeGeneration) { + return + } + renameSync(tmpFile, this.file) + renamed = true + this.persistedActiveView = activeView + } finally { + if (!renamed) { + await rm(tmpFile).catch(() => {}) + } + } + } + + flushOrThrow(): void { + 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 + } + mkdirSync(dirname(this.file), { recursive: true }) + const tmpFile = `${this.file}.${process.pid}.${this.writeGeneration}.tmp` + writeFileSync(tmpFile, serializeActiveView(this.activeView), 'utf-8') + renameSync(tmpFile, this.file) + this.persistedActiveView = this.activeView + } + + async waitForPendingWrite(): Promise { + if (this.pendingWrite) { + await this.pendingWrite + } + } +} diff --git a/src/main/ipc/app.test.ts b/src/main/ipc/app.test.ts index de9a13753..7702dcd63 100644 --- a/src/main/ipc/app.test.ts +++ b/src/main/ipc/app.test.ts @@ -10,7 +10,8 @@ const { destroySystemTrayMock, relaunchAppMock, showOpenDialogMock, - grantFloatingWorkspaceDirectoryMock + grantFloatingWorkspaceDirectoryMock, + registerRendererShutdownCheckpointHandlerMock } = vi.hoisted(() => ({ handlers: new Map unknown>(), appExitMock: vi.fn(), @@ -20,7 +21,8 @@ const { destroySystemTrayMock: vi.fn(), relaunchAppMock: vi.fn(), showOpenDialogMock: vi.fn(), - grantFloatingWorkspaceDirectoryMock: vi.fn() + grantFloatingWorkspaceDirectoryMock: vi.fn(), + registerRendererShutdownCheckpointHandlerMock: vi.fn() })) vi.mock('node:child_process', () => ({ @@ -103,6 +105,10 @@ vi.mock('./floating-workspace-directory', () => ({ resolveFloatingTerminalCwd: vi.fn() })) +vi.mock('./renderer-shutdown-checkpoint', () => ({ + registerRendererShutdownCheckpointHandler: registerRendererShutdownCheckpointHandlerMock +})) + import { registerAppHandlers } from './app' describe('registerAppHandlers', () => { @@ -123,6 +129,7 @@ describe('registerAppHandlers', () => { relaunchAppMock.mockImplementation(() => appRelaunchMock()) showOpenDialogMock.mockReset() grantFloatingWorkspaceDirectoryMock.mockReset() + registerRendererShutdownCheckpointHandlerMock.mockReset() processKillSpy = vi.spyOn(process, 'kill').mockReturnValue(true) Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }) }) @@ -133,6 +140,14 @@ describe('registerAppHandlers', () => { Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }) }) + it('registers the combined renderer shutdown checkpoint', () => { + const store = {} + + registerAppHandlers(store as never) + + expect(registerRendererShutdownCheckpointHandlerMock).toHaveBeenCalledWith(store) + }) + it('marks relaunch as expected shutdown before exiting', async () => { const onBeforeRelaunch = vi.fn() registerAppHandlers({} as never, { onBeforeRelaunch }) diff --git a/src/main/ipc/app.ts b/src/main/ipc/app.ts index f97ae1e2c..d943d18dc 100644 --- a/src/main/ipc/app.ts +++ b/src/main/ipc/app.ts @@ -21,6 +21,7 @@ import { resolveFloatingTerminalCwd } from './floating-workspace-directory' import { isMarkdownDocumentName, markdownDocumentFromFilePath } from './markdown-documents' +import { registerRendererShutdownCheckpointHandler } from './renderer-shutdown-checkpoint' const KEYBOARD_INPUT_SOURCE_TIMEOUT_MS = 500 const MAC_HITOOLBOX_DOMAIN = 'com.apple.HIToolbox' @@ -253,6 +254,8 @@ async function readKeyboardInputSourceId(): Promise { } export function registerAppHandlers(store: Store, options: RegisterAppHandlersOptions = {}): void { + registerRendererShutdownCheckpointHandler(store) + ipcMain.handle('app:getFeatureWallAssetBaseUrl', (): string => getFeatureWallAssetBaseUrl()) ipcMain.handle('app:getIdentity', (): AppIdentity => { diff --git a/src/main/ipc/renderer-shutdown-checkpoint.test.ts b/src/main/ipc/renderer-shutdown-checkpoint.test.ts new file mode 100644 index 000000000..ca6e335f0 --- /dev/null +++ b/src/main/ipc/renderer-shutdown-checkpoint.test.ts @@ -0,0 +1,124 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { syncHandlers } = vi.hoisted(() => ({ + syncHandlers: new Map< + string, + (event: { returnValue?: unknown }, args: Record) => void + >() +})) + +vi.mock('electron', () => ({ + ipcMain: { + on: vi.fn( + ( + channel: string, + handler: (event: { returnValue?: unknown }, args: Record) => void + ) => { + syncHandlers.set(channel, handler) + } + ) + } +})) + +import { registerRendererShutdownCheckpointHandler } from './renderer-shutdown-checkpoint' + +describe('registerRendererShutdownCheckpointHandler', () => { + beforeEach(() => { + syncHandlers.clear() + }) + + it('commits every shutdown state mutation before flushing both stores', () => { + const callOrder: string[] = [] + const store = { + setWorkspaceSession: 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')) + } + registerRendererShutdownCheckpointHandler(store as never) + + const handler = syncHandlers.get('app:persist-before-unload-sync') + expect(handler).toBeDefined() + const event: { returnValue?: unknown } = {} + const localSession = { activeWorktreeId: 'local-worktree' } + const remoteSession = { activeWorktreeId: 'remote-worktree' } + handler?.(event, { + sessions: [{ state: localSession }, { state: remoteSession, hostId: 'runtime:host-1' }], + ui: { activeView: 'settings' } + }) + + expect(store.setWorkspaceSession).toHaveBeenNthCalledWith(1, localSession, undefined) + expect(store.setWorkspaceSession).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(event.returnValue).toEqual({ ok: true }) + }) + + it('reports a failed durable checkpoint so the renderer can retry', () => { + const store = { + setWorkspaceSession: vi.fn(), + updateUI: vi.fn(), + flushOrThrow: vi.fn(() => { + throw new Error('disk full') + }), + flushActiveViewPreferenceOrThrow: vi.fn() + } + registerRendererShutdownCheckpointHandler(store as never) + + const handler = syncHandlers.get('app:persist-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). + const store = { + setWorkspaceSession: vi.fn(), + updateUI: vi.fn(), + flushOrThrow: vi.fn(() => { + throw new Error('disk full') + }), + flushActiveViewPreferenceOrThrow: vi.fn() + } + registerRendererShutdownCheckpointHandler(store as never) + + const handler = syncHandlers.get('app:persist-before-unload-sync') + const event: { returnValue?: unknown } = {} + handler?.(event, { sessions: [], ui: { activeView: 'settings' } }) + + expect(store.flushActiveViewPreferenceOrThrow).toHaveBeenCalledTimes(1) + expect(event.returnValue).toEqual({ ok: false }) + }) + + it('flushes the durable store even when the active-view flush throws', () => { + const store = { + setWorkspaceSession: vi.fn(), + updateUI: vi.fn(), + flushOrThrow: vi.fn(), + flushActiveViewPreferenceOrThrow: vi.fn(() => { + throw new Error('disk full') + }) + } + registerRendererShutdownCheckpointHandler(store as never) + + const handler = syncHandlers.get('app:persist-before-unload-sync') + const event: { returnValue?: unknown } = {} + handler?.(event, { sessions: [], ui: { activeView: 'settings' } }) + + expect(store.flushOrThrow).toHaveBeenCalledTimes(1) + expect(event.returnValue).toEqual({ ok: false }) + }) +}) diff --git a/src/main/ipc/renderer-shutdown-checkpoint.ts b/src/main/ipc/renderer-shutdown-checkpoint.ts new file mode 100644 index 000000000..d268d388d --- /dev/null +++ b/src/main/ipc/renderer-shutdown-checkpoint.ts @@ -0,0 +1,41 @@ +import { ipcMain } from 'electron' +import type { ExecutionHostId } from '../../shared/execution-host' +import type { PersistedUIState, WorkspaceSessionState } from '../../shared/types' +import type { Store } from '../persistence' + +type PersistBeforeUnloadSyncArgs = { + sessions: { state: WorkspaceSessionState; hostId?: ExecutionHostId }[] + ui: Partial +} + +export function registerRendererShutdownCheckpointHandler(store: Store): void { + ipcMain.on('app:persist-before-unload-sync', (event, args: PersistBeforeUnloadSyncArgs) => { + 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.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 + } + event.returnValue = { ok } + }) +} diff --git a/src/main/persistence.ts b/src/main/persistence.ts index e8ae1a7bd..381076305 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -217,6 +217,7 @@ import { normalizeTerminalLineHeight } from '../shared/terminal-line-height-sett import { normalizeUiLanguage } from '../shared/ui-language' import { normalizeBrowserPageZoomLevel } from '../shared/browser-page-zoom' import { persistedUIValuesEqual } from '../shared/persisted-ui-equality' +import { ActiveViewPreference } from './active-view-preference' import { normalizeFolderWorkspaceName, normalizeFolderWorkspaces @@ -2627,6 +2628,7 @@ export type StoreOptions = { export class Store { private state: PersistedState private readonly dataFile: string + private readonly activeViewPreference: ActiveViewPreference private readonly terminalScrollbackSnapshotStorage: TerminalScrollbackSnapshotStorage private writeTimer: ReturnType | null = null private pendingWrite: Promise | null = null @@ -2667,6 +2669,9 @@ export class Store { const loaded = this.load() const normalized = normalizePersistedPaneIdentityState(loaded) this.state = normalized.state + // Why: activeView is a frequent, tiny preference; keeping it beside the + // profile avoids serializing the multi-MB recovery store on navigation. + this.activeViewPreference = new ActiveViewPreference(this.dataFile, this.state.ui?.activeView) const adaptedProjectGroups = this.adaptFlatFolderScanProjectGroups() for (const entry of normalized.migrationUnsupportedEntries) { setMigrationUnsupportedPty(entry) @@ -3698,9 +3703,7 @@ export class Store { /** Wait for any in-flight async disk write to complete. Used in tests. */ async waitForPendingWrite(): Promise { - if (this.pendingWrite) { - await this.pendingWrite - } + await Promise.all([this.pendingWrite, this.activeViewPreference.waitForPendingWrite()]) } // Why githubCache is omitted: it is memory-only during the session (see @@ -3857,6 +3860,10 @@ export class Store { this.writeToDiskSync({ force: asyncWriteWasInFlight }) } + flushActiveViewPreferenceOrThrow(): void { + this.activeViewPreference.flushOrThrow() + } + // ── Repos ────────────────────────────────────────────────────────── getRepos(): Repo[] { @@ -5490,17 +5497,31 @@ export class Store { ), featureTipsSeenIds: normalizeFeatureTipIds(this.state.ui?.featureTipsSeenIds), contextualToursSeenIds: normalizeContextualTourIds(this.state.ui?.contextualToursSeenIds), - featureInteractions: normalizeFeatureInteractions(this.state.ui?.featureInteractions) + featureInteractions: normalizeFeatureInteractions(this.state.ui?.featureInteractions), + activeView: this.activeViewPreference.get() } } updateUI(updates: Partial): void { const sanitizedUpdates = stripMainOwnedTelemetryMarkerFromUI(updates) - const previousUI = this.getUI() + const { activeView, ...durableUpdates } = sanitizedUpdates + const activeViewChanged = this.activeViewPreference.set(activeView) + if (Object.keys(durableUpdates).length === 0) { + if (activeViewChanged) { + this.notifyUIChanged() + } + return + } const currentUI = { ...getDefaultUIState(), ...stripMainOwnedTelemetryMarkerFromUI(this.state.ui) } + const previousUI = { + ...this.getUI(), + // Why: the legacy field stays unchanged as a migration/downgrade + // fallback; the profile sidecar is authoritative in current builds. + activeView: currentUI.activeView + } const nextRightSidebarTab = sanitizedUpdates.rightSidebarTab !== undefined ? normalizeRightSidebarTab(sanitizedUpdates.rightSidebarTab) @@ -5519,16 +5540,17 @@ export class Store { ) const nextUI = { ...currentUI, - ...sanitizedUpdates, - groupBy: sanitizedUpdates.groupBy - ? normalizeGroupBy(sanitizedUpdates.groupBy) + ...durableUpdates, + groupBy: durableUpdates.groupBy + ? normalizeGroupBy(durableUpdates.groupBy) : normalizeGroupBy(this.state.ui?.groupBy), - sortBy: sanitizedUpdates.sortBy - ? normalizeSortBy(sanitizedUpdates.sortBy) + sortBy: durableUpdates.sortBy + ? normalizeSortBy(durableUpdates.sortBy) : normalizeSortBy(this.state.ui?.sortBy), projectOrderBy: updates.projectOrderBy ? normalizeProjectOrderBy(updates.projectOrderBy) : normalizeProjectOrderBy(this.state.ui?.projectOrderBy), + activeView: currentUI.activeView, rightSidebarTab: nextRightSidebarTab, rightSidebarExplorerView: nextRightSidebarExplorerView, worktreeCardProperties: @@ -5603,6 +5625,9 @@ export class Store { : normalizeFeatureInteractions(this.state.ui?.featureInteractions) } if (persistedUIValuesEqual(previousUI, nextUI)) { + if (activeViewChanged) { + this.notifyUIChanged() + } return } this.state.ui = nextUI @@ -6595,6 +6620,11 @@ export class Store { } catch (err) { console.error('[persistence] Failed to flush state:', err) } + try { + this.flushActiveViewPreferenceOrThrow() + } catch (err) { + console.error('[active-view] Failed to flush preference:', err) + } this.writeGithubCacheSnapshotSync() } diff --git a/src/main/window/createMainWindow.test.ts b/src/main/window/createMainWindow.test.ts index 3b69b9c12..5b84fd1f0 100644 --- a/src/main/window/createMainWindow.test.ts +++ b/src/main/window/createMainWindow.test.ts @@ -1484,6 +1484,7 @@ describe('createMainWindow', () => { windowHandlers['will-prevent-unload']() expect(onQuitAborted).toHaveBeenCalledTimes(1) + expect(webContents.send).toHaveBeenCalledWith('window:unload-prevented') }) it('allows close after the renderer process is gone', () => { diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index b12583377..c07ed4554 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -1148,6 +1148,7 @@ export function createMainWindow( // aborting Cmd+Q still gets their size saved. windowClosing = false opts?.onQuitAborted?.() + mainWindow.webContents.send('window:unload-prevented') }) const onConfirmClose = (): void => { diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 487f7b2e0..2e0d86ab3 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -928,6 +928,12 @@ 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: { + sessions: { state: WorkspaceSessionState; hostId?: ExecutionHostId }[] + ui: Partial + }) => void /** Resolves when the daemon PTY provider and hook receiver have either * started or failed open for the first BrowserWindow. */ awaitFirstWindowStartupServices: () => Promise diff --git a/src/preload/index.ts b/src/preload/index.ts index 5571f9349..da24ff26a 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -184,16 +184,13 @@ import type { NativeChatReadSessionResult, NativeChatSubscriptionFrame } from './api-types' -import { - ORCA_EDITOR_PREPARE_HOT_EXIT_EVENT, - type EditorPrepareHotExitDetail -} from '../shared/editor-save-events' import { ORCA_APP_RESTART_ABORTED_EVENT, ORCA_APP_RESTART_STARTED_EVENT, ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT } from '../shared/updater-renderer-events' +import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../shared/renderer-shutdown-events' import { ORCA_INTERNAL_FILE_DRAG_TYPE, createNativeFileDropPayload, @@ -228,11 +225,26 @@ import type { ReactErrorBoundaryReportResult } from '../shared/crash-reporting' import type { PreloadApi } from './api-types' +import { + createUpdaterQuitAbortRelay, + prepareRendererForAppRestart +} from './renderer-restart-preparation' type NativeFileDropCallback = (data: NativeFileDropPayload) => void const nativeFileDropCallbacks: NativeFileDropCallback[] = [] let nativeFileDropListenerRegistered = false +const updaterQuitAbortRelay = createUpdaterQuitAbortRelay( + window, + ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT +) + +ipcRenderer.on('updater:status', (_event, status: UpdateStatus) => { + updaterQuitAbortRelay.handleStatus(status) +}) +ipcRenderer.on('window:unload-prevented', () => { + window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT)) +}) function getLinuxDisplayServer(): 'wayland' | 'x11' | null { if (process.platform !== 'linux') { @@ -248,54 +260,6 @@ function getLinuxDisplayServer(): 'wayland' | 'x11' | null { return process.env.DISPLAY ? 'x11' : null } -type AppRestartPrepOptions = { - startedEventName: string - abortedEventName: string -} - -function requestEditorHotExitBackup(): Promise { - return new Promise((resolve, reject) => { - let claimed = false - window.dispatchEvent( - new CustomEvent(ORCA_EDITOR_PREPARE_HOT_EXIT_EVENT, { - detail: { - claim: () => { - claimed = true - }, - resolve, - reject: (message) => { - reject(new Error(message)) - } - } - }) - ) - - // Why: restart paths can run before the editor autosave controller mounts. - // With no claimant, there are no renderer-owned dirty buffers to back up. - if (!claimed) { - resolve() - } - }) -} - -async function prepareRendererForAppRestart({ - startedEventName, - abortedEventName -}: AppRestartPrepOptions): Promise { - window.dispatchEvent(new Event(startedEventName)) - - try { - await requestEditorHotExitBackup() - } catch (error) { - window.dispatchEvent(new Event(abortedEventName)) - throw error - } - - // Dispatch beforeunload now so terminal buffers are captured while panes are - // still mounted; update installs later bypass the ordinary close sequence. - window.dispatchEvent(new Event('beforeunload')) -} - const onNativeFileDrop = (_event: Electron.IpcRendererEvent, data: NativeFileDropPayload): void => { for (const callback of Array.from(nativeFileDropCallbacks)) { callback(data) @@ -474,7 +438,7 @@ const api = { ipcRenderer.invoke('app:getFeatureWallAssetBaseUrl'), relaunch: (): Promise => ipcRenderer.invoke('app:relaunch'), restart: async (): Promise => { - await prepareRendererForAppRestart({ + await prepareRendererForAppRestart(window, { startedEventName: ORCA_APP_RESTART_STARTED_EVENT, abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT }) @@ -486,6 +450,16 @@ const api = { } }, reload: (): Promise => ipcRenderer.invoke('app:reload'), + persistBeforeUnloadSync: ( + args: Parameters[0] + ) => { + const result = ipcRenderer.sendSync('app:persist-before-unload-sync', args) as { + ok?: unknown + } + if (result?.ok !== true) { + throw new Error('Failed to persist renderer state before unload.') + } + }, awaitFirstWindowStartupServices: (): Promise => ipcRenderer.invoke('app:awaitFirstWindowStartupServices'), startupDiagnostic: (event: string, details?: Record): Promise => @@ -2766,14 +2740,15 @@ const api = { download: () => ipcRenderer.invoke('updater:download'), dismissNudge: () => ipcRenderer.invoke('updater:dismissNudge'), quitAndInstall: async (): Promise => { - await prepareRendererForAppRestart({ + await prepareRendererForAppRestart(window, { startedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, abortedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT }) + updaterQuitAbortRelay.markPrepared() try { return await ipcRenderer.invoke('updater:quitAndInstall') } catch (error) { - window.dispatchEvent(new Event(ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT)) + updaterQuitAbortRelay.abort() throw error } }, diff --git a/src/preload/renderer-restart-preparation.test.ts b/src/preload/renderer-restart-preparation.test.ts new file mode 100644 index 000000000..c216ef93f --- /dev/null +++ b/src/preload/renderer-restart-preparation.test.ts @@ -0,0 +1,90 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import type { UpdateStatus } from '../shared/types' +import { + createUpdaterQuitAbortRelay, + prepareRendererForAppRestart +} from './renderer-restart-preparation' + +describe('prepareRendererForAppRestart', () => { + it('aborts when the dispatched shutdown checkpoint prevents unload', async () => { + const eventTarget = new EventTarget() + const started = vi.fn() + const aborted = vi.fn() + const checkpoint = vi.fn((event: Event) => event.preventDefault()) + eventTarget.addEventListener('restart-started', started) + eventTarget.addEventListener('restart-aborted', aborted) + eventTarget.addEventListener('beforeunload', checkpoint) + + await expect( + prepareRendererForAppRestart(eventTarget, { + startedEventName: 'restart-started', + abortedEventName: 'restart-aborted' + }) + ).rejects.toThrow('Renderer shutdown checkpoint was not completed.') + + expect(started).toHaveBeenCalledTimes(1) + expect(checkpoint).toHaveBeenCalledTimes(1) + expect(aborted).toHaveBeenCalledTimes(1) + }) +}) + +describe('createUpdaterQuitAbortRelay', () => { + it('resets a prepared update restart when async updater status reports failure', () => { + const eventTarget = new EventTarget() + const aborted = vi.fn() + eventTarget.addEventListener('update-restart-aborted', aborted) + const relay = createUpdaterQuitAbortRelay(eventTarget, 'update-restart-aborted') + relay.markPrepared() + + relay.handleStatus({ state: 'error', message: 'install failed' } satisfies UpdateStatus) + relay.handleStatus({ state: 'error', message: 'duplicate failure' } satisfies UpdateStatus) + + expect(aborted).toHaveBeenCalledTimes(1) + }) + + it('ignores updater errors when no update restart was prepared', () => { + const eventTarget = new EventTarget() + const aborted = vi.fn() + eventTarget.addEventListener('update-restart-aborted', aborted) + const relay = createUpdaterQuitAbortRelay(eventTarget, 'update-restart-aborted') + + relay.handleStatus({ state: 'error', message: 'check failed' } satisfies UpdateStatus) + + expect(aborted).not.toHaveBeenCalled() + }) +}) + +describe('preload restart wiring', () => { + const source = readFileSync(join(process.cwd(), 'src/preload/index.ts'), 'utf8') + + it('relays prevented unload and async updater failure IPC into renderer lifecycle events', () => { + expect(source).toContain("ipcRenderer.on('updater:status'") + expect(source).toContain('updaterQuitAbortRelay.handleStatus(status)') + expect(source).toContain("ipcRenderer.on('window:unload-prevented'") + expect(source).toContain( + 'window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT))' + ) + }) + + it('marks updater preparation before invoking main and aborts it on immediate IPC failure', () => { + const start = source.indexOf('quitAndInstall: async (): Promise => {') + const end = source.indexOf('onStatus: (callback) => {', start) + expect(start).toBeGreaterThanOrEqual(0) + expect(end).toBeGreaterThan(start) + const block = source.slice(start, end) + const prepare = block.indexOf('await prepareRendererForAppRestart(window, {') + const markPrepared = block.indexOf('updaterQuitAbortRelay.markPrepared()') + const invoke = block.indexOf("ipcRenderer.invoke('updater:quitAndInstall')") + const abort = block.indexOf('updaterQuitAbortRelay.abort()') + + expect(prepare).toBeGreaterThanOrEqual(0) + expect(markPrepared).toBeGreaterThan(prepare) + expect(invoke).toBeGreaterThan(markPrepared) + expect(abort).toBeGreaterThan(invoke) + expect(block).toMatch( + /try \{\s*return await ipcRenderer\.invoke\('updater:quitAndInstall'\)\s*\} catch \(error\) \{\s*updaterQuitAbortRelay\.abort\(\)\s*throw error\s*\}/ + ) + }) +}) diff --git a/src/preload/renderer-restart-preparation.ts b/src/preload/renderer-restart-preparation.ts new file mode 100644 index 000000000..4f526844b --- /dev/null +++ b/src/preload/renderer-restart-preparation.ts @@ -0,0 +1,89 @@ +import { + ORCA_EDITOR_PREPARE_HOT_EXIT_EVENT, + type EditorPrepareHotExitDetail +} from '../shared/editor-save-events' +import type { UpdateStatus } from '../shared/types' + +export type AppRestartPrepOptions = { + startedEventName: string + abortedEventName: string +} + +function requestEditorHotExitBackup(eventTarget: EventTarget): Promise { + return new Promise((resolve, reject) => { + let claimed = false + eventTarget.dispatchEvent( + new CustomEvent(ORCA_EDITOR_PREPARE_HOT_EXIT_EVENT, { + detail: { + claim: () => { + claimed = true + }, + resolve, + reject: (message) => { + reject(new Error(message)) + } + } + }) + ) + + // Why: restart paths can run before the editor autosave controller mounts. + // With no claimant, there are no renderer-owned dirty buffers to back up. + if (!claimed) { + resolve() + } + }) +} + +export async function prepareRendererForAppRestart( + eventTarget: EventTarget, + { startedEventName, abortedEventName }: AppRestartPrepOptions +): Promise { + eventTarget.dispatchEvent(new Event(startedEventName)) + + try { + await requestEditorHotExitBackup(eventTarget) + // Why: update installs can bypass native close. A cancelable synthetic + // unload both captures mounted terminals and reports checkpoint failure. + const accepted = eventTarget.dispatchEvent(new Event('beforeunload', { cancelable: true })) + if (!accepted) { + throw new Error('Renderer shutdown checkpoint was not completed.') + } + } catch (error) { + eventTarget.dispatchEvent(new Event(abortedEventName)) + throw error + } +} + +export type UpdaterQuitAbortRelay = { + markPrepared: () => void + abort: () => void + handleStatus: (status: UpdateStatus) => void +} + +export function createUpdaterQuitAbortRelay( + eventTarget: EventTarget, + abortedEventName: string +): UpdaterQuitAbortRelay { + let prepared = false + const abort = (): void => { + if (!prepared) { + return + } + prepared = false + eventTarget.dispatchEvent(new Event(abortedEventName)) + } + + return { + markPrepared(): void { + prepared = true + }, + abort, + handleStatus(status): void { + // Why: quitAndInstall IPC resolves after scheduling; a later updater + // error is the authoritative signal that the app will remain open. + if (status.state === 'error') { + abort() + } + } + } +} diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 6a150add5..977b32d30 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -110,16 +110,26 @@ import { useWebSessionTabsSync } from './runtime/web-session-tabs-sync' import { useGlobalFileDrop } from './hooks/useGlobalFileDrop' import { useRadixBodyPointerEventsRecovery } from './hooks/useRadixBodyPointerEventsRecovery' import { registerUpdaterBeforeUnloadBypass } from './lib/updater-beforeunload' +import { + ORCA_APP_RESTART_ABORTED_EVENT, + ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT +} from '../../shared/updater-renderer-events' +import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../../shared/renderer-shutdown-events' import { buildWorkspaceSessionPayload, shouldPersistWorkspaceSession } from './lib/workspace-session' import { createSessionWriteSubscriber } from './lib/session-write-subscriber' +import { buildActiveViewUnloadPatch } from './lib/active-view-persist' import { + buildWorkspaceSessionHostSnapshots, fetchWorkspaceSessionWithRuntimeHostOwners, - patchWorkspaceSessionByHost, - persistWorkspaceSessionByHostSync + patchWorkspaceSessionByHost } from './lib/workspace-session-host-persistence' +import { + createShutdownCheckpointBeforeUnloadHandler, + createShutdownCheckpointGuard +} from './lib/shutdown-checkpoint-guard' import { collectFolderWorkspaceKeysFromSession } from './lib/workspace-session-hydration-keys' import { getStartupErrorFallbackUI, @@ -1342,8 +1352,8 @@ function App(): React.JSX.Element { }) }, []) - // On shutdown, capture terminal scrollback buffers and flush to disk. - // Runs synchronously in beforeunload: capture → Zustand set → sendSync → flush. + // On shutdown, capture terminal scrollback buffers and flush all durable + // renderer state through one synchronous main-process checkpoint. useEffect(() => { // Why: beforeunload fires twice during a manual quit — once from the // synthetic dispatch in the onWindowCloseRequested handler (captures @@ -1352,39 +1362,50 @@ function App(): React.JSX.Element { // two firings, PTY exit events can arrive and unmount TerminalPanes, // emptying shutdownBufferCaptures. The guard prevents the second call // from overwriting the good session data with an empty snapshot. - let shutdownBuffersCaptured = false - const captureAndFlush = (): void => { - if (shutdownBuffersCaptured) { - return - } - if (!shouldPersistWorkspaceSession(useAppStore.getState())) { - return - } - for (const capture of shutdownBufferCaptures.values()) { - try { - capture({ includeLocalBuffers: false }) - } catch { - // Don't let one pane's failure block the rest. + const shutdownCheckpoint = createShutdownCheckpointGuard(() => { + const shouldCaptureSession = shouldPersistWorkspaceSession(useAppStore.getState()) + if (shouldCaptureSession) { + for (const capture of shutdownBufferCaptures.values()) { + try { + capture({ includeLocalBuffers: false }) + } catch { + // Don't let one pane's failure block the rest. + } } + // Why: agent provider session ids live only in agentStatusByPaneKey, + // which is in-memory. Capture them into the persisted sleeping-session + // map so a daemon/session death while the app is closed can still + // cold-restore via the agent's resume command (#5232). + useAppStore.getState().captureAllSleepingAgentSessions('quit') } - // Why: agent provider session ids live only in agentStatusByPaneKey, - // which is in-memory. Capture them into the persisted sleeping-session - // map so a daemon/session death while the app is closed can still - // cold-restore via the agent's resume command (#5232). - useAppStore.getState().captureAllSleepingAgentSessions('quit') // Why: re-read state after capture() calls populated scrollback buffers // into the store via Zustand setters. The earlier read is only for the // gating flags and would miss those updates. const freshState = useAppStore.getState() - persistWorkspaceSessionByHostSync( - window.api.session, - buildWorkspaceSessionPayload(freshState), - freshState + 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({ + sessions: sessionSnapshots, + ui: buildActiveViewUnloadPatch(freshState) + }) + }) + const persistBeforeUnload = createShutdownCheckpointBeforeUnloadHandler(shutdownCheckpoint) + window.addEventListener('beforeunload', persistBeforeUnload) + window.addEventListener(ORCA_APP_RESTART_ABORTED_EVENT, shutdownCheckpoint.reset) + window.addEventListener(ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, shutdownCheckpoint.reset) + window.addEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, shutdownCheckpoint.reset) + return () => { + window.removeEventListener('beforeunload', persistBeforeUnload) + window.removeEventListener(ORCA_APP_RESTART_ABORTED_EVENT, shutdownCheckpoint.reset) + window.removeEventListener( + ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, + shutdownCheckpoint.reset ) - shutdownBuffersCaptured = true + window.removeEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, shutdownCheckpoint.reset) } - window.addEventListener('beforeunload', captureAndFlush) - return () => window.removeEventListener('beforeunload', captureAndFlush) }, []) // Why: beforeunload never fires on a hard kill (crash, forced update @@ -1448,10 +1469,10 @@ function App(): React.JSX.Element { hideAutomationGeneratedWorkspaces, showDotfilesByWorktree, filterRepoIds, - // Why: persist the active view so a reload restores it. openTaskPage etc. - // mutate activeView directly (not via setActiveView), so the value-keyed - // writer is what catches every transition. - activeView, + // Why (#9002): activeView is deliberately NOT included here. It used to + // ride this same 150ms writer (#8265), which meant every top-level view + // switch scheduled a full durable-state save. The narrow preference + // effect below persists it without touching the recovery snapshot. // Why: rides the same debounced save so dashboard auto-acks (which fire // on focus/visibility) and the in-memory ack cleanup paths in // agent-status.ts (close/dismiss) both flow to disk through map @@ -1478,10 +1499,18 @@ function App(): React.JSX.Element { hideAutomationGeneratedWorkspaces, showDotfilesByWorktree, filterRepoIds, - activeView, acknowledgedAgentsByPaneKey ]) + // Why (#9002): activeView has its own tiny profile preference, so it can track + // every switch without scheduling the multi-MB durable-state writer. + useEffect(() => { + if (!persistedUIReady) { + return + } + void window.api.ui.set({ activeView }) + }, [activeView, persistedUIReady]) + // Apply theme to document useEffect(() => { if (!settings) { diff --git a/src/renderer/src/app-startup-routing.test.ts b/src/renderer/src/app-startup-routing.test.ts index 119ac5f77..62f9040d3 100644 --- a/src/renderer/src/app-startup-routing.test.ts +++ b/src/renderer/src/app-startup-routing.test.ts @@ -254,4 +254,71 @@ describe('renderer startup runtime routing', () => { expect(source).toContain('statusBarVisible ? (') expect(source).toContain('h-6 min-h-[24px] shrink-0 border-t border-border') }) + + it('keeps activeView off the 150ms debounced UI writer hot path (#9002)', () => { + const source = readFileSync(join(process.cwd(), 'src/renderer/src/App.tsx'), 'utf8') + const writerStart = source.indexOf('const timer = window.setTimeout(() => {') + const writerEnd = source.indexOf('}, 150)', writerStart) + const writerBlock = source.slice(writerStart, writerEnd) + + expect(writerStart).toBeGreaterThanOrEqual(0) + expect(writerEnd).toBeGreaterThan(writerStart) + // Why: this field riding the writer's payload (#8265) is exactly the + // #9002 regression — every switch scheduled a full durable-state save. It + // must persist through its narrow preference or unload path instead. Matched as + // a standalone object-literal property (not the surrounding prose, which + // legitimately references the field name) so the assertion is precise. + expect(writerBlock).not.toMatch(/^\s*activeView,\s*$/m) + + const depsStart = source.indexOf('}, [', writerEnd) + const depsEnd = source.indexOf('])', depsStart) + const depsBlock = source.slice(depsStart, depsEnd) + expect(depsBlock).not.toMatch(/^\s*activeView,?\s*$/m) + }) + + it('persists activeView through its narrow preference on every switch (#9002)', () => { + const source = readFileSync(join(process.cwd(), 'src/renderer/src/App.tsx'), 'utf8') + + const preferenceEffect = [ + '// Why (#9002): activeView has its own tiny profile preference', + 'void window.api.ui.set({ activeView })', + '}, [activeView, persistedUIReady])' + ] + for (const marker of preferenceEffect) { + expect(source).toContain(marker) + } + expect(source).not.toContain('createActiveViewIdleFlush') + expect(source).not.toContain("window.addEventListener('blur', handleBlur)") + }) + + it('checkpoints activeView and all session snapshots through one beforeunload handler (#9002)', () => { + const source = readFileSync(join(process.cwd(), 'src/renderer/src/App.tsx'), 'utf8') + const checkpointStart = source.indexOf( + 'const shutdownCheckpoint = createShutdownCheckpointGuard(() => {' + ) + const checkpointEnd = source.indexOf( + 'const persistBeforeUnload = createShutdownCheckpointBeforeUnloadHandler(shutdownCheckpoint)', + checkpointStart + ) + expect(checkpointStart).toBeGreaterThanOrEqual(0) + expect(checkpointEnd).toBeGreaterThan(checkpointStart) + const checkpointBlock = source.slice(checkpointStart, checkpointEnd) + + expect(checkpointBlock).toContain('const sessionSnapshots = shouldCaptureSession') + expect(checkpointBlock).toContain( + 'buildWorkspaceSessionHostSnapshots(buildWorkspaceSessionPayload(freshState), freshState)' + ) + expect(checkpointBlock).toContain('window.api.app.persistBeforeUnloadSync({') + expect(checkpointBlock).toContain('sessions: sessionSnapshots') + expect(checkpointBlock).toContain('ui: buildActiveViewUnloadPatch(freshState)') + expect(source).toContain( + 'window.addEventListener(ORCA_APP_RESTART_ABORTED_EVENT, shutdownCheckpoint.reset)' + ) + expect(source).toContain( + 'window.addEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, shutdownCheckpoint.reset)' + ) + expect(source).toContain("window.addEventListener('beforeunload', persistBeforeUnload)") + expect(source.match(/window\.addEventListener\('beforeunload'/g) ?? []).toHaveLength(1) + expect(source).not.toContain('window.api.ui.setSync') + }) }) diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index 14df288fd..5c2b0830a 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -34,6 +34,7 @@ import { requestEditorSaveQuiesce } from './editor/editor-autosave' import { isIntentionalAppRestartInProgress } from '@/lib/updater-beforeunload' +import { preventUnloadAndScheduleShutdownCheckpointReset } from '@/lib/shutdown-checkpoint-guard' import EditorAutosaveController from './editor/EditorAutosaveController' import type { Tab, TabContentType, TabGroupLayoutNode, TuiAgent } from '../../../shared/types' import { hasFeatureInteraction } from '../../../shared/feature-interactions' @@ -448,40 +449,49 @@ function Terminal(): React.JSX.Element | null { // dirty, so an immediate confirm would leave the window open with no UI. const windowCloseAfterDirtyRef = useRef<{ isQuitting: boolean } | null>(null) - const proceedToNativeWindowClose = useCallback((isQuitting: boolean) => { - // Why: defer this synthetic unload until we are actually ready to close so - // a dirty-tab preventDefault() does not fire during the initial quit IPC - // (that path can emit will-prevent-unload and clear isQuitting in main). - window.dispatchEvent(new Event('beforeunload')) - if (!isQuitting) { - const state = useAppStore.getState() - const localPtyIds = Object.entries(state.tabsByWorktree).flatMap( - ([worktreeId, worktreeTabs]) => { - const connectionId = getConnectionId(worktreeId) - if (connectionId !== null) { - return [] - } - return worktreeTabs - .flatMap((tab) => state.ptyIdsByTabId[tab.id] ?? []) - .filter((ptyId) => !isRemoteRuntimePtyId(ptyId)) - } - ) - if (localPtyIds.length > 0) { - void Promise.all(localPtyIds.map((id) => window.api.pty.hasChildProcesses(id))).then( - (results) => { - if (results.some(Boolean)) { - setWindowCloseDialogOpen(true) - } else { - window.api.ui.confirmWindowClose() - } - } - ) - return - } + const confirmNativeWindowClose = useCallback(() => { + // Why: capture only after every close guard has committed. A canceled child- + // process prompt must not consume App's synthetic/native unload guard. + const accepted = window.dispatchEvent(new Event('beforeunload', { cancelable: true })) + if (!accepted) { + return } window.api.ui.confirmWindowClose() }, []) + const proceedToNativeWindowClose = useCallback( + (isQuitting: boolean) => { + if (!isQuitting) { + const state = useAppStore.getState() + const localPtyIds = Object.entries(state.tabsByWorktree).flatMap( + ([worktreeId, worktreeTabs]) => { + const connectionId = getConnectionId(worktreeId) + if (connectionId !== null) { + return [] + } + return worktreeTabs + .flatMap((tab) => state.ptyIdsByTabId[tab.id] ?? []) + .filter((ptyId) => !isRemoteRuntimePtyId(ptyId)) + } + ) + if (localPtyIds.length > 0) { + void Promise.all(localPtyIds.map((id) => window.api.pty.hasChildProcesses(id))).then( + (results) => { + if (results.some(Boolean)) { + setWindowCloseDialogOpen(true) + } else { + confirmNativeWindowClose() + } + } + ) + return + } + } + confirmNativeWindowClose() + }, + [confirmNativeWindowClose] + ) + const waitForFileClosed = useCallback((fileId: string, timeoutMs: number): Promise => { if (!useAppStore.getState().openFiles.some((f) => f.id === fileId)) { return Promise.resolve(true) @@ -2061,7 +2071,7 @@ function Terminal(): React.JSX.Element | null { } const dirtyFiles = useAppStore.getState().openFiles.filter((f) => f.isDirty) if (dirtyFiles.length > 0) { - e.preventDefault() + preventUnloadAndScheduleShutdownCheckpointReset(e, window) } } window.addEventListener('beforeunload', handler) @@ -2545,7 +2555,7 @@ function Terminal(): React.JSX.Element | null { autoFocus onClick={() => { setWindowCloseDialogOpen(false) - window.api.ui.confirmWindowClose() + confirmNativeWindowClose() }} > {translate('auto.components.Terminal.73768427cf', 'Close')} diff --git a/src/renderer/src/components/editor/editor-autosave-controller.test.ts b/src/renderer/src/components/editor/editor-autosave-controller.test.ts index 9d50997d9..70abf77c6 100644 --- a/src/renderer/src/components/editor/editor-autosave-controller.test.ts +++ b/src/renderer/src/components/editor/editor-autosave-controller.test.ts @@ -384,7 +384,7 @@ describe('attachEditorAutosaveController', () => { } }) - it('backs up dirty editor drafts for hot exit without writing files', async () => { + it('leaves dirty editor drafts ready for the combined hot-exit checkpoint', async () => { const writeFile = vi.fn().mockResolvedValue(undefined) const setSync = vi.fn() const eventTarget = new EventTarget() @@ -422,13 +422,7 @@ describe('attachEditorAutosaveController', () => { await vi.advanceTimersByTimeAsync(1000) expect(writeFile).not.toHaveBeenCalled() - expect(setSync).toHaveBeenCalledTimes(1) - expect(setSync.mock.calls[0][0].openFilesByWorktree['wt-1'][0]).toEqual( - expect.objectContaining({ - filePath: '/repo/file.md', - dirtyDraftContent: '' - }) - ) + expect(setSync).not.toHaveBeenCalled() expect(store.getState().openFiles[0]?.isDirty).toBe(true) expect(store.getState().editorDrafts['/repo/file.md']).toBe('') } finally { diff --git a/src/renderer/src/components/editor/editor-autosave-controller.ts b/src/renderer/src/components/editor/editor-autosave-controller.ts index 8c8e7ec16..09763e148 100644 --- a/src/renderer/src/components/editor/editor-autosave-controller.ts +++ b/src/renderer/src/components/editor/editor-autosave-controller.ts @@ -5,11 +5,7 @@ import type { StoreApi } from 'zustand' import type { AppState } from '@/store' import type { OpenFile } from '@/store/slices/editor' import { getConnectionIdForFile } from '@/lib/connection-context' -import { - buildWorkspaceSessionPayload, - shouldPersistWorkspaceSession -} from '@/lib/workspace-session' -import { persistWorkspaceSessionByHostSync } from '@/lib/workspace-session-host-persistence' +import { shouldPersistWorkspaceSession } from '@/lib/workspace-session' import { findWorktreeById } from '@/store/slices/worktree-helpers' import { writeRuntimeFile } from '@/runtime/runtime-file-client' import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' @@ -340,17 +336,8 @@ export function attachEditorAutosaveController(store: AppStoreApi): () => void { return } - // Why: restart/update may quit before the debounced session writer fires. - // Write the full session now so dirty drafts restore as unsaved tabs. - if (shouldPersistWorkspaceSession(state)) { - // Why: runtime-owned worktree slices persist under their host - // partition, mirroring the debounced writer's split. - persistWorkspaceSessionByHostSync( - window.api.session, - buildWorkspaceSessionPayload(state), - state - ) - } + // Why: preload dispatches beforeunload immediately after this resolves; + // App owns the one combined session/UI checkpoint for restart and update. detail.resolve() } catch (error) { detail.reject(String((error as Error)?.message ?? error)) diff --git a/src/renderer/src/lib/active-view-persist.test.ts b/src/renderer/src/lib/active-view-persist.test.ts new file mode 100644 index 000000000..37df61d2e --- /dev/null +++ b/src/renderer/src/lib/active-view-persist.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from 'vitest' +import { buildActiveViewUnloadPatch } from './active-view-persist' + +describe('buildActiveViewUnloadPatch', () => { + it('does not overwrite persisted UI before startup hydration completes', () => { + expect(buildActiveViewUnloadPatch({ activeView: 'terminal', persistedUIReady: false })).toEqual( + {} + ) + }) + + it('checkpoints the latest view after startup hydration completes', () => { + expect(buildActiveViewUnloadPatch({ activeView: 'tasks', persistedUIReady: true })).toEqual({ + activeView: 'tasks' + }) + }) +}) diff --git a/src/renderer/src/lib/active-view-persist.ts b/src/renderer/src/lib/active-view-persist.ts new file mode 100644 index 000000000..79f965b33 --- /dev/null +++ b/src/renderer/src/lib/active-view-persist.ts @@ -0,0 +1,14 @@ +import type { PersistedUIState, TopLevelView } from '../../../shared/types' + +type ActiveViewUnloadState = { + activeView: TopLevelView + persistedUIReady: boolean +} + +export function buildActiveViewUnloadPatch( + state: ActiveViewUnloadState +): Partial { + // Why: unloading during startup must not overwrite the saved view with the + // renderer default before persisted UI hydration finishes. + return state.persistedUIReady ? { activeView: state.activeView } : {} +} diff --git a/src/renderer/src/lib/shutdown-checkpoint-guard.test.ts b/src/renderer/src/lib/shutdown-checkpoint-guard.test.ts new file mode 100644 index 000000000..516b60354 --- /dev/null +++ b/src/renderer/src/lib/shutdown-checkpoint-guard.test.ts @@ -0,0 +1,111 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import { + createShutdownCheckpointBeforeUnloadHandler, + createShutdownCheckpointGuard, + preventUnloadAndScheduleShutdownCheckpointReset +} from './shutdown-checkpoint-guard' +import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../../../shared/renderer-shutdown-events' + +describe('createShutdownCheckpointGuard', () => { + it('dedupes the synthetic and native unload events in one close attempt', () => { + const persist = vi.fn() + const guard = createShutdownCheckpointGuard(persist) + + expect(guard.persistOnce()).toBe(true) + expect(guard.persistOnce()).toBe(true) + + expect(persist).toHaveBeenCalledTimes(1) + }) + + it('allows a new checkpoint after an aborted restart resets the attempt', () => { + const persist = vi.fn() + const guard = createShutdownCheckpointGuard(persist) + + expect(guard.persistOnce()).toBe(true) + guard.reset() + expect(guard.persistOnce()).toBe(true) + + expect(persist).toHaveBeenCalledTimes(2) + }) + + it('retries when the blocking checkpoint throws', () => { + const persist = vi.fn().mockImplementationOnce(() => { + throw new Error('disk full') + }) + const guard = createShutdownCheckpointGuard(persist) + + expect(guard.persistOnce()).toBe(false) + expect(guard.persistOnce()).toBe(true) + + expect(persist).toHaveBeenCalledTimes(2) + }) + + it('retries after a prevented reload resets the completed checkpoint', () => { + const eventTarget = new EventTarget() + const persist = vi.fn() + const guard = createShutdownCheckpointGuard(persist) + const checkpoint = createShutdownCheckpointBeforeUnloadHandler(guard) + const preventReload = (event: Event): void => event.preventDefault() + eventTarget.addEventListener('beforeunload', checkpoint) + eventTarget.addEventListener('beforeunload', preventReload) + + expect(eventTarget.dispatchEvent(new Event('beforeunload', { cancelable: true }))).toBe(false) + guard.reset() + eventTarget.removeEventListener('beforeunload', preventReload) + expect(eventTarget.dispatchEvent(new Event('beforeunload', { cancelable: true }))).toBe(true) + + expect(persist).toHaveBeenCalledTimes(2) + }) + + it('cancels unload when persistence fails and remains retryable', () => { + const eventTarget = new EventTarget() + const persist = vi.fn().mockImplementationOnce(() => { + throw new Error('disk full') + }) + const guard = createShutdownCheckpointGuard(persist) + const checkpoint = createShutdownCheckpointBeforeUnloadHandler(guard) + eventTarget.addEventListener('beforeunload', checkpoint) + + expect(eventTarget.dispatchEvent(new Event('beforeunload', { cancelable: true }))).toBe(false) + expect(eventTarget.dispatchEvent(new Event('beforeunload', { cancelable: true }))).toBe(true) + + expect(persist).toHaveBeenCalledTimes(2) + }) + + it('resets after a paired-web dirty-file veto regardless of listener order', async () => { + const eventTarget = new EventTarget() + const persist = vi.fn() + const guard = createShutdownCheckpointGuard(persist) + const preventReload = (event: Event): void => { + preventUnloadAndScheduleShutdownCheckpointReset(event, eventTarget) + } + eventTarget.addEventListener('beforeunload', preventReload) + eventTarget.addEventListener('beforeunload', createShutdownCheckpointBeforeUnloadHandler(guard)) + eventTarget.addEventListener(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT, guard.reset) + + expect(eventTarget.dispatchEvent(new Event('beforeunload', { cancelable: true }))).toBe(false) + await Promise.resolve() + eventTarget.removeEventListener('beforeunload', preventReload) + expect(eventTarget.dispatchEvent(new Event('beforeunload', { cancelable: true }))).toBe(true) + + expect(persist).toHaveBeenCalledTimes(2) + }) + + it('wires dirty editor unload vetoes to the paired-web checkpoint reset', () => { + const source = readFileSync( + join(process.cwd(), 'src/renderer/src/components/Terminal.tsx'), + 'utf8' + ) + const dirtyGuardStart = source.indexOf( + 'const dirtyFiles = useAppStore.getState().openFiles.filter((f) => f.isDirty)' + ) + const dirtyGuardEnd = source.indexOf("window.addEventListener('beforeunload', handler)") + expect(dirtyGuardStart).toBeGreaterThanOrEqual(0) + expect(dirtyGuardEnd).toBeGreaterThan(dirtyGuardStart) + expect(source.slice(dirtyGuardStart, dirtyGuardEnd)).toContain( + 'preventUnloadAndScheduleShutdownCheckpointReset(e, window)' + ) + }) +}) diff --git a/src/renderer/src/lib/shutdown-checkpoint-guard.ts b/src/renderer/src/lib/shutdown-checkpoint-guard.ts new file mode 100644 index 000000000..b2f81eed8 --- /dev/null +++ b/src/renderer/src/lib/shutdown-checkpoint-guard.ts @@ -0,0 +1,51 @@ +import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../../../shared/renderer-shutdown-events' + +export type ShutdownCheckpointGuard = { + persistOnce: () => boolean + reset: () => void +} + +export function createShutdownCheckpointGuard(persist: () => void): ShutdownCheckpointGuard { + let persisted = false + return { + persistOnce(): boolean { + if (persisted) { + return true + } + try { + persist() + } catch { + // Why: browser event targets swallow listener exceptions. Returning a + // failure lets the caller cancel unload and keep this attempt retryable. + return false + } + persisted = true + return true + }, + reset(): void { + persisted = false + } + } +} + +export function createShutdownCheckpointBeforeUnloadHandler( + guard: ShutdownCheckpointGuard +): (event: Event) => void { + return (event): void => { + if (!guard.persistOnce()) { + event.preventDefault() + } + } +} + +export function preventUnloadAndScheduleShutdownCheckpointReset( + event: Event, + eventTarget: EventTarget +): void { + event.preventDefault() + // Why: paired web has no Electron will-prevent-unload callback. Defer until + // all beforeunload listeners finish so their successful checkpoint is reset. + queueMicrotask(() => { + eventTarget.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT)) + }) +} diff --git a/src/renderer/src/lib/workspace-session-host-persistence.test.ts b/src/renderer/src/lib/workspace-session-host-persistence.test.ts index 6c6db4933..e44ffa761 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.test.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.test.ts @@ -4,10 +4,13 @@ import type { WorkspaceSessionState } from '../../../shared/types' import { folderWorkspaceKey, worktreeWorkspaceKey } from '../../../shared/workspace-scope' import { buildHostIdByWorktreeId, + buildWorkspaceSessionHostSnapshots, fetchWorkspaceSessionFromHosts, fetchWorkspaceSessionWithRuntimeHostOwners, patchWorkspaceSessionByHost, - persistWorkspaceSessionByHost + persistWorkspaceSessionByHost, + persistWorkspaceSessionByHostSync, + type HostPersistenceState } from './workspace-session-host-persistence' describe('fetchWorkspaceSessionFromHosts', () => { @@ -384,6 +387,55 @@ describe('fetchWorkspaceSessionFromHosts', () => { expect(owner(worktreeWorkspaceKey(worktreeId))).toBe('runtime:env-1') }) + + it('builds local-first host snapshots reused by synchronous persistence', () => { + const localWorktreeId = 'local-repo::C:\\src\\local' + const remoteWorktreeId = 'remote-repo::/srv/remote' + const makeTab = (id: string, worktreeId: string) => ({ + id, + ptyId: null, + worktreeId, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + }) + const payload: WorkspaceSessionState = { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [localWorktreeId]: [makeTab('local-tab', localWorktreeId)], + [remoteWorktreeId]: [makeTab('remote-tab', remoteWorktreeId)] + } + } + const state = { + repos: [ + { id: 'local-repo', connectionId: null, executionHostId: 'local' }, + { id: 'remote-repo', connectionId: null, executionHostId: 'runtime:env-1' } + ], + worktreesByRepo: { + 'local-repo': [{ id: localWorktreeId, repoId: 'local-repo' }], + 'remote-repo': [{ id: remoteWorktreeId, repoId: 'remote-repo', hostId: 'runtime:env-1' }] + } + } satisfies HostPersistenceState + + const snapshots = buildWorkspaceSessionHostSnapshots(payload, state) + + expect(snapshots.map((snapshot) => snapshot.hostId)).toEqual([undefined, 'runtime:env-1']) + expect(snapshots[0].state.tabsByWorktree).toEqual({ + [localWorktreeId]: [expect.objectContaining({ id: 'local-tab' })] + }) + expect(snapshots[1].state.tabsByWorktree).toEqual({ + [remoteWorktreeId]: [expect.objectContaining({ id: 'remote-tab' })] + }) + + const setSync = vi.fn() + persistWorkspaceSessionByHostSync({ get: vi.fn(), patch: vi.fn(), setSync }, payload, state) + + expect(setSync.mock.calls).toEqual( + snapshots.map((snapshot) => [snapshot.state, snapshot.hostId]) + ) + }) }) describe('persistWorkspaceSessionByHost', () => { diff --git a/src/renderer/src/lib/workspace-session-host-persistence.ts b/src/renderer/src/lib/workspace-session-host-persistence.ts index ab7b04c2f..767db0cf9 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.ts @@ -43,6 +43,11 @@ export type WorkspaceSessionHostRead = { runtimeHostIdByWorkspaceSessionKey: Record } +export type WorkspaceSessionHostSnapshot = { + state: WorkspaceSessionState + hostId?: ExecutionHostId +} + const WORKSPACE_SESSION_KEYED_FIELDS = [ 'tabsByWorktree', 'openFilesByWorktree', @@ -245,16 +250,26 @@ export async function persistWorkspaceSessionByHost( await api.flush() } +/** Build local-first full-session snapshots for the beforeunload / quit paths. */ +export function buildWorkspaceSessionHostSnapshots( + payload: WorkspaceSessionState, + state: HostPersistenceState +): WorkspaceSessionHostSnapshot[] { + const slices = splitWorkspaceSessionByHost(payload, buildHostIdByWorktreeId(state)) + return [ + { state: slices[LOCAL_EXECUTION_HOST_ID] ?? payload }, + ...nonLocalEntries(slices).map(([hostId, hostState]) => ({ state: hostState, hostId })) + ] +} + /** Synchronous full-session split for the beforeunload / quit paths. */ export function persistWorkspaceSessionByHostSync( api: SessionApi, payload: WorkspaceSessionState, state: HostPersistenceState ): void { - const slices = splitWorkspaceSessionByHost(payload, buildHostIdByWorktreeId(state)) - api.setSync(slices[LOCAL_EXECUTION_HOST_ID] ?? payload) - for (const [hostId, slice] of nonLocalEntries(slices)) { - api.setSync(slice, hostId) + for (const snapshot of buildWorkspaceSessionHostSnapshots(payload, state)) { + api.setSync(snapshot.state, snapshot.hostId) } } diff --git a/src/renderer/src/store/slices/ui.ts b/src/renderer/src/store/slices/ui.ts index 4be3cc90b..713c20466 100644 --- a/src/renderer/src/store/slices/ui.ts +++ b/src/renderer/src/store/slices/ui.ts @@ -35,6 +35,7 @@ import { applyManualRepoOrder, normalizeManualRepoOrder } from '../../../../shared/manual-repo-order' +import { isTopLevelView } from '../../../../shared/top-level-view' import type { UsagePercentageDisplay } from '../../../../shared/usage-percentage-display' import { DEFAULT_USAGE_PERCENTAGE_DISPLAY, @@ -503,26 +504,13 @@ function hydratedUIPartialMatchesState(state: AppState, hydrated: Partial = { - terminal: true, - settings: true, - tasks: true, - activity: true, - automations: true, - space: true, - skills: true, - mobile: true -} -const KNOWN_TOP_LEVEL_VIEWS = new Set(Object.keys(TOP_LEVEL_VIEW_LOOKUP)) - function sanitizeHydratedActiveView( value: PersistedUIState['activeView'], experimentalActivityEnabled: boolean ): TopLevelView { // Why: older data (pre-activeView) or a view a different build doesn't have // falls back to terminal rather than rendering nothing. - if (typeof value !== 'string' || !KNOWN_TOP_LEVEL_VIEWS.has(value)) { + if (!isTopLevelView(value)) { return 'terminal' } // Why: activity is hidden when its setting is off, so restoring it lands on a @@ -531,7 +519,7 @@ function sanitizeHydratedActiveView( if (value === 'activity' && !experimentalActivityEnabled) { return 'terminal' } - return value as TopLevelView + return value } let agentSendTargetModeInstanceCounter = 0 diff --git a/src/renderer/src/web/web-preload-api.test.ts b/src/renderer/src/web/web-preload-api.test.ts index bb95dcad9..68cf1f100 100644 --- a/src/renderer/src/web/web-preload-api.test.ts +++ b/src/renderer/src/web/web-preload-api.test.ts @@ -140,6 +140,41 @@ function installClipboardImageBase64(contentBase64: string): void { }) } +describe('web before-unload persistence', () => { + beforeEach(() => { + vi.resetModules() + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('persists final UI and host-partitioned sessions synchronously', async () => { + const { api, storage } = await installApi('Linux') + + api.app.persistBeforeUnloadSync({ + sessions: [ + { state: { activeWorktreeId: 'local-worktree' } as never }, + { + state: { activeWorktreeId: 'remote-worktree' } as never, + hostId: 'runtime:web-env-1' + } + ], + ui: { activeView: 'settings' } + }) + + expect(JSON.parse(storage.getItem('orca.web.workspaceSession.v1') ?? '{}')).toMatchObject({ + activeWorktreeId: 'local-worktree' + }) + expect( + JSON.parse(storage.getItem('orca.web.workspaceSession.v1.runtime:web-env-1') ?? '{}') + ).toMatchObject({ activeWorktreeId: 'remote-worktree' }) + expect(JSON.parse(storage.getItem('orca.web.ui.v1') ?? '{}')).toMatchObject({ + activeView: 'settings' + }) + }) +}) + function installClipboardImageBlob(blob: Blob): { getType: ReturnType read: ReturnType diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 41fb98336..b899324a4 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -495,6 +495,14 @@ function createWebPreloadApi(): Partial { relaunch: () => Promise.resolve(window.location.reload()), restart: () => Promise.resolve(window.location.reload()), reload: () => Promise.resolve(window.location.reload()), + persistBeforeUnloadSync: ({ 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) { + writeJson(sessionStorageKeyForHost(hostId), sanitizeWebRuntimeWorkspaceSession(state)) + } + writeJson(UI_STORAGE_KEY, mergeWebUIState(readLocalWebUIState(), ui)) + }, awaitFirstWindowStartupServices: () => Promise.resolve(), startupDiagnostic: () => Promise.resolve(), getKeyboardInputSourceId: () => Promise.resolve(null), diff --git a/src/shared/renderer-shutdown-events.ts b/src/shared/renderer-shutdown-events.ts new file mode 100644 index 000000000..6d679e33b --- /dev/null +++ b/src/shared/renderer-shutdown-events.ts @@ -0,0 +1 @@ +export const ORCA_RENDERER_UNLOAD_PREVENTED_EVENT = 'orca:renderer-unload-prevented' diff --git a/src/shared/top-level-view.ts b/src/shared/top-level-view.ts new file mode 100644 index 000000000..574d64475 --- /dev/null +++ b/src/shared/top-level-view.ts @@ -0,0 +1,20 @@ +import type { TopLevelView } from './types' + +// Record keys are exhaustive so adding a top-level view also updates every +// persistence boundary that validates values loaded from disk or IPC. +const TOP_LEVEL_VIEW_LOOKUP: Record = { + terminal: true, + settings: true, + tasks: true, + activity: true, + automations: true, + space: true, + skills: true, + mobile: true +} + +export function isTopLevelView(value: unknown): value is TopLevelView { + // Why: hasOwn (not `in`) so inherited keys like "constructor"/"__proto__" from a + // corrupt sidecar can't pass as a view and leave the main surface blank. + return typeof value === 'string' && Object.hasOwn(TOP_LEVEL_VIEW_LOOKUP, value) +} diff --git a/tests/e2e/active-view-restart-restore.spec.ts b/tests/e2e/active-view-restart-restore.spec.ts index 093e5774d..28b64ce80 100644 --- a/tests/e2e/active-view-restart-restore.spec.ts +++ b/tests/e2e/active-view-restart-restore.spec.ts @@ -3,11 +3,11 @@ * * Reproduces the reported bug (renderer reload / relaunch always snapped back * to the terminal, discarding whichever top-level view — Tasks, Automations, - * etc. — the user had open) and asserts the fix: activeView now rides the - * PersistedUIState pipeline and is restored on the first (startup) hydration. + * etc. — the user had open) and asserts the fix: activeView now rides its + * profile preference pipeline and is restored on the first startup hydration. * * Restart-persistence lives in E2E, not a store unit test: it needs the real - * write -> orca-data.json -> ui.get() -> hydratePersistedUI round-trip across + * write -> active-view.json -> ui.get() -> hydratePersistedUI round-trip across * two Electron launches sharing one userDataDir, then the render layer proving * the page actually came back — with a real repo/worktree attached so the * relaunch also exercises the startup worktree hydration path (which must not @@ -15,7 +15,7 @@ */ import { existsSync, readFileSync } from 'node:fs' -import type { ElectronApplication, Page } from '@stablyai/playwright-test' +import type { ElectronApplication } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { getStoreState, waitForSessionReady } from './helpers/store' import { attachRepoAndOpenTerminal, createRestartSession } from './helpers/orca-restart' @@ -29,10 +29,6 @@ function seededRepoPathOrSkip(): string { return repoPath } -async function readPersistedActiveView(page: Page): Promise { - return page.evaluate(() => window.api.ui.get().then((ui) => ui.activeView)) -} - test('restores the active top-level view (Tasks) after an app restart', async (// oxlint-disable-next-line no-empty-pattern -- Playwright's second fixture arg is testInfo; the first must be an object destructure to opt out of the default fixture set. {}, testInfo) => { test.setTimeout(300_000) @@ -72,12 +68,8 @@ test('restores the active top-level view (Tasks) after an app restart', async (/ // And the terminal grid is not the active surface. await expect(first.page.locator('.xterm')).not.toBeVisible({ timeout: 10_000 }) - // The debounced writer must flush the view to the main-process UI state - // before we quit, so the relaunch reads it back from disk. - await expect - .poll(async () => readPersistedActiveView(first.page), { timeout: 10_000 }) - .toBe('tasks') - + // Closing also exercises the synchronous checkpoint that covers the race + // where exit starts before the tiny asynchronous preference write finishes. await session.close(firstApp) firstApp = null