Skip to content

refactor: canonicalize publisher_domain storage to eliminate normalization drift #1229

Description

@ChrisHuie

Verified status + spec research (main @19d3efd5 · adcp 5.7.0 / spec 3.1.0-beta.3 · 2026-06-12)

Premise — not started, not subsumed

Sequencing

#1248 (adcp SDK upgrade) is closed and the SDK is now 5.7.0 — the "post #1248" gate (see comment) is satisfied. #1247 (AdCP storyboard compliance roadmap) is still open.

Spec claims — verified against adcp 5.7.0 source (adcp/adagents.py)

Both criticisms of #1207 hold against the current library:

  • Directional, not symmetric — confirmed. domain_matches(property_domain, agent_domain_pattern) (:346) expands a bare domain to www/m only on the agent_domain_pattern side.
  • www/m only, no mobile. — confirmed. common_subdomains = ["www", "m"] inside domain_matches. Our _filter_properties_by_domain strips www/m/mobile; the extra mobile is the non-spec divergence refactor: canonicalize publisher_domain storage to eliminate normalization drift #1229 calls out.
  • Wildcards — confirmed. *.example.com supported.
  • Bonus: _validate_publisher_domain (:265) now runs an SSRF gate (_check_safe_host, :186 — rejects IP literals + internal hostnames). Routing intake through it hardens the write path for free.

⚠️ Caveat on AC #4 / proposed-fix step 3 ("revert the sync filter to plain domain in domain_identifiers")

Unsound for this consumer. _filter_properties_by_domain(properties, domain) compares our publisher_domain against the external adagents.json property identifiers — not under our canonicalization control, and possibly carrying www./m./subdomain/wildcard forms. Canonicalizing our storage to bare does not make strict equality correct: "x.com" in ["www.x.com"] still fails (the #1201 bug, inverted).

So: canonicalize intake to kill storage drift + enable dedup, but keep spec-compliant matching at the sync filter — replace bespoke _domains_match with library domain_matches(property_domain=<adagents identifier>, agent_domain_pattern=<our canonical domain>) (mind the directionality), not strict equality. Better: evaluate identifiers_match (:393), which wraps domain_matches for domain identifiers and handles non-domain types (bundle_id, roku_store_id) the current helper ignores. Strict equality is safe only at intra-DB consumers (uniqueness check, product-tag validation, GROUP BY) where both sides are our canonical storage.

On the "storyboard"

The C2 conformance artifact is an issue-cluster index, not behavioral scenarios — no domain-matching storyboard to validate against. The real overlap is the property_list/collection_list surface: #1302 (end-to-end property_list, no-match handling) and PR #1389 consume publisher_domain via the publisher-property-selector contract (selection_type: all|by_id|by_tag, each carrying publisher_domain). Canonicalization must stay consistent across that work.

Stale reference corrections

  • adcp SDK (written vs 4.x; now 5.7.0 — functions intact by name/signature, relocated): _normalize_domain:155, _validate_publisher_domain:265, domain_matches:346 (ignore :20-150 / :140-148 / :141-148).
  • property_discovery_service.py sync filter: :249:159. products.py: :1068:1065. tenant_config.py: :52:49. capabilities.py: :128:135. properties.py: :88:82.
  • Intake refs publisher_partners.py:92-101 and authorized_properties.py:44 remain exact.

Proposed fix / migration plan below remain valid except AC #4 / step 3 as noted.


Summary

Publisher domains are stored un-normalized throughout the codebase: at least five intake sites accept raw user input with inconsistent normalization, and seven+ consumer sites assume strict equality against
those values. PR #1207 patched one consumer (the adagents sync filter) with a bespoke _domains_match helper, but the other consumers — product property validation, uniqueness checks, re-sync fetches,
verification fetches, and the AdCP client-facing responses — still compare strict-equals and will silently mismatch whenever intake diverges. This is the root-cause follow-up to #1201 / #1207: fix normalization at the
boundary (write path), delete the per-consumer workaround, and align with the AdCP library spec (adcp.adagents.domain_matches).

Context / how we got here

Customer bug #1201: publisher registered as www.ladepeche.fr, adagents.json identifier was ladepeche.fr → sync reported 10 verified publishers but zero properties were persisted. PR #1207 stopped the bleed by
normalizing on comparison in the sync filter (src/services/property_discovery_service.py:249). That unblocks the customer, but the normalization drift lives one layer deeper — it's a write-side problem being
patched on the read side. Every other consumer that compares publisher_domain by strict equality remains broken for the same class of input.

Three additional problems with #1207 as a final fix:

  1. It introduces a mobile. prefix that is not in the AdCP spec (adcp.adagents.domain_matches only recognizes www and m — see .venv/lib/python3.12/site-packages/adcp/adagents.py:140-148).
  2. It makes matching symmetric (www.x.comx.com in both directions); the AdCP spec is directional (property_domain vs agent_domain_pattern).
  3. It duplicates helpers that already exist (adcp.adagents._normalize_domain, adcp.domain_matches) — violates the DRY invariant in CLAUDE.md and the "library defines the protocol spec" principle in
    .claude/notes/typed-boundaries-principle.md.

Current behavior / evidence

Intake sites (write path) — inconsistent normalization

File:line Normalization applied
src/admin/blueprints/publisher_partners.py:92-101 strip().lower() + strip http(s):// + rstrip("/"). No www./m. stripping.
src/admin/blueprints/authorized_properties.py:44 (form parse) .strip() only. No case, scheme, prefix, or slash handling.
src/admin/blueprints/authorized_properties.py:174-181 (bulk import create) Writes caller's raw value.
src/admin/blueprints/authorized_properties.py:839-846 (single-property create) Writes parsed value (only stripped).
src/services/property_discovery_service.py:409-414, 524-529 Writes caller's raw publisher_domain arg.
src/core/database/database.py:125-131 Hardcoded "example.com" seed — fine, but confirms no shared normalizer is called.

No single call site uses adcp.adagents._validate_publisher_domain or an equivalent. Multiple user-facing entry points, multiple subtly different normalization policies.

Consumer sites (read path) — assume strict equality

File:line Comparison Broken by un-normalized intake?
src/admin/blueprints/products.py:1068 AuthorizedProperty.publisher_domain == domain (product tag validation) Yes
src/admin/blueprints/publisher_partners.py:50-52 GROUP BY AuthorizedProperty.publisher_domain, dict-lookup against partner.publisher_domain Yes
src/admin/blueprints/publisher_partners.py:119 filter_by(publisher_domain=...) uniqueness check before insert Yes (duplicate rows)
src/admin/blueprints/publisher_partners.py:510 fetch_adagents(partner.publisher_domain) — re-sync uses stored value verbatim as HTTP host Yes
src/services/property_discovery_service.py:111-117 SELECT DISTINCT publisher_domainfetch_adagents(domain) Yes
src/services/property_verification_service.py:80 fetch_adagents(property_obj.publisher_domain) Yes
src/core/database/repositories/tenant_config.py:52 Returns stored values verbatim (downstream AdCP response) Leaks to clients
src/core/tools/capabilities.py:128 Emits PublisherDomain(root=partner.publisher_domain) in get_agent_capabilities response Leaks to clients
src/core/tools/properties.py:88 Emits publisher_domains list in list_authorized_properties AdCP response Leaks to clients

#1207 fixes only the first read path in property_discovery_service.py:249. The rest continue to silently produce wrong results, allow duplicate partner rows (www.x.com + x.com both pass the uq_tenant_publisher
uniqueness check), and leak www. / mixed-case / trailing-slash forms to AdCP clients.

Library already provides the helpers

adcp.adagents ships:

  • _normalize_domain(domain) — strip, lowercase, remove trailing / and ., validate.
  • _validate_publisher_domain(domain) — stricter: strips scheme, strips path, rejects injection chars, length check.
  • domain_matches(property_domain, agent_domain_pattern) — spec-compliant directional match supporting bare-domain → www./m. and *.example.com wildcards. Re-exported as adcp.domain_matches.

We should not be re-implementing these.

Proposed fix

  1. Single canonical-form helper at intake.
    Add src/core/publisher_domain.py exposing canonicalize_publisher_domain(raw: str) -> str that delegates to adcp.adagents._validate_publisher_domain for scheme/path/slash/case/injection handling, then
    additionally strips www. and m. prefixes (the AdCP spec treats bare domains as canonical — see adcp/adagents.py:141-148). If _validate_publisher_domain remains name-private in the library, file an upstream
    request to make it a public export; in the interim, import it with an inline comment acknowledging the underscore.
    Replace every intake-site ad-hoc normalization with canonicalize_publisher_domain(...). Fail-loud on validation errors (surface AdagentsValidationError as HTTP 400 in admin blueprints).

  2. One-shot Alembic migration to backfill stored data.

    • alembic revision -m "canonicalize publisher_domain in publisher_partners and authorized_properties"
    • upgrade():
      • For publisher_partners: (a) compute canonical form per row in Python via op.get_bind(), (b) for each tenant, if canonicalization would produce a duplicate under uq_tenant_publisher, merge: keep the
        row with the most recent last_synced_at (tie-break on is_verified=true, then earliest created_at), delete the losers, rewrite the survivor's publisher_domain. (c) Update remaining rows in place.
      • For authorized_properties: no uniqueness constraint on (tenant_id, publisher_domain) (PK is (property_id, tenant_id)), so no dedup needed — just rewrite every row's publisher_domain to canonical form.
    • downgrade(): no-op with an explicit log warning (we can't reverse the www. strip without the original data; acceptable because the canonical form is a strict subset of valid domains).
  3. Delete fix: normalize domains in property filtering to handle www/m subdomains #1207's helpers. Once intake is canonical, strict equality works end-to-end. Remove:

    • _normalize_domain and _domains_match from src/services/property_discovery_service.py
    • tests/unit/test_property_discovery_domain_matching.py
    • Revert the sync-filter call site at property_discovery_service.py:249 back to elif domain in domain_identifiers:.
  4. Where spec-compliant matching is genuinely needed, use adcp.domain_matches. If a future case requires matching a non-www/m subdomain against the canonical publisher domain, switch that one site to
    adcp.domain_matches(property_domain, agent_domain_pattern)not a roll-our-own helper.

  5. Guardrail (optional, stretch). Add a model-level @validates("publisher_domain") hook that rejects non-canonical values at write time, so the only way to get a www. prefix into the DB is to bypass the ORM.

Acceptance criteria

  • canonicalize_publisher_domain helper added and used at all intake sites listed above.
  • Alembic migration backfills both tables; PublisherPartner duplicate-coalesce logic is exercised by a migration-specific test against a fixture with www.x.com + x.com rows under the same tenant_id.
  • _normalize_domain, _domains_match, and tests/unit/test_property_discovery_domain_matching.py introduced in fix: normalize domains in property filtering to handle www/m subdomains #1207 are deleted. If they cannot be deleted, the root cause is not fixed and this issue stays
    open.
  • Sync filter at src/services/property_discovery_service.py:249 is restored to plain domain in domain_identifiers (or uses adcp.domain_matches if a genuine spec-compliance need remains — not a bespoke
    helper).
  • No .lower(), .strip(), replace("https://", ""), or rstrip("/") on publisher_domain remains outside the shared helper (grep check).
  • Integration test covering the Sales agent can't see properties #1201 scenario: create a PublisherPartner with www.ladepeche.fr, mock its adagents.json with identifier ladepeche.fr, run sync_properties_from_adagents, assert the property
    is persisted. Test passes without any per-consumer normalization helper.
  • AdCP responses from get_agent_capabilities and list_authorized_properties emit canonical forms only (regression test with explicit assertion or snapshot).
  • make quality passes; tox -e integration passes; migration runs cleanly against a DB seeded with mixed-case / www. / trailing-slash forms.

Data migration plan

  1. Pre-migration audit query (run in staging first):
    SELECT tenant_id, publisher_domain, COUNT(*) FROM publisher_partners GROUP BY 1, 2;
    SELECT DISTINCT publisher_domain FROM publisher_partners                                                                                                                                                              
      WHERE publisher_domain ~ '[A-Z]'                                                                                                                                                                                    
         OR publisher_domain LIKE 'www.%'                                                                                                                                                                                 
         OR publisher_domain LIKE 'm.%'                                                                                                                                                                                   
         OR publisher_domain LIKE '%/';                                                                                                                                                                                   
  2. Snapshot both tables.
  3. Run migration. It will:
    - Read all rows into memory (small tables; publisher partners per tenant are O(10-100)).
    - Compute canonical form per row.
    - Group PublisherPartner rows by (tenant_id, canonical_domain); for each group size > 1, pick survivor and delete losers.
    - Rewrite publisher_domain on survivors and all AuthorizedProperty rows.
  4. Post-migration verification: re-run audit query; all values should be canonical; uq_tenant_publisher still holds.
  5. Test fixtures in tests/integration/conftest.py and tests/factories/ already use canonical forms (bare domains, lowercase) — no fixture changes required.

Out of scope

  • Adapter-level domain handling (GAM, Kevel, etc.) — those compare against their own SSP identifiers, not publisher_domain.
  • tenant.primary_domain / tenant.subdomain / tenant.virtual_host — separate concerns (tenant self-identity, not publisher partnership).
  • Upstream change to make adcp.adagents._validate_publisher_domain public — track as a separate upstream PR; in the interim we import the private name with an inline comment.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Status
    Ready for Dev

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions