fix(terminal): harden shell detection, WSL support, and error recovery (#321) (#333)

Replace the fragile SHELL_ALLOWLIST in shell.ts with delegation to
vscode.env.shell (stable since VS Code 1.37), which internally resolves
terminal.integrated.defaultProfile.platform - including PowerShell version
deduction, WSL detection, and per-platform system fallback.

Connect the Zoo Code terminal profile override to both execution paths:
the VS Code integrated terminal (shellPath/shellArgs in createTerminal)
and the execa inline fallback (shell resolution + profile env + WSL args).

Shell Detection (shell.ts + Terminal.ts)
- getShell(): profile override -> vscode.env.shell -> os.userInfo().shell
  -> COMSPEC/SHELL -> platform default; WSL paths canonicalized to
  WSL_EXE_PATH regardless of casing or slash direction
- isActiveShellPowerShell(): replace source-string heuristic with actual
  PATH lookup (pwsh.exe / pwsh), fixing false positives on macOS/Linux
  where PowerShell is the VS Code default but pwsh is not installed
- Shell identity methods: isCmdExe, isPowerShell, isFish,
  isActiveShellCmdExe, isActiveShellPowerShell, isActiveShellFish

WSL Inline Execution (ExecaTerminalProcess.ts)
- Two-tier WSL detection: tier 1 - getShell() authoritative; tier 2 -
  getConfiguredWslProfileArgs() trusted-scope VS Code config for distro
- 3-tier path conversion: drive-letter -> /mnt/drive/ ->
  WSL UNC \\wsl$\Distro\ -> wsl.exe wslpath fallback
- Respect profile override shellArgs before falling back to VS Code
  default WSL args, preventing distro mismatch when Zoo Code overrides
  the active WSL profile
- Propagate profile env (sanitized, blocking ZDOTDIR/LD_PRELOAD/etc.)
  to the WSL execa invocation
- Direct wsl.exe spawn (execa file + args array) to avoid cmd.exe
  nested-quoting; --cd omitted with warning when wslpath fails

Terminal Process Error Recovery (TerminalProcess.ts + ExecaTerminalProcess.ts)
- SIGTERM instead of SIGKILL for process.kill on Windows compatibility
- Real exit code captured from execa subprocess; abort path preserves
  signal info instead of reporting exitCode:0
- Bounded Ctrl+C retry loop for processes that trap SIGINT (#266)
- Guard against overlapping abort retry loops via aborting flag
- completionTimeout safety net for lost shell_execution_complete
  (VS Code bug 237208); timeout cleared in finally to prevent
  unhandled Promise rejection
- TextDecoder with stream:true + final flush to avoid U+FFFD on
  multi-byte UTF-8 split across chunk boundaries
- WeakRef deref guarded in all event handlers against GC'd terminals
- C-marker search regression: restore commandOutputStarted boolean
  so OSC 633;C arriving after the first chunk is detected
- isHot cleared on stream timeout to prevent task loop stall
- Multiline scripts wrapped for single shell execution:
  PowerShell .{ }, fish begin/end, POSIX { }

Profile Override (Terminal.ts + BaseTerminal.ts)
- getConfiguredProfiles() / getConfiguredDefaultProfileName(): read only
  defaultValue + globalValue, workspace settings intentionally excluded
- getConfiguredWslProfileArgs(): detect WSL by source field or name
  pattern; normalize args (string -> [string])
- getProfileShell(): resolve profile to shellPath/shellArgs/env;
  sanitize env (block ZDOTDIR, LD_PRELOAD, DYLD_INSERT_LIBRARIES,
  BASH_ENV, etc.; only string/null values)
- Skip ZDOTDIR injection when profile override is active (VS Code's
  injector handles it; two ZDOTDIRs fight per issue 96295)

Execute Command Tool (ExecuteCommandTool.ts + Task.ts)
- canRetryShellIntegrationError(): type guard for commandSubmitted:false
  (shell startup race -> safe to retry via execa); commandSubmitted:true
  (already submitted -> warn, do not retry)
- Show shell_integration_warning when falling back to execa
- cmd.exe fast-path: force execa provider (cmd.exe cannot emit
  OSC 633;C per VS Code issue 164646)
- Reset hasAskedForCommandOutput on ask error for future retries
- 5s safety-net timeout for onCompleted promise
- Add abort guard to ask() pWaitFor to prevent deadlock on cancel
- Add abort/abandoned guard to userMessageContentReady pWaitFor

Tests (6 test files)
- shell.spec.ts: vscode.env.shell passthrough, WSL canonicalization
  (case + slashes), fallback chain, profile override
- TerminalProfile.spec.ts: profile resolution, shell identity methods,
  WSL args, ZDOTDIR guard, createTerminal integration, isFish
- TerminalProcess.spec.ts: C-marker regression, abort retry loop,
  multiline wrapping, hot timer, commandSubmitted branches
- ExecaTerminalProcess.spec.ts: WSL args merging, path conversion,
  profile env, abort behavior
- Terminal.spec.ts: profile-based constructor, reuseKey
- terminal-profile.test.ts (E2E): profile override lifecycle, transient
  shell_integration_warning acceptance

Fixes #321 and all 14 code-review defects from PR #333

Author: F915

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

@@ -126,7 +126,7 @@ suite("Terminal Profile", function () {
 		await sleep(100)
 	})
 
-	test("executes command through profile override without shell integration warning", async function () {
+	test("executes command through profile override (shell startup race may emit transient warning)", async function () {
 		const api = globalThis.api
 		const messages: ClineMessage[] = []
 
@@ -155,7 +155,12 @@ suite("Terminal Profile", function () {
 			const gotWarning = messages.some((m) => m.type === "say" && m.say === "shell_integration_warning")
 			const gotError = messages.some((m) => m.type === "say" && m.say === "error")
 
-			assert.strictEqual(gotWarning, false, "Shell integration warning should not fire with a valid profile")
+			// shell_integration_warning is expected when shell integration has a
+			// startup race (commandSubmitted: false → execa fallback). This is
+			// environment-dependent: common in WSL, rare on native Linux/macOS.
+			if (gotWarning) {
+				console.info("shell_integration_warning fired — shell startup race occurred, execa fallback used")
+			}
 			assert.strictEqual(
 				gotError,
 				false,

src/core/task/Task.ts

@@ -1300,7 +1300,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		// Wait for askResponse to be set
 		await pWaitFor(
 			() => {
-				if (this.askResponse !== undefined || this.lastMessageTs !== askTs) {
+				if (this.askResponse !== undefined || this.lastMessageTs !== askTs || this.abort) {
 					return true
 				}
 

src/core/tools/ExecuteCommandTool.ts

@@ -141,7 +141,11 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
 				task.supersedePendingAsk()
 
 				if (canRetryShellIntegrationError(error)) {
-					// Silent retry via execa — shell startup race, command was not submitted.
+					// Shell integration not available — command was not submitted to the
+					// VS Code terminal. Show warning so the user knows the terminal mode
+					// changed, then fall back to execa.
+					await task.say("shell_integration_warning")
+
 					const status: CommandExecutionStatus = { executionId, status: "fallback" }
 					provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
 
@@ -356,7 +360,10 @@ export async function executeCommandInTerminal(
 					process.continue()
 				}
 			} catch (_error) {
-				// Silently handle ask errors (e.g., "Current ask promise was ignored")
+				// Silently handle ask errors (e.g., "Current ask promise was ignored").
+				// Reset the flag so a future ask can be triggered if the command is
+				// still producing output.
+				hasAskedForCommandOutput = false
 			}
 		},
 		onCompleted: async (output: string | undefined) => {
@@ -493,7 +500,10 @@ export async function executeCommandInTerminal(
 	// This ensures persistedResult is set before we try to use it, fixing the race
 	// condition where exitDetails is set (sync) before the async onCompleted finishes.
 	if (exitDetails && onCompletedPromise) {
-		await onCompletedPromise
+		await Promise.race([
+			onCompletedPromise,
+			new Promise<void>((resolve) => setTimeout(resolve, 5_000)),
+		])
 	}
 
 	if (message) {

src/integrations/terminal/ExecaTerminalProcess.ts

@@ -6,11 +6,15 @@ import type { RooTerminal } from "./types"
 import { BaseTerminal } from "./BaseTerminal"
 import { BaseTerminalProcess } from "./BaseTerminalProcess"
 import { getShell, WSL_EXE_PATH } from "../../utils/shell"
+import { Terminal } from "./Terminal"
 
 // Matches \\wsl$\Distro\... and \\wsl.localhost\Distro\...
 const WSL_UNC_PREFIX = /^\/\/wsl(?:\$|\.localhost)\/([^\/]+)\/?(.*)$/i
 
-async function convertWindowsPathToWsl(windowsPath: string): Promise<string | null> {
+async function convertWindowsPathToWsl(
+	windowsPath: string,
+	profileArgs?: string[],
+): Promise<string | null> {
 	const forward = windowsPath.replace(/\\/g, "/")
 
 	// Already a POSIX/WSL path — no Windows-to-WSL conversion needed.
@@ -35,9 +39,12 @@ async function convertWindowsPathToWsl(windowsPath: string): Promise<string | nu
 		return subPath.startsWith("/") ? subPath : `/${subPath}`
 	}
 
-	// Tier 3: Arbitrary UNC → wslpath fallback
+	// Tier 3: Arbitrary UNC → wslpath fallback.
+	// Pass the configured WSL distro args so wslpath runs in the correct
+	// distro — the system default may have different mount points.
 	try {
-		const { stdout } = await execa(WSL_EXE_PATH, ["wslpath", windowsPath], {
+		const wslpathArgs = [...(profileArgs ?? []), "wslpath", windowsPath]
+		const { stdout } = await execa(WSL_EXE_PATH, wslpathArgs, {
 			timeout: 5_000,
 			stdin: "ignore",
 		})
@@ -62,7 +69,11 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 		this.terminalRef = new WeakRef(terminal)
 
 		this.once("completed", () => {
-			this.terminal.busy = false
+			try {
+				this.terminal.busy = false
+			} catch {
+				// Terminal has been garbage collected — nothing to clean up.
+			}
 		})
 	}
 
@@ -85,27 +96,52 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 			const execaShellPath = BaseTerminal.getExecaShellPath()
 			const resolvedShell = execaShellPath || getShell()
 
+			// Resolve the profile shell for env propagation. Even in the execa fallback
+			// path, profile-specific environment variables (locale, PATH, etc.) should be
+			// honored. getShell() already handles the profile shell path; here we need the
+			// env portion of the profile definition.
+			const profileShell = execaShellPath ? undefined : Terminal.getProfileShell()
+
 			// WSL detection only applies when the user has NOT set an explicit execa shell.
 			const isWslShell = execaShellPath ? false : resolvedShell === WSL_EXE_PATH
 
 			if (isWslShell) {
 				// Spawn wsl.exe directly (not through cmd.exe) to avoid nested-quoting issues.
 				// execa(file, args, options) passes args as an array — no shell interpretation.
+				//
+				// WSL detection is two-tier:
+				//   Tier 1 — getShell() via getProfileShell() (authoritative):
+				//     decides whether we enter the WSL path at all
+				//   Tier 2 — profile override args, falling back to VS Code default:
+				//     supplements with user-configured profile args (e.g. distro selection).
+				//     When a Zoo Code terminalProfile override is active, use its shellArgs
+				//     directly. Otherwise fall back to the VS Code default WSL profile args.
+				const profileArgs = profileShell?.shellArgs?.length
+					? profileShell.shellArgs
+					: (Terminal.getConfiguredWslProfileArgs() ?? [])
 				const windowsCwd = this.terminal.getCurrentWorkingDirectory()
-				const wslCwd = await convertWindowsPathToWsl(windowsCwd)
+				const wslCwd = await convertWindowsPathToWsl(windowsCwd, profileArgs)
 
-				const wslArgs = ["--", "bash", "-c", command]
+				const wslArgs: string[] = [...profileArgs]
 
 				if (wslCwd) {
-					wslArgs.unshift("--cd", wslCwd)
+					wslArgs.push("--cd", wslCwd)
+				} else {
+					console.warn(
+						`[ExecaTerminalProcess] Could not convert Windows path to WSL: "${windowsCwd}". ` +
+							`Command will run in WSL home directory instead of expected CWD.`,
+					)
 				}
 
+				wslArgs.push("--", "bash", "-c", command)
+
 				this.subprocess = execa(WSL_EXE_PATH, wslArgs, {
 					cwd: undefined,
 					all: true,
 					stdin: "ignore",
 					env: {
 						...process.env,
+						...profileShell?.env,
 						LANG: "en_US.UTF-8",
 						LC_ALL: "en_US.UTF-8",
 					},
@@ -118,6 +154,10 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 					stdin: "ignore",
 					env: {
 						...process.env,
+						// Profile-specific environment variables (e.g. locale, custom PATH).
+						// Already sanitized by getProfileShell() — dangerous keys like
+						// ZDOTDIR, LD_PRELOAD are filtered.
+						...profileShell?.env,
 						LANG: "en_US.UTF-8",
 						LC_ALL: "en_US.UTF-8",
 					},
@@ -147,10 +187,23 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 
 			const rawStream = this.subprocess.iterable({ from: "all", preserveNewlines: true })
 
-			// Wrap the stream to ensure all chunks are strings (execa can return Uint8Array)
+			const decoder = new TextDecoder()
+
+			// Wrap the stream to ensure all chunks are strings.
+			// A single TextDecoder with { stream: true } handles multi-byte UTF-8
+			// characters split across chunk boundaries — a fresh decoder per chunk
+			// would produce U+FFFD replacement characters for partial sequences.
 			const stream = (async function* () {
 				for await (const chunk of rawStream) {
-					yield typeof chunk === "string" ? chunk : new TextDecoder().decode(chunk)
+					yield typeof chunk === "string" ? chunk : decoder.decode(chunk, { stream: true })
+				}
+				// Flush any remaining bytes buffered in the decoder.
+				// Without this call, an incomplete multi-byte UTF-8
+				// sequence at the very end of the stream would be
+				// silently dropped.
+				const final = decoder.decode()
+				if (final) {
+					yield final
 				}
 			})()
 
@@ -182,7 +235,11 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 					timeoutId = setTimeout(() => {
 						try {
 							this.subprocess?.kill("SIGKILL")
-						} catch (e) {}
+						} catch (e) {
+							console.warn(
+								`[ExecaTerminalProcess#run] kill timeout subprocess error: ${e instanceof Error ? e.message : String(e)}`,
+							)
+						}
 
 						resolve()
 					}, 5_000)
@@ -201,11 +258,49 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 				}
 			}
 
-			this.emit("shell_execution_complete", { exitCode: 0 })
+			// Capture real exit code from the execa subprocess.
+			// execa v9's iterable() does not throw on process failure — the
+			// for-await loop exits cleanly regardless of exit code.  Await the
+			// subprocess promise (already settled by now — the stream has
+			// ended) to get the actual exitCode and signalName.
+			let emitExitCode: number | undefined
+			let emitSignal: string | undefined
+
+			// Always attempt to read the real exit code.  If the subprocess already
+			// settled (stream ended), this gives us the actual result.  If it hasn't
+			// settled yet (abort killed it mid-stream), proceed to the abort signal path.
+			try {
+				const result = await this.subprocess
+				emitExitCode = result.exitCode
+				emitSignal = result.signal
+			} catch (error) {
+				if (error instanceof ExecaError) {
+					emitExitCode = error.exitCode
+					emitSignal = error.signal
+				} else {
+					// Unexpected error — re-throw to outer catch
+					throw error
+				}
+			}
+
+			if (this.aborted) {
+				// Subprocess was signalled but may have already exited normally
+				// before the abort flag was set.  Preserve the real exit code and
+				// signal if available.  Only default to SIGKILL when the process
+				// was killed mid-flight (no exit code available).
+				if (emitSignal === undefined && emitExitCode === undefined) {
+					emitSignal = "SIGKILL"
+				}
+			}
+
+			this.emit("shell_execution_complete", {
+				exitCode: emitExitCode,
+				signalName: emitSignal,
+			})
 		} catch (error) {
 			if (error instanceof ExecaError) {
 				console.error(`[ExecaTerminalProcess#run] shell execution error: ${error.message}`)
-				this.emit("shell_execution_complete", { exitCode: error.exitCode ?? 0, signalName: error.signal })
+				this.emit("shell_execution_complete", { exitCode: error.exitCode, signalName: error.signal })
 			} else {
 				console.error(
 					`[ExecaTerminalProcess#run] shell execution error: ${error instanceof Error ? error.message : String(error)}`,
@@ -216,7 +311,13 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 			this.subprocess = undefined
 		}
 
-		this.terminal.setActiveStream(undefined)
+		try {
+			this.terminal.setActiveStream(undefined)
+			this.terminal.running = false
+		} catch {
+			// Terminal has been garbage collected — nothing to clean up.
+		}
+
 		this.emitRemainingBufferIfListening()
 		this.stopHotTimer()
 		this.emit("completed", this.fullOutput)
@@ -249,7 +350,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 			// Kill the stored PID (which should be the actual command after our update)
 			if (this.pid) {
 				try {
-					process.kill(this.pid, "SIGKILL")
+					process.kill(this.pid, "SIGTERM")
 				} catch (e) {
 					console.warn(
 						`[ExecaTerminalProcess#abort] Failed to kill process ${this.pid}: ${e instanceof Error ? e.message : String(e)}`,
@@ -274,10 +375,10 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
 
 					for (const pid of pids) {
 						try {
-							process.kill(pid, "SIGKILL")
+							process.kill(pid, "SIGTERM")
 						} catch (e) {
 							console.warn(
-								`[ExecaTerminalProcess#abort] Failed to send SIGKILL to child PID ${pid}: ${e instanceof Error ? e.message : String(e)}`,
+								`[ExecaTerminalProcess#abort] Failed to send SIGTERM to child PID ${pid}: ${e instanceof Error ? e.message : String(e)}`,
 							)
 						}
 					}