fix(daemon): hold a hello-rejected daemon instead of adopting one that refused us

Found while reviewing why a mutation looked equivalent. Gating the hold on
health === 'unreachable' left 'rejected' — a daemon that answered and refused
the handshake — falling through to preserveDaemon(), whose adoption opens the
very hello it just refused. That throws, and the throw costs the app its
daemon and its Restart remedy. Killing it instead is no better: it can still
be hosting running agents.

Neither of those daemons can complete a handshake, so neither may be adopted,
and both must be held. Gate on that rather than on one of its two causes.

Adds regression tests for the round-four fixes: the self-spawned probe
exclusion is exact-match on both platforms, an adoptable pty-spawn-unhealthy
daemon is never routed to a mode that cannot adopt, evidence can only raise a
verdict, and the grace budget stays under the startup fail-open cap.

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

config/scripts/daemon-replacement-live-agent-pty-preservation-repro.mjs

@@ -595,11 +595,11 @@ function checkLauncherHoldsOccupiedDaemon() {
   // A daemon that did complete a hello is adoptable, so it must not be routed to a mode that
   // never adopts.
   const unverifiableGuard = occupiedBlock.text.match(
-    /if\s*\(\s*health\s*===\s*'unreachable'\s*&&\s*occupancy\.liveSessions\s*===\s*null\s*\)\s*\{/
+    /if\s*\(\s*health\s*===\s*'rejected'\s*\|\|\s*occupancy\.liveSessions\s*===\s*null\s*\)\s*\{/
   )
   assert(
     unverifiableGuard !== null,
-    `${relativePath}:${occupiedLine} does not gate the hold on an unreachable daemon with an unverifiable (null) session count`
+    `${relativePath}:${occupiedLine} does not gate the hold on a daemon that cannot be adopted (rejected, or an unverifiable session count)`
   )
   const unverifiableBlock = extractBlock(
     occupiedBlock.text,

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

@@ -2,7 +2,8 @@
 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'
+import { WEDGED_DAEMON_GRACE_BUDGET_MS, WEDGED_DAEMON_GRACE_RETRIES } from './daemon-init'
+import { LOCAL_PTY_STARTUP_FAIL_OPEN_TIMEOUT_MS } from '../startup/first-window-startup-services'
 
 const FAKE_USER_DATA_PATH = '/fake/userData'
 const FAKE_RUNTIME_DIR = join(FAKE_USER_DATA_PATH, 'daemon')
@@ -3244,6 +3245,54 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
     }
   })
 
+  it('adopts a PTY-spawn-unhealthy daemon in degraded mode instead of merely holding it', async () => {
+    // Why not 'held': 'pty-spawn-unhealthy' is only reachable after a successful hello, so this
+    // daemon CAN be adopted — it just cannot open new PTYs. Holding it (which never adopts)
+    // because the occupancy count happened to come from the process table strands its sessions.
+    const mod = await importFresh()
+    await mod.initDaemonPtyProvider()
+
+    const answeringDefault = function MockDaemonClient() {
+      return {
+        ensureConnected: vi.fn(async () => {}),
+        request: vi.fn(async () => ({ sessions: [] })),
+        disconnect: vi.fn()
+      }
+    }
+    // Hello succeeds; only listSessions is unanswerable, so occupancy falls back to the table.
+    daemonClientMock.mockImplementation(function MockPtySpawnUnhealthyClient() {
+      return {
+        ensureConnected: vi.fn(async () => {}),
+        request: vi.fn(async () => {
+          throw new Error('listSessions timed out')
+        }),
+        disconnect: vi.fn()
+      }
+    })
+    readVerifiedDaemonPidMock.mockResolvedValue({ pid: 4242 })
+    inspectDaemonPtyOwnershipMock.mockResolvedValue('owns-live-ptys')
+
+    const launcher = spawnerInstances[0].launcher as (
+      socketPath: string,
+      tokenPath: string
+    ) => Promise<{ shutdown(): Promise<void>; mode?: string }>
+    checkDaemonHealthMock.mockResolvedValueOnce('pty-spawn-unhealthy')
+    probeSocketExistsMock.mockReturnValue(true)
+    netConnectMock.mockImplementation(stubAliveSocketConnect)
+
+    try {
+      const handle = await launcher('/fake/socket', '/fake/token')
+
+      expect(handle.mode).toBe('degraded-new-pty-fallback')
+      expect(killStaleDaemonMock).not.toHaveBeenCalled()
+      expect(forkMock).not.toHaveBeenCalled()
+    } finally {
+      daemonClientMock.mockImplementation(answeringDefault)
+      readVerifiedDaemonPidMock.mockResolvedValue(null)
+      inspectDaemonPtyOwnershipMock.mockResolvedValue('unknown')
+    }
+  })
+
   it('keeps a daemon handle when the preserved daemon is too wedged to be adopted', async () => {
     // Why: adoption needs a hello, which is exactly what a daemon wedged enough to be
     // preserved cannot answer. Throwing here would abort initDaemonPtyProvider, leaving no
@@ -3291,11 +3340,128 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
     }
   })
 
+  it('holds a hello-rejected daemon that owns live terminals rather than adopting it', async () => {
+    // Why: 'rejected' means it answered and refused the handshake, so adoption can never
+    // succeed. Falling through to preserveDaemon() would throw and cost the app its daemon
+    // entirely — and killing it would end agents that are still running.
+    const mod = await importFresh()
+    await mod.initDaemonPtyProvider()
+
+    const answeringDefault = function MockDaemonClient() {
+      return {
+        ensureConnected: vi.fn(async () => {}),
+        request: vi.fn(async () => ({ sessions: [] })),
+        disconnect: vi.fn()
+      }
+    }
+    daemonClientMock.mockImplementation(function MockRejectingDaemonClient() {
+      return {
+        ensureConnected: vi.fn(async () => {
+          throw new Error('hello refused')
+        }),
+        getDaemonIdentity: vi.fn(readLaunchedDaemonIdentity),
+        request: vi.fn(),
+        disconnect: vi.fn()
+      }
+    })
+    readVerifiedDaemonPidMock.mockResolvedValue({ pid: 4242 })
+    inspectDaemonPtyOwnershipMock.mockResolvedValue('owns-live-ptys')
+
+    const launcher = spawnerInstances[0].launcher as (
+      socketPath: string,
+      tokenPath: string
+    ) => Promise<{ shutdown(): Promise<void>; mode?: string }>
+    checkDaemonHealthMock.mockResolvedValueOnce('rejected')
+    probeSocketExistsMock.mockReturnValue(true)
+    netConnectMock.mockImplementation(stubAliveSocketConnect)
+
+    try {
+      const handle = await launcher('/fake/socket', '/fake/token')
+
+      expect(handle.mode).toBe('held')
+      expect(killStaleDaemonMock).not.toHaveBeenCalled()
+      expect(forkMock).not.toHaveBeenCalled()
+    } finally {
+      daemonClientMock.mockImplementation(answeringDefault)
+      readVerifiedDaemonPidMock.mockResolvedValue(null)
+      inspectDaemonPtyOwnershipMock.mockResolvedValue('unknown')
+    }
+  })
+
   it('grace budget is generous enough to ride out a ~60s transient wedge', () => {
     // Why: each probe waits the client's 5s hello timeout, so 1 + 11 probes ≈ 60s of drain grace; don't cut without telemetry.
     expect(WEDGED_DAEMON_GRACE_RETRIES).toBeGreaterThanOrEqual(11)
   })
 
+  it('leaves headroom under the startup fail-open cap', () => {
+    // Why a wall-clock budget at all: the retry count does not bound the grace window, because
+    // each probe waits the client's hello timeout. Startup abandons the daemon provider entirely
+    // at the fail-open cap, so overrunning it trades a wedged daemon for no daemon at all.
+    expect(WEDGED_DAEMON_GRACE_BUDGET_MS).toBeLessThan(LOCAL_PTY_STARTUP_FAIL_OPEN_TIMEOUT_MS)
+  })
+
+  it('stops grace-retrying when the wall-clock budget runs out, with retries still left', async () => {
+    const mod = await importFresh()
+    await mod.initDaemonPtyProvider()
+
+    const answeringDefault = function MockDaemonClient() {
+      return {
+        ensureConnected: vi.fn(async () => {}),
+        request: vi.fn(async () => ({ sessions: [] })),
+        disconnect: vi.fn()
+      }
+    }
+    // Permanently wedged: without the budget this drains all WEDGED_DAEMON_GRACE_RETRIES probes.
+    daemonClientMock.mockImplementation(function MockWedgedDaemonClient() {
+      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')
+    probeSocketExistsMock.mockReturnValue(true)
+    netConnectMock.mockImplementation(stubAliveSocketConnect)
+    forkMock.mockImplementationOnce(() => {
+      throw new Error('stop after replacement decision')
+    })
+
+    // Stand in for the hello timeout each probe really waits out: mocked probes return instantly,
+    // so only a moving clock can exercise the budget without a slow (or flaky) real wait.
+    const startMs = Date.now()
+    let elapsedMs = 0
+    const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => {
+      const value = startMs + elapsedMs
+      elapsedMs += WEDGED_DAEMON_GRACE_BUDGET_MS / 3
+      return value
+    })
+    const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+
+    try {
+      await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow(
+        'stop after replacement decision'
+      )
+
+      const verdict = warnSpy.mock.calls
+        .map((call) => String(call[0]))
+        .find((message) => message.includes('graceRetries='))
+      expect(verdict).toBeDefined()
+      const graceRetries = Number(/graceRetries=(\d+)/.exec(verdict as string)?.[1])
+      expect(graceRetries).toBeLessThan(WEDGED_DAEMON_GRACE_RETRIES)
+    } finally {
+      nowSpy.mockRestore()
+      warnSpy.mockRestore()
+      daemonClientMock.mockImplementation(answeringDefault)
+    }
+  })
+
   it('preserves a daemon that stays wedged until the LAST allowed grace retry', async () => {
     // Why: daemon drains only on the last allowed probe (1 + WEDGED_DAEMON_GRACE_RETRIES) — must be preserved, not replaced.
     const mod = await importFresh()

src/main/daemon/daemon-init.ts

@@ -633,12 +633,14 @@ function createOutOfProcessLauncher(
             )
             return preserveDaemon('degraded-new-pty-fallback')
           }
-          // Why hold rather than adopt: no hello ever completed and only the process table
-          // could answer, so adoption would fail on the same silence. Holding keeps its
-          // terminals alive and routes fresh ones locally until it recovers or is restarted.
-          if (health === 'unreachable' && occupancy.liveSessions === null) {
+          // Why hold rather than adopt: adoption opens a hello, and neither of these daemons
+          // can complete one — 'rejected' answered and refused, and a count only the process
+          // table could supply means nothing answered at all. Attempting it would throw and
+          // cost the app its daemon entirely. Holding keeps their terminals alive and routes
+          // fresh ones locally until they recover or the user restarts them.
+          if (health === 'rejected' || occupancy.liveSessions === null) {
             console.warn(
-              '[daemon] DEGRADED MODE: holding a daemon that cannot report its sessions but still owns live terminal processes. Killing it would end them; fresh terminals run on the local provider WITHOUT daemon persistence until it recovers or you restart it (Manage Sessions → Restart).'
+              `[daemon] DEGRADED MODE: holding a daemon that cannot be adopted (health=${health}) but still owns ${owned}. Killing it would end them; fresh terminals run on the local provider WITHOUT daemon persistence until it recovers or you restart it (Manage Sessions → Restart).`
             )
             return holdIncumbentDaemon()
           }

src/main/daemon/daemon-live-pty-evidence.test.ts

@@ -109,6 +109,47 @@ describe('inspectDaemonPtyOwnership on POSIX', () => {
     ).resolves.toBe('no-live-ptys')
   })
 
+  it("excludes the daemon's own PTY-spawn probe, which forkpty also makes a session leader", async () => {
+    // Why the stat flag is not enough: the daemon opens this PTY itself, so a daemon hosting
+    // zero user terminals would be held forever on the strength of its own stuck health check.
+    await expect(
+      inspectDaemonPtyOwnership(DAEMON_PID, {
+        platform: 'darwin',
+        readPosixProcessTable: posixTable([
+          daemonRow,
+          row(101, DAEMON_PID, { stat: 'Ss', command: '/bin/sh -c exit 0' })
+        ])
+      })
+    ).resolves.toBe('no-live-ptys')
+  })
+
+  it("still counts a real terminal sitting beside the daemon's own probe", async () => {
+    await expect(
+      inspectDaemonPtyOwnership(DAEMON_PID, {
+        platform: 'darwin',
+        readPosixProcessTable: posixTable([
+          daemonRow,
+          row(101, DAEMON_PID, { stat: 'Ss', command: '/bin/sh -c exit 0' }),
+          row(202, DAEMON_PID, { stat: 'Ss+', command: 'claude' })
+        ])
+      })
+    ).resolves.toBe('owns-live-ptys')
+  })
+
+  it('does not exclude an agent whose command merely contains a probe command', async () => {
+    // Exact match, not prefix or substring: `sh -c` payloads are user-supplied, and treating one
+    // as the daemon's own probe discards proof that killing the daemon would end real work.
+    await expect(
+      inspectDaemonPtyOwnership(DAEMON_PID, {
+        platform: 'darwin',
+        readPosixProcessTable: posixTable([
+          daemonRow,
+          row(101, DAEMON_PID, { stat: 'Ss', command: '/bin/sh -c exit 0 && claude' })
+        ])
+      })
+    ).resolves.toBe('owns-live-ptys')
+  })
+
   it('counts a session leader reached through a non-session-leader hop', async () => {
     await expect(
       inspectDaemonPtyOwnership(DAEMON_PID, {
@@ -194,6 +235,43 @@ describe('inspectDaemonPtyOwnership on win32', () => {
     ).resolves.toBe('no-live-ptys')
   })
 
+  it('ignores the conpty warmup processes the daemon spawns for itself', async () => {
+    // Casing and padding vary with how the OS recorded the command line, and this branch counts
+    // *any* descendant — so without the match a daemon hosting nothing would look occupied.
+    await expect(
+      inspectDaemonPtyOwnership(DAEMON_PID, {
+        platform: 'win32',
+        queryWindowsDescendants: async () => [
+          { ...windowsCandidate(101), command: '  CMD.EXE /C EXIT  ' },
+          { ...windowsCandidate(102), command: 'Cmd /C Exit' }
+        ]
+      })
+    ).resolves.toBe('no-live-ptys')
+  })
+
+  it('still counts a hosted terminal sitting beside the conpty warmup', async () => {
+    await expect(
+      inspectDaemonPtyOwnership(DAEMON_PID, {
+        platform: 'win32',
+        queryWindowsDescendants: async () => [
+          { ...windowsCandidate(101), command: 'cmd.exe /c exit' },
+          { ...windowsCandidate(202), command: 'claude' }
+        ]
+      })
+    ).resolves.toBe('owns-live-ptys')
+  })
+
+  it('does not exclude a command that merely starts with the warmup command', async () => {
+    await expect(
+      inspectDaemonPtyOwnership(DAEMON_PID, {
+        platform: 'win32',
+        queryWindowsDescendants: async () => [
+          { ...windowsCandidate(101), command: 'cmd.exe /c exit && npm run dev' }
+        ]
+      })
+    ).resolves.toBe('owns-live-ptys')
+  })
+
   it('reports unknown when enumeration failed or the root was absent', async () => {
     await expect(
       inspectDaemonPtyOwnership(DAEMON_PID, {

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

@@ -1,5 +1,10 @@
 import { describe, expect, it, vi } from 'vitest'
-import { resolveDaemonOccupancy, type DaemonOccupancyDeps } from './daemon-occupancy'
+import {
+  raiseOccupancyWithProcessEvidence,
+  resolveDaemonOccupancy,
+  type DaemonOccupancy,
+  type DaemonOccupancyDeps
+} from './daemon-occupancy'
 import type { inspectDaemonPtyOwnership } from './daemon-live-pty-evidence'
 
 const SOCKET_PATH = '/tmp/orca-daemon.sock'
@@ -108,6 +113,83 @@ describe('resolveDaemonOccupancy when the daemon could not answer', () => {
   })
 })
 
+describe('raiseOccupancyWithProcessEvidence', () => {
+  const UNKNOWN: DaemonOccupancy = { state: 'unknown', liveSessions: null }
+
+  it('raises an unanswered verdict to occupied, with no count to report', async () => {
+    const inspectPtyOwnership = ownershipIs('owns-live-ptys')
+
+    await expect(
+      raiseOccupancyWithProcessEvidence(UNKNOWN, DAEMON_PID, { inspectPtyOwnership })
+    ).resolves.toEqual({ state: 'occupied', liveSessions: null })
+    expect(inspectPtyOwnership).toHaveBeenCalledWith(DAEMON_PID)
+  })
+
+  it('leaves an unanswered verdict unknown — never empty — when the table shows no live PTYs', async () => {
+    // The whole point of the raise-only contract: ps can miss PTYs it never observed, so an
+    // empty-looking table is not permission to kill a daemon that could not answer for itself.
+    await expect(
+      raiseOccupancyWithProcessEvidence(UNKNOWN, DAEMON_PID, {
+        inspectPtyOwnership: ownershipIs('no-live-ptys')
+      })
+    ).resolves.toEqual(UNKNOWN)
+  })
+
+  it('leaves an unanswered verdict unchanged when the table could not be read', async () => {
+    await expect(
+      raiseOccupancyWithProcessEvidence(UNKNOWN, DAEMON_PID, {
+        inspectPtyOwnership: ownershipIs('unknown')
+      })
+    ).resolves.toEqual(UNKNOWN)
+  })
+
+  it('does not inspect an unverified pid', async () => {
+    // A pid we could not tie back to this daemon may have been recycled; its children are
+    // evidence about the wrong process tree.
+    const inspectPtyOwnership = ownershipIs('owns-live-ptys')
+
+    await expect(
+      raiseOccupancyWithProcessEvidence(UNKNOWN, null, { inspectPtyOwnership })
+    ).resolves.toEqual(UNKNOWN)
+    expect(inspectPtyOwnership).not.toHaveBeenCalled()
+  })
+
+  it('returns an empty verdict untouched, without consulting the process table', async () => {
+    // 'empty' came from the daemon itself and is the one state that licenses a kill. Re-asking
+    // the table could only lower it back to 'unknown', discarding an IPC-proven answer.
+    const inspectPtyOwnership = ownershipIs('owns-live-ptys')
+
+    await expect(
+      raiseOccupancyWithProcessEvidence({ state: 'empty', liveSessions: 0 }, DAEMON_PID, {
+        inspectPtyOwnership
+      })
+    ).resolves.toEqual({ state: 'empty', liveSessions: 0 })
+    expect(inspectPtyOwnership).not.toHaveBeenCalled()
+  })
+
+  it('keeps the counted sessions of an already-occupied verdict', async () => {
+    // Raising an answered count to the countless 'occupied' would lose what the daemon reported.
+    const inspectPtyOwnership = ownershipIs('no-live-ptys')
+
+    await expect(
+      raiseOccupancyWithProcessEvidence({ state: 'occupied', liveSessions: 3 }, DAEMON_PID, {
+        inspectPtyOwnership
+      })
+    ).resolves.toEqual({ state: 'occupied', liveSessions: 3 })
+    expect(inspectPtyOwnership).not.toHaveBeenCalled()
+  })
+
+  it('leaves the verdict unchanged when the inspector throws', async () => {
+    await expect(
+      raiseOccupancyWithProcessEvidence(UNKNOWN, DAEMON_PID, {
+        inspectPtyOwnership: vi.fn<typeof inspectDaemonPtyOwnership>(async () => {
+          throw new Error('process table read exploded')
+        })
+      })
+    ).resolves.toEqual(UNKNOWN)
+  })
+})
+
 describe('resolveDaemonOccupancy when an injected dep throws', () => {
   it('degrades an inspector rejection to unknown', async () => {
     // Why: a question that could not be asked is exactly what the residual is for. Letting