Standardize shell execution on unified exec (#39757)

## What changed

- Remove the legacy `shell_command` handler and runtime, leaving `exec_command`
  and `write_stdin` as the shell execution tools.
- Treat legacy `shell_command` model metadata as `unified_exec`, and normalize
  legacy user opt-outs so they do not disable command execution. Managed feature
  requirements and `shell_tool` can still disable it.
- Preserve shell approvals, sandboxing, zsh-fork support, and output truncation
  through the unified execution path.

## Testing

- Cover legacy configuration and model-metadata compatibility.
- Exercise unified shell execution, approvals, truncation, and `apply_patch`
  serialization across the app-server and core test suites.

GitOrigin-RevId: 5c2fd6164fc3519cdae4944cb9db276b8467311c
This commit is contained in:
jif authored and copyberry committed 2026-08-20 17:46:05 +00:00
1 parent d0cc662b8c
commit 8a40095ea3
116 files changed
+1338 -4711

No files matched your search

+15 -56
View File
@@ -4,7 +4,6 @@ use app_test_support::DEFAULT_CLIENT_NAME;
use app_test_support::TestAppServer;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_sequence;
use app_test_support::create_shell_command_sse_response;
use app_test_support::to_response;
use app_test_support::write_chatgpt_auth;
use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url;
@@ -256,12 +255,6 @@ pub(crate) fn assert_basic_thread_initialized_event(
const METRICS_PLUGIN_ID: &str = "sample@openai-curated";
const TEST_CURATED_PLUGIN_SHA: &str = "0123456789abcdef0123456789abcdef01234567";
#[derive(Clone, Copy)]
enum PluginMetricsRuntime {
Classic,
Unified { remote: bool, background: bool },
}
fn write_curated_metrics_plugin(codex_home: &Path) -> Result<PathBuf> {
let plugin_id = PluginId::parse(METRICS_PLUGIN_ID)?;
let plugin_root = PluginStore::new(codex_home.to_path_buf()).plugin_root(
@@ -314,7 +307,7 @@ printf '%s' '{"version":1,"measurements":[{"name":"findings","value":3,"dimensio
Ok(script_path.into_path_buf())
}
async fn assert_plugin_measurement_analytics(runtime: PluginMetricsRuntime) -> Result<()> {
async fn assert_plugin_measurement_analytics(remote: bool, background: bool) -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_remote!(
Ok(()),
@@ -324,11 +317,6 @@ async fn assert_plugin_measurement_analytics(runtime: PluginMetricsRuntime) -> R
let codex_home = TempDir::new()?;
let script_path = write_curated_metrics_plugin(codex_home.path())?.canonicalize()?;
let (remote, background) = match runtime {
PluginMetricsRuntime::Classic => (false, false),
PluginMetricsRuntime::Unified { remote, background } => (remote, background),
};
let unified_exec = matches!(runtime, PluginMetricsRuntime::Unified { .. });
let mut command = vec![
"/bin/sh".to_string(),
script_path.to_string_lossy().into_owned(),
@@ -337,22 +325,15 @@ async fn assert_plugin_measurement_analytics(runtime: PluginMetricsRuntime) -> R
command.push("1.0".to_string());
}
let call_id = "curated-plugin-metrics";
let command_response = match runtime {
PluginMetricsRuntime::Classic => {
create_shell_command_sse_response(command, /*workdir*/ None, Some(5_000), call_id)?
}
PluginMetricsRuntime::Unified { .. } => {
let arguments = serde_json::to_string(&json!({
"cmd": shlex::try_join(command.iter().map(String::as_str))?,
"yield_time_ms": if background { 10 } else { 1_000 },
}))?;
responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call(call_id, "exec_command", &arguments),
responses::ev_completed("resp-1"),
])
}
};
let arguments = serde_json::to_string(&json!({
"cmd": shlex::try_join(command.iter().map(String::as_str))?,
"yield_time_ms": if background { 10 } else { 1_000 },
}))?;
let command_response = responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call(call_id, "exec_command", &arguments),
responses::ev_completed("resp-1"),
]);
let final_response = create_final_assistant_message_sse_response("done")?;
let server =
create_mock_responses_server_sequence(vec![command_response, final_response]).await;
@@ -372,7 +353,7 @@ async fn assert_plugin_measurement_analytics(runtime: PluginMetricsRuntime) -> R
[features]
plugins = true
remote_plugin = false
unified_exec = {unified_exec}
unified_exec = true
shell_zsh_fork = false
unified_exec_zsh_fork = false
@@ -577,49 +558,27 @@ enabled = true
Ok(())
}
#[cfg_attr(windows, ignore = "plugin metrics fixture is Unix-only")]
#[tokio::test]
async fn classic_plugin_script_emits_measurement_analytics() -> Result<()> {
assert_plugin_measurement_analytics(PluginMetricsRuntime::Classic).await
}
#[cfg_attr(windows, ignore = "plugin metrics fixture is Unix-only")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn unified_plugin_script_emits_measurement_analytics() -> Result<()> {
assert_plugin_measurement_analytics(PluginMetricsRuntime::Unified {
remote: false,
background: false,
})
.await
assert_plugin_measurement_analytics(/*remote*/ false, /*background*/ false).await
}
#[cfg_attr(windows, ignore = "plugin metrics fixture is Unix-only")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_unified_plugin_script_emits_measurement_analytics() -> Result<()> {
assert_plugin_measurement_analytics(PluginMetricsRuntime::Unified {
remote: true,
background: false,
})
.await
assert_plugin_measurement_analytics(/*remote*/ true, /*background*/ false).await
}
#[cfg_attr(windows, ignore = "plugin metrics fixture is Unix-only")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn unified_background_plugin_script_emits_measurements_after_turn_completion() -> Result<()> {
assert_plugin_measurement_analytics(PluginMetricsRuntime::Unified {
remote: false,
background: true,
})
.await
assert_plugin_measurement_analytics(/*remote*/ false, /*background*/ true).await
}
#[cfg_attr(windows, ignore = "plugin metrics fixture is Unix-only")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_unified_background_plugin_script_emits_measurements_after_turn_completion()
-> Result<()> {
assert_plugin_measurement_analytics(PluginMetricsRuntime::Unified {
remote: true,
background: true,
})
.await
assert_plugin_measurement_analytics(/*remote*/ true, /*background*/ true).await
}
@@ -2,9 +2,9 @@ use anyhow::Context;
use anyhow::Result;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_command_execution_sse_response;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use app_test_support::create_shell_command_sse_response;
use codex_app_server_protocol::CommandExecutionStatus;
use codex_app_server_protocol::ItemCompletedNotification;
use codex_app_server_protocol::ItemStartedNotification;
@@ -2942,7 +2942,7 @@ async fn websocket_v2_tool_call_delegated_turn_can_execute_shell_tool() -> Resul
// calls the `background_agent` function; the shell command is requested by the delegated
// background agent Responses turn that app-server starts after receiving that function call.
let main_loop = main_loop_responses(vec![
create_shell_command_sse_response(
create_command_execution_sse_response(
realtime_tool_ok_command(),
/*workdir*/ None,
// Windows CI can spend several seconds starting the nested PowerShell command. This
+2 -2
View File
@@ -1,10 +1,10 @@
use anyhow::Result;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_command_execution_sse_response;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_repeating_assistant;
use app_test_support::create_mock_responses_server_sequence;
use app_test_support::create_shell_command_sse_response;
use codex_app_server_protocol::ClientRequest;
use codex_app_server_protocol::ItemCompletedNotification;
use codex_app_server_protocol::ItemStartedNotification;
@@ -195,7 +195,7 @@ async fn review_start_runs_review_turn_and_emits_code_review_item() -> Result<()
#[ignore = "TODO(owenlin0): flaky"]
async fn review_start_exec_approval_item_id_matches_command_execution_item() -> Result<()> {
let responses = vec![
create_shell_command_sse_response(
create_command_execution_sse_response(
vec![
"git".to_string(),
"rev-parse".to_string(),
@@ -4,7 +4,7 @@ use anyhow::Context;
use anyhow::Result;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_escalated_shell_command_sse_response;
use app_test_support::create_escalated_command_execution_sse_response;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_sequence;
use app_test_support::create_mock_responses_server_sequence_unchecked;
@@ -888,7 +888,7 @@ fn blocked_turn_response() -> Result<String> {
"import time; time.sleep(10)".to_string(),
];
create_escalated_shell_command_sse_response(
create_escalated_command_execution_sse_response(
shell_command,
/*workdir*/ None,
/*timeout_ms*/ Some(10_000),
@@ -3,6 +3,7 @@ use app_test_support::ChatGptAuthFixture;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_apply_patch_sse_response;
use app_test_support::create_command_execution_sse_response;
use app_test_support::create_fake_paginated_rollout;
use app_test_support::create_fake_rollout;
use app_test_support::create_fake_rollout_with_text_elements;
@@ -10,7 +11,6 @@ use app_test_support::create_fake_rollout_with_token_usage;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_repeating_assistant;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use app_test_support::create_shell_command_sse_response;
use app_test_support::rollout_path;
use app_test_support::test_absolute_path;
use app_test_support::to_response;
@@ -4507,7 +4507,7 @@ async fn thread_resume_replays_pending_command_execution_request_approval() -> R
let responses = vec![
create_final_assistant_message_sse_response("seeded")?,
create_shell_command_sse_response(
create_command_execution_sse_response(
vec![
"python3".to_string(),
"-c".to_string(),
@@ -1,7 +1,7 @@
use anyhow::Result;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_escalated_shell_command_sse_response;
use app_test_support::create_escalated_command_execution_sse_response;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_sequence;
use app_test_support::format_with_current_shell_display;
@@ -202,7 +202,7 @@ async fn thread_shell_command_uses_existing_active_turn() -> Result<()> {
std::fs::create_dir(&workspace)?;
let responses = vec![
create_escalated_shell_command_sse_response(
create_escalated_command_execution_sse_response(
vec![
"python3".to_string(),
"-c".to_string(),
@@ -3,10 +3,10 @@
use anyhow::Result;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_command_execution_sse_response;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_sequence;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use app_test_support::create_shell_command_sse_response;
use codex_app_server_protocol::ClientRequest;
use codex_app_server_protocol::JSONRPCError;
use codex_app_server_protocol::RequestId;
@@ -53,14 +53,15 @@ async fn turn_interrupt_aborts_running_turn() -> Result<()> {
std::fs::create_dir(&working_directory)?;
// Mock server: long-running shell command then (after abort) nothing else needed.
let server =
create_mock_responses_server_sequence_unchecked(vec![create_shell_command_sse_response(
let server = create_mock_responses_server_sequence_unchecked(vec![
create_command_execution_sse_response(
shell_command.clone(),
Some(&working_directory),
Some(10_000),
"call_sleep",
)?])
.await;
)?,
])
.await;
MockResponsesConfig::new(&server.uri())
.with_sandbox_mode("workspace-write")
.with_root_config(r#"approvals_reviewer = "user""#)
@@ -218,13 +219,14 @@ async fn turn_interrupt_resolves_pending_command_approval_request() -> Result<()
let working_directory = tmp.path().join("workdir");
std::fs::create_dir(&working_directory)?;
let server = create_mock_responses_server_sequence(vec![create_shell_command_sse_response(
shell_command.clone(),
Some(&working_directory),
Some(10_000),
"call_sleep_approval",
)?])
.await;
let server =
create_mock_responses_server_sequence(vec![create_command_execution_sse_response(
shell_command.clone(),
Some(&working_directory),
Some(10_000),
"call_sleep_approval",
)?])
.await;
MockResponsesConfig::new(&server.uri())
.with_approval_policy("on-request")
.with_root_config(r#"approvals_reviewer = "user""#)
@@ -3,14 +3,14 @@ use anyhow::Result;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_apply_patch_sse_response;
use app_test_support::create_escalated_shell_command_sse_response;
use app_test_support::create_command_execution_sse_response;
use app_test_support::create_escalated_command_execution_sse_response;
use app_test_support::create_exec_command_sse_response;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_repeating_assistant;
use app_test_support::create_mock_responses_server_sequence;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use app_test_support::create_request_user_input_sse_response;
use app_test_support::create_shell_command_sse_response;
use app_test_support::format_with_current_shell_display;
use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url;
use app_test_support::write_models_cache;
@@ -2242,7 +2242,7 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> {
let first_shell_command = vec![
"python3".to_string(),
"-c".to_string(),
"import sys; print(sys.argv[1].endswith('7890'))".to_string(),
"import sys, time; time.sleep(0.5); print(sys.argv[1].endswith('7890'))".to_string(),
format!("Authorization: Bearer {bearer_token}"),
];
let expected_approval_command = format_with_current_shell_display(&shlex::try_join(
@@ -2254,14 +2254,14 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> {
// Mock server: first turn requests a shell call (elicitation), then completes.
// Second turn same, but we'll set approval_policy=never to avoid elicitation.
let responses = vec![
create_escalated_shell_command_sse_response(
create_escalated_command_execution_sse_response(
first_shell_command,
/*workdir*/ None,
Some(5000),
"call1",
)?,
create_final_assistant_message_sse_response("done 1")?,
create_shell_command_sse_response(
create_command_execution_sse_response(
vec![
"python3".to_string(),
"-c".to_string(),
@@ -2387,6 +2387,31 @@ async fn turn_start_exec_approval_toggle_v2() -> Result<()> {
}
}
let requests = server
.received_requests()
.await
.context("failed to fetch received requests")?;
assert!(
requests.iter().any(|request| {
request.url.path().ends_with("/responses")
&& serde_json::from_slice::<Value>(&request.body)
.ok()
.and_then(|body| {
body["input"].as_array().map(|items| {
items.iter().any(|item| {
item["type"] == "function_call_output"
&& item["call_id"] == "call1"
&& item["output"]
.as_str()
.is_some_and(|output| output.contains("True"))
})
})
})
.unwrap_or(false)
}),
"model request should include the command output confirming the original bearer token"
);
// Second turn with approval_policy=never should not elicit approval
let _: TurnStartResponse = mcp
.request(|request_id| ClientRequest::TurnStart {
@@ -2468,7 +2493,7 @@ async fn run_turn_start_exec_approval_rejection_v2(
expected_approval_command.replace(bearer_token, "[REDACTED_SECRET]");
let responses = vec![
create_escalated_shell_command_sse_response(
create_escalated_command_execution_sse_response(
shell_command,
/*workdir*/ None,
Some(5000),
@@ -2614,6 +2639,12 @@ async fn turn_start_explicit_local_environment_updates_legacy_cwd_between_turns(
let tmp = TempDir::new()?;
let codex_home = tmp.path().join("codex_home");
std::fs::create_dir(&codex_home)?;
let rules_dir = codex_home.join("rules");
std::fs::create_dir(&rules_dir)?;
std::fs::write(
rules_dir.join("default.rules"),
r#"prefix_rule(pattern=["echo"], decision="allow")"#,
)?;
let workspace_root = tmp.path().join("workspace");
std::fs::create_dir(&workspace_root)?;
let first_cwd = workspace_root.join("turn1");
@@ -2622,14 +2653,14 @@ async fn turn_start_explicit_local_environment_updates_legacy_cwd_between_turns(
std::fs::create_dir(&second_cwd)?;
let responses = vec![
create_shell_command_sse_response(
create_command_execution_sse_response(
vec!["echo".to_string(), "first".to_string(), "turn".to_string()],
/*workdir*/ None,
Some(5000),
"call-first",
)?,
create_final_assistant_message_sse_response("done first")?,
create_shell_command_sse_response(
create_command_execution_sse_response(
vec!["echo".to_string(), "second".to_string(), "turn".to_string()],
/*workdir*/ None,
Some(5000),
@@ -4639,7 +4670,7 @@ async fn command_execution_notifications_include_trusted_plugin_id() -> Result<(
}"#,
)?;
let responses = vec![
create_shell_command_sse_response(
create_command_execution_sse_response(
vec![
"/bin/sh".to_string(),
script_path.to_string_lossy().into_owned(),
@@ -9,11 +9,11 @@
use anyhow::Result;
use app_test_support::MockResponsesConfig;
use app_test_support::TestAppServer;
use app_test_support::create_escalated_shell_command_sse_response;
use app_test_support::create_command_execution_sse_response;
use app_test_support::create_escalated_command_execution_sse_response;
use app_test_support::create_final_assistant_message_sse_response;
use app_test_support::create_mock_responses_server_sequence;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use app_test_support::create_shell_command_sse_response;
use codex_app_server_protocol::CommandAction;
use codex_app_server_protocol::CommandExecutionApprovalDecision;
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
@@ -74,7 +74,7 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> {
let release_marker_escaped = release_marker.to_string_lossy().replace('\'', r#"'\''"#);
let wait_for_interrupt =
format!("while [ ! -f '{release_marker_escaped}' ]; do sleep 0.01; done");
let response = create_shell_command_sse_response(
let response = create_command_execution_sse_response(
vec!["/bin/sh".to_string(), "-c".to_string(), wait_for_interrupt],
/*workdir*/ None,
Some(5000),
@@ -96,7 +96,8 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> {
"never",
&BTreeMap::from([
(Feature::ShellZshFork, true),
(Feature::UnifiedExec, false),
(Feature::UnifiedExec, true),
(Feature::UnifiedExecZshFork, true),
(Feature::ShellSnapshot, false),
]),
)?;
@@ -192,7 +193,7 @@ async fn turn_start_shell_zsh_fork_exec_approval_decline_v2() -> Result<()> {
eprintln!("using zsh path for zsh-fork test: {}", zsh_path.display());
let responses = vec![
create_escalated_shell_command_sse_response(
create_escalated_command_execution_sse_response(
vec![
"python3".to_string(),
"-c".to_string(),
@@ -211,7 +212,8 @@ async fn turn_start_shell_zsh_fork_exec_approval_decline_v2() -> Result<()> {
"on-request",
&BTreeMap::from([
(Feature::ShellZshFork, true),
(Feature::UnifiedExec, false),
(Feature::UnifiedExec, true),
(Feature::UnifiedExecZshFork, true),
(Feature::ShellSnapshot, false),
]),
)?;
@@ -323,7 +325,7 @@ async fn turn_start_shell_zsh_fork_exec_approval_cancel_v2() -> Result<()> {
};
eprintln!("using zsh path for zsh-fork test: {}", zsh_path.display());
let responses = vec![create_escalated_shell_command_sse_response(
let responses = vec![create_escalated_command_execution_sse_response(
vec![
"python3".to_string(),
"-c".to_string(),
@@ -340,7 +342,8 @@ async fn turn_start_shell_zsh_fork_exec_approval_cancel_v2() -> Result<()> {
"on-request",
&BTreeMap::from([
(Feature::ShellZshFork, true),
(Feature::UnifiedExec, false),
(Feature::UnifiedExec, true),
(Feature::UnifiedExecZshFork, true),
(Feature::ShellSnapshot, false),
]),
)?;
@@ -466,15 +469,15 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2()
second_file.display()
);
let tool_call_arguments = serde_json::to_string(&serde_json::json!({
"command": shell_command,
"cmd": shell_command,
"workdir": serde_json::Value::Null,
"timeout_ms": 20000
"yield_time_ms": 20000
}))?;
let response = responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call(
"call-zsh-fork-subcommand-decline",
"shell_command",
"exec_command",
&tool_call_arguments,
),
responses::ev_completed("resp-1"),
@@ -495,7 +498,8 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2()
"on-request",
&BTreeMap::from([
(Feature::ShellZshFork, true),
(Feature::UnifiedExec, false),
(Feature::UnifiedExec, true),
(Feature::UnifiedExecZshFork, true),
(Feature::ShellSnapshot, false),
]),
)?;
@@ -3,9 +3,9 @@
use anyhow::Context;
use anyhow::Result;
use app_test_support::TestAppServer;
use app_test_support::create_command_execution_sse_response;
use app_test_support::create_mock_responses_server_sequence;
use app_test_support::create_mock_responses_server_sequence_unchecked;
use app_test_support::create_shell_command_sse_response;
use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url;
use codex_app_server::INPUT_TOO_LARGE_ERROR_CODE;
use codex_app_server::INVALID_PARAMS_ERROR_CODE;
@@ -124,14 +124,15 @@ async fn turn_steer_rejects_oversized_text_input() -> Result<()> {
let working_directory = tmp.path().join("workdir");
std::fs::create_dir(&working_directory)?;
let server =
create_mock_responses_server_sequence_unchecked(vec![create_shell_command_sse_response(
let server = create_mock_responses_server_sequence_unchecked(vec![
create_command_execution_sse_response(
shell_command.clone(),
Some(&working_directory),
Some(10_000),
"call_sleep",
)?])
.await;
)?,
])
.await;
write_mock_responses_config_toml_with_chatgpt_base_url(
&codex_home,
&server.uri(),
@@ -237,7 +238,7 @@ async fn turn_steer_returns_active_turn_id() -> Result<()> {
std::fs::create_dir(&working_directory)?;
let server = create_mock_responses_server_sequence_unchecked(vec![
create_shell_command_sse_response(
create_command_execution_sse_response(
shell_command.clone(),
Some(&working_directory),
Some(10_000),
@@ -371,7 +372,7 @@ async fn turn_steer_rejects_context_only_input_without_merging_context() -> Resu
std::fs::create_dir(&working_directory)?;
let server = create_mock_responses_server_sequence_unchecked(vec![
create_shell_command_sse_response(
create_command_execution_sse_response(
vec!["sleep".to_string(), "1".to_string()],
Some(&working_directory),
Some(10_000),