diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 22d1deeb1c..9766992819 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -371,6 +371,7 @@ async fn consume_truncated_output( } Err(_) => { // timeout + kill_child_process_group(&mut child)?; child.start_kill()?; // Debatable whether `child.wait().await` should be called here. (synthetic_exit_status(EXIT_CODE_SIGNAL_BASE + TIMEOUT_CODE), true) @@ -378,6 +379,7 @@ async fn consume_truncated_output( } } _ = tokio::signal::ctrl_c() => { + kill_child_process_group(&mut child)?; child.start_kill()?; (synthetic_exit_status(EXIT_CODE_SIGNAL_BASE + SIGKILL_CODE), false) } @@ -473,6 +475,38 @@ fn synthetic_exit_status(code: i32) -> ExitStatus { std::process::ExitStatus::from_raw(code.try_into().unwrap()) } +#[cfg(unix)] +fn kill_child_process_group(child: &mut Child) -> io::Result<()> { + use std::io::ErrorKind; + + if let Some(pid) = child.id() { + let pid = pid as libc::pid_t; + let pgid = unsafe { libc::getpgid(pid) }; + if pgid == -1 { + let err = std::io::Error::last_os_error(); + if err.kind() != ErrorKind::NotFound { + return Err(err); + } + return Ok(()); + } + + let result = unsafe { libc::killpg(pgid, libc::SIGKILL) }; + if result == -1 { + let err = std::io::Error::last_os_error(); + if err.kind() != ErrorKind::NotFound { + return Err(err); + } + } + } + + Ok(()) +} + +#[cfg(not(unix))] +fn kill_child_process_group(_: &mut Child) -> io::Result<()> { + Ok(()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/core/src/spawn.rs b/codex-rs/core/src/spawn.rs index 1c82df3180..ce08c35dbd 100644 --- a/codex-rs/core/src/spawn.rs +++ b/codex-rs/core/src/spawn.rs @@ -64,22 +64,29 @@ pub(crate) async fn spawn_child_async( // any child processes that were spawned as part of a `"shell"` tool call // to also be terminated. - // This relies on prctl(2), so it only works on Linux. - #[cfg(target_os = "linux")] + #[cfg(unix)] unsafe { cmd.pre_exec(|| { - // This prctl call effectively requests, "deliver SIGTERM when my - // current parent dies." - if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) == -1 { + if libc::setpgid(0, 0) == -1 { return Err(std::io::Error::last_os_error()); } - // Though if there was a race condition and this pre_exec() block is - // run _after_ the parent (i.e., the Codex process) has already - // exited, then the parent is the _init_ process (which will never - // die), so we should just terminate the child process now. - if libc::getppid() == 1 { - libc::raise(libc::SIGTERM); + // This relies on prctl(2), so it only works on Linux. + #[cfg(target_os = "linux")] + { + // This prctl call effectively requests, "deliver SIGTERM when my + // current parent dies." + if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) == -1 { + return Err(std::io::Error::last_os_error()); + } + + // Though if there was a race condition and this pre_exec() block is + // run _after_ the parent (i.e., the Codex process) has already + // exited, then the parent is the _init_ process (which will never + // die), so we should just terminate the child process now. + if libc::getppid() == 1 { + libc::raise(libc::SIGTERM); + } } Ok(()) }); diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index bb1df187c2..8419a7885c 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -167,7 +167,7 @@ fn create_unified_exec_tool() -> ToolSpec { "timeout_ms".to_string(), JsonSchema::Number { description: Some( - "Maximum time in milliseconds to wait for output after writing the input." + "Maximum time in milliseconds to wait for output after writing the input (default: 1000)." .to_string(), ), }, @@ -204,7 +204,9 @@ fn create_shell_tool() -> ToolSpec { properties.insert( "timeout_ms".to_string(), JsonSchema::Number { - description: Some("The timeout for the command in milliseconds".to_string()), + description: Some( + "The timeout for the command in milliseconds (default: 1000).".to_string(), + ), }, ); diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 4952aa01d9..4d062b2a43 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -247,7 +247,7 @@ pub struct ShellToolCallParams { pub command: Vec, pub workdir: Option, - /// This is the maximum time in milliseconds that the command is allowed to run. + /// Maximum time in milliseconds that the command is allowed to run (defaults to 1_000 ms when omitted). #[serde(alias = "timeout")] pub timeout_ms: Option, #[serde(skip_serializing_if = "Option::is_none")]