feat(property-list): honest-declaration reject, Kevel siteId compilation, and faithful intersection (PR 2 after error-emission)

[ChrisHuie]

closes #1445
Adds end-to-end property_list targeting support and makes every adapter honest about
whether it can honor it. A buyer's targeting_overlay.property_list is either compiled to
native ad-server targeting or rejected with a spec-compliant UNSUPPORTED_FEATURE envelope —
it is never silently dropped.

What's included

• Honest-declaration boundary. Each adapter declares supports_property_list_targeting
  (base default False; only Kevel sets True). _create_media_buy_impl /
  _update_media_buy_impl call raise_if_property_list_unsupported(...) at the boundary,
  which raises AdCPCapabilityNotSupportedError (UNSUPPORTED_FEATURE,
  recovery=correctable, carrying field + suggestion) when an enabled adapter cannot
  compile property_list.
• Kevel compilation. Kevel compiles domain/subdomain identifiers to native siteIds
  via KevelSiteResolver. Identifier types it cannot compile (e.g. ios_bundle) raise
  AdCPCapabilityNotSupportedError rather than being dropped; dry-run still validates
  identifier types.
• Faithful product intersection. get_products intersects a buyer's property_list
  against authorized properties across all three publisher_properties selector variants
  (all / by_id / by_tag) via PropertyIntersection + AuthorizedPropertyRepository.
  _create_media_buy_impl emits a zero-overlap advisory (accept-with-context; logs only,
  never rejects).

Error emission

All rejection is a typed AdCPCapabilityNotSupportedError — raised at the _impl boundary
(and, for Kevel's per-identifier-type case, inside the adapter). No Error(code=...)
construction and no structural-guard allowlist growth. The two-layer wire envelope
(code / recovery / field / suggestion) is verified across REST, A2A, and MCP.

Testing

• Unit: Kevel compilation + per-type raise, the site resolver, intersection across every
  selector variant, the zero-overlap advisory (log-only / never-raises), and the
  get_adcp_capabilities declaration.
• Integration: 3-transport wire-envelope tests (REST TestClient, A2A on_message_send
  artifact, MCP ToolError) asserting the UNSUPPORTED_FEATURE envelope.

Supersedes

Combines and replaces #1313, #1314, and #1315 (the property_list adapter / Kevel / intersection
series). Please close those in favor of this PR.

Known unrelated failures

Two pre-existing failures can surface in a local full-suite run; neither touches files changed
here:

• get-products-request schema alignment (push_notification_config) — resolved by refactor: error-drain Pattern A + boundary ValueErrors (PR 2 of 3) #1307;
  clears once this rebases past it.
• TestCreativeApprovalRetroactivePush (admin) — a stale mock-patch target already failing on
  main.

Design decisions (locked)

Grounded against the pinned AdCP 3.0.1 snapshots (schemas, prose, storyboards)
and recorded here so review can challenge the decision, not rediscover it:

• A2A submitted placement: a manual-approval create completes the A2A task
  (COMPLETED) with the artifact carrying the spec submitted variant, plus
  artifact.metadata.adcp_task_id. The spec's extraction algorithm reads
  artifacts only on final A2A states (interim SUBMITTED reads
  status.message, which carries no DataPart here), and the calling-an-agent
  prose explicitly sanctions a completed A2A task carrying a submitted AdCP
  response — the payload's own status field is the discriminator.
• Update-side recompile (full parity): targeting_overlay.property_list
  changes through update_media_buy now run the same adapter compile path as
  create: a pre-write identifier-type gate (validate_targeting_update, fires
  before the dry-run early-return) and a post-persist recompile push
  (update_package_targeting — Kevel PUTs the recompiled targeting to the
  flight; Mock documents persistence-as-compile; the base seam fails loud for
  any future capable adapter that forgets to implement it). Previously the
  overlay persisted to package_config while the live flight kept serving the
  old siteIds.
• Create/update envelope parity: identical overlay-targeting violations
  from the shared collector now raise through one shared raiser —
  byte-identical envelopes (INVALID_REQUEST, same field, same suggestion) on
  both paths. Update previously emitted a bare VALIDATION_ERROR.
• Testing-hook transport parity: resolve_identity derives the AdCP
  testing context from headers when the transport didn't pre-extract one.
  REST previously dropped the headers entirely — a buyer's X-Dry-Run create
  booked for real. MCP/A2A behavior is unchanged (explicit context wins);
  pinned by a REST dry-run wire test. The REST submitted-variant wire test
  deliberately runs without the session bypass so the production
  setup-checklist gate stays exercised.
• Unknown-list taxonomy: a 4xx from the buyer's list service (typo'd or
  inaccessible list_id) is the buyer's error — correctable
  VALIDATION_ERROR naming the reference — while 5xx/timeout/connect stay
  transient SERVICE_UNAVAILABLE. Previously every HTTP failure was
  transient, sending buyers into retry loops on input they must correct.
• Advisory wire contract: drop advisories carry explicit
  recovery: correctable (the spec's forward-compat rule otherwise tells
  receivers to assume transient — the wrong instruction for fix-the-list) and
  severity: "warning" (prose-defined for warnings in errors[]; legal via
  the Error schema's open extras). PRODUCT_UNAVAILABLE stays the code — the
  closest standard-table entry; the single builder makes a future swap one
  line.
• Success/error discriminator enforcement: the create/update success
  variants reject an errors key at construction time (the response oneOf
  forbids it via not: {required: ["errors"]}; the classes keep
  extra="allow" for protocol-field round-trips, so this was previously
  unguarded — an errors= kwarg would have shipped a response matching no
  spec variant).
• Kevel zero-match compile: siteIds: [] is written as-is on zero match
  (accept-with-context per the no-match storyboard). Kevel's public docs do
  not state whether an empty array restricts to nothing or lifts the
  restriction; this ships the match-nothing reading, recorded here so the
  assumption is visible.
• Submitted-replay fidelity: a retry of a submitted create currently
  replays a reconstructed completed-style success (pre-existing behavior from
  main) — the wire contract requires byte-for-byte replay carrying the same
  task_id. The fix is the idempotency-replay branch's verbatim success-cache;
  after it merges, the reconciliation here makes the submitted variant
  cacheable and replayable. Denoted in the relevant docstrings.

Shared infrastructure extracted in this batch: ThreadSafeTTLCache (one
locking/expiry contract for the property-list and Kevel site caches),
raise_if_overlay_targeting_violations, indexed package_field_path,
manual_approval test contextmanager; the submitted message cap is derived
from the model's MaxLen instead of hand-copied; the adapter False-pin
enumerates via a package walk so factory-disabled adapters (Xandr) cannot
escape it.

[ChrisHuie]

Finishing-pass decisions (for re-review)

Pinned spec throughout: AdCP 3.1.0-beta.3 (adcp==5.7.0).

Error-recovery taxonomy

1. Ad-server HTTP 403 → terminal, uniform across adapters. A 403 from a
   tenant's ad server (operator credential denied) maps to
   recovery=terminal / CONFIGURATION_ERROR for Broadstreet, Kevel, Xandr,
   Triton writes and the Kevel site read; the buyer-facing property-list
   reference fetch keeps 4xx → correctable. Grounding: core/error.json
   recovery definitions — terminal = "requires human action", correctable
   = "fix the request and resend"; a buyer cannot fix the tenant's ad-server
   credential. Wire emits the leak-safe SERVICE_UNAVAILABLE + terminal;
   per error-handling.mdx (L3) error.recovery is the normative carrier, so
   the code/recovery pairing is spec-legal (the spec ships the same shape for
   PERMISSION_DENIED). Single-sourced via one class selector consumed by both
   the factory and the configures-self-in-__init__ (BroadstreetAPIError)
   forms — no hand-copied table. The unused general tuple form was removed.

2. Broadstreet REST status line unified (P42). BroadstreetAPIError now
   derives (error_code, recovery, status_code) from the same table, so one
   upstream event yields one buyer-facing REST status regardless of adapter
   (403→500, 404→400, 5xx→502, matching wrap_request_errors). The upstream
   ad-server status is retained in the error message and response_body, not
   the wire status line.

3. Broadstreet credential no longer reaches the wire. The transport-failure
   message no longer interpolates str(e) (which stringifies the request URL
   carrying the access_token query param), and the 4xx arm no longer echoes
   the response body; the detail is kept server-side. Regression-pinned.

Security

4. SSRF DNS-rebinding TOCTOU closed (property-list fetch).
   resolve_validated_ip validates every getaddrinfo address (closes the
   multi-A-record bypass) and the fetch is pinned to the validated IP. The
   request URL keeps its hostname, so httpx's TLS SNI + certificate-hostname
   verification run normally (a rebound IP whose cert doesn't match the
   hostname fails). The pin overrides httpcore's _network_backend — the
   public sni_hostname request extension was rejected because it does not
   verify the cert hostname (confirmed empirically). The private-API seam is
   guarded by oracles asserting the connect targets the validated IP.

Deliberately NOT changed

5. Broad except Exception in the advisory builders, kept. The
   inventory_list_no_match storyboard is accept-with-context — advisory
   computation must never veto a booking. test_property_list_advisory.py
   injects RuntimeError into the intersection and the async resolver and
   asserts the booking proceeds; narrowing the except would break that tested
   invariant. Errors are logged at WARNING with full traceback under the
   [INTERSECTION-ADVISORY] marker.

6. Two _DEFAULT_CACHE_TTL_SECONDS = 300 literals, not unified — they
   partition different caches (buyer-list fetch vs Kevel site index) with
   independent tuning.

7. PRODUCT_UNAVAILABLE remains the zero-overlap advisory code —
   INSUFFICIENT_INVENTORY / INVALID_TARGETING are absent from the
   3.1.0-beta.3 error-code.json enum.

Notes

8. Citations in the property_list code were refreshed to 3.1.0-beta.3
   (Spec-Grounding Gate). Idempotency 3.0.1 references and the
   "Per AdCP 4.3 (commit …)" schema-introduction notes are intentionally left.

9. test_update_media_buy_push_webhook_delivery (e2e) is a pre-existing
   xfail(strict) flake — a documented update_media_buy webhook
   commit-ordering bug that xpasses intermittently. Untouched by this pass.

[KonstantinMirin]

Two small follow-ups — both about leaning on things this PR (or the BDD suite) already provides

This is a strong PR — the helper extractions are genuinely generalized to their sibling sites, the error-emission surface is single-sourced, and the async offload checks out end-to-end. Two observations, both minor, and both are really "use the thing that already exists" rather than "add something new."

1. identifier_matching re-derives an enum string the SDK already guarantees

src/services/identifier_matching.py:46 hand-rolls enum→string extraction:

return ident.type.value if hasattr(ident.type, "value") else str(ident.type)

This PR introduced enum_value() in src/core/enum_helpers.py:17 precisely to centralize that shape, and a sibling service (src/services/targeting_capabilities.py:18) already imports it — so at minimum this is a third copy that will drift from the SSOT.

But it's actually simpler than "import the helper." Identifier.type is typed by the SDK as PropertyIdentifierTypes, a real enum.Enum — a constructed Identifier always yields <PropertyIdentifierTypes.domain: 'domain'>, so .value is guaranteed present. Both callers here operate on already-typed Identifier lists (PropertyIntersection.filter_products(..., buyer_identifiers: list[Identifier]) and the resolve_property_list_typed_sync() output), so the hasattr(...) else str(...) guard is defending against a shape that can't reach this function. ident.type.value is enough — no helper, no fallback.

(Broader note for a separate pass, not this PR: enum_value shows up at ~15 internal call sites, some reading raw dicts right next to typed models — e.g. targeting_capabilities.py:393 reads item.system and :396 reads item.get("system", ""). That's the real reason the duck-typed fallback exists. Longer term it's worth treating enum_value as a boundary-only adapter and trusting the SDK enum types downstream, but that's out of scope here.)

2. The response-variant decision has two homes — and there are already-wired scenarios that cover it

The "which variant is this — submitted / success / error?" rule is encoded twice, on two different signals:

• src/a2a_server/adcp_a2a_server.py:480-499 decides on live status == "submitted"
• src/core/tools/media_buy_create.py:507-510 (_replay_cached_success) decides on stored task_id / media_buy_id

Both carry "keep the two in sync" comments, which is the signal to extract one classifier next to the schema union. The reassuring part: the BDD layer already exercises both homes, so this can be extracted with a live safety net rather than blind —

• tests/bdd/features/BR-UC-002-create-media-buy.feature:1877 — T-UC-002-v31-idempotency-replay, which tests/bdd/conftest.py:2566 (_UC002_IDEMPOTENCY_WIRED) runs as a real create_media_buy across all four transports. The A2A leg hits _reconstruct_response_object; the replay hits _replay_cached_success.
• tests/bdd/features/BR-UC-002-create-media-buy.feature:2596 — T-UC-002-storyboard-async-submitted-envelope-task-id-roundtrip, a storyboard-graded scenario that pins the exact submitted-envelope shape (status="submitted", carries task_id, no media_buy_id, no packages).

While we're here: two obligation tests whose behavior these scenarios own

tests/integration/test_create_media_buy_behavioral.py:1123 (test_seller_rejects_buyer_notified) and :1144 (test_buyer_can_poll_approval_progress) assert only that pending-approval setup succeeded (status == "submitted", task_id is not None) — they'd pass even if rejection or polling were fully broken, and their docstrings say as much. The behaviors they're tagged for are already driven by BDD:

• Rejection → tests/bdd/features/BR-UC-002-manual-overrides.feature:24 (T-UC-002-alt-manual-reject-override), asserting status rejected + rejection_reason. conftest.py:683 shows this hand-authored scenario deliberately replaced the auto-generated alt-manual-reject.
• Polling → the submitted-envelope-task-id-roundtrip storyboard above, whose comment states the contract directly: "Sellers that fabricate a fresh task_id break the buyer's polling contract."

So rather than tightening those two tests, it'd be cleaner to relabel them to what they actually pin (the pending-approval setup) and let the Covers: tags live on the scenarios that observe the behavior. Happy to take any of this on in a follow-up if useful.

[ChrisHuie]

All three taken, two with adjustments after tracing — details below. Commits 214307b, 04b96bb, 3b467c6.

1. Enum extraction — agreed, and went with your stronger form: identifier_type_str is deleted and the four call sites read ident.type.value directly, so an untyped object fails loud instead of being papered over. The SimpleNamespace test that pinned the dead fallback is gone with it. One provenance note: the hand-roll predates the SSOT — enum_value landed via the #1399 SDK upgrade and reached this branch in the 06-17 main merge, after identifier_matching.py was written. That doesn't change the conclusion, but it's why the hygiene passes missed it: nothing swept branch-authored helpers against helpers merges bring in. That sweep is now part of our process. The broader enum_value-as-boundary-adapter pass stays a follow-up as you suggested.

2. Variant classifier — extracted as classify_media_buy_response_payload next to the unions in schemas/_base.py; the A2A reconstructor and _replay_cached_success both consume it and the two co-change comments are deleted. It discriminates on required-field shape (media_buy_id ⇒ success, task_id ⇒ submitted, neither ⇒ error) rather than the protocol status key, because stored idempotency bodies never carry the envelope status (CreateMediaBuyResult injects it at serialization) — shape is the one signal valid at both homes, and it keeps stored payloads replaying as their original variant across deploys. Truth table pinned in tests/unit/test_media_buy_response_variant_classifier.py.

One correction on the safety net: of the scenarios you cited, only the idempotency-replay one is live — T-UC-002-alt-manual-reject-override and the submitted-envelope storyboard are blanket-xfailed by the UC-002 harness gate ("not yet wired for non-account scenarios", tests/bdd/conftest.py). The extraction was verified with the wired replay scenario plus the idempotency and A2A-reconstruction integration suites.

3. Obligation tests — because those two scenarios don't run yet, moving the Covers: tags onto them would have made the coverage claim vacuous, so we went the other way and made the tests observe the behavior: -09 now drives the rejection through the same tenant-scoped WorkflowRepository.update_status(status="rejected") call the admin route makes and asserts the buyer sees the rejected status and reason via get_task; -10 polls get_task with the returned task_id and asserts requires_approval plus the media-buy object mapping. Doing that exposed a harness fidelity gap — MediaBuyCreateEnv stubbed update_workflow_step to a no-op, so steps froze at in_progress in tests while production commits requires_approval; the harness now delegates to the real context manager like create/link already did. Webhook push on rejection remains unimplemented and stays honestly pinned by the xfailed override scenario; once the UC-002 harness wiring lands, moving the tags to BDD as you proposed is the right end state.

Verified at 3b467c6: make quality green; all harness-consuming integration files green (137 tests) plus A2A compliance/advisory wire (30); full local suite run clean apart from the documented in-network runner artifacts; CI green at the previous head.