feat: Improve terminal reuse logic to prevent proliferation

src/integrations/terminal/BaseTerminal.ts

@@ -20,6 +20,7 @@ export abstract class BaseTerminal implements RooTerminal {
 	protected streamClosed: boolean
 
 	public taskId?: string
+	public requestedCwd?: string
 	public process?: RooTerminalProcess
 	public completedProcesses: RooTerminalProcess[] = []
 

src/integrations/terminal/Terminal.ts

@@ -1,6 +1,7 @@
 import * as vscode from "vscode"
 import pWaitFor from "p-wait-for"
 
+import { arePathsEqual } from "../../utils/path"
 import type { RooTerminalCallbacks, RooTerminalProcessResultPromise } from "./types"
 import { BaseTerminal } from "./BaseTerminal"
 import { TerminalProcess } from "./TerminalProcess"
@@ -47,7 +48,14 @@ export class Terminal extends BaseTerminal {
 		this.busy = true
 
 		const process = new TerminalProcess(this)
-		process.command = command
+		const currentCwd = this.getCurrentWorkingDirectory()
+		let commandToRun = command
+
+		if (this.requestedCwd && !arePathsEqual(this.requestedCwd, currentCwd)) {
+			// Wrap path in quotes to handle spaces
+			commandToRun = `cd "${this.requestedCwd}" && ${command}`
+		}
+		process.command = commandToRun
 		this.process = process
 
 		// Set up event handlers from callbacks before starting process.
@@ -76,7 +84,7 @@ export class Terminal extends BaseTerminal {
 					ShellIntegrationManager.zshCleanupTmpDir(this.id)
 
 					// Run the command in the terminal
-					process.run(command)
+					process.run(commandToRun)
 				})
 				.catch(() => {
 					console.log(`[Terminal ${this.id}] Shell integration not available. Command execution aborted.`)

src/integrations/terminal/TerminalRegistry.ts

@@ -192,12 +192,22 @@ export class TerminalRegistry {
 			})
 		}
 
+		// Third priority: find any idle terminal with the same provider,
+		// preferably one with the same task ID.
+		if (!terminal) {
+			const idleTerminals = terminals.filter((t) => !t.busy && t.provider === provider)
+			if (idleTerminals.length > 0) {
+				terminal = idleTerminals.find((t) => t.taskId === taskId) ?? idleTerminals[0]
+			}
+		}
+
 		// If no suitable terminal found, create a new one.
 		if (!terminal) {
 			terminal = this.createTerminal(cwd, provider)
 		}
 
 		terminal.taskId = taskId
+		terminal.requestedCwd = cwd
 
 		return terminal
 	}

src/integrations/terminal/__tests__/Terminal.spec.ts

@@ -0,0 +1,112 @@
+// npx vitest run src/integrations/terminal/__tests__/Terminal.spec.ts
+
+import * as vscode from "vscode"
+import { Terminal } from "../Terminal"
+import { TerminalProcess } from "../TerminalProcess"
+import { vi } from "vitest"
+
+// Mock dependencies
+vi.mock("vscode", () => ({
+	window: {
+		createTerminal: vi.fn().mockReturnValue({
+			shellIntegration: {
+				cwd: { fsPath: "/initial/cwd" },
+			},
+			sendText: vi.fn(),
+			exitStatus: undefined,
+		}),
+	},
+	ThemeIcon: vi.fn(),
+}))
+
+vi.mock("p-wait-for", () => ({
+	default: vi.fn().mockResolvedValue(true),
+}))
+
+vi.mock("../TerminalProcess")
+vi.mock("../../../utils/path", () => ({
+	arePathsEqual: vi.fn((a, b) => a === b),
+}))
+
+describe("Terminal", () => {
+	describe("runCommand", () => {
+		let terminal: Terminal
+		let mockRun: any
+		let mockProcess: any
+
+		beforeEach(() => {
+			terminal = new Terminal(1, undefined, "/initial/cwd")
+			mockRun = vi.fn()
+			mockProcess = {
+				run: mockRun,
+				on: vi.fn(),
+				once: vi.fn(),
+				emit: vi.fn(),
+			}
+			vi.mocked(TerminalProcess).mockImplementation((): any => {
+				return mockProcess
+			})
+		})
+
+		afterEach(() => {
+			vi.clearAllMocks()
+		})
+
+		it("should prepend cd command if requestedCwd differs from currentCwd", async () => {
+			terminal.requestedCwd = "/requested/cwd"
+			vi.spyOn(terminal, "getCurrentWorkingDirectory").mockReturnValue("/initial/cwd")
+
+			const callbacks = {
+				onLine: vi.fn(),
+				onCompleted: vi.fn(),
+				onShellExecutionStarted: vi.fn(),
+				onShellExecutionComplete: vi.fn(),
+			}
+
+			terminal.runCommand("ls", callbacks)
+
+			// Wait for the async operation to complete
+			await vi.waitFor(() => {
+				expect(mockProcess.command).toBe('cd "/requested/cwd" && ls')
+				expect(mockRun).toHaveBeenCalledWith('cd "/requested/cwd" && ls')
+			})
+		})
+
+		it("should not prepend cd command if requestedCwd is the same as currentCwd", async () => {
+			terminal.requestedCwd = "/initial/cwd"
+			vi.spyOn(terminal, "getCurrentWorkingDirectory").mockReturnValue("/initial/cwd")
+			const callbacks = {
+				onLine: vi.fn(),
+				onCompleted: vi.fn(),
+				onShellExecutionStarted: vi.fn(),
+				onShellExecutionComplete: vi.fn(),
+			}
+
+			terminal.runCommand("ls", callbacks)
+
+			// Wait for the async operation to complete
+			await vi.waitFor(() => {
+				expect(mockProcess.command).toBe("ls")
+				expect(mockRun).toHaveBeenCalledWith("ls")
+			})
+		})
+
+		it("should not prepend cd command if requestedCwd is not set", async () => {
+			vi.spyOn(terminal, "getCurrentWorkingDirectory").mockReturnValue("/initial/cwd")
+			const callbacks = {
+				onLine: vi.fn(),
+				onCompleted: vi.fn(),
+				onShellExecutionStarted: vi.fn(),
+				onShellExecutionComplete: vi.fn(),
+			}
+
+			terminal.runCommand("ls", callbacks)
+
+			// Wait for the async operation to complete
+			await vi.waitFor(() => {
+				expect(mockProcess.command).toBe("ls")
+				expect(mockRun).toHaveBeenCalledWith("ls")
+			})
+		})
+	})
+})

src/integrations/terminal/__tests__/TerminalRegistry.spec.ts

@@ -3,9 +3,14 @@
 import * as vscode from "vscode"
 import { Terminal } from "../Terminal"
 import { TerminalRegistry } from "../TerminalRegistry"
+import { arePathsEqual } from "../../../utils/path"
 
 const PAGER = process.platform === "win32" ? "" : "cat"
 
+vi.mock("../../../utils/path", () => ({
+	arePathsEqual: vi.fn((a, b) => a === b),
+}))
+
 vi.mock("execa", () => ({
 	execa: vi.fn(),
 }))
@@ -119,4 +124,78 @@ describe("TerminalRegistry", () => {
 			}
 		})
 	})
+
+	describe("getOrCreateTerminal", () => {
+		let createTerminalSpy: any
+
+		beforeEach(() => {
+			// Reset terminals before each test
+			;(TerminalRegistry as any).terminals = []
+			createTerminalSpy = vi.spyOn(TerminalRegistry, "createTerminal")
+		})
+
+		afterEach(() => {
+			createTerminalSpy.mockRestore()
+		})
+
+		it("should create a new terminal if none exist", async () => {
+			await TerminalRegistry.getOrCreateTerminal("/test/path", "task1")
+			expect(createTerminalSpy).toHaveBeenCalledWith("/test/path", "vscode")
+		})
+
+		it("should reuse a terminal with the same task ID and cwd", async () => {
+			const existingTerminal = new Terminal(1, undefined, "/test/path")
+			existingTerminal.taskId = "task1"
+			;(TerminalRegistry as any).terminals.push(existingTerminal)
+
+			const terminal = await TerminalRegistry.getOrCreateTerminal("/test/path", "task1")
+			expect(terminal).toBe(existingTerminal)
+			expect(createTerminalSpy).not.toHaveBeenCalled()
+		})
+
+		it("should reuse an idle terminal with the same cwd", async () => {
+			const existingTerminal = new Terminal(1, undefined, "/test/path")
+			;(TerminalRegistry as any).terminals.push(existingTerminal)
+
+			const terminal = await TerminalRegistry.getOrCreateTerminal("/test/path", "task2")
+			expect(terminal).toBe(existingTerminal)
+			expect(createTerminalSpy).not.toHaveBeenCalled()
+		})
+
+		it("should reuse an idle terminal with a different cwd if no better option is available", async () => {
+			const existingTerminal = new Terminal(1, undefined, "/other/path")
+			;(TerminalRegistry as any).terminals.push(existingTerminal)
+
+			const terminal = await TerminalRegistry.getOrCreateTerminal("/test/path", "task1")
+			expect(terminal).toBe(existingTerminal)
+			expect(createTerminalSpy).not.toHaveBeenCalled()
+			expect(terminal.requestedCwd).toBe("/test/path")
+		})
+
+		it("should prioritize reusing a terminal with the same task ID when cwd has drifted", async () => {
+			const terminal1 = new Terminal(1, undefined, "/other/path1")
+			terminal1.taskId = "task1"
+			const terminal2 = new Terminal(2, undefined, "/other/path2")
+			terminal2.taskId = "task2"
+			;(TerminalRegistry as any).terminals.push(terminal1, terminal2)
+
+			const terminal = await TerminalRegistry.getOrCreateTerminal("/test/path", "task1")
+			expect(terminal).toBe(terminal1)
+			expect(createTerminalSpy).not.toHaveBeenCalled()
+		})
+
+		it("should create a new terminal if all existing terminals are busy", async () => {
+			const existingTerminal = new Terminal(1, undefined, "/test/path")
+			existingTerminal.busy = true
+			;(TerminalRegistry as any).terminals.push(existingTerminal)
+
+			await TerminalRegistry.getOrCreateTerminal("/test/path", "task1")
+			expect(createTerminalSpy).toHaveBeenCalledWith("/test/path", "vscode")
+		})
+
+		it("should set the requestedCwd on the returned terminal", async () => {
+			const terminal = await TerminalRegistry.getOrCreateTerminal("/test/path", "task1")
+			expect(terminal.requestedCwd).toBe("/test/path")
+		})
+	})
 })

src/integrations/terminal/types.ts

@@ -9,6 +9,7 @@ export interface RooTerminal {
 	running: boolean
 	taskId?: string
 	process?: RooTerminalProcess
+	requestedCwd?: string
 	getCurrentWorkingDirectory(): string
 	isClosed: () => boolean
 	runCommand: (command: string, callbacks: RooTerminalCallbacks) => RooTerminalProcessResultPromise