Skip to content

Commit 455853b

Browse files
chelsealong21copybara-github
authored andcommitted
fix: scope replay sequence to the current invocation
Merge #6498 Closes #6497 PiperOrigin-RevId: 955036653
1 parent 3a9a88c commit 455853b

3 files changed

Lines changed: 231 additions & 1 deletion

File tree

src/google/adk/workflow/utils/_replay_manager.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,12 @@ def _ensure_index(self, ctx: Context) -> None:
6363
self._indexed_event_count = len(events)
6464

6565
def _build_event_index(self, events: list[Event], invocation_id: str) -> None:
66-
"""Builds index of events grouped by parent path (both direct and transitive)."""
66+
"""Builds index of events grouped by parent path (both direct and transitive).
67+
68+
The index intentionally spans every invocation in the session so multi-turn
69+
conversation context stays visible during rehydration. Consumers that need
70+
a single invocation must therefore filter by `invocation_id` themselves.
71+
"""
6772
self._events_by_parent = {}
6873
self._transitive_events_by_parent = {}
6974
fc_to_parent: dict[str, str] = {}
@@ -176,8 +181,17 @@ def _scan_sequence(
176181
"""Extract chronological child completion sequence under base_path."""
177182
base_path_builder = _NodePathBuilder.from_string(base_path)
178183
sequence: list[str] = []
184+
invocation_id = ctx._invocation_context.invocation_id
179185

180186
for event in events:
187+
# The event index spans the whole session so multi-turn context stays
188+
# visible during rehydration. The replay sequence, however, must describe
189+
# only the current invocation: a terminal event from an earlier completed
190+
# invocation would otherwise block the barrier on a node that never runs
191+
# during this resume.
192+
if invocation_id and event.invocation_id != invocation_id:
193+
continue
194+
181195
event_node_path = event.node_info.path or ""
182196
event_path_builder = _NodePathBuilder.from_string(event_node_path)
183197

tests/unittests/workflow/test_workflow_hitl.py

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2091,3 +2091,111 @@ def source_b():
20912091
# We assert that the FIRST trigger (for from_a) was processed FIRST!
20922092
assert interrupt_id_2_first == 'req_d_from_a'
20932093
assert interrupt_id_2_second == 'req_e_from_b'
2094+
2095+
2096+
class _BranchRouterNode(BaseNode):
2097+
"""Routes the first invocation and later invocations down different branches.
2098+
2099+
Stands in for the LlmAgent classification in the original report: the model
2100+
is incidental, what matters is that two invocations in one session take
2101+
different branches, so the first invocation ends on a node the second never
2102+
runs.
2103+
"""
2104+
2105+
model_config = ConfigDict(arbitrary_types_allowed=True)
2106+
seen_invocations: list[str] = Field(default_factory=list)
2107+
2108+
@override
2109+
async def _run_impl(
2110+
self, *, ctx: Context, node_input: Any
2111+
) -> AsyncGenerator[Any, None]:
2112+
self.seen_invocations.append(ctx.invocation_id)
2113+
distinct = list(dict.fromkeys(self.seen_invocations))
2114+
route = 'DONE' if len(distinct) == 1 else 'NEEDS_INPUT'
2115+
yield Event(output=node_input, route=route)
2116+
2117+
2118+
class _ClarifyNode(BaseNode):
2119+
"""Pauses for input on first execution, emits the answer once resumed."""
2120+
2121+
model_config = ConfigDict(arbitrary_types_allowed=True)
2122+
rerun_on_resume: bool = Field(default=True)
2123+
2124+
@override
2125+
async def _run_impl(
2126+
self, *, ctx: Context, node_input: Any
2127+
) -> AsyncGenerator[Any, None]:
2128+
interrupt_id = f'clarify:{ctx.run_id}'
2129+
response = ctx.resume_inputs.get(interrupt_id)
2130+
if response is None:
2131+
yield RequestInput(interrupt_id=interrupt_id, message='Which city?')
2132+
return
2133+
yield Event(output=f'resumed:{response}')
2134+
2135+
2136+
@pytest.mark.asyncio
2137+
async def test_request_input_resume_after_earlier_invocation_completed(
2138+
request: pytest.FixtureRequest,
2139+
):
2140+
"""A completed earlier invocation must not block a later HITL resume.
2141+
2142+
Regression test for #6497. The first invocation finishes on the `finish`
2143+
branch. The second invocation takes the `clarify` branch and pauses for
2144+
input. When the replay sequence was built from every event in the session,
2145+
the terminal `finish` event of the first invocation entered the sequence
2146+
and, being chronologically first, was the only key the sequence barrier
2147+
unblocked. `finish` never runs during the resume, so the pending node waited
2148+
out the barrier timeout and raised "Replay divergence detected".
2149+
"""
2150+
prepare = _TestingNode(name='prepare_intent_text', message='prepped')
2151+
router = _BranchRouterNode(name='route')
2152+
finish = _TestingNode(name='finish', message='done')
2153+
clarify = _ClarifyNode(name='clarify')
2154+
2155+
agent = Workflow(
2156+
name='test_workflow_replay_across_invocations',
2157+
edges=[
2158+
Edge(from_node=START, to_node=prepare),
2159+
Edge(from_node=prepare, to_node=router),
2160+
Edge(from_node=router, to_node=finish, route='DONE'),
2161+
Edge(from_node=router, to_node=clarify, route='NEEDS_INPUT'),
2162+
],
2163+
)
2164+
app = App(
2165+
name=request.function.__name__,
2166+
root_agent=agent,
2167+
resumability_config=ResumabilityConfig(is_resumable=True),
2168+
)
2169+
runner = testing_utils.InMemoryRunner(app=app)
2170+
2171+
# Invocation 1: runs to completion down the `finish` branch.
2172+
events1 = await runner.run_async(
2173+
testing_utils.get_user_content('who are you')
2174+
)
2175+
outputs1 = [e.output for e in events1 if e.output is not None]
2176+
assert 'done' in outputs1
2177+
2178+
# Invocation 2, same session: takes the `clarify` branch and pauses.
2179+
events2 = await runner.run_async(
2180+
testing_utils.get_user_content('weather please')
2181+
)
2182+
request_input_event = workflow_testing_utils.find_function_call_event(
2183+
events2, REQUEST_INPUT_FUNCTION_CALL_NAME
2184+
)
2185+
assert request_input_event is not None
2186+
interrupt_id = get_request_input_interrupt_ids(request_input_event)[0]
2187+
invocation_id = request_input_event.invocation_id
2188+
2189+
# Resuming must reach the pending node instead of timing out on `finish`.
2190+
events3 = await runner.run_async(
2191+
new_message=testing_utils.UserContent(
2192+
create_request_input_response(interrupt_id, {'result': 'Berlin'})
2193+
),
2194+
invocation_id=invocation_id,
2195+
)
2196+
2197+
outputs3 = [e.output for e in events3 if e.output is not None]
2198+
assert any(
2199+
isinstance(output, str) and output.startswith('resumed:')
2200+
for output in outputs3
2201+
), outputs3

tests/unittests/workflow/utils/test_replay_manager.py

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
"""Tests for ReplayManager utility."""
1616

17+
import asyncio
1718
from unittest.mock import MagicMock
1819

1920
from google.adk.events.event import Event
@@ -185,3 +186,110 @@ def test_scan_workflow_events_recovers_children_from_transitive_descendant_event
185186
recovered, _ = mgr.scan_workflow_events(ctx)
186187

187188
assert "child_a@1" in recovered
189+
190+
191+
def test_scan_workflow_events_sequence_excludes_prior_invocation_events():
192+
"""Replay sequence covers only the current invocation.
193+
194+
A session may hold a completed earlier invocation followed by a second
195+
invocation that pauses for human input. Terminal events from the earlier
196+
invocation must not enter the replay sequence, otherwise the sequence
197+
barrier blocks on a node that never runs during the resume.
198+
"""
199+
mgr = ReplayManager()
200+
# Completed earlier invocation in the same session.
201+
prior = Event(
202+
author="node",
203+
node_info=NodeInfo(path="wf@1/finish@1", run_id="1"),
204+
invocation_id="inv-1",
205+
output="prior_out",
206+
)
207+
# Current invocation, ending on an unresolved RequestInput interrupt.
208+
current_first = Event(
209+
author="node",
210+
node_info=NodeInfo(path="wf@1/alpha@1", run_id="1"),
211+
invocation_id="inv-2",
212+
output="alpha_out",
213+
)
214+
current_pending = Event(
215+
author="node",
216+
node_info=NodeInfo(path="wf@1/beta@1", run_id="1"),
217+
invocation_id="inv-2",
218+
long_running_tool_ids=["clarify:1"],
219+
)
220+
221+
ctx = MagicMock()
222+
ctx._invocation_context = MagicMock()
223+
ctx._invocation_context.invocation_id = "inv-2"
224+
ctx._invocation_context.session = MagicMock()
225+
ctx._invocation_context.session.events = [
226+
prior,
227+
current_first,
228+
current_pending,
229+
]
230+
ctx.node_path = "wf@1"
231+
232+
recovered, sequence = mgr.scan_workflow_events(ctx)
233+
234+
assert sequence == ["alpha@1", "beta@1"]
235+
# Sequence and recovered state must agree; disagreement was the defect.
236+
assert "finish@1" not in recovered
237+
# The fix belongs in _scan_sequence, NOT in the event index: the index
238+
# deliberately spans the whole session so multi-turn context stays visible
239+
# during rehydration. Filtering there instead would pass the assertions
240+
# above while silently breaking cross-turn context.
241+
assert prior in mgr._transitive_events_by_parent["wf@1"]
242+
243+
244+
def test_prepare_parent_sequence_barrier_excludes_prior_invocation_events():
245+
"""Dynamic-node sequence barriers are also scoped to the current invocation."""
246+
mgr = ReplayManager()
247+
prior = Event(
248+
author="node",
249+
node_info=NodeInfo(path="wf@1/finish@1", run_id="1"),
250+
invocation_id="inv-1",
251+
output="prior_out",
252+
)
253+
current = Event(
254+
author="node",
255+
node_info=NodeInfo(path="wf@1/alpha@1", run_id="1"),
256+
invocation_id="inv-2",
257+
output="alpha_out",
258+
)
259+
260+
ctx = MagicMock()
261+
ctx._invocation_context = MagicMock()
262+
ctx._invocation_context.invocation_id = "inv-2"
263+
ctx._invocation_context.session = MagicMock()
264+
ctx._invocation_context.session.events = [prior, current]
265+
ctx.node_path = "wf@1"
266+
267+
barrier = mgr.prepare_parent_sequence_barrier(ctx, "wf@1")
268+
269+
assert barrier.sequence == ["alpha@1"]
270+
assert prior in mgr._events_by_parent["wf@1"]
271+
272+
273+
@pytest.mark.asyncio
274+
async def test_scan_workflow_events_sequence_empty_when_all_events_are_prior():
275+
"""A session holding only prior-invocation events yields a non-blocking barrier."""
276+
mgr = ReplayManager()
277+
prior = Event(
278+
author="node",
279+
node_info=NodeInfo(path="wf@1/finish@1", run_id="1"),
280+
invocation_id="inv-1",
281+
output="prior_out",
282+
)
283+
284+
ctx = MagicMock()
285+
ctx._invocation_context = MagicMock()
286+
ctx._invocation_context.invocation_id = "inv-2"
287+
ctx._invocation_context.session = MagicMock()
288+
ctx._invocation_context.session.events = [prior]
289+
ctx.node_path = "wf@1"
290+
291+
_, sequence = mgr.scan_workflow_events(ctx)
292+
293+
assert sequence == []
294+
# An empty sequence must fast-forward rather than deadlock.
295+
await asyncio.wait_for(mgr.sequence_barrier.wait("anything"), timeout=1)

0 commit comments

Comments
 (0)