fix(tck): resolve capability test failures for push notifications#804
fix(tck): resolve capability test failures for push notifications#804sherryfox wants to merge 1 commit intoa2aproject:0.3.x-compatfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates Kafka to version 4.2.0, enhances the deployment scripts with a PostgreSQL pod creation wait loop, and refactors JSON-RPC handling to improve validation and spec compliance. It introduces check() methods for request validation and a custom TypeAdapterFactory to ensure JSON-RPC 2.0 compliance for responses. Feedback was provided regarding the inconsistent validation of the mandatory jsonrpc field in streaming requests and potential precision loss when serializing numeric IDs.
2c46514 to
bd65c70
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements push notification configuration storage within DefaultRequestHandler and adds a null-safety check for extensions in the AgentCapabilities record. Review feedback identifies a potential NullPointerException risk when taskId is null due to its use as a key in a ConcurrentHashMap. Additionally, it is recommended to use List.copyOf in the AgentCapabilities constructor to ensure the list is unmodifiable and defensively copied.
11a025e to
b45fa88
Compare
…tifications - Default `extensions` to an empty list in `AgentCapabilities` to ensure it serializes as an array, fixing `test_capabilities_structure`. - Save `pushNotificationConfig` in `onMessageSend` for non-streaming messages, fixing `test_send_message_with_push_notification_config`
b45fa88 to
2919258
Compare
|
Thank you @sherryfox! Just want to check with @jmesnil since he's more up to speed with the current state of the TCK than I am |
pushNotificationConfiginonMessageSendfor non-streaming messages, fixingtest_send_message_with_push_notification_configFailures before the fixes:
After the fixes all tests are passing: