Support is_error flag for tool results - #2566
Conversation
Sorry, something went wrong.
…protocol#348)\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to let tool implementations explicitly mark a tool call result as an error (via CallToolResult.is_error) even when returning non-text content (e.g., images/audio), by supporting a 3-tuple return shape (unstructured_content, structured_content, is_error) and documenting the behavior in the migration guide.
Changes:
- Extend MCPServer._handle_call_tool() to interpret 3-tuples as (content, structured_content, is_error) and set CallToolResult.is_error accordingly.
- Add migration documentation describing the new 3-tuple tool return shape and the alternative of returning CallToolResult directly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/mcp/server/mcpserver/server.py | Adds tuple parsing logic intended to propagate an is_error flag into CallToolResult. |
| docs/migration.md | Documents the new 3-tuple return option for marking non-text tool results as errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| if isinstance(result, tuple): | ||
| # Support either (unstructured_content, structured_content) or | ||
| # (unstructured_content, structured_content, is_error). The third element, | ||
| # if present, controls the CallToolResult.is_error flag. | ||
| if len(result) == 2: | ||
| unstructured_content, structured_content = result | ||
| is_error = False | ||
| elif len(result) == 3: | ||
| unstructured_content, structured_content, is_error = result | ||
| else: | ||
| # Fallback: treat as a sequence of content blocks | ||
| return CallToolResult(content=list(result)) | ||
| return CallToolResult( | ||
| content=list(unstructured_content), # type: ignore[arg-type] | ||
| structured_content=structured_content, # type: ignore[arg-type] | ||
| is_error=bool(is_error), | ||
| ) |
| else: | ||
| # Fallback: treat as a sequence of content blocks | ||
| return CallToolResult(content=list(result)) | ||
| return CallToolResult( | ||
| content=list(unstructured_content), # type: ignore[arg-type] | ||
| structured_content=structured_content, # type: ignore[arg-type] | ||
| is_error=bool(is_error), |
| return CallToolResult( | ||
| content=list(unstructured_content), # type: ignore[arg-type] |
| Tools may now return a 3-tuple: (unstructured_content, structured_content, is_error). | ||
| If the third element is True, the resulting CallToolResult sent to clients will have | ||
| is_error=True. This allows returning non-text content (images, audio) while still | ||
| indicating the tool execution failed or produced an error state. | ||
|
|
||
| Alternatively, tools may return a full `CallToolResult` instance directly to control | ||
| is_error and other fields explicitly. |
|
This targets #348 rather than #2429. The discussion on #348 favors the raise-an-exception approach (#1824) over a new tuple return shape. CI is also failing across the matrix. Closing — happy to revisit if the direction on #348 changes. |
Sorry, something went wrong.
Fixes #348 — allow tools to return (unstructured_content, structured_content, is_error) so non-text results (e.g., images) can be marked as errors. Also document the behavior in docs/migration.md.