From 5484987bcac5bde093519d0cfe5fa60c9b8dc690 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 27 Aug 2026 20:58:53 -0700 Subject: [PATCH 01/10] Add failing repro: daemon terminal-host inspection coerces degraded reads into exit evidence --- ...ction-unverifiable-completion.unit.test.ts | 373 ++++++++++++++++++ 1 file changed, 373 insertions(+) create mode 100644 tests/e2e/daemon-completion-evidence/daemon-inspection-unverifiable-completion.unit.test.ts diff --git a/tests/e2e/daemon-completion-evidence/daemon-inspection-unverifiable-completion.unit.test.ts b/tests/e2e/daemon-completion-evidence/daemon-inspection-unverifiable-completion.unit.test.ts new file mode 100644 index 00000000000..8acec1d5820 --- /dev/null +++ b/tests/e2e/daemon-completion-evidence/daemon-inspection-unverifiable-completion.unit.test.ts @@ -0,0 +1,373 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import type * as pty from 'node-pty' + +const { execFileMock, execFileSyncMock, killWithDescendantSweepMock, forceKillPosixMock } = + vi.hoisted(() => ({ + execFileMock: vi.fn(), + execFileSyncMock: vi.fn(), + killWithDescendantSweepMock: vi.fn(), + forceKillPosixMock: vi.fn() + })) + +// The process-table snapshot (`ps -axo`) reads through node:child_process +// execFile; the foreground resolver and tracker stay REAL so this suite +// exercises the production degraded-read handling, not a stub of it. +vi.mock('child_process', () => ({ + execFile: execFileMock, + execFileSync: execFileSyncMock +})) + +vi.mock('../../../src/main/pty-descendant-termination', () => ({ + killWithDescendantSweep: killWithDescendantSweepMock +})) + +// Teardown escalation must never signal a real process group from a test. +vi.mock('../../../src/main/pty/posix-pty-process-groups', () => ({ + forceKillPosixPtyProcessGroups: forceKillPosixMock +})) + +import { TerminalHost } from '../../../src/main/daemon/terminal-host' +import { createDaemonPtySubprocessHandle } from '../../../src/main/daemon/pty-subprocess/subprocess-handle' +import { DaemonPtyAdapter } from '../../../src/main/daemon/daemon-pty-adapter' +import { inspectPtyProviderProcessForRenderer } from '../../../src/main/providers/pty-process-inspection' +import { resetProcessTableSnapshotForTests } from '../../../src/shared/process-table-snapshot' +import { + createAgentCompletionCoordinator, + resetAgentCompletionCoordinatorIdentitiesForTest +} from '../../../src/renderer/src/components/terminal-pane/agent-completion-coordinator' +import { resetAgentProcessInspectionQueueForTests } from '../../../src/renderer/src/components/terminal-pane/agent-process-inspection-queue' +import type { RuntimeTerminalProcessInspection } from '../../../src/renderer/src/runtime/runtime-terminal-inspection' + +/** + * "Failure becomes fact" coercion, DAEMON terminal-host inspection site — the + * default macOS path: local panes are daemon-hosted, so the completion + * monitor's inspectProcess lands in TerminalHost.inspectProcess via + * DaemonPtyProcessInspection. + * + * The daemon's synchronous foreground read degrades in ways that used to be + * indistinguishable from an observation: node-pty's POSIX title read silently + * falls back to the spawned shell file when the native read fails, and the + * async identity scan behind the 1s cache can be degraded (truncated table / + * failed ps fork) so the cached agent identity goes stale without ever being + * disproven. Both used to collapse into `{ foregroundProcess: , + * hasChildProcesses: false }` — the exact payload the agent-completion monitor + * reads as positive exit evidence. The truth is "could not ask", which must + * stay `unverifiable` and never become a completion + * (docs/reference/ssh-execution-boundary.md). Relay leg and local-provider leg + * fixed by the two parent changes; this is the daemon leg. + */ +describe('daemon terminal-host inspection under degraded reads', () => { + // Implausibly high so a leaked signal can never hit a real process. + const SHELL_PID = 999_999_242 + // Minted worktree ids are `${repoId}::${path}`; the parser rejects anything else. + const WORKTREE_ID = 'repo-daemon-evidence::/tmp/wt' + const SESSION_ID = `${WORKTREE_ID}@@repro001` + + type PsBehavior = 'healthy' | 'table-missing-pane' | 'ps-failure' | 'agent-exited' + let psBehavior: PsBehavior = 'healthy' + + function timeoutKilledError(command: string): Error { + const error = new Error(`spawn ${command} ETIMEDOUT`) as Error & { + killed: boolean + signal: string + code: null + } + error.killed = true + error.signal = 'SIGTERM' + error.code = null + return error + } + + function installExecFile(): void { + execFileMock.mockImplementation( + (command: string, args: string[], _opts: unknown, cb: unknown) => { + const callback = cb as ( + err: Error | null, + result: { stdout: string; stderr: string } + ) => void + if (command !== 'ps' || args[0] !== '-axo') { + callback(new Error(`unexpected command ${command}`), { stdout: '', stderr: '' }) + return + } + switch (psBehavior) { + case 'healthy': + callback(null, { + stdout: [ + `${SHELL_PID} 1 Ss -zsh`, + `999999555 ${SHELL_PID} S+ node /home/dev/.local/bin/codex` + ].join('\n'), + stderr: '' + }) + return + case 'table-missing-pane': + // The scan "succeeded" but the table never contained the pane's + // shell or its descendants. + callback(null, { stdout: '1 0 Ss /sbin/launchd', stderr: '' }) + return + case 'ps-failure': + callback(timeoutKilledError(command), { stdout: '', stderr: '' }) + return + case 'agent-exited': + callback(null, { stdout: `${SHELL_PID} 1 Ss+ -zsh`, stderr: '' }) + } + } + ) + } + + type MockNodePty = pty.IPty & { + process: string + _fireExit: (exitCode: number) => void + } + + function createMockNodePtyProcess(): MockNodePty { + const exitListeners: ((e: { exitCode: number; signal?: number }) => void)[] = [] + const proc = { + pid: SHELL_PID, + cols: 80, + rows: 24, + // node-pty reports the wrapper entrypoint for node-launched agents; the + // daemon's descendant scan resolves it to the real agent. + process: 'node', + handleFlowControl: false, + onData: vi.fn(() => ({ dispose: vi.fn() })), + onExit: vi.fn((cb: (e: { exitCode: number; signal?: number }) => void) => { + exitListeners.push(cb) + return { dispose: vi.fn() } + }), + write: vi.fn(), + resize: vi.fn(), + clear: vi.fn(), + pause: vi.fn(), + resume: vi.fn(), + kill: vi.fn(() => { + setTimeout(() => proc._fireExit(0), 5) + }), + _fireExit: (exitCode: number) => { + exitListeners.forEach((cb) => cb({ exitCode, signal: 0 })) + } + } + return proc as unknown as MockNodePty + } + + let platformDescriptor: PropertyDescriptor | undefined + let host: TerminalHost + let adapter: DaemonPtyAdapter + let mockProc: MockNodePty + + beforeEach(async () => { + vi.useFakeTimers() + vi.spyOn(Math, 'random').mockReturnValue(0.5) + // The default macOS topology: a daemon-hosted local pane. + platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' }) + psBehavior = 'healthy' + execFileMock.mockReset() + installExecFile() + killWithDescendantSweepMock.mockReset() + killWithDescendantSweepMock.mockResolvedValue(undefined) + forceKillPosixMock.mockReset() + resetProcessTableSnapshotForTests() + + mockProc = createMockNodePtyProcess() + // Teardown escalation must still produce a physical exit for the session. + forceKillPosixMock.mockImplementation(() => { + setTimeout(() => mockProc._fireExit(137), 1) + }) + host = new TerminalHost({ + // The REAL daemon subprocess handle (and with it the real foreground + // tracker) over a scripted node-pty process. + spawnSubprocess: (opts) => + createDaemonPtySubprocessHandle({ + process: mockProc, + shellPath: '/bin/zsh', + spawnCwd: '/tmp/wt', + env: { PATH: '/usr/bin' }, + startupCommandDeliveredInShellArgs: false, + reportsChildExitStatus: true, + requestedCwd: '/tmp/wt', + sessionId: opts.sessionId, + startupAgentRecognition: null + }) + }) + await host.createOrAttach({ + sessionId: SESSION_ID, + cols: 80, + rows: 24, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + + // The REAL app-side client leg: DaemonPtyAdapter.inspectProcess with its + // socket call routed straight to the host. The daemon-request-router's + // 'inspectProcess' / 'listSessions' cases are one-line delegations to + // TerminalHost, so this preserves every production code path around the + // wire itself (which the sibling wire test covers over a real socket). + adapter = new DaemonPtyAdapter({ + socketPath: join(tmpdir(), 'daemon-evidence-unused.sock'), + tokenPath: join(tmpdir(), 'daemon-evidence-unused.token') + }) + type ClientInternals = { + client: { + request: (type: string, payload: unknown) => Promise + disconnect: () => void + ensureConnected: () => Promise + ensureConnectedWithin: (ms: number) => Promise + getDaemonIdentity: () => null + onEvent: (cb: (raw: unknown) => void) => () => void + } + } + ;(adapter as unknown as ClientInternals).client = { + request: async (type: string, payload: unknown) => { + if (type === 'inspectProcess') { + return host.inspectProcess((payload as { sessionId: string }).sessionId) + } + if (type === 'listSessions') { + return { sessions: host.listSessions() } + } + throw new Error(`unexpected daemon request: ${type}`) + }, + disconnect: () => {}, + ensureConnected: async () => {}, + ensureConnectedWithin: async () => {}, + getDaemonIdentity: () => null, + onEvent: () => () => {} + } + // The production adoption flow that teaches the adapter about + // daemon-surviving sessions (and makes hasPty answer for them). + await adapter.reconcileOnStartup(new Set([WORKTREE_ID])) + }) + + afterEach(async () => { + resetAgentProcessInspectionQueueForTests() + resetAgentCompletionCoordinatorIdentitiesForTest() + resetProcessTableSnapshotForTests() + adapter?.dispose() + vi.useRealTimers() + await host?.dispose() + if (platformDescriptor) { + Object.defineProperty(process, 'platform', platformDescriptor) + } + vi.restoreAllMocks() + }) + + function createCoordinator(paneKey: string, dispatchCompletion: ReturnType) { + return createAgentCompletionCoordinator({ + paneKey, + getPtyId: () => SESSION_ID, + getSettings: () => null, + // Real production adapter chain for a daemon-hosted pane: the renderer's + // window.api.pty.inspectProcess lands in the pty IPC handler, which + // calls exactly this. + inspectProcess: async (_settings, ptyId) => + (await inspectPtyProviderProcessForRenderer( + adapter, + ptyId + )) as RuntimeTerminalProcessInspection, + dispatchCompletion, + isLive: () => true + }) + } + + it('does not conclude an agent finished when the daemon reads degrade', async () => { + const dispatchCompletion = vi.fn() + const coordinator = createCoordinator('tab-1:leaf-1', dispatchCompletion) + + coordinator.startProcessTracking() + coordinator.observeTitle('Codex working') + + // Healthy polls: the first poll's identity scan proves codex is live in + // this pane; the next poll surfaces it through the wrapper-hold. + await vi.advanceTimersByTimeAsync(5_000) + expect(dispatchCompletion).not.toHaveBeenCalled() + + // Distress: the cached table stops containing the pane's subtree, and + // node-pty's native title read degrades to the spawn file (the shell) — + // its documented fallback. Codex is still running; nothing observed it. + psBehavior = 'table-missing-pane' + mockProc.process = 'zsh' + await vi.advanceTimersByTimeAsync(4_000) + + // Full distress: the ps fork now fails outright. + psBehavior = 'ps-failure' + await vi.advanceTimersByTimeAsync(20_000) + + expect(dispatchCompletion).not.toHaveBeenCalled() + + coordinator.dispose() + }) + + it('still confirms a real exit once the table answers again', async () => { + const dispatchCompletion = vi.fn() + const coordinator = createCoordinator('tab-2:leaf-1', dispatchCompletion) + + coordinator.startProcessTracking() + coordinator.observeTitle('Codex working') + await vi.advanceTimersByTimeAsync(5_000) + + psBehavior = 'ps-failure' + mockProc.process = 'zsh' + await vi.advanceTimersByTimeAsync(10_000) + expect(dispatchCompletion).not.toHaveBeenCalled() + + // Recovery, and the agent has genuinely exited: the shell holds the + // foreground again and the table positively shows no descendants. + psBehavior = 'agent-exited' + await vi.advanceTimersByTimeAsync(30_000) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchCompletion).toHaveBeenCalledWith('codex', { + source: 'process-exit', + quietedHookDone: false, + terminalIdleConfirmed: true + }) + + coordinator.dispose() + }) + + it('publishes unchanged legacy fields beside the evidence', async () => { + // Warm the identity: a healthy scan resolves the node wrapper to codex. + await inspectPtyProviderProcessForRenderer(adapter, SESSION_ID) + await vi.advanceTimersByTimeAsync(600) + const healthy = await inspectPtyProviderProcessForRenderer(adapter, SESSION_ID) + expect(healthy).toEqual({ + foregroundProcess: 'codex', + hasChildProcesses: true, + processEvidence: { + foreground: { verdict: 'observed', processName: 'codex' }, + children: { verdict: 'live' } + } + }) + + // Degraded scan with a shell-fallback title: the legacy fields keep the + // exact pre-evidence collapse; the evidence says "could not ask". + psBehavior = 'ps-failure' + mockProc.process = 'zsh' + await vi.advanceTimersByTimeAsync(2_000) + const degraded = await inspectPtyProviderProcessForRenderer(adapter, SESSION_ID) + expect(degraded).toEqual({ + foregroundProcess: 'zsh', + hasChildProcesses: false, + processEvidence: { + foreground: { verdict: 'unverifiable', reason: expect.any(String) }, + children: { verdict: 'unverifiable', reason: expect.any(String) } + } + }) + + // A real exit is a real observation: legacy and evidence agree. The first + // read schedules the corroborating scan; the second observes its verdict. + psBehavior = 'agent-exited' + await vi.advanceTimersByTimeAsync(2_000) + await inspectPtyProviderProcessForRenderer(adapter, SESSION_ID) + await vi.advanceTimersByTimeAsync(1_500) + const exited = await inspectPtyProviderProcessForRenderer(adapter, SESSION_ID) + expect(exited).toEqual({ + foregroundProcess: 'zsh', + hasChildProcesses: false, + processEvidence: { + foreground: { verdict: 'observed', processName: 'zsh' }, + children: { verdict: 'exited' } + } + }) + }) +}) From ab9874cb605a756e9c236b804d79e1f44c179c94 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 01:13:18 -0700 Subject: [PATCH 02/10] fix(daemon): a degraded terminal-host read is unverifiable, never an agent exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daemon terminal-host's inspectProcess collapsed every foreground read into `{ foregroundProcess, hasChildProcesses }`, so a read that observed nothing — node-pty's POSIX title read silently falling back to the spawned shell, or a degraded/failed `ps` identity scan letting the cached agent identity go stale — produced the same payload as a genuinely idle shell. The agent-completion monitor reads that payload as positive exit evidence and reports a still-running agent as finished. The host now reports per-probe live/unverifiable/exited evidence beside byte-identical legacy fields (docs/reference/ssh-execution-boundary.md), the same additive `processEvidence` contract the relay and local-provider legs use. A shell-shaped title is an observation only while a completed scan corroborates it; a scan that could not answer — unavailable, table missing the pane, or thrown — settles as unverifiable and withdraws that corroboration. `SubprocessHandle.observeForegroundProcess` is optional and a handle that omits it reads as `unverifiable`, so the unknown case falls toward the conservative arm rather than toward a synthesized observation. --- .../daemon/daemon-pty-process-inspection.ts | 7 + ...ss-foreground-observation-evidence.test.ts | 128 ++++++++ .../foreground-identity-refresh.ts | 230 ++++++++++++++ .../foreground-process-tracker.ts | 297 ++++++------------ .../pty-subprocess/subprocess-handle.ts | 1 + src/main/daemon/session-subprocess-handle.ts | 33 ++ src/main/daemon/session.ts | 7 +- ...al-host-inspection-degraded-handle.test.ts | 81 +++++ .../terminal-host-process-evidence.test.ts | 69 ++++ .../daemon/terminal-host-process-evidence.ts | 42 +++ src/main/daemon/terminal-host.ts | 18 +- ...emon-inspection-evidence-wire.unit.test.ts | 157 +++++++++ 12 files changed, 858 insertions(+), 212 deletions(-) create mode 100644 src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts create mode 100644 src/main/daemon/pty-subprocess/foreground-identity-refresh.ts create mode 100644 src/main/daemon/terminal-host-inspection-degraded-handle.test.ts create mode 100644 src/main/daemon/terminal-host-process-evidence.test.ts create mode 100644 src/main/daemon/terminal-host-process-evidence.ts create mode 100644 tests/e2e/daemon-completion-evidence/daemon-inspection-evidence-wire.unit.test.ts diff --git a/src/main/daemon/daemon-pty-process-inspection.ts b/src/main/daemon/daemon-pty-process-inspection.ts index bbc44568501..00f3757153e 100644 --- a/src/main/daemon/daemon-pty-process-inspection.ts +++ b/src/main/daemon/daemon-pty-process-inspection.ts @@ -8,6 +8,7 @@ import { type SessionInfo } from './types' import type { PtyProcessInspection } from '../providers/pty-process-inspection' +import type { PtyProcessInspectionEvidence } from '../../shared/pty-process-inspection-evidence' export abstract class DaemonPtyProcessInspection extends DaemonPtyBufferSnapshots { // Why: daemon-backed PTYs can host long-lived agents while detached; cleanup prompts must not treat them as idle shells. @@ -42,6 +43,12 @@ export abstract class DaemonPtyProcessInspection extends DaemonPtyBufferSnapshot return this.client.request<{ foregroundProcess: string | null hasChildProcesses: boolean + // Why optional: daemons survive in-place app updates, so a pre-evidence + // daemon omits the field and readers fall back to the legacy collapse + // (readPtyProcessInspectionEvidence). The composed pre-v27 result above + // stays evidence-less for the same reason: an old daemon's published + // values are the only answer it can give. + processEvidence?: PtyProcessInspectionEvidence }>('inspectProcess', { sessionId: id }) } diff --git a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts new file mode 100644 index 00000000000..222b862c867 --- /dev/null +++ b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts @@ -0,0 +1,128 @@ +// The daemon's synchronous foreground read cannot tell node-pty's silent +// shell-title fallback from a real idle shell on its own, so a shell-shaped +// title is only an observation while a completed scan corroborates it. Pins +// how each scan outcome settles (docs/reference/ssh-execution-boundary.md). +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as pty from 'node-pty' + +const resolveAgentForegroundProcessMock = vi.hoisted(() => vi.fn()) +vi.mock('../providers/agent-foreground-process', () => ({ + resolveAgentForegroundProcessWithAvailability: (...args: unknown[]) => + resolveAgentForegroundProcessMock(...args) +})) + +import { createDaemonPtySubprocessHandle } from './pty-subprocess/subprocess-handle' +import type { SubprocessHandle } from './session-subprocess-handle' + +const SHELL_PID = 999_999_517 +// Above the idle-shell refresh throttle (5s) so each read starts a fresh scan. +const PAST_THE_SCAN_THROTTLE_MS = 6_000 + +describe('daemon foreground observation evidence', () => { + let platformDescriptor: PropertyDescriptor | undefined + let nodePty: pty.IPty & { process: string } + let handle: SubprocessHandle + + async function readAfterSettledScan(): Promise + > | null> { + // A read schedules the next scan rather than awaiting one, and the + // throttle can swallow the read that follows a scan. Two cycles guarantee + // one scan both started and settled under the behavior set by this test. + for (let cycle = 0; cycle < 2; cycle++) { + handle.observeForegroundProcess?.() + await vi.advanceTimersByTimeAsync(PAST_THE_SCAN_THROTTLE_MS) + } + return handle.observeForegroundProcess?.() ?? null + } + + beforeEach(() => { + vi.useFakeTimers() + platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' }) + resolveAgentForegroundProcessMock.mockReset() + // Default: a scan that runs and finds no agent. Individual tests override. + resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'zsh' }) + nodePty = { + pid: SHELL_PID, + process: 'zsh', + onData: vi.fn(() => ({ dispose: vi.fn() })), + onExit: vi.fn(() => ({ dispose: vi.fn() })), + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn() + } as unknown as pty.IPty & { process: string } + handle = createDaemonPtySubprocessHandle({ + process: nodePty, + shellPath: '/bin/zsh', + spawnCwd: '/tmp/wt', + env: { PATH: '/usr/bin' }, + startupCommandDeliveredInShellArgs: false, + reportsChildExitStatus: true, + requestedCwd: '/tmp/wt', + sessionId: 'repo-observe::/tmp/wt@@observe01', + startupAgentRecognition: null + }) + }) + + afterEach(() => { + vi.useRealTimers() + if (platformDescriptor) { + Object.defineProperty(process, 'platform', platformDescriptor) + } + vi.restoreAllMocks() + }) + + it('withholds observation from a shell title no scan has corroborated', () => { + const observation = handle.observeForegroundProcess?.() + + expect(observation?.processName).toBe('zsh') + expect(observation?.evidence.verdict).toBe('unverifiable') + }) + + it('observes the shell once a completed scan agrees the pane is idle', async () => { + resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'zsh' }) + + const observation = await readAfterSettledScan() + + expect(observation?.processName).toBe('zsh') + expect(observation?.evidence).toEqual({ verdict: 'observed', processName: 'zsh' }) + }) + + it('withholds observation when the scan ran but could not answer', async () => { + resolveAgentForegroundProcessMock.mockResolvedValue({ available: false, processName: 'zsh' }) + + const observation = await readAfterSettledScan() + + expect(observation?.evidence.verdict).toBe('unverifiable') + }) + + it('withholds observation after a corroborating scan is followed by a thrown one', async () => { + resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'zsh' }) + expect((await readAfterSettledScan())?.evidence.verdict).toBe('observed') + + // A scan that rejects observed nothing; the corroboration it would have + // refreshed must not be inherited from the last one that succeeded. + resolveAgentForegroundProcessMock.mockRejectedValue(new Error('ps fork failed')) + const observation = await readAfterSettledScan() + + expect(observation?.evidence.verdict).toBe('unverifiable') + }) + + it('keeps a live agent title an observation without needing a scan', () => { + nodePty.process = 'codex' + + expect(handle.observeForegroundProcess?.()?.evidence).toEqual({ + verdict: 'observed', + processName: 'codex' + }) + }) + + it('leaves the legacy foreground read identical to the observed name', async () => { + resolveAgentForegroundProcessMock.mockResolvedValue({ available: false, processName: 'zsh' }) + await readAfterSettledScan() + + // The wire's legacy field must not change shape when evidence degrades. + expect(handle.getForegroundProcess()).toBe('zsh') + }) +}) diff --git a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts new file mode 100644 index 00000000000..daa93c372d9 --- /dev/null +++ b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts @@ -0,0 +1,230 @@ +import type * as pty from 'node-pty' +import { resolveAgentForegroundProcessWithAvailability } from '../../providers/agent-foreground-process' +import { + judgeCachedAgentJobEvidence, + WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS +} from '../../providers/windows-cached-agent-revalidation' +import { + isWindowsPtyJobReadable, + readWindowsPtyJobProcessIds +} from '../../providers/windows-pty-job-membership' +import { + isAgentForegroundWrapperProcess, + recognizeAgentProcess +} from '../../../shared/agent-process-recognition' +import { shouldInspectOuterWrapperForegroundProcess } from '../../../shared/foreground-wrapper-agent' +import { isShellProcess } from '../../../shared/shell-process-detection' + +export const FOREGROUND_AGENT_CACHE_TTL_MS = 1000 +const SHELL_FOREGROUND_REFRESH_RETRY_MS = 5_000 +const WINDOWS_IDLE_SHELL_FOREGROUND_REFRESH_RETRY_MS = 15_000 +const SHELL_FOREGROUND_OUTPUT_HOT_WINDOW_MS = 10_000 +// Why 30s: comfortably above the slowest idle refresh cadence (15s on Windows) +// so a healthy idle pane always holds corroboration, while a pane whose scans +// stopped settling loses shell-exit authority within seconds. +const SHELL_TITLE_SCAN_CORROBORATION_MAX_AGE_MS = 30_000 + +// `pid` anchors the identity to the row that proved it (null when ambiguous). +export type CachedAgentForeground = { + processName: string + pid: number | null + refreshedAt: number +} + +/** Outcome of the most recently settled identity scan. `sawAgent` is only + * meaningful when `available` — an unavailable scan observed nothing. */ +export type ForegroundScanSettlement = { at: number; available: boolean; sawAgent: boolean } + +export type ForegroundIdentityState = { + cachedAgentForeground: CachedAgentForeground | null + startupAgentForeground: { processName: string; expiresAt: number } | null + lastScanSettlement: ForegroundScanSettlement | null + refreshInFlight: boolean + lastRefreshStartedAt: number + lastOutputAt: number +} + +export function getActiveStartupAgent( + state: ForegroundIdentityState, + now = Date.now() +): { processName: string; expiresAt: number } | null { + if (!state.startupAgentForeground) { + return null + } + if (now > state.startupAgentForeground.expiresAt) { + state.startupAgentForeground = null + return null + } + return state.startupAgentForeground +} + +/** + * A shell-shaped title is exit evidence only when a completed scan agrees the + * pane is idle. node-pty's POSIX title read silently falls back to the spawned + * shell file when the native read fails, so under the same distress that + * degrades the scan, "title == shell" observes nothing — and a scan that last + * saw the agent cannot corroborate its absence either. + */ +export function isShellTitleCorroborated( + settlement: ForegroundScanSettlement | null, + now: number +): boolean { + return ( + settlement !== null && + settlement.available && + !settlement.sawAgent && + now - settlement.at <= SHELL_TITLE_SCAN_CORROBORATION_MAX_AGE_MS + ) +} + +/** + * The throttled async identity scan behind the tracker's synchronous reads: + * resolves the pane's real foreground agent through the process table, keeps + * or retires the cached identity, and records how its last scan settled so + * the sync read can tell an observed idle shell from a degraded read. + */ +export function createForegroundIdentityRefresh(args: { + process: pty.IPty + state: ForegroundIdentityState + contextPaths: string[] + isDead: () => boolean + getFallbackProcess: () => string | null + shouldInspectFallback: (fallbackProcess: string | null) => boolean +}): (fallbackProcess: string | null) => void { + const proc = args.process + const state = args.state + return (fallbackProcess) => { + if (args.isDead() || !proc.pid) { + return + } + const fallbackIsShell = fallbackProcess !== null && isShellProcess(fallbackProcess) + const fallbackRecognition = recognizeAgentProcess(fallbackProcess) + if ( + !fallbackProcess || + (fallbackRecognition !== null && + !shouldInspectOuterWrapperForegroundProcess(fallbackRecognition)) || + !args.shouldInspectFallback(fallbackProcess) + ) { + return + } + const now = Date.now() + const idleNoEvidenceShell = + fallbackIsShell && !getActiveStartupAgent(state, now) && !state.cachedAgentForeground + const retryMs = !idleNoEvidenceShell + ? FOREGROUND_AGENT_CACHE_TTL_MS + : process.platform === 'win32' && + now - state.lastOutputAt > SHELL_FOREGROUND_OUTPUT_HOT_WINDOW_MS + ? WINDOWS_IDLE_SHELL_FOREGROUND_REFRESH_RETRY_MS + : SHELL_FOREGROUND_REFRESH_RETRY_MS + if (state.refreshInFlight || now - state.lastRefreshStartedAt < retryMs) { + return + } + state.refreshInFlight = true + state.lastRefreshStartedAt = now + const identityOlderThan = (ms: number): boolean => + state.cachedAgentForeground !== null && + Date.now() - state.cachedAgentForeground.refreshedAt > ms + const retireStaleForegroundIdentity = ({ onlyWhenAged = false } = {}): void => { + const currentFallbackProcess = args.getFallbackProcess() + if ( + fallbackIsShell && + !getActiveStartupAgent(state) && + currentFallbackProcess !== null && + isShellProcess(currentFallbackProcess) && + (!onlyWhenAged || identityOlderThan(WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS)) + ) { + state.cachedAgentForeground = null + state.startupAgentForeground = null + } else if ( + identityOlderThan(FOREGROUND_AGENT_CACHE_TTL_MS) && + currentFallbackProcess !== null && + isAgentForegroundWrapperProcess(currentFallbackProcess) + ) { + state.cachedAgentForeground = null + } + } + const anchor = state.cachedAgentForeground + void resolveAgentForegroundProcessWithAvailability(proc.pid, fallbackProcess, { + contextPaths: args.contextPaths, + ...(anchor?.pid != null + ? { anchorProcessId: anchor.pid, anchorProcessName: anchor.processName } + : {}) + }) + .then(({ processName, processId, available, anchorPidForeign }) => { + state.lastScanSettlement = { + at: Date.now(), + available, + sawAgent: available && recognizeAgentProcess(processName) !== null + } + if (args.isDead() || !available) { + return + } + if (!processName || !recognizeAgentProcess(processName)) { + if ( + process.platform === 'win32' && + fallbackIsShell && + state.cachedAgentForeground !== null + ) { + // Job, not console: needs no console attachment, so no fork (#10857). + const verdict = judgeCachedAgentJobEvidence({ + jobProcessIds: readWindowsPtyJobProcessIds(proc), + jobSupported: isWindowsPtyJobReadable(), + shellPid: proc.pid, + anchorProcessId: state.cachedAgentForeground.pid, + identityAgeMs: Date.now() - state.cachedAgentForeground.refreshedAt + }) + // Unverifiable is never exit proof (ssh-execution-boundary.md): hold. + if (verdict === 'unavailable') { + return + } + if (verdict === 'unsupported') { + // No job to consult on this build, and the scan that got here was + // available and found no agent. Trust it, as every other platform + // does, rather than holding a dead name forever (#16059). + retireStaleForegroundIdentity() + return + } + if (verdict === 'confirmed' || verdict === 'recheck') { + if (anchorPidForeign === true) { + // The scan proved the pid recycled to a non-agent: retire now. + retireStaleForegroundIdentity() + return + } + // The anchor pid is still in the job: the scan lost the row, not + // the agent. Restamp so a live agent never ages out (#9258). + state.cachedAgentForeground = { + ...state.cachedAgentForeground, + refreshedAt: Date.now() + } + return + } + if (verdict === 'exited' || verdict === 'anchor-exited') { + // Safe mid-restart: an available scan already found no agent. + retireStaleForegroundIdentity() + return + } + // Unanchored superset evidence cannot tell a working agent from a + // leftover; the age bound settles it. + retireStaleForegroundIdentity({ onlyWhenAged: true }) + return + } + retireStaleForegroundIdentity() + return + } + state.cachedAgentForeground = { + processName, + pid: processId ?? null, + refreshedAt: Date.now() + } + state.startupAgentForeground = null + return processName + }) + .catch(() => { + // Best-effort only: foreground enrichment must never affect PTY health. + state.lastScanSettlement = { at: Date.now(), available: false, sawAgent: false } + }) + .finally(() => { + state.refreshInFlight = false + }) + } +} diff --git a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts index 115b795202d..c47340e5ae4 100644 --- a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts +++ b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts @@ -2,15 +2,6 @@ import type * as pty from 'node-pty' import { getAgentForegroundContextPaths } from '../../providers/agent-foreground-context-paths' import { resolveAgentForegroundProcessWithAvailability } from '../../providers/agent-foreground-process' import { confirmPtyShellForeground } from './pty-shell-foreground-confirmation' -import { - judgeCachedAgentJobEvidence, - WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS -} from '../../providers/windows-cached-agent-revalidation' -import { - isWindowsPtyJobReadable, - readWindowsPtyJobProcessIds -} from '../../providers/windows-pty-job-membership' - import { readWindowsConsoleAttachedProcessIds } from '../../providers/windows-console-attached-processes' import { isAgentForegroundWrapperProcess, @@ -24,19 +15,22 @@ import { import { isShellProcess } from '../../../shared/shell-process-detection' import { resolveFallbackForegroundProcess } from './foreground-fallback-process' import { parsePtySessionId } from '../pty-session-id' +import type { ForegroundProcessObservation } from '../session-subprocess-handle' +import { + createForegroundIdentityRefresh, + FOREGROUND_AGENT_CACHE_TTL_MS, + getActiveStartupAgent, + isShellTitleCorroborated, + type ForegroundIdentityState +} from './foreground-identity-refresh' -const FOREGROUND_AGENT_CACHE_TTL_MS = 1000 -const SHELL_FOREGROUND_REFRESH_RETRY_MS = 5_000 -const WINDOWS_IDLE_SHELL_FOREGROUND_REFRESH_RETRY_MS = 15_000 -const SHELL_FOREGROUND_OUTPUT_HOT_WINDOW_MS = 10_000 const STARTUP_AGENT_FOREGROUND_BOOTSTRAP_MS = 5_000 -type CachedAgentForeground = { processName: string; pid: number | null; refreshedAt: number } - export type PtyForegroundProcessTracker = { recordOutput(data: string): void markDead(): void getForegroundProcess(): string | null + observeForegroundProcess(): ForegroundProcessObservation confirmForegroundProcess(): Promise confirmShellForeground(): Promise } @@ -50,214 +44,121 @@ export function createPtyForegroundProcessTracker(args: { isDead: () => boolean }): PtyForegroundProcessTracker { const proc = args.process - let lastOutputAt = 0 - // `pid` anchors the identity to the row that proved it (null when ambiguous). - let cachedAgentForeground: CachedAgentForeground | null = null const contextPaths = getAgentForegroundContextPaths({ cwd: args.cwd, worktreeId: parsePtySessionId(args.sessionId).worktreeId }) - let startupAgentForeground: { processName: string; expiresAt: number } | null = - args.startupAgentRecognition + const state: ForegroundIdentityState = { + cachedAgentForeground: null, + startupAgentForeground: args.startupAgentRecognition ? { processName: args.startupAgentRecognition.processName, expiresAt: Date.now() + STARTUP_AGENT_FOREGROUND_BOOTSTRAP_MS } - : null - let foregroundRefreshInFlight = false - let lastForegroundRefreshStartedAt = 0 + : null, + lastScanSettlement: null, + refreshInFlight: false, + lastRefreshStartedAt: 0, + lastOutputAt: 0 + } const getFallbackProcess = (): string | null => resolveFallbackForegroundProcess(proc.process, args.shellPath) - const getActiveStartupAgent = ( - now = Date.now() - ): { processName: string; expiresAt: number } | null => { - if (!startupAgentForeground) { - return null - } - if (now > startupAgentForeground.expiresAt) { - startupAgentForeground = null - return null - } - return startupAgentForeground - } const shouldInspectFallback = (fallbackProcess: string | null): boolean => fallbackProcess !== null && (isShellProcess(fallbackProcess) || isAgentForegroundWrapperProcess(fallbackProcess) || shouldInspectOuterWrapperForegroundName(fallbackProcess) || process.platform !== 'win32') + const scheduleRefresh = createForegroundIdentityRefresh({ + process: proc, + state, + contextPaths, + isDead: args.isDead, + getFallbackProcess, + shouldInspectFallback + }) - const scheduleRefresh = (fallbackProcess: string | null): void => { - if (args.isDead() || !proc.pid) { - return + const observed = (processName: string | null): ForegroundProcessObservation => ({ + processName, + evidence: { verdict: 'observed', processName } + }) + const unverifiable = ( + processName: string | null, + reason: string + ): ForegroundProcessObservation => ({ + processName, + evidence: { verdict: 'unverifiable', reason } + }) + + const observeForegroundProcess = (): ForegroundProcessObservation => { + if (args.isDead()) { + // The pty's own exit marked death: a host observation, not a failed read. + return observed(null) } - const fallbackIsShell = fallbackProcess !== null && isShellProcess(fallbackProcess) - const fallbackRecognition = recognizeAgentProcess(fallbackProcess) - if ( - !fallbackProcess || - (fallbackRecognition !== null && - !shouldInspectOuterWrapperForegroundProcess(fallbackRecognition)) || - !shouldInspectFallback(fallbackProcess) - ) { - return - } - const now = Date.now() - const idleNoEvidenceShell = - fallbackIsShell && !getActiveStartupAgent(now) && !cachedAgentForeground - const retryMs = !idleNoEvidenceShell - ? FOREGROUND_AGENT_CACHE_TTL_MS - : process.platform === 'win32' && now - lastOutputAt > SHELL_FOREGROUND_OUTPUT_HOT_WINDOW_MS - ? WINDOWS_IDLE_SHELL_FOREGROUND_REFRESH_RETRY_MS - : SHELL_FOREGROUND_REFRESH_RETRY_MS - if (foregroundRefreshInFlight || now - lastForegroundRefreshStartedAt < retryMs) { - return - } - foregroundRefreshInFlight = true - lastForegroundRefreshStartedAt = now - const identityOlderThan = (ms: number): boolean => - cachedAgentForeground !== null && Date.now() - cachedAgentForeground.refreshedAt > ms - const retireStaleForegroundIdentity = ({ onlyWhenAged = false } = {}): void => { - const currentFallbackProcess = getFallbackProcess() - if ( - fallbackIsShell && - !getActiveStartupAgent() && - currentFallbackProcess !== null && - isShellProcess(currentFallbackProcess) && - (!onlyWhenAged || identityOlderThan(WINDOWS_DETACHED_DESCENDANT_IDENTITY_MAX_AGE_MS)) - ) { - cachedAgentForeground = null - startupAgentForeground = null - } else if ( - identityOlderThan(FOREGROUND_AGENT_CACHE_TTL_MS) && - currentFallbackProcess !== null && - isAgentForegroundWrapperProcess(currentFallbackProcess) - ) { - cachedAgentForeground = null + try { + const fallbackProcess = getFallbackProcess() + const fallbackRecognition = recognizeAgentProcess(fallbackProcess) + const inspectOuterWrapper = + fallbackRecognition !== null && + shouldInspectOuterWrapperForegroundProcess(fallbackRecognition) + if (fallbackProcess && fallbackRecognition && !inspectOuterWrapper) { + state.cachedAgentForeground = { + processName: fallbackProcess, + pid: null, + refreshedAt: Date.now() + } + state.startupAgentForeground = null + return observed(fallbackProcess) } + scheduleRefresh(fallbackProcess) + const now = Date.now() + if ( + state.cachedAgentForeground && + now - state.cachedAgentForeground.refreshedAt <= FOREGROUND_AGENT_CACHE_TTL_MS + ) { + return observed(state.cachedAgentForeground.processName) + } + if ( + state.cachedAgentForeground && + fallbackProcess !== null && + (isAgentForegroundWrapperProcess(fallbackProcess) || + inspectOuterWrapper || + (process.platform === 'win32' && isShellProcess(fallbackProcess))) + ) { + return observed(state.cachedAgentForeground.processName) + } + const activeStartupAgentForeground = getActiveStartupAgent(state, now) + if (fallbackProcess && isShellProcess(fallbackProcess) && activeStartupAgentForeground) { + return observed(activeStartupAgentForeground.processName) + } + if (fallbackProcess === null) { + // A successful read that named nothing usable observed nothing. + return unverifiable(null, 'pty reported no usable foreground title') + } + if ( + isShellProcess(fallbackProcess) && + !isShellTitleCorroborated(state.lastScanSettlement, now) + ) { + return unverifiable(fallbackProcess, 'shell title without a corroborating foreground scan') + } + return observed(fallbackProcess) + } catch { + return unverifiable(null, 'foreground title read threw') } - const anchor = cachedAgentForeground - void resolveAgentForegroundProcessWithAvailability(proc.pid, fallbackProcess, { - contextPaths, - ...(anchor?.pid != null - ? { anchorProcessId: anchor.pid, anchorProcessName: anchor.processName } - : {}) - }) - .then(({ processName, processId, available, anchorPidForeign }) => { - if (args.isDead() || !available) { - return - } - if (!processName || !recognizeAgentProcess(processName)) { - if (process.platform === 'win32' && fallbackIsShell && cachedAgentForeground !== null) { - // Job, not console: needs no console attachment, so no fork (#10857). - const verdict = judgeCachedAgentJobEvidence({ - jobProcessIds: readWindowsPtyJobProcessIds(proc), - jobSupported: isWindowsPtyJobReadable(), - shellPid: proc.pid, - anchorProcessId: cachedAgentForeground.pid, - identityAgeMs: Date.now() - cachedAgentForeground.refreshedAt - }) - // Unverifiable is never exit proof (ssh-execution-boundary.md): hold. - if (verdict === 'unavailable') { - return - } - if (verdict === 'unsupported') { - // No job to consult on this build, and the scan that got here was - // available and found no agent. Trust it, as every other platform - // does, rather than holding a dead name forever (#16059). - retireStaleForegroundIdentity() - return - } - if (verdict === 'confirmed' || verdict === 'recheck') { - if (anchorPidForeign === true) { - // The scan proved the pid recycled to a non-agent: retire now. - retireStaleForegroundIdentity() - return - } - // The anchor pid is still in the job: the scan lost the row, not - // the agent. Restamp so a live agent never ages out (#9258). - cachedAgentForeground = { ...cachedAgentForeground, refreshedAt: Date.now() } - return - } - if (verdict === 'exited' || verdict === 'anchor-exited') { - // Safe mid-restart: an available scan already found no agent. - retireStaleForegroundIdentity() - return - } - // Unanchored superset evidence cannot tell a working agent from a - // leftover; the age bound settles it. - retireStaleForegroundIdentity({ onlyWhenAged: true }) - return - } - retireStaleForegroundIdentity() - return - } - cachedAgentForeground = { processName, pid: processId ?? null, refreshedAt: Date.now() } - startupAgentForeground = null - return processName - }) - .catch(() => { - // Best-effort only: foreground enrichment must never affect PTY health. - }) - .finally(() => { - foregroundRefreshInFlight = false - }) } return { recordOutput: (data) => { if (data.length > 0) { - lastOutputAt = Date.now() + state.lastOutputAt = Date.now() } }, markDead: () => { - cachedAgentForeground = null - startupAgentForeground = null - }, - getForegroundProcess: () => { - if (args.isDead()) { - return null - } - try { - const fallbackProcess = getFallbackProcess() - const fallbackRecognition = recognizeAgentProcess(fallbackProcess) - const inspectOuterWrapper = - fallbackRecognition !== null && - shouldInspectOuterWrapperForegroundProcess(fallbackRecognition) - if (fallbackProcess && fallbackRecognition && !inspectOuterWrapper) { - cachedAgentForeground = { - processName: fallbackProcess, - pid: null, - refreshedAt: Date.now() - } - startupAgentForeground = null - return fallbackProcess - } - scheduleRefresh(fallbackProcess) - const now = Date.now() - if ( - cachedAgentForeground && - now - cachedAgentForeground.refreshedAt <= FOREGROUND_AGENT_CACHE_TTL_MS - ) { - return cachedAgentForeground.processName - } - if ( - cachedAgentForeground && - fallbackProcess !== null && - (isAgentForegroundWrapperProcess(fallbackProcess) || - inspectOuterWrapper || - (process.platform === 'win32' && isShellProcess(fallbackProcess))) - ) { - return cachedAgentForeground.processName - } - const activeStartupAgentForeground = getActiveStartupAgent(now) - if (fallbackProcess && isShellProcess(fallbackProcess) && activeStartupAgentForeground) { - return activeStartupAgentForeground.processName - } - return fallbackProcess - } catch { - return null - } + state.cachedAgentForeground = null + state.startupAgentForeground = null }, + getForegroundProcess: () => observeForegroundProcess().processName, + observeForegroundProcess, confirmForegroundProcess: async () => { if (args.isDead() || !proc.pid) { return null @@ -294,16 +195,16 @@ export function createPtyForegroundProcessTracker(args: { } const recognized = recognizeAgentProcess(resolution.processName) if (recognized) { - cachedAgentForeground = { + state.cachedAgentForeground = { processName: recognized.processName, pid: resolution.processId ?? null, refreshedAt: Date.now() } - startupAgentForeground = null + state.startupAgentForeground = null return recognized.processName } - cachedAgentForeground = null - startupAgentForeground = null + state.cachedAgentForeground = null + state.startupAgentForeground = null return resolution.processName } catch { return null diff --git a/src/main/daemon/pty-subprocess/subprocess-handle.ts b/src/main/daemon/pty-subprocess/subprocess-handle.ts index e2abd59c6ea..fdbf438ab79 100644 --- a/src/main/daemon/pty-subprocess/subprocess-handle.ts +++ b/src/main/daemon/pty-subprocess/subprocess-handle.ts @@ -70,6 +70,7 @@ export function createDaemonPtySubprocessHandle(args: { ? { startupCommandDeliveredInShellArgs: true } : {}), getForegroundProcess: foreground.getForegroundProcess, + observeForegroundProcess: foreground.observeForegroundProcess, confirmForegroundProcess: foreground.confirmForegroundProcess, confirmShellForeground: foreground.confirmShellForeground, write: (data) => { diff --git a/src/main/daemon/session-subprocess-handle.ts b/src/main/daemon/session-subprocess-handle.ts index f14469afbb4..5095f72d401 100644 --- a/src/main/daemon/session-subprocess-handle.ts +++ b/src/main/daemon/session-subprocess-handle.ts @@ -1,12 +1,26 @@ import type { TerminalExitCause } from '../../shared/terminal-exit-cause' +import type { PtyForegroundProcessEvidence } from '../../shared/pty-process-inspection-evidence' import type { JobTerminationOutcome } from '../windows/windows-pty-job' +/** A foreground read plus the evidence verdict behind it. `processName` keeps + * the exact legacy collapse every existing caller sees; the evidence says + * whether anything actually observed the pane, so a degraded read is never + * exit evidence (docs/reference/ssh-execution-boundary.md). */ +export type ForegroundProcessObservation = { + processName: string | null + evidence: PtyForegroundProcessEvidence +} + export type SubprocessHandle = { pid: number /** Live foreground process name of the PTY (node-pty's `.process`), e.g. * 'claude' / 'codex' / 'zsh'. Null once the child has exited. */ getForegroundProcess(): string | null + /** getForegroundProcess plus the evidence verdict behind the read. Optional so + * a handle that cannot report evidence reads as `unverifiable` (Session's + * fallback) rather than as an observation — the conservative arm. */ + observeForegroundProcess?(): ForegroundProcessObservation /** Await process-table evidence captured after this confirmation request. */ confirmForegroundProcess?(): Promise /** Proves a fresh post-boundary PTY process tree contains only the shell. */ @@ -44,3 +58,22 @@ export type SubprocessHandle = { /** Release the native PTY handle via node-pty's destroy(). Idempotent; safe to call after exit. */ dispose(): void } + +/** Read a handle's foreground observation. A handle with no evidence channel + * proves nothing about the pane, so it reads as `unverifiable` rather than as + * an observation (docs/reference/ssh-execution-boundary.md). */ +export function observeSubprocessForeground( + subprocess: Pick +): ForegroundProcessObservation { + const observe = subprocess.observeForegroundProcess + if (observe) { + return observe.call(subprocess) + } + return { + processName: subprocess.getForegroundProcess(), + evidence: { + verdict: 'unverifiable', + reason: 'subprocess handle reports no foreground evidence' + } + } +} diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index b0f1dfa538d..6cdb544b001 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -8,7 +8,8 @@ import { SessionTerminationController, IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS } from './session-termination-controller' -import type { SubprocessHandle } from './session-subprocess-handle' +import { observeSubprocessForeground } from './session-subprocess-handle' +import type { ForegroundProcessObservation, SubprocessHandle } from './session-subprocess-handle' import type { JobTerminationOutcome } from '../windows/windows-pty-job' import type { SessionOptions } from './session-options' import type { TuiAgent } from '../../shared/tui-agent' @@ -252,8 +253,8 @@ export class Session { return this.output.getCwd() } - getForegroundProcess(): string | null { - return this.subprocess.getForegroundProcess() + observeForegroundProcess(): ForegroundProcessObservation { + return observeSubprocessForeground(this.subprocess) } async confirmForegroundProcess(): Promise { diff --git a/src/main/daemon/terminal-host-inspection-degraded-handle.test.ts b/src/main/daemon/terminal-host-inspection-degraded-handle.test.ts new file mode 100644 index 00000000000..7cdb0b5fe95 --- /dev/null +++ b/src/main/daemon/terminal-host-inspection-degraded-handle.test.ts @@ -0,0 +1,81 @@ +// A subprocess handle with no evidence channel proves nothing about the pane. +// Session must read it as `unverifiable` rather than letting the legacy +// foreground collapse reach the completion monitor as an observation +// (docs/reference/ssh-execution-boundary.md). +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { TerminalHost } from './terminal-host' +import type { SubprocessHandle } from './session-subprocess-handle' + +vi.mock('../pty-descendant-termination', () => ({ killWithDescendantSweep: vi.fn() })) + +function createEvidencelessSubprocess(readForeground: () => string | null): SubprocessHandle { + let onExitCb: ((code: number) => void) | null = null + return { + pid: 999_999_412, + getForegroundProcess: readForeground, + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(() => setTimeout(() => onExitCb?.(0), 5)), + terminateOwnedTree: () => 'unavailable' as const, + forceKill: vi.fn(() => onExitCb?.(137)), + signal: vi.fn(), + onData: vi.fn(), + onExit(cb) { + onExitCb = cb + }, + dispose: vi.fn() + } as unknown as SubprocessHandle +} + +describe('daemon inspection of a handle that cannot report evidence', () => { + const SESSION_ID = 'repo-degraded-handle::/tmp/wt@@degraded01' + let host: TerminalHost + let platformDescriptor: PropertyDescriptor | undefined + let foreground: string | null + + beforeEach(async () => { + platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'linux' }) + foreground = 'zsh' + host = new TerminalHost({ + spawnSubprocess: () => createEvidencelessSubprocess(() => foreground) + }) + await host.createOrAttach({ + sessionId: SESSION_ID, + cols: 80, + rows: 24, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + }) + + afterEach(async () => { + await host.dispose() + if (platformDescriptor) { + Object.defineProperty(process, 'platform', platformDescriptor) + } + }) + + it('publishes unverifiable evidence beside the unchanged legacy fields', () => { + const result = host.inspectProcess(SESSION_ID) + + expect(result.foregroundProcess).toBe('zsh') + expect(result.hasChildProcesses).toBe(false) + expect(result.processEvidence?.foreground.verdict).toBe('unverifiable') + expect(result.processEvidence?.children.verdict).toBe('unverifiable') + }) + + it('does not upgrade a named agent to observed either', () => { + // The conservative arm cuts both ways: an unverifiable read cannot prove + // the agent is live any more than it can prove it exited. + foreground = 'codex' + const result = host.inspectProcess(SESSION_ID) + + expect(result.hasChildProcesses).toBe(true) + expect(result.processEvidence?.foreground.verdict).toBe('unverifiable') + expect(result.processEvidence?.children.verdict).toBe('unverifiable') + }) + + it('leaves the legacy foreground read untouched', () => { + expect(host.getForegroundProcess(SESSION_ID)).toBe('zsh') + }) +}) diff --git a/src/main/daemon/terminal-host-process-evidence.test.ts b/src/main/daemon/terminal-host-process-evidence.test.ts new file mode 100644 index 00000000000..4dfa89389de --- /dev/null +++ b/src/main/daemon/terminal-host-process-evidence.test.ts @@ -0,0 +1,69 @@ +// Pins the direction every arm of the daemon inspection collapse falls in: a +// foreground read that observed nothing must reach the caller as +// `unverifiable`, never as the `exited` the completion monitor acts on +// (docs/reference/ssh-execution-boundary.md). +import { describe, expect, it } from 'vitest' +import { buildDaemonInspectProcessResult } from './terminal-host-process-evidence' +import type { ForegroundProcessObservation } from './session-subprocess-handle' + +function observed(processName: string | null): ForegroundProcessObservation { + return { processName, evidence: { verdict: 'observed', processName } } +} + +function unverifiable(processName: string | null, reason: string): ForegroundProcessObservation { + return { processName, evidence: { verdict: 'unverifiable', reason } } +} + +describe('buildDaemonInspectProcessResult', () => { + it('reports a degraded read as unverifiable on both probes, never as an exit', () => { + const result = buildDaemonInspectProcessResult(unverifiable('zsh', 'scan never settled')) + + expect(result.processEvidence?.foreground).toEqual({ + verdict: 'unverifiable', + reason: 'scan never settled' + }) + expect(result.processEvidence?.children).toEqual({ + verdict: 'unverifiable', + reason: 'scan never settled' + }) + }) + + it('keeps the legacy fields byte-identical to the pre-evidence collapse', () => { + // The exact payload an old client still receives: a degraded read publishes + // the shell title it fell back to and hasChildProcesses:false. Only the + // additive evidence field distinguishes it from an observed idle shell. + const degraded = buildDaemonInspectProcessResult(unverifiable('zsh', 'scan never settled')) + expect(degraded.foregroundProcess).toBe('zsh') + expect(degraded.hasChildProcesses).toBe(false) + + const live = buildDaemonInspectProcessResult(observed('codex')) + expect(live.foregroundProcess).toBe('codex') + expect(live.hasChildProcesses).toBe(true) + }) + + it('calls children live only when the observation named a non-shell process', () => { + expect(buildDaemonInspectProcessResult(observed('codex')).processEvidence?.children).toEqual({ + verdict: 'live' + }) + }) + + it('calls children exited only on a positive observation of an idle pane', () => { + expect(buildDaemonInspectProcessResult(observed('zsh')).processEvidence?.children).toEqual({ + verdict: 'exited' + }) + // The host watched the pty die: absence it observed itself. + expect(buildDaemonInspectProcessResult(observed(null)).processEvidence?.children).toEqual({ + verdict: 'exited' + }) + }) + + it('never lets the children verdict outrank an unverifiable foreground', () => { + // An agent name carried by a read nothing corroborated is still not proof + // the agent is live, and a shell name is still not proof it exited. + for (const name of ['codex', 'zsh', null]) { + const children = buildDaemonInspectProcessResult(unverifiable(name, 'title read threw')) + .processEvidence?.children + expect(children?.verdict).toBe('unverifiable') + } + }) +}) diff --git a/src/main/daemon/terminal-host-process-evidence.ts b/src/main/daemon/terminal-host-process-evidence.ts new file mode 100644 index 00000000000..990d82b1015 --- /dev/null +++ b/src/main/daemon/terminal-host-process-evidence.ts @@ -0,0 +1,42 @@ +import { isShellProcess } from '../../shared/agent-detection' +import type { PtyChildProcessesEvidence } from '../../shared/pty-process-inspection-evidence' +import type { PtyProcessInspection } from '../providers/pty-process-inspection' +import type { ForegroundProcessObservation } from './session-subprocess-handle' + +/** + * Wire result for the daemon 'inspectProcess' request. The legacy fields keep + * the exact pre-evidence collapse (a degraded read still publishes the shell + * title it fell back to); only the optional `processEvidence` distinguishes + * "observed idle" from "could not ask", so a degraded read is never exit + * evidence (docs/reference/ssh-execution-boundary.md). The field is additive: + * older app clients ignore it and see byte-identical content. + */ +export function buildDaemonInspectProcessResult( + observation: ForegroundProcessObservation +): PtyProcessInspection { + const foregroundProcess = observation.processName + return { + foregroundProcess, + hasChildProcesses: foregroundProcess !== null && !isShellProcess(foregroundProcess), + processEvidence: { + foreground: observation.evidence, + children: deriveChildProcessesEvidence(observation) + } + } +} + +/** A daemon pane's only child signal IS the foreground observation, so the + * children verdict can never outrank the foreground's. Observed null means + * the host itself watched the pane die — positive absence, not a failure. */ +function deriveChildProcessesEvidence( + observation: ForegroundProcessObservation +): PtyChildProcessesEvidence { + if (observation.evidence.verdict !== 'observed') { + return { verdict: 'unverifiable', reason: observation.evidence.reason } + } + const processName = observation.evidence.processName + if (processName !== null && !isShellProcess(processName)) { + return { verdict: 'live' } + } + return { verdict: 'exited' } +} diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index f85980b5453..8fd1ed2f105 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -19,7 +19,8 @@ import { resolveTerminalHostSessionCwd } from './terminal-host-session-cwd' import { TerminalHostTombstones } from './terminal-host-tombstones' import { listLiveTerminalHostSessions } from './terminal-host-session-listing' import { createOrAttachTerminalSession } from './terminal-host-session-create' -import { isShellProcess } from '../../shared/agent-detection' +import { buildDaemonInspectProcessResult } from './terminal-host-process-evidence' +import type { PtyProcessInspection } from '../providers/pty-process-inspection' import { TerminalAttachCanceledError } from './daemon-errors' export type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract' @@ -211,18 +212,13 @@ export class TerminalHost { if (!session || !session.isAlive) { return null } - return session.getForegroundProcess() + return session.observeForegroundProcess().processName } - inspectProcess(sessionId: string): { - foregroundProcess: string | null - hasChildProcesses: boolean - } { - const foregroundProcess = this.getAliveSession(sessionId).getForegroundProcess() - return { - foregroundProcess, - hasChildProcesses: foregroundProcess !== null && !isShellProcess(foregroundProcess) - } + inspectProcess(sessionId: string): PtyProcessInspection { + return buildDaemonInspectProcessResult( + this.getAliveSession(sessionId).observeForegroundProcess() + ) } async confirmForegroundProcess(sessionId: string): Promise { diff --git a/tests/e2e/daemon-completion-evidence/daemon-inspection-evidence-wire.unit.test.ts b/tests/e2e/daemon-completion-evidence/daemon-inspection-evidence-wire.unit.test.ts new file mode 100644 index 00000000000..378efdcebf3 --- /dev/null +++ b/tests/e2e/daemon-completion-evidence/daemon-inspection-evidence-wire.unit.test.ts @@ -0,0 +1,157 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { rmSync } from 'node:fs' +import type * as pty from 'node-pty' + +const { execFileMock, execFileSyncMock, killWithDescendantSweepMock, forceKillPosixMock } = + vi.hoisted(() => ({ + execFileMock: vi.fn(), + execFileSyncMock: vi.fn(), + killWithDescendantSweepMock: vi.fn(), + forceKillPosixMock: vi.fn() + })) + +// The identity scan's `ps -axo` fork; scripted to fail so no scan can settle. +vi.mock('child_process', () => ({ + execFile: execFileMock, + execFileSync: execFileSyncMock +})) + +vi.mock('../../../src/main/pty-descendant-termination', () => ({ + killWithDescendantSweep: killWithDescendantSweepMock +})) + +// Teardown escalation must never signal a real process group from a test. +vi.mock('../../../src/main/pty/posix-pty-process-groups', () => ({ + forceKillPosixPtyProcessGroups: forceKillPosixMock +})) + +import { createDaemonPtySubprocessHandle } from '../../../src/main/daemon/pty-subprocess/subprocess-handle' +import { + startDaemonAdapterHarness, + type DaemonAdapterHarness +} from '../../../src/main/daemon/daemon-pty-adapter-test-harness' +import { inspectPtyProviderProcessForRenderer } from '../../../src/main/providers/pty-process-inspection' + +/** + * The daemon inspection evidence must survive the REAL wire: DaemonServer → + * request router → JSON socket → DaemonPtyAdapter. The sibling suite proves + * the monitor consequences in-process; this one proves the `processEvidence` + * field actually crosses the daemon protocol, beside byte-identical legacy + * fields, on the exact socket a macOS daemon pane uses. + */ +describe('daemon inspection evidence over the daemon socket', () => { + const SHELL_PID = 999_999_311 + const AGENT_SESSION_ID = 'repo-daemon-wire::/tmp/wt@@wire0001' + const SHELL_SESSION_ID = 'repo-daemon-wire::/tmp/wt@@wire0002' + + type MockNodePty = pty.IPty & { process: string; _fireExit: (exitCode: number) => void } + + function createMockNodePtyProcess(): MockNodePty { + const exitListeners: ((e: { exitCode: number; signal?: number }) => void)[] = [] + const proc = { + pid: SHELL_PID, + cols: 80, + rows: 24, + process: 'codex', + handleFlowControl: false, + onData: vi.fn(() => ({ dispose: vi.fn() })), + onExit: vi.fn((cb: (e: { exitCode: number; signal?: number }) => void) => { + exitListeners.push(cb) + return { dispose: vi.fn() } + }), + write: vi.fn(), + resize: vi.fn(), + clear: vi.fn(), + pause: vi.fn(), + resume: vi.fn(), + kill: vi.fn(() => { + setTimeout(() => proc._fireExit(0), 5) + }), + _fireExit: (exitCode: number) => { + exitListeners.forEach((cb) => cb({ exitCode, signal: 0 })) + } + } + return proc as unknown as MockNodePty + } + + let harness: DaemonAdapterHarness + let procsBySession: Map + + beforeEach(async () => { + execFileMock.mockReset() + execFileMock.mockImplementation( + (_command: string, _args: string[], _opts: unknown, cb: unknown) => { + ;(cb as (err: Error | null, result: { stdout: string; stderr: string }) => void)( + new Error('ps unavailable in wire test'), + { stdout: '', stderr: '' } + ) + } + ) + killWithDescendantSweepMock.mockReset() + killWithDescendantSweepMock.mockResolvedValue(undefined) + forceKillPosixMock.mockReset() + procsBySession = new Map() + forceKillPosixMock.mockImplementation(() => { + setTimeout(() => procsBySession.forEach((proc) => proc._fireExit(137)), 1) + }) + harness = await startDaemonAdapterHarness((opts) => { + const proc = createMockNodePtyProcess() + proc.process = opts.sessionId === AGENT_SESSION_ID ? 'codex' : 'zsh' + procsBySession.set(opts.sessionId, proc) + return createDaemonPtySubprocessHandle({ + process: proc, + shellPath: '/bin/zsh', + spawnCwd: '/tmp/wt', + env: { PATH: '/usr/bin' }, + startupCommandDeliveredInShellArgs: false, + reportsChildExitStatus: true, + requestedCwd: '/tmp/wt', + sessionId: opts.sessionId, + startupAgentRecognition: null + }) + }) + }) + + afterEach(async () => { + harness.adapter?.dispose() + await harness.server?.shutdown() + rmSync(harness.dir, { recursive: true, force: true }) + vi.restoreAllMocks() + }) + + it('carries observed and unverifiable evidence across the socket, legacy fields intact', async () => { + const agentPane = await harness.adapter.spawn({ + sessionId: AGENT_SESSION_ID, + cols: 80, + rows: 24 + }) + const shellPane = await harness.adapter.spawn({ + sessionId: SHELL_SESSION_ID, + cols: 80, + rows: 24 + }) + + // A recognized agent title is a direct observation — no scan involved. + expect(await inspectPtyProviderProcessForRenderer(harness.adapter, agentPane.id)).toEqual({ + foregroundProcess: 'codex', + hasChildProcesses: true, + processEvidence: { + foreground: { verdict: 'observed', processName: 'codex' }, + children: { verdict: 'live' } + } + }) + + // A shell-shaped title with no settled scan: node-pty's silent shell + // fallback makes it indistinguishable from a degraded read. The legacy + // fields keep the exact pre-evidence collapse the monitor used to read as + // exit proof; the evidence now says "could not ask". + expect(await inspectPtyProviderProcessForRenderer(harness.adapter, shellPane.id)).toEqual({ + foregroundProcess: 'zsh', + hasChildProcesses: false, + processEvidence: { + foreground: { verdict: 'unverifiable', reason: expect.any(String) }, + children: { verdict: 'unverifiable', reason: expect.any(String) } + } + }) + }) +}) From b16541afcb5a14d1370c571f93f63f101cf259b5 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 05:52:14 -0700 Subject: [PATCH 03/10] Pin the 30s shell-title corroboration bound from both sides Ablation on the refreshed composed tree showed the constant itself was unpinned: widening SHELL_TITLE_SCAN_CORROBORATION_MAX_AGE_MS to Infinity left the whole matrix green. A wedged `ps` is the trade-off this PR accepts deliberately, so the bound that scopes it is load-bearing. --- ...ss-foreground-observation-evidence.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts index 222b862c867..d7811f572e5 100644 --- a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts +++ b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts @@ -17,6 +17,9 @@ import type { SubprocessHandle } from './session-subprocess-handle' const SHELL_PID = 999_999_517 // Above the idle-shell refresh throttle (5s) so each read starts a fresh scan. const PAST_THE_SCAN_THROTTLE_MS = 6_000 +// Bracket the 30s shell-title corroboration window from both sides. +const INSIDE_CORROBORATION_WINDOW_MS = 25_000 +const PAST_CORROBORATION_WINDOW_MS = 35_000 describe('daemon foreground observation evidence', () => { let platformDescriptor: PropertyDescriptor | undefined @@ -109,6 +112,22 @@ describe('daemon foreground observation evidence', () => { expect(observation?.evidence.verdict).toBe('unverifiable') }) + it('stops observing a shell title once corroboration ages past the 30s bound', async () => { + resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'zsh' }) + expect((await readAfterSettledScan())?.evidence.verdict).toBe('observed') + + // A permanently wedged `ps`: the scan starts and never settles, so nothing + // refreshes corroboration and it simply ages out. Brackets the 30s bound on + // both sides so widening or removing it fails here. + resolveAgentForegroundProcessMock.mockReturnValue(new Promise(() => {})) + + await vi.advanceTimersByTimeAsync(INSIDE_CORROBORATION_WINDOW_MS) + expect(handle.observeForegroundProcess?.()?.evidence.verdict).toBe('observed') + + await vi.advanceTimersByTimeAsync(PAST_CORROBORATION_WINDOW_MS - INSIDE_CORROBORATION_WINDOW_MS) + expect(handle.observeForegroundProcess?.()?.evidence.verdict).toBe('unverifiable') + }) + it('keeps a live agent title an observation without needing a scan', () => { nodePty.process = 'codex' From 7eefcdaac3156d80583518046bcd9fb665b89e7d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 07:50:18 -0700 Subject: [PATCH 04/10] test(daemon): pin the observation arms the degraded-read verdict rests on Deepens the terminal-host evidence coverage the gate audit found thinnest. Each arm below goes red with that arm reverted alone: - an empty node-pty title read is a failed read, not an observed exit - a title read that throws is unverifiable - a corroborating scan followed by an unavailable one retires the corroboration rather than inheriting it (the settlement has to be recorded on unavailable scans too, or the last successful one stands forever) - the pty's own exit stays a positive observation, so the conservative arms cannot swallow a real completion - the children verdict never outranks a non-observed foreground The Windows unverifiable-job hold gains the read the existing assertion could not reach: a read returns before the scan it schedules settles, so the hold has to outlive that scan too. Ablating the hold with only the pre-existing assertion present stays green; it goes red with the added one. --- ...ubprocess-foreground-degraded-scan.test.ts | 3 + ...ss-foreground-observation-evidence.test.ts | 80 ++++++++++++++++++- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts b/src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts index 8d37578bcf3..334d0c1c175 100644 --- a/src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts +++ b/src/main/daemon/pty-subprocess-foreground-degraded-scan.test.ts @@ -218,6 +218,9 @@ describe('daemon pty foreground degraded-scan handling', () => { await readForegroundAt(handle, 0) expect(await readForegroundAt(handle, 120_000)).toBe('claude') + // A read returns before the scan it schedules settles, so the hold has to + // outlive that scan too -- an unreadable job stays unverifiable at any age. + expect(await readForegroundAt(handle, 120_001)).toBe('claude') }) it('retires an anchored agent immediately when its pid leaves the job, despite a leftover', async () => { diff --git a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts index d7811f572e5..1529cb53510 100644 --- a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts +++ b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts @@ -12,7 +12,8 @@ vi.mock('../providers/agent-foreground-process', () => ({ })) import { createDaemonPtySubprocessHandle } from './pty-subprocess/subprocess-handle' -import type { SubprocessHandle } from './session-subprocess-handle' +import { buildDaemonInspectProcessResult } from './terminal-host-process-evidence' +import type { ForegroundProcessObservation, SubprocessHandle } from './session-subprocess-handle' const SHELL_PID = 999_999_517 // Above the idle-shell refresh throttle (5s) so each read starts a fresh scan. @@ -25,6 +26,26 @@ describe('daemon foreground observation evidence', () => { let platformDescriptor: PropertyDescriptor | undefined let nodePty: pty.IPty & { process: string } let handle: SubprocessHandle + let exitListeners: ((event: { exitCode: number; signal?: number }) => void)[] + + function observe(): ForegroundProcessObservation { + const observation = handle.observeForegroundProcess?.() + if (!observation) { + throw new Error('handle exposes no foreground evidence channel') + } + return observation + } + + /** The children verdict the completion monitor actually acts on. */ + function childrenVerdict(observation: ForegroundProcessObservation): string | undefined { + return buildDaemonInspectProcessResult(observation).processEvidence?.children.verdict + } + + function exitPty(): void { + for (const listener of exitListeners) { + listener({ exitCode: 0 }) + } + } async function readAfterSettledScan(): Promise @@ -46,11 +67,15 @@ describe('daemon foreground observation evidence', () => { resolveAgentForegroundProcessMock.mockReset() // Default: a scan that runs and finds no agent. Individual tests override. resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'zsh' }) + exitListeners = [] nodePty = { pid: SHELL_PID, process: 'zsh', onData: vi.fn(() => ({ dispose: vi.fn() })), - onExit: vi.fn(() => ({ dispose: vi.fn() })), + onExit: vi.fn((listener) => { + exitListeners.push(listener) + return { dispose: vi.fn() } + }), write: vi.fn(), resize: vi.fn(), kill: vi.fn() @@ -144,4 +169,55 @@ describe('daemon foreground observation evidence', () => { // The wire's legacy field must not change shape when evidence degrades. expect(handle.getForegroundProcess()).toBe('zsh') }) + + it('withholds observation when the title read names nothing usable', () => { + // node-pty's POSIX title read reports an empty name when the native read + // fails on a live pane, so "no name" is a failed read, not an observed exit. + nodePty.process = '' + + const observation = observe() + + expect(observation.processName).toBeNull() + expect(observation.evidence.verdict).toBe('unverifiable') + expect(childrenVerdict(observation)).toBe('unverifiable') + }) + + it('withholds observation when the title read throws', () => { + Object.defineProperty(nodePty, 'process', { + configurable: true, + get: () => { + throw new Error('foreground title read failed') + } + }) + + const observation = observe() + + expect(observation.processName).toBeNull() + expect(observation.evidence.verdict).toBe('unverifiable') + expect(childrenVerdict(observation)).toBe('unverifiable') + }) + + it('withholds observation after a corroborating scan is followed by an unavailable one', async () => { + resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'zsh' }) + expect((await readAfterSettledScan())?.evidence.verdict).toBe('observed') + + // A scan that ran but could not answer is a relay host's normal steady + // state. It settles too, so the corroboration it failed to refresh must + // retire rather than be inherited from the last scan that succeeded. + resolveAgentForegroundProcessMock.mockResolvedValue({ available: false, processName: 'zsh' }) + const observation = await readAfterSettledScan() + + expect(observation?.evidence.verdict).toBe('unverifiable') + }) + + it('observes the exit node-pty itself reported', () => { + // The conservative arms must not swallow a real completion: once the host + // watched the pty exit, absence is something it observed happen. + exitPty() + + const observation = observe() + + expect(observation.evidence).toEqual({ verdict: 'observed', processName: null }) + expect(childrenVerdict(observation)).toBe('exited') + }) }) From 8a38dff48214e235057f0faf17e0d543a1e181ba Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:37:53 -0700 Subject: [PATCH 05/10] fix(daemon): stop corroborating a shell title with a pre-agent settlement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The synchronous title fast path stamps a recognized agent without running a scan, so the last agent-free settlement can still sit inside the 30s window while an agent is live. When node-pty's title read then falls back to the spawned shell file, that stale settlement corroborated the shell title and the tracker published `observed(shell)` for a running agent — which the completion monitor reads as the agent having exited. Corroboration now also requires the settlement to be no older than the most recent positive agent observation, which `cachedAgentForeground.refreshedAt` already records for both the scan and the fast path. An idle pane holds no cached identity, so healthy corroboration is unchanged. Verified against the checks skipped by --no-verify (no node_modules in the throwaway worktree): oxlint, oxfmt and the full src/main/daemon suite were run on byte-identical file contents in the stack tip's worktree. --- ...ss-foreground-observation-evidence.test.ts | 23 +++++++++++++++++++ .../foreground-identity-refresh.ts | 10 ++++++-- .../foreground-process-tracker.ts | 2 +- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts index 1529cb53510..8d682b7aa67 100644 --- a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts +++ b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts @@ -162,6 +162,29 @@ describe('daemon foreground observation evidence', () => { }) }) + it('stops corroborating with a settlement older than an agent the title fast path saw', async () => { + // The sync fast path stamps a recognized title without running a scan, so the + // last agent-free settlement can still be inside the 30s window while an agent + // is running. Corroborating from it publishes a live agent as an idle shell. + resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: 'zsh' }) + expect((await readAfterSettledScan())?.evidence.verdict).toBe('observed') + + await vi.advanceTimersByTimeAsync(2_000) + nodePty.process = 'codex' + expect(handle.observeForegroundProcess?.()?.evidence).toEqual({ + verdict: 'observed', + processName: 'codex' + }) + + // A wedged scan never refreshes the settlement, and node-pty's title read + // falls back to the spawned shell while the agent is still running. + resolveAgentForegroundProcessMock.mockReturnValue(new Promise(() => {})) + nodePty.process = 'zsh' + await vi.advanceTimersByTimeAsync(2_000) + + expect(handle.observeForegroundProcess?.()?.evidence.verdict).toBe('unverifiable') + }) + it('leaves the legacy foreground read identical to the observed name', async () => { resolveAgentForegroundProcessMock.mockResolvedValue({ available: false, processName: 'zsh' }) await readAfterSettledScan() diff --git a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts index daa93c372d9..fa6e23f8290 100644 --- a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts +++ b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts @@ -64,16 +64,22 @@ export function getActiveStartupAgent( * shell file when the native read fails, so under the same distress that * degrades the scan, "title == shell" observes nothing — and a scan that last * saw the agent cannot corroborate its absence either. + * + * `agentEvidence` outdates the settlement: the synchronous title fast path + * stamps a recognized agent without running a scan, so an agent-free settlement + * can still be inside the window while an agent that started after it is live. */ export function isShellTitleCorroborated( settlement: ForegroundScanSettlement | null, - now: number + now: number, + agentEvidence: CachedAgentForeground | null ): boolean { return ( settlement !== null && settlement.available && !settlement.sawAgent && - now - settlement.at <= SHELL_TITLE_SCAN_CORROBORATION_MAX_AGE_MS + now - settlement.at <= SHELL_TITLE_SCAN_CORROBORATION_MAX_AGE_MS && + (agentEvidence === null || settlement.at >= agentEvidence.refreshedAt) ) } diff --git a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts index c47340e5ae4..9ef7d97865a 100644 --- a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts +++ b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts @@ -137,7 +137,7 @@ export function createPtyForegroundProcessTracker(args: { } if ( isShellProcess(fallbackProcess) && - !isShellTitleCorroborated(state.lastScanSettlement, now) + !isShellTitleCorroborated(state.lastScanSettlement, now, state.cachedAgentForeground) ) { return unverifiable(fallbackProcess, 'shell title without a corroborating foreground scan') } From 2b5d72158a91e95546e6371c76ec8b453750776c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 15:43:05 -0700 Subject: [PATCH 06/10] fix(daemon): date scan corroboration by its snapshot, not by when it settled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The settlement was stamped when the scan settled, but its process table was read when the scan started. An agent that appears in between is invisible to that snapshot, so a pre-agent scan settling after the title fast path recognized the agent still had the newer timestamp and went on corroborating the shell title — the same false completion the previous commit set out to close, one ordering over. Carry `startedAt` on the settlement and order against that. The 30s staleness bound still runs off `at`, which is the right clock for "how old is this answer". --- ...ss-foreground-observation-evidence.test.ts | 34 +++++++++++++++++++ .../foreground-identity-refresh.ts | 30 ++++++++++++---- 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts index 8d682b7aa67..57e287712a0 100644 --- a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts +++ b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts @@ -185,6 +185,40 @@ describe('daemon foreground observation evidence', () => { expect(handle.observeForegroundProcess?.()?.evidence.verdict).toBe('unverifiable') }) + it('stops corroborating with a snapshot captured before an agent the scan could not see', async () => { + // A scan stamps its settlement when it SETTLES, but its process snapshot was taken + // when it STARTED. An agent that appears in between is invisible to that snapshot, + // so settle-time ordering alone still lets a pre-agent scan corroborate the shell. + let settleScan: (value: { available: boolean; processName: string }) => void = () => {} + resolveAgentForegroundProcessMock.mockReturnValue( + new Promise<{ available: boolean; processName: string }>((resolve) => { + settleScan = resolve + }) + ) + // Starts the agent-free scan; its snapshot is taken now. + expect(handle.observeForegroundProcess?.()?.evidence.verdict).toBe('unverifiable') + + // The agent starts while that scan is still in flight. + await vi.advanceTimersByTimeAsync(2_000) + nodePty.process = 'codex' + expect(handle.observeForegroundProcess?.()?.evidence).toEqual({ + verdict: 'observed', + processName: 'codex' + }) + + // Only now does the pre-agent scan settle, so its `at` is newer than the agent stamp. + await vi.advanceTimersByTimeAsync(1_000) + settleScan({ available: true, processName: 'zsh' }) + await vi.advanceTimersByTimeAsync(0) + + // The title degrades back to the shell while the agent is still running. + resolveAgentForegroundProcessMock.mockReturnValue(new Promise(() => {})) + nodePty.process = 'zsh' + await vi.advanceTimersByTimeAsync(2_000) + + expect(handle.observeForegroundProcess?.()?.evidence.verdict).toBe('unverifiable') + }) + it('leaves the legacy foreground read identical to the observed name', async () => { resolveAgentForegroundProcessMock.mockResolvedValue({ available: false, processName: 'zsh' }) await readAfterSettledScan() diff --git a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts index fa6e23f8290..9bd23035b25 100644 --- a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts +++ b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts @@ -32,8 +32,16 @@ export type CachedAgentForeground = { } /** Outcome of the most recently settled identity scan. `sawAgent` is only - * meaningful when `available` — an unavailable scan observed nothing. */ -export type ForegroundScanSettlement = { at: number; available: boolean; sawAgent: boolean } + * meaningful when `available` — an unavailable scan observed nothing. + * `startedAt` dates the process snapshot the scan read; `at` dates the answer. An + * agent that appears between the two is invisible to the snapshot, so ordering + * against other evidence must use `startedAt`. */ +export type ForegroundScanSettlement = { + at: number + startedAt: number + available: boolean + sawAgent: boolean +} export type ForegroundIdentityState = { cachedAgentForeground: CachedAgentForeground | null @@ -65,9 +73,11 @@ export function getActiveStartupAgent( * degrades the scan, "title == shell" observes nothing — and a scan that last * saw the agent cannot corroborate its absence either. * - * `agentEvidence` outdates the settlement: the synchronous title fast path - * stamps a recognized agent without running a scan, so an agent-free settlement - * can still be inside the window while an agent that started after it is live. + * `agentEvidence` outdates the settlement: the synchronous title fast path stamps a + * recognized agent without running a scan, so an agent-free settlement can still be + * inside the window while an agent that started after it is live. Compared against + * `startedAt`, not `at` — a scan that settles after the agent appeared still read a + * process table from before it. */ export function isShellTitleCorroborated( settlement: ForegroundScanSettlement | null, @@ -79,7 +89,7 @@ export function isShellTitleCorroborated( settlement.available && !settlement.sawAgent && now - settlement.at <= SHELL_TITLE_SCAN_CORROBORATION_MAX_AGE_MS && - (agentEvidence === null || settlement.at >= agentEvidence.refreshedAt) + (agentEvidence === null || settlement.startedAt >= agentEvidence.refreshedAt) ) } @@ -159,6 +169,7 @@ export function createForegroundIdentityRefresh(args: { .then(({ processName, processId, available, anchorPidForeign }) => { state.lastScanSettlement = { at: Date.now(), + startedAt: now, available, sawAgent: available && recognizeAgentProcess(processName) !== null } @@ -227,7 +238,12 @@ export function createForegroundIdentityRefresh(args: { }) .catch(() => { // Best-effort only: foreground enrichment must never affect PTY health. - state.lastScanSettlement = { at: Date.now(), available: false, sawAgent: false } + state.lastScanSettlement = { + at: Date.now(), + startedAt: now, + available: false, + sawAgent: false + } }) .finally(() => { state.refreshInFlight = false From abce3dfe53da609657464496c4c9f6144fb751db Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 16:20:55 -0700 Subject: [PATCH 07/10] fix(daemon): stop a pre-agent scan retiring the agent it never sampled Dating corroboration by the scan's snapshot closed the direct path but not the one through retirement. A scan whose process table predates the agent still settles "no agent", and when the title has already degraded back to the shell, retireStaleForegroundIdentity clears the cached identity on that answer. The corroboration guard's no-evidence arm then accepts the very same stale settlement, so the live agent reads as an observed idle shell again. Retirement now refuses an identity stamped after this scan sampled the table, which is the same ordering rule the corroboration check already applies. Ordinary retirement is untouched: an agent that exits was stamped before the scan that finds it gone. --- ...ss-foreground-observation-evidence.test.ts | 32 +++++++++++++++++++ .../foreground-identity-refresh.ts | 5 +++ 2 files changed, 37 insertions(+) diff --git a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts index 57e287712a0..41ca6c9ed10 100644 --- a/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts +++ b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts @@ -219,6 +219,38 @@ describe('daemon foreground observation evidence', () => { expect(handle.observeForegroundProcess?.()?.evidence.verdict).toBe('unverifiable') }) + it('does not let a pre-agent scan retire the agent it could not see', async () => { + // The settling scan finds no agent and retires the cached identity — but its snapshot + // predates that agent. Retiring it hands the same stale scan corroboration by way of + // the no-evidence arm, so the guard has to hold on both sides. + let settleScan: (value: { available: boolean; processName: string }) => void = () => {} + resolveAgentForegroundProcessMock.mockReturnValue( + new Promise<{ available: boolean; processName: string }>((resolve) => { + settleScan = resolve + }) + ) + expect(handle.observeForegroundProcess?.()?.evidence.verdict).toBe('unverifiable') + + await vi.advanceTimersByTimeAsync(2_000) + nodePty.process = 'codex' + expect(handle.observeForegroundProcess?.()?.evidence).toEqual({ + verdict: 'observed', + processName: 'codex' + }) + + // The title degrades back to the shell BEFORE the pre-agent scan settles, so + // retirement sees a shell on both sides and clears the agent evidence. + nodePty.process = 'zsh' + await vi.advanceTimersByTimeAsync(1_000) + settleScan({ available: true, processName: 'zsh' }) + await vi.advanceTimersByTimeAsync(0) + + resolveAgentForegroundProcessMock.mockReturnValue(new Promise(() => {})) + await vi.advanceTimersByTimeAsync(2_000) + + expect(handle.observeForegroundProcess?.()?.evidence.verdict).toBe('unverifiable') + }) + it('leaves the legacy foreground read identical to the observed name', async () => { resolveAgentForegroundProcessMock.mockResolvedValue({ available: false, processName: 'zsh' }) await readAfterSettledScan() diff --git a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts index 9bd23035b25..f9e90d18a2c 100644 --- a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts +++ b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts @@ -141,6 +141,11 @@ export function createForegroundIdentityRefresh(args: { state.cachedAgentForeground !== null && Date.now() - state.cachedAgentForeground.refreshedAt > ms const retireStaleForegroundIdentity = ({ onlyWhenAged = false } = {}): void => { + // This scan sampled the process table at `now`; an identity stamped after that is + // evidence it never had a chance to see, so it cannot be retired on this answer. + if (state.cachedAgentForeground !== null && state.cachedAgentForeground.refreshedAt > now) { + return + } const currentFallbackProcess = args.getFallbackProcess() if ( fallbackIsShell && From 5898fdcbe91be9df68054cd0336c29a55f9d075b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:30:55 -0700 Subject: [PATCH 08/10] fix(daemon): order a foreground scan by the table it read, not by when it asked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two ordering guards on the shell-title corroboration path both compare against `now` — the moment this pane's refresh started. But the POSIX process table is a process-wide 500ms cache shared by every pane (process-table-snapshot.ts), so a refresh is routinely answered from a scan another pane started earlier. `now` then claims the table is more current than it is, and an agent that began in that window is missing from rows the guards treat as newer than it. The whole chain follows from there: the settlement is dated by the ask, so retirement clears an identity the scan never sampled, and the corroboration guard's no-evidence arm accepts the same stale settlement — a live agent publishes as an observed idle shell, which the completion monitor reads as `exited`. The reader now stamps when each scan began, alongside the capture time TTL reuse already needs, and carries it out through the resolver as `tableScanStartedAtMs`. Ordering uses the measured scan; `now` remains the fallback for answers that read no table (Windows job evidence, early returns). --- ...s-foreground-shared-snapshot-cache.test.ts | 139 +++++++++++++++++ .../foreground-identity-refresh.ts | 142 ++++++++++-------- ...ound-process-snapshot-availability.test.ts | 20 ++- .../providers/agent-foreground-process.ts | 16 +- src/shared/process-table-snapshot.ts | 57 +++++-- 5 files changed, 284 insertions(+), 90 deletions(-) create mode 100644 src/main/daemon/pty-subprocess-foreground-shared-snapshot-cache.test.ts diff --git a/src/main/daemon/pty-subprocess-foreground-shared-snapshot-cache.test.ts b/src/main/daemon/pty-subprocess-foreground-shared-snapshot-cache.test.ts new file mode 100644 index 00000000000..35603eb071f --- /dev/null +++ b/src/main/daemon/pty-subprocess-foreground-shared-snapshot-cache.test.ts @@ -0,0 +1,139 @@ +/** + * `getProcessTableSnapshot` is a process-wide 500ms cache shared by every pane + * (process-table-snapshot.ts), so a refresh can be answered from a table another pane + * captured before this pane's agent existed. The scan's own `startedAt` is the time it + * ASKED, not the time the table was read, and both ordering guards in + * foreground-identity-refresh compare against it. Driven through the real resolver and + * the real cache — a resolver double cannot expose the gap, because the gap is the cache. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as pty from 'node-pty' +import type * as childProcess from 'node:child_process' + +const execFileMock = vi.hoisted(() => vi.fn()) +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, execFile: execFileMock } +}) + +import { createDaemonPtySubprocessHandle } from './pty-subprocess/subprocess-handle' +import { buildDaemonInspectProcessResult } from './terminal-host-process-evidence' +import type { SubprocessHandle } from './session-subprocess-handle' +import { + getProcessTableSnapshot, + resetProcessTableSnapshotForTests +} from '../../shared/process-table-snapshot' + +const SHELL_PID = 999_999_611 +const AGENT_PID = 999_999_612 +const T0 = 1_700_000_000_000 + +/** A table with the pane's shell and no agent beneath it: available, and idle. */ +const NO_AGENT_TABLE = `${SHELL_PID} 1 Ss+ -zsh\n` +const WITH_AGENT_TABLE = `${SHELL_PID} 1 Ss -zsh\n${AGENT_PID} ${SHELL_PID} S+ codex\n` + +describe('foreground scan answered from another pane s cached snapshot', () => { + let platformDescriptor: PropertyDescriptor | undefined + let nodePty: pty.IPty & { process: string } + let handle: SubprocessHandle + let psOutput: string + + /** The verdict the completion monitor acts on. */ + function childrenVerdict(): string | undefined { + const observation = handle.observeForegroundProcess?.() + if (!observation) { + throw new Error('handle exposes no foreground evidence channel') + } + return buildDaemonInspectProcessResult(observation).processEvidence?.children.verdict + } + + async function settle(): Promise { + await vi.advanceTimersByTimeAsync(0) + await vi.advanceTimersByTimeAsync(0) + } + + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(T0) + resetProcessTableSnapshotForTests() + platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' }) + psOutput = NO_AGENT_TABLE + execFileMock.mockReset() + execFileMock.mockImplementation( + ( + _command: string, + _args: readonly string[], + _options: unknown, + callback: (error: null, result: { stdout: string }) => void + ) => { + callback(null, { stdout: psOutput }) + } + ) + nodePty = { + pid: SHELL_PID, + process: 'zsh', + onData: vi.fn(() => ({ dispose: vi.fn() })), + onExit: vi.fn(() => ({ dispose: vi.fn() })), + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn() + } as unknown as pty.IPty & { process: string } + handle = createDaemonPtySubprocessHandle({ + process: nodePty, + shellPath: '/bin/zsh', + spawnCwd: '/tmp/wt', + env: { PATH: '/usr/bin' }, + startupCommandDeliveredInShellArgs: false, + reportsChildExitStatus: true, + requestedCwd: '/tmp/wt', + sessionId: 'repo-cache::/tmp/wt@@cache01', + startupAgentRecognition: null + }) + }) + + afterEach(() => { + resetProcessTableSnapshotForTests() + vi.useRealTimers() + if (platformDescriptor) { + Object.defineProperty(process, 'platform', platformDescriptor) + } + vi.restoreAllMocks() + }) + + it('does not publish a live agent as an idle shell when the table predates it', async () => { + // This pane's own first scan, so its refresh throttle is armed from T0. + handle.observeForegroundProcess?.() + await settle() + expect(execFileMock).toHaveBeenCalledTimes(1) + + // T0+900: ANOTHER pane scans. Its snapshot — still agent-free — becomes the shared + // cache entry, and it is the one this pane will be answered from. + vi.setSystemTime(T0 + 900) + await getProcessTableSnapshot() + expect(execFileMock).toHaveBeenCalledTimes(2) + + // T0+950: the agent starts. The sync title fast path stamps it without any scan. + vi.setSystemTime(T0 + 950) + psOutput = WITH_AGENT_TABLE + nodePty.process = 'codex' + expect(handle.observeForegroundProcess?.()?.evidence).toEqual({ + verdict: 'observed', + processName: 'codex' + }) + + // T0+1000: node-pty's title read degrades back to the spawned shell, and the pane's + // refresh throttle (1s) is up, so a scan runs — and is served the T0+900 table, + // which never saw the agent. No third `ps` fork proves the cache answered. + vi.setSystemTime(T0 + 1000) + nodePty.process = 'zsh' + handle.observeForegroundProcess?.() + await settle() + expect(execFileMock).toHaveBeenCalledTimes(2) + + // The agent is running. Reporting `exited` here is the completion monitor's cue to + // publish the agent as finished. + vi.setSystemTime(T0 + 1100) + expect(childrenVerdict()).toBe('live') + }) +}) diff --git a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts index f9e90d18a2c..c717028b8cc 100644 --- a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts +++ b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts @@ -137,13 +137,22 @@ export function createForegroundIdentityRefresh(args: { } state.refreshInFlight = true state.lastRefreshStartedAt = now + // Why not `now` everywhere below: the POSIX process table is a process-wide cache + // shared by every pane, so this scan can be answered from one another pane started + // earlier. `now` is when this pane ASKED; the table can be a TTL older, and an agent + // that began in between is simply missing from it. Replaced by the measured scan + // start once the resolver reports one. + let scanStartedAt = now const identityOlderThan = (ms: number): boolean => state.cachedAgentForeground !== null && Date.now() - state.cachedAgentForeground.refreshedAt > ms const retireStaleForegroundIdentity = ({ onlyWhenAged = false } = {}): void => { - // This scan sampled the process table at `now`; an identity stamped after that is - // evidence it never had a chance to see, so it cannot be retired on this answer. - if (state.cachedAgentForeground !== null && state.cachedAgentForeground.refreshedAt > now) { + // This scan sampled the process table at `scanStartedAt`; an identity stamped after + // that is evidence it never had a chance to see, so it cannot be retired on this answer. + if ( + state.cachedAgentForeground !== null && + state.cachedAgentForeground.refreshedAt > scanStartedAt + ) { return } const currentFallbackProcess = args.getFallbackProcess() @@ -171,81 +180,84 @@ export function createForegroundIdentityRefresh(args: { ? { anchorProcessId: anchor.pid, anchorProcessName: anchor.processName } : {}) }) - .then(({ processName, processId, available, anchorPidForeign }) => { - state.lastScanSettlement = { - at: Date.now(), - startedAt: now, - available, - sawAgent: available && recognizeAgentProcess(processName) !== null - } - if (args.isDead() || !available) { - return - } - if (!processName || !recognizeAgentProcess(processName)) { - if ( - process.platform === 'win32' && - fallbackIsShell && - state.cachedAgentForeground !== null - ) { - // Job, not console: needs no console attachment, so no fork (#10857). - const verdict = judgeCachedAgentJobEvidence({ - jobProcessIds: readWindowsPtyJobProcessIds(proc), - jobSupported: isWindowsPtyJobReadable(), - shellPid: proc.pid, - anchorProcessId: state.cachedAgentForeground.pid, - identityAgeMs: Date.now() - state.cachedAgentForeground.refreshedAt - }) - // Unverifiable is never exit proof (ssh-execution-boundary.md): hold. - if (verdict === 'unavailable') { - return - } - if (verdict === 'unsupported') { - // No job to consult on this build, and the scan that got here was - // available and found no agent. Trust it, as every other platform - // does, rather than holding a dead name forever (#16059). - retireStaleForegroundIdentity() - return - } - if (verdict === 'confirmed' || verdict === 'recheck') { - if (anchorPidForeign === true) { - // The scan proved the pid recycled to a non-agent: retire now. + .then( + ({ processName, processId, available, anchorPidForeign, tableScanStartedAtMs }) => { + scanStartedAt = tableScanStartedAtMs ?? now + state.lastScanSettlement = { + at: Date.now(), + startedAt: scanStartedAt, + available, + sawAgent: available && recognizeAgentProcess(processName) !== null + } + if (args.isDead() || !available) { + return + } + if (!processName || !recognizeAgentProcess(processName)) { + if ( + process.platform === 'win32' && + fallbackIsShell && + state.cachedAgentForeground !== null + ) { + // Job, not console: needs no console attachment, so no fork (#10857). + const verdict = judgeCachedAgentJobEvidence({ + jobProcessIds: readWindowsPtyJobProcessIds(proc), + jobSupported: isWindowsPtyJobReadable(), + shellPid: proc.pid, + anchorProcessId: state.cachedAgentForeground.pid, + identityAgeMs: Date.now() - state.cachedAgentForeground.refreshedAt + }) + // Unverifiable is never exit proof (ssh-execution-boundary.md): hold. + if (verdict === 'unavailable') { + return + } + if (verdict === 'unsupported') { + // No job to consult on this build, and the scan that got here was + // available and found no agent. Trust it, as every other platform + // does, rather than holding a dead name forever (#16059). retireStaleForegroundIdentity() return } - // The anchor pid is still in the job: the scan lost the row, not - // the agent. Restamp so a live agent never ages out (#9258). - state.cachedAgentForeground = { - ...state.cachedAgentForeground, - refreshedAt: Date.now() + if (verdict === 'confirmed' || verdict === 'recheck') { + if (anchorPidForeign === true) { + // The scan proved the pid recycled to a non-agent: retire now. + retireStaleForegroundIdentity() + return + } + // The anchor pid is still in the job: the scan lost the row, not + // the agent. Restamp so a live agent never ages out (#9258). + state.cachedAgentForeground = { + ...state.cachedAgentForeground, + refreshedAt: Date.now() + } + return } + if (verdict === 'exited' || verdict === 'anchor-exited') { + // Safe mid-restart: an available scan already found no agent. + retireStaleForegroundIdentity() + return + } + // Unanchored superset evidence cannot tell a working agent from a + // leftover; the age bound settles it. + retireStaleForegroundIdentity({ onlyWhenAged: true }) return } - if (verdict === 'exited' || verdict === 'anchor-exited') { - // Safe mid-restart: an available scan already found no agent. - retireStaleForegroundIdentity() - return - } - // Unanchored superset evidence cannot tell a working agent from a - // leftover; the age bound settles it. - retireStaleForegroundIdentity({ onlyWhenAged: true }) + retireStaleForegroundIdentity() return } - retireStaleForegroundIdentity() - return + state.cachedAgentForeground = { + processName, + pid: processId ?? null, + refreshedAt: Date.now() + } + state.startupAgentForeground = null + return processName } - state.cachedAgentForeground = { - processName, - pid: processId ?? null, - refreshedAt: Date.now() - } - state.startupAgentForeground = null - return processName - }) + ) .catch(() => { // Best-effort only: foreground enrichment must never affect PTY health. state.lastScanSettlement = { at: Date.now(), - startedAt: now, + startedAt: scanStartedAt, available: false, sawAgent: false } diff --git a/src/main/providers/agent-foreground-process-snapshot-availability.test.ts b/src/main/providers/agent-foreground-process-snapshot-availability.test.ts index 3331f4409b7..a6a20bd0921 100644 --- a/src/main/providers/agent-foreground-process-snapshot-availability.test.ts +++ b/src/main/providers/agent-foreground-process-snapshot-availability.test.ts @@ -20,6 +20,8 @@ function mockPs(stdout: string): void { }) } +const SCAN_AT_MS = 1_700_000_000_000 + /** A cached snapshot that never contained the pane observed nothing about it: * it must degrade (`available: false`) instead of reading as "no agent". */ describe('cached snapshot pane availability', () => { @@ -50,11 +52,21 @@ describe('cached snapshot pane availability', () => { }) it('still scans a cached snapshot that has the pane children but lost the shell row', async () => { + vi.useFakeTimers() + vi.setSystemTime(SCAN_AT_MS) mockPs('101 100 S+ node /Users/dev/.nvm/versions/node/bin/codex') - await expect(resolveAgentForegroundProcessWithAvailability(100, 'node')).resolves.toEqual({ - available: true, - processName: 'codex' - }) + try { + await expect(resolveAgentForegroundProcessWithAvailability(100, 'node')).resolves.toEqual({ + available: true, + processName: 'codex', + // The scan's own start travels with the rows. A pane served this snapshot from + // the shared cache asks later than this, so it must order the table by when the + // table was read — its own clock would claim the rows are newer than they are. + tableScanStartedAtMs: SCAN_AT_MS + }) + } finally { + vi.useRealTimers() + } }) }) diff --git a/src/main/providers/agent-foreground-process.ts b/src/main/providers/agent-foreground-process.ts index 089630d18b8..89788bdd3f2 100644 --- a/src/main/providers/agent-foreground-process.ts +++ b/src/main/providers/agent-foreground-process.ts @@ -2,7 +2,8 @@ import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process import { resolveOuterWrapperForegroundProcess } from '../../shared/foreground-wrapper-agent' import { getFreshProcessTableSnapshot, - getProcessTableSnapshot, + getFreshProcessTableSnapshotProvenance, + getProcessTableSnapshotProvenance, type ProcessTableRow } from '../../shared/process-table-snapshot' import { @@ -25,6 +26,10 @@ export type AgentForegroundProcessResolution = { processId?: number /** Windows: the scan proved the caller's `anchorProcessId` is now a non-agent. */ anchorPidForeign?: boolean + /** When the scan behind this answer began. Present only for POSIX process-table + * reads, which share a process-wide cache: the caller's own clock can be up to a + * TTL newer than the table it was handed, so ordering must use this instead. */ + tableScanStartedAtMs?: number } type ShellForegroundConfirmationOptions = { @@ -167,9 +172,9 @@ export async function resolveAgentForegroundProcessWithAvailability( } try { - const rows = options.fresh - ? await getFreshProcessTableSnapshot() - : await getProcessTableSnapshot() + const { value: rows, scanStartedAtMs } = options.fresh + ? await getFreshProcessTableSnapshotProvenance() + : await getProcessTableSnapshotProvenance() if (options.fresh && !rows.some((row) => row.pid === shellPid)) { return { available: false, processName: fallbackProcess } } @@ -181,7 +186,8 @@ export async function resolveAgentForegroundProcessWithAvailability( } return { available: true, - processName: resolveAgentForegroundProcessFromPs(rows, shellPid) ?? fallbackProcess + processName: resolveAgentForegroundProcessFromPs(rows, shellPid) ?? fallbackProcess, + tableScanStartedAtMs: scanStartedAtMs } } catch { // Why: a failed scan cannot prove fallback ownership; callers retain the last recognized agent. diff --git a/src/shared/process-table-snapshot.ts b/src/shared/process-table-snapshot.ts index 9827d1e8523..7a349f207c4 100644 --- a/src/shared/process-table-snapshot.ts +++ b/src/shared/process-table-snapshot.ts @@ -50,7 +50,7 @@ export function parseProcessTableRows(stdout: string): ProcessTableRow[] { return rows } -type Snapshot = { value: T; capturedAtMs: number } +type Snapshot = { value: T; capturedAtMs: number; scanStartedAtMs: number } type ProcessTableSnapshotReaderDeps = { runPs: () => Promise @@ -71,23 +71,30 @@ export function createProcessTableSnapshotReader( ): { getSnapshot: () => Promise getFreshSnapshot: () => Promise + getSnapshotProvenance: () => Promise> + getFreshSnapshotProvenance: () => Promise> reset: () => void } { const ttlMs = deps.ttlMs ?? DEFAULT_SNAPSHOT_TTL_MS let cached: Snapshot | null = null - let inFlight: Promise | null = null + let inFlight: Promise> | null = null let sequence = 0 - let freshQueued: { promise: Promise; startSequence: number | null } | null = null + let freshQueued: { promise: Promise>; startSequence: number | null } | null = null - async function runSnapshot(): Promise { - const promise = deps.runPs() + async function runSnapshot(): Promise> { + // Why both stamps: TTL reuse must not hand back a snapshot already older than + // its window, so freshness dates the answer. But ordering this table against + // other evidence needs the earliest moment it could have been read — a process + // that started after the scan began may be missing from it either way. + const scanStartedAtMs = deps.now() + const promise = deps.runPs().then((value) => { + const snapshot: Snapshot = { value, capturedAtMs: deps.now(), scanStartedAtMs } + cached = snapshot + return snapshot + }) inFlight = promise try { - const value = await promise - // Why: stamp capture time AFTER the scan returns so a slow scan can't - // hand back a snapshot that is already older than its TTL. - cached = { value, capturedAtMs: deps.now() } - return value + return await promise } finally { if (inFlight === promise) { inFlight = null @@ -95,9 +102,9 @@ export function createProcessTableSnapshotReader( } } - async function getSnapshot(): Promise { + async function getSnapshot(): Promise> { if (cached && deps.now() - cached.capturedAtMs < ttlMs) { - return cached.value + return cached } if (inFlight) { return inFlight @@ -110,14 +117,14 @@ export function createProcessTableSnapshotReader( return runSnapshot() } - function getFreshSnapshot(): Promise { + function getFreshSnapshot(): Promise> { const requestSequence = ++sequence if (freshQueued?.startSequence === null) { return freshQueued.promise } const priorFresh = freshQueued?.promise ?? null const priorScan = inFlight - const entry: { promise: Promise; startSequence: number | null } = { + const entry: { promise: Promise>; startSequence: number | null } = { promise: Promise.resolve(undefined as never), startSequence: null } @@ -152,8 +159,11 @@ export function createProcessTableSnapshotReader( } return { - getSnapshot, - getFreshSnapshot, + getSnapshot: async () => (await getSnapshot()).value, + getFreshSnapshot: async () => (await getFreshSnapshot()).value, + /** The same reads, with the provenance an ordering comparison needs. */ + getSnapshotProvenance: getSnapshot, + getFreshSnapshotProvenance: getFreshSnapshot, // Why: lets tests that mock `ps` per case clear the cross-call cache so one // case's snapshot can't satisfy the next within the TTL window. reset: () => { @@ -193,6 +203,21 @@ export function getFreshProcessTableSnapshot(): Promise { return defaultReader.getFreshSnapshot() } +export type ProcessTableSnapshotProvenance = { + value: ProcessTableRow[] + /** When the scan behind these rows began. A pane reading a cached snapshot asked + * later than this, so its own clock overstates how current the table is. */ + scanStartedAtMs: number +} + +export function getProcessTableSnapshotProvenance(): Promise { + return defaultReader.getSnapshotProvenance() +} + +export function getFreshProcessTableSnapshotProvenance(): Promise { + return defaultReader.getFreshSnapshotProvenance() +} + /** * Test-only: clear the shared snapshot cache so suites that mock `ps` between * cases don't have one case's snapshot served to the next within the TTL. From 09fcecfcfd8a72d01d49f96e0b931e63cfbed060 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 18:30:24 -0700 Subject: [PATCH 09/10] fix(daemon): treat equal scan and identity stamps as unordered, not as proof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both ordering guards on the shell-title corroboration path accepted the ambiguous equal case. At millisecond resolution a table captured in the agent's own millisecond is unordered against it, not simultaneous — it may never have sampled the agent. Read as "at least as new", a pre-agent scan retired the live identity and then corroborated the degraded shell title, publishing `exited` for a running agent. Both comparisons now require the scan to be strictly newer, so the tie keeps the agent and an unknown order resolves to `unverifiable` rather than to an exit. --- ...s-foreground-shared-snapshot-cache.test.ts | 56 +++++++++++++++++++ .../foreground-identity-refresh.ts | 15 +++-- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/main/daemon/pty-subprocess-foreground-shared-snapshot-cache.test.ts b/src/main/daemon/pty-subprocess-foreground-shared-snapshot-cache.test.ts index 35603eb071f..e39f03c5acf 100644 --- a/src/main/daemon/pty-subprocess-foreground-shared-snapshot-cache.test.ts +++ b/src/main/daemon/pty-subprocess-foreground-shared-snapshot-cache.test.ts @@ -136,4 +136,60 @@ describe('foreground scan answered from another pane s cached snapshot', () => { vi.setSystemTime(T0 + 1100) expect(childrenVerdict()).toBe('live') }) + + it('does not let a table captured in the agent s own millisecond retire it', async () => { + // Same shape, with the cached scan and the agent stamped at the same millisecond. + // Date.now() cannot order two events inside one tick, and both guards read that tie as + // "the scan is at least as new as the agent" — the one reading that publishes an exit. + handle.observeForegroundProcess?.() + await settle() + expect(execFileMock).toHaveBeenCalledTimes(1) + + // T0+900: another pane captures an agent-free table, and in that same millisecond the + // agent starts and the title fast path stamps it. Neither observed the other. + vi.setSystemTime(T0 + 900) + await getProcessTableSnapshot() + expect(execFileMock).toHaveBeenCalledTimes(2) + psOutput = WITH_AGENT_TABLE + nodePty.process = 'codex' + expect(handle.observeForegroundProcess?.()?.evidence).toEqual({ + verdict: 'observed', + processName: 'codex' + }) + + // T0+1000: the title read degrades to the shell and the throttle is up, so a scan runs + // and is answered from that same-millisecond table. No third fork: the cache answered. + vi.setSystemTime(T0 + 1000) + nodePty.process = 'zsh' + handle.observeForegroundProcess?.() + await settle() + expect(execFileMock).toHaveBeenCalledTimes(2) + + vi.setSystemTime(T0 + 1100) + expect(childrenVerdict()).toBe('live') + }) + + it('does not corroborate a shell title with a table from the agent s own millisecond', async () => { + // Past the identity's 1s TTL the corroboration guard is what answers, and it faces the + // same tie: the only settled scan started in the millisecond the agent was stamped. + handle.observeForegroundProcess?.() + await settle() + + vi.setSystemTime(T0 + 900) + await getProcessTableSnapshot() + psOutput = WITH_AGENT_TABLE + nodePty.process = 'codex' + handle.observeForegroundProcess?.() + + vi.setSystemTime(T0 + 1000) + nodePty.process = 'zsh' + handle.observeForegroundProcess?.() + await settle() + expect(execFileMock).toHaveBeenCalledTimes(2) + + // T0+2000: the identity is older than its TTL, so the shell title now needs the scan to + // corroborate it. That scan cannot: it may never have sampled the agent. + vi.setSystemTime(T0 + 2000) + expect(childrenVerdict()).toBe('unverifiable') + }) }) diff --git a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts index c717028b8cc..89acbfbdf25 100644 --- a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts +++ b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts @@ -78,6 +78,11 @@ export function getActiveStartupAgent( * inside the window while an agent that started after it is live. Compared against * `startedAt`, not `at` — a scan that settles after the agent appeared still read a * process table from before it. + * + * Strictly newer, because equal stamps are unordered rather than simultaneous: Date.now() + * cannot separate two events inside one millisecond, so a table captured in the agent's own + * millisecond may or may not have seen it. Only a scan that demonstrably started after the + * agent was stamped can speak for its absence; the tie keeps the agent. */ export function isShellTitleCorroborated( settlement: ForegroundScanSettlement | null, @@ -89,7 +94,7 @@ export function isShellTitleCorroborated( settlement.available && !settlement.sawAgent && now - settlement.at <= SHELL_TITLE_SCAN_CORROBORATION_MAX_AGE_MS && - (agentEvidence === null || settlement.startedAt >= agentEvidence.refreshedAt) + (agentEvidence === null || settlement.startedAt > agentEvidence.refreshedAt) ) } @@ -147,11 +152,13 @@ export function createForegroundIdentityRefresh(args: { state.cachedAgentForeground !== null && Date.now() - state.cachedAgentForeground.refreshedAt > ms const retireStaleForegroundIdentity = ({ onlyWhenAged = false } = {}): void => { - // This scan sampled the process table at `scanStartedAt`; an identity stamped after - // that is evidence it never had a chance to see, so it cannot be retired on this answer. + // This scan sampled the process table at `scanStartedAt`; an identity stamped at or + // after that is evidence it may never have had a chance to see, so it cannot be retired + // on this answer. Equal stamps are unordered, not simultaneous — Date.now() cannot say + // which came first inside a millisecond — and an unknown order is not proof of an exit. if ( state.cachedAgentForeground !== null && - state.cachedAgentForeground.refreshedAt > scanStartedAt + state.cachedAgentForeground.refreshedAt >= scanStartedAt ) { return } From 8ef1975783dd5641f9e88d23c8bbf9e5832d7941 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 19:19:41 -0700 Subject: [PATCH 10/10] fix(daemon): stop a confirmation scan retiring agent evidence it never saw confirmForegroundProcess cleared the pane's agent identity on any answer that named no agent, without comparing the scan's provenance. A scan that starts before the agent is recognized and resolves after it read a process table that could not have contained the agent, so its silence retired a live one. The cost is not only the lost name: retirement nulls cachedAgentForeground, and the shell-title corroboration gate treats absent agent evidence as nothing to outrank, so the next degraded shell title is read as an observed idle pane. Guarded with the same comparison the refresh path already makes, now extracted as agentEvidenceOutdatesScan and shared by all three readers. --- ...foreground-confirmation-provenance.test.ts | 209 ++++++++++++++++++ .../foreground-identity-refresh.ts | 23 +- .../foreground-process-tracker.ts | 19 +- 3 files changed, 244 insertions(+), 7 deletions(-) create mode 100644 src/main/daemon/pty-subprocess-foreground-confirmation-provenance.test.ts diff --git a/src/main/daemon/pty-subprocess-foreground-confirmation-provenance.test.ts b/src/main/daemon/pty-subprocess-foreground-confirmation-provenance.test.ts new file mode 100644 index 00000000000..a2d48da95f2 --- /dev/null +++ b/src/main/daemon/pty-subprocess-foreground-confirmation-provenance.test.ts @@ -0,0 +1,209 @@ +/* The confirmation scan's provenance. Everything below the `ps` fork is real: the shared + * process-table snapshot cache (its TTL, its dedupe and its scan-start stamps), the agent + * resolver that reads it, and the tracker that keeps or retires the pane's identity. Only the + * OS scan itself is substituted, so the orderings under test are the ones production has. */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { execFileMock } = vi.hoisted(() => { + const mock = vi.fn() + // promisify() runs while the snapshot module is evaluated — before this file's body — so the + // custom hook has to be attached here, or the reader awaits a bare stdout instead of + // `{ stdout }` and every scan resolves to undefined. + Object.defineProperty(mock, Symbol.for('nodejs.util.promisify.custom'), { + value: mock, + configurable: true, + writable: true + }) + return { execFileMock: mock } +}) + +vi.mock('node:child_process', () => ({ execFile: execFileMock })) + +import { createPtyForegroundProcessTracker } from './pty-subprocess/foreground-process-tracker' +import { resetProcessTableSnapshotForTests } from '../../shared/process-table-snapshot' +import type * as pty from 'node-pty' + +const SHELL_PID = 4242 +const SHELL_PATH = '/bin/zsh' +// Why not 0: the refresh throttle compares `Date.now()` against a `lastRefreshStartedAt` that +// initialises to 0, so a clock parked at the epoch throttles away the pane's first scan. +const BASE = 1_700_000_000_000 + +/** `ps -axo pid=,ppid=,stat=,command=` rows for a pane with no agent under its shell. */ +function agentFreeTable(): string { + return `${SHELL_PID} 1 Ss+ ${SHELL_PATH}\n` +} + +type Deferred = { resolve: (stdout: string) => void } + +/** Queues one controllable `ps` answer per scan, so a scan can be held open across other work. */ +function queueScans(): { next: () => Deferred; settleAll: (stdout: string) => Promise } { + const pending: Deferred[] = [] + execFileMock.mockImplementation( + () => + new Promise<{ stdout: string }>((resolvePromise) => { + pending.push({ resolve: (stdout) => resolvePromise({ stdout }) }) + }) + ) + return { + next: () => { + const deferred = pending.shift() + if (!deferred) { + throw new Error('no ps scan was started') + } + return deferred + }, + settleAll: async (stdout) => { + // Flush first: the scan is only registered once the resolver's own awaits have run. + await flush() + while (pending.length > 0) { + pending.shift()!.resolve(stdout) + await flush() + } + } + } +} + +async function flush(): Promise { + for (let i = 0; i < 12; i++) { + await Promise.resolve() + } +} + +function trackerFor(ptyTitle: { + value: string +}): ReturnType { + const fakePty = { + pid: SHELL_PID, + get process() { + return ptyTitle.value + } + } as unknown as pty.IPty + return createPtyForegroundProcessTracker({ + process: fakePty, + shellPath: SHELL_PATH, + sessionId: 'wt-1::/repo@@abc', + startupAgentRecognition: null, + isDead: () => false + }) +} + +describe('a confirmation scan that resolves after an agent was recognized', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(BASE) + resetProcessTableSnapshotForTests() + execFileMock.mockReset() + }) + + afterEach(() => { + vi.useRealTimers() + resetProcessTableSnapshotForTests() + }) + + it('does not retire the newer live-agent identity it never had a chance to see', async () => { + const title = { value: 'zsh' } + const tracker = trackerFor(title) + const scans = queueScans() + + // A settled, agent-free scan first: this is what later gives a shell-shaped title the + // corroboration it needs, and it is only safe while no newer agent evidence outranks it. + vi.setSystemTime(BASE) + tracker.observeForegroundProcess() + await scans.settleAll(agentFreeTable()) + + // The confirmation scan starts here, and its process table is sampled now. + vi.setSystemTime(BASE + 10) + const confirmation = tracker.confirmForegroundProcess() + await flush() + const confirmationScan = scans.next() + + // While it is out, the agent starts and the pty title names it. The synchronous fast path + // stamps it without a scan, so this identity is strictly newer than the table above. + vi.setSystemTime(BASE + 20) + title.value = 'claude' + expect(tracker.observeForegroundProcess().processName).toBe('claude') + + // Only now does the older scan answer, reporting a table taken before the agent existed. + vi.setSystemTime(BASE + 25) + title.value = 'zsh' + confirmationScan.resolve(agentFreeTable()) + await confirmation + await flush() + + // Inside the identity's 1s TTL the cache answers directly, so this reads whether the + // confirmation retired it at all. + vi.setSystemTime(BASE + 100) + expect(tracker.observeForegroundProcess().processName).toBe('claude') + + // Past the TTL the fast path can no longer answer, which is where the retirement's real + // cost lands: whether a shell-shaped title now counts as corroborated exit evidence. + vi.setSystemTime(BASE + 1_500) + const observation = tracker.observeForegroundProcess() + + expect( + observation.evidence.verdict, + 'a scan that sampled the table before the agent started cannot report it gone' + ).toBe('unverifiable') + }) + + /** The caller's own clock is not the table's. A fresh scan is queued behind whatever is + * already running, so an agent can be stamped after this call was made and still be older + * than the process table the answer is built from — in which case the scan really did look + * for it. Ordering on the request time instead would hold a dead agent name forever. */ + it("retires an identity the scan's own table was late enough to have seen", async () => { + const title = { value: 'zsh' } + const tracker = trackerFor(title) + const scans = queueScans() + + vi.setSystemTime(BASE) + tracker.observeForegroundProcess() + await scans.settleAll(agentFreeTable()) + + // The confirmation is requested here, but its scan has not started yet. + vi.setSystemTime(BASE + 50) + const confirmation = tracker.confirmForegroundProcess() + + // The agent is stamped after the request... + vi.setSystemTime(BASE + 60) + title.value = 'claude' + expect(tracker.observeForegroundProcess().processName).toBe('claude') + + // ...but before the queued scan actually samples the table, so that table did see the pane + // as it is now and its silence about an agent is a real observation. + vi.setSystemTime(BASE + 70) + title.value = 'zsh' + await scans.settleAll(agentFreeTable()) + await confirmation + await flush() + + // Inside the TTL, so a surviving identity would answer here: it must not. + vi.setSystemTime(BASE + 100) + expect(tracker.observeForegroundProcess().processName).toBe('zsh') + }) + + it('still retires an identity a later scan really did look for', async () => { + const title = { value: 'claude' } + const tracker = trackerFor(title) + const scans = queueScans() + + // The agent is recognized first... + vi.setSystemTime(BASE) + expect(tracker.observeForegroundProcess().processName).toBe('claude') + + // ...and the confirmation scan starts strictly after it, so its table did see the pane + // as the agent left it. This answer is entitled to retire the identity. + vi.setSystemTime(BASE + 50) + title.value = 'zsh' + const confirmation = tracker.confirmForegroundProcess() + await flush() + await scans.settleAll(agentFreeTable()) + await confirmation + await flush() + + // Inside the TTL the cached identity would still answer if it had been kept. + vi.setSystemTime(BASE + 100) + + expect(tracker.observeForegroundProcess().processName).toBe('zsh') + }) +}) diff --git a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts index 89acbfbdf25..53415f1d3c9 100644 --- a/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts +++ b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts @@ -94,10 +94,26 @@ export function isShellTitleCorroborated( settlement.available && !settlement.sawAgent && now - settlement.at <= SHELL_TITLE_SCAN_CORROBORATION_MAX_AGE_MS && - (agentEvidence === null || settlement.startedAt > agentEvidence.refreshedAt) + !agentEvidenceOutdatesScan(agentEvidence, settlement.startedAt) ) } +/** + * Whether the pane's agent identity postdates the process table a scan read. Such a scan cannot + * speak for that agent's absence: the table it sampled at `scanStartedAt` may never have had the + * chance to contain it, so its silence is not an observation. + * + * Strictly newer, because equal stamps are unordered rather than simultaneous: Date.now() cannot + * separate two events inside one millisecond, so a table captured in the agent's own millisecond + * may or may not have seen it. An unknown order is not proof of an exit; the tie keeps the agent. + */ +export function agentEvidenceOutdatesScan( + agentEvidence: CachedAgentForeground | null, + scanStartedAt: number +): boolean { + return agentEvidence !== null && agentEvidence.refreshedAt >= scanStartedAt +} + /** * The throttled async identity scan behind the tracker's synchronous reads: * resolves the pane's real foreground agent through the process table, keeps @@ -156,10 +172,7 @@ export function createForegroundIdentityRefresh(args: { // after that is evidence it may never have had a chance to see, so it cannot be retired // on this answer. Equal stamps are unordered, not simultaneous — Date.now() cannot say // which came first inside a millisecond — and an unknown order is not proof of an exit. - if ( - state.cachedAgentForeground !== null && - state.cachedAgentForeground.refreshedAt >= scanStartedAt - ) { + if (agentEvidenceOutdatesScan(state.cachedAgentForeground, scanStartedAt)) { return } const currentFallbackProcess = args.getFallbackProcess() diff --git a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts index 9ef7d97865a..7f281d85514 100644 --- a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts +++ b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts @@ -19,6 +19,7 @@ import type { ForegroundProcessObservation } from '../session-subprocess-handle' import { createForegroundIdentityRefresh, FOREGROUND_AGENT_CACHE_TTL_MS, + agentEvidenceOutdatesScan, getActiveStartupAgent, isShellTitleCorroborated, type ForegroundIdentityState @@ -175,6 +176,10 @@ export function createPtyForegroundProcessTracker(args: { ) { return fallbackProcess } + // Why stamped before the call: the POSIX table is a process-wide cache, so this answer + // can come from a scan another pane started. Replaced by the measured scan start when + // the resolver reports one; this is only the floor for the Windows path, which has none. + const scanRequestedAt = Date.now() const resolution = await resolveAgentForegroundProcessWithAvailability( proc.pid, fallbackProcess, @@ -203,8 +208,18 @@ export function createPtyForegroundProcessTracker(args: { state.startupAgentForeground = null return recognized.processName } - state.cachedAgentForeground = null - state.startupAgentForeground = null + // An agent recognized while this scan was in flight is evidence the scan's table could + // not have held, so this answer may not retire it — and retiring it would also strip the + // shell-title corroboration gate of the very evidence that holds it closed. + if ( + !agentEvidenceOutdatesScan( + state.cachedAgentForeground, + resolution.tableScanStartedAtMs ?? scanRequestedAt + ) + ) { + state.cachedAgentForeground = null + state.startupAgentForeground = null + } return resolution.processName } catch { return null