fix: set preferred transport on agent cards#1689
Conversation
Signed-off-by: Gnani Rahul <gnutakki@radiantlogic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Sets PreferredTransport on controller-generated A2A agent cards to maintain a populated transport field and adds regression coverage to prevent future regressions (per #1684).
Changes:
- Populate
PreferredTransportwith"JSONRPC"inGetA2AAgentCard. - Add a regression assertion in
TestGetA2AAgentCardto verifyPreferredTransportis set.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| go/core/internal/controller/translator/agent/utils.go | Sets PreferredTransport during agent card construction. |
| go/core/internal/controller/translator/agent/utils_test.go | Adds regression assertion verifying PreferredTransport remains populated. |
Comments suppressed due to low confidence (1)
go/core/internal/controller/translator/agent/utils.go:1
PreferredTransportis being set via a newly introduced magic string ("JSONRPC"). To reduce drift/typos and make future updates easier, consider defining a package-level constant (or reusing an existing shared constant/enum if one exists in the codebase) and referencing that instead of repeating a raw string literal.
package agent
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if assert.NotNil(t, card.PreferredTransport) { | ||
| assert.Equal(t, "JSONRPC", *card.PreferredTransport) | ||
| } |
There was a problem hiding this comment.
This pattern works, but it’s more idiomatic and provides clearer failure behavior to use require.NotNil(t, card.PreferredTransport) and then assert the value on the next line (no conditional needed). That keeps the test intent straightforward and avoids silently skipping the value assertion after a nil failure.
There was a problem hiding this comment.
Good call. I tightened this in the follow-up push: the test now uses require.NotNil(t, card.PreferredTransport) and then asserts the JSON-RPC value on the next line.
Signed-off-by: Gnani Rahul <gnutakki@radiantlogic.com>
|
I pushed a follow-up commit after checking the failing CI job more closely. The What changed in the follow-up:
Local verification on the rebased branch:
|
Summary
PreferredTransporttoJSONRPCwhen the controller builds A2A agent cardsGetA2AAgentCardso controller-generated cards keep that field populatedTesting
/opt/homebrew/bin/go test ./core/internal/controller/translator/agent -run TestGetA2AAgentCardFixes #1684