feat: round-trip property_list and add collection_list on media-buy (#1275)

[ChrisHuie]

Closes part of #1247 (gaps #6, #7) — AdCP storyboard inventory_list_targeting parity. Schema, persistence, response, and validation aligned with spec.

Schema additions

• New CollectionListReference (mirrors PropertyListReference shape per AdCP 3.0.6 core/collection-list-ref.json; local re-export because adcp 4.3 generates the type but doesn't surface it on the
  public adcp.types namespace).
• Targeting accepts collection_list and collection_list_exclude as typed fields, so dev/CI extra="forbid" doesn't drop them.

Response surface

• GetMediaBuysPackage exposes targeting_overlay, populated by _get_media_buys_impl from MediaPackage.package_config (with the existing legacy "targeting" key fallback). model_dump override prevents
  internal Targeting fields from leaking.

Validation

• _create_media_buy_impl and _update_media_buy_impl reject property_list targeting against products with property_targeting_allowed=False per AdCP 3.0.6 core/targeting.json:191. Validator helper is
  None-safe (regression coverage in test_overlay_validation_v3.py::TestValidatePropertyTargetingAllowed).

Tests

• Schema round-trip tests and TargetingFactory + list-reference factories.
• get_media_buys round-trip tests asserting the storyboard's literal field paths (packages[0].targeting_overlay.{property,collection}_list.list_id).
• Update behavioral tests for property_targeting_allowed enforcement and collection_list-only path.
• Integration tests for create/update enforcement (skip locally without Docker).
• Obligations UC-002-MAIN-14a/b and UC-003-MAIN-13/14 wired via Covers: tags.
• Shared helpers in tests/utils/database_helpers.py replace duplicated setup blocks across two integration test files (DRY count 102 to 101).

Review-feedback follow-ups (commits 91af5f8, 49fa242)

• N1 real bug: validate_property_targeting_allowed crash on None product fixed + 6-case regression suite. Reachable when an admin deleted a product still referenced by a package and the buyer called
  update_media_buy with property_list.
• m1 repository hygiene: ProductRepository.get_by_id replaces the raw select(ProductModel) in the update-side validator block.
• m2 test tightening: test_internal_targeting_fields_not_leaked now asserts the full Targeting excluded set (key_value_pairs, tenant_id, created_at, updated_at, metadata, had_city_targeting).
• Was-M1 forward-compat marker: FIXME(inventory-targeting-A1-update) on the update return-envelope block, documenting that it converts to raise AdCPValidationError when PR refactor: error-drain Pattern A + boundary ValueErrors (PR 2 of 3) #1307 sub-batch 3 drains
  the file's Pattern A sites.
• Was-M2 specialism rationale: comment on capabilities.py documenting why sales-non-guaranteed is declared before all bundled scenarios are green (CI storyboard job is advisory pending feat: AdCP storyboard compliance — gap analysis and roadmap #1247 gap Welcome to salesagent Discussions! #1;
  public declaration forces prioritization).
• Spec-ref refresh: AdCP 3.0.1 citations bumped to 3.0.6 across validator, comments, integration test docstring, and UC-002/UC-003 obligation docs.

Pre-commit hygiene (pre-existing drift, surfaced by this PR)

• Bumped pre-commit mypy hook adcp pin from 3.2.0 to 4.3.0 to match pyproject.toml. The drift caused phantom "no attribute" errors on UpdateMediaBuyRequest fields under the pre-commit mypy gate.
• Bumped .type-ignore-baseline from 42 to 60 to reflect the actual count on main. The baseline was stale before this PR — verified that this PR adds zero new type: ignore markers (git diff confirms).

[KonstantinMirin]

Good stuff — extracting validate_property_targeting_allowed to a shared service is the right call, and the spec-drift guards for CollectionListReference are solid.

A few things need fixing before this is done. Mostly around consistency between create/update paths for the same validation rule, and test DRY — the kind of duplication where someone fixes one copy and misses the other. Details in inline comments.

[KonstantinMirin]

Create wraps this in f"Targeting validation failed: {violation}" (media_buy_create.py:1647), this passes the raw string. Same rule, same helper — should produce the same wire format.

[KonstantinMirin]

Every other error path in this file passes response.model_dump(mode="json") to update_workflow_step (lines 267, 426, 483, 506). This one hand-builds a raw dict. Build the UpdateMediaBuyError first, then call .model_dump(mode="json") on it — keeps the workflow audit trail consistent with the API response.

[KonstantinMirin]

product: Any + getattr — the function accesses .product_id and .property_targeting_allowed, both defined on the Product model. Type it as Product | None (use TYPE_CHECKING if circular import is a concern), drop the getattr calls, let mypy do its job.

[KonstantinMirin]

_future_dates() is identical to the one in test_targeting_validation_chain.py:33. Extract to tests/utils/database_helpers.py — this PR already extends that module.

[KonstantinMirin]

PrincipalFactory.make_identity() exists and does exactly this. Use it instead of hand-rolling a duplicate.

[ChrisHuie]

missed this one. Addressing now

[KonstantinMirin]

When the call succeeds, zero assertions run — this passes vacuously even if the validation code is deleted. Add assert not isinstance(response, CreateMediaBuyError) so the happy path actually asserts something. Same for test_create_accepts_collection_list_without_property_list.

[KonstantinMirin]

The OR in "property_targeting_allowed" in errors_text or "VALIDATION_ERROR" in errors_text accepts any VALIDATION_ERROR — budget, format, whatever. The unit test at test_update_media_buy_behavioral.py:1755 checks both with AND. Match that here.

[KonstantinMirin]

This 5-decorator @patch stack is copied on every method in the class — 6 times here, 4 more in TestGetMediaBuysImpl. A class-level fixture or test env would eliminate all 50 lines and prevent the "update one patch target, miss the other nine" bug.

[KonstantinMirin]

Code Review: PR #1276 — Round 9 (Final)

All round-8 feedback verified as addressed. Clean across all 5 review dimensions — no blockers, no majors, no mediums from the PR's own changes.

One new finding surfaced during this pass:

_update_media_buy_impl missing three targeting validators

src/core/tools/media_buy_update.py ~line 298-325

The create path runs four targeting validations on every targeting_overlay:

validate_unknown_targeting_fields(pkg.targeting_overlay)
validate_overlay_targeting(pkg.targeting_overlay)
validate_geo_overlap(pkg.targeting_overlay)
validate_property_targeting_allowed(product, pkg.targeting_overlay)  # added by this PR

The update path now runs validate_property_targeting_allowed (correctly added by this PR) but skips the other three. A buyer can bypass geo-overlap checks, unknown-field rejection, and managed-only dimension checks by sending targeting changes through update instead of create.

All three validators are already in targeting_capabilities.py and importable — they just need to be called at the same location where validate_property_targeting_allowed runs in the update path.

This is pre-existing (not introduced by this PR), but this PR is the right moment to close it — the validation callsite is already open, the imports are already available, and shipping without it means the asymmetry is now partially fixed (property_targeting_allowed) but still exploitable on the other three dimensions.

Everything else

The rest is clean:

• fail_workflow_step_for_exception helper correctly threads structured Error into async push payloads
• All 4 DRY extractions verified (patched_internals, future_iso_date_range, advisory helpers, validation raiser)
• Integration round-trip through real Postgres covers property_list, collection_list, and both-together
• GAM lifecycle tautology assertions replaced with specific behavioral pins
• All inline imports hoisted, raw dicts replaced with Error(...), PrincipalFactory.make_identity() adopted everywhere

[KonstantinMirin]

All Round 9 feedback addressed — targeting-validator asymmetry closed by 48b9242 (verified at media_buy_update.py:331-333, all four validators now run in lockstep with the create path). Approving.

Note: the Security Audit failure (Markdown 3.10 / PYSEC-2026-89 and PyJWT 2.12.1 / PYSEC-2025-183) isn't from this PR — both lack fix versions. Handle separately when you're ready (probably the same --ignore-vulns pattern as the existing two).

[KonstantinMirin]

FYI on the Security Audit failure — the two flagged vulns (PYSEC-2026-89 Markdown, PYSEC-2025-183 PyJWT) are both addressed in #1334 via the new scripts/security-audit.sh (single source of truth for the ignore list, called by both CI and run_all_tests.sh). Documented rationale per-CVE in that script: Markdown is already past the published fix (malformed OSV affected-range makes uv-secure flag every version), and PyJWT is supplier-disputed plus we only invoke it with verify_signature: False for OIDC ID-token parsing. Once #1334 lands the audit will go green here too — no action needed on this PR.