From a4d69d7b45db41e4eab498b261afb29ccb4464ce Mon Sep 17 00:00:00 2001
From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Date: Fri, 28 Aug 2026 07:46:57 -0700
Subject: [PATCH] test(pty): pin the unavailable arm's twin, and eight
 neighbouring refusal arms

A gate ablation found the `unavailable` arm of the completion monitor free
while its structurally identical out-of-contract twin was pinned. After an
`unavailable` sample the pending exit could stop being discarded and nothing
would notice: a pending exit would survive the gap and be confirmed by the next
positive sample, so the two "consecutive" samples that dispatch a completion
would be separated by a read that observed nothing. `unavailable` is strictly
less evidence than an out-of-contract verdict, so if either arm deserves the
re-arm it is this one. Its backoff charge was free for the same reason and is
pinned here too, or an unreachable host gets polled at the active tier forever.

Also pinned, each red with that arm reverted alone: the exit-1-is-not-absence
timed-out check on both pgrep and ps, the ps spawn-failure staying
unverifiable, `processName: null` from a completed scan reading as positive
absence, an unowned pty reading as positive absence, and a pty replaced
mid-scan staying unverifiable.

`!result.signal` in subprocessRanAndMatchedNothing is left unpinned on
purpose: Node reports `code` and `signal` mutually exclusively, so
`code === 1` with a signal is not an input production emits, and a test for it
would assert an unreachable branch.
---
 .../local-pty-process-evidence.test.ts        |  15 +++
 ...-provider-inspect-process-evidence.test.ts |  45 +++++++
 src/relay/pty-process-probes.test.ts          |  38 ++++++
 ...t-completion-unverifiable-evidence.test.ts | 116 ++++++++++++++++++
 4 files changed, 214 insertions(+)

diff --git a/src/main/providers/local-pty-process-evidence.test.ts b/src/main/providers/local-pty-process-evidence.test.ts
index 873cc41a6e0..aedd7a849d0 100644
--- a/src/main/providers/local-pty-process-evidence.test.ts
+++ b/src/main/providers/local-pty-process-evidence.test.ts
@@ -84,6 +84,21 @@ describe('classifyLocalPtyChildProcesses', () => {
     ).toEqual({ hasChildProcesses: false, evidence: { verdict: 'exited' } })
   })
 
+  it('reads a completed scan that observed nothing as positive absence', () => {
+    // The canonical completion input: the agent exited, the scan ran to
+    // completion and resolved no foreground process, and node-pty's title is
+    // back to the shell. `processName: null` is the observation, not an
+    // absent one, so this is the one shape that may say exited.
+    expect(
+      classifyLocalPtyChildProcesses({
+        procPresent: true,
+        titleRead: { ok: true, title: 'zsh' },
+        shell: 'zsh',
+        foreground: { verdict: 'observed', processName: null }
+      })
+    ).toEqual({ hasChildProcesses: false, evidence: { verdict: 'exited' } })
+  })
+
   it('reads a shell title under a degraded scan as unverifiable', () => {
     // node-pty's POSIX title read silently falls back to the spawned shell
     // when the native read fails, so under scan distress "title == shell"
diff --git a/src/main/providers/local-pty-provider-inspect-process-evidence.test.ts b/src/main/providers/local-pty-provider-inspect-process-evidence.test.ts
index deb61584eb1..a5dd3c4676f 100644
--- a/src/main/providers/local-pty-provider-inspect-process-evidence.test.ts
+++ b/src/main/providers/local-pty-provider-inspect-process-evidence.test.ts
@@ -204,6 +204,51 @@ describe('LocalPtyProvider.inspectProcess evidence', () => {
     })
   })
 
+  it('reports a pty the provider no longer owns as positive absence', async () => {
+    // The provider owns this table, so a missing entry is a reaped PTY rather
+    // than a failed probe. It has to stay a positive observation: the monitor
+    // refuses to complete on anything it cannot read, so publishing
+    // `unverifiable` here would strand the pane's last completion forever.
+    await expect(provider.inspectProcess('pty-the-provider-never-owned')).resolves.toEqual({
+      foregroundProcess: null,
+      hasChildProcesses: false,
+      processEvidence: {
+        foreground: { verdict: 'observed', processName: null },
+        children: { verdict: 'exited' }
+      }
+    })
+  })
+
+  it('marks a pty replaced mid-scan unverifiable instead of an exit', async () => {
+    const { id } = await provider.spawn({ cols: 80, rows: 24, sessionId: 'pane-1' })
+    await recognizeCodex(id)
+
+    mockProc.process = 'zsh'
+    resolveAgentForegroundProcessMock.mockImplementationOnce(async () => {
+      // The scan outlives the pane: the PTY exits and the same caller session
+      // id is reattached to a fresh node-pty while the scan is still running.
+      exitCb?.({ exitCode: 0 })
+      const replacement = createLocalPtyMockProcess({
+        get: () => exitCb,
+        set: (cb) => {
+          exitCb = cb
+        }
+      })
+      replacement.process = 'zsh'
+      spawnMock.mockReturnValue(replacement)
+      await provider.spawn({ cols: 80, rows: 24, sessionId: 'pane-1' })
+      return { available: true, processName: 'zsh' }
+    })
+
+    // The stale scan answered about a process that is no longer this pane's.
+    // Its `null` name predates the evidence contract and reads as an exit.
+    await expect(provider.inspectProcess(id)).resolves.toMatchObject({
+      processEvidence: {
+        foreground: { verdict: 'unverifiable', reason: 'pty replaced during inspection' }
+      }
+    })
+  })
+
   it('treats a successful Windows job confirmation as an observation', async () => {
     Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
     mockProc.process = 'powershell.exe'
diff --git a/src/relay/pty-process-probes.test.ts b/src/relay/pty-process-probes.test.ts
index 2be8b2d2709..0e4bd4077e3 100644
--- a/src/relay/pty-process-probes.test.ts
+++ b/src/relay/pty-process-probes.test.ts
@@ -85,6 +85,20 @@ function timedOutProbe(): ProbeProcessResult {
   return { code: null, signal: 'SIGTERM', timedOut: true }
 }
 
+/**
+ * A probe killed at its deadline that still reports a numeric exit status.
+ *
+ * runProcess sets `timedOut` from its own timer and then resolves from the
+ * child's `close`, so the deadline kill and a real exit race: on Windows the
+ * kill itself is the exit code (no signal), and on POSIX a probe that exited
+ * as the timer fired reports its own code. `1` is exactly the code pgrep/ps
+ * use for "ran and matched nothing", so without the timed-out check this shape
+ * would read as positive absence.
+ */
+function timedOutProbeReportingExitOne(): ProbeProcessResult {
+  return { code: 1, signal: null, timedOut: true }
+}
+
 /**
  * Feed the native Windows snapshot. A real snapshot always contains the
  * querying process, and the reader rejects a table without it.
@@ -355,6 +369,12 @@ describe('probeProcessChildren', () => {
     await expect(probeProcessChildren(100)).resolves.toMatchObject({ verdict: 'unverifiable' })
   })
 
+  it('keeps a timed-out pgrep unverifiable even when it reports exit 1', async () => {
+    mockRunProcess(() => timedOutProbeReportingExitOne())
+
+    await expect(probeProcessChildren(100)).resolves.toMatchObject({ verdict: 'unverifiable' })
+  })
+
   it('keeps a pgrep fatal error (exit 3) unverifiable', async () => {
     mockRunProcess(() => ({ code: 3 }))
 
@@ -436,6 +456,24 @@ describe('observeForegroundProcess', () => {
     })
   })
 
+  it('keeps a ps binary that could not be spawned unverifiable, never an observed absence', async () => {
+    mockExecFile(() => new Error('ps table unavailable'))
+    mockRunProcess(() => spawnFailure('spawn ps ENOENT', 'ENOENT'))
+
+    await expect(observeForegroundProcess(100)).resolves.toMatchObject({
+      verdict: 'unverifiable'
+    })
+  })
+
+  it('keeps a timed-out ps read unverifiable even when it reports exit 1', async () => {
+    mockExecFile(() => new Error('ps table unavailable'))
+    mockRunProcess(() => timedOutProbeReportingExitOne())
+
+    await expect(observeForegroundProcess(100)).resolves.toMatchObject({
+      verdict: 'unverifiable'
+    })
+  })
+
   it('keeps the node-pty fallback observed when descendant enrichment fails', async () => {
     mockExecFile(() => new Error('ps table unavailable'))
     mockRunProcess(() => timedOutProbe())
diff --git a/src/renderer/src/components/terminal-pane/agent-completion-unverifiable-evidence.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-unverifiable-evidence.test.ts
index a297ebd05a3..a61da8050a4 100644
--- a/src/renderer/src/components/terminal-pane/agent-completion-unverifiable-evidence.test.ts
+++ b/src/renderer/src/components/terminal-pane/agent-completion-unverifiable-evidence.test.ts
@@ -43,6 +43,14 @@ function confirmedExit(foregroundProcess: string | null): RuntimeTerminalProcess
   }
 }
 
+/** Feeds one scripted inspection per poll, repeating the last step forever. */
+function scripted(
+  steps: RuntimeTerminalProcessInspection[]
+): () => RuntimeTerminalProcessInspection {
+  let index = 0
+  return () => steps[Math.min(index++, steps.length - 1)]!
+}
+
 describe('agent completion with inspection evidence', () => {
   useAgentCompletionCoordinatorLifecycle()
 
@@ -153,6 +161,114 @@ describe('agent completion with inspection evidence', () => {
     expect(dispatchCompletion).not.toHaveBeenCalled()
   })
 
+  it('re-arms the two-sample exit confirmation after an unverifiable sample', async () => {
+    // Completion needs two consecutive positively-exited samples. The
+    // unverifiable sample in the middle observed nothing, so it cannot stand
+    // in for either half of the pair: the pending exit it interrupts has to be
+    // discarded, not carried across the gap to be confirmed by the next one.
+    const { dispatchCompletion } = startCoordinator(
+      scripted([
+        processResult('codex'),
+        confirmedExit('zsh'),
+        unverifiableChildren('zsh'),
+        confirmedExit('zsh'),
+        unverifiableChildren('zsh')
+      ])
+    )
+
+    await vi.advanceTimersByTimeAsync(120_000)
+
+    expect(dispatchCompletion).not.toHaveBeenCalled()
+  })
+
+  it('re-arms the two-sample exit confirmation after an unavailable sample too', async () => {
+    // The twin of the case above, on the other arm. `unavailable` is the
+    // transport saying it could not reach the host at all — strictly less
+    // evidence than an out-of-contract verdict — so it must discard a pending
+    // exit for the same reason. Reverting either arm's re-arm alone has to go
+    // red, or one of the two is only held up by the other.
+    const { dispatchCompletion } = startCoordinator(
+      scripted([
+        processResult('codex'),
+        confirmedExit('zsh'),
+        { foregroundProcess: null, hasChildProcesses: false, unavailable: true },
+        confirmedExit('zsh'),
+        { foregroundProcess: null, hasChildProcesses: false, unavailable: true }
+      ])
+    )
+
+    await vi.advanceTimersByTimeAsync(120_000)
+
+    expect(dispatchCompletion).not.toHaveBeenCalled()
+  })
+
+  it('confirms the exit from two consecutive positively-exited samples', async () => {
+    // The control for the re-arm above: the same script without the
+    // unverifiable sample in the middle must complete, so the refusal there is
+    // the interruption and not the sequence length.
+    const { dispatchCompletion } = startCoordinator(
+      scripted([
+        processResult('codex'),
+        confirmedExit('zsh'),
+        confirmedExit('zsh'),
+        unverifiableChildren('zsh')
+      ])
+    )
+
+    await vi.advanceTimersByTimeAsync(120_000)
+
+    expect(dispatchCompletion).toHaveBeenCalledTimes(1)
+    expect(dispatchCompletion).toHaveBeenCalledWith('codex', {
+      source: 'process-exit',
+      quietedHookDone: false,
+      terminalIdleConfirmed: true
+    })
+  })
+
+  it('keeps charging inspection errors while the host stays unverifiable', async () => {
+    // The error count drives the poll backoff, and an unverifiable sample was
+    // not a readable inspection: it must not reset the count on its way to
+    // incrementing it, or the backoff stays pinned at a single error and the
+    // client keeps hammering a host that has already said it cannot answer.
+    const inspectedAt: number[] = []
+    const next = scripted([processResult('codex'), unverifiableChildren('zsh')])
+    const { dispatchCompletion } = startCoordinator(() => {
+      inspectedAt.push(Date.now())
+      return next()
+    })
+
+    await vi.advanceTimersByTimeAsync(120_000)
+
+    expect(dispatchCompletion).not.toHaveBeenCalled()
+    expect(inspectedAt.length).toBeGreaterThan(4)
+    // The active tier polls every 750ms; a run of unverifiable samples has to
+    // walk the backoff all the way up to its 10s ceiling.
+    expect(inspectedAt.at(-1)! - inspectedAt.at(-2)!).toBeGreaterThanOrEqual(10_000)
+  })
+
+  it('keeps charging inspection errors while the host stays unavailable too', async () => {
+    // The twin of the case above on the other arm. `unavailable` is the
+    // transport failing to reach the host at all, so it is even less of a
+    // readable inspection than an out-of-contract verdict: it has to walk the
+    // same backoff, or the client polls an unreachable host at the active tier
+    // forever.
+    const inspectedAt: number[] = []
+    const next = scripted([
+      processResult('codex'),
+      { foregroundProcess: null, hasChildProcesses: false, unavailable: true }
+    ])
+    const { dispatchCompletion } = startCoordinator(() => {
+      inspectedAt.push(Date.now())
+      return next()
+    })
+
+    await vi.advanceTimersByTimeAsync(120_000)
+
+    expect(dispatchCompletion).not.toHaveBeenCalled()
+    expect(inspectedAt.length).toBeGreaterThan(4)
+    expect(inspectedAt.at(-1)! - inspectedAt.at(-2)!).toBeGreaterThanOrEqual(10_000)
+  })
+
   it('still completes from legacy fields when the host predates evidence', async () => {
     // An old host publishes no processEvidence; its legacy answer remains the
     // only available reading and completion detection must keep working.