diff --git a/src/integrations/terminal/BaseTerminal.ts b/src/integrations/terminal/BaseTerminal.ts index a79d417b07..4e4c47a557 100644 --- a/src/integrations/terminal/BaseTerminal.ts +++ b/src/integrations/terminal/BaseTerminal.ts @@ -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 } } diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 8bf2072f3d..24acfc6ffa 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -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((resolve, reject) => { diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index eb0424fe8d..d1d59979e4 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -16,6 +16,8 @@ import { Terminal } from "./Terminal" export class TerminalProcess extends BaseTerminalProcess { private terminalRef: WeakRef + 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", "") - 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() }) @@ -72,6 +82,27 @@ export class TerminalProcess extends BaseTerminalProcess { return } + // Check if command contains compound operators (&&, ||, ;, |) + if (this.isCompoundCommand(command)) { + console.info( + `[TerminalProcess] Detected compound command, executing as single shell command to preserve context`, + ) + } + + // Execute all commands (simple or compound) through shell integration + // This preserves shell context for compound commands (e.g., cd affects subsequent commands) + await this.runSingleCommand(command) + } + + private lastExitCode?: number + + /** + * Executes a command through VSCode's shell integration. + * Handles both simple and compound commands. + */ + private async runSingleCommand(command: string): Promise { + const terminal = this.terminal.terminal + // Create a promise that resolves when the stream becomes available const streamAvailable = new Promise>((resolve, reject) => { const timeoutId = setTimeout(() => { @@ -101,7 +132,15 @@ export class TerminalProcess extends BaseTerminalProcess { // Create promise that resolves when shell execution completes for this terminal const shellExecutionComplete = new Promise((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 @@ -127,9 +166,9 @@ export class TerminalProcess extends BaseTerminalProcess { commandToExecute += ` ; start-sleep -milliseconds ${Terminal.getCommandDelay()}` } - terminal.shellIntegration.executeCommand(commandToExecute) + terminal.shellIntegration!.executeCommand(commandToExecute) } else { - terminal.shellIntegration.executeCommand(command) + terminal.shellIntegration!.executeCommand(command) } this.isHot = true @@ -153,7 +192,7 @@ export class TerminalProcess extends BaseTerminalProcess { // Emit continue event to allow execution to proceed this.emit("continue") - return + return "" } let preOutput = "" @@ -209,7 +248,8 @@ export class TerminalProcess extends BaseTerminalProcess { this.terminal.setActiveStream(undefined) // Wait for shell execution to complete. - await shellExecutionComplete + const exitDetails = await shellExecutionComplete + this.lastExitCode = exitDetails.exitCode this.isHot = false @@ -237,7 +277,7 @@ export class TerminalProcess extends BaseTerminalProcess { this.continue() // Return early since we can't process output without shell integration markers - return + return "" } // fullOutput begins after C marker so we only need to trim off D marker @@ -253,8 +293,10 @@ export class TerminalProcess extends BaseTerminalProcess { // command is finished, we still want to consider it 'hot' in case // so that api request stalls to let diagnostics catch up"). this.stopHotTimer() + this.emit("completed", this.removeEscapeSequences(this.fullOutput)) this.emit("continue") + return this.removeEscapeSequences(this.fullOutput) } public override continue() { @@ -464,4 +506,63 @@ export class TerminalProcess extends BaseTerminalProcess { return match133 !== undefined ? match133 : match633 } + + /** + * Checks if a command contains compound operators (&&, ||, ;, |) + * that would typically spawn multiple processes. + * + * @param command The command string to check + * @returns True if the command contains compound operators + */ + private isCompoundCommand(command: string): boolean { + // Quick check for compound operators outside of quotes + let inSingleQuote = false + let inDoubleQuote = false + let escaped = false + + for (let i = 0; i < command.length; i++) { + const char = command[i] + const nextChar = command[i + 1] + + // Handle escape sequences + if (escaped) { + escaped = false + continue + } + + if (char === "\\" && !inSingleQuote) { + escaped = true + continue + } + + // Handle quotes + if (char === "'" && !inDoubleQuote) { + inSingleQuote = !inSingleQuote + continue + } + + if (char === '"' && !inSingleQuote) { + inDoubleQuote = !inDoubleQuote + continue + } + + // If we're inside quotes, skip operator detection + if (inSingleQuote || inDoubleQuote) { + continue + } + + // Check for operators (only outside quotes) + if (char === "&" && nextChar === "&") { + return true + } else if (char === "|" && nextChar === "|") { + return true + } else if (char === ";") { + return true + } else if (char === "|" && nextChar !== "|") { + return true + } + } + + return false + } } diff --git a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts index 04c31bd93a..39e444c56a 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.spec.ts @@ -165,6 +165,97 @@ describe("TerminalProcess", () => { await completePromise expect(terminalProcess.isHot).toBe(false) }) + + it("handles compound commands as single command", async () => { + // Test that compound commands are executed as a single command + // to preserve shell context between segments + const compoundCommand = "cd foo && npm test" + + let executedCommand = "" + + mockStream = (async function* () { + yield "\x1b]633;C\x07" + yield "Changed directory to foo\n" + yield "Running tests...\n" + yield "Tests passed" + yield "\x1b]633;D\x07" + terminalProcess.emit("shell_execution_complete", { exitCode: 0 }) + })() + + mockTerminal.shellIntegration.executeCommand.mockImplementation((cmd: string) => { + executedCommand = cmd + return { + read: vi.fn().mockReturnValue(mockStream), + } + }) + + const runPromise = terminalProcess.run(compoundCommand) + + // Emit stream available with the mock stream + terminalProcess.emit("stream_available", mockStream) + + await runPromise + + // Verify the compound command was executed as a single command + expect(executedCommand).toBe(compoundCommand) + expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledTimes(1) + expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledWith(compoundCommand) + }) + + it("preserves shell context in compound commands", async () => { + // This test verifies that compound commands maintain context + // For example, cd in the first part affects the second part + const command = "cd /tmp && pwd" + + mockStream = (async function* () { + yield "\x1b]633;C\x07" + yield "/tmp\n" // pwd should output /tmp since cd was in same shell context + yield "\x1b]633;D\x07" + terminalProcess.emit("shell_execution_complete", { exitCode: 0 }) + })() + + mockTerminal.shellIntegration.executeCommand.mockReturnValue({ + read: vi.fn().mockReturnValue(mockStream), + }) + + let output = "" + terminalProcess.on("completed", (result) => { + output = result || "" + }) + + const runPromise = terminalProcess.run(command) + terminalProcess.emit("stream_available", mockStream) + await runPromise + + expect(output.trim()).toBe("/tmp") + // Verify it was executed as a single command + expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledWith(command) + }) + + it("handles complex compound commands with multiple operators", async () => { + const complexCommand = "git add . && git commit -m 'test' && git push || echo 'Push failed'" + + mockStream = (async function* () { + yield "\x1b]633;C\x07" + yield "Files added\n" + yield "Committed\n" + yield "Pushed successfully\n" + yield "\x1b]633;D\x07" + terminalProcess.emit("shell_execution_complete", { exitCode: 0 }) + })() + + mockTerminal.shellIntegration.executeCommand.mockReturnValue({ + read: vi.fn().mockReturnValue(mockStream), + }) + + const runPromise = terminalProcess.run(complexCommand) + terminalProcess.emit("stream_available", mockStream) + await runPromise + + // Should execute as single command + expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledTimes(1) + expect(mockTerminal.shellIntegration.executeCommand).toHaveBeenCalledWith(complexCommand) + }) }) describe("continue", () => {