From 2e154a35bc6df9239dff23b6552076fc2a8a49b1 Mon Sep 17 00:00:00 2001 From: gabec-openai Date: Wed, 4 Mar 2026 13:43:52 +0900 Subject: [PATCH] Add role-specific subagent nickname overrides (#13218) ## Summary - add `nickname_candidates` to agent role config - use role-specific nickname pools for spawned and resumed subagents - validate and schema-generate the new config surface ## Testing - `just fmt` - `just write-config-schema` - `just fix -p codex-core` - `cargo test -p codex-core` - `cargo test` --------- Co-authored-by: Codex --- codex-rs/core/config.schema.json | 7 + codex-rs/core/src/agent/control.rs | 76 +++++++++- codex-rs/core/src/agent/role.rs | 37 +++-- codex-rs/core/src/config/mod.rs | 222 +++++++++++++++++++++++++++++ 4 files changed, 330 insertions(+), 12 deletions(-) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 749f6de23b..71d624d2c0 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -20,6 +20,13 @@ "description": { "description": "Human-facing role documentation used in spawn tool guidance.", "type": "string" + }, + "nickname_candidates": { + "description": "Candidate nicknames for agents spawned with this role.", + "items": { + "type": "string" + }, + "type": "array" } }, "type": "object" diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index fa643ff3ce..e7547f9c9d 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -1,5 +1,7 @@ use crate::agent::AgentStatus; use crate::agent::guards::Guards; +use crate::agent::role::DEFAULT_ROLE_NAME; +use crate::agent::role::resolve_role_config; use crate::agent::status::is_final; use crate::error::CodexErr; use crate::error::Result as CodexResult; @@ -32,7 +34,7 @@ pub(crate) struct SpawnAgentOptions { pub(crate) fork_parent_spawn_call_id: Option, } -fn agent_nickname_list() -> Vec<&'static str> { +fn default_agent_nickname_list() -> Vec<&'static str> { AGENT_NAMES .lines() .map(str::trim) @@ -40,6 +42,23 @@ fn agent_nickname_list() -> Vec<&'static str> { .collect() } +fn agent_nickname_candidates( + config: &crate::config::Config, + role_name: Option<&str>, +) -> Vec { + let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME); + if let Some(candidates) = + resolve_role_config(config, role_name).and_then(|role| role.nickname_candidates.clone()) + { + return candidates; + } + + default_agent_nickname_list() + .into_iter() + .map(ToOwned::to_owned) + .collect() +} + /// Control-plane handle for multi-agent operations. /// `AgentControl` is held by each session (via `SessionServices`). It provides capability to /// spawn new agents and the inter-agent communication layer. @@ -94,7 +113,10 @@ impl AgentControl { agent_role, .. })) => { - let agent_nickname = reservation.reserve_agent_nickname(&agent_nickname_list())?; + let candidate_names = agent_nickname_candidates(&config, agent_role.as_deref()); + let candidate_name_refs: Vec<&str> = + candidate_names.iter().map(String::as_str).collect(); + let agent_nickname = reservation.reserve_agent_nickname(&candidate_name_refs)?; Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { parent_thread_id, depth, @@ -225,8 +247,12 @@ impl AgentControl { let reserved_agent_nickname = resumed_agent_nickname .as_deref() .map(|agent_nickname| { + let candidate_names = + agent_nickname_candidates(&config, resumed_agent_role.as_deref()); + let candidate_name_refs: Vec<&str> = + candidate_names.iter().map(String::as_str).collect(); reservation.reserve_agent_nickname_with_preference( - &agent_nickname_list(), + &candidate_name_refs, Some(agent_nickname), ) }) @@ -467,6 +493,7 @@ mod tests { use crate::CodexThread; use crate::ThreadManager; use crate::agent::agent_status_from_event; + use crate::config::AgentRoleConfig; use crate::config::Config; use crate::config::ConfigBuilder; use crate::config_loader::LoaderOverrides; @@ -1378,6 +1405,49 @@ mod tests { assert_eq!(agent_role, Some("explorer".to_string())); } + #[tokio::test] + async fn spawn_thread_subagent_uses_role_specific_nickname_candidates() { + let mut harness = AgentControlHarness::new().await; + harness.config.agent_roles.insert( + "researcher".to_string(), + AgentRoleConfig { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(vec!["Atlas".to_string()]), + }, + ); + let (parent_thread_id, _parent_thread) = harness.start_thread().await; + + let child_thread_id = harness + .control + .spawn_agent( + harness.config.clone(), + text_input("hello child"), + Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn { + parent_thread_id, + depth: 1, + agent_nickname: None, + agent_role: Some("researcher".to_string()), + })), + ) + .await + .expect("child spawn should succeed"); + + let child_thread = harness + .manager + .get_thread(child_thread_id) + .await + .expect("child thread should be registered"); + let snapshot = child_thread.config_snapshot().await; + + let SessionSource::SubAgent(SubAgentSource::ThreadSpawn { agent_nickname, .. }) = + snapshot.session_source + else { + panic!("expected thread-spawn sub-agent source"); + }; + assert_eq!(agent_nickname, Some("Atlas".to_string())); + } + #[tokio::test] async fn resume_thread_subagent_restores_stored_nickname_and_role() { let (home, mut config) = test_config().await; diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index 3ca496d6e1..0793febe54 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -38,15 +38,9 @@ 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 (config_file, is_built_in) = config - .agent_roles - .get(role_name) - .map(|role| (&role.config_file, false)) - .or_else(|| { - built_in::configs() - .get(role_name) - .map(|role| (&role.config_file, true)) - }) + 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)) .ok_or_else(|| format!("unknown agent_type '{role_name}'"))?; let Some(config_file) = config_file.as_ref() else { return Ok(()); @@ -139,6 +133,16 @@ pub(crate) async fn apply_role_to_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 { use super::*; @@ -196,6 +200,7 @@ mod built_in { AgentRoleConfig { description: Some("Default agent.".to_string()), config_file: None, + nickname_candidates: None, } ), ( @@ -210,6 +215,7 @@ Rules: - Run explorers in parallel when useful. - Reuse existing explorers for related questions."#.to_string()), config_file: Some("explorer.toml".to_string().parse().unwrap_or_default()), + nickname_candidates: None, } ), ( @@ -224,6 +230,7 @@ Rules: - Explicitly assign **ownership** of the task (files / responsibility). - Always tell workers they are **not alone in the codebase**, and they should ignore edits made by others without touching them."#.to_string()), config_file: None, + nickname_candidates: None, } ), // Awaiter is temp removed @@ -354,6 +361,7 @@ mod tests { AgentRoleConfig { description: None, config_file: Some(PathBuf::from("/path/does/not/exist.toml")), + nickname_candidates: None, }, ); @@ -373,6 +381,7 @@ mod tests { AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -403,6 +412,7 @@ mod tests { AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -456,6 +466,7 @@ model_provider = "test-provider" AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -512,6 +523,7 @@ model_provider = "role-provider" AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -569,6 +581,7 @@ model_provider = "base-provider" AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -630,6 +643,7 @@ model_reasoning_effort = "high" AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -671,6 +685,7 @@ writable_roots = ["./sandbox-root"] AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -724,6 +739,7 @@ writable_roots = ["./sandbox-root"] AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -764,6 +780,7 @@ enabled = false AgentRoleConfig { description: None, config_file: Some(role_path), + nickname_candidates: None, }, ); @@ -791,6 +808,7 @@ enabled = false AgentRoleConfig { description: Some("user override".to_string()), config_file: None, + nickname_candidates: None, }, ), ("researcher".to_string(), AgentRoleConfig::default()), @@ -811,6 +829,7 @@ enabled = false AgentRoleConfig { description: Some("first".to_string()), config_file: None, + nickname_candidates: None, }, )]); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 3b71f8fac4..2d833d8b51 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -78,6 +78,7 @@ use serde::Deserialize; use serde::Serialize; use similar::DiffableStr; use std::collections::BTreeMap; +use std::collections::BTreeSet; use std::collections::HashMap; use std::io::ErrorKind; use std::path::Path; @@ -1385,6 +1386,7 @@ pub struct AgentsToml { /// [agents.researcher] /// description = "Research-focused role." /// config_file = "./agents/researcher.toml" + /// nickname_candidates = ["Herodotus", "Ibn Battuta"] /// ``` #[serde(default, flatten)] pub roles: BTreeMap, @@ -1396,6 +1398,8 @@ pub struct AgentRoleConfig { pub description: Option, /// Path to a role-specific config layer. pub config_file: Option, + /// Candidate nicknames for agents spawned with this role. + pub nickname_candidates: Option>, } #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] @@ -1407,6 +1411,9 @@ pub struct AgentRoleToml { /// Path to a role-specific config layer. /// Relative paths are resolved relative to the `config.toml` that defines them. pub config_file: Option, + + /// Candidate nicknames for agents spawned with this role. + pub nickname_candidates: Option>, } impl From for Tools { @@ -1895,11 +1902,16 @@ impl Config { let config_file = role.config_file.as_ref().map(AbsolutePathBuf::to_path_buf); Self::validate_agent_role_config_file(name, config_file.as_deref())?; + let nickname_candidates = Self::normalize_agent_role_nickname_candidates( + name, + role.nickname_candidates.as_deref(), + )?; Ok(( name.clone(), AgentRoleConfig { description: role.description.clone(), config_file, + nickname_candidates, }, )) }) @@ -2361,6 +2373,58 @@ impl Config { } } + fn normalize_agent_role_nickname_candidates( + role_name: &str, + nickname_candidates: Option<&[String]>, + ) -> std::io::Result>> { + let Some(nickname_candidates) = nickname_candidates else { + return Ok(None); + }; + + if nickname_candidates.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("agents.{role_name}.nickname_candidates must contain at least one name"), + )); + } + + let mut normalized_candidates = Vec::with_capacity(nickname_candidates.len()); + let mut seen_candidates = BTreeSet::new(); + + for nickname in nickname_candidates { + let normalized_nickname = nickname.trim(); + if normalized_nickname.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("agents.{role_name}.nickname_candidates cannot contain blank names"), + )); + } + + if !seen_candidates.insert(normalized_nickname.to_owned()) { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("agents.{role_name}.nickname_candidates cannot contain duplicates"), + )); + } + + if !normalized_nickname + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, ' ' | '-' | '_')) + { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!( + "agents.{role_name}.nickname_candidates may only contain ASCII letters, digits, spaces, hyphens, and underscores" + ), + )); + } + + normalized_candidates.push(normalized_nickname.to_owned()); + } + + Ok(Some(normalized_candidates)) + } + pub fn set_windows_sandbox_enabled(&mut self, value: bool) { self.permissions.windows_sandbox_mode = if value { Some(WindowsSandboxModeToml::Unelevated) @@ -4662,6 +4726,7 @@ model = "gpt-5.1-codex" AgentRoleToml { description: Some("Research role".to_string()), config_file: Some(AbsolutePathBuf::from_absolute_path(missing_path)?), + nickname_candidates: None, }, )]), }), @@ -4698,6 +4763,7 @@ model = "gpt-5.1-codex" r#"[agents.researcher] description = "Research role" config_file = "./agents/researcher.toml" +nickname_candidates = ["Hypatia", "Noether"] "#, ) .await?; @@ -4714,6 +4780,162 @@ config_file = "./agents/researcher.toml" .and_then(|role| role.config_file.as_ref()), Some(&role_config_path) ); + assert_eq!( + config + .agent_roles + .get("researcher") + .and_then(|role| role.nickname_candidates.as_ref()) + .map(|candidates| candidates.iter().map(String::as_str).collect::>()), + Some(vec!["Hypatia", "Noether"]) + ); + + Ok(()) + } + + #[test] + fn load_config_normalizes_agent_role_nickname_candidates() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + agents: Some(AgentsToml { + max_threads: None, + max_depth: None, + job_max_runtime_seconds: None, + roles: BTreeMap::from([( + "researcher".to_string(), + AgentRoleToml { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(vec![ + " Hypatia ".to_string(), + "Noether".to_string(), + ]), + }, + )]), + }), + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + + assert_eq!( + config + .agent_roles + .get("researcher") + .and_then(|role| role.nickname_candidates.as_ref()) + .map(|candidates| candidates.iter().map(String::as_str).collect::>()), + Some(vec!["Hypatia", "Noether"]) + ); + + Ok(()) + } + + #[test] + fn load_config_rejects_empty_agent_role_nickname_candidates() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + agents: Some(AgentsToml { + max_threads: None, + max_depth: None, + job_max_runtime_seconds: None, + roles: BTreeMap::from([( + "researcher".to_string(), + AgentRoleToml { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(Vec::new()), + }, + )]), + }), + ..Default::default() + }; + + let result = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + ); + let err = result.expect_err("empty nickname candidates should be rejected"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + err.to_string() + .contains("agents.researcher.nickname_candidates") + ); + + Ok(()) + } + + #[test] + fn load_config_rejects_duplicate_agent_role_nickname_candidates() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + agents: Some(AgentsToml { + max_threads: None, + max_depth: None, + job_max_runtime_seconds: None, + roles: BTreeMap::from([( + "researcher".to_string(), + AgentRoleToml { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(vec![ + "Hypatia".to_string(), + " Hypatia ".to_string(), + ]), + }, + )]), + }), + ..Default::default() + }; + + let result = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + ); + let err = result.expect_err("duplicate nickname candidates should be rejected"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + err.to_string() + .contains("agents.researcher.nickname_candidates cannot contain duplicates") + ); + + Ok(()) + } + + #[test] + fn load_config_rejects_unsafe_agent_role_nickname_candidates() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + agents: Some(AgentsToml { + max_threads: None, + max_depth: None, + job_max_runtime_seconds: None, + roles: BTreeMap::from([( + "researcher".to_string(), + AgentRoleToml { + description: Some("Research role".to_string()), + config_file: None, + nickname_candidates: Some(vec!["Agent ".to_string()]), + }, + )]), + }), + ..Default::default() + }; + + let result = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + ); + let err = result.expect_err("unsafe nickname candidates should be rejected"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!(err.to_string().contains( + "agents.researcher.nickname_candidates may only contain ASCII letters, digits, spaces, hyphens, and underscores" + )); Ok(()) }