mirror of
https://github.com/openai/codex.git
synced 2026-05-21 11:42:55 +00:00
[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.
This commit is contained in:
@@ -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<Config> {
|
||||
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}}")
|
||||
|
||||
@@ -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!(
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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::<Vec<_>>()
|
||||
.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::<Vec<_>>()
|
||||
.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(
|
||||
|
||||
@@ -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)]
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user