diff --git a/src/main/providers/windows-agent-foreground-process-scan-volume.test.ts b/src/main/providers/windows-agent-foreground-process-scan-volume.test.ts new file mode 100644 index 00000000000..28a97aa5437 --- /dev/null +++ b/src/main/providers/windows-agent-foreground-process-scan-volume.test.ts @@ -0,0 +1,127 @@ +// Regression guard: bound the volume of full-process-table PowerShell/CIM scans +// driven by Windows agent foreground-process inspection — the Windows analogue of +// issue #6288 (POSIX `ps`). +// +// Drives queryWindowsProcessDescendants across several concurrently-inspecting +// agent panes on the agent-completion cadence (ACTIVE_POLL_INTERVAL_MS = 750ms) +// and counts how many powershell.exe process-table scans actually spawn. Pre-fix +// the call site forked one powershell.exe per pane per tick; with the shared +// snapshot cache the scans collapse to ~one per tick regardless of pane count, +// while each pane still resolves the same descendant set. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { execFileMock, powershellScanCount } = vi.hoisted(() => ({ + execFileMock: vi.fn(), + powershellScanCount: { value: 0 } +})) + +vi.mock('child_process', () => ({ execFile: execFileMock })) + +import { + queryWindowsProcessDescendants, + resetWindowsProcessRowsSnapshotForTests +} from './windows-foreground-process-rows' + +const ACTIVE_POLL_INTERVAL_MS = 750 +const PANE_COUNT = 6 +const WINDOW_SECONDS = 30 +const TICKS = Math.floor((WINDOW_SECONDS * 1000) / ACTIVE_POLL_INTERVAL_MS) + +const shellPid = (pane: number): number => 100 + pane * 1000 + +// A real CIM query returns the whole system, so one shared snapshot must contain +// every pane's shell + foreground node/codex child. Each pane resolves its own +// descendant from the single scan. +const PROCESS_TABLE_JSON = JSON.stringify( + Array.from({ length: PANE_COUNT }, (_, pane) => { + const shell = shellPid(pane) + return [ + { + ProcessId: shell, + ParentProcessId: 99, + Name: 'cmd.exe', + CommandLine: 'cmd.exe', + ExecutablePath: 'C:/Windows/System32/cmd.exe' + }, + { + ProcessId: shell + 1, + ParentProcessId: shell, + Name: 'node.exe', + CommandLine: 'node C:/Users/dev/AppData/codex/bin/codex.js', + ExecutablePath: 'C:/Program Files/nodejs/node.exe' + } + ] + }).flat() +) + +function installCountingPowerShellMock(): void { + execFileMock.mockImplementation((cmd: string, _args: unknown, _opts: unknown, cb: unknown) => { + const callback = cb as (err: unknown, result: { stdout: string; stderr: string }) => void + if (cmd === 'powershell.exe') { + powershellScanCount.value += 1 + } + callback(null, { stdout: PROCESS_TABLE_JSON, stderr: '' }) + }) +} + +describe('windows agent foreground inspection powershell-scan volume', () => { + let platform: PropertyDescriptor | undefined + + beforeEach(() => { + execFileMock.mockReset() + resetWindowsProcessRowsSnapshotForTests() + powershellScanCount.value = 0 + platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + vi.useFakeTimers({ toFake: ['Date'] }) + vi.setSystemTime(0) + }) + + afterEach(() => { + vi.useRealTimers() + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + }) + + it('bounds powershell scans by poll ticks, not by pane count, while resolving every pane', async () => { + installCountingPowerShellMock() + + for (let tick = 0; tick < TICKS; tick++) { + vi.setSystemTime(tick * ACTIVE_POLL_INTERVAL_MS) + // All panes inspect concurrently within the tick (worst case). + const resolved = await Promise.all( + Array.from({ length: PANE_COUNT }, (_, pane) => + queryWindowsProcessDescendants(shellPid(pane)) + ) + ) + // Caching must not change the answer: every pane still finds its foreground + // node child as the sole descendant of its shell. + for (let pane = 0; pane < PANE_COUNT; pane++) { + const candidates = resolved[pane] + expect(candidates).not.toBeNull() + expect(candidates).toHaveLength(1) + expect(candidates?.[0]?.pid).toBe(shellPid(pane) + 1) + } + } + + const totalInspections = PANE_COUNT * TICKS + // Pre-fix this equals totalInspections (one powershell.exe per inspection). + // With the shared cache, concurrent panes within a tick share one scan and + // the 500ms TTL forces a fresh scan each new 750ms tick -> ~one per tick. + expect(powershellScanCount.value).toBeLessThanOrEqual(TICKS + 1) + expect(powershellScanCount.value).toBeLessThan(totalInspections / 2) + }) + + it('collapses a burst of concurrent panes into a single scan', async () => { + installCountingPowerShellMock() + + await Promise.all( + Array.from({ length: PANE_COUNT }, (_, pane) => + queryWindowsProcessDescendants(shellPid(pane)) + ) + ) + + expect(powershellScanCount.value).toBe(1) + }) +}) diff --git a/src/main/providers/windows-foreground-process-rows.ts b/src/main/providers/windows-foreground-process-rows.ts index d19d6bf3ebd..d26067122a5 100644 --- a/src/main/providers/windows-foreground-process-rows.ts +++ b/src/main/providers/windows-foreground-process-rows.ts @@ -1,5 +1,6 @@ import { execFile } from 'node:child_process' import { promisify } from 'node:util' +import { createProcessTableSnapshotReader } from '../../shared/process-table-snapshot' const execFileAsync = promisify(execFile) const WINDOWS_PROCESS_QUERY_TIMEOUT_MS = 3_000 @@ -22,12 +23,48 @@ export type WindowsProcessRow = { export type WindowsProcessCandidate = WindowsProcessRow & { depth: number } +// Why: agent foreground inspection forks a whole-process-table PowerShell/CIM +// scan per pane on the same 750ms/2000ms cadence as the POSIX `ps` path. Without +// dedup, K concurrent agent panes fork K powershell.exe cold-starts, each ~10-40x +// heavier than `ps` — the Windows analogue of the idle-CPU churn #6288/#6667 fixed +// for POSIX. Reuse the same TTL + single-in-flight reader, caching parsed rows so +// a burst of panes collapses to ~2 scans/sec; every caller runs its own descendant +// walk over the shared snapshot. +async function runWindowsProcessRows(): Promise { + const rows = + (await queryWindowsProcessesWithPowerShell()) ?? (await queryWindowsProcessesWithWmic()) + if (!rows) { + // Reject so the reader does not cache the miss; callers fall through to + // node-pty's process name (the prior null-return contract is preserved by + // queryWindowsProcessDescendants catching this). + throw new Error('windows process enumeration unavailable') + } + return rows +} + +const windowsProcessRowsReader = createProcessTableSnapshotReader({ + runPs: runWindowsProcessRows, + now: () => Date.now() +}) + export async function queryWindowsProcessDescendants( rootPid: number ): Promise { - const rows = - (await queryWindowsProcessesWithPowerShell()) ?? (await queryWindowsProcessesWithWmic()) - return rows ? collectDescendants(rows, rootPid).sort((a, b) => b.depth - a.depth) : null + let rows: WindowsProcessRow[] + try { + rows = await windowsProcessRowsReader.getSnapshot() + } catch { + return null + } + return collectDescendants(rows, rootPid).sort((a, b) => b.depth - a.depth) +} + +/** + * Test-only: clear the shared Windows process-table snapshot so suites that mock + * execFile between cases don't get one case's rows served to the next within TTL. + */ +export function resetWindowsProcessRowsSnapshotForTests(): void { + windowsProcessRowsReader.reset() } function parseWindowsProcessValueRows(stdout: string): WindowsProcessRow[] { diff --git a/src/shared/process-table-snapshot.ts b/src/shared/process-table-snapshot.ts index 7234cd84333..358cb39a650 100644 --- a/src/shared/process-table-snapshot.ts +++ b/src/shared/process-table-snapshot.ts @@ -21,31 +21,35 @@ const PS_TIMEOUT_MS = 3000 // identically to a fresh fork. const DEFAULT_SNAPSHOT_TTL_MS = 500 -type Snapshot = { stdout: string; capturedAtMs: number } +type Snapshot = { value: T; capturedAtMs: number } -type ProcessTableSnapshotReaderDeps = { - runPs: () => Promise +type ProcessTableSnapshotReaderDeps = { + runPs: () => Promise now: () => number ttlMs?: number } /** * Build a process-table snapshot reader that deduplicates concurrent and - * near-simultaneous `ps` scans behind a single in-flight promise + short TTL. + * near-simultaneous scans behind a single in-flight promise + short TTL. * Exposed as a factory so tests can inject the scan and clock; production code - * uses the shared `getProcessTableSnapshot` instance below. + * uses the shared `getProcessTableSnapshot` instance below. Generic over the + * scan result so the Windows path can cache parsed rows while POSIX caches the + * raw `ps` stdout string (the default). */ -export function createProcessTableSnapshotReader(deps: ProcessTableSnapshotReaderDeps): { - getSnapshot: () => Promise +export function createProcessTableSnapshotReader( + deps: ProcessTableSnapshotReaderDeps +): { + getSnapshot: () => Promise reset: () => void } { const ttlMs = deps.ttlMs ?? DEFAULT_SNAPSHOT_TTL_MS - let cached: Snapshot | null = null - let inFlight: Promise | null = null + let cached: Snapshot | null = null + let inFlight: Promise | null = null - async function getSnapshot(): Promise { + async function getSnapshot(): Promise { if (cached && deps.now() - cached.capturedAtMs < ttlMs) { - return cached.stdout + return cached.value } if (inFlight) { return inFlight @@ -53,11 +57,11 @@ export function createProcessTableSnapshotReader(deps: ProcessTableSnapshotReade const promise = deps.runPs() inFlight = promise try { - const stdout = await promise - // Why: stamp capture time AFTER the scan returns so a slow `ps` can't + 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 = { stdout, capturedAtMs: deps.now() } - return stdout + cached = { value, capturedAtMs: deps.now() } + return value } finally { // Clear in-flight on success and failure so a transient `ps` error // (timeout, nonzero exit) retries on the next call instead of being