From a569385b8ee129f0375aecb1c8705af2464093b6 Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 24 Jul 2026 20:06:29 +0200 Subject: [PATCH] fix(shell): captured find -exec child stdio into scope streams - find -exec/-execdir children inherited the omp process's real stdout/stderr, spamming output into the TUI terminal and bypassing shell redirects; they also inherited the host env instead of the shell's exported environment. - Added pi_uutils_ctx::run_captured (moved from uu-xargs' private helper): stdin null, stdout streamed into scope stdout, stderr drained on a helper thread and forwarded after exit. - uu-find exec matchers now use env_clear + env_snapshot and run_captured; MultiExecMatcher rebuilds a std Command from the argmax command's accumulated state (argmax only Derefs immutably). - uu-xargs reuses the shared helper; added pi-shell regression test asserting -exec child stdout flows through the shell redirect with the exported env. --- crates/pi-shell/src/shell.rs | 18 +++++++++ crates/pi-uutils-ctx/src/lib.rs | 39 ++++++++++++++++++ .../vendor/uu-find/src/find/matchers/exec.rs | 17 +++++++- crates/vendor/uu-xargs/src/xargs/mod.rs | 40 +------------------ packages/coding-agent/CHANGELOG.md | 4 ++ 5 files changed, 78 insertions(+), 40 deletions(-) diff --git a/crates/pi-shell/src/shell.rs b/crates/pi-shell/src/shell.rs index 85e41fd12..cbc24316f 100644 --- a/crates/pi-shell/src/shell.rs +++ b/crates/pi-shell/src/shell.rs @@ -3023,6 +3023,24 @@ mod tests { "-exec {{}} should be operand-relative and run in the shell cwd" ); + // -exec children must write through the scope streams — an inherited + // process stdout would bypass the shell redirect and spam the host + // TUI's terminal — and must see the shell's exported environment. + session + .shell + .run_string( + "export PI_EXEC_ENV=zed; find . -name keep.log -exec sh -c 'echo \"$PI_EXEC_ENV $1\"' sh {} ';' > cap.txt", + &si, + ¶ms, + ) + .await + .expect("exec capture"); + assert_eq!( + read("cap.txt"), + "zed ./keep.log\n", + "-exec child stdout must flow through the shell redirect with the shell's exported env" + ); + let _ = std::fs::remove_dir_all(&tmp); } diff --git a/crates/pi-uutils-ctx/src/lib.rs b/crates/pi-uutils-ctx/src/lib.rs index 09d258e64..31c18e912 100644 --- a/crates/pi-uutils-ctx/src/lib.rs +++ b/crates/pi-uutils-ctx/src/lib.rs @@ -342,6 +342,45 @@ pub fn stdin() -> CtxStdin { CtxStdin } +/// Runs `command` with stdin from the null device and stdout/stderr piped +/// back into the context streams, returning the child's exit status. +/// +/// The context streams are in-process `Write` handles (pipes or in-memory +/// buffers), not inheritable file descriptors, and the host process's own +/// fd 0/1/2 belong to the TUI — a child must never inherit stdio. Child +/// stdout streams into the context stdout on the calling thread while a +/// helper thread drains stderr into a buffer (the context streams are +/// thread-local to the scope thread, so the helper cannot write directly); +/// the buffered stderr is forwarded once the child exits. +/// +/// Callers remain responsible for `current_dir` and the scope environment +/// (`env_clear().envs(env_snapshot())`). +pub fn run_captured(command: &mut std::process::Command) -> io::Result { + command + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()); + let mut child = command.spawn()?; + + let mut child_err = child.stderr.take(); + let stderr_thread = std::thread::spawn(move || { + let mut buf = Vec::new(); + if let Some(err) = child_err.as_mut() { + let _ = err.read_to_end(&mut buf); + } + buf + }); + + if let Some(mut out) = child.stdout.take() { + let _ = io::copy(&mut out, &mut stdout()); + } + let status = child.wait(); + if let Ok(buf) = stderr_thread.join() { + let _ = stderr().write_all(&buf); + } + status +} + /// Generate the usage string for clap without evaluating argv-dependent /// statics. /// diff --git a/crates/vendor/uu-find/src/find/matchers/exec.rs b/crates/vendor/uu-find/src/find/matchers/exec.rs index 13c6a0f3b..a207412b5 100644 --- a/crates/vendor/uu-find/src/find/matchers/exec.rs +++ b/crates/vendor/uu-find/src/find/matchers/exec.rs @@ -82,7 +82,10 @@ impl Matcher for SingleExecMatcher { // operand-relative `{}` against the shell cwd, not the host cwd. command.current_dir(pi_uutils_ctx::cwd()); } - match command.status() { + command.env_clear().envs(pi_uutils_ctx::env_snapshot()); + // The host process's stdio belongs to the embedding TUI; route the + // child's output through the scope streams instead of inheriting. + match pi_uutils_ctx::run_captured(&mut command) { Ok(status) => status.success(), Err(e) => { writeln!(&mut stderr(), "Failed to run {}: {}", self.executable, e).unwrap(); @@ -132,7 +135,17 @@ impl MultiExecMatcher { } fn run_command(&self, command: &mut argmax::Command, matcher_io: &mut MatcherIO) { - match command.status() { + // `argmax::Command` only Derefs immutably into `std::process::Command`, + // so rebuild a std command from its accumulated state to attach the + // scope environment and context-captured stdio — the host process's + // stdio belongs to the embedding TUI and must never be inherited. + let mut std_command = Command::new(command.get_program()); + std_command.args(command.get_args()); + if let Some(dir) = command.get_current_dir() { + std_command.current_dir(dir); + } + std_command.env_clear().envs(pi_uutils_ctx::env_snapshot()); + match pi_uutils_ctx::run_captured(&mut std_command) { Ok(status) => { if !status.success() { matcher_io.set_exit_code(1); diff --git a/crates/vendor/uu-xargs/src/xargs/mod.rs b/crates/vendor/uu-xargs/src/xargs/mod.rs index 93666969d..bd83cd825 100644 --- a/crates/vendor/uu-xargs/src/xargs/mod.rs +++ b/crates/vendor/uu-xargs/src/xargs/mod.rs @@ -11,7 +11,7 @@ use std::{ fmt::Display, fs, io::{self, BufRead, BufReader, Read, Write}, - process::{Command, ExitStatus, Stdio}, + process::Command, }; use clap::{Arg, ArgAction, crate_version, error::ErrorKind}; @@ -410,7 +410,7 @@ impl CommandBuilder<'_> { .current_dir(pi_uutils_ctx::cwd()) .env_clear() .envs(&self.options.env); - match run_command_captured(&mut command) { + match pi_uutils_ctx::run_captured(&mut command) { Ok(status) => { if status.success() { Ok(CommandResult::Success) @@ -458,42 +458,6 @@ impl CommandBuilder<'_> { } } -/// Runs `command` with stdin from the null device and stdout/stderr piped -/// back into the context streams, returning the child's exit status. -/// -/// The context streams are in-process `Write` handles (pipes or in-memory -/// buffers), not inheritable file descriptors, and the host process's own -/// fd 0/1/2 belong to the TUI — a child must never inherit stdio. Child -/// stdout streams into the context stdout on the calling thread while a -/// helper thread drains stderr into a buffer (the context streams are -/// thread-local to the scope thread, so the helper cannot write directly); -/// the buffered stderr is forwarded once the child exits. -fn run_command_captured(command: &mut Command) -> io::Result { - command - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()); - let mut child = command.spawn()?; - - let mut child_err = child.stderr.take(); - let stderr_thread = std::thread::spawn(move || { - let mut buf = Vec::new(); - if let Some(err) = child_err.as_mut() { - let _ = err.read_to_end(&mut buf); - } - buf - }); - - if let Some(mut out) = child.stdout.take() { - let _ = io::copy(&mut out, &mut pi_uutils_ctx::stdout()); - } - let status = child.wait(); - if let Ok(buf) = stderr_thread.join() { - let _ = pi_uutils_ctx::stderr().write_all(&buf); - } - status -} - trait ArgumentReader { fn next(&mut self) -> io::Result>; } diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e150972d1..be942a238 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the in-process `find` builtin's `-exec`/`-execdir` children inheriting the omp process's real stdout/stderr, so commands like `find … -exec ls -ld {} \;` spammed their output straight into the terminal (corrupting the TUI) instead of the tool's captured output, and bypassed shell redirects. Exec children now stream stdout/stderr back through the shell's scope streams (matching `xargs`/`ifne`) and run with the shell's exported environment (`env_clear` + scope snapshot) instead of the host process environment. + ## [17.1.2] - 2026-07-24 ### Added