mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
fix: improve busy state management for compound terminal commands
- Add tracking for duplicate shell_execution_complete events - Prevent race conditions in busy state management by using hasCompleted flag - Move busy state management to TerminalProcess to ensure proper coordination - Fix callback handling in Terminal.ts to ensure busy is always cleared on completion - Remove duplicate busy state clearing in BaseTerminal.shellExecutionComplete This addresses the issue where compound commands could cause the terminal busy state to be incorrectly managed due to duplicate shell_execution_complete events or race conditions between different components.
This commit is contained in:
parent
7a095647ed
commit
add8ecb860
3 changed files with 41 additions and 8 deletions
|
|
@ -71,17 +71,25 @@ export abstract class BaseTerminal implements RooTerminal {
|
|||
* @param exitDetails The exit details of the shell execution
|
||||
*/
|
||||
public shellExecutionComplete(exitDetails: ExitCodeDetails) {
|
||||
this.busy = false
|
||||
this.running = false
|
||||
|
||||
// Only update state if we have an active process
|
||||
// This prevents duplicate calls from affecting the state
|
||||
if (this.process) {
|
||||
this.running = false
|
||||
|
||||
// Add to the front of the queue (most recent first).
|
||||
if (this.process.hasUnretrievedOutput()) {
|
||||
this.completedProcesses.unshift(this.process)
|
||||
}
|
||||
|
||||
// Emit the event before clearing the process reference
|
||||
this.process.emit("shell_execution_complete", exitDetails)
|
||||
|
||||
// Clear the process reference
|
||||
const completedProcess = this.process
|
||||
this.process = undefined
|
||||
|
||||
// The busy state will be managed by the TerminalProcess itself
|
||||
// to prevent race conditions with compound commands
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -54,9 +54,16 @@ export class Terminal extends BaseTerminal {
|
|||
// This ensures that we don't miss any events because they are
|
||||
// configured before the process starts.
|
||||
process.on("line", (line) => callbacks.onLine(line, process))
|
||||
process.once("completed", (output) => callbacks.onCompleted(output, process))
|
||||
process.once("completed", (output) => {
|
||||
// Ensure busy is set to false when completed
|
||||
this.busy = false
|
||||
callbacks.onCompleted(output, process)
|
||||
})
|
||||
process.once("shell_execution_started", (pid) => callbacks.onShellExecutionStarted(pid, process))
|
||||
process.once("shell_execution_complete", (details) => callbacks.onShellExecutionComplete(details, process))
|
||||
process.once("shell_execution_complete", (details) => {
|
||||
// Note: busy state is managed by TerminalProcess and BaseTerminal
|
||||
callbacks.onShellExecutionComplete(details, process)
|
||||
})
|
||||
process.once("no_shell_integration", (msg) => callbacks.onNoShellIntegration?.(msg, process))
|
||||
|
||||
const promise = new Promise<void>((resolve, reject) => {
|
||||
|
|
|
|||
|
|
@ -16,6 +16,8 @@ import { Terminal } from "./Terminal"
|
|||
|
||||
export class TerminalProcess extends BaseTerminalProcess {
|
||||
private terminalRef: WeakRef<Terminal>
|
||||
private shellExecutionCompleteCount: number = 0
|
||||
private hasCompleted: boolean = false
|
||||
|
||||
constructor(terminal: Terminal) {
|
||||
super()
|
||||
|
|
@ -23,12 +25,20 @@ export class TerminalProcess extends BaseTerminalProcess {
|
|||
this.terminalRef = new WeakRef(terminal)
|
||||
|
||||
this.once("completed", () => {
|
||||
this.terminal.busy = false
|
||||
// Only set busy to false if not already done
|
||||
if (!this.hasCompleted) {
|
||||
this.hasCompleted = true
|
||||
this.terminal.busy = false
|
||||
}
|
||||
})
|
||||
|
||||
this.once("no_shell_integration", () => {
|
||||
this.emit("completed", "<no shell integration>")
|
||||
this.terminal.busy = false
|
||||
// Only set busy to false if not already done
|
||||
if (!this.hasCompleted) {
|
||||
this.hasCompleted = true
|
||||
this.terminal.busy = false
|
||||
}
|
||||
this.terminal.setActiveStream(undefined)
|
||||
this.continue()
|
||||
})
|
||||
|
|
@ -122,7 +132,15 @@ export class TerminalProcess extends BaseTerminalProcess {
|
|||
|
||||
// Create promise that resolves when shell execution completes for this terminal
|
||||
const shellExecutionComplete = new Promise<ExitCodeDetails>((resolve) => {
|
||||
this.once("shell_execution_complete", (details: ExitCodeDetails) => resolve(details))
|
||||
this.once("shell_execution_complete", (details: ExitCodeDetails) => {
|
||||
this.shellExecutionCompleteCount++
|
||||
if (this.shellExecutionCompleteCount > 1) {
|
||||
console.warn(
|
||||
`[TerminalProcess] shell_execution_complete fired ${this.shellExecutionCompleteCount} times for command: ${command}`,
|
||||
)
|
||||
}
|
||||
resolve(details)
|
||||
})
|
||||
})
|
||||
|
||||
// Execute command
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue