From 13ba649c2244a57733712766e6fdfe7f2bfe1a17 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:34:16 -0700 Subject: [PATCH] fix(terminal): let a runtime-created Windows terminal BE the requested shell (#20825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(terminal): let a runtime-created Windows terminal BE the requested shell `orca terminal create --environment --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$\\...` 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. --- .gitignore | 1 + AGENTS.md | 1 + .../scripts/generate-rpc-params-catalog.mjs | 6 +- .../windows-terminal-shell-selection.md | 54 +++++ pnpm-workspace.yaml | 2 +- src/cli/flag-help-text.ts | 106 +++++++++ src/cli/handlers/terminal.test.ts | 95 +++++++- src/cli/handlers/terminal.ts | 36 ++++ src/cli/help.ts | 110 +--------- src/cli/root-help-text-secondary.ts | 2 +- src/cli/specs/core.ts | 11 +- .../pty-spawn-shell-override-parity.test.ts | 26 +++ .../spawn-preflight-requested-shell.test.ts | 83 +++++++ src/main/ipc/pty/runtime/spawn-preflight.ts | 7 +- src/main/ipc/pty/runtime/spawn-state.ts | 1 + .../orca-runtime-create-terminal-desktop.ts | 8 + .../runtime/orca-runtime-create-terminal.ts | 1 + ...runtime-resolve-worktree-removal-target.ts | 23 +- .../terminal-creation-and-readiness.spec.ts | 126 ++++++++++- .../terminal/terminal-lifecycle-methods.ts | 1 + .../runtime-pty-controller-contract.ts | 2 + .../runtime/runtime-terminal-contracts.ts | 9 + ...rminal-shell-override-host-support.test.ts | 204 ++++++++++++++++++ .../terminal-shell-override-host-support.ts | 90 ++++++++ src/main/ssh/ssh-remote-orca-cli.test.ts | 30 +++ src/main/ssh/ssh-remote-orca-cli.ts | 5 +- src/relay/pty-handler.ts | 21 +- .../ipc-events/terminal-request-ipc-bridge.ts | 2 +- src/shared/protocol-version.ts | 6 + .../terminal-create-shell-param.test.ts | 51 +++++ .../rpc-contract/terminal-unary-params.ts | 18 +- src/shared/runtime-terminal-contracts.ts | 2 + .../source-scan/source-tree-walk.test.ts | 1 - src/shared/windows-terminal-shell.test.ts | 87 +++++++- src/shared/windows-terminal-shell.ts | 44 ++++ 35 files changed, 1129 insertions(+), 143 deletions(-) create mode 100644 docs/reference/windows-terminal-shell-selection.md create mode 100644 src/cli/flag-help-text.ts create mode 100644 src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts create mode 100644 src/main/ipc/pty/runtime/spawn-preflight-requested-shell.test.ts create mode 100644 src/main/runtime/terminal-shell-override-host-support.test.ts create mode 100644 src/main/runtime/terminal-shell-override-host-support.ts create mode 100644 src/shared/rpc-contract/terminal-create-shell-param.test.ts diff --git a/.gitignore b/.gitignore index 3d51edb0009..97fc6affcac 100644 --- a/.gitignore +++ b/.gitignore @@ -130,6 +130,7 @@ docs/** !docs/reference/ssh-host-key-verification.md !docs/reference/ssh-reconnect-source-recovery.md !docs/reference/windows-setup-shell.md +!docs/reference/windows-terminal-shell-selection.md !docs/reference/worktree-scan-fingerprint.md !docs/reference/wsl-command-execution.md !docs/reference/wsl-probe-failure-semantics.md diff --git a/AGENTS.md b/AGENTS.md index 0c11d13a9ca..664d986da13 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,6 +61,7 @@ Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior beh - **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`](./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`](./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`](./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`](./docs/reference/windows-process-enumeration.md). diff --git a/config/scripts/generate-rpc-params-catalog.mjs b/config/scripts/generate-rpc-params-catalog.mjs index 9810f952e77..bd7ddcd53f6 100644 --- a/config/scripts/generate-rpc-params-catalog.mjs +++ b/config/scripts/generate-rpc-params-catalog.mjs @@ -40,8 +40,12 @@ const repoPath = (absolute) => posix(path.relative(REPO_ROOT, absolute)) // Every module the catalog may import from: the extracted params modules plus the // pre-existing src/shared schemas the RPC methods already bind directly. function indexableModules() { + // Tests are excluded here for the same reason as the RPC_DIR walk below: bundling one pulls + // vitest into the CJS catalog build, which throws on require(). const modules = new Set( - globSync('*.ts', { cwd: CONTRACT_DIR }).map((name) => path.join(CONTRACT_DIR, name)) + globSync('*.ts', { cwd: CONTRACT_DIR }) + .filter((name) => !name.endsWith('.test.ts')) + .map((name) => path.join(CONTRACT_DIR, name)) ) modules.delete(OUTPUT_PATH) for (const file of globSync('**/*.ts', { cwd: RPC_DIR })) { diff --git a/docs/reference/windows-terminal-shell-selection.md b/docs/reference/windows-terminal-shell-selection.md new file mode 100644 index 00000000000..b0e361c5447 --- /dev/null +++ b/docs/reference/windows-terminal-shell-selection.md @@ -0,0 +1,54 @@ +# Windows terminal shell selection + +Two different things can put `cmd.exe` on a Windows terminal, and only one of them makes the +terminal _be_ cmd. + +- **`--shell` / `shellOverride`** names the executable the PTY is spawned as. The terminal's own + process is that shell for its whole life. +- **`--command` / `startupCommand`** is text the provider types into whatever shell it spawned. + `--command cmd.exe` therefore starts cmd as a **child** of the host's default shell. + +The difference is invisible until the child exits. Leaving that cmd returns the caller's handle to +a Git Bash or PowerShell prompt it never asked for, and anything that keyed off "this terminal is +cmd" is now wrong — while `terminal list` still shows one connected, healthy terminal, because the +PTY never changed. + +## Why the runtime path needed its own fix + +There are two spawn preflights, and they are twins: + +- `src/main/ipc/pty/ipc/spawn-preflight.ts` — renderer/IPC spawns (a terminal tab in the app). +- `src/main/ipc/pty/runtime/spawn-preflight.ts` — runtime spawns: `terminal.create` from the CLI, + headless `orca serve`, and every paired remote environment. + +Only the IPC twin read the caller's requested shell. The runtime twin passed a literal `undefined`, +so a runtime-created terminal could only ever be the host's default shell. `orca terminal create +--command cmd.exe` against a Windows environment had no way to say "be cmd" — it could only type +`cmd.exe` into Git Bash. `src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts` pins the pair. + +## Rules + +- A caller choosing a shell passes `--shell`; a caller running a program passes `--command`. Do not + route a shell choice through `command` — it looks like it worked. +- The allowlist is `isSupportedWindowsShellOverride` in `src/shared/windows-terminal-shell.ts`, and + it is the reason `--shell` cannot name an arbitrary executable. The CLI, the `terminal.create` + RPC schema, and the relay all check the same set; add a shell in one place only. +- Bare shell names only. A path or anything with arguments is refused, so `--shell` can never carry + a command line into `pty.spawn`. +- A host that predates `--shell` STRIPS it (`terminal.create` params are a zod object, which drops + unknown keys) and answers with a healthy terminal running its default shell — a reply that reads + as success. So the CLI gates on `TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY` and refuses + before creating anything, rather than creating the wrong shell quietly. +- `--shell` is Windows-only, and a host that cannot apply it REFUSES the create + (`terminalShellOverrideRefusal`). macOS and Linux execution hosts spawn the login shell, and a + terminal routed over SSH resolves its shell on the SSH host, whose platform and installed shells + this runtime cannot see. Refusing is the point: spawning the default shell and reporting success + is the failure `--shell` exists to remove. +- A project's execution runtime decides which MACHINE the shell runs on, so it outranks a + per-terminal pick — but it outranks it by REFUSING, not by rewriting. A `--shell` that + contradicts the project runtime (a Windows shell on a WSL project, or a WSL name on a + Windows-host project) is refused. `resolveLocalWindowsTerminalRuntimeOptions` would otherwise + rewrite the value — a WSL project forces `wsl.exe`, a Windows-host project discards a WSL name in + favour of `COMSPEC` — and hand back a terminal running something the caller never asked for. It + also splits an agent launch's quoting from the shell that receives it: POSIX-quoted args typed + into cmd, or cmd-quoted args typed into a WSL shell. diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 68ae103f6ec..92de08dfe22 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -54,7 +54,7 @@ allowBuilds: node-pty: true sherpa-onnx: true ssh2: false - "@orca/windows-registry": false + '@orca/windows-registry': false overrides: monaco-editor>dompurify: 3.4.14 diff --git a/src/cli/flag-help-text.ts b/src/cli/flag-help-text.ts new file mode 100644 index 00000000000..3cdee00060d --- /dev/null +++ b/src/cli/flag-help-text.ts @@ -0,0 +1,106 @@ +/** One-line flag descriptions shared by every command's help output. */ +export const FLAG_HELP_TEXT: Record = { + agent: '--agent Launch a known TUI agent in the first terminal', + 'base-branch': '--base-branch Base branch/ref to create the worktree from', + command: '--command Command to run in the terminal on startup', + comment: '--comment Comment stored in Orca metadata', + cursor: '--cursor Line cursor from a previous read (returns only new output)', + action: '--action Secondary accessibility action name', + activate: '--activate Reveal the new worktree in the Orca app', + app: '--app App name, bundle ID, or pid:N', + direction: + '--direction Direction: up|down|left|right for scroll, horizontal|vertical for split', + 'display-name': '--display-name Override the Orca display name', + 'element-index': '--element-index Element index from get-app-state', + title: '--title Custom title for the terminal tab (omit to reset)', + enter: '--enter Append Enter after sending text', + force: + '--force Force worktree removal when supported; does not force branch deletion', + focus: '--focus Reveal the created terminal session in Orca', + for: '--for exit|tui-idle Wait condition to satisfy', + 'from-element-index': '--from-element-index Source element index from get-app-state', + 'from-x': '--from-x Source window-local x coordinate', + 'from-y': '--from-y Source window-local y coordinate', + help: '--help Show this help message', + 'include-visual-layouts': '--include-visual-layouts Include tab and pane topology in JSON output', + interrupt: '--interrupt Send as an interrupt-style input when supported', + id: '--id Identifier for a target item or permission', + issue: '--issue Linked GitHub issue number', + 'linear-issue': + '--linear-issue Linked Linear issue identifier or URL; null clears on set', + json: '--json Emit machine-readable JSON', + key: '--key Key argument for this command', + limit: '--limit Maximum number of rows to return', + local: '--local Target the current project instead of the global install', + skill: '--skill Bundled skill to act on; repeat for several', + mode: '--mode Mode such as edit, diff, or both', + model: '--model Provider model id for a new agent launch', + effort: '--effort Reasoning effort for the selected model', + 'mouse-button': '--mouse-button Mouse button: left, right, or middle', + modifiers: '--modifiers Modifier keys held only for this click', + name: '--name Name for the new worktree or automation', + 'no-parent': '--no-parent Force no parent lineage for unrelated work', + 'no-screenshot': '--no-screenshot Skip screenshot capture after the operation', + pages: '--pages Number of scroll pages', + 'parent-worktree': + '--parent-worktree Parent worktree selector such as identity:, id:::, branch:, issue:, path:, or active/current', + path: '--path Path argument for the command', + prompt: '--prompt Prompt text for agent-backed commands', + query: '--query Search text for matching refs', + ref: '--ref Base ref to persist for the repo', + repo: '--repo Repo selector such as id:, name:, or path:', + 'restore-window': '--restore-window Bring the target app/window forward before the operation', + session: '--session Snapshot namespace for a related computer-use workflow', + setup: '--setup run|skip|inherit Setup policy for repo-defined setup hooks', + shell: '--shell Windows shell the terminal itself runs as', + terminal: '--terminal Runtime-issued terminal handle', + text: '--text Text payload to send or type', + 'text-stdin': '--text-stdin Read text payload from stdin', + 'task-id': '--task-id Task id to include in orchestration payload JSON', + 'task-title': '--task-title Concise title for an orchestration task', + 'dispatch-id': '--dispatch-id Dispatch id to include in orchestration payload JSON', + 'files-modified': '--files-modified Comma-separated files for orchestration payload JSON', + 'report-path': '--report-path Report path to include in orchestration payload JSON', + phase: '--phase Worker phase to include in orchestration payload JSON', + 'timeout-ms': '--timeout-ms Maximum wait time before timing out', + 'to-element-index': '--to-element-index Destination element index from get-app-state', + 'to-x': '--to-x Destination window-local x coordinate', + 'to-y': '--to-y Destination window-local y coordinate', + worktree: + '--worktree Worktree selector such as identity:, id:::, name:, branch:, issue:, path:, or active/current', + workspace: '--workspace Existing worktree selector for automation runs', + 'workspace-status': + '--workspace-status Board status id (defaults: todo, in-progress, in-review, completed)', + staged: '--staged Open staged source-control changes', + provider: '--provider Agent id such as codex, claude, or gemini', + 'source-context': + '--source-context Explicit TaskSourceContext for automation task/provider data', + trigger: '--trigger Automation schedule preset, cron, or RRULE', + schedule: '--schedule Alias for --trigger', + time: '--time Time used with daily/weekdays/weekly presets', + day: '--day <0-6> Day used with weekly preset, Sunday=0', + timezone: '--timezone IANA timezone for the automation', + enabled: '--enabled Enable the automation', + disabled: '--disabled Disable the automation', + 'reuse-session': + '--reuse-session Reuse the previous live session for existing-workspace runs', + 'fresh-session': '--fresh-session Disable session reuse for future runs', + 'workspace-mode': '--workspace-mode existing or new-per-run', + 'missed-run-grace-minutes': '--missed-run-grace-minutes Missed-run grace window', + 'value-stdin': '--value-stdin Read set-value payload from stdin', + 'window-id': '--window-id Target a window id from list-windows', + 'window-index': '--window-index Target a window index from list-windows', + // Browser automation flags + element: '--element Element ref from snapshot (e.g. e3)', + url: '--url URL to navigate to', + value: '--value Value to fill or select', + input: '--input Text to type at current focus', + expression: '--expression JavaScript expression to evaluate', + amount: '--amount Scroll distance in pixels', + index: '--index Tab index to switch to', + page: '--page Stable browser page id from `orca tab list --json`', + profile: '--profile Browser profile id', + 'show-profile': '--show-profile Include tab profile in text output', + 'no-ua-spoof': "--no-ua-spoof Keep Electron's native user agent", + format: '--format Screenshot image format' +} diff --git a/src/cli/handlers/terminal.test.ts b/src/cli/handlers/terminal.test.ts index 21f296b261a..90dbc7fd459 100644 --- a/src/cli/handlers/terminal.test.ts +++ b/src/cli/handlers/terminal.test.ts @@ -1,6 +1,9 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { RuntimeClientError, type RuntimeClient } from '../runtime-client' -import { TERMINAL_PROMPT_DELIVERY_RUNTIME_CAPABILITY } from '../../shared/protocol-version' +import { + TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY, + TERMINAL_PROMPT_DELIVERY_RUNTIME_CAPABILITY +} from '../../shared/protocol-version' import { parseArgs } from '../args' import { printHelp } from '../help' import { COMMAND_SPECS } from '../specs' @@ -685,3 +688,93 @@ describe('terminal send CLI', () => { ]) }) }) + +describe('terminal create --shell', () => { + const WORKTREE = 'path:C:/src/app' + + const shellClient = ( + call: ReturnType, + supported: boolean, + reachable = true + ): RuntimeClient => { + const client = { + call, + isRemote: false, + getCliStatus: vi.fn().mockResolvedValue({ + result: { + runtime: reachable + ? { + reachable: true, + runtimeId: 'runtime-current', + capabilities: supported ? [TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY] : [] + } + : { reachable: false, runtimeId: null } + } + }) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: `terminal create` reads only `call`, `isRemote`, and `getCliStatus`, all stubbed above; RuntimeClient is a class, so a structural double cannot satisfy it without the cast. + return client as unknown as RuntimeClient + } + + afterEach(() => { + vi.restoreAllMocks() + process.exitCode = ORIGINAL_EXIT_CODE + }) + + function createTerminal(client: RuntimeClient, shell: string) { + return TERMINAL_HANDLERS['terminal create']({ + flags: new Map([ + ['worktree', WORKTREE], + ['shell', shell] + ]), + client, + cwd: 'C:/src/app', + json: true + }) + } + + it('sends the shell selection alongside an empty startup command', async () => { + const call = vi.fn().mockResolvedValue({ + result: { terminal: { handle: 'term_1', worktreeId: 'repo::C:/src/app', title: null } } + }) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await createTerminal(shellClient(call, true), 'cmd.exe') + + expect(call).toHaveBeenCalledWith( + 'terminal.create', + expect.objectContaining({ shell: 'cmd.exe', command: undefined }) + ) + }) + + it('refuses a shell the host cannot spawn without making the round trip', async () => { + const call = vi.fn() + + await expect(createTerminal(shellClient(call, true), 'nu.exe')).rejects.toThrow( + /--shell must be one of/ + ) + expect(call).not.toHaveBeenCalled() + }) + + // An older host strips the unknown param and answers with its default shell, which reads as a + // successful create. Creating the wrong shell silently is worse than refusing. + it('refuses rather than creating a default-shell terminal on a host without the capability', async () => { + const call = vi.fn() + + await expect(createTerminal(shellClient(call, false), 'cmd.exe')).rejects.toThrow( + /does not support --shell/ + ) + expect(call).not.toHaveBeenCalled() + }) + + // A status probe that fails or times out reports no capabilities either; blaming the host + // version would send the caller to update a host that may already be current. + it('reports an unreachable host as unavailable rather than incompatible', async () => { + const call = vi.fn() + + await expect(createTerminal(shellClient(call, false, false), 'cmd.exe')).rejects.toMatchObject({ + code: 'runtime_unavailable' + }) + expect(call).not.toHaveBeenCalled() + }) +}) diff --git a/src/cli/handlers/terminal.ts b/src/cli/handlers/terminal.ts index c409a6a9f9a..2da49ad8ca8 100644 --- a/src/cli/handlers/terminal.ts +++ b/src/cli/handlers/terminal.ts @@ -31,6 +31,11 @@ import { type WithAnnotatedHostScope } from '../omitted-host-scope-selectors' import { RuntimeClientError } from '../runtime-client' +import { + isSupportedWindowsShellOverride, + listSupportedWindowsShellOverrides +} from '../../shared/windows-terminal-shell' +import { TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY } from '../../shared/protocol-version' import { getBrowserWorktreeSelector, getOptionalWorktreeSelector, @@ -153,9 +158,40 @@ export const TERMINAL_HANDLERS: Record = { const useRendererBackedInteractiveTerminal = !client.isRemote && shouldUseRendererBackedInteractiveTerminal(command) const focus = flags.get('focus') === true + const shell = getOptionalStringFlag(flags, 'shell') + if (shell !== undefined) { + if (!isSupportedWindowsShellOverride(shell)) { + throw new RuntimeClientError( + 'invalid_argument', + `--shell must be one of: ${listSupportedWindowsShellOverrides().join(', ')}` + ) + } + // Why refused rather than sent hopefully: an older host strips the unknown param and hands + // back a healthy terminal running its DEFAULT shell. Nothing in that reply says the shell + // was ignored, so a caller that wanted cmd would drive a PowerShell session believing it won. + const status = await client.getCliStatus() + // An unreachable host reports no capabilities at all; that is not evidence it lacks --shell. + if (!status.result.runtime.reachable) { + throw new RuntimeClientError( + 'runtime_unavailable', + 'Orca could not verify --shell support on the execution host, so no terminal was created. Wait for the execution host to become reachable and retry.' + ) + } + if ( + status.result.runtime.capabilities?.includes( + TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY + ) !== true + ) { + throw new RuntimeClientError( + 'incompatible_runtime', + 'This Orca host does not support --shell, and would silently create a terminal running its default shell instead. No terminal was created; update Orca on the execution host.' + ) + } + } const result = await client.call<{ terminal: RuntimeTerminalCreate }>('terminal.create', { worktree: await getBrowserWorktreeSelector(flags, cwd, client), command, + ...(shell !== undefined ? { shell } : {}), title: getOptionalStringFlag(flags, 'title'), // Why: interactive local agent TUIs need the renderer-backed terminal // path for browser-side features, but CLI creates must stay backgrounded diff --git a/src/cli/help.ts b/src/cli/help.ts index 9d722388ae4..79e5b2b92c4 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -1,6 +1,7 @@ import type { CommandSpec } from './args' import { findCommandSpec, isCommandGroup, supportsBrowserPageFlag } from './args' import { unknownCommandData } from './command-suggestion' +import { FLAG_HELP_TEXT } from './flag-help-text' import { formatSkillsCommandFlagHelp } from './skills-command-flag-help' import { ROOT_HELP_TEXT_PRIMARY } from './root-help-text-primary' import { ROOT_HELP_TEXT_SECONDARY } from './root-help-text-secondary' @@ -182,113 +183,6 @@ function formatCommandFlagHelp(flag: string, commandPath: string[]): string { } export function formatFlagHelp(flag: string): string { - const helpByFlag: Record = { - agent: '--agent Launch a known TUI agent in the first terminal', - 'base-branch': '--base-branch Base branch/ref to create the worktree from', - command: '--command Command to run in the terminal on startup', - comment: '--comment Comment stored in Orca metadata', - cursor: '--cursor Line cursor from a previous read (returns only new output)', - action: '--action Secondary accessibility action name', - activate: '--activate Reveal the new worktree in the Orca app', - app: '--app App name, bundle ID, or pid:N', - direction: - '--direction Direction: up|down|left|right for scroll, horizontal|vertical for split', - 'display-name': '--display-name Override the Orca display name', - 'element-index': '--element-index Element index from get-app-state', - title: '--title Custom title for the terminal tab (omit to reset)', - enter: '--enter Append Enter after sending text', - force: - '--force Force worktree removal when supported; does not force branch deletion', - focus: '--focus Reveal the created terminal session in Orca', - for: '--for exit|tui-idle Wait condition to satisfy', - 'from-element-index': '--from-element-index Source element index from get-app-state', - 'from-x': '--from-x Source window-local x coordinate', - 'from-y': '--from-y Source window-local y coordinate', - help: '--help Show this help message', - 'include-visual-layouts': - '--include-visual-layouts Include tab and pane topology in JSON output', - interrupt: '--interrupt Send as an interrupt-style input when supported', - id: '--id Identifier for a target item or permission', - issue: '--issue Linked GitHub issue number', - 'linear-issue': - '--linear-issue Linked Linear issue identifier or URL; null clears on set', - json: '--json Emit machine-readable JSON', - key: '--key Key argument for this command', - limit: '--limit Maximum number of rows to return', - local: '--local Target the current project instead of the global install', - skill: '--skill Bundled skill to act on; repeat for several', - mode: '--mode Mode such as edit, diff, or both', - model: '--model Provider model id for a new agent launch', - effort: '--effort Reasoning effort for the selected model', - 'mouse-button': '--mouse-button Mouse button: left, right, or middle', - modifiers: '--modifiers Modifier keys held only for this click', - name: '--name Name for the new worktree or automation', - 'no-parent': '--no-parent Force no parent lineage for unrelated work', - 'no-screenshot': '--no-screenshot Skip screenshot capture after the operation', - pages: '--pages Number of scroll pages', - 'parent-worktree': - '--parent-worktree Parent worktree selector such as identity:, id:::, branch:, issue:, path:, or active/current', - path: '--path Path argument for the command', - prompt: '--prompt Prompt text for agent-backed commands', - query: '--query Search text for matching refs', - ref: '--ref Base ref to persist for the repo', - repo: '--repo Repo selector such as id:, name:, or path:', - 'restore-window': - '--restore-window Bring the target app/window forward before the operation', - session: '--session Snapshot namespace for a related computer-use workflow', - setup: '--setup run|skip|inherit Setup policy for repo-defined setup hooks', - terminal: '--terminal Runtime-issued terminal handle', - text: '--text Text payload to send or type', - 'text-stdin': '--text-stdin Read text payload from stdin', - 'task-id': '--task-id Task id to include in orchestration payload JSON', - 'task-title': '--task-title Concise title for an orchestration task', - 'dispatch-id': '--dispatch-id Dispatch id to include in orchestration payload JSON', - 'files-modified': '--files-modified Comma-separated files for orchestration payload JSON', - 'report-path': '--report-path Report path to include in orchestration payload JSON', - phase: '--phase Worker phase to include in orchestration payload JSON', - 'timeout-ms': '--timeout-ms Maximum wait time before timing out', - 'to-element-index': '--to-element-index Destination element index from get-app-state', - 'to-x': '--to-x Destination window-local x coordinate', - 'to-y': '--to-y Destination window-local y coordinate', - worktree: - '--worktree Worktree selector such as identity:, id:::, name:, branch:, issue:, path:, or active/current', - workspace: '--workspace Existing worktree selector for automation runs', - 'workspace-status': - '--workspace-status Board status id (defaults: todo, in-progress, in-review, completed)', - staged: '--staged Open staged source-control changes', - provider: '--provider Agent id such as codex, claude, or gemini', - 'source-context': - '--source-context Explicit TaskSourceContext for automation task/provider data', - trigger: '--trigger Automation schedule preset, cron, or RRULE', - schedule: '--schedule Alias for --trigger', - time: '--time Time used with daily/weekdays/weekly presets', - day: '--day <0-6> Day used with weekly preset, Sunday=0', - timezone: '--timezone IANA timezone for the automation', - enabled: '--enabled Enable the automation', - disabled: '--disabled Disable the automation', - 'reuse-session': - '--reuse-session Reuse the previous live session for existing-workspace runs', - 'fresh-session': '--fresh-session Disable session reuse for future runs', - 'workspace-mode': '--workspace-mode existing or new-per-run', - 'missed-run-grace-minutes': '--missed-run-grace-minutes Missed-run grace window', - 'value-stdin': '--value-stdin Read set-value payload from stdin', - 'window-id': '--window-id Target a window id from list-windows', - 'window-index': '--window-index Target a window index from list-windows', - // Browser automation flags - element: '--element Element ref from snapshot (e.g. e3)', - url: '--url URL to navigate to', - value: '--value Value to fill or select', - input: '--input Text to type at current focus', - expression: '--expression JavaScript expression to evaluate', - amount: '--amount Scroll distance in pixels', - index: '--index Tab index to switch to', - page: '--page Stable browser page id from `orca tab list --json`', - profile: '--profile Browser profile id', - 'show-profile': '--show-profile Include tab profile in text output', - 'no-ua-spoof': "--no-ua-spoof Keep Electron's native user agent", - format: '--format Screenshot image format' - } - if (flag === 'current') { return '--current Use the current Orca worktree linked Linear issue' } @@ -314,5 +208,5 @@ export function formatFlagHelp(flag: string): string { return '--full Include all supported V1 issue context within caps' } - return helpByFlag[flag] ?? `--${flag}` + return FLAG_HELP_TEXT[flag] ?? `--${flag}` } diff --git a/src/cli/root-help-text-secondary.ts b/src/cli/root-help-text-secondary.ts index 324fe847855..7a892c94ec8 100644 --- a/src/cli/root-help-text-secondary.ts +++ b/src/cli/root-help-text-secondary.ts @@ -62,7 +62,7 @@ export const ROOT_HELP_TEXT_SECONDARY = [ ' orca terminal read [--terminal ] [--cursor ] [--limit ] [--json]', ' orca terminal send [--terminal ] [--text ] [--enter] [--interrupt] [--wait-submit ] [--retry-request ] [--json]', ' orca terminal wait [--terminal ] --for exit|tui-idle [--timeout-ms ] [--json]', - ' orca terminal create [--worktree ] [--title ] [--command ] [--focus] [--json]', + ' orca terminal create [--worktree ] [--title ] [--command ] [--shell ] [--focus] [--json]', ' orca terminal split [--terminal ] [--direction horizontal|vertical] [--json]', ' orca terminal switch [--terminal ] [--json]', ' orca terminal close ([--terminal ] [--tab] | --worktree --all) [--json]', diff --git a/src/cli/specs/core.ts b/src/cli/specs/core.ts index d029b0cd43d..98301b36e53 100644 --- a/src/cli/specs/core.ts +++ b/src/cli/specs/core.ts @@ -250,17 +250,20 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [ path: ['terminal', 'create'], summary: 'Create a terminal session in the current worktree', usage: - 'orca terminal create [--worktree ] [--title ] [--command ] [--focus] [--json]', - allowedFlags: [...GLOBAL_FLAGS, 'worktree', 'command', 'title', 'focus'], + 'orca terminal create [--worktree ] [--title ] [--command ] [--shell ] [--focus] [--json]', + allowedFlags: [...GLOBAL_FLAGS, 'worktree', 'command', 'shell', 'title', 'focus'], notes: [ 'Creates a visible terminal tab without switching focus when possible; falls back to a background handle if the UI cannot adopt it. Pass --focus to switch to it.', - 'Use this, not worktree create, for a fresh agent in the current checkout.' + 'Use this, not worktree create, for a fresh agent in the current checkout.', + '--shell picks the shell the terminal IS on a Windows host (cmd.exe, powershell.exe, pwsh.exe, wsl.exe, bash.exe, git-bash); --command is typed into whatever shell the host started, so `--command cmd.exe` leaves a cmd running INSIDE the default shell and exiting it drops back to that shell.', + 'A host that cannot apply --shell refuses the create rather than quietly spawning its default shell: macOS and Linux execution hosts spawn the login shell, terminals routed over SSH resolve their shell on the SSH host, a --shell that contradicts the project execution runtime (WSL vs Windows host) is refused, and an Orca host older than --shell is refused by the CLI.' ], examples: [ 'orca terminal create --json', 'orca terminal create --worktree active --command "codex" --json', 'orca terminal create --worktree path:/projects/myapp --title "RUNNER" --command "opencode"', - 'orca terminal create --worktree path:/projects/myapp --command "opencode" --focus' + 'orca terminal create --worktree path:/projects/myapp --command "opencode" --focus', + 'orca terminal create --worktree path:C:/src/app --shell cmd.exe --json' ] }, { diff --git a/src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts b/src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts new file mode 100644 index 00000000000..67bee9ca700 --- /dev/null +++ b/src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts @@ -0,0 +1,26 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +/** + * The two spawn preflights are twins: one serves renderer/IPC spawns, the other serves runtime + * spawns (`terminal.create` from the CLI, headless serve, and paired remote environments). They + * had drifted — the runtime twin passed a literal `undefined` for the caller's shell, so on a + * Windows host a runtime-created terminal could only ever be the host's default shell. A caller + * asking for cmd or PowerShell had to send it as `command`, which the provider TYPES into that + * default shell: the pty stayed the default shell with the requested one running inside it, and + * leaving that child dropped the caller's handle back onto a prompt it never asked for. + * + * Source-level because the functional seam is a whole spawn pipeline; what actually regressed is + * one twin silently not reading a field the other reads. + */ +const PREFLIGHTS = ['ipc', 'runtime'] as const + +describe.each(PREFLIGHTS)('%s pty spawn preflight', (lane) => { + const source = readFileSync(join(__dirname, lane, 'spawn-preflight.ts'), 'utf8') + + it("resolves Windows terminal runtime options from the caller's requested shell", () => { + expect(source).toContain('requestedShellOverride: args.shellOverride') + expect(source).not.toContain('requestedShellOverride: undefined') + }) +}) diff --git a/src/main/ipc/pty/runtime/spawn-preflight-requested-shell.test.ts b/src/main/ipc/pty/runtime/spawn-preflight-requested-shell.test.ts new file mode 100644 index 00000000000..46f16081efa --- /dev/null +++ b/src/main/ipc/pty/runtime/spawn-preflight-requested-shell.test.ts @@ -0,0 +1,83 @@ +import { afterEach, describe, expect, it } from 'vitest' +import type { BrowserWindow } from 'electron' +import { getDefaultSettings } from '../../../../shared/constants' +import { finishPtyShutdown } from '../provider/liveness' +import { prepareRuntimePtySpawn } from './spawn-preflight' +import { buildRuntimePtySpawnOptions } from './spawn-options' +import { createRuntimePtySpawnState, type RuntimePtySpawnArgs } from './spawn-state' +import type { PtyRuntimeControllerDeps } from './controller-deps' + +const HOST_DEFAULT_SHELL = 'powershell.exe' +const hostPlatform = process.platform + +function makeDeps(): PtyRuntimeControllerDeps { + const noCodexResumeLaunch: PtyRuntimeControllerDeps['noCodexResumeLaunch'] = (command) => ({ + codexResumeHome: null, + command, + notifyResumeUnavailable: false, + droppedResumeArgv: false, + providerSession: null + }) + return { + store: undefined, + getSettings: () => ({ + ...getDefaultSettings('/tmp'), + terminalWindowsShell: HOST_DEFAULT_SHELL + }), + adoptStablePane: async () => null, + getLocalPtyStartupPromise: () => undefined, + getLocalPtyProviderStartupPromise: () => undefined, + prepareCodexResumeHome: () => null, + resolveCodexResumeLaunch: async (command) => noCodexResumeLaunch(command), + noCodexResumeLaunch, + reconcileSharedRuntimeResumeHome: async (resumeHome) => resumeHome.codexHomePath, + stripSequencedStartupResumeArgv: (env) => env, + assertFolderWorkspacePtyPathUsable: () => undefined, + resolvePtySpawnStartupCwd: (_worktreeId, cwd) => cwd, + requestSerializedBuffer: async () => null, + shutdownProviderAndDetectExit: async () => false, + rememberSyntheticKillExit: () => {}, + rememberRetiredRejectedPty: () => {}, + sendPtyExitToRenderer: () => {}, + sendPtySpawnedToRenderer: () => {}, + finishPtyShutdown, + trustedTerminalHandleEnv: new Set(), + retiredRejectedPtyIds: new Map(), + reversibleStopOwnersByPtyId: new Map(), + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: only `operations.ts` (write/clearBuffer) reads `mainWindow`; the spawn preflight and option build never touch it, and a real BrowserWindow cannot exist in vitest. + mainWindow: {} as BrowserWindow + } +} + +/** Runs the preflight and option build the way `spawnPtyFromRuntimeController` sequences them. */ +async function resolveSpawnShell(shellOverride: string | undefined): Promise { + const args: RuntimePtySpawnArgs = { cols: 120, rows: 40, shellOverride } + const ctx = createRuntimePtySpawnState(makeDeps(), args) + await prepareRuntimePtySpawn(ctx) + await buildRuntimePtySpawnOptions(ctx) + ctx.finishTerminalInstall() + return ctx.spawnOptions.shellOverride +} + +/** + * Behavioural twin of `pty-spawn-shell-override-parity.test.ts`: a local Windows runtime spawn + * (`terminal create --shell`, headless serve) must hand the caller's shell to the provider, not + * the host default with the request typed into it. + */ +describe('runtime pty spawn preflight: requested shell on a local Windows host', () => { + afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: hostPlatform }) + }) + + it('spawns the requested shell as the pty', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + + await expect(resolveSpawnShell('cmd.exe')).resolves.toBe('cmd.exe') + }) + + it('keeps the host default shell when nothing was requested', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + + await expect(resolveSpawnShell(undefined)).resolves.toBe(HOST_DEFAULT_SHELL) + }) +}) diff --git a/src/main/ipc/pty/runtime/spawn-preflight.ts b/src/main/ipc/pty/runtime/spawn-preflight.ts index 43fd2778119..aa743b259d0 100644 --- a/src/main/ipc/pty/runtime/spawn-preflight.ts +++ b/src/main/ipc/pty/runtime/spawn-preflight.ts @@ -72,10 +72,15 @@ export async function prepareRuntimePtySpawn( throw new Error(CLAUDE_AUTH_SWITCH_IN_PROGRESS_MESSAGE) } // Why: runtime-created terminals carry no renderer-computed projectRuntime; resolve from worktreeId to honor the project's Windows runtime. + // `args.shellOverride` is the per-request pick (`terminal create --shell`), read here the way + // the renderer twin (ipc/spawn-preflight.ts) reads a tab's override. Without it a runtime create + // could only ever get the host default shell, so a caller asking for cmd/PowerShell got the + // default shell with the request typed into it. Still Windows-only: the override names a + // Windows shell, and spawn-options applies it under the same platform gate. ctx.terminalRuntimeOptions = process.platform === 'win32' && !args.connectionId ? resolveLocalWindowsTerminalRuntimeOptions({ - requestedShellOverride: undefined, + requestedShellOverride: args.shellOverride, settings: ctx.deps.getSettings?.(), projectRuntime: resolveLocalProjectRuntimeForWorktreeId(ctx.deps.store, args.worktreeId), fallbackHostShell: process.env.COMSPEC || 'powershell.exe' diff --git a/src/main/ipc/pty/runtime/spawn-state.ts b/src/main/ipc/pty/runtime/spawn-state.ts index 81878fa029b..2884dff5685 100644 --- a/src/main/ipc/pty/runtime/spawn-state.ts +++ b/src/main/ipc/pty/runtime/spawn-state.ts @@ -102,6 +102,7 @@ export type RuntimePtySpawnArgs = { tabId?: string leafId?: string sessionId?: string + shellOverride?: string isNewSession?: boolean persistHostSessionBinding?: boolean expectedSourceBinding?: PtyBindingSourceExpectation diff --git a/src/main/runtime/orca-runtime-create-terminal-desktop.ts b/src/main/runtime/orca-runtime-create-terminal-desktop.ts index 7e02a749447..9cd955428ee 100644 --- a/src/main/runtime/orca-runtime-create-terminal-desktop.ts +++ b/src/main/runtime/orca-runtime-create-terminal-desktop.ts @@ -18,6 +18,13 @@ export async function createDesktopTerminal( const launchOpts = workspace ? await runtime.resolveAgentTerminalCreateOptions(workspace, opts) : opts + // `resolveAgentTerminalCreateOptions` refuses an unapplicable shell, and it only runs with a + // workspace; a worktree-less create has no execution host to apply one to either. + if (!workspace && opts.shellOverride) { + throw new Error( + `--shell ${opts.shellOverride} needs a workspace, because the shell is resolved on the workspace's execution host. No terminal was created.` + ) + } const worktreeId = workspace?.id const cwd = workspace ? runtime.resolveWorkspaceTerminalStartupCwd(workspace, launchOpts.cwd) @@ -63,6 +70,7 @@ export async function createDesktopTerminal( ...(launchOpts.launchAgent ? { launchAgent: launchOpts.launchAgent } : {}), ...(launchOpts.viewMode ? { viewMode: launchOpts.viewMode } : {}), startupCommandDelivery: launchOpts.startupCommandDelivery, + ...(launchOpts.shellOverride ? { shellOverride: launchOpts.shellOverride } : {}), title: launchOpts.title, activate: presentation === 'focused', ...(presentation ? { presentation } : {}), diff --git a/src/main/runtime/orca-runtime-create-terminal.ts b/src/main/runtime/orca-runtime-create-terminal.ts index ec8a3e78941..5e7d4393a6d 100644 --- a/src/main/runtime/orca-runtime-create-terminal.ts +++ b/src/main/runtime/orca-runtime-create-terminal.ts @@ -146,6 +146,7 @@ export class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTerminalCreate preAllocatedHandle, tabId, leafId, + ...(launchOpts.shellOverride ? { shellOverride: launchOpts.shellOverride } : {}), ...(terminalColorQueryReplies ? { terminalColorQueryReplies } : {}), ...(launchOpts.agentSessionClaim ? { diff --git a/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts b/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts index 44f4524c782..795531bbc4a 100644 --- a/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts +++ b/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts @@ -16,6 +16,9 @@ import type { TerminalWorkspaceLaunchScope } from './runtime-legacy-worker-termi import type { TerminalCreateOptions } from './runtime-terminal-contracts' import { resolveLocalWindowsAgentStartupShell } from '../../shared/windows-terminal-shell' import { isTuiAgentEnabled } from '../../shared/tui-agent-selection' +import { terminalShellOverrideRefusal } from './terminal-shell-override-host-support' +import { resolveTerminalStartupCwd } from '../../shared/terminal-startup-cwd' +import { resolveLocalProjectRuntimeForWorktreeId } from '../local-project-runtime-resolution' import { resolveBareAgentLaunchCommand } from './runtime-agent-launch-resolution' import { buildAgentStartupPlan } from '../../shared/tui-agent-startup' import { @@ -152,6 +155,23 @@ export class OrcaRuntimeWithResolveWorktreeRemovalTarget extends OrcaRuntimeWith workspace: TerminalWorkspaceLaunchScope, opts: TerminalCreateOptions ): Promise { + // Before any early return: every create lane funnels through here, and a host that cannot + // apply the requested shell must refuse rather than spawn its default one. + const shellRefusal = terminalShellOverrideRefusal({ + shellOverride: opts.shellOverride, + connectionId: workspace.connectionId, + platform: process.platform, + projectRuntime: + opts.shellOverride && this.store + ? resolveLocalProjectRuntimeForWorktreeId(this.store, workspace.id) + : undefined, + // Same resolution as the spawn lanes below, so the refusal judges the cwd the PTY gets. + cwd: resolveTerminalStartupCwd(workspace.path, opts.cwd) ?? workspace.path, + workspacePath: workspace.path + }) + if (shellRefusal) { + throw shellRefusal + } // Why: raw shell commands like `codex exec` must remain user-authored shell. // Only unmanaged, repo-backed, bare agent launches get Settings defaults. const callerSuppliedLaunch = @@ -186,7 +206,8 @@ export class OrcaRuntimeWithResolveWorktreeRemovalTarget extends OrcaRuntimeWith const queuedShell = resolveLocalWindowsAgentStartupShell({ platform, isRemote, - terminalWindowsShell: settings.terminalWindowsShell + // A requested shell is the one this PTY will actually be, so it owns the quoting family. + terminalWindowsShell: opts.shellOverride ?? settings.terminalWindowsShell }) if (opts.startupAgent && !isTuiAgentEnabled(opts.startupAgent, settings.disabledTuiAgents)) { throw new Error(`Agent ${opts.startupAgent} is disabled. Choose an enabled agent.`) diff --git a/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness.spec.ts b/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness.spec.ts index bb6669da1a2..3a30c48e4b8 100644 --- a/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness.spec.ts +++ b/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { OrcaRuntimeService, makePaneKey } from '../orca-runtime-test-mocks.spec' +import { OrcaRuntimeService, electronMocks, makePaneKey } from '../orca-runtime-test-mocks.spec' import { HEADLESS_LEAF_ID, RESTORED_AUTHORITY_TOKEN, @@ -92,6 +92,130 @@ describe('OrcaRuntimeService', () => { }) }) + it('asks the pty controller for the requested shell instead of a startup command', async () => { + const hostPlatform = Object.getOwnPropertyDescriptor(process, 'platform')! + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-shell' }) + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + + await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + shellOverride: 'cmd.exe', + title: 'win shell' + }) + + // The defect this pins: a caller asking for cmd could only pass it as `command`, which the + // provider types into whatever shell it spawned — so the pty stayed the default shell and + // leaving cmd dropped the handle back onto a prompt the caller never asked for. + expect(spawn).toHaveBeenCalledWith( + expect.objectContaining({ shellOverride: 'cmd.exe', command: undefined }) + ) + } finally { + Object.defineProperty(process, 'platform', hostPlatform) + } + }) + + it('refuses a requested shell the execution host cannot apply instead of spawning its default', async () => { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-unreachable-shell' }) + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + + // Host platform here is POSIX, which has no Windows shell to pick. + await expect( + runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { shellOverride: 'cmd.exe' }) + ).rejects.toThrow(/--shell cmd\.exe names a Windows shell/) + expect(spawn).not.toHaveBeenCalled() + }) + + it('quotes the agent startup command for the requested shell, not the host default', async () => { + const hostPlatform = Object.getOwnPropertyDescriptor(process, 'platform')! + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-shell-quoting' }) + const runtime = new OrcaRuntimeService({ + ...store, + getSettings: () => ({ + ...store.getSettings(), + disabledTuiAgents: [], + terminalWindowsShell: 'powershell.exe', + agentCmdOverrides: {}, + agentDefaultArgs: { claude: '--dangerously-skip-permissions' }, + agentDefaultEnv: {} + }) + }) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + + await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { command: 'claude' }) + await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + command: 'claude', + shellOverride: 'cmd.exe' + }) + + // The setting alone still quotes for PowerShell; the requested shell is the one that will + // read the command, so it owns the quoting family. PowerShell quoting typed into cmd is a + // syntax error at the prompt. + expect(spawn).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ command: "claude '--dangerously-skip-permissions'" }) + ) + expect(spawn).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + shellOverride: 'cmd.exe', + command: 'claude "--dangerously-skip-permissions"' + }) + ) + } finally { + Object.defineProperty(process, 'platform', hostPlatform) + } + }) + + it('refuses a requested shell with no workspace instead of creating a default-shell tab', async () => { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-no-workspace-shell' }) + const send = vi.fn() + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents: { send } + }) + + await expect( + runtime.createTerminal(undefined, { shellOverride: 'cmd.exe', rendererBacked: true }) + ).rejects.toThrow(/--shell cmd\.exe needs a workspace/) + expect(send).not.toHaveBeenCalled() + expect(spawn).not.toHaveBeenCalled() + }) + it('retires inherited launch authority when the agent command exits', async () => { const spawn = vi.fn().mockResolvedValue({ id: 'pty-authority', incarnationId: 'process-1' }) const retireAuthority = vi.fn() diff --git a/src/main/runtime/rpc/methods/terminal/terminal-lifecycle-methods.ts b/src/main/runtime/rpc/methods/terminal/terminal-lifecycle-methods.ts index 891ed65a13f..2fcdc2bc92c 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-lifecycle-methods.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-lifecycle-methods.ts @@ -53,6 +53,7 @@ export const TERMINAL_LIFECYCLE_METHODS = [ (canonicalWorktreeSelector, preAllocatedHandle) => runtime.createTerminal(canonicalWorktreeSelector, { command: params.command, + ...(params.shell ? { shellOverride: params.shell } : {}), startupCommandDelivery: params.startupCommandDelivery, env: params.env, envToDelete: params.envToDelete, diff --git a/src/main/runtime/runtime-pty-controller-contract.ts b/src/main/runtime/runtime-pty-controller-contract.ts index 73a75af017e..161b5440116 100644 --- a/src/main/runtime/runtime-pty-controller-contract.ts +++ b/src/main/runtime/runtime-pty-controller-contract.ts @@ -58,6 +58,8 @@ export type RuntimePtyController = { tabId?: string leafId?: string sessionId?: string + /** Windows shell to spawn AS this PTY, instead of the host default. */ + shellOverride?: string isNewSession?: boolean persistHostSessionBinding?: boolean expectedSourceBinding?: PtyBindingSourceExpectation diff --git a/src/main/runtime/runtime-terminal-contracts.ts b/src/main/runtime/runtime-terminal-contracts.ts index 227788af7e1..680bec98227 100644 --- a/src/main/runtime/runtime-terminal-contracts.ts +++ b/src/main/runtime/runtime-terminal-contracts.ts @@ -22,6 +22,15 @@ import type { WorkerTerminalHostScope } from './orchestration/worker-terminal-pr export type TerminalCreateOptions = { command?: string + /** + * Windows shell to spawn AS the PTY process, instead of the host default shell. + * + * Distinct from `command`, which is typed into whatever shell the host spawns: a caller asking + * for cmd or PowerShell through `command` gets it as a CHILD of the default shell, so the + * terminal's own process is still the default shell and leaving that child lands back on a + * prompt the caller never asked for. + */ + shellOverride?: string claudeAgentTeamsSourceCommand?: string cwd?: string env?: Record diff --git a/src/main/runtime/terminal-shell-override-host-support.test.ts b/src/main/runtime/terminal-shell-override-host-support.test.ts new file mode 100644 index 00000000000..bbef16c2adc --- /dev/null +++ b/src/main/runtime/terminal-shell-override-host-support.test.ts @@ -0,0 +1,204 @@ +import { describe, expect, it } from 'vitest' +import { terminalShellOverrideRefusal } from './terminal-shell-override-host-support' +import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' + +const NO_PROJECT_RUNTIME = undefined +const WINDOWS_WORKSPACE = 'C:\\Users\\u\\app' +const WSL_WORKSPACE = '\\\\wsl$\\Ubuntu\\home\\u\\app' +const ON_WINDOWS_HOST = { cwd: WINDOWS_WORKSPACE, workspacePath: WINDOWS_WORKSPACE } + +function resolvedRuntime(kind: 'windows-host' | 'wsl'): ProjectExecutionRuntimeResolution { + return kind === 'wsl' + ? { + status: 'resolved', + runtime: { + kind: 'wsl', + hostPlatform: 'wsl', + distro: 'Ubuntu', + projectId: 'p1', + reason: 'project-override', + cacheKey: 'p1:wsl:Ubuntu' + } + } + : { + status: 'resolved', + runtime: { + kind: 'windows-host', + hostPlatform: 'win32', + projectId: 'p1', + reason: 'project-override', + cacheKey: 'p1:windows-host' + } + } +} + +describe('terminalShellOverrideRefusal', () => { + it('allows a requested shell on a local Windows execution host', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + ...ON_WINDOWS_HOST + }) + ).toBeNull() + }) + + it('stays out of the way when no shell was requested', () => { + for (const platform of ['darwin', 'linux', 'win32'] as const) { + expect( + terminalShellOverrideRefusal({ + shellOverride: undefined, + connectionId: 'ssh-1', + platform, + projectRuntime: resolvedRuntime('wsl'), + ...ON_WINDOWS_HOST + }) + ).toBeNull() + } + }) + + // Both of these hosts would otherwise spawn their default shell and report success. + it('refuses when the spawn happens over SSH', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: 'ssh-1', + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + ...ON_WINDOWS_HOST + })?.message + ).toContain('over SSH') + }) + + it('refuses on a host that has no Windows shells to pick from', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'darwin', + projectRuntime: NO_PROJECT_RUNTIME, + ...ON_WINDOWS_HOST + })?.message + ).toContain('darwin') + }) + + // `resolveLocalWindowsTerminalRuntimeOptions` rewrites a shell that contradicts the project's + // execution runtime, which would hand back a terminal running something else entirely — and + // would quote an agent's startup command for the shell that was asked for, not the one running. + it('refuses a WSL shell when the project runs its terminals on the Windows host', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'wsl.exe', + connectionId: null, + platform: 'win32', + projectRuntime: resolvedRuntime('windows-host'), + ...ON_WINDOWS_HOST + })?.message + ).toContain('on the Windows host') + }) + + it('refuses a Windows shell when the project runs its terminals in WSL', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: resolvedRuntime('wsl'), + ...ON_WINDOWS_HOST + })?.message + ).toContain('in WSL') + }) + + it('allows a shell that agrees with the project runtime', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'wsl.exe', + connectionId: null, + platform: 'win32', + projectRuntime: resolvedRuntime('wsl'), + ...ON_WINDOWS_HOST + }) + ).toBeNull() + expect( + terminalShellOverrideRefusal({ + shellOverride: 'powershell.exe', + connectionId: null, + platform: 'win32', + projectRuntime: resolvedRuntime('windows-host'), + ...ON_WINDOWS_HOST + }) + ).toBeNull() + }) + + // `resolveWslSessionContext` forces wsl.exe for any `\\wsl$` cwd or workspace path, which is + // the one rewrite the project-runtime check cannot see: a folder workspace has no project. + describe('WSL UNC paths', () => { + it('refuses a Windows shell for a folder workspace inside a WSL distro with no project runtime', () => { + const message = terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: WSL_WORKSPACE, + workspacePath: WSL_WORKSPACE + })?.message + expect(message).toContain('inside WSL') + expect(message).toContain('No terminal was created') + expect(message).toContain('--shell wsl.exe') + }) + + it('refuses when only the workspace root is in WSL, since the session path forces wsl.exe too', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'powershell.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: WINDOWS_WORKSPACE, + workspacePath: WSL_WORKSPACE + })?.message + ).toContain(WSL_WORKSPACE) + }) + + it('accepts the forward-slash wsl.localhost spelling as a WSL path', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: '//wsl.localhost/Ubuntu/home/u/app/src', + workspacePath: '//wsl.localhost/Ubuntu/home/u/app' + }) + ).not.toBeNull() + }) + + it('allows wsl.exe for a WSL path', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'wsl.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: WSL_WORKSPACE, + workspacePath: WSL_WORKSPACE + }) + ).toBeNull() + }) + + it('still allows a Windows shell for a plain Windows path', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: 'C:\\Users\\u\\app\\src', + workspacePath: WINDOWS_WORKSPACE + }) + ).toBeNull() + }) + }) +}) diff --git a/src/main/runtime/terminal-shell-override-host-support.ts b/src/main/runtime/terminal-shell-override-host-support.ts new file mode 100644 index 00000000000..d91b202a532 --- /dev/null +++ b/src/main/runtime/terminal-shell-override-host-support.ts @@ -0,0 +1,90 @@ +import { isWslShellName } from '../../shared/local-windows-terminal-runtime' +import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' +import { parseWslUncPath } from '../../shared/wsl-paths' + +/** + * Whether the host about to spawn a terminal can honour a requested Windows shell. + * + * The whole point of `--shell` is that the caller stops having to guess which shell it got. A host + * that cannot apply the pick must say so: silently spawning its default shell returns a healthy + * terminal running something else, which is the exact failure `--shell` exists to remove. + */ +export function terminalShellOverrideRefusal(args: { + shellOverride: string | undefined + connectionId: string | null + platform: NodeJS.Platform + projectRuntime: ProjectExecutionRuntimeResolution | undefined + /** The cwd the PTY will spawn in, resolved the same way the spawn lanes resolve it. */ + cwd: string + workspacePath: string +}): Error | null { + if (!args.shellOverride) { + return null + } + if (args.connectionId) { + // The shell runs on the SSH host, whose platform and installed shells this runtime cannot see. + return new Error( + `This workspace runs its terminals over SSH, and Orca cannot apply --shell ${args.shellOverride} there. No terminal was created. Omit --shell, or create the terminal on the execution host itself.` + ) + } + if (args.platform !== 'win32') { + return new Error( + `--shell ${args.shellOverride} names a Windows shell, and this execution host is ${args.platform}, which spawns the user's login shell. No terminal was created; omit --shell.` + ) + } + return ( + projectRuntimeShellConflict(args.shellOverride, args.projectRuntime) ?? + wslUncPathShellConflict(args.shellOverride, args.cwd, args.workspacePath) + ) +} + +/** + * The project's execution runtime decides which MACHINE the shell runs on, so it outranks a + * per-terminal pick — and `resolveLocalWindowsTerminalRuntimeOptions` enforces that by rewriting + * the value: a WSL project forces `wsl.exe`, and a Windows-host project discards a WSL name in + * favour of the host shell. Either rewrite hands back a terminal running something the caller did + * not ask for, and it also splits the startup-command quoting from the shell that receives it + * (POSIX args typed into cmd, or cmd args typed into a WSL shell). Refusing the contradiction is + * the only answer that keeps the request and the terminal describing the same thing. + */ +function projectRuntimeShellConflict( + shellOverride: string, + projectRuntime: ProjectExecutionRuntimeResolution | undefined +): Error | null { + if (projectRuntime?.status !== 'resolved') { + return null + } + const runsInWsl = projectRuntime.runtime.kind === 'wsl' + if (runsInWsl === isWslShellName(shellOverride)) { + return null + } + return new Error( + runsInWsl + ? `This workspace's project runs its terminals in WSL, so --shell ${shellOverride} cannot be applied. No terminal was created. Use --shell wsl.exe, or change the project's execution runtime.` + : `This workspace's project runs its terminals on the Windows host, so --shell ${shellOverride} cannot be applied. No terminal was created. Change the project's execution runtime to WSL, or pass a Windows shell.` + ) +} + +/** + * A `\\wsl$\\...` path only exists inside that distro, so the providers force `wsl.exe` + * for it whatever shell was requested (`resolveWslSessionContext` keys off the cwd, then the + * workspace path behind the session). That rewrite is right for the global shell setting and + * wrong for `--shell`, which promised the caller the shell it named. This is the only check that + * still fires for a folder workspace, which has no project runtime to disagree with. + */ +function wslUncPathShellConflict( + shellOverride: string, + cwd: string, + workspacePath: string +): Error | null { + if (isWslShellName(shellOverride)) { + return null + } + const wslPath = [cwd, workspacePath].find((path) => parseWslUncPath(path) !== null) + if (wslPath === undefined) { + return null + } + return new Error( + `This terminal would run inside WSL because ${wslPath} lives in a WSL distro, so --shell ${shellOverride} cannot be applied. No terminal was created. Use --shell wsl.exe, or omit --shell.` + ) +} diff --git a/src/main/ssh/ssh-remote-orca-cli.test.ts b/src/main/ssh/ssh-remote-orca-cli.test.ts index 0b3695aec5c..276d9cad87c 100644 --- a/src/main/ssh/ssh-remote-orca-cli.test.ts +++ b/src/main/ssh/ssh-remote-orca-cli.test.ts @@ -180,6 +180,36 @@ describe('runRemoteOrcaCli', () => { } ) + // Why: `orca terminal create --shell` gates on these; without them an SSH pane was told the + // host was too old, when the accurate refusal is that SSH cannot apply the shell. + it('reports the execution host capabilities through the legacy status fallback', async () => { + const runtime = new OrcaRuntimeService() + vi.spyOn(runtime, 'getStatus').mockReturnValue({ + runtimeId: 'runtime-test', + rendererGraphEpoch: 1, + graphStatus: 'ready', + authoritativeWindowId: 1, + liveTabCount: 0, + liveLeafCount: 0, + capabilities: ['terminal.create-shell-selection.v1'] + }) + + const result = await runRemoteOrcaCli( + runtime, + { argv: ['status', '--json'], cwd: '/home/alice/repo', env: {} }, + LEGACY_FALLBACK_OPTIONS + ) + + expect(result.exitCode, result.stdout).toBe(0) + expect(JSON.parse(result.stdout)).toMatchObject({ + ok: true, + result: { + target: { kind: 'environment', environment: 'ssh' }, + runtime: { reachable: true, capabilities: ['terminal.create-shell-selection.v1'] } + } + }) + }) + it('uses the remote ORCA_TERMINAL_HANDLE as orchestration sender identity', async () => { const { runtime, db } = createRuntime() diff --git a/src/main/ssh/ssh-remote-orca-cli.ts b/src/main/ssh/ssh-remote-orca-cli.ts index dba1bc1e2d5..6f476fa0d84 100644 --- a/src/main/ssh/ssh-remote-orca-cli.ts +++ b/src/main/ssh/ssh-remote-orca-cli.ts @@ -185,7 +185,10 @@ async function dispatchRemoteCli( state: status.graphStatus === 'ready' ? 'ready' : 'graph_not_ready', reachable: true, connectionState: runtimeHostConnectionState({ hasStatusEntry: true, status }), - runtimeId: status.runtimeId + runtimeId: status.runtimeId, + // Why: `status.get` ran in-process on the execution host, so these ARE that host's + // capabilities; dropping them made `--shell` report an outdated host instead of SSH. + ...(status.capabilities ? { capabilities: status.capabilities } : {}) }, graph: { state: status.graphStatus } } diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index b012b30469e..4a55d6b587b 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -5,7 +5,7 @@ import { existsSync } from 'node:fs' import { basename, join } from 'node:path' import { randomUUID } from 'node:crypto' import { resolveWindowsGitBashShellPath } from '../main/git-bash' -import { WINDOWS_GIT_BASH_SHELL } from '../shared/windows-terminal-shell' +import { isSupportedWindowsShellOverride } from '../shared/windows-terminal-shell' import type { RelayDispatcher, RequestContext } from './dispatcher' import { resolveDefaultShell, @@ -365,22 +365,6 @@ const ALLOWED_SIGNALS = new Set([ 'SIGUSR2' ]) -const ALLOWED_WINDOWS_SHELL_OVERRIDES = new Set([ - 'powershell.exe', - 'powershell', - 'pwsh.exe', - 'pwsh', - 'cmd.exe', - 'cmd', - 'wsl.exe', - 'wsl', - // Why: both spellings classify as a POSIX startup family, so rejecting them here made the relay - // the one host that hard-failed a setting the local and daemon PTYs accept. - 'bash.exe', - 'bash', - WINDOWS_GIT_BASH_SHELL -]) - function resolvePtyShellOverride(shellOverride: string): string { if (!shellOverride) { return '' @@ -388,8 +372,7 @@ function resolvePtyShellOverride(shellOverride: string): string { if (process.platform !== 'win32') { return '' } - const normalized = shellOverride.toLowerCase() - if (!ALLOWED_WINDOWS_SHELL_OVERRIDES.has(normalized)) { + if (!isSupportedWindowsShellOverride(shellOverride)) { throw new Error(`Unsupported Windows shell override: ${shellOverride}`) } return resolveWindowsGitBashShellPath(shellOverride) ?? shellOverride diff --git a/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts index a297c751a29..2452d0f167d 100644 --- a/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts @@ -78,7 +78,7 @@ export function registerTerminalRequestIpcBridge(unsubs: (() => void)[]): void { recordInteraction: false, ...(data.cwd ? { startupCwd: data.cwd } : {}) } - const tab = store.createTab(worktreeId, data.targetGroupId, undefined, tabOptions) + const tab = store.createTab(worktreeId, data.targetGroupId, data.shellOverride, tabOptions) if (!shouldActivate) { // Why: renderer-backed Codex startup must mount its new TerminalPane without switching UI or connecting every saved tab. requestBackgroundTerminalWorktreeMount({ worktreeId, tabIds: [tab.id] }) diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index bd2ca84c95f..efe101a4eb0 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -130,6 +130,11 @@ export const ACCOUNT_IMPORT_RUNTIME_CAPABILITY = 'accounts.import-host-credentia // Why: older hosts cannot reconcile terminal.create's mutation after losing the reply, so clients may only retry unknown outcomes when advertised. export const TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY = 'terminal.create-idempotency.v2' as const +// Why: an older host strips terminal.create's unknown `shell` and answers with a terminal running +// the host default shell. That reply is indistinguishable from success, so a client asking for a +// shell must refuse rather than create the wrong one. +export const TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY = + 'terminal.create-shell-selection.v1' as const export const SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY = 'session-tabs.close-intent.v1' as const export const SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY = 'session-tabs.authoritative-inventory.v1' as const @@ -296,6 +301,7 @@ export const RUNTIME_CAPABILITIES = [ WORKTREE_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY, WORKTREE_ARCHIVE_FAILURE_BLOCKING_RUNTIME_CAPABILITY, TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY, + TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY, SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY, SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY, AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY, diff --git a/src/shared/rpc-contract/terminal-create-shell-param.test.ts b/src/shared/rpc-contract/terminal-create-shell-param.test.ts new file mode 100644 index 00000000000..ca28a0580e1 --- /dev/null +++ b/src/shared/rpc-contract/terminal-create-shell-param.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest' +import { TerminalCreateParams } from './terminal-unary-params' +import { resolveWindowsShellStartupFamily } from '../windows-terminal-shell' + +describe('TerminalCreateParams.shell', () => { + it('stays optional so older callers keep creating terminals', () => { + const parsed = TerminalCreateParams.parse({ worktree: 'path:/repo' }) + + expect(parsed.shell).toBeUndefined() + }) + + it('carries an allowed Windows shell through to the runtime', () => { + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'cmd.exe' }).shell).toBe( + 'cmd.exe' + ) + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'git-bash' }).shell).toBe( + 'git-bash' + ) + }) + + // The host canonicalizes even when a client did not, so the spawn path and the startup-command + // quoting only ever see the `.exe` spelling they exact-match. + it('canonicalizes an accepted spelling before it reaches the runtime', () => { + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'cmd' }).shell).toBe( + 'cmd.exe' + ) + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'pwsh' }).shell).toBe( + 'pwsh.exe' + ) + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'Git-Bash' }).shell).toBe( + 'git-bash' + ) + }) + + it('quotes a bare cmd override as cmd rather than PowerShell', () => { + const { shell } = TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'cmd' }) + + expect(resolveWindowsShellStartupFamily(shell)).toBe('cmd') + }) + + // The relay refuses these at spawn time; refusing here turns an opaque spawn failure into an + // answer the caller gets before the terminal exists. + it('refuses a shell the host will not spawn', () => { + expect(() => TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'nu.exe' })).toThrow( + /shell must be one of/ + ) + expect(() => + TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'cmd.exe && calc' }) + ).toThrow(/shell must be one of/) + }) +}) diff --git a/src/shared/rpc-contract/terminal-unary-params.ts b/src/shared/rpc-contract/terminal-unary-params.ts index e86bdf1fdc1..9b735952096 100644 --- a/src/shared/rpc-contract/terminal-unary-params.ts +++ b/src/shared/rpc-contract/terminal-unary-params.ts @@ -1,6 +1,11 @@ import { z } from 'zod' import { OptionalFiniteNumber, OptionalString, requiredString } from './rpc-param-primitives' import { isTuiAgent } from '../tui-agent-config' +import { + canonicalizeWindowsShellOverride, + isSupportedWindowsShellOverride, + listSupportedWindowsShellOverrides +} from '../windows-terminal-shell' import { TERMINAL_PANE_SPLIT_SOURCES } from '../feature-education-telemetry' export const TerminalHandle = z.object({ @@ -180,7 +185,18 @@ export const TerminalCreateParams = z.object({ activate: z.unknown().optional(), presentation: z.enum(['background', 'focused']).optional(), tabId: OptionalString, - leafId: OptionalString + leafId: OptionalString, + // Why refused at the boundary rather than at spawn: only the host knows the allowlist, and a + // relay-side throw reaches the caller as an opaque spawn failure after the round trip. + shell: z + .string() + .refine(isSupportedWindowsShellOverride, { + message: `shell must be one of: ${listSupportedWindowsShellOverrides().join(', ')}` + }) + // Why here: the host is authoritative, so it canonicalizes even when a client did not; the + // spawn path exact-matches `.exe` spellings and must never see `cmd` or `Git-Bash`. + .transform((shell) => canonicalizeWindowsShellOverride(shell) ?? shell) + .optional() }) export const TerminalSplit = TerminalHandle.extend({ diff --git a/src/shared/runtime-terminal-contracts.ts b/src/shared/runtime-terminal-contracts.ts index ad392a2b9a0..57560caa7a5 100644 --- a/src/shared/runtime-terminal-contracts.ts +++ b/src/shared/runtime-terminal-contracts.ts @@ -263,6 +263,8 @@ type RuntimeTerminalCreateBaseRequestPayload = { activate?: boolean presentation?: RuntimeTerminalPresentation surfaceOwner?: false + /** Windows shell the created tab spawns AS, instead of the host default. */ + shellOverride?: string } export type RuntimeTerminalCreateRequestPayload = diff --git a/src/shared/source-scan/source-tree-walk.test.ts b/src/shared/source-scan/source-tree-walk.test.ts index afc7c8dba88..4030fa2f37c 100644 --- a/src/shared/source-scan/source-tree-walk.test.ts +++ b/src/shared/source-scan/source-tree-walk.test.ts @@ -108,7 +108,6 @@ describe('scanSourceTree filesystem traversal', () => { expect(statSync).toHaveBeenCalledExactlyOnceWith(join(root, 'alias')) }) - it('still reports a broken link instead of silently dropping it', () => { const target = join(root, '.target') mkdirSync(target) diff --git a/src/shared/windows-terminal-shell.test.ts b/src/shared/windows-terminal-shell.test.ts index e7d86a07cc9..e9c9653bd99 100644 --- a/src/shared/windows-terminal-shell.test.ts +++ b/src/shared/windows-terminal-shell.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest' -import { resolveWindowsShellStartupFamily } from './windows-terminal-shell' +import { + canonicalizeWindowsShellOverride, + isSupportedWindowsShellOverride, + listSupportedWindowsShellOverrides, + resolveWindowsShellStartupFamily +} from './windows-terminal-shell' describe('resolveWindowsShellStartupFamily', () => { it('defaults to PowerShell when unset', () => { @@ -33,3 +38,83 @@ describe('resolveWindowsShellStartupFamily', () => { expect(resolveWindowsShellStartupFamily('C:\\Program Files\\Git\\bin\\bash')).toBe('posix') }) }) + +describe('isSupportedWindowsShellOverride', () => { + // Spelled out rather than looped over the list, which would assert the list against itself. + it('accepts exactly the shells the relay is willing to spawn', () => { + expect(listSupportedWindowsShellOverrides()).toEqual([ + 'bash', + 'bash.exe', + 'cmd', + 'cmd.exe', + 'git-bash', + 'powershell', + 'powershell.exe', + 'pwsh', + 'pwsh.exe', + 'wsl', + 'wsl.exe' + ]) + expect(isSupportedWindowsShellOverride('cmd.exe')).toBe(true) + expect(isSupportedWindowsShellOverride('powershell.exe')).toBe(true) + expect(isSupportedWindowsShellOverride('git-bash')).toBe(true) + }) + + it('accepts a differently cased spelling of an allowed shell', () => { + expect(isSupportedWindowsShellOverride('CMD.EXE')).toBe(true) + expect(isSupportedWindowsShellOverride('PowerShell.exe')).toBe(true) + }) + + // The allowlist is what stops `--shell` from naming an arbitrary executable to spawn. + it('refuses anything else, including a path to an allowed shell', () => { + expect(isSupportedWindowsShellOverride('nu.exe')).toBe(false) + expect(isSupportedWindowsShellOverride('')).toBe(false) + expect(isSupportedWindowsShellOverride('C:\\Windows\\System32\\cmd.exe')).toBe(false) + expect(isSupportedWindowsShellOverride('cmd.exe /c calc')).toBe(false) + }) +}) + +describe('canonicalizeWindowsShellOverride', () => { + it('maps every accepted spelling to the `.exe` name the spawn path exact-matches', () => { + expect(canonicalizeWindowsShellOverride('cmd')).toBe('cmd.exe') + expect(canonicalizeWindowsShellOverride('cmd.exe')).toBe('cmd.exe') + expect(canonicalizeWindowsShellOverride('powershell')).toBe('powershell.exe') + expect(canonicalizeWindowsShellOverride('powershell.exe')).toBe('powershell.exe') + expect(canonicalizeWindowsShellOverride('pwsh')).toBe('pwsh.exe') + expect(canonicalizeWindowsShellOverride('pwsh.exe')).toBe('pwsh.exe') + expect(canonicalizeWindowsShellOverride('wsl')).toBe('wsl.exe') + expect(canonicalizeWindowsShellOverride('wsl.exe')).toBe('wsl.exe') + expect(canonicalizeWindowsShellOverride('bash')).toBe('bash.exe') + expect(canonicalizeWindowsShellOverride('bash.exe')).toBe('bash.exe') + expect(canonicalizeWindowsShellOverride('git-bash')).toBe('git-bash') + }) + + // pwsh (PowerShell 7) and powershell (Windows PowerShell 5.1) are different binaries. + it('never collapses pwsh into powershell', () => { + expect(canonicalizeWindowsShellOverride('pwsh')).not.toBe('powershell.exe') + expect(canonicalizeWindowsShellOverride('pwsh.exe')).not.toBe('powershell.exe') + }) + + // `resolveWindowsGitBashShellPath` compares the marker case-sensitively. + it('folds case so a mixed-case spelling reaches the exact-match consumers', () => { + expect(canonicalizeWindowsShellOverride('Git-Bash')).toBe('git-bash') + expect(canonicalizeWindowsShellOverride('CMD')).toBe('cmd.exe') + expect(canonicalizeWindowsShellOverride('PowerShell.exe')).toBe('powershell.exe') + }) + + it('returns undefined for anything the allowlist refuses', () => { + expect(canonicalizeWindowsShellOverride('nu.exe')).toBeUndefined() + expect(canonicalizeWindowsShellOverride('C:\\Windows\\System32\\cmd.exe')).toBeUndefined() + expect(canonicalizeWindowsShellOverride('')).toBeUndefined() + }) + + // Bare `cmd` falls through resolveWindowsShellStartupFamily to PowerShell quoting; the canonical + // name is what keeps the PTY shell and the queued-command quoting in the same family. + it('yields the cmd startup family for a bare cmd override', () => { + expect(resolveWindowsShellStartupFamily('cmd')).toBe('powershell') + expect(resolveWindowsShellStartupFamily(canonicalizeWindowsShellOverride('cmd'))).toBe('cmd') + expect(resolveWindowsShellStartupFamily(canonicalizeWindowsShellOverride('Git-Bash'))).toBe( + 'posix' + ) + }) +}) diff --git a/src/shared/windows-terminal-shell.ts b/src/shared/windows-terminal-shell.ts index 1f55bf89d35..625eefbb414 100644 --- a/src/shared/windows-terminal-shell.ts +++ b/src/shared/windows-terminal-shell.ts @@ -54,3 +54,47 @@ export function resolveLocalWindowsAgentStartupShell(args: { } return resolveWindowsShellStartupFamily(args.terminalWindowsShell) } + +/** + * Shell names a caller may request for a single Windows terminal, keyed by accepted spelling and + * mapped to the one canonical spelling every downstream consumer keys on. + * + * The relay owns the spawn and has always refused anything outside this set, but the set lived + * only there — so a bad value from `terminal create --shell` surfaced as a spawn-time throw with + * no way for the CLI to answer before the round trip. Shared so the RPC boundary and the relay + * agree on the same names. + * + * Why canonicalize: `resolveWindowsShellStartupFamily`, the launch-arg builders, and the Git Bash + * path resolver all exact-match the `.exe` spelling, so a bare `cmd` accepted here would spawn cmd + * yet quote its startup command for PowerShell. `pwsh` and `powershell` are different binaries and + * are never collapsed into each other. + */ +const WINDOWS_SHELL_OVERRIDE_CANONICAL_NAMES: ReadonlyMap = new Map([ + ['powershell.exe', 'powershell.exe'], + ['powershell', 'powershell.exe'], + ['pwsh.exe', 'pwsh.exe'], + ['pwsh', 'pwsh.exe'], + ['cmd.exe', 'cmd.exe'], + ['cmd', 'cmd.exe'], + ['wsl.exe', 'wsl.exe'], + ['wsl', 'wsl.exe'], + // Why: both spellings classify as a POSIX startup family, so rejecting them here made the relay + // the one host that hard-failed a setting the local and daemon PTYs accept. + ['bash.exe', 'bash.exe'], + ['bash', 'bash.exe'], + [WINDOWS_GIT_BASH_SHELL, WINDOWS_GIT_BASH_SHELL] +]) + +/** Canonical spelling for an accepted override (case-insensitive), or undefined when refused. */ +export function canonicalizeWindowsShellOverride(shell: string): string | undefined { + return WINDOWS_SHELL_OVERRIDE_CANONICAL_NAMES.get(shell.toLowerCase()) +} + +export function isSupportedWindowsShellOverride(shell: string): boolean { + return canonicalizeWindowsShellOverride(shell) !== undefined +} + +/** Sorted for a stable error message; callers list these when refusing a value. */ +export function listSupportedWindowsShellOverrides(): string[] { + return [...WINDOWS_SHELL_OVERRIDE_CANONICAL_NAMES.keys()].sort() +}