mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
f97503580966560342db8cdc7fa5054efa9e1b39
7093
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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
|
||
|
Neil
|
6785dc092d |
fix(composer): close the Create Workspace dialog on the first Escape (#16027)
* fix(composer): close the Create Workspace dialog on the first Escape The modal copied the page-level "Esc blurs the focused field, then closes" rule from TaskPage/Automations. On a page that rule protects a focus the user chose; this dialog auto-focuses the name input on open, so its capture-phase handler preventDefault'd every first Escape (which also suppressed Radix's dismissal, since DismissableLayer skips a defaultPrevented event) and the dialog could only be closed with two presses. Drop the Escape branch and let the dialog's dismissable layer own it. Radix dismisses only the topmost layer, so nested popovers, selects and dialogs still consume their own Escape first. * test(e2e): pin the composer's auto-focus as the reason one Escape must close it |
||
 NeilandMelih
|
7ce11dcf55 |
fix(agent-resume): restore Copilot sessions after restart (#15879)
Co-authored-by: Melih <mberatsanli@gmail.com> |
||
|
Neil
|
15fd723bc4 | fix(terminal): drop conda's orphaned CONDA_SHLVL sentinel (#15885) | ||
|
Neil
|
636f428c25 | fix(rate-limits): stop showing Gemini failures as Antigravity "Refresh failed" (#15876) | ||
|
Neil
|
ac103f5d90 | fix(project-groups): route rename and delete to the group's owning host (#15889) | ||
 Neilandterry-li-hm
|
5f2fbd862e |
fix(pty): wrap codex without expanding or destroying a user alias (#15873)
Co-authored-by: terry-li-hm <12233004+terry-li-hm@users.noreply.github.com> |
||
 NeilandTauri-EPO
|
8c1e6ad0cf |
test(orchestration): distinguish the heartbeat straggler guard from the row's initial null (#15869)
Co-authored-by: Tauri-EPO <enrico.pin@gmail.com> |
||
|
NeilandTauri-EPO
|
d3869cd0b4 |
test(runtime): use the platform submit delay in the cancellation-during-verification test (#15870)
Co-authored-by: Tauri-EPO <enrico.pin@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
|
||
 erish
|
063b804298 |
fix(i18n): match CheckRunJobs succeeded to its sibling count-label register (#16013)
succeeded rendered as casual declarative 성공했다 ('it succeeded') next to
skipped's polite 건너뛰었습니다 and pending's noun-phrase 보류 중, in a summary
that joins all three after a count: '3 성공했다 · 1 건너뛰었습니다'. Machine
translation read succeeded as a finished sentence instead of the noun label
the other two siblings use. Switch to 성공 and pin it in the key-override
file so the catalog regen script can't revert it; #15875 fixed five other
keys in the same family but its hardcoded regression map didn't cover this
one.
|
||
 
|
fb5c9a1fe6 |
fix(ui): clear agent attention icon when Floating Workspace tab activated via keyboard (#15745)
* fix(ui): clear agent attention icon when Floating Workspace tab activated via keyboard When a tab in the Floating Workspace is activated through keyboard shortcuts while the Floating Workspace is not the active worktree, the agent completion notifications were not being acknowledged, leaving the yellow attention icon visible. The issue was that `useAutoAckViewedAgent` only checked the global `activeTabId`, which doesn't change when the Floating Workspace is not the active worktree. Now the hook also watches the Floating Workspace's active tab (`activeTabIdByWorktree[FLOATING_TERMINAL_WORKTREE_ID]`) and acknowledges agents when they become visible in the Floating Workspace, regardless of whether it's the active worktree. Fixes #15700 * fix(ui): gate Floating Workspace auto-ack on panel visibility The floating-workspace scan added in the previous commit had no visibility gate. The panel stays mounted while closed (use-floating-workspace-panel: shouldMountPanel), so its layout still resolves an active leaf and the hook acked a floating agent completion the moment it landed — silently killing the minimized toggle's attention dot (selectFloatingWorkspaceHasUnread), which is the only "unseen floating activity" signal a closed panel has. - Scan the floating tab only while the panel is actually visible (isFloatingWorkspacePanelVisible), so a closed panel keeps its dot. - Move the activeView filter onto the main-worktree target only: the panel is an overlay above every view, so it must ack from the activity/tasks views too. - Carry the owning worktree with each target instead of re-deriving it by tab id, and keep the first entry on a tab-id collision, so a duplicate id can no longer clear the wrong worktree's unread. Drops the `as string[]` cast. - Re-scan on TOGGLE_FLOATING_TERMINAL_EVENT (next frame, after aria-hidden commits) since panel open/closed is React state the store never sees — opening onto an already-active completed tab now acks. - Cover the new resolveAutoAckTabTargets helper, including the closed-panel regression asserted against selectFloatingWorkspaceHasUnread. * fix(ui): re-scan floating workspace auto-ack on every panel-open path The visibility gate read the panel's aria-hidden and only re-scanned on TOGGLE_FLOATING_TERMINAL_EVENT, so the two paths that open the panel without that event — the floatingWorkspace.maximize keybinding and the default floating-button toggle — left an already-active completed tab's attention icon lit. Drive the gate from the committed `enabled && open` state instead: that is what aria-hidden is derived from, it covers every open path, and it drops the requestAnimationFrame that existed only to outrun the un-committed DOM read. Adds a hook-level test (happy-dom) that fails both when the gate is removed and when the open re-scan is removed. * fix(ui): re-read store state per auto-ack target Acking the first target writes to the store and re-enters the scan synchronously, so the pre-write snapshot could re-ack a target the nested pass already handled. Idempotent today; a footgun for the next non-idempotent action. --------- Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> |
||
|
Neil
|
d07ce15cff | refactor(host): route secret storage through a SecretStore port (#15916) | ||
|
Neil
|
4828c6bed4 | UX: compact native density for context + dropdown menus (#15924) | ||
 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. |
||
|
JinjingandNeil
|
8d021c0b5a |
test(automations): assert in-place language switch on mounted picker (#15977)
* 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
* test(automations): assert in-place language switch on mounted picker
Add a test that changes the language while the weekly picker remains mounted,
then checks that the localized labels update while the underlying values
("0"–"6") stay stable. This validates the regression-prevention that stable
index keys (added in #15884) support — without them, a locale change would
unmount and remount options, breaking the persisted dayOfWeek value.
Wrap the picker in a LanguageAwarePicker harness that calls useTranslation(),
mirroring the root-level subscription in main.tsx that drives real
re-renders on language change.
---------
Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
|
||
|
Jinjing
|
6c1286b592 |
Add Artifacts and Skills pages to navigation history (#15969)
* Add Artifacts and Skills pages to navigation history - Record Artifacts and Skills visits in back/forward navigation like Automations - Both pages properly rewind history when closed to the previous live entry - Extract rewindHistoryIndexPastView() helper to deduplicate close-page logic across all page types - Add test coverage for Artifacts/Skills navigation, separate entries, and shared link handling * Add Artifacts and Skills pages to navigation history Back/forward buttons now appear when navigating to Artifacts and Skills pages, consistent with Terminal, Tasks, and Automations. |
||
|
Jinjing
|
9ea1d28970 |
Fix Cmd+J Enter for worktree creation (#15970)
* fix(cmd-j): allow Enter to create worktree * test: verify create dialog closes on Escape |
||
|
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>
|
||
  OrcaWinandm4air
|
72f896d677 |
feat(automations): navigate table search results with arrows (#15805)
* feat(automations): navigate search results with arrows * Set overview tab for external automations on arrow selection External automations lack a runs tab, so the detail pane must default to overview when navigating via arrow keys to keep the tab selection valid when the automation is later opened. --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain> |
||
|
Jinjing
|
80b2a02377 |
Move worktree palette search to top of sidebar nav (#15854)
* Move worktree palette search to top of sidebar nav The Cmd+J search button is now displayed as the first item in the sidebar navigation, improving discoverability. Styling is simplified to match the nav item layout with flex-based display and consistent spacing. * Test: add guard clause for worktree palette search button - Add explicit type annotation for the search button querySelector - Guard against missing button with clear error message - Use direct property access now that button existence is verified |
||
|
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. |
||
|
Neil
|
6b51ef4e2c |
feat(wsl): one runner for every wsl.exe invocation (#15903)
Five decisions have to be made on each `wsl.exe` call. Each has a right answer,
each is invisible in a diff, and each has shipped wrong:
- **Separator.** `--` makes wsl.exe expand `$name` in every forwarded argument
before the guest runs -- even with no shell in the command -- so `awk
'{print $2}'` loses its field reference (#12964).
- **Shell.** A login shell on a probe path sources `~/.profile`, so one blocking
line eats the whole timeout (#14288) and every call pays startup (#9768). No
login shell on a user-facing path means PATH does not match the user's own
terminal, so nvm-installed agents read as absent (#9725, #7563, #8366).
- **Fencing.** An interactive login shell runs the distro rc, and stock Ubuntu
writes its "run as administrator" hint to *stdout* -- so anything parsing that
stream reads the banner as data (#11327, #11823).
- **WSLENV.** Unset, a Windows-side variable silently never crosses (#12557).
- **Payload.** Scripts go in on stdin. A script on stdin has no quoting boundary
to escape from, which is what the base64 and `eval` wrappers work around
(#14292). `filesystem-watcher-wsl.ts` already does this and is the only WSL
caller with no quoting bug in its history.
`runWslProcess` makes them once, on top of W1's `runProcess` so it inherits
windowsHide, shell:false, timeouts and abort. `lane` is required with no
default: picking the wrong lane by omission is the most common WSL defect here.
The probe lane resolves the login PATH/HOME once per distro and then runs with
no shell at all, so #14288 and #9768 are closed by construction rather than by a
longer timeout. An unprobed distro degrades to the interactive lane -- "we could
not ask" must not become "run with no PATH".
Additive only: no call site is migrated yet. The new guard allowlists the 23
files that still spawn directly, and its length is the workstream's goalpost.
Two guard bugs found by testing the guards against planted call sites: a bare
`main/wsl` prefix also exempted `main/wsl.ts`, `wsl-availability.ts` and
`wsl-unc-delete.ts` -- three real offenders.
|
||
|
Neil
|
990b23611e | fix(i18n): correct five Korean strings that changed meaning in machine translation (#15875) | ||
 Neiland2sumtech
|
445c390170 |
fix(cli): allow an empty --value for storage set commands (#15863)
Co-authored-by: 2sumtech <2sumtech@gmail.com> |
||
|
Jinjing
|
9725654855 |
Revert "Revert "Update Android download links to 0.0.44 (#15898)" (#15899)" (#15905)
This reverts commit
|
||
|
Neil
|
84659e2eab |
fix(ports): report a Stop as succeeded when the listener already exited (#15888)
* fix(ports): report a Stop as succeeded when the listener already exited `killWorkspacePort` surfaced the raw `kill ESRCH` when the pid exited between the authorizing re-scan and the signal. The port is free at that point -- which is exactly what Stop was asked for -- so the UI reported a failure for work that had already completed. Also pins the pid we signal: `netstat -ano` and `lsof` report the process that owns the socket, so the scanned pid is the listener itself, not a supervising wrapper. Escalating this to a tree kill would reach descendants nobody asked to stop without freeing anything extra. * test(ports): give the spawn-stall test a budget longer than the stall It blocks the calling thread for the full watchdog budget plus margin (5.2s) and then asserts against vitest's 5s default, so it has been failing deterministically since #12217. |
||
|
Jinjing
|
a04fe24c80 |
Revert "Update Android download links to 0.0.44 (#15898)" (#15899)
This reverts commit
|
||
 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> |
||
|
Jinjing
|
5ce356cc4a | Update Android download links to 0.0.44 (#15898) | ||
|
ecfc547ece |
fix(windows): reach the pty job on the teardown path that actually runs (#15886)
W2 gave every ConPTY pane a job object and wired `terminateOwnedTree` into `local-pty-provider.ts`. Measured in #11047: worktree delete does not execute there. It runs in the terminal daemon, so on the path that matters the sweep still fell back to a parent-pid walk -- which a detached, reparented grandchild is not in. That process is the one that holds the worktree cwd open, so the delete this was meant to fix could still fail. Expose the job on `SubprocessHandle` and use it at the three daemon call sites (`terminal-session-teardown.ts` x2, `session-termination-controller.ts`). Also: on Windows `forceKill()` returned early after any `kill()`, to avoid double-closing the ConPTY handle node-pty owns. Correct, but it left force-kill a permanent no-op -- and a wedged ConPTY never fires `onExit`, so such a session had no escalation at all (#9854). Terminating the job is that escalation, and it does not touch node-pty's handle. A tree-walking guard now fails if any production `killWithDescendantSweep` call omits `terminateOwnedTree`, since the option is optional by design and its absence is invisible in review -- exactly how this gap survived W2. Co-authored-by: OrcaWin <orcawin@users.noreply.github.com> Co-authored-by: hanbong5938 <hanbong5938@users.noreply.github.com> |
||
 Jinwoo Hong
|
1354ff534f | fix(cmd-j): host-qualify browser and simulator tab candidates (STA-4965) (#15686) | ||
|
Neil
|
2b1254d681 |
fix(windows): own PTY process trees with job objects (#15755)
* fix(windows): own PTY process trees with job objects Teardown used to answer 'is this tree mine, and how do I kill it?' by scraping the process table, walking parent pids back to Orca, and running taskkill /T /F only if the walk said yes. Every step is a guess, and the code said so itself: windows-pty-root-identity.ts:35 already named the fix -- 'an inherited handle / Job Object'. The guesses fail in the ways users report. A pid walk cannot survive pid reuse, so teardown refused whenever it could not prove ownership, and a refused kill is an orphaned agent tree holding the worktree directory open (#9045, #10475, #10087). A descendant that reparented is invisible to the walk. The scrape itself could be blocked by policy, which read as 'no evidence'. node-pty now creates a job object per ConPTY and assigns the shell under CREATE_SUSPENDED, before it can spawn anything -- assigning afterwards leaves a window in which a fast child escapes. Termination is one TerminateJobObject; liveness is QueryInformationJobObject. Verified on Windows 11 against a shell whose grandchild was spawned detached: job membership came back [shell, grandchild] and one call killed both. Neither a parent-pid walk nor GetConsoleProcessList sees that grandchild -- it leaves the console and reparents, which is exactly the claude.exe/node.exe/cmd.exe orphan in #9045. KILL_ON_JOB_CLOSE means a daemon that dies without unwinding no longer strands shells (#9195, #10415). The job is the daemon's, not the app's, so an app-main crash still leaves sessions alive -- the guarantee win-crash-survival-e2e asserts. Both entry points report unavailable rather than a false success when a pty has no job: an outer job without BREAKAWAY_OK can refuse the assignment, and a pty from an older build has none. Reading 'we could not tell' as 'already dead' is the original bug, so the old probe stays as the fallback. * test(windows): pin job ownership against a real detached grandchild The unit tests pin the contract; this pins what the contract is for. A grandchild spawned detached leaves the pane's console and reparents, so GetConsoleProcessList and a parent-pid walk both miss it -- that is the process that outlived its pane and held the worktree directory open. Includes a guard that this build actually has job support, so a node-pty rebuilt from unpatched sources fails loudly instead of letting every assertion pass vacuously. * fix(windows): correct the job liveness contract to what Windows actually does I claimed an emptied tree would report [] and that this was the evidence a stale registry entry lacks (#15549). Running it on Windows 11 showed otherwise: node-pty drops its handle record and closes the job when the shell exits, so a dead tree reports null. Null therefore means unverifiable in the sense of docs/reference/ssh-execution-boundary.md -- no job support, not a ConPTY, or no longer tracked -- and is never evidence that processes died. A caller reading it as proof of death would have been right by accident after a normal exit and wrong on a host that refused the assignment. What the API does add is descendant liveness for a tree that is still tracked, including children that detached from the console. * fix(windows): stop a clean shell exit from reaping backgrounded processes Measured on Windows 11: with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE on the per-PTY job, releasing the handle when the shell exits also killed whatever the user had backgrounded. Typing 'exit' in a pane reaped a detached server that survived before this patch. That is a behaviour change nobody asked for. The approved change was that killing the terminal daemon reaps its shells -- not that a clean exit reaps your background job. The job's purpose is to make an EXPLICIT teardown exact, which TerminateJobObject still does. Reaping a dead daemon's shells now needs the daemon-level job the design called for: the daemon assigns itself, children inherit membership, and its closure on daemon death reaps them without touching clean-exit semantics. Not in this PR; noted in the reference doc. * test(windows): pin that a clean exit leaves backgrounded work alone The counterpart to the tree-kill test. Without it, re-adding JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE would look like a tightening rather than the regression it is. * fix(windows): stop a winpty pty id from matching a ConPTY job winpty.cc and conpty.cc each mint their 'pty' id from an independent counter, and windowsPtyAgent stores both in the same _pty field. So a winpty-backed terminal's id can collide with a live ConPTY baton -- and closing that pane would have terminated an unrelated pane's entire process tree. Both job entry points now take the shell pid and the native side refuses unless GetProcessId(hShell) matches, which makes the id unforgeable. Two more from the same read-through: - ResumeThread's failure was ignored. A shell left suspended is a pane that never prints and never exits, which is far harder to diagnose than a failed spawn; it now cleans up and throws. - handle->hJob was assigned before LoadConptyDll, which can throw. A baton carrying a job but never reaching SetupExitCallback has nothing left to close it, so the assignment moved down beside hShell. * docs(windows): record the unsynchronised node-pty baton table Pre-existing upstream -- the exit thread erases while the main thread reads -- but terminatePtyJob adds an instance of it, so it belongs in writing rather than in someone's head. * fix(windows): close four gaps found in review BREAKAWAY. The per-PTY job set no limits, so a child asking for CREATE_BREAKAWAY_FROM_JOB was refused with ERROR_ACCESS_DENIED. Installers, msiexec and some updater and service-control paths spawn that way deliberately -- they worked before this patch and would have failed only inside an Orca terminal, which is the worst shape a bug report can take. JOB_OBJECT_LIMIT_BREAKAWAY_OK restores it; a child still has to ask, so ordinary descendants stay owned. EMPTY IS NOT UNAVAILABLE. The native reader returns an empty list -- not an error -- when CreateToolhelp32Snapshot fails, which is what an EDR hook or a restricted token produces. Callers read that as 'nothing is running' and teardown concludes a live PTY root is already gone. The snapshot must contain the querying process; nothing else is unfalsifiable, and one predicate catches empty, truncated and permission-filtered tables alike. NO DEADLINE. Replacing execFile dropped its 3s timeout. The vendored reader latches a module-global while a request is in flight and clears it only after draining its callbacks, with no try/catch -- so one wedge leaves every later call queued behind a promise that never settles, and the process table is dead for the life of the app. The bound is back. GUESSED IMAGE PATH. executablePath was derived from the first space-delimited token, which reads 'C:\Program' out of an unquoted 'C:\Program Files\nodejs\node.exe ...'. Wrong evidence is worse than none, and the only consumer already had the full path in , so the field is gone rather than repaired. Also: remove_pty_baton no longer sits inside assert(), which NDEBUG would compile away along with the call, and the job accessors hold a lock across lookup and use -- handle values are recycled, so an unguarded read could pass the shell-pid check against an unrelated process and terminate the wrong job. * fix(windows): apply the job lock once per accessor The patch script matched a string its own replacement still contained, so PtyTerminateJob got two lock_guards named guard and PtyListJobProcessIds got none. MSVC caught it: error C2374 redefinition. * test(windows): pin that a child can still break away from the job Verified on Windows 11: 'start /b' writes its marker and no access-denied appears. Without JOB_OBJECT_LIMIT_BREAKAWAY_OK this fails, and it fails only inside an Orca terminal -- so the failure would look like Orca corrupting unrelated software rather than like a job-object change. * fix(windows): stop the ownership guard from reading a closing handle The guard called GetProcessId(hShell) to prove identity, but the exit watcher closes hShell on another thread -- so the guard could read a closed handle, and under strict handle checks that is fatal rather than merely wrong. Worse, it widened the gap between validating hJob and using it from two instructions to a kernel round-trip, and handle values recycle: the likeliest occupant of a freshly recycled value in this process is another pane's job. The pid never needed a handle. It is captured at spawn and compared as a DWORD, so the guard touches no handle at all, and hShell is now closed inside the same lock as hJob. Also from review: - reject CR/LF in a cmd argument. cmd ends the command at a raw line break whatever the quote state, so there is no escape for it; encoding one anyway truncates the argument and can leave the remainder to run as a command. Agent prompts are this encoder's motivating input. - ask the process table only for the fields a caller needs. Memory and CommandLine each cost an OpenProcess per process, inline, for every process on the box -- and the 1024 bound is patched out. Ancestry reads now skip both. - corpus gains the degenerate quote-only and two-quote arguments. - PtyListJobProcessIds' docblock still taught the empty-list contract that was corrected on the TS side, and now records that the ConPTY console host is never a job member. - drop a write to NumberOfAssignedProcesses, which is output-only. - pty_baton::hShell is initialised; ownsShell was only safe because && short-circuited ahead of it. The backgrounded-child test is rescoped: 'start /b' uses CREATE_NEW_CONSOLE, not CREATE_BREAKAWAY_FROM_JOB, so it proves job membership does not block backgrounding -- not that BREAKAWAY_OK works. That flag rests on the Win32 contract, and I have said so rather than letting the test imply coverage it does not have. * fix(windows): bound retries after the process table wedges The 3s deadline stops a caller hanging, but the timed-out call leaves its callback in the vendored module's queue -- and that queue drains only when the latched request completes, which in this wedge never happens. Retrying at the caller's poll rate would add a closure per tick forever. A 30s cooldown bounds it to one probe, and a late callback clears the cooldown because it proves the reader recovered. Also pins the deadlock invariant in the patch: the exit thread's lock must close before tsfn.BlockingCall, because that waits on the JS thread and the JS thread can be waiting on the same mutex inside PtyTerminateJob. Correct today by scoping; a comment so a later refactor does not widen it. * revert(windows): drop the field-selection API, which cannot pay off I added it for a real perf finding -- Memory and CommandLine each cost an OpenProcess per process -- and then never wired a caller, so the claim that ancestry reads skip them was wrong. Wiring it would have been worse than leaving it dead. The only ancestry consumer is the teardown identity probe, which needs a snapshot that started AFTER it asked, for pid-recycle detection. Bypassing the shared reader to get narrow fields would let that request join a scan already in flight -- trading a correctness guarantee for milliseconds. Field selection only pays off if callers can ask for less, and they cannot: one shared snapshot serves every caller so a 32-wide teardown collapses into a single scan, which means it has to carry every field. The reasoning now lives next to the flags instead of in a dead export. * fix(process): three P1s from review — a crash vector and two wedge bugs STDIN EPIPE COULD TAKE DOWN THE MAIN PROCESS. A child that exits without reading makes the queued write fail with EPIPE, and an unhandled error on a stream is an uncaught exception. The child's own error listener does not cover its stdin stream, so runProcess({ input }) against a short-lived child was a crash, not a failed call. THE COOLDOWN LEAKED A BATCH PER CYCLE INSTEAD OF BOUNDING IT. At expiry every concurrent caller passed the check before any of them re-armed it, so each enqueued a callback into the still-latched native queue and each cycle leaked another batch. The cooldown is now re-armed BEFORE probing, so exactly one caller gets through. A SYNCHRONOUS THROW LEFT ITS DEADLINE RUNNING. The timer was declared inside the try, so catch could not clear it; it fired later and wedged a reader that had already recovered. Hoisted and cleared, and wedge state now carries a generation so a request that lost its deadline cannot mutate it on behalf of the one that replaced it. Found by review once the prompts were short enough for the reviewer to finish -- the previous two rounds died on prompt length. * fix(process): stop a stream error from crashing the main process Same class as the stdin EPIPE finding, two instances further on: stdout and stderr had data listeners and no error listeners, and an unhandled error on a stream is an uncaught exception. Scoped to runProcess, which owns the child outright. spawnProcess hands the streams to its caller, and a blanket handler there defeats callers that track and remove their own listeners -- the SSH ProxyCommand transport does exactly that, and its cleanup test caught the attempt. Documented on spawnProcess so the boundary is explicit rather than inferred. * fix(windows): validate the ConPTY DLL before creating the process LoadConptyDll throws when conpty.dll is missing -- a real state, and one this branch hit during development. It ran after CreateProcessW and ResumeThread but before the baton and the exit watcher were installed, so a throw leaked the job, process and thread handles and left an untracked shell tree running. Once per attempt, so a broken install accumulates orphan shells on every retry. Resolving the DLL first costs nothing and leaves exactly two throws after creation: the CreateProcessW failure, where nothing exists yet, and the resume failure, which already cleans up after itself. This also closes the same leak for hProcess and hThread, which predates the job work. * feat(windows): add the daemon-level job the design called for The plan specified two nested jobs and I built one. That gap is why dropping KILL_ON_JOB_CLOSE from the per-PTY job cost the approved guarantee that a dead daemon reaps its shells -- I had one job trying to answer two questions, and the two answers conflict. They are separate jobs. The per-PTY job answers 'kill exactly this pane's tree, now', and cannot be kill-on-close because its handle is released when the shell exits, which would reap whatever the user backgrounded. The daemon assigns itself to a second job that IS kill-on-close; its handle is released only when the daemon dies. Children inherit membership, so every pty is covered and the per-PTY jobs nest inside it. Daemon, never app: an app-main crash must still leave sessions alive, which win-crash-survival-e2e asserts. Both jobs carry BREAKAWAY_OK, or a child asking to break away is refused at whichever level lacks it. Restores #9195 and #10415, which I withdrew from this PR earlier. * docs(windows): record what the host job does not cover An app-hosted PTY gets a per-PTY job but no crash reaping, because the alternative is a kill-on-close job on the app -- which is precisely what the crash-survival guarantee forbids. * ci(windows): run the win32 suites in the PR windows job Both were skip-on-non-win32 and had only ever run on one machine I drive by hand -- which went unreachable at exactly the moment I needed to verify the percent-escaping fix. Verification that depends on one box is not verification. The job already builds node-pty from patched source and already runs a useConptyDll test, so the ConPTY runtime files are in place by this step. This also makes the encoder a gate: the corpus is the only thing standing between an agent prompt and a mangled argv, and it now runs against real cmd.exe on every PR. * fix(deps): refresh the lockfile for the current patch hashes pnpm records a hash per patched dependency, and I regenerated both patches repeatedly across the review rounds without refreshing the lockfile. Every local run used --frozen-lockfile's looser sibling, so nothing caught it until CI did: ERR_PNPM_LOCKFILE_CONFIG_MISMATCH Cannot proceed with the frozen installation. The current "patchedDependencies" configuration doesn't match the value found in the lockfile Verified with pnpm install --frozen-lockfile locally this time. * ci(windows): build node-pty from source before the win32 suites CI proved the encoder fix on real cmd.exe -- 26/26 -- and in the same run proved the job suite had been testing an unpatched binary. node-pty prefers its upstream prebuild, which does not contain this patch, so every job-object export was absent and isPtyJobOwnershipAvailable() was false. That guard is why the failure was loud rather than a vacuous pass, and it is the reason the assertion exists. Packaging was never affected: rebuild-native-deps.mjs already builds node-pty from source for Electron and restores the ConPTY runtime files. The gap was the node-runtime test environment only. Not changing requiresPatchedNodePtySourceBuild's win32 exemption here. Its premise -- that the patch is Unix-only -- is now false, but lifting it also needs pnpm rebuild to force a source build, and I cannot validate that on macOS and Linux from here. Recorded as a follow-up instead of changed blind. * test(windows): gate the host-job guarantee in CI The daemon-level job had one hand-run proof and no automated coverage -- the same shape of gap that let an unpatched node-pty go unnoticed until CI caught it. It needs a real second process, because the assertion is about what happens when that process is force-killed: a host in a kill-on-close job must strand neither its pty nor a grandchild spawned detached, which is the process a parent-pid walk cannot see. Runs in the Windows PR job alongside the per-pty and encoder suites, so both halves of the two-job design are now gated rather than asserted. * fix(windows): serialise host-job creation Two callers racing PtyAssignCurrentProcessToJob would each create a job, put the process in both, and leak the first handle -- and the handle is what keeps a kill-on-close job alive, so a leaked one is never released. 'Only JS calls it' is not a guarantee: a worker thread with its own N-API env shares these statics. Also records the ordering requirement it depends on. AssignProcessToJobObject adds only the named process; children inherit membership, but a pty that already exists does not join retroactively and would not be reaped. The daemon assigns at startup, before the ConPTY warmup and before any session, which is correct today and now stated rather than implied. * fix(daemon): keep the host job off the startup path Assigning the host job at daemon startup resolves the node-pty native module, which loads the ConPTY addon -- and paying that before the endpoint is published delayed readiness enough that daemon-boot-smoke failed on windows-latest, deterministically. windows-conpty-warmup already carries the comment for this exact hazard ('setImmediate keeps the ready/handshake path ahead of the warm-up') and I put an eager load in front of it anyway. Moved to the pty spawn path, which already pays ConPTY cost, and memoised. Children inherit job membership, so assigning immediately before the first spawn still covers every pty -- and nothing can spawn one before the endpoint exists. |
||
 Hwanseok Choi
|
bb8c3b6360 | fix(agent-catalog): update outdated Claude Code doc links (#15387) | ||
  Jason BrashearandClaude Opus 5
|
b63292bee1 |
fix(editor): detect .liquid files as Liquid (#15213)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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. |
||
|
Jinjing
|
42a42281bd |
Remove agent map view from dashboard (#15853)
* Remove agent map view from dashboard Removes the view toggle and simplifies the dashboard to show only the kanban board layout. * Assert boardProps is initialized on drawer open |
||
|
Neil
|
a157f4cfec |
fix(worktree): drop a just-added fork remote when its head fetch fails (#15850)
Fork-PR setup adds the contributor's remote, then fetches the head. If that fetch fails the create aborts, but the remote stayed behind with no owner: cleanup only runs on worktree removal, and no worktree was ever created. Each retry then left another orphaned pr-* remote. Roll the remote back on fetch failure, on both the local and SSH paths, and only when this call is what added it -- a reused remote (Orca-created or not) is left alone. |
||
 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) |
||
|
Jinwoo Hong
|
e7b047f53a | fix(codex): suppress false restart notices after reauth (#15835) | ||
 Yeray
|
013eb539d5 | Change 'SEÑOR' to 'MR' in Spanish locale (#15594) | ||
|
Jinwoo Hong
|
da6b9d8065 | fix(terminal): stop orphaning live agent terminals across host restarts and graph syncs (#15644) | ||
|
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" |
||
|
Jinwoo Hong
|
8462fa72da | fix(rate-limits): support Codex 0.149 approval policy (#15823) | ||
|
Jinwoo Hong
|
1ce2e562b3 | fix(skills): isolate concurrent upload staging (#15693) | ||
|
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> |
||
  VincentandOrcaWin
|
59892a2f13 |
fix(terminal): preserve Shift+Enter during routing confirmation (#13598)
* fix(terminal): preserve Shift+Enter during routing confirmation Keep previously trusted CSI-u routing active only while local foreground revalidation is pending. Clear pending authority on inconclusive reads and never promote display-only identity.\n\nRefs #12541\nRefs #13597 * fix(terminal): respect global WSL routing gate * fix(terminal): bound the retained routing capability to a live read Two holes in the previous commit let one provider read authorize CSI-u indefinitely. First, routingConfirmationPending satisfied its own precondition, so a pending entry re-published itself on every reconfirmation request. Only one of the five callers is the Shift+Enter burst timer; the others fire on accepted submit/interrupt bytes, focus and visibility changes, and onAgentExited -- the last of which runs precisely when a shell title proves the agent is gone. On cmd.exe and Git Bash there is no OSC boundary to publish over the entry, so nothing decayed it. Second, the flag was published even when no confirmation read was actually scheduled -- a hidden pane, a command read already in flight, or a null pty id -- and only the read's inconclusive settle clears it. Require routingTrusted to grant the capability, and publish the flag only after sampling reports a read in flight, so it cannot outlive the read that justifies it. This also makes the canConfirmRouting gate redundant: the tracker already refuses WSL, SSH and remote pty ids. * review round 2: bound the retained capability to any in-flight read onVisiblePtyBound refuses to schedule while a higher-authority command read owns the pane, so gating the pending publish on it mistook 'a command-finished read already owns this' for 'no read at all' and dropped CSI-u for >=350ms — the window #13598 exists to close. Ask the tracker whether any read is in flight instead, and settle the visible confirmation from the command-finished branch that publishes nothing, so the flag cannot outlive the read that justifies it. * review round 2: tighten the reconfirmation gate and its comments hasReadInFlight already covers the visible-pty read that visibleForegroundSamplePending tracked, so the disjunction was redundant. Collapse the stacked Why blocks into one. * review round 3: release the retained capability when a read is abandoned Every outcome path now clears routingConfirmationPending, but the two abort guards in readForeground return without publishing or settling. A pty rebind during the multi-second inspection RPC — a detach/remount emits no onExit, so the store entry survives — therefore stranded the flag permanently, leaving Shift+Enter on CSI-u with nothing left to revalidate it. Worse, once sampling is suppressed by a live hook row the pane routes bytes on hook evidence alone, which the resolver excludes precisely because PTY output can forge it. Settle an abandoned read unless a newer generation will settle for it. * review round 4: release the retained capability at every exit without a successor Rounds 2 and 3 closed the outcome paths and the aborted-read paths, but two review lanes independently proved a third: cancelPendingRead bumps the generation, so a cancel never settles, and dispose plus the three untrackable early returns schedule no successor to settle for them. The store entry outlives the tracker — detach/remount emits no PTY exit — so the flag latched there with no read left alive. A visible remount self-heals, but a hidden pane does not: the dashboard card resolves the encoding for any pane key regardless of visibility. Release the capability from whichever exit ends the read, and say in the entry's own doc comment that the flag is Shift+Enter-scoped. * test(terminal): pin the remaining capability-release exits dispose and the command-finished exit were covered; the visible-bind and command-start variants are the same three-line pattern and were not. Neutering the release now kills all four. --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> |