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.