Skip to content

Commit bb41cc5

Browse files
Aarav Mittalcursoragent
authored andcommitted
feat(tools): let toolsets replace a failed tools listing
Apps fronting per-user OAuth MCP servers need a seam to contribute placeholder tools on HTTP 401 instead of the framework silently dropping the toolset. Add BaseToolset.on_tools_listing_error and invoke it from the agent and SkillToolset isolate-and-continue paths. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 735402d commit bb41cc5

6 files changed

Lines changed: 151 additions & 7 deletions

File tree

src/google/adk/agents/llm_agent.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -210,11 +210,11 @@ async def _convert_tool_union_to_tools(
210210
try:
211211
return await tool_union.get_tools_with_prefix(ctx)
212212
except Exception as e:
213-
# The agent still runs, just without this toolset's tools, and the model
214-
# will answer as though it never had them. That is a lost capability
215-
# rather than a degraded one, so report it at error level, name which
216-
# toolset was lost, and keep the traceback: str(e) is empty for several
217-
# of the exceptions raised by transport clients.
213+
# The agent still runs after a listing failure. Report at error level,
214+
# name which toolset was lost, and keep the traceback: str(e) is empty
215+
# for several of the exceptions raised by transport clients. Then let the
216+
# toolset decide what to contribute (default: nothing) so apps can return
217+
# placeholder tools for expected failures such as per-user OAuth 401s.
218218
logger.error(
219219
'Agent %s will run without the tools from toolset %s%s, which failed'
220220
' to load: %s',
@@ -228,7 +228,7 @@ async def _convert_tool_union_to_tools(
228228
e,
229229
exc_info=True,
230230
)
231-
return []
231+
return await tool_union.on_tools_listing_error(e, ctx)
232232

233233

234234
# TODO: drop the explicit abc.ABC base once BaseNode surfaces ABCMeta to

src/google/adk/tools/base_toolset.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,35 @@ async def get_tools(
9999
list[BaseTool]: A list of tools available under the specified context.
100100
"""
101101

102+
async def on_tools_listing_error(
103+
self,
104+
error: Exception,
105+
readonly_context: Optional[ReadonlyContext] = None,
106+
) -> list[BaseTool]:
107+
"""Decide what this toolset contributes when ``get_tools`` fails.
108+
109+
Framework call sites that load toolsets for an agent (or for skill
110+
additional tools) catch listing failures so one broken toolset cannot take
111+
down the rest. By default those call sites then contribute nothing from the
112+
failed toolset. Override this hook to contribute a replacement tool list
113+
instead — for example placeholder tools that prompt the user to complete
114+
per-user OAuth for an MCP server that returned HTTP 401.
115+
116+
Direct callers of ``get_tools`` / ``get_tools_with_prefix`` still see the
117+
original exception; this hook is only invoked by the framework's
118+
isolate-and-continue paths.
119+
120+
Args:
121+
error: The exception raised by ``get_tools`` / ``get_tools_with_prefix``.
122+
readonly_context: The same context passed to the failed listing call.
123+
124+
Returns:
125+
Tools to use in place of the failed listing. The default returns an
126+
empty list (contribute nothing), matching historical framework behavior.
127+
"""
128+
del error, readonly_context
129+
return []
130+
102131
@final
103132
async def get_tools_with_prefix(
104133
self,

src/google/adk/tools/skill_toolset.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1314,7 +1314,11 @@ async def _resolve_additional_tools_from_state(
13141314
ts_tools,
13151315
exc_info=ts_tools,
13161316
)
1317-
continue
1317+
# Let the toolset replace a failed listing (default: contribute
1318+
# nothing) so apps can surface expected auth failures as tools.
1319+
ts_tools = await toolset.on_tools_listing_error(
1320+
ts_tools, readonly_context
1321+
)
13181322
if isinstance(ts_tools, BaseException):
13191323
raise ts_tools
13201324
for t in ts_tools:

tests/unittests/agents/test_llm_agent_fields.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,38 @@ async def get_tools(self, readonly_context=None):
643643
# The traceback is what identifies where inside the toolset it broke.
644644
assert record.exc_info is not None
645645

646+
async def test_canonical_tools_uses_on_tools_listing_error_hook(self, caplog):
647+
"""A toolset can contribute placeholder tools after a listing failure."""
648+
from google.adk.tools.base_tool import BaseTool
649+
from google.adk.tools.base_toolset import BaseToolset
650+
651+
class AuthPromptToolset(BaseToolset):
652+
653+
async def get_tools(self, readonly_context=None):
654+
raise ConnectionError('HTTP 401 Unauthorized')
655+
656+
async def on_tools_listing_error(self, error, readonly_context=None):
657+
del readonly_context
658+
tool = mock.MagicMock(spec=BaseTool)
659+
tool.name = 'connect_mcp_server'
660+
tool.description = f'Authorize access ({error})'
661+
tool._get_declaration = mock.MagicMock(return_value=None)
662+
return [tool]
663+
664+
agent = LlmAgent(
665+
name='test_agent',
666+
model='gemini-pro',
667+
tools=[AuthPromptToolset()],
668+
)
669+
ctx = await _create_readonly_context(agent)
670+
671+
with caplog.at_level(logging.ERROR, logger='google_adk'):
672+
tools = await agent.canonical_tools(ctx)
673+
674+
assert len(tools) == 1
675+
assert tools[0].name == 'connect_mcp_server'
676+
assert any('failed to load' in r.getMessage() for r in caplog.records)
677+
646678

647679
# Tests for multi-provider model support via string model names
648680
@pytest.mark.parametrize(

tests/unittests/tools/test_base_toolset.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,3 +448,40 @@ async def test_get_tools_with_prefix_caching():
448448
readonly_context=readonly_context2
449449
)
450450
assert tools4 is not tools5
451+
452+
453+
@pytest.mark.asyncio
454+
async def test_on_tools_listing_error_default_contributes_nothing():
455+
"""Default hook returns an empty list so failed listings contribute nothing."""
456+
toolset = _TestingToolset()
457+
tools = await toolset.on_tools_listing_error(
458+
ConnectionError('MCP unauthorized'), readonly_context=None
459+
)
460+
assert tools == []
461+
462+
463+
@pytest.mark.asyncio
464+
async def test_on_tools_listing_error_can_be_overridden():
465+
"""Subclasses can return placeholder tools when listing fails."""
466+
467+
class _AuthPromptToolset(_TestingToolset):
468+
469+
async def on_tools_listing_error(
470+
self,
471+
error: Exception,
472+
readonly_context: Optional[ReadonlyContext] = None,
473+
) -> list[BaseTool]:
474+
del readonly_context
475+
return [
476+
_TestingTool(
477+
name='connect_mcp',
478+
description=f'Authorize MCP access ({error})',
479+
)
480+
]
481+
482+
toolset = _AuthPromptToolset()
483+
tools = await toolset.on_tools_listing_error(
484+
ConnectionError('HTTP 401'), readonly_context=None
485+
)
486+
assert len(tools) == 1
487+
assert tools[0].name == 'connect_mcp'

tests/unittests/tools/test_skill_toolset.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2403,6 +2403,8 @@ async def test_skill_toolset_resolution_isolates_failing_toolset(
24032403
failing_toolset.get_tools_with_prefix.side_effect = RuntimeError(
24042404
"MCP server unreachable"
24052405
)
2406+
# Default hook contributes nothing — same as historical skip behavior.
2407+
failing_toolset.on_tools_listing_error = mock.AsyncMock(return_value=[])
24062408

24072409
toolset = skill_toolset.SkillToolset(
24082410
[mock_skill1],
@@ -2428,6 +2430,46 @@ async def test_skill_toolset_resolution_isolates_failing_toolset(
24282430
assert "Skipping toolset" in caplog.text
24292431

24302432

2433+
@pytest.mark.asyncio
2434+
async def test_skill_toolset_resolution_uses_on_tools_listing_error_hook(
2435+
mock_skill1, caplog
2436+
):
2437+
"""A failing toolset can contribute placeholder tools via the hook."""
2438+
mock_skill1.frontmatter.metadata = {
2439+
"adk_additional_tools": ["connect_mcp_server"]
2440+
}
2441+
mock_skill1.name = "skill1"
2442+
2443+
placeholder = mock.create_autospec(skill_toolset.BaseTool, instance=True)
2444+
placeholder.name = "connect_mcp_server"
2445+
2446+
failing_toolset = mock.create_autospec(
2447+
skill_toolset.BaseToolset, instance=True
2448+
)
2449+
failing_toolset.get_tools_with_prefix.side_effect = ConnectionError(
2450+
"HTTP 401 Unauthorized"
2451+
)
2452+
failing_toolset.on_tools_listing_error = mock.AsyncMock(
2453+
return_value=[placeholder]
2454+
)
2455+
2456+
toolset = skill_toolset.SkillToolset(
2457+
[mock_skill1],
2458+
additional_tools=[failing_toolset],
2459+
)
2460+
ctx = _make_tool_context_with_agent()
2461+
2462+
load_tool = skill_toolset.LoadSkillTool(toolset)
2463+
await load_tool.run_async(args={"skill_name": "skill1"}, tool_context=ctx)
2464+
2465+
with caplog.at_level(logging.WARNING):
2466+
tools = await toolset.get_tools(readonly_context=ctx)
2467+
2468+
assert "connect_mcp_server" in {t.name for t in tools}
2469+
failing_toolset.on_tools_listing_error.assert_awaited_once()
2470+
assert "Skipping toolset" in caplog.text
2471+
2472+
24312473
@pytest.mark.asyncio
24322474
async def test_skill_toolset_resolution_propagates_system_exceptions(
24332475
mock_skill1,

0 commit comments

Comments
 (0)