From 7dc83a522e99902f98ac8cc7c4cfbbdebe47b556 Mon Sep 17 00:00:00 2001 From: John Richmond <5629+jr@users.noreply.github.com> Date: Tue, 3 Mar 2026 23:31:34 -0800 Subject: [PATCH] Allow selecting a specific shell (#11851) * Allow selecting a specific shell Add --terminal-shell CLI flag to specify which shell ExecaTerminalProcess uses for inline command execution. The shell path is validated at the CLI layer and passed through the standard settings mechanism (BaseTerminal static getter/setter), matching how all other CLI terminal settings flow through the system. * test(cli): make shell path access test cross-platform --- apps/cli/README.md | 1 + .../agent/__tests__/extension-host.test.ts | 36 +++++++++++++ apps/cli/src/agent/extension-host.ts | 6 +++ apps/cli/src/commands/cli/run.ts | 15 ++++++ apps/cli/src/index.ts | 1 + .../cli/src/lib/utils/__tests__/shell.test.ts | 54 +++++++++++++++++++ apps/cli/src/lib/utils/shell.ts | 47 ++++++++++++++++ apps/cli/src/types/types.ts | 1 + packages/types/src/global-settings.ts | 1 + packages/types/src/vscode-extension-host.ts | 1 + src/core/webview/webviewMessageHandler.ts | 2 + src/integrations/terminal/BaseTerminal.ts | 9 ++++ .../terminal/ExecaTerminalProcess.ts | 3 +- .../__tests__/ExecaTerminalProcess.spec.ts | 24 +++++++++ 14 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 apps/cli/src/lib/utils/__tests__/shell.test.ts create mode 100644 apps/cli/src/lib/utils/shell.ts diff --git a/apps/cli/README.md b/apps/cli/README.md index 7f7f54f918..d1fc3b2f5e 100644 --- a/apps/cli/README.md +++ b/apps/cli/README.md @@ -184,6 +184,7 @@ Tokens are valid for 90 days. The CLI will prompt you to re-authenticate when yo | `--provider ` | API provider (roo, anthropic, openai, openrouter, etc.) | `openrouter` (or `roo` if authenticated) | | `-m, --model ` | Model to use | `anthropic/claude-opus-4.6` | | `--mode ` | Mode to start in (code, architect, ask, debug, etc.) | `code` | +| `--terminal-shell ` | Absolute shell path for inline terminal command execution | Auto-detected shell | | `-r, --reasoning-effort ` | Reasoning effort level (unspecified, disabled, none, minimal, low, medium, high, xhigh) | `medium` | | `--consecutive-mistake-limit ` | Consecutive error/repetition limit before guidance prompt (`0` disables the limit) | `10` | | `--ephemeral` | Run without persisting state (uses temporary storage) | `false` | diff --git a/apps/cli/src/agent/__tests__/extension-host.test.ts b/apps/cli/src/agent/__tests__/extension-host.test.ts index b9093bef0d..a0f68286e6 100644 --- a/apps/cli/src/agent/__tests__/extension-host.test.ts +++ b/apps/cli/src/agent/__tests__/extension-host.test.ts @@ -158,6 +158,22 @@ describe("ExtensionHost", () => { createTestHost() expect(process.env.ROO_CLI_RUNTIME).toBe("1") }) + + it("should set execaShellPath in initialSettings when terminalShell is provided", () => { + const host = createTestHost({ terminalShell: "/bin/bash" }) + const emitSpy = vi.spyOn(host, "emit") + host.markWebviewReady() + const updateSettingsCall = emitSpy.mock.calls.find( + (call) => + call[0] === "webviewMessage" && + typeof call[1] === "object" && + call[1] !== null && + (call[1] as WebviewMessage).type === "updateSettings", + ) + expect(updateSettingsCall).toBeDefined() + const payload = updateSettingsCall?.[1] as WebviewMessage + expect(payload.updatedSettings?.execaShellPath).toBe("/bin/bash") + }) }) describe("webview provider registration", () => { @@ -238,6 +254,26 @@ describe("ExtensionHost", () => { ) expect(updateSettingsCall).toBeDefined() }) + + it("should force terminalShellIntegrationDisabled when terminalShell is provided", () => { + const host = createTestHost({ terminalShell: "/bin/bash" }) + const emitSpy = vi.spyOn(host, "emit") + + host.markWebviewReady() + + const updateSettingsCall = emitSpy.mock.calls.find( + (call) => + call[0] === "webviewMessage" && + typeof call[1] === "object" && + call[1] !== null && + (call[1] as WebviewMessage).type === "updateSettings", + ) + + expect(updateSettingsCall).toBeDefined() + const payload = updateSettingsCall?.[1] as WebviewMessage + expect(payload.type).toBe("updateSettings") + expect(payload.updatedSettings?.terminalShellIntegrationDisabled).toBe(true) + }) }) }) diff --git a/apps/cli/src/agent/extension-host.ts b/apps/cli/src/agent/extension-host.ts index 7e0eb07a55..393990301f 100644 --- a/apps/cli/src/agent/extension-host.ts +++ b/apps/cli/src/agent/extension-host.ts @@ -80,6 +80,7 @@ export interface ExtensionHostOptions { ephemeral: boolean debug: boolean exitOnComplete: boolean + terminalShell?: string /** * When true, exit the process on API request errors instead of retrying. */ @@ -257,6 +258,11 @@ export class ExtensionHost extends EventEmitter implements ExtensionHostInterfac this.initialSettings.reasoningEffort = this.options.reasoningEffort } } + + if (this.options.terminalShell) { + this.initialSettings.terminalShellIntegrationDisabled = true + this.initialSettings.execaShellPath = this.options.terminalShell + } } // ========================================================================== diff --git a/apps/cli/src/commands/cli/run.ts b/apps/cli/src/commands/cli/run.ts index 229fddb140..2e72359927 100644 --- a/apps/cli/src/commands/cli/run.ts +++ b/apps/cli/src/commands/cli/run.ts @@ -26,6 +26,7 @@ import { readWorkspaceTaskSessions, resolveWorkspaceResumeSessionId } from "@/li import { isRecord } from "@/lib/utils/guards.js" import { getEnvVarName, getApiKeyFromEnv } from "@/lib/utils/provider.js" import { runOnboarding } from "@/lib/utils/onboarding.js" +import { validateTerminalShellPath } from "@/lib/utils/shell.js" import { getDefaultExtensionPath } from "@/lib/utils/extension.js" import { VERSION } from "@/lib/utils/version.js" @@ -176,6 +177,19 @@ export async function run(promptArg: string | undefined, flagOptions: FlagOption process.exit(1) } + let terminalShell: string | undefined + if (flagOptions.terminalShell !== undefined) { + const validatedTerminalShell = await validateTerminalShellPath(flagOptions.terminalShell) + + if (!validatedTerminalShell.valid) { + console.error( + `[CLI] Warning: ignoring --terminal-shell "${flagOptions.terminalShell}" (${validatedTerminalShell.reason})`, + ) + } else { + terminalShell = validatedTerminalShell.shellPath + } + } + const extensionHostOptions: ExtensionHostOptions = { mode: effectiveMode, reasoningEffort: effectiveReasoningEffort === "unspecified" ? undefined : effectiveReasoningEffort, @@ -190,6 +204,7 @@ export async function run(promptArg: string | undefined, flagOptions: FlagOption ephemeral: flagOptions.ephemeral, debug: flagOptions.debug, exitOnComplete: effectiveExitOnComplete, + terminalShell, } // Roo Code Cloud Authentication diff --git a/apps/cli/src/index.ts b/apps/cli/src/index.ts index de614737d5..68bbd29229 100644 --- a/apps/cli/src/index.ts +++ b/apps/cli/src/index.ts @@ -47,6 +47,7 @@ program .option("--provider ", "API provider (roo, anthropic, openai, openrouter, etc.)") .option("-m, --model ", "Model to use", DEFAULT_FLAGS.model) .option("--mode ", "Mode to start in (code, architect, ask, debug, etc.)", DEFAULT_FLAGS.mode) + .option("--terminal-shell ", "Absolute path to shell executable for inline terminal commands") .option( "-r, --reasoning-effort ", "Reasoning effort level (unspecified, disabled, none, minimal, low, medium, high, xhigh)", diff --git a/apps/cli/src/lib/utils/__tests__/shell.test.ts b/apps/cli/src/lib/utils/__tests__/shell.test.ts new file mode 100644 index 0000000000..7e94131c3b --- /dev/null +++ b/apps/cli/src/lib/utils/__tests__/shell.test.ts @@ -0,0 +1,54 @@ +import fs from "fs/promises" + +import { validateTerminalShellPath } from "../shell.js" + +vi.mock("fs/promises", () => ({ + default: { + access: vi.fn(), + stat: vi.fn(), + }, +})) + +describe("validateTerminalShellPath", () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(fs.access).mockResolvedValue(undefined) + vi.mocked(fs.stat).mockResolvedValue({ + isFile: () => true, + } as unknown as Awaited>) + }) + + it("returns invalid for an empty path", async () => { + const result = await validateTerminalShellPath(" ") + expect(result).toEqual({ valid: false, reason: "shell path cannot be empty" }) + }) + + it("returns invalid for a relative path", async () => { + const result = await validateTerminalShellPath("bin/bash") + expect(result).toEqual({ valid: false, reason: "shell path must be absolute" }) + }) + + it("returns valid for an absolute executable path", async () => { + const result = await validateTerminalShellPath("/bin/bash") + expect(result).toEqual({ valid: true, shellPath: "/bin/bash" }) + }) + + it("returns invalid when the shell path cannot be accessed", async () => { + vi.mocked(fs.stat).mockRejectedValueOnce(new Error("ENOENT")) + const result = await validateTerminalShellPath("/missing/shell") + + expect(result.valid).toBe(false) + if (!result.valid) { + expect(result.reason).toContain("shell path") + } + }) + + it("returns invalid when the shell path points to a directory", async () => { + vi.mocked(fs.stat).mockResolvedValueOnce({ + isFile: () => false, + } as unknown as Awaited>) + const result = await validateTerminalShellPath("/bin") + + expect(result).toEqual({ valid: false, reason: "shell path must point to a file" }) + }) +}) diff --git a/apps/cli/src/lib/utils/shell.ts b/apps/cli/src/lib/utils/shell.ts new file mode 100644 index 0000000000..548df919b2 --- /dev/null +++ b/apps/cli/src/lib/utils/shell.ts @@ -0,0 +1,47 @@ +import fs from "fs/promises" +import { constants as fsConstants } from "fs" +import path from "path" + +export type TerminalShellValidationResult = + | { + valid: true + shellPath: string + } + | { + valid: false + reason: string + } + +export async function validateTerminalShellPath(rawShellPath: string): Promise { + const shellPath = rawShellPath.trim() + + if (!shellPath) { + return { valid: false, reason: "shell path cannot be empty" } + } + + if (!path.isAbsolute(shellPath)) { + return { valid: false, reason: "shell path must be absolute" } + } + + try { + const stats = await fs.stat(shellPath) + + if (!stats.isFile()) { + return { valid: false, reason: "shell path must point to a file" } + } + + if (process.platform !== "win32") { + await fs.access(shellPath, fsConstants.X_OK) + } + } catch { + return { + valid: false, + reason: + process.platform === "win32" + ? "shell path does not exist or is not a file" + : "shell path does not exist, is not a file, or is not executable", + } + } + + return { valid: true, shellPath } +} diff --git a/apps/cli/src/types/types.ts b/apps/cli/src/types/types.ts index 50a8bc9c45..2759b4f113 100644 --- a/apps/cli/src/types/types.ts +++ b/apps/cli/src/types/types.ts @@ -34,6 +34,7 @@ export type FlagOptions = { provider?: SupportedProvider model?: string mode?: string + terminalShell?: string reasoningEffort?: ReasoningEffortFlagOptions consecutiveMistakeLimit?: number ephemeral: boolean diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index 91b37f3d6d..288f6c2118 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -176,6 +176,7 @@ export const globalSettingsSchema = z.object({ terminalZshOhMy: z.boolean().optional(), terminalZshP10k: z.boolean().optional(), terminalZdotdir: z.boolean().optional(), + execaShellPath: z.string().optional(), diagnosticsEnabled: z.boolean().optional(), diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index ab06f0dabb..b20539afe4 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -282,6 +282,7 @@ export type ExtensionState = Pick< | "terminalZshOhMy" | "terminalZshP10k" | "terminalZdotdir" + | "execaShellPath" | "diagnosticsEnabled" | "language" | "modeApiConfigs" diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index f1be1316d2..d27fd6bec0 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -718,6 +718,8 @@ export const webviewMessageHandler = async ( if (value !== undefined) { Terminal.setTerminalZdotdir(value as boolean) } + } else if (key === "execaShellPath") { + Terminal.setExecaShellPath(value as string | undefined) } else if (key === "mcpEnabled") { newValue = value ?? true const mcpHub = provider.getMcpHub() diff --git a/src/integrations/terminal/BaseTerminal.ts b/src/integrations/terminal/BaseTerminal.ts index 121dc34313..ee26254934 100644 --- a/src/integrations/terminal/BaseTerminal.ts +++ b/src/integrations/terminal/BaseTerminal.ts @@ -161,6 +161,7 @@ export abstract class BaseTerminal implements RooTerminal { private static terminalZshOhMy: boolean = false private static terminalZshP10k: boolean = false private static terminalZdotdir: boolean = false + private static execaShellPath: string | undefined = undefined /** * Compresses terminal output by applying run-length encoding and truncating to line limit @@ -294,4 +295,12 @@ export abstract class BaseTerminal implements RooTerminal { public static getTerminalZdotdir(): boolean { return BaseTerminal.terminalZdotdir } + + public static setExecaShellPath(shellPath: string | undefined): void { + BaseTerminal.execaShellPath = shellPath + } + + public static getExecaShellPath(): string | undefined { + return BaseTerminal.execaShellPath + } } diff --git a/src/integrations/terminal/ExecaTerminalProcess.ts b/src/integrations/terminal/ExecaTerminalProcess.ts index 370bf0d377..cc2af93802 100644 --- a/src/integrations/terminal/ExecaTerminalProcess.ts +++ b/src/integrations/terminal/ExecaTerminalProcess.ts @@ -3,6 +3,7 @@ import psTree from "ps-tree" import process from "process" import type { RooTerminal } from "./types" +import { BaseTerminal } from "./BaseTerminal" import { BaseTerminalProcess } from "./BaseTerminalProcess" export class ExecaTerminalProcess extends BaseTerminalProcess { @@ -39,7 +40,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess { this.isHot = true this.subprocess = execa({ - shell: true, + shell: BaseTerminal.getExecaShellPath() || true, cwd: this.terminal.getCurrentWorkingDirectory(), all: true, // Ignore stdin to ensure non-interactive mode and prevent hanging diff --git a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index c87ee5ad05..5f0a21869e 100644 --- a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts @@ -23,6 +23,7 @@ vitest.mock("ps-tree", () => ({ import { execa } from "execa" import { ExecaTerminalProcess } from "../ExecaTerminalProcess" +import { BaseTerminal } from "../BaseTerminal" import type { RooTerminal } from "../types" describe("ExecaTerminalProcess", () => { @@ -32,6 +33,7 @@ describe("ExecaTerminalProcess", () => { beforeEach(() => { originalEnv = { ...process.env } + BaseTerminal.setExecaShellPath(undefined) mockTerminal = { provider: "execa", id: 1, @@ -91,6 +93,28 @@ describe("ExecaTerminalProcess", () => { expect(calledOptions.env.LANG).toBe("en_US.UTF-8") expect(calledOptions.env.LC_ALL).toBe("en_US.UTF-8") }) + + it("should use execaShellPath when set", async () => { + BaseTerminal.setExecaShellPath("/bin/bash") + await terminalProcess.run("echo test") + const execaMock = vitest.mocked(execa) + expect(execaMock).toHaveBeenCalledWith( + expect.objectContaining({ + shell: "/bin/bash", + }), + ) + }) + + it("should fall back to shell=true when execaShellPath is undefined", async () => { + BaseTerminal.setExecaShellPath(undefined) + await terminalProcess.run("echo test") + const execaMock = vitest.mocked(execa) + expect(execaMock).toHaveBeenCalledWith( + expect.objectContaining({ + shell: true, + }), + ) + }) }) describe("basic functionality", () => {