Repository navigation
fix(minimal): exit quietly when version output hits a closed pipe - #1894
norrietaylor merged 2 commits into
Conversation
min version | head -1 panicked with a broken-pipe error (exit 101) because cmd_version wrote to stdout via println!, which panics on EPIPE since Rust ignores SIGPIPE by default. Route cmd_version's output through an injected writer so a closed reader surfaces as an io::Error, and classify BrokenPipe in run() to exit quietly with 141 (128 + SIGPIPE) instead of printing an error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reached
This review includes 3 billable files and costs up to $0.75. Or wait 55 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe version command now writes its output to a supplied writer. The CLI passes locked stdout and exits quietly with code 141 when an error chain contains a broken-pipe error. ChangesCLI output handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Piped version output is intended to stop quietly when the reader closes the pipe. The supplied change context shows that write failures reach the quiet broken-pipe handler, with no actionable merge risk identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
A rabbit checks the output stream, Comment |
Pass the unlocked stdout handle so the lock is not held across the daemon handshake, exit quietly on a broken pipe only for `min version`, and test that cmd_version surfaces a closed writer as a broken pipe.
|
/review |
|
Approved: no findings at Nothing to do. Clobber: none: |
There was a problem hiding this comment.
Approved at 6545303: minimal-review approved at head, 0 open threads, all lanes green.
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABUywAcw
min version | head -1panicked withfailed printing to stdout: Broken pipe(exit 101). Rust ignores SIGPIPE, soprintln!panics on EPIPE:cmd_versionprints the client line, then waits on theGetVersionRPC, then prints the server/stdlib lines — by which timeheadhas exited.Fix: route
cmd_versionoutput through an injected writer (writeln!with?), so a closed reader surfaces as anio::Error; classifyBrokenPipeinrun()and exit quietly with 141 (128 + SIGPIPE) instead of printing an error. (informed by ADR-0001)Verification
cargo fmt --all --check— passedcargo build --workspace --locked— passedcargo clippy -p minimal --no-deps --locked -- -D warnings— passedcargo test -p minimal --locked --test cli— 43 passedcargo test -p minimal --locked --bin min broken_pipe— passedcargo run --bin min --locked -- version 2>/dev/null | head -1— prints client line, exit 0Note: full-workspace
cargo clippy --workspace --locked -- -D warningsfails on pre-existing code incrates/args/src/lib.rs(lintneedless_borrows_for_generic_args, new in stable 1.99.0; repo pins 1.97.0), andcargo test --workspace --lockedhas one pre-existing failure intests::legacy_network_spellings_parse_with_hint(stderr capture returns empty). Neither file is touched by this change.Summary by CodeRabbit