From 087ddf25a5830fec6c12dbfdb3f4494ced3d592b Mon Sep 17 00:00:00 2001 From: Roo Code Date: Tue, 4 Nov 2025 13:32:45 +0000 Subject: [PATCH] fix: address review feedback for terminal output capture - Fix logic error where streamDataReceived check was always false - Replace with chunksReceived counter to properly track stream chunks - Add detailed documentation explaining timing values and rationale - Improve comments to explain the 200ms delay and 500 char threshold choices --- src/integrations/terminal/Terminal.ts | 8 +++++++- src/integrations/terminal/TerminalProcess.ts | 12 +++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index d32578d916..6f2cf98ccc 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -81,7 +81,13 @@ export class Terminal extends BaseTerminal { ShellIntegrationManager.zshCleanupTmpDir(this.id) // For newly created terminals on the first command, add a small delay - // to ensure the shell integration stream is fully ready + // to ensure the shell integration stream is fully ready. + // This addresses a race condition where the first command's output might not + // be captured properly, especially with chained commands like "ENV=hello && echo $ENV". + // The 200ms delay was chosen empirically to balance between: + // - Giving enough time for shell integration to fully initialize + // - Not adding noticeable latency to command execution + // This may need adjustment for slower systems or different shells. if (this.isNewlyCreated && !this.firstCommandExecuted) { console.log(`[Terminal ${this.id}] Adding delay for first command in new terminal`) await new Promise((resolve) => setTimeout(resolve, 200)) diff --git a/src/integrations/terminal/TerminalProcess.ts b/src/integrations/terminal/TerminalProcess.ts index be3aac2605..2d142286de 100644 --- a/src/integrations/terminal/TerminalProcess.ts +++ b/src/integrations/terminal/TerminalProcess.ts @@ -158,7 +158,7 @@ export class TerminalProcess extends BaseTerminalProcess { let preOutput = "" let commandOutputStarted = false - let streamDataReceived = false + let chunksReceived = 0 /* * Extract clean output from raw accumulated output. FYI: @@ -172,7 +172,7 @@ export class TerminalProcess extends BaseTerminalProcess { // Process stream data for await (let data of stream) { - streamDataReceived = true + chunksReceived++ // Check for command output start marker if (!commandOutputStarted) { @@ -185,13 +185,15 @@ export class TerminalProcess extends BaseTerminalProcess { this.fullOutput = "" // Reset fullOutput when command actually starts this.emit("line", "") // Trigger UI to proceed } else { - // For the first chunk of data, if we don't see markers yet, - // wait a bit more to see if they arrive in the next chunk - if (!streamDataReceived && preOutput.length < 100) { + // For the first few chunks, wait to see if markers arrive + // This handles cases where markers might be split across chunks + if (chunksReceived < 3 && preOutput.length < 100) { continue } // If we have accumulated enough preOutput without finding markers, // treat it as command output to avoid losing data + // 500 chars threshold chosen to balance between waiting for markers + // and not losing legitimate output from fast commands if (preOutput.length > 500) { console.warn( `[Terminal Process] No start markers found after ${preOutput.length} chars, treating as output`,