refactor: simplify command allow/deny implementation

- Remove redundant allowCommand/denyCommand message types from WebviewMessage
- Remove corresponding handlers from webviewMessageHandler
- Remove unused test file for allowCommand functionality
- Remove unused i18n keys for command_allowed and command_denied
- Simplify to use existing allowedCommands/deniedCommands infrastructure
This commit is contained in:
hannesrudolph 2025-07-15 17:27:34 -06:00
parent 2f321e98ce
commit 83b6f3cc80
4 changed files with 1 additions and 268 deletions

View file

@ -1,216 +0,0 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
import * as vscode from "vscode"
import { webviewMessageHandler } from "../webviewMessageHandler"
import { ClineProvider } from "../ClineProvider"
import { Package } from "../../../shared/package"
import { t } from "../../../i18n"
// Mock vscode module
vi.mock("vscode", () => ({
window: {
showInformationMessage: vi.fn(),
showErrorMessage: vi.fn(),
},
workspace: {
getConfiguration: vi.fn(),
},
ConfigurationTarget: {
Global: 1,
},
}))
// Mock i18n
vi.mock("../../../i18n", () => ({
t: vi.fn((key: string, params?: any) => {
if (key === "common:info.command_allowed" && params?.pattern) {
return `Command pattern "${params.pattern}" has been allowed`
}
return key
}),
}))
// Mock Package
vi.mock("../../../shared/package", () => ({
Package: {
name: "roo-code",
},
}))
describe("webviewMessageHandler - allowCommand", () => {
let mockProvider: any
let mockContextProxy: any
let mockConfigUpdate: any
beforeEach(() => {
vi.clearAllMocks()
// Setup mock for workspace configuration
mockConfigUpdate = vi.fn().mockResolvedValue(undefined)
vi.mocked(vscode.workspace.getConfiguration).mockReturnValue({
update: mockConfigUpdate,
} as any)
// Create mock context proxy
mockContextProxy = {
getValue: vi.fn(),
setValue: vi.fn(),
}
// Create mock provider
mockProvider = {
contextProxy: mockContextProxy,
postStateToWebview: vi.fn().mockResolvedValue(undefined),
log: vi.fn(),
} as any
})
afterEach(() => {
vi.restoreAllMocks()
})
it("should add a new command pattern to the allowed commands list", async () => {
// Setup initial state
mockContextProxy.getValue.mockReturnValue(["npm test", "git status"])
// Create message
const message = {
type: "allowCommand",
pattern: "npm run build",
}
// Call handler
await webviewMessageHandler(mockProvider, message as any)
// Verify the pattern was added
expect(mockContextProxy.setValue).toHaveBeenCalledWith("allowedCommands", [
"npm test",
"git status",
"npm run build",
])
// Verify user was notified
expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(
'Command pattern "npm run build" has been allowed',
)
// Verify state was posted to webview
expect(mockProvider.postStateToWebview).toHaveBeenCalled()
})
it("should not add duplicate patterns", async () => {
// Setup initial state with existing pattern
mockContextProxy.getValue.mockReturnValue(["npm test", "git status", "npm run build"])
// Create message with duplicate pattern
const message = {
type: "allowCommand",
pattern: "npm run build",
}
// Call handler
await webviewMessageHandler(mockProvider, message as any)
// Verify setValue was NOT called (no update needed)
expect(mockContextProxy.setValue).not.toHaveBeenCalled()
// Verify user was NOT notified
expect(vscode.window.showInformationMessage).not.toHaveBeenCalled()
// Verify state was NOT posted to webview
expect(mockProvider.postStateToWebview).not.toHaveBeenCalled()
})
it("should handle empty allowed commands list", async () => {
// Setup with no existing commands
mockContextProxy.getValue.mockReturnValue(undefined)
// Create message
const message = {
type: "allowCommand",
pattern: "echo 'Hello, World!'",
}
// Call handler
await webviewMessageHandler(mockProvider, message as any)
// Verify the pattern was added as the first item
expect(mockContextProxy.setValue).toHaveBeenCalledWith("allowedCommands", ["echo 'Hello, World!'"])
// Verify user was notified
expect(vscode.window.showInformationMessage).toHaveBeenCalledWith(
`Command pattern "echo 'Hello, World!'" has been allowed`,
)
})
it("should filter out invalid commands", async () => {
// Setup with some invalid commands
mockContextProxy.getValue.mockReturnValue(["npm test", "", " ", null, "git status"])
// Create message
const message = {
type: "allowCommand",
pattern: "npm run dev",
}
// Call handler
await webviewMessageHandler(mockProvider, message as any)
// Verify only valid commands were kept
expect(mockContextProxy.setValue).toHaveBeenCalledWith("allowedCommands", [
"npm test",
"git status",
"npm run dev",
])
})
it("should handle missing pattern gracefully", async () => {
// Create message without pattern
const message = {
type: "allowCommand",
}
// Call handler
await webviewMessageHandler(mockProvider, message as any)
// Verify nothing was updated
expect(mockContextProxy.setValue).not.toHaveBeenCalled()
expect(vscode.window.showInformationMessage).not.toHaveBeenCalled()
expect(mockProvider.postStateToWebview).not.toHaveBeenCalled()
})
it("should handle non-string pattern gracefully", async () => {
// Create message with non-string pattern
const message = {
type: "allowCommand",
pattern: 123, // Invalid type
}
// Call handler
await webviewMessageHandler(mockProvider, message as any)
// Verify nothing was updated
expect(mockContextProxy.setValue).not.toHaveBeenCalled()
expect(vscode.window.showInformationMessage).not.toHaveBeenCalled()
expect(mockProvider.postStateToWebview).not.toHaveBeenCalled()
})
it("should handle complex command patterns with special characters", async () => {
// Setup initial state
mockContextProxy.getValue.mockReturnValue(["npm test"])
// Create message with complex pattern
const message = {
type: "allowCommand",
pattern: 'echo "Hello, World!" && echo $HOME',
}
// Call handler
await webviewMessageHandler(mockProvider, message as any)
// Verify the pattern was added correctly
expect(mockContextProxy.setValue).toHaveBeenCalledWith("allowedCommands", [
"npm test",
'echo "Hello, World!" && echo $HOME',
])
})
})

View file

@ -771,29 +771,6 @@ export const webviewMessageHandler = async (
break
}
case "allowCommand": {
// Add a command pattern to the allowed commands list
if (message.pattern && typeof message.pattern === "string") {
const currentCommands = getGlobalState("allowedCommands") ?? []
const validCommands = Array.isArray(currentCommands)
? currentCommands.filter((cmd) => typeof cmd === "string" && cmd.trim().length > 0)
: []
// Add the new pattern if it's not already in the list
if (!validCommands.includes(message.pattern)) {
validCommands.push(message.pattern)
await updateGlobalState("allowedCommands", validCommands)
// Show confirmation to the user
vscode.window.showInformationMessage(t("common:info.command_allowed", { pattern: message.pattern }))
// Update the webview state
await provider.postStateToWebview()
}
}
break
}
case "deniedCommands": {
// Validate and sanitize the commands array
const commands = message.commands ?? []
@ -805,29 +782,6 @@ export const webviewMessageHandler = async (
break
}
case "denyCommand": {
// Add a command pattern to the denied commands list
if (message.pattern && typeof message.pattern === "string") {
const currentCommands = getGlobalState("deniedCommands") ?? []
const validCommands = Array.isArray(currentCommands)
? currentCommands.filter((cmd) => typeof cmd === "string" && cmd.trim().length > 0)
: []
// Add the new pattern if it's not already in the list
if (!validCommands.includes(message.pattern)) {
validCommands.push(message.pattern)
await updateGlobalState("deniedCommands", validCommands)
// Show confirmation to the user
vscode.window.showInformationMessage(t("common:info.command_denied", { pattern: message.pattern }))
// Update the webview state
await provider.postStateToWebview()
}
}
break
}
case "openCustomModesSettings": {
const customModesFilePath = await provider.customModesManager.getCustomModesFilePath()

View file

@ -102,9 +102,7 @@
"image_copied_to_clipboard": "Image data URI copied to clipboard",
"image_saved": "Image saved to {{path}}",
"mode_exported": "Mode '{{mode}}' exported successfully",
"mode_imported": "Mode imported successfully",
"command_allowed": "Command pattern '{{pattern}}' has been added to the allowed commands list",
"command_denied": "Command pattern '{{pattern}}' has been added to the denied commands list"
"mode_imported": "Mode imported successfully"
},
"answers": {
"yes": "Yes",

View file

@ -36,9 +36,7 @@ export interface WebviewMessage {
| "getListApiConfiguration"
| "customInstructions"
| "allowedCommands"
| "allowCommand"
| "deniedCommands"
| "denyCommand"
| "alwaysAllowReadOnly"
| "alwaysAllowReadOnlyOutsideWorkspace"
| "alwaysAllowWrite"
@ -237,7 +235,6 @@ export interface WebviewMessage {
visibility?: ShareVisibility // For share visibility
hasContent?: boolean // For checkRulesDirectoryResult
checkOnly?: boolean // For deleteCustomMode check
pattern?: string // For allowCommand
codeIndexSettings?: {
// Global state settings
codebaseIndexEnabled: boolean