Skip to content

Commit a6ce187

Browse files
roomoteroomote
committed
test: fix windows path expectations and cover multi-root tool paths
1 parent 550f2c3 commit a6ce187

4 files changed

Lines changed: 213 additions & 6 deletions

File tree

‎src/core/tools/__tests__/applyPatchTool.execute.spec.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ vi.mock("../apply-patch", () => ({
6464
}))
6565

6666
describe("ApplyPatchTool.execute", () => {
67-
const cwd = path.join(path.sep, "workspace", "primary")
68-
const secondaryRoot = path.join(path.sep, "workspace", "secondary")
67+
const cwd = process.platform === "win32" ? "C:\\workspace\\primary" : "/workspace/primary"
68+
const secondaryRoot = process.platform === "win32" ? "C:\\workspace\\secondary" : "/workspace/secondary"
6969

7070
const mockedReadFile = fs.readFile as MockedFunction<typeof fs.readFile>
7171
const mockedWriteFile = fs.writeFile as MockedFunction<typeof fs.writeFile>
@@ -84,6 +84,9 @@ describe("ApplyPatchTool.execute", () => {
8484
let askApproval: ReturnType<typeof vi.fn>
8585
let pushToolResult: ReturnType<typeof vi.fn>
8686

87+
const getExpectedDiffPath = (absolutePath: string, relPath: string) =>
88+
absolutePath === path.resolve(cwd, relPath) ? relPath : absolutePath
89+
8790
beforeEach(() => {
8891
vi.clearAllMocks()
8992

@@ -226,7 +229,7 @@ describe("ApplyPatchTool.execute", () => {
226229

227230
expect(task.rooIgnoreController.validateAccess).toHaveBeenNthCalledWith(1, absolutePath)
228231
expect(task.rooIgnoreController.validateAccess).toHaveBeenNthCalledWith(2, moveAbsolutePath)
229-
expect(task.diffViewProvider.open).toHaveBeenCalledWith(relPath)
232+
expect(task.diffViewProvider.open).toHaveBeenCalledWith(getExpectedDiffPath(absolutePath, relPath))
230233
expect(mockedMkdir).toHaveBeenCalledWith(path.dirname(moveAbsolutePath), { recursive: true })
231234
expect(mockedWriteFile).toHaveBeenCalledWith(moveAbsolutePath, "new\n", "utf8")
232235
expect(mockedUnlink).toHaveBeenCalledWith(absolutePath)

‎src/core/tools/__tests__/listFilesTool.spec.ts‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import type { MockedFunction } from "vitest"
2+
3+
import { listFiles } from "../../../services/glob/list-files"
4+
import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../../utils/pathUtils"
5+
import type { ToolUse } from "../../../shared/tools"
6+
import { listFilesTool } from "../ListFilesTool"
7+
8+
vi.mock("../../../services/glob/list-files", () => ({
9+
listFiles: vi.fn(),
10+
}))
11+
12+
vi.mock("../../../utils/pathUtils", async () => {
13+
const actual = await vi.importActual<typeof import("../../../utils/pathUtils")>("../../../utils/pathUtils")
14+
return {
15+
...actual,
16+
resolvePathInWorkspace: vi.fn(),
17+
getWorkspaceReadablePath: vi.fn(),
18+
isPathOutsideWorkspace: vi.fn(),
19+
}
20+
})
21+
22+
vi.mock("../../prompts/responses", () => ({
23+
formatResponse: {
24+
formatFilesList: vi.fn(),
25+
},
26+
}))
27+
28+
describe("listFilesTool", () => {
29+
const cwd = "/workspace/primary"
30+
const relPath = "secondary/src"
31+
const absolutePath = "/workspace/secondary/src"
32+
33+
const mockedListFiles = listFiles as MockedFunction<typeof listFiles>
34+
const mockedResolvePathInWorkspace = resolvePathInWorkspace as MockedFunction<typeof resolvePathInWorkspace>
35+
const mockedGetWorkspaceReadablePath = getWorkspaceReadablePath as MockedFunction<typeof getWorkspaceReadablePath>
36+
const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction<typeof isPathOutsideWorkspace>
37+
38+
let task: any
39+
let askApproval: ReturnType<typeof vi.fn>
40+
let pushToolResult: ReturnType<typeof vi.fn>
41+
let handleError: ReturnType<typeof vi.fn>
42+
43+
beforeEach(() => {
44+
vi.clearAllMocks()
45+
46+
mockedResolvePathInWorkspace.mockResolvedValue(absolutePath)
47+
mockedGetWorkspaceReadablePath.mockReturnValue("secondary/src")
48+
mockedIsPathOutsideWorkspace.mockReturnValue(false)
49+
mockedListFiles.mockResolvedValue([["a.ts", "b.ts"], false])
50+
51+
task = {
52+
cwd,
53+
consecutiveMistakeCount: 0,
54+
didToolFailInCurrentTurn: false,
55+
recordToolError: vi.fn(),
56+
rooIgnoreController: {},
57+
rooProtectedController: {},
58+
providerRef: {
59+
deref: vi.fn().mockReturnValue({
60+
getState: vi.fn().mockResolvedValue({ showRooIgnoredFiles: true }),
61+
}),
62+
},
63+
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing path"),
64+
ask: vi.fn().mockResolvedValue(undefined),
65+
}
66+
67+
askApproval = vi.fn().mockResolvedValue(true)
68+
pushToolResult = vi.fn()
69+
handleError = vi.fn()
70+
})
71+
72+
it("lists files from the resolved workspace path and surfaces the workspace-readable path", async () => {
73+
const { formatResponse } = await import("../../prompts/responses")
74+
vi.mocked(formatResponse.formatFilesList).mockReturnValue("formatted file list")
75+
76+
await listFilesTool.execute({ path: relPath, recursive: true }, task, {
77+
askApproval,
78+
pushToolResult,
79+
handleError,
80+
})
81+
82+
expect(mockedResolvePathInWorkspace).toHaveBeenCalledWith(cwd, relPath)
83+
expect(mockedListFiles).toHaveBeenCalledWith(absolutePath, true, 200)
84+
expect(formatResponse.formatFilesList).toHaveBeenCalledWith(
85+
absolutePath,
86+
["a.ts", "b.ts"],
87+
false,
88+
task.rooIgnoreController,
89+
true,
90+
task.rooProtectedController,
91+
)
92+
expect(askApproval).toHaveBeenCalledWith("tool", expect.stringContaining('"path":"secondary/src"'))
93+
expect(pushToolResult).toHaveBeenCalledWith("formatted file list")
94+
})
95+
96+
it("uses the resolved workspace path in partial payloads", async () => {
97+
const block: ToolUse<"list_files"> = {
98+
type: "tool_use",
99+
name: "list_files",
100+
params: { path: relPath, recursive: "true" },
101+
partial: true,
102+
}
103+
104+
await listFilesTool.handlePartial(task, block)
105+
106+
expect(task.ask).toHaveBeenCalledWith("tool", expect.stringContaining('"path":"secondary/src"'), true)
107+
expect(task.ask).toHaveBeenCalledWith("tool", expect.stringContaining('"tool":"listFilesRecursive"'), true)
108+
})
109+
})

‎src/core/tools/__tests__/searchFilesTool.spec.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import type { MockedFunction } from "vitest"
2+
3+
import { regexSearchFiles } from "../../../services/ripgrep"
4+
import { getWorkspaceReadablePath, isPathOutsideWorkspace, resolvePathInWorkspace } from "../../../utils/pathUtils"
5+
import type { ToolUse } from "../../../shared/tools"
6+
import { searchFilesTool } from "../SearchFilesTool"
7+
8+
vi.mock("../../../services/ripgrep", () => ({
9+
regexSearchFiles: vi.fn(),
10+
}))
11+
12+
vi.mock("../../../utils/pathUtils", async () => {
13+
const actual = await vi.importActual<typeof import("../../../utils/pathUtils")>("../../../utils/pathUtils")
14+
return {
15+
...actual,
16+
resolvePathInWorkspace: vi.fn(),
17+
getWorkspaceReadablePath: vi.fn(),
18+
isPathOutsideWorkspace: vi.fn(),
19+
}
20+
})
21+
22+
describe("searchFilesTool", () => {
23+
const cwd = "/workspace/primary"
24+
const relPath = "secondary/src"
25+
const absolutePath = "/workspace/secondary/src"
26+
27+
const mockedRegexSearchFiles = regexSearchFiles as MockedFunction<typeof regexSearchFiles>
28+
const mockedResolvePathInWorkspace = resolvePathInWorkspace as MockedFunction<typeof resolvePathInWorkspace>
29+
const mockedGetWorkspaceReadablePath = getWorkspaceReadablePath as MockedFunction<typeof getWorkspaceReadablePath>
30+
const mockedIsPathOutsideWorkspace = isPathOutsideWorkspace as MockedFunction<typeof isPathOutsideWorkspace>
31+
32+
let task: any
33+
let askApproval: ReturnType<typeof vi.fn>
34+
let pushToolResult: ReturnType<typeof vi.fn>
35+
let handleError: ReturnType<typeof vi.fn>
36+
37+
beforeEach(() => {
38+
vi.clearAllMocks()
39+
40+
mockedResolvePathInWorkspace.mockResolvedValue(absolutePath)
41+
mockedGetWorkspaceReadablePath.mockReturnValue("secondary/src")
42+
mockedIsPathOutsideWorkspace.mockReturnValue(false)
43+
mockedRegexSearchFiles.mockResolvedValue("match results")
44+
45+
task = {
46+
cwd,
47+
consecutiveMistakeCount: 0,
48+
didToolFailInCurrentTurn: false,
49+
recordToolError: vi.fn(),
50+
rooIgnoreController: {},
51+
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing param"),
52+
ask: vi.fn().mockResolvedValue(undefined),
53+
}
54+
55+
askApproval = vi.fn().mockResolvedValue(true)
56+
pushToolResult = vi.fn()
57+
handleError = vi.fn()
58+
})
59+
60+
it("searches from the resolved workspace path and surfaces the workspace-readable path", async () => {
61+
await searchFilesTool.execute({ path: relPath, regex: "TODO", file_pattern: "*.ts" }, task, {
62+
askApproval,
63+
pushToolResult,
64+
handleError,
65+
})
66+
67+
expect(mockedResolvePathInWorkspace).toHaveBeenCalledWith(cwd, relPath)
68+
expect(mockedRegexSearchFiles).toHaveBeenCalledWith(cwd, absolutePath, "TODO", "*.ts", task.rooIgnoreController)
69+
expect(askApproval).toHaveBeenCalledWith("tool", expect.stringContaining('"path":"secondary/src"'))
70+
expect(pushToolResult).toHaveBeenCalledWith("match results")
71+
})
72+
73+
it("uses the resolved workspace path in partial payloads", async () => {
74+
const block: ToolUse<"search_files"> = {
75+
type: "tool_use",
76+
name: "search_files",
77+
params: { path: relPath, regex: "TODO", file_pattern: "*.ts" },
78+
partial: true,
79+
}
80+
81+
await searchFilesTool.handlePartial(task, block)
82+
83+
expect(task.ask).toHaveBeenCalledWith("tool", expect.stringContaining('"path":"secondary/src"'), true)
84+
expect(task.ask).toHaveBeenCalledWith("tool", expect.stringContaining('"tool":"searchFiles"'), true)
85+
})
86+
})

‎src/core/tools/__tests__/writeToFileTool.spec.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ describe("writeToFileTool", () => {
123123
let mockPushToolResult: ReturnType<typeof vi.fn>
124124
let toolResult: ToolResponse | undefined
125125

126+
const getExpectedDiffPath = (absolutePath: string, relPath: string) =>
127+
absolutePath === path.resolve(mockCline.cwd, relPath) ? relPath : absolutePath
128+
126129
beforeEach(() => {
127130
vi.clearAllMocks()
128131
writeToFileTool.resetPartialState()
@@ -258,7 +261,9 @@ describe("writeToFileTool", () => {
258261
await executeWriteFileTool({}, { accessAllowed: true })
259262

260263
expect(mockCline.rooIgnoreController.validateAccess).toHaveBeenCalledWith(absoluteFilePath)
261-
expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath)
264+
expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(
265+
getExpectedDiffPath(absoluteFilePath, testFilePath),
266+
)
262267
})
263268

264269
it("opens the absolute diff path when the resolver lands outside task.cwd", async () => {
@@ -388,7 +393,9 @@ describe("writeToFileTool", () => {
388393
await executeWriteFileTool({}, { fileExists: false })
389394

390395
expect(mockCline.consecutiveMistakeCount).toBe(0)
391-
expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath)
396+
expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(
397+
getExpectedDiffPath(absoluteFilePath, testFilePath),
398+
)
392399
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(testContent, true)
393400
expect(mockAskApproval).toHaveBeenCalled()
394401
expect(mockCline.diffViewProvider.saveChanges).toHaveBeenCalled()
@@ -435,7 +442,9 @@ describe("writeToFileTool", () => {
435442
// Second call with same path - path is now stabilized, file operations proceed
436443
await executeWriteFileTool({}, { isPartial: true })
437444
expect(mockCline.ask).toHaveBeenCalled()
438-
expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath)
445+
expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(
446+
getExpectedDiffPath(absoluteFilePath, testFilePath),
447+
)
439448
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(testContent, false)
440449
})
441450
})

0 commit comments

Comments
 (0)