diff --git a/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs b/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs index f54d33c00..4270b88b6 100644 --- a/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs @@ -816,10 +816,13 @@ await sessionManager.Ask(new SendUserMessage Assert.True(_fakeChatClient.ReceivedToolNames.Count >= 6); - Assert.Contains("browser_chrome_devtools/navigate_page", _fakeChatClient.ReceivedToolNames[2]); - Assert.Contains("browser_chrome_devtools/navigate_page", _fakeChatClient.ReceivedToolNames[3]); - Assert.Contains("browser_chrome_devtools/navigate_page", _fakeChatClient.ReceivedToolNames[4]); - Assert.DoesNotContain("browser_chrome_devtools/navigate_page", _fakeChatClient.ReceivedToolNames[5]); + // ReceivedToolNames records the AIFunction.Name surfaced to the LLM, + // which for MCP tools is the Anthropic-safe sanitized alias + // (server__tool), not the canonical server/tool form. + Assert.Contains("browser_chrome_devtools__navigate_page", _fakeChatClient.ReceivedToolNames[2]); + Assert.Contains("browser_chrome_devtools__navigate_page", _fakeChatClient.ReceivedToolNames[3]); + Assert.Contains("browser_chrome_devtools__navigate_page", _fakeChatClient.ReceivedToolNames[4]); + Assert.DoesNotContain("browser_chrome_devtools__navigate_page", _fakeChatClient.ReceivedToolNames[5]); } [Fact] diff --git a/src/Netclaw.Actors.Tests/Tools/McpToolAdapterTests.cs b/src/Netclaw.Actors.Tests/Tools/McpToolAdapterTests.cs index 4705d7376..4d0de5a75 100644 --- a/src/Netclaw.Actors.Tests/Tools/McpToolAdapterTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/McpToolAdapterTests.cs @@ -31,6 +31,37 @@ public void Construction_produces_correct_shaped_adapter() Assert.Equal("custom:grant", withOverride.GrantCategory); } + [Fact] + public void SanitizedName_uses_double_underscore_separator() + { + // MCP tool names commonly include single underscores + // (e.g. find_completed_tasks); double underscore between server and + // tool keeps the boundary unambiguous when parsing the alias back. + var fakeTool = AIFunctionFactory.Create(() => "result", "find_completed_tasks"); + var adapter = new McpToolAdapter(fakeTool, "todoist", "find_completed_tasks"); + + Assert.Equal("todoist/find_completed_tasks", adapter.Name); + Assert.Equal("todoist__find_completed_tasks", adapter.SanitizedName); + } + + [Theory] + [InlineData("memorizer", "store")] + [InlineData("todoist", "find-tasks")] + [InlineData("browser_chrome_devtools", "navigate_page")] + [InlineData("bamboohr", "get_employee_details")] + public void SanitizedName_matches_Anthropic_tool_name_regex(string server, string tool) + { + // Anthropic's documented tool-name constraint is + // ^[a-zA-Z0-9_-]{1,64}$ (see Define tools docs). The whole point of + // the alias is to satisfy this — pin the contract so future name + // formats can't silently regress. + var fakeTool = AIFunctionFactory.Create(() => "result", tool); + var adapter = new McpToolAdapter(fakeTool, server, tool); + + Assert.Matches("^[a-zA-Z0-9_-]{1,64}$", adapter.SanitizedName); + Assert.Matches("^[a-zA-Z0-9_-]{1,64}$", ((AIFunction)adapter.ToAITool()).Name); + } + [Fact] public void ToAITool_ReturnsSanitizedWrapper() { @@ -41,7 +72,12 @@ public void ToAITool_ReturnsSanitizedWrapper() // Should return a sanitized wrapper, not the raw tool Assert.IsAssignableFrom(aiTool); var func = (AIFunction)aiTool; - Assert.Equal("memorizer/store", func.Name); + // The LLM-facing AIFunction exposes the Anthropic-safe alias so the + // tool name passes ^[a-zA-Z0-9_-]{1,64}$. The canonical Name on the + // adapter itself still uses the '/' separator for skill text and + // registry keys. + Assert.Equal("memorizer__store", func.Name); + Assert.Equal("memorizer/store", adapter.Name); } [Fact] diff --git a/src/Netclaw.Actors.Tests/Tools/McpToolAudienceGrantsTests.cs b/src/Netclaw.Actors.Tests/Tools/McpToolAudienceGrantsTests.cs index b0e3d9718..ab082121e 100644 --- a/src/Netclaw.Actors.Tests/Tools/McpToolAudienceGrantsTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/McpToolAudienceGrantsTests.cs @@ -240,7 +240,9 @@ public void FilterExposedTools_RemovesToolsBlockedByGrants() var filtered = policy.FilterExposedTools(aiTools, registry, trustContext); Assert.Single(filtered); - Assert.Equal("memorizer/search_memories", ((AIFunction)filtered[0]).Name); + // FilterExposedTools surfaces the AIFunction wrapper that goes to the LLM, + // which uses the Anthropic-safe sanitized alias (server__tool). + Assert.Equal("memorizer__search_memories", ((AIFunction)filtered[0]).Name); } // ── load_tool denial ── diff --git a/src/Netclaw.Actors.Tests/Tools/ToolRegistryTests.cs b/src/Netclaw.Actors.Tests/Tools/ToolRegistryTests.cs index ace67b4db..24731e3b7 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolRegistryTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ToolRegistryTests.cs @@ -222,6 +222,42 @@ public void GenerateCompressedIndex_for_public_hides_blocked_capabilities() Assert.DoesNotContain("memorizer", index); } + [Fact] + public void GetByName_resolves_McpTool_by_sanitized_alias() + { + // tool_use responses from Anthropic come back with the sanitized name + // (server__tool), but skill text and load_tool results use the + // canonical server/tool form. The registry has to accept either. + var registry = new ToolRegistry(); + var adapter = new McpToolAdapter( + CreateFakeTool("store"), "memorizer", "store"); + registry.Register(adapter); + + var byCanonical = registry.GetByName("memorizer/store"); + var bySanitized = registry.GetByName("memorizer__store"); + + Assert.NotNull(byCanonical); + Assert.NotNull(bySanitized); + Assert.Same(adapter, byCanonical); + Assert.Same(adapter, bySanitized); + } + + [Fact] + public void GetRegistrationByToolName_resolves_McpTool_by_sanitized_alias() + { + var registry = new ToolRegistry(); + var adapter = new McpToolAdapter( + CreateFakeTool("search_memories"), "memorizer", "search_memories"); + registry.Register(adapter); + + var byCanonical = registry.GetRegistrationByToolName("memorizer/search_memories"); + var bySanitized = registry.GetRegistrationByToolName("memorizer__search_memories"); + + Assert.NotNull(byCanonical); + Assert.NotNull(bySanitized); + Assert.Same(byCanonical, bySanitized); + } + private static AIFunction CreateFakeTool(string name) { return AIFunctionFactory.Create(() => "result", name); diff --git a/src/Netclaw.Actors/Tools/McpToolAdapter.cs b/src/Netclaw.Actors/Tools/McpToolAdapter.cs index 61229e16f..70cfb6ce5 100644 --- a/src/Netclaw.Actors/Tools/McpToolAdapter.cs +++ b/src/Netclaw.Actors/Tools/McpToolAdapter.cs @@ -44,6 +44,13 @@ public McpToolAdapter( _invoker = invoker; ServerName = serverName; Name = $"{serverName}/{toolName}"; + // LLM-facing alias with Anthropic-safe separator. Anthropic API rejects + // tool names with `/` (regex ^[a-zA-Z0-9_-]{1,128}$). The internal Name + // keeps the slash for backward compat (skill text, registry keys); + // SanitizedName is what's surfaced to the LLM in tool definitions and + // what comes back in tool_use responses. ToolRegistry.GetByName + // accepts either form. + SanitizedName = $"{serverName}__{toolName}"; GrantCategory = grantCategory ?? $"mcp:{serverName}"; if (mcpTool is AIFunction func) @@ -55,7 +62,7 @@ public McpToolAdapter( _rawSchema = func.JsonSchema; var sanitized = McpSchemaSanitizer.SanitizeSchema(func.JsonSchema); ParameterSchema = McpSchemaSanitizer.InjectMetaProperties(sanitized, Name, logger); - _sanitizedTool = new SanitizedAIFunction(func, Name, Description, ParameterSchema); + _sanitizedTool = new SanitizedAIFunction(func, SanitizedName, Description, ParameterSchema); } else { @@ -87,6 +94,12 @@ internal static string ClampDescription(string description, int maxChars) } public string Name { get; } + /// + /// Anthropic-safe alias for . Format + /// {server}__{tool}. Surfaced to the LLM in tool definitions so the + /// Anthropic API does not reject the request for invalid tool-name chars. + /// + public string SanitizedName { get; } public string Description { get; } public string GrantCategory { get; } public string ServerName { get; } diff --git a/src/Netclaw.Actors/Tools/ToolRegistry.cs b/src/Netclaw.Actors/Tools/ToolRegistry.cs index 55a10cf42..599d4e79d 100644 --- a/src/Netclaw.Actors/Tools/ToolRegistry.cs +++ b/src/Netclaw.Actors/Tools/ToolRegistry.cs @@ -59,12 +59,24 @@ public IReadOnlyList GetToolsForGrants(IReadOnlySet grantedCateg .Select(t => t.Tool.ToAITool()) .ToList(); - /// Find a tool by name for dispatch. + /// Find a tool by name for dispatch. Accepts either the + /// canonical name (server/tool for MCP) or the LLM-facing + /// sanitized alias (server__tool). public INetclawTool? GetByName(string name) => - _tools.FirstOrDefault(t => t.Tool.Name == name)?.Tool; + FindRegistration(name)?.Tool; public ToolRegistration? GetRegistrationByToolName(string name) => - _tools.FirstOrDefault(t => t.Tool.Name == name); + FindRegistration(name); + + private ToolRegistration? FindRegistration(string name) + { + var direct = _tools.FirstOrDefault(t => t.Tool.Name == name); + if (direct is not null) + return direct; + return _tools.FirstOrDefault(t => + t.Tool is McpToolAdapter mcp + && string.Equals(mcp.SanitizedName, name, StringComparison.Ordinal)); + } /// /// Returns tools that should always be loaded into the LLM context.