refactor(terminal): scope profile picker to VS Code integrated terminal, filter source-only profiles

Author: edelauna

packages/types/src/vscode-extension-host.ts

@@ -459,6 +459,7 @@ export interface WebviewMessage {
 		| "getVSCodeSetting"
 		| "vsCodeSetting"
 		| "requestTerminalProfiles"
+		| "openTerminalProfilePicker"
 		| "updateCondensingPrompt"
 		| "playSound"
 		| "playTts"

src/core/webview/__tests__/webviewMessageHandler.spec.ts

@@ -105,6 +105,9 @@ vi.mock("vscode", () => {
 			openTextDocument,
 			getConfiguration: vi.fn(() => ({ get: vi.fn() })),
 		},
+		commands: {
+			executeCommand: vi.fn().mockResolvedValue(undefined),
+		},
 	}
 })
 
@@ -897,17 +900,48 @@ describe("webviewMessageHandler - terminalProfile", () => {
 describe("webviewMessageHandler - requestTerminalProfiles", () => {
 	beforeEach(() => {
 		vi.clearAllMocks()
+		vi.spyOn(Terminal, "resolveProfilePath").mockImplementation((profilePath) => {
+			const candidates = Array.isArray(profilePath) ? profilePath : [profilePath]
+			const value = candidates.find(
+				(candidate) =>
+					typeof candidate === "string" && candidate.trim().length > 0 && !candidate.includes("missing"),
+			)
+			return typeof value === "string" ? value.trim() : undefined
+		})
+	})
+
+	afterEach(() => {
+		vi.restoreAllMocks()
 	})
 
-	it("posts sorted profile names for the active platform", async () => {
-		const mockGet = vi.fn().mockReturnValue({ "Git Bash": {}, PowerShell: {}, "Command Prompt": {} })
+	it("posts sorted path-resolvable profile names for the active platform", async () => {
+		const mockGet = vi.fn().mockReturnValue({
+			"Git Bash": { path: "C:\\Git\\bin\\bash.exe" },
+			bash: { path: "/bin/bash" },
+			PowerShell: { source: "PowerShell" }, // source-only — must be excluded
+		})
 		vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
 
 		await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
 
 		expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
 			type: "terminalProfiles",
-			profiles: ["Command Prompt", "Git Bash", "PowerShell"],
+			profiles: ["Git Bash", "bash"],
+		})
+	})
+
+	it("excludes source-only profiles that have no path field", async () => {
+		const mockGet = vi.fn().mockReturnValue({
+			PowerShell: { source: "PowerShell" },
+			"Windows PowerShell": { source: "PowerShell" },
+		})
+		vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
+
+		await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
+
+		expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
+			type: "terminalProfiles",
+			profiles: [],
 		})
 	})
 
@@ -935,6 +969,64 @@ describe("webviewMessageHandler - requestTerminalProfiles", () => {
 			profiles: [],
 		})
 	})
+
+	it("excludes profiles with empty or whitespace-only path strings", async () => {
+		const mockGet = vi.fn().mockReturnValue({
+			empty: { path: "" },
+			whitespace: { path: "   " },
+			valid: { path: "/bin/bash" },
+		})
+		vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
+
+		await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
+
+		expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
+			type: "terminalProfiles",
+			profiles: ["valid"],
+		})
+	})
+
+	it("excludes profiles with path arrays containing only empty or whitespace strings", async () => {
+		const mockGet = vi.fn().mockReturnValue({
+			emptyArray: { path: [] },
+			whitespaceArray: { path: ["", "   "] },
+			valid: { path: ["/bin/bash", "/usr/bin/bash"] },
+		})
+		vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
+
+		await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
+
+		expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
+			type: "terminalProfiles",
+			profiles: ["valid"],
+		})
+	})
+
+	it("excludes profiles whose executable cannot be resolved", async () => {
+		const mockGet = vi.fn().mockReturnValue({
+			missing: { path: "/missing/bash" },
+			valid: { path: "/bin/bash" },
+		})
+		vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({ get: mockGet } as any)
+
+		await webviewMessageHandler(mockClineProvider, { type: "requestTerminalProfiles" })
+
+		expect(mockClineProvider.postMessageToWebview).toHaveBeenCalledWith({
+			type: "terminalProfiles",
+			profiles: ["valid"],
+		})
+	})
+})
+
+describe("webviewMessageHandler - openTerminalProfilePicker", () => {
+	beforeEach(() => {
+		vi.clearAllMocks()
+	})
+
+	it("executes the VS Code selectDefaultShell command", async () => {
+		await webviewMessageHandler(mockClineProvider, { type: "openTerminalProfilePicker" })
+		expect(vscode.commands.executeCommand).toHaveBeenCalledWith("workbench.action.terminal.selectDefaultShell")
+	})
 })
 
 describe("webviewMessageHandler - requestCommands", () => {

src/core/webview/webviewMessageHandler.ts

@@ -1311,6 +1311,12 @@ export const webviewMessageHandler = async (
 
 			break
 		}
+		case "openTerminalProfilePicker": {
+			// Open VS Code's native terminal profile picker so the user can set the
+			// default shell without leaving VS Code's own settings UI.
+			await vscode.commands.executeCommand("workbench.action.terminal.selectDefaultShell")
+			break
+		}
 		case "openKeyboardShortcuts": {
 			// Open VSCode keyboard shortcuts settings and optionally filter to show the Roo Code commands
 			const searchQuery = message.text || ""
@@ -1517,18 +1523,30 @@ export const webviewMessageHandler = async (
 			// return only the sanitized profile names. The terminal profile dropdown
 			// only needs names, so this avoids routing it through the generic
 			// `getVSCodeSetting` handler (which reads any key the webview supplies).
+			// Only profiles with a resolvable `path` are returned — source-only
+			// profiles (e.g. { source: "PowerShell" }) cannot be mapped to a shell
+			// binary by an extension and would silently fall back to the default.
 			try {
 				const names = new Set<string>()
 
-				const platformKey =
-					process.platform === "win32" ? "windows" : process.platform === "darwin" ? "osx" : "linux"
+				const platformKey = Terminal.getPlatformProfileKey()
 				const profiles = vscode.workspace
 					.getConfiguration("terminal.integrated.profiles")
 					.get<Record<string, unknown>>(platformKey)
 
 				if (profiles && typeof profiles === "object") {
-					for (const name of Object.keys(profiles)) {
-						names.add(name)
+					for (const [name, entry] of Object.entries(profiles)) {
+						if (!entry || typeof entry !== "object") {
+							continue
+						}
+
+						const { path } = entry as { path?: unknown }
+
+						// Source-only profiles and paths that cannot be found on disk or
+						// PATH are excluded because the override would fail at launch.
+						if (Terminal.resolveProfilePath(path)) {
+							names.add(name)
+						}
 					}
 				}
 

src/integrations/terminal/BaseTerminal.ts

@@ -298,9 +298,9 @@ export abstract class BaseTerminal implements RooTerminal {
 	}
 
 	/**
-	 * Sets the name of the VS Code terminal profile to use for the inline
-	 * (shell-integration) terminal. An empty/undefined value falls back to
-	 * VS Code's default terminal behavior.
+	 * Sets the name of the VS Code terminal profile to use for the integrated
+	 * terminal. An empty/undefined value falls back to VS Code's default terminal
+	 * behavior.
 	 * @param profile The terminal profile name, or undefined for the default
 	 */
 	public static setTerminalProfile(profile: string | undefined): void {
@@ -309,7 +309,7 @@ export abstract class BaseTerminal implements RooTerminal {
 	}
 
 	/**
-	 * Gets the name of the VS Code terminal profile to use for the inline terminal.
+	 * Gets the name of the VS Code terminal profile to use for the integrated terminal.
 	 * @returns The terminal profile name, or undefined when the default should be used
 	 */
 	public static getTerminalProfile(): string | undefined {

src/integrations/terminal/Terminal.ts

@@ -1,4 +1,5 @@
 import { existsSync } from "fs"
+import * as path from "path"
 
 import * as vscode from "vscode"
 import pWaitFor from "p-wait-for"
@@ -25,10 +26,10 @@ export class Terminal extends BaseTerminal {
 		} else {
 			const options: vscode.TerminalOptions = { cwd, name: "Roo Code", iconPath, env }
 
-			// When the user has chosen a specific terminal profile, resolve it to a
-			// shell path/args/env so the inline terminal uses that shell (e.g. Git Bash
-			// with a UTF-8 charset on Windows). When unset, we leave shellPath/shellArgs
-			// undefined so VS Code's default terminal behavior is preserved (#119).
+			// When the user has chosen a VS Code terminal profile, resolve it to a
+			// shell path/args/env so the integrated terminal uses that shell. When
+			// unset, shellPath/shellArgs are left undefined so VS Code's default
+			// terminal behavior is preserved.
 			const profileShell = Terminal.getProfileShell()
 
 			if (profileShell?.shellPath) {
@@ -225,7 +226,7 @@ export class Terminal extends BaseTerminal {
 	 * Returns the VS Code config section key (`windows`/`osx`/`linux`) used for
 	 * platform-specific terminal profiles.
 	 */
-	private static getPlatformProfileKey(platform: NodeJS.Platform = process.platform): "windows" | "osx" | "linux" {
+	public static getPlatformProfileKey(platform: NodeJS.Platform = process.platform): "windows" | "osx" | "linux" {
 		if (platform === "win32") {
 			return "windows"
 		}
@@ -238,13 +239,69 @@ export class Terminal extends BaseTerminal {
 	}
 
 	/**
-	 * Resolves the configured inline terminal profile (see `terminalProfile`
+	 * Resolves a profile path to an executable on disk. VS Code's built-in Unix
+	 * profiles commonly use bare command names such as `bash`, so check PATH in
+	 * addition to explicit filesystem paths.
+	 */
+	public static resolveProfilePath(
+		profilePath: unknown,
+		platform: NodeJS.Platform = process.platform,
+		env: NodeJS.ProcessEnv = process.env,
+	): string | undefined {
+		const candidates = Array.isArray(profilePath) ? profilePath : [profilePath]
+		const pathValue = env.PATH ?? env.Path ?? env.path
+		const pathEntries = pathValue?.split(platform === "win32" ? ";" : ":") ?? []
+
+		for (const value of candidates) {
+			if (typeof value !== "string") {
+				continue
+			}
+
+			const candidate = value.trim()
+
+			if (!candidate) {
+				continue
+			}
+
+			if (/[\\/]/.test(candidate)) {
+				if (existsSync(candidate)) {
+					return candidate
+				}
+
+				continue
+			}
+
+			const extensions =
+				platform === "win32" && path.extname(candidate) === ""
+					? (env.PATHEXT ?? ".COM;.EXE;.BAT;.CMD").split(";")
+					: [""]
+
+			for (const entry of pathEntries) {
+				const directory = entry.replace(/^"(.*)"$/, "$1")
+
+				for (const extension of extensions) {
+					const resolved = path.join(directory, `${candidate}${extension}`)
+
+					if (existsSync(resolved)) {
+						return resolved
+					}
+				}
+			}
+		}
+
+		return undefined
+	}
+
+	/**
+	 * Resolves the configured VS Code terminal profile (see `terminalProfile`
 	 * setting / {@link Terminal.getTerminalProfile}) into a shell path and args by
 	 * reading VS Code's `terminal.integrated.profiles.<platform>` configuration.
 	 *
 	 * This reuses VS Code's terminal profile concept so users can pick, for
-	 * example, a Git Bash profile (UTF-8) instead of the default cmd/PowerShell
-	 * (which may use a non-UTF-8 charset such as GBK) on Windows (#119).
+	 * example, a Git Bash profile instead of the default shell. Only profiles
+	 * with a resolvable `path` are supported; source-only profiles (e.g.
+	 * `{ source: "PowerShell" }`) cannot be mapped to a shell binary by an
+	 * extension and return undefined.
 	 *
 	 * @returns The resolved shell path/args, or undefined when no profile is
 	 *   configured or the profile cannot be resolved (default behavior).
@@ -279,12 +336,7 @@ export class Terminal extends BaseTerminal {
 			return undefined
 		}
 
-		// A `path` may be a single string or an array of candidate paths. VS Code
-		// picks the first candidate that exists on disk, so mirror that: prefer the
-		// first existing path, otherwise fall back to the first non-empty candidate.
-		const candidates = Array.isArray(profile.path) ? profile.path : [profile.path]
-		const nonEmpty = candidates.filter((p): p is string => typeof p === "string" && p.length > 0)
-		const pathValue = nonEmpty.find((p) => existsSync(p)) ?? nonEmpty[0]
+		const pathValue = Terminal.resolveProfilePath(profile.path, platform)
 
 		if (!pathValue) {
 			// Profiles defined only by `source` (e.g. "PowerShell") can't be mapped to