refactor: remove commandPatterns.ts and simplify command parsing

- Remove unnecessary commandPatterns.ts wrapper module
- Use extractPatternsFromCommand directly from command-parser.ts
- Simplify command/output parsing logic in CommandExecution.tsx
- Move CommandPattern interface to components that use it
- All tests passing
This commit is contained in:
Daniel Riccio 2025-07-24 12:16:10 -05:00
parent 627ea8462d
commit cbc756d8b5
No known key found for this signature in database
GPG key ID: FFD5FD825F8E8209
6 changed files with 44 additions and 560 deletions

View file

@ -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<CommandPattern[]>(() => {
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

View file

@ -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[]

View file

@ -21,19 +21,6 @@ vi.mock("../../common/CodeBlock", () => ({
default: ({ source }: { source: string }) => <div data-testid="code-block">{source}</div>,
}))
// Mock the commandPatterns module but use the actual implementation
vi.mock("../../../utils/commandPatterns", async () => {
const actual = await vi.importActual<typeof import("../../../utils/commandPatterns")>(
"../../../utils/commandPatterns",
)
return {
...actual,
parseCommandAndOutput: actual.parseCommandAndOutput,
extractCommandPatterns: actual.extractCommandPatterns,
getPatternDescription: actual.getPatternDescription,
}
})
vi.mock("../CommandPatternSelector", () => ({
CommandPatternSelector: ({ patterns, onAllowPatternChange, onDenyPatternChange }: any) => (
<div data-testid="command-pattern-selector">

View file

@ -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: () => ({

View file

@ -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"])
})
})

View file

@ -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
}