Просмотр исходного кода

fix: add symlink support to list_files tool (#4654) (#4859)

Co-authored-by: Daniel Riccio <[email protected]>
Hannes Rudolph 6 месяцев назад
Родитель
Сommit
7a8483dc12

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

@@ -387,6 +387,124 @@ This directory contains various files and subdirectories for testing the list_fi
 		}
 	})
 
+	test("Should list symlinked files and directories", async function () {
+		const api = globalThis.api
+		const messages: ClineMessage[] = []
+		let taskCompleted = false
+		let toolExecuted = false
+		let listResults: string | null = null
+
+		// Listen for messages
+		const messageHandler = ({ message }: { message: ClineMessage }) => {
+			messages.push(message)
+
+			// Check for tool execution and capture results
+			if (message.type === "say" && message.say === "api_req_started") {
+				const text = message.text || ""
+				if (text.includes("list_files")) {
+					toolExecuted = true
+					console.log("list_files tool executed (symlinks):", text.substring(0, 200))
+
+					// Extract list results from the tool execution
+					try {
+						const jsonMatch = text.match(/\{"request":".*?"\}/)
+						if (jsonMatch) {
+							const requestData = JSON.parse(jsonMatch[0])
+							if (requestData.request && requestData.request.includes("Result:")) {
+								listResults = requestData.request
+								console.log("Captured symlink test results:", listResults?.substring(0, 300))
+							}
+						}
+					} catch (e) {
+						console.log("Failed to parse symlink test results:", e)
+					}
+				}
+			}
+		}
+		api.on("message", messageHandler)
+
+		// Listen for task completion
+		const taskCompletedHandler = (id: string) => {
+			if (id === taskId) {
+				taskCompleted = true
+			}
+		}
+		api.on("taskCompleted", taskCompletedHandler)
+
+		let taskId: string
+		try {
+			// Create a symlink test directory
+			const testDirName = `symlink-test-${Date.now()}`
+			const testDir = path.join(workspaceDir, testDirName)
+			await fs.mkdir(testDir, { recursive: true })
+
+			// Create a source directory with content
+			const sourceDir = path.join(testDir, "source")
+			await fs.mkdir(sourceDir, { recursive: true })
+			const sourceFile = path.join(sourceDir, "source-file.txt")
+			await fs.writeFile(sourceFile, "Content from symlinked file")
+
+			// Create symlinks to file and directory
+			const symlinkFile = path.join(testDir, "link-to-file.txt")
+			const symlinkDir = path.join(testDir, "link-to-dir")
+
+			try {
+				await fs.symlink(sourceFile, symlinkFile)
+				await fs.symlink(sourceDir, symlinkDir)
+				console.log("Created symlinks successfully")
+			} catch (symlinkError) {
+				console.log("Symlink creation failed (might be platform limitation):", symlinkError)
+				// Skip test if symlinks can't be created
+				console.log("Skipping symlink test - platform doesn't support symlinks")
+				return
+			}
+
+			// Start task to list files in symlink test directory
+			taskId = await api.startNewTask({
+				configuration: {
+					mode: "code",
+					autoApprovalEnabled: true,
+					alwaysAllowReadOnly: true,
+					alwaysAllowReadOnlyOutsideWorkspace: true,
+				},
+				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.`,
+			})
+
+			console.log("Symlink test Task ID:", taskId)
+
+			// Wait for task completion
+			await waitFor(() => taskCompleted, { timeout: 60_000 })
+
+			// Verify the list_files tool was executed
+			assert.ok(toolExecuted, "The list_files tool should have been executed")
+
+			// Verify the tool returned results
+			assert.ok(listResults, "Tool execution results should be captured")
+
+			const results = listResults as string
+			console.log("Symlink test results:", results)
+
+			// Check that symlinked items are visible
+			assert.ok(
+				results.includes("link-to-file.txt") || results.includes("source-file.txt"),
+				"Should see either the symlink or the target file",
+			)
+			assert.ok(
+				results.includes("link-to-dir") || results.includes("source/"),
+				"Should see either the symlink or the target directory",
+			)
+
+			console.log("Test passed! Symlinked files and directories are now visible")
+
+			// Cleanup
+			await fs.rm(testDir, { recursive: true, force: true })
+		} finally {
+			// Clean up
+			api.off("message", messageHandler)
+			api.off("taskCompleted", taskCompletedHandler)
+		}
+	})
+
 	test("Should list files in workspace root directory", async function () {
 		const api = globalThis.api
 		const messages: ClineMessage[] = []

+ 123 - 0
src/services/glob/__tests__/list-files.spec.ts

@@ -0,0 +1,123 @@
+import { vi, describe, it, expect, beforeEach } from "vitest"
+import * as path from "path"
+
+// Mock ripgrep to avoid filesystem dependencies
+vi.mock("../../ripgrep", () => ({
+	getBinPath: vi.fn().mockResolvedValue("/mock/path/to/rg"),
+}))
+
+// Mock vscode
+vi.mock("vscode", () => ({
+	env: {
+		appRoot: "/mock/app/root",
+	},
+}))
+
+// Mock filesystem operations
+vi.mock("fs", () => ({
+	promises: {
+		access: vi.fn().mockRejectedValue(new Error("Not found")),
+		readFile: vi.fn().mockResolvedValue(""),
+		readdir: vi.fn().mockResolvedValue([]),
+	},
+}))
+
+vi.mock("child_process", () => ({
+	spawn: vi.fn(),
+}))
+
+vi.mock("../../path", () => ({
+	arePathsEqual: vi.fn().mockReturnValue(false),
+}))
+
+import { listFiles } from "../list-files"
+import * as childProcess from "child_process"
+
+describe("list-files symlink support", () => {
+	beforeEach(() => {
+		vi.clearAllMocks()
+	})
+
+	it("should include --follow flag in ripgrep arguments", async () => {
+		const mockSpawn = vi.mocked(childProcess.spawn)
+		const mockProcess = {
+			stdout: {
+				on: vi.fn((event, callback) => {
+					if (event === "data") {
+						// Simulate some output to complete the process
+						setTimeout(() => callback("test-file.txt\n"), 10)
+					}
+				}),
+			},
+			stderr: {
+				on: vi.fn(),
+			},
+			on: vi.fn((event, callback) => {
+				if (event === "close") {
+					setTimeout(() => callback(0), 20)
+				}
+				if (event === "error") {
+					// No error simulation
+				}
+			}),
+			kill: vi.fn(),
+		}
+
+		mockSpawn.mockReturnValue(mockProcess as any)
+
+		// Call listFiles to trigger ripgrep execution
+		await listFiles("/test/dir", false, 100)
+
+		// Verify that spawn was called with --follow flag (the critical fix)
+		const [rgPath, args] = mockSpawn.mock.calls[0]
+		expect(rgPath).toBe("/mock/path/to/rg")
+		expect(args).toContain("--files")
+		expect(args).toContain("--hidden")
+		expect(args).toContain("--follow") // This is the critical assertion - the fix should add this flag
+
+		// Platform-agnostic path check - verify the last argument is the resolved path
+		const expectedPath = path.resolve("/test/dir")
+		expect(args[args.length - 1]).toBe(expectedPath)
+	})
+
+	it("should include --follow flag for recursive listings too", async () => {
+		const mockSpawn = vi.mocked(childProcess.spawn)
+		const mockProcess = {
+			stdout: {
+				on: vi.fn((event, callback) => {
+					if (event === "data") {
+						setTimeout(() => callback("test-file.txt\n"), 10)
+					}
+				}),
+			},
+			stderr: {
+				on: vi.fn(),
+			},
+			on: vi.fn((event, callback) => {
+				if (event === "close") {
+					setTimeout(() => callback(0), 20)
+				}
+				if (event === "error") {
+					// No error simulation
+				}
+			}),
+			kill: vi.fn(),
+		}
+
+		mockSpawn.mockReturnValue(mockProcess as any)
+
+		// Call listFiles with recursive=true
+		await listFiles("/test/dir", true, 100)
+
+		// Verify that spawn was called with --follow flag (the critical fix)
+		const [rgPath, args] = mockSpawn.mock.calls[0]
+		expect(rgPath).toBe("/mock/path/to/rg")
+		expect(args).toContain("--files")
+		expect(args).toContain("--hidden")
+		expect(args).toContain("--follow") // This should be present in recursive mode too
+
+		// Platform-agnostic path check - verify the last argument is the resolved path
+		const expectedPath = path.resolve("/test/dir")
+		expect(args[args.length - 1]).toBe(expectedPath)
+	})
+})

+ 1 - 1
src/services/glob/list-files.ts

@@ -93,7 +93,7 @@ async function listFilesWithRipgrep(
  */
 function buildRipgrepArgs(dirPath: string, recursive: boolean): string[] {
 	// Base arguments to list files
-	const args = ["--files", "--hidden"]
+	const args = ["--files", "--hidden", "--follow"]
 
 	if (recursive) {
 		return [...args, ...buildRecursiveArgs(), dirPath]