diff --git a/webview-ui/src/components/chat/CommandExecution.tsx b/webview-ui/src/components/chat/CommandExecution.tsx index 8bc5e4e001..0b931c2e70 100644 --- a/webview-ui/src/components/chat/CommandExecution.tsx +++ b/webview-ui/src/components/chat/CommandExecution.tsx @@ -9,7 +9,7 @@ import { safeJsonParse } from "@roo/safeJsonParse" import { vscode } from "@src/utils/vscode" import { parseCommandAndOutput } from "@src/utils/commandParsing" -import { extractCommandPattern, getPatternDescription } from "@src/utils/commandPatterns" +import { extractCommandPatterns, getPatternDescription } from "@src/utils/commandPatterns" import { useExtensionState } from "@src/context/ExtensionStateContext" import { useAppTranslation } from "@src/i18n/TranslationContext" import { cn } from "@src/lib/utils" @@ -61,97 +61,14 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec // If no LLM suggestions but we have a command, extract patterns programmatically if (!command?.trim()) return [] - // Check if this is a chained command - const operators = ["&&", "||", ";", "|"] - const patterns: Array<{ pattern: string; description: string }> = [] + // Use the new extractCommandPatterns function which handles all parsing + const extractedPatterns = extractCommandPatterns(command) - // Split by operators while respecting quotes - let inSingleQuote = false - let inDoubleQuote = false - let escapeNext = false - let currentCommand = "" - let i = 0 - - while (i < command.length) { - const char = command[i] - - if (escapeNext) { - currentCommand += char - escapeNext = false - i++ - continue - } - - if (char === "\\") { - escapeNext = true - currentCommand += char - i++ - continue - } - - if (char === "'" && !inDoubleQuote) { - inSingleQuote = !inSingleQuote - currentCommand += char - i++ - continue - } - - if (char === '"' && !inSingleQuote) { - inDoubleQuote = !inDoubleQuote - currentCommand += char - i++ - continue - } - - // Check for operators outside quotes - if (!inSingleQuote && !inDoubleQuote) { - let foundOperator = false - for (const op of operators) { - if (command.substring(i, i + op.length) === op) { - // Found an operator, process the current command - const trimmedCommand = currentCommand.trim() - if (trimmedCommand) { - // Extract pattern for the command - const pattern = extractCommandPattern(trimmedCommand) - if (pattern) { - patterns.push({ - pattern, - description: getPatternDescription(pattern), - }) - } - } - currentCommand = "" - i += op.length - foundOperator = true - break - } - } - if (foundOperator) continue - } - - currentCommand += char - i++ - } - - // Process the last command - const trimmedCommand = currentCommand.trim() - if (trimmedCommand) { - // Extract pattern for the command - const pattern = extractCommandPattern(trimmedCommand) - if (pattern) { - patterns.push({ - pattern, - description: getPatternDescription(pattern), - }) - } - } - - // Remove duplicates - const uniquePatterns = patterns.filter( - (item, index, self) => index === self.findIndex((p) => p.pattern === item.pattern), - ) - - return uniquePatterns + // Convert to the expected format with descriptions + return extractedPatterns.map((pattern) => ({ + pattern, + description: getPatternDescription(pattern), + })) }, [command, suggestions]) // The command's output can either come from the text associated with the diff --git a/webview-ui/src/utils/__tests__/commandPatterns.spec.ts b/webview-ui/src/utils/__tests__/commandPatterns.spec.ts index 2453928baa..783c0d6215 100644 --- a/webview-ui/src/utils/__tests__/commandPatterns.spec.ts +++ b/webview-ui/src/utils/__tests__/commandPatterns.spec.ts @@ -1,197 +1,142 @@ import { describe, it, expect } from "vitest" -import { extractCommandPatterns, extractCommandPattern, getPatternDescription } from "../commandPatterns" +import { extractCommandPatterns, getPatternDescription } from "../commandPatterns" -describe("commandPatterns", () => { - describe("extractCommandPatterns", () => { - it("should handle empty or null commands", () => { - expect(extractCommandPatterns("")).toEqual([]) - expect(extractCommandPatterns(" ")).toEqual([]) - expect(extractCommandPatterns(null as any)).toEqual([]) - expect(extractCommandPatterns(undefined as any)).toEqual([]) +describe("extractCommandPatterns", () => { + describe("command chains", () => { + it("should not include the full command chain as a pattern", () => { + const patterns = extractCommandPatterns("cd backend && npm install") + expect(patterns).not.toContain("cd backend && npm install") + expect(patterns).toContain("cd") + expect(patterns).toContain("npm") + expect(patterns).toContain("npm install") }) - it("should extract simple commands", () => { - expect(extractCommandPatterns("ls")).toEqual(["ls"]) - expect(extractCommandPatterns("pwd")).toEqual(["pwd"]) - expect(extractCommandPatterns("echo hello")).toEqual(["echo"]) + it("should handle multiple operators", () => { + const patterns = extractCommandPatterns("git status && git add . || git commit -m 'test'") + expect(patterns).not.toContain("git status && git add . || git commit -m 'test'") + expect(patterns).toContain("git") + expect(patterns).toContain("git status") + expect(patterns).toContain("git add") + expect(patterns).toContain("git commit") }) - it("should handle npm commands correctly", () => { - expect(extractCommandPatterns("npm install")).toEqual(["npm install"]) - expect(extractCommandPatterns("npm run build")).toEqual(["npm run"]) - expect(extractCommandPatterns("npm run test:unit")).toEqual(["npm run"]) - expect(extractCommandPatterns("npm test")).toEqual(["npm test"]) - expect(extractCommandPatterns("npm run build -- --watch")).toEqual(["npm run"]) + it("should handle pipe operators", () => { + const patterns = extractCommandPatterns("grep map | head") + expect(patterns).not.toContain("grep map | head") + expect(patterns).toContain("grep") + expect(patterns).toContain("head") }) - it("should handle yarn/pnpm/bun commands", () => { - expect(extractCommandPatterns("yarn install")).toEqual(["yarn install"]) - expect(extractCommandPatterns("pnpm run dev")).toEqual(["pnpm run"]) - expect(extractCommandPatterns("bun test")).toEqual(["bun test"]) + it("should handle semicolon separators", () => { + const patterns = extractCommandPatterns("cd src; npm test") + expect(patterns).not.toContain("cd src; npm test") + expect(patterns).toContain("cd") + expect(patterns).toContain("npm") + expect(patterns).toContain("npm test") }) - it("should handle git commands", () => { - expect(extractCommandPatterns("git status")).toEqual(["git status"]) - expect(extractCommandPatterns('git commit -m "message"')).toEqual(["git commit"]) - expect(extractCommandPatterns("git push origin main")).toEqual(["git push"]) - expect(extractCommandPatterns("git log --oneline")).toEqual(["git log"]) + it("should handle complex chains from real examples", () => { + const patterns = extractCommandPatterns( + "unzip -l builds/roo-cline-3.21.5-error-boundary-component.vsix|grep map|head", + ) + expect(patterns).not.toContain( + "unzip -l builds/roo-cline-3.21.5-error-boundary-component.vsix|grep map|head", + ) + expect(patterns).toContain("unzip") + expect(patterns).toContain("grep") + expect(patterns).toContain("head") }) - it("should handle docker/kubectl/helm commands", () => { - expect(extractCommandPatterns("docker run nginx")).toEqual(["docker run"]) - expect(extractCommandPatterns("kubectl get pods")).toEqual(["kubectl get"]) - expect(extractCommandPatterns("helm install myapp ./chart")).toEqual(["helm install"]) + it("should handle chains with spaces around operators", () => { + const patterns = extractCommandPatterns("git diff | xclip") + expect(patterns).not.toContain("git diff | xclip") + expect(patterns).toContain("git") + expect(patterns).toContain("git diff") + expect(patterns).toContain("xclip") + }) + }) + + describe("single commands", () => { + it("should extract base command", () => { + const patterns = extractCommandPatterns("git") + expect(patterns).toEqual(["git"]) }) - it("should handle make commands", () => { - expect(extractCommandPatterns("make build")).toEqual(["make build"]) - expect(extractCommandPatterns("make test")).toEqual(["make test"]) - expect(extractCommandPatterns("make -j4 all")).toEqual(["make"]) + it("should extract command with subcommand", () => { + const patterns = extractCommandPatterns("git status") + expect(patterns).toContain("git") + expect(patterns).toContain("git status") }) - it("should handle interpreter commands", () => { - expect(extractCommandPatterns("python script.py")).toEqual(["python"]) - expect(extractCommandPatterns("python3 -m venv env")).toEqual(["python3"]) - expect(extractCommandPatterns("node index.js --port 3000")).toEqual(["node"]) - expect(extractCommandPatterns("ruby app.rb")).toEqual(["ruby"]) - }) - - it("should handle dangerous commands", () => { - expect(extractCommandPatterns("rm -rf node_modules")).toEqual(["rm"]) - expect(extractCommandPatterns("chmod 755 script.sh")).toEqual(["chmod"]) - expect(extractCommandPatterns('find . -name "*.log" -delete')).toEqual(["find"]) - }) - - it("should handle cd commands", () => { - expect(extractCommandPatterns("cd /home/user")).toEqual(["cd"]) - expect(extractCommandPatterns("cd ..")).toEqual(["cd"]) - expect(extractCommandPatterns("cd ~/projects")).toEqual(["cd"]) - }) - - it("should handle chained commands with &&", () => { - const patterns = extractCommandPatterns("npm install && npm run build") - expect(patterns).toEqual(["npm install", "npm run"]) - }) - - it("should handle chained commands with ||", () => { - const patterns = extractCommandPatterns('npm test || echo "Tests failed"') - expect(patterns).toEqual(["echo", "npm test"]) - }) - - it("should handle chained commands with ;", () => { - const patterns = extractCommandPatterns("cd /tmp; ls -la; pwd") - expect(patterns).toEqual(["cd", "ls", "pwd"]) - }) - - it("should handle piped commands", () => { - const patterns = extractCommandPatterns("ps aux | grep node") - expect(patterns).toEqual(["grep", "ps"]) - }) - - it("should handle complex chained commands", () => { - const patterns = extractCommandPatterns('git pull && npm install && npm run build || echo "Build failed"') - expect(patterns).toEqual(["echo", "git pull", "npm install", "npm run"]) - }) - - it("should handle quoted arguments correctly", () => { - expect(extractCommandPatterns('echo "hello world"')).toEqual(["echo"]) - expect(extractCommandPatterns("echo 'hello world'")).toEqual(["echo"]) - expect(extractCommandPatterns('git commit -m "feat: add new feature"')).toEqual(["git commit"]) - }) - - it("should handle escaped characters", () => { - expect(extractCommandPatterns("echo hello\\ world")).toEqual(["echo"]) - expect(extractCommandPatterns('echo "hello \\"world\\""')).toEqual(["echo"]) - }) - - it("should handle environment variables", () => { - expect(extractCommandPatterns("NODE_ENV=production npm start")).toEqual(["npm start"]) - expect(extractCommandPatterns("PORT=3000 node server.js")).toEqual(["node"]) + it("should handle npm run commands", () => { + const patterns = extractCommandPatterns("npm run test") + expect(patterns).toContain("npm") + expect(patterns).toContain("npm run") + // Should stop at 'run' to allow any script }) it("should handle script files", () => { - expect(extractCommandPatterns("./deploy.sh")).toEqual(["./deploy.sh"]) - expect(extractCommandPatterns("/usr/local/bin/script.py")).toEqual(["/usr/local/bin/script.py"]) - expect(extractCommandPatterns("./scripts/test.js --verbose")).toEqual(["./scripts/test.js"]) + const patterns = extractCommandPatterns("../repackage.sh") + expect(patterns).toEqual(["../repackage.sh"]) }) - it("should handle complex npm scripts", () => { - expect(extractCommandPatterns("npm run build:prod -- --source-maps")).toEqual(["npm run"]) - expect(extractCommandPatterns("npm run test:coverage -- --watch")).toEqual(["npm run"]) + it("should stop at flags", () => { + const patterns = extractCommandPatterns("git diff --stat") + expect(patterns).toContain("git") + expect(patterns).toContain("git diff") + expect(patterns).not.toContain("git diff --stat") }) - it("should return unique sorted patterns", () => { - const patterns = extractCommandPatterns("npm install && npm install && npm run build") - expect(patterns).toEqual(["npm install", "npm run"]) - }) - - it("should handle commands with glob patterns", () => { - expect(extractCommandPatterns("rm *.log")).toEqual(["rm"]) - expect(extractCommandPatterns("ls *.{js,ts}")).toEqual(["ls"]) - }) - - it("should handle commands with redirects", () => { - expect(extractCommandPatterns('echo "test" > file.txt')).toEqual(["echo"]) - expect(extractCommandPatterns("cat file.txt >> output.log")).toEqual(["cat"]) - }) - - it("should handle subshells and command substitution", () => { - expect(extractCommandPatterns("echo $(date)")).toEqual(["echo"]) - expect(extractCommandPatterns("echo `pwd`")).toEqual(["echo"]) + it("should handle environment variables", () => { + const patterns = extractCommandPatterns("NODE_ENV=production npm start") + expect(patterns).toContain("npm") + expect(patterns).toContain("npm start") + expect(patterns).not.toContain("NODE_ENV=production") }) }) - describe("extractCommandPattern", () => { - it("should return the first pattern for backward compatibility", () => { - expect(extractCommandPattern("npm install && npm run build")).toBe("npm install") - expect(extractCommandPattern("git status")).toBe("git status") - expect(extractCommandPattern("")).toBe("") - }) - }) - - describe("getPatternDescription", () => { - it("should describe npm patterns", () => { - expect(getPatternDescription("npm run")).toBe("all npm run scripts") - expect(getPatternDescription("npm install")).toBe("npm install commands") - expect(getPatternDescription("npm test")).toBe("npm test commands") + describe("edge cases", () => { + it("should handle empty command", () => { + const patterns = extractCommandPatterns("") + expect(patterns).toEqual([]) }) - it("should describe git patterns", () => { - expect(getPatternDescription("git commit")).toBe("git commit commands") - expect(getPatternDescription("git push")).toBe("git push commands") + it("should handle whitespace only", () => { + const patterns = extractCommandPatterns(" ") + expect(patterns).toEqual([]) }) - it("should describe script files", () => { - expect(getPatternDescription("./deploy.sh")).toBe("this specific script") - expect(getPatternDescription("/usr/bin/script.py")).toBe("this specific script") + it("should handle quoted strings with operators", () => { + const patterns = extractCommandPatterns('echo "test && test"') + expect(patterns).toContain("echo") + expect(patterns).not.toContain("test") }) - it("should describe interpreter patterns", () => { - expect(getPatternDescription("python")).toBe("python scripts") - expect(getPatternDescription("node")).toBe("node scripts") - expect(getPatternDescription("ruby")).toBe("ruby scripts") - }) - - it("should describe docker/kubectl patterns", () => { - expect(getPatternDescription("docker run")).toBe("docker run commands") - expect(getPatternDescription("kubectl get")).toBe("kubectl get commands") - }) - - it("should describe make patterns", () => { - expect(getPatternDescription("make build")).toBe("make build target") - expect(getPatternDescription("make test")).toBe("make test target") - }) - - it("should describe cd pattern", () => { - expect(getPatternDescription("cd")).toBe("directory navigation") - }) - - it("should handle empty patterns", () => { - expect(getPatternDescription("")).toBe("") - }) - - it("should provide default description", () => { - expect(getPatternDescription("custom-command")).toBe("custom-command commands") + it("should return sorted unique patterns", () => { + const patterns = extractCommandPatterns("git status && git add && git status") + expect(patterns).toEqual(["git", "git add", "git status"]) }) }) }) + +describe("getPatternDescription", () => { + it("should describe npm run patterns", () => { + expect(getPatternDescription("npm run")).toBe("all npm run scripts") + }) + + it("should describe git patterns", () => { + expect(getPatternDescription("git status")).toBe("git status commands") + }) + + it("should describe script files", () => { + expect(getPatternDescription("./build.sh")).toBe("this specific script") + }) + + it("should describe cd", () => { + expect(getPatternDescription("cd")).toBe("directory navigation") + }) + + it("should describe generic commands", () => { + expect(getPatternDescription("ls")).toBe("ls commands") + }) +}) diff --git a/webview-ui/src/utils/commandPatterns.ts b/webview-ui/src/utils/commandPatterns.ts index b4d60e9834..6a0cfb02dc 100644 --- a/webview-ui/src/utils/commandPatterns.ts +++ b/webview-ui/src/utils/commandPatterns.ts @@ -18,9 +18,11 @@ export function extractCommandPatterns(command: string): string[] { const commands = splitByOperators(command, chainOperators) for (const cmd of commands) { - const pattern = extractSingleCommandPattern(cmd.trim()) - if (pattern) { - patterns.add(pattern) + const cmdPatterns = extractSingleCommandPattern(cmd.trim()) + for (const pattern of cmdPatterns) { + if (pattern) { + patterns.add(pattern) + } } } @@ -100,16 +102,18 @@ function splitByOperators(command: string, operators: string[]): string[] { } /** - * Extract pattern from a single command (not chained) + * Extract patterns from a single command (not chained) + * Returns an array of patterns instead of a single pattern */ -function extractSingleCommandPattern(command: string): string { - if (!command) return "" +function extractSingleCommandPattern(command: string): string[] { + if (!command) return [] try { const parsed = parse(command) - if (parsed.length === 0) return "" + if (parsed.length === 0) return [] const patterns: string[] = [] + const allPatterns: string[] = [] let i = 0 while (i < parsed.length) { @@ -159,9 +163,10 @@ function extractSingleCommandPattern(command: string): string { strToken.endsWith(".js") || strToken.endsWith(".rb") ) { - return strToken + return [strToken] } patterns.push(strToken) + allPatterns.push(strToken) // Add base command i++ continue } @@ -185,6 +190,7 @@ function extractSingleCommandPattern(command: string): string { // Include subcommand if (!strToken.startsWith("-") && !strToken.includes("/")) { patterns.push(strToken) + allPatterns.push(patterns.join(" ")) // For 'run' commands, stop here to allow any script if (strToken === "run") { break @@ -198,6 +204,7 @@ function extractSingleCommandPattern(command: string): string { else if (baseCmd === "git") { if (!strToken.startsWith("-")) { patterns.push(strToken) + allPatterns.push(patterns.join(" ")) } break } @@ -206,6 +213,7 @@ function extractSingleCommandPattern(command: string): string { else if (["docker", "kubectl", "helm"].includes(baseCmd)) { if (!strToken.startsWith("-")) { patterns.push(strToken) + allPatterns.push(patterns.join(" ")) } break } @@ -214,6 +222,7 @@ function extractSingleCommandPattern(command: string): string { else if (baseCmd === "make") { if (!strToken.startsWith("-")) { patterns.push(strToken) + allPatterns.push(patterns.join(" ")) } break } @@ -257,11 +266,11 @@ function extractSingleCommandPattern(command: string): string { i++ } - return patterns.join(" ") + return allPatterns } catch (_error) { // If parsing fails, fall back to simple first token const tokens = command.split(/\s+/) - return tokens[0] || "" + return tokens[0] ? [tokens[0]] : [] } } @@ -331,5 +340,6 @@ export function getPatternDescription(pattern: string): string { */ export function extractCommandPattern(command: string): string { const patterns = extractCommandPatterns(command) - return patterns[0] || "" + // Return the most specific pattern (usually the last one) + return patterns[patterns.length - 1] || "" }