mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-09 03:17:58 +00:00
fix: remove forced directory changes that break shell integration
Forcing terminals to `cd` back to the project directory was disrupting shell state without providing feedback to the model. This caused issues with capturing output from subsequent commands, particularly with custom shell prompts. Instead of forcing directory changes, we now track terminal state through shell integration with a fallback mechanism, and provide explicit working directory feedback to the model. This allows terminals to maintain their natural state while ensuring accurate command output capture. Changes: - Remove forced `cd` commands that were disrupting terminal state - Add getCurrentWorkingDirectory() method with shell integration fallback - Add customCwd parameter to executeCommandTool for flexible directory handling - Add requiredCwd parameter to control terminal selection behavior - Refactor terminal selection logic for more consistent state management - Modify environment details to include terminal working directory feedback - Update XML schema to include optional working directory parameter in execute_command The environment details now provide explicit feedback about terminal state: Command executed in terminal N from '/path/to/dir'. Exit code: 0 Fixes: #1388 Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
This commit is contained in:
parent
e27c6aadc1
commit
1a1432d1d1
5 changed files with 99 additions and 42 deletions
|
|
@ -919,8 +919,31 @@ export class Cline {
|
|||
|
||||
// Tools
|
||||
|
||||
async executeCommandTool(command: string): Promise<[boolean, ToolResponse]> {
|
||||
const terminalInfo = await TerminalRegistry.getOrCreateTerminal(cwd, this.taskId)
|
||||
async executeCommandTool(command: string, customCwd?: string): Promise<[boolean, ToolResponse]> {
|
||||
let workingDir: string
|
||||
if (!customCwd) {
|
||||
workingDir = cwd
|
||||
} else if (path.isAbsolute(customCwd)) {
|
||||
workingDir = customCwd
|
||||
} else {
|
||||
workingDir = path.resolve(cwd, customCwd)
|
||||
}
|
||||
|
||||
// Check if directory exists
|
||||
try {
|
||||
await fs.access(workingDir)
|
||||
} catch (error) {
|
||||
return [false, `Working directory '${workingDir}' does not exist.`]
|
||||
}
|
||||
|
||||
const terminalInfo = await TerminalRegistry.getOrCreateTerminal(workingDir, !!customCwd, this.taskId)
|
||||
|
||||
// Update the working directory in case the terminal we asked for has
|
||||
// a different working directory so that the model will know where the
|
||||
// command actually executed:
|
||||
workingDir = terminalInfo.getCurrentWorkingDirectory()
|
||||
|
||||
const workingDirInfo = workingDir ? ` from '${workingDir.toPosix()}'` : ""
|
||||
terminalInfo.terminal.show() // weird visual bug when creating new terminals (even manually) where there's an empty space at the top.
|
||||
const process = terminalInfo.runCommand(command)
|
||||
|
||||
|
|
@ -989,7 +1012,7 @@ export class Cline {
|
|||
return [
|
||||
true,
|
||||
formatResponse.toolResult(
|
||||
`Command is still running in the user's terminal.${
|
||||
`Command is still running in terminal ${terminalInfo.id}${workingDirInfo}.${
|
||||
result.length > 0 ? `\nHere's the output so far:\n${result}` : ""
|
||||
}\n\nThe user provided the following feedback:\n<feedback>\n${userFeedback.text}\n</feedback>`,
|
||||
userFeedback.images,
|
||||
|
|
@ -1009,11 +1032,18 @@ export class Cline {
|
|||
exitStatus = `Exit code: ${exitDetails.exitCode}`
|
||||
}
|
||||
}
|
||||
return [false, `Command executed. ${exitStatus}${result.length > 0 ? `\nOutput:\n${result}` : ""}`]
|
||||
const workingDirInfo = workingDir ? ` from '${workingDir.toPosix()}'` : ""
|
||||
|
||||
const outputInfo = `\nOutput:\n${result}`
|
||||
|
||||
return [
|
||||
false,
|
||||
`Command executed in terminal ${terminalInfo.id}${workingDirInfo}. ${exitStatus}${outputInfo}`,
|
||||
]
|
||||
} else {
|
||||
return [
|
||||
false,
|
||||
`Command is still running in the user's terminal.${
|
||||
`Command is still running in terminal ${terminalInfo.id}${workingDirInfo}.${
|
||||
result.length > 0 ? `\nHere's the output so far:\n${result}` : ""
|
||||
}\n\nYou will be updated on the terminal status and new output in the future.`,
|
||||
]
|
||||
|
|
@ -2494,6 +2524,7 @@ export class Cline {
|
|||
}
|
||||
case "execute_command": {
|
||||
const command: string | undefined = block.params.command
|
||||
const customCwd: string | undefined = block.params.cwd
|
||||
try {
|
||||
if (block.partial) {
|
||||
await this.ask("command", removeClosingTag("command", command), block.partial).catch(
|
||||
|
|
@ -2527,7 +2558,7 @@ export class Cline {
|
|||
if (!didApprove) {
|
||||
break
|
||||
}
|
||||
const [userRejected, result] = await this.executeCommandTool(command)
|
||||
const [userRejected, result] = await this.executeCommandTool(command, customCwd)
|
||||
if (userRejected) {
|
||||
this.didRejectTool = true
|
||||
}
|
||||
|
|
|
|||
|
|
@ -56,6 +56,7 @@ export const toolParamNames = [
|
|||
"operations",
|
||||
"mode",
|
||||
"message",
|
||||
"cwd",
|
||||
] as const
|
||||
|
||||
export type ToolParamName = (typeof toolParamNames)[number]
|
||||
|
|
@ -71,7 +72,7 @@ export interface ToolUse {
|
|||
export interface ExecuteCommandToolUse extends ToolUse {
|
||||
name: "execute_command"
|
||||
// Pick<Record<ToolParamName, string>, "command"> makes "command" required, but Partial<> makes it optional
|
||||
params: Partial<Pick<Record<ToolParamName, string>, "command">>
|
||||
params: Partial<Pick<Record<ToolParamName, string>, "command" | "cwd">>
|
||||
}
|
||||
|
||||
export interface ReadFileToolUse extends ToolUse {
|
||||
|
|
|
|||
|
|
@ -2,16 +2,24 @@ import { ToolArgs } from "./types"
|
|||
|
||||
export function getExecuteCommandDescription(args: ToolArgs): string | undefined {
|
||||
return `## execute_command
|
||||
Description: Request to execute a CLI command on the system. Use this when you need to perform system operations or run specific commands to accomplish any step in the user's task. You must tailor your command to the user's system and provide a clear explanation of what the command does. For command chaining, use the appropriate chaining syntax for the user's shell. Prefer to execute complex CLI commands over creating executable scripts, as they are more flexible and easier to run. Commands will be executed in the current working directory: ${args.cwd}
|
||||
Description: Request to execute a CLI command on the system. Use this when you need to perform system operations or run specific commands to accomplish any step in the user's task. You must tailor your command to the user's system and provide a clear explanation of what the command does. For command chaining, use the appropriate chaining syntax for the user's shell. Prefer to execute complex CLI commands over creating executable scripts, as they are more flexible and easier to run. Prefer relative commands and paths that avoid location sensitivity for terminal consistency, e.g: \`touch ./testdata/example.file\`, \`dir ./examples/model1/data/yaml\`, or \`go test ./cmd/front --config ./cmd/front/config.yml\`. If directed by the user, you may open a terminal in a different directory by using the \`cwd\` parameter.
|
||||
Parameters:
|
||||
- command: (required) The CLI command to execute. This should be valid for the current operating system. Ensure the command is properly formatted and does not contain any harmful instructions.
|
||||
- cwd: (optional) The working directory to execute the command in (default: ${args.cwd})
|
||||
Usage:
|
||||
<execute_command>
|
||||
<command>Your command here</command>
|
||||
<cwd>Working directory path (optional)</cwd>
|
||||
</execute_command>
|
||||
|
||||
Example: Requesting to execute npm run dev
|
||||
<execute_command>
|
||||
<command>npm run dev</command>
|
||||
</execute_command>
|
||||
|
||||
Example: Requesting to execute ls in a specific directory if directed
|
||||
<execute_command>
|
||||
<command>ls -la</command>
|
||||
<cwd>/home/user/projects</cwd>
|
||||
</execute_command>`
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,13 +12,32 @@ export class Terminal {
|
|||
public process?: TerminalProcess
|
||||
public taskId?: string
|
||||
public completedProcesses: TerminalProcess[] = []
|
||||
private initialCwd: string
|
||||
|
||||
constructor(id: number, terminal: vscode.Terminal) {
|
||||
constructor(id: number, terminal: vscode.Terminal, cwd: string) {
|
||||
this.id = id
|
||||
this.terminal = terminal
|
||||
this.busy = false
|
||||
this.running = false
|
||||
this.streamClosed = false
|
||||
|
||||
// Initial working directory is used as a fallback when
|
||||
// shell integration is not yet initialized or unavailable:
|
||||
this.initialCwd = cwd
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the current working directory from shell integration or falls back to initial cwd
|
||||
* @returns The current working directory
|
||||
*/
|
||||
public getCurrentWorkingDirectory(): string {
|
||||
// Try to get the cwd from shell integration if available
|
||||
if (this.terminal.shellIntegration?.cwd) {
|
||||
return this.terminal.shellIntegration.cwd.fsPath
|
||||
} else {
|
||||
// Fall back to the initial cwd
|
||||
return this.initialCwd
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -68,7 +68,7 @@ export class TerminalRegistry {
|
|||
}
|
||||
}
|
||||
|
||||
static createTerminal(cwd?: string | vscode.Uri | undefined): Terminal {
|
||||
static createTerminal(cwd: string | vscode.Uri): Terminal {
|
||||
const terminal = vscode.window.createTerminal({
|
||||
cwd,
|
||||
name: "Roo Code",
|
||||
|
|
@ -87,7 +87,8 @@ export class TerminalRegistry {
|
|||
},
|
||||
})
|
||||
|
||||
const newTerminal = new Terminal(this.nextTerminalId++, terminal)
|
||||
const cwdString = cwd.toString()
|
||||
const newTerminal = new Terminal(this.nextTerminalId++, terminal, cwdString)
|
||||
|
||||
this.terminals.push(newTerminal)
|
||||
return newTerminal
|
||||
|
|
@ -211,57 +212,54 @@ export class TerminalRegistry {
|
|||
/**
|
||||
* Gets an existing terminal or creates a new one for the given working directory
|
||||
* @param cwd The working directory path
|
||||
* @param requiredCwd Whether the working directory is required (if false, may reuse any non-busy terminal)
|
||||
* @param taskId Optional task ID to associate with the terminal
|
||||
* @returns A Terminal instance
|
||||
*/
|
||||
static async getOrCreateTerminal(cwd: string, taskId?: string): Promise<Terminal> {
|
||||
static async getOrCreateTerminal(cwd: string, requiredCwd: boolean = false, taskId?: string): Promise<Terminal> {
|
||||
const terminals = this.getAllTerminals()
|
||||
let terminal: Terminal | undefined
|
||||
|
||||
// First priority: Find a terminal already assigned to this task with matching directory
|
||||
if (taskId) {
|
||||
const taskTerminal = terminals.find((t) => {
|
||||
terminal = terminals.find((t) => {
|
||||
if (t.busy || t.taskId !== taskId) {
|
||||
return false
|
||||
}
|
||||
const terminalCwd = t.terminal.shellIntegration?.cwd
|
||||
const terminalCwd = t.getCurrentWorkingDirectory()
|
||||
if (!terminalCwd) {
|
||||
return false
|
||||
}
|
||||
return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd.fsPath)
|
||||
return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd)
|
||||
})
|
||||
if (taskTerminal) {
|
||||
return taskTerminal
|
||||
}
|
||||
}
|
||||
|
||||
// Second priority: Find any available terminal with matching directory
|
||||
const matchingTerminal = terminals.find((t) => {
|
||||
if (t.busy) {
|
||||
return false
|
||||
}
|
||||
const terminalCwd = t.terminal.shellIntegration?.cwd // one of cline's commands could have changed the cwd of the terminal
|
||||
if (!terminalCwd) {
|
||||
return false
|
||||
}
|
||||
return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd.fsPath)
|
||||
})
|
||||
if (matchingTerminal) {
|
||||
matchingTerminal.taskId = taskId
|
||||
return matchingTerminal
|
||||
if (!terminal) {
|
||||
terminal = terminals.find((t) => {
|
||||
if (t.busy) {
|
||||
return false
|
||||
}
|
||||
const terminalCwd = t.getCurrentWorkingDirectory()
|
||||
if (!terminalCwd) {
|
||||
return false
|
||||
}
|
||||
return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd)
|
||||
})
|
||||
}
|
||||
|
||||
// Third priority: Find any non-busy terminal
|
||||
const availableTerminal = terminals.find((t) => !t.busy)
|
||||
if (availableTerminal) {
|
||||
// Navigate back to the desired directory
|
||||
await availableTerminal.runCommand(`cd "${cwd}"`)
|
||||
availableTerminal.taskId = taskId
|
||||
return availableTerminal
|
||||
// Third priority: Find any non-busy terminal (only if directory is not required)
|
||||
if (!terminal && !requiredCwd) {
|
||||
terminal = terminals.find((t) => !t.busy)
|
||||
}
|
||||
|
||||
// If all terminals are busy, create a new one
|
||||
const newTerminal = this.createTerminal(cwd)
|
||||
newTerminal.taskId = taskId
|
||||
return newTerminal
|
||||
// If no suitable terminal found, create a new one
|
||||
if (!terminal) {
|
||||
terminal = this.createTerminal(cwd)
|
||||
}
|
||||
|
||||
terminal.taskId = taskId
|
||||
|
||||
return terminal
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue