refactor: simplify compound command handling to preserve shell context

- Execute compound commands as single shell commands instead of splitting
- Preserves shell context between command segments (e.g., cd affects subsequent commands)
- Deprecate the segment-splitting approach in favor of simpler solution
- Add comprehensive tests for compound command behavior

This approach is simpler and more reliable as it lets the shell naturally
handle operator logic and state preservation.
This commit is contained in:
Roo Code 2025-08-26 21:06:22 +00:00
parent 37dc97a5c3
commit 9a8634e1ce
2 changed files with 127 additions and 61 deletions

View file

@ -78,7 +78,10 @@ export class TerminalProcess extends BaseTerminalProcess {
if (parsedCommand.isCompound) {
console.info(`[TerminalProcess] Detected compound command with ${parsedCommand.segments.length} segments`)
await this.runCompoundCommand(parsedCommand.segments)
console.info(`[TerminalProcess] Executing compound command as a single shell command to preserve context`)
// Execute compound commands as a single command to preserve shell context
// This ensures operators like && and || work correctly and state is maintained
await this.runSingleCommand(command)
return
}
@ -87,67 +90,27 @@ export class TerminalProcess extends BaseTerminalProcess {
}
/**
* Executes a compound command by running each segment sequentially
* based on the operator logic (&&, ||, ;, |)
* @deprecated This method is no longer used. Compound commands are now executed
* as a single shell command to preserve context and proper operator behavior.
* Keeping for reference only.
*
* Previously attempted to execute compound commands by splitting them into segments,
* but this approach lost shell context between commands (e.g., cd wouldn't affect
* subsequent commands).
*/
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 async runCompoundCommand_DEPRECATED(segments: CommandSegment[]) {
// This method is intentionally left empty and deprecated
// Compound commands are now handled by executing them as a single command
throw new Error("runCompoundCommand is deprecated. Compound commands should be executed as a single command.")
}
private lastExitCode?: number
/**
* Executes a single command (extracted from the original run method)
* Executes a command through VSCode's shell integration.
* Handles both simple and compound commands.
*/
private async runSingleCommand(command: string, emitStarted: boolean = true): Promise<string> {
private async runSingleCommand(command: string): Promise<string> {
const terminal = this.terminal.terminal
// Create a promise that resolves when the stream becomes available
@ -333,12 +296,6 @@ export class TerminalProcess extends BaseTerminalProcess {
// 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)

View file

@ -6,6 +6,7 @@ import { mergePromise } from "../mergePromise"
import { TerminalProcess } from "../TerminalProcess"
import { Terminal } from "../Terminal"
import { TerminalRegistry } from "../TerminalRegistry"
import { parseCommand } from "../commandParser"
vi.mock("execa", () => ({
execa: vi.fn(),
@ -165,6 +166,114 @@ describe("TerminalProcess", () => {
await completePromise
expect(terminalProcess.isHot).toBe(false)
})
it("handles compound commands as single command", async () => {
// Test that compound commands are executed as a single command
// to preserve shell context between segments
const compoundCommand = "cd foo && npm test"
let executedCommand = ""
mockStream = (async function* () {
yield "\x1b]633;C\x07"
yield "Changed directory to foo\n"
yield "Running tests...\n"
yield "Tests passed"
yield "\x1b]633;D\x07"
terminalProcess.emit("shell_execution_complete", { exitCode: 0 })
})()
mockTerminal.shellIntegration.executeCommand.mockImplementation((cmd: string) => {
executedCommand = cmd
return {
read: vi.fn().mockReturnValue(mockStream),
}
})
const runPromise = terminalProcess.run(compoundCommand)
// Emit stream available with the mock stream
terminalProcess.emit("stream_available", mockStream)
await runPromise
// Verify the compound command was executed as a single command
expect(executedCommand).toBe(compoundCommand)
expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledTimes(1)
expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledWith(compoundCommand)
})
it("detects compound commands correctly", async () => {
// Test various compound command patterns
const testCases = [
{ command: "cd foo && npm test", isCompound: true },
{ command: "npm test || echo 'Tests failed'", isCompound: true },
{ command: "echo start; npm test; echo done", isCompound: true },
{ command: "ls -la | grep test", isCompound: true },
{ command: "npm test", isCompound: false },
{ command: "echo 'test && test'", isCompound: false }, // Quoted operators shouldn't be detected
]
for (const testCase of testCases) {
const parsed = parseCommand(testCase.command)
expect(parsed.isCompound).toBe(testCase.isCompound)
}
})
it("preserves shell context in compound commands", async () => {
// This test verifies that compound commands maintain context
// For example, cd in the first part affects the second part
const command = "cd /tmp && pwd"
mockStream = (async function* () {
yield "\x1b]633;C\x07"
yield "/tmp\n" // pwd should output /tmp since cd was in same shell context
yield "\x1b]633;D\x07"
terminalProcess.emit("shell_execution_complete", { exitCode: 0 })
})()
mockTerminal.shellIntegration.executeCommand.mockReturnValue({
read: vi.fn().mockReturnValue(mockStream),
})
let output = ""
terminalProcess.on("completed", (result) => {
output = result || ""
})
const runPromise = terminalProcess.run(command)
terminalProcess.emit("stream_available", mockStream)
await runPromise
expect(output.trim()).toBe("/tmp")
// Verify it was executed as a single command
expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledWith(command)
})
it("handles complex compound commands with multiple operators", async () => {
const complexCommand = "git add . && git commit -m 'test' && git push || echo 'Push failed'"
mockStream = (async function* () {
yield "\x1b]633;C\x07"
yield "Files added\n"
yield "Committed\n"
yield "Pushed successfully\n"
yield "\x1b]633;D\x07"
terminalProcess.emit("shell_execution_complete", { exitCode: 0 })
})()
mockTerminal.shellIntegration.executeCommand.mockReturnValue({
read: vi.fn().mockReturnValue(mockStream),
})
const runPromise = terminalProcess.run(complexCommand)
terminalProcess.emit("stream_available", mockStream)
await runPromise
// Should execute as single command
expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledTimes(1)
expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledWith(complexCommand)
})
})
describe("continue", () => {