Commit Graph
6 Commits
Author SHA1 Message Date
Neil 64c992cd56 fix(memory): report the Windows number that predicts paging, not just resident pages (#16211) (#16589)
* fix(memory): report Windows commit charge, not just working set (#16211)

On Windows the per-process figure was working set — resident pages only.
An agent whose pages Windows has trimmed to the pagefile shrinks its
working set while still holding the commit that pushes the host into
paging, so Resource Manager and `orca diagnostics memory` understated an
owned tree by 10-40x (9 codex.exe: 1.4 GB working set, 13.4 GB private)
and could not warn before the host was already thrashing.

Add committed private bytes as a second, separately-labelled quantity
rather than redefining the existing one:

- CIM sweep gains one property (PageFileUsage, UInt32 KB); the typeperf
  fallback gains one counter (\Process(*)\Private Bytes). Both ride the
  sweep that already runs.
- MemorySnapshot gains optional `privateMemory` per app/worktree/session
  plus `processCommitMetric` and `totalPrivateMemory`. Rule 1 additive
  optional fields: old clients ignore them, and absence reads as "not
  measured", never as zero — Unix hosts and older hosts send nothing.
- `totalMemory` and `processMemoryMetric` keep their exact meaning, so
  the "shared pages may repeat" copy stays true; the working-set copy now
  also says paged-out memory is not counted.
- Resource Manager shows "Σ Private" beside "Σ WS", and tints the badge
  yellow/red once tracked commit passes 60/80% of physical RAM — the same
  thresholds `usageTextColorClass` already uses for host usage. Tint and
  tooltip only; no toast, and the badge number is unchanged.

The parsers move to windows-process-sample-parsing.ts and the Windows
sweep tests to their own file to stay under max-lines.

Not migrating the collector to windows-process-table.ts: the native
snapshot exposes no commit figure and no CPU times, and truncates
WorkingSetSize through a DWORD. Documented in the enumeration reference.

* fix(memory): derive the typeperf field cap from the counter list

The fallback parser's 8192-field cap was sized for three `\Process(*)`
counters. Adding `Private Bytes` cut the parsable process count from ~2730
to ~2047, and overrun is a blackout (`parseTypeperfCsvLine` returns `[]`, so
the whole sweep reports nothing) rather than a truncation. The counter list
now lives beside the decoder that reads those names back out of the PDH
header, and the cap is derived from it.

Also collapses the four spellings of "omit privateMemory when unmeasured"
in collector.ts onto one `commitField` helper, drops the unread parameter
and the never-rendered `columnLabel` from `getResourceCommitMetricCopy`,
folds `getCommitPressurePercent` into the only function that called it, and
reverts unrelated Prettier churn in the Windows enumeration doc.

The commit tint's doc comment no longer claims to predict host paging: it
measures Orca's own share of physical RAM. Host commit charge / commit
limit stays a follow-up (#16211).
2026-08-26 15:43:02 -07:00
Neil 19e9ec695b perf(windows): ship the native process table to Windows relay hosts (#16598)
* feat(windows): let a relay host bind the native process table directly

The CIM fallback from #16550 answers on relay hosts, but it costs a
powershell.exe and ~1.4s per scan where the native reader costs ~57ms.
It is a parachute, not the destination.

Teach the loader a second source: the desktop app keeps resolving the
npm package, and a relay host -- which has none of our node_modules --
binds a bare `windows-process-tree.node` staged beside the bundle. The
CIM scan stays as the last resort, so a host with neither is unchanged.

Bind the addon directly rather than its package wrapper. lib/index.js
adds only a queue over getProcessList, and that queue is the wedge this
module already defends against: it latches a module-global
requestInProgress with no try/catch. We hold our own single-flight and
deadline, so going straight to the addon drops the duplicate.

Measured on a Windows 11 SSH host with ~1490 processes, running the
relay-externals bundle from the deployed relay directory:

  no addon staged   nativeAvailable=false  1247ms  (CIM)
  addon staged      nativeAvailable=true     57ms  memory restored

Degradation was exercised on that host, not just in fakes: a truncated
upload, a text file, and a foreign-arch ELF each fall through to the
scan rather than throwing, and restoring a good addon recovers. A file
that loads but lacks getProcessList is rejected by shape, because
binding to it would reject every read forever where falling through
still answers.

No artifact is staged yet, so this is inert until the packaging change
lands: today every relay takes the same CIM path it does now.

* build(relay): ship the Windows process-table addon to relay hosts

The CIM scan restored correctness on Windows SSH hosts, but it costs a
powershell.exe and ~1.4s per read where the native addon costs ~57ms. It
was always the floor, not the destination.

The addon cannot be npm-installed on a relay host: it carries a
binding.gyp, so npm rebuilds from source and the build wants
Spectre-mitigated libraries even where MSVC is already present. The
binary inside the published tarball loads, but predates our patch and
still caps enumeration at 1024 processes -- on a 1486-process host it
returned exactly 1024 rows with the querying process among the missing,
which reads as unavailable only under load. No published alternative
clears the bar either; the one fork with a working prebuild story still
carries the same cap.

So build it where a compiler exists and ship the result. The build script
refuses unpatched source -- checking the source rather than trusting the
install, because the Spectre hunk fails loudly while the 1024 hunk fails
silently -- and verifies the PE machine field so a cross-build cannot
emit host arch for another target.

The artifact is optional: hashed when present so a relay carrying it
never shares an immutable directory with one that does not, and never
probed, since requiring a file only a Windows build machine can produce
would make a correct relay read as MISSING and redeploy forever. Builds
on any other OS keep using the scan, unchanged.

arm64 cross-compiles from the x64 runner but needs the optional MSVC
ARM64 toolset, so it stays best-effort: a runner image without that
component should cost arm64 relays the fast path, not fail the release
the x64 relay is riding on. ORCA_REQUIRE_RELAY_NATIVE_ADDONS is a
per-arch list rather than a flag for exactly that reason.

* build(relay): require the arm64 process-table addon too

The arm64 cross-compile is no longer unproven. On a Windows x64 machine
with the MSVC v143 ARM64 build tools component installed, node-gyp
--arch=arm64 produces a genuine ARM64 image:

  x64    machine=0x8664  152064 bytes
  arm64  machine=0xaa64  139776 bytes

So arm64 stops being best-effort and joins x64 in the required list. It
was only best-effort because the component is optional and I had not seen
it succeed; a runner image without it now fails the build with MSB8020
naming the missing component, and that step runs before the long
packaging step so the failure costs seconds rather than twenty minutes.

The env var stays a per-arch list rather than reverting to a flag, so a
future arch can land best-effort before being promoted the same way.
2026-08-26 03:14:45 -07:00
Neil 7f034a182f 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.
2026-08-26 02:59:08 -07:00
JinjingandNeil e4d95e032d fix(windows): restore a CIM fallback for relay hosts with no native binding (#16550)
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
2026-08-25 23:15:37 -07:00
Neil 2b1254d681 fix(windows): own PTY process trees with job objects (#15755)
* fix(windows): own PTY process trees with job objects

Teardown used to answer 'is this tree mine, and how do I kill it?' by
scraping the process table, walking parent pids back to Orca, and running
taskkill /T /F only if the walk said yes. Every step is a guess, and the
code said so itself: windows-pty-root-identity.ts:35 already named the
fix -- 'an inherited handle / Job Object'.

The guesses fail in the ways users report. A pid walk cannot survive pid
reuse, so teardown refused whenever it could not prove ownership, and a
refused kill is an orphaned agent tree holding the worktree directory
open (#9045, #10475, #10087). A descendant that reparented is invisible
to the walk. The scrape itself could be blocked by policy, which read as
'no evidence'.

node-pty now creates a job object per ConPTY and assigns the shell under
CREATE_SUSPENDED, before it can spawn anything -- assigning afterwards
leaves a window in which a fast child escapes. Termination is one
TerminateJobObject; liveness is QueryInformationJobObject.

Verified on Windows 11 against a shell whose grandchild was spawned
detached: job membership came back [shell, grandchild] and one call
killed both. Neither a parent-pid walk nor GetConsoleProcessList sees
that grandchild -- it leaves the console and reparents, which is exactly
the claude.exe/node.exe/cmd.exe orphan in #9045.

KILL_ON_JOB_CLOSE means a daemon that dies without unwinding no longer
strands shells (#9195, #10415). The job is the daemon's, not the app's,
so an app-main crash still leaves sessions alive -- the guarantee
win-crash-survival-e2e asserts.

Both entry points report unavailable rather than a false success when a
pty has no job: an outer job without BREAKAWAY_OK can refuse the
assignment, and a pty from an older build has none. Reading 'we could
not tell' as 'already dead' is the original bug, so the old probe stays
as the fallback.

* test(windows): pin job ownership against a real detached grandchild

The unit tests pin the contract; this pins what the contract is for. A
grandchild spawned detached leaves the pane's console and reparents, so
GetConsoleProcessList and a parent-pid walk both miss it -- that is the
process that outlived its pane and held the worktree directory open.

Includes a guard that this build actually has job support, so a node-pty
rebuilt from unpatched sources fails loudly instead of letting every
assertion pass vacuously.

* fix(windows): correct the job liveness contract to what Windows actually does

I claimed an emptied tree would report [] and that this was the evidence
a stale registry entry lacks (#15549). Running it on Windows 11 showed
otherwise: node-pty drops its handle record and closes the job when the
shell exits, so a dead tree reports null.

Null therefore means unverifiable in the sense of
docs/reference/ssh-execution-boundary.md -- no job support, not a ConPTY,
or no longer tracked -- and is never evidence that processes died. A
caller reading it as proof of death would have been right by accident
after a normal exit and wrong on a host that refused the assignment.

What the API does add is descendant liveness for a tree that is still
tracked, including children that detached from the console.

* fix(windows): stop a clean shell exit from reaping backgrounded processes

Measured on Windows 11: with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE on the
per-PTY job, releasing the handle when the shell exits also killed
whatever the user had backgrounded. Typing 'exit' in a pane reaped a
detached server that survived before this patch.

That is a behaviour change nobody asked for. The approved change was
that killing the terminal daemon reaps its shells -- not that a clean
exit reaps your background job. The job's purpose is to make an EXPLICIT
teardown exact, which TerminateJobObject still does.

Reaping a dead daemon's shells now needs the daemon-level job the design
called for: the daemon assigns itself, children inherit membership, and
its closure on daemon death reaps them without touching clean-exit
semantics. Not in this PR; noted in the reference doc.

* test(windows): pin that a clean exit leaves backgrounded work alone

The counterpart to the tree-kill test. Without it, re-adding
JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE would look like a tightening rather
than the regression it is.

* fix(windows): stop a winpty pty id from matching a ConPTY job

winpty.cc and conpty.cc each mint their 'pty' id from an independent
counter, and windowsPtyAgent stores both in the same _pty field. So a
winpty-backed terminal's id can collide with a live ConPTY baton -- and
closing that pane would have terminated an unrelated pane's entire
process tree.

Both job entry points now take the shell pid and the native side refuses
unless GetProcessId(hShell) matches, which makes the id unforgeable.

Two more from the same read-through:
- ResumeThread's failure was ignored. A shell left suspended is a pane
  that never prints and never exits, which is far harder to diagnose
  than a failed spawn; it now cleans up and throws.
- handle->hJob was assigned before LoadConptyDll, which can throw. A
  baton carrying a job but never reaching SetupExitCallback has nothing
  left to close it, so the assignment moved down beside hShell.

* docs(windows): record the unsynchronised node-pty baton table

Pre-existing upstream -- the exit thread erases while the main thread
reads -- but terminatePtyJob adds an instance of it, so it belongs in
writing rather than in someone's head.

* fix(windows): close four gaps found in review

BREAKAWAY. The per-PTY job set no limits, so a child asking for
CREATE_BREAKAWAY_FROM_JOB was refused with ERROR_ACCESS_DENIED.
Installers, msiexec and some updater and service-control paths spawn
that way deliberately -- they worked before this patch and would have
failed only inside an Orca terminal, which is the worst shape a bug
report can take. JOB_OBJECT_LIMIT_BREAKAWAY_OK restores it; a child
still has to ask, so ordinary descendants stay owned.

EMPTY IS NOT UNAVAILABLE. The native reader returns an empty list --
not an error -- when CreateToolhelp32Snapshot fails, which is what an
EDR hook or a restricted token produces. Callers read that as 'nothing
is running' and teardown concludes a live PTY root is already gone. The
snapshot must contain the querying process; nothing else is
unfalsifiable, and one predicate catches empty, truncated and
permission-filtered tables alike.

NO DEADLINE. Replacing execFile dropped its 3s timeout. The vendored
reader latches a module-global while a request is in flight and clears
it only after draining its callbacks, with no try/catch -- so one wedge
leaves every later call queued behind a promise that never settles, and
the process table is dead for the life of the app. The bound is back.

GUESSED IMAGE PATH. executablePath was derived from the first
space-delimited token, which reads 'C:\Program' out of an unquoted
'C:\Program Files\nodejs\node.exe ...'. Wrong evidence is worse than
none, and the only consumer already had the full path in , so
the field is gone rather than repaired.

Also: remove_pty_baton no longer sits inside assert(), which NDEBUG
would compile away along with the call, and the job accessors hold a
lock across lookup and use -- handle values are recycled, so an
unguarded read could pass the shell-pid check against an unrelated
process and terminate the wrong job.

* fix(windows): apply the job lock once per accessor

The patch script matched a string its own replacement still contained, so
PtyTerminateJob got two lock_guards named guard and PtyListJobProcessIds
got none. MSVC caught it: error C2374 redefinition.

* test(windows): pin that a child can still break away from the job

Verified on Windows 11: 'start /b' writes its marker and no access-denied
appears. Without JOB_OBJECT_LIMIT_BREAKAWAY_OK this fails, and it fails
only inside an Orca terminal -- so the failure would look like Orca
corrupting unrelated software rather than like a job-object change.

* fix(windows): stop the ownership guard from reading a closing handle

The guard called GetProcessId(hShell) to prove identity, but the exit
watcher closes hShell on another thread -- so the guard could read a
closed handle, and under strict handle checks that is fatal rather than
merely wrong. Worse, it widened the gap between validating hJob and
using it from two instructions to a kernel round-trip, and handle values
recycle: the likeliest occupant of a freshly recycled value in this
process is another pane's job.

The pid never needed a handle. It is captured at spawn and compared as a
DWORD, so the guard touches no handle at all, and hShell is now closed
inside the same lock as hJob.

Also from review:
- reject CR/LF in a cmd argument. cmd ends the command at a raw line
  break whatever the quote state, so there is no escape for it; encoding
  one anyway truncates the argument and can leave the remainder to run
  as a command. Agent prompts are this encoder's motivating input.
- ask the process table only for the fields a caller needs. Memory and
  CommandLine each cost an OpenProcess per process, inline, for every
  process on the box -- and the 1024 bound is patched out. Ancestry
  reads now skip both.
- corpus gains the degenerate quote-only and two-quote arguments.
- PtyListJobProcessIds' docblock still taught the empty-list contract
  that was corrected on the TS side, and now records that the ConPTY
  console host is never a job member.
- drop a write to NumberOfAssignedProcesses, which is output-only.
- pty_baton::hShell is initialised; ownsShell was only safe because &&
  short-circuited ahead of it.

The backgrounded-child test is rescoped: 'start /b' uses
CREATE_NEW_CONSOLE, not CREATE_BREAKAWAY_FROM_JOB, so it proves job
membership does not block backgrounding -- not that BREAKAWAY_OK works.
That flag rests on the Win32 contract, and I have said so rather than
letting the test imply coverage it does not have.

* fix(windows): bound retries after the process table wedges

The 3s deadline stops a caller hanging, but the timed-out call leaves its
callback in the vendored module's queue -- and that queue drains only
when the latched request completes, which in this wedge never happens.
Retrying at the caller's poll rate would add a closure per tick forever.
A 30s cooldown bounds it to one probe, and a late callback clears the
cooldown because it proves the reader recovered.

Also pins the deadlock invariant in the patch: the exit thread's lock
must close before tsfn.BlockingCall, because that waits on the JS thread
and the JS thread can be waiting on the same mutex inside
PtyTerminateJob. Correct today by scoping; a comment so a later refactor
does not widen it.

* revert(windows): drop the field-selection API, which cannot pay off

I added it for a real perf finding -- Memory and CommandLine each cost an
OpenProcess per process -- and then never wired a caller, so the claim
that ancestry reads skip them was wrong.

Wiring it would have been worse than leaving it dead. The only ancestry
consumer is the teardown identity probe, which needs a snapshot that
started AFTER it asked, for pid-recycle detection. Bypassing the shared
reader to get narrow fields would let that request join a scan already in
flight -- trading a correctness guarantee for milliseconds.

Field selection only pays off if callers can ask for less, and they
cannot: one shared snapshot serves every caller so a 32-wide teardown
collapses into a single scan, which means it has to carry every field.
The reasoning now lives next to the flags instead of in a dead export.

* fix(process): three P1s from review — a crash vector and two wedge bugs

STDIN EPIPE COULD TAKE DOWN THE MAIN PROCESS. A child that exits without
reading makes the queued write fail with EPIPE, and an unhandled error on
a stream is an uncaught exception. The child's own error listener does
not cover its stdin stream, so runProcess({ input }) against a
short-lived child was a crash, not a failed call.

THE COOLDOWN LEAKED A BATCH PER CYCLE INSTEAD OF BOUNDING IT. At expiry
every concurrent caller passed the check before any of them re-armed it,
so each enqueued a callback into the still-latched native queue and each
cycle leaked another batch. The cooldown is now re-armed BEFORE probing,
so exactly one caller gets through.

A SYNCHRONOUS THROW LEFT ITS DEADLINE RUNNING. The timer was declared
inside the try, so catch could not clear it; it fired later and wedged a
reader that had already recovered. Hoisted and cleared, and wedge state
now carries a generation so a request that lost its deadline cannot
mutate it on behalf of the one that replaced it.

Found by review once the prompts were short enough for the reviewer to
finish -- the previous two rounds died on prompt length.

* fix(process): stop a stream error from crashing the main process

Same class as the stdin EPIPE finding, two instances further on: stdout
and stderr had data listeners and no error listeners, and an unhandled
error on a stream is an uncaught exception.

Scoped to runProcess, which owns the child outright. spawnProcess hands
the streams to its caller, and a blanket handler there defeats callers
that track and remove their own listeners -- the SSH ProxyCommand
transport does exactly that, and its cleanup test caught the attempt.
Documented on spawnProcess so the boundary is explicit rather than
inferred.

* fix(windows): validate the ConPTY DLL before creating the process

LoadConptyDll throws when conpty.dll is missing -- a real state, and one
this branch hit during development. It ran after CreateProcessW and
ResumeThread but before the baton and the exit watcher were installed,
so a throw leaked the job, process and thread handles and left an
untracked shell tree running. Once per attempt, so a broken install
accumulates orphan shells on every retry.

Resolving the DLL first costs nothing and leaves exactly two throws
after creation: the CreateProcessW failure, where nothing exists yet,
and the resume failure, which already cleans up after itself.

This also closes the same leak for hProcess and hThread, which predates
the job work.

* feat(windows): add the daemon-level job the design called for

The plan specified two nested jobs and I built one. That gap is why
dropping KILL_ON_JOB_CLOSE from the per-PTY job cost the approved
guarantee that a dead daemon reaps its shells -- I had one job trying to
answer two questions, and the two answers conflict.

They are separate jobs. The per-PTY job answers 'kill exactly this
pane's tree, now', and cannot be kill-on-close because its handle is
released when the shell exits, which would reap whatever the user
backgrounded. The daemon assigns itself to a second job that IS
kill-on-close; its handle is released only when the daemon dies.
Children inherit membership, so every pty is covered and the per-PTY
jobs nest inside it.

Daemon, never app: an app-main crash must still leave sessions alive,
which win-crash-survival-e2e asserts. Both jobs carry BREAKAWAY_OK, or a
child asking to break away is refused at whichever level lacks it.

Restores #9195 and #10415, which I withdrew from this PR earlier.

* docs(windows): record what the host job does not cover

An app-hosted PTY gets a per-PTY job but no crash reaping, because the
alternative is a kill-on-close job on the app -- which is precisely what
the crash-survival guarantee forbids.

* ci(windows): run the win32 suites in the PR windows job

Both were skip-on-non-win32 and had only ever run on one machine I drive
by hand -- which went unreachable at exactly the moment I needed to
verify the percent-escaping fix. Verification that depends on one box is
not verification.

The job already builds node-pty from patched source and already runs a
useConptyDll test, so the ConPTY runtime files are in place by this
step. This also makes the encoder a gate: the corpus is the only thing
standing between an agent prompt and a mangled argv, and it now runs
against real cmd.exe on every PR.

* fix(deps): refresh the lockfile for the current patch hashes

pnpm records a hash per patched dependency, and I regenerated both
patches repeatedly across the review rounds without refreshing the
lockfile. Every local run used --frozen-lockfile's looser sibling, so
nothing caught it until CI did:

  ERR_PNPM_LOCKFILE_CONFIG_MISMATCH  Cannot proceed with the frozen
  installation. The current "patchedDependencies" configuration doesn't
  match the value found in the lockfile

Verified with pnpm install --frozen-lockfile locally this time.

* ci(windows): build node-pty from source before the win32 suites

CI proved the encoder fix on real cmd.exe -- 26/26 -- and in the same run
proved the job suite had been testing an unpatched binary. node-pty
prefers its upstream prebuild, which does not contain this patch, so
every job-object export was absent and isPtyJobOwnershipAvailable() was
false.

That guard is why the failure was loud rather than a vacuous pass, and
it is the reason the assertion exists.

Packaging was never affected: rebuild-native-deps.mjs already builds
node-pty from source for Electron and restores the ConPTY runtime files.
The gap was the node-runtime test environment only.

Not changing requiresPatchedNodePtySourceBuild's win32 exemption here.
Its premise -- that the patch is Unix-only -- is now false, but lifting
it also needs pnpm rebuild to force a source build, and I cannot
validate that on macOS and Linux from here. Recorded as a follow-up
instead of changed blind.

* test(windows): gate the host-job guarantee in CI

The daemon-level job had one hand-run proof and no automated coverage --
the same shape of gap that let an unpatched node-pty go unnoticed until
CI caught it.

It needs a real second process, because the assertion is about what
happens when that process is force-killed: a host in a kill-on-close job
must strand neither its pty nor a grandchild spawned detached, which is
the process a parent-pid walk cannot see.

Runs in the Windows PR job alongside the per-pty and encoder suites, so
both halves of the two-job design are now gated rather than asserted.

* fix(windows): serialise host-job creation

Two callers racing PtyAssignCurrentProcessToJob would each create a job,
put the process in both, and leak the first handle -- and the handle is
what keeps a kill-on-close job alive, so a leaked one is never released.
'Only JS calls it' is not a guarantee: a worker thread with its own
N-API env shares these statics.

Also records the ordering requirement it depends on.
AssignProcessToJobObject adds only the named process; children inherit
membership, but a pty that already exists does not join retroactively
and would not be reaped. The daemon assigns at startup, before the
ConPTY warmup and before any session, which is correct today and now
stated rather than implied.

* fix(daemon): keep the host job off the startup path

Assigning the host job at daemon startup resolves the node-pty native
module, which loads the ConPTY addon -- and paying that before the
endpoint is published delayed readiness enough that daemon-boot-smoke
failed on windows-latest, deterministically.

windows-conpty-warmup already carries the comment for this exact
hazard ('setImmediate keeps the ready/handshake path ahead of the
warm-up') and I put an eager load in front of it anyway.

Moved to the pty spawn path, which already pays ConPTY cost, and
memoised. Children inherit job membership, so assigning immediately
before the first spawn still covers every pty -- and nothing can spawn
one before the endpoint exists.
2026-08-21 22:31:36 -07:00
Neil 057fbfcffc perf(windows): read the process table natively instead of forking PowerShell (#15749)
* perf(windows): read the process table natively instead of forking PowerShell

Seven independent readers each forked powershell.exe to run
Get-CimInstance Win32_Process, with a wmic fallback that Windows 11 24H2
has removed. On a domain-joined host with PowerShell Transcription
enabled by policy, one of them running every ~2s recorded ~289GB across
1.4 million files (#15209). The same scan cost ~700ms and ran per pane
(#15036), and a Group Policy or AV block turned it into 'unavailable',
which callers read as 'no evidence' -- which is how a PTY tree survives
its own teardown (#9045, #10475).

A Toolhelp32 snapshot answers the same question with no child process.
Measured on Windows 11 with 1050 processes, p50/p95:

  pid+ppid+name          15.9 / 17.5 ms
  +memory +command line  30.6 / 33.7 ms
  Get-CimInstance         706 / 723  ms

Two upstream defects needed patching, both found by running it on real
hardware. The binding requires Spectre-mitigated libraries our agents do
not carry (node-pty is patched the same way). And enumeration stopped
after 1024 processes: on a host with 1051 the module returned exactly
1024, and the querying process was itself among the 27 missing -- a
truncated snapshot silently hides the descendants teardown is looking
for, which is the failure this whole change exists to remove.

Migrated: the foreground/descendant reader (the #15209 scraper and the
teardown identity gate) and the port scanner's PID attribution. NOT
migrated: the memory collector and three identity probes, which need
Win32_Process.CreationDate and have no native equivalent. Start time is
a proxy for identity anyway; an inherited job handle is the real answer,
so those belong with the job-object work rather than here.

Packaging follows the windows-native-registry contract exactly:
optional, absent from onlyBuiltDependencies so macOS/Linux never run
node-gyp, win32-only in the packaged runtime. Asserted by the existing
contract test, which also stops pinning a whole source literal that only
tested its own formatting.

* chore(process): ratchet the child_process allowlist down

windows-foreground-process-rows.ts no longer spawns anything, so its
allowlist line is stale. The guard fails on a stale entry as well as a
new one, precisely so a migrated file cannot keep a slot open and hide
the next regression in the same path.

* fix(ports): import the process-table reader the scanner uses

Missing import: the migration replaced the PowerShell call but the new
symbol was never imported, so tsc failed. Vitest transpiles without
typechecking, which is why the port-scanner suite stayed green.

* fix(deps): sync this branch's lockfile with its patch set

Same class as the fix on the tip branch: pnpm records a hash per patched
dependency, and this branch introduces the windows-process-tree patch
without its lockfile entry matching. Every job here failed at install
with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH.

Verified with --frozen-lockfile, which is what CI runs and what my local
runs were not.

* test(relay): drive the relay's Windows fixtures from the native snapshot

Two relay cases fed a PowerShell CIM payload through a mocked execFile.
That reader is gone, so both failed -- deterministically, on every PR
run for this branch and the one above it.

I did not catch it because my own verification sweep was
'src/main src/shared config/scripts' and never included src/relay. The
relay is a first-class consumer of the process table; leaving it out of
the sweep is how a deterministic failure survived six review rounds.
2026-08-21 21:54:57 -07:00