Repository navigation
fix(signing)!: preserve Agent Card presence for the next major release - #1360
UncleSaurus wants to merge 4 commits into
Conversation
BREAKING CHANGE: Agent Card signing and verification preserve required fields, explicitly present optional defaults and extension JSON values. Affected cards must be re-signed with coordinated verifier rollout. Propose the next SDK major release with an explicit 2.0.0 release boundary. Required-field warning policy is unchanged. Related to a2aproject#1278 and a2aproject/A2A#2122; their broader discussions remain open.
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/routes/agent_card_routes.py | 85.00% | 85.71% | 🟢 +0.71% |
| src/a2a/utils/proto_utils.py | 95.97% | 96.00% | 🟢 +0.03% |
| src/a2a/utils/signing.py | 96.15% | 97.14% | 🟢 +0.99% |
| Total | 93.52% | 93.54% | 🟢 +0.02% |
Generated by coverage-comment.yml
Independent verification — a2a-python PR #1360Tested head Results
Finding:
|
chrikrah
left a comment
There was a problem hiding this comment.
@UncleSaurus nothing in the signing code blocks this for 2.0.0. On main a scope-less requirement such as {"schemes": {"key": {}}} never reaches the signed bytes, so deleting it keeps the signature valid.
$ .venv/bin/python -I probe/strip.py # sign, del signed.security_requirements[:], verify
# head d11c05d
card without its apiKey requirement: InvalidSignaturesError
# src/a2a/utils/signing.py from a2dbedd (merge base)
card without its apiKey requirement: signature VERIFIES
$ .venv/bin/python -m pytest -q -p no:cacheprovider tests/utils/test_signing_presence.py tests/utils/test_signing.py tests/utils/test_jcs.py tests/server/routes/test_agent_card_routes.py
223 passed, 6 warnings in 5.50s # d11c05d, CPython 3.10
15 failed, 208 passed, 6 warnings in 2.71s # signing.py from a2dbedd, your tests kept
# not run: the full suite, the JS SDK, a real CDN
non-blocking: #1029 bumps past patch with an empty Release-As: commit, which leaves nothing to undo. I'd drop the release-as pin.
non-blocking: keeping [{}] signs [{}, X] apart from [X], but chopmob-cloud found it is the last byte difference from JS-signed cards, and cross-SDK verification is why this waits for 2.0.0. Could that be pinned on a2aproject/A2A#2122 before merge?
As for 1.3.0 (a2aproject#1389), the release owner sets 2.0.0 with an empty commit that ends with a Release-As line, so no config override is left to remove.
|
Thanks @chopmob-cloud for testing this against the frozen card, and @chrikrah for the review and the probe. @chopmob-cloud, you have the mechanism right. On Whether an element that becomes empty should collapse is still open in a2aproject/A2A#2122, and a2aproject/a2a-tck#246 records both readings as S1-009 and S1-010. I agree this should follow that decision before it merges. I'm converting it to draft until #2122 settles, and I'll match whatever it decides. @chrikrah, agreed on the |
Agent Card signing currently drops required defaults and explicitly present optional defaults, and recursively removes valid JSON empties. This draft next-major proposal preserves field presence before the existing RFC 8785 canonicalizer so the public signer and verifier follow the published A2A 1.0.1 presence rules.
This intentionally changes some signed bytes. Following the major-release requirement on #1355, it proposes SDK 2.0.0, independently of the A2A wire-protocol version. It leaves the required-field warning policy unchanged and adds no legacy-signature retry or alternate canonicalization path. Cross-SDK agreement and the release window remain upstream decisions; this does not close #1278 or a2aproject/A2A#2122.
always-bump-patchstrategy selects a patch even for a breaking commit, so, as for 1.3.0 (chore: release 1.3.0 #1389), the release owner sets 2.0.0 with an emptyRelease-Ascommit. No release manifest, tag, or publication is advanced here.Validation:
./scripts/lint.sh,uv run pytest, anduv run pytest --cov=src --cov-report=term-missingpass on CPython 3.10.19: 2285 passed, 183 skipped, 3 xfailed, 1 xpassed, 93% coverage. Skips are missing Postgres/MySQL DSNs and SQLite row locking. A completed full-worktree Claude Opus 5.5 review approved headd11c05d, independently exercising presence-byte/signature oracles, valid-card controls, cache rollout orderings, protobuf 5.29.5/7.36.2, and release-please 17.6.0's real manifest/strategy selection. It also ran focused checks on Python 3.14.2. No other language SDK, real CDN, or publication was exercised; fresh repository CI remains required.Related to #1278, #1355, and a2aproject/A2A#2122.