From 2c4ac265d94ce295ab84a78734e34126b6db4c79 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Thu, 13 Aug 2026 15:51:02 +0200 Subject: [PATCH] fix(decisioning): disable sync task webhooks by default --- MIGRATION_v6_to_v7.md | 20 +++++++ docs/handler-authoring.md | 32 +++++++---- examples/README.md | 2 +- examples/hello_seller.py | 8 ++- examples/hello_seller_async_handoff.py | 8 +-- examples/hello_seller_audience.py | 5 +- examples/hello_seller_brand_rights.py | 5 +- examples/hello_seller_catalog.py | 5 +- examples/hello_seller_creative.py | 9 ++-- examples/hello_seller_signals.py | 7 ++- examples/hello_seller_with_webhooks.py | 14 +++-- examples/v3_reference_seller/src/app.py | 18 +++---- scripts/run_emma_matrix.sh | 8 +-- src/adcp/decisioning/dispatch.py | 48 +++++++++++++---- src/adcp/decisioning/handler.py | 22 ++++---- src/adcp/decisioning/serve.py | 54 ++++++++++++------- src/adcp/decisioning/webhook_emit.py | 65 ++++++++++------------ src/adcp/testing/decisioning.py | 10 ++-- tests/test_decisioning_handler_shims.py | 10 ++-- tests/test_decisioning_serve.py | 39 +++++++++----- tests/test_decisioning_webhook_emit.py | 71 +++++++++++++++++++------ tests/test_proposal_lifecycle_e2e.py | 43 +++++++++++++++ tests/test_testing_decisioning.py | 10 ++-- 23 files changed, 339 insertions(+), 174 deletions(-) diff --git a/MIGRATION_v6_to_v7.md b/MIGRATION_v6_to_v7.md index fa190ceaf..8e09b8b77 100644 --- a/MIGRATION_v6_to_v7.md +++ b/MIGRATION_v6_to_v7.md @@ -44,3 +44,23 @@ AdCP 3.0 is upgraded on reads and downgraded on writes. AdCP 3.1 requires the `media_buy.features.canonical_creatives` capability or unambiguous request-local evidence. AdCP 3.2 will be canonical by contract once supported; advertising `canonical_creatives: false` there will be an error. + +## Synchronous completion webhooks + +`auto_emit_completion_webhooks` now defaults to `False`. AdCP forbids a task +webhook when the initial response is already terminal: the result is available +inline and no registry task exists for a webhook `task_id`. + +If an existing buyer depends on receiving both copies, temporarily pass +`auto_emit_completion_webhooks=True` to `serve()` or +`create_adcp_server_from_platform()`. This retains the former behavior as a +non-conformant compatibility extension with a synthetic, unpollable `sync-*` +task ID. Update the buyer to consume the inline result, then remove the opt-in. + +This setting only controls synthetic synchronous-completion delivery. Terminal +webhooks for real `TaskHandoff` requests remain enabled when the request supplies +`push_notification_config` and a webhook sender or supervisor is configured. The +framework rejects a push-configured handoff before task creation when no transport +is available, rather than returning `submitted` and silently dropping the callback. +Adopters that deliver terminal task webhooks themselves can set the independent +`auto_emit_task_webhooks=False` ownership flag. diff --git a/docs/handler-authoring.md b/docs/handler-authoring.md index d62315458..54c8430fc 100644 --- a/docs/handler-authoring.md +++ b/docs/handler-authoring.md @@ -1277,16 +1277,28 @@ MCP for production agents. ## Webhooks -When `auto_emit_completion_webhooks=True` (the default), the framework fires a -sync-completion webhook after every successfully-dispatched tool call whose task -type is in the spec's webhook-eligible set (`create_media_buy`, `activate_signal`, -and their siblings). Buyers who register `push_notification_config.url` receive -these notifications automatically. - -The framework requires a sender or supervisor at boot — it raises `AdcpError` -rather than silently dropping notifications if neither is wired and auto-emit is on. -Set `auto_emit_completion_webhooks=False` only if you emit webhooks manually inside -your platform methods. +Synchronous terminal responses do not emit task webhooks. This is the default: +`auto_emit_completion_webhooks=False`. The buyer already has the result inline, +and no registry task exists for a webhook `task_id`. + +`TaskHandoff` is different. When the initial response is `submitted` and the +request includes `push_notification_config`, the framework delivers the required +terminal completion or failure webhook through the configured sender or supervisor. +The sync-completion compatibility flag does not disable that async delivery. A +push-configured handoff is rejected before task creation when no delivery transport +is configured, so the server cannot return `submitted` and then silently drop the +required callback. + +`auto_emit_task_webhooks=True` controls framework ownership of these real task +notifications. Set it to `False` only when adopter code owns terminal webhook +delivery itself. This is separate from `auto_emit_completion_webhooks`, which only +controls the legacy synthetic sync behavior. + +Existing integrations that relied on duplicate inline and webhook delivery can +temporarily set `auto_emit_completion_webhooks=True`. This is a non-conformant +compatibility extension: it synthesizes an unpollable `sync-*` task ID. The +framework requires a sender or supervisor at boot when this mode is enabled. +Migrate buyers to consume the inline terminal response, then remove the opt-in. ### Sender constructors diff --git a/examples/README.md b/examples/README.md index 258693764..1e9367387 100644 --- a/examples/README.md +++ b/examples/README.md @@ -83,7 +83,7 @@ Related skills: `build-seller-agent`, `build-generative-seller-agent`, ## Webhooks -- [`hello_seller_with_webhooks.py`](hello_seller_with_webhooks.py) — wire an `InMemoryWebhookDeliverySupervisor` so completion webhooks reach buyers who register `push_notification_config.url`; uses `WebhookSender.from_bearer_token`. +- [`hello_seller_with_webhooks.py`](hello_seller_with_webhooks.py) — legacy, non-conformant sync-completion compatibility example using `InMemoryWebhookDeliverySupervisor`; normal `TaskHandoff` notifications require no sync opt-in. See `docs/handler-authoring.md#webhooks` for the full `WebhookSender` constructor comparison (bearer vs RFC 9421 JWK signing). diff --git a/examples/hello_seller.py b/examples/hello_seller.py index f0c8eb62c..6e5f1131d 100644 --- a/examples/hello_seller.py +++ b/examples/hello_seller.py @@ -373,10 +373,8 @@ def _get_packages(req: Any) -> list[dict[str, Any]]: # server. Default port 3001 over streamable-http; override via # ``serve(seller, port=...)``. # - # ``auto_emit_completion_webhooks=False`` opts out here because this - # example has no signing key. Production sellers want webhooks on so - # buyers who register ``push_notification_config.url`` get sync- - # completion notifications. Pick a constructor and pass + # Synchronous terminal responses do not emit task webhooks. For real + # TaskHandoff completion notifications, pick a constructor and pass # ``webhook_supervisor=`` (retry + circuit breaker, recommended) or # ``webhook_sender=`` (transport only): # @@ -399,4 +397,4 @@ def _get_packages(req: Any) -> list[dict[str, Any]]: # serve(HelloSeller(), name="hello-seller", webhook_supervisor=supervisor) # # See docs/handler-authoring.md#webhooks for the full wiring recipe. - serve(HelloSeller(), name="hello-seller", auto_emit_completion_webhooks=False) + serve(HelloSeller(), name="hello-seller") diff --git a/examples/hello_seller_async_handoff.py b/examples/hello_seller_async_handoff.py index df012be78..d81252a2c 100644 --- a/examples/hello_seller_async_handoff.py +++ b/examples/hello_seller_async_handoff.py @@ -288,9 +288,9 @@ def _echo_packages(req: Any) -> list[dict[str, Any]]: serve( HelloSellerHybrid(), name="hello-seller-hybrid", - # Opt out of F12 auto-emit so the example boots without a - # ``webhook_sender``. Production sellers wire ``webhook_sender=`` - # so buyers who register ``push_notification_config.url`` get - # completion notifications when their TaskHandoff finishes. + # Pin the conformant sync-completion default explicitly. + # Production sellers wire ``webhook_sender=`` so buyers who + # register ``push_notification_config.url`` get completion + # notifications when their TaskHandoff finishes. auto_emit_completion_webhooks=False, ) diff --git a/examples/hello_seller_audience.py b/examples/hello_seller_audience.py index 620064602..426e2c941 100644 --- a/examples/hello_seller_audience.py +++ b/examples/hello_seller_audience.py @@ -64,9 +64,8 @@ def sync_audiences( def main() -> None: """Boot the seller on http://localhost:3001/mcp. - ``auto_emit_completion_webhooks=False`` opts out so this example - boots without a ``webhook_sender``. In production, wire - ``webhook_sender=`` for buyer notification. + Synchronous terminal responses remain inline-only by default. Wire + ``webhook_sender=`` when adding ``TaskHandoff`` support. """ serve(HelloAudienceSeller(), auto_emit_completion_webhooks=False) diff --git a/examples/hello_seller_brand_rights.py b/examples/hello_seller_brand_rights.py index d26b705f8..c7587d291 100644 --- a/examples/hello_seller_brand_rights.py +++ b/examples/hello_seller_brand_rights.py @@ -87,9 +87,8 @@ def acquire_rights( def main() -> None: """Boot the seller on http://localhost:3001/mcp. - ``auto_emit_completion_webhooks=False`` opts out so this example - boots without a ``webhook_sender``. In production, wire - ``webhook_sender=`` for buyer notification. + Synchronous terminal responses remain inline-only by default. Wire + ``webhook_sender=`` when adding ``TaskHandoff`` support. """ serve(HelloBrandRightsSeller(), auto_emit_completion_webhooks=False) diff --git a/examples/hello_seller_catalog.py b/examples/hello_seller_catalog.py index dc15db942..d97175b90 100644 --- a/examples/hello_seller_catalog.py +++ b/examples/hello_seller_catalog.py @@ -93,9 +93,8 @@ def sync_catalogs(self, req: Any, ctx: RequestContext[Any]) -> list[dict[str, An def main() -> None: """Boot the seller on http://localhost:3001/mcp. - ``auto_emit_completion_webhooks=False`` opts out so this example - boots without a ``webhook_sender``. In production, wire - ``webhook_sender=`` for buyer notification. + Synchronous terminal responses remain inline-only by default. Wire + ``webhook_sender=`` when adding ``TaskHandoff`` support. """ serve(HelloCatalogSeller(), auto_emit_completion_webhooks=False) diff --git a/examples/hello_seller_creative.py b/examples/hello_seller_creative.py index ad4943e81..9fbe737bb 100644 --- a/examples/hello_seller_creative.py +++ b/examples/hello_seller_creative.py @@ -102,16 +102,15 @@ def main() -> None: governance tools (per-specialism filter). * ``tools/call build_creative`` returns the synthesized manifest. - The ``auto_emit_completion_webhooks=False`` opt-out keeps this - example minimal. In production, wire ``webhook_sender=`` so - buyers who register ``push_notification_config.url`` get - completion notifications: + Synchronous terminal responses remain inline-only. If this seller + returns a ``TaskHandoff``, wire ``webhook_sender=`` so buyers who + register ``push_notification_config.url`` get terminal notifications: from adcp.webhook_sender import WebhookSender sender = WebhookSender.from_jwk(...) serve(HelloCreativeSeller(), webhook_sender=sender) """ - serve(HelloCreativeSeller(), auto_emit_completion_webhooks=False) + serve(HelloCreativeSeller()) if __name__ == "__main__": diff --git a/examples/hello_seller_signals.py b/examples/hello_seller_signals.py index 7c72ed459..3245d509a 100644 --- a/examples/hello_seller_signals.py +++ b/examples/hello_seller_signals.py @@ -137,14 +137,13 @@ async def _async_activation(self, task_ctx: Any) -> dict[str, Any]: def main() -> None: """Boot the seller on http://localhost:3001/mcp. - ``auto_emit_completion_webhooks=False`` opts out of the sync - completion-webhook auto-emit so this example boots without a - ``webhook_sender``. In production, wire ``webhook_sender=`` so + Synchronous terminal responses remain inline-only by default. In + production, wire ``webhook_sender=`` so buyers who register ``push_notification_config.url`` on ``activate_signal`` get notifications when a TaskHandoff completes. """ - serve(HelloSignalsSeller(), auto_emit_completion_webhooks=False) + serve(HelloSignalsSeller()) if __name__ == "__main__": diff --git a/examples/hello_seller_with_webhooks.py b/examples/hello_seller_with_webhooks.py index b83d1beb8..0ad007123 100644 --- a/examples/hello_seller_with_webhooks.py +++ b/examples/hello_seller_with_webhooks.py @@ -1,17 +1,20 @@ """Hello-seller-with-webhooks — canonical ``WebhookSender`` + supervisor wiring. Extends ``hello_seller.py`` with a wired :class:`InMemoryWebhookDeliverySupervisor` -so sync-completion webhooks are delivered to buyers who register -``push_notification_config.url``. Uses :meth:`WebhookSender.from_bearer_token` -as the auth mode — no key management, simplest first step. +and explicitly enables the legacy sync-completion compatibility mode. Uses +:meth:`WebhookSender.from_bearer_token` as the auth mode — no key management, +simplest first step. Run:: WEBHOOK_BEARER_TOKEN= uv run python examples/hello_seller_with_webhooks.py -The server boots on http://localhost:3001/mcp. Any buyer that registers +The server boots on http://localhost:3001/mcp. Any buyer that registers ``push_notification_config.url`` on a ``create_media_buy`` request receives a -completion notification POSTed with ``Authorization: Bearer ``. +duplicate completion notification POSTed with ``Authorization: Bearer ``. +This behavior is non-conformant and exists only to migrate integrations that +depended on the SDK's former default. New integrations consume the inline +terminal response; normal async ``TaskHandoff`` notifications need no opt-in. To use RFC 9421 JWK signing instead (AdCP spec baseline, required for buyers that verify body signatures), swap :meth:`~WebhookSender.from_bearer_token` @@ -55,4 +58,5 @@ HelloSeller(), name="hello-seller-with-webhooks", webhook_supervisor=supervisor, + auto_emit_completion_webhooks=True, ) diff --git a/examples/v3_reference_seller/src/app.py b/examples/v3_reference_seller/src/app.py index 4b105119a..36e6fe46f 100644 --- a/examples/v3_reference_seller/src/app.py +++ b/examples/v3_reference_seller/src/app.py @@ -293,10 +293,9 @@ def main() -> None: # with ``mode='live'`` in their ``AccountStore.resolve`` and declare # :attr:`V3ReferenceSeller.upstream_url` to their production URL. # Wire the webhook supervisor iff signing material is present. When - # the env vars are unset, the seller falls back to the - # ``auto_emit_completion_webhooks=False`` posture below — a buyer - # registering ``push_notification_config.url`` will not receive - # auto-emitted completion webhooks, but boot succeeds without a key. + # the env vars are unset, a buyer registering + # ``push_notification_config.url`` cannot receive TaskHandoff terminal + # webhooks, but boot succeeds without a key. # The framework's #384 validator binds these two posture knobs # together: capabilities advertise signing iff the supervisor is # wired with an RFC 9421 key. @@ -390,14 +389,11 @@ def main() -> None: if debug_token is not None else None ), - # Auto-emit binds to the supervisor: when a webhook-signing PEM - # is wired via the ADCP_WEBHOOK_SIGNING_KEY_PATH env var, the - # supervisor signs every auto-emitted completion webhook per - # RFC 9421 and the seller advertises the matching capability. - # When unwired, auto-emit stays off so the F12 boot gate doesn't - # trip on the missing sender (no silent webhook drops). + # When a webhook-signing PEM is wired, the supervisor signs + # spec-required TaskHandoff terminal webhooks per RFC 9421 and + # the seller advertises the matching capability. Synchronous + # terminal responses remain inline-only. webhook_supervisor=webhook_supervisor, - auto_emit_completion_webhooks=webhook_supervisor is not None, # FastMCP's TransportSecurityMiddleware enforces DNS-rebinding # protection: its default ``allowed_hosts`` accepts only # loopback (``127.0.0.1:*``, ``localhost:*``, ``[::1]:*``), so diff --git a/scripts/run_emma_matrix.sh b/scripts/run_emma_matrix.sh index dc0255bd2..54bec731e 100755 --- a/scripts/run_emma_matrix.sh +++ b/scripts/run_emma_matrix.sh @@ -44,7 +44,7 @@ SDK at ${ADCP_SDK_PATH}. Setup: Build: 1. DecisioningPlatform subclass claiming \`sales-non-guaranteed\`. 2. Stub all 5 required sales methods with believable in-memory state. -3. CRITICAL: pass \`auto_emit_completion_webhooks=False\` to \`serve()\` (boot-time webhook gate from PR #339 requires this when no webhook_sender is wired). +3. Rely on the conformant \`auto_emit_completion_webhooks=False\` default; do not enable the legacy sync-completion compatibility mode. 4. Boot via \`from adcp.decisioning import serve\` and hit it with: tools/list, get_products, create_media_buy, sync_creatives, get_media_buy_delivery via the mcp client. 5. Verify tools/list shows ONLY sales tools (per-specialism filter from PR #339) — should NOT include build_creative, acquire_rights, check_governance, get_signals. @@ -85,7 +85,7 @@ Build: 2. get_signals returns a small catalog (3 signals: demographic, in-market, purchase-intent). 3. activate_signal sync-success arm. 4. CRITICAL: on a SECOND activate_signal call, return a TaskHandoff via ctx.handoff_to_task. Verify framework projects to {task_id, status:"submitted"}. -5. CRITICAL: pass \`auto_emit_completion_webhooks=False\` to \`serve()\`. +5. Rely on the conformant \`auto_emit_completion_webhooks=False\` default. 6. Boot, hit tools/list + get_signals + activate_signal (sync) + activate_signal (handoff). 7. Verify tools/list narrows to just signals tools (per-specialism filter from PR #339). @@ -122,7 +122,7 @@ SDK at ${ADCP_SDK_PATH}. Setup: Build: 1. DecisioningPlatform subclass claiming \`creative-generative\`. 2. build_creative wires AudioStack Generate API (mock the key, stub realistic responses). -3. CRITICAL: pass \`auto_emit_completion_webhooks=False\` to \`serve()\`. +3. Rely on the conformant \`auto_emit_completion_webhooks=False\` default. 4. Boot, hit tools/list and build_creative. 5. Verify tools/list narrows to creative tools only (no sales/signals/governance leaks). @@ -153,7 +153,7 @@ SDK at ${ADCP_SDK_PATH}. Setup: Build: 1. DecisioningPlatform subclass claiming \`creative-generative\`. 2. build_creative wires Stability /v2beta/stable-image/generate (mock key, realistic shape). -3. CRITICAL: pass \`auto_emit_completion_webhooks=False\` to \`serve()\`. +3. Rely on the conformant \`auto_emit_completion_webhooks=False\` default. 4. Test the SHORTHAND ergonomic arm: return a bare CreativeManifest (Pydantic model), NOT a fully-shaped BuildCreativeSuccessResponse. 5. CRITICAL: deliberately make a mistake on the first attempt (e.g., omit width/height on ImageContent). Verify the error response is FOCUSED (just ImageAsset.width / ImageAsset.height) — NOT the 60-line dump that 5/10 verdict reported pre-PR-#340. 6. Boot, hit tools/list and build_creative. diff --git a/src/adcp/decisioning/dispatch.py b/src/adcp/decisioning/dispatch.py index e7672d3eb..c86778d47 100644 --- a/src/adcp/decisioning/dispatch.py +++ b/src/adcp/decisioning/dispatch.py @@ -66,7 +66,11 @@ is_task_handoff, is_workflow_handoff, ) -from adcp.decisioning.webhook_emit import emit_terminal_completion_webhook +from adcp.decisioning.webhook_emit import ( + SPEC_WEBHOOK_TASK_TYPES, + _extract_push_notification_url_and_token, + emit_terminal_completion_webhook, +) if TYPE_CHECKING: from collections.abc import Awaitable, Callable @@ -1368,10 +1372,11 @@ async def _invoke_platform_method( (async) arm uses it — the sync arm's auto-emit is a separate call in the handler shim. - :param webhook_auto_emit: Forwarded to :func:`_project_handoff`; - mirrors the handler's ``auto_emit_completion_webhooks`` so an - adopter emitting webhooks manually never gets a framework - double-delivery on the handoff path. + :param webhook_auto_emit: Forwarded to :func:`_project_handoff` as + the async task-webhook delivery gate. Production handlers keep + this enabled because a submitted task with push configuration + requires a terminal webhook. Direct low-level callers may + disable it when they own that delivery themselves. :param pre_handoff_reject: Optional zero-arg callback invoked when the adapter returned a :class:`TaskHandoff`, BEFORE @@ -1705,16 +1710,41 @@ async def _project_handoff( ``None`` (and the no-push case) skips delivery — the buyer polls ``tasks/get`` instead. The framework's polling path is unchanged. - :param webhook_auto_emit: Mirrors the handler's - ``auto_emit_completion_webhooks`` flag. When ``False`` the - adopter emits webhooks manually inside their handler; the - framework skips the terminal emission so it never double-delivers. + :param webhook_auto_emit: Async task-webhook delivery gate. The + production handler defaults this to ``True`` independently of + the legacy sync-completion compatibility flag. Callers pass + ``False`` only when they own terminal task delivery. The handoff fn is extracted via the type-identity dispatch in :func:`adcp.decisioning.types.is_task_handoff`. Subclassed TaskHandoff instances (deliberate non-feature) silently take the sync-return path before reaching this function. """ + if ( + webhook_auto_emit + and method_name in SPEC_WEBHOOK_TASK_TYPES + and _extract_push_notification_url_and_token(request_params) is not None + and webhook_target is None + ): + rejection = AdcpError( + "INVALID_REQUEST", + message=( + "push_notification_config requires webhook_sender or " + "webhook_supervisor before this request can enter the " + "TaskHandoff lifecycle" + ), + recovery="correctable", + field="push_notification_config", + suggestion=( + "Configure webhook delivery, omit push_notification_config " + "and poll tasks/get, or set auto_emit_task_webhooks=False " + "only when adopter code owns terminal webhook delivery" + ), + ) + if on_failure is not None: + await _safe_on_failure_call(on_failure, rejection, method_name) + raise rejection + fn = handoff._fn # Extract the buyer's ``context`` extension from the original diff --git a/src/adcp/decisioning/handler.py b/src/adcp/decisioning/handler.py index d6428762c..e4ae2ef02 100644 --- a/src/adcp/decisioning/handler.py +++ b/src/adcp/decisioning/handler.py @@ -1289,7 +1289,8 @@ def __init__( resource_resolver: ResourceResolver | None = None, webhook_sender: WebhookSender | None = None, webhook_supervisor: WebhookDeliverySupervisor | None = None, - auto_emit_completion_webhooks: bool = True, + auto_emit_completion_webhooks: bool = False, + auto_emit_task_webhooks: bool = True, buyer_agent_registry: BuyerAgentRegistry | None = None, brand_authorization_gate: BrandAuthorizationGate | None = None, config_store: ProductConfigStore | None = None, @@ -1306,6 +1307,7 @@ def __init__( self._webhook_sender = webhook_sender self._webhook_supervisor = webhook_supervisor self._auto_emit_completion_webhooks = auto_emit_completion_webhooks + self._auto_emit_task_webhooks = auto_emit_task_webhooks self._buyer_agent_registry = buyer_agent_registry self._brand_authorization_gate = brand_authorization_gate self._config_store = config_store @@ -1480,7 +1482,7 @@ def _maybe_auto_emit_sync_completion( params: Any, result: Any, ) -> None: - """Fire the F12 sync-completion webhook if applicable. + """Fire the legacy sync-completion webhook if explicitly enabled. Skips TaskHandoff projections — on the async (handoff) arm the terminal completion / failure webhook is delivered from the @@ -1491,8 +1493,9 @@ def _maybe_auto_emit_sync_completion( when the buyer registered ``push_notification_config``. This gate fires on the sync-success arm only; skipping the submitted projection here is what keeps the two paths from double-delivering. - Mirrors the JS-side ``routeIfHandoff`` logic at - ``src/lib/server/decisioning/runtime/from-platform.ts``. + The sync gate defaults off because AdCP forbids synthesizing a + task webhook for an inline terminal response. Explicit opt-in is + retained only as a legacy, non-conformant compatibility mode. TaskHandoff projection returns the exact 2-key dict ``{"task_id": ..., "status": "submitted"}`` from ``_project_handoff``; we @@ -1530,14 +1533,15 @@ def _handoff_webhook_kwargs(self) -> dict[str, Any]: request hands off AND the buyer registered ``push_notification_config``, the background completion path delivers the terminal completion / failure webhook to that - target. The sync arm's auto-emit gate is wired separately via - :meth:`_maybe_auto_emit_sync_completion`; both honor the same - ``auto_emit_completion_webhooks`` flag so an adopter emitting - manually never gets a framework double-delivery on either arm. + target. The sync arm's legacy compatibility gate is wired + separately via :meth:`_maybe_auto_emit_sync_completion`. + ``TaskHandoff`` terminal delivery is required by AdCP and has a + separate ownership flag so adopters with manual delivery can + suppress framework sends without enabling legacy sync behavior. """ return { "webhook_target": self._webhook_supervisor or self._webhook_sender, - "webhook_auto_emit": self._auto_emit_completion_webhooks, + "webhook_auto_emit": self._auto_emit_task_webhooks, } def _build_ctx( diff --git a/src/adcp/decisioning/serve.py b/src/adcp/decisioning/serve.py index 05e4ac2c9..fda65d8b3 100644 --- a/src/adcp/decisioning/serve.py +++ b/src/adcp/decisioning/serve.py @@ -86,7 +86,8 @@ def create_adcp_server_from_platform( resource_resolver: ResourceResolver | None = None, webhook_sender: WebhookSender | None = None, webhook_supervisor: WebhookDeliverySupervisor | None = None, - auto_emit_completion_webhooks: bool = True, + auto_emit_completion_webhooks: bool = False, + auto_emit_task_webhooks: bool = True, buyer_agent_registry: BuyerAgentRegistry | None = None, brand_authz_resolver: BrandAuthorizationResolver | None = None, brand_identity_resolver: BrandIdentityResolver | None = None, @@ -177,17 +178,20 @@ def create_adcp_server_from_platform( — pre-trust beta adopters running existing key-based auth without commercial gating omit this and the dispatch path falls through to ``AccountStore.resolve`` unchanged. - :param auto_emit_completion_webhooks: F12 feature gate. When - ``True`` (default), the framework auto-fires a completion - webhook on the sync-success arm of mutating tools whenever the - request supplied ``push_notification_config.url`` AND the tool - is in :data:`adcp.decisioning.webhook_emit.SPEC_WEBHOOK_TASK_TYPES`. - Buyers passing the URL expect notification regardless of - whether the seller routed sync vs HITL. Set ``False`` for - adopters who emit webhooks manually inside their handlers - (avoid duplicate delivery; idempotency-key dedup at the - receiver would handle it but explicit suppression matches the - v5 manual-emit posture for adopters mid-migration). + :param auto_emit_completion_webhooks: Legacy compatibility gate for + sync-completion webhooks. Defaults to ``False`` because AdCP + forbids a task webhook when the initial response is already + terminal. Setting ``True`` preserves the former SDK behavior as + a non-conformant extension: the result is delivered both inline + and by webhook, with a synthetic ``task_id`` that cannot be read + through ``tasks/get``. Async ``TaskHandoff`` terminal webhooks are + spec-required and are not controlled by this flag. + :param auto_emit_task_webhooks: Framework ownership of terminal + webhooks for real ``TaskHandoff`` requests. Defaults to ``True``. + When a request supplies ``push_notification_config``, the + framework rejects the handoff before creating a task unless a + sender or supervisor is configured. Set ``False`` only when + adopter code owns required task-webhook delivery itself. :param media_buy_store: Opt-in :class:`adcp.decisioning.MediaBuyStore` wrapper that gates ``targeting_overlay`` echo on the seller's declared specialisms. Typically built via @@ -365,6 +369,7 @@ def create_adcp_server_from_platform( webhook_sender=webhook_sender, webhook_supervisor=webhook_supervisor, auto_emit_completion_webhooks=auto_emit_completion_webhooks, + auto_emit_task_webhooks=auto_emit_task_webhooks, buyer_agent_registry=buyer_agent_registry, brand_authorization_gate=brand_authorization_gate, config_store=config_store, @@ -384,7 +389,7 @@ def create_adcp_server_from_platform( fetcher=property_list_fetcher, ) - # F12 boot-time fail-fast (Emma sales-direct P0 root cause): if + # Legacy sync-completion compatibility boot-time fail-fast: if # the platform's claimed specialisms expose any spec-eligible # webhook task type (create_media_buy, activate_signal, etc.) AND # auto-emit is on AND no webhook_sender is wired, every buyer @@ -460,7 +465,8 @@ def serve( resource_resolver: ResourceResolver | None = None, webhook_sender: WebhookSender | None = None, webhook_supervisor: WebhookDeliverySupervisor | None = None, - auto_emit_completion_webhooks: bool = True, + auto_emit_completion_webhooks: bool = False, + auto_emit_task_webhooks: bool = True, buyer_agent_registry: BuyerAgentRegistry | None = None, brand_authz_resolver: BrandAuthorizationResolver | None = None, brand_identity_resolver: BrandIdentityResolver | None = None, @@ -495,7 +501,9 @@ def serve( terminal completion / failure notification on the async (handoff) path of any spec-eligible verb when the buyer registered ``push_notification_config``. Transport only — one attempt, no - retry. ``None`` disables emission silently. + retry. When framework-owned task-webhook delivery is enabled, + a push-configured ``TaskHandoff`` is rejected before submission + if neither a sender nor supervisor is configured. :param webhook_supervisor: BYO :class:`~adcp.webhook_supervisor.WebhookDeliverySupervisor` for reliable delivery (retry, circuit breaker, attempt audit). @@ -503,11 +511,16 @@ def serve( when both are passed. Production sellers typically pass an :class:`~adcp.webhook_supervisor.InMemoryWebhookDeliverySupervisor` wrapping their sender. - :param auto_emit_completion_webhooks: F12 — auto-fire a completion - webhook on the sync-success arm of mutating tools when the - request supplied ``push_notification_config.url``. Default - ``True``. Set ``False`` for adopters who emit webhooks - manually inside their handlers. + :param auto_emit_completion_webhooks: Legacy compatibility gate for + sync-completion webhooks. Defaults to ``False`` for AdCP + conformance. Set ``True`` only while migrating an integration + that relies on duplicate inline and webhook delivery; the + compatibility webhook uses an unpollable synthetic ``task_id``. + Async ``TaskHandoff`` terminal webhooks are unaffected. + :param auto_emit_task_webhooks: Framework ownership of required + terminal webhooks for real ``TaskHandoff`` requests. Defaults to + ``True``. Set ``False`` only when adopter code owns that delivery; + this is independent of the legacy sync-completion flag. :param mock_ad_server: Optional :class:`adcp.decisioning.MockAdServer` whose ``get_traffic()`` is wired into ``GET /_debug/traffic`` when ``enable_debug_endpoints=True``. Default ``None`` — @@ -579,6 +592,7 @@ def serve( webhook_sender=webhook_sender, webhook_supervisor=webhook_supervisor, auto_emit_completion_webhooks=auto_emit_completion_webhooks, + auto_emit_task_webhooks=auto_emit_task_webhooks, buyer_agent_registry=buyer_agent_registry, brand_authz_resolver=brand_authz_resolver, brand_identity_resolver=brand_identity_resolver, diff --git a/src/adcp/decisioning/webhook_emit.py b/src/adcp/decisioning/webhook_emit.py index 2ae5c63eb..8c3db5997 100644 --- a/src/adcp/decisioning/webhook_emit.py +++ b/src/adcp/decisioning/webhook_emit.py @@ -1,21 +1,16 @@ -"""Auto-emit completion webhook on sync-success arm of mutating tools. - -When a buyer supplies ``push_notification_config.url`` on a request and -the seller answers via the sync fast path (NOT a :class:`TaskHandoff`), -the framework fires a completion webhook to that URL after the response -so buyers get consistent notification regardless of how the seller -routed the call. Without this, a buyer registering a webhook URL would -get notifications only on the HITL path — sync responses would leave -them polling. - -Mirrors the JS-side ``emitSyncCompletionWebhook`` at -``src/lib/server/decisioning/runtime/from-platform.ts`` (commits -``8dc427f9`` and ``7a887dfa``). Wire-format is identical: same -``task_type``, ``status: 'completed'``, ``result`` field carrying the -projected sync response, and an echoed ``token`` if the buyer -registered one. ``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``. +"""Legacy sync-completion webhook compatibility support. + +AdCP task webhooks describe status changes after the initial response. +When that response is already terminal, the buyer has the result inline +and no task webhook is emitted. ``auto_emit_completion_webhooks`` +therefore defaults to ``False``. + +Setting the flag to ``True`` preserves the former SDK behavior as a +non-conformant compatibility extension. It duplicates the inline result +in a webhook and synthesizes ``task_id`` as ``f"sync-{uuid4()}"``. +Because no registry task exists for a synchronous response, that ID +cannot be read through ``tasks/get``; compatibility consumers must +correlate through resource IDs embedded in ``result``. **Fire-and-forget.** Webhook delivery runs in a background asyncio task; the sync response returns inline immediately. A buyer-supplied @@ -33,9 +28,8 @@ aren't in the spec enum (adopter-only specialism methods) skip delivery and rely on ``publishStatusChange`` for state updates. -Adopters who emit webhooks manually inside their handlers pass -``auto_emit_completion_webhooks=False`` to -:func:`adcp.decisioning.serve` to avoid duplicate delivery. +Async :class:`TaskHandoff` completion and failure webhooks are separate, +spec-required behavior and are not disabled by this compatibility flag. """ from __future__ import annotations @@ -215,7 +209,7 @@ def maybe_emit_sync_completion( Skips silently when: - * ``enabled`` is False (operator opted out). + * ``enabled`` is False (the conformant default). * The request didn't carry ``push_notification_config.url``. Logs a WARNING when: @@ -223,9 +217,9 @@ def maybe_emit_sync_completion( * ``sender`` is None but the buyer DID register ``push_notification_config.url`` — the buyer's notification registration is being silently dropped, which the adopter - almost certainly didn't intend. Wire ``webhook_sender`` into - :func:`adcp.decisioning.serve` or pass - ``auto_emit_completion_webhooks=False`` to silence this. + explicitly requested compatibility delivery but did not configure + its transport. Wire ``webhook_sender`` into + :func:`adcp.decisioning.serve` or disable the compatibility flag. * ``method_name`` isn't in :data:`SPEC_WEBHOOK_TASK_TYPES` (the adopter extended the tool surface beyond the spec enum). @@ -281,7 +275,7 @@ def maybe_emit_sync_completion( "has neither webhook_sender nor webhook_supervisor — " "webhook silently dropped. Pass one to " "adcp.decisioning.serve.create_adcp_server_from_platform, " - "or set auto_emit_completion_webhooks=False to silence " + "or disable auto_emit_completion_webhooks to silence " "this warning.", url_for_log if url_for_log else "", method_name, @@ -384,8 +378,7 @@ async def emit_terminal_completion_webhook( Skips silently when: - * ``enabled`` is False (operator opted out via - ``auto_emit_completion_webhooks=False`` — they emit manually). + * ``enabled`` is False (a low-level caller owns task delivery). * ``method_name`` isn't in :data:`SPEC_WEBHOOK_TASK_TYPES`. This gate runs FIRST, before any target check. SDK-internal, non-spec task types (e.g. ``finalize_proposal``, an interception @@ -453,8 +446,7 @@ async def emit_terminal_completion_webhook( "(url=%s) for async %s (task_id=%s) but neither webhook_sender " "nor webhook_supervisor is wired — terminal %s webhook silently " "dropped. Pass one to " - "adcp.decisioning.serve.create_adcp_server_from_platform, or set " - "auto_emit_completion_webhooks=False to silence this warning.", + "adcp.decisioning.serve.create_adcp_server_from_platform.", url_for_log if url_for_log else "", method_name, task_id, @@ -508,10 +500,10 @@ def validate_webhook_sender_for_platform( auto_emit: bool, supervisor: Any = None, ) -> None: - """Server-boot fail-fast for the F12 misconfig (Emma sales-direct - P0 root cause). + """Validate explicit legacy sync-completion compatibility wiring. - When an adopter claims a specialism whose tool surface includes + When an adopter explicitly enables compatibility mode and claims a + specialism whose tool surface includes any spec-eligible webhook task type (e.g., ``create_media_buy``, ``activate_signal``, ``acquire_rights``) AND auto-emit is on AND neither ``webhook_sender`` nor ``webhook_supervisor`` is wired, @@ -545,7 +537,8 @@ def validate_webhook_sender_for_platform( raise AdcpError( "INVALID_REQUEST", message=( - "auto_emit_completion_webhooks is enabled and the platform's " + "legacy auto_emit_completion_webhooks compatibility mode is " + "enabled and the platform's " "claimed specialisms expose webhook-eligible tools " f"{sorted(eligible)!r}, but neither webhook_sender nor " "webhook_supervisor was wired. Buyers who register " @@ -554,8 +547,8 @@ def validate_webhook_sender_for_platform( "WebhookSender (transport only) or InMemoryWebhookDeliverySupervisor " "(retry + circuit breaker) to " "adcp.decisioning.serve.create_adcp_server_from_platform, " - "or set auto_emit_completion_webhooks=False if you handle " - "webhooks manually inside your platform methods." + "or disable auto_emit_completion_webhooks. This compatibility " + "mode is non-conformant for synchronous terminal responses." ), recovery="terminal", details={ diff --git a/src/adcp/testing/decisioning.py b/src/adcp/testing/decisioning.py index bed0e785a..e3fbf97f5 100644 --- a/src/adcp/testing/decisioning.py +++ b/src/adcp/testing/decisioning.py @@ -13,9 +13,9 @@ :class:`adcp.decisioning.DecisioningPlatform` without binding a port. Useful for in-process integration tests via ``httpx.AsyncClient``, ``starlette.testclient.TestClient``, or direct ASGI invocation. The - default ``auto_emit_completion_webhooks=False`` skips the F12 boot - gate that otherwise refuses to start a sales platform without a - webhook sender wired. + default ``auto_emit_completion_webhooks=False`` matches the production + server's conformant default and suppresses legacy sync-completion + compatibility delivery. * :func:`build_test_client` — async context manager that combines :func:`build_asgi_app`, ``asgi_lifespan.LifespanManager``, and @@ -186,8 +186,8 @@ def build_asgi_app( :func:`create_mcp_server`. Default ``False`` (override-detection filter on; matches :func:`serve`). :param auto_emit_completion_webhooks: Forwarded to - :func:`create_adcp_server_from_platform`. Default ``False`` for - test ergonomics — production :func:`serve` defaults to ``True``. + :func:`create_adcp_server_from_platform`. Default ``False``, + matching production :func:`serve`. :param allowed_hosts: Host header values the MCP transport-security layer will accept. ``None`` → FastMCP's loopback-only default (``localhost``, ``127.0.0.1``, ``[::1]``). Pass the hostname diff --git a/tests/test_decisioning_handler_shims.py b/tests/test_decisioning_handler_shims.py index aba7645b7..43a8df68e 100644 --- a/tests/test_decisioning_handler_shims.py +++ b/tests/test_decisioning_handler_shims.py @@ -927,7 +927,7 @@ def update_rights(self, req, ctx): assert result == {"rights_id": "r_1", "status": "updated"} -# ---- F12 auto-emit on new webhook-eligible shims ---- +# ---- Legacy sync-completion compatibility on webhook-eligible shims ---- def _push_config_params(req_cls, *, url: str = "https://buyer.example.com/wh", **extra): @@ -983,7 +983,7 @@ def activate_signal(self, req, ctx): @pytest.mark.asyncio async def test_acquire_rights_auto_emits_completion_webhook(executor) -> None: - """``acquire_rights`` is in the spec enum; auto-emit fires.""" + """``acquire_rights`` supports the explicit compatibility opt-in.""" sender = AsyncMock() class _BrandRights(DecisioningPlatform): @@ -1004,6 +1004,7 @@ def acquire_rights(self, req, ctx): executor=executor, registry=InMemoryTaskRegistry(), webhook_sender=sender, + auto_emit_completion_webhooks=True, ) from adcp.types import AcquireRightsRequest @@ -1037,6 +1038,7 @@ def sync_audiences(self, audiences, ctx): executor=executor, registry=InMemoryTaskRegistry(), webhook_sender=sender, + auto_emit_completion_webhooks=True, ) from adcp.types import SyncAudiencesRequest @@ -1054,7 +1056,7 @@ def sync_audiences(self, audiences, ctx): @pytest.mark.asyncio -async def test_property_list_ops_dont_auto_emit_because_schema_forbids_push_notif( +async def test_property_list_ops_support_sync_completion_compatibility( executor, ) -> None: """Property-list requests now carry ``push_notification_config`` and @@ -1087,6 +1089,7 @@ def delete_property_list(self, req, ctx): executor=executor, registry=InMemoryTaskRegistry(), webhook_sender=sender, + auto_emit_completion_webhooks=True, ) from adcp.types import CreatePropertyListRequest @@ -1126,6 +1129,7 @@ def get_creative_delivery(self, req, ctx): executor=executor, registry=InMemoryTaskRegistry(), webhook_sender=sender, + auto_emit_completion_webhooks=True, ) from adcp.types import GetCreativeDeliveryRequest diff --git a/tests/test_decisioning_serve.py b/tests/test_decisioning_serve.py index 8897795eb..dfd8a4c1b 100644 --- a/tests/test_decisioning_serve.py +++ b/tests/test_decisioning_serve.py @@ -19,6 +19,7 @@ import os from concurrent.futures import ThreadPoolExecutor +from inspect import signature from unittest.mock import patch import pytest @@ -34,6 +35,7 @@ _default_thread_pool_size, _is_production_env, create_adcp_server_from_platform, + serve, ) @@ -445,15 +447,14 @@ async def creative_format(self, format_id, *, revalidate=False): executor.shutdown(wait=True) -# ---- F12 boot-time webhook gate (Emma sales-direct P0) ---- +# ---- Legacy sync-completion compatibility boot gate ---- def test_serve_fails_fast_when_sales_platform_missing_webhook_sender() -> None: """Sales-non-guaranteed exposes create_media_buy + sync_creatives, both in SPEC_WEBHOOK_TASK_TYPES. With no webhook_sender wired and - auto_emit on (the default), the framework MUST fail at boot — - otherwise buyers register push_notification_config.url and silently - never get notifications. Emma sales-direct verdict 2/10 root cause. + legacy sync auto-emit explicitly enabled, the framework MUST fail at + boot rather than accept a compatibility mode it cannot deliver. The gate raises ``AdcpError("INVALID_REQUEST")`` for parity with ``validate_platform``'s sibling boot-time gates (governance opt-in, @@ -462,7 +463,7 @@ def test_serve_fails_fast_when_sales_platform_missing_webhook_sender() -> None: adtech-product-expert review on PR #339).""" platform = _SalesPlatformWithRequiredMethods() with pytest.raises(AdcpError) as exc_info: - create_adcp_server_from_platform(platform) + create_adcp_server_from_platform(platform, auto_emit_completion_webhooks=True) assert exc_info.value.code == "INVALID_REQUEST" msg = str(exc_info.value) assert "webhook_sender" in msg @@ -480,22 +481,36 @@ def test_serve_passes_with_webhook_sender_wired() -> None: platform = _SalesPlatformWithRequiredMethods() sender = MagicMock() - handler, executor, _ = create_adcp_server_from_platform(platform, webhook_sender=sender) + handler, executor, _ = create_adcp_server_from_platform( + platform, + webhook_sender=sender, + auto_emit_completion_webhooks=True, + ) assert handler._webhook_sender is sender executor.shutdown(wait=True) -def test_serve_passes_with_auto_emit_disabled() -> None: - """Adopter who handles webhooks manually opts out via - auto_emit_completion_webhooks=False — gate doesn't fire.""" +def test_create_adcp_server_defaults_sync_completion_auto_emit_off() -> None: + """The public builder defaults to conformant sync webhook behavior.""" platform = _SalesPlatformWithRequiredMethods() - handler, executor, _ = create_adcp_server_from_platform( - platform, auto_emit_completion_webhooks=False - ) + handler, executor, _ = create_adcp_server_from_platform(platform) assert handler._auto_emit_completion_webhooks is False + assert handler._auto_emit_task_webhooks is True executor.shutdown(wait=True) +def test_public_server_entrypoints_default_sync_completion_auto_emit_off() -> None: + """Both production entrypoints expose the same conformant default.""" + assert ( + signature(create_adcp_server_from_platform) + .parameters["auto_emit_completion_webhooks"] + .default + is False + ) + assert signature(serve).parameters["auto_emit_completion_webhooks"].default is False + assert signature(serve).parameters["auto_emit_task_webhooks"].default is True + + def test_serve_does_not_fire_gate_for_platform_without_webhook_eligible_tools() -> None: """Bare platform claiming no specialism → no per-instance webhook surface → gate doesn't fire. Test fixtures and discovery-only diff --git a/tests/test_decisioning_webhook_emit.py b/tests/test_decisioning_webhook_emit.py index e44595208..a26155cc5 100644 --- a/tests/test_decisioning_webhook_emit.py +++ b/tests/test_decisioning_webhook_emit.py @@ -1,4 +1,4 @@ -"""F12: auto-emit completion webhook on sync-success arm. +"""Sync-completion compatibility and async task webhook coverage. Mirrors the JS test file ``test/server-decisioning-auto-emit-completion.test.js`` (commits @@ -25,6 +25,7 @@ import pytest from adcp.decisioning import ( + AdcpError, DecisioningCapabilities, DecisioningPlatform, SingletonAccounts, @@ -580,15 +581,15 @@ def get_media_buy_delivery(self, req, ctx): @pytest.mark.asyncio -async def test_handler_fires_auto_emit_on_sync_success(executor) -> None: - """End-to-end: sync mutating tool with push URL → auto-emit fires.""" +async def test_handler_explicit_compatibility_opt_in_emits_on_sync_success(executor) -> None: + """The explicit legacy opt-in preserves sync webhook delivery.""" sender = AsyncMock() handler = PlatformHandler( _SyncSuccessPlatform(), executor=executor, registry=InMemoryTaskRegistry(), webhook_sender=sender, - auto_emit_completion_webhooks=True, + auto_emit_completion_webhooks=True, # non-conformant compatibility mode ) await handler.create_media_buy(_make_request(with_url=True), ToolContext()) while _BACKGROUND_WEBHOOK_TASKS: @@ -611,7 +612,8 @@ async def test_handler_fires_exactly_one_completion_webhook_on_handoff_path(exec executor=executor, registry=InMemoryTaskRegistry(), webhook_sender=sender, - auto_emit_completion_webhooks=True, + # Default False applies only to synthetic sync-completion webhooks. + # A real submitted task still requires terminal delivery. ) result = await handler.create_media_buy(_make_request(with_url=True), ToolContext()) # At submit time, no webhook yet — the bg task hasn't completed. @@ -639,9 +641,49 @@ async def test_handler_fires_exactly_one_completion_webhook_on_handoff_path(exec @pytest.mark.asyncio -async def test_handler_opt_out_suppresses_auto_emit(executor) -> None: - """``auto_emit_completion_webhooks=False`` → no delivery on sync - success, even with URL set. Adopter middleware emits manually.""" +async def test_handler_rejects_push_handoff_without_webhook_transport(executor) -> None: + """A submitted task must not promise push delivery with no transport.""" + registry = InMemoryTaskRegistry() + handler = PlatformHandler( + _HandoffPlatform(), + executor=executor, + registry=registry, + ) + + with pytest.raises(AdcpError) as exc_info: + await handler.create_media_buy(_make_request(with_url=True), ToolContext()) + + assert exc_info.value.code == "INVALID_REQUEST" + assert exc_info.value.field == "push_notification_config" + assert registry._records == {} + + +@pytest.mark.asyncio +async def test_handler_task_webhook_opt_out_suppresses_framework_delivery(executor) -> None: + """Adopter-owned task delivery can disable the framework sender.""" + sender = AsyncMock() + registry = InMemoryTaskRegistry() + handler = PlatformHandler( + _HandoffPlatform(), + executor=executor, + registry=registry, + webhook_sender=sender, + auto_emit_task_webhooks=False, + ) + + result = await handler.create_media_buy(_make_request(with_url=True), ToolContext()) + for _ in range(40): + record = await registry.get(result["task_id"]) + if record is not None and record["state"] == "completed": + break + await asyncio.sleep(0.02) + + sender.send_mcp.assert_not_called() + + +@pytest.mark.asyncio +async def test_handler_explicit_false_suppresses_sync_compatibility_emit(executor) -> None: + """An explicit ``False`` suppresses legacy sync delivery.""" sender = AsyncMock() handler = PlatformHandler( _SyncSuccessPlatform(), @@ -672,9 +714,8 @@ async def test_handler_no_url_no_emit(executor) -> None: @pytest.mark.asyncio -async def test_handler_default_is_enabled(executor) -> None: - """``auto_emit_completion_webhooks`` defaults to True — adopter - not setting the flag still gets webhook delivery.""" +async def test_handler_default_does_not_emit_for_sync_terminal_response(executor) -> None: + """A synchronous terminal response emits no webhook by default.""" sender = AsyncMock() handler = PlatformHandler( _SyncSuccessPlatform(), @@ -684,15 +725,13 @@ async def test_handler_default_is_enabled(executor) -> None: # NOT passing auto_emit_completion_webhooks — testing default. ) await handler.create_media_buy(_make_request(with_url=True), ToolContext()) - while _BACKGROUND_WEBHOOK_TASKS: - await asyncio.sleep(0) - sender.send_mcp.assert_awaited_once() + await asyncio.sleep(0.05) + sender.send_mcp.assert_not_called() @pytest.mark.asyncio async def test_handler_no_sender_no_emit(executor) -> None: - """No webhook_sender wired (the default for ``serve()``) → silent - skip. Adopters who don't want webhooks just don't pass one.""" + """Explicit compatibility mode without a sender cannot deliver.""" handler = PlatformHandler( _SyncSuccessPlatform(), executor=executor, diff --git a/tests/test_proposal_lifecycle_e2e.py b/tests/test_proposal_lifecycle_e2e.py index e41a16770..70f89fad6 100644 --- a/tests/test_proposal_lifecycle_e2e.py +++ b/tests/test_proposal_lifecycle_e2e.py @@ -1506,6 +1506,49 @@ async def _handoff_body(task_ctx: Any) -> Any: assert task_record["error"]["code"] == "GOVERNANCE_DENIED" +@pytest.mark.asyncio +async def test_create_media_buy_undeliverable_push_releases_reservation( + executor: ThreadPoolExecutor, + registry: InMemoryTaskRegistry, +) -> None: + """Rejecting a push-enabled handoff before task issuance must still + release the proposal reservation so the buyer can retry.""" + handoff_started = False + + async def _handoff_body(task_ctx: Any) -> Any: + nonlocal handoff_started + del task_ctx + handoff_started = True + return {"media_buy_id": "mb_should_not_exist", "status": "active"} + + router = _build_handoff_create_media_buy_router(_handoff_body) + store = router.proposal_store_for_tenant("default") + handler = _build_handler(router, executor, registry) + + await _seed_committed_proposal(handler) + request = _build_create_media_buy_request("no-transport") + request = request.__class__.model_validate( + { + **request.model_dump(mode="json"), + "push_notification_config": { + "url": "https://buyer.example/webhooks/adcp", + }, + } + ) + + with pytest.raises(AdcpError) as exc_info: + await handler.create_media_buy(request, ToolContext()) + + assert exc_info.value.code == "INVALID_REQUEST" + assert exc_info.value.field == "push_notification_config" + assert handoff_started is False + + record = await store.get(PROPOSAL_ID, expected_account_id="acct_demo") + assert record is not None + assert record.state == ProposalState.COMMITTED + assert registry._records == {} + + @pytest.mark.asyncio async def test_create_media_buy_handoff_buyer_can_retry_after_release( executor: ThreadPoolExecutor, diff --git a/tests/test_testing_decisioning.py b/tests/test_testing_decisioning.py index 433a0370b..846f5026d 100644 --- a/tests/test_testing_decisioning.py +++ b/tests/test_testing_decisioning.py @@ -127,13 +127,11 @@ def test_build_asgi_app_returns_asgi_callable() -> None: assert callable(app) -def test_build_asgi_app_default_skips_webhook_gate() -> None: - """A sales platform without webhook_sender wired would normally - trip the F12 boot-time gate. The helper's - ``auto_emit_completion_webhooks=False`` default skips it so tests - can construct the app without wiring webhook infra.""" +def test_build_asgi_app_uses_conformant_webhook_default() -> None: + """The test helper matches the production default and needs no + sync-completion webhook transport.""" platform = _SalesPlatformWithMethods() - # Should not raise the F12 gate AdcpError. + # Should not require legacy sync-completion transport wiring. app = build_asgi_app(platform) assert app is not None