fix: properly handle multi-line commands in quotes

- Fix parseCommand to preserve newlines within quoted strings instead of incorrectly splitting them
- Add UI filtering to exclude multi-line patterns from command pattern selector
- Multi-line git commit messages now work correctly for auto-approval validation
- UI shows clean patterns like 'git' and 'git commit' instead of unwieldy full commit text
- Add comprehensive tests for multi-line command handling
- Much simpler approach than placeholder substitution in PR #8792

Fixes the issue where multi-line git commit messages were being split into
separate commands and causing cluttered UI in the auto-approved commands list.
This commit is contained in:
daniel-lxs 2025-11-05 17:27:01 -05:00
parent 39448f4010
commit ad35b4fe42
No known key found for this signature in database
GPG key ID: 21C74479048B3AA6
4 changed files with 93 additions and 15 deletions

View file

@ -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)
}
})

View file

@ -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(
<ExtensionStateWrapper>
<CommandExecution executionId="test-18" text={multiLineGitCommand} />
</ExtensionStateWrapper>,
)
// 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")
})
})
})

View file

@ -110,15 +110,40 @@ 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
expect(parseCommand(commandWithNewlineInQuotes)).toEqual(['echo "Hello\nWorld"', "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
expect(parseCommand(multiLineCommit)).toEqual([
`git commit -m "feat: add new feature\n\n- Point A\n- Point B"`,
"git status",
])
})
it("handles mixed single and double quotes with newlines", () => {
const mixedQuotes = `echo 'single line'
echo "multi
line"
echo 'another single'`
expect(parseCommand(mixedQuotes)).toEqual([
"echo 'single line'",
'echo "multi\nline"',
"echo 'another single'",
])
})
it("handles quoted strings on single line", () => {

View file

@ -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)
}