Skip to content

Commit f8d421e

Browse files
bokelleyclaude
andcommitted
feat(decisioning): framework-wide async-completion webhooks + #930 review fixes
Deliver the spec-required terminal completion / failure webhook on the async (handoff) path of every spec-eligible verb when the buyer registered push_notification_config (adcp#5389). Previously the SDK emitted only on the sync path; create_media_buy / get_products / get_signals and all other async ops left a registered push URL silent and the buyer polling tasks/get. Seam (approach a): emit centrally from the background completion path in dispatch._project_handoff — the single seam every async task flows through (_invoke_platform_method for all verbs, plus proposal finalize). The terminal webhook fires EXACTLY ONCE after registry.complete / registry.fail, with the buyer's operation_id echoed verbatim and the registry task_id included. The sync auto-emit gate already skips the {task_id, status} submitted projection, so the two paths never double-deliver. Webhook emission lives in the decisioning runtime (webhook_emit.emit_terminal_completion_webhook), reusing the sync path's WebhookSender / supervisor and send_mcp payload builder; the pluggable TaskRegistry stays webhook-agnostic. Gated by auto_emit_completion_webhooks so manual-emit adopters are unaffected; no change to the sync defaults. - webhook_emit.py: emit_terminal_completion_webhook (self-isolating, logged-and-swallowed) + operation_id extraction from push config. - dispatch.py: thread webhook_target / webhook_auto_emit through _invoke_platform_method -> _project_handoff; fire on both the registry.complete (completed) and _fail (failed) terminal arms. - handler.py: _handoff_webhook_kwargs() threaded into every spec-eligible shim (create/update_media_buy, sync_creatives, get/activate_signal, get_products, sync_audiences/catalogs, brand/rights, property_list). - webhook_supervisor{,_pg}.py: add operation_id to send_mcp; Pg persists it on the queue row (CREATE column + ADD COLUMN IF NOT EXISTS backfill) and replays it from the worker. Review fixes on #930: - MUST-FIX docstrings: corrected the now-true claims — async terminal completion delivers via push webhook when configured, always via tasks/get polling (handler / serve / specialism docstrings). - MUST-FIX dead guard arm (c): removed the unreachable post-dispatch assert_account_resolved_for_async call; documented that compose_caller_identity owns the no-push-handoff + unresolved-account case (fails closed terminally at _build_ctx before any task is minted). Added a test asserting no registry row is issued. - SHOULD-FIX side-effect leak (b): reject_wholesale_handoff_before_launch wired as a pre_handoff_reject callback so a wholesale handoff is rejected BEFORE the registry row / background task / draft / webhook -- no side effects for a buyer told 'rejected'. Test asserts an empty registry after rejection. - SHOULD-FIX persist-draft: regression test pinning that fields= / pagination= projections shape only the wire response, never the persisted draft (full product pricing retained). - SHOULD-FIX wire-validator: tightened the discovery submitted test to assert variant == 'submitted' (no skip escape hatch). - Added async-completion webhook tests on get_products, get_signals, create_media_buy (one completed webhook; operation_id echoed, task_id, result in payload), the failure path (one failed webhook), and the no-push no-webhook case. Updated the create_media_buy handoff test to assert the conformant exactly-once behavior. No ADCP_VERSION bump, no schema re-download, no public-API export change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a603199 commit f8d421e

13 files changed

Lines changed: 1015 additions & 73 deletions

src/adcp/decisioning/discovery_guards.py

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -114,30 +114,60 @@ def assert_discovery_push_consistent(req: Any, *, mode_field: str) -> None:
114114
)
115115

116116

117+
def _wholesale_handoff_error(mode_field: str) -> AdcpError:
118+
"""Build the guard (b) rejection. Shared by the pre-dispatch callback
119+
and the post-dispatch belt-and-braces check so both emit an identical
120+
diagnostic."""
121+
return AdcpError(
122+
"INVALID_REQUEST",
123+
message=(
124+
f"{mode_field}='wholesale' returned a task handoff, but "
125+
"wholesale discovery MUST complete synchronously. Return the "
126+
"catalog directly; signal partial completion via the "
127+
"response's incomplete[] field rather than handing off to a "
128+
"background task. The 'brief' mode is the async-capable path."
129+
),
130+
field=mode_field,
131+
recovery="correctable",
132+
)
133+
134+
135+
def reject_wholesale_handoff_before_launch(mode_field: str) -> None:
136+
"""Guard (b) pre-dispatch arm: reject a wholesale handoff BEFORE the
137+
framework mints a registry row or launches the background task.
138+
139+
Wired as the ``pre_handoff_reject`` callback on
140+
:func:`adcp.decisioning.dispatch._invoke_platform_method` only when the
141+
resolved mode is wholesale. The dispatcher calls it the instant it
142+
detects the adapter returned a :class:`TaskHandoff` — before
143+
``_project_handoff`` issues a task or starts background work — so a
144+
rejected wholesale handoff leaves NO task row, NO persisted draft, NO
145+
background coroutine, and NO completion webhook. Without this, the
146+
post-dispatch check below would fire only after those side effects had
147+
already happened.
148+
149+
:raises AdcpError: ``INVALID_REQUEST`` / ``correctable``,
150+
``field=<mode_field>``.
151+
"""
152+
raise _wholesale_handoff_error(mode_field)
153+
154+
117155
def reject_wholesale_handoff(result: Any, *, mode: str | None, mode_field: str) -> None:
118-
"""Guard (b): post-dispatch reject of an adopter handoff on a wholesale call.
156+
"""Guard (b) post-dispatch arm: belt-and-braces reject of an adopter
157+
handoff on a wholesale call.
119158
159+
Defense-in-depth for :func:`reject_wholesale_handoff_before_launch`.
120160
Called after ``_invoke_platform_method`` with the (already projected)
121-
result. When the resolved mode is wholesale and the result is the
161+
result; covers any dispatch path that did not wire the pre-launch
162+
callback. When the resolved mode is wholesale and the result is the
122163
framework's submitted projection, the adopter's wholesale code path
123164
handed off — a contract violation. Reject (``field=<mode_field>``).
124165
125166
:param mode: The request's coerced mode string.
126167
:raises AdcpError: ``INVALID_REQUEST`` / ``correctable``.
127168
"""
128169
if mode == "wholesale" and _is_submitted_projection(result):
129-
raise AdcpError(
130-
"INVALID_REQUEST",
131-
message=(
132-
f"{mode_field}='wholesale' returned a task handoff, but "
133-
"wholesale discovery MUST complete synchronously. Return the "
134-
"catalog directly; signal partial completion via the "
135-
"response's incomplete[] field rather than handing off to a "
136-
"background task. The 'brief' mode is the async-capable path."
137-
),
138-
field=mode_field,
139-
recovery="correctable",
140-
)
170+
raise _wholesale_handoff_error(mode_field)
141171

142172

143173
def assert_account_resolved_for_async(
@@ -156,13 +186,21 @@ def assert_account_resolved_for_async(
156186
:func:`adcp.decisioning.dispatch.compose_caller_identity`'s contract
157187
across derived / implicit / explicit account modes.
158188
159-
Call this both pre-dispatch (with ``result=None``) when the buyer
160-
supplied ``push_notification_config`` — so the rejection fires as a
161-
clean ``INVALID_REQUEST`` before any platform / registry interaction —
162-
and post-dispatch (passing the projected ``result``) so an adopter
163-
handoff against an unresolved account is also caught. The framework's
164-
registry independently rejects an empty ``account_id`` at issue-time;
165-
this guard surfaces the buyer-facing ``field='account'`` diagnostic.
189+
Used pre-dispatch (with ``result=None``) when the buyer supplied
190+
``push_notification_config`` — so the rejection fires as a clean
191+
``INVALID_REQUEST`` / ``correctable`` with the buyer-facing
192+
``field='account'`` diagnostic BEFORE any platform / registry
193+
interaction.
194+
195+
The no-push handoff case (the adopter hands off against an unresolved
196+
account without a push config) is NOT routed through this guard:
197+
:func:`adcp.decisioning.dispatch.compose_caller_identity` already fails
198+
closed at ``_build_ctx`` — which runs BEFORE the platform method — with
199+
a terminal ``INVALID_REQUEST``, so no task row is ever minted against an
200+
unresolved account. A post-dispatch call here passing the projected
201+
``result`` would be unreachable for an unresolved account (control never
202+
reaches it because ``_build_ctx`` raised first) and a no-op for a
203+
resolved one, so the shims do not make that call.
166204
167205
:raises AdcpError: ``INVALID_REQUEST`` / ``correctable`` with
168206
``field='account'``.
@@ -245,4 +283,5 @@ def reject_hand_rolled_submitted(result: Any) -> None:
245283
"assert_discovery_push_consistent",
246284
"reject_hand_rolled_submitted",
247285
"reject_wholesale_handoff",
286+
"reject_wholesale_handoff_before_launch",
248287
]

src/adcp/decisioning/dispatch.py

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
is_task_handoff,
6767
is_workflow_handoff,
6868
)
69+
from adcp.decisioning.webhook_emit import emit_terminal_completion_webhook
6970

7071
if TYPE_CHECKING:
7172
from collections.abc import Awaitable, Callable
@@ -77,6 +78,10 @@
7778
from adcp.decisioning.registry import BuyerAgent
7879
from adcp.decisioning.types import Account
7980
from adcp.server.base import ToolContext
81+
from adcp.webhook_sender import WebhookSender
82+
from adcp.webhook_supervisor import WebhookDeliverySupervisor
83+
84+
WebhookDeliveryTarget = WebhookSender | WebhookDeliverySupervisor
8085

8186
logger = logging.getLogger(__name__)
8287

@@ -1305,6 +1310,9 @@ async def _invoke_platform_method(
13051310
extra_kwargs: dict[str, Any] | None = None,
13061311
on_complete: Callable[[Any], Awaitable[None]] | None = None,
13071312
on_failure: Callable[[BaseException], Awaitable[None]] | None = None,
1313+
webhook_target: WebhookDeliveryTarget | None = None,
1314+
webhook_auto_emit: bool = True,
1315+
pre_handoff_reject: Callable[[], None] | None = None,
13081316
) -> Any:
13091317
"""Invoke a platform method, projecting hybrid returns.
13101318
@@ -1351,6 +1359,30 @@ async def _invoke_platform_method(
13511359
Symmetric with ``on_complete``. Used by v1.5 create_media_buy
13521360
to release the consumption reservation so the buyer can retry.
13531361
Hook errors are logged but never block exception propagation.
1362+
1363+
:param webhook_target: Forwarded to :func:`_project_handoff` so the
1364+
background completion path can deliver the terminal completion /
1365+
failure webhook when the buyer registered
1366+
``push_notification_config``. The handler wires its
1367+
``webhook_sender`` / ``webhook_supervisor``; only the handoff
1368+
(async) arm uses it — the sync arm's auto-emit is a separate
1369+
call in the handler shim.
1370+
1371+
:param webhook_auto_emit: Forwarded to :func:`_project_handoff`;
1372+
mirrors the handler's ``auto_emit_completion_webhooks`` so an
1373+
adopter emitting webhooks manually never gets a framework
1374+
double-delivery on the handoff path.
1375+
1376+
:param pre_handoff_reject: Optional zero-arg callback invoked when
1377+
the adapter returned a :class:`TaskHandoff`, BEFORE
1378+
:func:`_project_handoff` mints a registry row or launches the
1379+
background task. Raising from it (e.g. an :class:`AdcpError`)
1380+
rejects the handoff with NO side effects — no task row, no
1381+
background work, no completion webhook. The discovery
1382+
wholesale-is-synchronous guard uses this so an adopter handing
1383+
off on a ``wholesale`` request is rejected cleanly instead of
1384+
leaking a task the buyer was told was rejected. Runs only on the
1385+
``TaskHandoff`` arm; sync / workflow-handoff returns ignore it.
13541386
"""
13551387
# pydantic is a required dep; import here (not at module level) to mirror
13561388
# the lazy-import discipline used throughout this module.
@@ -1513,6 +1545,13 @@ async def _invoke_platform_method(
15131545
raise wrapped from exc
15141546

15151547
if is_task_handoff(result):
1548+
# Reject before any side effect (registry row, background task,
1549+
# completion webhook) is created. The wholesale discovery guard
1550+
# uses this so an adopter handing off on a synchronous-only
1551+
# wholesale request never leaks a task the buyer is told is
1552+
# rejected.
1553+
if pre_handoff_reject is not None:
1554+
pre_handoff_reject()
15161555
return await _project_handoff(
15171556
result,
15181557
ctx,
@@ -1522,6 +1561,8 @@ async def _invoke_platform_method(
15221561
on_complete=on_complete,
15231562
on_failure=on_failure,
15241563
request_params=params,
1564+
webhook_target=webhook_target,
1565+
webhook_auto_emit=webhook_auto_emit,
15251566
)
15261567
if is_workflow_handoff(result):
15271568
return await _project_workflow_handoff(
@@ -1588,6 +1629,8 @@ async def _project_handoff(
15881629
on_complete: Callable[[Any], Awaitable[None]] | None = None,
15891630
on_failure: Callable[[BaseException], Awaitable[None]] | None = None,
15901631
request_params: BaseModel | None = None,
1632+
webhook_target: WebhookDeliveryTarget | None = None,
1633+
webhook_auto_emit: bool = True,
15911634
) -> dict[str, Any]:
15921635
"""Promote a TaskHandoff to a background task.
15931636
@@ -1647,7 +1690,26 @@ async def _project_handoff(
16471690
(``registry.fail``) paths — closes #563. Mirrors the sync
16481691
AdcpError path's context-passthrough (PR #560). When ``None``,
16491692
no echo happens (e.g. test fixtures invoking the handoff
1650-
helper directly).
1693+
helper directly). Also the source of the buyer's
1694+
``push_notification_config`` (url / token / operation_id) for
1695+
the terminal-completion webhook.
1696+
1697+
:param webhook_target: The wired :class:`~adcp.webhook_sender.WebhookSender`
1698+
or :class:`~adcp.webhook_supervisor.WebhookDeliverySupervisor`. When
1699+
the buyer supplied ``push_notification_config`` on
1700+
``request_params``, the background completion path emits the
1701+
terminal completion / failure webhook to that URL EXACTLY ONCE —
1702+
on success after ``registry.complete``, on failure after
1703+
``registry.fail``. This is the async-path half of the spec
1704+
webhook contract (adcp#5389): a ``Submitted`` task carrying a
1705+
push config MUST deliver at least the terminal notification.
1706+
``None`` (and the no-push case) skips delivery — the buyer polls
1707+
``tasks/get`` instead. The framework's polling path is unchanged.
1708+
1709+
:param webhook_auto_emit: Mirrors the handler's
1710+
``auto_emit_completion_webhooks`` flag. When ``False`` the
1711+
adopter emits webhooks manually inside their handler; the
1712+
framework skips the terminal emission so it never double-delivers.
16511713
16521714
The handoff fn is extracted via the type-identity dispatch in
16531715
:func:`adcp.decisioning.types.is_task_handoff`. Subclassed
@@ -1699,7 +1761,23 @@ async def _fail(exc: AdcpError) -> None:
16991761
"still recorded in the registry",
17001762
task_id,
17011763
)
1702-
await registry.fail(task_id, exc.to_wire())
1764+
error_wire = exc.to_wire()
1765+
await registry.fail(task_id, error_wire)
1766+
# Terminal failure webhook (spec MUST when push config present).
1767+
# Fired AFTER registry.fail so the buyer's tasks/get poll and the
1768+
# push notification observe the same terminal state. The error
1769+
# wire dict is the payload `result`; the helper is self-isolating
1770+
# (logged-and-swallowed) so a delivery failure never re-raises into
1771+
# the background task.
1772+
await emit_terminal_completion_webhook(
1773+
target=webhook_target,
1774+
enabled=webhook_auto_emit,
1775+
method_name=method_name,
1776+
params=request_params,
1777+
status="failed",
1778+
task_id=task_id,
1779+
result=error_wire,
1780+
)
17031781

17041782
async def _run() -> None:
17051783
try:
@@ -1792,6 +1870,22 @@ async def _run() -> None:
17921870
# surfaces it under the top-level ``context`` key; nothing to
17931871
# do here on the result path.
17941872
await registry.complete(task_id, persisted)
1873+
# Terminal completion webhook (spec MUST when push config present).
1874+
# Fired AFTER registry.complete so the buyer's tasks/get poll and
1875+
# the push notification observe the same terminal artifact. EXACTLY
1876+
# ONCE — the sync auto-emit gate skips the {task_id, status}
1877+
# submitted projection, so the handoff path owns the completion
1878+
# notification end-to-end. The helper is self-isolating
1879+
# (logged-and-swallowed); a delivery failure never re-raises here.
1880+
await emit_terminal_completion_webhook(
1881+
target=webhook_target,
1882+
enabled=webhook_auto_emit,
1883+
method_name=method_name,
1884+
params=request_params,
1885+
status="completed",
1886+
task_id=task_id,
1887+
result=persisted,
1888+
)
17951889

17961890
# ``asyncio.create_task`` only weak-refs the resulting Task — under
17971891
# GC pressure or with no outer awaiter, the task can be collected

0 commit comments

Comments
 (0)