Skip to content

Conversation

@ehsavoie
Copy link
Collaborator

@ehsavoie ehsavoie commented Nov 24, 2025

[Feat]: Use protobuf as the serialization mechanism

Fixes #<issue_number_goes_here> 🦕

@ehsavoie ehsavoie requested review from fjuma and kabir November 24, 2025 14:41
@ehsavoie ehsavoie force-pushed the updating_payloads branch 2 times, most recently from fb75b4d to 729bc88 Compare November 24, 2025 14:51
@fjuma
Copy link
Collaborator

fjuma commented Nov 24, 2025

It might be good to add some info in the PR description or commit message to indicate the commit hash that was used when copying the .proto file to this repo.

@ehsavoie ehsavoie force-pushed the updating_payloads branch 3 times, most recently from 431152c to e879e8c Compare November 24, 2025 15:54
ehsavoie and others added 3 commits November 25, 2025 09:51
- a2a.proto from commit b0784199543eebf2e95dcb02e9336cb213923506
- use "Dskip.protobuf.generate=false" to generate source code from
  a2a.proto

Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
The spec-grpc classes are ugly, and we can't let users use them.
When using MapStruct an effort has been made to not use 'manual'
conversion (i.e. a 'default' implementation of the Mapper interface methods).

When automatic conversion happens, when the spec-grpc and spec classes differ
a compile error will be raised. Which in turn helps us keep our manually
generated spec classes in sync with the spec-grpc ones.
@ehsavoie ehsavoie force-pushed the updating_payloads branch 3 times, most recently from cd1eaf6 to 6a2441f Compare December 1, 2025 21:01
@ehsavoie
Copy link
Collaborator Author

ehsavoie commented Dec 2, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a significant and valuable refactoring to align the codebase with the latest protobuf specification. Key changes include the simplification of the AgentCard data model by replacing url, preferredTransport, and additionalInterfaces with a unified supportedInterfaces list. This greatly improves clarity. Another major improvement is the refactoring of the JSON-RPC transport to use protobuf data models internally, which brings consistency across all transport implementations (gRPC, REST, and JSON-RPC) and reduces code duplication. The build process for protobuf generation has also been modernized. Overall, these changes enhance maintainability and consistency. I have a couple of suggestions to further improve readability by using modern Java language features.


public JSONRPCTransport(AgentCard agentCard) {
this(null, agentCard, agentCard.url(), null);
this(null, agentCard, agentCard.supportedInterfaces().get(0).url(), null);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can supportedInterfaces() be empty?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kabir ^^

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://a2a-protocol.org/latest/definitions/, it can be empty:

message AgentCard {
  [...]
  // Ordered list of supported interfaces. First entry is preferred.
  repeated AgentInterface supported_interfaces = 19;
}

repeated means 0 or more

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ehsavoie I guess if null/empty we should throw some kind of exception

/**
* Converts domain DataPart to proto DataPart.
* Uses CommonFieldMapper for Map → Struct conversion.
* Metadata is ignored (not part of proto definition).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does that mean that there can't be metadata attached to a data part?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kabir maybe you can answer there

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it seems that the DataPartmessage is protobuf does not have metadata according to https://github.com/a2aproject/A2A/blob/b6ef9eec558c877fb69024df090a8bb63c542a1c/specification/grpc/a2a.proto#L228C9-L228C17.
The spec does not mention them either: https://a2a-protocol.org/latest/specification/#418-datapart

So is this something that was present in a previous spec but is no longer relevant?
Now it seems that metadata can only be attached to artifacts and not to their parts.

Copy link
Collaborator

@kabir kabir Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ehsavoie @jmesnil Yeah, I remember seeing some issues and discussions about metadata being present in the other protocols but not gRPC.
Maybe we can open a follow-up issue to clean that up? If you agree, please link the issue here so we track it (and add the P0 label)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd make sense to me to be able to attach metadata to parts (eg for a file, it would allow to put its creation/update date which would not make sense if it was at the artifact level)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created #497 to track this, and made it a P0 issue in https://github.com/orgs/a2aproject/projects/4. I don't think this needs to block the merge

@ehsavoie ehsavoie changed the title Updating to newest proto feat: Updating to newest proto to align with 1.0Draft specifications Dec 2, 2025
@ehsavoie ehsavoie changed the title feat: Updating to newest proto to align with 1.0Draft specifications feat!: Updating to newest proto to align with 1.0Draft specifications Dec 2, 2025
@ehsavoie ehsavoie force-pushed the updating_payloads branch 5 times, most recently from dee90e7 to e81d66f Compare December 3, 2025 08:26
@ehsavoie ehsavoie marked this pull request as ready for review December 3, 2025 09:35
* Update AgentCard and transport discovery. Removing the deprecated fields for now
* Added ProtoUtils.FromProto.agentCard() method to convert io.a2a.grpc.AgentCard to io.a2a.spec.AgentCard.
* Fixing the AGENT_CARD test constant to use the correct protobuf JSON format
* Updated ProtoUtils.ToProto and ProtoUtils.FromProto
* Update streaming test JSON to protobuf format
* Update SSE listener to use proto JSON instead of Jackson
* Updating and renaming all JSON-RPC method names to the new standard.
* Fixing NullSpecify
* Fixing Making unmarshalResponse type-safe
* Fixing cloud example
* Fixing streaming tests with http client
* Fixing issues with configId
* Fixing issues with missing id in jsonrpc requests.
* Add comprehensive tests for JSONRPCUtils
* Update protocol version to 1.0.0
* Unifying spec/grpc name clashes

Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
@ehsavoie ehsavoie changed the title feat!: Updating to newest proto to align with 1.0Draft specifications feat!: Updating to newest proto to align with 1.0 Draft specifications Dec 3, 2025
@kabir kabir merged commit cb084ec into a2aproject:main Dec 3, 2025
11 checks passed
@ehsavoie ehsavoie deleted the updating_payloads branch December 11, 2025 08:23
@jmesnil jmesnil added this to the 1.0.0 milestone Dec 11, 2025
kabir added a commit to kabir/a2a-java that referenced this pull request Dec 23, 2025
a2aproject#484)

[Feat]: Use protobuf as the serialization mechanism

Fixes #<issue_number_goes_here> 🦕

---------

Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
Co-authored-by: Kabir Khan <kkhan@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants