From 57182b5653d359f794e2c9be3832b3da9e2cdabf Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 21 Apr 2026 14:51:58 -0700 Subject: [PATCH] Polish sticky environment API wiring Group thread-start options for lint-friendly callsites and update generated v2 schema for sticky environment selections. Co-authored-by: Codex --- .../schema/json/v2/ThreadStartParams.json | 19 +++++++ .../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/thread_manager.rs | 52 +++++++++++-------- 5 files changed, 65 insertions(+), 36 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json index 9b1cba6d1a..e40fe65cc7 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json @@ -1,6 +1,10 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "AbsolutePathBuf": { + "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", + "type": "string" + }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `guardian_subagent` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request.", "enum": [ @@ -114,6 +118,21 @@ "clear" ], "type": "string" + }, + "TurnEnvironmentParams": { + "properties": { + "cwd": { + "$ref": "#/definitions/AbsolutePathBuf" + }, + "environmentId": { + "type": "string" + } + }, + "required": [ + "cwd", + "environmentId" + ], + "type": "object" } }, "properties": { diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 8ff4de3e76..b99725423c 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; @@ -2613,20 +2614,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/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 8c74cd3506..6b765fc57f 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>`. @@ -494,38 +504,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