Repository navigation
fix: skip reserved ADK framework names in MCP toolsets - #1673
NewPeople-star wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to filter out MCP server tools that collide with reserved ADK framework tool names by introducing McpToolNameFilter and applying it to both McpAsyncToolset and McpToolset. A new test suite is also added to verify this behavior. The feedback suggests adding a null check for toolName in McpToolNameFilter to prevent potential NullPointerExceptions and deduplicating the warning logs to avoid flooding.
| package com.google.adk.tools.mcp; | ||
|
|
||
| import com.google.common.collect.ImmutableSet; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| /** Excludes MCP server tools that collide with ADK framework tools. */ | ||
| final class McpToolNameFilter { | ||
| private static final Logger logger = LoggerFactory.getLogger(McpToolNameFilter.class); | ||
|
|
||
| // Keep these framework-reserved names aligned with ADK Python's MCP toolset. | ||
| private static final ImmutableSet<String> RESERVED_NAMES = | ||
| ImmutableSet.of( | ||
| "set_model_response", | ||
| "transfer_to_agent", | ||
| "adk_request_confirmation", | ||
| "adk_request_credential", | ||
| "adk_request_input"); | ||
|
|
||
| private McpToolNameFilter() {} | ||
|
|
||
| static boolean isAllowed(String toolName) { | ||
| if (RESERVED_NAMES.contains(toolName)) { | ||
| logger.warn( | ||
| "Skipping MCP tool '{}' because it collides with a reserved ADK framework tool name.", | ||
| toolName); | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
To prevent log flooding when getTools() is called repeatedly, we should ensure that the warning for a skipped reserved tool name is logged only once. Additionally, we should add a null check for toolName to prevent potential NullPointerException when calling RESERVED_NAMES.contains(toolName) (since ImmutableSet does not permit null elements).
package com.google.adk.tools.mcp;
import com.google.common.collect.ImmutableSet;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import org.jspecify.annotations.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Excludes MCP server tools that collide with ADK framework tools. */
final class McpToolNameFilter {
private static final Logger logger = LoggerFactory.getLogger(McpToolNameFilter.class);
private static final Set<String> warnedTools = ConcurrentHashMap.newKeySet();
// Keep these framework-reserved names aligned with ADK Python's MCP toolset.
private static final ImmutableSet<String> RESERVED_NAMES =
ImmutableSet.of(
"set_model_response",
"transfer_to_agent",
"adk_request_confirmation",
"adk_request_credential",
"adk_request_input");
private McpToolNameFilter() {}
static boolean isAllowed(@Nullable String toolName) {
if (toolName == null) {
return false;
}
if (RESERVED_NAMES.contains(toolName)) {
if (warnedTools.add(toolName)) {
logger.warn(
"Skipping MCP tool '{}' because it collides with a reserved ADK framework tool name.",
toolName);
}
return false;
}
return true;
}
}
Link to Issue or Description of Change
Closes #1513.
MCP tools named
set_model_responseortransfer_to_agentcan collide with ADK's framework tools and abort request construction withDuplicate tool name.Following the requested scope, both
McpToolsetandMcpAsyncToolsetnow skip these five framework-reserved names with a warning, matching ADK Python:set_model_responsetransfer_to_agentadk_request_confirmationadk_request_credentialadk_request_inputThe exclusion applies before user-defined filters, even when a filter explicitly selects a reserved name. Other server tool names, including
google_search, retain their existing behavior. The async toolset keeps its internal cache intact so the session can still be closed when every server tool is filtered out.Testing Plan
Unit Tests:
The new tests cover reserved names, normal tool ordering, empty results, name and predicate filters, context forwarding, session cleanup, and construction of a request containing ADK's
SetModelResponseTool.Against the original production code, the new suite produced 8 assertion failures and 2
Duplicate tool nameerrors. With the fix, all 12 pass. Google Java formatting andgit diff --checkalso pass. The full core test suite was not run.Manual End-to-End (E2E) Tests:
Not run against external MCP or LLM services; the regression tests use mocked MCP clients.
To verify manually, connect either toolset to an MCP server exposing an ordinary tool and the five reserved names. Confirm that only the ordinary tool is returned and warnings identify the skipped names. Use the toolset with an agent that adds
set_model_responseortransfer_to_agentand confirm request construction succeeds.Checklist
No dependent changes are required.