fix(eventqueue): add JSON marshal/unmarshal to Message so events survive roundtrip (fixes #349) - #360
Conversation
…ive roundtrip (fixes a2aproject#349) Message contains an a2a.Event (interface type) which Go's default encoding/json cannot deserialize. Add custom MarshalJSON/UnmarshalJSON that delegates to a2a.StreamResponse for the Event field, using the same polymorphic wrapper pattern already established in a2a/core.go. Add queue_test.go with roundtrip tests covering all four event types: TaskStatusUpdateEvent, TaskArtifactUpdateEvent, Message, Task.
There was a problem hiding this comment.
Code Review
This pull request implements custom JSON marshaling and unmarshaling for the Message struct in a2asrv/eventqueue/queue.go to ensure that the a2a.Event interface survives a JSON roundtrip. It also introduces comprehensive unit tests in a2asrv/eventqueue/queue_test.go to verify the roundtrip behavior across different event types. The review feedback suggests adding defensive checks in both MarshalJSON and UnmarshalJSON to handle nil event scenarios and prevent potential runtime panics or invalid states.
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.
| func (m *Message) UnmarshalJSON(data []byte) error { | ||
| var wrapper messageJSON | ||
| if err := json.Unmarshal(data, &wrapper); err != nil { | ||
| return err | ||
| } | ||
| m.Event = wrapper.Event.Event | ||
| m.TaskVersion = wrapper.TaskVersion | ||
| m.Protocol = wrapper.Protocol | ||
| return nil | ||
| } |
There was a problem hiding this comment.
If the incoming JSON is missing the event field or contains an invalid event type, wrapper.Event.Event will be nil. Assigning this to m.Event results in a nil interface, which will cause a panic when methods are called on it. Additionally, we should defensively check if the receiver m is nil to prevent a panic during assignment.
func (m *Message) UnmarshalJSON(data []byte) error {
if m == nil {
return errors.New("eventqueue: UnmarshalJSON on nil Message pointer")
}
var wrapper messageJSON
if err := json.Unmarshal(data, &wrapper); err != nil {
return err
}
if wrapper.Event.Event == nil {
return errors.New("eventqueue: message event cannot be nil")
}
m.Event = wrapper.Event.Event
m.TaskVersion = wrapper.TaskVersion
m.Protocol = wrapper.Protocol
return nil
}| func (m Message) MarshalJSON() ([]byte, error) { | ||
| return json.Marshal(messageJSON{ | ||
| Event: a2a.StreamResponse{Event: m.Event}, | ||
| TaskVersion: m.TaskVersion, | ||
| Protocol: m.Protocol, | ||
| }) | ||
| } |
There was a problem hiding this comment.
If m.Event is nil, marshaling will proceed with a nil event, which represents an invalid state and could lead to issues downstream. Consider adding a defensive check to return an error if m.Event is nil.
func (m Message) MarshalJSON() ([]byte, error) {
if m.Event == nil {
return nil, errors.New("eventqueue: cannot marshal Message with nil Event")
}
return json.Marshal(messageJSON{
Event: a2a.StreamResponse{Event: m.Event},
TaskVersion: m.TaskVersion,
Protocol: m.Protocol,
})
}There was a problem hiding this comment.
This makes sense, please address this and other gemini-code-assist comment
| @@ -0,0 +1,103 @@ | |||
| // Copyright 2025 The A2A Authors | |||
| func (m Message) MarshalJSON() ([]byte, error) { | ||
| return json.Marshal(messageJSON{ | ||
| Event: a2a.StreamResponse{Event: m.Event}, | ||
| TaskVersion: m.TaskVersion, | ||
| Protocol: m.Protocol, | ||
| }) | ||
| } |
There was a problem hiding this comment.
This makes sense, please address this and other gemini-code-assist comment
…JSON MarshalJSON now returns ErrNilEvent when Event is nil instead of producing invalid JSON. UnmarshalJSON returns ErrNilEvent when the incoming JSON is missing or has an invalid event field. Fixes review feedback from nahapetyan-serob on a2aproject#360.
|
Addressed review feedback:
All existing tests still pass. PTAL @nahapetyan-serob |
Addresses the remaining review note on the new test file's header year. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🤖 I have created a release *beep* *boop* --- ## [2.4.0](v2.3.1...v2.4.0) (2026-07-28) ### Features * provide generic pull event queue ([#354](#354)) ([6a48d4b](6a48d4b)) * **push:** push sender SSRF protection enabled by default (fixes [#373](#373)) ([#374](#374)) ([0a4f17a](0a4f17a)) ### Bug Fixes * **a2acompat/a2av0:** implement A2A v0.3 REST wire format ([#371](#371)) ([1ca80f9](1ca80f9)), closes [#370](#370) * **a2asrv:** skip TaskID mismatch check when message has no task reference (fixes [#350](#350)) ([#359](#359)) ([8c0dd99](8c0dd99)) * allow empty request bodies and enable GET method for task subscriptions ([#381](#381)) ([8363365](8363365)), closes [#380](#380) * **cli:** emit non-null required Agent Card list fields in synthesized card (fixes [#369](#369)) ([#372](#372)) ([0640869](0640869)) * **eventqueue:** add JSON marshal/unmarshal to Message so events survive roundtrip (fixes [#349](#349)) ([#360](#360)) ([d52d5a1](d52d5a1)) * itk grpc compatibility for v0.3 SDKs ([#367](#367)) ([11340a7](11340a7)) * ListTaskshistoryLength missing ([8b91364](8b91364)) * ListTaskshistoryLength missing (issue [#355](#355)) ([#361](#361)) ([8b91364](8b91364)) * **taskstore,push:** enforce cross-tenant authorization on Get and push config stores ([#357](#357)) ([a9f9c64](a9f9c64)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Serob Nahapetyan <serob@google.com>
What
Add custom
MarshalJSON/UnmarshalJSONtoeventqueue.Messageso that itsEventfield (ana2a.Eventinterface) correctly survives a JSON roundtrip.Why
eventqueue.Messageis the core message type flowing through the event queue betweenAgentExecutorand the A2A server stack. When this type is serialized to JSON (e.g., for persistence, external event buses, or debugging), theEventfield — which is ana2a.Eventinterface — cannot be deserialized by Go's defaultencoding/json.Fix
Delegate to
a2a.StreamResponsefor theEventfield, reusing the same polymorphic JSON wrapper pattern (eventstruct withomitemptyfields for each concrete type) already established ina2a/core.go.Test
Added
queue_test.gowith roundtrip tests covering all four event types:TaskStatusUpdateEventTaskArtifactUpdateEventMessageTaskAll 38 packages pass with
-race.Closes #349