From 15b5eb30ed09ce7fb3a4b85f44f447180e66a2fe Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Wed, 19 Nov 2025 03:32:48 -0800 Subject: [PATCH] fix(core) Support changing /approvals before conversation (#6836) ## Summary Setting `/approvals` before the start of a conversation was not updating the environment_context for a conversation. Not sure exactly when this problem was introduced, but this should reduce model confusion dramatically. ## Testing - [x] Added unit test to reproduce bug, confirmed fix with update - [x] Tested locally --- codex-rs/core/src/codex.rs | 5 +- codex-rs/core/tests/suite/prompt_caching.rs | 84 +++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index a68ea2fd4..e5b4e4c31 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1335,7 +1335,10 @@ impl Session { } async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiver) { - let mut previous_context: Option> = None; + // Seed with context in case there is an OverrideTurnContext first. + let mut previous_context: Option> = + Some(sess.new_turn(SessionSettingsUpdate::default()).await); + // To break out of this loop, send Op::Shutdown. while let Ok(sub) = rx_sub.recv().await { debug!(?sub, "Submission"); diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index e3bd64238..55e8f6641 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -3,6 +3,7 @@ use codex_core::features::Feature; use codex_core::model_family::find_family_for_model; use codex_core::protocol::AskForApproval; +use codex_core::protocol::ENVIRONMENT_CONTEXT_OPEN_TAG; use codex_core::protocol::EventMsg; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; @@ -372,6 +373,89 @@ async fn overrides_turn_context_but_keeps_cached_prefix_and_key_constant() -> an Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn override_before_first_turn_emits_environment_context() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let req = mount_sse_once(&server, sse_completed("resp-1")).await; + + let TestCodex { codex, .. } = test_codex().build(&server).await?; + + codex + .submit(Op::OverrideTurnContext { + cwd: None, + approval_policy: Some(AskForApproval::Never), + sandbox_policy: None, + model: None, + effort: None, + summary: None, + }) + .await?; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "first message".into(), + }], + }) + .await?; + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; + + let body = req.single_request().body_json(); + let input = body["input"] + .as_array() + .expect("input array must be present"); + assert!( + !input.is_empty(), + "expected at least environment context and user message" + ); + + let env_msg = &input[1]; + let env_text = env_msg["content"][0]["text"] + .as_str() + .expect("environment context text"); + assert!( + env_text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG), + "second entry should be environment context, got: {env_text}" + ); + assert!( + env_text.contains("never"), + "environment context should reflect overridden approval policy: {env_text}" + ); + + let env_count = input + .iter() + .filter(|msg| { + msg["content"] + .as_array() + .and_then(|content| { + content.iter().find(|item| { + item["type"].as_str() == Some("input_text") + && item["text"] + .as_str() + .map(|text| text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG)) + .unwrap_or(false) + }) + }) + .is_some() + }) + .count(); + assert_eq!( + env_count, 2, + "environment context should appear exactly twice, found {env_count}" + ); + + let user_msg = &input[2]; + let user_text = user_msg["content"][0]["text"] + .as_str() + .expect("user message text"); + assert_eq!(user_text, "first message"); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn per_turn_overrides_keep_cached_prefix_and_key_constant() -> anyhow::Result<()> { skip_if_no_network!(Ok(()));