fix(agent-harness): dedup visible tool specs in all provider-call paths (#2446)

src/openhuman/agent/harness/session/builder.rs

@@ -39,7 +39,7 @@ use std::sync::Arc;
 /// 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(super) fn dedup_visible_tool_specs(specs: Vec<ToolSpec>) -> Vec<ToolSpec> {
+pub(crate) fn dedup_visible_tool_specs(specs: Vec<ToolSpec>) -> Vec<ToolSpec> {
     let mut seen: std::collections::HashSet<String> = std::collections::HashSet::new();
     let mut deduped: Vec<ToolSpec> = Vec::with_capacity(specs.len());
     let mut dropped: Vec<String> = Vec::new();

src/openhuman/agent/harness/session/mod.rs

@@ -33,3 +33,8 @@ pub use migration::{migrate_session_layout_if_needed, MigrationOutcome};
 mod tests;
 
 pub use types::{Agent, AgentBuilder};
+
+// Re-export the duplicate-tool-spec guard for sibling harness modules
+// (`tool_loop`, `subagent_runner`) so all three provider call sites
+// share one tested implementation.
+pub(crate) use builder::dedup_visible_tool_specs;

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

@@ -862,20 +862,37 @@ async fn run_typed_mode(
         None
     };
 
-    let mut filtered_specs: Vec<ToolSpec> = allowed_indices
-        .iter()
-        .map(|&i| parent.all_tool_specs[i].clone())
-        .collect();
+    // Build provider-visible tool schemas in EXECUTION-PRECEDENCE order:
+    // `dynamic_tools` (extra_tools at runtime) before parent specs, because
+    // the inner loop's name lookup (see end of this fn) resolves
+    // `extra_tools` first and only falls back to `parent_tools`. Aligning
+    // the dedup order with the runtime lookup order guarantees the schema
+    // the model sees and the tool that actually executes describe the same
+    // behaviour. (CodeRabbit review on PR #2446.)
+    let mut filtered_specs: Vec<ToolSpec> = dynamic_tools.iter().map(|t| t.spec()).collect();
+    filtered_specs.extend(
+        allowed_indices
+            .iter()
+            .map(|&i| parent.all_tool_specs[i].clone()),
+    );
     let mut allowed_names: HashSet<String> = allowed_indices
         .iter()
         .map(|&i| parent.all_tools[i].name().to_string())
         .collect();
-    // Append dynamic tool specs / names so they're discoverable by the
-    // provider (native tool-calling) and by the inner loop's allowlist.
+    // Dynamic tool names must also be in the allowlist so the inner loop
+    // accepts model tool_calls that reference them.
     for tool in &dynamic_tools {
-        filtered_specs.push(tool.spec());
         allowed_names.insert(tool.name().to_string());
     }
+    // Dedup by name: first occurrence wins. Dynamic Composio action tools
+    // can share a name with an inherited parent-registry spec when the
+    // agent's AllowedAll scope includes a same-named skill tool. Some
+    // providers (Anthropic, OpenHuman cloud after the uniqueness-enforcement
+    // rollout) 400 on duplicate tool names — see TAURI-RUST-4. Because
+    // `filtered_specs` is in execution order (dynamic first), the kept
+    // schema matches what the runtime will actually dispatch.
+    let filtered_specs =
+        crate::openhuman::agent::harness::session::dedup_visible_tool_specs(filtered_specs);
 
     // Dedup by tool name before the specs reach the provider (see
     // `dedup_tool_specs_by_name` for why duplicates appear here).

src/openhuman/agent/harness/tool_loop.rs

@@ -133,12 +133,20 @@ pub(crate) async fn run_tool_call_loop(
         }
     };
 
-    let tool_specs: Vec<crate::openhuman::tools::ToolSpec> = tools_registry
+    // Filter to visible tools, then dedup by name before sending to the
+    // provider. Registry tools may collide with per-turn synthesised
+    // extra_tools (e.g. an `ArchetypeDelegationTool` whose
+    // `delegate_name = "research"` shadowing a same-named skill). Some
+    // providers (Anthropic, OpenHuman cloud after the uniqueness-enforcement
+    // rollout) 400 on duplicate tool names — see TAURI-RUST-4.
+    let filtered_specs: Vec<crate::openhuman::tools::ToolSpec> = tools_registry
         .iter()
         .chain(extra_tools.iter())
         .filter(|tool| is_visible(tool.name()))
         .map(|tool| tool.spec())
         .collect();
+    let tool_specs =
+        crate::openhuman::agent::harness::session::dedup_visible_tool_specs(filtered_specs);
     let use_native_tools = provider.supports_native_tools() && !tool_specs.is_empty();
 
     log::debug!(

src/openhuman/agent/harness/tool_loop_tests.rs

@@ -886,3 +886,128 @@ async fn run_tool_call_loop_applies_per_tool_max_result_size_cap() {
         tool_results.content.len()
     );
 }
+
+// ── TAURI-RUST-4 regression guard ────────────────────────────────────
+//
+// Some providers (Anthropic, OpenHuman cloud after the uniqueness-
+// enforcement rollout) reject chat requests whose `tools` list contains
+// two specs with the same `name` — HTTP 400 "Tool names must be unique."
+// `run_tool_call_loop` chains the persistent `tools_registry` with the
+// per-turn synthesised `extra_tools`; if any name collides across the
+// two lists, both would have made it to the provider before the fix.
+//
+// This test wires a capturing provider, builds a colliding tool list
+// (one `EchoTool` in the registry + a second `EchoTool` clone in
+// `extra_tools`), and asserts the names the provider sees contain
+// `"echo"` exactly once.
+
+/// Provider that records the tool-spec names of every `chat()` request
+/// it sees, then returns the next scripted response.
+struct CapturingProvider {
+    /// One entry per `chat()` call — the tool-name list extracted from
+    /// `ChatRequest.tools`. `None` if `tools` was `None`.
+    captured: Mutex<Vec<Option<Vec<String>>>>,
+    responses: Mutex<Vec<anyhow::Result<ChatResponse>>>,
+    native_tools: bool,
+}
+
+#[async_trait]
+impl Provider for CapturingProvider {
+    async fn chat_with_system(
+        &self,
+        _system_prompt: Option<&str>,
+        _message: &str,
+        _model: &str,
+        _temperature: f64,
+    ) -> Result<String> {
+        Ok("fallback".into())
+    }
+
+    async fn chat(
+        &self,
+        request: ChatRequest<'_>,
+        _model: &str,
+        _temperature: f64,
+    ) -> Result<ChatResponse> {
+        let names = request
+            .tools
+            .map(|specs| specs.iter().map(|s| s.name.clone()).collect::<Vec<_>>());
+        self.captured.lock().push(names);
+        let mut guard = self.responses.lock();
+        guard.remove(0)
+    }
+
+    fn capabilities(&self) -> ProviderCapabilities {
+        ProviderCapabilities {
+            native_tool_calling: self.native_tools,
+            vision: false,
+            ..ProviderCapabilities::default()
+        }
+    }
+}
+
+#[tokio::test]
+async fn run_tool_call_loop_dedups_duplicate_tool_names_before_provider_call() {
+    // Provider returns a single final text response — no tool calls —
+    // so the loop terminates after exactly one `chat()` invocation,
+    // and the captured tool list reflects what the fix is supposed to
+    // guard against (no duplicate names reaching the wire).
+    let provider = CapturingProvider {
+        captured: Mutex::new(Vec::new()),
+        responses: Mutex::new(vec![Ok(ChatResponse {
+            text: Some("done".into()),
+            tool_calls: vec![],
+            usage: None,
+        })]),
+        // Native tool-calling on: only when the provider supports native
+        // tools does `run_tool_call_loop` populate `ChatRequest.tools`.
+        native_tools: true,
+    };
+
+    // Registry has `EchoTool` (name = "echo"). `extra_tools` adds a
+    // second tool also named "echo" — the exact collision pattern from
+    // the bug report (a synthesised delegation tool whose
+    // `delegate_name` shadows a same-named skill tool).
+    let registry: Vec<Box<dyn Tool>> = vec![Box::new(EchoTool)];
+    let extra: Vec<Box<dyn Tool>> = vec![Box::new(EchoTool)];
+
+    let mut history = vec![ChatMessage::user("hi")];
+    let result = run_tool_call_loop(
+        &provider,
+        &mut history,
+        &registry,
+        "test-provider",
+        "model",
+        0.0,
+        true,
+        None,
+        "channel",
+        &crate::openhuman::config::MultimodalConfig::default(),
+        2,
+        None,
+        None,
+        &extra,
+        None,
+        None,
+    )
+    .await
+    .expect("loop should succeed with deduplicated tool list");
+    assert_eq!(result, "done");
+
+    let captured = provider.captured.lock();
+    assert_eq!(
+        captured.len(),
+        1,
+        "exactly one chat() call expected for a final-only response"
+    );
+    let names = captured[0]
+        .as_ref()
+        .expect("native_tools=true should populate ChatRequest.tools");
+    let echo_count = names.iter().filter(|n| n.as_str() == "echo").count();
+    assert_eq!(
+        echo_count, 1,
+        "duplicate tool names must be dropped before the provider call \
+         (TAURI-RUST-4) — got names={:?}",
+        names
+    );
+}