Merge pull request #1416 from RooVetGit/cte/handle-outputless-commands

Handle outputless commands

Author: cte

.changeset/wise-bats-perform.md

@@ -0,0 +1,5 @@
+---
+"roo-cline": patch
+---
+
+Handle outputless commands

src/core/Cline.ts

@@ -971,6 +971,12 @@ export class Cline {
 			await this.say("shell_integration_warning")
 		})
 
+		process.once("stream_stalled", async (id: number) => {
+			if (id === terminalInfo.id && !didContinue) {
+				sendCommandOutput("")
+			}
+		})
+
 		await process
 
 		// Wait for a short delay to ensure all messages are sent to the webview.

src/integrations/terminal/TerminalProcess.ts

@@ -41,12 +41,17 @@ export interface TerminalProcessEvents {
 	error: [error: Error]
 	no_shell_integration: []
 	/**
-	 * Emitted when a shell execution completes
+	 * Emitted when a shell execution completes.
 	 * @param id The terminal ID
 	 * @param exitDetails Contains exit code and signal information if process was terminated by signal
 	 */
 	shell_execution_complete: [id: number, exitDetails: ExitCodeDetails]
 	stream_available: [id: number, stream: AsyncIterable<string>]
+	/**
+	 * Emitted when an execution fails to emit a "line" event for a given period of time.
+	 * @param id The terminal ID
+	 */
+	stream_stalled: [id: number]
 }
 
 export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
@@ -55,7 +60,7 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 
 	private isListening = true
 	private terminalInfo: TerminalInfo | undefined
-	private lastEmitTime_ms = 0
+	private lastEmitAt = 0
 	private outputBuilder?: OutputBuilder
 	private hotTimer: NodeJS.Timeout | null = null
 
@@ -67,14 +72,18 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 		this._isHot = value
 	}
 
-	constructor(private readonly terminalOutputLimit: number) {
+	constructor(
+		private readonly terminalOutputLimit: number,
+		private readonly stallTimeout: number = 5_000,
+	) {
 		super()
 	}
 
 	async run(terminal: vscode.Terminal, command: string) {
 		if (terminal.shellIntegration && terminal.shellIntegration.executeCommand) {
-			// Get terminal info to access stream
+			// Get terminal info to access stream.
 			const terminalInfo = TerminalRegistry.getTerminalInfoByTerminal(terminal)
+
 			if (!terminalInfo) {
 				console.error("[TerminalProcess] Terminal not found in registry")
 				this.emit("no_shell_integration")
@@ -127,11 +136,9 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 
 			this.outputBuilder = new OutputBuilder({ maxSize: this.terminalOutputLimit })
 
-			/**
-			 * Some commands won't result in output flushing until the command
-			 * completes. This locks the UI. Should we set a timer to prompt
-			 * the user to continue?
-			 */
+			let stallTimer: NodeJS.Timeout | null = setTimeout(() => {
+				this.emit("stream_stalled", terminalInfo.id)
+			}, this.stallTimeout)
 
 			for await (let data of stream) {
 				// Check for command output start marker.
@@ -158,11 +165,17 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 				// right away but this wouldn't happen until it emits a line break, so
 				// as soon as we get any output we emit to let webview know to show spinner.
 				const now = Date.now()
-				const timeSinceLastEmit = now - this.lastEmitTime_ms
+				const timeSinceLastEmit = now - this.lastEmitAt
 
 				if (this.isListening && timeSinceLastEmit > EMIT_INTERVAL) {
-					this.flushLine()
-					this.lastEmitTime_ms = now
+					if (this.flushLine()) {
+						if (stallTimer) {
+							clearTimeout(stallTimer)
+							stallTimer = null
+						}
+
+						this.lastEmitAt = now
+					}
 				}
 
 				// Set isHot depending on the command.
@@ -258,7 +271,10 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 
 		if (line) {
 			this.emit("line", line)
+			return true
 		}
+
+		return false
 	}
 
 	private flushAll() {
@@ -270,7 +286,10 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 
 		if (buffer) {
 			this.emit("line", buffer)
+			return true
 		}
+
+		return false
 	}
 
 	private processOutput(outputToProcess: string) {

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

@@ -20,6 +20,9 @@ jest.mock("vscode", () => ({
 	ThemeIcon: jest.fn(),
 }))
 
+const TERMINAL_OUTPUT_LIMIT = 100 * 1024
+const STALL_TIMEOUT = 100
+
 describe("TerminalProcess", () => {
 	let terminalProcess: TerminalProcess
 	let mockTerminal: jest.Mocked<
@@ -34,7 +37,7 @@ describe("TerminalProcess", () => {
 	let mockStream: AsyncIterableIterator<string>
 
 	beforeEach(() => {
-		terminalProcess = new TerminalProcess(100 * 1024)
+		terminalProcess = new TerminalProcess(TERMINAL_OUTPUT_LIMIT, STALL_TIMEOUT)
 
 		// Create properly typed mock terminal
 		mockTerminal = {
@@ -173,4 +176,156 @@ describe("TerminalProcess", () => {
 			expect(terminalProcess["isListening"]).toBe(false)
 		})
 	})
+
+	describe("stalled stream handling", () => {
+		it("emits stream_stalled event when no output is received within timeout", async () => {
+			// Create a promise that resolves when stream_stalled is emitted
+			const streamStalledPromise = new Promise<number>((resolve) => {
+				terminalProcess.once("stream_stalled", (id: number) => {
+					resolve(id)
+				})
+			})
+
+			// Create a stream that doesn't emit any data
+			mockStream = (async function* () {
+				yield "\x1b]633;C\x07" // Command start sequence
+				// No data is yielded after this, causing the stall
+				await new Promise((resolve) => setTimeout(resolve, STALL_TIMEOUT * 2))
+				// This would normally be yielded, but the stall timer will fire first
+				yield "Output after stall"
+				yield "\x1b]633;D\x07" // Command end sequence
+				terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 })
+			})()
+
+			mockExecution = {
+				read: jest.fn().mockReturnValue(mockStream),
+			}
+
+			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
+
+			// Start the terminal process
+			const runPromise = terminalProcess.run(mockTerminal, "test command")
+			terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream)
+
+			// Wait for the stream_stalled event
+			const stalledId = await streamStalledPromise
+
+			// Verify the event was emitted with the correct terminal ID
+			expect(stalledId).toBe(mockTerminalInfo.id)
+
+			// Complete the run
+			await runPromise
+		})
+
+		it("clears stall timer when output is received", async () => {
+			// Spy on the emit method to check if stream_stalled is emitted
+			const emitSpy = jest.spyOn(terminalProcess, "emit")
+
+			// Create a stream that emits data before the stall timeout
+			mockStream = (async function* () {
+				yield "\x1b]633;C\x07" // Command start sequence
+				yield "Initial output\n" // This should clear the stall timer
+
+				// Wait longer than the stall timeout
+				await new Promise((resolve) => setTimeout(resolve, STALL_TIMEOUT * 2))
+
+				yield "More output\n"
+				yield "\x1b]633;D\x07" // Command end sequence
+				terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 })
+			})()
+
+			mockExecution = {
+				read: jest.fn().mockReturnValue(mockStream),
+			}
+
+			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
+
+			// Start the terminal process
+			const runPromise = terminalProcess.run(mockTerminal, "test command")
+			terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream)
+
+			// Wait for the run to complete
+			await runPromise
+
+			// Wait a bit longer to ensure the stall timer would have fired if not cleared
+			await new Promise((resolve) => setTimeout(resolve, STALL_TIMEOUT * 2))
+
+			// Verify stream_stalled was not emitted
+			expect(emitSpy).not.toHaveBeenCalledWith("stream_stalled", expect.anything())
+		})
+
+		it("returns true from flushLine when a line is emitted", async () => {
+			// Create a stream with output
+			mockStream = (async function* () {
+				yield "\x1b]633;C\x07" // Command start sequence
+				yield "Test output\n" // This should be flushed as a line
+				yield "\x1b]633;D\x07" // Command end sequence
+				terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 })
+			})()
+
+			mockExecution = {
+				read: jest.fn().mockReturnValue(mockStream),
+			}
+
+			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
+
+			// Spy on the flushLine method
+			const flushLineSpy = jest.spyOn(terminalProcess as any, "flushLine")
+
+			// Spy on the emit method to check if line is emitted
+			const emitSpy = jest.spyOn(terminalProcess, "emit")
+
+			// Start the terminal process
+			const runPromise = terminalProcess.run(mockTerminal, "test command")
+			terminalProcess.emit("stream_available", mockTerminalInfo.id, mockStream)
+
+			// Wait for the run to complete
+			await runPromise
+
+			// Verify flushLine was called and returned true
+			expect(flushLineSpy).toHaveBeenCalled()
+			expect(flushLineSpy.mock.results.some((result) => result.value === true)).toBe(true)
+
+			// Verify line event was emitted
+			expect(emitSpy).toHaveBeenCalledWith("line", expect.any(String))
+		})
+
+		it("returns false from flushLine when no line is emitted", async () => {
+			// Create a stream with no complete lines
+			mockStream = (async function* () {
+				yield "\x1b]633;C\x07" // Command start sequence
+				yield "Test output" // No newline, so this won't be flushed as a line yet
+				yield "\x1b]633;D\x07" // Command end sequence
+				terminalProcess.emit("shell_execution_complete", mockTerminalInfo.id, { exitCode: 0 })
+			})()
+
+			mockExecution = {
+				read: jest.fn().mockReturnValue(mockStream),
+			}
+
+			mockTerminal.shellIntegration.executeCommand.mockReturnValue(mockExecution)
+
+			// Create a custom implementation to test flushLine directly
+			const testFlushLine = async () => {
+				// Create a new instance with the same configuration
+				const testProcess = new TerminalProcess(TERMINAL_OUTPUT_LIMIT, STALL_TIMEOUT)
+
+				// Set up the output builder with content that doesn't have a newline
+				testProcess["outputBuilder"] = {
+					readLine: jest.fn().mockReturnValue(""),
+					append: jest.fn(),
+					reset: jest.fn(),
+					content: "Test output",
+				} as any
+
+				// Call flushLine directly
+				const result = testProcess["flushLine"]()
+				return result
+			}
+
+			// Test flushLine directly
+			const flushLineResult = await testFlushLine()
+			expect(flushLineResult).toBe(false)
+		})
+	})
 })