Skip to content

Commit ba142cf

Browse files
AkshayShah03copybara-github
authored andcommitted
fix: stop rerunning a waiting wait_for_output node on resume
Merge #7991 On resume, a `wait_for_output` node whose earlier run ended waiting (no output, route or interrupt) is no longer run again with the input it already handled; it stays waiting until its next trigger. Agents in task or chat mode and nodes with `rerun_on_resume=True` still rerun. A custom node that waits on the user rather than on a trigger now needs `rerun_on_resume=True` to be rerun. Fixes #7905 PiperOrigin-RevId: 1015845387
1 parent c44adfd commit ba142cf

5 files changed

Lines changed: 257 additions & 3 deletions

File tree

‎.agents/skills/adk-architecture/references/interface-workflow.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,14 @@ Within a single step (`_execute_step`), when replay is enabled:
212212
`resume_inputs=recovered.resolved_responses`.
213213

214214
7. **No output, route, or interrupts in recovered events** —
215-
- If `node.wait_for_output` or `node.rerun_on_resume`: returns
215+
- If `node.wait_for_output` and the node is a `BaseAgent` (task or chat
216+
mode), or `node.rerun_on_resume` with `wait_for_output`: returns
216217
`should_run=True` with `resume_inputs=recovered.resolved_responses`.
217-
- Otherwise: returns `should_run = (current_run is not None and not current_run.is_static)`
218+
- Else if `node.rerun_on_resume`: fast-forwards (`should_run=False`) when
219+
`recovered.finished_after_resume`, otherwise returns `should_run=True`
220+
with `resume_inputs`.
221+
- Otherwise (including any other `wait_for_output` node, whose next trigger
222+
starts a new run): returns `should_run = (current_run is not None and not current_run.is_static)`
218223
(static nodes that completed with `None` output fast-forward with
219224
`should_run=False`, while dynamic nodes with no recorded outcome
220225
re-execute with `should_run=True`).

‎docs/guides/workflow/base_node/index.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ With the default `False`, a node completes as soon as its logic finishes, whethe
118118

119119
With `True`, the node completes only if it yields an output or a route. Finishing without yielding either leaves it in the `WAITING` state, and its successors are not triggered. A `WAITING` node still accepts new triggers, so a predecessor firing again re-runs it. That behavior is what you want for a node that accumulates across several triggers and only emits once it has enough, such as one keeping a running total in state and producing an output when that total crosses a threshold.
120120

121+
When an interrupted workflow resumes, a `WAITING` node in the graph is not run again with the input it already handled: it stays `WAITING` until its next trigger, which starts a new run. Agents in task or chat mode are the exception, since they wait for the user's next message rather than for a trigger, and rerun on resume. A custom node that waits on the user in the same way, rather than on another trigger, needs `rerun_on_resume=True` as well. This applies to a waiting run that recorded at least one event, such as a state update; a run that emitted nothing is still rerun today.
122+
121123
The failure mode is worth knowing before you set this. A node that is *never* triggered again stays `WAITING` to the end of the run, and the run does not wait for it. The workflow finishes straight away, having emitted nothing, and the whole downstream half of your graph is silently skipped. Nothing raises and nothing is logged about it, at any level. The framework treats this as a configuration mistake rather than a fault to report.
122124

123125
One node type ignores this flag entirely. `JoinNode` does not use it, because it synchronizes by waiting for all of its predecessors, which is a different mechanism.

‎src/google/adk/workflow/utils/_replay_interceptor.py‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from typing import Any
2222
from typing import TYPE_CHECKING
2323

24+
from ...agents.base_agent import BaseAgent
2425
from ...agents.context import Context
2526
from .._base_node import BaseNode
2627
from .._node_status import NodeStatus
@@ -172,7 +173,11 @@ def check_interception(
172173

173174
else:
174175
# Case 6: Cross-turn no events, or events contain no output, route, or interrupts.
175-
if node.wait_for_output:
176+
if node.wait_for_output and _reruns_while_waiting(node):
177+
# A waiting agent (task or chat mode) or rerun_on_resume node reruns to
178+
# take new input. Other wait_for_output nodes fall through: their next
179+
# trigger starts a new run, so rerunning this one would repeat its side
180+
# effects.
176181
should_run = True
177182
resume_inputs = recovered.resolved_responses
178183
elif node.rerun_on_resume:
@@ -235,3 +240,8 @@ def create_mock_context(
235240
mock_ctx._interrupt_ids = result.interrupts # pylint: disable=protected-access
236241

237242
return mock_ctx
243+
244+
245+
def _reruns_while_waiting(node: BaseNode) -> bool:
246+
"""Whether a wait_for_output node with no recorded outcome runs again."""
247+
return node.rerun_on_resume or isinstance(node, BaseAgent)

‎tests/unittests/workflow/test_workflow_hitl.py‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2704,3 +2704,189 @@ def book() -> dict[str, str]:
27042704

27052705
assert booked == ['trip']
27062706
assert {e.branch for e in events1 + events2 if e.author == 'booker'} == {None}
2707+
2708+
2709+
class _GatherNode(BaseNode):
2710+
"""Collects inputs in state and emits every `batch` inputs.
2711+
2712+
This is the accumulate-then-emit pattern `wait_for_output` is meant for.
2713+
"""
2714+
2715+
batch: int = 2
2716+
calls: list[Any] = Field(default_factory=list)
2717+
2718+
@override
2719+
async def _run_impl(
2720+
self, *, ctx: Context, node_input: Any
2721+
) -> AsyncGenerator[Any, None]:
2722+
self.calls.append(node_input)
2723+
seen = list(ctx.state.get('seen') or []) + [node_input]
2724+
ctx.state['seen'] = seen
2725+
if len(seen) % self.batch == 0:
2726+
yield Event(output=seen[-self.batch :])
2727+
2728+
2729+
def _gather_app(
2730+
request: pytest.FixtureRequest, agent: Workflow, resumable: bool
2731+
) -> App:
2732+
return App(
2733+
name=request.function.__name__,
2734+
root_agent=agent,
2735+
resumability_config=(
2736+
ResumabilityConfig(is_resumable=True) if resumable else None
2737+
),
2738+
)
2739+
2740+
2741+
@pytest.mark.parametrize('resumable', [False, True])
2742+
@pytest.mark.asyncio
2743+
async def test_resume_does_not_rerun_superseded_waiting_run(
2744+
request: pytest.FixtureRequest, resumable: bool
2745+
):
2746+
"""START -> A -> Gather, START -> B -> Gather, Gather -> Ask -> Done.
2747+
2748+
Gather waits after A's output (Gather@1) and emits after B's (Gather@2).
2749+
Answering Ask must not run Gather@1 again: Gather@2 already superseded it.
2750+
"""
2751+
gather = _GatherNode(name='Gather', wait_for_output=True)
2752+
ask = RequestInputNode(name='Ask')
2753+
done = InputCapturingNode(name='Done')
2754+
agent = Workflow(
2755+
name='test_agent',
2756+
edges=[
2757+
(START, _TestingNode(name='A', message='a'), gather),
2758+
(START, _TestingNode(name='B', message='b'), gather),
2759+
(gather, ask, done),
2760+
],
2761+
)
2762+
runner = testing_utils.InMemoryRunner(
2763+
app=_gather_app(request, agent, resumable)
2764+
)
2765+
2766+
events1 = await runner.run_async(testing_utils.get_user_content('go'))
2767+
[interrupt_id] = extract_event_interrupt_ids(
2768+
workflow_testing_utils.get_request_input_events(events1)[0]
2769+
)
2770+
assert sorted(gather.calls) == ['a', 'b']
2771+
2772+
await runner.run_async(
2773+
new_message=testing_utils.UserContent(
2774+
create_request_input_response(interrupt_id, {'yes': True})
2775+
),
2776+
invocation_id=events1[0].invocation_id,
2777+
)
2778+
2779+
assert sorted(gather.calls) == ['a', 'b']
2780+
assert done.received_inputs == [{'yes': True}]
2781+
assert sorted(runner.session.state['seen']) == ['a', 'b']
2782+
2783+
2784+
@pytest.mark.parametrize('resumable', [False, True])
2785+
@pytest.mark.asyncio
2786+
async def test_resume_does_not_rerun_waiting_run_before_its_next_trigger(
2787+
request: pytest.FixtureRequest, resumable: bool
2788+
):
2789+
"""START -> A -> Gather, START -> Ask -> B -> Gather, Gather -> Done.
2790+
2791+
Gather waits after A's output while Ask pauses for the user. Answering Ask
2792+
must not run Gather again with A's output: it should stay waiting until B's
2793+
output arrives, then emit both.
2794+
"""
2795+
gather = _GatherNode(name='Gather', wait_for_output=True)
2796+
ask = RequestInputNode(name='Ask')
2797+
done = InputCapturingNode(name='Done')
2798+
agent = Workflow(
2799+
name='test_agent',
2800+
edges=[
2801+
(START, _TestingNode(name='A', message='a'), gather),
2802+
(START, ask, _TestingNode(name='B', message='b'), gather),
2803+
(gather, done),
2804+
],
2805+
)
2806+
runner = testing_utils.InMemoryRunner(
2807+
app=_gather_app(request, agent, resumable)
2808+
)
2809+
2810+
events1 = await runner.run_async(testing_utils.get_user_content('go'))
2811+
[interrupt_id] = extract_event_interrupt_ids(
2812+
workflow_testing_utils.get_request_input_events(events1)[0]
2813+
)
2814+
assert gather.calls == ['a']
2815+
2816+
await runner.run_async(
2817+
new_message=testing_utils.UserContent(
2818+
create_request_input_response(interrupt_id, {'yes': True})
2819+
),
2820+
invocation_id=events1[0].invocation_id,
2821+
)
2822+
2823+
assert gather.calls == ['a', 'b']
2824+
assert done.received_inputs == [['a', 'b']]
2825+
assert runner.session.state['seen'] == ['a', 'b']
2826+
2827+
2828+
class _LapRouterNode(BaseNode):
2829+
"""Routes 'loop' until it has run `laps` times, then 'stop'."""
2830+
2831+
laps: int = 3
2832+
2833+
@override
2834+
async def _run_impl(
2835+
self, *, ctx: Context, node_input: Any
2836+
) -> AsyncGenerator[Any, None]:
2837+
lap = (ctx.state.get('lap') or 0) + 1
2838+
ctx.state['lap'] = lap
2839+
yield Event(output=lap, route='loop' if lap < self.laps else 'stop')
2840+
2841+
2842+
@pytest.mark.parametrize('resumable', [False, True])
2843+
@pytest.mark.asyncio
2844+
async def test_resume_does_not_rerun_waiting_runs_from_earlier_loop_laps(
2845+
request: pytest.FixtureRequest, resumable: bool
2846+
):
2847+
"""START -> A -> Gather, A -> B -> Gather, Gather -> Router.
2848+
2849+
Router loops back to A twice, then routes to Ask -> Done. Each lap leaves
2850+
one waiting run of Gather. Answering Ask must not rerun them, which would
2851+
feed Gather extra inputs, run the loop again and ask a second time.
2852+
"""
2853+
gather = _GatherNode(name='Gather', wait_for_output=True)
2854+
node_a = _TestingNode(name='A', message='a')
2855+
node_b = _TestingNode(name='B', message='b')
2856+
router = _LapRouterNode(name='Router')
2857+
ask = RequestInputNode(name='Ask')
2858+
done = InputCapturingNode(name='Done')
2859+
agent = Workflow(
2860+
name='test_agent',
2861+
edges=[
2862+
(START, node_a, gather),
2863+
(node_a, node_b, gather),
2864+
(gather, router),
2865+
(router, {'loop': node_a, 'stop': ask}),
2866+
(ask, done),
2867+
],
2868+
)
2869+
runner = testing_utils.InMemoryRunner(
2870+
app=_gather_app(request, agent, resumable)
2871+
)
2872+
2873+
events1 = await runner.run_async(testing_utils.get_user_content('go'))
2874+
[interrupt_id] = extract_event_interrupt_ids(
2875+
workflow_testing_utils.get_request_input_events(events1)[0]
2876+
)
2877+
assert gather.calls == ['a', 'b'] * 3
2878+
2879+
events2 = await asyncio.wait_for(
2880+
runner.run_async(
2881+
new_message=testing_utils.UserContent(
2882+
create_request_input_response(interrupt_id, {'yes': True})
2883+
),
2884+
invocation_id=events1[0].invocation_id,
2885+
),
2886+
timeout=10,
2887+
)
2888+
2889+
assert gather.calls == ['a', 'b'] * 3
2890+
assert done.received_inputs == [{'yes': True}]
2891+
assert not workflow_testing_utils.get_request_input_events(events2)
2892+
assert runner.session.state['lap'] == 3

‎tests/unittests/workflow/utils/test_replay_interceptor.py‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,57 @@ def test_cross_turn_no_outcome_static_vs_dynamic_run(
345345
assert dynamic_result.should_run
346346

347347

348+
class _AgentNode(BaseAgent):
349+
"""A minimal agent, standing in for a task- or chat-mode LlmAgent."""
350+
351+
async def _run_async_impl(self, ctx):
352+
yield # pragma: no cover
353+
354+
355+
@pytest.mark.parametrize('is_static', [True, False])
356+
def test_cross_turn_no_outcome_wait_for_output_node_like_plain_node(
357+
is_static: bool,
358+
):
359+
"""Case 6 treats a plain wait_for_output node like any other node.
360+
361+
Its next trigger starts a new run, so a static run is fast-forwarded rather
362+
than rerun with the input it already handled. A dynamic run still reruns.
363+
"""
364+
recovered = _ChildScanState(run_id='1')
365+
node = BaseNode(name='node', wait_for_output=True)
366+
run = DynamicNodeRun(
367+
state=NodeState(run_id='1'),
368+
recovered_state=recovered,
369+
is_static=is_static,
370+
)
371+
372+
result = check_interception(node=node, recovered=recovered, current_run=run)
373+
374+
assert result.should_run == (not is_static)
375+
376+
377+
@pytest.mark.parametrize(
378+
'node',
379+
[
380+
BaseNode(name='node', wait_for_output=True, rerun_on_resume=True),
381+
_AgentNode(name='agent', wait_for_output=True),
382+
],
383+
ids=['rerun_on_resume', 'agent'],
384+
)
385+
def test_cross_turn_no_outcome_waiting_agent_or_rerun_node_reruns(
386+
node: BaseNode,
387+
):
388+
"""Case 6 reruns a waiting agent or rerun_on_resume node to take new input."""
389+
recovered = _ChildScanState(run_id='1')
390+
run = DynamicNodeRun(
391+
state=NodeState(run_id='1'), recovered_state=recovered, is_static=True
392+
)
393+
394+
result = check_interception(node=node, recovered=recovered, current_run=run)
395+
396+
assert result.should_run
397+
398+
348399
# --- create_mock_context ---
349400

350401

0 commit comments

Comments
 (0)