Repository navigation
fix(mcp): return structured error with valid tool list on unknown tool - #561
sparkysolutions10-pixel wants to merge 2 commits into
Conversation
b3409c6 to
ef3f5c1
Compare
ef3f5c1 to
71817d6
Compare
…415) An unknown tool name threw, so the SDK surfaced a JSON-RPC transport error and the calling agent got nothing to branch on. It now answers with ERR_UNKNOWN_TOOL, the offending name, and the list of tools that do exist — which is the difference between an agent recovering and an agent stopping. Listing the valid tools in the reason is the detail worth keeping: a model that guessed a tool name can correct itself from the response instead of needing another round trip to ListTools. One fix on merge. tests/mcp.test.ts had `unknown tool > rejects instead of returning a best-effort result`, asserting the old throw — its own comment described the missing error code as the problem. That is the bug #415 describes, so the test was pinning it. Inverted the assertion rather than deleting it, and said so in the comment: it now checks isError, the ERR_UNKNOWN_TOOL code and that the reason names the tool. The dropped-test guard stays satisfied because the case still exists. #415 was assigned to rupesh-kumar-sah, and this is their PR. #561 implements the same issue and is noted there. Lens: 116 files, 1,428 tests passing, typecheck clean. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
|
Closing the loop on this one: #415 is assigned to @rupesh-kumar-sah, and their PR #444 implements the same fix. I've merged that as 8cc184e, so this is now superseded. The two are genuinely the same change — same three files ( The rule I'm applying is the one from your re-cut earlier: where two PRs implement the same issue, the assignee's goes first. That's not a judgement on the work — yours is fine — it's just the only tiebreak that is fair to whoever claimed it. Same situation on #569: it implements #505, which is assigned to @Zuliyat, whose #593 is open for it. I'll review theirs first for the same reason. Worth checking assignment before cutting a branch — there were unclaimed issues in this area and your time is better spent on those than on a tiebreak you can't win. Leaving this open rather than closing it myself, since it's yours to close. |
|
@Miracle656 Closing this out as Issue #415 has been addressed and merged via commit b9ab065. Glad to see the structured error model in place on main! |
Summary
Closes #415
Returns a structured error with a code and the list of valid tool names when an unknown tool is called, instead of throwing a raw protocol-level exception.
Changes
throw new Error(\Unknown tool: ...`)with structured error response usingERR_UNKNOWN_TOOL`.validTools: ['bazaar_search', 'paid_call']in the error response and message so calling agents can inspect and self-correct immediately.Tests
tests/mcp.test.ts:returns a structured error instead of throwing a raw exceptionlists all valid tool names in the error responselength-bounds and sanitises untrusted input in tool name echonpx vitest run tests/mcp.test.ts).