From 2ff969a76afd82b4af678b255cd50fe0da92d221 Mon Sep 17 00:00:00 2001 From: hannesrudolph Date: Mon, 7 Jul 2025 13:26:34 -0600 Subject: [PATCH] feat: redesign command approval UI with separate whitelist functionality - Remove three-button layout (Run, Add & Run, Reject) - Implement two-row design with Run Command/Reject buttons on top - Add pattern display and 'Always allow' button on bottom row - 'Always allow' only whitelists the pattern without running it - Remove addAndRunButtonClicked handling from backend - Add new addToWhitelist message type and handler - Update translations for new UI elements - Remove obsolete tests for old Add & Run functionality --- .../presentAssistantMessage.ts | 35 -- src/core/webview/webviewMessageHandler.ts | 24 + src/shared/WebviewMessage.ts | 2 + webview-ui/src/components/chat/ChatView.tsx | 139 +++--- .../__tests__/ChatView.addAndRun.spec.tsx | 424 ------------------ webview-ui/src/i18n/locales/en/chat.json | 4 + 6 files changed, 105 insertions(+), 523 deletions(-) delete mode 100644 webview-ui/src/components/chat/__tests__/ChatView.addAndRun.spec.tsx diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 3b11a60baa..ee3fa148b4 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -274,41 +274,6 @@ export async function presentAssistantMessage(cline: Task) { isProtected || false, ) - // Handle "Add & Run" button for command approval - if (response === "addAndRunButtonClicked" && type === "command") { - // The text field contains the extracted command pattern when "Add & Run" is clicked - if (text) { - // Add the command pattern to allowed commands - const provider = cline.providerRef.deref() - if (provider) { - // Get current allowed commands - const currentState = await provider.getState() - const currentAllowedCommands = currentState.allowedCommands || [] - - // Add the new pattern if it's not already in the list - if (!currentAllowedCommands.includes(text)) { - const updatedCommands = [...currentAllowedCommands, text] - - // Update global state using contextProxy - await provider.contextProxy.setValue("allowedCommands", updatedCommands) - - // Also update workspace settings - const vscode = await import("vscode") - const { Package } = await import("../../shared/package") - await vscode.workspace - .getConfiguration(Package.name) - .update("allowedCommands", updatedCommands, vscode.ConfigurationTarget.Global) - - // Post state update to webview - await provider.postStateToWebview() - } - } - } - - // Return true to indicate approval and continue with command execution - return true - } - if (response !== "yesButtonClicked") { // Handle both messageResponse and noButtonClicked with text. if (text) { diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index af6b04f9e6..c646224ba5 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -587,6 +587,30 @@ export const webviewMessageHandler = async ( break } + case "addToWhitelist": { + // Handle adding a command pattern to the whitelist without running it + if (message.pattern) { + // Get current allowed commands + const currentAllowedCommands = getGlobalState("allowedCommands") || [] + + // Add the new pattern if it's not already in the list + if (!currentAllowedCommands.includes(message.pattern)) { + const updatedCommands = [...currentAllowedCommands, message.pattern] + + // Update global state + await updateGlobalState("allowedCommands", updatedCommands) + + // Also update workspace settings + await vscode.workspace + .getConfiguration(Package.name) + .update("allowedCommands", updatedCommands, vscode.ConfigurationTarget.Global) + + // Post state update to webview + await provider.postStateToWebview() + } + } + break + } case "openCustomModesSettings": { const customModesFilePath = await provider.customModesManager.getCustomModesFilePath() diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 7de420e695..a261e4e711 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -197,6 +197,7 @@ export interface WebviewMessage { | "checkRulesDirectoryResult" | "saveCodeIndexSettingsAtomic" | "requestCodeIndexSecretStatus" + | "addToWhitelist" text?: string tab?: "settings" | "history" | "mcp" | "modes" | "chat" | "marketplace" | "account" disabled?: boolean @@ -238,6 +239,7 @@ export interface WebviewMessage { hasContent?: boolean // For checkRulesDirectoryResult checkOnly?: boolean // For deleteCustomMode check commandPattern?: string // For "Add & Run" button - the extracted command pattern to whitelist + pattern?: string // For "addToWhitelist" message - the command pattern to add to whitelist codeIndexSettings?: { // Global state settings codebaseIndexEnabled: boolean diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 4d4f484c97..c18d64bf9c 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -330,8 +330,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction { - switch (clineAsk) { - case "command": - // For the "Add & Run" button on command approval - // Extract the command pattern for whitelisting - const commandMessage = findLast( - messagesRef.current, - (msg) => msg.type === "ask" && msg.ask === "command", - ) - const commandText = commandMessage?.text || "" - const pattern = extractCommandPattern(commandText) - - // Send the pattern in the text field as expected by the backend - vscode.postMessage({ - type: "askResponse", - askResponse: "addAndRunButtonClicked", - text: pattern, // Send pattern in text field - images: images || [], - }) - // Clear input state after sending - setInputValue("") - setSelectedImages([]) - break - } - setSendingDisabled(true) - setClineAsk(undefined) - setEnableButtons(false) - }, - [clineAsk], // messagesRef is stable - ) - const handleSecondaryButtonClick = useCallback( (text?: string, images?: string[]) => { const trimmedInput = text?.trim() @@ -767,9 +735,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction ) : ( <> - {/* Three button layout for command approval */} - {tertiaryButtonText && clineAsk === "command" && !isStreaming ? ( + {/* Command approval with auto-approve pattern */} + {clineAsk === "command" && !isStreaming ? (
- {/* Top row: Run and Add & Run */} + {/* Top row: Run Command and Reject */}
handlePrimaryButtonClick(inputValue, selectedImages)}> + onClick={() => + handlePrimaryButtonClick(inputValue, selectedImages) + }> {primaryButtonText} + + + handleSecondaryButtonClick(inputValue, selectedImages) + }> + {secondaryButtonText} + + +
+ {/* Bottom row: Auto-approve pattern */} +
+
+ {(() => { + const commandMessage = findLast( + messagesRef.current, + (msg) => msg.type === "ask" && msg.ask === "command", + ) + const commandText = commandMessage?.text || "" + const pattern = extractCommandPattern(commandText) + return pattern || commandText + })()} +
{ const commandMessage = findLast( @@ -1717,28 +1708,36 @@ const ChatViewComponent: React.ForwardRefRenderFunction handleTertiaryButtonClick(inputValue, selectedImages)}> - {secondaryButtonText} + onClick={() => { + // Extract the command pattern + const commandMessage = findLast( + messagesRef.current, + (msg) => msg.type === "ask" && msg.ask === "command", + ) + const commandText = commandMessage?.text || "" + const pattern = extractCommandPattern(commandText) + + // Add to whitelist without running + vscode.postMessage({ + type: "addToWhitelist", + pattern: pattern, + }) + + // Clear the ask state + setSendingDisabled(true) + setClineAsk(undefined) + setEnableButtons(false) + }}> + {t("chat:alwaysAllow.title")}
- {/* Bottom row: Reject */} - - handleSecondaryButtonClick(inputValue, selectedImages)}> - {tertiaryButtonText} - -
) : ( /* Standard two button layout */ @@ -1754,23 +1753,33 @@ const ChatViewComponent: React.ForwardRefRenderFunction handlePrimaryButtonClick(inputValue, selectedImages)}> + className={ + secondaryButtonText ? "flex-1 mr-[6px]" : "flex-[2] mr-0" + } + onClick={() => + handlePrimaryButtonClick(inputValue, selectedImages) + }> {primaryButtonText} @@ -1792,7 +1801,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction handleSecondaryButtonClick(inputValue, selectedImages)}> + onClick={() => + handleSecondaryButtonClick(inputValue, selectedImages) + }> {isStreaming ? t("chat:cancel.title") : secondaryButtonText} diff --git a/webview-ui/src/components/chat/__tests__/ChatView.addAndRun.spec.tsx b/webview-ui/src/components/chat/__tests__/ChatView.addAndRun.spec.tsx deleted file mode 100644 index fe4a6a3d49..0000000000 --- a/webview-ui/src/components/chat/__tests__/ChatView.addAndRun.spec.tsx +++ /dev/null @@ -1,424 +0,0 @@ -import React from "react" -import { render, screen, fireEvent, waitFor } from "@testing-library/react" -import { vi } from "vitest" -import ChatView from "../ChatView" -import { ExtensionStateContextProvider } from "@src/context/ExtensionStateContext" -import { TranslationProvider } from "@src/i18n/TranslationContext" -import { QueryClient, QueryClientProvider } from "@tanstack/react-query" -import { TooltipProvider } from "@/components/ui/tooltip" -import { vscode } from "@src/utils/vscode" - -// Mock vscode API -vi.mock("@src/utils/vscode", () => ({ - vscode: { - postMessage: vi.fn(), - }, -})) - -// Mock extract-command-pattern -vi.mock("@src/utils/extract-command-pattern", () => ({ - extractCommandPattern: vi.fn((command: string) => { - // Simple mock implementation - if (command === "npm test") return "npm test" - if (command === "cd /path/to/project && npm run build:prod --verbose") return "cd * && npm run *" - return command - }), - getPatternDescription: vi.fn(() => "matches similar commands"), -})) - -// Mock use-sound -vi.mock("use-sound", () => ({ - default: () => [vi.fn()], -})) - -// Mock react-use -vi.mock("react-use", () => ({ - useEvent: vi.fn(), - useMount: vi.fn(), - useDeepCompareEffect: (fn: () => void, deps: any[]) => { - // Use regular useEffect for testing - // eslint-disable-next-line @typescript-eslint/no-require-imports - const React = require("react") - // eslint-disable-next-line react-hooks/exhaustive-deps - React.useEffect(fn, deps) - }, - useWindowSize: () => ({ width: 1024, height: 768 }), -})) - -// Mock debounce -vi.mock("debounce", () => ({ - default: (fn: any) => fn, -})) - -// Mock react-virtuoso -vi.mock("react-virtuoso", () => ({ - Virtuoso: ({ data, itemContent }: any) => ( -
- {data?.map((item: any, index: number) =>
{itemContent(index, item)}
)} -
- ), -})) - -// Mock all problematic dependencies -vi.mock("rehype-highlight", () => ({ - default: () => () => {}, -})) - -vi.mock("hast-util-to-text", () => ({ - default: () => "", -})) - -// Mock components that use ESM dependencies -vi.mock("../BrowserSessionRow", () => ({ - default: function MockBrowserSessionRow({ messages }: { messages: any[] }) { - return
{JSON.stringify(messages)}
- }, -})) - -vi.mock("../ChatRow", () => ({ - default: function MockChatRow({ message }: { message: any }) { - // Render the buttons if this is a command ask message - if (message.type === "ask" && message.ask === "command") { - return ( -
-
Command: {message.text}
-
- ) - } - return
{JSON.stringify(message)}
- }, -})) - -vi.mock("../TaskHeader", () => ({ - default: function MockTaskHeader({ task }: { task: any }) { - return
Task: {task.text}
- }, -})) - -vi.mock("../AutoApproveMenu", () => ({ - default: () => null, -})) - -vi.mock("@src/components/common/CodeBlock", () => ({ - default: () => null, - CODE_BLOCK_BG_COLOR: "rgb(30, 30, 30)", -})) - -vi.mock("@src/components/common/CodeAccordian", () => ({ - default: () => null, -})) - -vi.mock("@src/components/chat/ContextMenu", () => ({ - default: () => null, -})) - -// Mock i18n setup -vi.mock("@src/i18n/setup", () => { - const mockT = (key: string, _options?: any) => { - const translations: Record = { - "chat:runCommand.title": "Run Command", - "chat:addAndRunCommand.title": "Add & Run", - "chat:reject.title": "Reject", - "chat:typeMessage": "Type a message...", - "chat:typeTask": "Type a task...", - } - return translations[key] || key - } - - const mockI18n = { - language: "en", - changeLanguage: vi.fn(), - t: mockT, - use: vi.fn().mockReturnThis(), - init: vi.fn().mockReturnThis(), - } - - return { - default: mockI18n, - loadTranslations: vi.fn(), - } -}) - -// Mock react-i18next -vi.mock("react-i18next", () => { - const mockT = (key: string, _options?: any) => { - const translations: Record = { - "chat:runCommand.title": "Run Command", - "chat:addAndRunCommand.title": "Add & Run", - "chat:reject.title": "Reject", - "chat:typeMessage": "Type a message...", - "chat:typeTask": "Type a task...", - } - return translations[key] || key - } - - const mockI18n = { - language: "en", - changeLanguage: vi.fn(), - t: mockT, - use: vi.fn().mockReturnThis(), - init: vi.fn().mockReturnThis(), - } - - return { - useTranslation: () => ({ - t: mockT, - i18n: mockI18n, - }), - Trans: ({ i18nKey, children: _children }: any) => {i18nKey}, - initReactI18next: { - type: "3rdParty", - init: vi.fn(), - }, - } -}) - -// Mock window.postMessage to trigger state hydration -const mockPostMessage = (state: any) => { - window.postMessage( - { - type: "state", - state: { - version: "1.0.0", - clineMessages: [], - taskHistory: [], - shouldShowAnnouncement: false, - allowedCommands: [], - alwaysAllowExecute: false, - autoApprovalEnabled: false, - alwaysAllowBrowser: false, - alwaysAllowReadOnly: false, - alwaysAllowReadOnlyOutsideWorkspace: false, - alwaysAllowWrite: false, - alwaysAllowWriteOutsideWorkspace: false, - alwaysAllowWriteProtected: false, - alwaysAllowMcp: false, - alwaysAllowModeSwitch: false, - alwaysAllowSubtasks: false, - writeDelayMs: 0, - mode: "code", - customModes: [], - telemetrySetting: "enabled", - hasSystemPromptOverride: false, - historyPreviewCollapsed: false, - soundEnabled: false, - soundVolume: 0.5, - cwd: "/test", - filePaths: [], - openedTabs: [], - currentApiConfigName: "test-config", - listApiConfigMeta: [], - pinnedApiConfigs: {}, - customModePrompts: {}, - codebaseIndexConfig: { codebaseIndexEnabled: false }, - ...state, - }, - }, - "*", - ) -} - -describe("ChatView - Add & Run Button", () => { - const queryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - }, - }, - }) - - const renderChatView = () => { - return render( - - - - - {}} /> - - - - , - ) - } - - beforeEach(() => { - vi.clearAllMocks() - // Mock window.AUDIO_BASE_URI - ;(window as any).AUDIO_BASE_URI = "" - }) - - it("should display three buttons for command approval", async () => { - renderChatView() - - // Hydrate state with a task message first, then a command ask - mockPostMessage({ - clineMessages: [ - { - type: "say", - say: "task", - ts: Date.now() - 2000, - text: "Initial task", - partial: false, - }, - { - type: "ask", - ask: "command", - ts: Date.now(), - text: "npm test", - partial: false, - }, - ], - }) - - // Wait for the buttons to appear - await waitFor( - () => { - expect(screen.getByText("Run Command")).toBeInTheDocument() - }, - { timeout: 5000 }, - ) - - expect(screen.getByText("Add & Run")).toBeInTheDocument() - expect(screen.getByText("Reject")).toBeInTheDocument() - }) - - it("should send the command pattern when Add & Run button is clicked", async () => { - renderChatView() - - // Hydrate state with a task message first, then a command ask - mockPostMessage({ - clineMessages: [ - { - type: "say", - say: "task", - ts: Date.now() - 2000, - text: "Initial task", - partial: false, - }, - { - type: "ask", - ask: "command", - ts: Date.now(), - text: "npm test", - partial: false, - }, - ], - }) - - // Wait for the buttons to appear - await waitFor( - () => { - expect(screen.getByText("Run Command")).toBeInTheDocument() - expect(screen.getByText("Add & Run")).toBeInTheDocument() - expect(screen.getByText("Reject")).toBeInTheDocument() - }, - { timeout: 5000 }, - ) - - // Click the Add & Run button - const addAndRunButton = screen.getByText("Add & Run") - fireEvent.click(addAndRunButton) - - // Verify the correct message was sent - expect(vscode.postMessage).toHaveBeenCalledWith({ - type: "askResponse", - askResponse: "addAndRunButtonClicked", - text: "npm test", // The extracted pattern - images: [], - }) - }) - - it("should extract and send the correct pattern for complex commands", async () => { - const complexCommand = "cd /path/to/project && npm run build:prod --verbose" - - renderChatView() - - // Hydrate state with a task message first, then a command ask - mockPostMessage({ - clineMessages: [ - { - type: "say", - say: "task", - ts: Date.now() - 2000, - text: "Build project", - partial: false, - }, - { - type: "ask", - ask: "command", - ts: Date.now(), - text: complexCommand, - partial: false, - }, - ], - }) - - // Wait for the buttons to appear - await waitFor( - () => { - expect(screen.getByText("Add & Run")).toBeInTheDocument() - }, - { timeout: 5000 }, - ) - - // Click the Add & Run button - const addAndRunButton = screen.getByText("Add & Run") - fireEvent.click(addAndRunButton) - - // Verify the correct pattern was extracted and sent - expect(vscode.postMessage).toHaveBeenCalledWith({ - type: "askResponse", - askResponse: "addAndRunButtonClicked", - text: "cd * && npm run *", // The extracted pattern - images: [], - }) - }) - - it("should handle commands with user input", async () => { - renderChatView() - - // Hydrate state with a task message first, then a command ask - mockPostMessage({ - clineMessages: [ - { - type: "say", - say: "task", - ts: Date.now() - 2000, - text: "Initial task", - partial: false, - }, - { - type: "ask", - ask: "command", - ts: Date.now(), - text: "npm test", - partial: false, - }, - ], - }) - - // Wait for the buttons and input to appear - await waitFor( - () => { - expect(screen.getByText("Add & Run")).toBeInTheDocument() - }, - { timeout: 5000 }, - ) - - // Type some user input - const textarea = screen.getByPlaceholderText(/Type a message/i) - fireEvent.change(textarea, { target: { value: "additional feedback" } }) - - // Click the Add & Run button - const addAndRunButton = screen.getByText("Add & Run") - fireEvent.click(addAndRunButton) - - // Verify the pattern was sent (not the user input) - expect(vscode.postMessage).toHaveBeenCalledWith({ - type: "askResponse", - askResponse: "addAndRunButtonClicked", - text: "npm test", // The extracted pattern, not the user input - images: [], - }) - }) -}) diff --git a/webview-ui/src/i18n/locales/en/chat.json b/webview-ui/src/i18n/locales/en/chat.json index cee58aedca..cf8d4c8a4b 100644 --- a/webview-ui/src/i18n/locales/en/chat.json +++ b/webview-ui/src/i18n/locales/en/chat.json @@ -71,6 +71,10 @@ "title": "Add & Run", "tooltip": "Add command pattern to whitelist and run" }, + "alwaysAllow": { + "title": "Always allow", + "tooltip": "Add this command pattern to the whitelist" + }, "proceedWhileRunning": { "title": "Proceed While Running", "tooltip": "Continue despite warnings"