fix(mcp): surface MCP tool errors as clean attributed messages, not raw JSON (#1495)#1510
Merged
Aaronontheweb merged 5 commits intoJun 29, 2026
Conversation
…aw JSON (netclaw-dev#1495) netclaw-dev#1495 was filed as "fail explicitly when a tool is not loaded", but investigation showed that premise doesn't hold: netclaw only offers the model loaded tools, so it can't call an unloaded one, and there is no "not loaded" gate to add. The real defect is what the model sees when an MCP tool *fails*. The MCP SDK (McpClientTool.InvokeCoreAsync) returns clean AIContent for a successful result, but on isError=true it returns the ENTIRE CallToolResult serialized as a JsonElement. McpClientManager then did result?.ToString(), handing the model a raw blob — {"content":[{"type":"text","text":"..."}],"isError":true} — which it cannot distinguish from a transport error or a netclaw error. That is exactly the "error occurred invoking 'edit'" the reporter saw: the MCP server's own error text, buried in a JSON dump, which the agent then misdiagnosed as "not loaded". Adds McpToolResultFormatter: detects an isError JsonElement result, extracts the text content, and surfaces it as `Error: MCP tool 'server/tool' reported a failure: <text>`. Applied at both invocation sites (McpClientManager for the session-scoped/shared invoker path, McpToolAdapter for the bound-tool path). Successful and structured (non-error) results pass through unchanged; transport exceptions keep their existing `Error: MCP tool 'X' failed: <ex>` wrapper. Tests cover error extraction (single/multiple/empty text blocks), non-error passthrough, and plain/null results. Daemon MCP suite (59) green; build clean.
Verified findings from the workflow code review:
- Error detail no longer collapses to "no detail provided" when the server puts
it in structuredContent (or a non-text block): ExtractDetail now falls back to
structuredContent, so the message never hides detail the old raw blob exposed.
- Consistent attribution: McpClientManager passes the server-qualified
"server/tool" name (not the bare tool name) to the formatter, matching the
bound-tool path (McpToolAdapter.Name). Same error now reads the same regardless
of which invocation path produced it — and names which server failed when two
expose a same-named tool.
- Structured SUCCESS results (also serialized to a full CallToolResult by the
SDK) now surface their clean content instead of the {content,structuredContent,
isError:false} wrapper, so success isn't handed to the model as a raw blob with
a literal isError field either.
Also documents the MCP failure-surfacing behavior in the netclaw-operations skill
(tools reference) and bumps metadata.version 2.20.0 -> 2.21.0, per the System
Skills Sync Rule.
Tests expanded: structuredContent error-detail fallback, structured-success clean
surfacing (text and structured-only). Formatter+adapter (26) and Daemon MCP (59)
green; headers + slopwatch clean.
Aaronontheweb
added
tools
Aaronontheweb
enabled auto-merge (squash)
June 27, 2026 14:14
Aaronontheweb
disabled auto-merge
June 27, 2026 14:15
Aaronontheweb
enabled auto-merge (squash)
June 29, 2026 14:38
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
1 participant
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.
What
Fixes #1495. The issue was filed as "tool execution should fail explicitly when a tool is not loaded," but investigation showed that premise doesn't hold — and pointed at a different, real defect.
Investigation (why the premise was wrong)
FilterExposedTools→DiscoveredToolCache), so the model can't call an unloaded tool in normal operation — there's no "not loaded" gate to add.ModelContextProtocol.Core1.4.0,McpClientTool.InvokeCoreAsync) showed the actual mechanism: on a successful result it returns cleanAIContent; but when the server setsisError: trueit falls through and returns the entireCallToolResultserialized as aJsonElement.McpClientManagerthen didresult?.ToString(), handing the model a raw blob:{"content":[{"type":"text","text":"An error occurred invoking 'edit'"}],"isError":true}"error occurred invoking 'edit'"the reporter saw — the memorizer server's own error text, buried in a JSON dump. The agent couldn't tell a tool-reported failure from a transport/netclaw error, so it misdiagnosed it as "not loaded."Fix
McpToolResultFormatter: when an invocation result is anisErrorJsonElement, extract the text content and surface it as:Applied at both invocation sites:
McpClientManager.InvokeFunctionAsync(session-scoped / shared invoker path) — now threaded with the tool name.McpToolAdapter.ExecuteViaBoundToolAsync(bound-tool path).Unchanged: successful results, structured (non-error) results (also arrive as
JsonElement— must not be reframed as failures), and transport exceptions (keep their existingError: MCP tool 'X' failed: <ex>wrapper).Tests
McpToolResultFormatterTests: error → attributed message (single/multiple/empty text blocks); non-error JSON passthrough; plain-string and null passthrough. Daemon MCP suite (59) green; build + headers + slopwatch clean.Note
This reframes the issue from "not loaded gate" to "MCP error surfacing." If you'd prefer #1495 stay scoped to its original title, I can retitle/close and file this under a fresh issue — but the fix here is the concrete, evidence-backed improvement behind the report.