diff --git a/codex-rs/utils/pty/src/child_command.rs b/codex-rs/utils/pty/src/child_command.rs index ad5d6ee118..60a8195cd1 100644 --- a/codex-rs/utils/pty/src/child_command.rs +++ b/codex-rs/utils/pty/src/child_command.rs @@ -80,6 +80,10 @@ pub struct Command { #[cfg(target_os = "linux")] parent_pid: Option, #[cfg(unix)] + pub(crate) stdout_file: Option, + #[cfg(unix)] + pub(crate) stderr_file: Option, + #[cfg(unix)] pub(crate) arg0: Option, } @@ -107,6 +111,10 @@ impl Command { #[cfg(target_os = "linux")] parent_pid: None, #[cfg(unix)] + stdout_file: None, + #[cfg(unix)] + stderr_file: None, + #[cfg(unix)] arg0: None, } } @@ -298,6 +306,15 @@ impl Command { } } #[cfg(unix)] + { + if let Some(fd) = self.stdout_file { + self.inner.stdout(TokioStdio::from(fd)); + } + if let Some(fd) = self.stderr_file { + self.inner.stderr(TokioStdio::from(fd)); + } + } + #[cfg(unix)] let reaper = match self.drop_policy { ChildDropPolicy::KillAndReap => None, ChildDropPolicy::ReapOnly => Some(crate::child::reaper::sender()?), diff --git a/codex-rs/utils/pty/src/pipe.rs b/codex-rs/utils/pty/src/pipe.rs index dcf95e4717..3c7901b7d8 100644 --- a/codex-rs/utils/pty/src/pipe.rs +++ b/codex-rs/utils/pty/src/pipe.rs @@ -173,10 +173,11 @@ async fn spawn_process_with_stdin_mode( #[cfg(windows)] let job = crate::win::JobObject::create().map(Arc::new); #[cfg(target_os = "linux")] - let mut child = match crate::spawn_helper::spawn(&command).await? { - Some(child) => child, - None => command.spawn()?, - }; + let mut child = + match crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pipe).await? { + Some(child) => child, + None => command.spawn()?, + }; #[cfg(not(target_os = "linux"))] let mut child = command.spawn()?; #[cfg(windows)] diff --git a/codex-rs/utils/pty/src/posix_child.rs b/codex-rs/utils/pty/src/posix_child.rs index bf82774794..73ce7b2995 100644 --- a/codex-rs/utils/pty/src/posix_child.rs +++ b/codex-rs/utils/pty/src/posix_child.rs @@ -148,15 +148,19 @@ impl NativeChild { crate::ChildStdin::Null => (std::fs::File::open("/dev/null")?.into(), None), crate::ChildStdin::File(fd) => (fd.try_clone()?, None), }; - let (stdout_read, stdout_write) = io::pipe()?; - let (stderr_read, stderr_write) = io::pipe()?; + let (stdout_read, stdout_write) = child_output(request.stdout_file.as_ref())?; + let (stderr_read, stderr_write) = child_output(request.stderr_file.as_ref())?; let child_fds = [ child_fd(stdin_read)?, - child_fd(stdout_write.into())?, - child_fd(stderr_write.into())?, + child_fd(stdout_write)?, + child_fd(stderr_write)?, ]; - let stdout = ChildStdout::from_std(OwnedFd::from(stdout_read).into())?; - let stderr = ChildStderr::from_std(OwnedFd::from(stderr_read).into())?; + let stdout = stdout_read + .map(|fd| ChildStdout::from_std(fd.into())) + .transpose()?; + let stderr = stderr_read + .map(|fd| ChildStderr::from_std(fd.into())) + .transpose()?; let mut actions = MaybeUninit::uninit(); // SAFETY: Successful initialization makes each object valid; RAII only @@ -184,7 +188,7 @@ impl NativeChild { let result = libc::posix_spawn_file_actions_adddup2(&mut actions.0, *target, *target); // macOS file actions can reject valid high descriptors below - // RLIMIT_NOFILE. The existing dup2 fallback supports them. + // RLIMIT_NOFILE. The compatibility backend can inherit them directly. if result == libc::EBADF && request.fallback == crate::SpawnFallback::Compatible { return Ok(None); } @@ -293,8 +297,8 @@ impl NativeChild { Ok(Some(crate::Child { inner: super::ChildKind::Native(child), stdin, - stdout: Some(stdout), - stderr: Some(stderr), + stdout, + stderr, })) } @@ -453,3 +457,13 @@ impl Drop for Attributes { } } } + +/// Wire a caller-owned output descriptor or return a new pipe for the parent. +fn child_output(file: Option<&OwnedFd>) -> io::Result<(Option, OwnedFd)> { + if let Some(fd) = file { + Ok((None, fd.try_clone()?)) + } else { + let (reader, writer) = io::pipe()?; + Ok((Some(reader.into()), writer.into())) + } +} diff --git a/codex-rs/utils/pty/src/pty.rs b/codex-rs/utils/pty/src/pty.rs index 1b848b33b7..19599cf077 100644 --- a/codex-rs/utils/pty/src/pty.rs +++ b/codex-rs/utils/pty/src/pty.rs @@ -8,12 +8,12 @@ use std::os::fd::AsRawFd; use std::os::fd::FromRawFd; #[cfg(unix)] use std::os::fd::RawFd; -#[cfg(unix)] +#[cfg(all(unix, not(target_os = "linux")))] use std::os::unix::process::CommandExt; use std::path::Path; -#[cfg(unix)] +#[cfg(all(unix, not(target_os = "linux")))] use std::process::Command as StdCommand; -#[cfg(unix)] +#[cfg(all(unix, not(target_os = "linux")))] use std::process::Stdio; use std::sync::Arc; use std::sync::Mutex as StdMutex; @@ -141,6 +141,18 @@ pub async fn spawn_process( #[cfg(not(unix))] let _ = inherited_fds; + #[cfg(target_os = "linux")] + if crate::spawn_helper::is_available() + && inherited_fds.is_empty() + && Path::new(program).is_absolute() + && arg0.is_none() + && cwd.is_dir() + { + // Keep portable-pty's PATH, argv0, and missing-cwd behavior for requests + // whose launch settings require its command builder. + return spawn_process_preserving_fds(program, args, cwd, env, arg0, size, inherited_fds) + .await; + } #[cfg(unix)] if !inherited_fds.is_empty() { return spawn_process_preserving_fds(program, args, cwd, env, arg0, size, inherited_fds) @@ -299,84 +311,99 @@ async fn spawn_process_preserving_fds( ) -> Result { let (master, slave) = open_unix_pty(size)?; let io = crate::unix_io::PtyIo::new(master.as_raw_fd())?; - let mut command = StdCommand::new(program); - if let Some(arg0) = arg0 { - command.arg0(arg0); - } - command.current_dir(cwd); - command.env_clear(); - for arg in args { - command.arg(arg); - } - for (key, value) in env { - command.env(key, value); - } - - // The child should see one terminal on all three stdio streams. Cloning - // the slave fd gives us three owned handles to the same PTY slave device - // so Command can wire them up independently as stdin/stdout/stderr. - let stdin = slave.try_clone()?; - let stdout = slave.try_clone()?; - let stderr = slave.try_clone()?; - let inherited_fds = inherited_fds.to_vec(); - - unsafe { + let uses_portable_status = inherited_fds.is_empty(); + let stdin_close_behavior = if inherited_fds.is_empty() { + crate::unix_io::StdinCloseBehavior::SendEof + } else { + crate::unix_io::StdinCloseBehavior::NoEof + }; + #[cfg(target_os = "linux")] + let mut child = { + let mut command = crate::Command::new(program); + if inherited_fds.is_empty() && !env.contains_key("SHELL") { + // portable-pty supplies the login shell when SHELL is absent. + let mut builder = CommandBuilder::new(program); + builder.env_clear(); + command.env("SHELL", builder.get_shell()); + } command - .stdin(Stdio::from(stdin)) - .stdout(Stdio::from(stdout)) - .stderr(Stdio::from(stderr)) - .pre_exec(move || { - for signo in &[ - libc::SIGCHLD, - libc::SIGHUP, - libc::SIGINT, - libc::SIGQUIT, - libc::SIGTERM, - libc::SIGALRM, - ] { - libc::signal(*signo, libc::SIG_DFL); + .args(args) + .current_dir(cwd) + .envs(env) + .drop_policy(crate::child_command::ChildDropPolicy::ReapOnly) + .descriptor_policy(crate::DescriptorPolicy::Explicit) + .preserve_fds(inherited_fds) + .stdin(crate::ChildStdin::File(slave.try_clone()?.into())); + if let Some(arg0) = arg0 { + command.arg0(arg0); + } + command.stdout_file = Some(slave.try_clone()?.into()); + command.stderr_file = Some(slave.try_clone()?.into()); + match crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pty).await? { + Some(child) => child, + None if inherited_fds.is_empty() => { + // Free the first PTY before allocating its portable replacement. + drop(command); + drop(io); + drop(slave); + drop(master); + return spawn_process_portable(program, args, cwd, env, arg0, size).await; + } + None => { + // Explicit descriptors select the shared fallback on Linux. Keep + // terminal setup there when this executable cannot run the helper. + unsafe { + command.inner.pre_exec(configure_child_terminal); } - - let empty_set: libc::sigset_t = std::mem::zeroed(); - libc::sigprocmask(libc::SIG_SETMASK, &empty_set, std::ptr::null_mut()); - - if libc::setsid() == -1 { - return Err(std::io::Error::last_os_error()); - } - - // stdin now refers to the PTY slave, so make that fd the - // controlling terminal for the child's new session. stdout and - // stderr point at clones of the same slave device. - #[allow(clippy::cast_lossless)] - if libc::ioctl(0, libc::TIOCSCTTY as _, 0) == -1 { - return Err(std::io::Error::last_os_error()); - } - - close_inherited_fds_except(&inherited_fds); - Ok(()) - }); - } - - let mut child = command.spawn()?; + command.spawn()? + } + } + }; + #[cfg(not(target_os = "linux"))] + let mut child = { + let mut command = StdCommand::new(program); + if let Some(arg0) = arg0 { + command.arg0(arg0); + } + command.current_dir(cwd).env_clear().args(args).envs(env); + let inherited_fds = inherited_fds.to_vec(); + // The child sees the same terminal on stdin, stdout, and stderr. + unsafe { + command + .stdin(Stdio::from(slave.try_clone()?)) + .stdout(Stdio::from(slave.try_clone()?)) + .stderr(Stdio::from(slave.try_clone()?)) + .pre_exec(move || { + configure_child_terminal()?; + close_inherited_fds_except(&inherited_fds); + Ok(()) + }); + } + command.spawn()? + }; drop(slave); + #[cfg(target_os = "linux")] + let process_group_id = child + .id() + .ok_or_else(|| std::io::Error::other("missing child pid"))?; + #[cfg(not(target_os = "linux"))] let process_group_id = child.id(); let (writer_tx, writer_rx) = mpsc::channel::>(128); let (stdout_tx, stdout_rx) = mpsc::channel::>(128); let (_stderr_tx, stderr_rx) = mpsc::channel::>(1); - let (reader_handle, writer_handle) = io.spawn( - stdout_tx, - writer_rx, - crate::unix_io::StdinCloseBehavior::NoEof, - ); + let (reader_handle, writer_handle) = io.spawn(stdout_tx, writer_rx, stdin_close_behavior); let (exit_tx, exit_rx) = oneshot::channel::(); let exit_status = Arc::new(AtomicBool::new(false)); let wait_exit_status = Arc::clone(&exit_status); let exit_code = Arc::new(StdMutex::new(None)); let wait_exit_code = Arc::clone(&exit_code); - let wait_handle: JoinHandle<()> = tokio::task::spawn_blocking(move || { - let code = match child.wait() { + let on_exit = move |status: std::io::Result| { + let code = match status { + Ok(status) if uses_portable_status => { + portable_pty::ExitStatus::from(status).exit_code() as i32 + } Ok(status) => exit_code_from_status(status), Err(_) => -1, }; @@ -385,7 +412,11 @@ async fn spawn_process_preserving_fds( *guard = Some(code); } let _ = exit_tx.send(code); - }); + }; + #[cfg(target_os = "linux")] + let wait_handle = tokio::spawn(async move { on_exit(child.wait().await) }); + #[cfg(not(target_os = "linux"))] + let wait_handle = tokio::task::spawn_blocking(move || on_exit(child.wait())); let handles = PtyHandles { _slave: None, @@ -561,3 +592,42 @@ pub(crate) fn close_inherited_fds_except(preserved_fds: &[RawFd]) { } } } + +/// Establish the controlling terminal and signal state before a PTY target exec. +/// This also runs in the legacy pre-exec callback, so it must stay async-signal-safe. +#[cfg(unix)] +pub(crate) fn configure_child_terminal() -> std::io::Result<()> { + // SAFETY: These operations only change the calling child's signal/session state. + unsafe { + for signo in &[ + libc::SIGCHLD, + libc::SIGHUP, + libc::SIGINT, + libc::SIGQUIT, + libc::SIGTERM, + libc::SIGALRM, + ] { + libc::signal(*signo, libc::SIG_DFL); + } + + let empty_set: libc::sigset_t = std::mem::zeroed(); + libc::sigprocmask(libc::SIG_SETMASK, &empty_set, std::ptr::null_mut()); + + if libc::setsid() == -1 { + return Err(std::io::Error::last_os_error()); + } + + // stdin now refers to the PTY slave, so make that fd the + // controlling terminal for the child's new session. stdout and + // stderr point at clones of the same slave device. + #[allow(clippy::cast_lossless)] + if libc::ioctl(0, libc::TIOCSCTTY as _, 0) == -1 { + return Err(std::io::Error::last_os_error()); + } + } + Ok(()) +} + +#[cfg(all(test, target_os = "linux"))] +#[path = "pty_linux_tests.rs"] +mod linux_tests; diff --git a/codex-rs/utils/pty/src/pty_linux_tests.rs b/codex-rs/utils/pty/src/pty_linux_tests.rs new file mode 100644 index 0000000000..25b4c170b4 --- /dev/null +++ b/codex-rs/utils/pty/src/pty_linux_tests.rs @@ -0,0 +1,69 @@ +//! Portable PTY fallback must release the unsuccessful native launch's resources. + +use std::collections::HashMap; +use std::io; +use std::os::fd::AsRawFd; +use std::os::fd::FromRawFd; +use std::os::fd::OwnedFd; +use std::path::Path; + +use pretty_assertions::assert_eq; + +#[tokio::test] +async fn portable_fallback_releases_native_pty_descriptors() -> anyhow::Result<()> { + if std::env::var_os("CODEX_TEST_PTY_FALLBACK_PRESSURE").is_none() { + let output = std::process::Command::new(std::env::current_exe()?) + .args([ + "--exact", + "pty::linux_tests::portable_fallback_releases_native_pty_descriptors", + "--nocapture", + ]) + .env("CODEX_TEST_PTY_FALLBACK_PRESSURE", "1") + .output()?; + assert!(output.status.success(), "{output:?}"); + return Ok(()); + } + let source = std::fs::File::open("/dev/null")?; + let mut limit = std::mem::MaybeUninit::::uninit(); + // SAFETY: This isolated subprocess owns its descriptor limit and reservations. + assert_eq!( + unsafe { libc::getrlimit(libc::RLIMIT_NOFILE, limit.as_mut_ptr()) }, + 0 + ); + let mut limit = unsafe { limit.assume_init() }; + limit.rlim_cur = limit.rlim_cur.min(128); + assert_eq!(unsafe { libc::setrlimit(libc::RLIMIT_NOFILE, &limit) }, 0); + let mut occupied = Vec::new(); + loop { + let fd = unsafe { libc::fcntl(source.as_raw_fd(), libc::F_DUPFD_CLOEXEC, 3) }; + if fd == -1 { + assert_eq!( + io::Error::last_os_error().raw_os_error(), + Some(libc::EMFILE) + ); + break; + } + occupied.push(unsafe { OwnedFd::from_raw_fd(fd) }); + } + // The initial PTY fits, but helper setup cannot. The portable replacement + // fits only after the initial PTY and its stdio copies have been closed. + anyhow::ensure!(occupied.len() >= 10, "not enough descriptors to reserve"); + occupied.truncate(occupied.len() - 10); + let mut child = crate::spawn_pty_process( + "/bin/sh", + &["-c".to_owned(), "printf fallback".to_owned()], + Path::new("/"), + &HashMap::from([("SHELL".to_owned(), "/bin/sh".to_owned())]), + &None, + crate::TerminalSize::default(), + &[], + ) + .await?; + drop(occupied); + let mut output = Vec::new(); + while let Some(bytes) = child.stdout_rx.recv().await { + output.extend(bytes); + } + assert_eq!((child.exit_rx.await?, output), (0, b"fallback".to_vec())); + Ok(()) +} diff --git a/codex-rs/utils/pty/src/spawn_helper.rs b/codex-rs/utils/pty/src/spawn_helper.rs index 1437823b4b..488a5c5113 100644 --- a/codex-rs/utils/pty/src/spawn_helper.rs +++ b/codex-rs/utils/pty/src/spawn_helper.rs @@ -20,6 +20,14 @@ use tokio::io::AsyncWriteExt; use crate::child_command::ChildDropPolicy; +/// Setup that needs a fresh, single-threaded image before target exec. +pub(crate) enum Setup { + /// Detach from the terminal and terminate when the parent dies. + Pipe, + /// Establish a controlling terminal and reset interactive signal state. + Pty, +} + pub(super) const HELPER_ARG: &str = "--codex-run-as-process-setup"; pub(super) const MAX_ENV_BYTES: usize = 8 * 1024 * 1024; pub(super) const REPORT_PREFIX: u8 = 0; @@ -56,12 +64,19 @@ pub fn init_spawn_helper(args: impl IntoIterator) { } } +pub(crate) fn is_available() -> bool { + HELPER_READY.load(Ordering::Relaxed) +} + /// Start the helper and await target exec without blocking a runtime worker. /// Return `None` when registration, helper resource setup, or helper launch fails, /// preserving compatibility with callers and sandboxes that only permit the target. -pub(crate) async fn spawn(target: &crate::Command) -> io::Result> { +pub(crate) async fn spawn( + target: &crate::Command, + setup: Setup, +) -> io::Result> { target.validate()?; - if !HELPER_READY.load(Ordering::Relaxed) { + if !is_available() { return Ok(None); } let settings = target.inner.as_std(); @@ -88,6 +103,10 @@ pub(crate) async fn spawn(target: &crate::Command) -> io::Result "pipe", + Setup::Pty => "pty", + }) .arg( target .inherited_fds @@ -106,6 +125,16 @@ pub(crate) async fn spawn(target: &crate::Command) -> io::Result crate::ChildStdin::Null, crate::ChildStdin::File(fd) => crate::ChildStdin::File(fd.try_clone()?), }); + helper.stdout_file = target + .stdout_file + .as_ref() + .map(std::os::fd::OwnedFd::try_clone) + .transpose()?; + helper.stderr_file = target + .stderr_file + .as_ref() + .map(std::os::fd::OwnedFd::try_clone) + .transpose()?; helper.inherited_fds = target.inherited_fds.clone(); helper.inherited_fds.push(fd); Ok((control, helper, helper_control)) diff --git a/codex-rs/utils/pty/src/spawn_helper_main.rs b/codex-rs/utils/pty/src/spawn_helper_main.rs index 8763e0bf8e..e874a2ac83 100644 --- a/codex-rs/utils/pty/src/spawn_helper_main.rs +++ b/codex-rs/utils/pty/src/spawn_helper_main.rs @@ -1,4 +1,4 @@ -//! Early executable dispatch for Linux pipe spawns. All child setup happens +//! Early executable dispatch for Linux pipe and PTY spawns. All child setup happens //! after exec, before application initialization or runtime threads exist. //! A report prefix precedes target exec; failures append errno before exit. @@ -31,6 +31,14 @@ pub(super) fn dispatch(mut args: impl Iterator) -> ! .and_then(|arg| arg.to_str()?.parse::().ok()) .filter(|pid| *pid > 0) .ok_or_else(|| io::Error::from_raw_os_error(libc::EINVAL))?; + let setup = args + .next() + .ok_or_else(|| io::Error::from_raw_os_error(libc::EINVAL))?; + let setup = match setup.to_str() { + Some("pipe") => crate::spawn_helper::Setup::Pipe, + Some("pty") => crate::spawn_helper::Setup::Pty, + _ => return Err(io::Error::from_raw_os_error(libc::EINVAL)), + }; let preserved = args .next() .ok_or_else(|| io::Error::from_raw_os_error(libc::EINVAL))?; @@ -57,8 +65,13 @@ pub(super) fn dispatch(mut args: impl Iterator) -> ! if unsafe { libc::fcntl(control_fd, libc::F_SETFD, libc::FD_CLOEXEC) } == -1 { return Err(io::Error::last_os_error()); } - crate::process_group::detach_from_tty()?; - crate::process_group::set_parent_death_signal(parent_pid)?; + match setup { + crate::spawn_helper::Setup::Pipe => { + crate::process_group::detach_from_tty()?; + crate::process_group::set_parent_death_signal(parent_pid)?; + } + crate::spawn_helper::Setup::Pty => crate::pty::configure_child_terminal()?, + } // Allocation is safe in this fresh, single-threaded image. CLOEXEC // leaves the report socket open until the target actually execs. crate::pty::close_inherited_fds_except(&inherited_fds); diff --git a/codex-rs/utils/pty/src/spawn_helper_tests.rs b/codex-rs/utils/pty/src/spawn_helper_tests.rs index bc85d20b5e..771af1a07e 100644 --- a/codex-rs/utils/pty/src/spawn_helper_tests.rs +++ b/codex-rs/utils/pty/src/spawn_helper_tests.rs @@ -179,7 +179,9 @@ async fn native_helper_available() -> anyhow::Result { return Ok(false); } let command = crate::Command::new("/bin/true"); - let Some(child) = crate::spawn_helper::spawn(&command).await? else { + let Some(child) = + crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pipe).await? + else { return Ok(false); }; Ok(child.wait_with_output().await?.status.success()) @@ -263,7 +265,8 @@ async fn target_loader_environment_does_not_disable_helper_dispatch() -> anyhow: let mut command = crate::Command::new("/codex-missing-loader-env-target"); command.current_dir("/").env("LD_TRACE_LOADED_OBJECTS", "1"); // A failed helper bootstrap returns None; require the helper's target error. - let Err(error) = crate::spawn_helper::spawn(&command).await else { + let Err(error) = crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pipe).await + else { panic!("helper must reach target exec despite loader settings"); }; assert_eq!(error.raw_os_error(), Some(libc::ENOENT)); @@ -285,9 +288,12 @@ async fn large_environment_can_cross_the_control_socket() -> anyhow::Result<()> ]) .current_dir("/") .envs((0..8).map(|index| (format!("LARGE_{index}"), "x".repeat(8_192)))); - let child = tokio::time::timeout(Duration::from_secs(5), crate::spawn_helper::spawn(&command)) - .await?? - .expect("native helper must transfer the environment"); + let child = tokio::time::timeout( + Duration::from_secs(5), + crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pipe), + ) + .await?? + .expect("native helper must transfer the environment"); assert!(child.wait_with_output().await?.status.success()); Ok(()) } @@ -311,7 +317,7 @@ async fn helper_preserves_inheritable_fds_and_closes_unrequested_fds() -> anyhow .current_dir("/") .descriptor_policy(crate::DescriptorPolicy::Explicit) .preserve_fds(&[preserved]); - let child = crate::spawn_helper::spawn(&command) + let child = crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pipe) .await? .expect("native helper must preserve requested descriptors"); assert!(child.wait_with_output().await?.status.success()); @@ -550,6 +556,64 @@ async fn helper_exit_before_acknowledgement_falls_back() -> anyhow::Result<()> { wait_for_fallback_reaping(pid) } +#[tokio::test] +async fn pty_helper_establishes_a_controlling_terminal_without_forking() -> anyhow::Result<()> { + use std::cell::Cell; + thread_local! { static FORKS: Cell = const { Cell::new(0) }; } + extern "C" fn after_fork() { + FORKS.with(|count| count.set(count.get() + 1)); + } + FORKS.with(|count| count.set(0)); + // SAFETY: The parent callback only updates initialized thread-local storage. + assert_eq!( + unsafe { + libc::pthread_atfork(/*prepare*/ None, Some(after_fork), /*child*/ None) + }, + 0 + ); + let mut spawned = crate::spawn_pty_process( + "/bin/sh", + &[ + "-c".to_owned(), + "test -t 0 && test -t 1 && test -t 2 && printf ready >/dev/tty".to_owned(), + ], + std::path::Path::new("."), + &std::env::vars().collect(), + /*arg0*/ &None, + crate::TerminalSize::default(), + &[], + ) + .await?; + let mut output = Vec::new(); + while let Some(chunk) = spawned.stdout_rx.recv().await { + output.extend(chunk); + } + assert_eq!((spawned.exit_rx.await?, output), (0, b"ready".to_vec())); + if native_helper_available().await? { + assert_eq!(FORKS.with(Cell::get), 0); + } + Ok(()) +} + +#[tokio::test] +async fn pty_helper_preserves_portable_signal_exit_status() -> anyhow::Result<()> { + let env = std::env::vars().collect(); + for program in ["/bin/sh", "sh"] { + let spawned = crate::spawn_pty_process( + program, + &["-c".to_owned(), "kill -TERM $$".to_owned()], + std::path::Path::new("."), + &env, + /*arg0*/ &None, + crate::TerminalSize::default(), + &[], + ) + .await?; + assert_eq!(spawned.exit_rx.await?, 1); + } + Ok(()) +} + #[test] fn helper_dispatch_before_rust_initializes_argv() -> anyhow::Result<()> { let output = std::process::Command::new(std::env::current_exe()?) @@ -605,7 +669,9 @@ async fn helper_control_socket_survives_closed_stdio() -> anyhow::Result<()> { } let mut command = crate::Command::new("/bin/echo"); command.arg("closed-stdio").stdin(crate::ChildStdin::Null); - let Some(probe) = crate::spawn_helper::spawn(&command).await? else { + let Some(probe) = + crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pipe).await? + else { return Ok(()); }; assert!(probe.wait_with_output().await?.status.success()); @@ -618,7 +684,7 @@ async fn helper_control_socket_survives_closed_stdio() -> anyhow::Result<()> { for fd in [0, 1] { assert_eq!(unsafe { libc::close(fd) }, 0); } - let result = crate::spawn_helper::spawn(&command).await; + let result = crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pipe).await; for (fd, saved) in [0, 1].into_iter().zip(saved) { assert_eq!(unsafe { libc::dup2(saved.as_raw_fd(), fd) }, fd); } @@ -692,7 +758,11 @@ async fn helper_descriptor_pressure_falls_back_to_direct_spawn() -> anyhow::Resu // Direct spawning fits, but the helper also clones stdin and opens its control socket. anyhow::ensure!(occupied.len() >= 7, "not enough descriptors to reserve"); occupied.truncate(occupied.len() - 7); - assert!(crate::spawn_helper::spawn(&command).await?.is_none()); + assert!( + crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pipe) + .await? + .is_none() + ); let output = command.spawn()?.wait_with_output().await?; assert_eq!( (output.status.code(), output.stdout, output.stderr), @@ -723,7 +793,8 @@ async fn parent_loader_environment_does_not_disable_helper_dispatch() -> anyhow: } let mut command = crate::Command::new("/codex-missing-parent-loader-env-target"); command.current_dir("/"); - let Err(error) = crate::spawn_helper::spawn(&command).await else { + let Err(error) = crate::spawn_helper::spawn(&command, crate::spawn_helper::Setup::Pipe).await + else { panic!("helper must reach target exec despite parent loader settings"); }; assert_eq!(error.raw_os_error(), Some(libc::ENOENT)); diff --git a/codex-rs/utils/pty/src/tests.rs b/codex-rs/utils/pty/src/tests.rs index 8a7cca2499..8283a22e7a 100644 --- a/codex-rs/utils/pty/src/tests.rs +++ b/codex-rs/utils/pty/src/tests.rs @@ -20,11 +20,12 @@ mod windows_tests; pub(super) fn find_python() -> Option { for candidate in ["python3", "python"] { if let Ok(output) = std::process::Command::new(candidate) - .arg("--version") + .args(["-c", "import sys; print(sys.executable)"]) + .env("PYTHONIOENCODING", "utf-8") .output() && output.status.success() { - return Some(candidate.to_string()); + return Some(String::from_utf8(output.stdout).ok()?.trim().to_owned()); } } None