Skip to content

fix(examples): multi_platform_seller passes storyboards (drops continue-on-error)#508

Merged
bokelley merged 1 commit into
mainfrom
bokelley/fix-multi-platform-storyboards
May 4, 2026
Merged

fix(examples): multi_platform_seller passes storyboards (drops continue-on-error)#508
bokelley merged 1 commit into
mainfrom
bokelley/fix-multi-platform-storyboards

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 4, 2026

Summary

The two mock platforms behind PlatformRouter (MockGuaranteedPlatform, MockNonGuaranteedPlatform) have been silently failing the AdCP storyboard suite since PR #490 — masked by continue-on-error: true on the CI job. This PR aligns the mocks with the v3.0.6 wire contract and drops the masking so future drift blocks merge.

Pre-fix counts (from .storyboard-baseline/, last main artifact):

Tenant Passed Failed Skipped
tenant-a (MockGuaranteedPlatform) 7 8 44
tenant-b (MockNonGuaranteedPlatform) 12 6 41

The high skip counts are cascades — once create_media_buy failed, every downstream step in the same scenario was skipped. Fixing the create path unblocks dozens more.

Failure types and fixes

# Failure type Tenants Fix
1 INVALID_REQUEST[packages[].products]: package.products is empty tenant-a (×6) create_media_buy reads packages[].product_id (singular, per schemas/3.0.6/media-buy/package-request.json) and resolves against the in-memory catalog. budget is a flat number, not nested.
2 'media_buys' is a required property on get_media_buys tenant-b Implemented get_media_buys on both mocks (was missing). Echoes back persisted targeting_overlay so the inventory_list_targeting round-trip passes.
3 sync_creatives response oneOf validation failed both (only surfaces on tenant-b because tenant-a fails earlier) Per-item shape now {creative_id, action: "created", status: "approved"} per schemas/3.0.6/creative/sync-creatives-response.json. Old approval_status key wasn't a valid field.
4 pending_creatives_to_start: expected pending_creatives, got active tenant-b Non-guaranteed mock now starts buys in pending_creatives and advances pending_creatives → pending_start → active after creatives sync. (The state machine doesn't permit a direct jump.)
5 TERMS_REJECTED not raised on aggressive measurement_terms both Both mocks check measurement_terms.billing_measurement and raise TERMS_REJECTED when max_variance_percent < 5 or measurement_window is outside c3/c7 — mirrors examples/seller_agent.py.
6 update_unknown_package: succeeded silently (actual: null) tenant-b update_media_buy now validates packages[].package_id against the buy's persisted packages and raises PACKAGE_NOT_FOUND (recovery=correctable). Also persists targeting_overlay / measurement_terms patches.
7 MEDIA_BUY_NOT_FOUND recovery semantics both Changed from terminal to correctable — the buyer can retry with a fresh id.

Skipped (correctly) — proposal_finalize

Not in summary.storyboards_executed for either tenant. Gated by the media_buy.supports_proposals capability flag, which neither mock declares because neither implements the proposal lifecycle. The gate stays as-is — keeping the mocks honest about what they support, not faking storyboard pass. Upstream JS team will revisit when ProposalManager v1.5 ships.

CI gate

Drops continue-on-error: true from the storyboard-multi-platform-seller job. Future regressions in the mocks or the spec wire shape now block merge. Artifact upload still runs on if: always() so failures remain debuggable.

Test plan

  • ruff check src/ examples/multi_platform_seller/ — clean
  • mypy src/adcp/ — clean (777 source files)
  • pytest tests/ -x -q — 3760 passed, 32 skipped, 1 xfailed
  • Functional smoke: every error path (PRODUCT_NOT_FOUND, TERMS_REJECTED, PACKAGE_NOT_FOUND, MEDIA_BUY_NOT_FOUND) raises the right code; happy path lifecycle (pending_creatives → pending_start → active after sync) advances correctly; targeting_overlay round-trips through get_media_buys.
  • CI storyboard run on tenant-a + tenant-b — verifies the gate now passes (the JS storyboard runner isn't installed in the local sandbox; CI is the verification path).

Notes / upstream observations

  • The MCP transport projects AdcpError to a text-only payload (code[field]: message) instead of populating structuredContent.adcp_error.code. This is documented as a known limitation in src/adcp/server/translate.py (# Bridge, not endpoint block) — the storyboard runner's /adcp_error/code JSON-pointer check therefore reports actual: mcp_error for some error-code assertions even when the platform raises the right AdcpError. Out of scope for this PR; tracked by the FastMCP _make_error_result migration.

🤖 Generated with Claude Code

@bokelley @claude
fix(examples/multi_platform_seller): align mocks with wire contract; …
e5e325e 
…storyboard gate now blocking

The two mock platforms behind PlatformRouter (MockGuaranteedPlatform,
MockNonGuaranteedPlatform) were silently failing the AdCP storyboard
suite since PR #490 — masked by ``continue-on-error: true`` on the CI
job. Pre-fix counts: tenant-a 7 passed / 8 failed / 44 skipped (cascade
from create failures), tenant-b 12 passed / 6 failed / 41 skipped.

Wire-contract alignments:

* ``create_media_buy``: read ``packages[].product_id`` (singular,
  per ``schemas/3.0.6/media-buy/package-request.json``) instead of
  the non-existent ``packages[].products`` array. ``budget`` is a
  flat number, not a nested ``{total: ...}``.
* ``sync_creatives``: response items use ``{creative_id, action:
  "created", status: "approved"}`` per
  ``schemas/3.0.6/creative/sync-creatives-response.json``. The old
  ``approval_status`` key wasn't a valid field.
* ``get_media_buys``: implemented on both mocks (was missing,
  causing tenant-b's "'media_buys' is a required property"). Echoes
  ``targeting_overlay`` persisted at create / update time so the
  ``inventory_list_targeting`` storyboard can verify round-trip.
* ``update_media_buy``: validates ``packages[].package_id`` against
  the buy's persisted packages and raises ``PACKAGE_NOT_FOUND``
  (recovery=correctable) on unknown ids; persists ``targeting_overlay``
  + ``measurement_terms`` patches for round-trip.
* Both mocks now check buyer-proposed ``measurement_terms`` and raise
  ``TERMS_REJECTED`` when ``max_variance_percent < 5`` or
  ``measurement_window`` is outside ``c3``/``c7`` — mirrors the v3
  reference seller's policy in ``examples/seller_agent.py``.
* MockNonGuaranteedPlatform now starts buys in ``pending_creatives``
  and advances to ``active`` via ``pending_start`` only after
  creatives sync, so the ``pending_creatives_to_start`` storyboard
  passes. The state machine doesn't permit a direct
  pending_creatives → active jump.
* ``MEDIA_BUY_NOT_FOUND`` recovery changed from ``terminal`` to
  ``correctable`` to match spec semantics — the buyer can retry with
  a fresh id.

Storyboards skipped by capability gate (correct behavior, no change):
``proposal_finalize`` is gated by ``media_buy.supports_proposals``.
Neither mock declares it because neither implements the proposal
lifecycle. The gate stays as-is until ProposalManager v1.5 ships.

CI: drops ``continue-on-error: true`` from the
``storyboard-multi-platform-seller`` job. Future drift in the mocks
or the spec now blocks merge. Artifact upload still runs on
``if: always()`` so failures remain debuggable.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley mentioned this pull request May 4, 2026
Closed
5 tasks
@bokelley bokelley merged commit 863bb93 into main May 4, 2026
15 checks passed
@bokelley bokelley mentioned this pull request May 4, 2026
Closed
3 tasks
@github-actions github-actions Bot mentioned this pull request May 4, 2026
Merged
@bokelley bokelley mentioned this pull request May 4, 2026
Merged
5 tasks
bokelley added a commit that referenced this pull request May 4, 2026
@bokelley @claude
fix(examples/multi_platform_seller): list_creatives populates query_s…
c7cfe6f 
…ummary (closes #510) (#521)

Both MockGuaranteedPlatform and MockNonGuaranteedPlatform were
missing list_creatives entirely. The storyboard step
``creative_fate_after_cancellation/list_creatives_before_cancel``
fails for both tenants on the post-#508 artifact because the wire
contract requires query_summary on every list_creatives response.

Fix: each mock now persists creatives at sync_creatives time into
an in-memory library keyed by creative_id, and list_creatives
returns them with the required ``query_summary`` (total_matching,
returned) and ``pagination`` (has_more=False, total_count) blocks.

Per-item Creative shape projects to the
schemas/3.0.6/creative/list-creatives-response.json contract:
{creative_id, name, format_id, status: "approved", created_date,
updated_date}. Auto-approval mirrors the existing sync_creatives
policy on both mocks.

Pagination is intentionally trivial (returns the full library) —
the storyboard catalog is small and modeling cursor-based paging
adds noise without illustrating new platform-shape concerns.

Validated: ListCreativesResponse.model_validate accepts the output
on both mocks; full pytest suite stays green (3760 passed).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley mentioned this pull request May 4, 2026
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@bokelley