fix: resolve global mode export not including rules files (#5834) (#5837)

Co-authored-by: Daniel Riccio <ricciodaniel98@gmail.com>
This commit is contained in:
Hannes Rudolph 2025-07-21 12:16:11 -06:00 committed by GitHub
parent 9fce90be9d
commit 464a3ffa5f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 280 additions and 155 deletions

View file

@ -558,48 +558,54 @@ export class CustomModesManager {
*/
public async checkRulesDirectoryHasContent(slug: string): Promise<boolean> {
try {
// Get workspace path
const workspacePath = getWorkspacePath()
if (!workspacePath) {
return false
}
// First, find the mode to determine its source
const allModes = await this.getCustomModes()
const mode = allModes.find((m) => m.slug === slug)
// Check if .roomodes file exists and contains this mode
// This ensures we can only consolidate rules for modes that have been customized
const roomodesPath = path.join(workspacePath, ROOMODES_FILENAME)
try {
const roomodesExists = await fileExistsAtPath(roomodesPath)
if (roomodesExists) {
const roomodesContent = await fs.readFile(roomodesPath, "utf-8")
const roomodesData = yaml.parse(roomodesContent)
const roomodesModes = roomodesData?.customModes || []
// Check if this specific mode exists in .roomodes
const modeInRoomodes = roomodesModes.find((m: any) => m.slug === slug)
if (!modeInRoomodes) {
return false // Mode not customized in .roomodes, cannot consolidate
}
} else {
// If no .roomodes file exists, check if it's in global custom modes
const allModes = await this.getCustomModes()
const mode = allModes.find((m) => m.slug === slug)
if (!mode) {
return false // Not a custom mode, cannot consolidate
}
if (!mode) {
// If not in custom modes, check if it's in .roomodes (project-specific)
const workspacePath = getWorkspacePath()
if (!workspacePath) {
return false
}
} catch (error) {
// If we can't read .roomodes, fall back to checking custom modes
const allModes = await this.getCustomModes()
const mode = allModes.find((m) => m.slug === slug)
if (!mode) {
return false // Not a custom mode, cannot consolidate
const roomodesPath = path.join(workspacePath, ROOMODES_FILENAME)
try {
const roomodesExists = await fileExistsAtPath(roomodesPath)
if (roomodesExists) {
const roomodesContent = await fs.readFile(roomodesPath, "utf-8")
const roomodesData = yaml.parse(roomodesContent)
const roomodesModes = roomodesData?.customModes || []
// Check if this specific mode exists in .roomodes
const modeInRoomodes = roomodesModes.find((m: any) => m.slug === slug)
if (!modeInRoomodes) {
return false // Mode not found anywhere
}
} else {
return false // No .roomodes file and not in custom modes
}
} catch (error) {
return false // Cannot read .roomodes and not in custom modes
}
}
// Check for .roo/rules-{slug}/ directory
const modeRulesDir = path.join(workspacePath, ".roo", `rules-${slug}`)
// Determine the correct rules directory based on mode source
let modeRulesDir: string
const isGlobalMode = mode?.source === "global"
if (isGlobalMode) {
// For global modes, check in global .roo directory
const globalRooDir = getGlobalRooDirectory()
modeRulesDir = path.join(globalRooDir, `rules-${slug}`)
} else {
// For project modes, check in workspace .roo directory
const workspacePath = getWorkspacePath()
if (!workspacePath) {
return false
}
modeRulesDir = path.join(workspacePath, ".roo", `rules-${slug}`)
}
try {
const stats = await fs.stat(modeRulesDir)
@ -655,24 +661,23 @@ export class CustomModesManager {
// If mode not found in custom modes, check if it's a built-in mode that has been customized
if (!mode) {
// Only check workspace-based modes if workspace is available
const workspacePath = getWorkspacePath()
if (!workspacePath) {
return { success: false, error: "No workspace found" }
}
if (workspacePath) {
const roomodesPath = path.join(workspacePath, ROOMODES_FILENAME)
try {
const roomodesExists = await fileExistsAtPath(roomodesPath)
if (roomodesExists) {
const roomodesContent = await fs.readFile(roomodesPath, "utf-8")
const roomodesData = yaml.parse(roomodesContent)
const roomodesModes = roomodesData?.customModes || []
const roomodesPath = path.join(workspacePath, ROOMODES_FILENAME)
try {
const roomodesExists = await fileExistsAtPath(roomodesPath)
if (roomodesExists) {
const roomodesContent = await fs.readFile(roomodesPath, "utf-8")
const roomodesData = yaml.parse(roomodesContent)
const roomodesModes = roomodesData?.customModes || []
// Find the mode in .roomodes
mode = roomodesModes.find((m: any) => m.slug === slug)
// Find the mode in .roomodes
mode = roomodesModes.find((m: any) => m.slug === slug)
}
} catch (error) {
// Continue to check built-in modes
}
} catch (error) {
// Continue to check built-in modes
}
// If still not found, check if it's a built-in mode
@ -687,14 +692,25 @@ export class CustomModesManager {
}
}
// Get workspace path
const workspacePath = getWorkspacePath()
if (!workspacePath) {
return { success: false, error: "No workspace found" }
// Determine the base directory based on mode source
const isGlobalMode = mode.source === "global"
let baseDir: string
if (isGlobalMode) {
// For global modes, use the global .roo directory
baseDir = getGlobalRooDirectory()
} else {
// For project modes, use the workspace directory
const workspacePath = getWorkspacePath()
if (!workspacePath) {
return { success: false, error: "No workspace found" }
}
baseDir = workspacePath
}
// Check for .roo/rules-{slug}/ directory
const modeRulesDir = path.join(workspacePath, ".roo", `rules-${slug}`)
// Check for .roo/rules-{slug}/ directory (or rules-{slug}/ for global)
const modeRulesDir = isGlobalMode
? path.join(baseDir, `rules-${slug}`)
: path.join(baseDir, ".roo", `rules-${slug}`)
let rulesFiles: RuleFile[] = []
try {
@ -709,8 +725,10 @@ export class CustomModesManager {
const filePath = path.join(modeRulesDir, entry.name)
const content = await fs.readFile(filePath, "utf-8")
if (content.trim()) {
// Calculate relative path from .roo directory
const relativePath = path.relative(path.join(workspacePath, ".roo"), filePath)
// Calculate relative path based on mode source
const relativePath = isGlobalMode
? path.relative(baseDir, filePath)
: path.relative(path.join(baseDir, ".roo"), filePath)
rulesFiles.push({ relativePath, content: content.trim() })
}
}
@ -755,6 +773,77 @@ export class CustomModesManager {
}
}
/**
* Helper method to import rules files for a mode
* @param importMode - The mode being imported
* @param rulesFiles - The rules files to import
* @param source - The import source ("global" or "project")
*/
private async importRulesFiles(
importMode: ExportedModeConfig,
rulesFiles: RuleFile[],
source: "global" | "project",
): Promise<void> {
// Determine base directory and rules folder path based on source
let baseDir: string
let rulesFolderPath: string
if (source === "global") {
baseDir = getGlobalRooDirectory()
rulesFolderPath = path.join(baseDir, `rules-${importMode.slug}`)
} else {
const workspacePath = getWorkspacePath()
baseDir = path.join(workspacePath, ".roo")
rulesFolderPath = path.join(baseDir, `rules-${importMode.slug}`)
}
// Always remove the existing rules folder for this mode if it exists
// This ensures that if the imported mode has no rules, the folder is cleaned up
try {
await fs.rm(rulesFolderPath, { recursive: true, force: true })
logger.info(`Removed existing ${source} rules folder for mode ${importMode.slug}`)
} catch (error) {
// It's okay if the folder doesn't exist
logger.debug(`No existing ${source} rules folder to remove for mode ${importMode.slug}`)
}
// Only proceed with file creation if there are rules files to import
if (!rulesFiles || !Array.isArray(rulesFiles) || rulesFiles.length === 0) {
return
}
// Import the new rules files with path validation
for (const ruleFile of rulesFiles) {
if (ruleFile.relativePath && ruleFile.content) {
// Validate the relative path to prevent path traversal attacks
const normalizedRelativePath = path.normalize(ruleFile.relativePath)
// Ensure the path doesn't contain traversal sequences
if (normalizedRelativePath.includes("..") || path.isAbsolute(normalizedRelativePath)) {
logger.error(`Invalid file path detected: ${ruleFile.relativePath}`)
continue // Skip this file but continue with others
}
const targetPath = path.join(baseDir, normalizedRelativePath)
const normalizedTargetPath = path.normalize(targetPath)
const expectedBasePath = path.normalize(baseDir)
// Ensure the resolved path stays within the base directory
if (!normalizedTargetPath.startsWith(expectedBasePath)) {
logger.error(`Path traversal attempt detected: ${ruleFile.relativePath}`)
continue // Skip this file but continue with others
}
// Ensure directory exists
const targetDir = path.dirname(targetPath)
await fs.mkdir(targetDir, { recursive: true })
// Write the file
await fs.writeFile(targetPath, ruleFile.content, "utf-8")
}
}
}
/**
* Imports modes from YAML content, including their associated rules files
* @param yamlContent - The YAML content containing mode configurations
@ -821,100 +910,8 @@ export class CustomModesManager {
source: source, // Use the provided source parameter
})
// Handle project-level imports
if (source === "project") {
const workspacePath = getWorkspacePath()
// Always remove the existing rules folder for this mode if it exists
// This ensures that if the imported mode has no rules, the folder is cleaned up
const rulesFolderPath = path.join(workspacePath, ".roo", `rules-${importMode.slug}`)
try {
await fs.rm(rulesFolderPath, { recursive: true, force: true })
logger.info(`Removed existing rules folder for mode ${importMode.slug}`)
} catch (error) {
// It's okay if the folder doesn't exist
logger.debug(`No existing rules folder to remove for mode ${importMode.slug}`)
}
// Only create new rules files if they exist in the import
if (rulesFiles && Array.isArray(rulesFiles) && rulesFiles.length > 0) {
// Import the new rules files with path validation
for (const ruleFile of rulesFiles) {
if (ruleFile.relativePath && ruleFile.content) {
// Validate the relative path to prevent path traversal attacks
const normalizedRelativePath = path.normalize(ruleFile.relativePath)
// Ensure the path doesn't contain traversal sequences
if (normalizedRelativePath.includes("..") || path.isAbsolute(normalizedRelativePath)) {
logger.error(`Invalid file path detected: ${ruleFile.relativePath}`)
continue // Skip this file but continue with others
}
const targetPath = path.join(workspacePath, ".roo", normalizedRelativePath)
const normalizedTargetPath = path.normalize(targetPath)
const expectedBasePath = path.normalize(path.join(workspacePath, ".roo"))
// Ensure the resolved path stays within the .roo directory
if (!normalizedTargetPath.startsWith(expectedBasePath)) {
logger.error(`Path traversal attempt detected: ${ruleFile.relativePath}`)
continue // Skip this file but continue with others
}
// Ensure directory exists
const targetDir = path.dirname(targetPath)
await fs.mkdir(targetDir, { recursive: true })
// Write the file
await fs.writeFile(targetPath, ruleFile.content, "utf-8")
}
}
}
} else if (source === "global" && rulesFiles && Array.isArray(rulesFiles)) {
// For global imports, preserve the rules files structure in the global .roo directory
const globalRooDir = getGlobalRooDirectory()
// Always remove the existing rules folder for this mode if it exists
// This ensures that if the imported mode has no rules, the folder is cleaned up
const rulesFolderPath = path.join(globalRooDir, `rules-${importMode.slug}`)
try {
await fs.rm(rulesFolderPath, { recursive: true, force: true })
logger.info(`Removed existing global rules folder for mode ${importMode.slug}`)
} catch (error) {
// It's okay if the folder doesn't exist
logger.debug(`No existing global rules folder to remove for mode ${importMode.slug}`)
}
// Import the new rules files with path validation
for (const ruleFile of rulesFiles) {
if (ruleFile.relativePath && ruleFile.content) {
// Validate the relative path to prevent path traversal attacks
const normalizedRelativePath = path.normalize(ruleFile.relativePath)
// Ensure the path doesn't contain traversal sequences
if (normalizedRelativePath.includes("..") || path.isAbsolute(normalizedRelativePath)) {
logger.error(`Invalid file path detected: ${ruleFile.relativePath}`)
continue // Skip this file but continue with others
}
const targetPath = path.join(globalRooDir, normalizedRelativePath)
const normalizedTargetPath = path.normalize(targetPath)
const expectedBasePath = path.normalize(globalRooDir)
// Ensure the resolved path stays within the global .roo directory
if (!normalizedTargetPath.startsWith(expectedBasePath)) {
logger.error(`Path traversal attempt detected: ${ruleFile.relativePath}`)
continue // Skip this file but continue with others
}
// Ensure directory exists
const targetDir = path.dirname(targetPath)
await fs.mkdir(targetDir, { recursive: true })
// Write the file
await fs.writeFile(targetPath, ruleFile.content, "utf-8")
}
}
}
// Import rules files (this also handles cleanup of existing rules folders)
await this.importRulesFiles(importMode, rulesFiles || [], source)
}
// Refresh the modes after import

View file

@ -1373,7 +1373,7 @@ describe("CustomModesManager", () => {
})
describe("exportModeWithRules", () => {
it("should return error when no workspace is available", async () => {
it("should return error when mode is not found and no workspace is available", async () => {
// Create a fresh manager instance to avoid cache issues
const freshManager = new CustomModesManager(mockContext, mockOnUpdate)
@ -1391,7 +1391,7 @@ describe("CustomModesManager", () => {
const result = await freshManager.exportModeWithRules("test-mode")
expect(result.success).toBe(false)
expect(result.error).toBe("No workspace found")
expect(result.error).toBe("Mode not found")
})
it("should return error when mode is not found", async () => {
@ -1571,5 +1571,133 @@ describe("CustomModesManager", () => {
expect(result.success).toBe(true)
expect(result.yaml).toContain("test-mode")
})
it("should successfully export global mode with rules from global .roo directory", async () => {
// Mock a global mode
const globalMode = {
slug: "global-test-mode",
name: "Global Test Mode",
roleDefinition: "Global Test Role",
groups: ["read"],
source: "global",
}
// Create a fresh manager instance to avoid cache issues
const freshManager = new CustomModesManager(mockContext, mockOnUpdate)
;(fs.readFile as Mock).mockImplementation(async (path: string) => {
if (path === mockSettingsPath) {
return yaml.stringify({ customModes: [globalMode] })
}
if (path.includes("rules-global-test-mode") && path.includes("rule1.md")) {
return "Global rule content"
}
throw new Error("File not found")
})
;(fileExistsAtPath as Mock).mockImplementation(async (path: string) => {
return path === mockSettingsPath
})
;(fs.stat as Mock).mockImplementation(async (path: string) => {
if (path.includes("rules-global-test-mode")) {
return { isDirectory: () => true }
}
throw new Error("Directory not found")
})
;(fs.readdir as Mock).mockImplementation(async (path: string) => {
if (path.includes("rules-global-test-mode")) {
return [{ name: "rule1.md", isFile: () => true }]
}
return []
})
const result = await freshManager.exportModeWithRules("global-test-mode")
expect(result.success).toBe(true)
expect(result.yaml).toContain("global-test-mode")
expect(result.yaml).toContain("Global Test Mode")
expect(result.yaml).toContain("Global rule content")
})
it("should successfully export global mode without rules when global rules directory doesn't exist", async () => {
// Mock a global mode
const globalMode = {
slug: "global-test-mode",
name: "Global Test Mode",
roleDefinition: "Global Test Role",
groups: ["read"],
source: "global",
}
// Create a fresh manager instance to avoid cache issues
const freshManager = new CustomModesManager(mockContext, mockOnUpdate)
;(fs.readFile as Mock).mockImplementation(async (path: string) => {
if (path === mockSettingsPath) {
return yaml.stringify({ customModes: [globalMode] })
}
throw new Error("File not found")
})
;(fileExistsAtPath as Mock).mockImplementation(async (path: string) => {
return path === mockSettingsPath
})
;(fs.stat as Mock).mockRejectedValue(new Error("Directory not found"))
const result = await freshManager.exportModeWithRules("global-test-mode")
expect(result.success).toBe(true)
expect(result.yaml).toContain("global-test-mode")
expect(result.yaml).toContain("Global Test Mode")
// Should not contain rulesFiles since no rules directory exists
expect(result.yaml).not.toContain("rulesFiles")
})
it("should handle global mode export when workspace is not available", async () => {
// Mock a global mode
const globalMode = {
slug: "global-test-mode",
name: "Global Test Mode",
roleDefinition: "Global Test Role",
groups: ["read"],
source: "global",
}
// Create a fresh manager instance to avoid cache issues
const freshManager = new CustomModesManager(mockContext, mockOnUpdate)
// Mock no workspace folders
;(vscode.workspace as any).workspaceFolders = []
;(getWorkspacePath as Mock).mockReturnValue(null)
;(fs.readFile as Mock).mockImplementation(async (path: string) => {
if (path === mockSettingsPath) {
return yaml.stringify({ customModes: [globalMode] })
}
if (path.includes("rules-global-test-mode") && path.includes("rule1.md")) {
return "Global rule content"
}
throw new Error("File not found")
})
;(fileExistsAtPath as Mock).mockImplementation(async (path: string) => {
return path === mockSettingsPath
})
;(fs.stat as Mock).mockImplementation(async (path: string) => {
if (path.includes("rules-global-test-mode")) {
return { isDirectory: () => true }
}
throw new Error("Directory not found")
})
;(fs.readdir as Mock).mockImplementation(async (path: string) => {
if (path.includes("rules-global-test-mode")) {
return [{ name: "rule1.md", isFile: () => true }]
}
return []
})
const result = await freshManager.exportModeWithRules("global-test-mode")
// Should succeed even without workspace since it's a global mode
expect(result.success).toBe(true)
expect(result.yaml).toContain("global-test-mode")
expect(result.yaml).toContain("Global rule content")
})
})
})