Skip to content

Commit 1abb894

Browse files
Fix ADK single-turn agent-as-tool hanging on Conductor
google-adk materialises a tool wrapper into the parent's `tools` for every sub-agent declared `mode="single_turn"` or `mode="task"`, and leaves that sub-agent in `sub_agents` as well. ADK reconciles the duplication per request, when it assembles the model call; the server compiles from the serialized snapshot, where no such step exists, so both survive. The compiler then sees a coordinator with tools AND sub-agents and offers the model two routes per sub-agent with incompatible semantics: the bare sub-agent name, routed to FORK_JOIN_DYNAMIC as a SIMPLE task no worker is registered for (SCHEDULED forever, JOIN never completes), or a transfer control signal that ends the loop and hands off permanently to exactly one specialist. The workflow either hangs with no error or returns a partial answer. Two changes to the serializer: - Emit ADK agent-tools under the public `AgentTool` shape. Detection is an isinstance check against the exported base class, so it covers the private subclasses ADK materialises for single_turn/task without naming them. Serialization routes through the enclosing `_serialize` so the shared `seen` set terminates ADK's `parent_agent` back-reference. - Drop sub-agents already reachable as an agent-tool from `sub_agents`, keyed on object identity since ADK wraps the very same instance. Adds an e2e regression test, guarded by importorskip on google-adk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 61dc179 commit 1abb894

2 files changed

Lines changed: 447 additions & 0 deletions

File tree

Lines changed: 382 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,382 @@
1+
"""Regression e2e for ADK's ``mode="single_turn"`` agent-as-tool pattern.
2+
3+
An ADK "agent-as-tool" coordinator (a root ``Agent`` whose ``sub_agents`` each
4+
declare ``mode="single_turn"``) hangs permanently on Conductor: status stays
5+
``RUNNING``, no error, no timeout, no answer. The identical agent runs
6+
correctly under local ``adk web`` / ``adk run``.
7+
8+
Why it breaks
9+
-------------
10+
google-adk (>= 2.1.0) itself materialises a ``_SingleTurnAgentTool`` into the
11+
*parent's* ``tools`` list for every single-turn sub-agent, while still listing
12+
that agent under ``sub_agents``. ADK reconciles the duplication at request time
13+
(``_get_transfer_targets()`` excludes single-turn agents from transfer); the
14+
server compiles from the serialised snapshot instead, so both survive. It
15+
therefore sees a coordinator with tools AND sub-agents, dispatches to the
16+
hybrid compiler, and offers the LLM *two* tools per sub-agent:
17+
18+
1. the bare sub-agent name — routed to ``FORK_JOIN_DYNAMIC``, which forks a
19+
SIMPLE task typed with the sub-agent's own name. No worker is ever
20+
registered for it (the SDK only registers the leaf ``@tool`` functions
21+
nested inside each sub-agent), so the task sits SCHEDULED and the JOIN
22+
stays IN_PROGRESS forever; and
23+
2. ``<coordinator>_transfer_to_<sub>`` — a compiler-owned control signal that
24+
terminates the loop and hands off permanently to exactly one sub-agent.
25+
26+
Both are wrong, which is what makes this deterministic to test even though the
27+
LLM's choice between them is a coin flip:
28+
29+
* path 1 → never completes → caught by the status assertion
30+
* path 2 → completes with a *partial* answer, because a permanent transfer can
31+
only ever surface one specialist (and that specialist is handed the verbatim
32+
user prompt) → caught by the content assertion
33+
34+
A correct implementation offers exactly one callable path per single-turn
35+
sub-agent, wired to a real executor, and lets the coordinator call both and
36+
combine the results — which is precisely what its instruction asks for.
37+
38+
Run::
39+
40+
pytest e2e/test_adk_single_turn_agent_tool.py -v -s
41+
42+
Requirements:
43+
- Conductor server running (CONDUCTOR_SERVER_URL, default
44+
http://localhost:8080/api)
45+
- CONDUCTOR_AGENT_LLM_MODEL provider key configured *on the server*
46+
- google-adk installed (test skips otherwise)
47+
"""
48+
49+
import uuid
50+
from typing import Any, Dict, Iterator, List
51+
52+
import pytest
53+
54+
pytestmark = [
55+
pytest.mark.e2e,
56+
]
57+
58+
adk_agents = pytest.importorskip(
59+
"google.adk.agents", reason="google-adk not installed"
60+
)
61+
Agent = adk_agents.Agent
62+
63+
TIMEOUT = 300 # 5 min per run — CI runners are slower
64+
RUNS = 3 # the buggy tool choice is nondeterministic — sample it
65+
66+
# Unique per session so repeated local runs never collide with a stale
67+
# registered definition of the same agent name.
68+
SUFFIX = uuid.uuid4().hex[:8]
69+
WEATHER_AGENT = f"e2e_adk_st_weather_{SUFFIX}"
70+
TIME_AGENT = f"e2e_adk_st_time_{SUFFIX}"
71+
COORDINATOR = f"e2e_adk_st_coordinator_{SUFFIX}"
72+
73+
# Deterministic tool payloads. The LLM is live (no mocks, per suite
74+
# convention), but the tools are not — these sentinels let the content
75+
# assertion name exactly which specialist did or did not contribute.
76+
TEMP_SENTINEL = "11.5"
77+
TIME_SENTINEL = "14:37"
78+
79+
PROMPT = "What is the time and weather in Seattle?"
80+
81+
82+
# ===================================================================
83+
# The reported agent shape
84+
# ===================================================================
85+
86+
87+
def get_weather(city: str) -> str:
88+
"""Get the current temperature, wind speed, and humidity for a city."""
89+
return f"{city}: {TEMP_SENTINEL}°C, wind 8 km/h, humidity 72%"
90+
91+
92+
def get_current_time_by_city(city_name: str) -> str:
93+
"""Finds the current local time for a given city name."""
94+
return f"The current local time in {city_name} is {TIME_SENTINEL}."
95+
96+
97+
def _build_coordinator(model: str) -> Any:
98+
"""Coordinator with two ``mode="single_turn"`` sub-agents."""
99+
weather_specialist = Agent(
100+
name=WEATHER_AGENT,
101+
model=model,
102+
description="Handles questions about current weather conditions in a city.",
103+
instruction=(
104+
"You answer questions about the weather in a city using your "
105+
"get_weather tool. Only handle weather questions."
106+
),
107+
tools=[get_weather],
108+
mode="single_turn",
109+
)
110+
111+
time_specialist = Agent(
112+
name=TIME_AGENT,
113+
model=model,
114+
description="Handles questions about the current local time in a city.",
115+
instruction=(
116+
"You answer questions about the current local time in a city using "
117+
"your get_current_time_by_city tool. Only handle time questions."
118+
),
119+
tools=[get_current_time_by_city],
120+
mode="single_turn",
121+
)
122+
123+
return Agent(
124+
name=COORDINATOR,
125+
model=model,
126+
description=(
127+
"Coordinates weather and time questions by calling specialist "
128+
"sub-agents as tools."
129+
),
130+
instruction=(
131+
f"You are a coordinator. Call '{WEATHER_AGENT}' for weather questions "
132+
f"and '{TIME_AGENT}' for time questions. If a request needs both, call "
133+
"both, then combine their results into one final answer yourself."
134+
),
135+
sub_agents=[weather_specialist, time_specialist],
136+
)
137+
138+
139+
# ===================================================================
140+
# Helpers
141+
# ===================================================================
142+
143+
144+
def _walk_tasks(tasks: Any) -> Iterator[Dict[str, Any]]:
145+
"""Yield every task in a workflow def, descending into all nesting."""
146+
if not tasks:
147+
return
148+
for task in tasks:
149+
if not isinstance(task, dict):
150+
continue
151+
yield task
152+
yield from _walk_tasks(task.get("loopOver"))
153+
yield from _walk_tasks(task.get("defaultCase"))
154+
for branch in task.get("forkTasks") or []:
155+
yield from _walk_tasks(branch)
156+
for case_tasks in (task.get("decisionCases") or {}).values():
157+
yield from _walk_tasks(case_tasks)
158+
sub = (task.get("subWorkflowParam") or {}).get("workflowDefinition")
159+
if isinstance(sub, dict):
160+
yield from _walk_tasks(sub.get("tasks"))
161+
162+
163+
def _coordinator_tool_names(workflow_def: Dict[str, Any]) -> List[str]:
164+
"""Every tool name offered to the coordinator's own LLM task.
165+
166+
Scoped by task-reference prefix so a sub-agent's inlined SUB_WORKFLOW
167+
definition — which legitimately offers ``get_weather`` and friends —
168+
cannot contaminate the coordinator's tool list.
169+
"""
170+
names: List[str] = []
171+
for task in _walk_tasks(workflow_def.get("tasks")):
172+
if task.get("type") != "LLM_CHAT_COMPLETE":
173+
continue
174+
if not task.get("taskReferenceName", "").startswith(COORDINATOR):
175+
continue
176+
for spec in task.get("inputParameters", {}).get("tools") or []:
177+
if not isinstance(spec, dict):
178+
continue
179+
fn = spec.get("function")
180+
name = fn.get("name") if isinstance(fn, dict) else spec.get("name")
181+
if name:
182+
names.append(str(name))
183+
return names
184+
185+
186+
def _run_diagnostic(result) -> str:
187+
"""Build a diagnostic string from a run result for error messages."""
188+
parts = [f"status={result.status}", f"execution_id={result.execution_id}"]
189+
output = result.output
190+
if isinstance(output, dict):
191+
parts.append(f"output_keys={list(output.keys())}")
192+
if "finishReason" in output:
193+
parts.append(f"finishReason={output['finishReason']}")
194+
if getattr(result, "tool_calls", None):
195+
parts.append(
196+
f"tool_calls={[tc.get('name', '') for tc in result.tool_calls]}"
197+
)
198+
return " | ".join(parts)
199+
200+
201+
def _output_text(result) -> str:
202+
"""Flatten a run result's output to searchable text."""
203+
output = result.output
204+
if output is None:
205+
return ""
206+
if isinstance(output, dict):
207+
return str(output)
208+
return str(output)
209+
210+
211+
# ===================================================================
212+
# Fixtures
213+
# ===================================================================
214+
215+
216+
@pytest.fixture(scope="module")
217+
def compiled_plan(runtime, model):
218+
"""Compile the coordinator without executing it.
219+
220+
``plan()`` round-trips the serialised ADK config through the server's
221+
compiler and returns ``{workflowDef, requiredWorkers}`` — no workflow is
222+
started, no LLM is called, so this half of the regression is fully
223+
deterministic.
224+
"""
225+
return runtime.plan(_build_coordinator(model))
226+
227+
228+
@pytest.fixture(scope="module")
229+
def single_turn_runs(runtime, model):
230+
"""Execute the coordinator RUNS times and collect every result."""
231+
coordinator = _build_coordinator(model)
232+
results = []
233+
for i in range(RUNS):
234+
result = runtime.run(coordinator, PROMPT, timeout=TIMEOUT)
235+
print(
236+
f" Run {i + 1}/{RUNS}: status={result.status} "
237+
f"wf={result.execution_id}"
238+
)
239+
results.append(result)
240+
return results
241+
242+
243+
# ===================================================================
244+
# Tests
245+
# ===================================================================
246+
247+
248+
@pytest.mark.timeout(1800) # 30 min — three live multi-agent runs
249+
class TestAdkSingleTurnAgentTool:
250+
def test_one_callable_path_per_single_turn_subagent(self, compiled_plan):
251+
"""Each single-turn sub-agent is reachable exactly one way.
252+
253+
The deterministic half of the regression — no dependency on what the
254+
LLM chooses. Pre-fix the coordinator is offered both the bare
255+
sub-agent name and ``<coordinator>_transfer_to_<sub>``, i.e. two tools
256+
with incompatible semantics for one capability, only one of which has
257+
an executor at all.
258+
"""
259+
workflow_def = compiled_plan.get("workflowDef")
260+
assert workflow_def, (
261+
f"plan() returned no workflowDef. Keys: {list(compiled_plan.keys())}"
262+
)
263+
264+
tool_names = _coordinator_tool_names(workflow_def)
265+
print(f" coordinator tools: {tool_names}")
266+
print(f" requiredWorkers: {compiled_plan.get('requiredWorkers')}")
267+
assert tool_names, (
268+
"coordinator LLM task offered no tools — the compiled shape is not "
269+
"what this test assumes; inspect workflowDef before trusting the "
270+
"assertions below."
271+
)
272+
273+
for sub_agent in (WEATHER_AGENT, TIME_AGENT):
274+
paths = [n for n in tool_names if sub_agent.lower() in n.lower()]
275+
assert len(paths) == 1, (
276+
f"'{sub_agent}' is reachable via {len(paths)} tools {paths}; a "
277+
f"single-turn sub-agent must have exactly one callable path. "
278+
f"Two means the bare-name FORK_JOIN_DYNAMIC route and the "
279+
f"transfer control signal are both live. "
280+
f"All coordinator tools: {tool_names}"
281+
)
282+
283+
def test_server_requires_no_worker_the_sdk_cannot_supply(
284+
self, compiled_plan, model
285+
):
286+
"""Every ``requiredWorker`` must be one the SDK can actually register.
287+
288+
The compile response tells the client which workers to stand up. For
289+
this agent the server asks for the two single-turn sub-agent names on
290+
top of the two leaf ``@tool`` functions — but the SDK only ever
291+
extracts workers for the leaf callables, so two of the four are never
292+
supplied. Neither side reports the mismatch; the forked task simply
293+
sits SCHEDULED forever.
294+
295+
Deliberately fix-agnostic: it does not care *how* a single-turn
296+
sub-agent is executed (SUB_WORKFLOW, a real worker, anything else),
297+
only that the server never demands a worker the client cannot give it.
298+
"""
299+
from conductor.ai.agents.frameworks.serializer import serialize_agent
300+
301+
# An empty requiredWorkers is a legitimate outcome, not a red flag: once the
302+
# sub-agents compile to agent_tool they are dispatched as SUB_WORKFLOWs built
303+
# at runtime inside the fork, so neither collectSimpleTaskNames() nor the
304+
# top-level worker-typed tool scan contributes anything. The leaf @tool
305+
# workers are still registered — the SDK derives those from its own
306+
# serialization, not from this list. What matters is only the direction
307+
# below: nothing may be *required* that cannot be *supplied*.
308+
required = set(compiled_plan.get("requiredWorkers") or [])
309+
310+
_, workers = serialize_agent(_build_coordinator(model))
311+
available = {w.name for w in workers}
312+
313+
print(f" server requires : {sorted(required)}")
314+
print(f" SDK can supply : {sorted(available)}")
315+
316+
unsatisfiable = required - available
317+
assert not unsatisfiable, (
318+
f"server requires {len(required)} workers but the SDK can only "
319+
f"supply {len(available)}; nothing can ever execute "
320+
f"{sorted(unsatisfiable)}. Tasks of these types stay SCHEDULED and "
321+
f"their JOIN stays IN_PROGRESS forever. "
322+
f"required={sorted(required)} available={sorted(available)}"
323+
)
324+
325+
@pytest.mark.parametrize("run_index", range(RUNS))
326+
def test_single_turn_coordinator_completes(self, single_turn_runs, run_index):
327+
"""The workflow must terminate. Pre-fix it hangs RUNNING forever.
328+
329+
When the LLM calls the bare sub-agent name, the fork schedules a SIMPLE
330+
task typed with that name, no worker exists for it, and the JOIN waits
331+
on it indefinitely — no error, no timeout, no answer.
332+
"""
333+
result = single_turn_runs[run_index]
334+
diag = _run_diagnostic(result)
335+
print(f" {diag}")
336+
337+
assert result.execution_id, f"[run {run_index + 1}] no execution_id. {diag}"
338+
assert result.status == "COMPLETED", (
339+
f"[run {run_index + 1}] expected COMPLETED, got '{result.status}' "
340+
f"after {TIMEOUT}s. A status of RUNNING here is the "
341+
f"FORK_JOIN_DYNAMIC deadlock — check whether the "
342+
f"forked SIMPLE task named after a sub-agent is still SCHEDULED. "
343+
f"{diag}"
344+
)
345+
346+
@pytest.mark.parametrize("run_index", range(RUNS))
347+
def test_single_turn_coordinator_combines_both_specialists(
348+
self, single_turn_runs, run_index
349+
):
350+
"""Both specialists must contribute — that is what single_turn means.
351+
352+
``mode="single_turn"`` is call-and-return: the coordinator calls a
353+
sub-agent, gets its result, keeps looping, and composes the answer.
354+
The transfer path implements the opposite (permanent handoff), so it
355+
can only ever surface one specialist's output — which is why runs that
356+
*do* complete pre-fix still answer only half the question.
357+
"""
358+
result = single_turn_runs[run_index]
359+
if result.status != "COMPLETED":
360+
pytest.skip(
361+
f"run {run_index + 1} did not complete "
362+
f"({result.status}) — see the completion test"
363+
)
364+
365+
output = _output_text(result)
366+
print(f" wf={result.execution_id} output={output[:300]}")
367+
368+
missing = [
369+
label
370+
for label, sentinel in (
371+
("weather", TEMP_SENTINEL),
372+
("time", TIME_SENTINEL),
373+
)
374+
if sentinel not in output
375+
]
376+
assert not missing, (
377+
f"[run {run_index + 1}] answer is missing the "
378+
f"{' and '.join(missing)} specialist's result. The coordinator was "
379+
f"asked to call both and combine them; a permanent transfer to one "
380+
f"sub-agent cannot do that. "
381+
f"{_run_diagnostic(result)} | output={output!r}"
382+
)

0 commit comments

Comments
 (0)