scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-09-06 08:18:39 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor: improve terminal working directory handling to prevent proliferation

Browse source 
- Modified TerminalRegistry.getOrCreateTerminal() to prioritize task-specific terminal reuse
- Only create new terminals when requiredCwd=true and directories don't match
- Enhanced executeCommandTool to report current working directory changes to the model
- Updated tests to reflect new behavior that prevents excessive terminal creation
- Addresses feedback about terminal proliferation while maintaining directory tracking
This commit is contained in:
Roo Code 2025-07-18 17:33:32 +00:00
parent 156698f965
commit 5778565ac3
3 changed files with 56 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
src/core/tools/executeCommandTool.ts
View file

@ -11,6 +11,7 @@ import { Task } from "../task/Task"
import { ToolUse, AskApproval, HandleError, PushToolResult, RemoveClosingTag, ToolResponse } from "../../shared/tools"
import { formatResponse } from "../prompts/responses"
import { unescapeHtmlEntities } from "../../utils/text-normalization"
import { arePathsEqual } from "../../utils/path"
import { ExitCodeDetails, RooTerminalCallbacks, RooTerminalProcess } from "../../integrations/terminal/types"
import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
import { Terminal } from "../../integrations/terminal/Terminal"
@ -269,7 +270,13 @@ export async function executeCommand(
let workingDirInfo = ` within working directory '${workingDir.toPosix()}'`
const newWorkingDir = terminal.getCurrentWorkingDirectory()
return [false, `Command executed in terminal ${workingDirInfo}. ${exitStatus}\nOutput:\n${result}`]
// Include current working directory information if it has changed
let currentDirInfo = ""
if (!arePathsEqual(workingDir, newWorkingDir)) {
currentDirInfo = `\nCurrent working directory: '${newWorkingDir.toPosix()}'`
}
return [false, `Command executed in terminal ${workingDirInfo}. ${exitStatus}${currentDirInfo}\nOutput:\n${result}`]
} else {
return [
false,

32
src/integrations/terminal/TerminalRegistry.ts
View file

@ -159,27 +159,30 @@ export class TerminalRegistry {
const terminals = this.getAllTerminals()
let terminal: RooTerminal | undefined
// First priority: Find a terminal already assigned to this task with
// matching directory.
// First priority: Find a terminal already assigned to this task.
if (taskId) {
terminal = terminals.find((t) => {
if (t.busy || t.taskId !== taskId || t.provider !== provider) {
return false
}
const terminalCwd = t.getCurrentWorkingDirectory()
if (!terminalCwd) {
return false
// If directory is required, check if current working directory matches
if (requiredCwd) {
const terminalCwd = t.getCurrentWorkingDirectory()
if (!terminalCwd) {
return false
}
return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd)
}
return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd)
// If directory is not required, reuse the task terminal regardless of current directory
return true
})
}
// Second priority: Find any available terminal with matching directory
// that is not assigned to a different task.
if (!terminal) {
if (!terminal && requiredCwd) {
terminal = terminals.find((t) => {
if (t.busy || t.provider !== provider) {
return false
@ -200,9 +203,7 @@ export class TerminalRegistry {
})
}
// Third priority: Find any non-busy terminal (only if directory is not
// required AND no terminals exist with different working directories).
// This prevents reusing terminals that have changed their working directory.
// Third priority: Find any non-busy terminal (only if directory is not required).
if (!terminal && !requiredCwd) {
terminal = terminals.find((t) => {
if (t.busy || t.provider !== provider) {
@ -214,14 +215,7 @@ export class TerminalRegistry {
return false
}
// Only reuse terminals that are still in the requested working directory
// or have no shell integration (fallback to initial CWD)
const terminalCwd = t.getCurrentWorkingDirectory()
if (!terminalCwd) {
return false
}
return arePathsEqual(vscode.Uri.file(cwd).fsPath, terminalCwd)
return true
})
}

74
src/integrations/terminal/__tests__/TerminalRegistry.workingDirectory.spec.ts
View file

@ -44,7 +44,7 @@ describe("TerminalRegistry - Working Directory Tracking", () => {
})
describe("getOrCreateTerminal with changed working directory", () => {
it("should reuse terminal when working directory matches current directory", async () => {
it("should reuse task terminal regardless of current working directory when requiredCwd is false", async () => {
// Create a terminal with initial working directory
const terminal1 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, "task1", "vscode")
@ -61,15 +61,15 @@ describe("TerminalRegistry - Working Directory Tracking", () => {
// Mark terminal as not busy
terminal1.busy = false
// Request a terminal for the new working directory
const terminal2 = await TerminalRegistry.getOrCreateTerminal("/test/path/subdir", false, "task1", "vscode")
// Request a terminal for a different working directory but same task
const terminal2 = await TerminalRegistry.getOrCreateTerminal("/test/path/other", false, "task1", "vscode")
// Should reuse the same terminal since its current working directory matches
// Should reuse the same terminal since it's the same task and requiredCwd is false
expect(terminal2).toBe(terminal1)
expect(mockCreateTerminal).toHaveBeenCalledTimes(1) // Only one terminal created
})
it("should create new terminal when no existing terminal matches current working directory", async () => {
it("should create new terminal when requiredCwd is true and current working directory doesn't match", async () => {
// Create a terminal with initial working directory
const terminal1 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, "task1", "vscode")
@ -86,14 +86,39 @@ describe("TerminalRegistry - Working Directory Tracking", () => {
// Mark terminal as not busy
terminal1.busy = false
// Request a terminal for a different working directory
const terminal2 = await TerminalRegistry.getOrCreateTerminal("/test/path/other", false, "task1", "vscode")
// Request a terminal for a different working directory with requiredCwd=true
const terminal2 = await TerminalRegistry.getOrCreateTerminal("/test/path/other", true, "task1", "vscode")
// Should create a new terminal since no existing terminal matches the requested directory
// Should create a new terminal since requiredCwd is true and directories don't match
expect(terminal2).not.toBe(terminal1)
expect(mockCreateTerminal).toHaveBeenCalledTimes(2) // Two terminals created
})
it("should reuse terminal when requiredCwd is true and current working directory matches", async () => {
// Create a terminal with initial working directory
const terminal1 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, "task1", "vscode")
// Simulate the terminal's working directory changing (like after cd command)
if (terminal1 instanceof Terminal) {
// Mock the shell integration to return the new working directory
Object.defineProperty(terminal1.terminal.shellIntegration!, "cwd", {
value: vscode.Uri.file("/test/path/subdir"),
writable: true,
configurable: true,
})
}
// Mark terminal as not busy
terminal1.busy = false
// Request a terminal for the same working directory with requiredCwd=true
const terminal2 = await TerminalRegistry.getOrCreateTerminal("/test/path/subdir", true, "task1", "vscode")
// Should reuse the same terminal since directories match
expect(terminal2).toBe(terminal1)
expect(mockCreateTerminal).toHaveBeenCalledTimes(1) // Only one terminal created
})
it("should handle terminals without shell integration gracefully", async () => {
// Create a terminal without shell integration
mockCreateTerminal.mockImplementationOnce(
@ -118,43 +143,14 @@ describe("TerminalRegistry - Working Directory Tracking", () => {
const terminal1 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, "task1", "vscode")
terminal1.busy = false
// Request a terminal for the same working directory
// Request a terminal for the same task
const terminal2 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, "task1", "vscode")
// Should reuse the same terminal since it falls back to initial CWD
// Should reuse the same terminal since it's the same task
expect(terminal2).toBe(terminal1)
expect(mockCreateTerminal).toHaveBeenCalledTimes(1)
})
it("should prioritize task-specific terminals with matching current working directory", async () => {
// Create a terminal for task1
const terminal1 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, "task1", "vscode")
// Create a terminal for task2
const terminal2 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, "task2", "vscode")
// Simulate terminal1's working directory changing
if (terminal1 instanceof Terminal) {
// Mock the shell integration to return the new working directory
Object.defineProperty(terminal1.terminal.shellIntegration!, "cwd", {
value: vscode.Uri.file("/test/path/subdir"),
writable: true,
configurable: true,
})
}
// Mark both terminals as not busy
terminal1.busy = false
terminal2.busy = false
// Request a terminal for task1 with the new working directory
const terminal3 = await TerminalRegistry.getOrCreateTerminal("/test/path/subdir", false, "task1", "vscode")
// Should reuse terminal1 since it's assigned to task1 and has matching current working directory
expect(terminal3).toBe(terminal1)
expect(mockCreateTerminal).toHaveBeenCalledTimes(2) // Only two terminals created
})
it("should create separate terminals for different tasks", async () => {
// Create a terminal for task1
const terminal1 = await TerminalRegistry.getOrCreateTerminal("/test/path", false, "task1", "vscode")