Handle message.Contents named type in agentResultToMCPCallToolResult - #721
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes MCP tool-result conversion so message.Contents (the named slice type used by Message.Contents) is handled explicitly in agentResultToMCPCallToolResult, preserving per-block content semantics (text/image/audio blocks) and proper error signaling.
Changes:
- Add a
case message.Contents:branch that converts to[]message.Contentand reuses the existing multi-content handling. - Add a regression test ensuring a tool returning
message.Contentsyields multiple MCP content blocks and setsIsErrorwhen an error block is present.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tool/mcptool/mcp.go | Adds explicit handling for message.Contents so type-switching doesn’t fall back to JSON collapsing behavior. |
| tool/mcptool/mcp_test.go | Adds a regression test covering named-slice (message.Contents) tool results, including image conversion and error-flag propagation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment has been minimized.
This comment has been minimized.
A tool may return message.Contents (type Contents []Content), the named slice type Message.Contents is declared as. The type switch in agentResultToMCPCallToolResult only matched []message.Content, so a message.Contents value fell to the default branch and was JSON-marshaled into a single TextContent, collapsing multiple blocks, encoding image and audio DataContent as base64 JSON strings, and never setting IsError for error blocks. Add a case that delegates to the existing []message.Content branch, which iterates each block, converts image/audio to mcp.ImageContent/mcp.AudioContent, and sets IsError for error content. This also fixes the FunctionResultContent.Result path.
7282cdc to
d462628
Compare
Parity Review — ApprovedThis PR fixes a bug in the unexported Cross-repo parity: The fix aligns Go with .NET and Python semantics — surfacing each content block individually as distinct Labels: Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
What
agentResultToMCPCallToolResultintool/mcptool/mcp.gotype-switched oncase message.Content:andcase []message.Content:but had no case formessage.Contents— the named slice type (type Contents []Content) thatMessage.Contentsis actually declared as.A Go type switch matches on dynamic type identity, so a
message.Contentsvalue does not matchcase []message.Content:, nor does it satisfy theContentinterface. It fell through todefault -> structuredResultToMCPCallToolResult, whichjson.Marshaled the whole slice into a singleTextContent. That:DataContentas base64 JSON strings instead ofmcp.ImageContent/mcp.AudioContent, andIsErrorfor*message.ErrorContentblocks.The same defect affected the
FunctionResultContent.Resultpath viafunctionResultToMCPCallToolResult.Fix
Add
case message.Contents:that converts to[]message.Contentand delegates to the existing correct branch. That branch already iterates each block, callsagentContentToMCPContentper item (mapping image/audio/resource types), and setsIsErrorfor error content. Minimal and surgical.Parity
This restores the .NET/Python semantics of surfacing each content block individually to MCP (distinct image/audio/text blocks and an error flag), rather than one opaque JSON text block.
Testing
Added
TestAddToolReturnsNamedContentsSliceintool/mcptool/mcp_test.go: a stub tool returnsmessage.Contents{TextContent, image DataContent, ErrorContent}and the test asserts the resultingmcp.CallToolResulthas three blocks — distinct*mcp.TextContentand*mcp.ImageContent(not one JSON text) — withIsError == true. The test fails before the fix (one collapsed block) and passes after.go build ./...,go vet ./tool/mcptool/..., andgo test ./tool/mcptool/...are green.