diff --git a/src/integrations/terminal/BaseTerminal.ts b/src/integrations/terminal/BaseTerminal.ts index a79d417b07..a8560303bc 100644 --- a/src/integrations/terminal/BaseTerminal.ts +++ b/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[] = [] diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 8bf2072f3d..fc0da38fd3 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/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.`) diff --git a/src/integrations/terminal/TerminalRegistry.ts b/src/integrations/terminal/TerminalRegistry.ts index 6e0531bebe..0e6bf7123a 100644 --- a/src/integrations/terminal/TerminalRegistry.ts +++ b/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 } diff --git a/src/integrations/terminal/__tests__/Terminal.spec.ts b/src/integrations/terminal/__tests__/Terminal.spec.ts new file mode 100644 index 0000000000..c0da79bff0 --- /dev/null +++ b/src/integrations/terminal/__tests__/Terminal.spec.ts @@ -0,0 +1,103 @@ +// 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", () => { + 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) + + 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", () => { + 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) + + expect(mockProcess.command).toBe("ls") + expect(mockRun).toHaveBeenCalledWith("ls") + }) + + it("should not prepend cd command if requestedCwd is not set", () => { + 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) + + expect(mockProcess.command).toBe("ls") + expect(mockRun).toHaveBeenCalledWith("ls") + }) + }) +}) \ No newline at end of file diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index d3912caf47..ed7420d35f 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/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") + }) + }) }) diff --git a/src/integrations/terminal/types.ts b/src/integrations/terminal/types.ts index 65d521ba6e..bd85017a86 100644 --- a/src/integrations/terminal/types.ts +++ b/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