mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 08:03:58 +00:00
c8d2408bf2c6741dfcc8fc62b3504df4040839f8
1658
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jinwoo-H
|
fe28b2c18b | fix(native-chat): route SSH transcripts through owner host | ||
 Brennan Benson
|
677718c4a5 |
fix(codex): stop rebuilding shared Codex state from a read that failed (STA-4823) (#15417)
* fix(codex): stop rebuilding shared Codex state from a read that failed (STA-4823) Six shared files were rebuilt, erased or reported healthy after a read that had only failed. Batch A of the STA-4606 split: every one of these is reachable and testable on the host lane, so none of them wait on the WSL work. - `config-toml-trust.ts` upsertHookTrustEntries: `existsSync` reported a locked config.toml as absent, so the base content became '' and the upsert rewrote the file from the trust entries alone — a trust-only stub, with the user's model, provider, MCP servers, approvals and comments gone. It refuses now; every hook-service caller already turns that into "trust entries could not be written. Run /hooks in Codex to approve." - `codex-trust-grant-ledger.ts`: an unreadable ledger degraded to empty and the next write persisted a file holding only the home being written, dropping every other home's grants. The write paths refuse; the read path still degrades, and a corrupt ledger is still rebuilt. - `codex-pane-account-registry.ts`: an unreadable registry erased every pane's attribution AND cached that erasure, so it survived the file recovering. The failure is no longer cached, and both write sites refuse rather than persist a registry derived from an empty stand-in. - `hooks-json-read.ts`: the read arm already separated "no hooks" from "could not read", but the `existsSync` arm in front of it returned a valid empty config for a file that could not be opened. One read now classifies both. - `config-settings-baseline.ts`: absent, unparseable and unreadable all collapsed into `null`, so the snapshot rebuilt a baseline it could not read — recording an in-Codex edit as Orca's own write, after which promotion skips it forever. - `config-sync-stall.ts`: an unreadable runtime config read as absent and the status reported `synced` while the mirror was refusing. It reports `managed-home-unavailable`, the existing reason for exactly this, rather than borrowing a source-side one and blaming the wrong path. Absent and malformed still rebuild throughout — resetting corrupt state is the intent, and conflating it with unreadable would wedge a user on a broken file. * fix(codex): close shared state read-denial gaps * fix(codex): recover oversized settings baselines * fix(codex): name the stalled managed config * fix(codex): preserve hooks after failed source reads * test(codex): correct what the denyExistence rig actually models MEASURED on both platforms: a file-permission denial leaves existsSync TRUE and fails only the content read — chmod 000 gives EACCES on macOS, icacls /deny (R) gives EPERM errno -4048 on Windows, with stat/lstat succeeding in both. The rig's docblock claimed this mode modelled that denial. It does not. What it models is the UNC / \\wsl$ transport, where an unreachable distro reports errno UNKNOWN at every level and existsSync folds it to false. The distinction decides what the D29 guard is worth: under a permission denial the pre-fix code already failed safe, because existsSync was true so it took the read branch and threw. Only a transport that lies about existence reaches the rebuild-from-empty path. No behaviour change; the comment was wrong, not the code. * test(codex): exercise live baseline read denial * fix(codex): retry pane attribution writes * fix(codex): retry reconciliation registry writes * fix(codex): report unreadable sync baselines |
||
 Neil
|
c3a1694b1d |
perf(preflight): read the WSL mount table once, and make launch agree with detection (#16053)
* perf(preflight): read the WSL mount table once per shell, not once per CLI
The prelude is embedded in the lookup script, and the caller wraps that in
`for cmd in <every agent>`, so the unconditional assignment forked awk once per
probed CLI -- 36 of them inside the distro against a 10s detection budget. The
comment claimed it was read once outside the loop; it was not.
`${x+set}` rather than `[ -n ... ]`: a host with no Windows mounts yields the
empty string, which must still count as read.
Pinned by counting real awk forks through /bin/sh with a stub on PATH, because
nothing covered this expression at all -- a wrong-field mutation shipped green.
Verified to bind: the unconditional form counts 4 for 4 commands.
* fix(wsl): make launch resolve the same binary detection reported
Agent detection skips Windows mounts during the PATH walk; the Codex WSL
command builder and the WSL branch of isCommandOnPath did not. So Orca could
report the guest codex as installed and then launch the Windows one sitting
ahead of it on PATH, or disagree with itself between preflight and detection
about the same distro.
Both now pass the same option.
Verified on a real Windows host against a real WSL2 distro, with a Windows
binary planted ahead of a guest one on PATH:
plain `command -v orcaprobe` -> /mnt/c/Users/neil/orca-agree/orcaprobe
this lookup -> /home/neil/.orca-agree/bin/orcaprobe
That host reports /mnt/c as 9p, which the mount expression matches, so the
fstype list is confirmed against hardware rather than fixtures.
* test(preflight): prove the memoised mount list applies past the first command
Counting awk forks with a stub that reports no mounts cannot see what the
hoist trades correctness for. A mutant that empties `_orca_win_mounts` inside
the walk keeps the fork count at 1 and keeps every existing test green, while
every agent after the first stops skipping /mnt.
This runs two commands behind a stubbed Windows mount and asserts both resolve
to the guest binary. Verified against that exact mutant.
Credit: review counsel.
|
||
 NeilandMelih
|
b2902cb61e |
fix(agent-resume): restore Kimi Code sessions after restart (#15883)
Co-authored-by: Melih <mberatsanli@gmail.com> |
||
|
Neil
|
92315c4178 |
fix(preflight): do not count a Windows binary reached through interop as a WSL install (#16028)
* fix(preflight): do not count a Windows binary reached through interop as a WSL install WSL appends the Windows PATH to the guest PATH by default, so on a distro with no guest `claude`, `command -v claude` resolves to `/mnt/c/Users/me/.../claude.exe`. That path is POSIX-absolute, so the existing absolute-path check accepted it and preflight reported the agent as installed in the distro. That is worse than reporting it absent. Absent tells the user to install it; a false positive launches a Windows executable inside a Linux session, where it sees Windows paths, no guest $HOME and none of the distro's config -- and the failure surfaces later, somewhere less obvious. Rejects `/mnt/<drive>/` and any `.exe`, case-insensitively. A genuine guest install is unaffected. * fix(preflight): skip Windows mounts during the PATH walk, not after it The review caught this and it is the more important half of the fix. Rejecting the interop path in TypeScript happens after the guest walk has already stopped on it: the lookup breaks at the first executable, and the version-manager fallback dirs are APPENDED, so they sit behind the Windows entries WSL appends. A user with claude in nvm AND on the Windows PATH therefore went from a false positive to "not installed" -- the exact #9725 population the fallback dirs exist to serve. Worse than the bug being fixed. The lookup now takes `skipWindowsMountDirs` and skips those PATH components mid-walk, so the guest binary behind the shadow is still found. Matched by mount metadata from /proc/mounts (drvfs/9p/virtiofs), not by a `/mnt` name: the automount root is configurable, and `/mnt` is an ordinary directory on a Linux box. That also closes the custom-root hole the reviewers found in the name-based predicate. The TypeScript check stays as a secondary net for a mount the guest does not report, with a comment saying why it must never be the thing that decides. Proven with a real /bin/sh: a Windows `claude` ahead of an nvm `claude` on PATH now resolves to the nvm one. Credit: review counsel, and community PR #12794 (spfcraze), which proposed this shape first. * fix(preflight): let the mount table be the only word on what is a Windows path The name-based check could veto a path the walk had deliberately kept. /mnt/d is a perfectly ordinary Linux mount, so a guest binary there was resolved correctly by the walk and then discarded by its name -- the #9725 false negative, reintroduced by the belt-and-braces net I added "just in case". And if awk were missing, the name rule became the only rule, which is precisely the failure it was supposed to backstop. The walk skips components the guest itself reports as drvfs/9p/virtiofs. That is authoritative. Without a mount table we now degrade to main's behaviour (the old false positive) rather than inventing a new false negative. Net: one predicate, three fixtures and an import deleted. |
||
 Neilandhwantage
|
202d74a8a4 |
fix(git): enable Windows long paths for worktree creation (local, sparse, and SSH hosts) (#15866)
Co-authored-by: hwantage <hwantagexsw2@gmail.com> |
||
|
Neil
|
838f5bfb75 |
fix(secrets): tell Linux users when their secrets are only obfuscated (#16033)
On Linux with no keyring, Electron falls back to the `basic_text` backend, which "encrypts" with a hardcoded password. `isEncryptionAvailable()` returns true for it, so Orca reported those secrets as sealed. They are not. The obvious fix — returning false for basic_text — is wrong and would have been a credential regression: `decryptWithStatus()` skips decryption entirely when encryption is unavailable, so every already-stored secret would read back empty. Sealing genuinely works on basic_text and must keep working. So capability and trust are now separate questions. `isEncryptionAvailable()` still answers "can this host seal and unseal", and `describeProtectionGap()` (renamed from `describeUnavailable`) answers "is my data actually protected", covering both no-sealing and weak-sealing. That method had no production caller — the port documented a promise nothing kept. `reportSecretProtectionGap()` now reads it at startup. A user-visible surface is follow-up; this at least stops the silence. Adds a bootstrap wiring guard over all nine host port installs. The no-op defaults are correct for a renderer-less host and silently wrong for the desktop, and a dropped or reordered install fails no existing test. Verified in both directions: it fails when an install is removed, and when one moves after the runtime is constructed. |
||
|
Neil
|
f975035809 |
refactor(ipc): split preflight and SSH registry out of the ipcMain modules (#15927)
* refactor(preflight): split agent detection out of the ipcMain registration
First of the IPC extractions the revised design requires. `src/main/ipc/preflight.ts`
mixed 285 lines of agent/tool detection with 35 lines of `ipcMain.handle`
registration, and the runtime calls that detection during normal operation
(`orca-runtime.ts:573`, plus the preflight RPC methods). So the runtime dragged
`ipcMain` into its graph to reach pure logic.
Detection moves to `src/main/preflight/agent-detection.ts` — named for what it
contains, per AGENTS.md. `ipc/preflight.ts` keeps only the handler registration and
re-exports the domain module so existing importers are unaffected. The runtime and
its RPC methods now import the domain module directly.
Ratchet baseline 36 → 35: `src/main/ipc/preflight.ts` is no longer reachable from
the runtime. The gate detected the improvement and refused to pass until the
baseline tightened, which is the behaviour it was built for.
Verified: 2 files / 1,187 tests pass across every suite touching preflight;
`pnpm typecheck` clean; `oxlint` clean.
* refactor(ssh): split the SSH target registry out of the ipcMain module
Second IPC extraction, and by far the biggest win: this removes **eight** modules
from the runtime's Electron graph, taking the ratchet baseline 35 → 27.
The runtime needed five thin accessors from `src/main/ipc/ssh.ts` —
`connectRegisteredSshTarget`, `getRegisteredSshState`, `listRegisteredSshTargets`,
`listRegisteredRemovedSshTargetLabels`, `getActiveMultiplexer`. Each is a one-line
read over module-level state. Importing them dragged in `ipcMain`, `powerMonitor`
and a `BrowserWindow` accessor — and, transitively, `ipc/pty.ts` (8,031 lines),
`ssh-browse`, `ssh-passphrase`, `ssh-relay-deploy`, `ssh-remote-cli-host-passthrough`,
`wsl-hook-relay-launch` and `user-data-path`.
`src/main/ssh/ssh-target-registry.ts` now holds that state plus its accessors.
`registerSshHandlers` populates it; the runtime reads it. The indirection is kept
deliberately: SSH providers register after construction and may reconnect, so
callers must resolve the current generation rather than freeze one.
`ipc/ssh.ts` re-exports all five, so non-test importers are unaffected.
`connectRegisteredSshTarget` still throws `ssh_handlers_not_registered` when no
handler layer registered — a headless host must fail loudly rather than report a
target as unreachable, which would read as `exited` (see ssh-execution-boundary.md).
Verified: 9 files / 59 tests across the ssh, automations and trust-preset suites;
orca-runtime.test.ts 1,183 pass; `pnpm typecheck` clean; `oxlint` clean.
* refactor(host): resolve the app root through the port in fork-reachable modules
`parcel-watcher-entry-path.ts` and `session-scanner-service-entry-path.ts` read the
app root via `require('electron').app` inside a try/catch that already returns null
when Electron is absent. They were therefore correct under plain Node at runtime and
only failed the *static* text check — which is real, not pedantic: the comment in
`ports/port-scan-command-client.ts:19` records that the plain-node-entry-guard fails
on that literal text, try/catch or not.
`hasAppEnvironment() ? getAppEnvironment() : null` gives the identical "no app root
here" answer without the text. That restores `hasAppEnvironment`, which an earlier
commit in this stack deleted as unused — it now has the caller it was waiting for.
Ratchet baseline 27 → 25.
Verified: 74 files / 458 tests; `pnpm typecheck` clean; `oxlint` clean.
* test(ssh): mock the SSH target registry alongside the ipc/ssh mock
Thirty-eight suites mocked `vi.mock('./ssh')` for `getActiveMultiplexer`. That
factory went inert when production started importing the accessor from
`../ssh/ssh-target-registry`, so the real module loaded and the assertions drifted.
Adds a companion registry mock returning the same stub, plus a
`sshTargetRegistryModuleMock` builder beside the existing `sshModuleMock` so the
shared harness stays one place. No assertion changed.
Found by a full-suite run: the targeted ssh/runtime suites were green while
30 tests in ipc/worktrees and ipc/repos were not.
* refactor(runtime): read app paths and the packaged flag through the port
`orca-runtime.ts` is the last module in its own graph that imports `electron`
directly. Nineteen of its uses were `app.getPath` (12) and `app.isPackaged` (7) —
exactly what the AppEnvironment port already covers.
Also removes a dead `const { app } = require('electron')` inside
`getOrchestrationDb`. It was left unused once the path came from the port, and it
is precisely the dynamic-require pattern `plain-node-entry-guard.ts` exists to
catch, sitting in the runtime's own constructor path.
What still binds `orca-runtime.ts` to Electron is now three sites, not nineteen:
`new Notification(...)` (one), `BrowserWindow.fromId` (one), and the
`ipcMain.on('terminal:tabCreateReply')` renderer round-trip — which is the browser
tab path, and the same one that would hang a headless host for ten seconds.
Two suites drove `electronMocks.app.isPackaged` directly; they now install a fake
AppEnvironment reading the same mutable field, so their per-test toggles work
unchanged and no assertion moved.
Verified: 376 files / 4,717 tests across src/main/runtime; typecheck and oxlint clean.
* test(serve): add the built-artifact terminal round-trip acceptance smoke
"The server started" proves almost nothing. Terminal creation dispatches into
OrcaRuntimeService, and without an installed headless PTY controller that path
falls through to a renderer reply that never arrives and times out after ten
seconds. A boot probe, a port bind, and a `host.platform` call all pass against a
server whose terminals are dead — which is exactly the gap the design doc's own
boot proof was retracted for.
This boots the BUILT `out/main/index.js --serve`, parses its ready payload, pairs a
real client over the advertised endpoint, lists worktrees, creates a terminal, runs
a command through the PTY, asserts the output comes back, and asserts clean
shutdown. It drives nothing but the public pairing + RPC surface, so the same
script is the acceptance gate a future Node-only backend must pass unchanged.
The sentinel invokes `process.execPath` rather than `echo`, because the shell
differs per platform and node does not.
Verified both directions: passes against the real server, and fails with an
actionable message when the command produces no output — a smoke that cannot fail
is worthless.
* fix(ssh): fail loudly when the multiplexer resolver was never installed
`getActiveMultiplexer` resolves through a resolver that `ipc/ssh.ts` installs at
module scope. A process that never loads the SSH layer — which is the whole point
of the Node-only backend — would get `undefined` from every call.
`undefined` already means something specific here: "not connected". So a missing
resolver and a disconnected target were indistinguishable, and a host with no SSH
layer would quietly report every target as not connected. That is the
unverifiable-reported-as-exited conflation `docs/reference/ssh-execution-boundary.md`
exists to prevent — the doc is explicit that absence of contact is never evidence
of absence of the thing.
A missing resolver is a wiring error, not a connection state, so it throws, matching
what `connectRegisteredSshTarget` already does for unregistered handlers.
Verified: 432 files / 4,759 tests across ipc, ssh, preflight, automations and trust
presets; typecheck and oxlint clean.
* refactor(pty): stop faking a BrowserWindow for the headless PTY path
`registerHeadlessPtyRuntime` passed `registerPtyHandlers` a stub object cast to
`BrowserWindow` whose `isDestroyed()` returned true and whose `webContents.send`
was a no-op — a window-shaped thing that lied about being a window, purely to
satisfy the type. Adversarial review named it as the same "looks fine, silently
returns a lie" pattern this codebase rejects elsewhere, and it is the shape that
keeps `electron` on a path that otherwise needs none.
`registerPtyHandlers` now takes `BrowserWindow | null`. An absent renderer is
semantically identical to a destroyed one — all 42 call sites already guarded on
`isDestroyed()` and skipped — so `src/main/ipc/pty-renderer-surface.ts` states that
directly: `isRendererGone`, `sendToRenderer`, `rendererWebContents`. The compound
`isDestroyed() || webContents.isDestroyed()` guards collapse into one predicate.
`isPtyWriteEventFromMainWindow` becomes null-tolerant and fails closed: with no
renderer no sender can legitimately match, so every write is rejected. Those
handlers cannot fire headless today, but failing closed is the right answer if that
ever changes.
This is the precondition for installing a PTY controller without Electron, which is
what a Node-only backend needs and what `terminal.create` actually calls.
Verified: 129 files / 2,473 tests across ipc/pty, providers and orca-runtime; the
built-artifact acceptance smoke still passes end-to-end (boot → pair →
terminal.create → sentinel → close), which is the check that matters most here
since this changes the headless PTY path itself; typecheck and oxlint clean.
* refactor(pty): read app paths and the packaged flag through the port
Follows the fake-window removal. `ipc/pty.ts` had nine `app.*` reads — all
`getPath`, `getVersion` or `isPackaged` — which the AppEnvironment port already
covers. The `BrowserWindow` import was also dead after the null-window change.
What still binds this file to Electron is now `ipcMain` (75 uses, all handler
registration) and `powerMonitor` (2). That is a clean statement of the remaining
job: split logic from registration, the same shape already applied to preflight
and the SSH registry.
Test wiring: the shared `pty-ipc-suite-environment` beforeEach installs a fake
AppEnvironment that reads through the existing `vi.mock('electron')` app object
rather than freezing values — suites toggle `app.isPackaged` mid-test to exercise
dev-mode spawn paths, so the port has to observe the same mutable field. One edit
in the shared harness covers every pty suite.
Verified: 128 files / 1,290 tests across ipc/pty and providers; the built-artifact
acceptance smoke passes; typecheck and oxlint clean; ratchet unchanged at 25.
* refactor(pty): inject the ipcMain surface so the PTY module loads without Electron
This closes the round-3 blocker: "the doc never says how orcad installs
setPtyController without Electron."
`registerPtyHandlers` owns the `RuntimePtyController` that `terminal.create`
actually spawns through — the thing a Node backend needs and cannot get from the
provider thunks. The module was otherwise host-agnostic already; the only thing
pinning 8,031 lines to Electron was a static `ipcMain` / `powerMonitor` import used
purely to register renderer handlers that no headless host will ever receive.
`src/main/ipc/pty-host-bindings.ts` makes those surfaces settable, defaulting to
no-ops. Unlike AppEnvironment and SecretStore, the default does NOT throw: a host
with no renderer legitimately has nothing to register against, so not registering
handlers nobody can call is correct rather than a hidden downgrade. The desktop
installs the real objects in `attach-main-window-services` before its handlers run.
Also converts the remaining electron import to a top-level `import type`. oxlint's
`no-import-type-side-effects` caught that inline `type` specifiers still leave a
side-effect import — precisely the "type-only is not enough if esbuild still emits
require('electron')" trap a reviewer flagged.
**`src/main/ipc/pty.ts` now bundles with zero `require("electron")`.** A Node entry
can call `registerPtyHandlers(null, runtime, …)` and get a working PTY controller.
Verified: 128 files / 1,290 tests across ipc/pty and providers; the built-artifact
acceptance smoke passes end-to-end — which is the check that matters, since this
changes how every PTY handler registers; typecheck and oxlint clean.
* fix(pty-bindings): drop two unused eslint-disable directives
CI runs oxlint with unused-disable reporting; the two
`@typescript-eslint/no-explicit-any` suppressions I added were never triggered by
any enabled rule, so they failed static analysis as dead directives. The `any[]`
rest args stay — they mirror electron's own IpcMain signature, and narrowing them
would reject the real object at the desktop call site.
Verified with the exact CI invocation: `oxlint --format github` reports 0 warnings,
0 errors across the repo.
* fix(pty): install the host bindings per process, not per window
A real regression my own change introduced, caught by the SSH docker E2E
(`paired-startup-exec-readiness` — "recovers startup exec through a headed paired
desktop owner"). It reproduced on rerun, so it was not a flake.
`setPtyHostBindings` was called inside `attachMainWindowServices`, i.e. when a
window attaches. But `registerHeadlessPtyRuntime` (index.ts:3163) calls
`registerPtyHandlers` on the serve path *before* any window exists — so those
handlers registered against the no-op default and never reached the real `ipcMain`.
A paired desktop owner then attached to a runtime whose PTY handlers were wired to
nothing.
The bindings describe the *host*, not the *window*: an Electron main process always
has `ipcMain`, whether or not a window is open. Installing them beside
`setAppEnvironment`/`setSecretStore` at the top of bootstrap fixes both paths.
Verified: 128 files / 1,290 tests; the built-artifact acceptance smoke passes;
typecheck clean; `oxlint --format github` (the exact CI invocation) reports 0/0.
* feat(orcad): de-electron the runtime core and add the Node entry + build gate
**`src/main/runtime/orca-runtime.ts` — 41,048 lines — no longer imports electron.**
Its last three sites go through `runtime-desktop-surface.ts`: a native notification,
the authoritative-window lookup, and the one `ipcMain` channel used by the
renderer-backed tab-create fallback. All three are unreachable without a renderer —
`createTerminal` already takes the background branch when no window exists (#10333) —
so a Node host installs none and the runtime relays notifications to paired clients,
which is the better destination anyway. Ratchet 25 → 24.
Adds `src/main/orcad/orcad-entry.ts`: Node host adapters plus a `startOrcad` that
constructs the runtime, installs the PTY controller via `registerPtyHandlers(null, …)`,
and serves RPC. It sets two defaults the constructor gets wrong for a headless host —
`canRecoverPersistentLocalPtys: false` (no daemon here) and
`getDesktopWindowStatus: 'blocked'` (a Node host can never be promoted to a desktop
window, which is what `'openable'` claims).
Adds `config/scripts/build-orcad.mjs`, which **currently fails, on purpose**: 25
modules still import electron (browser and speech clusters, plugins, jira/proxy,
filesystem-watcher, and four `require('electron').app` one-liners). It names them.
Two bugs found while building it, both worth recording:
- The first bundle looked clean and was not. `electron` was bundleable, so esbuild
rewrote the metafile `path` to the resolved file under node_modules and a check for
`path === 'electron'` passed while the package was in the bundle — it failed at
runtime with electron's own installer message. The check now reads `original`, and
electron is marked external so a residual import fails loudly instead.
- `jsonc-parser`'s UMD build breaks the bundle at load; aliased to its ESM entry, the
same fix `build-relay.mjs` already carries.
Verified: desktop unchanged — the built-artifact acceptance smoke passes, runtime/pty/
provider suites green, typecheck clean, `oxlint --format github` 0/0.
* refactor(host): drop the last two require('electron') app lookups
`computer/sidecar-client.ts` and `ports/port-scan-command-client.ts` read the app
root through `require('electron').app` inside a try/catch. Both were already correct
under plain Node at runtime — they return null when it throws — but the literal text
fails the plain-Node entry guard regardless, which is why port-scan carried a comment
warning it must never become reachable from a fork entry.
Reading the AppEnvironment port gives the identical "no app root here" answer without
the text, so that warning is now obsolete and the comment says so.
Ratchet 24 → 22. Every remaining entry is a real coupling: the browser cluster (15,
which variant B does not ship), speech (2), plugins (2), and jira/proxy-settings (2,
needing an HttpClient port for Chromium session partitions).
Verified: 25 files / 209 tests; acceptance smoke passes; typecheck and
`oxlint --format github` clean.
* docs(orcad): record that the ratchet under-counts orcad's graph
The ratchet reports 22 electron importers; the orcad build reports 23. The extra is
agent-hooks/wsl-hook-relay-launch.ts, and the cause is a gap in the gate rather than
a rounding error: the ratchet measures what orca-runtime + runtime-rpc reach, while
orcad's entry also imports ipc/pty directly to install the PTY controller.
Once orcad ships it must become a ratchet entry point, or the two numbers drift and
the gate quietly stops covering the artifact it exists for.
* refactor(runtime): inject the browser commands factory
Drops 14 modules from the runtime's Electron graph in one change — the whole Chromium
browser cluster. Ratchet 22 → 8.
`OrcaRuntimeService` constructed `RuntimeBrowserCommands` as a field initializer, and
that construction is what pulled in `BrowserWindow`, `session`, `webContents` and the
cookie jars. Importing the class for its *type* is free; only building it costs.
So the class import becomes `import type`, and the instance comes from
`runtime-browser-commands-factory.ts`. The desktop installs the real factory at the
Electron entry. **All ~80 existing `this.browserCommands.*.bind(...)` delegations are
untouched** — a review round specifically warned that rewriting those was the
expensive, risky part, and this avoids it entirely.
With no factory installed, browser commands reject per call with `browser_unavailable`
rather than resolving to a stub that silently succeeds. The runtime already filters
browser capabilities out of `getStatus()` when no backend exists, so clients do not
offer the affordance in the first place.
Also corrects a stale comment in `pty-renderer-surface.ts` that still described the
fake window as present tense; it was deleted two commits ago.
Verified: 451 files / 5,513 tests across `src/main/browser` and `src/main/runtime` —
the entire browser automation suite; the built-artifact acceptance smoke passes;
`pnpm typecheck` and `oxlint --format github` clean.
* refactor(host): extract the plugin client list and port two app lookups
Ratchet 8 → 5.
- `listPluginsForClients` moves to `src/main/plugins/plugin-client-list.ts`. It needed
only three `plugins/*` helpers, none of them Electron — it was colocated with
`ipcMain.handle` registrations, so the runtime's `plugins.list` RPC dragged all of
Electron in to call a function that reads a lockfile. Same shape as preflight.
Dropping it also releases `ipc/plugin-marketplaces.ts`.
- `agent-hooks/wsl-hook-relay-launch.ts` and `speech/stt-service.ts` read `getAppPath`
and `isPackaged` through the AppEnvironment port.
The five that remain are all genuinely Chromium and need the HttpClient port or a
watcher split, not another mechanical swap: `browser/cdp-bridge` (webContents),
`ipc/filesystem-watcher` (ipcMain), `jira/authenticated-request` and
`network/proxy-settings` (net + session partitions), `speech/model-manager`
(`net.request`, which honors app proxy settings that Node https does not — replacing
it is a behaviour change, not a rename).
Verified: 219 files / 1,922 tests across plugins, speech, agent-hooks and the runtime
RPC methods; the built-artifact acceptance smoke passes; typecheck and
`oxlint --format github` clean.
* refactor(network): resolve the default proxy session lazily
Ratchet 5 → 4.
`proxy-settings.ts` needed exactly one Electron value: `session.defaultSession`, as
the fallback when a caller does not pass `options.proxySession`. Callers could already
inject a session; only the default was hard-wired. It now comes from a settable
resolver, so the module loads under plain Node.
**A resolver rather than a Session, because a Session eagerly throws.** The first
attempt installed `session.defaultSession` directly in pre-ready bootstrap and broke
startup outright — `TypeError: Session can only be received when app is ready`. The
acceptance smoke caught it before commit. Deferring to first use is always after ready.
Behaviour with no session is not a degradation: there is no Chromium proxy config to
discover, so `resolveProxy` is skipped and the environment variables become the whole
answer rather than a fallback. Applying rules to a session that does not exist is
likewise skipped; settings are still honoured because outbound requests read the env.
This reaches past Jira — a review round noted `ensureElectronProxyFromEnvironment` is
also on the Claude HTTP path via `oauth-refresh.ts` and `rate-limits/claude-fetcher.ts`.
Verified: 48 files / 526 tests across network, jira and rate-limits; the
built-artifact acceptance smoke passes; typecheck and `oxlint --format github` clean.
* fix(index): merge the duplicate proxy-settings import
CI's code-quality lint (`oxlint --config config/oxlint-code-quality-native-plugins.json
--deny-warnings`) flags a module imported twice in one file. My earlier insertion added
a second `./network/proxy-settings` import beside the existing one.
Verified with CI's exact invocation: exit 0.
* refactor(network): add the HttpClient port and lift BrowserError out of cdp-bridge
Ratchet 4 → 2.
Two unrelated couplings, both of the same shape — a small thing living inside a
Chromium-heavy file.
`BrowserError` is a seven-line error class with no dependencies, but it lived in
`browser/cdp-bridge.ts`, which imports `webContents`. The runtime catches that type on
paths with nothing to do with CDP, so one import kept a Node host from loading the
runtime at all. Moved to `browser/browser-error.ts`; cdp-bridge re-exports it.
`jira/authenticated-request.ts` fetches through `net.fetch` and reads
`session.defaultSession`. `network/http-client.ts` makes both settable. This one is a
**named port rather than a silent fallback, because the fallback is not transparent**:
Electron's net follows Chromium session/proxy state, avoids undici's stale keep-alive
sockets after a VPN path change, and sends a Chrome user agent that Jira's XSRF check
depends on. A Node host gets `globalThis.fetch`, reads proxy config from the
environment, and sends Node's user agent. That difference is documented at the port.
`session.defaultSession` is read per call, not captured at install — it throws before
the app is ready, which is the mistake the previous commit made and the acceptance
smoke caught.
Test wiring: `jira/client.test.ts` installs the port *inside* `loadClientModule`, after
its `vi.resetModules()`, since the reset gives the module a fresh singleton.
Verified: 461 files / 5,616 tests across jira, browser, network and runtime; the
built-artifact acceptance smoke passes; typecheck, `oxlint --format github` and the
code-quality lint with `--deny-warnings` all clean.
* fix(http-client): register the Node fetch fallback with the call-site audit
`global-fetch-call-site-audit.test.ts` guards every global-fetch use, because the
global runs on undici where an unread response body can crash the whole process
(orca#8695). The HttpClient port's Node fallback is a new such call site and was
unregistered — the guard caught it in a full-suite run.
Registered with the reasoning, and the port's doc comment now states the body-safety
contract explicitly: it hands the Response straight to its caller and never inspects
it, so the consume/cancel obligation stays exactly where it already was — with the
caller, unchanged from when they called Electron's net directly.
Two comments elsewhere mentioned the global by name and tripped the line scan as false
positives; reworded to describe the behaviour rather than name the API.
Verified: audit passes; typecheck and `oxlint --format github` clean.
* fix(app-environment): read hasAppEnvironment through the realm slot
|
||
|
NeilandMelih
|
7ce11dcf55 |
fix(agent-resume): restore Copilot sessions after restart (#15879)
Co-authored-by: Melih <mberatsanli@gmail.com> |
||
|
Neil
|
0bbc6c80e8 |
refactor(host): route app paths and version through an AppEnvironment port (#16019)
* refactor(host): route app paths and version through an AppEnvironment port
`app.getPath('userData')` is the single largest Electron coupling in the main
process — 37 call sites — and it is one of the things stopping the Orca runtime
from booting on plain Node. Give it the same treatment as SecretStore.
- `src/shared/app-environment.ts` — the port plus a settable registry, covering
the members the runtime's module graph actually reads: paths, app path,
version, packaged flag, shutdown hook, exit, and Chromium process metrics.
`getAppEnvironment()` throws until installed, for the same reason the secret
store does: a silent default resolves `userData` to the wrong directory and the
caller writes real state there before anyone notices. No `node:` imports,
because `src/shared/**` is in the web build graph.
- `src/main/host/electron-app-environment.ts` — the desktop adapter, a
pass-through to `electron.app`.
- 9 modules migrated: telemetry, opencode/mimo/pi hook services,
terminal-history-paths, terminal-scrollback-snapshots, cli-installer,
clipboard-image-temp-file, memory/collector.
Deliberately NOT migrated: `src/main/browser/**`. That cluster is Chromium-
adjacent by nature — cookie jars, download destinations, offscreen pages — and a
Node backend does not ship it at all, so porting it buys nothing and churns
heavily-mocked suites. Also left alone for now: the call sites that additionally
touch `app.asar` path literals or `app.setName`, which need more than a
mechanical swap.
`getAppMetrics` stays on the port rather than being injected because
memory/collector.ts is its only caller and reads it from module scope; a Node
host returns [], having no Chromium processes to measure.
Test wiring: the secret-store setup file becomes `vitest-host-ports-setup.ts` and
installs both ports, exporting `fakeAppEnvironment`/`installFakeAppEnvironment`
so suites needing one specific member state only that instead of restating all
seven — which is boilerplate, and had pushed one suite past the max-lines budget.
Verified: 159 files / 1651 tests pass across every touched area; `tsc` clean on
both the node and web projects; `oxlint` clean.
* fix(typecheck): list the vitest host-ports setup in the node project
Three suites import `installFakeAppEnvironment` from config/scripts, but that
directory is outside tsconfig.node.json's include list, so composite typecheck
failed with TS6307. Listing the one file matches how this config already pins
individual files it needs.
Local `tsc --composite false` does not reproduce this — only `pnpm typecheck`
does, which is what CI runs.
* refactor(host): drop two unused AppEnvironment exports
hasAppEnvironment() and resetAppEnvironmentForTests() had zero callers. The
secret-store equivalents are used, so these were mirror-symmetry rather than
need; add them back when something actually needs them.
* test(terminal-history): install the AppEnvironment fake instead of mocking electron
These three suites mocked `electron.app.getPath` to point at a fixture dir. The
production module now reads the port, so the mock was inert and the global test
default's temp dir won — which broke the WSL path assertions and every deletion
count.
Found by a full-suite run, not by the targeted checks around the migrated modules,
which is the argument for running the whole suite on a refactor this wide.
* test(host-ports): remove the per-environment temp dir on teardown
The setup allocated a mkdtemp directory at module scope, which vitest evaluates
once per test *environment* — one per test file, not one per worker. Nothing
removed them, so a full 6,000-file run left thousands behind.
Proven: with an isolated TMPDIR, a three-file run previously added directories and
now leaves zero.
* fix(app-environment): anchor the installed environment to a realm global
Same reason as the SecretStore: vi.resetModules() rebuilds the module registry,
and an environment installed before the reset read back as uninstalled.
|
||
|
Neil
|
e9e238c883 |
refactor(wsl): delete the environment-policy layer the reviews kept failing on (#16007)
* refactor(wsl): delete the environment-policy layer the reviews kept failing on
A design council (Opus, Grok, GPT-5.6-Sol) reviewed the merged runner after it
took eleven review rounds to land. All three reached the same conclusion: the
invocation half is sound, the environment/probe half is not, and every round had
been debugging the second one.
The finding that settled it, from Opus: `environmentResolved` had **54
references, all in tests and the runner itself. Not one production reader.** The
safety mechanism the strict default existed for was never wired to anything, so
all 19 degrading sites reported absence with full confidence anyway -- #9725
live at every one, under comments claiming it was handled. Two of those comments
say so out loud; I wrote them.
Root cause, in one line: every knob existed only because a failed probe was
fatal. So it no longer is.
- `allowDegradedEnvironment` and `WslGuestEnvironmentUnavailableError` are gone.
A missing login PATH is a fact in the result, not an exception. That deletes
23 opt-outs, six catch-and-remap blocks, the transient/rejected cooldown
split, `probedWithBudget`, and the 1.5x re-probe heuristic -- none of which
had a reason to exist once the case stopped throwing.
- `lane` + `allowDegradedEnvironment` collapse into `loginPath: 'none' |
'preferred'`. 19 of 23 sites passed the opt-out, and two said in comments that
they did not want the login PATH at all: the flag had become the `'none'` the
union was missing.
- The `interactive` lane is deleted. It had zero production callers and kept ~30
lines of fence plumbing alive for tests only.
Net -98 production lines; the runner itself sheds 86 for 38.
Also carries three fixes from the W3 orphan-PR sweep I had not done:
- `WSL_UTF8=1` in the runner. My relay migration deleted the only place setting
it, so wsl.exe's own error text arrived UTF-16LE and read as NUL-riddled.
A regression I introduced. Credit: #9010 (Chang-Jin-Lee).
- `GITLAB_HOST` is now named in WSLENV, so a ported self-hosted host actually
crosses into a distro-routed glab (#12557). Credit: #12558 (makoto-developer).
- The WSL skill-setup command pipes into `sh` instead of `eval "$(...)"`, whose
nested quoting produced `word unexpected (expecting "in")` (#14292). Credit:
#14785 (innocarpe).
* fix(wsl): restore the login PATH for the Codex availability lookup
loginPath:'none' on a PATH lookup reports an nvm-installed codex as absent,
which is #9725. A miss without a resolved environment is now 'could not
check', not 'not installed'.
Also hardens the guards that should have caught it:
- bashism ratchet is per-call, not per-file, and fails closed on lexer desync
- blankStringContents handles regex literals (an apostrophe in /'/g desynced
the lexer, so the scan silently found zero calls)
- windowsHide allowlist 85 -> 80, stale once the lexer parsed those files
Credit: Grok (P0), GPT-Sol (ratchet gaps).
* test(wsl): close the two ratchet gaps that let planted spawns pass
- variable-indirected wsl.exe (`const b = 'wsl.exe'; spawnProcess(b)`) is now
tracked, so the 5 files recorded only in a comment become real allowlist
entries. Three actually spawn that way; the other two never spawned wsl.exe
at all, so the prose record was wrong by three in the hiding direction.
- promisify(renamedAlias) is now resolved, so `const run = promisify(execFile)`
behind an `execFile as x` import can no longer skip windowsHide.
Each verified by planting the violation, watching it fail, restoring, watching
it pass. Credit: GPT-Sol.
* fix(source-scan): stop the regex-literal reader from eating block comments
At index 0 there is no preceding token, so a file opening with a banner
comment had its `/*` read as a pattern and swallowed to the next slash --
110k characters of preload/index.ts, in the direction that hides offenders.
Measured across the tree, old lexer vs new: worst-case over-blanking drops
from -110564 to -1116 characters, and files that desync drop from 51 to 22.
The remaining extra blanking is regex interiors, which is the intent.
Regression tests for both lexer bugs, each verified to fail with its fix
reverted. The first draft of the comment test did not bind -- it asserted on
text after the swallowed span.
* fix(wsl): restore the unverifiable signal on the two remaining probe sites
Round 2. Three call sites used to throw when the login-PATH probe failed;
the redesign rewired one (Codex) and left two reporting confident absence.
- skill-wsl-provider-detection: the script ends in `|| true`, so a lookup
without the login PATH exits 0 with empty stdout -- identical to 'nothing
installed'. Callers skip the ~/.codex and ~/.claude skill roots on an empty
list, losing an nvm-installed provider's skills.
- wsl-cli-installer: the dead catch is replaced by an explicit check. Its
`case ":$PATH:"` probe otherwise answers from the distro default PATH and
Settings states as fact that the CLI is not on PATH. Timeout is checked
first, since a timed-out run also leaves the environment unresolved.
Also narrows the regex-literal prev-token set. '!', '+', '-', '>' and '}' are
value terminators as often as operators, so postfix `n-- / 2` and JSX
`<A size={14} /> : <B` were read as patterns and their spans blanked -- 13
live JSX spans, and one swallowed execFile call that left no desync behind.
False negatives only risk a desync, and desync fails closed.
Plus: WSL_UTF8 on the probe spawn (#9010 reached the runner, not the probe),
and the allowlist header I shuffled by sorting comments along with entries.
Credit: Grok (both P1s), Opus (lexer false positives).
* docs(wsl): drop the lane comments the redesign made false
The interactive lane is gone, so 'both lanes' and the fenced-stdout note
described code that no longer exists. Also states plainly that
environmentResolved is always true under loginPath:'none' -- the field cannot
rescue a PATH lookup that was mislabelled, which is how #9725 came back.
Credit: Grok.
* fix(wsl): stop piping user scripts into the shell's stdin
The W3 migration moved hooks from `wsl.exe --exec bash -c <script>` to a
script piped into `bash -s`. Anything the script runs that reads stdin then
drains the rest of the script, bash hits EOF and exits 0, and the caller logs
success -- an orca.yaml hook of `ssh -T git@github.com || true` followed by
`pnpm install` silently never installs.
Scripts now travel in argv by default, which is what the pre-migration code
did and what --exec makes safe. `scriptDelivery: 'stdin'` stays for the one
caller that needs it: the hook-relay installer embeds a base64 JS bundle far
past any command-line limit, and reads no stdin.
A runner test already described this exact EOF hazard -- for the login shell,
not for the guest command it was itself creating.
Credit: code review.
* fix(skills): make the unverifiable check unconditional, and stop double-probing
Round 3.
- provider detection threw only on an EMPTY result, so a degraded partial hit
slipped through: `claude` visible on the default PATH via Windows interop
plus an nvm-only `codex` returns a plausible ['claude'], and the caller then
skips the ~/.codex skill roots for a provider that is installed. The
installer already got this right with an unconditional throw.
- three sites asked for 'preferred' without needing it. The GROK_HOME probe
runs its own `"$login_shell" -lc`, so the runner's probe was a second login
shell eating up to half an 8s budget; the two skill scans are
find/base64/head/printf/stat over $HOME.
- the indirection binder missed `private readonly x = 'wsl.exe'` (the
modifier was captured as the name), backtick literals, and
`spawnProcess(this.x)`. Commit
|
||
|
Neil
|
d07ce15cff | refactor(host): route secret storage through a SecretStore port (#15916) | ||
 Jinjing
|
02bee48e1d |
Retry transient ripgrep spawn failures instead of demanding install (#15983)
* retry transient ripgrep spawn failures instead of missing binary errors Fork/exec pressure (EAGAIN, EMFILE, ENFILE, ENOMEM, ETXTBSY) should not trigger ripgrep-not-found guidance. Add bounded retries (max 2x) for transient spawn failures in Quick Open and file listing, respecting cancellation signals. Introduce RipgrepLaunchFailureError to distinguish fork/exec pressure from unavailable ripgrep installations. * Handle cancellation during transient spawn failure retry window When a query is cancelled after a transient ripgrep spawn failure but before the retry decision resumes, the cancellation must be reported to the caller rather than proceeding with a retry attempt. |
||
|
NeilandJinjing
|
6e18c18e58 |
fix(automations): localize schedule weekday names and labels (#15884)
* fix(automations): localize schedule weekday names and labels
The Weekly Day picker rendered a hardcoded English tuple, and shared
schedule labels built copy as `${day}s at ${time}` from an OS-locale
Intl weekday, so a non-English UI showed Sunday…Saturday (or 星期五s).
Shared now emits deterministic English (the CLI contract) plus a
locale-free AutomationScheduleDescriptor; the renderer formats that
descriptor through translate() with Intl/CLDR weekday names resolved
from getIntlLocale(). Fixes #14404.
* test(automations): assert localized weekday copy in rendered DOM
The existing coverage walked the React element tree, so nothing proved the
Day dropdown and cron status row reach the DOM localized. Mount the picker
under happy-dom with the Radix Select swapped for a native <select> (the
pattern RepositoryWorktreeDefaultsSection.test.tsx already uses, since Radix
portals its content only once opened) and read real option text.
Also key the weekday SelectItems by index rather than by translated copy, so
a runtime language switch reconciles instead of remounting all seven items.
* fix(automations): keep the weekday SelectItem key off the array index
react-doctor(no-array-index-as-key) rejects `key={index}`; the localized
weekday name is already unique per locale, so keep it as the key.
* fix(automations): match the real AutomationDraft shape in the render test
The fixture invented `repoId`/`branchMode`/`enabled` fields; runtime ignored
them but `tsc` did not. Mirror AutomationSchedulePicker.test.ts's fixture.
* fix(automations): localize the custom-cron field chips
The five cron field headers rendered one row above the status row this PR
localizes were still hardcoded English, so a Chinese UI showed
Minute/Hour/Day/Month/Weekday. Same defect shape as the deleted DAY_OPTIONS
array. Chip keys move to stable field ids so a locale that renders two fields
with the same word cannot collide, and the truncated header carries a title so
longer copy (es 'Dia de la semana') stays readable.
* fix(automations): keep weekday option keys stable
---------
Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
|
||
|
Neil
|
5651662494 |
fix(wsl): migrate 21 call sites onto the WSL runner (#15923)
* fix(wsl): migrate 21 call sites onto the runner, after five review rounds Rebased onto main now that the runner (#15903) has landed. 21 sites across 15 files move off ad-hoc `execFile('wsl.exe', ...)`. Allowlist 23 -> 16 on the WSL guard; 163 -> 152 on the W1 child_process guard, which moved as a consequence. Five review rounds, each finding real defects -- several introduced by the previous round's fixes: 1. Hooks ran user orca.yaml scripts under dash; probe failure fell back to the login shell, reintroducing the ~/.profile stall the runner exists to remove. 2. An unparseable probe was cached permanently, disabling every WSL feature on the distro; hooks regressed from "runs degraded" to "fails". 3. Exit 127 had no expiry; a starved 5s probe hard-failed the 10s scan behind it; a joiner burned its budget on someone else's probe. 4. The comment stripper blanked live code, so the windowsHide guard walked past a real unguarded spawn and reported the file clean; an ownership-probe timeout silently deselected the user's Claude account. 5. Verification of the guards themselves. The recurring finding -- a call answering "is this installed?" on a degraded PATH -- was eventually fixed structurally rather than per-caller: the runner refuses an unresolved guest PATH unless the caller opts in. Per-site vigilance was demonstrably not holding; 3 of 8 sites had already forgotten the analogous exit-code check. Remaining 16 files need a runner mode that does not exist: a long-lived streaming child (OAuth logins, hook relay), a synchronous caller, or a host-level flag like --status that the guest-command API cannot express. * fix(wsl): close round 5's P1s -- degrade where PATH was never needed Round 5 measured the guards by re-executing their algorithms standalone rather than reading them, and found four things. P1 -- four skill/plugin paths gained a hard dependency on the login-shell probe that they never had. They ran under a plain non-login `sh -c` on main, so a probe failure now breaks WSL skill discovery and install on exactly the distro the runner was built for: one with a slow `~/.profile`. Worse, the throw escapes before each site's own error mapping, so the UI gets a raw internal string. They degrade now, per the rule this branch already wrote down in `wsl-fish-history-cleanup.ts`. P1 -- Codex and Claude were asymmetric. Claude's five credential sites degrade; Codex's were strict, so adding a WSL Codex account failed where adding a Claude one succeeded. Three of the four are byte-equivalent to Claude sites, and their scripts read `$HOME`/`$WSL_DISTRO_NAME`, which wsl.exe supplies without a login shell. `assertWslCodexCliAvailable` stays strict on purpose -- that one really does answer "is this installed?" (#9725). P1 -- the ownership-probe timeout fix did not survive the rebase onto main. A timeout still returned "not owned", which the caller *persists*, clearing the user's account selection. P1 -- `blankStringContents` desynced on a nested template literal (`` `${`x`}` ``), leaving 116 lines of a child_process importer outside the ratchet, with 27 importers structurally at risk. Now tracks template depth. Regenerating against the fixed blanker: 70 -> 68 offenders. Also: the windowsHide vacuity check could not fail while the allowlist alone exceeded its bound -- the exact defect the sibling guard documents avoiding. It now names a file that definitely offends. * fix(wsl): close round 6 -- my blanker fix had traded a false positive for a miss Round 6 re-derived the guard's answer from a TypeScript AST instead of trusting the regex, and caught two things. P1 -- the nested-template fix I shipped in round 5 introduced a worse bug than the one it closed. Switching to "code mode" inside `${...}` without also resetting the quote at a newline meant an apostrophe in a regex literal -- `` `'${value.replace(/'/g, "'\\''")}'` `` , which is exactly the shellQuote shape all over this codebase -- inverted the lexer for the rest of the file. `claude-accounts/service.ts` went blind from line 96, hiding a REAL unguarded `spawn` at :1097: the WSL Claude managed-login path, which opens a console and steals foreground on Windows. Round 5 traded one false positive for one false negative and I did not notice, because the offender count went down. The blanker now resets non-backtick quotes at a newline (the rule stripComments already had) and tracks brace depth per interpolation. The spawn is fixed rather than allowlisted, and the count is 69 -- the number the AST predicted. P1 -- the ownership-timeout guard was dead code: it threw into its own `catch` three lines below, which returned null, which the caller persists as "not owned" and clears the user's account selection. Now a typed sentinel the catch rethrows. P2 -- `WslGuestEnvironmentUnavailableError` reached the UI verbatim from the CLI installer and the Codex availability check. Both mapped. Method note: I had been regenerating the allowlist with a Python transcription of the scanner, and the two drifted -- the same two-implementations problem this workstream keeps finding. The allowlist is now generated by running the shipped test with an empty list and taking what it reports. * fix(guards): stop patching the lexer -- make the scanner fail closed instead Round 7 proved my round-6 fix also did not work, by planting a plainly-named unguarded `spawn` in `claude-accounts/service.ts` and watching the guard pass 3/3. That is three consecutive attempts at an exact lexer, each shipping a desync that hid real calls, and each time the offender count went DOWN, which I read as progress. Round 6's diagnosis was wrong too: the culprit is the `templates` brace-depth stack, which nothing resets, not quote state. So stop trying to be exact. `blankStringContentsDesynced` reports when the lexer lost its bearings, and the guard treats that as an offender. Over-reporting is a nuisance; under-reporting is a false clean, and a false clean is what let a real console-flash spawn out of the ratchet twice. The allowlist goes 69 -> 82: the 13 extra are files whose scan cannot be trusted, now named rather than assumed fine. The planted violation is now caught. Also from round 7: - `SPAWN_CALL` missed promisified and renamed bindings, so `exec('where gemini')` (a real Windows cmd.exe spawn) and a detached `shell: true` in `cli/runtime/launch.ts` were invisible. Added execAsync/execFileAsync/ execFileCb/spawnDetached. - `BASHISM` matched `set -o pipefail` but not `set -euo pipefail`, which is the only spelling this tree uses -- so the check could not have caught the #14292 signature it exists for. Fixed, and it immediately flagged a file; that one turned out to be a comment, so the bashism scan now strips comments too. - The CLI installer error mapping my round-6 commit claimed was "both mapped" was never applied -- only the Codex side had been. Now actually mapped. * fix(guards): close the four holes round 8 found by planting violations Round 8 stopped reasoning about the guard and planted spawns into it. Four holes, none of which reading had found: - `windowsHide: false` **passed**. The check was `args.includes('windowsHide')`, a substring test. Now matches `windowsHide: true`. - A ternary first argument was silently skipped: the method-declaration filter `/^\(\s*\w+\s*[:?]/` also matches `exec(useAlt ? 'a' : 'b', …)`. Now requires a type after the colon. - Renamed bindings were not covered, despite the comment I wrote saying they were -- I had hardcoded three names. Aliases are now resolved from the import. Each is verified closed by planting it and watching the guard fail. `fork` is deliberately still unscanned. Round 8 is right that Node forwards the option, but `ForkOptions` does not declare it, so the two live sites cannot be fixed without a cast. Recorded in the verification doc rather than left as a silent gap, along with two others worth knowing: the allowlist is file-granular, so its ~18 false-positive entries carry a standing pre-approval for real regressions in those files and cannot be retired by fixing code; and `stripComments` has no desync report, so the fail-closed check is only half applied. The doc now also says how to verify a guard change: plant a violation. Every guard fix here that was verified by reading was wrong. * fix(wsl): stop preflight reporting installed CLIs as absent on a slow distro Round 9's merge blocker, and the sharpest finding of the whole workstream: the branch built to close #9725 had reopened it from the other side. `preflight-wsl-command.ts` was one of five sites without `allowDegradedEnvironment`, so a guest-PATH probe failure threw. Every consumer collapses a throw into a verdict: `isCommandAvailable` and `isCommandOnPath` catch to `false` ("not installed"), `isGhAuthenticated` and `isGlabAuthenticated` read an empty payload as "not authenticated". So a slow distro made WSL git, gh and glab read as missing. Two things made it likely rather than theoretical. The probe took two thirds of a 5s budget, leaving the command ~1667ms where main gave it the full 5s inside its own login shell -- a cold WSL VM start routinely lands in that band. And a probe timeout is cached for 30s with a re-probe threshold of 1.5x the failed budget, which a 5s caller can never clear, so every preflight command short-circuited without spawning wsl.exe at all -- and Re-check does not invalidate the cache. Fixes: preflight degrades instead of refusing, and the probe is capped at half the caller's budget and at 4s, so no caller ends up with less time than it had before the runner existed. Also fixes a real console flash found on the way: `preflight-command-exec.ts` spawns git/gh/node through `promisify(execFile)` with no `windowsHide`. Round 9 also confirmed the credential paths are now *safer* than main: all 11 account sites degrade, every destructive guest operation is still marker-gated, and main's `getOwnedManagedAuthPath` could disown an account on a 5s timeout -- which this branch turns into a failed launch instead of a destroyed selection. * fix(wsl): make "Try again" able to succeed, and test the round-9 fix Round 10 returned MERGE with one residual worth closing first. A transient probe failure left the null-resolving promise in `inFlight`, so the only way back was `retryAfter` -- and the 4s probe cap made the 1.5x budget escape unreachable, because no caller can pass more than 4s. For the full 30s window the four non-degrading sites returned their error *without spawning wsl.exe at all*, and each of those errors says "Try again". The advice was guaranteed to fail. The entry is now dropped on a transient outcome and an explicit cooldown gate replaces it, so the window alone decides. The window drops 30s -> 5s: long enough to stop a stampede, short enough that the user's next click reaches a distro that has since warmed up. Round 10 also noted the round-9 fix shipped untested, which was fair. Added: the probe-budget floor for 5s/8s/10s callers, and preflight's degrade opt-in plus its stdout/stderr-carrying rejection, which isGhAuthenticated reads off the caught error as an auth-success fallback. * test(wsl): make the probe-budget guard actually guard Round 11 caught that the regression test I added for the probe cap did not bind: it seeded the guest environment, so the probe resolved in ~0ms and the assertion read the command leg's timeout instead. Reverting the cap to the old 2/3 split left all three cases green. Dropping the seed and asserting on the probe leg fixes it -- verified by reverting the cap and watching all three fail. A regression guard that cannot fail is the shape that has cost the most in this workstream: the windowsHide guard silently passed a real unguarded spawn twice for the same reason. |
||
 NeilandOrcaWin
|
98c03fe12f |
fix(win32): hide the console window for agent-browser and git helpers (#15887)
* fix(win32): hide the console window for agent-browser and git helpers W1 routed most child processes through `runProcess`, which always sets `windowsHide`. Six call sites still spawn directly, so each one opens a real console window on Windows: it flashes and steals foreground. For the git status poll, that is once per poll (#10488). A ratchet now scans every file that imports `child_process` and fails on a call without the flag. Its allowlist starts at the 76 files that still offend and can only shrink — it doubles as the worklist for routing them through the chokepoint, which is where the flag stops being a per-call-site decision at all. Diagnosed in #14589; the SSH and cookie-import sites it also covered are already fixed on main by the W1 migration. Co-authored-by: OrcaWin <orcawin@users.noreply.github.com> * test(wsl): stop the exec-mode guard scanning historical release checkouts The cross-version e2e lane checks whole past releases out under `tests/e2e/.cross-version-checkouts/`. The guard walked into them, so on any machine that had run that lane it reported 21 offenders -- every one a copy of shipped code we cannot edit -- and failed. Skip dot-directories; the >500-file vacuity assertion still holds. --------- Co-authored-by: OrcaWin <orcawin@users.noreply.github.com> |
||
 Jinwoo Hong
|
1354ff534f | fix(cmd-j): host-qualify browser and simulator tab candidates (STA-4965) (#15686) | ||
|
Neil
|
057fbfcffc |
perf(windows): read the process table natively instead of forking PowerShell (#15749)
* perf(windows): read the process table natively instead of forking PowerShell Seven independent readers each forked powershell.exe to run Get-CimInstance Win32_Process, with a wmic fallback that Windows 11 24H2 has removed. On a domain-joined host with PowerShell Transcription enabled by policy, one of them running every ~2s recorded ~289GB across 1.4 million files (#15209). The same scan cost ~700ms and ran per pane (#15036), and a Group Policy or AV block turned it into 'unavailable', which callers read as 'no evidence' -- which is how a PTY tree survives its own teardown (#9045, #10475). A Toolhelp32 snapshot answers the same question with no child process. Measured on Windows 11 with 1050 processes, p50/p95: pid+ppid+name 15.9 / 17.5 ms +memory +command line 30.6 / 33.7 ms Get-CimInstance 706 / 723 ms Two upstream defects needed patching, both found by running it on real hardware. The binding requires Spectre-mitigated libraries our agents do not carry (node-pty is patched the same way). And enumeration stopped after 1024 processes: on a host with 1051 the module returned exactly 1024, and the querying process was itself among the 27 missing -- a truncated snapshot silently hides the descendants teardown is looking for, which is the failure this whole change exists to remove. Migrated: the foreground/descendant reader (the #15209 scraper and the teardown identity gate) and the port scanner's PID attribution. NOT migrated: the memory collector and three identity probes, which need Win32_Process.CreationDate and have no native equivalent. Start time is a proxy for identity anyway; an inherited job handle is the real answer, so those belong with the job-object work rather than here. Packaging follows the windows-native-registry contract exactly: optional, absent from onlyBuiltDependencies so macOS/Linux never run node-gyp, win32-only in the packaged runtime. Asserted by the existing contract test, which also stops pinning a whole source literal that only tested its own formatting. * chore(process): ratchet the child_process allowlist down windows-foreground-process-rows.ts no longer spawns anything, so its allowlist line is stale. The guard fails on a stale entry as well as a new one, precisely so a migrated file cannot keep a slot open and hide the next regression in the same path. * fix(ports): import the process-table reader the scanner uses Missing import: the migration replaced the PowerShell call but the new symbol was never imported, so tsc failed. Vitest transpiles without typechecking, which is why the port-scanner suite stayed green. * fix(deps): sync this branch's lockfile with its patch set Same class as the fix on the tip branch: pnpm records a hash per patched dependency, and this branch introduces the windows-process-tree patch without its lockfile entry matching. Every job here failed at install with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. Verified with --frozen-lockfile, which is what CI runs and what my local runs were not. * test(relay): drive the relay's Windows fixtures from the native snapshot Two relay cases fed a PowerShell CIM payload through a mocked execFile. That reader is gone, so both failed -- deterministically, on every PR run for this branch and the one above it. I did not catch it because my own verification sweep was 'src/main src/shared config/scripts' and never included src/relay. The relay is a first-class consumer of the process table; leaving it out of the sweep is how a deterministic failure survived six review rounds. |
||
 Denis Darii
|
d5667376b0 |
feat(dashboard): add a keyboard shortcut to toggle the Agent Dashboard (#15353)
Adds a configurable, unbound-by-default `dashboard.toggle` action that toggles the Agent Dashboard (in-window drawer or pop-out, per the existing mode setting). - Wired through window-shortcut-policy, main-window dispatch, browser-guest dispatch, preload, and the renderer IPC handler. - Opening the in-window drawer reveals the sidebar first; closing leaves it alone. - Gated on the `experimentalAgentDashboardPopout` experiment, and the Settings shortcut row is hidden while that experiment is off. |
||
|
Neil
|
b7e79b7ca6 |
fix(windows): one chokepoint for every child process (#15746)
* feat(process): add the Windows-correct child-process chokepoint Six decisions have to be made every time Orca starts a child process -- console visibility, argument quoting, .cmd interpretation, binary resolution, timeout policy, and how the tree is later terminated. POSIX forgives all six. Windows punishes each differently, and made per-call site across 172 files they were right in some and wrong in others. runProcess/spawnProcess make them once: - windowsHide unconditionally, shell:false unconditionally (shell:true concatenates argv unescaped and silently disables windowsHide) - .cmd/.bat routed through cmd.exe /d /v:off /s /c with a verbatim line, because Node refuses to spawn them otherwise (EINVAL) The encoding was derived by measurement on Windows 11, not from the docs. An embedded quote is written "" rather than \" so cmd's naive quote count stays even -- with \" the parity flips and every later & | < > on the line stops being data. Measured before the fix, argv ["a b", 'c"d', "e%F%g", "h&i", "j^k"] arrived as ["a b", 'c"d', "e^%F^%g", "h"]: the & truncated the argument and ran its remainder as a command. Each % is broken out of the quoted run as "^%" because %VAR% expands even inside quotes. The import-boundary test is a ratchet seeded at today's 172 files; it only shrinks. * fix(process): route the console-flashing spawn sites through the chokepoint The ssh -G config probe fires on every connect and reconnect, and ssh.exe is console-subsystem, so a GUI-subsystem parent gets a fresh visible conhost that takes foreground -- keystrokes typed into an Orca terminal at that moment go into the black box (#10488, #14543). Same for the ProxyJump tunnel, the ProxyCommand cmd.exe wrapper, the font enumeration and the DPAPI cookie decrypt. Also stops spawning powershell by bare name: PATH under Electron is not the user's, so where policy has pruned the System32 entry the spawn fails and the font picker silently reports five hardcoded families rather than an error (#11771). Deletes system-fonts' 40-line bespoke execFileText -- timeout, output cap and kill are the chokepoint's job now. Adds runProcessSync so the sync callers have a compliant path; without one the ratchet could never reach zero. The three suites that mocked child_process directly now mock runProcess, which is the point: how a process gets started is no longer each module's business. Ratchet 173 -> 170. * fix(process): do not report a deliberately killed child as timed out runProcessSync inferred a timeout from signal === 'SIGTERM'. Measured: a real timeout sets error.code ETIMEDOUT and kills with SIGTERM, but so does anything else that terminates the child -- and those cases set no error at all. Reading the signal alone reports a process someone stopped on purpose as having timed out, which callers retry. * refactor(process): hold the ratchet as data and migrate the pwsh probes The allowlist and the adversarial argument corpus are read only by tests, so they were production modules in name only; they move to __fixtures__. pwsh.ts carried isTimeoutError() purely to reconcile two spellings of the same event -- execFileSync reports a timeout as ETIMEDOUT, the execFile callback as a SIGTERM kill with no code. runProcess reports one timedOut flag, so the helper and the reasoning behind it both go. Its sync probe also spawned without windowsHide, which flashes a console and steals foreground on every cold cache read. * refactor(process): migrate five more spawn sites onto the chokepoint Each one deletes a hand-rolled promise/timeout/kill wrapper and stops re-deciding console visibility for itself. Ratchet 170 -> 164. Two things this surfaced, both kept: runProcess now accepts string chunks as well as buffers. A stream someone called setEncoding on emits strings, and concatenating those as buffers throws inside a data handler -- where the rejection has nowhere to go and the caller simply hangs rather than failing. ProcessSpec keeps its AbortSignal. I had removed it as unused; the macOS PAM preflight passes one through from its own caller. ipc/app.ts is deliberately NOT migrated. Its probe spawns a three-stage pipeline detached so a timeout can reap the group with one negative-pid SIGKILL; runProcess kills only the root, which would orphan the plutil stages. Migrating it needs the chokepoint to own POSIX process-group termination first -- the same guarantee job objects give on Windows. Reverted and left on the ratchet. * test(process): do not assert a POSIX signal on Windows Windows has no signals, so the same deliberate kill reports an exit code there and a signal on POSIX. What has to hold on both is that neither shape reads as a timeout. Caught by running the suite on Windows. (cherry picked from commit 0a6e9902a22a369a0e85e113ea8d87b726f82e1f) * fix(process): settle a timed-out run even when the child ignores the kill close only fires once the child is actually gone, so a child that traps SIGTERM never emits it and the promise outlives its own deadline forever. That is the same wedge shape just fixed for the process table, and it is worse here: pwsh.ts and the snapshot reader both cache an in-flight probe, so one unkillable child hands every later caller the same dead promise. After the deadline it now escalates to SIGKILL and settles regardless, reporting timedOut with whatever output arrived. (cherry picked from commit 78ac169197c4e6faee1b9310a7186029cc11acbc) * fix(process): escalate an aborted child too, not just a timed-out one The grace escalation I added covered the timeout path and left abort on the old one, so an aborted caller with an unkillable child still waited forever -- the same defect, one path over. The macOS PAM preflight is a real caller that passes an AbortSignal. Both paths now share one stop-and-settle, and the result reports timedOut honestly: false when the caller aborted. (cherry picked from commit 7e9523a9e31172bb8183661b56f04c3ab6a03d0d) * fix(windows): stop percent escaping from forging an escaped quote escapePercentForCmd ran as a post-pass over the quoted string, so it inserted a quote wherever a percent was -- including straight after a backslash. CommandLineToArgvW reads backslash-quote as an escaped quote, so C:\Users\%USERNAME%\x arrived corrupted. That is about as common as Windows paths get, and my 20-case corpus had no backslash-before-percent entry to catch it. Percent handling is now part of the quoting loop, where the backslash run is known and can be doubled before the inserted quote. Two corpus cases cover the shape. The program path gets the same treatment. It was quoted but not percent-escaped, so a launcher under C:\Users\%USERNAME%\ had its own path expanded on the cmd hop. quoteWindowsArgument no longer takes a boolean. Passing it to values.map() handed map's index in as the flag -- which is how the first version of this fix was written, and the corpus test caught it. Separately: an AbortSignal that was already aborted never fires the event, so runProcess ran the child to its full timeout for a caller who had already given up. (cherry picked from commit f7e2e56b1ee1f27ab6d1035dde4501b38f95b374) |
||
|
Neil
|
080c95940f |
fix(ssh): let fork-PR worktrees add their contributor remote via the relay (#15827)
* fix(ssh): let fork-PR worktrees add their contributor remote via the relay Creating a workspace from a fork PR on an SSH host failed with "Destructive git remote operations are not allowed via exec". The relay's git.exec allowlist blocked every `remote` write subcommand, but SSH fork-PR creation has to run `git remote add <fork> <url>` on the host before it can fetch and track the contributor's branch, so the whole create aborted. Allow exactly the two shapes that flow needs -- `remote add <name> <url>` and `remote remove <name>` -- validated with the same remote-name and URL rules the relay already applies to every pushTarget-carrying RPC. Everything else (set-url, rename, prune, extra operands, flags before the action) stays blocked, and the URL must be a github.com clone/ssh URL, so no new reach is granted beyond what push/fetch already accept. `remote remove` was blocked too, which silently leaked fork remotes on SSH hosts: worktree removal swallows the cleanup error. It works again now. A host still running an older relay gets an actionable "reconnect to deploy the latest relay" message instead of the raw policy error. * test(git-exec): pin remote read/write mutation classification Misclassifying `git remote` / `remote get-url` as mutating would flush the relay and SSH provider git read caches on every remote probe, so pin both directions. |
||
|
Brennan Benson
|
3fca1d1648 |
fix(linear): unbound list-issues by default, surface truncation, bind cursor workspace (#15824)
Fixes STA-5076. list-issues capped at 50 by default and hard-clamped at 250, with hasMore buried under result.meta and no stderr warning for --json, so a page that stopped early read as a complete answer. Omitting --limit now walks Linear's pages until they run out (meta.limit is null), and --limit <n> is the only cap, paging past Linear's 250-per-request maximum to reach it. result.truncated sits next to result.issues and is set only when a cap actually held results back; human output prints "truncated: showing N". The read still has to fit the CLI's 60s RPC budget, so a 20s wall-clock deadline and a 200-page ceiling stop the walk early and report truncated with a continuation cursor rather than failing the command. Also: - issued --cursor values bind the resolved workspace, so call -> nextCursor -> call works without --workspace; raw Linear cursors still need one and now carry nextSteps - issued cursors whose payload smuggles back `all` or an empty workspace are rejected at decode, since either would widen the read past the bound workspace - JSON issue rows carry priorityLabel (none/urgent/high/medium/low), matching orca linear priority set - truncated and priorityLabel are optional on the wire, so a host that predates either is not read as "complete"; readers fall back to meta.hasMore - the truncation line prints the rows actually rendered, so a remote result with no meta.returned cannot print "showing undefined" |
||
 OrcaWinandBrennan Benson
|
2a68b78bb3 |
fix(worktree): let a configured worktree base outrank a built-in visibility source (#15232) (#15430)
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> |
||
|
OrcaWin
|
2c24025e1d | fix(worktrees): honor absolute Linux worktree base paths for WSL repos (STA-4772) (#15384) | ||
|
JinjingandJinwoo-H
|
6e25a90085 |
fix(terminal): keep a quick command queued until its own spawn takes it (#15630)
* Increase shell readiness timeout to match daemon barrier
Slow interactive rc files can take longer than 1.5s to initialize. Raise
the startup command readiness timeout from 1.5s to 15s to match the daemon
barrier and prevent queued commands from executing mid-startup.
* fix(terminal): keep a quick command queued until its own spawn takes it (STA-4876)
Triggering a quick command opened a terminal tab titled with the command's
label, the shell started and drew its prompt, and the command never ran.
TerminalPane snapshots `pendingStartupByTabId[tabId]` in a useState lazy
initializer, and a mount effect deleted the entry immediately. The pane's key
is `${tab.id}-${tab.generation ?? 0}`, so anything that bumps generation before
the command reaches a shell — the stall-recovery remount fired from
`requestTerminalPaneRecovery`, or the allDead activation regeneration — mounted
a second pane that re-read an emptied slot and spawned with no command at all.
The loss was permanent, which is why every scope failed alike: repo, global and
agent-prompt all funnel through the same queue-then-snapshot sequence.
Spend the entry at `onPtySpawn` instead, which is the one point that proves this
pane's own fresh spawn exists. A pane retired mid-connect never reaches it, so
the command stays queued for the next mount; reattach skips `onPtySpawn`, so it
cannot spend a command it never delivers.
Three details are load-bearing:
- Ownership is reference identity (`paneOwnsQueuedStartup`). Setup and issue
splits borrow the same `deps.startup` field for their own one-shot payload, and
that payload can be structurally identical to the queued command, so a
truthiness test would let a split pane spend a command it never runs.
- The consume runs after `bindActivePanePty`. While the tab still has no ptyId,
the queued entry is the only thing holding its worktree out of the
retention-budget force-park, so dropping it first unmounts the pane mid-spawn.
- The callback is one-shot. `onPtySpawn` fires on every fresh spawn a pane makes,
including hibernation wake and the respawn ladder, and a command queued after
the first launch belongs to that later launch.
Known residual, documented at the call site: the consume tracks "a pty exists",
not "the command ran". Windows embeds short commands in the shell argv, so they
execute before the spawn resolves and a pane retired in that window re-delivers
on remount; on POSIX the write waits for shell-ready, so a pty that dies in that
window loses a command already spent. Closing either needs a delivery signal
from main rather than this callback. Both windows are narrow, and both are
strictly better than losing the command unconditionally.
* fix(terminal): guard the queued-startup wiring the review found untested
Follow-ups from the final review pass on this branch.
- Collapse the ownership + one-shot decision into `createQueuedStartupConsumer`
so the call site is a single call rather than inline logic no test could
reach. Two mutants survived the whole suite before this: relaxing ownership to
a truthiness check, and dropping the one-shot guard. Both now fail.
- Rewrite the throwing-consume test. It asserted `updateTabPtyId` had been
called, which runs *before* the callback, so it passed with the try/catch
deleted. It now asserts the throw does not escape into the connect promise,
which is the invariant the try/catch actually provides.
- Correct the `onQueuedStartupSpawned` docblock. It claimed the callback is "the
first moment the command is guaranteed to reach a shell"; the diff's own caveat
says otherwise, since Windows runs an argv-embedded command before this fires
and a POSIX shell can die before the shell-ready write. It marks a live shell,
not delivery.
No behavior change: the consumer is the same predicate and the same one-shot,
moved behind one exported seam.
* fix(terminal): roll shell wrapper isolation into a fresh daemon
* Revert "Increase shell readiness timeout to match daemon barrier"
This reverts commit
|
||
|
Jinjing
|
d8e9fa1bb9 |
Revert "fix(terminal): apply pane padding on all four edges (#15544)" (#15623)
This reverts commit
|
||
 NeilandBrennan
|
c92f394cde |
fix(pty): delete the reply-withholding scheduler (#15578)
* fix(pty): answer a terminal colour query in its own turn Root-cause follow-up to #15559, which stopped a CPR overtaking a deferred colour reply but left the deferral itself in place. Orca answers terminal queries by writing to the PTY master, which a line discipline in ECHO copies straight back out as junk on a cooked prompt (#12112). The guard was to withhold the write until an `stty` subprocess proved ECHO clear — and forking is what forced the decision to be async. Any deferral, however short, lets a reply written later in the same turn overtake this one, so the async probe was the bug's root cause. Read the bit synchronously instead. Linux and the BSDs redirect a master's mode ioctls to the slave, so a `tcgetattr` on the master fd node-pty already owns answers for the slave with no fork: measured 0.26us against 2403us for the subprocess. With a verdict available inline, a querying program that already cleared ECHO — every raw-mode prober, including the colour probe behind the `gh auth login` report — is answered in its own turn and can never be reordered. The deferral stays for the genuinely cooked case, and the ordering guarantee stays underneath it: hosts whose node-pty predates this patch get no sync probe and fall back to the deferred path, which mixed client/host versions make a live production path. Reply routing is all-or-nothing: a payload needing neither containment nor ordering stays on the host's own path, so a CPR answered during shell startup cannot pass the daemon's post-ready flush gate and splice into the buffered startup command. Native side is fail-safe: a kernel that did not redirect would answer from the master's own termios, whose ECHO defaults set, so the degraded verdict is "echoing" — never a false "quiet". The JS half ships in the pnpm patch while the binding needs a source build, so ORCA_REQUIRE_NODE_PTY_ECHO_STATE=1 makes CI fail rather than silently skip when it is handed an upstream prebuild. Co-authored-by: Brennan <brennanb2025@users.noreply.github.com> * fix(pty): keep the flush ordered under synchronous re-entry Three defects found in external review of the reply-ordering work. node-pty delivers onData inside the master write, so a query can be answered while the queue is mid-flush. `flushPendingWrites` spliced the array off before writing, so that reply saw an empty queue, took the same-turn path, and landed ahead of entries the loop had not written yet — reproduced as 01, 99, 02, 03. It now shifts one entry at a time so a re-entrant reply queues behind the rest, bounded by the length at entry so a re-entrant push cannot spin the loop. An overflow flush can re-enter as far as teardown. `answer` did not re-check `closed` afterwards, so it queued behind a closed delivery, returned true, and the reply was never written and never reported. The payload router's ownership comment overstated its guarantee. The `any` semantics are deliberate — returning false after a constituent was already written would have the caller re-write the whole payload and duplicate it into the child's stdin — so the residual mixed-failure drop is now documented rather than implied away. * fix(pty): delete the reply-withholding scheduler Orca answered a terminal query by withholding the write until a probe proved the slave's ECHO bit was clear. That was the wrong mechanism, and it is now gone: replies are written in the caller's turn and their echo is contained on the output side, where it always was. Withholding never removed an echo. The wait was bounded and always ended in a write, so the output-side projections were doing the work the whole time — including the readline rewrite, which happens with the tty already raw and which therefore no reading of the ECHO bit can predict. What withholding did add was an asynchronous write path, and that is what let one reply overtake another and land in the next program's stdin (#15559), what produced a re-entrancy inversion inside its own flush, and what four rounds of regressions have lived in. The last thing it covered was the verbatim echo of a `stty -echoctl` tty. That shape is now projected directly. It starts with ESC, so it is matched only when complete and never held as a partial: holding it would take a bare trailing ESC from the query parser and an expired hold would release it raw, so a query torn at its own ESC would never be answered. Complete-match-only is what makes the shape safe to project at all. Measured on a real pty: a cooked-mode master write is both echoed AND delivered — ECHO copies the bytes without consuming them from the slave's input queue, so a program arming raw mode with TCSANOW/TCSADRAIN (libuv's setRawMode, hence every Node agent) still reads them. Only a TCSAFLUSH switcher discards it, which it does on every terminal, none of which gates a reply on termios state. Deletes the pending-write queue, the async stty probe, the poll budget and probe rate limit, the deadline-driven flush, and the answer/ answerInOrder split. Replies now leave in call order by construction. No packaging, native or CI surface is touched. * test(pty): restore stty-probe coverage and pin the duplicate-query retry Archaeology on how withholding got here, and what its tests were really protecting. Deleting the ECHO probe took four tests with it that were not about the probe at all: they cover createSttyProbe, which the shell-readiness line-editor probe still uses — in-flight sharing, the per-platform stty flag, and transient-versus-permanent failure latching. Restored against the line-editor probe, which is now their only caller. Also pins the property that answers the one case an immediate write cannot serve. A program that queries while cooked and then arms raw mode with TCSAFLUSH discards the reply with the rest of its input queue. Nothing can prevent that from the terminal side, and no terminal tries. What matters is that such a program re-queries after its own timeout: the ingress declines to answer an already-answered slot but forwards the duplicate downstream, so the renderer's emulator answers the retry, by which point the program is raw. The retry path is the recovery, not withholding. * ci(pty): keep the fish real-PTY test in the shell-contracts lane only Reverting pr.yml to main dropped the exclusion for the fish query-reply test, which this branch keeps, so it would have run in the sharded lane as well. Restores it to the shell-contracts include list and the shard exclude list, and drops the parallelism expectations for the deleted cooked-querier suite and the echo-state env guard. --------- Co-authored-by: Brennan <brennanb2025@users.noreply.github.com> |
||
|
Brennan Benson
|
4b2ed5ddd4 |
fix(terminal): apply pane padding on all four edges (#15544)
* fix(terminal): apply pane padding on all four edges Move the configured inset onto xterm so the terminal fills its pane while the fit calculation accounts for both sides of each axis. Add a geometry golden that forces cell remainders and verifies dynamic padding without relying on renderer pixels. * fix(terminal): normalize imported padding for fitting * fix(terminal): align stored and fitted padding |
||
|
OrcaWin
|
471bc9d8ce | Ship the WSL transcript helper with the Windows relay (STA-4831) (#15529) | ||
|
Neil
|
fdd4091ebd | fix(hooks): isolate lint-staged backups per worktree (#15388) | ||
|
Neil
|
d7a23c84a9 |
fix(pty): keep a CPR reply from overtaking a deferred colour reply (#15559)
A background-colour probe writes `OSC 11 ;? ST` then `CSI 6n` and reads exactly one response, using the CPR as its sentinel: a non-OSC first response means "unsupported" and it stops draining. #13309 routed live cooked-echo-risk replies through the ECHO-probe deferral while CPR kept the immediate path, so the CPR overtook the colour reply, the prober gave up, and the stray `ESC ]` was left in the tty for the next program — `gh auth login` died on it with an escape-sequence error. Queue a reply that needs no echo containment behind ones that do, FIFO, and only while something is actually deferred, so latency-critical replies stay immediate on every other path. Windows is unaffected: only posix-pty defers, so the queue is always empty there. Also: an in-flight echo probe is already the write continuation, so re-arming the timer for a queued reply would fork a second stty and throw away the first verdict; and teardown now hands queued uncontained writes to the pty best-effort instead of dropping bytes the caller was told were sent. Known scope limit, pinned by tests and tracked for follow-up: the guarantee is FIFO among recognised query replies, not over every byte — a reply coalesced with a keystroke, and ordinary typed input, still bypass the queue. Both were unordered before this change too. |
||
|
Jinjing
|
d541982b9c |
Improve cmd j search usefulness (#15551)
* Show last active time for workspace tabs in cmd+j palette Adds session-age formatting and activity tracking to help users find recently-used tabs. Replaces host badge display with last-active timestamps that reflect either agent activity or worktree PTY activity, whichever is more recent. * Improve cmd+j search ranking with direct fields and recency Prioritize results matching direct fields (titles, content) over container fields (worktree, branch, repo). Use tab focus time to break ranking ties. Makes search more useful for quick navigation. * Extract path flavor logic to cross-platform-path utility - Remove local pathFlavor function in favor of shared cross-platform utilities - Simplify buildExcludePathPrefixes to use relativePathInsideRoot and resolveRuntimePath - Ensures consistent path handling for both local and remote roots * Improve cmd+j search ranking with recency-based tiebreaking Track lastFocusedAt on tab creation/focus and use it to break ties between equally-ranked search results. This surfaces recently-used items first, improving search utility. Also fixes hasDirectHit to check field matches directly rather than evidence metadata. |
||
|
Brennan Benson
|
30bf2647fc |
fix(mobile): replay a delivery-ambiguous worktree.create instead of failing it (#15472)
* fix(mobile): replay a delivery-ambiguous worktree.create instead of failing it A socket close or response timeout rejects an in-flight worktree.create as delivery-unknown: the frame reached the wire, so the host may already have built the worktree. The client only replayed connection-migration cutovers, so every other ambiguity surfaced as a create failure for a create that may well have succeeded. Replay on the same clientMutationId — which the host already dedupes — after waiting for the transport to come back. * fix(mobile): bound the ambiguous worktree.create replay by the host's dedupe window The replay was bounded only by a retry count, but what makes a replay reconcile instead of building a second worktree is wall clock: the host drops a settled create's dedupe record 60s after it resolves, and past that the replay is just a fresh create that the host's suffix loop happily duplicates — for a folder workspace, into a second workspace with the very same name and no collision check at all. Two paths ran past that window: - The request-timeout path. A silently dropped response frame leaves the socket alive, so nothing rejects until WORKTREE_CREATE_TIMEOUT_MS — ten minutes, with no bound at all on when the host actually resolved. This was previously the path that replayed *soonest*, short-circuiting the reconnect wait because the transport still looked healthy. Invert it: every path that reports a real drop has already left 'connected' by the time the rejection surfaces, so still being 'connected' identifies the timeout and is now refused. - The reported-drop path. Worst-case detection is a full liveness idle period plus the missed-probe budget before the client even learns the socket is dead, and the old 20s wait on top of that overran the record. Derive the wait from the watchdog constants and the TTL instead of hardcoding it, and anchor a single deadline at the first ambiguity so a second wait gets the remainder rather than restarting. The TTL now has one definition shared by both processes, so the client asserts its budget against the host's real window instead of a copied literal. * fix(mobile): end the reconnect wait on a revoked pairing, and pin the wait's behavior waitForRpcClientReconnected resolves only on 'connected' or the timeout, but an 'auth-failed' client never reaches 'connected' — so a create interrupted by a revoked pairing sat out the full wait before surfacing the error it already had. Treat auth-failed as a terminal answer on both the fast path and the listener. The helper also shipped with no tests of its own: its already-connected fast path, its timeout path, and the synchronous-notification-during-subscribe teardown were only ever exercised indirectly through the retry suite, and neither RpcClient implementation notifies synchronously, so that branch had no coverage at all. Add a direct suite covering all of them, asserting listener and timer teardown rather than just the resolved value. Also give the fake-timer tests an explicit timeout. advanceTimersByTimeAsync yields through real macrotasks between ticks while vitest's own budget runs on real time, so on a loaded runner the default 5s is reachable — observed once as a spurious timeout in this suite. * fix(mobile): bound the ambiguous replay in wall clock, not timer time The replay window was derived from the liveness watchdog's own budget (idle + missed probes x probe timeout). That is a bound on how long the watchdog takes to *fire*, not on how much wall clock passed. iOS and Android suspend JS timers while the app is backgrounded, so across a background cycle the socket dies silently and the pending create rejects delivery-unknown minutes later with the timer-derived ceiling still reading ~44s. The replay then lands well past the host's 60s dedupe record and the suffix loop builds a SECOND worktree - for a folder workspace, one with the very same name and no collision check at all. Anchor the deadline on the watchdog's lastInboundAt instead: a wall-clock stamp of a frame that really arrived, so it stays honest across a suspension. Fall back to the send time when the transport can't vouch for one (relay sessions run with idleProbeMs: null), which errs toward refusing the replay. Also restore the delivery-unknown discrimination test that the still-connected guard had made vacuous, pin the still-connected guard itself against a live inbound stamp, and pin the deadline against being re-read from a fresher replacement session. |
||
|
Neil
|
36d78e88af |
fix(agent-hooks): stop Antigravity's Windows hook from spawning PowerShell on every event (#15520)
Antigravity was the last agent posting hook status through Windows PowerShell 5.1. Every
hook event — roughly one every 2-6s during an active session — paid a ~300ms interpreter
cold start, which is what made the console the agent allocates for each hook last long
enough to be seen as continuous flashing.
Move the Windows POST to the shared curl.exe builder every other agent already uses, via
the `extraFormLines` escape hatch for the `hook_event_name` field Antigravity uniquely
needs. Measured on Windows 11: 326ms -> 134ms per event.
Because the curl line percent-expands its arguments, the script also needs
`setlocal DisableDelayedExpansion` (#9358/#9941) so a `!` in a pane key or worktree path
is not eaten as a delayed reference.
curl omits a `--data-urlencode name@-` field entirely when stdin is empty, so accept an
absent or blank Antigravity payload as `{}` at the ingest boundary — the POSIX script
substitutes `{}` before posting and PowerShell did the same, and without this a
payload-less event lost the status transition its `hook_event_name` still carried. Scoped
to that source; every other agent keeps rejecting a body it cannot parse.
Adds a cross-agent guard asserting the invariant the original drift violated: a managed
Windows .cmd hook posts through fully-qualified curl.exe and spawns no interpreter.
Generated under a mocked win32 platform so the POSIX CI legs guard it too.
Validated on a real Windows 11 host, not an emulated platform check.
Fixes #15117
|
||
|
Neil
|
cb95582cea | feat(release): build unsigned Windows artifacts for the dev channels (#15465) | ||
|
Jinwoo Hong
|
0e5b348414 | fix(repos): keep the desktop-owned manual project order authoritative across paired clients and SSH catalog publishes (STA-4850) (#15538) | ||
|
Neil
|
a61b39a9a6 |
fix(runtime): stamp a runtime's own project setups as local, and report remote status about the remote (STA-4792) (#15376)
* fix(runtime): stamp a runtime's own project setups as local, and report remote status about the remote (STA-4792) Two independent frame-of-reference bugs, both from code describing one machine while labelled as another. #15366 — projectHostSetup.* persisted the caller's host id verbatim. Those `runtime:<environment-id>` ids are minted by the calling client's own pairing store, so they name a machine only relative to that client. A client sending one is addressing this runtime, and runtimes do not proxy these calls onward, so the host it names is us. Storing the client's spelling made one machine look like a different host to every other client, hid its rows from them, and defeated the (projectId, hostId) duplicate check — two laptops paired to one server each created their own setup for the same checkout. Re-spell it as `local` at the RPC boundary. Rows written earlier keep their old stamp; readers already project `local` back to `runtime:<their-id>`, so the client-visible model is unchanged and no ids are rewritten. STA-4792 defect 4 — `status --environment <name>` hardcoded app.running:false to mean "no desktop on THIS machine" while every other field in the same object described the target, including a desktopWindowStatus echoed straight from it. The result contradicted itself and read as "that run was headless" when the remote GUI was up. `app` now describes the target, keyed off the one window status that requires a live renderer, and the result names its own subject so the frame can't be misread again. The remote pid is not knowable, so it stays null. STA-4792 defect 2 gets a regression test rather than a fix: routing already made the client remote, which is what stops a Windows destination being joined to the local cwd. The test pins the exact reported invocation. * fix(status): share the remote app projection with the SSH host passthrough, and name the version gap on project host setup Two review follow-ups. The SSH host passthrough answered `app.running: true` unconditionally for the Orca host a caller reached over SSH, claiming a desktop app even for a headless `serve`. That is the same defect as the paired-server path, one transport over, so the projection moved to shared and both now answer the question the same way. `--host runtime:<id>` routes project commands to a paired server, which means a client can reach a server that predates project host setup without meaning to. That answered a raw `method_not_found`, which reads as an Orca bug rather than a version gap; the CLI now names it the way the desktop already does. Reverted a third change: making the persistence duplicate check treat `local` and `runtime:*` as one machine. That assumption holds at the RPC boundary, where a `runtime:` host means the runtime being addressed, but not in the store, which also records independent provisioning metadata for machines that are not itself. An existing test covers exactly that, and it was right. The duplicate convergence therefore stays bounded to rows written after the normalization. |
||
|
Brennan Benson
|
6415511a82 |
feat(remote): add a positional file read to the filesystem provider (#15517)
* feat(remote): add a positional file read to the filesystem provider Following a growing remote file means re-reading it from the top on every poll: the relay exposes only whole-file reads, so tailing an append-only log over SSH costs O(size) per tick. Adds fs.readFileRange plus a rangedReadVersion capability, and an optional readFileRange on IFilesystemProvider -- matching how lstat/ supportsQuickOpenSearch already declare degradable capabilities. Three deliberate choices: - The relay loops until the requested length is satisfied or the file truly ends, and REJECTS an over-cap request rather than clamping it. A clamped read is indistinguishable from EOF, so a caller advancing a cursor by bytesRead would silently skip data. - Bytes cross the wire base64-encoded. A range boundary can split a UTF-8 sequence at either edge, and a utf-8 round trip would substitute U+FFFD and shift every subsequent offset. - The provider throws a typed FileRangeReadUnsupportedError against an older relay instead of quietly falling back to a whole-file read. A tailing caller issues several reads per snapshot, so a per-call fallback is quadratic; callers probe supportsFileRangeRead once and snapshot instead. The response is validated before use -- a byte count disagreeing with the payload would shift every downstream offset while looking like success. Terminal-artifact reads/writes move to their own module, mirroring the relay's existing fs-handler-terminal-artifact split; the provider was at the max-lines ceiling and this was the cohesive piece to extract. * fix(remote): size the ranged read to what the relay writer can deliver The 4 MiB cap was justified against MAX_MESSAGE_SIZE (16 MiB), but that is the frame DECODER bound. Responses are gated by the writer's admission budget: a frame over DISPATCHER_CONTROL_QUEUE_MAX_BYTES (1 MiB) is demoted to the legacy-response lane, which is refused once the producer queue passes 2 MiB. A 4 MiB window is ~5.46 MiB of base64, so it was never admissible -- it came back as an opaque ResponseOverCapacity (-33008), which is neither of the PR's typed errors, and above ~1.4 MiB the outcome depended on unrelated queued traffic. Cap at STREAM_CHUNK_SIZE (256 KiB), the house per-frame budget for file bytes, which stays in the control lane unconditionally. Also: - Hoist the cap and offset validation into src/shared/file-range-read.ts so the client rejects an out-of-contract request locally instead of paying a round trip for an error that does not survive the wire as a type. - Validate filePath in the relay handler; a missing one threw a TypeError out of expandTilde despite the comment claiming hand-validated params. - Collapse the two fs.getCapabilities probes onto one cached fetch per multiplexer. They read one document, so probing per feature spent an extra round trip per connection and duplicated the eviction logic. - Reuse readFullStreamChunk instead of a second copy of the short-read fill. - allocUnsafe the window; only subarray(0, bytesRead) escapes, so a tailing poll no longer memsets the whole window per call. - Plain methods for readFileRange/supportsFileRangeRead rather than constructor-assigned arrows; both are unconditional, unlike downloadFolder. Tests: cover the dispatch path and fs.getCapabilities (neither was exercised), param validation at both boundaries, EOF at and past the end, and a full-cap read over a real RelayDispatcher. The transport guard fails at 4 MiB with the real -33008. * test(remote): pin the ranged-read cap to real control-queue headroom The cap comment claimed a full-cap window stays in the control lane "unconditionally" and the guard test only asserted one frame fits the lane, so a raise to 384-768 KiB stayed green while two concurrent full-cap responses would already overflow the shared control queue -- which for a response closes the client. Pin the two-deep headroom and state the real bound, including that widening the cap is a wire change against a host still advertising rangedReadVersion 1. Also cover the two behaviours the suite claimed but did not exercise: a regular file answers a full-cap read in one syscall, so the fill loop was untested (both mutations of readFullStreamChunk stayed green), and the merged capability document made the abort-does-not-evict guard load-bearing without any test reaching it. * fix(remote): harden ranged-read validation and retry |
||
|
Neil
|
3ffab9a6b3 |
feat(terminal): read the rendered screen with terminal read --screen (STA-4792) (#15380)
* feat(terminal): read the rendered screen with `terminal read --screen` (STA-4792) `terminal read` returns accumulated pty output with escape sequences stripped. That is the right answer for "what happened over time" and the wrong one for "what is on screen": any program that repaints a line comes back as stacked fragments, so one `clear` typed key by key reads as `cclclecleaclear`, and a prompt that draws a space by moving the cursor loses it. Nothing in the output said which question had been answered, so it was used as rendering evidence and produced false conclusions. The runtime already knew how to render — it replays the byte stream through a headless emulator — but only as a fallback for blank reads, alternate screen, and never-attached ptys. A normal attached terminal never reached it. `--screen` asks for it directly. Every read now reports its source, which also surfaces the pre-existing snapshot fallback that until now swapped rendered lines into an ordinary read with no indication. `screen-unavailable` distinguishes "asked for a screen, none could be rendered, here is the stream" from a stream the caller asked for, and an absent source means the host predates the field. Because an older host strips the unknown param and answers with its ordinary read, `--screen` against one fails with that explanation rather than passing the stream off as a screen. `--screen` and `--cursor` are mutually exclusive: a screen is the current frame and has nothing behind it to page. * refactor(terminal): stamp the screen source where rendered lines enter the read Inferring it from tail array identity worked but made a load-bearing contract out of reference equality; any later path spreading the read would silently mislabel. Rendered lines only enter through one builder, so it stamps there and anything still unlabelled is the stream. |
||
|
Brennan Benson
|
6386b540ad |
fix(quick-open): stop node:path from white-screening the renderer (#15405)
* fix(quick-open): stop node:path from white-screening the renderer #15158 pulled `quick-open-filter` into the renderer graph, but the module still used `import { posix, win32 } from 'node:path'`. The bundler stubs `node:path` in the renderer with an object that throws on any member read, and named bindings resolve at module evaluation — so the renderer threw before React mounted. `pnpm dev` white-screened on main. Switch to a namespace import and have `pathFlavor` return the flavor NAME instead of the module, so `node:path` stays untouched until the out-of-root `relative` fallback actually needs it. A namespace import alone is not enough: `pathFlavor` is called unconditionally by `buildExcludePathPrefixes`, so it still threw on any Quick Open with nested-worktree excludes. No behavior change for the main process or the relay. * fix(quick-open): keep exclude containment browser-safe |
||
|
Brennan Benson
|
c228030516 |
fix(tabs): keep an open diff focused while agents stream (STA-4697) (#15390)
* refactor(tabs): put the visible-tab-type projection in one place Three copies of toVisibleTabType had drifted: the runtime one omits 'simulator'. Move the canonical projection next to the two unions it maps between and replace the two copies that are already identical to it. The runtime copy is left alone on purpose - unifying it would change behavior, so it goes with the follow-up. * fix(tabs): keep an open diff focused while agents stream (STA-4697) resolveWebSessionVisibleTabId answered 'which tab is the user looking at' by inverting a many-to-one projection: it compared tab.contentType against the coarse activeTabType. Diff tabs open with activeTabType 'editor' but carry contentType 'diff', so the match never succeeded and the guard returned null - which is the reconciler's signal to fall through and activate a terminal. Every agent status echo republished the snapshot, so the diff lost focus ~300ms after opening, once per click. Same for conflict-review and check-details. Resolve the visible tab from group state instead, which is what is actually on screen and is the rule deriveActiveSurfaceForWorktree already uses. The coarse address survives only when there are no group records, now projected rather than compared exactly. Also follow the entity within the group when reconcile rematerializes the visible tab under a new id, and teach the browser-create focus guard to observe the group records the resolver now reads. |
||
|
Neil
|
1b5cc00870 |
fix(shell): content-address shell wrapper trees so builds stop clobbering each other (#15285)
Every writer sharing a userData dir -- main's local PTY path, the daemon fork, and the daemons of other builds that outlive the app that spawned them -- wrote one fixed `shell-ready/` tree. Last writer won, and the guard only re-checked that the files were present, never that they were this build's. A daemon whose spawn env no longer agreed with the wrapper on disk kept launching shells it could not read: the ready marker never fired and every startup command waited out the full 15s timeout, silently, with restarting the app powerless to fix it because the daemon outlives the app. Measured 15.17s vs 1.10s once the tree matched. The five live daemons on the machine this was found on spanned app versions 1.4.181-1.4.185, all from the same installed app across auto-updates, so this reaches ordinary installs. Name each tree after a hash of its contents, at `<userData>/shell-wrappers/<hash>/shell-ready/`. Different bytes are a different directory, so "present" means "written by this build" again. The hash sits above the `shell-ready` leaf because ZDOTDIR self-reference guards match that exact suffix. Nothing collects old trees: ~48KB each, a couple of MB a year against a userData dir in the tens of GB, not worth an `rm -rf` on the spawn path. Also publishes the resolved root to WSL over WSLENV `/p`, since the in-guest script cannot derive a hash, and reports readiness failures to the daemon's NDJSON log rather than a console the detached daemon discards. Generated wrapper content is byte-identical; snapshots unchanged. |
||
|
Jinwoo Hong
|
79be5b7fde |
feat(orchestration): report a worker blocked on a human prompt (STA-4513, STA-3714) (#15261)
* feat(orchestration): report a worker blocked on a human prompt (STA-4513, STA-3714) A lane parked on an approval, trust, or permission prompt looked exactly like a lane that was thinking or inside a long tool call. On origin/main, driving a real cursor-agent through Orca: surface running `sleep 60` awaiting approval worktree ps agents[].state working working terminal show / list no such field no such field terminal wait --for tui-idle satisfied: true satisfied: true worker-show no agent state no agent state The runtime already fuses hook state, OSC title, and matched prompt text into a `permission` verdict inside getTerminalAgentStatus — it was reachable only from the renderer, and it was blind to cursor-agent approvals. Two gaps, one boundary. Exposure: getTerminalInteractiveWait publishes that same fusion, minus the async foreground probe, as `agentWait` on `terminal show` and on `worker-show`'s observation. It carries the evidence that proved the wait (hook, prompt-text, or title) so a coordinator can weigh it. Null means no proof; a missing field means the host predates it — absence is never read as "not waiting". Detection: cursor-agent's hook set has no approval event and beforeShellExecution fires identically for auto-allowed commands, so its rendered menu is the only authority. Matched on the key-bound choices rather than the prose, requiring two, and self-clearing when the follow-up input line returns. Its live spinner title is exempted from the staleness rule that clears startup modals, because cursor keeps spinning while it waits. Falls out of routing it through the shared verdict: `dispatch --inject` into a cursor pane on an approval now refuses with agent_prompt_blocked instead of typing the preamble into the dialog. Fixtures are captured verbatim from cursor-agent 2026.08.11-e8db854 driven through Orca; the same case matrix was replayed live against a built runtime. terminal list stays untouched: its rows would each need a full tail scan, and STA-4694 owns the one-call-per-run aggregate. * fix(orchestration): only call a Cursor approval live while it owns the screen Independent review found the approval detector trusted one dismissal string, so any later output that did not contain cursor's follow-up line left the menu reading as a live wait. Reproduced: a tail of the real menu followed by two lines of ordinary output returned agent-approval-prompt, which fails tui-idle and refuses prompt injection on a healthy lane. Replaced with the structural property the string was standing in for: a live dialog owns the bottom of the screen, so the last choice may sit at most one line above the end of the retained tail. That tolerates a status footer or a partial line mid-redraw without admitting scrollback, and it drops the vendor prose. Being bottom-of-screen is also the dating this reason needed, so it no longer requires waitBlockedAt. A tail restored from terminal history carries none, and a lane parked on a prompt emits no bytes — so before this, an Orca restart made exactly the lane both issues are about go quiet for good. The startup modals keep the timestamp rule: their text lingers in scrollback with nothing to say whether it was answered. Also from review: - worker-show and federationShow reuse the verdict showTerminal already computed rather than rescanning the tail, so the two can no longer disagree. - The worker-show test now drives a real runtime, real PTY tail, and the real detector; it previously mocked getTerminalInteractiveWait, so it would have passed with detection permanently returning null. - The guard claim is now asserted against the guard: a blocked pane rejects both assertTerminalAgentSendable and sendTerminalAgentPrompt, and a working pane still passes. - Added a non-local (connectionId) pane case, since the verdict is derived from retained tail and title state on every host. * fix(agent-status): stop a hook wait from outliving its agent A third reviewer caught that the hook branch proved agent ownership from the pane title alone, while the shared verdict it claimed to reuse also probes the foreground process. A shell that takes a pane back usually sets something like `user@host: ~/repo`, which no title rule recognizes, and a hook row stays fresh for AGENT_STATUS_STALE_AFTER_MS — so a dead agent could be reported as waiting on a human for half an hour. Hook evidence now goes through getTerminalAgentStatus, which is the only thing that can answer whether an agent still owns this PTY. The two prompt branches skip it: a matched prompt is on the pane's screen now, so it proves itself. That makes the probe cost fall exactly where correctness needs it, and getTerminalInteractiveWait async, which only showTerminal had to absorb. Also trims the comments the same reviewer flagged as longer than the repo's rule. * test(agent-status): pin that a dead pane stops reporting a human wait A fourth reviewer noted the approval menu sits at the bottom of a dead pane's tail forever, and that no test covered process exit with no trailing output. The snapshot already refuses an exited pane, and worker-show gates agentWait on proven identity — this pins both so neither can drift into reporting a worker that needs intervention as one that needs an answer. * fix(orchestration): never report an unchecked worker as not waiting Automated review caught that the three worker paths which return before the wait is ever evaluated — unattached, missing, and identity_changed — then had their undefined coerced to null by the emitters. A worker whose process was replaced was reported as `agentWait: null`, which reads as "Orca looked and nobody is waiting" when Orca never looked. That is the false negative this field exists to remove. The field is now emitted only when it was evaluated, so a present null is a claim about the pane and an absent one means nobody looked — because the host predates the field, or the worker's identity could not be verified. The CLI and the worker-show note say that rather than blaming an old host. Covered on the context-only path, where the regression test fails against the previous behavior; the supervised and federated emitters take the identical one-line change. Also trims the two test-file headers to one statement of purpose. * fix(agent-status): tighten the Cursor menu match and stop guessing on unknowns Fourth review round, three findings, each reproduced before acting. Matching each choice marker with an independent lastIndexOf let text outside the menu carry the anchor. An agent narrating "next time I'll suggest Run Everything" after the menu was answered pulled the match down to the bottom of the screen and revived it. The match is now confined to the last lines of the tail, and a choice is a line that ends in the key that picks it — prose writes the same words but not the same shape. The one line of slack under the dialog went with it. It was a guess; every capture of a live dialog ends on its last choice, and one line is exactly enough room for that narration. A redraw caught mid-flight now reads as no wait until the next poll, which is the safe way to be wrong. The hook branch awaited a foreground probe that reaches a PTY controller which may be a remote host, so a wedged probe stalled every caller of showTerminal — a path that never probed before. It is bounded now, and a timeout leaves the wait unevaluated rather than claiming there is none. Which is the same distinction the previous commit only fixed one level up: getTerminalInteractiveWait itself turned an unreadable pane into `null`, so showTerminal published "looked, nobody waiting" for a pane it could not read. It returns undefined there, showTerminal omits the key, and worker-show's text output prints unknown rather than rendering it the same as none. * fix(agent-status): bound the wedged probe's cost and stop matching prose keys Fifth review round. No correctness defects in the shipped behaviour this time; two robustness holes and the documentation of the contract. The bounded probe abandoned the wait but not the request, so a coordinator watching a wedged remote host added one live probe on every poll. It is single-flighted per PTY now, the way the leaf-absence probe already is. The trailing-key rule that separates a menu row from the agent narrating a choice was written as a character class, and any lowercase run up to twelve characters satisfied it — "…suggest Run Everything (as before)" passed. Spelled out as key names instead, which also lets the glyph forms of those keys through. The contract wording said an absent agentWait meant an old host or an unverifiable identity. It also covers an unreadable pane and a probe that did not answer, and a reader diagnosing an old peer from that would be wrong. Corrected on the type, the worker-show note, and in docs/reference/remote-wire-compatibility.md, which had no entry for a field whose absent and null states mean different things. Also strengthens the worker-show agreement test, which compared the terminal and observation payloads without asserting either held the expected wait, so it passed when both were absent. |
||
|
Brennan Benson
|
0b80a773a4 |
fix(codex): stop overwriting and deleting Codex files that were merely unreadable (STA-4737) (#15287)
* fix(codex): stop overwriting and deleting Codex files that were merely unreadable (STA-4737)
Three modules shared by the host and WSL Codex lanes decided a file was absent
from a read that had only failed, and then wrote over it or removed it.
- `codex-config-mirror`: `existsSync` on the RUNTIME config.toml returned false
for a locked file exactly as for an absent one, so the mirror took the
"seed a fresh runtime config" branch and replaced the user's config wholesale.
- `config-settings-promotion`: an unreadable ~/.codex/config.toml counted as
having no promoted settings, and the write path then rebuilt the user's
canonical Codex config from Orca's runtime copy.
- `codex-home-paths`: both delete branches in `linkSystemCodexResource` remove
Orca's mirrored copy because the system resource "is not there". `existsSync`
and `systemResourceIsRegularFile`'s `catch { return false }` both reported
that for a source nobody could read, so one denied read on ~/.codex/AGENTS.md
removed the managed copy on the next launch.
`src/shared/definitive-filesystem-absence.ts` now owns the one errno allowlist —
ENOENT and ENOTDIR, with every other code including unrecognised ones treated as
indeterminate — and `host-codex-managed-home-ownership.ts` drops its private
copy rather than letting the two drift. `codex-path-observation.ts` builds the
three-valued observation on top of it.
The resource sync's two `existsSync`/`statSync` probes collapse into one
resolved stat, which answers reachability and regular-file-ness together and
closes the window between them.
`config-settings-promotion.ts` crossed its max-lines budget, so the write-target
resolution moves to its own module rather than taking a lint exemption.
Deliberately not here: the hook-service trust writes that run after a refused
mirror, and the promotion write target's own classification, which is
unreachable because it always resolves to the same file the read above already
refused. Both are noted in comments rather than half-built.
* fix(codex): preserve resource copies on indeterminate reads
|
||
|
Jinwoo Hong
|
a77a2f93f7 | fix(remote): search Quick Open paths on the host (#15158) | ||
|
Jinwoo Hong
|
bef76953d7 | fix(orchestration): record why a terminal's process is gone (STA-4603, STA-4536) (#15244) | ||
|
Brennan Benson
|
2a760e310b |
fix(computer): report unasserted accessibility actions (#15028)
* fix(computer): report unasserted accessibility actions * fix(computer): fail closed on missing action metadata * Fix merged tab search test fixture |
||
|
Brennan Benson
|
2f0f9a8a39 |
Revert "fix(agent-hooks): bind agent status to the pane its session was spawned into (STA-2069) (#14615)" (#15295)
Reverts #14615. Its premise does not reproduce, it does not reach the failure that does, and the correction it installs can misattribute status on a path that worked before. 1. PREMISE FALSE. #14615 asserts Claude Code >= 2.1.206 hosts TUI sessions under a shared daemon. On 2.1.233 `claude daemon status` reports "not running" with 69 live interactive sessions, and every client is a direct child of its own pane's shell. Measured across the fleet: 68 distinct pane keys, zero collisions. Foreground attribution was never broken. 2. DOES NOT FIX THE REAL BUG. The failure in #9236 is real but scoped to BACKGROUNDED sessions, whose workers inherit the dispatching pane's whole ORCA_* set. #14615 mints a binding only for launches Orca constructs, so a typed `claude --bg` produces none. Fixed properly in #15304. 3. INTRODUCES A MISATTRIBUTION. Bindings are removed only on PTY death, and a user who exits Claude keeps the pane's PTY. Resuming that session in another pane does not rebind (`--resume` is a session selector, so the pin declines), and resolveBoundPaneOverride then rewrites paneKey and tabId onto the ORIGINAL pane despite a correct posted key. Demonstrated with a failing test against main; causation isolated to resolveBoundPaneOverride. Kept #14706's observations.rebind() in the conflicting hunk — it postdates #14615 and is not part of this revert. |
||
 OrcaWinandOrcaWin
|
b7f2e17712 |
fix(git): fence buffered WSL login-shell reads (#15060)
* fix(git): fence buffered WSL login-shell reads Two git paths force the login shell unconditionally and buffer its whole stdout, so the distro's rc banner lands in front of the payload: - gitExecFileAsyncBuffer backs `git show :<path>` blob reads and hands the bytes straight to the diff/blob viewer, so the banner is prepended to displayed file content. - buildNetworkSshPolicyEnv probes `core.sshCommand` and treats any non-empty answer as a user-configured wrapper. A banner reads as configured, so the code skips the `ssh -o BatchMode=yes` fallback and silently disarms the guard that keeps non-interactive SSH from hanging on a prompt. Fencing is opt-in per call site rather than applied to the login-shell branch as a whole: streaming consumers (`git grep`, `ls-files -z`) parse records as they arrive, so an opening marker would be glued onto their first record. Only these two, both buffered by construction, opt in. Blob content can be binary, so the payload is sliced out of the raw bytes; decoding to find the fence would corrupt it. The markers are exposed on the captured command because the shared module is bundled for the renderer and cannot reference Buffer. Note this path is not a rare fallback: `preferWslDirectGit` is only set by gitStatusReadOptionsForWorktree, so every other WSL-routed git call takes the login shell on every invocation. * test(git): use findLast for the ssh-policy call lookup Satisfies the code-quality rule that flags filter-then-index. --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> |
||
|
OrcaWinandOrcaWin
|
3a9f40ed70 |
fix(wsl): read machine output from a fenced login shell (#15290)
* fix(wsl): read machine output from a fenced login shell Orca runs WSL reads through the distro's *interactive* login shell so PATH matches the user's own terminal (nvm, mise and asdf only install into rc files interactive shells read). An interactive shell also runs the distro's rc/motd, and stock Ubuntu 24.04 writes its "run a command as administrator" hint to stdout -- no user customization required. Every caller parsing that stream was reading the banner as data: statPath -> "To run a command as administrator...\n\ndirectory" readPath -> banner prepended to the contents of every file read preflight -> banner prepended to `gh --version` / auth output `.trim()` cannot recover any of these, so a WSL worktree's file explorer sees no valid entry types and file reads return junk. Three call sites had independently grown their own marker to survive this (`__ORCA_AGENT_PATH__`, `ORCA_WSL_GIT_READ_ENV_V1`, and a `>/dev/null` fd dance), which is the tell that it belongs in one place. Fence the payload once, in the shared builder, and hand callers a reader that returns just their bytes. The fence carries a per-call nonce so `cat`-ing a file that happens to quote a marker is not truncated. Exit status is preserved, so the ENOENT mapping still works. wsl-git-read-environment drops its bespoke marker and parsing. * test(wsl): fence the login-shell path-lookup boundary test It asserted a raw interactive login-shell read matched an absolute path, so the distro rc banner made it fail on any stock Ubuntu. It is part of the shell-contracts CI gate, where it skips on Linux and hid the break. * docs(wsl): record the guest command-execution contract Both failure modes are silent - the command runs, exits 0, and returns the wrong bytes - so the rules need to live somewhere a reader will find them before writing the next wsl.exe call site. * fix(codex): fence the WSL Codex identity probe buildWslCodexBinaryStamp reads the login shell's stdout positionally -- path before the first newline, version after -- through an interactive login shell. On a stock Ubuntu the rc banner lands ahead of the payload, so the first newline falls inside the banner and the stamp becomes path="To run a command as administrator..." with the rest as version. Both halves are non-empty, so nothing throws: the stamp is silently wrong, and an unstable stamp reads as "the Codex binary changed" and reissues the trust grant. The identity script ends in `exec`, so it never writes a closing fence; the reader returns everything after the opening one, which is exactly this case. buildWslCodexIdentityArgs becomes buildWslCodexIdentityProbe and returns the reader with the argv so the two cannot drift apart. The other three WSL Codex commands are deliberately left unfenced: availability is exit-code only, and app-server/login hand stdout to a long-running program. * fix(wsl): harden the capture fence after review - readStdout now takes the LAST opening fence, matching the lastIndexOf the wsl-git-read-environment marker used deliberately: a login shell can echo the command text before running it, repeating the fence. - local-worktree-filesystem throws instead of falling back to raw stdout when the fence is missing. The fallback silently reinstated the bug being fixed -- statPath would return the banner as a file type and readPath would return banner+contents, with no signal. Preflight keeps its fallback; its matchers scan the whole blob and tolerate a prefix. - The exit-status test asserted only that the script CONTAINS `exit $?`, which is true for any input and never executed those lines. It now runs a real distro and asserts status 2 reaches the caller, which is what statPath's ENOENT mapping depends on. - Corrected the doc: a sed backreference has no `$`, so `--` never rewrote it. Replaced with the positional and shell-local cases that were measured to differ. * fix(wsl): stop running a login shell for filesystem reads statPath/readPath/rm run coreutils at standard paths and shell builtins. They need nothing from the user's PATH, so there was never a reason to start a login shell -- and starting one is what put the distro's rc/motd on the stdout these callers parse. Fencing that output treated the symptom. Using a plain `sh -c` removes the cause: no profile, no rc, no banner, by construction. The fence and its missing-fence error go away with it. The fence stays where it is actually needed: the three places that must run the user's shell to resolve their PATH (the preflight CLI probe, the WSL git environment probe, and the Codex identity probe). Net -12 lines. --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> |