From d518f27ecc20ea41b13c074cb5a9b2e4cdbd9b79 Mon Sep 17 00:00:00 2001 From: MuriloFP Date: Tue, 8 Jul 2025 15:03:58 -0300 Subject: [PATCH] fix(glob): show top-level hidden directories in list_files - Modified list-files.ts to show hidden directories at top level - Added test coverage for hidden directory visibility behavior - Maintains existing functionality while fixing the .roo directory issue Addresses PR #5176 feedback with simplified implementation --- .../glob/__tests__/list-files.spec.ts | 121 ++++++++++++++++++ src/services/glob/constants.ts | 1 + src/services/glob/list-files.ts | 100 +++++++++++---- 3 files changed, 198 insertions(+), 24 deletions(-) diff --git a/src/services/glob/__tests__/list-files.spec.ts b/src/services/glob/__tests__/list-files.spec.ts index 6c133a732a..4e55f9558e 100644 --- a/src/services/glob/__tests__/list-files.spec.ts +++ b/src/services/glob/__tests__/list-files.spec.ts @@ -216,3 +216,124 @@ describe("list-files symlink support", () => { expect(hasCDir).toBe(true) }) }) + +describe("hidden directory exclusion", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("should exclude .git subdirectories from recursive directory listing", async () => { + // Mock filesystem structure with .git subdirectories + const mockReaddir = vi.fn() + vi.mocked(fs.promises).readdir = mockReaddir + + // Mock the directory structure: + // /test/ + // .git/ + // hooks/ + // objects/ + // src/ + // components/ + mockReaddir + .mockResolvedValueOnce([ + { name: ".git", isDirectory: () => true, isSymbolicLink: () => false }, + { name: "src", isDirectory: () => true, isSymbolicLink: () => false }, + ]) + .mockResolvedValueOnce([ + // src subdirectories (should be included) + { name: "components", isDirectory: () => true, isSymbolicLink: () => false }, + ]) + .mockResolvedValueOnce([]) // components/ is empty + + // Mock ripgrep to return no files + const mockSpawn = vi.mocked(childProcess.spawn) + const mockProcess = { + stdout: { + on: vi.fn((event, callback) => { + if (event === "data") { + // No files returned + } + }), + }, + stderr: { + on: vi.fn(), + }, + on: vi.fn((event, callback) => { + if (event === "close") { + setTimeout(() => callback(0), 10) + } + }), + kill: vi.fn(), + } + mockSpawn.mockReturnValue(mockProcess as any) + + // Call listFiles with recursive=true + const [result] = await listFiles("/test", true, 100) + + // Verify that .git subdirectories are NOT included + const directories = result.filter((item) => item.endsWith("/")) + + // More specific checks - look for exact paths + const hasSrcDir = directories.some((dir) => dir.endsWith("/test/src/") || dir.endsWith("src/")) + const hasComponentsDir = directories.some( + (dir) => + dir.endsWith("/test/src/components/") || dir.endsWith("src/components/") || dir.includes("components/"), + ) + const hasGitDir = directories.some((dir) => dir.includes(".git/")) + + // Should include src/ and src/components/ but NOT .git/ or its subdirectories + expect(hasSrcDir).toBe(true) + expect(hasComponentsDir).toBe(true) + + // Should NOT include .git (hidden directories are excluded) + expect(hasGitDir).toBe(false) + }) + + it("should allow explicit targeting of hidden directories", async () => { + // Mock filesystem structure for explicit .roo-memory targeting + const mockReaddir = vi.fn() + vi.mocked(fs.promises).readdir = mockReaddir + + // Mock .roo-memory directory contents + mockReaddir.mockResolvedValueOnce([ + { name: "tasks", isDirectory: () => true, isSymbolicLink: () => false }, + { name: "context", isDirectory: () => true, isSymbolicLink: () => false }, + ]) + + // Mock ripgrep to return no files + const mockSpawn = vi.mocked(childProcess.spawn) + const mockProcess = { + stdout: { + on: vi.fn((event, callback) => { + if (event === "data") { + // No files returned + } + }), + }, + stderr: { + on: vi.fn(), + }, + on: vi.fn((event, callback) => { + if (event === "close") { + setTimeout(() => callback(0), 10) + } + }), + kill: vi.fn(), + } + mockSpawn.mockReturnValue(mockProcess as any) + + // Call listFiles explicitly targeting .roo-memory directory + const [result] = await listFiles("/test/.roo-memory", true, 100) + + // When explicitly targeting a hidden directory, its subdirectories should be included + const directories = result.filter((item) => item.endsWith("/")) + + const hasTasksDir = directories.some((dir) => dir.includes(".roo-memory/tasks/") || dir.includes("tasks/")) + const hasContextDir = directories.some( + (dir) => dir.includes(".roo-memory/context/") || dir.includes("context/"), + ) + + expect(hasTasksDir).toBe(true) + expect(hasContextDir).toBe(true) + }) +}) diff --git a/src/services/glob/constants.ts b/src/services/glob/constants.ts index 1ddcc37df9..380e4afaf3 100644 --- a/src/services/glob/constants.ts +++ b/src/services/glob/constants.ts @@ -20,5 +20,6 @@ export const DIRS_TO_IGNORE = [ "deps", "pkg", "Pods", + ".git", ".*", ] diff --git a/src/services/glob/list-files.ts b/src/services/glob/list-files.ts index 027cb42766..2bb11cc4e7 100644 --- a/src/services/glob/list-files.ts +++ b/src/services/glob/list-files.ts @@ -245,8 +245,11 @@ function buildNonRecursiveArgs(): string[] { // Apply directory exclusions for non-recursive searches for (const dir of DIRS_TO_IGNORE) { if (dir === ".*") { - // For hidden files/dirs in non-recursive mode - args.push("-g", "!.*") + // For hidden directories in non-recursive mode, we want to show the directories + // themselves but not their contents. Since we're using --maxdepth 1, this + // naturally happens - we just need to avoid excluding the directories entirely. + // We'll let the directory scanning logic handle the visibility. + continue } else { // Direct children only args.push("-g", `!${dir}`) @@ -326,7 +329,11 @@ async function listFilteredDirectories( const absolutePath = path.resolve(dirPath) const directories: string[] = [] - async function scanDirectory(currentPath: string, isTargetDir: boolean = false): Promise { + async function scanDirectory( + currentPath: string, + isTargetDir: boolean = false, + insideExplicitHiddenTarget: boolean = false, + ): Promise { try { // List all entries in the current directory const entries = await fs.promises.readdir(currentPath, { withFileTypes: true }) @@ -338,19 +345,38 @@ async function listFilteredDirectories( const fullDirPath = path.join(currentPath, dirName) // Check if this directory should be included - if (shouldIncludeDirectory(dirName, fullDirPath, dirPath, ignoreInstance, isTargetDir)) { + // Subdirectories found during scanning are never target directories themselves + if (shouldIncludeDirectory(dirName, fullDirPath, dirPath, ignoreInstance, false, insideExplicitHiddenTarget)) { // Add the directory to our results (with trailing slash) const formattedPath = fullDirPath.endsWith("/") ? fullDirPath : `${fullDirPath}/` directories.push(formattedPath) + } - // If recursive mode and not a ignored directory, scan subdirectories - // Don't recurse into hidden directories unless they are the explicit target - const isHiddenDir = dirName.startsWith(".") - const shouldRecurse = - recursive && !isDirectoryExplicitlyIgnored(dirName) && !(isHiddenDir && !isTargetDir) - if (shouldRecurse) { - await scanDirectory(fullDirPath, false) // Subdirectories are not target dirs - } + // If recursive mode and not a ignored directory, scan subdirectories + // Don't recurse into hidden directories unless they are the explicit target + // or we're already inside an explicitly targeted hidden directory + const isHiddenDir = dirName.startsWith(".") + + // Use the same logic as shouldIncludeDirectory for recursion decisions + // When inside an explicitly targeted hidden directory, only block critical directories + let shouldRecurseIntoDir = true + if (insideExplicitHiddenTarget) { + // Only apply the most critical ignore patterns when inside explicit hidden target + const criticalIgnorePatterns = ["node_modules", ".git", "__pycache__", "venv", "env"] + shouldRecurseIntoDir = !criticalIgnorePatterns.includes(dirName) + } else { + shouldRecurseIntoDir = !isDirectoryExplicitlyIgnored(dirName) + } + + const shouldRecurse = + recursive && + shouldRecurseIntoDir && + !(isHiddenDir && DIRS_TO_IGNORE.includes(".*") && !isTargetDir && !insideExplicitHiddenTarget) + if (shouldRecurse) { + // If we're entering a hidden directory that's the target, or we're already inside one, + // mark that we're inside an explicitly targeted hidden directory + const newInsideExplicitHiddenTarget = insideExplicitHiddenTarget || (isHiddenDir && isTargetDir) + await scanDirectory(fullDirPath, false, newInsideExplicitHiddenTarget) } } } @@ -360,8 +386,12 @@ async function listFilteredDirectories( } } - // Start scanning from the root directory - this is the explicitly targeted directory - await scanDirectory(absolutePath, true) + // Start scanning from the root directory + // For environment details generation, we don't want to treat the root as a "target" + // if we're doing a general recursive scan, as this would include hidden directories + // Only treat as target if we're explicitly scanning a single hidden directory + const isExplicitHiddenTarget = path.basename(absolutePath).startsWith(".") + await scanDirectory(absolutePath, isExplicitHiddenTarget, isExplicitHiddenTarget) return directories } @@ -375,9 +405,10 @@ function shouldIncludeDirectory( basePath: string, ignoreInstance: ReturnType, isTargetDir: boolean = false, + insideExplicitHiddenTarget: boolean = false, ): boolean { - // If this is the explicitly targeted directory, always include it - // (unless it's explicitly ignored by name, not by the .* pattern) + // If this is the explicitly targeted directory, allow it even if it's hidden + // This preserves the ability to explicitly target hidden directories like .roo-memory if (isTargetDir) { // Only apply non-hidden-directory ignore rules to target directories const nonHiddenIgnorePatterns = DIRS_TO_IGNORE.filter((pattern) => pattern !== ".*") @@ -389,16 +420,32 @@ function shouldIncludeDirectory( return true } - // For non-target directories (subdirectories found during traversal), apply all ignore rules - - // Skip hidden directories if configured to ignore them - if (dirName.startsWith(".") && DIRS_TO_IGNORE.includes(".*")) { - return false + // If we're inside an explicitly targeted hidden directory, allow subdirectories + // even if they would normally be filtered out by the ".*" pattern or other ignore rules + if (insideExplicitHiddenTarget) { + // Only apply the most critical ignore patterns when inside explicit hidden target + // Allow temp, rules, etc. but still block node_modules, .git, etc. + const criticalIgnorePatterns = ["node_modules", ".git", "__pycache__", "venv", "env"] + for (const pattern of criticalIgnorePatterns) { + if (pattern === dirName || (pattern.includes("/") && pattern.split("/")[0] === dirName)) { + return false + } + } + // Check against gitignore patterns using the ignore library + const relativePath = path.relative(basePath, fullDirPath) + const normalizedPath = relativePath.replace(/\\/g, "/") + if (ignoreInstance.ignores(normalizedPath) || ignoreInstance.ignores(normalizedPath + "/")) { + return false + } + return true } - // Check against explicit ignore patterns - if (isDirectoryExplicitlyIgnored(dirName)) { - return false + // Check against explicit ignore patterns (excluding the ".*" pattern for now) + const nonHiddenIgnorePatterns = DIRS_TO_IGNORE.filter((pattern) => pattern !== ".*") + for (const pattern of nonHiddenIgnorePatterns) { + if (pattern === dirName || (pattern.includes("/") && pattern.split("/")[0] === dirName)) { + return false + } } // Check against gitignore patterns using the ignore library @@ -424,6 +471,11 @@ function isDirectoryExplicitlyIgnored(dirName: string): boolean { return true } + // Skip the ".*" pattern - it's handled specially to allow top-level visibility + if (pattern === ".*") { + continue + } + // Path patterns that contain / if (pattern.includes("/")) { const pathParts = pattern.split("/")