diff --git a/src/main/mac-update-install-attempt.probe-state.test.ts b/src/main/mac-update-install-attempt.probe-state.test.ts index 433028caeac..8ee47407b20 100644 --- a/src/main/mac-update-install-attempt.probe-state.test.ts +++ b/src/main/mac-update-install-attempt.probe-state.test.ts @@ -1,10 +1,13 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { + areMacUpdateVersionsEqual, armMacUpdateInstallAttempt, + decideMacUpdateInstallLaunch, getMacUpdateInstallAttemptPath, + getMacUpdateInstallHeartbeatPath, getMacUpdateProcessIdentityState, MAC_UPDATE_INSTALL_ATTEMPT_SCHEMA_VERSION, MAC_UPDATE_INSTALL_ATTEMPT_STALE_MS, @@ -164,6 +167,52 @@ describe('macOS update install startup probes', () => { }) }) +describe('macOS update version equality', () => { + it('treats formatting variants of the same version as installed', () => { + expect(areMacUpdateVersionsEqual('v1.4.192', '1.4.192')).toBe(true) + expect(areMacUpdateVersionsEqual('1.4.192+build.7', '1.4.192')).toBe(true) + expect(areMacUpdateVersionsEqual('1.4.192', '1.4.193')).toBe(false) + expect(areMacUpdateVersionsEqual('not-a-version', 'also-not')).toBe(false) + + expect( + decideMacUpdateInstallLaunch({ + attempt: createAttempt({ targetVersion: '1.4.192' }), + currentBundlePath: BUNDLE_PATH, + currentVersion: 'v1.4.192', + nowMs: 3_000, + monitorAlive: true, + shipItAlive: true + }) + ).toEqual({ action: 'allow-and-clear', reason: 'target-installed' }) + }) +}) + +describe('macOS update orphan heartbeat', () => { + it('reclaims a heartbeat file that has no attempt record', () => { + const appDataPath = createTempDir() + const attemptPath = getMacUpdateInstallAttemptPath(appDataPath) + writeMacUpdateInstallHeartbeat(attemptPath, { + schemaVersion: MAC_UPDATE_INSTALL_ATTEMPT_SCHEMA_VERSION, + attemptId: 'orphaned-attempt', + heartbeatAtMs: 5_000 + }) + + expect( + resolveMacUpdateInstallStartup({ + appDataPath, + appVersion: '1.4.191', + executablePath: EXECUTABLE_PATH, + isPackaged: true, + platform: 'darwin', + nowMs: 6_000, + readProcessStartedAtMs: () => null, + readProcessList: () => '' + }) + ).toEqual({ action: 'allow', reason: 'no-attempt' }) + expect(existsSync(getMacUpdateInstallHeartbeatPath(attemptPath))).toBe(false) + }) +}) + describe('macOS update install arming identity', () => { it.runIf(process.platform === 'darwin')( 'refuses to fence on a spawned pid whose recorded start time is implausible', diff --git a/src/main/mac-update-install-attempt.ts b/src/main/mac-update-install-attempt.ts index 4a7f77aebce..7918b174311 100644 --- a/src/main/mac-update-install-attempt.ts +++ b/src/main/mac-update-install-attempt.ts @@ -1,7 +1,7 @@ import { randomUUID } from 'node:crypto' import { existsSync } from 'node:fs' import { join, resolve } from 'node:path' -import { isValidAppVersion } from '../shared/app-version' +import { compareAppVersions, isValidAppVersion } from '../shared/app-version' import { spawnProcess } from '../shared/child-process/run-process' import { getProcessStartedAtMs } from './daemon/daemon-process-start-time' import { @@ -17,6 +17,7 @@ export { } from './mac-update-install-process-probes' import { clearMacUpdateInstallAttempt, + clearMacUpdateInstallHeartbeat, getMacUpdateInstallAttemptPath, MAC_UPDATE_INSTALL_ATTEMPT_SCHEMA_VERSION, readMacUpdateInstallAttempt, @@ -58,6 +59,19 @@ export type MacUpdateInstallLaunchDecision = } | { action: 'block'; reason: 'active-install' | 'shipit-alive' } +/** + * Canonical version equality: a feed may carry a leading `v` or build metadata while the + * bundle plist is bare. A successful install must never read as a failure over formatting. + */ +export function areMacUpdateVersionsEqual(left: string, right: string): boolean { + if (left === right) { + return true + } + return ( + isValidAppVersion(left) && isValidAppVersion(right) && compareAppVersions(left, right) === 0 + ) +} + export function resolveMacUpdateBundlePath(executablePath: string): string { const bundlePath = resolve(executablePath, '..', '..', '..') if (!bundlePath.toLowerCase().endsWith('.app')) { @@ -83,7 +97,7 @@ export function decideMacUpdateInstallLaunch(options: { if (!macPathsEqual(attempt.targetBundlePath, options.currentBundlePath)) { return { action: 'allow', reason: 'different-bundle' } } - if (options.currentVersion === attempt.targetVersion) { + if (areMacUpdateVersionsEqual(options.currentVersion, attempt.targetVersion)) { return { action: 'allow-and-clear', reason: 'target-installed' } } const ageMs = options.nowMs - attempt.createdAtMs @@ -233,6 +247,8 @@ function resolveMacUpdateInstallStartupUnsafe(options: { const attemptPath = getMacUpdateInstallAttemptPath(options.appDataPath) const attempt = readMacUpdateInstallAttempt(attemptPath) if (!attempt) { + // Why: a heartbeat with no attempt record is an orphan from a lost race; reclaim it. + clearMacUpdateInstallHeartbeat(attemptPath) return { action: 'allow', reason: 'no-attempt' } } const monitorAlive = diff --git a/src/main/mac-update-install-monitor.probe-safety.test.ts b/src/main/mac-update-install-monitor.probe-safety.test.ts index 38ca262f992..a7149b4d87f 100644 --- a/src/main/mac-update-install-monitor.probe-safety.test.ts +++ b/src/main/mac-update-install-monitor.probe-safety.test.ts @@ -121,6 +121,86 @@ describe('macOS update monitor probe safety', () => { ).toEqual({ action: 'fail', reason: 'install-timed-out' }) }) + it('measures the installer-appearance window from first verified source death', () => { + const attempt = createAttempt() + const wedgedQuitReleaseMs = attempt.createdAtMs + 25_000 + // 29s after arm but only 4s after the source verifiably died: not a failure yet. + expect( + decideMacUpdateMonitorStep({ + attempt, + observation: { bundleVersion: attempt.sourceVersion, shipIt: 'absent', source: 'dead' }, + nowMs: attempt.createdAtMs + 29_000, + shipItSeen: false, + shipItMissingSinceMs: null, + sourceDeadSinceMs: wedgedQuitReleaseMs + }).action + ).toBe('continue') + // The full window after source death has elapsed: now it is a failure. + expect( + decideMacUpdateMonitorStep({ + attempt, + observation: { bundleVersion: attempt.sourceVersion, shipIt: 'absent', source: 'dead' }, + nowMs: wedgedQuitReleaseMs + MAC_UPDATE_MONITOR_SHIPIT_APPEARANCE_MS, + shipItSeen: false, + shipItMissingSinceMs: null, + sourceDeadSinceMs: wedgedQuitReleaseMs + }) + ).toEqual({ action: 'fail', reason: 'installer-never-started' }) + }) + + it('cancels a failure when the confirm probe finds ShipIt alive again', async () => { + const { attempt, path } = createAttemptFile() + const launchRecovery = vi.fn().mockResolvedValue(true) + let calls = 0 + + await expect( + runMacUpdateInstallMonitor({ + attemptPath: path, + attemptId: attempt.attemptId, + now: () => attempt.createdAtMs + MAC_UPDATE_MONITOR_SHIPIT_APPEARANCE_MS + 1, + wait: async () => {}, + observe: async () => { + calls += 1 + // 1: verified absent, source dead -> fail decision. 2: confirm probe sees ShipIt + // alive -> failure cancelled. 3: install completes. + if (calls === 1) { + return { bundleVersion: attempt.sourceVersion, shipIt: 'absent', source: 'dead' } + } + if (calls === 2) { + return { bundleVersion: attempt.sourceVersion, shipIt: 'alive', source: 'dead' } + } + return { bundleVersion: attempt.targetVersion, shipIt: 'absent', source: 'dead' } + }, + launchRecovery + }) + ).resolves.toBe('completed') + expect(launchRecovery).not.toHaveBeenCalled() + expect(readMacUpdateInstallAttempt(path)).toBeNull() + }) + + it('completes instead of failing when the confirm probe sees the target version', async () => { + const { attempt, path } = createAttemptFile() + const launchRecovery = vi.fn().mockResolvedValue(true) + let calls = 0 + + await expect( + runMacUpdateInstallMonitor({ + attemptPath: path, + attemptId: attempt.attemptId, + now: () => attempt.createdAtMs + MAC_UPDATE_MONITOR_TIMEOUT_MS, + wait: async () => {}, + observe: async () => { + calls += 1 + return calls === 1 + ? { bundleVersion: attempt.sourceVersion, shipIt: 'absent', source: 'dead' } + : { bundleVersion: attempt.targetVersion, shipIt: 'absent', source: 'dead' } + }, + launchRecovery + }) + ).resolves.toBe('completed') + expect(launchRecovery).not.toHaveBeenCalled() + }) + it('does not resurrect an attempt cleared mid-observation via the heartbeat write', async () => { const { attempt, path } = createAttemptFile() let observations = 0 diff --git a/src/main/mac-update-install-monitor.test.ts b/src/main/mac-update-install-monitor.test.ts index 09422e9442a..83c5db72460 100644 --- a/src/main/mac-update-install-monitor.test.ts +++ b/src/main/mac-update-install-monitor.test.ts @@ -105,6 +105,8 @@ describe('macOS update install monitor', () => { const observations = [ { bundleVersion: attempt.sourceVersion, shipIt: 'alive' as const, source: 'dead' as const }, { bundleVersion: attempt.sourceVersion, shipIt: 'absent' as const, source: 'dead' as const }, + { bundleVersion: attempt.sourceVersion, shipIt: 'absent' as const, source: 'dead' as const }, + // Consumed by the pre-recovery confirm probe: still gone, still the old version. { bundleVersion: attempt.sourceVersion, shipIt: 'absent' as const, source: 'dead' as const } ] const times = [2_000, 3_000, 9_000] @@ -153,7 +155,9 @@ describe('macOS update install monitor', () => { sourcePid: process.pid, sourceStartedAtMs: 1 }) - const times = [31_001, 15 * 60_000 + 1_001] + // First verified source-dead observation starts the appearance window; the second + // poll lands past that window without reaching the overall timeout. + const times = [31_001, 61_002] const launchRecovery = vi.fn().mockResolvedValue(true) await expect( diff --git a/src/main/mac-update-install-monitor.ts b/src/main/mac-update-install-monitor.ts index d24ef681407..f745be21cd9 100644 --- a/src/main/mac-update-install-monitor.ts +++ b/src/main/mac-update-install-monitor.ts @@ -2,6 +2,7 @@ import { readFile } from 'node:fs/promises' import { join } from 'node:path' import { runProcess, spawnProcess } from '../shared/child-process/run-process' import { + areMacUpdateVersionsEqual, clearMacUpdateInstallAttempt, clearMacUpdateInstallHeartbeat, failMacUpdateInstallAttemptIfCurrent, @@ -40,9 +41,14 @@ export function decideMacUpdateMonitorStep(options: { nowMs: number shipItSeen: boolean shipItMissingSinceMs: number | null + /** First verified source-dead observation; the appearance window must not start earlier. */ + sourceDeadSinceMs?: number | null }): MacUpdateMonitorDecision { const { attempt, observation, nowMs } = options - if (observation.bundleVersion === attempt.targetVersion) { + if ( + observation.bundleVersion !== null && + areMacUpdateVersionsEqual(observation.bundleVersion, attempt.targetVersion) + ) { return { action: 'complete' } } if (nowMs - attempt.createdAtMs >= MAC_UPDATE_MONITOR_TIMEOUT_MS) { @@ -62,7 +68,14 @@ export function decideMacUpdateMonitorStep(options: { return { action: 'continue', shipItSeen, shipItMissingSinceMs: options.shipItMissingSinceMs } } if (!shipItSeen) { - if (nowMs - attempt.createdAtMs >= MAC_UPDATE_MONITOR_SHIPIT_APPEARANCE_MS) { + // Why the max: a wedged teardown can hold the source alive until the 20s exit watchdog, + // eating most of the appearance window. Launchd deserves the full window measured from + // when the source verifiably died, or a slow quit turns into a relaunch into ShipIt. + const appearanceBaselineMs = Math.max( + attempt.createdAtMs, + options.sourceDeadSinceMs ?? attempt.createdAtMs + ) + if (nowMs - appearanceBaselineMs >= MAC_UPDATE_MONITOR_SHIPIT_APPEARANCE_MS) { return { action: 'fail', reason: 'installer-never-started' } } return { action: 'continue', shipItSeen: false, shipItMissingSinceMs: null } @@ -90,6 +103,7 @@ export async function runMacUpdateInstallMonitor(options: { } let shipItSeen = false let shipItMissingSinceMs: number | null = null + let sourceDeadSinceMs: number | null = null for (;;) { const current = readMacUpdateInstallAttempt(options.attemptPath) @@ -100,18 +114,46 @@ export async function runMacUpdateInstallMonitor(options: { attempt = current const nowMs = now() const observation = await (options.observe ?? observeInstall)(attempt) + if (observation.source === 'dead') { + sourceDeadSinceMs ??= nowMs + } else if (observation.source === 'alive') { + sourceDeadSinceMs = null + } const decision = decideMacUpdateMonitorStep({ attempt, observation, nowMs, shipItSeen, - shipItMissingSinceMs + shipItMissingSinceMs, + sourceDeadSinceMs }) if (decision.action === 'complete') { clearMacUpdateInstallAttempt(options.attemptPath, attempt.attemptId) return 'completed' } if (decision.action === 'fail') { + // Why a confirm probe: the failing observation is up to a poll old, and launching + // recovery against a live ShipIt would relaunch the old app into the install window — + // the exact race this monitor exists to prevent. A late completion wins outright. + const confirm = await (options.observe ?? observeInstall)(attempt) + if ( + confirm.bundleVersion !== null && + areMacUpdateVersionsEqual(confirm.bundleVersion, attempt.targetVersion) + ) { + clearMacUpdateInstallAttempt(options.attemptPath, attempt.attemptId) + return 'completed' + } + if (decision.reason !== 'install-timed-out' && confirm.shipIt === 'alive') { + shipItSeen = true + shipItMissingSinceMs = null + writeMacUpdateInstallHeartbeat(options.attemptPath, { + schemaVersion: MAC_UPDATE_INSTALL_ATTEMPT_SCHEMA_VERSION, + attemptId: attempt.attemptId, + heartbeatAtMs: nowMs + }) + await wait(MAC_UPDATE_MONITOR_POLL_MS) + continue + } // Why guarded: cleanup may have cleared or replaced the attempt while this step observed; // writing an unconditional failure would resurrect a finished attempt and launch a // recovery app nobody asked for. diff --git a/src/main/updater.mac-install.test.ts b/src/main/updater.mac-install.test.ts index 61d32c2873e..57084a21345 100644 --- a/src/main/updater.mac-install.test.ts +++ b/src/main/updater.mac-install.test.ts @@ -443,7 +443,11 @@ describe('updater mac install handoff', () => { // abort an update that would otherwise succeed. await vi.waitFor(() => expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledOnce()) expect(armMacUpdateInstallAttemptMock).toHaveBeenCalledOnce() - expect(clearMacUpdateInstallAttemptMock).not.toHaveBeenCalled() + // Why: arming can throw after the record reached disk; the catch must clear the path + // unguarded or a live fence would block every relaunch until the age cap. + expect(clearMacUpdateInstallAttemptMock).toHaveBeenCalledWith( + '/tmp/orca-update-install-attempt.json' + ) } finally { if (resourcesPathDescriptor) { Object.defineProperty(process, 'resourcesPath', resourcesPathDescriptor) diff --git a/src/main/updater/updater-install-support.ts b/src/main/updater/updater-install-support.ts index 5e867c5b027..10308fd6467 100644 --- a/src/main/updater/updater-install-support.ts +++ b/src/main/updater/updater-install-support.ts @@ -104,6 +104,15 @@ export abstract class UpdaterInstallSupport extends UpdaterCheckState { { message: error instanceof Error ? error.message : String(error) }, { level: 'warn', message: 'Proceeding with the macOS install without the relaunch fence' } ) + // Why unconditional: arming can throw after the record reached disk (e.g. a directory + // fsync failure), leaving a live fence nothing in this process would ever clear — which + // would silently block every relaunch until the age cap. Clearing also cancels the + // just-spawned monitor within one poll. + try { + clearMacUpdateInstallAttempt(getMacUpdateInstallAttemptPath(app.getPath('appData'))) + } catch { + // The stale attempt self-expires via the age cap; never let cleanup mask the install. + } return null } }