fix(json): honor rich %A2A.AgentCard{} fields in encode_agent_card/2#52
Open
zeroasterisk wants to merge 1 commit into
Open
fix(json): honor rich %A2A.AgentCard{} fields in encode_agent_card/2#52zeroasterisk wants to merge 1 commit into
zeroasterisk wants to merge 1 commit into
Conversation
encode_agent_card/2 was typed to accept A2A.AgentCard.t() but only read
name/description/version/skills from the card — capabilities, provider,
security_schemes, supported_interfaces, signatures, documentation_url and
icon_url were sourced exclusively from opts. Passing a fully-populated
%A2A.AgentCard{} silently dropped those fields.
Resolve each rich field from opts first (explicit override, backward
compatible), then fall back to the card struct/map. The A2A.AgentCard.t()
spec is now accurate: a populated struct round-trips correctly while every
existing opts-based caller (A2A.Plug, A2A.get_agent_card) is unchanged.
Adds tests for struct-sourced fields and opts-override precedence.
TCK 1.0-dev Compatibility Results (experimental)
|
Contributor
Author
|
Filed #53 for the broader follow-up: consolidating on |
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.
Split out from #41 per review — single-scope, independently mergeable.
What
encode_agent_card/2is typed to accept anA2A.AgentCard.t(), but it only readsname,description,versionandskillsfrom the card. Every other field —capabilities,provider,security_schemes,supported_interfaces,signatures,documentation_url,icon_url— was sourced exclusively fromopts. Passing a fully-populated%A2A.AgentCard{}silently dropped those values.Fix
Each rich field is now resolved opts-first (explicit override), then from the card struct/map, then the default. So:
%A2A.AgentCard{}round-trips correctly.opts-based caller (A2A.Plug,A2A.get_agent_card/2) is unchanged — opts still win.Backward compatible: all 119 existing
json_testcases pass untouched; 2 new tests cover struct-sourced fields and opts-override precedence.Verification
mix test— 496 tests + 2 doctests, 0 failuresmix format --check-formatted— cleanmix credo— no issuesmix dialyzer— 0 errorsNote on the one-line version
The minimal change discussed in #41 was just the spec line:
That alone wouldn't actually be correct: it advertises that the function consumes an
A2A.AgentCard.t(), but the body ignores all of the struct's rich fields (they come fromopts), so a caller passing a populated struct gets a card with those fields silently dropped — and dialyzer stays green either way, so it wouldn't catch the gap. This PR makes the code honor the type instead of just relabeling it. Happy to trim to the spec-only change if you'd prefer that scope — let me know which you'd rather merge.