mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
Adds the parameter plumbing for agent-aware tool filtering without
changing any runtime behaviour. Every existing call site continues to
pass `None` / empty extras, so the LLM still sees the full unfiltered
registry — the actual routing logic that populates these fields lands
in commit 4b (dispatch.rs onboarding-flag → target_agent_id).
Why two parameters and not one filter:
Tools in this codebase are `Box<dyn Tool>` — owned trait objects with
no Clone impl, stored in a shared `Arc<Vec<Box<dyn Tool>>>`. We can't
cheaply build a per-turn filtered subset of the global registry, and
we can't mutate the Arc to remove entries. Two new parameters work
around this without touching the global registry's lifetime model:
* `visible_tool_names: Option<&HashSet<String>>` — whitelist filter
applied at the iteration site inside `run_tool_call_loop`. When
`Some(set)`, only tools whose `name()` is in the set contribute to
the function-calling schema and are eligible for execution; every
other tool in the combined registry is hidden from the model and
rejected if the model emits a call for it. `None` preserves the
legacy "everything visible" behaviour.
* `extra_tools: &[Box<dyn Tool>]` — per-turn synthesised tools spliced
alongside `tools_registry`. The dispatch path will use this to
surface delegation tools (`research`, `delegate_gmail`, …) that are
built fresh each turn from the active agent's `subagents` field and
the current Composio integration list — tools that don't exist in
the global startup-time registry because they depend on per-user
runtime state. Empty slice for agents that don't delegate.
Inside the loop, `tool_specs` is built from
`tools_registry.iter().chain(extra_tools.iter()).filter(is_visible)`,
and the tool-execution lookup uses the same chain + filter so the
function-calling schema and the execution surface stay in sync.
Files touched in this commit:
src/openhuman/agent/harness/tool_loop.rs
- Add `visible_tool_names` and `extra_tools` parameters to
`run_tool_call_loop`. Build `tool_specs` from chained iteration
with the visibility filter applied. Replace the `find_tool` call
at the execution site with an inline chain+filter lookup so
hallucinated calls to filtered-out tools surface as "unknown
tool" errors. Drop the now-unused `find_tool` import.
- Update the legacy `agent_turn` wrapper to pass `None, &[]`,
preserving its existing unfiltered behaviour.
- Update all 9 in-file test sites to pass `None, &[]`.
src/openhuman/agent/harness/tests.rs
- Update all 3 `run_tool_call_loop` test sites to pass `None, &[]`.
src/openhuman/agent/bus.rs
- Add `target_agent_id: Option<String>`, `visible_tool_names:
Option<HashSet<String>>`, and `extra_tools: Vec<Box<dyn Tool>>`
fields to `AgentTurnRequest`, with rustdoc explaining each.
- Destructure the new fields in the `agent.run_turn` handler;
thread `visible_tool_names.as_ref()` and `&extra_tools` through
to `run_tool_call_loop`. Augment the dispatch trace with
target_agent / extra_tool_count / visible_tool_count /
filter_active so production logs show whether scoping is active.
- Update the in-test `test_request()` helper to populate the new
fields with safe defaults.
src/openhuman/agent/triage/evaluator.rs
- Update the triage `AgentTurnRequest` initializer to set
`target_agent_id = Some("trigger_triage")` (for tracing) with
`visible_tool_names: None` + `extra_tools: Vec::new()` because
the classifier intentionally runs against an empty registry and
emits a structured JSON decision rather than calling tools.
src/openhuman/channels/runtime/dispatch.rs
- Update the channel-message `AgentTurnRequest` initializer to set
the three new fields to safe defaults (`None` / `None` / empty
vec). Commit 4b will replace these with the real onboarding-flag
based routing.
src/openhuman/tools/impl/agent/mod.rs
- Bug fix: `dispatch_subagent` previously took `_skill_filter:
Option<&str>` but discarded the value, hardcoding
`SubagentRunOptions::skill_filter_override = None`. That meant
`SkillDelegationTool::execute()` synthesising
`dispatch_subagent("skills_agent", ..., Some("gmail"))` never
actually narrowed `skills_agent`'s tool list — so even with the
orchestrator's view scoped, the spawned `skills_agent` subagent
would still see the full Composio catalog. Drop the underscore,
propagate `skill_filter` into `skill_filter_override`, and add a
tracing log line to make this path observable. This is the
downstream half of the #526 leak that commit 3's orchestrator-
side scoping alone wouldn't have caught.
Tests: 8/8 `tool_loop` tests pass, 3/3 harness `tests.rs` cases pass,
323/324 agent module tests pass overall (the one failure is the same
pre-existing Windows-path bug in `self_healing::tool_maker_prompt_
includes_command` that fails identically on the upstream baseline).
No existing test expectations were changed.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
d1c577d25a
commit
2898612962
@@ -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<mpsc::Sender<String>>,
|
||||
|
||||
// ── 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<String>,
|
||||
|
||||
/// 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<HashSet<String>>,
|
||||
|
||||
/// 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<Box<dyn Tool>>,
|
||||
}
|
||||
|
||||
/// 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("<unset>"),
|
||||
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(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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<tokio::sync::mpsc::Sender<String>>,
|
||||
visible_tool_names: Option<&HashSet<String>>,
|
||||
extra_tools: &[Box<dyn Tool>],
|
||||
) -> Result<String> {
|
||||
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<crate::openhuman::tools::ToolSpec> =
|
||||
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<crate::openhuman::tools::ToolSpec> = 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::<Vec<_>>()
|
||||
.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");
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<ToolResult> {
|
||||
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("<none>"),
|
||||
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()),
|
||||
|
||||
Reference in New Issue
Block a user