Skip to content

feat(webhooks)!: create_mcp_webhook_payload returns McpWebhookPayload#632

Merged
bokelley merged 1 commit intomainfrom
claude/issue-607-typed-mcp-webhook-payload
May 10, 2026
Merged

feat(webhooks)!: create_mcp_webhook_payload returns McpWebhookPayload#632
bokelley merged 1 commit intomainfrom
claude/issue-607-typed-mcp-webhook-payload

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Closes #607.

Summary

  • create_mcp_webhook_payload returns a typed McpWebhookPayload instance instead of dict[str, Any]. Adopters drop the McpWebhookPayload.model_construct(**dict) ceremony and get attribute access + IDE autocomplete directly.
  • task_type becomes a required TaskType | str argument, validated against the closed TaskType enum (the spec's complete set of async/tracked operations) at construction. Synchronous-only operations (get_products, list_creatives, etc.) are deliberately not in the enum and shouldn't appear in webhook payloads — failing fast at the publisher surfaces this here rather than as silent receiver rejection.
  • WebhookSender.send_mcp and WebhookDeliverySupervisor.send_mcp propagate the same required task_type. The PG supervisor coerces the TaskType enum to its on-wire string before binding to psycopg, preventing repr() corruption of the adcp_webhook_delivery_queue.task_type column.

Migration

Old New
payload["task_id"] payload.task_id
json=payload (HTTP) json=to_wire_dict(payload)
create_mcp_webhook_payload(...) without task_type required arg

Reviewer findings addressed

  • Blocker (code-reviewer): webhook_supervisor_pg.py had task_type: str | None = None diverging from the Protocol; psycopg would have written "TaskType.create_media_buy" (enum repr) to the queue. Mirrored the in-memory normalization at the boundary.
  • Must-fix (code-reviewer): webhooks.py docstring example for get_adcp_signed_headers_for_webhook used task_type="get_products" which now raises. Updated.
  • Indentation nit (code-reviewer): broken multi-line indent in two test_webhook_supervisor_pg.py callsites.

Follow-ups (not blocking)

Surfaced by ad-tech-protocol-expert review:

  1. The schema's protocol field exists on McpWebhookPayload but the builder doesn't surface it — file a separate issue to expose it as a builder kwarg.
  2. domain (the builder accepts it; spec doesn't) may be a naming-drift duplicate of protocol. Confirm with adopters and pick one.
  3. token is spec-grounded (echoed per push-notification-config.json) but stored via extra='allow' rather than as a typed field — file an upstream spec issue to promote it.
  4. Cross-SDK parity check: confirm buildTaskWebhookPayload in adcontextprotocol/adcp-node enforces the same TaskType closure.

Test plan

  • pytest tests/ --ignore=tests/integration — 4203 passed
  • mypy src/adcp/ — no issues
  • ruff check on changed files — all checks passed
  • black formatting via pre-commit
  • New regression: test_create_mcp_webhook_payload_returns_typed_instance and test_create_mcp_webhook_payload_rejects_invalid_task_type in tests/test_webhooks_to_wire_dict.py

🤖 Generated with Claude Code

Closes #607.

`create_mcp_webhook_payload` now returns a typed `McpWebhookPayload`
instance instead of `dict[str, Any]`. Adopters drop the
`McpWebhookPayload.model_construct(**dict)` ceremony and get attribute
access + IDE autocomplete directly. Pair with `to_wire_dict(payload)`
for HTTP transport.

`task_type` becomes a required `TaskType | str` argument and is
validated against the closed `TaskType` enum at construction.
Synchronous-only operations (`get_products`, `list_creatives`,
`preview_creative`, etc.) are deliberately not in the enum because
they don't flow through the task management system — they shouldn't
appear in webhook payloads. Failing fast at the publisher surfaces
the bug here rather than as a silent receiver-side rejection.

Also:
- `WebhookSender.send_mcp` and `WebhookDeliverySupervisor.send_mcp`
  propagate the same required `task_type: TaskType | str`.
- The PG supervisor coerces `TaskType` enum to its on-wire string
  before binding to psycopg, preventing `repr()` corruption of the
  `adcp_webhook_delivery_queue.task_type` column.
- `webhook_sender.py:550` callsite uses `payload.idempotency_key` and
  `to_wire_dict(payload)` for HTTP transport.

BREAKING CHANGE: `create_mcp_webhook_payload` returns a Pydantic model,
not a dict; `task_type` is now required.

Migration:

| Old | New |
|---|---|
| `payload["task_id"]` | `payload.task_id` |
| `json=payload` (HTTP) | `json=to_wire_dict(payload)` |
| `create_mcp_webhook_payload(...)` without `task_type` | required arg |

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley force-pushed the claude/issue-607-typed-mcp-webhook-payload branch from 4897b4a to 607bbae Compare May 10, 2026 13:20
@bokelley bokelley merged commit 9eb962c into main May 10, 2026
16 checks passed
@bokelley bokelley deleted the claude/issue-607-typed-mcp-webhook-payload branch May 10, 2026 13:25
bokelley added a commit that referenced this pull request May 10, 2026
…otocol enum) (#637)

* feat(webhooks)!: replace `domain` builder kwarg with typed `protocol`

Follow-up to #632. The schema's `mcp-webhook-payload.json` defines a
typed `protocol` field (`AdcpProtocol` enum: `media-buy | signals |
governance | creative | brand | sponsored-intelligence`); the builder
was instead accepting an undocumented `domain` string and stuffing it
into the wire body via `extra='allow'`. The JS reference implementation
(`buildTaskWebhookPayload` in `from-platform.ts`) sets `payload.protocol`
and never sets `domain` — so the Python SDK was the outlier.

Replaces `create_mcp_webhook_payload(..., domain=...)` and
`WebhookSender.send_mcp(..., domain=...)` with `protocol=...`. The kwarg
accepts either an `AdcpProtocol` enum value or a kebab-case string;
unknown values raise `ValidationError` at construction. `AdcpProtocol`
is now publicly exported from `adcp.types`.

Zero in-repo callers passed `domain=`; verified by grep before the swap.
The kwarg was added in #632's same migration window so the breaking-
change cycle is bundled.

BREAKING CHANGE: `domain` kwarg removed from `create_mcp_webhook_payload`
and `WebhookSender.send_mcp`. Migrate to `protocol` (kebab-case string
or `AdcpProtocol` enum value).

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

* feat(webhooks): auto-derive `protocol` from `task_type` in builder

Caller now passes only `task_type` and the builder fills `protocol`
from the canonical `TaskType` → `AdcpProtocol` mapping. Mirrors
`protocolForTool` in `adcontextprotocol/adcp-client:src/lib/server/
decisioning/runtime/protocol-for-tool.ts` so cross-SDK webhook bodies
classify operations identically.

The mapping covers all 20 spec values; explicit `protocol=` passed by
the caller still wins.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 10, 2026
…eturn

create_mcp_webhook_payload was changed in #632 to return McpWebhookPayload
(typed Pydantic model) instead of dict[str, Any]. The type-check test
still demonstrated the dict + cast() pattern, which no longer typechecks.

Update to demonstrate the new zero-ignore adopter pattern: typed
attribute access (payload.task_id) for reads, to_wire_dict(payload) for
HTTP serialization. Use TaskType.create_media_buy (a real async task
type — get_products is sync-only and not in the TaskType enum).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 10, 2026
…eturn

create_mcp_webhook_payload was changed in #632 to return McpWebhookPayload
(typed Pydantic model) instead of dict[str, Any]. The type-check test
still demonstrated the dict + cast() pattern, which no longer typechecks.

Update to demonstrate the new zero-ignore adopter pattern: typed
attribute access (payload.task_id) for reads, to_wire_dict(payload) for
HTTP serialization. Use TaskType.create_media_buy (a real async task
type — get_products is sync-only and not in the TaskType enum).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request May 10, 2026
…ract (#634)

* feat(testing): adopter type-checking test suite with zero-ignore contract

Adds tests/type_checks/ — seven files that exercise documented SDK
extension patterns under mypy --strict with zero # type: ignore lines.
Bundled with the AccountStore.resolution ClassVar fix required to make
the patterns type-check cleanly.

Closes #625

https://claude.ai/code/session_01NDYgk4r6ooU1Zj3qZwainB

* ci: wire adopter type-check suite into test matrix

Adds "Run adopter type-check suite" step to the test job so
mypy --strict tests/type_checks/ runs on every CI pass across all
four Python matrix versions.

https://claude.ai/code/session_01NDYgk4r6ooU1Zj3qZwainB

* fix(testing): address pre-PR review findings in type-check suite

- extend_response_with_override: replace Field on _-prefixed name
  (Pydantic v2 NameError) with PrivateAttr
- handler_three_branch_return: add AsyncSeller covering the async
  direct-return branch (all three SalesResult paths now present)
- pyproject.toml: re-enable import-not-found in tests.type_checks.*
  override so broken imports fail rather than pass silently

https://claude.ai/code/session_01NDYgk4r6ooU1Zj3qZwainB

* fix(testing): update webhook type-check for typed McpWebhookPayload return

create_mcp_webhook_payload was changed in #632 to return McpWebhookPayload
(typed Pydantic model) instead of dict[str, Any]. The type-check test
still demonstrated the dict + cast() pattern, which no longer typechecks.

Update to demonstrate the new zero-ignore adopter pattern: typed
attribute access (payload.task_id) for reads, to_wire_dict(payload) for
HTTP serialization. Use TaskType.create_media_buy (a real async task
type — get_products is sync-only and not in the TaskType enum).

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

* fix(testing): use GeneratedTaskStatus enum return type in webhook test

McpWebhookPayload.status is GeneratedTaskStatus (enum), not str.
extract_status return type and assertion updated accordingly.

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

---------

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.

feat(webhooks): create_mcp_webhook_payload should return McpWebhookPayload

1 participant