Skip to content

Commit f6642f6

Browse files
committed
fix: prevent command output from appearing in permissions UI
- Fixed CommandExecution.tsx to only extract patterns from actual commands, not AI suggestions - Enhanced extractCommandPatterns to filter out numeric patterns and common output words - Added comprehensive test coverage for the bug scenario - Ensures 'Manage Command Permissions' only shows actual executed commands Fixes the issue where output like '0 total' from wc commands was incorrectly shown as a command pattern
1 parent ebf1b24 commit f6642f6

File tree

4 files changed

+192
-30
lines changed

4 files changed

+192
-30
lines changed

webview-ui/src/components/chat/CommandExecution.tsx

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
3636
setDeniedCommands,
3737
} = useExtensionState()
3838

39-
const {
40-
command,
41-
output: parsedOutput,
42-
suggestions,
43-
} = useMemo(() => {
39+
const { command, output: parsedOutput } = useMemo(() => {
4440
// Use the enhanced parser from commandPatterns
4541
return parseCommandAndOutput(text || "")
4642
}, [text])
@@ -58,31 +54,23 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
5854
// streaming output (this is the case for running commands).
5955
const output = streamingOutput || parsedOutput
6056

61-
// Extract command patterns
57+
// Extract command patterns from the actual command that was executed
6258
const commandPatterns = useMemo<CommandPattern[]>(() => {
6359
const patterns: CommandPattern[] = []
6460

65-
// Use AI suggestions if available
66-
if (suggestions.length > 0) {
67-
suggestions.forEach((suggestion: string) => {
68-
patterns.push({
69-
pattern: suggestion,
70-
description: getPatternDescription(suggestion),
71-
})
72-
})
73-
} else {
74-
// Extract patterns programmatically
75-
const extractedPatterns = extractCommandPatterns(command)
76-
extractedPatterns.forEach((pattern) => {
77-
patterns.push({
78-
pattern,
79-
description: getPatternDescription(pattern),
80-
})
61+
// Always extract patterns from the actual command that was executed
62+
// We don't use AI suggestions because the patterns should reflect
63+
// what was actually executed, not what the AI thinks might be useful
64+
const extractedPatterns = extractCommandPatterns(command)
65+
extractedPatterns.forEach((pattern) => {
66+
patterns.push({
67+
pattern,
68+
description: getPatternDescription(pattern),
8169
})
82-
}
70+
})
8371

8472
return patterns
85-
}, [command, suggestions])
73+
}, [command])
8674

8775
// Handle pattern changes
8876
const handleAllowPatternChange = (pattern: string) => {

webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,10 +211,11 @@ Suggested patterns: npm, npm install, npm run`
211211
expect(codeBlocks[1]).toHaveTextContent("Suggested patterns: npm, npm install, npm run")
212212

213213
expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument()
214-
// Check that the patterns are present in the mock
214+
// Check that only patterns from the actual command are extracted, not from AI suggestions
215215
expect(screen.getByText("npm")).toBeInTheDocument()
216216
expect(screen.getAllByText("npm install").length).toBeGreaterThan(0)
217-
expect(screen.getByText("npm run")).toBeInTheDocument()
217+
// "npm run" should NOT be in the patterns since it's only in the AI suggestions, not the actual command
218+
expect(screen.queryByText("npm run")).not.toBeInTheDocument()
218219
})
219220

220221
it("should handle commands with pipes", () => {
@@ -417,10 +418,11 @@ Suggested patterns: npm, npm test, npm run
417418

418419
const selector = screen.getByTestId("command-pattern-selector")
419420
expect(selector).toBeInTheDocument()
420-
// Should show patterns from suggestions
421+
// Should show patterns only from the actual command, not from AI suggestions
421422
expect(screen.getAllByText("npm")[0]).toBeInTheDocument()
422423
expect(screen.getAllByText("npm test")[0]).toBeInTheDocument()
423-
expect(screen.getAllByText("npm run")[0]).toBeInTheDocument()
424+
// "npm run" should NOT be in the patterns since it's only in the AI suggestions
425+
expect(screen.queryByText("npm run")).not.toBeInTheDocument()
424426
})
425427

426428
it("should update both allowed and denied lists when patterns conflict", () => {
@@ -519,5 +521,71 @@ Without any command prefix`
519521
const codeBlocks = screen.getAllByTestId("code-block")
520522
expect(codeBlocks).toHaveLength(1) // Only the command block, no output block
521523
})
524+
525+
it("should not extract patterns from command output numbers", () => {
526+
// This tests the specific bug where "0 total" from wc output was being extracted as a command
527+
const commandWithNumericOutput = `wc -l *.go *.java
528+
Output:
529+
10 file1.go
530+
20 file2.go
531+
15 Main.java
532+
45 total`
533+
534+
render(
535+
<ExtensionStateWrapper>
536+
<CommandExecution executionId="test-16" text={commandWithNumericOutput} />
537+
</ExtensionStateWrapper>,
538+
)
539+
540+
// Should render the command and output
541+
const codeBlocks = screen.getAllByTestId("code-block")
542+
expect(codeBlocks[0]).toHaveTextContent("wc -l *.go *.java")
543+
544+
// Should show pattern selector
545+
const selector = screen.getByTestId("command-pattern-selector")
546+
expect(selector).toBeInTheDocument()
547+
548+
// Should only extract "wc" from the actual command
549+
expect(screen.getByText("wc")).toBeInTheDocument()
550+
551+
// Should NOT extract numeric patterns from output like "45 total"
552+
expect(screen.queryByText("45")).not.toBeInTheDocument()
553+
expect(screen.queryByText("total")).not.toBeInTheDocument()
554+
expect(screen.queryByText("45 total")).not.toBeInTheDocument()
555+
})
556+
557+
it("should handle the edge case of 0 total in output", () => {
558+
// This is the exact case from the bug report
559+
const commandWithZeroTotal = `wc -l *.go *.java
560+
Output:
561+
0 total`
562+
563+
render(
564+
<ExtensionStateWrapper>
565+
<CommandExecution executionId="test-17" text={commandWithZeroTotal} />
566+
</ExtensionStateWrapper>,
567+
)
568+
569+
// Should show pattern selector
570+
const selector = screen.getByTestId("command-pattern-selector")
571+
expect(selector).toBeInTheDocument()
572+
573+
// Should only extract "wc" from the actual command
574+
// Check within the pattern selector specifically
575+
const patternTexts = Array.from(selector.querySelectorAll("span")).map((el) => el.textContent)
576+
577+
// Should have "wc" as a pattern
578+
expect(patternTexts).toContain("wc")
579+
580+
// Should NOT have "0", "total", or "0 total" as patterns
581+
expect(patternTexts).not.toContain("0")
582+
expect(patternTexts).not.toContain("total")
583+
expect(patternTexts).not.toContain("0 total")
584+
585+
// The output should still be displayed in the code block
586+
const codeBlocks = screen.getAllByTestId("code-block")
587+
expect(codeBlocks.length).toBeGreaterThan(1)
588+
expect(codeBlocks[1]).toHaveTextContent("0 total")
589+
})
522590
})
523591
})

webview-ui/src/utils/__tests__/commandPatterns.spec.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,18 @@ describe("extractCommandPatterns", () => {
101101
const patterns = extractCommandPatterns("npm run build && git push")
102102
expect(patterns).toEqual([...patterns].sort())
103103
})
104+
105+
it("should handle numeric input like '0 total'", () => {
106+
const patterns = extractCommandPatterns("0 total")
107+
// Should return empty array since "0" is not a valid command
108+
expect(patterns).toEqual([])
109+
})
110+
111+
it("should handle pure numeric commands", () => {
112+
const patterns = extractCommandPatterns("0")
113+
// Should return empty array since pure numbers are not valid commands
114+
expect(patterns).toEqual([])
115+
})
104116
})
105117

106118
describe("getPatternDescription", () => {
@@ -415,3 +427,75 @@ describe("security integration with extractCommandPatterns", () => {
415427
expect(patterns).not.toContain("date")
416428
})
417429
})
430+
431+
describe("integration: parseCommandAndOutput with extractCommandPatterns", () => {
432+
it("should not extract patterns from output text", () => {
433+
const text = `wc -l *.go *.java
434+
Output:
435+
wc: *.go: open: No such file or directory
436+
wc: *.java: open: No such file or directory
437+
0 total`
438+
const { command } = parseCommandAndOutput(text)
439+
const patterns = extractCommandPatterns(command)
440+
441+
// Should only extract patterns from the command, not the output
442+
expect(patterns).toContain("wc")
443+
expect(patterns).not.toContain("0")
444+
expect(patterns).not.toContain("total")
445+
expect(patterns).not.toContain("0 total")
446+
})
447+
448+
it("should handle the specific wc command case", () => {
449+
const text = `wc -l *.go *.java
450+
Output:
451+
25 hello_world.go
452+
316 HelloWorld.java
453+
341 total`
454+
const { command } = parseCommandAndOutput(text)
455+
const patterns = extractCommandPatterns(command)
456+
457+
// Should only extract "wc" from the command
458+
expect(patterns).toEqual(["wc"])
459+
expect(patterns).not.toContain("341")
460+
expect(patterns).not.toContain("total")
461+
expect(patterns).not.toContain("341 total")
462+
})
463+
464+
it("should handle wc command with error output", () => {
465+
const text = `wc -l *.go *.java
466+
Output:
467+
wc: *.go: open: No such file or directory
468+
wc: *.java: open: No such file or directory
469+
0 total`
470+
const { command, output } = parseCommandAndOutput(text)
471+
const patterns = extractCommandPatterns(command)
472+
473+
// Should only extract "wc" from the command
474+
expect(command).toBe("wc -l *.go *.java")
475+
expect(output).toContain("0 total")
476+
expect(patterns).toEqual(["wc"])
477+
expect(patterns).not.toContain("0")
478+
expect(patterns).not.toContain("total")
479+
expect(patterns).not.toContain("0 total")
480+
})
481+
482+
it("should handle case where only output line is provided", () => {
483+
// This simulates if somehow only "0 total" is passed as the text
484+
const text = "0 total"
485+
const { command } = parseCommandAndOutput(text)
486+
const patterns = extractCommandPatterns(command)
487+
488+
// In this case, the entire text is treated as command
489+
expect(command).toBe("0 total")
490+
// But "0 total" is not a valid command pattern (starts with number)
491+
expect(patterns).toEqual([])
492+
})
493+
494+
it("should handle commands without output separator", () => {
495+
const text = "npm install"
496+
const { command } = parseCommandAndOutput(text)
497+
const patterns = extractCommandPatterns(command)
498+
499+
expect(patterns).toEqual(["npm", "npm install"])
500+
})
501+
})

webview-ui/src/utils/commandPatterns.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,16 @@ export function extractCommandPatterns(command: string): string[] {
3939
} catch (_error) {
4040
// If parsing fails, try to extract at least the main command
4141
const mainCommand = command.trim().split(/\s+/)[0]
42-
if (mainCommand) patterns.add(mainCommand)
42+
43+
// Apply same validation as in processCommand
44+
if (
45+
mainCommand &&
46+
!/^\d+$/.test(mainCommand) && // Skip pure numbers
47+
!["total", "error", "warning", "failed", "success", "done"].includes(mainCommand.toLowerCase()) &&
48+
(/[a-zA-Z]/.test(mainCommand) || mainCommand.includes("/"))
49+
) {
50+
patterns.add(mainCommand)
51+
}
4352
}
4453

4554
return Array.from(patterns).sort()
@@ -49,7 +58,20 @@ function processCommand(cmd: any[], patterns: Set<string>) {
4958
if (!cmd.length || typeof cmd[0] !== "string") return
5059

5160
const mainCmd = cmd[0]
52-
patterns.add(mainCmd)
61+
62+
// Skip if it's just a number (like "0" from "0 total")
63+
if (/^\d+$/.test(mainCmd)) return
64+
65+
// Skip common output patterns that aren't commands
66+
const skipWords = ["total", "error", "warning", "failed", "success", "done"]
67+
if (skipWords.includes(mainCmd.toLowerCase())) return
68+
69+
// Only add if it contains at least one letter or is a valid path
70+
if (/[a-zA-Z]/.test(mainCmd) || mainCmd.includes("/")) {
71+
patterns.add(mainCmd)
72+
} else {
73+
return // Don't process further if main command is invalid
74+
}
5375

5476
// Patterns that indicate we should stop looking for subcommands
5577
const stopPatterns = [/^-/, /[\\/.~ ]/]

0 commit comments

Comments
 (0)