fix(updater): close install handoff races

This commit is contained in:
Brennan Benson
2026-07-16 18:02:43 -07:00
parent d6d2e4c48f
commit 2bc1d681be
6 changed files with 103 additions and 50 deletions
+20 -20
View File
@@ -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.
@@ -70,7 +70,7 @@ describe('shouldDeferLaunchForUpdateInstall', () => {
expect(defer).toBe(true)
expect(execFileSyncMock).toHaveBeenCalledWith(
'ps',
'/bin/ps',
['-axo', 'command='],
expect.objectContaining({ encoding: 'utf8' })
)
@@ -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',
@@ -23,8 +23,9 @@ function readCurrentUserProcessList(): Promise<string> {
// 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) => {
+52
View File
@@ -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<void>((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 () => {
+22 -24
View File
@@ -582,30 +582,6 @@ async function performQuitAndInstall(): Promise<void> {
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<void> {
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
})