mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
OrcaWinandOrca Worker
fc5fa16870
* perf(windows): split the process table into two flag sets
MDE flags "suspicious memory activity" on the process-table reader: it
opened a handle into every process on the box and read each one's PEB on
a repeating cadence. Two changes narrow that.
Drop `Memory` outright. It cost a second
OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ) plus
GetProcessMemoryInfo per process, and nothing reads a working set off
this table -- the Resource Manager runs its own sweep, and the addon
stores WorkingSetSize into a DWORD so anything above 4 GB wraps.
Split the rest in two. `readWindowsProcessIdentityTable[Fresh]` is a
bare Toolhelp32 walk with zero per-process handles, and returns
`WindowsProcessIdentityRow`, which has no `command` to read.
`readWindowsProcessTable[Fresh]` keeps the command line for the callers
that match on it. PTY root identity and the owner start-time probe move
to the cheap reader; agent recognition, port attribution, codex turn
processes and structured-TUI matching all genuinely need the command
line and stay.
Two independently single-flighted caches, never one per caller: the
fan-out this module prevents is one scan per caller, and each reader
still serves every caller wanting its flag set. The wedge gate and the
3s deadline stay shared, because both readers call the same addon and
one wedged read latches its one `requestInProgress`. With no binding
there is only the 1.4s PowerShell scan to run, so the identity view
rides the detailed snapshot rather than forking a second one.
Measured on Windows 11, 492 processes (p50/p95): identity 6.3/7.0 ms,
detailed 12.3/13.4 ms, previous memory+commandLine 13.1/14.1 ms.
* fix(windows): serialize native process-table reads across flag sets
The two flag-set readers could both be in flight at once, and the
vendored wrapper does not tolerate that. `getRawProcessList` pushes the
callback onto one list and calls the addon only when no request is in
progress, so a second concurrent caller's `flags` are DISCARDED and it
is handed the first caller's rows. Measured against the real addon:
identity issued first, both callers got the same array, 0 of 541 rows
with a command line. A detailed read overlapping an identity read
therefore returned a table with every command line empty, which agent
recognition reads as "no agent" -- silently, and only under concurrency.
Nothing already here excluded that. Each snapshot cache single-flights
only within itself, and the wedge set latches only after a read misses
its 3s deadline, so through the healthy ~12ms of a scan neither reader
excluded the other. Overlap is the normal state: panes poll detailed at
750ms while a teardown takes identity snapshots.
`nativeReadGate` admits one native read at a time across both flag sets.
It also fixes the relay path, where `adaptAddon` has no queue at all and
two simultaneous CreateToolhelp32Snapshot calls are the crash the
vendor's queue exists to prevent. Every link settles, so a wedged read
never strands a waiter; the waiter re-checks the wedge and rejects. With
one call outstanding, retention stays bounded at one callback rather
than one per reader.
Also from review:
- The CIM fallback now belongs to the detailed flag set alone, and the
identity view projects that snapshot through `toIdentityRow`, so an
identity row carries no command line on a no-binding host either.
- The concurrency test modelled the wrapper's coalescing queue, which
the previous synchronous mock could not express; verified failing
without the gate and passing with it.
- `agent-session-process-identity-probe` early-returns when the
creation-time flag is unavailable, which no shipped addon build
provides, instead of scanning the table to produce null.
- Corrected the cost framing: Memory took an OpenProcess(...|VM_READ)
it never read through, so dropping it halves per-process handle opens
and leaves the PEB/ReadProcessMemory telemetry unchanged.
* test(windows): keep read exclusion across resets and flag each field
Two review follow-ups, both about tests passing for the wrong reason.
`resetNativeReaderState` replaced the read gate with a resolved promise,
so waiters still holding the old chain ran beside reads queued on the
new one. Reachable only from the `__set*ForTests` hooks, which is what
makes it worth fixing: it hands a suite two concurrent calls into its
own mock addon -- the exact condition the concurrency tests exist to
detect. Chain onto the gate instead; every link settles within the
deadline, so the bounded wait that costs is the right trade.
The coalescing mock shaped every field off the CommandLine bit, so an
identity read that did request CreationTime got `creationTimeMs`
stripped. The identity-side assertion was then only `!('command' in
row)`, which a correctly flagged read and a coalesced one satisfy
equally: a future regression losing identity flags under concurrency
would have kept the case green. Gate each field on its own bit and
assert `creationTimeMs` positively, inside the helper both orderings
share.
Concurrency assertions move to a new bare-addon mock. The coalescing
mock's own latch means it can never report more than one call in
flight, so measuring exclusion there proved nothing; the bare addon has
no queue -- like `adaptAddon` on a relay, where re-entering
CreateToolhelp32Snapshot is a real crash -- and makes re-entry visible.
Verified by deletion: restoring `nativeReadGate = Promise.resolve()`
fails the reset case with `expected 2 to be 1`, and restoring the
single-bit mock fails both overlap orderings on `creationTimeMs`.
* docs(windows): count the third test defect in the list that names them
The section opened "Two defects have now shipped", numbered two, then
described the third in its closing paragraph -- a list that reads as a
complete account while quietly omitting one, which is the exact failure
the section exists to warn about. Say three and number it, and note that
the third arrived inside the fix for the first two.
Also record why the creationTimeMs and flags-array assertions are not
redundant, in the doc and beside the assertions: the flags array catches
a read served another flag set's rows, the positional creationTimeMs
check catches field shaping (identity dropping CreationTime, or
toIdentityRow not forwarding it). Neither sees the other's failure.
* docs(windows): stop describing a PEB read this release removed
Every comment here that justified the flag split in terms of PEB reads became
false when the command-line reader moved to the kernel. Left alone, the
enumeration doc contradicted itself inside one file: the flag-set section
described three chained `ReadProcessMemory` calls per process while the
sections below it explained that the addon contains no such primitive and has
no PEB fallback.
The measurement is now attributed rather than merged. Dropping `Memory` halved
the per-process handle opens and nothing else -- both handles carried
`PROCESS_VM_READ` at the time -- and it was replacing the PEB walk that took
`PROCESS_VM_READ` and `ReadProcessMemory` out of the addon. Neither change
substitutes for the other, which is worth keeping straight: the split's
remaining value is the handle itself, not the memory access.
Also adds `relay/windows-port-scan.ts` to the caller table, the one caller this
effort introduced, and records that it reads only pid/name through the detailed
reader -- free while a pane is polling, not free on a headless relay.
* test(windows): pin the fresh links path against the identity TTL cache
The identity and detailed tables are separate snapshot readers with
independent TTLs, so the detailed path's existing freshness guard says
nothing about the ancestry walk's. Cover the identity reader on its own.
---------
Co-authored-by: Orca Worker <orca-worker@localhost>