fix: preserve tenant and protocolVersion from AgentInterface in ClientBuilder#773
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the transport selection logic in ClientBuilder to work directly with AgentInterface objects, allowing for the preservation of metadata like tenant IDs and protocol versions. The findBestClientTransport method was made package-private to support a more comprehensive test suite, which now includes scenarios for both client and server transport preferences. A performance improvement was suggested to replace a nested loop with a map-based lookup when matching client and server transport protocols.
…tBuilder. Also added unit tests around this behavior. Fixes a2aproject#772
Lirons01
force-pushed
the
issue-772-tenant-not-preserved-in-client-builder
branch
from
7a695e9 to
32d1c89
Compare
| private Map<String, String> getServerPreferredTransports() throws A2AClientException { | ||
| Map<String, String> serverPreferredTransports = new LinkedHashMap<>(); | ||
| if(agentCard.supportedInterfaces() == null || agentCard.supportedInterfaces().isEmpty()) { | ||
| private Map<String, AgentInterface> getServerInterfacesMap() throws A2AClientException { |
There was a problem hiding this comment.
Can you add a note that only the fist found protocol binding is considered ?
| @@ -24,26 +23,71 @@ public class ClientBuilderTest { | |||
|
|
|||
| private AgentCard card = AgentCard.builder() | |||
There was a problem hiding this comment.
Maybe this whole agent cards building could be simplified in a helper method ?
3 participants
Description
ClientBuilder.findBestClientTransport()was creating a new AgentInterface with only protocolBinding and url, discarding tenant and protocolVersion from the matched interface. This broke the intended fallback behavior where the AgentCard's default tenant should be used when no request-level tenant is provided.The fix returns the actual matched AgentInterface from the AgentCard's supportedInterfaces list instead of creating a new one. In addition, I added some tests in order to validate some of this behavior. In order to have the tests actually test this thing, I had to make the
findBestClientTransport()method package-private. LMK if any of this needs edits or does not match the actual intended behavior and I can make appropriate updates!Fixes #772