google / google/adk-python

convert_a2a_message_to_event and convert_a2a_artifact_update_to_event discard the long_running_tool_ids they recover, while the two sibling converters in the same file pass them through

Ouverte
#6,988 2 commentaires 0 réactions 1 personne assignée Réclamée par @llalitkumarrr Voir sur GitHub
a2a
Langage dominant
Python
Étoiles
21.5k
Forks
4k
Merge moyen
1 j 14 h
PR mergées (30 j)
37

Description

### Summary

`to_adk_event.py` has four inbound converters. All four call
`_convert_a2a_parts_to_adk_parts`, which correctly recovers long-running
function call ids from the `adk_is_long_running` part marker. Two of them then
discard that result and never pass it to `_create_event`, so the returned
`Event` has `long_running_tool_ids=None`.

| converter | `_create_event` call | passes ids |
|---|---|---|
| `convert_a2a_task_to_event` | `to_adk_event.py:537` | yes |
| `convert_a2a_status_update_to_event` | `to_adk_event.py:642` | yes |
| `convert_a2a_message_to_event` | `to_adk_event.py:584` | **no** |
| `convert_a2a_artifact_update_to_event` | `to_adk_event.py:684` | **no** |

Both failing sites share the same shape, discarding the second return value:

```python
output_parts, _ = _convert_a2a_parts_to_adk_parts(
a2a_message.parts, part_converter
)
```

and then omit the positional `long_running_function_ids` argument that the two
working converters pass.

This is not a guess about intent. The docstring on
`convert_a2a_message_to_event` states it returns "an ADK Event object with
converted content and long-running function metadata", and the sibling
converters in the same file do exactly that.

### Verification

Repro below builds the A2A message with a real ADK converter, so the wire
marker is what a served ADK agent actually emits. No credentials, no network.

```
adk-python HEAD : d637d1b449aa4382045f71ee84d3f674403d41d5

wire part metadata : [{'adk_type': 'function_call', 'adk_is_long_running': True}]
helper recovers : {'lrt-call-0001'} <- extraction works
expected : {'lrt-call-0001'}

converter _create_event long_running_tool_ids
----------------------------------------------------------------------------------
convert_a2a_task_to_event to_adk_event:537 ['lrt-call-0001'] PASS
convert_a2a_status_update_to_event to_adk_event:642 ['lrt-call-0001'] PASS
convert_a2a_message_to_event to_adk_event:584 None FAIL
convert_a2a_artifact_update_to_event to_adk_event:684 None FAIL
```

repro script

```python
"""Repro: two of the four inbound A2A converters discard long_running_tool_ids.

`_convert_a2a_parts_to_adk_parts` correctly recovers the ids from the
`adk_is_long_running` part marker in every case. Two of its four callers then
throw the result away and never pass it to `_create_event`, so the Event comes
back with `long_running_tool_ids=None`.

The wire input is produced by a real ADK converter, not hand-built, so the
marker is exactly what a served ADK agent emits.

No credentials, no network. Run: .venv/bin/python repro/repro_lrti.py
"""

from __future__ import annotations

import subprocess
import sys
import warnings
from pathlib import Path
from unittest import mock

warnings.filterwarnings("ignore")

from google.adk.a2a import _compat
from google.adk.a2a.converters import event_converter as legacy
from google.adk.a2a.converters import to_adk_event as inbound
from google.adk.events.event import Event
from google.genai import types as genai_types

CALL_ID = "lrt-call-0001"

def sent_event() -> Event:
"""What a served ADK agent emits when it starts a long-running tool."""
return Event(
invocation_id="inv-1",
author="remote_agent",
long_running_tool_ids={CALL_ID},
content=genai_types.Content(
role="model",
parts=[
genai_types.Part(
function_call=genai_types.FunctionCall(
id=CALL_ID,
name="wait_for_human_approval",
args={"ticket": "T-42"},
)
)
],
),
)

def wire_message():
"""A real A2A Message carrying the long-running marker."""
return legacy.convert_event_to_a2a_message(sent_event(), mock.MagicMock())

def main() -> int:
repo = Path(__file__).resolve().parent.parent / "adk-python"
sha = subprocess.run(["git", "-C", str(repo), "rev-parse", "HEAD"],
capture_output=True, text=True, check=False).stdout.strip()
print(f"adk-python HEAD : {sha}")
print(f"python : {sys.version.split()[0]}\n")

message = wire_message()
markers = [dict(_compat.part_metadata(p) or {}) for p in message.parts]
print(f"wire part metadata : {markers}")

# Control: the shared helper recovers the ids from that marker correctly.
_, recovered = inbound._convert_a2a_parts_to_adk_parts(
message.parts, inbound.convert_a2a_part_to_genai_part
)
print(f"helper recovers : {recovered or None} <- extraction works")
print(f"expected : {{'{CALL_ID}'}}\n")

task = _compat.make_task(
id="task", context_id="ctx",
status=_compat.make_task_status(_compat.TS_INPUT_REQUIRED, message=message),
)
status_update = _compat.make_task_status_update_event(
task_id="task", context_id="ctx",
status=_compat.make_task_status(_compat.TS_INPUT_REQUIRED, message=message),
final=True,
)

artifact_update = _compat.TaskArtifactUpdateEvent(
task_id="task", context_id="ctx", last_chunk=True,
artifact=_compat.make_artifact(
artifact_id="art-1", parts=list(message.parts)),
)

cases = [
("convert_a2a_task_to_event", 537,
lambda: inbound.convert_a2a_task_to_event(task)),
("convert_a2a_status_update_to_event", 642,
lambda: inbound.convert_a2a_status_update_to_event(status_update)),
("convert_a2a_message_to_event", 584,
lambda: inbound.convert_a2a_message_to_event(message)),
("convert_a2a_artifact_update_to_event", 684,
lambda: inbound.convert_a2a_artifact_update_to_event(artifact_update)),
]

print(f"{'converter':38} {'_create_event':>14} {'long_running_tool_ids':>24}")
print("-" * 82)
failures = []
for name, line, call in cases:
try:
event = call()
got = event.long_running_tool_ids if event else None
except Exception as exc:
got = f"ERROR {type(exc).__name__}"
ok = got == {CALL_ID}
if not ok:
failures.append(name)
shown = sorted(got) if isinstance(got, set) else got
print(f"{name:38} {'to_adk_event:' + str(line):>14} {str(shown):>24}"
f" {'PASS' if ok else 'FAIL'}")

print("\n================ VERDICT ================")
if failures:
print("CONFIRMED. The marker is on the wire and the shared helper recovers")
print("it, but these callers drop it before building the Event:")
for name in failures:
print(f" - {name}")
print("\nBoth use `output_parts, _ = _convert_a2a_parts_to_adk_parts(...)`")
print("and omit the long_running_function_ids argument to _create_event.")
print("The two passing converters pass it positionally.")
return 1
print("No gap: every converter preserved the field.")
return 0

if __name__ == "__main__":
raise SystemExit(main())
```

The `helper recovers` line is the control. Extraction works in every case, so
the loss is at the call site, not in the marker or the parsing.

### Why this matters

Both failing converters are the defaults on `A2aRemoteAgentConfig`
(`a2a/agent/config.py:112` and `:125`), and every `RemoteA2aAgent` builds that
config when none is passed (`remote_a2a_agent.py:713`).

They are reached through `_handle_a2a_response_v2`, which
`remote_a2a_agent.py:1636` selects when the response carries the new
integration extension:

```python
if metadata and _compat.metadata_get(
metadata, _NEW_A2A_ADK_INTEGRATION_EXTENSION
):
event = await self._handle_a2a_response_v2(a2a_response, ctx)
else:
event = await self._handle_a2a_response(a2a_response, ctx)
```

So this is not every A2A call. It is the path taken once both ends speak the
new extension, and the legacy path it replaces
(`event_converter.convert_a2a_message_to_event`) populates the field correctly.
Upgrading both ends silently moves a caller from working behaviour to a
long-running call that arrives looking complete. Nothing raises and nothing
logs.

I confirmed this from the call graph and the converter defaults, not from a
live two-agent run. Flagging that rather than claiming an end-to-end user
report.

Test coverage matches the gap: the only `long_running_tool_ids` assertions in
`tests/unittests/a2a/converters/test_to_adk.py` (lines 616 and 655) both cover
`convert_a2a_task_to_event`. Neither failing converter is asserted on, and
`test_event_round_trip.py::test_round_trip_function_call_event` does not check
the field.

### Suggested fix

Keep the recovered ids and pass them, matching the two working converters:

```python
output_parts, long_running_function_ids = _convert_a2a_parts_to_adk_parts(
a2a_message.parts, part_converter
)
...
return _create_event(
output_parts,
invocation_context,
author,
_extract_event_actions(a2a_message.metadata),
long_running_function_ids,
content_role=content_role,
**metadata_fields,
)
```

Same change at `to_adk_event.py:684` for the artifact update converter. Happy
to open a PR with both call sites and tests covering all four converters.

### Context

Filing this separately at @GWeale's request on #5185: "one gap is left, the
message converter still drops long_running_tool_ids, so please file that
separately." The scope turned out to be two converters rather than one.

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.