Conversation
…payload Agent Card canonicalization removed empty values with a type-blind pass over the MessageToDict output. This walks the AgentCard descriptor alongside the JSON, so the field each value belongs to is known at every level. No canonical bytes change. Nested messages, repeated fields and maps are pruned exactly as before, free-form google.protobuf values still go through _clean_empty, and the depth bound is kept. A differential run over 6000 randomly populated cards found no difference from the previous pruning. Refs a2aproject#1278
…nical payload
Section 8.4.1 of the A2A specification says a REQUIRED field stays in the canonical payload even when its value matches the default. The canonicalizer dropped it, so a card signed by an SDK that keeps description "" or skills [] in the signed payload failed verification here.
REQUIRED is read from google.api.field_behavior on the descriptors. REQUIRED strings are kept as "", REQUIRED repeated fields as [], REQUIRED maps and messages as {}. A REQUIRED message is present whether or not it was set, so the REQUIRED set does not depend on what the serializer emitted. Fields that are not REQUIRED are pruned as before.
This changes the canonical bytes of any card that has an empty REQUIRED field, including cards signed by earlier versions of this SDK. Cards with no empty REQUIRED field keep their bytes. The rule itself is under discussion in a2aproject/A2A#2122, and this commit is separate from the previous one so it can wait for that decision.
Fixes a2aproject#1278
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/cluster/database_event_stream.py | 96.94% | 92.86% | 🔴 -4.08% |
| src/a2a/utils/signing.py | 95.77% | 93.42% | 🔴 -2.35% |
| Total | 92.99% | 92.93% | 🔴 -0.05% |
Generated by coverage-comment.yml
665151d to
cdee28e
Compare
|
I measured both commits of this PR against the interop matrix from #1278 and against the signed corpus in a2aproject/a2a-tck#246. Other two SDKs as of today: 3f90712 (refactor): all 54 matrix cells are identical to the 1.2.1 baseline, and the corpus reading is unchanged. This agrees with "no canonical bytes change" on these cards. cdee28e (fix): 12 cells move, all in the three cases with an empty REQUIRED field:
So the expectation in the description holds on these cards. With the second commit, a card that has an empty REQUIRED field verifies between Python and Go, and stops verifying between Python and JS until the JS SDK canonicalizes the same way. Section 8.4.1 worked example: still not reproduced byte for byte. The only difference is that REQUIRED fields absent from the input are emitted at their default: The example's input omits those four fields and so does its output. Whether a REQUIRED field that was never set should appear in the signed bytes seems to belong with a2aproject/A2A#2122. It makes no difference on cards that carry every REQUIRED field, which is the case for the corpus and for the production card. Reproduce (from a checkout of ogasurfproject-jpg/horizon-shield at main, Use |
|
@ogasurfproject-jpg thank you, this is really useful. I reproduced the same One thing I checked after your comment: by the time verification runs, the card is already parsed into protobuf, and at that point That seems like the real question for a2aproject/A2A#2122. |
|
Confirmed on a2a-sdk 1.2.1. In the So on the Python public API, the open point in a2aproject/a2a-tck#245 is settled before the spec settles it. Of its two resolutions, That gives a2aproject/A2A#2122 a concrete form: does a verifier canonicalize the card as received, or the card as parsed? If parsed, the spec has to say that an absent REQUIRED field is signed at its default value, and the worked example changes. If received, the Python verifier needs the served bytes (or the dict before Nothing in a2a-card-sign-v01 (a2aproject/a2a-tck#246) depends on this: every vector there carries every REQUIRED field, so both resolutions give the same bytes on it. If it would help #2122, I can add a group where a REQUIRED field is absent, with one signature per resolution. |
|
Independent verification — heads of #1287 ( Method: pulled the frozen production card + JWKS at the pinned commit (hashes re-checked), the 13-vector corpus and the published The harness is anchored to a third-party run, not to my expectations: my pre-patch numbers reproduce the published Frozen production card (9420 /
Corpus (13 vectors / 6 cases) —
Complete leaf delta across the whole corpus — every added path is REQUIRED, nothing else moves So the four non-REQUIRED leaves from the Three things worth recording
Verdict from my side: commit 1 (descriptor walk + optional-default pruning) does what it claims and needs nothing from me; the open item is the reading switch you split into commit 2, which is exactly the #2122 dependency you flagged. Happy to run the same sweep against a two-reading/fail-open variant so #2122 has numbers for both options. |
|
Thank you, @kuangmi-bit. Your 13 lengths and hashes are the canonical bytes recorded in a2a-card-sign-v01 ( On the worked example, I agree with how you scope it. "Keep REQUIRED defaults only for fields the served JSON carried" is the presence-preserving resolution in a2aproject/a2a-tck#245. On the Python public API it needs the served bytes, because ParseDict erases presence for those fields (previous comment). A sweep against a two-reading variant would be useful for a2aproject/A2A#2122. The S1 pairs are built for it: each pair carries one signature per canonical form over the same card, so a verifier that tries both readings should accept both vectors of S1-001 to S1-006, and S1-007/S1-008 show what happens on a non-REQUIRED empty field. |
|
Addendum to the verification above — conformance view, now that #1286's corpus is public. Taking each corpus reading as a candidate resolution and scoring verifiers against it (independent ES256, candidates = rule-1-as-written vs prune-empty vs served-as-is):
So with #1287 applied, Python is fully conformant against the corpus under rule-1-as-written, and every leaf it adds is REQUIRED-only. Two things it does not settle, both now written up with numbers on #2122:
Detail + harness pointer: a2aproject/A2A#2122 (comment) |
|
Follow-up from the absent-field side, measured against cdee28e. When a REQUIRED field is absent from the served JSON, cdee28e emits it at its default before canonicalizing, so it rejects cards that a2a-sdk 1.2.1, @a2a-js/sdk 1.3.0 and a2a-go all accept today (S2-001, 003, 005, 007 and 009 in the corpus, now in a2aproject/a2a-tck#246). The section's worked example is reproduced only when rule 1 is scoped to the fields the JSON carries, and cdee28e does not accept it either. Numbers for all 24 vectors and a proposed scoping sentence are on a2aproject/A2A#2122. Commit 1 is unaffected; this is the reading choice in commit 2 again, now with the absent-field case measured. |
|
I ran both commits against all 24 vectors in a2aproject/a2a-tck#246 at
So commit 2 depends on the canonicalization-scope decision in a2aproject/A2A#2122. I can split the PR and leave that commit waiting on the ruling. Whether the behavior-neutral first commit is useful separately is up to you. |
|
Independent re-run of both commits against the 24 vectors in a2aproject/a2a-tck#246 at Method: each tree in its own process, Results:
For the split: commit 1 changes no bytes, so nothing downstream depends on when it lands, and it can merge on its own without touching interop. Commit 2's behavior is the descriptor reading, and a2aproject/A2A#2122 is heading for served scope, so commit 2 is the half that must wait for the ruling — or be rewritten if the ruling lands where the discussion currently points. The shape you propose is the right one. |
… decided This reverts commit cdee28e. The canonicalization scope is still open on a2aproject/A2A#2122. The branch keeps 3f90712, which changes no canonical bytes.
|
Following @kuangmi-bit's rerun, I reverted the second commit in |
Description
Part of #1278 🦕
This refactors canonical payload pruning to walk the AgentCard descriptors alongside the JSON values. It prepares the code for handling field requirements without introducing that behavior here.
The REQUIRED-default change has been reverted pending a2aproject/A2A#2122. The resulting tree is identical to
3f90712. No canonical-byte differences were found on the tested inputs.Evidence
a091c83is identical to the tree at3f90712.tests/utils/test_signing.py: 25 passed (22 before). The 3 new tests come from the first commit._clean_empty. The run is not part of the test suite.4568f93,3f90712is identical tofad0482in verdict and canonical bytes. @kuangmi-bit reproduced this independently.2df33ff1…) canonicalizes to 6410 bytes /c5d5384a…onfad0482and on the first commit.tests/integration/cross_versionon Python 3.10: 2169 passed in one of three runs. In the other two,tests/utils/test_telemetry.py::test_trace_function_sync_attribute_extractor_error_loggedfailed under-n 4. It passed in three runs on its own. This PR does not touch telemetry.ruff check,ruff format --checkandty checkpass on the changed module.Not in this PR
The REQUIRED default change. It waits for [Bug]: §8.4.1 canonicalization is under-determined - two SDKs produce different bytes for the same card, and neither reproduces the section's own worked example A2A#2122.
Optional fields with explicit presence that are set to their default (for example
iconUrl: "") are still pruned. That is a separate change with its own byte impact.Follow the
CONTRIBUTINGGuide.Make your Pull Request title in the Conventional Commits specification.
Ensure the tests and linter pass.
Appropriate docs were updated (not needed, no public API change).