diff --git a/webview-ui/src/components/chat/CommandExecution.tsx b/webview-ui/src/components/chat/CommandExecution.tsx index ca51a9d26e4c..4b7210755a62 100644 --- a/webview-ui/src/components/chat/CommandExecution.tsx +++ b/webview-ui/src/components/chat/CommandExecution.tsx @@ -63,8 +63,10 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec // Add all individual commands first allCommands.forEach((cmd) => { - if (cmd.trim()) { - allPatterns.add(cmd.trim()) + const trimmed = cmd.trim() + // Skip patterns containing newlines - these are multi-line and not useful for UI patterns + if (trimmed && !trimmed.includes("\n") && !trimmed.includes("\r")) { + allPatterns.add(trimmed) } }) diff --git a/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx b/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx index 5afbdd93d4f7..d754409bfa62 100644 --- a/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx @@ -564,5 +564,37 @@ Output: expect(codeBlocks.length).toBeGreaterThan(1) expect(codeBlocks[1]).toHaveTextContent("0 total") }) + + it("should filter out multi-line patterns from command pattern selector", () => { + // Test with a multi-line git commit command similar to the PR issue + const multiLineGitCommand = `git commit -m "feat: title + +- point a +- point b"` + + render( + + + , + ) + + // Should show pattern selector + const selector = screen.getByTestId("command-pattern-selector") + expect(selector).toBeInTheDocument() + + // Should show useful single-line patterns like "git" and "git commit" + expect(selector.textContent).toMatch(/git/) + + // Should NOT show the full collapsed multi-line content (which would be very long) + // The pattern selector should contain short, useful patterns only + const patterns = selector.textContent || "" + // Check that no pattern is suspiciously long (> 50 chars would indicate collapsed multi-line content) + const longPatternMatch = patterns.match(/\b\S{51,}\b/) + expect(longPatternMatch).toBeNull() + + // But the original command should still be shown in the code block + const codeBlock = screen.getByTestId("code-block") + expect(codeBlock.textContent).toContain("feat: title") + }) }) }) diff --git a/webview-ui/src/utils/__tests__/command-validation.spec.ts b/webview-ui/src/utils/__tests__/command-validation.spec.ts index e87bae07e51f..af12ccb1612c 100644 --- a/webview-ui/src/utils/__tests__/command-validation.spec.ts +++ b/webview-ui/src/utils/__tests__/command-validation.spec.ts @@ -110,15 +110,46 @@ describe("Command Validation", () => { ]) }) - it("splits on actual newlines even within quotes", () => { - // Note: Since we split by newlines first, actual newlines in the input - // will split the command, even if they appear to be within quotes + it("preserves newlines within quoted strings", () => { + // Newlines inside quoted strings should be preserved as part of the command // Using template literal to create actual newline const commandWithNewlineInQuotes = `echo "Hello -World" -git status` - // The quotes get stripped because they're no longer properly paired after splitting - expect(parseCommand(commandWithNewlineInQuotes)).toEqual(["echo Hello", "World", "git status"]) + World" + git status` + // The newlines inside quotes are preserved, so we get two commands + // Template literals preserve actual whitespace, not escaped \n + const parsed = parseCommand(commandWithNewlineInQuotes) + expect(parsed).toHaveLength(2) + expect(parsed[0]).toMatch(/echo "Hello[\s\S]*World"/) + expect(parsed[1]).toBe("git status") + }) + + it("handles multi-line git commit messages correctly", () => { + // Real-world case: multi-line git commit messages in quotes + const multiLineCommit = `git commit -m "feat: add new feature + + - Point A + - Point B" + git status` + // The multi-line commit message should be preserved as one command + // Template literals preserve actual whitespace, not escaped \n + const parsed = parseCommand(multiLineCommit) + expect(parsed).toHaveLength(2) + expect(parsed[0]).toMatch(/git commit -m "feat: add new feature[\s\S]*- Point A[\s\S]*- Point B"/) + expect(parsed[1]).toBe("git status") + }) + + it("handles mixed single and double quotes with newlines", () => { + const mixedQuotes = `echo 'single line' + echo "multi + line" + echo 'another single'` + // Template literals and shell parsing may strip quotes, but preserve multi-line content + const parsed = parseCommand(mixedQuotes) + expect(parsed).toHaveLength(3) + expect(parsed[0]).toMatch(/echo.*single line/) + expect(parsed[1]).toMatch(/echo.*multi[\s\S]*line/) + expect(parsed[2]).toMatch(/echo.*another single/) }) it("handles quoted strings on single line", () => { diff --git a/webview-ui/src/utils/command-validation.ts b/webview-ui/src/utils/command-validation.ts index 572ca32bad64..be68b4e98d9d 100644 --- a/webview-ui/src/utils/command-validation.ts +++ b/webview-ui/src/utils/command-validation.ts @@ -127,26 +127,45 @@ export function containsDangerousSubstitution(source: string): boolean { * chaining operators (&&, ||, ;, |, or &) and newlines. * * Uses shell-quote to properly handle: - * - Quoted strings (preserves quotes) + * - Quoted strings (preserves quotes and newlines within quotes) * - Subshell commands ($(cmd), `cmd`, <(cmd), >(cmd)) * - PowerShell redirections (2>&1) * - Chain operators (&&, ||, ;, |, &) - * - Newlines as command separators + * - Newlines as command separators (but not within quotes) */ export function parseCommand(command: string): string[] { if (!command?.trim()) return [] - // Split by newlines first (handle different line ending formats) - // This regex splits on \r\n (Windows), \n (Unix), or \r (old Mac) - const lines = command.split(/\r\n|\r|\n/) + // First protect quoted strings to avoid splitting on newlines inside quotes + const quotes: string[] = [] + let protectedCommand = command + + // Protect double-quoted strings (including multi-line) - simpler regex + protectedCommand = protectedCommand.replace(/"[^"]*"/gs, (match) => { + quotes.push(match) + return `__QUOTE_${quotes.length - 1}__` + }) + + // Protect single-quoted strings (including multi-line) - simpler regex + protectedCommand = protectedCommand.replace(/'[^']*'/gs, (match) => { + quotes.push(match) + return `__QUOTE_${quotes.length - 1}__` + }) + + // Now split by newlines (only unquoted newlines will be split) + const lines = protectedCommand.split(/\r\n|\r|\n/) const allCommands: string[] = [] for (const line of lines) { // Skip empty lines if (!line.trim()) continue + // Restore quotes in this line before processing + let restoredLine = line + restoredLine = restoredLine.replace(/__QUOTE_(\d+)__/g, (_, i) => quotes[parseInt(i)]) + // Process each line through the existing parsing logic - const lineCommands = parseCommandLine(line) + const lineCommands = parseCommandLine(restoredLine) allCommands.push(...lineCommands) }