feat: simplify command permissions UI to use full commands instead of patterns

- Replace pattern extraction with single text input for full command
- Allow users to edit command before approving/denying
- Show active state (check/x) based on current allowed/denied lists
- Remove command-parser.ts and related pattern extraction logic
- Update tests to match new simplified behavior

This change addresses user feedback requesting a simpler interface where
users can edit the full command before setting permissions.
This commit is contained in:
Daniel Riccio 2025-07-25 08:23:43 -05:00
parent ea360590af
commit 82350297a4
No known key found for this signature in database
GPG key ID: FFD5FD825F8E8209
6 changed files with 310 additions and 465 deletions

View file

@ -13,12 +13,6 @@ import { cn } from "@src/lib/utils"
import { Button } from "@src/components/ui"
import CodeBlock from "../common/CodeBlock"
import { CommandPatternSelector } from "./CommandPatternSelector"
import { extractPatternsFromCommand } from "../../utils/command-parser"
interface CommandPattern {
pattern: string
description?: string
}
interface CommandExecutionProps {
executionId: string
@ -43,7 +37,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
if (outputIndex !== -1) {
// Text is split into command and output
const cmd = (text ?? '').slice(0, outputIndex).trim()
const cmd = (text ?? "").slice(0, outputIndex).trim()
// Skip the newline and "Output:" text
const afterSeparator = outputIndex + 1 + outputSeparator.length
let startOfOutput = afterSeparator
@ -72,20 +66,11 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
// streaming output (this is the case for running commands).
const output = streamingOutput || parsedOutput
// Extract command patterns from the actual command that was executed
const commandPatterns = useMemo<CommandPattern[]>(() => {
// Extract patterns from the actual command that was executed
const extractedPatterns = extractPatternsFromCommand(command)
return extractedPatterns.map((pattern) => ({
pattern,
}))
}, [command])
// Handle pattern changes
const handleAllowPatternChange = (pattern: string) => {
const isAllowed = allowedCommands.includes(pattern)
const newAllowed = isAllowed ? allowedCommands.filter((p) => p !== pattern) : [...allowedCommands, pattern]
const newDenied = deniedCommands.filter((p) => p !== pattern)
// Handle command changes
const handleAllowCommandChange = (cmd: string) => {
const isAllowed = allowedCommands.includes(cmd)
const newAllowed = isAllowed ? allowedCommands.filter((c) => c !== cmd) : [...allowedCommands, cmd]
const newDenied = deniedCommands.filter((c) => c !== cmd)
setAllowedCommands(newAllowed)
setDeniedCommands(newDenied)
@ -93,10 +78,10 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
vscode.postMessage({ type: "deniedCommands", commands: newDenied })
}
const handleDenyPatternChange = (pattern: string) => {
const isDenied = deniedCommands.includes(pattern)
const newDenied = isDenied ? deniedCommands.filter((p) => p !== pattern) : [...deniedCommands, pattern]
const newAllowed = allowedCommands.filter((p) => p !== pattern)
const handleDenyCommandChange = (cmd: string) => {
const isDenied = deniedCommands.includes(cmd)
const newDenied = isDenied ? deniedCommands.filter((c) => c !== cmd) : [...deniedCommands, cmd]
const newAllowed = allowedCommands.filter((c) => c !== cmd)
setAllowedCommands(newAllowed)
setDeniedCommands(newDenied)
@ -193,13 +178,13 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
<CodeBlock source={command} language="shell" />
<OutputContainer isExpanded={isExpanded} output={output} />
</div>
{commandPatterns.length > 0 && (
{command && (
<CommandPatternSelector
patterns={commandPatterns}
command={command}
allowedCommands={allowedCommands}
deniedCommands={deniedCommands}
onAllowPatternChange={handleAllowPatternChange}
onDenyPatternChange={handleDenyPatternChange}
onAllowCommandChange={handleAllowCommandChange}
onDenyCommandChange={handleDenyCommandChange}
/>
)}
</div>

View file

@ -5,35 +5,33 @@ import { useTranslation, Trans } from "react-i18next"
import { VSCodeLink } from "@vscode/webview-ui-toolkit/react"
import { StandardTooltip } from "../ui/standard-tooltip"
interface CommandPattern {
pattern: string
description?: string
}
interface CommandPatternSelectorProps {
patterns: CommandPattern[]
command: string
allowedCommands: string[]
deniedCommands: string[]
onAllowPatternChange: (pattern: string) => void
onDenyPatternChange: (pattern: string) => void
onAllowCommandChange: (command: string) => void
onDenyCommandChange: (command: string) => void
}
export const CommandPatternSelector: React.FC<CommandPatternSelectorProps> = ({
patterns,
command,
allowedCommands,
deniedCommands,
onAllowPatternChange,
onDenyPatternChange,
onAllowCommandChange,
onDenyCommandChange,
}) => {
const { t } = useTranslation()
const [isExpanded, setIsExpanded] = useState(false)
const [editedCommand, setEditedCommand] = useState(command)
const getPatternStatus = (pattern: string): "allowed" | "denied" | "none" => {
if (allowedCommands.includes(pattern)) return "allowed"
if (deniedCommands.includes(pattern)) return "denied"
const getCommandStatus = (cmd: string): "allowed" | "denied" | "none" => {
if (allowedCommands.includes(cmd)) return "allowed"
if (deniedCommands.includes(cmd)) return "denied"
return "none"
}
const currentStatus = getCommandStatus(editedCommand)
return (
<div className="border-t border-vscode-panel-border bg-vscode-sideBar-background/30">
<button
@ -80,53 +78,48 @@ export const CommandPatternSelector: React.FC<CommandPatternSelectorProps> = ({
</button>
{isExpanded && (
<div className="px-3 pb-3 space-y-2">
{patterns.map((item) => {
const status = getPatternStatus(item.pattern)
return (
<div key={item.pattern} className="ml-5 flex items-center gap-2">
<div className="flex-1">
<span className="font-mono text-xs text-vscode-foreground">{item.pattern}</span>
{item.description && (
<span className="text-xs text-vscode-descriptionForeground ml-2">
- {item.description}
</span>
)}
</div>
<div className="flex items-center gap-1">
<button
className={cn("p-1 rounded transition-all", {
"bg-green-500/20 text-green-500 hover:bg-green-500/30":
status === "allowed",
"text-vscode-descriptionForeground hover:text-green-500 hover:bg-green-500/10":
status !== "allowed",
})}
onClick={() => onAllowPatternChange(item.pattern)}
aria-label={t(
status === "allowed"
? "chat:commandExecution.removeFromAllowed"
: "chat:commandExecution.addToAllowed",
)}>
<Check className="size-3.5" />
</button>
<button
className={cn("p-1 rounded transition-all", {
"bg-red-500/20 text-red-500 hover:bg-red-500/30": status === "denied",
"text-vscode-descriptionForeground hover:text-red-500 hover:bg-red-500/10":
status !== "denied",
})}
onClick={() => onDenyPatternChange(item.pattern)}
aria-label={t(
status === "denied"
? "chat:commandExecution.removeFromDenied"
: "chat:commandExecution.addToDenied",
)}>
<X className="size-3.5" />
</button>
</div>
</div>
)
})}
<div className="px-3 pb-3">
<div className="ml-5 flex items-center gap-2">
<div className="flex-1">
<input
type="text"
value={editedCommand}
onChange={(e) => setEditedCommand(e.target.value)}
className="font-mono text-xs bg-vscode-input-background text-vscode-input-foreground border border-vscode-input-border rounded px-2 py-1 w-full focus:outline-none focus:border-vscode-focusBorder"
placeholder={command}
/>
</div>
<div className="flex items-center gap-1">
<button
className={cn("p-1 rounded transition-all", {
"bg-green-500/20 text-green-500 hover:bg-green-500/30": currentStatus === "allowed",
"text-vscode-descriptionForeground hover:text-green-500 hover:bg-green-500/10":
currentStatus !== "allowed",
})}
onClick={() => onAllowCommandChange(editedCommand)}
aria-label={t(
currentStatus === "allowed"
? "chat:commandExecution.removeFromAllowed"
: "chat:commandExecution.addToAllowed",
)}>
<Check className="size-3.5" />
</button>
<button
className={cn("p-1 rounded transition-all", {
"bg-red-500/20 text-red-500 hover:bg-red-500/30": currentStatus === "denied",
"text-vscode-descriptionForeground hover:text-red-500 hover:bg-red-500/10":
currentStatus !== "denied",
})}
onClick={() => onDenyCommandChange(editedCommand)}
aria-label={t(
currentStatus === "denied"
? "chat:commandExecution.removeFromDenied"
: "chat:commandExecution.addToDenied",
)}>
<X className="size-3.5" />
</button>
</div>
</div>
</div>
)}
</div>

View file

@ -22,15 +22,11 @@ vi.mock("../../common/CodeBlock", () => ({
}))
vi.mock("../CommandPatternSelector", () => ({
CommandPatternSelector: ({ patterns, onAllowPatternChange, onDenyPatternChange }: any) => (
CommandPatternSelector: ({ command, onAllowCommandChange, onDenyCommandChange }: any) => (
<div data-testid="command-pattern-selector">
{patterns.map((p: any, i: number) => (
<div key={i}>
<span>{p.pattern}</span>
<button onClick={() => onAllowPatternChange(p.pattern)}>Allow {p.pattern}</button>
<button onClick={() => onDenyPatternChange(p.pattern)}>Deny {p.pattern}</button>
</div>
))}
<span>{command}</span>
<button onClick={() => onAllowCommandChange(command)}>Allow {command}</button>
<button onClick={() => onDenyCommandChange(command)}>Deny {command}</button>
</div>
),
}))
@ -88,7 +84,7 @@ describe("CommandExecution", () => {
expect(screen.getByTestId("custom-title")).toBeInTheDocument()
})
it("should show command pattern selector for simple commands", () => {
it("should show command pattern selector for commands", () => {
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-1" text="npm install express" />
@ -96,72 +92,85 @@ describe("CommandExecution", () => {
)
expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument()
expect(screen.getByText("npm")).toBeInTheDocument()
expect(screen.getByText("npm install")).toBeInTheDocument()
expect(screen.getByText("npm install express")).toBeInTheDocument()
})
it("should handle allow pattern change", () => {
it("should handle allow command change", () => {
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-1" text="git push" />
</ExtensionStateWrapper>,
)
const allowButton = screen.getByText("Allow git")
const allowButton = screen.getByText("Allow git push")
fireEvent.click(allowButton)
expect(mockExtensionState.setAllowedCommands).toHaveBeenCalledWith(["npm", "git"])
expect(mockExtensionState.setAllowedCommands).toHaveBeenCalledWith(["npm", "git push"])
expect(mockExtensionState.setDeniedCommands).toHaveBeenCalledWith(["rm"])
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "allowedCommands", commands: ["npm", "git"] })
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "allowedCommands", commands: ["npm", "git push"] })
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "deniedCommands", commands: ["rm"] })
})
it("should handle deny pattern change", () => {
it("should handle deny command change", () => {
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-1" text="docker run" />
</ExtensionStateWrapper>,
)
const denyButton = screen.getByText("Deny docker")
const denyButton = screen.getByText("Deny docker run")
fireEvent.click(denyButton)
expect(mockExtensionState.setAllowedCommands).toHaveBeenCalledWith(["npm"])
expect(mockExtensionState.setDeniedCommands).toHaveBeenCalledWith(["rm", "docker"])
expect(mockExtensionState.setDeniedCommands).toHaveBeenCalledWith(["rm", "docker run"])
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "allowedCommands", commands: ["npm"] })
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "deniedCommands", commands: ["rm", "docker"] })
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "deniedCommands", commands: ["rm", "docker run"] })
})
it("should toggle allowed pattern", () => {
it("should toggle allowed command", () => {
// Update the mock state to have "npm test" in allowedCommands
const stateWithNpmTest = {
...mockExtensionState,
allowedCommands: ["npm test"],
deniedCommands: ["rm"],
}
render(
<ExtensionStateWrapper>
<ExtensionStateContext.Provider value={stateWithNpmTest as any}>
<CommandExecution executionId="test-1" text="npm test" />
</ExtensionStateWrapper>,
</ExtensionStateContext.Provider>,
)
const allowButton = screen.getByText("Allow npm")
const allowButton = screen.getByText("Allow npm test")
fireEvent.click(allowButton)
// npm is already in allowedCommands, so it should be removed
expect(mockExtensionState.setAllowedCommands).toHaveBeenCalledWith([])
expect(mockExtensionState.setDeniedCommands).toHaveBeenCalledWith(["rm"])
// "npm test" is already in allowedCommands, so it should be removed
expect(stateWithNpmTest.setAllowedCommands).toHaveBeenCalledWith([])
expect(stateWithNpmTest.setDeniedCommands).toHaveBeenCalledWith(["rm"])
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "allowedCommands", commands: [] })
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "deniedCommands", commands: ["rm"] })
})
it("should toggle denied pattern", () => {
it("should toggle denied command", () => {
// Update the mock state to have "rm -rf" in deniedCommands
const stateWithRmRf = {
...mockExtensionState,
allowedCommands: ["npm"],
deniedCommands: ["rm -rf"],
}
render(
<ExtensionStateWrapper>
<ExtensionStateContext.Provider value={stateWithRmRf as any}>
<CommandExecution executionId="test-1" text="rm -rf" />
</ExtensionStateWrapper>,
</ExtensionStateContext.Provider>,
)
const denyButton = screen.getByText("Deny rm")
const denyButton = screen.getByText("Deny rm -rf")
fireEvent.click(denyButton)
// rm is already in deniedCommands, so it should be removed
expect(mockExtensionState.setAllowedCommands).toHaveBeenCalledWith(["npm"])
expect(mockExtensionState.setDeniedCommands).toHaveBeenCalledWith([])
// "rm -rf" is already in deniedCommands, so it should be removed
expect(stateWithRmRf.setAllowedCommands).toHaveBeenCalledWith(["npm"])
expect(stateWithRmRf.setDeniedCommands).toHaveBeenCalledWith([])
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "allowedCommands", commands: ["npm"] })
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "deniedCommands", commands: [] })
})
@ -181,7 +190,7 @@ Installing...`
expect(codeBlocks[0]).toHaveTextContent("npm install")
})
it("should parse command with AI suggestions", () => {
it("should parse command with output", () => {
const commandText = `npm install
Output:
Suggested patterns: npm, npm install, npm run`
@ -198,11 +207,8 @@ 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 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)
// "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()
// Should show the full command
expect(screen.getByText("npm install")).toBeInTheDocument()
})
it("should handle commands with pipes", () => {
@ -213,8 +219,7 @@ Suggested patterns: npm, npm install, npm run`
)
expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument()
expect(screen.getByText("ls")).toBeInTheDocument()
expect(screen.getByText("grep")).toBeInTheDocument()
expect(screen.getByText("ls -la | grep test")).toBeInTheDocument()
})
it("should handle commands with && operator", () => {
@ -225,9 +230,7 @@ Suggested patterns: npm, npm install, npm run`
)
expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument()
expect(screen.getByText("npm")).toBeInTheDocument()
expect(screen.getByText("npm install")).toBeInTheDocument()
expect(screen.getByText("npm test")).toBeInTheDocument()
expect(screen.getByText("npm install && npm test")).toBeInTheDocument()
})
it("should not show pattern selector for empty commands", () => {
@ -279,25 +282,32 @@ Output here`
expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument()
})
it("should handle pattern change when moving from denied to allowed", () => {
it("should handle command change when moving from denied to allowed", () => {
// Update the mock state to have "rm file.txt" in deniedCommands
const stateWithRmInDenied = {
...mockExtensionState,
allowedCommands: ["npm"],
deniedCommands: ["rm file.txt"],
}
render(
<ExtensionStateWrapper>
<ExtensionStateContext.Provider value={stateWithRmInDenied as any}>
<CommandExecution executionId="test-1" text="rm file.txt" />
</ExtensionStateWrapper>,
</ExtensionStateContext.Provider>,
)
const allowButton = screen.getByText("Allow rm")
const allowButton = screen.getByText("Allow rm file.txt")
fireEvent.click(allowButton)
// rm should be removed from denied and added to allowed
expect(mockExtensionState.setAllowedCommands).toHaveBeenCalledWith(["npm", "rm"])
expect(mockExtensionState.setDeniedCommands).toHaveBeenCalledWith([])
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "allowedCommands", commands: ["npm", "rm"] })
// "rm file.txt" should be removed from denied and added to allowed
expect(stateWithRmInDenied.setAllowedCommands).toHaveBeenCalledWith(["npm", "rm file.txt"])
expect(stateWithRmInDenied.setDeniedCommands).toHaveBeenCalledWith([])
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "allowedCommands", commands: ["npm", "rm file.txt"] })
expect(vscode.postMessage).toHaveBeenCalledWith({ type: "deniedCommands", commands: [] })
})
describe("integration with CommandPatternSelector", () => {
it("should extract patterns from complex commands with multiple operators", () => {
it("should show complex commands with multiple operators", () => {
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-6" text="npm install && npm test || echo 'failed'" />
@ -306,23 +316,20 @@ Output here`
const selector = screen.getByTestId("command-pattern-selector")
expect(selector).toBeInTheDocument()
expect(screen.getByText("npm")).toBeInTheDocument()
expect(screen.getByText("npm install")).toBeInTheDocument()
expect(screen.getByText("npm test")).toBeInTheDocument()
expect(screen.getByText("echo")).toBeInTheDocument()
expect(screen.getByText("npm install && npm test || echo 'failed'")).toBeInTheDocument()
})
it("should handle commands with malformed suggestions gracefully", () => {
const commandWithMalformedSuggestions = `npm install
it("should handle commands with output", () => {
const commandWithOutput = `npm install
Output:
Suggested patterns: npm, , npm install,
Installing packages...
Other output here`
render(
<ExtensionStateWrapper>
<CommandExecution
executionId="test-6"
text={commandWithMalformedSuggestions}
text={commandWithOutput}
icon={<span>icon</span>}
title={<span>Run Command</span>}
/>
@ -331,12 +338,11 @@ Other output here`
const selector = screen.getByTestId("command-pattern-selector")
expect(selector).toBeInTheDocument()
// Should still show valid patterns
expect(screen.getAllByText("npm")[0]).toBeInTheDocument()
expect(screen.getAllByText("npm install")[0]).toBeInTheDocument()
// Should show the command
expect(screen.getByText("npm install")).toBeInTheDocument()
})
it("should handle commands with subshells by not including them in patterns", () => {
it("should handle commands with subshells", () => {
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-7" text="echo $(whoami) && git status" />
@ -345,11 +351,7 @@ Other output here`
const selector = screen.getByTestId("command-pattern-selector")
expect(selector).toBeInTheDocument()
expect(screen.getByText("echo")).toBeInTheDocument()
expect(screen.getByText("git")).toBeInTheDocument()
expect(screen.getByText("git status")).toBeInTheDocument()
// Should not include subshell content
expect(screen.queryByText("whoami")).not.toBeInTheDocument()
expect(screen.getByText("echo $(whoami) && git status")).toBeInTheDocument()
})
it("should handle commands with backtick subshells", () => {
@ -361,13 +363,10 @@ Other output here`
const selector = screen.getByTestId("command-pattern-selector")
expect(selector).toBeInTheDocument()
expect(screen.getByText("git")).toBeInTheDocument()
expect(screen.getByText("git commit")).toBeInTheDocument()
// Should not include subshell content
expect(screen.queryByText("date")).not.toBeInTheDocument()
expect(screen.getByText("git commit -m `date`")).toBeInTheDocument()
})
it("should handle pattern changes for commands with special characters", () => {
it("should handle commands with special characters", () => {
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-9" text="cd ~/projects && npm start" />
@ -376,22 +375,15 @@ Other output here`
const selector = screen.getByTestId("command-pattern-selector")
expect(selector).toBeInTheDocument()
expect(screen.getByText("cd")).toBeInTheDocument()
expect(screen.getByText("npm")).toBeInTheDocument()
expect(screen.getByText("npm start")).toBeInTheDocument()
expect(screen.getByText("cd ~/projects && npm start")).toBeInTheDocument()
})
it("should handle commands with mixed content including output and suggestions", () => {
it("should handle commands with mixed content including output", () => {
const commandWithMixedContent = `npm test
Output:
Running tests...
Test 1 passed
Test 2 passed
Suggested patterns: npm, npm test, npm run
- npm
- npm test
- npm run test`
Test 2 passed`
render(
<ExtensionStateWrapper>
@ -406,18 +398,15 @@ Suggested patterns: npm, npm test, npm run
const selector = screen.getByTestId("command-pattern-selector")
expect(selector).toBeInTheDocument()
// 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()
// "npm run" should NOT be in the patterns since it's only in the AI suggestions
expect(screen.queryByText("npm run")).not.toBeInTheDocument()
// Should show the command
expect(screen.getByText("npm test")).toBeInTheDocument()
})
it("should update both allowed and denied lists when patterns conflict", () => {
it("should update both allowed and denied lists when commands conflict", () => {
const conflictState = {
...mockExtensionState,
allowedCommands: ["git"],
deniedCommands: ["git push"],
deniedCommands: ["git push origin main"],
}
render(
@ -426,31 +415,31 @@ Suggested patterns: npm, npm test, npm run
</ExtensionStateContext.Provider>,
)
// Click to allow "git push"
const allowButton = screen.getByText("Allow git push")
// Click to allow "git push origin main"
const allowButton = screen.getByText("Allow git push origin main")
fireEvent.click(allowButton)
// Should add to allowed and remove from denied
expect(conflictState.setAllowedCommands).toHaveBeenCalledWith(["git", "git push"])
expect(conflictState.setAllowedCommands).toHaveBeenCalledWith(["git", "git push origin main"])
expect(conflictState.setDeniedCommands).toHaveBeenCalledWith([])
})
it("should handle commands that cannot be parsed and fallback gracefully", () => {
// Test with a command that might cause parsing issues
const unparsableCommand = "echo 'test with unclosed quote"
it("should handle commands with special quotes", () => {
// Test with a command that has quotes
const commandWithQuotes = "echo 'test with unclosed quote"
render(
<ExtensionStateWrapper>
<CommandExecution executionId="test-12" text={unparsableCommand} />
<CommandExecution executionId="test-12" text={commandWithQuotes} />
</ExtensionStateWrapper>,
)
// Should still render the command
expect(screen.getByTestId("code-block")).toHaveTextContent("echo 'test with unclosed quote")
// Should show pattern selector with at least the main command
// Should show pattern selector with the full command
expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument()
expect(screen.getByText("echo")).toBeInTheDocument()
expect(screen.getByText("echo 'test with unclosed quote")).toBeInTheDocument()
})
it("should handle empty or whitespace-only commands", () => {
@ -486,9 +475,7 @@ Without any command prefix`
expect(codeBlock.textContent).toContain("Without any command prefix")
})
it("should handle fallback case where parsed command equals original text", () => {
// This tests the case where parseCommandAndOutput returns command === text
// which happens when there's no output separator or command prefix
it("should handle simple commands", () => {
const plainCommand = "docker build ."
render(
@ -500,18 +487,16 @@ Without any command prefix`
// Should render the command
expect(screen.getByTestId("code-block")).toHaveTextContent("docker build .")
// Should show pattern selector with extracted patterns
// Should show pattern selector with the full command
expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument()
expect(screen.getByText("docker")).toBeInTheDocument()
expect(screen.getByText("docker build")).toBeInTheDocument()
expect(screen.getByText("docker build .")).toBeInTheDocument()
// Verify no output is shown (since command === text means no output)
// Verify no output is shown (since there's no Output: separator)
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
it("should handle commands with numeric output", () => {
const commandWithNumericOutput = `wc -l *.go *.java
Output:
10 file1.go
@ -533,17 +518,15 @@ Output:
const selector = screen.getByTestId("command-pattern-selector")
expect(selector).toBeInTheDocument()
// Should only extract "wc" from the actual command
expect(screen.getByText("wc")).toBeInTheDocument()
// Should show the full command
expect(screen.getByText("wc -l *.go *.java")).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()
// The output should still be displayed in the code block
expect(codeBlocks.length).toBeGreaterThan(1)
expect(codeBlocks[1].textContent).toContain("45 total")
})
it("should handle the edge case of 0 total in output", () => {
// This is the exact case from the bug report
it("should handle commands with zero output", () => {
const commandWithZeroTotal = `wc -l *.go *.java
Output:
0 total`
@ -558,17 +541,8 @@ Output:
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")
// Should show the full command
expect(screen.getByText("wc -l *.go *.java")).toBeInTheDocument()
// The output should still be displayed in the code block
const codeBlocks = screen.getAllByTestId("code-block")

View file

@ -4,11 +4,6 @@ import { describe, it, expect, vi } from "vitest"
import { CommandPatternSelector } from "../CommandPatternSelector"
import { TooltipProvider } from "../../../components/ui/tooltip"
interface CommandPattern {
pattern: string
description?: string
}
// Mock react-i18next
vi.mock("react-i18next", () => ({
useTranslation: () => ({
@ -30,21 +25,15 @@ vi.mock("@vscode/webview-ui-toolkit/react", () => ({
const TestWrapper = ({ children }: { children: React.ReactNode }) => <TooltipProvider>{children}</TooltipProvider>
describe("CommandPatternSelector", () => {
const mockPatterns: CommandPattern[] = [
{ pattern: "npm", description: "npm commands" },
{ pattern: "npm install", description: "npm install commands" },
{ pattern: "git", description: "git commands" },
]
const defaultProps = {
patterns: mockPatterns,
allowedCommands: ["npm"],
deniedCommands: ["git"],
onAllowPatternChange: vi.fn(),
onDenyPatternChange: vi.fn(),
command: "npm install express",
allowedCommands: ["npm install"],
deniedCommands: ["git push"],
onAllowCommandChange: vi.fn(),
onDenyCommandChange: vi.fn(),
}
it("should render with unique pattern keys", () => {
it("should render with command input", () => {
const { container } = render(
<TestWrapper>
<CommandPatternSelector {...defaultProps} />
@ -58,39 +47,148 @@ describe("CommandPatternSelector", () => {
const expandButton = screen.getByRole("button", { name: /chat:commandExecution.expandManagement/i })
fireEvent.click(expandButton)
// Check that patterns are rendered
expect(screen.getByText("npm")).toBeInTheDocument()
expect(screen.getByText("npm install")).toBeInTheDocument()
expect(screen.getByText("git")).toBeInTheDocument()
// Check that the input is rendered with the command
const input = screen.getByDisplayValue("npm install express")
expect(input).toBeInTheDocument()
})
it("should handle duplicate patterns gracefully", () => {
// Test with duplicate patterns to ensure keys are still unique
const duplicatePatterns: CommandPattern[] = [
{ pattern: "npm", description: "npm commands" },
{ pattern: "npm", description: "duplicate npm commands" }, // Duplicate pattern
{ pattern: "git", description: "git commands" },
]
const props = {
...defaultProps,
patterns: duplicatePatterns,
}
// This should not throw an error even with duplicate patterns
const { container } = render(
it("should allow editing the command", () => {
render(
<TestWrapper>
<CommandPatternSelector {...props} />
<CommandPatternSelector {...defaultProps} />
</TestWrapper>,
)
expect(container).toBeTruthy()
// Click to expand the component
const expandButton = screen.getByRole("button", { name: /chat:commandExecution.expandManagement/i })
fireEvent.click(expandButton)
// Both instances of "npm" should be rendered
const npmElements = screen.getAllByText("npm")
expect(npmElements).toHaveLength(2)
// Get the input and change its value
const input = screen.getByDisplayValue("npm install express") as HTMLInputElement
fireEvent.change(input, { target: { value: "npm install react" } })
// Check that the input value has changed
expect(input.value).toBe("npm install react")
})
it("should show allowed status for commands in allowed list", () => {
const props = {
...defaultProps,
command: "npm install",
}
render(
<TestWrapper>
<CommandPatternSelector {...props} />
</TestWrapper>,
)
// Click to expand the component
const expandButton = screen.getByRole("button", { name: /chat:commandExecution.expandManagement/i })
fireEvent.click(expandButton)
// The allow button should have the active styling (we can check by aria-label)
const allowButton = screen.getByRole("button", { name: /chat:commandExecution.removeFromAllowed/i })
expect(allowButton).toBeInTheDocument()
})
it("should show denied status for commands in denied list", () => {
const props = {
...defaultProps,
command: "git push",
}
render(
<TestWrapper>
<CommandPatternSelector {...props} />
</TestWrapper>,
)
// Click to expand the component
const expandButton = screen.getByRole("button", { name: /chat:commandExecution.expandManagement/i })
fireEvent.click(expandButton)
// The deny button should have the active styling (we can check by aria-label)
const denyButton = screen.getByRole("button", { name: /chat:commandExecution.removeFromDenied/i })
expect(denyButton).toBeInTheDocument()
})
it("should call onAllowCommandChange when allow button is clicked", () => {
const mockOnAllowCommandChange = vi.fn()
const props = {
...defaultProps,
onAllowCommandChange: mockOnAllowCommandChange,
}
render(
<TestWrapper>
<CommandPatternSelector {...props} />
</TestWrapper>,
)
// Click to expand the component
const expandButton = screen.getByRole("button", { name: /chat:commandExecution.expandManagement/i })
fireEvent.click(expandButton)
// Click the allow button
const allowButton = screen.getByRole("button", { name: /chat:commandExecution.addToAllowed/i })
fireEvent.click(allowButton)
// Check that the callback was called with the command
expect(mockOnAllowCommandChange).toHaveBeenCalledWith("npm install express")
})
it("should call onDenyCommandChange when deny button is clicked", () => {
const mockOnDenyCommandChange = vi.fn()
const props = {
...defaultProps,
onDenyCommandChange: mockOnDenyCommandChange,
}
render(
<TestWrapper>
<CommandPatternSelector {...props} />
</TestWrapper>,
)
// Click to expand the component
const expandButton = screen.getByRole("button", { name: /chat:commandExecution.expandManagement/i })
fireEvent.click(expandButton)
// Click the deny button
const denyButton = screen.getByRole("button", { name: /chat:commandExecution.addToDenied/i })
fireEvent.click(denyButton)
// Check that the callback was called with the command
expect(mockOnDenyCommandChange).toHaveBeenCalledWith("npm install express")
})
it("should use edited command value when buttons are clicked", () => {
const mockOnAllowCommandChange = vi.fn()
const props = {
...defaultProps,
onAllowCommandChange: mockOnAllowCommandChange,
}
render(
<TestWrapper>
<CommandPatternSelector {...props} />
</TestWrapper>,
)
// Click to expand the component
const expandButton = screen.getByRole("button", { name: /chat:commandExecution.expandManagement/i })
fireEvent.click(expandButton)
// Edit the command
const input = screen.getByDisplayValue("npm install express") as HTMLInputElement
fireEvent.change(input, { target: { value: "npm install react" } })
// Click the allow button
const allowButton = screen.getByRole("button", { name: /chat:commandExecution.addToAllowed/i })
fireEvent.click(allowButton)
// Check that the callback was called with the edited command
expect(mockOnAllowCommandChange).toHaveBeenCalledWith("npm install react")
})
})

View file

@ -1,137 +0,0 @@
import { describe, it, expect } from "vitest"
import { extractPatternsFromCommand } from "../command-parser"
describe("extractPatternsFromCommand", () => {
it("should extract simple command pattern", () => {
const patterns = extractPatternsFromCommand("ls")
expect(patterns).toEqual(["ls"])
})
it("should extract command with subcommand", () => {
const patterns = extractPatternsFromCommand("git push origin main")
expect(patterns).toEqual(["git", "git push", "git push origin"])
})
it("should stop at flags", () => {
const patterns = extractPatternsFromCommand("git commit -m 'test'")
expect(patterns).toEqual(["git", "git commit"])
})
it("should stop at paths", () => {
const patterns = extractPatternsFromCommand("cd /usr/local/bin")
expect(patterns).toEqual(["cd"])
})
it("should handle pipes", () => {
const patterns = extractPatternsFromCommand("ls -la | grep test")
expect(patterns).toEqual(["grep", "grep test", "ls"])
})
it("should handle && operator", () => {
const patterns = extractPatternsFromCommand("npm install && git push origin main")
expect(patterns).toEqual(["git", "git push", "git push origin", "npm", "npm install"])
})
it("should handle || operator", () => {
const patterns = extractPatternsFromCommand("npm test || npm run test:ci")
expect(patterns).toEqual(["npm", "npm run", "npm test"])
})
it("should handle semicolon separator", () => {
const patterns = extractPatternsFromCommand("cd src; npm install")
expect(patterns).toEqual(["cd", "cd src", "npm", "npm install"])
})
it("should skip numeric commands", () => {
const patterns = extractPatternsFromCommand("0 total")
expect(patterns).toEqual([])
})
it("should handle empty command", () => {
const patterns = extractPatternsFromCommand("")
expect(patterns).toEqual([])
})
it("should handle null/undefined", () => {
expect(extractPatternsFromCommand(null as any)).toEqual([])
expect(extractPatternsFromCommand(undefined as any)).toEqual([])
})
it("should handle scripts", () => {
const patterns = extractPatternsFromCommand("./script.sh --verbose")
expect(patterns).toEqual(["./script.sh"])
})
it("should handle paths with dots", () => {
const patterns = extractPatternsFromCommand("git add .")
expect(patterns).toEqual(["git", "git add"])
})
it("should handle paths with tilde", () => {
const patterns = extractPatternsFromCommand("cd ~/projects")
expect(patterns).toEqual(["cd"])
})
it("should handle colons in arguments", () => {
const patterns = extractPatternsFromCommand("docker run image:tag")
expect(patterns).toEqual(["docker", "docker run"])
})
it("should return sorted patterns", () => {
const patterns = extractPatternsFromCommand("npm run build && git push")
expect(patterns).toEqual(["git", "git push", "npm", "npm run", "npm run build"])
})
it("should handle complex command with multiple operators", () => {
const patterns = extractPatternsFromCommand("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("echo")
})
it("should handle malformed commands gracefully", () => {
const patterns = extractPatternsFromCommand("echo 'unclosed quote")
expect(patterns).toContain("echo")
})
it("should not treat package managers specially", () => {
const patterns = extractPatternsFromCommand("npm run build")
expect(patterns).toEqual(["npm", "npm run", "npm run build"])
// Now includes "npm run build" with 3-level extraction
})
it("should extract at most 3 levels", () => {
const patterns = extractPatternsFromCommand("git push origin main --force")
expect(patterns).toEqual(["git", "git push", "git push origin"])
// Should NOT include deeper levels beyond 3
})
it("should handle multi-level commands like gh pr", () => {
const patterns = extractPatternsFromCommand("gh pr checkout 123")
expect(patterns).toEqual(["gh", "gh pr", "gh pr checkout"])
})
it("should extract 3 levels for git remote add", () => {
const patterns = extractPatternsFromCommand("git remote add origin https://github.com/user/repo.git")
expect(patterns).toEqual(["git", "git remote", "git remote add"])
})
it("should extract 3 levels for npm run build", () => {
const patterns = extractPatternsFromCommand("npm run build --production")
expect(patterns).toEqual(["npm", "npm run", "npm run build"])
})
it("should stop at file extensions even at third level", () => {
const patterns = extractPatternsFromCommand("node scripts test.js")
expect(patterns).toEqual(["node", "node scripts"])
// Should NOT include "node scripts test.js" because of .js
})
it("should stop at flags at any level", () => {
const patterns = extractPatternsFromCommand("docker run -it ubuntu")
expect(patterns).toEqual(["docker", "docker run"])
// Stops at -it flag
})
})

View file

@ -1,68 +0,0 @@
import { parse } from "shell-quote"
/**
* Extract command patterns from a command string.
* Returns at most 3 levels: base command, command + first argument, and command + first two arguments.
* Stops at flags (-), paths (/\~), file extensions (.ext), or special characters (:).
*/
export function extractPatternsFromCommand(command: string): string[] {
if (!command?.trim()) return []
const patterns = new Set<string>()
try {
const parsed = parse(command)
const commandSeparators = new Set(["|", "&&", "||", ";"])
let currentTokens: string[] = []
for (const token of parsed) {
if (typeof token === "object" && "op" in token && commandSeparators.has(token.op)) {
// Process accumulated tokens as a command
if (currentTokens.length > 0) {
extractFromTokens(currentTokens, patterns)
currentTokens = []
}
} else if (typeof token === "string") {
currentTokens.push(token)
}
}
// Process any remaining tokens
if (currentTokens.length > 0) {
extractFromTokens(currentTokens, patterns)
}
} catch (error) {
console.warn("Failed to parse command:", error)
// Fallback: just extract the first word
const firstWord = command.trim().split(/\s+/)[0]
if (firstWord) patterns.add(firstWord)
}
return Array.from(patterns).sort()
}
function isValidToken(token: string): boolean {
return !!token && !token.startsWith("-") && !token.match(/[/\\~:]/) && token !== "." && !token.match(/\.\w+$/)
}
function extractFromTokens(tokens: string[], patterns: Set<string>): void {
if (tokens.length === 0) return
const mainCmd = tokens[0]
// Skip numeric commands like "0" from "0 total"
if (/^\d+$/.test(mainCmd)) return
// Build patterns progressively up to 3 levels
let pattern = mainCmd
patterns.add(pattern)
for (let i = 1; i < Math.min(tokens.length, 3); i++) {
if (isValidToken(tokens[i])) {
pattern += ` ${tokens[i]}`
patterns.add(pattern)
} else {
break // Stop at first invalid token
}
}
}