mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
f5fd7303ab00bcfeff72c92f2bc33ba9364cd622
9102
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jinjing
|
f5fd7303ab |
test(e2e): cover tab-bar agent launches on Windows and WSL (#16110)
* test(e2e): gate the tab-bar agent launcher on Windows shells and WSL The `+` menu agent launcher had no golden coverage in the Windows lane, so a Windows-only break anywhere in its chain (detection row, startup-plan build, tab create, PTY spawn, startup-command injection) could ship unnoticed. Adds a golden spec that launches a stub agent from the menu and asserts the agent's own banner reached the pane — a tab that spawned a bare shell instead is indistinguishable at the store/tab layer. Runs two agents everywhere, and on Windows also PowerShell, cmd, Git Bash and a WSL project runtime. * test(e2e): track WSL stub agent staging state for precise cleanup Refactor `stageWslGoldenStubAgent` to track which artifacts it creates during setup, then only remove those artifacts during cleanup. This prevents the test from destructively removing pre-existing symlinks or state from previous runs, improving test isolation and idempotency. * test(e2e): track WSL stub agent staging state for precise cleanup - Back up and restore pre-existing stub agents to avoid destroying them - Simplify verbose test comments to match project style guidelines * test(e2e): serialize WSL stub agent setup with distributed lock - Add mkdir-based lock to prevent concurrent staging invocations - Reclaim stale locks after 10 minutes to recover from crashes - Track lock ownership in stage state for safe cleanup * test(e2e): track WSL stub agent staging state for precise cleanup Track which stubs this test helper stages by writing a marker file, then only remove stubs during stale-lock recovery if we created them. Prevents cleanup from removing stubs left by other processes. |
||
 Neil
|
afd76a4df9 | fix(terminal): preserve synchronized frames on reveal (#16026) | ||
|
Neil
|
95633a7883 |
Fix stale task-source flashes in new workspace input (#16145)
* fix(new-workspace): prevent stale GitHub URL selection * fix(new-workspace): guard all task URL transitions * test(e2e): make task URL frame proof runner-safe * fix(new-workspace): guard Enter during task URL lookup |
||
 Jinwoo Hong
|
4ee41fede2 | fix(automations): reveal full prompt from detail view (#16067) | ||
 Brennan Benson
|
55258f34ad |
test(agent-status): characterize title-derived agent identity before the resolver change (#16144)
* test(agent-status): characterize title-derived agent identity before the resolver change getAgentLabel is an ordered first-match-wins scan of substring predicates over a display title, so chain position rather than evidence strength decides identity. Pin the current answers — including the wrong ones — so the resolver change lands as a reviewable diff of assertions instead of silent behavior drift. Eight of the nineteen assertions record defects. Five are minimized from real recorded pane titles: four Grok panes that read as Codex and one that reads as Gemini CLI, in every case because a foreign agent name in free-form task text is checked before the `- <agent>` owner suffix that actually names the pane. The suite also pins the pairwise property behind them — both orderings of a name pair resolve to the same agent, which is the tell that the title carries no signal distinguishing them. Also pinned as correct so the resolver does not regress them: hyphenated worktree names (`review-14600-codex`) stay unclassified, and a Claude glyph still wins over foreign task text. Verified non-vacuous: applying PR #15535's narrowing to isGeminiTerminalTitle flips exactly four assertions, one of them a real corpus title, and the suite is green again on revert. No production code changes. * test(agent-status): re-pin the four assertions #15535 changed #15535 landed the Antigravity narrowing, so four characterized answers moved. Re-pinned against the new main rather than deleted, and the two that are now correct say why they are correct — a targeted exception cleared the path, not a structural fix. Added the general form as a new defect case: the same Grok pane without the word "Antigravity" in its task text still reads as Gemini CLI, because only that one pair has an exception. That is the case the resolver has to answer without a per-competitor clause. * test(agent-status): clarify characterization precedence |
||
|
Brennan Benson
|
4bc9e6b00c |
fix(workspace-cleanup): stop a wheel tick setting a filter, and land every filter patch (#15298)
A profile carried `activity.idleMinDays = 20` that the user never set, hiding 253 of 799 workspaces on open. Chromium mutates a *focused* number input on every wheel tick, and before #14629 the facet panel could not scroll, so the natural response -- cursor into the panel, spin the wheel -- walked the threshold up and persisted it. Two fixes: - `FacetNumberField` renders `type="text" inputMode="numeric"`. A wheel cannot mutate a text input, and the parser already takes strings. `preventDefault` on a focused number input would also work, but it blocks the wheel's default action -- which includes scrolling the nearest scrollable ancestor -- and would re-break the panel scrolling #14629 just fixed, in exactly the reported gesture. All five numeric facets share this one field. - `patchFilters` closed over the render's `browse` snapshot, so two patches in one tick dropped one. It now writes through a functional update against the latest store state. `toggleSortField` and `clearFilters` had the same defect. Both tests were confirmed to fail against the unfixed source before being kept. |
||
|
Neil
|
d9aa09cc8c |
fix(runtime): retire terminal snapshot probes before waiter timeout (#16149)
* refactor(renderer): use terminal paste sanitizer directly * fix(runtime): order visible probe timeout settlement * fix(runtime): skip unorderable visible probes |
||
|
Brennan Benson
|
41ddcbde06 |
fix(workspace-cleanup): stop pre-selecting workspaces for deletion (#15152)
* fix(workspace-cleanup): stop pre-selecting workspaces for deletion 'Ready' is Orca's verdict about the user's own work. The dialog already refuses to display the tier as a workspace fact, then pre-checked rows based on it anyway — acting on the verdict more loudly than showing it would. Open with an empty selection and let the user decide. Removing auto-select unmasks two selection defects it was hiding, both fixed here because they become reachable the moment select-all is the primary path: - The header checkbox compared a canQueue-scoped selected count against a canSelect-scoped selectable count, so hand-picking review-tier rows flipped it to fully-checked and the next click cleared the entire selection. - A filter change silently dropped selected rows; auto-select used to refill them instantly, so the loss was invisible. It is now reported. Also drops the scan toast's 'N cleanup suggestions' clause, which published the same verdict outside the dialog, and labels select-all with the count it actually takes (the deletable subset, not every matched row). * chore(i18n): sync the catalog for the cleanup selection strings * fix(workspace-cleanup): preserve explicit selections |
||
|
Brennan Benson
|
4c44116197 |
fix(workspace-cleanup): name the local context instead of totalling it (#15070)
* fix(workspace-cleanup): name the local context instead of totalling it
The delete confirmation showed 'Context: 2', a sum of five unrelated things
(terminal tabs, clean editor tabs, browser tabs, diff notes, finished agents)
rendered as plain text with no icon or tooltip. A reader cannot tell what the
number counts, which is the one thing that screen exists to tell them.
Reuse the breakdown the expanded row already renders ('Terminal tabs: 1,
Browser tabs: 1') so the confirmation names what deleting would discard. No new
strings: the per-kind labels already exist and were already translated.
* fix(workspace-cleanup): keep context labels legible
|
||
 erish
|
5bcbafff53 |
docs(cli): document worktree rm branch cleanup (#16167)
Document that Git worktree removal may also delete the checked-out local branch, while clarifying that --force does not force branch deletion and that Orca retains branches whose changes cannot be proven merged. |
||
|
Jinwoo Hong
|
0a613d5fed | test(e2e): stabilize paired Quick Open large-tree coverage (#16058) | ||
|
Jinwoo Hong
|
3d74f5fe03 | test(remote): preserve HTML inventory RPC failures (STA-5210) (#16056) | ||
|
Jinwoo Hong
|
6989c92b16 | fix(skills): recover upload admission lifecycle (#16057) | ||
|
Neil
|
0753f0a8dc | fix(crash-reporting): stamp React #185 boundary attribution as unreliable (#16153) | ||
|
Neil
|
0926c25854 | refactor: reuse canonical regex escaping (#16150) | ||
|
Neil
|
a7a4f5d987 | style(secrets): trim backend probe comments (#16136) | ||
|
Brennan Benson
|
ab3b1d07cd |
reland(opencode): session continuity without the command-finished deferral (STA-4557) (#15350)
* reland(opencode): session continuity without the command-finished deferral (STA-4557) Relands #14866 (reverted in #14943) minus its `orca-runtime.ts` change, which is what caused the revert. ## Why the original runtime change was wrong `retirePtyAgentLaunchAuthorityAfterCommandFinished` deferred launch-authority retirement behind an async foreground read, on the premise that OpenCode emits `command-finished` while still in the foreground. Raw PTY capture disproves it: OpenCode emits no OSC 133 of its own, and Orca's shell wrappers emit exactly one `133;D` per pane — at OpenCode's exit — under both zsh and bash. The event being deferred past only ever fires at exit, which is exactly when authority should be retired. Both call sites stay on the synchronous `retirePtyAgentLaunchAuthority`. ## Why the deferral was unsafe `confirmPtyAgentExit` uses the same async-foreground pattern four lines away, but its early return means "don't record an exit" — conservative. The deferral copied that shape into a site where the early return means "don't revoke a secret". Same code, inverted consequence: every guard failed open, so a stale or racing read silently kept a finished session's authority alive, and the pane's persisted `launchTokenHash` was never scrubbed — so it rehydrated as `restored` authority after an app restart. ## Why the deferral's guards could not have worked `ORCA_AGENT_LAUNCH_TOKEN` lives in the PTY environment, so every process started in that shell inherits it — both sessions in a reused pane post the same token. A pane-lifetime bearer secret cannot be a session identity baseline, by construction, and `incarnationId` tracks the PTY, not the agent. The only field that separates sessions is the provider `sessionID`. ## What lands - Status/session-boundary work from #14866: opencode emits `SessionStart` for root sessions (mimo-code does not), launch-token fencing, and `SessionStart` as an opencode turn boundary. - The two `server.ts` fixes from #14941: re-fence a still-authorized pane on a tokened `SessionStart`, and restore mimo-code's explicit-prompt restart boundary (mimo emits no `SessionStart`, so opencode-only stranded its panes). #14941's re-poll hunk is dropped along with the code it patched. - Five regression tests in `opencode-finished-session-authority.test.ts`. They pass here and all five go red if the deferral is re-added. * chore: drop incidental reformatting of files unrelated to this PR |
||
|
Jinwoo Hong
|
fe6f942d1f | Fix Ubuntu release dependency lockfile gate (STA-5109) (#16055) | ||
|
Neil
|
b89b58bf77 | fix(renderer): keep navigation responsive under terminal load (#16138) | ||
|
Neil
|
f0c718ecd5 | refactor(renderer): use canonical attachment labels (#16135) | ||
|
Neil
|
9814813b2e | refactor(skills): centralize observation matching (#16134) | ||
|
Neil
|
21c134bb66 | refactor(mobile): reuse repository color helper (#16131) | ||
|
Brennan Benson
|
da57e10dd9 |
fix(agent-status): stop an Antigravity pane reading as Gemini CLI (#15535)
Antigravity's models are named "Gemini <n.n> <Name>" — the real `agy models` output is already parsed in commit-message-agent-spec.test.ts — so an agy pane's own title carries a whole `gemini` token. getAgentLabel checks Gemini CLI before Antigravity, first match wins, so the model name won and the pane read as Gemini CLI. Measured on '⠋ agy · Gemini 3.7 Flash · high': geminiGlyphs false, geminiToken true, agyToken true, label 'Gemini CLI'. Even 'Antigravity · Gemini 3.7 Flash' resolved to Gemini CLI. This surfaced as the tab bar and the sidebar disagreeing about the same pane, because the two reach different copies of the chain and apply different precedence to its result. Defer only the token path: if a title carries an agy/antigravity token, the bare-`gemini` branch declines. The four Gemini OSC glyphs stay decisive, and agy emits none of them. Same shape as the existing isPiAgentTitle veto directly above, which exists because substring matching made paths like 'gemini-project' masquerade as Gemini CLI. Narrowing the token rather than reordering the chain, deliberately: a real recorded pane title from local terminal history is 'STA-4011 Linux Antigravity Commit Messages - grok' — a Grok pane whose task text contains the token Antigravity. It resolves correctly only because grok is checked before antigravity, so hoisting the Antigravity branch would break it. That title ships as a regression case. Both copies of the chain are fixed; the sidebar reaches one and the tab the other, so fixing one alone would only move the disagreement. |
||
|
Neil
|
8af02d658c |
Preserve code editor selections across tab switches (#16132)
* Preserve editor selections across tab switches * Defer editor selection caching to tab lifecycle |
||
|
Jinjing
|
7a72f341f7 |
Split pty-connection.ts into focused modules under 400 lines (#15166)
* refactor: split pty-connection.ts under 400 lines * rm design doc * refactor(pty-connection): extract reattach payload handlers as factories - Replace bindApplyReattachPayload with createReattachPayloadHandlers factory that returns handlers instead of mutating session directly, enabling better composability and testing - Extract waitForUserInitiatedSshConnect as standalone function for reuse across deferred session attach flows - Create ReattachPayloadSession type to document and isolate required session capabilities - Add test coverage for overlapping reattach payload attempts - Clean up comments to remove redundant prefixes (session.pane → pane, session.transport → transport) * fix(pty-connection): correct sequencing and state bugs in spawn and reat - Fix terminal tail slice to take prefix instead of suffix, preserving escape sequence markers needed by next scan - Clear pending pane serializer when direct SSH retry PTY is unclaimed - Initialize interrupt status baseline to undefined so first input advances sequence counter - Bump reattach generation only after confirming current attempt owns the stream, preventing superseded results from canceling in-flight prepaint * fix(pty-connection): correct sequencing and state bugs in spawn and reat - Fix terminal tail slice to take prefix instead of suffix, preserving escape sequence markers needed by next scan - Clear pending pane serializer when direct SSH retry PTY is unclaimed - Initialize interrupt status baseline to undefined so first input advances sequence counter - Bump reattach generation only after confirming current attempt owns the stream, preventing superseded results from canceling in-flight prepaint * fix(test): increase poll iterations to prevent Node 26 test leakage Increase event loop turns from 40 to 200 in the timer settlement loop. Node 26's libuv poll phase can briefly starve when concurrent workers transform tests, causing cleanup to leak into the next test. The higher iteration count ensures async operations complete before returning. * fix(foreground-output-budgets): use >= for budget window boundary check At the exact window boundary, the budget should roll over. Change the comparison from > to >= so the window resets when now equals windowStart + FOREGROUND_BUDGET_WINDOW_MS, not just after. Add tests to verify budget rejection and rollover behavior. * refactor(pty-connection): add status observations and routing improvemen - Track agent status observations with origin and transition metadata - Separate interactive redraw input timing from general terminal input - Restore pane authority on bind and reattach - Refine routing trust and confirmation state handling - Invoke queued startup callbacks when PTY is bound - Resolve Windows shell overrides with user settings * refactor: extract resolveLaunchAgentCandidate helper Consolidate duplicated launch-agent resolution logic into a shared helper to prevent future divergence between paneExpectsLaunchAgent and resolveExpectedLaunchTuiAgent. * refactor(pty-connection): use model snapshot for direct SSH reconnects Direct SSH reconnects now restore from the full SSH model snapshot (complete scrollback) when dimensions are compatible, instead of the bounded relay tail. Falls back gracefully when incompatible or alternate-screen was exited. * refactor(pty): retry unverifiable SSH reattaches via preserved bindings Preserve deferred SSH session IDs longer when they serve as the only retry binding, allowing the system to attempt recovery through direct SSH retries or PTY remounts when reattach fails in an unverifiable way. Simplify reconnect model restoration by removing the conditional model snapshot probe and using relay replay directly. * test: poll terminal readiness in expectSingleOwningPty Retry the terminal list assertion with polling to account for timing delays in PTY state reporting from the runtime. |
||
|
Brennan Benson
|
677718c4a5 |
fix(codex): stop rebuilding shared Codex state from a read that failed (STA-4823) (#15417)
* fix(codex): stop rebuilding shared Codex state from a read that failed (STA-4823) Six shared files were rebuilt, erased or reported healthy after a read that had only failed. Batch A of the STA-4606 split: every one of these is reachable and testable on the host lane, so none of them wait on the WSL work. - `config-toml-trust.ts` upsertHookTrustEntries: `existsSync` reported a locked config.toml as absent, so the base content became '' and the upsert rewrote the file from the trust entries alone — a trust-only stub, with the user's model, provider, MCP servers, approvals and comments gone. It refuses now; every hook-service caller already turns that into "trust entries could not be written. Run /hooks in Codex to approve." - `codex-trust-grant-ledger.ts`: an unreadable ledger degraded to empty and the next write persisted a file holding only the home being written, dropping every other home's grants. The write paths refuse; the read path still degrades, and a corrupt ledger is still rebuilt. - `codex-pane-account-registry.ts`: an unreadable registry erased every pane's attribution AND cached that erasure, so it survived the file recovering. The failure is no longer cached, and both write sites refuse rather than persist a registry derived from an empty stand-in. - `hooks-json-read.ts`: the read arm already separated "no hooks" from "could not read", but the `existsSync` arm in front of it returned a valid empty config for a file that could not be opened. One read now classifies both. - `config-settings-baseline.ts`: absent, unparseable and unreadable all collapsed into `null`, so the snapshot rebuilt a baseline it could not read — recording an in-Codex edit as Orca's own write, after which promotion skips it forever. - `config-sync-stall.ts`: an unreadable runtime config read as absent and the status reported `synced` while the mirror was refusing. It reports `managed-home-unavailable`, the existing reason for exactly this, rather than borrowing a source-side one and blaming the wrong path. Absent and malformed still rebuild throughout — resetting corrupt state is the intent, and conflating it with unreadable would wedge a user on a broken file. * fix(codex): close shared state read-denial gaps * fix(codex): recover oversized settings baselines * fix(codex): name the stalled managed config * fix(codex): preserve hooks after failed source reads * test(codex): correct what the denyExistence rig actually models MEASURED on both platforms: a file-permission denial leaves existsSync TRUE and fails only the content read — chmod 000 gives EACCES on macOS, icacls /deny (R) gives EPERM errno -4048 on Windows, with stat/lstat succeeding in both. The rig's docblock claimed this mode modelled that denial. It does not. What it models is the UNC / \\wsl$ transport, where an unreachable distro reports errno UNKNOWN at every level and existsSync folds it to false. The distinction decides what the D29 guard is worth: under a permission denial the pre-fix code already failed safe, because existsSync was true so it took the read branch and threw. Only a transport that lies about existence reaches the rebuild-from-empty path. No behaviour change; the comment was wrong, not the code. * test(codex): exercise live baseline read denial * fix(codex): retry pane attribution writes * fix(codex): retry reconciliation registry writes * fix(codex): report unreadable sync baselines |
||
|
Brennan Benson
|
d1127b9939 |
fix(agent-hooks): revive a retired pane on each provider's own new-turn event (#15344)
* fix(agent-hooks): revive a retired pane on each provider's own new-turn event The un-retirement gate matched two raw event-name literals, UserPromptSubmit and SessionStart. Only 5 of 18 hook sources name their turn boundary that way, so for the rest a reused pane stayed rowless forever: the user starts a new turn and no row ever appears in the sidebar or dashboard. Measured, not estimated — 10 providers fail the new test on main: gemini (BeforeAgent), antigravity (PreInvocation), amp (agent.start), cursor (beforeSubmitPrompt), pi/omp/prime-agent (before_agent_start), grok (user_prompt_submit), copilot (sessionStart, seen raw), hermes (pre_llm_call). The gate was also wrong in the other direction: opencode has no turn boundary at all, yet a literal SessionStart revived its pane. Both directions are covered. The correct per-provider classifier, isNewTurnEvent, was already imported into this file and already used 160 lines below — #14706 added that call specifically so consumers would stop re-deriving boundaries from literals, and left this one. Keeps the literal check when the remote envelope omits source: that field is optional, an older relay does not send it, and requiring it would have left every source-less remote pane retired forever. * fix(agent-hooks): tell an absent source apart from an unrecognized one Review of the first commit found that `isAgentHookSource(envelope.source) ? envelope.source : undefined` collapses two different wire conditions into one: an older relay that omits `source`, and a NEWER host relaying a provider this build does not know. They need opposite answers. Case two is the normal upgrade order — hosts and clients update independently — and its boundary event will not be UserPromptSubmit/SessionStart, because 13 of the 18 providers we already ship are named something else. So the legacy-literal fallback stranded that provider's panes permanently: the exact defect this branch fixes, silently reintroduced for traffic our own wire doctrine calls normal. Pass the raw wire value so the gate can distinguish them, and fail OPEN on an unrecognized provider. The costs are asymmetric: a stranded pane is invisible, permanent, and has no user-facing recovery, while a spurious revive produces a row that decays after AGENT_STATUS_STALE_AFTER_MS. Also from review: - Cover the source-less branches, which had no coverage at all — including a test pinning what the legacy shim CANNOT do, so nobody later widens the literal list to "fix" it. - Use the source as agentType rather than always 'claude'; 15 of 16 rows were describing a state that cannot occur. - Reword the opencode comment: no plugin in that family emits these literals, so this closes a hole rather than removing behavior. Checked and not changed: the local HTTP path 404s an unresolvable source, so the fallback is unreachable there and masks nothing. * test(agent-hooks): stop pinning opencode's fence behavior on a synthetic event The negative case asserted that an opencode pane stays retired after a literal SessionStart. That rests on a false premise: origin/main's opencode plugin never emits SessionStart at all — zero occurrences in opencode/hook-service.ts, and the shared family source is what mimo-code uses too. So the assertion pinned an event no plugin sends, and it would have become actively wrong the moment a pending change gives opencode a real SessionStart, silently re-breaking the rowless reused-pane case that change exists to fix. Assert against mimo-code and command-code instead. Both genuinely have no boundary event in any planned state, so the case tests what it claims to. * fix(agent-hooks): reject malformed relay sources at retired fence Only a non-empty unknown string can identify a future provider. Keep null, blank, numeric, and object source values behind the retired-pane fence instead of treating malformed wire data as a new turn. |
||
  Isaque Coelhoandm4air
|
853afdf80e |
fix(status-bar): remove pet menu reserved space (#13067)
* fix(status-bar): remove pet menu reserved space * test(status-bar): add pet segment layout validation tests - Unit test guards against pr-[6.5rem] padding reintroduction - E2E test measures trailing overhang instead of total width delta for more accurate layout validation - Extract enableExperimentalPet helper for test clarity --------- Co-authored-by: m4air <m4air@m4airs-Air.localdomain> |
||
 github-actions[bot]
|
e3327c2f31 | Update README downloads badge | ||
|
Jinjing
|
e50cc309c3 |
fix(runtime): prevent restored workers from appearing idle while busy (#15998)
* fix(runtime): classify tui-idle from the visible screen only The adopted-PTY tui-idle probe added in #15569 read the provider snapshot as `scrollbackAnsi + data`, and the Codex readiness classifier matches the startup banner. For a daemon-hosted adopted worker — where the retained tail stays empty forever — every wait re-probed and could resolve `satisfied: true` off banner history while Codex was actively working, turning a loud timeout into a silent false ready. - probe now requests and parses the visible grid, never scrollback - retirement of a timed-out provider acquisition is checked before the re-acquire branch, so a wider row request can no longer resurrect a hung provider - probe builds its result before clearing the poll interval, so a stale handle cannot leave the waiter with neither poll nor probe Fixture follow-ups from the same review: - resume legs pin the captured `launchConfig.agentCommand` to the fake instead of bare `codex`, which resolved the machine's real Codex off PATH - the command override is quoted for the Windows shell the runtime will actually use, and specs pin that shell alongside the override - fake agents acknowledge a bare submit after a short grace, so an unbracketed delivery path fails with a diagnosable ACK instead of a suite timeout Refs STA-4907, STA-4885 * test: assert tui-idle probes serialize visible grid only - Verify idle timeout probes exclude scrollback from serialization - Add test case for Git Bash shell path quoting with apostrophes - Simplify verbose test helper comments * test: improve fake agent paste protocol validation Refactor paste end detection to properly track both begin and end markers, validate bracketed paste protocol (RFC 2544) through chronological event sequencing, and emit correct error messages for protocol violations. This ensures reliable detection of when pastes complete even when delivered across multiple chunks, and correctly distinguishes between bracketed and unbracketed paste modes. * fix(runtime): reject provider snapshots when live output advances Provider snapshots become stale when live output is received after the snapshot is requested. Reject snapshots where the current output sequence exceeds the snapshot sequence, preventing callers from consuming outdated terminal state. Add tests verifying stale frame rejection. |
||
|
Neil
|
c3a1694b1d |
perf(preflight): read the WSL mount table once, and make launch agree with detection (#16053)
* perf(preflight): read the WSL mount table once per shell, not once per CLI
The prelude is embedded in the lookup script, and the caller wraps that in
`for cmd in <every agent>`, so the unconditional assignment forked awk once per
probed CLI -- 36 of them inside the distro against a 10s detection budget. The
comment claimed it was read once outside the loop; it was not.
`${x+set}` rather than `[ -n ... ]`: a host with no Windows mounts yields the
empty string, which must still count as read.
Pinned by counting real awk forks through /bin/sh with a stub on PATH, because
nothing covered this expression at all -- a wrong-field mutation shipped green.
Verified to bind: the unconditional form counts 4 for 4 commands.
* fix(wsl): make launch resolve the same binary detection reported
Agent detection skips Windows mounts during the PATH walk; the Codex WSL
command builder and the WSL branch of isCommandOnPath did not. So Orca could
report the guest codex as installed and then launch the Windows one sitting
ahead of it on PATH, or disagree with itself between preflight and detection
about the same distro.
Both now pass the same option.
Verified on a real Windows host against a real WSL2 distro, with a Windows
binary planted ahead of a guest one on PATH:
plain `command -v orcaprobe` -> /mnt/c/Users/neil/orca-agree/orcaprobe
this lookup -> /home/neil/.orca-agree/bin/orcaprobe
That host reports /mnt/c as 9p, which the mount expression matches, so the
fstype list is confirmed against hardware rather than fixtures.
* test(preflight): prove the memoised mount list applies past the first command
Counting awk forks with a stub that reports no mounts cannot see what the
hoist trades correctness for. A mutant that empties `_orca_win_mounts` inside
the walk keeps the fork count at 1 and keeps every existing test green, while
every agent after the first stops skipping /mnt.
This runs two commands behind a stubbed Windows mount and asserts both resolve
to the guest binary. Verified against that exact mutant.
Credit: review counsel.
|
||
 NeilandMelih
|
b2902cb61e |
fix(agent-resume): restore Kimi Code sessions after restart (#15883)
Co-authored-by: Melih <mberatsanli@gmail.com> |
||
  OrcaWinandOrcaWin
|
4c984d4c1b |
Fix Windows Git Bash console-capacity failures (#16045)
* fix(terminal): retain failed local console panes * fix(terminal): preserve failed pane restart context * fix(terminal): scope capacity recovery to PTY binding --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> |
||
|
Neil
|
e26f849682 |
fix(wsl): budget the whole command line, not just the script (#16032)
* fix(wsl): budget the whole command line, not just the script The argv/stdin threshold measured `script.length`, but the cap applies to the finished command line -- which also carries `PATH=<login PATH>` and `HOME=`. A login PATH is itself a few KB. That produced a perverse band: with a long enough PATH, a 7,999-char hook was placed on argv and CreateProcess refused it, while the SAME hook at 8,001 chars flipped to stdin and ran. Size decided how a hook behaved, in the wrong direction, and the failure looked like "your setup hook failed" with nothing pointing at length. Now the argv form is built, measured, and only used if the whole line fits; otherwise the script goes to stdin as before. The count over-estimates slightly (it charges quoting for every argument) because over-counting is the safe direction for a cap. The regression test uses a 7,000-char script -- deliberately under any script-only threshold -- with a 27KB PATH, and asserts it lands on stdin. My first attempt used 7,999 + `echo `, which is 8,004 and flipped under the old rule too, so it passed either way and proved nothing. Credit: Grok. * fix(wsl): charge quoting and measure the line that is actually spawned Two under-counts the review found in the estimator I added. The doc comment claimed it over-counts. It did not: libuv escapes every `"` and doubles a backslash run before a quote, so a quote-dense script costs more than its length. And `wsl.exe` plus `-d <distro> --exec` are prepended AFTER the measurement, so ~45 characters of the budget were never counted. Together those put a quote-heavy ~26KB script on argv and over the real 32767 ceiling -- where the old script-only rule would have sent it to stdin and it would have run. A narrower band than the one this PR removes, but the same shape of bug, so worth closing before merge rather than after. Now charges one character per `"` or backslash and measures the full spawn line. New test: 26,000 quote characters must land on stdin; verified to fail with the quoting charge removed. |
||
|
Neil
|
92315c4178 |
fix(preflight): do not count a Windows binary reached through interop as a WSL install (#16028)
* fix(preflight): do not count a Windows binary reached through interop as a WSL install WSL appends the Windows PATH to the guest PATH by default, so on a distro with no guest `claude`, `command -v claude` resolves to `/mnt/c/Users/me/.../claude.exe`. That path is POSIX-absolute, so the existing absolute-path check accepted it and preflight reported the agent as installed in the distro. That is worse than reporting it absent. Absent tells the user to install it; a false positive launches a Windows executable inside a Linux session, where it sees Windows paths, no guest $HOME and none of the distro's config -- and the failure surfaces later, somewhere less obvious. Rejects `/mnt/<drive>/` and any `.exe`, case-insensitively. A genuine guest install is unaffected. * fix(preflight): skip Windows mounts during the PATH walk, not after it The review caught this and it is the more important half of the fix. Rejecting the interop path in TypeScript happens after the guest walk has already stopped on it: the lookup breaks at the first executable, and the version-manager fallback dirs are APPENDED, so they sit behind the Windows entries WSL appends. A user with claude in nvm AND on the Windows PATH therefore went from a false positive to "not installed" -- the exact #9725 population the fallback dirs exist to serve. Worse than the bug being fixed. The lookup now takes `skipWindowsMountDirs` and skips those PATH components mid-walk, so the guest binary behind the shadow is still found. Matched by mount metadata from /proc/mounts (drvfs/9p/virtiofs), not by a `/mnt` name: the automount root is configurable, and `/mnt` is an ordinary directory on a Linux box. That also closes the custom-root hole the reviewers found in the name-based predicate. The TypeScript check stays as a secondary net for a mount the guest does not report, with a comment saying why it must never be the thing that decides. Proven with a real /bin/sh: a Windows `claude` ahead of an nvm `claude` on PATH now resolves to the nvm one. Credit: review counsel, and community PR #12794 (spfcraze), which proposed this shape first. * fix(preflight): let the mount table be the only word on what is a Windows path The name-based check could veto a path the walk had deliberately kept. /mnt/d is a perfectly ordinary Linux mount, so a guest binary there was resolved correctly by the walk and then discarded by its name -- the #9725 false negative, reintroduced by the belt-and-braces net I added "just in case". And if awk were missing, the name rule became the only rule, which is precisely the failure it was supposed to backstop. The walk skips components the guest itself reports as drvfs/9p/virtiofs. That is authoritative. Without a mount table we now degrade to main's behaviour (the old false positive) rather than inventing a new false negative. Net: one predicate, three fixtures and an import deleted. |
||
|
Neil
|
0e9f02baf3 |
fix(settings): an empty agent detection must not erase the saved default (#16043)
Three bugs in one screen, reported in #15256 with a diff of the user's orca-data.json showing defaultTuiAgent going from "claude" to null. 1. The Auto pill's handler writes null, and it was rendered as the ACTIVE choice whenever the stored agent was merely not detected right now. So the pill that already looked selected was destructive: one click erased the setting, and a later successful detection did not bring it back. Auto is now active only when null is actually stored. Detection is a transient fact; the stored value is not, and this control reports the stored value. 2. With zero agents detected there were no agent pills at all, so the stored choice was both invisible and unrecoverable -- nothing to click to put it back. The stored agent is now always offered, labelled as saved but not currently detected. 3. Refresh lived inside the Installed section, which only renders when at least one agent was found, so the only retry control vanished in exactly the state that needs it. An empty result now renders its own Refresh. This matters more now than when it was filed: #16028 makes WSL detection legitimately return an empty set on machines where the only agent was a Windows binary reached through interop, so the empty path is about to get more traffic. Each of the three tests was verified to fail with its own fix reverted. |
||
|
Neil
|
7a739c6bf5 |
fix(secrets): probe the Linux-only storage backend defensively (#16046)
`safeStorage.getSelectedStorageBackend` is `@platform linux`, so it is genuinely undefined on macOS and Windows — confirmed against the installed Electron 43, where it reads `undefined` on darwin and `function` on Linux. The shipped code called it behind a `process.platform === 'linux'` check, so it never threw, but the guard was the only thing standing between that call and a startup TypeError. The platform check now lives with the probe, alongside a typeof check and a try/catch, and an unreadable or unknown backend reports no gap — claiming one we cannot prove would be its own kind of lie. The gap this closes is in the tests, not just the code: every suite here mocks safeStorage with the method present, so the suite could stay green while the shipped app threw. The new case deletes the member from the live mock rather than re-mocking, because the module already holds that object and a later vi.doMock is inert — the first version of this test passed against the unguarded code, which is the failure mode it exists to catch. Verified against the expression currently on main: three cases fail. |
||
|
Neil
|
1375c57b16 |
fix(secrets): report the protection gap on change, not on every launch (#16044)
The gap warning fired every startup with no way to stop it. It usually needs a keyring installed and unlocked to fix, so repeating it every launch is nagging the user cannot act on and will learn to ignore. It now reports when the answer changes: once when the gap starts being true, again if it becomes true for a different reason, and once when it is fixed — because silence after a "your secrets are not protected" warning would leave the user assuming that is still the case. State lives beside the profile data file, which is why the call moved out of the port bootstrap: that state has nowhere to live until the profile exists. A corrupt state file re-reports rather than trusting it, and a failed write logs instead of failing startup, since re-reporting next launch is the safe direction. ORCA_ALWAYS_REPORT_SECRET_PROTECTION=1 forces a re-report for support without disturbing the stored state. |
||
 Neilandhwantage
|
202d74a8a4 |
fix(git): enable Windows long paths for worktree creation (local, sparse, and SSH hosts) (#15866)
Co-authored-by: hwantage <hwantagexsw2@gmail.com> |
||
 Neilandvam
|
af2e825626 |
fix(worktree): stop warning about a stale local base branch that does not exist yet (#15331) (#15871)
Co-authored-by: vam <a@a.com> |
||
 Neilandkriptoburak
|
8d1557c225 |
fix(skills): discover Hermes home skills (#15862)
Co-authored-by: kriptoburak <kriptoburak@users.noreply.github.com> |
||
|
Brennan Benson
|
9ea2636b38 |
fix(remote): keep editor focus after unfocused HTML side preview (STA-5001) (#15716)
Opening a remote HTML preview to the side created an empty split and made it the active group. The next host session-tab snapshot still had the terminal active, so the client treated that empty group as a terminal focus change. Do not activate the empty split for unfocused remote previews, and when a reserved preview group is still empty, keep the sibling editor as the visible tab instead of following the host terminal. |
||
|
Neil
|
0cb076f03c | fix(dictation): address visualizer review feedback (#16035) | ||
|
Neil
|
838f5bfb75 |
fix(secrets): tell Linux users when their secrets are only obfuscated (#16033)
On Linux with no keyring, Electron falls back to the `basic_text` backend, which "encrypts" with a hardcoded password. `isEncryptionAvailable()` returns true for it, so Orca reported those secrets as sealed. They are not. The obvious fix — returning false for basic_text — is wrong and would have been a credential regression: `decryptWithStatus()` skips decryption entirely when encryption is unavailable, so every already-stored secret would read back empty. Sealing genuinely works on basic_text and must keep working. So capability and trust are now separate questions. `isEncryptionAvailable()` still answers "can this host seal and unseal", and `describeProtectionGap()` (renamed from `describeUnavailable`) answers "is my data actually protected", covering both no-sealing and weak-sealing. That method had no production caller — the port documented a promise nothing kept. `reportSecretProtectionGap()` now reads it at startup. A user-visible surface is follow-up; this at least stops the silence. Adds a bootstrap wiring guard over all nine host port installs. The no-op defaults are correct for a renderer-less host and silently wrong for the desktop, and a dropped or reordered install fails no existing test. Verified in both directions: it fails when an install is removed, and when one moves after the runtime is constructed. |
||
|
erish
|
b851a5e13f |
fix(github): add GHES avatar fallback to PR and task views (#13981)
* fix(pr-page): route remaining user avatars through GitHubUserAvatar On a private-mode GitHub Enterprise instance the stored avatar URL 302s to /login, and the renderer's default Electron session carries no cookie, so the image never loads. #8784 added GitHubUserAvatar for exactly this — it degrades to an initials placeholder via onError — but three call sites in PullRequestPage kept a bare <img>: the reviewer picker, the comment author, and the @ mention suggestions. Each only guarded on avatarUrl being absent, so on GHE the URL is present, the placeholder branch never runs, and a broken image is left on screen. The authorAvatarUrl type comment already documents the intended contract ("falls back to the login URL and finally an initials placeholder"). GitHubUserAvatar was already imported in this file for the PR author, so this makes all five avatars in the page consistent. Note the three switched slots now carry the shared border/bg styling, matching the two that already did. Add a boundary test that fails if any avatar is rendered through a bare <img> again. Fixes #13976 * fix(task-page): route GitHub avatar cells through GitHubUserAvatar too Auditing the rest of the GHE avatar path turned up the same bare <img> in TaskPage: GitHubAssigneeAvatar, GHAssigneesCell and PRReviewCell. Fixing only the PR page would leave half of #13976 in place. GitHubAssigneeAvatar is the clearest case — ReviewChipAvatar directly above it already renders through GitHubUserAvatar, so two adjacent functions disagreed on how a GitHub user avatar is drawn. Its border also moves from border-border/40 to /50, matching the neighbour. Linear member avatars in this file are left alone; they use their own provider path and are out of scope here. Move the regression assertions into the existing repro-8784 file rather than a new boundary test — that file already guards PullRequestPage and TaskPage together, so it is where this belongs. The PR-page check matches the <img> pattern instead of specific field names, so a rename or a newly added avatar slot cannot slip past it; the TaskPage check is scoped per function to avoid catching the Linear cells. * test(github): scope the avatar guard per call site and cover TaskPage names Addresses review feedback on the regression guard. The field-name regex missed aliases and resolver expressions, and the PR-page assertion did not require GitHubUserAvatar in each migrated slot — deleting all three would have passed. Reject any bare <img> within the component scope instead, which is safe now that every assertion is scoped to one function. Drive all six slots from one table so each gets its own named case, and extend the display-name contract to TaskPage, which previously went unchecked. The ConversationTab entry carries displayName: null because PRComment has no display-name field. Reverting the fix now fails 11 cases instead of 3. |
||
|
Jinjing
|
36dfcc3955 |
fix(terminal): arm the restore baseline only on a snapshot that painted content (#16009)
The restored-snapshot baseline permanently drops every delivery chunk at or below the snapshot's seq, on the model's claim that those bytes are already painted. reconcileChunkAgainstRestoredSnapshot recovers when the baseline under-reports (a gap ahead re-restores; a rawLength mismatch re-restores) but has no path for a baseline that over-reports: those chunks return drop-duplicate forever, and an idle shell never re-sends them. Gate arming on whether the snapshot painted printable cells. A snapshot claiming seq > 0 while painting nothing cannot be the rendering of the output it claims to cover, so the claim is disproven and the redelivery is the only remaining copy. Also pins disposeHeadlessTerminal's two-part write ordering, which was previously unpinned and silently reversible. Refs STA-5179 |
||
|
Neil
|
03fcfdfb92 |
feat(orcad): boot the Orca runtime on plain Node (#15968)
* 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.
* feat(orcad): boot the Orca runtime on plain Node
Closes the last two Electron couplings and makes `orcad` a working artifact:
a 4.43 MB Node bundle that boots, pairs, registers a repo, creates a real git
worktree and round-trips a PTY — with zero `require("electron")`.
Ratchet 2 -> 0, so `config/runtime-electron-baseline.txt` is now empty and its
test asserts exactly that: any reachable electron import is a regression.
- speech: inject the service factories, so importing ModelManager for its type
no longer drags Electron's streaming net.request into the graph
- filesystem-watcher: add a WorktreeWatcherRemoval port. Every entry in those
maps arrives through an ipcMain handler carrying a renderer sender, so a host
with no renderer has nothing to close, restore or forget — the inert default
is what the desktop code does against empty maps, not a stub hiding work
- user-data-path / profile-storage-paths: resolve userData through
AppEnvironment. These surfaced only once orcad pulled the store in
Both host ports now anchor to a realm-global symbol. `vi.resetModules()` gives
the re-imported graph a fresh module copy, so a binding installed before the
reset silently read back as uninstalled.
The acceptance smoke drives both hosts through one code path (`--target
orcad|electron`) and seeds its own git repo, so it is hermetic and asserts the
same contract of each. Wired into PR CI.
* test(smoke): remove the seeded workspace container, not just the worktree
* test(smoke): surface the server's stderr when it dies before ready
* fix(smoke): build node-pty for Node before booting orcad in CI
* fix(smoke): drive the CLI built from this checkout, not one on PATH
* docs(ratchet): say the baseline must stay empty, not merely shrink
* build(orcad): externalize only the native modules actually in the graph
|
||
|
Neil
|
dee48498b2 |
feat(dictation): add sound-reactive grape visualizer (#16017)
* feat(dictation): add sound-reactive grape visualizer * perf: scope dictation meter updates |
||
|
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 |