Repository navigation
Conversation
BichengWang
left a comment
There was a problem hiding this comment.
Validated b5cac80 offline with Python 3.12.15, Anthropic 2.2.0 and MCP 2.3.0. All 13 tests pass; against current main, the same tests produce 9 failures and 4 passes. A real local stdio MCP server also confirms that explicit errors, empty error results and server exceptions reach execute_tools() with is_error=True in both serial and parallel execution, while successful text containing “Error” remains a success. Additional cancellation probes pass in both modes.
The change preserves exception causes and the existing successful-output behavior; I found no blocking issues in this patch. The test file passes Ruff; the tool file has the same import-order warning on main and this head. I did not run the API-backed example or combine this with #518, so this approval covers the current patch in isolation and leaves the documented landing-order question open.
Summary
MCP tool failures currently reach the agent as ordinary tool results.
MCPTool.execute()ignores the SDK result'sis_errorflag, and connection exceptions are returned as strings, so the existing executor never setstool_result.is_error.This raises a
RuntimeErrorfor either failure, allowing the existing executor to set the error flag and preserve the diagnostic text. Successful output, the no-text fallback, and cancellation propagation stay unchanged. It does not add error-text guessing or change how successful text blocks are selected.Coordination
This is a draft because #518 changes the same method to preserve multiple text blocks. That work is separate and should remain there. This patch also deliberately changes direct
MCPTool.execute()failures from returned strings to exceptions, so #518's exception test would need to follow that contract if both changes land. The combination has not been tested.Would this error propagation fit the example's intended contract? I can rebase on #518 once the order is settled, without duplicating its text-aggregation changes.
Test plan
CallToolResultand AnthropicToolUseBlockobjects through the existingexecute_tools()path; only the connection call is mocked.