diff --git a/src/core/context-tracking/FileContextTracker.ts b/src/core/context-tracking/FileContextTracker.ts index 4c5640afdf..a0a71c60f8 100644 --- a/src/core/context-tracking/FileContextTracker.ts +++ b/src/core/context-tracking/FileContextTracker.ts @@ -8,6 +8,7 @@ import fs from "fs/promises" import { ContextProxy } from "../config/ContextProxy" import type { FileMetadataEntry, RecordSource, TaskMetadata } from "./FileContextTrackerTypes" import { ClineProvider } from "../webview/ClineProvider" +import { getWorkspaceReadablePath, resolvePathInWorkspace } from "../../utils/pathUtils" // This class is responsible for tracking file operations that may result in stale context. // If a user modifies a file outside of Roo, the context may become stale and need to be updated. @@ -45,7 +46,7 @@ export class FileContextTracker { } // File watchers are set up for each file that is tracked in the task metadata. - async setupFileWatcher(filePath: string) { + async setupFileWatcher(filePath: string, absolutePath?: string) { // Only setup watcher if it doesn't already exist for this file if (this.fileWatchers.has(filePath)) { return @@ -57,7 +58,7 @@ export class FileContextTracker { } // Create a file system watcher for this specific file - const fileUri = vscode.Uri.file(path.resolve(cwd, filePath)) + const fileUri = vscode.Uri.file(absolutePath ?? (await resolvePathInWorkspace(cwd, filePath))) const watcher = vscode.workspace.createFileSystemWatcher( new vscode.RelativePattern(path.dirname(fileUri.fsPath), path.basename(fileUri.fsPath)), ) @@ -85,10 +86,13 @@ export class FileContextTracker { return } - await this.addFileToFileContextTracker(this.taskId, filePath, operation) + const absolutePath = await resolvePathInWorkspace(cwd, filePath) + const trackedPath = getWorkspaceReadablePath(cwd, absolutePath, filePath) + + await this.addFileToFileContextTracker(this.taskId, trackedPath, operation) // Set up file watcher for this file - await this.setupFileWatcher(filePath) + await this.setupFileWatcher(trackedPath, absolutePath) } catch (error) { console.error("Failed to track file operation:", error) } diff --git a/src/core/ignore/RooIgnoreController.ts b/src/core/ignore/RooIgnoreController.ts index 45054cce96..5b2a725294 100644 --- a/src/core/ignore/RooIgnoreController.ts +++ b/src/core/ignore/RooIgnoreController.ts @@ -5,6 +5,8 @@ import fsSync from "fs" import ignore, { Ignore } from "ignore" import * as vscode from "vscode" +import { getWorkspaceRelativePath, getWorkspaceRootForPath } from "../../utils/pathUtils" + export const LOCK_TEXT_SYMBOL = "\u{1F512}" /** @@ -15,6 +17,8 @@ export const LOCK_TEXT_SYMBOL = "\u{1F512}" export class RooIgnoreController { private cwd: string private ignoreInstance: Ignore + private ignoreInstances = new Map() + private rooIgnoreContents = new Map() private disposables: vscode.Disposable[] = [] rooIgnoreContent: string | undefined @@ -38,19 +42,23 @@ export class RooIgnoreController { * Set up the file watcher for .rooignore changes */ private setupFileWatcher(): void { - const rooignorePattern = new vscode.RelativePattern(this.cwd, ".rooignore") + this.setupFileWatcherForRoot(this.cwd) + } + + private setupFileWatcherForRoot(rootPath: string): void { + const rooignorePattern = new vscode.RelativePattern(rootPath, ".rooignore") const fileWatcher = vscode.workspace.createFileSystemWatcher(rooignorePattern) // Watch for changes and updates this.disposables.push( fileWatcher.onDidChange(() => { - this.loadRooIgnore() + this.loadRooIgnoreForRoot(rootPath) }), fileWatcher.onDidCreate(() => { - this.loadRooIgnore() + this.loadRooIgnoreForRoot(rootPath) }), fileWatcher.onDidDelete(() => { - this.loadRooIgnore() + this.loadRooIgnoreForRoot(rootPath) }), ) @@ -62,17 +70,28 @@ export class RooIgnoreController { * Load custom patterns from .rooignore if it exists */ private async loadRooIgnore(): Promise { + await this.loadRooIgnoreForRoot(this.cwd) + } + + private async loadRooIgnoreForRoot(rootPath: string): Promise { try { // Reset ignore instance to prevent duplicate patterns - this.ignoreInstance = ignore() - const ignorePath = path.join(this.cwd, ".rooignore") + const ignoreInstance = ignore() + const ignorePath = path.join(rootPath, ".rooignore") if (await fileExistsAtPath(ignorePath)) { const content = await fs.readFile(ignorePath, "utf8") - this.rooIgnoreContent = content - this.ignoreInstance.add(content) - this.ignoreInstance.add(".rooignore") + ignoreInstance.add(content) + ignoreInstance.add(".rooignore") + this.ignoreInstances.set(rootPath, ignoreInstance) + this.rooIgnoreContents.set(rootPath, content) } else { - this.rooIgnoreContent = undefined + this.ignoreInstances.set(rootPath, ignoreInstance) + this.rooIgnoreContents.set(rootPath, undefined) + } + + if (rootPath === this.cwd) { + this.ignoreInstance = ignoreInstance + this.rooIgnoreContent = this.rooIgnoreContents.get(rootPath) } } catch (error) { // Should never happen: reading file failed even though it exists @@ -80,6 +99,48 @@ export class RooIgnoreController { } } + private getIgnoreStateForRoot(rootPath: string): { ignoreInstance: Ignore; content: string | undefined } { + const cached = this.ignoreInstances.get(rootPath) + if (cached) { + return { ignoreInstance: cached, content: this.rooIgnoreContents.get(rootPath) } + } + + const ignoreInstance = ignore() + try { + const ignorePath = path.join(rootPath, ".rooignore") + if (fsSync.existsSync(ignorePath)) { + const content = fsSync.readFileSync(ignorePath, "utf8") + ignoreInstance.add(content) + ignoreInstance.add(".rooignore") + this.ignoreInstances.set(rootPath, ignoreInstance) + this.rooIgnoreContents.set(rootPath, content) + this.setupFileWatcherForRoot(rootPath) + return { ignoreInstance, content } + } + } catch (error) { + console.error("Unexpected error loading .rooignore:", error) + } + + this.ignoreInstances.set(rootPath, ignoreInstance) + this.rooIgnoreContents.set(rootPath, undefined) + this.setupFileWatcherForRoot(rootPath) + return { ignoreInstance, content: undefined } + } + + private getKnownWorkspaceRoots(): string[] { + const roots = new Set([this.cwd]) + for (const folder of vscode.workspace.workspaceFolders ?? []) { + roots.add(folder.uri.fsPath) + } + return [...roots] + } + + private getAvailableIgnoreContents(): Array<{ rootPath: string; content: string }> { + return this.getKnownWorkspaceRoots() + .map((rootPath) => ({ rootPath, content: this.getIgnoreStateForRoot(rootPath).content })) + .filter((entry): entry is { rootPath: string; content: string } => typeof entry.content === "string") + } + /** * Check if a file should be accessible to the LLM * Automatically resolves symlinks @@ -87,13 +148,21 @@ export class RooIgnoreController { * @returns true if file is accessible, false if ignored */ validateAccess(filePath: string): boolean { + const absolutePath = path.isAbsolute(filePath) ? path.resolve(filePath) : path.resolve(this.cwd, filePath) + const rootPath = getWorkspaceRootForPath(absolutePath, this.cwd) + + // Preserve backward compatibility for files outside the task workspace roots. + if (!rootPath) { + return true + } + + const { ignoreInstance, content } = this.getIgnoreStateForRoot(rootPath) // Always allow access if .rooignore does not exist - if (!this.rooIgnoreContent) { + if (!content) { return true } - try { - const absolutePath = path.resolve(this.cwd, filePath) + try { // Follow symlinks to get the real path let realPath: string try { @@ -105,10 +174,10 @@ export class RooIgnoreController { } // Convert real path to relative for .rooignore checking - const relativePath = path.relative(this.cwd, realPath).toPosix() + const relativePath = getWorkspaceRelativePath(rootPath, realPath) // Check if the real path is ignored - return !this.ignoreInstance.ignores(relativePath) + return !ignoreInstance.ignores(relativePath) } catch (error) { // Allow access to files outside cwd or on errors (backward compatibility) return true @@ -121,11 +190,6 @@ export class RooIgnoreController { * @returns path of file that is being accessed if it is being accessed, undefined if command is allowed */ validateCommand(command: string): string | undefined { - // Always allow if no .rooignore exists - if (!this.rooIgnoreContent) { - return undefined - } - // Split command into parts and get the base command const parts = command.trim().split(/\s+/) const baseCommand = parts[0].toLowerCase() @@ -153,12 +217,13 @@ export class RooIgnoreController { // Check each argument that could be a file path for (let i = 1; i < parts.length; i++) { const arg = parts[i] + const isWindowsAbsolutePath = path.win32.isAbsolute(arg) // Skip command flags/options (both Unix and PowerShell style) - if (arg.startsWith("-") || arg.startsWith("/")) { + if (arg.startsWith("-") || (arg.startsWith("/") && !path.isAbsolute(arg))) { continue } // Ignore PowerShell parameter names - if (arg.includes(":")) { + if (arg.includes(":") && !isWindowsAbsolutePath) { continue } // Validate file access @@ -204,10 +269,21 @@ export class RooIgnoreController { * @returns Formatted instructions or undefined if .rooignore doesn't exist */ getInstructions(): string | undefined { - if (!this.rooIgnoreContent) { + const ignoreEntries = this.getAvailableIgnoreContents() + if (ignoreEntries.length === 0) { return undefined } - return `# .rooignore\n\n(The following is provided by a root-level .rooignore file where the user has specified files and directories that should not be accessed. When using list_files, you'll notice a ${LOCK_TEXT_SYMBOL} next to files that are blocked. Attempting to access the file's contents e.g. through read_file will result in an error.)\n\n${this.rooIgnoreContent}\n.rooignore` + const sections = ignoreEntries + .map(({ rootPath, content }) => { + const workspaceName = + vscode.workspace.workspaceFolders?.find((folder) => folder.uri.fsPath === rootPath)?.name ?? + path.basename(rootPath) ?? + rootPath + return `## ${workspaceName}\n\n${content}\n.rooignore` + }) + .join("\n\n") + + return `# .rooignore\n\n(The following is provided by workspace-root .rooignore files where the user has specified files and directories that should not be accessed. When using list_files, you'll notice a ${LOCK_TEXT_SYMBOL} next to files that are blocked. Attempting to access the file's contents e.g. through read_file will result in an error.)\n\n${sections}` } } diff --git a/src/core/ignore/__tests__/RooIgnoreController.spec.ts b/src/core/ignore/__tests__/RooIgnoreController.spec.ts index 41d79476c6..f2d080786d 100644 --- a/src/core/ignore/__tests__/RooIgnoreController.spec.ts +++ b/src/core/ignore/__tests__/RooIgnoreController.spec.ts @@ -24,6 +24,7 @@ vi.mock("vscode", () => { return { workspace: { + workspaceFolders: undefined, createFileSystemWatcher: vi.fn(() => ({ onDidCreate: vi.fn(() => mockDisposable), onDidChange: vi.fn(() => mockDisposable), @@ -63,6 +64,7 @@ describe("RooIgnoreController", () => { // @ts-expect-error - Mocking vscode.workspace.createFileSystemWatcher.mockReturnValue(mockWatcher) + ;(vscode.workspace as any).workspaceFolders = undefined // Setup fs mocks mockFileExists = fileExistsAtPath as Mock @@ -198,6 +200,27 @@ describe("RooIgnoreController", () => { expect(controller.validateAccess(allowedAbsolutePath)).toBe(true) }) + it("should apply the .rooignore from a secondary workspace root for absolute paths", () => { + const secondaryRoot = "/test/secondary" + ;(vscode.workspace as any).workspaceFolders = [ + { uri: { fsPath: TEST_CWD }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + vi.mocked(fsSync.existsSync).mockImplementation( + (filePath) => filePath === path.join(secondaryRoot, ".rooignore"), + ) + vi.mocked(fsSync.readFileSync).mockImplementation((filePath) => { + if (filePath === path.join(secondaryRoot, ".rooignore")) { + return "private/**" + } + return "" + }) + + expect(controller.validateAccess(path.join(secondaryRoot, "private/secret.txt"))).toBe(false) + expect(controller.validateAccess(path.join(secondaryRoot, "src/app.ts"))).toBe(true) + }) + /** * Tests handling of paths outside cwd */ @@ -315,6 +338,42 @@ describe("RooIgnoreController", () => { expect(emptyController.validateCommand("cat node_modules/package.json")).toBeUndefined() expect(emptyController.validateCommand("grep pattern .git/config")).toBeUndefined() }) + + it("should enforce a secondary workspace .rooignore for execute_command paths", async () => { + const secondaryRoot = "/test/secondary" + ;(vscode.workspace as any).workspaceFolders = [ + { uri: { fsPath: TEST_CWD }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + mockFileExists.mockImplementation(async (filePath) => filePath === path.join(TEST_CWD, ".rooignore")) + mockReadFile.mockResolvedValue("node_modules\n.git\nsecrets/**\n*.log") + await controller.initialize() + + vi.mocked(fsSync.existsSync).mockImplementation( + (filePath) => filePath === path.join(secondaryRoot, ".rooignore"), + ) + vi.mocked(fsSync.readFileSync).mockImplementation((filePath) => { + if (filePath === path.join(secondaryRoot, ".rooignore")) { + return "private/**" + } + return "" + }) + + expect(controller.validateCommand(`cat ${path.join(secondaryRoot, "private", "secret.txt")}`)).toBe( + path.join(secondaryRoot, "private", "secret.txt"), + ) + }) + + it("should validate Windows absolute paths instead of treating them as PowerShell parameters", () => { + const windowsPath = String.raw`C:\secondary\private\secret.txt` + const validateAccessSpy = vi.spyOn(controller, "validateAccess").mockImplementation((candidate) => { + return candidate !== windowsPath + }) + + expect(controller.validateCommand(`type ${windowsPath}`)).toBe(windowsPath) + expect(validateAccessSpy).toHaveBeenCalledWith(windowsPath) + }) }) describe("filterPaths", () => { @@ -411,6 +470,40 @@ describe("RooIgnoreController", () => { const instructions = controller.getInstructions() expect(instructions).toBeUndefined() }) + + it("should include secondary workspace .rooignore content in instructions", async () => { + const secondaryRoot = "/test/secondary" + ;(vscode.workspace as any).workspaceFolders = [ + { uri: { fsPath: TEST_CWD }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + mockFileExists.mockImplementation(async (filePath) => filePath === path.join(TEST_CWD, ".rooignore")) + mockReadFile.mockResolvedValue("node_modules") + await controller.initialize() + + vi.mocked(fsSync.existsSync).mockImplementation( + (filePath) => + filePath === path.join(secondaryRoot, ".rooignore") || + filePath === path.join(TEST_CWD, ".rooignore"), + ) + vi.mocked(fsSync.readFileSync).mockImplementation((filePath) => { + if (filePath === path.join(secondaryRoot, ".rooignore")) { + return "private/**" + } + if (filePath === path.join(TEST_CWD, ".rooignore")) { + return "node_modules" + } + return "" + }) + + const instructions = controller.getInstructions() + + expect(instructions).toContain("## primary") + expect(instructions).toContain("## secondary") + expect(instructions).toContain("node_modules") + expect(instructions).toContain("private/**") + }) }) describe("dispose", () => { diff --git a/src/core/protect/RooProtectedController.ts b/src/core/protect/RooProtectedController.ts index 65676b2d11..b665b3c155 100644 --- a/src/core/protect/RooProtectedController.ts +++ b/src/core/protect/RooProtectedController.ts @@ -1,6 +1,8 @@ import path from "path" import ignore, { Ignore } from "ignore" +import { getWorkspaceRelativePath } from "../../utils/pathUtils" + export const SHIELD_SYMBOL = "\u{1F6E1}" /** @@ -39,13 +41,11 @@ export class RooProtectedController { */ isWriteProtected(filePath: string): boolean { try { - // Normalize path to be relative to cwd and use forward slashes - const absolutePath = path.resolve(this.cwd, filePath) - const relativePath = path.relative(this.cwd, absolutePath).toPosix() + const relativePath = getWorkspaceRelativePath(this.cwd, filePath) // Paths outside the cwd start with ".." and can't match any protected pattern. // The ignore library throws RangeError for such paths, so skip them early. - if (relativePath.startsWith("..")) { + if (relativePath.startsWith("..") || path.isAbsolute(relativePath)) { return false } diff --git a/src/core/protect/__tests__/RooProtectedController.spec.ts b/src/core/protect/__tests__/RooProtectedController.spec.ts index 0d1de58422..651c20f28e 100644 --- a/src/core/protect/__tests__/RooProtectedController.spec.ts +++ b/src/core/protect/__tests__/RooProtectedController.spec.ts @@ -1,11 +1,20 @@ import path from "path" import { RooProtectedController } from "../RooProtectedController" +vi.mock("vscode", () => ({ + workspace: { + workspaceFolders: undefined, + }, +})) + +import * as vscode from "vscode" + describe("RooProtectedController", () => { const TEST_CWD = "/test/workspace" let controller: RooProtectedController beforeEach(() => { + ;(vscode.workspace as any).workspaceFolders = undefined controller = new RooProtectedController(TEST_CWD) }) @@ -87,6 +96,17 @@ describe("RooProtectedController", () => { expect(controller.isWriteProtected(absolutePath)).toBe(true) }) + it("should protect absolute paths inside secondary workspace roots", () => { + const secondaryRoot = "/test/secondary" + ;(vscode.workspace as any).workspaceFolders = [ + { uri: { fsPath: TEST_CWD }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + expect(controller.isWriteProtected(path.join(secondaryRoot, "AGENTS.md"))).toBe(true) + expect(controller.isWriteProtected(path.join(secondaryRoot, "src/index.ts"))).toBe(false) + }) + it("should handle paths with different separators", () => { expect(controller.isWriteProtected(".roo\\config.json")).toBe(true) expect(controller.isWriteProtected(".roo/config.json")).toBe(true) diff --git a/src/core/tools/ApplyDiffTool.ts b/src/core/tools/ApplyDiffTool.ts index 3b664b3bd2..7432ceb788 100644 --- a/src/core/tools/ApplyDiffTool.ts +++ b/src/core/tools/ApplyDiffTool.ts @@ -4,7 +4,7 @@ import fs from "fs/promises" import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" -import { getReadablePath } from "../../utils/path" +import { getWorkspaceReadablePath, resolvePathInWorkspace } from "../../utils/pathUtils" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { fileExistsAtPath } from "../../utils/fs" @@ -47,7 +47,10 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { return } - const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) + const absolutePath = await resolvePathInWorkspace(task.cwd, relPath) + const diffPath = absolutePath === path.resolve(task.cwd, relPath) ? relPath : absolutePath + + const accessAllowed = task.rooIgnoreController?.validateAccess(absolutePath) if (!accessAllowed) { await task.say("rooignore_error", relPath) @@ -55,7 +58,6 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { return } - const absolutePath = path.resolve(task.cwd, relPath) const fileExists = await fileExistsAtPath(absolutePath) if (!fileExists) { @@ -136,11 +138,11 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { ) // Check if file is write-protected - const isWriteProtected = task.rooProtectedController?.isWriteProtected(relPath) || false + const isWriteProtected = task.rooProtectedController?.isWriteProtected(absolutePath) || false const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: diffContent, } @@ -177,7 +179,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { task.diffViewProvider.editType = "modify" task.diffViewProvider.originalContent = originalContent await task.diffViewProvider.saveDirectly( - relPath, + diffPath, diffResult.content, false, diagnosticsEnabled, @@ -187,7 +189,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { // Original behavior with diff view // Show diff view before asking for approval task.diffViewProvider.editType = "modify" - await task.diffViewProvider.open(relPath) + await task.diffViewProvider.open(diffPath) await task.diffViewProvider.update(diffResult.content, true) task.diffViewProvider.scrollToFirstDiff() @@ -278,9 +280,10 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> { return } + const absolutePath = relPath ? await resolvePathInWorkspace(task.cwd, relPath) : task.cwd const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: relPath ? getWorkspaceReadablePath(task.cwd, absolutePath, relPath) : "", diff: diffContent, } diff --git a/src/core/tools/ApplyPatchTool.ts b/src/core/tools/ApplyPatchTool.ts index 3f3295404b..947bfd66fc 100644 --- a/src/core/tools/ApplyPatchTool.ts +++ b/src/core/tools/ApplyPatchTool.ts @@ -3,8 +3,7 @@ import path from "path" import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" -import { getReadablePath } from "../../utils/path" -import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../utils/pathUtils" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" @@ -87,7 +86,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { // Process each hunk const readFile = async (filePath: string): Promise => { - const absolutePath = path.resolve(task.cwd, filePath) + const absolutePath = await resolvePathInWorkspace(task.cwd, filePath) return await fs.readFile(absolutePath, "utf8") } @@ -105,10 +104,11 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { // Process each file change for (const change of changes) { const relPath = change.path - const absolutePath = path.resolve(task.cwd, relPath) + const absolutePath = await resolvePathInWorkspace(task.cwd, relPath) + const diffPath = absolutePath === path.resolve(task.cwd, relPath) ? relPath : absolutePath // Check access permissions - const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) + const accessAllowed = task.rooIgnoreController?.validateAccess(absolutePath) if (!accessAllowed) { await task.say("rooignore_error", relPath) pushToolResult(formatResponse.rooIgnoreError(relPath)) @@ -116,17 +116,25 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { } // Check if file is write-protected - const isWriteProtected = task.rooProtectedController?.isWriteProtected(relPath) || false + const isWriteProtected = task.rooProtectedController?.isWriteProtected(absolutePath) || false if (change.type === "add") { // Create new file - await this.handleAddFile(change, absolutePath, relPath, task, callbacks, isWriteProtected) + await this.handleAddFile(change, absolutePath, diffPath, relPath, task, callbacks, isWriteProtected) } else if (change.type === "delete") { // Delete file await this.handleDeleteFile(absolutePath, relPath, task, callbacks, isWriteProtected) } else if (change.type === "update") { // Update file - await this.handleUpdateFile(change, absolutePath, relPath, task, callbacks, isWriteProtected) + await this.handleUpdateFile( + change, + absolutePath, + diffPath, + relPath, + task, + callbacks, + isWriteProtected, + ) } } @@ -141,6 +149,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { private async handleAddFile( change: ApplyPatchFileChange, absolutePath: string, + diffPath: string, relPath: string, task: Task, callbacks: ToolCallbacks, @@ -183,7 +192,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: sanitizedDiff, isOutsideWorkspace, } @@ -197,7 +206,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { // Show diff view if focus disruption prevention is disabled if (!isPreventFocusDisruptionEnabled) { - await task.diffViewProvider.open(relPath) + await task.diffViewProvider.open(diffPath) await task.diffViewProvider.update(newContent, true) task.diffViewProvider.scrollToFirstDiff() } @@ -215,7 +224,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { // Save the changes if (isPreventFocusDisruptionEnabled) { - await task.diffViewProvider.saveDirectly(relPath, newContent, true, diagnosticsEnabled, writeDelayMs) + await task.diffViewProvider.saveDirectly(diffPath, newContent, true, diagnosticsEnabled, writeDelayMs) } else { await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) } @@ -254,7 +263,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: `File will be deleted: ${relPath}`, isOutsideWorkspace, } @@ -290,6 +299,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { private async handleUpdateFile( change: ApplyPatchFileChange, absolutePath: string, + diffPath: string, relPath: string, task: Task, callbacks: ToolCallbacks, @@ -339,7 +349,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: sanitizedDiff, originalContent, isOutsideWorkspace, @@ -354,7 +364,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { // Show diff view if focus disruption prevention is disabled if (!isPreventFocusDisruptionEnabled) { - await task.diffViewProvider.open(relPath) + await task.diffViewProvider.open(diffPath) await task.diffViewProvider.update(newContent, true) task.diffViewProvider.scrollToFirstDiff() } @@ -372,10 +382,12 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { // Handle file move if specified if (change.movePath) { - const moveAbsolutePath = path.resolve(task.cwd, change.movePath) + const moveAbsolutePath = await resolvePathInWorkspace(task.cwd, change.movePath) + const moveDiffPath = + moveAbsolutePath === path.resolve(task.cwd, change.movePath) ? change.movePath : moveAbsolutePath // Validate destination path access permissions - const moveAccessAllowed = task.rooIgnoreController?.validateAccess(change.movePath) + const moveAccessAllowed = task.rooIgnoreController?.validateAccess(moveAbsolutePath) if (!moveAccessAllowed) { await task.say("rooignore_error", change.movePath) pushToolResult(formatResponse.rooIgnoreError(change.movePath)) @@ -384,7 +396,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { } // Check if destination path is write-protected - const isMovePathWriteProtected = task.rooProtectedController?.isWriteProtected(change.movePath) || false + const isMovePathWriteProtected = task.rooProtectedController?.isWriteProtected(moveAbsolutePath) || false if (isMovePathWriteProtected) { task.consecutiveMistakeCount++ task.recordToolError("apply_patch") @@ -410,7 +422,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { // Save new content to the new path if (isPreventFocusDisruptionEnabled) { await task.diffViewProvider.saveDirectly( - change.movePath, + moveDiffPath, newContent, false, diagnosticsEnabled, @@ -434,7 +446,7 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { } else { // Save changes to the same file if (isPreventFocusDisruptionEnabled) { - await task.diffViewProvider.saveDirectly(relPath, newContent, false, diagnosticsEnabled, writeDelayMs) + await task.diffViewProvider.saveDirectly(diffPath, newContent, false, diagnosticsEnabled, writeDelayMs) } else { await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) } @@ -455,8 +467,10 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> { const candidateRelPath = this.extractFirstPathFromPatch(patch) const fallbackDisplayPath = path.basename(task.cwd) || "workspace" const resolvedRelPath = candidateRelPath ?? "" - const absolutePath = path.resolve(task.cwd, resolvedRelPath) - const displayPath = candidateRelPath ? getReadablePath(task.cwd, candidateRelPath) : fallbackDisplayPath + const absolutePath = candidateRelPath ? await resolvePathInWorkspace(task.cwd, resolvedRelPath) : task.cwd + const displayPath = candidateRelPath + ? getWorkspaceReadablePath(task.cwd, absolutePath, candidateRelPath) + : fallbackDisplayPath let patchPreview: string | undefined if (patch) { diff --git a/src/core/tools/EditFileTool.ts b/src/core/tools/EditFileTool.ts index 2495a372bc..2378e25e28 100644 --- a/src/core/tools/EditFileTool.ts +++ b/src/core/tools/EditFileTool.ts @@ -3,8 +3,7 @@ import path from "path" import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" -import { getReadablePath } from "../../utils/path" -import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../utils/pathUtils" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" @@ -156,12 +155,12 @@ export class EditFileTool extends BaseTool<"edit_file"> { return } - const absolutePath = path.resolve(task.cwd, relPath) + const absolutePath = await resolvePathInWorkspace(task.cwd, relPath) const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: operationPreviewForErrorHandling, isOutsideWorkspace, } @@ -197,6 +196,8 @@ export class EditFileTool extends BaseTool<"edit_file"> { relPath = file_path } relPathForErrorHandling = relPath + const absolutePath = await resolvePathInWorkspace(task.cwd, file_path) + const diffPath = absolutePath === path.resolve(task.cwd, relPath) ? relPath : absolutePath operationPreviewForErrorHandling = old_string === "" @@ -206,7 +207,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { return `replacing: "${preview}"` })() - const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) + const accessAllowed = task.rooIgnoreController?.validateAccess(absolutePath) if (!accessAllowed) { // Finalize the partial tool preview before emitting any say() messages. @@ -218,9 +219,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { } // Check if file is write-protected - const isWriteProtected = task.rooProtectedController?.isWriteProtected(relPath) || false - - const absolutePath = path.resolve(task.cwd, relPath) + const isWriteProtected = task.rooProtectedController?.isWriteProtected(absolutePath) || false const fileExists = await fileExistsAtPath(absolutePath) let currentContent: string | null = null @@ -403,7 +402,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { const sharedMessageProps: ClineSayTool = { tool: isNewFile ? "newFileCreated" : "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: sanitizedDiff, isOutsideWorkspace, } @@ -417,7 +416,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { // Show diff view if focus disruption prevention is disabled if (!isPreventFocusDisruptionEnabled) { - await task.diffViewProvider.open(relPath) + await task.diffViewProvider.open(diffPath) await task.diffViewProvider.update(newContent, true) task.diffViewProvider.scrollToFirstDiff() } @@ -438,7 +437,7 @@ export class EditFileTool extends BaseTool<"edit_file"> { if (isPreventFocusDisruptionEnabled) { // Direct file write without diff view or opening the file await task.diffViewProvider.saveDirectly( - relPath, + diffPath, newContent, isNewFile, diagnosticsEnabled, @@ -511,12 +510,12 @@ export class EditFileTool extends BaseTool<"edit_file"> { this.didSendPartialToolAsk = true this.partialToolAskRelPath = relPath - const absolutePath = path.resolve(task.cwd, relPath) + const absolutePath = await resolvePathInWorkspace(task.cwd, relPath) const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: operationPreview, isOutsideWorkspace, } diff --git a/src/core/tools/EditTool.ts b/src/core/tools/EditTool.ts index 79338c17a6..20028b08b2 100644 --- a/src/core/tools/EditTool.ts +++ b/src/core/tools/EditTool.ts @@ -3,8 +3,7 @@ import path from "path" import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" -import { getReadablePath } from "../../utils/path" -import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../utils/pathUtils" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" @@ -64,7 +63,10 @@ export class EditTool extends BaseTool<"edit"> { return } - const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) + const absolutePath = await resolvePathInWorkspace(task.cwd, relPath) + const diffPath = absolutePath === path.resolve(task.cwd, relPath) ? relPath : absolutePath + + const accessAllowed = task.rooIgnoreController?.validateAccess(absolutePath) if (!accessAllowed) { await task.say("rooignore_error", relPath) @@ -73,9 +75,7 @@ export class EditTool extends BaseTool<"edit"> { } // Check if file is write-protected - const isWriteProtected = task.rooProtectedController?.isWriteProtected(relPath) || false - - const absolutePath = path.resolve(task.cwd, relPath) + const isWriteProtected = task.rooProtectedController?.isWriteProtected(absolutePath) || false const fileExists = await fileExistsAtPath(absolutePath) if (!fileExists) { @@ -178,7 +178,7 @@ export class EditTool extends BaseTool<"edit"> { const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: sanitizedDiff, isOutsideWorkspace, } @@ -192,7 +192,7 @@ export class EditTool extends BaseTool<"edit"> { // Show diff view if focus disruption prevention is disabled if (!isPreventFocusDisruptionEnabled) { - await task.diffViewProvider.open(relPath) + await task.diffViewProvider.open(diffPath) await task.diffViewProvider.update(newContent, true) task.diffViewProvider.scrollToFirstDiff() } @@ -212,7 +212,7 @@ export class EditTool extends BaseTool<"edit"> { // Save the changes if (isPreventFocusDisruptionEnabled) { // Direct file write without diff view or opening the file - await task.diffViewProvider.saveDirectly(relPath, newContent, false, diagnosticsEnabled, writeDelayMs) + await task.diffViewProvider.saveDirectly(diffPath, newContent, false, diagnosticsEnabled, writeDelayMs) } else { // Call saveChanges to update the DiffViewProvider properties await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) @@ -252,12 +252,12 @@ export class EditTool extends BaseTool<"edit"> { } // relPath is guaranteed non-null after hasPathStabilized - const absolutePath = path.resolve(task.cwd, relPath!) + const absolutePath = await resolvePathInWorkspace(task.cwd, relPath!) const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath!), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath!), diff: block.params.old_string ? "1 edit operation" : undefined, isOutsideWorkspace, } diff --git a/src/core/tools/ListFilesTool.ts b/src/core/tools/ListFilesTool.ts index 716d7ed784..a80463545e 100644 --- a/src/core/tools/ListFilesTool.ts +++ b/src/core/tools/ListFilesTool.ts @@ -1,12 +1,9 @@ -import * as path from "path" - import { type ClineSayTool } from "@roo-code/types" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { listFiles } from "../../services/glob/list-files" -import { getReadablePath } from "../../utils/path" -import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../utils/pathUtils" import type { ToolUse } from "../../shared/tools" import { BaseTool, ToolCallbacks } from "./BaseTool" @@ -34,7 +31,7 @@ export class ListFilesTool extends BaseTool<"list_files"> { task.consecutiveMistakeCount = 0 - const absolutePath = path.resolve(task.cwd, relDirPath) + const absolutePath = await resolvePathInWorkspace(task.cwd, relDirPath) const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const [files, didHitLimit] = await listFiles(absolutePath, recursive || false, 200) @@ -51,7 +48,7 @@ export class ListFilesTool extends BaseTool<"list_files"> { const sharedMessageProps: ClineSayTool = { tool: !recursive ? "listFilesTopLevel" : "listFilesRecursive", - path: getReadablePath(task.cwd, relDirPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relDirPath), isOutsideWorkspace, } @@ -73,12 +70,12 @@ export class ListFilesTool extends BaseTool<"list_files"> { const recursiveRaw: string | undefined = block.params.recursive const recursive = recursiveRaw?.toLowerCase() === "true" - const absolutePath = relDirPath ? path.resolve(task.cwd, relDirPath) : task.cwd + const absolutePath = relDirPath ? await resolvePathInWorkspace(task.cwd, relDirPath) : task.cwd const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const sharedMessageProps: ClineSayTool = { tool: !recursive ? "listFilesTopLevel" : "listFilesRecursive", - path: getReadablePath(task.cwd, relDirPath ?? ""), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relDirPath ?? ""), isOutsideWorkspace, } diff --git a/src/core/tools/ReadFileTool.ts b/src/core/tools/ReadFileTool.ts index 8ad6a3b33d..4b59c59779 100644 --- a/src/core/tools/ReadFileTool.ts +++ b/src/core/tools/ReadFileTool.ts @@ -18,7 +18,7 @@ import { isLegacyReadFileParams, type ClineSayTool } from "@roo-code/types" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" -import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../utils/pathUtils" import { getReadablePath } from "../../utils/path" import { extractTextFromFile, addLineNumbers, getSupportedBinaryFormats } from "../../integrations/misc/extract-text" import { readWithIndentation, readWithSlice } from "../../integrations/misc/indentation-reader" @@ -145,9 +145,10 @@ export class ReadFileTool extends BaseTool<"read_file"> { for (const fileResult of fileResults) { const relPath = fileResult.path + const fullPath = await resolvePathInWorkspace(task.cwd, relPath) // RooIgnore validation - const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) + const accessAllowed = task.rooIgnoreController?.validateAccess(fullPath) if (!accessAllowed) { await task.say("rooignore_error", relPath) const errorMsg = formatResponse.rooIgnoreError(relPath) @@ -177,7 +178,7 @@ export class ReadFileTool extends BaseTool<"read_file"> { if (fileResult.status !== "approved") continue const relPath = fileResult.path - const fullPath = path.resolve(task.cwd, relPath) + const fullPath = await resolvePathInWorkspace(task.cwd, relPath) const entry = fileResult.entry! try { @@ -433,17 +434,19 @@ export class ReadFileTool extends BaseTool<"read_file"> { if (filesToApprove.length > 1) { // Batch approval - const batchFiles = filesToApprove.map((fileResult) => { - const relPath = fileResult.path - const fullPath = path.resolve(task.cwd, relPath) - const isOutsideWorkspace = isPathOutsideWorkspace(fullPath) - const readablePath = getReadablePath(task.cwd, relPath) - - const lineSnippet = this.getLineSnippet(fileResult.entry!) - const key = `${readablePath}${lineSnippet ? ` (${lineSnippet})` : ""}` - - return { path: readablePath, lineSnippet, isOutsideWorkspace, key, content: fullPath } - }) + const batchFiles = await Promise.all( + filesToApprove.map(async (fileResult) => { + const relPath = fileResult.path + const fullPath = await resolvePathInWorkspace(task.cwd, relPath) + const isOutsideWorkspace = isPathOutsideWorkspace(fullPath) + const readablePath = getWorkspaceReadablePath(task.cwd, fullPath, relPath) + + const lineSnippet = this.getLineSnippet(fileResult.entry!) + const key = `${readablePath}${lineSnippet ? ` (${lineSnippet})` : ""}` + + return { path: readablePath, lineSnippet, isOutsideWorkspace, key, content: fullPath } + }), + ) const completeMessage = JSON.stringify({ tool: "readFile", batchFiles } satisfies ClineSayTool) const { response, text, images } = await task.ask("tool", completeMessage, false) @@ -500,15 +503,16 @@ export class ReadFileTool extends BaseTool<"read_file"> { // Single file approval const fileResult = filesToApprove[0] const relPath = fileResult.path - const fullPath = path.resolve(task.cwd, relPath) + const fullPath = await resolvePathInWorkspace(task.cwd, relPath) const isOutsideWorkspace = isPathOutsideWorkspace(fullPath) + const readablePath = getWorkspaceReadablePath(task.cwd, fullPath, relPath) const lineSnippet = this.getLineSnippet(fileResult.entry!) const startLine = this.getStartLine(fileResult.entry!) const completeMessage = JSON.stringify({ tool: "readFile", - path: getReadablePath(task.cwd, relPath), + path: readablePath, isOutsideWorkspace, content: fullPath, reason: lineSnippet, @@ -647,10 +651,12 @@ export class ReadFileTool extends BaseTool<"read_file"> { } } - const fullPath = filePath ? path.resolve(task.cwd, filePath) : "" + const fullPath = filePath ? await resolvePathInWorkspace(task.cwd, filePath) : "" const sharedMessageProps: ClineSayTool = { tool: "readFile", - path: getReadablePath(task.cwd, filePath), + path: filePath + ? getWorkspaceReadablePath(task.cwd, fullPath, filePath) + : getReadablePath(task.cwd, filePath), isOutsideWorkspace: filePath ? isPathOutsideWorkspace(fullPath) : false, } const partialMessage = JSON.stringify({ @@ -686,10 +692,10 @@ export class ReadFileTool extends BaseTool<"read_file"> { for (const entry of fileEntries) { const relPath = entry.path - const fullPath = path.resolve(task.cwd, relPath) + const fullPath = await resolvePathInWorkspace(task.cwd, relPath) // RooIgnore validation - const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) + const accessAllowed = task.rooIgnoreController?.validateAccess(fullPath) if (!accessAllowed) { await task.say("rooignore_error", relPath) const errorMsg = formatResponse.rooIgnoreError(relPath) @@ -699,6 +705,7 @@ export class ReadFileTool extends BaseTool<"read_file"> { // Request approval for single file const isOutsideWorkspace = isPathOutsideWorkspace(fullPath) + const readablePath = getWorkspaceReadablePath(task.cwd, fullPath, relPath) let lineSnippet = "" if (entry.lineRanges && entry.lineRanges.length > 0) { const ranges = entry.lineRanges.map((range: LineRange) => `(lines ${range.start}-${range.end})`) @@ -707,7 +714,7 @@ export class ReadFileTool extends BaseTool<"read_file"> { const completeMessage = JSON.stringify({ tool: "readFile", - path: getReadablePath(task.cwd, relPath), + path: readablePath, isOutsideWorkspace, content: fullPath, reason: lineSnippet || undefined, diff --git a/src/core/tools/SearchFilesTool.ts b/src/core/tools/SearchFilesTool.ts index 3230c043e0..d74815f78c 100644 --- a/src/core/tools/SearchFilesTool.ts +++ b/src/core/tools/SearchFilesTool.ts @@ -1,10 +1,7 @@ -import path from "path" - import { type ClineSayTool } from "@roo-code/types" import { Task } from "../task/Task" -import { getReadablePath } from "../../utils/path" -import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../utils/pathUtils" import { regexSearchFiles } from "../../services/ripgrep" import type { ToolUse } from "../../shared/tools" @@ -44,12 +41,12 @@ export class SearchFilesTool extends BaseTool<"search_files"> { task.consecutiveMistakeCount = 0 - const absolutePath = path.resolve(task.cwd, relDirPath) + const absolutePath = await resolvePathInWorkspace(task.cwd, relDirPath) const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const sharedMessageProps: ClineSayTool = { tool: "searchFiles", - path: getReadablePath(task.cwd, relDirPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relDirPath), regex: regex, filePattern: filePattern, isOutsideWorkspace, @@ -76,12 +73,12 @@ export class SearchFilesTool extends BaseTool<"search_files"> { const regex = block.params.regex const filePattern = block.params.file_pattern - const absolutePath = relDirPath ? path.resolve(task.cwd, relDirPath) : task.cwd + const absolutePath = relDirPath ? await resolvePathInWorkspace(task.cwd, relDirPath) : task.cwd const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const sharedMessageProps: ClineSayTool = { tool: "searchFiles", - path: getReadablePath(task.cwd, relDirPath ?? ""), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relDirPath ?? ""), regex: regex ?? "", filePattern: filePattern ?? "", isOutsideWorkspace, diff --git a/src/core/tools/SearchReplaceTool.ts b/src/core/tools/SearchReplaceTool.ts index 2d8817364f..741c34e72f 100644 --- a/src/core/tools/SearchReplaceTool.ts +++ b/src/core/tools/SearchReplaceTool.ts @@ -3,8 +3,7 @@ import path from "path" import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types" -import { getReadablePath } from "../../utils/path" -import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../utils/pathUtils" import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" @@ -69,7 +68,10 @@ export class SearchReplaceTool extends BaseTool<"search_replace"> { relPath = file_path } - const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) + const absolutePath = await resolvePathInWorkspace(task.cwd, file_path) + const diffPath = absolutePath === path.resolve(task.cwd, relPath) ? relPath : absolutePath + + const accessAllowed = task.rooIgnoreController?.validateAccess(absolutePath) if (!accessAllowed) { await task.say("rooignore_error", relPath) @@ -78,9 +80,7 @@ export class SearchReplaceTool extends BaseTool<"search_replace"> { } // Check if file is write-protected - const isWriteProtected = task.rooProtectedController?.isWriteProtected(relPath) || false - - const absolutePath = path.resolve(task.cwd, relPath) + const isWriteProtected = task.rooProtectedController?.isWriteProtected(absolutePath) || false const fileExists = await fileExistsAtPath(absolutePath) if (!fileExists) { @@ -174,7 +174,7 @@ export class SearchReplaceTool extends BaseTool<"search_replace"> { const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: sanitizedDiff, isOutsideWorkspace, } @@ -188,7 +188,7 @@ export class SearchReplaceTool extends BaseTool<"search_replace"> { // Show diff view if focus disruption prevention is disabled if (!isPreventFocusDisruptionEnabled) { - await task.diffViewProvider.open(relPath) + await task.diffViewProvider.open(diffPath) await task.diffViewProvider.update(newContent, true) task.diffViewProvider.scrollToFirstDiff() } @@ -208,7 +208,7 @@ export class SearchReplaceTool extends BaseTool<"search_replace"> { // Save the changes if (isPreventFocusDisruptionEnabled) { // Direct file write without diff view or opening the file - await task.diffViewProvider.saveDirectly(relPath, newContent, false, diagnosticsEnabled, writeDelayMs) + await task.diffViewProvider.saveDirectly(diffPath, newContent, false, diagnosticsEnabled, writeDelayMs) } else { // Call saveChanges to update the DiffViewProvider properties await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs) @@ -261,12 +261,12 @@ export class SearchReplaceTool extends BaseTool<"search_replace"> { relPath = path.relative(task.cwd, relPath) } - const absolutePath = path.resolve(task.cwd, relPath) + const absolutePath = await resolvePathInWorkspace(task.cwd, relPath) const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const sharedMessageProps: ClineSayTool = { tool: "appliedDiff", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), diff: operationPreview, isOutsideWorkspace, } diff --git a/src/core/tools/WriteToFileTool.ts b/src/core/tools/WriteToFileTool.ts index c8455ef3d9..d2505df47c 100644 --- a/src/core/tools/WriteToFileTool.ts +++ b/src/core/tools/WriteToFileTool.ts @@ -9,8 +9,7 @@ import { formatResponse } from "../prompts/responses" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" import { fileExistsAtPath, createDirectoriesForFile } from "../../utils/fs" import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text" -import { getReadablePath } from "../../utils/path" -import { isPathOutsideWorkspace } from "../../utils/pathUtils" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../utils/pathUtils" import { unescapeHtmlEntities } from "../../utils/text-normalization" import { EXPERIMENT_IDS, experiments } from "../../shared/experiments" import { convertNewFileToUnifiedDiff, computeDiffStats, sanitizeUnifiedDiff } from "../diff/stats" @@ -47,7 +46,11 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> { return } - const accessAllowed = task.rooIgnoreController?.validateAccess(relPath) + let fileExists: boolean + const absolutePath = await resolvePathInWorkspace(task.cwd, relPath) + const diffPath = absolutePath === path.resolve(task.cwd, relPath) ? relPath : absolutePath + + const accessAllowed = task.rooIgnoreController?.validateAccess(absolutePath) if (!accessAllowed) { await task.say("rooignore_error", relPath) @@ -55,10 +58,7 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> { return } - const isWriteProtected = task.rooProtectedController?.isWriteProtected(relPath) || false - - let fileExists: boolean - const absolutePath = path.resolve(task.cwd, relPath) + const isWriteProtected = task.rooProtectedController?.isWriteProtected(absolutePath) || false if (task.diffViewProvider.editType !== undefined) { fileExists = task.diffViewProvider.editType === "modify" @@ -85,12 +85,12 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> { newContent = unescapeHtmlEntities(newContent) } - const fullPath = relPath ? path.resolve(task.cwd, relPath) : "" + const fullPath = relPath ? absolutePath : "" const isOutsideWorkspace = isPathOutsideWorkspace(fullPath) const sharedMessageProps: ClineSayTool = { tool: fileExists ? "editedExistingFile" : "newFileCreated", - path: getReadablePath(task.cwd, relPath), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath), content: newContent, isOutsideWorkspace, isProtected: isWriteProtected, @@ -111,7 +111,6 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> { if (isPreventFocusDisruptionEnabled) { task.diffViewProvider.editType = fileExists ? "modify" : "create" if (fileExists) { - const absolutePath = path.resolve(task.cwd, relPath) task.diffViewProvider.originalContent = await fs.readFile(absolutePath, "utf-8") } else { task.diffViewProvider.originalContent = "" @@ -133,12 +132,12 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> { return } - await task.diffViewProvider.saveDirectly(relPath, newContent, false, diagnosticsEnabled, writeDelayMs) + await task.diffViewProvider.saveDirectly(diffPath, newContent, false, diagnosticsEnabled, writeDelayMs) } else { if (!task.diffViewProvider.isEditing) { const partialMessage = JSON.stringify(sharedMessageProps) await task.ask("tool", partialMessage, true).catch(() => {}) - await task.diffViewProvider.open(relPath) + await task.diffViewProvider.open(diffPath) } await task.diffViewProvider.update( @@ -215,7 +214,8 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> { // relPath is guaranteed non-null after hasPathStabilized let fileExists: boolean - const absolutePath = path.resolve(task.cwd, relPath!) + const absolutePath = await resolvePathInWorkspace(task.cwd, relPath!) + const diffPath = absolutePath === path.resolve(task.cwd, relPath!) ? relPath! : absolutePath if (task.diffViewProvider.editType !== undefined) { fileExists = task.diffViewProvider.editType === "modify" @@ -230,12 +230,12 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> { await createDirectoriesForFile(absolutePath) } - const isWriteProtected = task.rooProtectedController?.isWriteProtected(relPath!) || false + const isWriteProtected = task.rooProtectedController?.isWriteProtected(absolutePath) || false const isOutsideWorkspace = isPathOutsideWorkspace(absolutePath) const sharedMessageProps: ClineSayTool = { tool: fileExists ? "editedExistingFile" : "newFileCreated", - path: getReadablePath(task.cwd, relPath!), + path: getWorkspaceReadablePath(task.cwd, absolutePath, relPath!), content: newContent || "", isOutsideWorkspace, isProtected: isWriteProtected, @@ -246,7 +246,7 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> { if (newContent) { if (!task.diffViewProvider.isEditing) { - await task.diffViewProvider.open(relPath!) + await task.diffViewProvider.open(diffPath) } await task.diffViewProvider.update( diff --git a/src/core/tools/__tests__/applyDiffTool.spec.ts b/src/core/tools/__tests__/applyDiffTool.spec.ts new file mode 100644 index 0000000000..acd4f2479c --- /dev/null +++ b/src/core/tools/__tests__/applyDiffTool.spec.ts @@ -0,0 +1,167 @@ +import fs from "fs/promises" + +import type { MockedFunction } from "vitest" + +import { fileExistsAtPath } from "../../../utils/fs" +import { getWorkspaceReadablePath, resolvePathInWorkspace } from "../../../utils/pathUtils" +import type { ToolUse } from "../../../shared/tools" +import { applyDiffTool } from "../ApplyDiffTool" + +vi.mock("fs/promises", () => ({ + default: { + readFile: vi.fn(), + }, +})) + +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn(), +})) + +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + resolvePathInWorkspace: vi.fn(), + getWorkspaceReadablePath: vi.fn(), + } +}) + +vi.mock("../../prompts/responses", () => ({ + formatResponse: { + createPrettyPatch: vi.fn(() => "--- patch ---"), + rooIgnoreError: vi.fn((filePath: string) => `Access denied: ${filePath}`), + }, +})) + +vi.mock("../../diff/stats", () => ({ + sanitizeUnifiedDiff: vi.fn((diff: string) => diff), + computeDiffStats: vi.fn(() => ({ additions: 1, deletions: 1 })), +})) + +vi.mock("../../../shared/experiments", () => ({ + EXPERIMENT_IDS: { + PREVENT_FOCUS_DISRUPTION: "prevent-focus-disruption", + }, + experiments: { + isEnabled: vi.fn().mockReturnValue(false), + }, +})) + +describe("applyDiffTool", () => { + const cwd = "/workspace/primary" + const relPath = "secondary/file.ts" + const absolutePath = "/workspace/secondary/file.ts" + + const mockedReadFile = fs.readFile as MockedFunction + const mockedFileExistsAtPath = fileExistsAtPath as MockedFunction + const mockedResolvePathInWorkspace = resolvePathInWorkspace as MockedFunction + const mockedGetWorkspaceReadablePath = getWorkspaceReadablePath as MockedFunction + + let task: any + let askApproval: ReturnType + let pushToolResult: ReturnType + let handleError: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + + mockedReadFile.mockResolvedValue("old\n") + mockedFileExistsAtPath.mockResolvedValue(true) + mockedResolvePathInWorkspace.mockResolvedValue(absolutePath) + mockedGetWorkspaceReadablePath.mockReturnValue("secondary/file.ts") + + task = { + cwd, + taskId: "task-123", + api: { + getModel: vi.fn().mockReturnValue({ id: "claude-3" }), + }, + consecutiveMistakeCount: 0, + consecutiveMistakeCountForApplyDiff: new Map(), + didEditFile: false, + providerRef: { + deref: vi.fn().mockReturnValue({ + getState: vi.fn().mockResolvedValue({ + diagnosticsEnabled: true, + writeDelayMs: 10, + experiments: {}, + }), + }), + }, + rooIgnoreController: { + validateAccess: vi.fn().mockReturnValue(true), + }, + rooProtectedController: { + isWriteProtected: vi.fn().mockReturnValue(false), + }, + diffStrategy: { + applyDiff: vi.fn().mockResolvedValue({ success: true, content: "new\n" }), + }, + diffViewProvider: { + editType: undefined, + originalContent: "", + open: vi.fn().mockResolvedValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + scrollToFirstDiff: vi.fn(), + saveChanges: vi.fn().mockResolvedValue(undefined), + revertChanges: vi.fn().mockResolvedValue(undefined), + reset: vi.fn().mockResolvedValue(undefined), + pushToolWriteResult: vi.fn().mockResolvedValue("Tool result"), + }, + fileContextTracker: { + trackFileContext: vi.fn().mockResolvedValue(undefined), + }, + recordToolError: vi.fn(), + recordToolUsage: vi.fn(), + processQueuedMessages: vi.fn(), + say: vi.fn().mockResolvedValue(undefined), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing param"), + } + + askApproval = vi.fn().mockResolvedValue(true) + pushToolResult = vi.fn() + handleError = vi.fn() + }) + + it("uses the resolved path for access checks and the workspace-readable path in approval payloads", async () => { + await applyDiffTool.execute({ path: relPath, diff: "@@ -1 +1 @@\n-old\n+new\n" }, task, { + askApproval, + pushToolResult, + handleError, + }) + + expect(mockedResolvePathInWorkspace).toHaveBeenCalledWith(cwd, relPath) + expect(task.rooIgnoreController.validateAccess).toHaveBeenCalledWith(absolutePath) + expect(task.rooProtectedController.isWriteProtected).toHaveBeenCalledWith(absolutePath) + expect(task.diffViewProvider.open).toHaveBeenCalledWith(absolutePath) + expect(askApproval).toHaveBeenCalledWith( + "tool", + expect.stringContaining('"path":"secondary/file.ts"'), + undefined, + false, + ) + expect(task.fileContextTracker.trackFileContext).toHaveBeenCalledWith(relPath, "roo_edited") + expect(pushToolResult).toHaveBeenCalledWith(expect.stringContaining("Tool result")) + }) + + it("uses the resolved workspace path in partial payloads after the path stabilizes", async () => { + const block: ToolUse<"apply_diff"> = { + type: "tool_use", + name: "apply_diff", + params: { path: relPath, diff: "@@ -1 +1 @@\n-old\n+new\n" }, + partial: true, + } + + task.ask = vi.fn().mockResolvedValue(undefined) + + await applyDiffTool.handlePartial(task, block) + await applyDiffTool.handlePartial(task, block) + + expect(task.ask).toHaveBeenCalledWith( + "tool", + expect.stringContaining('"path":"secondary/file.ts"'), + true, + undefined, + ) + }) +}) diff --git a/src/core/tools/__tests__/applyPatchTool.execute.spec.ts b/src/core/tools/__tests__/applyPatchTool.execute.spec.ts new file mode 100644 index 0000000000..1de286fdb9 --- /dev/null +++ b/src/core/tools/__tests__/applyPatchTool.execute.spec.ts @@ -0,0 +1,239 @@ +import path from "path" +import fs from "fs/promises" + +import type { MockedFunction } from "vitest" + +import { ApplyPatchTool } from "../ApplyPatchTool" +import type { ToolCallbacks } from "../BaseTool" +import type { Task } from "../../task/Task" +import { fileExistsAtPath } from "../../../utils/fs" +import { isPathOutsideWorkspace, resolvePathInWorkspace, getWorkspaceReadablePath } from "../../../utils/pathUtils" +import { parsePatch, processAllHunks } from "../apply-patch" + +vi.mock("fs/promises", () => ({ + default: { + readFile: vi.fn(), + writeFile: vi.fn(), + mkdir: vi.fn(), + unlink: vi.fn(), + }, +})) + +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn(), +})) + +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + isPathOutsideWorkspace: vi.fn(), + resolvePathInWorkspace: vi.fn(), + getWorkspaceReadablePath: vi.fn(), + } +}) + +vi.mock("../../prompts/responses", () => ({ + formatResponse: { + createPrettyPatch: vi.fn((filePath: string, oldContent: string, newContent: string) => { + return `--- ${filePath}\n+++ ${filePath}\n-${oldContent}\n+${newContent}` + }), + rooIgnoreError: vi.fn((filePath: string) => `Access denied: ${filePath}`), + toolError: vi.fn((message: string) => `Error: ${message}`), + }, +})) + +vi.mock("../../diff/stats", () => ({ + sanitizeUnifiedDiff: vi.fn((diff: string) => diff), + computeDiffStats: vi.fn(() => ({ additions: 1, deletions: 1 })), +})) + +vi.mock("../../../shared/experiments", () => ({ + EXPERIMENT_IDS: { + PREVENT_FOCUS_DISRUPTION: "prevent-focus-disruption", + }, + experiments: { + isEnabled: vi.fn().mockReturnValue(false), + }, +})) + +vi.mock("../apply-patch", () => ({ + parsePatch: vi.fn(), + processAllHunks: vi.fn(), + ParseError: class ParseError extends Error {}, +})) + +describe("ApplyPatchTool.execute", () => { + const cwd = process.platform === "win32" ? "C:\\workspace\\primary" : "/workspace/primary" + const secondaryRoot = process.platform === "win32" ? "C:\\workspace\\secondary" : "/workspace/secondary" + + const mockedReadFile = fs.readFile as MockedFunction + const mockedWriteFile = fs.writeFile as MockedFunction + const mockedMkdir = fs.mkdir as MockedFunction + const mockedUnlink = fs.unlink as MockedFunction + const mockedFileExistsAtPath = fileExistsAtPath as MockedFunction + const mockedResolvePathInWorkspace = resolvePathInWorkspace as MockedFunction + const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction + const mockedGetWorkspaceReadablePath = getWorkspaceReadablePath as MockedFunction + const mockedParsePatch = parsePatch as MockedFunction + const mockedProcessAllHunks = processAllHunks as MockedFunction + + let tool: ApplyPatchTool + let task: any + let callbacks: ToolCallbacks + let askApproval: ReturnType + let pushToolResult: ReturnType + + const getExpectedDiffPath = (absolutePath: string, relPath: string) => + absolutePath === path.resolve(cwd, relPath) ? relPath : absolutePath + + beforeEach(() => { + vi.clearAllMocks() + + tool = new ApplyPatchTool() + askApproval = vi.fn().mockResolvedValue(true) + pushToolResult = vi.fn() + + callbacks = { + askApproval, + pushToolResult, + handleError: vi.fn(), + } + + task = { + cwd, + consecutiveMistakeCount: 0, + didEditFile: false, + diffViewProvider: { + editType: undefined, + originalContent: undefined, + open: vi.fn().mockResolvedValue(undefined), + update: vi.fn().mockResolvedValue(undefined), + scrollToFirstDiff: vi.fn(), + saveChanges: vi.fn().mockResolvedValue(undefined), + saveDirectly: vi.fn().mockResolvedValue(undefined), + pushToolWriteResult: vi.fn().mockResolvedValue("Tool result"), + reset: vi.fn().mockResolvedValue(undefined), + revertChanges: vi.fn().mockResolvedValue(undefined), + }, + fileContextTracker: { + trackFileContext: vi.fn().mockResolvedValue(undefined), + }, + providerRef: { + deref: vi.fn().mockReturnValue({ + getState: vi.fn().mockResolvedValue({ + diagnosticsEnabled: true, + writeDelayMs: 25, + experiments: {}, + }), + }), + }, + rooIgnoreController: { + validateAccess: vi.fn().mockReturnValue(true), + }, + rooProtectedController: { + isWriteProtected: vi.fn().mockReturnValue(false), + }, + processQueuedMessages: vi.fn(), + recordToolError: vi.fn(), + recordToolUsage: vi.fn(), + say: vi.fn().mockResolvedValue(undefined), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing param"), + } + + mockedReadFile.mockResolvedValue("") + mockedWriteFile.mockResolvedValue(undefined) + mockedMkdir.mockResolvedValue(undefined) + mockedUnlink.mockResolvedValue(undefined) + mockedFileExistsAtPath.mockResolvedValue(false) + mockedResolvePathInWorkspace.mockImplementation(async (_cwd, filePath) => path.join(cwd, filePath)) + mockedIsPathOutsideWorkspace.mockReturnValue(false) + mockedGetWorkspaceReadablePath.mockImplementation((_cwd, _absolutePath, fallbackPath) => fallbackPath ?? "file") + mockedParsePatch.mockReturnValue({ hunks: [{}] } as any) + mockedProcessAllHunks.mockResolvedValue([]) + }) + + it("opens add-file diffs against the resolved secondary-root absolute path", async () => { + const relPath = "nested/new-file.ts" + const absolutePath = path.join(secondaryRoot, relPath) + + mockedResolvePathInWorkspace.mockResolvedValue(absolutePath) + mockedGetWorkspaceReadablePath.mockReturnValue(`secondary/${relPath}`) + mockedProcessAllHunks.mockResolvedValue([ + { + type: "add", + path: relPath, + newContent: "export const value = 1\n", + }, + ] as any) + + await tool.execute( + { patch: "*** Begin Patch\n*** Add File: nested/new-file.ts\n*** End Patch" }, + task as Task, + callbacks, + ) + + expect(task.rooIgnoreController.validateAccess).toHaveBeenCalledWith(absolutePath) + expect(task.diffViewProvider.open).toHaveBeenCalledWith(absolutePath) + expect(task.diffViewProvider.update).toHaveBeenCalledWith("export const value = 1\n", true) + expect(askApproval).toHaveBeenCalledWith( + "tool", + expect.stringContaining(`"path":"secondary/${relPath}"`), + undefined, + false, + ) + expect(task.fileContextTracker.trackFileContext).toHaveBeenCalledWith(relPath, "roo_edited") + expect(pushToolResult).toHaveBeenCalledWith("Tool result") + }) + + it("writes moved files to the resolved secondary-root destination", async () => { + const relPath = "src/original.ts" + const absolutePath = path.join(cwd, relPath) + const movePath = "secondary/moved.ts" + const moveAbsolutePath = path.join(secondaryRoot, "moved.ts") + + mockedFileExistsAtPath.mockResolvedValue(true) + mockedResolvePathInWorkspace.mockImplementation(async (_cwd, filePath) => { + if (filePath === relPath) { + return absolutePath + } + if (filePath === movePath) { + return moveAbsolutePath + } + return path.join(cwd, filePath) + }) + mockedGetWorkspaceReadablePath.mockImplementation((_cwd, absolute, fallbackPath) => { + if (absolute === absolutePath) { + return relPath + } + if (absolute === moveAbsolutePath) { + return "secondary/moved.ts" + } + return fallbackPath ?? "file" + }) + mockedProcessAllHunks.mockResolvedValue([ + { + type: "update", + path: relPath, + originalContent: "old\n", + newContent: "new\n", + movePath, + }, + ] as any) + + await tool.execute( + { patch: "*** Begin Patch\n*** Update File: src/original.ts\n*** End Patch" }, + task as Task, + callbacks, + ) + + expect(task.rooIgnoreController.validateAccess).toHaveBeenNthCalledWith(1, absolutePath) + expect(task.rooIgnoreController.validateAccess).toHaveBeenNthCalledWith(2, moveAbsolutePath) + expect(task.diffViewProvider.open).toHaveBeenCalledWith(getExpectedDiffPath(absolutePath, relPath)) + expect(mockedMkdir).toHaveBeenCalledWith(path.dirname(moveAbsolutePath), { recursive: true }) + expect(mockedWriteFile).toHaveBeenCalledWith(moveAbsolutePath, "new\n", "utf8") + expect(mockedUnlink).toHaveBeenCalledWith(absolutePath) + expect(task.fileContextTracker.trackFileContext).toHaveBeenCalledWith(movePath, "roo_edited") + expect(pushToolResult).toHaveBeenCalledWith("Tool result") + }) +}) diff --git a/src/core/tools/__tests__/applyPatchTool.partial.spec.ts b/src/core/tools/__tests__/applyPatchTool.partial.spec.ts index 7fe241a126..cc743f06c9 100644 --- a/src/core/tools/__tests__/applyPatchTool.partial.spec.ts +++ b/src/core/tools/__tests__/applyPatchTool.partial.spec.ts @@ -7,9 +7,21 @@ import { isPathOutsideWorkspace } from "../../../utils/pathUtils" import type { Task } from "../../task/Task" import { ApplyPatchTool } from "../ApplyPatchTool" -vi.mock("../../../utils/pathUtils", () => ({ - isPathOutsideWorkspace: vi.fn(), -})) +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + isPathOutsideWorkspace: vi.fn(), + resolvePathInWorkspace: vi + .fn() + .mockImplementation(async (cwd: string, filePath: string) => path.resolve(cwd, filePath)), + getWorkspaceReadablePath: vi + .fn() + .mockImplementation( + (cwd: string, _absolutePath: string, fallbackPath?: string) => fallbackPath ?? path.basename(cwd), + ), + } +}) interface PartialApplyPatchPayload { tool: string diff --git a/src/core/tools/__tests__/editFileTool.spec.ts b/src/core/tools/__tests__/editFileTool.spec.ts index 80d431edab..86d06be913 100644 --- a/src/core/tools/__tests__/editFileTool.spec.ts +++ b/src/core/tools/__tests__/editFileTool.spec.ts @@ -44,9 +44,21 @@ vi.mock("../../prompts/responses", () => ({ }, })) -vi.mock("../../../utils/pathUtils", () => ({ - isPathOutsideWorkspace: vi.fn().mockReturnValue(false), -})) +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + isPathOutsideWorkspace: vi.fn().mockReturnValue(false), + resolvePathInWorkspace: vi + .fn() + .mockImplementation(async (cwd: string, filePath: string) => path.resolve(cwd, filePath)), + getWorkspaceReadablePath: vi + .fn() + .mockImplementation( + (_cwd: string, _absolutePath: string, fallbackPath?: string) => fallbackPath ?? "test/path.txt", + ), + } +}) vi.mock("../../../utils/path", () => ({ getReadablePath: vi.fn().mockReturnValue("test/path.txt"), diff --git a/src/core/tools/__tests__/editTool.spec.ts b/src/core/tools/__tests__/editTool.spec.ts index 9e61fcee23..3ea2700ad0 100644 --- a/src/core/tools/__tests__/editTool.spec.ts +++ b/src/core/tools/__tests__/editTool.spec.ts @@ -44,9 +44,21 @@ vi.mock("../../prompts/responses", () => ({ }, })) -vi.mock("../../../utils/pathUtils", () => ({ - isPathOutsideWorkspace: vi.fn().mockReturnValue(false), -})) +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + isPathOutsideWorkspace: vi.fn().mockReturnValue(false), + resolvePathInWorkspace: vi + .fn() + .mockImplementation(async (cwd: string, filePath: string) => path.resolve(cwd, filePath)), + getWorkspaceReadablePath: vi + .fn() + .mockImplementation( + (_cwd: string, _absolutePath: string, fallbackPath?: string) => fallbackPath ?? "test/path.txt", + ), + } +}) vi.mock("../../../utils/path", () => ({ getReadablePath: vi.fn().mockReturnValue("test/path.txt"), diff --git a/src/core/tools/__tests__/listFilesTool.spec.ts b/src/core/tools/__tests__/listFilesTool.spec.ts new file mode 100644 index 0000000000..723fb17968 --- /dev/null +++ b/src/core/tools/__tests__/listFilesTool.spec.ts @@ -0,0 +1,109 @@ +import type { MockedFunction } from "vitest" + +import { listFiles } from "../../../services/glob/list-files" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../../utils/pathUtils" +import type { ToolUse } from "../../../shared/tools" +import { listFilesTool } from "../ListFilesTool" + +vi.mock("../../../services/glob/list-files", () => ({ + listFiles: vi.fn(), +})) + +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + resolvePathInWorkspace: vi.fn(), + getWorkspaceReadablePath: vi.fn(), + isPathOutsideWorkspace: vi.fn(), + } +}) + +vi.mock("../../prompts/responses", () => ({ + formatResponse: { + formatFilesList: vi.fn(), + }, +})) + +describe("listFilesTool", () => { + const cwd = "/workspace/primary" + const relPath = "secondary/src" + const absolutePath = "/workspace/secondary/src" + + const mockedListFiles = listFiles as MockedFunction + const mockedResolvePathInWorkspace = resolvePathInWorkspace as MockedFunction + const mockedGetWorkspaceReadablePath = getWorkspaceReadablePath as MockedFunction + const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction + + let task: any + let askApproval: ReturnType + let pushToolResult: ReturnType + let handleError: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + + mockedResolvePathInWorkspace.mockResolvedValue(absolutePath) + mockedGetWorkspaceReadablePath.mockReturnValue("secondary/src") + mockedIsPathOutsideWorkspace.mockReturnValue(false) + mockedListFiles.mockResolvedValue([["a.ts", "b.ts"], false]) + + task = { + cwd, + consecutiveMistakeCount: 0, + didToolFailInCurrentTurn: false, + recordToolError: vi.fn(), + rooIgnoreController: {}, + rooProtectedController: {}, + providerRef: { + deref: vi.fn().mockReturnValue({ + getState: vi.fn().mockResolvedValue({ showRooIgnoredFiles: true }), + }), + }, + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing path"), + ask: vi.fn().mockResolvedValue(undefined), + } + + askApproval = vi.fn().mockResolvedValue(true) + pushToolResult = vi.fn() + handleError = vi.fn() + }) + + it("lists files from the resolved workspace path and surfaces the workspace-readable path", async () => { + const { formatResponse } = await import("../../prompts/responses") + vi.mocked(formatResponse.formatFilesList).mockReturnValue("formatted file list") + + await listFilesTool.execute({ path: relPath, recursive: true }, task, { + askApproval, + pushToolResult, + handleError, + }) + + expect(mockedResolvePathInWorkspace).toHaveBeenCalledWith(cwd, relPath) + expect(mockedListFiles).toHaveBeenCalledWith(absolutePath, true, 200) + expect(formatResponse.formatFilesList).toHaveBeenCalledWith( + absolutePath, + ["a.ts", "b.ts"], + false, + task.rooIgnoreController, + true, + task.rooProtectedController, + ) + expect(askApproval).toHaveBeenCalledWith("tool", expect.stringContaining('"path":"secondary/src"')) + expect(pushToolResult).toHaveBeenCalledWith("formatted file list") + }) + + it("uses the resolved workspace path in partial payloads", async () => { + const block: ToolUse<"list_files"> = { + type: "tool_use", + name: "list_files", + params: { path: relPath, recursive: "true" }, + partial: true, + } + + await listFilesTool.handlePartial(task, block) + + expect(task.ask).toHaveBeenCalledWith("tool", expect.stringContaining('"path":"secondary/src"'), true) + expect(task.ask).toHaveBeenCalledWith("tool", expect.stringContaining('"tool":"listFilesRecursive"'), true) + }) +}) diff --git a/src/core/tools/__tests__/searchFilesTool.spec.ts b/src/core/tools/__tests__/searchFilesTool.spec.ts new file mode 100644 index 0000000000..7173092ef1 --- /dev/null +++ b/src/core/tools/__tests__/searchFilesTool.spec.ts @@ -0,0 +1,86 @@ +import type { MockedFunction } from "vitest" + +import { regexSearchFiles } from "../../../services/ripgrep" +import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../../utils/pathUtils" +import type { ToolUse } from "../../../shared/tools" +import { searchFilesTool } from "../SearchFilesTool" + +vi.mock("../../../services/ripgrep", () => ({ + regexSearchFiles: vi.fn(), +})) + +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + resolvePathInWorkspace: vi.fn(), + getWorkspaceReadablePath: vi.fn(), + isPathOutsideWorkspace: vi.fn(), + } +}) + +describe("searchFilesTool", () => { + const cwd = "/workspace/primary" + const relPath = "secondary/src" + const absolutePath = "/workspace/secondary/src" + + const mockedRegexSearchFiles = regexSearchFiles as MockedFunction + const mockedResolvePathInWorkspace = resolvePathInWorkspace as MockedFunction + const mockedGetWorkspaceReadablePath = getWorkspaceReadablePath as MockedFunction + const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction + + let task: any + let askApproval: ReturnType + let pushToolResult: ReturnType + let handleError: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + + mockedResolvePathInWorkspace.mockResolvedValue(absolutePath) + mockedGetWorkspaceReadablePath.mockReturnValue("secondary/src") + mockedIsPathOutsideWorkspace.mockReturnValue(false) + mockedRegexSearchFiles.mockResolvedValue("match results") + + task = { + cwd, + consecutiveMistakeCount: 0, + didToolFailInCurrentTurn: false, + recordToolError: vi.fn(), + rooIgnoreController: {}, + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing param"), + ask: vi.fn().mockResolvedValue(undefined), + } + + askApproval = vi.fn().mockResolvedValue(true) + pushToolResult = vi.fn() + handleError = vi.fn() + }) + + it("searches from the resolved workspace path and surfaces the workspace-readable path", async () => { + await searchFilesTool.execute({ path: relPath, regex: "TODO", file_pattern: "*.ts" }, task, { + askApproval, + pushToolResult, + handleError, + }) + + expect(mockedResolvePathInWorkspace).toHaveBeenCalledWith(cwd, relPath) + expect(mockedRegexSearchFiles).toHaveBeenCalledWith(cwd, absolutePath, "TODO", "*.ts", task.rooIgnoreController) + expect(askApproval).toHaveBeenCalledWith("tool", expect.stringContaining('"path":"secondary/src"')) + expect(pushToolResult).toHaveBeenCalledWith("match results") + }) + + it("uses the resolved workspace path in partial payloads", async () => { + const block: ToolUse<"search_files"> = { + type: "tool_use", + name: "search_files", + params: { path: relPath, regex: "TODO", file_pattern: "*.ts" }, + partial: true, + } + + await searchFilesTool.handlePartial(task, block) + + expect(task.ask).toHaveBeenCalledWith("tool", expect.stringContaining('"path":"secondary/src"'), true) + expect(task.ask).toHaveBeenCalledWith("tool", expect.stringContaining('"tool":"searchFiles"'), true) + }) +}) diff --git a/src/core/tools/__tests__/searchReplaceTool.spec.ts b/src/core/tools/__tests__/searchReplaceTool.spec.ts index 1b1f78a128..69bf96e815 100644 --- a/src/core/tools/__tests__/searchReplaceTool.spec.ts +++ b/src/core/tools/__tests__/searchReplaceTool.spec.ts @@ -44,9 +44,21 @@ vi.mock("../../prompts/responses", () => ({ }, })) -vi.mock("../../../utils/pathUtils", () => ({ - isPathOutsideWorkspace: vi.fn().mockReturnValue(false), -})) +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + isPathOutsideWorkspace: vi.fn().mockReturnValue(false), + resolvePathInWorkspace: vi + .fn() + .mockImplementation(async (cwd: string, filePath: string) => path.resolve(cwd, filePath)), + getWorkspaceReadablePath: vi + .fn() + .mockImplementation( + (_cwd: string, _absolutePath: string, fallbackPath?: string) => fallbackPath ?? "test/path.txt", + ), + } +}) vi.mock("../../../utils/path", () => ({ getReadablePath: vi.fn().mockReturnValue("test/path.txt"), diff --git a/src/core/tools/__tests__/writeToFileTool.spec.ts b/src/core/tools/__tests__/writeToFileTool.spec.ts index 6c63387ee1..60ad2072a7 100644 --- a/src/core/tools/__tests__/writeToFileTool.spec.ts +++ b/src/core/tools/__tests__/writeToFileTool.spec.ts @@ -3,7 +3,7 @@ import * as path from "path" import type { MockedFunction } from "vitest" import { fileExistsAtPath, createDirectoriesForFile } from "../../../utils/fs" -import { isPathOutsideWorkspace } from "../../../utils/pathUtils" +import { isPathOutsideWorkspace, resolvePathInWorkspace } from "../../../utils/pathUtils" import { getReadablePath } from "../../../utils/path" import { unescapeHtmlEntities } from "../../../utils/text-normalization" import { everyLineHasLineNumbers, stripLineNumbers } from "../../../integrations/misc/extract-text" @@ -39,9 +39,21 @@ vi.mock("../../prompts/responses", () => ({ }, })) -vi.mock("../../../utils/pathUtils", () => ({ - isPathOutsideWorkspace: vi.fn().mockReturnValue(false), -})) +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + isPathOutsideWorkspace: vi.fn().mockReturnValue(false), + resolvePathInWorkspace: vi + .fn() + .mockImplementation(async (cwd: string, filePath: string) => path.resolve(cwd, filePath)), + getWorkspaceReadablePath: vi + .fn() + .mockImplementation( + (_cwd: string, _absolutePath: string, fallbackPath?: string) => fallbackPath ?? "test/path.txt", + ), + } +}) vi.mock("../../../utils/path", () => ({ getReadablePath: vi.fn().mockReturnValue("test/path.txt"), @@ -89,6 +101,8 @@ describe("writeToFileTool", () => { // Test data const testFilePath = "test/file.txt" const absoluteFilePath = process.platform === "win32" ? "C:\\test\\file.txt" : "/test/file.txt" + const outsideRootAbsolutePath = + process.platform === "win32" ? "C:\\secondary\\test\\file.txt" : "/secondary/test/file.txt" const testContent = "Line 1\nLine 2\nLine 3" const testContentWithMarkdown = "```javascript\nLine 1\nLine 2\n```" @@ -96,6 +110,7 @@ describe("writeToFileTool", () => { const mockedFileExistsAtPath = fileExistsAtPath as MockedFunction const mockedCreateDirectoriesForFile = createDirectoriesForFile as MockedFunction const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction + const mockedResolvePathInWorkspace = resolvePathInWorkspace as MockedFunction const mockedGetReadablePath = getReadablePath as MockedFunction const mockedUnescapeHtmlEntities = unescapeHtmlEntities as MockedFunction const mockedEveryLineHasLineNumbers = everyLineHasLineNumbers as MockedFunction @@ -108,11 +123,16 @@ describe("writeToFileTool", () => { let mockPushToolResult: ReturnType let toolResult: ToolResponse | undefined + const expectedInsideWorkspaceDiffPath = process.platform === "win32" ? absoluteFilePath : testFilePath + beforeEach(() => { vi.clearAllMocks() writeToFileTool.resetPartialState() mockedPathResolve.mockReturnValue(absoluteFilePath) + mockedResolvePathInWorkspace.mockImplementation(async (_cwd: string, filePath: string) => + path.resolve("/", filePath), + ) mockedFileExistsAtPath.mockResolvedValue(false) mockedIsPathOutsideWorkspace.mockReturnValue(false) mockedGetReadablePath.mockReturnValue("test/path.txt") @@ -239,8 +259,17 @@ describe("writeToFileTool", () => { it("validates and allows access when rooIgnoreController permits", async () => { await executeWriteFileTool({}, { accessAllowed: true }) - expect(mockCline.rooIgnoreController.validateAccess).toHaveBeenCalledWith(testFilePath) - expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath) + expect(mockCline.rooIgnoreController.validateAccess).toHaveBeenCalledWith(absoluteFilePath) + expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(expectedInsideWorkspaceDiffPath) + }) + + it("opens the absolute diff path when the resolver lands outside task.cwd", async () => { + mockedResolvePathInWorkspace.mockResolvedValue(outsideRootAbsolutePath) + + await executeWriteFileTool({}, { accessAllowed: true }) + + expect(mockCline.rooIgnoreController.validateAccess).toHaveBeenCalledWith(outsideRootAbsolutePath) + expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(outsideRootAbsolutePath) }) }) @@ -361,7 +390,7 @@ describe("writeToFileTool", () => { await executeWriteFileTool({}, { fileExists: false }) expect(mockCline.consecutiveMistakeCount).toBe(0) - expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath) + expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(expectedInsideWorkspaceDiffPath) expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(testContent, true) expect(mockAskApproval).toHaveBeenCalled() expect(mockCline.diffViewProvider.saveChanges).toHaveBeenCalled() @@ -408,7 +437,7 @@ describe("writeToFileTool", () => { // Second call with same path - path is now stabilized, file operations proceed await executeWriteFileTool({}, { isPartial: true }) expect(mockCline.ask).toHaveBeenCalled() - expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath) + expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(expectedInsideWorkspaceDiffPath) expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(testContent, false) }) }) diff --git a/src/core/webview/__tests__/webviewMessageHandler.readFileContent.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.readFileContent.spec.ts index 00230c077a..27260b2c1c 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.readFileContent.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.readFileContent.spec.ts @@ -40,17 +40,30 @@ vi.mock("../../../utils/fs") vi.mock("../../../utils/path") vi.mock("../../../utils/globalContext") -vi.mock("../../../utils/pathUtils", () => ({ - isPathOutsideWorkspace: vi.fn((filePath: string) => { - const nodePath = require("path") - const normalized = nodePath.resolve(filePath) - const workspaceRoot = nodePath.resolve("/mock/workspace") - // Path is inside workspace if it equals or is under workspace root - if (normalized === workspaceRoot) return false - if (normalized.startsWith(workspaceRoot + nodePath.sep)) return false - return true - }), -})) +vi.mock("../../../utils/pathUtils", async () => { + const actual = await vi.importActual("../../../utils/pathUtils") + return { + ...actual, + isPathOutsideWorkspace: vi.fn((filePath: string) => { + const nodePath = require("path") + const normalized = nodePath.resolve(filePath) + const workspaceRoot = nodePath.resolve("/mock/workspace") + // Path is inside workspace if it equals or is under workspace root + if (normalized === workspaceRoot) return false + if (normalized.startsWith(workspaceRoot + nodePath.sep)) return false + return true + }), + resolvePathInWorkspace: vi.fn().mockImplementation(async (cwd: string, filePath: string) => { + const nodePath = require("path") + return nodePath.resolve(cwd, filePath) + }), + getWorkspaceReadablePath: vi + .fn() + .mockImplementation( + (_cwd: string, _absolutePath: string, fallbackPath?: string) => fallbackPath ?? "mock/workspace", + ), + } +}) vi.mock("../../mentions/resolveImageMentions", () => ({ resolveImageMentions: vi.fn(async ({ text, images }: { text: string; images?: string[] }) => ({ diff --git a/src/utils/__tests__/pathUtils.spec.ts b/src/utils/__tests__/pathUtils.spec.ts new file mode 100644 index 0000000000..b2a807e635 --- /dev/null +++ b/src/utils/__tests__/pathUtils.spec.ts @@ -0,0 +1,219 @@ +import * as path from "path" +import * as os from "os" +import * as fs from "fs/promises" + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" + +const mockWorkspace = vi.hoisted(() => ({ + workspaceFolders: [] as Array<{ uri: { fsPath: string }; name: string; index: number }>, +})) + +vi.mock("vscode", () => ({ + workspace: mockWorkspace, +})) + +import { + getWorkspaceReadablePath, + getWorkspaceRelativePath, + getWorkspaceRootForPath, + resolvePathInWorkspace, +} from "../pathUtils" + +describe("pathUtils", () => { + let tempDir: string + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "roo-path-utils-")) + }) + + afterEach(async () => { + mockWorkspace.workspaceFolders = [] + await fs.rm(tempDir, { recursive: true, force: true }) + }) + + it("resolves a relative path from a secondary workspace root when it does not exist in cwd", async () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + await fs.mkdir(primaryRoot, { recursive: true }) + await fs.mkdir(secondaryRoot, { recursive: true }) + await fs.writeFile(path.join(secondaryRoot, "only-secondary.txt"), "secondary", "utf8") + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + await expect(resolvePathInWorkspace(primaryRoot, "only-secondary.txt")).resolves.toBe( + path.join(secondaryRoot, "only-secondary.txt"), + ) + }) + + it("returns absolute paths unchanged", async () => { + const absolutePath = path.join(tempDir, "absolute.txt") + await expect(resolvePathInWorkspace(path.join(tempDir, "cwd"), absolutePath)).resolves.toBe(absolutePath) + }) + + it("prefers the cwd path when the file already exists there", async () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + const fileName = "shared.txt" + + await fs.mkdir(primaryRoot, { recursive: true }) + await fs.mkdir(secondaryRoot, { recursive: true }) + await fs.writeFile(path.join(primaryRoot, fileName), "primary", "utf8") + await fs.writeFile(path.join(secondaryRoot, fileName), "secondary", "utf8") + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + await expect(resolvePathInWorkspace(primaryRoot, fileName)).resolves.toBe(path.join(primaryRoot, fileName)) + }) + + it("supports workspace-folder-name-prefixed paths before the file exists", async () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + await fs.mkdir(primaryRoot, { recursive: true }) + await fs.mkdir(secondaryRoot, { recursive: true }) + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + await expect(resolvePathInWorkspace(primaryRoot, path.join("secondary", "new-file.txt"))).resolves.toBe( + path.join(secondaryRoot, "new-file.txt"), + ) + }) + + it("resolves a new file into the only workspace root that already contains the parent directory", async () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + await fs.mkdir(path.join(primaryRoot, "src"), { recursive: true }) + await fs.mkdir(path.join(secondaryRoot, "nested", "dir"), { recursive: true }) + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + await expect(resolvePathInWorkspace(primaryRoot, path.join("nested", "dir", "new-file.txt"))).resolves.toBe( + path.join(secondaryRoot, "nested", "dir", "new-file.txt"), + ) + }) + + it("does not allow workspace-folder-name prefixes to escape the selected root", async () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + await fs.mkdir(primaryRoot, { recursive: true }) + await fs.mkdir(secondaryRoot, { recursive: true }) + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + await expect(resolvePathInWorkspace(primaryRoot, path.join("secondary", "..", "escaped.txt"))).resolves.toBe( + path.join(primaryRoot, "escaped.txt"), + ) + }) + + it("prefers the cwd-owned parent match when multiple roots contain the parent directory", async () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + const filePath = path.join("nested", "new-file.txt") + + await fs.mkdir(path.join(primaryRoot, "nested"), { recursive: true }) + await fs.mkdir(path.join(secondaryRoot, "nested"), { recursive: true }) + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + await expect(resolvePathInWorkspace(primaryRoot, filePath)).resolves.toBe(path.join(primaryRoot, filePath)) + }) + + it("falls back to the primary cwd path when parent matches are ambiguous", async () => { + const firstRoot = path.join(tempDir, "first") + const secondRoot = path.join(tempDir, "second") + const externalCwd = path.join(tempDir, "external") + const filePath = path.join("nested", "new-file.txt") + + await fs.mkdir(path.join(firstRoot, "nested"), { recursive: true }) + await fs.mkdir(path.join(secondRoot, "nested"), { recursive: true }) + await fs.mkdir(externalCwd, { recursive: true }) + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: firstRoot }, name: "first", index: 0 }, + { uri: { fsPath: secondRoot }, name: "second", index: 1 }, + ] + + await expect(resolvePathInWorkspace(externalCwd, filePath)).resolves.toBe(path.join(externalCwd, filePath)) + }) + + it("returns the matching workspace root or fallback cwd for an absolute path", () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + const fallbackRoot = path.join(tempDir, "fallback") + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + expect(getWorkspaceRootForPath(path.join(secondaryRoot, "nested", "file.txt"), fallbackRoot)).toBe( + secondaryRoot, + ) + expect(getWorkspaceRootForPath(path.join(fallbackRoot, "nested", "file.txt"), fallbackRoot)).toBe(fallbackRoot) + expect(getWorkspaceRootForPath(path.join(tempDir, "outside", "file.txt"), fallbackRoot)).toBeUndefined() + }) + + it("returns workspace-relative paths for absolute files", () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + expect(getWorkspaceRelativePath(primaryRoot, path.join(secondaryRoot, "nested", "file.txt"))).toBe( + "nested/file.txt", + ) + }) + + it("returns workspace-folder-qualified display paths in multi-root workspaces", () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + expect(getWorkspaceReadablePath(primaryRoot, path.join(secondaryRoot, "nested", "file.txt"), "file.txt")).toBe( + "secondary/nested/file.txt", + ) + }) + + it("returns the root name when the absolute path points at a workspace root", () => { + const primaryRoot = path.join(tempDir, "primary") + const secondaryRoot = path.join(tempDir, "secondary") + + mockWorkspace.workspaceFolders = [ + { uri: { fsPath: primaryRoot }, name: "primary", index: 0 }, + { uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 }, + ] + + expect(getWorkspaceReadablePath(primaryRoot, secondaryRoot, "secondary")).toBe("secondary") + }) + + it("falls back to cwd-readable paths when the file is outside known workspace roots", () => { + const cwd = path.join(tempDir, "cwd") + const outsidePath = path.join(tempDir, "outside", "file.txt") + + expect(getWorkspaceReadablePath(cwd, outsidePath, "outside/file.txt")).toBe("outside/file.txt") + }) +}) diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts index dae300f8f3..6afe1cd629 100644 --- a/src/utils/pathUtils.ts +++ b/src/utils/pathUtils.ts @@ -1,5 +1,8 @@ import * as vscode from "vscode" import * as path from "path" +import * as fs from "fs/promises" + +import { getReadablePath } from "./path" /** * Checks if a file path is outside all workspace folders @@ -17,8 +20,166 @@ export function isPathOutsideWorkspace(filePath: string): boolean { // Check if the path is within any workspace folder return !vscode.workspace.workspaceFolders.some((folder) => { - const folderPath = folder.uri.fsPath - // Path is inside a workspace if it equals the workspace path or is a subfolder - return absolutePath === folderPath || absolutePath.startsWith(folderPath + path.sep) + return isPathInside(absolutePath, folder.uri.fsPath) }) } + +function isPathInside(filePath: string, rootPath: string): boolean { + const normalizedPath = normalizePathForComparison(filePath) + const normalizedRoot = normalizePathForComparison(rootPath) + const relativePath = path.relative(normalizedRoot, normalizedPath) + return relativePath === "" || (!!relativePath && !relativePath.startsWith("..") && !path.isAbsolute(relativePath)) +} + +function normalizePathForComparison(filePath: string): string { + const resolvedPath = path.resolve(filePath) + return process.platform === "win32" ? resolvedPath.toLowerCase() : resolvedPath +} + +async function pathExists(filePath: string): Promise { + try { + await fs.stat(filePath) + return true + } catch { + return false + } +} + +async function pathHasExistingAncestorBelowRoot(filePath: string, rootPath: string): Promise { + let currentPath = path.dirname(path.resolve(filePath)) + const resolvedRoot = path.resolve(rootPath) + + while (isPathInside(currentPath, resolvedRoot)) { + if (currentPath === resolvedRoot) { + return false + } + + if (await pathExists(currentPath)) { + return true + } + + const parentPath = path.dirname(currentPath) + if (parentPath === currentPath) { + return false + } + currentPath = parentPath + } + + return false +} + +function pathSegments(filePath: string): string[] { + return filePath.split(/[\\/]+/).filter(Boolean) +} + +function namesEqual(a: string, b: string): boolean { + return process.platform === "win32" ? a.toLowerCase() === b.toLowerCase() : a === b +} + +/** + * Resolves a tool path against the current cwd and all VS Code workspace roots. + * + * In multi-root workspaces, task.cwd is only one root. If a relative path does + * not exist there, try the other workspace folders before surfacing ENOENT. + */ +export async function resolvePathInWorkspace(cwd: string, filePath: string): Promise { + if (path.isAbsolute(filePath)) { + return path.resolve(filePath) + } + + const primaryPath = path.resolve(cwd, filePath) + if (await pathExists(primaryPath)) { + return primaryPath + } + + const workspaceFolders = vscode.workspace.workspaceFolders ?? [] + const segments = pathSegments(filePath) + const firstSegment = segments[0] + + if (firstSegment) { + for (const folder of workspaceFolders) { + const folderPath = folder.uri.fsPath + const folderNames = [folder.name, path.basename(folderPath)] + if (folderNames.some((name) => namesEqual(name, firstSegment))) { + const candidate = path.resolve(folderPath, ...segments.slice(1)) + if (isPathInside(candidate, folderPath)) { + return candidate + } + } + } + } + + for (const folder of workspaceFolders) { + const candidate = path.resolve(folder.uri.fsPath, filePath) + if (await pathExists(candidate)) { + return candidate + } + } + + const parentMatches: Array<{ candidate: string; rootPath: string }> = [] + + for (const folder of workspaceFolders) { + const rootPath = folder.uri.fsPath + const candidate = path.resolve(rootPath, filePath) + if (await pathHasExistingAncestorBelowRoot(candidate, rootPath)) { + parentMatches.push({ candidate, rootPath }) + } + } + + if (parentMatches.length === 1) { + return parentMatches[0].candidate + } + + if (parentMatches.length > 1) { + const cwdMatches = parentMatches.filter(({ rootPath }) => isPathInside(cwd, rootPath)) + if (cwdMatches.length === 1) { + return cwdMatches[0].candidate + } + } + + return primaryPath +} + +export function getWorkspaceRootForPath(filePath: string, fallbackCwd?: string): string | undefined { + const resolvedPath = path.resolve(filePath) + const workspaceRoot = vscode.workspace.workspaceFolders?.find((folder) => + isPathInside(resolvedPath, folder.uri.fsPath), + )?.uri.fsPath + + if (workspaceRoot) { + return workspaceRoot + } + + if (fallbackCwd && isPathInside(resolvedPath, fallbackCwd)) { + return fallbackCwd + } + + return undefined +} + +export function getWorkspaceRelativePath(cwd: string, filePath: string): string { + const absolutePath = path.isAbsolute(filePath) ? path.resolve(filePath) : path.resolve(cwd, filePath) + const rootPath = getWorkspaceRootForPath(absolutePath, cwd) ?? cwd + return path.relative(rootPath, absolutePath).toPosix() +} + +export function getWorkspaceReadablePath(cwd: string, absolutePath: string, fallbackPath?: string): string { + const workspaceFolders = vscode.workspace.workspaceFolders ?? [] + const resolvedPath = path.resolve(absolutePath) + + for (const folder of workspaceFolders) { + const folderPath = folder.uri.fsPath + if (!isPathInside(resolvedPath, folderPath)) { + continue + } + + const relativePath = path.relative(folderPath, resolvedPath).toPosix() + if (workspaceFolders.length > 1) { + return relativePath ? `${folder.name}/${relativePath}`.toPosix() : folder.name.toPosix() + } + + return relativePath || path.basename(folderPath).toPosix() + } + + return getReadablePath(cwd, fallbackPath ?? resolvedPath) +}