From 5f34d598435a2a8d875a5dbb14344c201db0e75f Mon Sep 17 00:00:00 2001 From: Mateusz Krawiec Date: Thu, 26 Mar 2026 05:50:34 -0700 Subject: [PATCH] fix: revert changes to AbstractMcpTool, maintaining backwards compatible text_output field in the response PiperOrigin-RevId: 889787667 --- .../java/com/google/adk/models/Claude.java | 29 ++--------- .../google/adk/tools/mcp/AbstractMcpTool.java | 51 ++++++++++++++++++- .../com/google/adk/models/ClaudeTest.java | 24 ++------- .../adk/tools/mcp/AbstractMcpToolTest.java | 7 +-- 4 files changed, 60 insertions(+), 51 deletions(-) diff --git a/core/src/main/java/com/google/adk/models/Claude.java b/core/src/main/java/com/google/adk/models/Claude.java index 01feda1d4..79201c261 100644 --- a/core/src/main/java/com/google/adk/models/Claude.java +++ b/core/src/main/java/com/google/adk/models/Claude.java @@ -172,17 +172,11 @@ private ContentBlockParam partToAnthropicMessageBlock(Part part) { if (part.functionResponse().get().response().isPresent()) { Map responseData = part.functionResponse().get().response().get(); - Object contentObj = responseData.get("content"); Object resultObj = responseData.get("result"); - - if (contentObj instanceof List list && !list.isEmpty()) { - // Native MCP format: list of content blocks - content = extractMcpContentBlocks(list); - } else if (resultObj != null) { - // ADK tool result object - content = resultObj instanceof String s ? s : serializeToJson(resultObj); - } else if (!responseData.isEmpty()) { - // Fallback: arbitrary JSON structure + if (resultObj != null) { + content = resultObj.toString(); + } else { + // Fallback to json serialization of the function response. content = serializeToJson(responseData); } } @@ -196,21 +190,6 @@ private ContentBlockParam partToAnthropicMessageBlock(Part part) { throw new UnsupportedOperationException("Not supported yet."); } - private String extractMcpContentBlocks(List list) { - List textBlocks = new ArrayList<>(); - for (Object item : list) { - if (item instanceof Map m && "text".equals(m.get("type"))) { - Object textObj = m.get("text"); - textBlocks.add(textObj != null ? String.valueOf(textObj) : ""); - } else if (item instanceof String s) { - textBlocks.add(s); - } else { - textBlocks.add(serializeToJson(item)); - } - } - return String.join("\n", textBlocks); - } - private String serializeToJson(Object obj) { try { return JsonBaseModel.getMapper().writeValueAsString(obj); diff --git a/core/src/main/java/com/google/adk/tools/mcp/AbstractMcpTool.java b/core/src/main/java/com/google/adk/tools/mcp/AbstractMcpTool.java index 3b0c3d70a..d01b7c54e 100644 --- a/core/src/main/java/com/google/adk/tools/mcp/AbstractMcpTool.java +++ b/core/src/main/java/com/google/adk/tools/mcp/AbstractMcpTool.java @@ -16,6 +16,7 @@ package com.google.adk.tools.mcp; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.adk.tools.BaseTool; @@ -23,9 +24,13 @@ import com.google.common.collect.ImmutableMap; import com.google.genai.types.FunctionDeclaration; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; +import io.modelcontextprotocol.spec.McpSchema.Content; import io.modelcontextprotocol.spec.McpSchema.JsonSchema; +import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; import io.modelcontextprotocol.spec.McpSchema.ToolAnnotations; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Optional; @@ -110,7 +115,51 @@ protected static Map wrapCallResult( if (callResult == null) { return ImmutableMap.of("error", "MCP framework error: CallToolResult was null"); } + List contents = callResult.content(); + Boolean isToolError = callResult.isError(); - return objectMapper.convertValue(callResult, new TypeReference>() {}); + if (isToolError != null && isToolError) { + String errorMessage = "Tool execution failed."; + if (contents != null + && !contents.isEmpty() + && contents.get(0) instanceof TextContent textContent) { + if (textContent.text() != null && !textContent.text().isEmpty()) { + errorMessage += " Details: " + textContent.text(); + } + } + return ImmutableMap.of("error", errorMessage); + } + + if (contents == null || contents.isEmpty()) { + return ImmutableMap.of(); + } + + List textOutputs = new ArrayList<>(); + for (Content content : contents) { + if (content instanceof TextContent textContent) { + if (textContent.text() != null) { + textOutputs.add(textContent.text()); + } + } + } + + if (textOutputs.isEmpty()) { + return ImmutableMap.of( + "error", + "Tool '" + mcpToolName + "' returned content that is not TextContent.", + "content_details", + contents.toString()); + } + + List> resultMaps = new ArrayList<>(); + for (String textOutput : textOutputs) { + try { + resultMaps.add( + objectMapper.readValue(textOutput, new TypeReference>() {})); + } catch (JsonProcessingException e) { + resultMaps.add(ImmutableMap.of("text", textOutput)); + } + } + return ImmutableMap.of("text_output", resultMaps); } } diff --git a/core/src/test/java/com/google/adk/models/ClaudeTest.java b/core/src/test/java/com/google/adk/models/ClaudeTest.java index 677d40627..febcaf4be 100644 --- a/core/src/test/java/com/google/adk/models/ClaudeTest.java +++ b/core/src/test/java/com/google/adk/models/ClaudeTest.java @@ -21,7 +21,6 @@ import com.anthropic.client.AnthropicClient; import com.anthropic.models.messages.ContentBlockParam; import com.anthropic.models.messages.ToolResultBlockParam; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.genai.types.FunctionResponse; import com.google.genai.types.Part; @@ -51,11 +50,9 @@ public void setUp() throws Exception { } @Test - public void testPartToAnthropicMessageBlock_mcpNativeFormat() throws Exception { + public void testPartToAnthropicMessageBlock_mcpTool_legacyTextOutputKey() throws Exception { Map responseData = - ImmutableMap.of( - "content", - ImmutableList.of(ImmutableMap.of("type", "text", "text", "Extracted native MCP text"))); + ImmutableMap.of("text_output", ImmutableMap.of("text", "Legacy result text")); FunctionResponse funcParam = FunctionResponse.builder().name("test_tool").response(responseData).id("call_123").build(); Part part = Part.builder().functionResponse(funcParam).build(); @@ -64,21 +61,8 @@ public void testPartToAnthropicMessageBlock_mcpNativeFormat() throws Exception { (ContentBlockParam) partToAnthropicMessageBlockMethod.invoke(claude, part); ToolResultBlockParam toolResult = result.asToolResult(); - assertThat(toolResult.content().get().asString()).isEqualTo("Extracted native MCP text"); - } - - @Test - public void testPartToAnthropicMessageBlock_legacyResultKey() throws Exception { - Map responseData = ImmutableMap.of("result", "Legacy result text"); - FunctionResponse funcParam = - FunctionResponse.builder().name("test_tool").response(responseData).id("call_123").build(); - Part part = Part.builder().functionResponse(funcParam).build(); - - ContentBlockParam result = - (ContentBlockParam) partToAnthropicMessageBlockMethod.invoke(claude, part); - - ToolResultBlockParam toolResult = result.asToolResult(); - assertThat(toolResult.content().get().asString()).isEqualTo("Legacy result text"); + assertThat(toolResult.content().get().asString()) + .isEqualTo("{\"text_output\":{\"text\":\"Legacy result text\"}}"); } @Test diff --git a/core/src/test/java/com/google/adk/tools/mcp/AbstractMcpToolTest.java b/core/src/test/java/com/google/adk/tools/mcp/AbstractMcpToolTest.java index e8d9ea631..7a8b97bda 100644 --- a/core/src/test/java/com/google/adk/tools/mcp/AbstractMcpToolTest.java +++ b/core/src/test/java/com/google/adk/tools/mcp/AbstractMcpToolTest.java @@ -49,14 +49,11 @@ public void testWrapCallResult_success() { Map map = AbstractMcpTool.wrapCallResult(objectMapper, "my_tool", result); - assertThat(map).containsKey("content"); - List content = (List) map.get("content"); + assertThat(map).containsKey("text_output"); + List content = (List) map.get("text_output"); assertThat(content).hasSize(1); Map contentItem = (Map) content.get(0); - assertThat(contentItem).containsEntry("type", "text"); assertThat(contentItem).containsEntry("text", "success"); - - assertThat(map).containsEntry("isError", false); } }