Repository navigation
Conversation
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/request_handlers/default_request_handler_v2.py | 88.19% | 89.15% | 🟢 +0.96% |
| Total | 93.49% | 93.52% | 🟢 +0.03% |
Generated by coverage-comment.yml
1AbdullahButt
left a comment
There was a problem hiding this comment.
This fixes the #1409 repro, but the check is wider than "replayed snapshot". It also swallows a fresh interrupted state that the executor reports as its first event.
The issue: the first interrupted event is skipped even when it's new. The subscription uses replace_status_update_with_task=True (default_request_handler_v2.py:490), so a TaskStatusUpdateEvent from updater.requires_auth() or updater.requires_input() arrives as a Task. You can't tell it apart from a re-enqueued context.current_task. And is_first_event is true on any request, not only on continuations. Two cases change:
- Out-of-band auth.
AgentExecutor's docstring (agent_executor.py:42-48) says that for out-of-band auth the agent publishesTASK_STATE_AUTH_REQUIREDand "should not return fromexecute()" until auth finishes. If that is the executor's first event, as with an executor that doesn't enqueue aTaskfirst (the handler creates the task viacreate_task_if_missing=True), the event is skipped. The blocking call then waits for the whole auth round-trip, or forever if auth never happens, instead of returningAUTH_REQUIRED. The spec says a blocking send waits until the task reaches a terminal or interrupted state (A2A spec §3.2.2, Execution Mode). - Input re-validation on a continuation. The user answers "Friday", and the executor immediately asks again with
requires_input()and returns. That event is skipped. The result still comes back correct through theget_task()fallback after the producer finishes, so this case is only slower, not wrong.
Suggested fix: skip only an event that matches the inherited state. request_context.current_task is the stored task at turn start. A replayed snapshot carries the same status (including its timestamp). TaskUpdater.update_status stamps a new timestamp (task_updater.py:84-96), so a fresh status will differ:
prior_status = (
request_context.current_task.status
if request_context.current_task is not None
else None
)
...
if (
not params.configuration.return_immediately
and is_first_event
and prior_status is not None
and event.status == prior_status
):
continueTwo tests would pin this. In the first, the first turn ends INPUT_REQUIRED, and the second turn's first event is a fresh requires_input() followed by a return; the result should be INPUT_REQUIRED from the loop, not the fallback. In the second, a new task's first event is requires_auth() and the executor then waits on an asyncio.Event; the blocking call should return AUTH_REQUIRED without the event being set.
Other notes:
git merge-treeagainstmainconflicts intests/server/request_handlers/test_default_request_handler_v2.py(the branch is 5 commits behind), so it needs a rebase.- #1298 also deals with executors re-enqueuing
context.current_task(the history side, inEventConsumer._handle_initial_task). The two don't touch the same files, but they're worth reviewing together.
I didn't run the tests.
Reviewed with help from Claude Code.
On a blocking continuation, the handler now ignores the first event when it is an interrupted-state Task snapshot replayed at turn start, so a stale
INPUT_REQUIREDsnapshot no longer ends the wait.The turn's real outcome is returned normally once the executor reports it, or via the existing
get_task()fallback if the turn ends without one.Fixes #1409