fix(terminal): don't report agent exit on a degraded foreground scan (#9244)
* fix(terminal): don't report agent exit on a degraded foreground scan 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 timed-out scan returned the shell name, which the completion coordinator reads as "agent exited" and fires a false "agent done" notification while the agent is still working. Prefer the last positively-recognized agent across a degraded (available:false) scan so a transient failure no longer looks like an exit. An authoritative (available:true) scan still refreshes/clears the memory, so a genuine exit or crash is still detected. No-op on macOS, where the scan is always available. * fix(terminal): drop stale foreground scan results
This commit is contained in:
parent
cbdd750f73
commit
2f5fc65ba4
|
|
@ -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<string>((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()
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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<string, PtyShutdownOperation>()
|
||||
const ptyShellName = new Map<string, string>()
|
||||
const ptyAgentForegroundContextPaths = new Map<string, string[]>()
|
||||
// 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<string, string>()
|
||||
const ptyTerminalHandle = new Map<string, string>()
|
||||
const ptyInitialCwd = new Map<string, string>()
|
||||
// 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<string | null> {
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
})
|
||||
})
|
||||
|
|
@ -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
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue