diff --git a/src/main/daemon/daemon-authenticated-client-activity.test.ts b/src/main/daemon/daemon-authenticated-client-activity.test.ts new file mode 100644 index 00000000000..7da5e00e387 --- /dev/null +++ b/src/main/daemon/daemon-authenticated-client-activity.test.ts @@ -0,0 +1,77 @@ +import { readFileSync, mkdtempSync, rmSync } from 'node:fs' +import { connect, type Socket } from 'node:net' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { DaemonServer } from './daemon-server' +import { getDaemonSocketPath } from './daemon-spawner' +import { encodeNdjson } from './ndjson' +import type { SubprocessHandle } from './session' +import { PROTOCOL_VERSION } from './types' + +function unusedSubprocess(): SubprocessHandle { + throw new Error('Test must not create a PTY') +} + +describe('daemon authenticated client activity', () => { + let dir: string + let socketPath: string + let tokenPath: string + let server: DaemonServer + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'daemon-client-activity-')) + socketPath = getDaemonSocketPath(dir) + tokenPath = join(dir, 'daemon.token') + }) + + afterEach(async () => { + await server?.shutdown() + rmSync(dir, { recursive: true, force: true }) + }) + + async function connectHello(role: 'control' | 'stream', clientId: string): Promise { + const socket = connect(socketPath) + await new Promise((resolve) => socket.once('connect', resolve)) + socket.write( + encodeNdjson({ + type: 'hello', + version: PROTOCOL_VERSION, + token: readFileSync(tokenPath, 'utf8').trim(), + clientId, + role + }) + ) + await new Promise((resolve, reject) => { + socket.once('data', (data) => { + const response = JSON.parse(data.toString().trim()) as { ok?: boolean; error?: string } + if (response.ok) { + resolve() + } else { + reject(new Error(response.error ?? 'hello rejected')) + } + }) + socket.once('error', reject) + }) + return socket + } + + it('excludes control-only health probes and reports one complete app pair', async () => { + const onAuthenticatedClientPair = vi.fn() + server = new DaemonServer({ + socketPath, + tokenPath, + spawnSubprocess: unusedSubprocess, + onAuthenticatedClientPair + }) + await server.start() + + await connectHello('control', 'resolver-health-check') + expect(onAuthenticatedClientPair).not.toHaveBeenCalled() + + await connectHello('control', 'app-client') + expect(onAuthenticatedClientPair).not.toHaveBeenCalled() + await connectHello('stream', 'app-client') + expect(onAuthenticatedClientPair).toHaveBeenCalledOnce() + }) +}) diff --git a/src/main/daemon/daemon-entry.test.ts b/src/main/daemon/daemon-entry.test.ts index deedb82a2c8..6b02210d72a 100644 --- a/src/main/daemon/daemon-entry.test.ts +++ b/src/main/daemon/daemon-entry.test.ts @@ -97,4 +97,17 @@ describe('daemon-entry parseArgs', () => { it('still requires --socket and --token when --log-file is given', () => { expect(() => parseArgs(['--log-file', '/tmp/daemon.log'])).toThrow('Usage:') }) + + it('parses the GUI-only --login-session-watch flag and omits it when absent', () => { + expect( + parseArgs(['--socket', '/tmp/t.sock', '--token', '/tmp/t.token', '--login-session-watch']) + ).toEqual({ + socketPath: '/tmp/t.sock', + tokenPath: '/tmp/t.token', + loginSessionWatch: true + }) + expect(parseArgs(['--socket', '/tmp/t.sock', '--token', '/tmp/t.token'])).not.toHaveProperty( + 'loginSessionWatch' + ) + }) }) diff --git a/src/main/daemon/daemon-entry.ts b/src/main/daemon/daemon-entry.ts index 6488ffce048..ba98af21c5b 100644 --- a/src/main/daemon/daemon-entry.ts +++ b/src/main/daemon/daemon-entry.ts @@ -6,19 +6,27 @@ * Signals readiness to parent via IPC: { type: 'ready' } * Shuts down cleanly on SIGTERM. */ +import { readFileSync } from 'node:fs' import { startDaemon, type DaemonHandle } from './daemon-main' import { createPtySubprocess } from './pty-subprocess' import { warmWindowsConptyOnce } from './windows-conpty-warmup' import { warmPwshAvailabilityCache } from '../pwsh' import { createDaemonFileLog, createNoopDaemonFileLog } from './daemon-file-log' import { PROTOCOL_VERSION } from './types' -import { prepareMacosTccLoginShell } from '../providers/macos-tcc-login-shell' +import { + prepareMacosTccLoginShell, + probeMacosLoginSessionAlive +} from '../providers/macos-tcc-login-shell' +import { MacosLoginSessionDeathWatch } from './macos-login-session-death-watch' +import { readCurrentProcessMacSystemResolverHealth } from '../network/macos-system-resolver-health' export type ParsedDaemonArgs = { socketPath: string tokenPath: string pidPath?: string launchNonce?: string + /** GUI-spawned daemons only — headless serve/SSH daemons must survive session loss. */ + loginSessionWatch?: boolean /** Optional — absent for adopted old daemons and tests, which log nothing. */ logFilePath?: string } @@ -29,6 +37,7 @@ export function parseArgs(argv: string[]): ParsedDaemonArgs { let logFilePath = '' let pidPath = '' let launchNonce = '' + let loginSessionWatch = false for (let i = 0; i < argv.length; i++) { if (argv[i] === '--socket' && argv[i + 1]) { @@ -46,6 +55,8 @@ export function parseArgs(argv: string[]): ParsedDaemonArgs { } else if (argv[i] === '--launch-nonce' && argv[i + 1]) { launchNonce = argv[i + 1] i++ + } else if (argv[i] === '--login-session-watch') { + loginSessionWatch = true } } @@ -61,6 +72,7 @@ export function parseArgs(argv: string[]): ParsedDaemonArgs { socketPath, tokenPath, ...(pidPath ? { pidPath, launchNonce } : {}), + ...(loginSessionWatch ? { loginSessionWatch } : {}), ...(logFilePath ? { logFilePath } : {}) } } @@ -73,7 +85,7 @@ async function main(): Promise { // an otherwise healthy detached daemon. Swallow it: stderr is diagnostic only. process.stderr.on('error', () => {}) - const { socketPath, tokenPath, pidPath, launchNonce, logFilePath } = parseArgs( + const { socketPath, tokenPath, pidPath, launchNonce, loginSessionWatch, logFilePath } = parseArgs( process.argv.slice(2) ) const startedAtMs = Date.now() - process.uptime() * 1000 @@ -123,6 +135,7 @@ async function main(): Promise { }) let daemon: DaemonHandle | null = null + let deathWatch: MacosLoginSessionDeathWatch | null = null let shuttingDown = false // Bound the wait so a wedged native shutdown can't leave the daemon running // forever on SIGTERM/SIGINT (it would then survive a real quit, not just updates). @@ -134,6 +147,7 @@ async function main(): Promise { return } shuttingDown = true + deathWatch?.stop() daemonLog.log('shutdown', { reason }) try { if (daemon) { @@ -157,6 +171,63 @@ async function main(): Promise { process.on('SIGTERM', () => void shutdown('SIGTERM')) process.on('SIGINT', () => void shutdown('SIGINT')) + // 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 fail-safe path), else inconclusive. + const e2eProbeFile = process.env.ORCA_E2E_LOGIN_SESSION_PROBE_FILE + const readE2eVerdict = (): string => { + try { + return readFileSync(e2eProbeFile as string, 'utf8').trim() + } catch { + return '' + } + } + deathWatch = + loginSessionWatch && process.platform === 'darwin' + ? new MacosLoginSessionDeathWatch({ + probeLoginSession: e2eProbeFile + ? async () => { + const verdict = readE2eVerdict() + if (verdict === 'alive') { + return { ok: true, conclusive: true, reason: 'accepted' } + } + if (verdict === 'dead') { + return { ok: false, conclusive: true, reason: 'rejected' } + } + return { ok: false, conclusive: false, reason: 'timeout' } + } + : probeMacosLoginSessionAlive, + readResolverHealth: e2eProbeFile + ? async () => { + const verdict = readE2eVerdict() + return verdict === 'dead' || verdict === 'hang' ? 'unhealthy' : 'healthy' + } + : readCurrentProcessMacSystemResolverHealth, + ...(e2eProbeFile + ? { + timing: { + periodicProbeMs: 2_000, + rejectionRecheckMs: 500, + ptyExitDebounceMs: 200, + clientActivityMinGapMs: 1_000, + minProbeGapMs: 100 + } + } + : {}), + log: daemonLog, + onRetire: (details) => { + shuttingDown = true + daemonLog.log('login-session-dead-retire', details) + daemonLog.close() + // Why: crash-style exit (no PTY teardown) keeps session meta unclean so the + // replacement daemon cold-restores scrollback; stale socket/pid files ride + // the existing dead-endpoint recovery. + process.exit(1) + } + }) + : null + daemon = await startDaemon({ socketPath, tokenPath, @@ -165,14 +236,22 @@ async function main(): Promise { ...(pidPath ? { startedAtMs } : {}), log: daemonLog, preparePtySpawn: runMacosLoginPreflight, + ...(deathWatch + ? { + onPtySessionExit: () => deathWatch.notifyPtyExit(), + onAuthenticatedClientPair: () => deathWatch.notifyClientActivity() + } + : {}), spawnSubprocess: (opts) => createPtySubprocess(opts), onIdleShutdown: () => { + deathWatch?.stop() shuttingDown = true daemonLog.log('shutdown', { reason: 'idle' }) daemonLog.close() process.exit(0) } }) + deathWatch?.start() // Signal readiness to parent via IPC (if available) if (process.send) { diff --git a/src/main/daemon/daemon-init.test.ts b/src/main/daemon/daemon-init.test.ts index c1f72dbc63f..5a3003f767d 100644 --- a/src/main/daemon/daemon-init.test.ts +++ b/src/main/daemon/daemon-init.test.ts @@ -1086,7 +1086,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { it('respawns instead of reusing a healthy daemon launched from another app path', async () => { const mod = await importFresh() - await mod.initDaemonPtyProvider() + await mod.initDaemonPtyProvider(undefined, { macosLoginSessionWatch: true }) const launcher = spawnerInstances[0].launcher as ( socketPath: string, @@ -1137,6 +1137,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { '/fake/socket', '--token', '/fake/token', + '--login-session-watch', '--log-file', join(FAKE_USER_DATA_PATH, 'logs', 'daemon.log') ]), @@ -1513,6 +1514,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { await launcher('/fake/socket', '/fake/token') + const launchedArgsWithoutWatch = forkMock.mock.calls.at(-1)?.[1] as string[] + expect(launchedArgsWithoutWatch).not.toContain('--login-session-watch') + expect(offMock).toHaveBeenCalledWith('message', expect.any(Function)) expect(offMock).toHaveBeenCalledWith('error', expect.any(Function)) expect(offMock).toHaveBeenCalledWith('exit', expect.any(Function)) diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index 32e3baf9365..a748ea10a8b 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -321,7 +321,10 @@ async function shouldPreserveDaemonWithLiveSessions( return true } -function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher { +function createOutOfProcessLauncher( + runtimeDir: string, + macosLoginSessionWatch = false +): DaemonLauncher { return async (socketPath, tokenPath, suppliedPidPath, suppliedLaunchNonce) => { const entryPath = getDaemonEntryPath() const pidPath = suppliedPidPath ?? getDaemonPidPath(runtimeDir) @@ -448,6 +451,7 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher { pidPath, '--launch-nonce', launchNonce, + ...(macosLoginSessionWatch ? ['--login-session-watch'] : []), ...daemonLogArgs() ], { @@ -625,7 +629,10 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher { } } -export async function initDaemonPtyProvider(signal?: AbortSignal): Promise { +export async function initDaemonPtyProvider( + signal?: AbortSignal, + options: { macosLoginSessionWatch?: boolean } = {} +): Promise { logDaemonMilestone('daemon-init-start') // Why: e2e coverage for the startup PTY gate (#5232) needs a daemon init that deterministically outlasts the first-window timeout. const e2eInitDelayMs = Number(process.env.ORCA_E2E_DAEMON_INIT_DELAY_MS) @@ -636,7 +643,7 @@ export async function initDaemonPtyProvider(signal?: AbortSignal): Promise const newSpawner = new DaemonSpawner({ runtimeDir, - launcher: createOutOfProcessLauncher(runtimeDir) + launcher: createOutOfProcessLauncher(runtimeDir, options.macosLoginSessionWatch ?? false) }) // Why: assign the module-level spawner/adapter only after both succeed, so a failed ensureRunning() leaves no stale spawner. diff --git a/src/main/daemon/daemon-main.ts b/src/main/daemon/daemon-main.ts index 9884cdf0c80..3976c6c21e6 100644 --- a/src/main/daemon/daemon-main.ts +++ b/src/main/daemon/daemon-main.ts @@ -11,6 +11,8 @@ export type DaemonStartOptions = { protocolVersion?: number spawnSubprocess: DaemonServerOptions['spawnSubprocess'] preparePtySpawn?: DaemonServerOptions['preparePtySpawn'] + onPtySessionExit?: DaemonServerOptions['onPtySessionExit'] + onAuthenticatedClientPair?: DaemonServerOptions['onAuthenticatedClientPair'] log?: DaemonFileLog onIdleShutdown?: () => void initialAdoptionTestConfig?: DaemonServerOptions['initialAdoptionTestConfig'] @@ -30,6 +32,10 @@ export async function startDaemon(opts: DaemonStartOptions): Promise Promise preparePtySpawn?: () => Promise + // Why: login-session death detection (#7936) probes on PTY-exit bursts and fresh app connections. + onPtySessionExit?: (sessionId: string) => void + onAuthenticatedClientPair?: () => void log?: DaemonFileLog spawnSubprocess: (opts: { sessionId: string @@ -101,6 +104,7 @@ export class DaemonServer { private startedAtMs: number | null private protocolVersion: number private onIdleShutdown: () => void + private onAuthenticatedClientPair: () => void private ptySpawnHealthCheck: () => Promise private preparePtySpawn: () => Promise private log: DaemonFileLog @@ -180,7 +184,11 @@ export class DaemonServer { now: () => Date.now() } this.token = randomUUID() - this.host = new TerminalHost({ spawnSubprocess: opts.spawnSubprocess }) + this.onAuthenticatedClientPair = opts.onAuthenticatedClientPair ?? (() => {}) + this.host = new TerminalHost({ + spawnSubprocess: opts.spawnSubprocess, + ...(opts.onPtySessionExit ? { onSessionReaped: opts.onPtySessionExit } : {}) + }) this.ptySpawnHealthCheck = opts.ptySpawnHealthCheck ?? checkPtySpawnHealth this.preparePtySpawn = opts.preparePtySpawn ?? (() => Promise.resolve()) this.stopStreamBacklogProbe = startDaemonStreamBacklogProbe(() => ({ @@ -481,6 +489,8 @@ export class DaemonServer { } this.setupStreamSocket(socket, client) client.authenticatedPairEstablished = true + // Why: one-shot health probes authenticate only a control socket; they are not fresh app activity. + this.onAuthenticatedClientPair() // A complete app connection (unlike a probe) re-owns the endpoint and cancels pending retirement. this.initialAdoptionDeadlineMs = null this.retirementRequested = false diff --git a/src/main/daemon/macos-login-session-death-watch.test.ts b/src/main/daemon/macos-login-session-death-watch.test.ts new file mode 100644 index 00000000000..1531a33ab20 --- /dev/null +++ b/src/main/daemon/macos-login-session-death-watch.test.ts @@ -0,0 +1,374 @@ +import { describe, expect, it, vi } from 'vitest' +import type { LoginPreflightOutcome } from '../providers/macos-tcc-login-shell' +import { createNoopDaemonFileLog } from './daemon-file-log' +import { + MacosLoginSessionDeathWatch, + type MacosLoginSessionDeathWatchOptions +} from './macos-login-session-death-watch' +import type { SystemResolverHealth } from './types' + +const ACCEPTED: LoginPreflightOutcome = { ok: true, conclusive: true, reason: 'accepted' } +const REJECTED: LoginPreflightOutcome = { ok: false, conclusive: true, reason: 'rejected' } +const INCONCLUSIVE: LoginPreflightOutcome = { ok: false, conclusive: false, reason: 'timeout' } + +type FakeTimer = { at: number; callback: () => void; cleared: boolean } + +class FakeClock { + private timers: FakeTimer[] = [] + private nowMs = 0 + + setTimeout = (callback: () => void, delayMs: number): unknown => { + const timer: FakeTimer = { at: this.nowMs + delayMs, callback, cleared: false } + this.timers.push(timer) + return timer + } + + clearTimeout = (handle: unknown): void => { + ;(handle as FakeTimer).cleared = true + } + + now = (): number => this.nowMs + + async advance(ms: number): Promise { + const target = this.nowMs + ms + for (;;) { + const due = this.timers + .filter((t) => !t.cleared && t.at <= target) + .sort((a, b) => a.at - b.at)[0] + if (!due) { + break + } + this.nowMs = due.at + due.cleared = true + due.callback() + // Why: probes are async; let their promise chains settle before firing the next timer. + await drainMicrotasks() + } + this.nowMs = target + } + + pendingCount(): number { + return this.timers.filter((t) => !t.cleared).length + } +} + +async function drainMicrotasks(): Promise { + for (let i = 0; i < 10; i++) { + await Promise.resolve() + } +} + +function createWatch( + overrides: Partial & { + outcomes?: (LoginPreflightOutcome | null)[] + } = {} +): { + watch: MacosLoginSessionDeathWatch + clock: FakeClock + onRetire: ReturnType + probe: ReturnType + setResolverHealth: (health: SystemResolverHealth) => void +} { + const clock = new FakeClock() + const onRetire = vi.fn() + const outcomes = overrides.outcomes ?? [] + // Why length-check, not `??`: an explicit null outcome (wrapper not applicable) must reach the watch. + const probe = vi.fn(async () => (outcomes.length ? outcomes.shift()! : ACCEPTED)) + let resolverHealth: SystemResolverHealth = 'unhealthy' + const watch = new MacosLoginSessionDeathWatch({ + probeLoginSession: overrides.probeLoginSession ?? probe, + readResolverHealth: overrides.readResolverHealth ?? (async () => resolverHealth), + onRetire: overrides.onRetire ?? onRetire, + log: overrides.log ?? createNoopDaemonFileLog(), + clock, + timing: { + periodicProbeMs: 120_000, + rejectionRecheckMs: 10_000, + ptyExitDebounceMs: 2_000, + clientActivityMinGapMs: 30_000, + minProbeGapMs: 5_000, + ...overrides.timing + } + }) + return { + watch, + clock, + onRetire, + probe, + setResolverHealth: (health) => { + resolverHealth = health + } + } +} + +describe('MacosLoginSessionDeathWatch', () => { + it('retires after consecutive conclusive rejections once armed, with a degraded resolver', async () => { + const { watch, clock, onRetire } = createWatch({ + outcomes: [ACCEPTED, REJECTED, REJECTED, REJECTED] + }) + watch.start() + await drainMicrotasks() + expect(onRetire).not.toHaveBeenCalled() + + await clock.advance(120_000) // periodic → rejection 1 + await clock.advance(10_000) // recheck → rejection 2 + expect(onRetire).not.toHaveBeenCalled() + await clock.advance(10_000) // recheck → rejection 3 → retire + expect(onRetire).toHaveBeenCalledWith({ + cause: 'pam-rejections', + rejections: 3, + resolverHealth: 'unhealthy' + }) + }) + + it('never retires when the session was never conclusively accepted', async () => { + const { watch, clock, onRetire } = createWatch({ + outcomes: [REJECTED, REJECTED, REJECTED, REJECTED, REJECTED] + }) + watch.start() + await drainMicrotasks() + for (let i = 0; i < 4; i++) { + await clock.advance(120_000) + } + expect(onRetire).not.toHaveBeenCalled() + }) + + it('resets the rejection streak on a conclusive acceptance', async () => { + const { watch, clock, onRetire } = createWatch({ + outcomes: [ACCEPTED, REJECTED, REJECTED, ACCEPTED, REJECTED, REJECTED] + }) + watch.start() + await drainMicrotasks() + await clock.advance(120_000) // rejection 1 + await clock.advance(10_000) // rejection 2 + await clock.advance(10_000) // acceptance → reset + await clock.advance(120_000) // rejection 1 + await clock.advance(10_000) // rejection 2 + expect(onRetire).not.toHaveBeenCalled() + }) + + it('keeps the rejection streak across interleaved inconclusive probes', async () => { + const { watch, clock, onRetire } = createWatch({ + outcomes: [ACCEPTED, REJECTED, INCONCLUSIVE, REJECTED, INCONCLUSIVE, REJECTED] + }) + watch.start() + await drainMicrotasks() + await clock.advance(120_000) // rejection 1 + 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(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('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: [ACCEPTED, ...Array.from({ length: 10 }, () => INCONCLUSIVE)], + readResolverHealth + }) + watch.start() + await drainMicrotasks() + for (let i = 0; i < 10; i++) { + await clock.advance(120_000) + } + expect(probe).toHaveBeenCalledTimes(11) + expect(readResolverHealth).not.toHaveBeenCalled() + expect(onRetire).not.toHaveBeenCalled() + }) + + 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) + } + ) + + 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(1_999) + expect(probe).toHaveBeenCalledTimes(before) + await clock.advance(1) + expect(probe.mock.calls.length).toBe(before + 1) + }) + + it('probes on client activity only after the min gap', async () => { + const { watch, clock, probe } = createWatch({ + outcomes: [ACCEPTED, ACCEPTED, ACCEPTED] + }) + watch.start() + await drainMicrotasks() + const after = probe.mock.calls.length + watch.notifyClientActivity() // too soon after startup probe, so retain one deferred probe + await clock.advance(29_999) + expect(probe.mock.calls.length).toBe(after) + await clock.advance(1) + expect(probe.mock.calls.length).toBe(after + 1) + }) + + it('defers a PTY-exit trigger that lands inside the global probe gap', async () => { + const { watch, clock, probe } = createWatch({ outcomes: [ACCEPTED, ACCEPTED] }) + watch.start() + await drainMicrotasks() + + watch.notifyPtyExit() + await clock.advance(4_999) + expect(probe).toHaveBeenCalledOnce() + await clock.advance(1) + expect(probe).toHaveBeenCalledTimes(2) + }) + + it('retains one follow-up when a logout signal arrives during a probe', async () => { + let resolveStartup!: (outcome: LoginPreflightOutcome) => void + const startup = new Promise((resolve) => { + resolveStartup = resolve + }) + const probe = vi + .fn() + .mockReturnValueOnce(startup) + .mockResolvedValue(ACCEPTED) + const { watch, clock } = createWatch({ probeLoginSession: probe }) + watch.start() + watch.notifyPtyExit() + await clock.advance(2_000) + expect(probe).toHaveBeenCalledOnce() + + resolveStartup(ACCEPTED) + await drainMicrotasks() + await clock.advance(2_999) + expect(probe).toHaveBeenCalledOnce() + await clock.advance(1) + expect(probe).toHaveBeenCalledTimes(2) + }) + + it('retains client activity that arrives during an armed periodic probe', async () => { + let resolvePeriodic!: (outcome: LoginPreflightOutcome) => void + const periodic = new Promise((resolve) => { + resolvePeriodic = resolve + }) + const probe = vi + .fn() + .mockResolvedValueOnce(ACCEPTED) + .mockReturnValueOnce(periodic) + .mockResolvedValue(ACCEPTED) + const { watch, clock } = createWatch({ probeLoginSession: probe }) + watch.start() + await drainMicrotasks() + await clock.advance(120_000) + + watch.notifyClientActivity() + resolvePeriodic(ACCEPTED) + await drainMicrotasks() + await clock.advance(29_999) + expect(probe).toHaveBeenCalledTimes(2) + await clock.advance(1) + expect(probe).toHaveBeenCalledTimes(3) + }) + + it('disables itself when the wrapper machinery does not apply', async () => { + const { watch, clock, probe } = createWatch({ outcomes: [null] }) + watch.start() + await drainMicrotasks() + expect(probe).toHaveBeenCalledTimes(1) + await clock.advance(600_000) + watch.notifyPtyExit() + watch.notifyClientActivity() + await clock.advance(600_000) + expect(probe).toHaveBeenCalledTimes(1) + }) + + it('stop() cancels all pending timers', async () => { + const { watch, clock } = createWatch({ outcomes: [ACCEPTED] }) + watch.start() + await drainMicrotasks() + watch.notifyPtyExit() + watch.stop() + expect(clock.pendingCount()).toBe(0) + }) + + it('stop() aborts an in-flight subprocess probe', async () => { + let probeSignal: AbortSignal | undefined + const probe = vi.fn((signal?: AbortSignal) => { + probeSignal = signal + return new Promise(() => {}) + }) + const { watch } = createWatch({ probeLoginSession: probe }) + watch.start() + expect(probeSignal?.aborted).toBe(false) + + watch.stop() + + expect(probeSignal?.aborted).toBe(true) + }) + + it('stop() prevents an in-flight resolver check from retiring the daemon', async () => { + let resolveHealth!: (health: SystemResolverHealth) => void + let resolverSignal: AbortSignal | undefined + const resolverHealth = new Promise((resolve) => { + resolveHealth = resolve + }) + const { watch, clock, onRetire } = createWatch({ + outcomes: [ACCEPTED, REJECTED, REJECTED, REJECTED], + readResolverHealth: (signal) => { + resolverSignal = signal + return resolverHealth + } + }) + watch.start() + await drainMicrotasks() + await clock.advance(120_000) + await clock.advance(10_000) + await clock.advance(10_000) // retirement is now waiting on resolver health + + watch.stop() + expect(resolverSignal?.aborted).toBe(true) + resolveHealth('unhealthy') + await drainMicrotasks() + + expect(onRetire).not.toHaveBeenCalled() + }) + + it('logs and reschedules when a probe throws instead of surfacing a rejection', async () => { + const outcomes: (() => Promise)[] = [ + async () => ACCEPTED, + async () => { + throw new Error('launchctl exploded') + }, + async () => ACCEPTED + ] + const probe = vi.fn(() => (outcomes.shift() ?? (async () => ACCEPTED))()) + const { watch, clock, onRetire } = createWatch({ probeLoginSession: probe }) + watch.start() + await drainMicrotasks() + await clock.advance(120_000) // throwing probe + await clock.advance(120_000) // rescheduled probe still runs + expect(probe).toHaveBeenCalledTimes(3) + expect(onRetire).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/daemon/macos-login-session-death-watch.ts b/src/main/daemon/macos-login-session-death-watch.ts new file mode 100644 index 00000000000..d0cadccac18 --- /dev/null +++ b/src/main/daemon/macos-login-session-death-watch.ts @@ -0,0 +1,317 @@ +import type { LoginPreflightOutcome } from '../providers/macos-tcc-login-shell' +import type { DaemonFileLog } from './daemon-file-log' +import type { SystemResolverHealth } from './types' + +// Why: three conclusive PAM verdicts spread over recheck intervals keep a transient +// 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 +const PERIODIC_PROBE_MS = 120_000 +const REJECTION_RECHECK_MS = 10_000 +const PTY_EXIT_DEBOUNCE_MS = 2_000 +// Why: a client hello right after login is the fastest death signal for a stale +// daemon, but steady reconnects must not turn hellos into a PAM probe storm. +const CLIENT_ACTIVITY_MIN_GAP_MS = 30_000 +const MIN_PROBE_GAP_MS = 5_000 + +type WatchClock = { + setTimeout(callback: () => void, delayMs: number): unknown + clearTimeout(handle: unknown): void + now(): number +} + +export type MacosLoginSessionDeathWatchOptions = { + /** Fresh PAM probe (cache-bypassing); null when the login wrapper doesn't apply on this host. */ + probeLoginSession: (signal?: AbortSignal) => Promise + readResolverHealth: (signal?: AbortSignal) => Promise + onRetire: (details: { + cause: 'pam-rejections' + rejections: number + resolverHealth: SystemResolverHealth + }) => void + log: DaemonFileLog + /** Direct-construction seam for deterministic tests; production uses real timers. */ + clock?: WatchClock + timing?: Partial<{ + periodicProbeMs: number + rejectionRecheckMs: number + ptyExitDebounceMs: number + clientActivityMinGapMs: number + minProbeGapMs: number + }> +} + +/** + * Detects that the macOS GUI login session this daemon was born into has died + * (full logout / WindowServer session teardown) and retires the daemon so the + * next app start cold-starts a replacement inside the live session (#7936). + * + * A daemon in a dead login session is unsalvageable: its PAM context can no + * longer host `login(1)` spawns ("Login incorrect" zombies) and its Mach + * bootstrap namespace has lost the system DNS resolver, so every terminal it + * hosts has no egress. Retirement is the only converging heal. + * + * 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. 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'] + private readonly readResolverHealth: MacosLoginSessionDeathWatchOptions['readResolverHealth'] + private readonly onRetire: MacosLoginSessionDeathWatchOptions['onRetire'] + private readonly log: DaemonFileLog + private readonly clock: WatchClock + private readonly periodicProbeMs: number + private readonly rejectionRecheckMs: number + private readonly ptyExitDebounceMs: number + private readonly clientActivityMinGapMs: number + private readonly minProbeGapMs: number + + // Why: retire only a daemon that once proved its session could host login(1); + // a host where the wrapper never worked has no death signal to trust. + private armed = false + private consecutiveRejections = 0 + private lastProbeStartedAtMs: number | null = null + private probeInFlight = false + private stopped = false + private retired = false + private scheduledProbeTimer: unknown | null = null + private scheduledProbeAtMs: number | null = null + private ptyExitDebounceTimer: unknown | null = null + private pendingProbeTrigger: string | null = null + private probeAbortController: AbortController | null = null + + constructor(opts: MacosLoginSessionDeathWatchOptions) { + this.probeLoginSession = opts.probeLoginSession + this.readResolverHealth = opts.readResolverHealth + this.onRetire = opts.onRetire + this.log = opts.log + this.clock = opts.clock ?? { + setTimeout: (callback, delayMs) => { + const timer = setTimeout(callback, delayMs) + timer.unref() + return timer + }, + clearTimeout: (handle) => clearTimeout(handle as ReturnType), + now: () => Date.now() + } + this.periodicProbeMs = opts.timing?.periodicProbeMs ?? PERIODIC_PROBE_MS + this.rejectionRecheckMs = opts.timing?.rejectionRecheckMs ?? REJECTION_RECHECK_MS + this.ptyExitDebounceMs = opts.timing?.ptyExitDebounceMs ?? PTY_EXIT_DEBOUNCE_MS + this.clientActivityMinGapMs = opts.timing?.clientActivityMinGapMs ?? CLIENT_ACTIVITY_MIN_GAP_MS + this.minProbeGapMs = opts.timing?.minProbeGapMs ?? MIN_PROBE_GAP_MS + } + + start(): void { + if (this.stopped) { + return + } + void this.runProbe('startup') + } + + stop(): void { + this.stopped = true + this.probeAbortController?.abort() + this.probeAbortController = null + if (this.scheduledProbeTimer !== null) { + this.clock.clearTimeout(this.scheduledProbeTimer) + this.scheduledProbeTimer = null + this.scheduledProbeAtMs = null + } + if (this.ptyExitDebounceTimer !== null) { + this.clock.clearTimeout(this.ptyExitDebounceTimer) + this.ptyExitDebounceTimer = null + } + } + + /** A mass PTY-exit burst is what a logout's SIGHUP sweep looks like from inside the daemon. */ + notifyPtyExit(): void { + 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') + }, this.ptyExitDebounceMs) + } + + notifyClientActivity(): void { + if (this.stopped) { + return + } + if (this.probeInFlight) { + if (this.armed) { + this.retainPendingProbe('client-hello') + } + return + } + const elapsedSinceProbe = + this.lastProbeStartedAtMs === null ? null : this.clock.now() - this.lastProbeStartedAtMs + if (elapsedSinceProbe !== null && elapsedSinceProbe < this.clientActivityMinGapMs) { + // Why: dropping a post-login hello here can defer stale-daemon recovery to the two-minute backstop. + this.scheduleProbeNoLaterThan(this.clientActivityMinGapMs - elapsedSinceProbe, 'client-hello') + return + } + void this.runProbe('client-hello') + } + + private scheduleProbe(delayMs: number, trigger: string): void { + if (this.stopped) { + return + } + if (this.scheduledProbeTimer !== null) { + this.clock.clearTimeout(this.scheduledProbeTimer) + } + this.scheduledProbeAtMs = this.clock.now() + delayMs + this.scheduledProbeTimer = this.clock.setTimeout(() => { + this.scheduledProbeTimer = null + this.scheduledProbeAtMs = null + void this.runProbe(trigger) + }, delayMs) + } + + private scheduleNextProbe(delayMs: number): void { + this.scheduleProbe(delayMs, 'periodic') + } + + private scheduleProbeNoLaterThan(delayMs: number, trigger: string): void { + const requestedAtMs = this.clock.now() + delayMs + if (this.scheduledProbeAtMs !== null && this.scheduledProbeAtMs <= requestedAtMs) { + return + } + this.scheduleProbe(delayMs, trigger) + } + + private probeGapMs(trigger: string): number { + return trigger === 'client-hello' ? this.clientActivityMinGapMs : this.minProbeGapMs + } + + private retainPendingProbe(trigger: string): void { + if ( + this.pendingProbeTrigger === null || + this.probeGapMs(trigger) < this.probeGapMs(this.pendingProbeTrigger) + ) { + this.pendingProbeTrigger = trigger + } + } + + private async runProbe(trigger: string): Promise { + if (this.stopped || this.retired) { + return + } + if (this.probeInFlight) { + // Why: the current probe may describe the pre-logout state; retain one follow-up without polling. + this.retainPendingProbe(trigger) + return + } + const elapsedSinceProbe = + this.lastProbeStartedAtMs === null ? null : this.clock.now() - this.lastProbeStartedAtMs + if ( + trigger !== 'startup' && + elapsedSinceProbe !== null && + elapsedSinceProbe < this.minProbeGapMs + ) { + this.scheduleProbeNoLaterThan(this.minProbeGapMs - elapsedSinceProbe, trigger) + return + } + this.probeInFlight = true + this.lastProbeStartedAtMs = this.clock.now() + const abortController = new AbortController() + this.probeAbortController = abortController + try { + const outcome = await this.probeLoginSession(abortController.signal) + if (this.stopped || this.retired) { + return + } + if (outcome === null) { + // Why: no wrapper machinery means no PAM oracle — watching would only ever misfire. + this.log.log('login-session-watch-disabled', { trigger }) + this.stop() + return + } + if (!outcome.conclusive) { + // 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 + } + if (outcome.ok) { + if (!this.armed) { + this.log.log('login-session-watch-armed', { trigger }) + } + this.armed = true + this.consecutiveRejections = 0 + this.scheduleNextProbe(this.periodicProbeMs) + return + } + if (!this.armed) { + // Session never hosted login(1) here; the preflight already degraded spawns. + this.scheduleNextProbe(this.periodicProbeMs) + return + } + this.consecutiveRejections++ + this.log.log('login-session-probe-rejected', { + trigger, + rejections: this.consecutiveRejections + }) + if (this.consecutiveRejections < REQUIRED_CONSECUTIVE_REJECTIONS) { + this.scheduleNextProbe(this.rejectionRecheckMs) + return + } + await this.retireIfResolverDegraded(() => { + this.consecutiveRejections = REQUIRED_CONSECUTIVE_REJECTIONS - 1 + }, abortController.signal) + } catch (error) { + // Why: a probe failure is diagnostic only; an escaped rejection would trip the + // daemon's fatal unhandled-error path and kill live terminals. + this.log.log('login-session-probe-error', { message: (error as Error)?.message }) + this.scheduleNextProbe(this.periodicProbeMs) + } finally { + this.probeInFlight = false + if (this.probeAbortController === abortController) { + this.probeAbortController = null + } + const pendingTrigger = this.pendingProbeTrigger + this.pendingProbeTrigger = null + if (!this.stopped && !this.retired && pendingTrigger !== null) { + const elapsed = this.clock.now() - (this.lastProbeStartedAtMs ?? this.clock.now()) + this.scheduleProbeNoLaterThan( + Math.max(0, this.probeGapMs(pendingTrigger) - elapsed), + pendingTrigger + ) + } + } + } + + private async retireIfResolverDegraded( + holdAtThreshold: () => void, + signal: AbortSignal + ): Promise { + const resolverHealth = await this.readResolverHealth(signal) + if (this.stopped || this.retired) { + return + } + 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.periodicProbeMs) + return + } + this.retired = true + this.onRetire({ + cause: 'pam-rejections', + rejections: this.consecutiveRejections, + resolverHealth + }) + } +} diff --git a/src/main/daemon/terminal-host-options.ts b/src/main/daemon/terminal-host-options.ts index 55496f9e50f..b483914bba3 100644 --- a/src/main/daemon/terminal-host-options.ts +++ b/src/main/daemon/terminal-host-options.ts @@ -18,6 +18,8 @@ export type TerminalHostOptions = { terminalWindowsWslDistro?: string | null terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe' }) => SubprocessHandle + // Why: login-session death detection (#7936) needs subprocess exits even when no client is attached. + onSessionReaped?: (sessionId: string) => void // Why: graceful shutdown checkpoints must finish in-process before teardown. onFinalCheckpoint?: ( sessionId: string, diff --git a/src/main/daemon/terminal-host.test.ts b/src/main/daemon/terminal-host.test.ts index cee72eee1cf..e7e83e31a31 100644 --- a/src/main/daemon/terminal-host.test.ts +++ b/src/main/daemon/terminal-host.test.ts @@ -883,6 +883,8 @@ describe('TerminalHost', () => { }) it('does not list exited sessions', async () => { + const onSessionReaped = vi.fn() + host = new TerminalHost({ spawnSubprocess: spawnFn as MockSpawnFn, onSessionReaped }) await host.createOrAttach({ sessionId: 'session-1', cols: 80, @@ -892,6 +894,7 @@ describe('TerminalHost', () => { lastSubprocess._onExitCb?.(0) expect(host.listSessions()).toEqual([]) + expect(onSessionReaped).toHaveBeenCalledWith('session-1') }) it('never force-kills an exited session (recycled-pid SIGKILL safety)', async () => { diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index 796f1939fc6..6be96d27d8e 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -27,6 +27,7 @@ export class TerminalHost { private sessionTeardown = new TerminalSessionTeardown(this.sessions) private killedTombstones: TerminalHostTombstones private spawnSubprocess: TerminalHostOptions['spawnSubprocess'] + private onSessionReaped: TerminalHostOptions['onSessionReaped'] private onFinalCheckpoint: TerminalHostOptions['onFinalCheckpoint'] private maxTombstones: number private creationFenced = false @@ -36,6 +37,7 @@ export class TerminalHost { constructor(opts: TerminalHostOptions) { this.spawnSubprocess = opts.spawnSubprocess + this.onSessionReaped = opts.onSessionReaped this.onFinalCheckpoint = opts.onFinalCheckpoint this.maxTombstones = opts.maxTombstones ?? DEFAULT_MAX_TOMBSTONES this.killedTombstones = new TerminalHostTombstones(this.maxTombstones) @@ -119,6 +121,7 @@ export class TerminalHost { } session.dispose() this.sessions.delete(sessionId) + this.onSessionReaped?.(sessionId) } signal(sessionId: string, sig: string): void { diff --git a/src/main/index.ts b/src/main/index.ts index 494760c48c4..9ab35ae3b57 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -674,7 +674,11 @@ function startTerminalRuntimeStartupServices(): Promise { // Why: both desktop and headless serve must adopt the same persistent provider before creating terminals or a renderer. startDaemonPtyProvider: async (signal) => { logStartupMilestone('startup-service-start', { service: 'daemon-pty-provider' }) - await initDaemonPtyProvider(signal) + // Why: only GUI-spawned macOS daemons watch for login-session death; a headless + // serve daemon must survive its spawning session ending (SSH disconnect). + await initDaemonPtyProvider(signal, { + macosLoginSessionWatch: process.platform === 'darwin' && !isServeMode + }) logStartupMilestone('startup-service-done', { service: 'daemon-pty-provider' }) }, // Why: PTY spawn env reads ORCA_AGENT_HOOK_* from live server state, so the renderer awaits this before restored terminals reconnect. diff --git a/src/main/network/macos-system-resolver-health.test.ts b/src/main/network/macos-system-resolver-health.test.ts index 7cd29d6e22b..804847e6bbf 100644 --- a/src/main/network/macos-system-resolver-health.test.ts +++ b/src/main/network/macos-system-resolver-health.test.ts @@ -132,6 +132,23 @@ resolver #1 expect(child.listenerCount('close')).toBe(0) }) + it('kills scutil and removes listeners when its owner stops', async () => { + mockPlatform('darwin') + const child = createMockScutilProcess() + vi.mocked(spawn).mockReturnValue(child) + const abortController = new AbortController() + + const healthPromise = readCurrentProcessMacSystemResolverHealth(abortController.signal) + abortController.abort() + + await expect(healthPromise).resolves.toBe('unknown') + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + expect(child.stdout.listenerCount('data')).toBe(0) + expect(child.stderr.listenerCount('data')).toBe(0) + expect(child.listenerCount('error')).toBe(0) + expect(child.listenerCount('close')).toBe(0) + }) + it('removes scutil listeners when the child closes normally', async () => { mockPlatform('darwin') const child = createMockScutilProcess() diff --git a/src/main/network/macos-system-resolver-health.ts b/src/main/network/macos-system-resolver-health.ts index 3545db1963c..9a3dd6f1eb6 100644 --- a/src/main/network/macos-system-resolver-health.ts +++ b/src/main/network/macos-system-resolver-health.ts @@ -16,8 +16,10 @@ export function classifyMacSystemResolverHealth(scutilOutput: string): SystemRes return 'unknown' } -export async function readCurrentProcessMacSystemResolverHealth(): Promise { - if (process.platform !== 'darwin') { +export async function readCurrentProcessMacSystemResolverHealth( + signal?: AbortSignal +): Promise { + if (process.platform !== 'darwin' || signal?.aborted) { return 'unknown' } @@ -35,6 +37,10 @@ export async function readCurrentProcessMacSystemResolverHealth(): Promise { stderr += chunk } + const onAbort = (): void => { + child.kill('SIGKILL') + finish() + } const finish = (): void => { if (settled) { return @@ -48,6 +54,7 @@ export async function readCurrentProcessMacSystemResolverHealth(): Promise { @@ -62,5 +69,6 @@ export async function readCurrentProcessMacSystemResolverHealth(): Promise ({ + execFileMock: vi.fn(), + existsSyncMock: vi.fn(), + stdinEndMock: vi.fn() +})) + +vi.mock('node:child_process', () => ({ execFile: execFileMock })) +vi.mock('node:fs', () => ({ existsSync: existsSyncMock })) + +import { runMacosLoginSessionPtyProbe } from './macos-login-session-pty-probe' + +type ExecFileCallback = (error: Error | null, stdout: string, stderr: string) => void + +describe('runMacosLoginSessionPtyProbe', () => { + beforeEach(() => { + existsSyncMock.mockReturnValue(true) + execFileMock.mockReset() + stdinEndMock.mockReset() + }) + + it('runs login under expect-owned PTY and requires its marker plus a clean exit', async () => { + const abortController = new AbortController() + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(null, '^D\b\bORCA_LOGIN_PREFLIGHT_OK', '') + return { stdin: { end: stdinEndMock } } + } + ) + + await expect( + runMacosLoginSessionPtyProbe('ada', '/Users/ada', 4_000, 1_024, abortController.signal) + ).resolves.toEqual({ ok: true, conclusive: true, reason: 'accepted' }) + expect(execFileMock).toHaveBeenCalledWith( + '/usr/bin/expect', + [ + '-c', + expect.stringContaining( + 'spawn -noecho /usr/bin/login -flpq $env(ORCA_LOGIN_PROBE_USERNAME)' + ) + ], + expect.objectContaining({ + cwd: '/Users/ada', + env: expect.objectContaining({ ORCA_LOGIN_PROBE_USERNAME: 'ada' }), + killSignal: 'SIGKILL', + maxBuffer: 1_024, + signal: abortController.signal, + timeout: 4_000 + }), + expect.any(Function) + ) + expect(stdinEndMock).toHaveBeenCalledOnce() + expect(execFileMock.mock.calls[0]?.[1]?.[1]).toContain('send "\\004"; expect eof') + }) + + it('treats a natural exit without the marker as a conclusive rejection', async () => { + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(null, 'Login incorrect\r\nlogin: ', '') + return { stdin: { end: stdinEndMock } } + } + ) + + await expect(runMacosLoginSessionPtyProbe('ada', '/Users/ada', 4_000, 1_024)).resolves.toEqual({ + ok: false, + conclusive: true, + reason: 'rejected' + }) + }) + + it('keeps a timeout or output overflow inconclusive', async () => { + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(Object.assign(new Error('killed'), { killed: true }), '', '') + return { stdin: { end: stdinEndMock } } + } + ) + await expect(runMacosLoginSessionPtyProbe('ada', '/Users/ada', 4_000, 1_024)).resolves.toEqual({ + ok: false, + conclusive: false, + reason: 'timeout' + }) + + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback( + Object.assign(new Error('stdout maxBuffer length exceeded'), { + code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER' + }), + '', + '' + ) + return { stdin: { end: stdinEndMock } } + } + ) + await expect(runMacosLoginSessionPtyProbe('ada', '/Users/ada', 4_000, 1_024)).resolves.toEqual({ + ok: false, + conclusive: false, + reason: 'error' + }) + }) + + it('does not mistake an expect wrapper failure for a PAM rejection', async () => { + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(Object.assign(new Error('expect Tcl error'), { code: 1 }), '', '') + return { stdin: { end: stdinEndMock } } + } + ) + + await expect(runMacosLoginSessionPtyProbe('ada', '/Users/ada', 4_000, 1_024)).resolves.toEqual({ + ok: false, + conclusive: false, + reason: 'error' + }) + }) + + it('fails safe without spawning when expect is unavailable', async () => { + existsSyncMock.mockReturnValue(false) + + await expect(runMacosLoginSessionPtyProbe('ada', '/Users/ada', 4_000, 1_024)).resolves.toEqual({ + ok: false, + conclusive: false, + reason: 'error' + }) + expect(execFileMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/providers/macos-login-session-pty-probe.ts b/src/main/providers/macos-login-session-pty-probe.ts new file mode 100644 index 00000000000..ffcc2098482 --- /dev/null +++ b/src/main/providers/macos-login-session-pty-probe.ts @@ -0,0 +1,78 @@ +import { execFile, type ExecFileException } from 'node:child_process' +import { existsSync } from 'node:fs' + +const MACOS_EXPECT_PATH = '/usr/bin/expect' +const LOGIN_PREFLIGHT_MARKER = 'ORCA_LOGIN_PREFLIGHT_OK' +const LOGIN_PROBE_USERNAME_ENV = 'ORCA_LOGIN_PROBE_USERNAME' +// Why: expect owns the PTY without adding a long-lived native handle to the daemon. +const EXPECT_LOGIN_PROBE_SCRIPT = + 'log_user 1; ' + + 'spawn -noecho /usr/bin/login -flpq $env(ORCA_LOGIN_PROBE_USERNAME) /usr/bin/printf ORCA_LOGIN_PREFLIGHT_OK; ' + + 'send "\\004"; expect eof; wait; exit 0' + +export type LoginPreflightOutcome = { + ok: boolean + conclusive: boolean + reason: 'accepted' | 'rejected' | 'timeout' | 'error' +} + +export function classifyLoginPreflightError(error: ExecFileException): LoginPreflightOutcome { + // Why: a probe killed by our bound proves nothing about PAM and must not stick. + if (error.killed || error.code === 'ETIMEDOUT') { + return { ok: false, conclusive: false, reason: 'timeout' } + } + // Why: a natural nonzero exit is login(1)'s conclusive rejection verdict. + if (typeof error.code === 'number') { + return { ok: false, conclusive: true, reason: 'rejected' } + } + return { ok: false, conclusive: false, reason: 'error' } +} + +/** Runs the login-session oracle under a real PTY when the pipe probe cannot decide. */ +export function runMacosLoginSessionPtyProbe( + username: string, + accountHome: string, + timeoutMs: number, + maxOutputBytes: number, + signal?: AbortSignal +): Promise { + if (!existsSync(MACOS_EXPECT_PATH)) { + return Promise.resolve({ ok: false, conclusive: false, reason: 'error' }) + } + return new Promise((resolve) => { + try { + const child = execFile( + MACOS_EXPECT_PATH, + ['-c', EXPECT_LOGIN_PROBE_SCRIPT], + { + cwd: accountHome, + encoding: 'utf8', + env: { ...process.env, [LOGIN_PROBE_USERNAME_ENV]: username }, + killSignal: 'SIGKILL', + maxBuffer: maxOutputBytes, + signal, + timeout: timeoutMs + }, + (error, stdout) => { + if (error !== null) { + // Why: a nonzero expect exit can be its own Tcl/PTY failure, not PAM authority. + resolve( + error.killed || error.code === 'ETIMEDOUT' + ? { ok: false, conclusive: false, reason: 'timeout' } + : { ok: false, conclusive: false, reason: 'error' } + ) + return + } + resolve( + stdout.includes(LOGIN_PREFLIGHT_MARKER) + ? { ok: true, conclusive: true, reason: 'accepted' } + : { ok: false, conclusive: true, reason: 'rejected' } + ) + } + ) + child.stdin?.end() + } catch { + resolve({ ok: false, conclusive: false, reason: 'error' }) + } + }) +} diff --git a/src/main/providers/macos-tcc-login-shell.test.ts b/src/main/providers/macos-tcc-login-shell.test.ts index 565d40da9dd..6f1a66a90d3 100644 --- a/src/main/providers/macos-tcc-login-shell.test.ts +++ b/src/main/providers/macos-tcc-login-shell.test.ts @@ -1,23 +1,34 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as LoginSessionPtyProbe from './macos-login-session-pty-probe' -const { existsSyncMock, userInfoMock, execFileMock, stdinEndMock } = vi.hoisted(() => ({ - existsSyncMock: vi.fn(), - userInfoMock: vi.fn(), - execFileMock: vi.fn(), - stdinEndMock: vi.fn() -})) +const { existsSyncMock, userInfoMock, execFileMock, stdinEndMock, ptyProbeMock } = vi.hoisted( + () => ({ + existsSyncMock: vi.fn(), + userInfoMock: vi.fn(), + execFileMock: vi.fn(), + stdinEndMock: vi.fn(), + ptyProbeMock: vi.fn() + }) +) vi.mock('node:fs', () => ({ existsSync: existsSyncMock })) vi.mock('node:os', () => ({ userInfo: userInfoMock })) vi.mock('node:child_process', () => ({ execFile: execFileMock })) +vi.mock('./macos-login-session-pty-probe', async (importOriginal) => ({ + ...(await importOriginal()), + runMacosLoginSessionPtyProbe: ptyProbeMock +})) import { prepareMacosTccLoginShell, + probeMacosLoginSessionAlive, resetMacosLoginShellPreflightForTests, wrapShellSpawnForMacosTccAttribution } from './macos-tcc-login-shell' type ExecFileCallback = (error: Error | null, stdout: string, stderr: string) => void +const ACCEPTED_OUTCOME = { ok: true, conclusive: true, reason: 'accepted' } as const +const REJECTED_OUTCOME = { ok: false, conclusive: true, reason: 'rejected' } as const describe('wrapShellSpawnForMacosTccAttribution', () => { let origPlatform: PropertyDescriptor | undefined @@ -39,6 +50,7 @@ describe('wrapShellSpawnForMacosTccAttribution', () => { return { stdin: { end: stdinEndMock } } } ) + ptyProbeMock.mockResolvedValue({ ok: true, conclusive: true, reason: 'accepted' }) resetMacosLoginShellPreflightForTests() }) @@ -340,3 +352,200 @@ describe('wrapShellSpawnForMacosTccAttribution', () => { expect(execFileMock).not.toHaveBeenCalled() }) }) + +describe('probeMacosLoginSessionAlive', () => { + let origPlatform: PropertyDescriptor | undefined + let origDisable: string | undefined + + function setPlatform(value: string): void { + Object.defineProperty(process, 'platform', { configurable: true, value }) + } + + beforeEach(() => { + origPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + origDisable = process.env.ORCA_DISABLE_MACOS_LOGIN_SHELL + delete process.env.ORCA_DISABLE_MACOS_LOGIN_SHELL + existsSyncMock.mockReturnValue(true) + userInfoMock.mockReturnValue({ username: 'ada', homedir: '/Users/ada' }) + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(null, 'ORCA_LOGIN_PREFLIGHT_OK', '') + return { stdin: { end: stdinEndMock } } + } + ) + ptyProbeMock.mockResolvedValue({ ok: true, conclusive: true, reason: 'accepted' }) + resetMacosLoginShellPreflightForTests() + }) + + afterEach(() => { + if (origPlatform) { + Object.defineProperty(process, 'platform', origPlatform) + } + if (origDisable === undefined) { + delete process.env.ORCA_DISABLE_MACOS_LOGIN_SHELL + } else { + process.env.ORCA_DISABLE_MACOS_LOGIN_SHELL = origDisable + } + vi.restoreAllMocks() + vi.clearAllMocks() + }) + + it('re-probes even after a cached acceptance', async () => { + setPlatform('darwin') + await prepareMacosTccLoginShell() + expect(execFileMock).toHaveBeenCalledTimes(1) + const outcome = await probeMacosLoginSessionAlive() + expect(outcome).toEqual({ ok: true, conclusive: true, reason: 'accepted' }) + expect(execFileMock).toHaveBeenCalledTimes(2) + }) + + it('reuses an in-flight startup warmup instead of spawning a duplicate probe', async () => { + setPlatform('darwin') + let finishPreflight!: ExecFileCallback + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + finishPreflight = callback + return { stdin: { end: stdinEndMock } } + } + ) + + const warmup = prepareMacosTccLoginShell() + const freshProbe = probeMacosLoginSessionAlive() + expect(execFileMock).toHaveBeenCalledOnce() + + finishPreflight(null, 'ORCA_LOGIN_PREFLIGHT_OK', '') + await expect(Promise.all([warmup, freshProbe])).resolves.toEqual([ + ACCEPTED_OUTCOME, + ACCEPTED_OUTCOME + ]) + expect(execFileMock).toHaveBeenCalledOnce() + }) + + it('does not let a spawn-path probe overwrite a newer death verdict', async () => { + setPlatform('darwin') + const callbacks: ExecFileCallback[] = [] + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callbacks.push(callback) + return { stdin: { end: stdinEndMock } } + } + ) + + const freshProbe = probeMacosLoginSessionAlive() + const spawnProbe = prepareMacosTccLoginShell() + expect(execFileMock).toHaveBeenCalledTimes(2) + + callbacks[0](Object.assign(new Error('login incorrect'), { code: 1 }), '', '') + await expect(freshProbe).resolves.toEqual(REJECTED_OUTCOME) + callbacks[1](null, 'ORCA_LOGIN_PREFLIGHT_OK', '') + await expect(spawnProbe).resolves.toEqual(ACCEPTED_OUTCOME) + + expect(wrapShellSpawnForMacosTccAttribution('/bin/zsh', ['-l']).file).toBe('/bin/zsh') + }) + + it('flips the spawn wrapper off when a fresh probe conclusively rejects (dead login session)', async () => { + setPlatform('darwin') + await prepareMacosTccLoginShell() + expect(wrapShellSpawnForMacosTccAttribution('/bin/zsh', ['-l']).file).toBe('/usr/bin/login') + + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(Object.assign(new Error('login incorrect'), { code: 1 }), '', '') + return { stdin: { end: stdinEndMock } } + } + ) + const outcome = await probeMacosLoginSessionAlive() + expect(outcome).toEqual({ ok: false, conclusive: true, reason: 'rejected' }) + // The dead-session daemon must stop minting login(1) prompt zombies (#7936). + expect(wrapShellSpawnForMacosTccAttribution('/bin/zsh', ['-l']).file).toBe('/bin/zsh') + }) + + it('does not overwrite the cached verdict on an inconclusive probe', async () => { + setPlatform('darwin') + await prepareMacosTccLoginShell() + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(Object.assign(new Error('killed'), { killed: true }), '', '') + return { stdin: { end: stdinEndMock } } + } + ) + ptyProbeMock.mockResolvedValue({ ok: false, conclusive: false, reason: 'timeout' }) + const outcome = await probeMacosLoginSessionAlive() + expect(outcome).toEqual({ ok: false, conclusive: false, reason: 'timeout' }) + expect(wrapShellSpawnForMacosTccAttribution('/bin/zsh', ['-l']).file).toBe('/usr/bin/login') + }) + + it('returns null off macOS and when disabled', async () => { + setPlatform('linux') + expect(await probeMacosLoginSessionAlive()).toBeNull() + setPlatform('darwin') + process.env.ORCA_DISABLE_MACOS_LOGIN_SHELL = '1' + expect(await probeMacosLoginSessionAlive()).toBeNull() + expect(execFileMock).not.toHaveBeenCalled() + }) + + it('escalates an inconclusive pipe probe to a PTY probe and accepts its verdict', async () => { + setPlatform('darwin') + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(Object.assign(new Error('killed'), { killed: true }), '', '') + return { stdin: { end: stdinEndMock } } + } + ) + const outcome = await probeMacosLoginSessionAlive() + expect(outcome).toEqual({ ok: true, conclusive: true, reason: 'accepted' }) + expect(execFileMock).toHaveBeenCalledOnce() + expect(ptyProbeMock).toHaveBeenCalledWith('ada', '/Users/ada', 4_000, 1_024, undefined) + expect(wrapShellSpawnForMacosTccAttribution('/bin/zsh', ['-l']).file).toBe('/usr/bin/login') + }) + + it('treats a PTY-probe rejection as conclusive and flips the wrapper off', async () => { + setPlatform('darwin') + await prepareMacosTccLoginShell() + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(Object.assign(new Error('killed'), { killed: true }), '', '') + return { stdin: { end: stdinEndMock } } + } + ) + ptyProbeMock.mockResolvedValue({ ok: false, conclusive: true, reason: 'rejected' }) + const outcome = await probeMacosLoginSessionAlive() + expect(outcome).toEqual({ ok: false, conclusive: true, reason: 'rejected' }) + expect(wrapShellSpawnForMacosTccAttribution('/bin/zsh', ['-l']).file).toBe('/bin/zsh') + }) + + it('stays inconclusive when both pipe and PTY probes time out', async () => { + setPlatform('darwin') + await prepareMacosTccLoginShell() + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(Object.assign(new Error('killed'), { killed: true }), '', '') + return { stdin: { end: stdinEndMock } } + } + ) + ptyProbeMock.mockResolvedValue({ ok: false, conclusive: false, reason: 'timeout' }) + const outcome = await probeMacosLoginSessionAlive() + expect(outcome).toEqual({ ok: false, conclusive: false, reason: 'timeout' }) + // Inconclusive must not disturb the cached acceptance. + expect(wrapShellSpawnForMacosTccAttribution('/bin/zsh', ['-l']).file).toBe('/usr/bin/login') + }) + + it('does not start a PTY fallback after the watch cancels its pipe probe', async () => { + setPlatform('darwin') + const abortController = new AbortController() + abortController.abort() + execFileMock.mockImplementation( + (_file: string, _args: string[], _options: unknown, callback: ExecFileCallback) => { + callback(Object.assign(new Error('aborted'), { code: 'ABORT_ERR' }), '', '') + return { stdin: { end: stdinEndMock } } + } + ) + + await expect(probeMacosLoginSessionAlive(abortController.signal)).resolves.toEqual({ + ok: false, + conclusive: false, + reason: 'error' + }) + expect(ptyProbeMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/providers/macos-tcc-login-shell.ts b/src/main/providers/macos-tcc-login-shell.ts index bd92cacaac5..9ee3a709314 100644 --- a/src/main/providers/macos-tcc-login-shell.ts +++ b/src/main/providers/macos-tcc-login-shell.ts @@ -1,11 +1,21 @@ -import { execFile, type ExecFileException } from 'node:child_process' +import { execFile } from 'node:child_process' import { existsSync } from 'node:fs' import { userInfo } from 'node:os' +import { + classifyLoginPreflightError, + runMacosLoginSessionPtyProbe, + type LoginPreflightOutcome +} from './macos-login-session-pty-probe' + +export type { LoginPreflightOutcome } from './macos-login-session-pty-probe' const MACOS_LOGIN_PATH = '/usr/bin/login' const MACOS_ENV_PATH = '/usr/bin/env' const MACOS_PRINTF_PATH = '/usr/bin/printf' const LOGIN_PREFLIGHT_TIMEOUT_MS = 500 +// Why: the death-watch probe runs off the spawn path, so it can afford a bound +// that outlasts a PAM stack answering slowly rather than misreading it as a hang. +const LOGIN_SESSION_WATCH_PROBE_TIMEOUT_MS = 4_000 const LOGIN_PREFLIGHT_MARKER = 'ORCA_LOGIN_PREFLIGHT_OK' const LOGIN_PREFLIGHT_MAX_BUFFER_BYTES = 1024 const LOGIN_PREFLIGHT_RETRY_BASE_MS = 5_000 @@ -23,15 +33,11 @@ const DISABLE_ENV_VAR = 'ORCA_DISABLE_MACOS_LOGIN_SHELL' * reject) that may be cached; an inconclusive probe (our own timeout/SIGKILL, * maxBuffer, or spawn error) proves nothing about PAM and must not stick. */ -export type LoginPreflightOutcome = { - ok: boolean - conclusive: boolean - reason: 'accepted' | 'rejected' | 'timeout' | 'error' -} - let cachedLoginPreflightResult: boolean | null = null let loginPreflightInFlight: Promise | null = null let transientLoginPreflightFailure: { failureCount: number; retryAtMs: number } | null = null +let loginPreflightCacheEpoch = 0 +let loginSessionProbeInFlight = false function isDisabledByEnv(): boolean { const value = process.env[DISABLE_ENV_VAR] @@ -45,25 +51,15 @@ function loginPreflightRetryDelayMs(failureCount: number): number { ) } -function classifyPreflightError(error: ExecFileException): LoginPreflightOutcome { - // Why: our SIGKILL timeout cap (and maxBuffer, which also kills) is an - // environmental slow-path, not a PAM verdict — retry, don't cache (F1). - if (error.killed || error.code === 'ETIMEDOUT') { - return { ok: false, conclusive: false, reason: 'timeout' } - } - // A numeric exit code means login(1) ran to completion and rejected the user - // (it exits immediately on EOF-driven rejection); that verdict is cacheable. - if (typeof error.code === 'number') { - return { ok: false, conclusive: true, reason: 'rejected' } - } - // Spawn/EOF/other failure: inconclusive, fail open for this spawn but retry. - return { ok: false, conclusive: false, reason: 'error' } -} - // Fidelity limit: the probe runs over pipes while production shells run under a // real PTY, so a tty-sensitive PAM stack could diverge. It fails safe — a probe // pass with a prod failure only degrades to today's direct spawn (no wrapper). -function runLoginPreflight(username: string, accountHome: string): Promise { +function runLoginPreflight( + username: string, + accountHome: string, + timeoutMs = LOGIN_PREFLIGHT_TIMEOUT_MS, + signal?: AbortSignal +): Promise { return new Promise((resolve) => { try { const child = execFile( @@ -78,7 +74,8 @@ function runLoginPreflight(username: string, accountHome: string): Promise { if (error === null) { @@ -91,7 +88,7 @@ function runLoginPreflight(username: string, accountHome: string): Promise { // Why: cache only a conclusive PAM verdict; a killed/timed-out probe is // environmental and must be retried next spawn, not stuck forever (F1). - if (outcome.conclusive) { + const mayUpdateCache = !loginSessionProbeInFlight && cacheEpoch === loginPreflightCacheEpoch + if (outcome.conclusive && mayUpdateCache) { cachedLoginPreflightResult = outcome.ok transientLoginPreflightFailure = null - } else { + } else if (!outcome.conclusive && mayUpdateCache) { const failureCount = (transientLoginPreflightFailure?.failureCount ?? 0) + 1 transientLoginPreflightFailure = { failureCount, @@ -192,6 +191,64 @@ export function resetMacosLoginShellPreflightForTests(): void { cachedLoginPreflightResult = null loginPreflightInFlight = null transientLoginPreflightFailure = null + loginPreflightCacheEpoch = 0 + loginSessionProbeInFlight = false +} + +/** + * Fresh PAM probe for login-session death detection (#7936): bypasses the + * cached verdict and the transient backoff, and writes any conclusive verdict + * back into the cache — so a daemon whose login session died stops wrapping + * spawns in `login(1)` (which would only mint "Login incorrect" zombies) even + * before retirement completes. Escalates to a PTY-hosted probe when the pipe + * probe is inconclusive, since a dead session's PAM stack may only misbehave + * under a real tty. Returns null when the wrapper doesn't apply. + */ +export async function probeMacosLoginSessionAlive( + signal?: AbortSignal +): Promise { + if (process.platform !== 'darwin' || isDisabledByEnv() || !existsSync(MACOS_LOGIN_PATH)) { + return null + } + let username: string + let accountHome: string + try { + const account = userInfo() + username = account.username + accountHome = account.homedir + } catch { + return null + } + if (!username || !accountHome) { + return null + } + // Why: reuse the startup warmup when present, and fence older spawn-path results from restoring a stale verdict. + const existingPreflight = loginPreflightInFlight + loginSessionProbeInFlight = true + loginPreflightCacheEpoch++ + let outcome: LoginPreflightOutcome + try { + outcome = await (existingPreflight ?? + runLoginPreflight(username, accountHome, LOGIN_SESSION_WATCH_PROBE_TIMEOUT_MS, signal)) + if (!outcome.conclusive && !signal?.aborted) { + outcome = await runMacosLoginSessionPtyProbe( + username, + accountHome, + LOGIN_SESSION_WATCH_PROBE_TIMEOUT_MS, + LOGIN_PREFLIGHT_MAX_BUFFER_BYTES, + signal + ) + } + } finally { + // Why: invalidate spawn probes started during this fresh check before they can overwrite its newer verdict. + loginPreflightCacheEpoch++ + loginSessionProbeInFlight = false + } + if (outcome.conclusive) { + cachedLoginPreflightResult = outcome.ok + transientLoginPreflightFailure = null + } + return outcome } /** diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index f58d0610f57..3a5099edde2 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -56,6 +56,7 @@ import { OrcaRuntimeService, recentTerminalPathCandidatesIncludePath, recentTerminalOutputIncludesPath, + resolveWorktreeScanCacheTtlMs, type RuntimeTerminalAgentStatusEvent } from './orca-runtime' import { RecentPtyOutputBuffer } from './recent-pty-output-buffer' @@ -34979,3 +34980,83 @@ describe('OrcaRuntimeService', () => { }) }) }) + +describe('resolveWorktreeScanCacheTtlMs', () => { + const BASE_TTL_MS = 30_000 + const SCRATCH_TTL_MS = 5 * 60_000 + + it('keeps the base TTL for ordinary local repos', () => { + expect( + resolveWorktreeScanCacheTtlMs({ path: '/Users/dev/projects/app', connectionId: '' }) + ).toBe(BASE_TTL_MS) + }) + + it('extends the TTL for agent-scratch repo roots', () => { + expect( + resolveWorktreeScanCacheTtlMs({ + path: '/Users/dev/.codex-tmp/foragent-capsule-b1-repo-zP9Az6', + connectionId: '' + }) + ).toBe(SCRATCH_TTL_MS) + expect( + resolveWorktreeScanCacheTtlMs({ + path: '/Users/dev/.claude/skills/obsidian-second-brain', + connectionId: '' + }) + ).toBe(SCRATCH_TTL_MS) + }) + + it('never extends the TTL for SSH repos', () => { + // Why: scratch classification reads local path conventions; a remote path + // that merely looks similar must keep normal freshness. + expect( + resolveWorktreeScanCacheTtlMs({ + path: '/home/dev/.codex-tmp/capsule', + connectionId: 'ssh-1' + }) + ).toBe(BASE_TTL_MS) + }) + + it('keeps a scratch repo scan cached past the base TTL while normal repos rescan', async () => { + // Why: the whole fix lives in the cache-stamp call site; pin the wiring so + // a revert to the flat TTL fails CI, not just the pure-function tests. + vi.useFakeTimers() + // Why: the shared listWorktrees stub keeps call history across this file's + // tests; absolute counts need a clean baseline. + vi.mocked(listWorktrees).mockClear() + try { + const scratchPath = '/tmp/.codex-tmp/capsule-a' + const runtime = new OrcaRuntimeService({ + ...store, + getRepos: () => [ + { id: 'repo-1', path: '/tmp/repo', displayName: 'repo', badgeColor: 'blue', addedAt: 1 }, + { + id: 'repo-scratch', + path: scratchPath, + displayName: 'capsule', + badgeColor: 'blue', + addedAt: 1 + } + ] + } as never) + const internals = runtime as unknown as { listResolvedWorktrees: () => Promise } + const scanCallsFor = (path: string): number => + vi.mocked(listWorktrees).mock.calls.filter((call) => call[0] === path).length + + await internals.listResolvedWorktrees() + expect(scanCallsFor('/tmp/repo')).toBe(1) + expect(scanCallsFor(scratchPath)).toBe(1) + + vi.advanceTimersByTime(BASE_TTL_MS + 1_000) + await internals.listResolvedWorktrees() + expect(scanCallsFor('/tmp/repo')).toBe(2) + expect(scanCallsFor(scratchPath)).toBe(1) + + vi.advanceTimersByTime(SCRATCH_TTL_MS) + await internals.listResolvedWorktrees() + expect(scanCallsFor(scratchPath)).toBe(2) + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 5c25ce81792..bbeba293b58 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -314,6 +314,7 @@ import { } from '../../shared/worktree-ownership' import { createAgentScratchWorktreePathMatcher, + isAgentScratchRepoRootPath, type AgentScratchWorktreePathMatcher } from '../../shared/agent-scratch-worktrees' import { @@ -23853,7 +23854,7 @@ export class OrcaRuntimeService { generation, runtimeKey, result, - expiresAt: Date.now() + WORKTREE_SCAN_CACHE_TTL_MS + expiresAt: Date.now() + resolveWorktreeScanCacheTtlMs(repo) }) } return result @@ -29253,7 +29254,17 @@ const DEFAULT_WORKTREE_PS_LIMIT = 200 const DISCONNECTED_PTY_RECORD_MAX = 128 const RESOLVED_WORKTREE_CACHE_TTL_MS = 1000 const WORKTREE_SCAN_CACHE_TTL_MS = 30_000 +// Why: agent-scratch repos don't need 30s freshness — the steady-state scan +// fan-out was measured at ~128 git execs/min on real installs, mostly against +// these (crash-cluster diagnostics, 2026-07). +const WORKTREE_SCAN_AGENT_SCRATCH_TTL_MS = 5 * 60_000 const RESOLVED_WORKTREE_REPO_TIMEOUT_MS = 5000 + +export function resolveWorktreeScanCacheTtlMs(repo: Pick): number { + return !repo.connectionId && isAgentScratchRepoRootPath(repo.path) + ? WORKTREE_SCAN_AGENT_SCRATCH_TTL_MS + : WORKTREE_SCAN_CACHE_TTL_MS +} const PTY_CONTROLLER_LIST_TIMEOUT_MS = 3000 // Why: the renderer waits 15s; leave room for the verified failure response and release the spawn fence before its caller times out. const WORKTREE_TERMINAL_SLEEP_TIMEOUT_MS = 12_000 diff --git a/src/shared/agent-scratch-worktrees.test.ts b/src/shared/agent-scratch-worktrees.test.ts index b2714bd6464..839833c004e 100644 --- a/src/shared/agent-scratch-worktrees.test.ts +++ b/src/shared/agent-scratch-worktrees.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { createAgentScratchWorktreePathMatcher, + isAgentScratchRepoRootPath, isAgentScratchWorktreePath } from './agent-scratch-worktrees' @@ -101,3 +102,44 @@ describe('isAgentScratchWorktreePath', () => { expect(isAgentScratchWorktreePath('/Users/dev/app', '/orca/workspaces/app/feature')).toBe(false) }) }) + +describe('isAgentScratchRepoRootPath', () => { + it('matches codex scratch capsule repos', () => { + expect( + isAgentScratchRepoRootPath('/Users/dev/.codex-tmp/foragent-capsule-b1-repo-zP9Az6') + ).toBe(true) + expect(isAgentScratchRepoRootPath('/Users/dev/.codex-tmp/rc-fwd-qEXuEq')).toBe(true) + }) + + it('matches codex vendor imports and claude skills containers', () => { + expect(isAgentScratchRepoRootPath('/Users/dev/.codex/vendor_imports/skills')).toBe(true) + expect(isAgentScratchRepoRootPath('/Users/dev/.claude/skills/obsidian-second-brain')).toBe(true) + }) + + it('matches a repo registered at the scratch container itself', () => { + expect(isAgentScratchRepoRootPath('/Users/dev/.codex-tmp')).toBe(true) + expect(isAgentScratchRepoRootPath('/Users/dev/.codex/vendor_imports')).toBe(true) + }) + + it('matches scratch worktree containers used as repo roots', () => { + expect(isAgentScratchRepoRootPath('/Users/dev/app/.claude/worktrees/agent-a04ccaaa')).toBe(true) + expect(isAgentScratchRepoRootPath('/Users/dev/app/.gsd-workspaces/phase-1')).toBe(true) + }) + + it('matches Windows separators and casing', () => { + expect(isAgentScratchRepoRootPath('C:\\Users\\Dev\\.codex-tmp\\Capsule-X')).toBe(true) + expect(isAgentScratchRepoRootPath('C:\\Users\\Dev\\.Claude\\Skills\\foo')).toBe(true) + }) + + it('does not match ordinary user repos', () => { + expect(isAgentScratchRepoRootPath('/Users/dev/projects/app')).toBe(false) + expect(isAgentScratchRepoRootPath('/Users/dev/codex-tmp/app')).toBe(false) + expect(isAgentScratchRepoRootPath('/Users/dev/.codex/checkouts/app')).toBe(false) + expect(isAgentScratchRepoRootPath('/Users/dev/skills/.claude-app')).toBe(false) + }) + + it('does not match partial multi-segment markers', () => { + expect(isAgentScratchRepoRootPath('/Users/dev/.claude/config')).toBe(false) + expect(isAgentScratchRepoRootPath('/Users/dev/vendor_imports/app')).toBe(false) + }) +}) diff --git a/src/shared/agent-scratch-worktrees.ts b/src/shared/agent-scratch-worktrees.ts index 2ff216fbd6e..88be8646435 100644 --- a/src/shared/agent-scratch-worktrees.ts +++ b/src/shared/agent-scratch-worktrees.ts @@ -37,3 +37,27 @@ export function createAgentScratchWorktreePathMatcher( export function isAgentScratchWorktreePath(repoPath: string, worktreePath: string): boolean { return createAgentScratchWorktreePathMatcher([repoPath])(worktreePath) } + +/** Why: agent CLIs also mint whole scratch *repos* under these containers; a + * repo registered at such a root is agent-internal, not a user project (#9388). */ +const AGENT_SCRATCH_REPO_ROOT_SEGMENTS: readonly (readonly string[])[] = [ + ['.codex-tmp'], + ['.codex', 'vendor_imports'], + ['.claude', 'skills'], + ...AGENT_SCRATCH_PATH_PREFIXES +] + +export function isAgentScratchRepoRootPath(repoPath: string): boolean { + const segments = normalizeRuntimePathForComparison(repoPath).split('/') + for (const marker of AGENT_SCRATCH_REPO_ROOT_SEGMENTS) { + // Why: match the marker anywhere above the repo root (the repo lives at or + // under the scratch container), unlike worktree matching which anchors to a + // registered checkout path. + for (let index = 0; index + marker.length <= segments.length; index += 1) { + if (marker.every((segment, offset) => segments[index + offset] === segment)) { + return true + } + } + } + return false +}