Fix bug not to respect symbolic linked rules, if target is a directory or another symbolic link (#2513)

* read symbolic linked dir and files recursively

* add symlinked dir and nested symlink test case for custom-instructions

* enhance comments

* add changeset

.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

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()

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<boolean> {
 	}
 }
 
+const MAX_DEPTH = 5
+
+/**
+ * Recursively resolve directory entries and collect file paths
+ */
+async function resolveDirectoryEntry(
+	entry: Dirent,
+	dirPath: string,
+	filePaths: string[],
+	depth: number,
+): Promise<void> {
+	// 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<void> {
+	// 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<void>[] = []
+			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<Array<{ file
 
 		// Process all entries - regular files and symlinks that might point to files
 		const filePaths: string[] = []
+		// Collect promises for the initial resolution calls
+		const initialPromises: Promise<void>[] = []
 
 		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 {