Creative sync pipeline fixes, uptream fixes

[torbenbrodt]

Background

During internal development we identified and fixed several correctness and spec-compliance bugs in
the core creative sync pipeline (sync_creatives tool) and the creative agent registry. These
fixes are fully generic — they apply to any AdCP-compliant sales agent.

Each fix was validated against AdCP 3.1 (GA) by querying the AdCP knowledge base. Validation
findings are included inline.

We'd like to contribute these upstream. This issue collects the changes for code owner review
before we raise a PR.

---

Proposed changes

1. Fix format lookup to use normalized composite key

File: src/core/tools/creatives/_processing.py

Bug: Format lookup in both _update_existing_creative and _create_new_creative used object
equality (fmt.format_id == creative_format). This silently failed when agent URLs differed only
by a trailing slash (e.g. https://creative.adcontextprotocol.org vs
https://creative.adcontextprotocol.org/), causing creative sync to fall through to the "no
format found" branch.

Fix: Extract a _find_format() helper that normalizes both sides with normalize_agent_url()
and matches on the composite (agent_url, id) key. Applied in both call sites (DRY fix — the loop
was duplicated).

def _find_format(all_formats, creative_format) -> Format | None:
    target_agent = normalize_agent_url(str(creative_format.agent_url))
    target_id = creative_format.id
    for fmt in all_formats:
        if normalize_agent_url(str(fmt.format_id.agent_url)) == target_agent \
                and fmt.format_id.id == target_id:
            return fmt
    return None

AdCP 3.1 validation:

Spec-mandated, not optional. format-id.json explicitly states callers comparing two FormatId
values MUST canonicalize agent_url per AdCP URL canonicalization rules before treating two
formats as the same. The old object-equality comparison was incorrect.

AdCP 3.1 publishes a normative URL Canonicalization
reference page (8-step RFC 3986 §6.2.2/§6.2.3 algorithm) that governs all agent_url
comparisons across format-id resolution, adagents.json lookups, TMP authorization, and
request signing. The canonicalization surface is unchanged from 3.0 — the page is a 3.1
normative consolidation of rules that were already in force.

---

2. Fix static creative manifest to use AdCP-compliant format_id structure

File: src/core/tools/creatives/_processing.py

Bug: The creative_manifest sent to preview_creative used "format_id": format_id_str (a
bare string like "display_300x250"). AdCP spec requires format_id to be a structured object
{id, agent_url}. The old code also included creative_id (not a creative_manifest field) as a
required field.

Fix:

# Before
creative_manifest = {
    "creative_id": existing_creative.creative_id,  # ❌ not a creative_manifest field
    "name": creative.name,
    "format_id": format_id_str,                    # ❌ bare string, schema violation
}

# After
creative_manifest = {
    "format_id": {"id": creative_format.id, "agent_url": str(format_obj.agent_url)},  # ✅
    "assets": _validate_creative_assets(creative.assets) if creative.assets else {},   # ✅ required field
}

Also extracts _build_generative_manifest() helper for the generative path (same structure, DRY).

AdCP 3.1 validation:

format_id as a bare string was a schema violation — "Always a structured object {agent_url, id} — never a plain string." creative_id is not a creative_manifest field at all. assets
is the only required field on creative_manifest.

3.1 addition: Every asset value in creative_manifest.assets must now carry an asset_type
discriminator field (e.g. "asset_type": "image"). The 3.1 schemas switch from a 14-branch
anyOf to oneOf + discriminator: { propertyName: "asset_type" } — payloads that omit
asset_type now fail validation. Ensure _validate_creative_assets() propagates asset_type
on every asset it returns.

3.1 addition: sync_creatives now accepts format_kind (a CanonicalFormatKind value) as
an alternative to format_id for the 3.1+ canonical-format path. New integrations targeting
3.1+ SHOULD prefer format_kind with format_option_ref when routing depends on a product's
declared format option. The legacy format_id path remains fully supported.

---

3. Remove GEMINI_API_KEY dependency from build_creative; use ADCPMultiAgentClient

File: src/core/creative_agent_registry.py

Bug: CreativeAgentRegistry.build_creative() used a raw create_mcp_client +
call_tool("build_creative", params) workaround instead of the typed AdCP SDK. It required a
gemini_api_key parameter read from server config (failing hard if not set), used the wrong field
name format_id instead of target_format_id, and was missing idempotency_key (required on all
AdCP requests).

Fix: Remove gemini_api_key from the signature. Use ADCPMultiAgentClient +
BuildCreativeRequest:

request = BuildCreativeRequest.model_construct(
    message=message,
    target_format_id=FormatReferenceStructuredObject(agent_url=agent_url, id=format_id),
    idempotency_key=idempotency_key,   # ✅ now required
    finalize=finalize,
    creative_manifest=creative_manifest,
    brand=brand,
)
result = await client.agent(agent.name).build_creative(request)

AdCP 3.1 validation:

Issue	Old code	Fix	Verdict
Protocol layer	Raw MCP call_tool	ADCPMultiAgentClient	✅ correct
gemini_api_key	Passed as param	Dropped	✅ not a spec field; creative agent owns its model config
idempotency_key	Absent	Added	✅ required on every AdCP task request in 3.1 (read and mutating alike)
Field name	format_id (wrong)	target_format_id	✅ correct field on BuildCreativeRequest

3.1 change: idempotency_key is now required on every AdCP task request — read and
mutating alike. The 3.0 contract framed it as mutating-only; 3.1 extends the reliability model
uniformly. SDK users pick this up automatically; hand-rolled callers must generate a UUID v4 key
for read tasks as well as writes. Enforcement is staged: 3.1.0 sellers MUST accept reads that
carry idempotency_key and SHOULD reject reads that omit it; 3.2.0 sellers MUST reject.

---

4. Fix _KNOWN_ASSET_TYPES to include "url" asset type

File: src/core/creative_agent_registry.py

Bug: _validate_formats_tolerant() silently dropped any format whose assets included type
"url". The SDK's discriminated union (ImageFormatAsset | VideoFormatAsset | …) does not cover
all valid spec asset types — it is narrower than the spec enum.

Fix:

_KNOWN_ASSET_TYPES = _known_asset_types() | frozenset({"url"})

Also adds a mock text_ad_search format (with text + url assets) to the mock format list for
ADCP_TESTING=true mode.

AdCP 3.1 validation:

"url" is a first-class member of the asset type vocabulary. The SDK discriminated union is a
typing convenience, not the specification.

3.1 update: The full asset type set has grown to 19 members in 3.1 (up from 14 in the
3.0 discriminated union). The complete set is:

Asset type	Notes
image
video
audio
text
markdown
url	Added by this fix
html
css
javascript
vast
daast
webhook
brief
catalog
published_post	New in 3.1 — published-post references
zip	HTML5 banner bundles
vast_tracker	Decomposed per-event VAST tracker URL
daast_tracker	Decomposed per-event DAAST tracker URL
pixel_tracker	Renderer-fired HTTP tracker (web-rendered canonicals)
object	Structured sub-shape (carousels, generative video)

Follow-up question for code owners: Should _KNOWN_ASSET_TYPES be derived from the full
19-member asset type vocabulary rather than the SDK discriminated union? The current patch only
adds "url" — "published_post", "zip", "vast_tracker", "daast_tracker",
"pixel_tracker", and "object" would still be dropped silently.

---

5. Persist brand into stored creative data

Files: src/core/tools/creatives/_assets.py, src/core/tools/creatives/_processing.py,
src/core/helpers/creative_helpers.py

Context: Adapters need brand.domain from stored creative data for routing decisions.
Previously brand was only on the in-memory CreativeAsset object and was lost after the request.

Fix:

• _build_creative_data() in _assets.py serializes brand (Pydantic model, dict, or arbitrary
  object) into the stored data dict.
• media_buy_brand parameter threaded through _update_existing_creative,
  _create_new_creative, and process_and_upload_package_creatives.
• _brand_str_to_ref() helper normalizes plain-string brand values to valid BrandRef dicts
  (strips URL scheme, normalizes to lowercase):

def _brand_str_to_ref(brand_str: str) -> dict[str, str]:
    """Convert a plain brand string to a minimal valid AdCP BrandRef dict."""
    import re
    domain = re.sub(r"^https?://", "", brand_str, flags=re.IGNORECASE)
    domain = domain.split("/")[0].split("?")[0].split("#")[0]
    return {"domain": domain.lower().strip()}

AdCP 3.1 validation:

brand is an explicit field on BuildCreativeRequest typed as BrandRef. Per the 3.1
build_creative task reference, BrandRef requires domain as the only required field;
brand_id is optional. The domain value must be a bare hostname (no scheme, no path) —
hence the normalization helper. Passing brand to build_creative is spec-aligned.

3.1 context: BrandRef ({ domain, brand_id? }) is the canonical brand identity shape
across all 3.1 task schemas. The brand_id field was added in 3.0 when brand-manifest.json
was deleted; task schemas now resolve brand data from /.well-known/brand.json at the declared
domain at execution time. The _brand_str_to_ref() helper producing {"domain": ...} is
correct for the build_creative path where brand_id is not yet known.

---

6. Fix tenant_settings to branch on adapter type

File: src/admin/blueprints/tenants.py

Bug: tenant_settings always built a GAM-shaped adapter_config_dict (with network_code,
refresh_token, etc.) even for non-GAM adapters. Non-GAM adapters store config in config_json,
not dedicated columns, so the template received an empty dict.

Fix: Branch on adapter_config_obj.adapter_type:

if adapter_config_obj.adapter_type == "gam":
    adapter_config_dict = {"network_code": ..., "refresh_token": ..., ...}
else:
    # Schema-driven adapters store config in config_json
    adapter_config_dict = dict(adapter_config_obj.config_json or {})

AdCP relevance: None — pure Flask/admin UI fix. Relevant to any project supporting multiple
adapter types.

---

7. Secret-preservation in adapter config save

File: src/admin/blueprints/adapters.py

Bug: Saving the adapter config form overwrote the stored api_key with an empty string when
the user didn't re-enter it (form shows •••••••• placeholder, not the real value).

Fix: Load existing config before saving; preserve stored key when submitted value is blank or
the placeholder sentinel:

if not new_api_key or new_api_key == "••••••••":
    config_data["api_key"] = existing_config.get("api_key", "")

Same pattern applied to the test-connection endpoint: falls back to stored credentials when form
fields are blank.

AdCP relevance: None — pure admin UI UX fix.

---

8. implementation_config field on MediaPackage

File: src/core/schemas/_base.py

Adds implementation_config to MediaPackage as an escape hatch for adapter-specific per-package
configuration. Marked with Field(exclude=True) so it is never serialized to the wire:

implementation_config: dict[str, Any] | None = Field(
    default=None, exclude=True
)  # Internal: adapter-specific per-package config. Not serialized to wire.

AdCP 3.1 validation:

MediaPackage is an internal model (not a generated AdCP type), so no spec constraint is
violated. Field(exclude=True) is the correct pattern per the Python client extension guide —
it fires automatically at all nesting depths and cannot be forgotten at a call site.

Question for code owners: Is dict[str, Any] the right type here, or should this be a
typed model (e.g. AdapterPackageConfig)? The ext pattern (dict[str, Any]) is a spec-level
escape hatch for wire-level extension data from unknown parties — an internal adapter config has
a known shape and could be typed more narrowly.

---

Questions for code owners

1. Change 4 follow-up: Should _KNOWN_ASSET_TYPES be derived from the full 19-member asset
   type vocabulary (per AdCP 3.1) rather than the SDK discriminated union? This would future-proof
   the validator against new asset types added to the spec. Currently "published_post", "zip",
   "vast_tracker", "daast_tracker", "pixel_tracker", and "object" are still silently
   dropped.

2. Change 8: Is dict[str, Any] acceptable for implementation_config, or should we define a
   typed AdapterPackageConfig base model that adapters extend?

3. Scope: Changes 6 and 7 are admin UI fixes specific to the multi-adapter config page. Are
   these in scope for the upstream project, or should they be kept in the fork?

---

Files changed

File	Changes
src/core/tools/creatives/_processing.py	Changes 1, 2, 5
src/core/creative_agent_registry.py	Changes 3, 4
src/core/tools/creatives/_assets.py	Change 5
src/core/helpers/creative_helpers.py	Change 5
src/core/schemas/_base.py	Change 8
src/admin/blueprints/tenants.py	Change 6
src/admin/blueprints/adapters.py	Change 7

---

Test coverage

• _find_format normalization → tests/unit/test_processing_helpers.py
• build_creative via ADCPMultiAgentClient → mock registry tests
• _KNOWN_ASSET_TYPES / url asset type → tests/integration/test_creative_sync_processing.py
• Brand propagation → tests/unit/test_create_media_buy_behavioral.py

[KonstantinMirin]

That's cool. Will take a deeper look but quick notes:

1. Please verify that these changes would hold against v3.1-rc.1 at least. That's where we pinned requirements as of a week ago -- see dev call meeting notes in slack
2. We always use typed models, extended from the SDK types as needed. Anything that contradicts this is legacy and should be refactored. Model_dump calls are a marker if this mistake: there should be no model-dict conversions in the app, that is serialization boundary.

If you're looking into creatives, there is a more interesting refactoring: we ended up with Creative and CreativeAsset models. The former is from the otriginal implementation and carries fields for order insertion to gam that are not strictly about the creative. That's a separate issue, if you want to explore.

[torbenbrodt]

I attached a PR #1482 - it still requires some more work, some tests are failing. Shall I continue working on the PR or are there any refactorings taking place, which will conflict?

[torbenbrodt]

@KonstantinMirin can you provide feedback if the direction is okay, before I proceed with the changes.

[KonstantinMirin]

Thanks for pushing on this, Torben — and sorry for the slow reply. Direction is good. Concrete notes before you take #1482 to green:

1. Rebase onto current main first. Main is now on adcp==5.7.0 (spec 3.1.0-beta.3). That matters: several of your "3.1 addition" items are now exercisable against the installed SDK, so we can go forward-looking on 3.1 — good. One caveat: validate each claim against what 5.7 actually ships, not against a knowledge-base description of 3.1 GA. The SDK targets beta.3 and diverges from GA in places (e.g. idempotency_key-required-on-reads and the asset_type oneOf/discriminator enforcement are GA-framing that beta.3 may not enforce). Where 5.7 can't exercise a behavior, don't bake it in as required — keep it additive/tolerant.

2. Changes 1 & 2 are correct and version-independent — land them. agent_url normalization before format compare (RFC 3986) and the structured {id, agent_url} manifest with creative_id dropped are straight bug fixes. No objection.

3. Brand must stay a typed BrandRef end-to-end. This is the one blocking direction note. _brand_str_to_ref() returning a dict, and the brand.model_dump(mode="json") conversions, are exactly the model→dict pattern we don't do — serialization is a boundary concern, not something the app does internally. Pushing the model_dump to the wrapper satisfies the test_no_model_dump_in_impl guard by letter, but the brand should never become a loose dict in the first place. Carry BrandRef as the typed model; if you need to coerce a plain string, build a BrandRef, not a dict.

4. Change 4 — don't hand-maintain a 19-member asset list. Derive _KNOWN_ASSET_TYPES from the SDK's vocabulary. If 5.7's discriminated union is narrower than the beta.3 schema, handle the gap tolerantly rather than pasting a hardcoded list that drifts the moment the spec moves. Same source-of-truth principle as everywhere else.

5. Change 8 — type implementation_config. Same reasoning as brand: dict[str, Any] is the wire-extension escape hatch, but an internal adapter config has a known shape. Lower priority since it's exclude=True, but prefer a typed model.

6. Changes 6 & 7 (admin UI) — agree keeping them out of this PR. Fine to keep in your fork or split into a separate admin-only PR; they're not part of the AdCP compliance story.

7. CI must be green via the full integration + BDD suite, not just unit — that's our bar for "done" on anything touching the sync pipeline.

On the separate Creative/CreativeAsset split I mentioned: still a real refactor worth doing, but keep it out of this PR — it'll only make review harder.

Net: proceed. Rebase, make brand typed, ground the 3.1 claims in 5.7, get it green.