From 384b469bf1d85bcdce7f2130e9ec404484bac8ac Mon Sep 17 00:00:00 2001 From: Eric Wheeler Date: Fri, 7 Mar 2025 19:03:37 -0800 Subject: [PATCH] Revert "Handle outputless commands" This reverts commit 710284cc3da1b61b22169b621457d6ae77029ec8 which has been superseded by PR #1365. Fixes: #1416 --- src/core/Cline.ts | 6 - src/integrations/terminal/TerminalProcess.ts | 43 ++--- .../__tests__/TerminalProcess.test.ts | 157 +----------------- 3 files changed, 13 insertions(+), 193 deletions(-) diff --git a/src/core/Cline.ts b/src/core/Cline.ts index ba171ce3fa..9163b2a79e 100644 --- a/src/core/Cline.ts +++ b/src/core/Cline.ts @@ -978,12 +978,6 @@ 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. diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index b31185dcc5..047465e3fe 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -41,17 +41,12 @@ 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] - /** - * 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 { @@ -60,7 +55,7 @@ export class TerminalProcess extends EventEmitter { private isListening = true private terminalInfo: TerminalInfo | undefined - private lastEmitAt = 0 + private lastEmitTime_ms = 0 private outputBuilder?: OutputBuilder private hotTimer: NodeJS.Timeout | null = null @@ -72,18 +67,14 @@ export class TerminalProcess extends EventEmitter { this._isHot = value } - constructor( - private readonly terminalOutputLimit: number, - private readonly stallTimeout: number = 5_000, - ) { + constructor(private readonly terminalOutputLimit: number) { 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") @@ -136,9 +127,11 @@ export class TerminalProcess extends EventEmitter { this.outputBuilder = new OutputBuilder({ maxSize: this.terminalOutputLimit }) - let stallTimer: NodeJS.Timeout | null = setTimeout(() => { - this.emit("stream_stalled", terminalInfo.id) - }, this.stallTimeout) + /** + * 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? + */ for await (let data of stream) { // Check for command output start marker. @@ -165,17 +158,11 @@ export class TerminalProcess extends EventEmitter { // 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.lastEmitAt + const timeSinceLastEmit = now - this.lastEmitTime_ms if (this.isListening && timeSinceLastEmit > EMIT_INTERVAL) { - if (this.flushLine()) { - if (stallTimer) { - clearTimeout(stallTimer) - stallTimer = null - } - - this.lastEmitAt = now - } + this.flushLine() + this.lastEmitTime_ms = now } // Set isHot depending on the command. @@ -271,10 +258,7 @@ export class TerminalProcess extends EventEmitter { if (line) { this.emit("line", line) - return true } - - return false } private flushAll() { @@ -286,10 +270,7 @@ export class TerminalProcess extends EventEmitter { if (buffer) { this.emit("line", buffer) - return true } - - return false } private processOutput(outputToProcess: string) { diff --git a/src/integrations/terminal/__tests__/TerminalProcess.test.ts b/src/integrations/terminal/__tests__/TerminalProcess.test.ts index 16cd85230d..c90c517e3e 100644 --- a/src/integrations/terminal/__tests__/TerminalProcess.test.ts +++ b/src/integrations/terminal/__tests__/TerminalProcess.test.ts @@ -20,9 +20,6 @@ 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< @@ -37,7 +34,7 @@ describe("TerminalProcess", () => { let mockStream: AsyncIterableIterator beforeEach(() => { - terminalProcess = new TerminalProcess(TERMINAL_OUTPUT_LIMIT, STALL_TIMEOUT) + terminalProcess = new TerminalProcess(100 * 1024) // Create properly typed mock terminal mockTerminal = { @@ -176,156 +173,4 @@ 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((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) - }) - }) })