Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions codex-rs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 1 addition & 32 deletions codex-rs/core/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use crate::sandboxing::SandboxPermissions;
use crate::spawn::StdioPolicy;
use crate::spawn::spawn_child_async;
use crate::text_encoding::bytes_to_string_smart;
use codex_utils_pty::process_group::kill_child_process_group;

pub const DEFAULT_EXEC_COMMAND_TIMEOUT_MS: u64 = 10_000;

Expand Down Expand Up @@ -750,38 +751,6 @@ fn synthetic_exit_status(code: i32) -> ExitStatus {
std::process::ExitStatus::from_raw(code as u32)
}

#[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::*;
Expand Down
17 changes: 3 additions & 14 deletions codex-rs/core/src/spawn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,27 +70,16 @@ pub(crate) async fn spawn_child_async(
#[cfg(target_os = "linux")]
let parent_pid = libc::getpid();
cmd.pre_exec(move || {
if set_process_group && libc::setpgid(0, 0) == -1 {
return Err(std::io::Error::last_os_error());
if set_process_group {
codex_utils_pty::process_group::set_process_group()?;
}

// 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 parent will be the closest configured "subreaper"
// ancestor process, or PID 1 (init). If the Codex process has exited
// already, so should the child process.
if libc::getppid() != parent_pid {
libc::raise(libc::SIGTERM);
}
codex_utils_pty::process_group::set_parent_death_signal(parent_pid)?;
}
Ok(())
});
Expand Down
8 changes: 8 additions & 0 deletions codex-rs/core/src/tools/handlers/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ struct ExecCommandArgs {
shell: Option<String>,
#[serde(default = "default_login")]
login: bool,
#[serde(default = "default_tty")]
tty: bool,
#[serde(default = "default_exec_yield_time_ms")]
yield_time_ms: u64,
#[serde(default)]
Expand Down Expand Up @@ -67,6 +69,10 @@ fn default_login() -> bool {
true
}

fn default_tty() -> bool {
false
}

#[async_trait]
impl ToolHandler for UnifiedExecHandler {
fn kind(&self) -> ToolKind {
Expand Down Expand Up @@ -124,6 +130,7 @@ impl ToolHandler for UnifiedExecHandler {

let ExecCommandArgs {
workdir,
tty,
yield_time_ms,
max_output_tokens,
sandbox_permissions,
Expand Down Expand Up @@ -173,6 +180,7 @@ impl ToolHandler for UnifiedExecHandler {
yield_time_ms,
max_output_tokens,
workdir,
tty,
sandbox_permissions,
justification,
},
Expand Down
7 changes: 6 additions & 1 deletion codex-rs/core/src/tools/runtimes/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub struct UnifiedExecRequest {
pub command: Vec<String>,
pub cwd: PathBuf,
pub env: HashMap<String, String>,
pub tty: bool,
pub sandbox_permissions: SandboxPermissions,
pub justification: Option<String>,
pub exec_approval_requirement: ExecApprovalRequirement,
Expand All @@ -46,6 +47,7 @@ pub struct UnifiedExecRequest {
pub struct UnifiedExecApprovalKey {
pub command: Vec<String>,
pub cwd: PathBuf,
pub tty: bool,
pub sandbox_permissions: SandboxPermissions,
}

Expand All @@ -58,6 +60,7 @@ impl UnifiedExecRequest {
command: Vec<String>,
cwd: PathBuf,
env: HashMap<String, String>,
tty: bool,
sandbox_permissions: SandboxPermissions,
justification: Option<String>,
exec_approval_requirement: ExecApprovalRequirement,
Expand All @@ -66,6 +69,7 @@ impl UnifiedExecRequest {
command,
cwd,
env,
tty,
sandbox_permissions,
justification,
exec_approval_requirement,
Expand Down Expand Up @@ -96,6 +100,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
vec![UnifiedExecApprovalKey {
command: req.command.clone(),
cwd: req.cwd.clone(),
tty: req.tty,
sandbox_permissions: req.sandbox_permissions,
}]
}
Expand Down Expand Up @@ -189,7 +194,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
.env_for(spec)
.map_err(|err| ToolError::Codex(err.into()))?;
self.manager
.open_session_with_exec_env(&exec_env)
.open_session_with_exec_env(&exec_env, req.tty)
.await
.map_err(|err| match err {
UnifiedExecError::SandboxDenied { output, .. } => {
Expand Down
9 changes: 9 additions & 0 deletions codex-rs/core/src/tools/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,15 @@ fn create_exec_command_tool() -> ToolSpec {
),
},
),
(
"tty".to_string(),
JsonSchema::Boolean {
description: Some(
"Whether to allocate a TTY for the command. Defaults to false (plain pipes); set to true to open a PTY and access TTY process."
.to_string(),
),
}
),
(
"yield_time_ms".to_string(),
JsonSchema::Number {
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/core/src/unified_exec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pub(crate) struct ExecCommandRequest {
pub yield_time_ms: u64,
pub max_output_tokens: Option<usize>,
pub workdir: Option<PathBuf>,
pub tty: bool,
pub sandbox_permissions: SandboxPermissions,
pub justification: Option<String>,
}
Expand Down Expand Up @@ -200,6 +201,7 @@ mod tests {
yield_time_ms,
max_output_tokens: None,
workdir: None,
tty: true,
sandbox_permissions: SandboxPermissions::UseDefault,
justification: None,
},
Expand Down
34 changes: 25 additions & 9 deletions codex-rs/core/src/unified_exec/process_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ impl UnifiedExecProcessManager {
cwd.clone(),
request.sandbox_permissions,
request.justification,
request.tty,
context,
)
.await;
Expand Down Expand Up @@ -471,21 +472,34 @@ impl UnifiedExecProcessManager {
pub(crate) async fn open_session_with_exec_env(
&self,
env: &ExecEnv,
tty: bool,
) -> Result<UnifiedExecProcess, UnifiedExecError> {
let (program, args) = env
.command
.split_first()
.ok_or(UnifiedExecError::MissingCommandLine)?;

let spawned = codex_utils_pty::spawn_pty_process(
program,
args,
env.cwd.as_path(),
&env.env,
&env.arg0,
)
.await
.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;
let spawn_result = if tty {
codex_utils_pty::pty::spawn_process(
program,
args,
env.cwd.as_path(),
&env.env,
&env.arg0,
)
.await
} else {
codex_utils_pty::pipe::spawn_process(
program,
args,
env.cwd.as_path(),
&env.env,
&env.arg0,
)
.await
};
let spawned =
spawn_result.map_err(|err| UnifiedExecError::create_process(err.to_string()))?;
UnifiedExecProcess::from_spawned(spawned, env.sandbox).await
}

Expand All @@ -495,6 +509,7 @@ impl UnifiedExecProcessManager {
cwd: PathBuf,
sandbox_permissions: SandboxPermissions,
justification: Option<String>,
tty: bool,
context: &UnifiedExecContext,
) -> Result<UnifiedExecProcess, UnifiedExecError> {
let env = apply_unified_exec_env(create_env(&context.turn.shell_environment_policy));
Expand All @@ -517,6 +532,7 @@ impl UnifiedExecProcessManager {
command.to_vec(),
cwd,
env,
tty,
sandbox_permissions,
justification,
exec_approval_requirement,
Expand Down
16 changes: 14 additions & 2 deletions codex-rs/core/tests/suite/rmcp_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,13 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> {
.await;

let expected_env_value = "propagated-env-http";
let rmcp_http_server_bin = cargo_bin("test_streamable_http_server")?;
let rmcp_http_server_bin = match cargo_bin("test_streamable_http_server") {
Ok(path) => path,
Err(err) => {
eprintln!("test_streamable_http_server binary not available, skipping test: {err}");
return Ok(());
}
};

let listener = TcpListener::bind("127.0.0.1:0")?;
let port = listener.local_addr()?.port();
Expand Down Expand Up @@ -819,7 +825,13 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> {
let expected_token = "initial-access-token";
let client_id = "test-client-id";
let refresh_token = "initial-refresh-token";
let rmcp_http_server_bin = cargo_bin("test_streamable_http_server")?;
let rmcp_http_server_bin = match cargo_bin("test_streamable_http_server") {
Ok(path) => path,
Err(err) => {
eprintln!("test_streamable_http_server binary not available, skipping test: {err}");
return Ok(());
}
};

let listener = TcpListener::bind("127.0.0.1:0")?;
let port = listener.local_addr()?.port();
Expand Down
Loading
Loading