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

Author: roomote

src/core/config/CustomModesManager.ts

@@ -352,59 +352,128 @@ 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)
+
+			// Get modes from .roomodes if it exists.
+			const roomodesPath = await this.getWorkspaceRoomodes()
+			const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : []
 
-		// Create maps to store modes by source.
-		const projectModes = new Map<string, ModeConfig>()
-		const globalModes = new Map<string, ModeConfig>()
+			logger.debug("Loaded modes from files", {
+				settingsCount: settingsModes.length,
+				roomodesCount: roomodesModes.length,
+				settingsPath,
+				roomodesPath
+			})
 
-		// Add project modes (they take precedence).
-		for (const mode of roomodesModes) {
-			projectModes.set(mode.slug, { ...mode, source: "project" as const })
-		}
+			// Create maps to store modes by source.
+			const projectModes = new Map<string, ModeConfig>()
+			const globalModes = new Map<string, ModeConfig>()
 
-		// Add global modes.
-		for (const mode of settingsModes) {
-			if (!projectModes.has(mode.slug)) {
-				globalModes.set(mode.slug, { ...mode, source: "global" as const })
+			// 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 })
 			}
-		}
 
-		// 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 })),
-		]
+			// 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 })
+				}
+			}
 
-		await this.context.globalState.update("customModes", mergedModes)
+			// 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)
 
-		this.cachedModes = mergedModes
-		this.cachedAt = now
+			// Update cache
+			this.cachedModes = validModes
+			this.cachedAt = now
 
-		return mergedModes
+			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 []
+			}
+		}
 	}
 
 	public async updateCustomMode(slug: string, config: ModeConfig): Promise<void> {
 		try {
 			// 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
-				})
+					await this.updateModesInFile(targetPath, (modes) => {
+						const updatedModes = modes.filter((m) => m.slug !== slug)
+						updatedModes.push(modeWithSource)
+						return updatedModes
+					})
 
-				this.clearCache()
-				await this.refreshMergedState()
+					// 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
+				}
 			})
 		} 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()
+
+			logger.debug("Refreshing merged state", { settingsPath, roomodesPath })
+
+			const settingsModes = await this.loadModesFromFile(settingsPath)
+			const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : []
+			const mergedModes = await this.mergeCustomModes(roomodesModes, settingsModes)
 
-		const settingsModes = await this.loadModesFromFile(settingsPath)
-		const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : []
-		const mergedModes = await this.mergeCustomModes(roomodesModes, settingsModes)
+			logger.debug("Merged modes loaded", {
+				settingsCount: settingsModes.length,
+				roomodesCount: roomodesModes.length,
+				mergedCount: mergedModes.length
+			})
 
-		await this.context.globalState.update("customModes", mergedModes)
+			// Update global state with merged modes
+			await this.context.globalState.update("customModes", mergedModes)
 
-		this.clearCache()
+			// Update cache with fresh data
+			this.cachedModes = mergedModes
+			this.cachedAt = Date.now()
 
-		await this.onUpdate()
+			// 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