Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions codex-rs/Cargo.lock

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

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 @@ -32,6 +32,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 @@ -66,6 +68,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 @@ -127,6 +133,7 @@ impl ToolHandler for UnifiedExecHandler {

let ExecCommandArgs {
workdir,
tty,
yield_time_ms,
max_output_tokens,
sandbox_permissions,
Expand Down Expand Up @@ -176,6 +183,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<'_> {
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, UnifiedExecSession> 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 @@ -162,6 +162,15 @@ fn create_exec_command_tool() -> ToolSpec {
),
},
);
properties.insert(
"tty".to_string(),
JsonSchema::Boolean {
description: Some(
"Whether to allocate a TTY for the command. Defaults to true (PTY); set to false for plain stdio pipes."
.to_string(),
),
},
);
properties.insert(
"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/session_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ impl UnifiedExecSessionManager {
cwd.clone(),
request.sandbox_permissions,
request.justification,
request.tty,
context,
)
.await;
Expand Down Expand Up @@ -471,21 +472,34 @@ impl UnifiedExecSessionManager {
pub(crate) async fn open_session_with_exec_env(
&self,
env: &ExecEnv,
tty: bool,
) -> Result<UnifiedExecSession, 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_session(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_session(err.to_string()))?;
UnifiedExecSession::from_spawned(spawned, env.sandbox).await
}

Expand All @@ -495,6 +509,7 @@ impl UnifiedExecSessionManager {
cwd: PathBuf,
sandbox_permissions: SandboxPermissions,
justification: Option<String>,
tty: bool,
context: &UnifiedExecContext,
) -> Result<UnifiedExecSession, UnifiedExecError> {
let env = apply_unified_exec_env(create_env(&context.turn.shell_environment_policy));
Expand All @@ -517,6 +532,7 @@ impl UnifiedExecSessionManager {
command.to_vec(),
cwd,
env,
tty,
sandbox_permissions,
justification,
exec_approval_requirement,
Expand Down
163 changes: 163 additions & 0 deletions codex-rs/core/tests/suite/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use regex_lite::Regex;
use serde_json::Value;
use serde_json::json;
use tokio::time::Duration;
use which::which;

fn extract_output_text(item: &Value) -> Option<&str> {
item.get("output").and_then(|value| match value {
Expand Down Expand Up @@ -1286,6 +1287,168 @@ async fn exec_command_reports_chunk_and_exit_metadata() -> Result<()> {
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn unified_exec_defaults_to_tty() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
skip_if_windows!(Ok(()));

let python = match which("python").or_else(|_| which("python3")) {
Ok(path) => path,
Err(_) => {
eprintln!("python not found in PATH, skipping tty default test.");
return Ok(());
}
};

let server = start_mock_server().await;

let mut builder = test_codex().with_config(|config| {
config.features.enable(Feature::UnifiedExec);
});
let TestCodex {
codex,
cwd,
session_configured,
..
} = builder.build(&server).await?;

let call_id = "uexec-tty-default";
let args = serde_json::json!({
"cmd": format!("{} -c \"import sys; print(sys.stdin.isatty())\"", python.display()),
"yield_time_ms": 1500,
});

let responses = vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"),
]),
];
mount_sse_sequence(&server, responses).await;

let session_model = session_configured.model.clone();

codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: "check tty default".into(),
}],
final_output_json_schema: None,
cwd: cwd.path().to_path_buf(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::DangerFullAccess,
model: session_model,
effort: None,
summary: ReasoningSummary::Auto,
})
.await?;

wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await;

let bodies = get_responses_request_bodies(&server).await;
let outputs = collect_tool_outputs(&bodies)?;
let output = outputs
.get(call_id)
.expect("missing tty default unified exec output");
let normalized = output.output.replace("\r\n", "\n");

assert!(
normalized.contains("True"),
"stdin should be a tty by default: {normalized:?}"
);
assert_eq!(output.exit_code, Some(0));
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn unified_exec_can_disable_tty() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_sandbox!(Ok(()));
skip_if_windows!(Ok(()));

let python = match which("python").or_else(|_| which("python3")) {
Ok(path) => path,
Err(_) => {
eprintln!("python not found in PATH, skipping non-tty test.");
return Ok(());
}
};

let server = start_mock_server().await;

let mut builder = test_codex().with_config(|config| {
config.features.enable(Feature::UnifiedExec);
});
let TestCodex {
codex,
cwd,
session_configured,
..
} = builder.build(&server).await?;

let call_id = "uexec-no-tty";
let args = serde_json::json!({
"cmd": format!("{} -c \"import sys; print(sys.stdin.isatty())\"", python.display()),
"yield_time_ms": 1500,
"tty": false,
});

let responses = vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?),
ev_completed("resp-1"),
]),
sse(vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"),
]),
];
mount_sse_sequence(&server, responses).await;

let session_model = session_configured.model.clone();

codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: "check tty disabled".into(),
}],
final_output_json_schema: None,
cwd: cwd.path().to_path_buf(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::DangerFullAccess,
model: session_model,
effort: None,
summary: ReasoningSummary::Auto,
})
.await?;

wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await;

let bodies = get_responses_request_bodies(&server).await;
let outputs = collect_tool_outputs(&bodies)?;
let output = outputs
.get(call_id)
.expect("missing non-tty unified exec output");
let normalized = output.output.replace("\r\n", "\n");

assert!(
normalized.contains("False"),
"stdin should not be a tty when tty=false: {normalized:?}"
);
assert_eq!(output.exit_code, Some(0));
assert!(output.process_id.is_none(), "process should have exited");
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn unified_exec_respects_early_exit_notifications() -> Result<()> {
skip_if_no_network!(Ok(()));
Expand Down
4 changes: 3 additions & 1 deletion codex-rs/utils/pty/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ workspace = true
[dependencies]
anyhow = { workspace = true }
portable-pty = { workspace = true }
tokio = { workspace = true, features = ["macros", "rt-multi-thread", "sync", "time"] }
tokio = { workspace = true, features = ["io-util", "macros", "process", "rt-multi-thread", "sync", "time"] }

[target.'cfg(windows)'.dependencies]
filedescriptor = "0.8.3"
Expand All @@ -27,3 +27,5 @@ winapi = { version = "0.3.9", features = [
"winerror",
"winnt",
] }
[target.'cfg(unix)'.dependencies]
libc = { workspace = true }
Loading
Loading