Repository navigation
fix(a2a): send long-running calls once from the legacy executor - #7252
Open
chelsealong wants to merge 1 commit into
Open
chelsealong wants to merge 1 commit into
chelsealong wants to merge 1 commit into
Conversation
A2aAgentExecutor._handle_request streams every ADK event as a masked WORKING status update, then republishes the last event's content again in the terminal event. For a require_confirmation tool call, that means the adk_request_confirmation function call reaches RemoteA2aAgent (and any other streaming caller) twice: once as a thought=True preview and once as the real INPUT_REQUIRED/AUTH_REQUIRED pause, with the same call id both times. Hold long-running parts back from the intermediate stream and attach them only to the terminal status event, mirroring how the new (v2) executor's LongRunningFunctions already handles this. This addresses the adk_request_confirmation half of google#7247. The companion issue there (a completed text answer also being duplicated between the last streamed update and the final artifact) is a separate, broader change to the non-long-running completion path and is left for a follow-up.
3 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
adk_request_confirmationpause twice #7247This is a partial fix for #7247 — see "Additional context" below for what
is and isn't covered.
Problem:
A2aAgentExecutor._handle_request(the legacy, default streaming path usedby
to_a2a()+RemoteA2aAgenton a2a-sdk 1.x) streams every ADK event asa
TaskStatusUpdateEventwith its state masked toWORKING(
TaskResultAggregator.process_event), then afterwards republishes thesame content again in the terminal event built from whatever was
captured as
task_status_message.For a tool declared with
require_confirmation=True, theadk_request_confirmationfunction call is therefore delivered twice withthe same call id: once inline as a
WORKINGupdate (surfaced byRemoteA2aAgentas athought=Truepreview), and again as the realINPUT_REQUIRED/AUTH_REQUIREDterminal event. Both copies get yielded byRunner.run_asyncand persisted in the caller's session.Solution:
Hold long-running call parts back from the intermediate stream — instead of
letting them go out masked as
WORKING— and attach them only to theterminal status event, exactly once. This mirrors what the newer (v2)
executor already does via
LongRunningFunctions.process_event, so thechange reuses the same detection helpers (
is_long_running_part/is_euc_part, extracted from_create_status_update_eventso both pathsshare one implementation) instead of adding new logic.
Testing Plan
Unit Tests:
Added
test_handle_request_sends_long_running_call_oncetotests/unittests/a2a/executor/test_a2a_agent_executor.py: a realRunnerLlmAgentwith arequire_confirmation=TrueFunctionTool, run throughthe legacy
A2aAgentExecutor, asserting theadk_request_confirmationcall appears in the enqueued events exactly once and the terminal event's
state is
INPUT_REQUIRED.Confirmed it fails without the fix (
git checkout HEAD~1 -- <source files>,keeping the new test):
And passes with the fix restored:
Full
a2asuite:Full unit suite (
pytest tests/unittests -n auto): 15859 passed, 82skipped, 26 xfailed, 2 xpassed, 2 failed. The 2 failures
(
test_interactions_utils.py::...test_unrecognized_delta_logs_raw_warning_and_dropsand
test_litellm.py::...test_function_declaration_to_tool_param_response_schema_exceeds_max_length_budget)are pre-existing/order-dependent — reproduced on
git stash(pre-changetree) and unrelated to
a2a; thelitellmone passes cleanly when run inisolation.
pre-commit run(ruff, isort, pyink, addlicense, ADK compliance checks,codespell) passes on the changed files.
Manual End-to-End (E2E) Tests:
Not run — would need the three-way
to_a2a()server +RemoteA2aAgentsetup from the issue's own repro script, which the unit test above
exercises equivalently through the real
Runner/LlmAgent/executor stackwithout the network roundtrip.
Checklist
Additional context
Issue #7247 reports two distinct duplications on the legacy executor's
streaming path:
WORKINGupdate, once as the final artifact).
require_confirmationtool call'sadk_request_confirmationrequestis delivered twice with the same id (once masked
WORKING, once as thereal
INPUT_REQUIRED/AUTH_REQUIREDterminal event).This PR fixes (2), following the issue's own suggested direction for that
part verbatim ("send the call once, in the terminal
INPUT_REQUIRED/AUTH_REQUIREDstatus; keep it out of intermediateupdates, as
LongRunningFunctions.process_eventdoes in v2"). Long-runningpauses are the more consequential half of the bug — a duplicated call with
a stable id is more likely to confuse a caller resuming the flow than a
duplicated final answer.
(1) is left open. The issue's own writeup frames it as a choice between
several alternatives ("either close with a
COMPLETEDstatus and norepeated parts, or stream agent content as artifact updates, the way v2
does"), which changes the completion contract for every legacy-executor
run, not just the long-running-call path, and touches behavior several
existing tests pin down explicitly
(
test_handle_request_with_working_state_publishes_artifact_and_completedand friends). That's a bigger, more design-sensitive change that deserves
its own PR and maintainer input on which of the alternatives to take,
rather than being bundled into this scoped fix.
This change was written with AI assistance (Claude Code), with the
diagnosis and fix verified by running the repo's real unit tests
(regression test added and shown to fail without the fix / pass with it)
and
pre-commitlocally, as described above.