diff --git a/src/main/index.ts b/src/main/index.ts index 3868ebf63ea..6d2febb792e 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -442,6 +442,26 @@ const devAgentHookEndpointNamespace = devInstanceIdentity.isDev : undefined installUncaughtPipeErrorGuard() +configureDevUserDataPath(is.dev) +configureOrcaUserDataPathEnv() + +// Why: a launch that races Squirrel's in-flight update install would abort it +// ("App Still Running Error") and silently strand the user on the old +// version. Gate before PATH hydration so a rejected launch does not spawn a +// login shell or spend more time racing ShipIt's final running-app check. +if ( + shouldDeferLaunchForUpdateInstall({ + isPackaged: app.isPackaged, + userDataPath: app.getPath('userData'), + appVersion: app.getVersion() + }) +) { + console.warn( + '[updater] update install in flight for this bundle; exiting so the installer can finish and relaunch' + ) + app.exit(0) +} + // Why: propagate the Orca app version into `process.env` so PTY-env // construction in both main (local-pty-provider) and the forked daemon // (pty-subprocess) can set `TERM_PROGRAM_VERSION` without re-importing @@ -463,26 +483,6 @@ if (app.isPackaged && process.platform !== 'win32') { } }) } -configureDevUserDataPath(is.dev) -configureOrcaUserDataPathEnv() - -// Why: a launch that races Squirrel's in-flight update install would abort it -// ("App Still Running Error") and silently strand the user on the old -// version. Exit instead — ShipIt finishes the install and relaunches the -// updated app itself, so this click still ends with Orca open. Must run -// before the single-instance lock and all userData side effects. -if ( - shouldDeferLaunchForUpdateInstall({ - isPackaged: app.isPackaged, - userDataPath: app.getPath('userData'), - appVersion: app.getVersion() - }) -) { - console.warn( - '[updater] update install in flight for this bundle; exiting so the installer can finish and relaunch' - ) - app.exit(0) -} // Why: just past createMainWindow's 10s ready-to-show reveal fallback, // so a window revealed on that path still gets its tray icon. diff --git a/src/main/startup/update-install-launch-gate.test.ts b/src/main/startup/update-install-launch-gate.test.ts index fe5b1b4e17c..ff31792af5b 100644 --- a/src/main/startup/update-install-launch-gate.test.ts +++ b/src/main/startup/update-install-launch-gate.test.ts @@ -70,7 +70,7 @@ describe('shouldDeferLaunchForUpdateInstall', () => { expect(defer).toBe(true) expect(execFileSyncMock).toHaveBeenCalledWith( - 'ps', + '/bin/ps', ['-axo', 'command='], expect.objectContaining({ encoding: 'utf8' }) ) diff --git a/src/main/startup/update-install-launch-gate.ts b/src/main/startup/update-install-launch-gate.ts index cff239f32dc..b5f5555dff8 100644 --- a/src/main/startup/update-install-launch-gate.ts +++ b/src/main/startup/update-install-launch-gate.ts @@ -69,8 +69,8 @@ function readUpdateInstallHandoffMarker(userDataPath: string): UpdateInstallHand function readProcessCommandList(): string { // Why: ShipIt can run as root when the app bundle is not user-writable. - // The launch gate must see that privileged installer too. - return execFileSync('ps', ['-axo', 'command='], { + // Use the system binary so a shell-hydrated PATH cannot change detection. + return execFileSync('/bin/ps', ['-axo', 'command='], { encoding: 'utf8', timeout: PROCESS_LIST_TIMEOUT_MS, maxBuffer: PROCESS_LIST_MAX_BYTES @@ -82,8 +82,10 @@ function readProcessCommandList(): string { * this exact install is considered. */ export function isBundleShipItRunning(executablePath: string, processCommandList: string): boolean { // /Applications/Orca.app/Contents/MacOS/Orca → /Applications/Orca.app - const bundleRoot = path.resolve(executablePath, '..', '..', '..') - const shipItPath = path.join( + // Why: this matcher always receives a macOS path, even when its unit tests + // run on Windows, so host-native separators would corrupt the comparison. + const bundleRoot = path.posix.resolve(executablePath, '..', '..', '..') + const shipItPath = path.posix.join( bundleRoot, 'Contents', 'Frameworks', diff --git a/src/main/updater-conflicting-app-instances.ts b/src/main/updater-conflicting-app-instances.ts index 5df49cea31e..8fcacd39400 100644 --- a/src/main/updater-conflicting-app-instances.ts +++ b/src/main/updater-conflicting-app-instances.ts @@ -23,8 +23,9 @@ function readCurrentUserProcessList(): Promise { // user's Orca does not block ShipIt and must not block the update here. // On macOS `comm` is the full executable path, so instances launched from // the same bundle match exactly whatever their argv or profile is. + // Pin the system binary because Orca hydrates PATH from the user's shell. execFile( - 'ps', + '/bin/ps', ['-xo', 'pid=,comm='], { encoding: 'utf8', timeout: PROCESS_LIST_TIMEOUT_MS, maxBuffer: PROCESS_LIST_MAX_BYTES }, (error, stdout) => { diff --git a/src/main/updater.mac-install.test.ts b/src/main/updater.mac-install.test.ts index ee829904674..481e7c9c8a6 100644 --- a/src/main/updater.mac-install.test.ts +++ b/src/main/updater.mac-install.test.ts @@ -249,6 +249,58 @@ describe('updater mac install handoff', () => { } ) + it.runIf(process.platform === 'darwin')( + 'checks for blocking instances after pre-quit cleanup settles', + async () => { + let finishCleanup!: () => void + const onBeforeQuit = vi.fn( + () => + new Promise((resolve) => { + finishCleanup = resolve + }) + ) + const sendMock = vi.fn() + const mainWindow = { webContents: { send: sendMock } } + + autoUpdaterMock.checkForUpdates.mockResolvedValue(undefined) + const { setupAutoUpdater, quitAndInstall } = await import('./updater') + + setupAutoUpdater(mainWindow as never, { onBeforeQuit }) + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) + autoUpdaterMock.emit('checking-for-update') + autoUpdaterMock.emit('update-available', { version: '1.0.61' }) + await new Promise((resolve) => setTimeout(resolve, 0)) + autoUpdaterMock.emit('update-downloaded', { version: '1.0.61' }) + const nativeDownloadedHandler = nativeUpdaterMock.on.mock.calls.find( + ([eventName]) => eventName === 'update-downloaded' + )?.[1] as (() => void) | undefined + nativeDownloadedHandler?.() + + quitAndInstall() + await vi.waitFor(() => { + expect(onBeforeQuit).toHaveBeenCalledTimes(1) + }) + expect(findConflictingAppInstancePidsMock).not.toHaveBeenCalled() + + // Why: this models an agent/test instance launching during the bounded + // cleanup window; the last-moment scan must see it before ShipIt starts. + findConflictingAppInstancePidsMock.mockResolvedValue([4242]) + finishCleanup() + + await vi.waitFor(() => { + expect(sendMock).toHaveBeenCalledWith( + 'updater:status', + expect.objectContaining({ state: 'error', retryAction: 'install' }) + ) + }) + expect(findConflictingAppInstancePidsMock).toHaveBeenCalledTimes(1) + expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + expect(killAllPtyMock).not.toHaveBeenCalled() + } + ) + it.runIf(process.platform === 'darwin')( 'ignores duplicate quit requests while deferred mac install cleanup is running', async () => { diff --git a/src/main/updater.ts b/src/main/updater.ts index 4da7b3fc04c..518bc616978 100644 --- a/src/main/updater.ts +++ b/src/main/updater.ts @@ -582,30 +582,6 @@ async function performQuitAndInstall(): Promise { pendingQuitAndInstallTimer = null } - // Why: Squirrel.Mac's ShipIt waits for ALL instances of the installed - // bundle to exit before installing (and aborts if one appears mid-install), - // so a second packaged Orca — often an invisible agent/e2e-launched one — - // makes "Restart to Update" quit the app and then silently never install - // or relaunch. Surface those instances instead of starting a doomed quit. - const conflictingPids = await findConflictingAppInstancePids() - if (conflictingPids.length > 0) { - quitAndInstallInProgress = false - recordUpdaterLifecycle( - 'quit_and_install_blocked_by_other_instances', - { pids: conflictingPids.join(','), count: conflictingPids.length }, - { - level: 'warn', - message: `Update install blocked: ${conflictingPids.length} other running instance(s) of this app (pids ${conflictingPids.join(', ')})` - } - ) - sendErrorStatus( - `Another running copy of Orca is blocking the update (PID ${conflictingPids.join(', ')}) — possibly one launched by an agent or test run. Quit it, then install the update again.`, - undefined, - 'install' - ) - return - } - markMacQuitAndInstallInFlight() // Set this BEFORE anything else so the `activate` handler in index.ts @@ -633,6 +609,28 @@ async function performQuitAndInstall(): Promise { await runBeforeUpdateQuitCleanup() span.addEvent('pre_quit_cleanup_done') + // Why: scan after the bounded cleanup so an instance that starts or + // exits during those 2.5s cannot make the handoff decision stale. Keep + // this to one process-table read per install attempt. + const conflictingPids = await findConflictingAppInstancePids() + if (conflictingPids.length > 0) { + resetQuitForUpdateState() + recordUpdaterLifecycle( + 'quit_and_install_blocked_by_other_instances', + { pids: conflictingPids.join(','), count: conflictingPids.length }, + { + level: 'warn', + message: `Update install blocked: ${conflictingPids.length} other running instance(s) of this app (pids ${conflictingPids.join(', ')})` + } + ) + sendErrorStatus( + `Another running copy of Orca is blocking the update (PID ${conflictingPids.join(', ')}) — possibly one launched by an agent or test run. Quit it, then install the update again.`, + undefined, + 'install' + ) + return + } + recordUpdaterLifecycle('quit_and_install_invoking_native', { version: pendingVersion || null })