diff --git a/src/main/daemon/client.test.ts b/src/main/daemon/client.test.ts index 47b77cfb78e..dfca904ecea 100644 --- a/src/main/daemon/client.test.ts +++ b/src/main/daemon/client.test.ts @@ -335,6 +335,36 @@ describe('DaemonClient', () => { client = new DaemonClient({ socketPath, tokenPath }) await expect(client.ensureConnected()).rejects.toThrow() }) + + it('fails a retired endpoint at the connect, not at the token read', async () => { + // Why: the daemon unlinks its token on exit. Failing at the open produced an error no + // endpoint-gone predicate could classify, because they all key on syscall 'connect'. + rmSync(tokenPath) + + const error = (await client_ensureConnectedError()) as NodeJS.ErrnoException + + expect(error.syscall).toBe('connect') + expect(['ENOENT', 'ECONNREFUSED']).toContain(error.code) + }) + + it('still attempts the handshake when the token is gone but a daemon is listening', async () => { + // Why: a missing token is not proof of death — it can be rotated or externally removed while + // the daemon is alive. The daemon must get the chance to reject it. + const hellos: HelloMessage[] = [] + await startMockDaemon({ onHello: (msg) => hellos.push(msg) }) + rmSync(tokenPath) + + client = new DaemonClient({ socketPath, tokenPath }) + await client.ensureConnected() + + expect(hellos.length).toBeGreaterThan(0) + expect(hellos[0]?.token).toBe('') + }) + + async function client_ensureConnectedError(): Promise { + client = new DaemonClient({ socketPath, tokenPath }) + return client.ensureConnected().catch((err: unknown) => err) + } }) describe('RPC', () => { diff --git a/src/main/daemon/client.ts b/src/main/daemon/client.ts index d137ee84ea5..6f745cd4b2a 100644 --- a/src/main/daemon/client.ts +++ b/src/main/daemon/client.ts @@ -114,12 +114,31 @@ export class DaemonClient { } } + /** + * Why absence is a value, not a throw: the daemon unlinks its token on exit but leaves its socket, + * so throwing here preempts the connect — and the connect is what proves the endpoint gone + * (ENOENT/ECONNREFUSED with syscall 'connect'), which every recovery predicate keys on. Reading + * totally lets a dead daemon fail as a connect, and a live one reject the empty token as + * 'Invalid token' rather than being declared dead by a missing file. checkDaemonHealth has always + * read it this way; this client was the outlier. + */ + private readToken(): string { + try { + return readFileSync(this.tokenPath, 'utf-8').trim() + } catch (error) { + if ((error as NodeJS.ErrnoException | null)?.code === 'ENOENT') { + return '' + } + throw error + } + } + private async doConnect( timeoutMs: number, attemptGeneration: number, sharedBudget: boolean ): Promise { - const token = readFileSync(this.tokenPath, 'utf-8').trim() + const token = this.readToken() const deadlineMs = Date.now() + timeoutMs const remainingMs = (): number => sharedBudget ? Math.max(1, deadlineMs - Date.now()) : timeoutMs diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index b138fbe3b8d..1e41e9de9ee 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -595,6 +595,42 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { } }) + it('respawns after a retired daemon regardless of how the connection ended', async () => { + // Why this shape: the daemon retires when its last authenticated client drops, and that drop + // is often our own disconnect() — which observes no socket close. Once the token read stops + // preempting the connect, the retired endpoint fails as a connect and isDaemonGoneError + // classifies it, so recovery no longer depends on having witnessed the drop. + let respawnServer: DaemonServer | undefined + const respawn = vi.fn(async () => { + respawnServer = new DaemonServer({ + socketPath, + tokenPath, + spawnSubprocess: () => createMockSubprocess() + }) + await respawnServer.start() + }) + const healingAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const { id } = await healingAdapter.spawn({ cols: 80, rows: 24 }) + const client = (healingAdapter as unknown as { client: DaemonClient }).client + + client.disconnect() + await server.shutdown() + expect(existsSync(tokenPath)).toBe(false) + expect(client.hasObservedAuthenticatedDisconnect()).toBe(false) + + await expect( + healingAdapter.spawn({ sessionId: id, cols: 80, rows: 24 }) + ).resolves.toMatchObject({ id }) + expect(respawn).toHaveBeenCalledTimes(1) + } finally { + warn.mockRestore() + healingAdapter.dispose() + await respawnServer?.shutdown() + } + }) + it('does not spawn a daemon per keystroke after respawn fails', async () => { const respawn = vi.fn(async () => { throw new Error('daemon unavailable') diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index 9e8142922ac..c9a087b534c 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -1473,8 +1473,7 @@ export class DaemonPtyAdapter implements IPtyProvider { ) return processes } catch (error) { - const missingAuthenticatedToken = - isMissingTokenFileError(error) && this.client.hasObservedAuthenticatedDisconnect() + const missingAuthenticatedToken = this.isRetiredEndpointTokenMissing() const missingNamedPipe = isMissingWindowsNamedPipeError(error) this.observeAuditFailure( missingAuthenticatedToken @@ -2121,13 +2120,21 @@ export class DaemonPtyAdapter implements IPtyProvider { } // Why: on daemon-death errors, respawn a fresh daemon and retry once rather than leaving terminals broken until app restart. + /** + * Why a file check and not an errno shape: the client reads the token totally now, so a retired + * endpoint fails at the connect rather than the open. The fact worth auditing is unchanged — the + * token is gone after we were authenticated — and the filesystem answers it directly. An absent + * token before any authenticated session still proves nothing; a daemon may be mid-startup. + */ + private isRetiredEndpointTokenMissing(): boolean { + return this.client.hasObservedAuthenticatedDisconnect() && !existsSync(this.tokenPath) + } + private async withDaemonRetry(fn: () => Promise): Promise { try { return await fn() } catch (err) { - // Why: the token is removed only after an authenticated drop; an initial missing token may still hide a live daemon. - const missingRetiredEndpointToken = - isMissingTokenFileError(err) && this.client.hasObservedAuthenticatedDisconnect() + const missingRetiredEndpointToken = this.isRetiredEndpointTokenMissing() if (missingRetiredEndpointToken) { this.observeAuditFailure( 'token_missing_after_authenticated_disconnect', @@ -2533,14 +2540,6 @@ export function isDaemonGoneError(err: unknown): boolean { ) } -function isMissingTokenFileError(err: unknown): boolean { - if (!(err instanceof Error)) { - return false - } - const errno = err as NodeJS.ErrnoException - return errno.code === 'ENOENT' && errno.syscall === 'open' -} - function isMissingWindowsNamedPipeError(err: unknown): boolean { if (process.platform !== 'win32' || !(err instanceof Error)) { return false diff --git a/src/main/daemon/daemon-self-retirement-respawn.test.ts b/src/main/daemon/daemon-self-retirement-respawn.test.ts index a5df5453fe6..4786ab2e0b1 100644 --- a/src/main/daemon/daemon-self-retirement-respawn.test.ts +++ b/src/main/daemon/daemon-self-retirement-respawn.test.ts @@ -213,17 +213,33 @@ describe('daemon self-retirement respawn', () => { expect(respawn).not.toHaveBeenCalled() }) - it('does not treat an initial missing token as respawn authority', async () => { + // Why these two replace "does not treat an initial missing token as respawn authority": that guard + // was added alongside the token unlink, to stop an absent token from authorizing a respawn. The + // token read no longer preempts the connect, so authority comes from whether anything is accepting + // on the endpoint — a strictly stronger signal. The hazard the old guard covered is the first test + // here: a daemon that has bound its socket but not yet written its token must not be replaced. + it('does not respawn a listening daemon whose token is absent', async () => { + await startServer() + rmSync(tokenPath) const respawn = vi.fn(async () => {}) const adapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) - await expect(adapter.spawn({ sessionId: 'missing', cols: 80, rows: 24 })).rejects.toMatchObject( - { - code: 'ENOENT' - } - ) + await expect( + adapter.spawn({ sessionId: 'startup-window', cols: 80, rows: 24 }) + ).rejects.toThrow(/Invalid token/i) expect(respawn).not.toHaveBeenCalled() adapter.dispose() }) + + it('respawns when nothing is accepting on the endpoint', async () => { + const respawn = vi.fn(async () => {}) + const adapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn }) + + // Fails after the respawn attempt because the mock respawn starts no replacement. + await expect(adapter.spawn({ sessionId: 'missing', cols: 80, rows: 24 })).rejects.toThrow() + + expect(respawn).toHaveBeenCalledTimes(1) + adapter.dispose() + }) })