mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
codex: fix layered agent role metadata merge
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -3373,6 +3373,89 @@ model = "gpt-5"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn higher_precedence_agent_role_can_inherit_description_from_lower_layer()
|
||||
-> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let repo_root = TempDir::new()?;
|
||||
let nested_cwd = repo_root.path().join("packages").join("app");
|
||||
std::fs::create_dir_all(repo_root.path().join(".git"))?;
|
||||
std::fs::create_dir_all(&nested_cwd)?;
|
||||
|
||||
let workspace_key = repo_root.path().to_string_lossy().replace('\\', "\\\\");
|
||||
tokio::fs::write(
|
||||
codex_home.path().join(CONFIG_TOML_FILE),
|
||||
format!(
|
||||
r#"[projects."{workspace_key}"]
|
||||
trust_level = "trusted"
|
||||
|
||||
[agents.researcher]
|
||||
description = "Research role from config"
|
||||
config_file = "./agents/researcher.toml"
|
||||
"#
|
||||
),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let home_agents_dir = codex_home.path().join("agents");
|
||||
tokio::fs::create_dir_all(&home_agents_dir).await?;
|
||||
tokio::fs::write(
|
||||
home_agents_dir.join("researcher.toml"),
|
||||
r#"
|
||||
developer_instructions = "Research carefully"
|
||||
model = "gpt-5"
|
||||
"#,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let standalone_agents_dir = repo_root.path().join(".codex").join("agents");
|
||||
tokio::fs::create_dir_all(&standalone_agents_dir).await?;
|
||||
tokio::fs::write(
|
||||
standalone_agents_dir.join("researcher.toml"),
|
||||
r#"
|
||||
name = "researcher"
|
||||
nickname_candidates = ["Hypatia"]
|
||||
developer_instructions = "Research from file"
|
||||
model = "gpt-5-mini"
|
||||
"#,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.harness_overrides(ConfigOverrides {
|
||||
cwd: Some(nested_cwd),
|
||||
..Default::default()
|
||||
})
|
||||
.build()
|
||||
.await?;
|
||||
|
||||
assert_eq!(
|
||||
config
|
||||
.agent_roles
|
||||
.get("researcher")
|
||||
.and_then(|role| role.description.as_deref()),
|
||||
Some("Research role from config")
|
||||
);
|
||||
assert_eq!(
|
||||
config
|
||||
.agent_roles
|
||||
.get("researcher")
|
||||
.and_then(|role| role.config_file.as_ref()),
|
||||
Some(&standalone_agents_dir.join("researcher.toml"))
|
||||
);
|
||||
assert_eq!(
|
||||
config
|
||||
.agent_roles
|
||||
.get("researcher")
|
||||
.and_then(|role| role.nickname_candidates.as_ref())
|
||||
.map(|candidates| candidates.iter().map(String::as_str).collect::<Vec<_>>()),
|
||||
Some(vec!["Hypatia"])
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_config_normalizes_agent_role_nickname_candidates() -> std::io::Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
@@ -2681,9 +2681,9 @@ impl Config {
|
||||
return Ok(roles);
|
||||
}
|
||||
|
||||
let mut roles = BTreeMap::new();
|
||||
let mut roles: BTreeMap<String, AgentRoleConfig> = BTreeMap::new();
|
||||
for layer in layers {
|
||||
let mut layer_roles = BTreeMap::new();
|
||||
let mut layer_roles: BTreeMap<String, AgentRoleConfig> = BTreeMap::new();
|
||||
if let Some(agents_toml) = Self::agents_toml_from_layer(&layer.config)? {
|
||||
for (declared_role_name, role_toml) in &agents_toml.roles {
|
||||
let mut role =
|
||||
@@ -2699,10 +2699,13 @@ impl Config {
|
||||
role.nickname_candidates =
|
||||
parsed.nickname_candidates.or(role.nickname_candidates);
|
||||
}
|
||||
Self::validate_required_agent_role_description(
|
||||
&role_name,
|
||||
role.description.as_deref(),
|
||||
)?;
|
||||
if let Some(existing_role) = layer_roles.get(&role_name) {
|
||||
role.description = role.description.or(existing_role.description.clone());
|
||||
role.config_file = role.config_file.or(existing_role.config_file.clone());
|
||||
role.nickname_candidates = role
|
||||
.nickname_candidates
|
||||
.or(existing_role.nickname_candidates.clone());
|
||||
}
|
||||
|
||||
if layer_roles.insert(role_name.clone(), role).is_some() {
|
||||
return Err(std::io::Error::new(
|
||||
@@ -2724,7 +2727,23 @@ impl Config {
|
||||
}
|
||||
|
||||
for (role_name, role) in layer_roles {
|
||||
roles.insert(role_name, role);
|
||||
let mut merged_role = role;
|
||||
if let Some(existing_role) = roles.get(&role_name) {
|
||||
merged_role.description = merged_role
|
||||
.description
|
||||
.or(existing_role.description.clone());
|
||||
merged_role.config_file = merged_role
|
||||
.config_file
|
||||
.or(existing_role.config_file.clone());
|
||||
merged_role.nickname_candidates = merged_role
|
||||
.nickname_candidates
|
||||
.or(existing_role.nickname_candidates.clone());
|
||||
}
|
||||
Self::validate_required_agent_role_description(
|
||||
&role_name,
|
||||
merged_role.description.as_deref(),
|
||||
)?;
|
||||
roles.insert(role_name, merged_role);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2782,10 +2801,6 @@ impl Config {
|
||||
Err(err) => return Err(err),
|
||||
};
|
||||
let role_name = parsed.role_name;
|
||||
Self::validate_required_agent_role_description(
|
||||
&role_name,
|
||||
parsed.description.as_deref(),
|
||||
)?;
|
||||
if roles
|
||||
.insert(
|
||||
role_name.clone(),
|
||||
|
||||
Reference in New Issue
Block a user