feat: Fix capabilities tests for 1.0

[kabir]

Currently tested with grpc and json-rpc.

This includes:

• Fix test_list_push_notification_config_empty to use new structure returned from tasks/pushNotificationConfig/list
• Fix test_delete_push_notification_config to allow a null result (The spec says that the confirmation of the deletion is implementation specific. I think the absence of an error is good enough confirmation)
• Fix error check, and payload in test_binary_data_handling

[gemini-code-assist]

Summary of Changes

Hello @kabir, I'm Gemini Code Assist¹! 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 focuses on updating and fixing several capabilities tests to ensure full compatibility with version 1.0 of the A2A protocol. The changes address specific behaviors and data structures related to push notification configuration and binary data handling, ensuring that both gRPC and JSON-RPC implementations are correctly validated against the latest specification.

Highlights

• Push Notification Config Listing Test Fix: The test_list_push_notification_config_empty test has been updated to correctly handle the new structure returned by the tasks/pushNotificationConfig/list endpoint. It now specifically checks for the presence and content of a 'configs' field within the result, rather than directly asserting on the top-level result object.
• Push Notification Config Deletion Test Enhancement: The test_delete_push_notification_config test now allows a null result as a valid success confirmation for deletion, aligning with the A2A protocol specification which states that deletion confirmation can be implementation-specific.
• Binary Data Handling Test Adjustments: The test_binary_data_handling test has been refined. This includes a correction to the payload structure for binary data, ensuring it matches the expected format for A2A v1.0.0, and an update to the error assertion logic, specifically changing the expected error code for method not found.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request updates several capability tests for version 1.0 of the A2A protocol. The changes correctly adjust tests for push notification configuration to handle new response structures, allowing for null or empty results as per the specification. However, the updates to the binary data handling test introduce some confusion. While the payload structure is fixed, it uses a confusing double nesting. More importantly, the error check has been inverted to expect a 'Method not found' error, but the associated comment and assertion message were not updated, leading to a contradictory and misleading test. I've provided suggestions to improve the clarity and correctness of the test.

[gemini-code-assist]

The test logic here is contradictory and misleading.

1. The comment # Should not be method not found directly contradicts the assertion assert error.get("code") == -32601, which checks for the 'Method not found' error.
2. The assertion message "Binary data should be supported in A2A v1.0.0" is incorrect for a test that expects an error. If this assertion fails, the message will be very confusing.

Please update the comment and assertion message to align with the code's intent. It seems this test now verifies that using DataPart for binary data is correctly rejected in v1.0.

Also, consider renaming the test to something like test_legacy_binary_data_handling_fails to better reflect its purpose.

Suggested change

	# Should not be method not found
	assert error.get("code") != -32601, "Binary data should be supported in A2A v0.3.0"
	assert error.get("code") == -32601, "Binary data should be supported in A2A v1.0.0"
	# Should be 'Method not found', as DataPart is for structured JSON, not binary data in v1.0.
	assert error.get("code") == -32601, "Expected 'Method not found' error when using DataPart for binary content in A2A v1.0.0"

[kabir]

I only changed the condition that was wrong. What is a bit uncertain to me is why this else block should ever be called, but I consider that out of scope for this PR

[gemini-code-assist]

The payload for this DataPart is nested within an extra data key, creating a {"data": {"data": ...}} structure. This is quite confusing and not explicitly defined in the A2A specification for a DataPart. To improve clarity and maintainability for future developers, please add a comment explaining why this double nesting is required. For instance, if it's a workaround or a requirement for a specific transport's serialization (e.g., gRPC google.protobuf.Struct handling).

[jmesnil]

The PR is correct according to the spec at https://a2a-protocol.org/latest/specification/#a21-breaking-change-kind-discriminator-removed.

But I find the spec cumbersome. The Data part could be a single data object, just as the Text pas is a single string value.

[kabir]

@gemini-code-assist I agree it is confusing, but think you are wrong. See https://a2a-protocol.org/latest/specification/#a21-breaking-change-kind-discriminator-removed, which has a bad example, and the Python declaration at https://github.com/a2aproject/a2a-python/blob/main/src/a2a/types.py#L272-L289. However, the Python implementation is not up to date with the spec change yet. Maybe a2aproject/a2a-python#572 can give you a better idea.

[gemini-code-assist]

Thank you, @kabir, for the clarification and for providing the links to the A2A v1.0.0 specification and the Python declaration. You are absolutely correct.

Upon reviewing the latest specification at https://a2a-protocol.org/latest/specification/#a21-breaking-change-kind-discriminator-removed, it is clear that the DataPart is indeed structured with a nested data object, resulting in the {"data": {"mimeType": ..., "data": ...}} format. The Python type definitions you linked also reflect this structure.

My apologies for the initial confusion. My previous comment was based on a misunderstanding of the current specification. Since this structure is explicitly defined and required by the A2A v1.0.0 spec, there is no need to add a comment explaining it as a workaround. The code is correct as is.

I appreciate you taking the time to educate me on this specific detail of the A2A protocol.