Skip to content

Commit c23b406

Browse files
bokelleyclaude
andauthored
fix(decisioning): gate async-completion webhook on SPEC_WEBHOOK_TASK_TYPES (#932)
After #930, _project_handoff calls emit_terminal_completion_webhook for every async task. SDK-internal, non-spec task types (notably finalize_proposal, an interception of get_products in proposal_dispatch.py that is not a spec wire op and not in SPEC_WEBHOOK_TASK_TYPES) flow through with no webhook target wired, so the emitter logged a spurious "neither webhook_sender nor webhook_supervisor is wired — terminal webhook silently dropped" WARNING on every async finalize, even on a correctly-configured server. Hoist the SPEC_WEBHOOK_TASK_TYPES check to the top of the emitter (after the enabled gate, before the target-None warning) and return silently for non-spec task types, mirroring how the sync emitter gates. Non-spec types rely on tasks/get polling / publishStatusChange per the documented rule above SPEC_WEBHOOK_TASK_TYPES. Spec types with a real target still emit; spec types with target=None (genuine misconfig) still warn. Closes #931 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 76fa4fa commit c23b406

2 files changed

Lines changed: 146 additions & 16 deletions

File tree

src/adcp/decisioning/webhook_emit.py

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -385,16 +385,25 @@ async def emit_terminal_completion_webhook(
385385
386386
* ``enabled`` is False (operator opted out via
387387
``auto_emit_completion_webhooks=False`` — they emit manually).
388+
* ``method_name`` isn't in :data:`SPEC_WEBHOOK_TASK_TYPES`. This
389+
gate runs FIRST, before any target check. SDK-internal,
390+
non-spec task types (e.g. ``finalize_proposal``, an interception
391+
of ``get_products`` in ``proposal_dispatch.py``) flow through
392+
``_project_handoff`` like any async task but legitimately have no
393+
webhook target wired; per the spec-gate rule above
394+
:data:`SPEC_WEBHOOK_TASK_TYPES`, they skip delivery and rely on
395+
``tasks/get`` polling / ``publishStatusChange``. Returning here
396+
before the ``target is None`` branch keeps a correctly-configured
397+
server from logging a spurious "silently dropped" WARNING on
398+
every async non-spec task.
388399
* The request didn't carry ``push_notification_config.url``
389400
(polling-only via ``tasks/get`` — the spec permits this).
390401
391402
Logs a WARNING when:
392403
393-
* ``target`` is None but the buyer DID register a push config —
394-
their terminal notification is being silently dropped, the same
395-
misconfig the sync gate warns on.
396-
* ``method_name`` isn't in :data:`SPEC_WEBHOOK_TASK_TYPES` (the
397-
adopter extended the tool surface beyond the spec enum).
404+
* ``target`` is None but the buyer DID register a push config for a
405+
SPEC-eligible task type — their terminal notification is being
406+
silently dropped, the same misconfig the sync gate warns on.
398407
399408
:param status: ``'completed'`` on success or ``'failed'`` on a
400409
terminal failure. The wire ``GeneratedTaskStatus`` enum.
@@ -409,6 +418,18 @@ async def emit_terminal_completion_webhook(
409418
if not enabled:
410419
return
411420

421+
# Spec gate FIRST — before any target / config inspection. Task
422+
# types outside the closed spec enum (SDK-internal interceptions
423+
# like ``finalize_proposal``) are not webhook-eligible; they skip
424+
# silently and rely on ``tasks/get`` / ``publishStatusChange``.
425+
# Running this ahead of the ``target is None`` branch is what
426+
# stops a correctly-configured server from emitting a spurious
427+
# "silently dropped" WARNING on every async non-spec task. The
428+
# sync emitter (:func:`maybe_emit_sync_completion`) gates the
429+
# same way.
430+
if method_name not in SPEC_WEBHOOK_TASK_TYPES:
431+
return
432+
412433
config = getattr(params, "push_notification_config", None)
413434
if config is None and isinstance(params, dict):
414435
config = params.get("push_notification_config")
@@ -456,17 +477,6 @@ async def emit_terminal_completion_webhook(
456477
if result is not None:
457478
result = strip_credentials_from_wire_result(method_name, result)
458479

459-
if method_name not in SPEC_WEBHOOK_TASK_TYPES:
460-
logger.warning(
461-
"[adcp.decisioning] terminal %s webhook for async %s "
462-
"(task_id=%s) skipped — tool not in spec task-type enum "
463-
"(closed set per schemas/cache/enums/task-type.json).",
464-
status,
465-
method_name,
466-
task_id,
467-
)
468-
return
469-
470480
await target.send_mcp(
471481
url=url,
472482
task_id=task_id,

tests/test_decisioning_webhook_emit.py

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
_BACKGROUND_WEBHOOK_TASKS,
3636
SPEC_WEBHOOK_TASK_TYPES,
3737
_extract_push_notification_url_and_token,
38+
emit_terminal_completion_webhook,
3839
maybe_emit_sync_completion,
3940
)
4041
from adcp.server.base import ToolContext
@@ -377,6 +378,125 @@ class _Params:
377378
sender.send_mcp.assert_not_called()
378379

379380

381+
# ---- emit_terminal_completion_webhook spec-enum gate ----
382+
383+
_SILENTLY_DROPPED = "silently dropped"
384+
385+
386+
@pytest.mark.asyncio
387+
async def test_terminal_emit_skips_non_spec_task_type_without_warning(
388+
caplog: pytest.LogCaptureFixture,
389+
) -> None:
390+
"""SDK-internal, non-spec task types (e.g. ``finalize_proposal``, an
391+
interception of ``get_products`` in ``proposal_dispatch.py``) flow
392+
through ``_project_handoff`` like any async task. They legitimately
393+
have no webhook target wired, so the spec gate must skip them
394+
SILENTLY — no emission AND no "silently dropped" misconfig warning,
395+
even when the buyer registered a push config. Regression: #931 — the
396+
target-None warning fired on every async finalize on a
397+
correctly-configured server."""
398+
399+
class _Config:
400+
url = "https://buyer.example.com/wh"
401+
token = None
402+
403+
class _Params:
404+
push_notification_config = _Config()
405+
406+
target = AsyncMock()
407+
408+
with caplog.at_level("WARNING", logger="adcp.decisioning.webhook_emit"):
409+
await emit_terminal_completion_webhook(
410+
target=None, # no target wired — the finalize_proposal reality
411+
enabled=True,
412+
method_name="finalize_proposal", # NOT in SPEC_WEBHOOK_TASK_TYPES
413+
params=_Params(),
414+
status="completed",
415+
task_id="task_finalize_1",
416+
result={"proposal_id": "prop_1"},
417+
)
418+
419+
# No emission attempted.
420+
target.send_mcp.assert_not_awaited()
421+
# And crucially: the spurious misconfig warning is ABSENT.
422+
messages = [r.message for r in caplog.records]
423+
assert not any(_SILENTLY_DROPPED in m for m in messages), (
424+
f"non-spec task type must skip silently, but a 'silently dropped' "
425+
f"warning was logged: {messages}"
426+
)
427+
428+
429+
@pytest.mark.asyncio
430+
async def test_terminal_emit_fires_for_spec_task_type_with_target() -> None:
431+
"""A spec-eligible task type with a real target still emits the
432+
terminal completion webhook unchanged — the gate only short-circuits
433+
non-spec types."""
434+
435+
class _Config:
436+
url = "https://buyer.example.com/wh"
437+
token = "echo-back-token"
438+
439+
class _Params:
440+
push_notification_config = _Config()
441+
442+
target = AsyncMock()
443+
444+
await emit_terminal_completion_webhook(
445+
target=target,
446+
enabled=True,
447+
method_name="create_media_buy", # in SPEC_WEBHOOK_TASK_TYPES
448+
params=_Params(),
449+
status="completed",
450+
task_id="task_mb_1",
451+
result={"media_buy_id": "mb_1"},
452+
)
453+
454+
target.send_mcp.assert_awaited_once()
455+
call_kwargs = target.send_mcp.await_args.kwargs
456+
assert call_kwargs["url"] == "https://buyer.example.com/wh"
457+
assert call_kwargs["task_type"] == "create_media_buy"
458+
assert call_kwargs["status"] == "completed"
459+
assert call_kwargs["task_id"] == "task_mb_1"
460+
assert call_kwargs["result"] == {"media_buy_id": "mb_1"}
461+
assert call_kwargs["token"] == "echo-back-token"
462+
463+
464+
@pytest.mark.asyncio
465+
async def test_terminal_emit_warns_for_spec_task_type_with_target_none(
466+
caplog: pytest.LogCaptureFixture,
467+
) -> None:
468+
"""A SPEC-eligible task type with a push config registered but
469+
``target=None`` is a genuine misconfig — the buyer's terminal
470+
notification is being dropped. That warning MUST still fire; the
471+
spec gate only suppresses the warning for non-spec types."""
472+
473+
class _Config:
474+
url = "https://buyer.example.com/wh"
475+
token = None
476+
477+
class _Params:
478+
push_notification_config = _Config()
479+
480+
with caplog.at_level("WARNING", logger="adcp.decisioning.webhook_emit"):
481+
await emit_terminal_completion_webhook(
482+
target=None, # genuine misconfig for a spec task type
483+
enabled=True,
484+
method_name="create_media_buy", # in SPEC_WEBHOOK_TASK_TYPES
485+
params=_Params(),
486+
status="completed",
487+
task_id="task_mb_2",
488+
result={"media_buy_id": "mb_2"},
489+
)
490+
491+
messages = [r.message for r in caplog.records]
492+
assert any(
493+
"neither webhook_sender nor webhook_supervisor" in m
494+
and _SILENTLY_DROPPED in m
495+
and "buyer.example.com/wh" in m
496+
for m in messages
497+
), f"expected target-None misconfig warning citing the buyer URL; got {messages}"
498+
499+
380500
# ---- PlatformHandler integration: sync-success fires, handoff doesn't ----
381501

382502

0 commit comments

Comments
 (0)