Skip to content

Commit 7d557a8

Browse files
roomoteroomote
committed
fix: handle compound commands in terminal integration
- Add compound command detection to track multi-process commands - Implement process completion tracking for operators like &&, ||, ;, | - Wait for all processes in compound commands before reporting to LLM - Add comprehensive tests for compound command scenarios Fixes #7430
1 parent 934bfd0 commit 7d557a8

File tree

6 files changed

+454
-5
lines changed

6 files changed

+454
-5
lines changed

‎src/integrations/terminal/BaseTerminal.ts‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
RooTerminalProcess,
99
RooTerminalProcessResultPromise,
1010
ExitCodeDetails,
11+
CompoundProcessCompletion,
1112
} from "./types"
1213

1314
export abstract class BaseTerminal implements RooTerminal {
@@ -23,13 +24,21 @@ export abstract class BaseTerminal implements RooTerminal {
2324
public process?: RooTerminalProcess
2425
public completedProcesses: RooTerminalProcess[] = []
2526

27+
// Compound command tracking
28+
public isCompoundCommand: boolean = false
29+
public compoundProcessCompletions: CompoundProcessCompletion[] = []
30+
private expectedCompoundProcessCount: number = 0
31+
private compoundCommandWaitTimeout?: NodeJS.Timeout
32+
2633
constructor(provider: RooTerminalProvider, id: number, cwd: string) {
2734
this.provider = provider
2835
this.id = id
2936
this.initialCwd = cwd
3037
this.busy = false
3138
this.running = false
3239
this.streamClosed = false
40+
this.isCompoundCommand = false
41+
this.compoundProcessCompletions = []
3342
}
3443

3544
public getCurrentWorkingDirectory(): string {
@@ -66,6 +75,156 @@ export abstract class BaseTerminal implements RooTerminal {
6675
}
6776
}
6877

78+
/**
79+
* Detects if a command is a compound command (contains operators like &&, ||, ;)
80+
* @param command The command to check
81+
*/
82+
public detectCompoundCommand(command: string): void {
83+
// Common shell operators that create compound commands
84+
const compoundOperators = ["&&", "||", ";", "|", "&"]
85+
86+
// Check if command contains any compound operators
87+
this.isCompoundCommand = compoundOperators.some((op) => command.includes(op))
88+
89+
if (this.isCompoundCommand) {
90+
// Estimate the number of processes (this is a heuristic, not exact)
91+
// For && and ||, each operator adds one process
92+
// For ;, each semicolon adds one process
93+
// For |, each pipe adds one process
94+
// For &, it's a background process indicator
95+
let processCount = 1
96+
97+
// Count && and || operators
98+
const andMatches = command.match(/&&/g)
99+
const orMatches = command.match(/\|\|/g)
100+
const semiMatches = command.match(/;/g)
101+
const pipeMatches = command.match(/\|(?!\|)/g) // Match single | but not ||
102+
const bgMatches = command.match(/&(?!&)/g) // Match single & but not &&
103+
104+
if (andMatches) processCount += andMatches.length
105+
if (orMatches) processCount += orMatches.length
106+
if (semiMatches) processCount += semiMatches.length
107+
if (pipeMatches) processCount += pipeMatches.length
108+
if (bgMatches) processCount += bgMatches.length
109+
110+
this.expectedCompoundProcessCount = processCount
111+
112+
console.info(
113+
`[Terminal ${this.id}] Detected compound command with estimated ${processCount} processes:`,
114+
command,
115+
)
116+
117+
// Set a timeout to handle cases where we don't receive all expected completions
118+
this.compoundCommandWaitTimeout = setTimeout(() => {
119+
if (this.compoundProcessCompletions.length > 0) {
120+
console.warn(
121+
`[Terminal ${this.id}] Compound command timeout - processing ${this.compoundProcessCompletions.length} completions`,
122+
)
123+
this.finalizeCompoundCommand()
124+
}
125+
}, 10000) // 10 second timeout for compound commands
126+
} else {
127+
this.compoundProcessCompletions = []
128+
this.expectedCompoundProcessCount = 0
129+
}
130+
}
131+
132+
/**
133+
* Adds a compound process completion
134+
* @param exitDetails The exit details of the completed process
135+
* @param command The command that completed
136+
*/
137+
public addCompoundProcessCompletion(exitDetails: ExitCodeDetails, command: string): void {
138+
if (!this.isCompoundCommand) {
139+
console.warn(`[Terminal ${this.id}] Received compound process completion but not tracking compound command`)
140+
return
141+
}
142+
143+
this.compoundProcessCompletions.push({
144+
exitDetails,
145+
command,
146+
timestamp: Date.now(),
147+
})
148+
149+
console.info(
150+
`[Terminal ${this.id}] Added compound process completion ${this.compoundProcessCompletions.length}/${this.expectedCompoundProcessCount}:`,
151+
command,
152+
)
153+
154+
// Check if all expected processes have completed
155+
if (this.compoundProcessCompletions.length >= this.expectedCompoundProcessCount) {
156+
console.info(`[Terminal ${this.id}] All compound processes complete, finalizing`)
157+
this.finalizeCompoundCommand()
158+
}
159+
}
160+
161+
/**
162+
* Checks if all compound processes have completed
163+
* @returns True if all expected processes have completed
164+
*/
165+
public allCompoundProcessesComplete(): boolean {
166+
// If we're not tracking a compound command, consider it complete
167+
if (!this.isCompoundCommand) {
168+
return true
169+
}
170+
171+
// Check if we've received completions for all expected processes
172+
// We use >= because sometimes we might get more completions than expected
173+
const isComplete = this.compoundProcessCompletions.length >= this.expectedCompoundProcessCount
174+
175+
console.info(
176+
`[Terminal ${this.id}] Checking compound completion: ${this.compoundProcessCompletions.length}/${this.expectedCompoundProcessCount} = ${isComplete}`,
177+
)
178+
179+
return isComplete
180+
}
181+
182+
/**
183+
* Gets the combined output from all compound processes
184+
* @returns The combined output string
185+
*/
186+
public getCompoundProcessOutputs(): string {
187+
// Combine outputs from all completed processes
188+
const outputs: string[] = []
189+
190+
for (const completion of this.compoundProcessCompletions) {
191+
outputs.push(`[Command: ${completion.command}]`)
192+
outputs.push(`[Exit Code: ${completion.exitDetails.exitCode}]`)
193+
if (completion.exitDetails.signalName) {
194+
outputs.push(`[Signal: ${completion.exitDetails.signalName}]`)
195+
}
196+
}
197+
198+
return outputs.join("\n")
199+
}
200+
201+
/**
202+
* Finalizes a compound command execution
203+
*/
204+
private finalizeCompoundCommand(): void {
205+
// Clear the timeout if it exists
206+
if (this.compoundCommandWaitTimeout) {
207+
clearTimeout(this.compoundCommandWaitTimeout)
208+
this.compoundCommandWaitTimeout = undefined
209+
}
210+
211+
// Get the last exit details (from the final process in the chain)
212+
const lastCompletion = this.compoundProcessCompletions[this.compoundProcessCompletions.length - 1]
213+
const finalExitDetails = lastCompletion?.exitDetails || { exitCode: 0 }
214+
215+
console.info(
216+
`[Terminal ${this.id}] Finalizing compound command with ${this.compoundProcessCompletions.length} processes`,
217+
)
218+
219+
// Reset compound command tracking
220+
this.isCompoundCommand = false
221+
this.compoundProcessCompletions = []
222+
this.expectedCompoundProcessCount = 0
223+
224+
// Complete the terminal process with the final exit details
225+
this.shellExecutionComplete(finalExitDetails)
226+
}
227+
69228
/**
70229
* Handles shell execution completion for this terminal.
71230
* @param exitDetails The exit details of the shell execution

‎src/integrations/terminal/ExecaTerminal.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ export class ExecaTerminal extends BaseTerminal {
1818
public override runCommand(command: string, callbacks: RooTerminalCallbacks): RooTerminalProcessResultPromise {
1919
this.busy = true
2020

21+
// Detect if this is a compound command before creating the process
22+
this.detectCompoundCommand(command)
23+
2124
const process = new ExecaTerminalProcess(this)
2225
process.command = command
2326
this.process = process

‎src/integrations/terminal/Terminal.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ export class Terminal extends BaseTerminal {
4646
// from selecting the terminal for use during that time.
4747
this.busy = true
4848

49+
// Detect if this is a compound command before creating the process
50+
this.detectCompoundCommand(command)
51+
4952
const process = new TerminalProcess(this)
5053
process.command = command
5154
this.process = process

‎src/integrations/terminal/TerminalRegistry.ts‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,36 @@ export class TerminalRegistry {
9494
return
9595
}
9696

97-
if (!terminal.running) {
98-
console.error(
99-
"[TerminalRegistry] Shell execution end event received, but process is not running for terminal:",
100-
{ terminalId: terminal?.id, command: process?.command, exitCode: e.exitCode },
97+
// For compound commands, we need to track if this is just one part of a multi-process command
98+
// Check if the terminal has pending compound processes
99+
if (terminal.isCompoundCommand && !terminal.allCompoundProcessesComplete()) {
100+
console.info(
101+
"[TerminalRegistry] Compound command process completed, waiting for remaining processes:",
102+
{ terminalId: terminal.id, command: e.execution?.commandLine?.value, exitCode: e.exitCode },
101103
)
102104

103-
terminal.busy = false
105+
// Store this process completion but don't mark terminal as not busy yet
106+
terminal.addCompoundProcessCompletion(exitDetails, e.execution?.commandLine?.value || "")
107+
return
108+
}
109+
110+
if (!terminal.running) {
111+
// For compound commands that spawn processes quickly, we might get the end event
112+
// before the terminal is marked as running. Handle this gracefully.
113+
if (terminal.process && terminal.isCompoundCommand) {
114+
console.warn(
115+
"[TerminalRegistry] Shell execution end event received before terminal marked as running (compound command scenario):",
116+
{ terminalId: terminal?.id, command: process?.command, exitCode: e.exitCode },
117+
)
118+
// Store this completion for later processing
119+
terminal.addCompoundProcessCompletion(exitDetails, e.execution?.commandLine?.value || "")
120+
} else {
121+
console.error(
122+
"[TerminalRegistry] Shell execution end event received, but process is not running for terminal:",
123+
{ terminalId: terminal?.id, command: process?.command, exitCode: e.exitCode },
124+
)
125+
terminal.busy = false
126+
}
104127
return
105128
}
106129

0 commit comments

Comments
 (0)