fix(windows): restore a CIM fallback for relay hosts with no native binding (#16550)

Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>

docs/reference/windows-process-enumeration.md

@@ -37,6 +37,37 @@ Measured on Windows 11 with 1050 processes (p50 / p95):
 | + memory + command line | 30.6 ms | 33.7 ms |
 | `Get-CimInstance` via PowerShell | 706 ms | 723 ms |
 
+## The relay has no binding, and falls back
+
+Relay deployment installs only `node-pty` and `@parcel/watcher` on the remote
+host (`RELAY_NATIVE_DEPS` in `src/main/ssh/ssh-relay-deploy.ts`), so a Windows
+machine used as an SSH host has no `@vscode/windows-process-tree` at all. It is
+not added there on purpose: the package ships no prebuilds, so installing it
+would put a from-source `node-gyp` build — MSVC, the SDK, and the same
+Spectre-mitigated libraries described below — on the critical path of every
+Windows relay deploy, where today none is needed. pnpm patches also do not cross
+SSH, so the remote would get the unpatched 1024-process cap regardless.
+
+Instead, `windows-process-table.ts` falls back to
+`readWindowsProcessRowsWithCim` (`windows-process-table-cim-scan.ts`), the
+`Get-CimInstance` scan this module replaced. The gate is deliberately narrow:
+
+- it engages **only** when the module cannot be required, never when a loaded
+  module fails, wedges, or returns an unreadable table — a present-but-failing
+  reader must not silently start forking a shell at the caller's poll rate;
+- a fallback that also fails still rejects, so "unavailable" never degrades into
+  "nothing is running";
+- the scan applies the same self-presence guard as the native path.
+
+`src/main/ssh/relay-native-dependency-coverage.test.ts` asserts that every
+native addon reachable from the relay entry is either installed on relay hosts
+or listed there with the reason its absence is safe. That test exists because
+#15749 shipped this gap: the relay tests injected a fake module through
+`__setWindowsProcessTreeLoaderForTests`, so nothing exercised the real require.
+
+The native fast path stays unavailable on relay hosts until the toolchain or a
+prebuild story is solved. That is a real gap, tracked separately.
+
 ## Why the package is patched
 
 `config/patches/@vscode__windows-process-tree@0.8.0.patch` carries two hunks.

src/main/ssh/relay-native-dependency-coverage.test.ts

@@ -0,0 +1,102 @@
+import { existsSync, readFileSync } from 'node:fs'
+import { join, resolve } from 'node:path'
+import { build } from 'esbuild'
+import { describe, expect, it } from 'vitest'
+import { RELAY_NATIVE_DEPS } from './ssh-relay-deploy'
+
+/**
+ * The relay is a bundle deployed to a host that has none of Orca's node_modules.
+ * Every native addon it reaches therefore has to be installed by
+ * `installNativeDeps`, or the relay silently loses that capability forever.
+ *
+ * This exists because #15749 moved the Windows process table onto
+ * `@vscode/windows-process-tree` without adding it here. The relay tests all
+ * passed: they inject a fake module through
+ * `__setWindowsProcessTreeLoaderForTests`, so nothing ever exercised the real
+ * require that the remote host would fail. Assert against the real graph.
+ */
+
+const REPO_ROOT = resolve(import.meta.dirname, '..', '..', '..')
+
+/**
+ * Native addons the relay imports but deliberately does NOT install, each with
+ * the reason its absence is safe. Adding an entry is a decision, not a default:
+ * anything not listed must appear in RELAY_NATIVE_DEPS.
+ */
+const DEGRADES_WITHOUT_INSTALL: Record<string, string> = {
+  '@vscode/windows-process-tree':
+    'Windows-only and ships no prebuilds, so installing it would put a from-source ' +
+    'node-gyp build (MSVC + SDK + Spectre-mitigated libs) on the critical path of ' +
+    'every Windows relay deploy — and pnpm patches do not cross SSH, so the remote ' +
+    'would get the unpatched 1024-process cap anyway. windows-process-table.ts ' +
+    'falls back to a Get-CimInstance scan when the binding is absent.'
+}
+
+/** Addons are the packages npm has to build or unpack a binary for. */
+function nativeDependencyNames(): string[] {
+  const pkg = JSON.parse(readFileSync(join(REPO_ROOT, 'package.json'), 'utf8')) as {
+    dependencies?: Record<string, string>
+    optionalDependencies?: Record<string, string>
+  }
+  const declared = new Set([
+    ...Object.keys(pkg.dependencies ?? {}),
+    ...Object.keys(pkg.optionalDependencies ?? {})
+  ])
+  return [...declared].filter((name) => {
+    const dir = join(REPO_ROOT, 'node_modules', name)
+    return existsSync(join(dir, 'binding.gyp')) || existsSync(join(dir, 'prebuilds'))
+  })
+}
+
+/**
+ * Source files reachable from the relay entry.
+ *
+ * Why the metafile and not the emitted bundle: the process table resolves its
+ * addon through `createRequire`, which esbuild cannot see and minification
+ * rewrites, so the specifier only survives reliably in the sources.
+ */
+async function relayReachableSources(): Promise<string[]> {
+  const result = await build({
+    entryPoints: [join(REPO_ROOT, 'src', 'relay', 'relay.ts')],
+    bundle: true,
+    platform: 'node',
+    target: 'node18',
+    format: 'cjs',
+    write: false,
+    metafile: true,
+    external: ['node-pty', '@parcel/watcher', 'electron'],
+    define: { 'process.env.NODE_ENV': '"production"' }
+  })
+  return Object.keys(result.metafile.inputs).filter((input) => !input.includes('node_modules'))
+}
+
+describe('relay native dependency coverage', () => {
+  it('installs every native addon the relay bundle imports', async () => {
+    const sources = await relayReachableSources()
+    const text = sources.map((file) => readFileSync(join(REPO_ROOT, file), 'utf8')).join('\n')
+
+    const imported = nativeDependencyNames().filter(
+      (name) => text.includes(`'${name}'`) || text.includes(`"${name}"`)
+    )
+    expect(imported.length).toBeGreaterThan(0)
+
+    const uncovered = imported.filter(
+      (name) => !(name in RELAY_NATIVE_DEPS) && !(name in DEGRADES_WITHOUT_INSTALL)
+    )
+    expect(uncovered).toEqual([])
+  }, 60_000)
+
+  it('keeps every installed native dep at the version the app depends on', () => {
+    const pkg = JSON.parse(readFileSync(join(REPO_ROOT, 'package.json'), 'utf8')) as {
+      dependencies?: Record<string, string>
+      optionalDependencies?: Record<string, string>
+    }
+    const declared = { ...pkg.dependencies, ...pkg.optionalDependencies }
+    for (const [name, version] of Object.entries(RELAY_NATIVE_DEPS)) {
+      // The relay pins exact versions where the app carries a range, so compare
+      // the base: a relay on a different version than the app marshals the same
+      // node-pty/watcher data through a binding nothing else cross-checks.
+      expect(declared[name]?.replace(/^[\^~]/, ''), `${name} matches the app`).toBe(version)
+    }
+  })
+})

src/main/ssh/ssh-relay-deploy.ts

@@ -697,7 +697,10 @@ function uploadStageNamespaceIfSupported(
 
 const NODE_PTY_VERSION = '1.1.0'
 const NODE_PTY_CONSOLE_LIST_PATCH_FILENAME = 'node-pty-1.1.0-console-list-agent-patch.cjs'
-const RELAY_NATIVE_DEPS = {
+// Exported for the relay-native-dependency-coverage test, which asserts every
+// native addon the relay bundle imports is either installed here or explicitly
+// declared as degrading without it.
+export const RELAY_NATIVE_DEPS = {
   'node-pty': NODE_PTY_VERSION,
   '@parcel/watcher': '2.5.6'
 } as const

src/main/windows/windows-process-table-cim-scan.test.ts

@@ -0,0 +1,66 @@
+import { describe, expect, it } from 'vitest'
+import { parseWindowsCimProcessRows } from './windows-process-table-cim-scan'
+
+describe('parseWindowsCimProcessRows', () => {
+  it('reads the array form ConvertTo-Json emits for a real table', () => {
+    const stdout = JSON.stringify([
+      { CommandLine: 'node relay.js', Name: 'node.exe', ParentProcessId: 4, ProcessId: 100 },
+      { CommandLine: 'claude --resume', Name: 'claude.exe', ParentProcessId: 100, ProcessId: 200 }
+    ])
+    expect(parseWindowsCimProcessRows(stdout)).toEqual([
+      { pid: 100, ppid: 4, name: 'node.exe', command: 'node relay.js' },
+      { pid: 200, ppid: 100, name: 'claude.exe', command: 'claude --resume' }
+    ])
+  })
+
+  it('reads the bare-object form ConvertTo-Json emits for a single row', () => {
+    const stdout = JSON.stringify({
+      CommandLine: 'node relay.js',
+      Name: 'node.exe',
+      ParentProcessId: 4,
+      ProcessId: 100
+    })
+    expect(parseWindowsCimProcessRows(stdout)).toEqual([
+      { pid: 100, ppid: 4, name: 'node.exe', command: 'node relay.js' }
+    ])
+  })
+
+  it('falls back to the image name when a process denied its command line', () => {
+    const stdout = JSON.stringify([
+      { CommandLine: null, Name: 'lsass.exe', ParentProcessId: 4, ProcessId: 700 }
+    ])
+    expect(parseWindowsCimProcessRows(stdout)).toEqual([
+      { pid: 700, ppid: 4, name: 'lsass.exe', command: 'lsass.exe' }
+    ])
+  })
+
+  it('keeps a command line containing newlines on its own row', () => {
+    // The reason this reads JSON and not PowerShell's Key=Value list form.
+    const stdout = JSON.stringify([
+      {
+        CommandLine: 'app.exe "a\nProcessId=9\nb"',
+        Name: 'app.exe',
+        ParentProcessId: 4,
+        ProcessId: 5
+      }
+    ])
+    expect(parseWindowsCimProcessRows(stdout)).toEqual([
+      { pid: 5, ppid: 4, name: 'app.exe', command: 'app.exe "a\nProcessId=9\nb"' }
+    ])
+  })
+
+  it('drops rows with no usable pid rather than inventing one', () => {
+    const stdout = JSON.stringify([
+      { Name: 'ghost.exe', ParentProcessId: 4 },
+      { Name: 'real.exe', ParentProcessId: 4, ProcessId: 5 }
+    ])
+    expect(parseWindowsCimProcessRows(stdout)).toEqual([
+      { pid: 5, ppid: 4, name: 'real.exe', command: 'real.exe' }
+    ])
+  })
+
+  it('returns null on unparseable output so the caller can reject it', () => {
+    // A policy banner or an error on stdout must not read as an idle machine.
+    expect(parseWindowsCimProcessRows('Access is denied.')).toBeNull()
+  })
+})

src/main/windows/windows-process-table-cim-scan.ts

@@ -0,0 +1,101 @@
+import { runProcess } from '../../shared/child-process/run-process'
+import { windowsPowerShellPath } from '../../shared/child-process/windows-system-binary'
+import type { WindowsProcessRow } from './windows-process-table'
+
+/**
+ * The `Get-CimInstance Win32_Process` scan, kept only for hosts where the
+ * native Toolhelp32 binding is not installed.
+ *
+ * Why this still exists after #15749 retired it: the relay bundle is deployed to
+ * SSH hosts that only ever receive `node-pty` and `@parcel/watcher`, so
+ * `@vscode/windows-process-tree` is absent there and every native read rejects.
+ * Callers read that as "no evidence" and agent panes identify as powershell.exe
+ * forever. This restores the v1.4.188 answer on exactly those hosts; the local
+ * app ships the addon and never reaches this path.
+ */
+
+const WINDOWS_CIM_QUERY_TIMEOUT_MS = 3_000
+const WINDOWS_CIM_MAX_OUTPUT_BYTES = 8 * 1024 * 1024
+
+// Why JSON and not the `Key=Value` list form: CommandLine can itself contain
+// CR/LF, so an argument could otherwise masquerade as another row's field.
+const POWERSHELL_PROCESS_QUERY =
+  '[Console]::OutputEncoding = [System.Text.Encoding]::UTF8; ' +
+  'Get-CimInstance -ClassName Win32_Process -Property CommandLine,Name,ParentProcessId,ProcessId | ' +
+  'Select-Object CommandLine,Name,ParentProcessId,ProcessId | ' +
+  'ConvertTo-Json -Compress'
+
+type CimProcessRow = {
+  CommandLine?: unknown
+  Name?: unknown
+  ParentProcessId?: unknown
+  ProcessId?: unknown
+}
+
+function fieldAsString(value: unknown): string {
+  if (typeof value === 'string') {
+    return value
+  }
+  return value === null || value === undefined ? '' : String(value)
+}
+
+function fieldAsNumber(value: unknown): number {
+  if (typeof value === 'number') {
+    return value
+  }
+  return typeof value === 'string' ? Number.parseInt(value, 10) : Number.NaN
+}
+
+export function parseWindowsCimProcessRows(stdout: string): WindowsProcessRow[] | null {
+  const trimmed = stdout.trim()
+  if (!trimmed) {
+    return []
+  }
+  let parsed: unknown
+  try {
+    parsed = JSON.parse(trimmed)
+  } catch {
+    return null
+  }
+  const items = Array.isArray(parsed) ? parsed : [parsed]
+  return items.flatMap((item) => {
+    if (!item || typeof item !== 'object') {
+      return []
+    }
+    const row = item as CimProcessRow
+    const pid = fieldAsNumber(row.ProcessId)
+    const ppid = fieldAsNumber(row.ParentProcessId)
+    if (!Number.isFinite(pid) || !Number.isFinite(ppid)) {
+      return []
+    }
+    const name = fieldAsString(row.Name)
+    // memoryBytes stays undefined: Win32_Process reports WorkingSetSize, but no
+    // caller reads it off this table and asking widens an already costly scan.
+    return [{ pid, ppid, name, command: fieldAsString(row.CommandLine) || name }]
+  })
+}
+
+/**
+ * Read the whole process table through PowerShell.
+ *
+ * Throws rather than returning `[]` on any failure: an empty table is a claim
+ * that nothing is running, and callers act on that by declaring a tree dead.
+ */
+export async function readWindowsProcessRowsWithCim(): Promise<WindowsProcessRow[]> {
+  const result = await runProcess({
+    program: windowsPowerShellPath(),
+    args: ['-NoProfile', '-NonInteractive', '-Command', POWERSHELL_PROCESS_QUERY],
+    timeoutMs: WINDOWS_CIM_QUERY_TIMEOUT_MS,
+    maxOutputBytes: WINDOWS_CIM_MAX_OUTPUT_BYTES
+  })
+  if (result.timedOut || result.code !== 0) {
+    throw new Error(
+      `windows process table CIM scan failed (code=${result.code} timedOut=${result.timedOut})`
+    )
+  }
+  const rows = parseWindowsCimProcessRows(result.stdout)
+  if (!rows || rows.length === 0) {
+    throw new Error('windows process table CIM scan returned no rows')
+  }
+  return rows
+}

src/main/windows/windows-process-table.test.ts

@@ -1,5 +1,6 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import {
+  __setWindowsProcessTableCimScanForTests,
   __setWindowsProcessTreeLoaderForTests,
   isWindowsProcessTableAvailable,
   readWindowsProcessTable,
@@ -66,9 +67,13 @@ describe('windows process table', () => {
 
   it('rejects rather than reporting an empty machine when the module is absent', async () => {
     // A caller that reads "no processes" acts on it -- by declaring a tree dead,
-    // or by concluding a shell has no children. Absence must not look like that.
+    // or by concluding a shell has no children. Absence must not look like that,
+    // and neither must a fallback that also fails.
     __setWindowsProcessTreeLoaderForTests(() => null)
-    await expect(readWindowsProcessTableFresh()).rejects.toThrow(/unavailable/)
+    __setWindowsProcessTableCimScanForTests(async () => {
+      throw new Error('powershell unavailable')
+    })
+    await expect(readWindowsProcessTableFresh()).rejects.toThrow(/powershell unavailable/)
     expect(isWindowsProcessTableAvailable()).toBe(false)
   })
 
@@ -107,6 +112,77 @@ describe('windows process table', () => {
   })
 })
 
+// Why this path exists: relay deployment installs only node-pty and
+// @parcel/watcher, so a Windows SSH host has no native binding and every read
+// used to reject -- which agent recognition reads as "no evidence" forever.
+describe('PowerShell fallback when the native binding is absent', () => {
+  let platform: PropertyDescriptor | undefined
+  const cimScan = vi.fn()
+  const CIM_ROWS = [
+    { pid: process.pid, ppid: 0, name: 'node.exe', command: 'node relay.js' },
+    { pid: 200, ppid: process.pid, name: 'claude.exe', command: 'claude --resume' }
+  ]
+
+  beforeEach(() => {
+    platform = Object.getOwnPropertyDescriptor(process, 'platform')
+    Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
+    cimScan.mockReset()
+    cimScan.mockResolvedValue(CIM_ROWS)
+    __setWindowsProcessTableCimScanForTests(cimScan)
+  })
+
+  afterEach(() => {
+    __setWindowsProcessTableCimScanForTests()
+    __setWindowsProcessTreeLoaderForTests()
+    if (platform) {
+      Object.defineProperty(process, 'platform', platform)
+    }
+  })
+
+  it('engages when the module cannot be required', async () => {
+    __setWindowsProcessTreeLoaderForTests(() => null)
+    await expect(readWindowsProcessTableFresh()).resolves.toEqual(CIM_ROWS)
+    expect(cimScan).toHaveBeenCalledTimes(1)
+  })
+
+  it('does not engage when the native binding is present', async () => {
+    const getAllProcesses = vi.fn()
+    getAllProcesses.mockImplementation((cb: (rows: unknown) => void) => cb(NATIVE))
+    __setWindowsProcessTreeLoaderForTests(() => ({
+      ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 },
+      getAllProcesses
+    }))
+    await readWindowsProcessTableFresh()
+    expect(cimScan).not.toHaveBeenCalled()
+  })
+
+  it('does not engage when a present binding fails its read', async () => {
+    // A wedged or blocked reader must not silently start forking a shell at the
+    // caller's poll rate; only absence is unrecoverable.
+    const getAllProcesses = vi.fn()
+    getAllProcesses.mockImplementation((cb: (rows: unknown) => void) => cb([]))
+    __setWindowsProcessTreeLoaderForTests(() => ({
+      ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 },
+      getAllProcesses
+    }))
+    await expect(readWindowsProcessTableFresh()).rejects.toThrow(/unreadable/)
+    expect(cimScan).not.toHaveBeenCalled()
+  })
+
+  it('rejects a scan missing our own pid instead of reporting an idle machine', async () => {
+    __setWindowsProcessTreeLoaderForTests(() => null)
+    cimScan.mockResolvedValue([{ pid: 200, ppid: 4, name: 'claude.exe', command: 'claude' }])
+    await expect(readWindowsProcessTableFresh()).rejects.toThrow(/unreadable/)
+  })
+
+  it('stays off Windows-only: darwin still reports unavailable', async () => {
+    Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' })
+    __setWindowsProcessTreeLoaderForTests(() => null)
+    await expect(readWindowsProcessTableFresh()).rejects.toThrow(/unavailable/)
+    expect(cimScan).not.toHaveBeenCalled()
+  })
+})
+
 describe('wedge cooldown', () => {
   let platform: PropertyDescriptor | undefined
 

src/main/windows/windows-process-table.ts

@@ -1,5 +1,6 @@
 import { createRequire } from 'node:module'
 import { createProcessTableSnapshotReader } from '../../shared/process-table-snapshot'
+import { readWindowsProcessRowsWithCim } from './windows-process-table-cim-scan'
 
 /**
  * The only place Orca reads the Windows process table.
@@ -54,6 +55,7 @@ const requireFromMain = createRequire(__filename)
 
 let cachedModule: WindowsProcessTreeModule | null | undefined
 let moduleLoader: () => WindowsProcessTreeModule | null = loadWindowsProcessTree
+let cimScan: () => Promise<WindowsProcessRow[]> = readWindowsProcessRowsWithCim
 
 /**
  * Resolve the native module, or null where it cannot be used.
@@ -110,6 +112,14 @@ let readGeneration = 0
 function readNativeRows(): Promise<WindowsProcessRow[]> {
   const native = moduleLoader()
   if (!native) {
+    if (process.platform === 'win32') {
+      // Why only when the module is absent: a binding that loads is the fast
+      // path even when a read fails or wedges, so a failing native reader must
+      // never silently start forking shells at the caller's poll rate. Absence
+      // is the one condition that can never resolve itself — see
+      // docs/reference/windows-process-enumeration.md.
+      return readCimRows()
+    }
     // Reject rather than resolve empty: an empty table is a claim that nothing
     // is running, and callers act on that by force-killing or by declaring a
     // tree dead. "Unavailable" has to stay distinguishable from "empty".
@@ -186,6 +196,21 @@ function readNativeRows(): Promise<WindowsProcessRow[]> {
   })
 }
 
+/**
+ * Whole-table read for hosts with no native binding (the relay).
+ *
+ * Applies the same self-presence guard as the native path: a scan that omits
+ * our own pid is truncated or permission-filtered, not empty, and must reject
+ * so nothing downstream reads it as proof a process died.
+ */
+async function readCimRows(): Promise<WindowsProcessRow[]> {
+  const rows = await cimScan()
+  if (!rows.some((row) => row.pid === process.pid)) {
+    throw new Error('windows process table is unreadable')
+  }
+  return rows
+}
+
 // Why still cache: the snapshot is cheap but not free, and a worktree delete
 // tears down PTYs 32-wide. The shared TTL + single-in-flight reader collapses
 // that burst into one scan, exactly as the PowerShell path had to.
@@ -230,6 +255,14 @@ export function __setWindowsProcessTreeLoaderForTests(
   snapshotReader.reset()
 }
 
+/** Test-only: substitute the no-binding PowerShell scan, which spawns a child. */
+export function __setWindowsProcessTableCimScanForTests(
+  scan?: () => Promise<WindowsProcessRow[]>
+): void {
+  cimScan = scan ?? readWindowsProcessRowsWithCim
+  snapshotReader.reset()
+}
+
 /** Test-only: drop the shared snapshot so suites cannot serve each other's rows. */
 export function resetWindowsProcessTableForTests(): void {
   snapshotReader.reset()