fix: additional improvements to compound command handling

- Enhanced compound command detection logic
- Added more comprehensive test coverage
- Fixed edge cases in process completion tracking

Author: roomote

src/integrations/terminal/BaseTerminal.ts

@@ -27,7 +27,7 @@ export abstract class BaseTerminal implements RooTerminal {
 	// Compound command tracking
 	public isCompoundCommand: boolean = false
 	public compoundProcessCompletions: CompoundProcessCompletion[] = []
-	private expectedCompoundProcessCount: number = 0
+	public expectedCompoundProcessCount: number = 0
 	private compoundCommandWaitTimeout?: NodeJS.Timeout
 
 	constructor(provider: RooTerminalProvider, id: number, cwd: string) {
@@ -80,6 +80,16 @@ export abstract class BaseTerminal implements RooTerminal {
 	 * @param command The command to check
 	 */
 	public detectCompoundCommand(command: string): void {
+		// Reset previous compound command state
+		this.compoundProcessCompletions = []
+		this.expectedCompoundProcessCount = 0
+
+		// Clear any existing timeout
+		if (this.compoundCommandWaitTimeout) {
+			clearTimeout(this.compoundCommandWaitTimeout)
+			this.compoundCommandWaitTimeout = undefined
+		}
+
 		// Common shell operators that create compound commands
 		const compoundOperators = ["&&", "||", ";", "|", "&"]
 
@@ -99,7 +109,8 @@ export abstract class BaseTerminal implements RooTerminal {
 			const orMatches = command.match(/\|\|/g)
 			const semiMatches = command.match(/;/g)
 			const pipeMatches = command.match(/\|(?!\|)/g) // Match single | but not ||
-			const bgMatches = command.match(/&(?!&)/g) // Match single & but not &&
+			// Match single & but not &&, and not preceded by &
+			const bgMatches = command.match(/(?<!&)&(?!&)/g)
 
 			if (andMatches) processCount += andMatches.length
 			if (orMatches) processCount += orMatches.length
@@ -123,9 +134,6 @@ export abstract class BaseTerminal implements RooTerminal {
 					this.finalizeCompoundCommand()
 				}
 			}, 10000) // 10 second timeout for compound commands
-		} else {
-			this.compoundProcessCompletions = []
-			this.expectedCompoundProcessCount = 0
 		}
 	}
 
@@ -152,7 +160,8 @@ export abstract class BaseTerminal implements RooTerminal {
 		)
 
 		// Check if all expected processes have completed
-		if (this.compoundProcessCompletions.length >= this.expectedCompoundProcessCount) {
+		// Note: We check this after adding, so the finalization happens after the last process is added
+		if (this.allCompoundProcessesComplete()) {
 			console.info(`[Terminal ${this.id}] All compound processes complete, finalizing`)
 			this.finalizeCompoundCommand()
 		}
@@ -201,7 +210,7 @@ export abstract class BaseTerminal implements RooTerminal {
 	/**
 	 * Finalizes a compound command execution
 	 */
-	private finalizeCompoundCommand(): void {
+	public finalizeCompoundCommand(): void {
 		// Clear the timeout if it exists
 		if (this.compoundCommandWaitTimeout) {
 			clearTimeout(this.compoundCommandWaitTimeout)
@@ -216,13 +225,18 @@ export abstract class BaseTerminal implements RooTerminal {
 			`[Terminal ${this.id}] Finalizing compound command with ${this.compoundProcessCompletions.length} processes`,
 		)
 
-		// Reset compound command tracking
+		// Reset compound command tracking BEFORE calling shellExecutionComplete
+		// to prevent re-entrance issues
+		const wasCompound = this.isCompoundCommand
 		this.isCompoundCommand = false
 		this.compoundProcessCompletions = []
 		this.expectedCompoundProcessCount = 0
 
 		// Complete the terminal process with the final exit details
-		this.shellExecutionComplete(finalExitDetails)
+		// Only if we were actually tracking a compound command
+		if (wasCompound) {
+			this.shellExecutionComplete(finalExitDetails)
+		}
 	}
 
 	/**

src/integrations/terminal/TerminalRegistry.ts

@@ -95,16 +95,38 @@ export class TerminalRegistry {
 					}
 
 					// For compound commands, we need to track if this is just one part of a multi-process command
-					// Check if the terminal has pending compound processes
-					if (terminal.isCompoundCommand && !terminal.allCompoundProcessesComplete()) {
-						console.info(
-							"[TerminalRegistry] Compound command process completed, waiting for remaining processes:",
-							{ terminalId: terminal.id, command: e.execution?.commandLine?.value, exitCode: e.exitCode },
-						)
-
-						// Store this process completion but don't mark terminal as not busy yet
+					if (terminal.isCompoundCommand) {
+						// Check if this is the last process before adding the completion
+						const wasLastProcess =
+							terminal.compoundProcessCompletions.length ===
+							(terminal.expectedCompoundProcessCount || 0) - 1
+
+						// Store this process completion
+						// This may trigger finalization if it's the last process
 						terminal.addCompoundProcessCompletion(exitDetails, e.execution?.commandLine?.value || "")
-						return
+
+						// If this was the last process, the compound command has been finalized
+						// and terminal.busy has been set to false by finalizeCompoundCommand
+						if (wasLastProcess) {
+							console.info("[TerminalRegistry] All compound command processes completed and finalized:", {
+								terminalId: terminal.id,
+								busy: terminal.busy,
+							})
+							// The terminal has been finalized, just return
+							return
+						} else {
+							console.info(
+								"[TerminalRegistry] Compound command process completed, waiting for remaining processes:",
+								{
+									terminalId: terminal.id,
+									command: e.execution?.commandLine?.value,
+									exitCode: e.exitCode,
+									completedCount: terminal.compoundProcessCompletions.length,
+								},
+							)
+							// Still waiting for more processes
+							return
+						}
 					}
 
 					if (!terminal.running) {
@@ -115,8 +137,7 @@ export class TerminalRegistry {
 								"[TerminalRegistry] Shell execution end event received before terminal marked as running (compound command scenario):",
 								{ terminalId: terminal?.id, command: process?.command, exitCode: e.exitCode },
 							)
-							// Store this completion for later processing
-							terminal.addCompoundProcessCompletion(exitDetails, e.execution?.commandLine?.value || "")
+							// Already stored this completion above
 						} else {
 							console.error(
 								"[TerminalRegistry] Shell execution end event received, but process is not running for terminal:",
@@ -137,6 +158,7 @@ export class TerminalRegistry {
 					}
 
 					// Signal completion to any waiting processes.
+					// For compound commands, this will use the finalized exit details from all processes
 					terminal.shellExecutionComplete(exitDetails)
 					terminal.busy = false // Mark terminal as not busy when shell execution ends
 				},

src/integrations/terminal/__tests__/CompoundCommand.spec.ts

@@ -208,14 +208,17 @@ describe("Compound Command Handling", () => {
 		it("should format compound process outputs correctly", () => {
 			terminal.detectCompoundCommand("cd /tmp && ls")
 			terminal.addCompoundProcessCompletion({ exitCode: 0 }, "cd /tmp")
-			terminal.addCompoundProcessCompletion({ exitCode: 1 }, "ls")
 
-			const output = terminal.getCompoundProcessOutputs()
+			// Get output before the second completion triggers finalization
+			const outputBeforeFinalization = terminal.getCompoundProcessOutputs()
+			expect(outputBeforeFinalization).toContain("[Command: cd /tmp]")
+			expect(outputBeforeFinalization).toContain("[Exit Code: 0]")
+
+			terminal.addCompoundProcessCompletion({ exitCode: 1 }, "ls")
 
-			expect(output).toContain("[Command: cd /tmp]")
-			expect(output).toContain("[Exit Code: 0]")
-			expect(output).toContain("[Command: ls]")
-			expect(output).toContain("[Exit Code: 1]")
+			// After finalization, completions are cleared
+			const outputAfterFinalization = terminal.getCompoundProcessOutputs()
+			expect(outputAfterFinalization).toBe("")
 		})
 
 		it("should include signal information when present", () => {

src/integrations/terminal/__tests__/CompoundCommandSimple.spec.ts

@@ -0,0 +1,115 @@
+import { describe, it, expect, vi, beforeEach } from "vitest"
+import { BaseTerminal } from "../BaseTerminal"
+import type { ExitCodeDetails, RooTerminalCallbacks, RooTerminalProcessResultPromise } from "../types"
+
+// Create a concrete test implementation of BaseTerminal
+class TestTerminal extends BaseTerminal {
+	constructor(id: number, cwd: string) {
+		super("vscode", id, cwd)
+	}
+
+	isClosed(): boolean {
+		return false
+	}
+
+	runCommand(command: string, callbacks: RooTerminalCallbacks): RooTerminalProcessResultPromise {
+		throw new Error("Not implemented for test")
+	}
+}
+
+describe("Compound Command Simple Tests", () => {
+	let terminal: TestTerminal
+
+	beforeEach(() => {
+		terminal = new TestTerminal(1, "/test/path")
+		vi.clearAllMocks()
+		vi.useFakeTimers()
+	})
+
+	it("should properly finalize compound command and set busy to false", () => {
+		// Set up initial state
+		terminal.busy = true
+		terminal.running = true
+
+		// Create a mock process - need to implement EventEmitter interface
+		const mockProcess = {
+			command: "cd dir && ls",
+			emit: vi.fn(),
+			on: vi.fn(),
+			once: vi.fn(),
+			hasUnretrievedOutput: vi.fn(() => false),
+			getUnretrievedOutput: vi.fn(() => ""),
+			isHot: false,
+			run: vi.fn(),
+			continue: vi.fn(),
+			abort: vi.fn(),
+			// Add EventEmitter methods
+			addListener: vi.fn(),
+			removeListener: vi.fn(),
+			removeAllListeners: vi.fn(),
+			setMaxListeners: vi.fn(),
+			getMaxListeners: vi.fn(() => 10),
+			listeners: vi.fn(() => []),
+			rawListeners: vi.fn(() => []),
+			listenerCount: vi.fn(() => 0),
+			prependListener: vi.fn(),
+			prependOnceListener: vi.fn(),
+			eventNames: vi.fn(() => []),
+			off: vi.fn(),
+		}
+		terminal.process = mockProcess as any
+
+		// Detect compound command
+		terminal.detectCompoundCommand("cd dir && ls")
+		expect(terminal.isCompoundCommand).toBe(true)
+		expect(terminal.expectedCompoundProcessCount).toBe(2)
+
+		// Add first completion
+		terminal.addCompoundProcessCompletion({ exitCode: 0 }, "cd dir")
+		expect(terminal.busy).toBe(true) // Should still be busy
+		expect(terminal.isCompoundCommand).toBe(true) // Should still be compound
+		expect(terminal.compoundProcessCompletions).toHaveLength(1)
+
+		// Add second completion - this should trigger finalization
+		terminal.addCompoundProcessCompletion({ exitCode: 0 }, "ls")
+
+		// After finalization, terminal should not be busy
+		expect(terminal.busy).toBe(false)
+		expect(terminal.isCompoundCommand).toBe(false)
+		expect(terminal.compoundProcessCompletions).toHaveLength(0)
+
+		// Verify that shell_execution_complete was emitted
+		expect(mockProcess.emit).toHaveBeenCalledWith("shell_execution_complete", { exitCode: 0 })
+
+		// Process should be cleared
+		expect(terminal.process).toBeUndefined()
+	})
+
+	it("should handle timeout and finalize", () => {
+		terminal.busy = true
+		terminal.running = true
+
+		// Create a mock process
+		const mockProcess = {
+			command: "cd dir && ls",
+			emit: vi.fn(),
+			on: vi.fn(),
+			once: vi.fn(),
+			hasUnretrievedOutput: vi.fn(() => false),
+			getUnretrievedOutput: vi.fn(() => ""),
+		}
+		terminal.process = mockProcess as any
+
+		terminal.detectCompoundCommand("cd dir && ls")
+		terminal.addCompoundProcessCompletion({ exitCode: 0 }, "cd dir")
+
+		expect(terminal.busy).toBe(true)
+
+		// Fast forward to trigger timeout
+		vi.advanceTimersByTime(10001)
+
+		// Should be finalized after timeout
+		expect(terminal.busy).toBe(false)
+		expect(terminal.isCompoundCommand).toBe(false)
+	})
+})