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
This commit is contained in:
John Richmond 2026-03-03 23:31:34 -08:00 committed by GitHub
parent 9a58f76299
commit 7dc83a522e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 200 additions and 1 deletions

View file

@ -184,6 +184,7 @@ Tokens are valid for 90 days. The CLI will prompt you to re-authenticate when yo
| `--provider <provider>` | API provider (roo, anthropic, openai, openrouter, etc.) | `openrouter` (or `roo` if authenticated) |
| `-m, --model <model>` | Model to use | `anthropic/claude-opus-4.6` |
| `--mode <mode>` | Mode to start in (code, architect, ask, debug, etc.) | `code` |
| `--terminal-shell <path>` | Absolute shell path for inline terminal command execution | Auto-detected shell |
| `-r, --reasoning-effort <effort>` | Reasoning effort level (unspecified, disabled, none, minimal, low, medium, high, xhigh) | `medium` |
| `--consecutive-mistake-limit <n>` | Consecutive error/repetition limit before guidance prompt (`0` disables the limit) | `10` |
| `--ephemeral` | Run without persisting state (uses temporary storage) | `false` |

View file

@ -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)
})
})
})

View file

@ -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
}
}
// ==========================================================================

View file

@ -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

View file

@ -47,6 +47,7 @@ program
.option("--provider <provider>", "API provider (roo, anthropic, openai, openrouter, etc.)")
.option("-m, --model <model>", "Model to use", DEFAULT_FLAGS.model)
.option("--mode <mode>", "Mode to start in (code, architect, ask, debug, etc.)", DEFAULT_FLAGS.mode)
.option("--terminal-shell <path>", "Absolute path to shell executable for inline terminal commands")
.option(
"-r, --reasoning-effort <effort>",
"Reasoning effort level (unspecified, disabled, none, minimal, low, medium, high, xhigh)",

View file

@ -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<ReturnType<typeof fs.stat>>)
})
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<ReturnType<typeof fs.stat>>)
const result = await validateTerminalShellPath("/bin")
expect(result).toEqual({ valid: false, reason: "shell path must point to a file" })
})
})

View file

@ -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<TerminalShellValidationResult> {
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 }
}

View file

@ -34,6 +34,7 @@ export type FlagOptions = {
provider?: SupportedProvider
model?: string
mode?: string
terminalShell?: string
reasoningEffort?: ReasoningEffortFlagOptions
consecutiveMistakeLimit?: number
ephemeral: boolean

View file

@ -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(),

View file

@ -282,6 +282,7 @@ export type ExtensionState = Pick<
| "terminalZshOhMy"
| "terminalZshP10k"
| "terminalZdotdir"
| "execaShellPath"
| "diagnosticsEnabled"
| "language"
| "modeApiConfigs"

View file

@ -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()

View file

@ -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
}
}

View file

@ -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

View file

@ -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", () => {