Skip to content
Closed
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
cf6c380
fix: resolve all issues in PR #5491 - command whitelisting feature
hannesrudolph Jul 9, 2025
20363b4
fix: resolve knip unused files issue
hannesrudolph Jul 9, 2025
a0e8c24
feat: add denyCommand handler for individual command deny listing
hannesrudolph Jul 14, 2025
18e3cd1
feat: add unified UI for managing allow/deny command lists
hannesrudolph Jul 14, 2025
bacf751
fix: correct toggle behavior for allow/deny command buttons
hannesrudolph Jul 14, 2025
9a01e80
fix: ensure immediate UI updates when toggling command allow/deny status
hannesrudolph Jul 14, 2025
38fb80f
refactor: improve tooltip implementation to follow codebase standards
hannesrudolph Jul 14, 2025
63f4a27
fix: remove hardcoded npm pattern suggestions
hannesrudolph Jul 14, 2025
81ef373
chore: remove temporary PR fixer file from version control
hannesrudolph Jul 14, 2025
7d75a0c
fix: trim trailing spaces from command suggestions
hannesrudolph Jul 14, 2025
df69540
fix: remove trailing spaces from command suggestion examples
hannesrudolph Jul 14, 2025
4ec34ad
fix: use inclusive terminology in command permission instructions
hannesrudolph Jul 14, 2025
e98c404
refactor: streamline command permission instructions for LLM
hannesrudolph Jul 14, 2025
b7986c0
fix: address PR review comments and improve command permission UI
hannesrudolph Jul 14, 2025
c9964e2
chore: update all locale files with new command permission translations
hannesrudolph Jul 14, 2025
8bbecac
feat: add setting to disable LLM command suggestions
hannesrudolph Jul 15, 2025
10843e8
fix: add missing imports for vscode and Package in Task.ts
hannesrudolph Jul 15, 2025
70ca7a6
refactor: move command whitelisting from VS Code settings to plugin UI
hannesrudolph Jul 15, 2025
238bae6
fix: resolve command whitelisting issues
hannesrudolph Jul 15, 2025
0b714fd
fix: add missing translations for disableLlmSuggestions setting
hannesrudolph Jul 15, 2025
f903871
fix: prevent removed commands from reappearing due to workspace confi…
hannesrudolph Jul 15, 2025
0a067f0
feat: implement robust command pattern extraction using shell-quote
hannesrudolph Jul 15, 2025
a280f87
fix: prevent full command chains from being extracted as patterns
hannesrudolph Jul 15, 2025
60815f4
fix: prevent LLM from suggesting full command chains in execute-command
hannesrudolph Jul 15, 2025
edba73a
test: add comprehensive real-world test cases for command pattern parser
hannesrudolph Jul 15, 2025
d899a36
refactor: Remove LLM suggestion feature in favor of deterministic she…
hannesrudolph Jul 15, 2025
39a8fad
fix: Remove remaining references to disableLlmCommandSuggestions
hannesrudolph Jul 15, 2025
ded11f8
chore: remove temporary documentation files
hannesrudolph Jul 15, 2025
06c8cb8
fix: replace hardcoded aria-labels with i18n translation keys
hannesrudolph Jul 15, 2025
abb8612
chore: remove test_fix_summary.md temporary file
hannesrudolph Jul 15, 2025
51d010e
fix: add missing roo-cline.allowedCommands configuration registration
hannesrudolph Jul 15, 2025
2f321e9
fix: address PR #5491 review feedback
hannesrudolph Jul 15, 2025
83b6f3c
refactor: simplify command allow/deny implementation
hannesrudolph Jul 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/core/prompts/tools/execute-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,42 @@ import { ToolArgs } from "./types"
export function getExecuteCommandDescription(args: ToolArgs): string | undefined {
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})
- suggestions: (optional) Command patterns for the user to allow/deny for future auto-approval. Include 1-2 relevant patterns when executing common development commands. Use <suggest> tags.

**Suggestion Guidelines:**
- Suggestions use prefix matching (case-insensitive)
- Include the base command (e.g., "npm", "git") and optionally a more specific pattern
- Only suggest "*" (allow all) if explicitly requested by the user

Usage:
<execute_command>
<command>Your command here</command>
<cwd>Working directory path (optional)</cwd>
<suggestions>
<suggest>pattern 1</suggest>
<suggest>pattern 2</suggest>
</suggestions>
</execute_command>

Example: Requesting to execute npm run dev
<execute_command>
<command>npm run dev</command>
<suggestions>
<suggest>npm run</suggest>
<suggest>npm</suggest>
</suggestions>
</execute_command>

Example: Requesting to execute ls in a specific directory if directed
Example: Requesting to execute ls in a specific directory
<execute_command>
<command>ls -la</command>
<cwd>/home/user/projects</cwd>
<suggestions>
<suggest>ls</suggest>
</suggestions>
</execute_command>`
}
273 changes: 272 additions & 1 deletion src/core/tools/__tests__/executeCommandTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,45 @@ beforeEach(() => {
return
}

const didApprove = await askApproval("command", block.params.command)
// 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 <suggest> tags
if (!Array.isArray(suggestions)) {
const individualSuggestMatches = suggestionsString.match(/<suggest>(.*?)<\/suggest>/g)
if (individualSuggestMatches) {
suggestions = individualSuggestMatches
.map((match) => {
const content = match.match(/<suggest>(.*?)<\/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<suggestions>\n${suggestions.join("\n")}\n</suggestions>`
}
}

const didApprove = await askApproval("command", commandWithSuggestions)
if (!didApprove) {
return
}
Expand Down Expand Up @@ -309,4 +347,237 @@ describe("executeCommandTool", () => {
expect(mockOptions.commandExecutionTimeout).toBeDefined()
})
})

describe("Suggestions functionality", () => {
it("should pass command with suggestions when suggestions are provided as array", async () => {
// Setup
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
const expectedCommandWithSuggestions = `npm install
<suggestions>
npm install --save
npm install --save-dev
npm install --global
</suggestions>`
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
expect(mockExecuteCommand).toHaveBeenCalled()
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
})

it("should pass command with suggestions when suggestions are provided as JSON string", async () => {
// Setup
mockToolUse.params.command = "git commit"
mockToolUse.params.suggestions =
'["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
const expectedCommandWithSuggestions = `git commit
<suggestions>
git commit -m "Initial commit"
git commit --amend
git commit --no-verify
</suggestions>`
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
expect(mockExecuteCommand).toHaveBeenCalled()
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
})

it("should handle single suggestion as string", async () => {
// Setup
mockToolUse.params.command = "docker run"
mockToolUse.params.suggestions = "docker run -it ubuntu:latest"

// 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
const expectedCommandWithSuggestions = `docker run
<suggestions>
docker run -it ubuntu:latest
</suggestions>`
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
expect(mockExecuteCommand).toHaveBeenCalled()
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
})

it("should handle empty suggestions array", async () => {
// Setup
mockToolUse.params.command = "ls"
mockToolUse.params.suggestions = JSON.stringify([])

// 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(mockAskApproval).toHaveBeenCalledWith("command", "ls")
expect(mockExecuteCommand).toHaveBeenCalled()
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
})

it("should handle invalid JSON string in suggestions", async () => {
// Setup
mockToolUse.params.command = "echo test"
mockToolUse.params.suggestions = "invalid json {"

// 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 treat invalid JSON as single suggestion
const expectedCommandWithSuggestions = `echo test
<suggestions>
invalid json {
</suggestions>`
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
expect(mockExecuteCommand).toHaveBeenCalled()
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
})

it("should parse individual <suggest> XML tags correctly", async () => {
// Setup
mockToolUse.params.command = "npm install"
mockToolUse.params.suggestions =
"<suggest>npm install --save</suggest><suggest>npm install --save-dev</suggest><suggest>npm install --global</suggest>"

// 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
const expectedCommandWithSuggestions = `npm install
<suggestions>
npm install --save
npm install --save-dev
npm install --global
</suggestions>`
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
expect(mockExecuteCommand).toHaveBeenCalled()
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
})

it("should parse single <suggest> XML tag correctly", async () => {
// Setup
mockToolUse.params.command = "git push"
mockToolUse.params.suggestions = "<suggest>git push origin main</suggest>"

// 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
const expectedCommandWithSuggestions = `git push
<suggestions>
git push origin main
</suggestions>`
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
expect(mockExecuteCommand).toHaveBeenCalled()
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
})

it("should handle mixed content with <suggest> tags", async () => {
// Setup
mockToolUse.params.command = "docker run"
mockToolUse.params.suggestions =
"Some text before <suggest>docker run -it ubuntu</suggest> and <suggest>docker run -d nginx</suggest> with text after"

// 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
const expectedCommandWithSuggestions = `docker run
<suggestions>
docker run -it ubuntu
docker run -d nginx
</suggestions>`
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommandWithSuggestions)
expect(mockExecuteCommand).toHaveBeenCalled()
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
})

it("should work normally when no suggestions are provided", async () => {
// Setup
mockToolUse.params.command = "pwd"
// No suggestions property

// 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(mockAskApproval).toHaveBeenCalledWith("command", "pwd")
expect(mockExecuteCommand).toHaveBeenCalled()
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
})
})
})
49 changes: 48 additions & 1 deletion src/core/tools/executeCommandTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,54 @@ export async function executeCommandTool(
cline.consecutiveMistakeCount = 0

command = unescapeHtmlEntities(command) // Unescape HTML entities.
const didApprove = await askApproval("command", command)

// Parse suggestions if provided
Copy link

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This suggestion-parsing logic duplicates the code in parseCommandAndOutput; consider extracting it into a shared utility to avoid duplication.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this parsing should be centralized in Roo's core.

Alternatively, cherry-pick this commit which provides a generalized way of passing metadata down from the tool into the web view:

a2bd68b

let suggestions: string[] | undefined
if (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 <suggest> tags
if (!suggestions) {
const individualSuggestMatches = suggestionsString.match(/<suggest>(.*?)<\/suggest>/g)
if (individualSuggestMatches) {
suggestions = individualSuggestMatches
.map((match) => {
const content = match.match(/<suggest>(.*?)<\/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<suggestions>${JSON.stringify(suggestions)}</suggestions>`
Copy link

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggestions block is formatted as a JSON array inside <suggestions> tags, but tests expect each suggestion on its own line. Consider formatting as:

`${command}\n<suggestions>\n${suggestions.join("\n")}\n</suggestions>`
Suggested change
? `${command}\n<suggestions>${JSON.stringify(suggestions)}</suggestions>`
? `${command}\n<suggestions>\n${suggestions.join("\n")}\n</suggestions>`

Copilot uses AI. Check for mistakes.
: command

const didApprove = await askApproval("command", commandWithSuggestions)

if (!didApprove) {
return
Expand Down
Loading
Loading