From 3b3da61ca80f5a7bb6fe1416a8c7031b11d55cdf Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 30 Jun 2025 09:37:59 +0000 Subject: [PATCH] Fixes #5210: Add warning when deleting modes with associated rules folders - Enhanced deleteCustomMode confirmation dialog to warn users when .roo/rules-{mode} folder exists - Added new translation key 'delete_custom_mode_with_rules' for enhanced warning message - Modified webview message handler to check for rules folder existence before showing confirmation - Added comprehensive tests covering all scenarios: enhanced warnings, standard warnings, error handling - Preserves existing behavior for modes without rules folders - Handles edge cases gracefully (no workspace, file check failures, mode not found) --- .../__tests__/webviewMessageHandler.spec.ts | 215 ++++++++++++++++++ src/core/webview/webviewMessageHandler.ts | 38 +++- src/i18n/locales/en/common.json | 1 + 3 files changed, 253 insertions(+), 1 deletion(-) diff --git a/src/core/webview/__tests__/webviewMessageHandler.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.spec.ts index 46ace3ce85..fd405dabc8 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.spec.ts @@ -2,11 +2,33 @@ import type { Mock } from "vitest" // Mock dependencies - must come before imports vi.mock("../../../api/providers/fetchers/modelCache") +vi.mock("../../../utils/fs") +vi.mock("../../../utils/path") +vi.mock("../../../i18n", () => ({ + t: vi.fn((key: string) => key), // Return the key as-is for testing +})) +vi.mock("vscode", () => ({ + window: { + showInformationMessage: vi.fn(), + showErrorMessage: vi.fn(), + }, + workspace: { + workspaceFolders: [], + }, +})) import { webviewMessageHandler } from "../webviewMessageHandler" import type { ClineProvider } from "../ClineProvider" import { getModels } from "../../../api/providers/fetchers/modelCache" import type { ModelRecord } from "../../../shared/api" +import { fileExistsAtPath } from "../../../utils/fs" +import { getWorkspacePath } from "../../../utils/path" +import * as vscode from "vscode" + +const mockFileExistsAtPath = fileExistsAtPath as Mock +const mockGetWorkspacePath = getWorkspacePath as Mock +const mockShowInformationMessage = vscode.window.showInformationMessage as Mock +const mockShowErrorMessage = vscode.window.showErrorMessage as Mock const mockGetModels = getModels as Mock @@ -275,6 +297,199 @@ describe("webviewMessageHandler - requestRouterModels", () => { }) }) + describe("webviewMessageHandler - deleteCustomMode", () => { + const mockCustomModesManager = { + getCustomModes: vi.fn(), + deleteCustomMode: vi.fn(), + } + + const mockContextProxy = { + setValue: vi.fn(), + } + + const mockProvider = { + ...mockClineProvider, + customModesManager: mockCustomModesManager, + contextProxy: mockContextProxy, + postStateToWebview: vi.fn(), + } as unknown as ClineProvider + + beforeEach(() => { + vi.clearAllMocks() + mockGetWorkspacePath.mockReturnValue("/test/workspace") + }) + + it("shows enhanced warning when rules folder exists", async () => { + const testMode = { + slug: "test-mode", + name: "Test Mode", + source: "project" as const, + } + + mockCustomModesManager.getCustomModes.mockResolvedValue([testMode]) + mockFileExistsAtPath.mockResolvedValue(true) + mockShowInformationMessage.mockResolvedValue("common:answers.yes") + + await webviewMessageHandler(mockProvider, { + type: "deleteCustomMode", + slug: "test-mode", + }) + + // Verify rules folder check was performed + expect(mockFileExistsAtPath).toHaveBeenCalledWith("/test/workspace/.roo/rules-test-mode") + + // Verify enhanced warning message was shown + expect(mockShowInformationMessage).toHaveBeenCalledWith( + "common:confirmation.delete_custom_mode_with_rules", + { modal: true }, + "common:answers.yes", + ) + + // Verify deletion proceeded + expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test-mode") + }) + + it("shows standard warning when rules folder does not exist", async () => { + const testMode = { + slug: "test-mode", + name: "Test Mode", + source: "global" as const, + } + + mockCustomModesManager.getCustomModes.mockResolvedValue([testMode]) + mockFileExistsAtPath.mockResolvedValue(false) + mockShowInformationMessage.mockResolvedValue("common:answers.yes") + + await webviewMessageHandler(mockProvider, { + type: "deleteCustomMode", + slug: "test-mode", + }) + + // Verify standard warning message was shown + expect(mockShowInformationMessage).toHaveBeenCalledWith( + "common:confirmation.delete_custom_mode", + { modal: true }, + "common:answers.yes", + ) + + // Verify deletion proceeded + expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test-mode") + }) + + it("shows standard warning when workspace path is not available", async () => { + const testMode = { + slug: "test-mode", + name: "Test Mode", + source: "project" as const, + } + + mockCustomModesManager.getCustomModes.mockResolvedValue([testMode]) + mockGetWorkspacePath.mockReturnValue("") + mockShowInformationMessage.mockResolvedValue("common:answers.yes") + + await webviewMessageHandler(mockProvider, { + type: "deleteCustomMode", + slug: "test-mode", + }) + + // Verify file check was not performed + expect(mockFileExistsAtPath).not.toHaveBeenCalled() + + // Verify standard warning message was shown + expect(mockShowInformationMessage).toHaveBeenCalledWith( + "common:confirmation.delete_custom_mode", + { modal: true }, + "common:answers.yes", + ) + }) + + it("shows standard warning when file check fails", async () => { + const testMode = { + slug: "test-mode", + name: "Test Mode", + source: "project" as const, + } + + mockCustomModesManager.getCustomModes.mockResolvedValue([testMode]) + mockFileExistsAtPath.mockRejectedValue(new Error("File system error")) + mockShowInformationMessage.mockResolvedValue("common:answers.yes") + + await webviewMessageHandler(mockProvider, { + type: "deleteCustomMode", + slug: "test-mode", + }) + + // Verify standard warning message was shown (fallback) + expect(mockShowInformationMessage).toHaveBeenCalledWith( + "common:confirmation.delete_custom_mode", + { modal: true }, + "common:answers.yes", + ) + + // Verify deletion still proceeded + expect(mockCustomModesManager.deleteCustomMode).toHaveBeenCalledWith("test-mode") + }) + + it("does not delete when user cancels", async () => { + const testMode = { + slug: "test-mode", + name: "Test Mode", + source: "project" as const, + } + + mockCustomModesManager.getCustomModes.mockResolvedValue([testMode]) + mockFileExistsAtPath.mockResolvedValue(true) + mockShowInformationMessage.mockResolvedValue(undefined) // User cancelled + + await webviewMessageHandler(mockProvider, { + type: "deleteCustomMode", + slug: "test-mode", + }) + + // Verify deletion was not performed + expect(mockCustomModesManager.deleteCustomMode).not.toHaveBeenCalled() + }) + + it("shows error when mode is not found", async () => { + mockCustomModesManager.getCustomModes.mockResolvedValue([]) + + await webviewMessageHandler(mockProvider, { + type: "deleteCustomMode", + slug: "nonexistent-mode", + }) + + // Verify error message was shown + expect(mockShowErrorMessage).toHaveBeenCalledWith("common:customModes.errors.modeNotFound") + + // Verify deletion was not attempted + expect(mockCustomModesManager.deleteCustomMode).not.toHaveBeenCalled() + }) + + it("correctly identifies GLOBAL mode source", async () => { + const testMode = { + slug: "global-mode", + name: "Global Mode", + source: "global" as const, + } + + mockCustomModesManager.getCustomModes.mockResolvedValue([testMode]) + mockFileExistsAtPath.mockResolvedValue(true) + mockShowInformationMessage.mockResolvedValue("common:answers.yes") + + await webviewMessageHandler(mockProvider, { + type: "deleteCustomMode", + slug: "global-mode", + }) + + // Verify enhanced warning message shows GLOBAL + expect(mockShowInformationMessage).toHaveBeenCalledWith( + "common:confirmation.delete_custom_mode_with_rules", + { modal: true }, + "common:answers.yes", + ) + }) + }) + it("prefers config values over message values for LiteLLM", async () => { const mockModels: ModelRecord = {} mockGetModels.mockResolvedValue(mockModels) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index cac94aa0ce..d9372f0af5 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1485,8 +1485,44 @@ export const webviewMessageHandler = async ( break case "deleteCustomMode": if (message.slug) { + // Get the mode details to determine source and name + const customModes = await provider.customModesManager.getCustomModes() + const modeToDelete = customModes.find((mode) => mode.slug === message.slug) + + if (!modeToDelete) { + vscode.window.showErrorMessage(t("common:customModes.errors.modeNotFound")) + break + } + + // Check if rules folder exists + const workspacePath = getWorkspacePath() + let hasRulesFolder = false + + if (workspacePath) { + const rulesFolderPath = path.join(workspacePath, ".roo", `rules-${message.slug}`) + try { + hasRulesFolder = await fileExistsAtPath(rulesFolderPath) + } catch (error) { + // If we can't check the folder, fall back to standard confirmation + hasRulesFolder = false + } + } + + // Show appropriate confirmation dialog + let confirmationMessage: string + if (hasRulesFolder) { + const sourceText = modeToDelete.source === "project" ? "PROJECT" : "GLOBAL" + confirmationMessage = t("common:confirmation.delete_custom_mode_with_rules", { + source: sourceText, + modeName: modeToDelete.name, + slug: message.slug, + }) + } else { + confirmationMessage = t("common:confirmation.delete_custom_mode") + } + const answer = await vscode.window.showInformationMessage( - t("common:confirmation.delete_custom_mode"), + confirmationMessage, { modal: true }, t("common:answers.yes"), ) diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index 855a69c23b..d6fc3d136e 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -18,6 +18,7 @@ "reset_state": "Are you sure you want to reset all state and secret storage in the extension? This cannot be undone.", "delete_config_profile": "Are you sure you want to delete this configuration profile?", "delete_custom_mode": "Are you sure you want to delete this custom mode?", + "delete_custom_mode_with_rules": "Are you sure you want to delete this {{source}} mode '{{modeName}}'?\n\nNote: The associated rules folder at .roo/rules-{{slug}} will remain and must be deleted manually.", "delete_message": "What would you like to delete?", "just_this_message": "Just this message", "this_and_subsequent": "This and all subsequent messages"