Skip to content

Commit add8ecb

Browse files
committed
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.
1 parent 7a09564 commit add8ecb

File tree

3 files changed

+41
-8
lines changed

3 files changed

+41
-8
lines changed

src/integrations/terminal/BaseTerminal.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,25 @@ export abstract class BaseTerminal implements RooTerminal {
7171
* @param exitDetails The exit details of the shell execution
7272
*/
7373
public shellExecutionComplete(exitDetails: ExitCodeDetails) {
74-
this.busy = false
75-
this.running = false
76-
74+
// Only update state if we have an active process
75+
// This prevents duplicate calls from affecting the state
7776
if (this.process) {
77+
this.running = false
78+
7879
// Add to the front of the queue (most recent first).
7980
if (this.process.hasUnretrievedOutput()) {
8081
this.completedProcesses.unshift(this.process)
8182
}
8283

84+
// Emit the event before clearing the process reference
8385
this.process.emit("shell_execution_complete", exitDetails)
86+
87+
// Clear the process reference
88+
const completedProcess = this.process
8489
this.process = undefined
90+
91+
// The busy state will be managed by the TerminalProcess itself
92+
// to prevent race conditions with compound commands
8593
}
8694
}
8795

src/integrations/terminal/Terminal.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,16 @@ export class Terminal extends BaseTerminal {
5454
// This ensures that we don't miss any events because they are
5555
// configured before the process starts.
5656
process.on("line", (line) => callbacks.onLine(line, process))
57-
process.once("completed", (output) => callbacks.onCompleted(output, process))
57+
process.once("completed", (output) => {
58+
// Ensure busy is set to false when completed
59+
this.busy = false
60+
callbacks.onCompleted(output, process)
61+
})
5862
process.once("shell_execution_started", (pid) => callbacks.onShellExecutionStarted(pid, process))
59-
process.once("shell_execution_complete", (details) => callbacks.onShellExecutionComplete(details, process))
63+
process.once("shell_execution_complete", (details) => {
64+
// Note: busy state is managed by TerminalProcess and BaseTerminal
65+
callbacks.onShellExecutionComplete(details, process)
66+
})
6067
process.once("no_shell_integration", (msg) => callbacks.onNoShellIntegration?.(msg, process))
6168

6269
const promise = new Promise<void>((resolve, reject) => {

src/integrations/terminal/TerminalProcess.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,29 @@ import { Terminal } from "./Terminal"
1616

1717
export class TerminalProcess extends BaseTerminalProcess {
1818
private terminalRef: WeakRef<Terminal>
19+
private shellExecutionCompleteCount: number = 0
20+
private hasCompleted: boolean = false
1921

2022
constructor(terminal: Terminal) {
2123
super()
2224

2325
this.terminalRef = new WeakRef(terminal)
2426

2527
this.once("completed", () => {
26-
this.terminal.busy = false
28+
// Only set busy to false if not already done
29+
if (!this.hasCompleted) {
30+
this.hasCompleted = true
31+
this.terminal.busy = false
32+
}
2733
})
2834

2935
this.once("no_shell_integration", () => {
3036
this.emit("completed", "<no shell integration>")
31-
this.terminal.busy = false
37+
// Only set busy to false if not already done
38+
if (!this.hasCompleted) {
39+
this.hasCompleted = true
40+
this.terminal.busy = false
41+
}
3242
this.terminal.setActiveStream(undefined)
3343
this.continue()
3444
})
@@ -122,7 +132,15 @@ export class TerminalProcess extends BaseTerminalProcess {
122132

123133
// Create promise that resolves when shell execution completes for this terminal
124134
const shellExecutionComplete = new Promise<ExitCodeDetails>((resolve) => {
125-
this.once("shell_execution_complete", (details: ExitCodeDetails) => resolve(details))
135+
this.once("shell_execution_complete", (details: ExitCodeDetails) => {
136+
this.shellExecutionCompleteCount++
137+
if (this.shellExecutionCompleteCount > 1) {
138+
console.warn(
139+
`[TerminalProcess] shell_execution_complete fired ${this.shellExecutionCompleteCount} times for command: ${command}`,
140+
)
141+
}
142+
resolve(details)
143+
})
126144
})
127145

128146
// Execute command

0 commit comments

Comments
 (0)