mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
fix: resolve custom modes disappearing and creation issues
- Fix race condition in updateCustomMode method by reordering state refresh and cache clearing - Improve error handling and state recovery in refreshMergedState method - Add defensive checks for state consistency in getCustomModes method - Add comprehensive logging for debugging state issues - Ensure proper cache invalidation timing to prevent stale data - Add fallback recovery from global state when file operations fail - Validate merged modes before updating state to filter out invalid entries Fixes #5855
This commit is contained in:
Roo Code
parent
a28d50e20c
commit
acb360b9de
1 changed files with 195 additions and 74 deletions
|
|
@ -352,51 +352,120 @@ export class CustomModesManager {
|
|||
}
|
||||
|
||||
public async getCustomModes(): Promise<ModeConfig[]> {
|
||||
// Check if we have a valid cached result.
|
||||
const now = Date.now()
|
||||
try {
|
||||
// Check if we have a valid cached result.
|
||||
const now = Date.now()
|
||||
|
||||
if (this.cachedModes && now - this.cachedAt < CustomModesManager.cacheTTL) {
|
||||
return this.cachedModes
|
||||
}
|
||||
if (this.cachedModes && now - this.cachedAt < CustomModesManager.cacheTTL) {
|
||||
logger.debug("Returning cached custom modes", { count: this.cachedModes.length })
|
||||
return this.cachedModes
|
||||
}
|
||||
|
||||
// Get modes from settings file.
|
||||
const settingsPath = await this.getCustomModesFilePath()
|
||||
const settingsModes = await this.loadModesFromFile(settingsPath)
|
||||
logger.debug("Loading custom modes from files")
|
||||
|
||||
// Get modes from .roomodes if it exists.
|
||||
const roomodesPath = await this.getWorkspaceRoomodes()
|
||||
const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : []
|
||||
// Get modes from settings file.
|
||||
const settingsPath = await this.getCustomModesFilePath()
|
||||
const settingsModes = await this.loadModesFromFile(settingsPath)
|
||||
|
||||
// Create maps to store modes by source.
|
||||
const projectModes = new Map<string, ModeConfig>()
|
||||
const globalModes = new Map<string, ModeConfig>()
|
||||
// Get modes from .roomodes if it exists.
|
||||
const roomodesPath = await this.getWorkspaceRoomodes()
|
||||
const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : []
|
||||
|
||||
// Add project modes (they take precedence).
|
||||
for (const mode of roomodesModes) {
|
||||
projectModes.set(mode.slug, { ...mode, source: "project" as const })
|
||||
}
|
||||
logger.debug("Loaded modes from files", {
|
||||
settingsCount: settingsModes.length,
|
||||
roomodesCount: roomodesModes.length,
|
||||
settingsPath,
|
||||
roomodesPath
|
||||
})
|
||||
|
||||
// Add global modes.
|
||||
for (const mode of settingsModes) {
|
||||
if (!projectModes.has(mode.slug)) {
|
||||
globalModes.set(mode.slug, { ...mode, source: "global" as const })
|
||||
// Create maps to store modes by source.
|
||||
const projectModes = new Map<string, ModeConfig>()
|
||||
const globalModes = new Map<string, ModeConfig>()
|
||||
|
||||
// Add project modes (they take precedence).
|
||||
for (const mode of roomodesModes) {
|
||||
if (!mode.slug) {
|
||||
logger.warn("Found mode without slug in roomodes, skipping", { mode })
|
||||
continue
|
||||
}
|
||||
projectModes.set(mode.slug, { ...mode, source: "project" as const })
|
||||
}
|
||||
|
||||
// Add global modes.
|
||||
for (const mode of settingsModes) {
|
||||
if (!mode.slug) {
|
||||
logger.warn("Found mode without slug in settings, skipping", { mode })
|
||||
continue
|
||||
}
|
||||
if (!projectModes.has(mode.slug)) {
|
||||
globalModes.set(mode.slug, { ...mode, source: "global" as const })
|
||||
}
|
||||
}
|
||||
|
||||
// Combine modes in the correct order: project modes first, then global modes.
|
||||
const mergedModes = [
|
||||
...roomodesModes
|
||||
.filter((mode) => mode.slug) // Defensive check
|
||||
.map((mode) => ({ ...mode, source: "project" as const })),
|
||||
...settingsModes
|
||||
.filter((mode) => mode.slug && !projectModes.has(mode.slug)) // Defensive checks
|
||||
.map((mode) => ({ ...mode, source: "global" as const })),
|
||||
]
|
||||
|
||||
logger.debug("Merged custom modes", {
|
||||
totalCount: mergedModes.length,
|
||||
projectCount: projectModes.size,
|
||||
globalCount: globalModes.size
|
||||
})
|
||||
|
||||
// Validate merged modes before updating state
|
||||
const validModes = mergedModes.filter((mode) => {
|
||||
const isValid = mode.slug && mode.name && mode.roleDefinition && Array.isArray(mode.groups)
|
||||
if (!isValid) {
|
||||
logger.warn("Found invalid mode, filtering out", { mode })
|
||||
}
|
||||
return isValid
|
||||
})
|
||||
|
||||
if (validModes.length !== mergedModes.length) {
|
||||
logger.warn("Filtered out invalid modes", {
|
||||
original: mergedModes.length,
|
||||
valid: validModes.length
|
||||
})
|
||||
}
|
||||
|
||||
// Update global state with validated modes
|
||||
await this.context.globalState.update("customModes", validModes)
|
||||
|
||||
// Update cache
|
||||
this.cachedModes = validModes
|
||||
this.cachedAt = now
|
||||
|
||||
logger.info("Successfully loaded custom modes", { count: validModes.length })
|
||||
return validModes
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error)
|
||||
logger.error("Failed to get custom modes", { error: errorMessage })
|
||||
|
||||
// Try to recover from global state
|
||||
try {
|
||||
const fallbackModes = this.context.globalState.get("customModes", [])
|
||||
logger.info("Recovered custom modes from global state", { count: fallbackModes.length })
|
||||
|
||||
// Update cache with fallback data
|
||||
this.cachedModes = fallbackModes
|
||||
this.cachedAt = Date.now()
|
||||
|
||||
return fallbackModes
|
||||
} catch (recoveryError) {
|
||||
logger.error("Failed to recover custom modes from global state", {
|
||||
error: recoveryError instanceof Error ? recoveryError.message : String(recoveryError)
|
||||
})
|
||||
|
||||
// Return empty array as last resort
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
// Combine modes in the correct order: project modes first, then global modes.
|
||||
const mergedModes = [
|
||||
...roomodesModes.map((mode) => ({ ...mode, source: "project" as const })),
|
||||
...settingsModes
|
||||
.filter((mode) => !projectModes.has(mode.slug))
|
||||
.map((mode) => ({ ...mode, source: "global" as const })),
|
||||
]
|
||||
|
||||
await this.context.globalState.update("customModes", mergedModes)
|
||||
|
||||
this.cachedModes = mergedModes
|
||||
this.cachedAt = now
|
||||
|
||||
return mergedModes
|
||||
}
|
||||
|
||||
public async updateCustomMode(slug: string, config: ModeConfig): Promise<void> {
|
||||
|
|
@ -404,7 +473,7 @@ export class CustomModesManager {
|
|||
// Validate the mode configuration before saving
|
||||
const validationResult = modeConfigSchema.safeParse(config)
|
||||
if (!validationResult.success) {
|
||||
const errors = validationResult.error.errors.map((e) => e.message).join(", ")
|
||||
const errors = validationResult.error.errors.map((e: any) => e.message).join(", ")
|
||||
logger.error(`Invalid mode configuration for ${slug}`, { errors: validationResult.error.errors })
|
||||
throw new Error(`Invalid mode configuration: ${errors}`)
|
||||
}
|
||||
|
|
@ -433,25 +502,43 @@ export class CustomModesManager {
|
|||
}
|
||||
|
||||
await this.queueWrite(async () => {
|
||||
// Ensure source is set correctly based on target file.
|
||||
const modeWithSource = {
|
||||
...config,
|
||||
source: isProjectMode ? ("project" as const) : ("global" as const),
|
||||
try {
|
||||
// Ensure source is set correctly based on target file.
|
||||
const modeWithSource = {
|
||||
...config,
|
||||
source: isProjectMode ? ("project" as const) : ("global" as const),
|
||||
}
|
||||
|
||||
await this.updateModesInFile(targetPath, (modes) => {
|
||||
const updatedModes = modes.filter((m) => m.slug !== slug)
|
||||
updatedModes.push(modeWithSource)
|
||||
return updatedModes
|
||||
})
|
||||
|
||||
// Refresh state before clearing cache to ensure consistency
|
||||
await this.refreshMergedState()
|
||||
this.clearCache()
|
||||
|
||||
// Log successful update for debugging
|
||||
logger.info(`Successfully updated custom mode: ${slug}`, {
|
||||
source: modeWithSource.source,
|
||||
targetPath
|
||||
})
|
||||
} catch (writeError) {
|
||||
// If file write fails, ensure cache is still cleared to prevent stale data
|
||||
this.clearCache()
|
||||
throw writeError
|
||||
}
|
||||
|
||||
await this.updateModesInFile(targetPath, (modes) => {
|
||||
const updatedModes = modes.filter((m) => m.slug !== slug)
|
||||
updatedModes.push(modeWithSource)
|
||||
return updatedModes
|
||||
})
|
||||
|
||||
this.clearCache()
|
||||
await this.refreshMergedState()
|
||||
})
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error)
|
||||
logger.error("Failed to update custom mode", { slug, error: errorMessage })
|
||||
|
||||
// Clear cache on error to prevent inconsistent state
|
||||
this.clearCache()
|
||||
|
||||
vscode.window.showErrorMessage(t("common:customModes.errors.updateFailed", { error: errorMessage }))
|
||||
throw error // Re-throw to allow caller to handle
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -487,18 +574,52 @@ export class CustomModesManager {
|
|||
}
|
||||
|
||||
private async refreshMergedState(): Promise<void> {
|
||||
const settingsPath = await this.getCustomModesFilePath()
|
||||
const roomodesPath = await this.getWorkspaceRoomodes()
|
||||
try {
|
||||
const settingsPath = await this.getCustomModesFilePath()
|
||||
const roomodesPath = await this.getWorkspaceRoomodes()
|
||||
|
||||
const settingsModes = await this.loadModesFromFile(settingsPath)
|
||||
const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : []
|
||||
const mergedModes = await this.mergeCustomModes(roomodesModes, settingsModes)
|
||||
logger.debug("Refreshing merged state", { settingsPath, roomodesPath })
|
||||
|
||||
await this.context.globalState.update("customModes", mergedModes)
|
||||
const settingsModes = await this.loadModesFromFile(settingsPath)
|
||||
const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : []
|
||||
const mergedModes = await this.mergeCustomModes(roomodesModes, settingsModes)
|
||||
|
||||
this.clearCache()
|
||||
logger.debug("Merged modes loaded", {
|
||||
settingsCount: settingsModes.length,
|
||||
roomodesCount: roomodesModes.length,
|
||||
mergedCount: mergedModes.length
|
||||
})
|
||||
|
||||
await this.onUpdate()
|
||||
// Update global state with merged modes
|
||||
await this.context.globalState.update("customModes", mergedModes)
|
||||
|
||||
// Update cache with fresh data
|
||||
this.cachedModes = mergedModes
|
||||
this.cachedAt = Date.now()
|
||||
|
||||
// Notify listeners of the update
|
||||
await this.onUpdate()
|
||||
} catch (error) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error)
|
||||
logger.error("Failed to refresh merged state", { error: errorMessage })
|
||||
|
||||
// On error, clear cache to prevent stale data
|
||||
this.clearCache()
|
||||
|
||||
// Try to recover by loading from global state
|
||||
try {
|
||||
const fallbackModes = this.context.globalState.get("customModes", [])
|
||||
logger.info("Recovered custom modes from global state", { count: fallbackModes.length })
|
||||
this.cachedModes = fallbackModes
|
||||
this.cachedAt = Date.now()
|
||||
} catch (recoveryError) {
|
||||
logger.error("Failed to recover custom modes from global state", {
|
||||
error: recoveryError instanceof Error ? recoveryError.message : String(recoveryError)
|
||||
})
|
||||
}
|
||||
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
public async deleteCustomMode(slug: string): Promise<void> {
|
||||
|
|
@ -729,10 +850,10 @@ export class CustomModesManager {
|
|||
|
||||
// Merge custom prompts if provided
|
||||
if (customPrompts) {
|
||||
if (customPrompts.roleDefinition) exportMode.roleDefinition = customPrompts.roleDefinition
|
||||
if (customPrompts.description) exportMode.description = customPrompts.description
|
||||
if (customPrompts.whenToUse) exportMode.whenToUse = customPrompts.whenToUse
|
||||
if (customPrompts.customInstructions) exportMode.customInstructions = customPrompts.customInstructions
|
||||
if (customPrompts.roleDefinition) (exportMode as any).roleDefinition = customPrompts.roleDefinition
|
||||
if (customPrompts.description) (exportMode as any).description = customPrompts.description
|
||||
if (customPrompts.whenToUse) (exportMode as any).whenToUse = customPrompts.whenToUse
|
||||
if (customPrompts.customInstructions) (exportMode as any).customInstructions = customPrompts.customInstructions
|
||||
}
|
||||
|
||||
// Add rules files if any exist
|
||||
|
|
@ -799,24 +920,24 @@ export class CustomModesManager {
|
|||
// Validate the mode configuration
|
||||
const validationResult = modeConfigSchema.safeParse(modeConfig)
|
||||
if (!validationResult.success) {
|
||||
logger.error(`Invalid mode configuration for ${modeConfig.slug}`, {
|
||||
logger.error(`Invalid mode configuration for ${(modeConfig as any).slug}`, {
|
||||
errors: validationResult.error.errors,
|
||||
})
|
||||
return {
|
||||
success: false,
|
||||
error: `Invalid mode configuration for ${modeConfig.slug}: ${validationResult.error.errors.map((e) => e.message).join(", ")}`,
|
||||
error: `Invalid mode configuration for ${(modeConfig as any).slug}: ${validationResult.error.errors.map((e: any) => e.message).join(", ")}`,
|
||||
}
|
||||
}
|
||||
|
||||
// Check for existing mode conflicts
|
||||
const existingModes = await this.getCustomModes()
|
||||
const existingMode = existingModes.find((m) => m.slug === importMode.slug)
|
||||
const existingMode = existingModes.find((m) => m.slug === (importMode as any).slug)
|
||||
if (existingMode) {
|
||||
logger.info(`Overwriting existing mode: ${importMode.slug}`)
|
||||
logger.info(`Overwriting existing mode: ${(importMode as any).slug}`)
|
||||
}
|
||||
|
||||
// Import the mode configuration with the specified source
|
||||
await this.updateCustomMode(importMode.slug, {
|
||||
await this.updateCustomMode((importMode as any).slug, {
|
||||
...modeConfig,
|
||||
source: source, // Use the provided source parameter
|
||||
})
|
||||
|
|
@ -827,13 +948,13 @@ export class CustomModesManager {
|
|||
|
||||
// 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}`)
|
||||
const rulesFolderPath = path.join(workspacePath, ".roo", `rules-${(importMode as any).slug}`)
|
||||
try {
|
||||
await fs.rm(rulesFolderPath, { recursive: true, force: true })
|
||||
logger.info(`Removed existing rules folder for mode ${importMode.slug}`)
|
||||
logger.info(`Removed existing rules folder for mode ${(importMode as any).slug}`)
|
||||
} catch (error) {
|
||||
// It's okay if the folder doesn't exist
|
||||
logger.debug(`No existing rules folder to remove for mode ${importMode.slug}`)
|
||||
logger.debug(`No existing rules folder to remove for mode ${(importMode as any).slug}`)
|
||||
}
|
||||
|
||||
// Only create new rules files if they exist in the import
|
||||
|
|
@ -875,13 +996,13 @@ export class CustomModesManager {
|
|||
|
||||
// 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}`)
|
||||
const rulesFolderPath = path.join(globalRooDir, `rules-${(importMode as any).slug}`)
|
||||
try {
|
||||
await fs.rm(rulesFolderPath, { recursive: true, force: true })
|
||||
logger.info(`Removed existing global rules folder for mode ${importMode.slug}`)
|
||||
logger.info(`Removed existing global rules folder for mode ${(importMode as any).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}`)
|
||||
logger.debug(`No existing global rules folder to remove for mode ${(importMode as any).slug}`)
|
||||
}
|
||||
|
||||
// Import the new rules files with path validation
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue