Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spelling/allow.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
a2a

Check warning on line 1 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
A2A

Check warning on line 2 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
A2AFastAPI

Check warning on line 3 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
AAgent
Expand Down Expand Up @@ -28,7 +28,7 @@
AUser
autouse
backticks
base64url

Check warning on line 31 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
buf
bufbuild
cla
Expand All @@ -43,7 +43,7 @@
drivername
DSNs
dunders
ES256

Check warning on line 46 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
euo
EUR
evt
Expand All @@ -58,8 +58,8 @@
gle
GVsb
hazmat
HS256

Check warning on line 61 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
HS384

Check warning on line 62 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
ietf
importlib
initdb
Expand Down Expand Up @@ -100,10 +100,10 @@
Oneof
OpenAPI
openapiv
openapiv2

Check warning on line 103 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
opensource
otherurl
pb2

Check warning on line 106 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
podman
Podman
poolclass
Expand All @@ -126,7 +126,7 @@
respx
resub
rmi
RS256

Check warning on line 129 in .github/actions/spelling/allow.txt

View workflow job for this annotation

GitHub Actions / Check Spelling

Ignoring entry because it contains non-alpha characters (non-alpha-in-dictionary)
RUF
SECP256R1
SFIXED
Expand All @@ -150,4 +150,5 @@
typeerror
UIDs
vulnz
GC
whl
13 changes: 13 additions & 0 deletions src/a2a/server/agent_execution/active_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from __future__ import annotations

import asyncio
import contextlib
import logging
import uuid

Expand Down Expand Up @@ -577,6 +578,18 @@ async def _run_consumer(self) -> None:
async with self._lock:
self._reference_count -= 1
logger.debug('Consumer[%s]: Finishing', self._task_id)

# Cancel and await the producer before cleanup to prevent
# asyncio "Task was destroyed but it is pending!" warnings.
# Without this, the producer could still be parked on
# _request_queue.get() or inside its finally block when
# _maybe_cleanup() releases the ActiveTask reference,
# causing the still-pending task to be garbage-collected.
if self._producer_task and not self._producer_task.done():
self._producer_task.cancel()
with contextlib.suppress(asyncio.CancelledError):
await self._producer_task
Comment on lines +588 to +591

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.

high

Using contextlib.suppress(asyncio.CancelledError) when awaiting self._producer_task inside the finally block of _run_consumer is a known asyncio anti-pattern. If _run_consumer itself is cancelled while awaiting self._producer_task, the CancelledError raised will be the cancellation of _run_consumer itself, which will be silently swallowed by contextlib.suppress. This prevents the cancellation of _run_consumer from propagating correctly.

To avoid this, you can catch asyncio.CancelledError and only suppress it if the producer task itself was cancelled (i.e., self._producer_task.cancelled() is True).

Suggested change
if self._producer_task and not self._producer_task.done():
self._producer_task.cancel()
with contextlib.suppress(asyncio.CancelledError):
await self._producer_task
if self._producer_task and not self._producer_task.done():
self._producer_task.cancel()
try:
await self._producer_task
except asyncio.CancelledError:
if not self._producer_task.cancelled():
raise


await self._maybe_cleanup()

async def subscribe(
Expand Down
15 changes: 14 additions & 1 deletion src/a2a/server/tasks/base_push_notification_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,23 @@ async def _dispatch_notification(
) -> bool:
url = push_info.url
try:
headers = None
headers: dict[str, str] | None = None
if push_info.token:
headers = {'X-A2A-Notification-Token': push_info.token}

# Add Authorization header when authentication is configured.
# The A2A spec requires servers to authenticate push notifications
# when the client provides an authentication scheme in
# PushNotificationConfig.
if push_info.HasField('authentication'):
auth = push_info.authentication
scheme = auth.scheme
credentials = auth.credentials
if scheme and credentials:
if headers is None:
headers = {}
headers['Authorization'] = f'{scheme} {credentials}'
Comment on lines +97 to +100

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.

low

For consistency, prefer using conditional expressions (ternary operators) for simple assignments over if/else blocks. You can use a conditional expression to assign a default value to headers if it is None.

Suggested change
if scheme and credentials:
if headers is None:
headers = {}
headers['Authorization'] = f'{scheme} {credentials}'
if scheme and credentials:
headers = {} if headers is None else headers
headers['Authorization'] = f'{scheme} {credentials}'
References
  1. For consistency, prefer using conditional expressions (ternary operators) for simple assignments over if/else blocks.


response = await self._client.post(
url,
json=MessageToDict(to_stream_response(event)),
Expand Down
92 changes: 92 additions & 0 deletions tests/server/agent_execution/test_active_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -895,3 +895,95 @@ async def execute_mock(req, q):
assert len(events) == 0

await active_task.cancel(request_context)


@pytest.mark.timeout(5)
@pytest.mark.asyncio
async def test_producer_cancelled_on_normal_completion():
"""Verify producer is cancelled and awaited before cleanup to prevent GC warnings.

Regression test for #1121: when the consumer finishes first (normal
completion path), the producer must be cancelled and awaited before
_maybe_cleanup() releases the ActiveTask reference, otherwise asyncio
logs "Task was destroyed but it is pending!".
"""
agent_executor = Mock()
task_manager = Mock()
cleanup_called = False

def on_cleanup(_task: ActiveTask) -> None:
nonlocal cleanup_called
cleanup_called = True

active_task = ActiveTask(
agent_executor=agent_executor,
task_id='test-task-id',
task_manager=task_manager,
on_cleanup=on_cleanup,
)

# Simulate a producer that blocks waiting for a request.
# The consumer will finish first (after processing all events),
# triggering normal teardown.
execute_started = asyncio.Event()
execute_barrier = asyncio.Event()

async def execute_mock(req, q):
execute_started.set()
# Block until released — simulates producer waiting for next request
await execute_barrier.wait()

agent_executor.execute = AsyncMock(side_effect=execute_mock)
agent_executor.cancel = AsyncMock()
task_manager.get_task = AsyncMock(
return_value=Task(
id='test-task-id',
status=TaskStatus(state=TaskState.TASK_STATE_WORKING),
)
)
task_manager.save_task_event = AsyncMock()
task_manager.ensure_task_id = AsyncMock(
return_value=Task(
id='test-task-id',
status=TaskStatus(state=TaskState.TASK_STATE_WORKING),
)
)
task_manager.process = AsyncMock(side_effect=lambda x: x)

request_context = Mock(spec=RequestContext)
request_context.call_context = ServerCallContext()
request_context.context_id = 'test-context'
request_context.message = None

await active_task.enqueue_request(request_context)
await active_task.start(
call_context=ServerCallContext(), create_task_if_missing=True
)

# Wait for producer to be executing
await execute_started.wait()

# Cancel the consumer to force consumer teardown while producer is pending.
# This simulates the normal-completion race: consumer finishes first,
# triggering _maybe_cleanup while producer is still parked.
if active_task._consumer_task:
active_task._consumer_task.cancel()
try:
await active_task._consumer_task
except asyncio.CancelledError:
pass

# Release the producer so it can complete its CancelledError/finally path
execute_barrier.set()

# Wait for producer to finish
await active_task._is_finished.wait()

# Verify producer was properly completed, not left pending
assert active_task._producer_task is not None
assert active_task._producer_task.done(), (
'Producer task should be done after consumer teardown'
)
assert cleanup_called, (
'on_cleanup should be called after producer is drained'
)
111 changes: 111 additions & 0 deletions tests/server/tasks/test_push_notification_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
BasePushNotificationSender,
)
from a2a.types.a2a_pb2 import (
AuthenticationInfo,
StreamResponse,
Task,
TaskArtifactUpdateEvent,
Expand Down Expand Up @@ -228,3 +229,113 @@ async def test_send_notification_artifact_update_event(self) -> None:
json=MessageToDict(StreamResponse(artifact_update=event)),
headers=None,
)

async def test_send_notification_with_bearer_auth(self) -> None:
"""Push notification includes Authorization header for Bearer scheme."""
task_id = 'task_bearer_auth'
task_data = _create_sample_task(task_id=task_id)
config = TaskPushNotificationConfig(
id='cfg1',
url='http://notify.me/here',
authentication=AuthenticationInfo(
scheme='Bearer',
credentials='secret-token-abc',
),
)
self.mock_config_store.get_info_for_dispatch.return_value = [config]

mock_response = AsyncMock(spec=httpx.Response)
mock_response.status_code = 200
self.mock_httpx_client.post.return_value = mock_response

await self.sender.send_notification(task_id, task_data)

self.mock_httpx_client.post.assert_awaited_once_with(
config.url,
json=MessageToDict(StreamResponse(task=task_data)),
headers={'Authorization': 'Bearer secret-token-abc'},
)

async def test_send_notification_with_token_and_bearer_auth(self) -> None:
"""Both X-A2A-Notification-Token and Authorization headers are sent."""
task_id = 'task_token_and_auth'
task_data = _create_sample_task(task_id=task_id)
config = TaskPushNotificationConfig(
id='cfg1',
url='http://notify.me/here',
token='notification-token-xyz',
authentication=AuthenticationInfo(
scheme='Bearer',
credentials='secret-token-abc',
),
)
self.mock_config_store.get_info_for_dispatch.return_value = [config]

mock_response = AsyncMock(spec=httpx.Response)
mock_response.status_code = 200
self.mock_httpx_client.post.return_value = mock_response

await self.sender.send_notification(task_id, task_data)

self.mock_httpx_client.post.assert_awaited_once_with(
config.url,
json=MessageToDict(StreamResponse(task=task_data)),
headers={
'X-A2A-Notification-Token': 'notification-token-xyz',
'Authorization': 'Bearer secret-token-abc',
},
)

async def test_send_notification_auth_no_scheme_no_header(self) -> None:
"""Authentication with empty scheme should not add Authorization header."""
task_id = 'task_auth_no_scheme'
task_data = _create_sample_task(task_id=task_id)
config = TaskPushNotificationConfig(
id='cfg1',
url='http://notify.me/here',
authentication=AuthenticationInfo(
scheme='',
credentials='secret',
),
)
self.mock_config_store.get_info_for_dispatch.return_value = [config]

mock_response = AsyncMock(spec=httpx.Response)
mock_response.status_code = 200
self.mock_httpx_client.post.return_value = mock_response

await self.sender.send_notification(task_id, task_data)

self.mock_httpx_client.post.assert_awaited_once_with(
config.url,
json=MessageToDict(StreamResponse(task=task_data)),
headers=None,
)

async def test_send_notification_auth_no_credentials_no_header(
self,
) -> None:
"""Authentication with empty credentials should not add Authorization header."""
task_id = 'task_auth_no_creds'
task_data = _create_sample_task(task_id=task_id)
config = TaskPushNotificationConfig(
id='cfg1',
url='http://notify.me/here',
authentication=AuthenticationInfo(
scheme='Bearer',
credentials='',
),
)
self.mock_config_store.get_info_for_dispatch.return_value = [config]

mock_response = AsyncMock(spec=httpx.Response)
mock_response.status_code = 200
self.mock_httpx_client.post.return_value = mock_response

await self.sender.send_notification(task_id, task_data)

self.mock_httpx_client.post.assert_awaited_once_with(
config.url,
json=MessageToDict(StreamResponse(task=task_data)),
headers=None,
)
Loading