Fix shell integration race condition (and other minor fixup/cleanup) (#1660)

* fix: clarify PowerShell command completion workaround

The command completion detection approach in PowerShell requires an output
string to allow duplicate commands to execute in some versions of code.
Update the string to explicitly indicate it is a Roo PowerShell workaround,
making it clear in terminal output that this is intentional behavior rather
than a side effect.

Signed-off-by: Eric Wheeler <[email protected]>

* cleanup: improve terminal logging and error handling

No functional changes - purely improves error handling and logging clarity.

Terminal.ts:
- Handle undefined process state in setActiveStream without throwing
- Add terminal IDs to all log messages for better traceability
- Improve error message clarity in shell integration timeout

TerminalRegistry.ts:
- Reorganize shell execution event handlers for better flow
- Log shell execution events before processing for reliable debugging
- Add detailed context to terminal not found scenarios
- Include command and execution state in error messages

Signed-off-by: Eric Wheeler <[email protected]>

* feat: make terminal shell integration timeout configurable

Users with long shell startup times were encountering "Shell Integration Unavailable" errors due to the hard-coded 4s timeout. The timeout is now configurable through Advanced Settings (1-60s).

Thanks @FILTHY for troubleshooting and @kiwina for suggesting making the timeout configurable.

Fixes #1654

Signed-off-by: Eric Wheeler <[email protected]>

* critical fix: race condition that prevents command completion

Terminal running state is now managed in TerminalRegistry instead of Terminal to prevent race between stream close and shell completion.

While this race may not trigger on current VSCode versions, newer releases with additional terminal fixes may expose the issue. This proactively prevents "Shell execution end event received, but process is not running" errors.

Signed-off-by: Eric Wheeler <[email protected]>

* fix: improve command execution path reporting

Enhance clarity of command execution context and error reporting:

- Check to see if the directory changed because of the command
- Clarify execution path message
- Add explicit message when command exits with non-zero code

Signed-off-by: Eric Wheeler <[email protected]>

* system instructions: clarify terminal directory operations

Clear guidance for the AI system on:
- Working directory constraints
- Path handling requirements
- Tool vs terminal directory behavior

Signed-off-by: Eric Wheeler <[email protected]>

* test: update snapshots for system prompt working directory instructions

Signed-off-by: Eric Wheeler <[email protected]>

---------

Signed-off-by: Eric Wheeler <[email protected]>
Co-authored-by: Eric Wheeler <[email protected]>
Co-authored-by: Chris Estreich <[email protected]>

Author: 3 people

src/core/Cline.ts

@@ -1033,7 +1033,7 @@ export class Cline extends EventEmitter<ClineEvents> {
 				),
 			]
 		} else if (completed) {
-			let exitStatus: string
+			let exitStatus: string = ""
 			if (exitDetails !== undefined) {
 				if (exitDetails.signal) {
 					exitStatus = `Process terminated by signal ${exitDetails.signal} (${exitDetails.signalName})`
@@ -1044,13 +1044,22 @@ export class Cline extends EventEmitter<ClineEvents> {
 					result += "<VSCE exit code is undefined: terminal output and command execution status is unknown.>"
 					exitStatus = `Exit code: <undefined, notify user>`
 				} else {
-					exitStatus = `Exit code: ${exitDetails.exitCode}`
+					if (exitDetails.exitCode !== 0) {
+						exitStatus += "Command execution was not successful, inspect the cause and adjust as needed.\n"
+					}
+					exitStatus += `Exit code: ${exitDetails.exitCode}`
 				}
 			} else {
 				result += "<VSCE exitDetails == undefined: terminal output and command execution status is unknown.>"
 				exitStatus = `Exit code: <undefined, notify user>`
 			}
-			const workingDirInfo = workingDir ? ` from '${workingDir.toPosix()}'` : ""
+
+			let workingDirInfo: string = workingDir ? ` within working directory '${workingDir.toPosix()}'` : ""
+			const newWorkingDir = terminalInfo.getCurrentWorkingDirectory()
+
+			if (newWorkingDir !== workingDir) {
+				workingDirInfo += `; command changed working directory for this terminal to '${newWorkingDir.toPosix()} so be aware that future commands will be executed from this directory`
+			}
 
 			const outputInfo = `\nOutput:\n${result}`
 			return [

src/core/prompts/__tests__/__snapshots__/system.test.ts.snap

@@ -64,7 +64,8 @@ export function getRulesSection(
 
 RULES
 
-- Your current working directory is: ${cwd.toPosix()}
+- The project base directory is: ${cwd.toPosix()}
+- All all file paths must be relative to this directory. However, commands may change directories in terminals, so respect working directory specified by the response to <execute_command>.
 - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '${cwd.toPosix()}', so be sure to pass in the correct 'path' parameter when using tools that require a path.
 - Do not use the ~ character or $HOME to refer to the home directory.
 - Before using the execute_command tool, you must first think about the SYSTEM INFORMATION context provided to understand the user's environment and tailor your commands to ensure they are compatible with their system. You must also consider if the command you need to run should be executed in a specific directory outside of the current working directory '${cwd.toPosix()}', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '${cwd.toPosix()}'). For example, if you needed to run \`npm install\` in a project outside of '${cwd.toPosix()}', you would need to prepend with a \`cd\` i.e. pseudocode for this would be \`cd (path to project) && (command, in this case npm install)\`.

src/core/prompts/sections/rules.ts

@@ -6,6 +6,7 @@ import fs from "fs/promises"
 import os from "os"
 import pWaitFor from "p-wait-for"
 import * as path from "path"
+import { Terminal } from "../../integrations/terminal/Terminal"
 import * as vscode from "vscode"
 
 import { setPanel } from "../../activate/registerCommands"
@@ -355,9 +356,10 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 			setPanel(webviewView, "sidebar")
 		}
 
-		// Initialize sound enabled state
-		this.getState().then(({ soundEnabled }) => {
+		// Initialize out-of-scope variables that need to recieve persistent global state values
+		this.getState().then(({ soundEnabled, terminalShellIntegrationTimeout }) => {
 			setSoundEnabled(soundEnabled ?? false)
+			Terminal.setShellIntegrationTimeout(terminalShellIntegrationTimeout ?? 4000)
 		})
 
 		// Initialize tts enabled state
@@ -1403,6 +1405,13 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 						await this.updateGlobalState("terminalOutputLineLimit", message.value)
 						await this.postStateToWebview()
 						break
+					case "terminalShellIntegrationTimeout":
+						await this.updateGlobalState("terminalShellIntegrationTimeout", message.value)
+						await this.postStateToWebview()
+						if (message.value !== undefined) {
+							Terminal.setShellIntegrationTimeout(message.value)
+						}
+						break
 					case "mode":
 						await this.handleModeSwitch(message.text as Mode)
 						break
@@ -2369,6 +2378,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 			remoteBrowserEnabled,
 			writeDelayMs,
 			terminalOutputLineLimit,
+			terminalShellIntegrationTimeout,
 			fuzzyMatchThreshold,
 			mcpEnabled,
 			enableMcpServerCreation,
@@ -2432,6 +2442,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 			remoteBrowserEnabled: remoteBrowserEnabled ?? false,
 			writeDelayMs: writeDelayMs ?? 1000,
 			terminalOutputLineLimit: terminalOutputLineLimit ?? 500,
+			terminalShellIntegrationTimeout: terminalShellIntegrationTimeout ?? 4000,
 			fuzzyMatchThreshold: fuzzyMatchThreshold ?? 1.0,
 			mcpEnabled: mcpEnabled ?? true,
 			enableMcpServerCreation: enableMcpServerCreation ?? true,
@@ -2591,6 +2602,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 			fuzzyMatchThreshold: stateValues.fuzzyMatchThreshold ?? 1.0,
 			writeDelayMs: stateValues.writeDelayMs ?? 1000,
 			terminalOutputLineLimit: stateValues.terminalOutputLineLimit ?? 500,
+			terminalShellIntegrationTimeout: stateValues.terminalShellIntegrationTimeout ?? 4000,
 			mode: stateValues.mode ?? defaultModeSlug,
 			language: stateValues.language ?? formatLanguage(vscode.env.language),
 			mcpEnabled: stateValues.mcpEnabled ?? true,

src/core/webview/ClineProvider.ts

@@ -220,6 +220,7 @@ export type GlobalStateKey =
 	| "fuzzyMatchThreshold"
 	| "writeDelayMs"
 	| "terminalOutputLineLimit"
+	| "terminalShellIntegrationTimeout"
 	| "mcpEnabled"
 	| "enableMcpServerCreation"
 	| "alwaysApproveResubmit"

src/exports/roo-code.d.ts

@@ -4,6 +4,8 @@ import { ExitCodeDetails, mergePromise, TerminalProcess, TerminalProcessResultPr
 import { truncateOutput, applyRunLengthEncoding } from "../misc/extract-text"
 
 export class Terminal {
+	private static shellIntegrationTimeout: number = 4000
+
 	public terminal: vscode.Terminal
 	public busy: boolean
 	public id: number
@@ -57,16 +59,18 @@ export class Terminal {
 		if (stream) {
 			// New stream is available
 			if (!this.process) {
-				throw new Error(`Cannot set active stream on terminal ${this.id} because process is undefined`)
+				this.running = false
+				console.warn(
+					`[Terminal ${this.id}] process is undefined, so cannot set terminal stream (probably user-initiated non-Roo command)`,
+				)
+				return
 			}
 
 			this.streamClosed = false
-			this.running = true
 			this.process.emit("stream_available", stream)
 		} else {
 			// Stream is being closed
 			this.streamClosed = true
-			this.running = false
 		}
 	}
 
@@ -75,7 +79,6 @@ export class Terminal {
 	 * @param exitDetails The exit details of the shell execution
 	 */
 	public shellExecutionComplete(exitDetails: ExitCodeDetails): void {
-		this.running = false
 		this.busy = false
 
 		if (this.process) {
@@ -149,6 +152,9 @@ export class Terminal {
 	}
 
 	public runCommand(command: string): TerminalProcessResultPromise {
+		// We set busy before the command is running because the terminal may be waiting
+		// on terminal integration, and we must prevent another instance from selecting
+		// the terminal for use during that time.
 		this.busy = true
 
 		// Create process immediately
@@ -165,20 +171,20 @@ export class Terminal {
 			// Set up event handlers
 			process.once("continue", () => resolve())
 			process.once("error", (error) => {
-				console.error(`Error in terminal ${this.id}:`, error)
+				console.error(`[Terminal ${this.id}] error:`, error)
 				reject(error)
 			})
 
 			// Wait for shell integration before executing the command
-			pWaitFor(() => this.terminal.shellIntegration !== undefined, { timeout: 4000 })
+			pWaitFor(() => this.terminal.shellIntegration !== undefined, { timeout: Terminal.shellIntegrationTimeout })
 				.then(() => {
 					process.run(command)
 				})
 				.catch(() => {
-					console.log("[Terminal] Shell integration not available. Command execution aborted.")
+					console.log(`[Terminal ${this.id}] Shell integration not available. Command execution aborted.`)
 					process.emit(
 						"no_shell_integration",
-						"Shell integration initialization sequence '\\x1b]633;A' was not received within 4 seconds. Shell integration has been disabled for this terminal instance.",
+						"Shell integration initialization sequence '\\x1b]633;A' was not received within 4 seconds. Shell integration has been disabled for this terminal instance. Increase the timeout in the settings if necessary.",
 					)
 				})
 		})
@@ -244,6 +250,10 @@ export class Terminal {
 	 * @param input The terminal output to compress
 	 * @returns The compressed terminal output
 	 */
+	public static setShellIntegrationTimeout(timeoutMs: number): void {
+		Terminal.shellIntegrationTimeout = timeoutMs
+	}
+
 	public static compressTerminalOutput(input: string, lineLimit: number): string {
 		return truncateOutput(applyRunLengthEncoding(input), lineLimit)
 	}

src/integrations/terminal/Terminal.ts

@@ -285,7 +285,7 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 					(defaultWindowsShellProfile as string)?.toLowerCase().includes("powershell"))
 			if (isPowerShell) {
 				terminal.shellIntegration.executeCommand(
-					`${command} ; ${this.terminalInfo.cmdCounter++} > $null; start-sleep -milliseconds 150`,
+					`${command} ; "(Roo/PS Workaround: ${this.terminalInfo.cmdCounter++})" > $null; start-sleep -milliseconds 150`,
 				)
 			} else {
 				terminal.shellIntegration.executeCommand(command)
@@ -306,10 +306,7 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 					"<VSCE shell integration stream did not start: terminal output and command execution status is unknown>",
 				)
 
-				// Ensure terminal is marked as not busy
-				if (this.terminalInfo) {
-					this.terminalInfo.busy = false
-				}
+				this.terminalInfo.busy = false
 
 				// Emit continue event to allow execution to proceed
 				this.emit("continue")

src/integrations/terminal/TerminalProcess.ts

@@ -24,17 +24,22 @@ export class TerminalRegistry {
 					// Get a handle to the stream as early as possible:
 					const stream = e?.execution.read()
 					const terminalInfo = this.getTerminalByVSCETerminal(e.terminal)
-					if (terminalInfo) {
-						terminalInfo.setActiveStream(stream)
-					} else {
-						console.error("[TerminalRegistry] Stream failed, not registered for terminal")
-					}
 
 					console.info("[TerminalRegistry] Shell execution started:", {
 						hasExecution: !!e?.execution,
 						command: e?.execution?.commandLine?.value,
 						terminalId: terminalInfo?.id,
 					})
+
+					if (terminalInfo) {
+						terminalInfo.running = true
+						terminalInfo.setActiveStream(stream)
+					} else {
+						console.error(
+							"[TerminalRegistry] Shell execution started, but not from a Roo-registered terminal:",
+							e,
+						)
+					}
 				},
 			)
 
@@ -44,10 +49,20 @@ export class TerminalRegistry {
 					const terminalInfo = this.getTerminalByVSCETerminal(e.terminal)
 					const process = terminalInfo?.process
 
+					const exitDetails = TerminalProcess.interpretExitCode(e?.exitCode)
+
+					console.info("[TerminalRegistry] Shell execution ended:", {
+						hasExecution: !!e?.execution,
+						command: e?.execution?.commandLine?.value,
+						terminalId: terminalInfo?.id,
+						...exitDetails,
+					})
+
 					if (!terminalInfo) {
-						console.error("[TerminalRegistry] Shell execution ended but terminal not found:", {
-							exitCode: e?.exitCode,
-						})
+						console.error(
+							"[TerminalRegistry] Shell execution ended, but not from a Roo-registered terminal:",
+							e,
+						)
 						return
 					}
 
@@ -74,15 +89,9 @@ export class TerminalRegistry {
 						return
 					}
 
-					const exitDetails = TerminalProcess.interpretExitCode(e?.exitCode)
-					console.info("[TerminalRegistry] Shell execution ended:", {
-						...exitDetails,
-						terminalId: terminalInfo.id,
-						command: process?.command ?? "<unknown>",
-					})
-
 					// Signal completion to any waiting processes
 					if (terminalInfo) {
+						terminalInfo.running = false
 						terminalInfo.shellExecutionComplete(exitDetails)
 					}
 				},

src/integrations/terminal/TerminalRegistry.ts

@@ -138,6 +138,7 @@ export interface ExtensionState {
 	language?: string
 	writeDelayMs: number
 	terminalOutputLineLimit?: number
+	terminalShellIntegrationTimeout?: number
 	mcpEnabled: boolean
 	enableMcpServerCreation: boolean
 	enableCustomModeCreation?: boolean

src/shared/ExtensionMessage.ts

@@ -74,6 +74,7 @@ export interface WebviewMessage {
 		| "draggedImages"
 		| "deleteMessage"
 		| "terminalOutputLineLimit"
+		| "terminalShellIntegrationTimeout"
 		| "mcpEnabled"
 		| "enableMcpServerCreation"
 		| "enableCustomModeCreation"