diff --git a/src/main/updater-conflicting-app-instances.test.ts b/src/main/updater-conflicting-app-instances.test.ts index 2c778a78a65..4de428f68fc 100644 --- a/src/main/updater-conflicting-app-instances.test.ts +++ b/src/main/updater-conflicting-app-instances.test.ts @@ -4,7 +4,8 @@ import { describe, expect, it, vi } from 'vitest' import { describeConflictingAppInstances, findConflictingAppInstancePids, - parseRunningApplicationPids + parseRunningApplicationPids, + runningApplicationQueryOutput } from './updater-conflicting-app-instances' const APP_EXECUTABLE = '/Applications/Orca.app/Contents/MacOS/Orca' @@ -68,6 +69,30 @@ describe('findConflictingAppInstancePids', () => { }) }) +describe('runningApplicationQueryOutput', () => { + // Fail-open is the property that keeps a broken probe from blocking updates, + // and the runner reports these as data rather than throwing — so each one is a + // path that would otherwise look like a successful "no blockers" answer, or + // worse, like a partial list of them. + it('passes through the output of a query that exited cleanly', () => { + expect(runningApplicationQueryOutput({ timedOut: false, code: 0, stdout: '270\n' })).toBe( + '270\n' + ) + }) + + it('discards partial output from a timed-out query', () => { + expect(runningApplicationQueryOutput({ timedOut: true, code: null, stdout: '270\n' })).toBe('') + }) + + it('discards output from a query that exited non-zero', () => { + expect(runningApplicationQueryOutput({ timedOut: false, code: 1, stdout: '270\n' })).toBe('') + }) + + it('discards output from a query killed by a signal', () => { + expect(runningApplicationQueryOutput({ timedOut: false, code: null, stdout: '270\n' })).toBe('') + }) +}) + describe('describeConflictingAppInstances', () => { it('names a single blocking pid', () => { expect(describeConflictingAppInstances([270])).toBe( @@ -110,4 +135,11 @@ describe('conflicting-instance detection strategy', () => { expect(source).not.toMatch(/['"`]\/bin\/ps['"`]/) expect(source).not.toMatch(/\bpgrep\b/) }) + + it('spawns through the shared runner, not node:child_process', () => { + // The tree-level guard in src/shared/child-process owns this rule; asserting + // it here too keeps the reason next to the code that has to obey it. + expect(source).not.toContain('node:child_process') + expect(source).toContain("from '../shared/child-process/run-process'") + }) }) diff --git a/src/main/updater-conflicting-app-instances.ts b/src/main/updater-conflicting-app-instances.ts index 7052815ebd7..ada074e1a2a 100644 --- a/src/main/updater-conflicting-app-instances.ts +++ b/src/main/updater-conflicting-app-instances.ts @@ -1,4 +1,4 @@ -import { execFile } from 'node:child_process' +import { runProcess } from '../shared/child-process/run-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" @@ -46,36 +46,44 @@ export type RunningApplicationPidReader = ( 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) - } - ) +async function readRunningApplicationPids( + executablePath: string, + currentPid: number +): Promise { + // 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. + const result = await runProcess({ + program: '/usr/bin/osascript', + args: [ + '-l', + 'JavaScript', + '-e', + RUNNING_APPLICATION_QUERY, + '--', + executablePath, + String(currentPid) + ], + timeoutMs: RUNNING_APPLICATION_QUERY_TIMEOUT_MS, + maxOutputBytes: RUNNING_APPLICATION_QUERY_MAX_BYTES }) + return runningApplicationQueryOutput(result) +} + +/** + * Output only from a query that actually finished. + * + * Why it is a decision at all: `runProcess` reports a non-zero exit or a timeout + * as data rather than throwing, so partial stdout arrives looking like an answer. + * A probe that could not finish has proved nothing about who is running, and + * naming a blocker on that basis would refuse an install the user could have had. + */ +export function runningApplicationQueryOutput(result: { + timedOut: boolean + code: number | null + stdout: string +}): string { + return result.timedOut || result.code !== 0 ? '' : result.stdout } export function parseRunningApplicationPids(output: string, currentPid: number): number[] {