Skip to content

Commit dedd655

Browse files
authored
Fix a bug not to read rule files properly, under nested .roo/rules directories (#2405)
* fix .roo/rules/subdir/file path calculation * add changeset
1 parent 4c81f7e commit dedd655

File tree

3 files changed

+90
-9
lines changed

3 files changed

+90
-9
lines changed

.changeset/poor-dolphins-brush.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"roo-cline": patch
3+
---
4+
5+
Fix bug where nested .roo/rules directories are not respected properly

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

Lines changed: 84 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ describe("loadRuleFiles", () => {
127127

128128
// Simulate listing files
129129
readdirMock.mockResolvedValueOnce([
130-
{ name: "file1.txt", isFile: () => true },
131-
{ name: "file2.txt", isFile: () => true },
130+
{ name: "file1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" },
131+
{ name: "file2.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" },
132132
] as any)
133133

134134
statMock.mockImplementation(
@@ -201,6 +201,80 @@ describe("loadRuleFiles", () => {
201201
const result = await loadRuleFiles("/fake/path")
202202
expect(result).toBe("\n# Rules from .roorules:\nroo rules content\n")
203203
})
204+
205+
it("should read files from nested subdirectories in .roo/rules/", async () => {
206+
// Simulate .roo/rules directory exists
207+
statMock.mockResolvedValueOnce({
208+
isDirectory: jest.fn().mockReturnValue(true),
209+
} as any)
210+
211+
// Simulate listing files including subdirectories
212+
readdirMock.mockResolvedValueOnce([
213+
{ name: "subdir", isFile: () => false, isDirectory: () => true, parentPath: "/fake/path/.roo/rules" },
214+
{ name: "root.txt", isFile: () => true, isDirectory: () => false, parentPath: "/fake/path/.roo/rules" },
215+
{
216+
name: "nested1.txt",
217+
isFile: () => true,
218+
isDirectory: () => false,
219+
parentPath: "/fake/path/.roo/rules/subdir",
220+
},
221+
{
222+
name: "nested2.txt",
223+
isFile: () => true,
224+
isDirectory: () => false,
225+
parentPath: "/fake/path/.roo/rules/subdir/subdir2",
226+
},
227+
] as any)
228+
229+
statMock.mockImplementation((path: string) => {
230+
if (path.endsWith("txt")) {
231+
return Promise.resolve({
232+
isFile: jest.fn().mockReturnValue(true),
233+
isDirectory: jest.fn().mockReturnValue(false),
234+
} as any)
235+
}
236+
return Promise.resolve({
237+
isFile: jest.fn().mockReturnValue(false),
238+
isDirectory: jest.fn().mockReturnValue(true),
239+
} as any)
240+
})
241+
242+
readFileMock.mockImplementation((filePath: PathLike) => {
243+
const path = filePath.toString()
244+
if (path === "/fake/path/.roo/rules/root.txt") {
245+
return Promise.resolve("root file content")
246+
}
247+
if (path === "/fake/path/.roo/rules/subdir/nested1.txt") {
248+
return Promise.resolve("nested file 1 content")
249+
}
250+
if (path === "/fake/path/.roo/rules/subdir/subdir2/nested2.txt") {
251+
return Promise.resolve("nested file 2 content")
252+
}
253+
return Promise.reject({ code: "ENOENT" })
254+
})
255+
256+
const result = await loadRuleFiles("/fake/path")
257+
258+
// Check root file content
259+
expect(result).toContain("# Rules from /fake/path/.roo/rules/root.txt:")
260+
expect(result).toContain("root file content")
261+
262+
// Check nested files content
263+
expect(result).toContain("# Rules from /fake/path/.roo/rules/subdir/nested1.txt:")
264+
expect(result).toContain("nested file 1 content")
265+
expect(result).toContain("# Rules from /fake/path/.roo/rules/subdir/subdir2/nested2.txt:")
266+
expect(result).toContain("nested file 2 content")
267+
268+
// Verify correct paths were checked
269+
expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/root.txt")
270+
expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/subdir/nested1.txt")
271+
expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/subdir/subdir2/nested2.txt")
272+
273+
// Verify files were read with correct paths
274+
expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/root.txt", "utf-8")
275+
expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/subdir/nested1.txt", "utf-8")
276+
expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/subdir/subdir2/nested2.txt", "utf-8")
277+
})
204278
})
205279

206280
describe("addCustomInstructions", () => {
@@ -321,8 +395,8 @@ describe("addCustomInstructions", () => {
321395

322396
// Simulate listing files
323397
readdirMock.mockResolvedValueOnce([
324-
{ name: "rule1.txt", isFile: () => true },
325-
{ name: "rule2.txt", isFile: () => true },
398+
{ name: "rule1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules-test-mode" },
399+
{ name: "rule2.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules-test-mode" },
326400
] as any)
327401

328402
statMock.mockImplementation(
@@ -422,7 +496,9 @@ describe("addCustomInstructions", () => {
422496
)
423497

424498
// Simulate directory has files
425-
readdirMock.mockResolvedValueOnce([{ name: "rule1.txt", isFile: () => true }] as any)
499+
readdirMock.mockResolvedValueOnce([
500+
{ name: "rule1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules-test-mode" },
501+
] as any)
426502
readFileMock.mockReset()
427503

428504
// Set up stat mock for checking files
@@ -515,9 +591,9 @@ describe("Rules directory reading", () => {
515591

516592
// Simulate listing files
517593
readdirMock.mockResolvedValueOnce([
518-
{ name: "file1.txt", isFile: () => true },
519-
{ name: "file2.txt", isFile: () => true },
520-
{ name: "file3.txt", isFile: () => true },
594+
{ name: "file1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" },
595+
{ name: "file2.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" },
596+
{ name: "file3.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" },
521597
] as any)
522598

523599
statMock.mockImplementation((path) => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ async function readTextFilesFromDirectory(dirPath: string): Promise<Array<{ file
3939
const files = await fs
4040
.readdir(dirPath, { withFileTypes: true, recursive: true })
4141
.then((files) => files.filter((file) => file.isFile()))
42-
.then((files) => files.map((file) => path.resolve(dirPath, file.name)))
42+
.then((files) => files.map((file) => path.resolve(file.parentPath, file.name)))
4343

4444
const fileContents = await Promise.all(
4545
files.map(async (file) => {

0 commit comments

Comments
 (0)