Skip to content

Commit c2082fd

Browse files
committed
fix: skip full command suggestion when it contains separators in CommandPatternSelector
1 parent 3f966df commit c2082fd

File tree

2 files changed

+101
-5
lines changed

2 files changed

+101
-5
lines changed

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,26 @@ export const CommandPatternSelector: React.FC<CommandPatternSelectorProps> = ({
3535
window.postMessage({ type: "action", action: "settingsButtonClicked", values: { section: "autoApprove" } })
3636
}
3737

38-
// Create a combined list with full command first, then patterns
38+
// Create a combined list with full command first (only if it doesn't contain separators), then patterns
3939
const allPatterns = useMemo(() => {
4040
// Trim the command to ensure consistency with extracted patterns
4141
const trimmedCommand = command.trim()
42-
const fullCommandPattern: CommandPattern = { pattern: trimmedCommand }
42+
43+
// Check if command contains separators (&&, ||, ;, |, &)
44+
const containsSeparators = /(?:&&|\|\||;|\||&)/.test(trimmedCommand)
4345

4446
// Create a set to track unique patterns we've already seen
4547
const seenPatterns = new Set<string>()
46-
seenPatterns.add(trimmedCommand) // Add the trimmed full command first
4748

48-
// Filter out any patterns that are duplicates or are the same as the full command
49+
// Only add the full command if it doesn't contain separators
50+
const patternsToShow: CommandPattern[] = []
51+
if (!containsSeparators) {
52+
const fullCommandPattern: CommandPattern = { pattern: trimmedCommand }
53+
patternsToShow.push(fullCommandPattern)
54+
seenPatterns.add(trimmedCommand)
55+
}
56+
57+
// Filter out any patterns that are duplicates
4958
const uniquePatterns = patterns.filter((p) => {
5059
if (seenPatterns.has(p.pattern)) {
5160
return false
@@ -54,7 +63,7 @@ export const CommandPatternSelector: React.FC<CommandPatternSelectorProps> = ({
5463
return true
5564
})
5665

57-
return [fullCommandPattern, ...uniquePatterns]
66+
return [...patternsToShow, ...uniquePatterns]
5867
}, [command, patterns])
5968

6069
const getPatternStatus = (pattern: string): "allowed" | "denied" | "none" => {

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

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,4 +269,91 @@ describe("CommandPatternSelector", () => {
269269
expect(screen.getByText("npm install express")).toBeInTheDocument()
270270
expect(screen.queryByDisplayValue("npm install react")).not.toBeInTheDocument()
271271
})
272+
273+
it("should not show full command as first pattern when it contains separators", () => {
274+
const propsWithSeparators = {
275+
...defaultProps,
276+
command: "npm install && npm test",
277+
patterns: [
278+
{ pattern: "npm install", description: "Install npm packages" },
279+
{ pattern: "npm test", description: "Run tests" },
280+
],
281+
}
282+
283+
render(
284+
<TestWrapper>
285+
<CommandPatternSelector {...propsWithSeparators} />
286+
</TestWrapper>,
287+
)
288+
289+
// Click to expand the component
290+
const expandButton = screen.getByRole("button")
291+
fireEvent.click(expandButton)
292+
293+
// The full command with separators should NOT be shown
294+
expect(screen.queryByText("npm install && npm test")).not.toBeInTheDocument()
295+
296+
// But the individual patterns should be shown
297+
expect(screen.getByText("npm install")).toBeInTheDocument()
298+
expect(screen.getByText("npm test")).toBeInTheDocument()
299+
})
300+
301+
it("should show full command when it does not contain separators", () => {
302+
const propsWithoutSeparators = {
303+
...defaultProps,
304+
command: "npm install express",
305+
patterns: [
306+
{ pattern: "npm install", description: "Install npm packages" },
307+
{ pattern: "npm", description: "NPM command" },
308+
],
309+
}
310+
311+
render(
312+
<TestWrapper>
313+
<CommandPatternSelector {...propsWithoutSeparators} />
314+
</TestWrapper>,
315+
)
316+
317+
// Click to expand the component
318+
const expandButton = screen.getByRole("button")
319+
fireEvent.click(expandButton)
320+
321+
// The full command without separators should be shown
322+
expect(screen.getByText("npm install express")).toBeInTheDocument()
323+
324+
// And the patterns should also be shown
325+
expect(screen.getByText("npm install")).toBeInTheDocument()
326+
expect(screen.getByText("npm")).toBeInTheDocument()
327+
})
328+
329+
it("should handle various separator types correctly", () => {
330+
const testCases = [
331+
{ command: "git add . && git commit", shouldShowFull: false },
332+
{ command: "ls -la || echo 'failed'", shouldShowFull: false },
333+
{ command: "cd /tmp; rm file.txt", shouldShowFull: false },
334+
{ command: "ps aux | grep node", shouldShowFull: false },
335+
{ command: "npm start &", shouldShowFull: false },
336+
{ command: "git commit -m 'feat: add feature'", shouldShowFull: true },
337+
]
338+
339+
testCases.forEach(({ command, shouldShowFull }) => {
340+
const { unmount } = render(
341+
<TestWrapper>
342+
<CommandPatternSelector {...defaultProps} command={command} patterns={[]} />
343+
</TestWrapper>,
344+
)
345+
346+
// Click to expand the component
347+
const expandButton = screen.getByRole("button")
348+
fireEvent.click(expandButton)
349+
350+
if (shouldShowFull) {
351+
expect(screen.getByText(command)).toBeInTheDocument()
352+
} else {
353+
expect(screen.queryByText(command)).not.toBeInTheDocument()
354+
}
355+
356+
unmount()
357+
})
358+
})
272359
})

0 commit comments

Comments
 (0)