Skip to content

Commit 9b128df

Browse files
MiloszSobczykcopybara-github
MiloszSobczyk
authored and
copybara-github
committed
fix: keep structuredContent and non-text content in MCP tool results
PiperOrigin-RevId: 990288862
1 parent a88b093 commit 9b128df

2 files changed

Lines changed: 142 additions & 34 deletions

File tree

‎core/src/main/java/com/google/adk/tools/mcp/AbstractMcpTool.java‎

Lines changed: 37 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.List;
3434
import java.util.Map;
3535
import java.util.Optional;
36+
import org.jspecify.annotations.Nullable;
3637

3738
/**
3839
* Base class for MCP tools.
@@ -109,9 +110,15 @@ public Optional<FunctionDeclaration> declaration() {
109110
}
110111
}
111112

113+
/**
114+
* Converts a {@link CallToolResult} into a tool response map; a null or error result becomes a
115+
* single {@code error} entry. Text items go under {@code text_output}, each parsed as a JSON
116+
* object or else wrapped as {@code {"text": ...}}. {@code structuredContent} and {@code _meta}
117+
* are added when present, and the full ordered {@code content} list when any item is not text.
118+
*/
112119
@SuppressWarnings("PreferredInterfaceType") // BaseTool.runAsync() returns Map<String, Object>
113120
protected static Map<String, Object> wrapCallResult(
114-
ObjectMapper objectMapper, String mcpToolName, CallToolResult callResult) {
121+
ObjectMapper objectMapper, String mcpToolName, @Nullable CallToolResult callResult) {
115122
if (callResult == null) {
116123
return ImmutableMap.of("error", "MCP framework error: CallToolResult was null");
117124
}
@@ -130,36 +137,42 @@ protected static Map<String, Object> wrapCallResult(
130137
return ImmutableMap.of("error", errorMessage);
131138
}
132139

133-
if (contents == null || contents.isEmpty()) {
134-
return ImmutableMap.of();
135-
}
136-
137-
List<String> textOutputs = new ArrayList<>();
140+
List<Map<String, Object>> textOutputs = new ArrayList<>();
141+
boolean hasNonTextContent = false;
138142
for (Content content : contents) {
139143
if (content instanceof TextContent textContent) {
140-
if (textContent.text() != null) {
141-
textOutputs.add(textContent.text());
142-
}
144+
textOutputs.add(parseTextOutput(objectMapper, textContent.text()));
145+
} else {
146+
hasNonTextContent = true;
143147
}
144148
}
145149

146-
if (textOutputs.isEmpty()) {
147-
return ImmutableMap.of(
148-
"error",
149-
"Tool '" + mcpToolName + "' returned content that is not TextContent.",
150-
"content_details",
151-
contents.toString());
150+
ImmutableMap.Builder<String, Object> result = ImmutableMap.builder();
151+
if (!textOutputs.isEmpty()) {
152+
result.put("text_output", textOutputs);
153+
}
154+
// Skipped for text-only results, which would otherwise send their text twice.
155+
if (hasNonTextContent) {
156+
// Converted through the record so each item keeps its polymorphic "type" property.
157+
Map<String, Object> wireResult =
158+
objectMapper.convertValue(callResult, new TypeReference<Map<String, Object>>() {});
159+
result.put("content", wireResult.get("content"));
160+
}
161+
if (callResult.structuredContent() != null) {
162+
result.put("structuredContent", callResult.structuredContent());
152163
}
164+
if (callResult.meta() != null) {
165+
result.put("_meta", callResult.meta());
166+
}
167+
return result.buildOrThrow();
168+
}
153169

154-
List<Map<String, Object>> resultMaps = new ArrayList<>();
155-
for (String textOutput : textOutputs) {
156-
try {
157-
resultMaps.add(
158-
objectMapper.readValue(textOutput, new TypeReference<Map<String, Object>>() {}));
159-
} catch (JsonProcessingException e) {
160-
resultMaps.add(ImmutableMap.of("text", textOutput));
161-
}
170+
private static @Nullable Map<String, Object> parseTextOutput(
171+
ObjectMapper objectMapper, String text) {
172+
try {
173+
return objectMapper.readValue(text, new TypeReference<Map<String, Object>>() {});
174+
} catch (JsonProcessingException e) {
175+
return ImmutableMap.of("text", text);
162176
}
163-
return ImmutableMap.of("text_output", resultMaps);
164177
}
165178
}

‎core/src/test/java/com/google/adk/tools/mcp/AbstractMcpToolTest.java‎

Lines changed: 105 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,13 @@
2121
import static org.mockito.Mockito.mock;
2222

2323
import com.fasterxml.jackson.databind.ObjectMapper;
24+
import com.google.adk.JsonBaseModel;
2425
import com.google.common.collect.ImmutableList;
26+
import com.google.common.collect.ImmutableMap;
2527
import io.modelcontextprotocol.client.McpSyncClient;
2628
import io.modelcontextprotocol.spec.McpSchema;
2729
import io.modelcontextprotocol.spec.McpSchema.CallToolResult;
28-
import io.modelcontextprotocol.spec.McpSchema.TextContent;
29-
import java.util.List;
30+
import io.modelcontextprotocol.spec.McpSchema.ImageContent;
3031
import java.util.Map;
3132
import org.junit.Before;
3233
import org.junit.Test;
@@ -36,29 +37,123 @@
3637
@RunWith(JUnit4.class)
3738
public final class AbstractMcpToolTest {
3839

40+
private static final ImageContent IMAGE = ImageContent.builder("aW1hZ2U=", "image/png").build();
41+
private static final ImmutableMap<String, Object> IMAGE_JSON =
42+
ImmutableMap.of("type", "image", "data", "aW1hZ2U=", "mimeType", "image/png");
43+
3944
private ObjectMapper objectMapper;
4045

4146
@Before
4247
public void setUp() {
43-
objectMapper = new ObjectMapper();
48+
// The mapper McpTool uses by default, so tests see the production serialization.
49+
objectMapper = JsonBaseModel.getMapper();
50+
}
51+
52+
@Test
53+
public void wrapCallResult_textOnly_returnsOnlyTextOutput() {
54+
CallToolResult result =
55+
CallToolResult.builder().addTextContent("first").addTextContent("{\"a\":1}").build();
56+
57+
Map<String, Object> map = AbstractMcpTool.wrapCallResult(objectMapper, "my_tool", result);
58+
59+
assertThat(map)
60+
.containsExactly(
61+
"text_output",
62+
ImmutableList.of(ImmutableMap.of("text", "first"), ImmutableMap.of("a", 1)));
4463
}
4564

4665
@Test
47-
public void testWrapCallResult_success() {
66+
public void wrapCallResult_mixedContent_keepsTextOutputAndAddsContentAndStructuredContent() {
4867
CallToolResult result =
4968
CallToolResult.builder()
50-
.content(ImmutableList.of(new TextContent("success")))
69+
.addTextContent("first")
70+
.addTextContent("second")
71+
.addContent(IMAGE)
72+
.structuredContent(ImmutableMap.of("count", 2))
5173
.isError(false)
5274
.build();
5375

5476
Map<String, Object> map = AbstractMcpTool.wrapCallResult(objectMapper, "my_tool", result);
5577

56-
assertThat(map).containsKey("text_output");
57-
List<?> content = (List<?>) map.get("text_output");
58-
assertThat(content).hasSize(1);
78+
assertThat(map)
79+
.containsExactly(
80+
"text_output",
81+
ImmutableList.of(ImmutableMap.of("text", "first"), ImmutableMap.of("text", "second")),
82+
"content",
83+
ImmutableList.of(
84+
ImmutableMap.of("type", "text", "text", "first"),
85+
ImmutableMap.of("type", "text", "text", "second"),
86+
IMAGE_JSON),
87+
"structuredContent",
88+
ImmutableMap.of("count", 2));
89+
}
90+
91+
@Test
92+
public void wrapCallResult_textWithStructuredContent_keepsTextOutputAndAddsStructuredContent() {
93+
CallToolResult result =
94+
CallToolResult.builder()
95+
.addTextContent("{\"count\":2}")
96+
.structuredContent(ImmutableMap.of("count", 2))
97+
.build();
98+
99+
Map<String, Object> map = AbstractMcpTool.wrapCallResult(objectMapper, "my_tool", result);
100+
101+
assertThat(map)
102+
.containsExactly(
103+
"text_output",
104+
ImmutableList.of(ImmutableMap.of("count", 2)),
105+
"structuredContent",
106+
ImmutableMap.of("count", 2));
107+
}
108+
109+
@Test
110+
public void wrapCallResult_nonTextOnly_returnsContentWithoutError() {
111+
CallToolResult result = CallToolResult.builder().addContent(IMAGE).build();
112+
113+
Map<String, Object> map = AbstractMcpTool.wrapCallResult(objectMapper, "my_tool", result);
114+
115+
assertThat(map).containsExactly("content", ImmutableList.of(IMAGE_JSON));
116+
}
117+
118+
@Test
119+
public void wrapCallResult_emptyContent_returnsEmptyMap() {
120+
CallToolResult result = CallToolResult.builder().build();
121+
122+
Map<String, Object> map = AbstractMcpTool.wrapCallResult(objectMapper, "my_tool", result);
123+
124+
assertThat(map).isEmpty();
125+
}
126+
127+
@Test
128+
public void wrapCallResult_emptyContentWithStructuredContentAndMeta_returnsBoth() {
129+
CallToolResult result =
130+
CallToolResult.builder()
131+
.structuredContent(ImmutableMap.of("count", 2))
132+
.meta(ImmutableMap.of("trace", "abc"))
133+
.build();
134+
135+
Map<String, Object> map = AbstractMcpTool.wrapCallResult(objectMapper, "my_tool", result);
136+
137+
assertThat(map)
138+
.containsExactly(
139+
"structuredContent",
140+
ImmutableMap.of("count", 2),
141+
"_meta",
142+
ImmutableMap.of("trace", "abc"));
143+
}
144+
145+
@Test
146+
public void wrapCallResult_error_returnsOnlyError() {
147+
CallToolResult result =
148+
CallToolResult.builder()
149+
.addTextContent("boom")
150+
.structuredContent(ImmutableMap.of("count", 2))
151+
.isError(true)
152+
.build();
153+
154+
Map<String, Object> map = AbstractMcpTool.wrapCallResult(objectMapper, "my_tool", result);
59155

60-
Map<?, ?> contentItem = (Map<?, ?>) content.get(0);
61-
assertThat(contentItem).containsEntry("text", "success");
156+
assertThat(map).containsExactly("error", "Tool execution failed. Details: boom");
62157
}
63158

64159
@Test

0 commit comments

Comments
 (0)