mirror of
https://github.com/stablyai/orca.git
synced 2026-09-25 16:02:38 +00:00
fix(terminal): let a runtime-created Windows terminal BE the requested shell
`orca terminal create --environment <windows-host> --command 'cmd.exe'` never
created a cmd terminal. `--command` is text the provider TYPES into whatever
shell it spawned, so the PTY stayed the host's default shell with cmd running
inside it. Captured on `awin`, whose default is Git Bash:
$ orca terminal create --environment awin --command 'cmd.exe' --json
$ orca terminal send --environment awin --terminal term_10656cf7... \
--text exit --enter
$ orca terminal read --environment awin --terminal term_10656cf7... --screen
neil@awin MINGW64 ~/orca/orca ((30f820708f...))
$ cmd.exe
Microsoft Windows [Version 10.0.26200.9445]
C:\Users\neil\orca\orca>exit
neil@awin MINGW64 ~/orca/orca ((30f820708f...))
$
The handle is alive the whole time and `terminal list` shows one healthy
terminal, because the PTY never changed — so the only symptom is that the
caller's terminal is now a shell it never asked for, and every later `send` is
quoted for the wrong one. On `win-lowspec` (default pwsh) the same create lands
cmd inside PowerShell.
Root cause
----------
There are two spawn preflights and they are twins:
- `src/main/ipc/pty/ipc/spawn-preflight.ts` — renderer/IPC spawns, i.e. a
terminal tab opened in the app.
- `src/main/ipc/pty/runtime/spawn-preflight.ts` — runtime spawns: the CLI's
`terminal.create`, headless `orca serve`, and every paired remote
environment.
Only the IPC twin read the caller's requested shell. The runtime twin passed a
literal `requestedShellOverride: undefined`, so a runtime-created terminal on
Windows could only ever be the host default. Everything downstream of that
point — `spawn-options`, the daemon, `resolvePtyShellOverride` in the relay,
`local-pty-launch-plan` — already honoured `shellOverride`; nothing upstream
could supply one.
Change
------
- Thread `shellOverride` through the runtime lane: `RuntimePtySpawnArgs` ->
runtime `spawn-preflight` -> `RuntimePtyController.spawn` ->
`TerminalCreateOptions` -> the `terminal.create` RPC's new `shell` param ->
`orca terminal create --shell`.
- Thread it through the renderer-backed lane too (`createDesktopTerminal` ->
`terminal:requestTabCreate` -> `store.createTab`), so `--shell --focus` is not
silently dropped on a local Windows app.
- An agent launch quotes its startup command for the shell it will actually run
in, so a requested shell now owns the startup-shell family instead of the
global `terminalWindowsShell` setting.
- Lift the relay's `ALLOWED_WINDOWS_SHELL_OVERRIDES` into
`isSupportedWindowsShellOverride` in `src/shared/windows-terminal-shell.ts`
(membership unchanged) so the CLI, the zod param schema, and the relay refuse
the same names. `--shell` therefore cannot carry a path or a command line into
`pty.spawn`; only allowlisted bare shell names pass.
- Gate on `TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY`. An older host
strips the unknown `shell` param and answers with a healthy terminal running
its default shell — a reply indistinguishable from success — so the CLI
refuses before creating anything rather than creating the wrong shell quietly.
`--shell` stays Windows-only; macOS and Linux hosts spawn the login shell and
the relay drops the value off win32 rather than honouring it half-way. A WSL
project runtime still outranks it, unchanged.
Tests
-----
- `pty-spawn-shell-override-parity.test.ts` pins both preflights against the
exact drift that caused this (verified failing with the fix reverted).
- `createTerminal` passes `shellOverride` to `ptyController.spawn` with no
startup command.
- CLI: sends `shell`, refuses a shell the host cannot spawn, and refuses a host
without the capability — in both refusals without making the round trip.
- Allowlist and `terminal.create` schema accept/refuse cases, including paths
and appended arguments.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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.
|
||||
@@ -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<typeof vi.fn>, 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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<string, CommandHandler> = {
|
||||
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
|
||||
|
||||
@@ -62,7 +62,7 @@ export const ROOT_HELP_TEXT_SECONDARY = [
|
||||
' orca terminal read [--terminal <handle>] [--cursor <n>] [--limit <n>] [--json]',
|
||||
' orca terminal send [--terminal <handle>] [--text <text>] [--enter] [--interrupt] [--wait-submit <seconds>] [--retry-request <id>] [--json]',
|
||||
' orca terminal wait [--terminal <handle>] --for exit|tui-idle [--timeout-ms <ms>] [--json]',
|
||||
' orca terminal create [--worktree <selector>] [--title <name>] [--command <text>] [--focus] [--json]',
|
||||
' orca terminal create [--worktree <selector>] [--title <name>] [--command <text>] [--shell <shell>] [--focus] [--json]',
|
||||
' orca terminal split [--terminal <handle>] [--direction horizontal|vertical] [--json]',
|
||||
' orca terminal switch [--terminal <handle>] [--json]',
|
||||
' orca terminal close ([--terminal <handle>] [--tab] | --worktree <selector> --all) [--json]',
|
||||
|
||||
@@ -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 <selector>] [--title <name>] [--command <text>] [--focus] [--json]',
|
||||
allowedFlags: [...GLOBAL_FLAGS, 'worktree', 'command', 'title', 'focus'],
|
||||
'orca terminal create [--worktree <selector>] [--title <name>] [--command <text>] [--shell <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'
|
||||
]
|
||||
},
|
||||
{
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
})
|
||||
@@ -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'
|
||||
|
||||
@@ -102,6 +102,7 @@ export type RuntimePtySpawnArgs = {
|
||||
tabId?: string
|
||||
leafId?: string
|
||||
sessionId?: string
|
||||
shellOverride?: string
|
||||
isNewSession?: boolean
|
||||
persistHostSessionBinding?: boolean
|
||||
expectedSourceBinding?: PtyBindingSourceExpectation
|
||||
|
||||
@@ -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 } : {}),
|
||||
|
||||
@@ -146,6 +146,7 @@ export class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTerminalCreate
|
||||
preAllocatedHandle,
|
||||
tabId,
|
||||
leafId,
|
||||
...(launchOpts.shellOverride ? { shellOverride: launchOpts.shellOverride } : {}),
|
||||
...(terminalColorQueryReplies ? { terminalColorQueryReplies } : {}),
|
||||
...(launchOpts.agentSessionClaim
|
||||
? {
|
||||
|
||||
@@ -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.`)
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, string>
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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] })
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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/)
|
||||
})
|
||||
})
|
||||
@@ -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({
|
||||
|
||||
@@ -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 =
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<string> = 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()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user