Skip to content

Commit fa18d26

Browse files
AriYusaDeanChensj
authored andcommitted
fix: check if transfer target is a sibling agent
Merge google#3862 ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** - Closes: google#3850 **Problem:** `disallow_transfer_to_peers=True` only affected the instruction prompt. Subagents could still transfer tasks directly to sibling agents by inferring their names from conversation history or the user’s prompt. **Solution:** Added a check in `_get_agent_to_run`. When `disallow_transfer_to_peers=True`, the function now raises an error if a transfer to a sibling agent is attempted. I placed the check here because `_get_agent_to_run` already contains a closely related validation for transfers to non-existent agents. ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. **Manual End-to-End (E2E) Tests:** Manually tested before and after fix with `adk web` by prompting subagents to transfer directly to sibling and getting newly added error after fix ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [x] ~~Any dependent changes have been merged and published in downstream modules.~~ (not applicable) Co-authored-by: Shangjie Chen <deanchen@google.com> COPYBARA_INTEGRATE_REVIEW=google#3862 from AriYusa:fix-transfer-to-peers d0c665a PiperOrigin-RevId: 934090404
1 parent 037ec12 commit fa18d26

2 files changed

Lines changed: 112 additions & 0 deletions

File tree

src/google/adk/flows/llm_flows/base_llm_flow.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,6 +1278,16 @@ def _get_agent_to_run(
12781278
agent_to_run = root_agent.find_agent(agent_name)
12791279
if not agent_to_run:
12801280
raise ValueError(f'Agent {agent_name} not found in the agent tree.')
1281+
1282+
from google.adk.agents.llm_agent import LlmAgent
1283+
1284+
if (
1285+
isinstance(invocation_context.agent, LlmAgent)
1286+
and invocation_context.agent.disallow_transfer_to_peers
1287+
and agent_to_run.parent_agent == invocation_context.agent.parent_agent
1288+
and agent_to_run != invocation_context.agent
1289+
):
1290+
raise ValueError(f'Transfer to sibling agent {agent_name} is disallowed.')
12811291
return agent_to_run
12821292

12831293
async def _call_llm_async(

tests/unittests/flows/llm_flows/test_base_llm_flow.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
from google.adk.agents.live_request_queue import LiveRequestQueue
2222
from google.adk.agents.llm_agent import Agent
23+
from google.adk.agents.loop_agent import LoopAgent
2324
from google.adk.agents.run_config import RunConfig
2425
from google.adk.events.event import Event
2526
from google.adk.flows.llm_flows.base_llm_flow import _handle_after_model_callback
@@ -1683,3 +1684,104 @@ async def mock_receive():
16831684
call_req.live_connect_config.history_config.initial_history_in_client_content
16841685
is False
16851686
)
1687+
1688+
1689+
def _make_agent_tree():
1690+
root = Agent(name='root')
1691+
child1 = Agent(name='child1')
1692+
child2 = Agent(name='child2')
1693+
1694+
child1.parent_agent = root
1695+
child2.parent_agent = root
1696+
root.sub_agents = [child1, child2]
1697+
return root, child1, child2
1698+
1699+
1700+
@pytest.mark.asyncio
1701+
async def test_transfer_to_sibling_disallowed_raises_value_error():
1702+
"""Transfer to sibling raises ValueError when disallow_transfer_to_peers is True."""
1703+
# Arrange
1704+
root, child1, child2 = _make_agent_tree()
1705+
caller = child1
1706+
caller.disallow_transfer_to_peers = True
1707+
ctx = await testing_utils.create_invocation_context(caller)
1708+
flow = BaseLlmFlow()
1709+
1710+
# Act & Assert
1711+
with pytest.raises(
1712+
ValueError, match='Transfer to sibling agent child2 is disallowed'
1713+
):
1714+
flow._get_agent_to_run(ctx, 'child2')
1715+
1716+
1717+
@pytest.mark.asyncio
1718+
async def test_transfer_to_sibling_allowed_returns_agent():
1719+
"""Transfer to sibling returns the agent when disallow_transfer_to_peers is False."""
1720+
# Arrange
1721+
root, child1, child2 = _make_agent_tree()
1722+
caller = child1
1723+
caller.disallow_transfer_to_peers = False
1724+
ctx = await testing_utils.create_invocation_context(caller)
1725+
flow = BaseLlmFlow()
1726+
1727+
# Act
1728+
agent = flow._get_agent_to_run(ctx, 'child2')
1729+
1730+
# Assert
1731+
assert agent is not None
1732+
assert agent.name == 'child2'
1733+
1734+
1735+
@pytest.mark.asyncio
1736+
async def test_transfer_to_unknown_agent_raises_value_error():
1737+
"""Transfer to unknown agent name raises ValueError."""
1738+
# Arrange
1739+
root, child1, child2 = _make_agent_tree()
1740+
caller = child1
1741+
ctx = await testing_utils.create_invocation_context(caller)
1742+
flow = BaseLlmFlow()
1743+
1744+
# Act & Assert
1745+
with pytest.raises(ValueError, match='not found in the agent tree'):
1746+
flow._get_agent_to_run(ctx, 'not_in_tree')
1747+
1748+
1749+
@pytest.mark.asyncio
1750+
async def test_transfer_to_self_allowed_when_peers_disallowed():
1751+
"""Transfer to self is allowed even when disallow_transfer_to_peers is True."""
1752+
# Arrange
1753+
root, child1, child2 = _make_agent_tree()
1754+
caller = child1
1755+
caller.disallow_transfer_to_peers = True
1756+
ctx = await testing_utils.create_invocation_context(caller)
1757+
flow = BaseLlmFlow()
1758+
1759+
# Act
1760+
agent = flow._get_agent_to_run(ctx, 'child1')
1761+
1762+
# Assert
1763+
assert agent is not None
1764+
assert agent.name == 'child1'
1765+
1766+
1767+
@pytest.mark.asyncio
1768+
async def test_transfer_to_sibling_from_non_llm_agent_allowed():
1769+
"""Transfer to sibling is allowed when the caller is not an LlmAgent."""
1770+
# Arrange
1771+
root = Agent(name='root')
1772+
child1 = LoopAgent(name='child1')
1773+
child2 = Agent(name='child2')
1774+
1775+
child1.parent_agent = root
1776+
child2.parent_agent = root
1777+
root.sub_agents = [child1, child2]
1778+
1779+
ctx = await testing_utils.create_invocation_context(child1)
1780+
flow = BaseLlmFlow()
1781+
1782+
# Act
1783+
agent = flow._get_agent_to_run(ctx, 'child2')
1784+
1785+
# Assert
1786+
assert agent is not None
1787+
assert agent.name == 'child2'

0 commit comments

Comments
 (0)