fix(daemon): let the connect prove a retired daemon, not the token read (#14128)

* fix(daemon): let the connect prove a retired daemon, not the token read

The daemon unlinks its token on exit but leaves its socket, and the client read
that token as the first statement of doConnect. So a retired endpoint failed at
the open, producing an errno no recovery predicate could classify — every one of
them keys on syscall 'connect' — and the pane showed raw errno text.

Read the token totally instead. A dead endpoint now fails at the connect, where
isDaemonGoneError already authorizes a respawn and isDaemonEndpointGoneError
already explains itself; a live daemon whose token was rotated or removed
rejects the empty token as 'Invalid token' rather than being declared dead by a
missing file. checkDaemonHealth has always read the token this way.

* fix(daemon): require endpoint failure for respawn
This commit is contained in:
Brennan Benson
2026-08-12 18:28:35 -07:00
committed by GitHub
parent 6ac39b7331
commit d9b390f4fd
5 changed files with 122 additions and 26 deletions
+24
View File
@@ -335,6 +335,30 @@ 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 () => {
rmSync(tokenPath)
client = new DaemonClient({ socketPath, tokenPath })
const error = (await client
.ensureConnected()
.catch((err: unknown) => err)) 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 () => {
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('')
})
})
describe('RPC', () => {
+13 -1
View File
@@ -114,12 +114,24 @@ export class DaemonClient {
}
}
// Why: a missing token must not preempt the connect that proves whether the endpoint is gone.
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<void> {
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
@@ -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')
+8 -19
View File
@@ -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
@@ -2120,14 +2119,16 @@ export class DaemonPtyAdapter implements IPtyProvider {
return 'unavailable'
}
// Why: on daemon-death errors, respawn a fresh daemon and retry once rather than leaving terminals broken until app restart.
// Why: the token read no longer throws, so audit its absence directly after an authenticated drop.
private isRetiredEndpointTokenMissing(): boolean {
return this.client.hasObservedAuthenticatedDisconnect() && !existsSync(this.tokenPath)
}
private async withDaemonRetry<T>(fn: () => Promise<T>): Promise<T> {
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',
@@ -2135,11 +2136,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
['token_file']
)
}
if (
this.respawnAdoptionClosed ||
!this.respawnFn ||
(!isDaemonGoneError(err) && !missingRetiredEndpointToken)
) {
if (this.respawnAdoptionClosed || !this.respawnFn || !isDaemonGoneError(err)) {
throw err
}
if (!this.respawnPromise) {
@@ -2533,14 +2530,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
@@ -213,17 +213,52 @@ describe('daemon self-retirement respawn', () => {
expect(respawn).not.toHaveBeenCalled()
})
it('does not treat an initial missing token as respawn authority', async () => {
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('does not let stale disconnect evidence authorize respawning a listening daemon', async () => {
const original = await startServer()
const respawn = vi.fn(async () => {})
const adapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn })
await adapter.listProcesses()
const client = (
adapter as unknown as {
client: { hasObservedAuthenticatedDisconnect(): boolean }
}
).client
await original.shutdown()
await waitFor(() => client.hasObservedAuthenticatedDisconnect())
await startServer()
rmSync(tokenPath)
await expect(
adapter.spawn({ sessionId: 'replacement-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()
})
})