From ce5fca8b23114037cc709fd1cfb7ba2139de5471 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Mon, 20 Apr 2026 15:38:53 -0700 Subject: [PATCH] Fix sticky environment lint failures Group thread-start options for clippy and make the new environment tests comply with stricter CI lint settings. Co-authored-by: Codex --- .../app-server/src/codex_message_processor.rs | 13 ++--- .../app-server/tests/suite/v2/turn_start.rs | 16 +++--- codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/session/tests.rs | 14 ++++- codex-rs/core/src/thread_manager.rs | 52 +++++++++++-------- 5 files changed, 58 insertions(+), 38 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index d0ae05e92d..2ad411dd53 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -218,6 +218,7 @@ use codex_core::ForkSnapshot; use codex_core::NewThread; use codex_core::RolloutRecorder; use codex_core::SessionMeta; +use codex_core::StartThreadWithToolsOptions; use codex_core::SteerInputError; use codex_core::ThreadConfigSnapshot; use codex_core::ThreadManager; @@ -2612,20 +2613,20 @@ impl CodexMessageProcessor { match listener_task_context .thread_manager - .start_thread_with_tools_and_service_name( + .start_thread_with_tools_and_service_name(StartThreadWithToolsOptions { config, - match session_start_source + initial_history: match session_start_source .unwrap_or(codex_app_server_protocol::ThreadStartSource::Startup) { codex_app_server_protocol::ThreadStartSource::Startup => InitialHistory::New, codex_app_server_protocol::ThreadStartSource::Clear => InitialHistory::Cleared, }, - core_dynamic_tools, + dynamic_tools: core_dynamic_tools, persist_extended_history, - service_name, - request_trace, + metrics_service_name: service_name, + parent_trace: request_trace, environment_selections, - ) + }) .instrument(tracing::info_span!( "app_server.thread_start.create_thread", otel.name = "app_server.thread_start.create_thread", diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index c86227b920..5235dc75cc 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -1969,8 +1969,11 @@ async fn run_environment_selection_case( mcp.read_stream_until_notification_message("turn/started"), ) .await??; - let started: TurnStartedNotification = - serde_json::from_value(started_notification.params.expect("turn/started params"))?; + let started: TurnStartedNotification = serde_json::from_value( + started_notification + .params + .ok_or_else(|| anyhow::anyhow!("turn/started notification should include params"))?, + )?; assert_eq!(started.turn.id, turn.id, "{}", case.name); let completed_notification = timeout( @@ -1978,11 +1981,10 @@ async fn run_environment_selection_case( mcp.read_stream_until_notification_message("turn/completed"), ) .await??; - let completed: TurnCompletedNotification = serde_json::from_value( - completed_notification - .params - .expect("turn/completed params"), - )?; + let completed: TurnCompletedNotification = + serde_json::from_value(completed_notification.params.ok_or_else(|| { + anyhow::anyhow!("turn/completed notification should include params") + })?)?; assert_eq!(completed.turn.id, turn.id, "{}", case.name); assert_eq!( completed.turn.status, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index cf61c5faf9..4e87120d95 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -118,6 +118,7 @@ pub(crate) mod web_search; pub(crate) mod windows_sandbox_read_grants; pub use thread_manager::ForkSnapshot; pub use thread_manager::NewThread; +pub use thread_manager::StartThreadWithToolsOptions; pub use thread_manager::ThreadManager; pub use thread_manager::build_models_manager; pub use web_search::web_search_action_detail; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index cad35fde42..fb08564fee 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4137,7 +4137,12 @@ async fn multi_environment_feature_without_selections_uses_single_environment_be let (_session, turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx( CodexAuth::from_api_key("Test API Key"), Vec::new(), - |config| config.features.enable(Feature::MultiEnvironmentTools), + |config| { + config + .features + .enable(Feature::MultiEnvironmentTools) + .expect("multi-environment tools feature should enable"); + }, ) .await; @@ -4162,7 +4167,12 @@ async fn multi_environment_tools_enable_only_with_feature_and_selection() { let (session, _turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx( CodexAuth::from_api_key("Test API Key"), Vec::new(), - |config| config.features.enable(Feature::MultiEnvironmentTools), + |config| { + config + .features + .enable(Feature::MultiEnvironmentTools) + .expect("multi-environment tools feature should enable"); + }, ) .await; let selected_cwd = diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index fcf77ba15e..36ce6222bc 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -198,6 +198,16 @@ pub struct ThreadManager { _test_codex_home_guard: Option, } +pub struct StartThreadWithToolsOptions { + pub config: Config, + pub initial_history: InitialHistory, + pub dynamic_tools: Vec, + pub persist_extended_history: bool, + pub metrics_service_name: Option, + pub parent_trace: Option, + pub environment_selections: Option>, +} + /// Shared, `Arc`-owned state for [`ThreadManager`]. This `Arc` is required to have a single /// `Arc` reference that can be downgraded to by `AgentControl` while preventing every single /// function to require an `Arc<&Self>`. @@ -496,38 +506,34 @@ impl ThreadManager { dynamic_tools: Vec, persist_extended_history: bool, ) -> CodexResult { - Box::pin(self.start_thread_with_tools_and_service_name( - config, - InitialHistory::New, - dynamic_tools, - persist_extended_history, - /*metrics_service_name*/ None, - /*parent_trace*/ None, - /*environment_selections*/ None, - )) + Box::pin( + self.start_thread_with_tools_and_service_name(StartThreadWithToolsOptions { + config, + initial_history: InitialHistory::New, + dynamic_tools, + persist_extended_history, + metrics_service_name: None, + parent_trace: None, + environment_selections: None, + }), + ) .await } pub async fn start_thread_with_tools_and_service_name( &self, - config: Config, - initial_history: InitialHistory, - dynamic_tools: Vec, - persist_extended_history: bool, - metrics_service_name: Option, - parent_trace: Option, - environment_selections: Option>, + options: StartThreadWithToolsOptions, ) -> CodexResult { Box::pin(self.state.spawn_thread( - config, - initial_history, + options.config, + options.initial_history, Arc::clone(&self.state.auth_manager), self.agent_control(), - dynamic_tools, - persist_extended_history, - metrics_service_name, - parent_trace, - environment_selections, + options.dynamic_tools, + options.persist_extended_history, + options.metrics_service_name, + options.parent_trace, + options.environment_selections, /*user_shell_override*/ None, )) .await