From 4b5ede52782113b1f48d5a92cebf15cc3fa1146e Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 15 Aug 2025 16:07:41 +0000 Subject: [PATCH] feat: add multiline command converter to fix terminal hanging issue - Add multilineCommandConverter utility to convert multiline commands to single-line - Integrate converter into executeCommandTool with configurable setting - Add comprehensive tests for various shell constructs and edge cases - Handles POSIX shells (bash, zsh, fish) and PowerShell - Preserves functionality for Here Documents and other non-convertible patterns Fixes #7123 --- src/core/tools/executeCommandTool.ts | 27 ++ .../multilineCommandConverter.spec.ts | 333 ++++++++++++++++++ src/utils/multilineCommandConverter.ts | 98 ++++++ 3 files changed, 458 insertions(+) create mode 100644 src/utils/__tests__/multilineCommandConverter.spec.ts create mode 100644 src/utils/multilineCommandConverter.ts diff --git a/src/core/tools/executeCommandTool.ts b/src/core/tools/executeCommandTool.ts index c346526a2e..15f8e094ca 100644 --- a/src/core/tools/executeCommandTool.ts +++ b/src/core/tools/executeCommandTool.ts @@ -17,6 +17,7 @@ import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry" import { Terminal } from "../../integrations/terminal/Terminal" import { Package } from "../../shared/package" import { t } from "../../i18n" +import { convertMultilineToSingleLine, shouldConvertCommand } from "../../utils/multilineCommandConverter" class ShellIntegrationError extends Error {} @@ -54,12 +55,38 @@ export async function executeCommandTool( task.consecutiveMistakeCount = 0 command = unescapeHtmlEntities(command) // Unescape HTML entities. + + // Convert multiline commands to single line if enabled and applicable + const multilineConversionEnabled = vscode.workspace + .getConfiguration(Package.name) + .get("multilineCommandConversion", true) + + let originalCommand = command + let conversionApplied = false + + if (multilineConversionEnabled && shouldConvertCommand(command)) { + const conversionResult = convertMultilineToSingleLine(command) + if (conversionResult.success) { + command = conversionResult.command + conversionApplied = true + } else { + // Log conversion failure but continue with original command + console.log(`[executeCommandTool] Multiline conversion failed: ${conversionResult.reason}`) + } + } + + // Show the command that will be executed (converted or original) const didApprove = await askApproval("command", command) if (!didApprove) { return } + // If conversion was applied, log it (don't notify user to avoid disruption) + if (conversionApplied) { + console.log(`[executeCommandTool] Multiline command converted to single-line for execution`) + } + const executionId = task.lastMessageTs?.toString() ?? Date.now().toString() const provider = await task.providerRef.deref() const providerState = await provider?.getState() diff --git a/src/utils/__tests__/multilineCommandConverter.spec.ts b/src/utils/__tests__/multilineCommandConverter.spec.ts new file mode 100644 index 0000000000..7ea0d7b23a --- /dev/null +++ b/src/utils/__tests__/multilineCommandConverter.spec.ts @@ -0,0 +1,333 @@ +// npx vitest run src/utils/__tests__/multilineCommandConverter.spec.ts + +import { describe, it, expect } from "vitest" +import { convertMultilineToSingleLine, shouldConvertCommand } from "../multilineCommandConverter" + +describe("multilineCommandConverter", () => { + describe("shouldConvertCommand", () => { + it("should return false for single-line commands", () => { + expect(shouldConvertCommand('echo "hello world"')).toBe(false) + expect(shouldConvertCommand("ls -la")).toBe(false) + }) + + it("should return true for multiline commands", () => { + expect(shouldConvertCommand('echo "hello"\necho "world"')).toBe(true) + }) + + it("should return false for Here Documents", () => { + const hereDoc = `cat < { + describe("Basic functionality", () => { + it("should handle simple multiline commands", () => { + const input = `echo "hello" +echo "world"` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe('echo "hello" ; echo "world"') + }) + + it("should handle line continuations with backslash", () => { + const input = `echo "This is a very \\ +long command that \\ +spans multiple lines"` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe('echo "This is a very long command that spans multiple lines"') + }) + + it("should handle pipes at end of line", () => { + const input = `cat file.txt | +grep "pattern" | +sort` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + // Pipes don't need semicolons + expect(result.command).toBe('cat file.txt | grep "pattern" | sort') + }) + + it("should handle logical operators", () => { + const input = `command1 && +command2 || +command3` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + // Logical operators don't need semicolons + expect(result.command).toBe("command1 && command2 || command3") + }) + + it("should handle empty lines", () => { + const input = `echo "start" + +echo "middle" + +echo "end"` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe('echo "start" ; echo "middle" ; echo "end"') + }) + + it("should return original for single-line commands", () => { + const input = 'echo "hello world"' + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe(input) + }) + }) + + describe("Shell constructs", () => { + it("should handle if statements", () => { + const input = `if [ -f file.txt ] +then + echo "File exists" +else + echo "File not found" +fi` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + // Basic conversion joins with semicolons + expect(result.command).toContain("if [ -f file.txt ]") + expect(result.command).toContain("then") + expect(result.command).toContain('echo "File exists"') + expect(result.command).toContain("else") + expect(result.command).toContain('echo "File not found"') + expect(result.command).toContain("fi") + }) + + it("should handle for loops", () => { + const input = `for i in 1 2 3 +do + echo $i +done` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toContain("for i in 1 2 3") + expect(result.command).toContain("do") + expect(result.command).toContain("echo $i") + expect(result.command).toContain("done") + }) + + it("should handle while loops", () => { + const input = `while [ $count -lt 10 ] +do + echo $count + count=$((count + 1)) +done` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toContain("while [ $count -lt 10 ]") + expect(result.command).toContain("do") + expect(result.command).toContain("echo $count") + expect(result.command).toContain("count=$((count + 1))") + expect(result.command).toContain("done") + }) + + it("should handle functions", () => { + const input = `function myFunc() { + echo "Hello" + return 0 +}` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toContain("function myFunc() {") + expect(result.command).toContain('echo "Hello"') + expect(result.command).toContain("return 0") + expect(result.command).toContain("}") + }) + + it("should handle command grouping", () => { + const input = `( + cd /tmp + ls -la +)` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toContain("(") + expect(result.command).toContain("cd /tmp") + expect(result.command).toContain("ls -la") + expect(result.command).toContain(")") + }) + }) + + describe("PowerShell", () => { + it("should handle simple PowerShell commands", () => { + const input = `Write-Host "Hello" +Write-Host "World"` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe('Write-Host "Hello" ; Write-Host "World"') + }) + + it("should handle line continuations with backtick", () => { + const input = `Get-ChildItem \` + -Path "C:\\Users" \` + -Recurse` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe('Get-ChildItem -Path "C:\\Users" -Recurse') + }) + + it("should handle pipes in PowerShell", () => { + const input = `Get-Process | + Where-Object {$_.CPU -gt 10} | + Sort-Object CPU` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe("Get-Process | Where-Object {$_.CPU -gt 10} | Sort-Object CPU") + }) + + it("should handle PowerShell if statements", () => { + const input = `if ($true) { + Write-Host "True" +} else { + Write-Host "False" +}` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toContain("if ($true) {") + expect(result.command).toContain('Write-Host "True"') + expect(result.command).toContain("} else {") + expect(result.command).toContain('Write-Host "False"') + }) + + it("should handle PowerShell foreach loops", () => { + const input = `foreach ($item in $items) { + Write-Host $item +}` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toContain("foreach ($item in $items) {") + expect(result.command).toContain("Write-Host $item") + expect(result.command).toContain("}") + }) + }) + + describe("Here Documents", () => { + it("should not convert Here Documents", () => { + const input = `cat < { + const inputs = [ + `cat <<-EOF\nContent\nEOF`, + `cat <<"END"\nContent\nEND`, + `cat <<'MARKER'\nContent\nMARKER`, + `python < { + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(false) + expect(result.reason).toContain("Here Document") + }) + }) + }) + + describe("Edge cases", () => { + it("should handle commands with semicolons already present", () => { + const input = `echo "one"; +echo "two"; +echo "three"` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + // Should not add extra semicolons + expect(result.command).toBe('echo "one"; echo "two"; echo "three"') + }) + + it("should handle mixed line endings", () => { + const input = 'echo "one"\r\necho "two"\necho "three"' + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toContain('echo "one"') + expect(result.command).toContain('echo "two"') + expect(result.command).toContain('echo "three"') + }) + + it("should handle very long commands", () => { + const lines = [] + for (let i = 0; i < 100; i++) { + lines.push(`echo "Line ${i}"`) + } + const input = lines.join("\n") + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toContain('echo "Line 0"') + expect(result.command).toContain('echo "Line 99"') + expect(result.command.split(";").length).toBeGreaterThan(50) + }) + }) + + describe("Real-world examples", () => { + it("should handle a complex bash script", () => { + const input = `if [ -d "$DIR" ]; then + echo "Processing directory: $DIR" + for file in "$DIR"/*.txt; do + if [ -f "$file" ]; then + echo "Found: $(basename "$file")" + cat "$file" | + grep -E "pattern" | + sort > output.txt + fi + done +else + echo "Directory not found" +fi` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toContain('if [ -d "$DIR" ]') + expect(result.command).toContain('for file in "$DIR"/*.txt') + expect(result.command).toContain('grep -E "pattern"') + expect(result.command).toContain("sort > output.txt") + }) + + it("should handle a git command with multiple lines", () => { + const input = `git log --oneline \\ + --graph \\ + --decorate \\ + --all` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe("git log --oneline --graph --decorate --all") + }) + + it("should handle a docker command", () => { + const input = `docker run \\ + -d \\ + --name mycontainer \\ + -p 8080:80 \\ + -v /host/path:/container/path \\ + nginx:latest` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe( + "docker run -d --name mycontainer -p 8080:80 -v /host/path:/container/path nginx:latest", + ) + }) + + it("should handle a curl command", () => { + const input = `curl -X POST \\ + -H "Content-Type: application/json" \\ + -H "Authorization: Bearer token" \\ + -d '{"key": "value"}' \\ + https://api.example.com/endpoint` + const result = convertMultilineToSingleLine(input) + expect(result.success).toBe(true) + expect(result.command).toBe( + 'curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer token" -d \'{"key": "value"}\' https://api.example.com/endpoint', + ) + }) + }) + }) +}) diff --git a/src/utils/multilineCommandConverter.ts b/src/utils/multilineCommandConverter.ts new file mode 100644 index 0000000000..0fb7b5b3f0 --- /dev/null +++ b/src/utils/multilineCommandConverter.ts @@ -0,0 +1,98 @@ +/** + * Utility to convert multiline terminal commands into single-line equivalents + * to prevent terminal hanging issues when executing multiline commands. + * + * Supports POSIX shells (bash, zsh, fish) and PowerShell. + */ + +export interface ConversionResult { + success: boolean + command: string + reason?: string +} + +/** + * Detects if a command contains a Here Document which cannot be converted to single line + */ +function hasHereDocument(command: string): boolean { + // Check for Here Document patterns: < line.trim()) + .filter((line) => line) + + // Join lines with appropriate separators + const joined = lines.join(" ; ") + + // Clean up multiple semicolons and spaces + result = joined + .replace(/;\s*;/g, ";") + .replace(/\s+/g, " ") + .replace(/\{\s*;/g, "{") // Remove semicolon after opening brace + .replace(/\(\s*;/g, "(") // Remove semicolon after opening paren + .replace(/\|\s*;/g, "|") // Remove semicolon after pipe + .replace(/&&\s*;/g, "&&") // Remove semicolon after && + .replace(/\|\|\s*;/g, "||") // Remove semicolon after || + .trim() + + return { success: true, command: result } + } catch (error) { + // If conversion fails, return original command + return { + success: false, + command, + reason: `Conversion failed: ${error.message}`, + } + } +} + +/** + * Determines if a command should be converted based on configuration + */ +export function shouldConvertCommand(command: string): boolean { + // Always convert if command has multiple lines + if (!command.includes("\n")) { + return false + } + + // Check for patterns that should not be converted + if (hasHereDocument(command)) { + return false + } + + return true +}