From 029b5ba1cba614c37591146802425790c404e579 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Fri, 25 Sep 2026 23:02:23 -0700 Subject: [PATCH] fix(startup): hold desktop activations until the startup window exists (#22495) * fix(startup): hold desktop activations until the startup window exists A second-instance, open-url, or open-file activation that landed after app ready but before the startup window was created opened its own main window. The startup open then built a second window, threw on the duplicate 'window:isMaximized' handler, and aborted runtime launch: no runtime RPC, no orca-runtime.json, an orphan hidden window holding the trusted-renderer id, and the visible window refused trusted IPC. The desktop activation gate now starts 'initializing' for every launch mode. The startup window opener releases it once that window exists, so queued activations focus it, and a desktop launch that fails first releases it so later activations can still open a window. Serve mode keeps settling the gate after its RPC is ready. * fix(startup): release desktop activation after failed ready prerequisites --------- Co-authored-by: m4air --- src/main/startup/main-process-preflight.ts | 4 +- ...n-process-ready-activation-failure.test.ts | 133 ++++++++++ ...-process-ready-persistence-cleanup.test.ts | 36 +++ src/main/startup/main-process-ready.ts | 12 +- ...-process-runtime-launch-activation.test.ts | 232 ++++++++++++++++++ src/main/startup/serve-desktop-activation.ts | 15 ++ 6 files changed, 430 insertions(+), 2 deletions(-) create mode 100644 src/main/startup/main-process-ready-activation-failure.test.ts create mode 100644 src/main/startup/main-process-runtime-launch-activation.test.ts 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