diff --git a/src/main/startup/main-process-preflight.ts b/src/main/startup/main-process-preflight.ts index 2aa02ece192..100f1e21386 100644 --- a/src/main/startup/main-process-preflight.ts +++ b/src/main/startup/main-process-preflight.ts @@ -147,7 +147,9 @@ function initializeMainProcessPreflight(options: MainProcessPreflightOptions): b ? state.devInstanceIdentity.appUserModelId : undefined state.desktopActivationGate = createServeDesktopActivationGate({ - initialState: state.isServeMode ? 'initializing' : 'ready', + // Why held for desktop too: an activation before the startup window exists would open a + // second main window and abort launch; runtime launch releases it once that window exists. + initialState: 'initializing', activateWindow: () => { // Why: an updater replacement must not resurrect the old app bundle. if (!isQuittingForUpdate()) { diff --git a/src/main/startup/main-process-ready-activation-failure.test.ts b/src/main/startup/main-process-ready-activation-failure.test.ts new file mode 100644 index 00000000000..b3d265b3c25 --- /dev/null +++ b/src/main/startup/main-process-ready-activation-failure.test.ts @@ -0,0 +1,133 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { MainProcessRuntimeLaunchOptions } from './main-process-runtime-launch' + +const phases = vi.hoisted(() => ({ + foundation: vi.fn<() => Promise>(), + runtimeServices: vi.fn<() => Promise>(), + menu: vi.fn<() => Promise>(), + launch: vi.fn<(options: MainProcessRuntimeLaunchOptions) => Promise>() +})) + +vi.mock('./main-process-ready-foundation', () => ({ + initializeReadyFoundation: phases.foundation +})) +vi.mock('./main-process-ready-runtime', () => ({ + initializeReadyRuntimeServices: phases.runtimeServices +})) +vi.mock('./main-process-i18n-menu', () => ({ initializeMainProcessI18nAndMenu: phases.menu })) +vi.mock('./main-process-runtime-launch', () => ({ + initializeMainProcessRuntimeLaunch: phases.launch +})) + +const { initializeMainProcessReady } = await import('./main-process-ready') +const { mainProcessState: state } = await import('./main-process-state') +const { createServeDesktopActivationGate } = await import('./serve-desktop-activation') + +const activateWindow = vi.fn() +const launchOptions: MainProcessRuntimeLaunchOptions = { + openMainWindow: vi.fn(), + handleMacAppActivation: vi.fn() +} + +describe('desktop activation after ready-phase failures', () => { + beforeEach(() => { + vi.resetAllMocks() + phases.foundation.mockResolvedValue(undefined) + phases.runtimeServices.mockResolvedValue(undefined) + phases.menu.mockResolvedValue(undefined) + phases.launch.mockResolvedValue(undefined) + state.isServeMode = false + state.desktopActivationGate = createServeDesktopActivationGate({ + initialState: 'initializing', + activateWindow + }) + }) + + afterEach(() => { + state.desktopActivationGate = null + state.isServeMode = false + }) + + it.each(['foundation', 'runtimeServices'] as const)( + 'disables desktop activation after %s fails', + async (phase) => { + const error = new Error(`${phase} failed`) + phases[phase].mockRejectedValueOnce(error) + state.desktopActivationGate?.requestActivation() + + await expect(initializeMainProcessReady(launchOptions)).rejects.toBe(error) + + expect(activateWindow).not.toHaveBeenCalled() + expect(state.desktopActivationGate).toBeNull() + expect(phases.launch).not.toHaveBeenCalled() + state.desktopActivationGate?.requestActivation() + expect(activateWindow).not.toHaveBeenCalled() + } + ) + + it.each(['foundation', 'runtimeServices'] as const)( + 'disables serve promotion after %s fails', + async (phase) => { + const error = new Error(`${phase} failed`) + phases[phase].mockRejectedValueOnce(error) + state.isServeMode = true + state.desktopActivationGate?.requestActivation() + + await expect(initializeMainProcessReady(launchOptions)).rejects.toBe(error) + + expect(state.desktopActivationGate).toBeNull() + state.desktopActivationGate?.requestActivation() + expect(activateWindow).not.toHaveBeenCalled() + expect(phases.launch).not.toHaveBeenCalled() + } + ) + + it('keeps activations held when menu failure leaves window creation still pending', async () => { + const error = new Error('menu failed') + let finishLaunch = (): void => { + throw new Error('launch has not started') + } + phases.menu.mockRejectedValueOnce(error) + phases.launch.mockImplementationOnce( + (options) => + new Promise((resolve) => { + finishLaunch = () => { + options.openMainWindow() + resolve() + } + }) + ) + state.desktopActivationGate?.requestActivation() + + const ready = initializeMainProcessReady(launchOptions) + const rejected = expect(ready).rejects.toBe(error) + await vi.waitFor(() => expect(phases.launch).toHaveBeenCalledOnce()) + + expect(state.desktopActivationGate?.getState()).toBe('initializing') + expect(activateWindow).not.toHaveBeenCalled() + expect(launchOptions.openMainWindow).not.toHaveBeenCalled() + finishLaunch() + expect(launchOptions.openMainWindow).toHaveBeenCalledTimes(1) + expect(state.desktopActivationGate?.getState()).toBe('ready') + expect(activateWindow).toHaveBeenCalledTimes(1) + await rejected + expect(state.desktopActivationGate).toBeNull() + }) + + it('does not replay pending activations when window creation throws', async () => { + const error = new Error('window creation failed') + vi.mocked(launchOptions.openMainWindow).mockImplementationOnce(() => { + throw error + }) + phases.launch.mockImplementationOnce(async (options) => { + options.openMainWindow() + }) + state.desktopActivationGate?.requestActivation() + + await expect(initializeMainProcessReady(launchOptions)).rejects.toBe(error) + + expect(state.desktopActivationGate).toBeNull() + expect(activateWindow).not.toHaveBeenCalled() + expect(launchOptions.openMainWindow).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/startup/main-process-ready-persistence-cleanup.test.ts b/src/main/startup/main-process-ready-persistence-cleanup.test.ts index 988c78f1c12..6deac0e8d51 100644 --- a/src/main/startup/main-process-ready-persistence-cleanup.test.ts +++ b/src/main/startup/main-process-ready-persistence-cleanup.test.ts @@ -1,9 +1,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + createServeDesktopActivationGate, + type ServeDesktopActivationGate +} from './serve-desktop-activation' const { state, foundation, runtime, i18n, launch } = vi.hoisted(() => ({ state: { store: { freezeWritesAsync: vi.fn(async () => {}) }, profileStateAdmission: initialAdmission(), + desktopActivationGate: initialActivationGate(), mainProcessI18nReady: Promise.resolve() }, foundation: vi.fn(async () => {}), @@ -16,6 +21,10 @@ function initialAdmission(): { release(): void } | undefined { return undefined } +function initialActivationGate(): ServeDesktopActivationGate | null { + return null +} + vi.mock('./main-process-state', () => ({ mainProcessState: state })) vi.mock('./main-process-ready-foundation', () => ({ initializeReadyFoundation: foundation })) vi.mock('./main-process-ready-runtime', () => ({ initializeReadyRuntimeServices: runtime })) @@ -34,9 +43,36 @@ const options = { beforeEach(() => { vi.clearAllMocks() state.profileStateAdmission = { release: vi.fn() } + state.desktopActivationGate = null }) describe('startup persistence lifetime', () => { + it('disables activations before a failed launch closes its profile writer', async () => { + const activateWindow = vi.fn() + state.desktopActivationGate = createServeDesktopActivationGate({ + initialState: 'ready', + activateWindow + }) + const failure = new Error('runtime startup failed') + launch.mockRejectedValueOnce(failure) + let finishFreeze = (): void => {} + state.store.freezeWritesAsync.mockImplementationOnce( + () => + new Promise((resolve) => { + finishFreeze = resolve + }) + ) + const rejected = expect(initializeMainProcessReady(options)).rejects.toBe(failure) + + await vi.waitFor(() => expect(state.store.freezeWritesAsync).toHaveBeenCalledOnce()) + + expect(state.desktopActivationGate).toBeNull() + state.desktopActivationGate?.requestActivation() + expect(activateWindow).not.toHaveBeenCalled() + finishFreeze() + await rejected + }) + it('awaits writer release after a later startup phase fails', async () => { const failure = new Error('runtime startup failed') const admission = state.profileStateAdmission diff --git a/src/main/startup/main-process-ready.ts b/src/main/startup/main-process-ready.ts index a6dcf83a430..7c27f73a465 100644 --- a/src/main/startup/main-process-ready.ts +++ b/src/main/startup/main-process-ready.ts @@ -2,6 +2,7 @@ import { initializeMainProcessI18nAndMenu } from './main-process-i18n-menu' import { mainProcessState as state } from './main-process-state' import { initializeReadyFoundation } from './main-process-ready-foundation' import { initializeReadyRuntimeServices } from './main-process-ready-runtime' +import { releaseDesktopActivationAfter } from './serve-desktop-activation' import { initializeMainProcessRuntimeLaunch, type MainProcessRuntimeLaunchOptions @@ -9,8 +10,15 @@ import { /** Runs the ready-phase composition in the same dependency order as the legacy entry point. */ export async function initializeMainProcessReady( - options: MainProcessRuntimeLaunchOptions + launchOptions: MainProcessRuntimeLaunchOptions ): Promise { + const options: MainProcessRuntimeLaunchOptions = { + ...launchOptions, + openMainWindow: releaseDesktopActivationAfter( + state.desktopActivationGate, + launchOptions.openMainWindow + ) + } try { await initializeReadyFoundation() await initializeReadyRuntimeServices() @@ -28,6 +36,8 @@ export async function initializeMainProcessReady( } } } catch (error) { + // Startup now exits after failure; reopening would use a closing profile writer. + state.desktopActivationGate = null try { await state.store?.freezeWritesAsync() state.profileStateAdmission?.release() diff --git a/src/main/startup/main-process-runtime-launch-activation.test.ts b/src/main/startup/main-process-runtime-launch-activation.test.ts new file mode 100644 index 00000000000..42813af450e --- /dev/null +++ b/src/main/startup/main-process-runtime-launch-activation.test.ts @@ -0,0 +1,232 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const electronApp = vi.hoisted(() => ({ + isPackaged: false, + on: vi.fn(), + getPath: vi.fn(() => '/tmp/orca-user-data'), + getVersion: vi.fn(() => '0.0.0-test'), + isReady: vi.fn(() => true), + focus: vi.fn() +})) +const launchHooks = vi.hoisted(() => ({ + duringInstallDirRepair: (): void => {}, + failBeforeWindow: false +})) + +vi.mock('electron', () => ({ app: electronApp, powerMonitor: { on: vi.fn() } })) +vi.mock('@electron-toolkit/utils', () => ({ is: { dev: false } })) +vi.mock('../orca-profiles/profile-cloud-auth-config', () => ({ + getOrcaCloudAuthConfig: () => ({ configured: false }) +})) +vi.mock('../orca-profiles/profile-storage-paths', () => ({ getProfileUserDataPath: vi.fn() })) +vi.mock('../persistence', () => ({ + getCanonicalUserDataPath: () => '/tmp/orca-user-data', + migrateMobilePairingDataToCanonicalUserDataPath: vi.fn() +})) +vi.mock('../runtime/runtime-rpc', () => ({ + OrcaRuntimeRpcServer: class { + start = vi.fn(async () => {}) + setOnUnpairedDeviceAuthFailure = vi.fn() + } +})) +vi.mock('../ipc/mobile', () => ({ registerMobileHandlers: vi.fn() })) +vi.mock('../ipc/pty', () => ({ + getLocalPtyProvider: vi.fn(), + registerHeadlessPtyRuntime: vi.fn() +})) +vi.mock('../providers/local-pty-provider', () => ({ LocalPtyProvider: class {} })) +vi.mock('../browser/offscreen-browser-backend', () => ({ OffscreenBrowserBackend: class {} })) +vi.mock('../browser/browser-manager', () => ({ browserManager: {} })) +vi.mock('./main-process-relay-status', () => ({ + getDesktopRelayStatus: vi.fn(), + publishDesktopRelayStatus: vi.fn() +})) +vi.mock('../runtime/relay/desktop-relay-service', () => ({ DesktopRelayService: class {} })) +vi.mock('./main-process-serve', () => ({ + getServeOptions: vi.fn(() => null), + getBundledWebClientRoot: vi.fn(() => null), + printServeReady: vi.fn() +})) +vi.mock('./main-process-pty-startup', () => ({ + bindTerminalRuntimeStartupServices: vi.fn(), + handleCodexHomePtySpawned: vi.fn(), + handlePtyExit: vi.fn(), + startTerminalRuntimeStartupServices: vi.fn(() => ({})) +})) +vi.mock('./codex-launch-preparation', () => ({ prepareCodexRuntimeHomeForLaunch: vi.fn() })) +vi.mock('./codex-session-resume-launch', () => ({ prepareCodexSessionResumeForLaunch: vi.fn() })) +vi.mock('./windows-install-dir-acl-recovery', () => ({ + // The awaited gap before the first window, where a second-instance launch can land. + repairKnownPoisonedInstallDirBeforeWindow: vi.fn(async () => { + launchHooks.duringInstallDirRepair() + if (launchHooks.failBeforeWindow) { + throw new Error('install-dir repair failed') + } + return 'not-marked' + }) +})) +vi.mock('./serve-signal-handlers', () => ({ registerServeSignalHandlers: vi.fn() })) +vi.mock('../runtime/runtime-rpc-startup-failure', () => ({ + recordRuntimeRpcStartFailure: vi.fn(), + showRuntimeRpcStartupFailureDialog: vi.fn() +})) +vi.mock('../cli/cli-installer', () => ({ CliInstaller: class {} })) +vi.mock('../cli/linux-bare-orca-dispatcher', () => ({ installLinuxBareOrcaDispatcher: vi.fn() })) +vi.mock('../terminal-history-deletion', () => ({ scheduleAllPendingHistoryTreeRemovals: vi.fn() })) +vi.mock('../ipc/startup-notification-registration', () => ({ + triggerStartupNotificationRegistration: vi.fn() +})) +vi.mock('./main-process-push-startup', () => ({ startDesktopPushService: vi.fn() })) +vi.mock('./startup-diagnostics', () => ({ logStartupMilestone: vi.fn() })) +vi.mock('../server/serve-stdout-boundary', () => ({ emitServeBrowserIdentityActionLine: vi.fn() })) +vi.mock('../browser/browser-identity-mode-store', () => ({ + getBrowserIdentityModeStatus: vi.fn() +})) +const showWindowWithoutStealingFocus = vi.hoisted(() => vi.fn()) +vi.mock('../window/foreground-activation-policy', () => ({ + isBackgroundLaunch: () => true, + isWindowlessLaunch: () => false, + showWindowWithoutStealingFocus +})) + +vi.mock('./main-process-ready-foundation', () => ({ + initializeReadyFoundation: vi.fn(async () => {}) +})) +vi.mock('./main-process-ready-runtime', () => ({ + initializeReadyRuntimeServices: vi.fn(async () => {}) +})) +vi.mock('./main-process-i18n-menu', () => ({ + initializeMainProcessI18nAndMenu: vi.fn(async () => {}) +})) + +const { initializeMainProcessReady } = await import('./main-process-ready') +const { mainProcessState: state } = await import('./main-process-state') +const { createServeDesktopActivationGate } = await import('./serve-desktop-activation') +const { focusExistingMainWindow } = await import('../window/focus-existing-window') + +type FakeWindow = { + id: number + webContents: { id: number } + isDestroyed: () => boolean + isMinimized: () => boolean + restore: () => void + once: () => void +} + +const originalPlatform = process.platform + +describe('desktop startup activation', () => { + let windows: FakeWindow[] + let ipcHandles: Set + let trustedRendererId: number | null + + // Mirrors openMainWindow's non-idempotent side effects that broke in the field. + function openMainWindow(): FakeWindow { + const id = windows.length + 1 + const window: FakeWindow = { + id, + webContents: { id }, + isDestroyed: () => false, + isMinimized: () => false, + restore: vi.fn(), + once: vi.fn() + } + windows.push(window) + trustedRendererId = id + if (ipcHandles.has('window:isMaximized')) { + throw new Error("Attempted to register a second handler for 'window:isMaximized'") + } + ipcHandles.add('window:isMaximized') + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch only reads the fields FakeWindow provides. + state.mainWindow = window as unknown as NonNullable + return window + } + + beforeEach(() => { + windows = [] + showWindowWithoutStealingFocus.mockClear() + ipcHandles = new Set() + trustedRendererId = null + launchHooks.duringInstallDirRepair = () => {} + launchHooks.failBeforeWindow = false + state.mainWindow = null + state.isServeMode = false + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch only null-checks the runtime before the mocked RPC server takes it. + state.runtime = {} as NonNullable + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch only calls whenReady(). + state.windowsShellPathHydration = { + whenReady: () => Promise.resolve() + } as unknown as NonNullable + state.initialProxyApplicationReady = Promise.resolve() + // Built the way preflight builds it for a desktop launch. + state.desktopActivationGate = createServeDesktopActivationGate({ + initialState: 'initializing', + activateWindow: () => + focusExistingMainWindow({ + app: electronApp, + getWindow: () => state.mainWindow, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: focusExistingMainWindow only calls the FakeWindow methods. + openWindow: () => openMainWindow() as unknown as NonNullable + }) + }) + }) + + afterEach(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform }) + electronApp.isPackaged = false + }) + + it.each([ + ['darwin', false], + ['linux', false], + ['win32', true] + ] as const)( + 'focuses the startup window when a second instance lands before it exists (%s)', + async (platform, isPackaged) => { + Object.defineProperty(process, 'platform', { value: platform }) + electronApp.isPackaged = isPackaged + launchHooks.duringInstallDirRepair = () => state.desktopActivationGate?.requestActivation() + + await initializeMainProcessReady({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch only calls once() on the returned window. + openMainWindow: () => openMainWindow() as unknown as NonNullable, + handleMacAppActivation: vi.fn() + }) + + expect(windows).toHaveLength(1) + expect(trustedRendererId).toBe(windows[0].id) + expect(state.mainWindow).toBe(windows[0]) + expect(showWindowWithoutStealingFocus).toHaveBeenCalledWith(windows[0]) + expect(state.desktopActivationGate?.getState()).toBe('ready') + } + ) + + it('does not replay an activation when launch fails before the startup window', async () => { + launchHooks.duringInstallDirRepair = () => state.desktopActivationGate?.requestActivation() + launchHooks.failBeforeWindow = true + + await expect( + initializeMainProcessReady({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the launch only calls once() on the returned window. + openMainWindow: () => openMainWindow() as unknown as NonNullable, + handleMacAppActivation: vi.fn() + }) + ).rejects.toThrow('install-dir repair failed') + + expect(windows).toHaveLength(0) + expect(state.desktopActivationGate).toBeNull() + }) + + it('holds every launch mode behind the gate until startup settles it', () => { + const preflightSource = readFileSync( + join(process.cwd(), 'src/main/startup/main-process-preflight.ts'), + 'utf8' + ) + expect(preflightSource).toContain("initialState: 'initializing',") + expect(preflightSource).not.toContain( + "initialState: state.isServeMode ? 'initializing' : 'ready'" + ) + }) +}) diff --git a/src/main/startup/serve-desktop-activation.ts b/src/main/startup/serve-desktop-activation.ts index 2b7ad520891..0034c8859e0 100644 --- a/src/main/startup/serve-desktop-activation.ts +++ b/src/main/startup/serve-desktop-activation.ts @@ -27,6 +27,21 @@ export function settleServeDesktopActivation( gate.markReady() } +/** + * Wraps the desktop startup-window opener so activations queued since preflight are let through + * only once that window exists: they then focus it, where earlier they would open a duplicate. + */ +export function releaseDesktopActivationAfter( + gate: ServeDesktopActivationGate | null, + open: (...args: TArgs) => TResult +): (...args: TArgs) => TResult { + return (...args) => { + const result = open(...args) + gate?.markReady() + return result + } +} + export function createServeDesktopActivationGate(options: { initialState: 'initializing' | 'ready' activateWindow: () => void