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.
This commit is contained in:
Brennan Benson
2026-08-12 17:34:48 -07:00
parent ef8eea51af
commit 9c7fc2540a
5 changed files with 120 additions and 20 deletions
+30
View File
@@ -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<unknown> {
client = new DaemonClient({ socketPath, tokenPath })
return client.ensureConnected().catch((err: unknown) => err)
}
})
describe('RPC', () => {
+20 -1
View File
@@ -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<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')
+12 -13
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
@@ -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<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',
@@ -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
@@ -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()
})
})