fix(daemon): replace a permanently wedged daemon instead of preserving it forever (#8689) (#8697)

* fix(daemon): replace a permanently wedged daemon instead of preserving it forever (#8689)

A daemon whose socket accepts connections but whose event loop never answers the
'hello' handshake was adopted by the launcher unconditionally and never re-evaluated,
so every terminal spawn failed with 'DaemonProtocolError: Hello response timed out'
with no recovery.

- daemon-init.ts: bound the launcher's 'preserve any unresponsive-but-connectable
  daemon' with a grace window. A transient wedge (Windows update-relaunch AV/disk
  pressure) drains within ~20s and is preserved WITH its live sessions; a permanent
  wedge exhausts the grace and is replaced. Stays well under the 60s local-PTY
  fail-open cap.
- daemon-pty-adapter.ts: isDaemonGoneError now treats 'Hello response timed out' as
  daemon-gone, so a runtime wedge triggers withDaemonRetry's respawn (re-entering the
  same grace-bounded launcher) instead of failing every spawn until app restart.

Tests pin the grace magnitude so it cannot be silently shrunk.

Co-authored-by: Orca <help@stably.ai>

* fix(daemon): widen wedged-daemon grace window to ~60s

Bump WEDGED_DAEMON_GRACE_RETRIES 3 -> 11 (~20s -> ~60s) to keep live-session
loss on the transient-wedge (Windows update-relaunch) path as close to zero as
possible. A transient wedge drains early and stays under the 60s fail-open cap;
only a permanent wedge runs the full window. Export the constant and pin its
floor in tests so it can't be silently shrunk.

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>

src/main/daemon/daemon-init.test.ts

@@ -7,6 +7,7 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { join } from 'node:path'
 import { PROTOCOL_VERSION } from './types'
+import { WEDGED_DAEMON_GRACE_RETRIES } from './daemon-init'
 
 const FAKE_USER_DATA_PATH = '/fake/userData'
 const FAKE_RUNTIME_DIR = join(FAKE_USER_DATA_PATH, 'daemon')
@@ -1622,15 +1623,36 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
     expect(forkMock).toHaveBeenCalled()
   })
 
-  it('adopts an unresponsive daemon whose pipe still accepts connections (update-relaunch wedge)', async () => {
-    // Why: the Windows update-relaunch regression — post-install disk/AV load
-    // wedges the daemon past the 3s health budget AND the 5s hello budget of
-    // the session-list re-verification, while its sessions are still alive.
-    // The old fail-closed path killed the daemon here. A pipe that accepts a
-    // raw connection proves the daemon is alive, so the launcher must adopt.
+  // Why: a net.connect stub whose 'connect' fires — makes probeSocket() report
+  // the wedged daemon's pipe as alive on every grace re-check.
+  function stubAliveSocketConnect() {
+    const handlers: Record<string, (() => void)[]> = { connect: [], error: [] }
+    return {
+      on(event: string, cb: () => void) {
+        handlers[event]?.push(cb)
+        if (event === 'connect') {
+          queueMicrotask(() => cb())
+        }
+        return this
+      },
+      removeListener(event: string, cb: () => void) {
+        handlers[event] = handlers[event]?.filter((handler) => handler !== cb) ?? []
+        return this
+      },
+      destroy() {}
+    }
+  }
+
+  it('adopts a transiently wedged daemon that drains and reports live sessions within the grace window', async () => {
+    // Why: the Windows update-relaunch case — post-install disk/AV load wedges
+    // the daemon past the first hello budget, but it drains within seconds and
+    // still owns live sessions. The launcher must give it a bounded grace and
+    // ADOPT it rather than killing its live sessions.
     const mod = await importFresh()
     await mod.initDaemonPtyProvider()
 
+    // First probe times out (still draining); the retry within the grace
+    // window succeeds and reports a live session.
     daemonClientMock.mockImplementationOnce(function MockDaemonClient() {
       return {
         ensureConnected: vi.fn(async () => {
@@ -1640,36 +1662,156 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
         disconnect: vi.fn()
       }
     })
+    daemonClientMock.mockImplementationOnce(function MockDaemonClient() {
+      return {
+        ensureConnected: vi.fn(async () => {}),
+        request: vi.fn(async () => ({ sessions: [{ sessionId: 'wt-1@@live', isAlive: true }] })),
+        disconnect: vi.fn()
+      }
+    })
+
+    const launcher = spawnerInstances[0].launcher as (
+      socketPath: string,
+      tokenPath: string
+    ) => Promise<{ shutdown(): Promise<void> }>
+    checkDaemonHealthMock.mockResolvedValueOnce('unreachable')
+    probeSocketExistsMock.mockReturnValue(true)
+    netConnectMock.mockImplementation(stubAliveSocketConnect)
+
+    await launcher('/fake/socket', '/fake/token')
+
+    expect(killStaleDaemonMock).not.toHaveBeenCalled()
+    expect(forkMock).not.toHaveBeenCalled()
+  })
+
+  it('replaces a permanently wedged daemon after the grace window is exhausted (#8689)', async () => {
+    // Why: a daemon whose socket keeps accepting connections but whose event
+    // loop never answers hello would, under the old code, be preserved forever
+    // — every terminal spawn then failed with "Hello response timed out" with
+    // no recovery. After the bounded grace it must be replaced so the app gets
+    // working terminals again.
+    const mod = await importFresh()
+    await mod.initDaemonPtyProvider()
+
+    const answeringDefault = function MockDaemonClient() {
+      return {
+        ensureConnected: vi.fn(async () => {}),
+        request: vi.fn(async () => ({ sessions: [] })),
+        disconnect: vi.fn()
+      }
+    }
+    // Every probe across the whole grace window times out (permanent wedge).
+    daemonClientMock.mockImplementation(function MockDaemonClient() {
+      return {
+        ensureConnected: vi.fn(async () => {
+          throw new Error('Hello response timed out')
+        }),
+        request: vi.fn(),
+        disconnect: vi.fn()
+      }
+    })
 
     const launcher = spawnerInstances[0].launcher as (
       socketPath: string,
       tokenPath: string
     ) => Promise<{ shutdown(): Promise<void> }>
     checkDaemonHealthMock.mockResolvedValueOnce('unreachable')
-    // The raw pipe probe succeeds even though every RPC timed out.
     probeSocketExistsMock.mockReturnValue(true)
-    netConnectMock.mockImplementationOnce(() => {
-      const handlers: Record<string, (() => void)[]> = { connect: [], error: [] }
+    netConnectMock.mockImplementation(stubAliveSocketConnect)
+    forkMock.mockImplementationOnce(() => ({
+      pid: 12345,
+      on(event: string, cb: (arg?: unknown) => void) {
+        if (event === 'message') {
+          queueMicrotask(() => cb({ type: 'ready' }))
+        }
+        return this
+      },
+      off() {
+        return this
+      },
+      disconnect: vi.fn(),
+      unref: vi.fn()
+    }))
+
+    // Count only the launcher's own session-count probes.
+    daemonClientMock.mockClear()
+
+    try {
+      await launcher('/fake/socket', '/fake/token')
+
+      expect(killStaleDaemonMock).toHaveBeenCalledWith(
+        FAKE_RUNTIME_DIR,
+        '/fake/socket',
+        '/fake/token'
+      )
+      expect(forkMock).toHaveBeenCalled()
+      // The launcher probes the full grace budget before giving up: 1 initial
+      // probe + WEDGED_DAEMON_GRACE_RETRIES retries.
+      expect(daemonClientMock).toHaveBeenCalledTimes(1 + WEDGED_DAEMON_GRACE_RETRIES)
+    } finally {
+      // Restore the answering default so the persistent throwing impl above
+      // does not leak into later tests (clearAllMocks clears calls, not impls).
+      daemonClientMock.mockImplementation(answeringDefault)
+    }
+  })
+
+  it('grace budget is generous enough to ride out a ~60s transient wedge', () => {
+    // Why: pins the magnitude. Each probe waits out the client's 5s hello
+    // timeout, so 1 + 11 probes ≈ 60s of drain grace. Shrinking this narrows
+    // the window in which a transiently wedged daemon's live sessions are
+    // preserved instead of replaced — don't cut it without field telemetry.
+    expect(WEDGED_DAEMON_GRACE_RETRIES).toBeGreaterThanOrEqual(11)
+  })
+
+  it('preserves a daemon that stays wedged until the LAST allowed grace retry', async () => {
+    // Why: exercises the full grace loop end-to-end. The daemon throws on every
+    // probe except the final allowed one (1 + WEDGED_DAEMON_GRACE_RETRIES), on
+    // which it drains and reports a live session — so it must be preserved, not
+    // replaced. Cutting the retry budget below the drain point would replace a
+    // still-live daemon, which this test catches.
+    const mod = await importFresh()
+    await mod.initDaemonPtyProvider()
+
+    let probe = 0
+    const answeringDefault = function MockDaemonClient() {
       return {
-        on(event: string, cb: () => void) {
-          handlers[event]?.push(cb)
-          if (event === 'connect') {
-            queueMicrotask(() => cb())
+        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')
           }
-          return this
-        },
-        removeListener(event: string, cb: () => void) {
-          handlers[event] = handlers[event]?.filter((handler) => handler !== cb) ?? []
-          return this
-        },
-        destroy() {}
+        }),
+        request: vi.fn(async () => ({
+          sessions: drainsNow ? [{ sessionId: 'wt-1@@live', isAlive: true }] : []
+        })),
+        disconnect: vi.fn()
       }
     })
 
-    await launcher('/fake/socket', '/fake/token')
+    const launcher = spawnerInstances[0].launcher as (
+      socketPath: string,
+      tokenPath: string
+    ) => Promise<{ shutdown(): Promise<void> }>
+    checkDaemonHealthMock.mockResolvedValueOnce('unreachable')
+    probeSocketExistsMock.mockReturnValue(true)
+    netConnectMock.mockImplementation(stubAliveSocketConnect)
 
-    expect(killStaleDaemonMock).not.toHaveBeenCalled()
-    expect(forkMock).not.toHaveBeenCalled()
+    try {
+      await launcher('/fake/socket', '/fake/token')
+
+      expect(killStaleDaemonMock).not.toHaveBeenCalled()
+      expect(forkMock).not.toHaveBeenCalled()
+    } finally {
+      daemonClientMock.mockImplementation(answeringDefault)
+    }
   })
 
   it('replaces a hello-rejected daemon even though its pipe accepts connections', async () => {
@@ -1695,6 +1837,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
     ) => Promise<{ shutdown(): Promise<void> }>
     checkDaemonHealthMock.mockResolvedValueOnce('rejected')
     probeSocketExistsMock.mockReturnValue(true)
+    netConnectMock.mockImplementation(stubAliveSocketConnect)
     forkMock.mockImplementationOnce(() => ({
       pid: 12345,
       on(event: string, cb: (arg?: unknown) => void) {
@@ -1709,6 +1852,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
       disconnect: vi.fn(),
       unref: vi.fn()
     }))
+    daemonClientMock.mockClear()
 
     await launcher('/fake/socket', '/fake/token')
 
@@ -1718,6 +1862,10 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
       '/fake/token'
     )
     expect(forkMock).toHaveBeenCalled()
+    // Pins the 'rejected' fast-path: a daemon that actively refuses the
+    // handshake is never worth a grace window, so it is probed exactly once
+    // (no retries) before replacement.
+    expect(daemonClientMock).toHaveBeenCalledTimes(1)
   })
 
   it('adopts a healthy daemon whose pid-file identity cannot be verified (null startedAtMs metadata)', async () => {

src/main/daemon/daemon-init.ts

@@ -65,6 +65,24 @@ function logDaemonMilestone(event: string, details: Record<string, unknown> = {}
   }
 }
 
+// Why: how many extra hello+listSessions probes to make against a wedged-but-
+// connectable daemon before replacing it. Each probe waits out the client's 5s
+// hello timeout, so this spaces re-checks ~5s apart: 1 initial + 11 retries ≈
+// 60s of grace for a transiently wedged daemon (Windows update-relaunch drain)
+// to answer and be preserved WITH its live sessions, before a permanent wedge
+// (#8689) is replaced. Deliberately generous to keep live-session loss on the
+// transient path as close to zero as possible.
+//
+// A transient wedge drains early (well under the 60s local-PTY fail-open cap),
+// so its startup is short. Only a *permanent* wedge runs the full window; it can
+// then approach/exceed the fail-open cap, at which point restored panes fail
+// open to the in-process provider for the session and adopt the freshly forked
+// daemon on the next launch — a rare path that still recovers, versus the old
+// forever-broken behavior. Trade-off: a transient wedge owning live sessions
+// that takes longer than ~60s to drain is replaced (live processes lost, though
+// scrollback cold-restores). Raise this only alongside the fail-open cap.
+export const WEDGED_DAEMON_GRACE_RETRIES = 11
+
 let spawner: DaemonSpawner | null = null
 type DaemonProvider = DaemonPtyRouter | DaemonPtyAdapter | DegradedDaemonPtyProvider
 
@@ -257,7 +275,28 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
       // health check while the daemon is alive and owning terminals. Killing
       // it would destroy every live session, so re-verify with a session list
       // first.
-      const liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath)
+      let liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath)
+      // Why: on a Windows update relaunch the daemon can be transiently wedged
+      // past every RPC budget (final checkpoint flush + installer/AV disk
+      // pressure) while its sessions are still alive — replacing it here is what
+      // killed those sessions. A pipe that still accepts connections proves a
+      // live daemon, so give a wedged-but-connectable daemon a bounded grace to
+      // drain and answer before deciding. A PERMANENTLY wedged daemon (accepts
+      // connections but its event loop never answers hello — #8689) exhausts the
+      // grace and falls through to replacement below, instead of being preserved
+      // forever, which strands the app with zero working terminals. 'rejected'
+      // means the daemon answered and refused the handshake — it can never be
+      // adopted, so it skips the grace and replacement stays the only recovery.
+      let graceRetry = 0
+      while (
+        liveSessionCount === null &&
+        health !== 'rejected' &&
+        graceRetry < WEDGED_DAEMON_GRACE_RETRIES &&
+        (await probeSocket(socketPath))
+      ) {
+        liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath)
+        graceRetry++
+      }
       if (liveSessionCount !== null && liveSessionCount > 0) {
         if (health === 'pty-spawn-unhealthy') {
           console.warn(
@@ -274,20 +313,6 @@ function createOutOfProcessLauncher(runtimeDir: string): DaemonLauncher {
         )
         return createPreservedDaemonHandle(runtimeDir)
       }
-      // Why: on a Windows update relaunch the daemon can be wedged past every
-      // RPC budget (final checkpoint flush + installer/AV disk pressure), so
-      // both the health check AND the session list time out while sessions
-      // are still alive — failing closed here is what killed those sessions.
-      // A pipe that still accepts connections proves a live daemon: adopt it
-      // and let the adapter reconnect once the daemon drains. 'rejected'
-      // means the daemon answered and refused the handshake — it can never be
-      // adopted, so replacement stays the only recovery.
-      if (liveSessionCount === null && health !== 'rejected' && (await probeSocket(socketPath))) {
-        console.warn(
-          '[daemon] Preserving unresponsive daemon because its socket still accepts connections'
-        )
-        return createPreservedDaemonHandle(runtimeDir)
-      }
     }
 
     // Why: a raw socket can outlive a broken or wedged daemon. Kill by PID

src/main/daemon/daemon-pty-adapter.test.ts

@@ -4,6 +4,7 @@ import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { mkdtempSync, mkdirSync, rmSync, existsSync, readFileSync, writeFileSync } from 'node:fs'
 import { DaemonClient } from './client'
+import { DaemonProtocolError } from './daemon-errors'
 import { DaemonPtyAdapter } from './daemon-pty-adapter'
 import { DaemonServer } from './daemon-server'
 import { HeadlessEmulator } from './headless-emulator'
@@ -1945,6 +1946,34 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
       noRespawnAdapter.dispose()
     })
 
+    it('treats a hello handshake timeout as daemon-gone and respawns (#8689)', async () => {
+      // Why: a wedged daemon accepts the socket connection but never answers
+      // hello, so ensureConnected() rejects with "Hello response timed out".
+      // That must be classified as daemon-gone so withDaemonRetry respawns and
+      // retries — otherwise every terminal spawn fails against the wedge forever.
+      const realEnsureConnected = DaemonClient.prototype.ensureConnected
+      const ensureConnectedSpy = vi
+        .spyOn(DaemonClient.prototype, 'ensureConnected')
+        .mockImplementationOnce(async () => {
+          // The exact error type + message the real client raises on a wedge.
+          throw new DaemonProtocolError('Hello response timed out')
+        })
+        .mockImplementation(function (this: DaemonClient) {
+          return realEnsureConnected.call(this)
+        })
+      const respawnFn = vi.fn(async () => {})
+      const respawnAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, respawn: respawnFn })
+
+      try {
+        const result = await respawnAdapter.spawn({ cols: 80, rows: 24 })
+        expect(result.id).toBeDefined()
+        expect(respawnFn).toHaveBeenCalledOnce()
+      } finally {
+        ensureConnectedSpy.mockRestore()
+        respawnAdapter.dispose()
+      }
+    })
+
     it('coalesces concurrent respawns so only one daemon is forked', async () => {
       let respawnServer: DaemonServer | undefined
       const respawnFn = vi.fn(async () => {

src/main/daemon/daemon-pty-adapter.ts

@@ -1354,8 +1354,12 @@ export class DaemonPtyAdapter implements IPtyProvider {
 // 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.
+// active or stale connection. "Hello response timed out" means we reconnected
+// to a daemon whose socket accepts connections but whose event loop never
+// answers the handshake (a wedged daemon, #8689) — respawning re-enters the
+// grace-bounded launcher, which drains a transient wedge or replaces a
+// permanent one instead of failing every terminal forever. All indicate the
+// daemon is unusable and a respawn should be attempted.
 function isDaemonGoneError(err: unknown): boolean {
   if (!(err instanceof Error)) {
     return false
@@ -1365,5 +1369,5 @@ function isDaemonGoneError(err: unknown): boolean {
     return true
   }
   const msg = err.message
-  return msg === 'Connection lost' || msg === 'Not connected'
+  return msg === 'Connection lost' || msg === 'Not connected' || msg === 'Hello response timed out'
 }

src/main/daemon/issue-6814-daemon-failure-classification.test.ts

@@ -10,7 +10,8 @@
 // still answers protocol, so a regression that widened it to wedged daemons
 // would silently strand fresh terminals on a daemon that cannot serve them.
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
-import { mkdtempSync, rmSync } from 'node:fs'
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
+import { createServer } from 'node:net'
 import { tmpdir } from 'node:os'
 import { basename, join } from 'node:path'
 import { DaemonServer } from './daemon-server'
@@ -107,13 +108,34 @@ describe('issue #6814 repro: daemon failure-mode classification', () => {
       const health = await checkDaemonHealth(socketPath, tokenPath)
       // This is the key finding: wedged != degraded. #6830's degraded fallback
       // does NOT engage here; recovery still depends on the unreachable-path
-      // (preserve-if-live-else-replace) logic, not the degraded provider.
+      // (grace-bounded preserve-if-live-else-replace) logic, not the degraded
+      // provider.
       expect(health).toBe('unreachable')
     } finally {
       await server.shutdown()
     }
   }, 15000)
 
+  // #8689: a daemon whose socket accepts connections but never answers the
+  // hello handshake (event loop wedged before the protocol reply) also
+  // classifies as 'unreachable' — the launcher's grace-bounded path must
+  // eventually replace it rather than preserve it forever.
+  it('WEDGED-HELLO: a daemon that accepts connections but never answers hello classifies as unreachable (#8689)', async () => {
+    const wedged = createServer((sock) => {
+      // Swallow the hello bytes but never write a response — the exact wedge.
+      sock.on('data', () => {})
+    })
+    await new Promise<void>((resolve) => wedged.listen(socketPath, () => resolve()))
+    // The health check reads the token before connecting, so it must exist.
+    writeFileSync(tokenPath, 'wedged-token')
+    try {
+      const health = await checkDaemonHealth(socketPath, tokenPath)
+      expect(health).toBe('unreachable')
+    } finally {
+      await new Promise<void>((resolve) => wedged.close(() => resolve()))
+    }
+  }, 15000)
+
   // No daemon at all (or token missing) -> unreachable.
   it('UNREACHABLE: no daemon listening classifies as unreachable', async () => {
     await expect(checkDaemonHealth(socketPath, tokenPath)).resolves.toBe('unreachable')