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-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-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 new file mode 100644 index 00000000000..41ca6c9ed10 --- /dev/null +++ b/src/main/daemon/pty-subprocess-foreground-observation-evidence.test.ts @@ -0,0 +1,312 @@ +// 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 { 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. +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 + 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 + > | 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' }) + exitListeners = [] + nodePty = { + pid: SHELL_PID, + process: 'zsh', + onData: 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() + } 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('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' + + expect(handle.observeForegroundProcess?.()?.evidence).toEqual({ + verdict: 'observed', + processName: 'codex' + }) + }) + + 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('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('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() + + // 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') + }) +}) 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..e39f03c5acf --- /dev/null +++ b/src/main/daemon/pty-subprocess-foreground-shared-snapshot-cache.test.ts @@ -0,0 +1,195 @@ +/** + * `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') + }) + + 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 new file mode 100644 index 00000000000..53415f1d3c9 --- /dev/null +++ b/src/main/daemon/pty-subprocess/foreground-identity-refresh.ts @@ -0,0 +1,289 @@ +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. + * `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 + 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. + * + * `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. + * + * 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, + now: number, + agentEvidence: CachedAgentForeground | null +): boolean { + return ( + settlement !== null && + settlement.available && + !settlement.sawAgent && + now - settlement.at <= SHELL_TITLE_SCAN_CORROBORATION_MAX_AGE_MS && + !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 + * 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 + // 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 `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 (agentEvidenceOutdatesScan(state.cachedAgentForeground, scanStartedAt)) { + return + } + 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, 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 + } + 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(), + startedAt: scanStartedAt, + 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..7f281d85514 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,23 @@ 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, + agentEvidenceOutdatesScan, + 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 +45,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, state.cachedAgentForeground) + ) { + 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 @@ -274,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, @@ -294,16 +200,26 @@ 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 + // 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 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/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. 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) } + } + }) + }) +}) 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' } + } + }) + }) +})