From 503162cf9808d29c802b300a56af8ae2b0f8da75 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Fri, 11 Sep 2026 11:05:06 -0700 Subject: [PATCH] fix(main): verify the macOS sleep assertion is still held start() returned true whenever a child had ever been spawned, and refresh() stops the Electron blocker on that word -- so a caffeinate that died without delivering an exit event left the machine with no assertion at all, and nothing ever re-checked. Measured: 15h49m awake with the setting on and pmset showing zero Orca-owned assertions. Verify the recorded pid instead of trusting the handle, and run a watchdog while the assertion is meant to be held so a vanished child re-arms. Mirrors reconcileBlocker(), which already re-checks the Electron blocker with isStarted(). --- src/main/macos-system-sleep-assertion.test.ts | 68 ++++++++++++++++++ src/main/macos-system-sleep-assertion.ts | 70 ++++++++++++++++++- 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/src/main/macos-system-sleep-assertion.test.ts b/src/main/macos-system-sleep-assertion.test.ts index 8d17485a245..1e22cec1f97 100644 --- a/src/main/macos-system-sleep-assertion.test.ts +++ b/src/main/macos-system-sleep-assertion.test.ts @@ -2,6 +2,7 @@ import { EventEmitter } from 'node:events' import { describe, expect, it, vi } from 'vitest' import { MACOS_SYSTEM_SLEEP_ASSERTION_RETRY_MS, + MACOS_SYSTEM_SLEEP_ASSERTION_WATCHDOG_MS, MacosSystemSleepAssertion } from './macos-system-sleep-assertion' @@ -56,6 +57,7 @@ describe('MacosSystemSleepAssertion', () => { const assertion = new MacosSystemSleepAssertion({ logger: createLogger(), platform: 'darwin', + isProcessAlive: () => true, spawn }) @@ -220,4 +222,70 @@ describe('MacosSystemSleepAssertion', () => { expect(logger.debug).toHaveBeenCalledTimes(1) assertion.dispose() }) + + it('respawns when the recorded child vanished without an exit event', () => { + const spawn = vi.fn(() => new FakeCaffeinateProcess()) + const assertion = new MacosSystemSleepAssertion({ + logger: createLogger(), + platform: 'darwin', + isProcessAlive: () => false, + spawn + }) + + assertion.start('status-change') + expect(assertion.start('status-change')).toBe(true) + + // Without a liveness check the second start short-circuits and the OS holds nothing. + expect(spawn).toHaveBeenCalledTimes(2) + assertion.dispose() + }) + + it('reports a vanished assertion from the watchdog so the service re-arms', () => { + vi.useFakeTimers() + try { + let alive = true + const onUnexpectedFailure = vi.fn() + const assertion = new MacosSystemSleepAssertion({ + logger: createLogger(), + platform: 'darwin', + isProcessAlive: () => alive, + onUnexpectedFailure, + spawn: vi.fn(() => new FakeCaffeinateProcess()) + }) + + assertion.start('status-change') + vi.advanceTimersByTime(MACOS_SYSTEM_SLEEP_ASSERTION_WATCHDOG_MS) + expect(onUnexpectedFailure).not.toHaveBeenCalled() + + alive = false + vi.advanceTimersByTime(MACOS_SYSTEM_SLEEP_ASSERTION_WATCHDOG_MS) + + expect(onUnexpectedFailure).toHaveBeenCalledWith('macos-assertion-vanished') + assertion.dispose() + } finally { + vi.useRealTimers() + } + }) + + it('stops the watchdog once the assertion is intentionally released', () => { + vi.useFakeTimers() + try { + const onUnexpectedFailure = vi.fn() + const assertion = new MacosSystemSleepAssertion({ + logger: createLogger(), + platform: 'darwin', + isProcessAlive: () => false, + onUnexpectedFailure, + spawn: vi.fn(() => new FakeCaffeinateProcess()) + }) + + assertion.start('status-change') + assertion.stop('settings-change') + vi.advanceTimersByTime(MACOS_SYSTEM_SLEEP_ASSERTION_WATCHDOG_MS * 3) + + expect(onUnexpectedFailure).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) }) diff --git a/src/main/macos-system-sleep-assertion.ts b/src/main/macos-system-sleep-assertion.ts index 84250eb77af..dddaf8e911f 100644 --- a/src/main/macos-system-sleep-assertion.ts +++ b/src/main/macos-system-sleep-assertion.ts @@ -1,6 +1,9 @@ import { spawn as nodeSpawn } from 'node:child_process' export const MACOS_SYSTEM_SLEEP_ASSERTION_RETRY_MS = 30_000 +// Why: a killed or reaped caffeinate can leave `child` set with no exit event, and the +// service stops the Electron blocker on that word — so the belief must be re-verified. +export const MACOS_SYSTEM_SLEEP_ASSERTION_WATCHDOG_MS = 30_000 type Logger = Pick @@ -28,6 +31,8 @@ type MacosSystemSleepAssertionOptions = { onUnexpectedFailure?: (reason: string) => void platform?: NodeJS.Platform spawn?: CaffeinateSpawn + isProcessAlive?: (pid: number) => boolean + watchdogIntervalMs?: number } export class MacosSystemSleepAssertion { @@ -36,7 +41,11 @@ export class MacosSystemSleepAssertion { private readonly onUnexpectedFailure: (reason: string) => void private readonly platform: NodeJS.Platform private readonly spawn: CaffeinateSpawn + private readonly isProcessAlive: (pid: number) => boolean + private readonly watchdogIntervalMs: number private child: CaffeinateProcess | null = null + private childPid: number | null = null + private watchdogTimer: ReturnType | null = null private retryNotBefore: number | null = null private retryTimer: ReturnType | null = null private lastFailureKey: string | null = null @@ -51,6 +60,8 @@ export class MacosSystemSleepAssertion { this.onUnexpectedFailure = options.onUnexpectedFailure ?? (() => {}) this.platform = options.platform ?? process.platform this.spawn = options.spawn ?? nodeSpawn + this.isProcessAlive = options.isProcessAlive ?? processIsAlive + this.watchdogIntervalMs = options.watchdogIntervalMs ?? MACOS_SYSTEM_SLEEP_ASSERTION_WATCHDOG_MS } start(reason: string): boolean { @@ -58,7 +69,10 @@ export class MacosSystemSleepAssertion { return false } if (this.child) { - return true + if (this.isChildAlive()) { + return true + } + this.forgetChild() } if (this.retryNotBefore !== null && this.now() < this.retryNotBefore) { this.scheduleRetry() @@ -77,6 +91,7 @@ export class MacosSystemSleepAssertion { } this.child = child + this.childPid = typeof child.pid === 'number' ? child.pid : null const onError: CaffeinateErrorListener = (error) => { this.handleChildFailure(child, `error:${String(error.message)}`, 'error', reason, error) } @@ -92,6 +107,7 @@ export class MacosSystemSleepAssertion { }) child.on('error', onError) child.on('exit', onExit) + this.armWatchdog() this.resetRetrySuppression() this.resetFailureStreak() return true @@ -105,6 +121,8 @@ export class MacosSystemSleepAssertion { } const child = this.child this.child = null + this.childPid = null + this.clearWatchdog() this.intentionalStops.add(child) this.detachChildListeners(child) try { @@ -139,7 +157,7 @@ export class MacosSystemSleepAssertion { } this.reportedFailures.add(child) if (this.child === child) { - this.child = null + this.forgetChild() } this.handleFailure(failureKey, startReason, details, failureType) } @@ -185,6 +203,44 @@ export class MacosSystemSleepAssertion { this.logger.warn('[agent-awake] macOS system sleep assertion failed', payload) } + private isChildAlive(): boolean { + // No pid means an injected test double; there is nothing to verify against. + return this.childPid === null ? true : this.isProcessAlive(this.childPid) + } + + private forgetChild(): void { + const child = this.child + this.child = null + this.childPid = null + this.clearWatchdog() + if (child) { + this.detachChildListeners(child) + } + } + + private armWatchdog(): void { + this.clearWatchdog() + this.watchdogTimer = setInterval(() => { + if (this.isChildAlive()) { + return + } + this.logger.warn('[agent-awake] macOS system sleep assertion vanished without an exit event') + this.forgetChild() + this.onUnexpectedFailure('macos-assertion-vanished') + }, this.watchdogIntervalMs) + if (typeof this.watchdogTimer.unref === 'function') { + this.watchdogTimer.unref() + } + } + + private clearWatchdog(): void { + if (!this.watchdogTimer) { + return + } + clearInterval(this.watchdogTimer) + this.watchdogTimer = null + } + private scheduleRetry(): void { if (this.retryNotBefore === null || this.retryTimer) { return @@ -214,6 +270,16 @@ export class MacosSystemSleepAssertion { } } +function processIsAlive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch (error) { + // EPERM means the pid is live but not ours; only ESRCH proves it is gone. + return !isEsrchError(error) + } +} + function isEsrchError(error: unknown): boolean { return ( typeof error === 'object' &&