fix(terminal): harden daemon respawn against stale events and false ENOENT

- Add connection generation counter in DaemonClient so stale 'close'
  events from old sockets don't tear down a fresh post-respawn connection
- Tighten isDaemonGoneError to require syscall='connect' for ENOENT/
  ECONNREFUSED, avoiding false-positive respawn on token-file read errors
This commit is contained in:
Jinwoo-H 2026-04-20 22:38:30 -04:00
parent ea8f5249f4
commit 95d5c1d928
2 changed files with 18 additions and 12 deletions

View file

@ -28,6 +28,11 @@ export class DaemonClient {
private streamSocket: Socket | null = null
private connected = false
private disconnectArmed = false
// Why: after a disconnect + reconnect (daemon respawn), a stale 'close'
// event from the old sockets can fire. Without a generation check, that
// event would tear down the fresh connection. Each doConnect() increments
// the generation; handleDisconnect ignores events from old generations.
private connectionGeneration = 0
// Why: multiple concurrent spawn() calls from simultaneous pane mounts
// all call ensureConnected(). Without a lock, each starts a separate
// connection attempt, overwriting sockets and triggering "Connection lost".
@ -78,9 +83,10 @@ export class DaemonClient {
this.connected = true
this.disconnectArmed = true
this.connectionGeneration++
// Handle socket close
const handleClose = () => this.handleDisconnect()
const gen = this.connectionGeneration
const handleClose = () => this.handleDisconnect(gen)
this.controlSocket.on('close', handleClose)
this.controlSocket.on('error', handleClose)
this.streamSocket.on('close', handleClose)
@ -270,8 +276,8 @@ export class DaemonClient {
this.streamSocket.on('data', (chunk) => parser.feed(chunk.toString()))
}
private handleDisconnect(): void {
if (!this.disconnectArmed) {
private handleDisconnect(generation: number): void {
if (!this.disconnectArmed || generation !== this.connectionGeneration) {
return
}
this.disconnectArmed = false

View file

@ -448,18 +448,18 @@ export class DaemonPtyAdapter implements IPtyProvider {
}
}
// Why: ENOENT means the socket file was deleted (daemon crashed and cleaned
// up, or was killed). ECONNREFUSED means the file exists but nothing is
// listening (rare race). "Connection lost" / "Not connected" mean the daemon
// died while we had an active or stale connection — the client detected the
// socket close but we still tried to use it. All indicate the daemon is
// gone and a respawn should be attempted.
// Why: ENOENT/ECONNREFUSED with syscall 'connect' mean the socket is
// unreachable (daemon died). Checking syscall avoids false positives from
// token-file ENOENT (readFileSync), which has no syscall or syscall='open'.
// "Connection lost" / "Not connected" mean the daemon died while we had an
// active or stale connection. All indicate the daemon is gone and a respawn
// should be attempted.
function isDaemonGoneError(err: unknown): boolean {
if (!(err instanceof Error)) {
return false
}
const code = (err as NodeJS.ErrnoException).code
if (code === 'ENOENT' || code === 'ECONNREFUSED') {
const errno = err as NodeJS.ErrnoException
if ((errno.code === 'ENOENT' || errno.code === 'ECONNREFUSED') && errno.syscall === 'connect') {
return true
}
const msg = err.message