From 7f02ebac0c6c0478e802013f84e279d8a69fa9fb Mon Sep 17 00:00:00 2001 From: Na Yue Date: Wed, 9 Apr 2025 20:54:00 -0700 Subject: [PATCH 1/2] fix shell command execution output markdown block issue --- .../codewhispererChat/tools/executeBash.ts | 84 +++++++++++++++++-- 1 file changed, 78 insertions(+), 6 deletions(-) diff --git a/packages/core/src/codewhispererChat/tools/executeBash.ts b/packages/core/src/codewhispererChat/tools/executeBash.ts index 616ea49ed47..b7b2e86e7c6 100644 --- a/packages/core/src/codewhispererChat/tools/executeBash.ts +++ b/packages/core/src/codewhispererChat/tools/executeBash.ts @@ -117,6 +117,14 @@ export interface CommandValidation { warning?: string } +// Interface for timestamped output chunks +interface TimestampedChunk { + timestamp: number + isStdout: boolean + content: string + isFirst: boolean +} + export class ExecuteBash { private readonly command: string private readonly workingDirectory?: string @@ -207,8 +215,33 @@ export class ExecuteBash { const stdoutBuffer: string[] = [] const stderrBuffer: string[] = [] - let firstChunk = true - let firstStderrChunk = true + // Use a queue to maintain chronological order of chunks + // This ensures that the output is processed in the exact order it was generated by the child process. + const outputQueue: TimestampedChunk[] = [] + let processingQueue = false + const firstChunk = new AtomicBoolean(true) + + // Process the queue in order + const processQueue = () => { + if (processingQueue || outputQueue.length === 0) { + return + } + + processingQueue = true + + try { + // Sort by timestamp to ensure chronological order + outputQueue.sort((a, b) => a.timestamp - b.timestamp) + + while (outputQueue.length > 0) { + const chunk = outputQueue.shift()! + ExecuteBash.handleTimestampedChunk(chunk, stdoutBuffer, stderrBuffer, updates) + } + } finally { + processingQueue = false + } + } + const childProcessOptions: ChildProcessOptions = { spawnOptions: { cwd: this.workingDirectory, @@ -217,12 +250,26 @@ export class ExecuteBash { collect: false, waitForStreams: true, onStdout: (chunk: string) => { - ExecuteBash.handleChunk(firstChunk ? '```console\n' + chunk : chunk, stdoutBuffer, updates) - firstChunk = false + const isFirst = firstChunk.getAndSet(false) + const timestamp = Date.now() + outputQueue.push({ + timestamp, + isStdout: true, + content: chunk, + isFirst, + }) + processQueue() }, onStderr: (chunk: string) => { - ExecuteBash.handleChunk(firstStderrChunk ? '```console\n' + chunk : chunk, stderrBuffer, updates) - firstStderrChunk = false + const isFirst = firstChunk.getAndSet(false) + const timestamp = Date.now() + outputQueue.push({ + timestamp, + isStdout: false, + content: chunk, + isFirst, + }) + processQueue() }, } @@ -261,6 +308,17 @@ export class ExecuteBash { }) } + private static handleTimestampedChunk( + chunk: TimestampedChunk, + stdoutBuffer: string[], + stderrBuffer: string[], + updates?: Writable + ): void { + const buffer = chunk.isStdout ? stdoutBuffer : stderrBuffer + const content = chunk.isFirst ? '```console\n' + chunk.content : chunk.content + ExecuteBash.handleChunk(content, buffer, updates) + } + private static handleChunk(chunk: string, buffer: string[], updates?: Writable) { try { updates?.write(chunk) @@ -307,3 +365,17 @@ export class ExecuteBash { updates.end() } } + +class AtomicBoolean { + private value: boolean + + constructor(initialValue: boolean) { + this.value = initialValue + } + + public getAndSet(newValue: boolean): boolean { + const oldValue = this.value + this.value = newValue + return oldValue + } +} From 72a589ba3a0fefbee4914052340bfd95c1630cdf Mon Sep 17 00:00:00 2001 From: Na Yue Date: Thu, 10 Apr 2025 11:37:40 -0700 Subject: [PATCH 2/2] Use a closure boolean value to check the first chunk --- .../codewhispererChat/tools/executeBash.ts | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/packages/core/src/codewhispererChat/tools/executeBash.ts b/packages/core/src/codewhispererChat/tools/executeBash.ts index b7b2e86e7c6..605d6bbeb8f 100644 --- a/packages/core/src/codewhispererChat/tools/executeBash.ts +++ b/packages/core/src/codewhispererChat/tools/executeBash.ts @@ -215,11 +215,18 @@ export class ExecuteBash { const stdoutBuffer: string[] = [] const stderrBuffer: string[] = [] + // Use a closure boolean value firstChunk and a function to get and set its value + let isFirstChunk = true + const getAndSetFirstChunk = (newValue: boolean): boolean => { + const oldValue = isFirstChunk + isFirstChunk = newValue + return oldValue + } + // Use a queue to maintain chronological order of chunks // This ensures that the output is processed in the exact order it was generated by the child process. const outputQueue: TimestampedChunk[] = [] let processingQueue = false - const firstChunk = new AtomicBoolean(true) // Process the queue in order const processQueue = () => { @@ -249,8 +256,8 @@ export class ExecuteBash { }, collect: false, waitForStreams: true, - onStdout: (chunk: string) => { - const isFirst = firstChunk.getAndSet(false) + onStdout: async (chunk: string) => { + const isFirst = getAndSetFirstChunk(false) const timestamp = Date.now() outputQueue.push({ timestamp, @@ -260,8 +267,8 @@ export class ExecuteBash { }) processQueue() }, - onStderr: (chunk: string) => { - const isFirst = firstChunk.getAndSet(false) + onStderr: async (chunk: string) => { + const isFirst = getAndSetFirstChunk(false) const timestamp = Date.now() outputQueue.push({ timestamp, @@ -365,17 +372,3 @@ export class ExecuteBash { updates.end() } } - -class AtomicBoolean { - private value: boolean - - constructor(initialValue: boolean) { - this.value = initialValue - } - - public getAndSet(newValue: boolean): boolean { - const oldValue = this.value - this.value = newValue - return oldValue - } -}