From c5282d03a851d084b927740fc6b20a0e0eeddf0c Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Tue, 26 May 2026 08:42:37 +0200 Subject: [PATCH] test(agent-harness): restore TAURI-RUST-4 dedup seam test `run_tool_call_loop_dedups_duplicate_tool_names_before_provider_call` and its `CapturingProvider` helper (which records the tool-spec names reaching the provider) were dropped when tool_loop_tests.rs was rewritten in #2631. They guard TAURI-RUST-4: duplicate tool names (e.g. a synthesised delegation tool whose delegate_name shadows a same-named skill tool) must be deduplicated before the provider call, since some providers 400 on duplicate tool names. Restore both verbatim. The current 17-arg `run_tool_call_loop` signature and the dedup logic in tool_loop.rs are unchanged, so the test compiles and passes as a genuine regression guard. Co-authored-by: Claude Opus 4.7 (1M context) --- .../agent/harness/tool_loop_tests.rs | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/src/openhuman/agent/harness/tool_loop_tests.rs b/src/openhuman/agent/harness/tool_loop_tests.rs index 50e48fb5ec..d2f8151fba 100644 --- a/src/openhuman/agent/harness/tool_loop_tests.rs +++ b/src/openhuman/agent/harness/tool_loop_tests.rs @@ -1148,3 +1148,115 @@ fn hard_reject_distinct_args_do_not_trip_repeat() { "6 distinct hard rejects in a row should still trip the no-progress guard" ); } + +/// 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>>>, + responses: Mutex>>, + 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 { + Ok("fallback".into()) + } + + async fn chat( + &self, + request: ChatRequest<'_>, + _model: &str, + _temperature: f64, + ) -> Result { + let names = request + .tools + .map(|specs| specs.iter().map(|s| s.name.clone()).collect::>()); + 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> = vec![Box::new(EchoTool)]; + let extra: Vec> = vec![Box::new(EchoTool)]; + + let mut history = vec![ChatMessage::user("hi")]; + let result = run_tool_call_loop( + &provider, + &mut history, + ®istry, + "test-provider", + "model", + 0.0, + true, + None, + "channel", + &crate::openhuman::config::MultimodalConfig::default(), + 2, + None, + None, + &extra, + None, + None, + &crate::openhuman::tools::policy::DefaultToolPolicy, + ) + .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 + ); +}