test(daemon): kill the mutations that let the PTY veto ship as a no-op

Mutation testing found four survivors — changes that break the fix while
every test stays green:

- Swapping the POSIX reader to the 500ms-cached one passed. It is not just
  a staleness hazard: inside the TTL both sampling attempts receive the same
  array, collapsing the two-sample confirmation to one. Pin the fresh reader.
- Replacing killStaleDaemon's default inspector with one that never reports
  live PTYs — the veto disabled in production — passed, because every veto
  test injects the hook. Exercise the real seam.
- Adding the veto to cleanupDaemonForProtocol passed, which is verbatim the
  failure its own doc warns about: a user-initiated restart of a daemon
  owning live PTYs would refuse, then throw. Pin that call's arity.
- The ppid-cycle fixture put the cycle outside the daemon's subtree, so the
  walk never entered it and deleting the visited guard passed.

Also bound the Windows enumeration, which had no budget of its own: two CIM
queries with a wmic fallback can stall the launch path for tens of seconds.
Blind is a safe answer there; hanging is not.

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

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

@@ -22,6 +22,16 @@ import {
   startTimesWithinTolerance
 } from './daemon-health'
 import type { SubprocessHandle } from './session'
+import type { DaemonPtyOwnership } from './daemon-live-pty-evidence'
+
+// Why: the veto's production default is otherwise never exercised — every other test injects it.
+const { inspectDaemonPtyOwnershipMock } = vi.hoisted(() => ({
+  inspectDaemonPtyOwnershipMock: vi.fn(async (_pid: number) => 'unknown' as DaemonPtyOwnership)
+}))
+
+vi.mock('./daemon-live-pty-evidence', () => ({
+  inspectDaemonPtyOwnership: inspectDaemonPtyOwnershipMock
+}))
 
 function createMockSubprocess(): SubprocessHandle {
   return {
@@ -697,6 +707,26 @@ describe.skipIf(process.platform === 'win32')('killStaleDaemon live PTY ownershi
     }
   })
 
+  it('routes the opt-in through the real ownership inspector when no hook is injected', async () => {
+    // Why: production passes no hook, so a default that never sees live PTYs would ship a
+    // veto that silently never fires — indistinguishable from having no veto at all.
+    inspectDaemonPtyOwnershipMock.mockResolvedValueOnce('owns-live-ptys')
+    const killSpy = vi.spyOn(process, 'kill')
+
+    try {
+      await expect(
+        killStaleDaemon(dir, socketPath, tokenPath, undefined, {
+          probeEndpoint: async () => 'missing',
+          preserveWhenOwningLivePtys: true
+        })
+      ).resolves.toEqual({ killed: false, liveOwnerSurvived: true })
+      expect(inspectDaemonPtyOwnershipMock).toHaveBeenCalledWith(stubPid)
+      expect(terminationSignals(killSpy)).toEqual([])
+    } finally {
+      killSpy.mockRestore()
+    }
+  })
+
   it('never consults PTY ownership without the opt-in, so restarts stay killable', async () => {
     const inspectPtyOwnership = vi.fn(async () => 'owns-live-ptys' as const)
     const killSpy = vi.spyOn(process, 'kill')

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

@@ -1196,6 +1196,50 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
     }
   })
 
+  it('falls back to killStaleDaemon without the live-PTY veto, so an explicit restart always wins', async () => {
+    // Why: this is the documented escape hatch (Settings → Manage Sessions → Restart). Opting
+    // into the veto here would report liveOwnerSurvived and throw, leaving the user no daemon.
+    probeSocketExistsMock.mockReturnValue(true)
+    netConnectMock.mockImplementation(() => {
+      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() {}
+      }
+    })
+    const mod = await importFresh()
+    daemonClientMock.mockImplementationOnce(function MockWedgedDaemonClient() {
+      return {
+        ensureConnected: vi.fn(async () => {
+          throw new Error('Hello response timed out')
+        }),
+        request: vi.fn(),
+        disconnect: vi.fn()
+      }
+    })
+
+    await expect(
+      mod.cleanupDaemonForProtocol('/fake/daemon', PROTOCOL_VERSION)
+    ).resolves.toMatchObject({ cleaned: true })
+
+    expect(killStaleDaemonMock).toHaveBeenCalledWith(
+      '/fake/daemon',
+      `/fake/daemon/daemon-v${PROTOCOL_VERSION}.sock`,
+      `/fake/daemon/daemon-v${PROTOCOL_VERSION}.token`,
+      PROTOCOL_VERSION
+    )
+  })
+
   it('coalesces concurrent restartDaemon() calls so the 7-step sequence runs exactly once', async () => {
     const mod = await importFresh()
     await mod.initDaemonPtyProvider()
@@ -2884,7 +2928,10 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
     expect(forkMock).not.toHaveBeenCalled()
   })
 
-  it('replaces a health-check-failing daemon when live sessions cannot be verified and the pipe is dead', async () => {
+  // Note: only the adoption client is wedged here; the session probe answers, so this is the
+  // verified-zero case and the live-PTY veto stays off. The unverifiable path is covered by
+  // the grace-retry tests below.
+  it('replaces a health-check-failing daemon that reports no sessions when the pipe is dead', async () => {
     const mod = await importFresh()
     await mod.initDaemonPtyProvider()
 

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

@@ -1,8 +1,20 @@
-import { describe, expect, it, vi } from 'vitest'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { inspectDaemonPtyOwnership } from './daemon-live-pty-evidence'
 import type { ProcessTableRow } from '../../shared/process-table-snapshot'
 import type { WindowsProcessCandidate } from '../providers/windows-foreground-process-rows'
 
+const { readFreshProcessTable, readCachedProcessTable } = vi.hoisted(() => ({
+  readFreshProcessTable: vi.fn(async () => [] as ProcessTableRow[]),
+  readCachedProcessTable: vi.fn(async () => [] as ProcessTableRow[])
+}))
+
+// Spread the original: the Windows enumerator builds its reader from this module too.
+vi.mock('../../shared/process-table-snapshot', async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  getFreshProcessTableSnapshot: readFreshProcessTable,
+  getProcessTableSnapshot: readCachedProcessTable
+}))
+
 const DAEMON_PID = 4242
 
 function row(pid: number, ppid: number, overrides: Partial<ProcessTableRow> = {}): ProcessTableRow {
@@ -103,13 +115,20 @@ describe('inspectDaemonPtyOwnership on POSIX', () => {
     ).resolves.toBe('unknown')
   })
 
-  it('tolerates a ppid cycle without hanging', async () => {
+  it('tolerates a ppid cycle reachable from the daemon without hanging', async () => {
+    // Why this shape: `ps` is not atomic, so a re-parented process can appear twice and
+    // close a loop. The cycle must be reachable from the root or the walk never enters it.
     await expect(
       inspectDaemonPtyOwnership(DAEMON_PID, {
         platform: 'darwin',
-        readPosixProcessTable: posixTable([daemonRow, row(101, 102), row(102, 101)])
+        readPosixProcessTable: posixTable([
+          daemonRow,
+          row(101, DAEMON_PID),
+          row(102, 101),
+          row(101, 102)
+        ])
       })
-    ).resolves.toBe('no-live-ptys')
+    ).resolves.toBe('owns-live-ptys')
   })
 
   it('never consults the Windows enumerator on POSIX', async () => {
@@ -166,6 +185,21 @@ describe('inspectDaemonPtyOwnership on win32', () => {
     ).resolves.toBe('unknown')
   })
 
+  it('gives up as unknown when enumeration blows its deadline', async () => {
+    // Why bounded: the CIM query has no budget of its own and can queue behind an in-flight
+    // scan, so an unbounded wait would stall the launch path. Blind is safe here; hanging is not.
+    const queryWindowsDescendants = vi.fn(() => new Promise<WindowsProcessCandidate[]>(() => {}))
+
+    await expect(
+      inspectDaemonPtyOwnership(DAEMON_PID, {
+        platform: 'win32',
+        windowsDeadlineMs: 5,
+        queryWindowsDescendants
+      })
+    ).resolves.toBe('unknown')
+    expect(queryWindowsDescendants).toHaveBeenCalled()
+  })
+
   it('never consults the POSIX table on win32', async () => {
     const readPosixProcessTable = vi.fn(async () => [daemonRow])
 
@@ -238,3 +272,21 @@ describe('inspectDaemonPtyOwnership sampling', () => {
     expect(readPosixProcessTable.mock.calls.length).toBeGreaterThan(1)
   })
 })
+
+describe('inspectDaemonPtyOwnership POSIX process-table source', () => {
+  beforeEach(() => {
+    readFreshProcessTable.mockReset()
+    readCachedProcessTable.mockReset()
+    readFreshProcessTable.mockResolvedValue([daemonRow, row(101, DAEMON_PID)])
+    readCachedProcessTable.mockResolvedValue([])
+  })
+
+  it('reads an uncached table, since the cached one can predate the PTYs it protects', async () => {
+    // The 500ms TTL would also hand both samples the same array, collapsing the confirmation.
+    await expect(inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin' })).resolves.toBe(
+      'owns-live-ptys'
+    )
+    expect(readFreshProcessTable).toHaveBeenCalled()
+    expect(readCachedProcessTable).not.toHaveBeenCalled()
+  })
+})

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

@@ -21,17 +21,42 @@ export type DaemonPtyOwnershipDeps = {
   platform?: NodeJS.Platform
   readPosixProcessTable?: () => Promise<ProcessTableRow[]>
   queryWindowsDescendants?: typeof queryWindowsProcessDescendants
+  windowsDeadlineMs?: number
 }
 
 /**
- * Why generous, and why sampled twice: the load that wedges the daemon is the same
- * load that makes `ps` miss a tight budget, so the evidence would go blind exactly
- * when it matters most. A positive is confirmed by a second sample so a short-lived
- * child — a resolver probe, a health-check shell — cannot masquerade as an agent.
+ * Why sampled twice: the load that wedges the daemon is the same load that can blind
+ * the process-table read, so a single blind sample would lose the evidence exactly when
+ * it matters most. A second sample also lets a short-lived child — a resolver probe, a
+ * health-check shell — be recognized as transient rather than mistaken for an agent.
  * This runs only on the replace path, after ~60s of grace is already spent.
  */
 const PTY_OWNERSHIP_PROBE_ATTEMPTS = 2
 
+/**
+ * Windows enumeration has no budget of its own: each scan is a powershell CIM query that
+ * may fall back to wmic, and the shared reader can queue behind an in-flight scan — worst
+ * case tens of seconds on the launch path. Blind is a safe answer here; hanging is not.
+ */
+const WINDOWS_OWNERSHIP_PROBE_DEADLINE_MS = 6_000
+
+function withDeadline<T>(work: Promise<T>, deadlineMs: number, onDeadline: T): Promise<T> {
+  return new Promise<T>((resolve) => {
+    const timer = setTimeout(() => resolve(onDeadline), deadlineMs)
+    timer.unref?.()
+    void work.then(
+      (value) => {
+        clearTimeout(timer)
+        resolve(value)
+      },
+      () => {
+        clearTimeout(timer)
+        resolve(onDeadline)
+      }
+    )
+  })
+}
+
 // A wedged daemon cannot reap, so its exited PTYs linger as zombies. Counting them
 // would read "every agent already exited" as "agents still running" — and that
 // false positive is systematically correlated with the wedge we are diagnosing.
@@ -77,11 +102,12 @@ async function probeOnce(
   platform: NodeJS.Platform
 ): Promise<DaemonPtyOwnership> {
   if (platform === 'win32') {
-    const descendants = await (deps.queryWindowsDescendants ?? queryWindowsProcessDescendants)(
-      daemonPid,
-      { fresh: true }
+    const descendants = await withDeadline(
+      (deps.queryWindowsDescendants ?? queryWindowsProcessDescendants)(daemonPid, { fresh: true }),
+      deps.windowsDeadlineMs ?? WINDOWS_OWNERSHIP_PROBE_DEADLINE_MS,
+      null
     )
-    // null covers both enumeration failure and a root absent from the table.
+    // null covers enumeration failure, a root absent from the table, and our own deadline.
     if (descendants === null) {
       return 'unknown'
     }
@@ -97,10 +123,14 @@ async function probeOnce(
 }
 
 /**
- * PTY children are the daemon's only long-lived descendants, so a descendant that
- * survives two samples is positive proof that killing it would destroy running work.
- * Descendants rather than direct children: macOS wraps every shell in login(1) for
- * TCC attribution, so the agent is a grandchild at best.
+ * PTY children are the daemon's only long-lived descendants, so a live descendant is
+ * positive proof that killing it would destroy running work. Descendants rather than
+ * direct children: macOS wraps every shell in login(1) for TCC attribution, so the
+ * agent is a grandchild at best.
+ *
+ * Resolution between disagreeing samples: an observed-empty root wins (it is conclusive,
+ * and it is how a transient child is recognized), and otherwise a single sighting is
+ * enough to preserve — a blind read is not evidence against a live one.
  */
 export async function inspectDaemonPtyOwnership(
   daemonPid: number,