fix(terminal): harden profile settings and cleanup

Author: edelauna

apps/vscode-e2e/src/suite/tools/terminal-profile.test.ts

@@ -166,6 +166,17 @@ suite("Terminal Profile", function () {
 			assert.ok(content.includes("zoo-profile-override-ok"), `Output file should contain marker, got: ${content}`)
 
 			assert.ok(vscode.window.terminals.length >= 1, "At least one VS Code terminal should exist")
+			const profileTerminal = vscode.window.terminals.find((terminal) => {
+				const options = terminal.creationOptions as vscode.TerminalOptions
+				return (
+					options.name === "Zoo Code" &&
+					options.shellPath === "/bin/bash" &&
+					Array.isArray(options.shellArgs) &&
+					options.shellArgs.includes("--noprofile") &&
+					options.shellArgs.includes("--norc")
+				)
+			})
+			assert.ok(profileTerminal, "Expected a Zoo Code terminal created with the configured Bash profile")
 		} finally {
 			api.off(RooCodeEventName.Message, messageHandler)
 		}

packages/types/src/api.ts

@@ -140,7 +140,8 @@ export interface RooCodeAPI extends EventEmitter<RooCodeAPIEvents> {
 	 */
 	setActiveProfile(name: string): Promise<string | undefined>
 	/**
-	 * Activates a VS Code terminal profile override for Zoo Code commands.
+	 * Activates a process-wide VS Code terminal profile override for Zoo Code
+	 * commands. This is intended for trusted extension integrations.
 	 * Passing undefined restores the VS Code default profile behavior and
 	 * closes idle terminals so the next command starts fresh.
 	 */

src/core/tools/ExecuteCommandTool.ts

@@ -15,7 +15,9 @@ import { unescapeHtmlEntities } from "../../utils/text-normalization"
 import {
 	ExitCodeDetails,
 	RooTerminalCallbacks,
+	RooTerminalProvider,
 	RooTerminalProcess,
+	ShellIntegrationError,
 	ShellIntegrationErrorDetails,
 } from "../../integrations/terminal/types"
 import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
@@ -26,19 +28,22 @@ import { t } from "../../i18n"
 import { getTaskDirectoryPath } from "../../utils/storage"
 import { BaseTool, ToolCallbacks } from "./BaseTool"
 
-export class ShellIntegrationError extends Error {
-	constructor(
-		message: string,
-		public readonly commandSubmitted: boolean,
-	) {
-		super(message)
-	}
-}
+export { ShellIntegrationError } from "../../integrations/terminal/types"
 
 export function canRetryShellIntegrationError(error: unknown): error is ShellIntegrationError {
 	return error instanceof ShellIntegrationError && !error.commandSubmitted
 }
 
+export function getTerminalProviderForExecution(terminalShellIntegrationDisabled: boolean): {
+	terminalProvider: RooTerminalProvider
+	isCmdExeFallback: boolean
+} {
+	const isCmdExeFallback = !terminalShellIntegrationDisabled && Terminal.isActiveShellCmdExe()
+	const terminalProvider = terminalShellIntegrationDisabled || isCmdExeFallback ? "execa" : "vscode"
+
+	return { terminalProvider, isCmdExeFallback }
+}
+
 interface ExecuteCommandParams {
 	command: string
 	cwd?: string
@@ -224,8 +229,7 @@ export async function executeCommandInTerminal(
 	let shellIntegrationError: ShellIntegrationError | undefined
 	let hasAskedForCommandOutput = false
 
-	const isCmdExeFallback = !terminalShellIntegrationDisabled && Terminal.isActiveShellCmdExe()
-	const terminalProvider = terminalShellIntegrationDisabled || isCmdExeFallback ? "execa" : "vscode"
+	const { terminalProvider, isCmdExeFallback } = getTerminalProviderForExecution(terminalShellIntegrationDisabled)
 	const provider = await task.providerRef.deref()
 
 	// cmd.exe can't use shell integration — tell the webview to expand the output
@@ -515,31 +519,15 @@ export async function executeCommandInTerminal(
 			return [false, formatPersistedOutput(persistedResult, exitDetails, currentWorkingDir)]
 		}
 
-		// Use inline format for small outputs (original behavior with exit status)
-		let exitStatus: string = ""
-
-		if (exitDetails !== undefined) {
-			if (exitDetails.signalName) {
-				exitStatus = `Process terminated by signal ${exitDetails.signalName}`
-
-				if (exitDetails.coreDumpPossible) {
-					exitStatus += " - core dump possible"
-				}
-			} else if (exitDetails.exitCode === undefined) {
-				result += "<VSCE exit code is undefined: terminal output and command execution status is unknown.>"
-				exitStatus = `Exit code: <undefined, notify user>`
-			} else {
-				if (exitDetails.exitCode !== 0) {
-					exitStatus += "Command execution was not successful, inspect the cause and adjust as needed.\n"
-				}
-
-				exitStatus += `Exit code: ${exitDetails.exitCode}`
-			}
-		} else {
+		// Use inline format for small outputs (original behavior with exit status).
+		if (exitDetails === undefined) {
 			result += "<VSCE exitDetails == undefined: terminal output and command execution status is unknown.>"
-			exitStatus = `Exit code: <undefined, notify user>`
+		} else if (!exitDetails.signalName && exitDetails.exitCode === undefined) {
+			result += "<VSCE exit code is undefined: terminal output and command execution status is unknown.>"
 		}
 
+		const exitStatus = formatExitStatus(exitDetails)
+
 		return [
 			false,
 			`Command executed in terminal within working directory '${currentWorkingDir}'. ${exitStatus}\nOutput:\n${result}`,

src/core/tools/__tests__/executeCommandTool.spec.ts

@@ -7,6 +7,7 @@ import { Task } from "../../task/Task"
 import { formatResponse } from "../../prompts/responses"
 import { ToolUse, AskApproval, HandleError, PushToolResult } from "../../../shared/tools"
 import { unescapeHtmlEntities } from "../../../utils/text-normalization"
+import { Terminal } from "../../../integrations/terminal/Terminal"
 
 // Mock dependencies
 vitest.mock("execa", () => ({
@@ -268,6 +269,15 @@ describe("executeCommandTool", () => {
 
 			expect(executeCommandModule.canRetryShellIntegrationError(error)).toBe(false)
 		})
+
+		it("selects the Execa fallback provider for cmd.exe shell integration", () => {
+			vitest.spyOn(Terminal, "isActiveShellCmdExe").mockReturnValue(true)
+
+			expect(executeCommandModule.getTerminalProviderForExecution(false)).toEqual({
+				terminalProvider: "execa",
+				isCmdExeFallback: true,
+			})
+		})
 	})
 
 	describe("Command execution timeout configuration", () => {

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

@@ -172,6 +172,7 @@ vi.mock("../../mentions/resolveImageMentions", () => ({
 
 import { resolveImageMentions } from "../../mentions/resolveImageMentions"
 import { Terminal } from "../../../integrations/terminal/Terminal"
+import { TerminalRegistry } from "../../../integrations/terminal/TerminalRegistry"
 
 describe("webviewMessageHandler - requestLmStudioModels", () => {
 	beforeEach(() => {
@@ -874,32 +875,78 @@ describe("webviewMessageHandler - mcpEnabled", () => {
 describe("webviewMessageHandler - terminalProfile", () => {
 	beforeEach(() => {
 		vi.clearAllMocks()
+		Terminal.setTerminalProfile(undefined)
 	})
 
 	afterEach(() => {
+		Terminal.setTerminalProfile(undefined)
 		vi.restoreAllMocks()
 	})
 
-	it("bridges a saved terminalProfile from updateSettings into the process-wide terminal state", async () => {
-		const setTerminalProfileSpy = vi.spyOn(Terminal, "setTerminalProfile").mockImplementation(() => {})
+	it("normalizes and persists a saved terminalProfile, then closes stale idle terminals", async () => {
+		const closeIdleTerminalsSpy = vi.spyOn(TerminalRegistry, "closeIdleTerminals").mockImplementation(() => {})
 
 		await webviewMessageHandler(mockClineProvider, {
 			type: "updateSettings",
-			updatedSettings: { terminalProfile: "Git Bash" },
+			updatedSettings: { terminalProfile: " Git Bash " },
 		})
 
-		expect(setTerminalProfileSpy).toHaveBeenCalledWith("Git Bash")
+		expect(Terminal.getTerminalProfile()).toBe("Git Bash")
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("terminalProfile", "Git Bash")
+		expect(closeIdleTerminalsSpy).toHaveBeenCalledTimes(1)
 	})
 
-	it("clears the terminal profile when updateSettings sends undefined", async () => {
-		const setTerminalProfileSpy = vi.spyOn(Terminal, "setTerminalProfile").mockImplementation(() => {})
+	it("does not close idle terminals when hydration sends the unchanged profile", async () => {
+		Terminal.setTerminalProfile("Git Bash")
+		const closeIdleTerminalsSpy = vi.spyOn(TerminalRegistry, "closeIdleTerminals").mockImplementation(() => {})
 
 		await webviewMessageHandler(mockClineProvider, {
 			type: "updateSettings",
-			updatedSettings: { terminalProfile: undefined },
+			updatedSettings: { terminalProfile: " Git Bash " },
 		})
 
-		expect(setTerminalProfileSpy).toHaveBeenCalledWith(undefined)
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("terminalProfile", "Git Bash")
+		expect(closeIdleTerminalsSpy).not.toHaveBeenCalled()
+	})
+
+	it("clears the persisted profile when SettingsView sends the empty-string sentinel", async () => {
+		Terminal.setTerminalProfile("Git Bash")
+		const closeIdleTerminalsSpy = vi.spyOn(TerminalRegistry, "closeIdleTerminals").mockImplementation(() => {})
+
+		await webviewMessageHandler(mockClineProvider, {
+			type: "updateSettings",
+			updatedSettings: { terminalProfile: "" },
+		})
+
+		expect(Terminal.getTerminalProfile()).toBeUndefined()
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("terminalProfile", undefined)
+		expect(closeIdleTerminalsSpy).toHaveBeenCalledTimes(1)
+	})
+
+	it("does not close idle terminals when the empty-string sentinel leaves the profile unset", async () => {
+		const closeIdleTerminalsSpy = vi.spyOn(TerminalRegistry, "closeIdleTerminals").mockImplementation(() => {})
+
+		await webviewMessageHandler(mockClineProvider, {
+			type: "updateSettings",
+			updatedSettings: { terminalProfile: "" },
+		})
+
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("terminalProfile", undefined)
+		expect(closeIdleTerminalsSpy).not.toHaveBeenCalled()
+	})
+
+	it("treats non-string terminalProfile values as unset", async () => {
+		Terminal.setTerminalProfile("Git Bash")
+		const closeIdleTerminalsSpy = vi.spyOn(TerminalRegistry, "closeIdleTerminals").mockImplementation(() => {})
+
+		await webviewMessageHandler(mockClineProvider, {
+			type: "updateSettings",
+			updatedSettings: { terminalProfile: 42 as any },
+		})
+
+		expect(Terminal.getTerminalProfile()).toBeUndefined()
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("terminalProfile", undefined)
+		expect(closeIdleTerminalsSpy).toHaveBeenCalledTimes(1)
 	})
 })
 

src/core/webview/webviewMessageHandler.ts

@@ -728,11 +728,16 @@ export const webviewMessageHandler = async (
 							Terminal.setTerminalZdotdir(value as boolean)
 						}
 					} else if (key === "terminalProfile") {
-						Terminal.setTerminalProfile(value as string | undefined)
-						// Discard idle terminals so the next command gets a fresh
-						// terminal using the new profile's shell instead of reusing
-						// a stale one from the previous profile.
-						TerminalRegistry.closeIdleTerminals()
+						const previousProfile = Terminal.getTerminalProfile()
+						Terminal.setTerminalProfile(typeof value === "string" ? value : undefined)
+						newValue = Terminal.getTerminalProfile()
+
+						if (newValue !== previousProfile) {
+							// Discard idle terminals so the next command gets a fresh
+							// terminal using the new profile's shell instead of reusing
+							// a stale one from the previous profile.
+							TerminalRegistry.closeIdleTerminals()
+						}
 					} else if (key === "execaShellPath") {
 						Terminal.setExecaShellPath(value as string | undefined)
 					} else if (key === "mcpEnabled") {

src/extension.ts

@@ -31,6 +31,7 @@ import { formatLanguage } from "./shared/language"
 import { ContextProxy } from "./core/config/ContextProxy"
 import { ClineProvider } from "./core/webview/ClineProvider"
 import { DIFF_VIEW_URI_SCHEME } from "./integrations/editor/DiffViewProvider"
+import { Terminal } from "./integrations/terminal/Terminal"
 import { TerminalRegistry } from "./integrations/terminal/TerminalRegistry"
 import { openAiCodexOAuthManager } from "./integrations/openai-codex/oauth"
 import { McpServerManager } from "./services/mcp/McpServerManager"
@@ -387,5 +388,6 @@ export async function deactivate() {
 
 	await McpServerManager.cleanup(extensionContext)
 	TelemetryService.instance.shutdown()
+	Terminal.setTerminalProfile(undefined)
 	TerminalRegistry.cleanup()
 }

src/extension/__tests__/api-terminal-profile.spec.ts

@@ -0,0 +1,41 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
+import type * as vscode from "vscode"
+
+import { API } from "../api"
+import type { ClineProvider } from "../../core/webview/ClineProvider"
+import { Terminal } from "../../integrations/terminal/Terminal"
+import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
+
+vi.mock("@roo-code/ipc", () => ({
+	IpcServer: class {},
+}))
+
+describe("API - terminal profile", () => {
+	let api: API
+
+	beforeEach(() => {
+		const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel
+		const provider = {
+			context: {},
+			on: vi.fn(),
+		} as unknown as ClineProvider
+
+		Terminal.setTerminalProfile(undefined)
+		api = new API(outputChannel, provider)
+	})
+
+	afterEach(() => {
+		Terminal.setTerminalProfile(undefined)
+		vi.restoreAllMocks()
+	})
+
+	it("closes idle terminals only when the normalized profile changes", () => {
+		const closeIdleTerminalsSpy = vi.spyOn(TerminalRegistry, "closeIdleTerminals").mockImplementation(() => {})
+
+		api.setTerminalProfile(" Git Bash ")
+		api.setTerminalProfile("Git Bash")
+
+		expect(Terminal.getTerminalProfile()).toBe("Git Bash")
+		expect(closeIdleTerminalsSpy).toHaveBeenCalledTimes(1)
+	})
+})

src/extension/api.ts

@@ -480,8 +480,12 @@ export class API extends EventEmitter<RooCodeEvents> implements RooCodeAPI {
 	}
 
 	public setTerminalProfile(name: string | undefined): void {
+		const previousProfile = Terminal.getTerminalProfile()
 		Terminal.setTerminalProfile(name)
-		TerminalRegistry.closeIdleTerminals()
+
+		if (Terminal.getTerminalProfile() !== previousProfile) {
+			TerminalRegistry.closeIdleTerminals()
+		}
 	}
 
 	// Provider Profile Management

src/integrations/terminal/BaseTerminal.ts

@@ -44,7 +44,7 @@ export abstract class BaseTerminal implements RooTerminal {
 	/**
 	 * Sets the active stream for this terminal and notifies the process
 	 * @param stream The stream to set, or undefined to clean up
-	 * @throws Error if process is undefined when a stream is provided
+	 * If no process exists when a stream is provided, logs a warning and returns.
 	 */
 	public setActiveStream(stream: AsyncIterable<string> | undefined, pid?: number): void {
 		if (stream) {