diff --git a/src/main/providers/local-pty-provider.test.ts b/src/main/providers/local-pty-provider.test.ts index 54f8c2a11..40f43cd6f 100644 --- a/src/main/providers/local-pty-provider.test.ts +++ b/src/main/providers/local-pty-provider.test.ts @@ -67,10 +67,8 @@ vi.mock('./windows-powershell-executable', () => ({ })) vi.mock('./agent-foreground-process', () => ({ - resolveAgentForegroundProcessWithAvailability: async (...args: unknown[]) => ({ - available: true, - processName: await resolveAgentForegroundProcessMock(...args) - }) + resolveAgentForegroundProcessWithAvailability: (...args: unknown[]) => + resolveAgentForegroundProcessMock(...args) })) vi.mock('../wsl', () => ({ @@ -143,7 +141,10 @@ describe('LocalPtyProvider', () => { terminateDescendantSnapshotMock.mockReset() resolveAgentForegroundProcessMock.mockReset() resolveAgentForegroundProcessMock.mockImplementation( - async (_pid: number, fallbackProcess: string | null) => fallbackProcess + async (_pid: number, fallbackProcess: string | null) => ({ + available: true, + processName: fallbackProcess + }) ) exitCb = undefined @@ -1358,13 +1359,40 @@ describe('LocalPtyProvider', () => { it('returns null for unknown PTY ids', async () => { expect(await provider.getForegroundProcess('nonexistent')).toBeNull() }) + + it('keeps a recognized agent across an unavailable scan without adding probes', async () => { + resolveAgentForegroundProcessMock + .mockResolvedValueOnce({ available: true, processName: 'claude' }) + .mockResolvedValueOnce({ available: false, processName: 'powershell.exe' }) + const { id } = await provider.spawn({ cols: 80, rows: 24 }) + + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + await expect(provider.getForegroundProcess(id)).resolves.toBe('claude') + expect(resolveAgentForegroundProcessMock).toHaveBeenCalledTimes(2) + }) + + it('drops a delayed scan result after the PTY exits', async () => { + let resolveScan!: (resolution: { available: boolean; processName: string }) => void + resolveAgentForegroundProcessMock.mockReturnValue( + new Promise((resolve) => { + resolveScan = resolve + }) + ) + const { id } = await provider.spawn({ cols: 80, rows: 24 }) + + const foreground = provider.getForegroundProcess(id) + exitCb?.({ exitCode: 0 }) + resolveScan({ available: true, processName: 'droid' }) + + await expect(foreground).resolves.toBeNull() + }) }) describe('confirmForegroundProcess', () => { it('drops a delayed result after the PTY exits', async () => { - let resolveScan!: (processName: string) => void + let resolveScan!: (resolution: { available: boolean; processName: string }) => void resolveAgentForegroundProcessMock.mockReturnValue( - new Promise((resolve) => { + new Promise((resolve) => { resolveScan = resolve }) ) @@ -1372,7 +1400,7 @@ describe('LocalPtyProvider', () => { const confirmation = provider.confirmForegroundProcess(id) exitCb?.({ exitCode: 0 }) - resolveScan('droid') + resolveScan({ available: true, processName: 'droid' }) await expect(confirmation).resolves.toBeNull() }) diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 70e05d8a7..0faea3109 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -52,6 +52,7 @@ import { } from '../git-bash' import { WINDOWS_GIT_BASH_SHELL } from '../../shared/windows-terminal-shell' import { resolveAgentForegroundProcessWithAvailability } from './agent-foreground-process' +import { resolveStableForegroundProcess } from './stable-foreground-process' import { getAgentForegroundContextPaths } from './agent-foreground-context-paths' import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' import { @@ -91,6 +92,9 @@ type PtyShutdownOperation = { const ptyShutdownOperations = new Map() const ptyShellName = new Map() const ptyAgentForegroundContextPaths = new Map() +// Why: remembers the last positively-recognized agent foreground per PTY so a +// degraded/timed-out scan does not report the shell and look like an exit. +const ptyLastRecognizedForeground = new Map() const ptyTerminalHandle = new Map() const ptyInitialCwd = new Map() // Why: node-pty callbacks must be disposed before environment teardown, but @@ -226,6 +230,7 @@ function clearPtyState(id: string): void { ptyAgentSessionIds.delete(id) ptyShellName.delete(id) ptyAgentForegroundContextPaths.delete(id) + ptyLastRecognizedForeground.delete(id) ptyTerminalHandle.delete(id) ptyInitialCwd.delete(id) ptyLoadGeneration.delete(id) @@ -1183,6 +1188,7 @@ export class LocalPtyProvider implements IPtyProvider { async getForegroundProcess(id: string): Promise { const proc = ptyProcesses.get(id) if (!proc) { + ptyLastRecognizedForeground.delete(id) return null } try { @@ -1193,9 +1199,32 @@ export class LocalPtyProvider implements IPtyProvider { contextPaths: ptyAgentForegroundContextPaths.get(id) } ) - return resolution.processName + // Why: the scan can outlive PTY teardown or id reuse; stale results must + // not resurrect cache state for a process that no longer owns this id. + if (ptyProcesses.get(id) !== proc) { + return null + } + // Why: a degraded/timed-out scan must not report the shell as the + // foreground — the completion coordinator reads that as an exit and fires + // a false "agent done" while the agent is still working. Prefer the last + // recognized agent across a transient failure (e.g. a Windows CIM timeout). + const stable = resolveStableForegroundProcess( + resolution, + ptyLastRecognizedForeground.get(id) ?? null + ) + if (stable.lastRecognizedAgent) { + ptyLastRecognizedForeground.set(id, stable.lastRecognizedAgent) + } else { + ptyLastRecognizedForeground.delete(id) + } + return stable.processName } catch { - return null + if (ptyProcesses.get(id) !== proc) { + return null + } + // Why: an inspection error is itself a degraded read; fall back to the + // last recognized agent rather than null (which also reads as an exit). + return ptyLastRecognizedForeground.get(id) ?? null } } diff --git a/src/main/providers/stable-foreground-process.test.ts b/src/main/providers/stable-foreground-process.test.ts new file mode 100644 index 000000000..3e718324f --- /dev/null +++ b/src/main/providers/stable-foreground-process.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import { resolveStableForegroundProcess } from './stable-foreground-process' + +describe('resolveStableForegroundProcess', () => { + it('reports the last recognized agent when a scan is unavailable (Windows CIM timeout)', () => { + // A degraded scan falls back to the shell name. Without this, the shell + // reads as "agent exited" and fires a false completion while it still works. + const result = resolveStableForegroundProcess( + { available: false, processName: 'powershell.exe' }, + 'claude' + ) + expect(result.processName).toBe('claude') + expect(result.lastRecognizedAgent).toBe('claude') + }) + + it('remembers the agent from a completed scan that found it', () => { + const result = resolveStableForegroundProcess({ available: true, processName: 'claude' }, null) + expect(result.processName).toBe('claude') + expect(result.lastRecognizedAgent).toBe('claude') + }) + + it('reports a real exit and clears memory when a completed scan finds no agent', () => { + // Regression guard: a genuine exit/crash must still be detectable — an + // authoritative (available) scan with no agent overrides the memory. + const result = resolveStableForegroundProcess( + { available: true, processName: 'powershell.exe' }, + 'claude' + ) + expect(result.processName).toBe('powershell.exe') + expect(result.lastRecognizedAgent).toBeNull() + }) + + it('passes through when a scan is unavailable and nothing is remembered', () => { + const result = resolveStableForegroundProcess( + { available: false, processName: 'powershell.exe' }, + null + ) + expect(result.processName).toBe('powershell.exe') + expect(result.lastRecognizedAgent).toBeNull() + }) + + it('prefers the remembered agent even when a degraded scan returns null', () => { + const result = resolveStableForegroundProcess({ available: false, processName: null }, 'codex') + expect(result.processName).toBe('codex') + expect(result.lastRecognizedAgent).toBe('codex') + }) +}) diff --git a/src/main/providers/stable-foreground-process.ts b/src/main/providers/stable-foreground-process.ts new file mode 100644 index 000000000..2bb11433c --- /dev/null +++ b/src/main/providers/stable-foreground-process.ts @@ -0,0 +1,40 @@ +import { recognizeAgentProcess } from '../../shared/agent-process-recognition' +import type { AgentForegroundProcessResolution } from './agent-foreground-process' + +export type StableForegroundProcess = { + /** Foreground process name to report to callers. */ + processName: string | null + /** Agent name to remember for the next degraded read; null clears the memory. */ + lastRecognizedAgent: string | null +} + +/** + * Keep the reported foreground process stable across a degraded inspection. + * + * Why: on Windows/ConPTY the foreground scan (a `Get-CimInstance Win32_Process` + * PowerShell fork) can exceed its 3s budget under load, and there is no `wmic` + * fallback on Win11 24H2+. A degraded scan returns `available: false` and falls + * back to the shell name — which the completion coordinator reads as "the agent + * exited" and fires a false "agent done" notification while the agent is still + * working. On a degraded read, prefer the last agent we positively recognized so + * a transient scan failure never looks like an exit. A completed (`available`) + * scan is authoritative and refreshes — or clears — that memory, so a genuine + * exit is still detected. + */ +export function resolveStableForegroundProcess( + resolution: AgentForegroundProcessResolution, + lastRecognizedAgent: string | null +): StableForegroundProcess { + if (resolution.available) { + const isAgent = + resolution.processName !== null && recognizeAgentProcess(resolution.processName) !== null + return { + processName: resolution.processName, + lastRecognizedAgent: isAgent ? resolution.processName : null + } + } + return { + processName: lastRecognizedAgent ?? resolution.processName, + lastRecognizedAgent + } +}