Skip to content

fix: persist early producer failure as FAILED with originating message - #1106

Merged
mykytanetipa merged 5 commits into
a2aproject:mainfrom
AXEG0:codex-fix-early-producer-failure-task-state
Jul 20, 2026
Merged

mykytanetipa merged 5 commits into
a2aproject:mainfrom
AXEG0:codex-fix-early-producer-failure-task-state

Conversation

@AXEG0

@AXEG0 AXEG0 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Fixes #1067

Summary

  • seed the originating message into V2 ActiveTask TaskManager creation so newly created fallback tasks retain request history
  • synchronously persist TASK_STATE_FAILED from _run_producer when producer setup/execution raises, so persistence does not depend on the closing event queue
  • avoid appending the seeded originating message a second time when the first task update also carries it

Tests

  • ./scripts/lint.sh
  • uv run pytest tests/server/request_handlers/test_default_request_handler_v2.py::test_on_message_send_early_producer_exception_marks_task_failed tests/server/request_handlers/test_default_request_handler_v2.py::test_on_message_send_early_producer_exception_preserves_originating_message
  • uv run pytest tests/server/request_handlers/test_default_request_handler_v2.py

@AXEG0
AXEG0 requested a review from a team as a code owner June 16, 2026 08:59

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request ensures that early producer failures are persisted directly as failed tasks and that originating messages are preserved in the task history. It also adds corresponding unit tests. Feedback includes improving Protobuf message comparison by using message_id instead of direct equality, and directly assigning the task status state instead of using CopyFrom to avoid clearing other status fields.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/a2a/server/agent_execution/active_task.py
Comment thread src/a2a/server/agent_execution/active_task.py Outdated
@github-actions

github-actions Bot commented Jun 16, 2026 •

Copy link
Copy Markdown

🧪 Code Coverage (vs main)

⬇️ Download Full Report

Base PR Delta
src/a2a/server/agent_execution/active_task.py 95.92% 95.76% 🔴 -0.17%
Total 92.97% 92.97% ⚪️ -0.00%

Generated by coverage-comment.yml

@AXEG0 AXEG0 changed the title Fix early producer failure task persistence fix: persist early producer failure as FAILED with originating message Jun 16, 2026
Comment thread src/a2a/server/agent_execution/active_task.py Outdated
@AXEG0

AXEG0 commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed the review points:

  • Terminal guard: src/a2a/server/agent_execution/active_task.py:570 now preserves existing TERMINAL_TASK_STATES; the consumer fallback at :144 is guarded too, so the queued late exception cannot overwrite a persisted terminal result.
  • Regression coverage: tests/server/request_handlers/test_default_request_handler_v2.py:1209 covers persisted completed and canceled states followed by a late producer failure. The existing early-failure/history assertion remains at :1171.
  • Gemini suggestions were already covered by 5d71f74: history uses message_id comparison (active_task.py:263) and FAILED uses direct status.state assignment (:571).

Tests: UV_CACHE_DIR=/tmp/a2a-uv-cache uv run ruff check src/a2a/server/agent_execution/active_task.py tests/server/request_handlers/test_default_request_handler_v2.py; UV_CACHE_DIR=/tmp/a2a-uv-cache uv run pytest tests/server/request_handlers/test_default_request_handler_v2.py tests/server/agent_execution/test_active_task.py — 82 passed.

@mykytanetipa
mykytanetipa merged commit 4e3d724 into a2aproject:main Jul 20, 2026
14 checks passed
msampathkumar pushed a commit that referenced this pull request Jul 22, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.1.2](v1.1.1...v1.1.2)
(2026-07-20)


### Features

* **server:** add aclose() to drain ActiveTask background tasks
([#1101](#1101))
([#1105](#1105))
([9801f46](9801f46))


### Bug Fixes

* **agent_execution:** resolve ActiveTask 'destroyed but pending'
warning during teardown
([#1122](#1122))
([d19c4d2](d19c4d2))
* persist early producer failure as FAILED with originating message
([#1106](#1106))
([4e3d724](4e3d724))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Iwaniukooo11 added a commit that referenced this pull request Oct 23, 2026
# Description

When an executor enqueues its `Task` and raises before that Task is
saved, `_run_producer` and the consumer both insert the same row through
the shared `TaskManager`. With `DatabaseTaskStore` one insert fails, and
the request either returns the `SUBMITTED` task as a success or raises
`IntegrityError` instead of the executor's error.

This moves the failure write to `EventConsumer`, as suggested on the
issue:

- `_run_producer` no longer saves the failure. It only enqueues the
exception, as before.
- The consumer records the failure when it dequeues the exception, so
FAILED is written after every event the executor enqueued before
raising.
- `_write_failed_status` creates the fallback task through
`ensure_task_id`, with the context id from `_task_manager.context_id`,
only when no task exists, which keeps the case #1106 fixed. FAILED is
stamped with the failure time, so `last_updated` is set. The write goes
through `_handle_task_modification_event` rather than
`_handle_task_event`, so it still lands when the agent answered with a
Message.

Visible differences from the old producer write:

- FAILED carries the time of the failure. The old write kept the
previous status time, or none, which left `last_updated` NULL.
- The agent's last status message moves from `status.message` into
`history`, as on any status update.
- When a resumed task fails, the follow-up user message is now recorded
in `history`. The old write dropped it.

Three tests are added. One holds the store's insert of the agent's Task
until the executor has raised, which is the window a database round trip
leaves open. It fails on `main` with `UNIQUE constraint failed:
tasks.id` and passes with this change. The other two check that a task
which fails after reporting progress is stamped with the failure time,
and that a follow-up answered with a Message still leaves the task
FAILED.

Checks on current `main` (79a6f45), Python 3.10: the full suite passes
with Postgres 18 behind the database tests (2396 passed, 92 skipped; the
4 cross-version tests timed out starting their servers in that run and
pass on their own). I did not run the MySQL tests. `ruff check`, `ruff
format --check` and `ty check` show nothing new.

- [x] Follow the [`CONTRIBUTING`
Guide](https://github.com/a2aproject/a2a-python/blob/main/CONTRIBUTING.md).
- [x] Make your Pull Request title in the
<https://www.conventionalcommits.org/> specification.
- [x] Ensure the tests and linter pass
- [x] Appropriate docs were updated (if necessary)

Fixes #1351

---------

Co-authored-by: Mateusz Iwaniuk <45974414+Iwaniukooo11@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ActiveTaskRegistry.get_or_create produces orphan SUBMITTED rows on early _run_producer exception

2 participants