fix(subagent): dedup tool specs before sending to provider (#2485)

Co-authored-by: sanil-23 <sanil@alphahuman.xyz>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/openhuman/agent/harness/subagent_runner/ops.rs

@@ -326,6 +326,44 @@ pub async fn run_subagent(
 // Typed mode — narrow prompt, filtered tools, cheaper model
 // ─────────────────────────────────────────────────────────────────────────────
 
+/// Deduplicate assembled tool specs by name, keeping the first occurrence.
+///
+/// The sub-agent's `filtered_specs` is a `Vec` assembled from
+/// `parent.all_tool_specs` indices plus dynamic tools, so a delegation tool can
+/// shadow a same-named skill/integration tool (common for the wide-set
+/// `tools_agent`), leaving two specs with the same name. Strict providers reject
+/// such a request with `400 "Tool names must be unique."` The main-agent path
+/// dedups via [`session::builder::dedup_visible_tool_specs`]; this separate
+/// sub-agent assembly must do the same.
+///
+/// First occurrence wins so registration-order semantics are preserved (tool
+/// dispatch still resolves by name). Dropped duplicates are logged at `debug`
+/// (diagnostic instrumentation, per the repo Rust logging guideline).
+///
+/// Extracted as a free function so the regression suite can exercise the dedup
+/// without standing up the full `run_typed_mode` plumbing.
+fn dedup_tool_specs_by_name(agent_id: &str, specs: Vec<ToolSpec>) -> Vec<ToolSpec> {
+    let mut seen: HashSet<String> = HashSet::with_capacity(specs.len());
+    let mut deduped: Vec<ToolSpec> = Vec::with_capacity(specs.len());
+    let mut dropped: Vec<String> = Vec::new();
+    for spec in specs {
+        if seen.insert(spec.name.clone()) {
+            deduped.push(spec);
+        } else {
+            dropped.push(spec.name);
+        }
+    }
+    if !dropped.is_empty() {
+        tracing::debug!(
+            agent_id = %agent_id,
+            "[subagent_runner] dropped {} duplicate tool spec(s) before sending to provider: {:?}",
+            dropped.len(),
+            dropped
+        );
+    }
+    deduped
+}
+
 /// Execute a sub-agent in "Typed" mode.
 ///
 /// This mode builds a brand-new, minimized system prompt specifically for the
@@ -839,6 +877,10 @@ async fn run_typed_mode(
         allowed_names.insert(tool.name().to_string());
     }
 
+    // Dedup by tool name before the specs reach the provider (see
+    // `dedup_tool_specs_by_name` for why duplicates appear here).
+    let filtered_specs = dedup_tool_specs_by_name(&definition.id, filtered_specs);
+
     tracing::debug!(
         agent_id = %definition.id,
         model = %model,
@@ -1680,6 +1722,10 @@ pub(crate) fn user_is_signed_in_to_composio(config: &crate::openhuman::config::C
 #[path = "ops_tests.rs"]
 mod tests;
 
+#[cfg(test)]
+#[path = "ops_dedup_tests.rs"]
+mod dedup_tests;
+
 #[cfg(test)]
 #[path = "ops_truncation_tests.rs"]
 mod truncation_tests;

src/openhuman/agent/harness/subagent_runner/ops_dedup_tests.rs

@@ -0,0 +1,71 @@
+//! Focused unit tests for [`super::dedup_tool_specs_by_name`].
+//!
+//! Mirrors `session::builder::dedup_visible_tool_specs` coverage: the
+//! sub-agent assembly path must drop same-named duplicate tool specs
+//! (first occurrence wins) before they reach a strict provider that
+//! 400s on `"Tool names must be unique."`
+
+use super::*;
+use serde_json::json;
+
+fn spec(name: &str) -> ToolSpec {
+    ToolSpec {
+        name: name.to_string(),
+        description: format!("description for {name}"),
+        parameters: json!({}),
+    }
+}
+
+#[test]
+fn drops_duplicates_first_wins() {
+    // Real-world collision: a delegation tool (e.g. `tools_agent`) shadows a
+    // same-named skill/integration tool. Keep the *first* occurrence so
+    // registration-order semantics hold (dispatch still resolves by name).
+    let specs = vec![
+        spec("research"), // skill
+        spec("plan"),
+        spec("research"), // delegate, dropped
+        spec("run_code"),
+        spec("plan"), // dropped
+    ];
+
+    let deduped = dedup_tool_specs_by_name("test-agent", specs);
+
+    let names: Vec<&str> = deduped.iter().map(|s| s.name.as_str()).collect();
+    assert_eq!(names, vec!["research", "plan", "run_code"]);
+}
+
+#[test]
+fn passes_through_when_no_duplicates() {
+    let specs = vec![spec("a"), spec("b"), spec("c")];
+    let deduped = dedup_tool_specs_by_name("test-agent", specs);
+    let names: Vec<&str> = deduped.iter().map(|s| s.name.as_str()).collect();
+    assert_eq!(names, vec!["a", "b", "c"]);
+}
+
+#[test]
+fn handles_empty_input() {
+    let deduped = dedup_tool_specs_by_name("test-agent", Vec::<ToolSpec>::new());
+    assert!(deduped.is_empty());
+}
+
+#[test]
+fn preserves_full_spec_content_for_kept_entries() {
+    // Description + parameters must survive intact — the LLM uses both for
+    // tool-call decisions, and the kept entry must be the *first* one.
+    let mut first = spec("alpha");
+    first.description = "first alpha — should win".to_string();
+    first.parameters = json!({"type": "object", "required": ["x"]});
+
+    let mut dup = spec("alpha");
+    dup.description = "second alpha — should be dropped".to_string();
+
+    let deduped = dedup_tool_specs_by_name("test-agent", vec![first, dup]);
+
+    assert_eq!(deduped.len(), 1);
+    assert_eq!(deduped[0].description, "first alpha — should win");
+    assert_eq!(
+        deduped[0].parameters,
+        json!({"type": "object", "required": ["x"]})
+    );
+}