From 451eb9842c7bb0b6cdf6a38cdb5795b63ecc2e10 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 3 Aug 2026 18:53:14 +0200 Subject: [PATCH] fix: mapped tail builtin broken pipe to silent exit 141 - Handled broken pipe errors across tail output and follow paths by translating them to a silent SIGPIPE exit code. - Prevented stderr noise when downstream pipeline readers exit early, matching native tail behavior. --- crates/pi-shell/src/shell.rs | 13 ++-- crates/vendor/uu-tail/src/chunks.rs | 24 ++++---- crates/vendor/uu-tail/src/follow/files.rs | 6 +- crates/vendor/uu-tail/src/tail.rs | 72 +++++++++++++++++++---- packages/coding-agent/CHANGELOG.md | 5 ++ packages/natives/CHANGELOG.md | 4 ++ 6 files changed, 92 insertions(+), 32 deletions(-) diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index ca28b1d72..cd559502a 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -6622,21 +6622,22 @@ mod tests { /// Regression: builtin tail upstream of an early-exiting consumer printed /// "tail: Broken pipe" and failed (`tail -c N big.jsonl | jq …` with jq /// aborting on a parse error). Real tail dies silently from SIGPIPE; the - /// builtin must exit 141 with no diagnostic, leaving the pipeline status - /// to the last stage. + /// builtin must exit 141 with no diagnostic. `pipefail` exposes tail's own + /// status, so rc=141 proves the broken pipe actually fired (head closed + /// the pipe early) and was mapped to the silent SIGPIPE exit, not 1. #[tokio::test(flavor = "multi_thread")] async fn tail_builtin_is_silent_when_downstream_closes_pipe() { let dir = tempfile::tempdir().expect("tempdir"); let file = dir.path().join("big.txt"); - // ~589 KiB: forces the seekable bounded_tail path and overflows the OS - // pipe buffer so tail is still writing when head exits. + // ~589 KiB: forces the seekable bounded_tail path, and the 400 KB tail + // overflows the OS pipe buffer so tail is still writing when head exits. let command = format!( - "seq 1 100000 > '{file}'; tail -c 400000 '{file}' | head -c 10 > /dev/null; echo rc=$?", + "seq 1 100000 > '{file}'; set -o pipefail; tail -c 400000 '{file}' | head -c 10 > /dev/null; echo rc=$?", file = file.display() ); let (result, output) = execute_captured(command).await; assert_eq!(result.exit_code, Some(0)); - assert!(output.contains("rc=0"), "{output:?}"); + assert!(output.contains("rc=141"), "{output:?}"); assert!(!output.contains("Broken pipe"), "{output:?}"); } diff --git a/crates/vendor/uu-tail/src/chunks.rs b/crates/vendor/uu-tail/src/chunks.rs index 6df075f08..0f3bca3d8 100644 --- a/crates/vendor/uu-tail/src/chunks.rs +++ b/crates/vendor/uu-tail/src/chunks.rs @@ -15,11 +15,9 @@ use std::{ collections::VecDeque, fs::File, - io::{BufRead, Read, Seek, SeekFrom, Write}, + io::{self, BufRead, Read, Seek, SeekFrom, Write}, }; -use uucore::error::UResult; - /// When reading files in reverse in `bounded_tail`, this is the size of each /// block read at a time. pub const BLOCK_SIZE: u64 = 1 << 16; @@ -209,10 +207,10 @@ impl BytesChunk { /// Fills `self.buffer` with maximal [`BUFFER_SIZE`] number of bytes, /// draining the reader by that number of bytes. If EOF is reached (so 0 - /// bytes are read), it returns [`UResult`]; otherwise, it returns - /// [`UResult`], where bytes is the number of bytes read from + /// bytes are read), it returns `Ok(None)`; otherwise, it returns + /// `Ok(Some(bytes))`, where bytes is the number of bytes read from /// the source. - pub fn fill(&mut self, filehandle: &mut impl BufRead) -> UResult> { + pub fn fill(&mut self, filehandle: &mut impl BufRead) -> io::Result> { let num_bytes = filehandle.read(&mut self.buffer)?; self.bytes = num_bytes; if num_bytes == 0 { @@ -286,7 +284,7 @@ impl BytesChunkBuffer { /// let mut chunks = BytesChunkBuffer::new(num_print); /// chunks.fill(&mut reader).unwrap(); /// ``` - pub fn fill(&mut self, reader: &mut impl BufRead) -> UResult<()> { + pub fn fill(&mut self, reader: &mut impl BufRead) -> io::Result<()> { let mut chunk = Box::new(BytesChunk::new()); // fill chunks with all bytes from reader and reuse already instantiated chunks @@ -323,7 +321,7 @@ impl BytesChunkBuffer { Ok(()) } - pub fn print(&self, writer: &mut impl Write) -> UResult<()> { + pub fn print(&self, writer: &mut impl Write) -> io::Result<()> { for chunk in &self.chunks { writer.write_all(chunk.get_buffer())?; } @@ -459,7 +457,7 @@ impl LinesChunk { /// the [`BytesChunk::fill`] function besides that this function also counts /// and stores the number of lines encountered while reading from /// the `filehandle`. - pub fn fill(&mut self, filehandle: &mut impl BufRead) -> UResult> { + pub fn fill(&mut self, filehandle: &mut impl BufRead) -> io::Result> { match self.chunk.fill(filehandle)? { None => { self.lines = 0; @@ -517,7 +515,7 @@ impl LinesChunk { /// /// * `writer`: must implement [`Write`] /// * `offset`: An offset in number of lines. - pub fn write_lines(&self, writer: &mut impl Write, offset: usize) -> UResult<()> { + pub fn write_lines(&self, writer: &mut impl Write, offset: usize) -> io::Result<()> { self.write_bytes(writer, self.calculate_bytes_offset_from(offset)) } @@ -528,7 +526,7 @@ impl LinesChunk { /// /// * `writer`: must implement [`Write`] /// * `offset`: An offset in number of bytes. - pub fn write_bytes(&self, writer: &mut impl Write, offset: usize) -> UResult<()> { + pub fn write_bytes(&self, writer: &mut impl Write, offset: usize) -> io::Result<()> { writer.write_all(self.get_buffer_with(offset))?; Ok(()) } @@ -565,7 +563,7 @@ impl LinesChunkBuffer { /// chunks. If there are no chunks, for example because the piped stdin /// contained no lines, or `num_print = 0` then `iterator.next` will return /// None. - pub fn fill(&mut self, reader: &mut impl BufRead) -> UResult<()> { + pub fn fill(&mut self, reader: &mut impl BufRead) -> io::Result<()> { let mut chunk = Box::new(LinesChunk::new(self.delimiter)); while chunk.fill(reader)?.is_some() { @@ -620,7 +618,7 @@ impl LinesChunkBuffer { Ok(()) } - pub fn write(&self, mut writer: impl Write) -> UResult<()> { + pub fn write(&self, mut writer: impl Write) -> io::Result<()> { for chunk in &self.chunks { chunk.write_bytes(&mut writer, 0)?; } diff --git a/crates/vendor/uu-tail/src/follow/files.rs b/crates/vendor/uu-tail/src/follow/files.rs index 8c011aaa9..6c5f3636c 100644 --- a/crates/vendor/uu-tail/src/follow/files.rs +++ b/crates/vendor/uu-tail/src/follow/files.rs @@ -154,9 +154,11 @@ impl FileHandling { self.header_printer.print(display_name.as_str()); } + // pi-uutils: a closed downstream reader must end the follow loop + // silently (SIGPIPE semantics), not as a printed tail error. let mut writer = BufWriter::new(pi_uutils_ctx::stdout().lock()); - chunks.print(&mut writer)?; - writer.flush()?; + chunks.print(&mut writer).map_err(crate::map_output_error)?; + writer.flush().map_err(crate::map_output_error)?; self.last.replace(path.to_owned()); self.update_metadata(path, None); diff --git a/crates/vendor/uu-tail/src/tail.rs b/crates/vendor/uu-tail/src/tail.rs index 4b5c596c4..9dcda9394 100644 --- a/crates/vendor/uu-tail/src/tail.rs +++ b/crates/vendor/uu-tail/src/tail.rs @@ -123,6 +123,43 @@ fn rewrite_bsd_invocation(argv: &[OsString]) -> Option, Str Some(Ok(tac_argv)) } +/// Exit status of a process killed by SIGPIPE (128 + 13). +const SIGPIPE_EXIT_CODE: i32 = 141; + +/// pi-uutils: marker for a closed stdout pipe. Real tail is killed silently by +/// SIGPIPE when the downstream reader exits early (`tail file | head`); the +/// in-process builtin cannot die by signal, so output errors map to this +/// sentinel and [`run`] turns it into a bare exit 141 with no diagnostic, +/// mirroring the fd builtin's convention. +#[derive(Debug)] +struct BrokenPipeExit; + +impl std::fmt::Display for BrokenPipeExit { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("Broken pipe") + } +} + +impl std::error::Error for BrokenPipeExit {} + +impl UError for BrokenPipeExit { + fn code(&self) -> i32 { + SIGPIPE_EXIT_CODE + } +} + +/// Maps a stdout write error into the tail exit contract: `BrokenPipe` +/// becomes the silent [`BrokenPipeExit`] sentinel, everything else the +/// standard uucore io wrapper. Used by every path that copies data to the +/// context stdout, including follow mode. +pub(crate) fn map_output_error(err: io::Error) -> Box { + if err.kind() == ErrorKind::BrokenPipe { + Box::new(BrokenPipeExit) + } else { + err.into() + } +} + /// In-process builtin entry point. Unlike upstream's `#[uucore::main] uumain`, /// this renders clap help/usage/version to the context streams and never calls /// `std::process::exit`, so it is safe inside the long-lived host shell @@ -160,6 +197,11 @@ pub fn run(args: Vec) -> i32 { Ok(()) => pi_uutils_ctx::exit_code(), Err(err) => { let code = err.code(); + if code == SIGPIPE_EXIT_CODE { + // Downstream closed the pipe; exit silently like SIGPIPE would + // kill a real tail (see `BrokenPipeExit`). + return SIGPIPE_EXIT_CODE; + } let _ = writeln!(pi_uutils_ctx::stderr(), "tail: {err}"); if code == 0 { 1 } else { code } }, @@ -299,11 +341,11 @@ fn tail_file( && file.is_seekable(if input.is_stdin() { offset } else { 0 }) && (!st.is_file() || st.len() > blksize_limit) { - bounded_tail(&mut file, settings)?; + bounded_tail(&mut file, settings).map_err(map_output_error)?; reader = BufReader::new(file); } else { reader = BufReader::new(file); - unbounded_tail(&mut reader, settings)?; + unbounded_tail(&mut reader, settings).map_err(map_output_error)?; } if input.is_tailable() { observer.add_path( @@ -391,7 +433,7 @@ fn tail_stdin( // streaming (pipe) path. header_printer.print_input(input); let mut reader = BufReader::new(pi_uutils_ctx::stdin()); - unbounded_tail(&mut reader, settings)?; + unbounded_tail(&mut reader, settings).map_err(map_output_error)?; Ok(()) } @@ -531,7 +573,7 @@ fn backwards_thru_file(file: &mut File, num_delimiters: u64, delimiter: u8) { /// end of the file, and then read the file "backwards" in blocks of size /// `BLOCK_SIZE` until we find the location of the first line/byte. This ends up /// being a nice performance win for very large files. -fn bounded_tail(file: &mut File, settings: &Settings) -> UResult<()> { +fn bounded_tail(file: &mut File, settings: &Settings) -> io::Result<()> { debug_assert!(!settings.presume_input_pipe); let mut limit = None; @@ -568,7 +610,7 @@ fn bounded_tail(file: &mut File, settings: &Settings) -> UResult<()> { Ok(()) } -fn unbounded_tail(reader: &mut BufReader, settings: &Settings) -> UResult<()> { +fn unbounded_tail(reader: &mut BufReader, settings: &Settings) -> io::Result<()> { let mut writer = BufWriter::new(pi_uutils_ctx::stdout().lock()); match &settings.mode { FilterMode::Lines(Signum::Negative(count), sep) => { @@ -636,8 +678,8 @@ fn unbounded_tail(reader: &mut BufReader, settings: &Settings) -> UR // pi-uutils: upstream emulates Unix SIGPIPE on Windows by calling // `std::process::exit(13)` on a broken-pipe flush. That would kill the // long-lived host shell process. An in-process builtin must never - // `process::exit`; let the broken pipe surface as a normal `io::Error` and - // propagate to the caller, matching every other pi-uutils builtin. + // `process::exit`; the broken pipe surfaces as a normal `io::Error`, which + // callers map to the silent SIGPIPE exit via `map_output_error`. writer.flush()?; Ok(()) } @@ -834,12 +876,13 @@ mod tests { file.flush().expect("flush"); drop(file); + let stderr_buf = Arc::new(Mutex::new(Vec::new())); let io = pi_uutils_ctx::ScopeIo { stdin: Box::new(io::empty()), stdin_fd: None, stdin_is_search_input: false, stdout: Box::new(BrokenPipeWriter), - stderr: Box::new(io::sink()), + stderr: Box::new(SharedWriter { buf: stderr_buf.clone() }), cwd: std::env::temp_dir(), env: HashMap::new(), cancel: Arc::new(AtomicBool::new(false)), @@ -849,7 +892,10 @@ mod tests { crate::run(vec![OsString::from("tail"), OsString::from(&path)]) }); - assert_ne!(code, 0, "broken pipe must surface as a non-zero exit, not a panic"); + // Real tail dies silently from SIGPIPE (exit 128+13); the in-process + // builtin must match: no "tail: Broken pipe" noise on stderr. + assert_eq!(code, 141, "broken pipe must map to the silent SIGPIPE exit"); + assert_eq!(String::from_utf8(stderr_buf.lock().clone()).unwrap(), ""); } #[test] @@ -877,12 +923,13 @@ mod tests { } let input = b"1\n2\n3\n4\n5\n".to_vec(); + let stderr_buf = Arc::new(Mutex::new(Vec::new())); let io = pi_uutils_ctx::ScopeIo { stdin: Box::new(Cursor::new(input)), stdin_fd: None, stdin_is_search_input: false, stdout: Box::new(BrokenPipeWriter), - stderr: Box::new(io::sink()), + stderr: Box::new(SharedWriter { buf: stderr_buf.clone() }), cwd: std::env::temp_dir(), env: HashMap::new(), cancel: Arc::new(AtomicBool::new(false)), @@ -892,6 +939,9 @@ mod tests { crate::run(vec![OsString::from("tail"), OsString::from("-n"), OsString::from("3")]) }); - assert_ne!(code, 0, "broken pipe must surface as a non-zero exit, not process::exit"); + // Same silent-SIGPIPE contract as the bounded test: exit 141, no + // diagnostic — this is the `tail -c N file | jq | …` regression. + assert_eq!(code, 141, "broken pipe must map to the silent SIGPIPE exit"); + assert_eq!(String::from_utf8(stderr_buf.lock().clone()).unwrap(), ""); } } diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 46d565c01..73e700ce6 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,11 @@ ## [Unreleased] +### Fixed + +- Fixed the built-in `tail` printing `tail: Broken pipe` and failing when a downstream pipeline reader exited early (e.g. `tail -c N file.jsonl | jq …` with jq aborting on a parse error); it now exits silently with 141 (128+SIGPIPE) like a real tail, in every output path including `--follow`. +- Fixed the in-process ps shell builtin rejecting common procps/BSD format specifiers (`ps -o tpgid,...` failed with `unknown output format specifier`); added `tpgid`, `pri`, `flags`, real/effective user and group columns, `wchan`, fault counters, `sz`, and the STAT `+` foreground flag. + ## [17.2.6] - 2026-08-03 ### Added diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index 7ae3337bb..af31ff2c7 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added the missing procps/BSD output format specifiers to the in-process ps shell builtin: `tpgid`, `pri`, `f`/`flags`, `ruser`/`logname`, `ruid`, `rgroup`, `rgid`, `group`/`egroup`, `gid`/`egid`, `wchan`, `min_flt`/`maj_flt`, `times`/`cputimes`, `sz`, single-character `s`/`state`, and aliases `tgid`/`tid`/`spid`, `euser`, `bsdtime`, and `rsz`. `ps -j` now includes a TPGID column, `ps -l` prints the single-character S column, and STAT gains the `+` foreground flag for processes in their terminal's foreground process group. + ## [17.2.6] - 2026-08-03 ### Added