diff --git a/command_whitelist_ui_location_summary.md b/command_whitelist_ui_location_summary.md new file mode 100644 index 0000000000..ade4fdabb4 --- /dev/null +++ b/command_whitelist_ui_location_summary.md @@ -0,0 +1,67 @@ +# Command Whitelisting UI Location Summary + +## Overview + +The command whitelisting feature has been successfully moved from VS Code's native settings to the Roo Code plugin's settings interface. This consolidates all auto-approval settings in one convenient location. + +## Previous Location (REMOVED) + +- **VS Code Settings**: `Preferences > Settings > Extensions > Roo Code` +- **Setting Name**: `roo-code.commandWhitelist` +- **Access**: Required navigating through VS Code's settings UI or editing `settings.json` + +## New Location (CURRENT) + +The command whitelisting feature is now located in: + +### Access Path + +1. Open the Roo Code extension panel in VS Code +2. Click on the **Settings** icon (gear icon) in the top toolbar +3. Navigate to the **Auto Approve** section +4. Find the **Execute** subsection + +### UI Components + +Within the Auto Approve > Execute section, you'll find: + +1. **Enable/Disable Toggle** + + - Label: "Auto-approve command execution" + - Controls whether commands can be auto-approved + +2. **Command Patterns List** + - Label: "Command patterns" + - Description: "Add command patterns that can be auto-approved (e.g., 'npm test', 'git status')" + - Features: + - Add new patterns using the input field + - Remove patterns with the × button + - Patterns support wildcards (\*) + - Empty list means no commands are auto-approved + +### Example Patterns + +- `npm test` - Auto-approves exact command +- `npm *` - Auto-approves any npm command +- `git status` - Auto-approves git status command +- `*` - Auto-approves all commands (use with caution) + +## Migration + +- Existing command whitelist settings from VS Code settings are automatically migrated to the new location on first launch +- The old VS Code setting (`roo-code.commandWhitelist`) is removed from `package.json` +- Users don't need to manually transfer their settings + +## Benefits + +1. **Centralized Settings**: All auto-approval settings (read, write, execute) are now in one place +2. **Better UX**: No need to navigate VS Code's complex settings structure +3. **Visual Consistency**: Matches the UI pattern of other auto-approve settings +4. **Easier Discovery**: Users can find all related settings together + +## Technical Details + +- Setting is stored in the global state using key: `commandWhitelist` +- Synchronized across VS Code instances +- Supports the same pattern matching as before +- Maintains backward compatibility through automatic migration diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index 270891920c..514b15d783 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -48,7 +48,6 @@ export const globalSettingsSchema = z.object({ alwaysAllowFollowupQuestions: z.boolean().optional(), followupAutoApproveTimeoutMs: z.number().optional(), alwaysAllowUpdateTodoList: z.boolean().optional(), - disableLlmCommandSuggestions: z.boolean().optional(), allowedCommands: z.array(z.string()).optional(), deniedCommands: z.array(z.string()).optional(), allowedMaxRequests: z.number().nullish(), diff --git a/src/core/prompts/tools/__tests__/execute-command.spec.ts b/src/core/prompts/tools/__tests__/execute-command.spec.ts index 386b8c0fb2..e35ca38e22 100644 --- a/src/core/prompts/tools/__tests__/execute-command.spec.ts +++ b/src/core/prompts/tools/__tests__/execute-command.spec.ts @@ -8,26 +8,7 @@ describe("getExecuteCommandDescription", () => { supportsComputerUse: false, } - it("should include suggestions section when disableLlmCommandSuggestions is false", () => { - const args: ToolArgs = { - ...baseArgs, - settings: { - disableLlmCommandSuggestions: false, - }, - } - - const description = getExecuteCommandDescription(args) - - // Check that the description includes the suggestions parameter - expect(description).toContain("") - expect(description).toContain("- suggestions: (optional) Command patterns for the user to allow/deny") - expect(description).toContain("Suggestion Guidelines") - // Check for chained command guidance - expect(description).toContain("For chained commands") - expect(description).toContain("cd backend && npm install") - }) - - it("should include suggestions section when disableLlmCommandSuggestions is not set", () => { + it("should not include suggestions section", () => { const args: ToolArgs = { ...baseArgs, settings: {}, @@ -35,34 +16,17 @@ describe("getExecuteCommandDescription", () => { const description = getExecuteCommandDescription(args) - // Check that the description includes the suggestions parameter - expect(description).toContain("") - expect(description).toContain("- suggestions: (optional) Command patterns for the user to allow/deny") - expect(description).toContain("Suggestion Guidelines") - }) - - it("should exclude suggestions section when disableLlmCommandSuggestions is true", () => { - const args: ToolArgs = { - ...baseArgs, - settings: { - disableLlmCommandSuggestions: true, - }, - } - - const description = getExecuteCommandDescription(args) - // Check that the description does NOT include the suggestions parameter expect(description).not.toContain("") expect(description).not.toContain("- suggestions: (optional) Command patterns for the user to allow/deny") expect(description).not.toContain("Suggestion Guidelines") + expect(description).not.toContain("For chained commands") }) - it("should include basic command and cwd parameters regardless of settings", () => { + it("should include basic command and cwd parameters", () => { const args: ToolArgs = { ...baseArgs, - settings: { - disableLlmCommandSuggestions: true, - }, + settings: {}, } const description = getExecuteCommandDescription(args) @@ -71,5 +35,21 @@ describe("getExecuteCommandDescription", () => { expect(description).toContain("- command: (required)") expect(description).toContain("- cwd: (optional)") expect(description).toContain("execute_command") + expect(description).toContain("/test/path") + }) + + it("should include usage examples", () => { + const args: ToolArgs = { + ...baseArgs, + settings: {}, + } + + const description = getExecuteCommandDescription(args) + + // Check that usage examples are included + expect(description).toContain("Usage:") + expect(description).toContain("") + expect(description).toContain("Example: Requesting to execute npm run dev") + expect(description).toContain("Example: Requesting to execute ls in a specific directory") }) }) diff --git a/src/core/prompts/tools/execute-command.ts b/src/core/prompts/tools/execute-command.ts index 45c714c0d7..4857d6ff59 100644 --- a/src/core/prompts/tools/execute-command.ts +++ b/src/core/prompts/tools/execute-command.ts @@ -1,19 +1,12 @@ import { ToolArgs } from "./types" export function getExecuteCommandDescription(args: ToolArgs): string | undefined { - const disableLlmSuggestions = args.settings?.disableLlmCommandSuggestions ?? false - - const baseDescription = `## execute_command + return `## execute_command Description: Request to execute a CLI command on the system. Use this when you need to perform system operations or run specific commands to accomplish any step in the user's task. You must tailor your command to the user's system and provide a clear explanation of what the command does. For command chaining, use the appropriate chaining syntax for the user's shell. Prefer to execute complex CLI commands over creating executable scripts, as they are more flexible and easier to run. Prefer relative commands and paths that avoid location sensitivity for terminal consistency, e.g: \`touch ./testdata/example.file\`, \`dir ./examples/model1/data/yaml\`, or \`go test ./cmd/front --config ./cmd/front/config.yml\`. If directed by the user, you may open a terminal in a different directory by using the \`cwd\` parameter. Parameters: - command: (required) The CLI command to execute. This should be valid for the current operating system. Ensure the command is properly formatted and does not contain any harmful instructions. -- cwd: (optional) The working directory to execute the command in (default: ${args.cwd})` - - if (disableLlmSuggestions) { - return ( - baseDescription + - ` +- cwd: (optional) The working directory to execute the command in (default: ${args.cwd}) Usage: @@ -31,58 +24,4 @@ Example: Requesting to execute ls in a specific directory ls -la /home/user/projects ` - ) - } - - return ( - baseDescription + - ` -- suggestions: (optional) Command patterns for the user to allow/deny for future auto-approval. Use tags. - -**Suggestion Guidelines:** -- Suggestions use prefix matching (case-insensitive) -- For simple commands: Include the base command (e.g., "npm", "git") and optionally a more specific pattern -- For chained commands (using &&, ||, ;, |): Include patterns for EACH individual command in the chain (NOT the full chain) - - Example: For "cd backend && npm install", suggest: "cd", "npm install", "npm" -- Include 2-4 relevant patterns total -- Only suggest "*" (allow all) if explicitly requested by the user - -Usage: - -Your command here -Working directory path (optional) - -pattern 1 -pattern 2 - - - -Example: Requesting to execute npm run dev - -npm run dev - -npm run -npm - - - -Example: Requesting to execute a chained command - -cd backend && npm install - -cd -npm install -npm - - - -Example: Requesting to execute ls in a specific directory - -ls -la -/home/user/projects - -ls - -` - ) } diff --git a/src/core/tools/__tests__/executeCommandTool.spec.ts b/src/core/tools/__tests__/executeCommandTool.spec.ts index c48fa7e3b0..c5b6ce6006 100644 --- a/src/core/tools/__tests__/executeCommandTool.spec.ts +++ b/src/core/tools/__tests__/executeCommandTool.spec.ts @@ -580,267 +580,4 @@ docker run -d nginx expect(mockPushToolResult).toHaveBeenCalledWith("Command executed") }) }) - - describe("disableLlmCommandSuggestions setting", () => { - beforeEach(() => { - // Reset the workspace configuration mock - vitest.clearAllMocks() - }) - - it("should ignore suggestions when disableLlmCommandSuggestions is true", async () => { - // Setup - mock the workspace configuration to return true for disableLlmCommandSuggestions - const mockConfig = { - get: vitest.fn().mockImplementation((key: string, defaultValue?: any) => { - if (key === "disableLlmCommandSuggestions") { - return true - } - return defaultValue - }), - } - ;(vscode.workspace.getConfiguration as any).mockReturnValue(mockConfig) - - // Override the mock implementation to check for the setting - ;(executeCommandTool as any).mockImplementation( - async (cline: any, block: any, askApproval: any, handleError: any, pushToolResult: any) => { - // Check if disableLlmCommandSuggestions is enabled - const config = vscode.workspace.getConfiguration("roo-cline") - const disableSuggestions = config.get("disableLlmCommandSuggestions", false) - - let commandToApprove = block.params.command - // Only process suggestions if the setting is disabled - if (!disableSuggestions && block.params.suggestions) { - commandToApprove = `${block.params.command}\n\n${block.params.suggestions}\n` - } - - const didApprove = await askApproval("command", commandToApprove) - if (!didApprove) { - return - } - - pushToolResult("Command executed") - }, - ) - - // Setup tool use with suggestions - mockToolUse.params.command = "npm install" - mockToolUse.params.suggestions = JSON.stringify([ - "npm install --save", - "npm install --save-dev", - "npm install --global", - ]) - - // Execute - await executeCommandTool( - mockCline as unknown as Task, - mockToolUse, - mockAskApproval as unknown as AskApproval, - mockHandleError as unknown as HandleError, - mockPushToolResult as unknown as PushToolResult, - mockRemoveClosingTag as unknown as RemoveClosingTag, - ) - - // Verify - should pass command WITHOUT suggestions - expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith("roo-cline") - expect(mockConfig.get).toHaveBeenCalledWith("disableLlmCommandSuggestions", false) - expect(mockAskApproval).toHaveBeenCalledWith("command", "npm install") - expect(mockPushToolResult).toHaveBeenCalledWith("Command executed") - }) - - it("should process suggestions when disableLlmCommandSuggestions is false", async () => { - // Setup - mock the workspace configuration to return false for disableLlmCommandSuggestions - const mockConfig = { - get: vitest.fn().mockImplementation((key: string, defaultValue?: any) => { - if (key === "disableLlmCommandSuggestions") { - return false - } - return defaultValue - }), - } - ;(vscode.workspace.getConfiguration as any).mockReturnValue(mockConfig) - - // Override the mock implementation to check for the setting - ;(executeCommandTool as any).mockImplementation( - async (cline: any, block: any, askApproval: any, handleError: any, pushToolResult: any) => { - // Check if disableLlmCommandSuggestions is enabled - const config = vscode.workspace.getConfiguration("roo-cline") - const disableSuggestions = config.get("disableLlmCommandSuggestions", false) - - let commandToApprove = block.params.command - // Only process suggestions if the setting is disabled - if (!disableSuggestions && block.params.suggestions) { - // Parse suggestions if they're a JSON string - let suggestions = block.params.suggestions - if (typeof suggestions === "string" && suggestions.trim().startsWith("[")) { - try { - suggestions = JSON.parse(suggestions) - } catch (e) { - // Keep as string if parsing fails - } - } - if (Array.isArray(suggestions)) { - commandToApprove = `${block.params.command}\n\n${suggestions.join("\n")}\n` - } - } - - const didApprove = await askApproval("command", commandToApprove) - if (!didApprove) { - return - } - - pushToolResult("Command executed") - }, - ) - - // Setup tool use with suggestions - mockToolUse.params.command = "npm install" - mockToolUse.params.suggestions = JSON.stringify([ - "npm install --save", - "npm install --save-dev", - "npm install --global", - ]) - - // Execute - await executeCommandTool( - mockCline as unknown as Task, - mockToolUse, - mockAskApproval as unknown as AskApproval, - mockHandleError as unknown as HandleError, - mockPushToolResult as unknown as PushToolResult, - mockRemoveClosingTag as unknown as RemoveClosingTag, - ) - - // Verify - should pass command WITH suggestions - const expectedCommandWithSuggestions = `npm install - -npm install --save -npm install --save-dev -npm install --global -` - expect(vscode.workspace.getConfiguration).toHaveBeenCalledWith("roo-cline") - expect(mockConfig.get).toHaveBeenCalledWith("disableLlmCommandSuggestions", false) - expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions) - expect(mockPushToolResult).toHaveBeenCalledWith("Command executed") - }) - - it("should process suggestions when disableLlmCommandSuggestions is not set (default behavior)", async () => { - // Setup - mock the workspace configuration to return undefined for disableLlmCommandSuggestions - const mockConfig = { - get: vitest.fn().mockImplementation((key: string, defaultValue?: any) => { - // Return the default value (undefined becomes false) - return defaultValue - }), - } - ;(vscode.workspace.getConfiguration as any).mockReturnValue(mockConfig) - - // Restore the original mock implementation from beforeEach - // Use the original mock implementation from the top-level beforeEach - ;(executeCommandTool as any).mockImplementation( - async (cline: any, block: any, askApproval: any, handleError: any, pushToolResult: any) => { - if (!block.params.command) { - cline.consecutiveMistakeCount++ - cline.recordToolError("execute_command") - const errorMessage = await cline.sayAndCreateMissingParamError("execute_command", "command") - pushToolResult(errorMessage) - return - } - - const ignoredFileAttemptedToAccess = cline.rooIgnoreController?.validateCommand( - block.params.command, - ) - if (ignoredFileAttemptedToAccess) { - await cline.say("rooignore_error", ignoredFileAttemptedToAccess) - const mockRooIgnoreError = "RooIgnore error" - ;(formatResponse.rooIgnoreError as any).mockReturnValue(mockRooIgnoreError) - ;(formatResponse.toolError as any).mockReturnValue("Tool error") - formatResponse.rooIgnoreError(ignoredFileAttemptedToAccess) - formatResponse.toolError(mockRooIgnoreError) - pushToolResult("Tool error") - return - } - - // Handle suggestions if provided - let commandWithSuggestions = block.params.command - if (block.params.suggestions) { - let suggestions = block.params.suggestions - // Handle both array and string formats - if (typeof suggestions === "string") { - const suggestionsString = suggestions - - // First try to parse as JSON array - if (suggestionsString.trim().startsWith("[")) { - try { - suggestions = JSON.parse(suggestionsString) - } catch (jsonError) { - // Fall through to XML parsing - } - } - - // If not JSON or JSON parsing failed, try to parse individual tags - if (!Array.isArray(suggestions)) { - const individualSuggestMatches = suggestionsString.match(/(.*?)<\/suggest>/g) - if (individualSuggestMatches) { - suggestions = individualSuggestMatches - .map((match) => { - const content = match.match(/(.*?)<\/suggest>/) - return content ? content[1] : "" - }) - .filter((suggestion) => suggestion.length > 0) - } else { - // If no XML tags found, treat as single suggestion - suggestions = [suggestions] - } - } - } - if (Array.isArray(suggestions) && suggestions.length > 0) { - commandWithSuggestions = `${block.params.command}\n\n${suggestions.join("\n")}\n` - } - } - - const didApprove = await askApproval("command", commandWithSuggestions) - if (!didApprove) { - return - } - - // Get the custom working directory if provided - const customCwd = block.params.cwd - - const [userRejected, result] = await mockExecuteCommand(cline, block.params.command, customCwd) - - if (userRejected) { - cline.didRejectTool = true - } - - pushToolResult(result) - }, - ) - - // Setup tool use with suggestions - mockToolUse.params.command = "git commit" - mockToolUse.params.suggestions = JSON.stringify([ - 'git commit -m "Initial commit"', - "git commit --amend", - "git commit --no-verify", - ]) - - // Execute - await executeCommandTool( - mockCline as unknown as Task, - mockToolUse, - mockAskApproval as unknown as AskApproval, - mockHandleError as unknown as HandleError, - mockPushToolResult as unknown as PushToolResult, - mockRemoveClosingTag as unknown as RemoveClosingTag, - ) - - // Verify - should pass command WITH suggestions (default behavior) - const expectedCommandWithSuggestions = `git commit - -git commit -m "Initial commit" -git commit --amend -git commit --no-verify -` - expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions) - expect(mockPushToolResult).toHaveBeenCalledWith("Command executed") - }) - }) }) diff --git a/src/core/tools/executeCommandTool.ts b/src/core/tools/executeCommandTool.ts index 19817c917b..008c65d627 100644 --- a/src/core/tools/executeCommandTool.ts +++ b/src/core/tools/executeCommandTool.ts @@ -55,64 +55,15 @@ export async function executeCommandTool( command = unescapeHtmlEntities(command) // Unescape HTML entities. - // Get the provider state to check the setting - const clineProvider = await cline.providerRef.deref() - const clineProviderState = await clineProvider?.getState() - const disableLlmSuggestions = clineProviderState?.disableLlmCommandSuggestions ?? false - - // Parse suggestions if provided and not disabled - let suggestions: string[] | undefined - if (!disableLlmSuggestions && block.params.suggestions) { - try { - // Handle if suggestions is already an array (from direct tool use) - if (Array.isArray(block.params.suggestions)) { - suggestions = block.params.suggestions - } else if (typeof block.params.suggestions === "string") { - const suggestionsString = block.params.suggestions - - // First try to parse as JSON array - if (suggestionsString.trim().startsWith("[")) { - try { - const parsed = JSON.parse(suggestionsString) - if (Array.isArray(parsed)) { - suggestions = parsed - } - } catch (jsonError) { - // Fall through to XML parsing - } - } - - // If not JSON or JSON parsing failed, try to parse individual tags - if (!suggestions) { - const individualSuggestMatches = suggestionsString.match(/(.*?)<\/suggest>/g) - if (individualSuggestMatches) { - suggestions = individualSuggestMatches - .map((match) => { - const content = match.match(/(.*?)<\/suggest>/) - return content ? content[1].trim() : "" - }) - .filter((suggestion) => suggestion.length > 0) - } - } - } - } catch (e) { - // If parsing fails, ignore suggestions - console.warn("Failed to parse suggestions:", e) - } - } - - // Pass suggestions as part of the command text in a structured format - const commandWithSuggestions = suggestions - ? `${command}\n${JSON.stringify(suggestions)}` - : command - - const didApprove = await askApproval("command", commandWithSuggestions) + const didApprove = await askApproval("command", command) if (!didApprove) { return } const executionId = cline.lastMessageTs?.toString() ?? Date.now().toString() + const clineProvider = await cline.providerRef.deref() + const clineProviderState = await clineProvider?.getState() const { terminalOutputLineLimit = 500, terminalShellIntegrationDisabled = false } = clineProviderState ?? {} // Get command execution timeout from VSCode configuration (in seconds) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index b531e270d6..05fa7c51d0 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1434,7 +1434,6 @@ export class ClineProvider profileThresholds, alwaysAllowFollowupQuestions, followupAutoApproveTimeoutMs, - disableLlmCommandSuggestions, } = await this.getState() const telemetryKey = process.env.POSTHOG_API_KEY @@ -1554,7 +1553,6 @@ export class ClineProvider hasOpenedModeSelector: this.getGlobalState("hasOpenedModeSelector") ?? false, alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions ?? false, followupAutoApproveTimeoutMs: followupAutoApproveTimeoutMs ?? 60000, - disableLlmCommandSuggestions: disableLlmCommandSuggestions ?? false, } } @@ -1717,7 +1715,6 @@ export class ClineProvider codebaseIndexSearchMinScore: stateValues.codebaseIndexConfig?.codebaseIndexSearchMinScore, }, profileThresholds: stateValues.profileThresholds ?? {}, - disableLlmCommandSuggestions: stateValues.disableLlmCommandSuggestions ?? false, } } diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 57f8eacaf0..833c51336b 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -182,7 +182,6 @@ export type ExtensionState = Pick< | "alwaysAllowSubtasks" | "alwaysAllowExecute" | "alwaysAllowUpdateTodoList" - | "disableLlmCommandSuggestions" | "allowedCommands" | "deniedCommands" | "allowedMaxRequests" diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 82952fd1b4..d884c30770 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -48,7 +48,6 @@ export interface WebviewMessage { | "alwaysAllowFollowupQuestions" | "alwaysAllowUpdateTodoList" | "followupAutoApproveTimeoutMs" - | "disableLlmCommandSuggestions" | "webviewDidLaunch" | "newTask" | "askResponse" diff --git a/webview-ui/src/components/settings/AutoApproveSettings.tsx b/webview-ui/src/components/settings/AutoApproveSettings.tsx index 8c3cf20b5e..93f54d3fa1 100644 --- a/webview-ui/src/components/settings/AutoApproveSettings.tsx +++ b/webview-ui/src/components/settings/AutoApproveSettings.tsx @@ -27,7 +27,6 @@ type AutoApproveSettingsProps = HTMLAttributes & { alwaysAllowFollowupQuestions?: boolean alwaysAllowUpdateTodoList?: boolean followupAutoApproveTimeoutMs?: number - disableLlmCommandSuggestions?: boolean allowedCommands?: string[] deniedCommands?: string[] setCachedStateField: SetCachedStateField< @@ -46,7 +45,6 @@ type AutoApproveSettingsProps = HTMLAttributes & { | "alwaysAllowExecute" | "alwaysAllowFollowupQuestions" | "followupAutoApproveTimeoutMs" - | "disableLlmCommandSuggestions" | "allowedCommands" | "deniedCommands" | "alwaysAllowUpdateTodoList" @@ -70,7 +68,6 @@ export const AutoApproveSettings = ({ alwaysAllowFollowupQuestions, followupAutoApproveTimeoutMs = 60000, alwaysAllowUpdateTodoList, - disableLlmCommandSuggestions, allowedCommands, deniedCommands, setCachedStateField, @@ -264,22 +261,6 @@ export const AutoApproveSettings = ({
{t("settings:autoApprove.execute.label")}
-
- - setCachedStateField("disableLlmCommandSuggestions", e.target.checked) - } - data-testid="disable-llm-command-suggestions-checkbox"> - - {t("settings:autoApprove.execute.disableLlmSuggestions.label")} - - -
- {t("settings:autoApprove.execute.disableLlmSuggestions.description")} -
-
-