From b3a8571219d9ed35c95bd5ff2eb5b42baf40a131 Mon Sep 17 00:00:00 2001 From: Shijie Rao Date: Tue, 17 Feb 2026 17:50:06 -0800 Subject: [PATCH 1/3] 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/3] 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/3] 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