From 7f034a182fd47f8a5e0bd30e274d35d1f40fd222 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:59:08 -0700 Subject: [PATCH] docs(windows): correct why the process-tree addon is not installed on relay hosts (#16565) The note said the package "ships no prebuilds". It does: the published 0.8.0 tarball carries build/Release/windows_process_tree.node, apparently an accidentally published MSVC build directory (.obj and .tlog files ship with it). The conclusion was right and the reason was wrong, so record what was actually measured on a Windows SSH host with 1486 processes. Installing it normally rebuilds from source, because the tarball carries a binding.gyp and npm runs node-gyp regardless of what is already compiled inside. That build fails with MSB8040 (Spectre-mitigated libraries) even on a host that already has MSVC Build Tools 2022 -- the requirement our binding.gyp patch deletes, and patches do not cross SSH. Skipping the build keeps the tarball binary, which loads (it is N-API) but predates the src/process.cc patch and still caps enumeration at 1024. On that host it returned exactly 1024 rows with the querying process among the missing, which the self-presence guard rejects -- so it would work on a quiet machine and fail only under load, the shape of bug that survives testing. Also records the measured cost of the fallback, since the table's 706ms figure is from a 1050-process host and reads as more headroom than there is, and names the fix for the tracked gap: ship our own patched .node as a relay asset, as config/relay-assets already does for node-pty. --- docs/reference/windows-process-enumeration.md | 61 ++++++++++++++++--- .../relay-native-dependency-coverage.test.ts | 14 +++-- .../windows/windows-process-table-cim-scan.ts | 12 +++- 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/docs/reference/windows-process-enumeration.md b/docs/reference/windows-process-enumeration.md index 5bd023acfcc..208fcae8d9f 100644 --- a/docs/reference/windows-process-enumeration.md +++ b/docs/reference/windows-process-enumeration.md @@ -37,16 +37,59 @@ 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 | +Those CIM numbers are from a 1050-process host. The scan scales with process +count: on a 1486-process Windows SSH host it measured **1.36 s** and produced +**4.8 MiB** of JSON, against the fallback's 3 s and 8 MiB limits. Both limits +match the pre-#15749 reader, so relay hosts are at parity rather than newly at +risk — but the headroom is roughly 2x on time and 1.7x on bytes, not the ~4x the +706 ms figure implies. On overflow the output is truncated, the JSON fails to +parse, and the read rejects, so a busy host loses the table rather than +receiving a wrong one. + ## 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. +not added there on purpose. Both ways of installing it fail, and both were +checked on a real Windows SSH host with 1486 processes: + +**Installing it normally rebuilds from source, and that build fails.** The +tarball carries a `binding.gyp`, so npm runs `node-gyp rebuild` regardless of +what is already compiled inside it. On a host that *already had* MSVC Build +Tools 2022 installed, that build still failed: + +``` +error MSB8040: Spectre-mitigated libraries are required for this project. +``` + +That is the requirement the `binding.gyp` hunk of our patch deletes, and the +patch cannot reach a remote host — pnpm patches do not cross SSH. Relay deploy +would then break outright rather than degrade: `installNativeDeps` throws on +failure, and the toolchain-skip retry is gated to Linux. + +**Skipping the build and using the shipped binary returns a truncated table.** +Contrary to what this file used to claim, the published 0.8.0 tarball *does* +contain `build/Release/windows_process_tree.node` — an MSVC build directory that +looks accidentally published (`.obj` and `.tlog` files ship with it). It is +N-API, so it loads on any modern Node. But it predates our patch and still has +the `process_count < 1024` cap, so on that 1486-process host: + +``` +LOADED OK +rows=1024 +selfPid=21964 present=false +``` + +Exactly 1024 rows, with the querying process itself among the missing. The +self-presence guard rejects that, so the fallback engages anyway — but only on +hosts busy enough to cross the cap. That is worse than no binding at all: it +works on a quiet machine and fails silently under load, which is precisely the +shape of bug that survives testing. + +So the constraint is not that no binary exists to ship. It is that the only +binary available to ship is the broken one, and building the good one needs a +toolchain the remote does not have. Instead, `windows-process-table.ts` falls back to `readWindowsProcessRowsWithCim` (`windows-process-table-cim-scan.ts`), the @@ -65,8 +108,12 @@ 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. +The native fast path stays unavailable on relay hosts until we ship our own +patched `.node` as a relay asset — the addon is N-API, so one binary per +`RELAY_BUILD_PLATFORMS` Windows arch would load against whatever Node the remote +runs, and `config/relay-assets/` already carries a shipped-with-the-relay file +for node-pty. That is a packaging change, not a code change, and it is a real +gap tracked separately. ## Why the package is patched diff --git a/src/main/ssh/relay-native-dependency-coverage.test.ts b/src/main/ssh/relay-native-dependency-coverage.test.ts index 26203869d76..6da82e593a5 100644 --- a/src/main/ssh/relay-native-dependency-coverage.test.ts +++ b/src/main/ssh/relay-native-dependency-coverage.test.ts @@ -25,11 +25,15 @@ const REPO_ROOT = resolve(import.meta.dirname, '..', '..', '..') */ const DEGRADES_WITHOUT_INSTALL: Record = { '@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.' + 'Windows-only, and both ways of installing it fail. A normal install rebuilds ' + + 'from source because the tarball carries a binding.gyp, and that build fails ' + + 'with MSB8040 (Spectre-mitigated libraries) even on a host that already has ' + + 'MSVC Build Tools — our patch drops that requirement, and pnpm patches do not ' + + 'cross SSH. Skipping the build keeps the tarball binary, which predates the ' + + 'patch and still caps enumeration at 1024 processes, so a busy host gets a ' + + 'truncated table missing its own pid. windows-process-table.ts falls back to a ' + + 'Get-CimInstance scan when the binding is absent. See ' + + 'docs/reference/windows-process-enumeration.md.' } /** Addons are the packages npm has to build or unpack a binary for. */ diff --git a/src/main/windows/windows-process-table-cim-scan.ts b/src/main/windows/windows-process-table-cim-scan.ts index f638955d327..898f0c36f33 100644 --- a/src/main/windows/windows-process-table-cim-scan.ts +++ b/src/main/windows/windows-process-table-cim-scan.ts @@ -9,9 +9,15 @@ import type { WindowsProcessRow } from './windows-process-table' * 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. + * Callers read that as "no evidence", so a pane keeps whatever name node-pty + * reported -- the shell, usually -- instead of the agent running under it, for + * the life of the relay process. This restores the v1.4.188 answer on exactly + * those hosts; the local app ships the addon and never reaches this path. + * + * Measured on a Windows 11 SSH host with 1486 processes: 1.36s and 4.8MiB of + * JSON per scan, against the 3s / 8MiB limits below. Both limits match the + * pre-#15749 reader, so this is parity, but the headroom is thinner than the + * 706ms figure in docs/reference/windows-process-enumeration.md suggests. */ const WINDOWS_CIM_QUERY_TIMEOUT_MS = 3_000