Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
/**
* Connects to a MCP Server, and retrieves MCP Tools into ADK Tools.
*
* <p>Server tools named {@code set_model_response}, {@code transfer_to_agent}, {@code
* adk_request_confirmation}, {@code adk_request_credential}, or {@code adk_request_input} are
* skipped with a warning, regardless of the tool filter, because ADK reserves these names.
*
* <p>Attributes:
*
* <ul>
Expand Down Expand Up @@ -143,6 +147,9 @@ public Flowable<BaseTool> getTools(ReadonlyContext readonlyContext) {
.map(
tools ->
tools.stream()
// Retain the full cached list so close() can release the session even when
// every server tool has a reserved name.
.filter(tool -> McpToolNameFilter.isAllowed(tool.name()))
.filter(tool -> isToolSelected(tool, toolFilter, readonlyContext))
.toList())
.onErrorResumeNext(
Expand Down
47 changes: 47 additions & 0 deletions core/src/main/java/com/google/adk/tools/mcp/McpToolNameFilter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* 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
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

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;
}
}
Comment on lines +17 to +47

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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;
  }
}

5 changes: 5 additions & 0 deletions core/src/main/java/com/google/adk/tools/mcp/McpToolset.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@
/**
* Connects to a MCP Server, and retrieves MCP Tools into ADK Tools.
*
* <p>Server tools named {@code set_model_response}, {@code transfer_to_agent}, {@code
* adk_request_confirmation}, {@code adk_request_credential}, or {@code adk_request_input} are
* skipped with a warning, regardless of the tool filter, because ADK reserves these names.
*
* <p>Attributes:
*
* <ul>
Expand Down Expand Up @@ -287,6 +291,7 @@ public Flowable<BaseTool> getTools(ReadonlyContext readonlyContext) {
ListToolsResult toolsResponse = this.mcpSession.listTools();
return Flowable.fromStream(
toolsResponse.tools().stream()
.filter(tool -> McpToolNameFilter.isAllowed(tool.name()))
.map(
tool ->
new McpTool(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
/*
* 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
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.adk.tools.mcp;

import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import com.google.adk.JsonBaseModel;
import com.google.adk.agents.ReadonlyContext;
import com.google.adk.models.LlmRequest;
import com.google.adk.tools.BaseTool;
import com.google.adk.tools.BaseToolset;
import com.google.adk.tools.NamedToolPredicate;
import com.google.adk.tools.SetModelResponseTool;
import com.google.adk.tools.ToolPredicate;
import com.google.genai.types.Schema;
import io.modelcontextprotocol.client.McpAsyncClient;
import io.modelcontextprotocol.client.McpSyncClient;
import io.modelcontextprotocol.json.McpJsonDefaults;
import io.modelcontextprotocol.spec.McpSchema.ListToolsResult;
import io.modelcontextprotocol.spec.McpSchema.Tool;
import java.util.ArrayList;
import java.util.List;
import org.jspecify.annotations.Nullable;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
import reactor.core.publisher.Mono;

/** Runs the same reserved-name contract against both MCP toolset implementations. */
@RunWith(Parameterized.class)
public class McpToolsetReservedNamesTest {
private static final List<String> RESERVED_NAMES =
List.of(
"set_model_response",
"transfer_to_agent",
"adk_request_confirmation",
"adk_request_credential",
"adk_request_input");

@Parameters(name = "async={0}")
public static List<Boolean> parameters() {
return List.of(false, true);
}

private final boolean async;
private final ReadonlyContext context = mock(ReadonlyContext.class);
private final McpSyncClient syncClient = mock(McpSyncClient.class);
private final McpAsyncClient asyncClient = mock(McpAsyncClient.class);

public McpToolsetReservedNamesTest(boolean async) {
this.async = async;
}

@Test
public void getTools_skipsReservedNamesAndPreservesOtherToolsInOrder() {
List<String> names = new ArrayList<>(RESERVED_NAMES);
names.add(0, "get_weather");
names.add("google_search");
names.add("set_model_response_custom");

assertThat(toolNames(createToolset(names, null)))
.containsExactly("get_weather", "google_search", "set_model_response_custom")
.inOrder();
}

@Test
public void getTools_allReservedNames_returnsEmptyAndClosesSession() throws Exception {
BaseToolset toolset = createToolset(RESERVED_NAMES, null);

assertThat(toolNames(toolset)).isEmpty();
toolset.close();

if (async) {
verify(asyncClient, atLeastOnce()).closeGracefully();
} else {
verify(syncClient).close();
}
}

@Test
public void getTools_emptyServerResponse_returnsEmpty() {
assertThat(toolNames(createToolset(List.of(), null))).isEmpty();
}

@Test
public void getTools_nameFilterCannotReenableReservedTools() {
List<String> names = new ArrayList<>(RESERVED_NAMES);
names.add("get_weather");
names.add("get_time");
List<String> selectedNames = new ArrayList<>(RESERVED_NAMES);
selectedNames.add("get_weather");

assertThat(toolNames(createToolset(names, new NamedToolPredicate(selectedNames))))
.containsExactly("get_weather");
}

@Test
public void getTools_predicateOnlySeesNonReservedToolsAndReceivesContext() {
List<String> names = new ArrayList<>(RESERVED_NAMES);
names.add("get_weather");
names.add("get_time");
List<String> inspectedNames = new ArrayList<>();
ToolPredicate predicate =
(tool, readonlyContext) -> {
assertThat(readonlyContext.orElseThrow()).isSameInstanceAs(context);
inspectedNames.add(tool.name());
return !tool.name().equals("get_time");
};

assertThat(toolNames(createToolset(names, predicate))).containsExactly("get_weather");
assertThat(inspectedNames).containsExactly("get_weather", "get_time").inOrder();
}

@Test
public void getTools_canBeCombinedWithFrameworkOutputSchemaTool() {
BaseToolset toolset = createToolset(List.of("set_model_response", "get_weather"), null);
SetModelResponseTool frameworkTool =
new SetModelResponseTool(Schema.builder().type("OBJECT").build());

LlmRequest request =
LlmRequest.builder()
.appendTools(toolset.getTools(context).toList().blockingGet())
.appendTools(List.of(frameworkTool))
.build();

assertThat(request.tools()).containsKey("get_weather");
assertThat(request.tools()).containsEntry("set_model_response", frameworkTool);
assertThat(request.tools()).hasSize(2);
}

private List<String> toolNames(BaseToolset toolset) {
return toolset.getTools(context).map(BaseTool::name).toList().blockingGet();
}

private BaseToolset createToolset(List<String> names, @Nullable ToolPredicate toolFilter) {
List<Tool> tools =
names.stream()
.map(name -> Tool.builder(name, McpJsonDefaults.getMapper(), "{}").build())
.toList();
ListToolsResult response = ListToolsResult.builder(tools).build();
McpSessionManager manager = mock(McpSessionManager.class);
if (async) {
when(manager.createAsyncSession()).thenReturn(asyncClient);
when(asyncClient.initialize()).thenReturn(Mono.empty());
when(asyncClient.listTools()).thenReturn(Mono.just(response));
when(asyncClient.closeGracefully()).thenReturn(Mono.empty());
return new McpAsyncToolset(manager, JsonBaseModel.getMapper(), toolFilter);
}
when(manager.createSession()).thenReturn(syncClient);
when(syncClient.listTools()).thenReturn(response);
return toolFilter == null
? new McpToolset(manager, JsonBaseModel.getMapper())
: new McpToolset(manager, JsonBaseModel.getMapper(), toolFilter);
}
}
Loading