From 8faed8dc2cc4a87b9cb44b6458f4071a6ecedff1 Mon Sep 17 00:00:00 2001 From: serrrfirat Date: Fri, 26 Jun 2026 10:41:13 +0300 Subject: [PATCH] fix(reborn): align external tool provider names --- .../tests/reborn_dependency_boundaries.rs | 1 + .../local_dev/external_tool_capability.rs | 209 +++++++++++++++++- 2 files changed, 203 insertions(+), 7 deletions(-) diff --git a/crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs b/crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs index e8700f090b3..248abb3e052 100644 --- a/crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs +++ b/crates/ironclaw_architecture/tests/reborn_dependency_boundaries.rs @@ -709,6 +709,7 @@ fn provider_tool_names_stay_at_model_protocol_boundaries() { // Composition-local protocol surfaces that reconstruct provider-shaped // output or local-dev synthetic provider tools. "crates/ironclaw_reborn_composition/src/openai_compat_serve.rs", + "crates/ironclaw_reborn_composition/src/runtime/local_dev/external_tool_capability.rs", "crates/ironclaw_reborn_composition/src/runtime/local_dev/synthetic_capability.rs", "crates/ironclaw_reborn_composition/src/trace_capture.rs", ]); diff --git a/crates/ironclaw_reborn_composition/src/runtime/local_dev/external_tool_capability.rs b/crates/ironclaw_reborn_composition/src/runtime/local_dev/external_tool_capability.rs index e06884bdaab..e8370cfea6b 100644 --- a/crates/ironclaw_reborn_composition/src/runtime/local_dev/external_tool_capability.rs +++ b/crates/ironclaw_reborn_composition/src/runtime/local_dev/external_tool_capability.rs @@ -23,7 +23,7 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex as StdMutex}; use async_trait::async_trait; -use ironclaw_host_api::{CapabilityId, InvocationId, RuntimeKind}; +use ironclaw_host_api::{CapabilityId, InvocationId, ProviderToolName, RuntimeKind}; use ironclaw_loop_support::{ CapabilityResultWrite, LoopCapabilityInputResolver, LoopCapabilityResultWriter, }; @@ -64,11 +64,11 @@ pub(super) fn wrap_local_dev_external_tools( struct ResolvedSurface { version: CapabilitySurfaceVersion, specs_by_capability_id: HashMap, - capability_ids_by_tool_name: HashMap, + capability_ids_by_tool_name: HashMap, } struct ToolSpec { - tool_name: String, + tool_name: ProviderToolName, description: String, parameters_schema: serde_json::Value, } @@ -82,7 +82,7 @@ impl ToolSpec { capability_id: capability_id.clone(), provider: None, runtime: RuntimeKind::System, - safe_name: self.tool_name.clone(), + safe_name: self.tool_name.as_str().to_string(), safe_description: self.description.clone(), // External tools are client-side; the host never runs them in // parallel, and they always park, so mark them exclusive. @@ -132,6 +132,17 @@ fn external_tool_capability_id(tool_name: &str) -> Result Result { + ProviderToolName::new(tool_name).map_err(|_| { + AgentLoopHostError::new( + AgentLoopHostErrorKind::InvalidInvocation, + "external tool name cannot be represented as a provider tool name", + ) + }) +} + impl ExternalToolCapabilityPort { fn surface_version(&self) -> Result { self.surface @@ -161,7 +172,7 @@ impl ExternalToolCapabilityPort { .unwrap_or(false) } - fn capability_id_for_tool_name(&self, tool_name: &str) -> Option { + fn capability_id_for_tool_name(&self, tool_name: &ProviderToolName) -> Option { self.surface.lock().ok().and_then(|surface| { surface .as_ref() @@ -352,6 +363,7 @@ impl LoopCapabilityPort for ExternalToolCapabilityPort { "external tool name shadows a host capability", )); } + let tool_name = provider_tool_name_for_external_tool(spec.name())?; let capability_id = external_tool_capability_id(spec.name())?; if surface .descriptors @@ -364,9 +376,9 @@ impl LoopCapabilityPort for ExternalToolCapabilityPort { "external tool conflicts with another capability id", )); } - capability_ids_by_tool_name.insert(spec.name().to_string(), capability_id.clone()); + capability_ids_by_tool_name.insert(tool_name.clone(), capability_id.clone()); let tool_spec = ToolSpec { - tool_name: spec.name().to_string(), + tool_name, description: spec.description().to_string(), parameters_schema: spec.parameters_schema().clone(), }; @@ -444,3 +456,186 @@ fn catalog_error(_error: ironclaw_turns::ExternalToolCatalogError) -> AgentLoopH "external tool catalog is unavailable", ) } + +#[cfg(test)] +mod tests { + use super::*; + + use ironclaw_host_api::{TenantId, ThreadId}; + use ironclaw_loop_support::CapabilityWriteResult; + use ironclaw_turns::{ + ExternalToolSpec, InMemoryExternalToolCatalog, RunProfileResolutionRequest, + RunProfileResolver, TurnId, TurnScope, + run_profile::{CapabilityInputRef, InMemoryRunProfileResolver}, + }; + + struct EmptyInnerPort; + + #[async_trait] + impl LoopCapabilityPort for EmptyInnerPort { + async fn visible_capabilities( + &self, + _request: VisibleCapabilityRequest, + ) -> Result { + Ok(VisibleCapabilitySurface { + version: CapabilitySurfaceVersion::new("test.surface.v1").expect("surface version"), + descriptors: Vec::new(), + }) + } + + async fn invoke_capability( + &self, + _request: CapabilityInvocation, + ) -> Result { + Err(AgentLoopHostError::new( + AgentLoopHostErrorKind::InvalidInvocation, + "test inner port does not execute capabilities", + )) + } + + async fn invoke_capability_batch( + &self, + _request: CapabilityBatchInvocation, + ) -> Result { + Err(AgentLoopHostError::new( + AgentLoopHostErrorKind::InvalidInvocation, + "test inner port does not execute capability batches", + )) + } + } + + struct TestInputResolver; + + #[async_trait] + impl LoopCapabilityInputResolver for TestInputResolver { + async fn resolve_capability_input( + &self, + _run_context: &LoopRunContext, + _input_ref: &CapabilityInputRef, + ) -> Result { + Err(AgentLoopHostError::new( + AgentLoopHostErrorKind::InvalidInvocation, + "test input resolver does not resolve inputs", + )) + } + } + + struct TestResultWriter; + + #[async_trait] + impl LoopCapabilityResultWriter for TestResultWriter { + async fn write_capability_result( + &self, + _write: CapabilityResultWrite<'_>, + ) -> Result { + Err(AgentLoopHostError::new( + AgentLoopHostErrorKind::InvalidInvocation, + "test result writer does not write results", + )) + } + } + + async fn run_context() -> LoopRunContext { + let resolved = InMemoryRunProfileResolver::default() + .resolve_run_profile(RunProfileResolutionRequest::interactive_default()) + .await + .expect("profile resolves"); + LoopRunContext::new( + TurnScope::new( + TenantId::new("tenant-external-tools").expect("tenant id"), + None, + None, + ThreadId::new("thread-external-tools").expect("thread id"), + ), + TurnId::new(), + TurnRunId::new(), + resolved, + ) + } + + fn external_tool_spec(name: &str) -> ExternalToolSpec { + ExternalToolSpec::new( + name, + "client-side external tool", + serde_json::json!({"type": "object"}), + ) + .expect("external tool spec") + } + + async fn wrapped_port_with_specs( + specs: Vec, + ) -> (Arc, LoopRunContext) { + let run_context = run_context().await; + let catalog = Arc::new(InMemoryExternalToolCatalog::new()); + catalog + .register(run_context.run_id, specs) + .await + .expect("register external tools"); + let catalog: Arc = catalog; + ( + wrap_local_dev_external_tools( + Arc::new(EmptyInnerPort), + run_context.clone(), + Arc::new(TestInputResolver), + Arc::new(TestResultWriter), + catalog, + ), + run_context, + ) + } + + #[tokio::test] + async fn external_tool_surface_maps_provider_name_to_capability_id() { + let (port, _run_context) = + wrapped_port_with_specs(vec![external_tool_spec("client_tool")]).await; + + let surface = port + .visible_capabilities(VisibleCapabilityRequest) + .await + .expect("visible capabilities"); + assert_eq!(surface.descriptors.len(), 1); + assert_eq!( + surface.descriptors[0].capability_id.as_str(), + "external_tool.client_tool" + ); + assert_eq!(surface.descriptors[0].safe_name, "client_tool"); + + let definitions = port.tool_definitions().expect("tool definitions"); + assert_eq!(definitions.len(), 1); + assert_eq!(definitions[0].name.as_str(), "client_tool"); + + let ids = port + .provider_tool_call_capability_ids(&ProviderToolCall { + provider_id: "test-provider".to_string(), + provider_model_id: "test-model".to_string(), + turn_id: Some("turn-1".to_string()), + id: "call-1".to_string(), + name: ProviderToolName::new("client_tool").expect("provider tool name"), + arguments: serde_json::json!({}), + response_reasoning: None, + reasoning: None, + signature: None, + }) + .expect("capability ids"); + assert_eq!( + ids.provider_capability_id.as_str(), + "external_tool.client_tool" + ); + } + + #[tokio::test] + async fn external_tool_surface_rejects_names_that_are_not_provider_safe() { + let (port, _run_context) = + wrapped_port_with_specs(vec![external_tool_spec("client.tool")]).await; + + let error = port + .visible_capabilities(VisibleCapabilityRequest) + .await + .expect_err("invalid external tool name should fail closed"); + assert_eq!(error.kind, AgentLoopHostErrorKind::InvalidInvocation); + assert_eq!( + error.safe_summary, + "external tool name cannot be represented as a provider tool name" + ); + } +}