Follow symlinks for rules files (#2421)

Author: mrubens

src/core/prompts/sections/__tests__/custom-instructions.test.ts

@@ -10,11 +10,13 @@ jest.mock("fs/promises")
 const readFileMock = jest.fn()
 const statMock = jest.fn()
 const readdirMock = jest.fn()
+const readlinkMock = jest.fn()
 
 // Replace fs functions with our mocks
 fs.readFile = readFileMock as any
 fs.stat = statMock as any
 fs.readdir = readdirMock as any
+fs.readlink = readlinkMock as any
 
 // Mock path.resolve and path.join to be predictable in tests
 jest.mock("path", () => ({
@@ -127,8 +129,8 @@ describe("loadRuleFiles", () => {
 
 		// Simulate listing files
 		readdirMock.mockResolvedValueOnce([
-			{ name: "file1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" },
-			{ name: "file2.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" },
+			{ name: "file1.txt", isFile: () => true, isSymbolicLink: () => false, parentPath: "/fake/path/.roo/rules" },
+			{ name: "file2.txt", isFile: () => true, isSymbolicLink: () => false, parentPath: "/fake/path/.roo/rules" },
 		] as any)
 
 		statMock.mockImplementation(
@@ -154,6 +156,8 @@ describe("loadRuleFiles", () => {
 		expect(result).toContain("# Rules from /fake/path/.roo/rules/file2.txt:")
 		expect(result).toContain("content of file2")
 
+		// We expect both checks because our new implementation checks the files again for validation
+		expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules")
 		expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file1.txt")
 		expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file2.txt")
 		expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file1.txt", "utf-8")
@@ -210,17 +214,31 @@ describe("loadRuleFiles", () => {
 
 		// Simulate listing files including subdirectories
 		readdirMock.mockResolvedValueOnce([
-			{ name: "subdir", isFile: () => false, isDirectory: () => true, parentPath: "/fake/path/.roo/rules" },
-			{ name: "root.txt", isFile: () => true, isDirectory: () => false, parentPath: "/fake/path/.roo/rules" },
+			{
+				name: "subdir",
+				isFile: () => false,
+				isSymbolicLink: () => false,
+				isDirectory: () => true,
+				parentPath: "/fake/path/.roo/rules",
+			},
+			{
+				name: "root.txt",
+				isFile: () => true,
+				isSymbolicLink: () => false,
+				isDirectory: () => false,
+				parentPath: "/fake/path/.roo/rules",
+			},
 			{
 				name: "nested1.txt",
 				isFile: () => true,
+				isSymbolicLink: () => false,
 				isDirectory: () => false,
 				parentPath: "/fake/path/.roo/rules/subdir",
 			},
 			{
 				name: "nested2.txt",
 				isFile: () => true,
+				isSymbolicLink: () => false,
 				isDirectory: () => false,
 				parentPath: "/fake/path/.roo/rules/subdir/subdir2",
 			},
@@ -395,8 +413,18 @@ describe("addCustomInstructions", () => {
 
 		// Simulate listing files
 		readdirMock.mockResolvedValueOnce([
-			{ name: "rule1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules-test-mode" },
-			{ name: "rule2.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules-test-mode" },
+			{
+				name: "rule1.txt",
+				isFile: () => true,
+				isSymbolicLink: () => false,
+				parentPath: "/fake/path/.roo/rules-test-mode",
+			},
+			{
+				name: "rule2.txt",
+				isFile: () => true,
+				isSymbolicLink: () => false,
+				parentPath: "/fake/path/.roo/rules-test-mode",
+			},
 		] as any)
 
 		statMock.mockImplementation(
@@ -430,6 +458,7 @@ describe("addCustomInstructions", () => {
 		expect(result).toContain("# Rules from /fake/path/.roo/rules-test-mode/rule2.txt:")
 		expect(result).toContain("mode specific rule 2")
 
+		expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules-test-mode")
 		expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules-test-mode/rule1.txt")
 		expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules-test-mode/rule2.txt")
 		expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules-test-mode/rule1.txt", "utf-8")
@@ -579,6 +608,70 @@ describe("Directory existence checks", () => {
 
 // Indirectly test readTextFilesFromDirectory and formatDirectoryContent through loadRuleFiles
 describe("Rules directory reading", () => {
+	it("should follow symbolic links in the rules directory", async () => {
+		// Simulate .roo/rules directory exists
+		statMock.mockResolvedValueOnce({
+			isDirectory: jest.fn().mockReturnValue(true),
+		} 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)
+
+		// Simulate readlink response
+		readlinkMock.mockResolvedValueOnce("../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") {
+				return Promise.resolve({
+					isDirectory: jest.fn().mockReturnValue(true),
+					isFile: jest.fn().mockReturnValue(false),
+				} as any)
+			}
+
+			// For all files
+			return Promise.resolve({
+				isFile: jest.fn().mockReturnValue(true),
+				isDirectory: jest.fn().mockReturnValue(false),
+			} as any)
+		})
+
+		// Simulate file content reading
+		readFileMock.mockImplementation((filePath: PathLike) => {
+			if (filePath.toString() === "/fake/path/.roo/rules/regular.txt") {
+				return Promise.resolve("regular file content")
+			}
+			if (filePath.toString() === "/fake/path/.roo/rules/../symlink-target.txt") {
+				return Promise.resolve("symlink target content")
+			}
+			return Promise.reject({ code: "ENOENT" })
+		})
+
+		const result = await loadRuleFiles("/fake/path")
+
+		// Verify both regular file and symlink target content are included
+		expect(result).toContain("# Rules from /fake/path/.roo/rules/regular.txt:")
+		expect(result).toContain("regular file content")
+		expect(result).toContain("# Rules from /fake/path/.roo/rules/../symlink-target.txt:")
+		expect(result).toContain("symlink target content")
+
+		// Verify readlink was called with the symlink path
+		expect(readlinkMock).toHaveBeenCalledWith("/fake/path/.roo/rules/link.txt")
+
+		// 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")
+	})
 	beforeEach(() => {
 		jest.clearAllMocks()
 	})

src/core/prompts/sections/custom-instructions.ts

@@ -36,13 +36,36 @@ async function directoryExists(dirPath: string): Promise<boolean> {
  */
 async function readTextFilesFromDirectory(dirPath: string): Promise<Array<{ filename: string; content: string }>> {
 	try {
-		const files = await fs
-			.readdir(dirPath, { withFileTypes: true, recursive: true })
-			.then((files) => files.filter((file) => file.isFile()))
-			.then((files) => files.map((file) => path.resolve(file.parentPath, file.name)))
+		const entries = await fs.readdir(dirPath, { withFileTypes: true, recursive: true })
+
+		// Process all entries - regular files and symlinks that might point to files
+		const filePaths: string[] = []
+
+		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
+				}
+			}
+		}
 
 		const fileContents = await Promise.all(
-			files.map(async (file) => {
+			filePaths.map(async (file) => {
 				try {
 					// Check if it's a file (not a directory)
 					const stats = await fs.stat(file)