From abdbb45202e6ad6310cf4bd2843bce95c741a66d Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 28 Jul 2025 23:47:40 +0000 Subject: [PATCH 1/4] feat: add symlink support for AGENTS.md file loading - Add safeReadFileFollowingSymlinks function to handle symlink resolution - Update loadAgentRulesFile to use the new symlink-aware function - Add comprehensive tests for both symlink and regular file scenarios - Ensures AGENTS.md can be a symlink pointing to actual rules file --- .../__tests__/custom-instructions.spec.ts | 135 ++++++++++++++++++ .../prompts/sections/custom-instructions.ts | 30 +++- 2 files changed, 164 insertions(+), 1 deletion(-) diff --git a/src/core/prompts/sections/__tests__/custom-instructions.spec.ts b/src/core/prompts/sections/__tests__/custom-instructions.spec.ts index 01574406b2d..041b8313c72 100644 --- a/src/core/prompts/sections/__tests__/custom-instructions.spec.ts +++ b/src/core/prompts/sections/__tests__/custom-instructions.spec.ts @@ -54,12 +54,14 @@ const readFileMock = vi.fn() const statMock = vi.fn() const readdirMock = vi.fn() const readlinkMock = vi.fn() +const lstatMock = vi.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 +fs.lstat = lstatMock as any // Mock process.cwd const originalCwd = process.cwd @@ -509,6 +511,17 @@ describe("addCustomInstructions", () => { // Simulate no .roo/rules-test-mode directory statMock.mockRejectedValueOnce({ code: "ENOENT" }) + // Mock lstat to indicate AGENTS.md is NOT a symlink + lstatMock.mockImplementation((filePath: PathLike) => { + const pathStr = filePath.toString() + if (pathStr.endsWith("AGENTS.md")) { + return Promise.resolve({ + isSymbolicLink: vi.fn().mockReturnValue(false), + }) + } + return Promise.reject({ code: "ENOENT" }) + }) + readFileMock.mockImplementation((filePath: PathLike) => { const pathStr = filePath.toString() if (pathStr.endsWith("AGENTS.md")) { @@ -558,6 +571,17 @@ describe("addCustomInstructions", () => { // Simulate no .roo/rules-test-mode directory statMock.mockRejectedValueOnce({ code: "ENOENT" }) + // Mock lstat to indicate AGENTS.md is NOT a symlink + lstatMock.mockImplementation((filePath: PathLike) => { + const pathStr = filePath.toString() + if (pathStr.endsWith("AGENTS.md")) { + return Promise.resolve({ + isSymbolicLink: vi.fn().mockReturnValue(false), + }) + } + return Promise.reject({ code: "ENOENT" }) + }) + readFileMock.mockImplementation((filePath: PathLike) => { const pathStr = filePath.toString() if (pathStr.endsWith("AGENTS.md")) { @@ -602,6 +626,17 @@ describe("addCustomInstructions", () => { // Simulate no .roo/rules-test-mode directory statMock.mockRejectedValueOnce({ code: "ENOENT" }) + // Mock lstat to indicate AGENTS.md is NOT a symlink + lstatMock.mockImplementation((filePath: PathLike) => { + const pathStr = filePath.toString() + if (pathStr.endsWith("AGENTS.md")) { + return Promise.resolve({ + isSymbolicLink: vi.fn().mockReturnValue(false), + }) + } + return Promise.reject({ code: "ENOENT" }) + }) + readFileMock.mockImplementation((filePath: PathLike) => { const pathStr = filePath.toString() if (pathStr.endsWith("AGENTS.md")) { @@ -628,6 +663,106 @@ describe("addCustomInstructions", () => { expect(result).toContain("Roo rules content") }) + it("should follow symlinks when loading AGENTS.md", async () => { + // Simulate no .roo/rules-test-mode directory + statMock.mockRejectedValueOnce({ code: "ENOENT" }) + + // Mock lstat to indicate AGENTS.md is a symlink + lstatMock.mockImplementation((filePath: PathLike) => { + const pathStr = filePath.toString() + if (pathStr.endsWith("AGENTS.md")) { + return Promise.resolve({ + isSymbolicLink: vi.fn().mockReturnValue(true), + }) + } + return Promise.reject({ code: "ENOENT" }) + }) + + // Mock readlink to return the symlink target + readlinkMock.mockImplementation((filePath: PathLike) => { + const pathStr = filePath.toString() + if (pathStr.endsWith("AGENTS.md")) { + return Promise.resolve("../actual-agents-file.md") + } + return Promise.reject({ code: "ENOENT" }) + }) + + // Mock readFile to return content from the resolved path + readFileMock.mockImplementation((filePath: PathLike) => { + const pathStr = filePath.toString() + const normalizedPath = pathStr.replace(/\\/g, "/") + if (normalizedPath.endsWith("actual-agents-file.md")) { + return Promise.resolve("Agent rules from symlinked file") + } + return Promise.reject({ code: "ENOENT" }) + }) + + const result = await addCustomInstructions( + "mode instructions", + "global instructions", + "/fake/path", + "test-mode", + { settings: { maxConcurrentFileReads: 5, todoListEnabled: true, useAgentRules: true } }, + ) + + expect(result).toContain("# Agent Rules Standard (AGENTS.md):") + expect(result).toContain("Agent rules from symlinked file") + + // Verify lstat was called to check if it's a symlink + expect(lstatMock).toHaveBeenCalledWith(expect.stringContaining("AGENTS.md")) + + // Verify readlink was called to resolve the symlink + expect(readlinkMock).toHaveBeenCalledWith(expect.stringContaining("AGENTS.md")) + + // Verify the resolved path was read + expect(readFileMock).toHaveBeenCalledWith(expect.stringContaining("actual-agents-file.md"), "utf-8") + }) + + it("should handle AGENTS.md as a regular file when not a symlink", async () => { + // Simulate no .roo/rules-test-mode directory + statMock.mockRejectedValueOnce({ code: "ENOENT" }) + + // Mock lstat to indicate AGENTS.md is NOT a symlink + lstatMock.mockImplementation((filePath: PathLike) => { + const pathStr = filePath.toString() + if (pathStr.endsWith("AGENTS.md")) { + return Promise.resolve({ + isSymbolicLink: vi.fn().mockReturnValue(false), + }) + } + return Promise.reject({ code: "ENOENT" }) + }) + + // Mock readFile to return content directly from AGENTS.md + readFileMock.mockImplementation((filePath: PathLike) => { + const pathStr = filePath.toString() + if (pathStr.endsWith("AGENTS.md")) { + return Promise.resolve("Agent rules from regular file") + } + return Promise.reject({ code: "ENOENT" }) + }) + + const result = await addCustomInstructions( + "mode instructions", + "global instructions", + "/fake/path", + "test-mode", + { settings: { maxConcurrentFileReads: 5, todoListEnabled: true, useAgentRules: true } }, + ) + + expect(result).toContain("# Agent Rules Standard (AGENTS.md):") + expect(result).toContain("Agent rules from regular file") + + // Verify lstat was called + expect(lstatMock).toHaveBeenCalledWith(expect.stringContaining("AGENTS.md")) + + // Verify readlink was NOT called since it's not a symlink + expect(readlinkMock).not.toHaveBeenCalledWith(expect.stringContaining("AGENTS.md")) + + // Verify the file was read directly + expect(readFileMock).toHaveBeenCalledWith(expect.stringContaining("AGENTS.md"), "utf-8") + }) + it("should return empty string when no instructions provided", async () => { // Simulate no .roo/rules directory statMock.mockRejectedValueOnce({ code: "ENOENT" }) diff --git a/src/core/prompts/sections/custom-instructions.ts b/src/core/prompts/sections/custom-instructions.ts index ccd36b26627..6628d31e800 100644 --- a/src/core/prompts/sections/custom-instructions.ts +++ b/src/core/prompts/sections/custom-instructions.ts @@ -26,6 +26,34 @@ async function safeReadFile(filePath: string): Promise { } } +/** + * Safely read a file and follow symlinks if necessary + */ +async function safeReadFileFollowingSymlinks(filePath: string): Promise { + try { + // Check if the path is a symlink + const stats = await fs.lstat(filePath) + if (stats.isSymbolicLink()) { + // Resolve the symlink to get the actual file path + const linkTarget = await fs.readlink(filePath) + const resolvedPath = path.resolve(path.dirname(filePath), linkTarget) + // Read from the resolved path + const content = await fs.readFile(resolvedPath, "utf-8") + return content.trim() + } else { + // Not a symlink, read normally + const content = await fs.readFile(filePath, "utf-8") + return content.trim() + } + } catch (err) { + const errorCode = (err as NodeJS.ErrnoException).code + if (!errorCode || !["ENOENT", "EISDIR"].includes(errorCode)) { + throw err + } + return "" + } +} + /** * Check if a directory exists */ @@ -222,7 +250,7 @@ export async function loadRuleFiles(cwd: string): Promise { async function loadAgentRulesFile(cwd: string): Promise { try { const agentsPath = path.join(cwd, "AGENTS.md") - const content = await safeReadFile(agentsPath) + const content = await safeReadFileFollowingSymlinks(agentsPath) if (content) { return `# Agent Rules Standard (AGENTS.md):\n${content}` } From 8bdf36ab16207655fd231d28f1b747853b788a6b Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 28 Jul 2025 23:56:43 +0000 Subject: [PATCH 2/4] refactor: use existing symlink resolution pattern for AGENTS.md - Extracted resolveSymlinkPath function to handle symlink resolution - Removed duplicate safeReadFileFollowingSymlinks function - Updated loadAgentRulesFile to use resolveSymlinkPath + safeReadFile - Updated tests to match new implementation - Maintains same functionality while reusing existing patterns --- .../__tests__/custom-instructions.spec.ts | 5 ++ .../prompts/sections/custom-instructions.ts | 51 +++++++++++-------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/core/prompts/sections/__tests__/custom-instructions.spec.ts b/src/core/prompts/sections/__tests__/custom-instructions.spec.ts index 041b8313c72..cc85f0a446c 100644 --- a/src/core/prompts/sections/__tests__/custom-instructions.spec.ts +++ b/src/core/prompts/sections/__tests__/custom-instructions.spec.ts @@ -674,6 +674,11 @@ describe("addCustomInstructions", () => { return Promise.resolve({ isSymbolicLink: vi.fn().mockReturnValue(true), }) + } else if (pathStr.endsWith("actual-agents-file.md")) { + // The resolved target is not a symlink + return Promise.resolve({ + isSymbolicLink: vi.fn().mockReturnValue(false), + }) } return Promise.reject({ code: "ENOENT" }) }) diff --git a/src/core/prompts/sections/custom-instructions.ts b/src/core/prompts/sections/custom-instructions.ts index 6628d31e800..ab07bdf4740 100644 --- a/src/core/prompts/sections/custom-instructions.ts +++ b/src/core/prompts/sections/custom-instructions.ts @@ -27,30 +27,38 @@ async function safeReadFile(filePath: string): Promise { } /** - * Safely read a file and follow symlinks if necessary + * Resolve a symlink to its target path */ -async function safeReadFileFollowingSymlinks(filePath: string): Promise { +async function resolveSymlinkPath(symlinkPath: string, depth: number = 0): Promise { + // Avoid cyclic symlinks + if (depth > MAX_DEPTH) { + throw new Error(`Maximum symlink depth exceeded for ${symlinkPath}`) + } + try { // Check if the path is a symlink - const stats = await fs.lstat(filePath) - if (stats.isSymbolicLink()) { - // Resolve the symlink to get the actual file path - const linkTarget = await fs.readlink(filePath) - const resolvedPath = path.resolve(path.dirname(filePath), linkTarget) - // Read from the resolved path - const content = await fs.readFile(resolvedPath, "utf-8") - return content.trim() - } else { - // Not a symlink, read normally - const content = await fs.readFile(filePath, "utf-8") - return content.trim() + const stats = await fs.lstat(symlinkPath) + if (!stats.isSymbolicLink()) { + // Not a symlink, return the original path + return symlinkPath } - } catch (err) { - const errorCode = (err as NodeJS.ErrnoException).code - if (!errorCode || !["ENOENT", "EISDIR"].includes(errorCode)) { - throw err + + // Get the symlink target + const linkTarget = await fs.readlink(symlinkPath) + // Resolve the target path (relative to the symlink location) + const resolvedTarget = path.resolve(path.dirname(symlinkPath), linkTarget) + + // Check if the resolved target is also a symlink (handle nested symlinks) + const targetStats = await fs.lstat(resolvedTarget) + if (targetStats.isSymbolicLink()) { + // Recursively resolve nested symlinks + return resolveSymlinkPath(resolvedTarget, depth + 1) } - return "" + + return resolvedTarget + } catch (err) { + // If we can't resolve the symlink, return the original path + return symlinkPath } } @@ -250,7 +258,10 @@ export async function loadRuleFiles(cwd: string): Promise { async function loadAgentRulesFile(cwd: string): Promise { try { const agentsPath = path.join(cwd, "AGENTS.md") - const content = await safeReadFileFollowingSymlinks(agentsPath) + // Resolve symlinks if necessary + const resolvedPath = await resolveSymlinkPath(agentsPath) + // Read the file content + const content = await safeReadFile(resolvedPath) if (content) { return `# Agent Rules Standard (AGENTS.md):\n${content}` } From 3ad7a2d942776b3f6d7a5327ccb881059ea5d14d Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 30 Jul 2025 13:50:02 +0000 Subject: [PATCH 3/4] fix: simplify symlink resolution for AGENTS.md to fix Windows compatibility - Remove duplicate resolveSymlinkPath function as suggested by @mrubens - Use simpler inline symlink resolution in loadAgentRulesFile - Update tests to match simplified implementation - This should fix the failing Windows unit tests while maintaining functionality --- .../__tests__/custom-instructions.spec.ts | 5 -- .../prompts/sections/custom-instructions.ts | 52 +++++-------------- 2 files changed, 14 insertions(+), 43 deletions(-) diff --git a/src/core/prompts/sections/__tests__/custom-instructions.spec.ts b/src/core/prompts/sections/__tests__/custom-instructions.spec.ts index cc85f0a446c..041b8313c72 100644 --- a/src/core/prompts/sections/__tests__/custom-instructions.spec.ts +++ b/src/core/prompts/sections/__tests__/custom-instructions.spec.ts @@ -674,11 +674,6 @@ describe("addCustomInstructions", () => { return Promise.resolve({ isSymbolicLink: vi.fn().mockReturnValue(true), }) - } else if (pathStr.endsWith("actual-agents-file.md")) { - // The resolved target is not a symlink - return Promise.resolve({ - isSymbolicLink: vi.fn().mockReturnValue(false), - }) } return Promise.reject({ code: "ENOENT" }) }) diff --git a/src/core/prompts/sections/custom-instructions.ts b/src/core/prompts/sections/custom-instructions.ts index ab07bdf4740..1b6647fe28c 100644 --- a/src/core/prompts/sections/custom-instructions.ts +++ b/src/core/prompts/sections/custom-instructions.ts @@ -26,42 +26,6 @@ async function safeReadFile(filePath: string): Promise { } } -/** - * Resolve a symlink to its target path - */ -async function resolveSymlinkPath(symlinkPath: string, depth: number = 0): Promise { - // Avoid cyclic symlinks - if (depth > MAX_DEPTH) { - throw new Error(`Maximum symlink depth exceeded for ${symlinkPath}`) - } - - try { - // Check if the path is a symlink - const stats = await fs.lstat(symlinkPath) - if (!stats.isSymbolicLink()) { - // Not a symlink, return the original path - return symlinkPath - } - - // Get the symlink target - const linkTarget = await fs.readlink(symlinkPath) - // Resolve the target path (relative to the symlink location) - const resolvedTarget = path.resolve(path.dirname(symlinkPath), linkTarget) - - // Check if the resolved target is also a symlink (handle nested symlinks) - const targetStats = await fs.lstat(resolvedTarget) - if (targetStats.isSymbolicLink()) { - // Recursively resolve nested symlinks - return resolveSymlinkPath(resolvedTarget, depth + 1) - } - - return resolvedTarget - } catch (err) { - // If we can't resolve the symlink, return the original path - return symlinkPath - } -} - /** * Check if a directory exists */ @@ -258,8 +222,20 @@ export async function loadRuleFiles(cwd: string): Promise { async function loadAgentRulesFile(cwd: string): Promise { try { const agentsPath = path.join(cwd, "AGENTS.md") - // Resolve symlinks if necessary - const resolvedPath = await resolveSymlinkPath(agentsPath) + let resolvedPath = agentsPath + + // Check if the file is a symlink and resolve it if necessary + try { + const stats = await fs.lstat(agentsPath) + if (stats.isSymbolicLink()) { + const linkTarget = await fs.readlink(agentsPath) + resolvedPath = path.resolve(path.dirname(agentsPath), linkTarget) + } + } catch (err) { + // If we can't check symlink status, just use the original path + resolvedPath = agentsPath + } + // Read the file content const content = await safeReadFile(resolvedPath) if (content) { From b1b3ed3db384c672c62d5861b356ac4412e9f25b Mon Sep 17 00:00:00 2001 From: Daniel Riccio Date: Wed, 30 Jul 2025 17:41:46 -0500 Subject: [PATCH 4/4] refactor: use existing resolveSymLink function for AGENTS.md symlink support - Remove duplicate inline symlink resolution logic - Reuse existing resolveSymLink function with MAX_DEPTH protection - Adapt loadAgentRulesFile to work with resolveSymLink's fileInfo interface - Fix test to properly mock fs.stat for resolved symlink targets - All tests pass (36/36) --- .../__tests__/custom-instructions.spec.ts | 12 +++++++++++ .../prompts/sections/custom-instructions.ts | 20 +++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/core/prompts/sections/__tests__/custom-instructions.spec.ts b/src/core/prompts/sections/__tests__/custom-instructions.spec.ts index 041b8313c72..d0157483155 100644 --- a/src/core/prompts/sections/__tests__/custom-instructions.spec.ts +++ b/src/core/prompts/sections/__tests__/custom-instructions.spec.ts @@ -687,6 +687,18 @@ describe("addCustomInstructions", () => { return Promise.reject({ code: "ENOENT" }) }) + // Mock stat to indicate the resolved target is a file + statMock.mockImplementation((filePath: PathLike) => { + const pathStr = filePath.toString() + const normalizedPath = pathStr.replace(/\\/g, "/") + if (normalizedPath.endsWith("actual-agents-file.md")) { + return Promise.resolve({ + isFile: vi.fn().mockReturnValue(true), + }) + } + return Promise.reject({ code: "ENOENT" }) + }) + // Mock readFile to return content from the resolved path readFileMock.mockImplementation((filePath: PathLike) => { const pathStr = filePath.toString() diff --git a/src/core/prompts/sections/custom-instructions.ts b/src/core/prompts/sections/custom-instructions.ts index 1b6647fe28c..22fb6122ec2 100644 --- a/src/core/prompts/sections/custom-instructions.ts +++ b/src/core/prompts/sections/custom-instructions.ts @@ -224,19 +224,27 @@ async function loadAgentRulesFile(cwd: string): Promise { const agentsPath = path.join(cwd, "AGENTS.md") let resolvedPath = agentsPath - // Check if the file is a symlink and resolve it if necessary + // Check if AGENTS.md exists and handle symlinks try { const stats = await fs.lstat(agentsPath) if (stats.isSymbolicLink()) { - const linkTarget = await fs.readlink(agentsPath) - resolvedPath = path.resolve(path.dirname(agentsPath), linkTarget) + // Create a temporary fileInfo array to use with resolveSymLink + const fileInfo: Array<{ originalPath: string; resolvedPath: string }> = [] + + // Use the existing resolveSymLink function to handle symlink resolution + await resolveSymLink(agentsPath, fileInfo, 0) + + // Extract the resolved path from fileInfo + if (fileInfo.length > 0) { + resolvedPath = fileInfo[0].resolvedPath + } } } catch (err) { - // If we can't check symlink status, just use the original path - resolvedPath = agentsPath + // If lstat fails (file doesn't exist), return empty + return "" } - // Read the file content + // Read the content from the resolved path const content = await safeReadFile(resolvedPath) if (content) { return `# Agent Rules Standard (AGENTS.md):\n${content}`