fix: isolate toolset failures in SkillToolset additional-tools resolution - #6526
Open
mittalpk wants to merge 1 commit into
Open
fix: isolate toolset failures in SkillToolset additional-tools resolution#6526mittalpk wants to merge 1 commit into
mittalpk wants to merge 1 commit into
Conversation
…tion _resolve_additional_tools_from_state used a bare asyncio.gather() over every provided toolset's get_tools_with_prefix() call. Since gather() propagates the first exception by default, one toolset failing to list its tools (e.g. an McpToolset whose server is unreachable) aborted resolution of every additional tool for that invocation, including plain BaseTool instances and healthy toolsets with no relationship to the failing one. Pass return_exceptions=True and skip (with a warning log) any toolset whose listing raised, while still re-raising CancelledError so cancellation continues to propagate. This confines a toolset failure to that toolset alone. Fixes google#6507
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #6507.
_resolve_additional_tools_from_stateinskill_toolset.pyused a bareasyncio.gather(...)over every provided toolset'sget_tools_with_prefix()call. Sincegather()propagates the first exception by default, a single toolset that fails to list its tools (e.g. anMcpToolsetwhose server is unreachable/restarting) aborted resolution of every additional tool for that invocation — including plainBaseToolinstances and healthy toolsets with no relationship to the failing one.This is easy to hit with MCP-backed toolsets in practice:
McpToolset.get_toolsopens a live session, so any restart/503/network blip makes listing raise, and the blast radius extends to every unrelatedadk_additional_toolsentry.Fix
asyncio.gather(..., return_exceptions=True), then per-toolset:logger.warning) any toolset whose listing raised, so it contributes no tools for that invocation but doesn't block the othersasyncio.CancelledErrorso cancellation still propagates correctlyTesting plan
test_resolve_additional_tools_toolset_failure_does_not_abort_others: aSkillToolsetconfigured with one healthy individual tool, one healthy toolset, and one toolset whoseget_tools_with_prefixraisesConnectionError— asserts the healthy tool and healthy toolset's tool both resolve, and the failing toolset is silently skipped.test_resolve_additional_tools_reraises_cancelled_error: confirmsasyncio.CancelledErrorfrom a toolset still propagates out of resolution rather than being swallowed.git stash) and pass after the fix.pytest tests/unittests/tools/test_skill_toolset.py: 128 passed.pytest tests/unittests/tools/: 1694 passed, 0 failed (only pre-existing unrelated deprecation warnings).pre-commit run --files src/google/adk/tools/skill_toolset.py tests/unittests/tools/test_skill_toolset.py: all hooks pass (ruff, isort, pyink, addlicense, compliance-checks, codespell).🤖 Generated with Claude Code