Skip to content

Commit 5b6b41e

Browse files
committed
fix: support multi-root workspace tool paths
1 parent b6a23cf commit 5b6b41e

22 files changed

Lines changed: 730 additions & 179 deletions

src/core/context-tracking/FileContextTracker.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import fs from "fs/promises"
88
import { ContextProxy } from "../config/ContextProxy"
99
import type { FileMetadataEntry, RecordSource, TaskMetadata } from "./FileContextTrackerTypes"
1010
import { ClineProvider } from "../webview/ClineProvider"
11+
import { getWorkspaceReadablePath, resolvePathInWorkspace } from "../../utils/pathUtils"
1112

1213
// This class is responsible for tracking file operations that may result in stale context.
1314
// 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 {
4546
}
4647

4748
// File watchers are set up for each file that is tracked in the task metadata.
48-
async setupFileWatcher(filePath: string) {
49+
async setupFileWatcher(filePath: string, absolutePath?: string) {
4950
// Only setup watcher if it doesn't already exist for this file
5051
if (this.fileWatchers.has(filePath)) {
5152
return
@@ -57,7 +58,7 @@ export class FileContextTracker {
5758
}
5859

5960
// Create a file system watcher for this specific file
60-
const fileUri = vscode.Uri.file(path.resolve(cwd, filePath))
61+
const fileUri = vscode.Uri.file(absolutePath ?? (await resolvePathInWorkspace(cwd, filePath)))
6162
const watcher = vscode.workspace.createFileSystemWatcher(
6263
new vscode.RelativePattern(path.dirname(fileUri.fsPath), path.basename(fileUri.fsPath)),
6364
)
@@ -85,10 +86,13 @@ export class FileContextTracker {
8586
return
8687
}
8788

88-
await this.addFileToFileContextTracker(this.taskId, filePath, operation)
89+
const absolutePath = await resolvePathInWorkspace(cwd, filePath)
90+
const trackedPath = getWorkspaceReadablePath(cwd, absolutePath, filePath)
91+
92+
await this.addFileToFileContextTracker(this.taskId, trackedPath, operation)
8993

9094
// Set up file watcher for this file
91-
await this.setupFileWatcher(filePath)
95+
await this.setupFileWatcher(trackedPath, absolutePath)
9296
} catch (error) {
9397
console.error("Failed to track file operation:", error)
9498
}

src/core/ignore/RooIgnoreController.ts

Lines changed: 100 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import fsSync from "fs"
55
import ignore, { Ignore } from "ignore"
66
import * as vscode from "vscode"
77

8+
import { getWorkspaceRelativePath, getWorkspaceRootForPath } from "../../utils/pathUtils"
9+
810
export const LOCK_TEXT_SYMBOL = "\u{1F512}"
911

1012
/**
@@ -15,6 +17,8 @@ export const LOCK_TEXT_SYMBOL = "\u{1F512}"
1517
export class RooIgnoreController {
1618
private cwd: string
1719
private ignoreInstance: Ignore
20+
private ignoreInstances = new Map<string, Ignore>()
21+
private rooIgnoreContents = new Map<string, string | undefined>()
1822
private disposables: vscode.Disposable[] = []
1923
rooIgnoreContent: string | undefined
2024

@@ -38,19 +42,23 @@ export class RooIgnoreController {
3842
* Set up the file watcher for .rooignore changes
3943
*/
4044
private setupFileWatcher(): void {
41-
const rooignorePattern = new vscode.RelativePattern(this.cwd, ".rooignore")
45+
this.setupFileWatcherForRoot(this.cwd)
46+
}
47+
48+
private setupFileWatcherForRoot(rootPath: string): void {
49+
const rooignorePattern = new vscode.RelativePattern(rootPath, ".rooignore")
4250
const fileWatcher = vscode.workspace.createFileSystemWatcher(rooignorePattern)
4351

4452
// Watch for changes and updates
4553
this.disposables.push(
4654
fileWatcher.onDidChange(() => {
47-
this.loadRooIgnore()
55+
this.loadRooIgnoreForRoot(rootPath)
4856
}),
4957
fileWatcher.onDidCreate(() => {
50-
this.loadRooIgnore()
58+
this.loadRooIgnoreForRoot(rootPath)
5159
}),
5260
fileWatcher.onDidDelete(() => {
53-
this.loadRooIgnore()
61+
this.loadRooIgnoreForRoot(rootPath)
5462
}),
5563
)
5664

@@ -62,38 +70,99 @@ export class RooIgnoreController {
6270
* Load custom patterns from .rooignore if it exists
6371
*/
6472
private async loadRooIgnore(): Promise<void> {
73+
await this.loadRooIgnoreForRoot(this.cwd)
74+
}
75+
76+
private async loadRooIgnoreForRoot(rootPath: string): Promise<void> {
6577
try {
6678
// Reset ignore instance to prevent duplicate patterns
67-
this.ignoreInstance = ignore()
68-
const ignorePath = path.join(this.cwd, ".rooignore")
79+
const ignoreInstance = ignore()
80+
const ignorePath = path.join(rootPath, ".rooignore")
6981
if (await fileExistsAtPath(ignorePath)) {
7082
const content = await fs.readFile(ignorePath, "utf8")
71-
this.rooIgnoreContent = content
72-
this.ignoreInstance.add(content)
73-
this.ignoreInstance.add(".rooignore")
83+
ignoreInstance.add(content)
84+
ignoreInstance.add(".rooignore")
85+
this.ignoreInstances.set(rootPath, ignoreInstance)
86+
this.rooIgnoreContents.set(rootPath, content)
7487
} else {
75-
this.rooIgnoreContent = undefined
88+
this.ignoreInstances.set(rootPath, ignoreInstance)
89+
this.rooIgnoreContents.set(rootPath, undefined)
90+
}
91+
92+
if (rootPath === this.cwd) {
93+
this.ignoreInstance = ignoreInstance
94+
this.rooIgnoreContent = this.rooIgnoreContents.get(rootPath)
7695
}
7796
} catch (error) {
7897
// Should never happen: reading file failed even though it exists
7998
console.error("Unexpected error loading .rooignore:", error)
8099
}
81100
}
82101

102+
private getIgnoreStateForRoot(rootPath: string): { ignoreInstance: Ignore; content: string | undefined } {
103+
const cached = this.ignoreInstances.get(rootPath)
104+
if (cached) {
105+
return { ignoreInstance: cached, content: this.rooIgnoreContents.get(rootPath) }
106+
}
107+
108+
const ignoreInstance = ignore()
109+
try {
110+
const ignorePath = path.join(rootPath, ".rooignore")
111+
if (fsSync.existsSync(ignorePath)) {
112+
const content = fsSync.readFileSync(ignorePath, "utf8")
113+
ignoreInstance.add(content)
114+
ignoreInstance.add(".rooignore")
115+
this.ignoreInstances.set(rootPath, ignoreInstance)
116+
this.rooIgnoreContents.set(rootPath, content)
117+
this.setupFileWatcherForRoot(rootPath)
118+
return { ignoreInstance, content }
119+
}
120+
} catch (error) {
121+
console.error("Unexpected error loading .rooignore:", error)
122+
}
123+
124+
this.ignoreInstances.set(rootPath, ignoreInstance)
125+
this.rooIgnoreContents.set(rootPath, undefined)
126+
this.setupFileWatcherForRoot(rootPath)
127+
return { ignoreInstance, content: undefined }
128+
}
129+
130+
private getKnownWorkspaceRoots(): string[] {
131+
const roots = new Set<string>([this.cwd])
132+
for (const folder of vscode.workspace.workspaceFolders ?? []) {
133+
roots.add(folder.uri.fsPath)
134+
}
135+
return [...roots]
136+
}
137+
138+
private getAvailableIgnoreContents(): Array<{ rootPath: string; content: string }> {
139+
return this.getKnownWorkspaceRoots()
140+
.map((rootPath) => ({ rootPath, content: this.getIgnoreStateForRoot(rootPath).content }))
141+
.filter((entry): entry is { rootPath: string; content: string } => typeof entry.content === "string")
142+
}
143+
83144
/**
84145
* Check if a file should be accessible to the LLM
85146
* Automatically resolves symlinks
86147
* @param filePath - Path to check (relative to cwd)
87148
* @returns true if file is accessible, false if ignored
88149
*/
89150
validateAccess(filePath: string): boolean {
151+
const absolutePath = path.isAbsolute(filePath) ? path.resolve(filePath) : path.resolve(this.cwd, filePath)
152+
const rootPath = getWorkspaceRootForPath(absolutePath, this.cwd)
153+
154+
// Preserve backward compatibility for files outside the task workspace roots.
155+
if (!rootPath) {
156+
return true
157+
}
158+
159+
const { ignoreInstance, content } = this.getIgnoreStateForRoot(rootPath)
90160
// Always allow access if .rooignore does not exist
91-
if (!this.rooIgnoreContent) {
161+
if (!content) {
92162
return true
93163
}
94-
try {
95-
const absolutePath = path.resolve(this.cwd, filePath)
96164

165+
try {
97166
// Follow symlinks to get the real path
98167
let realPath: string
99168
try {
@@ -105,10 +174,10 @@ export class RooIgnoreController {
105174
}
106175

107176
// Convert real path to relative for .rooignore checking
108-
const relativePath = path.relative(this.cwd, realPath).toPosix()
177+
const relativePath = getWorkspaceRelativePath(rootPath, realPath)
109178

110179
// Check if the real path is ignored
111-
return !this.ignoreInstance.ignores(relativePath)
180+
return !ignoreInstance.ignores(relativePath)
112181
} catch (error) {
113182
// Allow access to files outside cwd or on errors (backward compatibility)
114183
return true
@@ -121,11 +190,6 @@ export class RooIgnoreController {
121190
* @returns path of file that is being accessed if it is being accessed, undefined if command is allowed
122191
*/
123192
validateCommand(command: string): string | undefined {
124-
// Always allow if no .rooignore exists
125-
if (!this.rooIgnoreContent) {
126-
return undefined
127-
}
128-
129193
// Split command into parts and get the base command
130194
const parts = command.trim().split(/\s+/)
131195
const baseCommand = parts[0].toLowerCase()
@@ -153,12 +217,13 @@ export class RooIgnoreController {
153217
// Check each argument that could be a file path
154218
for (let i = 1; i < parts.length; i++) {
155219
const arg = parts[i]
220+
const isWindowsAbsolutePath = path.win32.isAbsolute(arg)
156221
// Skip command flags/options (both Unix and PowerShell style)
157-
if (arg.startsWith("-") || arg.startsWith("/")) {
222+
if (arg.startsWith("-") || (arg.startsWith("/") && !path.isAbsolute(arg))) {
158223
continue
159224
}
160225
// Ignore PowerShell parameter names
161-
if (arg.includes(":")) {
226+
if (arg.includes(":") && !isWindowsAbsolutePath) {
162227
continue
163228
}
164229
// Validate file access
@@ -204,10 +269,21 @@ export class RooIgnoreController {
204269
* @returns Formatted instructions or undefined if .rooignore doesn't exist
205270
*/
206271
getInstructions(): string | undefined {
207-
if (!this.rooIgnoreContent) {
272+
const ignoreEntries = this.getAvailableIgnoreContents()
273+
if (ignoreEntries.length === 0) {
208274
return undefined
209275
}
210276

211-
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`
277+
const sections = ignoreEntries
278+
.map(({ rootPath, content }) => {
279+
const workspaceName =
280+
vscode.workspace.workspaceFolders?.find((folder) => folder.uri.fsPath === rootPath)?.name ??
281+
path.basename(rootPath) ??
282+
rootPath
283+
return `## ${workspaceName}\n\n${content}\n.rooignore`
284+
})
285+
.join("\n\n")
286+
287+
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}`
212288
}
213289
}

src/core/ignore/__tests__/RooIgnoreController.spec.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ vi.mock("vscode", () => {
2424

2525
return {
2626
workspace: {
27+
workspaceFolders: undefined,
2728
createFileSystemWatcher: vi.fn(() => ({
2829
onDidCreate: vi.fn(() => mockDisposable),
2930
onDidChange: vi.fn(() => mockDisposable),
@@ -63,6 +64,7 @@ describe("RooIgnoreController", () => {
6364

6465
// @ts-expect-error - Mocking
6566
vscode.workspace.createFileSystemWatcher.mockReturnValue(mockWatcher)
67+
;(vscode.workspace as any).workspaceFolders = undefined
6668

6769
// Setup fs mocks
6870
mockFileExists = fileExistsAtPath as Mock<typeof fileExistsAtPath>
@@ -198,6 +200,27 @@ describe("RooIgnoreController", () => {
198200
expect(controller.validateAccess(allowedAbsolutePath)).toBe(true)
199201
})
200202

203+
it("should apply the .rooignore from a secondary workspace root for absolute paths", () => {
204+
const secondaryRoot = "/test/secondary"
205+
;(vscode.workspace as any).workspaceFolders = [
206+
{ uri: { fsPath: TEST_CWD }, name: "primary", index: 0 },
207+
{ uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 },
208+
]
209+
210+
vi.mocked(fsSync.existsSync).mockImplementation(
211+
(filePath) => filePath === path.join(secondaryRoot, ".rooignore"),
212+
)
213+
vi.mocked(fsSync.readFileSync).mockImplementation((filePath) => {
214+
if (filePath === path.join(secondaryRoot, ".rooignore")) {
215+
return "private/**"
216+
}
217+
return ""
218+
})
219+
220+
expect(controller.validateAccess(path.join(secondaryRoot, "private/secret.txt"))).toBe(false)
221+
expect(controller.validateAccess(path.join(secondaryRoot, "src/app.ts"))).toBe(true)
222+
})
223+
201224
/**
202225
* Tests handling of paths outside cwd
203226
*/
@@ -315,6 +338,42 @@ describe("RooIgnoreController", () => {
315338
expect(emptyController.validateCommand("cat node_modules/package.json")).toBeUndefined()
316339
expect(emptyController.validateCommand("grep pattern .git/config")).toBeUndefined()
317340
})
341+
342+
it("should enforce a secondary workspace .rooignore for execute_command paths", async () => {
343+
const secondaryRoot = "/test/secondary"
344+
;(vscode.workspace as any).workspaceFolders = [
345+
{ uri: { fsPath: TEST_CWD }, name: "primary", index: 0 },
346+
{ uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 },
347+
]
348+
349+
mockFileExists.mockImplementation(async (filePath) => filePath === path.join(TEST_CWD, ".rooignore"))
350+
mockReadFile.mockResolvedValue("node_modules\n.git\nsecrets/**\n*.log")
351+
await controller.initialize()
352+
353+
vi.mocked(fsSync.existsSync).mockImplementation(
354+
(filePath) => filePath === path.join(secondaryRoot, ".rooignore"),
355+
)
356+
vi.mocked(fsSync.readFileSync).mockImplementation((filePath) => {
357+
if (filePath === path.join(secondaryRoot, ".rooignore")) {
358+
return "private/**"
359+
}
360+
return ""
361+
})
362+
363+
expect(controller.validateCommand(`cat ${path.join(secondaryRoot, "private", "secret.txt")}`)).toBe(
364+
path.join(secondaryRoot, "private", "secret.txt"),
365+
)
366+
})
367+
368+
it("should validate Windows absolute paths instead of treating them as PowerShell parameters", () => {
369+
const windowsPath = String.raw`C:\secondary\private\secret.txt`
370+
const validateAccessSpy = vi.spyOn(controller, "validateAccess").mockImplementation((candidate) => {
371+
return candidate !== windowsPath
372+
})
373+
374+
expect(controller.validateCommand(`type ${windowsPath}`)).toBe(windowsPath)
375+
expect(validateAccessSpy).toHaveBeenCalledWith(windowsPath)
376+
})
318377
})
319378

320379
describe("filterPaths", () => {
@@ -411,6 +470,40 @@ describe("RooIgnoreController", () => {
411470
const instructions = controller.getInstructions()
412471
expect(instructions).toBeUndefined()
413472
})
473+
474+
it("should include secondary workspace .rooignore content in instructions", async () => {
475+
const secondaryRoot = "/test/secondary"
476+
;(vscode.workspace as any).workspaceFolders = [
477+
{ uri: { fsPath: TEST_CWD }, name: "primary", index: 0 },
478+
{ uri: { fsPath: secondaryRoot }, name: "secondary", index: 1 },
479+
]
480+
481+
mockFileExists.mockImplementation(async (filePath) => filePath === path.join(TEST_CWD, ".rooignore"))
482+
mockReadFile.mockResolvedValue("node_modules")
483+
await controller.initialize()
484+
485+
vi.mocked(fsSync.existsSync).mockImplementation(
486+
(filePath) =>
487+
filePath === path.join(secondaryRoot, ".rooignore") ||
488+
filePath === path.join(TEST_CWD, ".rooignore"),
489+
)
490+
vi.mocked(fsSync.readFileSync).mockImplementation((filePath) => {
491+
if (filePath === path.join(secondaryRoot, ".rooignore")) {
492+
return "private/**"
493+
}
494+
if (filePath === path.join(TEST_CWD, ".rooignore")) {
495+
return "node_modules"
496+
}
497+
return ""
498+
})
499+
500+
const instructions = controller.getInstructions()
501+
502+
expect(instructions).toContain("## primary")
503+
expect(instructions).toContain("## secondary")
504+
expect(instructions).toContain("node_modules")
505+
expect(instructions).toContain("private/**")
506+
})
414507
})
415508

416509
describe("dispose", () => {

0 commit comments

Comments
 (0)