Fix: Reset terminal busy state after manual commands complete (#4583)

fix: reset terminal busy state after manual commands complete (#4319)

- Add terminal.busy = true when shell execution starts
- Add terminal.busy = false when shell execution ends for both Roo and non-Roo terminals
- Add comprehensive tests for busy flag management
- Fixes issue where terminals got stuck in busy state after manual commands

Author: hannesrudolph

src/integrations/terminal/TerminalRegistry.ts

@@ -59,6 +59,7 @@ export class TerminalRegistry {
 
 					if (terminal) {
 						terminal.setActiveStream(stream)
+						terminal.busy = true // Mark terminal as busy when shell execution starts
 					} else {
 						console.error(
 							"[onDidStartTerminalShellExecution] Shell execution started, but not from a Roo-registered terminal:",
@@ -99,6 +100,7 @@ export class TerminalRegistry {
 							{ terminalId: terminal?.id, command: process?.command, exitCode: e.exitCode },
 						)
 
+						terminal.busy = false
 						return
 					}
 
@@ -113,6 +115,7 @@ export class TerminalRegistry {
 
 					// Signal completion to any waiting processes.
 					terminal.shellExecutionComplete(exitDetails)
+					terminal.busy = false // Mark terminal as not busy when shell execution ends
 				},
 			)
 

src/integrations/terminal/__tests__/TerminalRegistry.test.ts

@@ -2,21 +2,39 @@
 
 import { Terminal } from "../Terminal"
 import { TerminalRegistry } from "../TerminalRegistry"
+import * as vscode from "vscode"
 
 const PAGER = process.platform === "win32" ? "" : "cat"
 
 // Mock vscode.window.createTerminal
 const mockCreateTerminal = jest.fn()
 
+// Event handlers for testing
+let mockStartHandler: any = null
+let mockEndHandler: any = null
+
 jest.mock("vscode", () => ({
 	window: {
 		createTerminal: (...args: any[]) => {
 			mockCreateTerminal(...args)
 			return {
+				name: "Roo Code",
 				exitStatus: undefined,
+				dispose: jest.fn(),
+				show: jest.fn(),
+				hide: jest.fn(),
+				sendText: jest.fn(),
 			}
 		},
 		onDidCloseTerminal: jest.fn().mockReturnValue({ dispose: jest.fn() }),
+		onDidStartTerminalShellExecution: jest.fn().mockImplementation((handler) => {
+			mockStartHandler = handler
+			return { dispose: jest.fn() }
+		}),
+		onDidEndTerminalShellExecution: jest.fn().mockImplementation((handler) => {
+			mockEndHandler = handler
+			return { dispose: jest.fn() }
+		}),
 	},
 	ThemeIcon: jest.fn(),
 }))
@@ -28,6 +46,15 @@ jest.mock("execa", () => ({
 describe("TerminalRegistry", () => {
 	beforeEach(() => {
 		mockCreateTerminal.mockClear()
+
+		// Reset event handlers
+		mockStartHandler = null
+		mockEndHandler = null
+
+		// Clear terminals array for each test
+		;(TerminalRegistry as any).terminals = []
+		;(TerminalRegistry as any).nextTerminalId = 1
+		;(TerminalRegistry as any).isInitialized = false
 	})
 
 	describe("createTerminal", () => {
@@ -113,4 +140,188 @@ describe("TerminalRegistry", () => {
 			}
 		})
 	})
+
+	describe("busy flag management", () => {
+		let mockVsTerminal: any
+
+		beforeEach(() => {
+			mockVsTerminal = {
+				name: "Roo Code",
+				exitStatus: undefined,
+				dispose: jest.fn(),
+				show: jest.fn(),
+				hide: jest.fn(),
+				sendText: jest.fn(),
+			}
+			mockCreateTerminal.mockReturnValue(mockVsTerminal)
+		})
+
+		// Helper function to get the created Roo terminal and its underlying VSCode terminal
+		const createTerminalAndGetVsTerminal = (path: string = "/test/path") => {
+			const rooTerminal = TerminalRegistry.createTerminal(path, "vscode")
+			// Get the actual VSCode terminal that was created and stored
+			const vsTerminal = (rooTerminal as any).terminal
+			return { rooTerminal, vsTerminal }
+		}
+
+		it("should initialize terminal with busy = false", () => {
+			const terminal = TerminalRegistry.createTerminal("/test/path", "vscode")
+			expect(terminal.busy).toBe(false)
+		})
+
+		it("should set busy = true when shell execution starts", () => {
+			// Initialize the registry to set up event handlers
+			TerminalRegistry.initialize()
+
+			// Create a terminal and get the actual VSCode terminal
+			const { rooTerminal, vsTerminal } = createTerminalAndGetVsTerminal()
+			expect(rooTerminal.busy).toBe(false)
+
+			// Simulate shell execution start event
+			const execution = {
+				commandLine: { value: "echo test" },
+				read: jest.fn().mockReturnValue({}),
+			} as any
+
+			if (mockStartHandler) {
+				mockStartHandler({
+					terminal: vsTerminal,
+					execution,
+				})
+			}
+
+			expect(rooTerminal.busy).toBe(true)
+		})
+
+		it("should set busy = false when shell execution ends for Roo terminals", () => {
+			// Initialize the registry to set up event handlers
+			TerminalRegistry.initialize()
+
+			// Create a terminal and get the actual VSCode terminal
+			const { rooTerminal, vsTerminal } = createTerminalAndGetVsTerminal()
+			rooTerminal.busy = true
+
+			// Set up a mock process to simulate running state
+			const mockProcess = {
+				command: "echo test",
+				isHot: false,
+				hasUnretrievedOutput: () => false,
+			}
+			rooTerminal.process = mockProcess as any
+
+			// Simulate shell execution end event
+			const execution = {
+				commandLine: { value: "echo test" },
+			} as any
+
+			if (mockEndHandler) {
+				mockEndHandler({
+					terminal: vsTerminal,
+					execution,
+					exitCode: 0,
+				})
+			}
+
+			expect(rooTerminal.busy).toBe(false)
+		})
+
+		it("should set busy = false when shell execution ends for non-Roo terminals (manual commands)", () => {
+			// Initialize the registry to set up event handlers
+			TerminalRegistry.initialize()
+
+			// Simulate a shell execution end event for a terminal not in our registry
+			const unknownVsTerminal = {
+				name: "Unknown Terminal",
+			}
+
+			const execution = {
+				commandLine: { value: "sleep 30" },
+			} as any
+
+			// This should not throw an error and should handle the case gracefully
+			expect(() => {
+				if (mockEndHandler) {
+					mockEndHandler({
+						terminal: unknownVsTerminal,
+						execution,
+						exitCode: 0,
+					})
+				}
+			}).not.toThrow()
+		})
+
+		it("should handle busy flag reset when terminal process is not running", () => {
+			// Initialize the registry to set up event handlers
+			TerminalRegistry.initialize()
+
+			// Create a terminal and get the actual VSCode terminal
+			const { rooTerminal, vsTerminal } = createTerminalAndGetVsTerminal()
+			rooTerminal.busy = true
+
+			// Ensure terminal.running returns false (no active process)
+			Object.defineProperty(rooTerminal, "running", {
+				get: () => false,
+				configurable: true,
+			})
+
+			// Simulate shell execution end event
+			const execution = {
+				commandLine: { value: "echo test" },
+			} as any
+
+			if (mockEndHandler) {
+				mockEndHandler({
+					terminal: vsTerminal,
+					execution,
+					exitCode: 0,
+				})
+			}
+
+			// Should reset busy flag even when not running
+			expect(rooTerminal.busy).toBe(false)
+		})
+
+		it("should maintain busy state during command execution lifecycle", () => {
+			// Initialize the registry to set up event handlers
+			TerminalRegistry.initialize()
+
+			// Create a terminal and get the actual VSCode terminal
+			const { rooTerminal, vsTerminal } = createTerminalAndGetVsTerminal()
+			expect(rooTerminal.busy).toBe(false)
+
+			// Start execution
+			const execution = {
+				commandLine: { value: "npm test" },
+				read: jest.fn().mockReturnValue({}),
+			} as any
+
+			if (mockStartHandler) {
+				mockStartHandler({
+					terminal: vsTerminal,
+					execution,
+				})
+			}
+
+			expect(rooTerminal.busy).toBe(true)
+
+			// Set up mock process for running state
+			const mockProcess = {
+				command: "npm test",
+				isHot: true,
+				hasUnretrievedOutput: () => true,
+			}
+			rooTerminal.process = mockProcess as any
+
+			// End execution
+			if (mockEndHandler) {
+				mockEndHandler({
+					terminal: vsTerminal,
+					execution,
+					exitCode: 0,
+				})
+			}
+
+			expect(rooTerminal.busy).toBe(false)
+		})
+	})
 })