feat(webhooks): adcp.webhooks.deliver() + A2A artifacts conformance#213
Merged
feat(webhooks): adcp.webhooks.deliver() + A2A artifacts conformance#213
Conversation
Closes #211. The SDK's A2A server already emits result.artifacts correctly across the success, list_scenarios, ControllerError, and unknown-scenario paths — this test drives the full A2A Starlette app through an ASGI transport and asserts the JSON-RPC wire shape so any future regression fires here before it surfaces in external storyboard validators. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes #212. Collapses the seller's six-step boilerplate (build envelope, serialize, sign, merge headers, POST, echo token) into one call so the signer and the wire see the *same bytes* — the serialization-format drift PR #205 fixed in the hand-rolled path is structurally impossible here. Covers the legacy AdCP 3.x authentication schemes (Bearer, HMAC-SHA256) and emits a one-shot DeprecationWarning pointing migrators at WebhookSender for RFC 9421. Missing or unknown authentication raises with a message that names the fix (use WebhookSender), not a silent unsigned POST. Token-echo is opt-in via ``token_field=`` — the AdCP spec says the token is "echoed back in the payload" but doesn't name the field, so the caller picks one the receiver agrees to read. Defense-in-depth at the helper boundary: * HTTPS-only URL; rejects embedded userinfo (getting logged by every HTTP intermediary is a footgun). * CRLF / NUL rejection on credentials + extra_headers (belt-and-braces over httpx's own header validation). * Reserved-header blocklist covers Authorization, Content-*, Host, Signature, Signature-Input, X-AdCP-*; each class gets a fix-hint tailored to the likely mistake. * 10MB body-size cap (shared with WebhookSender.send_raw for parity). * 64-entry extra_headers cap. * authentication must be a Mapping; schemes must be a list. Tests (22 for deliver + 1 for WebhookSender parity) cover: Bearer/HMAC happy paths, byte-identical signing-vs-wire invariant, retry byte-identity, token-echo opt-in shape (MCP top-level vs Task metadata), default-off echo, deprecation warning, and every boundary-validation failure mode. SKILL.md "Emitting Webhooks" section shows both the 4.0 default (WebhookSender) and the 3.x legacy (deliver) paths side-by-side with production notes (shared httpx.AsyncClient, egress transport, token_field coordination). Four expert reviews (code, protocol, DX, security) across three rounds. Deferred as follow-up: IP-pinned egress transport factory; upstream AdCP issue for 9421-vs-legacy precedence when both are on one config. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
…re form After rebasing onto main (which merged PR #213 adding deliver()), our compact-separator fix for get_adcp_signed_headers_for_webhook left deliver() self-inconsistent: it signed compact bytes via the signer but POSTed spaced bytes from an inline json.dumps(body_dict). The test_hmac_auth_signs_posted_bytes invariant on main explicitly verifies that HMAC(posted_bytes) matches the X-AdCP-Signature header — so the inconsistency failed CI. Fix --- - deliver() now serializes body_bytes with separators=(",", ":"), so signer input and transport bytes are byte-identical again. - test_signed_bytes_match_posted_bytes updated to expect compact bytes (was asserting against json.dumps(payload) default — main's test pre-dated adcp#2478's canonical-form pin). - Restore MemoryBackend + WebhookDedupStore re-exports from adcp.webhooks, dropped during the rebase. - Splice sign_legacy_webhook + _compute_legacy_signature back in alongside main's new deliver() — both pathways now share the same compact-separator serialization core. Verification: ruff + mypy clean, pytest 1798 passed / 17 skipped. All main-added tests (A2A comply_test_controller artifacts, test_webhooks_deliver full suite) pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Two issues closed after three expert-review rounds (code / protocol / DX / security).
tests/conformance/a2a/test_comply_test_controller_artifacts.pyso any regression fires here before it surfaces in external storyboard validators.adcp.webhooks.deliver(): one-shot legacy-auth dispatcher that collapses the seller's six-step boilerplate into one call. Signer and wire see the same bytes — the serialization-format drift PR chore(dx): pre-4.0 skill + SDK DX sweep #205 fixed in the hand-rolled path is structurally impossible here.What
deliver()doesurl, optionaltoken, optionalauthentication.{schemes, credentials}from aPushNotificationConfig/ReportingWebhook/ equivalent dict.Bearer→Authorization: Bearer ….HMAC-SHA256→X-AdCP-Signature+X-AdCP-Timestampover the exact POSTed bytes.authenticationraises pointing atWebhookSender(the AdCP 4.0 default); one-shotDeprecationWarningon first legacy-auth use.token_field=— the spec says "echoed in the payload" but doesn't name the field, so the caller picks one the receiver reads.Defense-in-depth
https://user:pass@host/gets logged by every intermediary).deliver()andWebhookSender.send_raw(parity).extra_headerscap.authenticationmust be a Mapping;schemesmust be a list.Docs
skills/build-seller-agent/SKILL.mdgets a new "Emitting Webhooks" section showing the 4.0 default (WebhookSender) and the 3.x legacy (deliver) paths side-by-side, plus production notes (sharedhttpx.AsyncClient, egress transport,token_fieldcoordination).Follow-ups
authenticationare present on aPushNotificationConfig. Thedeliver()/WebhookSendersplit in this PR doesn't need a decision first; unifying the two into a precedence-aware helper later does.httpx.AsyncTransportfactory for SSRF mitigation (~40 LOC + TOCTOU handling). The helper documents this as caller responsibility today.Test plan
ruff check src/— cleanmypy src/adcp/— 671 files, no issuespytest tests/ -q— 1724 passed / 13 skipped (22 newdelivertests + 4 A2A conformance + 1 WebhookSender body-cap)Expert-review rounds (summary)
timeout_seconds, missing config-error tests). All addressed.extra_headerscount cap,authenticationtype-check). All addressed.🤖 Generated with Claude Code