From 13ba649c2244a57733712766e6fdfe7f2bfe1a17 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:34:16 -0700 Subject: [PATCH 1/2] fix(terminal): let a runtime-created Windows terminal BE the requested shell (#20825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(terminal): let a runtime-created Windows terminal BE the requested shell `orca terminal create --environment --command 'cmd.exe'` never created a cmd terminal. `--command` is text the provider TYPES into whatever shell it spawned, so the PTY stayed the host's default shell with cmd running inside it. Captured on `awin`, whose default is Git Bash: $ orca terminal create --environment awin --command 'cmd.exe' --json $ orca terminal send --environment awin --terminal term_10656cf7... \ --text exit --enter $ orca terminal read --environment awin --terminal term_10656cf7... --screen neil@awin MINGW64 ~/orca/orca ((30f820708f...)) $ cmd.exe Microsoft Windows [Version 10.0.26200.9445] C:\Users\neil\orca\orca>exit neil@awin MINGW64 ~/orca/orca ((30f820708f...)) $ The handle is alive the whole time and `terminal list` shows one healthy terminal, because the PTY never changed — so the only symptom is that the caller's terminal is now a shell it never asked for, and every later `send` is quoted for the wrong one. On `win-lowspec` (default pwsh) the same create lands cmd inside PowerShell. Root cause ---------- There are two spawn preflights and they are twins: - `src/main/ipc/pty/ipc/spawn-preflight.ts` — renderer/IPC spawns, i.e. a terminal tab opened in the app. - `src/main/ipc/pty/runtime/spawn-preflight.ts` — runtime spawns: the CLI's `terminal.create`, headless `orca serve`, and every paired remote environment. Only the IPC twin read the caller's requested shell. The runtime twin passed a literal `requestedShellOverride: undefined`, so a runtime-created terminal on Windows could only ever be the host default. Everything downstream of that point — `spawn-options`, the daemon, `resolvePtyShellOverride` in the relay, `local-pty-launch-plan` — already honoured `shellOverride`; nothing upstream could supply one. Change ------ - Thread `shellOverride` through the runtime lane: `RuntimePtySpawnArgs` -> runtime `spawn-preflight` -> `RuntimePtyController.spawn` -> `TerminalCreateOptions` -> the `terminal.create` RPC's new `shell` param -> `orca terminal create --shell`. - Thread it through the renderer-backed lane too (`createDesktopTerminal` -> `terminal:requestTabCreate` -> `store.createTab`), so `--shell --focus` is not silently dropped on a local Windows app. - An agent launch quotes its startup command for the shell it will actually run in, so a requested shell now owns the startup-shell family instead of the global `terminalWindowsShell` setting. - Lift the relay's `ALLOWED_WINDOWS_SHELL_OVERRIDES` into `isSupportedWindowsShellOverride` in `src/shared/windows-terminal-shell.ts` (membership unchanged) so the CLI, the zod param schema, and the relay refuse the same names. `--shell` therefore cannot carry a path or a command line into `pty.spawn`; only allowlisted bare shell names pass. - Gate on `TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY`. An older host strips the unknown `shell` param and answers with a healthy terminal running its default shell — a reply indistinguishable from success — so the CLI refuses before creating anything rather than creating the wrong shell quietly. `--shell` stays Windows-only; macOS and Linux hosts spawn the login shell and the relay drops the value off win32 rather than honouring it half-way. A WSL project runtime still outranks it, unchanged. Tests ----- - `pty-spawn-shell-override-parity.test.ts` pins both preflights against the exact drift that caused this (verified failing with the fix reverted). - `createTerminal` passes `shellOverride` to `ptyController.spawn` with no startup command. - CLI: sends `shell`, refuses a shell the host cannot spawn, and refuses a host without the capability — in both refusals without making the round trip. - Allowlist and `terminal.create` schema accept/refuse cases, including paths and appended arguments. * fix(terminal): refuse a requested shell the execution host cannot apply The first commit made `--shell` reach the spawn, but only a LOCAL win32 execution host applies it: `spawn-options` gates the override on `process.platform === 'win32' && !args.connectionId`. So `--shell cmd.exe` against an SSH-routed worktree, or against a macOS/Linux host, still returned a healthy terminal running that host's default shell — the same indistinguishable-from-success reply the capability gate exists to prevent, one layer down. Refuse instead, before anything spawns. The check sits at the top of `resolveAgentTerminalCreateOptions`, which every create lane funnels through, so neither lane has to remember it; the desktop lane additionally refuses a worktree-less create, which has no execution host to resolve a shell on. An SSH host's platform and installed shells are not visible to this runtime, and a POSIX host has no Windows shell to pick. Neither can honour the request, and saying so is the whole point of the flag. Docs and the CLI spec now say "refused", not "ignored". * fix(terminal): refuse a shell that contradicts the project execution runtime `resolveLocalWindowsTerminalRuntimeOptions` does not merely rank the project's execution runtime above a per-terminal pick -- it REWRITES the pick, in both directions, and says nothing: - a WSL project forces `wsl.exe`, discarding `--shell cmd.exe`; - a Windows-host project discards a WSL name and falls back to `COMSPEC` (`getHostShellForProjectRuntime`), so `--shell wsl.exe` spawns cmd. That is the common case, not an edge: `resolveProjectExecutionRuntime` resolves `windows-host` for every project that is not WSL, while a repo belonging to no project honours `wsl.exe` -- so the same flag behaved differently depending on whether the repo was in a project. Either rewrite returns a healthy terminal running a shell the caller did not ask for, which is the failure `--shell` exists to remove. It also split an agent launch's quoting from the shell that receives it. The previous commit made the startup-shell family follow the REQUESTED shell, so `--shell wsl.exe --command codex` on a Windows-host project typed POSIX-quoted launch args into cmd. Refusing the contradiction removes that case rather than papering over it. Refuse instead, alongside the SSH and non-Windows refusals, from the same `resolveAgentTerminalCreateOptions` seam every create lane funnels through. Also from review: - the allowlist test looped the list against itself; spell the members out. - the runtime spec case claimed to prove the pty's shell when it asserts the controller received the field; name it for what it checks. Reported by an adversarial review of the branch. * fix(terminal): canonicalize --shell and refuse a WSL-path rewrite Review of the --shell create path turned up two ways the terminal could still end up being a shell the caller never asked for -- the exact failure --shell exists to remove. Bare and mixed-case spellings passed the allowlist but reached consumers that exact-match the canonical name: resolveWindowsShellStartupFamily classified `cmd` as the PowerShell family, resolveWindowsShellLaunchArgs fell through to empty shellArgs (no `chcp 65001`, no OSC 133 bootstrap that Windows foreground status depends on), and resolveWindowsGitBashShellPath compares case-sensitively so `Git-Bash` spawned a literal `Git-Bash`. The allowlist is now one canonical-name map and terminal.create canonicalizes on parse, so the spawn path only ever sees `.exe` spellings. `pwsh` and `powershell` stay distinct binaries. A `\\wsl$\\...` cwd made the providers force wsl.exe regardless of the request, and terminalShellOverrideRefusal only inspected the project runtime -- undefined for a folder workspace with no project. Refuse on the resolved cwd and the workspace path, judging what the PTY actually gets. Also: the capability gate reported an unreachable host as too old rather than unavailable; the SSH CLI shim dropped capabilities from status, so --shell there blamed the host version instead of naming SSH; and --shell had no help entry, rendering bare in `orca terminal create --help`. Adding that entry crossed help.ts's max-lines cap, so the flag table moved to flag-help-text.ts rather than suppressing the rule. Adds a behavioural test for the runtime preflight (the one-line fix was pinned only by a source-text scan), plus coverage for the startup-command quoting family, the no-workspace refusal, and the WSL-path refusal. * fix(build): keep tests out of the RPC params catalog bundle The catalog walk under methods/ already skips *.test.ts, but the contract directory glob took every .ts. terminal-create-shell-param.test.ts is the first test to live there, so the bundle pulled vitest into a CJS build and the generator threw on require(). Same exclusion, same reason. --- .gitignore | 1 + AGENTS.md | 1 + .../scripts/generate-rpc-params-catalog.mjs | 6 +- .../windows-terminal-shell-selection.md | 54 +++++ pnpm-workspace.yaml | 2 +- src/cli/flag-help-text.ts | 106 +++++++++ src/cli/handlers/terminal.test.ts | 95 +++++++- src/cli/handlers/terminal.ts | 36 ++++ src/cli/help.ts | 110 +--------- src/cli/root-help-text-secondary.ts | 2 +- src/cli/specs/core.ts | 11 +- .../pty-spawn-shell-override-parity.test.ts | 26 +++ .../spawn-preflight-requested-shell.test.ts | 83 +++++++ src/main/ipc/pty/runtime/spawn-preflight.ts | 7 +- src/main/ipc/pty/runtime/spawn-state.ts | 1 + .../orca-runtime-create-terminal-desktop.ts | 8 + .../runtime/orca-runtime-create-terminal.ts | 1 + ...runtime-resolve-worktree-removal-target.ts | 23 +- .../terminal-creation-and-readiness.spec.ts | 126 ++++++++++- .../terminal/terminal-lifecycle-methods.ts | 1 + .../runtime-pty-controller-contract.ts | 2 + .../runtime/runtime-terminal-contracts.ts | 9 + ...rminal-shell-override-host-support.test.ts | 204 ++++++++++++++++++ .../terminal-shell-override-host-support.ts | 90 ++++++++ src/main/ssh/ssh-remote-orca-cli.test.ts | 30 +++ src/main/ssh/ssh-remote-orca-cli.ts | 5 +- src/relay/pty-handler.ts | 21 +- .../ipc-events/terminal-request-ipc-bridge.ts | 2 +- src/shared/protocol-version.ts | 6 + .../terminal-create-shell-param.test.ts | 51 +++++ .../rpc-contract/terminal-unary-params.ts | 18 +- src/shared/runtime-terminal-contracts.ts | 2 + .../source-scan/source-tree-walk.test.ts | 1 - src/shared/windows-terminal-shell.test.ts | 87 +++++++- src/shared/windows-terminal-shell.ts | 44 ++++ 35 files changed, 1129 insertions(+), 143 deletions(-) create mode 100644 docs/reference/windows-terminal-shell-selection.md create mode 100644 src/cli/flag-help-text.ts create mode 100644 src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts create mode 100644 src/main/ipc/pty/runtime/spawn-preflight-requested-shell.test.ts create mode 100644 src/main/runtime/terminal-shell-override-host-support.test.ts create mode 100644 src/main/runtime/terminal-shell-override-host-support.ts create mode 100644 src/shared/rpc-contract/terminal-create-shell-param.test.ts diff --git a/.gitignore b/.gitignore index 3d51edb0009..97fc6affcac 100644 --- a/.gitignore +++ b/.gitignore @@ -130,6 +130,7 @@ docs/** !docs/reference/ssh-host-key-verification.md !docs/reference/ssh-reconnect-source-recovery.md !docs/reference/windows-setup-shell.md +!docs/reference/windows-terminal-shell-selection.md !docs/reference/worktree-scan-fingerprint.md !docs/reference/wsl-command-execution.md !docs/reference/wsl-probe-failure-semantics.md diff --git a/AGENTS.md b/AGENTS.md index 0c11d13a9ca..664d986da13 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,6 +61,7 @@ Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior beh - **Keyboard shortcuts**: Never hardcode `e.metaKey`. Use a platform check (`navigator.userAgent.includes('Mac')`) to pick `metaKey` on Mac and `ctrlKey` on Linux/Windows. Electron menu accelerators should use `CmdOrCtrl`. - **Shortcut labels in UI**: Display `⌘` / `⇧` on Mac and `Ctrl+` / `Shift+` on other platforms. - **File paths**: Use `path.join` or Electron/Node path utilities — never assume `/` or `\`. +- **Windows terminal shells**: `--shell` picks the shell a terminal *is*; `--command` is typed into whatever shell the host spawned, so a shell choice routed through `command` silently becomes a child process. See [`docs/reference/windows-terminal-shell-selection.md`](./docs/reference/windows-terminal-shell-selection.md). - **Windows setup scripts**: the setup/issue-command runner is a `.cmd` batch file unless the script starts with a `#!` line — never derive that from the user's terminal-shell preference, and never launch a `.cmd` runner with a bare `cmd.exe /c` from a Git Bash pane (MSYS rewrites the `/c`). See [`docs/reference/windows-setup-shell.md`](./docs/reference/windows-setup-shell.md). - **Windows child processes**: start them through `runProcess`/`spawnProcess` in `src/shared/child-process/` — never `child_process` directly. It pins `windowsHide`, refuses `shell: true`, and encodes `.cmd`/`.bat` arguments so neither `CommandLineToArgvW` nor `cmd.exe` mangles them. A ratchet test fails on any new direct import. Recognised npm/pnpm `.cmd` shims are resolved to their real target so the spawn skips `cmd.exe` entirely; see [`docs/reference/windows-cmd-shim-resolution.md`](./docs/reference/windows-cmd-shim-resolution.md) before adding a shim shape or debugging one. - **Windows process enumeration**: read the table through `src/main/windows/windows-process-table.ts`, never by forking `powershell.exe`. See [`docs/reference/windows-process-enumeration.md`](./docs/reference/windows-process-enumeration.md). diff --git a/config/scripts/generate-rpc-params-catalog.mjs b/config/scripts/generate-rpc-params-catalog.mjs index 9810f952e77..bd7ddcd53f6 100644 --- a/config/scripts/generate-rpc-params-catalog.mjs +++ b/config/scripts/generate-rpc-params-catalog.mjs @@ -40,8 +40,12 @@ const repoPath = (absolute) => posix(path.relative(REPO_ROOT, absolute)) // Every module the catalog may import from: the extracted params modules plus the // pre-existing src/shared schemas the RPC methods already bind directly. function indexableModules() { + // Tests are excluded here for the same reason as the RPC_DIR walk below: bundling one pulls + // vitest into the CJS catalog build, which throws on require(). const modules = new Set( - globSync('*.ts', { cwd: CONTRACT_DIR }).map((name) => path.join(CONTRACT_DIR, name)) + globSync('*.ts', { cwd: CONTRACT_DIR }) + .filter((name) => !name.endsWith('.test.ts')) + .map((name) => path.join(CONTRACT_DIR, name)) ) modules.delete(OUTPUT_PATH) for (const file of globSync('**/*.ts', { cwd: RPC_DIR })) { diff --git a/docs/reference/windows-terminal-shell-selection.md b/docs/reference/windows-terminal-shell-selection.md new file mode 100644 index 00000000000..b0e361c5447 --- /dev/null +++ b/docs/reference/windows-terminal-shell-selection.md @@ -0,0 +1,54 @@ +# Windows terminal shell selection + +Two different things can put `cmd.exe` on a Windows terminal, and only one of them makes the +terminal _be_ cmd. + +- **`--shell` / `shellOverride`** names the executable the PTY is spawned as. The terminal's own + process is that shell for its whole life. +- **`--command` / `startupCommand`** is text the provider types into whatever shell it spawned. + `--command cmd.exe` therefore starts cmd as a **child** of the host's default shell. + +The difference is invisible until the child exits. Leaving that cmd returns the caller's handle to +a Git Bash or PowerShell prompt it never asked for, and anything that keyed off "this terminal is +cmd" is now wrong — while `terminal list` still shows one connected, healthy terminal, because the +PTY never changed. + +## Why the runtime path needed its own fix + +There are two spawn preflights, and they are twins: + +- `src/main/ipc/pty/ipc/spawn-preflight.ts` — renderer/IPC spawns (a terminal tab in the app). +- `src/main/ipc/pty/runtime/spawn-preflight.ts` — runtime spawns: `terminal.create` from the CLI, + headless `orca serve`, and every paired remote environment. + +Only the IPC twin read the caller's requested shell. The runtime twin passed a literal `undefined`, +so a runtime-created terminal could only ever be the host's default shell. `orca terminal create +--command cmd.exe` against a Windows environment had no way to say "be cmd" — it could only type +`cmd.exe` into Git Bash. `src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts` pins the pair. + +## Rules + +- A caller choosing a shell passes `--shell`; a caller running a program passes `--command`. Do not + route a shell choice through `command` — it looks like it worked. +- The allowlist is `isSupportedWindowsShellOverride` in `src/shared/windows-terminal-shell.ts`, and + it is the reason `--shell` cannot name an arbitrary executable. The CLI, the `terminal.create` + RPC schema, and the relay all check the same set; add a shell in one place only. +- Bare shell names only. A path or anything with arguments is refused, so `--shell` can never carry + a command line into `pty.spawn`. +- A host that predates `--shell` STRIPS it (`terminal.create` params are a zod object, which drops + unknown keys) and answers with a healthy terminal running its default shell — a reply that reads + as success. So the CLI gates on `TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY` and refuses + before creating anything, rather than creating the wrong shell quietly. +- `--shell` is Windows-only, and a host that cannot apply it REFUSES the create + (`terminalShellOverrideRefusal`). macOS and Linux execution hosts spawn the login shell, and a + terminal routed over SSH resolves its shell on the SSH host, whose platform and installed shells + this runtime cannot see. Refusing is the point: spawning the default shell and reporting success + is the failure `--shell` exists to remove. +- A project's execution runtime decides which MACHINE the shell runs on, so it outranks a + per-terminal pick — but it outranks it by REFUSING, not by rewriting. A `--shell` that + contradicts the project runtime (a Windows shell on a WSL project, or a WSL name on a + Windows-host project) is refused. `resolveLocalWindowsTerminalRuntimeOptions` would otherwise + rewrite the value — a WSL project forces `wsl.exe`, a Windows-host project discards a WSL name in + favour of `COMSPEC` — and hand back a terminal running something the caller never asked for. It + also splits an agent launch's quoting from the shell that receives it: POSIX-quoted args typed + into cmd, or cmd-quoted args typed into a WSL shell. diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 68ae103f6ec..92de08dfe22 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -54,7 +54,7 @@ allowBuilds: node-pty: true sherpa-onnx: true ssh2: false - "@orca/windows-registry": false + '@orca/windows-registry': false overrides: monaco-editor>dompurify: 3.4.14 diff --git a/src/cli/flag-help-text.ts b/src/cli/flag-help-text.ts new file mode 100644 index 00000000000..3cdee00060d --- /dev/null +++ b/src/cli/flag-help-text.ts @@ -0,0 +1,106 @@ +/** One-line flag descriptions shared by every command's help output. */ +export const FLAG_HELP_TEXT: Record = { + agent: '--agent Launch a known TUI agent in the first terminal', + 'base-branch': '--base-branch Base branch/ref to create the worktree from', + command: '--command Command to run in the terminal on startup', + comment: '--comment Comment stored in Orca metadata', + cursor: '--cursor Line cursor from a previous read (returns only new output)', + action: '--action Secondary accessibility action name', + activate: '--activate Reveal the new worktree in the Orca app', + app: '--app App name, bundle ID, or pid:N', + direction: + '--direction Direction: up|down|left|right for scroll, horizontal|vertical for split', + 'display-name': '--display-name Override the Orca display name', + 'element-index': '--element-index Element index from get-app-state', + title: '--title Custom title for the terminal tab (omit to reset)', + enter: '--enter Append Enter after sending text', + force: + '--force Force worktree removal when supported; does not force branch deletion', + focus: '--focus Reveal the created terminal session in Orca', + for: '--for exit|tui-idle Wait condition to satisfy', + 'from-element-index': '--from-element-index Source element index from get-app-state', + 'from-x': '--from-x Source window-local x coordinate', + 'from-y': '--from-y Source window-local y coordinate', + help: '--help Show this help message', + 'include-visual-layouts': '--include-visual-layouts Include tab and pane topology in JSON output', + interrupt: '--interrupt Send as an interrupt-style input when supported', + id: '--id Identifier for a target item or permission', + issue: '--issue Linked GitHub issue number', + 'linear-issue': + '--linear-issue Linked Linear issue identifier or URL; null clears on set', + json: '--json Emit machine-readable JSON', + key: '--key Key argument for this command', + limit: '--limit Maximum number of rows to return', + local: '--local Target the current project instead of the global install', + skill: '--skill Bundled skill to act on; repeat for several', + mode: '--mode Mode such as edit, diff, or both', + model: '--model Provider model id for a new agent launch', + effort: '--effort Reasoning effort for the selected model', + 'mouse-button': '--mouse-button Mouse button: left, right, or middle', + modifiers: '--modifiers Modifier keys held only for this click', + name: '--name Name for the new worktree or automation', + 'no-parent': '--no-parent Force no parent lineage for unrelated work', + 'no-screenshot': '--no-screenshot Skip screenshot capture after the operation', + pages: '--pages Number of scroll pages', + 'parent-worktree': + '--parent-worktree Parent worktree selector such as identity:, id:::, branch:, issue:, path:, or active/current', + path: '--path Path argument for the command', + prompt: '--prompt Prompt text for agent-backed commands', + query: '--query Search text for matching refs', + ref: '--ref Base ref to persist for the repo', + repo: '--repo Repo selector such as id:, name:, or path:', + 'restore-window': '--restore-window Bring the target app/window forward before the operation', + session: '--session Snapshot namespace for a related computer-use workflow', + setup: '--setup run|skip|inherit Setup policy for repo-defined setup hooks', + shell: '--shell Windows shell the terminal itself runs as', + terminal: '--terminal Runtime-issued terminal handle', + text: '--text Text payload to send or type', + 'text-stdin': '--text-stdin Read text payload from stdin', + 'task-id': '--task-id Task id to include in orchestration payload JSON', + 'task-title': '--task-title Concise title for an orchestration task', + 'dispatch-id': '--dispatch-id Dispatch id to include in orchestration payload JSON', + 'files-modified': '--files-modified Comma-separated files for orchestration payload JSON', + 'report-path': '--report-path Report path to include in orchestration payload JSON', + phase: '--phase Worker phase to include in orchestration payload JSON', + 'timeout-ms': '--timeout-ms Maximum wait time before timing out', + 'to-element-index': '--to-element-index Destination element index from get-app-state', + 'to-x': '--to-x Destination window-local x coordinate', + 'to-y': '--to-y Destination window-local y coordinate', + worktree: + '--worktree Worktree selector such as identity:, id:::, name:, branch:, issue:, path:, or active/current', + workspace: '--workspace Existing worktree selector for automation runs', + 'workspace-status': + '--workspace-status Board status id (defaults: todo, in-progress, in-review, completed)', + staged: '--staged Open staged source-control changes', + provider: '--provider Agent id such as codex, claude, or gemini', + 'source-context': + '--source-context Explicit TaskSourceContext for automation task/provider data', + trigger: '--trigger Automation schedule preset, cron, or RRULE', + schedule: '--schedule Alias for --trigger', + time: '--time Time used with daily/weekdays/weekly presets', + day: '--day <0-6> Day used with weekly preset, Sunday=0', + timezone: '--timezone IANA timezone for the automation', + enabled: '--enabled Enable the automation', + disabled: '--disabled Disable the automation', + 'reuse-session': + '--reuse-session Reuse the previous live session for existing-workspace runs', + 'fresh-session': '--fresh-session Disable session reuse for future runs', + 'workspace-mode': '--workspace-mode existing or new-per-run', + 'missed-run-grace-minutes': '--missed-run-grace-minutes Missed-run grace window', + 'value-stdin': '--value-stdin Read set-value payload from stdin', + 'window-id': '--window-id Target a window id from list-windows', + 'window-index': '--window-index Target a window index from list-windows', + // Browser automation flags + element: '--element Element ref from snapshot (e.g. e3)', + url: '--url URL to navigate to', + value: '--value Value to fill or select', + input: '--input Text to type at current focus', + expression: '--expression JavaScript expression to evaluate', + amount: '--amount Scroll distance in pixels', + index: '--index Tab index to switch to', + page: '--page Stable browser page id from `orca tab list --json`', + profile: '--profile Browser profile id', + 'show-profile': '--show-profile Include tab profile in text output', + 'no-ua-spoof': "--no-ua-spoof Keep Electron's native user agent", + format: '--format Screenshot image format' +} diff --git a/src/cli/handlers/terminal.test.ts b/src/cli/handlers/terminal.test.ts index 21f296b261a..90dbc7fd459 100644 --- a/src/cli/handlers/terminal.test.ts +++ b/src/cli/handlers/terminal.test.ts @@ -1,6 +1,9 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { RuntimeClientError, type RuntimeClient } from '../runtime-client' -import { TERMINAL_PROMPT_DELIVERY_RUNTIME_CAPABILITY } from '../../shared/protocol-version' +import { + TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY, + TERMINAL_PROMPT_DELIVERY_RUNTIME_CAPABILITY +} from '../../shared/protocol-version' import { parseArgs } from '../args' import { printHelp } from '../help' import { COMMAND_SPECS } from '../specs' @@ -685,3 +688,93 @@ describe('terminal send CLI', () => { ]) }) }) + +describe('terminal create --shell', () => { + const WORKTREE = 'path:C:/src/app' + + const shellClient = ( + call: ReturnType, + supported: boolean, + reachable = true + ): RuntimeClient => { + const client = { + call, + isRemote: false, + getCliStatus: vi.fn().mockResolvedValue({ + result: { + runtime: reachable + ? { + reachable: true, + runtimeId: 'runtime-current', + capabilities: supported ? [TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY] : [] + } + : { reachable: false, runtimeId: null } + } + }) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: `terminal create` reads only `call`, `isRemote`, and `getCliStatus`, all stubbed above; RuntimeClient is a class, so a structural double cannot satisfy it without the cast. + return client as unknown as RuntimeClient + } + + afterEach(() => { + vi.restoreAllMocks() + process.exitCode = ORIGINAL_EXIT_CODE + }) + + function createTerminal(client: RuntimeClient, shell: string) { + return TERMINAL_HANDLERS['terminal create']({ + flags: new Map([ + ['worktree', WORKTREE], + ['shell', shell] + ]), + client, + cwd: 'C:/src/app', + json: true + }) + } + + it('sends the shell selection alongside an empty startup command', async () => { + const call = vi.fn().mockResolvedValue({ + result: { terminal: { handle: 'term_1', worktreeId: 'repo::C:/src/app', title: null } } + }) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await createTerminal(shellClient(call, true), 'cmd.exe') + + expect(call).toHaveBeenCalledWith( + 'terminal.create', + expect.objectContaining({ shell: 'cmd.exe', command: undefined }) + ) + }) + + it('refuses a shell the host cannot spawn without making the round trip', async () => { + const call = vi.fn() + + await expect(createTerminal(shellClient(call, true), 'nu.exe')).rejects.toThrow( + /--shell must be one of/ + ) + expect(call).not.toHaveBeenCalled() + }) + + // An older host strips the unknown param and answers with its default shell, which reads as a + // successful create. Creating the wrong shell silently is worse than refusing. + it('refuses rather than creating a default-shell terminal on a host without the capability', async () => { + const call = vi.fn() + + await expect(createTerminal(shellClient(call, false), 'cmd.exe')).rejects.toThrow( + /does not support --shell/ + ) + expect(call).not.toHaveBeenCalled() + }) + + // A status probe that fails or times out reports no capabilities either; blaming the host + // version would send the caller to update a host that may already be current. + it('reports an unreachable host as unavailable rather than incompatible', async () => { + const call = vi.fn() + + await expect(createTerminal(shellClient(call, false, false), 'cmd.exe')).rejects.toMatchObject({ + code: 'runtime_unavailable' + }) + expect(call).not.toHaveBeenCalled() + }) +}) diff --git a/src/cli/handlers/terminal.ts b/src/cli/handlers/terminal.ts index c409a6a9f9a..2da49ad8ca8 100644 --- a/src/cli/handlers/terminal.ts +++ b/src/cli/handlers/terminal.ts @@ -31,6 +31,11 @@ import { type WithAnnotatedHostScope } from '../omitted-host-scope-selectors' import { RuntimeClientError } from '../runtime-client' +import { + isSupportedWindowsShellOverride, + listSupportedWindowsShellOverrides +} from '../../shared/windows-terminal-shell' +import { TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY } from '../../shared/protocol-version' import { getBrowserWorktreeSelector, getOptionalWorktreeSelector, @@ -153,9 +158,40 @@ export const TERMINAL_HANDLERS: Record = { const useRendererBackedInteractiveTerminal = !client.isRemote && shouldUseRendererBackedInteractiveTerminal(command) const focus = flags.get('focus') === true + const shell = getOptionalStringFlag(flags, 'shell') + if (shell !== undefined) { + if (!isSupportedWindowsShellOverride(shell)) { + throw new RuntimeClientError( + 'invalid_argument', + `--shell must be one of: ${listSupportedWindowsShellOverrides().join(', ')}` + ) + } + // Why refused rather than sent hopefully: an older host strips the unknown param and hands + // back a healthy terminal running its DEFAULT shell. Nothing in that reply says the shell + // was ignored, so a caller that wanted cmd would drive a PowerShell session believing it won. + const status = await client.getCliStatus() + // An unreachable host reports no capabilities at all; that is not evidence it lacks --shell. + if (!status.result.runtime.reachable) { + throw new RuntimeClientError( + 'runtime_unavailable', + 'Orca could not verify --shell support on the execution host, so no terminal was created. Wait for the execution host to become reachable and retry.' + ) + } + if ( + status.result.runtime.capabilities?.includes( + TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY + ) !== true + ) { + throw new RuntimeClientError( + 'incompatible_runtime', + 'This Orca host does not support --shell, and would silently create a terminal running its default shell instead. No terminal was created; update Orca on the execution host.' + ) + } + } const result = await client.call<{ terminal: RuntimeTerminalCreate }>('terminal.create', { worktree: await getBrowserWorktreeSelector(flags, cwd, client), command, + ...(shell !== undefined ? { shell } : {}), title: getOptionalStringFlag(flags, 'title'), // Why: interactive local agent TUIs need the renderer-backed terminal // path for browser-side features, but CLI creates must stay backgrounded diff --git a/src/cli/help.ts b/src/cli/help.ts index 9d722388ae4..79e5b2b92c4 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -1,6 +1,7 @@ import type { CommandSpec } from './args' import { findCommandSpec, isCommandGroup, supportsBrowserPageFlag } from './args' import { unknownCommandData } from './command-suggestion' +import { FLAG_HELP_TEXT } from './flag-help-text' import { formatSkillsCommandFlagHelp } from './skills-command-flag-help' import { ROOT_HELP_TEXT_PRIMARY } from './root-help-text-primary' import { ROOT_HELP_TEXT_SECONDARY } from './root-help-text-secondary' @@ -182,113 +183,6 @@ function formatCommandFlagHelp(flag: string, commandPath: string[]): string { } export function formatFlagHelp(flag: string): string { - const helpByFlag: Record = { - agent: '--agent Launch a known TUI agent in the first terminal', - 'base-branch': '--base-branch Base branch/ref to create the worktree from', - command: '--command Command to run in the terminal on startup', - comment: '--comment Comment stored in Orca metadata', - cursor: '--cursor Line cursor from a previous read (returns only new output)', - action: '--action Secondary accessibility action name', - activate: '--activate Reveal the new worktree in the Orca app', - app: '--app App name, bundle ID, or pid:N', - direction: - '--direction Direction: up|down|left|right for scroll, horizontal|vertical for split', - 'display-name': '--display-name Override the Orca display name', - 'element-index': '--element-index Element index from get-app-state', - title: '--title Custom title for the terminal tab (omit to reset)', - enter: '--enter Append Enter after sending text', - force: - '--force Force worktree removal when supported; does not force branch deletion', - focus: '--focus Reveal the created terminal session in Orca', - for: '--for exit|tui-idle Wait condition to satisfy', - 'from-element-index': '--from-element-index Source element index from get-app-state', - 'from-x': '--from-x Source window-local x coordinate', - 'from-y': '--from-y Source window-local y coordinate', - help: '--help Show this help message', - 'include-visual-layouts': - '--include-visual-layouts Include tab and pane topology in JSON output', - interrupt: '--interrupt Send as an interrupt-style input when supported', - id: '--id Identifier for a target item or permission', - issue: '--issue Linked GitHub issue number', - 'linear-issue': - '--linear-issue Linked Linear issue identifier or URL; null clears on set', - json: '--json Emit machine-readable JSON', - key: '--key Key argument for this command', - limit: '--limit Maximum number of rows to return', - local: '--local Target the current project instead of the global install', - skill: '--skill Bundled skill to act on; repeat for several', - mode: '--mode Mode such as edit, diff, or both', - model: '--model Provider model id for a new agent launch', - effort: '--effort Reasoning effort for the selected model', - 'mouse-button': '--mouse-button Mouse button: left, right, or middle', - modifiers: '--modifiers Modifier keys held only for this click', - name: '--name Name for the new worktree or automation', - 'no-parent': '--no-parent Force no parent lineage for unrelated work', - 'no-screenshot': '--no-screenshot Skip screenshot capture after the operation', - pages: '--pages Number of scroll pages', - 'parent-worktree': - '--parent-worktree Parent worktree selector such as identity:, id:::, branch:, issue:, path:, or active/current', - path: '--path Path argument for the command', - prompt: '--prompt Prompt text for agent-backed commands', - query: '--query Search text for matching refs', - ref: '--ref Base ref to persist for the repo', - repo: '--repo Repo selector such as id:, name:, or path:', - 'restore-window': - '--restore-window Bring the target app/window forward before the operation', - session: '--session Snapshot namespace for a related computer-use workflow', - setup: '--setup run|skip|inherit Setup policy for repo-defined setup hooks', - terminal: '--terminal Runtime-issued terminal handle', - text: '--text Text payload to send or type', - 'text-stdin': '--text-stdin Read text payload from stdin', - 'task-id': '--task-id Task id to include in orchestration payload JSON', - 'task-title': '--task-title Concise title for an orchestration task', - 'dispatch-id': '--dispatch-id Dispatch id to include in orchestration payload JSON', - 'files-modified': '--files-modified Comma-separated files for orchestration payload JSON', - 'report-path': '--report-path Report path to include in orchestration payload JSON', - phase: '--phase Worker phase to include in orchestration payload JSON', - 'timeout-ms': '--timeout-ms Maximum wait time before timing out', - 'to-element-index': '--to-element-index Destination element index from get-app-state', - 'to-x': '--to-x Destination window-local x coordinate', - 'to-y': '--to-y Destination window-local y coordinate', - worktree: - '--worktree Worktree selector such as identity:, id:::, name:, branch:, issue:, path:, or active/current', - workspace: '--workspace Existing worktree selector for automation runs', - 'workspace-status': - '--workspace-status Board status id (defaults: todo, in-progress, in-review, completed)', - staged: '--staged Open staged source-control changes', - provider: '--provider Agent id such as codex, claude, or gemini', - 'source-context': - '--source-context Explicit TaskSourceContext for automation task/provider data', - trigger: '--trigger Automation schedule preset, cron, or RRULE', - schedule: '--schedule Alias for --trigger', - time: '--time Time used with daily/weekdays/weekly presets', - day: '--day <0-6> Day used with weekly preset, Sunday=0', - timezone: '--timezone IANA timezone for the automation', - enabled: '--enabled Enable the automation', - disabled: '--disabled Disable the automation', - 'reuse-session': - '--reuse-session Reuse the previous live session for existing-workspace runs', - 'fresh-session': '--fresh-session Disable session reuse for future runs', - 'workspace-mode': '--workspace-mode existing or new-per-run', - 'missed-run-grace-minutes': '--missed-run-grace-minutes Missed-run grace window', - 'value-stdin': '--value-stdin Read set-value payload from stdin', - 'window-id': '--window-id Target a window id from list-windows', - 'window-index': '--window-index Target a window index from list-windows', - // Browser automation flags - element: '--element Element ref from snapshot (e.g. e3)', - url: '--url URL to navigate to', - value: '--value Value to fill or select', - input: '--input Text to type at current focus', - expression: '--expression JavaScript expression to evaluate', - amount: '--amount Scroll distance in pixels', - index: '--index Tab index to switch to', - page: '--page Stable browser page id from `orca tab list --json`', - profile: '--profile Browser profile id', - 'show-profile': '--show-profile Include tab profile in text output', - 'no-ua-spoof': "--no-ua-spoof Keep Electron's native user agent", - format: '--format Screenshot image format' - } - if (flag === 'current') { return '--current Use the current Orca worktree linked Linear issue' } @@ -314,5 +208,5 @@ export function formatFlagHelp(flag: string): string { return '--full Include all supported V1 issue context within caps' } - return helpByFlag[flag] ?? `--${flag}` + return FLAG_HELP_TEXT[flag] ?? `--${flag}` } diff --git a/src/cli/root-help-text-secondary.ts b/src/cli/root-help-text-secondary.ts index 324fe847855..7a892c94ec8 100644 --- a/src/cli/root-help-text-secondary.ts +++ b/src/cli/root-help-text-secondary.ts @@ -62,7 +62,7 @@ export const ROOT_HELP_TEXT_SECONDARY = [ ' orca terminal read [--terminal ] [--cursor ] [--limit ] [--json]', ' orca terminal send [--terminal ] [--text ] [--enter] [--interrupt] [--wait-submit ] [--retry-request ] [--json]', ' orca terminal wait [--terminal ] --for exit|tui-idle [--timeout-ms ] [--json]', - ' orca terminal create [--worktree ] [--title ] [--command ] [--focus] [--json]', + ' orca terminal create [--worktree ] [--title ] [--command ] [--shell ] [--focus] [--json]', ' orca terminal split [--terminal ] [--direction horizontal|vertical] [--json]', ' orca terminal switch [--terminal ] [--json]', ' orca terminal close ([--terminal ] [--tab] | --worktree --all) [--json]', diff --git a/src/cli/specs/core.ts b/src/cli/specs/core.ts index d029b0cd43d..98301b36e53 100644 --- a/src/cli/specs/core.ts +++ b/src/cli/specs/core.ts @@ -250,17 +250,20 @@ export const CORE_COMMAND_SPECS: CommandSpec[] = [ path: ['terminal', 'create'], summary: 'Create a terminal session in the current worktree', usage: - 'orca terminal create [--worktree ] [--title ] [--command ] [--focus] [--json]', - allowedFlags: [...GLOBAL_FLAGS, 'worktree', 'command', 'title', 'focus'], + 'orca terminal create [--worktree ] [--title ] [--command ] [--shell ] [--focus] [--json]', + allowedFlags: [...GLOBAL_FLAGS, 'worktree', 'command', 'shell', 'title', 'focus'], notes: [ 'Creates a visible terminal tab without switching focus when possible; falls back to a background handle if the UI cannot adopt it. Pass --focus to switch to it.', - 'Use this, not worktree create, for a fresh agent in the current checkout.' + 'Use this, not worktree create, for a fresh agent in the current checkout.', + '--shell picks the shell the terminal IS on a Windows host (cmd.exe, powershell.exe, pwsh.exe, wsl.exe, bash.exe, git-bash); --command is typed into whatever shell the host started, so `--command cmd.exe` leaves a cmd running INSIDE the default shell and exiting it drops back to that shell.', + 'A host that cannot apply --shell refuses the create rather than quietly spawning its default shell: macOS and Linux execution hosts spawn the login shell, terminals routed over SSH resolve their shell on the SSH host, a --shell that contradicts the project execution runtime (WSL vs Windows host) is refused, and an Orca host older than --shell is refused by the CLI.' ], examples: [ 'orca terminal create --json', 'orca terminal create --worktree active --command "codex" --json', 'orca terminal create --worktree path:/projects/myapp --title "RUNNER" --command "opencode"', - 'orca terminal create --worktree path:/projects/myapp --command "opencode" --focus' + 'orca terminal create --worktree path:/projects/myapp --command "opencode" --focus', + 'orca terminal create --worktree path:C:/src/app --shell cmd.exe --json' ] }, { diff --git a/src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts b/src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts new file mode 100644 index 00000000000..67bee9ca700 --- /dev/null +++ b/src/main/ipc/pty/pty-spawn-shell-override-parity.test.ts @@ -0,0 +1,26 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' + +/** + * The two spawn preflights are twins: one serves renderer/IPC spawns, the other serves runtime + * spawns (`terminal.create` from the CLI, headless serve, and paired remote environments). They + * had drifted — the runtime twin passed a literal `undefined` for the caller's shell, so on a + * Windows host a runtime-created terminal could only ever be the host's default shell. A caller + * asking for cmd or PowerShell had to send it as `command`, which the provider TYPES into that + * default shell: the pty stayed the default shell with the requested one running inside it, and + * leaving that child dropped the caller's handle back onto a prompt it never asked for. + * + * Source-level because the functional seam is a whole spawn pipeline; what actually regressed is + * one twin silently not reading a field the other reads. + */ +const PREFLIGHTS = ['ipc', 'runtime'] as const + +describe.each(PREFLIGHTS)('%s pty spawn preflight', (lane) => { + const source = readFileSync(join(__dirname, lane, 'spawn-preflight.ts'), 'utf8') + + it("resolves Windows terminal runtime options from the caller's requested shell", () => { + expect(source).toContain('requestedShellOverride: args.shellOverride') + expect(source).not.toContain('requestedShellOverride: undefined') + }) +}) diff --git a/src/main/ipc/pty/runtime/spawn-preflight-requested-shell.test.ts b/src/main/ipc/pty/runtime/spawn-preflight-requested-shell.test.ts new file mode 100644 index 00000000000..46f16081efa --- /dev/null +++ b/src/main/ipc/pty/runtime/spawn-preflight-requested-shell.test.ts @@ -0,0 +1,83 @@ +import { afterEach, describe, expect, it } from 'vitest' +import type { BrowserWindow } from 'electron' +import { getDefaultSettings } from '../../../../shared/constants' +import { finishPtyShutdown } from '../provider/liveness' +import { prepareRuntimePtySpawn } from './spawn-preflight' +import { buildRuntimePtySpawnOptions } from './spawn-options' +import { createRuntimePtySpawnState, type RuntimePtySpawnArgs } from './spawn-state' +import type { PtyRuntimeControllerDeps } from './controller-deps' + +const HOST_DEFAULT_SHELL = 'powershell.exe' +const hostPlatform = process.platform + +function makeDeps(): PtyRuntimeControllerDeps { + const noCodexResumeLaunch: PtyRuntimeControllerDeps['noCodexResumeLaunch'] = (command) => ({ + codexResumeHome: null, + command, + notifyResumeUnavailable: false, + droppedResumeArgv: false, + providerSession: null + }) + return { + store: undefined, + getSettings: () => ({ + ...getDefaultSettings('/tmp'), + terminalWindowsShell: HOST_DEFAULT_SHELL + }), + adoptStablePane: async () => null, + getLocalPtyStartupPromise: () => undefined, + getLocalPtyProviderStartupPromise: () => undefined, + prepareCodexResumeHome: () => null, + resolveCodexResumeLaunch: async (command) => noCodexResumeLaunch(command), + noCodexResumeLaunch, + reconcileSharedRuntimeResumeHome: async (resumeHome) => resumeHome.codexHomePath, + stripSequencedStartupResumeArgv: (env) => env, + assertFolderWorkspacePtyPathUsable: () => undefined, + resolvePtySpawnStartupCwd: (_worktreeId, cwd) => cwd, + requestSerializedBuffer: async () => null, + shutdownProviderAndDetectExit: async () => false, + rememberSyntheticKillExit: () => {}, + rememberRetiredRejectedPty: () => {}, + sendPtyExitToRenderer: () => {}, + sendPtySpawnedToRenderer: () => {}, + finishPtyShutdown, + trustedTerminalHandleEnv: new Set(), + retiredRejectedPtyIds: new Map(), + reversibleStopOwnersByPtyId: new Map(), + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: only `operations.ts` (write/clearBuffer) reads `mainWindow`; the spawn preflight and option build never touch it, and a real BrowserWindow cannot exist in vitest. + mainWindow: {} as BrowserWindow + } +} + +/** Runs the preflight and option build the way `spawnPtyFromRuntimeController` sequences them. */ +async function resolveSpawnShell(shellOverride: string | undefined): Promise { + const args: RuntimePtySpawnArgs = { cols: 120, rows: 40, shellOverride } + const ctx = createRuntimePtySpawnState(makeDeps(), args) + await prepareRuntimePtySpawn(ctx) + await buildRuntimePtySpawnOptions(ctx) + ctx.finishTerminalInstall() + return ctx.spawnOptions.shellOverride +} + +/** + * Behavioural twin of `pty-spawn-shell-override-parity.test.ts`: a local Windows runtime spawn + * (`terminal create --shell`, headless serve) must hand the caller's shell to the provider, not + * the host default with the request typed into it. + */ +describe('runtime pty spawn preflight: requested shell on a local Windows host', () => { + afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: hostPlatform }) + }) + + it('spawns the requested shell as the pty', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + + await expect(resolveSpawnShell('cmd.exe')).resolves.toBe('cmd.exe') + }) + + it('keeps the host default shell when nothing was requested', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + + await expect(resolveSpawnShell(undefined)).resolves.toBe(HOST_DEFAULT_SHELL) + }) +}) diff --git a/src/main/ipc/pty/runtime/spawn-preflight.ts b/src/main/ipc/pty/runtime/spawn-preflight.ts index 43fd2778119..aa743b259d0 100644 --- a/src/main/ipc/pty/runtime/spawn-preflight.ts +++ b/src/main/ipc/pty/runtime/spawn-preflight.ts @@ -72,10 +72,15 @@ export async function prepareRuntimePtySpawn( throw new Error(CLAUDE_AUTH_SWITCH_IN_PROGRESS_MESSAGE) } // Why: runtime-created terminals carry no renderer-computed projectRuntime; resolve from worktreeId to honor the project's Windows runtime. + // `args.shellOverride` is the per-request pick (`terminal create --shell`), read here the way + // the renderer twin (ipc/spawn-preflight.ts) reads a tab's override. Without it a runtime create + // could only ever get the host default shell, so a caller asking for cmd/PowerShell got the + // default shell with the request typed into it. Still Windows-only: the override names a + // Windows shell, and spawn-options applies it under the same platform gate. ctx.terminalRuntimeOptions = process.platform === 'win32' && !args.connectionId ? resolveLocalWindowsTerminalRuntimeOptions({ - requestedShellOverride: undefined, + requestedShellOverride: args.shellOverride, settings: ctx.deps.getSettings?.(), projectRuntime: resolveLocalProjectRuntimeForWorktreeId(ctx.deps.store, args.worktreeId), fallbackHostShell: process.env.COMSPEC || 'powershell.exe' diff --git a/src/main/ipc/pty/runtime/spawn-state.ts b/src/main/ipc/pty/runtime/spawn-state.ts index 81878fa029b..2884dff5685 100644 --- a/src/main/ipc/pty/runtime/spawn-state.ts +++ b/src/main/ipc/pty/runtime/spawn-state.ts @@ -102,6 +102,7 @@ export type RuntimePtySpawnArgs = { tabId?: string leafId?: string sessionId?: string + shellOverride?: string isNewSession?: boolean persistHostSessionBinding?: boolean expectedSourceBinding?: PtyBindingSourceExpectation diff --git a/src/main/runtime/orca-runtime-create-terminal-desktop.ts b/src/main/runtime/orca-runtime-create-terminal-desktop.ts index 7e02a749447..9cd955428ee 100644 --- a/src/main/runtime/orca-runtime-create-terminal-desktop.ts +++ b/src/main/runtime/orca-runtime-create-terminal-desktop.ts @@ -18,6 +18,13 @@ export async function createDesktopTerminal( const launchOpts = workspace ? await runtime.resolveAgentTerminalCreateOptions(workspace, opts) : opts + // `resolveAgentTerminalCreateOptions` refuses an unapplicable shell, and it only runs with a + // workspace; a worktree-less create has no execution host to apply one to either. + if (!workspace && opts.shellOverride) { + throw new Error( + `--shell ${opts.shellOverride} needs a workspace, because the shell is resolved on the workspace's execution host. No terminal was created.` + ) + } const worktreeId = workspace?.id const cwd = workspace ? runtime.resolveWorkspaceTerminalStartupCwd(workspace, launchOpts.cwd) @@ -63,6 +70,7 @@ export async function createDesktopTerminal( ...(launchOpts.launchAgent ? { launchAgent: launchOpts.launchAgent } : {}), ...(launchOpts.viewMode ? { viewMode: launchOpts.viewMode } : {}), startupCommandDelivery: launchOpts.startupCommandDelivery, + ...(launchOpts.shellOverride ? { shellOverride: launchOpts.shellOverride } : {}), title: launchOpts.title, activate: presentation === 'focused', ...(presentation ? { presentation } : {}), diff --git a/src/main/runtime/orca-runtime-create-terminal.ts b/src/main/runtime/orca-runtime-create-terminal.ts index ec8a3e78941..5e7d4393a6d 100644 --- a/src/main/runtime/orca-runtime-create-terminal.ts +++ b/src/main/runtime/orca-runtime-create-terminal.ts @@ -146,6 +146,7 @@ export class OrcaRuntimeWithCreateTerminal extends OrcaRuntimeWithTerminalCreate preAllocatedHandle, tabId, leafId, + ...(launchOpts.shellOverride ? { shellOverride: launchOpts.shellOverride } : {}), ...(terminalColorQueryReplies ? { terminalColorQueryReplies } : {}), ...(launchOpts.agentSessionClaim ? { diff --git a/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts b/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts index 44f4524c782..795531bbc4a 100644 --- a/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts +++ b/src/main/runtime/orca-runtime-resolve-worktree-removal-target.ts @@ -16,6 +16,9 @@ import type { TerminalWorkspaceLaunchScope } from './runtime-legacy-worker-termi import type { TerminalCreateOptions } from './runtime-terminal-contracts' import { resolveLocalWindowsAgentStartupShell } from '../../shared/windows-terminal-shell' import { isTuiAgentEnabled } from '../../shared/tui-agent-selection' +import { terminalShellOverrideRefusal } from './terminal-shell-override-host-support' +import { resolveTerminalStartupCwd } from '../../shared/terminal-startup-cwd' +import { resolveLocalProjectRuntimeForWorktreeId } from '../local-project-runtime-resolution' import { resolveBareAgentLaunchCommand } from './runtime-agent-launch-resolution' import { buildAgentStartupPlan } from '../../shared/tui-agent-startup' import { @@ -152,6 +155,23 @@ export class OrcaRuntimeWithResolveWorktreeRemovalTarget extends OrcaRuntimeWith workspace: TerminalWorkspaceLaunchScope, opts: TerminalCreateOptions ): Promise { + // Before any early return: every create lane funnels through here, and a host that cannot + // apply the requested shell must refuse rather than spawn its default one. + const shellRefusal = terminalShellOverrideRefusal({ + shellOverride: opts.shellOverride, + connectionId: workspace.connectionId, + platform: process.platform, + projectRuntime: + opts.shellOverride && this.store + ? resolveLocalProjectRuntimeForWorktreeId(this.store, workspace.id) + : undefined, + // Same resolution as the spawn lanes below, so the refusal judges the cwd the PTY gets. + cwd: resolveTerminalStartupCwd(workspace.path, opts.cwd) ?? workspace.path, + workspacePath: workspace.path + }) + if (shellRefusal) { + throw shellRefusal + } // Why: raw shell commands like `codex exec` must remain user-authored shell. // Only unmanaged, repo-backed, bare agent launches get Settings defaults. const callerSuppliedLaunch = @@ -186,7 +206,8 @@ export class OrcaRuntimeWithResolveWorktreeRemovalTarget extends OrcaRuntimeWith const queuedShell = resolveLocalWindowsAgentStartupShell({ platform, isRemote, - terminalWindowsShell: settings.terminalWindowsShell + // A requested shell is the one this PTY will actually be, so it owns the quoting family. + terminalWindowsShell: opts.shellOverride ?? settings.terminalWindowsShell }) if (opts.startupAgent && !isTuiAgentEnabled(opts.startupAgent, settings.disabledTuiAgents)) { throw new Error(`Agent ${opts.startupAgent} is disabled. Choose an enabled agent.`) diff --git a/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness.spec.ts b/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness.spec.ts index bb6669da1a2..3a30c48e4b8 100644 --- a/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness.spec.ts +++ b/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { OrcaRuntimeService, makePaneKey } from '../orca-runtime-test-mocks.spec' +import { OrcaRuntimeService, electronMocks, makePaneKey } from '../orca-runtime-test-mocks.spec' import { HEADLESS_LEAF_ID, RESTORED_AUTHORITY_TOKEN, @@ -92,6 +92,130 @@ describe('OrcaRuntimeService', () => { }) }) + it('asks the pty controller for the requested shell instead of a startup command', async () => { + const hostPlatform = Object.getOwnPropertyDescriptor(process, 'platform')! + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-shell' }) + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + + await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + shellOverride: 'cmd.exe', + title: 'win shell' + }) + + // The defect this pins: a caller asking for cmd could only pass it as `command`, which the + // provider types into whatever shell it spawned — so the pty stayed the default shell and + // leaving cmd dropped the handle back onto a prompt the caller never asked for. + expect(spawn).toHaveBeenCalledWith( + expect.objectContaining({ shellOverride: 'cmd.exe', command: undefined }) + ) + } finally { + Object.defineProperty(process, 'platform', hostPlatform) + } + }) + + it('refuses a requested shell the execution host cannot apply instead of spawning its default', async () => { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-unreachable-shell' }) + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + + // Host platform here is POSIX, which has no Windows shell to pick. + await expect( + runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { shellOverride: 'cmd.exe' }) + ).rejects.toThrow(/--shell cmd\.exe names a Windows shell/) + expect(spawn).not.toHaveBeenCalled() + }) + + it('quotes the agent startup command for the requested shell, not the host default', async () => { + const hostPlatform = Object.getOwnPropertyDescriptor(process, 'platform')! + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-shell-quoting' }) + const runtime = new OrcaRuntimeService({ + ...store, + getSettings: () => ({ + ...store.getSettings(), + disabledTuiAgents: [], + terminalWindowsShell: 'powershell.exe', + agentCmdOverrides: {}, + agentDefaultArgs: { claude: '--dangerously-skip-permissions' }, + agentDefaultEnv: {} + }) + }) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + + await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { command: 'claude' }) + await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`, { + command: 'claude', + shellOverride: 'cmd.exe' + }) + + // The setting alone still quotes for PowerShell; the requested shell is the one that will + // read the command, so it owns the quoting family. PowerShell quoting typed into cmd is a + // syntax error at the prompt. + expect(spawn).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ command: "claude '--dangerously-skip-permissions'" }) + ) + expect(spawn).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + shellOverride: 'cmd.exe', + command: 'claude "--dangerously-skip-permissions"' + }) + ) + } finally { + Object.defineProperty(process, 'platform', hostPlatform) + } + }) + + it('refuses a requested shell with no workspace instead of creating a default-shell tab', async () => { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-no-workspace-shell' }) + const send = vi.fn() + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents: { send } + }) + + await expect( + runtime.createTerminal(undefined, { shellOverride: 'cmd.exe', rendererBacked: true }) + ).rejects.toThrow(/--shell cmd\.exe needs a workspace/) + expect(send).not.toHaveBeenCalled() + expect(spawn).not.toHaveBeenCalled() + }) + it('retires inherited launch authority when the agent command exits', async () => { const spawn = vi.fn().mockResolvedValue({ id: 'pty-authority', incarnationId: 'process-1' }) const retireAuthority = vi.fn() diff --git a/src/main/runtime/rpc/methods/terminal/terminal-lifecycle-methods.ts b/src/main/runtime/rpc/methods/terminal/terminal-lifecycle-methods.ts index 891ed65a13f..2fcdc2bc92c 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-lifecycle-methods.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-lifecycle-methods.ts @@ -53,6 +53,7 @@ export const TERMINAL_LIFECYCLE_METHODS = [ (canonicalWorktreeSelector, preAllocatedHandle) => runtime.createTerminal(canonicalWorktreeSelector, { command: params.command, + ...(params.shell ? { shellOverride: params.shell } : {}), startupCommandDelivery: params.startupCommandDelivery, env: params.env, envToDelete: params.envToDelete, diff --git a/src/main/runtime/runtime-pty-controller-contract.ts b/src/main/runtime/runtime-pty-controller-contract.ts index 73a75af017e..161b5440116 100644 --- a/src/main/runtime/runtime-pty-controller-contract.ts +++ b/src/main/runtime/runtime-pty-controller-contract.ts @@ -58,6 +58,8 @@ export type RuntimePtyController = { tabId?: string leafId?: string sessionId?: string + /** Windows shell to spawn AS this PTY, instead of the host default. */ + shellOverride?: string isNewSession?: boolean persistHostSessionBinding?: boolean expectedSourceBinding?: PtyBindingSourceExpectation diff --git a/src/main/runtime/runtime-terminal-contracts.ts b/src/main/runtime/runtime-terminal-contracts.ts index 227788af7e1..680bec98227 100644 --- a/src/main/runtime/runtime-terminal-contracts.ts +++ b/src/main/runtime/runtime-terminal-contracts.ts @@ -22,6 +22,15 @@ import type { WorkerTerminalHostScope } from './orchestration/worker-terminal-pr export type TerminalCreateOptions = { command?: string + /** + * Windows shell to spawn AS the PTY process, instead of the host default shell. + * + * Distinct from `command`, which is typed into whatever shell the host spawns: a caller asking + * for cmd or PowerShell through `command` gets it as a CHILD of the default shell, so the + * terminal's own process is still the default shell and leaving that child lands back on a + * prompt the caller never asked for. + */ + shellOverride?: string claudeAgentTeamsSourceCommand?: string cwd?: string env?: Record diff --git a/src/main/runtime/terminal-shell-override-host-support.test.ts b/src/main/runtime/terminal-shell-override-host-support.test.ts new file mode 100644 index 00000000000..bbef16c2adc --- /dev/null +++ b/src/main/runtime/terminal-shell-override-host-support.test.ts @@ -0,0 +1,204 @@ +import { describe, expect, it } from 'vitest' +import { terminalShellOverrideRefusal } from './terminal-shell-override-host-support' +import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' + +const NO_PROJECT_RUNTIME = undefined +const WINDOWS_WORKSPACE = 'C:\\Users\\u\\app' +const WSL_WORKSPACE = '\\\\wsl$\\Ubuntu\\home\\u\\app' +const ON_WINDOWS_HOST = { cwd: WINDOWS_WORKSPACE, workspacePath: WINDOWS_WORKSPACE } + +function resolvedRuntime(kind: 'windows-host' | 'wsl'): ProjectExecutionRuntimeResolution { + return kind === 'wsl' + ? { + status: 'resolved', + runtime: { + kind: 'wsl', + hostPlatform: 'wsl', + distro: 'Ubuntu', + projectId: 'p1', + reason: 'project-override', + cacheKey: 'p1:wsl:Ubuntu' + } + } + : { + status: 'resolved', + runtime: { + kind: 'windows-host', + hostPlatform: 'win32', + projectId: 'p1', + reason: 'project-override', + cacheKey: 'p1:windows-host' + } + } +} + +describe('terminalShellOverrideRefusal', () => { + it('allows a requested shell on a local Windows execution host', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + ...ON_WINDOWS_HOST + }) + ).toBeNull() + }) + + it('stays out of the way when no shell was requested', () => { + for (const platform of ['darwin', 'linux', 'win32'] as const) { + expect( + terminalShellOverrideRefusal({ + shellOverride: undefined, + connectionId: 'ssh-1', + platform, + projectRuntime: resolvedRuntime('wsl'), + ...ON_WINDOWS_HOST + }) + ).toBeNull() + } + }) + + // Both of these hosts would otherwise spawn their default shell and report success. + it('refuses when the spawn happens over SSH', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: 'ssh-1', + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + ...ON_WINDOWS_HOST + })?.message + ).toContain('over SSH') + }) + + it('refuses on a host that has no Windows shells to pick from', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'darwin', + projectRuntime: NO_PROJECT_RUNTIME, + ...ON_WINDOWS_HOST + })?.message + ).toContain('darwin') + }) + + // `resolveLocalWindowsTerminalRuntimeOptions` rewrites a shell that contradicts the project's + // execution runtime, which would hand back a terminal running something else entirely — and + // would quote an agent's startup command for the shell that was asked for, not the one running. + it('refuses a WSL shell when the project runs its terminals on the Windows host', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'wsl.exe', + connectionId: null, + platform: 'win32', + projectRuntime: resolvedRuntime('windows-host'), + ...ON_WINDOWS_HOST + })?.message + ).toContain('on the Windows host') + }) + + it('refuses a Windows shell when the project runs its terminals in WSL', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: resolvedRuntime('wsl'), + ...ON_WINDOWS_HOST + })?.message + ).toContain('in WSL') + }) + + it('allows a shell that agrees with the project runtime', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'wsl.exe', + connectionId: null, + platform: 'win32', + projectRuntime: resolvedRuntime('wsl'), + ...ON_WINDOWS_HOST + }) + ).toBeNull() + expect( + terminalShellOverrideRefusal({ + shellOverride: 'powershell.exe', + connectionId: null, + platform: 'win32', + projectRuntime: resolvedRuntime('windows-host'), + ...ON_WINDOWS_HOST + }) + ).toBeNull() + }) + + // `resolveWslSessionContext` forces wsl.exe for any `\\wsl$` cwd or workspace path, which is + // the one rewrite the project-runtime check cannot see: a folder workspace has no project. + describe('WSL UNC paths', () => { + it('refuses a Windows shell for a folder workspace inside a WSL distro with no project runtime', () => { + const message = terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: WSL_WORKSPACE, + workspacePath: WSL_WORKSPACE + })?.message + expect(message).toContain('inside WSL') + expect(message).toContain('No terminal was created') + expect(message).toContain('--shell wsl.exe') + }) + + it('refuses when only the workspace root is in WSL, since the session path forces wsl.exe too', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'powershell.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: WINDOWS_WORKSPACE, + workspacePath: WSL_WORKSPACE + })?.message + ).toContain(WSL_WORKSPACE) + }) + + it('accepts the forward-slash wsl.localhost spelling as a WSL path', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: '//wsl.localhost/Ubuntu/home/u/app/src', + workspacePath: '//wsl.localhost/Ubuntu/home/u/app' + }) + ).not.toBeNull() + }) + + it('allows wsl.exe for a WSL path', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'wsl.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: WSL_WORKSPACE, + workspacePath: WSL_WORKSPACE + }) + ).toBeNull() + }) + + it('still allows a Windows shell for a plain Windows path', () => { + expect( + terminalShellOverrideRefusal({ + shellOverride: 'cmd.exe', + connectionId: null, + platform: 'win32', + projectRuntime: NO_PROJECT_RUNTIME, + cwd: 'C:\\Users\\u\\app\\src', + workspacePath: WINDOWS_WORKSPACE + }) + ).toBeNull() + }) + }) +}) diff --git a/src/main/runtime/terminal-shell-override-host-support.ts b/src/main/runtime/terminal-shell-override-host-support.ts new file mode 100644 index 00000000000..d91b202a532 --- /dev/null +++ b/src/main/runtime/terminal-shell-override-host-support.ts @@ -0,0 +1,90 @@ +import { isWslShellName } from '../../shared/local-windows-terminal-runtime' +import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' +import { parseWslUncPath } from '../../shared/wsl-paths' + +/** + * Whether the host about to spawn a terminal can honour a requested Windows shell. + * + * The whole point of `--shell` is that the caller stops having to guess which shell it got. A host + * that cannot apply the pick must say so: silently spawning its default shell returns a healthy + * terminal running something else, which is the exact failure `--shell` exists to remove. + */ +export function terminalShellOverrideRefusal(args: { + shellOverride: string | undefined + connectionId: string | null + platform: NodeJS.Platform + projectRuntime: ProjectExecutionRuntimeResolution | undefined + /** The cwd the PTY will spawn in, resolved the same way the spawn lanes resolve it. */ + cwd: string + workspacePath: string +}): Error | null { + if (!args.shellOverride) { + return null + } + if (args.connectionId) { + // The shell runs on the SSH host, whose platform and installed shells this runtime cannot see. + return new Error( + `This workspace runs its terminals over SSH, and Orca cannot apply --shell ${args.shellOverride} there. No terminal was created. Omit --shell, or create the terminal on the execution host itself.` + ) + } + if (args.platform !== 'win32') { + return new Error( + `--shell ${args.shellOverride} names a Windows shell, and this execution host is ${args.platform}, which spawns the user's login shell. No terminal was created; omit --shell.` + ) + } + return ( + projectRuntimeShellConflict(args.shellOverride, args.projectRuntime) ?? + wslUncPathShellConflict(args.shellOverride, args.cwd, args.workspacePath) + ) +} + +/** + * The project's execution runtime decides which MACHINE the shell runs on, so it outranks a + * per-terminal pick — and `resolveLocalWindowsTerminalRuntimeOptions` enforces that by rewriting + * the value: a WSL project forces `wsl.exe`, and a Windows-host project discards a WSL name in + * favour of the host shell. Either rewrite hands back a terminal running something the caller did + * not ask for, and it also splits the startup-command quoting from the shell that receives it + * (POSIX args typed into cmd, or cmd args typed into a WSL shell). Refusing the contradiction is + * the only answer that keeps the request and the terminal describing the same thing. + */ +function projectRuntimeShellConflict( + shellOverride: string, + projectRuntime: ProjectExecutionRuntimeResolution | undefined +): Error | null { + if (projectRuntime?.status !== 'resolved') { + return null + } + const runsInWsl = projectRuntime.runtime.kind === 'wsl' + if (runsInWsl === isWslShellName(shellOverride)) { + return null + } + return new Error( + runsInWsl + ? `This workspace's project runs its terminals in WSL, so --shell ${shellOverride} cannot be applied. No terminal was created. Use --shell wsl.exe, or change the project's execution runtime.` + : `This workspace's project runs its terminals on the Windows host, so --shell ${shellOverride} cannot be applied. No terminal was created. Change the project's execution runtime to WSL, or pass a Windows shell.` + ) +} + +/** + * A `\\wsl$\\...` path only exists inside that distro, so the providers force `wsl.exe` + * for it whatever shell was requested (`resolveWslSessionContext` keys off the cwd, then the + * workspace path behind the session). That rewrite is right for the global shell setting and + * wrong for `--shell`, which promised the caller the shell it named. This is the only check that + * still fires for a folder workspace, which has no project runtime to disagree with. + */ +function wslUncPathShellConflict( + shellOverride: string, + cwd: string, + workspacePath: string +): Error | null { + if (isWslShellName(shellOverride)) { + return null + } + const wslPath = [cwd, workspacePath].find((path) => parseWslUncPath(path) !== null) + if (wslPath === undefined) { + return null + } + return new Error( + `This terminal would run inside WSL because ${wslPath} lives in a WSL distro, so --shell ${shellOverride} cannot be applied. No terminal was created. Use --shell wsl.exe, or omit --shell.` + ) +} diff --git a/src/main/ssh/ssh-remote-orca-cli.test.ts b/src/main/ssh/ssh-remote-orca-cli.test.ts index 0b3695aec5c..276d9cad87c 100644 --- a/src/main/ssh/ssh-remote-orca-cli.test.ts +++ b/src/main/ssh/ssh-remote-orca-cli.test.ts @@ -180,6 +180,36 @@ describe('runRemoteOrcaCli', () => { } ) + // Why: `orca terminal create --shell` gates on these; without them an SSH pane was told the + // host was too old, when the accurate refusal is that SSH cannot apply the shell. + it('reports the execution host capabilities through the legacy status fallback', async () => { + const runtime = new OrcaRuntimeService() + vi.spyOn(runtime, 'getStatus').mockReturnValue({ + runtimeId: 'runtime-test', + rendererGraphEpoch: 1, + graphStatus: 'ready', + authoritativeWindowId: 1, + liveTabCount: 0, + liveLeafCount: 0, + capabilities: ['terminal.create-shell-selection.v1'] + }) + + const result = await runRemoteOrcaCli( + runtime, + { argv: ['status', '--json'], cwd: '/home/alice/repo', env: {} }, + LEGACY_FALLBACK_OPTIONS + ) + + expect(result.exitCode, result.stdout).toBe(0) + expect(JSON.parse(result.stdout)).toMatchObject({ + ok: true, + result: { + target: { kind: 'environment', environment: 'ssh' }, + runtime: { reachable: true, capabilities: ['terminal.create-shell-selection.v1'] } + } + }) + }) + it('uses the remote ORCA_TERMINAL_HANDLE as orchestration sender identity', async () => { const { runtime, db } = createRuntime() diff --git a/src/main/ssh/ssh-remote-orca-cli.ts b/src/main/ssh/ssh-remote-orca-cli.ts index dba1bc1e2d5..6f476fa0d84 100644 --- a/src/main/ssh/ssh-remote-orca-cli.ts +++ b/src/main/ssh/ssh-remote-orca-cli.ts @@ -185,7 +185,10 @@ async function dispatchRemoteCli( state: status.graphStatus === 'ready' ? 'ready' : 'graph_not_ready', reachable: true, connectionState: runtimeHostConnectionState({ hasStatusEntry: true, status }), - runtimeId: status.runtimeId + runtimeId: status.runtimeId, + // Why: `status.get` ran in-process on the execution host, so these ARE that host's + // capabilities; dropping them made `--shell` report an outdated host instead of SSH. + ...(status.capabilities ? { capabilities: status.capabilities } : {}) }, graph: { state: status.graphStatus } } diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index b012b30469e..4a55d6b587b 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -5,7 +5,7 @@ import { existsSync } from 'node:fs' import { basename, join } from 'node:path' import { randomUUID } from 'node:crypto' import { resolveWindowsGitBashShellPath } from '../main/git-bash' -import { WINDOWS_GIT_BASH_SHELL } from '../shared/windows-terminal-shell' +import { isSupportedWindowsShellOverride } from '../shared/windows-terminal-shell' import type { RelayDispatcher, RequestContext } from './dispatcher' import { resolveDefaultShell, @@ -365,22 +365,6 @@ const ALLOWED_SIGNALS = new Set([ 'SIGUSR2' ]) -const ALLOWED_WINDOWS_SHELL_OVERRIDES = new Set([ - 'powershell.exe', - 'powershell', - 'pwsh.exe', - 'pwsh', - 'cmd.exe', - 'cmd', - 'wsl.exe', - 'wsl', - // Why: both spellings classify as a POSIX startup family, so rejecting them here made the relay - // the one host that hard-failed a setting the local and daemon PTYs accept. - 'bash.exe', - 'bash', - WINDOWS_GIT_BASH_SHELL -]) - function resolvePtyShellOverride(shellOverride: string): string { if (!shellOverride) { return '' @@ -388,8 +372,7 @@ function resolvePtyShellOverride(shellOverride: string): string { if (process.platform !== 'win32') { return '' } - const normalized = shellOverride.toLowerCase() - if (!ALLOWED_WINDOWS_SHELL_OVERRIDES.has(normalized)) { + if (!isSupportedWindowsShellOverride(shellOverride)) { throw new Error(`Unsupported Windows shell override: ${shellOverride}`) } return resolveWindowsGitBashShellPath(shellOverride) ?? shellOverride diff --git a/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts index a297c751a29..2452d0f167d 100644 --- a/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/terminal-request-ipc-bridge.ts @@ -78,7 +78,7 @@ export function registerTerminalRequestIpcBridge(unsubs: (() => void)[]): void { recordInteraction: false, ...(data.cwd ? { startupCwd: data.cwd } : {}) } - const tab = store.createTab(worktreeId, data.targetGroupId, undefined, tabOptions) + const tab = store.createTab(worktreeId, data.targetGroupId, data.shellOverride, tabOptions) if (!shouldActivate) { // Why: renderer-backed Codex startup must mount its new TerminalPane without switching UI or connecting every saved tab. requestBackgroundTerminalWorktreeMount({ worktreeId, tabIds: [tab.id] }) diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index bd2ca84c95f..efe101a4eb0 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -130,6 +130,11 @@ export const ACCOUNT_IMPORT_RUNTIME_CAPABILITY = 'accounts.import-host-credentia // Why: older hosts cannot reconcile terminal.create's mutation after losing the reply, so clients may only retry unknown outcomes when advertised. export const TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY = 'terminal.create-idempotency.v2' as const +// Why: an older host strips terminal.create's unknown `shell` and answers with a terminal running +// the host default shell. That reply is indistinguishable from success, so a client asking for a +// shell must refuse rather than create the wrong one. +export const TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY = + 'terminal.create-shell-selection.v1' as const export const SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY = 'session-tabs.close-intent.v1' as const export const SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY = 'session-tabs.authoritative-inventory.v1' as const @@ -296,6 +301,7 @@ export const RUNTIME_CAPABILITIES = [ WORKTREE_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY, WORKTREE_ARCHIVE_FAILURE_BLOCKING_RUNTIME_CAPABILITY, TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY, + TERMINAL_CREATE_SHELL_SELECTION_RUNTIME_CAPABILITY, SESSION_TAB_CLOSE_INTENT_RUNTIME_CAPABILITY, SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY, AGENT_SESSION_BOUNDARY_RUNTIME_CAPABILITY, diff --git a/src/shared/rpc-contract/terminal-create-shell-param.test.ts b/src/shared/rpc-contract/terminal-create-shell-param.test.ts new file mode 100644 index 00000000000..ca28a0580e1 --- /dev/null +++ b/src/shared/rpc-contract/terminal-create-shell-param.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest' +import { TerminalCreateParams } from './terminal-unary-params' +import { resolveWindowsShellStartupFamily } from '../windows-terminal-shell' + +describe('TerminalCreateParams.shell', () => { + it('stays optional so older callers keep creating terminals', () => { + const parsed = TerminalCreateParams.parse({ worktree: 'path:/repo' }) + + expect(parsed.shell).toBeUndefined() + }) + + it('carries an allowed Windows shell through to the runtime', () => { + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'cmd.exe' }).shell).toBe( + 'cmd.exe' + ) + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'git-bash' }).shell).toBe( + 'git-bash' + ) + }) + + // The host canonicalizes even when a client did not, so the spawn path and the startup-command + // quoting only ever see the `.exe` spelling they exact-match. + it('canonicalizes an accepted spelling before it reaches the runtime', () => { + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'cmd' }).shell).toBe( + 'cmd.exe' + ) + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'pwsh' }).shell).toBe( + 'pwsh.exe' + ) + expect(TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'Git-Bash' }).shell).toBe( + 'git-bash' + ) + }) + + it('quotes a bare cmd override as cmd rather than PowerShell', () => { + const { shell } = TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'cmd' }) + + expect(resolveWindowsShellStartupFamily(shell)).toBe('cmd') + }) + + // The relay refuses these at spawn time; refusing here turns an opaque spawn failure into an + // answer the caller gets before the terminal exists. + it('refuses a shell the host will not spawn', () => { + expect(() => TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'nu.exe' })).toThrow( + /shell must be one of/ + ) + expect(() => + TerminalCreateParams.parse({ worktree: 'path:/repo', shell: 'cmd.exe && calc' }) + ).toThrow(/shell must be one of/) + }) +}) diff --git a/src/shared/rpc-contract/terminal-unary-params.ts b/src/shared/rpc-contract/terminal-unary-params.ts index e86bdf1fdc1..9b735952096 100644 --- a/src/shared/rpc-contract/terminal-unary-params.ts +++ b/src/shared/rpc-contract/terminal-unary-params.ts @@ -1,6 +1,11 @@ import { z } from 'zod' import { OptionalFiniteNumber, OptionalString, requiredString } from './rpc-param-primitives' import { isTuiAgent } from '../tui-agent-config' +import { + canonicalizeWindowsShellOverride, + isSupportedWindowsShellOverride, + listSupportedWindowsShellOverrides +} from '../windows-terminal-shell' import { TERMINAL_PANE_SPLIT_SOURCES } from '../feature-education-telemetry' export const TerminalHandle = z.object({ @@ -180,7 +185,18 @@ export const TerminalCreateParams = z.object({ activate: z.unknown().optional(), presentation: z.enum(['background', 'focused']).optional(), tabId: OptionalString, - leafId: OptionalString + leafId: OptionalString, + // Why refused at the boundary rather than at spawn: only the host knows the allowlist, and a + // relay-side throw reaches the caller as an opaque spawn failure after the round trip. + shell: z + .string() + .refine(isSupportedWindowsShellOverride, { + message: `shell must be one of: ${listSupportedWindowsShellOverrides().join(', ')}` + }) + // Why here: the host is authoritative, so it canonicalizes even when a client did not; the + // spawn path exact-matches `.exe` spellings and must never see `cmd` or `Git-Bash`. + .transform((shell) => canonicalizeWindowsShellOverride(shell) ?? shell) + .optional() }) export const TerminalSplit = TerminalHandle.extend({ diff --git a/src/shared/runtime-terminal-contracts.ts b/src/shared/runtime-terminal-contracts.ts index ad392a2b9a0..57560caa7a5 100644 --- a/src/shared/runtime-terminal-contracts.ts +++ b/src/shared/runtime-terminal-contracts.ts @@ -263,6 +263,8 @@ type RuntimeTerminalCreateBaseRequestPayload = { activate?: boolean presentation?: RuntimeTerminalPresentation surfaceOwner?: false + /** Windows shell the created tab spawns AS, instead of the host default. */ + shellOverride?: string } export type RuntimeTerminalCreateRequestPayload = diff --git a/src/shared/source-scan/source-tree-walk.test.ts b/src/shared/source-scan/source-tree-walk.test.ts index afc7c8dba88..4030fa2f37c 100644 --- a/src/shared/source-scan/source-tree-walk.test.ts +++ b/src/shared/source-scan/source-tree-walk.test.ts @@ -108,7 +108,6 @@ describe('scanSourceTree filesystem traversal', () => { expect(statSync).toHaveBeenCalledExactlyOnceWith(join(root, 'alias')) }) - it('still reports a broken link instead of silently dropping it', () => { const target = join(root, '.target') mkdirSync(target) diff --git a/src/shared/windows-terminal-shell.test.ts b/src/shared/windows-terminal-shell.test.ts index e7d86a07cc9..e9c9653bd99 100644 --- a/src/shared/windows-terminal-shell.test.ts +++ b/src/shared/windows-terminal-shell.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest' -import { resolveWindowsShellStartupFamily } from './windows-terminal-shell' +import { + canonicalizeWindowsShellOverride, + isSupportedWindowsShellOverride, + listSupportedWindowsShellOverrides, + resolveWindowsShellStartupFamily +} from './windows-terminal-shell' describe('resolveWindowsShellStartupFamily', () => { it('defaults to PowerShell when unset', () => { @@ -33,3 +38,83 @@ describe('resolveWindowsShellStartupFamily', () => { expect(resolveWindowsShellStartupFamily('C:\\Program Files\\Git\\bin\\bash')).toBe('posix') }) }) + +describe('isSupportedWindowsShellOverride', () => { + // Spelled out rather than looped over the list, which would assert the list against itself. + it('accepts exactly the shells the relay is willing to spawn', () => { + expect(listSupportedWindowsShellOverrides()).toEqual([ + 'bash', + 'bash.exe', + 'cmd', + 'cmd.exe', + 'git-bash', + 'powershell', + 'powershell.exe', + 'pwsh', + 'pwsh.exe', + 'wsl', + 'wsl.exe' + ]) + expect(isSupportedWindowsShellOverride('cmd.exe')).toBe(true) + expect(isSupportedWindowsShellOverride('powershell.exe')).toBe(true) + expect(isSupportedWindowsShellOverride('git-bash')).toBe(true) + }) + + it('accepts a differently cased spelling of an allowed shell', () => { + expect(isSupportedWindowsShellOverride('CMD.EXE')).toBe(true) + expect(isSupportedWindowsShellOverride('PowerShell.exe')).toBe(true) + }) + + // The allowlist is what stops `--shell` from naming an arbitrary executable to spawn. + it('refuses anything else, including a path to an allowed shell', () => { + expect(isSupportedWindowsShellOverride('nu.exe')).toBe(false) + expect(isSupportedWindowsShellOverride('')).toBe(false) + expect(isSupportedWindowsShellOverride('C:\\Windows\\System32\\cmd.exe')).toBe(false) + expect(isSupportedWindowsShellOverride('cmd.exe /c calc')).toBe(false) + }) +}) + +describe('canonicalizeWindowsShellOverride', () => { + it('maps every accepted spelling to the `.exe` name the spawn path exact-matches', () => { + expect(canonicalizeWindowsShellOverride('cmd')).toBe('cmd.exe') + expect(canonicalizeWindowsShellOverride('cmd.exe')).toBe('cmd.exe') + expect(canonicalizeWindowsShellOverride('powershell')).toBe('powershell.exe') + expect(canonicalizeWindowsShellOverride('powershell.exe')).toBe('powershell.exe') + expect(canonicalizeWindowsShellOverride('pwsh')).toBe('pwsh.exe') + expect(canonicalizeWindowsShellOverride('pwsh.exe')).toBe('pwsh.exe') + expect(canonicalizeWindowsShellOverride('wsl')).toBe('wsl.exe') + expect(canonicalizeWindowsShellOverride('wsl.exe')).toBe('wsl.exe') + expect(canonicalizeWindowsShellOverride('bash')).toBe('bash.exe') + expect(canonicalizeWindowsShellOverride('bash.exe')).toBe('bash.exe') + expect(canonicalizeWindowsShellOverride('git-bash')).toBe('git-bash') + }) + + // pwsh (PowerShell 7) and powershell (Windows PowerShell 5.1) are different binaries. + it('never collapses pwsh into powershell', () => { + expect(canonicalizeWindowsShellOverride('pwsh')).not.toBe('powershell.exe') + expect(canonicalizeWindowsShellOverride('pwsh.exe')).not.toBe('powershell.exe') + }) + + // `resolveWindowsGitBashShellPath` compares the marker case-sensitively. + it('folds case so a mixed-case spelling reaches the exact-match consumers', () => { + expect(canonicalizeWindowsShellOverride('Git-Bash')).toBe('git-bash') + expect(canonicalizeWindowsShellOverride('CMD')).toBe('cmd.exe') + expect(canonicalizeWindowsShellOverride('PowerShell.exe')).toBe('powershell.exe') + }) + + it('returns undefined for anything the allowlist refuses', () => { + expect(canonicalizeWindowsShellOverride('nu.exe')).toBeUndefined() + expect(canonicalizeWindowsShellOverride('C:\\Windows\\System32\\cmd.exe')).toBeUndefined() + expect(canonicalizeWindowsShellOverride('')).toBeUndefined() + }) + + // Bare `cmd` falls through resolveWindowsShellStartupFamily to PowerShell quoting; the canonical + // name is what keeps the PTY shell and the queued-command quoting in the same family. + it('yields the cmd startup family for a bare cmd override', () => { + expect(resolveWindowsShellStartupFamily('cmd')).toBe('powershell') + expect(resolveWindowsShellStartupFamily(canonicalizeWindowsShellOverride('cmd'))).toBe('cmd') + expect(resolveWindowsShellStartupFamily(canonicalizeWindowsShellOverride('Git-Bash'))).toBe( + 'posix' + ) + }) +}) diff --git a/src/shared/windows-terminal-shell.ts b/src/shared/windows-terminal-shell.ts index 1f55bf89d35..625eefbb414 100644 --- a/src/shared/windows-terminal-shell.ts +++ b/src/shared/windows-terminal-shell.ts @@ -54,3 +54,47 @@ export function resolveLocalWindowsAgentStartupShell(args: { } return resolveWindowsShellStartupFamily(args.terminalWindowsShell) } + +/** + * Shell names a caller may request for a single Windows terminal, keyed by accepted spelling and + * mapped to the one canonical spelling every downstream consumer keys on. + * + * The relay owns the spawn and has always refused anything outside this set, but the set lived + * only there — so a bad value from `terminal create --shell` surfaced as a spawn-time throw with + * no way for the CLI to answer before the round trip. Shared so the RPC boundary and the relay + * agree on the same names. + * + * Why canonicalize: `resolveWindowsShellStartupFamily`, the launch-arg builders, and the Git Bash + * path resolver all exact-match the `.exe` spelling, so a bare `cmd` accepted here would spawn cmd + * yet quote its startup command for PowerShell. `pwsh` and `powershell` are different binaries and + * are never collapsed into each other. + */ +const WINDOWS_SHELL_OVERRIDE_CANONICAL_NAMES: ReadonlyMap = new Map([ + ['powershell.exe', 'powershell.exe'], + ['powershell', 'powershell.exe'], + ['pwsh.exe', 'pwsh.exe'], + ['pwsh', 'pwsh.exe'], + ['cmd.exe', 'cmd.exe'], + ['cmd', 'cmd.exe'], + ['wsl.exe', 'wsl.exe'], + ['wsl', 'wsl.exe'], + // Why: both spellings classify as a POSIX startup family, so rejecting them here made the relay + // the one host that hard-failed a setting the local and daemon PTYs accept. + ['bash.exe', 'bash.exe'], + ['bash', 'bash.exe'], + [WINDOWS_GIT_BASH_SHELL, WINDOWS_GIT_BASH_SHELL] +]) + +/** Canonical spelling for an accepted override (case-insensitive), or undefined when refused. */ +export function canonicalizeWindowsShellOverride(shell: string): string | undefined { + return WINDOWS_SHELL_OVERRIDE_CANONICAL_NAMES.get(shell.toLowerCase()) +} + +export function isSupportedWindowsShellOverride(shell: string): boolean { + return canonicalizeWindowsShellOverride(shell) !== undefined +} + +/** Sorted for a stable error message; callers list these when refusing a value. */ +export function listSupportedWindowsShellOverrides(): string[] { + return [...WINDOWS_SHELL_OVERRIDE_CANONICAL_NAMES.keys()].sort() +} From 6da72383dfc8bac6ce99d5ac5493f0755e08141e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:35:32 -0700 Subject: [PATCH 2/2] feat(agent-launch): one executor for agent launches, exposed as agent.launch (#19849) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(agent-launch): make the launch-mode decision surface-neutral `decideWorkerStartMode` was the only shared answer to "structured chat session or terminal agent?", but it lived in an orchestration-named module and spoke orchestration's vocabulary, so the other launch surfaces could not call it. Move the decision to `main/agent-launch/agent-launch-mode` unchanged and leave `orchestration-worker-start-mode` as the adapter that supplies the noun. A worker is not a special kind of launch; it is the same launch with a dispatch attached. Naming the receipt's subject is the only thing orchestration actually contributed, so that is the only thing the adapter keeps: "worker" in both sentences, plus the `--terminal` wording, which reads as nonsense anywhere a `--terminal` flag does not exist. Both are pinned, because they are asserted. No behavior change. The receipts are byte-identical for every reachable case, proven by running the new pin against both implementations. Also pins the wording, which nothing was holding. The existing suites assert `toContain` fragments ('terminal agent', 'cannot create') and the CLI suite asserts a receipt handed to it by a mock rather than one this code produced; all six files stayed green against a deliberately corrupted vocabulary. A dispatch receipt is the only place a structured-to-terminal downgrade explains itself, so the whole sentence is the contract, not a fragment of it. * feat(agent-launch): add the launch intent and the one executor that runs it The sequencing around the launch decision was duplicated per surface, and the duplicate is where the bug lives. A new worktree was created agent-first, so its startup terminal WAS the agent and the structured branch below it could never be reached — every new-worktree launch was a PTY regardless of the user's default. Orchestration fixed that for itself in #19431; mobile and the CLI still have it. `executeAgentLaunch` inverts the order once, for everyone. When the preference is structured the worktree is created with NO startup agent, the executing host is then asked whether it can host a session for the workspace that now exists, and only then is a surface created. The host verdict cannot be hoisted above creation: `agentSession.createSupport` only answers for a workspace it can resolve, which is why the decision stays in two halves. Agent-first creation is deliberately preserved for PTY launches — it is what sequences the agent's startup command behind the setup runner, so wait-for-setup comes for free there. What actually differs per surface is only how a surface is built (an orchestration worker's session takes a dispatch hold and a mailbox a plain launch must not take), so that is injected as a factory rather than branched on. The intent also strips the reserved agent fields from a migrated create payload: a caller moving off `worktree.create` passes its existing params, and a stale `startupAgent` in there would re-create the very path this replaces. Tests assert order and arguments, not just the resulting mode. Reintroducing agent-first creation reddens 4 of 11. * feat(agent-launch): expose the launch executor as the agent.launch RPC Adds `agent.launch` — one host-side method that decides structured-vs-terminal and creates the surface — wired to the real runtime factories: `createManagedWorktree` for the workspace, forking on `startupAgent` exactly as the orchestration worker path does; `createStructuredAgentSessionForWorktree` for a chat session; and `createTerminal` for a PTY agent. Allowlisted for mobile, which is the surface the routing gap was reported on. `worktree.create` is untouched. Its `startupAgent` keeps meaning "spawn a PTY agent" verbatim, because it answers with `agentTerminalHandle` only on that path: a host that quietly routed it to a structured session would hand every older client a response with no handle and no error. All new behaviour sits behind `agent.launch.v1`, which the host now advertises and a remote client must negotiate, so a client that does not gets today's behaviour unchanged. * fix(agent-launch): drop the deleted draft-prompt blocker from the reason map main removed the draft-prompt blocker in #19681 (a structured session now holds an unsent draft), so the exhaustive Record no longer typechecks. * chore(agent-launch): carry a SAFETY rationale on the agent placement cast The type-assertion gate landed after this branch's base, so the new file's copy of the worker-start cast is now a changed-code finding. * chore(agent-launch): carry agent.launch through main's RPC typing and casting gates The typed-method contract, the generated params catalog and the `assertionStyle: never` casting scan all landed after this branch's base. - AGENT_LAUNCH_METHODS kept an `RpcMethod[]` annotation, which widened its method name to `string` and broke assignability; every sibling infers instead. - `agent.launch` binds a schema under src/main, so it joins the catalog's RPC_METHODS_WITHOUT_SHARED_PARAMS and the parity gate's hand-listed twin. - The now-typed methods make most test casts unnecessary; the few that remain carry the line-specific SAFETY rationale the casting gate requires. * docs(agent-launch): stop the receipt-wording comment claiming a migration The decision was never moved out of orchestration-worker-start-mode; this PR adds a second copy beside it. Say so, and name the unenforced agreement. * docs(agent-launch): stop the executor comment claiming a migration that has not happened The header asserted two things the tree does not support: that every launch surface routes through the executor, and that the mode decision "already lived" in `agent-launch-mode`. `agent.launch` is the executor's only consumer, and `orchestration-worker-start-mode.ts` is byte-identical (blob 92dc5c644a89, 217 lines) at the merge base and all three stack heads, still used by workers.ts. Describe the two live copies and leave the cutover to later stack work. * fix(agent-launch): preserve setup and refusal fallbacks * fix(agent-launch): dedupe complete launch and cancel setup wait --------- Co-authored-by: Merge Sim --- .../agent-launch-executor.test.ts | 248 +++++++++ .../agent-launch/agent-launch-executor.ts | 253 +++++++++ ...ntime-start-tui-idle-visible-read-probe.ts | 14 +- ...nal-creation-and-readiness-part-06.spec.ts | 31 ++ .../rpc/methods/agent-launch-schemas.ts | 51 ++ .../rpc/methods/agent-launch-surfaces.ts | 83 +++ .../methods/agent-launch-worktree-creation.ts | 118 +++++ .../runtime/rpc/methods/agent-launch.test.ts | 482 ++++++++++++++++++ src/main/runtime/rpc/methods/agent-launch.ts | 116 +++++ src/main/runtime/rpc/methods/index.ts | 2 + .../runtime/rpc/rpc-params-type-parity.ts | 6 +- .../runtime-rpc-mobile-method-allowlist.ts | 1 + src/shared/agent-launch-intent.ts | 133 +++++ src/shared/protocol-version.ts | 22 +- .../rpc-params-catalog.generated.ts | 1 + 15 files changed, 1557 insertions(+), 4 deletions(-) create mode 100644 src/main/agent-launch/agent-launch-executor.test.ts create mode 100644 src/main/agent-launch/agent-launch-executor.ts create mode 100644 src/main/runtime/rpc/methods/agent-launch-schemas.ts create mode 100644 src/main/runtime/rpc/methods/agent-launch-surfaces.ts create mode 100644 src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts create mode 100644 src/main/runtime/rpc/methods/agent-launch.test.ts create mode 100644 src/main/runtime/rpc/methods/agent-launch.ts create mode 100644 src/shared/agent-launch-intent.ts diff --git a/src/main/agent-launch/agent-launch-executor.test.ts b/src/main/agent-launch/agent-launch-executor.test.ts new file mode 100644 index 00000000000..092262470a8 --- /dev/null +++ b/src/main/agent-launch/agent-launch-executor.test.ts @@ -0,0 +1,248 @@ +/** + * The executor's ordering contract, which is the defect this module exists to remove. + * + * The old shape created a new worktree agent-first, so its startup terminal WAS the agent and the + * structured branch below it could not be reached for any new worktree. The assertions that matter + * here are therefore about *order and arguments*, not just the returned mode: a structured launch + * must create the worktree with `startupAgent: undefined`, and it must ask the host only after the + * workspace exists. + */ + +import { describe, expect, it, vi } from 'vitest' +import { + AgentLaunchStructuredSessionRefusedError, + executeAgentLaunch, + type AgentLaunchExecution +} from './agent-launch-executor' +import type { AgentLaunchIntent } from '../../shared/agent-launch-intent' + +const STRUCTURED_PREFERENCE = { + experimentalNativeChat: true, + experimentalStructuredNativeChat: true, + openAgentTabsInChatByDefault: true +} + +function harness(options: { + settings?: Record | null + createSupport?: { supported: boolean; reason?: 'agent' | 'remote' | 'wsl' } + createSupportThrows?: boolean + structuredCreateError?: Error +}) { + const calls: string[] = [] + const createWorktree = vi.fn( + async (args: { create: Record; startupAgent: string | undefined }) => { + calls.push(`createWorktree(startupAgent=${String(args.startupAgent)})`) + return { + worktreeId: 'wt-new', + startupTerminalHandle: args.startupAgent ? 'term_agent_first' : undefined + } + } + ) + const getStructuredAgentSessionCreateSupport = vi.fn(async () => { + calls.push('createSupport') + if (options.createSupportThrows) { + throw new Error('host unreachable') + } + return options.createSupport ?? { supported: true } + }) + const createStructuredSession = vi.fn(async () => { + calls.push('createStructuredSession') + if (options.structuredCreateError) { + throw options.structuredCreateError + } + return { sessionId: 'sess-1', handle: 'handle_structured' } + }) + const createTerminalAgent = vi.fn(async () => { + calls.push('createTerminalAgent') + return { handle: 'term_1' } + }) + const runtime = { + getClientSettings: () => + options.settings === undefined ? STRUCTURED_PREFERENCE : options.settings, + getStructuredAgentSessionCreateSupport + } + return { + calls, + createWorktree, + createStructuredSession, + createTerminalAgent, + run: (intent: AgentLaunchIntent) => + executeAgentLaunch({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the stub implements only the two runtime methods the executor reaches, and each test asserts the calls made, so an omitted method throws rather than reading a wrong value. + runtime: runtime as unknown as AgentLaunchExecution['runtime'], + intent, + surfaces: { createStructuredSession, createTerminalAgent }, + workspaces: { createWorktree } + }) + } +} + +const CREATE_INTENT: AgentLaunchIntent = { + agent: 'claude', + target: { kind: 'create-worktree', create: { repo: 'id:repo-1', name: 'task' } } +} + +describe('a structured launch that creates its own worktree', () => { + it('creates the worktree with no startup agent, then asks the host, then opens a session', async () => { + const h = harness({}) + const result = await h.run(CREATE_INTENT) + + // The whole defect in one assertion: the worktree must not be created agent-first. + expect(h.calls).toEqual([ + 'createWorktree(startupAgent=undefined)', + 'createSupport', + 'createStructuredSession' + ]) + expect(result.outcome).toEqual({ + kind: 'structured', + sessionId: 'sess-1', + handle: 'handle_structured' + }) + expect(result.worktreeId).toBe('wt-new') + expect(result.receipt.mode).toBe('structured') + }) + + it('asks the host only after the workspace exists, never before', async () => { + const h = harness({}) + await h.run(CREATE_INTENT) + expect(h.calls.indexOf('createSupport')).toBeGreaterThan( + h.calls.indexOf('createWorktree(startupAgent=undefined)') + ) + }) + + it('falls back to a terminal in the worktree it just created when the host refuses', async () => { + const h = harness({ createSupport: { supported: false, reason: 'wsl' } }) + const result = await h.run(CREATE_INTENT) + + expect(h.calls).toEqual([ + 'createWorktree(startupAgent=undefined)', + 'createSupport', + 'createTerminalAgent' + ]) + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_1' }) + // Not a failed launch, and the workspace is the one just created. + expect(result.worktreeId).toBe('wt-new') + expect(result.receipt).toMatchObject({ mode: 'terminal', reason: 'wsl_execution_runtime' }) + }) + + it('falls back to a terminal when the host cannot be reached at all', async () => { + const h = harness({ createSupportThrows: true }) + const result = await h.run(CREATE_INTENT) + expect(result.outcome.kind).toBe('terminal') + expect(result.receipt).toMatchObject({ reason: 'structured_support_unknown' }) + }) + + it('falls back only for a definitive structured refusal after the worktree exists', async () => { + const h = harness({ + structuredCreateError: new AgentLaunchStructuredSessionRefusedError( + 'structured_agent_session_unsupported', + 'unsupported' + ) + }) + const result = await h.run(CREATE_INTENT) + + expect(h.calls).toEqual([ + 'createWorktree(startupAgent=undefined)', + 'createSupport', + 'createStructuredSession', + 'createTerminalAgent' + ]) + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_1' }) + expect(result.receipt).toMatchObject({ + mode: 'terminal', + reason: 'structured_unsupported_on_host' + }) + }) + + it('does not create a duplicate terminal when structured creation is unknown', async () => { + const h = harness({ + structuredCreateError: new AgentLaunchStructuredSessionRefusedError( + 'agent_session_operation_unknown', + 'unknown' + ) + }) + + await expect(h.run(CREATE_INTENT)).rejects.toThrow('unknown') + expect(h.calls).toEqual([ + 'createWorktree(startupAgent=undefined)', + 'createSupport', + 'createStructuredSession' + ]) + }) + + it('strips a stale startupAgent out of a migrated create payload', async () => { + const h = harness({}) + await h.run({ + agent: 'claude', + target: { + kind: 'create-worktree', + // Exactly what mobile sends `worktree.create` today. + create: { repo: 'id:repo-1', name: 'task', startupAgent: 'claude', startupDraft: 'url' } + } + }) + const passed = h.createWorktree.mock.calls[0]?.[0] + expect(passed?.create).not.toHaveProperty('startupAgent') + expect(passed?.create).not.toHaveProperty('startupDraft') + expect(passed?.create).toMatchObject({ repo: 'id:repo-1', name: 'task' }) + }) +}) + +describe('a launch the user did not ask to be structured', () => { + it('creates the worktree agent-first and never asks the host', async () => { + const h = harness({ settings: null }) + const result = await h.run(CREATE_INTENT) + + // Agent-first is preserved for PTY launches: it is what sequences the agent's startup command + // behind the setup runner, so the wait-for-setup gate comes for free. + expect(h.calls).toEqual(['createWorktree(startupAgent=claude)']) + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_agent_first' }) + expect(result.receipt).toMatchObject({ mode: 'terminal', reason: 'user_default' }) + }) +}) + +describe('a launch into a workspace that already exists', () => { + it('opens a session without creating anything', async () => { + const h = harness({}) + const result = await h.run({ agent: 'codex', target: { kind: 'existing', worktree: 'wt-7' } }) + expect(h.calls).toEqual(['createSupport', 'createStructuredSession']) + expect(h.createWorktree).not.toHaveBeenCalled() + expect(result.worktreeId).toBe('wt-7') + }) + + it('reuses a running terminal without creating or asking', async () => { + const h = harness({}) + const result = await h.run({ + agent: 'claude', + target: { kind: 'existing', worktree: 'wt-7' }, + reuseTerminal: { handle: 'term_live' } + }) + expect(h.calls).toEqual([]) + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_live' }) + expect(result.receipt).toMatchObject({ mode: 'terminal', reason: 'reused_terminal' }) + }) +}) + +describe('an agent with no structured session', () => { + it('stays a terminal without asking the host', async () => { + const h = harness({}) + const result = await h.run({ agent: 'grok', target: { kind: 'existing', worktree: 'wt-7' } }) + expect(h.calls).toEqual(['createTerminalAgent']) + expect(result.receipt).toMatchObject({ reason: 'agent_without_structured_session' }) + }) +}) + +describe('the prompt receipt', () => { + it('reports a requested prompt as undelivered rather than omitting it', async () => { + const h = harness({}) + const result = await h.run({ + ...CREATE_INTENT, + prompt: { text: 'do the thing', delivery: 'draft' } + }) + expect(result.prompt).toEqual({ delivery: 'draft', delivered: false }) + }) + + it('omits the receipt when no prompt was requested', async () => { + const h = harness({}) + expect((await h.run(CREATE_INTENT)).prompt).toBeUndefined() + }) +}) diff --git a/src/main/agent-launch/agent-launch-executor.ts b/src/main/agent-launch/agent-launch-executor.ts new file mode 100644 index 00000000000..3f02aaafb36 --- /dev/null +++ b/src/main/agent-launch/agent-launch-executor.ts @@ -0,0 +1,253 @@ +/** + * The one place an agent is actually started — for the surfaces moved onto it, which today is + * `agent.launch` alone. Orchestration dispatch, mobile create, CLI create and the desktop agent + * tab each still start agents their own way; moving them here is later stack work. + * + * The mode decision is duplicated rather than shared: `agent-launch-mode` is a surface-neutral + * second copy of orchestration's `orchestration-worker-start-mode`, which is unchanged and still + * the one orchestration uses, with nothing enforcing agreement between them. That cutover is later + * stack work too. What this module adds is the *sequencing*, and the sequencing is where the bug + * was: + * + * create the worktree agent-first -> its startup terminal IS the agent + * -> the structured branch below it is unreachable + * + * so every new-worktree launch was a PTY no matter what the user's default said. The order here is + * the inverse, and it is the whole point of the module: when the preference is structured the + * worktree is created with NO startup agent, the executing host is then asked whether it can host + * a session for the workspace that now exists, and only then is a surface created. A refusal + * becomes a terminal agent in the worktree just created, never a failed launch. + * + * The host verdict cannot be hoisted above creation: `agentSession.createSupport` can only answer + * for a workspace it can resolve. That is why the decision is in two halves rather than one. + * + * What genuinely differs per surface is only how a surface is *built* — an orchestration worker's + * session takes a dispatch hold and a mailbox that a plain launch must not take — so that is + * injected as a factory instead of branched on here. + */ + +import type { + AgentLaunchIntent, + AgentLaunchResult, + AgentLaunchTarget +} from '../../shared/agent-launch-intent' +import { withoutReservedAgentCreateFields } from '../../shared/agent-launch-intent' +import type { TuiAgent } from '../../shared/tui-agent' +import type { OrcaRuntimeService } from '../runtime/orca-runtime' +import { isDefinitiveAgentSessionCreateRefusal } from '../../shared/agent-session-definitive-refusal' +import { + decideAgentLaunchMode, + readAgentLaunchModeSettings, + resolveAgentLaunchModeOnHost, + type AgentLaunchModeReceipt, + type AgentLaunchModeVocabulary, + DEFAULT_LAUNCH_VOCABULARY +} from './agent-launch-mode' + +/** How a surface is built once the executor has decided which one. Injected because an + * orchestration worker's session carries a dispatch hold and a mailbox a plain launch must not + * take, while the decision and ordering above it are identical. */ +export type AgentLaunchSurfaceFactory = { + createStructuredSession(args: { + worktreeId: string + agent: 'claude' | 'codex' + options?: Readonly> + }): Promise<{ sessionId: string; handle: string }> + createTerminalAgent(args: { + worktreeId: string + agent: TuiAgent + options?: Readonly> + }): Promise<{ handle: string; warning?: string }> +} + +/** A structured create refusal that proves no session was committed, so the launch may downgrade. */ +export class AgentLaunchStructuredSessionRefusedError extends Error { + readonly code: string + + constructor(code: string, message: string) { + super(message) + this.name = 'AgentLaunchStructuredSessionRefusedError' + this.code = code + } +} + +/** Creating the workspace, when the intent asks for one. Injected so orchestration keeps recording + * its own worktree stages and residual-resource effects around the same call. */ +export type AgentLaunchWorkspaceFactory = { + createWorktree(args: { + create: Readonly> + /** Set only when the settled mode is a terminal agent: agent-first creation sequences the + * agent's startup command behind the setup runner, which is how a PTY launch gets its + * wait-for-setup gate for free. A structured launch has no startup command to sequence and + * must await that gate explicitly instead. */ + startupAgent: TuiAgent | undefined + }): Promise<{ worktreeId: string; startupTerminalHandle: string | undefined }> +} + +export type AgentLaunchExecution = { + runtime: Pick + intent: AgentLaunchIntent + surfaces: AgentLaunchSurfaceFactory + workspaces?: AgentLaunchWorkspaceFactory + vocabulary?: AgentLaunchModeVocabulary + /** Attributes a throw to the step that was running, the way a dispatch's own stages do. */ + onStage?: (stage: 'worktree_create' | 'mode_settle' | 'surface_create') => void +} + +export async function executeAgentLaunch( + execution: AgentLaunchExecution +): Promise { + const { intent, runtime } = execution + const vocabulary = execution.vocabulary ?? DEFAULT_LAUNCH_VOCABULARY + const settings = readAgentLaunchModeSettings(runtime) + const preflight = decideAgentLaunchMode({ + placement: { + agent: intent.agent, + ...(intent.reuseTerminal ? { terminal: intent.reuseTerminal.handle } : {}) + }, + settings, + vocabulary + }) + + // A reused terminal already downgraded in the pre-flight; there is nothing to create. + if (intent.reuseTerminal) { + return { + outcome: { kind: 'terminal', handle: intent.reuseTerminal.handle }, + worktreeId: existingWorktreeId(intent.target), + receipt: preflight, + ...promptReceipt(intent) + } + } + + const placed = await resolveWorkspace(execution, preflight) + // Agent-first creation already produced the agent, so the pre-flight verdict is final. + if (placed.startupTerminalHandle) { + return { + outcome: { kind: 'terminal', handle: placed.startupTerminalHandle }, + worktreeId: placed.worktreeId, + receipt: preflight, + ...promptReceipt(intent) + } + } + + execution.onStage?.('mode_settle') + let settled = await resolveAgentLaunchModeOnHost( + runtime, + preflight, + placed.worktreeId, + intent.agent, + vocabulary + ) + + execution.onStage?.('surface_create') + let outcome: AgentLaunchResult['outcome'] + try { + outcome = await createSurface(execution, placed.worktreeId, settled) + } catch (error) { + // The structured create path distinguishes a definitive pre-commit refusal from an unknown + // outcome. Only the former is safe to replace with a terminal in the same workspace; retrying + // after an unknown attach outcome could create two agents. + if ( + settled.mode !== 'structured' || + !(error instanceof AgentLaunchStructuredSessionRefusedError) || + !isDefinitiveAgentSessionCreateRefusal(error.code) + ) { + throw error + } + settled = downgradeAgentLaunchModeForStructuredRefusal(settled, vocabulary) + outcome = await execution.surfaces + .createTerminalAgent({ + worktreeId: placed.worktreeId, + agent: intent.agent, + ...(intent.sessionOptions ? { options: intent.sessionOptions } : {}) + }) + .then((terminal) => ({ + kind: 'terminal' as const, + handle: terminal.handle, + ...(terminal.warning ? { warning: terminal.warning } : {}) + })) + } + return { + outcome, + worktreeId: placed.worktreeId, + receipt: settled, + ...promptReceipt(intent) + } +} + +function downgradeAgentLaunchModeForStructuredRefusal( + receipt: AgentLaunchModeReceipt, + vocabulary: AgentLaunchModeVocabulary +): AgentLaunchModeReceipt { + return { + mode: 'terminal', + preferred: receipt.preferred, + reason: 'structured_unsupported_on_host', + detail: `Your default is a structured chat session, but the host refused to create one here; started ${vocabulary.terminal} instead.` + } +} + +async function resolveWorkspace( + execution: AgentLaunchExecution, + preflight: AgentLaunchModeReceipt +): Promise<{ worktreeId: string; startupTerminalHandle: string | undefined }> { + const { intent } = execution + if (intent.target.kind === 'existing') { + return { worktreeId: intent.target.worktree, startupTerminalHandle: undefined } + } + const workspaces = execution.workspaces + if (!workspaces) { + throw new Error('agent_launch_workspace_factory_required') + } + execution.onStage?.('worktree_create') + return workspaces.createWorktree({ + // A caller migrating from `worktree.create` passes its existing params; a stale `startupAgent` + // in there would re-create the agent-first path this executor exists to replace. + create: withoutReservedAgentCreateFields(intent.target.create), + startupAgent: preflight.mode === 'structured' ? undefined : intent.agent + }) +} + +async function createSurface( + execution: AgentLaunchExecution, + worktreeId: string, + settled: AgentLaunchModeReceipt +): Promise { + const { intent, surfaces } = execution + if (settled.mode === 'structured' && isStructuredProvider(intent.agent)) { + const session = await surfaces.createStructuredSession({ + worktreeId, + agent: intent.agent, + ...(intent.sessionOptions ? { options: intent.sessionOptions } : {}) + }) + return { kind: 'structured', sessionId: session.sessionId, handle: session.handle } + } + const terminal = await surfaces.createTerminalAgent({ + worktreeId, + agent: intent.agent, + ...(intent.sessionOptions ? { options: intent.sessionOptions } : {}) + }) + return { + kind: 'terminal', + handle: terminal.handle, + ...(terminal.warning ? { warning: terminal.warning } : {}) + } +} + +function isStructuredProvider(agent: TuiAgent): agent is 'claude' | 'codex' { + return agent === 'claude' || agent === 'codex' +} + +function existingWorktreeId(target: AgentLaunchTarget): string { + return target.kind === 'existing' ? target.worktree : '' +} + +/** Prompt delivery is the caller's, not the executor's: a PTY paste is observed by whoever owns + * the pane, and a structured first turn is sent through the session. The executor reports the + * requested delivery back undelivered so a caller cannot mistake silence for delivery. */ +function promptReceipt(intent: AgentLaunchIntent): Pick { + if (!intent.prompt) { + return {} + } + return { prompt: { delivery: intent.prompt.delivery, delivered: false } } +} diff --git a/src/main/runtime/orca-runtime-start-tui-idle-visible-read-probe.ts b/src/main/runtime/orca-runtime-start-tui-idle-visible-read-probe.ts index adaa5fde72f..aa630277078 100644 --- a/src/main/runtime/orca-runtime-start-tui-idle-visible-read-probe.ts +++ b/src/main/runtime/orca-runtime-start-tui-idle-visible-read-probe.ts @@ -96,7 +96,10 @@ export class OrcaRuntimeWithStartTuiIdleVisibleReadProbe extends OrcaRuntimeWith : buildTerminalWaitResult(handle, 'tui-idle', leaf) } - async waitForSetupTerminalCompletion(handle: string): Promise<{ exitCode: number | null }> { + async waitForSetupTerminalCompletion( + handle: string, + signal?: AbortSignal + ): Promise<{ exitCode: number | null }> { const ptyId = this.getLivePtyForHandle(handle)?.pty.ptyId if (!ptyId) { throw new Error('terminal_handle_stale') @@ -106,9 +109,13 @@ export class OrcaRuntimeWithStartTuiIdleVisibleReadProbe extends OrcaRuntimeWith return await new Promise<{ exitCode: number | null }>((resolve, reject) => { let settled = false let unsubscribe: (() => void) | null = null + const onAbort = (): void => { + fail(signal?.reason ?? new Error('request_aborted')) + } const cleanup = (): void => { unsubscribe?.() exitAbort.abort() + signal?.removeEventListener('abort', onAbort) } const finish = (exitCode: number | null): void => { if (settled) { @@ -127,6 +134,11 @@ export class OrcaRuntimeWithStartTuiIdleVisibleReadProbe extends OrcaRuntimeWith cleanup() reject(error) } + if (signal?.aborted) { + onAbort() + return + } + signal?.addEventListener('abort', onAbort, { once: true }) const scanner = completionToken ? createSetupCompletionScanner(completionToken, finish) : null if (scanner) { diff --git a/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-06.spec.ts b/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-06.spec.ts index 6d9326b37f2..1d0841b5e1c 100644 --- a/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-06.spec.ts +++ b/src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-06.spec.ts @@ -103,6 +103,37 @@ describe('OrcaRuntimeService', () => { await expect(waiting).resolves.toEqual({ exitCode: 9 }) }) + it('cancels setup completion observation when the caller aborts', async () => { + const runtime = new OrcaRuntimeService(store) + runtime.setPtyController({ + spawn: vi.fn().mockResolvedValue({ id: 'pty-cancelled-setup' }), + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + const { handle } = await runtime.createTerminal(`path:${TEST_WORKTREE_PATH}`) + ;( + runtime as unknown as { setupCompletionTokenByPtyId: Map } + ).setupCompletionTokenByPtyId.set('pty-cancelled-setup', 'token-cancelled') + const unsubscribe = vi.fn() + vi.spyOn(runtime, 'subscribeToTerminalData').mockReturnValue(unsubscribe) + const controller = new AbortController() + + const waiting = runtime.waitForSetupTerminalCompletion(handle, controller.signal) + expect(runtime.subscribeToTerminalData).toHaveBeenCalledWith( + 'pty-cancelled-setup', + expect.any(Function) + ) + + const reason = new Error('cancelled') + controller.abort(reason) + + await expect(waiting).rejects.toBe(reason) + expect(unsubscribe).toHaveBeenCalledOnce() + }) + it('keeps observing after an uncertain setup terminal status', async () => { const runtime = new OrcaRuntimeService(store) runtime.setPtyController({ diff --git a/src/main/runtime/rpc/methods/agent-launch-schemas.ts b/src/main/runtime/rpc/methods/agent-launch-schemas.ts new file mode 100644 index 00000000000..f71be37da3a --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-schemas.ts @@ -0,0 +1,51 @@ +/** + * The wire shape of `agent.launch`, mirroring `AgentLaunchIntent`. + * + * A caller states WHERE the agent lands and WHAT it should say; it never names a mode. There is + * deliberately no `structured` / `terminal` field and no startup-agent field on the create + * payload — the host decides, and `withoutReservedAgentCreateFields` strips a stale one out of a + * payload a caller migrated over from `worktree.create`. + */ + +import { z } from 'zod' +import { isTuiAgent } from '../../../../shared/tui-agent-config' +import type { TuiAgent } from '../../../../shared/tui-agent' +import { WorktreeCreate } from './worktree-create-schemas' + +const LaunchAgent = z + .unknown() + .superRefine((value, ctx) => { + if (!isTuiAgent(value)) { + ctx.addIssue({ code: z.ZodIssueCode.custom, message: 'Unknown TUI agent' }) + } + }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the superRefine above rejects anything isTuiAgent refuses, so the transform only ever runs on a TuiAgent. + .transform((value): TuiAgent => value as TuiAgent) + +export const AgentLaunch = z.object({ + agent: LaunchAgent, + target: z.discriminatedUnion('kind', [ + z.object({ + kind: z.literal('existing'), + /** Any selector the runtime resolves, the same as every other worktree-addressed method. */ + worktree: z.string().min(1, 'Missing worktree selector') + }), + z.object({ + kind: z.literal('create-worktree'), + /** The `worktree.create` request verbatim, so a caller migrating to this method keeps its + * existing payload; the agent fields in it are stripped rather than honoured. */ + create: WorktreeCreate + }) + ]), + prompt: z + .object({ + text: z.string(), + delivery: z.enum(['submit', 'draft']) + }) + .optional(), + /** Only the seedable string options a structured create accepts; a terminal launch ignores them. */ + sessionOptions: z.record(z.string(), z.string()).optional(), + reuseTerminal: z.object({ handle: z.string().min(1, 'Missing terminal handle') }).optional() +}) + +export type AgentLaunchParams = z.infer diff --git a/src/main/runtime/rpc/methods/agent-launch-surfaces.ts b/src/main/runtime/rpc/methods/agent-launch-surfaces.ts new file mode 100644 index 00000000000..8d60944b261 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-surfaces.ts @@ -0,0 +1,83 @@ +/** + * How `agent.launch` builds the surface the executor decided on. + * + * Both halves are deliberately the plain, user-facing forms: a structured session created for the + * worktree exactly as `agentSession.create` creates one, and a terminal agent created exactly as a + * new agent tab is. Orchestration's own factories are NOT reusable here — a worker's session + * carries a dispatch hold, a mailbox and a background tab that a launch the user asked for must + * not take — which is why the executor injects this rather than branching. + */ + +import { randomUUID } from 'node:crypto' +import { narrowStructuredLaunchSeedOptions } from '../../../../shared/native-chat-session-option-defaults' +import { createStructuredAgentSessionOperationId } from '../../../../shared/structured-agent-session-mutation' +import { structuredAgentSessionTabId } from '../../../../shared/structured-agent-session-projection' +import { + AgentLaunchStructuredSessionRefusedError, + type AgentLaunchSurfaceFactory +} from '../../../agent-launch/agent-launch-executor' +import { getStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry' +import type { StructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-host' +import type { RpcContext } from '../core' +import { structuredCallerFor } from './structured-agent-session-gate' +import { createStructuredAgentSessionForWorktree } from './structured-agent-session-create' + +export function agentLaunchSurfaceFactory(context: RpcContext): AgentLaunchSurfaceFactory { + return { + createStructuredSession: async ({ worktreeId, agent, options }) => { + const sessionId = randomUUID() + const seeded = narrowStructuredLaunchSeedOptions(options) + const created = await createStructuredAgentSessionForWorktree({ + runtime: context.runtime, + ensureHost: async () => { + await context.runtime.ensureStructuredAgentSessionHost() + return requireInstalledHost() + }, + caller: structuredCallerFor(context), + envelope: { + sessionId, + clientOperationId: createStructuredAgentSessionOperationId(randomUUID), + expectedRuntimeFence: null, + // Overwritten by `prepare` with the host's own attach fingerprint. The create-intent + // conflict check it would otherwise feed guards a replayed client operation id, and this + // id was minted here rather than accepted from one. + payloadFingerprint: '' + }, + worktree: `id:${worktreeId}`, + agent, + ...(seeded ? { options: seeded } : {}), + // The user asked for this chat, so it takes the surface — unlike a dispatched worker. + activate: true + }) + if (!created.ok) { + throw new AgentLaunchStructuredSessionRefusedError( + created.refusal.code, + created.refusal.message + ) + } + return { + sessionId: created.value.sessionId, + handle: structuredAgentSessionTabId(created.value.sessionId) + } + }, + createTerminalAgent: async ({ worktreeId, agent }) => { + const terminal = await context.runtime.createTerminal(`id:${worktreeId}`, { + // The agent id is not a shell command — `cursor` is the desktop app, its CLI is + // `cursor-agent` — so the runtime builds the configured launcher. + startupAgent: agent + }) + return { + handle: terminal.handle, + ...(terminal.warning ? { warning: terminal.warning } : {}) + } + } + } +} + +function requireInstalledHost(): StructuredAgentSessionHost { + const host = getStructuredAgentSessionHost() + if (!host) { + throw new Error('structured_agent_session_unsupported') + } + return host +} diff --git a/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts b/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts new file mode 100644 index 00000000000..edf8dade6c1 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts @@ -0,0 +1,118 @@ +/** + * Creating the worktree an `agent.launch` asks for. + * + * `startupAgent` is the whole fork, and it is the same one `worker-worktree-creation` makes: a + * terminal launch creates the worktree agent-first, so the startup terminal IS the agent, while a + * structured launch creates it with no agent at all and its session is created for the worktree + * afterwards. Setup, default tabs, provenance and lineage are identical either way. + * + * The executor owns which side of that fork this call lands on; nothing here re-decides it. + */ + +import { buildCliWorkspaceProvenance } from '../../../../shared/cli-workspace-provenance' +import type { TuiAgent } from '../../../../shared/tui-agent' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { + finishAutomationWorkspaceProvenanceRequest, + releaseAutomationWorkspaceProvenanceRequest, + resolveAutomationWorkspaceProvenance +} from '../../../automations/workspace-provenance' +import type { AgentLaunchWorkspaceFactory } from '../../../agent-launch/agent-launch-executor' +import type { RpcContext } from '../core' +import { resolveRpcWorkspaceCreatorProvenance } from '../workspace-creator-context' +import { buildManagedWorktreeCreateArgs } from './worktree-create-args' +import type { AgentLaunchParams } from './agent-launch-schemas' + +type WorktreeCreateParams = Extract< + AgentLaunchParams['target'], + { kind: 'create-worktree' } +>['create'] + +const STRUCTURED_SETUP_WAIT_TIMEOUT_MS = 60_000 + +export function agentLaunchWorkspaceFactory( + context: RpcContext, + agent: TuiAgent +): AgentLaunchWorkspaceFactory { + return { + createWorktree: async ({ create, startupAgent }) => { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: already validated by `AgentLaunch`; the executor only removed the reserved agent fields, so the rest of the payload is the parsed shape. + const params = create as WorktreeCreateParams + const { runtime } = context + const repo = await runtime.showRepo(params.repo) + const automationProvenance = resolveAutomationWorkspaceProvenance({ + authority: runtime, + repoSelector: params.repo, + repo, + request: params.automationProvenanceRequest + }) + // Reserved before creation so a retry can recover; a failed attempt has to release it. + try { + const result = await runtime.createManagedWorktree({ + ...buildManagedWorktreeCreateArgs( + { ...params, ...(startupAgent ? { startupAgent } : {}) }, + { + automationProvenance, + cliProvenance: buildCliWorkspaceProvenance(params.cliProvenanceRequest, { + startupAgent: agent, + createdAt: Date.now() + }), + creatorProvenance: resolveRpcWorkspaceCreatorProvenance(context) + }, + context.clientKind ? { clientKind: context.clientKind } : {} + ), + // The launch owns the agent whichever surface it settles on, so the workspace records + // it even when no startup terminal was created for it. + createdWithAgent: agent, + // Structured sessions have no startup command to sequence behind setup. Provision the + // setup terminal synchronously and attach a completion token so the launch can wait + // before creating the chat surface. + awaitTerminalProvisioning: true, + observeSetupCompletion: true + }) + if (!startupAgent) { + await waitForStructuredSetup(runtime, result.setupReceipt) + } + finishAutomationWorkspaceProvenanceRequest(params.automationProvenanceRequest) + return { + worktreeId: result.worktree.id, + startupTerminalHandle: result.startupTerminal?.handle + } + } catch (error) { + releaseAutomationWorkspaceProvenanceRequest(params.automationProvenanceRequest) + throw error + } + } + } +} + +async function waitForStructuredSetup( + runtime: Pick, + receipt: Awaited>['setupReceipt'] +): Promise { + if ( + !receipt || + receipt.startupPolicy !== 'wait-for-setup' || + receipt.state !== 'running' || + !receipt.terminalHandle + ) { + return + } + const abort = new AbortController() + let timer: ReturnType | undefined + try { + await Promise.race([ + runtime.waitForSetupTerminalCompletion(receipt.terminalHandle, abort.signal), + new Promise((resolve) => { + timer = setTimeout(() => { + abort.abort(new Error('structured_setup_wait_timeout')) + resolve() + }, STRUCTURED_SETUP_WAIT_TIMEOUT_MS) + }) + ]) + } catch { + // Setup completion is evidence, not a reason to strand a launch when the PTY disappears. + } finally { + clearTimeout(timer) + } +} diff --git a/src/main/runtime/rpc/methods/agent-launch.test.ts b/src/main/runtime/rpc/methods/agent-launch.test.ts new file mode 100644 index 00000000000..aaf99d79629 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch.test.ts @@ -0,0 +1,482 @@ +/** + * The RPC boundary of `agent.launch`: who may call it, what it accepts, and which runtime call + * each of its three factories makes. + * + * The last group is where the defect lived. A structured launch must reach + * `createManagedWorktree` with NO startup agent — an agent-first create makes the startup terminal + * the agent and puts the structured branch out of reach — and the old `worktree.create` contract + * must be observably untouched by any of it. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { AGENT_LAUNCH_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import type { RpcContext } from '../core' + +const createStructuredSession = vi.fn(async (_args: Record) => ({ + ok: true as const, + value: { sessionId: 'sess-1' } +})) + +vi.mock('./structured-agent-session-create', () => ({ + createStructuredAgentSessionForWorktree: (args: Record) => + createStructuredSession(args) +})) + +const { AGENT_LAUNCH_METHODS } = await import('./agent-launch') +const { WORKTREE_METHODS } = await import('./worktree') + +const STRUCTURED_PREFERENCE = { + experimentalNativeChat: true, + experimentalStructuredNativeChat: true, + openAgentTabsInChatByDefault: true +} + +function runtimeStub( + options: { + settings?: Record + createSupport?: { supported: boolean; reason?: 'agent' | 'remote' | 'wsl' } + setupReceipt?: { + startupPolicy: 'start-immediately' | 'wait-for-setup' + state: 'running' | 'skipped' | 'not_configured' | 'spawn_failed' + terminalHandle?: string + } + } = {} +) { + const worktreeCreateResults = new Map>() + const waitForSetupTerminalCompletion = vi.fn( + async (_handle: string, _signal?: AbortSignal): Promise<{ exitCode: number | null }> => ({ + exitCode: 0 + }) + ) + return { + getClientSettings: vi.fn(() => options.settings ?? STRUCTURED_PREFERENCE), + getStructuredAgentSessionCreateSupport: vi.fn( + async () => options.createSupport ?? { supported: true } + ), + dedupeWorktreeCreate: vi.fn( + (repo: string, key: string | undefined, run: () => Promise) => { + if (!key) { + return run() + } + const compositeKey = `${repo}\0${key}` + const existing = worktreeCreateResults.get(compositeKey) + if (existing) { + return existing + } + const result = run() + worktreeCreateResults.set(compositeKey, result) + void result.catch(() => worktreeCreateResults.delete(compositeKey)) + return result + } + ), + showRepo: vi.fn(async () => ({ id: 'repo-1' })), + createManagedWorktree: vi.fn(async (args: Record) => ({ + worktree: { id: 'wt-new' }, + startupTerminal: args.startupAgent ? { handle: 'term_agent_first' } : undefined, + ...(options.setupReceipt ? { setupReceipt: options.setupReceipt } : {}) + })), + createTerminal: vi.fn(async () => ({ handle: 'term_1' })), + showTerminal: vi.fn(async (handle: string) => ({ handle, worktreeId: 'wt-7' })), + isTerminalRunningAgent: vi.fn(async () => true), + showManagedTerminalWorkspace: vi.fn(async (selector: string) => ({ + id: selector.replace(/^id:/, '') + })), + ensureStructuredAgentSessionHost: vi.fn(async () => {}), + waitForSetupTerminalCompletion + } +} + +type RuntimeStub = ReturnType + +function methodNamed( + methods: readonly TMethod[], + name: TName +): Extract { + const found = methods.find( + (entry): entry is Extract => entry.name === name + ) + if (!found) { + throw new Error(`missing method ${name}`) + } + return found +} + +const AGENT_LAUNCH = methodNamed(AGENT_LAUNCH_METHODS, 'agent.launch') + +function parseLaunch(params: unknown) { + return AGENT_LAUNCH.params.safeParse(params) +} + +// The one call the stub cannot satisfy structurally; every method it does implement is asserted. +function rpcContext(runtime: RuntimeStub, context: Partial): RpcContext { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the stub implements only the runtime surface these methods reach, so a method it omits throws on call rather than reading a wrong value. + return { runtime, ...context } as unknown as RpcContext +} + +function createArgs(runtime: RuntimeStub): Record { + const [args] = runtime.createManagedWorktree.mock.calls[0] ?? [] + if (!args) { + throw new Error('createManagedWorktree was not called') + } + return args +} + +const CAPABLE_CLIENT: Partial = { + clientKind: 'mobile', + pairedDeviceId: 'device-1', + clientCapabilities: [AGENT_LAUNCH_RUNTIME_CAPABILITY] +} + +async function launch( + params: unknown, + runtime: RuntimeStub, + context: Partial = CAPABLE_CLIENT +) { + const parsed = parseLaunch(params) + if (!parsed.success) { + throw new Error(parsed.error.issues[0]?.message ?? 'invalid') + } + return AGENT_LAUNCH.handler(parsed.data, rpcContext(runtime, context)) +} + +const CREATE_LAUNCH = { + agent: 'claude', + target: { kind: 'create-worktree', create: { repo: 'id:repo-1', name: 'task' } } +} + +const IDEMPOTENT_CREATE_LAUNCH = { + agent: 'claude', + target: { + kind: 'create-worktree' as const, + create: { repo: 'id:repo-1', name: 'task', clientMutationId: 'launch-1' } + } +} + +beforeEach(() => { + createStructuredSession.mockClear() +}) + +describe('who may call agent.launch', () => { + it('refuses a paired client that did not negotiate the capability', async () => { + const runtime = runtimeStub() + await expect( + launch(CREATE_LAUNCH, runtime, { + clientKind: 'mobile', + pairedDeviceId: 'device-1', + clientCapabilities: [] + }) + ).rejects.toThrow('agent_launch_unsupported') + expect(runtime.createManagedWorktree).not.toHaveBeenCalled() + }) + + it('admits a client that advertises it', async () => { + const runtime = runtimeStub() + await launch(CREATE_LAUNCH, runtime) + expect(runtime.createManagedWorktree).toHaveBeenCalled() + }) + + it('admits an in-process caller, which negotiates nothing', async () => { + const runtime = runtimeStub() + await launch(CREATE_LAUNCH, runtime, {}) + expect(runtime.createManagedWorktree).toHaveBeenCalled() + }) +}) + +describe('what agent.launch accepts', () => { + it('rejects an agent Orca cannot launch', () => { + expect(parseLaunch({ ...CREATE_LAUNCH, agent: 'not-an-agent' }).success).toBe(false) + }) + + it('rejects a target that names neither an existing workspace nor a create', () => { + expect(parseLaunch({ agent: 'claude', target: { kind: 'somewhere' } }).success).toBe(false) + }) + + it('rejects an existing target with no selector', () => { + expect( + parseLaunch({ agent: 'claude', target: { kind: 'existing', worktree: '' } }).success + ).toBe(false) + }) + + it('rejects a create payload with no repo, the same as worktree.create does', () => { + expect( + parseLaunch({ agent: 'claude', target: { kind: 'create-worktree', create: { name: 'x' } } }) + .success + ).toBe(false) + }) + + it('accepts a prompt, seed options and a reused terminal', () => { + expect( + parseLaunch({ + agent: 'codex', + target: { kind: 'existing', worktree: 'id:wt-1' }, + prompt: { text: 'do the thing', delivery: 'draft' }, + sessionOptions: { model: 'gpt-5', effort: 'high' }, + reuseTerminal: { handle: 'term_live' } + }).success + ).toBe(true) + }) + + it('validates a reused terminal against the addressed workspace', async () => { + const runtime = runtimeStub() + const result = await launch( + { + agent: 'claude', + target: { kind: 'existing', worktree: 'id:wt-7' }, + reuseTerminal: { handle: 'term_live' } + }, + runtime + ) + + expect(runtime.showTerminal).toHaveBeenCalledWith('term_live') + expect(runtime.isTerminalRunningAgent).toHaveBeenCalledWith('term_live') + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_live' }) + }) + + it('rejects a reused terminal from a different workspace before launching', async () => { + const runtime = runtimeStub() + runtime.showTerminal.mockResolvedValue({ handle: 'term_live', worktreeId: 'wt-other' }) + + await expect( + launch( + { + agent: 'claude', + target: { kind: 'existing', worktree: 'id:wt-7' }, + reuseTerminal: { handle: 'term_live' } + }, + runtime + ) + ).rejects.toThrow('agent_launch_terminal_worktree_mismatch') + expect(runtime.createManagedWorktree).not.toHaveBeenCalled() + }) + + it('rejects reusing a terminal while creating a new workspace', async () => { + const runtime = runtimeStub() + await expect( + launch({ ...CREATE_LAUNCH, reuseTerminal: { handle: 'term_live' } }, runtime) + ).rejects.toThrow('agent_launch_reuse_requires_existing_workspace') + expect(runtime.showTerminal).not.toHaveBeenCalled() + expect(runtime.createManagedWorktree).not.toHaveBeenCalled() + }) +}) + +describe('the worktree factory', () => { + it('creates a structured launch’s worktree with no startup agent', async () => { + const runtime = runtimeStub() + const result = await launch(CREATE_LAUNCH, runtime) + + const args = createArgs(runtime) + expect(args.startupAgent).toBeUndefined() + expect(args.awaitTerminalProvisioning).toBe(true) + expect(args.observeSetupCompletion).toBe(true) + // Still recorded on the workspace: the launch owns the agent whichever surface it settles on. + expect(args.createdWithAgent).toBe('claude') + expect(result.outcome.kind).toBe('structured') + }) + + it('deduplicates concurrent launches through surface creation', async () => { + const runtime = runtimeStub() + + const results = await Promise.all([ + launch(IDEMPOTENT_CREATE_LAUNCH, runtime), + launch(IDEMPOTENT_CREATE_LAUNCH, runtime) + ]) + + expect(results[0]).toEqual(results[1]) + expect(runtime.dedupeWorktreeCreate).toHaveBeenCalledTimes(2) + expect(runtime.dedupeWorktreeCreate.mock.calls).toEqual([ + ['id:repo-1', 'agent.launch:launch-1', expect.any(Function)], + ['id:repo-1', 'agent.launch:launch-1', expect.any(Function)] + ]) + expect(runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).toHaveBeenCalledTimes(1) + }) + + it('reuses a completed launch result for a sequential retry', async () => { + const runtime = runtimeStub() + + const first = await launch(IDEMPOTENT_CREATE_LAUNCH, runtime) + const retried = await launch(IDEMPOTENT_CREATE_LAUNCH, runtime) + + expect(retried).toEqual(first) + expect(runtime.createManagedWorktree).toHaveBeenCalledTimes(1) + expect(createStructuredSession).toHaveBeenCalledTimes(1) + }) + + it('aborts the setup wait when its bounded timeout expires', async () => { + vi.useFakeTimers() + try { + const runtime = runtimeStub({ + setupReceipt: { + startupPolicy: 'wait-for-setup', + state: 'running', + terminalHandle: 'setup-1' + } + }) + let setupSignal: AbortSignal | undefined + runtime.waitForSetupTerminalCompletion.mockImplementation( + (_handle, signal) => + new Promise<{ exitCode: number | null }>((_resolve, reject) => { + setupSignal = signal + signal?.addEventListener('abort', () => reject(signal.reason), { once: true }) + }) + ) + + const result = await (async () => { + const pending = launch(CREATE_LAUNCH, runtime) + await vi.runAllTimersAsync() + return pending + })() + + expect(result.outcome.kind).toBe('structured') + expect(setupSignal?.aborted).toBe(true) + } finally { + vi.useRealTimers() + } + }) + + it('waits for a setup-gated structured workspace before creating its session', async () => { + const runtime = runtimeStub({ + setupReceipt: { + startupPolicy: 'wait-for-setup', + state: 'running', + terminalHandle: 'setup-1' + } + }) + const order: string[] = [] + runtime.waitForSetupTerminalCompletion.mockImplementation(async () => { + order.push('setup-complete') + return { exitCode: 0 } + }) + createStructuredSession.mockImplementationOnce(async () => { + order.push('structured-create') + return { ok: true as const, value: { sessionId: 'sess-1' } } + }) + + await launch(CREATE_LAUNCH, runtime) + + expect(order).toEqual(['setup-complete', 'structured-create']) + expect(runtime.waitForSetupTerminalCompletion).toHaveBeenCalledWith( + 'setup-1', + expect.any(AbortSignal) + ) + }) + + it('keeps agent-first creation for a launch the user wants as a terminal', async () => { + const runtime = runtimeStub({ settings: {} }) + const result = await launch(CREATE_LAUNCH, runtime) + + const args = createArgs(runtime) + expect(args.startupAgent).toBe('claude') + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_agent_first' }) + expect(runtime.getStructuredAgentSessionCreateSupport).not.toHaveBeenCalled() + }) + + it('drops a stale startupAgent a caller carried over from worktree.create', async () => { + const runtime = runtimeStub() + await launch( + { + agent: 'claude', + target: { + kind: 'create-worktree', + create: { + repo: 'id:repo-1', + name: 'task', + startupAgent: 'codex', + startupCommand: 'codex --yolo' + } + } + }, + runtime + ) + const args = createArgs(runtime) + expect(args.startupAgent).toBeUndefined() + expect(args.startup).toBeUndefined() + }) +}) + +describe('the structured session factory', () => { + it('creates the session for the worktree the launch just made, and activates it', async () => { + const runtime = runtimeStub() + const result = await launch(CREATE_LAUNCH, runtime) + + expect(createStructuredSession).toHaveBeenCalledTimes(1) + expect(createStructuredSession.mock.calls[0]?.[0]).toMatchObject({ + worktree: 'id:wt-new', + agent: 'claude', + activate: true + }) + expect(result.outcome).toEqual({ + kind: 'structured', + sessionId: 'sess-1', + handle: 'structured-agent-session-sess-1' + }) + expect(runtime.createTerminal).not.toHaveBeenCalled() + }) + + it('seeds only the options a structured create accepts', async () => { + const runtime = runtimeStub() + await launch( + { + ...CREATE_LAUNCH, + sessionOptions: { model: 'sonnet', effort: 'high', fastMode: 'yes' } + }, + runtime + ) + expect(createStructuredSession.mock.calls[0]?.[0]).toMatchObject({ + options: { model: 'sonnet', effort: 'high' } + }) + }) +}) + +describe('the terminal factory', () => { + it('starts the agent through the runtime launcher when the host refuses a session', async () => { + const runtime = runtimeStub({ createSupport: { supported: false, reason: 'wsl' } }) + const result = await launch(CREATE_LAUNCH, runtime) + + expect(runtime.createTerminal).toHaveBeenCalledWith('id:wt-new', { startupAgent: 'claude' }) + expect(createStructuredSession).not.toHaveBeenCalled() + expect(result.outcome).toEqual({ kind: 'terminal', handle: 'term_1' }) + // Never a failed launch, and never a silent downgrade. + expect(result.receipt).toMatchObject({ mode: 'terminal', reason: 'wsl_execution_runtime' }) + }) + + it('takes an existing workspace without creating one', async () => { + const runtime = runtimeStub() + const result = await launch( + { agent: 'grok', target: { kind: 'existing', worktree: 'id:wt-7' } }, + runtime + ) + + expect(runtime.createManagedWorktree).not.toHaveBeenCalled() + expect(runtime.showManagedTerminalWorkspace).toHaveBeenCalledWith('id:wt-7') + // Resolved to an id first: everything below re-prefixes it, so a raw selector reaches the + // runtime as `id:id:wt-7`. + expect(runtime.createTerminal).toHaveBeenCalledWith('id:wt-7', { startupAgent: 'grok' }) + expect(result.worktreeId).toBe('wt-7') + }) +}) + +describe('worktree.create is untouched by any of this', () => { + it('still answers a startupAgent create with a PTY agent and its handle', async () => { + const runtime = runtimeStub() + const create = methodNamed(WORKTREE_METHODS, 'worktree.create') + const parsed = create.params.safeParse({ + repo: 'id:repo-1', + name: 'task', + startupAgent: 'claude' + }) + if (!parsed.success) { + throw new Error(parsed.error.issues[0]?.message ?? 'invalid') + } + + const result = await create.handler(parsed.data, rpcContext(runtime, {})) + + expect(result).toMatchObject({ agentTerminalHandle: 'term_agent_first' }) + expect(runtime.createManagedWorktree.mock.calls[0]?.[0]).toMatchObject({ + startupAgent: 'claude' + }) + // The route is not consulted on this path, so no client's create can change surface under it. + expect(runtime.getStructuredAgentSessionCreateSupport).not.toHaveBeenCalled() + expect(createStructuredSession).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/runtime/rpc/methods/agent-launch.ts b/src/main/runtime/rpc/methods/agent-launch.ts new file mode 100644 index 00000000000..54e0e8a76f6 --- /dev/null +++ b/src/main/runtime/rpc/methods/agent-launch.ts @@ -0,0 +1,116 @@ +/** + * `agent.launch` — the one method that starts an agent, whatever surface it turns out to be. + * + * It exists because the routing decision had no host-side home: `worktree.create` never consulted + * it, so any client that created a worktree with `startupAgent` got a PTY agent no matter what the + * user's default said. That is not fixable inside `worktree.create`, because its contract is + * exactly "spawn a PTY agent and hand me its `agentTerminalHandle`" — a host that quietly answered + * it with a structured session would hand every older client a response with no handle and no + * error. So `worktree.create` keeps that meaning verbatim, forever, and everything that has to + * choose a surface comes here instead, behind a negotiated capability. + * + * A caller therefore never asks for a mode, and must read `outcome.kind` rather than assume one: + * the receipt always says which surface ran and why, so a downgrade is never silent. + */ + +import { AGENT_LAUNCH_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import type { AgentLaunchIntent, AgentLaunchTarget } from '../../../../shared/agent-launch-intent' +import { executeAgentLaunch } from '../../../agent-launch/agent-launch-executor' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { defineMethod, type RpcContext } from '../core' +import { AgentLaunch, type AgentLaunchParams } from './agent-launch-schemas' +import { agentLaunchSurfaceFactory } from './agent-launch-surfaces' +import { agentLaunchWorkspaceFactory } from './agent-launch-worktree-creation' + +/** + * Advertising `agent.launch.v1` is a client's statement that it understands EITHER outcome — a + * structured session it can open, or a terminal agent. A client that can only render one of the + * two must keep using the surface-specific methods instead. In-process callers are the same build + * as the host and negotiate nothing. + */ +export function supportsAgentLaunch( + context: Pick +): boolean { + return ( + context.clientKind === undefined || + context.clientCapabilities?.includes(AGENT_LAUNCH_RUNTIME_CAPABILITY) === true + ) +} + +/** + * A client addresses a workspace by selector, but the result's `worktreeId` is an id and every + * step below the executor re-prefixes it as `id:`. Resolving here is what keeps a + * caller's `id:wt-7` from reaching the runtime as `id:id:wt-7`; the terminal-workspace resolver is + * used rather than the git-worktree one so a folder workspace is addressable too. + */ +async function agentLaunchTarget( + params: AgentLaunchParams, + runtime: Pick +): Promise { + if (params.target.kind === 'create-worktree') { + return { kind: 'create-worktree', create: { ...params.target.create } } + } + const workspace = await runtime.showManagedTerminalWorkspace(params.target.worktree) + return { kind: 'existing', worktree: workspace.id } +} + +async function agentLaunchIntent( + params: AgentLaunchParams, + runtime: OrcaRuntimeService +): Promise { + return { + agent: params.agent, + target: await agentLaunchTarget(params, runtime), + ...(params.prompt ? { prompt: params.prompt } : {}), + ...(params.sessionOptions ? { sessionOptions: params.sessionOptions } : {}), + ...(params.reuseTerminal ? { reuseTerminal: params.reuseTerminal } : {}) + } +} + +async function validateReusedTerminal( + intent: AgentLaunchIntent, + runtime: Pick +): Promise { + if (!intent.reuseTerminal) { + return + } + if (intent.target.kind !== 'existing') { + throw new Error('agent_launch_reuse_requires_existing_workspace') + } + const terminal = await runtime.showTerminal(intent.reuseTerminal.handle) + if (terminal.worktreeId !== intent.target.worktree) { + throw new Error('agent_launch_terminal_worktree_mismatch') + } + if (!(await runtime.isTerminalRunningAgent(intent.reuseTerminal.handle))) { + throw new Error('agent_launch_terminal_not_running_agent') + } +} + +export const AGENT_LAUNCH_METHODS = [ + defineMethod({ + name: 'agent.launch', + params: AgentLaunch, + handler: async (params, context) => { + if (!supportsAgentLaunch(context)) { + throw new Error('agent_launch_unsupported') + } + const intent = await agentLaunchIntent(params, context.runtime) + await validateReusedTerminal(intent, context.runtime) + const execute = () => + executeAgentLaunch({ + runtime: context.runtime, + intent, + surfaces: agentLaunchSurfaceFactory(context), + workspaces: agentLaunchWorkspaceFactory(context, intent.agent) + }) + if (params.target.kind === 'create-worktree' && params.target.create.clientMutationId) { + return context.runtime.dedupeWorktreeCreate( + params.target.create.repo, + `agent.launch:${params.target.create.clientMutationId}`, + execute + ) + } + return execute() + } + }) +] diff --git a/src/main/runtime/rpc/methods/index.ts b/src/main/runtime/rpc/methods/index.ts index 3a53bccb9ce..3be913d7ca5 100644 --- a/src/main/runtime/rpc/methods/index.ts +++ b/src/main/runtime/rpc/methods/index.ts @@ -45,6 +45,7 @@ import { AGENT_SESSION_METHODS } from './agent-session' import { STRUCTURED_AGENT_SESSION_METHODS } from './structured-agent-session' import { ARTIFACT_METHODS } from './artifacts' import { AGENT_HOOK_METHODS } from './agent-hooks' +import { AGENT_LAUNCH_METHODS } from './agent-launch' // Why: a flat manifest keeps registration order explicit and provides one // grep-point for "what methods does the RPC server expose?" — useful when @@ -59,6 +60,7 @@ export const ALL_RPC_METHODS = [ ...WORKTREE_METHODS, ...AGENT_SESSION_METHODS, ...STRUCTURED_AGENT_SESSION_METHODS, + ...AGENT_LAUNCH_METHODS, ...TERMINAL_METHODS, ...TERMINAL_ORPHAN_METHODS, ...BROWSER_CORE_METHODS, diff --git a/src/main/runtime/rpc/rpc-params-type-parity.ts b/src/main/runtime/rpc/rpc-params-type-parity.ts index 263a7fe38cd..ae3a5fcc76f 100644 --- a/src/main/runtime/rpc/rpc-params-type-parity.ts +++ b/src/main/runtime/rpc/rpc-params-type-parity.ts @@ -8,7 +8,11 @@ import type { ALL_RPC_METHODS } from './methods' type RegisteredMethod = (typeof ALL_RPC_METHODS)[number] // These schemas reach into src/main and have no shared catalog entry. -type UncataloguedMethod = 'emulator.install' | 'orchestration.send' | 'orchestration.taskUpdate' +type UncataloguedMethod = + | 'agent.launch' + | 'emulator.install' + | 'orchestration.send' + | 'orchestration.taskUpdate' type IsAny = 0 extends 1 & T ? true : false diff --git a/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts b/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts index e536f76260c..69cd49ccd30 100644 --- a/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts +++ b/src/main/runtime/runtime-rpc/runtime-rpc-mobile-method-allowlist.ts @@ -6,6 +6,7 @@ export const MOBILE_RPC_METHOD_ALLOWLIST = new Set([ 'accounts.selectCodexForTarget', 'accounts.subscribe', 'accounts.unsubscribe', + 'agent.launch', 'aiVault.listSessions', 'aiVault.searchSessions', 'aiVault.searchStatus', diff --git a/src/shared/agent-launch-intent.ts b/src/shared/agent-launch-intent.ts new file mode 100644 index 00000000000..32bb4c38390 --- /dev/null +++ b/src/shared/agent-launch-intent.ts @@ -0,0 +1,133 @@ +/** + * What a caller asks for when it wants an agent running somewhere, independent of which surface + * asked and of whether the answer turns out to be a structured session or a terminal. + * + * Every launch surface builds one of these: the renderer's agent tabs and workspace creates, + * mobile's create sheet and new-tab button, `orchestration.workerStart`, and the CLI. The host + * resolves it once — settings default plus per-launch feasibility from + * `structured-native-chat-launch-route` — so no surface carries its own copy of that decision. + * + * The intent deliberately does NOT name a mode. A caller states what it wants to happen, not how + * to deliver it; picking structured vs terminal is the host's job and is reported back in the + * receipt rather than requested here. + */ + +import type { TuiAgent } from './tui-agent' + +/** How a launch's initial text reaches the agent. */ +export type AgentLaunchPromptDelivery = + /** Sent as the agent's first turn once it is ready. */ + | 'submit' + /** Left unsent for the user to edit and send. Historically this forced a terminal, because a + * draft lived in the TUI's input and chat only mirrored it; a structured session accepts one + * directly, so it no longer decides the route. */ + | 'draft' + +export type AgentLaunchPrompt = { + text: string + delivery: AgentLaunchPromptDelivery +} + +/** + * Where the agent lands. + * + * `create-worktree` is part of the intent rather than a separate call the caller makes first, + * because the route cannot be settled before the workspace exists: `agentSession.createSupport` + * can only answer for a workspace the host can resolve. Splitting the two is exactly what made + * every new-worktree launch a terminal — the worktree was created agent-first, so the structured + * branch below it was unreachable. + */ +export type AgentLaunchTarget = + /** A workspace that already exists, addressed by any selector the runtime resolves. */ + | { kind: 'existing'; worktree: string } + /** A worktree this launch creates. `create` is the `worktree.create` request minus its agent + * fields — the launch owns those, so a caller cannot set a startup agent behind the router. */ + | { kind: 'create-worktree'; create: Readonly> } + +/** An existing terminal the caller wants reused rather than a fresh surface. Always resolves to a + * terminal agent: a running PTY keeps its execution transport. */ +export type AgentLaunchReusedTerminal = { handle: string } + +/** + * Facts that only the calling surface knows and that the route has to see. These are inputs to the + * decision, not requests: a caller states that it is passing custom agent arguments, and the host + * concludes that a terminal is required. + */ +export type AgentLaunchCustomization = { + /** Explicit per-launch agent argv. Only a TUI applies these. */ + agentArgs?: string + /** A subdirectory the agent should start in. Only a TUI applies this. */ + cwd?: string +} + +export type AgentLaunchIntent = { + agent: TuiAgent + target: AgentLaunchTarget + prompt?: AgentLaunchPrompt + /** Seeded launch options, narrowed by the host to what a structured create accepts. */ + sessionOptions?: Readonly> + reuseTerminal?: AgentLaunchReusedTerminal + customization?: AgentLaunchCustomization +} + +/** The surface the host actually created. */ +export type AgentLaunchOutcome = + | { kind: 'structured'; sessionId: string; handle: string } + | { kind: 'terminal'; handle: string; warning?: string } + +/** Whether the launch text was delivered, for a caller that needs to report or retry it. */ +export type AgentLaunchPromptReceipt = { + delivery: AgentLaunchPromptDelivery + delivered: boolean +} + +export type AgentLaunchResult = { + outcome: AgentLaunchOutcome + /** The workspace the agent runs in, resolved or created. */ + worktreeId: string + /** Why the outcome is what it is — always populated, so a downgrade is never silent. */ + receipt: AgentLaunchModeReceipt + prompt?: AgentLaunchPromptReceipt +} + +/** Restates `WorkerStartModeReceipt` in surface-neutral terms so orchestration's receipt and a + * mobile or renderer launch report the same vocabulary. */ +export type AgentLaunchModeReceipt = { + mode: 'structured' | 'terminal' + /** The user's settings default for a new agent tab. */ + preferred: 'structured' | 'terminal' + reason: string + /** One sentence, always present. */ + detail: string +} + +export function agentLaunchTargetIsCreate( + target: AgentLaunchTarget +): target is Extract { + return target.kind === 'create-worktree' +} + +/** The agent fields a create payload must not carry: the launch owns placement, and a caller that + * sets one of these would route itself around the host's decision. */ +export const AGENT_LAUNCH_RESERVED_CREATE_FIELDS = [ + 'startupAgent', + 'startupCommand', + 'startupPrompt', + 'startupDraft', + 'startupLaunchConfig', + 'startupEnv', + 'startupCommandDelivery' +] as const + +/** Strips the reserved agent fields from a create payload. Callers migrating from + * `worktree.create` pass their existing params; this keeps a stale `startupAgent` from + * re-creating the agent-first path the router exists to replace. */ +export function withoutReservedAgentCreateFields( + create: Readonly> +): Record { + const stripped: Record = { ...create } + for (const field of AGENT_LAUNCH_RESERVED_CREATE_FIELDS) { + delete stripped[field] + } + return stripped +} diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index efe101a4eb0..6d34087aa24 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -235,6 +235,22 @@ export const AUTOMATION_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY = // Hosts without this capability have no notifications.registerPush RPC. export const NOTIFICATIONS_REMOTE_PUSH_RUNTIME_CAPABILITY = 'notifications.remote-push.v1' as const +/** + * `agent.launch` exists: one host-side method that decides structured-vs-terminal and creates the + * surface, instead of each client routing for itself. + * + * Negotiated rather than assumed because a client that cannot see it must keep using + * `worktree.create` + `startupAgent`, which stays supported verbatim. The reverse skew is the + * dangerous one: `worktree.create` returns `agentTerminalHandle` only when a startup agent was + * requested, so a host that quietly routed that call to a structured session would hand an old + * client a response with no handle and no error. + * + * Advertising it is a statement that the client understands EITHER outcome, since the host is what + * picks: a structured session it can open, or a terminal agent. A client that renders only one of + * the two keeps using the surface-specific methods. + */ +export const AGENT_LAUNCH_RUNTIME_CAPABILITY = 'agent.launch.v1' as const + // Generic native clients include the CLI and must not claim Electron-only page // placement support. export const NATIVE_REMOTE_RUNTIME_CLIENT_CAPABILITIES = [ @@ -244,7 +260,8 @@ export const NATIVE_REMOTE_RUNTIME_CLIENT_CAPABILITIES = [ WORKTREE_VISIBILITY_SOURCE_DEFAULTS_RUNTIME_CAPABILITY, WORKTREE_GITHUB_PR_SUPPRESSION_RUNTIME_CAPABILITY, AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, - AUTOMATION_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY + AUTOMATION_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY, + AGENT_LAUNCH_RUNTIME_CAPABILITY ] as const // Electron clients can decode client-hosted page placement; becoming a page @@ -339,7 +356,8 @@ export const RUNTIME_CAPABILITIES = [ AUTOMATION_LIST_HOST_SCOPE_RUNTIME_CAPABILITY, AUTOMATION_OWNER_FENCING_RUNTIME_CAPABILITY, AUTOMATION_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY, - NOTIFICATIONS_REMOTE_PUSH_RUNTIME_CAPABILITY + NOTIFICATIONS_REMOTE_PUSH_RUNTIME_CAPABILITY, + AGENT_LAUNCH_RUNTIME_CAPABILITY ] as const export type RuntimeCapability = (typeof RUNTIME_CAPABILITIES)[number] | (string & {}) diff --git a/src/shared/rpc-contract/rpc-params-catalog.generated.ts b/src/shared/rpc-contract/rpc-params-catalog.generated.ts index deae9bc0eaa..7824726bccd 100644 --- a/src/shared/rpc-contract/rpc-params-catalog.generated.ts +++ b/src/shared/rpc-contract/rpc-params-catalog.generated.ts @@ -1160,6 +1160,7 @@ export const RPC_PARAMS_BY_METHOD = { // Why: these methods bind a schema the shared contract cannot hold because its value // graph reaches into src/main. Listing them keeps the gap visible instead of absent. export const RPC_METHODS_WITHOUT_SHARED_PARAMS: readonly string[] = [ + 'agent.launch', 'emulator.install', 'orchestration.send', 'orchestration.taskUpdate'