mirror of
https://github.com/openai/codex.git
synced 2026-05-04 21:32:21 +03:00
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 <noreply@openai.com>
This commit is contained in:
@@ -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<String>,
|
||||
}
|
||||
|
||||
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<String> {
|
||||
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;
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
)]);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user