feat: add JSON-RPC client transport implementation - #79
Conversation
Implements JSON-RPC 2.0 client transport for A2A protocol communication per specification §7. Provides complete support for all 10 required methods with SSE streaming capabilities. Key features: - Full A2A JSONRPC 2.0 specification compliance (§6.11, §7) - All 10 required methods: message/send, message/stream, tasks/get, tasks/cancel, tasks/resubscribe, and push notification config methods - SSE streaming support for real-time task updates - Thread-safe agent card management with RWMutex - 5-second default timeout matching Python SDK behavior - Comprehensive test suite with race detection Implementation details: - Validates all required fields per A2A spec (Task: id/contextId/status.state, Message: id/role/parts) to ensure strict spec compliance - Proper resource cleanup with context cancellation monitoring - Behavioral parity with Python SDK reference implementation - 58.6% test coverage including concurrent access scenarios Fixes #73
Summary of ChangesHello @joshuafuller, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new JSON-RPC 2.0 client transport implementation for the Go SDK, designed to facilitate communication with A2A agents over HTTP. It provides comprehensive support for all required A2A JSON-RPC methods, including both standard and Server-Sent Events (SSE) streaming for real-time updates. The implementation emphasizes robustness with features like thread-safe agent card caching, a default 5-second HTTP timeout, and a minimal dependency footprint by relying only on the Go standard library. It has been thoroughly validated against the Python SDK reference implementation to ensure behavioral consistency and includes a robust test suite. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive implementation of a JSON-RPC 2.0 client transport for the A2A Go SDK, including support for all required methods, SSE streaming, and thread-safe caching. The implementation is well-structured and is accompanied by a thorough test suite. My review focuses on improving concurrency patterns, simplifying complex logic, and enhancing maintainability by removing hardcoded values. I've also identified a potential issue in a concurrent test case that could mask a race condition.
- Fix concurrent test to verify exactly 1 network call - Extract hardcoded strings to constants for maintainability - Remove unnecessary goroutine for context cancellation - Simplify agent card locking with standard double-checked pattern All tests pass with race detection.
|
Thanks for the review feedback! I've addressed all the points in commit fb75ea1: High Priority:
Medium Priority:
All tests pass with race detection. Net result: cleaner, more maintainable code (-18 lines). |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a solid implementation of a JSON-RPC 2.0 client transport for the Go SDK. The code is well-structured, follows Go best practices like using functional options, and includes a comprehensive test suite, notably with a good concurrency test for GetAgentCard. My review focuses on a few areas to enhance robustness and maintainability: ensuring clear resource ownership by removing redundant defer calls, improving error handling by wrapping errors with more context for better debugging, and promoting consistent error checking within the test suite.
There was a problem hiding this comment.
Code Review
This pull request introduces a well-structured and comprehensive implementation of a JSON-RPC 2.0 client transport for the A2A Go SDK. The code is clean, follows Go best practices, and includes an extensive test suite that covers core functionality, streaming, error handling, and concurrency. The use of functional options for configuration is a nice touch. I've identified a couple of minor issues related to resource management and error handling that could be improved for better robustness and debuggability. Overall, this is a high-quality contribution.
- Remove redundant defer body.Close() calls (parseSSEStream owns closure) - Add error context to unmarshalEvent for clearer failure messages - Add explicit error checking to test JSON decode calls All tests pass with race detection.
|
Addressed the second round of feedback in commit acdd074: Resource Ownership:
Error Context:
Test Error Handling:
All tests continue to pass with race detection. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive implementation of a JSON-RPC 2.0 client transport for the A2A Go SDK. The code is well-structured, follows Go idioms, and includes a good set of tests covering various scenarios, including concurrency. The implementation correctly handles non-streaming and streaming (SSE) requests, resource management (like closing HTTP bodies), and thread-safe caching of the agent card. My review includes suggestions to improve efficiency, reduce code duplication, and fix a potential data race condition to further enhance the robustness of this new transport.
- Fix GetAgentCard to return copies to prevent data races - Refactor SendMessage to use unmarshalEvent (avoid double unmarshaling) - Extract streamRequestToEvents helper to reduce code duplication All tests pass with race detection.
|
Addressed the third round of feedback in commit fdbe4ae: High Priority:
Medium Priority:
All tests pass with race detection. Net result: cleaner code with better efficiency (+31, -48 lines). |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive implementation of a JSON-RPC 2.0 client transport for the A2A Go SDK. The implementation is well-structured, following the existing grpcTransport pattern, and includes support for all required A2A methods, SSE streaming, and thread-safe agent card caching. The accompanying test suite is thorough, covering various scenarios including success cases, error handling, streaming, and concurrency, which provides good confidence in the new transport's correctness.
My review focuses on improving debuggability and performance. I've suggested enhancing error messages with response bodies for failed HTTP requests and optimizing the SSE parsing loop to reduce memory allocations. Overall, this is a solid contribution that significantly enhances the SDK's capabilities.
| if httpResp.StatusCode != http.StatusOK { | ||
| return nil, fmt.Errorf("unexpected HTTP status code: %d", httpResp.StatusCode) | ||
| } |
There was a problem hiding this comment.
When a non-200 HTTP status is received, the response body often contains useful error details. Including this body in the error message will significantly improve debuggability.
| if httpResp.StatusCode != http.StatusOK { | |
| return nil, fmt.Errorf("unexpected HTTP status code: %d", httpResp.StatusCode) | |
| } | |
| if httpResp.StatusCode != http.StatusOK { | |
| bodyBytes, _ := io.ReadAll(httpResp.Body) | |
| return nil, fmt.Errorf("unexpected HTTP status code: %d, body: %s", httpResp.StatusCode, string(bodyBytes)) | |
| } |
There was a problem hiding this comment.
we don't expect anything in the body, but I'd rather log status text than code:
--- a/a2aclient/jsonrpc.go
+++ b/a2aclient/jsonrpc.go
@@ -179,7 +179,7 @@ func (t *jsonrpcTransport) sendRequest(ctx context.Context, method string, param
defer func() { _ = httpResp.Body.Close() }()
if httpResp.StatusCode != http.StatusOK {
- return nil, fmt.Errorf("unexpected HTTP status code: %d", httpResp.StatusCode)
+ return nil, fmt.Errorf("unexpected HTTP status code: %s", httpResp.Status)
}| if httpResp.StatusCode != http.StatusOK { | ||
| httpResp.Body.Close() | ||
| return nil, fmt.Errorf("unexpected HTTP status code: %d", httpResp.StatusCode) | ||
| } |
There was a problem hiding this comment.
When a non-200 HTTP status is received, the response body often contains useful error details. Including this body in the error message will significantly improve debuggability. Note that the body is already being closed here in case of an error, so reading it first is safe.
| if httpResp.StatusCode != http.StatusOK { | |
| httpResp.Body.Close() | |
| return nil, fmt.Errorf("unexpected HTTP status code: %d", httpResp.StatusCode) | |
| } | |
| if httpResp.StatusCode != http.StatusOK { | |
| bodyBytes, _ := io.ReadAll(httpResp.Body) | |
| httpResp.Body.Close() | |
| return nil, fmt.Errorf("unexpected HTTP status code: %d, body: %s", httpResp.StatusCode, string(bodyBytes)) | |
| } |
|
After three rounds of automated review feedback, the implementation has been significantly improved:
All CI checks pass with 58.6% test coverage. The code is ready for maintainer review. Happy to continue iterating if needed, but I believe this is in good shape for the initial JSON-RPC transport contribution. Thanks for the feedback and guidance! |
Explicitly handle error returns from Close(), Write(), and Encode() calls in cleanup paths and test helpers.
Refactor parseSSEStream to eliminate unnecessary allocations in the streaming loop by using byte operations instead of string operations: - Use scanner.Bytes() instead of scanner.Text() to avoid string allocation - Replace strings.HasPrefix with bytes.HasPrefix for direct byte comparison - Hoist prefix bytes conversion outside loop to prevent repeated allocations - Use byte slicing instead of strings.TrimPrefix - Remove unnecessary byte-to-string conversions This optimization reduces GC pressure and improves throughput in high-volume streaming scenarios by eliminating intermediate allocations on each SSE event.
Address code review comments from yarolegovich:
1. Replace interface{} with any throughout the codebase
- Updated function parameters and type assertions to use 'any'
- Removed unused sync import
2. Refactor parseSSEStream resource ownership
- Changed signature from io.ReadCloser to io.Reader
- Moved defer body.Close() to caller for cleaner resource management
3. Implement kind-based event discrimination
- Added MarshalJSON methods to Message, Task, TaskStatusUpdateEvent,
and TaskArtifactUpdateEvent to inject 'kind' field per A2A spec
- Created UnmarshalEventJSON() function in a2a package for centralized
event unmarshaling using 'kind' discriminator
- Removed transport-specific unmarshalEvent() function that used
fragile field introspection
- Added comprehensive test coverage for new Event JSON functionality
4. Remove agent card caching from transport layer
- Simplified GetAgentCard to return provided card without caching
- Removed mutex and extended card fetching logic
- Updated tests to reflect simplified behavior
All changes follow existing patterns in the codebase (ContentParts
unmarshaling, TextPart marshaling) and align with A2A specification.
Test coverage: All existing tests pass with race detection enabled.
Added 4 new test functions covering Event JSON marshaling/unmarshaling.
|
Thanks for the detailed review! I've addressed all four comments in 1061a48:
All tests pass with race detection enabled. I also added comprehensive test coverage for the new Event JSON |
|
nice work, I've checked that SSE is working with a Python sample: can you please mark comments you've addressed (including those from gemini) as resolved so that it's easier to navigate the PR |
Addresses PR #79 review feedback from yarolegovich: "got before want" test pattern: - Update all test error messages to follow Go best practices - a2aclient/jsonrpc_test.go: 48 messages updated - a2a/event_json_test.go: all messages updated - Example: "Expected POST, got %s" → "got %s, want POST" - Reference: https://go.dev/wiki/TestComments#got-before-want Simplify checkJSON with wantSubstrings: - Replace checkJSON functions with declarative wantSubstrings field - Reduced code by ~40 lines while maintaining coverage Use strings.Contains from stdlib: - Remove custom contains() and containsAt() helper functions - Use stdlib strings.Contains() throughout Add TODO comments for error logging: - Add TODO(yarolegovich) for all swallowed errors (3 locations) - Track future logging implementation once approach is decided
Addresses PR #79 review feedback from yarolegovich to include JSON-RPC transport in default options. Changes: - Add WithJSONRPCTransport() to defaultOptions - Order transports to match other A2A SDKs: JSON-RPC first, then gRPC Cross-SDK research shows consistent defaults: - Python SDK: JSON-RPC is primary/fallback transport - Java SDK: JSON-RPC included by default in client artifact - JavaScript SDK: jsonrpc_transport_handler.ts as main transport - Go SDK: Now matches with JSON-RPC first, gRPC second This ensures consistent behavior across all A2A client implementations. References: - https://github.com/a2aproject/a2a-python/blob/main/src/a2a/client/client_factory.py - https://github.com/a2aproject/a2a-java - https://github.com/a2aproject/a2a-js
Addresses PR #79 review feedback from yarolegovich: Remove obsolete concurrent test: - Removed TestJSONRPCTransport_GetAgentCard_Concurrent - Test is no longer needed since agent card caching was removed in 1061a48 - Concurrency concerns are now trivial without cache/locking Add TODO for error conversion: - Add TODO(yarolegovich) for transport-agnostic error format - Need to support errors.Is(err, a2a.ErrMethodNotFound) - Applies to all transports (not yet in grpc either) - Reviewer will open ticket to track this work
|
lgtm, @mazas-google, @hyangah can you have a look please? |
|
got a green light to proceed with merge from @herczyn |
🤖 I have created a release *beep* *boop* --- ## 0.3.0 (2025-11-04) ### Features * add JSON-RPC client transport implementation ([#79](a2aproject#79)) ([1690088](a2aproject@1690088)) * agent card resolver ([#48](a2aproject#48)) ([0951293](a2aproject@0951293)) * blocking flag handling ([#97](a2aproject#97)) ([f7aa465](a2aproject@f7aa465)), closes [#96](a2aproject#96) * client API proposal ([#32](a2aproject#32)) ([b6ca54f](a2aproject@b6ca54f)) * client auth interceptor ([#90](a2aproject#90)) ([25b9aae](a2aproject@25b9aae)) * client interceptor invocations ([#51](a2aproject#51)) ([3e9f2ae](a2aproject@3e9f2ae)) * core types JSON codec ([#42](a2aproject#42)) ([c5b3982](a2aproject@c5b3982)) * define core types and interfaces ([#16](a2aproject#16)) ([69b96ea](a2aproject@69b96ea)) * disallow custom types and circular refs in Metadata ([#43](a2aproject#43)) ([53bc928](a2aproject@53bc928)) * get task implementation ([#59](a2aproject#59)) ([f74d854](a2aproject@f74d854)) * grpc authenticated agent card and producer utils ([#85](a2aproject#85)) ([9d82f31](a2aproject@9d82f31)), closes [#82](a2aproject#82) * grpc client transport ([#66](a2aproject#66)) ([fee703e](a2aproject@fee703e)) * grpc code generation from A2A .proto spec ([#11](a2aproject#11)) ([2993b98](a2aproject@2993b98)) * handling artifacts and implementing send message stream ([#52](a2aproject#52)) ([c3fa631](a2aproject@c3fa631)) * implement an a2aclient.Factory ([#50](a2aproject#50)) ([49deee7](a2aproject@49deee7)) * implementing grpc server wrapper ([#37](a2aproject#37)) ([071e952](a2aproject@071e952)) * implementing message-message interaction ([#34](a2aproject#34)) ([b568979](a2aproject@b568979)) * implementing task pushes ([#86](a2aproject#86)) ([c210240](a2aproject@c210240)) * input-required and auth-required handling ([#70](a2aproject#70)) ([3ac89ba](a2aproject@3ac89ba)) * jsonrpc server ([#91](a2aproject#91)) ([5491030](a2aproject@5491030)) * logger ([#56](a2aproject#56)) ([86ab9d2](a2aproject@86ab9d2)) * request context loading ([#60](a2aproject#60)) ([ab7a29b](a2aproject@ab7a29b)) * result aggregation part 1 - task store ([#38](a2aproject#38)) ([d3c02f5](a2aproject@d3c02f5)) * result aggregation part 3 - concurrent task executor ([#40](a2aproject#40)) ([265c3e7](a2aproject@265c3e7)) * result aggregation part 4 - integration ([#41](a2aproject#41)) ([bab72d9](a2aproject@bab72d9)) * SDK type utilities ([#31](a2aproject#31)) ([32b77b4](a2aproject@32b77b4)) * server middleware API ([#63](a2aproject#63)) ([738bf85](a2aproject@738bf85)) * server middleware integration ([#64](a2aproject#64)) ([5dc8be0](a2aproject@5dc8be0)) * smarter a2aclient ([#88](a2aproject#88)) ([322d05b](a2aproject@322d05b)) * task event factory ([#95](a2aproject#95)) ([fbf3bcf](a2aproject@fbf3bcf)), closes [#84](a2aproject#84) * task executor docs ([#36](a2aproject#36)) ([b6868df](a2aproject@b6868df)) * task update logic ([0ac987f](a2aproject@0ac987f)) ### Bug Fixes * Execute() callers missing events ([#74](a2aproject#74)) ([4c3389f](a2aproject@4c3389f)) * mark task failed when execution fails ([#94](a2aproject#94)) ([ee0e7ed](a2aproject@ee0e7ed)) * push semantics update ([#93](a2aproject#93)) ([76bff9f](a2aproject@76bff9f)) * race detector queue closed access ([c07b7d0](a2aproject@c07b7d0)) * regenerate proto and update converters ([#81](a2aproject#81)) ([c732060](a2aproject@c732060)) * streaming ([#92](a2aproject#92)) ([ca7a64b](a2aproject@ca7a64b)) ### Miscellaneous Chores * release 0.3.0 ([fa7cfba](a2aproject@fa7cfba)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
2 participants
Overview
Adds JSON-RPC 2.0 client transport support to the Go SDK. Implementation follows the structure and code style discussed in #73.
Implementation
a2aclient/jsonrpc.gofollowing thegrpcTransportpatternTesting & Validation
Following the guidance in #73, I validated the implementation against the Python SDK reference implementation to ensure behavioral consistency:
Test suite includes 58.6% coverage with race detection:
$ go test -race ./a2aclient -run TestJSONRPC PASS ok github.com/a2aproject/a2a-go/a2aclient 1.163sAll tests validate spec compliance including required field validation (Task: id/contextId/status.state, Message: id/role/parts).
Files
a2aclient/jsonrpc.go(558 lines) - Transport implementationa2aclient/jsonrpc_test.go(755 lines) - Test suiteFixes #73