MCP toolset: in-model built-ins (e.g. google_search) can be shadowed by a server tool; a server tool named set_model_response aborts the run

Summary

Same two defects as the Go port, reproduced locally against adk-java (4ee155b2, core). No live service was contacted.

In-model built-in tools do not occupy their name, so a server-provided tool can shadow them. GoogleSearchTool (and GoogleMapsTool, UrlContextTool, VertexAiSearchTool, BuiltInCodeExecutionTool) override processLlmRequest to append only to config.Tools, never calling appendTools. So the duplicate-name guard in LlmRequest.Builder.appendTools never sees them, and a callable MCP tool advertising google_search is accepted and wins dispatch — Functions.handleFunctionCalls resolves by name from a map where the MCP tool overwrote the built-in.
A server-provided tool named set_model_response aborts the whole run on an agent with an output schema (IllegalArgumentException: Duplicate tool name: set_model_response). Availability, not silent displacement — but a malicious/untrusted MCP server can deny service.

Evidence

MCP tools take the server's name, unfiltered by default — tools/mcp/McpToolset.java:260-279; tools/BaseToolset.java:60-64 (if (toolFilter == null) return true;); name source tools/mcp/AbstractMcpTool.java:52-56.
Callable-vs-callable is fail-closed — models/LlmRequest.java:204-222 (appendTools, throwing merger); all tools enter via tools/BaseTool.java:187-193.
set_model_response is in the guarded class — flows/llmflows/OutputSchema.java:60-65; SetModelResponseTool.java:33-45 (extends BaseTool, does not override processLlmRequest). Processor order: SingleFlow.java:27-36, BaseLlmFlow.java:103-104.
Built-ins bypass the guard — tools/GoogleSearchTool.java:45-76 (override that appends Tool.builder().googleSearch(...) only); same in GoogleMapsTool.java:65, UrlContextTool.java:46, VertexAiSearchTool.java:60, BuiltInCodeExecutionTool.java:44.
Dispatch by name — BaseLlmFlow.java:765-766; Functions.java:295 (tools.get(functionCall.name().get())).

Local reproduction

[probe] registered tool map keys: [google_search]
[probe]   config.Tool googleSearch=true functionDeclarations=[]
[probe]   config.Tool googleSearch=false functionDeclarations=[google_search]
[probe] google_search: NO ERROR. dispatch map maps google_search -> FakeMcpTool
[probe] set_model_response: REJECTED -> Duplicate tool name: set_model_response

Suggested fix

Preferred (availability-first): reject the server-provided tool via a reserved-name check in AbstractMcpTool's constructor or in McpToolset.getTools / McpAsyncToolset.getTools, covering set_model_response, transfer_to_agent, google_search, google_maps, url_context, vertex_ai_search, code_execution, load_artifacts.

Alternative: make the in-model tools call appendTools so the existing duplicate guard applies (note: google_search would then need a clean-failing runAsync).

Same class as the Go port (google/adk-go#1605) and a separately-triaged adk-js report.

@sushant-me

Hi @sushant-me, thank you for reporting this issue and we appreciate you putting together a PR to fix it. I have successfully reproduced the issue you described. Both the issue and your PR are now under review by the team. We will keep you updated and let you know if we need any additional information. Thank you.

fix(mcp): refuse MCP server tools that use reserved framework names

the reserved-name blocklist you're leaning toward trades a shadowing bug for a drift bug. that list (google_search, set_model_response, transfer_to_agent, url_context...) now has to stay in sync with every in-model built-in across providers and versions, and the day a new one ships the guard silently stops covering it, except now it reads as fixed. the collision isn't the root cause, it's that the built-ins never occupy their name in the dispatch map. your appendTools alternative is the one that actually closes it: the namespace itself becomes authoritative instead of a parallel list someone has to remember to update.

fix(mcp): refuse MCP server tools that use reserved framework names

@sushant-me

Hi @sushant-me, we closed #1515, so the built-in tool name collision behavior will stay as is to keep consistency with the other ADKs and preserve backward compatibility. This issue stays open for the other part: an MCP server tool named like one of ADK's own framework tools (set_model_response, transfer_to_agent, adk_request_confirmation, adk_request_credential, adk_request_input). In ADK Java today, a server tool named set_model_response or transfer_to_agent makes the run fail with "Duplicate tool name" when ADK also adds that tool itself (an agent with an output schema on models that use the set_model_response workaround, or an agent with sub-agents); otherwise the server tool is used as a normal tool, and the adk_request_* names are accepted. ADK Python and ADK Kotlin skip all five of these names from MCP servers with a warning, and ADK Go has an open PR (google/adk-go#1606) that drops them. A PR that brings the same skip-with-warning behavior to Java's MCP toolsets (McpToolset and McpAsyncToolset) would be welcome.

skip-with-warning also drops the tool where it works today: a server transfer_to_agent on an agent with no sub-agents is a normal tool now, and after this it disappears behind a log line nobody reads. worth gating the skip on an actual collision?

@m13v

Resolving the question you raised, @m13v — gated on an actual collision is the right answer, and ADK Go now works that way (google/adk-go#1606, 7fae8204).

The split that makes it decidable is who registers the name. A toolset sees only the MCP server's listing; it cannot see the agent's tool list. So:

Names the framework registers out of band (set_model_response, transfer_to_agent, finish_task, task_completed, adk_request_confirmation, adk_request_credential, adk_request_input) are withheld unconditionally — the toolset cannot tell whether the framework will claim one, and neither can the author.
Names the application configures (google_search, url_context, load_memory, load_artifacts, exit_loop) are handed through, and the collision is reported by the framework's existing duplicate-name check where the full tool list is known.

That keeps transfer_to_agent working for a server on an agent with no sub-agents, which is the case you were pointing at, and it removes the drift problem you described in your first comment: nothing has to stay in sync with a parallel built-in list, because the authority is the actual tool list rather than a constant.

One caveat worth stating: the "collision" signal on the application-configured side is an error naming the duplicate, not a skip-with-warning. Making it a skip requires a place to write the diagnostic, and ADK Go forbids library code writing to a process-wide sink until it has a logger of its own. If Java already has a logger to route through, that option is open to you in a way it is not in Go.

transfer_to_agent sits in your withheld-unconditionally list though, so a server tool by that name on an agent with no sub-agents still gets dropped. that's the exact case the gate was meant to save; it only survives if that name moves to the collision-checked side.

You're right, and it was the same defect one layer down. transfer_to_agent was on the withheld list, so a server serving that name on an agent with no sub-agents still lost the tool — exactly the case the gate was supposed to save. Fixed in f1793e8b on google/adk-go#1606.

Working out why I had it wrong produced a better rule than the one I was using. I had been dividing names by "framework vs application", which is not the property that matters. The property is whether the framework registers the name as a tool, or synthesizes and interprets the call itself:

	registered as a tool?	collision detectable?	withheld
`set_model_response`	yes — `appendTools` / `PackTool`	yes, `duplicate tool: %q`	no
`transfer_to_agent`	yes — `appendTools` (`shouldUseAutoFlow`)	yes	no
`finish_task`	yes — `PackTool` (workflow agents)	yes	no
`task_completed`	yes — injected into sub-agents	yes	no
`adk_request_confirmation`	no — built as a call (`toolconfirmation.FunctionCallName`)	no	yes
`adk_request_credential`	no — `requestEUCFunctionCallName`	no	yes
`adk_request_input`	no — `workflow.WorkflowInputFunctionCallName`	no	yes

So the list is now three names, not seven, and the test is "can something downstream notice the collision". For anything registered, appendTools/PackTool already fail with the tool's name in the error, so the toolset withholding it as well only costs agents that register nothing of the sort. For the adk_request_* three there is no registration to collide with — the framework resolves those names out of the request itself — so a server tool answering to one is ambiguous on the wire and nothing downstream can see it. Those stay.

One consequence worth stating plainly, because it is a real trade-off and not free: when the framework does register one of the four, the outcome is now the duplicate-name error rather than a silent drop. That fails the run. I think that is right — a collision between an operator's MCP server and a framework dispatch name is a configuration bug to surface, and the error names it — but it is a behaviour change and a maintainer may prefer to resolve in favour of the framework tool instead.

Verified: go build ./..., go vet, go test -race -shuffle=on ./tool/mcptoolset/ and the ./tool/... + ./internal/... suites green; golangci-lint fmt clean. The four accepts/* subtests for the registered names fail with the change reverted.

failing the run on the four registered names brings back defect 2 from this issue: an untrusted server advertising set_model_response can still abort any run that has an output schema. letting the framework tool win at appendTools time keeps the gate and also keeps the run alive.

You're right, and it's the trade-off I flagged in my own reply — I just did not act on it. Failing the run does bring back the second defect: a server advertising set_model_response can abort any invocation whose agent declares an output schema. Fixed in 05df92f4 on google/adk-go#1606.

Letting the framework tool win, at the point where both lists are visible. I said earlier that resolving this needed a call site that could see both tool lists, and it turned out toolPreprocess already sits at exactly that point because of an ordering the flow already depends on:

The request processors run. toolProcessor populates the agent's tool list; then the output-schema processor packs set_model_response and the agent-transfer processor packs transfer_to_agent into the request.
Then toolPreprocess adds the agent's own tools and its toolsets' tools.

So by the time the loop runs, the framework's registrations are already in req.Tools. Snapshotting that map before the loop separates the two cases cleanly:

Collision	Resolution
Tool vs a name already in `req.Tools` (framework-registered)	skip the tool, keep the run alive
Tool vs a name added by this same loop	still an error

The second row matters: two application tools sharing a name is not a framework claim and has no correct winner, so failing loudly is right. Keeping them in one place means the gate and the run-survival requirement are satisfied without a second list of names to maintain — which was your original objection to the blocklist.

Nothing usable is lost. Once the framework owns a name, the model's call to that name is resolved by the framework rather than by the server, so the colliding tool was unreachable whether or not it got registered. Skipping it is not a silent loss of working functionality; erroring was just a worse way to express the same outcome, with a denial-of-service attached.

Verified: go build ./..., go vet, go test -race -shuffle=on ./internal/llminternal/ and ./tool/... green. TestToolPreprocess_FrameworkNameDoesNotFailTheRun fails with the change reverted, and TestToolPreprocess_DuplicateAmongAgentToolsStillFails passes both ways so the application-level duplicate still fails.

One thing I could not improve and want to name: the skip is silent. Per this repo's AGENTS.md, library code may not write to a process-wide sink and ADK has no library logger yet, so there is nowhere to record "your tool was shadowed by a framework dispatch name". If you want that diagnostic, it needs a channel decision from a maintainer rather than a slog call from me.

the snapshot defines framework-owned as 'in req.Tools before the loop', which is ordering, not ownership. any custom processor or plugin that packs a tool ahead of toolPreprocess inherits the silent win, and the app tool it shadows just vanishes.

You're right, and this is the third time you've caught a real flaw in how I drew the line. "In req.Tools before the loop" is ordering, and I used it as a proxy for ownership. Those are not the same thing, and the gap is exactly the one you name: any request processor or plugin that packs a tool before toolPreprocess runs inherits the privilege, and the application's tool of that name disappears with nothing written anywhere.

My criterion should never have been positional. Ownership is known where the framework packs, by construction — there are six such sites, and each one knows it is the framework:

internal/llminternal/agent_transfer.go:92 and :166 — transfer_to_agent
internal/llminternal/outputschema_processor.go:56 — set_model_response
internal/workflowinternal/finish_task_tool.go:151
internal/workflowinternal/single_turn_tool.go:96
internal/workflowinternal/task_agent_tool.go:78

All six already funnel through appendTools or toolutils.PackTool. So the collision should be resolved there, not inferred later: give the framework its own packing path that evicts a colliding tool when the framework registers, and drop the snapshot in toolPreprocess entirely. Then a plugin's early-registered tool is just an ordinary tool, an application duplicate still fails loudly as it did before, and nothing depends on the order processors happen to run in.

That is a six-site change rather than a one-line one, and I would rather say so than post a rushed version of it into a framework the ADKs share. I'm implementing it that way; if you would rather it be scoped differently — for instance only the dispatch names and not the workflow tools — say so and I will cut it to that.

For the record, since all three rounds have been about me getting the boundary wrong: your first objection was that the blocklist was too broad, the second was that withholding a conditionally-registered name still dropped a tool that never collided, and this one is that my replacement for both still used the wrong evidence. Each was correct.

Following the issue's own "preferred (availability-first)" fix has forced me to notice that it and the last two review points are in direct conflict, and I would rather flag that than pick one and be wrong a fourth time.

The preferred fix is an unconditional reserved-name check at the McpToolset boundary, covering set_model_response, transfer_to_agent, google_search, google_maps, url_context, vertex_ai_search, code_execution, load_artifacts. But reserving a name unconditionally is exactly what the earlier objection was about: set_model_response is only registered when the agent has an output schema, and transfer_to_agent only when sub-agents exist. An agent with neither still cannot accept a server tool by that name, so a tool that would never have collided is rejected anyway. Availability-first and ownership-precise cannot both hold. Someone has to choose, and I think it should be you rather than me.

If the answer is ownership-precise — resolve at the point the framework registers, so the collision only exists when there really is one — there is a concrete obstacle that makes it more than a six-line change, and it is worth knowing before it is committed to.

toolutils.PackTool does not merely record the tool in req.Tools. It also appends the tool's declaration into a shared genai.Tool.FunctionDeclarations list in req.Config.Tools, and there is no code anywhere that removes an entry from either place:

req.Tools[name] = t
...
if funcTool == nil {
    req.Config.Tools = append(req.Config.Tools, &genai.Tool{
        FunctionDeclarations: []*genai.FunctionDeclaration{t.Declaration()},
    })
} else {
    funcTool.FunctionDeclarations = append(funcTool.FunctionDeclarations, t.Declaration())
}

So "let the framework win" cannot be implemented by evicting the server tool from req.Tools. The evicted tool's declaration survives in req.Config.Tools, the framework then appends its own declaration of the same name, and the request reaches the model advertising set_model_response twice. That is a worse failure than the one being fixed, and it fails silently — which is the property this whole thread has been trying to eliminate.

Doing it properly means removing the colliding declaration by name from the consolidated tool, which means PackTool grows a removal path, which means deciding what happens when two providers both contribute declarations under one name. That is the real cost of the ownership-precise route, and it is the part I do not want to guess at.

What I would suggest instead, for a first change that is smaller than either: keep PackTool and appendTools exactly as they are, and have McpToolset.getTools fail closed with an error naming the tool and its server, rather than silently dropping anything. A server tool that cannot be registered becomes a visible, attributable error at toolset construction, which is availability-preserving at the boundary that is actually untrusted, and it removes the silent-drop class entirely. It does not resolve the conditional-name question, but it also does not pretend to.

I have the reserved-set change and the toolPreprocess snapshot in a branch; I am holding both until you say which direction you want, rather than shipping a fourth attempt at the same boundary.

evict-on-register only fires if the colliding tool is already present, but by your own ordering the framework packs first and the MCP tool lands later in toolPreprocess, where appendTools still throws. the entry needs a framework-owned mark that a later same-name append yields to, or defect 2 is back.

fail-closed at getTools hands the untrusted server a bigger lever, not a smaller one: a single tool named transfer_to_agent now takes the whole toolset down at construction, on every agent, instead of one run on agents with sub-agents. that's defect 2 widened. if the error has to live there, scope it per tool and let the rest of the server's tools through.

Navigation Menu

Repository navigation

MCP toolset: in-model built-ins (e.g. google_search) can be shadowed by a server tool; a server tool named set_model_response aborts the run #1513

Summary

Evidence

Local reproduction

Suggested fix

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

MCP toolset: in-model built-ins (e.g. google_search) can be shadowed by a server tool; a server tool named set_model_response aborts the run #1513

Description

Summary

Evidence

Local reproduction

Suggested fix

Activity

hemasekhar-p commented on Sep 16, 2026

m13v commented on Sep 18, 2026

kvmilos commented on Oct 12, 2026

m13v commented on Oct 12, 2026

sushant-me commented on Oct 14, 2026

m13v commented on Oct 14, 2026

sushant-me commented on Oct 15, 2026

m13v commented on Oct 16, 2026

sushant-me commented on Oct 16, 2026

m13v commented on Oct 16, 2026

sushant-me commented on Oct 16, 2026

sushant-me commented on Oct 16, 2026

m13v commented on Oct 16, 2026

m13v commented on Oct 16, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions