From 3fdcbfaa1e775d79644e47f94b1ca9d280258a56 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 16 Oct 2026 13:45:59 -0700 Subject: [PATCH] fix(terminal): open the picked Windows shell on a managed host instead of typing it into PowerShell (#29836) * fix(terminal): open the picked Windows shell on a managed host instead of typing it into PowerShell * fix(terminal): keep the shell pick through materialization and folder runtimes * fix(terminal): refuse a shell pick on agent launches and update paired-tab expectations * test(terminal): type the shell-selection runtime fixture instead of casting --- ...obile-session-tabs-shell-selection.spec.ts | 137 ++++++++++++++++++ src/main/runtime/orca-runtime.test.ts | 1 + ...session-tabs-create-terminal-shell.test.ts | 74 ++++++++++ src/main/runtime/rpc/methods/session-tabs.ts | 1 + .../terminal/launch/mobile-terminal-create.ts | 1 + .../launch/mobile-terminal-owned-create.ts | 2 + .../launch/mobile-terminal-renderer-create.ts | 6 + .../mobile-terminal-shell-refusal.test.ts | 63 ++++++++ .../launch/mobile-terminal-shell-refusal.ts | 30 ++++ .../tab-group/useTabGroupCreationCommands.ts | 2 +- .../terminal/terminal-tab-actions.test.ts | 4 +- .../terminal/terminal-tab-create.ts | 2 +- .../components/use-terminal-create-actions.ts | 2 +- ...ntime-plain-terminal-create-params.test.ts | 55 +++++++ ...eb-runtime-plain-terminal-create-params.ts | 51 +++++++ .../src/runtime/web-runtime-session-types.ts | 2 + .../web-runtime-terminal-create-operation.ts | 19 +-- .../session-tabs-schemas-params.ts | 11 ++ .../rpc-contract/terminal-unary-params.ts | 23 +-- .../session-tabs-runtime-capabilities.ts | 8 +- 20 files changed, 460 insertions(+), 34 deletions(-) create mode 100644 src/main/runtime/orca-runtime-tests/mobile-session-tabs-shell-selection.spec.ts create mode 100644 src/main/runtime/rpc/methods/session-tabs-create-terminal-shell.test.ts create mode 100644 src/main/runtime/terminal/launch/mobile-terminal-shell-refusal.test.ts create mode 100644 src/main/runtime/terminal/launch/mobile-terminal-shell-refusal.ts create mode 100644 src/renderer/src/runtime/web-runtime-plain-terminal-create-params.test.ts create mode 100644 src/renderer/src/runtime/web-runtime-plain-terminal-create-params.ts diff --git a/src/main/runtime/orca-runtime-tests/mobile-session-tabs-shell-selection.spec.ts b/src/main/runtime/orca-runtime-tests/mobile-session-tabs-shell-selection.spec.ts new file mode 100644 index 00000000000..a18028fc5fd --- /dev/null +++ b/src/main/runtime/orca-runtime-tests/mobile-session-tabs-shell-selection.spec.ts @@ -0,0 +1,137 @@ +import { describe, expect, it, vi } from 'vitest' +import { OrcaRuntimeService, electronMocks, ipcMain } from '../orca-runtime-test-mocks.spec' +import { TEST_REPO_ID, TEST_WORKTREE_ID, store } from '../orca-runtime-test-fixtures.spec' +import { createMobileCreateTestNotifier } from '../orca-runtime-test-scenario-builders.spec' + +function makeHeadlessRuntime(runtimeStore: typeof store = store) { + const spawn = vi.fn().mockResolvedValue({ id: 'pty-shell-pick' }) + const runtime = new OrcaRuntimeService(runtimeStore) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => true, + getForegroundProcess: async () => null + }) + runtime.syncWindowGraph(0, { tabs: [], leaves: [] }) + return { runtime, spawn } +} + +async function onHostPlatform(platform: NodeJS.Platform, run: () => Promise): Promise { + const hostPlatform = Object.getOwnPropertyDescriptor(process, 'platform')! + Object.defineProperty(process, 'platform', { configurable: true, value: platform }) + try { + await run() + } finally { + Object.defineProperty(process, 'platform', hostPlatform) + } +} + +const onWindowsHost = (run: () => Promise) => onHostPlatform('win32', run) + +describe('OrcaRuntimeService session-tab shell selection', () => { + it('spawns the picked shell for a headless session terminal instead of typing it', async () => { + await onWindowsHost(async () => { + const { runtime, spawn } = makeHeadlessRuntime() + + await runtime.createMobileSessionTerminal(`id:${TEST_WORKTREE_ID}`, { + shellOverride: 'git-bash' + }) + + // The defect this pins: the pick rode as `command`, so the managed host typed + // `git-bash` into PowerShell and the tab stayed PowerShell. + expect(spawn).toHaveBeenCalledWith( + expect.objectContaining({ shellOverride: 'git-bash', command: undefined }) + ) + }) + }) + + it('keeps the pick when a renderer-created tab is materialized after its surface stalls', async () => { + await onWindowsHost(async () => { + vi.useFakeTimers() + try { + const leafId = '44444444-4444-4444-8444-444444444444' + const { runtime, spawn } = makeHeadlessRuntime() + runtime.setNotifier(createMobileCreateTestNotifier(vi.fn())) + const webContents = { send: vi.fn() } + webContents.send = vi.fn((_channel: string, payload: { requestId: string }) => { + ipcMain.emit( + 'terminal:tabCreateReply', + { sender: webContents }, + { requestId: payload.requestId, tabId: 'tab-pending', title: 'Terminal' } + ) + }) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + electronMocks.BrowserWindow.fromId.mockReturnValue({ + isDestroyed: () => false, + webContents + }) + + const create = runtime.createMobileSessionTerminal(`id:${TEST_WORKTREE_ID}`, { + shellOverride: 'cmd.exe' + }) + await vi.waitFor(() => expect(webContents.send).toHaveBeenCalledTimes(1)) + expect(webContents.send.mock.calls[0]?.[1]).toMatchObject({ shellOverride: 'cmd.exe' }) + runtime.syncWindowGraph(1, { + tabs: [], + leaves: [], + mobileSessionTabs: [ + { + worktree: TEST_WORKTREE_ID, + publicationEpoch: 'renderer-pending', + snapshotVersion: 1, + activeGroupId: 'group-1', + activeTabId: `tab-pending::${leafId}`, + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: `tab-pending::${leafId}`, + parentTabId: 'tab-pending', + leafId, + title: 'Terminal', + isActive: true + } + ] + } + ] + }) + await vi.advanceTimersByTimeAsync(1_000) + await create + + expect(spawn).toHaveBeenCalledWith( + expect.objectContaining({ tabId: 'tab-pending', shellOverride: 'cmd.exe' }) + ) + } finally { + vi.useRealTimers() + } + }) + }) + + it('refuses a Windows shell on a POSIX host before any terminal exists', async () => { + await onHostPlatform('linux', async () => { + const { runtime, spawn } = makeHeadlessRuntime() + + await expect( + runtime.createMobileSessionTerminal(`id:${TEST_WORKTREE_ID}`, { shellOverride: 'cmd.exe' }) + ).rejects.toThrow(/names a Windows shell/) + expect(spawn).not.toHaveBeenCalled() + }) + }) + + it('refuses a shell pick for a workspace that runs over SSH', async () => { + await onWindowsHost(async () => { + const remoteRepo = { ...store.getRepo(TEST_REPO_ID)!, connectionId: 'ssh-1' } + const { runtime, spawn } = makeHeadlessRuntime({ + ...store, + getRepos: () => [remoteRepo], + getRepo: (id: string) => (id === TEST_REPO_ID ? remoteRepo : undefined) + }) + + await expect( + runtime.createMobileSessionTerminal(`id:${TEST_WORKTREE_ID}`, { shellOverride: 'cmd.exe' }) + ).rejects.toThrow(/over SSH/) + expect(spawn).not.toHaveBeenCalled() + }) + }) +}) diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 14d01fac31d..efefe113899 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -86,6 +86,7 @@ await import('./orca-runtime-tests/mobile-session-tabs-part-13.spec') await import('./orca-runtime-tests/mobile-session-tabs-part-14.spec') await import('./orca-runtime-tests/mobile-session-tabs-cold-serve-hydrate.spec') await import('./orca-runtime-tests/mobile-session-tabs-empty-headless-snapshot.spec') +await import('./orca-runtime-tests/mobile-session-tabs-shell-selection.spec') await import('./orca-runtime-tests/exit-retirement-activation.spec') await import('./orca-runtime-tests/mobile-creation-and-orchestration.spec') await import('./orca-runtime-tests/mobile-creation-and-orchestration-part-02.spec') diff --git a/src/main/runtime/rpc/methods/session-tabs-create-terminal-shell.test.ts b/src/main/runtime/rpc/methods/session-tabs-create-terminal-shell.test.ts new file mode 100644 index 00000000000..af4eac0bf49 --- /dev/null +++ b/src/main/runtime/rpc/methods/session-tabs-create-terminal-shell.test.ts @@ -0,0 +1,74 @@ +import '../unused-default-rpc-methods.test-fixture' +import { describe, expect, it, vi } from 'vitest' +import { RpcDispatcher } from '../dispatcher' +import type { RpcRequest } from '../core' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { RUNTIME_CAPABILITIES } from '../../../../shared/protocol-version' +import { SESSION_TABS_CREATE_TERMINAL_SHELL_RUNTIME_CAPABILITY } from '../../../../shared/session-tabs-runtime-capabilities' +import { SESSION_TAB_METHODS } from './session-tabs' + +function makeRequest(params: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method: 'session.tabs.createTerminal', params } +} + +function makeRuntime(): OrcaRuntimeService { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the handler only calls these two members. + return { + getRuntimeId: () => 'test-runtime', + createMobileSessionTerminal: vi.fn().mockResolvedValue({ + tab: { type: 'terminal', id: 'tab-1::leaf-1' }, + publicationEpoch: 'epoch-1', + snapshotVersion: 1 + }) + } as unknown as OrcaRuntimeService +} + +describe('session.tabs.createTerminal shell selection', () => { + it('advertises shell selection so clients stop typing the shell into the default one', () => { + expect(RUNTIME_CAPABILITIES).toContain(SESSION_TABS_CREATE_TERMINAL_SHELL_RUNTIME_CAPABILITY) + }) + + it('creates the requested shell instead of a startup command', async () => { + const runtime = makeRuntime() + const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) + + const response = await dispatcher.dispatch(makeRequest({ worktree: 'id:wt-1', shell: 'Cmd' })) + + expect(response.ok).toBe(true) + const opts = vi.mocked(runtime.createMobileSessionTerminal).mock.calls[0]?.[1] + expect(opts).toMatchObject({ shellOverride: 'cmd.exe' }) + expect(opts?.command).toBeUndefined() + }) + + it('omits the override when no shell is requested', async () => { + const runtime = makeRuntime() + const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) + + await dispatcher.dispatch(makeRequest({ worktree: 'id:wt-1' })) + + const opts = vi.mocked(runtime.createMobileSessionTerminal).mock.calls[0]?.[1] + expect(opts).not.toHaveProperty('shellOverride') + }) + + it('refuses a shell pick combined with an agent launch, whose plan picks its own shell', async () => { + const runtime = makeRuntime() + const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) + + const response = await dispatcher.dispatch( + makeRequest({ worktree: 'id:wt-1', shell: 'cmd.exe', agent: 'claude' }) + ) + + expect(response.ok).toBe(false) + expect(runtime.createMobileSessionTerminal).not.toHaveBeenCalled() + }) + + it('refuses a shell outside the Windows allowlist before creating a terminal', async () => { + const runtime = makeRuntime() + const dispatcher = new RpcDispatcher({ runtime, methods: SESSION_TAB_METHODS }) + + const response = await dispatcher.dispatch(makeRequest({ worktree: 'id:wt-1', shell: 'zsh' })) + + expect(response.ok).toBe(false) + expect(runtime.createMobileSessionTerminal).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/runtime/rpc/methods/session-tabs.ts b/src/main/runtime/rpc/methods/session-tabs.ts index 2b2e0e7e0da..299abd48888 100644 --- a/src/main/runtime/rpc/methods/session-tabs.ts +++ b/src/main/runtime/rpc/methods/session-tabs.ts @@ -66,6 +66,7 @@ export const SESSION_TAB_METHODS = [ afterTabId: params.afterTabId, targetGroupId: params.targetGroupId, command: params.command, + ...(params.shell ? { shellOverride: params.shell } : {}), cwd: params.cwd, ...(params.env ? { env: params.env } : {}), ...(params.envToDelete ? { envToDelete: params.envToDelete } : {}), diff --git a/src/main/runtime/terminal/launch/mobile-terminal-create.ts b/src/main/runtime/terminal/launch/mobile-terminal-create.ts index 537c2e03cb4..0d4ae1a749a 100644 --- a/src/main/runtime/terminal/launch/mobile-terminal-create.ts +++ b/src/main/runtime/terminal/launch/mobile-terminal-create.ts @@ -32,6 +32,7 @@ export abstract class OrcaRuntimeWithCreateMobileSessionTerminal extends OrcaRun afterTabId?: string targetGroupId?: string command?: string + shellOverride?: string cwd?: string env?: Record envToDelete?: string[] diff --git a/src/main/runtime/terminal/launch/mobile-terminal-owned-create.ts b/src/main/runtime/terminal/launch/mobile-terminal-owned-create.ts index 43e5d7b6e0b..24e72e24620 100644 --- a/src/main/runtime/terminal/launch/mobile-terminal-owned-create.ts +++ b/src/main/runtime/terminal/launch/mobile-terminal-owned-create.ts @@ -27,6 +27,7 @@ const OWNED_MOBILE_DISPATCH_RECIPE_TTL_MS = 15 * 60_000 export type OwnedMobileTerminalCreateOptions = { command?: string + shellOverride?: string cwd?: string env?: Record envToDelete?: string[] @@ -114,6 +115,7 @@ export abstract class OrcaRuntimeWithCreateRuntimeOwnedMobileSessionTerminal ext : {}), ...(preAllocatedHandle ? { preAllocatedHandle } : {}), command: opts.command, + ...(opts.shellOverride ? { shellOverride: opts.shellOverride } : {}), cwd, env: opts.env, envToDelete: opts.envToDelete, diff --git a/src/main/runtime/terminal/launch/mobile-terminal-renderer-create.ts b/src/main/runtime/terminal/launch/mobile-terminal-renderer-create.ts index 389a8e2858d..f8562c8cc89 100644 --- a/src/main/runtime/terminal/launch/mobile-terminal-renderer-create.ts +++ b/src/main/runtime/terminal/launch/mobile-terminal-renderer-create.ts @@ -18,12 +18,14 @@ import { } from '../../orca-runtime-core' import { rendererPublicationThrottle } from '../../../window/renderer-publication-throttle' import { deterministicAgentSessionUuid } from '../../runtime-agent-launch-resolution' +import { assertMobileTerminalShellSupported } from './mobile-terminal-shell-refusal' export type MobileTerminalCreateOptions = { terminalImageViewerSupported?: boolean afterTabId?: string targetGroupId?: string command?: string + shellOverride?: string cwd?: string env?: Record envToDelete?: string[] @@ -99,6 +101,7 @@ export abstract class OrcaRuntimeWithRunCreateMobileSessionTerminal extends Orca const workspace = await this.resolveTerminalWorkspaceLaunchScope(worktreeSelector) const worktreeId = workspace.id const cwd = this.resolveWorkspaceTerminalStartupCwd(workspace, opts.cwd) + assertMobileTerminalShellSupported(this.store, workspace, opts.shellOverride, cwd) this.hydrateHeadlessMobileSessionTabsFromWorkspaceSession(worktreeId) // Older mobile clients append their optimistic tab, so make the host append too until the // client advertises the grouped placement contract. @@ -157,6 +160,7 @@ export abstract class OrcaRuntimeWithRunCreateMobileSessionTerminal extends Orca afterTabId, { command: startupCommand.command, + ...(opts.shellOverride ? { shellOverride: opts.shellOverride } : {}), cwd, env: startupCommand.env, envToDelete: startupCommand.envToDelete, @@ -217,6 +221,7 @@ export abstract class OrcaRuntimeWithRunCreateMobileSessionTerminal extends Orca afterTabId: afterDesktopTabId, targetGroupId: opts.targetGroupId, command: startupCommand.command, + ...(opts.shellOverride ? { shellOverride: opts.shellOverride } : {}), cwd, ...(startupCommand.env ? { env: startupCommand.env } : {}), ...(startupCommand.envToDelete ? { envToDelete: startupCommand.envToDelete } : {}), @@ -283,6 +288,7 @@ export abstract class OrcaRuntimeWithRunCreateMobileSessionTerminal extends Orca afterTabId, { command: startupCommand.command, + ...(opts.shellOverride ? { shellOverride: opts.shellOverride } : {}), cwd, env: startupCommand.env, envToDelete: startupCommand.envToDelete, diff --git a/src/main/runtime/terminal/launch/mobile-terminal-shell-refusal.test.ts b/src/main/runtime/terminal/launch/mobile-terminal-shell-refusal.test.ts new file mode 100644 index 00000000000..95a59b85e4e --- /dev/null +++ b/src/main/runtime/terminal/launch/mobile-terminal-shell-refusal.test.ts @@ -0,0 +1,63 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { TerminalWorkspaceLaunchScope } from '../../runtime-legacy-worker-terminal-recovery-types' + +const resolveLocalLaunchScopeRuntime = vi.hoisted(() => vi.fn()) +vi.mock('../../../local-project-runtime-resolution', () => ({ resolveLocalLaunchScopeRuntime })) + +const { assertMobileTerminalShellSupported } = await import('./mobile-terminal-shell-refusal') + +const folderScope: TerminalWorkspaceLaunchScope = { + id: 'folder:fw-1', + path: 'C:\\work\\folder', + connectionId: null, + repo: null, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: only the id is read through the mocked resolver. + folderWorkspace: { id: 'fw-1' } as TerminalWorkspaceLaunchScope['folderWorkspace'] +} + +describe('assertMobileTerminalShellSupported', () => { + const hostPlatform = Object.getOwnPropertyDescriptor(process, 'platform')! + + beforeEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + resolveLocalLaunchScopeRuntime.mockReset() + }) + + afterEach(() => { + Object.defineProperty(process, 'platform', hostPlatform) + }) + + it('refuses a Windows shell for a folder workspace whose project runs in WSL', () => { + resolveLocalLaunchScopeRuntime.mockReturnValue({ + status: 'resolved', + runtime: { + kind: 'wsl', + hostPlatform: 'wsl', + projectId: 'p-1', + distro: 'Ubuntu', + reason: 'project-override', + cacheKey: 'k' + } + }) + const store = {} + + expect(() => + assertMobileTerminalShellSupported(store, folderScope, 'cmd.exe', undefined) + ).toThrow(/in WSL/) + expect(resolveLocalLaunchScopeRuntime).toHaveBeenCalledWith(store, folderScope) + }) + + it('accepts the shell when the folder runs on the Windows host', () => { + resolveLocalLaunchScopeRuntime.mockReturnValue(undefined) + + expect(() => + assertMobileTerminalShellSupported({}, folderScope, 'cmd.exe', undefined) + ).not.toThrow() + }) + + it('skips resolution when no shell was picked', () => { + assertMobileTerminalShellSupported({}, folderScope, undefined, undefined) + + expect(resolveLocalLaunchScopeRuntime).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/runtime/terminal/launch/mobile-terminal-shell-refusal.ts b/src/main/runtime/terminal/launch/mobile-terminal-shell-refusal.ts new file mode 100644 index 00000000000..ece59892a3a --- /dev/null +++ b/src/main/runtime/terminal/launch/mobile-terminal-shell-refusal.ts @@ -0,0 +1,30 @@ +import { + resolveLocalLaunchScopeRuntime, + type LaunchScopeRuntimeStore +} from '../../../local-project-runtime-resolution' +import { terminalShellOverrideRefusal } from '../../terminal-shell-override-host-support' +import type { TerminalWorkspaceLaunchScope } from '../../runtime-legacy-worker-terminal-recovery-types' + +/** Throws before any surface exists when this host cannot run the requested shell. */ +export function assertMobileTerminalShellSupported( + store: LaunchScopeRuntimeStore | null | undefined, + workspace: TerminalWorkspaceLaunchScope, + shellOverride: string | undefined, + cwd: string | undefined +): void { + if (!shellOverride) { + return + } + const refusal = terminalShellOverrideRefusal({ + shellOverride, + connectionId: workspace.connectionId, + platform: process.platform, + // Why the launch scope: a folder workspace's runtime comes from its project, not a repo id. + projectRuntime: store ? resolveLocalLaunchScopeRuntime(store, workspace) : undefined, + cwd: cwd ?? workspace.path, + workspacePath: workspace.path + }) + if (refusal) { + throw refusal + } +} diff --git a/src/renderer/src/components/tab-group/useTabGroupCreationCommands.ts b/src/renderer/src/components/tab-group/useTabGroupCreationCommands.ts index 674212a524d..3f5508e66ab 100644 --- a/src/renderer/src/components/tab-group/useTabGroupCreationCommands.ts +++ b/src/renderer/src/components/tab-group/useTabGroupCreationCommands.ts @@ -118,7 +118,7 @@ export function useTabGroupCreationCommands({ worktreeId, environmentId, targetGroupId: groupId, - command: shellOverride, + shell: shellOverride, activate: true }) if (outcome.status === 'created' || isWebRuntimeSessionActive(environmentId)) { diff --git a/src/renderer/src/components/terminal/terminal-tab-actions.test.ts b/src/renderer/src/components/terminal/terminal-tab-actions.test.ts index 502e2779c06..6c1be4b2c40 100644 --- a/src/renderer/src/components/terminal/terminal-tab-actions.test.ts +++ b/src/renderer/src/components/terminal/terminal-tab-actions.test.ts @@ -134,7 +134,7 @@ describe('createNewTerminalTab', () => { expect(createWebRuntimeSessionTerminalMock).toHaveBeenCalledWith({ worktreeId: 'wt-1', environmentId: 'web-runtime', - command: 'pwsh', + shell: 'pwsh', activate: true }) expect(createTab).not.toHaveBeenCalled() @@ -158,7 +158,7 @@ describe('createNewTerminalTab', () => { expect(createWebRuntimeSessionTerminalMock).toHaveBeenCalledWith({ worktreeId: 'wt-1', environmentId: 'owner-runtime', - command: 'pwsh', + shell: 'pwsh', activate: true }) expect(createTab).not.toHaveBeenCalled() diff --git a/src/renderer/src/components/terminal/terminal-tab-create.ts b/src/renderer/src/components/terminal/terminal-tab-create.ts index bcfeac9fcd5..f280e58a7a0 100644 --- a/src/renderer/src/components/terminal/terminal-tab-create.ts +++ b/src/renderer/src/components/terminal/terminal-tab-create.ts @@ -26,7 +26,7 @@ export function createNewTerminalTab( void createWebRuntimeSessionTerminal({ worktreeId: activeWorktreeId, environmentId: runtimeEnvironmentId, - command: shellOverride, + shell: shellOverride, ...(options?.startupCwd ? { cwd: options.startupCwd } : {}), activate: true }) diff --git a/src/renderer/src/components/use-terminal-create-actions.ts b/src/renderer/src/components/use-terminal-create-actions.ts index b2b089336d2..8b76f1abc7e 100644 --- a/src/renderer/src/components/use-terminal-create-actions.ts +++ b/src/renderer/src/components/use-terminal-create-actions.ts @@ -48,7 +48,7 @@ export function useTerminalCreateActions(controller: TerminalColdActivationContr worktreeId: activeWorktreeId, environmentId: runtimeEnvironmentId, targetGroupId, - command: shellOverride, + shell: shellOverride, activate: true }) return diff --git a/src/renderer/src/runtime/web-runtime-plain-terminal-create-params.test.ts b/src/renderer/src/runtime/web-runtime-plain-terminal-create-params.test.ts new file mode 100644 index 00000000000..29c52ec9c07 --- /dev/null +++ b/src/renderer/src/runtime/web-runtime-plain-terminal-create-params.test.ts @@ -0,0 +1,55 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { SESSION_TABS_CREATE_TERMINAL_SHELL_RUNTIME_CAPABILITY } from '../../../shared/session-tabs-runtime-capabilities' + +const supportsCapability = vi.hoisted(() => vi.fn()) +vi.mock('./runtime-rpc-client', () => ({ + runtimeEnvironmentSupportsCapability: supportsCapability +})) + +const { buildPlainCreateTerminalParams, resolveCreateTerminalShellParams } = + await import('./web-runtime-plain-terminal-create-params') + +describe('resolveCreateTerminalShellParams', () => { + beforeEach(() => { + supportsCapability.mockReset() + }) + + it('sends the pick as the shell on hosts that advertise shell selection', async () => { + supportsCapability.mockResolvedValue(true) + + await expect(resolveCreateTerminalShellParams('env-1', { shell: 'git-bash' })).resolves.toEqual( + { command: undefined, shell: 'git-bash' } + ) + expect(supportsCapability).toHaveBeenCalledWith( + 'env-1', + SESSION_TABS_CREATE_TERMINAL_SHELL_RUNTIME_CAPABILITY + ) + }) + + it('falls back to the typed command for hosts that would drop the shell', async () => { + supportsCapability.mockResolvedValue(false) + + await expect(resolveCreateTerminalShellParams('env-1', { shell: 'cmd.exe' })).resolves.toEqual({ + command: 'cmd.exe' + }) + }) + + it('does not probe the host when no shell was picked', async () => { + await expect(resolveCreateTerminalShellParams('env-1', { command: 'ls' })).resolves.toEqual({ + command: 'ls' + }) + expect(supportsCapability).not.toHaveBeenCalled() + }) + + it('builds caller-selected plain terminal params with the shell pick', async () => { + supportsCapability.mockResolvedValue(true) + + const params = await buildPlainCreateTerminalParams('env-1', { + worktreeId: 'repo-1::/repo', + shell: 'cmd.exe' + }) + + expect(params).toMatchObject({ shell: 'cmd.exe', select: true, navigation: 'caller' }) + expect(params.command).toBeUndefined() + }) +}) diff --git a/src/renderer/src/runtime/web-runtime-plain-terminal-create-params.ts b/src/renderer/src/runtime/web-runtime-plain-terminal-create-params.ts new file mode 100644 index 00000000000..b526bba5558 --- /dev/null +++ b/src/renderer/src/runtime/web-runtime-plain-terminal-create-params.ts @@ -0,0 +1,51 @@ +import { SESSION_TABS_CREATE_TERMINAL_SHELL_RUNTIME_CAPABILITY } from '../../../shared/session-tabs-runtime-capabilities' +import { toHostSessionTabId } from '../../../shared/terminal-surface-id' +import { runtimeEnvironmentSupportsCapability } from './runtime-rpc-client' +import { toRuntimeWorktreeSelector } from './runtime-worktree-selector' +import type { CreateWebRuntimeSessionTerminalArgs } from './web-runtime-session-types' + +export type CreateTerminalShellParams = { command?: string; shell?: string } + +/** How a session.tabs.createTerminal call carries the user's shell pick to this host. */ +export async function resolveCreateTerminalShellParams( + environmentId: string, + args: { command?: string; shell?: string } +): Promise { + if (!args.shell) { + return { command: args.command } + } + if ( + await runtimeEnvironmentSupportsCapability( + environmentId, + SESSION_TABS_CREATE_TERMINAL_SHELL_RUNTIME_CAPABILITY + ) + ) { + return { command: args.command, shell: args.shell } + } + // Why: an older host drops `shell`; typing the pick still reaches it, nested in the default shell. + return { command: args.command ?? args.shell } +} + +/** session.tabs.createTerminal params for a plain (non-agent) terminal. */ +export async function buildPlainCreateTerminalParams( + environmentId: string, + args: CreateWebRuntimeSessionTerminalArgs +): Promise> { + return { + worktree: toRuntimeWorktreeSelector(args.worktreeId), + afterTabId: args.afterTabId ? toHostSessionTabId(args.afterTabId) : undefined, + targetGroupId: args.targetGroupId, + ...(await resolveCreateTerminalShellParams(environmentId, args)), + cwd: args.cwd, + ...(args.env ? { env: args.env } : {}), + ...(args.envToDelete ? { envToDelete: args.envToDelete } : {}), + startupCommandDelivery: args.startupCommandDelivery, + ...(args.launchConfig ? { launchConfig: args.launchConfig } : {}), + ...(args.launchToken ? { launchToken: args.launchToken } : {}), + ...(args.viewMode ? { viewMode: args.viewMode } : {}), + // Why: old hosts understand activate:false; new hosts use select/navigation for caller-local focus. + activate: false, + select: args.activate !== false, + navigation: 'caller' + } +} diff --git a/src/renderer/src/runtime/web-runtime-session-types.ts b/src/renderer/src/runtime/web-runtime-session-types.ts index 8664c9192ce..97fa9d4aa4d 100644 --- a/src/renderer/src/runtime/web-runtime-session-types.ts +++ b/src/renderer/src/runtime/web-runtime-session-types.ts @@ -20,6 +20,8 @@ export type CreateWebRuntimeSessionTerminalArgs = { afterTabId?: string targetGroupId?: string command?: string + /** The Windows shell the terminal runs as, never typed into the host default shell. */ + shell?: string cwd?: string env?: Record envToDelete?: string[] diff --git a/src/renderer/src/runtime/web-runtime-terminal-create-operation.ts b/src/renderer/src/runtime/web-runtime-terminal-create-operation.ts index bd46cc8eff1..fefa66f4f66 100644 --- a/src/renderer/src/runtime/web-runtime-terminal-create-operation.ts +++ b/src/renderer/src/runtime/web-runtime-terminal-create-operation.ts @@ -44,6 +44,7 @@ import { readCreatedAgentTerminalIdentity } from './web-runtime-terminal-identity' import { settleWebRuntimeTerminalPlacement } from './web-runtime-terminal-placement-settlement' +import { buildPlainCreateTerminalParams } from './web-runtime-plain-terminal-create-params' export async function createWebRuntimeSessionTerminalResult( args: CreateWebRuntimeSessionTerminalArgs @@ -221,23 +222,7 @@ export async function createWebRuntimeSessionTerminalResult( } else { const response = await callEnvironment({ method: 'session.tabs.createTerminal', - params: { - worktree: toRuntimeWorktreeSelector(args.worktreeId), - afterTabId: args.afterTabId ? toHostSessionTabId(args.afterTabId) : undefined, - targetGroupId: args.targetGroupId, - command: args.command, - cwd: args.cwd, - ...(args.env ? { env: args.env } : {}), - ...(args.envToDelete ? { envToDelete: args.envToDelete } : {}), - startupCommandDelivery: args.startupCommandDelivery, - ...(args.launchConfig ? { launchConfig: args.launchConfig } : {}), - ...(args.launchToken ? { launchToken: args.launchToken } : {}), - ...(args.viewMode ? { viewMode: args.viewMode } : {}), - // Why: old hosts understand activate:false; new hosts use select/navigation for caller-local focus. - activate: false, - select: args.activate !== false, - navigation: 'caller' - }, + params: await buildPlainCreateTerminalParams(environmentId, args), timeoutMs: 15_000 }) const created = unwrapRuntimeRpcResult( diff --git a/src/shared/rpc-contract/session-tabs-schemas-params.ts b/src/shared/rpc-contract/session-tabs-schemas-params.ts index 7b0c534eb45..9477838ac6b 100644 --- a/src/shared/rpc-contract/session-tabs-schemas-params.ts +++ b/src/shared/rpc-contract/session-tabs-schemas-params.ts @@ -6,6 +6,7 @@ import { sleepingAgentLaunchConfigSchema } from '../workspace-session-sleeping-a import type { TuiAgent } from '../tui-agent' import { isTuiAgent } from '../tui-agent-config' import { LEGACY_MAX_QUICK_COMMAND_AGENT_PROMPT_LENGTH } from '../terminal-quick-command-prompt-limit' +import { WindowsShellOverrideParam } from './terminal-unary-params' export const WorktreeTabSelector = z.object({ worktree: z @@ -143,6 +144,8 @@ export const CreateTerminalTab = WorktreeTabSelector.extend({ afterTabId: z.string().optional(), targetGroupId: z.string().optional(), command: z.string().optional(), + // Why: the shell the terminal IS; a shell sent as `command` runs nested in the host default. + shell: WindowsShellOverrideParam.optional(), cwd: z.string().min(1).optional(), env: z.record(z.string(), z.string()).optional(), envToDelete: z.array(z.string().min(1).max(256)).max(32).optional(), @@ -191,6 +194,14 @@ export const CreateTerminalTab = WorktreeTabSelector.extend({ message: 'Agent prompt cannot be combined with a startup command' }) } + // Why: agent launches plan their own shell, which would silently replace the pick. + if (value.shell !== undefined && (value.agent || value.launchAgent || value.launchConfig)) { + context.addIssue({ + code: 'custom', + path: ['shell'], + message: 'A shell pick cannot be combined with an agent launch' + }) + } }) export const MoveTabBase = { diff --git a/src/shared/rpc-contract/terminal-unary-params.ts b/src/shared/rpc-contract/terminal-unary-params.ts index 557a97fe68b..fff5df40254 100644 --- a/src/shared/rpc-contract/terminal-unary-params.ts +++ b/src/shared/rpc-contract/terminal-unary-params.ts @@ -11,6 +11,17 @@ import { import { TERMINAL_PANE_SPLIT_SOURCES } from '../feature-education-telemetry' import { OptionalExecutionHostId } from './worktree-params' +// 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. +export const WindowsShellOverrideParam = 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) + export const TerminalHandle = z.object({ terminal: requiredString('Missing terminal handle'), // Additive fence understood by newer hosts; legacy hosts safely ignore it. @@ -216,17 +227,7 @@ export const TerminalCreateParams = z.object({ tabId: OptionalString, leafId: OptionalString, spawnIntent: SpawnIntentParam.optional(), - // 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() + shell: WindowsShellOverrideParam.optional() }) export const TerminalSplit = TerminalHandle.extend({ diff --git a/src/shared/session-tabs-runtime-capabilities.ts b/src/shared/session-tabs-runtime-capabilities.ts index af34cfa4a0a..54d5472f31c 100644 --- a/src/shared/session-tabs-runtime-capabilities.ts +++ b/src/shared/session-tabs-runtime-capabilities.ts @@ -7,7 +7,13 @@ export const SESSION_TABS_SPLIT_GROUP_PLACEMENT_RUNTIME_CAPABILITY = export const SESSION_TABS_EMPTY_WORKTREE_ANSWER_RUNTIME_CAPABILITY = 'session-tabs.empty-worktree-answer.v1' as const +// Why: an older host strips session.tabs.createTerminal's unknown `shell` and spawns its default +// shell, so a client sends the pick there only when advertised. +export const SESSION_TABS_CREATE_TERMINAL_SHELL_RUNTIME_CAPABILITY = + 'session-tabs.create-terminal-shell.v1' as const + export const SESSION_TABS_RUNTIME_CAPABILITIES = [ SESSION_TABS_SPLIT_GROUP_PLACEMENT_RUNTIME_CAPABILITY, - SESSION_TABS_EMPTY_WORKTREE_ANSWER_RUNTIME_CAPABILITY + SESSION_TABS_EMPTY_WORKTREE_ANSWER_RUNTIME_CAPABILITY, + SESSION_TABS_CREATE_TERMINAL_SHELL_RUNTIME_CAPABILITY ] as const