Skip to content

Commit 6d4d32e

Browse files
committed
fix: add symlink support to list_files tool (#4654)
- Add --follow flag to ripgrep args in buildRipgrepArgs function - Enable traversal of symbolic links when listing files - Add comprehensive unit tests for symlink flag inclusion - Add E2E test case for symlink functionality - Aligns with existing patterns in file-search.ts and ShadowCheckpointService.ts Fixes #4654
1 parent 72cb248 commit 6d4d32e

File tree

3 files changed

+235
-1
lines changed

3 files changed

+235
-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: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import { vi, describe, it, expect, beforeEach } from "vitest"
2+
3+
// Mock ripgrep to avoid filesystem dependencies
4+
vi.mock("../../ripgrep", () => ({
5+
getBinPath: vi.fn().mockResolvedValue("/mock/path/to/rg"),
6+
}))
7+
8+
// Mock vscode
9+
vi.mock("vscode", () => ({
10+
env: {
11+
appRoot: "/mock/app/root",
12+
},
13+
}))
14+
15+
// Mock filesystem operations
16+
vi.mock("fs", () => ({
17+
promises: {
18+
access: vi.fn().mockRejectedValue(new Error("Not found")),
19+
readFile: vi.fn().mockResolvedValue(""),
20+
readdir: vi.fn().mockResolvedValue([]),
21+
},
22+
}))
23+
24+
vi.mock("child_process", () => ({
25+
spawn: vi.fn(),
26+
}))
27+
28+
vi.mock("../../path", () => ({
29+
arePathsEqual: vi.fn().mockReturnValue(false),
30+
}))
31+
32+
import { listFiles } from "../list-files"
33+
import * as childProcess from "child_process"
34+
35+
describe("list-files symlink support", () => {
36+
beforeEach(() => {
37+
vi.clearAllMocks()
38+
})
39+
40+
it("should include --follow flag in ripgrep arguments", async () => {
41+
const mockSpawn = vi.mocked(childProcess.spawn)
42+
const mockProcess = {
43+
stdout: {
44+
on: vi.fn((event, callback) => {
45+
if (event === "data") {
46+
// Simulate some output to complete the process
47+
setTimeout(() => callback("test-file.txt\n"), 10)
48+
}
49+
}),
50+
},
51+
stderr: {
52+
on: vi.fn(),
53+
},
54+
on: vi.fn((event, callback) => {
55+
if (event === "close") {
56+
setTimeout(() => callback(0), 20)
57+
}
58+
if (event === "error") {
59+
// No error simulation
60+
}
61+
}),
62+
kill: vi.fn(),
63+
}
64+
65+
mockSpawn.mockReturnValue(mockProcess as any)
66+
67+
// Call listFiles to trigger ripgrep execution
68+
await listFiles("/test/dir", false, 100)
69+
70+
// Verify that spawn was called with --follow flag (the critical fix)
71+
const [rgPath, args] = mockSpawn.mock.calls[0]
72+
expect(rgPath).toBe("/mock/path/to/rg")
73+
expect(args).toContain("--files")
74+
expect(args).toContain("--hidden")
75+
expect(args).toContain("--follow") // This is the critical assertion - the fix should add this flag
76+
expect(args[args.length - 1]).toContain("/test/dir") // Last argument should be the directory
77+
})
78+
79+
it("should include --follow flag for recursive listings too", async () => {
80+
const mockSpawn = vi.mocked(childProcess.spawn)
81+
const mockProcess = {
82+
stdout: {
83+
on: vi.fn((event, callback) => {
84+
if (event === "data") {
85+
setTimeout(() => callback("test-file.txt\n"), 10)
86+
}
87+
}),
88+
},
89+
stderr: {
90+
on: vi.fn(),
91+
},
92+
on: vi.fn((event, callback) => {
93+
if (event === "close") {
94+
setTimeout(() => callback(0), 20)
95+
}
96+
if (event === "error") {
97+
// No error simulation
98+
}
99+
}),
100+
kill: vi.fn(),
101+
}
102+
103+
mockSpawn.mockReturnValue(mockProcess as any)
104+
105+
// Call listFiles with recursive=true
106+
await listFiles("/test/dir", true, 100)
107+
108+
// Verify that spawn was called with --follow flag (the critical fix)
109+
const [rgPath, args] = mockSpawn.mock.calls[0]
110+
expect(rgPath).toBe("/mock/path/to/rg")
111+
expect(args).toContain("--files")
112+
expect(args).toContain("--hidden")
113+
expect(args).toContain("--follow") // This should be present in recursive mode too
114+
expect(args[args.length - 1]).toContain("/test/dir") // Last argument should be the directory
115+
})
116+
})

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)