From d6eb9fb1fc0a0ba65f4e7535db266affce410c01 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 15 Sep 2026 03:16:43 -0700 Subject: [PATCH] fix(terminal): let a runtime-created Windows terminal BE the requested shell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- .gitignore | 1 + AGENTS.md | 1 + .../windows-terminal-shell-selection.md | 46 +++++++++++ src/cli/handlers/terminal.test.ts | 78 ++++++++++++++++++- src/cli/handlers/terminal.ts | 29 +++++++ src/cli/root-help-text-secondary.ts | 2 +- src/cli/specs/core.ts | 11 ++- .../pty-spawn-shell-override-parity.test.ts | 26 +++++++ src/main/ipc/pty/runtime/spawn-preflight.ts | 7 +- src/main/ipc/pty/runtime/spawn-state.ts | 1 + .../orca-runtime-create-terminal-desktop.ts | 1 + .../runtime/orca-runtime-create-terminal.ts | 1 + ...runtime-resolve-worktree-removal-target.ts | 3 +- .../terminal-creation-and-readiness.spec.ts | 25 ++++++ .../terminal/terminal-lifecycle-methods.ts | 1 + .../runtime-pty-controller-contract.ts | 2 + .../runtime/runtime-terminal-contracts.ts | 9 +++ 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 | 30 +++++++ .../rpc-contract/terminal-unary-params.ts | 14 +++- src/shared/runtime-terminal-contracts.ts | 2 + src/shared/windows-terminal-shell.test.ts | 30 ++++++- src/shared/windows-terminal-shell.ts | 33 ++++++++ 25 files changed, 352 insertions(+), 30 deletions(-) create mode 100644 docs/reference/windows-terminal-shell-selection.md create mode 100644 src/main/ipc/pty/pty-spawn-shell-override-parity.test.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/docs/reference/windows-terminal-shell-selection.md b/docs/reference/windows-terminal-shell-selection.md new file mode 100644 index 00000000000..d4b25f4725c --- /dev/null +++ b/docs/reference/windows-terminal-shell-selection.md @@ -0,0 +1,46 @@ +# 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. macOS and Linux hosts spawn the login shell and ignore it; the relay + drops the value off `win32` rather than honouring it half-way. +- A WSL project runtime still wins over `--shell` (`resolveLocalWindowsTerminalRuntimeOptions`). + That is deliberate: the project's runtime decides which machine the shell runs on, and a + per-terminal pick may not override that. diff --git a/src/cli/handlers/terminal.test.ts b/src/cli/handlers/terminal.test.ts index 21f296b261a..e5f3347dc93 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,76 @@ describe('terminal send CLI', () => { ]) }) }) + +describe('terminal create --shell', () => { + const WORKTREE = 'path:C:/src/app' + + const shellClient = (call: ReturnType, supported: boolean): RuntimeClient => { + const client = { + call, + isRemote: false, + getCliStatus: vi.fn().mockResolvedValue({ + result: { + runtime: { + reachable: true, + runtimeId: 'runtime-current', + capabilities: supported ? [TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY] : [] + } + } + }) + } + // 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() + }) +}) diff --git a/src/cli/handlers/terminal.ts b/src/cli/handlers/terminal.ts index c409a6a9f9a..b4610eefe1a 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,33 @@ 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() + 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/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..be4daba708d 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.', + '--shell has no effect on macOS and Linux hosts, which spawn the login shell.' ], 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.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..b03d16ae303 100644 --- a/src/main/runtime/orca-runtime-create-terminal-desktop.ts +++ b/src/main/runtime/orca-runtime-create-terminal-desktop.ts @@ -63,6 +63,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..2cf0936f2d8 100644 --- a/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts +++ b/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts @@ -186,7 +186,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..88edc743b5e 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 @@ -92,6 +92,31 @@ describe('OrcaRuntimeService', () => { }) }) + it('spawns a requested shell AS the pty instead of typing it into the host default', async () => { + 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 }) + ) + }) + 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/relay/pty-handler.ts b/src/relay/pty-handler.ts index cdf436bca2a..a701cffa07b 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..57488559153 --- /dev/null +++ b/src/shared/rpc-contract/terminal-create-shell-param.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest' +import { TerminalCreateParams } from './terminal-unary-params' + +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 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..b993ab0cbec 100644 --- a/src/shared/rpc-contract/terminal-unary-params.ts +++ b/src/shared/rpc-contract/terminal-unary-params.ts @@ -1,6 +1,10 @@ import { z } from 'zod' import { OptionalFiniteNumber, OptionalString, requiredString } from './rpc-param-primitives' import { isTuiAgent } from '../tui-agent-config' +import { + isSupportedWindowsShellOverride, + listSupportedWindowsShellOverrides +} from '../windows-terminal-shell' import { TERMINAL_PANE_SPLIT_SOURCES } from '../feature-education-telemetry' export const TerminalHandle = z.object({ @@ -180,7 +184,15 @@ 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(', ')}` + }) + .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/windows-terminal-shell.test.ts b/src/shared/windows-terminal-shell.test.ts index e7d86a07cc9..464c1134b09 100644 --- a/src/shared/windows-terminal-shell.test.ts +++ b/src/shared/windows-terminal-shell.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { resolveWindowsShellStartupFamily } from './windows-terminal-shell' +import { + isSupportedWindowsShellOverride, + listSupportedWindowsShellOverrides, + resolveWindowsShellStartupFamily +} from './windows-terminal-shell' describe('resolveWindowsShellStartupFamily', () => { it('defaults to PowerShell when unset', () => { @@ -33,3 +37,27 @@ describe('resolveWindowsShellStartupFamily', () => { expect(resolveWindowsShellStartupFamily('C:\\Program Files\\Git\\bin\\bash')).toBe('posix') }) }) + +describe('isSupportedWindowsShellOverride', () => { + it('accepts every shell the relay is willing to spawn', () => { + for (const shell of listSupportedWindowsShellOverrides()) { + expect(isSupportedWindowsShellOverride(shell)).toBe(true) + } + expect(listSupportedWindowsShellOverrides()).toContain('cmd.exe') + expect(listSupportedWindowsShellOverrides()).toContain('powershell.exe') + expect(listSupportedWindowsShellOverrides()).toContain('git-bash') + }) + + 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) + }) +}) diff --git a/src/shared/windows-terminal-shell.ts b/src/shared/windows-terminal-shell.ts index 1f55bf89d35..0d9a9130cb7 100644 --- a/src/shared/windows-terminal-shell.ts +++ b/src/shared/windows-terminal-shell.ts @@ -54,3 +54,36 @@ export function resolveLocalWindowsAgentStartupShell(args: { } return resolveWindowsShellStartupFamily(args.terminalWindowsShell) } + +/** + * Shell names a caller may request for a single Windows terminal. + * + * 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. + */ +const WINDOWS_SHELL_OVERRIDES: ReadonlySet = 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 +]) + +export function isSupportedWindowsShellOverride(shell: string): boolean { + return WINDOWS_SHELL_OVERRIDES.has(shell.toLowerCase()) +} + +/** Sorted for a stable error message; callers list these when refusing a value. */ +export function listSupportedWindowsShellOverrides(): string[] { + return [...WINDOWS_SHELL_OVERRIDES].sort() +}