From 6301e904542a20f609b9234000de213265ae8a2b Mon Sep 17 00:00:00 2001 From: KJ7LNW <93454819+KJ7LNW@users.noreply.github.com> Date: Mon, 17 Mar 2025 23:57:20 -0700 Subject: [PATCH] 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 * 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 * 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 * 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 * 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 * 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 * test: update snapshots for system prompt working directory instructions Signed-off-by: Eric Wheeler --------- Signed-off-by: Eric Wheeler Co-authored-by: Eric Wheeler Co-authored-by: Chris Estreich --- src/core/Cline.ts | 15 +++++-- .../__snapshots__/system.test.ts.snap | 45 ++++++++++++------- src/core/prompts/sections/rules.ts | 3 +- src/core/webview/ClineProvider.ts | 16 ++++++- src/exports/roo-code.d.ts | 1 + src/integrations/terminal/Terminal.ts | 26 +++++++---- src/integrations/terminal/TerminalProcess.ts | 7 +-- src/integrations/terminal/TerminalRegistry.ts | 39 +++++++++------- src/shared/ExtensionMessage.ts | 1 + src/shared/WebviewMessage.ts | 1 + src/shared/globalState.ts | 1 + .../components/settings/AdvancedSettings.tsx | 36 ++++++++++++++- .../src/components/settings/SettingsView.tsx | 3 ++ .../src/context/ExtensionStateContext.tsx | 5 +++ 14 files changed, 149 insertions(+), 50 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index 246fc8518e..588195524e 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -1033,7 +1033,7 @@ export class Cline extends EventEmitter { ), ] } 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 { result += "" exitStatus = `Exit code: ` } 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 += "" exitStatus = `Exit code: ` } - 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 [ diff --git a/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap b/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap index 69b3a0b55b..c77816d99e 100644 --- a/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap +++ b/src/core/prompts/__tests__/__snapshots__/system.test.ts.snap @@ -270,7 +270,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -689,7 +690,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -1077,7 +1079,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -1414,7 +1417,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -1748,7 +1752,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -2082,7 +2087,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -2464,7 +2470,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -3253,7 +3260,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -3635,7 +3643,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -4030,7 +4039,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -4366,7 +4376,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -4889,7 +4900,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -5301,7 +5313,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -5589,7 +5602,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. @@ -6495,7 +6509,8 @@ MODES RULES -- Your current working directory is: /test/path +- The project base directory is: /test/path +- 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 . - You cannot \`cd\` into a different directory to complete a task. You are stuck operating from '/test/path', 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 '/test/path', and if so prepend with \`cd\`'ing into that directory && then executing the command (as one command since you are stuck operating from '/test/path'). For example, if you needed to run \`npm install\` in a project outside of '/test/path', 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)\`. diff --git a/src/core/prompts/sections/rules.ts b/src/core/prompts/sections/rules.ts index 86e554a157..a68017970c 100644 --- a/src/core/prompts/sections/rules.ts +++ b/src/core/prompts/sections/rules.ts @@ -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 . - 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)\`. diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 050f63d195..d71f7a65e5 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.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 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 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 implements remoteBrowserEnabled, writeDelayMs, terminalOutputLineLimit, + terminalShellIntegrationTimeout, fuzzyMatchThreshold, mcpEnabled, enableMcpServerCreation, @@ -2432,6 +2442,7 @@ export class ClineProvider extends EventEmitter 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 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, diff --git a/src/exports/roo-code.d.ts b/src/exports/roo-code.d.ts index 5cc9df16bd..daa147add4 100644 --- a/src/exports/roo-code.d.ts +++ b/src/exports/roo-code.d.ts @@ -220,6 +220,7 @@ export type GlobalStateKey = | "fuzzyMatchThreshold" | "writeDelayMs" | "terminalOutputLineLimit" + | "terminalShellIntegrationTimeout" | "mcpEnabled" | "enableMcpServerCreation" | "alwaysApproveResubmit" diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 531300aab9..d0cc660a6a 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.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) } diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index 468bfebabb..21d6557715 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -285,7 +285,7 @@ export class TerminalProcess extends EventEmitter { (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 { "", ) - // 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") diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index 13c11cc9e1..dcf1af76d4 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/src/integrations/terminal/TerminalRegistry.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 ?? "", - }) - // Signal completion to any waiting processes if (terminalInfo) { + terminalInfo.running = false terminalInfo.shellExecutionComplete(exitDetails) } }, diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 3ec2853fef..e88027b64d 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -138,6 +138,7 @@ export interface ExtensionState { language?: string writeDelayMs: number terminalOutputLineLimit?: number + terminalShellIntegrationTimeout?: number mcpEnabled: boolean enableMcpServerCreation: boolean enableCustomModeCreation?: boolean diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 1a54d390ff..60a43663fc 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -74,6 +74,7 @@ export interface WebviewMessage { | "draggedImages" | "deleteMessage" | "terminalOutputLineLimit" + | "terminalShellIntegrationTimeout" | "mcpEnabled" | "enableMcpServerCreation" | "enableCustomModeCreation" diff --git a/src/shared/globalState.ts b/src/shared/globalState.ts index e1c1f672b3..4c0adb0fcf 100644 --- a/src/shared/globalState.ts +++ b/src/shared/globalState.ts @@ -88,6 +88,7 @@ export const GLOBAL_STATE_KEYS = [ "fuzzyMatchThreshold", "writeDelayMs", "terminalOutputLineLimit", + "terminalShellIntegrationTimeout", "mcpEnabled", "enableMcpServerCreation", "alwaysApproveResubmit", diff --git a/webview-ui/src/components/settings/AdvancedSettings.tsx b/webview-ui/src/components/settings/AdvancedSettings.tsx index e217537ab6..a03676ad9f 100644 --- a/webview-ui/src/components/settings/AdvancedSettings.tsx +++ b/webview-ui/src/components/settings/AdvancedSettings.tsx @@ -14,14 +14,18 @@ import { Section } from "./Section" type AdvancedSettingsProps = HTMLAttributes & { rateLimitSeconds: number + terminalShellIntegrationTimeout: number | undefined diffEnabled?: boolean fuzzyMatchThreshold?: number - setCachedStateField: SetCachedStateField<"rateLimitSeconds" | "diffEnabled" | "fuzzyMatchThreshold"> + setCachedStateField: SetCachedStateField< + "rateLimitSeconds" | "diffEnabled" | "fuzzyMatchThreshold" | "terminalShellIntegrationTimeout" + > experiments: Record setExperimentEnabled: SetExperimentEnabled } export const AdvancedSettings = ({ rateLimitSeconds, + terminalShellIntegrationTimeout, diffEnabled, fuzzyMatchThreshold, setCachedStateField, @@ -62,6 +66,36 @@ export const AdvancedSettings = ({

+
+
+ Terminal shell integration timeout +
+ + setCachedStateField( + "terminalShellIntegrationTimeout", + Math.min(60000, Math.max(1000, parseInt(e.target.value))), + ) + } + className="h-2 focus:outline-0 w-4/5 accent-vscode-button-background" + /> + + {(terminalShellIntegrationTimeout ?? 4000) / 1000}s + +
+

+ Maximum time to wait for shell integration to initialize before executing commands. For + users with long shell startup times, this value may need to be increased if you see "Shell + Integration Unavailable" errors in the terminal. +

+
+
+
(({ onDone }, soundVolume, telemetrySetting, terminalOutputLineLimit, + terminalShellIntegrationTimeout, writeDelayMs, showRooIgnoredFiles, remoteBrowserEnabled, @@ -200,6 +201,7 @@ const SettingsView = forwardRef(({ onDone }, vscode.postMessage({ type: "writeDelayMs", value: writeDelayMs }) vscode.postMessage({ type: "screenshotQuality", value: screenshotQuality ?? 75 }) vscode.postMessage({ type: "terminalOutputLineLimit", value: terminalOutputLineLimit ?? 500 }) + vscode.postMessage({ type: "terminalShellIntegrationTimeout", value: terminalShellIntegrationTimeout }) vscode.postMessage({ type: "mcpEnabled", bool: mcpEnabled }) vscode.postMessage({ type: "alwaysApproveResubmit", bool: alwaysApproveResubmit }) vscode.postMessage({ type: "requestDelaySeconds", value: requestDelaySeconds }) @@ -455,6 +457,7 @@ const SettingsView = forwardRef(({ onDone },
void setSoundEnabled: (value: boolean) => void setSoundVolume: (value: number) => void + terminalShellIntegrationTimeout?: number + setTerminalShellIntegrationTimeout: (value: number) => void setTtsEnabled: (value: boolean) => void setTtsSpeed: (value: number) => void setDiffEnabled: (value: boolean) => void @@ -129,6 +131,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode browserViewportSize: "900x600", screenshotQuality: 75, terminalOutputLineLimit: 500, + terminalShellIntegrationTimeout: 4000, mcpEnabled: true, enableMcpServerCreation: true, alwaysApproveResubmit: false, @@ -273,6 +276,8 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode setScreenshotQuality: (value) => setState((prevState) => ({ ...prevState, screenshotQuality: value })), setTerminalOutputLineLimit: (value) => setState((prevState) => ({ ...prevState, terminalOutputLineLimit: value })), + setTerminalShellIntegrationTimeout: (value) => + setState((prevState) => ({ ...prevState, terminalShellIntegrationTimeout: value })), setMcpEnabled: (value) => setState((prevState) => ({ ...prevState, mcpEnabled: value })), setEnableMcpServerCreation: (value) => setState((prevState) => ({ ...prevState, enableMcpServerCreation: value })),