mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 00:03:20 +00:00
fix(updater): close remaining install guard races
This commit is contained in:
+1
-1
@@ -452,7 +452,7 @@ configureOrcaUserDataPathEnv()
|
||||
if (
|
||||
shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: app.isPackaged,
|
||||
userDataPath: app.getPath('userData'),
|
||||
appDataPath: app.getPath('appData'),
|
||||
appVersion: app.getVersion()
|
||||
})
|
||||
) {
|
||||
|
||||
@@ -1,11 +1,12 @@
|
||||
import { mkdtempSync, existsSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { mkdtempSync, existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import path from 'node:path'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
UPDATE_INSTALL_HANDOFF_MARKER_FILE,
|
||||
UPDATE_INSTALL_HANDOFF_ARMING_MS,
|
||||
UPDATE_INSTALL_HANDOFF_MAX_AGE_MS,
|
||||
clearUpdateInstallHandoffMarker,
|
||||
getUpdateInstallHandoffMarkerPath,
|
||||
isBundleShipItRunning,
|
||||
shouldDeferLaunchForUpdateInstall,
|
||||
writeUpdateInstallHandoffMarker
|
||||
@@ -20,14 +21,14 @@ const SHIPIT_ROW = `${SHIPIT_PATH} com.stablyai.orca.ShipIt`
|
||||
|
||||
const tempDirs: string[] = []
|
||||
|
||||
function makeUserData(): string {
|
||||
function makeAppData(): string {
|
||||
const dir = mkdtempSync(path.join(tmpdir(), 'orca-launch-gate-'))
|
||||
tempDirs.push(dir)
|
||||
return dir
|
||||
}
|
||||
|
||||
function markerFile(userData: string): string {
|
||||
return path.join(userData, UPDATE_INSTALL_HANDOFF_MARKER_FILE)
|
||||
function markerFile(appData: string, executablePath = APP_BIN): string {
|
||||
return getUpdateInstallHandoffMarkerPath(appData, executablePath)
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
@@ -57,15 +58,20 @@ describe('isBundleShipItRunning', () => {
|
||||
|
||||
describe('shouldDeferLaunchForUpdateInstall', () => {
|
||||
it('reads all users so a privileged ShipIt install is visible', () => {
|
||||
const userData = makeUserData()
|
||||
writeUpdateInstallHandoffMarker(userData, '1.0.51')
|
||||
const appData = makeAppData()
|
||||
const createdAt = 1_000_000
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51', createdAt)
|
||||
execFileSyncMock.mockReturnValue(SHIPIT_ROW)
|
||||
|
||||
const defer = shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
userDataPath: userData,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: { platform: 'darwin', executablePath: APP_BIN }
|
||||
deps: {
|
||||
platform: 'darwin',
|
||||
executablePath: APP_BIN,
|
||||
now: createdAt + UPDATE_INSTALL_HANDOFF_ARMING_MS + 1
|
||||
}
|
||||
})
|
||||
|
||||
expect(defer).toBe(true)
|
||||
@@ -76,59 +82,134 @@ describe('shouldDeferLaunchForUpdateInstall', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('defers a same-version launch while this bundle’s installer is alive', () => {
|
||||
const userData = makeUserData()
|
||||
writeUpdateInstallHandoffMarker(userData, '1.0.51')
|
||||
it('defers during marker arming without waiting for a process-table read', () => {
|
||||
const appData = makeAppData()
|
||||
const createdAt = 1_000_000
|
||||
const readProcessList = vi.fn(() => '/usr/bin/ps')
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51', createdAt)
|
||||
|
||||
const defer = shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
userDataPath: userData,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: { platform: 'darwin', executablePath: APP_BIN, readProcessList: () => SHIPIT_ROW }
|
||||
deps: {
|
||||
platform: 'darwin',
|
||||
executablePath: APP_BIN,
|
||||
now: createdAt + UPDATE_INSTALL_HANDOFF_ARMING_MS,
|
||||
readProcessList
|
||||
}
|
||||
})
|
||||
|
||||
expect(defer).toBe(true)
|
||||
expect(existsSync(markerFile(userData))).toBe(true)
|
||||
expect(readProcessList).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('lets the post-update relaunch through and clears the marker', () => {
|
||||
const userData = makeUserData()
|
||||
writeUpdateInstallHandoffMarker(userData, '1.0.51')
|
||||
it('defers a same-version launch while this bundle’s installer is alive', () => {
|
||||
const appData = makeAppData()
|
||||
const createdAt = 1_000_000
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51', createdAt)
|
||||
|
||||
const defer = shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
userDataPath: userData,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: {
|
||||
platform: 'darwin',
|
||||
executablePath: APP_BIN,
|
||||
now: createdAt + UPDATE_INSTALL_HANDOFF_ARMING_MS + 1,
|
||||
readProcessList: () => SHIPIT_ROW
|
||||
}
|
||||
})
|
||||
|
||||
expect(defer).toBe(true)
|
||||
expect(existsSync(markerFile(appData))).toBe(true)
|
||||
})
|
||||
|
||||
it('shares the bundle-scoped marker across isolated userData profiles', () => {
|
||||
const appData = makeAppData()
|
||||
const createdAt = 1_000_000
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51', createdAt)
|
||||
|
||||
const deferFromIsolatedProfile = shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: {
|
||||
platform: 'darwin',
|
||||
executablePath: APP_BIN,
|
||||
now: createdAt + 1,
|
||||
readProcessList: () => '/usr/bin/ps'
|
||||
}
|
||||
})
|
||||
|
||||
expect(deferFromIsolatedProfile).toBe(true)
|
||||
})
|
||||
|
||||
it('does not share markers between side-by-side app bundles', () => {
|
||||
const appData = makeAppData()
|
||||
const otherAppBin = '/Applications/Orca Preview.app/Contents/MacOS/Orca'
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51', 1_000_000)
|
||||
|
||||
expect(
|
||||
shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: {
|
||||
platform: 'darwin',
|
||||
executablePath: otherAppBin,
|
||||
now: 1_000_001,
|
||||
readProcessList: () => SHIPIT_ROW
|
||||
}
|
||||
})
|
||||
).toBe(false)
|
||||
expect(existsSync(markerFile(appData))).toBe(true)
|
||||
})
|
||||
|
||||
it('lets the post-update relaunch through and clears the marker', () => {
|
||||
const appData = makeAppData()
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51')
|
||||
|
||||
const defer = shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.61',
|
||||
deps: { platform: 'darwin', executablePath: APP_BIN, readProcessList: () => SHIPIT_ROW }
|
||||
})
|
||||
|
||||
expect(defer).toBe(false)
|
||||
expect(existsSync(markerFile(userData))).toBe(false)
|
||||
expect(existsSync(markerFile(appData))).toBe(false)
|
||||
})
|
||||
|
||||
it('launches normally once the installer has exited (aborted/failed install)', () => {
|
||||
const userData = makeUserData()
|
||||
writeUpdateInstallHandoffMarker(userData, '1.0.51')
|
||||
const appData = makeAppData()
|
||||
const createdAt = 1_000_000
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51', createdAt)
|
||||
|
||||
const defer = shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
userDataPath: userData,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: { platform: 'darwin', executablePath: APP_BIN, readProcessList: () => '/usr/bin/ps' }
|
||||
deps: {
|
||||
platform: 'darwin',
|
||||
executablePath: APP_BIN,
|
||||
now: createdAt + UPDATE_INSTALL_HANDOFF_ARMING_MS + 1,
|
||||
readProcessList: () => '/usr/bin/ps'
|
||||
}
|
||||
})
|
||||
|
||||
expect(defer).toBe(false)
|
||||
expect(existsSync(markerFile(userData))).toBe(false)
|
||||
expect(existsSync(markerFile(appData))).toBe(false)
|
||||
})
|
||||
|
||||
it('expires stale markers instead of gating launches', () => {
|
||||
const userData = makeUserData()
|
||||
const appData = makeAppData()
|
||||
const staleCreatedAt = 1_000_000
|
||||
writeUpdateInstallHandoffMarker(userData, '1.0.51', staleCreatedAt)
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51', staleCreatedAt)
|
||||
|
||||
const defer = shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
userDataPath: userData,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: {
|
||||
platform: 'darwin',
|
||||
@@ -139,30 +220,33 @@ describe('shouldDeferLaunchForUpdateInstall', () => {
|
||||
})
|
||||
|
||||
expect(defer).toBe(false)
|
||||
expect(existsSync(markerFile(userData))).toBe(false)
|
||||
expect(existsSync(markerFile(appData))).toBe(false)
|
||||
})
|
||||
|
||||
it('fails open on corrupt markers, unreadable process tables, and other platforms', () => {
|
||||
const userData = makeUserData()
|
||||
writeFileSync(markerFile(userData), 'not json')
|
||||
const appData = makeAppData()
|
||||
mkdirSync(path.dirname(markerFile(appData)), { recursive: true })
|
||||
writeFileSync(markerFile(appData), 'not json')
|
||||
expect(
|
||||
shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
userDataPath: userData,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: { platform: 'darwin', executablePath: APP_BIN, readProcessList: () => SHIPIT_ROW }
|
||||
})
|
||||
).toBe(false)
|
||||
|
||||
writeUpdateInstallHandoffMarker(userData, '1.0.51')
|
||||
const createdAt = 1_000_000
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51', createdAt)
|
||||
expect(
|
||||
shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
userDataPath: userData,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: {
|
||||
platform: 'darwin',
|
||||
executablePath: APP_BIN,
|
||||
now: createdAt + UPDATE_INSTALL_HANDOFF_ARMING_MS + 1,
|
||||
readProcessList: () => {
|
||||
throw new Error('ps timed out')
|
||||
}
|
||||
@@ -170,11 +254,11 @@ describe('shouldDeferLaunchForUpdateInstall', () => {
|
||||
})
|
||||
).toBe(false)
|
||||
|
||||
writeUpdateInstallHandoffMarker(userData, '1.0.51')
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51')
|
||||
expect(
|
||||
shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
userDataPath: userData,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: { platform: 'win32', executablePath: APP_BIN, readProcessList: () => SHIPIT_ROW }
|
||||
})
|
||||
@@ -183,17 +267,32 @@ describe('shouldDeferLaunchForUpdateInstall', () => {
|
||||
expect(
|
||||
shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: false,
|
||||
userDataPath: userData,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: { platform: 'darwin', executablePath: APP_BIN, readProcessList: () => SHIPIT_ROW }
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('fails open and clears a marker timestamped in the future', () => {
|
||||
const appData = makeAppData()
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51', 2_000_000)
|
||||
|
||||
expect(
|
||||
shouldDeferLaunchForUpdateInstall({
|
||||
isPackaged: true,
|
||||
appDataPath: appData,
|
||||
appVersion: '1.0.51',
|
||||
deps: { platform: 'darwin', executablePath: APP_BIN, now: 1_000_000 }
|
||||
})
|
||||
).toBe(false)
|
||||
expect(existsSync(markerFile(appData))).toBe(false)
|
||||
})
|
||||
|
||||
it('clearUpdateInstallHandoffMarker removes the marker', () => {
|
||||
const userData = makeUserData()
|
||||
writeUpdateInstallHandoffMarker(userData, '1.0.51')
|
||||
clearUpdateInstallHandoffMarker(userData)
|
||||
expect(existsSync(markerFile(userData))).toBe(false)
|
||||
const appData = makeAppData()
|
||||
writeUpdateInstallHandoffMarker(appData, APP_BIN, '1.0.51')
|
||||
clearUpdateInstallHandoffMarker(appData, APP_BIN)
|
||||
expect(existsSync(markerFile(appData))).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import { existsSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { createHash } from 'node:crypto'
|
||||
import { existsSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import path from 'node:path'
|
||||
|
||||
// Why: Squirrel.Mac installs by swapping the bundle AFTER the app exits, and
|
||||
@@ -11,7 +12,11 @@ import path from 'node:path'
|
||||
// launch that finds the marker fresh while this bundle's ShipIt installer is
|
||||
// still alive exits instead — ShipIt then finishes and relaunches the updated
|
||||
// app itself, so the user's click still ends with Orca open.
|
||||
export const UPDATE_INSTALL_HANDOFF_MARKER_FILE = 'update-install-handoff.json'
|
||||
export const UPDATE_INSTALL_HANDOFF_MARKER_DIRECTORY = 'orca-update-install-handoffs'
|
||||
// Why: quitAndInstall writes the marker immediately before spawning ShipIt.
|
||||
// During this bounded arming window, the marker is authoritative because the
|
||||
// installer may not be visible in the process table yet.
|
||||
export const UPDATE_INSTALL_HANDOFF_ARMING_MS = 5_000
|
||||
// Why: the handoff normally resolves in seconds; the ceiling only bounds how
|
||||
// long a crashed install could keep gating launches. The live-ShipIt check is
|
||||
// the real gate, so a stale marker never locks the app out.
|
||||
@@ -25,44 +30,84 @@ export type UpdateInstallHandoffMarker = {
|
||||
createdAtMs: number
|
||||
}
|
||||
|
||||
function markerPath(userDataPath: string): string {
|
||||
return path.join(userDataPath, UPDATE_INSTALL_HANDOFF_MARKER_FILE)
|
||||
function removeFileBestEffort(filePath: string): void {
|
||||
try {
|
||||
rmSync(filePath, { force: true })
|
||||
} catch {
|
||||
// Marker cleanup must never prevent an install or a normal app launch.
|
||||
}
|
||||
}
|
||||
|
||||
function bundleRootForExecutable(executablePath: string): string {
|
||||
// Why: these paths always describe a macOS bundle, even in cross-platform
|
||||
// unit tests, so host-native separators would change the identity.
|
||||
return path.posix.resolve(executablePath, '..', '..', '..')
|
||||
}
|
||||
|
||||
export function getUpdateInstallHandoffMarkerPath(
|
||||
appDataPath: string,
|
||||
executablePath: string
|
||||
): string {
|
||||
// Why: appData is shared by isolated Orca profiles, while the executable
|
||||
// hash keeps side-by-side app bundles from gating each other's launches.
|
||||
const bundleId = createHash('sha256')
|
||||
.update(bundleRootForExecutable(executablePath))
|
||||
.digest('hex')
|
||||
.slice(0, 16)
|
||||
return path.join(appDataPath, UPDATE_INSTALL_HANDOFF_MARKER_DIRECTORY, `${bundleId}.json`)
|
||||
}
|
||||
|
||||
/** Written by the updater immediately before the native install handoff. */
|
||||
export function writeUpdateInstallHandoffMarker(
|
||||
userDataPath: string,
|
||||
appDataPath: string,
|
||||
executablePath: string,
|
||||
appVersion: string,
|
||||
now = Date.now()
|
||||
): void {
|
||||
const finalPath = getUpdateInstallHandoffMarkerPath(appDataPath, executablePath)
|
||||
const temporaryPath = `${finalPath}.${process.pid}.${now}.tmp`
|
||||
try {
|
||||
const marker: UpdateInstallHandoffMarker = { appVersion, createdAtMs: now }
|
||||
writeFileSync(markerPath(userDataPath), JSON.stringify(marker))
|
||||
mkdirSync(path.dirname(finalPath), { recursive: true })
|
||||
writeFileSync(temporaryPath, JSON.stringify(marker))
|
||||
// Why: a direct overwrite can expose partial JSON to the launch racing
|
||||
// this write. Same-directory rename publishes the complete marker at once.
|
||||
renameSync(temporaryPath, finalPath)
|
||||
} catch {
|
||||
// Why: the marker only powers the relaunch-race guard; failing to write
|
||||
// it must never block the install itself.
|
||||
} finally {
|
||||
removeFileBestEffort(temporaryPath)
|
||||
}
|
||||
}
|
||||
|
||||
export function clearUpdateInstallHandoffMarker(userDataPath: string): void {
|
||||
try {
|
||||
rmSync(markerPath(userDataPath), { force: true })
|
||||
} catch {
|
||||
/* Removal is best-effort; a stale marker self-expires. */
|
||||
}
|
||||
export function clearUpdateInstallHandoffMarker(appDataPath: string, executablePath: string): void {
|
||||
removeFileBestEffort(getUpdateInstallHandoffMarkerPath(appDataPath, executablePath))
|
||||
}
|
||||
|
||||
function readUpdateInstallHandoffMarker(userDataPath: string): UpdateInstallHandoffMarker | null {
|
||||
function readUpdateInstallHandoffMarker(
|
||||
appDataPath: string,
|
||||
executablePath: string
|
||||
): UpdateInstallHandoffMarker | null {
|
||||
const filePath = getUpdateInstallHandoffMarkerPath(appDataPath, executablePath)
|
||||
try {
|
||||
if (!existsSync(markerPath(userDataPath))) {
|
||||
if (!existsSync(filePath)) {
|
||||
return null
|
||||
}
|
||||
const parsed = JSON.parse(readFileSync(markerPath(userDataPath), 'utf8'))
|
||||
if (typeof parsed?.appVersion !== 'string' || typeof parsed?.createdAtMs !== 'number') {
|
||||
const parsed = JSON.parse(readFileSync(filePath, 'utf8'))
|
||||
if (
|
||||
typeof parsed?.appVersion !== 'string' ||
|
||||
typeof parsed?.createdAtMs !== 'number' ||
|
||||
!Number.isFinite(parsed.createdAtMs)
|
||||
) {
|
||||
removeFileBestEffort(filePath)
|
||||
return null
|
||||
}
|
||||
return { appVersion: parsed.appVersion, createdAtMs: parsed.createdAtMs }
|
||||
} catch {
|
||||
// Why: a corrupt marker cannot ever become actionable; removing it avoids
|
||||
// repeated startup reads while preserving the fail-open contract.
|
||||
removeFileBestEffort(filePath)
|
||||
return null
|
||||
}
|
||||
}
|
||||
@@ -82,9 +127,7 @@ 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
|
||||
// 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 bundleRoot = bundleRootForExecutable(executablePath)
|
||||
const shipItPath = path.posix.join(
|
||||
bundleRoot,
|
||||
'Contents',
|
||||
@@ -112,11 +155,12 @@ export type UpdateInstallLaunchGateDeps = {
|
||||
* Decide whether this launch should exit to let an in-flight update install
|
||||
* finish. Sync by design — it must resolve before any startup side effects,
|
||||
* and the process-table read only happens on the rare fresh-marker path.
|
||||
* Fails open everywhere: an unreadable marker or process table means launch.
|
||||
* An unreadable marker fails open; after the bounded arming phase, an
|
||||
* unreadable process table does too.
|
||||
*/
|
||||
export function shouldDeferLaunchForUpdateInstall(options: {
|
||||
isPackaged: boolean
|
||||
userDataPath: string
|
||||
appDataPath: string
|
||||
appVersion: string
|
||||
deps?: UpdateInstallLaunchGateDeps
|
||||
}): boolean {
|
||||
@@ -124,33 +168,38 @@ export function shouldDeferLaunchForUpdateInstall(options: {
|
||||
if (platform !== 'darwin' || !options.isPackaged) {
|
||||
return false
|
||||
}
|
||||
const marker = readUpdateInstallHandoffMarker(options.userDataPath)
|
||||
const executablePath = options.deps?.executablePath ?? process.execPath
|
||||
const marker = readUpdateInstallHandoffMarker(options.appDataPath, executablePath)
|
||||
if (!marker) {
|
||||
return false
|
||||
}
|
||||
// A different running version means the install already happened (or was
|
||||
// rolled elsewhere) — this launch is the post-update relaunch.
|
||||
if (marker.appVersion !== options.appVersion) {
|
||||
clearUpdateInstallHandoffMarker(options.userDataPath)
|
||||
clearUpdateInstallHandoffMarker(options.appDataPath, executablePath)
|
||||
return false
|
||||
}
|
||||
const now = options.deps?.now ?? Date.now()
|
||||
if (now - marker.createdAtMs > UPDATE_INSTALL_HANDOFF_MAX_AGE_MS) {
|
||||
clearUpdateInstallHandoffMarker(options.userDataPath)
|
||||
const markerAgeMs = now - marker.createdAtMs
|
||||
if (markerAgeMs < 0 || markerAgeMs > UPDATE_INSTALL_HANDOFF_MAX_AGE_MS) {
|
||||
clearUpdateInstallHandoffMarker(options.appDataPath, executablePath)
|
||||
return false
|
||||
}
|
||||
if (markerAgeMs <= UPDATE_INSTALL_HANDOFF_ARMING_MS) {
|
||||
return true
|
||||
}
|
||||
let processList: string
|
||||
try {
|
||||
processList = (options.deps?.readProcessList ?? readProcessCommandList)()
|
||||
} catch {
|
||||
clearUpdateInstallHandoffMarker(options.userDataPath)
|
||||
clearUpdateInstallHandoffMarker(options.appDataPath, executablePath)
|
||||
return false
|
||||
}
|
||||
if (isBundleShipItRunning(options.deps?.executablePath ?? process.execPath, processList)) {
|
||||
if (isBundleShipItRunning(executablePath, processList)) {
|
||||
return true
|
||||
}
|
||||
// Same version and no installer left: the install died (aborted/failed).
|
||||
// Clear the marker so this and future launches proceed normally.
|
||||
clearUpdateInstallHandoffMarker(options.userDataPath)
|
||||
clearUpdateInstallHandoffMarker(options.appDataPath, executablePath)
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -1,47 +1,71 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
findConflictingAppInstancePids,
|
||||
parseSameExecutablePids
|
||||
parseRunningApplicationPids
|
||||
} from './updater-conflicting-app-instances'
|
||||
|
||||
const ORCA_BIN = '/Applications/Orca.app/Contents/MacOS/Orca'
|
||||
const execFileMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
describe('parseSameExecutablePids', () => {
|
||||
it('matches other pids running the exact executable path', () => {
|
||||
const output = [
|
||||
` 100 ${ORCA_BIN}`,
|
||||
' 200 /Applications/Orca.app/Contents/Frameworks/Orca Helper.app/Contents/MacOS/Orca Helper',
|
||||
` 300 ${ORCA_BIN}`,
|
||||
' 400 /usr/sbin/cfprefsd'
|
||||
].join('\n')
|
||||
vi.mock('node:child_process', () => ({ execFile: execFileMock }))
|
||||
|
||||
expect(parseSameExecutablePids(output, ORCA_BIN, 100)).toEqual([300])
|
||||
beforeEach(() => {
|
||||
execFileMock.mockReset()
|
||||
})
|
||||
|
||||
describe('parseRunningApplicationPids', () => {
|
||||
it('parses positive pids and excludes the current process', () => {
|
||||
expect(parseRunningApplicationPids('100\n 300\n', 100)).toEqual([300])
|
||||
})
|
||||
|
||||
it('keeps executable paths containing spaces intact', () => {
|
||||
const binWithSpaces = '/Users/dev/My Apps/Orca.app/Contents/MacOS/Orca'
|
||||
const output = ` 512 ${binWithSpaces}\n 513 ${binWithSpaces} Helper`
|
||||
|
||||
expect(parseSameExecutablePids(output, binWithSpaces, 1)).toEqual([512])
|
||||
})
|
||||
|
||||
it('ignores dev-shell noise and malformed rows', () => {
|
||||
const output = ['not-a-row', ' -5 /bin/zsh', ` 42 ${ORCA_BIN}extra`].join('\n')
|
||||
|
||||
expect(parseSameExecutablePids(output, ORCA_BIN, 1)).toEqual([])
|
||||
it('ignores malformed, negative, and zero rows', () => {
|
||||
expect(parseRunningApplicationPids('not-a-row\n-5\n0\n42extra', 1)).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('findConflictingAppInstancePids', () => {
|
||||
it('reports other same-binary instances on darwin', async () => {
|
||||
let readerArgs: [string, number] | null = null
|
||||
const pids = await findConflictingAppInstancePids({
|
||||
platform: 'darwin',
|
||||
executablePath: ORCA_BIN,
|
||||
currentPid: 100,
|
||||
readProcessList: async () => ` 100 ${ORCA_BIN}\n 777 ${ORCA_BIN}`
|
||||
readRunningApplicationPids: async (executablePath, currentPid) => {
|
||||
readerArgs = [executablePath, currentPid]
|
||||
return '777'
|
||||
}
|
||||
})
|
||||
|
||||
expect(pids).toEqual([777])
|
||||
expect(readerArgs).toEqual([ORCA_BIN, 100])
|
||||
})
|
||||
|
||||
it('uses one bounded AppKit query with path arguments kept out of the script', async () => {
|
||||
execFileMock.mockImplementation((...args: unknown[]) => {
|
||||
const callback = args[3] as (error: Error | null, stdout: string) => void
|
||||
callback(null, '777\n')
|
||||
})
|
||||
|
||||
const pids = await findConflictingAppInstancePids({
|
||||
platform: 'darwin',
|
||||
executablePath: ORCA_BIN,
|
||||
currentPid: 100
|
||||
})
|
||||
|
||||
expect(pids).toEqual([777])
|
||||
expect(execFileMock).toHaveBeenCalledTimes(1)
|
||||
const [file, args, options] = execFileMock.mock.calls[0]
|
||||
expect(file).toBe('/usr/bin/osascript')
|
||||
expect(args).toEqual([
|
||||
'-l',
|
||||
'JavaScript',
|
||||
'-e',
|
||||
expect.stringContaining('NSWorkspace.sharedWorkspace.runningApplications'),
|
||||
'--',
|
||||
ORCA_BIN,
|
||||
'100'
|
||||
])
|
||||
expect(options).toEqual(expect.objectContaining({ timeout: 2_000, maxBuffer: 64 * 1024 }))
|
||||
})
|
||||
|
||||
it('skips the check off darwin — Win/Linux installers own this', async () => {
|
||||
@@ -49,19 +73,19 @@ describe('findConflictingAppInstancePids', () => {
|
||||
platform: 'win32',
|
||||
executablePath: ORCA_BIN,
|
||||
currentPid: 100,
|
||||
readProcessList: async () => ` 777 ${ORCA_BIN}`
|
||||
readRunningApplicationPids: async () => '777'
|
||||
})
|
||||
|
||||
expect(pids).toEqual([])
|
||||
})
|
||||
|
||||
it('fails open when the process table is unreadable', async () => {
|
||||
it('fails open when the application query is unavailable', async () => {
|
||||
const pids = await findConflictingAppInstancePids({
|
||||
platform: 'darwin',
|
||||
executablePath: ORCA_BIN,
|
||||
currentPid: 100,
|
||||
readProcessList: async () => {
|
||||
throw new Error('ps timed out')
|
||||
readRunningApplicationPids: async () => {
|
||||
throw new Error('osascript timed out')
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -9,25 +9,66 @@ import { execFile } from 'node:child_process'
|
||||
// user sees "app quit but never relaunched, still on the old version".
|
||||
// Detecting those instances before the install handoff turns that silent
|
||||
// wedge into an actionable message.
|
||||
const PROCESS_LIST_TIMEOUT_MS = 2_000
|
||||
// Why: a busy dev machine's per-user process table can exceed execFile's 1MB
|
||||
// default; truncation would silently drop a conflicting instance.
|
||||
const PROCESS_LIST_MAX_BYTES = 16 * 1024 * 1024
|
||||
const RUNNING_APPLICATION_QUERY_TIMEOUT_MS = 2_000
|
||||
const RUNNING_APPLICATION_QUERY_MAX_BYTES = 64 * 1024
|
||||
|
||||
export type ProcessListReader = () => Promise<string>
|
||||
// Why: Squirrel identifies blockers as NSRunningApplication instances by
|
||||
// bundle identity. An exact-executable `ps` scan also catches Orca CLI
|
||||
// processes running through ELECTRON_RUN_AS_NODE, even though AppKit gives
|
||||
// those processes no bundle id or URL and Squirrel does not wait for them.
|
||||
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')
|
||||
}`
|
||||
|
||||
function readCurrentUserProcessList(): Promise<string> {
|
||||
export type RunningApplicationPidReader = (
|
||||
executablePath: string,
|
||||
currentPid: number
|
||||
) => Promise<string>
|
||||
|
||||
function readCurrentUserRunningApplicationPids(
|
||||
executablePath: string,
|
||||
currentPid: number
|
||||
): Promise<string> {
|
||||
return new Promise((resolve, reject) => {
|
||||
// Why: `-x` (without `-a`) scopes to the current user's processes, which
|
||||
// mirrors NSRunningApplication's login-session scope — another macOS
|
||||
// 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.
|
||||
// Why: query AppKit in one bounded subprocess instead of launching a probe
|
||||
// per candidate. Passing paths as argv avoids script interpolation, and
|
||||
// osascript needs no Accessibility permission for NSWorkspace metadata.
|
||||
execFile(
|
||||
'/bin/ps',
|
||||
['-xo', 'pid=,comm='],
|
||||
{ encoding: 'utf8', timeout: PROCESS_LIST_TIMEOUT_MS, maxBuffer: PROCESS_LIST_MAX_BYTES },
|
||||
'/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)
|
||||
@@ -39,22 +80,15 @@ function readCurrentUserProcessList(): Promise<string> {
|
||||
})
|
||||
}
|
||||
|
||||
/** Parse `ps -xo pid=,comm=` output into pids whose executable path equals
|
||||
* `executablePath`, excluding `currentPid`. Executable paths may contain
|
||||
* spaces, so only the leading pid column is positional. */
|
||||
export function parseSameExecutablePids(
|
||||
psOutput: string,
|
||||
executablePath: string,
|
||||
currentPid: number
|
||||
): number[] {
|
||||
export function parseRunningApplicationPids(output: string, currentPid: number): number[] {
|
||||
const pids: number[] = []
|
||||
for (const line of psOutput.split('\n')) {
|
||||
const match = /^\s*(\d+)\s+(.+?)\s*$/.exec(line)
|
||||
if (!match) {
|
||||
for (const line of output.split('\n')) {
|
||||
const normalized = line.trim()
|
||||
if (!/^\d+$/.test(normalized)) {
|
||||
continue
|
||||
}
|
||||
const pid = Number(match[1])
|
||||
if (pid !== currentPid && match[2] === executablePath) {
|
||||
const pid = Number(normalized)
|
||||
if (pid > 0 && pid !== currentPid) {
|
||||
pids.push(pid)
|
||||
}
|
||||
}
|
||||
@@ -65,7 +99,7 @@ export type ConflictingInstanceDeps = {
|
||||
platform?: NodeJS.Platform
|
||||
executablePath?: string
|
||||
currentPid?: number
|
||||
readProcessList?: ProcessListReader
|
||||
readRunningApplicationPids?: RunningApplicationPidReader
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -73,8 +107,8 @@ export type ConflictingInstanceDeps = {
|
||||
*
|
||||
* macOS-only by design: this mirrors Squirrel.Mac's pre-install wait/abort
|
||||
* semantics. The Windows and Linux installers manage running instances
|
||||
* themselves. Fails open — an undetectable process table must never block an
|
||||
* update install.
|
||||
* themselves. Fails open — an unavailable application query must never block
|
||||
* an update install.
|
||||
*/
|
||||
export async function findConflictingAppInstancePids(
|
||||
deps: ConflictingInstanceDeps = {}
|
||||
@@ -82,13 +116,14 @@ export async function findConflictingAppInstancePids(
|
||||
if ((deps.platform ?? process.platform) !== 'darwin') {
|
||||
return []
|
||||
}
|
||||
const executablePath = deps.executablePath ?? process.execPath
|
||||
const currentPid = deps.currentPid ?? process.pid
|
||||
try {
|
||||
const output = await (deps.readProcessList ?? readCurrentUserProcessList)()
|
||||
return parseSameExecutablePids(
|
||||
output,
|
||||
deps.executablePath ?? process.execPath,
|
||||
deps.currentPid ?? process.pid
|
||||
const output = await (deps.readRunningApplicationPids ?? readCurrentUserRunningApplicationPids)(
|
||||
executablePath,
|
||||
currentPid
|
||||
)
|
||||
return parseRunningApplicationPids(output, currentPid)
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
|
||||
@@ -198,7 +198,11 @@ describe('updater mac install handoff', () => {
|
||||
expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledWith(false, true)
|
||||
})
|
||||
// The relaunch-race handoff marker must be down before the native call.
|
||||
expect(writeHandoffMarkerMock).toHaveBeenCalledWith('/tmp/orca-test-user-data', '1.0.51')
|
||||
expect(writeHandoffMarkerMock).toHaveBeenCalledWith(
|
||||
'/tmp/orca-test-user-data',
|
||||
process.execPath,
|
||||
'1.0.51'
|
||||
)
|
||||
expect(writeHandoffMarkerMock.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
autoUpdaterMock.quitAndInstall.mock.invocationCallOrder[0]
|
||||
)
|
||||
|
||||
@@ -1295,8 +1295,15 @@ describe('updater', () => {
|
||||
expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1)
|
||||
expect(isQuittingForUpdate()).toBe(false)
|
||||
if (process.platform === 'darwin') {
|
||||
expect(writeHandoffMarkerMock).toHaveBeenCalledWith('/tmp/orca-test-user-data', '1.0.51')
|
||||
expect(clearHandoffMarkerMock).toHaveBeenCalledWith('/tmp/orca-test-user-data')
|
||||
expect(writeHandoffMarkerMock).toHaveBeenCalledWith(
|
||||
'/tmp/orca-test-user-data',
|
||||
process.execPath,
|
||||
'1.0.51'
|
||||
)
|
||||
expect(clearHandoffMarkerMock).toHaveBeenCalledWith(
|
||||
'/tmp/orca-test-user-data',
|
||||
process.execPath
|
||||
)
|
||||
}
|
||||
// Why: destructive prep runs only after quitAndInstall returns still in
|
||||
// progress; sync recovery clears flags first so PTYs stay alive.
|
||||
|
||||
+2
-2
@@ -645,7 +645,7 @@ async function performQuitAndInstall(): Promise<void> {
|
||||
if (process.platform === 'darwin') {
|
||||
// Why: lets a launch that races Squirrel's install back off instead of
|
||||
// aborting it (see update-install-launch-gate). Cleared on recovery.
|
||||
writeUpdateInstallHandoffMarker(app.getPath('userData'), app.getVersion())
|
||||
writeUpdateInstallHandoffMarker(app.getPath('appData'), process.execPath, app.getVersion())
|
||||
}
|
||||
// Why: invoke quitAndInstall before killAllPty/remove close listeners so a
|
||||
// sync 'error' (common "no filepath" path) recovers while windows and
|
||||
@@ -707,7 +707,7 @@ function resetQuitForUpdateState(): void {
|
||||
resetMacInstallState()
|
||||
// Why: install recovery keeps this process alive, so the handoff marker
|
||||
// would otherwise gate the user's next manual relaunch for no reason.
|
||||
clearUpdateInstallHandoffMarker(app.getPath('userData'))
|
||||
clearUpdateInstallHandoffMarker(app.getPath('appData'), process.execPath)
|
||||
}
|
||||
|
||||
// Why: electron-updater often reports quitAndInstall failures via the 'error'
|
||||
|
||||
Reference in New Issue
Block a user