From b3a8571219d9ed35c95bd5ff2eb5b42baf40a131 Mon Sep 17 00:00:00 2001 From: Shijie Rao Date: Tue, 17 Feb 2026 17:50:06 -0800 Subject: [PATCH 1/5] Chore: remove response model check and rely on header model for downgrade (#12061) ### Summary Ensure that we use the model value from the response header only so that we are guaranteed with the correct slug name. We are no longer checking against the model value from response so that we are less likely to have false positive. There are two different treatments - for SSE we use the header from the response and for websocket we check top-level events. --- MODULE.bazel.lock | 10 +- .../tests/suite/v2/safety_check_downgrade.rs | 4 +- codex-rs/codex-api/src/sse/responses.rs | 112 +++++++++--------- codex-rs/core/src/codex.rs | 4 +- .../tests/suite/safety_check_downgrade.rs | 59 ++++++++- 5 files changed, 126 insertions(+), 63 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 80e889a031..c1ab56b653 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -686,11 +686,11 @@ "chrono_0.4.43": "{\"dependencies\":[{\"features\":[\"derive\"],\"name\":\"arbitrary\",\"optional\":true,\"req\":\"^1.0.0\"},{\"kind\":\"dev\",\"name\":\"bincode\",\"req\":\"^1.3.0\"},{\"name\":\"defmt\",\"optional\":true,\"req\":\"^1.0.1\"},{\"features\":[\"fallback\"],\"name\":\"iana-time-zone\",\"optional\":true,\"req\":\"^0.1.45\",\"target\":\"cfg(unix)\"},{\"name\":\"js-sys\",\"optional\":true,\"req\":\"^0.3\",\"target\":\"cfg(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\"))))\"},{\"default_features\":false,\"name\":\"num-traits\",\"req\":\"^0.2\"},{\"name\":\"pure-rust-locales\",\"optional\":true,\"req\":\"^0.8.2\"},{\"default_features\":false,\"name\":\"rkyv\",\"optional\":true,\"req\":\"^0.7.43\"},{\"default_features\":false,\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0.99\"},{\"default_features\":false,\"kind\":\"dev\",\"name\":\"serde_derive\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1\"},{\"kind\":\"dev\",\"name\":\"similar-asserts\",\"req\":\"^1.6.1\"},{\"name\":\"wasm-bindgen\",\"optional\":true,\"req\":\"^0.2\",\"target\":\"cfg(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\"))))\"},{\"kind\":\"dev\",\"name\":\"wasm-bindgen-test\",\"req\":\"^0.3\",\"target\":\"cfg(all(target_arch = \\\"wasm32\\\", not(any(target_os = \\\"emscripten\\\", target_os = \\\"wasi\\\"))))\"},{\"kind\":\"dev\",\"name\":\"windows-bindgen\",\"req\":\"^0.66\"},{\"name\":\"windows-link\",\"optional\":true,\"req\":\"^0.2\",\"target\":\"cfg(windows)\"}],\"features\":{\"__internal_bench\":[],\"alloc\":[],\"clock\":[\"winapi\",\"iana-time-zone\",\"now\"],\"core-error\":[],\"default\":[\"clock\",\"std\",\"oldtime\",\"wasmbind\"],\"defmt\":[\"dep:defmt\",\"pure-rust-locales?/defmt\"],\"libc\":[],\"now\":[\"std\"],\"oldtime\":[],\"rkyv\":[\"dep:rkyv\",\"rkyv/size_32\"],\"rkyv-16\":[\"dep:rkyv\",\"rkyv?/size_16\"],\"rkyv-32\":[\"dep:rkyv\",\"rkyv?/size_32\"],\"rkyv-64\":[\"dep:rkyv\",\"rkyv?/size_64\"],\"rkyv-validation\":[\"rkyv?/validation\"],\"std\":[\"alloc\"],\"unstable-locales\":[\"pure-rust-locales\"],\"wasmbind\":[\"wasm-bindgen\",\"js-sys\"],\"winapi\":[\"windows-link\"]}}", "chunked_transfer_1.5.0": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.3\"}],\"features\":{}}", "cipher_0.4.4": "{\"dependencies\":[{\"name\":\"blobby\",\"optional\":true,\"req\":\"^0.3\"},{\"name\":\"crypto-common\",\"req\":\"^0.1.6\"},{\"name\":\"inout\",\"req\":\"^0.1\"},{\"default_features\":false,\"name\":\"zeroize\",\"optional\":true,\"req\":\"^1.5\"}],\"features\":{\"alloc\":[],\"block-padding\":[\"inout/block-padding\"],\"dev\":[\"blobby\"],\"rand_core\":[\"crypto-common/rand_core\"],\"std\":[\"alloc\",\"crypto-common/std\",\"inout/std\"]}}", - "clap_4.5.56": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1.0.14\"},{\"default_features\":false,\"kind\":\"dev\",\"name\":\"clap-cargo\",\"req\":\"^0.15.0\"},{\"default_features\":false,\"name\":\"clap_builder\",\"req\":\"=4.5.56\"},{\"name\":\"clap_derive\",\"optional\":true,\"req\":\"=4.5.55\"},{\"kind\":\"dev\",\"name\":\"jiff\",\"req\":\"^0.2.3\"},{\"kind\":\"dev\",\"name\":\"rustversion\",\"req\":\"^1.0.15\"},{\"kind\":\"dev\",\"name\":\"semver\",\"req\":\"^1.0.26\"},{\"kind\":\"dev\",\"name\":\"shlex\",\"req\":\"^1.3.0\"},{\"features\":[\"term-svg\"],\"kind\":\"dev\",\"name\":\"snapbox\",\"req\":\"^0.6.16\"},{\"kind\":\"dev\",\"name\":\"trybuild\",\"req\":\"^1.0.91\"},{\"default_features\":false,\"features\":[\"color-auto\",\"diff\",\"examples\"],\"kind\":\"dev\",\"name\":\"trycmd\",\"req\":\"^0.15.3\"}],\"features\":{\"cargo\":[\"clap_builder/cargo\"],\"color\":[\"clap_builder/color\"],\"debug\":[\"clap_builder/debug\",\"clap_derive?/debug\"],\"default\":[\"std\",\"color\",\"help\",\"usage\",\"error-context\",\"suggestions\"],\"deprecated\":[\"clap_builder/deprecated\",\"clap_derive?/deprecated\"],\"derive\":[\"dep:clap_derive\"],\"env\":[\"clap_builder/env\"],\"error-context\":[\"clap_builder/error-context\"],\"help\":[\"clap_builder/help\"],\"std\":[\"clap_builder/std\"],\"string\":[\"clap_builder/string\"],\"suggestions\":[\"clap_builder/suggestions\"],\"unicode\":[\"clap_builder/unicode\"],\"unstable-derive-ui-tests\":[],\"unstable-doc\":[\"clap_builder/unstable-doc\",\"derive\"],\"unstable-ext\":[\"clap_builder/unstable-ext\"],\"unstable-markdown\":[\"clap_derive/unstable-markdown\"],\"unstable-styles\":[\"clap_builder/unstable-styles\"],\"unstable-v5\":[\"clap_builder/unstable-v5\",\"clap_derive?/unstable-v5\",\"deprecated\"],\"usage\":[\"clap_builder/usage\"],\"wrap_help\":[\"clap_builder/wrap_help\"]}}", - "clap_builder_4.5.56": "{\"dependencies\":[{\"name\":\"anstream\",\"optional\":true,\"req\":\"^0.6.7\"},{\"name\":\"anstyle\",\"req\":\"^1.0.8\"},{\"name\":\"backtrace\",\"optional\":true,\"req\":\"^0.3.73\"},{\"name\":\"clap_lex\",\"req\":\"^0.7.4\"},{\"kind\":\"dev\",\"name\":\"color-print\",\"req\":\"^0.3.6\"},{\"kind\":\"dev\",\"name\":\"snapbox\",\"req\":\"^0.6.16\"},{\"kind\":\"dev\",\"name\":\"static_assertions\",\"req\":\"^1.1.0\"},{\"name\":\"strsim\",\"optional\":true,\"req\":\"^0.11.0\"},{\"name\":\"terminal_size\",\"optional\":true,\"req\":\"^0.4.0\"},{\"kind\":\"dev\",\"name\":\"unic-emoji-char\",\"req\":\"^0.9.0\"},{\"name\":\"unicase\",\"optional\":true,\"req\":\"^2.6.0\"},{\"name\":\"unicode-width\",\"optional\":true,\"req\":\"^0.2.0\"}],\"features\":{\"cargo\":[],\"color\":[\"dep:anstream\"],\"debug\":[\"dep:backtrace\"],\"default\":[\"std\",\"color\",\"help\",\"usage\",\"error-context\",\"suggestions\"],\"deprecated\":[],\"env\":[],\"error-context\":[],\"help\":[],\"std\":[\"anstyle/std\"],\"string\":[],\"suggestions\":[\"dep:strsim\",\"error-context\"],\"unicode\":[\"dep:unicode-width\",\"dep:unicase\"],\"unstable-doc\":[\"cargo\",\"wrap_help\",\"env\",\"unicode\",\"string\",\"unstable-ext\"],\"unstable-ext\":[],\"unstable-styles\":[\"color\"],\"unstable-v5\":[\"deprecated\"],\"usage\":[],\"wrap_help\":[\"help\",\"dep:terminal_size\"]}}", + "clap_4.5.58": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1.0.14\"},{\"default_features\":false,\"kind\":\"dev\",\"name\":\"clap-cargo\",\"req\":\"^0.15.0\"},{\"default_features\":false,\"name\":\"clap_builder\",\"req\":\"=4.5.58\"},{\"name\":\"clap_derive\",\"optional\":true,\"req\":\"=4.5.55\"},{\"kind\":\"dev\",\"name\":\"jiff\",\"req\":\"^0.2.3\"},{\"kind\":\"dev\",\"name\":\"rustversion\",\"req\":\"^1.0.15\"},{\"kind\":\"dev\",\"name\":\"semver\",\"req\":\"^1.0.26\"},{\"kind\":\"dev\",\"name\":\"shlex\",\"req\":\"^1.3.0\"},{\"features\":[\"term-svg\"],\"kind\":\"dev\",\"name\":\"snapbox\",\"req\":\"^0.6.16\"},{\"kind\":\"dev\",\"name\":\"trybuild\",\"req\":\"^1.0.91\"},{\"default_features\":false,\"features\":[\"color-auto\",\"diff\",\"examples\"],\"kind\":\"dev\",\"name\":\"trycmd\",\"req\":\"^0.15.3\"}],\"features\":{\"cargo\":[\"clap_builder/cargo\"],\"color\":[\"clap_builder/color\"],\"debug\":[\"clap_builder/debug\",\"clap_derive?/debug\"],\"default\":[\"std\",\"color\",\"help\",\"usage\",\"error-context\",\"suggestions\"],\"deprecated\":[\"clap_builder/deprecated\",\"clap_derive?/deprecated\"],\"derive\":[\"dep:clap_derive\"],\"env\":[\"clap_builder/env\"],\"error-context\":[\"clap_builder/error-context\"],\"help\":[\"clap_builder/help\"],\"std\":[\"clap_builder/std\"],\"string\":[\"clap_builder/string\"],\"suggestions\":[\"clap_builder/suggestions\"],\"unicode\":[\"clap_builder/unicode\"],\"unstable-derive-ui-tests\":[],\"unstable-doc\":[\"clap_builder/unstable-doc\",\"derive\"],\"unstable-ext\":[\"clap_builder/unstable-ext\"],\"unstable-markdown\":[\"clap_derive/unstable-markdown\"],\"unstable-styles\":[\"clap_builder/unstable-styles\"],\"unstable-v5\":[\"clap_builder/unstable-v5\",\"clap_derive?/unstable-v5\",\"deprecated\"],\"usage\":[\"clap_builder/usage\"],\"wrap_help\":[\"clap_builder/wrap_help\"]}}", + "clap_builder_4.5.58": "{\"dependencies\":[{\"name\":\"anstream\",\"optional\":true,\"req\":\"^0.6.7\"},{\"name\":\"anstyle\",\"req\":\"^1.0.8\"},{\"name\":\"backtrace\",\"optional\":true,\"req\":\"^0.3.73\"},{\"name\":\"clap_lex\",\"req\":\"^1.0.0\"},{\"kind\":\"dev\",\"name\":\"color-print\",\"req\":\"^0.3.6\"},{\"kind\":\"dev\",\"name\":\"snapbox\",\"req\":\"^0.6.16\"},{\"kind\":\"dev\",\"name\":\"static_assertions\",\"req\":\"^1.1.0\"},{\"name\":\"strsim\",\"optional\":true,\"req\":\"^0.11.0\"},{\"name\":\"terminal_size\",\"optional\":true,\"req\":\"^0.4.0\"},{\"kind\":\"dev\",\"name\":\"unic-emoji-char\",\"req\":\"^0.9.0\"},{\"name\":\"unicase\",\"optional\":true,\"req\":\"^2.6.0\"},{\"name\":\"unicode-width\",\"optional\":true,\"req\":\"^0.2.0\"}],\"features\":{\"cargo\":[],\"color\":[\"dep:anstream\"],\"debug\":[\"dep:backtrace\"],\"default\":[\"std\",\"color\",\"help\",\"usage\",\"error-context\",\"suggestions\"],\"deprecated\":[],\"env\":[],\"error-context\":[],\"help\":[],\"std\":[\"anstyle/std\"],\"string\":[],\"suggestions\":[\"dep:strsim\",\"error-context\"],\"unicode\":[\"dep:unicode-width\",\"dep:unicase\"],\"unstable-doc\":[\"cargo\",\"wrap_help\",\"env\",\"unicode\",\"string\",\"unstable-ext\"],\"unstable-ext\":[],\"unstable-styles\":[\"color\"],\"unstable-v5\":[\"deprecated\"],\"usage\":[],\"wrap_help\":[\"help\",\"dep:terminal_size\"]}}", "clap_complete_4.5.65": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1.0.14\"},{\"default_features\":false,\"features\":[\"std\"],\"name\":\"clap\",\"req\":\"^4.5.20\"},{\"default_features\":false,\"features\":[\"std\",\"derive\",\"help\"],\"kind\":\"dev\",\"name\":\"clap\",\"req\":\"^4.5.20\"},{\"name\":\"clap_lex\",\"optional\":true,\"req\":\"^0.7.0\"},{\"name\":\"completest\",\"optional\":true,\"req\":\"^0.4.2\"},{\"name\":\"completest-pty\",\"optional\":true,\"req\":\"^0.5.5\"},{\"name\":\"is_executable\",\"optional\":true,\"req\":\"^1.0.1\"},{\"name\":\"shlex\",\"optional\":true,\"req\":\"^1.3.0\"},{\"features\":[\"diff\",\"dir\",\"examples\"],\"kind\":\"dev\",\"name\":\"snapbox\",\"req\":\"^0.6.0\"},{\"default_features\":false,\"features\":[\"color-auto\",\"diff\",\"examples\"],\"kind\":\"dev\",\"name\":\"trycmd\",\"req\":\"^0.15.1\"}],\"features\":{\"debug\":[\"clap/debug\"],\"default\":[],\"unstable-doc\":[\"unstable-dynamic\"],\"unstable-dynamic\":[\"dep:clap_lex\",\"dep:shlex\",\"dep:is_executable\",\"clap/unstable-ext\"],\"unstable-shell-tests\":[\"dep:completest\",\"dep:completest-pty\"]}}", "clap_derive_4.5.55": "{\"dependencies\":[{\"name\":\"anstyle\",\"optional\":true,\"req\":\"^1.0.10\"},{\"name\":\"heck\",\"req\":\"^0.5.0\"},{\"name\":\"proc-macro2\",\"req\":\"^1.0.69\"},{\"default_features\":false,\"name\":\"pulldown-cmark\",\"optional\":true,\"req\":\"^0.13.0\"},{\"name\":\"quote\",\"req\":\"^1.0.9\"},{\"features\":[\"full\"],\"name\":\"syn\",\"req\":\"^2.0.8\"}],\"features\":{\"debug\":[],\"default\":[],\"deprecated\":[],\"raw-deprecated\":[\"deprecated\"],\"unstable-markdown\":[\"dep:pulldown-cmark\",\"dep:anstyle\"],\"unstable-v5\":[\"deprecated\"]}}", - "clap_lex_0.7.7": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1.0.14\"}],\"features\":{}}", + "clap_lex_1.0.0": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"automod\",\"req\":\"^1.0.14\"}],\"features\":{}}", "clipboard-win_5.4.1": "{\"dependencies\":[{\"name\":\"error-code\",\"req\":\"^3\",\"target\":\"cfg(windows)\"},{\"name\":\"windows-win\",\"optional\":true,\"req\":\"^3\",\"target\":\"cfg(windows)\"}],\"features\":{\"monitor\":[\"windows-win\"],\"std\":[\"error-code/std\"]}}", "cmake_0.1.57": "{\"dependencies\":[{\"name\":\"cc\",\"req\":\"^1.2.46\"}],\"features\":{}}", "cmp_any_0.8.1": "{\"dependencies\":[],\"features\":{}}", @@ -790,9 +790,9 @@ "enumflags2_0.7.12": "{\"dependencies\":[{\"name\":\"enumflags2_derive\",\"req\":\"=0.7.12\"},{\"default_features\":false,\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0.0\"}],\"features\":{\"std\":[]}}", "enumflags2_derive_0.7.12": "{\"dependencies\":[{\"name\":\"proc-macro2\",\"req\":\"^1.0\"},{\"name\":\"quote\",\"req\":\"^1.0\"},{\"default_features\":false,\"features\":[\"parsing\",\"printing\",\"derive\",\"proc-macro\"],\"name\":\"syn\",\"req\":\"^2.0\"}],\"features\":{}}", "env-flags_0.1.1": "{\"dependencies\":[],\"features\":{}}", - "env_filter_0.1.4": "{\"dependencies\":[{\"features\":[\"std\"],\"name\":\"log\",\"req\":\"^0.4.8\"},{\"default_features\":false,\"features\":[\"std\",\"perf\"],\"name\":\"regex\",\"optional\":true,\"req\":\"^1.0.3\"},{\"kind\":\"dev\",\"name\":\"snapbox\",\"req\":\"^0.6\"}],\"features\":{\"default\":[\"regex\"],\"regex\":[\"dep:regex\"]}}", + "env_filter_1.0.0": "{\"dependencies\":[{\"features\":[\"std\"],\"name\":\"log\",\"req\":\"^0.4.8\"},{\"default_features\":false,\"features\":[\"std\",\"perf\"],\"name\":\"regex\",\"optional\":true,\"req\":\"^1.0.3\"},{\"kind\":\"dev\",\"name\":\"snapbox\",\"req\":\"^0.6\"}],\"features\":{\"default\":[\"regex\"],\"regex\":[\"dep:regex\"]}}", "env_home_0.1.0": "{\"dependencies\":[],\"features\":{}}", - "env_logger_0.11.8": "{\"dependencies\":[{\"default_features\":false,\"features\":[\"wincon\"],\"name\":\"anstream\",\"optional\":true,\"req\":\"^0.6.11\"},{\"name\":\"anstyle\",\"optional\":true,\"req\":\"^1.0.6\"},{\"default_features\":false,\"name\":\"env_filter\",\"req\":\"^0.1.0\"},{\"default_features\":false,\"features\":[\"std\"],\"name\":\"jiff\",\"optional\":true,\"req\":\"^0.2.3\"},{\"features\":[\"std\"],\"name\":\"log\",\"req\":\"^0.4.21\"}],\"features\":{\"auto-color\":[\"color\",\"anstream/auto\"],\"color\":[\"dep:anstream\",\"dep:anstyle\"],\"default\":[\"auto-color\",\"humantime\",\"regex\"],\"humantime\":[\"dep:jiff\"],\"kv\":[\"log/kv\"],\"regex\":[\"env_filter/regex\"],\"unstable-kv\":[\"kv\"]}}", + "env_logger_0.11.9": "{\"dependencies\":[{\"default_features\":false,\"features\":[\"wincon\"],\"name\":\"anstream\",\"optional\":true,\"req\":\"^0.6.11\"},{\"name\":\"anstyle\",\"optional\":true,\"req\":\"^1.0.6\"},{\"default_features\":false,\"name\":\"env_filter\",\"req\":\"^1.0.0\"},{\"default_features\":false,\"features\":[\"std\"],\"name\":\"jiff\",\"optional\":true,\"req\":\"^0.2.3\"},{\"features\":[\"std\"],\"name\":\"log\",\"req\":\"^0.4.21\"}],\"features\":{\"auto-color\":[\"color\",\"anstream/auto\"],\"color\":[\"dep:anstream\",\"dep:anstyle\"],\"default\":[\"auto-color\",\"humantime\",\"regex\"],\"humantime\":[\"dep:jiff\"],\"kv\":[\"log/kv\"],\"regex\":[\"env_filter/regex\"],\"unstable-kv\":[\"kv\"]}}", "equivalent_1.0.2": "{\"dependencies\":[],\"features\":{}}", "erased-serde_0.3.31": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"rustversion\",\"req\":\"^1.0.13\"},{\"default_features\":false,\"name\":\"serde\",\"req\":\"^1.0.166\"},{\"kind\":\"dev\",\"name\":\"serde_cbor\",\"req\":\"^0.11.2\"},{\"kind\":\"dev\",\"name\":\"serde_derive\",\"req\":\"^1.0.166\"},{\"kind\":\"dev\",\"name\":\"serde_json\",\"req\":\"^1.0.99\"},{\"features\":[\"diff\"],\"kind\":\"dev\",\"name\":\"trybuild\",\"req\":\"^1.0.83\"}],\"features\":{\"alloc\":[\"serde/alloc\"],\"default\":[\"std\"],\"std\":[\"serde/std\"],\"unstable-debug\":[]}}", "errno_0.3.14": "{\"dependencies\":[{\"default_features\":false,\"name\":\"libc\",\"req\":\"^0.2\",\"target\":\"cfg(target_os=\\\"hermit\\\")\"},{\"default_features\":false,\"name\":\"libc\",\"req\":\"^0.2\",\"target\":\"cfg(target_os=\\\"wasi\\\")\"},{\"default_features\":false,\"name\":\"libc\",\"req\":\"^0.2\",\"target\":\"cfg(unix)\"},{\"features\":[\"Win32_Foundation\",\"Win32_System_Diagnostics_Debug\"],\"name\":\"windows-sys\",\"req\":\">=0.52, <0.62\",\"target\":\"cfg(windows)\"}],\"features\":{\"default\":[\"std\"],\"std\":[\"libc/std\"]}}", diff --git a/codex-rs/app-server/tests/suite/v2/safety_check_downgrade.rs b/codex-rs/app-server/tests/suite/v2/safety_check_downgrade.rs index 53acd492b6..8405d11490 100644 --- a/codex-rs/app-server/tests/suite/v2/safety_check_downgrade.rs +++ b/codex-rs/app-server/tests/suite/v2/safety_check_downgrade.rs @@ -99,7 +99,9 @@ async fn response_model_field_mismatch_emits_model_rerouted_notification_v2_when "type": "response.created", "response": { "id": "resp-1", - "model": SERVER_MODEL, + "headers": { + "OpenAI-Model": SERVER_MODEL + } } }), responses::ev_assistant_message("msg-1", "Done"), diff --git a/codex-rs/codex-api/src/sse/responses.rs b/codex-rs/codex-api/src/sse/responses.rs index 75c79ba5c1..3c039380c9 100644 --- a/codex-rs/codex-api/src/sse/responses.rs +++ b/codex-rs/codex-api/src/sse/responses.rs @@ -167,6 +167,7 @@ struct ResponseCompletedOutputTokensDetails { pub struct ResponsesStreamEvent { #[serde(rename = "type")] pub(crate) kind: String, + headers: Option, response: Option, item: Option, delta: Option, @@ -179,20 +180,27 @@ impl ResponsesStreamEvent { &self.kind } + /// Returns the effective model reported by the server, if present. + /// + /// Precedence: + /// 1. `response.headers` for standard Responses stream events. + /// 2. top-level `headers` for websocket metadata events (for example + /// `codex.response.metadata`). pub fn response_model(&self) -> Option { - self.response.as_ref().and_then(extract_server_model) - } -} + let response_headers_model = self + .response + .as_ref() + .and_then(|response| response.get("headers")) + .and_then(header_openai_model_value_from_json); -fn extract_server_model(value: &Value) -> Option { - value - .get("model") - .and_then(json_value_as_string) - .or_else(|| { - value - .get("headers") - .and_then(header_openai_model_value_from_json) - }) + match response_headers_model { + Some(model) => Some(model), + None => self + .headers + .as_ref() + .and_then(header_openai_model_value_from_json), + } + } } fn header_openai_model_value_from_json(value: &Value) -> Option { @@ -963,7 +971,7 @@ mod tests { } #[tokio::test] - async fn process_sse_emits_server_model_from_response_payload() { + async fn process_sse_ignores_response_model_field_in_payload() { let events = run_sse(vec![ json!({ "type": "response.created", @@ -982,14 +990,10 @@ mod tests { ]) .await; - assert_eq!(events.len(), 3); + assert_eq!(events.len(), 2); + assert_matches!(&events[0], ResponseEvent::Created); assert_matches!( - &events[0], - ResponseEvent::ServerModel(model) if model == CYBER_RESTRICTED_MODEL_FOR_TESTS - ); - assert_matches!(&events[1], ResponseEvent::Created); - assert_matches!( - &events[2], + &events[1], ResponseEvent::Completed { response_id, token_usage: None, @@ -1035,43 +1039,41 @@ mod tests { ); } - #[tokio::test] - async fn process_sse_emits_server_model_again_when_response_model_changes() { - let events = run_sse(vec![ - json!({ - "type": "response.created", - "response": { - "id": "resp-1", - "model": "gpt-5.2-codex" - } - }), - json!({ - "type": "response.completed", - "response": { - "id": "resp-1", - "model": "gpt-5.3-codex" - } - }), - ]) - .await; + #[test] + fn responses_stream_event_response_model_reads_top_level_headers() { + let ev: ResponsesStreamEvent = serde_json::from_value(json!({ + "type": "codex.response.metadata", + "headers": { + "openai-model": CYBER_RESTRICTED_MODEL_FOR_TESTS, + } + })) + .expect("expected event to deserialize"); - assert_eq!(events.len(), 4); - assert_matches!( - &events[0], - ResponseEvent::ServerModel(model) if model == "gpt-5.2-codex" + assert_eq!( + ev.response_model().as_deref(), + Some(CYBER_RESTRICTED_MODEL_FOR_TESTS) ); - assert_matches!(&events[1], ResponseEvent::Created); - assert_matches!( - &events[2], - ResponseEvent::ServerModel(model) if model == "gpt-5.3-codex" - ); - assert_matches!( - &events[3], - ResponseEvent::Completed { - response_id, - token_usage: None, - can_append: false - } if response_id == "resp-1" + } + + #[test] + fn responses_stream_event_response_model_prefers_response_headers() { + let ev: ResponsesStreamEvent = serde_json::from_value(json!({ + "type": "response.created", + "headers": { + "openai-model": "top-level-model" + }, + "response": { + "id": "resp-1", + "headers": { + "openai-model": CYBER_RESTRICTED_MODEL_FOR_TESTS + } + } + })) + .expect("expected event to deserialize"); + + assert_eq!( + ev.response_model().as_deref(), + Some(CYBER_RESTRICTED_MODEL_FOR_TESTS) ); } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 6fa5ae3b80..210809a812 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2440,7 +2440,9 @@ impl Session { server_model: String, ) -> bool { let requested_model = turn_context.model_info.slug.clone(); - if server_model == requested_model { + let server_model_normalized = server_model.to_ascii_lowercase(); + let requested_model_normalized = requested_model.to_ascii_lowercase(); + if server_model_normalized == requested_model_normalized { info!("server reported model {server_model} (matches requested model)"); return false; } diff --git a/codex-rs/core/tests/suite/safety_check_downgrade.rs b/codex-rs/core/tests/suite/safety_check_downgrade.rs index 62a46bf1de..6f4fbe553b 100644 --- a/codex-rs/core/tests/suite/safety_check_downgrade.rs +++ b/codex-rs/core/tests/suite/safety_check_downgrade.rs @@ -121,7 +121,9 @@ async fn response_model_field_mismatch_emits_warning_when_header_matches_request "type": "response.created", "response": { "id": "resp-1", - "model": SERVER_MODEL, + "headers": { + "OpenAI-Model": SERVER_MODEL + } } }), core_test_support::responses::ev_completed("resp-1"), @@ -250,3 +252,58 @@ async fn openai_model_header_mismatch_only_emits_one_warning_per_turn() -> Resul Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn openai_model_header_casing_only_mismatch_does_not_warn() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let requested_header = REQUESTED_MODEL.to_ascii_uppercase(); + let response = sse_response(sse_completed("resp-1")) + .insert_header("OpenAI-Model", requested_header.as_str()); + let _mock = mount_response_once(&server, response).await; + + let mut builder = test_codex().with_model(REQUESTED_MODEL); + let test = builder.build(&server).await?; + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "trigger casing check".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: REQUESTED_MODEL.to_string(), + effort: test.config.model_reasoning_effort, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await?; + + let mut reroute_count = 0; + let mut warning_count = 0; + loop { + let event = wait_for_event(&test.codex, |_| true).await; + match event { + EventMsg::ModelReroute(_) => reroute_count += 1, + EventMsg::Warning(warning) + if warning + .message + .contains("flagged for potentially high-risk cyber activity") => + { + warning_count += 1; + } + EventMsg::TurnComplete(_) => break, + _ => {} + } + } + + assert_eq!(reroute_count, 0); + assert_eq!(warning_count, 0); + + Ok(()) +} From db4d2599b589d24e30eb491d63d1a834c5807258 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Tue, 17 Feb 2026 17:55:57 -0800 Subject: [PATCH 2/5] feat(core): plumb distinct approval ids for command approvals (#12051) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit zsh fork PR stack: - https://github.com/openai/codex/pull/12051 👈 - https://github.com/openai/codex/pull/12052 With upcoming support for a fork of zsh that allows us to intercept `execve` and run execpolicy checks for each subcommand as part of a `CommandExecution`, it will be possible for there to be multiple approval requests for a shell command like `/path/to/zsh -lc 'git status && rg \"TODO\" src && make test'`. To support that, this PR introduces a new `approval_id` field across core, protocol, and app-server so that we can associate approvals properly for subcommands. --- ...CommandExecutionRequestApprovalParams.json | 7 + .../schema/json/EventMsg.json | 18 +- .../json/ExecCommandApprovalParams.json | 7 + .../schema/json/ServerNotification.json | 9 +- .../schema/json/ServerRequest.json | 14 ++ .../codex_app_server_protocol.schemas.json | 23 ++- .../json/v1/ForkConversationResponse.json | 9 +- .../json/v1/ResumeConversationResponse.json | 9 +- .../v1/SessionConfiguredNotification.json | 9 +- .../typescript/ExecApprovalRequestEvent.ts | 9 +- .../typescript/ExecCommandApprovalParams.ts | 6 +- .../CommandExecutionRequestApprovalParams.ts | 10 + .../src/protocol/common.rs | 2 + .../app-server-protocol/src/protocol/v1.rs | 2 + .../app-server-protocol/src/protocol/v2.rs | 12 ++ codex-rs/app-server-test-client/src/lib.rs | 4 +- codex-rs/app-server/README.md | 2 +- .../app-server/src/bespoke_event_handling.rs | 185 +++++++++++------- codex-rs/app-server/src/thread_state.rs | 1 + .../suite/codex_message_processor_flow.rs | 1 + codex-rs/core/src/codex.rs | 12 +- codex-rs/core/src/codex_delegate.rs | 15 +- codex-rs/core/src/tools/network_approval.rs | 1 + codex-rs/core/src/tools/runtimes/shell.rs | 1 + .../core/src/tools/runtimes/unified_exec.rs | 1 + codex-rs/core/tests/suite/approvals.rs | 6 +- codex-rs/core/tests/suite/codex_delegate.rs | 2 +- codex-rs/core/tests/suite/otel.rs | 12 +- codex-rs/docs/codex_mcp_interface.md | 2 +- codex-rs/mcp-server/src/codex_tool_runner.rs | 28 +-- codex-rs/mcp-server/src/exec_approval.rs | 2 +- codex-rs/protocol/src/approvals.rs | 17 +- codex-rs/tui/src/chatwidget/tests.rs | 7 + 33 files changed, 331 insertions(+), 114 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index 9257aec839..1938701952 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -113,6 +113,13 @@ } }, "properties": { + "approvalId": { + "description": "Unique identifier for this specific approval callback.\n\nFor regular shell/unified_exec approvals, this is null.\n\nFor zsh-exec-bridge subcommand approvals, multiple callbacks can belong to one parent `itemId`, so `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing.", + "type": [ + "string", + "null" + ] + }, "command": { "description": "The command to be executed.", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 5ec61285ca..f38c9b9944 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -1452,8 +1452,15 @@ }, { "properties": { + "approval_id": { + "description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).", + "type": [ + "string", + "null" + ] + }, "call_id": { - "description": "Identifier for the associated exec call, if available.", + "description": "Identifier for the associated command execution item.", "type": "string" }, "command": { @@ -6392,8 +6399,15 @@ }, { "properties": { + "approval_id": { + "description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).", + "type": [ + "string", + "null" + ] + }, "call_id": { - "description": "Identifier for the associated exec call, if available.", + "description": "Identifier for the associated command execution item.", "type": "string" }, "command": { diff --git a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalParams.json b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalParams.json index 977b1626a0..3855168e09 100644 --- a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalParams.json @@ -117,6 +117,13 @@ } }, "properties": { + "approvalId": { + "description": "Identifier for this specific approval callback.", + "type": [ + "string", + "null" + ] + }, "callId": { "description": "Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] and [codex_core::protocol::ExecCommandEndEvent].", "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 540e037563..b6d87cd442 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -2268,8 +2268,15 @@ }, { "properties": { + "approval_id": { + "description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).", + "type": [ + "string", + "null" + ] + }, "call_id": { - "description": "Identifier for the associated exec call, if available.", + "description": "Identifier for the associated command execution item.", "type": "string" }, "command": { diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 3fa487385d..6be3bebc5e 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -179,6 +179,13 @@ }, "CommandExecutionRequestApprovalParams": { "properties": { + "approvalId": { + "description": "Unique identifier for this specific approval callback.\n\nFor regular shell/unified_exec approvals, this is null.\n\nFor zsh-exec-bridge subcommand approvals, multiple callbacks can belong to one parent `itemId`, so `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing.", + "type": [ + "string", + "null" + ] + }, "command": { "description": "The command to be executed.", "type": [ @@ -264,6 +271,13 @@ }, "ExecCommandApprovalParams": { "properties": { + "approvalId": { + "description": "Identifier for this specific approval callback.", + "type": [ + "string", + "null" + ] + }, "callId": { "description": "Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] and [codex_core::protocol::ExecCommandEndEvent].", "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 82f5c3b7dc..b6002dabd3 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -2051,6 +2051,13 @@ "CommandExecutionRequestApprovalParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "approvalId": { + "description": "Unique identifier for this specific approval callback.\n\nFor regular shell/unified_exec approvals, this is null.\n\nFor zsh-exec-bridge subcommand approvals, multiple callbacks can belong to one parent `itemId`, so `approvalId` is a distinct opaque callback id (a UUID) used to disambiguate routing.", + "type": [ + "string", + "null" + ] + }, "command": { "description": "The command to be executed.", "type": [ @@ -3465,8 +3472,15 @@ }, { "properties": { + "approval_id": { + "description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).", + "type": [ + "string", + "null" + ] + }, "call_id": { - "description": "Identifier for the associated exec call, if available.", + "description": "Identifier for the associated command execution item.", "type": "string" }, "command": { @@ -4939,6 +4953,13 @@ "ExecCommandApprovalParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { + "approvalId": { + "description": "Identifier for this specific approval callback.", + "type": [ + "string", + "null" + ] + }, "callId": { "description": "Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] and [codex_core::protocol::ExecCommandEndEvent].", "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json index a73c550f69..f460c4a288 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json @@ -1452,8 +1452,15 @@ }, { "properties": { + "approval_id": { + "description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).", + "type": [ + "string", + "null" + ] + }, "call_id": { - "description": "Identifier for the associated exec call, if available.", + "description": "Identifier for the associated command execution item.", "type": "string" }, "command": { diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json index 6dc02934d3..a39f1fd8e2 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json @@ -1452,8 +1452,15 @@ }, { "properties": { + "approval_id": { + "description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).", + "type": [ + "string", + "null" + ] + }, "call_id": { - "description": "Identifier for the associated exec call, if available.", + "description": "Identifier for the associated command execution item.", "type": "string" }, "command": { diff --git a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json index 69967e6853..20f4771e5e 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json @@ -1452,8 +1452,15 @@ }, { "properties": { + "approval_id": { + "description": "Identifier for this specific approval callback.\n\nWhen absent, the approval is for the command item itself (`call_id`). This is present for subcommand approvals (via execve intercept).", + "type": [ + "string", + "null" + ] + }, "call_id": { - "description": "Identifier for the associated exec call, if available.", + "description": "Identifier for the associated command execution item.", "type": "string" }, "command": { diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts index 542ed879b0..d0a7fef5cb 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts @@ -7,9 +7,16 @@ import type { ParsedCommand } from "./ParsedCommand"; export type ExecApprovalRequestEvent = { /** - * Identifier for the associated exec call, if available. + * Identifier for the associated command execution item. */ call_id: string, +/** + * Identifier for this specific approval callback. + * + * When absent, the approval is for the command item itself (`call_id`). + * This is present for subcommand approvals (via execve intercept). + */ +approval_id?: string, /** * Turn ID that this command belongs to. * Uses `#[serde(default)]` for backwards compatibility. diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecCommandApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/ExecCommandApprovalParams.ts index b427337a84..5637d1305a 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ExecCommandApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ExecCommandApprovalParams.ts @@ -9,4 +9,8 @@ export type ExecCommandApprovalParams = { conversationId: ThreadId, * Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] * and [codex_core::protocol::ExecCommandEndEvent]. */ -callId: string, command: Array, cwd: string, reason: string | null, parsedCmd: Array, }; +callId: string, +/** + * Identifier for this specific approval callback. + */ +approvalId: string | null, command: Array, cwd: string, reason: string | null, parsedCmd: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts index 12b2521431..290ce5669c 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts @@ -5,6 +5,16 @@ import type { CommandAction } from "./CommandAction"; import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; export type CommandExecutionRequestApprovalParams = { threadId: string, turnId: string, itemId: string, +/** + * Unique identifier for this specific approval callback. + * + * For regular shell/unified_exec approvals, this is null. + * + * For zsh-exec-bridge subcommand approvals, multiple callbacks can belong to + * one parent `itemId`, so `approvalId` is a distinct opaque callback id + * (a UUID) used to disambiguate routing. + */ +approvalId?: string | null, /** * Optional explanatory reason (e.g. request for network access). */ diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 64e5d2c682..d3f906bcaf 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1003,6 +1003,7 @@ mod tests { let params = v1::ExecCommandApprovalParams { conversation_id, call_id: "call-42".to_string(), + approval_id: Some("approval-42".to_string()), command: vec!["echo".to_string(), "hello".to_string()], cwd: PathBuf::from("/tmp"), reason: Some("because tests".to_string()), @@ -1022,6 +1023,7 @@ mod tests { "params": { "conversationId": "67e55044-10b1-426f-9247-bb680e5fe0c8", "callId": "call-42", + "approvalId": "approval-42", "command": ["echo", "hello"], "cwd": "/tmp", "reason": "because tests", diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs index 25e60ca751..13308ffafe 100644 --- a/codex-rs/app-server-protocol/src/protocol/v1.rs +++ b/codex-rs/app-server-protocol/src/protocol/v1.rs @@ -258,6 +258,8 @@ pub struct ExecCommandApprovalParams { /// Use to correlate this with [codex_core::protocol::ExecCommandBeginEvent] /// and [codex_core::protocol::ExecCommandEndEvent]. pub call_id: String, + /// Identifier for this specific approval callback. + pub approval_id: Option, pub command: Vec, pub cwd: PathBuf, pub reason: Option, diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 96c14ef5f1..52eeaf883a 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -3181,7 +3181,18 @@ pub struct CommandExecutionRequestApprovalParams { pub thread_id: String, pub turn_id: String, pub item_id: String, + /// Unique identifier for this specific approval callback. + /// + /// For regular shell/unified_exec approvals, this is null. + /// + /// For zsh-exec-bridge subcommand approvals, multiple callbacks can belong to + /// one parent `itemId`, so `approvalId` is a distinct opaque callback id + /// (a UUID) used to disambiguate routing. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional = nullable)] + pub approval_id: Option, /// Optional explanatory reason (e.g. request for network access). + #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub reason: Option, /// The command to be executed. @@ -3197,6 +3208,7 @@ pub struct CommandExecutionRequestApprovalParams { #[ts(optional = nullable)] pub command_actions: Option>, /// Optional proposed execpolicy amendment to allow similar commands without prompting. + #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub proposed_execpolicy_amendment: Option, } diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index 7b55c30a34..3f2dafbfe0 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -1302,6 +1302,7 @@ impl CodexClient { thread_id, turn_id, item_id, + approval_id, reason, command, cwd, @@ -1310,7 +1311,8 @@ impl CodexClient { } = params; println!( - "\n< commandExecution approval requested for thread {thread_id}, turn {turn_id}, item {item_id}" + "\n< commandExecution approval requested for thread {thread_id}, turn {turn_id}, item {item_id}, approval {}", + approval_id.as_deref().unwrap_or("") ); if let Some(reason) = reason.as_deref() { println!("< reason: {reason}"); diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index e414f1da6e..bb378264e7 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -637,7 +637,7 @@ Certain actions (shell commands or modifying files) may require explicit user ap Order of messages: 1. `item/started` — shows the pending `commandExecution` item with `command`, `cwd`, and other fields so you can render the proposed action. -2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `reason`, plus `command`, `cwd`, and `commandActions` for friendly display. +2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), `reason`, plus `command`, `cwd`, and `commandActions` for friendly display. 3. Client response — `{ "decision": "accept", "acceptSettings": { "forSession": false } }` or `{ "decision": "decline" }`. 4. `item/completed` — final `commandExecution` item with `status: "completed" | "failed" | "declined"` and execution output. Render this as the authoritative result. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 3acdefc731..33bbb9c347 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -214,76 +214,88 @@ pub(crate) async fn apply_bespoke_event_handling( }); } }, - EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { - call_id, - turn_id, - command, - cwd, - reason, - proposed_execpolicy_amendment, - parsed_cmd, - .. - }) => match api_version { - ApiVersion::V1 => { - let params = ExecCommandApprovalParams { - conversation_id, - call_id: call_id.clone(), - command, - cwd, - reason, - parsed_cmd, - }; - let rx = outgoing - .send_request(ServerRequestPayload::ExecCommandApproval(params)) - .await; - tokio::spawn(async move { - on_exec_approval_response(call_id, event_turn_id, rx, conversation).await; - }); - } - ApiVersion::V2 => { - let item_id = call_id.clone(); - let command_actions = parsed_cmd - .iter() - .cloned() - .map(V2ParsedCommand::from) - .collect::>(); - let command_string = shlex_join(&command); - let proposed_execpolicy_amendment_v2 = - proposed_execpolicy_amendment.map(V2ExecPolicyAmendment::from); - - let params = CommandExecutionRequestApprovalParams { - thread_id: conversation_id.to_string(), - turn_id: turn_id.clone(), - // Until we migrate the core to be aware of a first class CommandExecutionItem - // and emit the corresponding EventMsg, we repurpose the call_id as the item_id. - item_id: item_id.clone(), - reason, - command: Some(command_string.clone()), - cwd: Some(cwd.clone()), - command_actions: Some(command_actions.clone()), - proposed_execpolicy_amendment: proposed_execpolicy_amendment_v2, - }; - let rx = outgoing - .send_request(ServerRequestPayload::CommandExecutionRequestApproval( - params, - )) - .await; - tokio::spawn(async move { - on_command_execution_request_approval_response( - event_turn_id, + EventMsg::ExecApprovalRequest(ev) => { + let approval_id_for_op = ev.effective_approval_id(); + let ExecApprovalRequestEvent { + call_id, + approval_id, + turn_id, + command, + cwd, + reason, + proposed_execpolicy_amendment, + parsed_cmd, + .. + } = ev; + match api_version { + ApiVersion::V1 => { + let params = ExecCommandApprovalParams { conversation_id, - item_id, - command_string, + call_id: call_id.clone(), + approval_id, + command, cwd, - command_actions, - rx, - conversation, - outgoing, - ) - .await; - }); + reason, + parsed_cmd, + }; + let rx = outgoing + .send_request(ServerRequestPayload::ExecCommandApproval(params)) + .await; + tokio::spawn(async move { + on_exec_approval_response( + approval_id_for_op, + event_turn_id, + rx, + conversation, + ) + .await; + }); + } + ApiVersion::V2 => { + let command_actions = parsed_cmd + .iter() + .cloned() + .map(V2ParsedCommand::from) + .collect::>(); + let command_string = shlex_join(&command); + let proposed_execpolicy_amendment_v2 = + proposed_execpolicy_amendment.map(V2ExecPolicyAmendment::from); + + let params = CommandExecutionRequestApprovalParams { + thread_id: conversation_id.to_string(), + turn_id: turn_id.clone(), + item_id: call_id.clone(), + approval_id: approval_id.clone(), + reason, + command: Some(command_string.clone()), + cwd: Some(cwd.clone()), + command_actions: Some(command_actions.clone()), + proposed_execpolicy_amendment: proposed_execpolicy_amendment_v2, + }; + let rx = outgoing + .send_request(ServerRequestPayload::CommandExecutionRequestApproval( + params, + )) + .await; + tokio::spawn(async move { + on_command_execution_request_approval_response( + event_turn_id, + conversation_id, + approval_id, + call_id, + command_string, + cwd, + command_actions, + rx, + conversation, + outgoing, + thread_state.clone(), + ) + .await; + }); + } } - }, + } EventMsg::RequestUserInput(request) => { if matches!(api_version, ApiVersion::V2) { let questions = request @@ -933,6 +945,14 @@ pub(crate) async fn apply_bespoke_event_handling( let cwd = exec_command_begin_event.cwd; let process_id = exec_command_begin_event.process_id; + { + let mut state = thread_state.lock().await; + state + .turn_summary + .command_execution_started + .insert(item_id.clone()); + } + let item = ThreadItem::CommandExecution { id: item_id, command, @@ -1020,6 +1040,14 @@ pub(crate) async fn apply_bespoke_event_handling( .. } = exec_command_end_event; + { + let mut state = thread_state.lock().await; + state + .turn_summary + .command_execution_started + .remove(&call_id); + } + let status: CommandExecutionStatus = (&status).into(); let command_actions = parsed_cmd .into_iter() @@ -1739,6 +1767,7 @@ async fn on_file_change_request_approval_response( async fn on_command_execution_request_approval_response( event_turn_id: String, conversation_id: ThreadId, + approval_id: Option, item_id: String, command: String, cwd: PathBuf, @@ -1746,6 +1775,7 @@ async fn on_command_execution_request_approval_response( receiver: oneshot::Receiver, conversation: Arc, outgoing: ThreadScopedOutgoingMessageSender, + thread_state: Arc>, ) { let response = receiver.await; let (decision, completion_status) = match response { @@ -1794,7 +1824,24 @@ async fn on_command_execution_request_approval_response( } }; - if let Some(status) = completion_status { + let suppress_subcommand_completion_item = { + // For regular shell/unified_exec approvals, approval_id is null. + // For zsh-fork subcommand approvals, approval_id is present and + // item_id points to the parent command item. + if approval_id.is_some() { + let state = thread_state.lock().await; + state + .turn_summary + .command_execution_started + .contains(&item_id) + } else { + false + } + }; + + if let Some(status) = completion_status + && !suppress_subcommand_completion_item + { complete_command_execution_item( conversation_id, event_turn_id.clone(), @@ -1811,7 +1858,7 @@ async fn on_command_execution_request_approval_response( if let Err(err) = conversation .submit(Op::ExecApproval { - id: item_id, + id: approval_id.unwrap_or_else(|| item_id.clone()), turn_id: Some(event_turn_id), decision, }) diff --git a/codex-rs/app-server/src/thread_state.rs b/codex-rs/app-server/src/thread_state.rs index 2ccb7eaae7..c7fd963c39 100644 --- a/codex-rs/app-server/src/thread_state.rs +++ b/codex-rs/app-server/src/thread_state.rs @@ -20,6 +20,7 @@ type PendingInterruptQueue = Vec<( #[derive(Default, Clone)] pub(crate) struct TurnSummary { pub(crate) file_change_started: HashSet, + pub(crate) command_execution_started: HashSet, pub(crate) last_error: Option, } diff --git a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs index 863a7bfc4f..e6f887dba0 100644 --- a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs @@ -298,6 +298,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() -> Result<()> { ExecCommandApprovalParams { conversation_id, call_id: "call1".to_string(), + approval_id: None, command: format_with_current_shell("python3 -c 'print(42)'"), cwd: working_directory.clone(), reason: None, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 210809a812..7744480ab4 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2155,7 +2155,7 @@ impl Session { /// Emit an exec approval request event and await the user's decision. /// - /// The request is keyed by `call_id` so matching responses are delivered + /// The request is keyed by `call_id` + `approval_id` so matching responses are delivered /// to the correct in-flight turn. If the task is aborted, this returns the /// default `ReviewDecision` (`Denied`). #[allow(clippy::too_many_arguments)] @@ -2163,32 +2163,36 @@ impl Session { &self, turn_context: &TurnContext, call_id: String, + approval_id: Option, command: Vec, cwd: PathBuf, reason: Option, network_approval_context: Option, proposed_execpolicy_amendment: Option, ) -> ReviewDecision { + // command-level approvals use `call_id`. + // `approval_id` is only present for subcommand callbacks (execve intercept) + let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone()); // Add the tx_approve callback to the map before sending the request. let (tx_approve, rx_approve) = oneshot::channel(); - let approval_id = call_id.clone(); let prev_entry = { let mut active = self.active_turn.lock().await; match active.as_mut() { Some(at) => { let mut ts = at.turn_state.lock().await; - ts.insert_pending_approval(approval_id.clone(), tx_approve) + ts.insert_pending_approval(effective_approval_id.clone(), tx_approve) } None => None, } }; if prev_entry.is_some() { - warn!("Overwriting existing pending approval for call_id: {approval_id}"); + warn!("Overwriting existing pending approval for call_id: {effective_approval_id}"); } let parsed_cmd = parse_command(&command); let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { call_id, + approval_id, turn_id: turn_context.sub_id.clone(), command, cwd, diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 69efeefeca..fb0f5ad93f 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -311,8 +311,10 @@ async fn handle_exec_approval( event: ExecApprovalRequestEvent, cancel_token: &CancellationToken, ) { + let approval_id_for_op = event.effective_approval_id(); let ExecApprovalRequestEvent { call_id, + approval_id, command, cwd, reason, @@ -320,23 +322,28 @@ async fn handle_exec_approval( proposed_execpolicy_amendment, .. } = event; - let approval_id = call_id.clone(); // Race approval with cancellation and timeout to avoid hangs. let approval_fut = parent_session.request_command_approval( parent_ctx, call_id, + approval_id, command, cwd, reason, network_approval_context, proposed_execpolicy_amendment, ); - let decision = - await_approval_with_cancel(approval_fut, parent_session, &approval_id, cancel_token).await; + let decision = await_approval_with_cancel( + approval_fut, + parent_session, + &approval_id_for_op, + cancel_token, + ) + .await; let _ = codex .submit(Op::ExecApproval { - id: approval_id, + id: approval_id_for_op, turn_id: Some(turn_id), decision, }) diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 51e6e3c98e..d5643e4feb 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -247,6 +247,7 @@ impl NetworkApprovalService { .request_command_approval( turn_context.as_ref(), attempt.call_id.clone(), + None, attempt.command.clone(), attempt.cwd.clone(), Some(format!( diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 3d7a61ef77..a277e48a58 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -109,6 +109,7 @@ impl Approvable for ShellRuntime { .request_command_approval( turn, call_id, + None, command, cwd, reason, diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 89b159c5b8..be049d0934 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -110,6 +110,7 @@ impl Approvable for UnifiedExecRuntime<'_> { .request_command_approval( turn, call_id, + None, command, cwd, reason, diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 09d6755d3d..14d9c79e5e 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -1603,7 +1603,7 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { } test.codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: decision.clone(), }) @@ -1823,7 +1823,7 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts test.codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: ReviewDecision::ApprovedExecpolicyAmendment { proposed_execpolicy_amendment: expected_execpolicy_amendment.clone(), @@ -1988,7 +1988,7 @@ async fn compound_command_with_one_safe_command_still_requires_approval() -> Res let approval = expect_exec_approval(&test, expected_command.as_str()).await; test.codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: ReviewDecision::Denied, }) diff --git a/codex-rs/core/tests/suite/codex_delegate.rs b/codex-rs/core/tests/suite/codex_delegate.rs index 0cbdcef06e..841dbbb2cb 100644 --- a/codex-rs/core/tests/suite/codex_delegate.rs +++ b/codex-rs/core/tests/suite/codex_delegate.rs @@ -100,7 +100,7 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() { // Approve via parent using the emitted approval call ID. test.codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: ReviewDecision::Approved, }) diff --git a/codex-rs/core/tests/suite/otel.rs b/codex-rs/core/tests/suite/otel.rs index 38f223769f..fd3d1ca16c 100644 --- a/codex-rs/core/tests/suite/otel.rs +++ b/codex-rs/core/tests/suite/otel.rs @@ -1042,7 +1042,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() { codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: ReviewDecision::Approved, }) @@ -1108,7 +1108,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision() codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: ReviewDecision::ApprovedForSession, }) @@ -1174,7 +1174,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() { codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: ReviewDecision::Approved, }) @@ -1240,7 +1240,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() { codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: ReviewDecision::Denied, }) @@ -1306,7 +1306,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision() codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: ReviewDecision::ApprovedForSession, }) @@ -1373,7 +1373,7 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() { codex .submit(Op::ExecApproval { - id: approval.call_id, + id: approval.effective_approval_id(), turn_id: None, decision: ReviewDecision::Denied, }) diff --git a/codex-rs/docs/codex_mcp_interface.md b/codex-rs/docs/codex_mcp_interface.md index 73512c03c8..503891a491 100644 --- a/codex-rs/docs/codex_mcp_interface.md +++ b/codex-rs/docs/codex_mcp_interface.md @@ -146,7 +146,7 @@ Example: When Codex needs approval to apply changes or run commands, the server issues JSON‑RPC requests to the client: - `applyPatchApproval { conversationId, callId, fileChanges, reason?, grantRoot? }` -- `execCommandApproval { conversationId, callId, command, cwd, reason? }` +- `execCommandApproval { conversationId, callId, approvalId?, command, cwd, reason? }` The client must reply with `{ decision: "allow" | "deny" }` for each request. diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 04fa658017..c20c877c41 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -213,16 +213,19 @@ async fn run_codex_tool_session_inner( .await; match event.msg { - EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { - turn_id: _, - command, - cwd, - call_id, - reason: _, - proposed_execpolicy_amendment: _, - parsed_cmd, - network_approval_context: _, - }) => { + EventMsg::ExecApprovalRequest(ev) => { + let approval_id = ev.effective_approval_id(); + let ExecApprovalRequestEvent { + turn_id: _, + command, + cwd, + call_id, + approval_id: _, + reason: _, + proposed_execpolicy_amendment: _, + parsed_cmd, + network_approval_context: _, + } = ev; handle_exec_approval_request( command, cwd, @@ -232,6 +235,7 @@ async fn run_codex_tool_session_inner( request_id_str.clone(), event.id.clone(), call_id, + approval_id, parsed_cmd, thread_id, ) @@ -254,9 +258,6 @@ async fn run_codex_tool_session_inner( EventMsg::Warning(_) => { continue; } - EventMsg::ModelReroute(_) => { - continue; - } EventMsg::ElicitationRequest(_) => { // TODO: forward elicitation requests to the client? continue; @@ -361,6 +362,7 @@ async fn run_codex_tool_session_inner( | EventMsg::RequestUserInput(_) | EventMsg::DynamicToolCallRequest(_) | EventMsg::ContextCompacted(_) + | EventMsg::ModelReroute(_) | EventMsg::ThreadRolledBack(_) | EventMsg::CollabAgentSpawnBegin(_) | EventMsg::CollabAgentSpawnEnd(_) diff --git a/codex-rs/mcp-server/src/exec_approval.rs b/codex-rs/mcp-server/src/exec_approval.rs index f4b0a3fe61..54994f9fdb 100644 --- a/codex-rs/mcp-server/src/exec_approval.rs +++ b/codex-rs/mcp-server/src/exec_approval.rs @@ -57,10 +57,10 @@ pub(crate) async fn handle_exec_approval_request( tool_call_id: String, event_id: String, call_id: String, + approval_id: String, codex_parsed_cmd: Vec, thread_id: ThreadId, ) { - let approval_id = call_id.clone(); let escaped_command = shlex::try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" ")); let message = format!( diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 4ee7bd8b30..f275ca66d7 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -57,8 +57,15 @@ pub struct NetworkApprovalContext { #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct ExecApprovalRequestEvent { - /// Identifier for the associated exec call, if available. + /// Identifier for the associated command execution item. pub call_id: String, + /// Identifier for this specific approval callback. + /// + /// When absent, the approval is for the command item itself (`call_id`). + /// This is present for subcommand approvals (via execve intercept). + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub approval_id: Option, /// Turn ID that this command belongs to. /// Uses `#[serde(default)]` for backwards compatibility. #[serde(default)] @@ -81,6 +88,14 @@ pub struct ExecApprovalRequestEvent { pub parsed_cmd: Vec, } +impl ExecApprovalRequestEvent { + pub fn effective_approval_id(&self) -> String { + self.approval_id + .clone() + .unwrap_or_else(|| self.call_id.clone()) + } +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct ElicitationRequestEvent { pub server_name: String, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index bfae9581b2..79232bddcb 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2472,6 +2472,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { // Trigger an exec approval request with a short, single-line command let ev = ExecApprovalRequestEvent { call_id: "call-short".into(), + approval_id: Some("call-short".into()), turn_id: "turn-short".into(), command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), @@ -2517,6 +2518,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { // Multiline command: modal should show full command, history records decision only let ev_multi = ExecApprovalRequestEvent { call_id: "call-multi".into(), + approval_id: Some("call-multi".into()), turn_id: "turn-multi".into(), command: vec!["bash".into(), "-lc".into(), "echo line1\necho line2".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), @@ -2570,6 +2572,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { let long = format!("echo {}", "a".repeat(200)); let ev_long = ExecApprovalRequestEvent { call_id: "call-long".into(), + approval_id: Some("call-long".into()), turn_id: "turn-long".into(), command: vec!["bash".into(), "-lc".into(), long], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), @@ -5684,6 +5687,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> { // Inject an exec approval request to display the approval modal. let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd".into(), + approval_id: Some("call-approve-cmd".into()), turn_id: "turn-approve-cmd".into(), command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), @@ -5743,6 +5747,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd-noreason".into(), + approval_id: Some("call-approve-cmd-noreason".into()), turn_id: "turn-approve-cmd-noreason".into(), command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), @@ -5791,6 +5796,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() let command = vec!["bash".into(), "-lc".into(), script]; let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd-multiline-trunc".into(), + approval_id: Some("call-approve-cmd-multiline-trunc".into()), turn_id: "turn-approve-cmd-multiline-trunc".into(), command: command.clone(), cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), @@ -6149,6 +6155,7 @@ async fn status_widget_and_approval_modal_snapshot() { // Now show an approval modal (e.g. exec approval). let ev = ExecApprovalRequestEvent { call_id: "call-approve-exec".into(), + approval_id: Some("call-approve-exec".into()), turn_id: "turn-approve-exec".into(), command: vec!["echo".into(), "hello world".into()], cwd: PathBuf::from("/tmp"), From eb68767f2fa4d7fff17f6a041eed946b8af56d08 Mon Sep 17 00:00:00 2001 From: Charley Cunningham Date: Tue, 17 Feb 2026 18:18:47 -0800 Subject: [PATCH 3/5] Unify remote compaction snapshot mocks around default endpoint behavior (#12050) ## Summary - standardize remote compaction test mocking around one default behavior in shared helpers - make default remote compact mocks mirror production shape: keep `message/user` + `message/developer`, drop assistant/tool artifacts, then append a summary user message - switch non-special `compact_remote` tests to the shared default mock instead of ad-hoc JSON payloads ## Special-case tests that still use explicit mocks - remote compaction error payload / HTTP failure behavior - summary-only compact output behavior - manual `/compact` with no prior user messages - stale developer-instruction injection coverage ## Why This removes inconsistent manual remote compaction fixtures and gives us one source of truth for normal remote compact behavior, while preserving explicit mocks only where tests intentionally cover non-default behavior. --- codex-rs/core/tests/common/responses.rs | 82 ++++ codex-rs/core/tests/suite/compact.rs | 125 ++++++ codex-rs/core/tests/suite/compact_remote.rs | 388 ++++++++++++++++-- ...n_strips_incoming_model_switch_shapes.snap | 32 ++ ...y_reinjects_above_last_summary_shapes.snap | 21 + ...summary_only_reinjects_context_shapes.snap | 19 + ...n_strips_incoming_model_switch_shapes.snap | 28 ++ 7 files changed, 655 insertions(+), 40 deletions(-) create mode 100644 codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_strips_incoming_model_switch_shapes.snap create mode 100644 codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_mid_turn_compaction_multi_summary_reinjects_above_last_summary_shapes.snap create mode 100644 codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_mid_turn_compaction_summary_only_reinjects_context_shapes.snap create mode 100644 codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_strips_incoming_model_switch_shapes.snap diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index c434c89717..702e13f81d 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -865,6 +865,88 @@ pub async fn mount_compact_json_once(server: &MockServer, body: serde_json::Valu .await } +/// Mount a `/responses/compact` mock that mirrors the default remote compaction shape: +/// keep user+developer messages from the request, drop assistant/tool artifacts, and append one +/// summary user message. +pub async fn mount_compact_user_history_with_summary_once( + server: &MockServer, + summary_text: &str, +) -> ResponseMock { + mount_compact_user_history_with_summary_sequence(server, vec![summary_text.to_string()]).await +} + +/// Same as [`mount_compact_user_history_with_summary_once`], but for multiple compact calls. +/// Each incoming compact request receives the next summary text in order. +pub async fn mount_compact_user_history_with_summary_sequence( + server: &MockServer, + summary_texts: Vec, +) -> ResponseMock { + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering; + + #[derive(Debug)] + struct UserHistorySummaryResponder { + num_calls: AtomicUsize, + summary_texts: Vec, + } + + impl Respond for UserHistorySummaryResponder { + fn respond(&self, request: &wiremock::Request) -> ResponseTemplate { + let call_num = self.num_calls.fetch_add(1, Ordering::SeqCst); + let Some(summary_text) = self.summary_texts.get(call_num) else { + panic!("no summary text for compact request {call_num}"); + }; + let body_bytes = decode_body_bytes( + &request.body, + request + .headers + .get("content-encoding") + .and_then(|value| value.to_str().ok()), + ); + let body_json: Value = serde_json::from_slice(&body_bytes) + .unwrap_or_else(|err| panic!("failed to parse compact request body: {err}")); + let mut output = body_json + .get("input") + .and_then(Value::as_array) + .cloned() + .unwrap_or_default() + .into_iter() + // Match current remote compaction behavior: keep user/developer messages and + // omit assistant/tool history entries. + .filter(|item| { + item.get("type").and_then(Value::as_str) == Some("message") + && matches!( + item.get("role").and_then(Value::as_str), + Some("user") | Some("developer") + ) + }) + .collect::>(); + // Append the synthetic summary message as the newest user item. + output.push(serde_json::json!({ + "type": "message", + "role": "user", + "content": [{"type": "input_text", "text": summary_text}], + })); + ResponseTemplate::new(200) + .insert_header("content-type", "application/json") + .set_body_json(serde_json::json!({ "output": output })) + } + } + + let num_calls = summary_texts.len(); + let responder = UserHistorySummaryResponder { + num_calls: AtomicUsize::new(0), + summary_texts, + }; + let (mock, response_mock) = compact_mock(); + mock.respond_with(responder) + .up_to_n_times(num_calls as u64) + .expect(num_calls as u64) + .mount(server) + .await; + response_mock +} + pub async fn mount_compact_response_once( server: &MockServer, response: ResponseTemplate, diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 22a6c72076..08850ec38f 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -3038,6 +3038,131 @@ async fn snapshot_request_shape_pre_turn_compaction_including_incoming_user_mess #[tokio::test(flavor = "multi_thread", worker_threads = 2)] // TODO(ccunningham): Update once pre-turn compaction context-overflow handling includes incoming // user input and emits richer oversized-input messaging. +async fn snapshot_request_shape_pre_turn_compaction_strips_incoming_model_switch() { + skip_if_no_network!(); + + let server = start_mock_server().await; + let previous_model = "gpt-5.1-codex-max"; + let next_model = "gpt-5.2-codex"; + + let request_log = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_assistant_message("m1", "BEFORE_SWITCH_REPLY"), + ev_completed_with_tokens("r1", 500), + ]), + sse(vec![ + ev_assistant_message("m2", "PRETURN_SWITCH_SUMMARY"), + ev_completed_with_tokens("r2", 100), + ]), + sse(vec![ + ev_assistant_message("m3", "AFTER_SWITCH_REPLY"), + ev_completed_with_tokens("r3", 100), + ]), + ], + ) + .await; + + let model_provider = non_openai_model_provider(&server); + let test = test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_model(previous_model) + .with_config(move |config| { + config.model_provider = model_provider; + set_test_compact_prompt(config); + config + .features + .enable(codex_core::features::Feature::RemoteModels); + config.model_auto_compact_token_limit = Some(200); + }) + .build(&server) + .await + .expect("build codex"); + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "BEFORE_SWITCH_USER".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: previous_model.to_string(), + effort: None, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await + .expect("submit first user turn"); + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "AFTER_SWITCH_USER".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: next_model.to_string(), + effort: None, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await + .expect("submit second user turn"); + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let requests = request_log.requests(); + assert_eq!( + requests.len(), + 3, + "expected first turn, pre-turn compact, and post-compact follow-up requests" + ); + + let compact_body = requests[1].body_json().to_string(); + assert!( + body_contains_text(&compact_body, SUMMARIZATION_PROMPT), + "pre-turn compaction request should include summarization prompt" + ); + assert!( + !compact_body.contains(""), + "pre-turn compaction request should strip incoming model-switch update item" + ); + + let follow_up_body = requests[2].body_json().to_string(); + assert!( + follow_up_body.contains(""), + "post-compaction follow-up should include model-switch update item" + ); + + insta::assert_snapshot!( + "pre_turn_compaction_strips_incoming_model_switch_shapes", + format_labeled_requests_snapshot( + "Pre-turn compaction during model switch (without pre-sampling model-switch compaction): current behavior strips incoming from the compact request and restores it in the post-compaction follow-up request.", + &[ + ("Initial Request (Previous Model)", &requests[0]), + ("Local Compaction Request", &requests[1]), + ("Local Post-Compaction History Layout", &requests[2]), + ] + ) + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn snapshot_request_shape_pre_turn_compaction_context_window_exceeded() { skip_if_no_network!(); diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index 6c31b8dbeb..c9acde41ab 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -216,15 +216,9 @@ async fn remote_compact_runs_automatically() -> Result<()> { ) .await; - let compacted_history = vec![ - responses::user_message_item("REMOTE_COMPACTED_SUMMARY"), - ResponseItem::Compaction { - encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), - }, - ]; - let compact_mock = responses::mount_compact_json_once( + let compact_mock = responses::mount_compact_user_history_with_summary_once( harness.server(), - serde_json::json!({ "output": compacted_history.clone() }), + "REMOTE_COMPACTED_SUMMARY", ) .await; @@ -249,7 +243,6 @@ async fn remote_compact_runs_automatically() -> Result<()> { let follow_up_request = responses_mock.single_request(); let follow_up_body = follow_up_request.body_json().to_string(); assert!(follow_up_body.contains("REMOTE_COMPACTED_SUMMARY")); - assert!(follow_up_body.contains("ENCRYPTED_COMPACTION_SUMMARY")); Ok(()) } @@ -318,9 +311,11 @@ async fn remote_compact_trims_function_call_history_to_fit_context_window() -> R .await?; wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; - let compact_mock = - responses::mount_compact_json_once(harness.server(), serde_json::json!({ "output": [] })) - .await; + let compact_mock = responses::mount_compact_user_history_with_summary_once( + harness.server(), + "REMOTE_COMPACT_SUMMARY", + ) + .await; codex.submit(Op::Compact).await?; wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; @@ -436,9 +431,11 @@ async fn auto_remote_compact_trims_function_call_history_to_fit_context_window() .await?; wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; - let compact_mock = - responses::mount_compact_json_once(harness.server(), serde_json::json!({ "output": [] })) - .await; + let compact_mock = responses::mount_compact_user_history_with_summary_once( + harness.server(), + "REMOTE_AUTO_COMPACT_SUMMARY", + ) + .await; codex .submit(Op::UserInput { @@ -667,9 +664,9 @@ async fn remote_compact_trim_estimate_uses_session_base_instructions() -> Result }) .await; - let baseline_compact_mock = responses::mount_compact_json_once( + let baseline_compact_mock = responses::mount_compact_user_history_with_summary_once( baseline_harness.server(), - serde_json::json!({ "output": [] }), + "REMOTE_BASELINE_SUMMARY", ) .await; @@ -766,9 +763,9 @@ async fn remote_compact_trim_estimate_uses_session_base_instructions() -> Result }) .await; - let override_compact_mock = responses::mount_compact_json_once( + let override_compact_mock = responses::mount_compact_user_history_with_summary_once( override_harness.server(), - serde_json::json!({ "output": [] }), + "REMOTE_OVERRIDE_SUMMARY", ) .await; @@ -814,15 +811,9 @@ async fn remote_manual_compact_emits_context_compaction_items() -> Result<()> { ) .await; - let compacted_history = vec![ - responses::user_message_item("REMOTE_COMPACTED_SUMMARY"), - ResponseItem::Compaction { - encrypted_content: "ENCRYPTED_COMPACTION_SUMMARY".to_string(), - }, - ]; - let compact_mock = responses::mount_compact_json_once( + let compact_mock = responses::mount_compact_user_history_with_summary_once( harness.server(), - serde_json::json!({ "output": compacted_history.clone() }), + "REMOTE_COMPACTED_SUMMARY", ) .await; @@ -1353,14 +1344,9 @@ async fn snapshot_request_shape_remote_pre_turn_compaction_including_incoming_us ) .await; - let compacted_history = vec![ - responses::user_message_item("USER_ONE"), - responses::user_message_item("USER_TWO"), - responses::user_message_item(&summary_with_prefix("REMOTE_PRE_TURN_SUMMARY")), - ]; - let compact_mock = responses::mount_compact_json_once( + let compact_mock = responses::mount_compact_user_history_with_summary_once( harness.server(), - serde_json::json!({ "output": compacted_history }), + &summary_with_prefix("REMOTE_PRE_TURN_SUMMARY"), ) .await; @@ -1424,6 +1410,142 @@ async fn snapshot_request_shape_remote_pre_turn_compaction_including_incoming_us Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn snapshot_request_shape_remote_pre_turn_compaction_strips_incoming_model_switch() +-> Result<()> { + skip_if_no_network!(Ok(())); + + let previous_model = "gpt-5.1-codex-max"; + let next_model = "gpt-5.2-codex"; + let harness = TestCodexHarness::with_builder( + test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_model(previous_model) + .with_config(|config| { + config.model_auto_compact_token_limit = Some(200); + }), + ) + .await?; + let codex = harness.test().codex.clone(); + + let initial_turn_request_mock = responses::mount_sse_once( + harness.server(), + responses::sse(vec![ + responses::ev_assistant_message("m1", "BEFORE_SWITCH_REPLY"), + responses::ev_completed_with_tokens("r1", 500), + ]), + ) + .await; + let post_compact_turn_request_mock = responses::mount_sse_once( + harness.server(), + responses::sse(vec![ + responses::ev_assistant_message("m2", "AFTER_SWITCH_REPLY"), + responses::ev_completed_with_tokens("r2", 80), + ]), + ) + .await; + let compact_mock = responses::mount_compact_user_history_with_summary_once( + harness.server(), + &summary_with_prefix("REMOTE_SWITCH_SUMMARY"), + ) + .await; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "BEFORE_SWITCH_USER".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + codex + .submit(Op::OverrideTurnContext { + cwd: None, + approval_policy: None, + sandbox_policy: None, + windows_sandbox_level: None, + model: Some(next_model.to_string()), + effort: None, + summary: None, + collaboration_mode: None, + personality: None, + }) + .await?; + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "AFTER_SWITCH_USER".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + assert_eq!( + compact_mock.requests().len(), + 1, + "expected a single remote pre-turn compaction request" + ); + assert_eq!( + initial_turn_request_mock.requests().len(), + 1, + "expected initial turn request" + ); + assert_eq!( + post_compact_turn_request_mock.requests().len(), + 1, + "expected post-compaction follow-up request" + ); + + let initial_turn_request = initial_turn_request_mock.single_request(); + let compact_request = compact_mock.single_request(); + let post_compact_turn_request = post_compact_turn_request_mock.single_request(); + let compact_body = compact_request.body_json().to_string(); + assert!( + !compact_body.contains("AFTER_SWITCH_USER"), + "current behavior excludes incoming user from the pre-turn remote compaction request" + ); + assert!( + !compact_body.contains(""), + "pre-turn remote compaction request should strip incoming model-switch update item" + ); + + let follow_up_body = post_compact_turn_request.body_json().to_string(); + assert!( + follow_up_body.contains("BEFORE_SWITCH_USER"), + "post-compaction follow-up should preserve older user messages when they fit" + ); + assert!( + follow_up_body.contains("AFTER_SWITCH_USER"), + "post-compaction follow-up should preserve incoming user message via runtime append" + ); + assert!( + follow_up_body.contains(""), + "post-compaction follow-up should include the model-switch update item" + ); + + insta::assert_snapshot!( + "remote_pre_turn_compaction_strips_incoming_model_switch_shapes", + format_labeled_requests_snapshot( + "Remote pre-turn compaction during model switch currently excludes incoming user input, strips incoming from the compact request payload, and restores it in the post-compaction follow-up request.", + &[ + ("Initial Request (Previous Model)", &initial_turn_request), + ("Remote Compaction Request", &compact_request), + ( + "Remote Post-Compaction History Layout", + &post_compact_turn_request + ), + ] + ) + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] // TODO(ccunningham): Update once remote pre-turn compaction context-overflow handling includes // incoming user input and emits richer oversized-input messaging. @@ -1555,13 +1677,9 @@ async fn snapshot_request_shape_remote_mid_turn_continuation_compaction() -> Res ) .await; - let compacted_history = vec![ - responses::user_message_item("USER_ONE"), - responses::user_message_item(&summary_with_prefix("REMOTE_MID_TURN_SUMMARY")), - ]; - let compact_mock = responses::mount_compact_json_once( + let compact_mock = responses::mount_compact_user_history_with_summary_once( harness.server(), - serde_json::json!({ "output": compacted_history }), + &summary_with_prefix("REMOTE_MID_TURN_SUMMARY"), ) .await; @@ -1599,6 +1717,196 @@ async fn snapshot_request_shape_remote_mid_turn_continuation_compaction() -> Res Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn snapshot_request_shape_remote_mid_turn_compaction_summary_only_reinjects_context() +-> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = TestCodexHarness::with_builder( + test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(|config| { + config.model_auto_compact_token_limit = Some(200); + }), + ) + .await?; + let codex = harness.test().codex.clone(); + + let initial_turn_request_mock = responses::mount_sse_once( + harness.server(), + responses::sse(vec![ + responses::ev_function_call("call-remote-summary-only", DUMMY_FUNCTION_NAME, "{}"), + responses::ev_completed_with_tokens("r1", 500), + ]), + ) + .await; + let post_compact_turn_request_mock = responses::mount_sse_once( + harness.server(), + responses::sse(vec![ + responses::ev_assistant_message("m2", "REMOTE_SUMMARY_ONLY_FINAL_REPLY"), + responses::ev_completed_with_tokens("r2", 80), + ]), + ) + .await; + + let compacted_history = vec![responses::user_message_item(&summary_with_prefix( + "REMOTE_SUMMARY_ONLY", + ))]; + let compact_mock = responses::mount_compact_json_once( + harness.server(), + serde_json::json!({ "output": compacted_history }), + ) + .await; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "USER_ONE".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + assert_eq!(compact_mock.requests().len(), 1); + assert_eq!( + initial_turn_request_mock.requests().len(), + 1, + "expected initial turn request" + ); + assert_eq!( + post_compact_turn_request_mock.requests().len(), + 1, + "expected post-compaction request" + ); + + let compact_request = compact_mock.single_request(); + let post_compact_turn_request = post_compact_turn_request_mock.single_request(); + insta::assert_snapshot!( + "remote_mid_turn_compaction_summary_only_reinjects_context_shapes", + format_labeled_requests_snapshot( + "Remote mid-turn compaction where compact output has only summary user content: continuation layout reinjects canonical context before that summary.", + &[ + ("Remote Compaction Request", &compact_request), + ( + "Remote Post-Compaction History Layout", + &post_compact_turn_request + ), + ] + ) + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn snapshot_request_shape_remote_mid_turn_compaction_multi_summary_reinjects_above_last_summary() +-> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = TestCodexHarness::with_builder( + test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(|config| { + config.model_auto_compact_token_limit = Some(200); + }), + ) + .await?; + let codex = harness.test().codex.clone(); + + let setup_turn_request_mock = responses::mount_sse_once( + harness.server(), + responses::sse(vec![ + responses::ev_assistant_message("setup", "REMOTE_SETUP_REPLY"), + responses::ev_completed_with_tokens("setup-response", 60), + ]), + ) + .await; + let second_turn_request_mock = responses::mount_sse_once( + harness.server(), + responses::sse(vec![ + responses::ev_shell_command_call("call-remote-multi-summary", "echo multi-summary"), + responses::ev_completed_with_tokens("r1", 1_000), + ]), + ) + .await; + + let compact_mock = responses::mount_compact_user_history_with_summary_sequence( + harness.server(), + vec![ + summary_with_prefix("REMOTE_OLDER_SUMMARY"), + summary_with_prefix("REMOTE_LATEST_SUMMARY"), + ], + ) + .await; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "USER_ONE".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + codex.submit(Op::Compact).await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "USER_TWO".to_string(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + assert_eq!(compact_mock.requests().len(), 2); + assert_eq!( + setup_turn_request_mock.requests().len(), + 1, + "expected setup turn request" + ); + assert_eq!( + second_turn_request_mock.requests().len(), + 1, + "expected second-turn pre-compaction request" + ); + + let compact_requests = compact_mock.requests(); + assert_eq!( + compact_requests.len(), + 2, + "expected one setup compact and one mid-turn compact request" + ); + let compact_request = compact_requests[1].clone(); + let second_turn_request = second_turn_request_mock.single_request(); + assert!( + compact_request.body_contains_text("REMOTE_OLDER_SUMMARY"), + "older summary should round-trip from conversation history into the next compact request" + ); + insta::assert_snapshot!( + "remote_mid_turn_compaction_multi_summary_reinjects_above_last_summary_shapes", + format_labeled_requests_snapshot( + "Remote mid-turn compaction after an earlier summary compaction: the older summary remains in model-visible history and round-trips into the next compact request.", + &[ + ( + "Second Turn Request (Before Mid-Turn Compaction)", + &second_turn_request + ), + ("Remote Compaction Request", &compact_request), + ] + ) + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] // TODO(ccunningham): Update once manual remote /compact with no prior user turn becomes a no-op. async fn snapshot_request_shape_remote_manual_compact_without_previous_user_messages() -> Result<()> diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_strips_incoming_model_switch_shapes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_strips_incoming_model_switch_shapes.snap new file mode 100644 index 0000000000..4d67af911d --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_strips_incoming_model_switch_shapes.snap @@ -0,0 +1,32 @@ +--- +source: core/tests/suite/compact.rs +assertion_line: 3152 +expression: "format_labeled_requests_snapshot(\"Pre-turn compaction during model switch (without pre-sampling model-switch compaction): current behavior strips incoming from the compact request and restores it in the post-compaction follow-up request.\",\n&[(\"Initial Request (Previous Model)\", &requests[0]),\n(\"Local Compaction Request\", &requests[1]),\n(\"Local Post-Compaction History Layout\", &requests[2]),])" +--- +Scenario: Pre-turn compaction during model switch (without pre-sampling model-switch compaction): current behavior strips incoming from the compact request and restores it in the post-compaction follow-up request. + +## Initial Request (Previous Model) +00:message/developer: +01:message/user: +02:message/user:> +03:message/developer: +04:message/user:BEFORE_SWITCH_USER + +## Local Compaction Request +00:message/developer: +01:message/user: +02:message/user:> +03:message/developer: +04:message/user:BEFORE_SWITCH_USER +05:message/assistant:BEFORE_SWITCH_REPLY +06:message/user: + +## Local Post-Compaction History Layout +00:message/developer: +01:message/developer: The user has requested a new communication st... +02:message/user: +03:message/user:> +04:message/user:BEFORE_SWITCH_USER +05:message/user:\nPRETURN_SWITCH_SUMMARY +06:message/developer:\nThe user was previously using a different model.... +07:message/user:AFTER_SWITCH_USER diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_mid_turn_compaction_multi_summary_reinjects_above_last_summary_shapes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_mid_turn_compaction_multi_summary_reinjects_above_last_summary_shapes.snap new file mode 100644 index 0000000000..1f51f96544 --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_mid_turn_compaction_multi_summary_reinjects_above_last_summary_shapes.snap @@ -0,0 +1,21 @@ +--- +source: core/tests/suite/compact_remote.rs +expression: "format_labeled_requests_snapshot(\"Remote mid-turn compaction after an earlier summary compaction: the older summary remains in model-visible history and round-trips into the next compact request.\",\n&[(\"Second Turn Request (Before Mid-Turn Compaction)\", &requests[1]),\n(\"Remote Compaction Request\", &compact_request),])" +--- +Scenario: Remote mid-turn compaction after an earlier summary compaction: the older summary remains in model-visible history and round-trips into the next compact request. + +## Second Turn Request (Before Mid-Turn Compaction) +00:message/user:USER_ONE +01:message/user:\nREMOTE_OLDER_SUMMARY +02:message/developer: +03:message/user: +04:message/user:> +05:message/user:\nREMOTE_LATEST_SUMMARY +06:message/user:USER_TWO + +## Remote Compaction Request +00:message/user:USER_ONE +01:message/developer: +02:message/user: +03:message/user:> +04:message/user:\nREMOTE_OLDER_SUMMARY diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_mid_turn_compaction_summary_only_reinjects_context_shapes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_mid_turn_compaction_summary_only_reinjects_context_shapes.snap new file mode 100644 index 0000000000..4ede5c3cad --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_mid_turn_compaction_summary_only_reinjects_context_shapes.snap @@ -0,0 +1,19 @@ +--- +source: core/tests/suite/compact_remote.rs +expression: "format_labeled_requests_snapshot(\"Remote mid-turn compaction where compact output has only summary user content: continuation layout reinjects canonical context before that summary.\",\n&[(\"Remote Compaction Request\", &compact_request),\n(\"Remote Post-Compaction History Layout\", &requests[1]),])" +--- +Scenario: Remote mid-turn compaction where compact output has only summary user content: continuation layout reinjects canonical context before that summary. + +## Remote Compaction Request +00:message/developer: +01:message/user: +02:message/user:> +03:message/user:USER_ONE +04:function_call/test_tool +05:function_call_output:unsupported call: test_tool + +## Remote Post-Compaction History Layout +00:message/developer: +01:message/user: +02:message/user:> +03:message/user:\nREMOTE_SUMMARY_ONLY diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_strips_incoming_model_switch_shapes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_strips_incoming_model_switch_shapes.snap new file mode 100644 index 0000000000..fba17d719e --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_strips_incoming_model_switch_shapes.snap @@ -0,0 +1,28 @@ +--- +source: core/tests/suite/compact_remote.rs +expression: "format_labeled_requests_snapshot(\"Remote pre-turn compaction during model switch currently excludes incoming user input, strips incoming from the compact request payload, and restores it in the post-compaction follow-up request.\",\n&[(\"Initial Request (Previous Model)\", &requests[0]),\n(\"Remote Compaction Request\", &compact_request),\n(\"Remote Post-Compaction History Layout\", &requests[1]),])" +--- +Scenario: Remote pre-turn compaction during model switch currently excludes incoming user input, strips incoming from the compact request payload, and restores it in the post-compaction follow-up request. + +## Initial Request (Previous Model) +00:message/developer: +01:message/user: +02:message/user:> +03:message/user:BEFORE_SWITCH_USER + +## Remote Compaction Request +00:message/developer: +01:message/user: +02:message/user:> +03:message/user:BEFORE_SWITCH_USER +04:message/assistant:BEFORE_SWITCH_REPLY + +## Remote Post-Compaction History Layout +00:message/user:BEFORE_SWITCH_USER +01:message/developer: +02:message/developer: The user has requested a new communication st... +03:message/user: +04:message/user:> +05:message/user:\nREMOTE_SWITCH_SUMMARY +06:message/developer:\nThe user was previously using a different model.... +07:message/user:AFTER_SWITCH_USER From fc810ba045d02edbc1526d71d02e102cec142f7b Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Tue, 17 Feb 2026 18:32:16 -0800 Subject: [PATCH 4/5] Use V2 websockets if feature enabled (#12071) --- codex-rs/core/src/client.rs | 6 +- .../core/tests/suite/client_websockets.rs | 102 ++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 99f8464acd..09ae0eb0d8 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -212,6 +212,8 @@ impl ModelClient { include_timing_metrics: bool, beta_features_header: Option, ) -> Self { + let enable_responses_websockets = + enable_responses_websockets || enable_responses_websockets_v2; Self { state: Arc::new(ModelClientState { auth_manager, @@ -351,7 +353,9 @@ impl ModelClient { /// to be eligible. pub fn responses_websocket_enabled(&self, model_info: &ModelInfo) -> bool { self.state.provider.supports_websockets - && (self.state.enable_responses_websockets || model_info.prefer_websockets) + && (self.state.enable_responses_websockets + || self.state.enable_responses_websockets_v2 + || model_info.prefer_websockets) } fn responses_websockets_v2_enabled(&self) -> bool { diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index 1db81ecf86..626d0695be 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -189,6 +189,108 @@ async fn responses_websocket_prewarm_uses_model_preference_when_feature_disabled server.shutdown().await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn responses_websocket_v2_prewarm_runs_when_only_v2_feature_enabled() { + skip_if_no_network!(); + + let server = start_websocket_server(vec![vec![vec![ + ev_response_created("resp-1"), + ev_completed("resp-1"), + ]]]) + .await; + + let harness = websocket_harness_with_options(&server, false, false, true, false).await; + let mut client_session = harness.client.new_session(); + client_session + .prewarm_websocket(&harness.otel_manager, &harness.model_info) + .await + .expect("websocket prewarm failed"); + + assert_eq!(server.handshakes().len(), 1); + assert_eq!(server.single_connection().len(), 0); + + let prompt = prompt_with_input(vec![message_item("hello")]); + stream_until_complete(&mut client_session, &harness, &prompt).await; + + assert_eq!(server.handshakes().len(), 1); + assert_eq!(server.single_connection().len(), 1); + + let handshake = server.single_handshake(); + let openai_beta_header = handshake + .header(OPENAI_BETA_HEADER) + .expect("missing OpenAI-Beta header"); + assert!( + openai_beta_header + .split(',') + .map(str::trim) + .any(|value| value == WS_V2_BETA_HEADER_VALUE) + ); + assert!( + !openai_beta_header + .split(',') + .map(str::trim) + .any(|value| value == OPENAI_BETA_RESPONSES_WEBSOCKETS) + ); + + server.shutdown().await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn responses_websocket_v2_requests_use_v2_when_model_prefers_websockets() { + skip_if_no_network!(); + + let server = start_websocket_server(vec![vec![ + vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "assistant output"), + ev_done_with_id("resp-1"), + ], + vec![ev_response_created("resp-2"), ev_completed("resp-2")], + ]]) + .await; + + let harness = websocket_harness_with_options(&server, false, false, true, true).await; + let mut client_session = harness.client.new_session(); + let prompt_one = prompt_with_input(vec![message_item("hello")]); + let prompt_two = prompt_with_input(vec![ + message_item("hello"), + assistant_message_item("msg-1", "assistant output"), + message_item("second"), + ]); + + stream_until_complete(&mut client_session, &harness, &prompt_one).await; + stream_until_complete(&mut client_session, &harness, &prompt_two).await; + + let connection = server.single_connection(); + assert_eq!(connection.len(), 2); + let second = connection.get(1).expect("missing request").body_json(); + assert_eq!(second["type"].as_str(), Some("response.create")); + assert_eq!(second["previous_response_id"].as_str(), Some("resp-1")); + assert_eq!( + second["input"], + serde_json::to_value(&prompt_two.input[2..]).unwrap() + ); + + let handshake = server.single_handshake(); + let openai_beta_header = handshake + .header(OPENAI_BETA_HEADER) + .expect("missing OpenAI-Beta header"); + assert!( + openai_beta_header + .split(',') + .map(str::trim) + .any(|value| value == WS_V2_BETA_HEADER_VALUE) + ); + assert!( + !openai_beta_header + .split(',') + .map(str::trim) + .any(|value| value == OPENAI_BETA_RESPONSES_WEBSOCKETS) + ); + + server.shutdown().await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[traced_test] async fn responses_websocket_emits_websocket_telemetry_events() { From edacbf7b6e7f1df6d8d703cca5a2223aef01f617 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Tue, 17 Feb 2026 20:19:53 -0800 Subject: [PATCH 5/5] feat(core): zsh exec bridge (#12052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit zsh fork PR stack: - https://github.com/openai/codex/pull/12051 - https://github.com/openai/codex/pull/12052 👈 ### Summary This PR introduces a feature-gated native shell runtime path that routes shell execution through a patched zsh exec bridge, removing MCP-specific behavior from the shell hot path while preserving existing CommandExecution lifecycle semantics. When shell_zsh_fork is enabled, shell commands run via patched zsh with per-`execve` interception through EXEC_WRAPPER. Core receives wrapper IPC requests over a Unix socket, applies existing approval policy, and returns allow/deny before the subcommand executes. ### What’s included **1) New zsh exec bridge runtime in core** - Wrapper-mode entrypoint (maybe_run_zsh_exec_wrapper_mode) for EXEC_WRAPPER invocations. - Per-execution Unix-socket IPC handling for wrapper requests/responses. - Approval callback integration using existing core approval orchestration. - Streaming stdout/stderr deltas to existing command output event pipeline. - Error handling for malformed IPC, denial/abort, and execution failures. **2) Session lifecycle integration** SessionServices now owns a `ZshExecBridge`. Session startup initializes bridge state; shutdown tears it down cleanly. **3) Shell runtime routing (feature-gated)** When `shell_zsh_fork` is enabled: - Build execution env/spec as usual. - Add wrapper socket env wiring. - Execute via `zsh_exec_bridge.execute_shell_request(...)` instead of the regular shell path. - Non-zsh-fork behavior remains unchanged. **4) Config + feature wiring** - Added `Feature::ShellZshFork` (under development). - Added config support for `zsh_path` (optional absolute path to patched zsh): - `Config`, `ConfigToml`, `ConfigProfile`, overrides, and schema. - Session startup validates that `zsh_path` exists/usable when zsh-fork is enabled. - Added startup test for missing `zsh_path` failure mode. **5) Seatbelt/sandbox updates for wrapper IPC** - Extended seatbelt policy generation to optionally allow outbound connection to explicitly permitted Unix sockets. - Wired sandboxing path to pass wrapper socket path through to seatbelt policy generation. - Added/updated seatbelt tests for explicit socket allow rule and argument emission. **6) Runtime entrypoint hooks** - This allows the same binary to act as the zsh wrapper subprocess when invoked via `EXEC_WRAPPER`. **7) Tool selection behavior** - ToolsConfig now prefers ShellCommand type when shell_zsh_fork is enabled. - Added test coverage for precedence with unified-exec enabled. --- codex-rs/app-server-test-client/src/lib.rs | 205 +++++- codex-rs/app-server/src/main.rs | 3 + codex-rs/app-server/tests/suite/v2/mod.rs | 1 + .../tests/suite/v2/turn_start_zsh_fork.rs | 676 ++++++++++++++++++ codex-rs/cli/src/main.rs | 3 + codex-rs/core/config.schema.json | 22 + codex-rs/core/src/codex.rs | 103 ++- codex-rs/core/src/config/mod.rs | 14 + codex-rs/core/src/config/profile.rs | 2 + codex-rs/core/src/exec.rs | 4 +- codex-rs/core/src/features.rs | 8 + codex-rs/core/src/lib.rs | 2 + codex-rs/core/src/sandboxing/mod.rs | 7 + codex-rs/core/src/seatbelt.rs | 101 ++- codex-rs/core/src/state/service.rs | 2 + codex-rs/core/src/tools/runtimes/shell.rs | 31 + codex-rs/core/src/tools/spec.rs | 19 + codex-rs/core/src/zsh_exec_bridge/mod.rs | 554 ++++++++++++++ codex-rs/exec/src/lib.rs | 1 + 19 files changed, 1735 insertions(+), 23 deletions(-) create mode 100644 codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs create mode 100644 codex-rs/core/src/zsh_exec_bridge/mod.rs diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index 3f2dafbfe0..7b846c80f1 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -1,4 +1,5 @@ use std::collections::VecDeque; +use std::ffi::OsString; use std::fs; use std::fs::OpenOptions; use std::io::BufRead; @@ -29,6 +30,7 @@ use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::CommandExecutionApprovalDecision; use codex_app_server_protocol::CommandExecutionRequestApprovalParams; use codex_app_server_protocol::CommandExecutionRequestApprovalResponse; +use codex_app_server_protocol::CommandExecutionStatus; use codex_app_server_protocol::DynamicToolSpec; use codex_app_server_protocol::FileChangeApprovalDecision; use codex_app_server_protocol::FileChangeRequestApprovalParams; @@ -55,6 +57,7 @@ use codex_app_server_protocol::SendUserMessageParams; use codex_app_server_protocol::SendUserMessageResponse; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; +use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadListParams; use codex_app_server_protocol::ThreadListResponse; use codex_app_server_protocol::ThreadResumeParams; @@ -78,6 +81,30 @@ use tungstenite::stream::MaybeTlsStream; use url::Url; use uuid::Uuid; +const NOTIFICATIONS_TO_OPT_OUT: &[&str] = &[ + // Legacy codex/event (v1-style) deltas. + "codex/event/agent_message_content_delta", + "codex/event/agent_message_delta", + "codex/event/agent_reasoning_delta", + "codex/event/reasoning_content_delta", + "codex/event/reasoning_raw_content_delta", + "codex/event/exec_command_output_delta", + // Other legacy events. + "codex/event/exec_approval_request", + "codex/event/exec_command_begin", + "codex/event/exec_command_end", + "codex/event/exec_output", + "codex/event/item_started", + "codex/event/item_completed", + // v2 item deltas. + "item/agentMessage/delta", + "item/plan/delta", + "item/commandExecution/outputDelta", + "item/fileChange/outputDelta", + "item/reasoning/summaryTextDelta", + "item/reasoning/textDelta", +]; + /// Minimal launcher that initializes the Codex app-server and logs the handshake. #[derive(Parser)] #[command(author = "Codex", version, about = "Bootstrap Codex app-server", long_about = None)] @@ -180,6 +207,18 @@ enum CliCommand { /// Follow-up user message for the second turn. follow_up_message: String, }, + /// Trigger zsh-fork multi-subcommand approvals and assert expected approval behavior. + #[command(name = "trigger-zsh-fork-multi-cmd-approval")] + TriggerZshForkMultiCmdApproval { + /// Optional prompt; defaults to an explicit `/usr/bin/true && /usr/bin/true` command. + user_message: Option, + /// Minimum number of command-approval callbacks expected in the turn. + #[arg(long, default_value_t = 2)] + min_approvals: usize, + /// One-based approval index to abort (e.g. --abort-on 2 aborts the second approval). + #[arg(long)] + abort_on: Option, + }, /// Trigger the ChatGPT login flow and wait for completion. TestLogin, /// Fetch the current account rate limits from the Codex app-server. @@ -265,6 +304,21 @@ pub fn run() -> Result<()> { &dynamic_tools, ) } + CliCommand::TriggerZshForkMultiCmdApproval { + user_message, + min_approvals, + abort_on, + } => { + let endpoint = resolve_endpoint(codex_bin, url)?; + trigger_zsh_fork_multi_cmd_approval( + &endpoint, + &config_overrides, + user_message, + min_approvals, + abort_on, + &dynamic_tools, + ) + } CliCommand::TestLogin => { ensure_dynamic_tools_unused(&dynamic_tools, "test-login")?; let endpoint = resolve_endpoint(codex_bin, url)?; @@ -470,6 +524,101 @@ fn send_message_v2_endpoint( ) } +fn trigger_zsh_fork_multi_cmd_approval( + endpoint: &Endpoint, + config_overrides: &[String], + user_message: Option, + min_approvals: usize, + abort_on: Option, + dynamic_tools: &Option>, +) -> Result<()> { + if let Some(abort_on) = abort_on + && abort_on == 0 + { + bail!("--abort-on must be >= 1 when provided"); + } + + let default_prompt = "Run this exact command using shell command execution without rewriting or splitting it: /usr/bin/true && /usr/bin/true"; + let message = user_message.unwrap_or_else(|| default_prompt.to_string()); + + let mut client = CodexClient::connect(endpoint, config_overrides)?; + let initialize = client.initialize()?; + println!("< initialize response: {initialize:?}"); + + let thread_response = client.thread_start(ThreadStartParams { + dynamic_tools: dynamic_tools.clone(), + ..Default::default() + })?; + println!("< thread/start response: {thread_response:?}"); + + client.command_approval_behavior = match abort_on { + Some(index) => CommandApprovalBehavior::AbortOn(index), + None => CommandApprovalBehavior::AlwaysAccept, + }; + client.command_approval_count = 0; + client.command_approval_item_ids.clear(); + client.command_execution_statuses.clear(); + client.last_turn_status = None; + + let mut turn_params = TurnStartParams { + thread_id: thread_response.thread.id.clone(), + input: vec![V2UserInput::Text { + text: message, + text_elements: Vec::new(), + }], + ..Default::default() + }; + turn_params.approval_policy = Some(AskForApproval::OnRequest); + turn_params.sandbox_policy = Some(SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::FullAccess, + }); + + let turn_response = client.turn_start(turn_params)?; + println!("< turn/start response: {turn_response:?}"); + client.stream_turn(&thread_response.thread.id, &turn_response.turn.id)?; + + if client.command_approval_count < min_approvals { + bail!( + "expected at least {min_approvals} command approvals, got {}", + client.command_approval_count + ); + } + let mut approvals_per_item = std::collections::BTreeMap::new(); + for item_id in &client.command_approval_item_ids { + *approvals_per_item.entry(item_id.clone()).or_insert(0usize) += 1; + } + let max_approvals_for_one_item = approvals_per_item.values().copied().max().unwrap_or(0); + if max_approvals_for_one_item < min_approvals { + bail!( + "expected at least {min_approvals} approvals for one command item, got max {max_approvals_for_one_item} with map {approvals_per_item:?}" + ); + } + + let last_command_status = client.command_execution_statuses.last(); + if abort_on.is_none() { + if last_command_status != Some(&CommandExecutionStatus::Completed) { + bail!("expected completed command execution, got {last_command_status:?}"); + } + if client.last_turn_status != Some(TurnStatus::Completed) { + bail!( + "expected completed turn in all-accept flow, got {:?}", + client.last_turn_status + ); + } + } else if last_command_status == Some(&CommandExecutionStatus::Completed) { + bail!( + "expected non-completed command execution in mixed approval/decline flow, got {last_command_status:?}" + ); + } + + println!( + "[zsh-fork multi-approval summary] approvals={}, approvals_per_item={approvals_per_item:?}, command_statuses={:?}, turn_status={:?}", + client.command_approval_count, client.command_execution_statuses, client.last_turn_status + ); + + Ok(()) +} + fn resume_message_v2( endpoint: &Endpoint, config_overrides: &[String], @@ -791,6 +940,17 @@ enum ClientTransport { struct CodexClient { transport: ClientTransport, pending_notifications: VecDeque, + command_approval_behavior: CommandApprovalBehavior, + command_approval_count: usize, + command_approval_item_ids: Vec, + command_execution_statuses: Vec, + last_turn_status: Option, +} + +#[derive(Debug, Clone, Copy)] +enum CommandApprovalBehavior { + AlwaysAccept, + AbortOn(usize), } impl CodexClient { @@ -804,6 +964,14 @@ impl CodexClient { fn spawn_stdio(codex_bin: &Path, config_overrides: &[String]) -> Result { let codex_bin_display = codex_bin.display(); let mut cmd = Command::new(codex_bin); + if let Some(codex_bin_parent) = codex_bin.parent() { + let mut path = OsString::from(codex_bin_parent.as_os_str()); + if let Some(existing_path) = std::env::var_os("PATH") { + path.push(":"); + path.push(existing_path); + } + cmd.env("PATH", path); + } for override_kv in config_overrides { cmd.arg("--config").arg(override_kv); } @@ -831,6 +999,11 @@ impl CodexClient { stdout: BufReader::new(stdout), }, pending_notifications: VecDeque::new(), + command_approval_behavior: CommandApprovalBehavior::AlwaysAccept, + command_approval_count: 0, + command_approval_item_ids: Vec::new(), + command_execution_statuses: Vec::new(), + last_turn_status: None, }) } @@ -847,6 +1020,11 @@ impl CodexClient { socket: Box::new(socket), }, pending_notifications: VecDeque::new(), + command_approval_behavior: CommandApprovalBehavior::AlwaysAccept, + command_approval_count: 0, + command_approval_item_ids: Vec::new(), + command_execution_statuses: Vec::new(), + last_turn_status: None, }) } @@ -862,7 +1040,12 @@ impl CodexClient { }, capabilities: Some(InitializeCapabilities { experimental_api: true, - opt_out_notification_methods: None, + opt_out_notification_methods: Some( + NOTIFICATIONS_TO_OPT_OUT + .iter() + .map(|method| (*method).to_string()) + .collect(), + ), }), }, }; @@ -1121,10 +1304,14 @@ impl CodexClient { println!("\n< item started: {:?}", payload.item); } ServerNotification::ItemCompleted(payload) => { + if let ThreadItem::CommandExecution { status, .. } = payload.item.clone() { + self.command_execution_statuses.push(status); + } println!("< item completed: {:?}", payload.item); } ServerNotification::TurnCompleted(payload) => { if payload.turn.id == turn_id { + self.last_turn_status = Some(payload.turn.status.clone()); println!("\n< turn/completed notification: {:?}", payload.turn.status); if payload.turn.status == TurnStatus::Failed && let Some(error) = payload.turn.error @@ -1314,6 +1501,8 @@ impl CodexClient { "\n< commandExecution approval requested for thread {thread_id}, turn {turn_id}, item {item_id}, approval {}", approval_id.as_deref().unwrap_or("") ); + self.command_approval_count += 1; + self.command_approval_item_ids.push(item_id.clone()); if let Some(reason) = reason.as_deref() { println!("< reason: {reason}"); } @@ -1332,11 +1521,21 @@ impl CodexClient { println!("< proposed execpolicy amendment: {execpolicy_amendment:?}"); } + let decision = match self.command_approval_behavior { + CommandApprovalBehavior::AlwaysAccept => CommandExecutionApprovalDecision::Accept, + CommandApprovalBehavior::AbortOn(index) if self.command_approval_count == index => { + CommandExecutionApprovalDecision::Cancel + } + CommandApprovalBehavior::AbortOn(_) => CommandExecutionApprovalDecision::Accept, + }; let response = CommandExecutionRequestApprovalResponse { - decision: CommandExecutionApprovalDecision::Accept, + decision: decision.clone(), }; self.send_server_request_response(request_id, &response)?; - println!("< approved commandExecution request for item {item_id}"); + println!( + "< commandExecution decision for approval #{} on item {item_id}: {:?}", + self.command_approval_count, decision + ); Ok(()) } diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index 5c4e5eacc7..b4c5d098e4 100644 --- a/codex-rs/app-server/src/main.rs +++ b/codex-rs/app-server/src/main.rs @@ -23,6 +23,9 @@ struct AppServerArgs { } fn main() -> anyhow::Result<()> { + if codex_core::maybe_run_zsh_exec_wrapper_mode()? { + return Ok(()); + } arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { let args = AppServerArgs::parse(); let managed_config_path = managed_config_path_from_debug_env(); diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index 1eacc2a84e..e7705c295c 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -28,4 +28,5 @@ mod thread_start; mod thread_unarchive; mod turn_interrupt; mod turn_start; +mod turn_start_zsh_fork; mod turn_steer; diff --git a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs new file mode 100644 index 0000000000..4233ce6494 --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs @@ -0,0 +1,676 @@ +#![cfg(not(windows))] +// +// Running these tests with the patched zsh fork: +// +// The suite uses `CODEX_TEST_ZSH_PATH` when set. Example: +// CODEX_TEST_ZSH_PATH="$HOME/.local/codex-zsh-77045ef/bin/zsh" \ +// cargo test -p codex-app-server turn_start_zsh_fork -- --nocapture +// +// For a single test: +// CODEX_TEST_ZSH_PATH="$HOME/.local/codex-zsh-77045ef/bin/zsh" \ +// cargo test -p codex-app-server turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2 -- --nocapture + +use anyhow::Result; +use app_test_support::McpProcess; +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 codex_app_server_protocol::CommandExecutionApprovalDecision; +use codex_app_server_protocol::CommandExecutionRequestApprovalResponse; +use codex_app_server_protocol::CommandExecutionStatus; +use codex_app_server_protocol::ItemCompletedNotification; +use codex_app_server_protocol::ItemStartedNotification; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ServerRequest; +use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadStartParams; +use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnCompletedNotification; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStatus; +use codex_app_server_protocol::UserInput as V2UserInput; +use codex_core::features::FEATURES; +use codex_core::features::Feature; +use core_test_support::responses; +use core_test_support::skip_if_no_network; +use pretty_assertions::assert_eq; +use std::collections::BTreeMap; +use std::path::Path; +use tempfile::TempDir; +use tokio::time::timeout; + +#[cfg(windows)] +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15); +#[cfg(not(windows))] +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); + +#[tokio::test] +async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { + skip_if_no_network!(Ok(())); + + let tmp = TempDir::new()?; + let codex_home = tmp.path().join("codex_home"); + std::fs::create_dir(&codex_home)?; + let workspace = tmp.path().join("workspace"); + std::fs::create_dir(&workspace)?; + + let Some(zsh_path) = find_test_zsh_path() else { + eprintln!("skipping zsh fork test: no zsh executable found"); + return Ok(()); + }; + eprintln!("using zsh path for zsh-fork test: {}", zsh_path.display()); + + let responses = vec![create_shell_command_sse_response( + vec!["echo".to_string(), "hi".to_string()], + None, + Some(5000), + "call-zsh-fork", + )?]; + let server = create_mock_responses_server_sequence(responses).await; + create_config_toml( + &codex_home, + &server.uri(), + "never", + &BTreeMap::from([ + (Feature::ShellZshFork, true), + (Feature::UnifiedExec, false), + (Feature::ShellSnapshot, false), + ]), + &zsh_path, + )?; + + let mut mcp = McpProcess::new(&codex_home).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + cwd: Some(workspace.to_string_lossy().into_owned()), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id, + input: vec![V2UserInput::Text { + text: "run echo hi".to_string(), + text_elements: Vec::new(), + }], + cwd: Some(workspace.clone()), + approval_policy: Some(codex_app_server_protocol::AskForApproval::Never), + sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess), + model: Some("mock-model".to_string()), + effort: Some(codex_protocol::openai_models::ReasoningEffort::Medium), + summary: Some(codex_core::protocol_config_types::ReasoningSummary::Auto), + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + + let started_command_execution = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let started_notif = mcp + .read_stream_until_notification_message("item/started") + .await?; + let started: ItemStartedNotification = + serde_json::from_value(started_notif.params.clone().expect("item/started params"))?; + if let ThreadItem::CommandExecution { .. } = started.item { + return Ok::(started.item); + } + } + }) + .await??; + let ThreadItem::CommandExecution { + id, + status, + command, + cwd, + .. + } = started_command_execution + else { + unreachable!("loop ensures we break on command execution items"); + }; + assert_eq!(id, "call-zsh-fork"); + assert_eq!(status, CommandExecutionStatus::InProgress); + assert!(command.starts_with(&zsh_path.display().to_string())); + assert!(command.contains(" -lc 'echo hi'")); + assert_eq!(cwd, workspace); + + Ok(()) +} + +#[tokio::test] +async fn turn_start_shell_zsh_fork_exec_approval_decline_v2() -> Result<()> { + skip_if_no_network!(Ok(())); + + let tmp = TempDir::new()?; + let codex_home = tmp.path().join("codex_home"); + std::fs::create_dir(&codex_home)?; + let workspace = tmp.path().join("workspace"); + std::fs::create_dir(&workspace)?; + + let Some(zsh_path) = find_test_zsh_path() else { + eprintln!("skipping zsh fork decline test: no zsh executable found"); + return Ok(()); + }; + eprintln!("using zsh path for zsh-fork test: {}", zsh_path.display()); + + let responses = vec![ + create_shell_command_sse_response( + vec![ + "python3".to_string(), + "-c".to_string(), + "print(42)".to_string(), + ], + None, + Some(5000), + "call-zsh-fork-decline", + )?, + create_final_assistant_message_sse_response("done")?, + ]; + let server = create_mock_responses_server_sequence(responses).await; + create_config_toml( + &codex_home, + &server.uri(), + "untrusted", + &BTreeMap::from([ + (Feature::ShellZshFork, true), + (Feature::UnifiedExec, false), + (Feature::ShellSnapshot, false), + ]), + &zsh_path, + )?; + + let mut mcp = McpProcess::new(&codex_home).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + cwd: Some(workspace.to_string_lossy().into_owned()), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: "run python".to_string(), + text_elements: Vec::new(), + }], + cwd: Some(workspace.clone()), + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + + let server_req = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_request_message(), + ) + .await??; + let ServerRequest::CommandExecutionRequestApproval { request_id, params } = server_req else { + panic!("expected CommandExecutionRequestApproval request"); + }; + assert_eq!(params.item_id, "call-zsh-fork-decline"); + assert_eq!(params.thread_id, thread.id); + + mcp.send_response( + request_id, + serde_json::to_value(CommandExecutionRequestApprovalResponse { + decision: CommandExecutionApprovalDecision::Decline, + })?, + ) + .await?; + + let completed_command_execution = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let completed_notif = mcp + .read_stream_until_notification_message("item/completed") + .await?; + let completed: ItemCompletedNotification = serde_json::from_value( + completed_notif + .params + .clone() + .expect("item/completed params"), + )?; + if let ThreadItem::CommandExecution { .. } = completed.item { + return Ok::(completed.item); + } + } + }) + .await??; + let ThreadItem::CommandExecution { + id, + status, + exit_code, + aggregated_output, + .. + } = completed_command_execution + else { + unreachable!("loop ensures we break on command execution items"); + }; + assert_eq!(id, "call-zsh-fork-decline"); + assert_eq!(status, CommandExecutionStatus::Declined); + assert!(exit_code.is_none()); + assert!(aggregated_output.is_none()); + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("codex/event/task_complete"), + ) + .await??; + + Ok(()) +} + +#[tokio::test] +async fn turn_start_shell_zsh_fork_exec_approval_cancel_v2() -> Result<()> { + skip_if_no_network!(Ok(())); + + let tmp = TempDir::new()?; + let codex_home = tmp.path().join("codex_home"); + std::fs::create_dir(&codex_home)?; + let workspace = tmp.path().join("workspace"); + std::fs::create_dir(&workspace)?; + + let Some(zsh_path) = find_test_zsh_path() else { + eprintln!("skipping zsh fork cancel test: no zsh executable found"); + return Ok(()); + }; + eprintln!("using zsh path for zsh-fork test: {}", zsh_path.display()); + + let responses = vec![create_shell_command_sse_response( + vec![ + "python3".to_string(), + "-c".to_string(), + "print(42)".to_string(), + ], + None, + Some(5000), + "call-zsh-fork-cancel", + )?]; + let server = create_mock_responses_server_sequence(responses).await; + create_config_toml( + &codex_home, + &server.uri(), + "untrusted", + &BTreeMap::from([ + (Feature::ShellZshFork, true), + (Feature::UnifiedExec, false), + (Feature::ShellSnapshot, false), + ]), + &zsh_path, + )?; + + let mut mcp = McpProcess::new(&codex_home).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + cwd: Some(workspace.to_string_lossy().into_owned()), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: "run python".to_string(), + text_elements: Vec::new(), + }], + cwd: Some(workspace.clone()), + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + + let server_req = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_request_message(), + ) + .await??; + let ServerRequest::CommandExecutionRequestApproval { request_id, params } = server_req else { + panic!("expected CommandExecutionRequestApproval request"); + }; + assert_eq!(params.item_id, "call-zsh-fork-cancel"); + assert_eq!(params.thread_id, thread.id.clone()); + + mcp.send_response( + request_id, + serde_json::to_value(CommandExecutionRequestApprovalResponse { + decision: CommandExecutionApprovalDecision::Cancel, + })?, + ) + .await?; + + let completed_command_execution = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let completed_notif = mcp + .read_stream_until_notification_message("item/completed") + .await?; + let completed: ItemCompletedNotification = serde_json::from_value( + completed_notif + .params + .clone() + .expect("item/completed params"), + )?; + if let ThreadItem::CommandExecution { .. } = completed.item { + return Ok::(completed.item); + } + } + }) + .await??; + let ThreadItem::CommandExecution { id, status, .. } = completed_command_execution else { + unreachable!("loop ensures we break on command execution items"); + }; + assert_eq!(id, "call-zsh-fork-cancel"); + assert_eq!(status, CommandExecutionStatus::Declined); + + let completed_notif = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + let completed: TurnCompletedNotification = serde_json::from_value( + completed_notif + .params + .expect("turn/completed params must be present"), + )?; + assert_eq!(completed.thread_id, thread.id); + assert_eq!(completed.turn.status, TurnStatus::Interrupted); + + Ok(()) +} + +#[tokio::test] +async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() -> Result<()> { + skip_if_no_network!(Ok(())); + + let tmp = TempDir::new()?; + let codex_home = tmp.path().join("codex_home"); + std::fs::create_dir(&codex_home)?; + let workspace = tmp.path().join("workspace"); + std::fs::create_dir(&workspace)?; + + let Some(zsh_path) = find_test_zsh_path() else { + eprintln!("skipping zsh fork subcommand decline test: no zsh executable found"); + return Ok(()); + }; + if !supports_exec_wrapper_intercept(&zsh_path) { + eprintln!( + "skipping zsh fork subcommand decline test: zsh does not support EXEC_WRAPPER intercepts ({})", + zsh_path.display() + ); + return Ok(()); + } + eprintln!("using zsh path for zsh-fork test: {}", zsh_path.display()); + + let tool_call_arguments = serde_json::to_string(&serde_json::json!({ + "command": "/usr/bin/true && /usr/bin/true", + "workdir": serde_json::Value::Null, + "timeout_ms": 5000 + }))?; + let response = responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_function_call( + "call-zsh-fork-subcommand-decline", + "shell_command", + &tool_call_arguments, + ), + responses::ev_completed("resp-1"), + ]); + let server = create_mock_responses_server_sequence(vec![response]).await; + create_config_toml( + &codex_home, + &server.uri(), + "on-request", + &BTreeMap::from([ + (Feature::ShellZshFork, true), + (Feature::UnifiedExec, false), + (Feature::ShellSnapshot, false), + ]), + &zsh_path, + )?; + + let mut mcp = McpProcess::new(&codex_home).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + cwd: Some(workspace.to_string_lossy().into_owned()), + ..Default::default() + }) + .await?; + let start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(start_resp)?; + + let turn_id = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: "run true true".to_string(), + text_elements: Vec::new(), + }], + cwd: Some(workspace.clone()), + approval_policy: Some(codex_app_server_protocol::AskForApproval::OnRequest), + sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::ReadOnly { + access: codex_app_server_protocol::ReadOnlyAccess::FullAccess, + }), + model: Some("mock-model".to_string()), + effort: Some(codex_protocol::openai_models::ReasoningEffort::Medium), + summary: Some(codex_core::protocol_config_types::ReasoningSummary::Auto), + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), + ) + .await??; + + let mut approval_ids = Vec::new(); + for decision in [ + CommandExecutionApprovalDecision::Accept, + CommandExecutionApprovalDecision::Cancel, + ] { + let server_req = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_request_message(), + ) + .await??; + let ServerRequest::CommandExecutionRequestApproval { request_id, params } = server_req + else { + panic!("expected CommandExecutionRequestApproval request"); + }; + assert_eq!(params.item_id, "call-zsh-fork-subcommand-decline"); + approval_ids.push( + params + .approval_id + .clone() + .expect("approval_id must be present for zsh subcommand approvals"), + ); + assert_eq!(params.thread_id, thread.id); + mcp.send_response( + request_id, + serde_json::to_value(CommandExecutionRequestApprovalResponse { decision })?, + ) + .await?; + } + + let parent_completed_command_execution = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let completed_notif = mcp + .read_stream_until_notification_message("item/completed") + .await?; + let completed: ItemCompletedNotification = serde_json::from_value( + completed_notif + .params + .clone() + .expect("item/completed params"), + )?; + if let ThreadItem::CommandExecution { id, .. } = &completed.item + && id == "call-zsh-fork-subcommand-decline" + { + return Ok::(completed.item); + } + } + }) + .await??; + + let ThreadItem::CommandExecution { + id, + status, + aggregated_output, + .. + } = parent_completed_command_execution + else { + unreachable!("loop ensures we break on parent command execution item"); + }; + assert_eq!(id, "call-zsh-fork-subcommand-decline"); + assert_eq!(status, CommandExecutionStatus::Declined); + assert!( + aggregated_output.is_none() + || aggregated_output == Some("exec command rejected by user".to_string()) + ); + assert_eq!(approval_ids.len(), 2); + assert_ne!(approval_ids[0], approval_ids[1]); + + Ok(()) +} + +fn create_config_toml( + codex_home: &Path, + server_uri: &str, + approval_policy: &str, + feature_flags: &BTreeMap, + zsh_path: &Path, +) -> std::io::Result<()> { + let mut features = BTreeMap::from([(Feature::RemoteModels, false)]); + for (feature, enabled) in feature_flags { + features.insert(*feature, *enabled); + } + let feature_entries = features + .into_iter() + .map(|(feature, enabled)| { + let key = FEATURES + .iter() + .find(|spec| spec.id == feature) + .map(|spec| spec.key) + .unwrap_or_else(|| panic!("missing feature key for {feature:?}")); + format!("{key} = {enabled}") + }) + .collect::>() + .join("\n"); + let config_toml = codex_home.join("config.toml"); + std::fs::write( + config_toml, + format!( + r#" +model = "mock-model" +approval_policy = "{approval_policy}" +sandbox_mode = "read-only" +zsh_path = "{zsh_path}" + +model_provider = "mock_provider" + +[features] +{feature_entries} + +[model_providers.mock_provider] +name = "Mock provider for test" +base_url = "{server_uri}/v1" +wire_api = "responses" +request_max_retries = 0 +stream_max_retries = 0 +"#, + approval_policy = approval_policy, + zsh_path = zsh_path.display() + ), + ) +} + +fn find_test_zsh_path() -> Option { + if let Some(path) = std::env::var_os("CODEX_TEST_ZSH_PATH") { + let path = std::path::PathBuf::from(path); + if path.is_file() { + return Some(path); + } + panic!( + "CODEX_TEST_ZSH_PATH is set but is not a file: {}", + path.display() + ); + } + + for candidate in ["/bin/zsh", "/usr/bin/zsh"] { + let path = Path::new(candidate); + if path.is_file() { + return Some(path.to_path_buf()); + } + } + + let shell = std::env::var_os("SHELL")?; + let shell_path = std::path::PathBuf::from(shell); + if shell_path + .file_name() + .is_some_and(|file_name| file_name == "zsh") + && shell_path.is_file() + { + return Some(shell_path); + } + + None +} + +fn supports_exec_wrapper_intercept(zsh_path: &Path) -> bool { + let status = std::process::Command::new(zsh_path) + .arg("-fc") + .arg("/usr/bin/true") + .env("EXEC_WRAPPER", "/usr/bin/false") + .status(); + match status { + Ok(status) => !status.success(), + Err(_) => false, + } +} diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index cae322c77e..33c9f161bb 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -543,6 +543,9 @@ fn stage_str(stage: codex_core::features::Stage) -> &'static str { } fn main() -> anyhow::Result<()> { + if codex_core::maybe_run_zsh_exec_wrapper_mode()? { + return Ok(()); + } arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { cli_main(codex_linux_sandbox_exe).await?; Ok(()) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index e3f7456c95..bf2d970497 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -292,6 +292,9 @@ "shell_tool": { "type": "boolean" }, + "shell_zsh_fork": { + "type": "boolean" + }, "skill_env_var_dependency_prompt": { "type": "boolean" }, @@ -380,6 +383,14 @@ } ], "default": null + }, + "zsh_path": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution." } }, "type": "object" @@ -1432,6 +1443,9 @@ "shell_tool": { "type": "boolean" }, + "shell_zsh_fork": { + "type": "boolean" + }, "skill_env_var_dependency_prompt": { "type": "boolean" }, @@ -1788,6 +1802,14 @@ "windows_wsl_setup_acknowledged": { "description": "Tracks whether the Windows onboarding screen has been acknowledged.", "type": "boolean" + }, + "zsh_path": { + "allOf": [ + { + "$ref": "#/definitions/AbsolutePathBuf" + } + ], + "description": "Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution." } }, "title": "ConfigToml", diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 7744480ab4..b054423b6b 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -241,6 +241,7 @@ use crate::turn_diff_tracker::TurnDiffTracker; use crate::unified_exec::UnifiedExecProcessManager; use crate::util::backoff; use crate::windows_sandbox::WindowsSandboxLevelExt; +use crate::zsh_exec_bridge::ZshExecBridge; use codex_async_utils::OrCancelExt; use codex_otel::OtelManager; use codex_otel::TelemetryAuthMode; @@ -1191,7 +1192,22 @@ impl Session { config.active_profile.clone(), ); - let mut default_shell = shell::default_user_shell(); + let use_zsh_fork_shell = config.features.enabled(Feature::ShellZshFork); + let mut default_shell = if use_zsh_fork_shell { + let zsh_path = config.zsh_path.as_ref().ok_or_else(|| { + anyhow::anyhow!( + "zsh fork feature enabled, but `zsh_path` is not configured; set `zsh_path` in config.toml" + ) + })?; + shell::get_shell(shell::ShellType::Zsh, Some(zsh_path)).ok_or_else(|| { + anyhow::anyhow!( + "zsh fork feature enabled, but zsh_path `{}` is not usable; set `zsh_path` to a valid zsh executable", + zsh_path.display() + ) + })? + } else { + shell::default_user_shell() + }; // Create the mutable state for the Session. let shell_snapshot_tx = if config.features.enabled(Feature::ShellSnapshot) { ShellSnapshot::start_snapshotting( @@ -1262,10 +1278,17 @@ impl Session { (None, None) }; + let zsh_exec_bridge = + ZshExecBridge::new(config.zsh_path.clone(), config.codex_home.clone()); + zsh_exec_bridge + .initialize_for_session(&conversation_id.to_string()) + .await; + let services = SessionServices { mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::default(), + zsh_exec_bridge, analytics_events_client: AnalyticsEventsClient::new( Arc::clone(&config), Arc::clone(&auth_manager), @@ -3969,6 +3992,7 @@ mod handlers { .unified_exec_manager .terminate_all_processes() .await; + sess.services.zsh_exec_bridge.shutdown().await; info!("Shutting down Codex instance"); let history = sess.clone_history().await; let turn_count = history @@ -7201,6 +7225,81 @@ mod tests { } } + #[tokio::test] + async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { + let codex_home = tempfile::tempdir().expect("create temp dir"); + let mut config = build_test_config(codex_home.path()).await; + config.features.enable(Feature::ShellZshFork); + config.zsh_path = None; + let config = Arc::new(config); + + let auth_manager = + AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key")); + let models_manager = Arc::new(ModelsManager::new( + config.codex_home.clone(), + auth_manager.clone(), + )); + let model = ModelsManager::get_model_offline_for_tests(config.model.as_deref()); + let model_info = + ModelsManager::construct_model_info_offline_for_tests(model.as_str(), &config); + let collaboration_mode = CollaborationMode { + mode: ModeKind::Default, + settings: Settings { + model, + reasoning_effort: config.model_reasoning_effort, + developer_instructions: None, + }, + }; + let session_configuration = SessionConfiguration { + provider: config.model_provider.clone(), + collaboration_mode, + model_reasoning_summary: config.model_reasoning_summary, + developer_instructions: config.developer_instructions.clone(), + user_instructions: config.user_instructions.clone(), + personality: config.personality, + base_instructions: config + .base_instructions + .clone() + .unwrap_or_else(|| model_info.get_model_instructions(config.personality)), + compact_prompt: config.compact_prompt.clone(), + approval_policy: config.permissions.approval_policy.clone(), + sandbox_policy: config.permissions.sandbox_policy.clone(), + windows_sandbox_level: WindowsSandboxLevel::from_config(&config), + cwd: config.cwd.clone(), + codex_home: config.codex_home.clone(), + thread_name: None, + original_config_do_not_use: Arc::clone(&config), + session_source: SessionSource::Exec, + dynamic_tools: Vec::new(), + persist_extended_history: false, + }; + + let (tx_event, _rx_event) = async_channel::unbounded(); + let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); + let result = Session::new( + session_configuration, + Arc::clone(&config), + auth_manager, + models_manager, + ExecPolicyManager::default(), + tx_event, + agent_status_tx, + InitialHistory::New, + SessionSource::Exec, + Arc::new(SkillsManager::new(config.codex_home.clone())), + Arc::new(FileWatcher::noop()), + AgentControl::default(), + ) + .await; + + let err = match result { + Ok(_) => panic!("expected startup to fail"), + Err(err) => err, + }; + let msg = format!("{err:#}"); + assert!(msg.contains("zsh fork feature enabled, but `zsh_path` is not configured")); + } + // todo: use online model info pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { let (tx_event, _rx_event) = async_channel::unbounded(); @@ -7274,6 +7373,7 @@ mod tests { mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::default(), + zsh_exec_bridge: ZshExecBridge::default(), analytics_events_client: AnalyticsEventsClient::new( Arc::clone(&config), Arc::clone(&auth_manager), @@ -7422,6 +7522,7 @@ mod tests { mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::default(), + zsh_exec_bridge: ZshExecBridge::default(), analytics_events_client: AnalyticsEventsClient::new( Arc::clone(&config), Arc::clone(&auth_manager), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 9874cc2774..c2b753f51c 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -338,6 +338,8 @@ pub struct Config { /// Optional absolute path to the Node runtime used by `js_repl`. pub js_repl_node_path: Option, + /// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution. + pub zsh_path: Option, /// Value to use for `reasoning.effort` when making a request using the /// Responses API. @@ -977,6 +979,8 @@ pub struct ConfigToml { /// Optional absolute path to the Node runtime used by `js_repl`. pub js_repl_node_path: Option, + /// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution. + pub zsh_path: Option, /// Profile to use from the `profiles` map. pub profile: Option, @@ -1355,6 +1359,7 @@ pub struct ConfigOverrides { pub config_profile: Option, pub codex_linux_sandbox_exe: Option, pub js_repl_node_path: Option, + pub zsh_path: Option, pub base_instructions: Option, pub developer_instructions: Option, pub personality: Option, @@ -1482,6 +1487,7 @@ impl Config { config_profile: config_profile_key, codex_linux_sandbox_exe, js_repl_node_path: js_repl_node_path_override, + zsh_path: zsh_path_override, base_instructions, developer_instructions, personality, @@ -1742,6 +1748,9 @@ impl Config { let js_repl_node_path = js_repl_node_path_override .or(config_profile.js_repl_node_path.map(Into::into)) .or(cfg.js_repl_node_path.map(Into::into)); + let zsh_path = zsh_path_override + .or(config_profile.zsh_path.map(Into::into)) + .or(cfg.zsh_path.map(Into::into)); let review_model = override_review_model.or(cfg.review_model); @@ -1866,6 +1875,7 @@ impl Config { file_opener: cfg.file_opener.unwrap_or(UriBasedFileOpener::VsCode), codex_linux_sandbox_exe, js_repl_node_path, + zsh_path, hide_agent_reasoning: cfg.hide_agent_reasoning.unwrap_or(false), show_raw_agent_reasoning: cfg @@ -4196,6 +4206,7 @@ model_verbosity = "high" file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, js_repl_node_path: None, + zsh_path: None, hide_agent_reasoning: false, show_raw_agent_reasoning: false, model_reasoning_effort: Some(ReasoningEffort::High), @@ -4309,6 +4320,7 @@ model_verbosity = "high" file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, js_repl_node_path: None, + zsh_path: None, hide_agent_reasoning: false, show_raw_agent_reasoning: false, model_reasoning_effort: None, @@ -4420,6 +4432,7 @@ model_verbosity = "high" file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, js_repl_node_path: None, + zsh_path: None, hide_agent_reasoning: false, show_raw_agent_reasoning: false, model_reasoning_effort: None, @@ -4517,6 +4530,7 @@ model_verbosity = "high" file_opener: UriBasedFileOpener::VsCode, codex_linux_sandbox_exe: None, js_repl_node_path: None, + zsh_path: None, hide_agent_reasoning: false, show_raw_agent_reasoning: false, model_reasoning_effort: Some(ReasoningEffort::High), diff --git a/codex-rs/core/src/config/profile.rs b/codex-rs/core/src/config/profile.rs index ff343cbf53..9523313372 100644 --- a/codex-rs/core/src/config/profile.rs +++ b/codex-rs/core/src/config/profile.rs @@ -30,6 +30,8 @@ pub struct ConfigProfile { pub chatgpt_base_url: Option, /// Optional path to a file containing model instructions. pub model_instructions_file: Option, + /// Optional absolute path to patched zsh used by zsh-exec-bridge-backed shell execution. + pub zsh_path: Option, pub js_repl_node_path: Option, /// Deprecated: ignored. Use `model_instructions_file`. #[schemars(skip)] diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 0f0f0609a2..4e1220b87b 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -75,7 +75,7 @@ pub struct ExecParams { } /// Mechanism to terminate an exec invocation before it finishes naturally. -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum ExecExpiration { Timeout(Duration), DefaultTimeout, @@ -97,7 +97,7 @@ impl From for ExecExpiration { } impl ExecExpiration { - async fn wait(self) { + pub(crate) async fn wait(self) { match self { ExecExpiration::Timeout(duration) => tokio::time::sleep(duration).await, ExecExpiration::DefaultTimeout => { diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 49363641bb..86cacae5b6 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -84,6 +84,8 @@ pub enum Feature { JsReplToolsOnly, /// Use the single unified PTY-backed exec tool. UnifiedExec, + /// Route shell tool execution through the zsh exec bridge. + ShellZshFork, /// Include the freeform apply_patch tool. ApplyPatchFreeform, /// Allow the model to request web searches that fetch live content. @@ -431,6 +433,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: !cfg!(windows), }, + FeatureSpec { + id: Feature::ShellZshFork, + key: "shell_zsh_fork", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::ShellSnapshot, key: "shell_snapshot", diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index bbfef7f8fa..ccfd340dfe 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -109,6 +109,7 @@ pub mod terminal; mod tools; pub mod turn_diff_tracker; mod turn_metadata; +mod zsh_exec_bridge; pub use rollout::ARCHIVED_SESSIONS_SUBDIR; pub use rollout::INTERACTIVE_SESSION_SOURCES; pub use rollout::RolloutRecorder; @@ -152,6 +153,7 @@ pub use file_watcher::FileWatcherEvent; pub use safety::get_platform_sandbox; pub use tools::spec::parse_tool_input_schema; pub use turn_metadata::build_turn_metadata_header; +pub use zsh_exec_bridge::maybe_run_zsh_exec_wrapper_mode; // Re-export the protocol types from the standalone `codex-protocol` crate so existing // `codex_core::protocol::...` references continue to work across the workspace. pub use codex_protocol::protocol; diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 487278b619..4bf8f66092 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -164,12 +164,19 @@ impl SandboxManager { SandboxType::MacosSeatbelt => { let mut seatbelt_env = HashMap::new(); seatbelt_env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); + let zsh_exec_bridge_wrapper_socket = env + .get(crate::zsh_exec_bridge::ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR) + .map(PathBuf::from); + let zsh_exec_bridge_allowed_unix_sockets = zsh_exec_bridge_wrapper_socket + .as_ref() + .map_or_else(Vec::new, |path| vec![path.clone()]); let mut args = create_seatbelt_command_args( command.clone(), policy, sandbox_policy_cwd, enforce_managed_network, network, + &zsh_exec_bridge_allowed_unix_sockets, ); let mut full_command = Vec::with_capacity(1 + args.len()); full_command.push(MACOS_PATH_TO_SEATBELT_EXECUTABLE.to_string()); diff --git a/codex-rs/core/src/seatbelt.rs b/codex-rs/core/src/seatbelt.rs index 7f738797b8..6ae1b061af 100644 --- a/codex-rs/core/src/seatbelt.rs +++ b/codex-rs/core/src/seatbelt.rs @@ -39,8 +39,14 @@ pub async fn spawn_command_under_seatbelt( network: Option<&NetworkProxy>, mut env: HashMap, ) -> std::io::Result { - let args = - create_seatbelt_command_args(command, sandbox_policy, sandbox_policy_cwd, false, network); + let args = create_seatbelt_command_args( + command, + sandbox_policy, + sandbox_policy_cwd, + false, + network, + &[], + ); let arg0 = None; env.insert(CODEX_SANDBOX_ENV_VAR.to_string(), "seatbelt".to_string()); spawn_child_async(SpawnChildRequest { @@ -181,6 +187,7 @@ pub(crate) fn create_seatbelt_command_args( sandbox_policy_cwd: &Path, enforce_managed_network: bool, network: Option<&NetworkProxy>, + allowed_unix_socket_paths: &[PathBuf], ) -> Vec { create_seatbelt_command_args_with_extensions( command, @@ -189,6 +196,7 @@ pub(crate) fn create_seatbelt_command_args( enforce_managed_network, network, None, + allowed_unix_socket_paths, ) } @@ -199,6 +207,7 @@ pub(crate) fn create_seatbelt_command_args_with_extensions( enforce_managed_network: bool, network: Option<&NetworkProxy>, extensions: Option<&MacOsSeatbeltProfileExtensions>, + allowed_unix_socket_paths: &[PathBuf], ) -> Vec { let (file_write_policy, file_write_dir_params) = { if sandbox_policy.has_full_disk_write_access() { @@ -295,6 +304,8 @@ pub(crate) fn create_seatbelt_command_args_with_extensions( let proxy = proxy_policy_inputs(network); let network_policy = dynamic_network_policy(sandbox_policy, enforce_managed_network, &proxy); + let (unix_socket_policy, unix_socket_params) = + unix_socket_policy_and_params(allowed_unix_socket_paths); let seatbelt_extensions = extensions.map_or_else( || { // Backward-compatibility default when no extension profile is provided. @@ -303,20 +314,24 @@ pub(crate) fn create_seatbelt_command_args_with_extensions( build_seatbelt_extensions, ); - let full_policy = if seatbelt_extensions.policy.is_empty() { - format!( - "{MACOS_SEATBELT_BASE_POLICY}\n{file_read_policy}\n{file_write_policy}\n{network_policy}" - ) - } else { - format!( - "{MACOS_SEATBELT_BASE_POLICY}\n{file_read_policy}\n{file_write_policy}\n{network_policy}\n{}", - seatbelt_extensions.policy - ) - }; + let mut policy_sections = vec![ + MACOS_SEATBELT_BASE_POLICY.to_string(), + file_read_policy, + file_write_policy, + network_policy, + ]; + if !unix_socket_policy.is_empty() { + policy_sections.push(unix_socket_policy); + } + if !seatbelt_extensions.policy.is_empty() { + policy_sections.push(seatbelt_extensions.policy.clone()); + } + let full_policy = policy_sections.join("\n"); let dir_params = [ file_read_dir_params, file_write_dir_params, + unix_socket_params, macos_dir_params(), seatbelt_extensions.dir_params, ] @@ -332,6 +347,27 @@ pub(crate) fn create_seatbelt_command_args_with_extensions( seatbelt_args } +fn unix_socket_policy_and_params( + allowed_unix_socket_paths: &[PathBuf], +) -> (String, Vec<(String, PathBuf)>) { + if allowed_unix_socket_paths.is_empty() { + return (String::new(), Vec::new()); + } + + let mut policy = String::from("; allow outbound connect to explicitly-approved unix sockets\n"); + let mut params = Vec::with_capacity(allowed_unix_socket_paths.len()); + + for (index, path) in allowed_unix_socket_paths.iter().enumerate() { + let key = format!("ALLOWED_UNIX_SOCKET_{index}"); + policy.push_str(&format!( + "(allow network-outbound (remote unix-socket (path (param \"{key}\"))))\n" + )); + params.push((key, path.clone())); + } + + (policy, params) +} + /// Wraps libc::confstr to return a String. fn confstr(name: libc::c_int) -> Option { let mut buf = vec![0_i8; (libc::PATH_MAX as usize) + 1]; @@ -451,6 +487,7 @@ mod tests { macos_accessibility: true, macos_calendar: true, }), + &[], ); let policy = &args[1]; @@ -469,6 +506,7 @@ mod tests { cwd.as_path(), false, None, + &[], ); let policy = &args[1]; assert!(policy.contains("(allow user-preference-read)")); @@ -485,6 +523,7 @@ mod tests { false, None, Some(&MacOsSeatbeltProfileExtensions::default()), + &[], ); let policy = &args[1]; assert!(!policy.contains("appleevent-send")); @@ -494,6 +533,32 @@ mod tests { assert!(!policy.contains("user-preference-write")); } + #[test] + fn seatbelt_args_allow_explicit_wrapper_unix_socket_path() { + let cwd = std::env::temp_dir(); + let socket_path = std::env::temp_dir().join("codex-zsh-wrapper-test.sock"); + let args = create_seatbelt_command_args( + vec!["echo".to_string(), "ok".to_string()], + &SandboxPolicy::new_read_only_policy(), + cwd.as_path(), + false, + None, + std::slice::from_ref(&socket_path), + ); + let policy = &args[1]; + let param_key = "ALLOWED_UNIX_SOCKET_0"; + + assert!( + policy.contains("(allow network-outbound (remote unix-socket (path (param \"ALLOWED_UNIX_SOCKET_0\"))))"), + "policy should contain explicit unix-socket allow rule:\n{policy}" + ); + assert!( + args.iter() + .any(|arg| arg == &format!("-D{param_key}={}", socket_path.to_string_lossy())), + "args should include unix-socket path param" + ); + } + #[test] fn create_seatbelt_args_allows_local_binding_when_explicitly_enabled() { let policy = dynamic_network_policy( @@ -649,7 +714,8 @@ mod tests { .iter() .map(std::string::ToString::to_string) .collect(); - let args = create_seatbelt_command_args(shell_command.clone(), &policy, &cwd, false, None); + let args = + create_seatbelt_command_args(shell_command.clone(), &policy, &cwd, false, None, &[]); // Build the expected policy text using a raw string for readability. // Note that the policy includes: @@ -746,7 +812,7 @@ mod tests { .map(std::string::ToString::to_string) .collect(); let write_hooks_file_args = - create_seatbelt_command_args(shell_command_git, &policy, &cwd, false, None); + create_seatbelt_command_args(shell_command_git, &policy, &cwd, false, None, &[]); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&write_hooks_file_args) .current_dir(&cwd) @@ -777,7 +843,7 @@ mod tests { .map(std::string::ToString::to_string) .collect(); let write_allowed_file_args = - create_seatbelt_command_args(shell_command_allowed, &policy, &cwd, false, None); + create_seatbelt_command_args(shell_command_allowed, &policy, &cwd, false, None, &[]); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&write_allowed_file_args) .current_dir(&cwd) @@ -838,7 +904,7 @@ mod tests { .iter() .map(std::string::ToString::to_string) .collect(); - let args = create_seatbelt_command_args(shell_command, &policy, &cwd, false, None); + let args = create_seatbelt_command_args(shell_command, &policy, &cwd, false, None, &[]); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&args) @@ -869,7 +935,7 @@ mod tests { .map(std::string::ToString::to_string) .collect(); let gitdir_args = - create_seatbelt_command_args(shell_command_gitdir, &policy, &cwd, false, None); + create_seatbelt_command_args(shell_command_gitdir, &policy, &cwd, false, None, &[]); let output = Command::new(MACOS_PATH_TO_SEATBELT_EXECUTABLE) .args(&gitdir_args) .current_dir(&cwd) @@ -932,6 +998,7 @@ mod tests { vulnerable_root.as_path(), false, None, + &[], ); let tmpdir_env_var = std::env::var("TMPDIR") diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 6f640131f6..fe82ec84c0 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -15,6 +15,7 @@ use crate::state_db::StateDbHandle; use crate::tools::network_approval::NetworkApprovalService; use crate::tools::sandboxing::ApprovalStore; use crate::unified_exec::UnifiedExecProcessManager; +use crate::zsh_exec_bridge::ZshExecBridge; use codex_hooks::Hooks; use codex_otel::OtelManager; use tokio::sync::Mutex; @@ -26,6 +27,7 @@ pub(crate) struct SessionServices { pub(crate) mcp_connection_manager: Arc>, pub(crate) mcp_startup_cancellation_token: Mutex, pub(crate) unified_exec_manager: UnifiedExecProcessManager, + pub(crate) zsh_exec_bridge: ZshExecBridge, pub(crate) analytics_events_client: AnalyticsEventsClient, pub(crate) hooks: Hooks, pub(crate) rollout: Mutex>, diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index a277e48a58..6f1e4b44af 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -26,6 +26,7 @@ use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::with_cached_approval; +use crate::zsh_exec_bridge::ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR; use codex_network_proxy::NetworkProxy; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; @@ -182,6 +183,36 @@ impl ToolRuntime for ShellRuntime { command }; + if ctx.session.features().enabled(Feature::ShellZshFork) { + let wrapper_socket_path = ctx + .session + .services + .zsh_exec_bridge + .next_wrapper_socket_path(); + let mut zsh_fork_env = req.env.clone(); + zsh_fork_env.insert( + ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR.to_string(), + wrapper_socket_path.to_string_lossy().to_string(), + ); + let spec = build_command_spec( + &command, + &req.cwd, + &zsh_fork_env, + req.timeout_ms.into(), + req.sandbox_permissions, + req.justification.clone(), + )?; + let env = attempt + .env_for(spec, req.network.as_ref()) + .map_err(|err| ToolError::Codex(err.into()))?; + return ctx + .session + .services + .zsh_exec_bridge + .execute_shell_request(&env, ctx.session, ctx.turn, &ctx.call_id) + .await; + } + let spec = build_command_spec( &command, &req.cwd, diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 6ea7edd00f..ca49c50c34 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -69,6 +69,8 @@ impl ToolsConfig { let shell_type = if !features.enabled(Feature::ShellTool) { ConfigShellToolType::Disabled + } else if features.enabled(Feature::ShellZshFork) { + ConfigShellToolType::ShellCommand } else if features.enabled(Feature::UnifiedExec) { // If ConPTY not supported (for old Windows versions), fallback on ShellCommand. if codex_utils_pty::conpty_supported() { @@ -2346,6 +2348,23 @@ mod tests { assert_contains_tool_names(&tools, &subset); } + #[test] + fn shell_zsh_fork_prefers_shell_command_over_unified_exec() { + let config = test_config(); + let model_info = ModelsManager::construct_model_info_offline_for_tests("o3", &config); + let mut features = Features::with_defaults(); + features.enable(Feature::UnifiedExec); + features.enable(Feature::ShellZshFork); + + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + features: &features, + web_search_mode: Some(WebSearchMode::Live), + }); + + assert_eq!(tools_config.shell_type, ConfigShellToolType::ShellCommand); + } + #[test] #[ignore] fn test_parallel_support_flags() { diff --git a/codex-rs/core/src/zsh_exec_bridge/mod.rs b/codex-rs/core/src/zsh_exec_bridge/mod.rs new file mode 100644 index 0000000000..2e3821acfe --- /dev/null +++ b/codex-rs/core/src/zsh_exec_bridge/mod.rs @@ -0,0 +1,554 @@ +use crate::exec::ExecToolCallOutput; +use crate::tools::sandboxing::ToolError; +use std::path::PathBuf; +use tokio::sync::Mutex; +use uuid::Uuid; + +#[cfg(unix)] +use crate::error::CodexErr; +#[cfg(unix)] +use crate::error::SandboxErr; +#[cfg(unix)] +use crate::protocol::EventMsg; +#[cfg(unix)] +use crate::protocol::ExecCommandOutputDeltaEvent; +#[cfg(unix)] +use crate::protocol::ExecOutputStream; +#[cfg(unix)] +use crate::protocol::ReviewDecision; +#[cfg(unix)] +use anyhow::Context as _; +#[cfg(unix)] +use codex_protocol::approvals::ExecPolicyAmendment; +#[cfg(unix)] +use codex_utils_pty::process_group::kill_child_process_group; +#[cfg(unix)] +use serde::Deserialize; +#[cfg(unix)] +use serde::Serialize; +#[cfg(unix)] +use std::io::Read; +#[cfg(unix)] +use std::io::Write; +#[cfg(unix)] +use std::time::Instant; +#[cfg(unix)] +use tokio::io::AsyncReadExt; +#[cfg(unix)] +use tokio::net::UnixListener; +#[cfg(unix)] +use tokio::net::UnixStream; + +pub(crate) const ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR: &str = + "CODEX_ZSH_EXEC_BRIDGE_WRAPPER_SOCKET"; +pub(crate) const ZSH_EXEC_WRAPPER_MODE_ENV_VAR: &str = "CODEX_ZSH_EXEC_WRAPPER_MODE"; +#[cfg(unix)] +pub(crate) const EXEC_WRAPPER_ENV_VAR: &str = "EXEC_WRAPPER"; + +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub(crate) struct ZshExecBridgeSessionState { + pub(crate) initialized_session_id: Option, +} + +#[derive(Debug, Default)] +pub(crate) struct ZshExecBridge { + zsh_path: Option, + state: Mutex, +} + +#[cfg(unix)] +#[derive(Debug, Deserialize, Serialize)] +#[serde(tag = "type", rename_all = "snake_case")] +enum WrapperIpcRequest { + ExecRequest { + request_id: String, + file: String, + argv: Vec, + cwd: String, + }, +} + +#[cfg(unix)] +#[derive(Debug, Deserialize, Serialize)] +#[serde(tag = "type", rename_all = "snake_case")] +enum WrapperIpcResponse { + ExecResponse { + request_id: String, + action: WrapperExecAction, + reason: Option, + }, +} + +#[cfg(unix)] +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +enum WrapperExecAction { + Run, + Deny, +} + +impl ZshExecBridge { + pub(crate) fn new(zsh_path: Option, _codex_home: PathBuf) -> Self { + Self { + zsh_path, + state: Mutex::new(ZshExecBridgeSessionState::default()), + } + } + + pub(crate) async fn initialize_for_session(&self, session_id: &str) { + let mut state = self.state.lock().await; + state.initialized_session_id = Some(session_id.to_string()); + } + + pub(crate) async fn shutdown(&self) { + let mut state = self.state.lock().await; + state.initialized_session_id = None; + } + + pub(crate) fn next_wrapper_socket_path(&self) -> PathBuf { + let socket_id = Uuid::new_v4().as_simple().to_string(); + let temp_dir = std::env::temp_dir(); + let canonical_temp_dir = temp_dir.canonicalize().unwrap_or(temp_dir); + canonical_temp_dir.join(format!("czs-{}.sock", &socket_id[..12])) + } + + #[cfg(not(unix))] + pub(crate) async fn execute_shell_request( + &self, + _req: &crate::sandboxing::ExecRequest, + _session: &crate::codex::Session, + _turn: &crate::codex::TurnContext, + _call_id: &str, + ) -> Result { + let _ = &self.zsh_path; + Err(ToolError::Rejected( + "shell_zsh_fork is only supported on unix".to_string(), + )) + } + + #[cfg(unix)] + pub(crate) async fn execute_shell_request( + &self, + req: &crate::sandboxing::ExecRequest, + session: &crate::codex::Session, + turn: &crate::codex::TurnContext, + call_id: &str, + ) -> Result { + let zsh_path = self.zsh_path.clone().ok_or_else(|| { + ToolError::Rejected( + "shell_zsh_fork enabled, but zsh_path is not configured".to_string(), + ) + })?; + + let command = req.command.clone(); + if command.is_empty() { + return Err(ToolError::Rejected("command args are empty".to_string())); + } + + let wrapper_socket_path = req + .env + .get(ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR) + .map(PathBuf::from) + .unwrap_or_else(|| self.next_wrapper_socket_path()); + + let listener = { + let _ = std::fs::remove_file(&wrapper_socket_path); + UnixListener::bind(&wrapper_socket_path).map_err(|err| { + ToolError::Rejected(format!( + "bind wrapper socket at {}: {err}", + wrapper_socket_path.display() + )) + })? + }; + + let wrapper_path = std::env::current_exe().map_err(|err| { + ToolError::Rejected(format!("resolve current executable path: {err}")) + })?; + + let mut cmd = tokio::process::Command::new(&command[0]); + if command.len() > 1 { + cmd.args(&command[1..]); + } + cmd.current_dir(&req.cwd); + cmd.stdin(std::process::Stdio::null()); + cmd.stdout(std::process::Stdio::piped()); + cmd.stderr(std::process::Stdio::piped()); + cmd.kill_on_drop(true); + cmd.env_clear(); + cmd.envs(&req.env); + cmd.env( + ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR, + wrapper_socket_path.to_string_lossy().to_string(), + ); + cmd.env(EXEC_WRAPPER_ENV_VAR, &wrapper_path); + cmd.env(ZSH_EXEC_WRAPPER_MODE_ENV_VAR, "1"); + + let mut child = cmd.spawn().map_err(|err| { + ToolError::Rejected(format!( + "failed to start zsh fork command {} with zsh_path {}: {err}", + command[0], + zsh_path.display() + )) + })?; + + let (stream_tx, mut stream_rx) = + tokio::sync::mpsc::unbounded_channel::<(ExecOutputStream, Vec)>(); + + if let Some(mut out) = child.stdout.take() { + let tx = stream_tx.clone(); + tokio::spawn(async move { + let mut buf = [0_u8; 8192]; + loop { + let read = match out.read(&mut buf).await { + Ok(0) => break, + Ok(n) => n, + Err(err) => { + tracing::warn!("zsh fork stdout read error: {err}"); + break; + } + }; + let _ = tx.send((ExecOutputStream::Stdout, buf[..read].to_vec())); + } + }); + } + + if let Some(mut err) = child.stderr.take() { + let tx = stream_tx.clone(); + tokio::spawn(async move { + let mut buf = [0_u8; 8192]; + loop { + let read = match err.read(&mut buf).await { + Ok(0) => break, + Ok(n) => n, + Err(err) => { + tracing::warn!("zsh fork stderr read error: {err}"); + break; + } + }; + let _ = tx.send((ExecOutputStream::Stderr, buf[..read].to_vec())); + } + }); + } + drop(stream_tx); + + let mut stdout_bytes = Vec::new(); + let mut stderr_bytes = Vec::new(); + let mut child_exit = None; + let mut timed_out = false; + let mut stream_open = true; + let mut user_rejected = false; + let start = Instant::now(); + + let expiration = req.expiration.clone().wait(); + tokio::pin!(expiration); + + while child_exit.is_none() || stream_open { + tokio::select! { + result = child.wait(), if child_exit.is_none() => { + child_exit = Some(result.map_err(|err| ToolError::Rejected(format!("wait for zsh fork command exit: {err}")))?); + } + stream = stream_rx.recv(), if stream_open => { + if let Some((output_stream, chunk)) = stream { + match output_stream { + ExecOutputStream::Stdout => stdout_bytes.extend_from_slice(&chunk), + ExecOutputStream::Stderr => stderr_bytes.extend_from_slice(&chunk), + } + session + .send_event( + turn, + EventMsg::ExecCommandOutputDelta(ExecCommandOutputDeltaEvent { + call_id: call_id.to_string(), + stream: output_stream, + chunk, + }), + ) + .await; + } else { + stream_open = false; + } + } + accept_result = listener.accept(), if child_exit.is_none() => { + let (stream, _) = accept_result.map_err(|err| { + ToolError::Rejected(format!("failed to accept wrapper request: {err}")) + })?; + if self + .handle_wrapper_request(stream, req.justification.clone(), session, turn, call_id) + .await? + { + user_rejected = true; + } + } + _ = &mut expiration, if child_exit.is_none() => { + timed_out = true; + kill_child_process_group(&mut child).map_err(|err| { + ToolError::Rejected(format!("kill zsh fork command process group: {err}")) + })?; + child.start_kill().map_err(|err| { + ToolError::Rejected(format!("kill zsh fork command process: {err}")) + })?; + } + } + } + + let _ = std::fs::remove_file(&wrapper_socket_path); + + let status = child_exit.ok_or_else(|| { + ToolError::Rejected("zsh fork command did not return exit status".to_string()) + })?; + + if user_rejected { + return Err(ToolError::Rejected("rejected by user".to_string())); + } + + let stdout_text = crate::text_encoding::bytes_to_string_smart(&stdout_bytes); + let stderr_text = crate::text_encoding::bytes_to_string_smart(&stderr_bytes); + let output = ExecToolCallOutput { + exit_code: status.code().unwrap_or(-1), + stdout: crate::exec::StreamOutput::new(stdout_text.clone()), + stderr: crate::exec::StreamOutput::new(stderr_text.clone()), + aggregated_output: crate::exec::StreamOutput::new(format!( + "{stdout_text}{stderr_text}" + )), + duration: start.elapsed(), + timed_out, + }; + + Self::map_exec_result(req.sandbox, output) + } + + #[cfg(unix)] + async fn handle_wrapper_request( + &self, + mut stream: UnixStream, + approval_reason: Option, + session: &crate::codex::Session, + turn: &crate::codex::TurnContext, + call_id: &str, + ) -> Result { + let mut request_buf = Vec::new(); + stream.read_to_end(&mut request_buf).await.map_err(|err| { + ToolError::Rejected(format!("read wrapper request from socket: {err}")) + })?; + let request_line = String::from_utf8(request_buf).map_err(|err| { + ToolError::Rejected(format!("decode wrapper request as utf-8: {err}")) + })?; + let request = parse_wrapper_request_line(request_line.trim())?; + + let (request_id, file, argv, cwd) = match request { + WrapperIpcRequest::ExecRequest { + request_id, + file, + argv, + cwd, + } => (request_id, file, argv, cwd), + }; + + let command_for_approval = if argv.is_empty() { + vec![file.clone()] + } else { + argv.clone() + }; + + let approval_id = Uuid::new_v4().to_string(); + let decision = session + .request_command_approval( + turn, + call_id.to_string(), + Some(approval_id), + command_for_approval, + PathBuf::from(cwd), + approval_reason, + None, + None::, + ) + .await; + + let (action, reason, user_rejected) = match decision { + ReviewDecision::Approved + | ReviewDecision::ApprovedForSession + | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { + (WrapperExecAction::Run, None, false) + } + ReviewDecision::Denied => ( + WrapperExecAction::Deny, + Some("command denied by host approval policy".to_string()), + true, + ), + ReviewDecision::Abort => ( + WrapperExecAction::Deny, + Some("command aborted by host approval policy".to_string()), + true, + ), + }; + + write_json_line( + &mut stream, + &WrapperIpcResponse::ExecResponse { + request_id, + action, + reason, + }, + ) + .await?; + + Ok(user_rejected) + } + + #[cfg(unix)] + fn map_exec_result( + sandbox: crate::exec::SandboxType, + output: ExecToolCallOutput, + ) -> Result { + if output.timed_out { + return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { + output: Box::new(output), + }))); + } + + if crate::exec::is_likely_sandbox_denied(sandbox, &output) { + return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(output), + network_policy_decision: None, + }))); + } + + Ok(output) + } +} + +pub fn maybe_run_zsh_exec_wrapper_mode() -> anyhow::Result { + if std::env::var_os(ZSH_EXEC_WRAPPER_MODE_ENV_VAR).is_none() { + return Ok(false); + } + + run_exec_wrapper_mode()?; + Ok(true) +} + +fn run_exec_wrapper_mode() -> anyhow::Result<()> { + #[cfg(not(unix))] + { + anyhow::bail!("zsh exec wrapper mode is only supported on unix"); + } + + #[cfg(unix)] + { + use std::os::unix::net::UnixStream as StdUnixStream; + + let args: Vec = std::env::args().collect(); + if args.len() < 2 { + anyhow::bail!("exec wrapper mode requires target executable path"); + } + let file = args[1].clone(); + let argv = if args.len() > 2 { + args[2..].to_vec() + } else { + vec![file.clone()] + }; + let cwd = std::env::current_dir() + .context("resolve wrapper cwd")? + .to_string_lossy() + .to_string(); + let socket_path = std::env::var(ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR) + .context("missing wrapper socket path env var")?; + + let request_id = Uuid::new_v4().to_string(); + let request = WrapperIpcRequest::ExecRequest { + request_id: request_id.clone(), + file: file.clone(), + argv: argv.clone(), + cwd, + }; + + let mut stream = StdUnixStream::connect(&socket_path) + .with_context(|| format!("connect to wrapper socket at {socket_path}"))?; + let encoded = serde_json::to_string(&request).context("serialize wrapper request")?; + stream + .write_all(encoded.as_bytes()) + .context("write wrapper request")?; + stream + .write_all(b"\n") + .context("write wrapper request newline")?; + stream + .shutdown(std::net::Shutdown::Write) + .context("shutdown wrapper write")?; + + let mut response_buf = String::new(); + stream + .read_to_string(&mut response_buf) + .context("read wrapper response")?; + let response: WrapperIpcResponse = + serde_json::from_str(response_buf.trim()).context("parse wrapper response")?; + + let (response_request_id, action, reason) = match response { + WrapperIpcResponse::ExecResponse { + request_id, + action, + reason, + } => (request_id, action, reason), + }; + if response_request_id != request_id { + anyhow::bail!( + "wrapper response request_id mismatch: expected {request_id}, got {response_request_id}" + ); + } + + if action == WrapperExecAction::Deny { + if let Some(reason) = reason { + tracing::warn!("execution denied: {reason}"); + } else { + tracing::warn!("execution denied"); + } + std::process::exit(1); + } + + let mut command = std::process::Command::new(&file); + if argv.len() > 1 { + command.args(&argv[1..]); + } + command.env_remove(ZSH_EXEC_WRAPPER_MODE_ENV_VAR); + command.env_remove(ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR); + command.env_remove(EXEC_WRAPPER_ENV_VAR); + let status = command.status().context("spawn wrapped executable")?; + std::process::exit(status.code().unwrap_or(1)); + } +} + +#[cfg(unix)] +fn parse_wrapper_request_line(request_line: &str) -> Result { + serde_json::from_str(request_line) + .map_err(|err| ToolError::Rejected(format!("parse wrapper request payload: {err}"))) +} + +#[cfg(unix)] +async fn write_json_line( + writer: &mut W, + message: &T, +) -> Result<(), ToolError> { + let encoded = serde_json::to_string(message) + .map_err(|err| ToolError::Rejected(format!("serialize wrapper message: {err}")))?; + tokio::io::AsyncWriteExt::write_all(writer, encoded.as_bytes()) + .await + .map_err(|err| ToolError::Rejected(format!("write wrapper message: {err}")))?; + tokio::io::AsyncWriteExt::write_all(writer, b"\n") + .await + .map_err(|err| ToolError::Rejected(format!("write wrapper newline: {err}")))?; + tokio::io::AsyncWriteExt::flush(writer) + .await + .map_err(|err| ToolError::Rejected(format!("flush wrapper message: {err}")))?; + Ok(()) +} + +#[cfg(all(test, unix))] +mod tests { + use super::*; + + #[test] + fn parse_wrapper_request_line_rejects_malformed_json() { + let err = parse_wrapper_request_line("this-is-not-json").unwrap_err(); + let ToolError::Rejected(message) = err else { + panic!("expected ToolError::Rejected"); + }; + assert!(message.starts_with("parse wrapper request payload:")); + } +} diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 49163c900b..1533e7e637 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -252,6 +252,7 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any model_provider: model_provider.clone(), codex_linux_sandbox_exe, js_repl_node_path: None, + zsh_path: None, base_instructions: None, developer_instructions: None, personality: None,