From 7348703cc8a1b731833ec8e58e2a49fab58589e5 Mon Sep 17 00:00:00 2001 From: Patrick-SCH03 Date: Fri, 25 Sep 2026 23:02:33 +0900 Subject: [PATCH 1/2] fix(server): warn when an AgentCard is missing required fields AgentCard marks eight fields REQUIRED, but nothing checks them, so an empty card is served as `{}`. As the first, non-breaking step suggested in #1267, log a warning listing the missing fields when a card is passed to create_agent_card_routes, DefaultRequestHandler or DefaultRequestHandlerV2 (agent_card and extended_agent_card). The check runs once at setup, never raises, and skips objects that are not Protobuf messages such as test doubles. Fixes #1267 --- .../default_request_handler.py | 9 +++ .../default_request_handler_v2.py | 9 +++ src/a2a/server/routes/agent_card_routes.py | 5 ++ src/a2a/utils/proto_utils.py | 34 ++++++++++ .../test_default_request_handler.py | 62 ++++++++++++++++++ .../test_default_request_handler_v2.py | 63 +++++++++++++++++++ tests/server/routes/test_agent_card_routes.py | 58 ++++++++++++++++- tests/utils/test_proto_utils.py | 58 +++++++++++++++++ 8 files changed, 297 insertions(+), 1 deletion(-) diff --git a/src/a2a/server/request_handlers/default_request_handler.py b/src/a2a/server/request_handlers/default_request_handler.py index da5090613..209d693a7 100644 --- a/src/a2a/server/request_handlers/default_request_handler.py +++ b/src/a2a/server/request_handlers/default_request_handler.py @@ -63,6 +63,7 @@ UnsupportedOperationError, ) from a2a.utils.input_mode_validator import validate_input_modes +from a2a.utils.proto_utils import warn_on_missing_required_fields from a2a.utils.task import ( apply_history_length, validate_history_length, @@ -146,6 +147,14 @@ def __init__( # noqa: PLR0913 self._validate_input_modes = validate_input_modes self.extended_agent_card = extended_agent_card self.extended_card_modifier = extended_card_modifier + warn_on_missing_required_fields( + agent_card, 'agent_card passed to DefaultRequestHandler:' + ) + if extended_agent_card is not None: + warn_on_missing_required_fields( + extended_agent_card, + 'extended_agent_card passed to DefaultRequestHandler:', + ) self._request_context_builder = ( request_context_builder or SimpleRequestContextBuilder( diff --git a/src/a2a/server/request_handlers/default_request_handler_v2.py b/src/a2a/server/request_handlers/default_request_handler_v2.py index 79cff4fab..b135f529f 100644 --- a/src/a2a/server/request_handlers/default_request_handler_v2.py +++ b/src/a2a/server/request_handlers/default_request_handler_v2.py @@ -61,6 +61,7 @@ UnsupportedOperationError, ) from a2a.utils.input_mode_validator import validate_input_modes +from a2a.utils.proto_utils import warn_on_missing_required_fields from a2a.utils.task import ( apply_history_length, validate_history_length, @@ -151,6 +152,14 @@ def __init__( # noqa: PLR0913 ) warnings.warn(message, stacklevel=2) logger.warning(message) + warn_on_missing_required_fields( + agent_card, 'agent_card passed to DefaultRequestHandlerV2:' + ) + if extended_agent_card is not None: + warn_on_missing_required_fields( + extended_agent_card, + 'extended_agent_card passed to DefaultRequestHandlerV2:', + ) self._request_context_builder = ( request_context_builder or SimpleRequestContextBuilder( diff --git a/src/a2a/server/routes/agent_card_routes.py b/src/a2a/server/routes/agent_card_routes.py index 37c09ad1e..5cf779a3c 100644 --- a/src/a2a/server/routes/agent_card_routes.py +++ b/src/a2a/server/routes/agent_card_routes.py @@ -29,6 +29,7 @@ from a2a.server.request_handlers.response_helpers import agent_card_to_dict from a2a.types.a2a_pb2 import AgentCard from a2a.utils.constants import AGENT_CARD_WELL_KNOWN_PATH +from a2a.utils.proto_utils import warn_on_missing_required_fields def _etag_for(card_dict: dict[str, Any]) -> str: @@ -96,6 +97,10 @@ def create_agent_card_routes( 'It can be installed as part of `a2a-sdk` optional dependencies, `a2a-sdk[http-server]`.' ) + warn_on_missing_required_fields( + agent_card, 'agent_card passed to create_agent_card_routes:' + ) + async def _get_agent_card(request: Request) -> Response: """Returns the public AgentCard describing this agent's capabilities, supported transports, and skills.""" card_to_serve = agent_card diff --git a/src/a2a/utils/proto_utils.py b/src/a2a/utils/proto_utils.py index 38346b5eb..bbafa4dea 100644 --- a/src/a2a/utils/proto_utils.py +++ b/src/a2a/utils/proto_utils.py @@ -17,6 +17,8 @@ This module provides helper functions for common proto type operations. """ +import logging + from typing import TYPE_CHECKING, Any, TypedDict, cast from google.api.field_behavior_pb2 import FieldBehavior, field_behavior @@ -45,6 +47,9 @@ ) +logger = logging.getLogger(__name__) + + # Define Event type locally to avoid circular imports Event = Message | Task | TaskStatusUpdateEvent | TaskArtifactUpdateEvent @@ -316,6 +321,35 @@ def validate_proto_required_fields(msg: ProtobufMessage) -> None: ) +def warn_on_missing_required_fields(msg: ProtobufMessage, source: str) -> bool: + """Log a warning if fields marked as REQUIRED are missing on the message. + + Unlike `validate_proto_required_fields`, this never raises, so it can + surface spec violations without breaking callers that currently rely on + non-compliant messages. + + Args: + msg: The Protobuf message to check. + source: Where the message was passed in, used in the log message. + + Returns: + True if every REQUIRED field is present, False otherwise. Objects that + are not Protobuf messages (for example test doubles) are not checked. + """ + if not isinstance(msg, ProtobufMessage): + return True + errors = _validate_proto_required_fields_internal(msg) + if not errors: + return True + logger.warning( + '%s %s is missing fields marked REQUIRED by the A2A spec: %s', + source, + msg.DESCRIPTOR.name, + ', '.join(err['field'] for err in errors), + ) + return False + + def validation_errors_to_bad_request( errors: list[ValidationDetail], ) -> error_details_pb2.BadRequest: diff --git a/tests/server/request_handlers/test_default_request_handler.py b/tests/server/request_handlers/test_default_request_handler.py index eb0444eb0..925019900 100644 --- a/tests/server/request_handlers/test_default_request_handler.py +++ b/tests/server/request_handlers/test_default_request_handler.py @@ -56,6 +56,8 @@ from a2a.types.a2a_pb2 import ( AgentCapabilities, AgentCard, + AgentInterface, + AgentSkill, Artifact, CancelTaskRequest, DeleteTaskPushNotificationConfigRequest, @@ -3389,3 +3391,63 @@ async def test_on_message_send_rejects_invalid_push_url(agent_card): InvalidParamsError, match='Invalid push notification URL' ): await request_handler.on_message_send(params, context) + + +def _complete_agent_card() -> AgentCard: + """Returns an AgentCard with every field the A2A spec marks REQUIRED.""" + return AgentCard( + name='complete_agent', + description='An agent card with all required fields.', + supported_interfaces=[ + AgentInterface( + url='http://localhost:8000', + protocol_binding='JSONRPC', + protocol_version='1.0', + ) + ], + version='1.0', + capabilities=AgentCapabilities(), + default_input_modes=['text/plain'], + default_output_modes=['text/plain'], + skills=[ + AgentSkill( + id='echo', + name='Echo', + description='Echoes the input.', + tags=['test'], + ) + ], + ) + + +def test_init_warns_about_incomplete_agent_cards( + agent_card: AgentCard, + caplog: pytest.LogCaptureFixture, +) -> None: + """Cards missing REQUIRED fields are accepted, but each one is logged.""" + with caplog.at_level(logging.WARNING, logger='a2a.utils.proto_utils'): + DefaultRequestHandler( + agent_executor=MockAgentExecutor(), + task_store=InMemoryTaskStore(), + agent_card=agent_card, + extended_agent_card=AgentCard(), + ) + messages = [record.getMessage() for record in caplog.records] + assert len(messages) == 2 + assert messages[0].startswith('agent_card passed to DefaultRequestHandler:') + assert messages[1].startswith( + 'extended_agent_card passed to DefaultRequestHandler:' + ) + + +def test_init_does_not_warn_for_complete_agent_cards( + caplog: pytest.LogCaptureFixture, +) -> None: + with caplog.at_level(logging.WARNING, logger='a2a.utils.proto_utils'): + DefaultRequestHandler( + agent_executor=MockAgentExecutor(), + task_store=InMemoryTaskStore(), + agent_card=_complete_agent_card(), + extended_agent_card=_complete_agent_card(), + ) + assert caplog.records == [] diff --git a/tests/server/request_handlers/test_default_request_handler_v2.py b/tests/server/request_handlers/test_default_request_handler_v2.py index 52c5e9c4b..b2d25f95a 100644 --- a/tests/server/request_handlers/test_default_request_handler_v2.py +++ b/tests/server/request_handlers/test_default_request_handler_v2.py @@ -46,6 +46,8 @@ from a2a.types.a2a_pb2 import ( AgentCapabilities, AgentCard, + AgentInterface, + AgentSkill, Artifact, CancelTaskRequest, DeleteTaskPushNotificationConfigRequest, @@ -2528,3 +2530,64 @@ async def send(tag: str, message: Message) -> None: assert agent.seen_tags == ['request-1', 'request-2'] await handler.aclose() + + +def _complete_agent_card() -> AgentCard: + """Returns an AgentCard with every field the A2A spec marks REQUIRED.""" + return AgentCard( + name='complete_agent', + description='An agent card with all required fields.', + supported_interfaces=[ + AgentInterface( + url='http://localhost:8000', + protocol_binding='JSONRPC', + protocol_version='1.0', + ) + ], + version='1.0', + capabilities=AgentCapabilities(), + default_input_modes=['text/plain'], + default_output_modes=['text/plain'], + skills=[ + AgentSkill( + id='echo', + name='Echo', + description='Echoes the input.', + tags=['test'], + ) + ], + ) + + +def test_init_warns_about_incomplete_agent_cards( + caplog: pytest.LogCaptureFixture, +) -> None: + """Cards missing REQUIRED fields are accepted, but each one is logged.""" + with caplog.at_level(logging.WARNING, logger='a2a.utils.proto_utils'): + DefaultRequestHandlerV2( + agent_executor=MockAgentExecutor(), + task_store=InMemoryTaskStore(), + agent_card=create_default_agent_card(), + extended_agent_card=AgentCard(), + ) + messages = [record.getMessage() for record in caplog.records] + assert len(messages) == 2 + assert messages[0].startswith( + 'agent_card passed to DefaultRequestHandlerV2:' + ) + assert messages[1].startswith( + 'extended_agent_card passed to DefaultRequestHandlerV2:' + ) + + +def test_init_does_not_warn_for_complete_agent_cards( + caplog: pytest.LogCaptureFixture, +) -> None: + with caplog.at_level(logging.WARNING, logger='a2a.utils.proto_utils'): + DefaultRequestHandlerV2( + agent_executor=MockAgentExecutor(), + task_store=InMemoryTaskStore(), + agent_card=_complete_agent_card(), + extended_agent_card=_complete_agent_card(), + ) + assert caplog.records == [] diff --git a/tests/server/routes/test_agent_card_routes.py b/tests/server/routes/test_agent_card_routes.py index cde9d51f0..f3cb03ed6 100644 --- a/tests/server/routes/test_agent_card_routes.py +++ b/tests/server/routes/test_agent_card_routes.py @@ -1,11 +1,17 @@ import copy +import logging from unittest.mock import AsyncMock import pytest from a2a.server.routes.agent_card_routes import create_agent_card_routes -from a2a.types.a2a_pb2 import AgentCard +from a2a.types.a2a_pb2 import ( + AgentCapabilities, + AgentCard, + AgentInterface, + AgentSkill, +) from a2a.utils.signing import create_agent_card_signer from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import ec @@ -231,3 +237,53 @@ async def modifier(card: AgentCard) -> AgentCard: second = signed_client('b').get(url) assert first.headers['etag'] != second.headers['etag'] + + +def _complete_agent_card() -> AgentCard: + """Returns an AgentCard with every field the A2A spec marks REQUIRED.""" + return AgentCard( + name='complete_agent', + description='An agent card with all required fields.', + supported_interfaces=[ + AgentInterface( + url='http://localhost:8000', + protocol_binding='JSONRPC', + protocol_version='1.0', + ) + ], + version='1.0', + capabilities=AgentCapabilities(), + default_input_modes=['text/plain'], + default_output_modes=['text/plain'], + skills=[ + AgentSkill( + id='echo', + name='Echo', + description='Echoes the input.', + tags=['test'], + ) + ], + ) + + +def test_route_creation_warns_about_missing_required_fields( + agent_card: AgentCard, caplog: pytest.LogCaptureFixture +) -> None: + """An incomplete card is still served, but a warning is logged once.""" + with caplog.at_level(logging.WARNING, logger='a2a.utils.proto_utils'): + routes = create_agent_card_routes(agent_card=agent_card) + [record] = caplog.records + message = record.getMessage() + assert 'agent_card passed to create_agent_card_routes:' in message + assert 'name, description, supported_interfaces' in message + + client = TestClient(Starlette(routes=routes)) + assert client.get('/.well-known/agent-card.json').status_code == 200 + + +def test_route_creation_does_not_warn_for_complete_card( + caplog: pytest.LogCaptureFixture, +) -> None: + with caplog.at_level(logging.WARNING, logger='a2a.utils.proto_utils'): + create_agent_card_routes(agent_card=_complete_agent_card()) + assert caplog.records == [] diff --git a/tests/utils/test_proto_utils.py b/tests/utils/test_proto_utils.py index 6073b9939..298ea5c50 100644 --- a/tests/utils/test_proto_utils.py +++ b/tests/utils/test_proto_utils.py @@ -3,6 +3,10 @@ This module tests the proto utilities including to_stream_response and dictionary normalization. """ +import logging + +from typing import Any, cast + import httpx import pytest @@ -328,3 +332,57 @@ def test_nested_required_fields(self): fields = [e['field'] for e in errors] assert 'status.state' in fields + + +class TestWarnOnMissingRequiredFields: + """Tests for warn_on_missing_required_fields function.""" + + def test_complete_message_returns_true_without_logging( + self, caplog: pytest.LogCaptureFixture + ) -> None: + msg = Message( + message_id='msg-1', + role=Role.ROLE_USER, + parts=[Part(text='hello')], + ) + with caplog.at_level(logging.WARNING, logger=proto_utils.__name__): + assert proto_utils.warn_on_missing_required_fields(msg, 'test:') + assert caplog.records == [] + + def test_missing_fields_are_logged_instead_of_raised( + self, caplog: pytest.LogCaptureFixture + ) -> None: + with caplog.at_level(logging.WARNING, logger=proto_utils.__name__): + assert not proto_utils.warn_on_missing_required_fields( + Message(), 'message passed to test:' + ) + [record] = caplog.records + assert record.levelno == logging.WARNING + assert record.getMessage() == ( + 'message passed to test: Message is missing fields marked ' + 'REQUIRED by the A2A spec: message_id, role, parts' + ) + + def test_non_message_objects_are_not_checked( + self, caplog: pytest.LogCaptureFixture + ) -> None: + with caplog.at_level(logging.WARNING, logger=proto_utils.__name__): + assert proto_utils.warn_on_missing_required_fields( + cast('Any', object()), 'test:' + ) + assert caplog.records == [] + + def test_nested_missing_fields_are_reported_with_their_path( + self, caplog: pytest.LogCaptureFixture + ) -> None: + task = Task( + id='task-1', + context_id='ctx-1', + status=TaskStatus(state=TaskState.TASK_STATE_WORKING), + history=[Message()], + ) + with caplog.at_level(logging.WARNING, logger=proto_utils.__name__): + assert not proto_utils.warn_on_missing_required_fields( + task, 'test:' + ) + assert 'history[0].message_id' in caplog.text From 8fe8a932e8ff0e5e0199447a730f5958511b24e5 Mon Sep 17 00:00:00 2001 From: Patrick-SCH03 Date: Wed, 7 Oct 2026 23:05:38 +0900 Subject: [PATCH 2/2] fix(server): make the AgentCard spec warning actionable and cover serving and signing Address review on #1279: - Explain in the warning that a non-compliant card is still accepted but may not verify or interoperate across SDKs and could be rejected in a future major release, link to spec section 5.7, and say "missing or empty". - Also warn in agent_card_to_dict and signing._canonicalize_agent_card. --- .../request_handlers/response_helpers.py | 4 ++ src/a2a/utils/proto_utils.py | 16 ++++++-- src/a2a/utils/signing.py | 4 ++ .../request_handlers/test_response_helpers.py | 39 +++++++++++++++++++ tests/utils/test_proto_utils.py | 16 +++++++- tests/utils/test_signing.py | 14 +++++++ 6 files changed, 88 insertions(+), 5 deletions(-) diff --git a/src/a2a/server/request_handlers/response_helpers.py b/src/a2a/server/request_handlers/response_helpers.py index d41bab1d9..262e1030b 100644 --- a/src/a2a/server/request_handlers/response_helpers.py +++ b/src/a2a/server/request_handlers/response_helpers.py @@ -44,6 +44,7 @@ UnsupportedOperationError, VersionNotSupportedError, ) +from a2a.utils.proto_utils import warn_on_missing_required_fields EXCEPTION_MAP: dict[type[A2AError], type[JSONRPCError]] = { @@ -86,6 +87,9 @@ def agent_card_to_dict(card: AgentCard) -> dict[str, Any]: """Convert AgentCard to dict and inject backward compatibility fields.""" + warn_on_missing_required_fields( + card, 'agent_card passed to agent_card_to_dict:' + ) result = MessageToDict(card) try: diff --git a/src/a2a/utils/proto_utils.py b/src/a2a/utils/proto_utils.py index bbafa4dea..d8e3189cd 100644 --- a/src/a2a/utils/proto_utils.py +++ b/src/a2a/utils/proto_utils.py @@ -321,8 +321,14 @@ def validate_proto_required_fields(msg: ProtobufMessage) -> None: ) +_SPEC_FIELD_PRESENCE_URL = ( + 'https://a2a-protocol.org/latest/specification/' + '#57-field-presence-and-optionality' +) + + def warn_on_missing_required_fields(msg: ProtobufMessage, source: str) -> bool: - """Log a warning if fields marked as REQUIRED are missing on the message. + """Log a warning if fields marked as REQUIRED are missing or empty. Unlike `validate_proto_required_fields`, this never raises, so it can surface spec violations without breaking callers that currently rely on @@ -333,7 +339,7 @@ def warn_on_missing_required_fields(msg: ProtobufMessage, source: str) -> bool: source: Where the message was passed in, used in the log message. Returns: - True if every REQUIRED field is present, False otherwise. Objects that + True if every REQUIRED field is set, False otherwise. Objects that are not Protobuf messages (for example test doubles) are not checked. """ if not isinstance(msg, ProtobufMessage): @@ -342,10 +348,14 @@ def warn_on_missing_required_fields(msg: ProtobufMessage, source: str) -> bool: if not errors: return True logger.warning( - '%s %s is missing fields marked REQUIRED by the A2A spec: %s', + '%s %s is not spec-compliant - REQUIRED fields missing or empty: ' + '%s. This is allowed and does not raise, but it may not verify or ' + 'interoperate across SDKs and could be rejected in a future major ' + 'release. See %s', source, msg.DESCRIPTOR.name, ', '.join(err['field'] for err in errors), + _SPEC_FIELD_PRESENCE_URL, ) return False diff --git a/src/a2a/utils/signing.py b/src/a2a/utils/signing.py index 555b4ef07..c6540d162 100644 --- a/src/a2a/utils/signing.py +++ b/src/a2a/utils/signing.py @@ -22,6 +22,7 @@ from a2a.types import AgentCard, AgentCardSignature from a2a.utils._jcs import MAX_DEPTH, CanonicalizationError, canonicalize +from a2a.utils.proto_utils import warn_on_missing_required_fields class SignatureVerificationError(Exception): @@ -198,6 +199,9 @@ def _clean_empty(d: Any, depth: int = 1) -> Any: def _canonicalize_agent_card(agent_card: AgentCard) -> str: """Canonicalizes the Agent Card JSON according to RFC 8785 (JCS).""" + warn_on_missing_required_fields( + agent_card, 'agent_card passed to _canonicalize_agent_card:' + ) card_dict = MessageToDict( agent_card, ) diff --git a/tests/server/request_handlers/test_response_helpers.py b/tests/server/request_handlers/test_response_helpers.py index 2e7caeba3..7236f2b61 100644 --- a/tests/server/request_handlers/test_response_helpers.py +++ b/tests/server/request_handlers/test_response_helpers.py @@ -1,3 +1,4 @@ +import logging import unittest from a2a.server.request_handlers.response_helpers import ( @@ -13,6 +14,7 @@ AgentCapabilities, AgentCard, AgentInterface, + AgentSkill, Task, TaskState, TaskStatus, @@ -420,3 +422,40 @@ def test_prepare_response_object_with_invalid_response(self) -> None: if __name__ == '__main__': unittest.main() + + +class TestAgentCardToDictRequiredFields(unittest.TestCase): + def test_warns_when_required_fields_are_missing(self) -> None: + with self.assertLogs('a2a.utils.proto_utils', logging.WARNING) as logs: + result = agent_card_to_dict(AgentCard(name='partial')) + self.assertEqual(result['name'], 'partial') + [message] = logs.output + self.assertIn('agent_card passed to agent_card_to_dict:', message) + self.assertIn('description', message) + + def test_complete_card_does_not_warn(self) -> None: + card = AgentCard( + name='complete_agent', + description='An agent card with all required fields.', + supported_interfaces=[ + AgentInterface( + url='http://localhost:8000', + protocol_binding='JSONRPC', + protocol_version='1.0', + ) + ], + version='1.0', + capabilities=AgentCapabilities(), + default_input_modes=['text/plain'], + default_output_modes=['text/plain'], + skills=[ + AgentSkill( + id='echo', + name='Echo', + description='Echoes the input.', + tags=['test'], + ) + ], + ) + with self.assertNoLogs('a2a.utils.proto_utils', logging.WARNING): + agent_card_to_dict(card) diff --git a/tests/utils/test_proto_utils.py b/tests/utils/test_proto_utils.py index 298ea5c50..594ed9797 100644 --- a/tests/utils/test_proto_utils.py +++ b/tests/utils/test_proto_utils.py @@ -359,8 +359,12 @@ def test_missing_fields_are_logged_instead_of_raised( [record] = caplog.records assert record.levelno == logging.WARNING assert record.getMessage() == ( - 'message passed to test: Message is missing fields marked ' - 'REQUIRED by the A2A spec: message_id, role, parts' + 'message passed to test: Message is not spec-compliant - ' + 'REQUIRED fields missing or empty: message_id, role, parts. ' + 'This is allowed and does not raise, but it may not verify or ' + 'interoperate across SDKs and could be rejected in a future ' + 'major release. See ' + 'https://a2a-protocol.org/latest/specification/#57-field-presence-and-optionality' ) def test_non_message_objects_are_not_checked( @@ -386,3 +390,11 @@ def test_nested_missing_fields_are_reported_with_their_path( task, 'test:' ) assert 'history[0].message_id' in caplog.text + + def test_empty_repeated_field_is_reported( + self, caplog: pytest.LogCaptureFixture + ) -> None: + msg = Message(message_id='msg-1', role=Role.ROLE_USER, parts=[]) + with caplog.at_level(logging.WARNING, logger=proto_utils.__name__): + assert not proto_utils.warn_on_missing_required_fields(msg, 'test:') + assert 'REQUIRED fields missing or empty: parts.' in caplog.text diff --git a/tests/utils/test_signing.py b/tests/utils/test_signing.py index 616aab9f3..f22c60573 100644 --- a/tests/utils/test_signing.py +++ b/tests/utils/test_signing.py @@ -1,3 +1,5 @@ +import logging + from typing import Any import pytest @@ -287,3 +289,15 @@ def test_clean_empty_does_not_mutate_input(): signing._clean_empty(original) assert original == original_copy + + +def test_canonicalize_agent_card_warns_about_missing_required_fields( + caplog: pytest.LogCaptureFixture, +): + with caplog.at_level(logging.WARNING, logger='a2a.utils.proto_utils'): + signing._canonicalize_agent_card(AgentCard(name='partial')) + [record] = caplog.records + assert record.getMessage().startswith( + 'agent_card passed to _canonicalize_agent_card: AgentCard is not ' + 'spec-compliant' + )