diff --git a/app/src/utils/__tests__/toolTimelineFormatting.test.ts b/app/src/utils/__tests__/toolTimelineFormatting.test.ts index b0b2b073c2..5fbbdb0d2e 100644 --- a/app/src/utils/__tests__/toolTimelineFormatting.test.ts +++ b/app/src/utils/__tests__/toolTimelineFormatting.test.ts @@ -68,43 +68,23 @@ describe('formatTimelineEntry', () => { }); }); - it('formats delegate_to_integrations_agent with a known toolkit arg', () => { + it('labels a direct connected-service action by its provider', () => { expect( formatTimelineEntry( - entry({ - name: 'delegate_to_integrations_agent', - argsBuffer: JSON.stringify({ - toolkit: 'gmail', - prompt: 'Find the latest invoice from Stripe.', - }), - }) + entry({ name: 'GMAIL_SEND_EMAIL', argsBuffer: JSON.stringify({ to: 'alex@example.com' }) }) ) - ).toEqual({ - title: 'Making requests to your Gmail account', - detail: 'Find the latest invoice from Stripe.', + ).toEqual({ title: 'Making requests to your Gmail account', detail: 'Send email' }); + expect(formatTimelineEntry(entry({ name: 'GOOGLE_CALENDAR_CREATE_EVENT' }))).toEqual({ + title: 'Updating your Google Calendar', + detail: 'Create event', }); }); - it('formats delegate_to_integrations_agent with an unknown toolkit arg', () => { - expect( - formatTimelineEntry( - entry({ - name: 'delegate_to_integrations_agent', - argsBuffer: JSON.stringify({ toolkit: 'slack_bot', prompt: 'post update' }), - }) - ) - ).toEqual({ title: 'Checking your Slack Bot', detail: 'post update' }); - }); - - it('formats delegate_to_integrations_agent without a toolkit arg as a generic connected-app label', () => { - expect( - formatTimelineEntry( - entry({ - name: 'delegate_to_integrations_agent', - argsBuffer: JSON.stringify({ prompt: 'do something useful' }), - }) - ) - ).toEqual({ title: 'Checking your connected app', detail: 'do something useful' }); + it('keeps the generic label for upper-case names on unknown toolkits', () => { + expect(formatTimelineEntry(entry({ name: 'STRIPE_LIST_CHARGES' }))).toEqual({ + title: 'STRIPE LIST CHARGES', + detail: undefined, + }); }); it('formats delegate_tools_agent with toolkit context from args', () => { @@ -402,7 +382,6 @@ describe('isKnownClientTool', () => { expect(isKnownClientTool('file_read')).toBe(true); expect(isKnownClientTool('shell')).toBe(true); expect(isKnownClientTool('subagent:researcher')).toBe(true); - expect(isKnownClientTool('delegate_to_integrations_agent')).toBe(true); // The streamed search-slot name, so the client label wins over the // server's humanized "Web Search Tool". expect(isKnownClientTool('web_search_tool')).toBe(true); diff --git a/app/src/utils/toolTimelineFormatting.ts b/app/src/utils/toolTimelineFormatting.ts index 96653692ef..8d792a1d7e 100644 --- a/app/src/utils/toolTimelineFormatting.ts +++ b/app/src/utils/toolTimelineFormatting.ts @@ -311,21 +311,23 @@ export function formatTimelineEntry(entry: ToolTimelineEntry): { title: string; inferIntegrationNameFromPrompt(parsedArgs?.prompt) ?? inferIntegrationName(entry.name); - let title: string; - if (provider) { - title = integrationActivityTitle(provider); - } else if (entry.name === 'delegate_to_integrations_agent') { - const rawToolkit = parsedArgs?.toolkit?.trim(); - title = rawToolkit - ? integrationActivityTitle(humanizeIdentifier(rawToolkit)) - : 'Checking your connected app'; - } else { - title = humanizeIdentifier(entry.name); - } - + const title = provider ? integrationActivityTitle(provider) : humanizeIdentifier(entry.name); return { title, detail: entry.detail ?? parsedArgs?.prompt }; } + // A connected-service action called directly (`GMAIL_SEND_EMAIL`, + // `SLACK_SEND_MESSAGE`): the orchestrator finds these through + // `tool_search` and calls them itself, so this is the row a user sees + // for "send that email". Label it by the service, with the action as + // the detail, rather than a raw humanised slug. + const directAction = inferIntegrationActionName(entry.name); + if (directAction) { + return { + title: integrationActivityTitle(directAction.provider), + detail: entry.detail ?? directAction.action, + }; + } + // ── Tool-specific formatting with args-derived detail ────────────── // Pass the completed result text so args-aware formatters can surface // details only known post-execution (e.g. the resolved search provider). @@ -646,6 +648,32 @@ function inferIntegrationName(input?: string): string | undefined { return undefined; } +/** + * Split a Composio action slug (`GMAIL_SEND_EMAIL`) into its known provider + * and a readable action ("Send email"). `undefined` for anything that is not + * an upper-case `_` name on a known toolkit, so ordinary + * tools and unknown toolkits keep their generic label. + */ +function inferIntegrationActionName( + name: string +): { provider: string; action: string } | undefined { + if (!/^[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+$/.test(name)) return undefined; + // Try the longest toolkit prefix first (`GOOGLE_CALENDAR_...`), then the + // shortest (`GMAIL_...`). + const parts = name.split('_'); + for (let i = Math.min(parts.length - 1, 2); i >= 1; i -= 1) { + const toolkit = parts.slice(0, i).join('_'); + if (KNOWN_TOOLKIT_RE.test(toolkit)) { + const action = parts.slice(i).join(' ').toLowerCase(); + return { + provider: normalizeIntegrationName(toolkit), + action: action.charAt(0).toUpperCase() + action.slice(1), + }; + } + } + return undefined; +} + function integrationActivityTitle(provider: string): string { switch (provider) { case 'GitHub': diff --git a/crates/openhuman-app/Cargo.lock b/crates/openhuman-app/Cargo.lock index 4904cec2de..47474fb0a3 100644 --- a/crates/openhuman-app/Cargo.lock +++ b/crates/openhuman-app/Cargo.lock @@ -7502,7 +7502,7 @@ name = "tinyjuice" version = "0.2.5" dependencies = [ "async-trait", - "dirs", + "dirs 7.0.0", "hex", "log", "once_cell", @@ -7522,6 +7522,7 @@ name = "tinyjuice-bus" version = "0.2.5" dependencies = [ "serde", + "serde_json", ] [[package]] diff --git a/crates/openhuman-app/src/lib.rs b/crates/openhuman-app/src/lib.rs index 930c1b8255..09c6bebd42 100644 --- a/crates/openhuman-app/src/lib.rs +++ b/crates/openhuman-app/src/lib.rs @@ -2370,8 +2370,9 @@ pub fn run() { // `core_process::CoreProcessHandle::ensure_running` via // `tokio::spawn(run_server_embedded(..))`) runs *on* that runtime, so // every JSON-RPC handler — including the deep tower - // `web channel chat → orchestrator turn → delegate_to_integrations_agent - // → sub-agent → composio_list_tools → load_config_with_timeout` — + // `web channel chat → orchestrator turn → integration action tool + // → composio execute → load_config_with_timeout` (and, at the time, the + // now-removed integrations sub-agent spawn in between) — // burns through the same 2 MB. In `crahs.log` (2026-05-17, build // 0.53.49) that tower plus the serde-monomorphised `Config` Visitor // frames pushed past the guard page and aborted with diff --git a/crates/openhuman-core/src/agent/bus.rs b/crates/openhuman-core/src/agent/bus.rs index c257cf9580..c70570d96d 100644 --- a/crates/openhuman-core/src/agent/bus.rs +++ b/crates/openhuman-core/src/agent/bus.rs @@ -114,9 +114,9 @@ pub struct AgentTurnRequest { 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 + /// The dispatch path uses this to carry `ArchetypeDelegationTool` + /// instances and deferred Composio action tools 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. diff --git a/crates/openhuman-core/src/agent/debug/mod.rs b/crates/openhuman-core/src/agent/debug/mod.rs index 0d484a29e5..209865d0bd 100644 --- a/crates/openhuman-core/src/agent/debug/mod.rs +++ b/crates/openhuman-core/src/agent/debug/mod.rs @@ -449,8 +449,8 @@ async fn render_integrations_agent(config: &Config, toolkit: &str) -> Result> = rendered_tools .iter() .map(|t| PromptTool { - name: t.name(), - description: t.description(), + name: std::borrow::Cow::Borrowed(t.name()), + description: std::borrow::Cow::Borrowed(t.description()), parameters_schema: Some(t.parameters_schema().to_string()), }) .collect(); diff --git a/crates/openhuman-core/src/agent/harness/definition/agent_definition.rs b/crates/openhuman-core/src/agent/harness/definition/agent_definition.rs index 21275ba78c..8985cb991c 100644 --- a/crates/openhuman-core/src/agent/harness/definition/agent_definition.rs +++ b/crates/openhuman-core/src/agent/harness/definition/agent_definition.rs @@ -208,18 +208,16 @@ pub struct AgentDefinition { /// agent's `delegate_name` override) and whose description is the /// target agent's [`AgentDefinition::when_to_use`]. /// - /// * [`SubagentEntry::Skills`] — a single collapsed - /// [`SkillDelegationTool`] named `delegate_to_integrations_agent` - /// that takes the toolkit slug as an argument and routes to the - /// generic `integrations_agent` with the corresponding - /// `skill_filter` pre-populated (#1335). + /// * [`SubagentEntry::Skills`] — no delegation tool. The connected + /// Composio toolkits' actions join this agent's `Deferred` catalogue + /// (reached through `tool_search`, called directly), and the entry + /// admits no sub-agent id: see [`AgentDefinition::allowed_subagent_ids`]. /// /// `subagents` is intentionally separate from [`AgentDefinition::tools`] /// so that reading a TOML makes the distinction obvious: `tools` is /// "what I execute directly", `subagents` is "what I can delegate to". /// /// [`ArchetypeDelegationTool`]: crate::agent::orchestration::tools::ArchetypeDelegationTool - /// [`SkillDelegationTool`]: crate::agent::orchestration::tools::SkillDelegationTool #[serde(default, deserialize_with = "deserialize_subagent_entries")] pub subagents: Vec, @@ -275,6 +273,24 @@ pub struct AgentDefinition { } impl AgentDefinition { + /// The agent ids this definition may spawn, derived from + /// [`AgentDefinition::subagents`]. Only [`SubagentEntry::AgentId`] + /// entries admit a target; the `{ skills = "*" }` wildcard used to map to + /// `integrations_agent` here, which is what let a chat agent spin up a + /// sub-agent for one integration action it can now search for and call + /// itself. The runner's spawn gate (`parent.allowed_subagent_ids`) reads + /// this, so a definition without a bare id for an agent cannot reach it + /// through `spawn_async_subagent` either. + pub fn allowed_subagent_ids(&self) -> Vec { + self.subagents + .iter() + .filter_map(|entry| match entry { + SubagentEntry::AgentId(id) => Some(id.clone()), + SubagentEntry::Skills(_) => None, + }) + .collect() + } + /// Display name with fallback to id. pub fn display_name(&self) -> &str { self.display_name.as_deref().unwrap_or(&self.id) diff --git a/crates/openhuman-core/src/agent/harness/definition/subagents.rs b/crates/openhuman-core/src/agent/harness/definition/subagents.rs index 24c000bcf5..bfbb0ed7c8 100644 --- a/crates/openhuman-core/src/agent/harness/definition/subagents.rs +++ b/crates/openhuman-core/src/agent/harness/definition/subagents.rs @@ -22,11 +22,11 @@ use serde::{Deserialize, Deserializer, Serialize}; pub enum SubagentEntry { /// Delegate to a specific built-in or custom agent by id. AgentId(String), - /// Expand at build time to a single collapsed - /// `delegate_to_integrations_agent` tool whose `toolkit` argument - /// selects which connected Composio toolkit to route to, with - /// `skill_filter` pre-set on the underlying `integrations_agent` - /// dispatch (#1335). + /// Expand at build time to the connected Composio toolkits' actions as + /// `Deferred` tools — off the wire, found through the harness's + /// `tool_search`, and called directly by this agent. No sub-agent is + /// reachable through this entry: it widens the searchable catalogue, + /// not the spawnable set. Skills(SkillsWildcard), } diff --git a/crates/openhuman-core/src/agent/harness/definition/tier.rs b/crates/openhuman-core/src/agent/harness/definition/tier.rs index 73d734c170..9cdf67cb79 100644 --- a/crates/openhuman-core/src/agent/harness/definition/tier.rs +++ b/crates/openhuman-core/src/agent/harness/definition/tier.rs @@ -79,10 +79,9 @@ impl std::fmt::Display for AgentTier { /// pairs at boot (see /// [`crate::agent::registry::agents::validate_tier_hierarchy`]). The /// runtime spawn gate (`run_subagent`) reuses it as defense-in-depth, but -/// deliberately exempts worker *parents* — at runtime a worker only reaches the -/// spawn chokepoint via the documented collapsed `delegate_to_integrations_agent` -/// path (→ `integrations_agent`, itself a worker), which the loader intentionally -/// leaves untouched. +/// deliberately exempts worker *parents* — a worker's `subagents` list holds +/// no agent id (the loader rejects one), so the only runtime spawn a worker +/// reaches is one the host dispatched for it, not one it chose. pub fn validate_tier_transition(parent: AgentTier, child: AgentTier) -> Result<(), String> { match (parent, child) { (AgentTier::Worker, _) => Err(format!( diff --git a/crates/openhuman-core/src/agent/orchestration/README.md b/crates/openhuman-core/src/agent/orchestration/README.md index a22a39256d..0bebe469fe 100644 --- a/crates/openhuman-core/src/agent/orchestration/README.md +++ b/crates/openhuman-core/src/agent/orchestration/README.md @@ -124,9 +124,11 @@ name: - Control: `steer_subagent`, `continue_subagent`, `close_subagent`, `wait_subagent`, `wait`, `wait_loop`, `list_subagents`. - Delegation: `DelegateGraphTool` (`delegate_graph.rs`), - `ArchetypeDelegationTool` and `SkillDelegationTool` (names set per - instance, e.g. `delegate_to_integrations_agent`), `CollapsedDelegationTool` - (`delegate_to`), and `agent_prepare_context`. + `ArchetypeDelegationTool` (name set per instance, e.g. `research`), + `CollapsedDelegationTool` (`delegate_to`), and `agent_prepare_context`. + There is no integrations delegate: connected Composio actions are + `Deferred` tools on the orchestrator's own belt, found through + `tool_search` and called directly (`tools/orchestrator_tools.rs`). `dispatch.rs` (`dispatch_subagent`, the shared spawn path every tool above calls), `awaiting_user.rs` (the awaiting-user envelope), and diff --git a/crates/openhuman-core/src/agent/orchestration/tools.rs b/crates/openhuman-core/src/agent/orchestration/tools.rs index a69e29af82..a5cad62ba5 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools.rs @@ -8,8 +8,7 @@ //! - **Control**: `steer_subagent`, `continue_subagent`, `close_subagent`, //! `wait_subagent`, `wait` / `wait_loop`, `list_subagents`. //! - **Delegation**: `DelegateGraphTool`, `ArchetypeDelegationTool`, -//! `SkillDelegationTool`, `CollapsedDelegationTool` (`delegate_to`), and -//! `agent_prepare_context`. +//! `CollapsedDelegationTool` (`delegate_to`), and `agent_prepare_context`. //! //! `dispatch.rs`, `awaiting_user.rs`, and `worker_thread.rs` are `pub(crate)` //! helpers shared by the tools above (the common spawn path, the awaiting-user @@ -39,8 +38,6 @@ mod delegate_graph; mod dispatch; #[path = "tools/list_subagents.rs"] mod list_subagents; -#[path = "tools/skill_delegation.rs"] -mod skill_delegation; #[path = "tools/spawn_async_subagent.rs"] mod spawn_async_subagent; #[path = "tools/spawn_parallel_agents.rs"] @@ -94,7 +91,6 @@ pub(crate) use delegate_graph::DelegateGraphDispatch; pub use delegate_graph::DelegateGraphTool; pub(crate) use list_subagents::ListSubagentsDispatch; pub use list_subagents::ListSubagentsTool; -pub use skill_delegation::{SkillDelegationTool, INTEGRATIONS_DELEGATE_TOOL_NAME}; pub(crate) use spawn_async_subagent::SpawnAsyncSubagentDispatch; pub use spawn_async_subagent::{scope_spawn_async_subagent_spec, SpawnAsyncSubagentTool}; pub(crate) use spawn_parallel_agents::SpawnParallelAgentsDispatch; diff --git a/crates/openhuman-core/src/agent/orchestration/tools/agent_prepare_context_tests.rs b/crates/openhuman-core/src/agent/orchestration/tools/agent_prepare_context_tests.rs index 5b0bc37529..e3b77490f0 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools/agent_prepare_context_tests.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools/agent_prepare_context_tests.rs @@ -36,14 +36,14 @@ fn build_scout_prompt_includes_request_focus_and_catalog() { let prompt = AgentPrepareContextTool::build_scout_prompt( "summarise my unread gmail", Some("last 24h"), - "- delegate_to_integrations_agent: route to a connected integration\n", + "- research: web and docs crawler\n", ); assert!(prompt.contains("[Request]")); assert!(prompt.contains("summarise my unread gmail")); assert!(prompt.contains("[Focus]")); assert!(prompt.contains("last 24h")); assert!(prompt.contains("[Orchestrator tools]")); - assert!(prompt.contains("delegate_to_integrations_agent")); + assert!(prompt.contains("research")); assert!(prompt.contains("[context_bundle]")); } @@ -398,16 +398,13 @@ async fn catalog_lists_the_parents_synthesised_delegates_from_its_visible_specs( ], vec![ catalog_spec("echo", "durable"), - catalog_spec( - "delegate_to_integrations_agent", - "route to a connected integration", - ), + catalog_spec("research", "web and docs crawler"), catalog_spec("agent_prepare_context", "this tool"), ], ); let catalog = AgentPrepareContextTool::render_parent_tool_catalog(Some(&ctx)); assert!( - catalog.contains("- delegate_to_integrations_agent: route to a connected integration\n"), + catalog.contains("- research: web and docs crawler\n"), "the parent's delegate must be recommendable: {catalog:?}" ); assert!(catalog.contains("- echo: durable\n")); diff --git a/crates/openhuman-core/src/agent/orchestration/tools/collapsed_delegation.rs b/crates/openhuman-core/src/agent/orchestration/tools/collapsed_delegation.rs index 766ac4b3d6..55a2cce0c8 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools/collapsed_delegation.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools/collapsed_delegation.rs @@ -15,12 +15,10 @@ //! enum, with each target's `when_to_use` kept verbatim in the description — //! the routing information survives in full, the repetition does not. //! -//! This is the same collapse [`SkillDelegationTool`] already applied to the -//! *other* delegation axis (#1335): one `delegate_to_integrations_agent` with -//! a `toolkit` argument, instead of one `delegate_` per connected -//! Composio integration. That change made the schema constant in the -//! integration dimension; this one makes it constant in the sub-agent -//! dimension. The two are now consistent. +//! The integration axis went a step further: connected Composio actions are +//! `Deferred` tools reached through `tool_search` and called directly, so +//! that axis has no delegation tool at all (`orchestrator_tools`). This one +//! makes the sub-agent axis constant in the sub-agent dimension. //! //! # Why collapse rather than pack //! @@ -52,12 +50,9 @@ //! builder's collision guard resolves a clash by dropping the *synthesised* //! tool. Naming this one `delegate` would therefore have removed the //! orchestrator's entire delegation surface for exactly those users, silently. -//! It also puts this tool in the same family as its sibling -//! `delegate_to_integrations_agent`. //! //! [`DelegateTool`]: crate::agent::tools::DelegateTool //! [`ArchetypeDelegationTool`]: super::ArchetypeDelegationTool -//! [`SkillDelegationTool`]: super::SkillDelegationTool //! [`ToolExposure::Hidden`]: tinytools::ToolExposure::Hidden use async_trait::async_trait; @@ -102,11 +97,7 @@ impl CollapsedDelegationTool { /// Build the collapsed tool, or `None` when there is nothing to route to. /// /// `None` rather than an empty enum: a `delegate` tool whose `agent` has no - /// valid value is a schema the model can only call wrongly, and the - /// sibling [`SkillDelegationTool::for_connected`] already returns `None` on - /// an empty toolkit list for the same reason. - /// - /// [`SkillDelegationTool::for_connected`]: super::SkillDelegationTool::for_connected + /// valid value is a schema the model can only call wrongly. pub fn for_targets(targets: Vec) -> Option { if targets.is_empty() { return None; diff --git a/crates/openhuman-core/src/agent/orchestration/tools/collapsed_delegation_tests.rs b/crates/openhuman-core/src/agent/orchestration/tools/collapsed_delegation_tests.rs index 0de914fdcb..96a2281805 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools/collapsed_delegation_tests.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools/collapsed_delegation_tests.rs @@ -31,7 +31,7 @@ fn tool() -> CollapsedDelegationTool { #[test] fn an_empty_target_list_produces_no_tool() { // An `agent` enum with no valid value is a schema the model can only call - // wrongly. Mirrors `SkillDelegationTool::for_connected`. + // wrongly. assert!(CollapsedDelegationTool::for_targets(Vec::new()).is_none()); } diff --git a/crates/openhuman-core/src/agent/orchestration/tools/dispatch.rs b/crates/openhuman-core/src/agent/orchestration/tools/dispatch.rs index 354d72d84d..0757411768 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools/dispatch.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools/dispatch.rs @@ -30,9 +30,6 @@ enum DelegationDispatchKind { Collapsed { targets: Result, String>, }, - Integrations { - connected_toolkits: Vec, - }, Archetype, } @@ -46,17 +43,6 @@ impl DelegationDispatch { ), } } - super::skill_delegation::INTEGRATIONS_DELEGATE_TOOL_NAME => { - let connected_toolkits = tool - .parameters_schema() - .pointer("/properties/toolkit/enum") - .and_then(serde_json::Value::as_array) - .into_iter() - .flatten() - .filter_map(|value| value.as_str().map(str::to_owned)) - .collect(); - DelegationDispatchKind::Integrations { connected_toolkits } - } // Only synthesized archetype delegate names enter this path. // `delegate_graph` is a concrete durable graph tool with its own // typed dispatcher, and arbitrary `delegate_*` tools must not be @@ -114,22 +100,6 @@ impl ToolDispatch<(), crate::agent::tinyagents::host::OpenHumanRunContext> for D ) .await } - DelegationDispatchKind::Integrations { connected_toolkits } => { - let connected_toolkits: Vec<(String, String)> = connected_toolkits - .iter() - .cloned() - .map(|slug| (slug, String::new())) - .collect(); - super::skill_delegation::execute_skill_delegation_with_live_parent( - self.tool.name(), - &connected_toolkits, - arguments, - Some(&tool_context), - child, - Some(parent), - ) - .await - } DelegationDispatchKind::Archetype => { let Some(agent_id) = AgentDefinitionRegistry::global().and_then(|registry| { registry.list().into_iter().find_map(|definition| { @@ -418,16 +388,11 @@ pub(crate) async fn dispatch_subagent_with_live_parent( prompt.chars().count() ); - // Propagate the per-call toolkit scope into the subagent runner so - // that the collapsed `SkillDelegationTool` can narrow - // `integrations_agent` to a single Composio toolkit (e.g. - // `delegate_to_integrations_agent { toolkit: "gmail" }` → - // integrations_agent + toolkit="gmail"). Earlier code plumbed this through - // `skill_filter_override` (which matches `{skill}__` QuickJS-style - // names), but Composio actions are named `GMAIL_*` / `NOTION_*` — - // so the filter excluded every Composio tool instead of narrowing - // them. `toolkit_override` applies the correct `{TOOLKIT}_` prefix - // check, restricted to skill-category tools. + // Propagate a per-call toolkit scope into the subagent runner as + // `toolkit_override` (the `{TOOLKIT}_` prefix check on skill-category + // tools), never as `skill_filter_override` (which matches `{skill}__` + // QuickJS-style names and would exclude every Composio action). The + // delegation tools synthesised today all pass `None` here. let worktree_action_dir = parent_workspace_descriptor .as_ref() .map(|descriptor| descriptor.root.clone()); diff --git a/crates/openhuman-core/src/agent/orchestration/tools/dispatch_tests.rs b/crates/openhuman-core/src/agent/orchestration/tools/dispatch_tests.rs index 6bb7948b51..48714b1cb3 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools/dispatch_tests.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools/dispatch_tests.rs @@ -68,19 +68,24 @@ fn typed_dispatch_registration_recognises_every_synthesised_delegate_surface() { }]) .expect("one collapsed target is routable"), ); - let integration = Arc::new(DelegationRegistrationTool { + assert!( + DelegationDispatch::for_tool(collapsed.clone()).is_some(), + "every synthesised delegation name must select the typed dispatch: {}", + collapsed.name(), + ); + // The retired integrations delegate is not a delegation surface any more: + // its name must not select a dispatcher, so a stale tool by that name + // cannot spawn a sub-agent for one integration action. + let retired = Arc::new(DelegationRegistrationTool { name: "delegate_to_integrations_agent", parameters: serde_json::json!({ "properties": { "toolkit": { "enum": ["gmail"] } } }), }); - for tool in [collapsed, integration] { - assert!( - DelegationDispatch::for_tool(tool.clone()).is_some(), - "every synthesised delegation name must select the typed dispatch: {}", - tool.name(), - ); - } + assert!( + DelegationDispatch::for_tool(retired).is_none(), + "delegate_to_integrations_agent must no longer select the typed dispatch" + ); } #[test] @@ -453,7 +458,7 @@ fn an_async_delegation_keeps_its_output_untouched() { #[test] fn the_incomplete_envelope_frames_a_stub_without_claiming_success() { let envelope = super::incomplete_envelope( - "delegate_to_integrations_agent", + "research", "returned an unexecuted tool call instead of a result", "GMAIL_LIST_MESSAGES", super::DispatchMode::Blocking, @@ -478,7 +483,7 @@ fn an_unfinished_envelope_never_claims_completeness() { assert!(done.contains("complete as returned")); let unfinished = super::incomplete_envelope( - "delegate_to_integrations_agent", + "research", "hit its iteration cap", "partial", super::DispatchMode::Blocking, diff --git a/crates/openhuman-core/src/agent/orchestration/tools/skill_delegation.rs b/crates/openhuman-core/src/agent/orchestration/tools/skill_delegation.rs deleted file mode 100644 index 525639ec1d..0000000000 --- a/crates/openhuman-core/src/agent/orchestration/tools/skill_delegation.rs +++ /dev/null @@ -1,383 +0,0 @@ -//! Single collapsed delegation tool for Composio-backed integrations -//! (#1335). -//! -//! Replaces the previous per-toolkit fan-out where the orchestrator's -//! function-calling schema gained a new `delegate_` entry for -//! every connected integration. Every one of those tools dispatched to -//! the same `integrations_agent` with a different `skill_filter`, so -//! exposing them separately bloated the orchestrator's tool list -//! linearly with no behavioural benefit. -//! -//! The collapsed tool keeps the routing handle the orchestrator needs -//! ("send this to integrations, scoped to toolkit X") while making the -//! orchestrator's schema cost constant in the integration dimension. -//! -//! The list of connected toolkits is rendered inline in the tool -//! description so the orchestrator still discovers which integrations -//! are available without each one being its own schema entry. - -use async_trait::async_trait; -use serde_json::json; - -use crate::tools::orchestrator_tools::sanitise_slug; -use tinytools::ToolRunContext; -use tinytools::{PermissionLevel, Tool, ToolCallOptions, ToolCategory, ToolResult}; - -/// Canonical tool name surfaced to the orchestrator LLM. -pub const INTEGRATIONS_DELEGATE_TOOL_NAME: &str = "delegate_to_integrations_agent"; - -/// Single collapsed delegation tool for all connected Composio toolkits. -/// -/// Carries the slugs + one-line descriptions of every connected toolkit -/// so the tool's `description()` (which is what the orchestrator's LLM -/// sees) enumerates the routing choices without needing N tools to -/// represent them. -pub struct SkillDelegationTool { - pub tool_name: String, - /// `(slug, description)` for every currently-connected toolkit. - /// `slug` is already `sanitise_slug`'d so it can be matched against - /// the LLM-provided `toolkit` argument with a plain `==`. - pub connected_toolkits: Vec<(String, String)>, - pub tool_description: String, -} - -impl SkillDelegationTool { - /// Build the canonical collapsed tool from the connected-toolkit - /// list. Returns `None` when there are zero connected toolkits — - /// callers in `collect_orchestrator_tools` interpret that as "don't - /// expose any integrations delegation surface at all", which is the - /// right thing to do because the orchestrator can't usefully route - /// to an empty set. - pub fn for_connected(connected: Vec<(String, String)>) -> Option { - if connected.is_empty() { - return None; - } - let description = build_description(&connected); - Some(Self { - tool_name: INTEGRATIONS_DELEGATE_TOOL_NAME.to_string(), - connected_toolkits: connected, - tool_description: description, - }) - } -} - -fn build_description(connected: &[(String, String)]) -> String { - // One sentence of routing plus one short line per connected service. The - // catalogue blurbs are marketing copy ("Gmail is Google's email service, - // featuring spam protection, ...") and were costing a paragraph per - // service on every turn; a clause is enough to disambiguate a slug. - const DESCRIPTION_MAX_CHARS: usize = 80; - let mut buf = String::from( - "Act on a connected service (read or write its data) through the integrations \ - agent: `toolkit` is one of the connected slugs below, `prompt` the user's task. \ - Connected:", - ); - for (slug, desc) in connected { - buf.push_str("\n - "); - buf.push_str(slug); - let trimmed = desc.trim(); - if !trimmed.is_empty() { - buf.push_str(": "); - if trimmed.chars().count() > DESCRIPTION_MAX_CHARS { - let cut: String = trimmed.chars().take(DESCRIPTION_MAX_CHARS).collect(); - let cut = cut.rsplit_once(' ').map_or(cut.as_str(), |(head, _)| head); - buf.push_str(cut.trim_end_matches([',', ';', ':'])); - buf.push_str("..."); - } else { - buf.push_str(trimmed); - } - } - } - buf -} - -// Test-only override for the live status fetch. When set, the live re-check -// returns this value instead of touching `Config::load_or_init` / -// `fetch_connected_integrations_status`, which would otherwise read the host -// machine's login/config state and could hit the Composio backend over HTTP. -// `Some(None)` forces the "Unavailable" outcome (no live data); -// `Some(Some(vec))` injects a deterministic connected set. -#[cfg(test)] -thread_local! { - static LIVE_FETCH_OVERRIDE: std::cell::RefCell>>> = - const { std::cell::RefCell::new(None) }; -} - -#[cfg(test)] -fn set_live_fetch_override(value: Option>) { - LIVE_FETCH_OVERRIDE.with(|o| *o.borrow_mut() = Some(value)); -} - -#[cfg(test)] -fn clear_live_fetch_override() { - LIVE_FETCH_OVERRIDE.with(|o| *o.borrow_mut() = None); -} - -async fn fetch_live_connected_toolkit_slugs_once() -> Option> { - #[cfg(test)] - { - if let Some(injected) = LIVE_FETCH_OVERRIDE.with(|o| o.borrow().clone()) { - return injected; - } - } - let config = crate::config::Config::load_or_init().await.ok()?; - match crate::integrations::composio::fetch_connected_integrations_status(&config).await { - crate::integrations::composio::FetchConnectedIntegrationsStatus::Authoritative(entries) => { - let mut toolkits: Vec = entries - .into_iter() - .filter(|entry| entry.connected) - .map(|entry| sanitise_slug(&entry.toolkit)) - .collect(); - toolkits.sort(); - toolkits.dedup(); - Some(toolkits) - } - crate::integrations::composio::FetchConnectedIntegrationsStatus::Unavailable => None, - } -} - -fn resolve_connected_toolkits( - snapshot: &[(String, String)], - slug: &str, - live_connected: Option<&[String]>, -) -> (bool, Vec) { - let allowed: Vec = snapshot.iter().map(|(slug, _)| slug.clone()).collect(); - if snapshot.iter().any(|(known_slug, _)| known_slug == slug) { - return (true, allowed); - } - if let Some(live) = live_connected { - if live.iter().any(|s| s == slug) { - return (true, live.to_vec()); - } - } - (false, allowed) -} - -#[async_trait] -impl Tool for SkillDelegationTool { - fn name(&self) -> &str { - &self.tool_name - } - - fn description(&self) -> &str { - &self.tool_description - } - - fn parameters_schema(&self) -> serde_json::Value { - let slugs: Vec<&str> = self - .connected_toolkits - .iter() - .map(|(slug, _)| slug.as_str()) - .collect(); - json!({ - "type": "object", - "required": ["toolkit", "prompt"], - "properties": { - "toolkit": { - "type": "string", - "enum": slugs, - "description": "Composio toolkit slug to route to (e.g. `gmail`, `notion`). \ - Must match one of the connected toolkits enumerated in this tool's description." - }, - // `prompt` and `model` are described once in the parent's - // prompt.md ("Structured handoffs") rather than here, matching - // `ArchetypeDelegationTool`. `toolkit` keeps its description: - // it is the routing signal and points at the slugs enumerated - // in this tool's own description. - "prompt": { "type": "string" }, - "model": { - "type": "string", - "description": "Pin the child to this exact model id. Omit unless you have a reason." - } - } - }) - } - - fn permission_level(&self) -> PermissionLevel { - PermissionLevel::Execute - } - - fn category(&self) -> ToolCategory { - ToolCategory::System - } - - async fn execute(&self, args: serde_json::Value) -> anyhow::Result { - self.execute_with_context(args, ToolCallOptions::default(), None) - .await - } - - async fn execute_with_context( - &self, - args: serde_json::Value, - _options: ToolCallOptions, - tool_context: Option<&dyn ToolRunContext>, - ) -> anyhow::Result { - let mut run_context = crate::agent::tinyagents::host::OpenHumanRunContext::new(); - run_context.thread_id = tool_context - .and_then(ToolRunContext::thread_id) - .map(ToOwned::to_owned); - execute_skill_delegation( - &self.tool_name, - &self.connected_toolkits, - args, - tool_context, - run_context, - ) - .await - } -} - -/// Execute an integration hand-off with an explicit child run carrier. -pub(crate) async fn execute_skill_delegation( - tool_name: &str, - connected_toolkits: &[(String, String)], - args: serde_json::Value, - tool_context: Option<&dyn ToolRunContext>, - run_context: crate::agent::tinyagents::host::OpenHumanRunContext, -) -> anyhow::Result { - if let Some(live_parent) = super::ambient_parent_run_context("direct-skill-delegation") { - let run_context = live_parent.data.child(); - return execute_skill_delegation_with_live_parent( - tool_name, - connected_toolkits, - args, - tool_context, - run_context, - Some(&live_parent), - ) - .await; - } - execute_skill_delegation_with_live_parent( - tool_name, - connected_toolkits, - args, - tool_context, - run_context, - None, - ) - .await -} - -/// Typed-harness counterpart that preserves a live parent for the blocking -/// integrations child. -pub(crate) async fn execute_skill_delegation_with_live_parent( - tool_name: &str, - connected_toolkits: &[(String, String)], - args: serde_json::Value, - tool_context: Option<&dyn ToolRunContext>, - run_context: crate::agent::tinyagents::host::OpenHumanRunContext, - live_parent: Option< - &tinyagents_harness::context::RunContext< - crate::agent::tinyagents::host::OpenHumanRunContext, - >, - >, -) -> anyhow::Result { - let raw_toolkit = args - .get("toolkit") - .and_then(|v| v.as_str()) - .unwrap_or("") - .trim() - .to_string(); - log::debug!( - "[skill-delegation] execute start tool='{}' raw_toolkit={:?} prompt_chars={}", - tool_name, - raw_toolkit, - args.get("prompt") - .and_then(|v| v.as_str()) - .map(|s| s.chars().count()) - .unwrap_or(0) - ); - if raw_toolkit.is_empty() { - log::debug!( - "[skill-delegation] reject: missing `toolkit` argument for tool='{}'", - tool_name - ); - return Ok(ToolResult::error(format!( - "{}: `toolkit` is required and must match a connected integration slug", - tool_name - ))); - } - let slug = sanitise_slug(&raw_toolkit); - let mut live_connected: Option> = None; - let mut known = connected_toolkits - .iter() - .any(|(known_slug, _)| known_slug == &slug); - if !known { - // Safety net for same-thread OAuth races: do one live status - // refresh before rejecting an unknown toolkit, mirroring the - // spawn_subagent integrations pre-flight. - live_connected = fetch_live_connected_toolkit_slugs_once().await; - } - let (known_after_recheck, allowed) = - resolve_connected_toolkits(connected_toolkits, &slug, live_connected.as_deref()); - if known_after_recheck && !known { - log::info!( - "[skill-delegation] toolkit '{}' accepted after live re-check (session schema stale)", - slug - ); - } - known = known_after_recheck; - if !known { - log::debug!( - "[skill-delegation] reject: toolkit '{}' (sanitised='{}') not in connected set {:?}", - raw_toolkit, - slug, - allowed - ); - return Ok(ToolResult::error(format!( - "{}: toolkit `{raw_toolkit}` is not connected — allowed: [{}]", - tool_name, - allowed.join(", ") - ))); - } - - let prompt = args - .get("prompt") - .and_then(|v| v.as_str()) - .unwrap_or("") - .trim() - .to_string(); - if prompt.is_empty() { - log::debug!( - "[skill-delegation] reject: empty `prompt` for tool='{}' toolkit='{}'", - tool_name, - slug - ); - return Ok(ToolResult::error(format!( - "{}: `prompt` is required", - tool_name - ))); - } - - let model_override = args - .get("model") - .and_then(|v| v.as_str()) - .map(str::trim) - .filter(|s| !s.is_empty()); - - log::debug!( - "[skill-delegation] dispatching toolkit='{}' to integrations_agent (prompt_chars={})", - slug, - prompt.chars().count() - ); - // Integration delegations stay blocking: their outcomes (send the - // email, create the page, …) are usually approval-gated mid-turn and - // the orchestrator's reply reports the concrete result. The durable - // async default applies to archetype delegations only for now. - super::dispatch::dispatch_subagent_with_live_parent( - "integrations_agent", - tool_name, - &prompt, - Some(&slug), - model_override, - tool_context, - super::dispatch::DispatchMode::Blocking, - run_context, - live_parent, - ) - .await -} - -#[cfg(test)] -#[path = "skill_delegation_tests.rs"] -mod tests; diff --git a/crates/openhuman-core/src/agent/orchestration/tools/skill_delegation_tests.rs b/crates/openhuman-core/src/agent/orchestration/tools/skill_delegation_tests.rs deleted file mode 100644 index 14fcd14746..0000000000 --- a/crates/openhuman-core/src/agent/orchestration/tools/skill_delegation_tests.rs +++ /dev/null @@ -1,174 +0,0 @@ -use super::*; - -#[test] -fn for_connected_returns_none_when_no_toolkits() { - assert!(SkillDelegationTool::for_connected(vec![]).is_none()); -} - -#[test] -fn for_connected_uses_canonical_tool_name() { - let tool = SkillDelegationTool::for_connected(vec![( - "gmail".to_string(), - "Email access.".to_string(), - )]) - .unwrap(); - assert_eq!(tool.name(), INTEGRATIONS_DELEGATE_TOOL_NAME); - assert_eq!(tool.name(), "delegate_to_integrations_agent"); -} - -#[test] -fn description_enumerates_connected_toolkits() { - let tool = SkillDelegationTool::for_connected(vec![ - ("gmail".to_string(), "Email access.".to_string()), - ("notion".to_string(), "Pages and databases.".to_string()), - ]) - .unwrap(); - let desc = tool.description(); - assert!(desc.contains("gmail")); - assert!(desc.contains("notion")); - assert!(desc.contains("Email access.")); - assert!(desc.contains("Pages and databases.")); -} - -#[test] -fn parameters_schema_enforces_toolkit_enum_against_connected_slugs() { - let tool = SkillDelegationTool::for_connected(vec![ - ("gmail".to_string(), "Email.".to_string()), - ("notion".to_string(), "Docs.".to_string()), - ]) - .unwrap(); - let schema = tool.parameters_schema(); - let enum_vals = schema["properties"]["toolkit"]["enum"] - .as_array() - .expect("toolkit enum is an array"); - let collected: Vec<&str> = enum_vals.iter().map(|v| v.as_str().unwrap()).collect(); - assert_eq!(collected, vec!["gmail", "notion"]); - - let required = schema["required"].as_array().expect("required is an array"); - let required: Vec<&str> = required.iter().map(|v| v.as_str().unwrap()).collect(); - assert!(required.contains(&"toolkit")); - assert!(required.contains(&"prompt")); -} - -#[tokio::test] -async fn execute_rejects_missing_toolkit_argument() { - let tool = - SkillDelegationTool::for_connected(vec![("gmail".to_string(), "Email.".to_string())]) - .unwrap(); - let result = tool.execute(json!({"prompt": "x"})).await.unwrap(); - assert!(result.is_error); - assert!(result.output().contains("toolkit")); -} - -#[tokio::test] -async fn execute_rejects_unknown_toolkit_with_allowed_list() { - // Force the live re-check to return "Unavailable" so the test never - // reads host config or reaches the Composio backend — the reject must - // come purely from the in-memory snapshot (gmail/notion, no slack). - set_live_fetch_override(None); - let tool = SkillDelegationTool::for_connected(vec![ - ("gmail".to_string(), "Email.".to_string()), - ("notion".to_string(), "Docs.".to_string()), - ]) - .unwrap(); - let result = tool - .execute(json!({"toolkit": "slack", "prompt": "hi"})) - .await - .unwrap(); - clear_live_fetch_override(); - assert!(result.is_error); - let body = result.output(); - assert!(body.contains("slack")); - assert!(body.contains("gmail")); - assert!(body.contains("notion")); -} - -#[tokio::test] -async fn execute_rejects_empty_prompt() { - let tool = - SkillDelegationTool::for_connected(vec![("gmail".to_string(), "Email.".to_string())]) - .unwrap(); - let result = tool - .execute(json!({"toolkit": "gmail", "prompt": " "})) - .await - .unwrap(); - assert!(result.is_error); - assert!(result.output().contains("prompt")); -} - -#[tokio::test] -async fn execute_normalises_toolkit_input_before_matching() { - // Mixed-case + odd-character user input must collapse onto the - // canonical slug before the connectedness check fires. - // Pin the live re-check to the same snapshot so the test is hermetic - // (no host config / backend read): `gmail` stays unknown, while the - // normalised `google_calendar` is accepted. - set_live_fetch_override(Some(vec!["google_calendar".to_string()])); - let tool = SkillDelegationTool::for_connected(vec![( - "google_calendar".to_string(), - "Calendar.".to_string(), - )]) - .unwrap(); - // "GMail" sanitises to `gmail` — NOT in the connected set, so it - // must be rejected with the unknown-toolkit message that - // enumerates the allowed slugs. - let bad = tool - .execute(json!({"toolkit": "GMail", "prompt": "x"})) - .await - .unwrap(); - assert!(bad.is_error); - let bad_body = bad.output(); - assert!( - bad_body.contains("not connected"), - "expected unknown-toolkit error path, got: {bad_body}" - ); - assert!(bad_body.contains("google_calendar")); - - // "Google-Calendar" sanitises to `google_calendar`, which IS in - // the connected set, so the toolkit gate must let it through. - // Dispatch will then fail because no agent registry is wired up - // in this unit-test process — but the error must NOT be the - // unknown-toolkit branch, because that branch was supposed to - // be bypassed by the slug normalisation. - let ok = tool - .execute(json!({"toolkit": "Google-Calendar", "prompt": "do thing"})) - .await; - match ok { - Ok(result) => { - let body = result.output(); - assert!( - !body.contains("not connected"), - "normalised slug should pass the toolkit gate, got: {body}" - ); - } - Err(err) => { - let msg = err.to_string(); - assert!( - !msg.contains("not connected"), - "normalised slug should pass the toolkit gate, got: {msg}" - ); - } - } - clear_live_fetch_override(); -} - -#[test] -fn resolve_connected_toolkits_prefers_live_recheck_for_unknown_slug() { - let snapshot = vec![("gmail".to_string(), "Email".to_string())]; - - let (known_snapshot, allowed_snapshot) = resolve_connected_toolkits(&snapshot, "gmail", None); - assert!(known_snapshot); - assert_eq!(allowed_snapshot, vec!["gmail".to_string()]); - - let live = vec!["gmail".to_string(), "notion".to_string()]; - let (known_live, allowed_live) = - resolve_connected_toolkits(&snapshot, "notion", Some(live.as_slice())); - assert!(known_live); - assert_eq!(allowed_live, live); - - let live_no_match = vec!["gmail".to_string(), "notion".to_string()]; - let (known_none, allowed_none) = - resolve_connected_toolkits(&snapshot, "slack", Some(live_no_match.as_slice())); - assert!(!known_none); - assert_eq!(allowed_none, vec!["gmail".to_string()]); -} diff --git a/crates/openhuman-core/src/agent/orchestration/tools/tools_e2e_tests.rs b/crates/openhuman-core/src/agent/orchestration/tools/tools_e2e_tests.rs index 057bb51604..6cca4e02b6 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools/tools_e2e_tests.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools/tools_e2e_tests.rs @@ -1,7 +1,4 @@ -use super::{ - ArchetypeDelegationTool, DelegationTarget, SkillDelegationTool, SpawnSubagentTool, - SpawnWorkerThreadTool, -}; +use super::{ArchetypeDelegationTool, DelegationTarget, SpawnSubagentTool, SpawnWorkerThreadTool}; use crate::agent::harness::definition::AgentDefinitionRegistry; use crate::agent::harness::{with_parent_context, ParentExecutionContext}; use crate::agent::messages::ChatMessage; @@ -20,7 +17,6 @@ use tinytools::Tool; const SPAWN_SUBAGENT_CANARY: &str = "tool-e2e-spawn-subagent-canary"; const ARCHETYPE_DELEGATION_CANARY: &str = "tool-e2e-archetype-delegation-canary"; -const SKILL_DELEGATION_CANARY: &str = "tool-e2e-skill-delegation-canary"; const WORKER_THREAD_CANARY: &str = "tool-e2e-worker-thread-canary"; #[tokio::test] @@ -348,63 +344,6 @@ async fn continue_subagent_without_checkpoint_or_durable_session_names_the_roste ); } -#[tokio::test] -async fn skill_delegation_tool_runs_integrations_agent_e2e() { - let _ = AgentDefinitionRegistry::init_global_builtins(); - let workspace = tempfile::TempDir::new().expect("workspace"); - let provider = Arc::new(ScriptedModel::new(vec![( - SKILL_DELEGATION_CANARY, - "skill-delegation-child-answer", - )])); - let tool = SkillDelegationTool::for_connected(vec![( - "gmail".to_string(), - "Email access.".to_string(), - )]) - .expect("delegation tool"); - - let result = with_parent_context( - parent_context( - workspace.path(), - provider.clone(), - vec![ConnectedIntegration { - toolkit: "gmail".to_string(), - description: "Email access.".to_string(), - tools: Vec::new(), - gated_tools: Vec::new(), - connected: true, - connections: Vec::new(), - non_active_status: None, - }], - ), - async { - tool.execute(json!({ - "toolkit": "gmail", - "prompt": format!("Summarize inbox state for {SKILL_DELEGATION_CANARY}"), - "model": "test-model" - })) - .await - }, - ) - .await - .expect("tool execution"); - - assert!(!result.is_error, "{}", result.output()); - // The sub-agent's answer comes back verbatim, followed by the - // inline-result note: this delegation is blocking and registers no - // worker, so the orchestrator must not go hunting for one (#6033). - let output = result.output(); - assert!( - output.starts_with("skill-delegation-child-answer"), - "the child's answer must lead the result: {output}" - ); - assert!( - output.contains("[INLINE_RESULT]") && output.contains("no sub-agent worker"), - "a blocking delegation must say its result is inline: {output}" - ); - assert!(provider.saw(SKILL_DELEGATION_CANARY)); - assert!(provider.saw("gmail")); -} - #[tokio::test] async fn spawn_worker_thread_tool_persists_worker_thread_e2e() { let _ = AgentDefinitionRegistry::init_global_builtins(); diff --git a/crates/openhuman-core/src/agent/prompts/mod_tests.rs b/crates/openhuman-core/src/agent/prompts/mod_tests.rs index 1e40785c09..939aa759dc 100644 --- a/crates/openhuman-core/src/agent/prompts/mod_tests.rs +++ b/crates/openhuman-core/src/agent/prompts/mod_tests.rs @@ -212,3 +212,63 @@ fn tool_call_format_maps_to_the_dialect_and_harness_vocabulary() { assert_eq!(host.code_style(), style); } } + +/// The prompt catalogue is the callable surface on a text dialect, so a +/// deferred tool must leave it and the discovery bridge must take its place. +/// Rendering the deferred set instead (what the policy allow-set does, since +/// it admits those names to keep a found tool callable) both spends the bytes +/// deferral exists to save and tells the model to search for a signature it +/// can already read. +#[test] +fn swapping_deferred_entries_leaves_the_bridge_in_their_place() { + let mut tools = vec![ + PromptTool::new("shell", "Run a command."), + PromptTool::new("GMAIL_SEND_EMAIL", "Send an email."), + PromptTool::new("GMAIL_FETCH_EMAILS", "Read email."), + ]; + let mut visible: HashSet = ["shell", "GMAIL_SEND_EMAIL", "GMAIL_FETCH_EMAILS"] + .into_iter() + .map(str::to_string) + .collect(); + let deferred: HashSet = ["GMAIL_SEND_EMAIL", "GMAIL_FETCH_EMAILS"] + .into_iter() + .map(str::to_string) + .collect(); + + swap_deferred_for_discovery_bridge(&mut tools, &mut visible, &deferred); + + assert!(visible.contains("shell"), "a direct tool stays advertised"); + assert!( + !visible.contains("GMAIL_SEND_EMAIL") && !visible.contains("GMAIL_FETCH_EMAILS"), + "deferred tools must not be rendered into the catalogue: {visible:?}" + ); + assert!( + visible.contains("tool_search") && visible.contains("tool_call"), + "the bridge replaces them so the model can reach what it finds: {visible:?}" + ); + for name in ["tool_search", "tool_call"] { + assert!( + tools.iter().any(|tool| tool.name == name + && tool + .parameters_schema + .as_deref() + .is_some_and(|schema| schema.contains("\"type\":\"object\""))), + "{name} must reach the catalogue with a callable schema" + ); + } +} + +/// Nothing deferred: the catalogue and the advertised set are untouched, and +/// a belt that never opted into discovery does not pay for two bridge +/// schemas it cannot use. +#[test] +fn swapping_is_a_no_op_without_a_deferred_set() { + let mut tools = vec![PromptTool::new("shell", "Run a command.")]; + let mut visible: HashSet = ["shell"].into_iter().map(str::to_string).collect(); + + swap_deferred_for_discovery_bridge(&mut tools, &mut visible, &HashSet::new()); + + assert_eq!(tools.len(), 1); + assert_eq!(visible.len(), 1); + assert!(visible.contains("shell")); +} diff --git a/crates/openhuman-core/src/agent/prompts/sections.rs b/crates/openhuman-core/src/agent/prompts/sections.rs index 6039767474..12904c2c61 100644 --- a/crates/openhuman-core/src/agent/prompts/sections.rs +++ b/crates/openhuman-core/src/agent/prompts/sections.rs @@ -381,7 +381,7 @@ impl PromptSection for ToolsSection { let visible: Vec = ctx .tools .iter() - .filter(|tool| !has_filter || ctx.visible_tool_names.contains(tool.name)) + .filter(|tool| !has_filter || ctx.visible_tool_names.contains(tool.name.as_ref())) .map(|tool| { let parameters = match tool.parameters_schema.as_deref() { Some(schema) => match serde_json::from_str(schema) { diff --git a/crates/openhuman-core/src/agent/prompts/types.rs b/crates/openhuman-core/src/agent/prompts/types.rs index a30f01e42b..3d77285c03 100644 --- a/crates/openhuman-core/src/agent/prompts/types.rs +++ b/crates/openhuman-core/src/agent/prompts/types.rs @@ -233,24 +233,39 @@ pub struct ConnectedIntegrationTool { /// description)` tuples) all adapt to this. #[derive(Debug, Clone)] pub struct PromptTool<'a> { - pub name: &'a str, - pub description: &'a str, + pub name: std::borrow::Cow<'a, str>, + pub description: std::borrow::Cow<'a, str>, pub parameters_schema: Option, } impl<'a> PromptTool<'a> { pub fn new(name: &'a str, description: &'a str) -> Self { Self { - name, - description, + name: std::borrow::Cow::Borrowed(name), + description: std::borrow::Cow::Borrowed(description), parameters_schema: None, } } + /// An entry the catalogue owns rather than borrows: a tool that exists + /// only for this prompt build (the harness's `tool_search` / `tool_call` + /// bridge), with no registration to borrow a name from. + pub fn owned( + name: String, + description: String, + parameters_schema: String, + ) -> PromptTool<'static> { + PromptTool { + name: std::borrow::Cow::Owned(name), + description: std::borrow::Cow::Owned(description), + parameters_schema: Some(parameters_schema), + } + } + pub fn with_schema(name: &'a str, description: &'a str, parameters_schema: String) -> Self { Self { - name, - description, + name: std::borrow::Cow::Borrowed(name), + description: std::borrow::Cow::Borrowed(description), parameters_schema: Some(parameters_schema), } } @@ -272,14 +287,55 @@ impl<'a> PromptTool<'a> { tools .into_iter() .map(|t| PromptTool { - name: t.name(), - description: t.description(), + name: std::borrow::Cow::Borrowed(t.name()), + description: std::borrow::Cow::Borrowed(t.description()), parameters_schema: Some(t.parameters_schema().to_string()), }) .collect() } } +/// Swap a prompt catalogue's `Deferred` entries for the discovery bridge. +/// +/// On a TEXT dialect (P-Format / code) the catalogue this prompt renders IS +/// the model's callable surface: the harness folds it into the system prompt +/// and clears `request.tools`. Two things follow, and both were wrong before +/// this helper existed: +/// +/// * **Deferred tools must leave the catalogue.** The filter each prompt site +/// used is the policy's allow-set, which deliberately admits deferred names +/// so a found tool stays *callable* (`reachable_names` in the session +/// builder). Filtering the prompt by it rendered every deferred schema into +/// the prompt — measured live at 107 connected Composio actions for 55 KB of +/// a 71 KB prompt, the exact cost deferral exists to avoid — and told the +/// model to search for an action whose signature it could already read. +/// +/// * **The bridge must take their place.** The harness mints `tool_search` / +/// `tool_call` onto `request.tools`, which a text dialect drops, and with +/// `host_renders_tool_catalogue` it appends nothing itself. Without these +/// entries the model reads "invoke a match with `tool_call`" in a search +/// result and has no signature for that name; observed live as a turn that +/// narrates the call it is about to make and then stops. +/// +/// A native-tool-calling provider is unaffected: it reads `request.tools`, +/// where the harness already puts exactly this pair. +pub fn swap_deferred_for_discovery_bridge<'a>( + prompt_tools: &mut Vec>, + visible_tool_names: &mut std::collections::HashSet, + deferred_tool_names: &std::collections::HashSet, +) { + if deferred_tool_names.is_empty() { + return; + } + visible_tool_names.retain(|name| !deferred_tool_names.contains(name)); + for bridge in + crate::agent::tinyagents::discovery::bridge_prompt_tools(deferred_tool_names.len()) + { + visible_tool_names.insert(bridge.name.to_string()); + prompt_tools.push(bridge); + } +} + /// How TinyTools should render and parse an agent's tool calls. /// /// The prompt layer carries this only to select the TinyTools dialect; it does diff --git a/crates/openhuman-core/src/agent/registry/README.md b/crates/openhuman-core/src/agent/registry/README.md index abf028cd35..9825ca74cb 100644 --- a/crates/openhuman-core/src/agent/registry/README.md +++ b/crates/openhuman-core/src/agent/registry/README.md @@ -97,7 +97,7 @@ The 29 archetypes in this directory: | `goals_agent` | Background: keeps `MEMORY_GOALS.md` fresh from session context | | `help` | Answers "how does OpenHuman work" questions from the bundled GitBook docs | | `image_agent` | Image generation/edit specialist | -| `integrations_agent` | Drives a single Composio toolkit (gmail, notion, github, …) per spawn | +| `integrations_agent` | Drives a single Composio toolkit (gmail, notion, github, …) per spawn; no chat agent delegates to it — the orchestrator searches for and calls connected actions itself | | `mcp_agent` (feature `mcp`) | Calls tools on an already-connected MCP server | | `morning_briefing` | Proactive scheduled daily summary (tasks, calendar, email, skills) | | `orchestrator` | Default user-facing `chat`-tier agent; direct-first, delegates only when it materially helps | diff --git a/crates/openhuman-core/src/agent/registry/agents/loader.rs b/crates/openhuman-core/src/agent/registry/agents/loader.rs index 368cc03a58..5160a04393 100644 --- a/crates/openhuman-core/src/agent/registry/agents/loader.rs +++ b/crates/openhuman-core/src/agent/registry/agents/loader.rs @@ -349,15 +349,12 @@ fn builtin_enabled(_b: &BuiltinAgent) -> bool { /// * `Reasoning` agents MUST NOT list another `Reasoning` agent in /// `subagents`. /// * `Worker` agents MUST NOT list any [`SubagentEntry::AgentId`] -/// entries. (Workflow wildcards are allowed: they expand to the generic -/// `integrations_agent`, which is itself a `Worker`, and the call -/// happens via a single delegation tool rather than recursive spawn.) +/// entries. (Skills wildcards are allowed: they expand to the connected +/// integrations' actions as searchable tools on the agent's own belt, +/// not to a spawn.) /// -/// Workflow-wildcard entries (`{ skills = "*" }`) are intentionally -/// untouched: they collapse to one `delegate_to_integrations_agent` -/// tool whose target is a `Worker` and whose use sites are well -/// understood. Mis-tiering of the `integrations_agent` itself is still -/// caught because it appears as a normal entry elsewhere. +/// Skills-wildcard entries (`{ skills = "*" }`) are intentionally +/// untouched: they name no agent, so there is no tier pair to check. /// /// Called from [`load_builtins`] for the bundled archetype set and from /// [`crate::agent::harness::definition::AgentDefinitionRegistry::load`] @@ -372,9 +369,8 @@ pub fn validate_tier_hierarchy(defs: &[AgentDefinition]) -> Result<()> { for entry in &def.subagents { let child_id = match entry { SubagentEntry::AgentId(id) => id.as_str(), - // Workflow wildcards always route to `integrations_agent` - // (a Worker) via a single collapsed delegation tool — - // not subject to the tier-mismatch rule. + // Skills wildcards expand to searchable integration + // actions, not to an agent — nothing to tier-check. SubagentEntry::Skills(_) => continue, }; diff --git a/crates/openhuman-core/src/agent/registry/agents/loader_tests_orchestrator_tier_tests.rs b/crates/openhuman-core/src/agent/registry/agents/loader_tests_orchestrator_tier_tests.rs index 97031c47d9..fe40dd7e39 100644 --- a/crates/openhuman-core/src/agent/registry/agents/loader_tests_orchestrator_tier_tests.rs +++ b/crates/openhuman-core/src/agent/registry/agents/loader_tests_orchestrator_tier_tests.rs @@ -112,7 +112,6 @@ fn crypto_agent_has_narrow_wallet_market_tools_and_safety_on() { "composio_list_tools", "spawn_subagent", "spawn_worker_thread", - "delegate_to_integrations_agent", // Synthesised delegation tools use the unprefixed // `delegate_name` overrides — forbid those names too. "run_code", @@ -504,9 +503,8 @@ fn rejects_worker_with_subagents() { #[test] fn allows_skill_wildcards_on_any_non_worker_tier() { - // Skills wildcards collapse to delegate_to_integrations_agent - // and must not be policed by the tier check (it'd be a false - // positive — they fan out to a worker anyway). + // Skills wildcards expand to searchable integration actions, not to + // an agent, so there is no tier pair for the check to police. let mut defs = load_builtins().unwrap(); let planner = defs.iter_mut().find(|d| d.id == "planner").unwrap(); planner.subagents.push(SubagentEntry::Skills( diff --git a/crates/openhuman-core/src/agent/registry/agents/orchestrator/agent.toml b/crates/openhuman-core/src/agent/registry/agents/orchestrator/agent.toml index ef737824ae..dc91fc297a 100644 --- a/crates/openhuman-core/src/agent/registry/agents/orchestrator/agent.toml +++ b/crates/openhuman-core/src/agent/registry/agents/orchestrator/agent.toml @@ -49,9 +49,11 @@ omit_memory_md = false # as the tool name, and the target's `when_to_use` as the # LLM-visible tool description. # -# * `{ skills = "*" }` → one `SkillDelegationTool` per connected -# Composio toolkit, all routing to the generic `integrations_agent` with -# the toolkit slug pre-filled as `skill_filter`. +# * `{ skills = "*" }` → one `Deferred` action tool per action of every +# connected Composio toolkit. Off the wire, found through `tool_search`, +# called directly. This used to be a `delegate_to_integrations_agent` +# spawn; one clear service action is now a search and a call, and the +# wildcard admits no sub-agent id (`AgentDefinition::allowed_subagent_ids`). # # The orchestrator LLM sees these as first-class entries in its # function-calling schema, so routing decisions happen at the tool- @@ -206,14 +208,14 @@ hint = "coding" # delegating. # # `composio_list_connections` lets the agent detect newly-authorised -# integrations mid-session (the session-start fetch froze the Delegation -# Guide's connected list). `composio_connect` lets it raise an inline +# integrations mid-session (the session-start fetch froze the Connected +# Integrations block). `composio_connect` lets it raise an inline # connect card for ANY toolkit the user asks to connect — including ones # not yet connected (#3993); the tool validates the slug against the # backend allowlist and the OAuth handoff itself runs inside the card. -# Toolkit *action* listing and execution still live downstream in -# `integrations_agent` — the orchestrator never calls composio_list_tools -# / composio_execute directly. +# Toolkit *actions* are the per-action `Deferred` tools synthesised from +# `{ skills = "*" }` below and reached through `tool_search` — the +# orchestrator never calls composio_list_tools / composio_execute directly. named = [ # Tool packs add this proxy to the visible set when they withhold member # schemas. The hosted runtime also enforces this definition as its dispatch diff --git a/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt.md b/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt.md index b3d4a41fe2..47c556066a 100644 --- a/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt.md +++ b/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt.md @@ -3,8 +3,8 @@ Take the first branch that applies: 1. **Answerable without tools**: reply. Small talk, simple Q&A, general knowledge. -1b. **Needs a capability you do not see listed**: call `tool_search` with the intent in plain words before delegating or declining. Your list is a core set; one clear action on a connected service (send this message, create that issue) is a search-then-call, not a delegation. -2. **Needs a connected service's own data or actions** (inbox, messages, calendar, docs, tickets, "send/check X"): call `delegate_to_integrations_agent` with the `toolkit` from **Connected Integrations**. Use the live service even when memory could plausibly answer. A service being connected is not a reason to touch it: general knowledge, web/news lookups, headlines, date/time, math, and anything public on the web (a public repository, a product page, docs) never delegate here; those are `web_fetch` / `web_search_tool` / `research` work. Delegate to a toolkit only for the user's own account data or actions on it. Not connected? Raise a connect card with `composio_connect`: the list shows what is connected, not what is connectable, so never refuse from it or send the user to settings, and never paste OAuth URLs. If the connect call reports the toolkit unavailable, relay its message; that is the only honest refusal. +1b. **Needs a capability you do not see listed**: `tool_search` with the intent in plain words before delegating or declining; if nothing comes back, say so. +2. **Needs a connected service's own data or actions** (inbox, messages, calendar, docs, tickets, "send/check X"): `tool_search` for the action ("send an email", "list calendar events"), then call what it returns. No sub-agent runs it for you, and an announced search never runs: emit it. Use the live service even when memory could plausibly answer. A service being connected is not a reason to touch it: general knowledge, web/news lookups, headlines, date/time, math, and anything public on the web (a public repository, a product page, docs) never go to a service; those are `web_fetch` / `web_search_tool` / `research` work. Reach for a toolkit only for the user's own account data or actions on it. Not connected? Raise a connect card with `composio_connect`: the list shows what is connected, not what is connectable, so never refuse from it or send the user to settings, and never paste OAuth URLs. If the connect call reports the toolkit unavailable, relay its message; that is the only honest refusal. 3. **Solvable with a direct tool**: do it yourself. `web_search_tool` and `web_fetch` for a fact or a page, `memory_recall` and `memory_store` for the user's own facts, `shell` plus `apply_patch` for repository work. Keep code work end-to-end: edit and verify in the same turn; never delegate merely because a task touches a repository. 4. **Needs a specialist**: the specialists you can call are in your tool list with their own descriptions. **Capabilities not in your tool list** names the ones a skill holds; reach those through `use_skill`. Workers return only their result; carry out any `## Handoff Plan` they return yourself, under the approval gate. 5. **Distill every delegated reply**: keep what answers the question, drop the worker's notes. Never paste a sub-agent's response verbatim. @@ -31,7 +31,7 @@ Three or more steps? Track them on `todo` cards. Don't stop with a plan: execute - Preserve numeric evidence exactly: copy numbers, dates, durations, currencies and ids as observed; don't round or recompute unless asked, and then show the working. - A sub-agent's summary is claims: check it against its `Evidence used`, `Actions taken` and `Failed tool calls`. Do not introduce facts its evidence does not support. Output marked truncated, oversized, partial or unavailable is not complete: fetch more or say so. - Never pass off fabricated output as a result. If a step failed, say so and what you did instead. -- `retrieve_memory` walks already-ingested history, not a live API; for what is in an inbox right now, delegate to the live integration. +- `retrieve_memory` walks already-ingested history, not a live API; for what is in an inbox right now, search for and call the live integration's action. ## Scheduling and workflows diff --git a/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt.rs b/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt.rs index 76163d6ad0..f8d1b4605e 100644 --- a/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt.rs +++ b/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt.rs @@ -2,20 +2,19 @@ //! //! The orchestrator follows a direct-first policy: respond directly or use //! cheap direct tools whenever possible, and delegate only for specialised -//! execution. It never executes Composio actions itself; the integration -//! block points to the single collapsed `delegate_to_integrations_agent` -//! tool (synthesised by `orchestrator_tools::collect_orchestrator_tools`, -//! #1335) for true external-service operations, with the toolkit slug -//! passed as an argument. That prose lives here (not in the shared -//! prompts module) so the skill-executor voice stays in -//! `integrations_agent/prompt.rs` and nobody has to branch on `agent_id` -//! in a shared section impl. +//! execution. Connected Composio integrations are part of that direct +//! surface: every connected toolkit's actions are registered as `Deferred` +//! tools (`orchestrator_tools::collect_deferred_integration_actions`), so +//! the `## Connected Integrations` block tells the model to `tool_search` +//! for the action and call it — there is no integrations sub-agent to +//! delegate to any more. That prose lives here (not in the shared prompts +//! module) so nobody has to branch on `agent_id` in a shared section impl. use crate::agent::harness::definition::SubagentEntry; use crate::agent::harness::AgentDefinitionRegistry; use crate::agent::prompts::{ render_datetime, render_identity, render_tools, render_user_files, render_workspace, - ConnectedIntegration, PromptContext, ToolCallFormat, + ConnectedIntegration, PromptContext, }; use crate::skills::ops_types::Workflow; use crate::tools::orchestrator_tools::sanitise_slug; @@ -92,7 +91,7 @@ pub fn build(ctx: &PromptContext<'_>) -> Result { push(&mut out, &render_withheld_specialists(ctx)); push( &mut out, - &render_delegation_guide(ctx.connected_integrations, ctx.tool_call_format), + &render_connected_integrations(ctx.connected_integrations), ); push( &mut out, @@ -146,8 +145,8 @@ fn render_withheld_specialists(ctx: &PromptContext<'_>) -> String { let mut rows: Vec<(String, &'static str)> = Vec::new(); for entry in &definition.subagents { - // `Skills(_)` expands to `delegate_to_integrations_agent`, which the - // `## Connected Integrations` block below documents in full. + // `Skills(_)` expands to searchable integration actions, not a + // delegate tool; the `## Connected Integrations` block covers them. let SubagentEntry::AgentId(agent_id) = entry else { continue; }; @@ -359,7 +358,7 @@ fn render_installed_skills( } /// Render the `## Connected MCP Servers` block from the live connection -/// registry. The MCP analogue of [`render_delegation_guide`]: it lists each +/// registry. The MCP analogue of [`render_connected_integrations`]: it lists each /// connected MCP server and tells the orchestrator to hand matching requests to /// the `mcp_agent` worker — NOT to call a server's tools itself or claim it /// can't. This is what lets the orchestrator pick up a connected server @@ -466,57 +465,49 @@ fn format_connected_mcp_block( out } -/// Render the delegator-voice `## Connected Integrations` block. Only -/// toolkits the user has actively connected are listed — unauthorised -/// toolkits are hidden so the orchestrator cannot hallucinate a delegation -/// to an integration whose `delegate_*` tool does not actually exist. -/// When every toolkit is unconnected the whole section is omitted. +/// Render the `## Connected Integrations` block. Only toolkits the user has +/// actively connected are listed — unauthorised toolkits are hidden so the +/// orchestrator cannot claim access to a service it does not have. When +/// every toolkit is unconnected the whole section is omitted. /// -/// The tool name printed in the prompt is derived with the same -/// `sanitise_slug` function that `collect_orchestrator_tools` uses when -/// synthesising the real tool objects, so the names in the prompt always -/// match the names in the function-calling schema. +/// The connected toolkits' actions are `Deferred` tools on this agent's own +/// belt (`collect_deferred_integration_actions`), so the block teaches one +/// route: `tool_search` for the action, then call it. The old collapsed +/// `delegate_to_integrations_agent` spawn is gone; an integration action is +/// a search and a call, not a sub-agent run. /// -/// `tool_call_format` lets the guide adapt to the active provider. Providers -/// with native structured tool-calling (`ToolCallFormat::Native`) get the -/// historic guide unchanged. Text-protocol providers (`PFormat`/`Json`) — the -/// dispatcher chosen for models that force `native_tool_calling = false`, i.e. -/// local runtimes like Ollama / LM Studio / MLX / llama.cpp — additionally get -/// an explicit "when NOT to delegate" carve-out. Weak local models over-select -/// from the prose tool catalogue and the coercive "you MUST delegate" wording, -/// spuriously routing greetings and local-filesystem actions into -/// `delegate_to_integrations_agent` (issue #4361: "Ciao" → Connections, -/// "create a folder on Desktop" → Calendar). The carve-out is additive: the -/// always-delegate contract for genuine service requests is preserved. -fn render_delegation_guide( - integrations: &[ConnectedIntegration], - tool_call_format: ToolCallFormat, -) -> String { +/// The slug printed beside each toolkit uses the same `sanitise_slug` as the +/// rest of the prompt surface so the model's `composio_connect` argument and +/// its searches name the toolkit consistently. +/// +/// The gated-tools appendix used to live in the integrations sub-agent's +/// prompt. It moves here with the catalogue: an action behind a permission +/// toggle is not in the searchable set, so without this list the model would +/// answer "can you do X?" with a wrong "no" instead of the unlock path. +fn render_connected_integrations(integrations: &[ConnectedIntegration]) -> String { let connected: Vec<&ConnectedIntegration> = integrations.iter().filter(|ci| ci.connected).collect(); tracing::debug!( total_integrations = integrations.len(), connected_count = connected.len(), - "[delegation-guide] rendering integration section ({} connected / {} total)", + "[connected-integrations] rendering integration section ({} connected / {} total)", connected.len(), integrations.len() ); if connected.is_empty() { - tracing::debug!("[delegation-guide] section omitted — no connected integrations"); + tracing::debug!("[connected-integrations] section omitted — no connected integrations"); return String::new(); } let mut out = String::from( "## Connected Integrations\n\n\ - Their tools live in `integrations_agent`, not in your list: act on them only through \ - `delegate_to_integrations_agent` with the toolkit slug, and only when the request \ - operates on that service's data or actions (a connected service is not a reason to \ - touch it for general-knowledge, web/news, date/time or math questions). Never claim \ - you cannot access one without delegating first.\n\n", + Their actions are not in your listed tools: `tool_search` for the action in plain \ + words (\"send an email\", \"list calendar events\"), then call the tool it returns — \ + no sub-agent. Act on a service only when the request operates on that service's data \ + or actions (a connected service is not a reason to touch it for general-knowledge, \ + web/news, date/time or math questions). Never claim you cannot access one without \ + searching first.\n\n", ); - for ci in connected { - // Use the same slug canonicalisation as `collect_orchestrator_tools` - // so the `toolkit` arg the orchestrator emits always matches the - // enum the synthesised tool accepts. + for ci in &connected { let slug = sanitise_slug(&ci.toolkit); if ci.connections.len() > 1 { let _ = writeln!( @@ -550,10 +541,8 @@ fn render_delegation_guide( // SUBSET of the real per-toolkit catalogue (no bulk-delete, no // batch-modify, no admin/destructive actions, etc.). The result is a // confident wrong refusal ("nope, I can't delete emails") even when - // the action is in the actual tool list. The `integrations_agent` - // has the ground-truth tool catalogue (`tools` + `gated_tools`); only - // it can answer "can I do X?" honestly. Force-delegate capability - // questions, not just task requests. + // the action is in the catalogue. `tool_search` is the ground truth for + // callable actions and the gated appendix below for the rest. // The cross-chat bullet names the canonical header literal verbatim // so the model knows exactly which block to mistrust. Sourced from // CROSS_CHAT_HEADER (single source of truth) — drift would silently @@ -565,34 +554,56 @@ fn render_delegation_guide( "\n### Capability questions about connected toolkits\n\n\ Your prior knowledge of what a toolkit can do is unreliable: the live catalogue and \ the user's scopes decide. For \"can you do X with {{toolkit}}?\" or any action on a \ - connected toolkit, delegate first and let `integrations_agent` inspect its tools \ - (including `gated_tools`); the only honest \"no\" is one it reported. A past \ - \"I can / can't\" in the `{cross_chat_header_for_prompt}` block is a stale snapshot, \ - never an answer.\n\n", + connected toolkit, `tool_search` first; the only honest \"no\" is an empty search that \ + the permission-gated list below does not explain. A past \"I can / can't\" in the \ + `{cross_chat_header_for_prompt}` block is a stale snapshot, never an answer.\n\n", ); - if tool_call_format != ToolCallFormat::Native { + // Pref-gated actions: the toolkit has them, the user has not granted the + // scope, so they are not in the searchable catalogue. The agent cannot + // call them and cannot flip the scope itself — the per-row `unlock + // paths` carry the exact UI hint to show the user. + let gated: Vec<&&ConnectedIntegration> = connected + .iter() + .filter(|ci| !ci.gated_tools.is_empty()) + .collect(); + tracing::debug!( + connected_with_gated = gated.len(), + "[connected-integrations] gated-tools scan complete" + ); + if !gated.is_empty() { out.push_str( - "### When NOT to delegate\n\n\ - Some requests are NOT integration work — handle them directly and do NOT call \ - `delegate_to_integrations_agent`:\n\ - - **Greetings and small talk** (\"hi\", \"hello\", \"ciao\", \"thanks\", \"how are \ - you?\") — just reply.\n\ - - **Local-machine actions**: creating, reading, writing, moving, or listing files \ - and folders on this computer (e.g. \"create a folder on the Desktop\", \"make a \ - directory\", \"save this to a file\") — use your local filesystem tools. A local \ - folder/file request is NOT a Calendar, Drive, or any connected-service request.\n\n\ - Delegate ONLY when the request clearly names or operates on one of the connected \ - services listed above (its email, calendar, messages, documents, etc.). When a \ - request mixes a local action with a connected service (\"save my latest email to a \ - file on the Desktop\"), do the local part directly and delegate only the \ - service part.\n\n", + "### Additional capabilities behind a permission toggle\n\n\ + These actions exist in the toolkit but are NOT searchable or callable — the user \ + has not granted the required scope. Do NOT pretend they're unavailable. When the \ + user asks for one (or you'd otherwise need it), tell them what the action does and \ + present ALL of its `unlock paths` listed below so the user can choose how to enable \ + it. Never drop a path or rewrite it into your own framing.\n\n", ); + for ci in gated { + let _ = writeln!(out, "- **{}**:", ci.toolkit); + for gt in &ci.gated_tools { + let desc = if gt.description.is_empty() { + "(no description)" + } else { + gt.description.as_str() + }; + let _ = writeln!( + out, + " - `{}` — {} (requires `{}` scope)", + gt.name, desc, gt.required_scope + ); + for path in >.unlock_paths { + let _ = writeln!(out, " - unlock path: {path}"); + } + } + } + out.push('\n'); } tracing::debug!( section_len = out.len(), - "[delegation-guide] section emitted ({} bytes)", + "[connected-integrations] section emitted ({} bytes)", out.len() ); out diff --git a/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt_tests.rs b/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt_tests.rs index 9859eeea87..1b0cf40e4b 100644 --- a/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt_tests.rs +++ b/crates/openhuman-core/src/agent/registry/agents/orchestrator/prompt_tests.rs @@ -280,9 +280,15 @@ fn build_includes_direct_first_decision_tree() { assert!(body.contains("Take the first branch that applies:")); assert!(body.contains("**Answerable without tools**: reply.")); // Step 2 of the decision tree routes live external-service requests to - // `delegate_to_integrations_agent` rather than memory. + // a `tool_search` + direct call rather than memory or a sub-agent. assert!(body.contains("Needs a connected service's own data or actions")); assert!(body.contains("Use the live service even when memory could plausibly answer")); + assert!(body.contains("No sub-agent runs it for you")); + // The lead-in rule lives on the branch where the failure was observed: a + // live run had the model answer "let me search for the right tool" and + // end the turn without emitting the `tool_search` call. + assert!(body.contains("an announced search never runs")); + assert!(!body.contains("delegate_to_integrations_agent")); } #[test] @@ -292,7 +298,7 @@ fn build_routes_live_facts_to_research_tool() { assert!(body.contains("weather, forecasts, prices, recent news")); assert!(body.contains("\"use live data\"")); // A lead-in line is welcome, but only in the same message as the call. - assert!(body.contains("Don't stop at a lead-in; make the tool call in the same message.")); + assert!(body.contains("an announced search never runs: emit it")); assert!( !body.contains("delegate_researcher"), "orchestrator prompt should name the synthesized researcher tool" @@ -312,7 +318,7 @@ fn build_routes_code_repo_work_to_run_code_tool() { } #[test] -fn build_emits_delegation_guide_with_collapsed_tool() { +fn build_emits_connected_integrations_as_search_then_call() { let integrations = vec![ConnectedIntegration { toolkit: "gmail".into(), description: "Email access.".into(), @@ -324,18 +330,22 @@ fn build_emits_delegation_guide_with_collapsed_tool() { }]; let body = build(&ctx_with(&integrations)).unwrap(); assert!(body.contains("## Connected Integrations")); - assert!(body.contains("delegate_to_integrations_agent")); assert!(body.contains("toolkit: \"gmail\"")); - // Must NOT contain the old per-toolkit fan-out tool names. + // The route is the harness's search bridge, not a sub-agent. + assert!(body.contains("`tool_search` for the action")); + assert!(body.contains("no sub-agent")); + // The removed delegate and the old per-toolkit fan-out must be gone. + assert!(!body.contains("delegate_to_integrations_agent")); assert!(!body.contains("delegate_gmail")); - // Must NOT contain the old verbose spawn_subagent snippet. + assert!(!body.contains("integrations_agent")); assert!(!body.contains("spawn_subagent(agent_id=\"integrations_agent\"")); - // Delegator voice must NOT use the skill-executor wording. + // The "you have direct access" skill-executor wording stays out: the + // actions are not on the wire, they are searchable. assert!(!body.contains("You have direct access")); - // Must keep the always-delegate contract for real service asks. + // Must keep the always-try contract for real service asks. assert!( - body.contains("Never claim you cannot access one without delegating first"), - "delegation guide must instruct the model to always attempt delegation" + body.contains("Never claim you cannot access one without searching first"), + "the block must instruct the model to search before refusing" ); } @@ -343,13 +353,13 @@ fn build_emits_delegation_guide_with_collapsed_tool() { fn build_scope_gates_integrations_delegation() { // Regression: a connected service (e.g. Gmail) is not, by itself, a // reason to operate on it — a general-knowledge / web / date ask that - // names no service must NOT spawn `delegate_to_integrations_agent`. + // names no service must NOT reach for a service action. // Guards both the static Step-2 scope gate and the rendered - // delegation-guide clause. + // connected-integrations clause. let no_integrations = build(&ctx_with(&[])).unwrap(); assert!( - no_integrations.contains("general knowledge, web/news lookups, headlines, date/time, math, and anything public on the web (a public repository, a product page, docs) never delegate here"), - "Step-2 scope gate must keep general/web/date asks off integrations delegation" + no_integrations.contains("general knowledge, web/news lookups, headlines, date/time, math, and anything public on the web (a public repository, a product page, docs) never go to a service"), + "Step-2 scope gate must keep general/web/date asks off integration actions" ); assert!( no_integrations.contains("A service being connected is not a reason to touch it"), @@ -369,10 +379,10 @@ fn build_scope_gates_integrations_delegation() { assert!( with_gmail .contains("a connected service is not a reason to touch it for general-knowledge"), - "delegation guide must carry the scoping clause when integrations are connected" + "connected-integrations block must carry the scoping clause when integrations are connected" ); - // The existing always-delegate contract for real service asks is preserved. - assert!(with_gmail.contains("Never claim you cannot access one without delegating first")); + // The existing always-try contract for real service asks is preserved. + assert!(with_gmail.contains("Never claim you cannot access one without searching first")); } #[test] @@ -387,25 +397,6 @@ fn build_does_not_route_scope_errors_as_disconnected() { assert!(body.contains("`composio_connect`")); } -#[test] -fn delegation_guide_uses_compact_collapsed_format() { - let integrations = vec![ConnectedIntegration { - toolkit: "gmail".into(), - description: "Email access.".into(), - tools: Vec::new(), - gated_tools: Vec::new(), - connected: true, - connections: Vec::new(), - non_active_status: None, - }]; - let body = build(&ctx_with(&integrations)).unwrap(); - assert!(body.contains("## Connected Integrations")); - assert!(body.contains("delegate_to_integrations_agent")); - // Old verbose / per-toolkit forms must be gone. - assert!(!body.contains("delegate_gmail")); - assert!(!body.contains("spawn_subagent(agent_id=\"integrations_agent\"")); -} - fn gmail_only() -> Vec { vec![ConnectedIntegration { toolkit: "gmail".into(), @@ -418,66 +409,54 @@ fn gmail_only() -> Vec { }] } -// Regression for #4361: on local providers (`native_tool_calling = false` -// → PFormat/Json dispatcher) the whole tool catalogue is prose and weak -// models mis-route trivial requests through the integrations delegate -// ("Ciao" → Connections, "create a folder on Desktop" → Calendar). The -// delegation guide must add an explicit non-delegation carve-out for those -// text-protocol providers. +// The block is the same for every dispatcher. The text-protocol "When NOT to +// delegate" carve-out (#4361) guarded a delegate tool that no longer exists; +// the scoping clause in the block body carries that rule for every format. #[test] -fn delegation_guide_adds_local_guardrail_for_text_protocol() { - let integrations = gmail_only(); - for format in [ToolCallFormat::PFormat, ToolCallFormat::Json] { - let guide = render_delegation_guide(&integrations, format); - assert!( - guide.contains("### When NOT to delegate"), - "text-protocol ({format:?}) guide must carve out non-integration work" - ); - // The two reported failure modes are named explicitly. - assert!( - guide.contains("create a folder on the Desktop"), - "guardrail must keep local folder/file actions off delegation ({format:?})" - ); - assert!( - guide.to_ascii_lowercase().contains("greetings"), - "guardrail must keep greetings off delegation ({format:?})" - ); - // Additive: the always-delegate contract for real service requests - // is preserved — the guardrail narrows, it does not remove it. - assert!( - guide.contains("Never claim you cannot access one without delegating first"), - "always-delegate contract must remain for genuine service asks ({format:?})" - ); - } +fn connected_integrations_block_is_format_independent() { + let guide = render_connected_integrations(&gmail_only()); + assert!(guide.contains("## Connected Integrations")); + assert!(guide.contains("`tool_search` for the action")); + assert!(!guide.contains("### When NOT to delegate")); + assert!(guide.contains("a connected service is not a reason to touch it")); + assert!(guide.contains("Never claim you cannot access one without searching first")); } -// Native structured-tool-calling providers (cloud) keep the historic guide -// byte-for-byte: no over-delegation problem, so no carve-out. +// Capability questions are answered from the searchable catalogue, never +// from priors and never by spawning a worker to look. #[test] -fn delegation_guide_omits_local_guardrail_for_native() { - let guide = render_delegation_guide(&gmail_only(), ToolCallFormat::Native); - assert!(guide.contains("## Connected Integrations")); - assert!( - !guide.contains("### When NOT to delegate"), - "native providers must keep the delegation guide unchanged" - ); - assert!(guide.contains("Never claim you cannot access one without delegating first")); +fn connected_integrations_block_routes_capability_questions_to_search() { + let guide = render_connected_integrations(&gmail_only()); + assert!(guide.contains("### Capability questions about connected toolkits")); + assert!(guide.contains("`tool_search` first")); + assert!(!guide.contains("integrations_agent")); } -// With no connected integrations the section is omitted for every format — -// the guardrail must never resurrect an otherwise-empty block. +// Pref-gated actions are not searchable, so the block lists them with their +// unlock paths — the appendix that used to live in the integrations +// sub-agent's prompt. #[test] -fn delegation_guide_empty_without_connections_for_all_formats() { - for format in [ - ToolCallFormat::PFormat, - ToolCallFormat::Json, - ToolCallFormat::Native, - ] { - assert!( - render_delegation_guide(&[], format).is_empty(), - "empty connections must omit the section ({format:?})" - ); - } +fn connected_integrations_block_lists_gated_actions_with_unlock_paths() { + let mut integrations = gmail_only(); + integrations[0].gated_tools = vec![crate::agent::prompts::GatedIntegrationTool { + name: "GMAIL_DELETE_MESSAGE".into(), + description: "Delete a message".into(), + required_scope: "delete".into(), + unlock_paths: vec!["Connections → Gmail → Delete".into()], + }]; + let guide = render_connected_integrations(&integrations); + assert!(guide.contains("### Additional capabilities behind a permission toggle")); + assert!(guide.contains("`GMAIL_DELETE_MESSAGE` — Delete a message (requires `delete` scope)")); + assert!(guide.contains("unlock path: Connections → Gmail → Delete")); + + let without = render_connected_integrations(&gmail_only()); + assert!(!without.contains("### Additional capabilities behind a permission toggle")); +} + +// With no connected integrations the section is omitted. +#[test] +fn connected_integrations_block_empty_without_connections() { + assert!(render_connected_integrations(&[]).is_empty()); } #[test] @@ -554,13 +533,15 @@ fn build_includes_evidence_aware_synthesis_contract() { assert!(body.contains("truncated, oversized, partial or unavailable")); assert!(body.contains("Preserve numeric evidence exactly")); assert!(body.contains("plus whatever `tool_search` returns")); - assert!(body.contains("call `tool_search` with the intent in plain words")); + assert!(body.contains("`tool_search` with the intent in plain words")); // Under the native dialect no tool is "listed in this prompt"; a model told // that its tools are the listed ones concluded it had no web search while // `web_search_tool` sat in its tool list (thread-7e52b, 2026-09-22). assert!(!body.contains("listed in this prompt"), "{body}"); assert!(body.contains("`web_search_tool` and `web_fetch` are usually in it")); - assert!(body.contains("anything public on the web (a public repository, a product page, docs) never delegate here")); + assert!(body.contains( + "anything public on the web (a public repository, a product page, docs) never go to a service" + )); } #[test] diff --git a/crates/openhuman-core/src/agent/registry/agents/planner/prompt.md b/crates/openhuman-core/src/agent/registry/agents/planner/prompt.md index 3b7d9e8c5a..87f0e979cf 100644 --- a/crates/openhuman-core/src/agent/registry/agents/planner/prompt.md +++ b/crates/openhuman-core/src/agent/registry/agents/planner/prompt.md @@ -33,14 +33,13 @@ Return **only** valid JSON matching this schema: ## Available Agent IDs - `code_executor` — Writes and runs code. Use for implementation tasks. -- `integrations_agent` — Executes skill tools (Notion, Gmail, etc.). Use for service interactions. - `tool_maker` — Writes polyfill scripts. Rarely needed in planning. - `researcher` — Reads docs, web searches. Use for information gathering. - `critic` — Reviews code quality and security. Use after code changes. ## Rules -0. **You are the reasoning tier.** The chat-tier Orchestrator handed off to you because the task needs sustained thinking. Compose plans for the **worker tier** — `code_executor`, `researcher`, `critic`, `integrations_agent`, `archivist`. **Never delegate to another reasoning agent** (no planner-spawns-planner, no planner-spawns-orchestrator); the loader rejects this at boot, and the planned runtime depth gate will reject it at spawn time. If a single worker can't cover a node, split the node — don't smuggle a second reasoning hop in. +0. **You are the reasoning tier.** The chat-tier Orchestrator handed off to you because the task needs sustained thinking. Compose plans for the **worker tier** — `code_executor`, `researcher`, `critic`, `archivist`. Connected-service actions belong to the orchestrator, not a worker. **Never delegate to another reasoning agent** (no planner-spawns-planner, no planner-spawns-orchestrator); the loader rejects this at boot, and the planned runtime depth gate will reject it at spawn time. If a single worker can't cover a node, split the node — don't smuggle a second reasoning hop in. 1. **Gather before planning** — Search memory and the web first. Don't guess what you can look up. 2. **Minimise tasks** — Use the fewest nodes needed. Don't over-decompose. 3. **Dependencies matter** — Use `depends_on` to express ordering. Independent tasks run in parallel. diff --git a/crates/openhuman-core/src/agent/registry/agents/tools_agent/agent.toml b/crates/openhuman-core/src/agent/registry/agents/tools_agent/agent.toml index 0c75a3b199..f1de4c11a4 100644 --- a/crates/openhuman-core/src/agent/registry/agents/tools_agent/agent.toml +++ b/crates/openhuman-core/src/agent/registry/agents/tools_agent/agent.toml @@ -17,7 +17,7 @@ hint = "burst" # surface. Composio meta-tools and dynamic `_*` action tools are # stripped at runtime (see `filter_non_composio_indices` in the subagent # runner), so the LLM never sees integration-specific tools here; those belong -# to `integrations_agent`. Specialist-owned trading tools are also +# to the orchestrator (searched through `tool_search`). Specialist-owned trading tools are also # stripped via `disallowed_tools` above so they route through their dedicated # agents exclusively. wildcard = {} diff --git a/crates/openhuman-core/src/agent/registry/agents/tools_agent/prompt.md b/crates/openhuman-core/src/agent/registry/agents/tools_agent/prompt.md index e283562617..a532b33827 100644 --- a/crates/openhuman-core/src/agent/registry/agents/tools_agent/prompt.md +++ b/crates/openhuman-core/src/agent/registry/agents/tools_agent/prompt.md @@ -4,7 +4,7 @@ You are the **Tools Agent**. You complete ad-hoc tasks using only OpenHuman's bu ## Scope -- You do **NOT** have access to Composio / managed OAuth integrations. If a task requires acting on an external SaaS account (Gmail, Notion, GitHub, Slack, …), stop and report back — the orchestrator will spawn `integrations_agent` with the correct toolkit. +- You do **NOT** have access to Composio / managed OAuth integrations. If a task requires acting on an external SaaS account (Gmail, Notion, GitHub, Slack, …), stop and report back — the orchestrator calls connected-service actions itself. - You **DO** handle: running commands, reading and writing files in the workspace, scraping the web, searching the user's memory, querying structured data, chaining simple transformations. ## Operating rules diff --git a/crates/openhuman-core/src/agent/session_host/announcement_notes.rs b/crates/openhuman-core/src/agent/session_host/announcement_notes.rs index 222eafd24b..36db9893df 100644 --- a/crates/openhuman-core/src/agent/session_host/announcement_notes.rs +++ b/crates/openhuman-core/src/agent/session_host/announcement_notes.rs @@ -15,7 +15,7 @@ Do not tell the user to reconnect or restart."; pub(super) fn integration_announcement_note(slugs: &[String]) -> Option { (!slugs.is_empty()).then(|| format!( - "[integration update] These integration(s) connected during this conversation and are available now via delegate_to_integrations_agent with the matching toolkit slug: {}. {ANNOUNCEMENT_TRAILER}", + "[integration update] These integration(s) connected during this conversation and are available now; their actions are reachable through tool_search: {}. {ANNOUNCEMENT_TRAILER}", slugs.join(", ") )) } diff --git a/crates/openhuman-core/src/agent/session_host/builder/factory.rs b/crates/openhuman-core/src/agent/session_host/builder/factory.rs index 64a2f994b2..1b14220b44 100644 --- a/crates/openhuman-core/src/agent/session_host/builder/factory.rs +++ b/crates/openhuman-core/src/agent/session_host/builder/factory.rs @@ -583,10 +583,9 @@ impl OpenHumanSessionHost { // // For an agent with `[subagents] allowlist = [...]` in its TOML (today: // orchestrator), `collect_orchestrator_tools` synthesises one - // `ArchetypeDelegationTool` per named sub-agent plus a single - // collapsed `SkillDelegationTool` - // (`delegate_to_integrations_agent`) whose `toolkit` argument - // selects among the connected Composio toolkits (#1335). + // `ArchetypeDelegationTool` per named sub-agent plus, for the + // `{ skills = "*" }` wildcard, one `Deferred` action tool per + // connected Composio action (reached through `tool_search`). // // For an agent without `subagents` (today: welcome, critic, // archivist, etc.), no delegation tools are synthesised — the @@ -597,7 +596,7 @@ impl OpenHumanSessionHost { // This builder is synchronous and sits on the CLI / REPL / // Tauri-web code path. It still opportunistically reuses the // process-wide Composio cache when one is already warm, which - // lets the session start with the right `delegate_` + // lets the session start with the right integration action // surface and prompt block without paying a turn-1 fetch. On a // cold cache we still fall back to the empty slice and let the // first turn repair the session state if needed. diff --git a/crates/openhuman-core/src/agent/session_host/builder/mod.rs b/crates/openhuman-core/src/agent/session_host/builder/mod.rs index f870468ed9..bb773421e9 100644 --- a/crates/openhuman-core/src/agent/session_host/builder/mod.rs +++ b/crates/openhuman-core/src/agent/session_host/builder/mod.rs @@ -223,7 +223,6 @@ pub(super) fn should_synthesize_delegation_tools(def: &AgentDefinition) -> bool /// the name extends at an `_` boundary. Empty when the registry is not up or /// the id resolves to nothing, which leaves the schema untouched. fn allowed_subagent_ids_for(agent_id: &str) -> Vec { - use crate::agent::harness::definition::SubagentEntry; let Some(registry) = crate::agent::harness::AgentDefinitionRegistry::global() else { return Vec::new(); }; @@ -244,15 +243,5 @@ fn allowed_subagent_ids_for(agent_id: &str) -> Vec { let Some(definition) = definition else { return Vec::new(); }; - definition - .subagents - .iter() - .filter_map(|entry| match entry { - SubagentEntry::AgentId(id) => Some(id.clone()), - SubagentEntry::Skills(wildcard) if wildcard.matches_all() => { - Some("integrations_agent".to_string()) - } - SubagentEntry::Skills(_) => None, - }) - .collect() + definition.allowed_subagent_ids() } diff --git a/crates/openhuman-core/src/agent/session_host/runtime_adapter_tests.rs b/crates/openhuman-core/src/agent/session_host/runtime_adapter_tests.rs index b0b68fe35c..0b127693c9 100644 --- a/crates/openhuman-core/src/agent/session_host/runtime_adapter_tests.rs +++ b/crates/openhuman-core/src/agent/session_host/runtime_adapter_tests.rs @@ -400,7 +400,8 @@ fn availability_notes_are_status_not_instructions() { ); assert!(note.contains("Do not tell the user to reconnect or restart")); } - assert!(notes[0].contains("delegate_to_integrations_agent") && notes[0].contains("gmail")); + assert!(notes[0].contains("tool_search") && notes[0].contains("gmail")); + assert!(!notes[0].contains("delegate_to_integrations_agent")); assert!(notes[1].contains("use_mcp_server") && notes[1].contains("filesystem, github")); assert!(notes[2].contains("run_skill") && notes[2].contains("deploy")); assert!(integration_announcement_note(&[]).is_none()); diff --git a/crates/openhuman-core/src/agent/session_host/runtime_session.rs b/crates/openhuman-core/src/agent/session_host/runtime_session.rs index d2bc0de7fc..76e8a02e01 100644 --- a/crates/openhuman-core/src/agent/session_host/runtime_session.rs +++ b/crates/openhuman-core/src/agent/session_host/runtime_session.rs @@ -357,8 +357,13 @@ impl OpenHumanTurnPrelude { .chain(surface.synthesized_tools.iter()) .map(|tool| tool.as_ref()) .collect::>(); - let prompt_tools = PromptTool::from_tool_refs(tool_refs.iter().copied()); - let visible_tool_names = surface.tool_policy_session.visible_tool_names_for_prompt(); + let mut prompt_tools = PromptTool::from_tool_refs(tool_refs.iter().copied()); + let mut visible_tool_names = surface.tool_policy_session.visible_tool_names_for_prompt(); + crate::agent::prompts::swap_deferred_for_discovery_bridge( + &mut prompt_tools, + &mut visible_tool_names, + &surface.deferred_tool_names, + ); let agents_md = if self.config.agents_md_enabled { crate::agent::prompts::load_agents_md_layers(&self.workspace_dir, &self.action_dir) } else { @@ -1538,23 +1543,7 @@ impl OpenHumanSessionHost { run_queue: self.run_queue.clone(), allowed_subagent_ids: self .resolved_definition() - .map(|definition| { - definition - .subagents - .iter() - .filter_map(|entry| match entry { - crate::agent::harness::definition::SubagentEntry::AgentId(id) => { - Some(id.clone()) - } - crate::agent::harness::definition::SubagentEntry::Skills( - wildcard, - ) if wildcard.matches_all() => { - Some("integrations_agent".to_string()) - } - crate::agent::harness::definition::SubagentEntry::Skills(_) => None, - }) - .collect() - }) + .map(|definition| definition.allowed_subagent_ids().into_iter().collect()) .unwrap_or_default(), sandbox_mode: self .resolved_definition() diff --git a/crates/openhuman-core/src/agent/session_host/turn/context.rs b/crates/openhuman-core/src/agent/session_host/turn/context.rs index 6854356508..ccdf326be7 100644 --- a/crates/openhuman-core/src/agent/session_host/turn/context.rs +++ b/crates/openhuman-core/src/agent/session_host/turn/context.rs @@ -215,8 +215,14 @@ impl OpenHumanSessionHost { // prompt build. The synthesised delegates belong here: the catalogue // this renders is what tells the model a `delegate_*` tool exists. let all_tools = self.all_tool_refs(); - let prompt_tools = PromptTool::from_tool_refs(all_tools.iter().copied()); - let prompt_visible_tool_names = self.tool_policy_session.visible_tool_names_for_prompt(); + let mut prompt_tools = PromptTool::from_tool_refs(all_tools.iter().copied()); + let mut prompt_visible_tool_names = + self.tool_policy_session.visible_tool_names_for_prompt(); + crate::agent::prompts::swap_deferred_for_discovery_bridge( + &mut prompt_tools, + &mut prompt_visible_tool_names, + &self.deferred_tool_names, + ); // Load AGENTS.md instruction layers once per system-prompt build (never // re-read per turn — the caller builds the prompt once at session start // and reuses the bytes, preserving the frozen-prefix / KV-cache diff --git a/crates/openhuman-core/src/agent/session_host/turn/tools.rs b/crates/openhuman-core/src/agent/session_host/turn/tools.rs index 9c78c61065..62f8390b55 100644 --- a/crates/openhuman-core/src/agent/session_host/turn/tools.rs +++ b/crates/openhuman-core/src/agent/session_host/turn/tools.rs @@ -38,23 +38,7 @@ impl OpenHumanSessionHost { } let allowed_subagent_ids = self .resolved_definition() - .map(|definition| { - definition - .subagents - .iter() - .filter_map(|entry| match entry { - crate::agent::harness::definition::SubagentEntry::AgentId(id) => { - Some(id.clone()) - } - crate::agent::harness::definition::SubagentEntry::Skills(wildcard) - if wildcard.matches_all() => - { - Some("integrations_agent".to_string()) - } - crate::agent::harness::definition::SubagentEntry::Skills(_) => None, - }) - .collect() - }) + .map(|definition| definition.allowed_subagent_ids().into_iter().collect()) .unwrap_or_default(); harness::ParentExecutionContext { diff --git a/crates/openhuman-core/src/agent/subagent_host/ops/runner.rs b/crates/openhuman-core/src/agent/subagent_host/ops/runner.rs index 10714ab74c..c19bad6e19 100644 --- a/crates/openhuman-core/src/agent/subagent_host/ops/runner.rs +++ b/crates/openhuman-core/src/agent/subagent_host/ops/runner.rs @@ -65,10 +65,9 @@ use super::provider::{ /// initialised. A `None` parent yields `Ok(())`: we skip rather than mask, the /// same defensive posture the loader takes for unknown child ids. /// -/// A **worker** parent is also exempted. At runtime a worker only reaches the -/// spawn chokepoint via the documented collapsed `delegate_to_integrations_agent` -/// path (→ `integrations_agent`, itself a worker) — a shape the loader -/// intentionally leaves untouched. Re-denying it here would turn valid custom +/// A **worker** parent is also exempted. A worker's `subagents` list holds no +/// agent id (the loader rejects one), so any spawn it reaches at runtime is +/// one the host dispatched for it. Re-denying it here would turn valid custom /// worker agents that use `{ skills = "*" }` into runtime failures. The /// worker-leaf authoring rule stays enforced statically at boot, and the /// per-parent allowlist gate blocks any other worker spawn. @@ -1371,14 +1370,14 @@ async fn run_typed_mode( .map(|&i| { let t = parent.all_tools[i].as_ref(); PromptTool { - name: t.name(), - description: t.description(), + name: std::borrow::Cow::Borrowed(t.name()), + description: std::borrow::Cow::Borrowed(t.description()), parameters_schema: Some(t.parameters_schema().to_string()), } }) .chain(dynamic_tools.iter().map(|t| PromptTool { - name: t.name(), - description: t.description(), + name: std::borrow::Cow::Borrowed(t.name()), + description: std::borrow::Cow::Borrowed(t.description()), parameters_schema: Some(t.parameters_schema().to_string()), })) .collect(); diff --git a/crates/openhuman-core/src/agent/subagent_host/ops_tests_tier_gate_tests.rs b/crates/openhuman-core/src/agent/subagent_host/ops_tests_tier_gate_tests.rs index 0cf2602b67..a024fe8af5 100644 --- a/crates/openhuman-core/src/agent/subagent_host/ops_tests_tier_gate_tests.rs +++ b/crates/openhuman-core/src/agent/subagent_host/ops_tests_tier_gate_tests.rs @@ -67,13 +67,14 @@ fn tier_gate_allows_legal_descending_hops() { } #[test] -fn tier_gate_allows_worker_parent_for_collapsed_integration() { +fn tier_gate_allows_worker_parent() { use crate::agent::harness::definition::AgentTier; - // A worker only reaches the runtime spawn chokepoint via the documented - // collapsed `delegate_to_integrations_agent` path (→ `integrations_agent`, - // itself a worker). The gate must NOT re-deny that — the worker-leaf rule - // is a static boot-time authoring constraint, not a runtime one. Regression - // for the wildcard-integration case (CodeRabbit P2 on PR #4102). + // A worker's `subagents` list holds no agent id (the loader rejects + // one), so any spawn it reaches at runtime is one the host dispatched for + // it. The gate must NOT re-deny that — the worker-leaf rule is a static + // boot-time authoring constraint, not a runtime one; the per-parent + // allowlist gate blocks any other worker spawn. Regression for the + // wildcard-integration case (CodeRabbit P2 on PR #4102). let mut parent = make_def_named_tools(&[]); let child = make_def_named_tools(&[]); // worker by default parent.agent_tier = AgentTier::Worker; diff --git a/crates/openhuman-core/src/agent/subagent_host/tool_prep_tests.rs b/crates/openhuman-core/src/agent/subagent_host/tool_prep_tests.rs index 13bff36209..888470fbfc 100644 --- a/crates/openhuman-core/src/agent/subagent_host/tool_prep_tests.rs +++ b/crates/openhuman-core/src/agent/subagent_host/tool_prep_tests.rs @@ -417,13 +417,11 @@ fn a_dynamic_tool_list_without_spawn_tools_is_untouched() { /// Render the `## Tools` section the way a sub-agent prompt does, at `format`. fn render_tools_at(format: crate::agent::prompts::ToolCallFormat, instructions: &str) -> String { use crate::agent::prompts::{LearnedContextData, PromptTool}; - let tools = [PromptTool { - name: "composio_execute", - description: "Run one Composio action.", - parameters_schema: Some( - r#"{"type":"object","properties":{"action":{"type":"string"}}}"#.into(), - ), - }]; + let tools = [PromptTool::with_schema( + "composio_execute", + "Run one Composio action.", + r#"{"type":"object","properties":{"action":{"type":"string"}}}"#.into(), + )]; let visible = std::collections::HashSet::new(); let ctx = PromptContext { workspace_dir: std::path::Path::new("/tmp"), diff --git a/crates/openhuman-core/src/agent/tinyagents/discovery/discovery_tests.rs b/crates/openhuman-core/src/agent/tinyagents/discovery/discovery_tests.rs index 406a2084f5..effa922a89 100644 --- a/crates/openhuman-core/src/agent/tinyagents/discovery/discovery_tests.rs +++ b/crates/openhuman-core/src/agent/tinyagents/discovery/discovery_tests.rs @@ -81,3 +81,50 @@ async fn overlap_ranker_ranks_by_token_overlap_and_names_its_kind() { .await .is_err()); } + +/// A text dialect renders the prompt catalogue and the harness clears +/// `request.tools`, so the bridge only reaches the model through the host's +/// own catalogue. Without these entries the model reads a search result +/// telling it to "invoke a match with `tool_call`" and has no signature for +/// that name — observed live as a turn that narrates the call it is about to +/// make and then stops. +#[test] +fn bridge_prompt_tools_advertise_search_and_call_when_something_is_deferred() { + let _g = guard(); + apply_tool_search_config(&ToolSearchConfig::default()); + let bridge = bridge_prompt_tools(12); + let names: Vec<&str> = bridge.iter().map(|t| t.name.as_ref()).collect(); + assert_eq!(names, vec!["tool_search", "tool_call"]); + for tool in &bridge { + assert!( + tool.parameters_schema + .as_deref() + .is_some_and(|schema| schema.contains("\"type\":\"object\"")), + "{} must carry a callable schema, got {:?}", + tool.name, + tool.parameters_schema + ); + } +} + +/// The manifest is left out on purpose: naming every deferred tool in the +/// prompt is the cost deferral exists to avoid. +#[test] +fn bridge_prompt_tools_do_not_enumerate_the_deferred_catalogue() { + let _g = guard(); + apply_tool_search_config(&ToolSearchConfig::default()); + let search = bridge_prompt_tools(3).into_iter().next().expect("bridge"); + assert!( + search.description.len() < 1_000, + "the bridge description must stay a pointer to the search, not a catalogue: {} bytes", + search.description.len() + ); +} + +/// Nothing deferred, nothing to advertise: a belt that did not opt into +/// discovery must not pay two schemas for a bridge it cannot use. +#[test] +fn bridge_prompt_tools_are_empty_without_a_deferred_catalogue() { + let _g = guard(); + assert!(bridge_prompt_tools(0).is_empty()); +} diff --git a/crates/openhuman-core/src/agent/tinyagents/discovery/mod.rs b/crates/openhuman-core/src/agent/tinyagents/discovery/mod.rs index f77dafa80d..9f4988fc29 100644 --- a/crates/openhuman-core/src/agent/tinyagents/discovery/mod.rs +++ b/crates/openhuman-core/src/agent/tinyagents/discovery/mod.rs @@ -122,6 +122,50 @@ pub(crate) fn discovery_policy() -> ToolDiscoveryPolicy { policy } +/// The `tool_search` / `tool_call` bridge, rendered for a TEXT dialect's +/// prompt catalogue. +/// +/// The harness mints these two schemas onto `request.tools` whenever a run +/// has a deferred catalogue, which is all a native-tool-calling provider +/// needs. A text dialect (P-Format / code) never sees that set: the dialect +/// folds the catalogue into the system prompt and clears `request.tools`, +/// and because OpenHuman sets `host_renders_tool_catalogue` the harness +/// appends nothing of its own. So on those dialects the bridge only reaches +/// the model if the host's own catalogue carries it — otherwise the model +/// reads "invoke a match with `tool_call`" in a search result naming a tool +/// no signature in its catalogue describes, and answers with intent instead +/// of a call. +/// +/// Built from [`tinyagents_harness::tool::discover::bridge_schemas`] rather +/// than hand-written prose, so the signature the model reads is the one +/// admission accepts. `deferred` only sizes the placeholder catalogue the +/// manifest is rendered from; the per-tool manifest itself is deliberately +/// left out of the prompt, because listing every deferred tool there is the +/// cost deferral exists to avoid. +pub(crate) fn bridge_prompt_tools( + deferred: usize, +) -> Vec> { + if deferred == 0 { + return Vec::new(); + } + use tinyagents_harness::tool::discover::{bridge_schemas, DeferredCatalog}; + let mut policy = discovery_policy(); + // A zero budget renders the manifest as a bare count instead of naming + // every deferred tool — the prompt advertises that a search exists, not + // what it would find. + policy.manifest_token_budget = 0; + bridge_schemas(&DeferredCatalog::build(Vec::new()), &policy) + .into_iter() + .map(|schema| { + crate::agent::prompts::PromptTool::owned( + schema.name, + schema.description, + schema.parameters.to_string(), + ) + }) + .collect() +} + /// The verb-gated token-overlap ranker the Composio sub-agent narrows its /// toolkit with (`tinyagents_harness::tool::select::rank_tools_by_prompt`), /// behind the [`ToolRanker`] trait so it can be installed, compared, or diff --git a/crates/openhuman-core/src/agent/tinyagents/host/definition_registry.rs b/crates/openhuman-core/src/agent/tinyagents/host/definition_registry.rs index c1ccc0eda4..97b9237526 100644 --- a/crates/openhuman-core/src/agent/tinyagents/host/definition_registry.rs +++ b/crates/openhuman-core/src/agent/tinyagents/host/definition_registry.rs @@ -59,9 +59,9 @@ //! [`Self::with_registered_tools`], failing closed when that is absent. //! //! **4. `SubagentEntry::Skills` entries are omitted.** A `{ skills = "*" }` -//! entry is not an agent id — it collapses into the single -//! `delegate_to_integrations_agent` tool. Emitting a synthetic id here would -//! invent a delegate the host never authorized. +//! entry is not an agent id — it expands to the connected integrations' +//! actions as searchable tools. Emitting a synthetic id here would invent a +//! delegate the host never authorized. //! use std::collections::HashSet; @@ -397,8 +397,8 @@ impl OpenHumanDefinitionRegistry { /// Declared subagent **agent ids** only. /// /// [`SubagentEntry::Skills`] entries are skipped: they are a wildcard that -/// collapses to the single `delegate_to_integrations_agent` tool, not an agent -/// the parent may address by id. +/// expands to searchable integration actions, not an agent the parent may +/// address by id. fn declared_subagent_ids(def: &HostAgentDefinition) -> Vec { def.subagents .iter() diff --git a/crates/openhuman-core/src/channels/runtime/dispatch/mod_scoping_tests_tests.rs b/crates/openhuman-core/src/channels/runtime/dispatch/mod_scoping_tests_tests.rs index 0c4c2ebe38..e95ae435ef 100644 --- a/crates/openhuman-core/src/channels/runtime/dispatch/mod_scoping_tests_tests.rs +++ b/crates/openhuman-core/src/channels/runtime/dispatch/mod_scoping_tests_tests.rs @@ -110,7 +110,7 @@ fn named_scope_without_extras_returns_named_only() { /// `ToolScope::Named` with extras returns the union of the TOML /// named list and the extras' names. This is the orchestrator's /// path: direct tools from the TOML + the synthesised delegation -/// tools (`research`, `plan`, `delegate_to_integrations_agent`) +/// tools (`research`, `plan`) and deferred integration actions /// → all of them visible to the orchestrator's LLM. The stub /// names in this test are arbitrary; they exercise the union /// logic, not the real synthesiser. diff --git a/crates/openhuman-core/src/channels/runtime/dispatch/routing.rs b/crates/openhuman-core/src/channels/runtime/dispatch/routing.rs index b376f5a577..d2f7f5afd0 100644 --- a/crates/openhuman-core/src/channels/runtime/dispatch/routing.rs +++ b/crates/openhuman-core/src/channels/runtime/dispatch/routing.rs @@ -114,8 +114,8 @@ pub(super) async fn resolve_target_agent(channel: &str) -> AgentScoping { // // Crucially, a transient failure (backend 5xx / no client for a beat) or a // timeout must NOT be laundered into "zero connected integrations": that - // would drop `delegate_to_integrations_agent` from the turn's tool surface - // and leave the channel agent unable to reach Gmail/Slack/etc. — the exact + // would drop every integration action from the turn's searchable tool + // catalogue and leave the channel agent unable to reach Gmail/Slack/etc. — the exact // "just normal inference, no tool calling" symptom. So we take the // status-returning fetch and, on `Unavailable`/timeout, fall back to the // last cached snapshot (same defence the first-party turn path uses) rather @@ -138,7 +138,7 @@ pub(super) async fn resolve_target_agent(channel: &str) -> AgentScoping { channel = %channel, target_agent = target_id, timed_out = fetched.is_none(), - "[dispatch::routing] Composio unavailable/timed out — using cached integration snapshot instead of an empty set (keeps delegate_to_integrations_agent live)" + "[dispatch::routing] Composio unavailable/timed out — using cached integration snapshot instead of an empty set (keeps the integration action catalogue live)" ); } // Use the expiry-tolerant read for the fallback: a transient blip that @@ -190,9 +190,9 @@ pub(super) async fn resolve_target_agent(channel: &str) -> AgentScoping { /// /// Only an `Authoritative` result (the backend explicitly reporting the current /// set, even if empty) is taken at face value. `Unavailable` or a timeout falls -/// back to `cached`, so a one-off 5xx/slow call can't drop -/// `delegate_to_integrations_agent` and silently disable tool calling for the -/// turn (the "just normal inference" bug). With no cache to fall back on the +/// back to `cached`, so a one-off 5xx/slow call can't drop the integration +/// action catalogue and silently disable tool calling for the turn (the +/// "just normal inference" bug). With no cache to fall back on the /// result is empty — the same conservative default as before, but reached only /// when we genuinely have no better truth. pub(super) fn connected_with_fallback( @@ -211,8 +211,8 @@ pub(super) fn connected_with_fallback( /// * every tool name in the agent's `[tools] named = [...]` list /// (when the scope is [`ToolScope::Named`]); and /// * every name produced by the per-turn synthesised delegation tools -/// in `extra_tools` (e.g. `research`, `plan`, -/// `delegate_to_integrations_agent`). +/// in `extra_tools` (e.g. `research`, `plan`) and the deferred +/// integration action tools beside them. /// /// When the agent's tool scope is [`ToolScope::Wildcard`] **and** there /// are no `extra_tools`, returns `None` to preserve the legacy diff --git a/crates/openhuman-core/src/integrations/composio/connected_integrations/cache.rs b/crates/openhuman-core/src/integrations/composio/connected_integrations/cache.rs index 941d71f7ac..0b75e6453d 100644 --- a/crates/openhuman-core/src/integrations/composio/connected_integrations/cache.rs +++ b/crates/openhuman-core/src/integrations/composio/connected_integrations/cache.rs @@ -120,8 +120,8 @@ pub fn cached_active_integrations(config: &Config) -> Optionx", Some(""))); + assert!(is_html( + "x", + Some("") + )); } #[test] @@ -156,7 +159,9 @@ fn the_schema_offers_the_raw_escape_hatch() { #[test] fn the_declared_cap_is_sized_for_extracted_markdown_not_raw_markup() { let tool = WebFetchTool::new(Arc::new(SecurityPolicy::default()), vec![], None, None); - let cap = tool.max_result_size_chars().expect("web_fetch declares a cap"); + let cap = tool + .max_result_size_chars() + .expect("web_fetch declares a cap"); assert!( (8_000..=32_000).contains(&cap), "cap should sit in the same range as Hermes (15k chars) and Codex \ diff --git a/crates/openhuman-core/src/tools/orchestrator_tools.rs b/crates/openhuman-core/src/tools/orchestrator_tools.rs index 19d4934a52..1f50a01073 100644 --- a/crates/openhuman-core/src/tools/orchestrator_tools.rs +++ b/crates/openhuman-core/src/tools/orchestrator_tools.rs @@ -8,18 +8,20 @@ //! contains discoverable, well-named tools like `research`, `plan`, //! `run_code`, etc. //! -//! For [`SubagentEntry::Skills`] wildcard expansions (#1335) we synthesise -//! a single collapsed `delegate_to_integrations_agent` tool that takes the -//! toolkit slug as an argument — keeping the orchestrator's schema cost -//! constant in the integration dimension instead of scaling with the -//! number of connected toolkits. +//! For [`SubagentEntry::Skills`] wildcard expansions we synthesise one +//! `ToolExposure::Deferred` [`ComposioActionTool`] per action of every +//! connected Composio toolkit. Those never reach the wire: a belt that opted +//! into discovery (`tool_search` in its `[tools] named`) finds them through +//! the harness's `tool_search` bridge and calls them directly, so "send this +//! email" is one search and one call. There is no `delegate_to_integrations_ +//! agent` any more — routing a single action through a sub-agent spawn cost a +//! blocking agentic round-trip and a second prompt for work the parent could +//! do in one call. //! -//! Each synthesised tool's description is pulled live from the target -//! agent's [`AgentDefinition::when_to_use`] (for -//! [`SubagentEntry::AgentId`]) or from the connected Composio toolkit -//! metadata (for [`SubagentEntry::Skills`] wildcard expansions) — so -//! descriptions automatically stay in sync with the definitions and -//! never drift from a hardcoded table. +//! Each synthesised delegation tool's description is pulled live from the +//! target agent's [`AgentDefinition::when_to_use`] — so descriptions +//! automatically stay in sync with the definitions and never drift from a +//! hardcoded table. //! //! Called from [`crate::agent::session_host::builder`] at //! agent-build time, with the orchestrator's own definition, the global @@ -36,9 +38,9 @@ use crate::integrations::composio::ComposioActionTool; // SpawnWorkerThreadTool import kept commented while the worker-thread spawn is // temporarily disabled (see tinyhumansai/openhuman#1624). +use super::ArchetypeDelegationTool; #[allow(unused_imports)] use super::SpawnWorkerThreadTool; -use super::{ArchetypeDelegationTool, SkillDelegationTool}; use crate::agent::orchestration::tools::DelegationTarget; use tinytools::Tool; @@ -52,23 +54,17 @@ use tinytools::Tool; /// `when_to_use` — so editing an agent's TOML description immediately /// updates the tool schema the orchestrator LLM sees, with zero drift. /// -/// Each [`SubagentEntry::Skills`] wildcard expands to a single -/// collapsed [`SkillDelegationTool`] named -/// `delegate_to_integrations_agent` whose `toolkit` argument selects -/// among the slugs of every connected Composio integration in -/// `connected_integrations`. The tool routes to the generic -/// `integrations_agent` with the chosen toolkit's slug passed as -/// `skill_filter`. The collapsed form keeps the orchestrator's -/// function-calling schema constant in the integration dimension -/// (#1335). +/// Each [`SubagentEntry::Skills`] wildcard expands to the connected +/// integrations' actions as `Deferred` tools +/// ([`collect_deferred_integration_actions`]): off the wire, reachable +/// through the harness's `tool_search`, and callable directly by the agent +/// that found them. No delegation tool is synthesised for the wildcard. /// /// Entries that reference unknown agent ids (not in the registry) are /// logged at `warn` and skipped — the orchestrator still builds, just -/// without the broken delegation. Entries that reference Skills wildcards -/// with an empty `connected_integrations` slice produce zero tools, which -/// is the correct behaviour when the user has not yet connected any -/// integrations (the LLM should not see a `delegate_to_integrations_agent` -/// tool with an empty enum). +/// without the broken delegation. A Skills wildcard with an empty +/// `connected_integrations` slice produces zero tools, which is the correct +/// behaviour when the user has not yet connected any integrations. /// /// Returns an empty Vec when `definition.subagents` is empty — callers /// (notably the builder) handle this by not extending the visible-tool @@ -147,109 +143,17 @@ pub fn collect_orchestrator_tools( ); continue; } - // Collapsed delegation tool (#1335). Previously this loop - // emitted one `delegate_` tool per connected - // integration. Every one of those tools dispatched to the - // same `integrations_agent` with a different `skill_filter`, - // so the fan-out cost the orchestrator schema bytes without - // buying any new routing capability. We now emit at most - // one `delegate_to_integrations_agent` tool that takes the - // toolkit slug as an argument; the description enumerates - // the connected toolkits so the orchestrator still - // discovers which integrations are routable. - // `sanitise_slug` is lossy — `Slack.Bot` and `Slack-Bot` - // both collapse to `slack_bot`. Once the raw id is - // discarded, one upstream integration would silently - // shadow the other. Detect the collision here, drop - // every duplicate after the first, and warn so routing - // stays unambiguous (the first arrival keeps the slug; - // later arrivals are unreachable through this enum and - // safer to omit than silently re-target). - let mut connected: Vec<(String, String)> = Vec::new(); - let mut seen: std::collections::HashSet = std::collections::HashSet::new(); - for integration in connected_integrations { - if !integration.connected { - log::debug!( - "[orchestrator_tools] skipping unconnected integration: {}", - integration.toolkit - ); - continue; - } - // Slug the toolkit name into a tool-name-safe - // (and argument-safe) form so the LLM-facing - // enum stays predictable across odd toolkit - // names (dashes, dots, spaces, mixed case). - let slug = sanitise_slug(&integration.toolkit); - if !seen.insert(slug.clone()) { - log::warn!( - "[orchestrator_tools] duplicate sanitised slug '{slug}' from raw \ - toolkit '{raw}' — dropping to keep collapsed delegation routing \ - unambiguous", - raw = integration.toolkit - ); - continue; - } - // Empty integration descriptions otherwise render as a - // bare ` - slug` line in the collapsed tool description, - // which gives the orchestrator LLM no hint about what - // the toolkit actually does. Fall back to the - // generic per-toolkit phrasing the old fan-out path - // used so brand-new or under-populated toolkits stay - // informative. - let description = if integration.description.trim().is_empty() { - format!( - "External integration via {} — see the toolkit docs for available actions.", - integration.toolkit - ) - } else { - integration.description.clone() - }; - connected.push((slug, description)); - } - // Order the enum by slug, because the order it arrives in is - // not a contract and the order it is *advertised* in is. - // - // This tool's schema and description both enumerate the - // toolkits, and the tool block is rendered ahead of the - // conversation in every provider's cached prefix — so a - // backend that returns the same integrations in a different - // order would otherwise re-write the schema, and with it - // invalidate the whole prefix including the system prompt the - // turn loop freezes for exactly that reason. The rest of the - // pipeline already treats order as meaningless: - // `connected_set_hash` sorts before hashing, which is what - // stops a reordering from reaching a reconcile at the turn - // boundary in the first place. Sorting here makes the - // advertised surface agree with that view instead of - // contradicting it a layer down. - // - // Sorting AFTER the dedup loop, never before: the collision - // rule above is "the first arrival keeps the slug", which is a - // statement about arrival order and would change meaning if the - // list were sorted first. - connected.sort_by(|(a, _), (b, _)| a.cmp(b)); - match SkillDelegationTool::for_connected(connected) { - Some(tool) => { - log::debug!( - "[orchestrator_tools] registering collapsed integrations delegation tool ({} toolkits)", - tool.connected_toolkits.len() - ); - tools.push(Box::new(tool)); - } - None => { - log::debug!( - "[orchestrator_tools] no connected integrations — collapsed delegation tool omitted" - ); - } - } - // The same toolkits' actions, one `Deferred` tool each. Never - // on the wire: a belt that opted into discovery reaches them - // through the harness's `tool_search`, so one clear action is - // a search and a call rather than an `integrations_agent` - // run. A belt that did not opt in never sees them — the - // session builder leaves them prompt-hidden, which the - // direct-call gate refuses. Approval and channel permission - // apply per call exactly as on the sub-agent path. + // The connected toolkits' actions, one `Deferred` tool each. + // Never on the wire: a belt that opted into discovery reaches + // them through the harness's `tool_search`, so one clear + // action is a search and a call. A belt that did not opt in + // never sees them — the session builder leaves them + // prompt-hidden, which the direct-call gate refuses. Approval + // and channel permission apply per call. This used to sit + // beside a collapsed `delegate_to_integrations_agent` tool + // that spawned `integrations_agent` per toolkit; with the + // actions searchable that spawn only added a blocking + // sub-agent round-trip, so the delegation tool is gone. let actions = collect_deferred_integration_actions(connected_integrations); if !actions.is_empty() { log::debug!( @@ -280,7 +184,7 @@ pub fn collect_orchestrator_tools( /// Gated actions are left out: the model cannot call them and the prompt's /// Connected Integrations section already explains how to unlock them. /// A collision on an action slug across two toolkits keeps the first -/// arrival, like `sanitise_slug` collisions above. +/// arrival. pub fn collect_deferred_integration_actions( connected_integrations: &[ConnectedIntegration], ) -> Vec> { @@ -315,9 +219,8 @@ pub fn collect_deferred_integration_actions( /// an underscore. OpenAI-style function names only accept /// `[a-zA-Z0-9_-]{1,64}`, so this is the conservative subset. /// -/// Used both when synthesising `delegate_*` tools and when rendering the -/// delegation guide in prompts — they must agree on slug canonicalisation -/// so the prompt always references a tool name that actually exists. +/// Used when rendering integration slugs in prompts so the prompt and any +/// argument-facing enum agree on slug canonicalisation. pub(crate) fn sanitise_slug(raw: &str) -> String { raw.chars() .map(|c| { diff --git a/crates/openhuman-core/src/tools/orchestrator_tools_tests.rs b/crates/openhuman-core/src/tools/orchestrator_tools_tests.rs index a78d896325..0a797dc579 100644 --- a/crates/openhuman-core/src/tools/orchestrator_tools_tests.rs +++ b/crates/openhuman-core/src/tools/orchestrator_tools_tests.rs @@ -79,18 +79,44 @@ fn integration(toolkit: &str, description: &str) -> ConnectedIntegration { } } +fn integration_with_actions( + toolkit: &str, + description: &str, + actions: &[&str], +) -> ConnectedIntegration { + let mut ci = integration(toolkit, description); + ci.tools = actions + .iter() + .map(|name| crate::agent::prompts::ConnectedIntegrationTool { + name: (*name).to_string(), + description: format!("{name} action"), + parameters: None, + }) + .collect(); + ci +} + /// Baseline: an orchestrator with 2 AgentId entries + a Skills /// wildcard, against a registry that knows both targets and a /// connected_integrations list with three toolkits, should produce -/// 2 archetype tools + 1 collapsed integrations delegation tool -/// (#1335) — independent of how many integrations are connected. +/// 2 archetype tools plus one `Deferred` action tool per connected +/// action — and no `delegate_to_integrations_agent`. One clear service +/// action is a `tool_search` and a call, never a sub-agent spawn. #[test] -fn collects_agentid_entries_and_collapses_skills_wildcard() { +fn collects_agentid_entries_and_expands_skills_wildcard_to_deferred_actions() { let orch = sample_orchestrator(); let reg = registry_with_targets(); let integrations = vec![ - integration("gmail", "Send and read email via Gmail."), - integration("github", "Manage repos, issues, and pull requests."), + integration_with_actions( + "gmail", + "Send and read email via Gmail.", + &["GMAIL_SEND_EMAIL", "GMAIL_FETCH_EMAILS"], + ), + integration_with_actions( + "github", + "Manage repos, issues, and pull requests.", + &["GITHUB_CREATE_ISSUE"], + ), integration("notion", "Read and write pages and databases."), ]; @@ -106,9 +132,16 @@ fn collects_agentid_entries_and_collapses_skills_wildcard() { // restored. "research", // researcher's delegate_name override "delegate_archivist", // archivist has no delegate_name → default - "delegate_to_integrations_agent", + // Actions sorted by toolkit, then action name. + "GITHUB_CREATE_ISSUE", + "GMAIL_FETCH_EMAILS", + "GMAIL_SEND_EMAIL", ], - "skills wildcard must collapse to a single delegate_to_integrations_agent tool" + "skills wildcard must expand to the connected actions, not a delegation tool" + ); + assert!( + !names.iter().any(|name| name.starts_with("delegate_to_")), + "no integrations delegation tool may be synthesised" ); // Archetype tool descriptions come from `when_to_use`. @@ -118,47 +151,58 @@ fn collects_agentid_entries_and_collapses_skills_wildcard() { "delegate description is the target's when_to_use" ); - // The collapsed delegation tool enumerates every connected toolkit - // in its description so the orchestrator still discovers what's - // routable. - let delegate_tool = tools - .iter() - .find(|t| t.name() == "delegate_to_integrations_agent") - .unwrap(); - let desc = delegate_tool.description(); - assert!(desc.contains("gmail")); - assert!(desc.contains("github")); - assert!(desc.contains("notion")); + // Every action is `Deferred`: off the wire, reachable through + // `tool_search`. (The archetype delegates are `Hidden` — the collapsed + // `delegate_to` tool advertises them — so only the actions are checked.) + for tool in tools.iter().filter(|t| t.name().starts_with("G")) { + assert_eq!( + tool.exposure(), + tinytools::ToolExposure::Deferred, + "exposure of {}", + tool.name() + ); + } } -/// The collapsed delegation tool's count is constant in the -/// integration dimension (#1335 primary acceptance criterion). +/// The synthesised set scales only with the connected *actions*, never +/// adds a per-toolkit or collapsed delegation handle. #[test] -fn collapsed_delegation_tool_count_is_constant_across_integration_counts() { +fn skills_wildcard_adds_no_delegation_tool_for_any_integration_count() { let orch = sample_orchestrator(); let reg = registry_with_targets(); for n in [1usize, 3, 7, 20] { let integrations: Vec<_> = (0..n) - .map(|i| integration(&format!("tool{i}"), &format!("Toolkit number {i}."))) + .map(|i| { + integration_with_actions( + &format!("tool{i}"), + &format!("Toolkit number {i}."), + &[&format!("TOOL{i}_ACT")], + ) + }) .collect(); let tools = collect_orchestrator_tools(&orch, ®, &integrations); let delegation_count = tools .iter() - .filter(|t| t.name() == "delegate_to_integrations_agent") + .filter(|t| t.name().starts_with("delegate_to_")) .count(); assert_eq!( - delegation_count, 1, - "expected exactly one collapsed delegation tool for {n} integrations" + delegation_count, 0, + "no integrations delegate for {n} integrations" ); + let action_count = tools + .iter() + .filter(|t| t.exposure() == tinytools::ToolExposure::Deferred) + .count(); + assert_eq!(action_count, n, "one deferred action per connected action"); } } /// An orchestrator with a Skills wildcard but no connected -/// integrations should produce zero integrations delegation tools — -/// the LLM must not be shown a routing handle for an empty set. +/// integrations should produce zero integration tools — nothing to +/// search for, nothing to advertise. #[test] -fn skills_wildcard_with_no_integrations_produces_no_delegation_tool() { +fn skills_wildcard_with_no_integrations_produces_no_integration_tools() { let orch = sample_orchestrator(); let reg = registry_with_targets(); let tools = collect_orchestrator_tools(&orch, ®, &[]); @@ -270,148 +314,81 @@ fn sanitise_slug_lowercases_and_replaces_invalid_chars() { assert_eq!(sanitise_slug("weird name!"), "weird_name_"); } -/// Unconnected integrations must be silently dropped from the -/// collapsed delegation tool's enum. Otherwise the orchestrator -/// could supply `toolkit = ""` and trigger a pre-flight -/// rejection downstream that says "not connected". +/// Unconnected integrations contribute no actions: the orchestrator +/// must not find (and call) an action on a toolkit the user has not +/// authorised and hit a "not connected" rejection downstream. #[test] -fn unconnected_integrations_are_omitted_from_collapsed_tool() { +fn unconnected_integrations_contribute_no_actions() { let orch = sample_orchestrator(); let reg = registry_with_targets(); let integrations = vec![ - integration("gmail", "Send and read email."), + integration_with_actions("gmail", "Send and read email.", &["GMAIL_SEND_EMAIL"]), ConnectedIntegration { toolkit: "github".into(), description: "GitHub access.".into(), - tools: vec![], + tools: vec![crate::agent::prompts::ConnectedIntegrationTool { + name: "GITHUB_CREATE_ISSUE".into(), + description: "Create an issue".into(), + parameters: None, + }], gated_tools: vec![], - connected: false, // not connected — must not appear in the enum + connected: false, // not connected — its actions must not appear connections: Vec::new(), non_active_status: None, }, - integration("notion", "Read and write pages."), + integration_with_actions("notion", "Read and write pages.", &["NOTION_CREATE_PAGE"]), ]; let tools = collect_orchestrator_tools(&orch, ®, &integrations); - let delegate_tool = tools - .iter() - .find(|t| t.name() == "delegate_to_integrations_agent") - .expect("collapsed delegation tool must exist when at least one integration is connected"); - let desc = delegate_tool.description(); - assert!(desc.contains("gmail")); - assert!(desc.contains("notion")); + let names: Vec<&str> = tools.iter().map(|t| t.name()).collect(); + assert!(names.contains(&"GMAIL_SEND_EMAIL")); + assert!(names.contains(&"NOTION_CREATE_PAGE")); assert!( - !desc.contains("github"), - "unconnected github must not leak into the delegation tool description" + !names.contains(&"GITHUB_CREATE_ISSUE"), + "unconnected github must not leak an action into the catalogue" ); - - let schema = delegate_tool.parameters_schema(); - let enum_vals = schema["properties"]["toolkit"]["enum"] - .as_array() - .expect("toolkit enum must be present"); - let slugs: Vec<&str> = enum_vals.iter().map(|v| v.as_str().unwrap()).collect(); - assert_eq!(slugs, vec!["gmail", "notion"]); } -/// Quirky toolkit slugs (dashes, mixed case) must be canonicalised -/// before they land in the collapsed tool's enum so the -/// LLM-provided argument can be matched with `==` rather than a -/// fuzzy comparison. +/// Actions are advertised in a stable order — toolkit, then action — +/// whatever order the backend listed the connections in, because the +/// synthesised set feeds the tool specs a session freezes. #[test] -fn collapsed_tool_enum_uses_sanitised_slugs() { +fn deferred_actions_are_sorted_by_toolkit_then_action() { let mut orch = def("orchestrator", "t", None); orch.subagents = vec![SubagentEntry::Skills(SkillsWildcard { skills: "*".into() })]; let reg = registry_with_targets(); let integrations = vec![ - integration("Google-Calendar", "Calendar."), - integration("Slack.Bot", "Chat."), + integration_with_actions( + "slack", + "Chat.", + &["SLACK_SEND_MESSAGE", "SLACK_LIST_CHANNELS"], + ), + integration_with_actions("gmail", "Email.", &["GMAIL_SEND_EMAIL"]), ]; let tools = collect_orchestrator_tools(&orch, ®, &integrations); - let delegate_tool = tools - .iter() - .find(|t| t.name() == "delegate_to_integrations_agent") - .expect("collapsed tool present"); - let schema = delegate_tool.parameters_schema(); - let enum_vals = schema["properties"]["toolkit"]["enum"].as_array().unwrap(); - let slugs: Vec<&str> = enum_vals.iter().map(|v| v.as_str().unwrap()).collect(); - assert_eq!(slugs, vec!["google_calendar", "slack_bot"]); -} - -/// An integration with an empty description must not render as a -/// bare ` - slug` line in the collapsed tool description — the -/// orchestrator LLM would have no signal about what the toolkit -/// does. The synthesiser falls back to a generic descriptive -/// phrase keyed on the raw toolkit name. -#[test] -fn empty_integration_description_falls_back_to_generic_label() { - let mut orch = def("orchestrator", "t", None); - orch.subagents = vec![SubagentEntry::Skills(SkillsWildcard { skills: "*".into() })]; - let reg = registry_with_targets(); - let integrations = vec![ - ConnectedIntegration { - toolkit: "Brand.New".into(), - description: " ".into(), - tools: vec![], - gated_tools: vec![], - connected: true, - connections: Vec::new(), - non_active_status: None, - }, - integration("gmail", "Email."), - ]; - let tools = collect_orchestrator_tools(&orch, ®, &integrations); - let delegate_tool = tools - .iter() - .find(|t| t.name() == "delegate_to_integrations_agent") - .expect("collapsed tool present"); - let desc = delegate_tool.description(); - assert!( - desc.contains("External integration via Brand.New"), - "expected fallback phrasing, got: {desc}" + let names: Vec<&str> = tools.iter().map(|t| t.name()).collect(); + assert_eq!( + names, + vec![ + "GMAIL_SEND_EMAIL", + "SLACK_LIST_CHANNELS", + "SLACK_SEND_MESSAGE" + ] ); - assert!(desc.contains("Email.")); } -/// Two upstream toolkits whose names sanitise to the same slug -/// must not silently both land in the collapsed enum — the second -/// arrival is dropped (with a warn log) so the orchestrator's -/// routing handle stays unambiguous. Without this guard, -/// `Slack.Bot` and `Slack-Bot` would both render as `slack_bot` -/// in the enum and the orchestrator could no longer distinguish -/// them. +/// The same action slug arriving from two toolkits keeps the first +/// arrival (by sorted toolkit) so the catalogue never carries two tools +/// under one name. #[test] -fn duplicate_sanitised_slug_drops_later_collisions() { +fn duplicate_action_names_keep_the_first_arrival() { let mut orch = def("orchestrator", "t", None); orch.subagents = vec![SubagentEntry::Skills(SkillsWildcard { skills: "*".into() })]; let reg = registry_with_targets(); let integrations = vec![ - integration("Slack.Bot", "First slack."), - integration("Slack-Bot", "Second slack — must be dropped."), - integration("Notion", "Pages."), + integration_with_actions("slack", "Chat.", &["SHARED_ACTION"]), + integration_with_actions("gmail", "Email.", &["SHARED_ACTION", "GMAIL_SEND_EMAIL"]), ]; let tools = collect_orchestrator_tools(&orch, ®, &integrations); - let delegate_tool = tools - .iter() - .find(|t| t.name() == "delegate_to_integrations_agent") - .expect("collapsed tool present"); - let schema = delegate_tool.parameters_schema(); - let enum_vals = schema["properties"]["toolkit"]["enum"].as_array().unwrap(); - let slugs: Vec<&str> = enum_vals.iter().map(|v| v.as_str().unwrap()).collect(); - // Sorted, not arrival order: the enum is advertised in a cached prefix, so - // its order is fixed by slug rather than by however the backend happened to - // list the connections (see `collect_orchestrator_tools`). The collision - // rule this test is actually about is unaffected — "first arrival keeps the - // slug" is decided before the sort, and the description assertions below - // are what pin which of the two Slacks won. - assert_eq!( - slugs, - vec!["notion", "slack_bot"], - "second slack_bot collision must be dropped, not silently shadowed, \ - and the surviving slugs must be advertised in sorted order" - ); - // The dropped description must not appear in the tool description - // either — otherwise the orchestrator would think there's a route - // it can't actually distinguish. - let desc = delegate_tool.description(); - assert!(desc.contains("First slack.")); - assert!(!desc.contains("Second slack")); + let names: Vec<&str> = tools.iter().map(|t| t.name()).collect(); + assert_eq!(names, vec!["GMAIL_SEND_EMAIL", "SHARED_ACTION"]); } diff --git a/docs/plans/jev-tool-search-baseline.md b/docs/plans/jev-tool-search-baseline.md index 578d1bb23b..2f36c1eaee 100644 --- a/docs/plans/jev-tool-search-baseline.md +++ b/docs/plans/jev-tool-search-baseline.md @@ -13,7 +13,9 @@ answer. Before this work the orchestrator reached a Composio action only through `delegate_to_integrations_agent` → an `integrations_agent` sub-run whose -toolkit was narrowed by `rank_tools_by_prompt` (the `overlap` row). Every +toolkit was narrowed by `rank_tools_by_prompt` (the `overlap` row). That +delegate has since been removed from the orchestrator: search-then-call is +its only route to an integration action. Every `tool_search` row is one search followed by a direct call of the tool it returns; no sub-agent. diff --git a/docs/prompt-evals.md b/docs/prompt-evals.md index 8579b52623..7b99452671 100644 --- a/docs/prompt-evals.md +++ b/docs/prompt-evals.md @@ -26,8 +26,9 @@ A green tier 1 is therefore not evidence of comprehension. Only tier 2 is. The six cases cover the agents where mis-routing has cost something: `workflow_builder` (must reach `propose_workflow`, never three consecutive -catalog searches), `orchestrator` (hands integration work off, never holds the -raw Composio or cron tools), `integrations_agent`, `scheduler_agent`, and the +catalog searches), `orchestrator` (searches for and calls integration actions +itself, never holds the raw Composio or cron tools), `integrations_agent`, +`scheduler_agent`, and the two zero-belt agents `summarizer` and `trigger_triage` (advertise nothing). Fleet-wide static coverage of all agents lives in the prompt tests under `crates/openhuman-core/src/agent/registry/agents/`, not here. @@ -130,7 +131,7 @@ scoring proves too coarse. | `orchestrator-reminder` | orchestration → scheduler | hands off through `schedule_task` | **a cron job**, remove it afterwards | | `orchestrator-direct-answer` | orchestration | answers a trivial question without spawning | nothing | | `orchestrator-research-trip` | orchestration | a research question goes straight to `web_search_tool` and streams an answer; never `request_plan_review`, `todo` or a spawn | nothing | -| `composio-gmail-read` | composio | reads the latest Gmail subject via `delegate_to_integrations_agent`; never sends, deletes or reconnects | nothing | +| `composio-gmail-read` | composio | reads the latest Gmail subject via `tool_search` and a direct `GMAIL_*` call; never sends, deletes or reconnects | nothing | | `skill-notion-read` | skills | lists Notion pages through `run_skill`; never installs a skill | nothing | | `mcp-none-configured` | MCP, **error path** | with no MCP server configured, says so; never installs one, never fabricates results | nothing | | `web-search-fact` | web search | one built-in `web_search_tool` lookup, not a `research` spawn | nothing | @@ -149,10 +150,11 @@ MCP server to make MCP "testable"; that changes the baseline being measured. Cases run in file order, by increasing account risk. Cases that touch nothing run first, because they also validate the rig on real inference; the only writing case (`orchestrator-reminder`) runs last. -**`composio-gmail-read` is disabled by default.** The toolkit-scoped `integrations_agent` -runs in text mode, so its calls may not reach `calls` in the same shape as -native tool calls. Until a real transcript has shown one landing in a form its -`GMAIL_*` forbids match, those forbids are unproven. They would fail to notice +**`composio-gmail-read` is disabled by default.** The orchestrator finds the +`GMAIL_*` action through `tool_search` and calls it directly; that call may +reach `calls` under the action name or through the harness's `tool_call` +bridge. Until a real transcript has shown one landing in a form its `GMAIL_*` +forbids match, those forbids are unproven. They would fail to notice a send, not prevent it. Run the earlier cases, inspect a real `calls` field, and only then run it. Do not repair the matcher mid-run. The runner skips this case unless `--allow-disabled` is explicitly supplied after matcher validation. diff --git a/gitbooks/developing/architecture/agent-harness.md b/gitbooks/developing/architecture/agent-harness.md index ace546bac9..321f66c360 100644 --- a/gitbooks/developing/architecture/agent-harness.md +++ b/gitbooks/developing/architecture/agent-harness.md @@ -344,7 +344,7 @@ Each archetype lives under `agents//` with an `agent.toml` (metadata, tool | `archivist` | Memory distillation - what to persist, what to forget. | | `tool_maker` | Self-healing - writes polyfills for missing shell commands. | | `tools_agent` | Generic specialist for arbitrary tool-bound tasks. | -| `integrations_agent` | Bound to a specific Composio toolkit (Gmail, GitHub, Slack…) for that toolkit's actions. | +| `integrations_agent` | Bound to a specific Composio toolkit (Gmail, GitHub, Slack…) for that toolkit's actions. Not reachable from chat: the orchestrator finds a connected action through `tool_search` and calls it directly. | | `trigger_triage` | Classifies incoming external events into drop / notify / spawn-reactor / spawn-agent. | | `trigger_reactor` | Lightweight reaction to a triaged trigger that doesn't need a full orchestrator turn. | | `morning_briefing` | Curated daily digest run by cron. | @@ -399,7 +399,7 @@ Each `AgentDefinition` carries an `agent_tier` field (`chat` / `reasoning` / `wo | `reasoning` | `worker` | another `reasoning`, any `chat` | `planner` (today the canonical one) | | `worker` | nothing[^1] | anything | researcher, code_executor, critic, archivist, tool_maker, integrations_agent, … | -[^1]: Skill-wildcard entries (`{ skills = "*" }`) are exempt because they collapse to a single `delegate_to_integrations_agent` tool whose target is a worker; they're a fan-out delegation surface, not a recursive spawn. +[^1]: Skill-wildcard entries (`{ skills = "*" }`) are exempt because they name no agent: they expand to the connected Composio actions as `Deferred` tools the agent reaches through `tool_search`, not to a spawn. **Why the rules.** diff --git a/scripts/prompt-eval/cases.json b/scripts/prompt-eval/cases.json index d1c53d2511..6de529d889 100644 --- a/scripts/prompt-eval/cases.json +++ b/scripts/prompt-eval/cases.json @@ -145,13 +145,14 @@ "id": "composio-gmail-read", "enabled": false, "surface": "composio", - "_why": "Integration hand-off to a connected toolkit, read-only. Exercises delegate_to_integrations_agent and the text-mode integrations_agent (see #6334).", + "_why": "Read-only action on a connected toolkit. Exercises the orchestrator's tool_search → direct GMAIL_* call route (the delegate_to_integrations_agent hand-off is gone; see #6334 for the earlier shape).", "entry": "agent_chat", "message": "What is the subject line of the most recent email in my Gmail inbox? Just read it, don't change anything.", "expect_calls": [ - "delegate_to_integrations_agent" + "tool_search" ], "forbid_calls": [ + "delegate_to_integrations_agent", "composio_connect", "GMAIL_SEND_EMAIL", "GMAIL_DELETE_MESSAGE", @@ -169,7 +170,7 @@ "params": {}, "expect_regex": "(?s)\\{[^{}]*\"status\"\\s*:\\s*\"ACTIVE\"[^{}]*\"toolkit\"\\s*:\\s*\"gmail\"[^{}]*\\}|\\{[^{}]*\"toolkit\"\\s*:\\s*\"gmail\"[^{}]*\"status\"\\s*:\\s*\"ACTIVE\"[^{}]*\\}" }, - "_gate": "Do not run until a real transcript has shown a text-mode integrations_agent call in `calls` in a form these forbids match. Until then GMAIL_* forbids are unproven against the live inbox." + "_gate": "Do not run until a real transcript has shown the orchestrator's direct GMAIL_* call in `calls` in a form these forbids match. Until then GMAIL_* forbids are unproven against the live inbox." }, { "id": "orchestrator-reminder", diff --git a/tests/agent_prompt_comprehension_e2e.rs b/tests/agent_prompt_comprehension_e2e.rs index 6592a6e727..92f2bd0d56 100644 --- a/tests/agent_prompt_comprehension_e2e.rs +++ b/tests/agent_prompt_comprehension_e2e.rs @@ -287,9 +287,9 @@ async fn current_user(_headers: HeaderMap) -> Json { Json(json!({ "success": true, "data": { "_id": "e2e-user-1", "username": "e2e" } })) } -/// One connected Gmail toolkit, so the orchestrator is offered -/// `delegate_to_integrations_agent` and the integrations agent has a toolkit to -/// bind to. Shapes from `tools_approval_channels_raw_coverage_e2e.rs`. +/// One connected Gmail toolkit, so the orchestrator gets its actions as a +/// searchable catalogue and the integrations agent has a toolkit to bind to. +/// Shapes from `tools_approval_channels_raw_coverage_e2e.rs`. async fn composio_toolkits() -> Json { Json(json!({ "success": true, "data": { "toolkits": ["gmail"] } })) } @@ -849,38 +849,43 @@ fn workflow_builder_reaches_propose_workflow() { }); } -/// The orchestrator routes integration work through the hand-off, and never -/// holds the raw Composio or cron tools its specialists own. +/// The orchestrator reaches an integration action by searching for it and +/// calling it directly — no integrations sub-agent — and never holds the raw +/// Composio or cron tools its specialists own. The action itself is +/// `Deferred`: off the advertised belt, found through `tool_search`. #[test] -#[ignore = "TODO(#6376): hosted TinyAgents omits integration delegation tools"] -fn orchestrator_hands_integration_work_to_the_specialist() { +#[ignore = "TODO(#6376): hosted TinyAgents omits the deferred integration catalogue"] +fn orchestrator_searches_for_and_calls_the_integration_action() { run_case(Case { agent: "orchestrator", agent_marker: "## How you work", entry: Entry::WebChat, user_message: "Check my Gmail for anything from my landlord.", scripted_completions: vec![ - call( - "delegate_to_integrations_agent", - json!({ "toolkit": "gmail", "prompt": "Find emails from my landlord." }), - ), - text_completion("No emails from your landlord."), + call("tool_search", json!({ "query": "fetch gmail emails" })), + call("GMAIL_FETCH_EMAILS", json!({ "query": "from:landlord" })), text_completion("You have no emails from your landlord."), ], - must_call: &["delegate_to_integrations_agent"], - must_not_call: &["composio_execute"], + must_call: &["tool_search", "GMAIL_FETCH_EMAILS"], + must_not_call: &["composio_execute", "delegate_to_integrations_agent"], // Not `schedule_task`: it resolves when called (see the scheduler case) // but a named agent's up-front belt does not list synthesised delegates. - must_advertise: &["delegate_to_integrations_agent", "research"], - must_not_advertise: &["composio_execute", "composio_list_tools", "cron_add"], + must_advertise: &["tool_search", "research"], + must_not_advertise: &[ + "delegate_to_integrations_agent", + "composio_execute", + "composio_list_tools", + "cron_add", + ], advertises_nothing: false, max_consecutive_calls_of: None, extra_config: "", }); } -/// The integrations specialist, reached through that hand-off, holds the -/// Composio execution surface and none of the orchestrator's hand-offs. +/// The integrations specialist (still spawnable by the runner with a toolkit, +/// no longer reachable from chat) holds the Composio execution surface and +/// none of the orchestrator's hand-offs. /// /// The toolkit-scoped integrations agent runs in text mode, so this also pins /// the text-mode `Call as: NAME[...]` catalogue rather than only native tool @@ -894,10 +899,6 @@ fn integrations_agent_holds_the_composio_surface() { entry: Entry::WebChat, user_message: "Check my Gmail for anything from my landlord.", scripted_completions: vec![ - call( - "delegate_to_integrations_agent", - json!({ "toolkit": "gmail", "prompt": "Find emails from my landlord." }), - ), // The child runs in text mode, so its own calls would be // `` text with parser-assigned ids; this case pins its // belt only. @@ -907,7 +908,7 @@ fn integrations_agent_holds_the_composio_surface() { must_call: &[], must_not_call: &[], must_advertise: &["composio_execute", "composio_list_tools"], - must_not_advertise: &["delegate_to_integrations_agent", "schedule_task", "shell"], + must_not_advertise: &["research", "schedule_task", "shell"], advertises_nothing: false, max_consecutive_calls_of: None, extra_config: "", diff --git a/tests/composio_list_tools_stack_overflow_regression.rs b/tests/composio_list_tools_stack_overflow_regression.rs index 3c651921b9..4d99c3f6ee 100644 --- a/tests/composio_list_tools_stack_overflow_regression.rs +++ b/tests/composio_list_tools_stack_overflow_regression.rs @@ -4,7 +4,10 @@ //! //! While a user chat went through //! `web_channel → orchestrator turn → delegate_to_integrations_agent -//! → integrations_agent → composio_list_tools`, the in-process core +//! → integrations_agent → composio_list_tools` (the orchestrator has since +//! stopped spawning `integrations_agent` — it searches for and calls the +//! action itself — but the sub-agent runner path below is unchanged), the +//! in-process core //! aborted with `EXC_BAD_ACCESS (SIGBUS) — KERN_PROTECTION_FAILURE` //! at an address inside the **stack guard page** of a `tokio-rt-worker` //! thread. That's a stack overflow — not a Rust panic. The kernel @@ -20,7 +23,7 @@ //! ← config::ops::load_config_with_timeout //! ← ComposioListToolsTool::execute //! ← subagent_runner::run_inner_loop / run_typed_mode / run_subagent -//! ← SkillDelegationTool::execute (delegate_to_integrations_agent) +//! ← SkillDelegationTool::execute (delegate_to_integrations_agent, since removed) //! ← Agent::execute_tool_call / execute_tools / turn //! ← web_chat::run_chat_task //! ``` @@ -51,11 +54,10 @@ //! //! Faithful reproduction in cargo-test is awkward: we can't easily //! rebuild the upper chat-channel layers (`web_chat:: -//! run_chat_task → Agent::turn → execute_tools → SkillDelegationTool`) +//! run_chat_task → Agent::turn → execute_tools → `) //! without standing up an HTTP + Socket.IO stack. We drive the production -//! path from `run_subagent` downward — i.e. everything below -//! `delegate_to_integrations_agent::execute` — on a production-realistic -//! 2 MB tokio worker stack. +//! path from `run_subagent` downward — i.e. everything below a delegation +//! tool's `execute` — on a production-realistic 2 MB tokio worker stack. //! //! **Caveat — what this test does and does not catch.** Because the //! upper ~30 frames are missing, the bare path here fits in 2 MB even @@ -93,8 +95,8 @@ //! hide for longer, //! * `OPENHUMAN_WORKSPACE` pointed at a tempdir with a representative //! `config.toml` so the TOML parser does real work, -//! * `run_subagent(integrations_agent)` exactly like -//! `delegate_to_integrations_agent` does, with a stubbed `ChatModel` +//! * `run_subagent(integrations_agent)` exactly like a delegation tool +//! does, with a stubbed `ChatModel` //! that emits one `composio_list_tools` tool call on iteration 1 //! and stops on iteration 2. //! diff --git a/tests/raw_coverage/inference_agent_raw_coverage_e2e.rs b/tests/raw_coverage/inference_agent_raw_coverage_e2e.rs index 4a7874b575..cc8a107615 100644 --- a/tests/raw_coverage/inference_agent_raw_coverage_e2e.rs +++ b/tests/raw_coverage/inference_agent_raw_coverage_e2e.rs @@ -2593,7 +2593,7 @@ async fn agent_public_tools_cover_validation_and_metadata_paths() { AskClarificationTool, DelegateToPersonalityTool, DelegateTool, RUN_WORKFLOW_TOOL_NAME, RunWorkflowTool, TodoTool, }; - use openhuman_core::tools::{ArchetypeDelegationTool, SkillDelegationTool}; + use openhuman_core::tools::ArchetypeDelegationTool; let ask = AskClarificationTool::new(); assert_eq!(ask.name(), "ask_user_clarification"); @@ -2660,29 +2660,6 @@ async fn agent_public_tools_cover_validation_and_metadata_paths() { .expect("missing archetype prompt"); assert!(missing_prompt.is_error); - assert!(SkillDelegationTool::for_connected(vec![]).is_none()); - let skill_delegate = SkillDelegationTool::for_connected(vec![ - ("gmail".into(), "Email access.".into()), - ("notion".into(), "Docs.".into()), - ]) - .expect("connected tool"); - assert!(skill_delegate.description().contains("gmail")); - let unknown_toolkit = skill_delegate - .execute(json!({ "toolkit": "slack", "prompt": "search" })) - .await - .expect("unknown toolkit"); - assert!(unknown_toolkit.is_error); - assert!( - unknown_toolkit - .output() - .contains("allowed: [gmail, notion]") - ); - let blank_skill_prompt = skill_delegate - .execute(json!({ "toolkit": "gmail", "prompt": " " })) - .await - .expect("blank prompt"); - assert!(blank_skill_prompt.output().contains("`prompt` is required")); - let todo = TodoTool::new(); assert_eq!(todo.name(), "todo"); let bad_todo_op = todo diff --git a/tests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rs b/tests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rs index 60143ac618..ad11fe9e86 100644 --- a/tests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rs +++ b/tests/raw_coverage/tools_approval_channels_raw_coverage_e2e.rs @@ -1637,8 +1637,12 @@ async fn orchestrator_tool_synthesis_covers_agent_and_integration_delegation_edg ], ); + // No `delegate_to_integrations_agent`: the Skills wildcard expands to + // the connected actions as deferred tools, and these integrations carry + // no actions, so only the archetype delegate is synthesised. The + // disconnected and duplicate-slug entries contribute nothing either way. let names = tools.iter().map(|tool| tool.name()).collect::>(); - assert_eq!(names, vec!["research", "delegate_to_integrations_agent"]); + assert_eq!(names, vec!["research"]); let research = &tools[0]; // The delegation tool's description is the target agent's `when_to_use` @@ -1662,40 +1666,26 @@ async fn orchestrator_tool_synthesis_covers_agent_and_integration_delegation_edg assert!(missing_prompt.is_error); assert!(missing_prompt.output().contains("prompt")); - let integrations = &tools[1]; - let schema = integrations.parameters_schema(); - assert_eq!( - schema.pointer("/properties/toolkit/enum"), - Some(&json!(["gmail_pro", "slack_bot"])) - ); - let description = integrations.description(); - assert!(description.contains("gmail_pro: Send and triage mail.")); - assert!(description.contains("slack_bot: External integration via Slack-Bot")); - assert!(!description.contains("Slack.Bot")); - assert!(!description.contains("Disconnected")); - - let missing_toolkit = integrations - .execute(json!({ "prompt": "send a message" })) - .await - .expect("missing toolkit returns tool error"); - assert!(missing_toolkit.is_error); - assert!(missing_toolkit.output().contains("toolkit")); - - let unknown_toolkit = integrations - .execute(json!({ "toolkit": "calendar", "prompt": "create an event" })) - .await - .expect("unknown toolkit returns tool error"); - assert!(unknown_toolkit.is_error); - assert!(unknown_toolkit.output().contains("gmail_pro")); - assert!(unknown_toolkit.output().contains("slack_bot")); - - let blank_prompt = integrations - .execute(json!({ "toolkit": "GMail-Pro", "prompt": " " })) - .await - .expect("blank prompt returns tool error after slug normalization"); - assert!(blank_prompt.is_error); - assert!(blank_prompt.output().contains("prompt")); - + // With actions on a connected toolkit, each becomes a `Deferred` tool + // the orchestrator reaches through `tool_search`; an unconnected + // toolkit's actions stay out. + let mut gmail = coverage_connected_integration("GMail Pro", "Send and triage mail.", true); + gmail.tools = vec![openhuman_core::agent::prompts::ConnectedIntegrationTool { + name: "GMAIL_SEND_EMAIL".into(), + description: "Send an email.".into(), + parameters: None, + }]; + let mut off = coverage_connected_integration("Disconnected", "Should be skipped.", false); + off.tools = vec![openhuman_core::agent::prompts::ConnectedIntegrationTool { + name: "OFF_ACTION".into(), + description: "Never advertised.".into(), + parameters: None, + }]; + let tools = collect_orchestrator_tools(&orchestrator, ®istry, &[gmail, off]); + let names = tools.iter().map(|tool| tool.name()).collect::>(); + assert_eq!(names, vec!["research", "GMAIL_SEND_EMAIL"]); + assert_eq!(tools[1].exposure(), tinytools::ToolExposure::Deferred); + assert_eq!(tools[1].description(), "Send an email."); } #[tokio::test]