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
This commit is contained in:
Roo Code 2025-11-04 13:32:45 +00:00
parent 2962282b4f
commit 087ddf25a5
2 changed files with 14 additions and 6 deletions

View file

@ -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))

View file

@ -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`,