Skip to content

Commit 0880bbe

Browse files
wukathGWeale
andauthored
fix: end a ParallelAgent early only when a direct sub-agent escalates (cherry-pick to release/candidate) (#6918)
Co-authored-by: George Weale <gweale@google.com>
1 parent 401cd6d commit 0880bbe

2 files changed

Lines changed: 100 additions & 9 deletions

File tree

‎src/google/adk/agents/parallel_agent.py‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,21 @@ def _create_branch_ctx_for_sub_agent(
5555
return invocation_context
5656

5757

58-
def _has_escalate_action(event: Event) -> bool:
59-
"""Returns whether the event asks the parent workflow to exit early."""
60-
return bool(event.actions.escalate)
58+
def _asks_this_agent_to_exit(event: Event, sub_agent_names: set[str]) -> bool:
59+
"""Returns whether the event asks this parallel agent to exit early.
60+
61+
An escalation ends the workflow that directly encloses the escalating agent,
62+
and that workflow re-yields the event while unwinding, so only an escalation
63+
authored by a direct sub-agent is addressed to this one.
64+
65+
Args:
66+
event: The event to inspect.
67+
sub_agent_names: Names of this agent's direct sub-agents.
68+
69+
Returns:
70+
Whether this parallel agent should stop its remaining branches.
71+
"""
72+
return bool(event.actions.escalate) and event.author in sub_agent_names
6173

6274

6375
def _cancel_tasks(tasks: list[asyncio.Task[None]]) -> None:
@@ -69,6 +81,7 @@ def _cancel_tasks(tasks: list[asyncio.Task[None]]) -> None:
6981

7082
async def _merge_agent_run(
7183
agent_runs: list[AsyncGenerator[Event, None]],
84+
sub_agent_names: set[str],
7285
) -> AsyncGenerator[Event, None]:
7386
"""Merges agent runs using asyncio.TaskGroup on Python 3.11+."""
7487
sentinel = _AgentRunComplete()
@@ -121,7 +134,7 @@ async def process_an_agent(
121134
raise payload
122135
else:
123136
yield event
124-
if _has_escalate_action(event):
137+
if _asks_this_agent_to_exit(event, sub_agent_names):
125138
_cancel_tasks(tasks)
126139
return
127140
# Signal to agent that it should generate next event.
@@ -135,6 +148,7 @@ async def process_an_agent(
135148
# TODO - remove once Python <3.11 is no longer supported.
136149
async def _merge_agent_run_pre_3_11(
137150
agent_runs: list[AsyncGenerator[Event, None]],
151+
sub_agent_names: set[str],
138152
) -> AsyncGenerator[Event, None]:
139153
"""Merges agent runs for Python 3.10 without asyncio.TaskGroup.
140154
@@ -143,6 +157,7 @@ async def _merge_agent_run_pre_3_11(
143157
144158
Args:
145159
agent_runs: Async generators that yield events from each agent.
160+
sub_agent_names: Names of the parallel agent's direct sub-agents.
146161
147162
Yields:
148163
Event: The next event from the merged generator.
@@ -190,7 +205,7 @@ async def process_an_agent(
190205
raise payload
191206
else:
192207
yield event
193-
if _has_escalate_action(event):
208+
if _asks_this_agent_to_exit(event, sub_agent_names):
194209
_cancel_tasks(tasks)
195210
return
196211
# Signal to agent that event has been processed by runner and it can
@@ -246,6 +261,7 @@ async def _run_async_impl(
246261
yield self._create_agent_state_event(ctx)
247262

248263
agent_runs = []
264+
sub_agent_names = {sub_agent.name for sub_agent in self.sub_agents}
249265
# Prepare and collect async generators for each sub-agent.
250266
for sub_agent in self.sub_agents:
251267
sub_agent_ctx = _create_branch_ctx_for_sub_agent(self, sub_agent, ctx)
@@ -261,10 +277,10 @@ async def _run_async_impl(
261277
if sys.version_info >= (3, 11)
262278
else _merge_agent_run_pre_3_11
263279
)
264-
async with Aclosing(merge_func(agent_runs)) as agen:
280+
async with Aclosing(merge_func(agent_runs, sub_agent_names)) as agen:
265281
async for event in agen:
266282
yield event
267-
if _has_escalate_action(event):
283+
if _asks_this_agent_to_exit(event, sub_agent_names):
268284
escalated = True
269285
if ctx.should_pause_invocation(event):
270286
pause_invocation = True

‎tests/unittests/agents/test_parallel_agent.py‎

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from google.adk.agents.base_agent import BaseAgent
2424
from google.adk.agents.base_agent import BaseAgentState
2525
from google.adk.agents.invocation_context import InvocationContext
26+
from google.adk.agents.loop_agent import LoopAgent
2627
from google.adk.agents.parallel_agent import _merge_agent_run_pre_3_11
2728
from google.adk.agents.parallel_agent import ParallelAgent
2829
from google.adk.agents.sequential_agent import SequentialAgent
@@ -461,7 +462,7 @@ async def test_sub_agent_failure_reaches_busy_caller(
461462
async def test_merge_agent_run_pre_3_11_surfaces_failure_without_events():
462463
"""A branch failing before it emits anything must not look successful."""
463464
with pytest.raises(ValueError, match='simulated sub-agent failure'):
464-
async for _ in _merge_agent_run_pre_3_11([_failing_agent()]):
465+
async for _ in _merge_agent_run_pre_3_11([_failing_agent()], set()):
465466
pass
466467

467468

@@ -475,7 +476,7 @@ async def test_merge_agent_run_pre_3_11_no_aclose_error_on_failure():
475476
agent_runs = [_slow_agent_with_cleanup_delay(), _failing_agent()]
476477

477478
with pytest.raises(ValueError, match='simulated sub-agent failure'):
478-
async for _ in _merge_agent_run_pre_3_11(agent_runs):
479+
async for _ in _merge_agent_run_pre_3_11(agent_runs, set()):
479480
pass
480481

481482
# If tasks were not properly awaited, aclose() on a still-running generator
@@ -705,3 +706,77 @@ def mock_should_pause(event: Event) -> bool:
705706
assert events[0].author == fast_agent.name
706707
assert events[1].author == escalating_agent.name
707708
assert events[1].actions.escalate
709+
710+
711+
@pytest.mark.asyncio
712+
@pytest.mark.parametrize('use_pre_3_11_merge', [False, True])
713+
async def test_run_async_keeps_siblings_when_a_nested_loop_ends_itself(
714+
request: pytest.FixtureRequest,
715+
monkeypatch: pytest.MonkeyPatch,
716+
use_pre_3_11_merge: bool,
717+
):
718+
"""A sub-agent ending its own LoopAgent must not cancel sibling branches."""
719+
720+
ticks: dict[str, int] = {}
721+
722+
class _LoopingAgent(_TestingAgent):
723+
"""Escalates on its `escalate_on`-th run to end its enclosing loop."""
724+
725+
escalate_on: int = 1
726+
727+
@override
728+
async def _run_async_impl(
729+
self, ctx: InvocationContext
730+
) -> AsyncGenerator[Event, None]:
731+
await asyncio.sleep(self.delay)
732+
ticks[self.name] = ticks.get(self.name, 0) + 1
733+
escalating = ticks[self.name] >= self.escalate_on
734+
yield self.event(
735+
ctx,
736+
text=f'{self.name}#{ticks[self.name]}',
737+
actions=EventActions(escalate=True) if escalating else EventActions(),
738+
)
739+
740+
if use_pre_3_11_merge:
741+
monkeypatch.setattr(
742+
parallel_agent_module,
743+
'sys',
744+
SimpleNamespace(version_info=(3, 10)),
745+
)
746+
747+
fast_agent = _LoopingAgent(
748+
name=f'{request.function.__name__}_test_fast_agent',
749+
escalate_on=1,
750+
)
751+
slow_agent = _LoopingAgent(
752+
name=f'{request.function.__name__}_test_slow_agent',
753+
delay=0.05,
754+
escalate_on=3,
755+
)
756+
parallel_agent = ParallelAgent(
757+
name=f'{request.function.__name__}_test_parallel_agent',
758+
sub_agents=[
759+
LoopAgent(
760+
name=f'{request.function.__name__}_test_fast_loop',
761+
sub_agents=[fast_agent],
762+
max_iterations=5,
763+
),
764+
LoopAgent(
765+
name=f'{request.function.__name__}_test_slow_loop',
766+
sub_agents=[slow_agent],
767+
max_iterations=5,
768+
),
769+
],
770+
)
771+
parent_ctx = await _create_parent_invocation_context(
772+
request.function.__name__, parallel_agent
773+
)
774+
775+
events = [e async for e in parallel_agent.run_async(parent_ctx)]
776+
777+
assert [event.content.parts[0].text for event in events] == [
778+
f'{fast_agent.name}#1',
779+
f'{slow_agent.name}#1',
780+
f'{slow_agent.name}#2',
781+
f'{slow_agent.name}#3',
782+
]

0 commit comments

Comments
 (0)