Repository navigation
a2a: the regenerated agent card declares no extended card - #814
Open
asivanadi0 wants to merge 2 commits into
Open
asivanadi0 wants to merge 2 commits into
asivanadi0 wants to merge 2 commits into
Conversation
The caller's node regenerates the public agent card, but GetExtendedAgentCard (JSON-RPC) and GET .../extendedAgentCard (HTTP+JSON) are proxied untouched, so the extended card a client gets through the mesh names the agent's private interfaces. A client that follows A2A 1.0 section 3.1.11 replaces its card with that one and dials the agent directly, skipping the caller's node. regenerateAgentCardForMesh now sets capabilities.extendedAgentCard to false, and the JS and Python SDK rewrites drop it the same way. Stock clients then do not ask for the extended card: a2a-go's Client.GetExtendedAgentCard returns ErrExtendedCardNotConfigured without sending anything. This adds no work to the egress proxy hot path. A client that calls the method anyway is still proxied as before; refusing that on the wire for JSON-RPC needs the request body read that regenerating the extended card would also need. TestA2ACUJ gives the agent an extended card with private URLs and checks the regenerated card declares none and the stock client never fetches it. The node and SDK card tests cover the flag. Fixes google#675 Signed-off-by: Anpoo Sivanadi <sivanadi08@gmail.com>
Signed-off-by: Anpoo Sivanadi <sivanadi08@gmail.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request ensures that regenerated agent cards do not declare an extended agent card, preventing clients from attempting to connect directly to private interface URLs. This change is implemented across the Go backend, JS and Python SDKs, integration tests, and documentation. The review feedback correctly identifies two instances in the Go unit and integration tests where assertion messages violate Rule 3.8 of the style guide by failing to output both the observed and expected values.
Comment on lines
+225
to
+227
| if card.Capabilities.ExtendedAgentCard { | ||
| t.Error("extendedAgentCard must be off in the regenerated card: the extended card is relayed with the agent's own URLs") | ||
| } |
Contributor
There was a problem hiding this comment.
Rule 3.8 Violation\n\nAccording to the Repository Style Guide (Section 3, Rule 8), test assertion messages must state both the observed and the expected value. The current assertion message only states that the capability must be off, without printing the actual observed value.
\tif card.Capabilities.ExtendedAgentCard {\n\t\tt.Errorf(\"extendedAgentCard = %v, want false: the extended card is relayed with the agent's own URLs\", card.Capabilities.ExtendedAgentCard)\n\t}References
- Tests must be plain Go on the standard testing package and assertion messages must state both the observed and the expected value. (link)
Comment on lines
+230
to
+232
| if extendedCardServed.Load() { | ||
| t.Error("the agent served its extended card through the mesh") | ||
| } |
Contributor
There was a problem hiding this comment.
Rule 3.8 Violation\n\nAccording to the Repository Style Guide (Section 3, Rule 8), test assertion messages must state both the observed and the expected value. The current assertion message does not print the observed value of extendedCardServed.Load().
\tif extendedCardServed.Load() {\n\t\tt.Errorf(\"extendedCardServed = %v, want false: the agent served its extended card through the mesh\", extendedCardServed.Load())\n\t}References
- Tests must be plain Go on the standard testing package and assertion messages must state both the observed and the expected value. (link)
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #675
The caller's node regenerates the public agent card, but
GetExtendedAgentCard(JSON-RPC) andGET .../extendedAgentCard(HTTP+JSON) are proxied as they are, so the extended card that comes back names the agent's private interfaces. A client that follows A2A 1.0 section 3.1.11 swaps its card for that one and dials the agent directly, skipping the caller's node.This is option 1 from the issue (refuse), which @vbcherepanov also preferred:
regenerateAgentCardForMeshsetscapabilities.extendedAgentCardto false, and the JS and Python SDK rewrites (rewriteAgentCard,rewrite_agent_card) drop it the same way, matching the node'somitemptyoutput. Stock clients then don't ask for the extended card: a2a-go'sClient.GetExtendedAgentCardreturnsErrExtendedCardNotConfiguredwithout sending anything (v2.6.0,a2aclient/client.go:212). Nothing changes on the egress proxy hot path. Everything else on the card, streaming included, stays as declared. The two doc pages that describe card regeneration mention the exception.Tradeoff: callers lose the extended card through the mesh, and a client that calls the method anyway is still proxied as before. Blocking that call on the wire is easy for HTTP+JSON (the path is enough) but needs a read of the JSON-RPC request body, which is the same cost as option 2 (regenerating the extended card's
result). I kept both out to avoid adding a body read to every a2a POST. If you'd rather have option 2, or just the HTTP+JSON block on top of this, I can do it as a follow-up.Tests:
TestA2ACUJ: the agent now declaresextendedAgentCard: trueand serves an extended card withhttp://127.0.0.1:7777and a gRPC interface. The test checks that the regenerated card declares none, that the stock client'sGetExtendedAgentCardreturnsErrExtendedCardNotConfigured, and that the agent never served it. Without the node change it fails with the agent's private URLs, as in the issue.TestA2AServeAgentCardRegenerateschecks the flag is off.rewriteAgentCard declares no extended card) and Python (test_rewrite_agent_card_declares_no_extended_card) unit tests.go build ./...,go test -race ./internal/node/,TestA2ACUJ,TestNativeSDK*,make sdk-python, the JS SDK tests, build and typecheck, andgolangci-linton the touched Go packages pass on the branch with currentmainmerged in.