diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 0a6771151f..60cfd00eff 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -57,6 +57,8 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use codex_utils_pty::process_group::kill_child_process_group; +use codex_utils_pty::process_group::kill_process_group; +use codex_utils_pty::process_group::terminate_process_group; pub const DEFAULT_EXEC_COMMAND_TIMEOUT_MS: u64 = 10_000; @@ -1051,7 +1053,7 @@ async fn consume_output( // remaining members of the original process group. let process_group_id = child.id(); let should_escalate = if let Some(process_group_id) = process_group_id { - codex_utils_pty::process_group::terminate_process_group(process_group_id)? + terminate_process_group(process_group_id)? } else { false }; @@ -1066,13 +1068,13 @@ async fn consume_output( if should_escalate && let Some(process_group_id) = process_group_id { - codex_utils_pty::process_group::kill_process_group( - process_group_id, - )?; + kill_process_group(process_group_id)?; } } Err(_) => { - kill_child_process_group(&mut child)?; + if let Some(process_group_id) = process_group_id { + kill_process_group(process_group_id)?; + } child.start_kill()?; } } @@ -1156,8 +1158,7 @@ async fn consume_output( match drained { Some(Ok(output)) => output, failure => { - let cleanup = process_group_id - .map_or(Ok(()), codex_utils_pty::process_group::kill_process_group); + let cleanup = process_group_id.map_or(Ok(()), kill_process_group); stdout_handle.abort(); stderr_handle.abort(); if !stdout_done { diff --git a/codex-rs/rmcp-client/src/http_headers.rs b/codex-rs/rmcp-client/src/http_headers.rs index d0b2d28c98..59667de368 100644 --- a/codex-rs/rmcp-client/src/http_headers.rs +++ b/codex-rs/rmcp-client/src/http_headers.rs @@ -19,10 +19,8 @@ use codex_exec_server::HttpRedirectPolicy; use codex_exec_server::HttpRequestParams; use codex_exec_server::HttpRequestResponse; use codex_exec_server::HttpResponseBodyStream; -#[cfg(all(unix, not(target_os = "macos")))] +#[cfg(unix)] use codex_utils_pty::process_group::kill_process_group; -#[cfg(target_os = "macos")] -use codex_utils_pty::process_group::kill_process_group_with_member_fallback as kill_process_group; use futures::FutureExt; use futures::future::BoxFuture; use futures::future::Shared; diff --git a/codex-rs/rmcp-client/src/stdio_server_launcher.rs b/codex-rs/rmcp-client/src/stdio_server_launcher.rs index 84cc008e0f..9ef3b18c4d 100644 --- a/codex-rs/rmcp-client/src/stdio_server_launcher.rs +++ b/codex-rs/rmcp-client/src/stdio_server_launcher.rs @@ -38,14 +38,10 @@ use codex_exec_server::ExecProcess; use codex_protocol::config_types::ShellEnvironmentPolicyInherit; use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathUri; -#[cfg(all(unix, not(target_os = "macos")))] +#[cfg(unix)] use codex_utils_pty::process_group::kill_process_group; -#[cfg(target_os = "macos")] -use codex_utils_pty::process_group::kill_process_group_with_member_fallback as kill_process_group; -#[cfg(all(unix, not(target_os = "macos")))] +#[cfg(unix)] use codex_utils_pty::process_group::terminate_process_group; -#[cfg(target_os = "macos")] -use codex_utils_pty::process_group::terminate_process_group_with_member_fallback as terminate_process_group; use futures::FutureExt; use futures::future::BoxFuture; use rmcp::service::RoleClient; diff --git a/codex-rs/utils/pty/src/pipe.rs b/codex-rs/utils/pty/src/pipe.rs index e189c13be6..867d246452 100644 --- a/codex-rs/utils/pty/src/pipe.rs +++ b/codex-rs/utils/pty/src/pipe.rs @@ -63,16 +63,11 @@ impl ChildTerminator for PipeChildTerminator { } fn kill(&mut self) -> io::Result<()> { - #[cfg(all(unix, not(target_os = "macos")))] + #[cfg(unix)] { crate::process_group::kill_process_group(self.process_group_id) } - #[cfg(target_os = "macos")] - { - crate::process_group::kill_process_group_with_member_fallback(self.process_group_id) - } - #[cfg(windows)] { match &self.windows { diff --git a/codex-rs/utils/pty/src/process_group.rs b/codex-rs/utils/pty/src/process_group.rs index 3d2e3ca452..f403b7e647 100644 --- a/codex-rs/utils/pty/src/process_group.rs +++ b/codex-rs/utils/pty/src/process_group.rs @@ -13,6 +13,8 @@ //! `SIGTERM` when the parent exits, and re-checks the parent PID to avoid //! races during fork/exec. //! +//! On macOS, `terminate_process_group` and `kill_process_group` retry denied +//! group signals against individual members. //! On non-Unix platforms these helpers are no-ops. use std::io; @@ -222,7 +224,7 @@ fn signal_process_group_with_member_fallback( } } -#[cfg(unix)] +#[cfg(all(unix, not(target_os = "macos")))] /// Send SIGTERM to a specific process group ID (best-effort). /// /// Returns `Ok(true)` when SIGTERM was delivered to an existing group and @@ -232,8 +234,10 @@ pub fn terminate_process_group(process_group_id: u32) -> io::Result { } #[cfg(target_os = "macos")] -/// Retry a denied SIGTERM against the exact group's individual members. -pub fn terminate_process_group_with_member_fallback(process_group_id: u32) -> io::Result { +/// Send SIGTERM to a specific process group, retrying denied signals against its members. +/// +/// Returns `Ok(true)` when the group or at least one member was signalled. +pub fn terminate_process_group(process_group_id: u32) -> io::Result { signal_process_group_with_member_fallback( process_group_id, libc::SIGTERM, @@ -260,15 +264,15 @@ pub fn interrupt_process_group(_process_group_id: u32) -> io::Result<()> { Ok(()) } -#[cfg(unix)] +#[cfg(all(unix, not(target_os = "macos")))] /// Kill a specific process group ID (best-effort). pub fn kill_process_group(process_group_id: u32) -> io::Result<()> { signal_process_group_id(process_group_id as libc::pid_t, libc::SIGKILL).map(|_| ()) } #[cfg(target_os = "macos")] -/// Retry a denied SIGKILL against the exact group's individual members. -pub fn kill_process_group_with_member_fallback(process_group_id: u32) -> io::Result<()> { +/// Kill a specific process group, retrying denied signals against its members (best-effort). +pub fn kill_process_group(process_group_id: u32) -> io::Result<()> { signal_process_group_with_member_fallback( process_group_id, libc::SIGKILL, diff --git a/codex-rs/utils/pty/src/process_group_tests.rs b/codex-rs/utils/pty/src/process_group_tests.rs index 4b580ccb95..a3c8f3c3d0 100644 --- a/codex-rs/utils/pty/src/process_group_tests.rs +++ b/codex-rs/utils/pty/src/process_group_tests.rs @@ -12,7 +12,7 @@ use tokio::time::timeout; use super::signal_process_group_with_member_fallback; use super::signal_process_id; -use super::terminate_process_group_with_member_fallback; +use super::terminate_process_group; #[tokio::test] async fn denied_group_signal_terminates_owned_descendants_and_preserves_escalation() -> Result<()> { @@ -87,7 +87,7 @@ async fn denied_group_signal_terminates_owned_descendants_and_preserves_escalati #[test] fn denied_group_signal_rejects_unsafe_process_group_ids() { for process_group_id in [0, u32::MAX] { - let error = terminate_process_group_with_member_fallback(process_group_id) + let error = terminate_process_group(process_group_id) .expect_err("unsafe process group ID should be rejected"); assert_eq!(error.kind(), io::ErrorKind::InvalidInput); }