diff --git a/CHANGELOG.md b/CHANGELOG.md index f04026df4..4edd3ffbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## Unreleased + +### ⚠ BREAKING CHANGES + +* **types:** `CreativeAsset.format_kind`, `Creative.format_kind`, and + `CreativeManifest.format_kind` now reject values outside `CanonicalFormatKind`. + Valid strings still normalize to enum members. Inputs remain strict, while + buyer manifest readback preserves future kinds through private tolerant views + in delivery, preview, build (including nested variants and async results), and + trusted-match offers. Direct `Creative` and `CreativeAsset` response fields + remain strict. Update stored and incoming creative kinds using the + [migration guide](docs/canonical-format-kinds-migration.md). + This closes [#1241](https://github.com/adcontextprotocol/adcp-client-python/issues/1241). + ## [8.0.0-rc.2](https://github.com/adcontextprotocol/adcp-client-python/compare/v8.0.0-rc.1...v8.0.0-rc.2) (2026-09-29) diff --git a/docs/canonical-format-kinds-migration.md b/docs/canonical-format-kinds-migration.md new file mode 100644 index 000000000..1a2c26a38 --- /dev/null +++ b/docs/canonical-format-kinds-migration.md @@ -0,0 +1,81 @@ +# Canonical creative format-kind validation + +`CreativeAsset.format_kind`, `Creative.format_kind`, and +`CreativeManifest.format_kind` now reject strings outside `CanonicalFormatKind`. +This is a breaking change from releases through 8.0.0-rc.2, which accepted and +preserved arbitrary strings in these fields. It aligns their enum validation +with the pinned `core/canonical-format-kind.json` schema. + +Recognized wire strings still normalize to enum members and serialize to their +original string values. The public type stubs and validation JSON Schema now +describe the same closed set. `CreativeAsset` and `Creative` still require a +non-null kind. `CreativeManifest` retains its optional `None` default for model +composition; a complete wire manifest must also satisfy the versioned schema's +identity and asset requirements. + +## Updating callers + +Use a recognized canonical kind when validating creative data: + +```python +from adcp.types import CanonicalFormatKind, CreativeAsset + +creative = CreativeAsset.model_validate( + { + "creative_id": "creative-1", + "name": "Product image", + "format_kind": "image", + "assets": {}, + } +) +kind: CanonicalFormatKind = creative.format_kind +assert kind is CanonicalFormatKind.image +``` + +Validate stored values before upgrading workflows that read existing creatives. +Unknown values such as `"totally_bogus"` now raise Pydantic `ValidationError` +during construction, `model_validate`, and `model_validate_json`, including +nested creative and manifest input fields. Correct each value to the canonical kind +whose contract the creative satisfies. Applications receiving a kind introduced +by a newer protocol version need an SDK version that supports that kind. + +Adopter-defined formats use the existing `custom` kind with a corresponding +format declaration's `format_shape` and `format_schema`. Use it when the creative +conforms to that custom contract, rather than as a fallback for unknown values. + +The public input annotations are `CanonicalFormatKind` for `CreativeAsset` and +`Creative`, and `CanonicalFormatKind | None` for `CreativeManifest`. Remove +application branches that treat kinds validated by these types as arbitrary strings. + +## Buyer manifest readback + +The SDK preserves unknown `format_kind` strings when reading manifests returned +by another agent. Known kinds still normalize to enum members. This applies to +all manifest-bearing response paths: + +| Response | Manifest path | +| --- | --- | +| Canonical and legacy creative delivery | `creatives[].variants[].manifest` | +| `LegacyPreviewCreativeResponse3` | `manifest` | +| `LegacyBuildCreativeResponse1` | `creative_manifest` | +| `LegacyBuildCreativeResponse3` | `creative_manifests[]` | +| `LegacyBuildCreativeResponse4` | `creatives[].variants[].creative_manifest` | +| Trusted-match router and provider responses | `offers[].creative_manifest` | + +Completed async build and preview results follow the same rule, including the +webhook result wrapper. `DeliveryCreative.format_kind` also remains +`CanonicalFormatKind | str | None`. This is tolerant SDK response parsing; it +does not widen the versioned wire schema's enum. + +Direct `Creative.format_kind` and `CreativeAsset.format_kind` fields remain +strict, including `ListCreativesResponse.creatives[].format_kind`. The tolerance +applies to response manifests, not to these directly embedded creative types. + +Responses use private manifest views, with private enclosing variants and offers +where needed. These types are reachable through responses but are not exported +from `adcp.types`. The public and generated `CreativeManifest` types remain +strict inputs. To reuse a returned manifest as input, dump it and validate it +with `CreativeManifest.model_validate(returned_manifest.model_dump())`. An +unknown kind fails this validation; choose a supported kind or upgrade the SDK +before submitting it. Passing the tolerant instance directly also cannot bypass +the input validator. diff --git a/src/adcp/types/_forward_compat.py b/src/adcp/types/_forward_compat.py index c3eb37429..d080c8e13 100644 --- a/src/adcp/types/_forward_compat.py +++ b/src/adcp/types/_forward_compat.py @@ -30,24 +30,30 @@ from __future__ import annotations import json -from copy import copy +from collections.abc import Callable +from copy import copy, deepcopy from functools import partial +from types import GenericAlias from typing import Annotated, Any, cast, get_args from pydantic import ( BaseModel, + ConfigDict, Field, GetCoreSchemaHandler, GetPydanticSchema, SerializerFunctionWrapHandler, ValidationError, ValidatorFunctionWrapHandler, + create_model, + model_validator, ) from pydantic.fields import FieldInfo from pydantic.json_schema import SkipJsonSchema from pydantic_core import CoreSchema, InitErrorDetails, core_schema from adcp.types.aliases import FormatAssetUnion, GroupFormatAssetUnion, RepeatableAssetGroup +from adcp.types.base import AdCPBaseModel from adcp.types.canonical_creative import PackageRequest as PublicPackageRequest from adcp.types.canonical_creative import PackageUpdate as PublicPackageUpdate from adcp.types.generated_poc.bundled.protocol.get_adcp_capabilities_response import ( @@ -65,16 +71,35 @@ from adcp.types.generated_poc.bundled.protocol.get_adcp_capabilities_response import ( PublisherDomain as BundledPublisherDomain, ) +from adcp.types.generated_poc.core.async_response_data import AdcpAsyncResponseData from adcp.types.generated_poc.core.canonical_format_kind import CanonicalFormatKind from adcp.types.generated_poc.core.canonical_product import PublisherDomain from adcp.types.generated_poc.core.creative_manifest import CreativeManifest +from adcp.types.generated_poc.core.creative_variant import CreativeVariant from adcp.types.generated_poc.core.format import Format +from adcp.types.generated_poc.core.mcp_webhook_payload import McpWebhookPayload from adcp.types.generated_poc.core.media_buy_features import MediaBuyFeatures from adcp.types.generated_poc.core.targeting import TargetingOverlay from adcp.types.generated_poc.core.targeting_input import TargetingOverlayInput +from adcp.types.generated_poc.core.version_envelope import AdcpVersionEnvelope from adcp.types.generated_poc.creative.get_creative_delivery_response import ( Creative as DeliveryCreative, ) +from adcp.types.generated_poc.creative.get_creative_delivery_response import ( + GetCreativeDeliveryResponse, +) +from adcp.types.generated_poc.creative.preview_creative_response import PreviewCreativeResponse3 +from adcp.types.generated_poc.media_buy.build_creative_response import ( + BuildCreativeResponse1, + BuildCreativeResponse3, + BuildCreativeResponse4, +) +from adcp.types.generated_poc.media_buy.build_creative_response import ( + Creative as BuildCreative, +) +from adcp.types.generated_poc.media_buy.build_creative_response import ( + Variant as BuildCreativeVariant, +) from adcp.types.generated_poc.media_buy.create_media_buy_request import CreateMediaBuyRequest from adcp.types.generated_poc.media_buy.package_control import PackageControl from adcp.types.generated_poc.media_buy.package_request import PackageRequest @@ -84,6 +109,13 @@ AcceptancePolicyDiscovery, PrimaryCountry, ) +from adcp.types.generated_poc.trusted_match.context_match_response import ( + ContextMatchResponseRouterPublisher, +) +from adcp.types.generated_poc.trusted_match.offer import Offer +from adcp.types.generated_poc.trusted_match.provider_context_match_response import ( + ContextMatchResponseProviderRouter, +) _OpenCanonicalFormatKind = Annotated[ CanonicalFormatKind | str, @@ -91,6 +123,92 @@ ] +class _ManifestReadbackModel(AdCPBaseModel): + """Independent from strict inputs: tolerant instances must not validate as them.""" + + model_config = ConfigDict(extra="allow") + + @model_validator(mode="before") + @classmethod + def _normalize_readback(cls, data: Any) -> Any: + if isinstance(data, AdCPBaseModel) and not isinstance(data, cls): + return data.model_dump(mode="python") + return data + + +class _VersionedManifestReadbackModel(AdcpVersionEnvelope, _ManifestReadbackModel): + """Keep the shared version envelope on build response nodes.""" + + +def _manifest_readback_clone( + name: str, + source: type[AdCPBaseModel], + overrides: dict[str, Any], + *, + validators: dict[str, Any] | None = None, +) -> type[AdCPBaseModel]: + # Copy all field constraints without inheriting the strict source model. + # A tolerant subclass would pass its parent's default instance validation. + fields: dict[str, Any] = { + key: (overrides.get(key, field.annotation), deepcopy(field)) + for key, field in source.model_fields.items() + } + return create_model( + name, + __base__=( + _VersionedManifestReadbackModel + if issubclass(source, AdcpVersionEnvelope) + else _ManifestReadbackModel + ), + __module__=__name__, + __validators__=validators, + **fields, + ) + + +def _normalize_readback_manifest(data: Any) -> Any: + # Pydantic binds the generated validator proxy to a callable at runtime. + normalize = cast(Callable[[Any], Any], CreativeManifest._coerce_standalone_assets) + return normalize(data) + + +_ReadbackCreativeManifest = _manifest_readback_clone( + "_ReadbackCreativeManifest", + CreativeManifest, + {"format_kind": _OpenCanonicalFormatKind | None}, + validators={ + # Preserve the generated manifest's standalone-asset normalization, + # without widening that input model or inheriting from it. + "_coerce_standalone_assets": model_validator(mode="before")(_normalize_readback_manifest), + }, +) +_DeliveryVariant = _manifest_readback_clone( + "_DeliveryVariant", + CreativeVariant, + {"manifest": _ReadbackCreativeManifest | None}, +) +_BuildReadbackVariant = _manifest_readback_clone( + "_BuildReadbackVariant", + BuildCreativeVariant, + {"creative_manifest": _ReadbackCreativeManifest}, +) +_BuildReadbackCreative = _manifest_readback_clone( + "_BuildReadbackCreative", + BuildCreative, + { + # This constraint is inside the optional union in the generated type, + # so it must stay on the non-None arm when replacing that annotation. + "variants": Annotated[GenericAlias(list, _BuildReadbackVariant), Field(min_length=1)] + | None, + }, +) +_ReadbackOffer = _manifest_readback_clone( + "_ReadbackOffer", + Offer, + {"creative_manifest": _ReadbackCreativeManifest | None}, +) + + def _patch_model_field(model: type[BaseModel], field_name: str, new_annotation: Any) -> None: """Replace a Pydantic model field's annotation in-place. @@ -256,22 +374,40 @@ def _apply_forward_compat() -> None: # Refresh its cached nested validator as well as the package model itself. CreateMediaBuyRequest.model_rebuild(force=True) - # Canonical format kinds are an open enum on consumer boundaries. Preserve - # values introduced by a newer protocol revision instead of rejecting the - # entire creative manifest. Known values still coerce to the StrEnum arm. + # All response manifests retain unknown future kinds. Patch the generated + # response classes themselves so public aliases and indirect wrappers agree; + # public/generated input manifests and direct Creative/CreativeAsset fields + # stay strict. Private readback nodes cannot bypass strict input validation. _patch_model_field( - CreativeManifest, + DeliveryCreative, "format_kind", _OpenCanonicalFormatKind | None, ) - CreativeManifest.model_rebuild(force=True) + _patch_model_field(DeliveryCreative, "variants", GenericAlias(list, _DeliveryVariant)) + DeliveryCreative.model_rebuild(force=True) + GetCreativeDeliveryResponse.model_rebuild(force=True) + _patch_model_field(PreviewCreativeResponse3, "manifest", _ReadbackCreativeManifest | None) + PreviewCreativeResponse3.model_rebuild(force=True) + _patch_model_field(BuildCreativeResponse1, "creative_manifest", _ReadbackCreativeManifest) + BuildCreativeResponse1.model_rebuild(force=True) _patch_model_field( - DeliveryCreative, - "format_kind", - _OpenCanonicalFormatKind | None, + BuildCreativeResponse3, "creative_manifests", GenericAlias(list, _ReadbackCreativeManifest) ) - DeliveryCreative.model_rebuild(force=True) + BuildCreativeResponse3.model_rebuild(force=True) + _patch_model_field( + BuildCreativeResponse4, "creatives", GenericAlias(list, _BuildReadbackCreative) + ) + BuildCreativeResponse4.model_rebuild(force=True) + + for response in (ContextMatchResponseRouterPublisher, ContextMatchResponseProviderRouter): + _patch_model_field(response, "offers", GenericAlias(list, _ReadbackOffer)) + response.model_rebuild(force=True) + + # These eager wrappers captured build/preview validators before the patches. + # Refresh both levels so completed task callbacks retain typed manifests. + AdcpAsyncResponseData.model_rebuild(force=True) + McpWebhookPayload.model_rebuild(force=True) _patch_model_field(Format, "assets", list[FormatAssetUnion] | None) Format.model_rebuild(force=True) diff --git a/src/adcp/types/canonical_creative.py b/src/adcp/types/canonical_creative.py index d1c1e8a3b..dcc21dbf3 100644 --- a/src/adcp/types/canonical_creative.py +++ b/src/adcp/types/canonical_creative.py @@ -588,24 +588,18 @@ def _validate_custom_shape(self) -> Format: CreativeAsset = _canonical_clone( "CreativeAsset", _CanonicalCreativeWire, - overrides={"format_kind": (_OpenCanonicalFormatKind, Field())}, + overrides={"format_kind": (CanonicalFormatKind, Field())}, ) Creative = _canonical_clone( "Creative", _CanonicalListedCreative, - overrides={"format_kind": (_OpenCanonicalFormatKind, Field())}, + overrides={"format_kind": (CanonicalFormatKind, Field())}, ) _CreativeManifestBase = _canonical_clone( "_CreativeManifestBase", _CanonicalCreativeManifestWire, - overrides={ - "format_kind": ( - _OpenCanonicalFormatKind | None, - copy.deepcopy(_CanonicalCreativeManifestWire.model_fields["format_kind"]), - ) - }, ) @@ -644,12 +638,62 @@ def wire_value(value: Any) -> Any: overrides={"manifest": (CreativeManifest | None, Field(default=None))}, ) + +_DeliveryCreativeManifestBase = _canonical_clone( + "_DeliveryCreativeManifestBase", + _CanonicalCreativeManifestWire, + overrides={ + "format_kind": ( + _OpenCanonicalFormatKind | None, + copy.deepcopy(_CanonicalCreativeManifestWire.model_fields["format_kind"]), + ) + }, +) + + +class _DeliveryCreativeManifest(_DeliveryCreativeManifestBase): + """Tolerant served output, deliberately not a subtype of the strict input.""" + + @model_validator(mode="before") + @classmethod + def _normalize_readback(cls, data: Any) -> Any: + if isinstance(data, AdCPBaseModel) and not isinstance(data, cls): + data = data.model_dump(mode="python") + return CreativeManifest._normalize_standalone_assets(data) + + +_DeliveryCreativeVariantBase = _canonical_clone( + "_DeliveryCreativeVariantBase", + _LegacyCreativeVariant, + overrides={ + "manifest": ( + _DeliveryCreativeManifest | None, + copy.deepcopy(_LegacyCreativeVariant.model_fields["manifest"]), + ) + }, +) + + +class _DeliveryCreativeVariant(_DeliveryCreativeVariantBase): + """A delivery row whose rendered manifest may use a future format kind.""" + + @model_validator(mode="before") + @classmethod + def _normalize_readback(cls, data: Any) -> Any: + if isinstance(data, AdCPBaseModel) and not isinstance(data, cls): + return data.model_dump(mode="python") + return data + + DeliveryCreative = _canonical_clone( "DeliveryCreative", _LegacyDeliveryCreative, overrides={ "format_kind": (_OpenCanonicalFormatKind | None, Field(default=None)), - "variants": (list[CreativeVariant], Field()), + "variants": ( + list[_DeliveryCreativeVariant], + copy.deepcopy(_LegacyDeliveryCreative.model_fields["variants"]), + ), }, ) diff --git a/src/adcp/types/canonical_creative.pyi b/src/adcp/types/canonical_creative.pyi index fdbf569e0..8879b862b 100644 --- a/src/adcp/types/canonical_creative.pyi +++ b/src/adcp/types/canonical_creative.pyi @@ -76,25 +76,32 @@ class Product(CanonicalBoundaryModel): class CreativeAsset(CanonicalBoundaryModel): creative_id: str - format_kind: CanonicalFormatKind | str + format_kind: CanonicalFormatKind format_option_ref: Any class Creative(CanonicalBoundaryModel): creative_id: str - format_kind: CanonicalFormatKind | str + format_kind: CanonicalFormatKind format_option_ref: Any class CreativeManifest(CanonicalBoundaryModel): - format_kind: CanonicalFormatKind | str | None = ... + format_kind: CanonicalFormatKind | None = ... assets: dict[str, Any] class CreativeVariant(CanonicalBoundaryModel): manifest: CreativeManifest | None +class _DeliveryCreativeManifest(CanonicalBoundaryModel): + format_kind: CanonicalFormatKind | str | None = ... + assets: dict[str, Any] + +class _DeliveryCreativeVariant(CanonicalBoundaryModel): + manifest: _DeliveryCreativeManifest | None + class DeliveryCreative(CanonicalBoundaryModel): creative_id: str format_kind: CanonicalFormatKind | str | None - variants: list[CreativeVariant] + variants: list[_DeliveryCreativeVariant] class CreativeFilters(CanonicalBoundaryModel): ... class ProductFilters(CanonicalBoundaryModel): ... diff --git a/tests/test_delivery_manifest_readback.py b/tests/test_delivery_manifest_readback.py new file mode 100644 index 000000000..c032a980a --- /dev/null +++ b/tests/test_delivery_manifest_readback.py @@ -0,0 +1,199 @@ +"""Delivery manifest tolerance must not weaken creative input validation (#1241).""" + +from __future__ import annotations + +import json + +import pytest +from pydantic import ValidationError + +import adcp.types as public_types +from adcp.protocols.mcp import MCPAdapter +from adcp.types import ( + CanonicalFormatKind, + CreativeManifest, + CreativeVariant, + GetCreativeDeliveryResponse, + ImageContent, + LegacyGetCreativeDeliveryResponse, +) +from adcp.types.core import AgentConfig, Protocol, TaskResult, TaskStatus +from adcp.types.generated_poc.core.creative_manifest import ( + CreativeManifest as WireCreativeManifest, +) +from adcp.types.generated_poc.core.creative_variant import ( + CreativeVariant as WireCreativeVariant, +) + +FUTURE_KIND = "future_canonical_format" +RESPONSE_MODELS = (GetCreativeDeliveryResponse, LegacyGetCreativeDeliveryResponse) + + +def delivery_payload(): + return { + "currency": "USD", + "reporting_period": { + "start": "2026-09-01T00:00:00Z", + "end": "2026-09-02T00:00:00Z", + }, + "creatives": [ + { + "creative_id": "creative-1", + "format_kind": FUTURE_KIND, + "variants": [ + { + "variant_id": "variant-known", + "manifest": {"assets": {}, "format_kind": "image"}, + "impressions": 3, + }, + { + "variant_id": "variant-future", + "manifest": { + "assets": {}, + "format_kind": FUTURE_KIND, + "vendor_annotation": {"notes": ["preserved"]}, + }, + "locale_variant_id": "fr-FR", + "impressions": 7, + }, + ], + } + ], + } + + +@pytest.mark.parametrize("model", RESPONSE_MODELS, ids=["canonical", "legacy"]) +@pytest.mark.parametrize("json_input", [False, True], ids=["python", "json"]) +def test_delivery_response_round_trips_unknown_nested_manifest(model, json_input): + payload = delivery_payload() + response = ( + model.model_validate_json(json.dumps(payload)) + if json_input + else model.model_validate(payload) + ) + creative = response.creatives[0] + assert creative.format_kind == FUTURE_KIND + assert creative.variants[0].manifest.format_kind is CanonicalFormatKind.image + variant = creative.variants[1] + assert variant.manifest.format_kind == FUTURE_KIND + assert variant.impressions == 7 + assert variant.locale_variant_id == "fr-FR" + + wire = json.loads(response.model_dump_json()) + manifest = wire["creatives"][0]["variants"][1]["manifest"] + assert manifest["format_kind"] == FUTURE_KIND + assert manifest["vendor_annotation"] == {"notes": ["preserved"]} + reparsed = model.model_validate(wire) + assert reparsed.creatives[0].variants[1].manifest.format_kind == FUTURE_KIND + + +@pytest.mark.parametrize("model", RESPONSE_MODELS, ids=["canonical", "legacy"]) +@pytest.mark.parametrize("input_model", [CreativeManifest, WireCreativeManifest]) +@pytest.mark.parametrize("reuse_instance", [False, True], ids=["dump", "instance"]) +def test_delivery_manifest_cannot_bypass_strict_input(model, input_model, reuse_instance): + response = model.model_validate(delivery_payload()) + manifest = response.creatives[0].variants[1].manifest + # A tolerant subclass could bypass Pydantic's default instance revalidation. + value = manifest if reuse_instance else manifest.model_dump(mode="json") + with pytest.raises(ValidationError): + input_model.model_validate(value) + assert not isinstance(manifest, input_model) + + +@pytest.mark.parametrize("model", RESPONSE_MODELS, ids=["canonical", "legacy"]) +@pytest.mark.parametrize("input_model", [CreativeVariant, WireCreativeVariant]) +def test_delivery_variant_cannot_bypass_strict_input(model, input_model): + response = model.model_validate(delivery_payload()) + variant = response.creatives[0].variants[1] + with pytest.raises(ValidationError): + input_model.model_validate(variant) + with pytest.raises(ValidationError): + input_model.model_validate(variant.model_dump(mode="json")) + assert not isinstance(variant, input_model) + + +@pytest.mark.parametrize("model", RESPONSE_MODELS, ids=["canonical", "legacy"]) +def test_delivery_variants_keep_non_kind_constraints(model): + payload = delivery_payload() + payload["creatives"][0]["variants"][1]["locale_variant_id"] = "x" * 256 + with pytest.raises(ValidationError) as error: + model.model_validate(payload) + assert any( + item["loc"][-1] == "locale_variant_id" and item["type"] == "string_too_long" + for item in error.value.errors() + ) + + +@pytest.mark.parametrize("model", RESPONSE_MODELS, ids=["canonical", "legacy"]) +def test_delivery_only_types_are_not_top_level_exports(model): + response = model.model_validate(delivery_payload()) + variant = response.creatives[0].variants[1] + for model in (type(variant), type(variant.manifest)): + assert model.__name__ not in public_types.__all__ + assert not hasattr(public_types, model.__name__) + + +@pytest.mark.parametrize("model", RESPONSE_MODELS, ids=["canonical", "legacy"]) +@pytest.mark.parametrize("mcp_content", [False, True], ids=["dict", "mcp-text"]) +def test_adapter_keeps_successful_delivery_result(model, mcp_content): + payload = delivery_payload() + data = [{"type": "text", "text": json.dumps(payload)}] if mcp_content else payload + adapter = MCPAdapter( + AgentConfig( + id="delivery-agent", agent_uri="https://seller.example/mcp", protocol=Protocol.MCP + ) + ) + result = adapter._parse_response(TaskResult(status=TaskStatus.COMPLETED, data=data), model) + assert result.success, result.error + assert result.status is TaskStatus.COMPLETED + assert result.data.creatives[0].variants[1].manifest.format_kind == FUTURE_KIND + + +@pytest.mark.parametrize("model", RESPONSE_MODELS, ids=["canonical", "legacy"]) +@pytest.mark.parametrize("input_model", [CreativeManifest, WireCreativeManifest]) +@pytest.mark.parametrize("as_variant", [False, True], ids=["manifest", "variant"]) +def test_delivery_accepts_known_input_models(model, input_model, as_variant): + manifest = input_model(format_kind="image", assets={}) + variant = {"variant_id": "variant-known", "manifest": manifest, "impressions": 3} + if as_variant: + variant_model = CreativeVariant if input_model is CreativeManifest else WireCreativeVariant + variant = variant_model.model_validate(variant) + payload = delivery_payload() + payload["creatives"][0]["variants"][0] = variant + response = model.model_validate(payload) + served = response.creatives[0].variants[0] + assert served.manifest.format_kind is CanonicalFormatKind.image + assert served.impressions == 3 + + +@pytest.mark.parametrize("model", RESPONSE_MODELS, ids=["canonical", "legacy"]) +def test_delivery_manifest_normalizes_standalone_assets(model): + payload = delivery_payload() + image = ImageContent(url="https://example.com/image.png", width=300, height=250) + payload["creatives"][0]["variants"][1]["manifest"]["assets"] = {"image_main": image} + response = model.model_validate(payload) + assets = response.model_dump(mode="json")["creatives"][0]["variants"][1]["manifest"]["assets"] + assert assets["image_main"]["url"] == "https://example.com/image.png" + + +def test_delivery_keeps_legacy_identity_only_on_legacy_surface(): + payload = delivery_payload() + manifest = payload["creatives"][0]["variants"][1]["manifest"] + manifest.pop("format_kind") + manifest["format_id"] = {"agent_url": "https://seller.example", "id": "legacy-banner"} + legacy = LegacyGetCreativeDeliveryResponse.model_validate(payload) + wire = legacy.model_dump(mode="json") + assert wire["creatives"][0]["variants"][1]["manifest"]["format_id"]["id"] == "legacy-banner" + with pytest.raises(ValidationError, match="legacy creative identity"): + GetCreativeDeliveryResponse.model_validate(payload) + + +@pytest.mark.parametrize("model", RESPONSE_MODELS, ids=["canonical", "legacy"]) +def test_delivery_keeps_optional_manifest_and_kind_defaults(model): + payload = delivery_payload() + variants = payload["creatives"][0]["variants"] + variants[0].pop("manifest") + variants[1]["manifest"].pop("format_kind") + response = model.model_validate(payload) + assert response.creatives[0].variants[0].manifest is None + assert response.creatives[0].variants[1].manifest.format_kind is None diff --git a/tests/test_forward_compat_format_kind.py b/tests/test_forward_compat_format_kind.py index 79fe7c57f..108b982e1 100644 --- a/tests/test_forward_compat_format_kind.py +++ b/tests/test_forward_compat_format_kind.py @@ -1,10 +1,12 @@ -"""Regression tests for open-enum canonical format kinds (issue #1140).""" +"""Closed creative format kinds and tolerant delivery readback (issues #1241/#1140).""" from __future__ import annotations +import json from datetime import datetime, timezone import pytest +from jsonschema import Draft202012Validator from pydantic import ValidationError from adcp.types import ( @@ -12,10 +14,22 @@ Creative, CreativeAsset, CreativeManifest, + CreativeVariant, DeliveryCreative, Format, + GetCreativeDeliveryResponse, + SyncCreativesRequest, ) from adcp.types.aliases import DeliveryCreative as AliasDeliveryCreative +from adcp.types.creative import Creative as PartialCreative +from adcp.types.creative import CreativeAsset as PartialCreativeAsset +from adcp.types.creative import CreativeManifest as PartialCreativeManifest +from adcp.types.generated_poc.core.creative_manifest import ( + CreativeManifest as GeneratedCreativeManifest, +) +from adcp.types.generated_poc.creative.get_creative_delivery_response import ( + GetCreativeDeliveryResponse as GeneratedGetCreativeDeliveryResponse, +) FUTURE_FORMAT_KIND = "future_canonical_format" @@ -55,25 +69,170 @@ def _creative_manifest(format_kind: str) -> CreativeManifest: @pytest.mark.parametrize( "factory", - [_creative_asset, _creative, _delivery_creative, _creative_manifest], + [_creative_asset, _creative, _creative_manifest], ) -def test_unknown_format_kind_is_preserved(factory) -> None: - model = factory(FUTURE_FORMAT_KIND) +@pytest.mark.parametrize("value", [FUTURE_FORMAT_KIND, "totally_bogus", "IMAGE", ""]) +def test_unknown_format_kind_is_rejected(factory, value) -> None: + with pytest.raises(ValidationError) as error: + factory(value) - assert model.format_kind == FUTURE_FORMAT_KIND - assert type(model.format_kind) is str - assert model.model_dump(mode="json")["format_kind"] == FUTURE_FORMAT_KIND + assert [(detail["loc"], detail["type"]) for detail in error.value.errors()] == [ + (("format_kind",), "enum") + ] + + +@pytest.mark.parametrize("factory", [_creative_asset, _creative, _creative_manifest]) +@pytest.mark.parametrize("json_input", [False, True], ids=["python", "json"]) +def test_raw_creative_validation_rejects_unknown_format_kind(factory, json_input) -> None: + model = factory("image") + payload = model.model_dump(mode="json", exclude_unset=True) + payload["format_kind"] = FUTURE_FORMAT_KIND + + with pytest.raises(ValidationError) as error: + if json_input: + type(model).model_validate_json(json.dumps(payload)) + else: + type(model).model_validate(payload) + + assert error.value.errors()[0]["loc"] == ("format_kind",) + + +@pytest.mark.parametrize("factory", [_creative_asset, _creative, _creative_manifest]) +def test_creative_validation_schema_rejects_unknown_format_kind(factory) -> None: + model = factory("image") + validator = Draft202012Validator(type(model).model_json_schema()) + payload = model.model_dump(mode="json", exclude_unset=True) + validator.validate(payload) + + payload["format_kind"] = FUTURE_FORMAT_KIND + errors = list(validator.iter_errors(payload)) + assert errors + assert all(list(error.path) == ["format_kind"] for error in errors) + + +@pytest.mark.parametrize( + ("public", "partial"), + [ + (CreativeAsset, PartialCreativeAsset), + (Creative, PartialCreative), + (CreativeManifest, PartialCreativeManifest), + ], +) +def test_partial_imports_use_the_same_creative_models(public, partial) -> None: + assert public is partial + + +@pytest.mark.parametrize("factory", [_creative_asset, _creative]) +def test_creative_format_kind_remains_required_and_non_nullable(factory) -> None: + model = factory("image") + payload = model.model_dump(mode="json", exclude_unset=True) + del payload["format_kind"] + with pytest.raises(ValidationError) as missing: + type(model).model_validate(payload) + assert missing.value.errors()[0]["loc"] == ("format_kind",) + assert missing.value.errors()[0]["type"] == "missing" + + payload["format_kind"] = None + with pytest.raises(ValidationError) as null: + type(model).model_validate(payload) + assert null.value.errors()[0]["loc"] == ("format_kind",) + + +def test_manifest_format_kind_keeps_its_optional_default() -> None: + omitted = CreativeManifest(assets={}) + explicit = CreativeManifest(assets={}, format_kind=None) + assert omitted.format_kind is None + assert explicit.format_kind is None + assert "format_kind" not in omitted.model_dump(exclude_unset=True) + assert explicit.model_dump(exclude_unset=True, exclude_none=False)["format_kind"] is None + + +@pytest.mark.parametrize("json_input", [False, True], ids=["python", "json"]) +def test_sync_request_rejects_unknown_creative_format_kind(json_input) -> None: + payload = { + "account": {"account_id": "account-1"}, + "idempotency_key": "creative-sync-idempotency-1", + "creatives": [_creative_asset("image").model_dump(mode="json", exclude_unset=True)], + } + SyncCreativesRequest.model_validate(payload) + payload["creatives"][0]["format_kind"] = FUTURE_FORMAT_KIND + + with pytest.raises(ValidationError) as error: + if json_input: + SyncCreativesRequest.model_validate_json(json.dumps(payload)) + else: + SyncCreativesRequest.model_validate(payload) + assert error.value.errors()[0]["loc"] == ("creatives", 0, "format_kind") + + +def test_public_variant_rejects_unknown_manifest_format_kind() -> None: + variant = {"variant_id": "variant-1", "manifest": {"assets": {}, "format_kind": "image"}} + CreativeVariant.model_validate(variant) + variant["manifest"]["format_kind"] = FUTURE_FORMAT_KIND + + with pytest.raises(ValidationError) as error: + CreativeVariant.model_validate(variant) + assert error.value.errors()[0]["loc"][-2:] == ("manifest", "format_kind") + + +@pytest.mark.parametrize( + ("response_type", "strict_manifest_type"), + [ + (GetCreativeDeliveryResponse, CreativeManifest), + (GeneratedGetCreativeDeliveryResponse, GeneratedCreativeManifest), + ], + ids=["canonical", "generated"], +) +def test_unknown_nested_manifest_kind_round_trips_in_delivery_readback( + response_type, strict_manifest_type +) -> None: + payload = { + "currency": "USD", + "reporting_period": { + "start": "2026-09-01T00:00:00Z", + "end": "2026-09-02T00:00:00Z", + }, + "creatives": [ + { + "creative_id": "creative-1", + "format_kind": FUTURE_FORMAT_KIND, + "variants": [ + { + "variant_id": "variant-1", + "manifest": {"assets": {}, "format_kind": FUTURE_FORMAT_KIND}, + } + ], + } + ], + } + delivery = response_type.model_validate(payload) + manifest = delivery.creatives[0].variants[0].manifest + assert manifest is not None + assert type(manifest) is not strict_manifest_type + assert type(delivery.creatives[0].variants[0]) is not CreativeVariant + assert manifest.format_kind == FUTURE_FORMAT_KIND + + encoded = delivery.model_dump_json() + assert response_type.model_validate_json(encoded).model_dump(mode="json") == ( + delivery.model_dump(mode="json") + ) + + with pytest.raises(ValidationError) as error: + strict_manifest_type.model_validate(payload["creatives"][0]["variants"][0]["manifest"]) + assert error.value.errors()[0]["loc"] == ("format_kind",) @pytest.mark.parametrize( "factory", [_creative_asset, _creative, _delivery_creative, _creative_manifest], ) -def test_known_format_kind_still_coerces_to_enum(factory) -> None: - model = factory("image") +@pytest.mark.parametrize("kind", list(CanonicalFormatKind)) +def test_known_format_kind_still_coerces_to_enum(factory, kind) -> None: + model = factory(kind.value) - assert model.format_kind is CanonicalFormatKind.image - assert model.model_dump(mode="json")["format_kind"] == "image" + assert model.format_kind is kind + assert model.model_dump(mode="json")["format_kind"] == kind.value + assert type(model).model_validate_json(model.model_dump_json()).format_kind is kind def test_delivery_creative_alias_is_open_and_keeps_its_identity() -> None: diff --git a/tests/test_manifest_response_readback.py b/tests/test_manifest_response_readback.py new file mode 100644 index 000000000..a74f6989c --- /dev/null +++ b/tests/test_manifest_response_readback.py @@ -0,0 +1,308 @@ +"""Strict manifest inputs and tolerant buyer response views (issue #1241).""" + +from __future__ import annotations + +import json + +import pytest +from pydantic import TypeAdapter, ValidationError + +import adcp.types as public_types +from adcp import ADCPClient +from adcp.protocols.mcp import MCPAdapter +from adcp.types import ( + CanonicalFormatKind, + ContextMatchResponse, + CreativeManifest, + LegacyBuildCreativeResponse, + LegacyBuildCreativeResponse1, + LegacyBuildCreativeResponse3, + LegacyBuildCreativeResponse4, + LegacyPreviewCreativeResponse, + LegacyPreviewCreativeResponse3, + ListCreativesResponse, + McpWebhookPayload, + TmpOffer, +) +from adcp.types.core import AgentConfig, Protocol, TaskResult, TaskStatus +from adcp.types.generated_poc.core.async_response_data import AdcpAsyncResponseData +from adcp.types.generated_poc.core.creative_manifest import ( + CreativeManifest as InputManifest, +) +from adcp.types.generated_poc.core.version_envelope import AdcpVersionEnvelope +from adcp.types.generated_poc.media_buy.build_creative_response import ( + Creative as InputBuildCreative, +) +from adcp.types.generated_poc.trusted_match.provider_context_match_response import ( + ContextMatchResponseProviderRouter, +) + +FUTURE_KIND = "future_canonical_format" +TASK_CASES = ("preview", "build-single", "build-multi", "build-variants") +CASES = (*TASK_CASES, "context-match", "provider-context-match") + + +def response_case(case, kind=FUTURE_KIND): + manifest = { + "format_kind": kind, + "assets": {}, + "vendor_annotation": {"notes": ["preserved"]}, + } + if case == "preview": + return LegacyPreviewCreativeResponse3, { + "response_type": "variant", + "variant_id": "variant-1", + "previews": [ + { + "preview_id": "preview-1", + "renders": [ + { + "render_id": "render-1", + "output_format": "html", + "preview_html": "

Preview

", + "role": "primary", + } + ], + } + ], + "manifest": manifest, + } + if case == "build-single": + return LegacyBuildCreativeResponse1, {"creative_manifest": manifest} + if case == "build-multi": + return LegacyBuildCreativeResponse3, {"creative_manifests": [manifest]} + if case in ("context-match", "provider-context-match"): + model = ( + ContextMatchResponse if case == "context-match" else ContextMatchResponseProviderRouter + ) + return model, { + "request_id": "request-1", + "offers": [{"package_id": "package-1", "creative_manifest": manifest}], + } + return LegacyBuildCreativeResponse4, { + "creatives": [ + { + "build_creative_id": "creative-1", + "variants": [ + { + "build_variant_id": "variant-1", + "creative_manifest": manifest, + "rank": 1, + } + ], + } + ], + } + + +def read_manifest(response, case): + if case == "preview": + return response.manifest + if case == "build-single": + return response.creative_manifest + if case == "build-multi": + return response.creative_manifests[0] + if case in ("context-match", "provider-context-match"): + return response.offers[0].creative_manifest + return response.creatives[0].variants[0].creative_manifest + + +@pytest.mark.parametrize("kind", ["image", FUTURE_KIND, None]) +@pytest.mark.parametrize("case", CASES) +@pytest.mark.parametrize("json_input", [False, True], ids=["python", "json"]) +def test_manifest_response_round_trip(case, kind, json_input): + model, payload = response_case(case, kind) + response = ( + model.model_validate_json(json.dumps(payload)) + if json_input + else model.model_validate(payload) + ) + manifest = read_manifest(response, case) + if kind == "image": + assert manifest.format_kind is CanonicalFormatKind.image + else: + assert manifest.format_kind == kind + assert manifest.model_dump()["vendor_annotation"] == {"notes": ["preserved"]} + assert model.model_validate_json(response.model_dump_json()).model_dump(mode="json") == ( + response.model_dump(mode="json") + ) + assert type(manifest).__name__ not in public_types.__all__ + assert not hasattr(public_types, type(manifest).__name__) + + +@pytest.mark.parametrize("case", CASES) +@pytest.mark.parametrize("mcp_content", [False, True], ids=["dict", "mcp-text"]) +def test_adapter_keeps_manifest_response(case, mcp_content): + model, payload = response_case(case) + union = ( + LegacyPreviewCreativeResponse + if case == "preview" + else LegacyBuildCreativeResponse if case.startswith("build-") else model + ) + data = [{"type": "text", "text": json.dumps(payload)}] if mcp_content else payload + adapter = MCPAdapter( + AgentConfig( + id="creative-agent", + agent_uri="https://seller.example/mcp", + protocol=Protocol.MCP, + ) + ) + result = adapter._parse_response(TaskResult(status=TaskStatus.COMPLETED, data=data), union) + assert result.success, result.error + assert result.status is TaskStatus.COMPLETED + assert read_manifest(result.data, case).format_kind == FUTURE_KIND + reparsed = TypeAdapter(union).validate_json(result.data.model_dump_json()) + assert read_manifest(reparsed, case).format_kind == FUTURE_KIND + + +def completed_task(case): + _, result = response_case(case) + return { + "idempotency_key": "whk_manifest_readback_1", + "operation_id": "operation-1", + "task_id": "task-1", + "task_type": "preview_creative" if case == "preview" else "build_creative", + "status": "completed", + "timestamp": "2026-09-01T00:00:00Z", + "result": result, + } + + +@pytest.mark.parametrize("case", TASK_CASES) +def test_async_response_union_and_webhook_round_trip(case): + model, payload = response_case(case) + result = AdcpAsyncResponseData.model_validate_json(json.dumps(payload)) + assert isinstance(result.root, model) + assert read_manifest(result.root, case).format_kind == FUTURE_KIND + reparsed = AdcpAsyncResponseData.model_validate_json(result.model_dump_json()) + assert isinstance(reparsed.root, model) + assert read_manifest(reparsed.root, case).format_kind == FUTURE_KIND + + webhook = McpWebhookPayload.model_validate_json(json.dumps(completed_task(case))) + assert isinstance(webhook.result.root, model) + assert read_manifest(webhook.result.root, case).format_kind == FUTURE_KIND + reparsed_webhook = McpWebhookPayload.model_validate_json(webhook.model_dump_json()) + assert isinstance(reparsed_webhook.result.root, model) + assert read_manifest(reparsed_webhook.result.root, case).format_kind == FUTURE_KIND + + +@pytest.mark.asyncio +@pytest.mark.parametrize("case", TASK_CASES) +async def test_public_completed_task_parser_keeps_manifest_readback(case): + model, _ = response_case(case) + payload = completed_task(case) + client = ADCPClient( + AgentConfig( + id="creative-agent", agent_uri="https://seller.example/mcp", protocol=Protocol.MCP + ), + allow_unauthenticated_webhooks=True, + ) + with pytest.deprecated_call(match="handle_webhook_legacy"): + result = await client.handle_webhook_legacy( + payload, + task_type=payload["task_type"], + operation_id=payload["operation_id"], + ) + assert result.success, result.error + assert result.status is TaskStatus.COMPLETED + # A successful untyped fallback would silently hide strict-union rejection. + assert isinstance(result.data.root, model) + assert read_manifest(result.data.root, case).format_kind == FUTURE_KIND + reparsed = TaskResult[AdcpAsyncResponseData].model_validate_json(result.model_dump_json()) + assert isinstance(reparsed.data.root, model) + assert read_manifest(reparsed.data.root, case).format_kind == FUTURE_KIND + + +@pytest.mark.parametrize("case", CASES) +@pytest.mark.parametrize("input_model", [CreativeManifest, InputManifest]) +@pytest.mark.parametrize("reuse_instance", [False, True], ids=["dump", "instance"]) +def test_manifest_readback_cannot_bypass_input_validation(case, input_model, reuse_instance): + model, payload = response_case(case) + manifest = read_manifest(model.model_validate(payload), case) + value = manifest if reuse_instance else manifest.model_dump(mode="json") + with pytest.raises(ValidationError): + input_model.model_validate(value) + assert not isinstance(manifest, input_model) + + +@pytest.mark.parametrize("case", ["build-variants", "context-match", "provider-context-match"]) +def test_nested_readback_accepts_known_source_models(case): + model, payload = response_case(case, "image") + collection = "creatives" if case == "build-variants" else "offers" + source = InputBuildCreative if case == "build-variants" else TmpOffer + payload[collection][0] = source.model_validate(payload[collection][0]) + response = model.model_validate(payload) + assert read_manifest(response, case).format_kind is CanonicalFormatKind.image + node = getattr(response, collection)[0] + assert type(node).__name__ not in public_types.__all__ + assert not hasattr(public_types, type(node).__name__) + + +def test_nested_build_readback_preserves_envelopes_and_constraints(): + model, payload = response_case("build-variants") + response = model.model_validate(payload) + creative = response.creatives[0] + variant = creative.variants[0] + for node in (creative, variant): + assert isinstance(node, AdcpVersionEnvelope) + assert type(node).__name__ not in public_types.__all__ + assert not hasattr(public_types, type(node).__name__) + + payload["creatives"][0]["variants"][0]["rank"] = 0 + with pytest.raises(ValidationError) as invalid_rank: + model.model_validate(payload) + assert invalid_rank.value.errors()[0]["type"] == "greater_than_equal" + payload["creatives"][0]["variants"] = [] + with pytest.raises(ValidationError) as empty_variants: + model.model_validate(payload) + assert empty_variants.value.errors()[0]["type"] == "too_short" + payload["creatives"] = [] + with pytest.raises(ValidationError) as empty_creatives: + model.model_validate(payload) + assert empty_creatives.value.errors()[0]["type"] == "too_short" + + +def test_build_readback_preserves_empty_and_error_branch_rules(): + with pytest.raises(ValidationError) as empty_manifests: + LegacyBuildCreativeResponse3.model_validate({"creative_manifests": []}) + assert empty_manifests.value.errors()[0]["type"] == "too_short" + + response = LegacyBuildCreativeResponse4.model_validate( + { + "creatives": [ + { + "build_creative_id": "creative-1", + "errors": [ + {"code": "BUILD_FAILED", "message": "Unable to build this creative"} + ], + } + ], + } + ) + assert response.creatives[0].variants is None + assert response.creatives[0].errors[0].code == "BUILD_FAILED" + + +def test_direct_listed_creative_kind_stays_strict(): + payload = { + "query_summary": {"total_matching": 1, "returned": 1}, + "pagination": {"has_more": False}, + "creatives": [ + { + "creative_id": "creative-1", + "name": "Image", + "format_kind": "image", + "status": "approved", + "created_date": "2026-09-01T00:00:00Z", + "updated_date": "2026-09-01T00:00:00Z", + } + ], + } + assert ListCreativesResponse.model_validate(payload).creatives[0].format_kind is ( + CanonicalFormatKind.image + ) + payload["creatives"][0]["format_kind"] = FUTURE_KIND + with pytest.raises(ValidationError) as error: + ListCreativesResponse.model_validate(payload) + assert error.value.errors()[0]["loc"] == ("creatives", 0, "format_kind") + assert error.value.errors()[0]["type"] == "enum" diff --git a/tests/test_rc3_media_buy_runtime.py b/tests/test_rc3_media_buy_runtime.py index f47cf1899..a6f7aae51 100644 --- a/tests/test_rc3_media_buy_runtime.py +++ b/tests/test_rc3_media_buy_runtime.py @@ -347,7 +347,7 @@ def test_a_clear_survives_on_sync_creatives_localization() -> None: { "creative_id": "cr-1", "name": "Creative One", - "format_kind": "third_party_tag", + "format_kind": "display_tag", "status": "approved", "assets": {}, "created_date": "2026-09-01T00:00:00Z", diff --git a/tests/type_checks/creative_asset_binding.py b/tests/type_checks/creative_asset_binding.py index 6ad41ad3e..c20b81802 100644 --- a/tests/type_checks/creative_asset_binding.py +++ b/tests/type_checks/creative_asset_binding.py @@ -1,6 +1,15 @@ -"""Static contract for the public canonical CreativeAsset binding (issue #1141).""" +"""Public canonical model identity and closed format-kind types (#1141/#1241).""" -from adcp.types import CanonicalFormatKind, CreativeAsset, CreativeManifest, DeliveryCreative +from typing_extensions import assert_type + +from adcp.types import ( + CanonicalFormatKind, + Creative, + CreativeAsset, + CreativeManifest, + DeliveryCreative, + GetCreativeDeliveryResponse, +) from adcp.types.canonical_creative import CanonicalBoundaryModel @@ -14,18 +23,31 @@ def accepts_canonical_class(model: type[CanonicalBoundaryModel]) -> None: { "creative_id": "creative-1", "name": "Creative", - "format_kind": "future_canonical_format", + "format_kind": "image", "assets": {}, } ) -format_kind: CanonicalFormatKind | str = asset.format_kind +assert_type(asset.format_kind, CanonicalFormatKind) + + +def listed_kind(creative: Creative) -> CanonicalFormatKind: + assert_type(creative.format_kind, CanonicalFormatKind) + return creative.format_kind + manifest = CreativeManifest(assets={}) -manifest_kind: CanonicalFormatKind | str | None = manifest.format_kind +assert_type(manifest.format_kind, CanonicalFormatKind | None) delivery = DeliveryCreative( creative_id="creative-1", format_kind="future_canonical_format", variants=[], ) -delivery_kind: CanonicalFormatKind | str | None = delivery.format_kind +assert_type(delivery.format_kind, CanonicalFormatKind | str | None) + + +def check_served_manifest(response: GetCreativeDeliveryResponse) -> None: + for creative in response.creatives: + for variant in creative.variants: + if variant.manifest is not None: + assert_type(variant.manifest.format_kind, CanonicalFormatKind | str | None)