From 7097e4fb506ea93e63be4dd8dde9604e424da487 Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Wed, 1 Jul 2026 15:23:09 +0800
Subject: [PATCH] docs(bash-local): clarify the stdin error handler swallows
 ANY write error, not just EPIPE
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review noted the handler's comment said "EPIPE" while the code swallowed every
stdin 'error'. Swallowing any stdin-write error IS correct here — the write is
best-effort and the command's authoritative outcome is its exit code + captured
output (reported by the `close` handler regardless of whether the write landed).
A rare non-EPIPE pipe fault means the command ran with incomplete stdin, which
it surfaces itself via its own exit/output; rejecting `done` would instead
discard that real output and turn it into an opaque infrastructure error. Widen
the comment to state this rather than implying only EPIPE is caught. No behavior
change.
---
 packages/bash/bash-local/src/run.ts | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/packages/bash/bash-local/src/run.ts b/packages/bash/bash-local/src/run.ts
index 4b043e3fe3..5b67bbdc1b 100644
--- a/packages/bash/bash-local/src/run.ts
+++ b/packages/bash/bash-local/src/run.ts
@@ -308,12 +308,6 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
     detached: true,
   })
 
-  // A child that exits without reading stdin makes the write error EPIPE —
-  // swallow it (the command's outcome rides on its exit code/output, not the
-  // stdin write) so it never crashes the host or rejects `done`.
-  child.stdin.on('error', () => { /* EPIPE: child closed stdin early; outcome rides on exit. */ })
-  child.stdin.end(spec.stdin ?? '')
-
   const stdout = new OutputCollector(spec.maxOutputBytes, 'stdout', spillDir)
   const stderr = new OutputCollector(spec.maxOutputBytes, 'stderr', spillDir)
   child.stdout.on('data', (chunk: Buffer) => { stdout.push(chunk) })
@@ -347,6 +341,20 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
   }
   spec.signal?.addEventListener('abort', onAbort, { once: true })
 
+  // Write stdin and close it. This handler must exist: an unhandled 'error' on
+  // the stream would throw and crash the host. We swallow the error rather than
+  // reject `done`, and that is correct for ANY stdin-write error, not just the
+  // common one — the stdin write is BEST-EFFORT, while the command's authoritative
+  // outcome is its exit code + captured output, which the `close` handler reports
+  // regardless of whether the write landed. The expected case is EPIPE (the child
+  // exited without reading, so closing our end of a still-full pipe fails); a rare
+  // non-EPIPE pipe fault means the command ran with incomplete stdin, and it
+  // surfaces that itself through its own exit/output (e.g. a hook that gets
+  // truncated JSON errors out) — rejecting here would instead discard that real
+  // output and turn it into an opaque infrastructure error, which is worse.
+  child.stdin.on('error', () => { /* stdin write is best-effort; outcome rides on exit/output. */ })
+  child.stdin.end(spec.stdin ?? '')
+
   const done = new Promise<SpawnOutcome>((resolve, reject) => {
     child.on('error', (error) => {
       // Spawn-level failure (ENOENT cwd, EACCES, …): no close event with