Skip to content

Commit 087ddf2

Browse files
committed
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
1 parent 2962282 commit 087ddf2

File tree

2 files changed

+14
-6
lines changed

2 files changed

+14
-6
lines changed

src/integrations/terminal/Terminal.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,13 @@ export class Terminal extends BaseTerminal {
8181
ShellIntegrationManager.zshCleanupTmpDir(this.id)
8282

8383
// For newly created terminals on the first command, add a small delay
84-
// to ensure the shell integration stream is fully ready
84+
// to ensure the shell integration stream is fully ready.
85+
// This addresses a race condition where the first command's output might not
86+
// be captured properly, especially with chained commands like "ENV=hello && echo $ENV".
87+
// The 200ms delay was chosen empirically to balance between:
88+
// - Giving enough time for shell integration to fully initialize
89+
// - Not adding noticeable latency to command execution
90+
// This may need adjustment for slower systems or different shells.
8591
if (this.isNewlyCreated && !this.firstCommandExecuted) {
8692
console.log(`[Terminal ${this.id}] Adding delay for first command in new terminal`)
8793
await new Promise((resolve) => setTimeout(resolve, 200))

src/integrations/terminal/TerminalProcess.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ export class TerminalProcess extends BaseTerminalProcess {
158158

159159
let preOutput = ""
160160
let commandOutputStarted = false
161-
let streamDataReceived = false
161+
let chunksReceived = 0
162162

163163
/*
164164
* Extract clean output from raw accumulated output. FYI:
@@ -172,7 +172,7 @@ export class TerminalProcess extends BaseTerminalProcess {
172172

173173
// Process stream data
174174
for await (let data of stream) {
175-
streamDataReceived = true
175+
chunksReceived++
176176

177177
// Check for command output start marker
178178
if (!commandOutputStarted) {
@@ -185,13 +185,15 @@ export class TerminalProcess extends BaseTerminalProcess {
185185
this.fullOutput = "" // Reset fullOutput when command actually starts
186186
this.emit("line", "") // Trigger UI to proceed
187187
} else {
188-
// For the first chunk of data, if we don't see markers yet,
189-
// wait a bit more to see if they arrive in the next chunk
190-
if (!streamDataReceived && preOutput.length < 100) {
188+
// For the first few chunks, wait to see if markers arrive
189+
// This handles cases where markers might be split across chunks
190+
if (chunksReceived < 3 && preOutput.length < 100) {
191191
continue
192192
}
193193
// If we have accumulated enough preOutput without finding markers,
194194
// treat it as command output to avoid losing data
195+
// 500 chars threshold chosen to balance between waiting for markers
196+
// and not losing legitimate output from fast commands
195197
if (preOutput.length > 500) {
196198
console.warn(
197199
`[Terminal Process] No start markers found after ${preOutput.length} chars, treating as output`,

0 commit comments

Comments
 (0)