Merged
Conversation
Directed links between agents with relationship semantics (peer/superior/subordinate), a shared instance-level database, and a send_agent_message tool that routes through the existing MessagingManager pipeline. Foundation for the 0.2.0 release. - Instance database (instance.db) with separate migration path for cross-agent data - AgentLink model with direction (one_way/two_way) and relationship policies - LinkStore CRUD backed by instance.db - [[links]] config section with startup sync - send_agent_message tool: resolves target, validates link, injects InboundMessage - ROLE.md identity file for operational responsibilities - Org context prompt fragment (superiors/subordinates/peers hierarchy) - Link context prompt fragment for agent-to-agent channels - AgentMessageSent/Received process events with SSE forwarding - Link CRUD API endpoints + topology snapshot endpoint
Links are defined in [[links]] config sections and stored in a shared ArcSwap<Vec<AgentLink>> that hot-reloads when config.toml changes. No separate database needed since config is already runtime-mutable through the API. - Remove InstanceDb, migrations_instance/, LinkStore - Links parsed from config at startup via AgentLink::from_config() - Shared via Arc<ArcSwap<Vec<AgentLink>>> passed to agents and API - File watcher reloads links on config.toml changes - API endpoints simplified to read-only (list, agent links, topology) - Deterministic link channel IDs from sorted agent ID pairs - ROLE.md added to file watcher identity change detection
- Added new API endpoints for creating, updating, and deleting agent links. - Introduced data structures for managing agent links and their relationships. - Enhanced the frontend with a TopologyGraph component to visualize agent connections. - Updated the LiveContext to track active links based on message activity. - Modified Overview component to integrate the new TopologyGraph and display active links.
…unctionality - Added optional display name and role fields to the AgentInfo and TopologyAgent interfaces. - Introduced new TopologyGroup interface and related API endpoints for creating, updating, and deleting agent groups. - Updated CreateAgentDialog to support input for display names and roles. - Enhanced TopologyGraph to visualize agent groups and their relationships. - Modified API and state management to handle agent groups effectively.
- Updated ToolActivityIndicator and WebChatPanel for better layout and responsiveness. - Introduced FloatingChatInput for a more user-friendly input experience. - Added ThinkingIndicator for visual feedback during processing. - Enhanced message display logic for clearer user interactions. - Improved styling and structure for better visual consistency across components.
- Added support for managing org-level humans with CRUD operations via new API endpoints. - Introduced TopologyHuman interface to represent humans in the topology. - Enhanced the ApiState to include a list of humans for topology visualization. - Updated the configuration to support human definitions and ensure persistence. - Modified the TopologyGraph component to accommodate human nodes alongside agents. - Improved error handling for human-related operations in the API.
…tures - Renamed LinkRelationship to LinkKind to better reflect the nature of connections between nodes. - Updated related interfaces and API structures to use the new LinkKind terminology. - Enhanced the TopologyGraph component to accommodate changes in link representation. - Added optional bio field to human definitions for improved context in the topology. - Improved backward compatibility for existing configurations using the old relationship terminology.
- Introduced a new lockfile (bun.lock) to manage dependencies for the project. - Updated the agents documentation to include a communication graph for agent coordination, enhancing the description of agent capabilities and configuration. - Added details on agent roles and definitions, improving clarity on agent functionalities and their interactions within the system.
… editing - Added an API endpoint to update agent details, including display name and role. - Introduced a new AgentEditDialog component for editing agent information within the TopologyGraph. - Enhanced the TopologyGraph to support edit actions for both human and agent nodes. - Improved UI elements for better interaction and visibility of edit options in the graph.
… routing - Implemented a new API endpoint for deleting channels and their message history. - Added a delete button in the ChannelCard component to trigger channel deletion. - Enhanced internal message handling to route replies back to the appropriate link channels, preserving conversation context. - Updated the ChannelStore to support channel deletion with proper database operations. - Improved link channel management to ensure each agent has a dedicated conversation ID for better message routing.
- Added functionality for link channels, allowing agents to maintain dedicated channels for communication and track conversation history. - Implemented a `conclude_link` tool to gracefully end link conversations, routing summaries back to the originating channel. - Updated agent status blocks to display active link conversations and their turn counts. - Enhanced internal message handling to support the new link management features, including safety caps for conversation turns. - Improved documentation to reflect changes in link channel behavior and the new conclusion process.
- Addressed a naming mismatch between the webchat adapter and frontend session IDs, proposing a unified naming convention for clarity. - Updated message logging to include sender names for both user and bot messages, improving message traceability in the conversation history. - Enhanced internal message handling to propagate originating source information for better routing in link conversations. - Introduced new methods for logging bot messages with display names, ensuring accurate representation in the conversation log.
- Introduced checks for delegation targets in link channels to prevent loops and ensure proper message routing. - Added functionality to manage originating channels for agent messages, improving clarity in link conversation flows. - Updated the SendAgentMessageTool to handle upstream counterparty checks, preventing incorrect message delegation. - Enhanced internal message handling to support late-arriving link conclusions and ensure proper routing back to originating channels. - Improved logging for link conclusion messages to maintain conversation integrity.
…unication-graph # Conflicts: # src/agent/channel.rs
Introduced a new function `truncate_for_log` to handle message truncation for logging, ensuring messages do not exceed a specified character limit. Updated the `init_background_tracing` and `init_foreground_tracing` functions to utilize this new truncation logic. Added unit tests to verify the functionality of the truncation, including handling of multibyte characters.
…itialize links and agent_names in trigger_warmup
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
Comment on lines
190
to
197
| // Check for duplicate | ||
| let existing = state.agent_links.load(); | ||
| let duplicate = existing | ||
| .iter() | ||
| .any(|link| link.from_agent_id == request.from && link.to_agent_id == request.to); | ||
| if duplicate { | ||
| return Err(StatusCode::CONFLICT); | ||
| } |
Contributor
There was a problem hiding this comment.
Duplicate detection probably wants to consider both orientations. As-is you can create both A -> B and B -> A, which makes find_link_between() ambiguous.
Suggested change
| // Check for duplicate | |
| let existing = state.agent_links.load(); | |
| let duplicate = existing | |
| .iter() | |
| .any(|link| link.from_agent_id == request.from && link.to_agent_id == request.to); | |
| if duplicate { | |
| return Err(StatusCode::CONFLICT); | |
| } | |
| let duplicate = existing.iter().any(|link| { | |
| (link.from_agent_id == request.from && link.to_agent_id == request.to) | |
| || (link.from_agent_id == request.to && link.to_agent_id == request.from) | |
| }); |
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
WalkthroughAdds a topology-driven multi-agent communication graph: config-backed links/groups/humans, new API endpoints/types, inter-agent tools (send_agent_message, conclude_link), runtime link handling and status, prompt/template updates, a ReactFlow TopologyGraph UI, and hot-reload of config.toml and runtime state. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Frontend)
participant UI as TopologyGraph
participant API as Backend API
participant Agent as Agent Runtime
participant Peer as Peer Agent
participant Store as config.toml
User->>UI: Create link {from,to,direction,kind}
UI->>API: POST /links {from,to,direction,kind}
API->>Store: Persist link in config.toml
API->>API: Update in-memory agent_links
API-->>UI: Return created link
UI->>UI: Refresh topology view
User->>Agent: Trigger action -> send_agent_message
Agent->>Agent: Validate link, build internal inbound message
Agent->>API: Emit SSE AgentMessageSent
Agent->>Peer: Inject inbound message on link channel
Peer->>Peer: Process message, may call conclude_link
Peer->>Agent: Route conclusion (AgentMessageReceived)
API-->>UI: SSE updates activeLinks
UI->>UI: Highlight edge (3s)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
interface/src/hooks/useChannelLiveState.ts (1)
606-617:⚠️ Potential issue | 🟠 Major
agent_message_sent/agent_message_receivedSSE events are never handled — dashboard visibility is broken.The server emits these two new event types (see
src/api/system.rslines 96–97), but neither appears in thehandlersmap. Any inter-agent message event streamed to the client will be silently discarded. TheApiEventunion ininterface/src/api/client.tsalso needs these variants with a matching payload type.🛠️ Skeleton for the missing handlers
+import type { AgentMessageSentEvent, AgentMessageReceivedEvent } from "../api/client"; // In useChannelLiveState, add two handlers: +const handleAgentMessageSent = useCallback((data: unknown) => { + const event = data as AgentMessageSentEvent; + // e.g., emit a toast, update a badge, or push a synthetic timeline item +}, []); + +const handleAgentMessageReceived = useCallback((data: unknown) => { + const event = data as AgentMessageReceivedEvent; + // surface in the relevant channel timeline or a dedicated feed +}, []); const handlers = { // ...existing entries... + agent_message_sent: handleAgentMessageSent, + agent_message_received: handleAgentMessageReceived, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/hooks/useChannelLiveState.ts` around lines 606 - 617, The handlers map in useChannelLiveState.ts is missing entries for the new SSE event types agent_message_sent and agent_message_received, so inter-agent message events are dropped; add handler functions (or reuse existing inbound/outbound handlers) for "agent_message_sent" and "agent_message_received" to the handlers object and implement corresponding functions (e.g., handleAgentMessageSent, handleAgentMessageReceived) or route them to handleInboundMessage/handleOutboundMessage; also update the ApiEvent union in interface/src/api/client.ts to include agent_message_sent and agent_message_received variants with the correct payload type so the client correctly types and processes these events.tests/context_dump.rs (2)
413-424:⚠️ Potential issue | 🔴 CriticalBuild failure: same
add_channel_toolsarity mismatch (12 expected, 8 supplied).Same issue as the earlier call site at line 201. Both invocations need to be updated with the 4 additional arguments that the expanded function signature now requires.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/context_dump.rs` around lines 413 - 424, The test call to add_channel_tools is using the old 8-argument arity; update the invocation of spacebot::tools::add_channel_tools to pass the four new parameters required by the expanded signature (i.e., match the function add_channel_tools(...) parameter list) — modify both test call sites in this test to supply the additional arguments (use appropriate variables or sensible defaults as used elsewhere in the codebase) so the call matches the new 12-argument signature and compiles.
201-212:⚠️ Potential issue | 🔴 CriticalUpdate both
add_channel_toolscall sites with 4 additional arguments.The function now requires 12 parameters, but both calls supply only 8. Add the 4 missing
Optionparameters at the end of each call:
conclude_link: Option<(ConcludeLinkFlag, ConcludeLinkSummary)>message_source: Option<String>originating_channel_override: Option<String>originating_source_override: Option<String>Pass
Nonefor each if not applicable. Both call sites intests/context_dump.rsneed this update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/context_dump.rs` around lines 201 - 212, The call sites to spacebot::tools::add_channel_tools are passing only 8 arguments but the function signature now expects 12; update both calls (the ones that end with .await.expect("failed to add channel tools")) to append the four new Option parameters in order: conclude_link: Option<(ConcludeLinkFlag, ConcludeLinkSummary)>, message_source: Option<String>, originating_channel_override: Option<String>, originating_source_override: Option<String>, using None for each when not applicable (i.e., add None, None, None, None at the end of the existing arg list before .await).
♻️ Duplicate comments (6)
src/agent/channel.rs (1)
1150-1160:agent_idin conclusion message targets self — verify routing is correct.
agent_idis set toself.deps.agent_id(the concluding agent). Since the originating channel belongs to this same agent, routing should work. However,sender_idis set topeer_agent(a display name, not an ID), which may cause issues if downstream code expects a stable identifier rather than a display name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 1150 - 1160, The InboundMessage construction uses agent_id = self.deps.agent_id (OK) but sets sender_id to peer_agent (a display name) which may break routing; change sender_id to the stable identifier (e.g., peer_agent_id or whatever stable id is available in scope) or, if only a display name exists, add/derive a stable sender id and set sender_id to that and keep peer_agent in formatted_author or metadata; update the InboundMessage creation (reference: InboundMessage, agent_id, sender_id, peer_agent, self.deps.agent_id) so downstream code receives a stable sender identifier rather than a display name.tests/bulletin.rs (1)
73-84:⚠️ Potential issue | 🔴 Critical
AgentDepsinitializer is missing the requiredagent_namesfield — CI compile failure.
AgentDepsnow also requiresagent_names, which was not added alongsidelinks. The CI pipeline confirms this with:E0063: missing field `agent_names` in initializer of `AgentDeps`🛠️ Proposed fix
links: Arc::new(arc_swap::ArcSwap::from_pointee(Vec::new())), + agent_names: Arc::new(std::collections::HashMap::new()), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/bulletin.rs` around lines 73 - 84, AgentDeps initializer is missing the newly required agent_names field; add an agent_names entry to the returned AgentDeps struct (similar to links) to satisfy the type: for example set agent_names: Arc::new(arc_swap::ArcSwap::from_pointee(Vec::new())) or otherwise provide the appropriate Arc/ArcSwap-wrapped Vec<String> expected by AgentDeps so the struct compiles.tests/context_dump.rs (1)
73-84:⚠️ Potential issue | 🔴 CriticalBuild failure: missing
agent_namesfield inAgentDepsinitializer.The pipeline reports
E0063: missing field 'agent_names'. TheAgentDepsstruct (seesrc/lib.rsline 203) now requires anagent_names: Arc<HashMap<String, String>>field, but this initializer only addslinks.🔧 Proposed fix
links: Arc::new(arc_swap::ArcSwap::from_pointee(Vec::new())), + agent_names: Arc::new(std::collections::HashMap::new()),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/context_dump.rs` around lines 73 - 84, The AgentDeps initializer in tests/context_dump.rs is missing the new required field agent_names; update the construction of spacebot::AgentDeps to include agent_names: an Arc<HashMap<String, String>> (e.g., Arc::new(HashMap::new()) or a populated map as appropriate for the test) alongside the existing links field so the struct literal matches the AgentDeps definition; ensure you import or reference std::collections::HashMap and std::sync::Arc if needed.src/conversation/channels.rs (1)
164-181:⚠️ Potential issue | 🟠 MajorWrap the two
DELETEstatements in a transaction to avoid partial deletion.If the first
DELETE(messages) succeeds but the second (channel row) fails, you'll have orphaned a channel with no messages and no way for the caller to know the state is inconsistent. The doc comment even mentions cascade behavior, but the code doesn't use a transaction.🔧 Proposed fix
pub async fn delete(&self, channel_id: &str) -> crate::error::Result<bool> { - sqlx::query("DELETE FROM conversation_messages WHERE channel_id = ?") + let mut tx = self.pool.begin().await.map_err(|e| anyhow::anyhow!(e))?; + + sqlx::query("DELETE FROM conversation_messages WHERE channel_id = ?") .bind(channel_id) - .execute(&self.pool) + .execute(&mut *tx) .await .map_err(|e| anyhow::anyhow!(e))?; let result = sqlx::query("DELETE FROM channels WHERE id = ?") .bind(channel_id) - .execute(&self.pool) + .execute(&mut *tx) .await .map_err(|e| anyhow::anyhow!(e))?; + tx.commit().await.map_err(|e| anyhow::anyhow!(e))?; + Ok(result.rows_affected() > 0) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/conversation/channels.rs` around lines 164 - 181, The delete method can leave the DB in an inconsistent state because it runs two separate DELETEs; wrap both deletes in a single transaction in the delete function so they commit atomically: begin a transaction (e.g., via self.pool.begin() / sqlx::Transaction), run the DELETE FROM conversation_messages WHERE channel_id = ? and then DELETE FROM channels WHERE id = ? using the transaction connection, check the rows_affected from the channels DELETE, and commit the transaction; on any error ensure the transaction is rolled back and propagate the error so partial deletes cannot occur.src/api/agents.rs (1)
664-675:⚠️ Potential issue | 🟠 Majoragent_names ignores display_name, breaking name-based resolution.
This map is used for name resolution (e.g., send_agent_message). Mapping id → id means agents created via the API won’t be addressable by display_name.
♻️ Proposed fix
- .map(|c| (c.id.clone(), c.id.clone())) + .map(|c| (c.id.clone(), c.display_name.clone().unwrap_or_else(|| c.id.clone())))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/agents.rs` around lines 664 - 675, The agent_names map currently maps each agent id to itself, ignoring display_name and breaking name-based resolution; update the construction of agent_names (where state.agent_configs.load() is iterated and (c.id.clone(), c.id.clone()) is used) to map the id to the agent's display name instead, falling back to the id when display_name is empty or None (use c.display_name or equivalent field on the config, with a fallback to c.id.clone()) so name-based lookups (e.g., send_agent_message) resolve by display name.src/api/links.rs (1)
190-195:⚠️ Potential issue | 🟡 MinorDuplicate detection should consider reversed endpoints.
Right now
A → BandB → Acan coexist even when the intent is a single bidirectional link, which can create ambiguity. Consider treating reversed pairs as duplicates.🛠️ Suggested fix
- let duplicate = existing - .iter() - .any(|link| link.from_agent_id == request.from && link.to_agent_id == request.to); + let duplicate = existing.iter().any(|link| { + (link.from_agent_id == request.from && link.to_agent_id == request.to) + || (link.from_agent_id == request.to && link.to_agent_id == request.from) + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/links.rs` around lines 190 - 195, The duplicate-check currently only matches links with the same direction; update the detection in the block using state.agent_links.load() (the existing variable and duplicate calculation) to treat reversed endpoints as duplicates by checking both directions — e.g. change the closure in existing.iter().any(...) to return true if (link.from_agent_id == request.from && link.to_agent_id == request.to) || (link.from_agent_id == request.to && link.to_agent_id == request.from), or alternatively normalize endpoints (min/max) for comparison so A→B and B→A are considered the same.
🧹 Nitpick comments (18)
src/identity/files.rs (1)
56-69:ROLE.mdis not scaffolded for new agents.
DEFAULT_IDENTITY_FILESincludes templates forSOUL.md,IDENTITY.md, andUSER.md, but notROLE.md. New agents created viascaffold_identity_fileswon't get aROLE.mdplaceholder. If this is intentional (ROLE.md is only relevant in multi-agent setups), consider adding a brief comment here to explain the omission so future contributors don't treat it as a bug.Suggested addition
const DEFAULT_IDENTITY_FILES: &[(&str, &str)] = &[ ( "SOUL.md", "<!-- Define this agent's soul: personality, values, communication style, boundaries. -->\n", ), ( "IDENTITY.md", "<!-- Define this agent's identity: name, nature, purpose. -->\n", ), ( "USER.md", "<!-- Describe the human this agent interacts with: name, preferences, context. -->\n", ), + // ROLE.md is intentionally omitted — it's optional and primarily used in multi-agent setups. + // Agents can add a ROLE.md manually to define operational responsibilities. ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/identity/files.rs` around lines 56 - 69, DEFAULT_IDENTITY_FILES omits a ROLE.md scaffold so scaffold_identity_files won't create a ROLE.md for new agents; either add a ("ROLE.md", "<!-- Describe this agent's role... -->\n") tuple into DEFAULT_IDENTITY_FILES alongside SOUL.md/IDENTITY.md/USER.md or, if ROLE.md is intentionally excluded for single-agent setups, add a clear inline comment next to DEFAULT_IDENTITY_FILES explaining that omission and when ROLE.md is used; update references to DEFAULT_IDENTITY_FILES and scaffold_identity_files accordingly so reviewers can see the rationale or new template entry.src/agent/status.rs (1)
131-133: Only outbound messages are tracked — consider documenting the semantics.
update()handlesAgentMessageSentbut notAgentMessageReceived, soturn_countreflects only messages sent by this agent to the peer, not the full bidirectional conversation. This is a reasonable per-agent perspective, but worth a brief doc comment onturn_countso consumers don't misinterpret it as total round-trip turns.Suggested doc clarification
pub struct LinkConversationStatus { pub peer_agent: String, pub started_at: DateTime<Utc>, + /// Number of messages sent *to* this peer (outbound only). pub turn_count: u32, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/status.rs` around lines 131 - 133, update() currently increments turn_count only on ProcessEvent::AgentMessageSent via track_link_conversation(to_agent_id), so turn_count represents outbound messages from this agent, not full bidirectional turns; add a brief doc comment on the relevant struct field or method (e.g., the turn_count field on the Status struct and/or the track_link_conversation method) stating that turn_count counts only messages sent by this agent (not messages received) and that AgentMessageReceived is intentionally not tracked, or alternatively implement handling for ProcessEvent::AgentMessageReceived if you want bidirectional counting; reference ProcessEvent::AgentMessageSent, ProcessEvent::AgentMessageReceived, update(), track_link_conversation, and turn_count in the comment.src/daemon.rs (1)
210-217: Minor inconsistency with background tracing's truncation format.The foreground path splits the ellipsis into a separate
write!call (Lines 212–213), while the background path (Line 152) combines it into one:write!(writer, "{}={}...", ...). Both produce identical output, but aligning them would improve readability.Align with background tracing style
let (truncated, was_truncated) = truncate_for_log(&formatted, MAX_MESSAGE_CHARS); if was_truncated { - write!(writer, "{}={}", field_name, truncated)?; - write!(writer, "...")?; + write!(writer, "{}={}...", field_name, truncated)?; } else { write!(writer, "{}={formatted}", field_name)?; } Ok(())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/daemon.rs` around lines 210 - 217, The truncation branch in the writer block uses two write! calls for the ellipsis, causing an inconsistency with the background tracing style; update the foreground branch in the function that calls truncate_for_log (using symbols writer, field_name, formatted, MAX_MESSAGE_CHARS) to emit the truncated value and ellipsis in a single write! call (e.g. write!(writer, "{}={}...", field_name, truncated)) instead of two separate writes so the formatting matches the background path.src/tools.rs (2)
294-302:link_counterparty_for_agentduplicates logic inSendAgentMessageTool::current_link_counterparty_id.Both this function and
SendAgentMessageTool::current_link_counterparty_id(insrc/tools/send_agent_message.rs, lines 320-332) perform the same"link:"prefix strip →split_once(':')→ match-self logic. Consider extracting a shared utility (e.g. insrc/links.rs) to avoid the duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools.rs` around lines 294 - 302, The logic in link_counterparty_for_agent and SendAgentMessageTool::current_link_counterparty_id is duplicated; extract a shared utility (e.g. parse_link_counterparty or link_counterparty_for) into a new module (suggested name links.rs) that performs the "link:" prefix strip, split_once(':') and matching against agent_id, then return Option<String>; replace the body of link_counterparty_for_agent and the implementation in SendAgentMessageTool::current_link_counterparty_id to call this new utility to remove duplication and centralize parsing logic.
184-197: Consider grouping per-turn link context into a struct.
add_channel_toolsnow takes 12 parameters. Bundling the link-related ones (send_agent_message_tool,conclude_link,message_source,originating_channel_override,originating_source_override) into a small options struct would reduce the argument count and make future extensions easier.Example struct
pub struct LinkTurnContext { pub send_agent_message_tool: Option<SendAgentMessageTool>, pub conclude_link: Option<(ConcludeLinkFlag, ConcludeLinkSummary)>, pub message_source: Option<String>, pub originating_channel: Option<String>, pub originating_source: Option<String>, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools.rs` around lines 184 - 197, The function add_channel_tools has become unwieldy with many link-related parameters; introduce a small struct (e.g., LinkTurnContext) that groups send_agent_message_tool: Option<SendAgentMessageTool>, conclude_link: Option<(ConcludeLinkFlag, ConcludeLinkSummary)>, message_source: Option<String>, originating_channel: Option<String>, and originating_source: Option<String>, then replace those individual params in add_channel_tools with a single link_context: LinkTurnContext parameter, update the function body to use fields off link_context (or destructure them), and update all call sites to construct and pass LinkTurnContext accordingly so the API and internals continue to work without changing behavior.src/prompts/engine.rs (1)
453-483:render_channel_prompt_with_linksis a superset ofrender_channel_prompt.The new method renders the same
"channel"template with two additional optional context values. If the existingrender_channel_prompt(line 388) no longer has callers, consider removing it to avoid drift. If it does, consider consolidating into one method whereorg_contextandlink_contextdefault toNone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/prompts/engine.rs` around lines 453 - 483, The PR adds render_channel_prompt_with_links which duplicates render_channel_prompt except for two extra Option<String> contexts (org_context and link_context); audit callers of render_channel_prompt and either remove the old method if unused, or consolidate by having render_channel_prompt delegate to render_channel_prompt_with_links passing None for org_context and link_context (or replace both with a single method where org_context and link_context default to None). Update references to use render_channel_prompt_with_links (or the unified method) and remove the duplicate implementation to avoid drift, ensuring you keep the render(...) call and context keys identical.src/tools/conclude_link.rs (1)
91-106: Minor: unnecessary.clone()onargs.summary.
argsis consumed bycall, so you can capture the length first and then move the string into the lock:Avoid the clone
async fn call(&self, args: Self::Args) -> Result<Self::Output, Self::Error> { self.flag.store(true, Ordering::Relaxed); - *self.summary.write().await = Some(args.summary.clone()); + let summary_len = args.summary.len(); + *self.summary.write().await = Some(args.summary); let _ = self .response_tx .send(OutboundResponse::Status(crate::StatusUpdate::StopTyping)) .await; tracing::info!( - summary_len = args.summary.len(), + summary_len, "conclude_link tool called, ending link conversation" ); Ok(ConcludeLinkOutput { concluded: true }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/conclude_link.rs` around lines 91 - 106, The call method unnecessarily clones args.summary; instead read its length into a local (e.g., let summary_len = args.summary.len()) then move the String into the RwLock assignment (e.g., *self.summary.write().await = Some(args.summary)); update the tracing::info! to use summary_len and remove the .clone() so args is consumed without cloning in the ConcludeLink::call implementation.src/agent/channel.rs (1)
862-910: Link conclusion cascade and safety cap look solid.The cascade path (lines 868-887) correctly re-routes conclusions that arrive on already-concluded channels, preventing lost summaries in multi-hop chains. The 20-turn safety cap is a reasonable guardrail. One note: messages are silently dropped after the cap — consider logging the content or notifying the originating channel that the link was forcibly terminated, so the upstream agent isn't left waiting indefinitely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 862 - 910, When a link conversation is force-terminated by the safety cap (check in the block that uses LINK_MAX_TURNS, is_link_channel, and self.link_turn_count for channel id self.id), add an explicit notification or richer logging before returning Ok(()); specifically either send a short system message back to the originating channel indicating the link was forcibly terminated (so upstream agents won't wait) or include the dropped message summary/body in the tracing::warn payload; ensure any notification uses the existing route_link_conclusion or the channel's send mechanism so it follows normal message delivery paths.src/tools/send_agent_message.rs (1)
347-364: Case-insensitive name resolution is user-friendly but consider ambiguity.If two agents share the same case-insensitive display name (e.g., "Dev" and "dev"), the first match from HashMap iteration wins non-deterministically. This is unlikely but could be guarded with an ambiguity check.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tools/send_agent_message.rs` around lines 347 - 364, resolve_agent_id currently returns the first case-insensitive match from agent_names which can be non-deterministic if multiple agents share the same case-insensitive display name; change resolve_agent_id to collect all agent_ids whose name.to_lowercase() == target_lower, return the single match when exactly one is found, and return None (or otherwise surface ambiguity) when multiple matches exist so callers can handle the ambiguity; update the function body that iterates self.agent_names and replace the early-return logic with collecting matches then branching on matches.len() to return the unique agent_id or None.src/api/channels.rs (1)
151-182: Clean delete handler — consider using a path parameter for the channel identifier.The implementation is correct and the error handling is solid. One minor API design note: RESTful convention would typically use
DELETE /api/agents/:agent_id/channels/:channel_idwith path parameters rather than query parameters for resource identifiers. Query params for destructive operations can be inadvertently cached/logged by intermediaries. Not a blocker for an internal API, but worth considering for consistency with the other resource endpoints in the PR (e.g.,/api/agents/:id/links).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/channels.rs` around lines 151 - 182, Change the delete_channel handler to accept the channel identifier as a path parameter instead of a query: replace DeleteChannelQuery/Query extractor with an axum Path extractor (or a Deserialize struct used with Path) to pull agent_id and channel_id from the URL, update the function signature of delete_channel accordingly, and ensure the router/route for this handler is registered as DELETE /api/agents/:agent_id/channels/:channel_id so the handler uses the path values when looking up the pool and calling ChannelStore::delete.src/links/types.rs (1)
10-16: Consider derivingPartialEqonAgentLinkfor easier testing.The struct derives
Debug, Clone, Serialize, Deserializebut notPartialEq. Adding it would simplify assertions in tests (e.g., comparing parsed links against expected values).Proposed change
-#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct AgentLink {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/links/types.rs` around lines 10 - 16, The AgentLink struct lacks PartialEq which makes test assertions harder; update the derive list on AgentLink (the #[derive(Debug, Clone, Serialize, Deserialize)] attribute) to also include PartialEq (and optionally Eq) so tests can directly compare AgentLink instances (refer to the AgentLink type in src/links/types.rs).interface/src/components/ChannelCard.tsx (1)
129-142: Missing disabled/loading state on delete button.While the mutation is in-flight, the button remains clickable, allowing duplicate delete requests. Consider disabling the button or showing a spinner while
deleteChannel.isPendingis true.Proposed fix
<button onClick={(e) => { e.preventDefault(); e.stopPropagation(); deleteChannel.mutate(); }} - className="rounded p-1 text-ink-faint opacity-0 transition-opacity hover:bg-ink/10 hover:text-ink group-hover/card:opacity-100" + disabled={deleteChannel.isPending} + className={`rounded p-1 text-ink-faint opacity-0 transition-opacity hover:bg-ink/10 hover:text-ink group-hover/card:opacity-100 ${deleteChannel.isPending ? "pointer-events-none opacity-50" : ""}`} title="Delete channel" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/ChannelCard.tsx` around lines 129 - 142, The delete button in ChannelCard allows duplicate requests because it doesn't reflect the mutation state; update the button that calls deleteChannel.mutate() to be disabled and show a loading affordance while deleteChannel.isPending is true: add the disabled attribute bound to deleteChannel.isPending, prevent clicks when pending, and swap the SVG for a spinner or adjust classNames to indicate loading; ensure the onClick still calls e.preventDefault()/e.stopPropagation() only when not pending so deleteChannel.mutate() is invoked once.interface/src/components/TopologyGraph.tsx (2)
839-861: Setting state during render is fragile with concurrent React.Lines 839–861 (and similarly 864–873) update
nodes/edgesstate directly during the render phase by comparing refs. While this works for synchronizing external data, it can trigger additional re-renders and behave unpredictably under React 19's concurrent mode. AuseEffectis the idiomatic way to sync derived state.Refactor to useEffect for data sync
- const prevDataRef = useRef(data); - const prevProfilesRef = useRef(agentProfiles); - if (data !== prevDataRef.current || agentProfiles !== prevProfilesRef.current) { - prevDataRef.current = data; - prevProfilesRef.current = agentProfiles; - if (data) { - ... - setNodes(mergedNodes); - setEdges(newEdges); - } - } + useEffect(() => { + if (!data) return; + const { initialNodes: newNodes, initialEdges: newEdges } = buildGraph( + data, activeEdges, agentProfiles, + ); + setNodes((current) => { + const positionMap = new Map(current.map((n) => [n.id, n.position])); + return patchEditCallbacks(newNodes.map((n) => ({ + ...n, + position: positionMap.get(n.id) ?? n.position, + }))); + }); + setEdges(newEdges); + }, [data, agentProfiles]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/TopologyGraph.tsx` around lines 839 - 861, The render-phase state updates for nodes/edges should be moved into a useEffect to avoid setting state during render; create a useEffect that depends on data, agentProfiles, and activeEdges (and any other relevant inputs), compare data and agentProfiles against prevDataRef/prevProfilesRef inside that effect, call buildGraph(data, activeEdges, agentProfiles), preserve existing node positions by building positionMap from the current nodes ref/state, apply patchEditCallbacks to merged nodes, then call setNodes and setEdges and update prevDataRef.current and prevProfilesRef.current; ensure the effect uses stable references (or refs) for nodes to avoid stale closures and only runs when the external inputs change.
1557-1572: O(n) parent lookup inside node loop.
allNodes.find()at line 1562 scans all nodes for each child node. Consider building a position map for parent nodes first to avoid the nested search.Proposed optimization
+ // Pre-build parent position map for absolute position calculation + const parentPositionMap = new Map<string, { x: number; y: number }>(); + for (const node of allNodes) { + if (node.type === "group") { + parentPositionMap.set(node.id, node.position); + } + } + const nodePositionMap = new Map<string, { x: number; y: number }>(); for (const node of allNodes) { if (node.parentId) { - const parent = allNodes.find((n) => n.id === node.parentId); - if (parent) { + const parentPos = parentPositionMap.get(node.parentId); + if (parentPos) { nodePositionMap.set(node.id, { - x: parent.position.x + node.position.x, - y: parent.position.y + node.position.y, + x: parentPos.x + node.position.x, + y: parentPos.y + node.position.y, }); continue; } } nodePositionMap.set(node.id, node.position); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/TopologyGraph.tsx` around lines 1557 - 1572, The loop in TopologyGraph.tsx builds nodePositionMap but uses allNodes.find(node => node.id === node.parentId) causing O(n^2); instead create a lookup map first (e.g., nodeById or parentPositionMap) keyed by node.id from allNodes, then iterate allNodes and compute absolute positions using that map when node.parentId is present, updating nodePositionMap accordingly to eliminate the nested find.interface/src/hooks/useLiveContext.tsx (1)
46-76: Timer cleanup missing on unmount.The
setTimeouthandles stored intimersRefare never cleaned up when the provider unmounts. While this is unlikely to cause user-visible issues (the provider typically lives for the app lifetime), it's good hygiene to clear pending timers on teardown.Add a cleanup effect
+ import { useEffect } from "react"; ... const timersRef = useRef<Map<string, ReturnType<typeof setTimeout>>>(new Map()); + + useEffect(() => { + return () => { + for (const timer of timersRef.current.values()) { + clearTimeout(timer); + } + timersRef.current.clear(); + }; + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/hooks/useLiveContext.tsx` around lines 46 - 76, The markEdgeActive implementation uses timersRef to track setTimeout IDs but never clears them on unmount; add a cleanup effect in the same hook (useLiveContext / the provider component) that iterates timersRef.current, calls clearTimeout for each stored timeout, clears timersRef.current and optionally resets activeLinks (via setActiveLinks(new Set())) to ensure no dangling timers or leftover state (timersRef, LINK_ACTIVE_DURATION and markEdgeActive should be referenced to locate the logic).interface/src/api/client.ts (1)
989-994: Use LinkDirection/LinkKind for TopologyLink.This keeps topology responses aligned with the rest of the link type system and avoids stringly‑typed drift.
♻️ Suggested tweak
export interface TopologyLink { from: string; to: string; - direction: string; - kind: string; + direction: LinkDirection; + kind: LinkKind; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/api/client.ts` around lines 989 - 994, The TopologyLink interface currently types direction and kind as plain strings; update it to use the existing LinkDirection and LinkKind types to avoid stringly-typed drift: change TopologyLink's direction: string and kind: string to direction: LinkDirection and kind: LinkKind, and add the necessary import or reference to LinkDirection and LinkKind where TopologyLink is declared; ensure any code constructing or reading TopologyLink values is updated to use those enums/types.src/api/links.rs (1)
510-552: No way to clear a group color once set.
UpdateGroupRequestcan set a new color but can’t remove it. Consider treating an empty string as clear, or useOption<Option<String>>so clients can sendnullto delete the color.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/links.rs` around lines 510 - 552, The update flow currently accepts request.color only as Some(String) and can’t clear an existing color; change the UpdateGroupRequest color field to Option<Option<String>> (outer Option = field present, inner Option = value or explicit null) and update handling in the group update code: when request.color is Some(None) set updated.color = None and remove the "color" entry from the toml table (e.g., table.remove("color")), when request.color is Some(Some(s)) set updated.color = Some(s.clone()) and write table["color"] = toml_edit::value(s.as_str()), and when request.color is None leave the existing color unchanged; update any client parsing/validation accordingly so null can be sent to clear the color.src/config.rs (1)
3885-3962: Config hot‑reload updates links but leaves groups/humans stale.If config.toml is edited manually, links refresh but groups/humans won’t reflect until restart. Consider reloading those sections too (or explicitly document that only links are hot‑reloaded).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 3885 - 3962, The hot-reload currently updates llm_manager via llm_manager.reload_config and replaces bindings and agent_links (bindings.store and agent_links.store after parsing with AgentLink::from_config) but does not refresh groups/humans (or other instance-level sections) from the newly loaded Config; extend the block that runs when new_config is Some(...) to also reload config.groups and config.humans (and any provider keys/permissions fields present on Config, e.g., config.providers or config.permissions) by storing them into the same in-memory stores used elsewhere (e.g., groups_store.store(Arc::new(config.groups.clone())) and humans_store.store(Arc::new(config.humans.clone()))) and add tracing::info()/tracing::error() around those operations similar to the existing bindings/links handling so groups/humans reflect edits without a restart.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
docs/bun.lockis excluded by!**/*.lock,!**/*.lockinterface/bun.lockis excluded by!**/*.lock,!**/*.lockinterface/package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsoninterface/package.jsonis excluded by!**/*.json
📒 Files selected for processing (46)
docs/content/docs/(core)/agents.mdxdocs/content/docs/(deployment)/metrics.mdxdocs/design-docs/multi-agent-communication-graph.mdinterface/src/api/client.tsinterface/src/components/ChannelCard.tsxinterface/src/components/CreateAgentDialog.tsxinterface/src/components/TopologyGraph.tsxinterface/src/components/WebChatPanel.tsxinterface/src/hooks/useChannelLiveState.tsinterface/src/hooks/useLiveContext.tsxinterface/src/router.tsxinterface/src/routes/ChannelDetail.tsxinterface/src/routes/Overview.tsxinterface/src/ui/style/style.scssprompts/en/channel.md.j2prompts/en/fragments/link_context.md.j2prompts/en/fragments/org_context.md.j2prompts/en/tools/conclude_link_description.md.j2prompts/en/tools/send_agent_message_description.md.j2src/agent/channel.rssrc/agent/status.rssrc/api.rssrc/api/agents.rssrc/api/channels.rssrc/api/links.rssrc/api/server.rssrc/api/state.rssrc/api/system.rssrc/config.rssrc/conversation/channels.rssrc/conversation/history.rssrc/daemon.rssrc/db.rssrc/identity/files.rssrc/lib.rssrc/links.rssrc/links/types.rssrc/main.rssrc/prompts/engine.rssrc/prompts/text.rssrc/tools.rssrc/tools/conclude_link.rssrc/tools/reply.rssrc/tools/send_agent_message.rstests/bulletin.rstests/context_dump.rs
Member
Author
Review feedback addressedWent through the CodeRabbit review and fixed the valid findings. Build compiles, clippy is clean (no new warnings). Compile errors (3)
Bugs (5)
Minor (4)
|
- Updated agent ID handling to use display names where available. - Improved duplicate link detection to account for bidirectional links. - Refactored channel deletion to use transactions for better consistency. - Added logging for channel ID retrieval in agent links. - Enhanced prompt rendering to support additional parameters. - Updated tests to initialize agent names for better coverage.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/api/links.rs (1)
200-244:⚠️ Potential issue | 🟠 MajorSerialize config.toml mutations to avoid lost updates.
Line 200-244 performs read → modify → write without a shared lock; concurrent link/group/human mutations can clobber each other in memory and on disk. Consider guarding all config writes with a shared async mutex (or a dedicated config writer) and refreshing state from the written snapshot. This same pattern appears in update/delete for links, groups, and humans.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/links.rs` around lines 200 - 244, The read->modify->write to config.toml (using config_path, content.parse -> doc, links_array.push and tokio::fs::write) is racy and can clobber concurrent mutations; protect all config mutations (add/update/delete for links/groups/humans) with a shared async mutex or a single dedicated config-writer task so only one write happens at a time, perform the read/parse/modify/write while holding that lock, then refresh in-memory state via state.set_agent_links (and the analogous state setters for groups/humans) from the written snapshot before releasing the lock to ensure memory and disk remain consistent.src/agent/channel.rs (1)
1111-1168:⚠️ Potential issue | 🟠 MajorFix link conclusion sender/target attribution.
Line 1158-1168 currently sets
sender_id/formatted_authorto the peer andagent_idto the concluding agent, which can misattribute the message and route to the wrong agent. Usereply_to_agentfrom metadata as the target and attribute the sender to the concluding agent.Suggested fix
- // Derive the peer agent name from the link channel conversation_id - let peer_agent = self - .conversation_id - .as_deref() - .and_then(|cid| cid.strip_prefix("link:")) - .and_then(|rest| { - // Format is "link:{self}:{peer}", skip past self - let parts: Vec<&str> = rest.splitn(2, ':').collect(); - parts.get(1).copied() - }) - .map(|id| { - self.deps - .agent_names - .get(id) - .cloned() - .unwrap_or_else(|| id.to_string()) - }) - .unwrap_or_else(|| "agent".to_string()); + let concluding_agent = self.agent_display_name().to_string(); @@ - let conclusion_text = format!( - "[Link conversation with {} concluded]\n{}", - peer_agent, summary - ); + let conclusion_text = format!( + "[Link conversation with {} concluded]\n{}", + concluding_agent, summary + ); @@ - metadata.insert("link_conclusion_peer".into(), serde_json::json!(peer_agent)); + metadata.insert("link_conclusion_peer".into(), serde_json::json!(concluding_agent)); + + let target_agent = last_message + .metadata + .get("reply_to_agent") + .and_then(|v| v.as_str()) + .map(Arc::from) + .unwrap_or_else(|| self.deps.agent_id.clone()); @@ - sender_id: peer_agent.clone(), - agent_id: Some(self.deps.agent_id.clone()), + sender_id: self.deps.agent_id.to_string(), + agent_id: Some(target_agent), @@ - formatted_author: Some(format!("[{}]", peer_agent)), + formatted_author: Some(format!("[{}]", concluding_agent)), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agent/channel.rs` around lines 1111 - 1168, The conclusion message currently attributes sender_id/formatted_author to the peer_agent and agent_id to the concluding agent, causing misrouting; change construction of the conclusion_message so the sender is the concluding agent and the target agent comes from the captured metadata key "reply_to_agent": set sender_id to self.deps.agent_id (or the concluding agent identifier), set formatted_author to the concluding agent display (e.g. format!("[{}]", self.deps.agent_names.get(&self.deps.agent_id).cloned().unwrap_or(self.deps.agent_id.clone()))), and set agent_id to the reply_to_agent value pulled from the metadata (falling back to the existing self.deps.agent_id if missing); keep conversation_id as originating and preserve peer_agent in metadata for context.
🧹 Nitpick comments (1)
src/prompts/engine.rs (1)
490-512: Consider constrainingrelationshipto a typed enum.If relationship values are limited (e.g., peer/superior/subordinate), a small enum can prevent invalid strings from leaking into prompts. If free‑form values are intended, feel free to ignore.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/prompts/engine.rs` around lines 490 - 512, Replace the free-form relationship String in LinkContext with a small typed enum to prevent invalid values: add an enum (e.g., Relationship with variants Peer, Superior, Subordinate, maybe Other(String) if you need an escape hatch) deriving Debug, Clone, serde::Serialize (and Deserialize) and use serde attributes (e.g., rename_all = "lowercase" or a custom Serialize/Deserialize) so JSON/text stays the same; then change LinkContext.relationship: String -> Relationship and update any constructors/tests/places that build LinkContext to construct the enum variant instead of passing raw strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/agent/channel.rs`:
- Around line 819-863: The code seeds self.state.history with
original_sent_message unconditionally for internal messages, causing
duplicate/misattributed first inbound messages; update the block that checks
metadata.get("original_sent_message") in channel.rs to only seed when the
incoming message is actually a reply targeting this agent by comparing the
originating identifiers in metadata to this channel's identifiers (e.g. check
metadata "originating_source" vs this adapter/source field and/or
"originating_channel" vs self.state.channel_id or self.originating_channel)
before pushing into self.state.history and calling
conversation_logger.log_bot_message_with_name; keep the existing logic that sets
self.originating_source/originating_channel but gate the seeding on a match so
initial inbound messages are not seeded.
In `@src/api/agents.rs`:
- Around line 664-680: The agent_names map is built only from existing
state.agent_configs, so the newly created agent won't appear; update the
agent_names construction (the block that uses state.agent_configs.load() and
maps c.id -> c.display_name) to also insert the new agent's entry (use the new
agent's id and display_name fallback to id) into the collected map before
wrapping in Arc (i.e., add (agent.id.clone(),
agent.display_name.clone().unwrap_or_else(|| agent.id.clone())) into the
Vec/HashMap you collect in the agent_names block so the new agent resolves its
own display name immediately).
In `@src/api/links.rs`:
- Around line 491-519: In update_group, reject empty names by validating
request.name before applying or checking conflicts: if request.name is Some and
request.name.trim().is_empty() then return Err(StatusCode::BAD_REQUEST); perform
this check (using request.name or new_name) before the conflict check that looks
for existing.iter().any(|g| g.name == new_name), and before assigning
updated.name so no empty string can be saved.
---
Duplicate comments:
In `@src/agent/channel.rs`:
- Around line 1111-1168: The conclusion message currently attributes
sender_id/formatted_author to the peer_agent and agent_id to the concluding
agent, causing misrouting; change construction of the conclusion_message so the
sender is the concluding agent and the target agent comes from the captured
metadata key "reply_to_agent": set sender_id to self.deps.agent_id (or the
concluding agent identifier), set formatted_author to the concluding agent
display (e.g. format!("[{}]",
self.deps.agent_names.get(&self.deps.agent_id).cloned().unwrap_or(self.deps.agent_id.clone()))),
and set agent_id to the reply_to_agent value pulled from the metadata (falling
back to the existing self.deps.agent_id if missing); keep conversation_id as
originating and preserve peer_agent in metadata for context.
In `@src/api/links.rs`:
- Around line 200-244: The read->modify->write to config.toml (using
config_path, content.parse -> doc, links_array.push and tokio::fs::write) is
racy and can clobber concurrent mutations; protect all config mutations
(add/update/delete for links/groups/humans) with a shared async mutex or a
single dedicated config-writer task so only one write happens at a time, perform
the read/parse/modify/write while holding that lock, then refresh in-memory
state via state.set_agent_links (and the analogous state setters for
groups/humans) from the written snapshot before releasing the lock to ensure
memory and disk remain consistent.
---
Nitpick comments:
In `@src/prompts/engine.rs`:
- Around line 490-512: Replace the free-form relationship String in LinkContext
with a small typed enum to prevent invalid values: add an enum (e.g.,
Relationship with variants Peer, Superior, Subordinate, maybe Other(String) if
you need an escape hatch) deriving Debug, Clone, serde::Serialize (and
Deserialize) and use serde attributes (e.g., rename_all = "lowercase" or a
custom Serialize/Deserialize) so JSON/text stays the same; then change
LinkContext.relationship: String -> Relationship and update any
constructors/tests/places that build LinkContext to construct the enum variant
instead of passing raw strings.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/agent/channel.rssrc/api/agents.rssrc/api/links.rssrc/conversation/channels.rssrc/links/types.rssrc/prompts/engine.rssrc/tools/conclude_link.rssrc/tools/send_agent_message.rstests/bulletin.rstests/context_dump.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/tools/conclude_link.rs
- Simplified conditional checks for display name, role, and bio in agent and link creation. - Enhanced readability by using `&&` in if let statements. - Added clippy lints to address complexity in `add_channel_tools` and `SendAgentMessageTool` constructors.
- Improved agent name handling by ensuring unique display names are added to the agent names map. - Added validation to prevent empty group names during link updates, returning a BAD_REQUEST status when necessary.
This was referenced Feb 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Foundation for 0.2.0 — adds a directed communication graph between agents so they can coordinate, delegate, escalate, and share context through explicit links with relationship semantics.
[[links]]config sections, hot-reloadable via the existing file watcher. Config is already runtime-mutable through the API so no separate database needed.send_agent_messagetool resolves target agent, validates link direction, constructs anInboundMessagewithsource: "internal", and pushes throughMessagingManager::inject_message(). No new transport.ROLE.mdidentity file for operational responsibilities — separates what the agent does from who it is (IDENTITY.md) and how it behaves (SOUL.md).AgentMessageSent/AgentMessageReceivedwith SSE forwarding for dashboard visibility./api/links), per-agent links (/api/agents/:id/links), topology snapshot (/api/topology) for the future React Flow graph editor.Note
Technical implementation: New
src/links/module with types, config parsing, and link resolution utilities. UI addsTopologyGraphcomponent for future visualization. Adds two new tools (send_agent_message,conclude_link) with validation logic to prevent messaging across invalid relationships. Context injection happens via prompt fragments (link_context,org_context) that dynamically load agent relationships. All link definitions flow through existing config hot-reload system without requiring database changes. Test coverage updated for new topology APIs.Written by Tembo for commit 5816be9