fix: handle compound terminal commands by sequential execution

- Add commandParser module to detect and split compound commands
- Parse commands with &&, ||, ;, and | operators
- Execute compound command segments sequentially with proper operator logic
- Respect exit codes for && and || operators
- Add comprehensive tests for command parsing logic

Fixes #7430
This commit is contained in:
Roo Code 2025-08-26 20:58:36 +00:00
parent 934bfd0a54
commit 37dc97a5c3
3 changed files with 509 additions and 5 deletions

View file

@ -13,6 +13,7 @@ import { inspect } from "util"
import type { ExitCodeDetails } from "./types"
import { BaseTerminalProcess } from "./BaseTerminalProcess"
import { Terminal } from "./Terminal"
import { parseCommand, CommandSegment } from "./commandParser"
export class TerminalProcess extends BaseTerminalProcess {
private terminalRef: WeakRef<Terminal>
@ -72,6 +73,83 @@ export class TerminalProcess extends BaseTerminalProcess {
return
}
// Parse the command to check for compound operators
const parsedCommand = parseCommand(command)
if (parsedCommand.isCompound) {
console.info(`[TerminalProcess] Detected compound command with ${parsedCommand.segments.length} segments`)
await this.runCompoundCommand(parsedCommand.segments)
return
}
// Execute single command as before
await this.runSingleCommand(command)
}
/**
* Executes a compound command by running each segment sequentially
* based on the operator logic (&&, ||, ;, |)
*/
private async runCompoundCommand(segments: CommandSegment[]) {
let lastExitCode = 0
let accumulatedOutput = ""
let finalExitCode = 0
for (let i = 0; i < segments.length; i++) {
const segment = segments[i]
// Check if this segment should execute based on previous exit code
if (i > 0 && !segment.shouldExecute(lastExitCode)) {
console.info(`[TerminalProcess] Skipping segment "${segment.command}" due to operator logic`)
continue
}
console.info(
`[TerminalProcess] Executing compound segment ${i + 1}/${segments.length}: "${segment.command}"`,
)
// For pipe operators, we need special handling (future enhancement)
if (segment.operator === "|" && i < segments.length - 1) {
// For now, we'll execute pipes as a single command
// This is a limitation we can document
const pipeCommand = segments
.slice(i)
.map((s) => s.command)
.join(" | ")
await this.runSingleCommand(pipeCommand)
return
}
// Execute the segment
const segmentOutput = await this.runSingleCommand(segment.command, i === 0)
if (segmentOutput) {
if (accumulatedOutput && !accumulatedOutput.endsWith("\n")) {
accumulatedOutput += "\n"
}
accumulatedOutput += segmentOutput
}
// Get the exit code from the last execution
lastExitCode = this.lastExitCode ?? 0
finalExitCode = lastExitCode
}
// Emit the final accumulated output
this.fullOutput = accumulatedOutput
this.emit("completed", this.removeEscapeSequences(accumulatedOutput))
this.emit("shell_execution_complete", { exitCode: finalExitCode })
this.emit("continue")
}
private lastExitCode?: number
/**
* Executes a single command (extracted from the original run method)
*/
private async runSingleCommand(command: string, emitStarted: boolean = true): Promise<string> {
const terminal = this.terminal.terminal
// Create a promise that resolves when the stream becomes available
const streamAvailable = new Promise<AsyncIterable<string>>((resolve, reject) => {
const timeoutId = setTimeout(() => {
@ -127,9 +205,9 @@ export class TerminalProcess extends BaseTerminalProcess {
commandToExecute += ` ; start-sleep -milliseconds ${Terminal.getCommandDelay()}`
}
terminal.shellIntegration.executeCommand(commandToExecute)
terminal.shellIntegration!.executeCommand(commandToExecute)
} else {
terminal.shellIntegration.executeCommand(command)
terminal.shellIntegration!.executeCommand(command)
}
this.isHot = true
@ -153,7 +231,7 @@ export class TerminalProcess extends BaseTerminalProcess {
// Emit continue event to allow execution to proceed
this.emit("continue")
return
return ""
}
let preOutput = ""
@ -209,7 +287,8 @@ export class TerminalProcess extends BaseTerminalProcess {
this.terminal.setActiveStream(undefined)
// Wait for shell execution to complete.
await shellExecutionComplete
const exitDetails = await shellExecutionComplete
this.lastExitCode = exitDetails.exitCode
this.isHot = false
@ -237,7 +316,7 @@ export class TerminalProcess extends BaseTerminalProcess {
this.continue()
// Return early since we can't process output without shell integration markers
return
return ""
}
// fullOutput begins after C marker so we only need to trim off D marker
@ -253,8 +332,16 @@ export class TerminalProcess extends BaseTerminalProcess {
// command is finished, we still want to consider it 'hot' in case
// so that api request stalls to let diagnostics catch up").
this.stopHotTimer()
// For compound commands, we handle completion differently
if (!emitStarted) {
// Return output without emitting completion (handled by runCompoundCommand)
return this.removeEscapeSequences(this.fullOutput)
}
this.emit("completed", this.removeEscapeSequences(this.fullOutput))
this.emit("continue")
return this.removeEscapeSequences(this.fullOutput)
}
public override continue() {

View file

@ -0,0 +1,230 @@
import { describe, it, expect } from "vitest"
import { parseCommand, isCompoundCommand, splitCompoundCommand } from "../commandParser"
describe("commandParser", () => {
describe("parseCommand", () => {
it("should identify simple commands as non-compound", () => {
const result = parseCommand("ls -la")
expect(result.isCompound).toBe(false)
expect(result.segments).toHaveLength(1)
expect(result.segments[0].command).toBe("ls -la")
expect(result.segments[0].operator).toBeUndefined()
})
it("should parse && operator correctly", () => {
const result = parseCommand("cd foo && npm test")
expect(result.isCompound).toBe(true)
expect(result.segments).toHaveLength(2)
expect(result.segments[0].command).toBe("cd foo")
expect(result.segments[0].operator).toBe("&&")
expect(result.segments[1].command).toBe("npm test")
expect(result.segments[1].operator).toBeUndefined()
})
it("should parse || operator correctly", () => {
const result = parseCommand("npm test || echo 'Tests failed'")
expect(result.isCompound).toBe(true)
expect(result.segments).toHaveLength(2)
expect(result.segments[0].command).toBe("npm test")
expect(result.segments[0].operator).toBe("||")
expect(result.segments[1].command).toBe("echo 'Tests failed'")
})
it("should parse semicolon operator correctly", () => {
const result = parseCommand("echo 'Starting'; npm test; echo 'Done'")
expect(result.isCompound).toBe(true)
expect(result.segments).toHaveLength(3)
expect(result.segments[0].command).toBe("echo 'Starting'")
expect(result.segments[0].operator).toBe(";")
expect(result.segments[1].command).toBe("npm test")
expect(result.segments[1].operator).toBe(";")
expect(result.segments[2].command).toBe("echo 'Done'")
expect(result.segments[2].operator).toBeUndefined()
})
it("should parse pipe operator correctly", () => {
const result = parseCommand("ls -la | grep test")
expect(result.isCompound).toBe(true)
expect(result.segments).toHaveLength(2)
expect(result.segments[0].command).toBe("ls -la")
expect(result.segments[0].operator).toBe("|")
expect(result.segments[1].command).toBe("grep test")
})
it("should handle mixed operators", () => {
const result = parseCommand("cd app && npm install || echo 'Install failed'; npm test")
expect(result.isCompound).toBe(true)
expect(result.segments).toHaveLength(4)
expect(result.segments[0].operator).toBe("&&")
expect(result.segments[1].operator).toBe("||")
expect(result.segments[2].operator).toBe(";")
expect(result.segments[3].operator).toBeUndefined()
})
it("should respect single quotes", () => {
const result = parseCommand("echo 'test && test'")
expect(result.isCompound).toBe(false)
expect(result.segments).toHaveLength(1)
expect(result.segments[0].command).toBe("echo 'test && test'")
})
it("should respect double quotes", () => {
const result = parseCommand('echo "test || test"')
expect(result.isCompound).toBe(false)
expect(result.segments).toHaveLength(1)
expect(result.segments[0].command).toBe('echo "test || test"')
})
it("should handle escaped characters", () => {
const result = parseCommand("echo test \\&& echo test2")
expect(result.isCompound).toBe(false)
expect(result.segments).toHaveLength(1)
expect(result.segments[0].command).toBe("echo test \\&& echo test2")
})
it("should handle complex quoted strings", () => {
const result = parseCommand(`echo "It's a test" && echo 'He said "hello"'`)
expect(result.isCompound).toBe(true)
expect(result.segments).toHaveLength(2)
expect(result.segments[0].command).toBe(`echo "It's a test"`)
expect(result.segments[1].command).toBe(`echo 'He said "hello"'`)
})
it("should handle empty segments gracefully", () => {
const result = parseCommand("&& npm test")
expect(result.segments).toHaveLength(1)
expect(result.segments[0].command).toBe("npm test")
})
it("should handle trailing operators", () => {
const result = parseCommand("npm test &&")
expect(result.segments).toHaveLength(1)
expect(result.segments[0].command).toBe("npm test")
expect(result.segments[0].operator).toBe("&&")
})
})
describe("isCompoundCommand", () => {
it("should return false for simple commands", () => {
expect(isCompoundCommand("ls -la")).toBe(false)
expect(isCompoundCommand("npm test")).toBe(false)
expect(isCompoundCommand("echo 'test && test'")).toBe(false)
})
it("should return true for compound commands", () => {
expect(isCompoundCommand("cd foo && npm test")).toBe(true)
expect(isCompoundCommand("npm test || echo failed")).toBe(true)
expect(isCompoundCommand("echo start; npm test")).toBe(true)
expect(isCompoundCommand("ls | grep test")).toBe(true)
})
})
describe("splitCompoundCommand", () => {
it("should return single segment for simple commands", () => {
const segments = splitCompoundCommand("npm test")
expect(segments).toHaveLength(1)
expect(segments[0].command).toBe("npm test")
})
it("should split compound commands correctly", () => {
const segments = splitCompoundCommand("cd app && npm install && npm test")
expect(segments).toHaveLength(3)
expect(segments[0].command).toBe("cd app")
expect(segments[1].command).toBe("npm install")
expect(segments[2].command).toBe("npm test")
})
})
describe("CommandSegment.shouldExecute", () => {
it("should handle && operator logic correctly", () => {
const result = parseCommand("cmd1 && cmd2")
const segment2 = result.segments[1]
// Should execute if previous command succeeded (exit code 0)
expect(segment2.shouldExecute(0)).toBe(true)
// Should not execute if previous command failed
expect(segment2.shouldExecute(1)).toBe(false)
expect(segment2.shouldExecute(127)).toBe(false)
})
it("should handle || operator logic correctly", () => {
const result = parseCommand("cmd1 || cmd2")
const segment2 = result.segments[1]
// Should not execute if previous command succeeded
expect(segment2.shouldExecute(0)).toBe(false)
// Should execute if previous command failed
expect(segment2.shouldExecute(1)).toBe(true)
expect(segment2.shouldExecute(127)).toBe(true)
})
it("should handle ; operator logic correctly", () => {
const result = parseCommand("cmd1; cmd2")
const segment2 = result.segments[1]
// Should always execute regardless of previous exit code
expect(segment2.shouldExecute(0)).toBe(true)
expect(segment2.shouldExecute(1)).toBe(true)
expect(segment2.shouldExecute(127)).toBe(true)
})
it("should handle | operator logic correctly", () => {
const result = parseCommand("cmd1 | cmd2")
const segment2 = result.segments[1]
// Should always execute (pipes always run)
expect(segment2.shouldExecute(0)).toBe(true)
expect(segment2.shouldExecute(1)).toBe(true)
})
it("should handle first segment correctly", () => {
const result = parseCommand("cmd1 && cmd2 || cmd3")
const segment1 = result.segments[0]
// First segment should always execute
expect(segment1.shouldExecute(0)).toBe(true)
expect(segment1.shouldExecute(1)).toBe(true)
})
})
describe("edge cases", () => {
it("should handle multiple spaces between commands and operators", () => {
const result = parseCommand("cmd1 && cmd2")
expect(result.segments).toHaveLength(2)
expect(result.segments[0].command).toBe("cmd1")
expect(result.segments[1].command).toBe("cmd2")
})
it("should handle tabs and other whitespace", () => {
const result = parseCommand("cmd1\t&&\tcmd2")
expect(result.segments).toHaveLength(2)
expect(result.segments[0].command).toBe("cmd1")
expect(result.segments[1].command).toBe("cmd2")
})
it("should handle complex real-world commands", () => {
const cmd = "git add . && git commit -m 'feat: add feature' && git push origin main || echo 'Push failed'"
const result = parseCommand(cmd)
expect(result.isCompound).toBe(true)
expect(result.segments).toHaveLength(4)
expect(result.segments[0].command).toBe("git add .")
expect(result.segments[1].command).toBe("git commit -m 'feat: add feature'")
expect(result.segments[2].command).toBe("git push origin main")
expect(result.segments[3].command).toBe("echo 'Push failed'")
})
it("should handle commands with redirections", () => {
const result = parseCommand("echo test > file.txt && cat file.txt")
expect(result.segments).toHaveLength(2)
expect(result.segments[0].command).toBe("echo test > file.txt")
expect(result.segments[1].command).toBe("cat file.txt")
})
it("should handle commands with environment variables", () => {
const result = parseCommand("NODE_ENV=test npm test && echo $NODE_ENV")
expect(result.segments).toHaveLength(2)
expect(result.segments[0].command).toBe("NODE_ENV=test npm test")
expect(result.segments[1].command).toBe("echo $NODE_ENV")
})
})
})

View file

@ -0,0 +1,187 @@
/**
* Utility module for parsing and handling compound shell commands.
* Detects and splits commands with operators like &&, ||, ;, and |
* to enable sequential execution and proper process tracking.
*/
export interface ParsedCommand {
/** The original full command string */
original: string
/** Whether this is a compound command with operators */
isCompound: boolean
/** Individual command segments if compound, otherwise array with single command */
segments: CommandSegment[]
}
export interface CommandSegment {
/** The command text */
command: string
/** The operator that follows this command (if any) */
operator?: "&&" | "||" | ";" | "|"
/** Whether execution should continue based on previous exit code */
shouldExecute: (previousExitCode: number) => boolean
}
/**
* Parses a command string to detect compound command operators.
* Handles &&, ||, ;, and | operators while respecting quotes and escapes.
*
* @param command The command string to parse
* @returns Parsed command information
*/
export function parseCommand(command: string): ParsedCommand {
const segments: CommandSegment[] = []
let current = ""
let inSingleQuote = false
let inDoubleQuote = false
let escaped = false
let i = 0
while (i < command.length) {
const char = command[i]
const nextChar = command[i + 1]
// Handle escape sequences
if (escaped) {
current += char
escaped = false
i++
continue
}
if (char === "\\" && !inSingleQuote) {
escaped = true
current += char
i++
continue
}
// Handle quotes
if (char === "'" && !inDoubleQuote) {
inSingleQuote = !inSingleQuote
current += char
i++
continue
}
if (char === '"' && !inSingleQuote) {
inDoubleQuote = !inDoubleQuote
current += char
i++
continue
}
// If we're inside quotes, just add the character
if (inSingleQuote || inDoubleQuote) {
current += char
i++
continue
}
// Check for operators (only outside quotes)
let operator: CommandSegment["operator"] | undefined
let operatorLength = 0
if (char === "&" && nextChar === "&") {
operator = "&&"
operatorLength = 2
} else if (char === "|" && nextChar === "|") {
operator = "||"
operatorLength = 2
} else if (char === ";") {
operator = ";"
operatorLength = 1
} else if (char === "|" && nextChar !== "|") {
operator = "|"
operatorLength = 1
}
if (operator) {
// Found an operator, save current segment
const trimmedCommand = current.trim()
if (trimmedCommand) {
const segment = createSegment(trimmedCommand, operator)
segments.push(segment)
}
current = ""
i += operatorLength
} else {
current += char
i++
}
}
// Add the last segment
const trimmedCommand = current.trim()
if (trimmedCommand) {
segments.push(createSegment(trimmedCommand, undefined))
}
// Now set the shouldExecute logic based on the PREVIOUS segment's operator
for (let i = 1; i < segments.length; i++) {
const prevOperator = segments[i - 1].operator
switch (prevOperator) {
case "&&":
// Execute only if previous command succeeded (exit code 0)
segments[i].shouldExecute = (exitCode) => exitCode === 0
break
case "||":
// Execute only if previous command failed (non-zero exit code)
segments[i].shouldExecute = (exitCode) => exitCode !== 0
break
case ";":
case "|":
default:
// Always execute regardless of previous exit code
segments[i].shouldExecute = () => true
break
}
}
// If we only have one segment with no operator, it's not compound
const isCompound = segments.length > 1 || (segments.length === 1 && segments[0].operator !== undefined)
return {
original: command,
isCompound,
segments,
}
}
/**
* Creates a command segment with appropriate execution logic based on operator
*/
function createSegment(command: string, operator: CommandSegment["operator"]): CommandSegment {
// The shouldExecute function determines if THIS segment should execute
// based on the PREVIOUS segment's exit code and the PREVIOUS segment's operator
// By default, segments always execute (first segment or after semicolon)
let shouldExecute: (previousExitCode: number) => boolean = () => true
return {
command,
operator,
shouldExecute,
}
}
/**
* Checks if a command contains compound operators that would spawn multiple processes
*
* @param command The command to check
* @returns True if the command contains compound operators
*/
export function isCompoundCommand(command: string): boolean {
return parseCommand(command).isCompound
}
/**
* Splits a compound command into individual commands for sequential execution.
* Preserves the execution logic of operators like && and ||.
*
* @param command The compound command to split
* @returns Array of individual commands with execution conditions
*/
export function splitCompoundCommand(command: string): CommandSegment[] {
return parseCommand(command).segments
}