shell: finish the command when its buffered stdin is the last stdio to close

[robobun]

Problem

• await $`cmd < ${buffer}` , where the child exits without reading a redirect bigger than the pipe, sometimes never resolves (2 of 8 runs on bun 1.4.0), and the process does not exit either, since the interpreter still counts as pending activity. It hangs every time if something holds the child's stdin open for a moment after the child exits.
• A command running a subprocess is complete once it has an exit code and stdin, stdout and stderr have all closed. Those four events arrive as separate event-loop callbacks in no fixed order.
• Three of the four moved the command to Done when they completed it. The stdin close only recorded itself, so whenever it landed last, the command was complete and nothing finished it.
• It lands last often: the pending stdin write only fails with EPIPE once the read end of the pipe is gone, which is at the same time as, or after, the exit and the stdout/stderr EOFs.

Fix

• The stdin close now does what the stdout/stderr closes and the exit already did: all three end in one shared tail that marks the command Done when it is complete and returns the resumption for the caller to run.
• Correct because completion is a conjunction of four independently delivered events, so each must be able to finish the command; exactly one does, since the transition tears the subprocess down synchronously and the other events have already been delivered by then.
• Running that transition can free the subprocess, so the stdin-close and exit callbacks take a raw pointer instead of &mut self (the shape the stdout/stderr readers already had; the exit path had the same latent problem), and the shared pipe writer nulls its process backref before its single close notification.
• Verification: six new tests (Buffer and Blob redirects, a helper that exits or drains, the command inside a pipeline and on the left of ||) force the stdin close to be the last event. All six time out without the src/ changes and pass with them on bun 1.4.0 and a debug ASAN build; the existing shell, spawn and install-scanner tests still pass.

Background

• A < ${buffer} (or Blob or Response) redirect in the Bun shell is pumped into the child's stdin over a pipe by a StaticPipeWriter, a writer Bun.spawn and the install security scanner also use. It reports its close to its owning process through a raw backref.
• A shell Cmd that spawned a subprocess tracks the exit code plus a closed flag per piped stdio and is finished only when all are in, whoever holds the pipes: a helper that inherited the child's stdin keeps the stdin side open after the child is gone, and the shell keeps pumping to it.
• Shell callbacks do not advance the interpreter directly. They return a Yield (resume this node, or nothing) that the caller runs; running it can complete the parent node and free the Cmd and its subprocess, so nothing may still borrow either at that point.

---

no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/shell/bunshell.test.ts

Original description

Repro

import { $ } from "bun";
// The child exits without reading its stdin; the buffer is bigger than the pipe,
// so the shell's stdin write is still pending when it does.
await $`sh -c 'exit 0' < ${Buffer.alloc(1 << 20, "a")}`.quiet();

This never resolves on some runs (2 of 8 with bun 1.4.0 here, about 2 of 3 on a debug build). Whether it hangs depends on the order in which the event loop happens to deliver four events that all become ready when the child exits. This variant forces the problematic order and hangs every time: a helper keeps the child's stdin open (without reading it) for a moment after the child itself has exited.

await $`sh -c 'exec 3<&0; sleep 0.5 <&3 >/dev/null 2>&1 & exit 0' < ${Buffer.alloc(1 << 20, "a")}`.quiet();

Cause

A Cmd running a subprocess is complete once it has an exit code and every piped stdio has closed (Cmd::has_finished). For a Buffer/Blob/Response redirect, stdin is piped: the bytes are pumped into the child by a StaticPipeWriter, and BufferedIoClosed::stdin has to be marked closed too. Three of the four events that can complete the command checked has_finished() and moved the Cmd to Done (Cmd::on_exit, and Cmd::buffered_output_close for stdout and stderr). The fourth, Cmd::buffered_input_close (reached from the writer's close through ShellSubprocess::on_close_io and on_static_pipe_writer_done), only set the flag.

When the child exits without draining its stdin, the pending write fails with EPIPE only once the pipe's read end is gone, so the stdin close is processed at the same time as, or after, the exit and the stdout/stderr EOFs. Whenever it lands last, the command is complete and nothing transitions it. The promise never settles, and since the interpreter still counts as pending activity the process does not exit either (the script above has to be killed), even though the child and all of its pipes are gone by then.

Debug trace of a hanging run (`BUN_DEBUG_SHELL=1 BUN_DEBUG_SHELL_SUBPROC=1 BUN_DEBUG_StaticPipeWriter=1`)

[shell_subproc] onProcessExit(...)
[shell] cmd exit code=0 has_finished=false
[shell] cmd close buffered stderr
[shell] BufferedIOClosed all_closed=false stdin=false stdout=false stderr=true
[shell] cmd close buffered stdout
[shell] BufferedIOClosed all_closed=false stdin=false stdout=true stderr=true
[staticpipewriter] StaticPipeWriter(...) onError(err=EPIPE: Broken pipe (send()))
[staticpipewriter] StaticPipeWriter(...) onClose()
[shell_subproc] Subproc(...) onStaticPipeWriterDone(cmd=Node#2)
(nothing further)

Fix

• Cmd::buffered_input_close, buffered_output_close and on_exit all end in the same finish_if_done tail (has_finished() -> state = Done -> Yield::Next(this), including the existing "spawn has not returned yet" gate that transition_to_exec resumes from) and return the Yield to their caller. Previously only the output closes and the exit did this, and on_exit ran the Yield itself. buffered_input_close stays a no-op once deinit has taken exec, which is the only way it is reached during teardown (deinit_in_flight_io).
• The two ShellSubprocess callbacks that run those Yields, on_stdin_writer_close (replaces on_static_pipe_writer_done plus the stdin arm of on_close_io) and on_process_exit, take this: *mut Self and are forwarded raw by the StaticPipeWriterProcess shim and the link_impl_ProcessExit! thunk. Running the Yield can reach Cmd::deinit, which frees the subprocess and recycles the Cmd's arena slot, so no &mut to either may be on the stack at that point; this is the shape PipeReader::on_reader_done / finish_after_state_set already use for stdout/stderr, and it also removes the pre-existing instance of the problem in the exit path. on_stdin_writer_close empties the Writable::Buffer slot before signalling (the subprocess may be gone afterwards); dropping create()'s ref there cannot free the writer the callback is running inside of, because every path into StaticPipeWriter::on_close holds start()'s ref (or the one deinit_in_flight_io claims) until after the callback returns.
• on_close_io is left with the stdout/stderr reader path, its only remaining caller. Its old Writable::Pipe arm was unreachable: the shell never creates a FileSink stdin, and a FileSink reports its close through Writable::on_close, not this function.
• StaticPipeWriter (src/spawn/static_pipe_writer.rs, shared with Bun.spawn and the install security scanner) so far documented its process backref as outliving the writer. The shell's callback can now free the process from inside on_close_io and run the rest of the script while doing so; nothing in the writer used the backref after that call before either, but the trait doc now states the contract, and on_close nulls the field before the (single) dispatch so a future use after it faults in every impl instead of dangling only for the shell. The other two impls are unaffected (they get the same pointer they got before).

Why this shape: the completion condition is a conjunction of four independently delivered events, so each of them has to be able to perform the transition, and making the stdin close do what the other three already do is the whole fix. Exactly one transition happens per command in any interleaving: the transition tears the subprocess down synchronously, and the other events have by then already been delivered (they are what made has_finished() true). The alternative would be to close the stdin writer when the child exits, as Bun.spawn's Subprocess does for its buffer stdin; that would also end the hang, but it changes what < means: a process that inherited the child's stdin and outlives it would get a truncated redirect. Waiting for the writer keeps stdin consistent with how the shell already treats stdout/stderr capture (the command finishes when the pipes close, whoever holds them), and it is what BufferedIoClosed tracking stdin was evidently written for; the new tests pin that choice.

#37774 fixes a separate leak of the writer's start() ref on the same EPIPE path; it also edits StaticPipeWriter::on_close, so one of the two needs a trivial rebase, and they compose (it releases the ref after the dispatch). #37652 refactors the SubprocExec fields finish_if_done reads. #36895 adds ReadableStream stdin and calls the old on_static_pipe_writer_done from the exit handler; on top of this PR that call would need to move to the Cmd side, since the replacement runs the Yield. None of them covers this hang.

Tests

test/js/bun/shell/bunshell.test.ts, "stdin redirect still held open by a helper after the command's process has exited". The child (bun -e) spawns a detached helper that inherits its stdin, writes its own pid to a file, prints to stdout and stderr and exits 3 without reading anything. The test waits until that pid is gone (the exit and both EOFs are then processed, or queued ahead of anything the helper can still cause) and only then releases the helper through a file, so the stdin close is always the last event, on every platform and without a fixed delay. Every helper records what it did once released, so a helper that died early (which also fails the pending write) cannot pass as one that exited on cue; the redirect is 4 MiB so the write is still pending when the child exits whatever the socket buffer size. The command's environment drops BUN_FEATURE_FLAG_NO_ORPHANS, which the ASAN CI lanes set and under which the child kills the helper as it exits (that is what the first CI run of the draining cases showed). Cases, for a Buffer and a Blob redirect each:

• the helper exits without reading (the pending write fails): exit code, captured output and the helper's "released" record must come back;
• the helper drains the redirect after the child is gone: the command must still wait for it, and the helper must receive all 4 MiB (a close-at-exit implementation would pass the first group and fail this one; this also exercises the drained close rather than the error close);
• plus the exiting helper with the command inside a pipeline and as the left operand of ||, so the completion reaches a Pipeline and a Binary parent, not only a Stmt.

All six time out without the src/ changes and pass with them, both with and without BUN_FEATURE_FLAG_NO_ORPHANS=1 in the test process's environment (bun 1.4.0 and a debug ASAN build). Also passing on the debug ASAN build: the rest of bunshell.test.ts (422 tests), shell-worker-terminate-leak (covers deinit_in_flight_io closing the writer mid-flight), shell-hang, epipe, lazy, yield, pipeline_stack, bunshell-file, shelloutput, exec; for the other users of the writer, spawn.test.ts plus the stdin spawn tests both with memfd and with BUN_FEATURE_FLAG_DISABLE_MEMFD=1 (which forces Bun.spawn onto the pipe writer), and bun-install-security-provider.test.ts.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 7 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 79521487-66ef-49ab-bb7b-39bf1ea5f955

📥 Commits

Reviewing files that changed from the base of the PR and between 9a543cc and 5b16549.

📒 Files selected for processing (4)

• src/runtime/shell/states/Cmd.rs
• src/runtime/shell/subproc.rs
• src/spawn/static_pipe_writer.rs
• test/js/bun/shell/bunshell.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 5:05 PM PT - Aug 12th, 2026}

❌ @robobun, your commit 5b16549 has some failures in Build #93510 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 37799

That installs a local version of the PR into your bun-37799 executable, so you can run:

bun-37799 --bun

[robobun]

Status: ready for review. CI build 93510 on the current head (5b16549) finished with 179 of 181 jobs passed and no test failures; the other 2 are macOS test shards that Buildkite expired without ever getting an agent, so they would need a retry of just those two jobs. (The earlier build 93088, with the shell fix and the first version of the tests, passed all 181 jobs including macOS; the current tests have since run green on the Linux, ASAN and both Windows lanes.)

Reproduced with await $\sh -c 'exit 0' < ${Buffer.alloc(1 << 20, "a")}`.quiet()(hangs on a fraction of runs with bun 1.4.0 and a debug build) and deterministically with the variant in the description that keeps the child's stdin open briefly after the child exits. The six cases intest/js/bun/shell/bunshell.test.tstime out on a build without thesrc/changes and pass with them, with and withoutBUN_FEATURE_FLAG_NO_ORPHANS=1` (which the ASAN lanes set).

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 src/runtime/shell/states/Cmd.rs:75-76 — The doc comment on SubprocExec.interp still references ShellSubprocess::on_static_pipe_writer_done, which the follow-up commit c87211c removed and replaced with on_stdin_writer_close. The sibling doc on finish_if_done was updated to the new names (PipeReader::finish_after_state_set, on_stdin_writer_close / on_process_exit) — this one was just missed in the rename.

  Extended reasoning...

  What the issue is

  The doc comment on SubprocExec.interp at Cmd.rs:75-76 was updated by this PR's first commit to say the returned Yield is driven by "PipeReader::run_yield / ShellSubprocess::on_static_pipe_writer_done". The PR's second commit (c87211c) then removed on_static_pipe_writer_done entirely and replaced it with on_stdin_writer_close (to fix the &mut self-across-free UB flagged in the earlier review), but this doc comment was not updated in that rename.

  Step-by-step proof

  1. Grep the tree for on_static_pipe_writer_done: the only hit in src/ is this doc comment at Cmd.rs:76. No function by that name exists.
  2. In subproc.rs, the StaticPipeWriterProcess::on_close_io trait shim now forwards to Self::on_stdin_writer_close(this), and that function is what calls buffered_input_close() and drives y.run(&handle.interp).
  3. The sibling doc on finish_if_done (a few lines below in the same PR) correctly names the drivers as "PipeReader::finish_after_state_set, ShellSubprocess::on_stdin_writer_close / on_process_exit" — confirming the intended names and that this one comment was simply missed during the second commit's rename sweep.
  4. The other comment updates in this PR (e.g. Cmd.rs:886 "via stdin on_stdin_writer_close → buffered_input_close", subproc.rs:526 "re-enters on_stdin_writer_close") all use the new name.

  Also note PipeReader::run_yield is slightly imprecise for the same reason — the actual caller that drives the trampoline for stdout/stderr is PipeReader::finish_after_state_set (which calls run_yield_with), matching what the finish_if_done doc says.

  Why this matters

  Per REVIEW.md ("One source of truth; update every consumer atomically" — renames must grep the whole repo), a doc comment that names a removed function will send the next reader to grep for something that no longer exists. It's not a runtime defect, so it doesn't block merge, but since the PR itself introduced both the reference and the rename, it's cleanest to fix here.

  Fix

  Align this comment with the finish_if_done doc:

  /// `Yield::Next(this_id)` for the caller (`PipeReader::finish_after_state_set`,
  /// `ShellSubprocess::on_stdin_writer_close` / `on_process_exit`) to drive.
  

[robobun]

The stale on_static_pipe_writer_done reference in the SubprocExec.interp doc was already replaced in db06282 (the doc is now a single line pointing at finish_if_done); that review ran against c87211c. No references to the old name remain in the tree.

[claude]

I re-reviewed after c87211c/db06282 and confirmed the earlier &mut self-protector concern is addressed: on_stdin_writer_close and on_process_exit now take *mut Self end-to-end (the link_impl_ProcessExit! thunk binds this: *mut T, and the StaticPipeWriterProcess shim forwards raw), and the Cmd-side callbacks return their Yield so no &mut Cmd spans y.run(). No new issues found. Given this reworks subprocess-lifecycle teardown ordering with hand-reasoned refcount and Stacked-Borrows invariants, a maintainer look would still be worthwhile.

What was reviewed

• finish_if_done unification: same gate (null interp during spawn) preserved from the old buffered_output_close/on_exit, and transition_to_exec still resumes from state = Done.
• deinit_in_flight_io re-entry into on_stdin_writer_close: with exec already taken, buffered_input_close returns Yield::suspended() and run() on Suspended is a no-op; the writer's create/start refs are still released once each.
• Removed Writable::Pipe arm of on_close_io: POSIX Writable::init never returns Pipe, and Windows FileSink closes go through Writable::on_close — the arm was unreachable from StaticPipeWriter's callback.
• y.run(&handle.interp): ParentRef<Interpreter> implements Deref<Target=Interpreter>, matching Yield::run(&Interpreter).

Extended reasoning...

Overview

The PR fixes a hang where a shell command with a < ${buffer} stdin redirect never completes when the child exits without draining stdin and the stdin-close event happens to be delivered last. The fix makes Cmd::buffered_input_close a full completion path (mirroring buffered_output_close and on_exit) via a shared finish_if_done tail, and reshapes the ShellSubprocess callbacks that drive those Yields to take *mut Self so no &mut ShellSubprocess/&mut Cmd argument protector spans the trampoline that can synchronously free them. Three files: src/runtime/shell/states/Cmd.rs, src/runtime/shell/subproc.rs, and four new tests in test/js/bun/shell/bunshell.test.ts.

Security risks

None identified. This is internal event-ordering / lifecycle plumbing; no new user-controlled input parsing, no auth/crypto/permissions surface. The subprocess spawn path itself is unchanged.

Level of scrutiny

High. Per REVIEW.md this sits squarely in the most-blocked category (native memory safety, refcount balancing on every terminal path, self-freeing callbacks). The change hand-reasons about Stacked-Borrows protectors, RefPtr ref balances across create()/start(), and re-entrancy through deinit_in_flight_io. My earlier round found a real protector-UB issue that was fixed correctly, which itself signals this needs careful eyes. The PR description also flags rebase interactions with three other in-flight PRs (#37774, #37652, #36895), which a maintainer should coordinate.

Other factors

• The Stacked-Borrows fix I flagged was applied at both the stdin-close and process-exit paths (the latter being a pre-existing instance the PR now also cleans up). I verified link_impl_ProcessExit! binds this as *mut T (src/dispatch/lib.rs:33), so the raw forward is genuine.
• I traced the deinit → deinit_in_flight_io → on_stdin_writer_close re-entry: exec is taken to None before the close fires, so buffered_input_close short-circuits to Yield::suspended(), and Yield::run on Suspended with an empty pipeline stack returns immediately — no double transition or use-after-free there.
• The removed on_close_io stdin arms: Writable::Pipe is only produced on Windows and its close routes through Writable::on_close (via FileSink::source), and StaticPipeWriterProcess::on_close_io is only called for stdin — so the reduction to a stdout/stderr-only on_close_io plus a dedicated on_stdin_writer_close looks correct.
• Tests cover Buffer, Blob, pipeline parent, and || parent, forcing the stdin-close-last ordering deterministically. The PR reports they time out on 1.4.0 and pass on the fix, and that the broader shell suite passes on debug ASAN.
• All prior inline comments (mine and comment-cop) are resolved on the current head.

[claude]

I re-reviewed after ede94fd and 4372d29 — the earlier findings (the &mut self protector across y.run, the ASAN-lane test failure, and the failure-path cleanup) are all addressed, and this pass found nothing new. Given the scope — self-freeing callbacks through raw pointers in the shell subprocess lifecycle, plus a contract change to StaticPipeWriter shared with Bun.spawn and the security scanner — a human look is still worthwhile.

What was reviewed:

• on_stdin_writer_close / on_process_exit now take *mut Self end-to-end; verified no &mut ShellSubprocess or &mut Cmd protector spans y.run(&handle.interp), matching the PipeReader::on_reader_done shape.
• StaticPipeWriter::on_close nulls process before the single dispatch; checked the Windows tail (release_start_ref deref) only touches the writer, which write()'s +1 keeps alive past a freed process.
• buffered_input_close returning Yield::suspended() when exec is already taken keeps the deinit_in_flight_io re-entry a no-op.
• Test run() helper now releases the helper and joins the command in finally; the BUN_FEATURE_FLAG_NO_ORPHANS unset explains the earlier ASAN failure.

Extended reasoning...

Overview

Fixes a hang where await $cmd < ${buffer}`` never resolves when the child exits without draining stdin and the stdin-close event arrives after the exit and both output EOFs. Touches src/runtime/shell/states/Cmd.rs (unifies the four completion events into a shared `finish_if_done` tail), `src/runtime/shell/subproc.rs` (`on_stdin_writer_close` and `on_process_exit` take `*mut Self` and run the returned `Yield`; the stdin arm is split out of `on_close_io`), `src/spawn/static_pipe_writer.rs` (nulls the process backref before dispatching `on_close_io`, and documents that the impl may free it), and adds six tests to `bunshell.test.ts`.

Security risks

None identified. No new user-controlled input parsing; the change is control-flow ordering in an existing subprocess-lifecycle path. The StaticPipeWriter contract change is a tightening (null-before-dispatch) and does not weaken any check.

Level of scrutiny

High. This is memory-safety-sensitive unsafe Rust: raw *mut Self callbacks that may synchronously free their receiver, Stacked-Borrows argument-protector reasoning, and a shared component (StaticPipeWriter) used by Bun.spawn, the shell, and the install security scanner. The reasoning is careful and the shape mirrors the existing PipeReader::on_reader_done/finish_after_state_set pattern, but the class of bug (UAF via re-entrant teardown) is exactly what REVIEW.md flags as the most-blocked category, so it should not be auto-approved.

Other factors

• My two prior review rounds each found issues; all are resolved and the author's responses correctly diagnosed the ASAN-lane failure (BUN_FEATURE_FLAG_NO_ORPHANS set by scripts/runner.node.mjs, not load).
• The removed Writable::Pipe arm of on_close_io was verified unreachable per the PR description (shell never creates a FileSink stdin; Writable::init's POSIX Stdio::Pipe arm panics).
• CI: build 93088 (fix alone) was green; 93409 (current tests) green apart from github.com download failures; 93437 (HEAD, adds only test failure-path cleanup) did not compile due to the same infra outage. No test signal against HEAD itself, though the delta from 93409 is test-only cleanup.
• The PR description names three overlapping open PRs (#37774, #37652, #36895) that will need coordination on merge.

[claude]

I reviewed this PR and didn't find new issues; the earlier concerns (the &mut self protector spanning y.run(), the ASAN-lane test failure under BUN_FEATURE_FLAG_NO_ORPHANS, and the failure-path helper cleanup) are addressed in the current revision. Given the amount of raw-pointer / self-freeing-callback reasoning across Cmd, ShellSubprocess, and the shared StaticPipeWriter, a human look is still worthwhile.

What was reviewed:

• on_stdin_writer_close / on_process_exit now take *mut Self end-to-end and the trait shim / exit thunk forward raw, so no &mut ShellSubprocess or &mut Cmd protector spans the trampoline that can reach Cmd::deinit.
• StaticPipeWriter::on_close nulls process before the single dispatch; the writer's own &mut self is kept live by start()'s ref (or the one deinit_in_flight_io claims) across the callback, so freeing the process there doesn't free the writer under it.
• The deinit re-entry via buffered_input_close after exec is taken hits the Exec::None arm and returns Yield::suspended() — no double transition.
• The other two StaticPipeWriterProcess impls (Bun.spawn's Subprocess, the install security scanner) receive the same pointer they got before; the trait's changed contract only widens what an impl may do.

Extended reasoning...

Overview

This PR fixes a hang in Bun's shell where await $cmd < ${largeBuffer}`` never resolves when the stdin-close event happens to arrive after the process exit and both output EOFs. The fix touches four files: src/runtime/shell/states/Cmd.rs (unifies the four completion-event tails into a shared `finish_if_done`; `buffered_input_close` now returns a `Yield` like the other three), `src/runtime/shell/subproc.rs` (`on_stdin_writer_close` replaces `on_static_pipe_writer_done` + the stdin arm of `on_close_io`; both it and `on_process_exit` take `*mut Self` and are forwarded raw so no `&mut` protector spans the trampoline that can free the subprocess), `src/spawn/static_pipe_writer.rs` (trait doc now states the impl may free the process; `on_close` nulls the backref before dispatch), and `test/js/bun/shell/bunshell.test.ts` (six new tests forcing stdin-close-last, covering Buffer/Blob × exiting/draining helper, plus pipeline and `||` parents).

Security risks

None identified. The change is internal lifecycle plumbing in the shell's subprocess state machine. No new user-controlled input parsing, no auth/crypto/permission surface. The StaticPipeWriter trait contract change is shared with Bun.spawn and the install security scanner, but those impls are unaffected (they get the same pointer they got before and don't free themselves from the callback).

Level of scrutiny

High. This is exactly the category REVIEW.md flags as most-blocked: raw *mut Self callbacks that may synchronously free their receiver, intrusive refcount balancing across three owners (the writer's create() ref, start() ref, and the one deinit_in_flight_io claims), and Stacked Borrows protector reasoning that already required one round of correction on this PR. The finish_if_done refactor is straightforward, but the surrounding pointer discipline (CmdHandle copy-out before the borrow, the exec-taken re-entry guard in buffered_input_close, the interp.is_null() spawn-frame gate) is load-bearing and easy to get subtly wrong. The six new tests are well-constructed (each helper records what it did so an early death can't pass; try/finally releases the helper and joins the command on the failure path; BUN_FEATURE_FLAG_NO_ORPHANS is dropped from the command's env), but they exercise a deliberately-forced ordering, so the memory-safety argument still rests on the code review.

Other factors

I reviewed this PR twice previously with concrete findings; all three were addressed (c87211c for the protector issue, ede94fd for the ASAN-lane orphan-kill, 4372d29 for the failure-path cleanup). The comment-cop nags were also resolved. The bug hunting system found nothing this run. CI on the current head (5b16549, an empty retrigger) is still building. The PR description is unusually thorough about the mechanism, the alternatives considered, and the interaction with three overlapping open PRs (#37774, #37652, #36895), which a human reviewer will want to weigh.

[robobun]

This came up again from the other direction (a child that exits without reading < ${buffer} intermittently never settles; on a debug build it hangs on the first run, because the child is a zombie before spawn_async registers its polls, so the pidfd is dispatched ahead of the writer's EPIPE). Same cause and same fix as here, so no second PR. For reference the branch is farm/2c5eb7b5/shell-buffer-stdin-close-finishes-cmd: buffered_input_close shares on_exit's finish-if-done tail, the subprocess empties its stdin slot before notifying the Cmd, the stdin arm of on_close_io goes away, plus two fixture tests in epipe.test.ts (plain command and pipeline).

Two things about this PR's current state:

• It now conflicts with main in src/spawn/static_pipe_writer.rs: spawn: release the buffer-stdin writer's start() ref when its write fails on POSIX #37774 merged and changed the same on_close hunk (it releases start()'s ref after the on_close_io dispatch). The branch above is written on top of spawn: release the buffer-stdin writer's start() ref when its write fails on POSIX #37774 if that helps with the rebase; the two changes compose, the dispatch just has to stay the last use of process.
• In build 93510 only the darwin test lanes are red, and they show as "Expired" (never picked up), not as failures; every other lane passed.