From 62f75665942fa6d9d3a965ae004e57b24400bb09 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 11 May 2026 18:58:23 +0300 Subject: [PATCH 1/5] Honor role-defined spawn service tiers --- codex-rs/core/src/agent/role.rs | 68 ++++--- codex-rs/core/src/agent/role_tests.rs | 59 ++++++ .../src/tools/handlers/multi_agents/spawn.rs | 8 +- .../src/tools/handlers/multi_agents_common.rs | 8 +- .../src/tools/handlers/multi_agents_tests.rs | 168 ++++++++++++++++++ .../tools/handlers/multi_agents_v2/spawn.rs | 8 +- 6 files changed, 289 insertions(+), 30 deletions(-) diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index 2ab16cd22a25..e613d8443277 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -29,8 +29,16 @@ use toml::Value as TomlValue; pub const DEFAULT_ROLE_NAME: &str = "default"; const AGENT_TYPE_UNAVAILABLE_ERROR: &str = "agent type is currently not available"; +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub(crate) struct AppliedRoleConfig { + pub(crate) service_tier: Option, +} + /// Applies a named role layer to `config` while preserving caller-owned model selection. /// +/// The returned settings capture role-owned values that spawn orchestration must keep locked after +/// the config reload completes. +/// /// The role layer is inserted at session-flag precedence so it can override persisted config, but /// the caller's current `profile` and `model_provider` remain sticky runtime choices unless the /// role explicitly sets `profile`, explicitly sets `model_provider`, or rewrites the active @@ -40,7 +48,7 @@ const AGENT_TYPE_UNAVAILABLE_ERROR: &str = "agent type is currently not availabl pub(crate) async fn apply_role_to_config( config: &mut Config, role_name: Option<&str>, -) -> Result<(), String> { +) -> Result { let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME); let role = resolve_role_config(config, role_name) @@ -59,18 +67,19 @@ async fn apply_role_to_config_inner( config: &mut Config, role_name: &str, role: &AgentRoleConfig, -) -> anyhow::Result<()> { +) -> anyhow::Result { let is_built_in = !config.agent_roles.contains_key(role_name); let Some(config_file) = role.config_file.as_ref() else { - return Ok(()); + return Ok(AppliedRoleConfig::default()); }; let role_layer_toml = load_role_layer_toml(config, config_file, is_built_in, role_name).await?; if role_layer_toml .as_table() .is_some_and(toml::map::Map::is_empty) { - return Ok(()); + return Ok(AppliedRoleConfig::default()); } + let role_sets_service_tier = role_layer_toml.get("service_tier").is_some(); let (preserve_current_profile, preserve_current_provider) = preservation_policy(config, &role_layer_toml); @@ -81,7 +90,11 @@ async fn apply_role_to_config_inner( preserve_current_provider, ) .await?; - Ok(()) + Ok(AppliedRoleConfig { + service_tier: role_sets_service_tier + .then(|| config.service_tier.clone()) + .flatten(), + }) } async fn load_role_layer_toml( @@ -317,28 +330,39 @@ pub(crate) mod spawn_tool_spec { }) .and_then(|contents| toml::from_str::(&contents).ok()) .map(|role_toml| { - let model = role_toml - .get("model") - .and_then(TomlValue::as_str); + let model = role_toml.get("model").and_then(TomlValue::as_str); let reasoning_effort = role_toml .get("model_reasoning_effort") .and_then(TomlValue::as_str); + let service_tier = role_toml + .get("service_tier") + .and_then(TomlValue::as_str); + let mut locked_settings = Vec::new(); + if let Some(model) = model { + locked_settings.push(format!("model is set to `{model}`")); + } + if let Some(reasoning_effort) = reasoning_effort { + locked_settings.push(format!( + "reasoning effort is set to `{reasoning_effort}`" + )); + } + if let Some(service_tier) = service_tier { + locked_settings + .push(format!("service tier is set to `{service_tier}`")); + } - match (model, reasoning_effort) { - (Some(model), Some(reasoning_effort)) => format!( - "\n- This role's model is set to `{model}` and its reasoning effort is set to `{reasoning_effort}`. These settings cannot be changed." - ), - (Some(model), None) => { - format!( - "\n- This role's model is set to `{model}` and cannot be changed." - ) + match locked_settings.as_slice() { + [] => String::new(), + [setting] => { + format!("\n- This role's {setting} and cannot be changed.") } - (None, Some(reasoning_effort)) => { - format!( - "\n- This role's reasoning effort is set to `{reasoning_effort}` and cannot be changed." - ) - } - (None, None) => String::new(), + [left, right] => format!( + "\n- This role's {left} and its {right}. These settings cannot be changed." + ), + [left, middle, right] => format!( + "\n- This role's {left}, its {middle}, and its {right}. These settings cannot be changed." + ), + _ => unreachable!("role locked settings are limited to three fields"), } }) .unwrap_or_default(); diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 1c99fb5950f3..a60fad3f049e 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -6,6 +6,7 @@ use crate::skills_load_input_from_config; use codex_config::ConfigLayerStackOrdering; use codex_core_plugins::PluginsManager; use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::config_types::ServiceTier; use codex_protocol::config_types::Verbosity; use codex_protocol::openai_models::ReasoningEffort; use codex_utils_absolute_path::test_support::PathExt; @@ -214,6 +215,39 @@ async fn apply_role_preserves_unspecified_keys() { ); } +#[tokio::test] +async fn apply_role_reports_explicit_service_tier() { + let (home, mut config) = test_config_with_cli_overrides(Vec::new()).await; + let role_path = write_role_config( + &home, + "tiered-role.toml", + r#"developer_instructions = "Stay focused" +service_tier = "priority" +"#, + ) + .await; + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: None, + config_file: Some(role_path), + nickname_candidates: None, + }, + ); + + let applied_role = apply_role_to_config(&mut config, Some("custom")) + .await + .expect("custom role should apply"); + + assert_eq!( + (applied_role.service_tier, config.service_tier,), + ( + Some(ServiceTier::Fast.request_value().to_string()), + Some(ServiceTier::Fast.request_value().to_string()), + ) + ); +} + #[tokio::test] async fn apply_role_preserves_active_profile_and_model_provider() { let home = TempDir::new().expect("create temp dir"); @@ -766,6 +800,31 @@ fn spawn_tool_spec_marks_role_locked_reasoning_effort_only() { )); } +#[test] +fn spawn_tool_spec_marks_role_locked_service_tier() { + let tempdir = TempDir::new().expect("create temp dir"); + let role_path = tempdir.path().join("reviewer.toml"); + fs::write( + &role_path, + "developer_instructions = \"Review quickly\"\nservice_tier = \"priority\"\n", + ) + .expect("write role config"); + let user_defined_roles = BTreeMap::from([( + "reviewer".to_string(), + AgentRoleConfig { + description: Some("Review quickly.".to_string()), + config_file: Some(role_path), + nickname_candidates: None, + }, + )]); + + let spec = spawn_tool_spec::build(&user_defined_roles); + + assert!(spec.contains( + "Review quickly.\n- This role's service tier is set to `priority` and cannot be changed." + )); +} + #[test] fn built_in_config_file_contents_resolves_explorer_only() { assert_eq!( 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 67f109b738ef..49bcd23b976f 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -82,12 +82,13 @@ impl ToolHandler for Handler { .await; let mut config = build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; - if args.fork_context { + let applied_role = if args.fork_context { reject_full_fork_spawn_overrides( role_name, args.model.as_deref(), args.reasoning_effort, )?; + Default::default() } else { apply_requested_spawn_agent_model_overrides( &session, @@ -99,11 +100,12 @@ impl ToolHandler for Handler { .await?; apply_role_to_config(&mut config, role_name) .await - .map_err(FunctionCallError::RespondToModel)?; - } + .map_err(FunctionCallError::RespondToModel)? + }; apply_spawn_agent_service_tier( &session, &mut config, + applied_role.service_tier.as_deref(), turn.config.service_tier.as_deref(), args.service_tier.as_deref(), ) 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 bb9ff530a767..b3b31c629fa1 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_common.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_common.rs @@ -339,10 +339,14 @@ pub(crate) async fn apply_requested_spawn_agent_model_overrides( pub(crate) async fn apply_spawn_agent_service_tier( session: &Session, config: &mut Config, + role_service_tier: Option<&str>, parent_service_tier: Option<&str>, requested_service_tier: Option<&str>, ) -> Result<(), FunctionCallError> { - let Some(candidate_service_tier) = requested_service_tier.or(parent_service_tier) else { + let Some(candidate_service_tier) = role_service_tier + .or(requested_service_tier) + .or(parent_service_tier) + else { config.service_tier = None; return Ok(()); }; @@ -362,7 +366,7 @@ pub(crate) async fn apply_spawn_agent_service_tier( return Ok(()); } - if requested_service_tier.is_none() { + if role_service_tier.is_none() && requested_service_tier.is_none() { config.service_tier = None; return Ok(()); } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index f769cd0aeaa5..a4a2599c08d1 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -133,6 +133,38 @@ model_reasoning_effort = "minimal" role_name } +async fn install_role_with_service_tier(turn: &mut TurnContext, model: &str) -> String { + let role_name = "tier-role".to_string(); + tokio::fs::create_dir_all(&turn.config.codex_home) + .await + .expect("codex home should be created"); + let role_config_path = turn + .config + .codex_home + .as_path() + .join(format!("{role_name}.toml")); + tokio::fs::write( + &role_config_path, + format!( + "developer_instructions = \"Use the configured tier\"\nmodel = \"{model}\"\nservice_tier = \"{}\"\n", + ServiceTier::Fast.request_value() + ), + ) + .await + .expect("role config should be written"); + let mut config = (*turn.config).clone(); + config.agent_roles.insert( + role_name.clone(), + AgentRoleConfig { + description: Some("Tiered role".to_string()), + config_file: Some(role_config_path), + nickname_candidates: None, + }, + ); + turn.config = Arc::new(config); + role_name +} + fn expect_text_output(output: T) -> (String, Option) where T: ToolOutput, @@ -490,6 +522,52 @@ async fn spawn_agent_service_tier_override_uses_supported_child_model_tier() { ); } +#[tokio::test] +async fn spawn_agent_role_service_tier_overrides_spawn_argument() { + #[derive(Debug, Deserialize)] + struct SpawnAgentResult { + agent_id: String, + } + + let (mut session, mut turn) = make_session_and_context().await; + let role_name = install_role_with_service_tier(&mut turn, "gpt-5.4").await; + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + + let output = SpawnAgentHandler::default() + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "agent_type": role_name, + "service_tier": "turbo" + })), + )) + .await + .expect("role-owned service tier should win over the spawn argument"); + let (content, _) = expect_text_output(output); + let result: SpawnAgentResult = + serde_json::from_str(&content).expect("spawn_agent result should be json"); + let snapshot = manager + .get_thread(parse_agent_id(&result.agent_id)) + .await + .expect("spawned agent thread should exist") + .config_snapshot() + .await; + + assert_eq!( + snapshot.service_tier, + Some(ServiceTier::Fast.request_value().to_string()) + ); +} + #[tokio::test] async fn spawn_agent_service_tier_override_rejects_unknown_tier() { let (session, turn) = make_session_and_context().await; @@ -516,6 +594,32 @@ async fn spawn_agent_service_tier_override_rejects_unknown_tier() { ); } +#[tokio::test] +async fn spawn_agent_role_service_tier_rejects_unsupported_child_model_tier() { + let (session, mut turn) = make_session_and_context().await; + let role_name = install_role_with_service_tier(&mut turn, "gpt-5.3-codex").await; + let err = SpawnAgentHandler::default() + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "agent_type": role_name + })), + )) + .await + .expect_err("role-owned service tier should validate against the final role model"); + + assert_eq!( + err, + FunctionCallError::RespondToModel( + "Service tier `priority` is not supported for model `gpt-5.3-codex`. Supported service tiers: none" + .to_string() + ) + ); +} + #[tokio::test] async fn spawn_agent_service_tier_override_rejects_tier_unsupported_by_child_model() { let (session, turn) = make_session_and_context().await; @@ -542,6 +646,70 @@ async fn spawn_agent_service_tier_override_rejects_tier_unsupported_by_child_mod ); } +#[tokio::test] +async fn multi_agent_v2_spawn_role_service_tier_persists_in_child_config() { + #[derive(Debug, Deserialize)] + struct SpawnAgentResult { + task_name: String, + } + + let (mut session, mut turn) = make_session_and_context().await; + let role_name = install_role_with_service_tier(&mut turn, "gpt-5.4").await; + let mut config = (*turn.config).clone(); + config + .features + .enable(Feature::MultiAgentV2) + .expect("test config should allow feature update"); + turn.config = Arc::new(config); + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + let session = Arc::new(session); + let turn = Arc::new(turn); + + let output = SpawnAgentHandlerV2::default() + .handle(invocation( + session.clone(), + turn.clone(), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "task_name": "tiered_role", + "agent_type": role_name + })), + )) + .await + .expect("role-owned service tier should persist in v2 child config"); + let (content, _) = expect_text_output(output); + let result: SpawnAgentResult = + serde_json::from_str(&content).expect("spawn_agent result should be json"); + let child_thread_id = session + .services + .agent_control + .resolve_agent_reference( + session.conversation_id, + &turn.session_source, + result.task_name.as_str(), + ) + .await + .expect("spawned task name should resolve"); + let snapshot = manager + .get_thread(child_thread_id) + .await + .expect("spawned agent thread should exist") + .config_snapshot() + .await; + + assert_eq!( + snapshot.service_tier, + Some(ServiceTier::Fast.request_value().to_string()) + ); +} + #[tokio::test] async fn spawn_agent_inherits_supported_parent_service_tier() { #[derive(Debug, Deserialize)] 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 79772be91f82..e96bbefb73f2 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 @@ -81,12 +81,13 @@ impl ToolHandler for Handler { .await; let mut config = build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; - if matches!(fork_mode, Some(SpawnAgentForkMode::FullHistory)) { + let applied_role = if matches!(fork_mode, Some(SpawnAgentForkMode::FullHistory)) { reject_full_fork_spawn_overrides( role_name, args.model.as_deref(), args.reasoning_effort, )?; + Default::default() } else { apply_requested_spawn_agent_model_overrides( &session, @@ -98,11 +99,12 @@ impl ToolHandler for Handler { .await?; apply_role_to_config(&mut config, role_name) .await - .map_err(FunctionCallError::RespondToModel)?; - } + .map_err(FunctionCallError::RespondToModel)? + }; apply_spawn_agent_service_tier( &session, &mut config, + applied_role.service_tier.as_deref(), turn.config.service_tier.as_deref(), args.service_tier.as_deref(), ) From 6cd2781d5118ebf6205283e12351bce95fc8903f Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 11 May 2026 19:50:50 +0300 Subject: [PATCH 2/5] codex: fix CI failure on PR #22169 --- codex-rs/core/src/tools/handlers/multi_agents_tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index a4a2599c08d1..b63cb6aa753c 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -679,7 +679,8 @@ async fn multi_agent_v2_spawn_role_service_tier_persists_in_child_config() { function_payload(json!({ "message": "inspect this repo", "task_name": "tiered_role", - "agent_type": role_name + "agent_type": role_name, + "fork_turns": "1" })), )) .await From 04a51a35429239a7f67b58b171f1d6ef4c04fea4 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 11 May 2026 20:29:02 +0300 Subject: [PATCH 3/5] Simplify role service tier flow --- codex-rs/core/src/agent/role.rs | 68 ++++++------------- codex-rs/core/src/agent/role_tests.rs | 34 +--------- .../src/tools/handlers/multi_agents/spawn.rs | 8 +-- .../src/tools/handlers/multi_agents_common.rs | 14 ++-- .../src/tools/handlers/multi_agents_tests.rs | 33 +-------- .../tools/handlers/multi_agents_v2/spawn.rs | 8 +-- 6 files changed, 41 insertions(+), 124 deletions(-) diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index e613d8443277..2ab16cd22a25 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -29,16 +29,8 @@ use toml::Value as TomlValue; pub const DEFAULT_ROLE_NAME: &str = "default"; const AGENT_TYPE_UNAVAILABLE_ERROR: &str = "agent type is currently not available"; -#[derive(Debug, Clone, Default, PartialEq, Eq)] -pub(crate) struct AppliedRoleConfig { - pub(crate) service_tier: Option, -} - /// Applies a named role layer to `config` while preserving caller-owned model selection. /// -/// The returned settings capture role-owned values that spawn orchestration must keep locked after -/// the config reload completes. -/// /// The role layer is inserted at session-flag precedence so it can override persisted config, but /// the caller's current `profile` and `model_provider` remain sticky runtime choices unless the /// role explicitly sets `profile`, explicitly sets `model_provider`, or rewrites the active @@ -48,7 +40,7 @@ pub(crate) struct AppliedRoleConfig { pub(crate) async fn apply_role_to_config( config: &mut Config, role_name: Option<&str>, -) -> Result { +) -> Result<(), String> { let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME); let role = resolve_role_config(config, role_name) @@ -67,19 +59,18 @@ async fn apply_role_to_config_inner( config: &mut Config, role_name: &str, role: &AgentRoleConfig, -) -> anyhow::Result { +) -> anyhow::Result<()> { let is_built_in = !config.agent_roles.contains_key(role_name); let Some(config_file) = role.config_file.as_ref() else { - return Ok(AppliedRoleConfig::default()); + return Ok(()); }; let role_layer_toml = load_role_layer_toml(config, config_file, is_built_in, role_name).await?; if role_layer_toml .as_table() .is_some_and(toml::map::Map::is_empty) { - return Ok(AppliedRoleConfig::default()); + return Ok(()); } - let role_sets_service_tier = role_layer_toml.get("service_tier").is_some(); let (preserve_current_profile, preserve_current_provider) = preservation_policy(config, &role_layer_toml); @@ -90,11 +81,7 @@ async fn apply_role_to_config_inner( preserve_current_provider, ) .await?; - Ok(AppliedRoleConfig { - service_tier: role_sets_service_tier - .then(|| config.service_tier.clone()) - .flatten(), - }) + Ok(()) } async fn load_role_layer_toml( @@ -330,39 +317,28 @@ pub(crate) mod spawn_tool_spec { }) .and_then(|contents| toml::from_str::(&contents).ok()) .map(|role_toml| { - let model = role_toml.get("model").and_then(TomlValue::as_str); + let model = role_toml + .get("model") + .and_then(TomlValue::as_str); let reasoning_effort = role_toml .get("model_reasoning_effort") .and_then(TomlValue::as_str); - let service_tier = role_toml - .get("service_tier") - .and_then(TomlValue::as_str); - let mut locked_settings = Vec::new(); - if let Some(model) = model { - locked_settings.push(format!("model is set to `{model}`")); - } - if let Some(reasoning_effort) = reasoning_effort { - locked_settings.push(format!( - "reasoning effort is set to `{reasoning_effort}`" - )); - } - if let Some(service_tier) = service_tier { - locked_settings - .push(format!("service tier is set to `{service_tier}`")); - } - match locked_settings.as_slice() { - [] => String::new(), - [setting] => { - format!("\n- This role's {setting} and cannot be changed.") - } - [left, right] => format!( - "\n- This role's {left} and its {right}. These settings cannot be changed." + match (model, reasoning_effort) { + (Some(model), Some(reasoning_effort)) => format!( + "\n- This role's model is set to `{model}` and its reasoning effort is set to `{reasoning_effort}`. These settings cannot be changed." ), - [left, middle, right] => format!( - "\n- This role's {left}, its {middle}, and its {right}. These settings cannot be changed." - ), - _ => unreachable!("role locked settings are limited to three fields"), + (Some(model), None) => { + format!( + "\n- This role's model is set to `{model}` and cannot be changed." + ) + } + (None, Some(reasoning_effort)) => { + format!( + "\n- This role's reasoning effort is set to `{reasoning_effort}` and cannot be changed." + ) + } + (None, None) => String::new(), } }) .unwrap_or_default(); diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index a60fad3f049e..509eeb664631 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -235,16 +235,13 @@ service_tier = "priority" }, ); - let applied_role = apply_role_to_config(&mut config, Some("custom")) + apply_role_to_config(&mut config, Some("custom")) .await .expect("custom role should apply"); assert_eq!( - (applied_role.service_tier, config.service_tier,), - ( - Some(ServiceTier::Fast.request_value().to_string()), - Some(ServiceTier::Fast.request_value().to_string()), - ) + config.service_tier, + Some(ServiceTier::Fast.request_value().to_string()) ); } @@ -800,31 +797,6 @@ fn spawn_tool_spec_marks_role_locked_reasoning_effort_only() { )); } -#[test] -fn spawn_tool_spec_marks_role_locked_service_tier() { - let tempdir = TempDir::new().expect("create temp dir"); - let role_path = tempdir.path().join("reviewer.toml"); - fs::write( - &role_path, - "developer_instructions = \"Review quickly\"\nservice_tier = \"priority\"\n", - ) - .expect("write role config"); - let user_defined_roles = BTreeMap::from([( - "reviewer".to_string(), - AgentRoleConfig { - description: Some("Review quickly.".to_string()), - config_file: Some(role_path), - nickname_candidates: None, - }, - )]); - - let spec = spawn_tool_spec::build(&user_defined_roles); - - assert!(spec.contains( - "Review quickly.\n- This role's service tier is set to `priority` and cannot be changed." - )); -} - #[test] fn built_in_config_file_contents_resolves_explorer_only() { assert_eq!( 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 49bcd23b976f..67f109b738ef 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -82,13 +82,12 @@ impl ToolHandler for Handler { .await; let mut config = build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; - let applied_role = if args.fork_context { + if args.fork_context { reject_full_fork_spawn_overrides( role_name, args.model.as_deref(), args.reasoning_effort, )?; - Default::default() } else { apply_requested_spawn_agent_model_overrides( &session, @@ -100,12 +99,11 @@ impl ToolHandler for Handler { .await?; apply_role_to_config(&mut config, role_name) .await - .map_err(FunctionCallError::RespondToModel)? - }; + .map_err(FunctionCallError::RespondToModel)?; + } apply_spawn_agent_service_tier( &session, &mut config, - applied_role.service_tier.as_deref(), turn.config.service_tier.as_deref(), args.service_tier.as_deref(), ) 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 b3b31c629fa1..9c28b5426ab5 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_common.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_common.rs @@ -339,13 +339,13 @@ pub(crate) async fn apply_requested_spawn_agent_model_overrides( pub(crate) async fn apply_spawn_agent_service_tier( session: &Session, config: &mut Config, - role_service_tier: Option<&str>, parent_service_tier: Option<&str>, requested_service_tier: Option<&str>, ) -> Result<(), FunctionCallError> { - let Some(candidate_service_tier) = role_service_tier - .or(requested_service_tier) - .or(parent_service_tier) + let Some(candidate_service_tier) = requested_service_tier + .map(str::to_string) + .or_else(|| config.service_tier.clone()) + .or_else(|| parent_service_tier.map(str::to_string)) else { config.service_tier = None; return Ok(()); @@ -361,12 +361,12 @@ pub(crate) async fn apply_spawn_agent_service_tier( .get_model_info(model.as_str(), &config.to_models_manager_config()) .await; - if model_info.supports_service_tier(candidate_service_tier) { - config.service_tier = Some(candidate_service_tier.to_string()); + if model_info.supports_service_tier(candidate_service_tier.as_str()) { + config.service_tier = Some(candidate_service_tier); return Ok(()); } - if role_service_tier.is_none() && requested_service_tier.is_none() { + if requested_service_tier.is_none() { config.service_tier = None; return Ok(()); } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index b63cb6aa753c..94f8ed3d8724 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -523,7 +523,7 @@ async fn spawn_agent_service_tier_override_uses_supported_child_model_tier() { } #[tokio::test] -async fn spawn_agent_role_service_tier_overrides_spawn_argument() { +async fn spawn_agent_role_service_tier_persists_in_child_config() { #[derive(Debug, Deserialize)] struct SpawnAgentResult { agent_id: String, @@ -546,12 +546,11 @@ async fn spawn_agent_role_service_tier_overrides_spawn_argument() { "spawn_agent", function_payload(json!({ "message": "inspect this repo", - "agent_type": role_name, - "service_tier": "turbo" + "agent_type": role_name })), )) .await - .expect("role-owned service tier should win over the spawn argument"); + .expect("role-configured service tier should persist in the child config"); let (content, _) = expect_text_output(output); let result: SpawnAgentResult = serde_json::from_str(&content).expect("spawn_agent result should be json"); @@ -594,32 +593,6 @@ async fn spawn_agent_service_tier_override_rejects_unknown_tier() { ); } -#[tokio::test] -async fn spawn_agent_role_service_tier_rejects_unsupported_child_model_tier() { - let (session, mut turn) = make_session_and_context().await; - let role_name = install_role_with_service_tier(&mut turn, "gpt-5.3-codex").await; - let err = SpawnAgentHandler::default() - .handle(invocation( - Arc::new(session), - Arc::new(turn), - "spawn_agent", - function_payload(json!({ - "message": "inspect this repo", - "agent_type": role_name - })), - )) - .await - .expect_err("role-owned service tier should validate against the final role model"); - - assert_eq!( - err, - FunctionCallError::RespondToModel( - "Service tier `priority` is not supported for model `gpt-5.3-codex`. Supported service tiers: none" - .to_string() - ) - ); -} - #[tokio::test] async fn spawn_agent_service_tier_override_rejects_tier_unsupported_by_child_model() { let (session, turn) = make_session_and_context().await; 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 e96bbefb73f2..79772be91f82 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 @@ -81,13 +81,12 @@ impl ToolHandler for Handler { .await; let mut config = build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; - let applied_role = if matches!(fork_mode, Some(SpawnAgentForkMode::FullHistory)) { + if matches!(fork_mode, Some(SpawnAgentForkMode::FullHistory)) { reject_full_fork_spawn_overrides( role_name, args.model.as_deref(), args.reasoning_effort, )?; - Default::default() } else { apply_requested_spawn_agent_model_overrides( &session, @@ -99,12 +98,11 @@ impl ToolHandler for Handler { .await?; apply_role_to_config(&mut config, role_name) .await - .map_err(FunctionCallError::RespondToModel)? - }; + .map_err(FunctionCallError::RespondToModel)?; + } apply_spawn_agent_service_tier( &session, &mut config, - applied_role.service_tier.as_deref(), turn.config.service_tier.as_deref(), args.service_tier.as_deref(), ) From 9b0fc86046b7a4dbf63ac552e083e63e7ce1bf57 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 11 May 2026 23:50:47 +0300 Subject: [PATCH 4/5] Match role service tier precedence --- .../src/tools/handlers/multi_agents/spawn.rs | 3 ++ .../src/tools/handlers/multi_agents_common.rs | 12 +++-- .../src/tools/handlers/multi_agents_tests.rs | 46 +++++++++++++++++++ .../tools/handlers/multi_agents_v2/spawn.rs | 3 ++ 4 files changed, 60 insertions(+), 4 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 67f109b738ef..5d4d1d6ac156 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -82,6 +82,9 @@ impl ToolHandler for Handler { .await; let mut config = build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; + if let Some(service_tier) = args.service_tier.as_ref() { + config.service_tier = Some(service_tier.clone()); + } if args.fork_context { reject_full_fork_spawn_overrides( role_name, 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 9c28b5426ab5..7058024acdab 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_common.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_common.rs @@ -342,9 +342,13 @@ pub(crate) async fn apply_spawn_agent_service_tier( parent_service_tier: Option<&str>, requested_service_tier: Option<&str>, ) -> Result<(), FunctionCallError> { - let Some(candidate_service_tier) = requested_service_tier - .map(str::to_string) - .or_else(|| config.service_tier.clone()) + let requested_service_tier_is_effective = + requested_service_tier.is_some_and(|requested_service_tier| { + config.service_tier.as_deref() == Some(requested_service_tier) + }); + let Some(candidate_service_tier) = config + .service_tier + .clone() .or_else(|| parent_service_tier.map(str::to_string)) else { config.service_tier = None; @@ -366,7 +370,7 @@ pub(crate) async fn apply_spawn_agent_service_tier( return Ok(()); } - if requested_service_tier.is_none() { + if !requested_service_tier_is_effective { config.service_tier = None; return Ok(()); } diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 94f8ed3d8724..fa5dd442df47 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -567,6 +567,52 @@ async fn spawn_agent_role_service_tier_persists_in_child_config() { ); } +#[tokio::test] +async fn spawn_agent_role_service_tier_overrides_spawn_argument() { + #[derive(Debug, Deserialize)] + struct SpawnAgentResult { + agent_id: String, + } + + let (mut session, mut turn) = make_session_and_context().await; + let role_name = install_role_with_service_tier(&mut turn, "gpt-5.4").await; + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + + let output = SpawnAgentHandler::default() + .handle(invocation( + Arc::new(session), + Arc::new(turn), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "agent_type": role_name, + "service_tier": "turbo" + })), + )) + .await + .expect("role-configured service tier should win over the spawn argument"); + let (content, _) = expect_text_output(output); + let result: SpawnAgentResult = + serde_json::from_str(&content).expect("spawn_agent result should be json"); + let snapshot = manager + .get_thread(parse_agent_id(&result.agent_id)) + .await + .expect("spawned agent thread should exist") + .config_snapshot() + .await; + + assert_eq!( + snapshot.service_tier, + Some(ServiceTier::Fast.request_value().to_string()) + ); +} + #[tokio::test] async fn spawn_agent_service_tier_override_rejects_unknown_tier() { let (session, turn) = make_session_and_context().await; 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 79772be91f82..5e4b7863a2ee 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 @@ -81,6 +81,9 @@ impl ToolHandler for Handler { .await; let mut config = build_agent_spawn_config(&session.get_base_instructions().await, turn.as_ref())?; + if let Some(service_tier) = args.service_tier.as_ref() { + config.service_tier = Some(service_tier.clone()); + } if matches!(fork_mode, Some(SpawnAgentForkMode::FullHistory)) { reject_full_fork_spawn_overrides( role_name, From a33758b396702084802a9854099b9fc3eed874a9 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Mon, 11 May 2026 23:55:58 +0300 Subject: [PATCH 5/5] Cover v2 role service tier precedence --- .../src/tools/handlers/multi_agents_tests.rs | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index fa5dd442df47..230ec46fce35 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -730,6 +730,72 @@ async fn multi_agent_v2_spawn_role_service_tier_persists_in_child_config() { ); } +#[tokio::test] +async fn multi_agent_v2_spawn_role_service_tier_overrides_spawn_argument() { + #[derive(Debug, Deserialize)] + struct SpawnAgentResult { + task_name: String, + } + + let (mut session, mut turn) = make_session_and_context().await; + let role_name = install_role_with_service_tier(&mut turn, "gpt-5.4").await; + let mut config = (*turn.config).clone(); + config + .features + .enable(Feature::MultiAgentV2) + .expect("test config should allow feature update"); + turn.config = Arc::new(config); + let manager = thread_manager(); + let root = manager + .start_thread((*turn.config).clone()) + .await + .expect("root thread should start"); + session.services.agent_control = manager.agent_control(); + session.conversation_id = root.thread_id; + let session = Arc::new(session); + let turn = Arc::new(turn); + + let output = SpawnAgentHandlerV2::default() + .handle(invocation( + session.clone(), + turn.clone(), + "spawn_agent", + function_payload(json!({ + "message": "inspect this repo", + "task_name": "tiered_role_override", + "agent_type": role_name, + "service_tier": "turbo", + "fork_turns": "1" + })), + )) + .await + .expect("role-configured service tier should win over the spawn argument"); + let (content, _) = expect_text_output(output); + let result: SpawnAgentResult = + serde_json::from_str(&content).expect("spawn_agent result should be json"); + let child_thread_id = session + .services + .agent_control + .resolve_agent_reference( + session.conversation_id, + &turn.session_source, + result.task_name.as_str(), + ) + .await + .expect("spawned task name should resolve"); + let snapshot = manager + .get_thread(child_thread_id) + .await + .expect("spawned agent thread should exist") + .config_snapshot() + .await; + + assert_eq!( + snapshot.service_tier, + Some(ServiceTier::Fast.request_value().to_string()) + ); +} + #[tokio::test] async fn spawn_agent_inherits_supported_parent_service_tier() { #[derive(Debug, Deserialize)]