From 8a7c854bffcb93c36512884f50f4abf5539fca32 Mon Sep 17 00:00:00 2001 From: jif Date: Thu, 16 Jul 2026 18:03:28 +0000 Subject: [PATCH] Validate reasoning effort after applying spawn roles (#33656) ## Why Agent roles can override the model or reasoning effort after the initial `spawn_agent` configuration is validated, leaving an unsupported combination unchecked. ## What changed - Validate the final reasoning effort against model metadata whenever a role changes the spawned agent's model settings. - Preserve independently configured subagent defaults when a role does not override them. - Apply the same role validation in both multi-agent spawn implementations. ## Testing Added coverage for independent model and reasoning defaults, instruction-only roles, and rejection of a reasoning effort unsupported by a role-selected model. GitOrigin-RevId: 1885314e9380f778603f067fc04a6225ee963428 --- .../src/tools/handlers/multi_agents/spawn.rs | 5 +- .../src/tools/handlers/multi_agents_common.rs | 41 +++++ .../tools/handlers/multi_agents_v2/spawn.rs | 5 +- .../tests/suite/subagent_notifications.rs | 156 +++++++++++++++++- 4 files changed, 198 insertions(+), 9 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index 24983cb0834a..a0cbee7f4c7b 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -5,7 +5,6 @@ use crate::agent::control::render_input_preview; use crate::agent::exceeds_thread_spawn_depth_limit; use crate::agent::next_thread_spawn_depth; use crate::agent::role::DEFAULT_ROLE_NAME; -use crate::agent::role::apply_role_to_config; use crate::tools::handlers::multi_agents_spec::SpawnAgentToolOptions; use crate::tools::handlers::multi_agents_spec::create_spawn_agent_tool_v1; use codex_tools::ToolSpec; @@ -103,9 +102,7 @@ async fn handle_spawn_agent( ) .await?; if !args.fork_context { - apply_role_to_config(&mut config, role_name) - .await - .map_err(FunctionCallError::RespondToModel)?; + apply_spawn_agent_role(&session, &mut config, role_name).await?; } apply_spawn_agent_service_tier( &session, diff --git a/codex-rs/core/src/tools/handlers/multi_agents_common.rs b/codex-rs/core/src/tools/handlers/multi_agents_common.rs index 7c6f2f349d1b..352fc5ba5ac4 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_common.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_common.rs @@ -1,3 +1,4 @@ +use crate::agent::role::apply_role_to_config; use crate::config::Config; use crate::config::DEFAULT_MULTI_AGENT_V2_MIN_WAIT_TIMEOUT_MS; use crate::config::HARD_MAX_MULTI_AGENT_V2_TIMEOUT_MS; @@ -353,6 +354,46 @@ pub(crate) async fn apply_spawn_agent_service_tier( Ok(()) } +pub(crate) async fn apply_spawn_agent_role( + session: &Session, + config: &mut Config, + role_name: Option<&str>, +) -> Result<(), FunctionCallError> { + let previous_model = config.model.clone(); + let previous_reasoning_effort = config.model_reasoning_effort.clone(); + apply_role_to_config(config, role_name) + .await + .map_err(FunctionCallError::RespondToModel)?; + if config.model == previous_model && config.model_reasoning_effort == previous_reasoning_effort + { + return Ok(()); + } + + let Some(reasoning_effort) = config.model_reasoning_effort.clone() else { + return Ok(()); + }; + let model = config.model.clone().ok_or_else(|| { + FunctionCallError::RespondToModel( + "spawn_agent could not resolve the child model for reasoning effort validation" + .to_string(), + ) + })?; + let model_info = session + .services + .models_manager + .get_model_info(&model, &config.to_models_manager_config()) + .await; + if model_info.used_fallback_model_metadata { + return Ok(()); + } + + validate_spawn_agent_reasoning_effort( + &model, + &model_info.supported_reasoning_levels, + &reasoning_effort, + ) +} + fn find_spawn_agent_model_name( available_models: &[ModelPreset], requested_model: &str, diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 2db21f27fe00..67f8057e435b 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -3,7 +3,6 @@ use crate::agent::control::SpawnAgentForkMode; use crate::agent::control::SpawnAgentOptions; use crate::agent::next_thread_spawn_depth; use crate::agent::role::DEFAULT_ROLE_NAME; -use crate::agent::role::apply_role_to_config; use crate::agent_communication::AgentCommunicationContext; use crate::agent_communication::AgentCommunicationKind; use crate::tools::handlers::multi_agents_spec::SpawnAgentToolOptions; @@ -77,9 +76,7 @@ async fn handle_spawn_agent( ) .await?; if !is_full_history_fork { - apply_role_to_config(&mut config, role_name) - .await - .map_err(FunctionCallError::RespondToModel)?; + apply_spawn_agent_role(&session, &mut config, role_name).await?; } apply_spawn_agent_service_tier( &session, diff --git a/codex-rs/core/tests/suite/subagent_notifications.rs b/codex-rs/core/tests/suite/subagent_notifications.rs index ef62ade72fbc..cf42a7c2f539 100644 --- a/codex-rs/core/tests/suite/subagent_notifications.rs +++ b/codex-rs/core/tests/suite/subagent_notifications.rs @@ -471,7 +471,7 @@ async fn setup_turn_one_with_custom_spawned_child( config.model = Some(INHERITED_MODEL.to_string()); config.model_reasoning_effort = Some(INHERITED_REASONING_EFFORT); })); - let test = builder.build(server).await?; + let test = builder.build_with_auto_env(server).await?; test.submit_turn(TURN_1_PROMPT).await?; if child_response_delay.is_none() && wait_for_parent_notification { let _ = wait_for_requests(&child_request_log).await?; @@ -1162,6 +1162,47 @@ async fn spawn_agent_uses_configured_subagent_defaults() -> Result<()> { Ok(()) } +#[test_case( + Some(REQUESTED_MODEL), + None, + REQUESTED_MODEL, + Some(ReasoningEffort::Medium); + "model only" +)] +#[test_case( + None, + Some(REQUESTED_REASONING_EFFORT), + INHERITED_MODEL, + Some(REQUESTED_REASONING_EFFORT); + "reasoning effort only" +)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn spawn_agent_uses_independent_configured_subagent_defaults( + default_model: Option<&str>, + default_reasoning_effort: Option, + expected_model: &str, + expected_reasoning_effort: Option, +) -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let default_model = default_model.map(str::to_string); + let child_snapshot = + spawn_child_and_capture_snapshot(&server, json!({ "message": CHILD_PROMPT }), |builder| { + builder.with_config(move |config| { + config.agent_default_subagent_model = default_model; + config.agent_default_subagent_reasoning_effort = default_reasoning_effort; + }) + }) + .await?; + + assert_eq!( + (child_snapshot.model, child_snapshot.reasoning_effort), + (expected_model.to_string(), expected_reasoning_effort) + ); + Ok(()) +} + #[test_case(true, false; "unsupported child")] #[test_case(false, true; "supported child")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -1719,6 +1760,119 @@ async fn spawn_agent_role_overrides_requested_model_and_reasoning_settings() -> Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn spawn_agent_preserves_configured_defaults_through_unrelated_role() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let child_snapshot = spawn_child_and_capture_snapshot( + &server, + json!({ + "message": CHILD_PROMPT, + "agent_type": "custom", + }), + |builder| { + builder.with_config(|config| { + let role_path = config.codex_home.join("instructions-only-role.toml"); + std::fs::write(&role_path, "developer_instructions = \"Stay focused\"\n") + .expect("write role config"); + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: Some("Custom role".to_string()), + config_file: Some(role_path.to_path_buf()), + nickname_candidates: None, + }, + ); + config.agent_default_subagent_model = Some(REQUESTED_MODEL.to_string()); + config.agent_default_subagent_reasoning_effort = Some(REQUESTED_REASONING_EFFORT); + }) + }, + ) + .await?; + + assert_eq!( + (child_snapshot.model, child_snapshot.reasoning_effort), + ( + REQUESTED_MODEL.to_string(), + Some(REQUESTED_REASONING_EFFORT) + ) + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn spawn_agent_rejects_reasoning_effort_unsupported_by_role_model() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let spawn_args = serde_json::to_string(&json!({ + "message": CHILD_PROMPT, + "agent_type": "custom", + }))?; + mount_sse_once_match( + &server, + |request: &wiremock::Request| body_contains(request, TURN_1_PROMPT), + sse(vec![ + ev_response_created("resp-turn1-1"), + ev_function_call_with_namespace( + SPAWN_CALL_ID, + MULTI_AGENT_V1_NAMESPACE, + "spawn_agent", + &spawn_args, + ), + ev_completed("resp-turn1-1"), + ]), + ) + .await; + let tool_output = mount_sse_once_match( + &server, + |request: &wiremock::Request| body_contains(request, SPAWN_CALL_ID), + sse(vec![ + ev_response_created("resp-turn1-2"), + ev_completed("resp-turn1-2"), + ]), + ) + .await; + + let test = test_codex() + .with_config(|config| { + config + .features + .enable(Feature::Collab) + .expect("test config should allow feature update"); + let role_path = config.codex_home.join("model-only-role.toml"); + std::fs::write(&role_path, format!("model = \"{ROLE_MODEL}\"\n")) + .expect("write role config"); + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: Some("Custom role".to_string()), + config_file: Some(role_path.to_path_buf()), + nickname_candidates: None, + }, + ); + config.agent_default_subagent_model = Some("gpt-5.6-sol".to_string()); + config.agent_default_subagent_reasoning_effort = Some(ReasoningEffort::Ultra); + }) + .build_with_auto_env(&server) + .await?; + + test.submit_turn(TURN_1_PROMPT).await?; + + let (output, _) = tool_output + .single_request() + .function_call_output_content_and_success(SPAWN_CALL_ID) + .expect("spawn_agent output"); + assert_eq!( + output.as_deref(), + Some( + "Reasoning effort `ultra` is not supported for model `gpt-5.4`. Supported reasoning efforts: low, medium, high, xhigh" + ) + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn spawn_agent_tool_description_mentions_role_locked_settings() -> Result<()> { skip_if_no_network!(Ok(()));