diff --git a/src/main/daemon/daemon-protocol-version.test.ts b/src/main/daemon/daemon-protocol-version.test.ts index dfe84947ff5..4b4b07ceb55 100644 --- a/src/main/daemon/daemon-protocol-version.test.ts +++ b/src/main/daemon/daemon-protocol-version.test.ts @@ -3,6 +3,7 @@ import { AGENT_SESSION_CLAIM_DAEMON_PROTOCOL_VERSION, AGENT_SESSION_CREATE_OPERATION_DAEMON_PROTOCOL_VERSION, ASYNC_CWD_VALIDATION_DAEMON_PROTOCOL_VERSION, + AUTHORITATIVE_COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, CODEX_SHELL_LAUNCH_PREFLIGHT_DAEMON_PROTOCOL_VERSION, COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, CONTENT_ADDRESSED_SHELL_WRAPPER_DAEMON_PROTOCOL_VERSION, @@ -19,7 +20,8 @@ import { describe('daemon protocol version', () => { it('ships bounded history transfer after the 2031-unsubscribe fact', () => { - expect(PROTOCOL_VERSION).toBe(36) + expect(PROTOCOL_VERSION).toBe(37) + expect(AUTHORITATIVE_COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION).toBe(37) expect(CONTENT_ADDRESSED_SHELL_WRAPPER_DAEMON_PROTOCOL_VERSION).toBe(36) expect(ASYNC_CWD_VALIDATION_DAEMON_PROTOCOL_VERSION).toBe(35) expect(CODEX_SHELL_LAUNCH_PREFLIGHT_DAEMON_PROTOCOL_VERSION).toBe(34) @@ -33,7 +35,7 @@ describe('daemon protocol version', () => { expect(AGENT_SESSION_CLAIM_DAEMON_PROTOCOL_VERSION).toBe(26) expect(AGENT_SESSION_CREATE_OPERATION_DAEMON_PROTOCOL_VERSION).toBe(26) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toEqual( - Array.from({ length: 35 }, (_, index) => index + 1) + Array.from({ length: 36 }, (_, index) => index + 1) ) }) diff --git a/src/main/daemon/daemon-protocol-version.ts b/src/main/daemon/daemon-protocol-version.ts index bae2ae972dc..5334d1d37b8 100644 --- a/src/main/daemon/daemon-protocol-version.ts +++ b/src/main/daemon/daemon-protocol-version.ts @@ -1,6 +1,7 @@ // Why: daemons survive app updates, so wire behavior must be version-gated. -// v36 launches shells from content-addressed wrapper trees; older owners stay attachable. -export const PROTOCOL_VERSION = 36 +// v37 makes completion-sensitive process inspection fail closed; older owners stay attachable. +export const PROTOCOL_VERSION = 37 +export const AUTHORITATIVE_COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION = 37 export const CONTENT_ADDRESSED_SHELL_WRAPPER_DAEMON_PROTOCOL_VERSION = 36 export const ASYNC_CWD_VALIDATION_DAEMON_PROTOCOL_VERSION = 35 export const CODEX_SHELL_LAUNCH_PREFLIGHT_DAEMON_PROTOCOL_VERSION = 34 @@ -30,7 +31,7 @@ export const CLEAN_DISCONNECT_PROTOCOL_VERSION = 24 export const MODE_2031_UNSUBSCRIBE_FACT_PROTOCOL_VERSION = 29 export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, - 28, 29, 30, 31, 32, 33, 34, 35 + 28, 29, 30, 31, 32, 33, 34, 35, 36 ] as const export function supportsPtyStartupIngress(protocolVersion: number): boolean { diff --git a/src/main/daemon/daemon-pty-adapter-protocol-compatibility.test.ts b/src/main/daemon/daemon-pty-adapter-protocol-compatibility.test.ts index aa47fac186e..e37ea3922c0 100644 --- a/src/main/daemon/daemon-pty-adapter-protocol-compatibility.test.ts +++ b/src/main/daemon/daemon-pty-adapter-protocol-compatibility.test.ts @@ -5,6 +5,7 @@ import { DaemonClient } from './client' import { DaemonPtyAdapter } from './daemon-pty-adapter' import { LIVENESS_PROBE_TIMEOUT_MS } from './daemon-pty-session-control' import { + AUTHORITATIVE_COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, GET_FOREGROUND_PROCESS_PROTOCOL_VERSION, GET_SIZE_PROTOCOL_VERSION, @@ -623,7 +624,8 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(await legacy.inspectProcess('sess-a')).toEqual({ foregroundProcess: 'codex', - hasChildProcesses: true + hasChildProcesses: true, + unavailable: true }) expect(request).toHaveBeenCalledWith('getForegroundProcess', { sessionId: 'sess-a' }) expect(request).not.toHaveBeenCalledWith('inspectProcess', expect.anything()) @@ -639,7 +641,8 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(await legacy.inspectProcess('sess-a')).toEqual({ foregroundProcess: 'bash', - hasChildProcesses: false + hasChildProcesses: false, + unavailable: true }) legacy.dispose() @@ -653,7 +656,8 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(await legacy.inspectProcess('sess-a')).toEqual({ foregroundProcess: null, - hasChildProcesses: false + hasChildProcesses: false, + unavailable: true }) legacy.dispose() @@ -674,8 +678,25 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { legacy.dispose() }) - it.each([COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, PROTOCOL_VERSION])( - 'delegates protocol %s inspection to inspectProcess', + it.each([ + COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, + AUTHORITATIVE_COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION - 1 + ])('marks protocol %s inspectProcess results unavailable', async (protocolVersion) => { + const request = vi.fn(async () => ({ foregroundProcess: null, hasChildProcesses: false })) + const legacy = createInspectionAdapter(protocolVersion, request) + + await expect(legacy.inspectProcess('sess-a')).resolves.toEqual({ + foregroundProcess: null, + hasChildProcesses: false, + unavailable: true + }) + expect(request).toHaveBeenCalledWith('inspectProcess', { sessionId: 'sess-a' }) + + legacy.dispose() + }) + + it.each([AUTHORITATIVE_COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, PROTOCOL_VERSION])( + 'delegates authoritative protocol %s inspection to inspectProcess', async (protocolVersion) => { const request = vi.fn(async () => ({ foregroundProcess: 'codex', hasChildProcesses: true })) const current = createInspectionAdapter(protocolVersion, request) diff --git a/src/main/daemon/daemon-pty-process-inspection.ts b/src/main/daemon/daemon-pty-process-inspection.ts index bbc44568501..b1ad5669874 100644 --- a/src/main/daemon/daemon-pty-process-inspection.ts +++ b/src/main/daemon/daemon-pty-process-inspection.ts @@ -2,6 +2,7 @@ import { isShellProcess } from '../../shared/agent-detection' import { DaemonPtyBufferSnapshots } from './daemon-pty-buffer-snapshots' import { parsePtySessionId } from './pty-session-id' import { + AUTHORITATIVE_COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, GET_FOREGROUND_PROCESS_PROTOCOL_VERSION, type ListSessionsResult, @@ -27,22 +28,27 @@ export abstract class DaemonPtyProcessInspection extends DaemonPtyBufferSnapshot return { foregroundProcess: null, hasChildProcesses: true, unavailable: true } } if (this.protocolVersion < COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION) { - // Why: pre-v27 daemons survive an in-place app update; compose the inspection client-side from the - // one call they do support instead of throwing, or completion detection stays dead until recreate. - // Requests directly (not via getForegroundProcess) so a dead socket still rejects rather than - // reading as an idle foreground and dispatching a false completion. + // Why: pre-v27 daemons expose only a best-effort read that swallows scan failures into null. + // Preserve positive evidence, but never turn its shell/null result into completion authority. const { foregroundProcess } = await this.client.request<{ foregroundProcess: string | null }>('getForegroundProcess', { sessionId: id }) return { foregroundProcess, - hasChildProcesses: this.hasChildProcessesFromForeground(foregroundProcess) + hasChildProcesses: this.hasChildProcessesFromForeground(foregroundProcess), + unavailable: true } } - return this.client.request<{ + const inspection = await this.client.request<{ foregroundProcess: string | null hasChildProcesses: boolean }>('inspectProcess', { sessionId: id }) + return { + ...inspection, + ...(this.protocolVersion < AUTHORITATIVE_COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION + ? { unavailable: true as const } + : {}) + } } async getForegroundProcess(id: string): Promise { diff --git a/src/main/daemon/pty-subprocess/foreground-process-inspection.ts b/src/main/daemon/pty-subprocess/foreground-process-inspection.ts new file mode 100644 index 00000000000..c3121400347 --- /dev/null +++ b/src/main/daemon/pty-subprocess/foreground-process-inspection.ts @@ -0,0 +1,104 @@ +import type * as pty from 'node-pty' +import { recognizeAgentProcess } from '../../../shared/agent-process-recognition' +import { shouldInspectOuterWrapperForegroundProcess } from '../../../shared/foreground-wrapper-agent' +import { + inspectAgentPtyProcess, + resolveAgentForegroundProcessWithAvailability +} from '../../providers/agent-foreground-process' +import { readWindowsConsoleAttachedProcessIds } from '../../providers/windows-console-attached-processes' +import { readWindowsPtyJobProcessIds } from '../../providers/windows-pty-job-membership' + +type ForegroundInspectionArgs = { + process: pty.IPty + contextPaths: readonly string[] + isDead: () => boolean + getFallbackProcess: () => string | null + shouldInspectFallback: (fallbackProcess: string | null) => boolean + cachedAgentName: () => string | null + setRecognizedAgent: (processName: string, processId: number | null) => void + clearAgentEvidence: () => void +} + +export async function inspectTrackedForegroundProcess( + args: ForegroundInspectionArgs +): Promise<{ processName: string | null; available: boolean }> { + const proc = args.process + if (args.isDead() || !proc.pid) { + return { processName: null, available: false } + } + try { + const fallbackProcess = args.getFallbackProcess() + const fallbackRecognition = recognizeAgentProcess(fallbackProcess) + if ( + fallbackProcess && + ((fallbackRecognition !== null && + process.platform !== 'win32' && + !shouldInspectOuterWrapperForegroundProcess(fallbackRecognition)) || + (process.platform !== 'win32' && !args.shouldInspectFallback(fallbackProcess))) + ) { + return { processName: fallbackProcess, available: true } + } + const resolution = await resolveAgentForegroundProcessWithAvailability( + proc.pid, + fallbackProcess, + { + contextPaths: args.contextPaths, + fresh: true, + ...(process.platform === 'win32' + ? { + forceProcessScan: true, + readWindowsConsoleAttachedProcessIds: () => + readWindowsConsoleAttachedProcessIds(proc.pid) + } + : {}) + } + ) + if (args.isDead() || !resolution.available) { + return { processName: args.cachedAgentName() ?? fallbackProcess, available: false } + } + const recognized = recognizeAgentProcess(resolution.processName) + if (recognized) { + args.setRecognizedAgent(recognized.processName, resolution.processId ?? null) + return { processName: recognized.processName, available: true } + } + args.clearAgentEvidence() + return { processName: resolution.processName, available: true } + } catch { + return { processName: args.cachedAgentName(), available: false } + } +} + +export async function inspectTrackedPtyProcess(args: ForegroundInspectionArgs): Promise<{ + foregroundProcess: string | null + hasChildProcesses: boolean + unavailable?: true +}> { + const proc = args.process + if (args.isDead() || !proc.pid) { + return { foregroundProcess: null, hasChildProcesses: false, unavailable: true } + } + const inspection = await inspectAgentPtyProcess(proc.pid, args.getFallbackProcess(), { + contextPaths: args.contextPaths, + forceProcessScan: true, + ...(process.platform === 'win32' + ? { + readWindowsConsoleAttachedProcessIds: () => + readWindowsConsoleAttachedProcessIds(proc.pid), + readWindowsPtyJobProcessIds: () => readWindowsPtyJobProcessIds(proc) + } + : {}) + }) + if (inspection.available) { + const recognized = recognizeAgentProcess(inspection.processName) + if (recognized) { + args.setRecognizedAgent(recognized.processName, inspection.processId ?? null) + } else { + args.clearAgentEvidence() + } + } + return { + foregroundProcess: inspection.processName, + hasChildProcesses: inspection.hasChildProcesses, + ...(inspection.available ? {} : { unavailable: true }) + } +} diff --git a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts index 115b795202d..520b6f5c6c4 100644 --- a/src/main/daemon/pty-subprocess/foreground-process-tracker.ts +++ b/src/main/daemon/pty-subprocess/foreground-process-tracker.ts @@ -11,7 +11,6 @@ import { readWindowsPtyJobProcessIds } from '../../providers/windows-pty-job-membership' -import { readWindowsConsoleAttachedProcessIds } from '../../providers/windows-console-attached-processes' import { isAgentForegroundWrapperProcess, recognizeAgentProcess, @@ -24,6 +23,10 @@ import { import { isShellProcess } from '../../../shared/shell-process-detection' import { resolveFallbackForegroundProcess } from './foreground-fallback-process' import { parsePtySessionId } from '../pty-session-id' +import { + inspectTrackedForegroundProcess, + inspectTrackedPtyProcess +} from './foreground-process-inspection' const FOREGROUND_AGENT_CACHE_TTL_MS = 1000 const SHELL_FOREGROUND_REFRESH_RETRY_MS = 5_000 @@ -37,6 +40,11 @@ export type PtyForegroundProcessTracker = { recordOutput(data: string): void markDead(): void getForegroundProcess(): string | null + inspectProcess(): Promise<{ + foregroundProcess: string | null + hasChildProcesses: boolean + unavailable?: true + }> confirmForegroundProcess(): Promise confirmShellForeground(): Promise } @@ -203,6 +211,24 @@ export function createPtyForegroundProcessTracker(args: { }) } + const inspectionArgs = { + process: proc, + contextPaths, + isDead: args.isDead, + getFallbackProcess, + shouldInspectFallback, + cachedAgentName: () => cachedAgentForeground?.processName ?? null, + setRecognizedAgent: (processName: string, pid: number | null) => { + cachedAgentForeground = { processName, pid, refreshedAt: Date.now() } + startupAgentForeground = null + }, + clearAgentEvidence: () => { + cachedAgentForeground = null + startupAgentForeground = null + } + } + const inspectForegroundProcess = () => inspectTrackedForegroundProcess(inspectionArgs) + return { recordOutput: (data) => { if (data.length > 0) { @@ -258,56 +284,10 @@ export function createPtyForegroundProcessTracker(args: { return null } }, + inspectProcess: () => inspectTrackedPtyProcess(inspectionArgs), confirmForegroundProcess: async () => { - if (args.isDead() || !proc.pid) { - return null - } - try { - const fallbackProcess = getFallbackProcess() - const fallbackRecognition = recognizeAgentProcess(fallbackProcess) - if ( - !fallbackProcess || - (fallbackRecognition !== null && - process.platform !== 'win32' && - !shouldInspectOuterWrapperForegroundProcess(fallbackRecognition)) || - (process.platform !== 'win32' && !shouldInspectFallback(fallbackProcess)) - ) { - return fallbackProcess - } - const resolution = await resolveAgentForegroundProcessWithAvailability( - proc.pid, - fallbackProcess, - { - contextPaths, - fresh: true, - ...(process.platform === 'win32' - ? { - forceProcessScan: true, - readWindowsConsoleAttachedProcessIds: () => - readWindowsConsoleAttachedProcessIds(proc.pid) - } - : {}) - } - ) - if (args.isDead() || !resolution.available) { - return null - } - const recognized = recognizeAgentProcess(resolution.processName) - if (recognized) { - cachedAgentForeground = { - processName: recognized.processName, - pid: resolution.processId ?? null, - refreshedAt: Date.now() - } - startupAgentForeground = null - return recognized.processName - } - cachedAgentForeground = null - startupAgentForeground = null - return resolution.processName - } catch { - return null - } + const inspection = await inspectForegroundProcess() + return inspection.available ? inspection.processName : null }, confirmShellForeground: () => confirmPtyShellForeground({ process: proc, shellPath: args.shellPath, isDead: args.isDead }) diff --git a/src/main/daemon/pty-subprocess/subprocess-handle.ts b/src/main/daemon/pty-subprocess/subprocess-handle.ts index e2abd59c6ea..715cddb0a75 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, + inspectProcess: foreground.inspectProcess, confirmForegroundProcess: foreground.confirmForegroundProcess, confirmShellForeground: foreground.confirmShellForeground, write: (data) => { diff --git a/src/main/daemon/session-snapshot-output.ts b/src/main/daemon/session-snapshot-output.ts new file mode 100644 index 00000000000..ea3c724fbc0 --- /dev/null +++ b/src/main/daemon/session-snapshot-output.ts @@ -0,0 +1,32 @@ +import type { TakePendingOutputResult, TerminalSnapshot } from './types' + +export function takeSessionPendingOutput(args: { + disposed: boolean + includeSnapshot: boolean + teardownSnapshot: boolean + prepareForFinalSnapshot: () => string + takePendingOutput: ( + includeSnapshot: boolean, + releasedHeldBytes: string, + getSnapshot: () => TerminalSnapshot | null + ) => TakePendingOutputResult | null + getSnapshot: () => TerminalSnapshot | null +}): TakePendingOutputResult | null { + if (args.disposed) { + return null + } + const releasedHeldBytes = + args.includeSnapshot && args.teardownSnapshot ? args.prepareForFinalSnapshot() : '' + return args.takePendingOutput(args.includeSnapshot, releasedHeldBytes, args.getSnapshot) +} + +export function prepareSessionFinalSnapshot(args: { + releaseHeldBytes: () => string + snapshotIngressBarrier: () => void + flushRecoveryBarrier: () => void +}): string { + const held = args.releaseHeldBytes() + args.snapshotIngressBarrier() + args.flushRecoveryBarrier() + return held +} diff --git a/src/main/daemon/session-subprocess-handle.ts b/src/main/daemon/session-subprocess-handle.ts index f14469afbb4..bd3f9bc2c67 100644 --- a/src/main/daemon/session-subprocess-handle.ts +++ b/src/main/daemon/session-subprocess-handle.ts @@ -7,6 +7,11 @@ export type SubprocessHandle = { /** 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 + inspectProcess?(): Promise<{ + foregroundProcess: string | null + hasChildProcesses: boolean + unavailable?: true + }> /** 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 +49,15 @@ export type SubprocessHandle = { /** Release the native PTY handle via node-pty's destroy(). Idempotent; safe to call after exit. */ dispose(): void } + +export function inspectSessionSubprocess(handle: SubprocessHandle) { + const foregroundProcess = handle.getForegroundProcess() + return ( + handle.inspectProcess?.() ?? + Promise.resolve({ + foregroundProcess, + hasChildProcesses: foregroundProcess !== null, + unavailable: true as const + }) + ) +} diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index b0f1dfa538d..b3671fc6dda 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -8,12 +8,13 @@ import { SessionTerminationController, IMMEDIATE_KILL_PHYSICAL_EXIT_TIMEOUT_MS } from './session-termination-controller' -import type { SubprocessHandle } from './session-subprocess-handle' +import { inspectSessionSubprocess, type 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' import { randomUUID } from 'node:crypto' import { PtyStartupIngress } from '../../shared/pty-startup-ingress' +import { prepareSessionFinalSnapshot, takeSessionPendingOutput } from './session-snapshot-output' import type { SessionState, @@ -238,23 +239,23 @@ export class Session { includeSnapshot: boolean, opts: { teardownSnapshot?: boolean } = {} ): TakePendingOutputResult | null { - if (this._disposed) { - return null - } - const releasedHeldBytes = - includeSnapshot && opts.teardownSnapshot === true ? this.prepareForFinalSnapshot() : '' - return this.output.takePendingOutput(includeSnapshot, releasedHeldBytes, () => - this.getSnapshot() - ) + return takeSessionPendingOutput({ + disposed: this._disposed, + includeSnapshot, + teardownSnapshot: opts.teardownSnapshot === true, + prepareForFinalSnapshot: () => this.prepareForFinalSnapshot(), + takePendingOutput: (...args) => this.output.takePendingOutput(...args), + getSnapshot: () => this.getSnapshot() + }) } getCwd(): string | null { return this.output.getCwd() } - getForegroundProcess(): string | null { - return this.subprocess.getForegroundProcess() - } + getForegroundProcess = (): string | null => this.subprocess.getForegroundProcess() + + inspectProcess = () => inspectSessionSubprocess(this.subprocess) async confirmForegroundProcess(): Promise { return this.subprocess.confirmForegroundProcess?.() ?? this.subprocess.getForegroundProcess() @@ -275,12 +276,11 @@ export class Session { } prepareForFinalSnapshot(): string { - const held = this.shellReady.releaseHeldBytes() - this.startupIngress.snapshotBarrier() - // Why last: snapshotBarrier can emit held spans into the barrier, and a - // teardown checkpoint mid-episode must not lose the barrier's queued bytes. - this.recoveryBarrier.flushPending() - return held + return prepareSessionFinalSnapshot({ + releaseHeldBytes: () => this.shellReady.releaseHeldBytes(), + snapshotIngressBarrier: () => this.startupIngress.snapshotBarrier(), + flushRecoveryBarrier: () => this.recoveryBarrier.flushPending() + }) } dispose(): void { diff --git a/src/main/daemon/terminal-host.test.ts b/src/main/daemon/terminal-host.test.ts index 8755005b42e..db2be02c531 100644 --- a/src/main/daemon/terminal-host.test.ts +++ b/src/main/daemon/terminal-host.test.ts @@ -93,8 +93,8 @@ describe('TerminalHost', () => { } }) - it('rejects missing strict inspection', () => - expect(() => host.inspectProcess('missing-session')).toThrow('not found')) + it('rejects missing strict inspection', async () => + await expect(host.inspectProcess('missing-session')).rejects.toThrow('not found')) describe('createOrAttach', () => { it('creates a new session when none exists', async () => { diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index f85980b5453..e79e1f47631 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -19,7 +19,6 @@ 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 { TerminalAttachCanceledError } from './daemon-errors' export type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract' @@ -214,15 +213,12 @@ export class TerminalHost { return session.getForegroundProcess() } - inspectProcess(sessionId: string): { + async inspectProcess(sessionId: string): Promise<{ foregroundProcess: string | null hasChildProcesses: boolean - } { - const foregroundProcess = this.getAliveSession(sessionId).getForegroundProcess() - return { - foregroundProcess, - hasChildProcesses: foregroundProcess !== null && !isShellProcess(foregroundProcess) - } + unavailable?: true + }> { + return this.getAliveSession(sessionId).inspectProcess() } async confirmForegroundProcess(sessionId: string): Promise { diff --git a/src/main/daemon/types.ts b/src/main/daemon/types.ts index a90e9f5eee5..a9010a30fd1 100644 --- a/src/main/daemon/types.ts +++ b/src/main/daemon/types.ts @@ -29,6 +29,7 @@ export { AGENT_SESSION_CLAIM_DAEMON_PROTOCOL_VERSION, AGENT_SESSION_CREATE_OPERATION_DAEMON_PROTOCOL_VERSION, ASYNC_CWD_VALIDATION_DAEMON_PROTOCOL_VERSION, + AUTHORITATIVE_COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, CLEAN_DISCONNECT_PROTOCOL_VERSION, COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, GET_FOREGROUND_PROCESS_PROTOCOL_VERSION, diff --git a/src/main/providers/agent-foreground-process.ts b/src/main/providers/agent-foreground-process.ts index ef5d6cb58f8..31589d9fb88 100644 --- a/src/main/providers/agent-foreground-process.ts +++ b/src/main/providers/agent-foreground-process.ts @@ -27,6 +27,10 @@ export type AgentForegroundProcessResolution = { anchorPidForeign?: boolean } +export type AgentPtyProcessInspection = AgentForegroundProcessResolution & { + hasChildProcesses: boolean +} + type ShellForegroundConfirmationOptions = { readWindowsPtyJobProcessIds?: () => | ReadonlySet @@ -34,6 +38,72 @@ type ShellForegroundConfirmationOptions = { | Promise | null> } +export async function inspectPtyChildProcesses( + shellPid: number | null | undefined, + options: ShellForegroundConfirmationOptions = {} +): Promise<{ hasChildProcesses: boolean; available: boolean }> { + if (!shellPid) { + return { hasChildProcesses: false, available: false } + } + if (process.platform === 'win32') { + try { + const processIds = await options.readWindowsPtyJobProcessIds?.() + return processIds + ? { hasChildProcesses: processIds.size > 1, available: processIds.has(shellPid) } + : { hasChildProcesses: false, available: false } + } catch { + return { hasChildProcesses: false, available: false } + } + } + try { + const rows = await getFreshProcessTableSnapshot() + if (!rows.some((row) => row.pid === shellPid)) { + return { hasChildProcesses: false, available: false } + } + return { hasChildProcesses: collectDescendants(rows, shellPid).length > 0, available: true } + } catch { + return { hasChildProcesses: false, available: false } + } +} + +export async function inspectAgentPtyProcess( + shellPid: number | null | undefined, + fallbackProcess: string | null, + options: AgentForegroundResolutionOptions & ShellForegroundConfirmationOptions = {} +): Promise { + if (!shellPid) { + return { available: false, processName: fallbackProcess, hasChildProcesses: false } + } + if (process.platform === 'win32') { + const [foreground, processIds] = await Promise.all([ + resolveAgentForegroundProcessWithAvailability(shellPid, fallbackProcess, { + ...options, + fresh: true + }), + Promise.resolve(options.readWindowsPtyJobProcessIds?.() ?? null).catch(() => null) + ]) + const childInspectionAvailable = processIds?.has(shellPid) === true + return { + ...foreground, + available: foreground.available && childInspectionAvailable, + hasChildProcesses: childInspectionAvailable && processIds!.size > 1 + } + } + try { + const rows = await getFreshProcessTableSnapshot() + if (!rows.some((row) => row.pid === shellPid)) { + return { available: false, processName: fallbackProcess, hasChildProcesses: false } + } + return { + available: true, + processName: resolveAgentForegroundProcessFromPs(rows, shellPid) ?? fallbackProcess, + hasChildProcesses: collectDescendants(rows, shellPid).length > 0 + } + } catch { + return { available: false, processName: fallbackProcess, hasChildProcesses: false } + } +} + function collectDescendants( rows: Row[], rootPid: number diff --git a/src/main/providers/agent-pty-process-inspection.test.ts b/src/main/providers/agent-pty-process-inspection.test.ts new file mode 100644 index 00000000000..5f074bbde6c --- /dev/null +++ b/src/main/providers/agent-pty-process-inspection.test.ts @@ -0,0 +1,82 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const execFileMock = vi.hoisted(() => vi.fn()) +vi.mock('child_process', () => ({ execFile: execFileMock })) + +import { resetProcessTableSnapshotForTests } from '../../shared/process-table-snapshot' +import { inspectAgentPtyProcess, inspectPtyChildProcesses } from './agent-foreground-process' + +function mockPs(stdout: string): void { + execFileMock.mockImplementation((_cmd, _args, _opts, callback) => { + callback(null, { stdout, stderr: '' }) + }) +} + +describe('agent PTY process inspection', () => { + let platform: PropertyDescriptor | undefined + + beforeEach(() => { + execFileMock.mockReset() + resetProcessTableSnapshotForTests() + platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' }) + }) + + afterEach(() => { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + }) + + it('resolves POSIX identity and children from one fresh snapshot', async () => { + mockPs(['100 99 Ss+ /bin/zsh -l', '101 100 S+ sleep 100'].join('\n')) + + await expect(inspectAgentPtyProcess(100, 'zsh')).resolves.toEqual({ + available: true, + processName: 'zsh', + hasChildProcesses: true + }) + expect(execFileMock).toHaveBeenCalledTimes(1) + }) + + it('reports an available child-free POSIX shell', async () => { + mockPs('100 99 Ss+ /bin/zsh -l') + await expect(inspectAgentPtyProcess(100, 'zsh')).resolves.toEqual({ + available: true, + processName: 'zsh', + hasChildProcesses: false + }) + }) + + it('treats a missing POSIX root and failed scan as unavailable', async () => { + mockPs('101 999 S+ sleep 100') + await expect(inspectPtyChildProcesses(100)).resolves.toEqual({ + available: false, + hasChildProcesses: false + }) + + resetProcessTableSnapshotForTests() + execFileMock.mockImplementation((_cmd, _args, _opts, callback) => { + callback(new Error('ps unavailable'), { stdout: '', stderr: '' }) + }) + await expect(inspectPtyChildProcesses(100)).resolves.toEqual({ + available: false, + hasChildProcesses: false + }) + }) + + it('reads Windows child evidence from ConPTY membership', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + await expect( + inspectPtyChildProcesses(100, { + readWindowsPtyJobProcessIds: async () => new Set([100, 101]) + }) + ).resolves.toEqual({ available: true, hasChildProcesses: true }) + await expect( + inspectPtyChildProcesses(100, { readWindowsPtyJobProcessIds: async () => new Set([100]) }) + ).resolves.toEqual({ available: true, hasChildProcesses: false }) + await expect( + inspectPtyChildProcesses(100, { readWindowsPtyJobProcessIds: async () => null }) + ).resolves.toEqual({ available: false, hasChildProcesses: false }) + }) +}) diff --git a/src/main/providers/local-pty-provider-foreground-process.test.ts b/src/main/providers/local-pty-provider-foreground-process.test.ts index a10fd9a0579..b3517b33fa1 100644 --- a/src/main/providers/local-pty-provider-foreground-process.test.ts +++ b/src/main/providers/local-pty-provider-foreground-process.test.ts @@ -9,6 +9,7 @@ const { writeFileSyncMock, spawnMock, prepareMacosTccLoginShellMock, + inspectAgentPtyProcessMock, resolveAgentForegroundProcessMock, readWindowsPtyJobProcessIdsMock, killWithDescendantSweepMock, @@ -23,6 +24,7 @@ const { writeFileSyncMock: vi.fn(), spawnMock: vi.fn(), prepareMacosTccLoginShellMock: vi.fn(), + inspectAgentPtyProcessMock: vi.fn(), resolveAgentForegroundProcessMock: vi.fn(), readWindowsPtyJobProcessIdsMock: vi.fn(), killWithDescendantSweepMock: vi.fn(), @@ -80,6 +82,7 @@ vi.mock('./windows-powershell-executable', () => ({ })) vi.mock('./agent-foreground-process', () => ({ + inspectAgentPtyProcess: (...args: unknown[]) => inspectAgentPtyProcessMock(...args), resolveAgentForegroundProcessWithAvailability: (...args: unknown[]) => resolveAgentForegroundProcessMock(...args) })) @@ -153,6 +156,14 @@ describe('LocalPtyProvider', () => { } }) spawnMock.mockReturnValue(mockProc) + inspectAgentPtyProcessMock.mockReset() + inspectAgentPtyProcessMock.mockImplementation( + async (_pid: number, fallbackProcess: string | null) => ({ + available: true, + processName: fallbackProcess, + hasChildProcesses: false + }) + ) provider = new LocalPtyProvider() }) @@ -207,9 +218,10 @@ describe('LocalPtyProvider', () => { }) it('marks a degraded local scan unavailable for completion-sensitive inspection', async () => { - resolveAgentForegroundProcessMock.mockResolvedValue({ + inspectAgentPtyProcessMock.mockResolvedValue({ available: false, - processName: 'zsh' + processName: 'zsh', + hasChildProcesses: false }) const { id } = await provider.spawn({ cols: 80, rows: 24 }) @@ -222,13 +234,33 @@ describe('LocalPtyProvider', () => { it('reports live child evidence in completion-sensitive inspection', async () => { const { id } = await provider.spawn({ cols: 80, rows: 24 }) - mockProc.process = 'node' - resolveAgentForegroundProcessMock.mockResolvedValue({ available: true, processName: null }) - - await expect(provider.inspectProcess(id)).resolves.toEqual({ - foregroundProcess: null, + inspectAgentPtyProcessMock.mockResolvedValue({ + available: true, + processName: 'zsh', hasChildProcesses: true }) + + await expect(provider.inspectProcess(id)).resolves.toEqual({ + foregroundProcess: 'zsh', + hasChildProcesses: true + }) + }) + + it('leaves WSL completion inspection unavailable without distro-side evidence', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + const { id } = await provider.spawn({ + cols: 80, + rows: 24, + shellOverride: 'wsl.exe', + terminalWindowsWslDistro: 'Ubuntu' + }) + + await expect(provider.inspectProcess(id)).resolves.toEqual({ + foregroundProcess: 'zsh', + hasChildProcesses: false, + unavailable: true + }) + expect(inspectAgentPtyProcessMock).not.toHaveBeenCalled() }) it('keeps a recognized agent across an unavailable scan without adding probes', async () => { diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 8a753bfc492..9c8e9a903d2 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -61,6 +61,7 @@ import { import { WINDOWS_GIT_BASH_SHELL } from '../../shared/windows-terminal-shell' import { confirmShellForegroundProcess, + inspectAgentPtyProcess, resolveAgentForegroundProcessWithAvailability } from './agent-foreground-process' import { resolveStableForegroundProcess } from './stable-foreground-process' @@ -1549,26 +1550,44 @@ export class LocalPtyProvider implements IPtyProvider { if (!proc) { return { foregroundProcess: null, hasChildProcesses: false, unavailable: true } } - const foreground = await this.inspectForegroundProcessResolution(id) - if (ptyProcesses.get(id) !== proc) { - return { foregroundProcess: null, hasChildProcesses: false, unavailable: true } - } - let hasChildProcesses: boolean - try { - const shell = ptyShellName.get(id) - const currentProcess = resolveForegroundFallbackProcess(proc.process || null, shell) - hasChildProcesses = !shell || (currentProcess !== null && currentProcess !== shell) - } catch { + if (ptyWslDistroById.get(id) !== null && ptyWslDistroById.has(id)) { return { - foregroundProcess: foreground.processName, + foregroundProcess: (await this.inspectForegroundProcessResolution(id)).processName, hasChildProcesses: false, unavailable: true } } - return { - foregroundProcess: foreground.processName, - hasChildProcesses, - ...(foreground.available ? {} : { unavailable: true as const }) + try { + const shell = ptyShellName.get(id) + const inspection = await inspectAgentPtyProcess( + proc.pid, + resolveForegroundFallbackProcess(proc.process || null, shell), + { + contextPaths: ptyAgentForegroundContextPaths.get(id), + forceProcessScan: true, + ...(process.platform === 'win32' + ? { + readWindowsConsoleAttachedProcessIds: () => + readWindowsConsoleAttachedProcessIds(proc.pid), + readWindowsPtyJobProcessIds: () => readWindowsPtyJobProcessIds(proc) + } + : {}) + } + ) + if (ptyProcesses.get(id) !== proc) { + return { foregroundProcess: null, hasChildProcesses: false, unavailable: true } + } + return { + foregroundProcess: inspection.processName, + hasChildProcesses: inspection.hasChildProcesses, + ...(inspection.available ? {} : { unavailable: true as const }) + } + } catch { + return { + foregroundProcess: null, + hasChildProcesses: false, + unavailable: true + } } } diff --git a/src/main/providers/provider-dispatch.test.ts b/src/main/providers/provider-dispatch.test.ts index cf2d9979a88..b6329774658 100644 --- a/src/main/providers/provider-dispatch.test.ts +++ b/src/main/providers/provider-dispatch.test.ts @@ -110,6 +110,7 @@ describe('PTY provider dispatch', () => { acknowledgeDataEvent: vi.fn(), hasChildProcesses: vi.fn(), getForegroundProcess: vi.fn(), + inspectProcess: vi.fn(), serialize: vi.fn(), revive: vi.fn(), listProcesses: vi.fn(), diff --git a/src/main/providers/pty-process-inspection.test.ts b/src/main/providers/pty-process-inspection.test.ts index ab697311248..e5c38a7b3a5 100644 --- a/src/main/providers/pty-process-inspection.test.ts +++ b/src/main/providers/pty-process-inspection.test.ts @@ -61,7 +61,7 @@ describe('PTY provider process inspection', () => { await expect(inspectPtyProviderProcess(provider, 'pty-1')).resolves.toEqual(inspection) }) - it('falls back to the existing provider process APIs', async () => { + it('keeps providers without completion-sensitive inspection unavailable', async () => { const getForegroundProcess = vi.fn().mockResolvedValue('codex') const hasChildProcesses = vi.fn().mockResolvedValue(true) const provider = { @@ -70,8 +70,57 @@ describe('PTY provider process inspection', () => { } as Pick as IPtyProvider await expect(inspectPtyProviderProcess(provider, 'pty-1')).resolves.toEqual({ - foregroundProcess: 'codex', - hasChildProcesses: true + foregroundProcess: null, + hasChildProcesses: false, + unavailable: true + }) + expect(getForegroundProcess).not.toHaveBeenCalled() + expect(hasChildProcesses).not.toHaveBeenCalled() + }) + + it('keeps a partial inspection unavailable', async () => { + const provider = { + inspectProcess: vi.fn(async () => ({ hasChildProcesses: false })) + } as unknown as IPtyProvider + + await expect(inspectPtyProviderProcess(provider, 'pty-1')).resolves.toEqual({ + foregroundProcess: null, + hasChildProcesses: false, + unavailable: true }) }) + + it('preserves positive live evidence from a partial inspection', async () => { + const provider = { + inspectProcess: vi.fn(async () => ({ + foregroundProcess: 'codex', + hasChildProcesses: 'unknown' + })) + } as unknown as IPtyProvider + + await expect(inspectPtyProviderProcess(provider, 'pty-1')).resolves.toEqual({ + foregroundProcess: 'codex', + hasChildProcesses: false, + unavailable: true + }) + }) + + it('snapshots adapter fields once before validating them', async () => { + let foregroundReads = 0 + const provider = { + inspectProcess: vi.fn(async () => ({ + get foregroundProcess() { + foregroundReads += 1 + return foregroundReads === 1 ? null : undefined + }, + hasChildProcesses: false + })) + } as unknown as IPtyProvider + + await expect(inspectPtyProviderProcess(provider, 'pty-1')).resolves.toEqual({ + foregroundProcess: null, + hasChildProcesses: false + }) + expect(foregroundReads).toBe(1) + }) }) diff --git a/src/main/providers/pty-process-inspection.ts b/src/main/providers/pty-process-inspection.ts index 852960c6b7d..ae34676a577 100644 --- a/src/main/providers/pty-process-inspection.ts +++ b/src/main/providers/pty-process-inspection.ts @@ -6,8 +6,37 @@ export type PtyProcessInspection = { unavailable?: true } -type CompletionSensitivePtyProvider = IPtyProvider & { - inspectProcess?: (id: string) => Promise +function unavailablePtyProcessInspection( + value?: Partial +): PtyProcessInspection { + return { + foregroundProcess: + typeof value?.foregroundProcess === 'string' ? value.foregroundProcess : null, + hasChildProcesses: value?.hasChildProcesses === true, + unavailable: true + } +} + +export function normalizePtyProcessInspection(value: unknown): PtyProcessInspection { + if (typeof value !== 'object' || value === null) { + return unavailablePtyProcessInspection() + } + const inspection = value as Partial + const foregroundProcess = inspection.foregroundProcess + const hasChildProcesses = inspection.hasChildProcesses + const unavailable = inspection.unavailable + if ( + (foregroundProcess !== null && typeof foregroundProcess !== 'string') || + typeof hasChildProcesses !== 'boolean' || + (unavailable !== undefined && unavailable !== true) + ) { + return unavailablePtyProcessInspection({ foregroundProcess, hasChildProcesses }) + } + return { + foregroundProcess, + hasChildProcesses, + ...(unavailable === true ? { unavailable: true } : {}) + } } export async function inspectPtyProviderProcess( @@ -17,13 +46,10 @@ export async function inspectPtyProviderProcess( if (provider.hasPty?.(ptyId) === false) { throw new Error('terminal_gone') } - const inspectProcess = (provider as CompletionSensitivePtyProvider).inspectProcess - if (inspectProcess) { - return inspectProcess.call(provider, ptyId) + if (typeof provider.inspectProcess !== 'function') { + return unavailablePtyProcessInspection() } - const foregroundProcess = await provider.getForegroundProcess(ptyId) - const hasChildProcesses = await provider.hasChildProcesses(ptyId) - return { foregroundProcess, hasChildProcesses } + return normalizePtyProcessInspection(await provider.inspectProcess(ptyId)) } export async function inspectPtyProviderProcessForRenderer( diff --git a/src/main/providers/pty-provider-contract.ts b/src/main/providers/pty-provider-contract.ts index d77d6aec1a5..44e60abcd30 100644 --- a/src/main/providers/pty-provider-contract.ts +++ b/src/main/providers/pty-provider-contract.ts @@ -210,6 +210,12 @@ export type IPtyProvider = { acknowledgeDataEvent(id: string, charCount: number): void hasChildProcesses(id: string): Promise getForegroundProcess(id: string): Promise + /** Atomic completion-sensitive inspection; failures must reject or return unavailable. */ + inspectProcess(id: string): Promise<{ + foregroundProcess: string | null + hasChildProcesses: boolean + unavailable?: true + }> /** Strong process evidence captured after the caller's command boundary. */ confirmForegroundProcess?: (id: string) => Promise /** Fresh execution-host proof that the spawned shell owns the PTY foreground. */ diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index b9bd685ab66..135f2eeb6f7 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -61,6 +61,7 @@ import { OrchestrationDb } from './orchestration/db' import type { MessagePriority, MessageRow, MessageType } from './orchestration/types' import { AUTHORITATIVE_TERMINAL_SNAPSHOT_TIMEOUT_MS, + PTY_FOREGROUND_PROCESS_READ_TIMEOUT_MS, appendNormalizedToTailBuffer, buildPreview, OrcaRuntimeService, @@ -10608,7 +10609,13 @@ describe('OrcaRuntimeService', () => { const providerQuery = vi.fn(async () => { throw new Error('socket_closed') }) - setLocalPtyProvider({ getForegroundProcess: providerQuery } as unknown as IPtyProvider) + setLocalPtyProvider({ + getForegroundProcess: providerQuery, + inspectProcess: async () => ({ + foregroundProcess: await providerQuery(), + hasChildProcesses: false + }) + } as unknown as IPtyProvider) onTestFinished(() => setLocalPtyProvider(originalProvider)) runtime.setPtyController({ write: () => true, @@ -10636,6 +10643,147 @@ describe('OrcaRuntimeService', () => { ]) }) + it('does not certify an adapter that lacks completion-sensitive inspection', async () => { + const { runtime, batches } = createSideEffectRuntime() + syncSinglePty(runtime) + runtime.onPtySpawned('pty-1', 'inc-legacy-inspection', { awaitsRegistration: false }) + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + batches.length = 0 + + const originalProvider = getLocalPtyProvider() + setLocalPtyProvider({ + getForegroundProcess: async () => null, + hasChildProcesses: async () => false + } as unknown as IPtyProvider) + onTestFinished(() => setLocalPtyProvider(originalProvider)) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: getForegroundProcessFromRuntimeController, + inspectProcess: inspectProcessFromRuntimeController + }) + + runtime.onPtyData('pty-1', '\x1b]0;bichir\x07', 100) + + await vi.waitFor(() => + expect( + ( + runtime as unknown as { + ptyForegroundProcessReads: Map + } + ).ptyForegroundProcessReads.size + ).toBe(0) + ) + expect(batches.flatMap((batch) => batch.facts)).toEqual([ + { kind: 'title', normalizedTitle: 'bichir', rawTitle: 'bichir' }, + { kind: 'agent-exited' } + ]) + }) + + it.each([ + ['resolved null', async () => null], + ['resolved undefined', async () => undefined], + ['missing child evidence', async () => ({ foregroundProcess: null })], + ['missing foreground evidence', async () => ({ hasChildProcesses: false })], + ['rejection', () => Promise.reject(new Error('socket_closed'))], + [ + 'synchronous throw', + () => { + throw new Error('socket_closed') + } + ] + ])('does not certify a %s inspection result', async (_shape, inspectProcess) => { + const { runtime, batches } = createSideEffectRuntime() + syncSinglePty(runtime) + runtime.onPtySpawned('pty-1', 'inc-malformed-inspection', { awaitsRegistration: false }) + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + batches.length = 0 + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + inspectProcess: inspectProcess as never + }) + + runtime.onPtyData('pty-1', '\x1b]0;bichir\x07', 100) + + await vi.waitFor(() => + expect( + ( + runtime as unknown as { + ptyForegroundProcessReads: Map + } + ).ptyForegroundProcessReads.size + ).toBe(0) + ) + expect(batches.flatMap((batch) => batch.facts)).toEqual([ + { kind: 'title', normalizedTitle: 'bichir', rawTitle: 'bichir' }, + { kind: 'agent-exited' } + ]) + }) + + it('times out a stalled completion-sensitive inspection as unavailable', async () => { + vi.useFakeTimers() + try { + const { runtime, batches } = createSideEffectRuntime() + syncSinglePty(runtime) + runtime.onPtySpawned('pty-1', 'inc-stalled-inspection', { awaitsRegistration: false }) + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + batches.length = 0 + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + inspectProcess: () => new Promise(() => {}) + }) + + runtime.onPtyData('pty-1', '\x1b]0;bichir\x07', 100) + await vi.advanceTimersByTimeAsync(PTY_FOREGROUND_PROCESS_READ_TIMEOUT_MS) + + expect( + ( + runtime as unknown as { + ptyForegroundProcessReads: Map + } + ).ptyForegroundProcessReads.size + ).toBe(0) + expect(batches.flatMap((batch) => batch.facts)).not.toContainEqual( + expect.objectContaining({ executionHostConfirmed: true }) + ) + } finally { + vi.useRealTimers() + } + }) + + it('keeps recognized foreground evidence from a partial inspection live', async () => { + const { runtime, batches } = createSideEffectRuntime() + syncSinglePty(runtime) + runtime.onPtySpawned('pty-1', 'inc-partial-live-inspection', { awaitsRegistration: false }) + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + batches.length = 0 + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + inspectProcess: async () => ({ foregroundProcess: 'codex' }) as never + }) + + runtime.onPtyData('pty-1', '\x1b]0;bichir\x07', 100) + + await vi.waitFor(() => + expect( + ( + runtime as unknown as { + ptyForegroundProcessReads: Map + } + ).ptyForegroundProcessReads.size + ).toBe(0) + ) + expect(batches.flatMap((batch) => batch.facts)).toEqual([ + { kind: 'title', normalizedTitle: 'bichir', rawTitle: 'bichir' } + ]) + }) + it('still certifies an agent exit when process inspection confirms no foreground agent', async () => { const { runtime, batches } = createSideEffectRuntime() syncSinglePty(runtime) @@ -10647,7 +10795,11 @@ describe('OrcaRuntimeService', () => { const providerQuery = vi.fn(async () => null) setLocalPtyProvider({ getForegroundProcess: providerQuery, - hasChildProcesses: async () => false + hasChildProcesses: async () => false, + inspectProcess: async () => ({ + foregroundProcess: await providerQuery(), + hasChildProcesses: false + }) } as unknown as IPtyProvider) onTestFinished(() => setLocalPtyProvider(originalProvider)) runtime.setPtyController({ @@ -10808,6 +10960,33 @@ describe('OrcaRuntimeService', () => { ) }) + it('does not certify a synthetic local disconnect while inspection is pending', async () => { + const { runtime, batches } = createSideEffectRuntime() + runtime.registerPty('pty-1', TEST_WORKTREE_ID) + syncSinglePty(runtime) + runtime.onPtySpawned('pty-1', 'inc-local-unverified-stop', { awaitsRegistration: false }) + runtime.ingestSyntheticTitleFrame('pty-1', '\x1b]0;Codex ready\x07') + let rejectInspection!: (error: Error) => void + const inspection = new Promise((_resolve, reject) => { + rejectInspection = reject + }) + runtime.setPtyController({ + write: () => true, + kill: () => true, + getForegroundProcess: async () => null, + inspectProcess: () => inspection + }) + + runtime.onPtyData('pty-1', '\x1b]0;bichir\x07', 100) + runtime.onPtyExit('pty-1', -1, 'inc-local-unverified-stop') + rejectInspection(new Error('shutdown failed')) + await Promise.resolve() + + expect(batches.flatMap((batch) => batch.facts)).not.toContainEqual( + expect.objectContaining({ executionHostConfirmed: true }) + ) + }) + it('drops a confirmed exit from a replaced PTY incarnation', async () => { const { runtime, batches } = createSideEffectRuntime() syncSinglePty(runtime) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 0f36dbdf878..bacdcdbf57e 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -768,6 +768,7 @@ import type { PtySpawnResult, PtyTransientFact } from '../providers/types' +import { normalizePtyProcessInspection } from '../providers/pty-process-inspection' import { ClaudeAgentTeamsService } from './claude-agent-teams-service' import type { AgentTeamsTmuxCompatRequest, @@ -2235,6 +2236,7 @@ async function waitForAgentPromptDelay(delayMs: number, signal?: AbortSignal): P } const MOBILE_TERMINAL_SURFACE_TIMEOUT_MS = 10_000 +export const PTY_FOREGROUND_PROCESS_READ_TIMEOUT_MS = 10_000 // Why: the split already failed; the caller waits on this teardown only to learn whether the // fallback kill is needed, so keep it short — an unreachable host must not stall the rejection. const REJECTED_SPLIT_PTY_STOP_TIMEOUT_MS = 2_000 @@ -15951,7 +15953,7 @@ export class OrcaRuntimeService { this.remoteDesktopViewerRevisions.delete(ptyId) this.disposeHeadlessTerminal(ptyId) if (pty) { - if (options?.hostExitConfirmed === true) { + if (processDeathCertified) { pty.hostExitConfirmed = true } pty.connected = false @@ -19685,11 +19687,14 @@ export class OrcaRuntimeService { try { processRead = verifiedAbsence ? controller.inspectProcess - ? Promise.resolve(controller.inspectProcess(ptyId)).then((inspection) => ({ - process: inspection.foregroundProcess, - hasChildProcesses: inspection.hasChildProcesses, - available: inspection.unavailable !== true - })) + ? Promise.resolve(controller.inspectProcess(ptyId)).then((value) => { + const inspection = normalizePtyProcessInspection(value) + return { + process: inspection.foregroundProcess, + hasChildProcesses: inspection.hasChildProcesses, + available: inspection.unavailable !== true + } + }) : Promise.resolve({ process: null, hasChildProcesses: null, available: false }) : Promise.resolve(controller.getForegroundProcess(ptyId)).then((process) => ({ process, @@ -19712,7 +19717,11 @@ export class OrcaRuntimeService { return entry.promise } let entry: PtyForegroundProcessReadEntry - const promise = processRead + const promise = withTimeout(processRead, PTY_FOREGROUND_PROCESS_READ_TIMEOUT_MS, { + process: null, + hasChildProcesses: null, + available: false + }) .then((result) => ({ controller, ...result })) .catch(() => unavailable) .finally(() => { @@ -19737,7 +19746,7 @@ export class OrcaRuntimeService { if (this.getPtyLivenessVerdict(ptyId)?.status === 'unverifiable') { return true } - return Boolean(pty?.connectionId && !pty.connected && !pty.hostExitConfirmed) + return Boolean(pty && !pty.connected && !pty.hostExitConfirmed) } private settlePendingPtyAgentExitOnCommandFinished(ptyId: string): void { diff --git a/src/relay/pty-child-process-inspection.ts b/src/relay/pty-child-process-inspection.ts new file mode 100644 index 00000000000..6184911b5e0 --- /dev/null +++ b/src/relay/pty-child-process-inspection.ts @@ -0,0 +1,22 @@ +import { execFile as execFileCb } from 'node:child_process' +import { promisify } from 'node:util' + +const execFile = promisify(execFileCb) + +export async function inspectProcessChildren(pid: number): Promise<{ + hasChildProcesses: boolean + unavailable?: true +}> { + try { + const { stdout } = await execFile('pgrep', ['-P', String(pid)], { + encoding: 'utf-8', + timeout: 3000 + }) + return { hasChildProcesses: stdout.trim().length > 0 } + } catch (error) { + if ((error as { code?: string | number }).code === 1) { + return { hasChildProcesses: false } + } + return { hasChildProcesses: false, unavailable: true } + } +} diff --git a/src/relay/pty-handler-spawn-admission.test.ts b/src/relay/pty-handler-spawn-admission.test.ts index aae35a2e166..8ee5c978102 100644 --- a/src/relay/pty-handler-spawn-admission.test.ts +++ b/src/relay/pty-handler-spawn-admission.test.ts @@ -91,6 +91,18 @@ describe('PtyHandler', () => { ) }) + it('preserves an unavailable relay child inspection', async () => { + await spawnPty({ cols: 80, rows: 24 }) + vi.mocked(ptyShellUtils.inspectProcessChildren).mockResolvedValueOnce({ + hasChildProcesses: false, + unavailable: true + }) + + await expect(dispatcher.callRequest('pty.inspectProcess', { id: 'pty-1' })).resolves.toEqual( + expect.objectContaining({ hasChildProcesses: false, unavailable: true }) + ) + }) + it('spawns a PTY and returns an id', async () => { const result = await spawnPty({ cols: 80, rows: 24 }) expect(result).toEqual({ id: 'pty-1', incarnationId: expect.any(String) }) diff --git a/src/relay/pty-handler-test-harness.ts b/src/relay/pty-handler-test-harness.ts index 04dc828e507..59c5cc2e4f2 100644 --- a/src/relay/pty-handler-test-harness.ts +++ b/src/relay/pty-handler-test-harness.ts @@ -104,6 +104,9 @@ export function beginPtyHandlerTest(mocks: PtyHandlerTestMocks): { dispose: vi.fn() }) vi.spyOn(ptyShellUtils, 'processHasChildren').mockResolvedValue(false) + vi.spyOn(ptyShellUtils, 'inspectProcessChildren').mockResolvedValue({ + hasChildProcesses: false + }) mockPtySpawn.mockReturnValue({ ...mockPtyInstance }) diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index a9fd21a4fe2..e15c196e01e 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -12,6 +12,7 @@ import { resolveDefaultCwd, resolveProcessCwd, processHasChildren, + inspectProcessChildren, getForegroundProcessName, isProcessAlive, listShellProfiles @@ -2083,6 +2084,7 @@ export class PtyHandler { private async inspectProcess(params: Record): Promise<{ foregroundProcess: string | null hasChildProcesses: boolean + unavailable?: true }> { const id = params.id as string const managed = this.ptys.get(id) @@ -2093,9 +2095,10 @@ export class PtyHandler { managed.pty.pid, managed.pty.process || null ) + const childInspection = await inspectProcessChildren(managed.pty.pid) return { foregroundProcess, - hasChildProcesses: await processHasChildren(managed.pty.pid) + ...childInspection } } diff --git a/src/relay/pty-shell-utils.ts b/src/relay/pty-shell-utils.ts index 9860917c588..95cc52c63cd 100644 --- a/src/relay/pty-shell-utils.ts +++ b/src/relay/pty-shell-utils.ts @@ -3,6 +3,8 @@ import { existsSync, readFileSync } from 'node:fs' import { homedir } from 'node:os' import { win32 as pathWin32 } from 'node:path' import { promisify } from 'node:util' +import { inspectProcessChildren } from './pty-child-process-inspection' +export { inspectProcessChildren } from './pty-child-process-inspection' import { isAgentForegroundWrapperProcess, isExpectedAgentProcess, @@ -169,15 +171,7 @@ export async function resolveProcessCwd(pid: number, fallbackCwd: string): Promi * Check whether a process has child processes (via pgrep). */ export async function processHasChildren(pid: number): Promise { - try { - const { stdout } = await execFile('pgrep', ['-P', String(pid)], { - encoding: 'utf-8', - timeout: 3000 - }) - return stdout.trim().length > 0 - } catch { - return false - } + return (await inspectProcessChildren(pid)).hasChildProcesses } // Why: signal 0 probes existence without delivering a signal. Only ESRCH ("no