mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
OrcaWinandOrca Worker
687a22e1ee
* fix(computer-use): run the Windows runtime as one persistent helper
Microsoft Defender for Endpoint raised multi-stage Execution + Collection
incidents against Orca on Windows ("Screenshots were taken unexpectedly on
this device... Screen capture code was found in a script launched by
powershell.exe", factor "Executes suspicious MSIL code"). The desktop script
provider spawned a fresh powershell.exe per operation, so a single computer-use
session produced a burst of short-lived PIDs and re-emitted runtime.ps1's
inline Add-Type P/Invoke assembly on every click.
runtime.ps1 gains a -Serve mode that loads its assemblies once and then reads
NDJSON requests from stdin, and a new DesktopScriptRuntimeHost owns one
long-lived child: lazy spawn, strict serialization, a 30s per-request timeout,
restart on crash, a 120s idle shutdown, and dispose() on provider teardown. The
one-shot -OperationPath path stays as the fallback, and Linux keeps its python3
bridge unchanged.
Both Windows spawn sites now use -ExecutionPolicy RemoteSigned instead of
Bypass, falling back once to Bypass (and logging) when a Restricted host
refuses the unsigned script.
* fix(computer-use): recover the runtime host instead of latching it off
Review follow-up on the persistent Windows computer-use helper.
A helper that died before producing a line set an unavailable flag nothing ever
cleared, and the client then dropped the host for the life of the session. One
transient bad spawn — a Defender scan, a locked CSC temp directory — silently
restored the per-click powershell.exe burst and per-operation MSIL emission this
work exists to remove, with computer use still working so nothing looked wrong.
Start failures are now retried, then cool down for 60s, then re-probed; the
client keeps the host so it can come back. Repeated post-answer crashes cool
down too, and a single reply no longer clears the failure count.
The one-shot bridge decided its execution-policy retry from a message that fell
back to stdout, so a window title containing "SecurityError" could replay a
non-idempotent operation — a double click, keystroke or paste — and stick the
session on Bypass. The retry now requires empty stdout and a matching stderr.
Serve-mode replies carry an echoed request id. Without one a single stray stdout
line would make every later response answer the previous request, acting on
stale element indexes with no error raised; a mismatch now kills the child.
Non-JSON noise is ignored rather than counted as the helper having answered.
Also: warnings reach the main process over the sidecar's IPC channel rather than
its piped, unread stdio; the child is watched on close rather than exit; dispose
latches so a queued request cannot respawn during teardown; and the host is
split into a serve channel and an availability policy to stay under max-lines.
* fix(computer-use): prove a helper never started before replaying its request
The retry that replaced the permanent-latch bug could deliver unrequested
input. send() re-sent the same request whenever the helper died without
replying, but "no reply came back" is not "the operation did not run":
runtime.ps1 synthesizes the click and only then builds the snapshot, which
allocates a full-window bitmap and walks the UIA tree — a native GDI+/UIA fault
there is uncatchable, and leaves the click already delivered. A deterministic
fault meant three clicks from the host plus a fourth from the one-shot bridge,
surfaced as a single failed operation.
-Serve now writes one {"ready":true} line after its Add-Type work and before
its first read, so "never started" is a fact rather than an inference. A request
is replayed only when the helper died before announcing. A runtime.ps1 that
predates the announcement — reachable through the provider path override — is
covered by an observation-tool allowlist until a ready line proves otherwise.
Host-detected aborts (timeout, desynchronised reply, oversized line) suppress
the exit handler, so they were bypassing failure accounting entirely and a
helper failing that way was respawned once per operation forever. They now
count and are logged.
Also stop charging twice for one outage: entering the cooldown resets the
failure count, so the first death after recovery no longer re-enters a full
cooldown and an interleaved workload cannot be stranded on the one-shot bridge.
* fix(computer-use): ignore a stdin write callback from a torn-down helper
stop() destroys stdin, so a write still queued at teardown calls back with
ERR_STREAM_DESTROYED. The callback carried no channel or request identity and
write() had no closed guard, so it ran abortChannel a second time: stopChannel
no-opped but recordFailure and the warning did not, charging two failures for
one operation and reaching the 3-strike cooldown at half the intended rate.
That feeds the same accounting that keeps a persistently broken helper from
respawning once per operation.
The same root also allowed a late callback landing after a replacement channel
existed to stop that channel and reject a different request with the previous
one's error. Node fires the destroyed-stream callback on the next tick, well
before a new request arrives, so the double-count is the reachable effect;
binding the callback closes both.
write() now drops payloads and error reports once closed, and the host ignores
any report whose channel or request id is no longer current.
* test(computer-use): pin each stale-write guard independently
The channel's closed guard and the host's request-identity check are redundant
by design, and the existing tests only failed when both were absent. Someone
deleting one, believing the other was the covered one, would have got a green
suite and a live regression — the same shape as a test that passes without the
fix it was written for.
Each is now pinned on its own. The channel's half is tested against the channel
directly: after stop() it takes no writes and reports no error from one already
queued, which the host cannot observe because it drops the channel at the same
moment. The host's half is pinned by the case the channel cannot see — a live
channel whose request was already answered, where backpressure delivers a write
callback for a request that is no longer pending.
Removing either guard alone now fails a test. Both carry a comment saying they
are deliberately redundant and separately pinned, so the next reader does not
have to rediscover this from the diff.
* ci(windows): run the computer-use runtime host suite in CI
The win32 suite only self-skips off Windows, so it passed vacuously in
every lane. Register it the way the cmd-shim suite is registered.
* fix(computer-use): time the runtime host cooldown on a monotonic clock
The start-failure cooldown was a wall-clock deadline, so a backwards step —
an NTP correction, a VM snapshot restore, a user changing the clock — left
`remainingCooldown()` returning the cooldown plus the whole step. A one-hour
step measured 3,660,000ms, and ten real minutes later still 3,060,000ms.
Nothing shortens it from there. Only `recordSuccess()` clears the cooldown on
a non-dispose path, and no request can reach a helper to succeed while it
holds, so every `send()` throws `runtime_host_unavailable` first. The host is
built with no `now` override and its lifecycle is a module-level singleton
that shuts down at process exit, so the latch held for the sidecar's life —
computer use kept working via the one-shot bridge while the per-click
powershell.exe burst this host exists to remove came back silently.
Store the instant the cooldown began and compare elapsed monotonic time,
following the two fixes in #17884. The field is `number | null` rather than
sentinel 0 because `performance.now()` legitimately returns 0.
Both new tests leave `now` unset, because the bug was in the default the host
picks and a test that injects a clock cannot see it.
* fix(computer-use): give a queued request its own deadline
The 30s request timeout was armed only in `sendOnce`, once a request reached
a helper. A request behind N timing-out ones therefore waited roughly N times
that with no deadline of its own: bounded, but the caller sees an `await` that
looks hung for minutes and gets no error to act on.
Move the serialization tail into its own class and arm a deadline at enqueue
time. Only the wait is bounded — a request that reaches a helper still gets
its full execution budget, so nothing that used to succeed now fails. An
expired request is dropped rather than sent late: the caller has already been
told it failed, and a click delivered after that is worse than no click.
The tail keeps its never-rejecting shape and chains on the turn rather than on
the raced promise, so a caller giving up early cannot release the next request
while its predecessor is still in flight.
* fix(computer-use): stop reading a locked file as an execution policy block
`UnauthorizedAccess` is the FullyQualifiedErrorId PowerShell reports for a
policy block, and it is also a strict prefix of `UnauthorizedAccessException`,
which .NET raises for any ordinary locked or ACL-denied file. The predicate
matched the token unanchored, so an AV scan holding runtime.ps1 or a locked
CSC temp directory was read as a policy block.
Two consequences, both bad. `escalateExecutionPolicy()` has no path back, so
one false match spent the rest of the session on `-ExecutionPolicy Bypass` —
the exact command line token this stack exists to stop emitting. And on the
one-shot path `isPolicyBlockedStart` re-runs the operation: one-shot mode
writes stdout only after the operation returns, so a crash partway through an
action is indistinguishable from a helper that never started, and the click
lands twice.
Measured on Windows against all three records, which the test carries verbatim
as fixtures:
policy/Restricted FullyQualifiedErrorId: UnauthorizedAccess
policy/RemoteSigned FullyQualifiedErrorId: UnauthorizedAccess
genuine access denied FullyQualifiedErrorId: UnauthorizedAccessException
`\b` is the whole discriminator: between `s` and `E` both sides are word
characters, so no boundary exists there and the exception cannot match.
Dropped two alternatives that measurement showed were wrong. `PSSecurityException`
never appears — the record surfaces through a native-command wrapper and reports
`ParentContainsErrorRecordException`. The prose is wrong three times over: it
differs by policy, it is localized, and PowerShell hard-wraps it mid-sentence.
Anchoring on the `FullyQualifiedErrorId:`/`CategoryInfo:` labels would be more
precise again, but those labels are localized where the values are not, so it
would lose a real block on a non-English host and strand it with no fallback.
Matching the values with word boundaries keeps both directions; a fixture with
translated labels pins it.
The escalation stays sticky. With the predicate correct, it only fires on a
machine that really does block, where re-probing the preferred policy would buy
a guaranteed failed spawn per operation.
* fix(computer-use): route a malformed request back to the request that caused it
`ConvertFrom-Json` throws before `$requestId` is read, so the serve loop
answered an unparseable request with an untagged error. On the client that is
not an error at all: `deliver()` sees no matching id, calls `abortChannel`,
kills the helper and charges a failure — and the helper's own message is
discarded. A parse failure was reported as a stream desync with no trace of
the real cause, and three of them walked into the 60s cooldown behind three
misleading "did not match" messages.
Recover the id from the raw line when the parse fails. No wire change: the
response shape is untouched and `BridgeResponse.requestId` already documents
this echo. It is the same shape the helper already returns for `not_a_tool`,
where the id survives because it is read before the operation runs. Both
mixed pairings degrade safely — a new script with an old client resolves the
error normally, and an old script with a new client still aborts, but now
reports what the helper said.
When the line is mangled past recovering an id, the desync abort is the honest
outcome, so keep it and carry the helper's text into it rather than replacing
it. A line the helper could not tag is usually the only account of the cause.
Proven against the real `runtime.ps1 -Serve`: the host can only write
well-formed JSON, so the parse-failure branch is unreachable through it and
the test drives the channel directly.
* fix(computer-use): keep the Bypass escalation only when Bypass actually works
AppLocker and WDAC constrained language mode raise PSSecurityException under
the same SecurityError category a real execution-policy block uses, so the
predicate matches them - correctly, on the evidence available. But those block
the script at parse time, which `-ExecutionPolicy Bypass` cannot lift. The
escalation was sticky unconditionally, so on a WDAC host we misdiagnosed,
retried, failed again, and then latched: every later command line carried the
most heavily weighted MDE token there is, on exactly the hardened, monitored
enterprise machine that is watching for it.
Treat the escalation as the diagnosis it is. A fallback that cannot start a
helper either disproves it - the policy was not what stopped the first attempt
- so revert to RemoteSigned instead of latching. When Bypass does start a
helper the diagnosis is confirmed and it stays sticky exactly as before, so a
genuinely Restricted machine still never pays a re-probe per operation.
The revert lands inside the outage rather than only at its end, so a
misdiagnosis costs one Bypass command line instead of one per attempt, and an
escalation that never proved itself does not outlive the cooldown that ends
the outage. Deliberately not a permanent "fallback is useless" flag: a Bypass
attempt that failed for a transient reason would then disable the fallback for
the session, which is the same latch in the other direction.
Only `runtime_host_unavailable` proves no helper started, so only that reverts;
a helper that started and then died proves Bypass works. That also makes the
policy branch reachable on a final attempt for the first time, so it now
rejects as unavailable rather than a generic error - that code is what routes
the operation to the one-shot bridge, which carries its own policy fallback,
and without it an all-blocked host would fail operations outright instead of
degrading. The pre-existing "reports itself unavailable when Bypass is also
refused" test pins that.
---------
Co-authored-by: Orca Worker <orca-worker@localhost>