From 5d7a4d2266218fd751b299a866cf9cc7fedec4b7 Mon Sep 17 00:00:00 2001 From: John Richmond <5629+jr@users.noreply.github.com> Date: Tue, 3 Mar 2026 22:18:17 -0800 Subject: [PATCH 1/2] 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. --- 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 7f7f54f918f..d1fc3b2f5e2 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 b9093bef0d7..a0f68286e61 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 7e0eb07a553..393990301f1 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 229fddb140a..2e723599271 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 de614737d52..68bbd29229f 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 00000000000..ed4c004e6e0 --- /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.access).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 00000000000..548df919b2b --- /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 50a8bc9c45e..2759b4f113a 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 91b37f3d6d1..288f6c2118c 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 ab06f0dabbe..b20539afe49 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 f1be1316d20..d27fd6bec09 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 121dc343136..ee262549346 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 370bf0d377b..cc2af938027 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 c87ee5ad05d..5f0a21869ec 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", () => { From 678b17824c10b36a41f7fc9c3d0c7de0333c3aff Mon Sep 17 00:00:00 2001 From: John Richmond <5629+jr@users.noreply.github.com> Date: Tue, 3 Mar 2026 22:36:48 -0800 Subject: [PATCH 2/2] test(cli): make shell path access test cross-platform --- apps/cli/src/lib/utils/__tests__/shell.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/cli/src/lib/utils/__tests__/shell.test.ts b/apps/cli/src/lib/utils/__tests__/shell.test.ts index ed4c004e6e0..7e94131c3b5 100644 --- a/apps/cli/src/lib/utils/__tests__/shell.test.ts +++ b/apps/cli/src/lib/utils/__tests__/shell.test.ts @@ -34,7 +34,7 @@ describe("validateTerminalShellPath", () => { }) it("returns invalid when the shell path cannot be accessed", async () => { - vi.mocked(fs.access).mockRejectedValueOnce(new Error("ENOENT")) + vi.mocked(fs.stat).mockRejectedValueOnce(new Error("ENOENT")) const result = await validateTerminalShellPath("/missing/shell") expect(result.valid).toBe(false)