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
This commit is contained in:
hannesrudolph 2025-07-07 13:26:34 -06:00
parent 53bb163883
commit 2ff969a76a
6 changed files with 105 additions and 523 deletions

View file

@ -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) {

View file

@ -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()

View file

@ -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

View file

@ -330,8 +330,8 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
setClineAsk("command")
setEnableButtons(!isPartial)
setPrimaryButtonText(t("chat:runCommand.title"))
setSecondaryButtonText(t("chat:addAndRunCommand.title"))
setTertiaryButtonText(t("chat:reject.title"))
setSecondaryButtonText(t("chat:reject.title"))
setTertiaryButtonText(undefined)
break
case "command_output":
setSendingDisabled(false)
@ -615,38 +615,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
[clineAsk, startNewTask],
)
const handleTertiaryButtonClick = useCallback(
(_text?: string, images?: string[]) => {
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<ChatViewRef, ChatViewPro
case "secondaryButtonClick":
handleSecondaryButtonClick(message.text ?? "", message.images ?? [])
break
case "tertiaryButtonClick":
handleTertiaryButtonClick(message.text ?? "", message.images ?? [])
break
}
break
case "condenseTaskContextResponse":
@ -796,7 +761,6 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
handleSetChatBoxMessage,
handlePrimaryButtonClick,
handleSecondaryButtonClick,
handleTertiaryButtonClick,
],
)
@ -1693,20 +1657,47 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
</StandardTooltip>
) : (
<>
{/* Three button layout for command approval */}
{tertiaryButtonText && clineAsk === "command" && !isStreaming ? (
{/* Command approval with auto-approve pattern */}
{clineAsk === "command" && !isStreaming ? (
<div className="flex flex-col gap-[6px]">
{/* Top row: Run and Add & Run */}
{/* Top row: Run Command and Reject */}
<div className="flex gap-[6px]">
<StandardTooltip content={t("chat:runCommand.tooltip")}>
<VSCodeButton
appearance="primary"
disabled={!enableButtons}
className="flex-1"
onClick={() => handlePrimaryButtonClick(inputValue, selectedImages)}>
onClick={() =>
handlePrimaryButtonClick(inputValue, selectedImages)
}>
{primaryButtonText}
</VSCodeButton>
</StandardTooltip>
<StandardTooltip content={t("chat:reject.tooltip")}>
<VSCodeButton
appearance="secondary"
disabled={!enableButtons}
className="flex-1"
onClick={() =>
handleSecondaryButtonClick(inputValue, selectedImages)
}>
{secondaryButtonText}
</VSCodeButton>
</StandardTooltip>
</div>
{/* Bottom row: Auto-approve pattern */}
<div className="flex items-center gap-[6px]">
<div className="flex-1 px-2 py-1 bg-vscode-input-background text-vscode-input-foreground rounded text-sm font-mono">
{(() => {
const commandMessage = findLast(
messagesRef.current,
(msg) => msg.type === "ask" && msg.ask === "command",
)
const commandText = commandMessage?.text || ""
const pattern = extractCommandPattern(commandText)
return pattern || commandText
})()}
</div>
<StandardTooltip
content={(() => {
const commandMessage = findLast(
@ -1717,28 +1708,36 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
const pattern = extractCommandPattern(commandText)
const description = getPatternDescription(pattern)
return pattern
? `${t("chat:addAndRunCommand.tooltip")} Will whitelist: "${pattern}" (${description})`
: t("chat:addAndRunCommand.tooltip")
? `${t("chat:alwaysAllow.tooltip")} Will whitelist: "${pattern}" (${description})`
: t("chat:alwaysAllow.tooltip")
})()}>
<VSCodeButton
appearance="secondary"
disabled={!enableButtons}
className="flex-1"
onClick={() => 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")}
</VSCodeButton>
</StandardTooltip>
</div>
{/* Bottom row: Reject */}
<StandardTooltip content={t("chat:reject.tooltip")}>
<VSCodeButton
appearance="secondary"
disabled={!enableButtons}
className="w-full"
onClick={() => handleSecondaryButtonClick(inputValue, selectedImages)}>
{tertiaryButtonText}
</VSCodeButton>
</StandardTooltip>
</div>
) : (
/* Standard two button layout */
@ -1754,23 +1753,33 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
? t("chat:approve.tooltip")
: primaryButtonText === t("chat:runCommand.title")
? t("chat:runCommand.tooltip")
: primaryButtonText === t("chat:startNewTask.title")
: primaryButtonText ===
t("chat:startNewTask.title")
? t("chat:startNewTask.tooltip")
: primaryButtonText === t("chat:resumeTask.title")
: primaryButtonText ===
t("chat:resumeTask.title")
? t("chat:resumeTask.tooltip")
: primaryButtonText ===
t("chat:proceedAnyways.title")
? t("chat:proceedAnyways.tooltip")
: primaryButtonText ===
t("chat:proceedWhileRunning.title")
? t("chat:proceedWhileRunning.tooltip")
t(
"chat:proceedWhileRunning.title",
)
? t(
"chat:proceedWhileRunning.tooltip",
)
: undefined
}>
<VSCodeButton
appearance="primary"
disabled={!enableButtons}
className={secondaryButtonText ? "flex-1 mr-[6px]" : "flex-[2] mr-0"}
onClick={() => handlePrimaryButtonClick(inputValue, selectedImages)}>
className={
secondaryButtonText ? "flex-1 mr-[6px]" : "flex-[2] mr-0"
}
onClick={() =>
handlePrimaryButtonClick(inputValue, selectedImages)
}>
{primaryButtonText}
</VSCodeButton>
</StandardTooltip>
@ -1792,7 +1801,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
appearance="secondary"
disabled={!enableButtons && !(isStreaming && !didClickCancel)}
className={isStreaming ? "flex-[2] ml-0" : "flex-1 ml-[6px]"}
onClick={() => handleSecondaryButtonClick(inputValue, selectedImages)}>
onClick={() =>
handleSecondaryButtonClick(inputValue, selectedImages)
}>
{isStreaming ? t("chat:cancel.title") : secondaryButtonText}
</VSCodeButton>
</StandardTooltip>

View file

@ -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) => (
<div data-testid="virtuoso">
{data?.map((item: any, index: number) => <div key={index}>{itemContent(index, item)}</div>)}
</div>
),
}))
// 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 <div data-testid="browser-session">{JSON.stringify(messages)}</div>
},
}))
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 (
<div data-testid="chat-row">
<div>Command: {message.text}</div>
</div>
)
}
return <div data-testid="chat-row">{JSON.stringify(message)}</div>
},
}))
vi.mock("../TaskHeader", () => ({
default: function MockTaskHeader({ task }: { task: any }) {
return <div data-testid="task-header">Task: {task.text}</div>
},
}))
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<string, string> = {
"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<string, string> = {
"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) => <span>{i18nKey}</span>,
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(
<ExtensionStateContextProvider>
<QueryClientProvider client={queryClient}>
<TooltipProvider>
<TranslationProvider>
<ChatView isHidden={false} showAnnouncement={false} hideAnnouncement={() => {}} />
</TranslationProvider>
</TooltipProvider>
</QueryClientProvider>
</ExtensionStateContextProvider>,
)
}
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: [],
})
})
})

View file

@ -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"