diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 03f86cedf0..c3724aee0a 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -494,6 +494,10 @@ impl ModelClient { let auth_env_telemetry = collect_auth_env_telemetry(model_provider.info(), codex_api_key_env_enabled); let include_attestation = model_provider.supports_attestation(); + // Reviewers use their own request-level effort even when managed requirements + // pin the parent's feature on. Share this decision with update injection and pinning. + let reasoning_effort_override_enabled = reasoning_effort_override_enabled + && !crate::guardian::is_basic_session_source(&session_source); Self { state: Arc::new(ModelClientState { thread_id, @@ -524,6 +528,10 @@ impl ModelClient { } } + pub(crate) fn reasoning_effort_override_enabled(&self) -> bool { + self.state.reasoning_effort_override_enabled + } + pub(crate) fn with_restored_history(mut self, restored_history: bool) -> Self { self.restored_history = restored_history; self diff --git a/codex-rs/core/src/session/reasoning_effort.rs b/codex-rs/core/src/session/reasoning_effort.rs index 5ff84dc2d1..3ff9e4049f 100644 --- a/codex-rs/core/src/session/reasoning_effort.rs +++ b/codex-rs/core/src/session/reasoning_effort.rs @@ -3,12 +3,12 @@ //! Only trusted harness items establish overrides. Replay preserves startup prewarm's //! baseline while it is retained. //! Successful compaction retires the overrides and allows a fresh request baseline. +//! Synchronous Guardian reviewers always use their selected request-level effort. use super::session::Session; use super::step_context::StepContext; use super::step_settings::ResolvedStepSettings; use crate::state::ReasoningEffortPin; -use codex_features::Feature; use codex_history::CodexHarnessMetadata; use codex_history::ResponseItemEnvelope; use codex_protocol::models::ConfigurationReasoning; @@ -96,7 +96,11 @@ impl Session { usage: RequestEffortUsage, ) -> Option { let selected_effort = settings.reasoning_effort().cloned(); - if !self.enabled(Feature::ReasoningEffortOverride) { + if !self + .services + .model_client + .reasoning_effort_override_enabled() + { return selected_effort; } if usage == RequestEffortUsage::Compaction @@ -130,7 +134,10 @@ impl Session { &self, settings: &ResolvedStepSettings, ) -> Option { - if !self.enabled(Feature::ReasoningEffortOverride) + if !self + .services + .model_client + .reasoning_effort_override_enabled() || !settings.model_info.use_responses_lite || !self.provider().await.is_openai() { diff --git a/codex-rs/core/src/session/reasoning_effort_tests.rs b/codex-rs/core/src/session/reasoning_effort_tests.rs index 3ce2739c65..2b8ce76afc 100644 --- a/codex-rs/core/src/session/reasoning_effort_tests.rs +++ b/codex-rs/core/src/session/reasoning_effort_tests.rs @@ -2,10 +2,11 @@ use super::RequestEffortUsage; use crate::session::step_settings::ResolvedStepSettings; -use crate::session::tests::make_session_and_context; +use crate::session::tests::make_session_and_context_with_auth_and_config_and_rx; use codex_features::Feature; use codex_history::InitialHistory; use codex_history::ResumedHistory; +use codex_login::CodexAuth; use codex_protocol::ThreadId; use codex_protocol::openai_models::ReasoningEffort; use pretty_assertions::assert_eq; @@ -20,11 +21,17 @@ use test_case::test_case; }); "resume")] #[tokio::test] async fn initial_replay_preserves_prewarmed_effort(history: InitialHistory) { - let (mut session, turn_context) = make_session_and_context().await; - session - .features - .enable(Feature::ReasoningEffortOverride) - .unwrap(); + let (session, turn_context, _events) = make_session_and_context_with_auth_and_config_and_rx( + CodexAuth::from_api_key("Test API Key"), + Vec::new(), + |config| { + config + .features + .enable(Feature::ReasoningEffortOverride) + .unwrap(); + }, + ) + .await; let mut model_info = Arc::clone(&turn_context.initial_settings.model_info); Arc::make_mut(&mut model_info).use_responses_lite = true; let mut selected = turn_context.initial_settings.selected().clone(); @@ -59,11 +66,17 @@ async fn initial_replay_preserves_prewarmed_effort(history: InitialHistory) { #[tokio::test] async fn compaction_effort_lookup_preserves_pin_for_fallback_models() { - let (mut session, turn_context) = make_session_and_context().await; - session - .features - .enable(Feature::ReasoningEffortOverride) - .unwrap(); + let (session, turn_context, _events) = make_session_and_context_with_auth_and_config_and_rx( + CodexAuth::from_api_key("Test API Key"), + Vec::new(), + |config| { + config + .features + .enable(Feature::ReasoningEffortOverride) + .unwrap(); + }, + ) + .await; session .state .lock() diff --git a/codex-rs/core/tests/suite/reasoning_effort_override.rs b/codex-rs/core/tests/suite/reasoning_effort_override.rs index 50dc637b5d..0099d94d28 100644 --- a/codex-rs/core/tests/suite/reasoning_effort_override.rs +++ b/codex-rs/core/tests/suite/reasoning_effort_override.rs @@ -1,8 +1,10 @@ //! Trusted reasoning-effort updates follow surviving history and the next turn's selected settings. +use codex_config::test_support::CloudConfigBundleFixture; use codex_core::ForkSnapshot; use codex_core::RecoverTurnRequest; use codex_core::StartIfIdleSubmission; +use codex_core::StartThreadOptions; use codex_core::SuspendTurnOutcome; use codex_core::TurnInput; use codex_core::TurnInputRequest; @@ -16,8 +18,12 @@ use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::InternalSessionSource; use codex_protocol::protocol::Op; +use codex_protocol::protocol::SessionSource; +use codex_protocol::protocol::SubAgentSource; use codex_protocol::protocol::ThreadSettingsOverrides; +use codex_protocol::protocol::ThreadSource; use codex_protocol::user_input::UserInput; use core_test_support::responses; use core_test_support::responses::ResponsesRequest; @@ -70,6 +76,126 @@ fn message(role: &str, text: &str) -> Value { }) } +#[test_case(SessionSource::Internal(InternalSessionSource::Guardian); "internal guardian")] +#[test_case(SessionSource::SubAgent(SubAgentSource::Other("guardian".to_string())); "legacy guardian")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn sync_guardian_ignores_managed_reasoning_override( + session_source: SessionSource, +) -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + let server = responses::start_mock_server().await; + let mut mocks = Vec::new(); + for id in [ + "parent-medium", + "parent-high", + "reviewer-low", + "reviewer-high", + ] { + mocks.push( + responses::mount_sse_once(&server, responses::sse(vec![responses::ev_completed(id)])) + .await, + ); + } + let mut test = override_builder() + .with_cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement( + "[features]\nreasoning_effort_override = true\n", + ), + ) + .with_config(|config| config.model_provider.supports_websockets = false) + .build_with_auto_env(&server) + .await?; + test.submit_text_turn("first parent turn").await?; + submit_thread_settings( + &test.codex, + ThreadSettingsOverrides { + effort: Some(Some(ReasoningEffort::High)), + ..Default::default() + }, + ) + .await?; + test.submit_text_turn("second parent turn").await?; + let parent = Arc::clone(&test.codex); + parent.shutdown_and_wait().await?; + + // Fork real parent history, including its trusted effort updates, into a reviewer. + let config = test.config.clone(); + assert!(config.features.enabled(Feature::ReasoningEffortOverride)); + let mut options = StartThreadOptions::new(config); + options.session_source = Some(session_source); + options.thread_source = Some(ThreadSource::GuardianReview); + let forked = test + .thread_manager + .fork_thread( + ForkSnapshot::Interrupted, + options, + parent.rollout_path().expect("parent rollout path"), + ) + .await?; + test.codex = forked.thread; + test.session_configured = forked.session_configured; + for effort in [ReasoningEffort::Low, ReasoningEffort::High] { + submit_thread_settings( + &test.codex, + ThreadSettingsOverrides { + effort: Some(Some(effort)), + ..Default::default() + }, + ) + .await?; + test.submit_text_turn("review an action").await?; + } + test.codex.shutdown_and_wait().await?; + + let requests = mocks + .iter() + .map(responses::ResponseMock::single_request) + .collect::>(); + assert_eq!( + requests + .iter() + .map(|request| request.body_json()["reasoning"]["effort"].clone()) + .collect::>(), + vec![ + Value::from("medium"), + Value::from("medium"), + Value::from("low"), + Value::from("high"), + ], + ); + let inherited_updates = vec![ + effort_update(ReasoningEffort::Medium), + effort_update(ReasoningEffort::High), + ]; + assert_eq!( + requests.iter().map(effort_updates).collect::>(), + vec![ + vec![effort_update(ReasoningEffort::Medium)], + inherited_updates.clone(), + vec![], + vec![], + ], + ); + // Reviewers neither append new updates nor erase inherited durable history. + for thread in [&parent, &test.codex] { + let saved_updates = thread + .load_history(/*include_archived*/ false) + .await? + .items + .into_iter() + .filter_map(|item| match item { + RolloutItem::ResponseItem(ResponseItemEnvelope { + item: item @ ResponseItem::ConfigurationUpdate { .. }, + .. + }) => Some(serde_json::to_value(item).expect("serialize saved update")), + _ => None, + }) + .collect::>(); + assert_eq!(saved_updates, inherited_updates); + } + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn reasoning_effort_override_recovery_reuses_trusted_tail_update() -> anyhow::Result<()> { skip_if_no_network!(Ok(()));