mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
Follow symlinks for rules files (#2421)
This commit is contained in:
parent
b01615f122
commit
c2fef2d01f
2 changed files with 127 additions and 11 deletions
|
|
@ -10,11 +10,13 @@ jest.mock("fs/promises")
|
||||||
const readFileMock = jest.fn()
|
const readFileMock = jest.fn()
|
||||||
const statMock = jest.fn()
|
const statMock = jest.fn()
|
||||||
const readdirMock = jest.fn()
|
const readdirMock = jest.fn()
|
||||||
|
const readlinkMock = jest.fn()
|
||||||
|
|
||||||
// Replace fs functions with our mocks
|
// Replace fs functions with our mocks
|
||||||
fs.readFile = readFileMock as any
|
fs.readFile = readFileMock as any
|
||||||
fs.stat = statMock as any
|
fs.stat = statMock as any
|
||||||
fs.readdir = readdirMock as any
|
fs.readdir = readdirMock as any
|
||||||
|
fs.readlink = readlinkMock as any
|
||||||
|
|
||||||
// Mock path.resolve and path.join to be predictable in tests
|
// Mock path.resolve and path.join to be predictable in tests
|
||||||
jest.mock("path", () => ({
|
jest.mock("path", () => ({
|
||||||
|
|
@ -127,8 +129,8 @@ describe("loadRuleFiles", () => {
|
||||||
|
|
||||||
// Simulate listing files
|
// Simulate listing files
|
||||||
readdirMock.mockResolvedValueOnce([
|
readdirMock.mockResolvedValueOnce([
|
||||||
{ name: "file1.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, parentPath: "/fake/path/.roo/rules" },
|
{ name: "file2.txt", isFile: () => true, isSymbolicLink: () => false, parentPath: "/fake/path/.roo/rules" },
|
||||||
] as any)
|
] as any)
|
||||||
|
|
||||||
statMock.mockImplementation(
|
statMock.mockImplementation(
|
||||||
|
|
@ -154,6 +156,8 @@ describe("loadRuleFiles", () => {
|
||||||
expect(result).toContain("# Rules from /fake/path/.roo/rules/file2.txt:")
|
expect(result).toContain("# Rules from /fake/path/.roo/rules/file2.txt:")
|
||||||
expect(result).toContain("content of file2")
|
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/file1.txt")
|
||||||
expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file2.txt")
|
expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file2.txt")
|
||||||
expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file1.txt", "utf-8")
|
expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file1.txt", "utf-8")
|
||||||
|
|
@ -210,17 +214,31 @@ describe("loadRuleFiles", () => {
|
||||||
|
|
||||||
// Simulate listing files including subdirectories
|
// Simulate listing files including subdirectories
|
||||||
readdirMock.mockResolvedValueOnce([
|
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",
|
name: "nested1.txt",
|
||||||
isFile: () => true,
|
isFile: () => true,
|
||||||
|
isSymbolicLink: () => false,
|
||||||
isDirectory: () => false,
|
isDirectory: () => false,
|
||||||
parentPath: "/fake/path/.roo/rules/subdir",
|
parentPath: "/fake/path/.roo/rules/subdir",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "nested2.txt",
|
name: "nested2.txt",
|
||||||
isFile: () => true,
|
isFile: () => true,
|
||||||
|
isSymbolicLink: () => false,
|
||||||
isDirectory: () => false,
|
isDirectory: () => false,
|
||||||
parentPath: "/fake/path/.roo/rules/subdir/subdir2",
|
parentPath: "/fake/path/.roo/rules/subdir/subdir2",
|
||||||
},
|
},
|
||||||
|
|
@ -395,8 +413,18 @@ describe("addCustomInstructions", () => {
|
||||||
|
|
||||||
// Simulate listing files
|
// Simulate listing files
|
||||||
readdirMock.mockResolvedValueOnce([
|
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)
|
] as any)
|
||||||
|
|
||||||
statMock.mockImplementation(
|
statMock.mockImplementation(
|
||||||
|
|
@ -430,6 +458,7 @@ describe("addCustomInstructions", () => {
|
||||||
expect(result).toContain("# Rules from /fake/path/.roo/rules-test-mode/rule2.txt:")
|
expect(result).toContain("# Rules from /fake/path/.roo/rules-test-mode/rule2.txt:")
|
||||||
expect(result).toContain("mode specific rule 2")
|
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/rule1.txt")
|
||||||
expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules-test-mode/rule2.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")
|
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
|
// Indirectly test readTextFilesFromDirectory and formatDirectoryContent through loadRuleFiles
|
||||||
describe("Rules directory reading", () => {
|
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(() => {
|
beforeEach(() => {
|
||||||
jest.clearAllMocks()
|
jest.clearAllMocks()
|
||||||
})
|
})
|
||||||
|
|
|
||||||
|
|
@ -36,13 +36,36 @@ async function directoryExists(dirPath: string): Promise<boolean> {
|
||||||
*/
|
*/
|
||||||
async function readTextFilesFromDirectory(dirPath: string): Promise<Array<{ filename: string; content: string }>> {
|
async function readTextFilesFromDirectory(dirPath: string): Promise<Array<{ filename: string; content: string }>> {
|
||||||
try {
|
try {
|
||||||
const files = await fs
|
const entries = await fs.readdir(dirPath, { withFileTypes: true, recursive: true })
|
||||||
.readdir(dirPath, { withFileTypes: true, recursive: true })
|
|
||||||
.then((files) => files.filter((file) => file.isFile()))
|
// Process all entries - regular files and symlinks that might point to files
|
||||||
.then((files) => files.map((file) => path.resolve(file.parentPath, file.name)))
|
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(
|
const fileContents = await Promise.all(
|
||||||
files.map(async (file) => {
|
filePaths.map(async (file) => {
|
||||||
try {
|
try {
|
||||||
// Check if it's a file (not a directory)
|
// Check if it's a file (not a directory)
|
||||||
const stats = await fs.stat(file)
|
const stats = await fs.stat(file)
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue