Skip to content

Commit 7a8483d

Browse files
fix: add symlink support to list_files tool (#4654) (#4859)
Co-authored-by: Daniel Riccio <[email protected]>
1 parent 5012922 commit 7a8483d

File tree

3 files changed

+242
-1
lines changed

3 files changed

+242
-1
lines changed

apps/vscode-e2e/src/suite/tools/list-files.test.ts

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,124 @@ This directory contains various files and subdirectories for testing the list_fi
387387
}
388388
})
389389

390+
test("Should list symlinked files and directories", async function () {
391+
const api = globalThis.api
392+
const messages: ClineMessage[] = []
393+
let taskCompleted = false
394+
let toolExecuted = false
395+
let listResults: string | null = null
396+
397+
// Listen for messages
398+
const messageHandler = ({ message }: { message: ClineMessage }) => {
399+
messages.push(message)
400+
401+
// Check for tool execution and capture results
402+
if (message.type === "say" && message.say === "api_req_started") {
403+
const text = message.text || ""
404+
if (text.includes("list_files")) {
405+
toolExecuted = true
406+
console.log("list_files tool executed (symlinks):", text.substring(0, 200))
407+
408+
// Extract list results from the tool execution
409+
try {
410+
const jsonMatch = text.match(/\{"request":".*?"\}/)
411+
if (jsonMatch) {
412+
const requestData = JSON.parse(jsonMatch[0])
413+
if (requestData.request && requestData.request.includes("Result:")) {
414+
listResults = requestData.request
415+
console.log("Captured symlink test results:", listResults?.substring(0, 300))
416+
}
417+
}
418+
} catch (e) {
419+
console.log("Failed to parse symlink test results:", e)
420+
}
421+
}
422+
}
423+
}
424+
api.on("message", messageHandler)
425+
426+
// Listen for task completion
427+
const taskCompletedHandler = (id: string) => {
428+
if (id === taskId) {
429+
taskCompleted = true
430+
}
431+
}
432+
api.on("taskCompleted", taskCompletedHandler)
433+
434+
let taskId: string
435+
try {
436+
// Create a symlink test directory
437+
const testDirName = `symlink-test-${Date.now()}`
438+
const testDir = path.join(workspaceDir, testDirName)
439+
await fs.mkdir(testDir, { recursive: true })
440+
441+
// Create a source directory with content
442+
const sourceDir = path.join(testDir, "source")
443+
await fs.mkdir(sourceDir, { recursive: true })
444+
const sourceFile = path.join(sourceDir, "source-file.txt")
445+
await fs.writeFile(sourceFile, "Content from symlinked file")
446+
447+
// Create symlinks to file and directory
448+
const symlinkFile = path.join(testDir, "link-to-file.txt")
449+
const symlinkDir = path.join(testDir, "link-to-dir")
450+
451+
try {
452+
await fs.symlink(sourceFile, symlinkFile)
453+
await fs.symlink(sourceDir, symlinkDir)
454+
console.log("Created symlinks successfully")
455+
} catch (symlinkError) {
456+
console.log("Symlink creation failed (might be platform limitation):", symlinkError)
457+
// Skip test if symlinks can't be created
458+
console.log("Skipping symlink test - platform doesn't support symlinks")
459+
return
460+
}
461+
462+
// Start task to list files in symlink test directory
463+
taskId = await api.startNewTask({
464+
configuration: {
465+
mode: "code",
466+
autoApprovalEnabled: true,
467+
alwaysAllowReadOnly: true,
468+
alwaysAllowReadOnlyOutsideWorkspace: true,
469+
},
470+
text: `I have created a test directory with symlinks at "${testDirName}". Use the list_files tool to list the contents of this directory. It should show both the original files/directories and the symlinked ones. The directory contains symlinks to both a file and a directory.`,
471+
})
472+
473+
console.log("Symlink test Task ID:", taskId)
474+
475+
// Wait for task completion
476+
await waitFor(() => taskCompleted, { timeout: 60_000 })
477+
478+
// Verify the list_files tool was executed
479+
assert.ok(toolExecuted, "The list_files tool should have been executed")
480+
481+
// Verify the tool returned results
482+
assert.ok(listResults, "Tool execution results should be captured")
483+
484+
const results = listResults as string
485+
console.log("Symlink test results:", results)
486+
487+
// Check that symlinked items are visible
488+
assert.ok(
489+
results.includes("link-to-file.txt") || results.includes("source-file.txt"),
490+
"Should see either the symlink or the target file",
491+
)
492+
assert.ok(
493+
results.includes("link-to-dir") || results.includes("source/"),
494+
"Should see either the symlink or the target directory",
495+
)
496+
497+
console.log("Test passed! Symlinked files and directories are now visible")
498+
499+
// Cleanup
500+
await fs.rm(testDir, { recursive: true, force: true })
501+
} finally {
502+
// Clean up
503+
api.off("message", messageHandler)
504+
api.off("taskCompleted", taskCompletedHandler)
505+
}
506+
})
507+
390508
test("Should list files in workspace root directory", async function () {
391509
const api = globalThis.api
392510
const messages: ClineMessage[] = []
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { vi, describe, it, expect, beforeEach } from "vitest"
2+
import * as path from "path"
3+
4+
// Mock ripgrep to avoid filesystem dependencies
5+
vi.mock("../../ripgrep", () => ({
6+
getBinPath: vi.fn().mockResolvedValue("/mock/path/to/rg"),
7+
}))
8+
9+
// Mock vscode
10+
vi.mock("vscode", () => ({
11+
env: {
12+
appRoot: "/mock/app/root",
13+
},
14+
}))
15+
16+
// Mock filesystem operations
17+
vi.mock("fs", () => ({
18+
promises: {
19+
access: vi.fn().mockRejectedValue(new Error("Not found")),
20+
readFile: vi.fn().mockResolvedValue(""),
21+
readdir: vi.fn().mockResolvedValue([]),
22+
},
23+
}))
24+
25+
vi.mock("child_process", () => ({
26+
spawn: vi.fn(),
27+
}))
28+
29+
vi.mock("../../path", () => ({
30+
arePathsEqual: vi.fn().mockReturnValue(false),
31+
}))
32+
33+
import { listFiles } from "../list-files"
34+
import * as childProcess from "child_process"
35+
36+
describe("list-files symlink support", () => {
37+
beforeEach(() => {
38+
vi.clearAllMocks()
39+
})
40+
41+
it("should include --follow flag in ripgrep arguments", async () => {
42+
const mockSpawn = vi.mocked(childProcess.spawn)
43+
const mockProcess = {
44+
stdout: {
45+
on: vi.fn((event, callback) => {
46+
if (event === "data") {
47+
// Simulate some output to complete the process
48+
setTimeout(() => callback("test-file.txt\n"), 10)
49+
}
50+
}),
51+
},
52+
stderr: {
53+
on: vi.fn(),
54+
},
55+
on: vi.fn((event, callback) => {
56+
if (event === "close") {
57+
setTimeout(() => callback(0), 20)
58+
}
59+
if (event === "error") {
60+
// No error simulation
61+
}
62+
}),
63+
kill: vi.fn(),
64+
}
65+
66+
mockSpawn.mockReturnValue(mockProcess as any)
67+
68+
// Call listFiles to trigger ripgrep execution
69+
await listFiles("/test/dir", false, 100)
70+
71+
// Verify that spawn was called with --follow flag (the critical fix)
72+
const [rgPath, args] = mockSpawn.mock.calls[0]
73+
expect(rgPath).toBe("/mock/path/to/rg")
74+
expect(args).toContain("--files")
75+
expect(args).toContain("--hidden")
76+
expect(args).toContain("--follow") // This is the critical assertion - the fix should add this flag
77+
78+
// Platform-agnostic path check - verify the last argument is the resolved path
79+
const expectedPath = path.resolve("/test/dir")
80+
expect(args[args.length - 1]).toBe(expectedPath)
81+
})
82+
83+
it("should include --follow flag for recursive listings too", async () => {
84+
const mockSpawn = vi.mocked(childProcess.spawn)
85+
const mockProcess = {
86+
stdout: {
87+
on: vi.fn((event, callback) => {
88+
if (event === "data") {
89+
setTimeout(() => callback("test-file.txt\n"), 10)
90+
}
91+
}),
92+
},
93+
stderr: {
94+
on: vi.fn(),
95+
},
96+
on: vi.fn((event, callback) => {
97+
if (event === "close") {
98+
setTimeout(() => callback(0), 20)
99+
}
100+
if (event === "error") {
101+
// No error simulation
102+
}
103+
}),
104+
kill: vi.fn(),
105+
}
106+
107+
mockSpawn.mockReturnValue(mockProcess as any)
108+
109+
// Call listFiles with recursive=true
110+
await listFiles("/test/dir", true, 100)
111+
112+
// Verify that spawn was called with --follow flag (the critical fix)
113+
const [rgPath, args] = mockSpawn.mock.calls[0]
114+
expect(rgPath).toBe("/mock/path/to/rg")
115+
expect(args).toContain("--files")
116+
expect(args).toContain("--hidden")
117+
expect(args).toContain("--follow") // This should be present in recursive mode too
118+
119+
// Platform-agnostic path check - verify the last argument is the resolved path
120+
const expectedPath = path.resolve("/test/dir")
121+
expect(args[args.length - 1]).toBe(expectedPath)
122+
})
123+
})

src/services/glob/list-files.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ async function listFilesWithRipgrep(
9393
*/
9494
function buildRipgrepArgs(dirPath: string, recursive: boolean): string[] {
9595
// Base arguments to list files
96-
const args = ["--files", "--hidden"]
96+
const args = ["--files", "--hidden", "--follow"]
9797

9898
if (recursive) {
9999
return [...args, ...buildRecursiveArgs(), dirPath]

0 commit comments

Comments
 (0)