mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-08 03:07:53 +00:00
fix: track terminal working directory changes across commands
- Add lastUsedTerminal property to Task to persist terminal reference - Update executeCommandTool to use terminal current directory for subsequent commands - Resolve relative paths based on terminal cwd instead of task cwd - Add comprehensive tests for directory tracking scenarios Fixes #7567
This commit is contained in:
parent
63b71d8299
commit
4108f30ba6
3 changed files with 304 additions and 3 deletions
|
|
@ -66,7 +66,7 @@ import { RepoPerTaskCheckpointService } from "../../services/checkpoints"
|
|||
// integrations
|
||||
import { DiffViewProvider } from "../../integrations/editor/DiffViewProvider"
|
||||
import { findToolName, formatContentBlockToMarkdown } from "../../integrations/misc/export-markdown"
|
||||
import { RooTerminalProcess } from "../../integrations/terminal/types"
|
||||
import { RooTerminalProcess, RooTerminal } from "../../integrations/terminal/types"
|
||||
import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
|
||||
|
||||
// utils
|
||||
|
|
@ -224,6 +224,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
fileContextTracker: FileContextTracker
|
||||
urlContentFetcher: UrlContentFetcher
|
||||
terminalProcess?: RooTerminalProcess
|
||||
lastUsedTerminal?: RooTerminal
|
||||
|
||||
// Computer User
|
||||
browserSession: BrowserSession
|
||||
|
|
|
|||
286
src/core/tools/__tests__/executeCommandDirectoryTracking.spec.ts
Normal file
286
src/core/tools/__tests__/executeCommandDirectoryTracking.spec.ts
Normal file
|
|
@ -0,0 +1,286 @@
|
|||
/**
|
||||
* Test for GitHub Issue #7567: Regression: task directory is not followed on terminal requests
|
||||
*
|
||||
* This test verifies that when a command changes directory (e.g., `cd subdir`),
|
||||
* subsequent commands use the terminal's updated working directory instead of
|
||||
* spawning a new terminal.
|
||||
*/
|
||||
|
||||
import * as path from "path"
|
||||
import * as fs from "fs/promises"
|
||||
import { vi, describe, it, expect, beforeEach } from "vitest"
|
||||
|
||||
import { ExecuteCommandOptions, executeCommand } from "../executeCommandTool"
|
||||
import { TerminalRegistry } from "../../../integrations/terminal/TerminalRegistry"
|
||||
import { Terminal } from "../../../integrations/terminal/Terminal"
|
||||
import type { RooTerminalCallbacks, RooTerminal } from "../../../integrations/terminal/types"
|
||||
|
||||
// Mock fs to control directory existence checks
|
||||
vi.mock("fs/promises")
|
||||
|
||||
// Mock TerminalRegistry to control terminal creation
|
||||
vi.mock("../../../integrations/terminal/TerminalRegistry")
|
||||
|
||||
// Mock Terminal class
|
||||
vi.mock("../../../integrations/terminal/Terminal")
|
||||
|
||||
describe("Terminal Directory Tracking (Issue #7567)", () => {
|
||||
let mockTask: any
|
||||
let mockTerminal: any
|
||||
let mockProcess: any
|
||||
let mockProvider: any
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
||||
// Mock fs.access to simulate directory existence
|
||||
;(fs.access as any).mockResolvedValue(undefined)
|
||||
|
||||
// Create mock provider
|
||||
mockProvider = {
|
||||
postMessageToWebview: vi.fn(),
|
||||
getState: vi.fn().mockResolvedValue({
|
||||
terminalOutputLineLimit: 500,
|
||||
terminalShellIntegrationDisabled: false,
|
||||
}),
|
||||
}
|
||||
|
||||
// Create mock task
|
||||
mockTask = {
|
||||
cwd: "/test/project",
|
||||
taskId: "test-task-123",
|
||||
providerRef: {
|
||||
deref: vi.fn().mockResolvedValue(mockProvider),
|
||||
},
|
||||
say: vi.fn().mockResolvedValue(undefined),
|
||||
terminalProcess: undefined,
|
||||
lastUsedTerminal: undefined, // This is the new property we're testing
|
||||
}
|
||||
|
||||
// Create mock process that resolves immediately
|
||||
mockProcess = Promise.resolve()
|
||||
mockProcess.continue = vi.fn()
|
||||
mockProcess.abort = vi.fn()
|
||||
|
||||
// Create mock terminal with getCurrentWorkingDirectory method
|
||||
mockTerminal = {
|
||||
provider: "vscode",
|
||||
id: 1,
|
||||
initialCwd: "/test/project",
|
||||
getCurrentWorkingDirectory: vi.fn().mockReturnValue("/test/project"),
|
||||
isClosed: vi.fn().mockReturnValue(false),
|
||||
runCommand: vi.fn().mockReturnValue(mockProcess),
|
||||
terminal: {
|
||||
show: vi.fn(),
|
||||
shellIntegration: {
|
||||
cwd: { fsPath: "/test/project" },
|
||||
},
|
||||
},
|
||||
}
|
||||
})
|
||||
|
||||
it("should track terminal working directory changes across multiple commands", async () => {
|
||||
// Setup: First command will be `cd subdir`
|
||||
const initialCwd = "/test/project"
|
||||
const subdirCwd = "/test/project/subdir"
|
||||
|
||||
// Mock terminal behavior for first command (cd subdir)
|
||||
let currentWorkingDir = initialCwd
|
||||
mockTerminal.getCurrentWorkingDirectory.mockImplementation(() => currentWorkingDir)
|
||||
|
||||
// First command execution
|
||||
mockTerminal.runCommand.mockImplementationOnce((command: string, callbacks: RooTerminalCallbacks) => {
|
||||
// Simulate directory change
|
||||
currentWorkingDir = subdirCwd
|
||||
mockTerminal.terminal.shellIntegration.cwd.fsPath = subdirCwd
|
||||
|
||||
setTimeout(() => {
|
||||
callbacks.onCompleted("", mockProcess)
|
||||
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
|
||||
}, 0)
|
||||
return mockProcess
|
||||
})
|
||||
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValueOnce(mockTerminal)
|
||||
|
||||
// Execute first command: cd subdir
|
||||
const options1: ExecuteCommandOptions = {
|
||||
executionId: "test-123",
|
||||
command: "cd subdir",
|
||||
terminalShellIntegrationDisabled: false,
|
||||
terminalOutputLineLimit: 500,
|
||||
}
|
||||
|
||||
const [rejected1, result1] = await executeCommand(mockTask, options1)
|
||||
|
||||
// Verify first command
|
||||
expect(rejected1).toBe(false)
|
||||
expect(TerminalRegistry.getOrCreateTerminal).toHaveBeenCalledWith(initialCwd, mockTask.taskId, "vscode")
|
||||
expect(mockTask.lastUsedTerminal).toBe(mockTerminal) // Terminal should be stored
|
||||
expect(result1).toContain(`within working directory '${subdirCwd}'`)
|
||||
|
||||
// Reset mocks for second command
|
||||
vi.mocked(TerminalRegistry.getOrCreateTerminal).mockClear()
|
||||
|
||||
// Second command execution - should use the updated directory
|
||||
mockTerminal.runCommand.mockImplementationOnce((command: string, callbacks: RooTerminalCallbacks) => {
|
||||
setTimeout(() => {
|
||||
callbacks.onCompleted("command_from_subdir output", mockProcess)
|
||||
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
|
||||
}, 0)
|
||||
return mockProcess
|
||||
})
|
||||
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValueOnce(mockTerminal)
|
||||
|
||||
// Execute second command: command_from_subdir
|
||||
const options2: ExecuteCommandOptions = {
|
||||
executionId: "test-456",
|
||||
command: "command_from_subdir",
|
||||
terminalShellIntegrationDisabled: false,
|
||||
terminalOutputLineLimit: 500,
|
||||
}
|
||||
|
||||
const [rejected2, result2] = await executeCommand(mockTask, options2)
|
||||
|
||||
// Verify second command uses the subdirectory
|
||||
expect(rejected2).toBe(false)
|
||||
// IMPORTANT: This should be called with subdirCwd, not initialCwd
|
||||
// This is the key assertion - the second command should use the updated directory from the first command
|
||||
expect(TerminalRegistry.getOrCreateTerminal).toHaveBeenCalledWith(subdirCwd, mockTask.taskId, "vscode")
|
||||
expect(result2).toContain(`within working directory '${subdirCwd}'`)
|
||||
})
|
||||
|
||||
it("should handle relative paths based on terminal's current directory", async () => {
|
||||
// Setup: Terminal is in a subdirectory
|
||||
const subdirCwd = "/test/project/src"
|
||||
mockTask.lastUsedTerminal = mockTerminal
|
||||
mockTerminal.getCurrentWorkingDirectory.mockReturnValue(subdirCwd)
|
||||
|
||||
mockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
|
||||
setTimeout(() => {
|
||||
callbacks.onCompleted("Command output", mockProcess)
|
||||
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
|
||||
}, 0)
|
||||
return mockProcess
|
||||
})
|
||||
|
||||
// Create a new mock terminal that will be returned with the resolved path
|
||||
const newMockTerminal = {
|
||||
...mockTerminal,
|
||||
getCurrentWorkingDirectory: vi.fn().mockReturnValue(path.resolve(subdirCwd, "components")),
|
||||
}
|
||||
|
||||
const resolvedPath = path.resolve(subdirCwd, "components")
|
||||
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(newMockTerminal)
|
||||
|
||||
// Execute command with relative custom cwd
|
||||
const options: ExecuteCommandOptions = {
|
||||
executionId: "test-789",
|
||||
command: "ls",
|
||||
customCwd: "components", // Relative to terminal's current directory
|
||||
terminalShellIntegrationDisabled: false,
|
||||
terminalOutputLineLimit: 500,
|
||||
}
|
||||
|
||||
const [rejected, result] = await executeCommand(mockTask, options)
|
||||
|
||||
// Verify it resolves relative to terminal's cwd, not task's cwd
|
||||
expect(rejected).toBe(false)
|
||||
expect(TerminalRegistry.getOrCreateTerminal).toHaveBeenCalledWith(resolvedPath, mockTask.taskId, "vscode")
|
||||
expect(result).toContain(`within working directory '${resolvedPath.toPosix()}'`)
|
||||
})
|
||||
|
||||
it("should fallback to task cwd when terminal is closed", async () => {
|
||||
// Setup: Previous terminal exists but is closed
|
||||
mockTask.lastUsedTerminal = mockTerminal
|
||||
mockTerminal.isClosed.mockReturnValue(true)
|
||||
|
||||
// Create a new terminal for this test
|
||||
const newMockTerminal = {
|
||||
...mockTerminal,
|
||||
id: 2,
|
||||
isClosed: vi.fn().mockReturnValue(false),
|
||||
}
|
||||
|
||||
newMockTerminal.runCommand.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
|
||||
setTimeout(() => {
|
||||
callbacks.onCompleted("Command output", mockProcess)
|
||||
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
|
||||
}, 0)
|
||||
return mockProcess
|
||||
})
|
||||
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(newMockTerminal)
|
||||
|
||||
const options: ExecuteCommandOptions = {
|
||||
executionId: "test-999",
|
||||
command: "echo test",
|
||||
terminalShellIntegrationDisabled: false,
|
||||
terminalOutputLineLimit: 500,
|
||||
}
|
||||
|
||||
const [rejected, result] = await executeCommand(mockTask, options)
|
||||
|
||||
// Verify it falls back to task.cwd when terminal is closed
|
||||
expect(rejected).toBe(false)
|
||||
expect(TerminalRegistry.getOrCreateTerminal).toHaveBeenCalledWith(mockTask.cwd, mockTask.taskId, "vscode")
|
||||
expect(mockTask.lastUsedTerminal).toBe(newMockTerminal) // Should update to new terminal
|
||||
})
|
||||
|
||||
it("should handle sequence of cd commands correctly", async () => {
|
||||
// This test simulates: cd dir1 && command1, then cd ../dir2 && command2
|
||||
let currentWorkingDir = "/test/project"
|
||||
|
||||
mockTerminal.getCurrentWorkingDirectory.mockImplementation(() => currentWorkingDir)
|
||||
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockTerminal)
|
||||
|
||||
// First command: cd dir1 && command1
|
||||
mockTerminal.runCommand.mockImplementationOnce((command: string, callbacks: RooTerminalCallbacks) => {
|
||||
currentWorkingDir = "/test/project/dir1"
|
||||
setTimeout(() => {
|
||||
callbacks.onCompleted("", mockProcess)
|
||||
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
|
||||
}, 0)
|
||||
return mockProcess
|
||||
})
|
||||
|
||||
const options1: ExecuteCommandOptions = {
|
||||
executionId: "test-seq-1",
|
||||
command: "cd dir1 && command1",
|
||||
terminalShellIntegrationDisabled: false,
|
||||
terminalOutputLineLimit: 500,
|
||||
}
|
||||
|
||||
await executeCommand(mockTask, options1)
|
||||
expect(mockTask.lastUsedTerminal).toBe(mockTerminal)
|
||||
|
||||
// Clear mock for next call
|
||||
vi.mocked(TerminalRegistry.getOrCreateTerminal).mockClear()
|
||||
|
||||
// Second command: cd ../dir2 && command2
|
||||
mockTerminal.runCommand.mockImplementationOnce((command: string, callbacks: RooTerminalCallbacks) => {
|
||||
currentWorkingDir = "/test/project/dir2"
|
||||
setTimeout(() => {
|
||||
callbacks.onCompleted("", mockProcess)
|
||||
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
|
||||
}, 0)
|
||||
return mockProcess
|
||||
})
|
||||
|
||||
const options2: ExecuteCommandOptions = {
|
||||
executionId: "test-seq-2",
|
||||
command: "cd ../dir2 && command2",
|
||||
terminalShellIntegrationDisabled: false,
|
||||
terminalOutputLineLimit: 500,
|
||||
}
|
||||
|
||||
const [rejected2, result2] = await executeCommand(mockTask, options2)
|
||||
|
||||
// Should request terminal with dir1 path (from previous command)
|
||||
expect(TerminalRegistry.getOrCreateTerminal).toHaveBeenCalledWith(
|
||||
"/test/project/dir1",
|
||||
mockTask.taskId,
|
||||
"vscode",
|
||||
)
|
||||
// But result should show dir2 (after cd command)
|
||||
expect(result2).toContain("within working directory '/test/project/dir2'")
|
||||
})
|
||||
})
|
||||
|
|
@ -160,11 +160,22 @@ export async function executeCommand(
|
|||
let workingDir: string
|
||||
|
||||
if (!customCwd) {
|
||||
workingDir = task.cwd
|
||||
// If we have a previously used terminal, use its current working directory
|
||||
// This ensures that cd commands are properly tracked across execute_command calls
|
||||
if (task.lastUsedTerminal && !task.lastUsedTerminal.isClosed()) {
|
||||
workingDir = task.lastUsedTerminal.getCurrentWorkingDirectory()
|
||||
} else {
|
||||
workingDir = task.cwd
|
||||
}
|
||||
} else if (path.isAbsolute(customCwd)) {
|
||||
workingDir = customCwd
|
||||
} else {
|
||||
workingDir = path.resolve(task.cwd, customCwd)
|
||||
// Resolve relative paths against the last terminal's cwd or task cwd
|
||||
const basePath =
|
||||
task.lastUsedTerminal && !task.lastUsedTerminal.isClosed()
|
||||
? task.lastUsedTerminal.getCurrentWorkingDirectory()
|
||||
: task.cwd
|
||||
workingDir = path.resolve(basePath, customCwd)
|
||||
}
|
||||
|
||||
try {
|
||||
|
|
@ -240,6 +251,9 @@ export async function executeCommand(
|
|||
|
||||
const terminal = await TerminalRegistry.getOrCreateTerminal(workingDir, task.taskId, terminalProvider)
|
||||
|
||||
// Store the terminal reference in the task for future commands
|
||||
task.lastUsedTerminal = terminal
|
||||
|
||||
if (terminal instanceof Terminal) {
|
||||
terminal.terminal.show(true)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue