Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -816,10 +816,13 @@ await sessionManager.Ask<CommandAck>(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]
Expand Down
38 changes: 37 additions & 1 deletion src/Netclaw.Actors.Tests/Tools/McpToolAdapterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -41,7 +72,12 @@ public void ToAITool_ReturnsSanitizedWrapper()
// Should return a sanitized wrapper, not the raw tool
Assert.IsAssignableFrom<AIFunction>(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]
Expand Down
4 changes: 3 additions & 1 deletion src/Netclaw.Actors.Tests/Tools/McpToolAudienceGrantsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ──
Expand Down
36 changes: 36 additions & 0 deletions src/Netclaw.Actors.Tests/Tools/ToolRegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
15 changes: 14 additions & 1 deletion src/Netclaw.Actors/Tools/McpToolAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
{
Expand Down Expand Up @@ -87,6 +94,12 @@ internal static string ClampDescription(string description, int maxChars)
}

public string Name { get; }
/// <summary>
/// Anthropic-safe alias for <see cref="Name"/>. Format
/// <c>{server}__{tool}</c>. Surfaced to the LLM in tool definitions so the
/// Anthropic API does not reject the request for invalid tool-name chars.
/// </summary>
public string SanitizedName { get; }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this is documented in their tooling guidance here https://platform.claude.com/docs/en/agents-and-tools/tool-use/define-tools

public string Description { get; }
public string GrantCategory { get; }
public string ServerName { get; }
Expand Down
18 changes: 15 additions & 3 deletions src/Netclaw.Actors/Tools/ToolRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,24 @@ public IReadOnlyList<AITool> GetToolsForGrants(IReadOnlySet<string> grantedCateg
.Select(t => t.Tool.ToAITool())
.ToList();

/// <summary>Find a tool by name for dispatch.</summary>
/// <summary>Find a tool by name for dispatch. Accepts either the
/// canonical name (<c>server/tool</c> for MCP) or the LLM-facing
/// sanitized alias (<c>server__tool</c>).</summary>
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));
}

/// <summary>
/// Returns tools that should always be loaded into the LLM context.
Expand Down
Loading