From e17c98d4251ad136d69f29b489f8bee407ab62bb Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 31 Aug 2026 02:41:11 -0700 Subject: [PATCH] fix(daemon): bound the whole boot-recovery sequence with one budget (STA-5732) (#17427) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(daemon): bound the whole boot-recovery sequence with one budget (STA-5732) * fix(daemon): keep socket probes inside recovery budget * fix(daemon): size the recovery budget against the real post-kill tail The 24s budget reserved only 9s for everything after the deadline, leaving 27s of the startup PTY gate's fail-open cap unused — and every unused second is one where a daemon that would have drained gets killed with its live PTYs instead. Reserve each post-deadline stage's actual hard cap (kill 10.5s, fork 10s, lease 5s) and spend the rest: 24s -> 32s of adopt window. * fix(daemon): keep the last-resort endpoint rescue outside the recovery budget The rescue probe in the launcher's outer catch was clamped to the recovery budget's remainder, but it runs *after* that budget by construction — past prepareDaemonReplacement, killStaleDaemon, the fork and the adoption lease. The remainder is therefore essentially always negative, so Math.max(1, ...) handed a live socket a 1ms connect window. On the loaded machine this path exists for the probe loses to its own timer, the launcher rethrows, and a recoverable degraded adoption becomes total daemon loss for the whole run — the outcome the comment above it exists to prevent. Restore the 1s default and pin the window with a test that drives the launcher to that catch with the budget already spent. Also make the deliberate narrowing legible instead of implicit: - daemon-recovery-budget.ts: TRANSIENT_WEDGE_DRAIN_MS documented 20s as the grace #8697 sized, but #8697's merged second commit (840d3277d1d) widened it to 11 retries ~= 60s. Record that 20s is the drain estimate and that the budget deliberately sits under #8697's shipped grace. - daemon-init-wedged-daemon-grace.test.ts: pin the trade directly — a wedge draining after the budget is replaced and loses its live sessions. - Rewrite 'preserves a daemon that stays wedged until the LAST allowed grace retry' onto the simulated clock. It never mocked Date.now, so its 12 probes elapsed ~0ms and asserted a retry grace the wall clock can no longer deliver; it now pins the last drain the budget still adopts. * fix(daemon): name the socket probe default and correct the grace-retry rationale Answers the review round on the budget accounting: the outer-catch endpoint rescue is deliberately outside it, and the preflight clamp no longer duplicates probeDaemonSocket's default as a bare literal. --- .../daemon-init-child-startup-failure.test.ts | 4 + .../daemon-init-endpoint-adoption.test.ts | 123 +++++++++ src/main/daemon/daemon-init-fresh-import.ts | 1 + ...mon-init-live-session-preservation.test.ts | 21 +- ...mon-init-packaged-bundle-staleness.test.ts | 3 +- .../daemon-init-replacement-reporting.test.ts | 3 + src/main/daemon/daemon-init-test-harness.ts | 2 + .../daemon-init-wedged-daemon-grace.test.ts | 236 ++++++++++++++---- src/main/daemon/daemon-launch-paths.ts | 26 +- .../daemon/daemon-out-of-process-launcher.ts | 13 +- src/main/daemon/daemon-recovery-budget.ts | 56 +++++ .../daemon/daemon-replacement-preflight.ts | 57 ++++- 12 files changed, 480 insertions(+), 65 deletions(-) create mode 100644 src/main/daemon/daemon-recovery-budget.ts diff --git a/src/main/daemon/daemon-init-child-startup-failure.test.ts b/src/main/daemon/daemon-init-child-startup-failure.test.ts index 27d23a400d2..cab69025735 100644 --- a/src/main/daemon/daemon-init-child-startup-failure.test.ts +++ b/src/main/daemon/daemon-init-child-startup-failure.test.ts @@ -60,6 +60,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { function basicClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: vi.fn(async () => ({ sessions: [] })), disconnect: vi.fn() } @@ -144,6 +145,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ensureConnected: vi.fn(async () => { throw new Error('adoption unavailable') }), + ensureConnectedWithin: vi.fn(async () => { + throw new Error('adoption unavailable') + }), request: vi.fn(), disconnect } diff --git a/src/main/daemon/daemon-init-endpoint-adoption.test.ts b/src/main/daemon/daemon-init-endpoint-adoption.test.ts index 5503188d430..3400f192256 100644 --- a/src/main/daemon/daemon-init-endpoint-adoption.test.ts +++ b/src/main/daemon/daemon-init-endpoint-adoption.test.ts @@ -5,6 +5,7 @@ import { FAKE_RUNTIME_DIR, FAKE_DAEMON_ENTRY_PATH } from './daemon-init-test-harness' +import { DAEMON_RECOVERY_BUDGET_MS } from './daemon-recovery-budget' const { isPackagedMock, @@ -16,6 +17,8 @@ const { killStaleDaemonMock, replaceDaemonPidFileMock, daemonClientMock, + netConnectMock, + probeSocketExistsMock, spawnerInstances, trackDaemonReplacedMock, importFresh, @@ -220,6 +223,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementation(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: vi.fn(async () => ({ sessions: [{ sessionId: 's1', isAlive: true }] })), disconnect: vi.fn() } @@ -243,6 +247,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ensureConnected: vi.fn(async () => { events.push('full-pair') }), + ensureConnectedWithin: vi.fn(async () => { + events.push('full-pair') + }), request: vi.fn(), disconnect } @@ -279,6 +286,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockAdoptionClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), getDaemonIdentity: vi.fn(() => endpointIdentity), request: vi.fn(), disconnect: vi.fn() @@ -337,6 +345,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockAdoptionClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), getDaemonIdentity: vi.fn(() => endpointIdentity), request: vi.fn(), disconnect: vi.fn() @@ -385,6 +394,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockAdoptionClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), getDaemonIdentity: vi.fn(() => endpointIdentity), request: vi.fn(), disconnect: vi.fn() @@ -456,6 +466,113 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { } }) + it('rescues the endpoint owner with a full probe window after the recovery budget is spent', async () => { + // Why: this last-resort probe runs AFTER the budget — past the kill, the fork and the lease — + // so anything clamped to the remainder is a ~1ms probe that loses to its own timer against a + // live socket, and the rescue that saves every persistent session degrades into total loss. + const mod = await importFresh() + checkDaemonHealthMock.mockResolvedValue('unreachable') + await mod.initDaemonPtyProvider() + const clock = { now: 1_700_000_000_000 } + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => clock.now) + // The real post-deadline tail: kill, fork and lease all run after recovery has decided. + killStaleDaemonMock.mockImplementationOnce(async () => { + clock.now += DAEMON_RECOVERY_BUDGET_MS + 5_000 + return { killed: true, liveOwnerSurvived: false } + }) + function basicClient() { + return { + ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), + request: vi.fn(async () => ({ sessions: [] })), + disconnect: vi.fn() + } + } + daemonClientMock.mockImplementationOnce(basicClient) + daemonClientMock.mockImplementationOnce(basicClient) + // The fresh child lost the endpoint to another daemon: the lease rejects on identity. + daemonClientMock.mockImplementationOnce(function MockPostReadyClient() { + return { + ...basicClient(), + getDaemonIdentity: vi.fn(() => ({ + pid: 999, + startedAtMs: 900_000, + launchNonce: 'stale-launch' + })) + } + }) + const exitHandlers: ((code?: unknown) => void)[] = [] + const child = { + pid: 12345, + connected: true, + exitCode: null as number | null, + signalCode: null as NodeJS.Signals | null, + on(event: string, callback: (arg?: unknown) => void) { + if (event === 'exit') { + exitHandlers.push(callback) + } + if (event === 'message') { + queueMicrotask(() => callback({ type: 'ready', startedAtMs: 1_000_000 })) + } + return this + }, + once(event: string, callback: (arg?: unknown) => void) { + return child.on(event, callback) + }, + off() { + return child + }, + kill: vi.fn(() => true), + disconnect: vi.fn(() => { + child.connected = false + }), + unref: vi.fn() + } + forkMock.mockReturnValueOnce(child) + const kill = vi.spyOn(process, 'kill').mockImplementation(() => { + queueMicrotask(() => { + child.exitCode = 0 + for (const callback of exitHandlers.slice()) { + callback(0) + } + }) + return true + }) + probeSocketExistsMock.mockReturnValue(true) + // A loaded host answers late but well inside probeDaemonSocket's own 1s default. + netConnectMock.mockImplementation(() => ({ + on(event: string, callback: () => void) { + if (event === 'connect') { + setTimeout(callback, 500) + } + return this + }, + removeListener() { + return this + }, + destroy() {} + })) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const launcher = spawnerInstances[0].launcher as ( + socketPath: string, + tokenPath: string, + pidPath?: string, + launchNonce?: string + ) => Promise<{ mode?: string; releaseAdoptionLease?(): void }> + + try { + const handle = await launcher('/fake/socket', '/fake/token', '/fake/daemon.pid', 'launch-new') + + expect(handle.mode).toBe('degraded-new-pty-fallback') + handle.releaseAdoptionLease?.() + } finally { + warn.mockRestore() + kill.mockRestore() + nowSpy.mockRestore() + probeSocketExistsMock.mockReturnValue(false) + } + }) + it('disconnects every temporary client when healthy adoption fails', async () => { const mod = await importFresh() await mod.initDaemonPtyProvider() @@ -467,6 +584,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ensureConnected: vi.fn(async () => { throw new Error('initial adoption failed') }), + ensureConnectedWithin: vi.fn(async () => { + throw new Error('initial adoption failed') + }), request: vi.fn(), disconnect: initialDisconnect } @@ -476,6 +596,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ensureConnected: vi.fn(async () => { throw new Error('replacement adoption failed') }), + ensureConnectedWithin: vi.fn(async () => { + throw new Error('replacement adoption failed') + }), request: vi.fn(), disconnect: replacementDisconnect } diff --git a/src/main/daemon/daemon-init-fresh-import.ts b/src/main/daemon/daemon-init-fresh-import.ts index 1d4f35d655c..e1cc0416337 100644 --- a/src/main/daemon/daemon-init-fresh-import.ts +++ b/src/main/daemon/daemon-init-fresh-import.ts @@ -91,6 +91,7 @@ export async function importFreshDaemonInit(state: DaemonInitMockState) { daemonClientMock.mockImplementation(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), getDaemonIdentity: vi.fn(readLaunchedDaemonIdentity), request: vi.fn(async () => ({ sessions: [] })), disconnect: vi.fn() diff --git a/src/main/daemon/daemon-init-live-session-preservation.test.ts b/src/main/daemon/daemon-init-live-session-preservation.test.ts index 2a3eece9937..c3d36f59799 100644 --- a/src/main/daemon/daemon-init-live-session-preservation.test.ts +++ b/src/main/daemon/daemon-init-live-session-preservation.test.ts @@ -70,6 +70,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: requestMock, disconnect: disconnectMock } @@ -89,7 +90,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { '/fake/token', FAKE_DAEMON_ENTRY_PATH ) - expect(requestMock).toHaveBeenCalledWith('listSessions', undefined) + expect(requestMock).toHaveBeenCalledWith('listSessions', undefined, expect.any(Number)) expect(disconnectMock).toHaveBeenCalledOnce() expect(killStaleDaemonMock).not.toHaveBeenCalled() expect(forkMock).not.toHaveBeenCalled() @@ -110,6 +111,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: requestMock, disconnect: disconnectMock } @@ -123,7 +125,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { await launcher('/fake/socket', '/fake/token') - expect(requestMock).toHaveBeenCalledWith('listSessions', undefined) + expect(requestMock).toHaveBeenCalledWith('listSessions', undefined, expect.any(Number)) expect(disconnectMock).toHaveBeenCalledOnce() expect(killStaleDaemonMock).not.toHaveBeenCalled() expect(forkMock).not.toHaveBeenCalled() @@ -208,6 +210,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: requestMock, disconnect: disconnectMock } @@ -222,7 +225,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { await launcher('/fake/socket', '/fake/token') expect(getMacDaemonSystemResolverHealthMock).toHaveBeenCalledWith('/fake/socket', '/fake/token') - expect(requestMock).toHaveBeenCalledWith('listSessions', undefined) + expect(requestMock).toHaveBeenCalledWith('listSessions', undefined, expect.any(Number)) expect(disconnectMock).toHaveBeenCalledOnce() expect(getDaemonLaunchIdentityMock).not.toHaveBeenCalled() expect(killStaleDaemonMock).not.toHaveBeenCalled() @@ -246,6 +249,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: requestMock, disconnect: disconnectMock } @@ -259,7 +263,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { await launcher('/fake/socket', '/fake/token') - expect(requestMock).toHaveBeenCalledWith('listSessions', undefined) + expect(requestMock).toHaveBeenCalledWith('listSessions', undefined, expect.any(Number)) expect(disconnectMock).toHaveBeenCalledOnce() expect(killStaleDaemonMock).not.toHaveBeenCalled() expect(forkMock).not.toHaveBeenCalled() @@ -282,6 +286,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: requestMock, disconnect: disconnectMock } @@ -295,7 +300,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { await launcher('/fake/socket', '/fake/token') - expect(requestMock).toHaveBeenCalledWith('listSessions', undefined) + expect(requestMock).toHaveBeenCalledWith('listSessions', undefined, expect.any(Number)) expect(disconnectMock).toHaveBeenCalledOnce() expect(killStaleDaemonMock).not.toHaveBeenCalled() expect(forkMock).not.toHaveBeenCalled() @@ -317,6 +322,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: requestMock, disconnect: vi.fn() } @@ -333,7 +339,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { const handle = await launcher('/fake/socket', '/fake/token') - expect(requestMock).toHaveBeenCalledWith('listSessions', undefined) + expect(requestMock).toHaveBeenCalledWith('listSessions', undefined, expect.any(Number)) expect(handle.mode).toBe('degraded-new-pty-fallback') expect(killStaleDaemonMock).not.toHaveBeenCalled() expect(forkMock).not.toHaveBeenCalled() @@ -348,6 +354,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ensureConnected: vi.fn(async () => { throw new Error('daemon is wedged') }), + ensureConnectedWithin: vi.fn(async () => { + throw new Error('daemon is wedged') + }), request: vi.fn(), disconnect: vi.fn() } diff --git a/src/main/daemon/daemon-init-packaged-bundle-staleness.test.ts b/src/main/daemon/daemon-init-packaged-bundle-staleness.test.ts index 6683e627e6d..f080083d370 100644 --- a/src/main/daemon/daemon-init-packaged-bundle-staleness.test.ts +++ b/src/main/daemon/daemon-init-packaged-bundle-staleness.test.ts @@ -164,6 +164,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: requestMock, disconnect: disconnectMock } @@ -184,7 +185,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { '/fake/token', '1.2.3' ) - expect(requestMock).toHaveBeenCalledWith('listSessions', undefined) + expect(requestMock).toHaveBeenCalledWith('listSessions', undefined, expect.any(Number)) expect(disconnectMock).toHaveBeenCalledOnce() expect(killStaleDaemonMock).not.toHaveBeenCalled() expect(forkMock).not.toHaveBeenCalled() diff --git a/src/main/daemon/daemon-init-replacement-reporting.test.ts b/src/main/daemon/daemon-init-replacement-reporting.test.ts index 92bed823fb3..ba7bae2a71e 100644 --- a/src/main/daemon/daemon-init-replacement-reporting.test.ts +++ b/src/main/daemon/daemon-init-replacement-reporting.test.ts @@ -304,6 +304,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ensureConnected: vi.fn(async () => { throw new Error('connect ENOENT') }), + ensureConnectedWithin: vi.fn(async () => { + throw new Error('connect ENOENT') + }), request: vi.fn(), disconnect: vi.fn() } diff --git a/src/main/daemon/daemon-init-test-harness.ts b/src/main/daemon/daemon-init-test-harness.ts index 106e862237b..6c38720a40b 100644 --- a/src/main/daemon/daemon-init-test-harness.ts +++ b/src/main/daemon/daemon-init-test-harness.ts @@ -102,6 +102,7 @@ function createDaemonInitMockState(): DaemonInitMockState { const daemonClientMock = vi.fn().mockImplementation(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), getDaemonIdentity: vi.fn(readLaunchedDaemonIdentity), request: vi.fn(async () => ({ sessions: [] })), disconnect: vi.fn() @@ -212,6 +213,7 @@ export function createDaemonInitMocks(): DaemonInitMocks { state.daemonClientMock.mockImplementationOnce(function MockAdoptionClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: vi.fn(), disconnect: vi.fn() } diff --git a/src/main/daemon/daemon-init-wedged-daemon-grace.test.ts b/src/main/daemon/daemon-init-wedged-daemon-grace.test.ts index 18debd1342a..58d5fa2602e 100644 --- a/src/main/daemon/daemon-init-wedged-daemon-grace.test.ts +++ b/src/main/daemon/daemon-init-wedged-daemon-grace.test.ts @@ -1,6 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { WEDGED_DAEMON_GRACE_RETRIES } from './daemon-init' import { FAKE_RUNTIME_DIR } from './daemon-init-test-harness' +import { + DAEMON_RECOVERY_BUDGET_MS, + DAEMON_RECOVERY_PROBE_MS, + TRANSIENT_WEDGE_DRAIN_MS +} from './daemon-recovery-budget' +import { LOCAL_PTY_STARTUP_FAIL_OPEN_TIMEOUT_MS } from '../startup/first-window-startup-services' const { probeSocketExistsMock, @@ -64,6 +70,121 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { } } + // The real client's own defaults, which is what an unbudgeted probe pays. Mirrored here so the + // simulated clock below charges a wedge exactly what production would have spent on it. + const CLIENT_CONNECT_TIMEOUT_MS = 5_000 + const CLIENT_REQUEST_TIMEOUT_MS = 30_000 + const HEALTH_CHECK_TIMEOUT_MS = 3_000 + // What still has to fit inside the startup PTY gate once recovery decides, at each stage's own + // hard cap: killStaleDaemon's two identity inspections, SIGTERM wait, SIGKILL confirm and + // endpoint probe (daemon-stale-kill.ts, daemon-process-identity-query.ts, + // daemon-endpoint-probe.ts), then launchDaemonChild's readiness timeout + // (daemon-launched-child.ts), then the adoption lease and adapter connects — the last against a + // daemon that just reported ready, so an allowance rather than the client's unbudgeted 4x5s. + const POST_RECOVERY_KILL_MS = 3_000 + 3_000 + 3_000 + 1_000 + 500 + const POST_RECOVERY_FORK_MS = 10_000 + const POST_RECOVERY_LEASE_MS = 5_000 + const POST_RECOVERY_RELAUNCH_MS = + POST_RECOVERY_KILL_MS + POST_RECOVERY_FORK_MS + POST_RECOVERY_LEASE_MS + + /** + * Runs the launcher against a daemon that accepts connections and answers nothing until + * `drainsAfterMs` of simulated elapsed time, on a hand-driven clock: each mocked wait advances + * Date.now by exactly what the real call would have blocked for, or stops at the drain — the + * probe already in flight when the daemon comes back is the one that gets an answer. Returns + * how long the adopt-or-replace decision took. + */ + async function runWedgedRecovery( + wedgeAt: 'handshake' | 'listSessions', + drainsAfterMs = Number.POSITIVE_INFINITY + ): Promise { + const mod = await importFresh() + await mod.initDaemonPtyProvider() + + const startedAtMs = 1_700_000_000_000 + const drainsAtMs = startedAtMs + drainsAfterMs + const clock = { now: startedAtMs } + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => clock.now) + const wedge = { active: true, drained: false } + + const stall = async (timeoutMs: number, message: string): Promise => { + if (drainsAtMs > clock.now + timeoutMs) { + clock.now += timeoutMs + throw new Error(message) + } + clock.now = Math.max(clock.now, drainsAtMs) + wedge.active = false + wedge.drained = true + } + daemonClientMock.mockImplementation(function MockDaemonClient() { + const connect = async (timeoutMs = CLIENT_CONNECT_TIMEOUT_MS): Promise => { + if (wedge.active && wedgeAt === 'handshake') { + await stall(timeoutMs, 'Hello response timed out') + } + } + return { + ensureConnected: vi.fn(() => connect()), + ensureConnectedWithin: vi.fn(connect), + getDaemonIdentity: vi.fn(readLaunchedDaemonIdentity), + request: vi.fn(async (_type: string, _payload: unknown, timeoutMs?: number) => { + if (wedge.active && wedgeAt === 'listSessions') { + await stall(timeoutMs ?? CLIENT_REQUEST_TIMEOUT_MS, 'Request timed out') + } + // A daemon that drained still owns the sessions this grace exists to preserve. + return { sessions: wedge.drained ? [{ sessionId: 'wt-1@@live', isAlive: true }] : [] } + }), + disconnect: vi.fn() + } + }) + checkDaemonHealthMock.mockImplementationOnce(async () => { + clock.now += HEALTH_CHECK_TIMEOUT_MS + return 'unreachable' + }) + // Ending the wedge on the kill keeps the replacement daemon answering its adoption lease. + killStaleDaemonMock.mockImplementationOnce(async () => { + wedge.active = false + return { killed: true, liveOwnerSurvived: false } + }) + probeSocketExistsMock.mockReturnValue(true) + netConnectMock.mockImplementation(stubAliveSocketConnect) + forkMock.mockImplementationOnce(() => ({ + pid: 12345, + on(event: string, cb: (arg?: unknown) => void) { + if (event === 'message') { + queueMicrotask(() => cb({ type: 'ready', startedAtMs: 1_000_000 })) + } + return this + }, + off() { + return this + }, + disconnect: vi.fn(), + unref: vi.fn() + })) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const launcher = spawnerInstances[0].launcher as ( + socketPath: string, + tokenPath: string + ) => Promise<{ shutdown(): Promise }> + try { + await launcher('/fake/socket', '/fake/token') + } finally { + warnSpy.mockRestore() + nowSpy.mockRestore() + daemonClientMock.mockImplementation(function MockDaemonClient() { + return { + ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), + request: vi.fn(async () => ({ sessions: [] })), + disconnect: vi.fn() + } + }) + } + // Nothing after the adopt-or-replace decision advances the simulated clock, so this is it. + return clock.now - startedAtMs + } + it('adopts a transiently wedged daemon that drains and reports live sessions within the grace window', async () => { // Why: Windows update-relaunch — post-install load wedges the daemon briefly; it still owns live sessions, so grace-adopt not kill. const mod = await importFresh() @@ -75,6 +196,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ensureConnected: vi.fn(async () => { throw new Error('Hello response timed out') }), + ensureConnectedWithin: vi.fn(async () => { + throw new Error('Hello response timed out') + }), request: vi.fn(), disconnect: vi.fn() } @@ -82,6 +206,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { daemonClientMock.mockImplementationOnce(function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: vi.fn(async () => ({ sessions: [{ sessionId: 'wt-1@@live', isAlive: true }] })), @@ -111,6 +236,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { const answeringDefault = function MockDaemonClient() { return { ensureConnected: vi.fn(async () => {}), + ensureConnectedWithin: vi.fn(async () => {}), request: vi.fn(async () => ({ sessions: [] })), disconnect: vi.fn() } @@ -125,6 +251,11 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { throw new Error('Hello response timed out') } }), + ensureConnectedWithin: vi.fn(async () => { + if (daemonClientConstructionCount <= 2 + WEDGED_DAEMON_GRACE_RETRIES) { + throw new Error('Hello response timed out') + } + }), getDaemonIdentity: vi.fn(readLaunchedDaemonIdentity), request: vi.fn(), disconnect: vi.fn() @@ -183,56 +314,70 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { } }) - it('grace budget is generous enough to ride out a ~60s transient wedge', () => { - // Why: each probe waits the client's 5s hello timeout, so 1 + 11 probes ≈ 60s of drain grace; don't cut without telemetry. - expect(WEDGED_DAEMON_GRACE_RETRIES).toBeGreaterThanOrEqual(11) + it('bounds recovery inside the startup PTY gate without cutting into the drain grace', () => { + // Lower bound: #8697 bought adoption of a daemon that drains within ~20s *with* its live + // sessions; a budget that expires first turns that adoption back into a kill. + expect(DAEMON_RECOVERY_BUDGET_MS).toBeGreaterThan(TRANSIENT_WEDGE_DRAIN_MS) + // Upper bound: recovery is only the first phase inside the gate's fail-open cap — the kill + // and the relaunch that follow it run inside the same cap. + expect(DAEMON_RECOVERY_BUDGET_MS + POST_RECOVERY_RELAUNCH_MS).toBeLessThan( + LOCAL_PTY_STARTUP_FAIL_OPEN_TIMEOUT_MS + ) + // Why the probe outruns the health check: it is the second opinion on that check's verdict, + // so on the loaded machine it exists for, the same bar would just reproduce it. + expect(DAEMON_RECOVERY_PROBE_MS).toBeGreaterThan(HEALTH_CHECK_TIMEOUT_MS) + // Why: the count is a spin guard for instantly-failing probes only. If it could bind first, a + // wedge would again be graced for however long its probes happened to take. + expect(WEDGED_DAEMON_GRACE_RETRIES * DAEMON_RECOVERY_PROBE_MS).toBeGreaterThan( + DAEMON_RECOVERY_BUDGET_MS + ) }) - it('preserves a daemon that stays wedged until the LAST allowed grace retry', async () => { - // Why: daemon drains only on the last allowed probe (1 + WEDGED_DAEMON_GRACE_RETRIES) — must be preserved, not replaced. - const mod = await importFresh() - await mod.initDaemonPtyProvider() + it('preserves a daemon that drains on the last probe the recovery budget allows', async () => { + // Why rewritten onto the clock: this used to drain on probe 1 + WEDGED_DAEMON_GRACE_RETRIES + // with real Date.now, so its 12 probes elapsed ~0ms and the budget never bound — a grace + // production can no longer deliver, since each failing probe costs up to + // DAEMON_RECOVERY_PROBE_MS. The count's exhaustion side stays pinned by the #8689 test above. + const recoveryMs = await runWedgedRecovery('handshake', DAEMON_RECOVERY_BUDGET_MS - 1_000) - let probe = 0 - const answeringDefault = function MockDaemonClient() { - return { - ensureConnected: vi.fn(async () => {}), - request: vi.fn(async () => ({ sessions: [] })), - disconnect: vi.fn() - } - } - daemonClientMock.mockImplementation(function MockDaemonClient() { - probe += 1 - const drainsNow = probe >= 1 + WEDGED_DAEMON_GRACE_RETRIES - return { - ensureConnected: vi.fn(async () => { - if (!drainsNow) { - throw new Error('Hello response timed out') - } - }), - request: vi.fn(async () => ({ - sessions: drainsNow ? [{ sessionId: 'wt-1@@live', isAlive: true }] : [] - })), - disconnect: vi.fn() - } - }) + expect(killStaleDaemonMock).not.toHaveBeenCalled() + expect(forkMock).not.toHaveBeenCalled() + expect(recoveryMs).toBeLessThanOrEqual(DAEMON_RECOVERY_BUDGET_MS) + }) - const launcher = spawnerInstances[0].launcher as ( - socketPath: string, - tokenPath: string - ) => Promise<{ shutdown(): Promise }> - checkDaemonHealthMock.mockResolvedValueOnce('unreachable') - probeSocketExistsMock.mockReturnValue(true) - netConnectMock.mockImplementation(stubAliveSocketConnect) + it('ends the grace on the recovery budget when every handshake stalls', async () => { + // Why: unbudgeted, this cost the client's 5s connect default once per probe across 12 probes + // — ~68s with the health check, past the gate's 60s fail-open cap (STA-5732). + expect(await runWedgedRecovery('handshake')).toBeLessThanOrEqual(DAEMON_RECOVERY_BUDGET_MS) + expect(killStaleDaemonMock).toHaveBeenCalled() + }) - try { - await launcher('/fake/socket', '/fake/token') + it('ends the grace on the recovery budget when the daemon answers hello and then wedges', async () => { + // Why: this is the shape the ticket reported — listSessions fell back to the client's 30s + // request default, so 12 probes stalled startup for minutes. + expect(await runWedgedRecovery('listSessions')).toBeLessThanOrEqual(DAEMON_RECOVERY_BUDGET_MS) + expect(killStaleDaemonMock).toHaveBeenCalled() + }) - expect(killStaleDaemonMock).not.toHaveBeenCalled() - expect(forkMock).not.toHaveBeenCalled() - } finally { - daemonClientMock.mockImplementation(answeringDefault) - } + it('still adopts a wedge that drains at the far edge of the documented window (#8697)', async () => { + // Why: the wall clock now ends the grace, so the budget is the only thing keeping the + // Windows update-relaunch wedge adoptable. It comes back owning live sessions well after the + // probes start failing; recovery has to still be probing then instead of having killed it. + const recoveryMs = await runWedgedRecovery('handshake', TRANSIENT_WEDGE_DRAIN_MS) + + expect(killStaleDaemonMock).not.toHaveBeenCalled() + expect(forkMock).not.toHaveBeenCalled() + expect(recoveryMs).toBeGreaterThanOrEqual(TRANSIENT_WEDGE_DRAIN_MS) + }) + + it('replaces a wedge that drains after the recovery budget (accepted trade vs #8697)', async () => { + // Why pinned rather than left implicit: #8697's merged grace was 11 retries ~= 60s, chosen to + // keep live-session loss near zero. Bounding it at DAEMON_RECOVERY_BUDGET_MS is a deliberate + // narrowing — a Windows update-relaunch wedge that drains after the budget is now replaced and + // its live terminal/agent sessions are destroyed. Only the window size is tunable. + await runWedgedRecovery('handshake', DAEMON_RECOVERY_BUDGET_MS + 5_000) + + expect(killStaleDaemonMock).toHaveBeenCalled() }) it('replaces a hello-rejected daemon even though its pipe accepts connections', async () => { @@ -245,6 +390,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { ensureConnected: vi.fn(async () => { throw new Error('Hello rejected') }), + ensureConnectedWithin: vi.fn(async () => { + throw new Error('Hello rejected') + }), request: vi.fn(), disconnect: vi.fn() } diff --git a/src/main/daemon/daemon-launch-paths.ts b/src/main/daemon/daemon-launch-paths.ts index 9f01d2770e9..9b5b2c3dca8 100644 --- a/src/main/daemon/daemon-launch-paths.ts +++ b/src/main/daemon/daemon-launch-paths.ts @@ -4,6 +4,8 @@ import { join } from 'node:path' import { getAppEnvironment } from '../../shared/app-environment' import { getDaemonLogFilePath } from '../observability/logs-directory' import { DaemonClient } from './client' +import { daemonRecoveryProbeTimeoutMs } from './daemon-recovery-budget' +import { remainingDaemonRequestTimeoutMs } from './daemon-request-deadline' import { PROTOCOL_VERSION, type ListSessionsResult } from './types' export function getDaemonRuntimeDir(): string { @@ -44,8 +46,14 @@ export function daemonLogArgs(): string[] { return disabled === '1' || disabled === 'true' ? [] : ['--log-file', getDaemonLogFilePath()] } +/** Named so a caller clamping this probe to a deadline cannot silently decouple from its default. */ +export const DAEMON_SOCKET_PROBE_TIMEOUT_MS = 1_000 + // Why: a socket that accepts a connection proves a daemon survived a previous app session and can be reused. -export function probeDaemonSocket(socketPath: string): Promise { +export function probeDaemonSocket( + socketPath: string, + timeoutMs = DAEMON_SOCKET_PROBE_TIMEOUT_MS +): Promise { const { promise, resolve } = Promise.withResolvers() if (process.platform !== 'win32' && !existsSync(socketPath)) { resolve(false) @@ -69,21 +77,31 @@ export function probeDaemonSocket(socketPath: string): Promise { } const onConnect = (): void => finish(true, true) const onError = (): void => finish(false) - timer = setTimeout(() => finish(false, true), 1000) + timer = setTimeout(() => finish(false, true), timeoutMs) socket.on('connect', onConnect) socket.on('error', onError) return promise } +// Why recoveryDeadlineMs is required: this probe only ever runs on a startup path that has a +// budget, and the client's own defaults are far larger than any of them. export async function getAliveDaemonSessionCount( socketPath: string, tokenPath: string, + recoveryDeadlineMs: number, protocolVersion = PROTOCOL_VERSION ): Promise { const client = new DaemonClient({ socketPath, tokenPath, protocolVersion }) + // Why one slice for both: a wedged handshake must not leave the request its own fresh 30s. + const probeTimeoutMs = daemonRecoveryProbeTimeoutMs(recoveryDeadlineMs) + const probeDeadlineMs = Date.now() + probeTimeoutMs try { - await client.ensureConnected() - const result = await client.request('listSessions', undefined) + await client.ensureConnectedWithin(probeTimeoutMs) + const result = await client.request( + 'listSessions', + undefined, + remainingDaemonRequestTimeoutMs(probeDeadlineMs) + ) return result.sessions.filter((session) => session.isAlive).length } catch { return null diff --git a/src/main/daemon/daemon-out-of-process-launcher.ts b/src/main/daemon/daemon-out-of-process-launcher.ts index e5b14aa8cb0..b59535a249a 100644 --- a/src/main/daemon/daemon-out-of-process-launcher.ts +++ b/src/main/daemon/daemon-out-of-process-launcher.ts @@ -14,6 +14,7 @@ import { } from './daemon-launched-child' import { getDaemonEntryPath, probeDaemonSocket as probeSocket } from './daemon-launch-paths' import { materializeRelocatedDaemonHost } from './daemon-host-relocation' +import { DAEMON_RECOVERY_BUDGET_MS, daemonRecoveryProbeTimeoutMs } from './daemon-recovery-budget' import { cleanupDaemonForProtocol } from './daemon-protocol-cleanup' import { getDaemonPidPath, @@ -56,6 +57,9 @@ export function createOutOfProcessLauncher( ): DaemonLauncher { return async (socketPath, tokenPath, suppliedPidPath, suppliedLaunchNonce) => { const entryPath = getDaemonEntryPath() + // Why here: everything up to the fork is one recovery, so the adoption connect and the + // preflight's probes share a single absolute budget rather than each carrying its own. + const recoveryDeadlineMs = Date.now() + DAEMON_RECOVERY_BUDGET_MS const pidPath = suppliedPidPath ?? getDaemonPidPath(runtimeDir) const launchNonce = suppliedLaunchNonce ?? randomUUID() // One-shot: whichever launch consumes it owns the attribution, so a later unrelated launch can't @@ -69,7 +73,9 @@ export function createOutOfProcessLauncher( }) try { // Why: acquire the full pair before control-only probes so an expired inherited deadline can't fire in the probe-to-adoption gap. - await adoptionClient.ensureConnected() + // Why bounded: unbudgeted this grants a fresh 5s to each of four connect/hello steps, so a + // wedged endpoint burns more before recovery starts than recovery itself is allowed. + await adoptionClient.ensureConnectedWithin(daemonRecoveryProbeTimeoutMs(recoveryDeadlineMs)) await reconcileDaemonPidOwnership(adoptionClient, pidPath) } catch { adoptionClient.disconnect() @@ -99,6 +105,7 @@ export function createOutOfProcessLauncher( socketPath, tokenPath, entryPath, + recoveryDeadlineMs, attributedReason, releaseAdoptionClient, preserveDaemon @@ -193,6 +200,10 @@ export function createOutOfProcessLauncher( // giving up here costs the user every persistent session for the whole run. Something // answering the endpoint now is a daemon worth adopting, not a reason to fall back to // local PTYs. + // Why unbudgeted: the recovery deadline bounds the adopt-or-replace decision, and this runs + // after it — past the kill, the fork and the lease. Clamping to the remainder yields a 1ms + // probe that loses to its own timer against a live socket, turning the rescue into the total + // daemon loss it exists to prevent. if (await probeSocket(socketPath)) { console.warn( '[daemon] DEGRADED MODE: adopting the daemon that owns the endpoint after a replacement could not publish onto it. Existing sessions keep working; fresh terminals run on the local provider WITHOUT daemon persistence until you restart the daemon (Manage Sessions → Restart).' diff --git a/src/main/daemon/daemon-recovery-budget.ts b/src/main/daemon/daemon-recovery-budget.ts new file mode 100644 index 00000000000..ea6f6ebb76f --- /dev/null +++ b/src/main/daemon/daemon-recovery-budget.ts @@ -0,0 +1,56 @@ +/** + * How long a transient wedge takes to drain — the Windows update-relaunch AV/disk-pressure shape + * #8697 measured. The grace exists to adopt that daemon *with* its live sessions instead of + * killing them, so the recovery budget has to outlast this. + * + * Note this is the drain estimate, not the grace #8697 shipped: its merged second commit + * (840d3277d1d) widened WEDGED_DAEMON_GRACE_RETRIES 3 -> 11 (~20s -> ~60s) precisely to keep + * live-session loss near zero on that path. DAEMON_RECOVERY_BUDGET_MS below deliberately sits + * under that ~60s, because the unbounded version blew the startup PTY gate and lost the sessions + * anyway (STA-5732) — so a wedge draining between the budget and ~60s is now replaced where + * #8697 would have adopted it. That trade is pinned in daemon-init-wedged-daemon-grace.test.ts. + */ +export const TRANSIENT_WEDGE_DRAIN_MS = 20_000 + +/** + * What one connect (+ listSessions) attempt may spend. Deliberately more generous than the 3s + * daemon health check: this probe is the second opinion on that check's verdict, and a machine + * loaded enough to time the check out on a live daemon would time out a probe held to the same + * bar too — turning "could not verify" into "dead". Left to its own defaults the client instead + * grants a fresh 5s to each of four connect/hello steps plus 30s to the request, so one probe of + * a wedged daemon could outlast the entire recovery. + */ +export const DAEMON_RECOVERY_PROBE_MS = 8_000 + +/** + * One absolute wall-clock budget for adopting-or-replacing whatever daemon already owns the + * endpoint at startup. Every caller of the out-of-process launcher shares it — the desktop + * startup gate, orcad, and user-initiated restart — because a wedged endpoint costs the same + * wherever it is met. + * + * Both bounds matter and daemon-init-wedged-daemon-grace.test.ts pins them. Above + * TRANSIENT_WEDGE_DRAIN_MS, with room for the probe in flight when the daemon finally answers to + * finish its hello + listSessions rather than expire on the deadline. Below what the startup PTY + * gate can still absorb, since the kill, fork and lease that follow run inside the same fail-open + * cap: grace used to be a probe count with no clock at all, so it ran past that cap and the + * sessions were lost anyway, after the user watched the app hang (STA-5732). + * + * Sized against that cap rather than guessed, because every second not spent here is a second a + * daemon that would have drained gets killed instead. What still has to fit after the deadline, + * at each stage's own hard cap: killStaleDaemon 10.5s (two identity inspections at 3s + the 3s + * SIGTERM wait + the 1s SIGKILL confirm + the 0.5s endpoint probe), the fork's 10s readiness + * timeout, and 5s for the adoption lease and adapter connects — those two run against a daemon + * that has just reported ready over IPC, so they get a realistic allowance, not the client's + * unbudgeted 4x5s. 60 - 25.5 leaves 34.5s; take 32s and keep the rest as margin. + * + * Outside that accounting by design: the launcher's outer-catch endpoint rescue, which only + * runs once the replacement has already failed. Clamping it to the remainder is what turned a + * recoverable degraded adoption into total daemon loss, so it keeps its own probe default and + * the gate may fail open ahead of it — no worse than not rescuing, and better whenever it wins. + */ +export const DAEMON_RECOVERY_BUDGET_MS = 32_000 + +/** One attempt's share of the recovery budget, never reaching past the deadline. */ +export function daemonRecoveryProbeTimeoutMs(recoveryDeadlineMs: number): number { + return Math.max(1, Math.min(DAEMON_RECOVERY_PROBE_MS, recoveryDeadlineMs - Date.now())) +} diff --git a/src/main/daemon/daemon-replacement-preflight.ts b/src/main/daemon/daemon-replacement-preflight.ts index b6f9d7d395c..71e7303d174 100644 --- a/src/main/daemon/daemon-replacement-preflight.ts +++ b/src/main/daemon/daemon-replacement-preflight.ts @@ -3,7 +3,11 @@ import type { DaemonReplaceReason } from '../../shared/daemon-lifecycle-telemetr import { isDaemonStaleForCurrentBundle } from './daemon-bundle-staleness' import { DaemonEndpointOwnershipError } from './daemon-endpoint-adoption' import { checkDaemonHealth, getMacDaemonSystemResolverHealth } from './daemon-health' -import { getAliveDaemonSessionCount, probeDaemonSocket as probeSocket } from './daemon-launch-paths' +import { + DAEMON_SOCKET_PROBE_TIMEOUT_MS, + getAliveDaemonSessionCount, + probeDaemonSocket as probeSocket +} from './daemon-launch-paths' import { trackDaemonReplaced } from './daemon-lifecycle-event' import { getDaemonLaunchIdentity } from './daemon-pid-identity' import { cleanupDaemonForProtocol } from './daemon-protocol-cleanup' @@ -12,7 +16,9 @@ import { killStaleDaemon } from './daemon-stale-kill' import { getMacDaemonTccAttributionHealth } from './daemon-tcc-attribution' import { PROTOCOL_VERSION } from './types' -// Why: extra hello+listSessions probes (~5s each) giving a wedged-but-connectable daemon ~60s grace to answer and keep its live sessions before a permanent wedge (#8689) is replaced; raise only alongside the fail-open cap. +// Why a count on top of the wall clock: DAEMON_RECOVERY_BUDGET_MS ends the grace, but a socket +// that accepts and then resets the hello answers both probes instantly, so without this the loop +// would spin hot for the whole budget. export const WEDGED_DAEMON_GRACE_RETRIES = 11 type PreserveDaemon = (mode?: 'degraded-new-pty-fallback') => Promise @@ -22,6 +28,8 @@ type ReplacementPreflightOptions = { socketPath: string tokenPath: string entryPath: string + /** Absolute deadline for the whole adopt-or-replace decision; see DAEMON_RECOVERY_BUDGET_MS. */ + recoveryDeadlineMs: number attributedReason: DaemonReplaceReason | null releaseAdoptionClient: () => void preserveDaemon: PreserveDaemon @@ -35,6 +43,7 @@ export async function prepareDaemonReplacement( socketPath, tokenPath, entryPath, + recoveryDeadlineMs, attributedReason, releaseAdoptionClient, preserveDaemon @@ -50,7 +59,11 @@ export async function prepareDaemonReplacement( if (health === 'healthy') { const resolverHealth = await getMacDaemonSystemResolverHealth(socketPath, tokenPath) if (resolverHealth === 'unhealthy') { - const liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath) + const liveSessionCount = await getAliveDaemonSessionCount( + socketPath, + tokenPath, + recoveryDeadlineMs + ) if (liveSessionCount !== 0) { console.warn( liveSessionCount === null @@ -81,7 +94,14 @@ export async function prepareDaemonReplacement( const replacementLabel = stalePackagedBundle ? 'launched before the current app bundle was installed' : 'launched from a different app path' - if (await shouldPreserveDaemonWithLiveSessions(socketPath, tokenPath, replacementLabel)) { + if ( + await shouldPreserveDaemonWithLiveSessions( + socketPath, + tokenPath, + recoveryDeadlineMs, + replacementLabel + ) + ) { return preserveDaemon() } console.warn( @@ -105,7 +125,11 @@ export async function prepareDaemonReplacement( if (attributionHealth === 'severed') { // Why: replacing with live sessions would kill them; Settings → Developer // Permissions surfaces the Manage Sessions → Restart remedy instead. - const liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath) + const liveSessionCount = await getAliveDaemonSessionCount( + socketPath, + tokenPath, + recoveryDeadlineMs + ) if (liveSessionCount === 0) { console.warn( '[daemon] Replacing daemon whose macOS TCC attribution is severed (spawning app binary no longer exists)' @@ -124,16 +148,26 @@ export async function prepareDaemonReplacement( } } else { // Why: a busy machine can time out the health check on a live daemon; re-verify with a session list before killing its sessions. - let liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath) + let liveSessionCount = await getAliveDaemonSessionCount( + socketPath, + tokenPath, + recoveryDeadlineMs + ) // Why: a wedged-but-connectable daemon (Windows update relaunch) may still own live sessions, so grace-retry before replacing; a permanent wedge (#8689) exhausts the grace, and 'rejected' skips it (handshake refused = never adoptable). + // Why the clock term: without it the grace is however long the probes happen to take, which + // ran past the startup PTY gate's fail-open cap and hung terminal restore (STA-5732). let graceRetry = 0 while ( liveSessionCount === null && health !== 'rejected' && graceRetry < WEDGED_DAEMON_GRACE_RETRIES && - (await probeSocket(socketPath)) + Date.now() < recoveryDeadlineMs && + (await probeSocket( + socketPath, + Math.max(1, Math.min(DAEMON_SOCKET_PROBE_TIMEOUT_MS, recoveryDeadlineMs - Date.now())) + )) ) { - liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath) + liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath, recoveryDeadlineMs) graceRetry++ } if (liveSessionCount !== null && liveSessionCount > 0) { @@ -213,9 +247,14 @@ export async function prepareDaemonReplacement( async function shouldPreserveDaemonWithLiveSessions( socketPath: string, tokenPath: string, + recoveryDeadlineMs: number, replacementLabel: string ): Promise { - const liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath) + const liveSessionCount = await getAliveDaemonSessionCount( + socketPath, + tokenPath, + recoveryDeadlineMs + ) if (liveSessionCount === 0) { return false }