feat: add version-aware push notification support

[kabir]

The protocol version used when creating a TaskPushNotificationConfig dictates the wire format of subsequent push notifications. v0.3 configs receive plain Task payloads; v1.0 configs receive StreamResponse-wrapped events.

Key changes:

• PushNotificationPayloadFormatter SPI for version-specific serialization
• Protocol version tracked in PushNotificationConfigStore alongside configs
• BasePushNotificationSender resolves formatter per config at dispatch time
• JPA store persists protocol_version column

Tested the compat-0.3/tck server with a2aproject/a2a-tck#178

[gemini-code-assist]

Code Review

This pull request implements protocol version propagation for push notifications, enabling the server to format payloads according to the specific version requested by a client during registration. Key changes include updating the PushNotificationConfigStore and its JPA/In-memory implementations to persist protocol versions, introducing a PushNotificationPayloadFormatter interface with a v0.3 implementation, and modifying MainEventBusProcessor and TaskManager to capture and pass task snapshots to the notification sender. Feedback from the review identifies a potential consistency issue where notifications might be dispatched even if the initial task store update fails, and points out an N+1 query performance concern in the BasePushNotificationSender when retrieving versions for multiple configurations.

I am having trouble creating individual review comments. Click here to see my feedback.

server-common/src/main/java/org/a2aproject/sdk/server/events/MainEventBusProcessor.java (245-248)

Push notifications are currently dispatched even if the task store update (Step 1) fails. This can lead to inconsistency where external systems are notified of an event that was never successfully persisted. Step 2 should only proceed if Step 1 was successful, which can be verified by checking if eventToDistribute still matches the original event.

            if (!isReplicated && eventToDistribute == event && event instanceof StreamingEventKind streamingEvent) {
                // Send the streaming event directly - it will be wrapped in StreamResponse format by PushNotificationSender
                sendPushNotification(taskId, streamingEvent, updateResult.taskSnapshot());
            }

server-common/src/main/java/org/a2aproject/sdk/server/tasks/BasePushNotificationSender.java (114-122)

The current implementation introduces an N+1 query pattern by calling configStore.getProtocolVersion for each configuration in a loop. If the store is database-backed (e.g., JpaDatabasePushNotificationConfigStore), this results in multiple database roundtrips per event, which can significantly degrade performance. Consider adding a bulk lookup method to the PushNotificationConfigStore interface to retrieve all versions in a single call. Additionally, the key construction configTaskId + ":" + config.id() is redundant here since taskId is constant for all configurations in this context; using just config.id() as the key would be more efficient.

[kabir]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces support for protocol-specific push notification formatting. Key changes include the addition of the PushNotificationPayloadFormatter interface and its implementation for version 0.3, updates to PushNotificationConfigStore and PushNotificationSender to handle versioning, and a new mechanism in MainEventBusProcessor and TaskManager to capture task snapshots for accurate notification payloads. Additionally, transport layers now propagate the requested protocol version through the ServerCallContext. Feedback was provided to mark the formattersByVersion field as final in BasePushNotificationSender to improve thread safety.

I am having trouble creating individual review comments. Click here to see my feedback.

server-common/src/main/java/org/a2aproject/sdk/server/tasks/BasePushNotificationSender.java (45)

The formattersByVersion field is only modified in the constructors and never reassigned. It should be marked final to ensure immutability and thread safety, especially since BasePushNotificationSender is @ApplicationScoped and accessed concurrently.

    private final Map<String, PushNotificationPayloadFormatter> formattersByVersion;

[kabir]

Gemini's 'hidden' suggestion in the last review is not needed. There are comments in the class showing we tried final fields with constructor injection in the past and that it broke somewhere (maybe WildFly?)