mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: add nick name to sub-agents (#12320)
Adding random nick name to sub-agents. Used for UX At the same time, also storing and wiring the role of the sub-agent
This commit is contained in:
committed by
GitHub
Unverified
parent
03ff04cd65
commit
0f9eed3a6f
@@ -0,0 +1,100 @@
|
||||
Atlas
|
||||
Nova
|
||||
Orion
|
||||
Iris
|
||||
Milo
|
||||
Juno
|
||||
Mars
|
||||
Vesta
|
||||
Luna
|
||||
Sol
|
||||
Eros
|
||||
Nyx
|
||||
Zeus
|
||||
Hera
|
||||
Ares
|
||||
Ajax
|
||||
Echo
|
||||
Leto
|
||||
Rhea
|
||||
Gaia
|
||||
Hades
|
||||
Apollo
|
||||
Pallas
|
||||
Circe
|
||||
Nereus
|
||||
Triton
|
||||
Selene
|
||||
Helios
|
||||
Castor
|
||||
Pollux
|
||||
Astra
|
||||
Aura
|
||||
Thalia
|
||||
Clio
|
||||
Erato
|
||||
Euterpe
|
||||
Urania
|
||||
Calliope
|
||||
Minos
|
||||
Linus
|
||||
Cato
|
||||
Brutus
|
||||
Seneca
|
||||
Ovid
|
||||
Virgil
|
||||
Horace
|
||||
Remus
|
||||
Romulus
|
||||
Titus
|
||||
Lucian
|
||||
Felix
|
||||
Maximus
|
||||
Octavia
|
||||
Claudia
|
||||
Livia
|
||||
Aelia
|
||||
Aurel
|
||||
Cassia
|
||||
Sabina
|
||||
Flavia
|
||||
Numa
|
||||
Ceres
|
||||
Diana
|
||||
Venus
|
||||
Pluto
|
||||
Pan
|
||||
Ramius
|
||||
Aether
|
||||
Chaos
|
||||
Logos
|
||||
Ethos
|
||||
Tethys
|
||||
Chiron
|
||||
Talos
|
||||
Icarus
|
||||
Daedalus
|
||||
Hyacinth
|
||||
Adonis
|
||||
Perseus
|
||||
Theseus
|
||||
Argos
|
||||
Lemnos
|
||||
Delos
|
||||
Sparta
|
||||
Attica
|
||||
Corinth
|
||||
Thebes
|
||||
Ephyra
|
||||
Achaea
|
||||
Cyrene
|
||||
Sidon
|
||||
Tyre
|
||||
Ilium
|
||||
Etrus
|
||||
Vercos
|
||||
Aurex
|
||||
Novian
|
||||
Helion
|
||||
Casson
|
||||
Aurelix
|
||||
@@ -3,7 +3,9 @@ use crate::agent::guards::Guards;
|
||||
use crate::agent::status::is_final;
|
||||
use crate::error::CodexErr;
|
||||
use crate::error::Result as CodexResult;
|
||||
use crate::find_thread_path_by_id_str;
|
||||
use crate::session_prefix::format_subagent_notification_message;
|
||||
use crate::state_db;
|
||||
use crate::thread_manager::ThreadManagerState;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::Op;
|
||||
@@ -11,11 +13,20 @@ use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::TokenUsage;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Weak;
|
||||
use tokio::sync::watch;
|
||||
|
||||
const AGENT_NAMES: &str = include_str!("agent_names.txt");
|
||||
|
||||
fn agent_nickname_list() -> Vec<&'static str> {
|
||||
AGENT_NAMES
|
||||
.lines()
|
||||
.map(str::trim)
|
||||
.filter(|name| !name.is_empty())
|
||||
.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.
|
||||
@@ -48,7 +59,24 @@ impl AgentControl {
|
||||
session_source: Option<SessionSource>,
|
||||
) -> CodexResult<ThreadId> {
|
||||
let state = self.upgrade()?;
|
||||
let reservation = self.state.reserve_spawn_slot(config.agent_max_threads)?;
|
||||
let mut reservation = self.state.reserve_spawn_slot(config.agent_max_threads)?;
|
||||
let session_source = match session_source {
|
||||
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth,
|
||||
agent_role,
|
||||
..
|
||||
})) => {
|
||||
let agent_nickname = reservation.reserve_agent_nickname(&agent_nickname_list())?;
|
||||
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth,
|
||||
agent_nickname: Some(agent_nickname),
|
||||
agent_role,
|
||||
}))
|
||||
}
|
||||
other => other,
|
||||
};
|
||||
let notification_source = session_source.clone();
|
||||
|
||||
// The same `AgentControl` is sent to spawn the thread.
|
||||
@@ -77,12 +105,51 @@ impl AgentControl {
|
||||
pub(crate) async fn resume_agent_from_rollout(
|
||||
&self,
|
||||
config: crate::config::Config,
|
||||
rollout_path: PathBuf,
|
||||
thread_id: ThreadId,
|
||||
session_source: SessionSource,
|
||||
) -> CodexResult<ThreadId> {
|
||||
let state = self.upgrade()?;
|
||||
let reservation = self.state.reserve_spawn_slot(config.agent_max_threads)?;
|
||||
let mut reservation = self.state.reserve_spawn_slot(config.agent_max_threads)?;
|
||||
let session_source = match session_source {
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth,
|
||||
..
|
||||
}) => {
|
||||
// Collab resume callers rebuild a placeholder ThreadSpawn source. Rehydrate the
|
||||
// stored nickname/role from sqlite when available; otherwise leave both unset.
|
||||
let (resumed_agent_nickname, resumed_agent_role) =
|
||||
if let Some(state_db_ctx) = state_db::get_state_db(&config, None).await {
|
||||
match state_db_ctx.get_thread(thread_id).await {
|
||||
Ok(Some(metadata)) => (metadata.agent_nickname, metadata.agent_role),
|
||||
Ok(None) | Err(_) => (None, None),
|
||||
}
|
||||
} else {
|
||||
(None, None)
|
||||
};
|
||||
let reserved_agent_nickname = resumed_agent_nickname
|
||||
.as_deref()
|
||||
.map(|agent_nickname| {
|
||||
reservation.reserve_agent_nickname_with_preference(
|
||||
&agent_nickname_list(),
|
||||
Some(agent_nickname),
|
||||
)
|
||||
})
|
||||
.transpose()?;
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth,
|
||||
agent_nickname: reserved_agent_nickname,
|
||||
agent_role: resumed_agent_role,
|
||||
})
|
||||
}
|
||||
other => other,
|
||||
};
|
||||
let notification_source = session_source.clone();
|
||||
let rollout_path =
|
||||
find_thread_path_by_id_str(config.codex_home.as_path(), &thread_id.to_string())
|
||||
.await?
|
||||
.ok_or_else(|| CodexErr::ThreadNotFound(thread_id))?;
|
||||
|
||||
let resumed_thread = state
|
||||
.resume_thread_from_rollout_with_source(
|
||||
@@ -234,6 +301,8 @@ mod tests {
|
||||
use crate::agent::agent_status_from_event;
|
||||
use crate::config::Config;
|
||||
use crate::config::ConfigBuilder;
|
||||
use crate::config_loader::LoaderOverrides;
|
||||
use crate::features::Feature;
|
||||
use crate::session_prefix::SUBAGENT_NOTIFICATION_OPEN_TAG;
|
||||
use assert_matches::assert_matches;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
@@ -261,6 +330,12 @@ mod tests {
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(home.path().to_path_buf())
|
||||
.cli_overrides(cli_overrides)
|
||||
.loader_overrides(LoaderOverrides {
|
||||
#[cfg(target_os = "macos")]
|
||||
managed_preferences_base64: Some(String::new()),
|
||||
macos_managed_config_requirements_base64: Some(String::new()),
|
||||
..LoaderOverrides::default()
|
||||
})
|
||||
.build()
|
||||
.await
|
||||
.expect("load default test config");
|
||||
@@ -441,11 +516,7 @@ mod tests {
|
||||
let control = AgentControl::default();
|
||||
let (_home, config) = test_config().await;
|
||||
let err = control
|
||||
.resume_agent_from_rollout(
|
||||
config,
|
||||
PathBuf::from("/tmp/missing-rollout.jsonl"),
|
||||
SessionSource::Exec,
|
||||
)
|
||||
.resume_agent_from_rollout(config, ThreadId::new(), SessionSource::Exec)
|
||||
.await
|
||||
.expect_err("resume_agent should fail without a manager");
|
||||
assert_eq!(
|
||||
@@ -717,12 +788,6 @@ mod tests {
|
||||
.spawn_agent(config.clone(), text_input("hello"), None)
|
||||
.await
|
||||
.expect("spawn_agent should succeed");
|
||||
let rollout_path = manager
|
||||
.get_thread(resumable_id)
|
||||
.await
|
||||
.expect("thread should exist")
|
||||
.rollout_path()
|
||||
.expect("rollout path should exist");
|
||||
let _ = control
|
||||
.shutdown_agent(resumable_id)
|
||||
.await
|
||||
@@ -734,7 +799,7 @@ mod tests {
|
||||
.expect("spawn_agent should succeed for active slot");
|
||||
|
||||
let err = control
|
||||
.resume_agent_from_rollout(config, rollout_path, SessionSource::Exec)
|
||||
.resume_agent_from_rollout(config, resumable_id, SessionSource::Exec)
|
||||
.await
|
||||
.expect_err("resume should respect max threads");
|
||||
let CodexErr::AgentLimitReached {
|
||||
@@ -766,9 +831,8 @@ mod tests {
|
||||
);
|
||||
let control = manager.agent_control();
|
||||
|
||||
let missing_rollout = config.codex_home.join("sessions/missing-rollout.jsonl");
|
||||
let _ = control
|
||||
.resume_agent_from_rollout(config.clone(), missing_rollout, SessionSource::Exec)
|
||||
.resume_agent_from_rollout(config.clone(), ThreadId::new(), SessionSource::Exec)
|
||||
.await
|
||||
.expect_err("resume should fail for missing rollout path");
|
||||
|
||||
@@ -795,6 +859,8 @@ mod tests {
|
||||
Some(SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth: 1,
|
||||
agent_nickname: None,
|
||||
agent_role: Some("explorer".to_string()),
|
||||
})),
|
||||
)
|
||||
.await
|
||||
@@ -812,4 +878,176 @@ mod tests {
|
||||
|
||||
assert_eq!(wait_for_subagent_notification(&parent_thread).await, true);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn spawn_thread_subagent_gets_random_nickname_in_session_source() {
|
||||
let harness = AgentControlHarness::new().await;
|
||||
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("explorer".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 {
|
||||
parent_thread_id: seen_parent_thread_id,
|
||||
depth,
|
||||
agent_nickname,
|
||||
agent_role,
|
||||
}) = snapshot.session_source
|
||||
else {
|
||||
panic!("expected thread-spawn sub-agent source");
|
||||
};
|
||||
assert_eq!(seen_parent_thread_id, parent_thread_id);
|
||||
assert_eq!(depth, 1);
|
||||
assert!(agent_nickname.is_some());
|
||||
assert_eq!(agent_role, Some("explorer".to_string()));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resume_thread_subagent_restores_stored_nickname_and_role() {
|
||||
let (home, mut config) = test_config().await;
|
||||
config.features.enable(Feature::Sqlite);
|
||||
let manager = ThreadManager::with_models_provider_and_home_for_tests(
|
||||
CodexAuth::from_api_key("dummy"),
|
||||
config.model_provider.clone(),
|
||||
config.codex_home.clone(),
|
||||
);
|
||||
let control = manager.agent_control();
|
||||
let harness = AgentControlHarness {
|
||||
_home: home,
|
||||
config,
|
||||
manager,
|
||||
control,
|
||||
};
|
||||
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("explorer".to_string()),
|
||||
})),
|
||||
)
|
||||
.await
|
||||
.expect("child spawn should succeed");
|
||||
|
||||
let child_thread = harness
|
||||
.manager
|
||||
.get_thread(child_thread_id)
|
||||
.await
|
||||
.expect("child thread should exist");
|
||||
let mut status_rx = harness
|
||||
.control
|
||||
.subscribe_status(child_thread_id)
|
||||
.await
|
||||
.expect("status subscription should succeed");
|
||||
if matches!(status_rx.borrow().clone(), AgentStatus::PendingInit) {
|
||||
timeout(Duration::from_secs(5), async {
|
||||
loop {
|
||||
status_rx
|
||||
.changed()
|
||||
.await
|
||||
.expect("child status should advance past pending init");
|
||||
if !matches!(status_rx.borrow().clone(), AgentStatus::PendingInit) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
})
|
||||
.await
|
||||
.expect("child should initialize before shutdown");
|
||||
}
|
||||
let original_snapshot = child_thread.config_snapshot().await;
|
||||
let original_nickname = original_snapshot
|
||||
.session_source
|
||||
.get_nickname()
|
||||
.expect("spawned sub-agent should have a nickname");
|
||||
let state_db = child_thread
|
||||
.state_db()
|
||||
.expect("sqlite state db should be available for nickname resume test");
|
||||
timeout(Duration::from_secs(5), async {
|
||||
loop {
|
||||
if let Ok(Some(metadata)) = state_db.get_thread(child_thread_id).await
|
||||
&& metadata.agent_nickname.is_some()
|
||||
&& metadata.agent_role.as_deref() == Some("explorer")
|
||||
{
|
||||
break;
|
||||
}
|
||||
sleep(Duration::from_millis(10)).await;
|
||||
}
|
||||
})
|
||||
.await
|
||||
.expect("child thread metadata should be persisted to sqlite before shutdown");
|
||||
|
||||
let _ = harness
|
||||
.control
|
||||
.shutdown_agent(child_thread_id)
|
||||
.await
|
||||
.expect("child shutdown should submit");
|
||||
|
||||
let resumed_thread_id = harness
|
||||
.control
|
||||
.resume_agent_from_rollout(
|
||||
harness.config.clone(),
|
||||
child_thread_id,
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth: 1,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
}),
|
||||
)
|
||||
.await
|
||||
.expect("resume should succeed");
|
||||
assert_eq!(resumed_thread_id, child_thread_id);
|
||||
|
||||
let resumed_snapshot = harness
|
||||
.manager
|
||||
.get_thread(resumed_thread_id)
|
||||
.await
|
||||
.expect("resumed child thread should exist")
|
||||
.config_snapshot()
|
||||
.await;
|
||||
let SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id: resumed_parent_thread_id,
|
||||
depth: resumed_depth,
|
||||
agent_nickname: resumed_nickname,
|
||||
agent_role: resumed_role,
|
||||
}) = resumed_snapshot.session_source
|
||||
else {
|
||||
panic!("expected thread-spawn sub-agent source");
|
||||
};
|
||||
assert_eq!(resumed_parent_thread_id, parent_thread_id);
|
||||
assert_eq!(resumed_depth, 1);
|
||||
assert_eq!(resumed_nickname, Some(original_nickname));
|
||||
assert_eq!(resumed_role, Some("explorer".to_string()));
|
||||
|
||||
let _ = harness
|
||||
.control
|
||||
.shutdown_agent(resumed_thread_id)
|
||||
.await
|
||||
.expect("resumed child shutdown should submit");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,8 @@ use crate::error::Result;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use rand::prelude::IndexedRandom;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
@@ -17,10 +19,18 @@ use std::sync::atomic::Ordering;
|
||||
/// is).
|
||||
#[derive(Default)]
|
||||
pub(crate) struct Guards {
|
||||
threads_set: Mutex<HashSet<ThreadId>>,
|
||||
active_agents: Mutex<ActiveAgents>,
|
||||
total_count: AtomicUsize,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct ActiveAgents {
|
||||
threads_set: HashSet<ThreadId>,
|
||||
thread_agent_nicknames: HashMap<ThreadId, String>,
|
||||
used_agent_nicknames: HashSet<String>,
|
||||
nickname_reset_count: usize,
|
||||
}
|
||||
|
||||
fn session_depth(session_source: &SessionSource) -> i32 {
|
||||
match session_source {
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn { depth, .. }) => *depth,
|
||||
@@ -52,28 +62,69 @@ impl Guards {
|
||||
Ok(SpawnReservation {
|
||||
state: Arc::clone(self),
|
||||
active: true,
|
||||
reserved_agent_nickname: None,
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn release_spawned_thread(&self, thread_id: ThreadId) {
|
||||
let removed = {
|
||||
let mut threads = self
|
||||
.threads_set
|
||||
let mut active_agents = self
|
||||
.active_agents
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
threads.remove(&thread_id)
|
||||
let removed = active_agents.threads_set.remove(&thread_id);
|
||||
active_agents.thread_agent_nicknames.remove(&thread_id);
|
||||
removed
|
||||
};
|
||||
if removed {
|
||||
self.total_count.fetch_sub(1, Ordering::AcqRel);
|
||||
}
|
||||
}
|
||||
|
||||
fn register_spawned_thread(&self, thread_id: ThreadId) {
|
||||
let mut threads = self
|
||||
.threads_set
|
||||
fn register_spawned_thread(&self, thread_id: ThreadId, agent_nickname: Option<String>) {
|
||||
let mut active_agents = self
|
||||
.active_agents
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
threads.insert(thread_id);
|
||||
active_agents.threads_set.insert(thread_id);
|
||||
if let Some(agent_nickname) = agent_nickname {
|
||||
active_agents
|
||||
.used_agent_nicknames
|
||||
.insert(agent_nickname.clone());
|
||||
active_agents
|
||||
.thread_agent_nicknames
|
||||
.insert(thread_id, agent_nickname);
|
||||
}
|
||||
}
|
||||
|
||||
fn reserve_agent_nickname(&self, names: &[&str], preferred: Option<&str>) -> Option<String> {
|
||||
let mut active_agents = self
|
||||
.active_agents
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
let agent_nickname = if let Some(preferred) = preferred {
|
||||
preferred.to_string()
|
||||
} else {
|
||||
if names.is_empty() {
|
||||
return None;
|
||||
}
|
||||
let available_names: Vec<&str> = names
|
||||
.iter()
|
||||
.copied()
|
||||
.filter(|name| !active_agents.used_agent_nicknames.contains(*name))
|
||||
.collect();
|
||||
if let Some(name) = available_names.choose(&mut rand::rng()) {
|
||||
(*name).to_string()
|
||||
} else {
|
||||
active_agents.used_agent_nicknames.clear();
|
||||
active_agents.nickname_reset_count += 1;
|
||||
names.choose(&mut rand::rng())?.to_string()
|
||||
}
|
||||
};
|
||||
active_agents
|
||||
.used_agent_nicknames
|
||||
.insert(agent_nickname.clone());
|
||||
Some(agent_nickname)
|
||||
}
|
||||
|
||||
fn try_increment_spawned(&self, max_threads: usize) -> bool {
|
||||
@@ -98,11 +149,41 @@ impl Guards {
|
||||
pub(crate) struct SpawnReservation {
|
||||
state: Arc<Guards>,
|
||||
active: bool,
|
||||
reserved_agent_nickname: Option<String>,
|
||||
}
|
||||
|
||||
impl SpawnReservation {
|
||||
pub(crate) fn commit(mut self, thread_id: ThreadId) {
|
||||
self.state.register_spawned_thread(thread_id);
|
||||
pub(crate) fn reserve_agent_nickname(&mut self, names: &[&str]) -> Result<String> {
|
||||
self.reserve_agent_nickname_with_preference(names, None)
|
||||
}
|
||||
|
||||
pub(crate) fn reserve_agent_nickname_with_preference(
|
||||
&mut self,
|
||||
names: &[&str],
|
||||
preferred: Option<&str>,
|
||||
) -> Result<String> {
|
||||
let agent_nickname = self
|
||||
.state
|
||||
.reserve_agent_nickname(names, preferred)
|
||||
.ok_or_else(|| {
|
||||
CodexErr::UnsupportedOperation("no available agent nicknames".to_string())
|
||||
})?;
|
||||
self.reserved_agent_nickname = Some(agent_nickname.clone());
|
||||
Ok(agent_nickname)
|
||||
}
|
||||
|
||||
pub(crate) fn commit(self, thread_id: ThreadId) {
|
||||
self.commit_with_agent_nickname(thread_id, None);
|
||||
}
|
||||
|
||||
pub(crate) fn commit_with_agent_nickname(
|
||||
mut self,
|
||||
thread_id: ThreadId,
|
||||
agent_nickname: Option<String>,
|
||||
) {
|
||||
let agent_nickname = self.reserved_agent_nickname.take().or(agent_nickname);
|
||||
self.state
|
||||
.register_spawned_thread(thread_id, agent_nickname);
|
||||
self.active = false;
|
||||
}
|
||||
}
|
||||
@@ -130,6 +211,8 @@ mod tests {
|
||||
let session_source = SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id: ThreadId::new(),
|
||||
depth: 1,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
});
|
||||
let child_depth = next_thread_spawn_depth(&session_source);
|
||||
assert_eq!(child_depth, 2);
|
||||
@@ -232,4 +315,83 @@ mod tests {
|
||||
.expect("slot released after second thread removal");
|
||||
drop(reservation);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn failed_spawn_keeps_nickname_marked_used() {
|
||||
let guards = Arc::new(Guards::default());
|
||||
let mut reservation = guards.reserve_spawn_slot(None).expect("reserve slot");
|
||||
let agent_nickname = reservation
|
||||
.reserve_agent_nickname(&["alpha"])
|
||||
.expect("reserve agent name");
|
||||
assert_eq!(agent_nickname, "alpha");
|
||||
drop(reservation);
|
||||
|
||||
let mut reservation = guards.reserve_spawn_slot(None).expect("reserve slot");
|
||||
let agent_nickname = reservation
|
||||
.reserve_agent_nickname(&["alpha", "beta"])
|
||||
.expect("unused name should still be preferred");
|
||||
assert_eq!(agent_nickname, "beta");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_nickname_resets_used_pool_when_exhausted() {
|
||||
let guards = Arc::new(Guards::default());
|
||||
let mut first = guards.reserve_spawn_slot(None).expect("reserve first slot");
|
||||
let first_name = first
|
||||
.reserve_agent_nickname(&["alpha"])
|
||||
.expect("reserve first agent name");
|
||||
let first_id = ThreadId::new();
|
||||
first.commit(first_id);
|
||||
assert_eq!(first_name, "alpha");
|
||||
|
||||
let mut second = guards
|
||||
.reserve_spawn_slot(None)
|
||||
.expect("reserve second slot");
|
||||
let second_name = second
|
||||
.reserve_agent_nickname(&["alpha"])
|
||||
.expect("name should be reused after pool reset");
|
||||
assert_eq!(second_name, "alpha");
|
||||
let active_agents = guards
|
||||
.active_agents
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
assert_eq!(active_agents.nickname_reset_count, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn released_nickname_stays_used_until_pool_reset() {
|
||||
let guards = Arc::new(Guards::default());
|
||||
|
||||
let mut first = guards.reserve_spawn_slot(None).expect("reserve first slot");
|
||||
let first_name = first
|
||||
.reserve_agent_nickname(&["alpha"])
|
||||
.expect("reserve first agent name");
|
||||
let first_id = ThreadId::new();
|
||||
first.commit(first_id);
|
||||
assert_eq!(first_name, "alpha");
|
||||
|
||||
guards.release_spawned_thread(first_id);
|
||||
|
||||
let mut second = guards
|
||||
.reserve_spawn_slot(None)
|
||||
.expect("reserve second slot");
|
||||
let second_name = second
|
||||
.reserve_agent_nickname(&["alpha", "beta"])
|
||||
.expect("released name should still be marked used");
|
||||
assert_eq!(second_name, "beta");
|
||||
let second_id = ThreadId::new();
|
||||
second.commit(second_id);
|
||||
guards.release_spawned_thread(second_id);
|
||||
|
||||
let mut third = guards.reserve_spawn_slot(None).expect("reserve third slot");
|
||||
let third_name = third
|
||||
.reserve_agent_nickname(&["alpha", "beta"])
|
||||
.expect("pool reset should permit a duplicate");
|
||||
assert!(third_name == "alpha" || third_name == "beta");
|
||||
let active_agents = guards
|
||||
.active_agents
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
assert_eq!(active_agents.nickname_reset_count, 1);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -172,6 +172,8 @@ mod tests {
|
||||
originator: "test_originator".to_string(),
|
||||
cli_version: "test_version".to_string(),
|
||||
source: SessionSource::Cli,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: None,
|
||||
base_instructions: None,
|
||||
dynamic_tools: None,
|
||||
|
||||
@@ -39,7 +39,7 @@ pub struct ThreadsPage {
|
||||
}
|
||||
|
||||
/// Summary information for a thread rollout file.
|
||||
#[derive(Debug, PartialEq)]
|
||||
#[derive(Debug, PartialEq, Default)]
|
||||
pub struct ThreadItem {
|
||||
/// Absolute path to the rollout file.
|
||||
pub path: PathBuf,
|
||||
@@ -57,6 +57,10 @@ pub struct ThreadItem {
|
||||
pub git_origin_url: Option<String>,
|
||||
/// Session source from session metadata.
|
||||
pub source: Option<SessionSource>,
|
||||
/// Random unique nickname from session metadata for AgentControl-spawned sub-agents.
|
||||
pub agent_nickname: Option<String>,
|
||||
/// Role (agent_role) from session metadata for AgentControl-spawned sub-agents.
|
||||
pub agent_role: Option<String>,
|
||||
/// Model provider from session metadata.
|
||||
pub model_provider: Option<String>,
|
||||
/// CLI version from session metadata.
|
||||
@@ -87,6 +91,8 @@ struct HeadTailSummary {
|
||||
git_sha: Option<String>,
|
||||
git_origin_url: Option<String>,
|
||||
source: Option<SessionSource>,
|
||||
agent_nickname: Option<String>,
|
||||
agent_role: Option<String>,
|
||||
model_provider: Option<String>,
|
||||
cli_version: Option<String>,
|
||||
created_at: Option<String>,
|
||||
@@ -715,6 +721,8 @@ async fn build_thread_item(
|
||||
git_sha,
|
||||
git_origin_url,
|
||||
source,
|
||||
agent_nickname,
|
||||
agent_role,
|
||||
model_provider,
|
||||
cli_version,
|
||||
created_at,
|
||||
@@ -733,6 +741,8 @@ async fn build_thread_item(
|
||||
git_sha,
|
||||
git_origin_url,
|
||||
source,
|
||||
agent_nickname,
|
||||
agent_role,
|
||||
model_provider,
|
||||
cli_version,
|
||||
created_at,
|
||||
@@ -1017,6 +1027,8 @@ async fn read_head_summary(path: &Path, head_limit: usize) -> io::Result<HeadTai
|
||||
RolloutItem::SessionMeta(session_meta_line) => {
|
||||
if !summary.saw_session_meta {
|
||||
summary.source = Some(session_meta_line.meta.source.clone());
|
||||
summary.agent_nickname = session_meta_line.meta.agent_nickname.clone();
|
||||
summary.agent_role = session_meta_line.meta.agent_role.clone();
|
||||
summary.model_provider = session_meta_line.meta.model_provider.clone();
|
||||
summary.thread_id = Some(session_meta_line.meta.id);
|
||||
summary.cwd = Some(session_meta_line.meta.cwd.clone());
|
||||
|
||||
@@ -47,6 +47,8 @@ pub(crate) fn builder_from_session_meta(
|
||||
session_meta.meta.source.clone(),
|
||||
);
|
||||
builder.model_provider = session_meta.meta.model_provider.clone();
|
||||
builder.agent_nickname = session_meta.meta.agent_nickname.clone();
|
||||
builder.agent_role = session_meta.meta.agent_role.clone();
|
||||
builder.cwd = session_meta.meta.cwd.clone();
|
||||
builder.cli_version = Some(session_meta.meta.cli_version.clone());
|
||||
builder.sandbox_policy = SandboxPolicy::new_read_only_policy();
|
||||
@@ -512,6 +514,8 @@ mod tests {
|
||||
originator: "cli".to_string(),
|
||||
cli_version: "0.0.0".to_string(),
|
||||
source: SessionSource::default(),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some("openai".to_string()),
|
||||
base_instructions: None,
|
||||
dynamic_tools: None,
|
||||
@@ -660,6 +664,8 @@ mod tests {
|
||||
originator: "cli".to_string(),
|
||||
cli_version: "0.0.0".to_string(),
|
||||
source: SessionSource::default(),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some("test-provider".to_string()),
|
||||
base_instructions: None,
|
||||
dynamic_tools: None,
|
||||
|
||||
@@ -391,6 +391,8 @@ impl RolloutRecorder {
|
||||
cwd: config.cwd.clone(),
|
||||
originator: originator().value,
|
||||
cli_version: env!("CARGO_PKG_VERSION").to_string(),
|
||||
agent_nickname: source.get_nickname(),
|
||||
agent_role: source.get_agent_role(),
|
||||
source,
|
||||
model_provider: Some(config.model_provider_id.clone()),
|
||||
base_instructions: Some(base_instructions),
|
||||
@@ -929,9 +931,12 @@ impl From<codex_state::ThreadsPage> for ThreadsPage {
|
||||
git_sha: item.git_sha,
|
||||
git_origin_url: item.git_origin_url,
|
||||
source: Some(
|
||||
serde_json::from_value(Value::String(item.source))
|
||||
serde_json::from_str(item.source.as_str())
|
||||
.or_else(|_| serde_json::from_value(Value::String(item.source)))
|
||||
.unwrap_or(SessionSource::Unknown),
|
||||
),
|
||||
agent_nickname: item.agent_nickname,
|
||||
agent_role: item.agent_role,
|
||||
model_provider: Some(item.model_provider),
|
||||
cli_version: Some(item.cli_version),
|
||||
created_at: Some(item.created_at.to_rfc3339_opts(SecondsFormat::Secs, true)),
|
||||
|
||||
@@ -560,6 +560,8 @@ async fn test_list_conversations_latest_first() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some("2025-01-03T12-00-00".into()),
|
||||
@@ -574,6 +576,8 @@ async fn test_list_conversations_latest_first() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some("2025-01-02T12-00-00".into()),
|
||||
@@ -588,6 +592,8 @@ async fn test_list_conversations_latest_first() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some("2025-01-01T12-00-00".into()),
|
||||
@@ -695,6 +701,8 @@ async fn test_pagination_cursor() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some("2025-03-05T09-00-00".into()),
|
||||
@@ -709,6 +717,8 @@ async fn test_pagination_cursor() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some("2025-03-04T09-00-00".into()),
|
||||
@@ -759,6 +769,8 @@ async fn test_pagination_cursor() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some("2025-03-03T09-00-00".into()),
|
||||
@@ -773,6 +785,8 @@ async fn test_pagination_cursor() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some("2025-03-02T09-00-00".into()),
|
||||
@@ -814,6 +828,8 @@ async fn test_pagination_cursor() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some("2025-03-01T09-00-00".into()),
|
||||
@@ -894,6 +910,8 @@ async fn test_get_thread_contents() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some(ts.into()),
|
||||
@@ -1086,6 +1104,8 @@ async fn test_updated_at_uses_file_mtime() -> Result<()> {
|
||||
originator: "test_originator".into(),
|
||||
cli_version: "test_version".into(),
|
||||
source: SessionSource::VSCode,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some("test-provider".into()),
|
||||
base_instructions: None,
|
||||
dynamic_tools: None,
|
||||
@@ -1203,6 +1223,8 @@ async fn test_stable_ordering_same_second_pagination() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some(ts.to_string()),
|
||||
@@ -1217,6 +1239,8 @@ async fn test_stable_ordering_same_second_pagination() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some(ts.to_string()),
|
||||
@@ -1258,6 +1282,8 @@ async fn test_stable_ordering_same_second_pagination() {
|
||||
git_sha: None,
|
||||
git_origin_url: None,
|
||||
source: Some(SessionSource::VSCode),
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some(TEST_PROVIDER.to_string()),
|
||||
cli_version: Some("test_version".to_string()),
|
||||
created_at: Some(ts.to_string()),
|
||||
|
||||
@@ -156,7 +156,11 @@ mod spawn {
|
||||
.spawn_agent(
|
||||
config,
|
||||
input_items,
|
||||
Some(thread_spawn_source(session.conversation_id, child_depth)),
|
||||
Some(thread_spawn_source(
|
||||
session.conversation_id,
|
||||
child_depth,
|
||||
role_name,
|
||||
)),
|
||||
)
|
||||
.await
|
||||
.map_err(collab_spawn_error);
|
||||
@@ -284,7 +288,6 @@ mod send_input {
|
||||
mod resume_agent {
|
||||
use super::*;
|
||||
use crate::agent::next_thread_spawn_depth;
|
||||
use crate::rollout::find_thread_path_by_id_str;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
@@ -331,15 +334,7 @@ mod resume_agent {
|
||||
.await;
|
||||
let error = if matches!(status, AgentStatus::NotFound) {
|
||||
// If the thread is no longer active, attempt to restore it from rollout.
|
||||
match try_resume_closed_agent(
|
||||
&session,
|
||||
&turn,
|
||||
receiver_thread_id,
|
||||
&args.id,
|
||||
child_depth,
|
||||
)
|
||||
.await
|
||||
{
|
||||
match try_resume_closed_agent(&session, &turn, receiver_thread_id, child_depth).await {
|
||||
Ok(resumed_status) => {
|
||||
status = resumed_status;
|
||||
None
|
||||
@@ -388,33 +383,16 @@ mod resume_agent {
|
||||
session: &Arc<Session>,
|
||||
turn: &Arc<TurnContext>,
|
||||
receiver_thread_id: ThreadId,
|
||||
receiver_id: &str,
|
||||
child_depth: i32,
|
||||
) -> Result<AgentStatus, FunctionCallError> {
|
||||
let rollout_path = find_thread_path_by_id_str(
|
||||
turn.config.codex_home.as_path(),
|
||||
receiver_id,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
"tool failed: failed to locate rollout for agent {receiver_thread_id}: {err}"
|
||||
))
|
||||
})?
|
||||
.ok_or_else(|| {
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
"agent with id {receiver_thread_id} not found"
|
||||
))
|
||||
})?;
|
||||
|
||||
let config = build_agent_resume_config(turn.as_ref(), child_depth)?;
|
||||
let resumed_thread_id = session
|
||||
.services
|
||||
.agent_control
|
||||
.resume_agent_from_rollout(
|
||||
config,
|
||||
rollout_path,
|
||||
thread_spawn_source(session.conversation_id, child_depth),
|
||||
receiver_thread_id,
|
||||
thread_spawn_source(session.conversation_id, child_depth, None),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| collab_agent_error(receiver_thread_id, err))?;
|
||||
@@ -733,10 +711,16 @@ fn collab_agent_error(agent_id: ThreadId, err: CodexErr) -> FunctionCallError {
|
||||
}
|
||||
}
|
||||
|
||||
fn thread_spawn_source(parent_thread_id: ThreadId, depth: i32) -> SessionSource {
|
||||
fn thread_spawn_source(
|
||||
parent_thread_id: ThreadId,
|
||||
depth: i32,
|
||||
agent_role: Option<&str>,
|
||||
) -> SessionSource {
|
||||
SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id,
|
||||
depth,
|
||||
agent_nickname: None,
|
||||
agent_role: agent_role.map(str::to_string),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1068,6 +1052,8 @@ mod tests {
|
||||
turn.session_source = SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id: session.conversation_id,
|
||||
depth: DEFAULT_AGENT_MAX_DEPTH,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
});
|
||||
|
||||
let invocation = invocation(
|
||||
@@ -1104,6 +1090,8 @@ mod tests {
|
||||
turn.session_source = SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id: session.conversation_id,
|
||||
depth: DEFAULT_AGENT_MAX_DEPTH,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
});
|
||||
|
||||
let invocation = invocation(
|
||||
@@ -1487,6 +1475,8 @@ mod tests {
|
||||
turn.session_source = SessionSource::SubAgent(SubAgentSource::ThreadSpawn {
|
||||
parent_thread_id: session.conversation_id,
|
||||
depth: DEFAULT_AGENT_MAX_DEPTH,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
});
|
||||
|
||||
let invocation = invocation(
|
||||
|
||||
Reference in New Issue
Block a user