Skip to content

fix: make agent_names hot-reloadable via ArcSwap to fix unknown agent errors #390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Dakai666 wants to merge 2 commits into from
Closed

fix: make agent_names hot-reloadable via ArcSwap to fix unknown agent errors #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e56129
fix: use byte slice in loop_guard to avoid UTF-8 panic on multibyte c…
Dakai666 Mar 10, 2026
480c8f6
fix: make agent_names hot-reloadable via ArcSwap to fix unknown agent…
Dakai666 Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/agent/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,12 +516,13 @@ impl Channel {
}

/// Get the agent's display name (falls back to agent ID).
fn agent_display_name(&self) -> &str {
fn agent_display_name(&self) -> String {
self.deps
.agent_names
.load()
.get(self.deps.agent_id.as_ref())
.map(String::as_str)
.unwrap_or(self.deps.agent_id.as_ref())
.cloned()
.unwrap_or_else(|| self.deps.agent_id.to_string())
}

fn current_adapter(&self) -> Option<&str> {
Expand Down Expand Up @@ -793,10 +794,11 @@ impl Channel {
.with_label_values(&[&self.deps.agent_id, channel_type])
.inc();
}
let display_name = self.agent_display_name();
self.state.conversation_logger.log_bot_message_with_name(
&self.state.channel_id,
&text,
Some(self.agent_display_name()),
Some(&display_name),
);
}
Err(error) => {
Expand Down Expand Up @@ -1944,6 +1946,7 @@ impl Channel {
let name = self
.deps
.agent_names
.load()
.get(other_id.as_str())
.cloned()
.unwrap_or_else(|| other_id.clone());
Expand Down Expand Up @@ -2541,10 +2544,11 @@ impl Channel {
if extracted.is_some() {
tracing::warn!(channel_id = %self.id, "extracted reply from malformed tool syntax in LLM text output");
}
let display_name = self.agent_display_name();
self.state.conversation_logger.log_bot_message_with_name(
&self.state.channel_id,
&final_text,
Some(self.agent_display_name()),
Some(&display_name),
);
self.send_outbound_text(final_text, "failed to send fallback reply")
.await;
Expand Down
3 changes: 2 additions & 1 deletion src/agent/cortex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2519,7 +2519,8 @@ async fn pickup_one_ready_task(deps: &AgentDeps, logger: &CortexLogger) -> anyho
let logger = logger.clone();
let injection_tx = deps.injection_tx.clone();
let links = deps.links.clone();
let agent_names = deps.agent_names.clone();
let agent_names: Arc<std::collections::HashMap<String, String>> =
deps.agent_names.load_full();
let sqlite_pool = deps.sqlite_pool.clone();
let secrets_snapshot = deps.runtime_config.secrets.load().clone();
let process_control_registry = deps.process_control_registry.clone();
Expand Down
38 changes: 17 additions & 21 deletions src/api/agents.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ pub(super) async fn trigger_warmup(
let task_store_registry = state.task_store_registry.clone();
let injection_tx = state.injection_tx.clone();
let humans = (**state.agent_humans.load()).clone();
let agent_names = state.agent_names.clone();
tokio::spawn(async move {
let (event_tx, memory_event_tx) = crate::create_process_event_buses();
let project_store =
Expand All @@ -432,7 +433,7 @@ pub(super) async fn trigger_warmup(
task_store,
project_store,
links: Arc::new(arc_swap::ArcSwap::from_pointee(Vec::new())),
agent_names: Arc::new(std::collections::HashMap::new()),
agent_names,
humans: Arc::new(arc_swap::ArcSwap::from_pointee(humans)),
task_store_registry,
process_control_registry: Arc::new(
Expand Down Expand Up @@ -764,6 +765,20 @@ pub async fn create_agent_internal(
// Inject active project root paths into the sandbox allowlist.
crate::projects::refresh_sandbox_project_paths(&project_store, &arc_agent_id, &sandbox).await;

// Update the shared agent name registry to include this new agent so all
// existing agents can resolve it immediately via their Arc<ArcSwap<...>>.
{
let new_agent_name = request
.display_name
.as_deref()
.filter(|s| !s.is_empty())
.unwrap_or(&agent_id)
.to_string();
let mut names = (**state.agent_names.load()).clone();
names.insert(agent_id.clone(), new_agent_name);
state.agent_names.store(std::sync::Arc::new(names));
Comment on lines +777 to +779

@tembo tembo Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clone+store can drop updates if multiple agent creations happen concurrently (last writer wins). ArcSwap::rcu gives you an atomic read-modify-write loop.

Suggested change
let mut names = (**state.agent_names.load()).clone();
names.insert(agent_id.clone(), new_agent_name);
state.agent_names.store(std::sync::Arc::new(names));
state.agent_names.rcu(|current| {
let mut names = (**current).clone();
names.insert(agent_id.clone(), new_agent_name.clone());
std::sync::Arc::new(names)
});

}

let deps = crate::AgentDeps {
agent_id: arc_agent_id.clone(),
memory_search: memory_search.clone(),
Expand All @@ -789,26 +804,7 @@ pub async fn create_agent_internal(
crate::agent::process_control::ProcessControlRegistry::new(),
),
injection_tx: state.injection_tx.clone(),
agent_names: {
let configs = state.agent_configs.load();
let mut names: std::collections::HashMap<String, String> = configs
.iter()
.map(|c| {
(
c.id.clone(),
c.display_name.clone().unwrap_or_else(|| c.id.clone()),
)
})
.collect();
names.entry(agent_id.clone()).or_insert_with(|| {
request
.display_name
.clone()
.filter(|s| !s.is_empty())
.unwrap_or_else(|| agent_id.clone())
});
Arc::new(names)
},
agent_names: state.agent_names.clone(),
humans: Arc::new(arc_swap::ArcSwap::from_pointee(
(**state.agent_humans.load()).clone(),
)),
Expand Down
4 changes: 4 additions & 0 deletions src/api/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ pub struct ApiState {
/// Cross-agent task store registry for delegation.
pub task_store_registry:
Arc<ArcSwap<std::collections::HashMap<String, Arc<crate::tasks::TaskStore>>>>,
/// Shared agent name map for resolving agent IDs to display names (hot-reloadable).
pub agent_names: Arc<ArcSwap<std::collections::HashMap<String, String>>>,
/// Sender for cross-agent message injection.
pub injection_tx: mpsc::Sender<crate::ChannelInjection>,
/// Instance-level agent links for the communication graph.
Expand Down Expand Up @@ -289,6 +291,7 @@ impl ApiState {
task_store_registry: Arc<
ArcSwap<std::collections::HashMap<String, Arc<crate::tasks::TaskStore>>>,
>,
agent_names: Arc<ArcSwap<std::collections::HashMap<String, String>>>,
) -> Self {
let (event_tx, _) = broadcast::channel(512);
Self {
Expand Down Expand Up @@ -328,6 +331,7 @@ impl ApiState {
agent_tx,
agent_remove_tx,
task_store_registry,
agent_names,
injection_tx,
webchat_adapter: ArcSwap::from_pointee(None),
agent_links: ArcSwap::from_pointee(Vec::new()),
Expand Down
8 changes: 2 additions & 6 deletions src/hooks/loop_guard.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,12 +448,8 @@ impl LoopGuard {
hasher.update(b"|");
hasher.update(args.as_bytes());
hasher.update(b"|");
let truncated = if result.len() > RESULT_HASH_TRUNCATION {
&result[..RESULT_HASH_TRUNCATION]
} else {
result
};
hasher.update(truncated.as_bytes());
let result_bytes = result.as_bytes();
hasher.update(&result_bytes[..result_bytes.len().min(RESULT_HASH_TRUNCATION)]);
Comment on lines +451 to +452

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Prefix-only hashing still treats appended output as identical.

This still hashes only the first RESULT_HASH_TRUNCATION bytes. If a tool result grows only at the tail, record_outcome() will keep counting it as the same outcome and can poison a healthy polling loop. Include the total length and a suffix sample so tail-only changes stop colliding.

🔧 Possible fix 
-        let result_bytes = result.as_bytes();
-        hasher.update(&result_bytes[..result_bytes.len().min(RESULT_HASH_TRUNCATION)]);
+        let result_bytes = result.as_bytes();
+        if result_bytes.len() <= RESULT_HASH_TRUNCATION {
+            hasher.update(result_bytes);
+        } else {
+            let prefix_len = RESULT_HASH_TRUNCATION / 2;
+            let suffix_len = RESULT_HASH_TRUNCATION - prefix_len;
+            hasher.update(&result_bytes[..prefix_len]);
+            hasher.update(&(result_bytes.len() as u64).to_le_bytes());
+            hasher.update(&result_bytes[result_bytes.len() - suffix_len..]);
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let result_bytes = result.as_bytes();
hasher.update(&result_bytes[..result_bytes.len().min(RESULT_HASH_TRUNCATION)]);
let result_bytes = result.as_bytes();
if result_bytes.len() <= RESULT_HASH_TRUNCATION {
hasher.update(result_bytes);
} else {
let prefix_len = RESULT_HASH_TRUNCATION / 2;
let suffix_len = RESULT_HASH_TRUNCATION - prefix_len;
hasher.update(&result_bytes[..prefix_len]);
hasher.update(&(result_bytes.len() as u64).to_le_bytes());
hasher.update(&result_bytes[result_bytes.len() - suffix_len..]);
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/loop_guard.rs` around lines 451 - 452, The current hashing in
record_outcome() only uses the first RESULT_HASH_TRUNCATION bytes
(hasher.update(&result_bytes[..min(...)]) ), so tail-only changes collide;
modify record_outcome() to feed the hasher the prefix (first N bytes), the total
length (as bytes), and a suffix sample (last M bytes, e.g., min(N, len) bytes)
before finalizing the hash, still using RESULT_HASH_TRUNCATION for the prefix
size and a defined SUFFIX_SAMPLE_SIZE for the suffix so outcomes that only grow
at the tail produce different hashes.

hex::encode(hasher.finalize())
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,8 @@ pub struct AgentDeps {
pub sandbox: Arc<sandbox::Sandbox>,
pub links: Arc<arc_swap::ArcSwap<Vec<links::AgentLink>>>,
/// Map of all agent IDs to display names, for inter-agent message routing.
pub agent_names: Arc<std::collections::HashMap<String, String>>,
/// Hot-reloadable: updated when agents are added or reconfigured.
pub agent_names: Arc<arc_swap::ArcSwap<std::collections::HashMap<String, String>>>,
Comment on lines +396 to +397

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Expect at least one stale initializer if any fixtures still use the old type.
rg -n -U -P 'agent_names:\s*Arc::new\(\s*(?:std::collections::)?HashMap::new\(\)\s*\)' --type rust

Repository: spacedriveapp/spacebot

Length of output: 243

Update remaining AgentDeps initializers to the new ArcSwap type.

Two test fixtures still initialize agent_names as Arc<HashMap<_, _>> and will fail to compile after this public field change:

  • tests/context_dump.rs:123
  • tests/bulletin.rs:124

Both need an ArcSwap::from_pointee(...) wrapper:

Expected fix 
-        agent_names: Arc::new(std::collections::HashMap::new()),
+        agent_names: Arc::new(arc_swap::ArcSwap::from_pointee(
+            std::collections::HashMap::new(),
+        )),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Hot-reloadable: updated when agents are added or reconfigured.
pub agent_names: Arc<arc_swap::ArcSwap<std::collections::HashMap<String, String>>>,
agent_names: Arc::new(arc_swap::ArcSwap::from_pointee(
std::collections::HashMap::new(),
)),
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/lib.rs` around lines 396 - 397, AgentDeps's public field agent_names was
changed to Arc<arc_swap::ArcSwap<HashMap<String,String>>> so update the
remaining test fixtures that still pass an Arc<HashMap<_,_>> by wrapping the
existing HashMap value with arc_swap::ArcSwap::from_pointee(...); locate the
test initializers that construct AgentDeps (look for AgentDeps { ...
agent_names: ... }) and replace the direct Arc<HashMap<_,_>> assignment with
Arc::new(arc_swap::ArcSwap::from_pointee(your_hashmap_here)) (or equivalent to
match surrounding code) so the field type matches the new ArcSwap wrapper.

/// Org-level human definitions (hot-reloadable). Used by `build_org_context()`
/// to surface human display names, roles, and descriptions in agent prompts.
pub humans: Arc<arc_swap::ArcSwap<Vec<config::HumanDef>>>,
Expand Down
30 changes: 19 additions & 11 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1444,13 +1444,18 @@ async fn run(
ArcSwap<std::collections::HashMap<String, Arc<spacebot::tasks::TaskStore>>>,
> = Arc::new(ArcSwap::from_pointee(std::collections::HashMap::new()));

let agent_names_registry: Arc<
ArcSwap<std::collections::HashMap<String, String>>,
> = Arc::new(ArcSwap::from_pointee(std::collections::HashMap::new()));

// Start HTTP API server if enabled
let mut api_state = spacebot::api::ApiState::new_with_provider_sender(
provider_tx,
agent_tx,
agent_remove_tx,
injection_tx.clone(),
task_store_registry.clone(),
agent_names_registry.clone(),
Comment on lines 1452 to +1458

@coderabbitai coderabbitai Bot Mar 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Update the remaining ApiState::new_with_provider_sender call site.

src/api/agents.rs:1643-1659 still calls this constructor with the old parameter list, so the test target will stop compiling once this signature change lands.

Suggested fix in src/api/agents.rs 
 fn test_api_state() -> Arc<ApiState> {
         let (provider_setup_tx, _provider_setup_rx) = tokio::sync::mpsc::channel(1);
         let (agent_tx, _agent_rx) = tokio::sync::mpsc::channel(1);
         let (agent_remove_tx, _agent_remove_rx) = tokio::sync::mpsc::channel(1);

         let (injection_tx, _injection_rx) = tokio::sync::mpsc::channel(1);
         let task_store_registry = Arc::new(arc_swap::ArcSwap::from_pointee(
             std::collections::HashMap::new(),
         ));
+        let agent_names = Arc::new(arc_swap::ArcSwap::from_pointee(
+            std::collections::HashMap::new(),
+        ));
         Arc::new(ApiState::new_with_provider_sender(
             provider_setup_tx,
             agent_tx,
             agent_remove_tx,
             injection_tx,
             task_store_registry,
+            agent_names,
         ))
     }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/main.rs` around lines 1452 - 1458, Update the remaining call to
ApiState::new_with_provider_sender in src/api/agents.rs to match the new
constructor signature used elsewhere: pass provider_tx, agent_tx,
agent_remove_tx, injection_tx.clone(), task_store_registry.clone(), and
agent_names_registry.clone() (in that order) so the call matches the
ApiState::new_with_provider_sender(...) invocation shown in the diff.

);
api_state.auth_token = config.api.auth_token.clone();
let api_state = Arc::new(api_state);
Expand Down Expand Up @@ -1608,6 +1613,7 @@ async fn run(
agent_humans.clone(),
injection_tx.clone(),
task_store_registry.clone(),
agent_names_registry.clone(),
&bootstrapped_store,
)
.await?;
Expand Down Expand Up @@ -2337,6 +2343,7 @@ async fn run(
agent_humans.clone(),
injection_tx.clone(),
task_store_registry.clone(),
agent_names_registry.clone(),
&bootstrapped_store,
).await {
Ok(()) => {
Expand Down Expand Up @@ -2478,20 +2485,21 @@ async fn initialize_agents(
task_store_registry: Arc<
ArcSwap<std::collections::HashMap<String, Arc<spacebot::tasks::TaskStore>>>,
>,
agent_names_registry: Arc<ArcSwap<std::collections::HashMap<String, String>>>,
bootstrapped_store: &Option<Arc<spacebot::secrets::store::SecretsStore>>,
) -> anyhow::Result<()> {
let resolved_agents = config.resolve_agents();

// Build agent name map for inter-agent message routing
let agent_name_map: Arc<std::collections::HashMap<String, String>> = Arc::new(
resolved_agents
.iter()
.map(|a| {
let name = a.display_name.clone().unwrap_or_else(|| a.id.clone());
(a.id.clone(), name)
})
.collect(),
);
// Build agent name map and publish to the shared registry so all agents
// (including those already running) see the updated names immediately.
let agent_name_map: std::collections::HashMap<String, String> = resolved_agents
.iter()
.map(|a| {
let name = a.display_name.clone().unwrap_or_else(|| a.id.clone());
(a.id.clone(), name)
})
.collect();
agent_names_registry.store(Arc::new(agent_name_map));

for agent_config in &resolved_agents {
tracing::info!(agent_id = %agent_config.id, "initializing agent");
Expand Down Expand Up @@ -2713,7 +2721,7 @@ async fn initialize_agents(
messaging_manager: None,
sandbox,
links: agent_links.clone(),
agent_names: agent_name_map.clone(),
agent_names: agent_names_registry.clone(),
humans: agent_humans.clone(),
task_store_registry: task_store_registry.clone(),
process_control_registry: Arc::new(
Expand Down
2 changes: 2 additions & 0 deletions src/tools.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ pub async fn add_channel_tools(
let agent_display_name = state
.deps
.agent_names
.load()
.get(state.deps.agent_id.as_ref())
.cloned()
.unwrap_or_else(|| state.deps.agent_id.to_string());
Expand All @@ -369,6 +370,7 @@ pub async fn add_channel_tools(
let send_message_display_name = state
.deps
.agent_names
.load()
.get(state.deps.agent_id.as_ref())
.cloned()
.unwrap_or_else(|| state.deps.agent_id.to_string());
Expand Down
12 changes: 8 additions & 4 deletions src/tools/send_agent_message.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub struct SendAgentMessageTool {
agent_id: crate::AgentId,
links: Arc<ArcSwap<Vec<AgentLink>>>,
/// Map of known agent IDs to display names, for resolving targets.
agent_names: Arc<HashMap<String, String>>,
agent_names: Arc<ArcSwap<HashMap<String, String>>>,
/// Cross-agent task store registry for creating tasks on target agents.
task_store_registry: Arc<ArcSwap<HashMap<String, Arc<TaskStore>>>>,
/// Per-agent conversation logger for writing link channel audit records.
Expand All @@ -54,7 +54,7 @@ impl SendAgentMessageTool {
pub fn new(
agent_id: crate::AgentId,
links: Arc<ArcSwap<Vec<AgentLink>>>,
agent_names: Arc<HashMap<String, String>>,
agent_names: Arc<ArcSwap<HashMap<String, String>>>,
task_store_registry: Arc<ArcSwap<HashMap<String, Arc<TaskStore>>>>,
conversation_logger: ConversationLogger,
) -> Self {
Expand Down Expand Up @@ -85,14 +85,16 @@ impl SendAgentMessageTool {
/// Resolve an agent target string to an agent ID.
/// Checks both IDs and display names (case-insensitive).
fn resolve_agent_id(&self, target: &str) -> Option<String> {
let names = self.agent_names.load();

// Direct ID match
if self.agent_names.contains_key(target) {
if names.contains_key(target) {
return Some(target.to_string());
}

// Name match (case-insensitive)
let target_lower = target.to_lowercase();
for (agent_id, name) in self.agent_names.iter() {
for (agent_id, name) in names.iter() {
if name.to_lowercase() == target_lower {
return Some(agent_id.clone());
}
Expand Down Expand Up @@ -199,6 +201,7 @@ impl Tool for SendAgentMessageTool {

let target_display = self
.agent_names
.load()
.get(receiving_agent_id)
.cloned()
.unwrap_or_else(|| receiving_agent_id.to_string());
Expand Down Expand Up @@ -249,6 +252,7 @@ impl Tool for SendAgentMessageTool {
// Log delegation record in the link channel (system message).
let sender_display = self
.agent_names
.load()
.get(sending_agent_id)
.cloned()
.unwrap_or_else(|| sending_agent_id.to_string());
Expand Down