Skip to content

feat(decisioning): F12 — auto-emit completion webhook on sync mutating responses#331

Merged
bokelley merged 2 commits intomainfrom
bokelley/decisioning-f12-auto-emit
May 1, 2026
Merged

feat(decisioning): F12 — auto-emit completion webhook on sync mutating responses#331
bokelley merged 2 commits intomainfrom
bokelley/decisioning-f12-auto-emit

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 1, 2026

Summary

First PR of the breadth sprint per the parity audit. Ports F12 from JS reference (commits 8dc427f9 and 7a887dfa on src/lib/server/decisioning/runtime/from-platform.ts).

The behavior change buyers see: when they pass push_notification_config.url on a sync mutating call (create_media_buy, update_media_buy, sync_creatives), the framework now auto-fires a completion webhook. Previously only the HITL TaskHandoff path emitted, so buyers got inconsistent notification depending on how the seller routed the call.

New module adcp.decisioning.webhook_emit:

  • SPEC_WEBHOOK_TASK_TYPES (closed 20-value set, drift-guarded against schemas/cache/enums/task-type.json)
  • maybe_emit_sync_completion (fire-and-forget gate)
  • _BACKGROUND_WEBHOOK_TASKS (strong-ref pin; mirrors dispatch._BACKGROUND_HANDOFF_TASKS)

Fire-and-forget posture (DoS defense): webhook delivery runs in a background asyncio.create_task; sync response returns inline. A buyer-supplied slowloris webhook URL cannot hold the seller's request worker. Same posture as JS round-2 fix.

No double-fire: _maybe_auto_emit_sync_completion detects the projected Submitted envelope (status == 'submitted' shape) and skips — the HITL path's registry completion emits its own webhook on terminal state.

Configuration on serve() / create_adcp_server_from_platform:

  • webhook_sender: WebhookSender | None = None — BYO emitter; None silently disables.
  • auto_emit_completion_webhooks: bool = True — default-on. Adopters who emit manually pass False.

Test plan

  • 21 new tests in tests/test_decisioning_webhook_emit.py covering: drift-guard, URL+token extraction, all skip branches, happy-path delivery, token echo, delivery-failure swallow, sync-success fires, TaskHandoff doesn't double-fire, opt-out, default-on, no-sender silent, sync_creatives fires.
  • pytest tests/ — 2204 passed, 15 skipped, 1 xfailed (was 2183 before).
  • ruff check clean
  • mypy src/adcp/decisioning/ clean

Release plan

Accumulates into the held release-please PR #328 alongside the foundation (#316), codemod ergonomics (#329), and parity rename + Tier 1 docs (#330). Foundation + F12 ship together once salesagent validates.

Next breadth-sprint PRs

Per the parity audit findings, in priority order:

  • 8 missing specialism Protocols (Creative, Audience, Signals, Governance, ContentStandards, BrandRights, PropertyLists/CollectionLists)
  • Helpers (batchPoll, validationError, upstreamError, RequestShape)
  • TenantRegistry + admin router

🤖 Generated with Claude Code

bokelley and others added 2 commits April 30, 2026 20:13
…g responses

Sync ``create_media_buy``, ``update_media_buy``, ``sync_creatives``
responses now auto-fire a completion webhook when the buyer supplied
``push_notification_config.url``. Previously only the HITL
TaskHandoff path emitted, so sync responses left buyers polling.

Mirrors the JS-side ``emitSyncCompletionWebhook`` implementation
(commits ``8dc427f9`` and ``7a887dfa`` on
``src/lib/server/decisioning/runtime/from-platform.ts``). Wire-format
is identical: ``task_type``, ``status: 'completed'``, ``result``
field carrying the projected sync response, echoed ``token`` via
``X-AdCP-Push-Token`` header. ``task_id`` is synthesized as
``f"sync-{uuid4()}"`` since sync responses don't allocate a registry
task; buyers correlate via the resource ids embedded in ``result``.

New module ``adcp.decisioning.webhook_emit``:

* ``SPEC_WEBHOOK_TASK_TYPES`` — closed 20-value set mirroring the
  on-disk spec enum at ``schemas/cache/enums/task-type.json``. The
  ``test_spec_webhook_task_types_matches_schema_cache`` test pins
  the constant so out-of-band drift surfaces in CI.
* ``maybe_emit_sync_completion`` — fire-and-forget gate. Skips when
  disabled, no sender wired, no push URL on the request, or the
  tool isn't in the spec enum (logged warning so adopters notice
  they extended the surface beyond spec).
* ``_BACKGROUND_WEBHOOK_TASKS`` — module-level strong-ref pin so the
  asyncio loop's weak-ref behavior doesn't garbage-collect in-flight
  emissions mid-flight. Mirrors the same pattern in
  ``dispatch._BACKGROUND_HANDOFF_TASKS``.

**Fire-and-forget posture (DoS defense).** Webhook delivery runs in a
background asyncio task; the sync response returns inline
immediately. A buyer-supplied slowloris webhook URL must not be able
to hold the seller's request worker for the full retry budget — the
JS round-2 fix at ``7a887dfa`` documented this DoS vector and Python
preserves the same posture from the start.

**TaskHandoff path doesn't double-fire.** The
``_maybe_auto_emit_sync_completion`` helper detects the projected
Submitted envelope (``status == 'submitted'`` shape) and skips
delivery. The HITL path's registry completion emits its own webhook
on terminal state.

Configuration on ``create_adcp_server_from_platform`` and ``serve``:

* ``webhook_sender: WebhookSender | None = None`` — BYO emitter.
  ``None`` silently disables auto-emit.
* ``auto_emit_completion_webhooks: bool = True`` — default-on.
  Adopters who emit webhooks manually inside their handlers pass
  ``False`` to avoid duplicate delivery.

21 new tests cover: drift-guard against the on-disk schema cache,
URL+token extraction (incl. dict-params test fixtures), gate skips
(disabled, no sender, no URL, tool outside spec enum, no running
loop), happy-path delivery via ``WebhookSender.send_mcp``, token
echo via ``X-AdCP-Push-Token`` header, delivery-failure swallow,
sync-success fires, TaskHandoff doesn't double-fire, opt-out
suppresses, default-on, no-sender silent, sync_creatives fires too.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n isolation

Two P0 expert-review findings on PR #331:

P0-1 (cross-language wire divergence): the buyer's
``push_notification_config.token`` was being echoed via
``X-AdCP-Push-Token`` HTTP header. Per
``schemas/cache/core/push_notification_config.json`` ("Echoed back in
webhook payload to validate request authenticity") AND the JS
reference impl (``buildTaskWebhookPayload`` in
``src/lib/server/decisioning/runtime/from-platform.ts``), the token
belongs on ``payload.token``. Buyers validating against the spec read
``body.token``, not custom headers — header echo would silently fail
their auth check.

Fix: extend ``create_mcp_webhook_payload`` and
``WebhookSender.send_mcp`` to accept ``token`` and write it onto the
payload. Update F12's ``_emit_sync_completion_webhook`` to pass
``token=`` through instead of building ``extra_headers``.
Cross-language wire-parity restored.

P0-2 (exception isolation): ``maybe_emit_sync_completion`` runs
AFTER the platform method's successful return. ANY exception in the
gate body — extraction quirk on a weird ``params`` shape,
``loop.create_task`` failure — would propagate to the handler shim
and lose the buyer's sync response.

Fix: wrap the entire gate body in ``try/except Exception``;
logged-and-swallowed. Last-line defense ensures the post-success
path can never poison the buyer's response.

P1 fixes folded in:

* Submitted-shape detection tightened to the EXACT 2-key dict
  ``{"task_id", "status"}`` (not the loose
  ``status == "submitted"`` predicate). An adopter who legitimately
  returns a sync ``{"status": "submitted", ...}`` with extra metadata
  (queue acceptance) now correctly gets the auto-emit fired.
* No-running-loop branch bumped from ``logger.debug`` to
  ``logger.warning`` — production code landing here is mis-wired and
  should be visible.

Round-2 tests added:

* ``test_handler_returns_before_webhook_delivers`` — pins the
  non-blocking invariant (sync response returns before webhook
  delivery completes).
* ``test_concurrent_emissions_dont_corrupt_strong_ref_set`` — 100
  concurrent emissions exercising the
  ``_BACKGROUND_WEBHOOK_TASKS`` add/discard pattern.
* ``test_handler_does_not_skip_loose_submitted_shape`` — pins the
  tightened submitted-shape detection.
* ``test_gate_swallows_unexpected_exceptions`` — pins the
  exception-isolation invariant via a sender that raises on
  attribute access.

25 F12 tests pass total (up from 21); 2208 total tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 9d29754 into main May 1, 2026
12 checks passed
bokelley added a commit that referenced this pull request May 1, 2026
…ionLists Protocols (breadth sprint Batch 4 — FINAL) (#335)

Final batch of the breadth-sprint per the parity audit. Ports the
remaining four specialism Protocols from JS reference. With this
PR, every spec specialism slug except ``governance-aware-seller``
has a Protocol class + REQUIRED_METHODS coverage.

New Protocols:

* ``BrandRightsPlatform`` — covers ``brand-rights``. Identity
  discovery + licensing. Required (3, sync): ``get_brand_identity``,
  ``get_rights``, ``acquire_rights``. ``acquire_rights`` returns a
  3-arm discriminated success union (acquired / pending / rejected)
  — rejection-as-data for spec-defined GRANT rejection, AdcpError
  only for buyer-fixable REQUEST rejection. Async outcomes for the
  ``pending`` arm flow via ``push_notification_config`` webhook
  (NOT a polling tool).

* ``ContentStandardsPlatform`` — covers ``content-standards``.
  Brand safety policies, content adjacency rules, per-creative
  compliance verification. Required (6): list/get/create/update,
  ``calibrate_content``, ``validate_content_delivery``. Optional
  (2, analyzer reads): ``get_media_buy_artifacts``,
  ``get_creative_features``.

* ``PropertyListsPlatform`` + ``CollectionListsPlatform``
  (specialisms/lists.py) — covers ``property-lists`` and
  ``collection-lists``. Parallel CRUD shapes (5 methods each, all
  required) with token-issuance semantics: ``create_*`` returns a
  per-seller-scoped ``fetch_token``, ``delete_*`` revokes it.
  Compromise-driven revocation MUST trigger the delete path.

Required-method coverage in ``REQUIRED_METHODS_PER_SPECIALISM``:
``brand-rights`` (3), ``content-standards`` (6),
``property-lists`` (5), ``collection-lists`` (5).

Public re-exports added at ``adcp.decisioning.__all__``:
``BrandRightsPlatform``, ``ContentStandardsPlatform``,
``PropertyListsPlatform``, ``CollectionListsPlatform``.

Test coverage in ``tests/test_decisioning_specialisms.py`` (13 new
tests, 43 total in the file):

* ``runtime_checkable`` conformance per Protocol.
* ``validate_platform`` enforcement per slug — including a
  security-relevant test that ``property-lists`` REQUIRES
  ``delete_property_list`` (revocation path) so adopters can't ship
  list-publishing without revocation primitives.
* Contract pins per slug.
* **Breadth-sprint completeness pin**:
  ``test_every_spec_slug_except_governance_aware_seller_is_enforced``
  asserts that ``SPEC_SPECIALISM_ENUM - REQUIRED_METHODS.keys()``
  yields exactly ``{governance-aware-seller, signed-requests}`` —
  the two slugs unenforced by design (composition claim and
  deprecated-moved-to-universal respectively).

One existing dispatch test updated:
``test_validate_platform_warns_on_unenforced_spec_specialism``
swapped its canonical "spec-recognized but unenforced" example
from ``brand-rights`` (now enforced) to ``governance-aware-seller``
(the only remaining unenforced spec slug — by design).

**Breadth sprint COMPLETE.** All 8 missing specialism Protocols
from the parity audit are now ported. 9 PRs total accumulating in
the held release PR #328:

* #316 foundation
* #329 codemod ergonomics
* #330 parity rename + Tier 1 docs
* #331 F12 auto-emit
* #332 Signals + Audience (Batch 1)
* #333 Creative Builder + AdServer (Batch 2)
* #334 Campaign Governance (Batch 3)
* #335 Brand + Content + Lists (Batch 4 — this PR)

Ready for salesagent validation against editable install before
tagging 4.4.0.

2252 tests pass (up from 2239).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant