From 82be8ea41a5411e3ada9f744129edf66c6008c10 Mon Sep 17 00:00:00 2001 From: argus Date: Sun, 11 Oct 2026 10:14:17 +0900 Subject: [PATCH 1/5] fix(shell): decode UTF-8 in the read builtin (Fixes #6543) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The vendored read builtin turned each input byte into a char, so every non-ASCII byte became its own Latin-1 character: `café` read as `café`, and `read -n 1` split a multibyte character in half. Port the oh-my-pi fix for this, as triage on #6543 suggested: - f3ce813d: read and mapfile take input through a shared LineInput that reads regular files in blocks and seeks unread bytes back on drop - b636b6ae: mapfile gives read-ahead back before each -C callback - b6e8ee12: mapfile refuses a readonly array before reading - 68fb9863: read -n/-N count characters, not bytes read.rs, mapfile.rs and the new line_input.rs match oh-my-pi at 68fb9863, apart from the gajae-code header and existing local comments. read keeps its cancellation, -t timeout, and -t 0 readiness guards. rustix is added for the FIONREAD readiness count upstream uses, and tempfile as a pi-shell dev-dependency for the ported tests. Confidence: high Scope-risk: moderate Tested: cargo test -p pi-shell --lib (232 passed, 7 new); brush-builtins read unit tests (22 passed); bun run check:rs; issue repro through the native executeShell binding Not-tested: Windows and Linux locally Co-authored-by: Claude --- Cargo.lock | 2 + crates/brush-builtins-vendored/Cargo.toml | 5 + crates/brush-builtins-vendored/src/lib.rs | 2 + .../brush-builtins-vendored/src/line_input.rs | 115 ++++++++++ crates/brush-builtins-vendored/src/mapfile.rs | 57 +++-- crates/brush-builtins-vendored/src/read.rs | 206 +++++++++++------- crates/pi-shell/Cargo.toml | 3 + crates/pi-shell/src/shell.rs | 122 +++++++++++ docs/rust-porting-inventory.md | 2 +- .../6543-bash-read-decodes-utf8.md | 8 + 10 files changed, 421 insertions(+), 101 deletions(-) create mode 100644 crates/brush-builtins-vendored/src/line_input.rs create mode 100644 packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md diff --git a/Cargo.lock b/Cargo.lock index bfe05bab007..480ccf85ab1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -491,6 +491,7 @@ dependencies = [ "nix 0.31.3", "procfs", "rlimit", + "rustix", "strum", "strum_macros", "thiserror 2.0.18", @@ -3192,6 +3193,7 @@ dependencies = [ "serde_json", "sha2 0.10.9", "smallvec", + "tempfile", "tokio", "tokio-util", "toml", diff --git a/crates/brush-builtins-vendored/Cargo.toml b/crates/brush-builtins-vendored/Cargo.toml index 82e3056eb55..3e2aec59d5c 100644 --- a/crates/brush-builtins-vendored/Cargo.toml +++ b/crates/brush-builtins-vendored/Cargo.toml @@ -241,6 +241,11 @@ features = [ [target."cfg(unix)".dependencies.rlimit] version = "0.11.0" +[target."cfg(unix)".dependencies.rustix] +version = "1.1.4" +features = ["std"] +default-features = false + [lints.clippy] bool_to_int_with_if = "allow" cognitive_complexity = "allow" diff --git a/crates/brush-builtins-vendored/src/lib.rs b/crates/brush-builtins-vendored/src/lib.rs index fd25e5b2d69..9125c0e5d76 100644 --- a/crates/brush-builtins-vendored/src/lib.rs +++ b/crates/brush-builtins-vendored/src/lib.rs @@ -60,6 +60,8 @@ mod jobs; mod kill; #[cfg(feature = "builtin.let")] mod let_; +#[cfg(any(feature = "builtin.mapfile", feature = "builtin.read"))] +mod line_input; #[cfg(feature = "builtin.mapfile")] mod mapfile; #[cfg(feature = "builtin.popd")] diff --git a/crates/brush-builtins-vendored/src/line_input.rs b/crates/brush-builtins-vendored/src/line_input.rs new file mode 100644 index 00000000000..219019d9a87 --- /dev/null +++ b/crates/brush-builtins-vendored/src/line_input.rs @@ -0,0 +1,115 @@ +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/line_input.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: no. +//! Byte input for the line-reading builtins (`read`, `mapfile`). + +use std::io::{self, Read, Seek, SeekFrom}; + +use brush_core::openfiles::OpenFile; + +/// Bytes per read-ahead. +const BLOCK: usize = 4096; + +/// ASCII control character for Ctrl+C (ETX - End of Text). +pub(crate) const CTRL_C: u8 = 0x03; +/// ASCII control character for Ctrl+D (EOT - End of Transmission). +pub(crate) const CTRL_D: u8 = 0x04; + +/// The bytes of a read line as text: UTF-8 decoded once per line, invalid +/// sequences replaced rather than each byte read as a Latin-1 character. +pub(crate) fn decode_line(line: Vec) -> String { + String::from_utf8(line).unwrap_or_else(|error| String::from_utf8_lossy(error.as_bytes()).into_owned()) +} + +/// When a [`LineInput`] may read past the bytes it hands out. +#[derive(Clone, Copy)] +pub(crate) enum ReadAhead { + /// The caller consumes everything up to EOF, so reading ahead can never + /// take input meant for someone else. + ToEof, + /// The caller may stop mid-input: read ahead only from a regular file, + /// whose offset can be given back. + Seekable, +} + +/// A shell descriptor read one byte at a time by its caller. +/// +/// Where reading ahead is safe, the descriptor is read a block at a time. +/// On drop, a regular file's shared offset is seeked back over the bytes read +/// but not handed out, so the next reader of the descriptor resumes right +/// after the last one consumed (bash's `read` does the same). Elsewhere every +/// byte is its own read, so nothing past the last consumed byte is taken from +/// a pipe or terminal another command shares. +pub(crate) struct LineInput { + input: OpenFile, + blocks: bool, + block: Vec, + pos: usize, +} + +impl LineInput { + /// Terminal input is always read byte by byte, so interactive keys + /// arrive as typed. Only a plain file handle counts as a regular file: + /// std's `Stdin` keeps a buffer of its own, which a seek on the + /// descriptor could not give back. + pub(crate) fn new(input: OpenFile, read_ahead: ReadAhead) -> Self { + let blocks = !input.is_terminal() + && match read_ahead { + ReadAhead::ToEof => true, + ReadAhead::Seekable => { + matches!(&input, OpenFile::File(file) if file.metadata().is_ok_and(|m| m.is_file())) + }, + }; + Self { input, blocks, block: Vec::new(), pos: 0 } + } + + /// The underlying descriptor, e.g. to poll it before a read that could + /// block. + pub(crate) const fn input(&self) -> &OpenFile { + &self.input + } + + /// Whether reads come from a read-ahead block, never waiting on input. + pub(crate) const fn reads_ahead(&self) -> bool { + self.blocks + } + + /// The next byte, or `None` at end of input. + pub(crate) fn next_byte(&mut self) -> io::Result> { + if !self.blocks { + let mut byte = [0_u8; 1]; + return Ok((self.input.read(&mut byte)? == 1).then_some(byte[0])); + } + if self.pos == self.block.len() { + self.block.resize(BLOCK, 0); + let n = self.input.read(&mut self.block)?; + self.block.truncate(n); + self.pos = 0; + if n == 0 { + return Ok(None); + } + } + self.pos += 1; + Ok(Some(self.block[self.pos - 1])) + } + + /// Gives the bytes read ahead but not handed out back to a regular + /// file's shared offset, so another reader of the descriptor resumes + /// right after the last byte consumed. Bash's `mapfile` does the same + /// before each `-C` callback. Read-ahead that cannot be given back is + /// kept and handed out as usual. + pub(crate) fn give_back(&mut self) { + let unread = self.block.len() - self.pos; + if unread > 0 + && let OpenFile::File(file) = &mut self.input + && file.seek(SeekFrom::Current(-(unread as i64))).is_ok() + { + self.block.clear(); + self.pos = 0; + } + } +} + +impl Drop for LineInput { + fn drop(&mut self) { + self.give_back(); + } +} diff --git a/crates/brush-builtins-vendored/src/mapfile.rs b/crates/brush-builtins-vendored/src/mapfile.rs index d258f246ba6..5725daeeea8 100644 --- a/crates/brush-builtins-vendored/src/mapfile.rs +++ b/crates/brush-builtins-vendored/src/mapfile.rs @@ -1,9 +1,11 @@ -// Vendored from can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1:crates/pi-builtins/src/mapfile.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, retained local array validation and synchronous file descriptor reads. -use std::io::{Read, Write}; +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/mapfile.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, retained local array validation and synchronous file descriptor reads. +use std::io::Write; use brush_core::{ErrorKind, ExecutionExitCode, ExecutionResult, builtins, env, escape, variables}; use clap::Parser; +use crate::line_input::{CTRL_C, CTRL_D, LineInput, ReadAhead, decode_line}; + /// Read lines from standard input into an indexed array variable. #[derive(Parser)] pub(crate) struct MapFileCommand { @@ -59,6 +61,12 @@ impl builtins::Command for MapFileCommand { } if let Some((_, var)) = context.shell.env().get(&self.array_var_name) { + // Refused before any input is read, as in bash. Assigning an element + // does not check this itself (so -O wrote into a readonly array), and + // a failure mid-loop would lose input read ahead of a pipe. + if var.is_readonly() { + return Err(ErrorKind::ReadonlyVariable.into()); + } if matches!( var.value(), variables::ShellValue::AssociativeArray(_) @@ -100,7 +108,7 @@ impl builtins::Command for MapFileCommand { impl MapFileCommand { async fn read_entries( &self, - mut input_file: brush_core::openfiles::OpenFile, + input_file: brush_core::openfiles::OpenFile, context: &mut brush_core::ExecutionContext<'_, SE>, ) -> Result, brush_core::Error> { let _term_mode = setup_terminal_settings(&input_file)?; @@ -115,27 +123,30 @@ impl MapFileCommand { None => b'\n', }; - let mut buf = [0u8; 1]; + let terminal = input_file.is_terminal(); + // Without -n or -C every byte up to EOF is consumed before anything + // else reads the descriptor, so any input is read in blocks. -n may + // stop early, and a -C callback may read the descriptor itself or + // end the loop; then only a regular file, whose offset is given + // back, is (bash's mapfile reads a pipe unbuffered too). + let read_ahead = + if max_count == 0 && self.callback.is_none() { ReadAhead::ToEof } else { ReadAhead::Seekable }; + let mut input = LineInput::new(input_file, read_ahead); while max_count == 0 || entry_count < max_count { let mut line = vec![]; let mut saw_delimiter = false; - loop { - match input_file.read(&mut buf) { - Ok(0) => break, // End of input - Ok(1) if buf[0] == b'\x03' => break, // Ctrl+C - Ok(1) if buf[0] == b'\x04' && line.is_empty() => break, // Ctrl+D - Ok(1) => { - let byte = buf[0]; - line.push(byte); - if byte == delimiter { - saw_delimiter = true; - break; - } - }, - Ok(_) => unreachable!("input can only be 0, 1, or error"), - Err(e) => return Err(e.into()), + while let Some(byte) = input.next_byte()? { + // Interactive keys end input only on a terminal; in a file or + // pipe they are data. + if terminal && (byte == CTRL_C || byte == CTRL_D && line.is_empty()) { + break; + } + line.push(byte); + if byte == delimiter { + saw_delimiter = true; + break; } } @@ -152,12 +163,13 @@ impl MapFileCommand { line.pop(); } - let line_str = String::from_utf8_lossy(&line).to_string(); + let line_str = decode_line(line); let array_index = self.origin.unwrap_or(0) + i64::try_from(entry_count)?; if let Some(callback) = &self.callback && (entry_count + 1).is_multiple_of(callback_group_size) { + input.give_back(); let result = run_callback(callback, array_index, &line_str, context).await?; if !result.is_normal_flow() { return Ok(Some(result)); @@ -204,6 +216,11 @@ async fn run_callback( fn setup_terminal_settings( file: &brush_core::openfiles::OpenFile, ) -> Result, brush_core::Error> { + // The guard dups the descriptor and reads its terminal attributes; a + // pipe or file has none. + if !file.is_terminal() { + return Ok(None); + } let mode = brush_core::terminal::AutoModeGuard::new(file.to_owned()).ok(); if let Some(mode) = &mode { let config = brush_core::terminal::Settings::builder() diff --git a/crates/brush-builtins-vendored/src/read.rs b/crates/brush-builtins-vendored/src/read.rs index b1e03420436..94d1feef658 100644 --- a/crates/brush-builtins-vendored/src/read.rs +++ b/crates/brush-builtins-vendored/src/read.rs @@ -1,28 +1,26 @@ -// Vendored from can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1:crates/pi-builtins/src/read.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, preserves cancellation, timeout, and readiness guards. +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/read.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, preserves cancellation, timeout, and readiness guards. use std::{ collections::VecDeque, - io::{Read, Write}, + io::Write, time::{Duration, Instant}, }; -use brush_core::{ErrorKind, builtins, env, error, variables}; +use brush_core::{ErrorKind, builtins, env, error, openfiles::OpenFile, variables}; use clap::Parser; use itertools::Itertools; +use crate::line_input::{CTRL_C, CTRL_D, LineInput, ReadAhead, decode_line}; + /// Exit code returned when `read` times out. /// This is 128 + SIGALRM (14) = 142, matching bash behavior. const TIMEOUT_EXIT_CODE: u8 = 142; -/// ASCII control character for Ctrl+C (ETX - End of Text). -const CTRL_C: char = '\x03'; -/// ASCII control character for Ctrl+D (EOT - End of Transmission). -const CTRL_D: char = '\x04'; /// Backslash character used for escape processing. -const BACKSLASH: char = '\\'; +const BACKSLASH: u8 = b'\\'; /// Default line delimiter (newline). -const DEFAULT_DELIMITER: char = '\n'; +const DEFAULT_DELIMITER: u8 = b'\n'; /// NUL character used as delimiter when `-d ''` is specified. -const NUL_DELIMITER: char = '\0'; +const NUL_DELIMITER: u8 = b'\0'; /// Parse standard input. #[derive(Parser)] @@ -294,25 +292,20 @@ enum ReadResult { InputNotReady, } -/// Helper struct that encapsulates the state for reading input character by -/// character. +/// Low-level input reader that handles timeouts and control characters. /// -/// This separates the concerns of character-level I/O with timeout handling +/// This separates the concerns of byte-level I/O with timeout handling /// from the higher-level logic of line building and escape processing. struct InputReader { - /// The input source. - input: brush_core::openfiles::OpenFile, + /// The input source: a regular file is read ahead, and what lies past + /// the line is given back when `read` returns. + source: LineInput, /// Optional deadline for timeout. deadline: Option, - /// Single-byte read buffer. - /// - /// TODO(utf-8): This only handles ASCII correctly. Multi-byte UTF-8 - /// characters will be read as separate bytes and incorrectly interpreted. - /// To fix this, we would need to buffer up to 4 bytes and decode - /// incrementally using `std::str::from_utf8`. Note that bash's `-n` counts - /// bytes, not Unicode codepoints, so the fix needs to preserve that - /// behavior. - buffer: [u8; 1], + /// Terminal input: Ctrl-C and Ctrl-D bytes are keys, not data. + terminal: bool, + /// Bytes a stream reported ready, readable without waiting for input. + ready: u64, /// Terminal mode guard - kept alive for RAII cleanup on drop. /// The guard restores original terminal settings when dropped, even though /// we don't access the field directly after construction. @@ -324,8 +317,8 @@ struct InputReader { /// Events that can occur when reading input. enum InputEvent { - /// A regular character was read. - Char(char), + /// A data byte was read. + Byte(u8), /// End of file was reached. Eof, /// The read operation timed out. @@ -350,14 +343,16 @@ where impl InputReader { /// Creates a new input reader with optional timeout. fn new( - input: brush_core::openfiles::OpenFile, + input: OpenFile, timeout: Option, term_mode: Option, ) -> Self { + let terminal = input.is_terminal(); Self { - input, + source: LineInput::new(input, ReadAhead::Seekable), deadline: timeout.map(|t| Instant::now() + t), - buffer: [0; 1], + terminal, + ready: 0, _term_mode: term_mode, } } @@ -365,7 +360,7 @@ impl InputReader { /// Checks if input is immediately available (for `-t 0`). Returns `false` if /// an error occurs while checking for available input. fn check_input_available(&self) -> bool { - brush_core::sys::poll::poll_for_input(&self.input, Duration::ZERO).unwrap_or(false) + brush_core::sys::poll::poll_for_input(self.source.input(), Duration::ZERO).unwrap_or(false) } #[cfg(unix)] @@ -375,7 +370,7 @@ impl InputReader { { const INPUT_POLL_INTERVAL_MS: u64 = 100; - if self.deadline.is_none() && self.input.try_borrow_as_fd().is_err() { + if self.deadline.is_none() && self.source.input().try_borrow_as_fd().is_err() { ensure_not_cancelled(is_cancelled)?; return Ok(None); } @@ -394,7 +389,7 @@ impl InputReader { Duration::from_millis(INPUT_POLL_INTERVAL_MS) }; - match brush_core::sys::poll::poll_for_input(&self.input, timeout) { + match brush_core::sys::poll::poll_for_input(self.source.input(), timeout) { Ok(true) => return Ok(None), Ok(false) => continue, Err(e) => return Err(e.into()), @@ -415,7 +410,7 @@ impl InputReader { return Ok(Some(InputEvent::Timeout)); } - match brush_core::sys::poll::poll_for_input(&self.input, remaining) { + match brush_core::sys::poll::poll_for_input(self.source.input(), remaining) { Ok(true) => {}, Ok(false) => return Ok(Some(InputEvent::Timeout)), Err(e) => return Err(e.into()), @@ -425,43 +420,97 @@ impl InputReader { Ok(None) } + /// Bytes readable from a stream right now without blocking; at least 1 + /// once the stream polled readable (an EOF reports 0 ready but must + /// still be read to be seen). + fn bytes_ready(&self) -> u64 { + #[cfg(unix)] + let ready = self + .source + .input() + .try_borrow_as_fd() + .ok() + .and_then(|fd| rustix::io::ioctl_fionread(fd).ok()) + .unwrap_or(0); + #[cfg(not(unix))] + let ready = 0; + ready.max(1) + } + /// Reads the next input event, handling timeout and control characters. fn read_event(&mut self, is_cancelled: &F) -> Result where F: Fn() -> bool, { - if let Some(event) = self.wait_for_input(is_cancelled)? { - return Ok(event); + if !self.source.reads_ahead() { + if self.ready == 0 { + if let Some(event) = self.wait_for_input(is_cancelled)? { + return Ok(event); + } + // Once a stream polls readable, learn how much is buffered so + // the bytes after this one skip the poll (and the descriptor + // classification it repeats) entirely. + self.ready = self.bytes_ready(); + } else if self.deadline.is_some_and(|deadline| Instant::now() >= deadline) { + return Ok(InputEvent::Timeout); + } + self.ready -= 1; } - ensure_not_cancelled(is_cancelled)?; - - let n = self.input.read(&mut self.buffer)?; - if n == 0 { + let Some(byte) = self.source.next_byte()? else { + self.ready = 0; return Ok(InputEvent::Eof); - } - - let ch = self.buffer[0] as char; + }; - // Map control characters to events. - Ok(match ch { - CTRL_C => InputEvent::CtrlC, - CTRL_D => InputEvent::CtrlD, - _ => InputEvent::Char(ch), + // Control keys only mean something on a terminal; in a file or pipe + // they are data. + Ok(match byte { + CTRL_C if self.terminal => InputEvent::CtrlC, + CTRL_D if self.terminal => InputEvent::CtrlD, + _ => InputEvent::Byte(byte), }) } } /// Configuration for line reading behavior. struct LineReaderConfig { - /// Character that terminates input (None for -N mode). - delimiter: Option, + /// Byte that terminates input (None for -N mode). + delimiter: Option, /// Maximum characters to read (for -n or -N). char_limit: Option, /// Whether to process backslash escapes (false for -r mode). process_escapes: bool, } +/// Characters read toward a `-n`/`-N` limit. Bash counts characters, not +/// bytes: a UTF-8 lead byte starts one, and the limit is reached only once +/// its continuation bytes are in, so a multibyte character is never split. +#[derive(Default)] +struct CharCount { + chars: usize, + continuation: u8, +} + +impl CharCount { + fn push(&mut self, byte: u8) { + if self.continuation > 0 && byte & 0xC0 == 0x80 { + self.continuation -= 1; + } else { + self.chars += 1; + self.continuation = match byte { + 0xC0..=0xDF => 1, + 0xE0..=0xEF => 2, + 0xF0..=0xF7 => 3, + _ => 0, + }; + } + } + + fn reached(&self, limit: Option) -> bool { + self.continuation == 0 && limit.is_some_and(|limit| self.chars >= limit) + } +} + /// Reads a complete line of input using the given reader and configuration. /// /// Returns a `ReadResult` indicating success, EOF, timeout, or interruption. @@ -480,7 +529,8 @@ fn read_line_with_reader( where F: Fn() -> bool, { - let mut line = String::new(); + let mut line = Vec::new(); + let mut count = CharCount::default(); let mut pending_backslash = false; loop { @@ -489,7 +539,7 @@ where match event { InputEvent::Eof => { // Bash discards pending backslash on EOF. - return Ok(ReadResult::Eof(if line.is_empty() { None } else { Some(line) })); + return Ok(ReadResult::Eof((!line.is_empty()).then(|| decode_line(line)))); }, InputEvent::Timeout => { @@ -497,7 +547,7 @@ where if pending_backslash { line.push(BACKSLASH); } - return Ok(ReadResult::TimedOut(if line.is_empty() { None } else { Some(line) })); + return Ok(ReadResult::TimedOut((!line.is_empty()).then(|| decode_line(line)))); }, InputEvent::CtrlC => { @@ -510,60 +560,54 @@ where return Ok(if line.is_empty() && !pending_backslash { ReadResult::Eof(None) } else { - ReadResult::Line(line) + ReadResult::Line(decode_line(line)) }); }, - InputEvent::Char(ch) => { + InputEvent::Byte(byte) => { // Handle backslash escape processing (when enabled). if config.process_escapes { if pending_backslash { pending_backslash = false; // Backslash-delimiter is line continuation. - if let Some(delim) = config.delimiter - && ch == delim - { + if config.delimiter == Some(byte) { continue; // Line continuation. } - // For other chars, add char literally (backslash consumed). - line.push(ch); + // For other bytes, add the byte literally (backslash consumed). + line.push(byte); + count.push(byte); // Check character limit (based on output length). - if let Some(limit) = config.char_limit - && line.len() >= limit - { - return Ok(ReadResult::Line(line)); + if count.reached(config.char_limit) { + return Ok(ReadResult::Line(decode_line(line))); } continue; } - if ch == BACKSLASH { + if byte == BACKSLASH { pending_backslash = true; continue; } } // Check for delimiter. - if let Some(delim) = config.delimiter - && ch == delim - { - return Ok(ReadResult::Line(line)); + if config.delimiter == Some(byte) { + return Ok(ReadResult::Line(decode_line(line))); } // Ignore non-whitespace control characters. - if ch.is_ascii_control() && !ch.is_ascii_whitespace() { + if byte.is_ascii_control() && !byte.is_ascii_whitespace() { continue; } - line.push(ch); + line.push(byte); + count.push(byte); // Check character limit (based on output length). - if let Some(limit) = config.char_limit - && line.len() >= limit - { - return Ok(ReadResult::Line(line)); + if count.reached(config.char_limit) { + return Ok(ReadResult::Line(decode_line(line))); } }, } @@ -598,15 +642,12 @@ impl ReadCommand { } } - // Determine delimiter based on options. + // Determine delimiter based on options. Input is matched byte by byte, + // so a multi-byte `-d` delimiter splits at its first byte. let delimiter = if self.return_after_n_chars_no_delimiter.is_some() { None } else if let Some(delimiter_str) = &self.delimiter { - if delimiter_str.is_empty() { - Some(NUL_DELIMITER) - } else { - delimiter_str.chars().next() - } + Some(delimiter_str.as_bytes().first().copied().unwrap_or(NUL_DELIMITER)) } else { Some(DEFAULT_DELIMITER) }; @@ -638,6 +679,11 @@ impl ReadCommand { &self, file: &brush_core::openfiles::OpenFile, ) -> Result, brush_core::Error> { + // The guard dups the descriptor and reads its terminal attributes; a + // pipe or file has none, so skip it in every `while read` iteration. + if !file.is_terminal() { + return Ok(None); + } let mode = brush_core::terminal::AutoModeGuard::new(file.to_owned()).ok(); if let Some(mode) = &mode { let config = brush_core::terminal::Settings::builder() diff --git a/crates/pi-shell/Cargo.toml b/crates/pi-shell/Cargo.toml index aae98f1f04e..e1cde655a02 100644 --- a/crates/pi-shell/Cargo.toml +++ b/crates/pi-shell/Cargo.toml @@ -35,3 +35,6 @@ libc.workspace = true smallvec.workspace = true windows-sys.workspace = true winreg.workspace = true + +[dev-dependencies] +tempfile = "3" diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index 3092e38eb18..b40f62e5411 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -3632,6 +3632,128 @@ mod tests { ); } + async fn execute_captured(command: String) -> (ShellExecuteResult, String) { + #[cfg(unix)] + let _process_test_guard = PROCESS_TEST_LOCK.lock().await; + + let (tx, mut rx) = mpsc::unbounded_channel::(); + let result = execute_shell( + ShellExecuteOptions { command, ..Default::default() }, + Some(tx), + CancelToken::default(), + ) + .await + .expect("shell execution"); + let mut output = String::new(); + while let Some(chunk) = rx.recv().await { + output.push_str(&chunk); + } + (result, output) + } + + /// Contract: `read` from a file consumes exactly one line of the shared + /// offset — the next `read`, and any later reader of the same descriptor, + /// resumes right after it. + #[tokio::test(flavor = "multi_thread")] + async fn read_from_a_file_leaves_the_rest_for_the_next_reader() { + let dir = tempfile::tempdir().expect("temporary directory"); + let path = dir.path().join("lines.txt"); + std::fs::write(&path, "one\ntwo\nthree\nfour\n").expect("write fixture"); + let path = path.to_string_lossy().replace('\\', "/"); + let (result, output) = execute_captured(format!( + "{{ read -r a; read -r b; echo \"[$a][$b]\"; cat; }} < '{path}'" + )) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[one][two]\nthree\nfour\n"); + } + + /// Contract: `read` assigns UTF-8 input as text, not one Latin-1 + /// character per byte. + #[tokio::test(flavor = "multi_thread")] + async fn read_decodes_utf8_input() { + let (result, output) = execute_captured( + "printf 'é ü\\n' | { read -r first rest; echo \"$first|$rest\"; }".to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "é|ü\n"); + } + + /// Contract: `read -n` counts characters, not bytes, as bash does in a + /// UTF-8 locale; a multibyte character is never split. + #[tokio::test(flavor = "multi_thread")] + async fn read_count_takes_whole_utf8_characters() { + let (result, output) = execute_captured( + "printf 'éa\\n' | { read -r -n 1 first; read -r rest; echo \"$first|$rest\"; }".to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "é|a\n"); + } + + /// Contract: `mapfile -n` from a file consumes exactly the lines it + /// stores; a later reader of the descriptor gets the rest. + #[tokio::test(flavor = "multi_thread")] + async fn mapfile_count_from_a_file_leaves_the_rest_for_the_next_reader() { + let dir = tempfile::tempdir().expect("temporary directory"); + let path = dir.path().join("lines.txt"); + std::fs::write(&path, "one\ntwo\nthree\nfour\n").expect("write fixture"); + let path = path.to_string_lossy().replace('\\', "/"); + let (result, output) = execute_captured(format!( + "{{ mapfile -t -n 2 lines; echo \"${{lines[*]}}\"; cat; }} < '{path}'" + )) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "one two\nthree\nfour\n"); + } + + /// Contract: a `mapfile -C` callback that reads the same piped stdin gets + /// the lines after the one mapfile stored, as in bash; mapfile must not + /// have read them ahead. + #[tokio::test(flavor = "multi_thread")] + async fn mapfile_callback_reads_the_next_line_of_a_pipe() { + let (result, output) = execute_captured( + "cb() { read -r x; echo \"cb:$1:$2:$x\"; }; seq 1 4 | { mapfile -t -C cb -c 1 arr; echo \ + \"arr=${arr[*]}\"; }" + .to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "cb:0:1:2\ncb:1:3:4\narr=1 3\n"); + } + + /// Contract: the same holds for a regular file, whose read-ahead mapfile + /// gives back to the shared offset before each callback. + #[tokio::test(flavor = "multi_thread")] + async fn mapfile_callback_reads_the_next_line_of_a_file() { + let dir = tempfile::tempdir().expect("temporary directory"); + let path = dir.path().join("lines.txt"); + std::fs::write(&path, "1\n2\n3\n4\n").expect("write fixture"); + let path = path.to_string_lossy().replace('\\', "/"); + let (result, output) = execute_captured(format!( + "cb() {{ read -r x; echo \"cb:$1:$2:$x\"; }}; {{ mapfile -t -C cb -c 1 arr; echo \ + \"arr=${{arr[*]}}\"; }} < '{path}'" + )) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "cb:0:1:2\ncb:1:3:4\narr=1 3\n"); + } + + /// Contract: `mapfile` into a readonly array fails before reading, as in + /// bash: the array is unchanged and the piped input stays for the next + /// reader. + #[tokio::test(flavor = "multi_thread")] + async fn mapfile_into_a_readonly_array_leaves_the_input_unread() { + let (_, output) = execute_captured( + "printf 'a\\nb\\n' | { arr=(x); readonly arr; mapfile -t -O 1 arr; echo \"rc=$? \ + ${arr[*]}\"; cat; }" + .to_owned(), + ) + .await; + assert!(output.ends_with("rc=1 x\na\nb\n"), "{output:?}"); + } + #[tokio::test(flavor = "multi_thread")] async fn cd_physical_mode_supports_exit_on_failed_resolution() { #[cfg(unix)] diff --git a/docs/rust-porting-inventory.md b/docs/rust-porting-inventory.md index ee059a193c2..bbb755fc2f7 100644 --- a/docs/rust-porting-inventory.md +++ b/docs/rust-porting-inventory.md @@ -42,7 +42,7 @@ Upstream pin: `can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1` | A-PI-WALKER | crate | `crates/pi-walker@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-walker`; `crates/pi-natives/src/iofs.rs` | infra/overlap | glob, invalidateFsScanCache, walkerPoolStatus | Filesystem traversal substrate; `packages/coding-agent/src/tools/find.ts` | Recorded glob/fd/workspace outputs on fixture trees | `tools:glob`, `startup`, `tools` | S1-S5,S7 | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Integrated traversal substrate and pi-edit dependency; absorbs fs_cache policy and keeps the serial-on-pool-failure branch. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6027, https://github.com/Yeachan-Heo/gajae-code/pull/6039. | — | — | — | | A-PI-EDIT | crate | `crates/pi-edit@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-edit`; `crates/pi-natives/src/edit.rs` | (a) | `editFindMatch`, `editSeekSequence`, hashline, patch, EditSession, EditStore APIs | `packages/coding-agent/src/edit/{modes/replace,patch,apply-patch}.ts`; `edit/apply-patch/*`; `edit/{diff,normalize,streaming,notebook}.ts`; `hashline/*.ts` | Edit-benchmark TS matcher goldens including errors; per-mode goldens through 2.10 | `edit-hotspots` H01,H02,H06; `tools:edit`; `bench:edit`; `replay` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/6016 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Matcher merged via #5980 and fuzzy scoring fix #6016; edit-hotspots H01/H02 covered by the pi-edit fuzzy A/B in #6016. | — | — | `packages/coding-agent/src/edit/modes/replace.ts`; `packages/coding-agent/src/edit/streaming.ts` | A-PI-VCS | crate | `crates/pi-vcs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-vcs`; `crates/pi-natives/src/vcs.rs` | (a), size-gated (D5) | VCS read APIs | `packages/coding-agent/src/utils/git.ts`; `autoresearch/git.ts`; `status-line/git-utils.ts`; `commit/agentic/tools/git-*.ts` | Git CLI oracle over fixture repositories | `status-line`, `tools:git` | — | rejected | | | D5 perf gate: large-worktree read suite upper-CI p50 ratio was 0.826 (needs <0.80), and status was 1.20x slower. | — | — | — | -| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. | `crates/brush-builtins-vendored/src/read.rs` | — | — | +| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. Per-row bump for #6543: `read.rs`, `mapfile.rs` and the new `line_input.rs` are vendored from `can1357/oh-my-pi` commit `68fb9863b377af566d3b56aa882a964654b5760a` (upstream `f3ce813d`, `b636b6ae`, `b6e8ee12`, `68fb9863`): UTF-8 line decoding, character-counted `read -n`/`-N`, and block read-ahead that gives unread bytes back to a regular file. | `crates/brush-builtins-vendored/src/read.rs` | — | — | | A-PI-BUILTINS-UTILS | crate | `pi-builtins/src/utils@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | Bash tool builtins | — | — | — | rejected | | | GNU/BSD behavior diverges; utility builtins pull C-backed onig_sys and pcre2-sys. uucore is already a pre-existing printf dependency and is not a rejection criterion. | — | — | — | | A-PI-VOICE | crate | `crates/pi-voice@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | gjc STT uses `packages/coding-agent/src/stt/recorder.ts` | — | — | — | rejected | | | opus/webrtc and dynamically loaded PulseAudio/ALSA are out of scope; gjc uses sox/ffmpeg/arecord. | — | — | — | diff --git a/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md b/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md new file mode 100644 index 00000000000..74b5bd51152 --- /dev/null +++ b/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md @@ -0,0 +1,8 @@ +### Fixed + +- The Bash tool's `read` builtin now decodes UTF-8 input instead of turning each byte into a Latin-1 character, so `while IFS= read -r line` loops pass non-ASCII text through intact, and `read -n`/`-N` count characters rather than bytes (#6543). +- `mapfile` into a readonly array now fails before reading, leaving piped input for the next reader as bash does. + +### Performance + +- `read` and `mapfile` read a regular file in blocks and give the unread bytes back to the file offset, and `mapfile` without `-n` or `-C` reads a pipe in blocks, instead of one system call per byte. From e7a41dcac60046cfe37476fe4a392262a3cf7bbf Mon Sep 17 00:00:00 2001 From: argus Date: Sun, 11 Oct 2026 14:21:31 +0900 Subject: [PATCH 2/5] fix(shell): fix three read regressions from the oh-my-pi port Review of the oh-my-pi port found three regressions in the ported code: - read -n/-N counted a byte that cannot start a character as a lead byte, so `read -n 1` on C0 41 took the A as well. Count a character when the pending bytes decode or can no longer decode, as bash's mbrtowc loop does, so C0 alone is one character and A is left - a failed refill left the read-ahead block at its full 4096-byte length, so drop sought back over bytes that were already handed out. Clear the block on a read error, as bash's zreadc empties its buffer - a cached FIONREAD count let read skip the poll for the next bytes. When another reader drained the pipe in between, the read blocked where neither -t nor cancellation is checked. Poll a stream before every byte again, as read did before the port, and drop rustix read.rs and line_input.rs now differ from oh-my-pi 68fb9863; their headers and the A-PI-BUILTINS-BASE inventory row record how. Also adds a read -N regression over a newline and 3- and 4-byte characters. Confidence: high Scope-risk: narrow Tested: cargo test -p pi-shell --lib (234 passed); brush-builtins unit tests (35 passed, 2 new fail before this fix); bun run check:rs; changelog and porting-inventory checks; repros through the native executeShell binding Not-tested: Windows and Linux locally Co-authored-by: Claude --- Cargo.lock | 1 - crates/brush-builtins-vendored/Cargo.toml | 5 - .../brush-builtins-vendored/src/line_input.rs | 52 +++++- crates/brush-builtins-vendored/src/read.rs | 152 ++++++++++++------ crates/pi-shell/src/shell.rs | 31 ++++ docs/rust-porting-inventory.md | 2 +- 6 files changed, 184 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 480ccf85ab1..a071c4438d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -491,7 +491,6 @@ dependencies = [ "nix 0.31.3", "procfs", "rlimit", - "rustix", "strum", "strum_macros", "thiserror 2.0.18", diff --git a/crates/brush-builtins-vendored/Cargo.toml b/crates/brush-builtins-vendored/Cargo.toml index 3e2aec59d5c..82e3056eb55 100644 --- a/crates/brush-builtins-vendored/Cargo.toml +++ b/crates/brush-builtins-vendored/Cargo.toml @@ -241,11 +241,6 @@ features = [ [target."cfg(unix)".dependencies.rlimit] version = "0.11.0" -[target."cfg(unix)".dependencies.rustix] -version = "1.1.4" -features = ["std"] -default-features = false - [lints.clippy] bool_to_int_with_if = "allow" cognitive_complexity = "allow" diff --git a/crates/brush-builtins-vendored/src/line_input.rs b/crates/brush-builtins-vendored/src/line_input.rs index 219019d9a87..03af067a306 100644 --- a/crates/brush-builtins-vendored/src/line_input.rs +++ b/crates/brush-builtins-vendored/src/line_input.rs @@ -1,4 +1,4 @@ -// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/line_input.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: no. +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/line_input.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, a failed refill leaves no read-ahead to give back. //! Byte input for the line-reading builtins (`read`, `mapfile`). use std::io::{self, Read, Seek, SeekFrom}; @@ -80,9 +80,18 @@ impl LineInput { } if self.pos == self.block.len() { self.block.resize(BLOCK, 0); - let n = self.input.read(&mut self.block)?; - self.block.truncate(n); self.pos = 0; + let n = match self.input.read(&mut self.block) { + Ok(n) => n, + Err(error) => { + // Nothing was read into the block, and everything before it + // was handed out; leaving it at full length would make + // `give_back` seek back over bytes already consumed. + self.block.clear(); + return Err(error); + }, + }; + self.block.truncate(n); if n == 0 { return Ok(None); } @@ -113,3 +122,40 @@ impl Drop for LineInput { self.give_back(); } } + +#[cfg(test)] +#[expect(clippy::panic_in_result_fn)] +mod tests { + use std::io::{Seek, Write}; + + use anyhow::Result; + + use super::*; + + /// Regression: a refill that fails must not leave the block at full + /// length, or dropping the input seeks the shared offset back over bytes + /// that were never read (here, over bytes just written). + #[test] + fn failed_refill_gives_nothing_back() -> Result<()> { + let path = std::env::temp_dir() + .join(format!("brush-line-input-failed-refill-{}.txt", std::process::id())); + // Write-only, so every read fails. + let mut file = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .open(&path)?; + file.write_all(&[b'a'; 5000])?; + let mut offset = file.try_clone()?; + + let mut input = LineInput::new(OpenFile::File(file), ReadAhead::Seekable); + assert!(input.reads_ahead()); + assert!(input.next_byte().is_err()); + drop(input); + + let position = offset.stream_position()?; + std::fs::remove_file(&path)?; + assert_eq!(position, 5000); + Ok(()) + } +} diff --git a/crates/brush-builtins-vendored/src/read.rs b/crates/brush-builtins-vendored/src/read.rs index 94d1feef658..e969cd54902 100644 --- a/crates/brush-builtins-vendored/src/read.rs +++ b/crates/brush-builtins-vendored/src/read.rs @@ -1,4 +1,4 @@ -// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/read.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, preserves cancellation, timeout, and readiness guards. +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/read.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, preserves cancellation, timeout, and readiness guards; polls a stream before every byte; counts -n/-N characters by UTF-8 validity. use std::{ collections::VecDeque, io::Write, @@ -304,8 +304,6 @@ struct InputReader { deadline: Option, /// Terminal input: Ctrl-C and Ctrl-D bytes are keys, not data. terminal: bool, - /// Bytes a stream reported ready, readable without waiting for input. - ready: u64, /// Terminal mode guard - kept alive for RAII cleanup on drop. /// The guard restores original terminal settings when dropped, even though /// we don't access the field directly after construction. @@ -352,7 +350,6 @@ impl InputReader { source: LineInput::new(input, ReadAhead::Seekable), deadline: timeout.map(|t| Instant::now() + t), terminal, - ready: 0, _term_mode: term_mode, } } @@ -420,45 +417,22 @@ impl InputReader { Ok(None) } - /// Bytes readable from a stream right now without blocking; at least 1 - /// once the stream polled readable (an EOF reports 0 ready but must - /// still be read to be seen). - fn bytes_ready(&self) -> u64 { - #[cfg(unix)] - let ready = self - .source - .input() - .try_borrow_as_fd() - .ok() - .and_then(|fd| rustix::io::ioctl_fionread(fd).ok()) - .unwrap_or(0); - #[cfg(not(unix))] - let ready = 0; - ready.max(1) - } - /// Reads the next input event, handling timeout and control characters. fn read_event(&mut self, is_cancelled: &F) -> Result where F: Fn() -> bool, { - if !self.source.reads_ahead() { - if self.ready == 0 { - if let Some(event) = self.wait_for_input(is_cancelled)? { - return Ok(event); - } - // Once a stream polls readable, learn how much is buffered so - // the bytes after this one skip the poll (and the descriptor - // classification it repeats) entirely. - self.ready = self.bytes_ready(); - } else if self.deadline.is_some_and(|deadline| Instant::now() >= deadline) { - return Ok(InputEvent::Timeout); - } - self.ready -= 1; + // A stream is polled before every byte. A count of bytes ready, taken + // once and trusted for later bytes, goes stale when another reader of + // a shared pipe drains it; the read would then block where neither + // `-t` nor cancellation is checked. + if !self.source.reads_ahead() + && let Some(event) = self.wait_for_input(is_cancelled)? + { + return Ok(event); } ensure_not_cancelled(is_cancelled)?; let Some(byte) = self.source.next_byte()? else { - self.ready = 0; return Ok(InputEvent::Eof); }; @@ -483,31 +457,36 @@ struct LineReaderConfig { } /// Characters read toward a `-n`/`-N` limit. Bash counts characters, not -/// bytes: a UTF-8 lead byte starts one, and the limit is reached only once -/// its continuation bytes are in, so a multibyte character is never split. +/// bytes, in a UTF-8 locale: each byte extends the pending sequence until it +/// decodes as one character or can no longer become one, and either way the +/// sequence counts once. A byte that can never begin a character (such as +/// 0xC0) is therefore a character by itself, and the byte after it is left +/// for the next reader; a multibyte character is never split. #[derive(Default)] struct CharCount { - chars: usize, - continuation: u8, + chars: usize, + pending: [u8; 4], + len: usize, } impl CharCount { fn push(&mut self, byte: u8) { - if self.continuation > 0 && byte & 0xC0 == 0x80 { - self.continuation -= 1; - } else { + // A UTF-8 sequence is at most four bytes, and four bytes always decode + // or fail, so `len` is back to 0 before it could reach 4. + self.pending[self.len] = byte; + self.len += 1; + let incomplete = matches!( + std::str::from_utf8(&self.pending[..self.len]), + Err(error) if error.error_len().is_none() + ); + if !incomplete { self.chars += 1; - self.continuation = match byte { - 0xC0..=0xDF => 1, - 0xE0..=0xEF => 2, - 0xF0..=0xF7 => 3, - _ => 0, - }; + self.len = 0; } } fn reached(&self, limit: Option) -> bool { - self.continuation == 0 && limit.is_some_and(|limit| self.chars >= limit) + self.len == 0 && limit.is_some_and(|limit| self.chars >= limit) } } @@ -994,4 +973,79 @@ mod tests { .expect_err("cancelled state must interrupt read builtin"); assert_eq!(err.to_string(), "interrupted"); } + + /// A pipe reader that, right after handing out its first byte, lets a + /// second reader of the same pipe drain whatever else was ready. + #[cfg(unix)] + struct DrainedAfterFirstByte { + reader: std::io::PipeReader, + other: std::io::PipeReader, + drained: bool, + } + + #[cfg(unix)] + impl std::io::Read for DrainedAfterFirstByte { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + let n = self.reader.read(buf)?; + if !self.drained { + self.drained = true; + let mut rest = [0_u8; 1]; + self.other.read_exact(&mut rest)?; + } + Ok(n) + } + } + + #[cfg(unix)] + impl std::io::Write for DrainedAfterFirstByte { + fn write(&mut self, _buf: &[u8]) -> std::io::Result { + Err(std::io::ErrorKind::Unsupported.into()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + #[cfg(unix)] + impl brush_core::openfiles::Stream for DrainedAfterFirstByte { + fn clone_box(&self) -> Box { + unimplemented!("not cloned by read") + } + + fn try_clone_to_owned(&self) -> Result { + Err(brush_core::ErrorKind::CannotConvertToNativeFd.into()) + } + + fn try_borrow_as_fd(&self) -> Result, brush_core::Error> { + use std::os::fd::AsFd; + Ok(self.reader.as_fd()) + } + } + + /// Regression: another reader of a shared pipe can drain bytes this + /// `read` saw ready. Every byte must still wait through the poll, so the + /// read times out instead of blocking while the writer stays open. + #[cfg(unix)] + #[test] + #[expect(clippy::panic_in_result_fn)] + fn test_read_line_times_out_after_another_reader_drains_the_pipe() -> anyhow::Result<()> { + use std::io::Write; + + let command = test_command(); + let (reader, mut writer) = std::io::pipe()?; + let other = reader.try_clone()?; + writer.write_all(b"ab")?; + let input_file = brush_core::openfiles::OpenFile::Stream(Box::new(DrainedAfterFirstByte { + reader, + other, + drained: false, + })); + + let result = + command.read_line(input_file, Vec::::new(), Some(Duration::from_millis(200)), || false)?; + assert!(matches!(result, ReadResult::TimedOut(Some(ref line)) if line == "a")); + drop(writer); + Ok(()) + } } diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index b40f62e5411..13c6f4f6491 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -3692,6 +3692,37 @@ mod tests { assert_eq!(output, "é|a\n"); } + /// Contract: `read -N` counts characters too, and only the count ends + /// it: a newline inside the limit is data, and three- and four-byte + /// characters are taken whole. + #[tokio::test(flavor = "multi_thread")] + async fn read_exact_count_takes_whole_utf8_characters_across_newlines() { + let (result, output) = execute_captured( + "printf '한\\n😀x' | { read -r -N 3 first; read -r rest; echo \"[$first][$rest]\"; }" + .to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[한\n😀][x]\n"); + } + + /// Contract: a byte that cannot begin a UTF-8 character counts as one + /// character on its own, as in bash, so `read -n 1` stops after it and + /// leaves the next byte for the next reader. The same holds for a lead + /// byte whose next byte cannot continue it. The bytes taken decode with + /// replacement characters. + #[tokio::test(flavor = "multi_thread")] + async fn read_count_takes_one_invalid_byte_and_leaves_the_rest() { + let (result, output) = execute_captured( + "for input in '\\300A' '\\340\\200A'; do printf \"$input\\n\" | { read -r -n 1 first; \ + read -r rest; echo \"[$first][$rest]\"; }; done" + .to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[\u{FFFD}][A]\n[\u{FFFD}\u{FFFD}][A]\n"); + } + /// Contract: `mapfile -n` from a file consumes exactly the lines it /// stores; a later reader of the descriptor gets the rest. #[tokio::test(flavor = "multi_thread")] diff --git a/docs/rust-porting-inventory.md b/docs/rust-porting-inventory.md index bbb755fc2f7..23603eecf55 100644 --- a/docs/rust-porting-inventory.md +++ b/docs/rust-porting-inventory.md @@ -42,7 +42,7 @@ Upstream pin: `can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1` | A-PI-WALKER | crate | `crates/pi-walker@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-walker`; `crates/pi-natives/src/iofs.rs` | infra/overlap | glob, invalidateFsScanCache, walkerPoolStatus | Filesystem traversal substrate; `packages/coding-agent/src/tools/find.ts` | Recorded glob/fd/workspace outputs on fixture trees | `tools:glob`, `startup`, `tools` | S1-S5,S7 | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Integrated traversal substrate and pi-edit dependency; absorbs fs_cache policy and keeps the serial-on-pool-failure branch. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6027, https://github.com/Yeachan-Heo/gajae-code/pull/6039. | — | — | — | | A-PI-EDIT | crate | `crates/pi-edit@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-edit`; `crates/pi-natives/src/edit.rs` | (a) | `editFindMatch`, `editSeekSequence`, hashline, patch, EditSession, EditStore APIs | `packages/coding-agent/src/edit/{modes/replace,patch,apply-patch}.ts`; `edit/apply-patch/*`; `edit/{diff,normalize,streaming,notebook}.ts`; `hashline/*.ts` | Edit-benchmark TS matcher goldens including errors; per-mode goldens through 2.10 | `edit-hotspots` H01,H02,H06; `tools:edit`; `bench:edit`; `replay` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/6016 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Matcher merged via #5980 and fuzzy scoring fix #6016; edit-hotspots H01/H02 covered by the pi-edit fuzzy A/B in #6016. | — | — | `packages/coding-agent/src/edit/modes/replace.ts`; `packages/coding-agent/src/edit/streaming.ts` | A-PI-VCS | crate | `crates/pi-vcs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-vcs`; `crates/pi-natives/src/vcs.rs` | (a), size-gated (D5) | VCS read APIs | `packages/coding-agent/src/utils/git.ts`; `autoresearch/git.ts`; `status-line/git-utils.ts`; `commit/agentic/tools/git-*.ts` | Git CLI oracle over fixture repositories | `status-line`, `tools:git` | — | rejected | | | D5 perf gate: large-worktree read suite upper-CI p50 ratio was 0.826 (needs <0.80), and status was 1.20x slower. | — | — | — | -| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. Per-row bump for #6543: `read.rs`, `mapfile.rs` and the new `line_input.rs` are vendored from `can1357/oh-my-pi` commit `68fb9863b377af566d3b56aa882a964654b5760a` (upstream `f3ce813d`, `b636b6ae`, `b6e8ee12`, `68fb9863`): UTF-8 line decoding, character-counted `read -n`/`-N`, and block read-ahead that gives unread bytes back to a regular file. | `crates/brush-builtins-vendored/src/read.rs` | — | — | +| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. Per-row bump for #6543: `read.rs`, `mapfile.rs` and the new `line_input.rs` are vendored from `can1357/oh-my-pi` commit `68fb9863b377af566d3b56aa882a964654b5760a` (upstream `f3ce813d`, `b636b6ae`, `b6e8ee12`, `68fb9863`): UTF-8 line decoding, character-counted `read -n`/`-N`, and block read-ahead that gives unread bytes back to a regular file. Local changes on top: a failed refill leaves nothing to give back, `read` polls a stream before every byte instead of trusting a `FIONREAD` count, and `-n`/`-N` count malformed UTF-8 the way bash does. | `crates/brush-builtins-vendored/src/read.rs` | — | — | | A-PI-BUILTINS-UTILS | crate | `pi-builtins/src/utils@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | Bash tool builtins | — | — | — | rejected | | | GNU/BSD behavior diverges; utility builtins pull C-backed onig_sys and pcre2-sys. uucore is already a pre-existing printf dependency and is not a rejection criterion. | — | — | — | | A-PI-VOICE | crate | `crates/pi-voice@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | gjc STT uses `packages/coding-agent/src/stt/recorder.ts` | — | — | — | rejected | | | opus/webrtc and dynamically loaded PulseAudio/ALSA are out of scope; gjc uses sox/ffmpeg/arecord. | — | — | — | From 257dd838f2d36c7b5e7b56eebab1ccbf278085e6 Mon Sep 17 00:00:00 2001 From: argus Date: Sun, 11 Oct 2026 18:02:01 +0900 Subject: [PATCH 3/5] fix(shell): count read -n characters the way the line decodes read -n/-N counted an invalid sequence as one character only after the byte that ended it, and counted that byte into the same character. With C3 41 and -n 1, read took both bytes and stored two characters, U+FFFD and A, so the limit was exceeded and the A was lost to the next reader. Count characters as decode_line decodes them: a complete character is one, and so is each invalid sequence it replaces with U+FFFD. The byte that shows a lead byte to be invalid starts the next character. When that byte lies past the limit, a regular file takes it back (new LineInput::unread, across a block boundary too) so the next reader gets it. A pipe or terminal cannot take a byte back, so it stays in the variable rather than being dropped. Confidence: high Scope-risk: narrow Tested: cargo test -p pi-shell --lib (235 passed); brush-builtins unit tests (37 passed); bun run check:rs; changelog and porting-inventory checks; malformed-input repros through the native executeShell binding Not-tested: Windows and Linux locally Co-authored-by: Claude --- .../brush-builtins-vendored/src/line_input.rs | 74 ++++++++++++++++- crates/brush-builtins-vendored/src/read.rs | 81 +++++++++++++------ crates/pi-shell/src/shell.rs | 37 ++++++--- docs/rust-porting-inventory.md | 2 +- 4 files changed, 158 insertions(+), 36 deletions(-) diff --git a/crates/brush-builtins-vendored/src/line_input.rs b/crates/brush-builtins-vendored/src/line_input.rs index 03af067a306..c4cf08e6591 100644 --- a/crates/brush-builtins-vendored/src/line_input.rs +++ b/crates/brush-builtins-vendored/src/line_input.rs @@ -1,4 +1,4 @@ -// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/line_input.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, a failed refill leaves no read-ahead to give back. +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/line_input.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, a failed refill leaves no read-ahead to give back, and read can take back the bytes past its -n/-N limit. //! Byte input for the line-reading builtins (`read`, `mapfile`). use std::io::{self, Read, Seek, SeekFrom}; @@ -100,6 +100,33 @@ impl LineInput { Ok(Some(self.block[self.pos - 1])) } + /// Takes back the last `n` bytes handed out, so the next reader of a + /// regular file gets them. Only read-ahead from a regular file can do + /// this; a byte read from a pipe or terminal is gone, so this returns + /// `false` and changes nothing. + pub(crate) fn unread(&mut self, n: usize) -> bool { + if !self.blocks || !matches!(self.input, OpenFile::File(_)) { + return false; + } + if let Some(pos) = self.pos.checked_sub(n) { + self.pos = pos; + return true; + } + // Some of the bytes came from the previous block: seek back over this + // block and those bytes, and start over from there. + let Ok(back) = i64::try_from(self.block.len() - self.pos + n) else { + return false; + }; + if let OpenFile::File(file) = &mut self.input + && file.seek(SeekFrom::Current(-back)).is_ok() + { + self.block.clear(); + self.pos = 0; + return true; + } + false + } + /// Gives the bytes read ahead but not handed out back to a regular /// file's shared offset, so another reader of the descriptor resumes /// right after the last byte consumed. Bash's `mapfile` does the same @@ -158,4 +185,49 @@ mod tests { assert_eq!(position, 5000); Ok(()) } + + /// Contract: bytes taken back from a regular file go to the next reader + /// of the descriptor, also when they came from the previous block. + #[test] + fn unread_bytes_go_back_to_the_file() -> Result<()> { + let path = std::env::temp_dir() + .join(format!("brush-line-input-unread-{}.txt", std::process::id())); + std::fs::write(&path, vec![b'a'; BLOCK + 10])?; + let file = std::fs::File::open(&path)?; + let mut offset = file.try_clone()?; + + let mut input = LineInput::new(OpenFile::File(file), ReadAhead::Seekable); + for _ in 0..5 { + input.next_byte()?; + } + assert!(input.unread(2)); + drop(input); + let within_block = offset.stream_position()?; + + offset.seek(SeekFrom::Start(0))?; + let mut input = LineInput::new(OpenFile::File(offset.try_clone()?), ReadAhead::Seekable); + for _ in 0..=BLOCK { + input.next_byte()?; + } + assert!(input.unread(2)); + drop(input); + let across_blocks = offset.stream_position()?; + + std::fs::remove_file(&path)?; + assert_eq!(within_block, 3); + assert_eq!(across_blocks, u64::try_from(BLOCK)? - 1); + Ok(()) + } + + /// Contract: a pipe byte cannot be taken back. + #[test] + fn unread_from_a_pipe_does_nothing() -> Result<()> { + let (reader, mut writer) = std::io::pipe()?; + writer.write_all(b"ab")?; + let mut input = LineInput::new(OpenFile::PipeReader(reader), ReadAhead::Seekable); + assert_eq!(input.next_byte()?, Some(b'a')); + assert!(!input.unread(1)); + assert_eq!(input.next_byte()?, Some(b'b')); + Ok(()) + } } diff --git a/crates/brush-builtins-vendored/src/read.rs b/crates/brush-builtins-vendored/src/read.rs index e969cd54902..62a070507f4 100644 --- a/crates/brush-builtins-vendored/src/read.rs +++ b/crates/brush-builtins-vendored/src/read.rs @@ -1,4 +1,4 @@ -// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/read.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, preserves cancellation, timeout, and readiness guards; polls a stream before every byte; counts -n/-N characters by UTF-8 validity. +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/read.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, preserves cancellation, timeout, and readiness guards; polls a stream before every byte; counts -n/-N characters as the line decodes and gives a regular file the byte read past the limit back. use std::{ collections::VecDeque, io::Write, @@ -417,6 +417,17 @@ impl InputReader { Ok(None) } + /// The text of a `-n`/`-N` read whose limit fell before the last byte of + /// `line`, which took `input_len` bytes of input. A regular file takes + /// those bytes back for the next reader. A pipe or terminal cannot, so + /// the byte stays in the line rather than being lost. + fn line_before_last_byte(&mut self, mut line: Vec, input_len: usize) -> String { + if self.source.unread(input_len) { + line.pop(); + } + decode_line(line) + } + /// Reads the next input event, handling timeout and control characters. fn read_event(&mut self, is_cancelled: &F) -> Result where @@ -456,12 +467,12 @@ struct LineReaderConfig { process_escapes: bool, } -/// Characters read toward a `-n`/`-N` limit. Bash counts characters, not -/// bytes, in a UTF-8 locale: each byte extends the pending sequence until it -/// decodes as one character or can no longer become one, and either way the -/// sequence counts once. A byte that can never begin a character (such as -/// 0xC0) is therefore a character by itself, and the byte after it is left -/// for the next reader; a multibyte character is never split. +/// Characters read toward a `-n`/`-N` limit, counted as `decode_line` will +/// decode them: a complete UTF-8 character is one, and so is each invalid +/// sequence it replaces with one U+FFFD. A multibyte character is never +/// split. A lead byte is known to be invalid only once the byte after it +/// arrives, and that byte then starts the next character, so it can lie past +/// the limit. #[derive(Default)] struct CharCount { chars: usize, @@ -469,24 +480,42 @@ struct CharCount { len: usize, } +/// Where a byte leaves a [`CharCount`] relative to its limit. +enum Limit { + /// Below the limit, or inside a character not yet complete. + NotReached, + /// Reached by the character this byte completes. + Reached, + /// Reached by the invalid sequence this byte ended without belonging to: + /// the byte itself lies past the limit. + Passed, +} + impl CharCount { - fn push(&mut self, byte: u8) { - // A UTF-8 sequence is at most four bytes, and four bytes always decode - // or fail, so `len` is back to 0 before it could reach 4. + fn push(&mut self, byte: u8, limit: Option) -> Limit { + // The bytes pending before a push are the start of one valid + // character, at most three, so there is room for this one. self.pending[self.len] = byte; self.len += 1; - let incomplete = matches!( - std::str::from_utf8(&self.pending[..self.len]), - Err(error) if error.error_len().is_none() - ); - if !incomplete { + while let Some(char_len) = self.next_char_len() { self.chars += 1; - self.len = 0; + self.pending.copy_within(char_len..self.len, 0); + self.len -= char_len; + if limit.is_some_and(|limit| self.chars >= limit) { + return if self.len == 0 { Limit::Reached } else { Limit::Passed }; + } } + Limit::NotReached } - fn reached(&self, limit: Option) -> bool { - self.len == 0 && limit.is_some_and(|limit| self.chars >= limit) + /// The length of the first pending character once it is known: a + /// complete character, or an invalid sequence that decodes to one + /// U+FFFD. `None` while the pending bytes can still become a character. + fn next_char_len(&self) -> Option { + (1..=self.len).find_map(|end| match std::str::from_utf8(&self.pending[..end]) { + Ok(_) => Some(end), + Err(error) => error.error_len(), + }) } } @@ -556,13 +585,14 @@ where // For other bytes, add the byte literally (backslash consumed). line.push(byte); - count.push(byte); // Check character limit (based on output length). - if count.reached(config.char_limit) { - return Ok(ReadResult::Line(decode_line(line))); + match count.push(byte, config.char_limit) { + Limit::NotReached => continue, + Limit::Reached => return Ok(ReadResult::Line(decode_line(line))), + // The backslash before the byte lies past the limit too. + Limit::Passed => return Ok(ReadResult::Line(reader.line_before_last_byte(line, 2))), } - continue; } if byte == BACKSLASH { @@ -582,11 +612,12 @@ where } line.push(byte); - count.push(byte); // Check character limit (based on output length). - if count.reached(config.char_limit) { - return Ok(ReadResult::Line(decode_line(line))); + match count.push(byte, config.char_limit) { + Limit::NotReached => {}, + Limit::Reached => return Ok(ReadResult::Line(decode_line(line))), + Limit::Passed => return Ok(ReadResult::Line(reader.line_before_last_byte(line, 1))), } }, } diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index 13c6f4f6491..df0287904b2 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -3706,21 +3706,40 @@ mod tests { assert_eq!(output, "[한\n😀][x]\n"); } - /// Contract: a byte that cannot begin a UTF-8 character counts as one - /// character on its own, as in bash, so `read -n 1` stops after it and - /// leaves the next byte for the next reader. The same holds for a lead - /// byte whose next byte cannot continue it. The bytes taken decode with - /// replacement characters. + /// Contract: `read -n` counts characters as the line decodes them, so + /// each invalid sequence, replaced by one U+FFFD, is one character. A lead + /// byte is known to be invalid only once the next byte arrives, and a + /// pipe cannot take that byte back: when it lies past the limit it stays + /// in the variable rather than being lost. #[tokio::test(flavor = "multi_thread")] - async fn read_count_takes_one_invalid_byte_and_leaves_the_rest() { + async fn read_count_counts_invalid_bytes_as_they_decode() { let (result, output) = execute_captured( - "for input in '\\300A' '\\340\\200A'; do printf \"$input\\n\" | { read -r -n 1 first; \ - read -r rest; echo \"[$first][$rest]\"; }; done" + "printf '\\300A\\n' | { read -r -n 1 a; read -r b; echo \"[$a][$b]\"; }; printf \ + '\\303AB\\n' | { read -r -n 2 a; read -r b; echo \"[$a][$b]\"; }; printf \ + '\\340\\200A\\n' | { read -r -n 1 a; read -r b; echo \"[$a][$b]\"; }" .to_owned(), ) .await; assert_eq!(result.exit_code, Some(0), "{output:?}"); - assert_eq!(output, "[\u{FFFD}][A]\n[\u{FFFD}\u{FFFD}][A]\n"); + assert_eq!(output, "[\u{FFFD}][A]\n[\u{FFFD}A][B]\n[\u{FFFD}\u{FFFD}][A]\n"); + } + + /// Contract: from a regular file, the byte that showed a lead byte to be + /// invalid goes back to the file when it lies past the `-n` limit, so + /// the next reader gets it. + #[tokio::test(flavor = "multi_thread")] + async fn read_count_gives_a_file_the_byte_past_the_limit_back() { + let dir = tempfile::tempdir().expect("temporary directory"); + let path = dir.path().join("invalid.txt"); + std::fs::write(&path, b"\xC3A\n\xE0\x80A\n").expect("write fixture"); + let path = path.to_string_lossy().replace('\\', "/"); + let (result, output) = execute_captured(format!( + "{{ read -r -n 1 a; read -r b; read -r -n 1 c; read -r d; echo \"[$a][$b][$c][$d]\"; }} \ + < '{path}'" + )) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[\u{FFFD}][A][\u{FFFD}][\u{FFFD}A]\n"); } /// Contract: `mapfile -n` from a file consumes exactly the lines it diff --git a/docs/rust-porting-inventory.md b/docs/rust-porting-inventory.md index 23603eecf55..9e16e935e50 100644 --- a/docs/rust-porting-inventory.md +++ b/docs/rust-porting-inventory.md @@ -42,7 +42,7 @@ Upstream pin: `can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1` | A-PI-WALKER | crate | `crates/pi-walker@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-walker`; `crates/pi-natives/src/iofs.rs` | infra/overlap | glob, invalidateFsScanCache, walkerPoolStatus | Filesystem traversal substrate; `packages/coding-agent/src/tools/find.ts` | Recorded glob/fd/workspace outputs on fixture trees | `tools:glob`, `startup`, `tools` | S1-S5,S7 | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Integrated traversal substrate and pi-edit dependency; absorbs fs_cache policy and keeps the serial-on-pool-failure branch. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6027, https://github.com/Yeachan-Heo/gajae-code/pull/6039. | — | — | — | | A-PI-EDIT | crate | `crates/pi-edit@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-edit`; `crates/pi-natives/src/edit.rs` | (a) | `editFindMatch`, `editSeekSequence`, hashline, patch, EditSession, EditStore APIs | `packages/coding-agent/src/edit/{modes/replace,patch,apply-patch}.ts`; `edit/apply-patch/*`; `edit/{diff,normalize,streaming,notebook}.ts`; `hashline/*.ts` | Edit-benchmark TS matcher goldens including errors; per-mode goldens through 2.10 | `edit-hotspots` H01,H02,H06; `tools:edit`; `bench:edit`; `replay` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/6016 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Matcher merged via #5980 and fuzzy scoring fix #6016; edit-hotspots H01/H02 covered by the pi-edit fuzzy A/B in #6016. | — | — | `packages/coding-agent/src/edit/modes/replace.ts`; `packages/coding-agent/src/edit/streaming.ts` | A-PI-VCS | crate | `crates/pi-vcs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-vcs`; `crates/pi-natives/src/vcs.rs` | (a), size-gated (D5) | VCS read APIs | `packages/coding-agent/src/utils/git.ts`; `autoresearch/git.ts`; `status-line/git-utils.ts`; `commit/agentic/tools/git-*.ts` | Git CLI oracle over fixture repositories | `status-line`, `tools:git` | — | rejected | | | D5 perf gate: large-worktree read suite upper-CI p50 ratio was 0.826 (needs <0.80), and status was 1.20x slower. | — | — | — | -| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. Per-row bump for #6543: `read.rs`, `mapfile.rs` and the new `line_input.rs` are vendored from `can1357/oh-my-pi` commit `68fb9863b377af566d3b56aa882a964654b5760a` (upstream `f3ce813d`, `b636b6ae`, `b6e8ee12`, `68fb9863`): UTF-8 line decoding, character-counted `read -n`/`-N`, and block read-ahead that gives unread bytes back to a regular file. Local changes on top: a failed refill leaves nothing to give back, `read` polls a stream before every byte instead of trusting a `FIONREAD` count, and `-n`/`-N` count malformed UTF-8 the way bash does. | `crates/brush-builtins-vendored/src/read.rs` | — | — | +| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. Per-row bump for #6543: `read.rs`, `mapfile.rs` and the new `line_input.rs` are vendored from `can1357/oh-my-pi` commit `68fb9863b377af566d3b56aa882a964654b5760a` (upstream `f3ce813d`, `b636b6ae`, `b6e8ee12`, `68fb9863`): UTF-8 line decoding, character-counted `read -n`/`-N`, and block read-ahead that gives unread bytes back to a regular file. Local changes on top: a failed refill leaves nothing to give back, `read` polls a stream before every byte instead of trusting a `FIONREAD` count, and `-n`/`-N` count malformed UTF-8 as the line decodes it, giving a regular file the byte read past the limit back. | `crates/brush-builtins-vendored/src/read.rs` | — | — | | A-PI-BUILTINS-UTILS | crate | `pi-builtins/src/utils@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | Bash tool builtins | — | — | — | rejected | | | GNU/BSD behavior diverges; utility builtins pull C-backed onig_sys and pcre2-sys. uucore is already a pre-existing printf dependency and is not a rejection criterion. | — | — | — | | A-PI-VOICE | crate | `crates/pi-voice@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | gjc STT uses `packages/coding-agent/src/stt/recorder.ts` | — | — | — | rejected | | | opus/webrtc and dynamically loaded PulseAudio/ALSA are out of scope; gjc uses sox/ffmpeg/arecord. | — | — | — | From c74a87a4373a0f47ce9b4c9f6b9ab110ffba5373 Mon Sep 17 00:00:00 2001 From: argus Date: Mon, 12 Oct 2026 13:57:06 +0900 Subject: [PATCH 4/5] fix(shell): read one byte at a time and hold read -n/-N lookahead Addresses snowykr's review 5483596213 (F1-F4) and probepark's review 5482809198 (P2, pipe -n/-N limits) on #6572. - Remove the block read-ahead and every seek from LineInput. read and mapfile read one byte per OS read again, as before the port, so a failed mapfile assignment or a -C callback leaves the rest of a pipe for the next reader (F2), and nothing rewinds a shared file offset (F4). - When a read stops at a -n/-N limit, the byte it looked ahead at (with the backslash that escaped it) is held in brush-core's new openfiles::HeldInput. Every try_clone of an OpenFile shares it, so the next builtin read of the same open file description takes it first. A value never holds more characters than its limit, on pipes too (F1, P2). External commands do not see held bytes, as with bash's zungetc. - read takes held input by one atomic pop after the cancellation check and polls before every other byte; -t 0 reports held input as ready. - CharCount::finish counts a character left incomplete at EOF, so read -N 2 on "a\xE2" + EOF succeeds as in bash (F3). - Tests: 10 pi-shell, 5 read unit, 4 line_input unit. Inventory rows 45 and 113 updated and a keep-local row added; the changelog drops the read-ahead performance note. --- .../brush-builtins-vendored/src/line_input.rs | 269 +++++++----------- crates/brush-builtins-vendored/src/mapfile.rs | 19 +- crates/brush-builtins-vendored/src/read.rs | 209 +++++++++++--- crates/brush-core-vendored/src/ioutils.rs | 2 +- crates/brush-core-vendored/src/openfiles.rs | 153 ++++++---- .../brush-core-vendored/src/shell/history.rs | 2 +- .../brush-core-vendored/src/sys/windows/fd.rs | 4 +- crates/pi-shell/src/shell.rs | 168 ++++++++++- docs/rust-porting-inventory.md | 5 +- .../6543-bash-read-decodes-utf8.md | 4 - 10 files changed, 556 insertions(+), 279 deletions(-) diff --git a/crates/brush-builtins-vendored/src/line_input.rs b/crates/brush-builtins-vendored/src/line_input.rs index c4cf08e6591..5910e7e5582 100644 --- a/crates/brush-builtins-vendored/src/line_input.rs +++ b/crates/brush-builtins-vendored/src/line_input.rs @@ -1,13 +1,10 @@ -// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/line_input.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, a failed refill leaves no read-ahead to give back, and read can take back the bytes past its -n/-N limit. +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/line_input.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, block read-ahead and its seek-back removed: input is read one byte at a time and never seeked, and bytes a reader takes back are held with the open file description for its next reader. //! Byte input for the line-reading builtins (`read`, `mapfile`). -use std::io::{self, Read, Seek, SeekFrom}; +use std::io::{self, Read}; use brush_core::openfiles::OpenFile; -/// Bytes per read-ahead. -const BLOCK: usize = 4096; - /// ASCII control character for Ctrl+C (ETX - End of Text). pub(crate) const CTRL_C: u8 = 0x03; /// ASCII control character for Ctrl+D (EOT - End of Transmission). @@ -19,46 +16,20 @@ pub(crate) fn decode_line(line: Vec) -> String { String::from_utf8(line).unwrap_or_else(|error| String::from_utf8_lossy(error.as_bytes()).into_owned()) } -/// When a [`LineInput`] may read past the bytes it hands out. -#[derive(Clone, Copy)] -pub(crate) enum ReadAhead { - /// The caller consumes everything up to EOF, so reading ahead can never - /// take input meant for someone else. - ToEof, - /// The caller may stop mid-input: read ahead only from a regular file, - /// whose offset can be given back. - Seekable, -} - /// A shell descriptor read one byte at a time by its caller. /// -/// Where reading ahead is safe, the descriptor is read a block at a time. -/// On drop, a regular file's shared offset is seeked back over the bytes read -/// but not handed out, so the next reader of the descriptor resumes right -/// after the last one consumed (bash's `read` does the same). Elsewhere every -/// byte is its own read, so nothing past the last consumed byte is taken from -/// a pipe or terminal another command shares. +/// Every byte is its own OS read, so a builtin takes only what it consumes +/// and never seeks: another reader of the descriptor (an external command, +/// a `mapfile -C` callback, a background job) resumes right after the last +/// byte handed out. Bytes taken back are held with the open file +/// description, and its next builtin reader gets them first. pub(crate) struct LineInput { - input: OpenFile, - blocks: bool, - block: Vec, - pos: usize, + input: OpenFile, } impl LineInput { - /// Terminal input is always read byte by byte, so interactive keys - /// arrive as typed. Only a plain file handle counts as a regular file: - /// std's `Stdin` keeps a buffer of its own, which a seek on the - /// descriptor could not give back. - pub(crate) fn new(input: OpenFile, read_ahead: ReadAhead) -> Self { - let blocks = !input.is_terminal() - && match read_ahead { - ReadAhead::ToEof => true, - ReadAhead::Seekable => { - matches!(&input, OpenFile::File(file) if file.metadata().is_ok_and(|m| m.is_file())) - }, - }; - Self { input, blocks, block: Vec::new(), pos: 0 } + pub(crate) const fn new(input: OpenFile) -> Self { + Self { input } } /// The underlying descriptor, e.g. to poll it before a read that could @@ -67,86 +38,39 @@ impl LineInput { &self.input } - /// Whether reads come from a read-ahead block, never waiting on input. - pub(crate) const fn reads_ahead(&self) -> bool { - self.blocks + /// Whether input is held for the next read, without taking it. + pub(crate) fn has_held(&self) -> bool { + self.input.held_input().is_some_and(|held| !held.is_empty()) } - /// The next byte, or `None` at end of input. - pub(crate) fn next_byte(&mut self) -> io::Result> { - if !self.blocks { - let mut byte = [0_u8; 1]; - return Ok((self.input.read(&mut byte)? == 1).then_some(byte[0])); - } - if self.pos == self.block.len() { - self.block.resize(BLOCK, 0); - self.pos = 0; - let n = match self.input.read(&mut self.block) { - Ok(n) => n, - Err(error) => { - // Nothing was read into the block, and everything before it - // was handed out; leaving it at full length would make - // `give_back` seek back over bytes already consumed. - self.block.clear(); - return Err(error); - }, - }; - self.block.truncate(n); - if n == 0 { - return Ok(None); - } - } - self.pos += 1; - Ok(Some(self.block[self.pos - 1])) + /// Takes the first held byte, if any, in one step, so a concurrent + /// reader of the description cannot take the same byte. + pub(crate) fn take_held(&self) -> Option { + self.input.held_input().and_then(|held| held.pop()) } - /// Takes back the last `n` bytes handed out, so the next reader of a - /// regular file gets them. Only read-ahead from a regular file can do - /// this; a byte read from a pipe or terminal is gone, so this returns - /// `false` and changes nothing. - pub(crate) fn unread(&mut self, n: usize) -> bool { - if !self.blocks || !matches!(self.input, OpenFile::File(_)) { - return false; - } - if let Some(pos) = self.pos.checked_sub(n) { - self.pos = pos; - return true; - } - // Some of the bytes came from the previous block: seek back over this - // block and those bytes, and start over from there. - let Ok(back) = i64::try_from(self.block.len() - self.pos + n) else { - return false; - }; - if let OpenFile::File(file) = &mut self.input - && file.seek(SeekFrom::Current(-back)).is_ok() - { - self.block.clear(); - self.pos = 0; - return true; - } - false + /// One byte from the descriptor, or `None` at end of input. Never looks + /// at held input. + pub(crate) fn read_byte(&mut self) -> io::Result> { + let mut byte = [0_u8; 1]; + Ok((self.input.read(&mut byte)? == 1).then_some(byte[0])) } - /// Gives the bytes read ahead but not handed out back to a regular - /// file's shared offset, so another reader of the descriptor resumes - /// right after the last byte consumed. Bash's `mapfile` does the same - /// before each `-C` callback. Read-ahead that cannot be given back is - /// kept and handed out as usual. - pub(crate) fn give_back(&mut self) { - let unread = self.block.len() - self.pos; - if unread > 0 - && let OpenFile::File(file) = &mut self.input - && file.seek(SeekFrom::Current(-(unread as i64))).is_ok() - { - self.block.clear(); - self.pos = 0; + /// The next byte, held input first, or `None` at end of input. + pub(crate) fn next_byte(&mut self) -> io::Result> { + match self.take_held() { + Some(byte) => Ok(Some(byte)), + None => self.read_byte(), } } -} -impl Drop for LineInput { - fn drop(&mut self) { - self.give_back(); + /// Holds `bytes` ahead of any held input, so the next builtin read of + /// the description gets them first. A write-only descriptor has no + /// store; its reads fail before anything could be taken back. + pub(crate) fn unread(&self, bytes: &[u8]) { + if let Some(held) = self.input.held_input() { + held.put_back(bytes); + } } } @@ -159,75 +83,94 @@ mod tests { use super::*; - /// Regression: a refill that fails must not leave the block at full - /// length, or dropping the input seeks the shared offset back over bytes - /// that were never read (here, over bytes just written). - #[test] - fn failed_refill_gives_nothing_back() -> Result<()> { - let path = std::env::temp_dir() - .join(format!("brush-line-input-failed-refill-{}.txt", std::process::id())); - // Write-only, so every read fails. - let mut file = std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .open(&path)?; - file.write_all(&[b'a'; 5000])?; - let mut offset = file.try_clone()?; - - let mut input = LineInput::new(OpenFile::File(file), ReadAhead::Seekable); - assert!(input.reads_ahead()); - assert!(input.next_byte().is_err()); - drop(input); - - let position = offset.stream_position()?; + /// A file holding `contents`, already unlinked, and a std duplicate of + /// it that shares its offset. + fn file_with(name: &str, contents: &[u8]) -> Result<(std::fs::File, std::fs::File)> { + let path = + std::env::temp_dir().join(format!("brush-line-input-{name}-{}.txt", std::process::id())); + std::fs::write(&path, contents)?; + let file = std::fs::File::open(&path)?; + let other = file.try_clone()?; std::fs::remove_file(&path)?; - assert_eq!(position, 5000); - Ok(()) + Ok((file, other)) } - /// Contract: bytes taken back from a regular file go to the next reader - /// of the descriptor, also when they came from the previous block. + /// Regression: a reader takes only the bytes it hands out, so another + /// reader of the same open file resumes right after them while the input + /// is still in use, and nothing is rewound when it goes away. #[test] - fn unread_bytes_go_back_to_the_file() -> Result<()> { - let path = std::env::temp_dir() - .join(format!("brush-line-input-unread-{}.txt", std::process::id())); - std::fs::write(&path, vec![b'a'; BLOCK + 10])?; - let file = std::fs::File::open(&path)?; - let mut offset = file.try_clone()?; + fn a_reader_takes_only_the_bytes_it_hands_out() -> Result<()> { + let (file, mut other) = file_with("takes-only", b"abcd")?; - let mut input = LineInput::new(OpenFile::File(file), ReadAhead::Seekable); - for _ in 0..5 { - input.next_byte()?; - } - assert!(input.unread(2)); + let mut input = LineInput::new(OpenFile::from(file)); + assert_eq!(input.next_byte()?, Some(b'a')); + let mut middle = [0_u8; 2]; + std::io::Read::read_exact(&mut other, &mut middle)?; + assert_eq!(&middle, b"bc"); + assert_eq!(input.next_byte()?, Some(b'd')); drop(input); - let within_block = offset.stream_position()?; + assert_eq!(other.stream_position()?, 4); + Ok(()) + } - offset.seek(SeekFrom::Start(0))?; - let mut input = LineInput::new(OpenFile::File(offset.try_clone()?), ReadAhead::Seekable); - for _ in 0..=BLOCK { - input.next_byte()?; - } - assert!(input.unread(2)); + /// Contract: bytes taken back from a regular file are held for the next + /// reader of the open file; the shared offset is not seeked back. + #[test] + fn bytes_taken_back_from_a_file_are_held_not_seeked() -> Result<()> { + let (file, mut offset) = file_with("held-not-seeked", b"abc")?; + let file = OpenFile::from(file); + + let mut input = LineInput::new(file.try_clone()?); + assert_eq!(input.next_byte()?, Some(b'a')); + assert_eq!(input.next_byte()?, Some(b'b')); + input.unread(b"b"); drop(input); - let across_blocks = offset.stream_position()?; + assert_eq!(offset.stream_position()?, 2); - std::fs::remove_file(&path)?; - assert_eq!(within_block, 3); - assert_eq!(across_blocks, u64::try_from(BLOCK)? - 1); + let mut input = LineInput::new(file); + assert_eq!(input.next_byte()?, Some(b'b')); + assert_eq!(input.next_byte()?, Some(b'c')); + assert_eq!(input.next_byte()?, None); Ok(()) } - /// Contract: a pipe byte cannot be taken back. + /// Contract: bytes taken back from a pipe are held for the next reader, + /// in the same input and in a later one of the same pipe. #[test] - fn unread_from_a_pipe_does_nothing() -> Result<()> { + fn bytes_taken_back_from_a_pipe_are_held_for_the_next_reader() -> Result<()> { let (reader, mut writer) = std::io::pipe()?; - writer.write_all(b"ab")?; - let mut input = LineInput::new(OpenFile::PipeReader(reader), ReadAhead::Seekable); + writer.write_all(b"abc")?; + drop(writer); + let pipe = OpenFile::from(reader); + + let mut input = LineInput::new(pipe.try_clone()?); assert_eq!(input.next_byte()?, Some(b'a')); - assert!(!input.unread(1)); assert_eq!(input.next_byte()?, Some(b'b')); + input.unread(b"b"); + assert_eq!(input.next_byte()?, Some(b'b')); + input.unread(b"b"); + drop(input); + + let mut input = LineInput::new(pipe); + assert_eq!(input.next_byte()?, Some(b'b')); + assert_eq!(input.next_byte()?, Some(b'c')); + assert_eq!(input.next_byte()?, None); + Ok(()) + } + + /// Contract: bytes held later come first, each batch in input order, + /// and the descriptor is read once held input runs out. + #[test] + fn held_input_comes_first_in_input_order() -> Result<()> { + let (reader, mut writer) = std::io::pipe()?; + writer.write_all(b"d")?; + drop(writer); + let mut input = LineInput::new(OpenFile::from(reader)); + input.unread(b"c"); + input.unread(b"ab"); + for expected in [Some(b'a'), Some(b'b'), Some(b'c'), Some(b'd'), None] { + assert_eq!(input.next_byte()?, expected); + } Ok(()) } } diff --git a/crates/brush-builtins-vendored/src/mapfile.rs b/crates/brush-builtins-vendored/src/mapfile.rs index 5725daeeea8..8c3d5eb1513 100644 --- a/crates/brush-builtins-vendored/src/mapfile.rs +++ b/crates/brush-builtins-vendored/src/mapfile.rs @@ -1,10 +1,10 @@ -// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/mapfile.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, retained local array validation and synchronous file descriptor reads. +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/mapfile.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, retained local array validation and synchronous file descriptor reads, one byte at a time with no read-ahead. use std::io::Write; use brush_core::{ErrorKind, ExecutionExitCode, ExecutionResult, builtins, env, escape, variables}; use clap::Parser; -use crate::line_input::{CTRL_C, CTRL_D, LineInput, ReadAhead, decode_line}; +use crate::line_input::{CTRL_C, CTRL_D, LineInput, decode_line}; /// Read lines from standard input into an indexed array variable. #[derive(Parser)] @@ -63,7 +63,7 @@ impl builtins::Command for MapFileCommand { if let Some((_, var)) = context.shell.env().get(&self.array_var_name) { // Refused before any input is read, as in bash. Assigning an element // does not check this itself (so -O wrote into a readonly array), and - // a failure mid-loop would lose input read ahead of a pipe. + // failing at the first element would still consume its line. if var.is_readonly() { return Err(ErrorKind::ReadonlyVariable.into()); } @@ -124,14 +124,10 @@ impl MapFileCommand { }; let terminal = input_file.is_terminal(); - // Without -n or -C every byte up to EOF is consumed before anything - // else reads the descriptor, so any input is read in blocks. -n may - // stop early, and a -C callback may read the descriptor itself or - // end the loop; then only a regular file, whose offset is given - // back, is (bash's mapfile reads a pipe unbuffered too). - let read_ahead = - if max_count == 0 && self.callback.is_none() { ReadAhead::ToEof } else { ReadAhead::Seekable }; - let mut input = LineInput::new(input_file, read_ahead); + // Bytes are read one at a time, so a -C callback, a failed assignment + // or a later command finds the input right after the last line + // stored (bash's mapfile reads a pipe unbuffered too). + let mut input = LineInput::new(input_file); while max_count == 0 || entry_count < max_count { let mut line = vec![]; @@ -169,7 +165,6 @@ impl MapFileCommand { if let Some(callback) = &self.callback && (entry_count + 1).is_multiple_of(callback_group_size) { - input.give_back(); let result = run_callback(callback, array_index, &line_str, context).await?; if !result.is_normal_flow() { return Ok(Some(result)); diff --git a/crates/brush-builtins-vendored/src/read.rs b/crates/brush-builtins-vendored/src/read.rs index 62a070507f4..e2ba5850342 100644 --- a/crates/brush-builtins-vendored/src/read.rs +++ b/crates/brush-builtins-vendored/src/read.rs @@ -1,4 +1,4 @@ -// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/read.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, preserves cancellation, timeout, and readiness guards; polls a stream before every byte; counts -n/-N characters as the line decodes and gives a regular file the byte read past the limit back. +// Vendored from can1357/oh-my-pi@68fb9863b377af566d3b56aa882a964654b5760a:crates/pi-builtins/src/read.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 Stencil Labs, Inc. Modified for gajae-code: yes, preserves cancellation, timeout, and readiness guards; reads one byte at a time with no read-ahead, taking held input before polling a stream for every other byte; counts -n/-N characters as the line decodes, counting an incomplete character at EOF, and holds the input read past the limit with the open file description for the next read. use std::{ collections::VecDeque, io::Write, @@ -9,7 +9,7 @@ use brush_core::{ErrorKind, builtins, env, error, openfiles::OpenFile, variables use clap::Parser; use itertools::Itertools; -use crate::line_input::{CTRL_C, CTRL_D, LineInput, ReadAhead, decode_line}; +use crate::line_input::{CTRL_C, CTRL_D, LineInput, decode_line}; /// Exit code returned when `read` times out. /// This is 128 + SIGALRM (14) = 142, matching bash behavior. @@ -297,8 +297,8 @@ enum ReadResult { /// This separates the concerns of byte-level I/O with timeout handling /// from the higher-level logic of line building and escape processing. struct InputReader { - /// The input source: a regular file is read ahead, and what lies past - /// the line is given back when `read` returns. + /// The input source, read one byte at a time; input read past a + /// `-n`/`-N` limit is held for the next read. source: LineInput, /// Optional deadline for timeout. deadline: Option, @@ -347,17 +347,20 @@ impl InputReader { ) -> Self { let terminal = input.is_terminal(); Self { - source: LineInput::new(input, ReadAhead::Seekable), + source: LineInput::new(input), deadline: timeout.map(|t| Instant::now() + t), terminal, _term_mode: term_mode, } } - /// Checks if input is immediately available (for `-t 0`). Returns `false` if - /// an error occurs while checking for available input. + /// Checks if input is immediately available (for `-t 0`), held input + /// included, without consuming it. Returns `false` if an error occurs + /// while checking for available input. fn check_input_available(&self) -> bool { - brush_core::sys::poll::poll_for_input(self.source.input(), Duration::ZERO).unwrap_or(false) + self.source.has_held() + || brush_core::sys::poll::poll_for_input(self.source.input(), Duration::ZERO) + .unwrap_or(false) } #[cfg(unix)] @@ -418,13 +421,12 @@ impl InputReader { } /// The text of a `-n`/`-N` read whose limit fell before the last byte of - /// `line`, which took `input_len` bytes of input. A regular file takes - /// those bytes back for the next reader. A pipe or terminal cannot, so - /// the byte stays in the line rather than being lost. - fn line_before_last_byte(&mut self, mut line: Vec, input_len: usize) -> String { - if self.source.unread(input_len) { - line.pop(); - } + /// `line`. The `input` that byte took (with the backslash that escaped + /// it) is held for the next read of the open file description, so the + /// value never exceeds the limit and no input is lost. + fn line_before_last_byte(&self, mut line: Vec, input: &[u8]) -> String { + line.pop(); + self.source.unread(input); decode_line(line) } @@ -433,18 +435,23 @@ impl InputReader { where F: Fn() -> bool, { - // A stream is polled before every byte. A count of bytes ready, taken - // once and trusted for later bytes, goes stale when another reader of - // a shared pipe drains it; the read would then block where neither - // `-t` nor cancellation is checked. - if !self.source.reads_ahead() - && let Some(event) = self.wait_for_input(is_cancelled)? - { - return Ok(event); - } ensure_not_cancelled(is_cancelled)?; - let Some(byte) = self.source.next_byte()? else { - return Ok(InputEvent::Eof); + // Held input is taken by one atomic pop and never waits. Any other byte + // is polled first: a ready count goes stale when another reader of a + // shared pipe drains it, and the read would then block where neither + // `-t` nor cancellation is checked. Never test for held input and then + // read. + let byte = if let Some(byte) = self.source.take_held() { + byte + } else { + if let Some(event) = self.wait_for_input(is_cancelled)? { + return Ok(event); + } + ensure_not_cancelled(is_cancelled)?; + let Some(byte) = self.source.read_byte()? else { + return Ok(InputEvent::Eof); + }; + byte }; // Control keys only mean something on a terminal; in a file or pipe @@ -508,6 +515,17 @@ impl CharCount { Limit::NotReached } + /// At EOF the pending start of a character decodes to one U+FFFD, so it + /// counts as one character. Returns whether that reached `limit`. + fn finish(&mut self, limit: Option) -> bool { + if self.len == 0 { + return false; + } + self.len = 0; + self.chars += 1; + limit.is_some_and(|limit| self.chars >= limit) + } + /// The length of the first pending character once it is known: a /// complete character, or an invalid sequence that decodes to one /// U+FFFD. `None` while the pending bytes can still become a character. @@ -546,6 +564,11 @@ where match event { InputEvent::Eof => { + // An incomplete character that reaches the limit completes the + // read, as in bash. + if count.finish(config.char_limit) { + return Ok(ReadResult::Line(decode_line(line))); + } // Bash discards pending backslash on EOF. return Ok(ReadResult::Eof((!line.is_empty()).then(|| decode_line(line)))); }, @@ -591,7 +614,9 @@ where Limit::NotReached => continue, Limit::Reached => return Ok(ReadResult::Line(decode_line(line))), // The backslash before the byte lies past the limit too. - Limit::Passed => return Ok(ReadResult::Line(reader.line_before_last_byte(line, 2))), + Limit::Passed => { + return Ok(ReadResult::Line(reader.line_before_last_byte(line, &[BACKSLASH, byte]))); + }, } } @@ -617,7 +642,7 @@ where match count.push(byte, config.char_limit) { Limit::NotReached => {}, Limit::Reached => return Ok(ReadResult::Line(decode_line(line))), - Limit::Passed => return Ok(ReadResult::Line(reader.line_before_last_byte(line, 1))), + Limit::Passed => return Ok(ReadResult::Line(reader.line_before_last_byte(line, &[byte]))), } }, } @@ -1067,11 +1092,10 @@ mod tests { let (reader, mut writer) = std::io::pipe()?; let other = reader.try_clone()?; writer.write_all(b"ab")?; - let input_file = brush_core::openfiles::OpenFile::Stream(Box::new(DrainedAfterFirstByte { - reader, - other, - drained: false, - })); + let input_file = brush_core::openfiles::OpenFile::Stream( + Box::new(DrainedAfterFirstByte { reader, other, drained: false }), + brush_core::openfiles::HeldInput::default(), + ); let result = command.read_line(input_file, Vec::::new(), Some(Duration::from_millis(200)), || false)?; @@ -1079,4 +1103,123 @@ mod tests { drop(writer); Ok(()) } + + /// Contract: a `-n`/`-N` read of a pipe holds the byte read past its + /// limit, and the next read of the same pipe gets it first. + #[cfg(unix)] + #[test] + #[expect(clippy::panic_in_result_fn)] + fn test_read_line_count_holds_the_byte_past_the_limit() -> anyhow::Result<()> { + use std::io::Write; + + let (reader, mut writer) = std::io::pipe()?; + writer.write_all(b"\xC3AB\n\xC3AB\n")?; + drop(writer); + let pipe = OpenFile::from(reader); + let plain = ReadCommand { raw_mode: true, ..test_command() }; + let count = ReadCommand { raw_mode: true, return_after_n_chars: Some(1), ..test_command() }; + let exact = + ReadCommand { raw_mode: true, return_after_n_chars_no_delimiter: Some(1), ..test_command() }; + + for limited in [count, exact] { + let result = limited.read_line(pipe.try_clone()?, Vec::::new(), None, || false)?; + assert!(matches!(result, ReadResult::Line(ref line) if line == "\u{FFFD}")); + let result = plain.read_line(pipe.try_clone()?, Vec::::new(), None, || false)?; + assert!(matches!(result, ReadResult::Line(ref line) if line == "AB")); + } + Ok(()) + } + + /// Contract: `-t 0` reports held input as ready without taking it. + #[cfg(unix)] + #[test] + #[expect(clippy::panic_in_result_fn)] + fn test_read_line_t_0_reports_held_input_ready() -> anyhow::Result<()> { + let command = test_command(); + let (reader, _writer) = std::io::pipe()?; + let pipe = OpenFile::from(reader); + let held = pipe.held_input().expect("a pipe holds input").clone(); + + let result = command.read_line(pipe.try_clone()?, Vec::::new(), Some(Duration::ZERO), || false)?; + assert!(matches!(result, ReadResult::InputNotReady)); + held.put_back(b"A"); + let result = command.read_line(pipe, Vec::::new(), Some(Duration::ZERO), || false)?; + assert!(matches!(result, ReadResult::InputReady)); + assert_eq!(held.pop(), Some(b'A')); + Ok(()) + } + + /// Contract: a read cancelled before it takes a byte leaves held input + /// in place. + #[cfg(unix)] + #[test] + #[expect(clippy::panic_in_result_fn)] + fn test_read_line_cancelled_leaves_held_input() -> anyhow::Result<()> { + let command = test_command(); + let (reader, _writer) = std::io::pipe()?; + let pipe = OpenFile::from(reader); + let held = pipe.held_input().expect("a pipe holds input").clone(); + held.put_back(b"A"); + + let err = command + .read_line(pipe, Vec::::new(), None, || true) + .expect_err("cancelled state must interrupt read builtin"); + assert_eq!(err.to_string(), "interrupted"); + assert_eq!(held.pop(), Some(b'A')); + Ok(()) + } + + /// Contract: held input is served without waiting on the descriptor; + /// the deadline applies once it runs out. + #[cfg(unix)] + #[test] + #[expect(clippy::panic_in_result_fn)] + fn test_read_line_serves_held_input_before_waiting() -> anyhow::Result<()> { + let command = test_command(); + let (reader, _writer) = std::io::pipe()?; + let pipe = OpenFile::from(reader); + pipe.held_input().expect("a pipe holds input").put_back(b"A"); + + let result = + command.read_line(pipe, Vec::::new(), Some(Duration::from_millis(100)), || false)?; + assert!(matches!(result, ReadResult::TimedOut(Some(ref line)) if line == "A")); + Ok(()) + } + + /// Regression: when another reader of the description takes the held + /// byte between this read's checks, the read must poll before reading, + /// so it times out instead of blocking while the writer stays open. + #[cfg(unix)] + #[test] + #[expect(clippy::panic_in_result_fn)] + fn test_read_line_times_out_when_another_reader_drains_held_input() -> anyhow::Result<()> { + let (reader, writer) = std::io::pipe()?; + let a = OpenFile::from(reader); + a.held_input().expect("a pipe holds input").put_back(b"A"); + let b = a.try_clone()?; + let drain = b.held_input().expect("a pipe holds input").clone(); + + let (sender, receiver) = std::sync::mpsc::channel(); + let thread = std::thread::spawn(move || { + // The cancellation check runs right before the pop, so the other + // reader drains the byte exactly there. + let result = test_command().read_line( + a, + Vec::::new(), + Some(Duration::from_millis(200)), + move || { + drain.pop(); + false + }, + ); + let _ = sender.send(matches!(result, Ok(ReadResult::TimedOut(None)))); + }); + + let outcome = receiver.recv_timeout(Duration::from_secs(5)); + drop(writer); + let _ = thread.join(); + assert_eq!(outcome, Ok(true), "read must time out, not block or take the drained byte"); + assert!(b.held_input().expect("a pipe holds input").is_empty()); + Ok(()) + } } diff --git a/crates/brush-core-vendored/src/ioutils.rs b/crates/brush-core-vendored/src/ioutils.rs index 011af7c31b7..b8211380e2d 100644 --- a/crates/brush-core-vendored/src/ioutils.rs +++ b/crates/brush-core-vendored/src/ioutils.rs @@ -71,6 +71,6 @@ impl std::io::Write for FailingReaderWriter { impl From for openfiles::OpenFile { fn from(frw: FailingReaderWriter) -> Self { - Self::Stream(Box::new(frw)) + Self::Stream(Box::new(frw), openfiles::HeldInput::default()) } } diff --git a/crates/brush-core-vendored/src/openfiles.rs b/crates/brush-core-vendored/src/openfiles.rs index 52de65f189c..a7122ab6519 100644 --- a/crates/brush-core-vendored/src/openfiles.rs +++ b/crates/brush-core-vendored/src/openfiles.rs @@ -1,6 +1,11 @@ //! Managing files open within a shell instance. -use std::{collections::HashMap, io::IsTerminal, process::Stdio}; +use std::{ + collections::{HashMap, VecDeque}, + io::IsTerminal, + process::Stdio, + sync::{Arc, Mutex, MutexGuard, PoisonError}, +}; use crate::{ShellFd, error, ioutils, sys}; @@ -24,22 +29,57 @@ pub trait Stream: std::io::Read + std::io::Write + Send + Sync { fn try_borrow_as_fd(&self) -> Result, error::Error>; } +/// Input that a builtin read from an open file description past what it +/// consumed, like bash's `zungetc` pushback. The next builtin read of the +/// description takes it first; external commands never see it. +/// +/// Every duplicate made by [`OpenFile::try_clone`] shares the store, and it +/// ends with the last duplicate. Identity is the `OpenFile` value and its +/// `try_clone` lineage, so an `OpenFile` built from a raw descriptor starts +/// with an empty store. +#[derive(Clone, Default)] +pub struct HeldInput(Arc>>); + +impl HeldInput { + /// Takes the first held byte, if any. + pub fn pop(&self) -> Option { + self.lock().pop_front() + } + + /// Holds `bytes` ahead of anything already held, in input order. + pub fn put_back(&self, bytes: &[u8]) { + let mut held = self.lock(); + for &byte in bytes.iter().rev() { + held.push_front(byte); + } + } + + /// Whether nothing is held. + pub fn is_empty(&self) -> bool { + self.lock().is_empty() + } + + fn lock(&self) -> MutexGuard<'_, VecDeque> { + self.0.lock().unwrap_or_else(PoisonError::into_inner) + } +} + /// Represents a file open in a shell context. pub enum OpenFile { /// The original standard input this process was started with. - Stdin(std::io::Stdin), + Stdin(std::io::Stdin, HeldInput), /// The original standard output this process was started with. Stdout(std::io::Stdout), /// The original standard error this process was started with. Stderr(std::io::Stderr), /// A file open for reading or writing. - File(std::fs::File), + File(std::fs::File, HeldInput), /// A read end of a pipe. - PipeReader(std::io::PipeReader), + PipeReader(std::io::PipeReader, HeldInput), /// A write end of a pipe. PipeWriter(std::io::PipeWriter), /// A custom stream. - Stream(Box), + Stream(Box, HeldInput), } #[cfg(feature = "serde")] @@ -49,13 +89,13 @@ impl serde::Serialize for OpenFile { S: serde::Serializer, { match self { - Self::Stdin(_) => serializer.serialize_str("stdin"), + Self::Stdin(..) => serializer.serialize_str("stdin"), Self::Stdout(_) => serializer.serialize_str("stdout"), Self::Stderr(_) => serializer.serialize_str("stderr"), - Self::File(_) => serializer.serialize_str("file"), - Self::PipeReader(_) => serializer.serialize_str("pipe_reader"), + Self::File(..) => serializer.serialize_str("file"), + Self::PipeReader(..) => serializer.serialize_str("pipe_reader"), Self::PipeWriter(_) => serializer.serialize_str("pipe_writer"), - Self::Stream(_) => serializer.serialize_str("stream"), + Self::Stream(..) => serializer.serialize_str("stream"), } } } @@ -85,7 +125,7 @@ impl<'de> serde::Deserialize<'de> for OpenFile { /// Returns an open file that will discard all I/O. pub fn null() -> Result { let file = sys::fs::open_null_file()?; - Ok(OpenFile::File(file)) + Ok(file.into()) } impl Clone for OpenFile { @@ -101,13 +141,13 @@ impl Clone for OpenFile { impl std::fmt::Display for OpenFile { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Stdin(_) => write!(f, "stdin"), + Self::Stdin(..) => write!(f, "stdin"), Self::Stdout(_) => write!(f, "stdout"), Self::Stderr(_) => write!(f, "stderr"), - Self::File(_) => write!(f, "file"), - Self::PipeReader(_) => write!(f, "pipe reader"), + Self::File(..) => write!(f, "file"), + Self::PipeReader(..) => write!(f, "pipe reader"), Self::PipeWriter(_) => write!(f, "pipe writer"), - Self::Stream(_) => write!(f, "stream"), + Self::Stream(..) => write!(f, "stream"), } } } @@ -116,13 +156,13 @@ impl OpenFile { /// Tries to duplicate the open file. pub fn try_clone(&self) -> Result { let result = match self { - Self::Stdin(_) => std::io::stdin().into(), + Self::Stdin(_, held) => Self::Stdin(std::io::stdin(), held.clone()), Self::Stdout(_) => std::io::stdout().into(), Self::Stderr(_) => std::io::stderr().into(), - Self::File(f) => f.try_clone()?.into(), - Self::PipeReader(f) => f.try_clone()?.into(), + Self::File(f, held) => Self::File(f.try_clone()?, held.clone()), + Self::PipeReader(f, held) => Self::PipeReader(f.try_clone()?, held.clone()), Self::PipeWriter(f) => f.try_clone()?.into(), - Self::Stream(s) => Self::Stream(s.clone_box()), + Self::Stream(s, held) => Self::Stream(s.clone_box(), held.clone()), }; Ok(result) @@ -134,13 +174,13 @@ impl OpenFile { use std::os::fd::AsFd as _; match self { - Self::Stdin(f) => Ok(f.as_fd().try_clone_to_owned()?), + Self::Stdin(f, _) => Ok(f.as_fd().try_clone_to_owned()?), Self::Stdout(f) => Ok(f.as_fd().try_clone_to_owned()?), Self::Stderr(f) => Ok(f.as_fd().try_clone_to_owned()?), - Self::File(f) => Ok(f.into()), - Self::PipeReader(r) => Ok(std::os::fd::OwnedFd::from(r)), + Self::File(f, _) => Ok(f.into()), + Self::PipeReader(r, _) => Ok(std::os::fd::OwnedFd::from(r)), Self::PipeWriter(w) => Ok(std::os::fd::OwnedFd::from(w)), - Self::Stream(s) => s.try_clone_to_owned(), + Self::Stream(s, _) => s.try_clone_to_owned(), } } @@ -155,13 +195,13 @@ impl OpenFile { use std::os::fd::AsFd as _; match self { - Self::Stdin(f) => Ok(f.as_fd()), + Self::Stdin(f, _) => Ok(f.as_fd()), Self::Stdout(f) => Ok(f.as_fd()), Self::Stderr(f) => Ok(f.as_fd()), - Self::File(f) => Ok(f.as_fd()), - Self::PipeReader(r) => Ok(r.as_fd()), + Self::File(f, _) => Ok(f.as_fd()), + Self::PipeReader(r, _) => Ok(r.as_fd()), Self::PipeWriter(w) => Ok(w.as_fd()), - Self::Stream(s) => s.try_borrow_as_fd(), + Self::Stream(s, _) => s.try_borrow_as_fd(), } } @@ -175,13 +215,13 @@ impl OpenFile { #[cfg(not(unix))] { let stdio = match self { - Self::Stdin(_) => Stdio::inherit(), + Self::Stdin(..) => Stdio::inherit(), Self::Stdout(_) => Stdio::inherit(), Self::Stderr(_) => Stdio::inherit(), - Self::File(f) => f.into(), - Self::PipeReader(f) => f.into(), + Self::File(f, _) => f.into(), + Self::PipeReader(f, _) => f.into(), Self::PipeWriter(f) => f.into(), - Self::Stream(_) => return Err(error::ErrorKind::CannotConvertToNativeFd.into()), + Self::Stream(..) => return Err(error::ErrorKind::CannotConvertToNativeFd.into()), }; Ok(stdio) } @@ -189,20 +229,31 @@ impl OpenFile { pub(crate) fn is_dir(&self) -> bool { match self { - Self::Stdin(_) | Self::Stdout(_) | Self::Stderr(_) => false, - Self::File(file) => file.metadata().is_ok_and(|m| m.is_dir()), - Self::PipeReader(_) | Self::PipeWriter(_) | Self::Stream(_) => false, + Self::Stdin(..) | Self::Stdout(_) | Self::Stderr(_) => false, + Self::File(file, _) => file.metadata().is_ok_and(|m| m.is_dir()), + Self::PipeReader(..) | Self::PipeWriter(_) | Self::Stream(..) => false, } } /// Checks if the open file is associated with a terminal. pub fn is_terminal(&self) -> bool { match self { - Self::Stdin(f) => f.is_terminal(), + Self::Stdin(f, _) => f.is_terminal(), Self::Stdout(f) => f.is_terminal(), Self::Stderr(f) => f.is_terminal(), - Self::File(f) => f.is_terminal(), - Self::PipeReader(_) | Self::PipeWriter(_) | Self::Stream(_) => false, + Self::File(f, _) => f.is_terminal(), + Self::PipeReader(..) | Self::PipeWriter(_) | Self::Stream(..) => false, + } + } + + /// The input held for the next builtin read of this open file + /// description; `None` for a write-only file. + pub const fn held_input(&self) -> Option<&HeldInput> { + match self { + Self::Stdin(_, held) | Self::File(_, held) | Self::PipeReader(_, held) | Self::Stream(_, held) => { + Some(held) + }, + Self::Stdout(_) | Self::Stderr(_) | Self::PipeWriter(_) => None, } } } @@ -210,7 +261,7 @@ impl OpenFile { impl From for OpenFile { /// Creates an `OpenFile` from standard input. fn from(stdin: std::io::Stdin) -> Self { - Self::Stdin(stdin) + Self::Stdin(stdin, HeldInput::default()) } } @@ -230,13 +281,13 @@ impl From for OpenFile { impl From for OpenFile { fn from(file: std::fs::File) -> Self { - Self::File(file) + Self::File(file, HeldInput::default()) } } impl From for OpenFile { fn from(reader: std::io::PipeReader) -> Self { - Self::PipeReader(reader) + Self::PipeReader(reader, HeldInput::default()) } } @@ -249,19 +300,19 @@ impl From for OpenFile { impl std::io::Read for OpenFile { fn read(&mut self, buf: &mut [u8]) -> std::io::Result { match self { - Self::Stdin(f) => f.read(buf), + Self::Stdin(f, _) => f.read(buf), Self::Stdout(_) => { Err(std::io::Error::other(error::ErrorKind::OpenFileNotReadable("stdout"))) }, Self::Stderr(_) => { Err(std::io::Error::other(error::ErrorKind::OpenFileNotReadable("stderr"))) }, - Self::File(f) => f.read(buf), - Self::PipeReader(reader) => reader.read(buf), + Self::File(f, _) => f.read(buf), + Self::PipeReader(reader, _) => reader.read(buf), Self::PipeWriter(_) => { Err(std::io::Error::other(error::ErrorKind::OpenFileNotReadable("pipe writer"))) }, - Self::Stream(s) => s.read(buf), + Self::Stream(s, _) => s.read(buf), } } } @@ -269,29 +320,29 @@ impl std::io::Read for OpenFile { impl std::io::Write for OpenFile { fn write(&mut self, buf: &[u8]) -> std::io::Result { match self { - Self::Stdin(_) => { + Self::Stdin(..) => { Err(std::io::Error::other(error::ErrorKind::OpenFileNotWritable("stdin"))) }, Self::Stdout(f) => f.write(buf), Self::Stderr(f) => f.write(buf), - Self::File(f) => f.write(buf), - Self::PipeReader(_) => { + Self::File(f, _) => f.write(buf), + Self::PipeReader(..) => { Err(std::io::Error::other(error::ErrorKind::OpenFileNotWritable("pipe reader"))) }, Self::PipeWriter(writer) => writer.write(buf), - Self::Stream(s) => s.write(buf), + Self::Stream(s, _) => s.write(buf), } } fn flush(&mut self) -> std::io::Result<()> { match self { - Self::Stdin(_) => Ok(()), + Self::Stdin(..) => Ok(()), Self::Stdout(f) => f.flush(), Self::Stderr(f) => f.flush(), - Self::File(f) => f.flush(), - Self::PipeReader(_) => Ok(()), + Self::File(f, _) => f.flush(), + Self::PipeReader(..) => Ok(()), Self::PipeWriter(writer) => writer.flush(), - Self::Stream(s) => s.flush(), + Self::Stream(s, _) => s.flush(), } } } diff --git a/crates/brush-core-vendored/src/shell/history.rs b/crates/brush-core-vendored/src/shell/history.rs index d11298a7caa..c7f1ab12431 100644 --- a/crates/brush-core-vendored/src/shell/history.rs +++ b/crates/brush-core-vendored/src/shell/history.rs @@ -18,7 +18,7 @@ impl crate::Shell { let mut history_file = self.open_file(&options, history_path, &self.default_exec_params())?; // Check on the file's size. - if let openfiles::OpenFile::File(file) = &mut history_file { + if let openfiles::OpenFile::File(file, _) = &mut history_file { let file_metadata = file.metadata()?; let file_size = file_metadata.len(); diff --git a/crates/brush-core-vendored/src/sys/windows/fd.rs b/crates/brush-core-vendored/src/sys/windows/fd.rs index 593d0657755..50c62d5ca47 100644 --- a/crates/brush-core-vendored/src/sys/windows/fd.rs +++ b/crates/brush-core-vendored/src/sys/windows/fd.rs @@ -6,7 +6,7 @@ use crate::{ShellFd, openfiles}; /// the current process. pub fn try_iter_open_fds() -> impl Iterator { vec![ - (openfiles::OpenFiles::STDIN_FD, openfiles::OpenFile::Stdin(std::io::stdin())), + (openfiles::OpenFiles::STDIN_FD, openfiles::OpenFile::from(std::io::stdin())), (openfiles::OpenFiles::STDOUT_FD, openfiles::OpenFile::Stdout(std::io::stdout())), (openfiles::OpenFiles::STDERR_FD, openfiles::OpenFile::Stderr(std::io::stderr())), ] @@ -18,7 +18,7 @@ pub fn try_iter_open_fds() -> impl Iterator Option { match fd { - openfiles::OpenFiles::STDIN_FD => Some(openfiles::OpenFile::Stdin(std::io::stdin())), + openfiles::OpenFiles::STDIN_FD => Some(openfiles::OpenFile::from(std::io::stdin())), openfiles::OpenFiles::STDOUT_FD => Some(openfiles::OpenFile::Stdout(std::io::stdout())), openfiles::OpenFiles::STDERR_FD => Some(openfiles::OpenFile::Stderr(std::io::stderr())), _ => None, diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index df0287904b2..da63a2fbdbb 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -3708,9 +3708,8 @@ mod tests { /// Contract: `read -n` counts characters as the line decodes them, so /// each invalid sequence, replaced by one U+FFFD, is one character. A lead - /// byte is known to be invalid only once the next byte arrives, and a - /// pipe cannot take that byte back: when it lies past the limit it stays - /// in the variable rather than being lost. + /// byte is known to be invalid only once the next byte arrives; when that + /// byte lies past the limit it is held for the next read of the input. #[tokio::test(flavor = "multi_thread")] async fn read_count_counts_invalid_bytes_as_they_decode() { let (result, output) = execute_captured( @@ -3721,14 +3720,14 @@ mod tests { ) .await; assert_eq!(result.exit_code, Some(0), "{output:?}"); - assert_eq!(output, "[\u{FFFD}][A]\n[\u{FFFD}A][B]\n[\u{FFFD}\u{FFFD}][A]\n"); + assert_eq!(output, "[\u{FFFD}][A]\n[\u{FFFD}A][B]\n[\u{FFFD}][\u{FFFD}A]\n"); } - /// Contract: from a regular file, the byte that showed a lead byte to be - /// invalid goes back to the file when it lies past the `-n` limit, so - /// the next reader gets it. + /// Contract: from a regular file too, the byte that showed a lead byte + /// to be invalid is held for the next read when it lies past the `-n` + /// limit; nothing seeks. #[tokio::test(flavor = "multi_thread")] - async fn read_count_gives_a_file_the_byte_past_the_limit_back() { + async fn read_count_holds_a_file_byte_past_the_limit_for_the_next_read() { let dir = tempfile::tempdir().expect("temporary directory"); let path = dir.path().join("invalid.txt"); std::fs::write(&path, b"\xC3A\n\xE0\x80A\n").expect("write fixture"); @@ -3742,6 +3741,155 @@ mod tests { assert_eq!(output, "[\u{FFFD}][A][\u{FFFD}][\u{FFFD}A]\n"); } + /// Contract: a `-n`/`-N` value never holds more characters than its + /// limit, also from a pipe: the byte read past the limit is held, and + /// the next `read` of the pipe gets it first. + #[tokio::test(flavor = "multi_thread")] + async fn read_count_holds_the_byte_past_the_limit_for_the_next_read() { + let (result, output) = execute_captured( + "printf '\\303AB\\n' | { read -r -n 1 x; read -r y; echo \"[$x][$y]\"; }; printf \ + '\\303AB\\n' | { read -r -N 1 x; read -r y; echo \"[$x][$y]\"; }; printf \ + '\\342\\202A\\n' | { read -r -n 1 x; read -r y; echo \"[$x][$y]\"; }" + .to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[\u{FFFD}][AB]\n[\u{FFFD}][AB]\n[\u{FFFD}][A]\n"); + } + + /// Contract: when the byte past the limit was escaped, the backslash + /// before it is held too, so the next read sees both input bytes. + #[tokio::test(flavor = "multi_thread")] + async fn read_count_holds_both_bytes_of_an_escape_past_the_limit() { + let (result, output) = execute_captured( + "printf '\\303\\134\\134B\\n' | { read -n 1 x; read -r y; printf '[%s][%s]\\n' \"$x\" \ + \"$y\"; }" + .to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[\u{FFFD}][\\\\B]\n"); + } + + /// Contract: held input belongs to the input it was read from, not to a + /// descriptor number: a here-string read in between does not see it, + /// and the next read of the pipe does. + #[tokio::test(flavor = "multi_thread")] + async fn read_held_input_stays_with_its_input_not_the_descriptor_number() { + let (result, output) = execute_captured( + "printf '\\303A' | { read -r -n 1 x; read -r y <<< Z; read -r w; echo \"[$x][$y][$w]\"; }" + .to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[\u{FFFD}][Z][A]\n"); + } + + /// Contract: `mapfile` reads held input first and decodes each line as + /// UTF-8, replacing invalid sequences. + #[tokio::test(flavor = "multi_thread")] + async fn mapfile_reads_held_input_first_and_decodes_utf8() { + let (result, output) = execute_captured( + "printf '\\303AB\\n\\303\\251\\n\\303A\\n' | { read -r -n 1 x; mapfile -t rest; echo \ + \"[$x][${rest[*]}]\"; }" + .to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[\u{FFFD}][AB é \u{FFFD}A]\n"); + } + + /// Contract: a subshell shares the held input of the pipe it reads, so + /// a held byte is consumed once. + #[tokio::test(flavor = "multi_thread")] + async fn read_held_input_is_consumed_once_across_a_subshell() { + let (result, output) = execute_captured( + "printf '\\303AB\\n' | { read -r -n 1 x; ( read -r y; echo \"sub[$y]\" ); read -r z; \ + echo \"[$x][$z]\"; }" + .to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "sub[AB]\n[\u{FFFD}][]\n"); + } + + /// Regression: `mapfile` reads a pipe only up to each delimiter, so when + /// assigning a line fails the rest of the pipe stays for the next reader. + #[tokio::test(flavor = "multi_thread")] + async fn mapfile_assignment_failure_leaves_the_rest_of_a_pipe() { + let (_, output) = execute_captured( + "printf 'a\\nb\\nc\\n' | { mapfile -O 1 EPOCHSECONDS; echo \"rc=$?\"; cat; }".to_owned(), + ) + .await; + assert!(output.contains("not yet implemented"), "{output:?}"); + assert!(output.ends_with("rc=99\nb\nc\n"), "{output:?}"); + } + + /// Contract: at end of input, a character left incomplete decodes to one + /// U+FFFD and counts toward `-n`/`-N`, so reaching the limit with it is + /// success, as in bash; a short value is still end of input. + #[tokio::test(flavor = "multi_thread")] + async fn read_count_counts_an_incomplete_character_at_eof() { + let (result, output) = execute_captured( + "printf 'a\\342' | { read -r -N 2 x; echo \"rc=$? [$x]\"; }; printf 'a\\342' | { read -r \ + -n 2 x; echo \"rc=$? [$x]\"; }; printf 'a\\342' | { read -r -n 3 x; echo \"rc=$? \ + [$x]\"; }; printf '\\342' | { read -r -n 1 x; echo \"rc=$? [$x]\"; }; printf 'ab' | { \ + read -r -N 3 x; echo \"rc=$? [$x]\"; }; printf 'a\\342' | { read -r -N 3 x; echo \ + \"rc=$? [$x]\"; }; printf 'a\\342' | { read -r x; echo \"rc=$? [$x]\"; }" + .to_owned(), + ) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!( + output, + "rc=0 [a\u{FFFD}]\nrc=0 [a\u{FFFD}]\nrc=1 [a\u{FFFD}]\nrc=0 [\u{FFFD}]\nrc=1 [ab]\nrc=1 \ + [a\u{FFFD}]\nrc=1 [a\u{FFFD}]\n" + ); + } + + /// Contract: `read -t 0` reports input that is ready without reading it. + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread")] + async fn read_t_0_reports_ready_input() { + let (result, output) = execute_captured("read -t 0 <<< x; echo \"rc=$?\"".to_owned()).await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "rc=0\n"); + } + + /// Contract: a byte held from a regular file is not given back to the + /// file, so an external command reading next does not see it (as with + /// bash's pushback); the next builtin read does. + #[tokio::test(flavor = "multi_thread")] + async fn read_count_holds_a_file_byte_that_external_readers_do_not_see() { + let dir = tempfile::tempdir().expect("temporary directory"); + let path = dir.path().join("invalid.txt"); + std::fs::write(&path, b"\xC3AB\n").expect("write fixture"); + let path = path.to_string_lossy().replace('\\', "/"); + let (result, output) = execute_captured(format!( + "{{ read -r -n 1 x; cat; read -r y; printf '[%s][%s]\\n' \"$x\" \"$y\"; }} < '{path}'" + )) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "B\n[\u{FFFD}][A]\n"); + } + + /// Contract: held input belongs to one open of a file; opening the same + /// file again starts from its beginning and does not take it. + #[tokio::test(flavor = "multi_thread")] + async fn read_from_a_reopened_file_does_not_see_held_input() { + let dir = tempfile::tempdir().expect("temporary directory"); + let path = dir.path().join("invalid.txt"); + std::fs::write(&path, b"\xC3AB\n").expect("write fixture"); + let path = path.to_string_lossy().replace('\\', "/"); + let (result, output) = execute_captured(format!( + "{{ read -r -n 1 x; read -r y < '{path}'; read -r z; printf '[%s][%s][%s]\\n' \"$x\" \ + \"$y\" \"$z\"; }} < '{path}'" + )) + .await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[\u{FFFD}][\u{FFFD}AB][AB]\n"); + } + /// Contract: `mapfile -n` from a file consumes exactly the lines it /// stores; a later reader of the descriptor gets the rest. #[tokio::test(flavor = "multi_thread")] @@ -3773,8 +3921,8 @@ mod tests { assert_eq!(output, "cb:0:1:2\ncb:1:3:4\narr=1 3\n"); } - /// Contract: the same holds for a regular file, whose read-ahead mapfile - /// gives back to the shared offset before each callback. + /// Contract: the same holds for a regular file: mapfile reads only up to + /// each delimiter. #[tokio::test(flavor = "multi_thread")] async fn mapfile_callback_reads_the_next_line_of_a_file() { let dir = tempfile::tempdir().expect("temporary directory"); diff --git a/docs/rust-porting-inventory.md b/docs/rust-porting-inventory.md index 9e16e935e50..474441a9b4d 100644 --- a/docs/rust-porting-inventory.md +++ b/docs/rust-porting-inventory.md @@ -42,7 +42,7 @@ Upstream pin: `can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1` | A-PI-WALKER | crate | `crates/pi-walker@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-walker`; `crates/pi-natives/src/iofs.rs` | infra/overlap | glob, invalidateFsScanCache, walkerPoolStatus | Filesystem traversal substrate; `packages/coding-agent/src/tools/find.ts` | Recorded glob/fd/workspace outputs on fixture trees | `tools:glob`, `startup`, `tools` | S1-S5,S7 | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Integrated traversal substrate and pi-edit dependency; absorbs fs_cache policy and keeps the serial-on-pool-failure branch. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6027, https://github.com/Yeachan-Heo/gajae-code/pull/6039. | — | — | — | | A-PI-EDIT | crate | `crates/pi-edit@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-edit`; `crates/pi-natives/src/edit.rs` | (a) | `editFindMatch`, `editSeekSequence`, hashline, patch, EditSession, EditStore APIs | `packages/coding-agent/src/edit/{modes/replace,patch,apply-patch}.ts`; `edit/apply-patch/*`; `edit/{diff,normalize,streaming,notebook}.ts`; `hashline/*.ts` | Edit-benchmark TS matcher goldens including errors; per-mode goldens through 2.10 | `edit-hotspots` H01,H02,H06; `tools:edit`; `bench:edit`; `replay` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/6016 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Matcher merged via #5980 and fuzzy scoring fix #6016; edit-hotspots H01/H02 covered by the pi-edit fuzzy A/B in #6016. | — | — | `packages/coding-agent/src/edit/modes/replace.ts`; `packages/coding-agent/src/edit/streaming.ts` | A-PI-VCS | crate | `crates/pi-vcs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-vcs`; `crates/pi-natives/src/vcs.rs` | (a), size-gated (D5) | VCS read APIs | `packages/coding-agent/src/utils/git.ts`; `autoresearch/git.ts`; `status-line/git-utils.ts`; `commit/agentic/tools/git-*.ts` | Git CLI oracle over fixture repositories | `status-line`, `tools:git` | — | rejected | | | D5 perf gate: large-worktree read suite upper-CI p50 ratio was 0.826 (needs <0.80), and status was 1.20x slower. | — | — | — | -| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. Per-row bump for #6543: `read.rs`, `mapfile.rs` and the new `line_input.rs` are vendored from `can1357/oh-my-pi` commit `68fb9863b377af566d3b56aa882a964654b5760a` (upstream `f3ce813d`, `b636b6ae`, `b6e8ee12`, `68fb9863`): UTF-8 line decoding, character-counted `read -n`/`-N`, and block read-ahead that gives unread bytes back to a regular file. Local changes on top: a failed refill leaves nothing to give back, `read` polls a stream before every byte instead of trusting a `FIONREAD` count, and `-n`/`-N` count malformed UTF-8 as the line decodes it, giving a regular file the byte read past the limit back. | `crates/brush-builtins-vendored/src/read.rs` | — | — | +| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. Per-row bump for #6543: `read.rs`, `mapfile.rs` and the new `line_input.rs` are vendored from `can1357/oh-my-pi` commit `68fb9863b377af566d3b56aa882a964654b5760a` (upstream `f3ce813d`, `b636b6ae`, `b6e8ee12`, `68fb9863`): UTF-8 line decoding and character-counted `read -n`/`-N`. Local changes on top: upstream's block read-ahead and its seek-back are removed, so `read` and `mapfile` read one byte at a time and never seek, as before the port; `read` polls a stream before every byte instead of trusting a `FIONREAD` count; `-n`/`-N` count malformed UTF-8 as the line decodes it, count an incomplete character at EOF, and hold the byte read past the limit with the open file description (brush-core `openfiles::HeldInput`) for the next `read`/`mapfile`. | `crates/brush-builtins-vendored/src/read.rs` | — | — | | A-PI-BUILTINS-UTILS | crate | `pi-builtins/src/utils@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | Bash tool builtins | — | — | — | rejected | | | GNU/BSD behavior diverges; utility builtins pull C-backed onig_sys and pcre2-sys. uucore is already a pre-existing printf dependency and is not a rejection criterion. | — | — | — | | A-PI-VOICE | crate | `crates/pi-voice@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | gjc STT uses `packages/coding-agent/src/stt/recorder.ts` | — | — | — | rejected | | | opus/webrtc and dynamically loaded PulseAudio/ALSA are out of scope; gjc uses sox/ffmpeg/arecord. | — | — | — | @@ -110,8 +110,9 @@ Each ledger row is `retain` or `reconcile`. Guard tests must pass on the pre-mer | D-PI-SHELL-FIXUP | keep-local | `crates/pi-shell/src/fixup.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-shell/src/fixup.rs` | keep-local | applyBashFixups | Bash command execution | Fixup unit tests | `tools:bash` | — | keep-local | | | Preserve local shell fixup behavior. | `crates/pi-shell/src/fixup.rs` | — | — | | D-PI-SHELL-GUARDIAN | keep-local | `crates/pi-shell/src/shell.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | Shell HMAC guardian ownership ledger | keep-local | Guardian process ownership | Bash process lifecycle | Ownership-ledger and guardian tests | `tools:bash` | — | keep-local | | | Preserve guardian ownership authentication and cleanup. | `packages/coding-agent/test/bash-executor.test.ts` | — | — | | D-PI-SHELL-CHILD-SESSION | keep-local | `crates/pi-shell/src/shell.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | Child-session action and output truncation | keep-local | Shell session action | Bash child process and output handling | Rust truth table and truncation tests | `tools:bash` | — | keep-local | | | Preserve detach semantics, output budgets and truncation. | `crates/pi-natives/src/shell.rs`; `packages/coding-agent/test/bash-executor.test.ts` | — | — | -| D-BUILTINS-READ-CANCEL | keep-local | `crates/brush-builtins-vendored/src/read.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | Vendored brush read builtin | keep-local | Shell read builtin | Bash `read` cancellation/timeout | Read builtin unit tests | `tools:bash` | — | keep-local | | | Preserve cancellation, `-t` timeout, Interrupted, and `-t 0` readiness semantics. | `crates/brush-builtins-vendored/src/read.rs` | — | — | +| D-BUILTINS-READ-CANCEL | keep-local | `crates/brush-builtins-vendored/src/read.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | Vendored brush read builtin | keep-local | Shell read builtin | Bash `read` cancellation/timeout | Read builtin unit tests | `tools:bash` | — | keep-local | | | Preserve cancellation, `-t` timeout, Interrupted, and `-t 0` readiness semantics, including input held past a `-n`/`-N` limit. | `crates/brush-builtins-vendored/src/read.rs` | — | — | | D-BRUSH-CORE-WINDOWS | keep-local | `crates/brush-core-vendored/src/sys/windows/commands.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | brush-core-vendored Windows commands | keep-local | CommandWindowControlExt | Windows shell commands | Windows shell tests | `tools:bash` | — | keep-local | | | Preserve local Windows command/window-control patches. | `crates/brush-core-vendored/src/sys/windows/commands.rs`; `packages/natives/test/windows-hidden-shell.windows.test.ts` | — | — | +| D-BRUSH-CORE-HELD-INPUT | keep-local | `crates/brush-core-vendored/src/openfiles.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | brush-core-vendored `OpenFile` held input | keep-local | `openfiles::HeldInput` | Bash `read -n`/`-N`, `mapfile` | line_input and read unit tests; pi-shell `read_count_*` tests | `tools:bash` | — | keep-local | | | Preserve the held-input store that every duplicate of an `OpenFile` shares; `read` and `mapfile` never seek. | `crates/brush-builtins-vendored/src/line_input.rs`; `crates/pi-shell/src/shell.rs` | — | — | | D-PI-AST-ISO | keep-local | `crates/pi-ast/Cargo.toml`, `crates/pi-iso/Cargo.toml@a85bd5228d9f0f619deade1db78fa49420a721e1` | pi-ast features and pi-iso local features | keep-local | `nativeBuildInfo().languageSet`; iso* | AST and platform consumers | LanguageSet test; pi-iso Rust tests | `tools:ast_grep`, `rss` | S1-S5,S7 | keep-local | | | Retain full-langs + perl and local windows-sys Wdk features. | `crates/pi-natives/src/ast.rs`; `crates/pi-iso/Cargo.toml` | — | — | | D-TASK-CATCH-UNWIND | keep-local | `crates/pi-natives/src/task.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-natives/src/task.rs` | keep-local | Abort task APIs | Async NAPI operations | Task panic-to-rejection test | `natives-grep` | — | keep-local | | | Preserve catch_unwind to napi::Error contract. | `crates/pi-natives/src/task.rs` | — | — | | D-NATIVES-PACKAGING | keep-local | `packages/natives/native/loader-state.js@a85bd5228d9f0f619deade1db78fa49420a721e1` | loader-state.js, build-native.ts, gen-enums.ts, embed-native.ts, platform packages | keep-local | Native loader symbols and platform artifacts | Native package entrypoints | Loader/staging/variant tests | `startup` | — | keep-local | | | Preserve AVX2 variants, embedded addon, sentinels, napi-rs profiles and all five platform packages. | `packages/natives/test/native.test.ts`; `packages/natives/test/variant-override.test.ts` | — | — | diff --git a/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md b/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md index 74b5bd51152..cdd21ef1720 100644 --- a/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md +++ b/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md @@ -2,7 +2,3 @@ - The Bash tool's `read` builtin now decodes UTF-8 input instead of turning each byte into a Latin-1 character, so `while IFS= read -r line` loops pass non-ASCII text through intact, and `read -n`/`-N` count characters rather than bytes (#6543). - `mapfile` into a readonly array now fails before reading, leaving piped input for the next reader as bash does. - -### Performance - -- `read` and `mapfile` read a regular file in blocks and give the unread bytes back to the file offset, and `mapfile` without `-n` or `-C` reads a pipe in blocks, instead of one system call per byte. From 04859cd189314b2d6f3968c9773f045c05958316 Mon Sep 17 00:00:00 2001 From: clawdbot Date: Tue, 13 Oct 2026 01:05:59 +0000 Subject: [PATCH 5/5] fix(shell): preserve Windows ACP input in read Builtin readers consumed raw pipeline bytes without the UTF-8-first/Windows ACP decoding already used for shell output. Share that decoder from brush-core so read and mapfile preserve native command text. Fixes #6543 Tested: cargo test -p pi-shell output_decode::tests; cargo test -p pi-shell read_; cargo test -p pi-shell mapfile_; cargo check -p pi-shell --target x86_64-pc-windows-msvc --tests --- .../brush-builtins-vendored/src/line_input.rs | 8 +- crates/brush-core-vendored/Cargo.toml | 1 + crates/brush-core-vendored/src/lib.rs | 1 + .../brush-core-vendored/src/output_decode.rs | 380 ++++++++++++++++++ crates/pi-shell/src/output_decode.rs | 329 +-------------- crates/pi-shell/src/shell.rs | 24 ++ docs/rust-porting-inventory.md | 4 +- .../6543-bash-read-decodes-utf8.md | 2 +- 8 files changed, 425 insertions(+), 324 deletions(-) create mode 100644 crates/brush-core-vendored/src/output_decode.rs diff --git a/crates/brush-builtins-vendored/src/line_input.rs b/crates/brush-builtins-vendored/src/line_input.rs index 5910e7e5582..37e2cfc3667 100644 --- a/crates/brush-builtins-vendored/src/line_input.rs +++ b/crates/brush-builtins-vendored/src/line_input.rs @@ -3,17 +3,17 @@ use std::io::{self, Read}; -use brush_core::openfiles::OpenFile; +use brush_core::{openfiles::OpenFile, output_decode::decode_owned_bytes}; /// ASCII control character for Ctrl+C (ETX - End of Text). pub(crate) const CTRL_C: u8 = 0x03; /// ASCII control character for Ctrl+D (EOT - End of Transmission). pub(crate) const CTRL_D: u8 = 0x04; -/// The bytes of a read line as text: UTF-8 decoded once per line, invalid -/// sequences replaced rather than each byte read as a Latin-1 character. +/// The bytes of a read line as text: UTF-8 first, with Windows ACP fallback +/// for byte-oriented native commands and replacement for invalid input. pub(crate) fn decode_line(line: Vec) -> String { - String::from_utf8(line).unwrap_or_else(|error| String::from_utf8_lossy(error.as_bytes()).into_owned()) + decode_owned_bytes(line) } /// A shell descriptor read one byte at a time by its caller. diff --git a/crates/brush-core-vendored/Cargo.toml b/crates/brush-core-vendored/Cargo.toml index e6d064bdd1f..f68b86dfec0 100644 --- a/crates/brush-core-vendored/Cargo.toml +++ b/crates/brush-core-vendored/Cargo.toml @@ -193,6 +193,7 @@ version = "2.1.1" version = "0.59" features = [ "Win32_Foundation", + "Win32_Globalization", "Win32_Security", "Win32_Storage_FileSystem", "Win32_System_Console", diff --git a/crates/brush-core-vendored/src/lib.rs b/crates/brush-core-vendored/src/lib.rs index c50a2371d67..3d23de44096 100644 --- a/crates/brush-core-vendored/src/lib.rs +++ b/crates/brush-core-vendored/src/lib.rs @@ -23,6 +23,7 @@ pub mod jobs; mod keywords; pub mod namedoptions; pub mod openfiles; +pub mod output_decode; pub mod options; pub mod pathcache; pub mod pathsearch; diff --git a/crates/brush-core-vendored/src/output_decode.rs b/crates/brush-core-vendored/src/output_decode.rs new file mode 100644 index 00000000000..dd4fb478a03 --- /dev/null +++ b/crates/brush-core-vendored/src/output_decode.rs @@ -0,0 +1,380 @@ +// Vendored from +// can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1:crates/pi-shell/ +// src/output_decode.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 +// Stencil Labs, Inc. Modified for gajae-code: yes, shared by shell output and +// builtin line input. +//! Decode UTF-8 and Windows ACP byte streams into Unicode. +//! +//! Modern tools emit UTF-8 (Python with `PYTHONUTF8`, Node, Rust). Native +//! Windows programs on a CJK system write the ANSI code page to pipes — CP936 +//! (GBK) on Simplified Chinese — and treating those bytes as UTF-8 replaces +//! them with U+FFFD (乱码). +//! +//! Policy: decode as UTF-8 until a complete invalid sequence; on Windows, +//! switch the remainder of that stream to the process ANSI code page +//! (`GetACP`) unless that page is already UTF-8. +//! +//! Valid UTF-8 always wins, including bytes that are also valid ACP (GBK 一 is +//! `D2 BB`, which is U+04BB in UTF-8). Overlap is ambiguous; this prefers the +//! modern-tool default over guessing language. The ACP switch is permanent +//! for the stream: command output is one encoding, not mixed. A single +//! invalid byte therefore ACP-decodes everything after it. + +use std::str; + +const REPLACEMENT: &str = "\u{FFFD}"; +/// UTF-8 console / system code page. Fallback is a no-op: invalid UTF-8 stays +/// replacement characters rather than being re-decoded as itself. +#[cfg(windows)] +const CP_UTF8: u32 = 65001; + +enum Mode { + Utf8, + #[cfg(windows)] + Acp, +} + +/// Incremental decoder for one byte stream. +pub struct OutputDecoder { + pending: Vec, + mode: Mode, + #[cfg(windows)] + fallback_codepage: u32, +} + +impl Default for OutputDecoder { + fn default() -> Self { + Self::new() + } +} + +impl OutputDecoder { + /// Decode with the host ANSI code page as the Windows fallback. + /// Not `const`: `GetACP` is an FFI call. + #[cfg(windows)] + pub fn new() -> Self { + Self::with_fallback_codepage(acp()) + } + + /// Decode as UTF-8 only; no ANSI fallback exists off Windows. + #[cfg(not(windows))] + pub const fn new() -> Self { + Self { pending: Vec::new(), mode: Mode::Utf8 } + } + + /// Decode with an explicit ANSI fallback. Used by tests so GBK fixtures + /// run on any Windows host, not only a Chinese ACP. + #[cfg(windows)] + pub const fn with_fallback_codepage(codepage: u32) -> Self { + Self { + pending: Vec::new(), + mode: Mode::Utf8, + fallback_codepage: codepage, + } + } + + /// Push the next bytes. Returns text that is safe to emit now; + /// incomplete UTF-8 / DBCS sequences stay buffered. + pub fn push(&mut self, bytes: &[u8]) -> String { + if bytes.is_empty() { + return String::new(); + } + self.pending.extend_from_slice(bytes); + self.drain(false) + } + + /// Flush held bytes at EOF. + pub fn finish(&mut self) -> String { + self.drain(true) + } + + fn drain(&mut self, eof: bool) -> String { + match self.mode { + Mode::Utf8 => self.drain_utf8(eof), + #[cfg(windows)] + Mode::Acp => self.drain_acp(eof), + } + } + + fn drain_utf8(&mut self, eof: bool) -> String { + let mut out = String::new(); + loop { + if self.pending.is_empty() { + return out; + } + match str::from_utf8(&self.pending) { + Ok(text) => { + out.push_str(text); + self.pending.clear(); + return out; + }, + Err(err) => { + let valid_up_to = err.valid_up_to(); + if valid_up_to > 0 { + // SAFETY: [..valid_up_to] is valid UTF-8 by valid_up_to(). + out.push_str(unsafe { str::from_utf8_unchecked(&self.pending[..valid_up_to]) }); + self.pending.drain(..valid_up_to); + } + if let Some(invalid_len) = err.error_len() { + #[cfg(windows)] + if self.can_fallback_to_acp() { + // One encoding per command. Do not resume UTF-8 after + // a bad byte: later UTF-8 would be ACP-mojibake, but + // cmd/chcp emit ACP for the whole stream. + self.mode = Mode::Acp; + out.push_str(&self.drain_acp(eof)); + return out; + } + + out.push_str(REPLACEMENT); + self.pending.drain(..invalid_len); + } else { + if eof { + #[cfg(windows)] + if self.can_fallback_to_acp() { + self.mode = Mode::Acp; + out.push_str(&self.drain_acp(true)); + return out; + } + + out.push_str(REPLACEMENT); + self.pending.clear(); + } + return out; + } + }, + } + } + } + + #[cfg(windows)] + const fn can_fallback_to_acp(&self) -> bool { + self.fallback_codepage != 0 && self.fallback_codepage != CP_UTF8 + } + + #[cfg(windows)] + fn drain_acp(&mut self, eof: bool) -> String { + if self.pending.is_empty() { + return String::new(); + } + let len = complete_acp_len(self.fallback_codepage, &self.pending, eof); + if len == 0 { + return String::new(); + } + let decoded = decode_codepage(self.fallback_codepage, &self.pending[..len]) + .unwrap_or_else(|| String::from_utf8_lossy(&self.pending[..len]).into_owned()); + self.pending.drain(..len); + decoded + } +} + +/// Decode a complete buffer with the same UTF-8-then-ACP policy as +/// [`OutputDecoder`]. +pub fn decode_bytes(bytes: &[u8]) -> String { + let mut decoder = OutputDecoder::new(); + let mut out = decoder.push(bytes); + out.push_str(&decoder.finish()); + out +} + +/// Decode an owned complete buffer without copying it into the decoder. +pub fn decode_owned_bytes(bytes: Vec) -> String { + let mut decoder = OutputDecoder::new(); + decoder.pending = bytes; + decoder.drain(true) +} + +#[cfg(windows)] +fn acp() -> u32 { + // SAFETY: GetACP has no preconditions. + unsafe { windows_sys::Win32::Globalization::GetACP() } +} + +#[cfg(windows)] +fn is_dbcs_lead(codepage: u32, byte: u8) -> bool { + // SAFETY: `IsDBCSLeadByteEx` accepts any code-page id; unknown pages return + // false. + unsafe { windows_sys::Win32::Globalization::IsDBCSLeadByteEx(codepage, byte) != 0 } +} + +/// Bytes at the start of `bytes` that form complete ACP characters. +/// +/// A trail byte in GBK/Shift-JIS can itself be a lead-byte value, so holding +/// back whatever happens to be last is wrong — walk from the start of the +/// ACP segment instead. +#[cfg(windows)] +fn complete_acp_len(codepage: u32, bytes: &[u8], eof: bool) -> usize { + let mut index = 0; + while index < bytes.len() { + if is_dbcs_lead(codepage, bytes[index]) { + if index + 1 < bytes.len() { + index += 2; + } else { + return if eof { bytes.len() } else { index }; + } + } else { + index += 1; + } + } + index +} + +#[cfg(windows)] +fn decode_codepage(codepage: u32, bytes: &[u8]) -> Option { + if bytes.is_empty() { + return Some(String::new()); + } + let len = i32::try_from(bytes.len()).ok()?; + // SAFETY: `bytes` is a valid slice of `len` bytes; a null wide-char buffer + // with cchWideChar=0 is the documented size-query form. + let needed = unsafe { + windows_sys::Win32::Globalization::MultiByteToWideChar( + codepage, + 0, + bytes.as_ptr(), + len, + std::ptr::null_mut(), + 0, + ) + }; + if needed <= 0 { + return None; + } + let mut wide = vec![0u16; needed as usize]; + // SAFETY: `wide` has `needed` UTF-16 units, matching the size query. + let written = unsafe { + windows_sys::Win32::Globalization::MultiByteToWideChar( + codepage, + 0, + bytes.as_ptr(), + len, + wide.as_mut_ptr(), + needed, + ) + }; + if written <= 0 { + return None; + } + Some(String::from_utf16_lossy(&wide[..written as usize])) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn utf8_passthrough() { + let mut decoder = OutputDecoder::new(); + assert_eq!(decoder.push("hello 中文\n".as_bytes()), "hello 中文\n"); + assert_eq!(decoder.finish(), ""); + } + + #[test] + fn incomplete_utf8_is_held_then_flushed() { + let mut decoder = OutputDecoder::new(); + // First byte of UTF-8 中 (E4 B8 AD). + assert_eq!(decoder.push(&[0xe4]), ""); + assert_eq!(decoder.push(&[0xb8, 0xad]), "中"); + assert_eq!(decoder.finish(), ""); + } + + #[test] + fn incomplete_utf8_at_eof_becomes_replacement() { + // Pin UTF-8-only decoding: on Windows `new()` falls back to the host ACP, + // where a trailing 0xE4 legitimately decodes (e.g. `ä` under cp1252). + #[cfg(windows)] + let mut decoder = OutputDecoder::with_fallback_codepage(CP_UTF8); + #[cfg(not(windows))] + let mut decoder = OutputDecoder::new(); + assert_eq!(decoder.push(&[0xe4]), ""); + assert_eq!(decoder.finish(), "\u{FFFD}"); + } + + #[cfg(windows)] + #[test] + fn gbk_decodes_to_chinese() { + // GBK for 中文. + let mut decoder = OutputDecoder::with_fallback_codepage(936); + assert_eq!(decoder.push(&[0xd6, 0xd0, 0xce, 0xc4]), "中文"); + assert_eq!(decoder.finish(), ""); + } + + #[cfg(windows)] + #[test] + fn host_acp_decodes_gbk_when_system_is_936() { + if acp() != 936 { + return; + } + assert_eq!(decode_bytes(&[0xd6, 0xd0, 0xce, 0xc4]), "中文"); + } + + #[cfg(windows)] + #[test] + fn gbk_character_split_across_reads() { + let mut decoder = OutputDecoder::with_fallback_codepage(936); + assert_eq!(decoder.push(&[0xd6]), ""); + assert_eq!(decoder.push(&[0xd0, 0xce, 0xc4, b'\n']), "中文\n"); + assert_eq!(decoder.finish(), ""); + } + + #[cfg(windows)] + #[test] + fn gbk_holds_real_lead_not_trail_that_looks_like_lead() { + let mut decoder = OutputDecoder::with_fallback_codepage(936); + // 文 is CE C4; C4 is also a GBK lead value, so a last-byte heuristic + // would hold it back and mangle 文. + assert_eq!(decoder.push(&[0xce, 0xc4]), "文"); + assert_eq!(decoder.finish(), ""); + } + + #[cfg(windows)] + #[test] + fn ascii_prefix_then_gbk() { + let mut decoder = OutputDecoder::with_fallback_codepage(936); + assert_eq!(decoder.push(b"cmd"), "cmd"); + assert_eq!(decoder.push(&[0xd6, 0xd0, 0xce, 0xc4]), "中文"); + assert_eq!(decoder.finish(), ""); + } + + #[cfg(windows)] + #[test] + fn utf8_preferred_over_gbk_fallback() { + let mut decoder = OutputDecoder::with_fallback_codepage(936); + assert_eq!(decoder.push("中文".as_bytes()), "中文"); + assert_eq!(decoder.finish(), ""); + } + + #[cfg(windows)] + #[test] + fn incomplete_utf8_at_eof_falls_back_to_acp() { + // CP1252 for é is also an incomplete three-byte UTF-8 prefix. EOF proves + // this stream is not valid UTF-8, so it must still trigger fallback. + let mut decoder = OutputDecoder::with_fallback_codepage(1252); + assert_eq!(decoder.push(&[0xe9]), ""); + assert_eq!(decoder.finish(), "é"); + } + + #[cfg(windows)] + #[test] + fn cp1252_external_line_decodes_with_its_newline() { + let mut decoder = OutputDecoder::with_fallback_codepage(1252); + assert_eq!(decoder.push(&[0xe9, b'\n']), "é\n"); + assert_eq!(decoder.finish(), ""); + } + + #[cfg(windows)] + #[test] + fn utf8_system_codepage_keeps_replacement() { + let mut decoder = OutputDecoder::with_fallback_codepage(65001); + assert_eq!(decoder.push(&[0xff]), "\u{FFFD}"); + assert_eq!(decoder.finish(), ""); + } + + #[cfg(not(windows))] + #[test] + fn invalid_utf8_becomes_replacement() { + let mut decoder = OutputDecoder::new(); + assert_eq!(decoder.push(&[0xff]), "\u{FFFD}"); + assert_eq!(decoder.finish(), ""); + } +} diff --git a/crates/pi-shell/src/output_decode.rs b/crates/pi-shell/src/output_decode.rs index 3e7c06e615f..01bbd8ed52e 100644 --- a/crates/pi-shell/src/output_decode.rs +++ b/crates/pi-shell/src/output_decode.rs @@ -1,255 +1,4 @@ -// Vendored from -// can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1:crates/pi-shell/ -// src/output_decode.rs — MIT (c) 2025 Mario Zechner, 2025-2026 Can Bölük, 2026 -// Stencil Labs, Inc. Modified for gajae-code: yes, integrated with shell output -// budgets and buffered minimizer captures. -//! Decode child-process stdout/stderr into Unicode. -//! -//! Modern tools emit UTF-8 (Python with `PYTHONUTF8`, Node, Rust). Native -//! Windows programs on a CJK system write the ANSI code page to pipes — CP936 -//! (GBK) on Simplified Chinese — and treating those bytes as UTF-8 replaces -//! them with U+FFFD (乱码). -//! -//! Policy: decode as UTF-8 until a complete invalid sequence; on Windows, -//! switch the remainder of that stream to the process ANSI code page -//! (`GetACP`) unless that page is already UTF-8. -//! -//! Valid UTF-8 always wins, including bytes that are also valid ACP (GBK 一 is -//! `D2 BB`, which is U+04BB in UTF-8). Overlap is ambiguous; this prefers the -//! modern-tool default over guessing language. The ACP switch is permanent -//! for the stream: command output is one encoding, not mixed. A single -//! invalid byte therefore ACP-decodes everything after it. - -use std::str; - -const REPLACEMENT: &str = "\u{FFFD}"; -/// UTF-8 console / system code page. Fallback is a no-op: invalid UTF-8 stays -/// replacement characters rather than being re-decoded as itself. -#[cfg(windows)] -const CP_UTF8: u32 = 65001; - -enum Mode { - Utf8, - #[cfg(windows)] - Acp, -} - -/// Incremental decoder for one stdout/stderr stream. -pub struct OutputDecoder { - pending: Vec, - mode: Mode, - #[cfg(windows)] - fallback_codepage: u32, -} - -impl Default for OutputDecoder { - fn default() -> Self { - Self::new() - } -} - -impl OutputDecoder { - /// Decode with the host ANSI code page as the Windows fallback. - /// Not `const`: `GetACP` is an FFI call. - #[cfg(windows)] - pub fn new() -> Self { - Self::with_fallback_codepage(acp()) - } - - /// Decode as UTF-8 only; no ANSI fallback exists off Windows. - #[cfg(not(windows))] - pub const fn new() -> Self { - Self { pending: Vec::new(), mode: Mode::Utf8 } - } - - /// Decode with an explicit ANSI fallback. Used by tests so GBK fixtures - /// run on any Windows host, not only a Chinese ACP. - #[cfg(windows)] - pub const fn with_fallback_codepage(codepage: u32) -> Self { - Self { - pending: Vec::new(), - mode: Mode::Utf8, - fallback_codepage: codepage, - } - } - - /// Push the next pipe read. Returns text that is safe to emit now; - /// incomplete UTF-8 / DBCS sequences stay buffered. - pub fn push(&mut self, bytes: &[u8]) -> String { - if bytes.is_empty() { - return String::new(); - } - self.pending.extend_from_slice(bytes); - self.drain(false) - } - - /// Flush held bytes at EOF. - pub fn finish(&mut self) -> String { - self.drain(true) - } - - fn drain(&mut self, eof: bool) -> String { - match self.mode { - Mode::Utf8 => self.drain_utf8(eof), - #[cfg(windows)] - Mode::Acp => self.drain_acp(eof), - } - } - - fn drain_utf8(&mut self, eof: bool) -> String { - let mut out = String::new(); - loop { - if self.pending.is_empty() { - return out; - } - match str::from_utf8(&self.pending) { - Ok(text) => { - out.push_str(text); - self.pending.clear(); - return out; - }, - Err(err) => { - let valid_up_to = err.valid_up_to(); - if valid_up_to > 0 { - // SAFETY: [..valid_up_to] is valid UTF-8 by valid_up_to(). - out.push_str(unsafe { str::from_utf8_unchecked(&self.pending[..valid_up_to]) }); - self.pending.drain(..valid_up_to); - } - if let Some(invalid_len) = err.error_len() { - #[cfg(windows)] - if self.can_fallback_to_acp() { - // One encoding per command. Do not resume UTF-8 after - // a bad byte: later UTF-8 would be ACP-mojibake, but - // cmd/chcp emit ACP for the whole stream. - self.mode = Mode::Acp; - out.push_str(&self.drain_acp(eof)); - return out; - } - - out.push_str(REPLACEMENT); - self.pending.drain(..invalid_len); - } else { - if eof { - #[cfg(windows)] - if self.can_fallback_to_acp() { - self.mode = Mode::Acp; - out.push_str(&self.drain_acp(true)); - return out; - } - - out.push_str(REPLACEMENT); - self.pending.clear(); - } - return out; - } - }, - } - } - } - - #[cfg(windows)] - const fn can_fallback_to_acp(&self) -> bool { - self.fallback_codepage != 0 && self.fallback_codepage != CP_UTF8 - } - - #[cfg(windows)] - fn drain_acp(&mut self, eof: bool) -> String { - if self.pending.is_empty() { - return String::new(); - } - let len = complete_acp_len(self.fallback_codepage, &self.pending, eof); - if len == 0 { - return String::new(); - } - let decoded = decode_codepage(self.fallback_codepage, &self.pending[..len]) - .unwrap_or_else(|| String::from_utf8_lossy(&self.pending[..len]).into_owned()); - self.pending.drain(..len); - decoded - } -} - -/// Decode a complete buffer with the same UTF-8-then-ACP policy as -/// [`OutputDecoder`]. -pub fn decode_bytes(bytes: &[u8]) -> String { - let mut decoder = OutputDecoder::new(); - let mut out = decoder.push(bytes); - out.push_str(&decoder.finish()); - out -} - -#[cfg(windows)] -fn acp() -> u32 { - // SAFETY: GetACP has no preconditions. - unsafe { windows_sys::Win32::Globalization::GetACP() } -} - -#[cfg(windows)] -fn is_dbcs_lead(codepage: u32, byte: u8) -> bool { - // SAFETY: `IsDBCSLeadByteEx` accepts any code-page id; unknown pages return - // false. - unsafe { windows_sys::Win32::Globalization::IsDBCSLeadByteEx(codepage, byte) != 0 } -} - -/// Bytes at the start of `bytes` that form complete ACP characters. -/// -/// A trail byte in GBK/Shift-JIS can itself be a lead-byte value, so holding -/// back whatever happens to be last is wrong — walk from the start of the -/// ACP segment instead. -#[cfg(windows)] -fn complete_acp_len(codepage: u32, bytes: &[u8], eof: bool) -> usize { - let mut index = 0; - while index < bytes.len() { - if is_dbcs_lead(codepage, bytes[index]) { - if index + 1 < bytes.len() { - index += 2; - } else { - return if eof { bytes.len() } else { index }; - } - } else { - index += 1; - } - } - index -} - -#[cfg(windows)] -fn decode_codepage(codepage: u32, bytes: &[u8]) -> Option { - if bytes.is_empty() { - return Some(String::new()); - } - let len = i32::try_from(bytes.len()).ok()?; - // SAFETY: `bytes` is a valid slice of `len` bytes; a null wide-char buffer - // with cchWideChar=0 is the documented size-query form. - let needed = unsafe { - windows_sys::Win32::Globalization::MultiByteToWideChar( - codepage, - 0, - bytes.as_ptr(), - len, - std::ptr::null_mut(), - 0, - ) - }; - if needed <= 0 { - return None; - } - let mut wide = vec![0u16; needed as usize]; - // SAFETY: `wide` has `needed` UTF-16 units, matching the size query. - let written = unsafe { - windows_sys::Win32::Globalization::MultiByteToWideChar( - codepage, - 0, - bytes.as_ptr(), - len, - wide.as_mut_ptr(), - needed, - ) - }; - if written <= 0 { - return None; - } - Some(String::from_utf16_lossy(&wide[..written as usize])) -} +pub(crate) use brush_core::output_decode::{OutputDecoder, decode_bytes}; #[cfg(test)] mod tests { @@ -265,101 +14,47 @@ mod tests { #[test] fn incomplete_utf8_is_held_then_flushed() { let mut decoder = OutputDecoder::new(); - // First byte of UTF-8 中 (E4 B8 AD). assert_eq!(decoder.push(&[0xe4]), ""); assert_eq!(decoder.push(&[0xb8, 0xad]), "中"); assert_eq!(decoder.finish(), ""); } #[test] - fn incomplete_utf8_at_eof_becomes_replacement() { - // Pin UTF-8-only decoding: on Windows `new()` falls back to the host ACP, - // where a trailing 0xE4 legitimately decodes (e.g. `ä` under cp1252). + fn invalid_utf8_becomes_replacement() { #[cfg(windows)] - let mut decoder = OutputDecoder::with_fallback_codepage(CP_UTF8); + let mut decoder = OutputDecoder::with_fallback_codepage(65001); #[cfg(not(windows))] let mut decoder = OutputDecoder::new(); - assert_eq!(decoder.push(&[0xe4]), ""); - assert_eq!(decoder.finish(), "\u{FFFD}"); - } - - #[cfg(windows)] - #[test] - fn gbk_decodes_to_chinese() { - // GBK for 中文. - let mut decoder = OutputDecoder::with_fallback_codepage(936); - assert_eq!(decoder.push(&[0xd6, 0xd0, 0xce, 0xc4]), "中文"); + assert_eq!(decoder.push(&[0xff]), "\u{FFFD}"); assert_eq!(decoder.finish(), ""); } - #[cfg(windows)] #[test] - fn host_acp_decodes_gbk_when_system_is_936() { - if acp() != 936 { - return; - } - assert_eq!(decode_bytes(&[0xd6, 0xd0, 0xce, 0xc4]), "中文"); + fn complete_utf8_buffer_decodes() { + assert_eq!(decode_bytes("hello 中文".as_bytes()), "hello 中文"); } #[cfg(windows)] #[test] - fn gbk_character_split_across_reads() { - let mut decoder = OutputDecoder::with_fallback_codepage(936); - assert_eq!(decoder.push(&[0xd6]), ""); - assert_eq!(decoder.push(&[0xd0, 0xce, 0xc4, b'\n']), "中文\n"); - assert_eq!(decoder.finish(), ""); - } - - #[cfg(windows)] - #[test] - fn gbk_holds_real_lead_not_trail_that_looks_like_lead() { + fn gbk_decodes_to_chinese() { let mut decoder = OutputDecoder::with_fallback_codepage(936); - // 文 is CE C4; C4 is also a GBK lead value, so a last-byte heuristic - // would hold it back and mangle 文. - assert_eq!(decoder.push(&[0xce, 0xc4]), "文"); + assert_eq!(decoder.push(&[0xd6, 0xd0, 0xce, 0xc4]), "中文"); assert_eq!(decoder.finish(), ""); } #[cfg(windows)] #[test] - fn ascii_prefix_then_gbk() { - let mut decoder = OutputDecoder::with_fallback_codepage(936); - assert_eq!(decoder.push(b"cmd"), "cmd"); - assert_eq!(decoder.push(&[0xd6, 0xd0, 0xce, 0xc4]), "中文"); + fn cp1252_native_line_decodes_with_newline() { + let mut decoder = OutputDecoder::with_fallback_codepage(1252); + assert_eq!(decoder.push(&[0xe9, b'\n']), "é\n"); assert_eq!(decoder.finish(), ""); } #[cfg(windows)] #[test] - fn utf8_preferred_over_gbk_fallback() { + fn utf8_is_preferred_over_gbk_fallback() { let mut decoder = OutputDecoder::with_fallback_codepage(936); assert_eq!(decoder.push("中文".as_bytes()), "中文"); assert_eq!(decoder.finish(), ""); } - - #[cfg(windows)] - #[test] - fn incomplete_utf8_at_eof_falls_back_to_acp() { - // CP1252 for é is also an incomplete three-byte UTF-8 prefix. EOF proves - // this stream is not valid UTF-8, so it must still trigger fallback. - let mut decoder = OutputDecoder::with_fallback_codepage(1252); - assert_eq!(decoder.push(&[0xe9]), ""); - assert_eq!(decoder.finish(), "é"); - } - - #[cfg(windows)] - #[test] - fn utf8_system_codepage_keeps_replacement() { - let mut decoder = OutputDecoder::with_fallback_codepage(65001); - assert_eq!(decoder.push(&[0xff]), "\u{FFFD}"); - assert_eq!(decoder.finish(), ""); - } - - #[cfg(not(windows))] - #[test] - fn invalid_utf8_becomes_replacement() { - let mut decoder = OutputDecoder::new(); - assert_eq!(decoder.push(&[0xff]), "\u{FFFD}"); - assert_eq!(decoder.finish(), ""); - } } diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index da63a2fbdbb..50f2d8ffee5 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -3680,6 +3680,30 @@ mod tests { assert_eq!(output, "é|ü\n"); } + /// Contract: `read` preserves Windows ACP bytes from an external producer. + #[cfg(windows)] + #[tokio::test(flavor = "multi_thread")] + async fn read_decodes_windows_acp_bytes_from_native_pipeline() { + // This fixture is CP1252-specific; other Windows ACPs are covered by the + // shared decoder's explicit code-page tests. + // SAFETY: GetACP has no preconditions. + if unsafe { windows_sys::Win32::Globalization::GetACP() } != 1252 { + return; + } + + let script = "$stream = [Console]::OpenStandardOutput(); $stream.WriteByte(0xE9); \ + $stream.WriteByte(0x0A)"; + let utf16: Vec = script.encode_utf16().flat_map(u16::to_le_bytes).collect(); + let encoded = test_base64(&utf16); + let command = format!( + "powershell -NoProfile -NonInteractive -EncodedCommand {encoded} | {{ read -r x; printf \ + '[%s]\\n' \"$x\"; }}" + ); + let (result, output) = execute_captured(command).await; + assert_eq!(result.exit_code, Some(0), "{output:?}"); + assert_eq!(output, "[é]\n"); + } + /// Contract: `read -n` counts characters, not bytes, as bash does in a /// UTF-8 locale; a multibyte character is never split. #[tokio::test(flavor = "multi_thread")] diff --git a/docs/rust-porting-inventory.md b/docs/rust-porting-inventory.md index 474441a9b4d..39d98ed1c0e 100644 --- a/docs/rust-porting-inventory.md +++ b/docs/rust-porting-inventory.md @@ -42,7 +42,7 @@ Upstream pin: `can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1` | A-PI-WALKER | crate | `crates/pi-walker@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-walker`; `crates/pi-natives/src/iofs.rs` | infra/overlap | glob, invalidateFsScanCache, walkerPoolStatus | Filesystem traversal substrate; `packages/coding-agent/src/tools/find.ts` | Recorded glob/fd/workspace outputs on fixture trees | `tools:glob`, `startup`, `tools` | S1-S5,S7 | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Integrated traversal substrate and pi-edit dependency; absorbs fs_cache policy and keeps the serial-on-pool-failure branch. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6027, https://github.com/Yeachan-Heo/gajae-code/pull/6039. | — | — | — | | A-PI-EDIT | crate | `crates/pi-edit@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-edit`; `crates/pi-natives/src/edit.rs` | (a) | `editFindMatch`, `editSeekSequence`, hashline, patch, EditSession, EditStore APIs | `packages/coding-agent/src/edit/{modes/replace,patch,apply-patch}.ts`; `edit/apply-patch/*`; `edit/{diff,normalize,streaming,notebook}.ts`; `hashline/*.ts` | Edit-benchmark TS matcher goldens including errors; per-mode goldens through 2.10 | `edit-hotspots` H01,H02,H06; `tools:edit`; `bench:edit`; `replay` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/6016 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Matcher merged via #5980 and fuzzy scoring fix #6016; edit-hotspots H01/H02 covered by the pi-edit fuzzy A/B in #6016. | — | — | `packages/coding-agent/src/edit/modes/replace.ts`; `packages/coding-agent/src/edit/streaming.ts` | A-PI-VCS | crate | `crates/pi-vcs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-vcs`; `crates/pi-natives/src/vcs.rs` | (a), size-gated (D5) | VCS read APIs | `packages/coding-agent/src/utils/git.ts`; `autoresearch/git.ts`; `status-line/git-utils.ts`; `commit/agentic/tools/git-*.ts` | Git CLI oracle over fixture repositories | `status-line`, `tools:git` | — | rejected | | | D5 perf gate: large-worktree read suite upper-CI p50 ratio was 0.826 (needs <0.80), and status was 1.20x slower. | — | — | — | -| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. Per-row bump for #6543: `read.rs`, `mapfile.rs` and the new `line_input.rs` are vendored from `can1357/oh-my-pi` commit `68fb9863b377af566d3b56aa882a964654b5760a` (upstream `f3ce813d`, `b636b6ae`, `b6e8ee12`, `68fb9863`): UTF-8 line decoding and character-counted `read -n`/`-N`. Local changes on top: upstream's block read-ahead and its seek-back are removed, so `read` and `mapfile` read one byte at a time and never seek, as before the port; `read` polls a stream before every byte instead of trusting a `FIONREAD` count; `-n`/`-N` count malformed UTF-8 as the line decodes it, count an incomplete character at EOF, and hold the byte read past the limit with the open file description (brush-core `openfiles::HeldInput`) for the next `read`/`mapfile`. | `crates/brush-builtins-vendored/src/read.rs` | — | — | +| A-PI-BUILTINS-BASE | crate | `pi-builtins/src/.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/brush-builtins-vendored` | overlap | — | Shell builtins; port changes one builtin at a time. | Existing fork tests plus per-builtin guard tests. | `builtins`, `shell` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | Integrated portable base-builtin changes; per-builtin parity: eight builtins (echo, printf, test/[, read, cd/pwd, export, type/command -v, for/arithmetic) produce byte-identical stdout on 01511ffd5f and dev in the `builtins` suite. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6117. Per-row bump for #6543: `read.rs`, `mapfile.rs` and the new `line_input.rs` are vendored from `can1357/oh-my-pi` commit `68fb9863b377af566d3b56aa882a964654b5760a` (upstream `f3ce813d`, `b636b6ae`, `b6e8ee12`, `68fb9863`): UTF-8 line decoding with Windows ACP fallback and character-counted `read -n`/`-N`. Local changes on top: upstream's block read-ahead and its seek-back are removed, so `read` and `mapfile` read one byte at a time and never seek, as before the port; `read` polls a stream before every byte instead of trusting a `FIONREAD` count; `-n`/`-N` count malformed UTF-8 as the line decodes it, count an incomplete character at EOF, and hold the byte read past the limit with the open file description (brush-core `openfiles::HeldInput`) for the next `read`/`mapfile`; line decoding reuses the shared brush-core UTF-8-first/Windows ACP decoder so native pipeline text is preserved. | `crates/brush-builtins-vendored/src/read.rs` | — | — | | A-PI-BUILTINS-UTILS | crate | `pi-builtins/src/utils@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | Bash tool builtins | — | — | — | rejected | | | GNU/BSD behavior diverges; utility builtins pull C-backed onig_sys and pcre2-sys. uucore is already a pre-existing printf dependency and is not a rejection criterion. | — | — | — | | A-PI-VOICE | crate | `crates/pi-voice@a85bd5228d9f0f619deade1db78fa49420a721e1` | — | (c) | — | gjc STT uses `packages/coding-agent/src/stt/recorder.ts` | — | — | — | rejected | | | opus/webrtc and dynamically loaded PulseAudio/ALSA are out of scope; gjc uses sox/ffmpeg/arecord. | — | — | — | @@ -81,7 +81,7 @@ Upstream pin: `can1357/oh-my-pi@a85bd5228d9f0f619deade1db78fa49420a721e1` | C-SIXEL | overlap | `crates/pi-natives/src/sixel.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-natives/src/sixel.rs` | overlap | Sixel encoding | Image display | License-clean Sixel fixtures | `read(image)` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | ≈0.20k/0.06k; icy_sixel 0.7. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. | — | — | — | | C-CLIPBOARD | overlap | `crates/pi-natives/src/clipboard.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-natives/src/clipboard.rs` | overlap | Clipboard APIs | Clipboard/image paths | Image payload fixtures | `clipboard` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | ≈0.37k/0.08k; clipboard-win and image paths. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6129. | — | — | — | | C-PTY | overlap | `crates/pi-natives/src/pty.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-natives/src/pty.rs` | overlap | PTY APIs | Shell/TUI terminal paths | PTY byte capture and teardown | `pty`, `tui-input-write` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | ≈1.13k/1.25k; retain local hardening. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062. | `packages/natives/test/native.test.ts` | — | — | -| C-SHELL | overlap | `crates/pi-natives/src/shell.rs`, `pi-shell@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-natives/src/shell.rs`; `crates/pi-shell` | overlap | executeShell | `packages/coding-agent/src/tools/bash.ts` | Shell behavior corpus and cancellation cases | `tools:bash`, `startup` | S1,S3,S7 | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Re-synced the shell stream decoder and portable base-builtin improvements; retained local process, cancellation, Windows PATH, minimizer, fixup, guardian, output budget, read cancellation, and hidden-console guards. Brush-core Fs/async-dependent changes, process/job API expansions, and upstream utility builtins are intentionally not adopted. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. | `crates/pi-natives/src/shell.rs`; `packages/coding-agent/test/bash-executor.test.ts`; `packages/coding-agent/test/tools/bash-command-fixup.test.ts`; `packages/coding-agent/test/tools/bash-master-owner-session-id.test.ts`; `packages/natives/test/shell-read-builtin-abort.test.ts` | — | — | +| C-SHELL | overlap | `crates/pi-natives/src/shell.rs`, `pi-shell@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-natives/src/shell.rs`; `crates/pi-shell`; `crates/brush-core-vendored/src/output_decode.rs` | overlap | executeShell | `packages/coding-agent/src/tools/bash.ts` | Shell behavior corpus and cancellation cases | `tools:bash`, `startup` | S1,S3,S7 | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Re-synced the shell stream decoder and portable base-builtin improvements; the UTF-8-first/Windows ACP decoder is shared from brush-core with builtin line input. Retained local process, cancellation, Windows PATH, minimizer, fixup, guardian, output budget, read cancellation, and hidden-console guards. Brush-core Fs/async-dependent changes, process/job API expansions, and upstream utility builtins are intentionally not adopted. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. | `crates/pi-natives/src/shell.rs`; `packages/coding-agent/test/bash-executor.test.ts`; `packages/coding-agent/test/tools/bash-command-fixup.test.ts`; `packages/coding-agent/test/tools/bash-master-owner-session-id.test.ts`; `packages/natives/test/shell-read-builtin-abort.test.ts` | — | — | | C-PS | overlap | `crates/pi-natives/src/ps.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-natives/src/ps.rs`; `crates/pi-shell/src/process.rs` | overlap | Process observation/control | Shell process observation | Child process lifecycle tests | `tools:bash` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | ≈0.24k/0.28k; local pi_shell::process remains. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. | `packages/coding-agent/test/bash-executor.test.ts` | — | — | | C-ISO | overlap | `crates/pi-natives/src/iso.rs`, `crates/pi-iso@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-natives/src/iso.rs`; `crates/pi-iso` | overlap | iso* | Platform-specific path/process helpers | ISO diff goldens on fixture trees | `iso`, `rss` | S1-S5,S7 | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36538316951 | ≈0.25k/0.24k; preserve local windows-sys Wdk features. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. Follow-up fixes: https://github.com/Yeachan-Heo/gajae-code/pull/6062, https://github.com/Yeachan-Heo/gajae-code/pull/6089. | `crates/pi-natives/src/iso.rs` | — | — | | C-WORKSPACE | overlap | `crates/pi-natives/src/workspace.rs@a85bd5228d9f0f619deade1db78fa49420a721e1` | `crates/pi-natives/src/workspace.rs`; walker | overlap | listWorkspace | Workspace discovery | Recorded fixture trees | `startup`, `tools` | — | adopted | https://github.com/Yeachan-Heo/gajae-code/pull/5980 | https://github.com/Yeachan-Heo/gajae-code/actions/runs/36352209021 | Workspace discovery uses the integrated pi-walker traversal substrate. A/B vs pre-port dev `01511ffd5f`: `docs/rust-porting/evidence/native-ab-vs-01511ffd5f.json`. | — | — | — | diff --git a/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md b/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md index cdd21ef1720..2cb91fa8456 100644 --- a/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md +++ b/packages/coding-agent/changelog.d/6543-bash-read-decodes-utf8.md @@ -1,4 +1,4 @@ ### Fixed -- The Bash tool's `read` builtin now decodes UTF-8 input instead of turning each byte into a Latin-1 character, so `while IFS= read -r line` loops pass non-ASCII text through intact, and `read -n`/`-N` count characters rather than bytes (#6543). +- The Bash tool's `read` builtin now decodes valid UTF-8 as text and preserves Windows ANSI-code-page text from native pipelines instead of replacing it, so `while IFS= read -r line` loops pass non-ASCII text through intact, and `read -n`/`-N` count UTF-8 characters rather than bytes (#6543). - `mapfile` into a readonly array now fails before reading, leaving piped input for the next reader as bash does.