Skip to content

fix(types): CreateMediaBuy handler return type covers all 3 branches + add Submitted alias#575

Merged
bokelley merged 2 commits intomainfrom
bokelley/issue-570-create-media-buy-types
May 4, 2026
Merged

fix(types): CreateMediaBuy handler return type covers all 3 branches + add Submitted alias#575
bokelley merged 2 commits intomainfrom
bokelley/issue-570-create-media-buy-types

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 4, 2026

Summary

CreateMediaBuyResponse is a 3-branch anyOf union (success / error / submitted), but the SDK previously typed PlatformHandler.create_media_buy as returning only the success branch and exposed semantic aliases for only two of the three branches. This PR ships the SDK-side type-system fixes:

  • PlatformHandler.create_media_buy is now annotated -> CreateMediaBuyResponse (the union). The internal cast() matches. The handoff path legitimately returns the Submitted envelope, so the prior success-only annotation was misleading.
  • CreateMediaBuySubmittedResponse semantic alias added alongside the existing CreateMediaBuySuccessResponse / CreateMediaBuyErrorResponse. It maps to CreateMediaBuyResponse3 (the status='submitted' + task_id branch). Exported from adcp.types and re-exported from the top-level adcp package.
  • New test file tests/test_create_media_buy_response_types.py covers alias identity, model_validate of a spec-compliant submitted payload, missing-task_id rejection, and the handler return-type annotation.
  • tests/fixtures/public_api_snapshot.json updated for the new adcp.types and top-level adcp exports.
  • src/adcp/types/guards.py: widened the local CreateMediaBuyResponse alias to the full 3-branch union; added is_create_media_buy_submitted TypeGuard; updated success/error guards to short-circuit on the submitted branch.

Out of scope (filed separately)

The original FastMCP error reported in #570 was caused by a malformed Submitted-branch payload (missing task_id) hitting jsonschema's deepest-error heuristic. The two underlying fixes belong elsewhere:

  • salesagent — emit a valid Submitted payload (task_id populated) when handing off to long-running tasks.
  • adcp spec repo — tighten create-media-buy-response.json so the validator's deepest-error path produces a useful message on submitted-branch failures.

Closes #570 (partial — schema and platform fixes tracked separately).

Notes for reviewer

  • src/adcp/__init__.py has been updated to re-export CreateMediaBuySubmittedResponse at the top level (added in 54275f29). Earlier draft noted it was omitted — that's now resolved.
  • No salesagent or schemas/cache/ files were touched.

Test plan

  • ruff check src/ — All checks passed
  • mypy src/adcp/ — Success: no issues found in 783 source files
  • pytest tests/ -v -k "media_buy or create_media_buy or public_api or type_aliases" — 184 passed, 5 skipped
  • pytest tests/ (full suite) — 4170 passed, 24 skipped, 1 xfailed
  • Pre-commit hooks (black, ruff, mypy, bandit, etc.) all pass

…+ add Submitted alias

The wire CreateMediaBuyResponse is a 3-branch anyOf union (success / error /
submitted), but the SDK previously typed PlatformHandler.create_media_buy as
returning only the success branch and exposed semantic aliases for only two
of the three branches.

Two SDK-side fixes:

1. PlatformHandler.create_media_buy is annotated `-> CreateMediaBuyResponse`
   (the union) and the internal cast() matches. The handoff path legitimately
   returns the Submitted envelope; the prior success-only annotation
   misrepresented the contract.

2. Add CreateMediaBuySubmittedResponse semantic alias (-> CreateMediaBuyResponse3,
   the status='submitted'+task_id branch) alongside the existing
   CreateMediaBuySuccessResponse / CreateMediaBuyErrorResponse aliases.
   Exported from adcp.types.

Tests cover alias identity, model_validate of a spec-compliant submitted
payload, the missing-task_id rejection path, and the handler return-type
annotation.

The submitted-branch payload validation fix (the actual FastMCP error
trigger) belongs in salesagent; the schema tightening belongs in the spec
repo. Both are filed separately.

Closes #570 (partial — schema and platform fixes tracked separately).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Spec-repo follow-up filed for the third leg of the original triage plan (schema tightening on the submitted-branch file): adcontextprotocol/adcp#4077

@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Noted — adcontextprotocol/adcp#4077 logged as the spec-side follow-up for the submitted-branch schema tightening. No action needed on this PR's diff.


Generated by Claude Code

@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Thanks — adcontextprotocol/adcp#4077 logged for the spec-side tightening is a clean handoff. This PR's diff is ready for review as-is.


Generated by Claude Code

…e-export Submitted

The local CreateMediaBuyResponse alias in guards.py was a stale 2-branch
union (Success | Error). PlatformHandler.create_media_buy now returns the
full 3-branch union including the async Submitted envelope, so callers
running is_create_media_buy_success on a handler result would silently
mis-classify the submitted branch.

Changes:
- guards.py: widen CreateMediaBuyResponse to include
  CreateMediaBuySubmittedResponse; add is_create_media_buy_submitted
  TypeGuard; update success/error guards to short-circuit on the
  submitted branch (it's neither sync success nor terminal error).
- adcp/__init__.py: re-export CreateMediaBuySubmittedResponse alongside
  the existing Success/Error aliases.
- tests: cover the new submitted guard (positive + success/error
  negatives); update public_api snapshot for the top-level re-export.
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Pushed follow-up commit 54275f29 addressing two reviewer notes:

1. guards.py 3-branch widening (the silent-misclassify bug)
The local CreateMediaBuyResponse alias in src/adcp/types/guards.py was stale — a 2-branch Success | Error union. With the handler return type now widened, callers using is_create_media_buy_success on a handler result would have classified the async submitted envelope as success.

  • Widened the local alias to include CreateMediaBuySubmittedResponse.
  • Added is_create_media_buy_submitted (status-discriminator + task_id check), exported via __all__.
  • Updated the existing success/error guards to short-circuit on the submitted branch — submitted is neither a sync success nor a terminal error (it can carry advisory non-blocking errors).
  • Tests: positive case (submitted payload returns True from the new guard, False from the other two) + negative cases (success and error payloads return False from the new guard).

2. Top-level CreateMediaBuySubmittedResponse re-export
Added to src/adcp/__init__.py (both the import block and __all__) alongside the existing Success/Error aliases. Snapshot at tests/fixtures/public_api_snapshot.json updated to match.

All checks green: ruff check src/, mypy src/adcp/ (783 files, no issues), and pytest -k "guard or media_buy or public_api" (153 passed).

@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Noted — 54275f29 looks complete: guards widened to all 3 branches, is_create_media_buy_submitted added, and the top-level re-export landed. All checks green. Ready for reviewer sign-off.


Generated by Claude Code

@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Acknowledged. Also updated the PR body — the "Notes for reviewer" section had a stale line saying the top-level __init__.py re-export was omitted; 54275f29 did add it, so that note was misleading for reviewers. Body now reflects the actual state.


Triaged by Claude Code


Generated by Claude Code

@bokelley bokelley merged commit f7960ed into main May 4, 2026
16 checks passed
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