From c2082fd2a33910918cf1d5a270838b150c4acfea Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sun, 3 Aug 2025 05:37:06 +0000 Subject: [PATCH] fix: skip full command suggestion when it contains separators in CommandPatternSelector --- .../chat/CommandPatternSelector.tsx | 19 ++-- .../__tests__/CommandPatternSelector.spec.tsx | 87 +++++++++++++++++++ 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/webview-ui/src/components/chat/CommandPatternSelector.tsx b/webview-ui/src/components/chat/CommandPatternSelector.tsx index 87ccb1bab7..e1b8122df5 100644 --- a/webview-ui/src/components/chat/CommandPatternSelector.tsx +++ b/webview-ui/src/components/chat/CommandPatternSelector.tsx @@ -35,17 +35,26 @@ export const CommandPatternSelector: React.FC = ({ window.postMessage({ type: "action", action: "settingsButtonClicked", values: { section: "autoApprove" } }) } - // Create a combined list with full command first, then patterns + // Create a combined list with full command first (only if it doesn't contain separators), then patterns const allPatterns = useMemo(() => { // Trim the command to ensure consistency with extracted patterns const trimmedCommand = command.trim() - const fullCommandPattern: CommandPattern = { pattern: trimmedCommand } + + // Check if command contains separators (&&, ||, ;, |, &) + const containsSeparators = /(?:&&|\|\||;|\||&)/.test(trimmedCommand) // Create a set to track unique patterns we've already seen const seenPatterns = new Set() - seenPatterns.add(trimmedCommand) // Add the trimmed full command first - // Filter out any patterns that are duplicates or are the same as the full command + // Only add the full command if it doesn't contain separators + const patternsToShow: CommandPattern[] = [] + if (!containsSeparators) { + const fullCommandPattern: CommandPattern = { pattern: trimmedCommand } + patternsToShow.push(fullCommandPattern) + seenPatterns.add(trimmedCommand) + } + + // Filter out any patterns that are duplicates const uniquePatterns = patterns.filter((p) => { if (seenPatterns.has(p.pattern)) { return false @@ -54,7 +63,7 @@ export const CommandPatternSelector: React.FC = ({ return true }) - return [fullCommandPattern, ...uniquePatterns] + return [...patternsToShow, ...uniquePatterns] }, [command, patterns]) const getPatternStatus = (pattern: string): "allowed" | "denied" | "none" => { diff --git a/webview-ui/src/components/chat/__tests__/CommandPatternSelector.spec.tsx b/webview-ui/src/components/chat/__tests__/CommandPatternSelector.spec.tsx index 18c5ddd5aa..b652d738cb 100644 --- a/webview-ui/src/components/chat/__tests__/CommandPatternSelector.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/CommandPatternSelector.spec.tsx @@ -269,4 +269,91 @@ describe("CommandPatternSelector", () => { expect(screen.getByText("npm install express")).toBeInTheDocument() expect(screen.queryByDisplayValue("npm install react")).not.toBeInTheDocument() }) + + it("should not show full command as first pattern when it contains separators", () => { + const propsWithSeparators = { + ...defaultProps, + command: "npm install && npm test", + patterns: [ + { pattern: "npm install", description: "Install npm packages" }, + { pattern: "npm test", description: "Run tests" }, + ], + } + + render( + + + , + ) + + // Click to expand the component + const expandButton = screen.getByRole("button") + fireEvent.click(expandButton) + + // The full command with separators should NOT be shown + expect(screen.queryByText("npm install && npm test")).not.toBeInTheDocument() + + // But the individual patterns should be shown + expect(screen.getByText("npm install")).toBeInTheDocument() + expect(screen.getByText("npm test")).toBeInTheDocument() + }) + + it("should show full command when it does not contain separators", () => { + const propsWithoutSeparators = { + ...defaultProps, + command: "npm install express", + patterns: [ + { pattern: "npm install", description: "Install npm packages" }, + { pattern: "npm", description: "NPM command" }, + ], + } + + render( + + + , + ) + + // Click to expand the component + const expandButton = screen.getByRole("button") + fireEvent.click(expandButton) + + // The full command without separators should be shown + expect(screen.getByText("npm install express")).toBeInTheDocument() + + // And the patterns should also be shown + expect(screen.getByText("npm install")).toBeInTheDocument() + expect(screen.getByText("npm")).toBeInTheDocument() + }) + + it("should handle various separator types correctly", () => { + const testCases = [ + { command: "git add . && git commit", shouldShowFull: false }, + { command: "ls -la || echo 'failed'", shouldShowFull: false }, + { command: "cd /tmp; rm file.txt", shouldShowFull: false }, + { command: "ps aux | grep node", shouldShowFull: false }, + { command: "npm start &", shouldShowFull: false }, + { command: "git commit -m 'feat: add feature'", shouldShowFull: true }, + ] + + testCases.forEach(({ command, shouldShowFull }) => { + const { unmount } = render( + + + , + ) + + // Click to expand the component + const expandButton = screen.getByRole("button") + fireEvent.click(expandButton) + + if (shouldShowFull) { + expect(screen.getByText(command)).toBeInTheDocument() + } else { + expect(screen.queryByText(command)).not.toBeInTheDocument() + } + + unmount() + }) + }) })