fix(mcp): expose Anthropic-safe alias for MCP tool names#1134
Merged
Aaronontheweb merged 3 commits intoMay 21, 2026
Merged
Conversation
…claw-dev#1133 Anthropic's API enforces tool-name regex ^[a-zA-Z0-9_-]{1,128}$, which rejects the '/' that McpToolAdapter uses to namespace MCP tools as {server}/{tool}. Once the LLM tool list contains an MCP tool, every turn fails with: tools.NN.custom.name: String should match pattern '^[a-zA-Z0-9_-]{1,128}$' Adds an Anthropic-safe alias on McpToolAdapter (`{server}__{tool}`, double underscore boundary) and surfaces THAT to the LLM via SanitizedAIFunction.Name. The canonical Name stays `{server}/{tool}` so skill text, registry keys, and existing operator workflows that reference `server/tool` keep working. ToolRegistry.GetByName / GetRegistrationByToolName accept either form so tool_use responses (which now arrive with the sanitized name) dispatch correctly. Verified locally: pre-patch, every weekly-report run died at LLM call with the BadRequest above as soon as a Todoist tool was loaded; post-patch, the LLM receives `todoist__find-tasks` and the request succeeds.
Update three tests whose assertions were pinned to the old slashed
LLM-facing tool names, and add positive coverage for the new alias:
- McpToolAdapterTests.ToAITool_ReturnsSanitizedWrapper: expect
memorizer__store from the AIFunction wrapper while pinning the
canonical adapter.Name to memorizer/store.
- McpToolAudienceGrantsTests.FilterExposedTools_RemovesToolsBlockedByGrants:
expect sanitized name in the filtered AIFunction output.
- LlmSessionIntegrationTests.Discovered_tools_are_retained_then_expire_after_lease_window:
ReceivedToolNames now records the sanitized alias.
New tests:
- SanitizedName_uses_double_underscore_separator pins the alias format.
- SanitizedName_matches_Anthropic_tool_name_regex (theory) pins the
documented ^[a-zA-Z0-9_-]{1,64}$ contract so future regressions fail
loudly.
- ToolRegistry.GetByName / GetRegistrationByToolName now have explicit
bidirectional-lookup coverage (canonical and sanitized both resolve
to the same registration).
Aaronontheweb
added
mcp
| /// <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; } |
Collaborator
There was a problem hiding this comment.
yeah this is documented in their tooling guidance here https://platform.claude.com/docs/en/agents-and-tools/tool-use/define-tools
This was referenced May 21, 2026
Merged
Aaronontheweb
added a commit
that referenced
this pull request
May 22, 2026
* chore: release 0.20.1 * fix(mcp): look up approval grants by canonical tool name PR #1134 introduced an Anthropic-safe sanitized alias (`server__tool`) for MCP tool names. The LLM now emits tool_use with the sanitized form, but ToolAccessPolicy builds the approval context — and LlmSessionActor.PersistApprovalCandidatesAsync records the grant — under the canonical `server/tool`. DispatchingToolExecutor.AuthorizeCoreAsync was still calling _approvalService.CheckApprovalAsync with the sanitized form (toolCall.Name), so every post-approval retry missed the recorded grant and re-threw ToolApprovalRequiredException. In production this surfaced as "I encountered an error executing a tool" loops on Notion writes: user approves the session, sees the canonical name in the Slack prompt, click registers — but the next attempt fails the gate immediately on the same session. Switch the lookup to tool.Name (canonical, already resolved via the registry's two-form GetByName) so the storage and lookup keys agree. Regression test: Mcp_session_approval_recorded_under_canonical_name_authorizes_sanitized_alias_retry mirrors the production flow — LLM-side sanitized alias on the FunctionCallContent, canonical-name RecordApprovalAsync, and the retry that must pass the gate. Also covers the canonical-name dispatch path so the registry's both-form acceptance stays exercised. * fix(mcp): drop duplicated comment block above approval lookup Self-inflicted duplication from a revert + re-apply during testing. Same comment appeared twice above the CheckApprovalAsync call.
This was referenced May 22, 2026
Merged
Aaronontheweb
added a commit
to Aaronontheweb/netclaw
that referenced
this pull request
May 22, 2026
PR netclaw-dev#1134 created two name forms for MCP tools (canonical `server/tool` for everything internal, sanitized `server__tool` for the LLM wire) but stored both as bare strings. The follow-on bug (netclaw-dev#1147) showed how easy it is to grab the wrong one — the approval lookup keyed on sanitized while the grant store keyed on canonical, no compiler signal. Make the LLM-facing form a type so misuse is a build error, not a runtime regression: - New `LlmFacingToolName` value object in Netclaw.Tools.Abstractions. `FromCanonical(string)` sanitizes `/` -> `__` and asserts the result satisfies Anthropic's tool-name regex; throws for anything that can't be expressed (spaces, dots, colons, >128 chars). No implicit string conversion — per the constitution rule on value objects. - `INetclawTool.LlmFacingName` becomes a required member. Renames the documented intent of `Name` to "canonical, operator-facing" (the prior doc lied for MCP tools). - NetclawToolGenerator emits a static readonly `_generatedLlmFacingName` via `FromCanonical`, so first-party tools with malformed names fail at type-init (i.e. at registry warm-up on daemon start) rather than at the Anthropic API boundary on the user's first call. - McpToolAdapter swaps its `SanitizedName` string for `LlmFacingName`. `SanitizedAIFunction` is constructed from `LlmFacingName.Value`. - ToolRegistry's inner `AIToolAdapter` and the two hand-written test fakes (FakeNetclawTool, RecordingContextTool) implement the new member; the registry's two-form `FindRegistration` now matches against `LlmFacingName.Value`. No behavioral changes — every call site that previously read `SanitizedName` now reads `LlmFacingName.Value` and gets the same string. This is foundation for the follow-up cleanup that pushes the canonical-vs-LLM-facing distinction out to the actual boundary (LlmSessionActor ingress/egress) instead of having every consumer guess which form to use. 586 Tools/Approval/Mcp tests pass; slopwatch clean; headers verified.
Aaronontheweb
added a commit
that referenced
this pull request
May 22, 2026
…rced (#1153) * refactor(mcp): introduce LlmFacingToolName value object PR #1134 created two name forms for MCP tools (canonical `server/tool` for everything internal, sanitized `server__tool` for the LLM wire) but stored both as bare strings. The follow-on bug (#1147) showed how easy it is to grab the wrong one — the approval lookup keyed on sanitized while the grant store keyed on canonical, no compiler signal. Make the LLM-facing form a type so misuse is a build error, not a runtime regression: - New `LlmFacingToolName` value object in Netclaw.Tools.Abstractions. `FromCanonical(string)` sanitizes `/` -> `__` and asserts the result satisfies Anthropic's tool-name regex; throws for anything that can't be expressed (spaces, dots, colons, >128 chars). No implicit string conversion — per the constitution rule on value objects. - `INetclawTool.LlmFacingName` becomes a required member. Renames the documented intent of `Name` to "canonical, operator-facing" (the prior doc lied for MCP tools). - NetclawToolGenerator emits a static readonly `_generatedLlmFacingName` via `FromCanonical`, so first-party tools with malformed names fail at type-init (i.e. at registry warm-up on daemon start) rather than at the Anthropic API boundary on the user's first call. - McpToolAdapter swaps its `SanitizedName` string for `LlmFacingName`. `SanitizedAIFunction` is constructed from `LlmFacingName.Value`. - ToolRegistry's inner `AIToolAdapter` and the two hand-written test fakes (FakeNetclawTool, RecordingContextTool) implement the new member; the registry's two-form `FindRegistration` now matches against `LlmFacingName.Value`. No behavioral changes — every call site that previously read `SanitizedName` now reads `LlmFacingName.Value` and gets the same string. This is foundation for the follow-up cleanup that pushes the canonical-vs-LLM-facing distinction out to the actual boundary (LlmSessionActor ingress/egress) instead of having every consumer guess which form to use. 586 Tools/Approval/Mcp tests pass; slopwatch clean; headers verified. * refactor(mcp): canonicalize tool names at the LLM-actor boundary Adds the inbound/outbound seam that pushes the canonical-vs-LLM-facing distinction out to the actor boundary, so internal consumers (audit log, ToolCallOutput events, duplicate-tool fingerprint, persisted assistant message, memory ledger) stop guessing which form to use. Inbound (LlmSessionActor.HandleToolCallResponse): rewrites every FunctionCallContent in the incoming AiChatMessage and the toolCalls list from the LLM-facing alias (`server__tool`) to the canonical name (`server/tool`) before any consumer reads `tc.Name`. The persisted SerializableChatMessage that flows into history therefore carries the canonical form. Tools that don't resolve (unknown name) pass through unchanged and the executor rejects them downstream as it did before. Outbound (SessionMessageAssembler.Assemble -> ChatMessageConverter. ToAiMessages): an optional `toolNameResolver` Func is threaded into the converter so it maps the persisted canonical name back to the LLM-facing alias when reconstructing FunctionCallContent for the wire. Without this hop Anthropic returns 400 on the `/` character. The session actor passes `_toolRegistry.ToLlmFacingName`; tests and internal re-drive paths leave the resolver null and get the persisted name back unchanged. Two new ToolRegistry helpers — `ToCanonicalName` and `ToLlmFacingName` — wrap the registry's existing two-form lookup. First-party tool names round-trip as identity in both directions (name is already LLM-safe by construction). Tests: - ToolRegistry: round-trip and idempotence for MCP and first-party. - ChatMessageConverter: resolver applied / not applied paths. - Full Actors test suite (1956 tests) still green. Behavioral consequence: for MCP tools the audit log, ToolCallOutput event stream, duplicate-tool nudge, and memory ledger now record `notion/notion-create-pages` instead of `notion__notion-create-pages` — matching what the operator sees in the Slack approval prompt and the persisted approval grant. No change for first-party tools. * refactor(mcp): align LLM-facing surfaces with the wire form search_tools / load_tool / discovered-tool activation all surfaced the canonical name (server/tool) while the LLM emits and receives the sanitized alias (server__tool) in tool definitions and tool_use callbacks. The model saw the same tool under two distinct strings within a single turn — confusing for weaker models, and a cross- reference hazard between the system-prompt index, the search_tools suggestion list, the load_tool result, and the tool definitions Anthropic actually returns. - SearchToolsTool: emit `tool.LlmFacingName` in both the exact-match list and the "did you mean" suggestion list. - LoadToolTool: return `tool.LlmFacingName.Value` as the confirmation string the model echoes back. - LlmSessionActor.TryActivateDiscoveredTool: resolve via the registry's two-form lookup (no change in behavior — the registry already accepted either form) and then cache + log under the canonical name so per-tool lease accounting and audit lines are unambiguous regardless of which form the LLM sent. Operator-facing surfaces (audit, ToolCallOutput, Slack approval prompt) are still canonical via the inbound seam from the previous commit. The LLM and the operator now have separate consistent identifiers, one form per audience. Test expectations updated for SearchToolsToolTests and McpToolAudienceGrantsTests where they pinned the prior (canonical) LLM-facing emission. 1956 Actors tests pass; slopwatch clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1133.
Summary
Anthropic's API enforces tool-name regex
^[a-zA-Z0-9_-]{1,128}$, which rejects the/thatMcpToolAdapteruses to namespace MCP tools as{server}/{tool}. Once the LLM tool list contains an MCP tool, every turn fails:This patch adds an Anthropic-safe alias on
McpToolAdapter({server}__{tool}, double-underscore boundary) and surfaces that to the LLM viaSanitizedAIFunction.Name. The canonicalNamestays{server}/{tool}so skill text, registry keys, and existing operator workflows that referenceserver/toolkeep working.ToolRegistry.GetByName/GetRegistrationByToolNameaccept either form sotool_useresponses (which now arrive with the sanitized name) dispatch correctly.Why
__(double underscore) and not singleMCP tool names already use single underscores (e.g.
find_completed_tasks,get_gmail_messages_content_batch). A single-underscore boundary would make the server/tool split ambiguous when consumers want to parse the name back into its parts. Double underscore is unambiguous, fits the regex, and keeps a visual server prefix.Verification
Pre-patch on 0.20.0: weekly-report skill dies at LLM call with the BadRequest above as soon as Todoist tools are loaded (reproduced repeatedly).
Post-patch: LLM receives
todoist__find-tasks; request succeeds; tool dispatches viaGetByNamematching on the sanitized alias.Test plan
McpToolAdapter/ToolRegistryunit tests still pass🤖 Generated with Claude Code