diff --git a/src/openhuman/agent/harness/archivist/lifecycle.rs b/src/openhuman/agent/harness/archivist/lifecycle.rs index 600b293ee8..a29be8e719 100644 --- a/src/openhuman/agent/harness/archivist/lifecycle.rs +++ b/src/openhuman/agent/harness/archivist/lifecycle.rs @@ -91,7 +91,7 @@ impl ArchivistHook { /// `new`, `disabled` or `new_with_stubs*` — and the tests that do need a /// deterministic LLM install the task-local around the call itself, which /// is where it has to be anyway for `summarise` to see it. - pub fn with_config(mut self, config: Config) -> Self { + pub fn with_config(mut self, config: std::sync::Arc) -> Self { // Probe the summariser: can this host build a chat model for the recap // role right now? The model is dropped immediately — see above. let probe = crate::openhuman::inference::provider::create_chat_model_with_model_id( @@ -559,7 +559,10 @@ impl ArchivistHook { segment.segment_id, summary ); crate::openhuman::memory::goals::spawn_enrich_goals( - cfg.clone(), + // The hook now shares the factory's `Arc`; this + // detached task still owns a `Config`, so materialise one + // here. Once per closed segment, not once per live agent. + cfg.as_ref().clone(), cfg.workspace_dir.clone(), context, ); diff --git a/src/openhuman/agent/harness/archivist/types.rs b/src/openhuman/agent/harness/archivist/types.rs index e3d8096d14..0ea6b54374 100644 --- a/src/openhuman/agent/harness/archivist/types.rs +++ b/src/openhuman/agent/harness/archivist/types.rs @@ -24,7 +24,13 @@ pub struct ArchivistHook { /// /// When `None`, the tree-ingest path is skipped. Set via /// [`ArchivistHook::with_config`] on the production path. - pub(super) config: Option, + /// + /// Held behind an `Arc` so the hook shares the session factory's single + /// `Config` snapshot rather than deep-cloning a 95-field struct with + /// nested `Vec`s into every live agent (openhuman#6218). `Config` is + /// immutable after construction, so sharing and copying are behaviourally + /// identical here. + pub(super) config: Option>, /// Whether an LLM summariser can be built for this workspace. `false` /// means the heuristic bookend summary is used instead. /// diff --git a/src/openhuman/agent/harness/fork_context.rs b/src/openhuman/agent/harness/fork_context.rs index 6663e98e58..c873930893 100644 --- a/src/openhuman/agent/harness/fork_context.rs +++ b/src/openhuman/agent/harness/fork_context.rs @@ -54,7 +54,7 @@ pub struct ParentExecutionContext { /// the provider for prefix-cache reuse. The parent's synthesised /// delegation specs are deliberately absent: a sub-agent is never handed a /// `delegate_*` tool (#4452), so there is no instance here for one. - pub all_tool_specs: Arc>, + pub all_tool_specs: Arc>>, /// Names of the tools the parent actually advertises and will execute this /// turn. Consumers that recommend or directly invoke parent tools consult @@ -68,7 +68,7 @@ pub struct ParentExecutionContext { /// building a child's tool set — that is [`Self::all_tools`] + /// [`Self::all_tool_specs`], which carry no delegate. Empty when the /// builder does not know the parent's surface. - pub visible_tool_specs: Arc>, + pub visible_tool_specs: Arc>>, /// Explicit profile/channel ceiling inherited by child agents. This is not /// the parent's role-specific visible surface: an orchestrator may delegate diff --git a/src/openhuman/agent/harness/session/builder/builder_build.rs b/src/openhuman/agent/harness/session/builder/builder_build.rs new file mode 100644 index 0000000000..dc1549727c --- /dev/null +++ b/src/openhuman/agent/harness/session/builder/builder_build.rs @@ -0,0 +1,305 @@ +//! `AgentBuilder::build` — validates and assembles the final [`Agent`]. + +use super::{dedup_visible_tool_specs, visible_tool_specs_for_policy}; +use crate::openhuman::agent::context::ContextManager; +use crate::openhuman::agent::harness::session::types::{Agent, AgentBuilder}; +use crate::openhuman::tools::agent_policy::ToolPolicyEngine; +use crate::openhuman::tools::{Tool, ToolSpec}; +use anyhow::Result; +use std::sync::Arc; + +impl AgentBuilder { + /// Validates the configuration and constructs a new `Agent` instance. + /// + /// This method is responsible for wiring together the provided components, + /// setting up the context manager, and initializing the conversation history. + /// It ensures that all required fields (provider, tools, memory, etc.) are present. + pub fn build(self) -> Result { + let tools = self + .tools + .ok_or_else(|| anyhow::anyhow!("tools are required"))?; + // The synthesised set lives beside the durable registry, never inside + // it (`Agent::synthesized_tools`); a durable name wins a collision. + let synthesized_tools = super::drop_synthesized_name_collisions( + &tools, + self.synthesized_tools.unwrap_or_default(), + ); + let synthesized_tool_names: std::collections::HashSet = synthesized_tools + .iter() + .map(|tool| tool.name().to_string()) + .collect(); + // Durable specs first, synthesised after — every reader's order. + // + // Each schema is built once and handed out behind an `Arc`. The three + // spec views an agent keeps (`durable_tool_specs`, `tool_specs`, + // `visible_tool_specs`) overlap heavily — the durable set is a prefix + // of the full set, and the visible set is a filtered subset of it — so + // materialising them as independent `Vec` kept every + // JSON-Schema `parameters` value resident up to three times per agent. + // Sharing the leaves makes the extra views cost one pointer per entry + // (openhuman#6218). + let durable_tool_specs: Vec> = + tools.iter().map(|tool| Arc::new(tool.spec())).collect(); + let tool_specs: Vec> = durable_tool_specs + .iter() + .cloned() + .chain(synthesized_tools.iter().map(|tool| Arc::new(tool.spec()))) + .collect(); + + let mut visible_names = self.visible_tool_names.unwrap_or_default(); + // Resolved here rather than at its historical position below: the pack + // withholding is per-agent (a pack is skipped for the specialist that + // owns its family), so the id has to exist before the strip. + let agent_definition_name = self + .agent_definition_name + .clone() + .unwrap_or_else(|| "main".to_string()); + // On-demand tool disclosure: withhold packed tools' schemas from the + // provider and advertise `load_skill` / `use_skill` in their place. The + // tools stay in the registry below and stay executable — only the + // advertised surface shrinks. Applied here, before the policy filter, + // so the visible set and the policy session cannot disagree. + if visible_names.is_empty() { + visible_names = tools + .iter() + .chain(synthesized_tools.iter()) + .map(|tool| tool.name().to_string()) + .collect(); + } + crate::openhuman::tools::toolpacks::strip_packed_from_visible( + &mut visible_names, + &agent_definition_name, + ); + let config = self.config.clone().unwrap_or_default(); + let event_session_id = self + .event_session_id + .clone() + .unwrap_or_else(|| "standalone".to_string()); + let event_channel = self + .event_channel + .clone() + .unwrap_or_else(|| "internal".to_string()); + // Classify both sets: a synthesised delegate needs a decision too. + let all_tools: Vec<&dyn Tool> = tools + .iter() + .chain(synthesized_tools.iter()) + .map(|tool| tool.as_ref()) + .collect(); + let tool_policy_session = ToolPolicyEngine::build_session_from_refs( + &agent_definition_name, + &event_channel, + "session", + &config.channel_permissions, + &all_tools, + &visible_names, + ); + + // A child agent inherits explicit profile and channel restrictions, but + // not the primary agent's own role-specific tool scope. The Master Agent + // can write directly, while specialists may still need tools outside its + // intentionally compact default surface. Conflating those two surfaces + // silently strips specialist capabilities (#5118 merge). + // + // Build a second policy snapshot without the role visibility filter. + // `tool_policy_session` marks both channel-blocked and role-hidden tools + // as restricted, so deriving the child ceiling from it would reintroduce + // exactly that conflation. + let channel_policy_session = ToolPolicyEngine::build_session_from_refs( + &agent_definition_name, + &event_channel, + "session", + &config.channel_permissions, + &all_tools, + &std::collections::HashSet::new(), + ); + let mut subagent_tool_ceiling_names = self.subagent_tool_ceiling_names.unwrap_or_default(); + if channel_policy_session.has_restrictions() { + let policy_allowed: std::collections::HashSet = tool_specs + .iter() + .filter(|spec| channel_policy_session.is_allowed(&spec.name)) + .map(|spec| spec.name.clone()) + .collect(); + if subagent_tool_ceiling_names.is_empty() { + subagent_tool_ceiling_names = policy_allowed; + } else { + subagent_tool_ceiling_names.retain(|name| policy_allowed.contains(name)); + if subagent_tool_ceiling_names.is_empty() { + subagent_tool_ceiling_names.insert("__subagent_no_tools__".to_string()); + } + } + } + + // Build the filtered spec list that the main agent sends to the + // provider. The explicit visible-tool allowlist and the resolved + // channel permission policy must stay aligned so prompt-visible + // tools cannot exceed the runtime execution boundary. + let visible_tool_specs_unfiltered = + visible_tool_specs_for_policy(&tool_specs, &visible_names, &tool_policy_session); + + // Dedupe by tool name. Anthropic (and other strict providers) + // rejects a chat/completions request that lists two tools with + // the same name — OpenHuman's own backend and OpenAI silently + // accept duplicates, which hid this bug until #1710's per-role + // routing started sending the same tool list to Anthropic. + let visible_tool_specs: Vec> = + dedup_visible_tool_specs(visible_tool_specs_unfiltered); + + let visible_names_list: Vec<&str> = + visible_tool_specs.iter().map(|s| s.name.as_str()).collect(); + log::info!( + "[agent] tool spec filter: total={} visible={} (filter_active={} policy_restricted={}) names=[{}]", + tool_specs.len(), + visible_tool_specs.len(), + !visible_names.is_empty(), + tool_policy_session.has_restrictions(), + visible_names_list.join(", ") + ); + + // Pull the model source out of the builder once; the Agent holds it and + // builds a fresh tiered crate `ChatModel` set from it per turn. + let turn_model_source = self + .turn_model_source + .ok_or_else(|| anyhow::anyhow!("provider is required"))?; + + let prompt_builder = self.prompt_builder.unwrap_or_else( + crate::openhuman::agent::context::prompt::SystemPromptBuilder::with_defaults, + ); + + let model_name = self + .model_name + .unwrap_or_else(|| crate::openhuman::config::DEFAULT_MODEL.into()); + + // Assemble the per-session ContextManager. The manager owns + // the prompt builder, the reduction pipeline, and the + // summarizer — every concern that touches "what's in the + // model's context window" routes through this single handle. + let context_config = self.context_config.unwrap_or_default(); + + // Live history reduction moved to the tinyagents graph + // (`ContextCompressionMiddleware` + `MessageTrimMiddleware`, issue + // #4249), so the session no longer constructs an in-turn summarizer + // here. The archivist hook still drives durable segment recaps on its + // own post-turn path; it is no longer coupled to context compaction. + let context = ContextManager::new(&context_config, prompt_builder); + + let workspace_dir = self + .workspace_dir + .unwrap_or_else(|| std::path::PathBuf::from(".")); + let action_dir = self.action_dir.unwrap_or_else(|| workspace_dir.clone()); + let memory_subdir = self.memory_subdir.unwrap_or_else(|| "memory".to_string()); + let session_raw_subdir = self + .session_raw_subdir + .unwrap_or_else(|| "session_raw".to_string()); + + let tools = Arc::new(tools); + // The pack tools live inside this registry, so they can only be pointed + // at it once it exists. Re-bind after any later rebuild of this `Arc`. + crate::openhuman::tools::toolpacks::bind_pack_registry(&tools); + + Ok(Agent { + turn_model_source, + tools, + synthesized_tools: Arc::new(synthesized_tools), + tool_specs: Arc::new(tool_specs), + durable_tool_specs: Arc::new(durable_tool_specs), + visible_tool_specs: Arc::new(visible_tool_specs), + visible_tool_names: visible_names, + subagent_tool_ceiling_names, + tool_policy_session, + memory: self + .memory + .ok_or_else(|| anyhow::anyhow!("memory is required"))?, + shared_experience_memory: self.shared_experience_memory, + auto_recall: self.auto_recall, + tool_dispatcher: std::sync::Arc::from( + self.tool_dispatcher + .ok_or_else(|| anyhow::anyhow!("tool_dispatcher is required"))?, + ), + config, + model_name, + model_vision: self.model_vision.unwrap_or(false), + temperature: self.temperature.unwrap_or(0.7), + workspace_dir, + action_dir, + workspace_descriptor: self.workspace_descriptor, + workflows: self.workflows.unwrap_or_default(), + auto_save: self.auto_save.unwrap_or(false), + last_memory_context: None, + last_turn_citations: Vec::new(), + pending_citations: None, + last_turn_usage_totals: None, + last_turn_hit_cap: false, + history: Vec::new(), + post_turn_hooks: self.post_turn_hooks, + learning_enabled: self.learning_enabled, + explicit_preferences_enabled: self.explicit_preferences_enabled, + event_session_id, + event_channel, + agent_definition_name: agent_definition_name.clone(), + // Canonical registry id — captured here at build time + // before any caller can call `set_agent_definition_name` + // and clobber the transcript-facing name. Used by + // `refresh_delegation_tools` to re-resolve the agent's + // `subagents` declaration against the global registry. + agent_definition_id: agent_definition_name.clone(), + active_profile_id: self.active_profile_id, + personality_soul_md: self.personality_soul_md, + personality_memory_md: self.personality_memory_md, + memory_subdir, + session_raw_subdir, + session_transcript_path: None, + session_history: None, + session_history_locator: self.session_history_locator, + persisted_transcript_messages: Vec::new(), + session_key: { + let unix_ts = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let sanitized: String = agent_definition_name + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '_' || c == '-' { + c + } else { + '_' + } + }) + .collect(); + format!("{unix_ts}_{sanitized}") + }, + session_parent_prefix: self.session_parent_prefix, + cached_transcript_messages: None, + context, + on_progress: None, + run_queue: None, + connected_integrations: Vec::new(), + connected_integrations_initialized: false, + runtime_config: None, + // Default to `true` (omit) so legacy / custom agents built + // without a definition stay lean. Opt-in agents thread their + // `omit_profile = false` through the builder. + omit_profile: self.omit_profile.unwrap_or(true), + omit_memory_md: self.omit_memory_md.unwrap_or(true), + payload_summarizer: self.payload_summarizer, + trigger_memory_agent: self.trigger_memory_agent.unwrap_or_default(), + tokenjuice_compression: self.tokenjuice_compression, + tool_policy: self.tool_policy.unwrap_or_else(|| { + Arc::new(crate::openhuman::agent::tool_policy::AllowAllToolPolicy) + }), + last_seen_integrations_hash: 0, + composio_integrations_rx: None, + skill_events_rx: None, + announced_integrations: std::collections::HashSet::new(), + pending_integration_announcement: Vec::new(), + announced_mcp_servers: std::collections::HashSet::new(), + pending_mcp_announcement: Vec::new(), + announced_skills: std::collections::HashSet::new(), + pending_skill_announcement: Vec::new(), + pending_skill_retraction: Vec::new(), + archivist_hook: self.archivist_hook, + synthesized_tool_names, + pending_turn_overrides: super::super::types::TurnOverrides::default(), + }) + } +} diff --git a/src/openhuman/agent/harness/session/builder/builder_tests.rs b/src/openhuman/agent/harness/session/builder/builder_tests.rs index c9004fd23a..8518ad5bdf 100644 --- a/src/openhuman/agent/harness/session/builder/builder_tests.rs +++ b/src/openhuman/agent/harness/session/builder/builder_tests.rs @@ -70,6 +70,8 @@ mod part_01_tests; mod part_02_tests; #[path = "builder_tests_part_03_tests.rs"] mod part_03_tests; +#[path = "builder_tests_part_04_tests.rs"] +mod part_04_tests; // ── load_skill's advertised spec is scoped to the session ─────────────────── @@ -126,9 +128,11 @@ fn load_skill_spec_from_registry() -> ToolSpec { /// nobody calls advertises every pack exactly as before. #[test] fn visible_specs_scope_load_skills_index_to_the_session() { - let specs = vec![ - load_skill_spec_from_registry(), - spec(crate::openhuman::tools::toolpacks::USE_SKILL), + // `Arc` leaves, because the three spec views share them — the assertions + // below are unchanged, only the carrier is. + let specs: Vec> = vec![ + std::sync::Arc::new(load_skill_spec_from_registry()), + std::sync::Arc::new(spec(crate::openhuman::tools::toolpacks::USE_SKILL)), ]; let visible: std::collections::HashSet = specs.iter().map(|s| s.name.clone()).collect(); // Reachable: one workflows tool. Everything else in every other pack is @@ -169,9 +173,11 @@ fn visible_specs_scope_load_skills_index_to_the_session() { /// A session that can reach no pack at all should not carry the pack tools. #[test] fn visible_specs_drop_the_pack_tools_when_no_pack_is_reachable() { - let specs = vec![ - load_skill_spec_from_registry(), - spec(crate::openhuman::tools::toolpacks::USE_SKILL), + // `Arc` leaves, because the three spec views share them — the assertions + // below are unchanged, only the carrier is. + let specs: Vec> = vec![ + std::sync::Arc::new(load_skill_spec_from_registry()), + std::sync::Arc::new(spec(crate::openhuman::tools::toolpacks::USE_SKILL)), ]; let visible: std::collections::HashSet = specs.iter().map(|s| s.name.clone()).collect(); let session = session_allowing(&[ @@ -242,12 +248,14 @@ fn a_realistic_withheld_session_keeps_its_packs_advertised() { &visible, ); - let specs: Vec = tools + let specs: Vec> = tools .iter() - .map(|t| ToolSpec { - name: t.name().to_string(), - description: t.description().to_string(), - parameters: t.parameters_schema(), + .map(|t| { + std::sync::Arc::new(ToolSpec { + name: t.name().to_string(), + description: t.description().to_string(), + parameters: t.parameters_schema(), + }) }) .collect(); diff --git a/src/openhuman/agent/harness/session/builder/builder_tests_part_04_tests.rs b/src/openhuman/agent/harness/session/builder/builder_tests_part_04_tests.rs new file mode 100644 index 0000000000..31ccba6a6d --- /dev/null +++ b/src/openhuman/agent/harness/session/builder/builder_tests_part_04_tests.rs @@ -0,0 +1,140 @@ +use super::*; +/// The three spec views an agent keeps share their leaf schemas. +/// +/// `durable_tool_specs` is a prefix of `tool_specs`, and `visible_tool_specs` +/// is a filtered subset of it. Before openhuman#6218 each was an independent +/// `Vec`, so every JSON-Schema `parameters` value was resident up to +/// three times per live agent — ~1.1 MiB of the ~2.5 MiB marginal cost of a +/// `fleet` agent. Pointer identity is the property that keeps it at one copy, +/// so assert it directly rather than asserting equal contents (which the old +/// deep-cloning shape also satisfied). +/// +/// One spec is exempt by design — see the `load_skill` branch below. +#[test] +fn the_three_spec_views_share_their_leaf_schemas() { + crate::openhuman::agent::harness::AgentDefinitionRegistry::init_global_builtins().unwrap(); + let tmp = tempfile::TempDir::new().unwrap(); + let config = test_config(&tmp); + + let agent = crate::openhuman::agent::Agent::from_config_for_agent(&config, "orchestrator") + .expect("orchestrator session build"); + + let durable = agent.durable_tool_specs_arc(); + let all = agent.tool_specs_arc(); + let visible = agent.visible_tool_specs_arc(); + + assert!( + !durable.is_empty(), + "the orchestrator carries a durable registry, so there is something to share" + ); + assert!( + durable.len() <= all.len(), + "the durable set is a prefix of the full set" + ); + + for (i, spec) in durable.iter().enumerate() { + assert!( + std::sync::Arc::ptr_eq(spec, &all[i]), + "durable spec `{}` must be the same allocation as the full view's entry, \ + not a deep copy", + spec.name + ); + } + + assert!( + !visible.is_empty(), + "the orchestrator advertises tools, so the visible view is non-empty" + ); + // `load_skill` is the one deliberate exception, and it is deliberate in the + // other direction: `visible_tool_specs_for_policy` rewrites its pack index + // and `skill` enum down to the packs THIS session can actually call, so the + // visible entry must NOT be the durable one — `durable_tool_specs` stays the + // unscoped truth, and sharing the leaf would scope it for every view that + // holds it. `use_skill` is filtered wholesale, never rewritten, so it shares + // like everything else. + // + // Asserted rather than merely excluded: a future change that made the + // rewrite mutate in place would silently scope the durable set, and a + // change that deep-copied everything again would still pass an + // exclusion-only test. + let load_skill = crate::openhuman::tools::toolpacks::LOAD_SKILL; + let mut saw_scoped_load_skill = false; + + for spec in visible.iter() { + let shared = all + .iter() + .any(|candidate| std::sync::Arc::ptr_eq(candidate, spec)); + if spec.name == load_skill { + assert!( + !shared, + "`{load_skill}` must be scoped into its own allocation — sharing the leaf would rewrite the durable set's copy too" + ); + saw_scoped_load_skill = true; + continue; + } + assert!( + shared, + "visible spec `{}` must point at the full view's allocation, not a deep copy", + spec.name + ); + } + + assert!( + saw_scoped_load_skill, + "the orchestrator advertises `{load_skill}`, so the scoped-copy exception above must actually have been exercised rather than vacuously skipped" + ); +} + +/// Failure path: a duplicate name must not smuggle a *different* allocation +/// through the dedup. +/// +/// `dedup_visible_tool_specs` keeps the first occurrence. With shared leaves +/// the survivor must still be the exact entry that was handed in — a helper +/// that rebuilt the kept spec would reintroduce the per-agent copy the sharing +/// exists to remove, while every content-equality assertion still passed. +#[test] +fn dedup_keeps_the_original_allocation_of_the_winning_spec() { + let first = std::sync::Arc::new(spec("research")); + let mut shadow = spec("research"); + shadow.description = "the delegate that must lose".to_string(); + let shadow = std::sync::Arc::new(shadow); + let other = std::sync::Arc::new(spec("plan")); + + let deduped = dedup_visible_tool_specs(vec![ + std::sync::Arc::clone(&first), + std::sync::Arc::clone(&other), + std::sync::Arc::clone(&shadow), + ]); + + assert_eq!(deduped.len(), 2, "the shadowing duplicate must be dropped"); + assert!( + std::sync::Arc::ptr_eq(&deduped[0], &first), + "the surviving `research` spec must be the first allocation, not a rebuild" + ); + assert!( + !std::sync::Arc::ptr_eq(&deduped[0], &shadow), + "the shadowing delegate's schema must not reach the provider" + ); + assert!(std::sync::Arc::ptr_eq(&deduped[1], &other)); +} + +/// Failure path: an agent with no tools at all must still build, and its three +/// spec views must be empty rather than desynchronised. +#[test] +fn spec_views_stay_consistent_for_a_tool_less_agent() { + let model: std::sync::Arc> = + std::sync::Arc::new(tinyagents_harness::testkit::ScriptedModel::new(Vec::new())); + let agent = crate::openhuman::agent::AgentBuilder::new() + .chat_model(model) + .tools(Vec::new()) + .memory(crate::openhuman::memory::test_support::noop_memory()) + .tool_dispatcher(Box::new( + crate::openhuman::agent::dispatcher::XmlToolDispatcher, + )) + .build() + .expect("a tool-less agent is a legal build"); + + assert!(agent.tool_specs().is_empty()); + assert!(agent.durable_tool_specs_arc().is_empty()); + assert!(agent.visible_tool_specs_arc().is_empty()); +} diff --git a/src/openhuman/agent/harness/session/builder/factory.rs b/src/openhuman/agent/harness/session/builder/factory.rs index 2e443c046f..6c12f8cae7 100644 --- a/src/openhuman/agent/harness/session/builder/factory.rs +++ b/src/openhuman/agent/harness/session/builder/factory.rs @@ -740,7 +740,7 @@ impl Agent { archivist_provider, true, ) - .with_config(config.clone()), + .with_config(Arc::clone(&base_config)), ); post_turn_hooks .push(Arc::clone(&hook) as Arc); @@ -1245,7 +1245,11 @@ impl Agent { let connected_integrations_initialized = prewarmed_integrations.is_some(); agent.connected_integrations = prewarmed_integrations.unwrap_or_default(); agent.connected_integrations_initialized = connected_integrations_initialized; - agent.runtime_config = Some(Arc::new(config.clone())); + // The same snapshot `base_config` already holds — `Config` is immutable + // after construction, so a second deep clone bought nothing but a + // second resident copy of a 95-field struct with nested `Vec`s + // (openhuman#6218). + agent.runtime_config = Some(Arc::clone(&base_config)); agent.last_seen_integrations_hash = crate::openhuman::integrations::composio::connected_set_hash( &agent.connected_integrations, diff --git a/src/openhuman/agent/harness/session/builder/mod.rs b/src/openhuman/agent/harness/session/builder/mod.rs index 7cb04362bf..bcbc3290d8 100644 --- a/src/openhuman/agent/harness/session/builder/mod.rs +++ b/src/openhuman/agent/harness/session/builder/mod.rs @@ -6,6 +6,7 @@ //! registry from a loaded [`Config`]. Per-turn behaviour lives in //! [`super::turn`]; accessors and run-helpers live in [`super::runtime`]. +mod builder_build; mod factory; mod helpers; mod setters; @@ -16,6 +17,7 @@ mod builder_tests; use crate::openhuman::agent::harness::definition::{AgentDefinition, ToolScope}; use crate::openhuman::tools::agent_policy::ToolPolicySession; use crate::openhuman::tools::{Tool, ToolSpec}; +use std::sync::Arc; /// Drop entries with duplicate `name` fields, first occurrence wins. /// @@ -30,15 +32,21 @@ use crate::openhuman::tools::{Tool, ToolSpec}; /// list — initial build, post-composio refresh, scope-filter change — /// so the request the provider sees is always name-unique regardless /// of which path produced it. -pub(crate) fn dedup_visible_tool_specs(specs: Vec) -> Vec { +/// +/// Generic over the element type so the two carriers of a spec list share one +/// implementation: the main agent holds `Arc` (the three spec views +/// share their leaves), while the sub-agent assembly still materialises owned +/// `ToolSpec`s for the public `AgentTurnRequest`. +pub(crate) fn dedup_visible_tool_specs>(specs: Vec) -> Vec { let mut seen: std::collections::HashSet = std::collections::HashSet::new(); - let mut deduped: Vec = Vec::with_capacity(specs.len()); + let mut deduped: Vec = Vec::with_capacity(specs.len()); let mut dropped: Vec = Vec::new(); for spec in specs { - if seen.insert(spec.name.clone()) { + let name = spec.borrow().name.clone(); + if seen.insert(name.clone()) { deduped.push(spec); } else { - dropped.push(spec.name); + dropped.push(name); } } if !dropped.is_empty() { @@ -87,10 +95,10 @@ pub(crate) fn drop_synthesized_name_collisions( } pub(super) fn visible_tool_specs_for_policy( - tool_specs: &[ToolSpec], + tool_specs: &[Arc], visible_names: &std::collections::HashSet, tool_policy: &ToolPolicySession, -) -> Vec { +) -> Vec> { // `load_skill`'s description carries the pack index, and its `skill` enum // carries the pack ids. Both are built once in `LoadSkillTool::new`, before // any session exists, so every agent was told all ten packs were loadable — @@ -119,8 +127,14 @@ pub(super) fn visible_tool_specs_for_policy( if spec.name == crate::openhuman::tools::toolpacks::LOAD_SKILL { // `false` means no pack has a callable tool: an empty index and // an empty enum are not a tool, so drop it rather than ship one. + // `Arc::make_mut`, not `&mut spec`: the three spec views share + // their leaves, so rewriting the pack index through the `Arc` + // would rewrite it for every view that holds this schema — and + // `durable_tool_specs` is meant to stay the unscoped truth. + // This copies exactly the one spec being rewritten and leaves + // the other ~48 visible schemas shared. return crate::openhuman::tools::toolpacks::scope_load_skill_spec( - &mut spec, + Arc::make_mut(&mut spec), &is_callable, ) .then_some(spec); diff --git a/src/openhuman/agent/harness/session/builder/setters.rs b/src/openhuman/agent/harness/session/builder/setters.rs index b2754cd8bc..2c1656530a 100644 --- a/src/openhuman/agent/harness/session/builder/setters.rs +++ b/src/openhuman/agent/harness/session/builder/setters.rs @@ -1,17 +1,11 @@ -//! `AgentBuilder` fluent setters and the `build()` validator. -//! -//! All setter methods return `Self` for chaining. `build()` validates that -//! required fields are present and assembles the final [`Agent`]. - -use super::{dedup_visible_tool_specs, visible_tool_specs_for_policy}; -use crate::openhuman::agent::context::ContextManager; -use crate::openhuman::agent::harness::session::types::{Agent, AgentBuilder}; +//! `AgentBuilder` fluent setters. See `builder_build.rs` for the `build()` +//! validator that assembles the final `Agent`. + +use crate::openhuman::agent::harness::session::types::AgentBuilder; use crate::openhuman::agent::harness::TriggerMemoryAgent; use crate::openhuman::config::ContextConfig; use crate::openhuman::memory::Memory; -use crate::openhuman::tools::agent_policy::ToolPolicyEngine; -use crate::openhuman::tools::{Tool, ToolSpec}; -use anyhow::Result; +use crate::openhuman::tools::Tool; use std::sync::Arc; impl AgentBuilder { @@ -459,288 +453,4 @@ impl AgentBuilder { self.tokenjuice_compression = profile; self } - - /// Validates the configuration and constructs a new `Agent` instance. - /// - /// This method is responsible for wiring together the provided components, - /// setting up the context manager, and initializing the conversation history. - /// It ensures that all required fields (provider, tools, memory, etc.) are present. - pub fn build(self) -> Result { - let tools = self - .tools - .ok_or_else(|| anyhow::anyhow!("tools are required"))?; - // The synthesised set lives beside the durable registry, never inside - // it (`Agent::synthesized_tools`); a durable name wins a collision. - let synthesized_tools = super::drop_synthesized_name_collisions( - &tools, - self.synthesized_tools.unwrap_or_default(), - ); - let synthesized_tool_names: std::collections::HashSet = synthesized_tools - .iter() - .map(|tool| tool.name().to_string()) - .collect(); - // Durable specs first, synthesised after — every reader's order. - let durable_tool_specs: Vec = tools.iter().map(|tool| tool.spec()).collect(); - let tool_specs: Vec = durable_tool_specs - .iter() - .cloned() - .chain(synthesized_tools.iter().map(|tool| tool.spec())) - .collect(); - - let mut visible_names = self.visible_tool_names.unwrap_or_default(); - // Resolved here rather than at its historical position below: the pack - // withholding is per-agent (a pack is skipped for the specialist that - // owns its family), so the id has to exist before the strip. - let agent_definition_name = self - .agent_definition_name - .clone() - .unwrap_or_else(|| "main".to_string()); - // On-demand tool disclosure: withhold packed tools' schemas from the - // provider and advertise `load_skill` / `use_skill` in their place. The - // tools stay in the registry below and stay executable — only the - // advertised surface shrinks. Applied here, before the policy filter, - // so the visible set and the policy session cannot disagree. - if visible_names.is_empty() { - visible_names = tools - .iter() - .chain(synthesized_tools.iter()) - .map(|tool| tool.name().to_string()) - .collect(); - } - crate::openhuman::tools::toolpacks::strip_packed_from_visible( - &mut visible_names, - &agent_definition_name, - ); - let config = self.config.clone().unwrap_or_default(); - let event_session_id = self - .event_session_id - .clone() - .unwrap_or_else(|| "standalone".to_string()); - let event_channel = self - .event_channel - .clone() - .unwrap_or_else(|| "internal".to_string()); - // Classify both sets: a synthesised delegate needs a decision too. - let all_tools: Vec<&dyn Tool> = tools - .iter() - .chain(synthesized_tools.iter()) - .map(|tool| tool.as_ref()) - .collect(); - let tool_policy_session = ToolPolicyEngine::build_session_from_refs( - &agent_definition_name, - &event_channel, - "session", - &config.channel_permissions, - &all_tools, - &visible_names, - ); - - // A child agent inherits explicit profile and channel restrictions, but - // not the primary agent's own role-specific tool scope. The Master Agent - // can write directly, while specialists may still need tools outside its - // intentionally compact default surface. Conflating those two surfaces - // silently strips specialist capabilities (#5118 merge). - // - // Build a second policy snapshot without the role visibility filter. - // `tool_policy_session` marks both channel-blocked and role-hidden tools - // as restricted, so deriving the child ceiling from it would reintroduce - // exactly that conflation. - let channel_policy_session = ToolPolicyEngine::build_session_from_refs( - &agent_definition_name, - &event_channel, - "session", - &config.channel_permissions, - &all_tools, - &std::collections::HashSet::new(), - ); - let mut subagent_tool_ceiling_names = self.subagent_tool_ceiling_names.unwrap_or_default(); - if channel_policy_session.has_restrictions() { - let policy_allowed: std::collections::HashSet = tool_specs - .iter() - .filter(|spec| channel_policy_session.is_allowed(&spec.name)) - .map(|spec| spec.name.clone()) - .collect(); - if subagent_tool_ceiling_names.is_empty() { - subagent_tool_ceiling_names = policy_allowed; - } else { - subagent_tool_ceiling_names.retain(|name| policy_allowed.contains(name)); - if subagent_tool_ceiling_names.is_empty() { - subagent_tool_ceiling_names.insert("__subagent_no_tools__".to_string()); - } - } - } - - // Build the filtered spec list that the main agent sends to the - // provider. The explicit visible-tool allowlist and the resolved - // channel permission policy must stay aligned so prompt-visible - // tools cannot exceed the runtime execution boundary. - let visible_tool_specs_unfiltered = - visible_tool_specs_for_policy(&tool_specs, &visible_names, &tool_policy_session); - - // Dedupe by tool name. Anthropic (and other strict providers) - // rejects a chat/completions request that lists two tools with - // the same name — OpenHuman's own backend and OpenAI silently - // accept duplicates, which hid this bug until #1710's per-role - // routing started sending the same tool list to Anthropic. - let visible_tool_specs: Vec = - dedup_visible_tool_specs(visible_tool_specs_unfiltered); - - let visible_names_list: Vec<&str> = - visible_tool_specs.iter().map(|s| s.name.as_str()).collect(); - log::info!( - "[agent] tool spec filter: total={} visible={} (filter_active={} policy_restricted={}) names=[{}]", - tool_specs.len(), - visible_tool_specs.len(), - !visible_names.is_empty(), - tool_policy_session.has_restrictions(), - visible_names_list.join(", ") - ); - - // Pull the model source out of the builder once; the Agent holds it and - // builds a fresh tiered crate `ChatModel` set from it per turn. - let turn_model_source = self - .turn_model_source - .ok_or_else(|| anyhow::anyhow!("provider is required"))?; - - let prompt_builder = self.prompt_builder.unwrap_or_else( - crate::openhuman::agent::context::prompt::SystemPromptBuilder::with_defaults, - ); - - let model_name = self - .model_name - .unwrap_or_else(|| crate::openhuman::config::DEFAULT_MODEL.into()); - - // Assemble the per-session ContextManager. The manager owns - // the prompt builder, the reduction pipeline, and the - // summarizer — every concern that touches "what's in the - // model's context window" routes through this single handle. - let context_config = self.context_config.unwrap_or_default(); - - // Live history reduction moved to the tinyagents graph - // (`ContextCompressionMiddleware` + `MessageTrimMiddleware`, issue - // #4249), so the session no longer constructs an in-turn summarizer - // here. The archivist hook still drives durable segment recaps on its - // own post-turn path; it is no longer coupled to context compaction. - let context = ContextManager::new(&context_config, prompt_builder); - - let workspace_dir = self - .workspace_dir - .unwrap_or_else(|| std::path::PathBuf::from(".")); - let action_dir = self.action_dir.unwrap_or_else(|| workspace_dir.clone()); - let memory_subdir = self.memory_subdir.unwrap_or_else(|| "memory".to_string()); - let session_raw_subdir = self - .session_raw_subdir - .unwrap_or_else(|| "session_raw".to_string()); - - let tools = Arc::new(tools); - // The pack tools live inside this registry, so they can only be pointed - // at it once it exists. Re-bind after any later rebuild of this `Arc`. - crate::openhuman::tools::toolpacks::bind_pack_registry(&tools); - - Ok(Agent { - turn_model_source, - tools, - synthesized_tools: Arc::new(synthesized_tools), - tool_specs: Arc::new(tool_specs), - durable_tool_specs: Arc::new(durable_tool_specs), - visible_tool_specs: Arc::new(visible_tool_specs), - visible_tool_names: visible_names, - subagent_tool_ceiling_names, - tool_policy_session, - memory: self - .memory - .ok_or_else(|| anyhow::anyhow!("memory is required"))?, - shared_experience_memory: self.shared_experience_memory, - auto_recall: self.auto_recall, - tool_dispatcher: std::sync::Arc::from( - self.tool_dispatcher - .ok_or_else(|| anyhow::anyhow!("tool_dispatcher is required"))?, - ), - config, - model_name, - model_vision: self.model_vision.unwrap_or(false), - temperature: self.temperature.unwrap_or(0.7), - workspace_dir, - action_dir, - workspace_descriptor: self.workspace_descriptor, - workflows: self.workflows.unwrap_or_default(), - auto_save: self.auto_save.unwrap_or(false), - last_memory_context: None, - last_turn_citations: Vec::new(), - pending_citations: None, - last_turn_usage_totals: None, - last_turn_hit_cap: false, - history: Vec::new(), - post_turn_hooks: self.post_turn_hooks, - learning_enabled: self.learning_enabled, - explicit_preferences_enabled: self.explicit_preferences_enabled, - event_session_id, - event_channel, - agent_definition_name: agent_definition_name.clone(), - // Canonical registry id — captured here at build time - // before any caller can call `set_agent_definition_name` - // and clobber the transcript-facing name. Used by - // `refresh_delegation_tools` to re-resolve the agent's - // `subagents` declaration against the global registry. - agent_definition_id: agent_definition_name.clone(), - active_profile_id: self.active_profile_id, - personality_soul_md: self.personality_soul_md, - personality_memory_md: self.personality_memory_md, - memory_subdir, - session_raw_subdir, - session_transcript_path: None, - session_history: None, - session_history_locator: self.session_history_locator, - persisted_transcript_messages: Vec::new(), - session_key: { - let unix_ts = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); - let sanitized: String = agent_definition_name - .chars() - .map(|c| { - if c.is_ascii_alphanumeric() || c == '_' || c == '-' { - c - } else { - '_' - } - }) - .collect(); - format!("{unix_ts}_{sanitized}") - }, - session_parent_prefix: self.session_parent_prefix, - cached_transcript_messages: None, - context, - on_progress: None, - run_queue: None, - connected_integrations: Vec::new(), - connected_integrations_initialized: false, - runtime_config: None, - // Default to `true` (omit) so legacy / custom agents built - // without a definition stay lean. Opt-in agents thread their - // `omit_profile = false` through the builder. - omit_profile: self.omit_profile.unwrap_or(true), - omit_memory_md: self.omit_memory_md.unwrap_or(true), - payload_summarizer: self.payload_summarizer, - trigger_memory_agent: self.trigger_memory_agent.unwrap_or_default(), - tokenjuice_compression: self.tokenjuice_compression, - tool_policy: self.tool_policy.unwrap_or_else(|| { - Arc::new(crate::openhuman::agent::tool_policy::AllowAllToolPolicy) - }), - last_seen_integrations_hash: 0, - composio_integrations_rx: None, - skill_events_rx: None, - announced_integrations: std::collections::HashSet::new(), - pending_integration_announcement: Vec::new(), - announced_mcp_servers: std::collections::HashSet::new(), - pending_mcp_announcement: Vec::new(), - announced_skills: std::collections::HashSet::new(), - pending_skill_announcement: Vec::new(), - pending_skill_retraction: Vec::new(), - archivist_hook: self.archivist_hook, - synthesized_tool_names, - pending_turn_overrides: super::super::types::TurnOverrides::default(), - }) - } } diff --git a/src/openhuman/agent/harness/session/runtime_impl_01_part_01.rs b/src/openhuman/agent/harness/session/runtime_impl_01_part_01.rs index dfb80caee4..70cb96572b 100644 --- a/src/openhuman/agent/harness/session/runtime_impl_01_part_01.rs +++ b/src/openhuman/agent/harness/session/runtime_impl_01_part_01.rs @@ -88,25 +88,25 @@ impl Agent { /// Borrow the agent's tool specs (pre-serialised). Captured at /// turn-start so sub-agents can pass byte-identical schemas to the /// provider for prefix-cache reuse. - pub fn tool_specs(&self) -> &[ToolSpec] { + pub fn tool_specs(&self) -> &[Arc] { self.tool_specs.as_slice() } /// Clone the agent's full tool specs `Arc` (durable and synthesised). - pub fn tool_specs_arc(&self) -> Arc> { + pub fn tool_specs_arc(&self) -> Arc>> { Arc::clone(&self.tool_specs) } /// Clone the agent's provider-facing spec list: visible, policy-allowed, /// de-duplicated, synthesised delegates included. - pub fn visible_tool_specs_arc(&self) -> Arc> { + pub fn visible_tool_specs_arc(&self) -> Arc>> { Arc::clone(&self.visible_tool_specs) } /// Clone the specs of the durable registry alone, index for index with /// [`Self::tools_arc`] — the pair a sub-agent is handed, so a child never /// sees a spec for a synthesised delegate it holds no instance for. - pub fn durable_tool_specs_arc(&self) -> Arc> { + pub fn durable_tool_specs_arc(&self) -> Arc>> { Arc::clone(&self.durable_tool_specs) } diff --git a/src/openhuman/agent/harness/session/session_tests_part_02_tests.rs b/src/openhuman/agent/harness/session/session_tests_part_02_tests.rs index 186033f88e..9ccfb756a2 100644 --- a/src/openhuman/agent/harness/session/session_tests_part_02_tests.rs +++ b/src/openhuman/agent/harness/session/session_tests_part_02_tests.rs @@ -644,7 +644,7 @@ fn a_durable_tool_owning_a_delegate_name_wins_everywhere_across_refreshes() { .any(|tool| tool.name() == DELEGATE), "the colliding delegate must not be synthesised beside the durable tool" ); - let specs: Vec<&crate::openhuman::tools::ToolSpec> = agent + let specs: Vec<&std::sync::Arc> = agent .tool_specs() .iter() .filter(|spec| spec.name == DELEGATE) diff --git a/src/openhuman/agent/harness/session/turn/context.rs b/src/openhuman/agent/harness/session/turn/context.rs index 91068a0f81..84e931d2b7 100644 --- a/src/openhuman/agent/harness/session/turn/context.rs +++ b/src/openhuman/agent/harness/session/turn/context.rs @@ -281,9 +281,20 @@ impl Agent { /// instructions and learned context. pub fn build_system_prompt(&self, learned: LearnedContextData) -> Result { let tools_slice: &[Box] = self.tools.as_slice(); + // `visible_tool_specs` holds shared `Arc` leaves (they are the + // same schema objects the durable and full views point at), while the + // `ToolDispatcher` trait — which embedders implement — takes an owned + // `&[ToolSpec]`. Materialise a borrow-slice for the call: this is one + // transient copy per system-prompt build, not a per-agent resident one, + // and keeping it here is what lets the trait stay source-compatible. + let visible_specs_owned: Vec = self + .visible_tool_specs + .iter() + .map(|spec| spec.as_ref().clone()) + .collect(); let instructions = self .tool_dispatcher - .prompt_instructions_for_specs(self.visible_tool_specs.as_slice()) + .prompt_instructions_for_specs(&visible_specs_owned) .unwrap_or_else(|| self.tool_dispatcher.prompt_instructions(tools_slice)); // Adapt the agent's whole callable surface into the shared PromptTool // shape that every prompt-building call-site uses. Temporary vec diff --git a/src/openhuman/agent/harness/session/turn/tools.rs b/src/openhuman/agent/harness/session/turn/tools.rs index ae16d7e79b..2a9e847cc3 100644 --- a/src/openhuman/agent/harness/session/turn/tools.rs +++ b/src/openhuman/agent/harness/session/turn/tools.rs @@ -591,8 +591,8 @@ impl Agent { ); let synthed_names: std::collections::HashSet = synthed.iter().map(|t| t.name().to_string()).collect(); - let synthed_specs: Vec = - synthed.iter().map(|t| t.spec()).collect(); + let synthed_specs: Vec> = + synthed.iter().map(|t| Arc::new(t.spec())).collect(); // Skip mutation when neither the previous nor the next synthesis // produced any names — saves work on agents without dynamic diff --git a/src/openhuman/agent/harness/session/types.rs b/src/openhuman/agent/harness/session/types.rs index 8bfc05bcef..e9128ce154 100644 --- a/src/openhuman/agent/harness/session/types.rs +++ b/src/openhuman/agent/harness/session/types.rs @@ -115,17 +115,29 @@ pub struct Agent { pub(super) synthesized_tools: Arc>>, /// Full tool specs: [`Self::tools`]' specs first, then the synthesised /// half, which [`Agent::refresh_delegation_tools`] swaps in place. - pub(super) tool_specs: Arc>, + /// + /// The leaves are `Arc` and are **shared** with + /// [`Self::durable_tool_specs`] and [`Self::visible_tool_specs`]: all three + /// views point at the same schema objects, so a JSON-Schema `parameters` + /// value is resident once per agent rather than three times + /// (openhuman#6218 — it was ~1.1 MiB of the ~2.5 MiB a live agent cost). + /// `refresh_delegation_tools` preserves that: `Arc::make_mut` clones the + /// vector of pointers, never the schemas behind them. Anything that + /// rebuilds an entry instead of cloning its `Arc` silently reintroduces the + /// copy, which is why + /// `builder_tests::part_01_tests::the_three_spec_views_share_their_leaf_schemas` + /// asserts pointer identity rather than equal contents. + pub(super) tool_specs: Arc>>, /// The specs of [`Self::tools`] alone, index for index. Sub-agents receive /// these via [`ParentExecutionContext::all_tool_specs`] beside /// [`Self::tools`], so a child's spec list can never name a synthesised /// delegate it holds no instance for (#4452). Fixed for the life of the /// agent, like the registry it describes. - pub(super) durable_tool_specs: Arc>, + pub(super) durable_tool_specs: Arc>>, /// Tool specs filtered by the visible-tool allowlist and session /// permission policy. These are the specs actually sent to the /// provider in the main agent's chat requests. - pub(super) visible_tool_specs: Arc>, + pub(super) visible_tool_specs: Arc>>, /// When non-empty, only these tool names are visible in the main /// agent's prompt and callable by the main agent. Sub-agents intersect /// their per-definition scopes with the effective parent-visible set. diff --git a/src/openhuman/agent/harness/subagent_runner/ops/runner.rs b/src/openhuman/agent/harness/subagent_runner/ops/runner.rs index ca40044b4f..2c112fa97f 100644 --- a/src/openhuman/agent/harness/subagent_runner/ops/runner.rs +++ b/src/openhuman/agent/harness/subagent_runner/ops/runner.rs @@ -1247,7 +1247,7 @@ async fn run_typed_mode( filtered_specs.extend( allowed_indices .iter() - .map(|&i| parent.all_tool_specs[i].clone()), + .map(|&i| parent.all_tool_specs[i].as_ref().clone()), ); let mut allowed_names: HashSet = allowed_indices .iter() diff --git a/src/openhuman/agent/harness/subagent_runner/ops_tests.rs b/src/openhuman/agent/harness/subagent_runner/ops_tests.rs index 0afcc67ff7..a7a159235c 100644 --- a/src/openhuman/agent/harness/subagent_runner/ops_tests.rs +++ b/src/openhuman/agent/harness/subagent_runner/ops_tests.rs @@ -240,8 +240,8 @@ fn make_parent( provider: Arc>, tools: Vec>, ) -> ParentExecutionContext { - let tool_specs: Vec = - tools.iter().map(|t| t.spec()).collect(); + let tool_specs: Vec> = + tools.iter().map(|t| Arc::new(t.spec())).collect(); ParentExecutionContext { workspace_descriptor: None, agent_definition_id: "orchestrator".into(), diff --git a/src/openhuman/agent/orchestration/agent_teams/runtime_tests.rs b/src/openhuman/agent/orchestration/agent_teams/runtime_tests.rs index f8142d6bc6..41db823464 100644 --- a/src/openhuman/agent/orchestration/agent_teams/runtime_tests.rs +++ b/src/openhuman/agent/orchestration/agent_teams/runtime_tests.rs @@ -20,7 +20,7 @@ use crate::openhuman::agent::harness::definition::AgentDefinitionRegistry; use crate::openhuman::agent::harness::fork_context::{with_parent_context, ParentExecutionContext}; use crate::openhuman::config::{AgentConfig, Config}; use crate::openhuman::memory::{Memory, MemoryCategory, MemoryEntry, NamespaceSummary, RecallOpts}; -use crate::openhuman::tools::{Tool, ToolSpec}; +use crate::openhuman::tools::Tool; use tinyagents_session::run_ledger::{ self, AgentTeamMemberStatus, AgentTeamMemberUpsert, AgentTeamStatus, AgentTeamTaskStatus, AgentTeamTaskUpsert, AgentTeamUpsert, @@ -114,7 +114,7 @@ fn mock_parent(model: Arc>) -> ParentExecutionContext { allowed_subagent_ids: HashSet::new(), turn_model_source: crate::openhuman::agent::tinyagents::TurnModelSource::from_model(model), all_tools: Arc::new(Vec::>::new()), - all_tool_specs: Arc::new(Vec::::new()), + all_tool_specs: Arc::new(Vec::new()), visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), diff --git a/src/openhuman/agent/orchestration/ops_tests.rs b/src/openhuman/agent/orchestration/ops_tests.rs index e5fb2b76c3..865641f2e6 100644 --- a/src/openhuman/agent/orchestration/ops_tests.rs +++ b/src/openhuman/agent/orchestration/ops_tests.rs @@ -4,7 +4,7 @@ use crate::openhuman::agent::harness::definition::AgentDefinitionRegistry; use crate::openhuman::agent::harness::fork_context::{with_parent_context, ParentExecutionContext}; use crate::openhuman::config::AgentConfig; use crate::openhuman::memory::{Memory, MemoryCategory, MemoryEntry, NamespaceSummary, RecallOpts}; -use crate::openhuman::tools::{Tool, ToolSpec}; +use crate::openhuman::tools::Tool; use async_trait::async_trait; use parking_lot::Mutex; use std::sync::{ @@ -88,7 +88,7 @@ fn parent_context(model: Arc>) -> ParentExecutionContext { }, ), all_tools: Arc::new(Vec::>::new()), - all_tool_specs: Arc::new(Vec::::new()), + all_tool_specs: Arc::new(Vec::new()), visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), diff --git a/src/openhuman/agent/orchestration/tools/agent_prepare_context_part_01.rs b/src/openhuman/agent/orchestration/tools/agent_prepare_context_part_01.rs index d6df199ef9..2b55fab966 100644 --- a/src/openhuman/agent/orchestration/tools/agent_prepare_context_part_01.rs +++ b/src/openhuman/agent/orchestration/tools/agent_prepare_context_part_01.rs @@ -584,7 +584,9 @@ impl AgentPrepareContextTool { return String::new(); }; let visible = &parent.visible_tool_names; - let specs: &[crate::openhuman::tools::ToolSpec] = if parent.visible_tool_specs.is_empty() { + let specs: &[std::sync::Arc] = + if parent.visible_tool_specs.is_empty() + { &parent.all_tool_specs } else { &parent.visible_tool_specs diff --git a/src/openhuman/agent/orchestration/tools/agent_prepare_context_tests.rs b/src/openhuman/agent/orchestration/tools/agent_prepare_context_tests.rs index b14af61cb2..d82bb4362b 100644 --- a/src/openhuman/agent/orchestration/tools/agent_prepare_context_tests.rs +++ b/src/openhuman/agent/orchestration/tools/agent_prepare_context_tests.rs @@ -325,12 +325,15 @@ fn credits_exhausted_scout_failure_does_not_reach_sentry() { // ───────────────────────────────────────────────────────────────────────────── /// A spec with the given name and description; the schema is irrelevant here. -fn catalog_spec(name: &str, description: &str) -> crate::openhuman::tools::ToolSpec { - crate::openhuman::tools::ToolSpec { +fn catalog_spec( + name: &str, + description: &str, +) -> std::sync::Arc { + std::sync::Arc::new(crate::openhuman::tools::ToolSpec { name: name.to_string(), description: description.to_string(), parameters: serde_json::json!({"type": "object"}), - } + }) } /// A parent context whose inheritable registry (`all_tool_specs`) and own @@ -338,8 +341,8 @@ fn catalog_spec(name: &str, description: &str) -> crate::openhuman::tools::ToolS /// `visible_tool_names` is derived from the visible specs, as the turn /// builder derives it. fn parent_context_with_specs( - all_tool_specs: Vec, - visible_tool_specs: Vec, + all_tool_specs: Vec>, + visible_tool_specs: Vec>, ) -> crate::openhuman::agent::harness::fork_context::ParentExecutionContext { use std::sync::Arc; let workspace = tempfile::TempDir::new().expect("temp workspace"); diff --git a/src/openhuman/agent/orchestration/workflow_runs/engine_tests.rs b/src/openhuman/agent/orchestration/workflow_runs/engine_tests.rs index 3fba51bc99..98772b1984 100644 --- a/src/openhuman/agent/orchestration/workflow_runs/engine_tests.rs +++ b/src/openhuman/agent/orchestration/workflow_runs/engine_tests.rs @@ -29,7 +29,7 @@ use crate::openhuman::agent::harness::definition::AgentDefinitionRegistry; use crate::openhuman::agent::harness::fork_context::{with_parent_context, ParentExecutionContext}; use crate::openhuman::config::{AgentConfig, Config}; use crate::openhuman::memory::{Memory, MemoryCategory, MemoryEntry, NamespaceSummary, RecallOpts}; -use crate::openhuman::tools::{Tool, ToolSpec}; +use crate::openhuman::tools::Tool; use tinyagents_session::run_ledger::{get_workflow_run, upsert_workflow_run, WorkflowRunUpsert}; use tinyinference::model::{ChatModel, ModelProfile, ModelRequest, ModelResponse}; @@ -176,7 +176,7 @@ fn mock_parent(model: Arc>) -> ParentExecutionContext { }, ), all_tools: Arc::new(Vec::>::new()), - all_tool_specs: Arc::new(Vec::::new()), + all_tool_specs: Arc::new(Vec::new()), visible_tool_specs: Arc::new(Vec::new()), visible_tool_names: std::collections::HashSet::new(), subagent_tool_ceiling_names: std::collections::HashSet::new(), diff --git a/src/openhuman/mcp/server/tools/dispatch.rs b/src/openhuman/mcp/server/tools/dispatch.rs index 4c9a139c20..222b5b2cdb 100644 --- a/src/openhuman/mcp/server/tools/dispatch.rs +++ b/src/openhuman/mcp/server/tools/dispatch.rs @@ -237,7 +237,11 @@ async fn list_core_tools() -> Result { async fn core_tool_instructions() -> Result { let agent = build_orchestrator_agent().await?; - let schemas: Vec<_> = agent.tool_specs().iter().map(spec_to_schema).collect(); + let schemas: Vec<_> = agent + .tool_specs() + .iter() + .map(|spec| spec_to_schema(spec)) + .collect(); Ok(tool_text_success( tinyagents_harness::tool::prompt_tool_instructions(&schemas), )) diff --git a/tests/calendar_grounding_e2e.rs b/tests/calendar_grounding_e2e.rs index f8f4a36d96..1a4bcd75bc 100644 --- a/tests/calendar_grounding_e2e.rs +++ b/tests/calendar_grounding_e2e.rs @@ -162,7 +162,7 @@ async fn test_integrations_agent_has_current_date_context() -> Result<()> { turn_model_source: openhuman_core::openhuman::agent::tinyagents::TurnModelSource::from_model(model), all_tools: Arc::new(vec![Box::new(MockCalendarTool)]), - all_tool_specs: Arc::new(vec![MockCalendarTool.spec()]), + all_tool_specs: Arc::new(vec![Arc::new(MockCalendarTool.spec())]), // #6145: empty means "same surface as `all_tool_specs`" — the // catalogue falls back to it, so these stubs keep the behaviour // they had before the parent's visible set became its own field. diff --git a/tests/raw_coverage/agent_archivist_debug_round21_raw_coverage_e2e.rs b/tests/raw_coverage/agent_archivist_debug_round21_raw_coverage_e2e.rs index bdfa0dec8c..494ce49196 100644 --- a/tests/raw_coverage/agent_archivist_debug_round21_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_archivist_debug_round21_raw_coverage_e2e.rs @@ -247,7 +247,7 @@ fn definition(max_iterations: usize) -> AgentDefinition { fn parent_context(workspace: &Path, model: Arc) -> ParentExecutionContext { let tools: Vec> = vec![Box::new(EchoTool)]; - let specs = tools.iter().map(|tool| tool.spec()).collect(); + let specs = tools.iter().map(|tool| Arc::new(tool.spec())).collect(); ParentExecutionContext { agent_definition_id: "orchestrator".into(), allowed_subagent_ids: [ diff --git a/tests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs b/tests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs index 2047876ad6..95e7095823 100644 --- a/tests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_harness_leftovers_raw_coverage_e2e.rs @@ -353,7 +353,7 @@ fn definition(max_result_chars: Option) -> AgentDefinition { fn parent_context(workspace: PathBuf, provider: Arc) -> ParentExecutionContext { let tools = vec![tool("echo")]; - let specs = tools.iter().map(|tool| tool.spec()).collect(); + let specs = tools.iter().map(|tool| Arc::new(tool.spec())).collect(); ParentExecutionContext { agent_definition_id: "orchestrator".into(), allowed_subagent_ids: [ diff --git a/tests/raw_coverage/agent_harness_raw_coverage_e2e.rs b/tests/raw_coverage/agent_harness_raw_coverage_e2e.rs index 32ec8e46a8..2422dfd621 100644 --- a/tests/raw_coverage/agent_harness_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_harness_raw_coverage_e2e.rs @@ -261,7 +261,7 @@ fn build_agent_with_tools( fn parent_context(workspace: PathBuf, provider: Arc) -> ParentExecutionContext { let tools: Vec> = vec![Box::new(EchoTool)]; - let tool_specs = tools.iter().map(|tool| tool.spec()).collect(); + let tool_specs = tools.iter().map(|tool| Arc::new(tool.spec())).collect(); ParentExecutionContext { agent_definition_id: "orchestrator".into(), allowed_subagent_ids: [ diff --git a/tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs b/tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs index 929d86ee43..b89a59d5d0 100644 --- a/tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_large_round25_raw_coverage_e2e.rs @@ -311,7 +311,7 @@ fn integrations_definition() -> AgentDefinition { fn parent(workspace_dir: PathBuf, model: Arc) -> ParentExecutionContext { let tools: Vec> = vec![Box::new(LargePayloadTool)]; - let specs = tools.iter().map(|tool| tool.spec()).collect(); + let specs = tools.iter().map(|tool| Arc::new(tool.spec())).collect(); ParentExecutionContext { agent_definition_id: "orchestrator".into(), allowed_subagent_ids: [ diff --git a/tests/raw_coverage/agent_prompts_subagent_raw_coverage_e2e.rs b/tests/raw_coverage/agent_prompts_subagent_raw_coverage_e2e.rs index b1feaa8b24..a07ecbb366 100644 --- a/tests/raw_coverage/agent_prompts_subagent_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_prompts_subagent_raw_coverage_e2e.rs @@ -273,7 +273,7 @@ fn definition(prompt: PromptSource) -> AgentDefinition { fn parent(workspace: PathBuf, model: Arc) -> ParentExecutionContext { let tools = vec![tool("echo"), tool("delegate_nested"), tool("other__skip")]; - let specs = tools.iter().map(|tool| tool.spec()).collect(); + let specs = tools.iter().map(|tool| Arc::new(tool.spec())).collect(); ParentExecutionContext { agent_definition_id: "orchestrator".into(), allowed_subagent_ids: [ diff --git a/tests/raw_coverage/agent_session_turn_raw_coverage_e2e.rs b/tests/raw_coverage/agent_session_turn_raw_coverage_e2e.rs index ac2c73d91b..65a2c41f97 100644 --- a/tests/raw_coverage/agent_session_turn_raw_coverage_e2e.rs +++ b/tests/raw_coverage/agent_session_turn_raw_coverage_e2e.rs @@ -1101,7 +1101,10 @@ async fn subagent_runner_parent_context_filters_tools_caps_output_and_reports_er Arc::new(AtomicUsize::new(0)), ), ]; - let all_specs = all_tools.iter().map(|tool| tool.spec()).collect::>(); + let all_specs = all_tools + .iter() + .map(|tool| Arc::new(tool.spec())) + .collect::>(); let parent = ParentExecutionContext { agent_definition_id: "orchestrator".into(), allowed_subagent_ids: [ diff --git a/tests/raw_coverage/tools_agent_credentials_state_raw_coverage_e2e.rs b/tests/raw_coverage/tools_agent_credentials_state_raw_coverage_e2e.rs index 8654ab9ba8..5bdd187bcd 100644 --- a/tests/raw_coverage/tools_agent_credentials_state_raw_coverage_e2e.rs +++ b/tests/raw_coverage/tools_agent_credentials_state_raw_coverage_e2e.rs @@ -348,7 +348,7 @@ fn response(text: Option<&str>, tool_calls: Vec) -> ModelResponse { fn parent_context(workspace: PathBuf, provider: Arc) -> ParentExecutionContext { let tools: Vec> = vec![Box::new(EchoTool)]; - let tool_specs = tools.iter().map(|tool| tool.spec()).collect(); + let tool_specs = tools.iter().map(|tool| Arc::new(tool.spec())).collect(); ParentExecutionContext { agent_definition_id: "orchestrator".into(), allowed_subagent_ids: [