starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-09-22 00:02:31 +00:00
Code Issues Packages Projects Releases Wiki Activity
10,351 Commits 7,622 Branches 1,105 Tags
44a74baf73b2bcbfa0a9727fa4d0505bc17f2386
Commit Graph
1004 Commits
Author SHA1 Message Date
Jinwoo Hong 15d0f8aedf skills: rewrite the seven non-orchestration guides to one outcome-first standard (#18724) 
<!-- orca-pr-loc -->
<!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. -->

| | Files | Added | Deleted | Net |
| :--- | ---: | ---: | ---: | ---: |
| Test | 6 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​544 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​49 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​495 |
| Prod | 36 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1719 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​1703 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​16 |

<!-- /orca-pr-loc -->

## ELI5

Orca ships eight skill guides that agents read before running the CLI. Seven of them (everything except `orchestration`, which #16904 rewrites) were command catalogs that had drifted from the binary. This PR rewrites them so an agent reads the outcome, the done bar, and the safe-failure rule first, loads reference material only at the step that needs it, and never sees a command or flag the installed CLI does not define.

## What changed

- **Seven guides rewritten** to one standard: outcome spine first (Result / Done / Safe failure), conditions instead of case lists, one done bar, one autonomy envelope, references loaded at the point of use via `skills get <topic> --full`, every runnable invocation spelled `ORCA`. `orca-cli` is 424→260 always-loaded lines with three references (browser, automations, publishing); `orca-per-workspace-env` is 794→397 with five (provider-vercel, ssh-host, docker-ssh, windows-scripts, failure-modes).
- **Defects fixed in shipped guides:** `emulator camera` (no such command), iOS `permissions` (backend refuses it), Android pane described as "in development" (shipped in June), `relayGracePeriodSeconds: 0` documented as immediate teardown (it is unbounded), doctor `ok: true` hiding `warn`, an SSH exemplar setting both `jumpHost` and `proxyCommand`, a provisioned-root fetch from `origin`, the Linear unconfirmed-write rule keyed on four verbs when ten emit it. Linear and emulator descriptions dropped embedded commands and angle-bracket placeholders (651→329, 732→404 chars).
- **Generator bundles references.** `skill-guides/<name>/references/*.md` is appended to `--full`; `skills get` help says compact by default, full with references.
- **Stubs single-authored.** The resolver ladder, placeholder rule, and older-binary fallback shared by all eight installable `SKILL.md` files come from one `skill-stubs/_shared/cli-resolution.md` fragment composed by the generator. Projections were byte-identical before the content fixes.
- **Guards:** every `ORCA <cmd>` and flag in every guide and reference resolves against `COMMAND_SPECS` (this found the camera defect); descriptions ≤1024 chars with no angle-bracket tokens; reference routing checked both directions; an always-loaded size ratchet (300 lines) that guides may leave but never join. `orchestration` (440 lines on main) is recorded as an exception until #16904 lands its kernel.

## Relationship to #16904

Split out of #16904 so that PR carries only the orchestration guide. On main, `terminal send` has no `--wait-submit` / `--retry-request` and the orchestration kernel still carries the resolver ladder and worktree-selector rule, so this branch pins `accepted: true` for handoff receipts and leaves the orchestration pins where main has them. The merge in either direction is mechanical: #16904 rebased on this becomes a one-file `orchestration.md` change plus dropping the two exceptions.

## Standard

Compound Engineering's portable skill-authoring guidance (outcome spine, conditions not cases, pinned fragile commands with an ordered hatch, references at point of use). NVIDIA SkillEvaluator Tier 1 (`schema,pii,license,quality,unicode,lint`) was run on every guide; its deterministic checks pass, its template nudges (Instructions/Examples sections, 50–150 char descriptions) do not apply to Orca's stub architecture and were not applied.

## Testing

- `pnpm typecheck:tsc:cli` clean; `check:code-quality:changed` and `check:react-doctor:changed` 0 findings
- `pnpm verify:bundled-skill-guides` and skill-bundle manifest verify clean
- vitest over `config/scripts`, `src/cli/skill-guide-cli-parity.test.ts`, `src/cli/skills.test.ts`, `src/cli/specs/skills.test.ts`, `src/cli/help.test.ts`, `src/main/skills`: 240 files / 2,019 pass
- Live smoke on the built CLI of every `skills get <topic>` and `--full`, every emulator, linear, and vm verb named in the guides, and every projection's resolver, GNOME warning, and bounded fallback (done on the #16904 branch before the split; the guide bodies are identical here except the send-receipt vocabulary noted above)

## Deferred product decisions

Merging `orca-emulator` and `orca-emulator-android` into one skill with a platform branch; collapsing `linear-tickets` to a guide alias; a `skills get --reference <name>` selector so a gate table can load one file; a fresh-agent routing eval before trimming the `orca-cli` (1,015 chars) and `orchestration` descriptions, whose quoted triggers each fixed a routing misroute.
 2026-09-06 16:10:16 -04:00
Brennan BensonandMerge Sim 298571ad9f fix(codex): uncap app-server stdio records (#18590) 
Co-authored-by: Merge Sim <sim@local>
 2026-09-06 11:49:42 -07:00
Jinwoo Hong 0c33f58e8a fix(ssh-relay): daemon owns the endpoint credential; a losing start never rotates it (#19052) 
<!-- orca-pr-loc -->
<!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. -->

| | Files | Added | Deleted | Net |
| :--- | ---: | ---: | ---: | ---: |
| Test | 19 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​962 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​136 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​826 |
| Prod | 18 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​295 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​116 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​179 |

<!-- /orca-pr-loc -->

## Symptom

Live 2026-09-05 (Orca 1.4.198 client, Ubuntu host): both relay processes `kill -STOP`ped for 20 s, then `-CONT`. The client redeployed while the host was frozen. Its fresh daemon lost the socket bind (`Socket path already in use`) but had **already rewritten** `relay-<id>.sock.credential`. The surviving daemon kept its in-memory credential, so every later `--connect` got `Endpoint credential mismatch; closing socket`, then `Grace started … timeoutMs=0 … ptys=1, clients=0` every ~20 s, forever. Only a manual `kill -TERM` cleared it. Receipts: `review-archive/orchestration-v3-pr16904/smoke-receipts-t012b/E16,E17,E18,E24`.

Three independent defects kept the wedge alive; each is fixed at its own seam.

## Fix

**1. The relay daemon owns credential publication (race-free under two concurrent starters).**
`relay-daemon.ts` binds the socket first, then publishes via the new `src/relay/relay-endpoint-credential-publication.ts`: adopt a valid pre-existing file (older clients still pre-write), else mint 32 random bytes and write temp+rename at 0600. A start that loses the bind exits inside `listen()` and never reaches the file. Why this option and not restore-on-loss or a client-side write: the only process that can *prove* ownership is the one whose `listen()` succeeded, and that proof is atomic with the bind. The client-side pre-write (`ssh-relay-endpoint-credential.ts`) and the launch-command `chmod 600`/`icacls` are removed on POSIX and Windows. The racing test also exposed that macOS reports a mid-bind collision as `EEXIST` rather than `EADDRINUSE`; `relay-socket-ownership.ts` now treats both as "held or stale".

**2. The client distinguishes "no daemon" from "daemon present but not answering", and never rewrites.**
A credential refusal is now typed on the wire: the daemon replies `orca-relay-handshake-credential-mismatch` (same frame type, no new opcode) and the bridge exits **43**; `waitForSentinel` maps it to `RelayCredentialMismatchError`, which the takeover treats as handshake-refusal evidence exactly like exit 42. A relay that holds the endpoint but **never refused** (the stalled-host shape: kernel backlog accepts the probe, handshake gets no answer) is now `RelayEndpointUnresponsiveError`, routed to the relay-lost backoff instead of the terminal Reset Relay path. Silence is not a decision (`docs/reference/ssh-execution-boundary.md`).

**2b. Deploy honours the verdict.** The 40 s live run exposed that the `--connect` catch block in `deployAndLaunchRelay` predates the incumbent probe and swallowed both verdicts as "probe failed, launch fresh", so a fresh daemon was still launched over the live one (it lost the bind by luck, which is exactly the collision in the incident). Held and Unresponsive now propagate; the session backs off on Unresponsive and surfaces Reset Relay on Held. Red-first in `ssh-relay-deploy-incumbent-verdict.test.ts`.

**3. The daemon cannot be wedged by a rotated file, because nothing can rotate it.**
The credential lives in the content-hashed relay dir, and after (1) the only writer is the daemon that owns the socket, so the "file changed under a live daemon" state the incident depended on is no longer reachable in-product. The credential is therefore fixed for the daemon's lifetime, as a plain secret should be. A hand-edited file is refused with the typed reply until restored (tested). Startup adoption of a pre-written file applies an owner-only + same-uid rule (review finding): anything else is replaced by a fresh mint. An earlier revision of this PR also re-read the file on mismatch and adopted it; that was removed as unreachable machinery that turned the credential into a per-handshake file-ownership check.

**3b. Fail closed between bind and publication.** A client that arrives after `listen()` resolves but before the credential is set is refused, not admitted as `unproved`. Nothing can be delivered in that window today; the guard makes the boundary structural instead of an event-loop ordering fact. Red-first in `relay-reconnect-listener-credential-gate.test.ts`.

**Wire compat.** New optional handshake reply only; an old `--connect` hits `Unknown handshake type` and exits 1 pre-sentinel, which it already treated as a generic failure. New daemon adopts an old client's pre-written file; new client still passes `--credential-file` so an old daemon reads it as before. Absence of exit 43 is never used as evidence.

**Also.** `terminal create` on a reconnecting SSH host now says what to do instead of a bare `No PTY provider for connection "<id>"` (prefix preserved; the renderer matches it).

## Tests (red first)

- `src/relay/subprocess.test.ts`: two `--detached` starts race one socket + credential file → exactly one reaches the sentinel, loser exits 1 with `Socket path already in use`, file valid + 0600, a `--connect` reading it reaches `relay.status` and reports the winner's pid. Red before (both starters died: daemon required a pre-existing file), green 6/6 after.
- `src/relay/relay-endpoint-credential-publication.test.ts`: mints after bind; adopts a pre-written 0600 file; replaces a pre-written 0644 file with a fresh mint; refuses a stale credential with exit 43 while still serving the real one, and keeps refusing a rewritten file until it is restored.
- `src/relay/relay-reconnect-listener-credential-gate.test.ts`: a client in the bind-to-publish window is refused and never attached; after publication the right credential is accepted and a wrong one refused; a daemon launched without a credential file is not gated. Red without the guard.
- `ssh-relay-deploy-incumbent-verdict.test.ts`: live-but-silent incumbent → `RelayEndpointUnresponsiveError`, refused → `RelayEndpointHeldError`, and in neither case is `--detached` launched; a failed `test -S` probe still launches fresh. Red 2/3 without the deploy change.
- `ssh-relay-deploy-helpers.test.ts` (exit 43), `ssh-relay-endpoint-takeover.test.ts` (refused → Held even with no `lsof`; silent → Unresponsive, nothing unlinked or signalled), `ssh-relay-session-terminal-error.test.ts` (Unresponsive → `onRelayLost`, not terminal). Deploy/namespace/native-deps tests updated to assert the client writes **no** credential.

## Live proof

New `tests/e2e/ssh-docker-relay-stall-credential.spec.ts` (claimed in `run-ssh-docker-e2e.mjs` and PR source routing), two cases: `kill -STOP` every relay pid in the container, send input during the freeze, hold **20 s** (the incident's duration, which races the mux liveness timeout) or **40 s** (past it for sure), `kill -CONT`; assert status back to `connected`, same pty, same daemon pid, same credential inode and content, relay.log did not shrink (a relaunch truncates it) and has zero `Endpoint credential mismatch` / `Socket path already in use` lines, in-stall input delivered at most once.

Run output (local, fixture image `orca-e2e-ssh-relay:3a864c665ba2cefd`, `ORCA_E2E_SSH_DOCKER=1 SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 … --project electron-headless --workers=1`, head `c2c20fd994`; re-run identically on the final head after the credential-lifetime change, 2 passed (1.7m), same annotations, and the bind-to-publish refusal never fired):

```
✓ keeps the same daemon and credential across a 20s relay freeze (38.3s)
    relay-processes-stopped: 2          relay-processes-continued: 2
    bridge-pids-before-after: 480 -> 480
    socket-clients-accepted-before-after: 1 -> 1
    in-stall-input-delivered: 1
✓ backs off and reattaches, never relaunching, across a 40s relay freeze (57.5s)
    relay-processes-stopped: 2          relay-processes-continued: 4
    bridge-pids-before-after: 480 -> 1202
    socket-clients-accepted-before-after: 1 -> 3
    in-stall-input-delivered: 1
2 passed (1.6m)
```

Client log in the 40 s case shows the new path end to end: `Relay channel lost … reconnect attempt 1/6` → `Socket probe result: "ALIVE"` → `Socket reconnect failed … Relay failed to start within 10s` → `Relay endpoint incumbent: … verdict=live evidence=accepted-connection holders=unenumerable` → `Failed to re-establish relay … A relay still owns … but did not answer the handshake … Orca will retry` → `reconnect attempt 2/6` → `Reconnected to existing relay via socket`. The 20 s case never left the frozen bridge (same bridge pid, one accept), so it exercises the "silence is not death" side of the same race. The 20 s case passed 6/6 across the session; the 40 s case was red on the prior head (`Socket path already in use` + `Startup failed: listen EADDRINUSE` in relay.log from the swallowed verdict) and is green after 2b. Before the fix the same injection produced a fresh daemon that rewrote the credential and a survivor refusing every client.

The `relay-processes-continued` count exceeds `stopped` in the 40 s case because the timed-out client's `--connect` bridge and the loser-side processes are parked behind the frozen listener when `CONT` runs; they exit on their own once it resumes.

## Gates

`pnpm test src/relay src/main/ssh` 332 files / 3884 tests pass · `pnpm typecheck:tsc:node` clean · `check:code-quality:changed` 0 findings · `check:react-doctor:changed` 0 findings · `pr-e2e-gate-contract.test.mjs` 42 pass · no lint disables or max-lines bumps added.

## Noted, not fixed here

- `terminal list` `orphaned:false` / `terminal close` `ptyKilled:true` for a pane whose relay is gone (`orca-runtime-stop-explicitly-closed-tab-ptys.ts`): different seam, `@ts-nocheck` characterization-covered file.
- On a host with no `lsof`, a stalled relay still cannot be enumerated as the holder; it is now retried rather than declared held, but a relay frozen past the backoff budget still ends in the existing "reconnect manually" banner.
 2026-09-06 14:39:25 -04:00
Jinwoo Hong 06a607a1d7 feat(orchestration): make multi-agent workflows durable (#16904) 
<!-- orca-pr-loc -->
<!-- Programmatic LoC summary. Do not edit by hand; rewritten on every commit. -->

| | Files | Added | Deleted | Net |
| :--- | ---: | ---: | ---: | ---: |
| Test | 225 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​21666 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​2820 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​18846 |
| Prod | 348 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​17107 | $\color{#cf222e}{\Huge{\mathbf{−}}}$​4706 | $\color{#1a7f37}{\Huge{\mathbf{+}}}$​12401 |

<!-- /orca-pr-loc -->

## ELI5

Orca now treats orchestration like a durable control plane instead of inferring success from terminal keystrokes. Agents can tell whether a prompt was accepted or a turn started, replay an ambiguous request without sending twice, and recover coordinator mail after a crash. Completed workers can be inspected, released, or retained, and their panes no longer auto-resume as if the work were still running.

## What changed

- **Run receipts** from `run-create/use/current/show/list` are the row without routing plumbing (`home_database`, `coordinator_pane_key`) and without the duplicate `binding` object.
- **`terminal send` receipts are honest and idempotent.** `input_accepted` and `turn_started` are the only stages; `--wait-submit` observes without resending; `--retry-request <uuid>` replays the exact request against the same process incarnation. A transport timeout keeps the retry ID; only a different runtime answering strips it. Value-less or non-UUID `--retry-request` is rejected on the CLI and the SSH shim.
- **Mailbox delivery is committed before wakeup.** Pointer writes are staged in the DB before any PTY byte, replayed once after restart, and never emit a naked Enter. The watermark that parks concurrent deliveries is released with the DB reservation. Restart rescans pointer-pending and `dispatch:` mailboxes.
- **Lifecycle is a guarded transition graph** (`lifecycle-transition.ts`) with a table-driven test over every caller edge. Task reopen/overturn stays in the public contract. A PTY exit during `worker-stop` is the stop succeeding, not a failure.
- **Worker lifecycle CLI:** `worker-start` (`--spec` creates Task + attempt in one call), `worker-show`, `worker-read` (provider transcript first, bounded terminal fallback with a typed reason, local/WSL/SSH), `worker-stop`, `worker-abandon`, `worker-release`, `worker-retain`, `worker-list` (rowid-fenced pagination, fleet liveness, `attention`, literal `nextAction`).
- **Release is an explicit ownership table** (`decideWorkerTerminalRelease`): only an `owned` resource can be settled, the archive is mandatory where reachable, and an owner whose process is proven exited can always get out of `retained` via `archive_status: unavailable`. User-taken-over, external, and transferred panes stay retained.
- **Settled-worker resume fence** (folds in #17651): a settled dispatch whose pane is still open is fenced at settlement, on stop/abandon/exit, and at startup; lifted on release, retain, takeover, and pane reuse.
- **Liveness is `live` / `unverifiable` / `exited` only**, from execution-host evidence. Fleet projection reads the evidence clock, not the relay delivery clock. A host-certified exit outranks the worker's settled state. `unverifiable` never authorizes stop, abandon, retry, or release, in code or in the guide.
- **Federation:** structured reads negotiate by `method_not_found` so every shipped host keeps transcript-first output; exited remote workers are closed before being reported closed; epoch fencing holds across peer restart, downgrade, and pairing rotation; no per-second forced capability probe.
- **Schema v35:** repairs databases stamped v34 by the pre-fix branch (mailbox_handle default, index predicates), drops the write-only `lifecycle_transition_receipts` ledger and five never-read v31 identity columns.
- **Schema v36:** `dispatch:<id>` mailboxes get a real consumer generation on `dispatch_contexts` and `remote_dispatch_attachments`, bumped and fenced in the same transaction on every re-attach (manual inject, worker-start, federated attach). A stale worker whose Dispatch moved to another process now gets `consumer_fenced` instead of silently acking the new worker's Delivery. Run mailboxes already worked this way.
- **Schema v37:** `dispatch_contexts` records its creator (`creator_handle`, `creator_pane_key`), so a coordinator's context-only self-dispatch is bookkeeping rather than a nesting parent; before this, one self-dispatch made every later `worker-start` from that coordinator fail the depth cap. Pre-v37 rows keep counting (fails closed).
- **Dispatch-mailbox ownership is checked, not inferred.** A `check` from a process whose pane no longer holds the Dispatch, or whose last Attempt was abandoned/failed and moved to another terminal, gets `consumer_fenced` instead of an empty inbox that reads as "no mail yet". `--peek`/`--all` stay readable. A paneless caller still gets `stable_pane_required` with the rebind recovery.
- **Liveness certification is stricter:** a `process_exited` stage whose termination reason is `unknown` (a stop that was issued but never observed) projects `unverifiable`, not `exited`. Federated `worker-show` carries the execution host's verdict and host kind instead of a local guess. A live, ready worker with nothing pending has `nextAction: none` rather than pointing at the `worker-show` that produced it.
- **Wire:** `workerShow` keeps `dispatch.task_id` next to `taskId` for shipped CLIs. `ask --json` uses the standard `{ok, result}` envelope like every sibling verb.
- **Migration start-version detection** treats the two v32 recovery columns as versioned. Before this, every shipped database stamped below 32 resolved to the v6 floor and replayed the whole chain (the v23 backfill synthesized 68 phantom retained workers on a real v30 profile). Verified on a copy of a real 62 MB v30 profile: starts at 30, no row delta, integrity ok, 11 ms.
- **Skill guide** rewritten as a ≤200-line kernel plus seven references, to the outcome-first standard (Result / Done / Safe failure first, conditions not case lists, one done bar, references loaded at the point of use). The canonical loop uses `worker-start --spec`, names `worker-list` for completion accounting, documents `--retry-request` / `request-show` / `--wait-submit`, and requires positive evidence before any stall action. The other seven guides get the same treatment in #18724, split out so this PR stays orchestration-only.
- **`rpc/methods/orchestration-*`** (126 flat files) regrouped into `orchestration/{worker,federation,messaging,runs,gates}/`.

## Why

User reports showed the same boundary failures: false `agent_prompt_stalled` causing duplicate sends (#15180), coordinators unable to trust screen scrapes, cold-parked terminals receiving a pointer without the submit, settled workers accumulating as live tabs and auto-resuming after restart, and no way to tell a stalled worker from a working one.

## Linked issues

Fixes #15180. Fixes #17935 (orchestration skill description is 866 characters; a guard now caps every bundled skill at 1,024). Supersedes #17651 (fence folded in). Advances #16660, #16522, #14907, #13047.

## Review record

This PR was reviewed adversarially after revival: eight independent lenses (lifecycle, mailbox, send, worker, federation, transcript, complexity, live ergonomics), each required to prove findings with a failing test. That produced 16 proven blockers, all fixed with red-then-green regression tests, followed by two re-review rounds and a third fix wave that caught 3 regressions introduced by the fixes and 7 fixes that missed their target; all closed. A final pass (five lenses incl. a live built-runtime smoke, then a re-review of the fix wave) found and fixed seven more, chiefly the stale-worker mailbox steal, the self-dispatch depth wedge, and the unproven-exit certification. Three independent Codex (gpt-6-astra) passes followed: the first found nothing new, the second found and fixed 3 defects (task-status reachability, WSL-local host classification, peer-capability epoch), the third found and fixed 6 (production PTY controller never installed settled writes, ambiguous in-flight pointer failures allowed duplicate replay, SSH/relay deadlines cut off a valid `--wait-submit`, stop-vs-exit race during inspection, and two release-recovery paths for vanished or exited terminals). The full record (findings, proof tests, triage, declines with reasons) is archived outside the repo.

**Rework after the live smoke.** A first live cross-host run on the shipped adhoc build (this Mac, a paired Windows host on the same build, a paired Mac on 1.4.195, and an SSH host) found a P1: a running local worker read `unverifiable`/`missing_status` because the fleet snapshot rows lacked the terminal handle the matcher keyed on. A 59-row failure table over every bug fixed during review showed the same two classes recurring: a fact dropped in transit through optional fields, and two authorities for one fact. Two blind designs (Opus, Codex) converged on the same mechanisms, and the scoped tranches landed here with red-then-green seam tests from the real producer to the real consumer, faults injected only at the transport or hook-ingest boundary:

- **Settlement (data-loss class):** one three-valued `WriteSettlement` (`accepted | refused{reason} | unverifiable{reason, bytesHandedToTransport}`) from the SSH multiplexer through daemon client, providers, controller, to pointer staging. No boolean, no rejection-as-third-state. The two silent degrades that fabricated a handoff are deleted; a provider that cannot settle refuses before any effect. Pointer text and Enter share the contract; a partial flush is `unverifiable`, never `refused`.
- **Evidence identity (false-liveness class):** fleet agent-status evidence is a tagged union (`binding: worker | pane | unresolved{reason}`, `clock: observed | delivery`) minted once at ingest, so a hook row captured on one process incarnation can never bind to a later dispatch on the same pane. The matcher's `!worker.paneKey ||` defaults are gone. One host-scope parser replaces two.
- **Small pre-merge items:** `capability_unsupported` from an old peer is no longer relabelled `host_unavailable`; a producer census test asserts every agent-status consumer path projects a pane-only hook row as `live`.

Two ergonomics defects the second live run surfaced on a real database are fixed here too: a pre-v3 dispatch already marked `completed` projected as `outcome_unknown` / `requiresAction: true` forever (three copies of the outcome ladder disagreed on legacy rows; now one resolver, legacy `completed` reads `succeeded` with nothing to act on, legacy `failed` stays actionable on the failure), and an unscoped `worker-list` enumerated the entire database (now defaults to the Run bound to the calling terminal, `--run` overrides, and the receipt's additive `scope` field says which).

A third live round on the shipped adhoc build of `b082443e1f` (same four hosts) plus an unscripted run in the user's own prompt style (a plain Claude Code shell, `/orchestration`, three workers, zero errors, bound-Run default confirmed) found two more branch defects, fixed with red-then-green tests: a worker freshly started on a paired server projected `unverifiable`/`host_indeterminate` with `requiresAction` for ~3 minutes, including after its own `worker_done`, because the host's federation observation returned `missing_liveness_verdict` for any PTY the liveness register had not yet swept (the host now reads a connected pane it owns locally as `live`; disconnected or SSH-scoped panes stay `unverifiable`); and six pre-v3 completed rows still carried an `input` category because settling through the task-status path or `failDispatch` never closed the Dispatch's pending question threads (both paths close them now, and schema v38 closes threads already pending on settled rows). The guide's `worker-start` examples now show `--model sonnet`, since an omitted model inherits the launcher's default.

A Codex adversarial pass on the tranche diff found one real design hole (identity minted at read time instead of ingest, now closed) and two daemon settlement paths that threw instead of settling (fixed). Two `@ts-nocheck` runtime mixins on these paths were extracted into checked modules; the repo-wide `@ts-nocheck` count is unchanged at 171.

Deletions during review: ~1,900 lines (write-only ledger, unread columns, dead v1 archive path, test harnesses shipped in prod, duplicated liveness and state-machine copies, self-capability checks that were compile-time true).

## Testing

- `pnpm typecheck:tsc:node|cli|web` clean
- `pnpm run check:code-quality:changed` 0 findings; `check:react-doctor:changed` 0
- `pnpm verify:bundled-skill-guides`, `verify:skill-bundle-manifest`
- full `pnpm test` on the integrated head: 72,332 pass / 292 skipped; the only failures were three non-PR files (two zsh live-shell suites hit a node-pty spawn-helper ENOENT while a concurrent native rebuild ran, 44/44 in isolation; `release-checkout.unit.test.ts` is a known 30 s load timeout that passes in isolation on `origin/main` too).
- CI on 70b4811267 (rerun, pre-Codex): the only reds are five SSH e2e specs plus `terminal-send-agent-prompt-submit:198`, each shown failing identically on main (main's E2E workflow is red on its last 40 runs). The terminal-send spec is root-caused and fixed separately in #18707. The Windows hook-service flake (#17721) and the federation load flake did not recur.
- Skills: `pnpm exec vitest run` over the skill gate files plus `src/cli`, `config/scripts`, `src/main/skills` pass; live smoke on the built CLI of `skills get orchestration` and `--full` (7 references).
- live headless runtime (`orca-dev serve`, isolated profile): canonical loop, stop, release, archive read, retry rejection, stale-handle check, SIGKILL-and-replay all verified with receipts
- Live cross-host smoke on the shipped adhoc build of `0d465e7931` (this Mac and a paired Windows host on the build, a paired Mac left on 1.4.195, an SSH host): local, paired-new, paired-old and SSH loops all settle; running workers read `live` on every host and `exited` after release; the old peer reads `capability_unsupported` and refuses release honestly. Injected 10 s relay stall with a send in flight: delivered exactly once after recovery, zero duplicates. Every liveness field across 104 receipts is only `live` / `unverifiable` / `exited`.
- Final live cross-host smoke on the shipped adhoc build of `b082443e1f` (same hosts): every loop settles; 942 of 948 legacy completed rows read settled with `requiresAction: false` before the question-thread fix and all of them after; `worker-list` scope reads `bound` / `flag` / `all` correctly; 122 JSON receipts carry only `live` / `unverifiable` / `exited`. Unscripted prompt-style run: clean.
- Confirmation smoke on the shipped adhoc build of `2da076d4e9` (this Mac and the paired Windows host, both updated): a freshly started Windows worker reads `live` on the first fleet poll and on all 20 that follow, with no `host_indeterminate` at any point, and `exited` after release; all 948 legacy completed rows read `requiresAction: false` with `nextAction: none` after schema v38; every verdict across 60 receipts is `live` / `unverifiable` / `exited`.
- Not physically exercised: WSL hosts, the renderer notification bell (headless has no renderer), same-session fence via a real pane close (renderer-only state), restart mid-delivery on a real app (covered by e2e only).

## Notes

- Remote-wire additions are optional fields or `method_not_found`-negotiated methods; one new Electron-only IPC channel (`agentStatus:legacyWorkerTerminalResumeFence`) never crosses the wire.
- SSH contact loss remains `unverifiable`; the execution host stays authoritative.
- Intentional wire projection change: an SSH host scope with an empty `targetId` now projects host id `ssh` instead of an empty string (remote-wire-compatibility rule 3, old clients decode the same field). A fleet pane key without a terminal handle is now `unidentifiable` rather than matched by pane key alone.
- Found live but pre-existing on main, filed separately: a relay daemon-start collision during transport loss rewrites the endpoint credential and wedges the surviving relay (host needs a manual kill); `terminal create` on a reconnecting SSH host reports an opaque `No PTY provider for connection`; `terminal list` reports `orphaned:false` and `terminal close` reports `ptyKilled:true` for a pane whose relay is gone (orchestration's own projection reads `unverifiable` correctly at the same moment).
- Downgrade after this PR is not a supported path: main opens a v37 database and early-returns (its inserts still work against the v36/v37 defaulted columns), but its one-outstanding-Delivery-per-Run index is a no-op against the branch's mailbox-scoped index of the same name.
- Known follow-ups (not blockers): `worker-list` materializes every dispatch row per call; a positive "agent absent" signal distinct from PTY liveness is a product decision left open (a headless fake agent never reaches `live`, so its `nextAction` stays `inspect`); a context-only self-dispatch still lists as `role: worker` in `worker-list`; `dispatch` task-not-found / task-not-ready / inject-rejected still surface as `runtime_error`; task and inbox receipts still expose raw row columns. Deferred skill product decisions live on #18724.
 2026-09-06 14:34:03 -04:00
Neil 3be526c5e6 test: cover SSH reattach replay and enable deterministic Codex CI (#19106) 
* test: cover SSH replay replies and run deterministic Codex restore scenarios

* test: register replay probe unit command in reliability gate
 2026-09-06 11:25:58 -07:00
Neil 4d9e963ffd test: enable localhost SSH terminal and hook journey in CI (#19097) 
* test: run localhost SSH terminal and hooks in CI

* test: isolate localhost SSH session fixtures across repetitions

* test: route remote agent hook source changes to localhost journey

* test: record localhost SSH reliability evidence and remaining gaps

* test: route the real SSH session hook authority
 2026-09-06 10:05:26 -07:00
Neil f5960cec00 test: enable Docker SSH browser network route coverage in CI (#19095) 
* test: enable Docker SSH browser network route journeys in CI

* test: register Docker browser job in token permissions contract

* test: declare SSH client dependency and narrow browser fixture routing
 2026-09-06 09:29:56 -07:00
Neil 3631f886a7 test: enable direct and client-hosted SSH browser coverage (#19090) 
* test: enable direct and client-hosted SSH browser coverage

* test: record twelve passing SSH browser journey repetitions

* test: distinguish SSH journey evidence from unit runtime budget
 2026-09-06 08:25:13 -07:00
Neil 1d2e00819f test: restore SSH bulk-open freeze coverage in headed CI (#19081) 
* test: restore SSH bulk-open freeze coverage in headed CI

* test: record ten passing headed SSH freeze repetitions

* test: record ten passing headed SSH freeze repetitions

* test: route changed SSH freeze spec only to its dedicated lane
 2026-09-06 07:42:50 -07:00
Neil f952f1ac96 test: run real WSL terminal launch and paste in PR CI (#19072) 
* test: continuously exercise real WSL terminal launch and paste

* test: establish live WSL reader before changing default shell

* ci: pin WSL kernel installer and participation selectors

* ci: route deleted WSL paths and record immutable run evidence

* test: require exactly three WSL repetitions in lane contract
 2026-09-06 05:24:54 -07:00
Brennan BensonandMerge Sim 6933fd70d7 fix(packaging): ship Claude agent SDK with desktop builds (#19042) 
* fix(packaging): include Claude agent SDK at runtime

* test(packaging): cover spaced runtime imports

* fix(packaging): verify every emitted main file for bare runtime imports

The packaged-main verifier read two fixed entry files, but rolldown hoists
modules shared by two entries into out/main/chunks. jsonc-parser is reached
only from a chunk today, so nothing verified it, and the agent-hooks entry
the list names contributes no coverage at all. An import that migrates into
a chunk would silently stop being checked -- the same blindness that let the
missing Claude agent SDK ship.

Scan every out/main/**/*.js entry in the asar instead, keeping the two
required-file assertions as a build-integrity check. Measured against the
shipped 1.4.198 app: 93 entries in 72ms, reporting the absent SDK and
nothing else.

Also tighten the specifier match with a (?<![.\w]) lookbehind. Orca has
three registry methods of its own named require(), two taking a string key,
so a minified registry.require('public-a') otherwise reads as a bare module
specifier and fails packaging with a confusing error -- a risk the wider
file set would have multiplied. The lookbehind drops nothing real: detection
over the shipped bundle is identical with and without it.

* test(packaging): cover the missing packaged main entry assertion

The required-file check had no test, so the refactor that split it out of
the scanning loop could have dropped it silently. Removing the assertion
now fails this case.

* docs(packaging): name the embedded-source-string limit of the main scan

ssh-relay-deploy builds a probe script for the REMOTE host as a string, and
its require("node-pty") / require("@parcel/watcher") survive into
out/main/index.js, where this scan counts them as desktop-main imports. Both
are packaged, so it is benign today, but a remote-only dependency added to
that script would fail desktop packaging with a false message -- and the two
obvious fixes (ship the remote dep, or weaken the guard) are both wrong.
Separating an embedded string from real code needs a parser.

* test(packaging): pin the exact import shape oxc emits for the SDK

The fixture only carried the spaced `import (` variant, so nothing pinned
the form a shipped build actually contains. Use the real emitted shape --
`p??=import(`@anthropic-ai/claude-agent-sdk`)`, no space, backticks, and the
`??=` that precedes it -- and keep the spaced variant on the second entry so
both stay covered.

* fix(packaging): keep the main scan able to see a spread require

The `(?<![.\w])` lookbehind also rejected `[...require("pkg")]`, because the
third dot of a spread satisfies it. That trade is not symmetric: excluding a
member call costs a loud release-build failure if it ever misfires, but
excluding a real specifier is this guard going blind -- the failure mode the
whole verifier exists to prevent. Readmit a dot that ends a spread.

Zero occurrences in the shipped bundle today, so this was latent. The chunk
test's asar mock now also emits directory nodes, because real listPackage does
and extractFile throws on them -- that makes the `.js` anchor's load-bearing
role something the tests can actually catch.

---------

Co-authored-by: Merge Sim <sim@local>
 2026-09-06 02:27:57 -07:00
OrcaWinOrca WorkerMerge Sim
 		6494f2a4f0 fix(native-chat): resume a structured chat from Agent Session History (#18933) 
* fix(native-chat): resume a structured chat from Agent Session History

Clicking Resume on a chat-UI row could only reveal an already-open tab. If the
chat had been closed, or this process had never published it, the click re-read
an inventory that did not contain it and toasted "Retry in a moment" — advice
that could never come true, because nothing republishes an unpublished tab. The
legacy `claude --resume` fallback is deliberately refused for structured-owned
rows, so the row had no way back at all.

`close` already keeps the record and the journal on disk so a session can be
attached again, and the hold path already resurrects one in full. What was
missing was the tab: `restoreReadableSessions` is latched to run once, at
startup, so nothing could ask for a single session later.

Adds `agentSession.reveal`. The host looks up its own record, restores the
session readable, and republishes the tab through the same call
`agentSession.create` uses. Deliberately narrow:

- It takes no hold. A provider child exists because a surface asked, and the
  chat pane asks when it binds.
- A journal it cannot read is not a refusal. A chat whose journal predates the
  SQLite store restores to nothing here, but attach still recovers it, so the
  tab is published and the pane's hold finishes the job.
- Workspace and provider come from the record, never the client, so a session
  id alone cannot aim the publication at another workspace.

Claude and Codex both, by construction: eligibility is `adapterSupportsRecord`,
which the router answers from the record's own provider.

Gated on a new advertised capability rather than probing for method_not_found,
matching agent-session.structured.hold.v1 — absence is visible during
negotiation instead of by calling.

* fix(native-chat): negotiate reveal against the host that owns the workspace

The capability gate read the LOCAL runtime's advertised capabilities while the
call went to the host that owns the workspace, which for a paired workspace is
a different build. On desktop the renderer and its local host are always the
same build, so the gate passed unconditionally and proved nothing about the
host being called: an older paired host still received the unknown method and
its method_not_found was reported to the user as 'this chat is no longer on
this host'. The cache it read also starts empty and resets to empty when
status.get fails, so 'not fetched yet' and 'unsupported' were the same value.

Gate on the environment that will answer, the way agentSession.close already
does, and skip the round trip entirely for a local host. Reveal now reports
four outcomes instead of a boolean, so a host that is merely too old is not
reported as a chat that is gone, and a host we could not reach keeps the
retryable message.

Also syncs the localization catalog: the 'gone' key shipped without an en.json
entry, which reddens static analysis and verify while typecheck stays green.

* fix(native-chat): tell a refused reveal apart from a missing chat

The host raises two refusals here and they mean opposite things to a user: it
holds no such record, or it holds one no adapter of its own can open. The
client collapsed both into 'this chat is no longer on this host', which is a
eulogy for a chat still sitting on disk. Read the refusal code, and fold the
host-side case in with the too-old host under one honest message, since the
remedy for both is the same.

Adds the coverage the readiness pass found missing: the host's reveal answer
itself (workspace and provider from the record, both refusals, an unreadable
journal, a live session), and the activation branches for a host that cannot
open the chat and for one that never answered.

* fix(native-chat): read a host version block as the host's age, not a lost link

The capability probe reaches assertRuntimeStatusCompatible, which throws a
runtime_compat_block error. Treating that as unreachable told a user with an
out-of-date host to retry, which is the one thing that cannot help. Branch on
isRuntimeCompatBlockError the way remote-agent-session-launch already does for
the same probe.

Also adds the refusal-code case a previous commit claimed and did not deliver:
nothing drove a structured_agent_session_unsupported reply through the reveal
client, which is the branch that commit existed to add. Corrects a doc comment
that reveal made wrong: attach is no longer the only call that builds the host.

* fix(native-chat): let a dragged history row reach the same reveal as a click

Dropping an Agent Session History row onto a pane activated the tab by id and,
on a miss, raised the very toast this PR exists to remove — so the same row
answered a click and a drop differently, and the drop kept the advice that can
never come true. The structured branch never used the drop pane, so routing it
through the shared activation loses nothing and gains the reveal.

The helper only ever read one field, so its parameter narrows to that field and
the drag payload satisfies it directly. A source ratchet holds both entry points
to the reveal-capable path, since a mounted drag harness does not exist for this
layer and what regresses is a call site, not a rendering.

* fix(native-chat): stop an advisory refresh ending the click, and one click per row

Manual QA found the reveal never ran: the inventory refresh that precedes it
is an optimization, but its failure returned early with 'not available yet,
retry in a moment' — reinstating the dead end this PR removes, one step
earlier. A failed refresh now falls through to the reveal, which is the repair
and does not need the refresh to have worked.

The click can chain a refresh, a capability probe, a reveal and a second
refresh, each with its own timeout, while nothing on the row says it is
working. A per-session in-flight guard keeps an impatient second click from
running the whole sequence again and landing its own toast.

Also drops an unreachable owner scope: the snapshot apply discards any
worktree whose execution host is not local before it reads one, so naming a
remote scope there described a synchronisation that cannot happen.

* fix(native-chat): bound the capability probe and stop naming the wrong machine

The in-flight guard releases when the activation settles, so an await that
never settles holds the row for the life of the process. The capability probe
was the one call in the chain not raced against a deadline: on a cache hit it
awaits a promise an earlier probe created, which may carry no deadline of its
own. Race it like the two calls around it.

A version block can name either side — evaluateRuntimeCompat reports
client-too-old as well as host-too-old — so a message that blamed the host
pointed half of those at the wrong machine. Name the remedy instead of the
machine, which is true for every case that reaches it.

* chore: remove a scratch repro file committed by mistake

It was swept into the previous commit by a broad `git add` while a diagnostic
ran in this worktree. It asserts the current renderer-sync defect as expected
behaviour, so it would fail the moment that defect is fixed.

* fix(native-chat): stop a reveal's own inventory refresh discarding its republished tab

Manual QA: the host answered reveal with ok:true and republished the tab, and
the chat still did not reopen — only a renderer reload brought it back.

The renderer publishes under one epoch string for its whole lifetime, and a
frame recorded under a different lineage retires that epoch permanently with
nothing to un-retire it. The Resume click asks for an inventory first, and a
worktree the host holds no entry for answers with the none/v0 sentinel; the
structured path recorded it, retiring the renderer's own epoch, so the tab the
reveal published a moment later was dropped. A reload minted a new epoch,
which is why reloading appeared to fix it.

A frame that carries no publication is not a later publication to fence
against. Treat the sentinel and a removal frame as a cursor reset, the way the
mainstream session-tabs path already clears its tracking — its comment names
this exact hazard: recording that sentinel would retire the host epoch and
reject the next live frame.

Pre-existing, and it swallows an ordinary new-tab launch on an empty worktree
too; the reveal is what turned a silent invisibility into a visible failure.

* fix(native-chat): let a retraction prune its rows without retiring the epoch

Correcting the previous commit. Skipping a retraction frame outright stopped it
pruning the mirrored rows, so a worktree the host no longer publishes would
have kept a chat on screen with nothing behind it. Apply the frame as before
and clear its cursors instead of recording them, which is what the mainstream
session-tabs path does.

The unpublished sentinel keeps its cursor now too: it is skipped rather than
cleared, so a stale frame arriving late is still fenced. Adds the case the
earlier version would have broken.

* fix(native-chat): keep the retraction's fences, and fence the reveal's refresh

Correcting the retraction handling again. Clearing its cursors was more than the
bug needed and cost a guard: the host mints a fresh epoch when it rebuilds a
pruned entry, so a republication is never gated by the retained cursor, while
dropping it left an inventory response issued before the close free to land
afterwards and strand a chat row for a worktree the host no longer publishes.
Skip only the recording. The mainstream path keeps its epoch history for the
same reason, as a tombstone fence.

The test that justified the stronger clearing asserted a host behaviour that
does not exist — a rebuilt entry republishing under the renderer's epoch with a
restarted counter. It now uses what publishStructuredAgentSessionTab actually
mints for a pruned entry, and a new case covers the frame that would strand.

Also fences the reveal's inventory refresh on the sync generation, which every
other caller that applies an inventory already does: structured chat can be
switched off mid-flight, and the answer would otherwise re-seed a row into a
renderer that just discarded them.

* fix(native-chat): drop the retraction's epoch history, keep its version cursor

Third and final shape for this branch, and the only one of the three that holds.

Keeping both maps re-poisons the epoch one cycle later: the consumer here is
also the publisher, so the history's current is the renderer's own lifetime
epoch, and recording the reveal's fresh epoch retires it. The next chat the
renderer publishes is then dropped — this bug again, one close later. Deleting
both loses the guard that stops a frame issued before the close landing after
it and stranding a row nothing republishes.

So: clear the history, keep the cursor. The mainstream path keeps its history
as a tombstone because there the epochs belong to a remote publisher; that
reasoning does not carry to a path that publishes under its own.

Each of the three variants now fails a different test.

* fix(native-chat): a retraction forgets what is current, not the tombstones

The delete lost a fence the cursor cannot replace: the version cursor only
compares within a lineage, so a delayed frame from an already-superseded epoch
had nothing left to stop it putting a chat row back for a worktree the host no
longer publishes. Keeping the record intact had the opposite fault — the
renderer's own epoch is the history's current, so the next frame under any
other epoch retired it.

Clearing only current does neither: noteRetiredValue retires nothing when there
is nothing current, and the tombstones stay. Each of the four shapes now fails
a different test.

* fix(native-chat): narrow the retraction frame through its own type

Typecheck caught what the tests could not: `removed` is not on
RuntimeMobileSessionTabsResult. The repo already names the shape —
RuntimeMobileSessionTabsRemovedResult — so this reads it through a guard rather
than the inline cast the mainstream path uses.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
Co-authored-by: Merge Sim <sim@local>
 2026-09-06 00:50:20 -07:00
OrcaWinandOrca Worker 0f27445789 fix(build): pin config/relay-assets LF so one release is one relay hash (#19024) 
* fix(build): pin config/relay-assets LF so one release is one relay hash

* test(build): correct why the negative fixtures exist

Review measured it: the first assertion checks the eol attribute via
check-attr, not file content, so it fails first without the pin. The
fixtures add over-broadness coverage, they do not carry the test.

* test(build): key the relay line-ending pin off the manifest, not a directory

A path glob proves the directory is non-empty, not that it is still the
directory build-relay reads from. Relocating an asset into config/scripts
(where only **/*.mjs is pinned) reintroduced the CRLF bug with the suite
fully green. RELAY_ARTIFACTS is the right anchor: build-relay refuses to
emit an artifact absent from it, so a relocated or new asset cannot slip
past. Bundles have no tracked source and drop out with zero hits.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
 2026-09-06 00:02:05 -07:00
Jinjing 891ae62df5 fix(release): revalidate draft state before patching generated notes (#19019) 
* fix(release): revalidate draft state before patching generated notes

The release listing is a snapshot taken before generate-notes runs. If the
draft is published in that window, the PATCH overwrote a live release body.
Re-read the release by id immediately before the update and skip it when the
release is no longer a draft.

* Handle publication race during draft release notes patch

Between the draft status check and the PATCH request, a release can be
published. The PATCH succeeds but now modifies published content. Check
the PATCH response—if draft=false, publication won; restore the
published body and leave generated notes unapplied.

* fix(release): only roll back the draft body we actually wrote

Re-read the release before the compensating PATCH and skip the rollback when the body no longer matches the notes we patched in, so a body written after our PATCH is not clobbered.
 2026-09-05 22:34:56 -07:00
Neil bf5f3c2ec4 test: cover native X11 Hangul-plus-digit PTY bytes in CI (#19013) 
* test: run the native Hangul terminating-digit regression in CI

* test: distinguish X11 byte coverage from the manual Wayland repro

* test: require native IME engagement proof for the digit case
 2026-09-05 22:12:21 -07:00
Jinjing bdad20b4c1 Support updating existing draft releases when regenerating notes (#19014) 
Move release existence check into create-draft-release.mjs. Draft releases
are updated via PATCH, published releases are skipped, making the
release-cut workflow idempotent.
 2026-09-05 21:52:54 -07:00
OrcaWinandOrca Worker ec030f1d35 fix(windows): sign the NSIS uninstaller via SignPath (#17868) 
* fix(windows): sign the NSIS uninstaller via SignPath

`Uninstall Orca.exe` ships NotSigned, and MDE's whole update cluster is
that one file: electron-builder copies it to `old-uninstaller.exe` and
runs it silently during every update.

The cause is narrower than "NSIS generates the uninstaller at install
time". app-builder-lib already builds the uninstaller in its own makensis
pass and calls `packager.signIf(uninstallerPath)` on it before embedding
it (NsisTarget.computeScriptAndSignUninstaller). Orca signs nothing during
electron-builder — SignPath signs afterwards, behind a human approval — so
that hook is a no-op and the file is deleted before CI can reach it.

Use the hook as a relay instead of a signer: the first Windows build
exports the uninstaller, it rides the existing inner-binaries SignPath
request (no third approval wait), and the rebuild-from-signed-tree pass
swaps the signed bytes back in before makensis embeds them.

Every added step is fail-open. A missing export, a SignPath artifact
configuration that does not cover `uninstaller/`, or a relay error costs
only the uninstaller signature — the inner-binary chain and the shipped
installer are unchanged.

* fix(windows): keep the uninstaller relay out of the packed checkout

Review fixes on the uninstaller signing chain.

The export path lived at `${{ github.workspace }}\uninstaller-signing\`.
`files` in the electron-builder config is all-negation, so app-builder
prepends `**/*` and packs whatever is left in the checkout root, and the
build step retries up to three times — attempt 1 wrote the file after
packing, attempts 2 and 3 would have packed an unsigned `.exe` into
app.asar. All seven relay sites move to `runner.temp`, and a contract test
now fails if any of them points back into the checkout.

The uninstaller staging block guarded with `Test-Path` but left `New-Item`
and `Copy-Item` able to throw. That step's outcome gates the upload of
every inner binary, so a locked file there would have cost all of them
their signatures — worse than before the chain existed. It is wrapped in
try/catch, asserted.

Also: test `signWindowsUninstallerViaSignPath` itself (it runs in a step
with no continue-on-error, so its no-throw property is load-bearing) and
the sha1+sha256 double invocation; make the rehearsal verify the
uninstaller the installer actually writes to disk rather than only the
relay receipt, whose digest comparison is equal by construction; correct
the staged-name comment, which asserted a collision that does not
reproduce; count what was reported rather than what was extracted; and
note two traps — a custom sign hook replaces signtool outright, and the
single-env-var relay would race if a second NSIS target or arch is added.

* fix(windows): stop the signing rehearsal failing on its own artefact

The rehearsal is the merge gate for this chain, so it must not be able to
fail on something that is not the thing under test.

It trusted whatever 7-Zip's NSIS handler emitted. That handler produces
partial or garbled output on some NSIS builds, and a truncated extract
would score NotSigned and be reported as "the shipped uninstaller is
unsigned" when nothing was wrong. It now has to reproduce the digest the
sign hook recorded before its output is trusted; otherwise it falls
through to the silent-install route, which is ground truth. A name miss
falls through the same way.

The install route only checked the signature. Comparing the on-disk file
against the receipt is what actually proves the shipped installer embedded
the SignPath-signed bytes — the release job's own comparison is equal by
construction, so this is the only place the claim is really tested.

Also: bound the silent install (a bare `-Wait` on an installer that ever
prompts hangs to the 360-minute job cap) and poll before stopping Orca,
since the oneClick installer launches the app as it finishes and the
process can appear after the installer has already exited.

Two smaller ones: `-ErrorAction Stop` on the staging New-Item/Copy-Item so
the catch above them does not depend on GitHub's $ErrorActionPreference
default; and the relay-path test now counts every occurrence rather than
the first, so a step carrying two paths cannot root one in RUNNER_TEMP and
leave the other bare-relative — the exact shape of the bug it guards.

* test(windows): stop a pre-existing elevate.exe defect masking the gate

The first real rehearsal (run 33484703381) proved the uninstaller relay
works end to end — the 7-Zip route read the embedded uninstaller, the
digest guard did not trip, SignPath accepted the new uninstaller/ zip
entry, and the shipped `Uninstall Orca.exe` came back signed.

It also failed, on `resources\elevate.exe`, for a reason that predates
this PR. app-builder-lib re-copies the pristine cached elevate.exe over
`resources\elevate.exe` on every nsis pack — `AppPackageHelper.packArch`
calls `elevateHelper.copy()` before `buildAppPackage`, and
`CopyElevateHelper.copy` does `copyFile(elevatePath, outFile, false)` then
`signIf(outFile)`, which signs nothing because this build configures no
certificate. The signed copy restored into win-unpacked is clobbered by
the rebuild.

That is not the sign hook displacing a signtool call: with no `sign` hook,
`signFile` already returned false at "no signing info identified", so
nothing was signing elevate.exe before either. release-cut.yml mitigates
it separately by pre-seeding the electron-builder cache; this workflow has
no such step, which is why the clobber is visible here and not there.

Downgrade elevate.exe alone to advisory so it cannot mask the uninstaller
result, and record it in the evidence artifact so downgrading stays
distinguishable from deleting the check. Both uninstaller verdicts stay
fatal, pinned by a contract test that also holds the escape hatch to
exactly one file. The underlying defect gets its own PR — it is a UAC
elevation helper and deserves more scrutiny than a footnote here.

* docs(windows): warn against relaxing the elevate.exe cache guard

The tempting edit, for anyone who finds the rehearsal red on
resources\elevate.exe, is to relax release-cut's `Valid` +
`CN=SignPath Foundation` guard so the cache swap runs under test-signing
and the rehearsal goes green.

That guard is the only thing stopping a test certificate from being seeded
into a cache a real release restores from — both workflows share the key
`electron-builder-win-<lockfile hash>`. Shipping users a binary signed by
"Test certificate for 'Orca agent ide [OSS]'" is worse than shipping it
unsigned, so say so at the place someone would make that edit.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
 2026-09-05 21:44:28 -07:00
OrcaWinandOrca Worker 8415d53a05 fix(release): stop shipping an unsigned elevate.exe on Windows (#18044) 
* fix(release): stop shipping an unsigned elevate.exe on Windows

The release cut swaps the SignPath-signed elevate.exe into the
electron-builder toolset cache so the NSIS rebuild's CopyElevateHelper
re-copy becomes a no-op. It searched `<cache>\nsis`, a directory no
app-builder-lib layout creates, and `-ErrorAction SilentlyContinue`
plus `exit 0` turned that miss into a green step — v1.4.193 and
v1.4.194 shipped an unsigned UAC elevation helper.

Move the lookup into a script that covers the real layouts
(`nsis-3.0.4.1/…`, `nsis@<toolset>/…`, `ELECTRON_BUILDER_NSIS_DIR`),
asks app-builder-lib for the authoritative path, and exits non-zero
with an ::error:: annotation when it finds nothing. The step stays
continue-on-error so the inner-signing chain remains fail-open.

* fix(release): make the elevate.exe swap prove it replaced the packed copy

Success was "some cached copy was replaced", which a stale release
directory carried in by the `electron-builder-win-` prefix restore can
satisfy on its own while the bundle the rebuild packs stays unsigned.
The app-builder-lib probe returns the exact path CopyElevateHelper will
pack, so make that the check and the directory scan the fallback: exit
non-zero when the probed copy was not replaced, and annotate a warning
when the probe could not run at all, so a green step never quietly means
the authoritative check was skipped.

Also pin both shebang scripts to LF: `core.autocrlf=true` gives a
Windows checkout CRLF, and CRLF plus a shebang breaks vite's transform,
so resolve-7za-path.test.mjs currently runs zero tests there.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
 2026-09-05 21:44:20 -07:00
OrcaWinandOrca Worker c252d855ac fix(windows): resolve npm/pnpm .cmd shims past cmd.exe (#17869) 
* fix(windows): resolve npm/pnpm .cmd shims past cmd.exe

A `.cmd` target forces every spawn through `cmd.exe /c` with each argument
caret-escaped, and Microsoft Defender for Endpoint scores a long `cmd.exe /c`
line carrying caret-escaped natural language as obfuscation. `codex.cmd` is
named in the spawn cluster of the MDE incident this addresses.

npm's `cmd-shim` and pnpm's `@zkochan/cmd-shim` generate files whose whole body
is "find node, run this script". Read one, and the spawn can go straight to
`node.exe <script> <args>` — no cmd.exe, no caret escaping. Anything the parser
does not recognise exactly, or whose target cannot be confirmed on disk, keeps
the existing cmd.exe path.

Incidentally fixes a real bug: cmd ends its command at a raw CR/LF whatever the
quote state, so a multi-line agent prompt through a `.cmd` shim had to be
rejected. Resolved shims have no such limit.

* fix(windows): refuse drive-relative shim paths and run the win32 tests in CI

Two blocking findings from review.

A drive-relative path defeated the absolute-path guard:
`win32.isAbsolute('D:evil.js')` is false, but `win32.resolve` reads the drive
letter and lands on `D:\evil.js`, outside the shim directory. cmd would have
built `C:\shim\D:evil.js` and failed; we would have executed the wrong file.
Adding `:` to the unsafe-character set closes it, and the alternate-data-stream
spelling `a.js:zone` with it. It costs no coverage: 84 of the 91 real shims on
this box still resolve, the same seven fall back.

Neither `windows-cmd-shim-resolution.test.ts` nor its `.win32` sibling was in
the Windows package job's file list, so the whole filesystem/resolution half and
the real-spawn equivalence suite ran nowhere. Both are now in
`WINDOWS_PACKAGE_TESTS` and in the pr.yml step.

Also from review: clear `windowsVerbatimArguments` explicitly on the resolved
branch rather than inheriting it, since there is no caller-built command line
there; document the kill switch and the PTY/hook-wrapper scope limits in
docs/reference; and cover drive-relative, BOM, line-ending, casing and `%*`
tampering in the platform-independent half of the tests.

* docs(windows): justify the shim-path colon guard from the filesystem rule

The guard was argued empirically ("none of the 91 shims on this box has one"),
which invites a future reader to relax it for a shim we have not seen. Windows
reserves `:` within a path segment, so a relative path cannot carry one at all:
the only spellings that can are drive-qualified, an alternate data stream, or a
`\?\` device path, and the last is already refused as absolute. That makes a
false refusal impossible rather than unobserved.

* refactor(child-process): move resolveSpawn into its own module

The merge with main pushed run-process.ts one line past the 300-line cap:
both sides grew it. The spawn-argv decision is already a pure, separately
tested unit, so it moves out rather than the cap moving up. run-process.ts
re-exports it, so no caller changes.

* perf(child-process): cache the shim interpreter lookup

The parse cache spared the shim read but not the PATH walk, so a second
resolution of the same .cmd did 0 reads and one statSync per PATH entry --
30 on a 30-entry PATH, synchronous on resolveSpawn, where one dead network
mount blocks the calling thread on every spawn.

Keyed by shim directory AND PATH, since the shim's own rule is
%~dp0\node.exe first then PATH, and a PATH edit between spawns must miss.
Corrects the stat comment, which accounted only for the shim itself.

* fix(child-process): revalidate a cached shim interpreter before using it

The node cache was held for process life and never rechecked, so a cached
node.exe that was later uninstalled -- or dropped from PATH by a version
manager -- was still handed to resolveSpawn, failing the spawn with ENOENT.
An uncached process in the same state returns null and falls back to
cmd.exe successfully, so the cache was strictly worse than no cache.

One statSync on a non-null hit, not one per PATH entry, so the walk this
cache exists to skip is still skipped. The stale-null direction stays
uncorrected on purpose: it only keeps the working cmd.exe fallback. Both
directions are now stated in the comment, along with the known miss for
callers that vary PATH per spawn.

* fix(child-process): honour PATHEXT when resolving the shim interpreter

The doc claimed a node.com/.bat/.cmd on PATH returned null and fell back to
cmd.exe. The scan actually skipped those entries and kept looking for a
node.exe, so PATH=C:\A;C:\B with C:\A\node.com and C:\B\node.exe resolved to
B's node.exe while the shim runs A's node.com -- a different binary, chosen
silently, on the one axis this module must not get wrong.

The scan now follows cmd's rule: first PATH directory holding any PATHEXT
spelling wins, PATHEXT order decides within it, and only an .exe winner is
returned. Anything else gives up and keeps the cmd.exe path, which restores
the strict-subset-of-cmd property everywhere except the documented cwd case.

PATHEXT is read from the child's env and joined into the cache key, since it
now changes the answer. Costs one stat per PATHEXT entry per node-less
directory, paid once per process behind the cache.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
 2026-09-05 21:33:16 -07:00
Neil ba86e1c83c perf: avoid repeated whitespace scans in diagnostic redaction (#18908) 
* perf: scan diagnostic environment lines without repeated whitespace searches

* perf(observability): skip redundant terminator scans in environment-line redaction

After a successful match ENV_LINE.lastIndex already sits at end of input
or a line terminator, and a failed start's whitespace skip plus terminator
scan collapse into one LINE_CONTENT exec. Byte-identical output; verified
against the pre-change regex over 280k generated inputs.

* perf(observability): find environment-line ends without entering the regex engine
 2026-09-05 21:17:46 -07:00
OrcaWinOrca WorkerNeil
 		0cbb01ef4b fix(security): apply the Windows path-hardening ACL that never ran (#17884) 
* fix(security): apply the Windows path-hardening ACL that never ran

`buildWindowsRestrictAclArgs` invoked the hardening script as
`powershell.exe -Command <script> <path> <sid> <isDir>`. `-Command` does
not populate `$args`; it appends the trailing tokens to the command text.
The script therefore read `$args[1]` as `$null`, threw `NullArrayIndex` at
`$allowedSids[$sidText] = $true` under `$ErrorActionPreference = 'Stop'`,
and exited 1. Both callers swallowed that: the async callback was empty and
`applySecurePathRestriction` returned `true` regardless, while the sync
`catch` returned `false` and nobody logged. Every Windows secure path has
been left on its inherited ACL since the ACL was introduced (#5006), and
nothing said so.

Replace PowerShell with `icacls.exe`, which takes plain argv. That removes
the quoting surface entirely rather than escaping it: interpolating a path
into the command text would have turned a dead no-op into arbitrary
PowerShell on a filesystem path, since `-Command` executes what it appends.
It also drops the execution-policy dependency and the `powershell.exe`
spawn an EDR flags, and runs ~25x faster than the PowerShell cold start.

Hardening is now three passes: `/reset` to purge explicit ACEs that
`/inheritance:r` leaves behind, `/inheritance:r` plus a `/grant:r` per
allowed SID, then a read-back that checks the DACL is protected and grants
only the intended rights. The predecessor's verification block was equally
dead, and an apply that is never read back is only half a control.

Failures stay non-fatal — non-NTFS volumes, network paths and restricted
tokens fail legitimately and must not break startup — but they are no
longer invisible: every failure is logged, and a failed async apply now
evicts its cache entry so the next call retries instead of trusting a
success that never happened.

Routing through `runProcess`/`runProcessSync` also retires this file's
`node:child_process` allowlist entry.

* fix(security): verify the hardened ACL by identity, not by shape

Review found the bug class this PR fixes surviving inside the fix. The
verify pass checked rule count, absence of the inherited marker, and exact
rights — never *who* the rules named. Granting Everyone full control
satisfies all three, so hardening reported success on a DACL that handed
the credential to every local account, and most of the real-filesystem
tests still passed.

Verification now reads the descriptor back with `icacls /save`, which emits
SDDL with raw SIDs, and compares the principal set exactly. That is also
locale-independent by construction: the previous parse read localized
account names out of icacls' OEM-codepage stdout, where a non-ASCII path
survived by accident rather than by the documented mechanism. SDDL parsing
moves to `windows-security-descriptor.ts`.

Two further self-inflicted problems, both measured:

The post-rename re-harden led with `/reset`, which re-widened a DACL that
was already correct — the staged file's protected DACL survives the rename,
so the pass had nothing to do but open a window. Polling an external
process during a write into a relocated root caught it: the e2ee keypair
dropped to `BUILTIN\Users:(RX)` plus `Authenticated Users:(M)` — read *and*
write — before tightening again. Hardening now verifies first and returns
early when the DACL already reads back correct, which closes the window and
cuts the steady state from three spawns to one. Re-measured: 158 samples,
one DACL state, zero broad.

Evicting the cache on every failed async apply reintroduced #4901. The env
store re-hardens on the read path at ~2/s, so on a host where hardening
cannot work (FAT32, network path, restricted token) that was two icacls
spawns and two warnings a second, forever. Async retries now take a retry
floor and a hard per-path attempt cap. The write path keeps retrying
unthrottled — it is user-driven, and a failed credential ACL must still be
retried on the next write.

Also: failures route through a reporter hook that the main process points
at the diagnostic tracer, because `console.warn` reaches nothing in a
packaged GUI-subsystem build; `writeSecureFile` returns whether hardening
took, and the async branch reports `pending` rather than claiming `applied`;
a transient `whoami` failure no longer disables hardening for the process
lifetime, and the SID is shape-validated; the `/c` guard now covers the
synchronous runner too.

* fix(security): re-probe hardening instead of latching a transient failure

The per-process attempt cap added for the read-path storm was a permanent
latch: one AV scan, momentary lock or %TEMP% blip and every later credential
write in that session went unhardened, silently, on a host where hardening
would now succeed. Same defect class as #17858's computer-use host, and
worse here because what stops happening is security hardening on credential
files and nothing said so.

The retry budget now bounds the *rate*, not the lifetime: at most three
attempts per path per minute, re-probing in every later window, forever. The
transition is announced in both directions — `throttled` once per window on
entry, `recovered` when a rate-limited path hardens again — so a host stuck
in the degraded state is diagnosable rather than merely quiet. The reporter
type covers both, and the main process ends the `recovered` span
successfully rather than failing it.

Extracted to secure-path-hardening-retry-budget.ts, which keeps
secure-file.ts under its line cap without a max-lines disable.

Also confirms the second flagged risk rather than assuming it: a real
unwritable %TEMP% is now covered by a test proving verification fails
closed, reports at the `verify` stage, and still leaves the ACL applied —
so that path loses proof, not protection, and with the lifetime cap gone it
can no longer combine into a permanent-off state.

* fix(security): verify a directory's whole inheritance flag set

The flag check tested only that `OI` was present — never that `CI` was, nor
that nothing else was. That was harmless while `/reset` + `/grant` ran on
every pass and repaired whatever was there. The verify-first short-circuit
made it load-bearing: what verification accepts is now left alone, so a
latent under-check went live because a different fix started depending on
it.

Two directory DACLs passed while being wrong — both protected, three
non-inherited full-control rules, correct SIDs, differing from correct only
in their flags:

  (OI)(F)        - no CI, so subdirectories are left unprotected
  (OI)(CI)(IO)   - inherit-only, so the directory object itself grants
                   nobody anything; the next writeFileSync into it fails
                   with EPERM, on a directory just cached as hardened

Verification now compares the whole flag set, which also rejects IO and NP,
and names the offending flags in the failure. Both shapes are planted in
real-filesystem regression tests, including an assertion that a write into
the repaired directory succeeds and its child inherits. Confirmed both tests
fail against the old check and pass against this one.

* fix(security): back the hardening retry off exponentially

The fixed one-minute window bounded the retry rate but left a standing floor
of three attempts per path per minute on a host where hardening can never
succeed — FAT32/exFAT, a network path, a redirected profile. That budget is
per path and there are several secure files, so the floor multiplied into
tens of thousands of icacls spawns a day for work guaranteed to fail.

The delay now doubles after each consecutive failure, from a one-minute
floor to a thirty-minute ceiling, and the attempt cap is gone entirely: once
the backoff elapses the path is re-probed however long it has been failing.
A permanently incapable host settles at ~2 attempts/hour.

Slowing the backstop costs almost nothing, because it is not the recovery
mechanism: the synchronous write path is deliberately unthrottled, so a host
that recovers hardens on its very next credential write regardless of what
the read-path budget says.

The `throttled`/`recovered` reports are unchanged and matter more here,
since the quiet periods between probes are now much longer.

The curve is pinned in a new unit test against the exported delay function
rather than a copy of its constants, covering the doubling, the ceiling
holding at 5000 consecutive failures, a 30-day failing path still
re-probing, one announcement per degraded episode, and per-path isolation.
The integration tests keep only what they uniquely prove: that the read path
is wired to the budget, and that a day of failures still re-probes.
Confirmed four of these fail against a reinstated lifetime cap.

* ci(windows): run the real-icacls DACL 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(security): describe the cache's real cost, which is icacls now

Both cache comments still justified themselves with PowerShell -- "~1-1.5s" and
"a PowerShell spawn every read" -- in the same file whose PR removed PowerShell
from this path. The caches are still right, but for different numbers, and the
old ones are the kind an engineer would reasonably delete a cache over.

The real shape: hardening verifies first and returns early, so an already-correct
DACL costs one synchronous icacls spawn and a rewrite costs four (verify, reset,
grant, verify). Still worth caching on the read path, which polls at ~2/s.

* test(security): make the DACL suite safe to schedule

Registering this spec in the Windows lane put it under two rules it had
never been measured against.

Teardown now goes through `removeTreeSync`, which the lane's boundary test
requires, and repairs the DACLs the suite plants on purpose first: those
retries only cover transient locks, so a regressed `(OI)(CI)(IO)` repair
leaves the root un-removable and `afterAll` throws EPERM.

And the no-permission case decides by elevation before it writes anything.
`windows-2022` runs elevated, where hardening succeeds: the old branch
asserted nothing about denial and instead replaced the `hosts` DACL, then
`icacls /reset` -- which is not a restore, it drops the explicit
`SYSTEM:(F)` that file ships with. Ephemeral in CI; permanent for a
developer running the lane from an elevated shell. Now it asserts or it
skips. The probe reads the token integrity SID rather than `icacls /save`,
which succeeds unelevated (`BUILTIN\Users:(RX)` carries READ_CONTROL) and
would have skipped the case on every machine.

* fix(security): measure the hardening latches on a clock that cannot go backwards

`mayAttemptHardening` compared wall-clock times, so any backwards step --
an NTP correction, a VM snapshot restore, a user changing the clock --
made the elapsed time negative and held every failing path below its delay
until the clock caught up. Measured at the 30-minute ceiling with the clock
stepped back a year, the path was refused at +0d, +1d, +30d, +180d and
+364d, and re-probed only at +366d. That is the permanent latch the
exponential backoff was added to remove, and it contradicts the module's
own "bounds the rate without ever bounding the lifetime".

The SID lookup's own one-minute window had the identical shape and is
worse: a failed lookup makes `planFor` return null, which disables the
synchronous *write* path too, so the write-path exemption that recovers
the read-path budget cannot recover it. Both now measure elapsed monotonic
time, following the repo's existing `monotonicNowMs` spelling.

Two things the write path was not doing, both found in the same pass:

- A successful synchronous apply now records the outcome. It is exempt
  from the budget, but it was also invisible to it, so a host that had
  demonstrably recovered kept the read path backing off for up to 30
  minutes and no `recovered` transition ever came from that lane. Only
  success is recorded; recording failure would put the exempt lane back
  under the budget.
- `writeSecureFile`'s JSDoc now says its boolean covers the file only. The
  directory harden is fire-and-forget and answers `pending` on Windows
  regardless, so a `true` says nothing about the directory's ACL.

* fix(security): stop the hardening test doubles from faking a no-op

Three CI failures on this branch, one failure shape: hardening silently
does nothing and the check that should have caught it agrees.

The auth critical-path test hand-rolled a `node:child_process` factory with
`execFileSync`/`execFile`. The rewritten ACL path goes through
`runProcessSync`, i.e. `spawnSync`, which the factory never returned — so
every spawn threw into the SID lookup's bare catch, `planFor` returned null,
and hardening no-opped. It mocks `child-process/run-process` now, the
boundary production code actually calls and the one sibling ACL tests
already mock: an export missing there fails loudly by name instead of
returning undefined. Its fake icacls writes a real UTF-16LE SDDL file, so
the pinned spawn count per write is a property of the ACL path rather than
of the double. The test forces `platform='win32'`, so this failed on every
platform, Linux CI included.

`windowsSystem32Binary` is a production bug, not a test bug: it builds a
Windows path with the host `join`, which off-platform yields the mixed
`C:\Windows/System32/whoami.exe`. On Windows the two joins agree, which is
why it survived; on Linux the SID lookup's whoami match missed and 27 of
secure-file's 32 tests exercised a lane that never ran. These are always
Windows paths, so `path.win32.join` is what it should have been.

The import-boundary pin still read 160 after this branch migrated
secure-path-windows-acl.ts off `node:child_process`; the ratchet correctly
refuses a pin left above reality.

* fix(security): resolve the machine-relative SDDL alias, and stop a denied read destroying the file

Path hardening verified the DACL it wrote by comparing the SIDs `icacls /save`
reports. SDDL substitutes two-letter aliases for well-known SIDs, and the
resolution table could only hold constants -- but `LA` and `LG` name an account
by RID inside the *machine's own* SID, so on a box whose user is the built-in
Administrator (a CI runner, an Administrator-only install) the current user read
back as `LA`, matched nothing, and hardening reported failure for every path.
Resolve those two against the machine authority derived from the user SID;
without one they stay unresolved and the comparison still fails closed.

Three secret stores treated any read failure as "malformed -- regenerate" and
overwrote. A hardened file granting a SID this process does not hold reads as
EPERM while its directory stays writable, so the overwrite succeeds: renaming
over an unreadable file needs FILE_DELETE_CHILD on the parent, not DELETE on the
file. That destroyed the E2EE secret key, every paired device's bearer token,
and the plugin vault. Distinguish EPERM/EACCES from a parse failure and refuse.

Also close the async lane's unhandled rejection: `void p.then(onSettled)` turned
a throw from `onSettled` into a dead main process, and the retry budget it calls
threw whenever nothing had configured it -- a contract held only by import
order. The budget now defaults its own bounds.

* test(windows): say which ACEs icacls listed when a planted DACL fails

`toHaveLength` reports only a count and vitest elides the array, so three
preconditions failing on the CI runner said "expected 3, got 6" and nothing
about what the sixth entry was. Name the entries in the failure.

* fix(security): stop three more stores overwriting what they were denied

Same swallow-default-overwrite shape as the readers already fixed, found by
sweeping every store that reads under a hardened root.

- plugin-storage-store.ts returned `{}` on any read failure and set()/delete()
  wrote it back, losing the plugin KV store. It is the secrets store's shape
  line for line, so the two now behave identically.
- relay-revoke-outbox.ts returned [] and save() wrote it, dropping revocations
  that never reached the relay -- a revoked device stays live.
- profile-cloud-session-store.ts mapped an EPERM read onto `decrypt-failed`,
  which fails the `status === 'found'` guard in clearCloudSessionIfUnchanged and
  falls through to an rmSync of the account session. A denied read now reports
  `unreadable`, which licenses nothing; the refresh path bails on it and the
  auth status surfaces it rather than reporting a bare reconnect.

All reuse isPermissionDeniedError. The predicate stays an EPERM/EACCES allow
list rather than "ENOENT defaults, everything else throws": these stores are
meant to self-heal a truncated or malformed file, and inverting it would turn a
corrupt keypair into an app that cannot start. The distinction that matters is
"could not read it" versus "read it and it was garbage".

* test(windows): plant fixture DACLs that cannot inherit what they did not plant

%TEMP% grants [SYSTEM, Administrators, <user>] (OI)(CI)(F) by default, and those
propagate into every fixture. Three preconditions read back 4 and 6 ACEs where 3
were planted, and the extras looked like Orca's own hardening because the shape
is identical -- on a runner whose user is the built-in Administrator, the
inherited trio IS the trio production grants.

Combining /inheritance:r with /grant:r leaves the argument order to icacls, and
that combined form drops the inherited ACEs on Windows 11 but keeps them as
explicit ones on the Windows Server runner. Removing inheritance in its own
invocation makes the grant the whole DACL on either host, and the fixture root
is de-inherited once up front so nothing propagates in.

Rooting the fixtures outside %TEMP% would not have fixed this: any directory
inherits from wherever it lives. The fix is to stop inheriting, not to move.

No assertion is relaxed -- the counts stay exact.

* test(windows): pick a foreign SID that stays foreign on an elevated runner

`S-1-5-32-544` is only foreign to a token that is not an administrator. The CI
runner is elevated AND logged in as the built-in Administrator, so granting
Administrators granted the reader full control: the file stayed readable, and
all six preservation assertions went vacuous rather than proving anything.

BUILTIN\Guests is resolvable everywhere and no interactive token is a member,
so the read is denied on an unelevated developer box and on the runner alike.
An unresolvable SID would have been the stronger choice but icacls rejects one
with ERROR_NONE_MAPPED (1332).

The premise guard is what caught this -- it asserted the file was actually
unreadable instead of trusting the grant, and named elevation as the suspect.

* fix(security): refuse on any read that never reached the contents, not just a denied one

isPermissionDeniedError becomes isUnreadableError, because "permission denied"
was never the concept -- "could not read it", as opposed to "read it and it was
garbage", is. EBUSY, EMFILE, ENFILE and EIO say exactly as little about a file's
contents as EACCES does, and they fell into the branch that regenerates and
overwrites. On Windows EBUSY is the likelier of the two: antivirus holding a
credential open at the moment of a startup read produces it, which makes it a
commoner path to the same permanent loss than the ACL case that motivated the
original fix.

Still an allow list, deliberately: ENOENT keeps licensing a create, and a parse
failure keeps self-healing. The stores are built to recover from a truncated
write, and turning that into a refusal would trade a recoverable state for an
unrecoverable one on the startup path.

Also fixes the regression suite's own premise on an elevated runner:
makeUnreadable combined /inheritance:r with /grant:r, and that form keeps
%TEMP%'s inherited [SYSTEM, Administrators, user] as explicit ACEs on Windows
Server -- so the file stayed readable and all six assertions were vacuous. Same
split-the-invocation fix as the ACL suite's planter.

* test(windows): skip the preservation suite where a read cannot be denied

An elevated token logged in as the built-in Administrator reads straight through
a DACL that grants it nothing -- confirmed on the CI runner against both
BUILTIN\Administrators and BUILTIN\Guests, and with the grant split into its own
icacls invocation so the DACL really was the planted one. On such a host the
premise these tests rest on does not hold, and every assertion would pass while
proving nothing.

So probe once at module scope and skip rather than assert vacuously -- the same
trade the ACL suite already makes for its unelevated-only case. The gate stays
in the compound `<win32 check> && <flag>` form the win32 lane ratchet detects, so
the file stays registered in both lane lists.

Coverage is not lost where it counts: isUnreadableError has unit tests that run
on every platform and every host, and the stores' refusal is exercised in full on
any machine where a denial is reproducible -- which is every developer box.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
 2026-09-05 21:13:06 -07:00
OrcaWinandOrca Worker 975bbdedcc fix(windows): scan ports natively instead of encoded PowerShell (#17861) 
* fix(windows): scan ports natively instead of encoded PowerShell

Microsoft Defender for Endpoint scored the relay's Windows port scan as
suspicious PowerShell plus network discovery (T1049). The command line was
`-ExecutionPolicy Bypass -EncodedCommand <base64>` around a
Get-NetTCPConnection/Get-Process join -- base64 next to a policy override is
the highest-weighted token pair on a PowerShell command line, and netstat only
ever ran as its fallback.

Invert the chain. `netstat.exe -ano` is now the primary reader and the owning
process name comes from the shared native process table, which exists to keep
PID lookups off PowerShell. The payload survives only as a last resort, and
without the override: execution policy gates script files, never `-Command`,
so nothing needed it (verified: `-ExecutionPolicy Restricted -Command` runs).

Drop `-p tcp` while inverting: on Windows that protocol name means IPv4 only,
so as a primary reader it would have hidden every `[::]` listener the payload
used to report. Names arrive as `sshd.exe` from the table and are published as
`sshd`, keeping the sshd filter and old clients' rendering intact.

Routes both spawns through runProcess, removing the file from the
child_process and windowsHide ratchets.

* fix(windows): read netstat state by shape and refuse a truncated table

Review of the port-scan inversion found two ways the new primary path could
be silently wrong, both of which would have kept the flagged PowerShell
payload running on exactly the hosts this change targets.

`LISTENING` is not in netstat.exe. It lives in System32\<locale>\netstat.exe.mui
and MUI selection follows the UI language, so the pinned-locale env in
relay-command-env.ts cannot reach it -- a German host prints `ABHOEREN` and the
word test parsed zero rows. The zero-listeners guard then read that as a
blocked reader and ran `Get-NetTCPConnection` every 12-30s forever, or returned
nothing at all where PowerShell is also restricted. Keep the word as the fast
path and, when it finds nothing over output that did contain TCP rows, re-read
by shape: only a listening socket has no peer. Measured on this host across all
four states present (LISTENING 47, ESTABLISHED 49, CLOSE_WAIT 29, TIME_WAIT
213): zero non-listening rows with a zero peer, zero listening rows without
one, and the same 47 rows parse after substituting the German state words.
Shape stays the fallback because `BOUND` also prints a zero peer.

Truncation was invisible: createOutputSink discards overflow, ProcessResult
carries no flag, so a capped read still exits 0 and its head still parses.
netstat orders IPv4 TCP, then IPv6 TCP, then UDP, so a host with tens of
thousands of TIME_WAIT rows would have lost every `[::]` listener -- the exact
loss dropping `-p tcp` exists to prevent, and one the zero-listeners guard
cannot see. Refuse the read instead. A `truncated` flag on the shared sink
would be cleaner and is left as a follow-up rather than widened into this PR.

Also: decline to wait on the shared process table once the request is aborted
(it takes no signal and must not be cancelled for other callers); note the
name lookup as best-effort, since a TTL-cached snapshot can hand a recycled
PID its previous owner name; log once on either fall-through, because both are
permanent and invisible when wrong; and drop a stderr assertion that any
PowerShell autoload banner would redden.

Correcting the cost claim in the previous commit: the aggregate win holds with
the native addon (netstat 21ms vs the retired payload 860ms at 532 processes),
not without it. The addon is optional, the snapshot TTL is 500ms and the scan
cadence is 12-30s, so a relay with no active agent pane never warms its own
cache and pays ~1.4s cold on the CIM path -- slower than what it replaced.

* fix(windows): log the port-scan fall-through on the relay diagnostic stream

Checked where this code actually runs before trusting the log. `console.warn`
did reach a file, but relayLogLine is the right call and the reasoning is worth
recording.

`scanWindowsListeningPorts` runs only in the detached relay daemon: relay.ts
returns early for --connect and --orca-cli, so PortScanHandler is reached only
through runRelayDaemon, and both launchers start it detached with a log file
(POSIX `> relay.log 2>&1`, Windows `1>relay.log 2>relay.err.log` via
Win32_Process.Create). installRelayLogRotation then wraps both streams into
relay.log, which is the file the documented diagnostics tail reads. Verified by
installing the real rotation over a temp path and reading the file back.

So the line surfaced -- but untimestamped, in a log whose format exists so
reconnect flaps can be correlated with the events around them (#7773).
relayLogLine is that format and the relay idiom in 41 other places, and
"since when has this host been stuck on PowerShell" is most of what this line
is for. The test spies on process.stderr to pin the stream and the ISO stamp
rather than just asserting something was called, since a fall-through logged
somewhere unread is the failure being guarded against.

Also fixes a comment that ended its own block early: `relay-*/relay.log` in a
doc comment contains `*/`.

* fix(windows): keep the dominant zero-peer state when reading a localized netstat

Shape alone promoted any zero-peer TCP row, not just listeners. `BOUND` and
`CLOSED` print a zero peer too, and on a localized host their state words are
exactly as unreadable as the listening one -- so a German host with listeners
plus one BOUND socket published a phantom listener. Reachable on an English
host too: with zero listeners a lone BOUND row is promoted AND, because the
result is then non-empty, it suppresses the blocked-reader fall-through.

Group the zero-peer rows by state word and keep only the largest group. A
transient BOUND or CLOSED socket cannot outnumber the listeners (51 against 0
on this host), so this removes the class rather than special-casing the words,
which would just be the localization bug again. An exact tie keeps every tied
group rather than guessing -- no worse than reading shape alone.

Verified against real netstat output: injecting a BOUND row into the localized
capture leaves the result identical to the English answer (47 rows, no phantom
65001). The new test has teeth -- reverting the grouping fails it and nothing
else.

Corrects two claims that were slightly wrong: the docblock said shape was the
fallback because BOUND prints a zero peer, which described the hazard without
saying it was unhandled; and a test comment said an English host "never sees a
bound socket", true only when it has at least one readable LISTENING row.

Also gates the fall-through log per reason instead of per module, so a host
that parses nothing today and truncates tomorrow reports both faults. Same
one-shot cost, and the vocabulary is two fixed strings so the set cannot grow.
That guard matters more than it looks: --log-file rotates stdout only, so the
file stderr can land in is unrotated.

* docs(windows): note the direction the zero-peer majority rule can fail in

The docblock described the tie case and stopped there, which reads as a
complete account of the limits when it is not: a majority rule inverts if the
majority is wrong, and enough transient zero-peer sockets would publish the
phantoms and drop the real listeners. Someone would reasonably have concluded
the rule was safe in both directions.

Trigger numbers and the repro stay in the PR discussion; the code only needs
the reader to know the rule has a direction, and the hatch (defer to the
PowerShell reader, which reads the state word instead of inferring it) since
that is the part a future editor would otherwise re-derive.

* ci(windows): run the real-netstat port scan 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.

* test(windows): lower both child-process ratchets to the ground this PR took

Migrating the port scan off `node:child_process` onto `runProcess` drops
`src/relay/windows-port-scan.ts` from both allowlists, so both offender
counts fall by one. Each ratchet pins the count from below as well as
above, so a pin left above reality fails and re-opens room for the next
direct import to land for free.

* docs(windows): qualify the no-PowerShell claim on the netstat scan

The scan starts no PowerShell of its own, but no released relay carries the
optional `windows-process-tree.node` addon (only dev-channel-win-build.yml
builds it), so the shared process-table read falls back to a CIM scan that
forks one `powershell.exe`. The EDR win is the removal of the
`-EncodedCommand` / `-ExecutionPolicy Bypass` shape, not the elimination of
PowerShell. Comment-only.

* docs(windows): record the identity-reader follow-up and the perf table's addon

attachWindowsProcessNames reads only `name`, so it should move to
`readWindowsProcessIdentityTable` once #17866 lands -- on that PR's detailed
reader it would open per-process handles for a field it discards. The reader
does not exist on this branch, so the call stays as-is with the follow-up
recorded rather than pulling #17866 in.

The process-table perf table's two Toolhelp32 rows assume the optional
`windows-process-tree.node` addon. The desktop bundles it; no released relay
does, so on an SSH host the CIM row is the operative number. Comment-only.

* docs(windows): state the CIM scan as the relay's normal path, not a fallback

No released relay carries the optional `windows-process-tree.node` addon --
release-cut.yml has zero references to it and only dev-channel-win-build.yml
builds it -- so the PowerShell CIM scan is what every SSH host runs. The
call-site docstring read as a conditional fallback standalone. Comment-only.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
 2026-09-05 21:12:59 -07:00
OrcaWinOrca WorkerNeil
 		bfc6a262a7 fix(windows): read command lines from the kernel, not each process's PEB (#17886) 
* fix(windows): read command lines from the kernel, not each process's PEB

MDE incident D scored Orca for suspicious memory activity: the vendored
`@vscode/windows-process-tree` recovered every process's command line by
opening it with `PROCESS_QUERY_INFORMATION | PROCESS_VM_READ` and chaining
three `ReadProcessMemory` calls through the PEB and
`RTL_USER_PROCESS_PARAMETERS`. On a 750ms/2s cadence over the whole table that
is the credential-dumping primitive, whatever the intent.

Windows 8.1 added `NtQueryInformationProcess`'s `ProcessCommandLineInformation`
class (60), which returns the same string as a kernel-built `UNICODE_STRING`
under `PROCESS_QUERY_LIMITED_INFORMATION` alone. Electron's floor is Windows
10, so every supported OS has it. The PEB reader stays behind a process-wide
latch that only `STATUS_INVALID_INFO_CLASS`/`NOT_SUPPORTED`/`NOT_IMPLEMENTED`
can set; a pid that merely denied a handle does not re-arm it, because
`PROCESS_QUERY_INFORMATION` implicitly grants the limited right and so cannot
be obtained where the weaker open already failed.

The same hunk drops `PROCESS_VM_READ` from `GetProcessMemoryUsage` and
`GetCpuUsage`, which acquired it and never read an address space.

Measured on Windows 11 (514 processes), counted in-process by swapping the
addon's import table entries for counting stubs, per CommandLine scan:
`ReadProcessMemory` 1128 -> 0, desired access 0x0410 -> 0x1000, p50 12.7ms ->
9.3ms. Command lines were byte-identical on every process both readers
recovered (376/376, 379/379 across runs), including a 24,068-character argv
with quotes, non-ASCII and trailing whitespace, and a WOW64 target. Three
processes that refused the old rights granted the new one; none went the other
way.

* chore(deps): refresh the windows-process-tree patch hash in the lockfile

* fix(windows): drop the PEB fallback and detect the unpatched prebuilt

Review of #17886 found three ways the reader could still perform, or silently
resume, the primitive it exists to remove.

The class-missing latch was a permanent, process-wide, one-way downgrade back
to the PEB read, and any single target returning STATUS_INVALID_INFO_CLASS /
NOT_SUPPORTED / NOT_IMPLEMENTED could trip it. On an EDR-hooked ntdll -- the
entire premise of this change -- a hook that does not recognise class 60 would
have restored PROCESS_VM_READ plus three ReadProcessMemory per pid per scan for
the life of the process, unobservably, on precisely the machines this was
written for. The fallback is deleted rather than guarded: GetProcessCommandLine
now returns false and leaves the command line empty, which callers already
handle, so the addon imports no ReadProcessMemory at all.

That absence is what makes the property checkable on the artifact. The
published 0.8.0 tarball ships a loadable prebuilt built from unpatched source;
it is node-addon-api, so a bare require() accepts it, allowBuilds is false and
CI installs with --ignore-scripts, and a rebuild that soft-exits on a Windows
file lock leaves it in place. Source-text guards could never see it.
windowsProcessTreeAddonReadsProcessMemory() checks the compiled binary instead,
and is wired into the install check, the rebuild, and the relay build.

The repair itself never worked: `git apply` run inside a work tree prefixes
patch paths with the cwd-relative prefix, skips what does not match, and exits
0, so the branch always fell through to its own post-check throw. The package
dir is always under the project root, while the fixture that covered it was in
%TEMP%, outside any repo. Blinding git with GIT_DIR fixes it, and the test now
runs inside a real work tree.

Also from review: bounds-check the returned UNICODE_STRING against the
allocation (not the size the second query clobbers) and cap the probe so a
bogus length cannot bad_alloc a whole scan; test NT_SUCCESS explicitly; value-
initialize ProcessInfo, which left `memory` as stack garbage -- measured, 82
processes reported the same bogus working set; and correct a comment in
windows-process-table.ts that still described the command line as a PEB read.

Re-measured on Windows 11 (543 processes): ReadProcessMemory 1128 -> 0, with
the symbol absent from the import table so the IAT hook finds no slot to
count; desired access 0x0410 -> 0x1000 on all 543 opens; p50 13.5 -> 12.3ms;
405/405 command lines byte-identical including a 24,087-character quoted
non-ASCII argv and a WOW64 target; 3 processes recovered only by the new path,
0 only by the old.

* chore(deps): refresh the windows-process-tree patch hash in the lockfile

* test(scripts): stage a script's local imports into the native-runtime fixture

ensure-native-runtime.mjs gained an import of windows-process-tree-gyp-rebuild.mjs,
but the fixture copied only the script itself, so every case in the suite died
with ERR_MODULE_NOT_FOUND before reaching its own assertions. copyScriptWithLocalModules
already walks a script's co-located imports for exactly this reason -- its own doc
comment names this failure -- so use it rather than listing files by hand.

The two Windows cases still fail here, on a missing node-pty ConPTY runtime that
also fails on main; this only stops a resolution error from standing in front of
whatever they were meant to catch.

* fix(windows): route a locked stale addon to the Windows file-lock message

`pnpm install` with Orca running aborted with a raw EPERM stack. The stale-binary
guard -- which deletes an addon that still imports ReadProcessMemory so a skipped
rebuild cannot use it -- ran outside the try whose catch classifies Windows file
locks, and whose message is literally "Close running Orca/Electron/dev processes
for this worktree": exactly this situation.

Measured rather than assumed: rmSync against a loaded (memory-mapped) addon throws
EPERM, and `force: true` does not help, since it only swallows ENOENT. Cold copies
of the same file delete fine. So the delete threw a page before the handler that
knows what it means.

Moving the guard inside the try is the whole fix; the classifier already matches
the EPERM text. The new case runs the real script against a temp project whose
stale addon is held open by a live child process, and fails against the old
placement with the raw `syscall: 'rm'` stack the report described.

* feat(windows): warn once when command-line recovery is refused host-wide

Removing the PEB fallback removed a total-defeat vector, but it left a cliff: if
NtQueryInformationProcess(ProcessCommandLineInformation) is refused -- a hooked
ntdll that does not know class 60 -- every command line comes back empty and
agent identity matching silently degrades to image names. The addon still loads
and still enumerates, so every health check the app has stays green. A cliff
nobody can see is the failure mode this area keeps producing.

The querying process is the unambiguous probe. A process can always open itself
with PROCESS_QUERY_LIMITED_INFORMATION, so its own command line coming back empty
means the query is refused for every process -- not that some target denied a
handle, which is normal for roughly a quarter of the table. Keying on our own row
rather than a fraction means no threshold to tune and no false positive on a
hardened box where most processes deny.

One warning per session, gated on the CommandLine flag actually being requested so
a future identity-only reader cannot trip it. The suite's own SELF fixture gains a
command line for the same reason: a self row without one is the alarm, not a
detail.

* fix(windows): check the relay's staged addon at load, and answer tri-state

Two gaps in the ReadProcessMemory check, both about what it does not see.

It only ever looked at node_modules/@vscode/windows-process-tree. A relay host
has no node_modules of ours: it loads ./windows-process-tree.node staged beside
the bundle. The relay build asserts the symbol on the artifact it produces, but a
bundle and the addon beside it redeploy independently, so a host that has not
taken a new bundle keeps whatever binary is already there -- and the published
prebuilt is node-addon-api, so it binds cleanly and then walks every process's
address space. loadWindowsProcessTree now checks that file too and refuses it,
falling back to the CIM scan: slower, but not the thing an EDR quarantines a host
for. The predicate is duplicated rather than imported, because the config-script
copy is install-time tooling that drags in node-gyp and child_process, and this
module is bundled into the app and the relay.

And it returned false for a binary that is not there. All three callers happened
to be safe, but the name read as a safety predicate, so a future caller would take
a missing binary as verified. inspectWindowsProcessTreeAddon() now answers
clean/unpatched/missing over an explicit binary path -- which is also what lets
the relay's staged addon be checked at all -- and each caller states which state
it acts on.

Both are covered by cases that fail against the old code: without the load-time
check the unpatched staged addon is bound and the CIM fallback never runs, and
with 'missing' folded back into 'clean' the absence case fails outright.

* test(windows): load the addon in beforeAll, not at collection time

loadAddon() ran while the file was being collected, so on a Windows checkout with
no built addon the require threw before any case existed and took the seven
patch-text cases down with it -- cases that read only the patch file and need no
binary at all. Verified both ways against a deliberately unresolvable addon path:
at collection time vitest reports "no tests" for the file; from beforeAll the
seven text cases pass and only the three addon cases go.

* fix(deps): normalize the windows-process-tree patch to LF and let pnpm own its hash

`pnpm install --frozen-lockfile` failed on this branch on every platform with
ERR_PNPM_LOCKFILE_CONFIG_MISMATCH, which breaks CI and the release build.

Two coupled defects. The patch file was committed with CRLF -- 174 CR bytes,
against zero on main -- and `.gitattributes` pins `/config/patches/*.patch -text`
precisely so checkout cannot convert it, so those bytes reached every runner. And
pnpm hashes a patch **LF-normalized**, so the raw sha256 of a CRLF file is a value
pnpm never computes:

  raw sha256      322965470c05f63d8527f7d8e892ee26ee444136b66b57fd64c362a9f2ff05d1
  LF-normalized   f8ea245391c94da5770045aeea01fa6de466c2199c6ef46b5b769b398aa9823e

The lockfile carried the raw one, at all three sites. It is the only one of the
seven patches where the two digests differ, which is why the other six passed.

Normalized the patch to LF and took pnpm's own value from
`pnpm install --no-frozen-lockfile`; nothing here is hand-computed. With the file
LF-only the two interpretations coincide, so the lockfile, the contract test's
no-CR assertion and its hash assertion all agree at one number -- and
`config/scripts/windows-process-tree-patch-contract.test.mjs`, which was red on
this branch for the same reason, is green again. The lockfile diff is exactly the
three hash lines.

The regression check is the installer, not a digest. Two separate reviews
"verified" the shipped hash by recomputing sha256(patchBytes) and matching the
lockfile; both were wrong, because both repeated the same wrong assumption about
which bytes pnpm hashes. A check that reproduces the original mistake is not
independent. So the new case runs `pnpm install --frozen-lockfile --lockfile-only
--ignore-scripts` against a copy of the manifest, lockfile and patches, and
asserts exit 0 -- verified by deletion: restoring the shipped hash fails it with
the exact ERR_PNPM_LOCKFILE_CONFIG_MISMATCH from the branch's package (windows)
job.

Also corrected the `.gitattributes` comment claiming pnpm hashes patches
byte-for-byte. The `-text` setting is right -- `git apply` needs the exact bytes --
but that sentence is the claim that produced the wrong hash twice.

* ci(windows): run the process-tree patch suites in CI

Both suites only self-skip off Windows, so the binary-level check that the
addon carries no ReadProcessMemory passed vacuously in every lane.

* fix(windows): force core.autocrlf=input for the patch repair

My LF normalization of the windows-process-tree patch broke the `git apply`
repair path introduced in this PR. The two are coupled and I checked only one.

Those 174 CR bytes were not editor noise. They sat on exactly the pre-image
lines and nowhere else -- 107/107 in src/process.cc, 67/67 in
src/process_commandline.cc, 0 on every added or context line -- because
@vscode/windows-process-tree@0.8.0 ships those two sources as CRLF. Normalizing
the patch made its pre-image stop matching the file it is applied against.

Measured, reconstructing the true CRLF pre-image from the pre-normalization
blob and applying the current LF patch:

  core.autocrlf   plain   -c core.autocrlf=input
  true            exit 0  exit 0
  input           exit 0  exit 0
  false           exit 1  exit 0

`false` is Git's own built-in default and what "checkout as-is" selects in the
Git for Windows installer -- on this box the `true` that hides it comes from the
installer's system gitconfig, not from anything in the repo. There the repair
throws, ensureWindowsProcessTreeCommandLinePatch reports "still reads the PEB,
and repairing it ... failed", isWindowsNativeLockError does not match that text,
and `pnpm install` dies with no path forward.

Forcing the mode rather than `--ignore-whitespace`: both fix every cell and both
leave the applied file fully LF, but `input` relaxes line endings only, so a hunk
whose real content drifted is still rejected. The repair rewrites a
security-relevant source file; it should stay strict about everything except the
thing that is legitimately ambiguous.

Not reverting the patch to CRLF: windows-process-tree-patch-contract.test.mjs
(pre-existing on main) forbids CR bytes in it, and pnpm computes the same hash
either way. LF plus the forced mode is the end state.

The suite could not have caught this. The fixture built its pre-image from the
patch itself and joined with '\n', so fixture and patch agreed by construction on
any encoding -- once again a test that passes without its fix. It now emits the
CRLF the real package ships, and the case runs under both autocrlf modes pinned
through a temp HOME gitconfig, because the repair blinds git to the repo and so
reads global config. Verified by deletion in both directions: with the flag
removed the autocrlf=false case fails with the exact "still reads the PEB" dead
end while autocrlf=true still passes, and with the fixture back on LF all eight
cases pass with no fix present at all.

Also corrected the .gitattributes comment I added last commit. It said `git
apply` needs the bytes the patch was written against, which is now false -- the
pinned bytes are LF and the bytes it was written against are CRLF. That is the
same class of confident-and-wrong claim that produced the bad hash twice.

* fix(windows): assert the rebuilt addon, and install the patch for real in tests

Three follow-ups from review.

**The packaged binary had no check.** The relay build asserts its own artifact
and ensure-native-runtime asserts what it loads, but nothing looked at the addon
copied into the packaged app -- so a rebuild that silently produced the upstream
reader shipped. `rebuild-native-deps.mjs` now asserts `clean` on it after
`rebuild()`. This is also the caller D4's tri-state was missing: every existing
site branches on `=== 'unpatched'`, so `missing` still behaved exactly like
`clean` everywhere, which was the thing making it a state rather than a boolean.
Here both non-clean states fail, and they fail differently: after a rebuild that
reported success, an absent binary is a broken build, not an absence to shrug at.

The fake `rebuild()` had to start producing a binary for that to mean anything,
so it now emits stand-in bytes and takes `addon: 'clean' | 'unpatched' | 'none'`.
Verified by deletion: with the assertion removed both new cases pass.

**The frozen-install case could not see a patch at all.** `--lockfile-only`
resolves and never applies one, so its coverage stops at hash consistency. Added
a case that installs `@vscode/windows-process-tree@0.8.0` for real with the patch
and asserts the materialized `src/process_commandline.cc` carries the marker and
no longer carries `ReadProcessMemory` -- about 1.5s for the pair.

Correcting the brief on that one: it does **not** catch the `git apply` breakage
from the previous commit. Measured -- with `-c core.autocrlf=input` removed it
passes cleanly, because `pnpm install` uses pnpm's own patch applier and never
runs our repair script. What it does catch is a patch pnpm can no longer apply:
corrupting one pre-image line fails both cases. The repair path stays covered by
the CRLF fixture in rebuild-native-deps-node-pty.test.mjs.

Worth recording, since it decides whether the LF normalization was safe at all:
pnpm applies the LF patch to the CRLF tarball sources without complaint, and
materializes them as LF with the marker present and `ReadProcessMemory` absent.
The primary install path was never affected -- only the `git apply` fallback was.

**Dead timeout.** The frozen-install case passed `timeoutMs: 300_000` to the
spawn while vitest capped the case itself at 30s, so on a cold runner vitest
would have killed it first. Both cases now declare the budget they use.

* test(windows): route the frozen-install check through the pnpm invocation owner

The new patched-dependencies check hand-rolled a PATH walk naming 'pnpm.cmd',
which the windows batch shim spawn boundary ratchet rejects: pnpm-cli-invocation
already owns that decision for every other script, and its allowlist only
shrinks.

Reuse resolvePnpmCliInvocation for the command and prefixArgs, and the shared
resolveCliCommand for the presence check, so no shim name is spelled here. Its
`shell` flag is dropped because runProcessSync refuses it and already drives a
shim through the interpreter itself.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
 2026-09-05 21:12:47 -07:00
OrcaWinandOrca Worker 687a22e1ee fix(computer-use): run the Windows runtime as one persistent helper (#17858) 
* 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>
 2026-09-05 21:12:33 -07:00
OrcaWinandOrca Worker fba90e017c fix(windows): copy the daemon host exe verbatim instead of renaming it (MDE T1036) (#17865) 
* docs(windows): document the EDR signal surface

Six Microsoft Defender for Endpoint incidents fired against Orca 1.4.192 in
eight days on one enterprise Windows 11 / Intune tenant. All six were
behavioural process-tree scoring, not signature hits; two escalated to
multi-stage incidents mapped to ATT&CK Execution and Collection.

Add a reference doc mapping each attack-technique-shaped behaviour to the code
that produces it and to why it exists: the renamed daemon image (T1036), the
per-process PEB read, encoded policy-bypassed PowerShell (T1049), caret-escaped
cmd.exe lines, and computer-use screen capture plus runtime-compiled MSIL
(T1113). Records that signing is not the gate -- reputation is signer plus
hash-keyed prevalence -- and carries the two evidence gaps the report noted.

Adds an engineer checklist, deployment guidance for admins (AV path exclusions
do not suppress EDR behavioural alerts; an MDE alert suppression rule does), and
an explicit pre-deployment warning about computer use.

* docs(windows): correct the PowerShell flag inventory and admin paths

Review corrections to the EDR posture doc.

The "encoded, policy-bypassing PowerShell" list conflated three different
shapes and was incomplete. Split it into the three tiers an EDR actually scores
differently -- bypass plus encoding, encoding alone, and bypass alone -- and add
the sites it missed, including windows-mobile-firewall.ts, which encodes a
script and launches it elevated through Start-Process -Verb RunAs. system-fonts.ts
(-Command) and desktop-script-provider-bridge.ts (-File) were listed as encoded
and are not. Notes that a raw grep under-reports, because the hook sites reach
-EncodedCommand through wrapWindowsPowerShellEncodedCommand.

Attribute the in-payload Set-ExecutionPolicy move to #16576 rather than to
#16003's measurement, which keyed on -WindowStyle Hidden + -EncodedCommand, and
record that the launcher's own tradeoff is unverified on a real box.

Admin guidance was missing two ways a suppression rule pinned to one full path
misses real activity: the .staging-<hex> sibling that exists mid-update, which
is when the update-cluster incidents fire, and the userData fallback when
LOCALAPPDATA is unset.

Also: state the measurement conditions on the process-table timings, note that
Hermes has surface even though we have no telemetry for it, note that the
uninstaller names are electron-builder-generated and in no repo file, drop a
volatile line count, and mark the per-operation computer-use shape as being
addressed by an unmerged change. Drops the duplicated AGENTS.md section, keeping
the indexed bullet.

* docs(windows): reconcile the EDR posture doc with the shipped remediation

Three claims in this doc became false once the rest of the Windows EDR set
landed, and two told engineers the opposite of what the release does.

The process-table section still described one shared snapshot taken with
`Memory | CommandLine | CreationTime`, argued that splitting the cache per
field set "would restore exactly the fan-out it exists to prevent", and
concluded the shape was unfixable because "the information is only in the
PEB". The split shipped (identity opens no handle at all), `Memory` is
retired, and the command line now comes from the kernel through
`ProcessCommandLineInformation` -- `ReadProcessMemory` is absent from the
compiled addon and a ratchet asserts it against the import table. An engineer
reading the old text would have concluded both fixes were dead ends.

The PowerShell site inventories were stale in three of four lists: the port
scan went native, every `-ExecutionPolicy Bypass` + `-EncodedCommand` pair
was dropped as a measured no-op, and of the unencoded-bypass list only
`wsl-cli-scripts.ts` survives. Regenerated against the merged tree, including
the sites that reach the flag through `wrapWindowsPowerShellEncodedCommand`
and never spell it, which a raw `rg` misses.

Incident-evidence sections are left alone: they record what the tenant observed
on 1.4.192, not what the code does now.

* fix(windows): copy the daemon host exe verbatim instead of renaming it

Microsoft Defender for Endpoint flagged `orca-terminal-daemon.exe` as MITRE
T1036 (Masquerading): Orca copied its own `Orca.exe` into %LOCALAPPDATA% under a
different name, specifically so the NSIS updater's `taskkill /IM Orca.exe` could
not match, then ran it detached. Because that process is what every other flagged
action was attributed to, the name mismatch acted as a reputation multiplier on
unrelated findings.

The rename was never what made the daemon survive. In app-builder-lib 26.15.3 the
installer's FIND_PROCESS/KILL_PROCESS select processes whose image path is under
$INSTDIR; `taskkill /IM` is only the fallback for hosts where PowerShell is
missing or blocked. Survival is a property of the path, and
%LOCALAPPDATA%\Orca\daemon-host is outside $INSTDIR whatever the file is called.

Derive the host exe name from process.execPath so the copy is byte-for-byte,
name included — it keeps its Authenticode signature and carries no renamed-image
signal. On the no-PowerShell fallback the daemon is now killed with the app and
terminals cold-restore, which is the documented pre-relocation outcome the update
harness already asserts, not a regression.

The uninstall macro no longer needs a distinct name to find the daemon; it kills
the app's own image name (plus the legacy name, for hosts left by older builds).

Adds docs/reference/windows-daemon-host-relocation.md with the survival contract,
the rejected alternatives and their measured costs, and the invariants to keep.

* fix(windows): apply daemon-host relocation review corrections

Scope the uninstall taskkill to the current user with `/FI "USERNAME eq
%USERNAME%"` via cmd.exe, matching upstream's per-user KILL_PROCESS — without it
an elevated machine-wide uninstall reaches another logged-on user's session, so
the "no collateral" claim in the comment was overstated.

Comment the rmSync-before-publish: Windows refuses to delete a running image, so
a live daemon already hosted in this version's dir (same-version reinstall, or a
dev channel reusing a version) throws and materialization fails open.

Doc corrections:
- The fallback selector is the full per-user `taskkill /F /IM "<app>.exe" /FI
  "PID ne $pid" /FI "USERNAME eq %USERNAME%"`, not a bare `taskkill /IM`.
- The probe reads `Get-ExecutionPolicy -Scope Process`, not the effective policy,
  and GPO writes MachinePolicy/UserPolicy — so GPO-managed hosts take the primary
  path-scoped branch. Narrow the fallback triggers accordingly.
- Drop the Authenticode sentence: the old name was equally byte-identical and
  equally signed, so a filename has no bearing on signature validity.
- Name the new update-abort path: the daemon now matches FIND_PROCESS, so on the
  fallback branch an unkillable host reaches the retry loop's MessageBox /SD
  IDCANCEL and Quits, aborting a silent update.
- Correct the customCheckAppRunning rejection. It is ~6 lines, not a rewrite; it
  is wrong because forcing the PowerShell branch where PowerShell is absent makes
  FIND/KILL silently no-op and leaves the real app running with files in use.
- Bound the win honestly: OriginalFilename is empty on the shipped binary, so the
  strongest T1036 indicator never fired, and the residual copy-and-run-detached
  shape still maps to T1036.005.

Reconcile docs/reference/windows-edr-posture.md, which documents the rename as a
live finding and would otherwise contradict this change. Content-only edit:
markdown under docs/reference/ is not oxfmt-formatted as a matter of practice and
nothing in CI gates it, so the file is left consistent with its neighbours.

* fix(windows): expand USERNAME in NSIS instead of spawning cmd.exe

The uninstall macro routed both taskkills through `"$SYSDIR\cmd.exe" /C` purely
so `%USERNAME%` would expand — two extra interpreter spawns on the uninstall
path, in a change whose whole point is not adding scored behaviour, and the
exact `cmd.exe /c` shape the new AGENTS.md EDR bullet warns about. NSIS reads
the variable itself with ReadEnvStr, so the spawns buy nothing.

Verified on Windows 11 that the generated command line does what the filter is
there for: a copy of cmd.exe running as orca-nonexistent-probe.exe (pid 34244)
was terminated by `taskkill /F /IM "orca-nonexistent-probe.exe" /FI "USERNAME eq
<user>"` — SUCCESS, exit 0, process gone.

Guarded on an empty USERNAME because the degenerate case is silent: taskkill
rejects an empty filter value outright ("The search filter cannot be
recognized") and kills nothing, which would leave exactly the orphaned daemon
this macro exists to reap. `*` is rejected as a filter value too, so there is no
branchless spelling. With no USERNAME to scope by it kills unfiltered, as the
macro did before the filter was added. Stack stays balanced: three pushes, two
nsExec pops, three restores.

Also strike the last stale row in windows-edr-posture.md's remediation table.
"Copying our own image under a different name" read as outstanding work; it is
done by this change, so the row now points at the relocation doc. Same class of
staleness as the section reconciled in the previous commit, and git would not
have flagged it either.

* fix(windows): port the daemon-host uninstall sweep into the live NSIS include

The uninstall macro this branch rewrote lived in config/nsis/daemon-host-uninstall.nsh,
which main no longer includes: #17906 consolidated every Windows installer hook into
config/nsis/orca-installer-hooks.nsh because electron-builder accepts exactly one
`nsis.include`. Merged as-is, the rewritten macro would have been dead code while the
shipped uninstaller kept running main's stale sweep — `taskkill /F /IM
orca-terminal-daemon.exe`, which matches nothing now that the relocated host is a
verbatim Orca.exe copy. The RMDir that follows then cannot delete the running image, so
a live orphaned daemon and its ~224 MB tree would survive every uninstall.

Ported into the live include: the ${APP_EXECUTABLE_FILENAME} kill, the USERNAME filter
that keeps an elevated machine-wide uninstall out of another logged-on user's session,
and the register save/restore around both. The legacy orca-terminal-daemon.exe kill
stays so hosts left by older builds are still reaped.

The ratchet that was meant to catch exactly this pinned only the legacy image name,
which main's stale macro already satisfied, so it passed both ways. It now asserts the
app-exe kill and the USERNAME filter, against comment-stripped script — the prose above
the macro names both image names, so a toContain over the raw file proves nothing.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
 2026-09-05 21:11:28 -07:00
Neil 8ed81ceb8d perf(tabs): index saved tab order during hydration repair (#18964) 2026-09-05 20:04:11 -07:00
Neil 56626e7daa perf(ssh): reuse and release relay startup buffers (#18953) 
* perf(ssh): reuse the searched relay startup prefix

* perf(ssh): release startup banners after relay readiness
 2026-09-05 20:03:58 -07:00
Neil bf073b833e perf(skills): skip symlink probes beyond discovery depth (#18937) 2026-09-05 20:03:37 -07:00
Neil 6f28e019b5 perf(hooks): use native reverse search for transcript lines (#18936) 2026-09-05 20:03:33 -07:00
Neil 9b76ff9217 perf(explorer): avoid redundant dotfile path filtering (#18929) 2026-09-05 20:03:24 -07:00
Neil fb7b75d55d perf(cli): skip feature formatters during help and error startup (#18923) 
* perf(cli): load error reporting without feature formatters

* test(cli): follow extracted error reporter in import guard

* chore(cli): track cli-error.ts in deferral equivalence baseline

The equivalence script restores TOUCHED files from the baseline rev to
rebuild the pre-deferral CLI. reportCliError/formatCliError moved from
format.ts into cli-error.ts, so the baseline arm must also drop
cli-error.ts (absent at older revs) or the old tree would still compile
against the new module.
 2026-09-05 20:03:19 -07:00
Neil 388e9fb776 perf: avoid rescanning emitted source in analysis guards (#18920) 2026-09-05 20:03:14 -07:00
Neil 4e8e14424d perf: avoid splitting every path during file autocomplete (#18919) 2026-09-05 20:03:08 -07:00
Neil 4204bdf717 perf: avoid repeated Quick Open exclusion string allocations (#18916) 2026-09-05 20:02:57 -07:00
Neil e9d9d42ccc perf: find mobile Markdown placeholder prefixes in one scan (#18914) 
* perf: find mobile Markdown placeholder prefixes in one scan

* style: format mobile Markdown benchmark guard

* docs: explain collision-free Markdown prefix length
 2026-09-05 20:02:53 -07:00
Neil a07d8fe19c perf: avoid file-to-file scans when selecting deletion roots (#18913) 2026-09-05 20:02:48 -07:00
Neil 7b44f3c0e3 perf: decode fragmented CLI replies without repeated scans (#18909) 
* perf: decode fragmented CLI replies without rescanning accumulated text

* bench: require an explicit CLI framing baseline
 2026-09-05 20:02:44 -07:00
Neil 926f3ff585 perf(browser): assemble fragmented tunnel frames once (#18893) 2026-09-05 20:02:30 -07:00
Neil bf87b1290f perf(repos): avoid quadratic icon source scans (#18892) 
* perf(repos): avoid quadratic icon source scans

* perf: avoid repeated malformed HTML icon scans

* bench: balance icon parser timing samples
 2026-09-05 20:02:25 -07:00
Neil 1c41d59203 perf(relay): drain fragmented frame buffers in linear time (#18891) 
* perf(relay): drain fragmented frame buffers in linear time

* style: follow block-body lint in relay buffer checks
 2026-09-05 20:02:20 -07:00
Neil ef3f507903 ci: verify release ref trust and preserve case twins during checkout (#18980) 2026-09-05 18:38:45 -07:00
Neil 6031c19e9f ci: reduce dependency, checkout, and test deadline overhead (#18968) 
* ci: reduce dependency, checkout, and test deadline overhead

* ci: avoid generic E2E jobs for native-only IME changes
 2026-09-05 18:20:12 -07:00
Neil b852aa74a6 fix(ci): store vetted refs in a reftable so case-twin branches don't fail the fetch (#18970) 
The adhoc mac and dev-channel Windows builds vet the requested ref by
mirroring every branch and tag of this repo into a scratch bare repo and
proving the commit is reachable. Both runner disks are case-insensitive,
and the repo now has two branches differing only in casing, so the files
backend refuses the fetch outright — the whole job dies before checkout.

reftable keys refs in a table rather than as file paths, so both refs
store and every ref stays in the reachability set.
 2026-09-05 18:03:01 -07:00
OrcaWinandOrca Worker b6ca8dad99 fix(hooks): register the Claude hook script directly on Windows (#18875) (#18905) 
* fix(hooks): register the Claude hook script directly on Windows (#18875)

The Windows Claude Code lifecycle hook was registered as
`powershell.exe -NoProfile -EncodedCommand <...>` whose entire decoded payload
was a `Test-Path` and a call to `~/.orca/agent-hooks/claude-hook.cmd`. Every
hook event paid a full PowerShell start-up to reach a script that exits at its
first `ORCA_PANE_KEY` guard, so sessions outside Orca paid it to do nothing.

Register the script path itself instead, with `|| echo {}` for the
neutral-JSON-when-missing contract (#14818). Measured on Windows 11, invoked as
Claude Code invokes it (`printf payload | bash -c -l "<command>"`):

  idle (n=12)          baseline 177ms | before 471ms | after 213ms
  10-way conc (n=40)            --    | before 656ms | after 296ms
  p95 under load                --    | before 696ms | after 337ms

It also drops an interpreter from the chain the hook's timeout kill must tear
down. Killing the hook does not kill its PowerShell grandchild, which still
holds the stdout handle the agent reads to EOF -- measured, EOF arrived 352ms
AFTER the kill, when the orphan exited by itself. msys2 creates children
suspended and resumes them after, so a kill landing in that window strands one
that never exits and EOF never comes; that is the reported frozen session.

The encoded launcher stays as the fallback for profile paths the shells cannot
carry bare (space, `%`, `^`, `&`, non-ASCII) and for hosts where Git Bash is not
resolvable, because PowerShell 5.1 rejects `||`. Every other agent's hook is
untouched, as is the remote/SSH path.

Not adopted from the report: `cmd.exe /d /c <path>` (MSYS rewrites the `/c`
under Git Bash -- measured, the invocation fails), and raising the 10s timeout
(the orphan survives the kill regardless; the fast path puts the hook 30x under
the budget so the kill effectively stops firing).

* fix(build): list the new hook launcher modules in the CLI tsconfig project

config/tsconfig.cli.json enumerates its files explicitly, so the two new
imports reached by src/main/claude/hook-settings.ts failed tc:cli with TS6307.
src/main/git-bash.ts pulls in only node:fs, node:path and a shared constant,
so it adds nothing heavy to the CLI project.

* fix(hooks): address review of the direct Windows Claude hook launcher

- Make the Windows hook suites host-independent. A box with a cmd.exe AutoRun
  (HKCU\...\Command Processor\AutoRun) failed them at HEAD too: the tests
  redirect USERPROFILE, the AutoRun target vanishes, and MSYS spawns a .cmd
  without /d so AutoRun runs and lands on the hook's stderr. Seed an empty
  target, including under the deliberately-absent profile.
- Note in managed-hook-stdin-lifecycle why the "missing managed script" case no
  longer exercises the fallback for the direct shape (it carries an absolute
  path, so a redirected profile changes nothing); that path is covered live in
  windows-direct-cmd-hook-command.test.ts.
- Keep the direct shape off UNC profiles: WINDOWS_CMD_SAFE_PATH admits them, but
  //server/share/... is not a command cmd.exe reliably starts.
- Correct the comments: `|| echo {}` also fires when cmd.exe itself exits
  non-zero (failing AutoRun), printing {} twice. The encoded launcher exited 1
  on that same box, so neither shape is clean there.
- Test the contract that replaced runtime %USERPROFILE% resolution (STA-3348): a
  stale absolute path reports not_installed and is rewritten on install.
- Record the standing unmeasured assumption in windows-edr-posture.md: `||` does
  not parse in Windows PowerShell 5.1, so a compat consumer that hosts hook
  strings there would fail closed. Measure before widening to another agent.
- Trim the launcher comments per AGENTS.md; the numbers live in the doc.

* test(win32): register the new Windows-gated hook test in the CI lane

win32-test-lane-registration guards against exactly this: a Windows-gated file
that self-skips on ubuntu and reports success, so it runs on no machine. The new
windows-direct-cmd-hook-command.test.ts needs both entries — WINDOWS_PACKAGE_TESTS
decides whether package_windows runs for a diff, and the workflow argv decides
whether the file runs once that job started.

* test(win32): remove the hook temp tree through the retrying helper

windows-lane-tree-removal-boundary scans exactly the specs in the Windows CI
lane, so registering windows-direct-cmd-hook-command.test.ts subjected it to the
rule: cmd.exe and bash have just exited in that tree, and a raw recursive rm
throws EPERM on Windows while their handles drain, turning a green spec into a
lane failure. Use removeTreeSync, which carries the repo's maxRetries policy.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
 2026-09-05 17:50:33 -07:00
Neil 9f0054d89c ci: skip idle Mac allocations and redundant native compiler setup (#18954) 
* ci: avoid idle Mac allocations and cached native toolchain installs

* test: anchor artifact fixtures before their fixed expiry
 2026-09-05 17:35:20 -07:00
Neil 7bb54cc2f7 ci: reduce runner overhead and disposable package compression (#18948) 
* ci: reduce PR runner overhead and package compression time

* ci: validate mobile when its dependency action changes
 2026-09-05 16:56:57 -07:00
Neil a730becd7a fix(automation): keep explicit background launches off screen (#18898) 2026-09-05 16:12:52 -07:00
Neil d7767fb196 perf(worktree): remove redundant creation and terminal startup work (#18793) 
* perf(worktree): remove redundant creation and terminal startup work

* test(worktree): cover optimized creation call signatures

Preserve explicit branch adoption, WSL callback routing and sparse cleanup expectations.

* perf: preserve user Git checkout worker settings

* perf(git): skip malformed remote base probes

* perf(cli): avoid loading other agent hooks for Codex preflight

* fix(build): retain Codex preflight entry for packaged CLI

* test(ssh): wait for replacement PTY before lease recovery input

* test(ssh): verify recovered shell execution and lease ownership

* test(electron): reap isolated macOS crash reporters on teardown

* test: allow either observed self-exit snapshot ordering

* test: capture frozen-host input recovery evidence
 2026-09-05 15:08:22 -07:00
Neil 1924c8f5b1 feat(perf): lint repeated sort setup and schedule regression contracts (#18822) 
* feat(perf): audit comparator setup and schedule performance contracts

* test(sqlite): close readers after expected busy failures

* ci(perf): trigger contract workflow on the contract files themselves

Without these paths a contract rename lands green on PR CI and only
breaks the next nightly, where nobody owns the failure. Also run the
OS-independent source audit once instead of on all three runners.
 2026-09-05 13:56:06 -07:00
Neil d8c4c83063 test(e2e): fence SSH recovery and release exited Electron pipes (#18880) 2026-09-05 13:53:27 -07:00