diff --git a/src/services/glob/__tests__/list-files.spec.ts b/src/services/glob/__tests__/list-files.spec.ts index 2a154248307..6c133a732a3 100644 --- a/src/services/glob/__tests__/list-files.spec.ts +++ b/src/services/glob/__tests__/list-files.spec.ts @@ -1,5 +1,7 @@ -import { describe, it, expect, vi } from "vitest" +import { vi, describe, it, expect, beforeEach } from "vitest" +import * as path from "path" import { listFiles } from "../list-files" +import * as childProcess from "child_process" vi.mock("../list-files", async () => { const actual = await vi.importActual("../list-files") @@ -16,3 +18,201 @@ describe("listFiles", () => { expect(result).toEqual([[], false]) }) }) + +// 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([]), + }, +})) + +// Import fs to set up mocks +import * as fs from "fs" + +vi.mock("child_process", () => ({ + spawn: vi.fn(), +})) + +vi.mock("../../path", () => ({ + arePathsEqual: vi.fn().mockReturnValue(false), +})) + +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) + }) + + it("should ensure first-level directories are included when limit is reached", async () => { + // Mock fs.promises.readdir to simulate a directory structure + const mockReaddir = vi.mocked(fs.promises.readdir) + + // Root directory with first-level directories + mockReaddir.mockResolvedValueOnce([ + { name: "a_dir", isDirectory: () => true, isSymbolicLink: () => false, isFile: () => false } as any, + { name: "b_dir", isDirectory: () => true, isSymbolicLink: () => false, isFile: () => false } as any, + { name: "c_dir", isDirectory: () => true, isSymbolicLink: () => false, isFile: () => false } as any, + { name: "file1.txt", isDirectory: () => false, isSymbolicLink: () => false, isFile: () => true } as any, + { name: "file2.txt", isDirectory: () => false, isSymbolicLink: () => false, isFile: () => true } as any, + ]) + + // Mock ripgrep to return many files (simulating hitting the limit) + const mockSpawn = vi.mocked(childProcess.spawn) + const mockProcess = { + stdout: { + on: vi.fn((event, callback) => { + if (event === "data") { + // Return many file paths to trigger the limit + const paths = + [ + "/test/dir/a_dir/", + "/test/dir/a_dir/subdir1/", + "/test/dir/a_dir/subdir1/file1.txt", + "/test/dir/a_dir/subdir1/file2.txt", + "/test/dir/a_dir/subdir2/", + "/test/dir/a_dir/subdir2/file3.txt", + "/test/dir/a_dir/file4.txt", + "/test/dir/a_dir/file5.txt", + "/test/dir/file1.txt", + "/test/dir/file2.txt", + // Note: b_dir and c_dir are missing from ripgrep output + ].join("\n") + "\n" + setTimeout(() => callback(paths), 10) + } + }), + }, + stderr: { + on: vi.fn(), + }, + on: vi.fn((event, callback) => { + if (event === "close") { + setTimeout(() => callback(0), 20) + } + }), + kill: vi.fn(), + } + mockSpawn.mockReturnValue(mockProcess as any) + + // Mock fs.promises.access to simulate .gitignore doesn't exist + vi.mocked(fs.promises.access).mockRejectedValue(new Error("File not found")) + + // Call listFiles with recursive=true and a small limit + const [results, limitReached] = await listFiles("/test/dir", true, 10) + + // Verify that we got results and hit the limit + expect(results.length).toBe(10) + expect(limitReached).toBe(true) + + // Count directories in results + const directories = results.filter((r) => r.endsWith("/")) + + // We should have at least the 3 first-level directories + // even if ripgrep didn't return all of them + expect(directories.length).toBeGreaterThanOrEqual(3) + + // Verify all first-level directories are included + const hasADir = results.some((r) => r.endsWith("a_dir/")) + const hasBDir = results.some((r) => r.endsWith("b_dir/")) + const hasCDir = results.some((r) => r.endsWith("c_dir/")) + + expect(hasADir).toBe(true) + expect(hasBDir).toBe(true) + expect(hasCDir).toBe(true) + }) +}) diff --git a/src/services/glob/list-files.ts b/src/services/glob/list-files.ts index 3164ed1eb01..05fa8a1d7b4 100644 --- a/src/services/glob/list-files.ts +++ b/src/services/glob/list-files.ts @@ -32,15 +32,105 @@ export async function listFiles(dirPath: string, recursive: boolean, limit: numb // Get ripgrep path const rgPath = await getRipgrepPath() - // Get files using ripgrep - const files = await listFilesWithRipgrep(rgPath, dirPath, recursive, limit) + if (!recursive) { + // For non-recursive, use the existing approach + const files = await listFilesWithRipgrep(rgPath, dirPath, false, limit) + const ignoreInstance = await createIgnoreInstance(dirPath) + const directories = await listFilteredDirectories(dirPath, false, ignoreInstance) + return formatAndCombineResults(files, directories, limit) + } - // Get directories with proper filtering using ignore library + // For recursive mode, use the original approach but ensure first-level directories are included + const files = await listFilesWithRipgrep(rgPath, dirPath, true, limit) const ignoreInstance = await createIgnoreInstance(dirPath) - const directories = await listFilteredDirectories(dirPath, recursive, ignoreInstance) + const directories = await listFilteredDirectories(dirPath, true, ignoreInstance) + + // Combine and check if we hit the limit + const [results, limitReached] = formatAndCombineResults(files, directories, limit) - // Combine and format the results - return formatAndCombineResults(files, directories, limit) + // If we hit the limit, ensure all first-level directories are included + if (limitReached) { + const firstLevelDirs = await getFirstLevelDirectories(dirPath, ignoreInstance) + return ensureFirstLevelDirectoriesIncluded(results, firstLevelDirs, limit) + } + + return [results, limitReached] +} + +/** + * Get only the first-level directories in a path + */ +async function getFirstLevelDirectories(dirPath: string, ignoreInstance: ReturnType): Promise { + const absolutePath = path.resolve(dirPath) + const directories: string[] = [] + + try { + const entries = await fs.promises.readdir(absolutePath, { withFileTypes: true }) + + for (const entry of entries) { + if (entry.isDirectory() && !entry.isSymbolicLink()) { + const fullDirPath = path.join(absolutePath, entry.name) + if (shouldIncludeDirectory(entry.name, fullDirPath, dirPath, ignoreInstance)) { + const formattedPath = fullDirPath.endsWith("/") ? fullDirPath : `${fullDirPath}/` + directories.push(formattedPath) + } + } + } + } catch (err) { + console.warn(`Could not read directory ${absolutePath}: ${err}`) + } + + return directories +} + +/** + * Ensure all first-level directories are included in the results + */ +function ensureFirstLevelDirectoriesIncluded( + results: string[], + firstLevelDirs: string[], + limit: number, +): [string[], boolean] { + // Create a set of existing paths for quick lookup + const existingPaths = new Set(results) + + // Find missing first-level directories + const missingDirs = firstLevelDirs.filter((dir) => !existingPaths.has(dir)) + + if (missingDirs.length === 0) { + // All first-level directories are already included + return [results, true] + } + + // We need to make room for the missing directories + // Remove items from the end (which are likely deeper in the tree) + const itemsToRemove = Math.min(missingDirs.length, results.length) + const adjustedResults = results.slice(0, results.length - itemsToRemove) + + // Add the missing directories at the beginning (after any existing first-level dirs) + // First, separate existing results into first-level and others + const resultPaths = adjustedResults.map((r) => path.resolve(r)) + const basePath = path.resolve(firstLevelDirs[0]).split(path.sep).slice(0, -1).join(path.sep) + + const firstLevelResults: string[] = [] + const otherResults: string[] = [] + + for (let i = 0; i < adjustedResults.length; i++) { + const resolvedPath = resultPaths[i] + const relativePath = path.relative(basePath, resolvedPath) + const depth = relativePath.split(path.sep).length + + if (depth === 1) { + firstLevelResults.push(adjustedResults[i]) + } else { + otherResults.push(adjustedResults[i]) + } + } + + // Combine: existing first-level dirs + missing first-level dirs + other results + const finalResults = [...firstLevelResults, ...missingDirs, ...otherResults].slice(0, limit) + + return [finalResults, true] } /** @@ -312,7 +402,6 @@ function isDirectoryExplicitlyIgnored(dirName: string): boolean { return false } - /** * Combine file and directory results and format them properly */