From 491039a776dd857ab52baaba76b854f9c64252ac Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Sun, 13 Jul 2025 10:58:09 -0600 Subject: [PATCH 1/3] fix: [5424] return the cwd in the exec tool's response so that the model is not lost after subsequent calls --- src/core/tools/executeCommandTool.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/tools/executeCommandTool.ts b/src/core/tools/executeCommandTool.ts index 795beccc06..dbda68332f 100644 --- a/src/core/tools/executeCommandTool.ts +++ b/src/core/tools/executeCommandTool.ts @@ -266,8 +266,7 @@ export async function executeCommand( exitStatus = `Exit code: ` } - let workingDirInfo = ` within working directory '${workingDir.toPosix()}'` - const newWorkingDir = terminal.getCurrentWorkingDirectory() + let workingDirInfo = ` within working directory '${terminal.getCurrentWorkingDirectory().toPosix()}'` return [false, `Command executed in terminal ${workingDirInfo}. ${exitStatus}\nOutput:\n${result}`] } else { From 0867718a7290aca7bc65a78cb9f0978e4b503ac2 Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Sun, 13 Jul 2025 13:09:52 -0600 Subject: [PATCH 2/3] initial tests for change to returning cwd. --- .../tools/__tests__/executeCommand.spec.ts | 474 ++++++++++++++++++ 1 file changed, 474 insertions(+) create mode 100644 src/core/tools/__tests__/executeCommand.spec.ts diff --git a/src/core/tools/__tests__/executeCommand.spec.ts b/src/core/tools/__tests__/executeCommand.spec.ts new file mode 100644 index 0000000000..cc93e687cc --- /dev/null +++ b/src/core/tools/__tests__/executeCommand.spec.ts @@ -0,0 +1,474 @@ +// +// Tests the ExecuteCommand tool itself vs calling the tool where the tool is mocked. +// +import * as path from "path" +import * as fs from "fs/promises" + +import { ExecuteCommandOptions } from "../executeCommandTool" +import { TerminalRegistry } from "../../../integrations/terminal/TerminalRegistry" +import { Terminal } from "../../../integrations/terminal/Terminal" +import { ExecaTerminal } from "../../../integrations/terminal/ExecaTerminal" +import type { RooTerminalCallbacks } from "../../../integrations/terminal/types" + +// Mock fs to control directory existence checks +vitest.mock("fs/promises") + +// Mock TerminalRegistry to control terminal creation +vitest.mock("../../../integrations/terminal/TerminalRegistry") + +// Mock Terminal and ExecaTerminal classes +vitest.mock("../../../integrations/terminal/Terminal") +vitest.mock("../../../integrations/terminal/ExecaTerminal") + +// Import the actual executeCommand function (not mocked) +import { executeCommand } from "../executeCommandTool" + +// Tests for the executeCommand function +describe("executeCommand", () => { + let mockTask: any + let mockTerminal: any + let mockProcess: any + let mockProvider: any + + beforeEach(() => { + vitest.clearAllMocks() + + // Mock fs.access to simulate directory existence + ;(fs.access as any).mockResolvedValue(undefined) + + // Create mock provider + mockProvider = { + postMessageToWebview: vitest.fn(), + getState: vitest.fn().mockResolvedValue({ + terminalOutputLineLimit: 500, + terminalShellIntegrationDisabled: false, + }), + } + + // Create mock task + mockTask = { + cwd: "/test/project", + taskId: "test-task-123", + providerRef: { + deref: vitest.fn().mockResolvedValue(mockProvider), + }, + say: vitest.fn().mockResolvedValue(undefined), + terminalProcess: undefined, + } + + // Create mock process that resolves immediately + mockProcess = Promise.resolve() + mockProcess.continue = vitest.fn() + + // Create mock terminal with getCurrentWorkingDirectory method + mockTerminal = { + provider: "vscode", + id: 1, + initialCwd: "/test/project", + getCurrentWorkingDirectory: vitest.fn().mockReturnValue("/test/project"), + runCommand: vitest.fn().mockReturnValue(mockProcess), + terminal: { + show: vitest.fn(), + }, + } + + // Mock TerminalRegistry.getOrCreateTerminal + ;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockTerminal) + }) + + describe("Working Directory Behavior", () => { + it("should use terminal.getCurrentWorkingDirectory() in the output message for completed commands", async () => { + // Setup: Mock terminal to return a different current working directory + const initialCwd = "/test/project" + const currentCwd = "/test/project/subdirectory" + + mockTask.cwd = initialCwd + mockTerminal.initialCwd = initialCwd + mockTerminal.getCurrentWorkingDirectory.mockReturnValue(currentCwd) + + // Mock the terminal process to complete successfully + mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + // Simulate command completion + setTimeout(() => { + callbacks.onCompleted("Command output", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess) + }, 0) + return mockProcess + }) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "echo test", + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify + expect(rejected).toBe(false) + expect(mockTerminal.getCurrentWorkingDirectory).toHaveBeenCalled() + expect(result).toContain(`within working directory '${currentCwd}'`) + expect(result).not.toContain(`within working directory '${initialCwd}'`) + }) + + it("should use terminal.getCurrentWorkingDirectory() for VSCode Terminal with shell integration", async () => { + // Setup: Mock VSCode Terminal instance + const vscodeTerminal = new Terminal(1, undefined, "/test/project") + const mockVSCodeTerminal = vscodeTerminal as any + + // Mock shell integration providing different cwd + mockVSCodeTerminal.terminal = { + show: vitest.fn(), + shellIntegration: { + cwd: { fsPath: "/test/project/changed-dir" }, + }, + } + mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project/changed-dir") + mockVSCodeTerminal.runCommand = vitest + .fn() + .mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Command output", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess) + }, 0) + return mockProcess + }) + ;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "echo test", + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify + expect(rejected).toBe(false) + expect(result).toContain("within working directory '/test/project/changed-dir'") + }) + + it("should use terminal.getCurrentWorkingDirectory() for ExecaTerminal (always returns initialCwd)", async () => { + // Setup: Mock ExecaTerminal instance + const execaTerminal = new ExecaTerminal(1, "/test/project") + const mockExecaTerminal = execaTerminal as any + + // ExecaTerminal always returns initialCwd + mockExecaTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project") + mockExecaTerminal.runCommand = vitest + .fn() + .mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Command output", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess) + }, 0) + return mockProcess + }) + ;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockExecaTerminal) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "echo test", + terminalShellIntegrationDisabled: true, // Forces ExecaTerminal + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify + expect(rejected).toBe(false) + expect(mockExecaTerminal.getCurrentWorkingDirectory).toHaveBeenCalled() + expect(result).toContain("within working directory '/test/project'") + }) + }) + + describe("Custom Working Directory", () => { + it("should handle absolute custom cwd and use terminal.getCurrentWorkingDirectory() in output", async () => { + const customCwd = "/custom/absolute/path" + + mockTerminal.getCurrentWorkingDirectory.mockReturnValue(customCwd) + mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Command output", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess) + }, 0) + return mockProcess + }) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "echo test", + customCwd, + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify + expect(rejected).toBe(false) + expect(TerminalRegistry.getOrCreateTerminal).toHaveBeenCalledWith( + customCwd, + true, // customCwd provided + mockTask.taskId, + "vscode", + ) + expect(result).toContain(`within working directory '${customCwd}'`) + }) + + it("should handle relative custom cwd and use terminal.getCurrentWorkingDirectory() in output", async () => { + const relativeCwd = "subdirectory" + const resolvedCwd = path.resolve(mockTask.cwd, relativeCwd) + + mockTerminal.getCurrentWorkingDirectory.mockReturnValue(resolvedCwd) + mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Command output", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess) + }, 0) + return mockProcess + }) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "echo test", + customCwd: relativeCwd, + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify + expect(rejected).toBe(false) + expect(TerminalRegistry.getOrCreateTerminal).toHaveBeenCalledWith( + resolvedCwd, + true, // customCwd provided + mockTask.taskId, + "vscode", + ) + expect(result).toContain(`within working directory '${resolvedCwd}'`) + }) + + it("should return error when custom working directory does not exist", async () => { + const nonExistentCwd = "/non/existent/path" + + // Mock fs.access to throw error for non-existent directory + ;(fs.access as any).mockRejectedValue(new Error("Directory does not exist")) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "echo test", + customCwd: nonExistentCwd, + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify + expect(rejected).toBe(false) + expect(result).toBe(`Working directory '${nonExistentCwd}' does not exist.`) + expect(TerminalRegistry.getOrCreateTerminal).not.toHaveBeenCalled() + }) + }) + + describe("Terminal Provider Selection", () => { + it("should use vscode provider when shell integration is enabled", async () => { + mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Command output", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess) + }, 0) + return mockProcess + }) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "echo test", + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + await executeCommand(mockTask, options) + + // Verify + expect(TerminalRegistry.getOrCreateTerminal).toHaveBeenCalledWith( + mockTask.cwd, + false, // no customCwd + mockTask.taskId, + "vscode", + ) + }) + + it("should use execa provider when shell integration is disabled", async () => { + mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Command output", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess) + }, 0) + return mockProcess + }) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "echo test", + terminalShellIntegrationDisabled: true, + terminalOutputLineLimit: 500, + } + + // Execute + await executeCommand(mockTask, options) + + // Verify + expect(TerminalRegistry.getOrCreateTerminal).toHaveBeenCalledWith( + mockTask.cwd, + false, // no customCwd + mockTask.taskId, + "execa", + ) + }) + }) + + describe("Command Execution States", () => { + it("should handle completed command with exit code 0", async () => { + mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project") + mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Command completed successfully", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess) + }, 0) + return mockProcess + }) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "echo success", + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify + expect(rejected).toBe(false) + expect(result).toContain("Exit code: 0") + expect(result).toContain("within working directory '/test/project'") + }) + + it("should handle completed command with non-zero exit code", async () => { + mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project") + mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Command failed", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 1 }, mockProcess) + }, 0) + return mockProcess + }) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "exit 1", + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify + expect(rejected).toBe(false) + expect(result).toContain("Command execution was not successful") + expect(result).toContain("Exit code: 1") + expect(result).toContain("within working directory '/test/project'") + }) + + it("should handle command terminated by signal", async () => { + mockTerminal.getCurrentWorkingDirectory.mockReturnValue("/test/project") + mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Command interrupted", mockProcess) + callbacks.onShellExecutionComplete( + { + exitCode: undefined, + signalName: "SIGINT", + coreDumpPossible: false, + }, + mockProcess, + ) + }, 0) + return mockProcess + }) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "long-running-command", + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify + expect(rejected).toBe(false) + expect(result).toContain("Process terminated by signal SIGINT") + expect(result).toContain("within working directory '/test/project'") + }) + }) + + describe("Terminal Working Directory Updates", () => { + it("should update working directory when terminal returns different cwd", async () => { + // Setup: Terminal initially at project root, but getCurrentWorkingDirectory returns different path + const initialCwd = "/test/project" + const updatedCwd = "/test/project/src" + + mockTask.cwd = initialCwd + mockTerminal.initialCwd = initialCwd + + // Mock Terminal instance behavior + const mockTerminalInstance = { + ...mockTerminal, + terminal: { show: vitest.fn() }, + getCurrentWorkingDirectory: vitest.fn().mockReturnValue(updatedCwd), + runCommand: vitest.fn().mockImplementation((command: string, callbacks: RooTerminalCallbacks) => { + setTimeout(() => { + callbacks.onCompleted("Directory changed", mockProcess) + callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess) + }, 0) + return mockProcess + }), + } + + ;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockTerminalInstance) + + const options: ExecuteCommandOptions = { + executionId: "test-123", + command: "cd src && pwd", + terminalShellIntegrationDisabled: false, + terminalOutputLineLimit: 500, + } + + // Execute + const [rejected, result] = await executeCommand(mockTask, options) + + // Verify the result uses the updated working directory + expect(rejected).toBe(false) + expect(result).toContain(`within working directory '${updatedCwd}'`) + expect(result).not.toContain(`within working directory '${initialCwd}'`) + + // Verify the terminal's getCurrentWorkingDirectory was called + expect(mockTerminalInstance.getCurrentWorkingDirectory).toHaveBeenCalled() + }) + }) +}) From 58f7ad19de88373369da3ef2c1b0008e4c7a48bc Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Sun, 13 Jul 2025 14:56:13 -0600 Subject: [PATCH 3/3] fix for xplatform --- src/core/tools/__tests__/executeCommand.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/tools/__tests__/executeCommand.spec.ts b/src/core/tools/__tests__/executeCommand.spec.ts index cc93e687cc..68dec5c456 100644 --- a/src/core/tools/__tests__/executeCommand.spec.ts +++ b/src/core/tools/__tests__/executeCommand.spec.ts @@ -254,7 +254,7 @@ describe("executeCommand", () => { mockTask.taskId, "vscode", ) - expect(result).toContain(`within working directory '${resolvedCwd}'`) + expect(result).toContain(`within working directory '${resolvedCwd.toPosix()}'`) }) it("should return error when custom working directory does not exist", async () => {