Skip to content

Commit 9dce0e8

Browse files
committed
Handle errors more gracefully when reading custom instructions from files
1 parent 7171a0e commit 9dce0e8

File tree

3 files changed

+245
-151
lines changed

3 files changed

+245
-151
lines changed
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import { loadRuleFiles, addCustomInstructions } from "../custom-instructions"
2+
import fs from "fs/promises"
3+
4+
// Mock fs/promises
5+
jest.mock("fs/promises")
6+
const mockedFs = jest.mocked(fs)
7+
8+
describe("loadRuleFiles", () => {
9+
beforeEach(() => {
10+
jest.clearAllMocks()
11+
})
12+
13+
it("should combine content from multiple rule files when they exist", async () => {
14+
mockedFs.readFile.mockImplementation(((filePath: string | Buffer | URL | number) => {
15+
if (filePath.toString().endsWith(".clinerules")) {
16+
return Promise.resolve("cline rules content")
17+
}
18+
if (filePath.toString().endsWith(".cursorrules")) {
19+
return Promise.resolve("cursor rules content")
20+
}
21+
return Promise.reject({ code: "ENOENT" })
22+
}) as any)
23+
24+
const result = await loadRuleFiles("/fake/path")
25+
expect(result).toBe(
26+
"\n# Rules from .clinerules:\ncline rules content\n" +
27+
"\n# Rules from .cursorrules:\ncursor rules content\n",
28+
)
29+
})
30+
31+
it("should handle when no rule files exist", async () => {
32+
mockedFs.readFile.mockRejectedValue({ code: "ENOENT" })
33+
34+
const result = await loadRuleFiles("/fake/path")
35+
expect(result).toBe("")
36+
})
37+
38+
it("should throw on unexpected errors", async () => {
39+
const error = new Error("Permission denied") as NodeJS.ErrnoException
40+
error.code = "EPERM"
41+
mockedFs.readFile.mockRejectedValue(error)
42+
43+
await expect(async () => {
44+
await loadRuleFiles("/fake/path")
45+
}).rejects.toThrow()
46+
})
47+
48+
it("should skip directories with same name as rule files", async () => {
49+
mockedFs.readFile.mockImplementation(((filePath: string | Buffer | URL | number) => {
50+
if (filePath.toString().endsWith(".clinerules")) {
51+
return Promise.reject({ code: "EISDIR" })
52+
}
53+
if (filePath.toString().endsWith(".cursorrules")) {
54+
return Promise.resolve("cursor rules content")
55+
}
56+
return Promise.reject({ code: "ENOENT" })
57+
}) as any)
58+
59+
const result = await loadRuleFiles("/fake/path")
60+
expect(result).toBe("\n# Rules from .cursorrules:\ncursor rules content\n")
61+
})
62+
})
63+
64+
describe("addCustomInstructions", () => {
65+
beforeEach(() => {
66+
jest.clearAllMocks()
67+
})
68+
69+
it("should combine all instruction types when provided", async () => {
70+
mockedFs.readFile.mockResolvedValue("mode specific rules")
71+
72+
const result = await addCustomInstructions(
73+
"mode instructions",
74+
"global instructions",
75+
"/fake/path",
76+
"test-mode",
77+
{ preferredLanguage: "Spanish" },
78+
)
79+
80+
expect(result).toContain("Language Preference:")
81+
expect(result).toContain("Spanish")
82+
expect(result).toContain("Global Instructions:\nglobal instructions")
83+
expect(result).toContain("Mode-specific Instructions:\nmode instructions")
84+
expect(result).toContain("Rules from .clinerules-test-mode:\nmode specific rules")
85+
})
86+
87+
it("should return empty string when no instructions provided", async () => {
88+
mockedFs.readFile.mockRejectedValue({ code: "ENOENT" })
89+
90+
const result = await addCustomInstructions("", "", "/fake/path", "", {})
91+
expect(result).toBe("")
92+
})
93+
94+
it("should handle missing mode-specific rules file", async () => {
95+
mockedFs.readFile.mockRejectedValue({ code: "ENOENT" })
96+
97+
const result = await addCustomInstructions(
98+
"mode instructions",
99+
"global instructions",
100+
"/fake/path",
101+
"test-mode",
102+
)
103+
104+
expect(result).toContain("Global Instructions:")
105+
expect(result).toContain("Mode-specific Instructions:")
106+
expect(result).not.toContain("Rules from .clinerules-test-mode")
107+
})
108+
109+
it("should throw on unexpected errors", async () => {
110+
const error = new Error("Permission denied") as NodeJS.ErrnoException
111+
error.code = "EPERM"
112+
mockedFs.readFile.mockRejectedValue(error)
113+
114+
await expect(async () => {
115+
await addCustomInstructions("", "", "/fake/path", "test-mode")
116+
}).rejects.toThrow()
117+
})
118+
119+
it("should skip mode-specific rule files that are directories", async () => {
120+
mockedFs.readFile.mockImplementation(((filePath: string | Buffer | URL | number) => {
121+
if (filePath.toString().includes(".clinerules-test-mode")) {
122+
return Promise.reject({ code: "EISDIR" })
123+
}
124+
return Promise.reject({ code: "ENOENT" })
125+
}) as any)
126+
127+
const result = await addCustomInstructions(
128+
"mode instructions",
129+
"global instructions",
130+
"/fake/path",
131+
"test-mode",
132+
)
133+
134+
expect(result).toContain("Global Instructions:\nglobal instructions")
135+
expect(result).toContain("Mode-specific Instructions:\nmode instructions")
136+
expect(result).not.toContain("Rules from .clinerules-test-mode")
137+
})
138+
})

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ export async function loadRuleFiles(cwd: string): Promise<string> {
1212
combinedRules += `\n# Rules from ${file}:\n${content.trim()}\n`
1313
}
1414
} catch (err) {
15-
// Silently skip if file doesn't exist
16-
if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
15+
const errorCode = (err as NodeJS.ErrnoException).code
16+
if (!errorCode || !["ENOENT", "EISDIR"].includes(errorCode)) {
1717
throw err
1818
}
1919
}
@@ -41,8 +41,8 @@ export async function addCustomInstructions(
4141
modeRuleContent = content.trim()
4242
}
4343
} catch (err) {
44-
// Silently skip if file doesn't exist
45-
if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
44+
const errorCode = (err as NodeJS.ErrnoException).code
45+
if (!errorCode || !["ENOENT", "EISDIR"].includes(errorCode)) {
4646
throw err
4747
}
4848
}

0 commit comments

Comments
 (0)