feat(decisioning): wire create_media_buy_store into PlatformHandler dispatch (closes #462)

[bokelley]

Summary

Wires the already-merged create_media_buy_store helper into the framework dispatch path. Adopters now opt in via a kwarg on create_adcp_server_from_platform instead of hand-rolling targeting_overlay persistence + echo per adapter.

• create_media_buy — persist_from_create rides the existing on_complete seam, chained with the proposal v1.5 hook when both are wired, so both sync and HITL completions persist.
• update_media_buy — merge_from_update on on_complete, honoring the spec contract that targeting_overlay echoes from the most recent create_media_buy or update_media_buy.
• get_media_buys — backfill called inline before return.

New _to_store_dict helper normalizes Pydantic → JSON-compatible dict via model_dump(mode='json', exclude_none=False). Adopters see the same shape that would go on the wire (AnyUrl → str, datetime → ISO-8601 str), and explicit nulls survive the merge contract (None = "clear this field").

API shape decision

The triage comment surfaced an open question: kwarg on create_adcp_server_from_platform vs. post-construction platform.media_buy_store = ... (as the issue body proposed). Went with the kwarg — matches buyer_agent_registry, webhook_sender, resource_resolver, and every other framework opt-in.

handler, executor, registry = create_adcp_server_from_platform(
    platform,
    media_buy_store=create_media_buy_store(adopter_store, capabilities=platform.capabilities),
)

Test plan

• 7 new integration tests in tests/test_media_buy_store_integration.py covering persist / merge / backfill through PlatformHandler, the no-op paths (store missing, seller lacks specialism), and a full create→get round-trip
• 10 existing tests/test_media_buy_store.py unit tests still pass
• Full test suite: 4881 passed, 0 regressions
• ruff check + mypy clean on changed files
• Manual smoke through an adopter platform (defer to follow-up if needed)

Closes #462.

🤖 Generated with Claude Code

[aao-ipr-bot]

Hold. The wiring shape is right and the closure / chaining work is clean — but _to_store_dict(params) is dumping the full CreateMediaBuyRequest / UpdateMediaBuyRequest across the adopter boundary, and those requests carry webhook bearer tokens.

Must fix before merge

1. Webhook credentials leak into the adopter store. handler.py:1683-1687 and :1740-1744 call _to_store_dict(params) on the full request before handing to store.persist_from_create / store.merge_from_update. The dump uses model_dump(mode='json', exclude_none=False, by_alias=False) with no scrub. CreateMediaBuyRequest.push_notification_config.authentication.credentials, .reporting_webhook.authentication.credentials, and .artifact_webhook.authentication.credentials are all ≥32-char Bearer / HMAC-SHA256 secrets (src/adcp/types/generated_poc/core/push_notification_config.py:27, reporting_webhook.py:28, create_media_buy_request.py:216-231). The adopter store sees the full request including those tokens. The obvious adopter pattern — persist the dict for audit/replay — silently archives buyer-supplied webhook secrets. _CREDENTIAL_SHAPED_KEY_SUFFIXES (dispatch.py:413-422) doesn't help here because that gate is ctx_metadata-only. Fix: project to a minimal {packages, media_buy_id, buyer_ref} dict before dumping, OR extend strip_credentials_from_wire_result / _scrub_dict (account_projection.py:345-436) to cover the three webhook authentication.credentials paths and run it on the dict before crossing the boundary. The store only needs packages[].targeting_overlay — give it that and nothing else. security-reviewer: HIGH.

Things I checked

• _to_store_dict (handler.py:807-826): by_alias=False is correct — verified no Field(alias=...) on CreateMediaBuyRequest, UpdateMediaBuyRequest, Package, or any targeting field. mode='json' normalizes AnyUrl / datetime for adopters who persist as JSON.
• Closure capture on the chained create_media_buy hook (handler.py:1675-1689): prior_on_complete, captured_store, captured_account_id are bound at definition. No late-binding bug. If _finalize_consumption_hook raises, the persist hook exits early — correct ordering for the happy path.
• HITL parity claim: dispatch.py _project_handoff fires on_complete with the typed result for both create and update handoffs, so the PR description holds.
• Specialism gating: _OVERLAY_ECHO_SPECIALISMS = {property-lists, collection-lists} matches schemas/cache/3.0/enums/specialism.json. No audience-lists exists in the canonical enum; gate is correct and conservative.
• Echo contract: schemas/cache/3.0/media-buy/get-media-buys-response.json matches the L1730 comment verbatim — "echoed from the most recent create_media_buy or update_media_buy."
• Tenant isolation: account.id scoping (handler.py:1678, :1736, :1835) matches the canonical scope key used by ProposalStore and TaskRegistry; no new cross-tenant leak. _resolve_account synthesizes f\"{account_id}:{principal}\" so the test assertion of \"acme:anonymous\" is right.
• ctx_metadata: none of the new wiring touches it. CLAUDE.md echo warning not implicated.
• Test suite: 7 new integration tests cover the persist / merge / backfill paths and the no-op specialism path. Conventional-commit prefix feat(decisioning): is correct — the kwarg defaults to None so this is additive, not breaking.

Follow-ups (non-blocking once the credential leak is fixed)

• Persist-failure / proposal-finalize ordering (handler.py:1680-1689). If persist_from_create raises after _finalize_consumption_hook marked the proposal CONSUMED, dispatch.py:1524-1530 fires _release_reservation_hook against an already-consumed reservation. The media buy was created in the adapter, the buyer sees a failed call, and proposal accounting goes divergent. The store is best-effort backfill, not a source of truth — wrap the persist call in try/except with logger.exception so a downstream persistence outage doesn't unwind a successful inventory commit.
• Backfill mutation skips the dispatcher's credential strip (handler.py:1834-1836). _invoke_platform_method returned a credential-stripped result (dispatch.py:1538), but backfill then mutates and we return the mutated dict. An adopter store backed by the same DB row as the seller's governance auth could re-inject stripped fields. Re-run strip_credentials_from_wire_result(\"get_media_buys\", result) after backfill returns. Defense in depth.
• Typed-vs-dict return contract (handler.py:1835). With a store wired, the return is always a dict; without one, it's whatever the platform returned. cast(\"GetMediaBuysResponse\", result) papers over the divergence. Tolerable today; future middleware doing isinstance(result, GetMediaBuysResponse) would break depending on whether a store happens to be wired. Either always normalize to dict, or re-validate into the typed model post-backfill.
• Merge semantics in the test fixture (tests/test_media_buy_store_integration.py:90-93). The recording store's existing.update(overlay) is key-level merge — media-buy/package-update.json says targeting_overlay uses replacement semantics ("the full overlay replaces the previous one"). A buyer sending {collection_list: X} to a package previously {property_list: Y, collection_list: Z} would see {property_list: Y, collection_list: X} from this fixture — spec says they should see {collection_list: X}. Framework code is unaffected; the test happens to encode an adopter pattern the spec doesn't sanction. Either fix the fixture to do full-replacement, or annotate it as deliberately non-spec.

Minor nits

1. Test response subscript on a typed cast (tests/test_media_buy_store_integration.py:312, 458). resp[\"media_buys\"][0]... works because the fixture platform returned a dict and the store-backfill normalization keeps it a dict; future readers seeing cast(\"GetMediaBuysResponse\", result) upstream may think the response is the typed model. One inline comment would prevent the confusion.

Flip to approve once the credential leak fix lands and tests cover the projection so we don't regress it.

[bokelley]

Thanks @aao-ipr-bot — pushed 97e650ae addressing the blocker and both flagged follow-ups.

Blocker: credential leak

Replaced the blanket _to_store_dict(params) with three whitelist projection helpers in handler.py:

• _project_create_request_for_store — projects to {packages: [{buyer_ref, targeting_overlay}, ...]} and nothing else
• _project_create_response_for_store — projects to {media_buy_id, packages: [{buyer_ref, package_id, targeting_overlay}, ...]}
• _project_update_patch_for_store — projects to {packages: [{package_id, targeting_overlay}, ...], new_packages: [{buyer_ref, package_id, targeting_overlay}, ...]}

Whitelist > blacklist exactly as you flagged — future schema additions of credential-shaped fields can't reintroduce the leak. Pydantic's model_dump(include=...) does the projection, with parallel manual paths for dict inputs.

Follow-ups addressed in the same commit

1. Persist-failure / proposal-finalize ordering — both persist_from_create and merge_from_update now run inside try/except Exception → logger.exception. Comment at the create-shim hook explains the ordering invariant: the create has already succeeded in the adapter and the proposal hook (if wired) has already marked the proposal CONSUMED, so unwinding via the exception path would surface a buyer-visible failure on a successful inventory commit.
2. Post-backfill credential strip — get_media_buys now re-runs strip_credentials_from_wire_result("get_media_buys", result) after backfill mutates the dict. Defense in depth against a buggy or malicious store re-injecting governance_agents[].authentication / billing_entity.bank.

Regression tests

Added four tests in tests/test_media_buy_store_integration.py:

• test_create_media_buy_does_not_leak_webhook_credentials_to_store — asserts the persist call's request_dict.keys() is exactly {"packages"} and that the bearer token literal is nowhere in the flattened structure
• test_update_media_buy_does_not_leak_webhook_credentials_to_store — same shape check for the merge call's patch
• test_create_media_buy_persist_failure_does_not_break_create_path — uses a _RaisingStore to verify the create succeeds normally despite a store outage
• test_get_media_buys_strips_credentials_re_injected_by_store — uses a _ReinjectingStore that re-injects governance_agents[].authentication during backfill and asserts the field is gone in the returned response

Other notes

• Merge semantics in the fixture (follow-up chore(main): release 1.0.0 #4): the recording store's key-level update() mirrors the existing on-main reference impl in tests/test_media_buy_store.py:214-237 (which also encodes key-merge via _test_merge_from_update_preserves_prior_fields_when_patch_omits_them). If the spec really mandates full replacement per package-update.json, the on-main reference impl is also wrong — happy to file a separate issue once we confirm the spec read, but I'd rather not change two test fixtures' semantics inline with a security fix.
• Typed-vs-dict return contract (follow-up chore(main): release 0.1.2 #3): noted, leaving for a separate cleanup PR — same reasoning, this PR is already scope-creeping.
• Nit on test response subscript: added inline comments in the new credential-leak tests explaining the dict shape.

Suite: 4894 passed, 0 regressions. ruff + mypy clean.