mirror of https://github.com/stablyai/orca.git synced 2026-09-22 08:02:28 +00:00

Files

T

Neil 13ba649c22 fix(terminal): let a runtime-created Windows terminal BE the requested shell (#20825 )

* fix(terminal): let a runtime-created Windows terminal BE the requested shell

`orca terminal create --environment <windows-host> --command 'cmd.exe'` never
created a cmd terminal. `--command` is text the provider TYPES into whatever
shell it spawned, so the PTY stayed the host's default shell with cmd running
inside it. Captured on `awin`, whose default is Git Bash:

    $ orca terminal create --environment awin --command 'cmd.exe' --json
    $ orca terminal send --environment awin --terminal term_10656cf7... \
        --text exit --enter
    $ orca terminal read --environment awin --terminal term_10656cf7... --screen
      neil@awin MINGW64 ~/orca/orca ((30f820708f...))
      $ cmd.exe
      Microsoft Windows [Version 10.0.26200.9445]
      C:\Users\neil\orca\orca>exit
      neil@awin MINGW64 ~/orca/orca ((30f820708f...))
      $

The handle is alive the whole time and `terminal list` shows one healthy
terminal, because the PTY never changed — so the only symptom is that the
caller's terminal is now a shell it never asked for, and every later `send` is
quoted for the wrong one. On `win-lowspec` (default pwsh) the same create lands
cmd inside PowerShell.

Root cause
----------
There are two spawn preflights and they are twins:

- `src/main/ipc/pty/ipc/spawn-preflight.ts` — renderer/IPC spawns, i.e. a
  terminal tab opened in the app.
- `src/main/ipc/pty/runtime/spawn-preflight.ts` — runtime spawns: the CLI's
  `terminal.create`, headless `orca serve`, and every paired remote
  environment.

Only the IPC twin read the caller's requested shell. The runtime twin passed a
literal `requestedShellOverride: undefined`, so a runtime-created terminal on
Windows could only ever be the host default. Everything downstream of that
point — `spawn-options`, the daemon, `resolvePtyShellOverride` in the relay,
`local-pty-launch-plan` — already honoured `shellOverride`; nothing upstream
could supply one.

Change
------
- Thread `shellOverride` through the runtime lane: `RuntimePtySpawnArgs` ->
  runtime `spawn-preflight` -> `RuntimePtyController.spawn` ->
  `TerminalCreateOptions` -> the `terminal.create` RPC's new `shell` param ->
  `orca terminal create --shell`.
- Thread it through the renderer-backed lane too (`createDesktopTerminal` ->
  `terminal:requestTabCreate` -> `store.createTab`), so `--shell --focus` is not
  silently dropped on a local Windows app.
- An agent launch quotes its startup command for the shell it will actually run
  in, so a requested shell now owns the startup-shell family instead of the
  global `terminalWindowsShell` setting.
- Lift the relay's `ALLOWED_WINDOWS_SHELL_OVERRIDES` into
  `isSupportedWindowsShellOverride` in `src/shared/windows-terminal-shell.ts`
  (membership unchanged) so the CLI, the zod param schema, and the relay refuse
  the same names. `--shell` therefore cannot carry a path or a command line into
  `pty.spawn`; only allowlisted bare shell names pass.
- Gate on `TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY`. An older host
  strips the unknown `shell` param and answers with a healthy terminal running
  its default shell — a reply indistinguishable from success — so the CLI
  refuses before creating anything rather than creating the wrong shell quietly.

`--shell` stays Windows-only; macOS and Linux hosts spawn the login shell and
the relay drops the value off win32 rather than honouring it half-way. A WSL
project runtime still outranks it, unchanged.

Tests
-----
- `pty-spawn-shell-override-parity.test.ts` pins both preflights against the
  exact drift that caused this (verified failing with the fix reverted).
- `createTerminal` passes `shellOverride` to `ptyController.spawn` with no
  startup command.
- CLI: sends `shell`, refuses a shell the host cannot spawn, and refuses a host
  without the capability — in both refusals without making the round trip.
- Allowlist and `terminal.create` schema accept/refuse cases, including paths
  and appended arguments.

* fix(terminal): refuse a requested shell the execution host cannot apply

The first commit made `--shell` reach the spawn, but only a LOCAL win32
execution host applies it: `spawn-options` gates the override on
`process.platform === 'win32' && !args.connectionId`. So `--shell cmd.exe`
against an SSH-routed worktree, or against a macOS/Linux host, still returned a
healthy terminal running that host's default shell — the same
indistinguishable-from-success reply the capability gate exists to prevent, one
layer down.

Refuse instead, before anything spawns. The check sits at the top of
`resolveAgentTerminalCreateOptions`, which every create lane funnels through, so
neither lane has to remember it; the desktop lane additionally refuses a
worktree-less create, which has no execution host to resolve a shell on.

An SSH host's platform and installed shells are not visible to this runtime, and
a POSIX host has no Windows shell to pick. Neither can honour the request, and
saying so is the whole point of the flag.

Docs and the CLI spec now say "refused", not "ignored".

* fix(terminal): refuse a shell that contradicts the project execution runtime

`resolveLocalWindowsTerminalRuntimeOptions` does not merely rank the project's
execution runtime above a per-terminal pick -- it REWRITES the pick, in both
directions, and says nothing:

- a WSL project forces `wsl.exe`, discarding `--shell cmd.exe`;
- a Windows-host project discards a WSL name and falls back to `COMSPEC`
  (`getHostShellForProjectRuntime`), so `--shell wsl.exe` spawns cmd. That is
  the common case, not an edge: `resolveProjectExecutionRuntime` resolves
  `windows-host` for every project that is not WSL, while a repo belonging to no
  project honours `wsl.exe` -- so the same flag behaved differently depending on
  whether the repo was in a project.

Either rewrite returns a healthy terminal running a shell the caller did not ask
for, which is the failure `--shell` exists to remove.

It also split an agent launch's quoting from the shell that receives it. The
previous commit made the startup-shell family follow the REQUESTED shell, so
`--shell wsl.exe --command codex` on a Windows-host project typed POSIX-quoted
launch args into cmd. Refusing the contradiction removes that case rather than
papering over it.

Refuse instead, alongside the SSH and non-Windows refusals, from the same
`resolveAgentTerminalCreateOptions` seam every create lane funnels through.

Also from review:
- the allowlist test looped the list against itself; spell the members out.
- the runtime spec case claimed to prove the pty's shell when it asserts the
  controller received the field; name it for what it checks.

Reported by an adversarial review of the branch.

* fix(terminal): canonicalize --shell and refuse a WSL-path rewrite

Review of the --shell create path turned up two ways the terminal could
still end up being a shell the caller never asked for -- the exact failure
--shell exists to remove.

Bare and mixed-case spellings passed the allowlist but reached consumers
that exact-match the canonical name: resolveWindowsShellStartupFamily
classified `cmd` as the PowerShell family, resolveWindowsShellLaunchArgs
fell through to empty shellArgs (no `chcp 65001`, no OSC 133 bootstrap that
Windows foreground status depends on), and resolveWindowsGitBashShellPath
compares case-sensitively so `Git-Bash` spawned a literal `Git-Bash`.
The allowlist is now one canonical-name map and terminal.create canonicalizes
on parse, so the spawn path only ever sees `.exe` spellings. `pwsh` and
`powershell` stay distinct binaries.

A `\\wsl$\<distro>\...` cwd made the providers force wsl.exe regardless of
the request, and terminalShellOverrideRefusal only inspected the project
runtime -- undefined for a folder workspace with no project. Refuse on the
resolved cwd and the workspace path, judging what the PTY actually gets.

Also: the capability gate reported an unreachable host as too old rather
than unavailable; the SSH CLI shim dropped capabilities from status, so
--shell there blamed the host version instead of naming SSH; and --shell
had no help entry, rendering bare in `orca terminal create --help`. Adding
that entry crossed help.ts's max-lines cap, so the flag table moved to
flag-help-text.ts rather than suppressing the rule.

Adds a behavioural test for the runtime preflight (the one-line fix was
pinned only by a source-text scan), plus coverage for the startup-command
quoting family, the no-workspace refusal, and the WSL-path refusal.

* fix(build): keep tests out of the RPC params catalog bundle

The catalog walk under methods/ already skips *.test.ts, but the contract
directory glob took every .ts. terminal-create-shell-param.test.ts is the
first test to live there, so the bundle pulled vitest into a CJS build and
the generator threw on require(). Same exclusion, same reason.

2026-09-15 16:34:16 -07:00

12 KiB

Raw Blame History

Design System

All UI work — layout, color, typography, spacing, component selection, UX behavior — must follow docs/STYLEGUIDE.md. Most of it is linted: pnpm run check:code-quality:changed fails on new restyles of a components/ui/ primitive, raw palette colors, and computed className strings; pnpm lint fails on any class Tailwind cannot generate. See the Enforcement section of the style guide before suppressing either. Use the tokens defined in src/renderer/src/assets/main.css (the canonical source) and the shadcn primitives in src/renderer/src/components/ui/. Don't invent new color values, font sizes, or shadow tiers when a documented one already covers the role. When STYLEGUIDE.md is silent, follow the resolution order in its final section.

Electron UI Validation

Always run tests and agent-launched apps in the background with ORCA_BACKGROUND_LAUNCH=1. Never steal monitor focus or reveal test windows: no show(), showInactive(), bringToFront(), app.focus(), or OS activation. Use CDP screenshots of hidden renderers. Keep native-focus and visible-window tests paused on the user's desktop; run them on an isolated display or CI. Rebuild modified launch-policy code before running an app; stale build wrappers are not safe.

Use the $electron skill and Playwright CDP for rendered Orca UI checks. Do not use computer-use for Orca UI validation.

Style

Reuse Before Reimplementing

Before writing new logic at any scale — a function, component, IPC channel, state store, or whole subsystem/flow — check whether an existing implementation already does the job (or nearly does). Extend or generalize it instead of building a parallel version; only write from scratch when nothing fits. Keep the check proportionate: a quick search for trivial code, a real one before building anything substantial.

Concise/Brief Non-obvious Comments ONLY

DO NOT: be verbose, explain the obvious, walk through the code ("WHY not HOW")
BE CONCISE. 1 LINE if possible

Lint Rules: Do Not Disable Max Lines

NEVER add a max-lines disable (eslint-disable max-lines, oxlint-disable max-lines, or line-specific variants), and never add a per-file max-lines bump in mobile/.oxlintrc.json.

File and Module Naming

Never use vague names like helpers, utils, common, misc, or shared-stuff for files, folders, or modules. They carry zero info and tend to become dumping grounds. Name files after what they actually contain — prefer the concrete domain concept (e.g. tab-group-state.ts, terminal-orphan-cleanup.ts) over the generic role (tabs-helpers.ts, terminal-utils.ts). If you find yourself reaching for helpers, the file probably has more than one responsibility and should be split, or there's a better name hiding in the code that describes what the functions operate on.

Type Declarations: Prefer `.ts` Over `.d.ts`

Type Assertions: Prefer Checked Types

Avoid type assertions except as const. Unavoidable casts need a line-specific SAFETY: explanation:

// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Explain the verified invariant here.

Verifying Changes

Typecheck: pnpm tc (or tc:node / tc:cli / tc:web)
Test: pnpm test [path/to/file.test.ts]
Lint: oxlint, or pnpm run check:code-quality:changed for changed files (full pnpm lint is slow); format with pnpm format
Design system: pnpm run lint:design-system for the full renderer report (not a gate); the changed-lines gate above is what CI enforces

Considerations

Worktree Safety

Always use the primary working directory (the worktree) for all file reads and edits. Never follow absolute paths from subagent results that point to the main repo.

Cross-Platform Support

Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior behind runtime checks:

Keyboard shortcuts: Never hardcode e.metaKey. Use a platform check (navigator.userAgent.includes('Mac')) to pick metaKey on Mac and ctrlKey on Linux/Windows. Electron menu accelerators should use CmdOrCtrl.
Shortcut labels in UI: Display ⌘ / ⇧ on Mac and Ctrl+ / Shift+ on other platforms.
File paths: Use path.join or Electron/Node path utilities — never assume / or \.
Windows terminal shells: --shell picks the shell a terminal is; --command is typed into whatever shell the host spawned, so a shell choice routed through command silently becomes a child process. See docs/reference/windows-terminal-shell-selection.md.
Windows setup scripts: the setup/issue-command runner is a .cmd batch file unless the script starts with a #! line — never derive that from the user's terminal-shell preference, and never launch a .cmd runner with a bare cmd.exe /c from a Git Bash pane (MSYS rewrites the /c). See docs/reference/windows-setup-shell.md.
Windows child processes: start them through runProcess/spawnProcess in src/shared/child-process/ — never child_process directly. It pins windowsHide, refuses shell: true, and encodes .cmd/.bat arguments so neither CommandLineToArgvW nor cmd.exe mangles them. A ratchet test fails on any new direct import. Recognised npm/pnpm .cmd shims are resolved to their real target so the spawn skips cmd.exe entirely; see docs/reference/windows-cmd-shim-resolution.md before adding a shim shape or debugging one.
Windows process enumeration: read the table through src/main/windows/windows-process-table.ts, never by forking powershell.exe. See docs/reference/windows-process-enumeration.md.
Windows daemon-host relocation: the terminal daemon runs from a copy of the app runtime under %LOCALAPPDATA%, which is what survives an auto-update. Before touching that copy, its exe name, or the NSIS uninstall macro, read docs/reference/windows-daemon-host-relocation.md.
Windows EDR signal: don't add -ExecutionPolicy Bypass, -EncodedCommand, cmd.exe /c with escaped free text, per-operation interpreter spawning, or runtime Add-Type compilation without reading docs/reference/windows-edr-posture.md first — behavioural EDR scores each of those, and being signed does not clear them.
WSL commands: build argv with buildWslExecArgs (always --exec — under --, wsl.exe expands $name in every argument and silently rewrites the script), and fence anything whose stdout you parse with buildWslCapturedLoginShellCommand, because the interactive login shell prints the distro banner to stdout. See docs/reference/wsl-command-execution.md.
Linux native modules: keep the glibc floor at Ubuntu 20.04 / glibc 2.31. A module compiled from source on a newer runner can reference symbol versions absent on the floor and crash the app on startup. See docs/reference/linux-glibc-compatibility.md; packaging fails if a bundled native binary needs newer glibc.

Native Dependency Installs

Ordinary pnpm install covers the host OS and CPU only. Before packaging for another architecture — including pnpm build:mac, which builds x64 and arm64 by default — run pnpm install:release. electron-builder only warns on a missing extraResources source, so the beforePack guard is what turns a thin install into a build failure instead of a silently broken artifact; see docs/reference/pnpm-install-policy.md.

SSH Use Case

All changes must consider the SSH use case. Don't assume local-only execution. Before changing anything that reports on, stops, or lists remote work, follow docs/reference/ssh-execution-boundary.md: the execution host owns everything that touches execution, and loss of contact is never evidence of process death — the verdict vocabulary is live / unverifiable / exited, with no synonyms.

Folder Workspace Use Case

All changes must consider folder workspaces as well as git worktrees. Don't assume every workspace is a git worktree.

Agent Status

The execution host owns agent status in one store, the hook server's, and every reader (sidebar, worktree ps, mobile, dashboard) subscribes to it. Before adding a producer, a cache, or a reader-side precedence rule, read docs/reference/agent-status-store.md: new producers write into that store, and readers keep only presentation policy.

Agent Terminal Screens

A rule that reads what an agent CLI paints on a terminal — readiness, blocked prompts, idle — must be written against a captured transcript, not a remembered screen. Record one with docs/reference/agent-pty-transcript-capture.md, which keeps escapes and wrapping intact and scrubs account identifiers before they reach git. Antigravity readiness has no transcript yet and five failed attempts without one; before touching it, read docs/reference/antigravity-readiness-evidence.md.

Remote Wire Compatibility

Clients and remote Orca servers update independently, so mixed versions are the normal state. Before changing anything a paired client and host exchange — RPC params, stream frames, or the content either side publishes over them — follow docs/reference/remote-wire-compatibility.md. A new optional field is safe; a new stream opcode must be capability-negotiated because decoders drop unknown opcodes silently; and changing what the host publishes reaches old clients even with no wire change.

Git Binary Compatibility

Orca runs the user's Git binary on native, WSL, and SSH hosts, which may all have different versions. Treat Git 2.25 as the core-workflow baseline and follow docs/reference/git-compatibility.md.

When adding or changing a Git command:

Check when every subcommand and option was introduced. For newer behavior, keep a baseline-compatible fallback or degrade safely.
Use GitCapabilityCache with a narrow unsupported-error predicate so recurring operations do not retry a known-invalid command. Do not rely only on git --version; wrappers such as simple-git do not remove host-version differences.
Scope capability state to the host that executes Git: native, WSL distro, SSH provider, or relay connection. Cover the first fallback, later cached calls, concurrent probes, and relevant host isolation in tests.
Keep the real-binary compatibility contract in PR CI current. When adopting a newer Git feature, add its version boundary so the preferred command and fallback both run against representative Git releases.
Preserve commands that begin with global Git options such as -c before the subcommand, including auto-maintenance suppression used by worktree-create fetches.

Git Scan Safety

Never enumerate every ref and then run git ls-tree -r or git show once per ref. That ref × tree fan-out can retain gigabytes of output before a downstream sort -u or search can make progress.
Prefer rg over the checked-out files for source searches. For history or refs, use a named ref, an explicit namespace/path, --max-count, and a bounded output; do not use an unqualified --all scan as a first diagnostic.
Keep repository-wide commands targeted to the current repository and worktree. If an unbounded scan is genuinely required, measure the ref count first, explain the cost, and get confirmation before running it.

Git Provider Compatibility

Source-control and review changes must consider GitLab and other supported git providers, not only GitHub. Keep provider-specific behavior behind explicit checks, and avoid GitHub-only naming for generic review concepts.

GitHub CLI Usage

Be mindful of the user's gh CLI API rate limit — batch requests where possible and avoid unnecessary calls. All code, commands, and scripts must be compatible with macOS, Linux, and Windows.

12 KiB Raw Blame History Unescape Escape