Repository navigation
feat(spring-ai): bridge Spring AI ToolCallback to ADK BaseTool - #1300
mekkiamiri wants to merge 1 commit into
Conversation
9e8b683 to
763ce23
Compare
|
Hi @mekkiamiri (1) Schema Degradation: buildDeclaration(...) prefers Schema.fromJson(inputSchema) and assigns it via builder.parameters(...): builder.parameters(Schema.fromJson(inputSchema)); // populates FunctionDeclaration.parameters() When these tools run on the SpringAI model, ToolConverter.convertToSpringAiTools must turn the FunctionDeclaration back into a Spring AI schema, and it branches on which field is set ToolConverter.java: if (declaration.parameters().isPresent()) { Because the bridge sets parameters(), every bridged tool takes the lossy branch. The PR's test schema ({city: string}) is too flat to reveal it; a real MCP tool with an array/items, enum, or $defs reaches the model with those stripped. The parametersJsonSchema fallback has the inverse problem: it passes the raw String (builder.parametersJsonSchema(inputSchema)), so the faithful branch does writeValueAsString("{...}") → a double-encoded, quoted string. Option buildDeclaration is currently static and has no ObjectMapper; pass the instance mapper in (it's already a field). Current: Suggestion (entirely within SpringAiToolCallbackBackedAdkTool): (2) Additional tests, not only mocks Nothing exercises a real SyncMcpToolCallback/AsyncMcpToolCallback or an MCP-shaped result (content arrays, isError, the JSON the MCP callback actually emits) (3) Exception handling This needs to be documented in the README as well (4) Autoconfigure can be updated to add automatic discovery of ToolCallback beans |
ddobrin
left a comment
ddobrin
left a comment
There was a problem hiding this comment.
Suggestions left in the PR comments
763ce23 to
85d439d
Compare
…autoconfig discovery Addresses 3 of the 4 review comments on PR google#1300: 1. Schema degradation (fidelity) — buildDeclaration now sets FunctionDeclaration.parametersJsonSchema(Map) with the parsed JSON schema, not parameters(Schema.fromJson). Downstream ToolConverter takes the faithful branch, preserving items/enum/format/anyOf/oneOf/ additionalProperties/$defs/$ref. Malformed schemas leave the field unset (logged warning) instead of emitting a double-encoded quoted string via the old parametersJsonSchema(String) fallback. 2. Exception handling — runAsync now catches exceptions thrown by ToolCallback.call(...) and returns Map.of("error", <message>), matching AbstractMcpTool.wrapCallResult(...). Falls back to the exception's simple class name when message is null. The agent flow observes a structured error result instead of an aborted Single. 3. Autoconfigure discovery — new SpringAiToolBridgeAutoConfiguration auto-discovers every ToolCallback bean and exposes them as @bean("springAiTools") List<BaseTool>. Users inject the list directly into their agent factory; no more manual wrapAll(...) call. Registered in META-INF/spring/AutoConfiguration.imports. Tests updated: 9 tests total (was 7). New tests cover the faithful schema path (rich schema with items/enum/additionalProperties preserved), exception handling with and without a message, and the unparseable- schema-leaves-fields-unset behaviour. README documents error handling behaviour, autoconfig discovery, and schema fidelity in three new subsections. Out of scope for this commit: adding integration tests against a real SyncMcpToolCallback / AsyncMcpToolCallback — those require an MCP testcontainer setup and will be handled in a follow-up.
|
@ddobrin I updated the pr. could you please check again ? |
…autoconfig discovery Addresses 3 of the 4 review comments on PR google#1300: 1. Schema degradation (fidelity) — buildDeclaration now sets FunctionDeclaration.parametersJsonSchema(Map) with the parsed JSON schema, not parameters(Schema.fromJson). Downstream ToolConverter takes the faithful branch, preserving items/enum/format/anyOf/oneOf/ additionalProperties/$defs/$ref. Malformed schemas leave the field unset (logged warning) instead of emitting a double-encoded quoted string via the old parametersJsonSchema(String) fallback. 2. Exception handling — runAsync now catches exceptions thrown by ToolCallback.call(...) and returns Map.of("error", <message>), matching AbstractMcpTool.wrapCallResult(...). Falls back to the exception's simple class name when message is null. The agent flow observes a structured error result instead of an aborted Single. 3. Autoconfigure discovery — new SpringAiToolBridgeAutoConfiguration auto-discovers every ToolCallback bean and exposes them as @bean("springAiTools") List<BaseTool>. Users inject the list directly into their agent factory; no more manual wrapAll(...) call. Registered in META-INF/spring/AutoConfiguration.imports. Tests updated: 9 tests total (was 7). New tests cover the faithful schema path (rich schema with items/enum/additionalProperties preserved), exception handling with and without a message, and the unparseable- schema-leaves-fields-unset behaviour. README documents error handling behaviour, autoconfig discovery, and schema fidelity in three new subsections. Out of scope for this commit: adding integration tests against a real SyncMcpToolCallback / AsyncMcpToolCallback — those require an MCP testcontainer setup and will be handled in a follow-up.
f2c6377 to
3bd1882
Compare
|
Hi @mekkiamiri, thank you for your contribution! We appreciate you taking the time to submit this pull request.To move forward with the review process, could you please squash your commits into a single commit? |
hemasekhar-p
added
the
waiting on reporter
3bd1882 to
9b0790c
Compare
|
@hemasekhar-p done |
|
@mekkiamiri, thank you for addressing the feedback and squashing your commits, Currently this PR is under review by our team, we will keep you posted if any additional information is required. thank you. |
hemasekhar-p
added
needs review
and removed
waiting on reporter
9b0790c to
499ab37
Compare
499ab37 to
70ff226
Compare
|
|
||
| ### How it works | ||
|
|
||
| `SpringAiToolCallbackBackedAdkTool` reads `ToolCallback.getToolDefinition()` to extract the tool name, description, and JSON Schema. The schema is converted to ADK's `Schema` type via `Schema.fromJson(...)`; if parsing fails the bridge falls back to the `parametersJsonSchema(Object)` escape hatch (no hard failure). At invocation time the bridge serializes the `Map<String, Object>` arguments to JSON, dispatches to `ToolCallback.call(String)`, and parses the JSON response back to `Map<String, Object>`. Non-object responses (primitives / arrays / arbitrary strings) are wrapped under a `"result"` key for structural consistency. |
There was a problem hiding this comment.
MEDIUM - Please rewrite this section for Spring AI 2.0. 2.0 removed the tool loop from every ChatModel (upgrade notes), so with .model(springAI) ADK runs the bridged tools. In 2.0, @Tool objects become callbacks through ToolCallbacks.from(...) or a MethodToolCallbackProvider, and MCP tools arrive through a ToolCallbackProvider. Also drop what no longer matches the code: Schema.fromJson, the lost items/enum (fixed in #1617), and the wrapCallResult comparison.
| public LlmAgent rootAgent(SpringAI springAI, List<ToolCallback> mcpToolCallbacks) { | ||
| return LlmAgent.builder() | ||
| .name("root_agent") | ||
| .model(springAI) | ||
| .tools(SpringAiToolCallbackBackedAdkTool.wrapAll(mcpToolCallbacks)) | ||
| .instruction("Use the available tools to answer the user.") | ||
| .build(); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| That's it. The `List<ToolCallback>` is auto-injected by `spring-ai-starter-mcp-client`'s `McpToolCallbackAutoConfiguration`. The bridge converts every callback into a `BaseTool`. The agent uses them transparently. |
There was a problem hiding this comment.
HIGH - Under Spring AI 2.0 this injects no MCP tools. The 2.0 MCP starter registers SyncMcpToolCallbackProvider/AsyncMcpToolCallbackProvider beans (source), not ToolCallbacks; verified with an ApplicationContextRunner (0 tools, no error). The provider lists tools lazily and drops its cache on McpToolsChangedEvent (source), so query it per request through a BaseToolset, like ADK Kotlin's SpringAiToolset.
|
|
||
| ### Coexistence with ADK's native MCP | ||
|
|
||
| ADK ships its own MCP client in `com.google.adk.tools.mcp.*` (CLI / non-Spring-Boot scenarios). The two paths can be mixed at the `.tools(...)` boundary — both produce `BaseTool` instances — but it is strongly recommended to **pick one** in any given application. The Spring AI MCP route is the natural choice for Spring Boot apps because everything is property-driven; ADK's native `McpToolset` remains the right choice for non-Spring usage. |
There was a problem hiding this comment.
LOW - Worth stating the trade-off here. Through Spring AI 2.0, MCP tools lose outputSchema and structuredContent: McpToolUtils.createToolDefinition copies only name, description and input schema (source), and SyncMcpToolCallback returns only content (source). ADK's McpToolset keeps the output schema as responseJsonSchema (source).
| @AutoConfiguration | ||
| @ConditionalOnClass({ToolCallback.class, SpringAiToolCallbackBackedAdkTool.class}) | ||
| public class SpringAiToolBridgeAutoConfiguration { | ||
|
|
||
| @Bean(name = "springAiTools") | ||
| @ConditionalOnBean(ToolCallback.class) | ||
| @ConditionalOnMissingBean(name = "springAiTools") | ||
| public List<BaseTool> springAiTools(List<ToolCallback> toolCallbacks) { | ||
| return SpringAiToolCallbackBackedAdkTool.wrapAll(toolCallbacks); |
There was a problem hiding this comment.
HIGH - This doesn't match how Spring AI 2.0 exposes tools. 2.0 dropped bean-name resolution (upgrade notes), so tools arrive as ToolCallback or ToolCallbackProvider beans (@Tool objects, MCP). Its own ToolCallingAutoConfiguration reads both and leaves MCP providers out of its eager list (source). This class reads ToolCallback only, so MCP and @Tool tools never show up (verified). Either mirror that with a toolset, ordering (@AutoConfigureOrder(LOWEST_PRECEDENCE), as SpringAIAutoConfiguration does since #1502), the adk.spring-ai.auto-configuration.enabled guard and tests, or drop it.
| /* | ||
| * Copyright 2026 Google LLC | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| */ |
There was a problem hiding this comment.
LOW - The license header is truncated: the "Unless required by applicable law..." paragraph is missing here and in the other two new files (see java.header).
| } | ||
| try { | ||
| String responseJson = toolCallback.call(requestJson); | ||
| return parseResponse(responseJson); |
There was a problem hiding this comment.
LOW - In Spring AI 2.0 the tool loop (ToolCallingAdvisor or ToolCallingManager) applies ToolMetadata.returnDirect(). ADK replaces that loop, so the flag is silently ignored. Map it to skipSummarization after a successful call, as Kotlin does (L74-L78).
| } catch (Exception callFailed) { | ||
| // Match AbstractMcpTool.wrapCallResult(...) — return a structured error map rather | ||
| // than propagate the raw exception through the ADK Single, so the agent flow can | ||
| // observe the failure as a tool result instead of aborting the invocation. | ||
| logger.warn( | ||
| "Spring AI tool '{}' invocation failed: {}", name(), callFailed.getMessage()); | ||
| return ImmutableMap.of( | ||
| "error", | ||
| callFailed.getMessage() == null | ||
| ? callFailed.getClass().getSimpleName() | ||
| : callFailed.getMessage()); |
There was a problem hiding this comment.
MEDIUM - In 2.0, ADK runs these tools, so Spring AI's DefaultToolExecutionExceptionProcessor never sees their failures. Its contract: a ToolExecutionException with a runtime cause becomes a message for the model, and everything else is rethrown (source). McpError is a hard failure in 2.0 (source). The catch-all sends everything to the model and skips ADK's onToolErrorCallback: live, it ran 0 times here and once with propagation. Apply that contract here
| private Map<String, Object> parseResponse(String responseJson) { | ||
| if (responseJson == null || responseJson.isBlank()) { | ||
| return ImmutableMap.of(); | ||
| } | ||
| try { | ||
| return objectMapper.readValue(responseJson, MAP_TYPE); | ||
| } catch (Exception notAnObject) { | ||
| Object decoded = tryDecodeAsJsonValue(responseJson); | ||
| return ImmutableMap.of("result", decoded); | ||
| } | ||
| } | ||
|
|
||
| private Object tryDecodeAsJsonValue(String responseJson) { | ||
| try { | ||
| return objectMapper.readTree(responseJson); | ||
| } catch (Exception notJson) { | ||
| return responseJson; | ||
| } | ||
| } |
There was a problem hiding this comment.
HIGH - Decode what Spring AI 2.0 actually returns. Its DefaultToolCallResultConverter writes JSON: null for a null result and quoted strings (source). MCP callbacks return the content array. Today a null aborts the run with an NPE, readTree turns "404 Not Found" into 404, and arrays stay JsonNodes, which FirestoreSessionService persists as {nodeType=ARRAY, ...} (all verified). Decode strictly into plain values, with null as an empty map, like Kotlin (L72-L73, L117-L123). Spring AI's JsonHelper won't help: its mapper turns off FAIL_ON_TRAILING_TOKENS (source).
objectMapper.readerFor(Object.class).with(DeserializationFeature.FAIL_ON_TRAILING_TOKENS).readValue(json)| } catch (Exception schemaUnparseable) { | ||
| // Genuinely malformed JSON: leave parameters unset rather than emit a degraded schema. | ||
| logger.warn( | ||
| "Spring AI tool '{}' has an unparseable input schema; declaring it with no parameter" | ||
| + " schema. Cause: {}", | ||
| def.name(), | ||
| schemaUnparseable.getMessage()); |
There was a problem hiding this comment.
MEDIUM - Spring AI 2.0's DefaultToolDefinition always has an input schema (source), but a custom ToolDefinition may not. Then this leaves the declaration with no parameters, and on the SpringAI path ToolConverter throws IllegalArgumentException: inputType cannot be null (reproduced). Fall back to an empty object schema, as Kotlin does (L103-L115).
| private static ToolCallback mockCallback(String name, String description, String inputSchema) { | ||
| ToolCallback cb = mock(ToolCallback.class); | ||
| ToolDefinition def = | ||
| DefaultToolDefinition.builder() | ||
| .name(name) | ||
| .description(description) | ||
| .inputSchema(inputSchema) | ||
| .build(); | ||
| when(cb.getToolDefinition()).thenReturn(def); | ||
| return cb; | ||
| } |
There was a problem hiding this comment.
MEDIUM - These mock ToolCallback, so none of the Spring AI 2.0 behavior above is exercised, and point (2) of the earlier review is still open. Use real 2.0 callbacks (FunctionToolCallback.builder, ToolCallbacks.from(new Tools()), ToolCallbackProvider.from(...)) and mirror ADK Kotlin's SpringAiToolTest: returnDirect, array, scalar and null results, blank schema, toolset, and the context key.
damianmomotgoogle
added
waiting on reporter
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
If applicable, please follow the issue templates to provide as much detail as
possible.
Problem:
Spring AI provides a rich ecosystem of
ToolCallbackimplementations:SyncMcpToolCallback/AsyncMcpToolCallbackproduced byspring-ai-starter-mcp-clientfromspring.ai.mcp.client.*properties,@Tool-annotated method callbacks, programmatically declaredFunctionToolCallbacks, and so on. Today, none of these can be used by an ADKLlmAgentwithout manual conversion. The existingToolConverterincontrib/spring-aionly converts ADKBaseTool→ Spring AIToolCallback(one direction). Spring Boot users who want their ADK agent to use MCP tools have to either reimplement MCP wiring via ADK's nativeMcpToolset.createMcpToolset(...)Java API, or write their own ToolCallback → BaseTool adapter.Solution:
Add
SpringAiToolCallbackBackedAdkTool— an adapter that wraps any Spring AIToolCallbackas an ADKBaseTool. It is the reverse direction of the existingToolConverter. Together they make ADK and Spring AI tool ecosystems fully interoperable.How it works:
ToolCallback.getToolDefinition()provides the tool name, description, and JSON Schema.SchemaviaSchema.fromJson(...). If parsing fails (e.g. malformed schema), the bridge falls back toparametersJsonSchema(Object)with a logged warning — no hard failure.Map<String, Object>arguments are serialized to JSON, dispatched toToolCallback.call(String), and the JSON response is parsed back toMap<String, Object>."result"key for structural consistency.wrapAll(List<? extends ToolCallback>)helper fans a list of callbacks (e.g. fromMcpToolCallbackProvider.getToolCallbacks()) into aList<BaseTool>in one call.Usage: