Skip to content

test(server): add xfail repros for cancel leaving no terminal state - #1171

Open
technicalpickles wants to merge 1 commit into
a2aproject:mainfrom
technicalpickles:cancel-no-terminal-state
Open

test(server): add xfail repros for cancel leaving no terminal state#1171
technicalpickles wants to merge 1 commit into
a2aproject:mainfrom
technicalpickles:cancel-no-terminal-state

Conversation

@technicalpickles

Copy link
Copy Markdown

Adds two failing scenarios for #1170, both xfail(strict=True) so they sit green in CI today and flip loudly the moment someone fixes the underlying bug.

No fix here, just the repro. #1170 has the analysis and two possible fix shapes, and which one you want depends on calls that are yours to make (particularly whether the framework should close a task out when the executor didn't, which an existing TODO in this file already raises).

What they cover

Both drive DefaultRequestHandlerV2 through the real client with an executor whose cancel() is empty:

  • test_scenario_19_mid_run_cancel_reaches_a_terminal_state cancels mid-run. The task stays working forever, so a caller has no way to know it can stop polling. The assertion is deliberately weak: any terminal state passes, not specifically canceled.
  • test_scenario_19_cancel_of_parked_task_does_not_silently_succeed cancels a task parked in input-required. Cancel returns success with the task unchanged. This one asserts only that the state must not come back unchanged, so it stays agnostic on whether a parked task should be cancelable. Cancelling it or raising TaskNotCancelableError would both pass.

Why an empty cancel()

test_scenario_cancel_working_task_empty_cancel already covers the mid-run case and passes, because its executor hand-enqueues the CANCELED event, right below a # TODO: this should be done automatically by the framework ? comment. These two are that same scenario with the hand-written event removed.

InputRequiredAgent, SlowAgent and DummyAgentExecutor in this file all define cancel() as pass already, so the shape isn't unusual. It's also what a real executor looks like when its cancel path only does teardown.

Notes

Only DefaultRequestHandlerV2, no use_legacy parametrize. The legacy handler hangs on both of these rather than failing, since on_cancel_task waits in consume_all for an event the empty executor never sends. That seemed like a separate thing and not worth entangling with this.

Pure addition, 117 lines, no existing test touched and no new dependencies. ruff check and ruff format --check are clean. tests/integration goes from 427 passed / 1 xpassed to 427 passed / 2 xfailed / 1 xpassed, and the two new ones run in about 0.2s.

🤖 Generated with Claude Code

Two integration scenarios against DefaultRequestHandlerV2, both using an
executor whose cancel() is empty. That is the shape InputRequiredAgent,
SlowAgent and DummyAgentExecutor already use in this file.

test_scenario_cancel_working_task_empty_cancel passes today only because
its executor hand-enqueues a CANCELED event, next to a "TODO: this should
be done automatically by the framework ?" comment. With that removed:

  - a mid-run cancel strands the task in `working` with no terminal state
    ever written, so a caller has no way to know it can stop polling
  - cancelling a task parked in `input-required` returns success with the
    task still `input_required`

Both marked xfail(strict=True) so they flip loudly when fixed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

🧪 Code Coverage (vs main)

⬇️ Download Full Report

Base PR Delta
src/a2a/server/events/event_queue_v2.py 91.79% 91.28% 🔴 -0.51%
src/a2a/utils/telemetry.py 91.47% 90.70% 🔴 -0.78%
Total 93.00% 92.97% 🔴 -0.02%

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