Skip to content

fix(mcp): failed generated tools return an MCP error, not success text - #59

Merged
Zernio-Elean merged 1 commit into
developfrom
fix/mcp-tool-errors-set-is-error
Oct 30, 2026
Merged

Zernio-Elean merged 1 commit into
developfrom
fix/mcp-tool-errors-set-is-error

Conversation

@Zernio-Elean

Copy link
Copy Markdown
Contributor

Why

Every generated MCP tool (1122 of them) caught the SDK exception and returned Error: ... as normal text, so clients got isError: false on a failed call. A customer's agent read an X DM safety rejection (403) from messages_create_inbox_conversation as an attempted send.

Feedback: feedback_6ae377fd1995255eb637b6ba

What

  • scripts/generate_mcp_tools.py emits raise ToolError(f'Error: {e}') from e, so the error text stays the same and is now flagged as an error.
  • Regenerated generated_tools.py: only the 1122 error lines and the import change (byte-identical otherwise).
  • Regression test through the call_tool proxy: a 403 comes back with is_error true and X's text (it fails on develop).

Follow-ups

  • Hand-written tools in server.py still return Error: ... strings (e.g. support_ask, which its tests assert). Not touched here.

Every generated tool caught the SDK exception and returned 'Error: ...' as
normal content, so MCP clients saw isError false on a rejected write (an X DM
safety rejection read as an attempted send). Raise ToolError instead, keeping
the same text.
@Zernio-Elean

Copy link
Copy Markdown
Contributor Author

Review (opus): APPROVE

  1. Correctness: fastmcp 3.4.4 (uv.lock) re-raises any FastMCPError (ToolError included) from call_tool before the masking branch, so the text reaches the client unmasked, and no server in src/late/mcp sets mask_error_details. The new test proves the call_tool proxy path: is_error true, [403] and the X text preserved. A direct call goes through the same FastMCP.call_tool handler.
  2. Regressions: nothing in src/late/mcp or tests calls a generated tool and inspects an Error: string. The hand-written Error: returns in server.py (support_ask, _platform_errors) are untouched, as the Follow-up says. The generator is the source, and generate.yml reruns it plus ruff, so future regenerations keep the change.
  3. Regenerated file: the diff is exactly 1122 return f"Error: {e}" lines changed to raise ToolError(f"Error: {e}") from e, plus the one import. ruff check and format are clean.
  4. Tests: full suite 1427 passed, 14 skipped, 1 xfailed. The test name states the behavior, and the module docstring states intent only.
  5. PR body: short Why/What/Follow-ups with no attribution line.

@Zernio-Elean
Zernio-Elean merged commit b65280f into develop Oct 30, 2026
4 checks passed
@Zernio-Elean
Zernio-Elean deleted the fix/mcp-tool-errors-set-is-error branch October 30, 2026 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant