From c53da029bcb8afe07e4019fa13dfd46b37d7cd7d Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 19 May 2026 15:40:41 -0700 Subject: [PATCH] [codex] Honor role-defined spawn service tiers (#22169) ## Why Custom agent roles are ordinary config layers, so a role file can already express `service_tier` just like other config values. The spawned-agent tier path needs to preserve that effective role config and follow the same precedence pattern as model/reasoning. ## What changed - Apply an explicit spawn-time `service_tier` onto the child config before role application, so a role config layer can override it just like role-defined model/reasoning settings do. - Validate the final effective child tier after the final child model is known, while still falling back to the parent tier when no child tier survives. - Add focused integration coverage for both v1 and v2 proving role TOML loads a service tier, spawned children keep that role-configured tier, and a role tier wins over a conflicting spawn-time tier. ## Validation - `just fmt` - `git diff --check` - Local Rust tests not run, per repo guidance; CI should exercise the new coverage. --- codex-rs/core/src/agent/role.rs | 31 ++++- codex-rs/core/src/agent/role_tests.rs | 86 ++++++++++++ .../src/tools/handlers/multi_agents/spawn.rs | 3 + .../src/tools/handlers/multi_agents_common.rs | 56 ++++---- .../src/tools/handlers/multi_agents_tests.rs | 123 ++++++++++++++++++ .../tools/handlers/multi_agents_v2/spawn.rs | 3 + 6 files changed, 275 insertions(+), 27 deletions(-) diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index 2ab16cd22a..a38930c00e 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -73,12 +73,14 @@ async fn apply_role_to_config_inner( } let (preserve_current_profile, preserve_current_provider) = preservation_policy(config, &role_layer_toml); + let preserve_current_service_tier = role_layer_toml.get("service_tier").is_none(); *config = reload::build_next_config( config, role_layer_toml, preserve_current_profile, preserve_current_provider, + preserve_current_service_tier, ) .await?; Ok(()) @@ -157,6 +159,7 @@ mod reload { role_layer_toml: TomlValue, preserve_current_profile: bool, preserve_current_provider: bool, + preserve_current_service_tier: bool, ) -> anyhow::Result { let active_profile_name = preserve_current_profile .then_some(config.active_profile.as_deref()) @@ -171,7 +174,11 @@ mod reload { let mut next_config = Config::load_config_with_layer_stack( LOCAL_FS.as_ref(), merged_config, - reload_overrides(config, preserve_current_provider), + reload_overrides( + config, + preserve_current_provider, + preserve_current_service_tier, + ), config.codex_home.clone(), config_layer_stack, ) @@ -261,10 +268,15 @@ mod reload { ConfigLayerEntry::new(ConfigLayerSource::SessionFlags, role_layer_toml) } - fn reload_overrides(config: &Config, preserve_current_provider: bool) -> ConfigOverrides { + fn reload_overrides( + config: &Config, + preserve_current_provider: bool, + preserve_current_service_tier: bool, + ) -> ConfigOverrides { ConfigOverrides { cwd: Some(config.cwd.to_path_buf()), model_provider: preserve_current_provider.then(|| config.model_provider_id.clone()), + service_tier: preserve_current_service_tier.then(|| config.service_tier.clone()), codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), main_execve_wrapper_exe: config.main_execve_wrapper_exe.clone(), ..Default::default() @@ -323,8 +335,11 @@ pub(crate) mod spawn_tool_spec { 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); - match (model, reasoning_effort) { + let model_and_reasoning_note = 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." ), @@ -339,7 +354,15 @@ pub(crate) mod spawn_tool_spec { ) } (None, None) => String::new(), - } + }; + let service_tier_note = service_tier + .map(|service_tier| { + format!( + "\n- This role's service tier is set to `{service_tier}`. If it is supported by the resolved model, it takes precedence over a valid spawn request service tier." + ) + }) + .unwrap_or_default(); + format!("{model_and_reasoning_note}{service_tier_note}") }) .unwrap_or_default(); format!("{name}: {{\n{description}{locked_settings_note}\n}}") diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 1c99fb5950..828fa5e591 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,66 @@ 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, + }, + ); + + apply_role_to_config(&mut config, Some("custom")) + .await + .expect("custom role should apply"); + + assert_eq!( + config.service_tier, + Some(ServiceTier::Fast.request_value().to_string()) + ); +} + +#[tokio::test] +async fn apply_role_preserves_existing_service_tier_without_override() { + let (home, mut config) = test_config_with_cli_overrides(Vec::new()).await; + config.service_tier = Some(ServiceTier::Fast.request_value().to_string()); + let role_path = write_role_config( + &home, + "default-tier-role.toml", + r#"developer_instructions = "Stay focused" +"#, + ) + .await; + config.agent_roles.insert( + "custom".to_string(), + AgentRoleConfig { + description: None, + config_file: Some(role_path), + nickname_candidates: None, + }, + ); + + apply_role_to_config(&mut config, Some("custom")) + .await + .expect("custom role should apply"); + + assert_eq!( + config.service_tier, + 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 +827,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("tiered.toml"); + fs::write( + &role_path, + "developer_instructions = \"Stay fast\"\nservice_tier = \"priority\"\n", + ) + .expect("write role config"); + let user_defined_roles = BTreeMap::from([( + "tiered".to_string(), + AgentRoleConfig { + description: Some("Stay fast.".to_string()), + config_file: Some(role_path), + nickname_candidates: None, + }, + )]); + + let spec = spawn_tool_spec::build(&user_defined_roles); + + assert!(spec.contains( + "Stay fast.\n- This role's service tier is set to `priority`. If it is supported by the resolved model, it takes precedence over a valid spawn request service tier." + )); +} + #[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 ceb46083c5..dc0518de6b 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -83,6 +83,9 @@ async fn handle_spawn_agent( .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, args.model.as_deref(), args.reasoning_effort)?; } else { 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 968810494d..4f459a0d0e 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_common.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_common.rs @@ -344,9 +344,16 @@ 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.or(parent_service_tier) else { + let candidate_service_tiers = [ + config.service_tier.clone(), + requested_service_tier.map(str::to_string), + parent_service_tier.map(str::to_string), + ]; + if candidate_service_tiers.iter().all(Option::is_none) { + config.service_tier = None; return Ok(()); - }; + } + let model = config.model.clone().ok_or_else(|| { FunctionCallError::RespondToModel( "spawn_agent could not resolve the child model for service tier validation".to_string(), @@ -358,29 +365,32 @@ 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()); - return Ok(()); + if let Some(requested_service_tier) = requested_service_tier + && !model_info.supports_service_tier(requested_service_tier) + { + let supported_service_tiers = if model_info.service_tiers.is_empty() { + "none".to_string() + } else { + model_info + .service_tiers + .iter() + .map(|tier| tier.id.as_str()) + .collect::>() + .join(", ") + }; + return Err(FunctionCallError::RespondToModel(format!( + "Service tier `{requested_service_tier}` is not supported for model `{model}`. Supported service tiers: {supported_service_tiers}" + ))); } - if requested_service_tier.is_none() { - config.service_tier = None; - return Ok(()); - } - - let supported_service_tiers = if model_info.service_tiers.is_empty() { - "none".to_string() - } else { - model_info - .service_tiers - .iter() - .map(|tier| tier.id.as_str()) - .collect::>() - .join(", ") - }; - Err(FunctionCallError::RespondToModel(format!( - "Service tier `{candidate_service_tier}` is not supported for model `{model}`. Supported service tiers: {supported_service_tiers}" - ))) + config.service_tier = + candidate_service_tiers + .into_iter() + .flatten() + .find(|candidate_service_tier| { + model_info.supports_service_tier(candidate_service_tier.as_str()) + }); + Ok(()) } fn find_spawn_agent_model_name( 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 ad1702b207..d1e2aff3fb 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -704,6 +704,129 @@ service_tier = "priority" } } +#[tokio::test] +async fn spawn_agent_role_service_tier_falls_back_to_supported_parent_tier() { + #[derive(Debug, Deserialize)] + struct SpawnAgentResult { + agent_id: String, + } + + let (mut session, turn) = make_session_and_context().await; + let mut turn = turn + .with_model("gpt-5.4".to_string(), &session.services.models_manager) + .await; + 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("tiered-role.toml"); + tokio::fs::write( + &role_config_path, + r#"model = "gpt-5.4" +service_tier = "turbo" +"#, + ) + .await + .expect("role config should be written"); + + let role_name = "tiered-role".to_string(); + let mut config = (*turn.config).clone(); + config.service_tier = Some(ServiceTier::Fast.request_value().to_string()); + config.agent_roles.insert( + role_name.clone(), + AgentRoleConfig { + description: Some("Role with an unsupported child tier".to_string()), + config_file: Some(role_config_path), + nickname_candidates: None, + }, + ); + 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 output = 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("spawn_agent should fall back to the supported parent tier"); + 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_role_service_tier_does_not_hide_invalid_spawn_request() { + let (session, mut turn) = make_session_and_context().await; + 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("tiered-role.toml"); + tokio::fs::write( + &role_config_path, + r#"model = "gpt-5.4" +service_tier = "priority" +"#, + ) + .await + .expect("role config should be written"); + + let role_name = "tiered-role".to_string(); + let mut config = (*turn.config).clone(); + config.agent_roles.insert( + role_name.clone(), + AgentRoleConfig { + description: Some("Role with a supported child tier".to_string()), + config_file: Some(role_config_path), + nickname_candidates: None, + }, + ); + turn.config = Arc::new(config); + + 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, + "service_tier": "turbo" + })), + )) + .await + .expect_err("invalid spawn service tier should still be rejected"); + + assert_eq!( + err, + FunctionCallError::RespondToModel( + "Service tier `turbo` is not supported for model `gpt-5.4`. Supported service tiers: priority" + .to_string() + ) + ); +} + #[tokio::test] async fn spawn_agent_full_history_fork_accepts_explicit_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 2daa1f07db..74a87d8a54 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 @@ -82,6 +82,9 @@ async fn handle_spawn_agent( .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, args.model.as_deref(), args.reasoning_effort)?; } else {