diff --git a/codex-rs/core/src/shell_snapshot.rs b/codex-rs/core/src/shell_snapshot.rs index 9c10eedf4..be316afa5 100644 --- a/codex-rs/core/src/shell_snapshot.rs +++ b/codex-rs/core/src/shell_snapshot.rs @@ -135,6 +135,13 @@ async fn run_shell_script_with_timeout( // returns a ref of handler. let mut handler = Command::new(&args[0]); handler.args(&args[1..]); + #[cfg(unix)] + unsafe { + handler.pre_exec(|| { + codex_utils_pty::process_group::detach_from_tty()?; + Ok(()) + }); + } handler.kill_on_drop(true); let output = timeout(snapshot_timeout, handler.output()) .await diff --git a/codex-rs/core/src/spawn.rs b/codex-rs/core/src/spawn.rs index c1a8d4457..b2a507fda 100644 --- a/codex-rs/core/src/spawn.rs +++ b/codex-rs/core/src/spawn.rs @@ -66,12 +66,12 @@ pub(crate) async fn spawn_child_async( #[cfg(unix)] unsafe { - let set_process_group = matches!(stdio_policy, StdioPolicy::RedirectForShellTool); + let detach_from_tty = matches!(stdio_policy, StdioPolicy::RedirectForShellTool); #[cfg(target_os = "linux")] let parent_pid = libc::getpid(); cmd.pre_exec(move || { - if set_process_group { - codex_utils_pty::process_group::set_process_group()?; + if detach_from_tty { + codex_utils_pty::process_group::detach_from_tty()?; } // This relies on prctl(2), so it only works on Linux. diff --git a/codex-rs/utils/pty/src/pipe.rs b/codex-rs/utils/pty/src/pipe.rs index 5d9eb0232..d77c11383 100644 --- a/codex-rs/utils/pty/src/pipe.rs +++ b/codex-rs/utils/pty/src/pipe.rs @@ -118,7 +118,7 @@ async fn spawn_process_with_stdin_mode( #[cfg(unix)] unsafe { command.pre_exec(move || { - crate::process_group::set_process_group()?; + crate::process_group::detach_from_tty()?; #[cfg(target_os = "linux")] crate::process_group::set_parent_death_signal(parent_pid)?; Ok(()) diff --git a/codex-rs/utils/pty/src/process_group.rs b/codex-rs/utils/pty/src/process_group.rs index ae77a36be..dadff29f9 100644 --- a/codex-rs/utils/pty/src/process_group.rs +++ b/codex-rs/utils/pty/src/process_group.rs @@ -4,6 +4,8 @@ //! command can be cleaned up reliably: //! - `set_process_group` is called in `pre_exec` so the child starts its own //! process group. +//! - `detach_from_tty` starts a new session so non-interactive children do not +//! inherit the controlling TTY. //! - `kill_process_group_by_pid` targets the whole group (children/grandchildren) //! - `kill_process_group` targets a known process group ID directly //! instead of a single PID. @@ -42,6 +44,26 @@ pub fn set_parent_death_signal(_parent_pid: i32) -> io::Result<()> { Ok(()) } +#[cfg(unix)] +/// Detach from the controlling TTY by starting a new session. +pub fn detach_from_tty() -> io::Result<()> { + let result = unsafe { libc::setsid() }; + if result == -1 { + let err = io::Error::last_os_error(); + if err.raw_os_error() == Some(libc::EPERM) { + return set_process_group(); + } + return Err(err); + } + Ok(()) +} + +#[cfg(not(unix))] +/// No-op on non-Unix platforms. +pub fn detach_from_tty() -> io::Result<()> { + Ok(()) +} + #[cfg(unix)] /// Put the calling process into its own process group. /// diff --git a/codex-rs/utils/pty/src/tests.rs b/codex-rs/utils/pty/src/tests.rs index 8c41d1dbd..89e72eeb4 100644 --- a/codex-rs/utils/pty/src/tests.rs +++ b/codex-rs/utils/pty/src/tests.rs @@ -152,6 +152,49 @@ async fn pipe_process_round_trips_stdin() -> anyhow::Result<()> { Ok(()) } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn pipe_process_detaches_from_parent_session() -> anyhow::Result<()> { + let parent_sid = unsafe { libc::getsid(0) }; + if parent_sid == -1 { + anyhow::bail!("failed to read parent session id"); + } + + let env_map: HashMap = std::env::vars().collect(); + let script = "echo $$; sleep 0.2"; + let (program, args) = shell_command(script); + let spawned = spawn_pipe_process(&program, &args, Path::new("."), &env_map, &None).await?; + + let mut output_rx = spawned.output_rx; + let pid_bytes = + tokio::time::timeout(tokio::time::Duration::from_millis(500), output_rx.recv()).await??; + let pid_text = String::from_utf8_lossy(&pid_bytes); + let child_pid: i32 = pid_text + .split_whitespace() + .next() + .ok_or_else(|| anyhow::anyhow!("missing child pid output: {pid_text:?}"))? + .parse()?; + + let child_sid = unsafe { libc::getsid(child_pid) }; + if child_sid == -1 { + anyhow::bail!("failed to read child session id"); + } + + assert_eq!(child_sid, child_pid, "expected child to be session leader"); + assert_ne!( + child_sid, parent_sid, + "expected child to be detached from parent session" + ); + + let exit_code = spawned.exit_rx.await.unwrap_or(-1); + assert_eq!( + exit_code, 0, + "expected detached pipe process to exit cleanly" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn pipe_and_pty_share_interface() -> anyhow::Result<()> { let env_map: HashMap = std::env::vars().collect();