Skip to content

fix: reject terminal tasks before persisting push config (V2) - #1178

Open
ez-lbz wants to merge 2 commits into
a2aproject:mainfrom
ez-lbz:fix/v2-push-config-terminal-check
Open

fix: reject terminal tasks before persisting push config (V2)#1178
ez-lbz wants to merge 2 commits into
a2aproject:mainfrom
ez-lbz:fix/v2-push-config-terminal-check

Conversation

@ez-lbz

@ez-lbz ez-lbz commented Aug 10, 2026

Copy link
Copy Markdown

What changed

1. Check terminal task state before persisting the push config (V2 handler)

Problem: In DefaultRequestHandlerV2._setup_active_task() (src/a2a/server/request_handlers/default_request_handler_v2.py), the push-notification config was written to the store via set_info() before the terminal-state check ran inside ActiveTask.start(). As a result, sending a message to an already-completed/canceled/failed/rejected task with a task_push_notification_config in the request still persisted the config for that terminal task. V1's handler checks terminal state before any persistence.

Fix (src/a2a/server/request_handlers/default_request_handler_v2.py):

  • When message.task_id is provided and the task exists, the terminal-state check now runs immediately after loading the task and before the request-context build / set_info() call, matching V1's ordering (default_request_handler.py _setup_message_execution).
  • Raises the same InvalidParamsError("Task {id} is in terminal state: {state}") that ActiveTask.start() raised before, so the error contract is unchanged; the start() check remains as a second line of defense.

Testing

  • ./.venv/Scripts/python -m pytest tests/server/request_handlers/test_default_request_handler_v2.py -q63 passed.
  • ./.venv/Scripts/python -m pytest tests/server/request_handlers/test_default_request_handler.py tests/utils/test_task.py -q82 passed (V1 unaffected).
  • Updated the two existing on_message_send / on_message_send_stream terminal-state tests to seed the terminal task through the task store (the check now runs earlier than the patched TaskManager.get_task), and added a new parametrized regression test test_on_message_send_terminal_task_skips_push_config_persistence asserting set_info is never awaited for terminal tasks.
  • ./.venv/Scripts/python -m ruff check on modified files: clean (the pre-existing too-many-positional-arguments finding on the handler __init__ exists on main).
  • Behavior change: push configs are no longer persisted for terminal tasks; the error raised to the client is unchanged.

@ez-lbz
ez-lbz requested a review from a team as a code owner August 10, 2026 16:09
@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

🧪 Code Coverage (vs main)

⬇️ Download Full Report

Base PR Delta
src/a2a/server/request_handlers/default_request_handler_v2.py 94.17% 94.27% 🟢 +0.10%
Total 93.00% 93.00% ⚪️ 0.00%

Generated by coverage-comment.yml

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.

1 participant