fix(BashTool): include captured output in non-zero-exit error result (#1231)

src/tools/BashTool/BashTool.tsx

@@ -48,7 +48,7 @@ import { parseSedEditCommand } from './sedEditParser.js';
 import { shouldUseSandbox } from './shouldUseSandbox.js';
 import { BASH_TOOL_NAME } from './toolName.js';
 import { BackgroundHint, renderToolResultMessage, renderToolUseErrorMessage, renderToolUseMessage, renderToolUseProgressMessage, renderToolUseQueuedMessage } from './UI.js';
-import { buildImageToolResult, isImageOutput, resetCwdIfOutsideProject, resizeShellImageOutput, stdErrAppendShellResetMessage, stripEmptyLines } from './utils.js';
+import { buildImageToolResult, isImageOutput, resetCwdIfOutsideProject, resizeShellImageOutput, selectFailureOutput, stdErrAppendShellResetMessage, stripEmptyLines } from './utils.js';
 const EOL = '\n';
 
 // Progress display constants
@@ -667,23 +667,32 @@ export const BashTool = buildTool({
 
       // Consume the generator and capture the return value
       let generatorResult;
+      // Capture the most recent `fullOutput` yielded by the streaming
+      // generator so we can fall back to it for failure messages when the
+      // final ExecResult.stdout slot ends up empty (#1231).
+      let lastProgressFullOutput = '';
       do {
         generatorResult = await commandGenerator.next();
-        if (!generatorResult.done && onProgress) {
+        if (!generatorResult.done) {
           const progress = generatorResult.value;
-          onProgress({
-            toolUseID: `bash-progress-${progressCounter++}`,
-            data: {
-              type: 'bash_progress',
-              output: progress.output,
-              fullOutput: progress.fullOutput,
-              elapsedTimeSeconds: progress.elapsedTimeSeconds,
-              totalLines: progress.totalLines,
-              totalBytes: progress.totalBytes,
-              taskId: progress.taskId,
-              timeoutMs: progress.timeoutMs
-            }
-          });
+          if (typeof progress.fullOutput === 'string' && progress.fullOutput.length > 0) {
+            lastProgressFullOutput = progress.fullOutput;
+          }
+          if (onProgress) {
+            onProgress({
+              toolUseID: `bash-progress-${progressCounter++}`,
+              data: {
+                type: 'bash_progress',
+                output: progress.output,
+                fullOutput: progress.fullOutput,
+                elapsedTimeSeconds: progress.elapsedTimeSeconds,
+                totalLines: progress.totalLines,
+                totalBytes: progress.totalBytes,
+                taskId: progress.taskId,
+                timeoutMs: progress.timeoutMs
+              }
+            });
+          }
         }
       } while (!generatorResult.done);
 
@@ -715,8 +724,30 @@ export const BashTool = buildTool({
         }
       }
 
-      // Annotate output with sandbox violations if any (stderr is in stdout)
-      const outputWithSbFailures = SandboxManager.annotateStderrWithSandboxFailures(input.command, result.stdout || '');
+      // Annotate output with sandbox violations if any (stderr is in stdout).
+      // Issue #1231: pick the best non-empty failure body across three
+      // sources, ordered by trust:
+      //   1. The accumulator (after stripping the synthetic "Exit code N"
+      //      marker we appended above) — mirrors the success-path stdout
+      //      that was appended at line 696 above.
+      //   2. result.stdout from the shell runner.
+      //   3. lastProgressFullOutput — the most recent fullOutput yielded by
+      //      the streaming generator. Recovers stdout when the shell runner
+      //      streamed every line through progress callbacks but the final
+      //      ExecResult.stdout slot was left empty (flush-after-result race,
+      //      exit before EOF, persisted to file path, etc.).
+      // Strip the trailing "Exit code N" so getErrorParts() doesn't
+      // duplicate it; ShellError carries the code separately.
+      const accumulatedOutput = stdoutAccumulator
+        .toString()
+        .replace(new RegExp(`\\nExit code ${result.code}$`), '')
+        .replace(new RegExp(`^Exit code ${result.code}$`), '');
+      const failureOutput = selectFailureOutput(
+        accumulatedOutput,
+        result.stdout,
+        lastProgressFullOutput,
+      );
+      const outputWithSbFailures = SandboxManager.annotateStderrWithSandboxFailures(input.command, failureOutput);
       if (result.preSpawnError) {
         throw new Error(result.preSpawnError);
       }

src/tools/BashTool/utils.test.ts

@@ -5,6 +5,7 @@ import {
   parseDataUri,
   formatOutput,
   createContentSummary,
+  selectFailureOutput,
 } from './utils.js'
 
 // =============================================================================
@@ -210,3 +211,47 @@ describe('createContentSummary', () => {
     expect(result).toContain('MCP Result')
   })
 })
+
+// =============================================================================
+// selectFailureOutput — picks the best source for a non-zero-exit failure body
+// =============================================================================
+
+describe('selectFailureOutput (#1231)', () => {
+  test('prefers the accumulator when it has content', () => {
+    expect(selectFailureOutput('build failed\nerror: missing tsc', 'unused', 'also unused'))
+      .toBe('build failed\nerror: missing tsc')
+  })
+
+  test('falls back to result.stdout when accumulator is empty', () => {
+    expect(selectFailureOutput('', 'stdout body', 'unused')).toBe('stdout body')
+  })
+
+  test('falls back to result.stdout when accumulator is whitespace-only', () => {
+    expect(selectFailureOutput('   \n\n  ', 'real stdout', 'unused')).toBe('real stdout')
+  })
+
+  test('recovers from progress.fullOutput when accumulator and result.stdout are both empty', () => {
+    expect(selectFailureOutput('', '', 'streamed line 1\nstreamed line 2'))
+      .toBe('streamed line 1\nstreamed line 2')
+  })
+
+  test('recovers from progress.fullOutput when result.stdout is undefined (shell runner left slot empty)', () => {
+    // Reproduces #1231: accumulator was only `\nExit code N` (stripped to ''),
+    // result.stdout was empty because the shell runner streamed everything
+    // through progress callbacks, but lastProgressFullOutput captured every
+    // line on the way through.
+    expect(selectFailureOutput('', undefined, 'tsc error TS2305\nbuild halted'))
+      .toBe('tsc error TS2305\nbuild halted')
+  })
+
+  test('returns empty string when all three sources are empty', () => {
+    expect(selectFailureOutput('', undefined, '')).toBe('')
+    expect(selectFailureOutput('', '', '')).toBe('')
+  })
+
+  test('whitespace-only progressFullOutput does not win over empty result.stdout', () => {
+    // Pure whitespace from a progress flush should not be treated as a
+    // recoverable failure body — would otherwise leak just "\n" or "   ".
+    expect(selectFailureOutput('', '', '   \n  \n')).toBe('')
+  })
+})

src/tools/BashTool/utils.ts

@@ -221,3 +221,31 @@ export function createContentSummary(content: ContentBlockParam[]): string {
 
   return `MCP Result: ${summary.join(', ')}${parts.length > 0 ? '\n\n' + parts.join('\n\n') : ''}`
 }
+
+/**
+ * Select the best failure body for a non-zero-exit Bash command (#1231).
+ *
+ * Sources, in order of preference:
+ *   1. `accumulatedOutput` — the truncating accumulator's view after stripping
+ *      the synthetic "Exit code N" marker. Non-empty when the success-path
+ *      stdout was appended into the accumulator.
+ *   2. `resultStdout` — `ExecResult.stdout` from the shell runner.
+ *   3. `progressFullOutput` — the most recent `fullOutput` value yielded by
+ *      the streaming generator. Recovers stdout when the shell runner
+ *      streamed output through progress callbacks but the final result-slot
+ *      stdout was left empty (e.g. flush-after-result race, exit before EOF,
+ *      output persisted to a file path).
+ *
+ * Returns the first source whose trimmed length is non-zero, falling back to
+ * `''` when all three sources are empty.
+ */
+export function selectFailureOutput(
+  accumulatedOutput: string,
+  resultStdout: string | undefined,
+  progressFullOutput: string,
+): string {
+  if (accumulatedOutput.trim() !== '') return accumulatedOutput
+  if (resultStdout && resultStdout.trim() !== '') return resultStdout
+  if (progressFullOutput.trim() !== '') return progressFullOutput
+  return ''
+}