diff --git a/src/main/updater-conflicting-app-instances.test.ts b/src/main/updater-conflicting-app-instances.test.ts new file mode 100644 index 00000000000..2c778a78a65 --- /dev/null +++ b/src/main/updater-conflicting-app-instances.test.ts @@ -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[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/) + }) +}) diff --git a/src/main/updater-conflicting-app-instances.ts b/src/main/updater-conflicting-app-instances.ts new file mode 100644 index 00000000000..7052815ebd7 --- /dev/null +++ b/src/main/updater-conflicting-app-instances.ts @@ -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 + +function readRunningApplicationPids(executablePath: string, currentPid: number): Promise { + 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 { + 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.` +} diff --git a/src/main/updater.conflicting-instance-install.test.ts b/src/main/updater.conflicting-instance-install.test.ts new file mode 100644 index 00000000000..6dc8c7db9a1 --- /dev/null +++ b/src/main/updater.conflicting-instance-install.test.ts @@ -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>() +})) + +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('./updater-conflicting-app-instances')), + findConflictingAppInstancePids: findConflictingAppInstancePidsMock +})) + +warmUpdaterModule() + +/** Asks the updater to install, then lets its deferral timer fire. */ +async function requestInstall(): Promise> { + 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): Record | undefined { + return send.mock.calls + .filter(([channel]) => channel === 'updater:status') + .map(([, status]) => status as Record) + .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((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) + }) +}) diff --git a/src/main/updater.headless-serve-install.test.ts b/src/main/updater.headless-serve-install.test.ts index bae6474cc66..885902eac1f 100644 --- a/src/main/updater.headless-serve-install.test.ts +++ b/src/main/updater.headless-serve-install.test.ts @@ -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('./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), diff --git a/src/main/updater.linux-root-package-install.test.ts b/src/main/updater.linux-root-package-install.test.ts index 01f489bd8ef..c84ab2af1b4 100644 --- a/src/main/updater.linux-root-package-install.test.ts +++ b/src/main/updater.linux-root-package-install.test.ts @@ -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('./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()) diff --git a/src/main/updater.mac-install.test.ts b/src/main/updater.mac-install.test.ts index 563b8562fd6..298dbb5cf3e 100644 --- a/src/main/updater.mac-install.test.ts +++ b/src/main/updater.mac-install.test.ts @@ -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('./updater-conflicting-app-instances')), + findConflictingAppInstancePids: vi.fn(async () => []) +})) vi.mock('./updater-changelog', () => ({ fetchChangelog: vi.fn().mockResolvedValue(null) diff --git a/src/main/updater.quit-and-install.test.ts b/src/main/updater.quit-and-install.test.ts index 0080db58991..f47d94580a5 100644 --- a/src/main/updater.quit-and-install.test.ts +++ b/src/main/updater.quit-and-install.test.ts @@ -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('./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()) diff --git a/src/main/updater/updater-install-execution.ts b/src/main/updater/updater-install-execution.ts index 257f8e4fa93..2bbda06e2f2 100644 --- a/src/main/updater/updater-install-execution.ts +++ b/src/main/updater/updater-install-execution.ts @@ -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.