feat: Add gRPC binding: client, ASP.NET Core server, and tests - #432
feat: Add gRPC binding: client, ASP.NET Core server, and tests#432janosSarusiKis wants to merge 4 commits into
Conversation
Signed-off-by: Janos Sarusi-Kis <janossk@cisco.com>
There was a problem hiding this comment.
Code Review
This pull request introduces the gRPC binding for the Agent2Agent (A2A) protocol, adding the A2A.Grpc client and A2A.Grpc.AspNetCore server host libraries along with comprehensive integration and unit tests. The feedback focuses on defensive programming improvements to prevent potential NullReferenceExceptions. Specifically, it is recommended to add null checks before iterating over card.SupportedInterfaces and card.Skills in ProtoMap.AgentCard.cs, before iterating over response.Tasks in ProtoMap.Messages.cs, and within the AddScopes helper method when the source dictionary is null.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| foreach (var agentInterface in card.SupportedInterfaces) | ||
| { | ||
| result.SupportedInterfaces.Add(ToProto(agentInterface)); | ||
| } | ||
|
|
||
| foreach (var skill in card.Skills) | ||
| { | ||
| result.Skills.Add(ToProto(skill)); | ||
| } |
There was a problem hiding this comment.
The properties card.SupportedInterfaces and card.Skills are iterated directly without null checks. If these properties are null (for example, if the AgentCard is manually constructed or partially deserialized), this will throw a NullReferenceException.
To ensure robust defensive programming and consistency with other collection properties in this class (such as card.SecuritySchemes and card.SecurityRequirements), please add null checks before iterating.
if (card.SupportedInterfaces is not null)
{
foreach (var agentInterface in card.SupportedInterfaces)
{
result.SupportedInterfaces.Add(ToProto(agentInterface));
}
}
if (card.Skills is not null)
{
foreach (var skill in card.Skills)
{
result.Skills.Add(ToProto(skill));
}
}| private static void AddScopes(Google.Protobuf.Collections.MapField<string, string> target, Dictionary<string, string> source) | ||
| { | ||
| foreach (var pair in source) | ||
| { | ||
| target[pair.Key] = pair.Value; | ||
| } | ||
| } |
There was a problem hiding this comment.
The AddScopes helper method does not check if the source dictionary is null. If a client manually constructs an OAuth flow (such as AuthorizationCodeOAuthFlow) and leaves the Scopes property uninitialized (null), this method will throw a NullReferenceException.
Adding a null check prevents this crash and aligns with defensive programming practices.
private static void AddScopes(Google.Protobuf.Collections.MapField<string, string> target, Dictionary<string, string>? source)
{
if (source is null)
{
return;
}
foreach (var pair in source)
{
target[pair.Key] = pair.Value;
}
}| { | ||
| result.Tasks.Add(ToProto(task)); | ||
| } | ||
|
|
There was a problem hiding this comment.
The response.Tasks collection is iterated directly without a null check. If response.Tasks is null, this will throw a NullReferenceException.
To be consistent with other response mapping methods (such as ToProto(ListTaskPushNotificationConfigResponse response) which checks response.Configs for null), please add a null check before iterating.
if (response.Tasks is not null)
{
foreach (var task in response.Tasks)
{
result.Tasks.Add(ToProto(task));
}
}|
Regarding the bot's comments: The existing null checks the suggestion points to (SecuritySchemes, SecurityRequirements, Configs) exist because those are declared nullable (List<…>?). The convention in the mapper is to null-check nullable collections and iterate non-nullable ones directly — which is why other non-nullable collections here (Message.Parts, Artifact.Parts, AgentSkill.Tags, DefaultInputModes/DefaultOutputModes) are also iterated without a guard. That said, I'm happy to add these guards for extra defensiveness — and if so I'll apply them consistently across all non-nullable collection iterations in the mapper, not just the three spots flagged here, to keep it uniform. |
Signed-off-by: Janos Sarusi-Kis <janossk@cisco.com>
Signed-off-by: Janos Sarusi-Kis <janossk@cisco.com>
Signed-off-by: Janos Sarusi-Kis <janossk@cisco.com>
3e082f5 to
ed10388
Compare
|
@darrelmiller If you have the time, could you check this one out? Thanks in advance! |
Adds first-party support for the A2A gRPC binding alongside the existing JSON-RPC and HTTP+JSON bindings, as requested in #397. All three bindings share the same IA2AClient contract and IA2ARequestHandler server pipeline — no business logic is forked.
What's included
Two new packages (kept separate so core/client consumers don't take an ASP.NET Core dependency):
A2A.Grpc — the canonical A2A .proto (vendored, with google.api transcoding imports stripped; generated at build via Grpc.Tools), a bidirectional proto↔domain mapping layer, and A2AGrpcClient (an IA2AClient over gRPC with unary + server-streaming parity). Registers the GRPC binding with A2AClientFactory.
A2A.Grpc.AspNetCore — A2AGrpcService mapping the generated service onto IA2ARequestHandler, plus AddA2AGrpc() / MapGrpcA2A() extensions mirroring MapA2A / MapHttpA2A.
Error handling — reuses the shared A2AErrorCodeMapping to translate A2AException↔RpcException, carrying the exact A2AErrorCode in a response trailer.
Tests — mapping round-trip tests (including Part oneofs, JSON metadata, push-config flattening, the full AgentCard/OAuth graph) and end-to-end client↔server integration tests over an in-memory TestServer (unary, both streaming RPCs, error propagation).
Notes
Builds on net8.0 and net10.0; libraries marked IsAotCompatible and build warning-clean.
The vendored .proto documents its source and the exact edits applied, so it can be re-derived when upstream changes.
Closes #397Adds first-party support for the A2A gRPC binding alongside the existing JSON-RPC and HTTP+JSON bindings, as requested in #397. All three bindings share the same IA2AClient contract and IA2ARequestHandler server pipeline — no business logic is forked.
What's included
Two new packages (kept separate so core/client consumers don't take an ASP.NET Core dependency):
A2A.Grpc — the canonical A2A .proto (vendored, with google.api transcoding imports stripped; generated at build via Grpc.Tools), a bidirectional proto↔domain mapping layer, and A2AGrpcClient (an IA2AClient over gRPC with unary + server-streaming parity). Registers the GRPC binding with A2AClientFactory.
A2A.Grpc.AspNetCore — A2AGrpcService mapping the generated service onto IA2ARequestHandler, plus AddA2AGrpc() / MapGrpcA2A() extensions mirroring MapA2A / MapHttpA2A.
Error handling — reuses the shared A2AErrorCodeMapping to translate A2AException↔RpcException, carrying the exact A2AErrorCode in a response trailer.
Tests — mapping round-trip tests (including Part oneofs, JSON metadata, push-config flattening, the full AgentCard/OAuth graph) and end-to-end client↔server integration tests over an in-memory TestServer (unary, both streaming RPCs, error propagation).
I also forked the repository behind https://agntcy.github.io/csit/a2a/ and used that to test the feature: https://janossarusikis.github.io/csit/a2a/ .
Notes
Builds on net8.0 and net10.0; libraries marked IsAotCompatible and build warning-clean.
The vendored .proto documents its source and the exact edits applied, so it can be re-derived when upstream changes.
Closes #397