diff --git a/src/openhuman/agent/bus.rs b/src/openhuman/agent/bus.rs index 4c0fa6bf1..cf8a279c4 100644 --- a/src/openhuman/agent/bus.rs +++ b/src/openhuman/agent/bus.rs @@ -13,6 +13,7 @@ //! See [`crate::openhuman::channels::runtime::dispatch`] for the primary //! caller. +use std::collections::HashSet; use std::sync::Arc; use tokio::sync::mpsc; @@ -84,6 +85,33 @@ pub struct AgentTurnRequest { /// chunks here so channel providers can update "draft" messages in /// real time. `None` disables streaming for this turn. pub on_delta: Option>, + + // ── Per-agent scoping (issues #525 / #526) ──────────────────────── + /// Identifier of the agent definition this turn represents (e.g. + /// `"orchestrator"`, `"welcome"`). Used for structured tracing and + /// downstream bookkeeping; the actual filtering is driven by + /// [`Self::visible_tool_names`] and [`Self::extra_tools`] below. + /// `None` preserves the legacy "generic unfiltered turn" behaviour. + pub target_agent_id: Option, + + /// Whitelist of tool names visible to the LLM this turn. When + /// `Some(set)`, the bus handler filters both the function-calling + /// schema and the tool-execution lookup to names in the set. + /// Pre-built on the dispatch side from the target agent's + /// definition (its `[tools] named` list unioned with the names of + /// any per-turn synthesised delegation tools). `None` means no + /// filter — every tool in `tools_registry` plus `extra_tools` is + /// visible. + pub visible_tool_names: Option>, + + /// Per-turn synthesised tools to splice alongside `tools_registry`. + /// The dispatch path uses this to carry `ArchetypeDelegationTool` / + /// `SkillDelegationTool` instances built fresh each turn from the + /// active agent's `subagents` field and the current Composio + /// integrations — tools that don't exist in the global startup + /// registry because they depend on per-user runtime state. + /// Empty vec for agents that don't delegate. + pub extra_tools: Vec>, } /// Final response from an agentic turn. @@ -114,14 +142,21 @@ pub fn register_agent_handlers() { multimodal, max_tool_iterations, on_delta, + target_agent_id, + visible_tool_names, + extra_tools, } = req; tracing::debug!( channel = %channel_name, + target_agent = target_agent_id.as_deref().unwrap_or(""), provider = %provider_name, model = %model, history_len = history.len(), tool_count = tools_registry.len(), + extra_tool_count = extra_tools.len(), + visible_tool_count = visible_tool_names.as_ref().map(|s| s.len()).unwrap_or(0), + filter_active = visible_tool_names.is_some(), streaming = on_delta.is_some(), "[agent::bus] dispatching {AGENT_RUN_TURN_METHOD}" ); @@ -143,6 +178,8 @@ pub fn register_agent_handlers() { &multimodal, max_tool_iterations, on_delta, + visible_tool_names.as_ref(), + &extra_tools, ) .await .map_err(|e| e.to_string())?; @@ -286,6 +323,9 @@ mod tests { multimodal: MultimodalConfig::default(), max_tool_iterations: 1, on_delta: None, + target_agent_id: None, + visible_tool_names: None, + extra_tools: Vec::new(), } } diff --git a/src/openhuman/agent/harness/tests.rs b/src/openhuman/agent/harness/tests.rs index 8aead7732..85cba5128 100644 --- a/src/openhuman/agent/harness/tests.rs +++ b/src/openhuman/agent/harness/tests.rs @@ -124,6 +124,8 @@ async fn run_tool_call_loop_returns_structured_error_for_non_vision_provider() { &crate::openhuman::config::MultimodalConfig::default(), 3, None, + None, + &[], ) .await .expect_err("provider without vision support should fail"); @@ -165,6 +167,8 @@ async fn run_tool_call_loop_rejects_oversized_image_payload() { &multimodal, 3, None, + None, + &[], ) .await .expect_err("oversized payload must fail"); @@ -200,6 +204,8 @@ async fn run_tool_call_loop_accepts_valid_multimodal_request_flow() { &crate::openhuman::config::MultimodalConfig::default(), 3, None, + None, + &[], ) .await .expect("valid multimodal payload should pass"); diff --git a/src/openhuman/agent/harness/tool_loop.rs b/src/openhuman/agent/harness/tool_loop.rs index cc53c7a72..324e55b80 100644 --- a/src/openhuman/agent/harness/tool_loop.rs +++ b/src/openhuman/agent/harness/tool_loop.rs @@ -4,12 +4,13 @@ use crate::openhuman::providers::{ChatMessage, ChatRequest, Provider, ProviderCa use crate::openhuman::tools::traits::ToolScope; use crate::openhuman::tools::Tool; use anyhow::Result; +use std::collections::HashSet; use std::fmt::Write as _; use std::io::Write as _; use super::credentials::scrub_credentials; use super::parse::{ - build_native_assistant_history, find_tool, parse_structured_tool_calls, parse_tool_calls, + build_native_assistant_history, parse_structured_tool_calls, parse_tool_calls, }; use crate::openhuman::context::guard::{ContextCheckResult, ContextGuard}; @@ -23,6 +24,11 @@ pub(crate) const DEFAULT_MAX_TOOL_ITERATIONS: usize = 10; /// Execute a single turn of the agent loop: send messages, parse tool calls, /// execute tools, and loop until the LLM produces a final text response. /// When `silent` is true, suppresses stdout (for channel use). +/// +/// This is a thin wrapper around [`run_tool_call_loop`] with the per-agent +/// filter and extra-tool plumbing disabled — i.e. the LLM sees the entire +/// `tools_registry` unchanged. Used by legacy call sites and harness tests +/// that don't need agent-aware scoping. #[allow(clippy::too_many_arguments)] pub(crate) async fn agent_turn( provider: &dyn Provider, @@ -48,12 +54,40 @@ pub(crate) async fn agent_turn( multimodal_config, max_tool_iterations, None, + None, + &[], ) .await } /// Execute a single turn of the agent loop: send messages, parse tool calls, /// execute tools, and loop until the LLM produces a final text response. +/// +/// # Per-agent tool scoping +/// +/// The last two parameters support per-agent tool filtering without +/// requiring callers to build a filtered copy of the (non-`Clone`able) +/// tool registry: +/// +/// * `visible_tool_names` — optional whitelist of tool names that are +/// allowed to reach the LLM. When `Some(set)`, only tools whose +/// `name()` is present in the set contribute to the function-calling +/// schema and are eligible for execution; every other tool in the +/// registry is hidden from the model and rejected if the model +/// somehow emits a call for it. When `None`, no filtering is applied +/// and every tool in the combined registry is visible (the legacy +/// behaviour used by CLI/REPL and harness tests). +/// +/// * `extra_tools` — per-turn synthesised tools to splice alongside the +/// persistent `tools_registry`. The agent-dispatch path uses this to +/// surface delegation tools (`research`, `delegate_gmail`, …) that +/// are synthesised fresh per turn from the active agent's +/// `subagents` field and the current Composio integration list, and +/// therefore are not registered in the global startup-time registry. +/// +/// The combined tool list seen by the LLM this turn is +/// `tools_registry.iter().chain(extra_tools.iter())`, further narrowed +/// by `visible_tool_names` when supplied. #[allow(clippy::too_many_arguments)] pub(crate) async fn run_tool_call_loop( provider: &dyn Provider, @@ -68,6 +102,8 @@ pub(crate) async fn run_tool_call_loop( multimodal_config: &crate::openhuman::config::MultimodalConfig, max_tool_iterations: usize, on_delta: Option>, + visible_tool_names: Option<&HashSet>, + extra_tools: &[Box], ) -> Result { let max_iterations = if max_tool_iterations == 0 { DEFAULT_MAX_TOOL_ITERATIONS @@ -75,16 +111,34 @@ pub(crate) async fn run_tool_call_loop( max_tool_iterations }; - let tool_specs: Vec = - tools_registry.iter().map(|tool| tool.spec()).collect(); + // Is a given tool name visible to the model this turn? `None` + // means no filter (legacy behaviour = everything visible). + let is_visible = |name: &str| -> bool { + match visible_tool_names { + Some(set) => set.contains(name), + None => true, + } + }; + + let tool_specs: Vec = tools_registry + .iter() + .chain(extra_tools.iter()) + .filter(|tool| is_visible(tool.name())) + .map(|tool| tool.spec()) + .collect(); let use_native_tools = provider.supports_native_tools() && !tool_specs.is_empty(); log::debug!( - "[tool-loop] Registry has {} tool(s): [{}]", + "[tool-loop] Registry has {} tool(s), extra {} tool(s), filter={} — {} visible in schema: [{}]", tools_registry.len(), - tools_registry + extra_tools.len(), + visible_tool_names + .map(|s| format!("whitelist({})", s.len())) + .unwrap_or_else(|| "none".to_string()), + tool_specs.len(), + tool_specs .iter() - .map(|t| t.name()) + .map(|s| s.name.as_str()) .collect::>() .join(", ") ); @@ -289,7 +343,16 @@ pub(crate) async fn run_tool_call_loop( } } - let tool_opt = find_tool(tools_registry, &call.name); + // Look up the tool by name in the combined registry + extras, + // subject to the visibility whitelist. If the model hallucinated + // a filtered-out tool name we treat it as unknown — the error + // path below produces a structured error message the LLM can + // correct in the next iteration. + let tool_opt: Option<&dyn Tool> = tools_registry + .iter() + .chain(extra_tools.iter()) + .find(|t| t.name() == call.name && is_visible(t.name())) + .map(|b| b.as_ref()); tracing::debug!( iteration, tool = call.name.as_str(), @@ -563,6 +626,8 @@ mod tests { &crate::openhuman::config::MultimodalConfig::default(), 1, None, + None, + &[], ) .await .expect_err("vision markers should be rejected"); @@ -597,6 +662,8 @@ mod tests { &crate::openhuman::config::MultimodalConfig::default(), 1, Some(tx), + None, + &[], ) .await .expect("final text should succeed"); @@ -646,6 +713,8 @@ mod tests { &crate::openhuman::config::MultimodalConfig::default(), 2, None, + None, + &[], ) .await .expect("loop should recover after denial"); @@ -698,6 +767,8 @@ mod tests { &crate::openhuman::config::MultimodalConfig::default(), 2, None, + None, + &[], ) .await .expect("native tool flow should succeed"); @@ -753,6 +824,8 @@ mod tests { &crate::openhuman::config::MultimodalConfig::default(), 2, None, + None, + &[], ) .await .expect("non-cli channels should auto-approve supervised tools"); @@ -801,6 +874,8 @@ mod tests { &crate::openhuman::config::MultimodalConfig::default(), 0, None, + None, + &[], ) .await .expect("default iteration fallback should still succeed"); @@ -853,6 +928,8 @@ mod tests { &crate::openhuman::config::MultimodalConfig::default(), 2, None, + None, + &[], ) .await .expect("loop should recover after tool errors"); @@ -889,6 +966,8 @@ mod tests { &crate::openhuman::config::MultimodalConfig::default(), 1, None, + None, + &[], ) .await .expect_err("provider error path should fail"); @@ -918,6 +997,8 @@ mod tests { &crate::openhuman::config::MultimodalConfig::default(), 1, None, + None, + &[], ) .await .expect_err("loop should stop after configured iterations"); diff --git a/src/openhuman/agent/triage/evaluator.rs b/src/openhuman/agent/triage/evaluator.rs index 259cc05ed..d390ff10b 100644 --- a/src/openhuman/agent/triage/evaluator.rs +++ b/src/openhuman/agent/triage/evaluator.rs @@ -229,6 +229,14 @@ pub async fn run_triage_with_resolved( // cap is only a safety net. max_tool_iterations: 1, on_delta: None, + // The triage classifier runs against an empty tools registry + // by design and emits a structured JSON decision rather than + // calling tools — record the agent identity for tracing but + // leave the visible-tool filter unset so the legacy unfiltered + // behaviour is preserved. + target_agent_id: Some("trigger_triage".to_string()), + visible_tool_names: None, + extra_tools: Vec::new(), }; tracing::debug!( provider = %provider_name, diff --git a/src/openhuman/channels/runtime/dispatch.rs b/src/openhuman/channels/runtime/dispatch.rs index 540cfc776..54cceade4 100644 --- a/src/openhuman/channels/runtime/dispatch.rs +++ b/src/openhuman/channels/runtime/dispatch.rs @@ -389,6 +389,13 @@ pub(crate) async fn process_channel_message( multimodal: ctx.multimodal.clone(), max_tool_iterations: ctx.max_tool_iterations, on_delta: delta_tx, + // Per-agent scoping fields are populated in commit 4b (the + // dispatch routing logic for #525). For now, leave them at + // their defaults so this commit ships zero behaviour change — + // every channel turn still sees the full unfiltered registry. + target_agent_id: None, + visible_tool_names: None, + extra_tools: Vec::new(), }; tracing::debug!( channel = %msg.channel, diff --git a/src/openhuman/tools/impl/agent/mod.rs b/src/openhuman/tools/impl/agent/mod.rs index 1f0354d8f..ffea55e18 100644 --- a/src/openhuman/tools/impl/agent/mod.rs +++ b/src/openhuman/tools/impl/agent/mod.rs @@ -15,7 +15,7 @@ pub(crate) async fn dispatch_subagent( agent_id: &str, tool_name: &str, prompt: &str, - _skill_filter: Option<&str>, + skill_filter: Option<&str>, ) -> anyhow::Result { let registry = match AgentDefinitionRegistry::global() { Some(reg) => reg, @@ -51,14 +51,22 @@ pub(crate) async fn dispatch_subagent( }); log::info!( - "[agent] delegating to {} via {} prompt_chars={}", + "[agent] delegating to {} via {} (skill_filter={}) prompt_chars={}", agent_id, tool_name, + skill_filter.unwrap_or(""), prompt.chars().count() ); + // Propagate the per-call skill filter into the subagent runner so + // that `SkillDelegationTool`s can narrow `skills_agent` to a single + // Composio toolkit (e.g. `delegate_gmail` → skills_agent + + // skill_filter="gmail"). Previously this argument was hardcoded to + // `None`, which meant the toolkit pre-selection never reached the + // subagent and skills_agent always saw the full Composio catalog — + // the downstream half of the #526 leak. let options = SubagentRunOptions { - skill_filter_override: None, + skill_filter_override: skill_filter.map(str::to_string), category_filter_override: None, context: None, task_id: Some(task_id.clone()),