diff --git a/.changeset/metal-papayas-think.md b/.changeset/metal-papayas-think.md new file mode 100644 index 00000000000..89affd64fb3 --- /dev/null +++ b/.changeset/metal-papayas-think.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Fix bug not to respect symbolic linked rules, if target is a directory or another symbolic link diff --git a/src/core/prompts/sections/__tests__/custom-instructions.test.ts b/src/core/prompts/sections/__tests__/custom-instructions.test.ts index 27492014c5e..77ccba07a04 100644 --- a/src/core/prompts/sections/__tests__/custom-instructions.test.ts +++ b/src/core/prompts/sections/__tests__/custom-instructions.test.ts @@ -615,30 +615,64 @@ describe("Rules directory reading", () => { } as any) // Simulate listing files including a symlink - readdirMock.mockResolvedValueOnce([ - { - name: "regular.txt", - isFile: () => true, - isSymbolicLink: () => false, - parentPath: "/fake/path/.roo/rules", - }, - { name: "link.txt", isFile: () => false, isSymbolicLink: () => true, parentPath: "/fake/path/.roo/rules" }, - ] as any) + readdirMock + .mockResolvedValueOnce([ + { + name: "regular.txt", + isFile: () => true, + isSymbolicLink: () => false, + parentPath: "/fake/path/.roo/rules", + }, + { + name: "link.txt", + isFile: () => false, + isSymbolicLink: () => true, + parentPath: "/fake/path/.roo/rules", + }, + { + name: "link_dir", + isFile: () => false, + isSymbolicLink: () => true, + parentPath: "/fake/path/.roo/rules", + }, + { + name: "nested_link.txt", + isFile: () => false, + isSymbolicLink: () => true, + parentPath: "/fake/path/.roo/rules", + }, + ] as any) + .mockResolvedValueOnce([ + { name: "subdir_link.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules/symlink-target-dir" }, + ] as any) // Simulate readlink response - readlinkMock.mockResolvedValueOnce("../symlink-target.txt") + readlinkMock + .mockResolvedValueOnce("../symlink-target.txt") + .mockResolvedValueOnce("../symlink-target-dir") + .mockResolvedValueOnce("../nested-symlink") + .mockResolvedValueOnce("nested-symlink-target.txt") // Reset and set up the stat mock with more granular control statMock.mockReset() statMock.mockImplementation((path: string) => { // For directory check - if (path === "/fake/path/.roo/rules") { + if (path === "/fake/path/.roo/rules" || path.endsWith("dir")) { return Promise.resolve({ isDirectory: jest.fn().mockReturnValue(true), isFile: jest.fn().mockReturnValue(false), } as any) } + // For symlink check + if (path.endsWith("symlink")) { + return Promise.resolve({ + isDirectory: jest.fn().mockReturnValue(false), + isFile: jest.fn().mockReturnValue(false), + isSymbolicLink: jest.fn().mockReturnValue(true), + } as any) + } + // For all files return Promise.resolve({ isFile: jest.fn().mockReturnValue(true), @@ -654,6 +688,12 @@ describe("Rules directory reading", () => { if (filePath.toString() === "/fake/path/.roo/rules/../symlink-target.txt") { return Promise.resolve("symlink target content") } + if (filePath.toString() === "/fake/path/.roo/rules/symlink-target-dir/subdir_link.txt") { + return Promise.resolve("regular file content under symlink target dir") + } + if (filePath.toString() === "/fake/path/.roo/rules/../nested-symlink-target.txt") { + return Promise.resolve("nested symlink target content") + } return Promise.reject({ code: "ENOENT" }) }) @@ -664,13 +704,20 @@ describe("Rules directory reading", () => { expect(result).toContain("regular file content") expect(result).toContain("# Rules from /fake/path/.roo/rules/../symlink-target.txt:") expect(result).toContain("symlink target content") + expect(result).toContain("# Rules from /fake/path/.roo/rules/symlink-target-dir/subdir_link.txt:") + expect(result).toContain("regular file content under symlink target dir") + expect(result).toContain("# Rules from /fake/path/.roo/rules/../nested-symlink-target.txt:") + expect(result).toContain("nested symlink target content") // Verify readlink was called with the symlink path expect(readlinkMock).toHaveBeenCalledWith("/fake/path/.roo/rules/link.txt") + expect(readlinkMock).toHaveBeenCalledWith("/fake/path/.roo/rules/link_dir") // Verify both files were read expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/regular.txt", "utf-8") expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/../symlink-target.txt", "utf-8") + expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/symlink-target-dir/subdir_link.txt", "utf-8") + expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/../nested-symlink-target.txt", "utf-8") }) beforeEach(() => { jest.clearAllMocks() diff --git a/src/core/prompts/sections/custom-instructions.ts b/src/core/prompts/sections/custom-instructions.ts index 22b846bf912..cf1aea24ff4 100644 --- a/src/core/prompts/sections/custom-instructions.ts +++ b/src/core/prompts/sections/custom-instructions.ts @@ -2,6 +2,7 @@ import fs from "fs/promises" import path from "path" import { LANGUAGES, isLanguage } from "../../../shared/language" +import { Dirent } from "fs" /** * Safely read a file and return its trimmed content @@ -31,6 +32,68 @@ async function directoryExists(dirPath: string): Promise { } } +const MAX_DEPTH = 5 + +/** + * Recursively resolve directory entries and collect file paths + */ +async function resolveDirectoryEntry( + entry: Dirent, + dirPath: string, + filePaths: string[], + depth: number, +): Promise { + // Avoid cyclic symlinks + if (depth > MAX_DEPTH) { + return + } + + const fullPath = path.resolve(entry.parentPath || dirPath, entry.name) + if (entry.isFile()) { + // Regular file + filePaths.push(fullPath) + } else if (entry.isSymbolicLink()) { + // Await the resolution of the symbolic link + await resolveSymLink(fullPath, filePaths, depth + 1) + } +} + +/** + * Recursively resolve a symbolic link and collect file paths + */ +async function resolveSymLink(fullPath: string, filePaths: string[], depth: number): Promise { + // Avoid cyclic symlinks + if (depth > MAX_DEPTH) { + return + } + try { + // Get the symlink target + const linkTarget = await fs.readlink(fullPath) + // Resolve the target path (relative to the symlink location) + const resolvedTarget = path.resolve(path.dirname(fullPath), linkTarget) + + // Check if the target is a file + const stats = await fs.stat(resolvedTarget) + if (stats.isFile()) { + filePaths.push(resolvedTarget) + } else if (stats.isDirectory()) { + const anotherEntries = await fs.readdir(resolvedTarget, { withFileTypes: true, recursive: true }) + // Collect promises for recursive calls within the directory + const directoryPromises: Promise[] = [] + for (const anotherEntry of anotherEntries) { + directoryPromises.push(resolveDirectoryEntry(anotherEntry, resolvedTarget, filePaths, depth + 1)) + } + // Wait for all entries in the resolved directory to be processed + await Promise.all(directoryPromises) + } else if (stats.isSymbolicLink()) { + // Handle nested symlinks by awaiting the recursive call + await resolveSymLink(resolvedTarget, filePaths, depth + 1) + } + } catch (err) { + // Skip invalid symlinks + } +} + /** * Read all text files from a directory in alphabetical order */ @@ -40,30 +103,16 @@ async function readTextFilesFromDirectory(dirPath: string): Promise[] = [] for (const entry of entries) { - const fullPath = path.resolve(entry.parentPath || dirPath, entry.name) - if (entry.isFile()) { - // Regular file - filePaths.push(fullPath) - } else if (entry.isSymbolicLink()) { - try { - // Get the symlink target - const linkTarget = await fs.readlink(fullPath) - // Resolve the target path (relative to the symlink location) - const resolvedTarget = path.resolve(path.dirname(fullPath), linkTarget) - - // Check if the target is a file - const stats = await fs.stat(resolvedTarget) - if (stats.isFile()) { - filePaths.push(resolvedTarget) - } - } catch (err) { - // Skip invalid symlinks - } - } + initialPromises.push(resolveDirectoryEntry(entry, dirPath, filePaths, 0)) } + // Wait for all asynchronous operations (including recursive ones) to complete + await Promise.all(initialPromises) + const fileContents = await Promise.all( filePaths.map(async (file) => { try {