Skip to content

Commit 4448228

Browse files
committed
fix: remove existing rules folder when importing mode without rules
- Always remove the existing rules folder for a mode during project-level import - This ensures that if the imported mode has no rules, the folder is cleaned up - Added tests to verify the behavior works correctly
1 parent 0649004 commit 4448228

File tree

2 files changed

+123
-12
lines changed

2 files changed

+123
-12
lines changed

src/core/config/CustomModesManager.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -719,11 +719,12 @@ export class CustomModesManager {
719719
source: source, // Use the provided source parameter
720720
})
721721

722-
// Only import rules files for project-level imports
723-
if (source === "project" && rulesFiles && Array.isArray(rulesFiles)) {
722+
// Handle project-level imports
723+
if (source === "project") {
724724
const workspacePath = getWorkspacePath()
725725

726-
// First, remove the existing rules folder for this mode if it exists
726+
// Always remove the existing rules folder for this mode if it exists
727+
// This ensures that if the imported mode has no rules, the folder is cleaned up
727728
const rulesFolderPath = path.join(workspacePath, ".roo", `rules-${importMode.slug}`)
728729
try {
729730
await fs.rm(rulesFolderPath, { recursive: true, force: true })
@@ -733,17 +734,20 @@ export class CustomModesManager {
733734
logger.debug(`No existing rules folder to remove for mode ${importMode.slug}`)
734735
}
735736

736-
// Now import the new rules files
737-
for (const ruleFile of rulesFiles) {
738-
if (ruleFile.relativePath && ruleFile.content) {
739-
const targetPath = path.join(workspacePath, ".roo", ruleFile.relativePath)
737+
// Only create new rules files if they exist in the import
738+
if (rulesFiles && Array.isArray(rulesFiles) && rulesFiles.length > 0) {
739+
// Import the new rules files
740+
for (const ruleFile of rulesFiles) {
741+
if (ruleFile.relativePath && ruleFile.content) {
742+
const targetPath = path.join(workspacePath, ".roo", ruleFile.relativePath)
740743

741-
// Ensure directory exists
742-
const targetDir = path.dirname(targetPath)
743-
await fs.mkdir(targetDir, { recursive: true })
744+
// Ensure directory exists
745+
const targetDir = path.dirname(targetPath)
746+
await fs.mkdir(targetDir, { recursive: true })
744747

745-
// Write the file
746-
await fs.writeFile(targetPath, ruleFile.content, "utf-8")
748+
// Write the file
749+
await fs.writeFile(targetPath, ruleFile.content, "utf-8")
750+
}
747751
}
748752
}
749753
} else if (source === "global" && rulesFiles && Array.isArray(rulesFiles)) {

src/core/config/__tests__/CustomModesManager.spec.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,6 +1034,113 @@ describe("CustomModesManager", () => {
10341034
expect(result.success).toBe(false)
10351035
expect(result.error).toContain("Permission denied")
10361036
})
1037+
1038+
it("should remove existing rules folder when importing mode without rules", async () => {
1039+
const importYaml = yaml.stringify({
1040+
customModes: [
1041+
{
1042+
slug: "test-mode",
1043+
name: "Test Mode",
1044+
roleDefinition: "Test Role",
1045+
groups: ["read"],
1046+
// No rulesFiles property - this mode has no rules
1047+
},
1048+
],
1049+
})
1050+
1051+
let roomodesContent: any = null
1052+
;(fs.readFile as Mock).mockImplementation(async (path: string) => {
1053+
if (path === mockSettingsPath) {
1054+
return yaml.stringify({ customModes: [] })
1055+
}
1056+
if (path === mockRoomodes && roomodesContent) {
1057+
return yaml.stringify(roomodesContent)
1058+
}
1059+
throw new Error("File not found")
1060+
})
1061+
;(fs.writeFile as Mock).mockImplementation(async (path: string, content: string) => {
1062+
if (path === mockRoomodes) {
1063+
roomodesContent = yaml.parse(content)
1064+
}
1065+
return Promise.resolve()
1066+
})
1067+
;(fs.rm as Mock).mockResolvedValue(undefined)
1068+
1069+
const result = await manager.importModeWithRules(importYaml)
1070+
1071+
expect(result.success).toBe(true)
1072+
1073+
// Verify that fs.rm was called to remove the existing rules folder
1074+
expect(fs.rm).toHaveBeenCalledWith(expect.stringContaining(path.join(".roo", "rules-test-mode")), {
1075+
recursive: true,
1076+
force: true,
1077+
})
1078+
1079+
// Verify mode was imported
1080+
expect(fs.writeFile).toHaveBeenCalledWith(
1081+
expect.stringContaining(".roomodes"),
1082+
expect.stringContaining("test-mode"),
1083+
"utf-8",
1084+
)
1085+
})
1086+
1087+
it("should remove existing rules folder and create new ones when importing mode with rules", async () => {
1088+
const importYaml = yaml.stringify({
1089+
customModes: [
1090+
{
1091+
slug: "test-mode",
1092+
name: "Test Mode",
1093+
roleDefinition: "Test Role",
1094+
groups: ["read"],
1095+
rulesFiles: [
1096+
{
1097+
relativePath: "rules-test-mode/new-rule.md",
1098+
content: "New rule content",
1099+
},
1100+
],
1101+
},
1102+
],
1103+
})
1104+
1105+
let roomodesContent: any = null
1106+
let writtenFiles: Record<string, string> = {}
1107+
;(fs.readFile as Mock).mockImplementation(async (path: string) => {
1108+
if (path === mockSettingsPath) {
1109+
return yaml.stringify({ customModes: [] })
1110+
}
1111+
if (path === mockRoomodes && roomodesContent) {
1112+
return yaml.stringify(roomodesContent)
1113+
}
1114+
throw new Error("File not found")
1115+
})
1116+
;(fs.writeFile as Mock).mockImplementation(async (path: string, content: string) => {
1117+
if (path === mockRoomodes) {
1118+
roomodesContent = yaml.parse(content)
1119+
} else {
1120+
writtenFiles[path] = content
1121+
}
1122+
return Promise.resolve()
1123+
})
1124+
;(fs.rm as Mock).mockResolvedValue(undefined)
1125+
;(fs.mkdir as Mock).mockResolvedValue(undefined)
1126+
1127+
const result = await manager.importModeWithRules(importYaml)
1128+
1129+
expect(result.success).toBe(true)
1130+
1131+
// Verify that fs.rm was called to remove the existing rules folder
1132+
expect(fs.rm).toHaveBeenCalledWith(expect.stringContaining(path.join(".roo", "rules-test-mode")), {
1133+
recursive: true,
1134+
force: true,
1135+
})
1136+
1137+
// Verify new rules files were created
1138+
expect(fs.mkdir).toHaveBeenCalledWith(expect.stringContaining("rules-test-mode"), { recursive: true })
1139+
1140+
// Verify file contents
1141+
const newRulePath = Object.keys(writtenFiles).find((p) => p.includes("new-rule.md"))
1142+
expect(writtenFiles[newRulePath!]).toBe("New rule content")
1143+
})
10371144
})
10381145
})
10391146

0 commit comments

Comments
 (0)