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() +}