feat(webhooks): receiver-side idempotency_key dedup on AsyncHandler#629
Merged
Conversation
bokelley
added a commit
that referenced
this pull request
Apr 19, 2026
…pe, dupe-activity hygiene Addresses code-reviewer, security-reviewer, and dx-expert feedback on PR #629. Security & correctness - Validate `idempotency_key` against the AdCP spec regex `^[A-Za-z0-9_.:-]{16,255}$` at the receiver. Keys failing the pattern (empty, too short, containing U+001F) are treated as missing rather than used to form a scoped key. - Reserve the scoped-key prefix `adcp\u001fwebhook\u001fv1\u001f...` so webhook dedup entries cannot collide with request-side idempotency entries when sharing an IdempotencyBackend. - `webhook_duplicate` activity drops `payload` to avoid re-logging potentially sensitive data on every retry; both `webhook_received` and `webhook_duplicate` now carry `idempotency_key` for correlation. Ergonomics - `WebhookMetadata.protocol: 'mcp' | 'a2a'` plumbed through the normalize path so handlers can branch on transport. - Missing-key warn now only fires for MCP (A2A lacks the field by protocol design) and the message is actionable: it names the spec regex, describes the fallback behavior, and links to docs. - JSDoc on `handlers` field points at the dedup section. - Docs: activity double-fire guidance, migration-from-ad-hoc dedup, A2A/invalid-key behavior. Tests - Concurrent race: five parallel retries produce exactly one handler call, four `webhook_duplicate` activities. - Handler exception does NOT release the claim (documents the at-most-once contract; rebuts release-on-error suggestion). - Invalid key (too short, contains separator byte) treated as missing. - A2A without key does NOT warn. - `webhook_duplicate` activity omits payload, includes key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Addressed review feedback in 3913076. Summary: Accepted
Rejected with rationale
Test suite: 4084 pass / 0 fail. New tests bring dedup coverage to 12 cases. |
bokelley
added a commit
to adcontextprotocol/adcp
that referenced
this pull request
Apr 19, 2026
…onformance Closes the conformance gap for outbound webhook behavior that storyboards cannot currently assert (#2426): - idempotency_key presence on every webhook payload (#2417) - idempotency_key byte-identical across retries of the same logical event - RFC 9421 webhook signature validity when advertised (#2423) Storyboards are unidirectional today (runner → agent). Verifying outbound webhooks requires the runner to host a receiver during test execution and observe live deliveries. This PR adds the spec-side surface. Clean seam: the runner does NOT reimplement signature verification or idempotency dedup. It delegates to @adcp/client primitives (AsyncHandlerConfig.webhookDedup, WebhookMetadata.idempotency_key, Activity.type == "webhook_duplicate") so the same code that production receivers rely on is what the conformance runner exercises. Referenced by URL in test-kits/webhook-callbacks-runner.yaml so the spec and client-lib PR (adcontextprotocol/adcp-client#629) cross-reference cleanly. Schema additions: - New specialism enum value "webhook-callbacks" - New specialisms/webhook-callbacks/index.yaml (preview status, four phases: capability discovery, idempotency_key presence, idempotency_key stability, signature validity) - New test-kits/webhook-callbacks-runner.yaml (endpoint modes: loopback_mock default, proxy_url for conformance; per-step receiver URLs; 5xx-then-2xx retry-replay shape) - New step types in storyboard-schema.yaml: expect_webhook, expect_webhook_retry_keys_stable, expect_webhook_signature_valid, plus substitution variables {{runner.webhook_base}} and {{runner.webhook_url:<step_id>}}, cross-specialism helpers (expect_max_deliveries_per_logical_event, requires_contract) Closing a long-standing gap in universal/idempotency.yaml: - The replay-side-effect invariant ("no duplicate webhooks on replay") was previously graded by a manual-audit step. Replaced with a programmatic expect_webhook assertion using expect_max_deliveries_per_logical_event: 1 gated on the webhook_callbacks_runner contract. Runners without a webhook receiver skip as not_applicable. Preview status pending @adcp/client runner implementation tracked at #2426. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
bokelley
added a commit
that referenced
this pull request
Apr 19, 2026
…pe, dupe-activity hygiene Addresses code-reviewer, security-reviewer, and dx-expert feedback on PR #629. Security & correctness - Validate `idempotency_key` against the AdCP spec regex `^[A-Za-z0-9_.:-]{16,255}$` at the receiver. Keys failing the pattern (empty, too short, containing U+001F) are treated as missing rather than used to form a scoped key. - Reserve the scoped-key prefix `adcp\u001fwebhook\u001fv1\u001f...` so webhook dedup entries cannot collide with request-side idempotency entries when sharing an IdempotencyBackend. - `webhook_duplicate` activity drops `payload` to avoid re-logging potentially sensitive data on every retry; both `webhook_received` and `webhook_duplicate` now carry `idempotency_key` for correlation. Ergonomics - `WebhookMetadata.protocol: 'mcp' | 'a2a'` plumbed through the normalize path so handlers can branch on transport. - Missing-key warn now only fires for MCP (A2A lacks the field by protocol design) and the message is actionable: it names the spec regex, describes the fallback behavior, and links to docs. - JSDoc on `handlers` field points at the dedup section. - Docs: activity double-fire guidance, migration-from-ad-hoc dedup, A2A/invalid-key behavior. Tests - Concurrent race: five parallel retries produce exactly one handler call, four `webhook_duplicate` activities. - Handler exception does NOT release the claim (documents the at-most-once contract; rebuts release-on-error suggestion). - Invalid key (too short, contains separator byte) treated as missing. - A2A without key does NOT warn. - `webhook_duplicate` activity omits payload, includes key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3913076 to
2294bee
Compare
AdCP 3.0 (#2417 upstream) requires idempotency_key on every webhook
payload. Plumb the field through the client, add a drop-in dedup
layer for the MCP envelope path, and harden against common footguns
raised in review (spec-regex validation, reserved scope prefix,
dup-activity payload hygiene, A2A protocol awareness).
Additions
- AsyncHandlerConfig.webhookDedup?: { backend, ttlSeconds? } reusing
IdempotencyBackend from @adcp/client/server. Per-agent scoping
matches the spec's "authenticated sender identity" rule; the
reserved key prefix `adcp\u001fwebhook\u001fv1\u001f...` makes
backend sharing with request-side idempotency collision-proof.
- WebhookMetadata.idempotency_key and WebhookMetadata.protocol
surfaced to all typed handlers so application code can log,
trace, or branch on transport.
- Activity union gains 'webhook_duplicate'; both webhook_received
and webhook_duplicate carry idempotency_key for correlation. The
duplicate event intentionally omits `payload` to avoid re-logging
potentially-sensitive data on every retry.
- Schemas synced (latest): idempotency_key now required on
MCPWebhookPayload, CollectionListChangedWebhook,
PropertyListChangedWebhook, ArtifactWebhookPayload,
RevocationNotification.
Behavior
- putIfAbsent closes the concurrent-retry race: five simultaneous
retries of the same key produce exactly one handler dispatch
and four webhook_duplicate activities.
- Invalid idempotency_key (empty, too short, contains U+001F) is
rejected by the spec regex `^[A-Za-z0-9_.:-]{16,255}$` and
treated as missing rather than forming a scoped key from
arbitrary sender bytes.
- Missing-key warn only fires for MCP; A2A webhooks (which do not
carry the field by protocol design) dispatch silently.
- Handler exceptions are caught and logged as today; the claim
is intentionally NOT released on handler error to preserve
at-most-once handler execution.
Governance list-change / artifact / revocation webhooks are not
currently routed through AsyncHandler; dedup for those payload
types is a follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
f5af730 to
f5ddda4
Compare
bokelley
added a commit
to adcontextprotocol/adcp
that referenced
this pull request
Apr 20, 2026
…onformance (#2431) * spec(compliance): webhook-callbacks specialism for outbound webhook conformance Closes the conformance gap for outbound webhook behavior that storyboards cannot currently assert (#2426): - idempotency_key presence on every webhook payload (#2417) - idempotency_key byte-identical across retries of the same logical event - RFC 9421 webhook signature validity when advertised (#2423) Storyboards are unidirectional today (runner → agent). Verifying outbound webhooks requires the runner to host a receiver during test execution and observe live deliveries. This PR adds the spec-side surface. Clean seam: the runner does NOT reimplement signature verification or idempotency dedup. It delegates to @adcp/client primitives (AsyncHandlerConfig.webhookDedup, WebhookMetadata.idempotency_key, Activity.type == "webhook_duplicate") so the same code that production receivers rely on is what the conformance runner exercises. Referenced by URL in test-kits/webhook-callbacks-runner.yaml so the spec and client-lib PR (adcontextprotocol/adcp-client#629) cross-reference cleanly. Schema additions: - New specialism enum value "webhook-callbacks" - New specialisms/webhook-callbacks/index.yaml (preview status, four phases: capability discovery, idempotency_key presence, idempotency_key stability, signature validity) - New test-kits/webhook-callbacks-runner.yaml (endpoint modes: loopback_mock default, proxy_url for conformance; per-step receiver URLs; 5xx-then-2xx retry-replay shape) - New step types in storyboard-schema.yaml: expect_webhook, expect_webhook_retry_keys_stable, expect_webhook_signature_valid, plus substitution variables {{runner.webhook_base}} and {{runner.webhook_url:<step_id>}}, cross-specialism helpers (expect_max_deliveries_per_logical_event, requires_contract) Closing a long-standing gap in universal/idempotency.yaml: - The replay-side-effect invariant ("no duplicate webhooks on replay") was previously graded by a manual-audit step. Replaced with a programmatic expect_webhook assertion using expect_max_deliveries_per_logical_event: 1 gated on the webhook_callbacks_runner contract. Runners without a webhook receiver skip as not_applicable. Preview status pending @adcp/client runner implementation tracked at #2426. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec(compliance): refactor webhook conformance to universal, not specialism Specialisms are opt-in capability claims (like signed-requests, where some agents verify signed requests and some don't). Webhook emission isn't like that — any agent that accepts push_notification_config on any operation MUST emit conformant webhooks in 3.0. There's no agent specializing in webhook callbacks; it's a cross-cutting requirement. Refactored: - specialisms/webhook-callbacks/index.yaml → universal/webhook-emission.yaml - test-kits/webhook-callbacks-runner.yaml → test-kits/webhook-receiver-runner.yaml (test-kit is renamed for clarity — it provides a webhook receiver; it doesn't belong to any specialism) - Test-kit applies_to now lists universals: webhook-emission (primary) and idempotency (no-duplicate-webhooks-on-replay assertion) - Dropped webhook-callbacks entry from specialism.json enum - Reframed universal/idempotency.yaml cross-reference from specialism to universal - Grading: universal grades not_applicable when the agent advertises no webhook-emitting operations, or when the runner doesn't host a webhook receiver. No more claim-to-participate; any agent that accepts push_notification_config is in scope automatically. Drive-by fix: push-notification-config.json description still said "adagents.json-published key" (stale from pre-#2423 second-pass). Updated to "brand.json agents[] jwks_uri" matching the spec text. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * spec(compliance): address 8 runner-implementation feedback items on webhook-emission From parallel runner work in adcp-client, eight spec gaps surfaced: 1. schema_ref overload on expect_webhook. Renamed to webhook_payload_schema_ref so it doesn't collide with the step-level schema_ref/response_schema_ref used for caller→agent request/response schemas on emitting steps. 2. operation_id "caller" ambiguity. Clarified that the runner mints the operation_id in the URL template; the agent MUST NOT mint its own and MUST echo the runner-supplied value back in the webhook payload. 3. Retry-stability + 9421 nonce gap. expect_webhook_retry_keys_stable now MUST verify the 9421 signature on EVERY delivery in the retry loop when 9421 is in effect, using the run-scoped (keyid, nonce) replay store. Catches publishers that stably reuse both idempotency_key (correct) and 9421 nonce (incorrect) — a class of bug that retry-stability alone would silently miss. 4. Cross-step replay store. The runner MUST share one (keyid, nonce) replay store across every webhook delivery in a storyboard run (across every expect_webhook_signature_valid and every retry under expect_webhook_retry_keys_stable). Per-step stores make cross-step nonce replay silently undetectable. 5. retry_trigger caps. Hard-capped retry_trigger.count at 10 and allowlisted retry_trigger.http_status to {429, 500, 502, 503, 504}. Prevents typo'd storyboards from turning runners into DoS amplifiers in proxy_url mode. 6. HTTPS required on proxy_url endpoint mode. loopback_mock is in-process (no TLS surface); proxy_url now explicitly requires https scheme, rejectable at load time. 7. shared_receiver deferred. Fan-in dedup semantics (filter matching across multiple emitters, retry-replay precedence, replay store scoping) are underspecified. shared_receiver is out of scope for v1; storyboard authors MUST use per-step receivers. Future fan-in storyboards will either flesh out shared_receiver or pick a different idiom. 8. Unresolved substitution behavior. Runners MUST grade the storyboard not_applicable (preflight) or the step failed (step-time) when a {{runner.*}} substitution can't be resolved. Shipping the literal {{...}} token on the wire is a runner-side conformance bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
Publisher-side webhook emission — the symmetric counterpart to PR #629's receiver dedup. One ctx.emitWebhook(url, payload, operation_id) call and the emitter handles: - RFC 9421 signing with fresh nonce per attempt (adcp#2423). - Stable idempotency_key per operation_id across retries (adcp#2417). - JSON serialized once with compact separators and posted byte-identically (adcp#2478) — closes the Python json.dumps-default-spacing trap. - Retry with exp backoff + jitter on 5xx/429. Terminal on 4xx and on 401 responses carrying WWW-Authenticate: Signature error="webhook_signature_*". - Pluggable WebhookIdempotencyKeyStore (default in-memory); swap in a durable backend for multi-replica publishers. - HMAC-SHA256 / Bearer fallback modes for legacy buyers. createAdcpServer integration: new webhooks? config option. When set, ctx.emitWebhook is populated on every handler's context — completion handlers post signed webhooks without constructing the signer, fetching, or tracking idempotency themselves. Tests: - 14 unit tests covering every delivery path (happy / retry / terminal / 401+Signature / cap / cross-call stability / HMAC + Bearer fallback / observability). - 3 full-stack E2E tests: createAdcpServer → ctx.emitWebhook → real HTTP → receiver captures → verifyWebhookSignature accepts. No mocks on the signer/verifier path. Closes the "haven't spun up an actual server" gap flagged during PR #631 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
Publisher-side webhook emission — the symmetric counterpart to PR #629's receiver dedup. One ctx.emitWebhook(url, payload, operation_id) call and the emitter handles: - RFC 9421 signing with fresh nonce per attempt (adcp#2423). - Stable idempotency_key per operation_id across retries (adcp#2417). - JSON serialized once with compact separators and posted byte-identically (adcp#2478) — closes the Python json.dumps-default-spacing trap. - Retry with exp backoff + jitter on 5xx/429. Terminal on 4xx and on 401 responses carrying WWW-Authenticate: Signature error="webhook_signature_*". - Pluggable WebhookIdempotencyKeyStore (default in-memory); swap in a durable backend for multi-replica publishers. - HMAC-SHA256 / Bearer fallback modes for legacy buyers. createAdcpServer integration: new webhooks? config option. When set, ctx.emitWebhook is populated on every handler's context — completion handlers post signed webhooks without constructing the signer, fetching, or tracking idempotency themselves. Tests: - 14 unit tests covering every delivery path (happy / retry / terminal / 401+Signature / cap / cross-call stability / HMAC + Bearer fallback / observability). - 3 full-stack E2E tests: createAdcpServer → ctx.emitWebhook → real HTTP → receiver captures → verifyWebhookSignature accepts. No mocks on the signer/verifier path. Closes the "haven't spun up an actual server" gap flagged during PR #631 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
…ation (#632) * feat(server): createWebhookEmitter + createAdcpServer integration Publisher-side webhook emission — the symmetric counterpart to PR #629's receiver dedup. One ctx.emitWebhook(url, payload, operation_id) call and the emitter handles: - RFC 9421 signing with fresh nonce per attempt (adcp#2423). - Stable idempotency_key per operation_id across retries (adcp#2417). - JSON serialized once with compact separators and posted byte-identically (adcp#2478) — closes the Python json.dumps-default-spacing trap. - Retry with exp backoff + jitter on 5xx/429. Terminal on 4xx and on 401 responses carrying WWW-Authenticate: Signature error="webhook_signature_*". - Pluggable WebhookIdempotencyKeyStore (default in-memory); swap in a durable backend for multi-replica publishers. - HMAC-SHA256 / Bearer fallback modes for legacy buyers. createAdcpServer integration: new webhooks? config option. When set, ctx.emitWebhook is populated on every handler's context — completion handlers post signed webhooks without constructing the signer, fetching, or tracking idempotency themselves. Tests: - 14 unit tests covering every delivery path (happy / retry / terminal / 401+Signature / cap / cross-call stability / HMAC + Bearer fallback / observability). - 3 full-stack E2E tests: createAdcpServer → ctx.emitWebhook → real HTTP → receiver captures → verifyWebhookSignature accepts. No mocks on the signer/verifier path. Closes the "haven't spun up an actual server" gap flagged during PR #631 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(scripts): agent-skill-storyboard dogfood harness Non-interactive Claude Code harness that answers "can an agent with our SDK + a build-*-agent skill actually stand up a server that passes a compliance storyboard?" Flow: 1. Pre-populates a tmp workspace with package.json pointing @adcp/client at the repo root (file:<repo>), runs npm install upfront so Claude never touches deps. 2. Invokes claude -p <prompt> --dangerously-skip-permissions with the skill content + target storyboard + fixed port embedded in the prompt. 3. After Claude exits cleanly, runs the start.sh it produced, waits for the port, invokes bin/adcp.js storyboard run <url> <id> --json, reports pass/fail, tears down the spawned server + workspace. Usage: npm run compliance:agent-skill -- \ --skill skills/build-seller-agent/SKILL.md \ --storyboard universal/idempotency Clean boundaries: Claude is a code-writer, harness is the orchestrator — keeps failure modes diagnosable (was it the skill guidance, the SDK, the grader, or the agent itself). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(scripts): agent-skill harness — --allow-http + diagnostic pass First live run surfaced two plumbing issues beyond the Claude prompt: 1. The compliance runner hard-refuses http:// URLs (production agents MUST terminate TLS). Since the harness always binds on loopback, it passes --allow-http explicitly. 2. The prompt's "verify the server starts, then kill it" step taught Claude to leave a process on port 4200 when the harness tried to start its own. Tightened the instruction to "do NOT run the server, not even to verify" and added defense-in-depth port-cleanup in the harness (lsof | kill -9) before bash start.sh. Validated end-to-end: Claude builds server.ts + start.sh, harness starts it, grader runs against it, storyboard reports real conformance results (1 partial pass on `idempotency` — ACCOUNT_NOT_FOUND and missing-key rejection are both skill-guidance gaps the build-seller-agent skill can be tightened against). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(skill): tighten build-seller-agent for compliance grader defaults First live agent-skill-storyboard harness run surfaced that the skill's resolveAccount example only handled { account_id } references — the idempotency conformance grader (and any other storyboard using the canonical { brand: { domain }, operator } shape) short-circuits with ACCOUNT_NOT_FOUND before the idempotency layer ever runs. That masked real conformance signal behind the wrong error code. Changes: 1. Example resolveAccount now handles BOTH AccountReference branches. Includes a dev-mode wildcard for brand+operator so compliance graders reach the handler, with an explicit note to replace with a real tenant lookup in production. 2. Idempotency section now calls out the ordering gotcha: resolveAccount runs BEFORE idempotency, so null-returning resolveAccount surfaces as ACCOUNT_NOT_FOUND rather than the missing-key/replay error the spec actually wants. Tells readers what to fix when their storyboard runs fail with unexpected error codes. Also: prettier-format the skill file (CI check). * chore(emitter): drop unused defaultRetries (CodeQL review) Retries are re-resolved per-emit inside `emit()` (to honor per-call overrides), so the factory-level `defaultRetries` was dead. CodeQL flagged it on PR review. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
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.
Summary
Closes the receiver-side half of AdCP #2417 in the client library. Every MCP webhook payload now carries a required
idempotency_key; this PR plumbs it through the pipeline and adds a drop-in dedup layer so at-least-once retries no longer trigger duplicate side effects in handler code.What's new
AsyncHandlerConfig.webhookDedup?: { backend, ttlSeconds? }— reusesIdempotencyBackendfrom@adcp/client/server, so the samememoryBackend()/pgBackend(...)used for request-side idempotency can back webhook dedup. Defaults to 24h.WebhookMetadata.idempotency_key?: string— extracted from the MCP envelope and passed to everyonXxxStatusChangehandler so application code can log, trace, or layer its own dedup.Activityunion gains'webhook_duplicate'— emitted viaonActivitywhen a repeat key is dropped. Typed handlers are NOT called for duplicates (no side effects).Behavior
webhook\u001f{agent_id}\u001f{idempotency_key}) — matches the spec's "scoped to authenticated sender identity" rule, keys from different senders never collide.putIfAbsentcloses the concurrent-retry race: when two retries race on the same fresh key, exactly one wins the claim and dispatches; the rest surface aswebhook_duplicate.idempotency_key(non-conforming senders, or A2A transport which doesn't carry the field) dispatch without dedup and log aconsole.warn. No silent drops, no tuple fallback.Schema sync
Pulled from AdCP
latest—idempotency_keynow required on:MCPWebhookPayloadCollectionListChangedWebhookPropertyListChangedWebhookArtifactWebhookPayloadRevocationNotification(brand rights — not previously flagged in #2417)Usage
Out of scope (follow-ups)
AsyncHandler; dedup for those payload types is a follow-up.@adcp/clientdoesn't currently emit webhooks for sellers. When added, it will need to generate oneidempotency_keyper event and reuse across retries.Changeset
minor— new optional config + new metadata field +Activityunion gains a new string literal. See.changeset/webhook-receiver-dedup.md.Test plan
npm run typecheck— cleannpm test— 4067 pass / 0 failtest/lib/async-handler-webhook-dedup.test.js:webhookDedupunsetidempotency_keypropagates from MCP envelope →WebhookMetadata→ handler