diff --git a/webview-ui/src/components/chat/CommandExecution.tsx b/webview-ui/src/components/chat/CommandExecution.tsx index 673bcca00b..cf0702e302 100644 --- a/webview-ui/src/components/chat/CommandExecution.tsx +++ b/webview-ui/src/components/chat/CommandExecution.tsx @@ -13,12 +13,12 @@ import { cn } from "@src/lib/utils" import { Button } from "@src/components/ui" import CodeBlock from "../common/CodeBlock" import { CommandPatternSelector } from "./CommandPatternSelector" -import { - extractCommandPatterns, - getPatternDescription, - parseCommandAndOutput, - CommandPattern, -} from "../../utils/commandPatterns" +import { extractPatternsFromCommand } from "../../utils/command-parser" + +interface CommandPattern { + pattern: string + description?: string +} interface CommandExecutionProps { executionId: string @@ -37,8 +37,28 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec } = useExtensionState() const { command, output: parsedOutput } = useMemo(() => { - // Use the enhanced parser from commandPatterns - return parseCommandAndOutput(text || "") + // Parse command and output using the "Output:" separator + const outputSeparator = "Output:" + const outputIndex = text?.indexOf(`\n${outputSeparator}`) ?? -1 + + if (outputIndex !== -1) { + // Text is split into command and output + const cmd = text!.slice(0, outputIndex).trim() + // Skip the newline and "Output:" text + const afterSeparator = outputIndex + 1 + outputSeparator.length + let startOfOutput = afterSeparator + if (text![afterSeparator] === "\n") { + startOfOutput = afterSeparator + 1 + } + const out = text!.slice(startOfOutput).trim() + return { command: cmd, output: out } + } else if (text?.indexOf(outputSeparator) === 0) { + // Edge case: text starts with "Output:" (no command) + return { command: "", output: text.slice(outputSeparator.length).trim() } + } else { + // No output separator found, the entire text is the command + return { command: text?.trim() || "", output: "" } + } }, [text]) // If we aren't opening the VSCode terminal for this command then we default @@ -54,20 +74,12 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec // Extract command patterns from the actual command that was executed const commandPatterns = useMemo(() => { - const patterns: CommandPattern[] = [] - - // Always extract patterns from the actual command that was executed - // We don't use AI suggestions because the patterns should reflect - // what was actually executed, not what the AI thinks might be useful - const extractedPatterns = extractCommandPatterns(command) - extractedPatterns.forEach((pattern) => { - patterns.push({ - pattern, - description: getPatternDescription(pattern), - }) - }) - - return patterns + // Extract patterns from the actual command that was executed + const extractedPatterns = extractPatternsFromCommand(command) + return extractedPatterns.map((pattern) => ({ + pattern, + description: `${pattern} commands`, + })) }, [command]) // Handle pattern changes diff --git a/webview-ui/src/components/chat/CommandPatternSelector.tsx b/webview-ui/src/components/chat/CommandPatternSelector.tsx index 17799a8aec..ac87b90cbc 100644 --- a/webview-ui/src/components/chat/CommandPatternSelector.tsx +++ b/webview-ui/src/components/chat/CommandPatternSelector.tsx @@ -3,9 +3,13 @@ import { Check, ChevronDown, Info, X } from "lucide-react" import { cn } from "../../lib/utils" import { useTranslation, Trans } from "react-i18next" import { VSCodeLink } from "@vscode/webview-ui-toolkit/react" -import { CommandPattern } from "../../utils/commandPatterns" import { StandardTooltip } from "../ui/standard-tooltip" +interface CommandPattern { + pattern: string + description?: string +} + interface CommandPatternSelectorProps { patterns: CommandPattern[] allowedCommands: string[] diff --git a/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx b/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx index 7b584d75e7..ed9e0034ea 100644 --- a/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx @@ -21,19 +21,6 @@ vi.mock("../../common/CodeBlock", () => ({ default: ({ source }: { source: string }) =>
{source}
, })) -// Mock the commandPatterns module but use the actual implementation -vi.mock("../../../utils/commandPatterns", async () => { - const actual = await vi.importActual( - "../../../utils/commandPatterns", - ) - return { - ...actual, - parseCommandAndOutput: actual.parseCommandAndOutput, - extractCommandPatterns: actual.extractCommandPatterns, - getPatternDescription: actual.getPatternDescription, - } -}) - vi.mock("../CommandPatternSelector", () => ({ CommandPatternSelector: ({ patterns, onAllowPatternChange, onDenyPatternChange }: any) => (
diff --git a/webview-ui/src/components/chat/__tests__/CommandPatternSelector.spec.tsx b/webview-ui/src/components/chat/__tests__/CommandPatternSelector.spec.tsx index d48d16c2d2..815d4dfbfb 100644 --- a/webview-ui/src/components/chat/__tests__/CommandPatternSelector.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/CommandPatternSelector.spec.tsx @@ -2,9 +2,13 @@ import React from "react" import { render, screen, fireEvent } from "@testing-library/react" import { describe, it, expect, vi } from "vitest" import { CommandPatternSelector } from "../CommandPatternSelector" -import { CommandPattern } from "../../../utils/commandPatterns" import { TooltipProvider } from "../../../components/ui/tooltip" +interface CommandPattern { + pattern: string + description?: string +} + // Mock react-i18next vi.mock("react-i18next", () => ({ useTranslation: () => ({ diff --git a/webview-ui/src/utils/__tests__/commandPatterns.spec.ts b/webview-ui/src/utils/__tests__/commandPatterns.spec.ts deleted file mode 100644 index ff563be1ac..0000000000 --- a/webview-ui/src/utils/__tests__/commandPatterns.spec.ts +++ /dev/null @@ -1,422 +0,0 @@ -import { describe, it, expect } from "vitest" -import { extractCommandPatterns, getPatternDescription, parseCommandAndOutput } from "../commandPatterns" - -describe("extractCommandPatterns", () => { - it("should extract simple command", () => { - const patterns = extractCommandPatterns("ls") - expect(patterns).toEqual(["ls"]) - }) - - it("should extract command with arguments", () => { - const patterns = extractCommandPatterns("npm install express") - expect(patterns).toEqual(["npm", "npm install", "npm install express"]) - }) - - it("should handle piped commands", () => { - const patterns = extractCommandPatterns("ls -la | grep test") - expect(patterns).toContain("ls") - expect(patterns).toContain("grep") - expect(patterns).toContain("grep test") - }) - - it("should handle chained commands with &&", () => { - const patterns = extractCommandPatterns("npm install && npm run build") - expect(patterns).toContain("npm") - expect(patterns).toContain("npm install") - expect(patterns).toContain("npm run") - expect(patterns).toContain("npm run build") - }) - - it("should handle chained commands with ||", () => { - const patterns = extractCommandPatterns("npm test || npm run test:ci") - expect(patterns).toContain("npm") - expect(patterns).toContain("npm test") - expect(patterns).toContain("npm run") - expect(patterns).not.toContain("npm run test:ci") - }) - - it("should handle semicolon separated commands", () => { - const patterns = extractCommandPatterns("cd src; npm install") - expect(patterns).toContain("cd") - expect(patterns).toContain("cd src") - expect(patterns).toContain("npm") - expect(patterns).toContain("npm install") - }) - - it("should stop at flags", () => { - const patterns = extractCommandPatterns('git commit -m "test message"') - expect(patterns).toContain("git") - expect(patterns).toContain("git commit") - expect(patterns).not.toContain("git commit -m") - }) - - it("should stop at paths with slashes", () => { - const patterns = extractCommandPatterns("cd /usr/local/bin") - expect(patterns).toContain("cd") - expect(patterns).not.toContain("cd /usr/local/bin") - }) - - it("should handle empty or null input", () => { - expect(extractCommandPatterns("")).toEqual([]) - expect(extractCommandPatterns(" ")).toEqual([]) - expect(extractCommandPatterns(null as any)).toEqual([]) - expect(extractCommandPatterns(undefined as any)).toEqual([]) - }) - - it("should handle complex command with multiple operators", () => { - const patterns = extractCommandPatterns('npm install && npm test | grep success || echo "failed"') - expect(patterns).toContain("npm") - expect(patterns).toContain("npm install") - expect(patterns).toContain("npm test") - expect(patterns).toContain("grep") - expect(patterns).toContain("grep success") - expect(patterns).toContain("echo") - }) - - it("should handle malformed commands gracefully", () => { - const patterns = extractCommandPatterns("npm install && ") - expect(patterns).toContain("npm") - expect(patterns).toContain("npm install") - }) - - it("should extract main command even if parsing fails", () => { - // Create a command that might cause parsing issues - const patterns = extractCommandPatterns('echo "unclosed quote') - expect(patterns).toContain("echo") - }) - - it("should handle commands with special characters in arguments", () => { - const patterns = extractCommandPatterns("git add .") - expect(patterns).toContain("git") - expect(patterns).toContain("git add") - expect(patterns).not.toContain("git add .") // dot is excluded - }) - - it("should return sorted patterns", () => { - const patterns = extractCommandPatterns("npm run build && git push") - expect(patterns).toEqual(["git", "git push", "npm", "npm run", "npm run build"]) - }) - - it("should handle numeric input like '0 total'", () => { - const patterns = extractCommandPatterns("0 total") - // Should return empty array since "0" is not a valid command - expect(patterns).toEqual([]) - }) - - it("should handle pure numeric commands", () => { - const patterns = extractCommandPatterns("0") - // Should return empty array since pure numbers are not valid commands - expect(patterns).toEqual([]) - }) -}) - -describe("getPatternDescription", () => { - it("should return pattern followed by commands", () => { - expect(getPatternDescription("cd")).toBe("cd commands") - expect(getPatternDescription("npm")).toBe("npm commands") - expect(getPatternDescription("npm install")).toBe("npm install commands") - expect(getPatternDescription("git")).toBe("git commands") - expect(getPatternDescription("git push")).toBe("git push commands") - expect(getPatternDescription("python")).toBe("python commands") - }) - - it("should handle any command pattern", () => { - expect(getPatternDescription("unknowncommand")).toBe("unknowncommand commands") - expect(getPatternDescription("custom-tool")).toBe("custom-tool commands") - }) - - it("should handle package managers", () => { - expect(getPatternDescription("yarn")).toBe("yarn commands") - expect(getPatternDescription("pnpm")).toBe("pnpm commands") - expect(getPatternDescription("bun")).toBe("bun commands") - }) - - it("should handle build tools", () => { - expect(getPatternDescription("make")).toBe("make commands") - expect(getPatternDescription("cmake")).toBe("cmake commands") - expect(getPatternDescription("cargo")).toBe("cargo commands") - expect(getPatternDescription("go build")).toBe("go build commands") - }) -}) - -describe("parseCommandAndOutput", () => { - it("should handle command with $ prefix without Output: separator", () => { - const text = "$ npm install\nInstalling packages..." - const result = parseCommandAndOutput(text) - // Without Output: separator, the entire text is treated as command - expect(result.command).toBe("$ npm install\nInstalling packages...") - expect(result.output).toBe("") - }) - - it("should handle command with ❯ prefix without Output: separator", () => { - const text = "❯ git status\nOn branch main" - const result = parseCommandAndOutput(text) - // Without Output: separator, the entire text is treated as command - expect(result.command).toBe("❯ git status\nOn branch main") - expect(result.output).toBe("") - }) - - it("should handle command with > prefix without Output: separator", () => { - const text = "> echo hello\nhello" - const result = parseCommandAndOutput(text) - // Without Output: separator, the entire text is treated as command - expect(result.command).toBe("> echo hello\nhello") - expect(result.output).toBe("") - }) - - it("should return original text if no command prefix found", () => { - const text = "npm install" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("npm install") - expect(result.output).toBe("") - }) - - it("should extract AI suggestions from output with Output: separator", () => { - const text = "npm install\nOutput:\nSuggested patterns: npm, npm install, npm run" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("npm install") - expect(result.suggestions).toEqual(["npm", "npm install", "npm run"]) - }) - - it("should extract suggestions with different formats", () => { - const text = "git push\nOutput:\nCommand patterns: git, git push" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("git push") - expect(result.suggestions).toEqual(["git", "git push"]) - }) - - it('should extract suggestions from "you can allow" format', () => { - const text = "docker run\nOutput:\nYou can allow: docker, docker run" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("docker run") - expect(result.suggestions).toEqual(["docker", "docker run"]) - }) - - it("should extract suggestions from bullet points", () => { - const text = `npm test -Output: -Output here... -- npm -- npm test -- npm run` - const result = parseCommandAndOutput(text) - expect(result.command).toBe("npm test") - expect(result.suggestions).toContain("npm") - expect(result.suggestions).toContain("npm test") - expect(result.suggestions).toContain("npm run") - }) - - it("should extract suggestions from various bullet formats", () => { - const text = `command -Output: -• npm -* git -- docker -▪ python` - const result = parseCommandAndOutput(text) - expect(result.command).toBe("command") - expect(result.suggestions).toContain("npm") - expect(result.suggestions).toContain("git") - expect(result.suggestions).toContain("docker") - expect(result.suggestions).toContain("python") - }) - - it("should extract suggestions with backticks", () => { - const text = "npm install\nOutput:\n- `npm`\n- `npm install`" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("npm install") - expect(result.suggestions).toContain("npm") - expect(result.suggestions).toContain("npm install") - }) - - it("should handle empty text", () => { - const result = parseCommandAndOutput("") - expect(result.command).toBe("") - expect(result.output).toBe("") - expect(result.suggestions).toEqual([]) - }) - - it("should handle multiline commands without Output: separator", () => { - const text = `$ npm install \\ - express \\ - mongoose -Installing...` - const result = parseCommandAndOutput(text) - // Without Output: separator, entire text is treated as command - expect(result.command).toBe(text) - expect(result.output).toBe("") - }) - - it("should include all suggestions from comma-separated list with Output: separator", () => { - const text = "test\nOutput:\nSuggested patterns: npm, npm install, npm run" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("test") - expect(result.suggestions).toEqual(["npm", "npm install", "npm run"]) - }) - - it("should handle case variations in suggestion patterns", () => { - const text = "test\nOutput:\nSuggested Patterns: npm, git\nCommand Patterns: docker" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("test") - // Now it should accumulate all suggestions - expect(result.suggestions).toContain("npm") - expect(result.suggestions).toContain("git") - expect(result.suggestions).toContain("docker") - }) - - it("should handle text already split by Output:", () => { - const text = "npm install && cd backend\nOutput:\ngithub-pr-contributors-tracker@1.0.0 prepare" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("npm install && cd backend") - expect(result.output).toBe("github-pr-contributors-tracker@1.0.0 prepare") - }) - - it("should preserve original command when Output: separator is present", () => { - const text = "npm install\nOutput:\n$ npm install\nInstalling packages..." - const result = parseCommandAndOutput(text) - expect(result.command).toBe("npm install") - expect(result.output).toBe("$ npm install\nInstalling packages...") - }) - - it("should handle Output: separator with no output", () => { - const text = "ls -la\nOutput:" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("ls -la") - expect(result.output).toBe("") - }) - - it("should handle Output: separator with whitespace", () => { - const text = "git status\nOutput: \n On branch main " - const result = parseCommandAndOutput(text) - expect(result.command).toBe("git status") - expect(result.output).toBe("On branch main") - }) - - it("should only use first Output: occurrence as separator", () => { - const text = 'echo "test"\nOutput:\nFirst output\nOutput: Second output' - const result = parseCommandAndOutput(text) - expect(result.command).toBe('echo "test"') - expect(result.output).toBe("First output\nOutput: Second output") - }) - - it("should handle output with numbers at the start of lines", () => { - const text = `wc -l *.go *.java -Output: -25 hello_world.go -316 HelloWorld.java -341 total` - const result = parseCommandAndOutput(text) - expect(result.command).toBe("wc -l *.go *.java") - expect(result.output).toBe("25 hello_world.go\n316 HelloWorld.java\n341 total") - expect(result.suggestions).toEqual([]) - }) - - it("should handle edge case where text starts with Output:", () => { - const text = "Output:\nSome output without a command" - const result = parseCommandAndOutput(text) - expect(result.command).toBe("") - expect(result.output).toBe("Some output without a command") - }) - - it("should not be confused by Output: appearing in the middle of output", () => { - const text = `echo "Output: test" -Output: -Output: test` - const result = parseCommandAndOutput(text) - expect(result.command).toBe('echo "Output: test"') - expect(result.output).toBe("Output: test") - }) - - it("should handle commands without shell prompt when Output: separator is present", () => { - const text = `npm install -Output: -Installing packages...` - const result = parseCommandAndOutput(text) - expect(result.command).toBe("npm install") - expect(result.output).toBe("Installing packages...") - }) - - it("should not parse shell prompts from output when Output: separator exists", () => { - const text = `ls -la -Output: -$ total 341 -drwxr-xr-x 10 user staff 320 Jan 22 12:00 . -drwxr-xr-x 20 user staff 640 Jan 22 11:00 ..` - const result = parseCommandAndOutput(text) - expect(result.command).toBe("ls -la") - expect(result.output).toContain("$ total 341") - expect(result.output).toContain("drwxr-xr-x") - }) -}) - -describe("integration: parseCommandAndOutput with extractCommandPatterns", () => { - it("should not extract patterns from output text", () => { - const text = `wc -l *.go *.java -Output: -wc: *.go: open: No such file or directory -wc: *.java: open: No such file or directory -0 total` - const { command } = parseCommandAndOutput(text) - const patterns = extractCommandPatterns(command) - - // Should only extract patterns from the command, not the output - expect(patterns).toContain("wc") - expect(patterns).not.toContain("0") - expect(patterns).not.toContain("total") - expect(patterns).not.toContain("0 total") - }) - - it("should handle the specific wc command case", () => { - const text = `wc -l *.go *.java -Output: -25 hello_world.go -316 HelloWorld.java -341 total` - const { command } = parseCommandAndOutput(text) - const patterns = extractCommandPatterns(command) - - // Should only extract "wc" from the command - expect(patterns).toEqual(["wc"]) - expect(patterns).not.toContain("341") - expect(patterns).not.toContain("total") - expect(patterns).not.toContain("341 total") - }) - - it("should handle wc command with error output", () => { - const text = `wc -l *.go *.java -Output: -wc: *.go: open: No such file or directory -wc: *.java: open: No such file or directory -0 total` - const { command, output } = parseCommandAndOutput(text) - const patterns = extractCommandPatterns(command) - - // Should only extract "wc" from the command - expect(command).toBe("wc -l *.go *.java") - expect(output).toContain("0 total") - expect(patterns).toEqual(["wc"]) - expect(patterns).not.toContain("0") - expect(patterns).not.toContain("total") - expect(patterns).not.toContain("0 total") - }) - - it("should handle case where only output line is provided", () => { - // This simulates if somehow only "0 total" is passed as the text - const text = "0 total" - const { command } = parseCommandAndOutput(text) - const patterns = extractCommandPatterns(command) - - // In this case, the entire text is treated as command - expect(command).toBe("0 total") - // But "0 total" is not a valid command pattern (starts with number) - expect(patterns).toEqual([]) - }) - - it("should handle commands without output separator", () => { - const text = "npm install" - const { command } = parseCommandAndOutput(text) - const patterns = extractCommandPatterns(command) - - expect(patterns).toEqual(["npm", "npm install"]) - }) -}) diff --git a/webview-ui/src/utils/commandPatterns.ts b/webview-ui/src/utils/commandPatterns.ts deleted file mode 100644 index e68a468b22..0000000000 --- a/webview-ui/src/utils/commandPatterns.ts +++ /dev/null @@ -1,101 +0,0 @@ -import { extractPatternsFromCommand } from "./command-parser" - -export interface CommandPattern { - pattern: string - description?: string -} - -export function extractCommandPatterns(command: string): string[] { - return extractPatternsFromCommand(command) -} - -/** - * Get a human-readable description for a command pattern. - * Simply returns the pattern followed by "commands". - */ -export function getPatternDescription(pattern: string): string { - return `${pattern} commands` -} - -export function parseCommandAndOutput(text: string): { - command: string - output: string - suggestions: string[] -} { - // Default result - const result = { - command: text, - output: "", - suggestions: [] as string[], - } - - // First check if the text already has been split by COMMAND_OUTPUT_STRING - // This happens when the command has already been executed and we have the output - const outputSeparator = "Output:" - const outputIndex = text.indexOf(`\n${outputSeparator}`) - - if (outputIndex !== -1) { - // Text is already split into command and output - // The command is everything before the output separator - result.command = text.slice(0, outputIndex).trim() - // The output is everything after the output separator - // We need to skip the newline and "Output:" text - const afterNewline = outputIndex + 1 // Skip the newline - const afterSeparator = afterNewline + outputSeparator.length // Skip "Output:" - // Check if there's a colon and potential space after it - let startOfOutput = afterSeparator - if (text[afterSeparator] === "\n") { - startOfOutput = afterSeparator + 1 // Skip additional newline after "Output:" - } - result.output = text.slice(startOfOutput).trim() - } else if (text.indexOf(outputSeparator) === 0) { - // Edge case: text starts with "Output:" (no command) - result.command = "" - result.output = text.slice(outputSeparator.length).trim() - } else { - // No output separator found, the entire text is the command - result.command = text.trim() - result.output = "" - } - - // Look for AI suggestions in the output - // These might be in a format like: - // "Suggested patterns: npm, npm install, npm run" - // or as a list - const suggestionPatterns = [ - /Suggested patterns?:\s*(.+?)(?:\n|$)/i, - /Command patterns?:\s*(.+?)(?:\n|$)/i, - /You (?:can|may|might) (?:want to )?(?:allow|add):\s*(.+?)(?:\n|$)/i, - ] - - for (const pattern of suggestionPatterns) { - const match = result.output.match(pattern) - if (match) { - // Split by common delimiters and clean up - const suggestions = match[1] - .split(/[,;]/) - .map((s) => s.trim()) - .filter((s) => s) // Allow multi-word patterns like "npm install" - - if (suggestions.length > 0) { - // Add to existing suggestions instead of replacing - result.suggestions.push(...suggestions) - } - } - } - - // Remove duplicates - result.suggestions = Array.from(new Set(result.suggestions)) - - // Also look for bullet points or numbered lists - // const listPattern = /^[\s\-*•·▪▫◦‣⁃]\s*`?([a-zA-Z0-9_-]+(?:\s+[a-zA-Z0-9_-]+)?)`?$/gm - const lines = result.output.split("\n") - for (const line of lines) { - const match = line.match(/^[\s\-*•·▪▫◦‣⁃]\s*`?([a-zA-Z0-9_-]+(?:\s+[a-zA-Z0-9_-]+)?)`?$/) - if (match && match[1] && !result.suggestions.includes(match[1])) { - result.suggestions.push(match[1]) - } - } - - return result -}