From 7c16f25d77152a5ea3195724e41f47b2024ef9ae Mon Sep 17 00:00:00 2001 From: Roo Code Date: Thu, 21 Aug 2025 14:41:47 +0000 Subject: [PATCH 1/2] feat: add support for symbolic links in .roo/commands directory - Modified scanCommandDirectory to check for both regular files and symbolic links - Added comprehensive test coverage for symbolic link functionality - Ensures symbolic links to markdown files are properly loaded as commands Fixes #7282 --- .../__tests__/symlink-commands.spec.ts | 215 ++++++++++++++++++ src/services/command/commands.ts | 3 +- 2 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 src/services/command/__tests__/symlink-commands.spec.ts diff --git a/src/services/command/__tests__/symlink-commands.spec.ts b/src/services/command/__tests__/symlink-commands.spec.ts new file mode 100644 index 0000000000..0ab6e460b7 --- /dev/null +++ b/src/services/command/__tests__/symlink-commands.spec.ts @@ -0,0 +1,215 @@ +import { describe, it, expect, beforeEach, vi } from "vitest" +import fs from "fs/promises" +import * as path from "path" +import { getCommands, getCommand } from "../commands" + +// Mock fs and path modules +vi.mock("fs/promises") +vi.mock("../../roo-config", () => ({ + getGlobalRooDirectory: vi.fn(() => "/mock/global"), + getProjectRooDirectoryForCwd: vi.fn(() => "/mock/project"), +})) + +const mockFs = vi.mocked(fs) + +describe("Symbolic link support for commands", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + describe("getCommands with symbolic links", () => { + it("should load commands from symbolic links", async () => { + const setupContent = `--- +description: Sets up the development environment +--- + +# Setup Command + +Setup instructions.` + + const deployContent = `--- +description: Deploys the application +--- + +# Deploy Command + +Deploy instructions.` + + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.readdir = vi.fn().mockResolvedValue([ + { name: "setup.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link + { name: "deploy.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file + { name: "build.md", isFile: () => false, isSymbolicLink: () => true }, // Another symbolic link + ]) + mockFs.readFile = vi + .fn() + .mockResolvedValueOnce(setupContent) + .mockResolvedValueOnce(deployContent) + .mockResolvedValueOnce("# Build Command\n\nBuild instructions.") + + const result = await getCommands("/test/cwd") + + expect(result).toHaveLength(3) + expect(result).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: "setup", + description: "Sets up the development environment", + }), + expect.objectContaining({ + name: "deploy", + description: "Deploys the application", + }), + expect.objectContaining({ + name: "build", + description: undefined, + }), + ]), + ) + }) + + it("should handle mix of regular files and symbolic links", async () => { + const testContent = `--- +description: Test command +argument-hint: test | debug +--- + +# Test Command + +Test content.` + + const linkContent = `--- +description: Linked command +--- + +# Linked Command + +This command is accessed via symbolic link.` + + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.readdir = vi.fn().mockResolvedValue([ + { name: "test.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file + { name: "linked.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link + { name: "not-markdown.txt", isFile: () => true, isSymbolicLink: () => false }, // Should be ignored + { name: "symlink.txt", isFile: () => false, isSymbolicLink: () => true }, // Non-markdown symlink, should be ignored + ]) + mockFs.readFile = vi.fn().mockResolvedValueOnce(testContent).mockResolvedValueOnce(linkContent) + + const result = await getCommands("/test/cwd") + + expect(result).toHaveLength(2) + expect(result).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + name: "test", + description: "Test command", + argumentHint: "test | debug", + }), + expect.objectContaining({ + name: "linked", + description: "Linked command", + }), + ]), + ) + }) + + it("should handle broken symbolic links gracefully", async () => { + const validContent = `# Valid Command + +This is a valid command.` + + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.readdir = vi.fn().mockResolvedValue([ + { name: "valid.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file + { name: "broken-link.md", isFile: () => false, isSymbolicLink: () => true }, // Broken symbolic link + ]) + mockFs.readFile = vi + .fn() + .mockResolvedValueOnce(validContent) + .mockRejectedValueOnce(new Error("ENOENT: no such file or directory")) // Broken link + + const result = await getCommands("/test/cwd") + + // Should only load the valid command, ignoring the broken link + expect(result).toHaveLength(1) + expect(result[0]).toEqual( + expect.objectContaining({ + name: "valid", + }), + ) + }) + + it("should prioritize project symbolic links over global commands", async () => { + const projectSymlinkContent = `--- +description: Project symbolic link command +--- + +# Project Symlink + +Project-specific command via symlink.` + + const globalContent = `--- +description: Global command +--- + +# Global Command + +Global command content.` + + // Mock both directories + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + + // First call for global directory scan, second for project directory scan + mockFs.readdir = vi + .fn() + .mockResolvedValueOnce([ + { name: "override.md", isFile: () => true, isSymbolicLink: () => false }, // Global regular file + ]) + .mockResolvedValueOnce([ + { name: "override.md", isFile: () => false, isSymbolicLink: () => true }, // Project symlink + ]) + + // First read is for global file, second is for project symlink + mockFs.readFile = vi.fn().mockResolvedValueOnce(globalContent).mockResolvedValueOnce(projectSymlinkContent) + + const result = await getCommands("/test/cwd") + + // Project symbolic link should override global command + expect(result).toHaveLength(1) + expect(result[0]).toEqual( + expect.objectContaining({ + name: "override", + description: "Project symbolic link command", + source: "project", + }), + ) + }) + }) + + describe("getCommand with symbolic links", () => { + it("should load a command from a symbolic link", async () => { + const commandContent = `--- +description: Command accessed via symbolic link +argument-hint: option1 | option2 +--- + +# Symlinked Command + +This command is loaded from a symbolic link.` + + mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.readFile = vi.fn().mockResolvedValue(commandContent) + + const result = await getCommand("/test/cwd", "symlinked") + + expect(result).toEqual({ + name: "symlinked", + content: "# Symlinked Command\n\nThis command is loaded from a symbolic link.", + source: "project", + filePath: path.join("/mock/project", "commands", "symlinked.md"), + description: "Command accessed via symbolic link", + argumentHint: "option1 | option2", + }) + }) + }) +}) diff --git a/src/services/command/commands.ts b/src/services/command/commands.ts index 452269511c..a4060c0ac9 100644 --- a/src/services/command/commands.ts +++ b/src/services/command/commands.ts @@ -136,7 +136,8 @@ async function scanCommandDirectory( const entries = await fs.readdir(dirPath, { withFileTypes: true }) for (const entry of entries) { - if (entry.isFile() && isMarkdownFile(entry.name)) { + // Check for both regular files and symbolic links + if ((entry.isFile() || entry.isSymbolicLink()) && isMarkdownFile(entry.name)) { const filePath = path.join(dirPath, entry.name) const commandName = getCommandNameFromFile(entry.name) From 90e983e8041b67b0b49b2cf48a7e6512758ff761 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 25 Aug 2025 14:52:13 -0500 Subject: [PATCH 2/2] fix: properly handle symbolic links to directories - Check if symbolic links point to files before processing - Prevent attempting to read directories as files - Add test coverage for symlinks pointing to directories --- .../__tests__/symlink-commands.spec.ts | 66 ++++++++++++++++--- src/services/command/commands.ts | 23 ++++++- 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/services/command/__tests__/symlink-commands.spec.ts b/src/services/command/__tests__/symlink-commands.spec.ts index 0ab6e460b7..d501d315af 100644 --- a/src/services/command/__tests__/symlink-commands.spec.ts +++ b/src/services/command/__tests__/symlink-commands.spec.ts @@ -35,7 +35,14 @@ description: Deploys the application Deploy instructions.` - mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.stat = vi + .fn() + // First call for directory check + .mockResolvedValueOnce({ isDirectory: () => true } as any) + // Subsequent calls for symlink target checks + .mockResolvedValueOnce({ isFile: () => true, isDirectory: () => false } as any) // setup.md symlink points to file + .mockResolvedValueOnce({ isFile: () => true, isDirectory: () => false } as any) // build.md symlink points to file + mockFs.readdir = vi.fn().mockResolvedValue([ { name: "setup.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link { name: "deploy.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file @@ -86,10 +93,17 @@ description: Linked command This command is accessed via symbolic link.` - mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + // Mock directory stat + mockFs.stat = vi + .fn() + // First call for directory check + .mockResolvedValueOnce({ isDirectory: () => true } as any) + // Second call for symlink target check (linked.md points to a file) + .mockResolvedValueOnce({ isFile: () => true, isDirectory: () => false } as any) + mockFs.readdir = vi.fn().mockResolvedValue([ { name: "test.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file - { name: "linked.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link + { name: "linked.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link to file { name: "not-markdown.txt", isFile: () => true, isSymbolicLink: () => false }, // Should be ignored { name: "symlink.txt", isFile: () => false, isSymbolicLink: () => true }, // Non-markdown symlink, should be ignored ]) @@ -118,15 +132,16 @@ This command is accessed via symbolic link.` This is a valid command.` - mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.stat = vi + .fn() + .mockResolvedValueOnce({ isDirectory: () => true } as any) // Directory check + .mockRejectedValueOnce(new Error("ENOENT: no such file or directory")) // Broken symlink + mockFs.readdir = vi.fn().mockResolvedValue([ { name: "valid.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file { name: "broken-link.md", isFile: () => false, isSymbolicLink: () => true }, // Broken symbolic link ]) - mockFs.readFile = vi - .fn() - .mockResolvedValueOnce(validContent) - .mockRejectedValueOnce(new Error("ENOENT: no such file or directory")) // Broken link + mockFs.readFile = vi.fn().mockResolvedValueOnce(validContent) const result = await getCommands("/test/cwd") @@ -139,6 +154,35 @@ This is a valid command.` ) }) + it("should ignore symbolic links pointing to directories", async () => { + const validContent = `# Valid Command + +This is a valid command.` + + mockFs.stat = vi + .fn() + .mockResolvedValueOnce({ isDirectory: () => true } as any) // Commands directory check + .mockResolvedValueOnce({ isFile: () => false, isDirectory: () => true } as any) // dir-link.md points to a directory + + mockFs.readdir = vi.fn().mockResolvedValue([ + { name: "valid.md", isFile: () => true, isSymbolicLink: () => false }, // Regular file + { name: "dir-link.md", isFile: () => false, isSymbolicLink: () => true }, // Symbolic link to directory + ]) + mockFs.readFile = vi.fn().mockResolvedValueOnce(validContent) + + const result = await getCommands("/test/cwd") + + // Should only load the valid command, ignoring the symlink to directory + expect(result).toHaveLength(1) + expect(result[0]).toEqual( + expect.objectContaining({ + name: "valid", + }), + ) + // readFile should only be called once (for the valid file) + expect(mockFs.readFile).toHaveBeenCalledTimes(1) + }) + it("should prioritize project symbolic links over global commands", async () => { const projectSymlinkContent = `--- description: Project symbolic link command @@ -157,7 +201,11 @@ description: Global command Global command content.` // Mock both directories - mockFs.stat = vi.fn().mockResolvedValue({ isDirectory: () => true }) + mockFs.stat = vi + .fn() + .mockResolvedValueOnce({ isDirectory: () => true } as any) // Global directory + .mockResolvedValueOnce({ isDirectory: () => true } as any) // Project directory + .mockResolvedValueOnce({ isFile: () => true, isDirectory: () => false } as any) // Project symlink target // First call for global directory scan, second for project directory scan mockFs.readdir = vi diff --git a/src/services/command/commands.ts b/src/services/command/commands.ts index a4060c0ac9..0d20bfb5cc 100644 --- a/src/services/command/commands.ts +++ b/src/services/command/commands.ts @@ -136,9 +136,28 @@ async function scanCommandDirectory( const entries = await fs.readdir(dirPath, { withFileTypes: true }) for (const entry of entries) { - // Check for both regular files and symbolic links - if ((entry.isFile() || entry.isSymbolicLink()) && isMarkdownFile(entry.name)) { + // Only process markdown files + if (isMarkdownFile(entry.name)) { const filePath = path.join(dirPath, entry.name) + + // Check if entry is a valid file or a symbolic link pointing to a file + let isValidFile = false + if (entry.isFile()) { + isValidFile = true + } else if (entry.isSymbolicLink()) { + try { + const stats = await fs.stat(filePath) // follows the symlink + isValidFile = stats.isFile() + } catch { + // Broken symlink or points to non-existent target + isValidFile = false + } + } + + if (!isValidFile) { + continue + } + const commandName = getCommandNameFromFile(entry.name) try {