From ebdd072e50a845fece86a773e2f840d6a6bc68ab Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:24:12 -0700 Subject: [PATCH] fix(daemon): keep login-session retirement conclusive --- src/main/daemon/daemon-entry.ts | 2 +- .../macos-login-session-death-watch.test.ts | 125 +++++------------- .../daemon/macos-login-session-death-watch.ts | 67 ++++------ 3 files changed, 62 insertions(+), 132 deletions(-) diff --git a/src/main/daemon/daemon-entry.ts b/src/main/daemon/daemon-entry.ts index 3cc24093a06..70ec417a415 100644 --- a/src/main/daemon/daemon-entry.ts +++ b/src/main/daemon/daemon-entry.ts @@ -172,7 +172,7 @@ async function main(): Promise { // Why: a dead macOS login session cannot be fabricated without root (PAM owns // audit-session teardown), so e2e drives the oracles from a verdict file: // 'alive' → accepted/healthy, 'dead' → rejected/unhealthy, 'hang' → - // timeout-inconclusive/unhealthy (the hang-shaped trigger), else inconclusive. + // timeout-inconclusive/unhealthy (the fail-safe path), else inconclusive. const e2eProbeFile = process.env.ORCA_E2E_LOGIN_SESSION_PROBE_FILE const readE2eVerdict = (): string => { try { diff --git a/src/main/daemon/macos-login-session-death-watch.test.ts b/src/main/daemon/macos-login-session-death-watch.test.ts index 410ce2ccce9..f4002c1865b 100644 --- a/src/main/daemon/macos-login-session-death-watch.test.ts +++ b/src/main/daemon/macos-login-session-death-watch.test.ts @@ -154,120 +154,67 @@ describe('MacosLoginSessionDeathWatch', () => { watch.start() await drainMicrotasks() await clock.advance(120_000) // rejection 1 - await clock.advance(10_000) // inconclusive timeout — streak holds, fast recheck - await clock.advance(10_000) // rejection 2 + await clock.advance(10_000) // inconclusive timeout — rejection streak holds + await clock.advance(120_000) // rejection 2 await clock.advance(10_000) // inconclusive - await clock.advance(10_000) // rejection 3 → retire + await clock.advance(120_000) // rejection 3 → retire expect(onRetire).toHaveBeenCalledTimes(1) expect(onRetire.mock.calls[0][0].rejections).toBe(3) expect(onRetire.mock.calls[0][0].cause).toBe('pam-rejections') }) - it('retires on a hang-shaped timeout streak once armed, with a degraded resolver', async () => { - const { watch, clock, onRetire } = createWatch({ - outcomes: [ACCEPTED, INCONCLUSIVE, INCONCLUSIVE, INCONCLUSIVE, INCONCLUSIVE, INCONCLUSIVE] - }) - watch.start() - await drainMicrotasks() - await clock.advance(120_000) // timeout 1 → fast recheck - await clock.advance(10_000) // timeout 2 - await clock.advance(10_000) // timeout 3 - await clock.advance(10_000) // timeout 4 - expect(onRetire).not.toHaveBeenCalled() - await clock.advance(10_000) // timeout 5 → retire - expect(onRetire).toHaveBeenCalledWith({ - cause: 'probe-timeouts', - rejections: 0, - resolverHealth: 'unhealthy' - }) - }) - - it('never counts timeouts toward retirement while unarmed', async () => { + it('keeps repeated inconclusive timeouts on the bounded periodic cadence', async () => { + const readResolverHealth = vi.fn(async () => 'unhealthy' as const) const { watch, clock, onRetire, probe } = createWatch({ - outcomes: Array.from({ length: 8 }, () => INCONCLUSIVE) + outcomes: [ACCEPTED, ...Array.from({ length: 10 }, () => INCONCLUSIVE)], + readResolverHealth }) watch.start() await drainMicrotasks() - for (let i = 0; i < 7; i++) { + for (let i = 0; i < 10; i++) { await clock.advance(120_000) } - expect(probe.mock.calls.length).toBeGreaterThanOrEqual(7) + expect(probe).toHaveBeenCalledTimes(11) + expect(readResolverHealth).not.toHaveBeenCalled() expect(onRetire).not.toHaveBeenCalled() }) - it('resets the timeout streak on any conclusive verdict', async () => { - const { watch, clock, onRetire } = createWatch({ - outcomes: [ - ACCEPTED, - INCONCLUSIVE, - INCONCLUSIVE, - INCONCLUSIVE, - INCONCLUSIVE, - ACCEPTED, - INCONCLUSIVE, - INCONCLUSIVE, - INCONCLUSIVE, - INCONCLUSIVE - ] - }) - watch.start() - await drainMicrotasks() - await clock.advance(120_000) // timeout 1 - for (let i = 0; i < 3; i++) { - await clock.advance(10_000) // timeouts 2-4 + it.each(['healthy', 'unknown'] as const)( + 'suppresses retirement while resolver health is %s, then retires on explicit degradation', + async (initialResolverHealth) => { + const { watch, clock, onRetire, probe, setResolverHealth } = createWatch({ + outcomes: [ACCEPTED, REJECTED, REJECTED, REJECTED, REJECTED] + }) + setResolverHealth(initialResolverHealth) + watch.start() + await drainMicrotasks() + await clock.advance(120_000) + await clock.advance(10_000) + await clock.advance(10_000) // threshold reached but resolver did not corroborate death + expect(onRetire).not.toHaveBeenCalled() + const probesAtSuppression = probe.mock.calls.length + setResolverHealth('unhealthy') + await clock.advance(10_000) + expect(probe).toHaveBeenCalledTimes(probesAtSuppression) + await clock.advance(110_000) // suppressed states return to the bounded periodic cadence + expect(onRetire).toHaveBeenCalledTimes(1) } - await clock.advance(10_000) // acceptance → both streaks reset - await clock.advance(120_000) // timeout 1 again - for (let i = 0; i < 3; i++) { - await clock.advance(10_000) // timeouts 2-4 - } - expect(onRetire).not.toHaveBeenCalled() - }) + ) - it('suppresses timeout-streak retirement while the resolver is healthy', async () => { - const { watch, clock, onRetire, setResolverHealth } = createWatch({ - outcomes: [ACCEPTED, ...Array.from({ length: 8 }, () => INCONCLUSIVE)] - }) - setResolverHealth('healthy') - watch.start() - await drainMicrotasks() - await clock.advance(120_000) // timeout 1 - for (let i = 0; i < 4; i++) { - await clock.advance(10_000) // timeouts 2-5 → threshold, suppressed - } - expect(onRetire).not.toHaveBeenCalled() - setResolverHealth('unhealthy') - await clock.advance(10_000) // held at threshold → next timeout re-verdicts → retire - expect(onRetire).toHaveBeenCalledTimes(1) - expect(onRetire.mock.calls[0][0].cause).toBe('probe-timeouts') - }) - - it('suppresses retirement while the system resolver is healthy, then retires when it degrades', async () => { - const { watch, clock, onRetire, setResolverHealth } = createWatch({ - outcomes: [ACCEPTED, REJECTED, REJECTED, REJECTED, REJECTED] - }) - setResolverHealth('healthy') - watch.start() - await drainMicrotasks() - await clock.advance(120_000) - await clock.advance(10_000) - await clock.advance(10_000) // threshold reached but resolver healthy → suppressed - expect(onRetire).not.toHaveBeenCalled() - setResolverHealth('unhealthy') - await clock.advance(10_000) // re-verdict → retire - expect(onRetire).toHaveBeenCalledTimes(1) - }) - - it('debounces PTY-exit bursts into one probe', async () => { + it('debounces a sustained PTY-exit burst into one trailing probe', async () => { const { watch, clock, probe } = createWatch({ outcomes: [ACCEPTED, ACCEPTED] }) watch.start() await drainMicrotasks() await clock.advance(60_000) const before = probe.mock.calls.length watch.notifyPtyExit() + await clock.advance(1_500) watch.notifyPtyExit() + await clock.advance(1_500) watch.notifyPtyExit() - await clock.advance(2_000) + await clock.advance(1_999) + expect(probe).toHaveBeenCalledTimes(before) + await clock.advance(1) expect(probe.mock.calls.length).toBe(before + 1) }) diff --git a/src/main/daemon/macos-login-session-death-watch.ts b/src/main/daemon/macos-login-session-death-watch.ts index e49e5b9091d..f08cf2dc03b 100644 --- a/src/main/daemon/macos-login-session-death-watch.ts +++ b/src/main/daemon/macos-login-session-death-watch.ts @@ -6,10 +6,6 @@ import type { SystemResolverHealth } from './types' // rejection storm (PAM db reload, OS update) from retiring a daemon whose login // session is still alive; a real logout rejects conclusively on every probe. const REQUIRED_CONSECUTIVE_REJECTIONS = 3 -// Why: a healthy session answers the PAM probe in milliseconds; probes that die by -// timeout repeatedly are a hang-shaped death signature (PAM blocking on a torn-down -// session). Higher threshold than rejections because load can also slow probes. -const REQUIRED_CONSECUTIVE_TIMEOUTS = 5 const PERIODIC_PROBE_MS = 120_000 const REJECTION_RECHECK_MS = 10_000 const PTY_EXIT_DEBOUNCE_MS = 2_000 @@ -29,7 +25,7 @@ export type MacosLoginSessionDeathWatchOptions = { probeLoginSession: () => Promise readResolverHealth: () => Promise onRetire: (details: { - cause: 'pam-rejections' | 'probe-timeouts' + cause: 'pam-rejections' rejections: number resolverHealth: SystemResolverHealth }) => void @@ -57,11 +53,9 @@ export type MacosLoginSessionDeathWatchOptions = { * * The oracle is the existing TCC login-shell PAM probe: it conclusively accepts * while the session is valid — including fast-user-switched-away sessions — and - * conclusively rejects once the session is destroyed. A hang-shaped death - * (probes repeatedly dying by timeout, which a live session never does) is a - * second, higher-threshold trigger. Retirement additionally requires the - * in-process system resolver to be degraded, so a PAM anomaly alone can never - * kill a healthy daemon. + * conclusively rejects once the session is destroyed. Retirement additionally + * requires the in-process system resolver to be explicitly degraded, so an + * inconclusive probe or PAM anomaly alone can never kill a healthy daemon. */ export class MacosLoginSessionDeathWatch { private readonly probeLoginSession: MacosLoginSessionDeathWatchOptions['probeLoginSession'] @@ -79,7 +73,6 @@ export class MacosLoginSessionDeathWatch { // a host where the wrapper never worked has no death signal to trust. private armed = false private consecutiveRejections = 0 - private consecutiveTimeouts = 0 private lastProbeStartedAtMs: number | null = null private probeInFlight = false private stopped = false @@ -129,9 +122,12 @@ export class MacosLoginSessionDeathWatch { /** A mass PTY-exit burst is what a logout's SIGHUP sweep looks like from inside the daemon. */ notifyPtyExit(): void { - if (this.stopped || this.ptyExitDebounceTimer !== null) { + if (this.stopped) { return } + if (this.ptyExitDebounceTimer !== null) { + this.clock.clearTimeout(this.ptyExitDebounceTimer) + } this.ptyExitDebounceTimer = this.clock.setTimeout(() => { this.ptyExitDebounceTimer = null void this.runProbe('pty-exit') @@ -189,28 +185,11 @@ export class MacosLoginSessionDeathWatch { return } if (!outcome.conclusive) { - if (this.armed && outcome.reason === 'timeout') { - // Why: healthy sessions answer in milliseconds even under load; a streak of - // SIGKILL-by-bound probes is PAM blocking on a torn-down session (the one - // failure shape a conclusive-verdict trigger would miss). - this.consecutiveTimeouts++ - this.log.log('login-session-probe-timeout', { - trigger, - timeouts: this.consecutiveTimeouts - }) - if (this.consecutiveTimeouts >= REQUIRED_CONSECUTIVE_TIMEOUTS) { - await this.retireIfResolverDegraded('probe-timeouts', () => { - this.consecutiveTimeouts = REQUIRED_CONSECUTIVE_TIMEOUTS - 1 - }) - return - } - this.scheduleNextProbe(this.rejectionRecheckMs) - return - } + // Why: repeated timeouts are still ambiguous on slow/offline PAM hosts; + // never turn an inconclusive probe into authority to orphan live PTYs. this.scheduleNextProbe(this.periodicProbeMs) return } - this.consecutiveTimeouts = 0 if (outcome.ok) { if (!this.armed) { this.log.log('login-session-watch-armed', { trigger }) @@ -234,7 +213,7 @@ export class MacosLoginSessionDeathWatch { this.scheduleNextProbe(this.rejectionRecheckMs) return } - await this.retireIfResolverDegraded('pam-rejections', () => { + await this.retireIfResolverDegraded(() => { this.consecutiveRejections = REQUIRED_CONSECUTIVE_REJECTIONS - 1 }) } catch (error) { @@ -247,23 +226,27 @@ export class MacosLoginSessionDeathWatch { } } - private async retireIfResolverDegraded( - cause: 'pam-rejections' | 'probe-timeouts', - holdAtThreshold: () => void - ): Promise { + private async retireIfResolverDegraded(holdAtThreshold: () => void): Promise { const resolverHealth = await this.readResolverHealth() if (this.stopped || this.retired) { return } - if (resolverHealth === 'healthy') { - // Why: a live session always has DNS configuration; the PAM anomaly alone must - // hold at the threshold and keep re-verdicting instead of killing live terminals. - this.log.log('login-session-retire-suppressed', { cause, resolverHealth }) + if (resolverHealth !== 'unhealthy') { + // Why: only explicit resolver degradation corroborates session death; unknown + // probe failures must preserve terminals and avoid a permanent fast retry loop. + this.log.log('login-session-retire-suppressed', { + cause: 'pam-rejections', + resolverHealth + }) holdAtThreshold() - this.scheduleNextProbe(this.rejectionRecheckMs) + this.scheduleNextProbe(this.periodicProbeMs) return } this.retired = true - this.onRetire({ cause, rejections: this.consecutiveRejections, resolverHealth }) + this.onRetire({ + cause: 'pam-rejections', + rejections: this.consecutiveRejections, + resolverHealth + }) } }