core/core-agent-ide
8
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: sub-agent role when using profiles (#14807)

Browse source 
Fix the layering conflict when a project profile is used with agents.
This PR clean the config layering and make sure the agent config >
project profile

Fix https://github.com/openai/codex/issues/13849,
https://github.com/openai/codex/issues/14671
This commit is contained in:
jif-oai 2026-03-16 16:08:16 +00:00 • committed by GitHub
parent 029aab5563
commit 33acc1e65f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 237 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

245
codex-rs/core/src/agent/role.rs
View file

@ -15,6 +15,7 @@ use crate::config_loader::ConfigLayerEntry;
use crate::config_loader::ConfigLayerStack;
use crate::config_loader::ConfigLayerStackOrdering;
use crate::config_loader::resolve_relative_paths_in_config_toml;
use anyhow::anyhow;
use codex_app_server_protocol::ConfigLayerSource;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
@ -39,46 +40,86 @@ pub(crate) async fn apply_role_to_config(
role_name: Option<&str>,
) -> Result<(), String> {
let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME);
let is_built_in = !config.agent_roles.contains_key(role_name);
let (config_file, is_built_in) = resolve_role_config(config, role_name)
.map(|role| (&role.config_file, is_built_in))
let role = resolve_role_config(config, role_name)
.cloned()
.ok_or_else(|| format!("unknown agent_type '{role_name}'"))?;
let Some(config_file) = config_file.as_ref() else {
apply_role_to_config_inner(config, role_name, &role)
.await
.map_err(|err| {
tracing::warn!("failed to apply role to config: {err}");
AGENT_TYPE_UNAVAILABLE_ERROR.to_string()
})
}
async fn apply_role_to_config_inner(
config: &mut Config,
role_name: &str,
role: &AgentRoleConfig,
) -> 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(());
};
let role_layer_toml = load_role_layer_toml(config, config_file, is_built_in, role_name).await?;
let (preserve_current_profile, preserve_current_provider) =
preservation_policy(config, &role_layer_toml);
*config = reload::build_next_config(
config,
role_layer_toml,
preserve_current_profile,
preserve_current_provider,
)?;
Ok(())
}
async fn load_role_layer_toml(
config: &Config,
config_file: &Path,
is_built_in: bool,
role_name: &str,
) -> anyhow::Result<TomlValue> {
let (role_config_toml, role_config_base) = if is_built_in {
let role_config_contents = built_in::config_file_contents(config_file)
.map(str::to_owned)
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_toml: TomlValue = toml::from_str(&role_config_contents)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
.ok_or(anyhow!("No corresponding config content"))?;
let role_config_toml: TomlValue = toml::from_str(&role_config_contents)?;
(role_config_toml, config.codex_home.as_path())
} else {
let role_config_contents = tokio::fs::read_to_string(config_file)
.await
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_config_contents = tokio::fs::read_to_string(config_file).await?;
let role_config_base = config_file
.parent()
.ok_or(anyhow!("No corresponding config content"))?;
let role_config_toml = parse_agent_role_file_contents(
&role_config_contents,
config_file,
config_file
.parent()
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?,
role_config_base,
Some(role_name),
)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?
)?
.config;
(
role_config_toml,
config_file
.parent()
.ok_or_else(|| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?,
)
(role_config_toml, role_config_base)
};
deserialize_config_toml_with_base(role_config_toml.clone(), role_config_base)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let role_layer_toml = resolve_relative_paths_in_config_toml(role_config_toml, role_config_base)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
deserialize_config_toml_with_base(role_config_toml.clone(), role_config_base)?;
Ok(resolve_relative_paths_in_config_toml(
role_config_toml,
role_config_base,
)?)
}
pub(crate) fn resolve_role_config<'a>(
config: &'a Config,
role_name: &str,
) -> Option<&'a AgentRoleConfig> {
config
.agent_roles
.get(role_name)
.or_else(|| built_in::configs().get(role_name))
}
fn preservation_policy(config: &Config, role_layer_toml: &TomlValue) -> (bool, bool) {
let role_selects_provider = role_layer_toml.get("model_provider").is_some();
let role_selects_profile = role_layer_toml.get("profile").is_some();
let role_updates_active_profile_provider = config
@ -93,63 +134,129 @@ pub(crate) async fn apply_role_to_config(
.map(|profile| profile.contains_key("model_provider"))
})
.unwrap_or(false);
// A role that does not explicitly take ownership of model selection should inherit the
// caller's current profile/provider choices across the config reload.
let preserve_current_profile = !role_selects_provider && !role_selects_profile;
let preserve_current_provider =
preserve_current_profile && !role_updates_active_profile_provider;
(preserve_current_profile, preserve_current_provider)
}
let mut layers: Vec<ConfigLayerEntry> = config
.config_layer_stack
.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true)
.into_iter()
.cloned()
.collect();
let layer = ConfigLayerEntry::new(ConfigLayerSource::SessionFlags, role_layer_toml);
let insertion_index =
layers.partition_point(|existing_layer| existing_layer.name <= layer.name);
layers.insert(insertion_index, layer);
mod reload {
use super::*;
let config_layer_stack = ConfigLayerStack::new(
layers,
config.config_layer_stack.requirements().clone(),
config.config_layer_stack.requirements_toml().clone(),
)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
pub(super) fn build_next_config(
config: &Config,
role_layer_toml: TomlValue,
preserve_current_profile: bool,
preserve_current_provider: bool,
) -> anyhow::Result<Config> {
let active_profile_name = preserve_current_profile
.then_some(config.active_profile.as_deref())
.flatten();
let config_layer_stack =
build_config_layer_stack(config, &role_layer_toml, active_profile_name)?;
let mut merged_config = deserialize_effective_config(config, &config_layer_stack)?;
if preserve_current_profile {
merged_config.profile = None;
}
let merged_toml = config_layer_stack.effective_config();
let merged_config = deserialize_config_toml_with_base(merged_toml, &config.codex_home)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
let next_config = Config::load_config_with_layer_stack(
merged_config,
let mut next_config = Config::load_config_with_layer_stack(
merged_config,
reload_overrides(config, preserve_current_provider),
config.codex_home.clone(),
config_layer_stack,
)?;
if preserve_current_profile {
next_config.active_profile = config.active_profile.clone();
}
Ok(next_config)
}
fn build_config_layer_stack(
config: &Config,
role_layer_toml: &TomlValue,
active_profile_name: Option<&str>,
) -> anyhow::Result<ConfigLayerStack> {
let mut layers = existing_layers(config);
if let Some(resolved_profile_layer) =
resolved_profile_layer(config, &layers, role_layer_toml, active_profile_name)?
{
insert_layer(&mut layers, resolved_profile_layer);
}
insert_layer(&mut layers, role_layer(role_layer_toml.clone()));
Ok(ConfigLayerStack::new(
layers,
config.config_layer_stack.requirements().clone(),
config.config_layer_stack.requirements_toml().clone(),
)?)
}
fn resolved_profile_layer(
config: &Config,
existing_layers: &[ConfigLayerEntry],
role_layer_toml: &TomlValue,
active_profile_name: Option<&str>,
) -> anyhow::Result<Option<ConfigLayerEntry>> {
let Some(active_profile_name) = active_profile_name else {
return Ok(None);
};
let mut layers = existing_layers.to_vec();
insert_layer(&mut layers, role_layer(role_layer_toml.clone()));
let merged_config = deserialize_effective_config(
config,
&ConfigLayerStack::new(
layers,
config.config_layer_stack.requirements().clone(),
config.config_layer_stack.requirements_toml().clone(),
)?,
)?;
let resolved_profile =
merged_config.get_config_profile(Some(active_profile_name.to_string()))?;
Ok(Some(ConfigLayerEntry::new(
ConfigLayerSource::SessionFlags,
TomlValue::try_from(resolved_profile)?,
)))
}
fn deserialize_effective_config(
config: &Config,
config_layer_stack: &ConfigLayerStack,
) -> anyhow::Result<crate::config::ConfigToml> {
Ok(deserialize_config_toml_with_base(
config_layer_stack.effective_config(),
&config.codex_home,
)?)
}
fn existing_layers(config: &Config) -> Vec<ConfigLayerEntry> {
config
.config_layer_stack
.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true)
.into_iter()
.cloned()
.collect()
}
fn insert_layer(layers: &mut Vec<ConfigLayerEntry>, layer: ConfigLayerEntry) {
let insertion_index =
layers.partition_point(|existing_layer| existing_layer.name <= layer.name);
layers.insert(insertion_index, layer);
}
fn role_layer(role_layer_toml: TomlValue) -> ConfigLayerEntry {
ConfigLayerEntry::new(ConfigLayerSource::SessionFlags, role_layer_toml)
}
fn reload_overrides(config: &Config, preserve_current_provider: bool) -> ConfigOverrides {
ConfigOverrides {
cwd: Some(config.cwd.clone()),
model_provider: preserve_current_provider.then(|| config.model_provider_id.clone()),
config_profile: preserve_current_profile
.then(|| config.active_profile.clone())
.flatten(),
codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(),
main_execve_wrapper_exe: config.main_execve_wrapper_exe.clone(),
js_repl_node_path: config.js_repl_node_path.clone(),
..Default::default()
},
config.codex_home.clone(),
config_layer_stack,
)
.map_err(|_| AGENT_TYPE_UNAVAILABLE_ERROR.to_string())?;
*config = next_config;
Ok(())
}
pub(crate) fn resolve_role_config<'a>(
config: &'a Config,
role_name: &str,
) -> Option<&'a AgentRoleConfig> {
config
.agent_roles
.get(role_name)
.or_else(|| built_in::configs().get(role_name))
}
}
}
pub(crate) mod spawn_tool_spec {

61
codex-rs/core/src/agent/role_tests.rs
View file

@ -4,6 +4,8 @@ use crate::config::ConfigBuilder;
use crate::config_loader::ConfigLayerStackOrdering;
use crate::plugins::PluginsManager;
use crate::skills::SkillsManager;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::Verbosity;
use codex_protocol::openai_models::ReasoningEffort;
use pretty_assertions::assert_eq;
use std::fs;
@ -243,6 +245,65 @@ model_provider = "test-provider"
assert_eq!(config.model_provider.name, "Test Provider");
}
#[tokio::test]
async fn apply_role_top_level_profile_settings_override_preserved_profile() {
let home = TempDir::new().expect("create temp dir");
tokio::fs::write(
home.path().join(CONFIG_TOML_FILE),
r#"
[profiles.base-profile]
model = "profile-model"
model_reasoning_effort = "low"
model_reasoning_summary = "concise"
model_verbosity = "low"
"#,
)
.await
.expect("write config.toml");
let mut config = ConfigBuilder::default()
.codex_home(home.path().to_path_buf())
.harness_overrides(ConfigOverrides {
config_profile: Some("base-profile".to_string()),
..Default::default()
})
.fallback_cwd(Some(home.path().to_path_buf()))
.build()
.await
.expect("load config");
let role_path = write_role_config(
&home,
"top-level-profile-settings-role.toml",
r#"developer_instructions = "Stay focused"
model = "role-model"
model_reasoning_effort = "high"
model_reasoning_summary = "detailed"
model_verbosity = "high"
"#,
)
.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.active_profile.as_deref(), Some("base-profile"));
assert_eq!(config.model.as_deref(), Some("role-model"));
assert_eq!(config.model_reasoning_effort, Some(ReasoningEffort::High));
assert_eq!(
config.model_reasoning_summary,
Some(ReasoningSummary::Detailed)
);
assert_eq!(config.model_verbosity, Some(Verbosity::High));
}
#[tokio::test]
async fn apply_role_uses_role_profile_instead_of_current_profile() {
let home = TempDir::new().expect("create temp dir");