diff --git a/crates/openhuman-core/src/agent/orchestration/fleet_tools.rs b/crates/openhuman-core/src/agent/orchestration/fleet_tools.rs new file mode 100644 index 0000000000..1d01d9f9e7 --- /dev/null +++ b/crates/openhuman-core/src/agent/orchestration/fleet_tools.rs @@ -0,0 +1,114 @@ +//! Which fleet-control tools a parent agent can actually call. +//! +//! The `[async_subagent_ref]` envelope and the ambient `[active_subagents]` +//! roster used to hard-code a full fleet vocabulary — `wait_subagent`, +//! `steer_subagent`, `wait_loop`, `close_subagent` — while the orchestrator's +//! definition deliberately dropped most of it (#5701: a sub-agent result is +//! delivered back automatically on a later turn, so nothing needs to block). +//! The model was told to call tools it did not have, spent an iteration +//! reasoning about the mismatch, and improvised (`shell echo "waiting for +//! subagent"`). Every delegation paid a full extra model call for nothing. +//! +//! This module reads the parent's definition once per render and answers +//! "does this parent see tool X?", so both texts only ever name tools that +//! are in the caller's belt. + +use crate::agent::harness::definition::{AgentDefinitionRegistry, ToolScope}; + +/// The fleet-control tools whose availability shapes the delegation texts. +const FLEET_TOOLS: &[&str] = &[ + "steer_subagent", + "wait_subagent", + "wait", + "wait_loop", + "close_subagent", + "continue_subagent", + "list_subagents", +]; + +/// The subset of [`FLEET_TOOLS`] a given parent definition exposes. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct FleetToolSet { + available: Vec<&'static str>, +} + +impl FleetToolSet { + /// Every fleet tool — the pre-#5701 assumption, used when the parent's + /// definition cannot be resolved so the texts degrade to their old shape + /// rather than to silence. + pub(crate) fn all() -> Self { + Self { + available: FLEET_TOOLS.to_vec(), + } + } + + /// Resolve the set for `agent_definition_id` from the global registry. + pub(crate) fn for_parent(agent_definition_id: &str) -> Self { + let Some(registry) = AgentDefinitionRegistry::global() else { + return Self::all(); + }; + let Some(definition) = registry.get(agent_definition_id) else { + log::debug!( + "[fleet_tools] parent definition '{}' not in registry; assuming full fleet vocabulary", + agent_definition_id + ); + return Self::all(); + }; + Self::from_scope(&definition.tools, &definition.disallowed_tools) + } + + /// Resolve the set from a turn's *effective* visible tool names — the + /// live, already-filtered membership set (hides, named restrictions, and + /// policy narrowing all applied), as opposed to [`Self::for_parent`]'s + /// static read of the parent's registered definition. Prefer this + /// whenever the caller already has that snapshot: a hide or restriction + /// applied mid-session can narrow a turn's real tool surface below what + /// the definition alone would suggest, and offering a control absent + /// from this snapshot invites a denied tool call. + pub(crate) fn from_visible_tool_names(names: &std::collections::HashSet) -> Self { + let available = FLEET_TOOLS + .iter() + .copied() + .filter(|name| names.contains(*name)) + .collect(); + Self { available } + } + + /// Derive the set from a definition's tool scope and denylist. A + /// `Named` scope exposes exactly the fleet tools it lists; `Wildcard` + /// exposes all of them. `disallowed_tools` (exact or trailing-`*` + /// prefix) removes entries from either. + pub(crate) fn from_scope(scope: &ToolScope, disallowed: &[String]) -> Self { + let denied = |name: &str| { + disallowed + .iter() + .any(|entry| match entry.strip_suffix('*') { + Some(prefix) => name.starts_with(prefix), + None => entry == name, + }) + }; + let available = FLEET_TOOLS + .iter() + .copied() + .filter(|name| match scope { + ToolScope::Wildcard => true, + ToolScope::Named(named) => named.iter().any(|n| n == name), + }) + .filter(|name| !denied(name)) + .collect(); + Self { available } + } + + pub(crate) fn has(&self, tool: &str) -> bool { + self.available.contains(&tool) + } + + /// Whether the parent can block on or poll a worker at all. + pub(crate) fn can_wait(&self) -> bool { + self.has("wait_subagent") + } +} + +#[cfg(test)] +#[path = "fleet_tools_tests.rs"] +mod tests; diff --git a/crates/openhuman-core/src/agent/orchestration/fleet_tools_tests.rs b/crates/openhuman-core/src/agent/orchestration/fleet_tools_tests.rs new file mode 100644 index 0000000000..ec0495d301 --- /dev/null +++ b/crates/openhuman-core/src/agent/orchestration/fleet_tools_tests.rs @@ -0,0 +1,67 @@ +use super::FleetToolSet; +use crate::agent::harness::definition::ToolScope; + +fn named(tools: &[&str]) -> ToolScope { + ToolScope::Named(tools.iter().map(|t| t.to_string()).collect()) +} + +#[test] +fn named_scope_exposes_only_listed_fleet_tools() { + let set = FleetToolSet::from_scope( + &named(&[ + "spawn_async_subagent", + "list_subagents", + "continue_subagent", + "shell", + ]), + &[], + ); + assert!(set.has("list_subagents")); + assert!(set.has("continue_subagent")); + assert!(!set.has("wait_subagent")); + assert!(!set.has("steer_subagent")); + assert!(!set.has("wait_loop")); + assert!(!set.can_wait()); +} + +#[test] +fn wildcard_scope_exposes_every_fleet_tool_minus_denylist() { + let set = FleetToolSet::from_scope(&ToolScope::Wildcard, &["wait*".to_string()]); + assert!(set.has("steer_subagent")); + assert!(set.has("close_subagent")); + assert!(!set.has("wait")); + assert!(!set.has("wait_loop")); + assert!(!set.has("wait_subagent")); + assert!(!set.can_wait()); +} + +#[test] +fn all_is_the_full_vocabulary() { + let set = FleetToolSet::all(); + for tool in [ + "steer_subagent", + "wait_subagent", + "wait", + "wait_loop", + "close_subagent", + "continue_subagent", + "list_subagents", + ] { + assert!(set.has(tool), "{tool}"); + } + assert!(set.can_wait()); +} + +/// The shipped orchestrator definition is the case that motivated this +/// module: it must not be told about wait/steer/close tools. +#[test] +fn builtin_orchestrator_has_no_wait_or_steer() { + let registry = crate::agent::harness::definition::AgentDefinitionRegistry::builtins_only(); + let def = registry.get("orchestrator").expect("built-in orchestrator"); + let set = FleetToolSet::from_scope(&def.tools, &def.disallowed_tools); + assert!(set.has("list_subagents")); + assert!(set.has("continue_subagent")); + assert!(!set.can_wait()); + assert!(!set.has("steer_subagent")); + assert!(!set.has("close_subagent")); +} diff --git a/crates/openhuman-core/src/agent/orchestration/mod.rs b/crates/openhuman-core/src/agent/orchestration/mod.rs index dc24338e6d..3175efefaf 100644 --- a/crates/openhuman-core/src/agent/orchestration/mod.rs +++ b/crates/openhuman-core/src/agent/orchestration/mod.rs @@ -20,6 +20,7 @@ pub(crate) mod background_completions; pub(crate) mod background_delivery; pub mod command_center; pub(crate) mod delegation; +pub(crate) mod fleet_tools; mod ops; pub(crate) mod parent_context; pub(crate) mod run_ledger_finalize; diff --git a/crates/openhuman-core/src/agent/orchestration/running_subagents/roster.rs b/crates/openhuman-core/src/agent/orchestration/running_subagents/roster.rs index c791c0d9bd..80da467719 100644 --- a/crates/openhuman-core/src/agent/orchestration/running_subagents/roster.rs +++ b/crates/openhuman-core/src/agent/orchestration/running_subagents/roster.rs @@ -3,6 +3,7 @@ //! `[active_subagents]` context block. use super::registry::{registry, SubagentStatus}; +use crate::agent::orchestration::fleet_tools::FleetToolSet; /// Compact, read-only view of one registered sub-agent, for ambient injection /// into a parent's turn context (see [`active_subagents_context_block`]). @@ -52,6 +53,40 @@ pub(crate) fn snapshot_for_parent(parent_session: &str) -> Vec out } +/// The follow-up guidance sentence, built from the tools the parent can see. +fn roster_guidance(fleet: &FleetToolSet) -> String { + let mut parts: Vec = Vec::new(); + if fleet.has("wait_subagent") { + parts.push("use wait_subagent to collect a `completed` one".into()); + } else { + parts.push( + "a `running` or `completed` worker's result is delivered to you automatically on a \ + later turn — do not wait or poll for it" + .into(), + ); + } + if fleet.has("steer_subagent") { + parts.push("steer_subagent to redirect a `running` one".into()); + } + if fleet.has("continue_subagent") { + parts.push( + "continue_subagent to answer an `awaiting_user` one or to RESUME an `idle` one with \ + a follow-up (it keeps its full prior context — do NOT re-delegate the same task \ + from scratch)" + .into(), + ); + } + if fleet.has("close_subagent") { + parts.push("close_subagent when done".into()); + } + if fleet.has("list_subagents") { + parts.push("list_subagents to re-enumerate".into()); + } + let mut sentence = parts.join(", "); + sentence.push('.'); + sentence +} + /// Most-recent durable sessions surfaced in the roster when they are not in /// the live registry (cold boot / later turn). Bounds prompt growth on /// threads with a long delegation history. @@ -71,9 +106,15 @@ const DURABLE_ROSTER_CAP: usize = 12; /// cold-booted parent had no idea its previous sub-agents existed and /// would re-delegate from scratch instead of resuming by /// `subagent_session_id` (the "fresh context from day 0" bug). +/// +/// `fleet` is the parent's fleet-control vocabulary: the guidance sentence +/// only names tools the parent can call (the orchestrator has no +/// `wait_subagent` / `steer_subagent` / `close_subagent` since #5701, and +/// telling it otherwise cost an iteration of confused reasoning per turn). pub(crate) fn active_subagents_context_block( parent_session: &str, workspace_dir: &std::path::Path, + fleet: &FleetToolSet, ) -> Option { let workers = snapshot_for_parent(parent_session); @@ -114,13 +155,10 @@ pub(crate) fn active_subagents_context_block( "[active_subagents]\n\ You have {} sub-agent worker(s) for this conversation (live and/or from earlier \ turns). This is your authoritative roster — trust it over memory. Track each by \ - subagent_session_id; use wait_subagent to collect a `completed` one, steer_subagent \ - to redirect a `running` one, continue_subagent to answer an `awaiting_user` one or \ - to RESUME an `idle` one with a follow-up (it keeps its full prior context — do NOT \ - re-delegate the same task from scratch), close_subagent when done, and \ - list_subagents to re-enumerate. Never fabricate a result for a worker still running \ + subagent_session_id. {} Never fabricate a result for a worker still running \ or one that has failed.\n", - workers.len() + durable.len() + workers.len() + durable.len(), + roster_guidance(fleet) ); for w in &workers { let session = w.subagent_session_id.as_deref().unwrap_or("(none)"); diff --git a/crates/openhuman-core/src/agent/orchestration/running_subagents_tests.rs b/crates/openhuman-core/src/agent/orchestration/running_subagents_tests.rs index cafe4f5518..4c04d5ee09 100644 --- a/crates/openhuman-core/src/agent/orchestration/running_subagents_tests.rs +++ b/crates/openhuman-core/src/agent/orchestration/running_subagents_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::agent::orchestration::fleet_tools::FleetToolSet; use crate::agent::orchestration::running_subagents::registry::DETACHED_LEDGER_TIMEOUT_MS; use crate::agent::orchestration::running_subagents::resolve::resume_ref_for_task; use crate::agent::orchestration::running_subagents::resolve::task_id_for_session; @@ -242,15 +243,45 @@ async fn snapshot_and_block_scope_to_parent_and_reflect_live_status() { assert_eq!(snap[1].status, "running"); let block = - active_subagents_context_block("fleet-parent", &test_workspace()).expect("block present"); + active_subagents_context_block("fleet-parent", &test_workspace(), &FleetToolSet::all()) + .expect("block present"); assert!(block.contains("[active_subagents]")); + assert!(block.contains("use wait_subagent to collect")); assert!(block.contains("You have 2 sub-agent worker(s)")); assert!(block.contains("session=subsess-a")); assert!(block.contains("session=subsess-b · task=task-fleet-b · status=awaiting_user")); assert!(block.ends_with("[/active_subagents]\n\n")); // A parent with no registered workers gets no block (no perturbation). - assert!(active_subagents_context_block("nobody-here", &test_workspace()).is_none()); + assert!( + active_subagents_context_block("nobody-here", &test_workspace(), &FleetToolSet::all()) + .is_none() + ); + + // The shipped orchestrator has no wait/steer/close tools (#5701): the + // guidance must not name them and must say results arrive on their own. + { + use crate::agent::harness::definition::AgentDefinitionRegistry; + let registry = AgentDefinitionRegistry::builtins_only(); + let def = registry.get("orchestrator").expect("built-in orchestrator"); + let fleet = FleetToolSet::from_scope(&def.tools, &def.disallowed_tools); + let block = active_subagents_context_block("fleet-parent", &test_workspace(), &fleet) + .expect("block present"); + for name in [ + "wait_subagent", + "steer_subagent", + "close_subagent", + "wait_loop", + ] { + assert!( + !block.contains(name), + "{name} named for a parent without it:\n{block}" + ); + } + assert!(block.contains("delivered to you automatically")); + assert!(block.contains("continue_subagent")); + assert!(block.contains("list_subagents")); + } // Durable-store fallback: a session persisted by an EARLIER turn / // process lifetime (empty live registry for this parent) must still @@ -295,13 +326,19 @@ async fn snapshot_and_block_scope_to_parent_and_reflect_live_status() { ) .expect("mark idle"); - let block = active_subagents_context_block("cold-parent", durable_ws.path()) - .expect("durable-only roster present"); + let block = + active_subagents_context_block("cold-parent", durable_ws.path(), &FleetToolSet::all()) + .expect("durable-only roster present"); assert!(block.contains(&format!("session={}", session.subagent_session_id))); assert!(block.contains("status=idle")); assert!(block.contains("about: Daily X trending email workflow")); // Other parents' durable sessions must not leak in. - assert!(active_subagents_context_block("unrelated-parent", durable_ws.path()).is_none()); + assert!(active_subagents_context_block( + "unrelated-parent", + durable_ws.path(), + &FleetToolSet::all() + ) + .is_none()); } let _ = tx_a.send(SubagentStatus::Completed { diff --git a/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent.rs b/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent.rs index a54240eabd..2b72e7cca4 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent.rs @@ -6,6 +6,7 @@ use crate::agent::harness::definition::AgentDefinitionRegistry; use crate::agent::messages::ChatMessage; +use crate::agent::orchestration::fleet_tools::FleetToolSet; use crate::agent::orchestration::running_subagents::{self, SubagentStatus}; use crate::agent::orchestration::subagent_sessions::{ self, DurableSubagentStatus, SubagentSessionSelector, SubagentSessionStore, @@ -196,15 +197,49 @@ impl SpawnAsyncSubagentTool { include!("spawn_async_subagent_execute.rs"); /// Format the user-facing acceptance text around a structured async sub-agent reference. -fn format_async_subagent_accepted(agent_id: &str, payload_json: &str) -> String { +/// +/// The wording follows what the parent can actually do: a parent without +/// `wait_subagent` (the orchestrator, #5701) is told the result arrives on its +/// own and not to poll, instead of being invited to "wait for completion". +fn format_async_subagent_accepted( + agent_id: &str, + payload_json: &str, + fleet: &FleetToolSet, +) -> String { + // Steering and waiting are independent fleet capabilities: a parent can + // have `wait_subagent` without `steer_subagent` (or vice versa), so the + // guidance text is built from each independently rather than gated + // entirely on `can_wait()` — otherwise a wait-only parent is told to + // "send more input" through a tool it does not have. + let can_send = fleet.has("steer_subagent"); + let can_wait = fleet.can_wait(); + let guidance = match (can_send, can_wait) { + (true, true) => { + "Use the structured reference below to send more input, wait for completion, or perform a short timeout tick to check status. If the user does not need the result now, continue without blocking." + } + (true, false) => { + "Use the structured reference below to send more input if needed. You cannot and need not wait or poll for it (no shell/sleep, no fake status checks); its result is delivered to you automatically on a later turn. If the user does not need the result now, continue without blocking." + } + (false, true) => { + "Use the structured reference below to wait for completion or perform a short timeout tick to check status. If the user does not need the result now, continue without blocking." + } + (false, false) => { + "Its result is delivered to you automatically on a later turn — you cannot and need not wait or poll for it (no shell/sleep, no fake status checks). Reply to the user now with what you know, say the result is on its way, and continue. The structured reference below lists the only follow-up tools you have for this worker." + } + }; format!( - "Accepted async sub-agent `{agent_id}`. Use the structured reference below to send more input, \ - wait for completion, or perform a short timeout tick to check status. If the user does not need \ - the result now, continue without blocking.\n\n[async_subagent_ref]\n{payload_json}\n[/async_subagent_ref]" + "Accepted async sub-agent `{agent_id}`. {guidance} + +[async_subagent_ref] +{payload_json} +[/async_subagent_ref]" ) } -/// Build the machine-readable reference the orchestrator uses to steer, wait, or poll a worker. +/// Build the machine-readable reference the orchestrator uses to follow up on a worker. +/// +/// Only tools in `fleet` are offered: an instruction naming a tool the parent +/// cannot see costs an iteration of confused reasoning per delegation. fn async_subagent_ref_payload( task_id: &str, subagent_session_id: &str, @@ -213,7 +248,102 @@ fn async_subagent_ref_payload( reused: bool, reuse_decision: &str, status: &str, + fleet: &FleetToolSet, ) -> serde_json::Value { + let mut instructions = serde_json::Map::new(); + let mut next_actions: Vec = Vec::new(); + + if fleet.has("steer_subagent") { + instructions.insert( + "send_message".into(), + json!({ + "tool": "steer_subagent", + "description": "Send additional instructions or context to this running async sub-agent.", + "arguments": { + "subagent_session_id": subagent_session_id, + "message": "", + "mode": "steer" + } + }), + ); + next_actions.push("call steer_subagent to send more input".into()); + } + if fleet.has("wait_subagent") { + instructions.insert( + "wait".into(), + json!({ + "tool": "wait_subagent", + "description": "Block until the async sub-agent finishes, up to the timeout.", + "arguments": { "subagent_session_id": subagent_session_id, "timeout_secs": 120 } + }), + ); + instructions.insert( + "timeout_tick".into(), + json!({ + "tool": "wait_subagent", + "description": "Perform a short status tick without committing the parent to a long wait.", + "arguments": { "subagent_session_id": subagent_session_id, "timeout_secs": 1 } + }), + ); + next_actions.push("call wait_subagent with timeout_secs to collect the result".into()); + next_actions + .push("call wait_subagent with timeout_secs=1 as a timeout tick/status check".into()); + let reminder = format!( + "Check async sub-agent {agent_id} status with wait_subagent using subagent_session_id {subagent_session_id}." + ); + if fleet.has("wait") { + instructions.insert( + "delayed_tick".into(), + json!({ + "tool": "wait", + "description": "Trigger a delayed callback before checking this async sub-agent again.", + "arguments": { "duration_secs": 30, "message": reminder } + }), + ); + } + if fleet.has("wait_loop") { + instructions.insert( + "delayed_loop".into(), + json!({ + "tool": "wait_loop", + "description": "Trigger repeatable delayed callbacks while this async sub-agent is still relevant.", + "arguments": { + "duration_secs": 30, + "message": reminder, + "loop_key": subagent_session_id, + "iteration": 1 + } + }), + ); + } + if fleet.has("wait") || fleet.has("wait_loop") { + next_actions.push( + "call wait or wait_loop with the returned message to trigger a delayed status check".into(), + ); + } + } + if fleet.has("continue_subagent") { + instructions.insert( + "answer_or_resume".into(), + json!({ + "tool": "continue_subagent", + "description": "Answer this worker if it pauses on ask_user_clarification (awaiting_user), or resume it later with a follow-up that keeps its context.", + "arguments": { "subagent_session_id": subagent_session_id, "message": "" } + }), + ); + next_actions.push( + "call continue_subagent only if this worker reports awaiting_user, or to resume it with a follow-up".into(), + ); + } + if fleet.has("list_subagents") { + next_actions.push("call list_subagents to re-enumerate your workers if this reference scrolls out of context".into()); + } + next_actions.push(if fleet.can_wait() { + "continue without waiting when the current user reply does not depend on the result".into() + } else { + "continue now: the result is delivered to you automatically on a later turn; never poll for it".into() + }); + json!({ "task_id": task_id, "taskId": task_id, @@ -228,58 +358,9 @@ fn async_subagent_ref_payload( "reused": reused, "reuse_decision": reuse_decision, "reuseDecision": reuse_decision, - "instructions": { - "send_message": { - "tool": "steer_subagent", - "description": "Send additional instructions or context to this running async sub-agent.", - "arguments": { - "subagent_session_id": subagent_session_id, - "message": "", - "mode": "steer" - } - }, - "wait": { - "tool": "wait_subagent", - "description": "Block until the async sub-agent finishes, up to the timeout.", - "arguments": { - "subagent_session_id": subagent_session_id, - "timeout_secs": 120 - } - }, - "timeout_tick": { - "tool": "wait_subagent", - "description": "Perform a short status tick without committing the parent to a long wait.", - "arguments": { - "subagent_session_id": subagent_session_id, - "timeout_secs": 1 - } - }, - "delayed_tick": { - "tool": "wait", - "description": "Trigger a delayed callback before checking this async sub-agent again.", - "arguments": { - "duration_secs": 30, - "message": format!("Check async sub-agent {agent_id} status with wait_subagent using subagent_session_id {subagent_session_id}.") - } - }, - "delayed_loop": { - "tool": "wait_loop", - "description": "Trigger repeatable delayed callbacks while this async sub-agent is still relevant.", - "arguments": { - "duration_secs": 30, - "message": format!("Check async sub-agent {agent_id} status with wait_subagent using subagent_session_id {subagent_session_id}."), - "loop_key": subagent_session_id, - "iteration": 1 - } - } - }, - "next_actions": [ - "call steer_subagent to send more input", - "call wait_subagent with timeout_secs to collect the result", - "call wait_subagent with timeout_secs=1 as a timeout tick/status check", - "call wait or wait_loop with the returned message to trigger a delayed status check", - "continue without waiting when the current user reply does not depend on the result" - ] + "result_delivery": "automatic", + "instructions": instructions, + "next_actions": next_actions }) } diff --git a/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent_execute.rs b/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent_execute.rs index 4fde0874e7..35074787a0 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent_execute.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent_execute.rs @@ -92,6 +92,18 @@ impl SpawnAsyncSubagentTool { } }; + // The follow-up vocabulary offered back to the parent is limited to + // the fleet tools visible on *this turn* (see `fleet_tools`), not + // just the parent definition's static scope: a hide or named + // restriction can narrow the turn's real tool surface below what the + // definition alone would suggest, and offering a control the parent + // cannot currently call invites a denied tool call. + let fleet = if parent.visible_tool_names.is_empty() { + FleetToolSet::for_parent(&parent.agent_definition_id) + } else { + FleetToolSet::from_visible_tool_names(&parent.visible_tool_names) + }; + if !parent.allowed_subagent_ids.contains(&definition.id) { log::warn!( "[spawn_async_subagent] blocked subagent outside allowlist parent={} requested={} allowed={:?}", @@ -256,11 +268,18 @@ impl SpawnAsyncSubagentTool { true, reuse_decision.as_str(), "running", + &fleet, ); + let follow_up = if fleet.can_wait() { + "Use the structured reference below to send more input, wait, or perform a short timeout tick." + } else { + "Its result is delivered to you automatically on a later turn; do not wait or poll for it." + }; return Ok(ToolResult::success(format!( "Continued reusable async sub-agent `{}`. It is already running and will pick up the new instruction at its next step. \ - Use the structured reference below to send more input, wait, or perform a short timeout tick.\n\n[async_subagent_ref]\n{}\n[/async_subagent_ref]", + {}\n\n[async_subagent_ref]\n{}\n[/async_subagent_ref]", payload["agent_id"].as_str().unwrap_or("subagent"), + follow_up, serde_json::to_string(&payload) .unwrap_or_else(|_| "{}".to_string()) ))); @@ -785,6 +804,7 @@ impl SpawnAsyncSubagentTool { reusable.is_some(), reuse_decision.as_str(), "running", + &fleet, ); let payload_json = match serde_json::to_string(&payload) { Ok(serialized) => { @@ -806,6 +826,7 @@ impl SpawnAsyncSubagentTool { Ok(ToolResult::success(format_async_subagent_accepted( payload["agent_id"].as_str().unwrap_or("subagent"), &payload_json, + &fleet, ))) } } diff --git a/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent_tests.rs b/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent_tests.rs index 3a0728e42d..646db6c067 100644 --- a/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent_tests.rs +++ b/crates/openhuman-core/src/agent/orchestration/tools/spawn_async_subagent_tests.rs @@ -48,7 +48,7 @@ fn background_contract_forbids_user_attention() { #[test] fn accepted_message_hides_task_id_from_prose() { let payload = r#"{"task_id":"sub-internal-123","agent_id":"archivist","mode":"async"}"#; - let message = format_async_subagent_accepted("archivist", payload); + let message = format_async_subagent_accepted("archivist", payload, &FleetToolSet::all()); let prose = message .split("[async_subagent_ref]") .next() @@ -60,6 +60,54 @@ fn accepted_message_hides_task_id_from_prose() { assert!(message.contains("sub-internal-123")); } +/// A parent with `wait_subagent` but not `steer_subagent` (a wait-only +/// fleet) must be told it can wait/poll but never invited to "send more +/// input" — that guidance, and the `send_message` instruction, require +/// `steer_subagent` specifically. Regression for CodeRabbit finding on +/// `format_async_subagent_accepted`/`async_subagent_ref_payload` gating both +/// messages on `can_wait()` alone. +#[test] +fn wait_only_fleet_omits_steering_guidance_and_instruction() { + let fleet = FleetToolSet::from_scope( + &crate::agent::harness::definition::ToolScope::Named(vec!["wait_subagent".to_string()]), + &[], + ); + assert!(fleet.can_wait()); + assert!(!fleet.has("steer_subagent")); + + let payload = async_subagent_ref_payload( + "sub-123", + "subsess-456", + "researcher", + Some("thread-worker"), + false, + "created", + "running", + &fleet, + ); + assert_eq!(payload["instructions"]["wait"]["tool"], "wait_subagent"); + assert!( + !payload["instructions"] + .as_object() + .expect("instructions map") + .contains_key("send_message"), + "send_message offered to a parent without steer_subagent" + ); + let serialized = serde_json::to_string(&payload).unwrap(); + assert!( + !serialized.contains("steer_subagent"), + "steer_subagent leaked into the envelope" + ); + + let message = format_async_subagent_accepted("researcher", &serialized, &fleet); + let prose = message.split("[async_subagent_ref]").next().unwrap(); + assert!(prose.contains("wait for completion")); + assert!( + !prose.contains("send more input"), + "wait-only parent told to send more input it cannot send" + ); +} + #[test] fn async_reference_payload_includes_agent_id_and_control_instructions() { let payload = async_subagent_ref_payload( @@ -70,6 +118,7 @@ fn async_reference_payload_includes_agent_id_and_control_instructions() { false, "created", "running", + &FleetToolSet::all(), ); assert_eq!(payload["agent_id"], "researcher"); @@ -87,6 +136,72 @@ fn async_reference_payload_includes_agent_id_and_control_instructions() { ); } +/// The shipped orchestrator (#5701) has no wait/steer/close tools; the envelope +/// must not name them, and must say the result arrives on its own. +#[test] +fn async_reference_matches_the_orchestrator_fleet_vocabulary() { + let registry = AgentDefinitionRegistry::builtins_only(); + let def = registry.get("orchestrator").expect("built-in orchestrator"); + let fleet = FleetToolSet::from_scope(&def.tools, &def.disallowed_tools); + + let payload = async_subagent_ref_payload( + "sub-123", + "subsess-456", + "integrations_agent", + None, + false, + "created", + "running", + &fleet, + ); + let instructions = payload["instructions"] + .as_object() + .expect("instructions map"); + for absent in [ + "wait", + "timeout_tick", + "delayed_tick", + "delayed_loop", + "send_message", + ] { + assert!( + !instructions.contains_key(absent), + "{absent} offered to a parent without it" + ); + } + assert_eq!( + payload["instructions"]["answer_or_resume"]["tool"], + "continue_subagent" + ); + assert_eq!(payload["result_delivery"], "automatic"); + let serialized = serde_json::to_string(&payload).unwrap(); + for name in [ + "wait_subagent", + "steer_subagent", + "wait_loop", + "close_subagent", + ] { + assert!( + !serialized.contains(name), + "{name} leaked into the envelope" + ); + } + let next: Vec<&str> = payload["next_actions"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str()) + .collect(); + assert!(next + .iter() + .any(|a| a.contains("delivered to you automatically"))); + + let message = format_async_subagent_accepted("integrations_agent", &serialized, &fleet); + let prose = message.split("[async_subagent_ref]").next().unwrap(); + assert!(prose.contains("delivered to you automatically")); + assert!(!prose.contains("wait for completion")); +} + #[test] fn durable_task_key_defaults_to_prompt_not_display_title() { let args = json!({ 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 4c6233c62c..e095d4865e 100644 --- a/crates/openhuman-core/src/agent/session_host/runtime_session.rs +++ b/crates/openhuman-core/src/agent/session_host/runtime_session.rs @@ -658,10 +658,34 @@ impl OpenHumanTurnPrelude { run_context.stop_hooks.push(Arc::new(hook)); } } + // Build the roster from this turn's *effective* visible tool set + // (snapshotted under the tool-surface lock, then released) rather + // than the parent definition's static scope alone: a hide or named + // restriction can narrow what this turn can actually call below the + // definition's baseline, and the roster must not advertise a fleet + // control the turn cannot invoke. + let turn_fleet = { + let visible = self + .tool_surface + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .visible_tool_names + .clone(); + if visible.is_empty() { + crate::agent::orchestration::fleet_tools::FleetToolSet::for_parent( + &self.agent_definition_id, + ) + } else { + crate::agent::orchestration::fleet_tools::FleetToolSet::from_visible_tool_names( + &visible, + ) + } + }; if let Some(block) = crate::agent::orchestration::running_subagents::active_subagents_context_block( &self.event_session_id, &self.workspace_dir, + &turn_fleet, ) { context.push_str(&block); diff --git a/crates/openhuman-core/src/agent/tinyagents/turn_models.rs b/crates/openhuman-core/src/agent/tinyagents/turn_models.rs index d3b98cf4b5..744b73b503 100644 --- a/crates/openhuman-core/src/agent/tinyagents/turn_models.rs +++ b/crates/openhuman-core/src/agent/tinyagents/turn_models.rs @@ -88,6 +88,23 @@ impl TurnModels { /// Host resolver for one live invocation. It exposes the exact pre-built /// primary and fallback route models already selected by OpenHuman, rather /// than constructing a fresh config-routed model during hosted preparation. +/// +/// # Who wins: the turn's selection or the definition's pin +/// +/// The **primary** is the model OpenHuman already chose for this turn — the +/// user's per-thread `model_override`, else `config.default_model` — and for +/// the turn's lead (the depth-0 agent, `is_team_lead`) it always wins. The +/// harness forwards the definition's `[model] hint`/`model` as `model_pin` +/// on every resolve; honouring it for the lead let the orchestrator's +/// `hint = "coding"` silently reroute every chat turn onto `coding-v1` +/// (DeepSeek V4 Pro) no matter which model the user picked in the UI, since +/// `coding-v1` is always a registered tier route. A pin is advisory +/// (`ModelResolveRequest::model_pin` docs) and the lead's selection is the +/// stronger, more explicit signal. +/// +/// Sub-agents (depth > 0) keep resolving their pin against the tier routes — +/// that is how `integrations_agent`'s `hint = "burst"` reaches `burst-v1` — +/// and fall back to the primary when the pin names no built route. pub(crate) struct TurnModelResolver { primary: TurnChatModel, routes: std::collections::HashMap, @@ -95,10 +112,17 @@ pub(crate) struct TurnModelResolver { impl TurnModelResolver { pub(crate) fn from_turn_models(models: &TurnModels) -> Self { - Self { - primary: models.primary.clone(), - routes: models.routes.iter().cloned().collect(), - } + Self::new( + models.primary.clone(), + models.routes.iter().cloned().collect(), + ) + } + + pub(crate) fn new( + primary: TurnChatModel, + routes: std::collections::HashMap, + ) -> Self { + Self { primary, routes } } } @@ -108,8 +132,19 @@ impl ModelResolver<()> for TurnModelResolver { &self, request: &ModelResolveRequest, ) -> tinyagents_harness::Result { - Ok(request - .model_pin() + let pin = request.model_pin(); + if request.is_team_lead { + if let Some(pin) = pin.filter(|pin| self.routes.contains_key(*pin)) { + tracing::debug!( + target: "tinyagents", + agent_id = %request.agent_id, + pin, + "[models][resolver] lead keeps the turn's selected primary; definition model pin ignored" + ); + } + return Ok(self.primary.clone()); + } + Ok(pin .and_then(|name| self.routes.get(name)) .cloned() .unwrap_or_else(|| self.primary.clone())) @@ -561,3 +596,7 @@ impl TurnModelSource { Err(anyhow::anyhow!("turn model source is missing a model")) } } + +#[cfg(test)] +#[path = "turn_models_tests.rs"] +mod tests; diff --git a/crates/openhuman-core/src/agent/tinyagents/turn_models_tests.rs b/crates/openhuman-core/src/agent/tinyagents/turn_models_tests.rs new file mode 100644 index 0000000000..3b8e05b2a0 --- /dev/null +++ b/crates/openhuman-core/src/agent/tinyagents/turn_models_tests.rs @@ -0,0 +1,91 @@ +use std::collections::HashMap; +use std::sync::Arc; + +use async_trait::async_trait; +use tinyagents_harness::host::{ModelResolveRequest, ModelResolver}; +use tinyinference_llm::message::ContentBlock; +use tinyinference_llm::model::{ChatModel, ModelRequest, ModelResponse}; + +use super::TurnModelResolver; + +/// A stub that answers with its own name so a test can tell which model the +/// resolver handed back. +struct NamedModel(&'static str); + +#[async_trait] +impl ChatModel<()> for NamedModel { + async fn invoke( + &self, + _state: &(), + _request: ModelRequest, + ) -> tinyinference_llm::Result { + Ok(ModelResponse::assistant(self.0)) + } +} + +async fn name_of(model: &Arc>) -> String { + let response = model + .invoke(&(), ModelRequest::default()) + .await + .expect("stub model never fails"); + response + .message + .content + .iter() + .filter_map(|block| match block { + ContentBlock::Text(text) => Some(text.as_str()), + _ => None, + }) + .collect() +} + +fn resolver() -> TurnModelResolver { + let primary: Arc> = + Arc::new(NamedModel("openrouter/deepseek/deepseek-v4.1-flash")); + let mut routes: HashMap>> = HashMap::new(); + routes.insert("coding-v1".to_string(), Arc::new(NamedModel("coding-v1"))); + routes.insert("burst-v1".to_string(), Arc::new(NamedModel("burst-v1"))); + TurnModelResolver::new(primary, routes) +} + +/// Regression for the orchestrator's `hint = "coding"` overriding the user's +/// UI model pick: the turn lead must get the selected primary even when its +/// definition pin names a built tier route. +#[tokio::test] +async fn lead_keeps_selected_primary_over_definition_pin() { + let request = ModelResolveRequest::new("orchestrator") + .as_team_lead() + .with_model_pin("coding-v1"); + let model = resolver().resolve(&request).await.expect("resolves"); + assert_eq!( + name_of(&model).await, + "openrouter/deepseek/deepseek-v4.1-flash" + ); +} + +#[tokio::test] +async fn lead_without_pin_gets_primary() { + let request = ModelResolveRequest::new("orchestrator").as_team_lead(); + let model = resolver().resolve(&request).await.expect("resolves"); + assert_eq!( + name_of(&model).await, + "openrouter/deepseek/deepseek-v4.1-flash" + ); +} + +#[tokio::test] +async fn subagent_pin_resolves_to_its_tier_route() { + let request = ModelResolveRequest::new("integrations_agent").with_model_pin("burst-v1"); + let model = resolver().resolve(&request).await.expect("resolves"); + assert_eq!(name_of(&model).await, "burst-v1"); +} + +#[tokio::test] +async fn subagent_pin_without_route_falls_back_to_primary() { + let request = ModelResolveRequest::new("worker").with_model_pin("vision-v1"); + let model = resolver().resolve(&request).await.expect("resolves"); + assert_eq!( + name_of(&model).await, + "openrouter/deepseek/deepseek-v4.1-flash" + ); +}