shell: support ReadableStream stdin redirect via SinkHandle - #36895
Open
robobun wants to merge 11 commits into
Open
shell: support ReadableStream stdin redirect via SinkHandle#36895robobun wants to merge 11 commits into
robobun wants to merge 11 commits into
Claude / Claude Code Review
completed
Aug 4, 2026 in 24m 2s
Code review found 1 important issue
Found 2 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/runtime/shell/subproc.rs:723-730 |
Spawned child process leaked on WritableInitError::Sys |
| 🟡 Nit | src/runtime/shell/subproc.rs:720-722 |
Windows ReadableStream stdin start failure still panics |
Annotations
Check failure on line 730 in src/runtime/shell/subproc.rs
claude / Claude Code Review
Spawned child process leaked on WritableInitError::Sys
The `WritableInitError::Sys` arm closes the stdout/stderr pipe fds but leaks the already-spawned child process (and, on Linux, its pidfd): `spawn_result.to_process()` hasn't run yet and `PosixSpawnResult` has no `Drop`, so returning here drops the pid/pidfd on the floor while the child keeps running. Every other post-spawn error path in this function kills and reaps the child; the CodeRabbit comment that prompted cada105 explicitly asked to "tear down the spawned child", and only the pipe-fd hal
Check warning on line 722 in src/runtime/shell/subproc.rs
claude / Claude Code Review
Windows ReadableStream stdin start failure still panics
The Windows `Stdio::Pipe | Stdio::ReadableStream(_)` arm of `Writable::init` still returns `WritableInitError::UnexpectedCreatingStdin` (→ `panic!` here) when `start_with_current_pipe()` fails — cada105 added `WritableInitError::Sys` but only wired it into the POSIX arm. Before this PR that Windows branch was dead for shell stdin; now `cmd < ${stream}` makes it user-reachable, so it should return `Sys(e)` too (REVIEW.md: user-reachable failures are recoverable errors, and POSIX/Windows branches
Loading