refactor: address PR review comments

- Fix client factory transport preference selection
- Update proto handling to use MergeFrom/CopyFrom correctly
- Remove preserving_proto_field_name from MessageToDict
- Fix history length limiting logic
- Improve SendMessageResponse construction in handler

Signed-off-by: Luca Muscariello <muscariello@ieee.org>

Author: muscariello

src/a2a/client/base_client.py

@@ -80,10 +80,6 @@ async def send_message(
 
         if configuration:
             config.MergeFrom(configuration)
-            # Proto3 doesn't support HasField for scalars, so MergeFrom won't
-            # override with default values (e.g. False). We explicitly set it here
-            # assuming configuration is authoritative.
-            config.blocking = configuration.blocking
 
         send_message_request = SendMessageRequest(
             message=request, configuration=config, metadata=request_metadata

src/a2a/client/client_factory.py

@@ -19,11 +19,11 @@
     AgentCard,
     AgentInterface,
 )
-
-
-TRANSPORT_PROTOCOLS_JSONRPC = 'JSONRPC'
-TRANSPORT_PROTOCOLS_GRPC = 'GRPC'
-TRANSPORT_PROTOCOLS_HTTP_JSON = 'HTTP+JSON'
+from a2a.utils.constants import (
+    TRANSPORT_GRPC,
+    TRANSPORT_HTTP_JSON,
+    TRANSPORT_JSONRPC,
+)
 
 
 try:
@@ -74,9 +74,9 @@ def __init__(
 
     def _register_defaults(self, supported: list[str]) -> None:
         # Empty support list implies JSON-RPC only.
-        if TRANSPORT_PROTOCOLS_JSONRPC in supported or not supported:
+        if TRANSPORT_JSONRPC in supported or not supported:
             self.register(
-                TRANSPORT_PROTOCOLS_JSONRPC,
+                TRANSPORT_JSONRPC,
                 lambda card, url, config, interceptors: JsonRpcTransport(
                     config.httpx_client or httpx.AsyncClient(),
                     card,
@@ -85,9 +85,9 @@ def _register_defaults(self, supported: list[str]) -> None:
                     config.extensions or None,
                 ),
             )
-        if TRANSPORT_PROTOCOLS_HTTP_JSON in supported:
+        if TRANSPORT_HTTP_JSON in supported:
             self.register(
-                TRANSPORT_PROTOCOLS_HTTP_JSON,
+                TRANSPORT_HTTP_JSON,
                 lambda card, url, config, interceptors: RestTransport(
                     config.httpx_client or httpx.AsyncClient(),
                     card,
@@ -96,14 +96,14 @@ def _register_defaults(self, supported: list[str]) -> None:
                     config.extensions or None,
                 ),
             )
-        if TRANSPORT_PROTOCOLS_GRPC in supported:
+        if TRANSPORT_GRPC in supported:
             if GrpcTransport is None:
                 raise ImportError(
                     'To use GrpcClient, its dependencies must be installed. '
                     'You can install them with \'pip install "a2a-sdk[grpc]"\''
                 )
             self.register(
-                TRANSPORT_PROTOCOLS_GRPC,
+                TRANSPORT_GRPC,
                 GrpcTransport.create,
             )
 
@@ -206,25 +206,30 @@ def create(
           If there is no valid matching of the client configuration with the
           server configuration, a `ValueError` is raised.
         """
-        server_set = {
-            x.protocol_binding: x.url for x in card.supported_interfaces
-        }
         client_set = self._config.supported_protocol_bindings or [
-            TRANSPORT_PROTOCOLS_JSONRPC
+            TRANSPORT_JSONRPC
         ]
         transport_protocol = None
         transport_url = None
         if self._config.use_client_preference:
-            for x in client_set:
-                if x in server_set:
-                    transport_protocol = x
-                    transport_url = server_set[x]
+            for protocol_binding in client_set:
+                supported_interface = next(
+                    (
+                        si
+                        for si in card.supported_interfaces
+                        if si.protocol_binding == protocol_binding
+                    ),
+                    None,
+                )
+                if supported_interface:
+                    transport_protocol = protocol_binding
+                    transport_url = supported_interface.url
                     break
         else:
-            for x, url in server_set.items():
-                if x in client_set:
-                    transport_protocol = x
-                    transport_url = url
+            for supported_interface in card.supported_interfaces:
+                if supported_interface.protocol_binding in client_set:
+                    transport_protocol = supported_interface.protocol_binding
+                    transport_url = supported_interface.url
                     break
         if not transport_protocol or not transport_url:
             raise ValueError('no compatible transports found.')

src/a2a/server/apps/rest/rest_adapter.py

@@ -158,7 +158,7 @@ async def handle_get_agent_card(
         if self.card_modifier:
             card_to_serve = self.card_modifier(card_to_serve)
 
-        return MessageToDict(card_to_serve, preserving_proto_field_name=True)
+        return MessageToDict(card_to_serve)
 
     async def handle_authenticated_agent_card(
         self, request: Request, call_context: ServerCallContext | None = None

src/a2a/server/request_handlers/jsonrpc_handler.py

@@ -20,6 +20,7 @@
     ListTaskPushNotificationConfigRequest,
     Message,
     SendMessageRequest,
+    SendMessageResponse,
     SetTaskPushNotificationConfigRequest,
     SubscribeToTaskRequest,
     Task,
@@ -114,22 +115,16 @@ async def on_message_send(
                 request, context
             )
             # Build result based on return type
+            response = SendMessageResponse()
             if isinstance(task_or_message, Task):
-                result = {
-                    'task': MessageToDict(
-                        task_or_message, preserving_proto_field_name=False
-                    )
-                }
+                response.task.CopyFrom(task_or_message)
             elif isinstance(task_or_message, Message):
-                result = {
-                    'message': MessageToDict(
-                        task_or_message, preserving_proto_field_name=False
-                    )
-                }
+                response.message.CopyFrom(task_or_message)
             else:
-                result = MessageToDict(
-                    task_or_message, preserving_proto_field_name=False
-                )
+                # Should we handle this fallthrough?
+                pass
+
+            result = MessageToDict(response)
             return _build_success_response(request_id, result)
         except ServerError as e:
             return _build_error_response(

src/a2a/server/tasks/task_manager.py

@@ -142,7 +142,7 @@ async def save_task_event(
             if task.status.HasField('message'):
                 task.history.append(task.status.message)
             if event.metadata:
-                task.metadata.update(dict(event.metadata))  # type: ignore[arg-type]
+                task.metadata.MergeFrom(event.metadata)
             task.status.CopyFrom(event.status)
         else:
             logger.debug('Appending artifact to task %s', task.id)

src/a2a/utils/task.py

@@ -96,6 +96,7 @@ def apply_history_length(task: Task, history_length: int | None) -> Task:
         task_copy = Task()
         task_copy.CopyFrom(task)
         # Clear and re-add history items
-        task_copy.history[:] = limited_history
+        del task_copy.history[:]
+        task_copy.history.extend(limited_history)
         return task_copy
     return task