diff --git a/webview-ui/src/components/chat/CommandExecution.tsx b/webview-ui/src/components/chat/CommandExecution.tsx index ad024a378e..4ffbd32d88 100644 --- a/webview-ui/src/components/chat/CommandExecution.tsx +++ b/webview-ui/src/components/chat/CommandExecution.tsx @@ -36,11 +36,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec setDeniedCommands, } = useExtensionState() - const { - command, - output: parsedOutput, - suggestions, - } = useMemo(() => { + const { command, output: parsedOutput } = useMemo(() => { // Use the enhanced parser from commandPatterns return parseCommandAndOutput(text || "") }, [text]) @@ -58,31 +54,23 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec // streaming output (this is the case for running commands). const output = streamingOutput || parsedOutput - // Extract command patterns + // Extract command patterns from the actual command that was executed const commandPatterns = useMemo(() => { const patterns: CommandPattern[] = [] - // Use AI suggestions if available - if (suggestions.length > 0) { - suggestions.forEach((suggestion: string) => { - patterns.push({ - pattern: suggestion, - description: getPatternDescription(suggestion), - }) + // 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), }) - } else { - // Extract patterns programmatically - const extractedPatterns = extractCommandPatterns(command) - extractedPatterns.forEach((pattern) => { - patterns.push({ - pattern, - description: getPatternDescription(pattern), - }) - }) - } + }) return patterns - }, [command, suggestions]) + }, [command]) // Handle pattern changes const handleAllowPatternChange = (pattern: string) => { diff --git a/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx b/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx index 9809726267..162ccd007c 100644 --- a/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx @@ -211,10 +211,11 @@ Suggested patterns: npm, npm install, npm run` expect(codeBlocks[1]).toHaveTextContent("Suggested patterns: npm, npm install, npm run") expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument() - // Check that the patterns are present in the mock + // Check that only patterns from the actual command are extracted, not from AI suggestions expect(screen.getByText("npm")).toBeInTheDocument() expect(screen.getAllByText("npm install").length).toBeGreaterThan(0) - expect(screen.getByText("npm run")).toBeInTheDocument() + // "npm run" should NOT be in the patterns since it's only in the AI suggestions, not the actual command + expect(screen.queryByText("npm run")).not.toBeInTheDocument() }) it("should handle commands with pipes", () => { @@ -417,10 +418,11 @@ Suggested patterns: npm, npm test, npm run const selector = screen.getByTestId("command-pattern-selector") expect(selector).toBeInTheDocument() - // Should show patterns from suggestions + // Should show patterns only from the actual command, not from AI suggestions expect(screen.getAllByText("npm")[0]).toBeInTheDocument() expect(screen.getAllByText("npm test")[0]).toBeInTheDocument() - expect(screen.getAllByText("npm run")[0]).toBeInTheDocument() + // "npm run" should NOT be in the patterns since it's only in the AI suggestions + expect(screen.queryByText("npm run")).not.toBeInTheDocument() }) it("should update both allowed and denied lists when patterns conflict", () => { @@ -519,5 +521,71 @@ Without any command prefix` const codeBlocks = screen.getAllByTestId("code-block") expect(codeBlocks).toHaveLength(1) // Only the command block, no output block }) + + it("should not extract patterns from command output numbers", () => { + // This tests the specific bug where "0 total" from wc output was being extracted as a command + const commandWithNumericOutput = `wc -l *.go *.java +Output: + 10 file1.go + 20 file2.go + 15 Main.java + 45 total` + + render( + + + , + ) + + // Should render the command and output + const codeBlocks = screen.getAllByTestId("code-block") + expect(codeBlocks[0]).toHaveTextContent("wc -l *.go *.java") + + // Should show pattern selector + const selector = screen.getByTestId("command-pattern-selector") + expect(selector).toBeInTheDocument() + + // Should only extract "wc" from the actual command + expect(screen.getByText("wc")).toBeInTheDocument() + + // Should NOT extract numeric patterns from output like "45 total" + expect(screen.queryByText("45")).not.toBeInTheDocument() + expect(screen.queryByText("total")).not.toBeInTheDocument() + expect(screen.queryByText("45 total")).not.toBeInTheDocument() + }) + + it("should handle the edge case of 0 total in output", () => { + // This is the exact case from the bug report + const commandWithZeroTotal = `wc -l *.go *.java +Output: + 0 total` + + render( + + + , + ) + + // Should show pattern selector + const selector = screen.getByTestId("command-pattern-selector") + expect(selector).toBeInTheDocument() + + // Should only extract "wc" from the actual command + // Check within the pattern selector specifically + const patternTexts = Array.from(selector.querySelectorAll("span")).map((el) => el.textContent) + + // Should have "wc" as a pattern + expect(patternTexts).toContain("wc") + + // Should NOT have "0", "total", or "0 total" as patterns + expect(patternTexts).not.toContain("0") + expect(patternTexts).not.toContain("total") + expect(patternTexts).not.toContain("0 total") + + // The output should still be displayed in the code block + const codeBlocks = screen.getAllByTestId("code-block") + expect(codeBlocks.length).toBeGreaterThan(1) + expect(codeBlocks[1]).toHaveTextContent("0 total") + }) }) }) diff --git a/webview-ui/src/utils/__tests__/commandPatterns.spec.ts b/webview-ui/src/utils/__tests__/commandPatterns.spec.ts index 9d26531a4e..92feda0412 100644 --- a/webview-ui/src/utils/__tests__/commandPatterns.spec.ts +++ b/webview-ui/src/utils/__tests__/commandPatterns.spec.ts @@ -101,6 +101,18 @@ describe("extractCommandPatterns", () => { const patterns = extractCommandPatterns("npm run build && git push") expect(patterns).toEqual([...patterns].sort()) }) + + 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", () => { @@ -415,3 +427,75 @@ describe("security integration with extractCommandPatterns", () => { expect(patterns).not.toContain("date") }) }) + +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 index 7fe97f074f..30fadc30df 100644 --- a/webview-ui/src/utils/commandPatterns.ts +++ b/webview-ui/src/utils/commandPatterns.ts @@ -39,7 +39,16 @@ export function extractCommandPatterns(command: string): string[] { } catch (_error) { // If parsing fails, try to extract at least the main command const mainCommand = command.trim().split(/\s+/)[0] - if (mainCommand) patterns.add(mainCommand) + + // Apply same validation as in processCommand + if ( + mainCommand && + !/^\d+$/.test(mainCommand) && // Skip pure numbers + !["total", "error", "warning", "failed", "success", "done"].includes(mainCommand.toLowerCase()) && + (/[a-zA-Z]/.test(mainCommand) || mainCommand.includes("/")) + ) { + patterns.add(mainCommand) + } } return Array.from(patterns).sort() @@ -49,7 +58,20 @@ function processCommand(cmd: any[], patterns: Set) { if (!cmd.length || typeof cmd[0] !== "string") return const mainCmd = cmd[0] - patterns.add(mainCmd) + + // Skip if it's just a number (like "0" from "0 total") + if (/^\d+$/.test(mainCmd)) return + + // Skip common output patterns that aren't commands + const skipWords = ["total", "error", "warning", "failed", "success", "done"] + if (skipWords.includes(mainCmd.toLowerCase())) return + + // Only add if it contains at least one letter or is a valid path + if (/[a-zA-Z]/.test(mainCmd) || mainCmd.includes("/")) { + patterns.add(mainCmd) + } else { + return // Don't process further if main command is invalid + } // Patterns that indicate we should stop looking for subcommands const stopPatterns = [/^-/, /[\\/.~ ]/]