fix(updater): refuse a macOS install other app instances would abort

Squirrel.Mac waits for every running instance of the bundle to exit before it
installs, and aborts if one appears mid-install. A second packaged Orca — a
manually opened copy, or an agent rig launching the app with a temp profile —
made "Restart to Update" quit the app and never relaunch, leaving the user on
the old version with no window and no explanation.

Name the blocking instances before the handoff and refuse with a retryable
error instead of quitting into an install that cannot succeed. Blockers are
identified by AppKit bundle identity, not the process table: the Orca CLI runs
from the same bundle executable under ELECTRON_RUN_AS_NODE, so path matching
would count every CLI invocation and refuse updates outright.
This commit is contained in:
Merge Sim
2026-09-07 01:48:09 -07:00
parent ffff6eaca2
commit abae7783aa
8 changed files with 452 additions and 0 deletions
@@ -0,0 +1,113 @@
import { readFileSync } from 'node:fs'
import path from 'node:path'
import { describe, expect, it, vi } from 'vitest'
import {
describeConflictingAppInstances,
findConflictingAppInstancePids,
parseRunningApplicationPids
} from './updater-conflicting-app-instances'
const APP_EXECUTABLE = '/Applications/Orca.app/Contents/MacOS/Orca'
function darwinDeps(overrides: Parameters<typeof findConflictingAppInstancePids>[0] = {}) {
return {
platform: 'darwin' as NodeJS.Platform,
executablePath: APP_EXECUTABLE,
currentPid: 100,
...overrides
}
}
describe('parseRunningApplicationPids', () => {
it('keeps pids and drops the querying process', () => {
expect(parseRunningApplicationPids('270\n100\n811\n', 100)).toEqual([270, 811])
})
it('ignores blank and non-numeric lines', () => {
expect(parseRunningApplicationPids('\n270\nnot-a-pid\n \n', 100)).toEqual([270])
})
})
describe('findConflictingAppInstancePids', () => {
it('reports other instances of this same executable', async () => {
const read = vi.fn().mockResolvedValue('270\n811\n')
expect(
await findConflictingAppInstancePids(darwinDeps({ readRunningApplicationPids: read }))
).toEqual([270, 811])
expect(read).toHaveBeenCalledWith(APP_EXECUTABLE, 100)
})
it('reports nothing when this is the only instance', async () => {
const read = vi.fn().mockResolvedValue('')
expect(
await findConflictingAppInstancePids(darwinDeps({ readRunningApplicationPids: read }))
).toEqual([])
})
it('fails open when the query throws', async () => {
const read = vi.fn().mockRejectedValue(new Error('osascript unavailable'))
expect(
await findConflictingAppInstancePids(darwinDeps({ readRunningApplicationPids: read }))
).toEqual([])
})
it('does not query off darwin, where the installers manage running instances', async () => {
const read = vi.fn().mockResolvedValue('270\n')
for (const platform of ['win32', 'linux'] as const) {
expect(
await findConflictingAppInstancePids(
darwinDeps({ platform, readRunningApplicationPids: read })
)
).toEqual([])
}
expect(read).not.toHaveBeenCalled()
})
})
describe('describeConflictingAppInstances', () => {
it('names a single blocking pid', () => {
expect(describeConflictingAppInstances([270])).toBe(
'Another copy of Orca is running (PID 270). macOS cannot replace the app while they are open — quit them, then try again.'
)
})
it('caps how many pids it lists', () => {
expect(describeConflictingAppInstances([1, 2, 3, 4, 5, 6])).toContain(
'6 other copies of Orca are running (PIDs 1, 2, 3, 4, 5, …)'
)
})
})
// Why this is a source assertion and not a behavioural one: the behaviour under
// test lives inside the AppKit query, so any test that injects a pid reader
// bypasses exactly the logic that must not regress.
describe('conflicting-instance detection strategy', () => {
const source = readFileSync(
path.join(import.meta.dirname, 'updater-conflicting-app-instances.ts'),
'utf8'
)
it('identifies blockers by bundle identity, so Orca CLI processes are not counted', () => {
// The Orca CLI runs from the SAME bundle executable under
// ELECTRON_RUN_AS_NODE, so `ps`-style matching on the executable path
// reports every CLI invocation as a blocking app instance and refuses the
// update outright on any machine that uses the CLI. AppKit gives those
// processes no bundle identity and Squirrel does not wait for them, so the
// non-null bundleIdentifier requirement is what makes this set match
// Squirrel's. Measured on a live machine: three processes shared the bundle
// executable path (the app plus two `ELECTRON_RUN_AS_NODE` CLI processes)
// and this query returned only the app.
expect(source).toContain('NSWorkspace')
expect(source).toContain('bundleIdentifier')
expect(source).toMatch(/if\s*\(\s*\n?\s*executableUrl\s*&&\s*\n?\s*bundleIdentifier/)
})
it('never enumerates blockers from the process table', () => {
expect(source).not.toMatch(/['"`]\/bin\/ps['"`]/)
expect(source).not.toMatch(/\bpgrep\b/)
})
})
@@ -0,0 +1,140 @@
import { execFile } from 'node:child_process'
// Why: Squirrel.Mac's ShipIt waits for EVERY running instance of the target
// bundle to exit before it installs, and aborts with "App Still Running Error"
// if one appears mid-install. A second packaged Orca — a manually opened copy,
// or an agent/e2e rig launching /Applications/Orca.app with a temp profile,
// often windowless and invisible — silently stalls the update forever while the
// user sees "the app quit but never relaunched, still on the old version".
// Naming those instances before the install handoff turns a silent wedge into
// an actionable message.
const RUNNING_APPLICATION_QUERY_TIMEOUT_MS = 2_000
const RUNNING_APPLICATION_QUERY_MAX_BYTES = 64 * 1024
// Why AppKit and not `ps`: the Orca CLI runs from this same executable under
// ELECTRON_RUN_AS_NODE, so an exact-executable process scan reports every CLI
// invocation as a blocker and refuses updates on any machine that uses the CLI.
// AppKit gives those processes no bundle identity, and Squirrel does not wait
// for them either — so requiring a non-null bundleIdentifier matches Squirrel's
// own blocking set. See the regression in the sibling test file.
const RUNNING_APPLICATION_QUERY = String.raw`
function run(argv) {
ObjC.import('AppKit')
const executablePath = argv[0]
const currentPid = Number(argv[1])
const applications = $.NSWorkspace.sharedWorkspace.runningApplications
const pids = []
for (let index = 0; index < applications.count; index += 1) {
const application = applications.objectAtIndex(index)
const executableUrl = application.executableURL
const bundleIdentifier = application.bundleIdentifier
const pid = Number(application.processIdentifier)
if (
executableUrl &&
bundleIdentifier &&
String(ObjC.unwrap(executableUrl.path)) === executablePath &&
pid !== currentPid
) {
pids.push(String(pid))
}
}
return pids.join('\n')
}`
export type RunningApplicationPidReader = (
executablePath: string,
currentPid: number
) => Promise<string>
function readRunningApplicationPids(executablePath: string, currentPid: number): Promise<string> {
return new Promise((resolve, reject) => {
// Why: one bounded subprocess instead of a probe per candidate. Paths go
// through argv so nothing is interpolated into the script, and NSWorkspace
// metadata needs no Accessibility permission.
execFile(
'/usr/bin/osascript',
[
'-l',
'JavaScript',
'-e',
RUNNING_APPLICATION_QUERY,
'--',
executablePath,
String(currentPid)
],
{
encoding: 'utf8',
timeout: RUNNING_APPLICATION_QUERY_TIMEOUT_MS,
maxBuffer: RUNNING_APPLICATION_QUERY_MAX_BYTES
},
(error, stdout) => {
if (error) {
reject(error)
return
}
resolve(stdout)
}
)
})
}
export function parseRunningApplicationPids(output: string, currentPid: number): number[] {
const pids: number[] = []
for (const line of output.split('\n')) {
const normalized = line.trim()
if (!/^\d+$/.test(normalized)) {
continue
}
const pid = Number(normalized)
if (pid > 0 && pid !== currentPid) {
pids.push(pid)
}
}
return pids
}
export type ConflictingInstanceDeps = {
platform?: NodeJS.Platform
executablePath?: string
currentPid?: number
readRunningApplicationPids?: RunningApplicationPidReader
}
/**
* Pids of other running app instances launched from this same executable.
*
* macOS-only by design: this mirrors Squirrel.Mac's pre-install wait/abort
* semantics, and the Windows and Linux installers manage running instances
* themselves. Fails open — an unavailable query must never block an install.
*/
export async function findConflictingAppInstancePids(
deps: ConflictingInstanceDeps = {}
): Promise<number[]> {
if ((deps.platform ?? process.platform) !== 'darwin') {
return []
}
const executablePath = deps.executablePath ?? process.execPath
const currentPid = deps.currentPid ?? process.pid
try {
const output = await (deps.readRunningApplicationPids ?? readRunningApplicationPids)(
executablePath,
currentPid
)
return parseRunningApplicationPids(output, currentPid)
} catch {
return []
}
}
const MAX_REPORTED_CONFLICTING_PIDS = 5
/** Actionable copy for the update card: what is blocking, and what to do. */
export function describeConflictingAppInstances(pids: readonly number[]): string {
const reported = pids.slice(0, MAX_REPORTED_CONFLICTING_PIDS)
const suffix = pids.length > reported.length ? ', …' : ''
const subject =
pids.length === 1
? `Another copy of Orca is running (PID ${reported[0]})`
: `${pids.length} other copies of Orca are running (PIDs ${reported.join(', ')}${suffix})`
return `${subject}. macOS cannot replace the app while they are open — quit them, then try again.`
}
@@ -0,0 +1,134 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { loadUpdaterModule, warmUpdaterModule } from './updater-test-module-loader'
import type * as ConflictingAppInstances from './updater-conflicting-app-instances'
type ConflictingAppInstancesModule = typeof ConflictingAppInstances
const { autoUpdaterMock, killAllPtyMock, moduleFactories, resetUpdaterMocks } = await vi.hoisted(
async () => (await import('./updater-test-harness')).createUpdaterMocks()
)
const { findConflictingAppInstancePidsMock } = vi.hoisted(() => ({
findConflictingAppInstancePidsMock: vi.fn<() => Promise<number[]>>()
}))
vi.mock('electron', () => moduleFactories.electron())
vi.mock('electron-updater', () => moduleFactories.electronUpdater())
vi.mock('./electron-updater-loader', () => moduleFactories.electronUpdaterLoader())
vi.mock('@electron-toolkit/utils', () => moduleFactories.electronToolkitUtils())
vi.mock('./ipc/pty', () => moduleFactories.ipcPty())
vi.mock('./linux-update-package-type', () => moduleFactories.linuxUpdatePackageType())
vi.mock('./updater-lifecycle-diagnostics', () => moduleFactories.updaterLifecycleDiagnostics())
vi.mock('./updater-changelog', () => moduleFactories.updaterChangelog())
vi.mock('./updater-nudge', () => moduleFactories.updaterNudge())
vi.mock('./update-install-exit-watchdog', () => moduleFactories.updateInstallExitWatchdog())
vi.mock('./updater-prerelease-feed', () => moduleFactories.updaterPrereleaseFeed())
vi.mock('./local-builds/local-build-switch', () => moduleFactories.localBuildSwitch())
vi.mock('./local-builds/local-build-feed-server', () => moduleFactories.localBuildFeedServer())
vi.mock('./startup/hydrate-shell-path', () => ({
runWithLaunchPath: (action: () => unknown): unknown => action()
}))
// Why partial: the message builder stays real, so a copy change cannot make
// these pass against text no user would ever see.
vi.mock('./updater-conflicting-app-instances', async () => ({
...(await vi.importActual<ConflictingAppInstancesModule>('./updater-conflicting-app-instances')),
findConflictingAppInstancePids: findConflictingAppInstancePidsMock
}))
warmUpdaterModule()
/** Asks the updater to install, then lets its deferral timer fire. */
async function requestInstall(): Promise<ReturnType<typeof vi.fn>> {
const send = vi.fn()
const { setupAutoUpdater, quitAndInstall } = await loadUpdaterModule()
setupAutoUpdater({ webContents: { send } } as never)
quitAndInstall()
await vi.advanceTimersByTimeAsync(100)
return send
}
function lastErrorStatus(send: ReturnType<typeof vi.fn>): Record<string, unknown> | undefined {
return send.mock.calls
.filter(([channel]) => channel === 'updater:status')
.map(([, status]) => status as Record<string, unknown>)
.findLast((status) => status.state === 'error')
}
describe('macOS install blocked by other running app instances', () => {
beforeEach(() => {
resetUpdaterMocks()
findConflictingAppInstancePidsMock.mockReset().mockResolvedValue([])
vi.useFakeTimers()
})
afterEach(() => {
vi.useRealTimers()
})
it('installs normally when this is the only running copy', async () => {
const send = await requestInstall()
expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1)
expect(lastErrorStatus(send)).toBeUndefined()
})
it('refuses the install instead of quitting into a handoff Squirrel will abort', async () => {
findConflictingAppInstancePidsMock.mockResolvedValue([270, 811])
const send = await requestInstall()
expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled()
// Killing PTYs is destructive and only earns its keep once the install commits.
expect(killAllPtyMock).not.toHaveBeenCalled()
expect(send).toHaveBeenCalledWith('updater:quitAndInstallAborted')
})
it('names the copies to quit and keeps the update retryable', async () => {
findConflictingAppInstancePidsMock.mockResolvedValue([270, 811])
const send = await requestInstall()
expect(lastErrorStatus(send)).toMatchObject({ state: 'error', retryable: true })
expect(String(lastErrorStatus(send)?.message)).toContain('270, 811')
})
it('leaves the update installable after a refusal, once the other copy quits', async () => {
findConflictingAppInstancePidsMock.mockResolvedValue([270])
const { setupAutoUpdater, quitAndInstall } = await loadUpdaterModule()
setupAutoUpdater({ webContents: { send: vi.fn() } } as never)
quitAndInstall()
await vi.advanceTimersByTimeAsync(100)
expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled()
// Why this matters: the refusal must release the in-progress claim, or the
// retry the message asks for would be swallowed as a duplicate request.
findConflictingAppInstancePidsMock.mockResolvedValue([])
quitAndInstall()
await vi.advanceTimersByTimeAsync(100)
expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1)
})
it('ignores a duplicate install request that arrives during the conflict scan', async () => {
let releaseScan: (pids: number[]) => void = () => {}
findConflictingAppInstancePidsMock.mockReturnValue(
new Promise<number[]>((resolve) => {
releaseScan = resolve
})
)
const { setupAutoUpdater, quitAndInstall } = await loadUpdaterModule()
setupAutoUpdater({ webContents: { send: vi.fn() } } as never)
quitAndInstall()
await vi.advanceTimersByTimeAsync(100)
// The scan is the first await in the install, so a second request lands
// inside a window where no install state has been set yet.
quitAndInstall()
await vi.advanceTimersByTimeAsync(100)
releaseScan([])
await vi.advanceTimersByTimeAsync(100)
expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1)
})
})
@@ -1,5 +1,8 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { loadUpdaterModule, warmUpdaterModule } from './updater-test-module-loader'
import type * as ConflictingAppInstances from './updater-conflicting-app-instances'
type ConflictingAppInstancesModule = typeof ConflictingAppInstances
const {
appMock,
@@ -84,6 +87,11 @@ vi.mock('./linux-update-package-type', () => ({
}))
vi.mock('@electron-toolkit/utils', () => ({ is: { dev: false } }))
vi.mock('./ipc/pty', () => ({ killAllPty: killAllPtyMock }))
// The real detector shells out to osascript; keep unit tests off the live machine.
vi.mock('./updater-conflicting-app-instances', async () => ({
...(await vi.importActual<ConflictingAppInstancesModule>('./updater-conflicting-app-instances')),
findConflictingAppInstancePids: vi.fn(async () => [])
}))
vi.mock('./updater-changelog', () => ({ fetchChangelog: vi.fn().mockResolvedValue(null) }))
vi.mock('./updater-nudge', () => ({
fetchNudge: vi.fn().mockResolvedValue(null),
@@ -4,6 +4,9 @@ import { tmpdir } from 'node:os'
import type * as UpdaterModule from './updater'
import type { LinuxRootPackageType, UpdateStatus } from '../shared/update-status-types'
import { loadUpdaterModule, warmUpdaterModule } from './updater-test-module-loader'
import type * as ConflictingAppInstances from './updater-conflicting-app-instances'
type ConflictingAppInstancesModule = typeof ConflictingAppInstances
const {
browserWindowMock,
@@ -23,6 +26,11 @@ vi.mock('electron-updater', () => moduleFactories.electronUpdater())
vi.mock('./electron-updater-loader', () => moduleFactories.electronUpdaterLoader())
vi.mock('@electron-toolkit/utils', () => moduleFactories.electronToolkitUtils())
vi.mock('./ipc/pty', () => moduleFactories.ipcPty())
// The real detector shells out to osascript; keep unit tests off the live machine.
vi.mock('./updater-conflicting-app-instances', async () => ({
...(await vi.importActual<ConflictingAppInstancesModule>('./updater-conflicting-app-instances')),
findConflictingAppInstancePids: vi.fn(async () => [])
}))
vi.mock('./linux-update-package-type', () => moduleFactories.linuxUpdatePackageType())
vi.mock('./updater-lifecycle-diagnostics', () => moduleFactories.updaterLifecycleDiagnostics())
vi.mock('./updater-changelog', () => moduleFactories.updaterChangelog())
+8
View File
@@ -1,5 +1,8 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { loadUpdaterModule, warmUpdaterModule } from './updater-test-module-loader'
import type * as ConflictingAppInstances from './updater-conflicting-app-instances'
type ConflictingAppInstancesModule = typeof ConflictingAppInstances
const {
appMock,
@@ -108,6 +111,11 @@ vi.mock('@electron-toolkit/utils', () => ({
vi.mock('./ipc/pty', () => ({
killAllPty: killAllPtyMock
}))
// The real detector shells out to osascript; keep unit tests off the live machine.
vi.mock('./updater-conflicting-app-instances', async () => ({
...(await vi.importActual<ConflictingAppInstancesModule>('./updater-conflicting-app-instances')),
findConflictingAppInstancePids: vi.fn(async () => [])
}))
vi.mock('./updater-changelog', () => ({
fetchChangelog: vi.fn().mockResolvedValue(null)
@@ -1,6 +1,9 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { PRE_COMMIT_INSTALL_FAILURE } from './updater-test-harness'
import { loadUpdaterModule, warmUpdaterModule } from './updater-test-module-loader'
import type * as ConflictingAppInstances from './updater-conflicting-app-instances'
type ConflictingAppInstancesModule = typeof ConflictingAppInstances
const {
nativeUpdaterMock,
@@ -22,6 +25,11 @@ vi.mock('electron-updater', () => moduleFactories.electronUpdater())
vi.mock('./electron-updater-loader', () => moduleFactories.electronUpdaterLoader())
vi.mock('@electron-toolkit/utils', () => moduleFactories.electronToolkitUtils())
vi.mock('./ipc/pty', () => moduleFactories.ipcPty())
// The real detector shells out to osascript; keep unit tests off the live machine.
vi.mock('./updater-conflicting-app-instances', async () => ({
...(await vi.importActual<ConflictingAppInstancesModule>('./updater-conflicting-app-instances')),
findConflictingAppInstancePids: vi.fn(async () => [])
}))
vi.mock('./linux-update-package-type', () => moduleFactories.linuxUpdatePackageType())
vi.mock('./updater-lifecycle-diagnostics', () => moduleFactories.updaterLifecycleDiagnostics())
vi.mock('./updater-changelog', () => moduleFactories.updaterChangelog())
@@ -7,6 +7,10 @@ import { armUpdateInstallExitWatchdog } from '../update-install-exit-watchdog'
import { getLinuxPackageType } from '../linux-update-package-type'
import { LINUX_PACKAGE_MARKER_UNUSABLE_MESSAGE } from '../linux-package-downloaded-status'
import { recordUpdaterLifecycle } from '../updater-lifecycle-diagnostics'
import {
describeConflictingAppInstances,
findConflictingAppInstancePids
} from '../updater-conflicting-app-instances'
import { requestServeUpdateHandoff, failServeUpdateHandoff } from '../serve-update-handoff'
import { UpdaterPackageRecovery } from './updater-package-recovery'
@@ -51,8 +55,37 @@ export abstract class UpdaterInstallExecution extends UpdaterPackageRecovery {
})
return
}
// Why the flag is claimed before the await: the conflict scan is the first
// asynchronous step in this method, and a duplicate install request landing
// inside that window would otherwise pass the in-progress check above and
// run a second handoff.
this.quitAndInstallInProgress = true
// Why here, before any other install state is set: Squirrel.Mac waits for
// every running instance of this bundle to exit and aborts if one appears
// mid-install, so quitting into a doomed handoff strands the user on the
// old version with no window and no explanation.
const conflictingInstancePids = await findConflictingAppInstancePids()
if (conflictingInstancePids.length > 0) {
// Nothing else has been armed yet, so releasing the claim is the whole rollback.
this.quitAndInstallInProgress = false
recordUpdaterLifecycle(
'quit_and_install_blocked_by_other_instances',
{ version: pendingVersion || null, instanceCount: conflictingInstancePids.length },
{ level: 'warn', message: 'Other running app instances would abort the macOS install' }
)
// The preload prepares renderer state before invoking; release it when main refuses.
this.mainWindowRef?.webContents.send('updater:quitAndInstallAborted')
this.sendInstallFailureStatus({
state: 'error',
message: describeConflictingAppInstances(conflictingInstancePids),
// The staged update is untouched; this is the user's to clear and retry.
retryable: true,
...(pendingVersion ? { version: pendingVersion } : {})
})
return
}
markMacQuitAndInstallInFlight()
// Set BEFORE anything else so the `activate` handler doesn't reopen the old version while ShipIt replaces the .app bundle.