Repository navigation
fix(projection): send undeclared format options as direct selectors on the canonical wire - #3194
Conversation
…n the canonical wire A 3.x seller may return products with only legacy format_ids. getProducts() mints format_option_ids for those options, but format_option_refs must match the seller's own format_options[], so the seller cannot resolve them. Record at discovery which options the seller's raw response declared. On the canonical wire, createMediaBuy and updateMediaBuy send a package selecting undeclared options as the equivalent format_kind + params selector, and syncCreatives drops creative pins to them. A selection one direct selector cannot express fails closed before dispatch. Seller-declared options, including SDK-minted ids from SDK-built sellers, are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean witness-not-translator fix with no blocking or medium findings.
Checked:
- Core change stops
createMediaBuy/updateMediaBuy/syncCreativesfrom sendingformat_option_ids a 3.x seller never declared, rewriting them to the equivalentformat_kind+paramsdirect selector and dropping creative pins (creative-delivery.ts:242). Selections one selector can't express fail closed withADCP_CREATIVE_FORMAT_PROJECTION_FAILEDbefore dispatch — correct fail-closed posture, no fabrication. - Origin decided by comparing the seller's authoritative
get_productsagainst the SDK-upgraded product (SingleAgentClient.ts:2680-2706), so seller-declaredmigrated_*ids still ship as refs — preserves wire fidelity. - New symbols stay internal (not re-exported from index.ts or v2/projection barrel), so "no public API change" holds and the
patchchangeset is correct per changeset-vs-wire-impact rule. - Tests cover create/update rewrite, sync pin removal, fail-closed refusal, and declared-id preservation.
- Pure projection logic — no new I/O, timeouts, or durable state.
Two acknowledged non-blocking follow-ups (cross-process route rebuild keeps old behavior; no-productId path returns routes[0] without selector-agreement check, near-nil risk) are noted but not actionable here.
No findings. high_risk false, gated_paths false, no team gate. Falls through to approve (row 9).
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking or medium findings.
This PR fixes a witness-not-translator bug: the canonical wire was emitting format_option_refs the seller never declared (SDK-minted ids from legacy format_ids). It now sends the equivalent format_kind+params direct selector, drops undeclared creative pins, and fails closed with ADCP_CREATIVE_FORMAT_PROJECTION_FAILED when a single selector can't express the selection. Seller-origin evidence is held out-of-band in a WeakMap propagated through all three metadata seams and preserved through the product cache (structuredCloneWithLegacyCreativeMetadata), so cache-hit and conditional-revalidation routes recover the verdict and fresh seller declarations override it.
Checked:
- Witness-not-translator: the change removes fabricated refs rather than introducing them — a correctness improvement.
- Changeset present and defensible as
patch(additive helpers + request-wire correctness fix, no export/response-shape break). - Comprehensive tests cover cache hits, revalidation, multi-account isolation, and fail-closed paths.
Gates: gated_paths false, high_risk false, no no-auto-approve team match. review_decision is REVIEW_REQUIRED but row 2 only fires when gated_paths is true — it is not here. Prior decision was approve with no findings; this run is consistent. No rows 1–8 fire, so row 9 applies.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean subsequent review, no blocking findings.
This is a subsequent push building on a prior approve. The delta is a single change: the tarball budget in scripts/check-package-size.mjs was bumped 20,360,000 → 20,390,000 bytes. The new budget minus the measured artifact (20,364,177) leaves ~25,823 bytes, preserving the existing ~25 KB compressor margin. Unpacked, file-count, schema, and declaration budgets stay fixed. This tracks actual artifact size with no behavior change. A changeset (.changeset/canonical-wire-undeclared-format-options.md) is present covering the src/lib/** changes.
No critical, high, or medium findings in this run. high_risk is false, gated_paths is false, and no no-auto-approve team match. None of decision rows 1–8 fire; falls through to row 9.
Note: review_decision is REVIEW_REQUIRED, but row 2 only applies when gated_paths is true (it is false here), so no escalation is required.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking findings.
The reviewer found no Critical/High/Medium findings. The delta adds opt-in snapshot compaction (compactSupersededSnapshots), a legacyRevisionReader fallback, a {revision_key} key-template guard, and storyboard authored-account precedence — all with matching changesets, docs, and tests. Verified items of note:
- Phase-1 re-check UPDATE correctly scopes
$5to a single revision; phase-2 shared-chunk fence preserves objects still referenced by a live row set (confirmed by PG tests). - Legacy fallback triggers only on the SDK-not-owned case (SDK_REVISION_NOT_OWNED); expired/corrupt/cursor/capacity errors throw first, so it cannot serve mismatched content under an SDK-owned id — no witness-not-translator violation.
- Storyboard authored-ID precedence intentionally reverses prior #1487/#1505 guards, documented in the changeset and migration note, fully re-tested.
Gate checks: gated_paths is false, high_risk is false, no no-auto-approve team match. review_decision is REVIEW_REQUIRED, but row 2 only applies when gated_paths is true — it is not, so the gate does not fire. Prior decision was approve with no findings; this run is consistent. No decision-table rows 1–8 fire → row 9 approve.
Problem
AdCP 3.x lets a product carry only legacy
format_idsduring the migration window. When a seller on the canonical creative wire (3.2, or 3.1 withcanonical_creatives: true) returns such a product,getProducts()upgrades it and mints aformat_option_idfor each option (migrated_<hash>, or a catalog-derived id).createMediaBuy/updateMediaBuythen sent those ids to the seller asformat_option_refs. The 3.2 schema requires each reference to "match one target productformat_options[]entry", so a seller that only declaredformat_idshas nothing to resolve them against. It rejects them withUNSUPPORTED_FEATUREor ignores them. Creativeformat_option_refpins onsync_creativeshad the same problem.Reproduced end to end over MCP: the seller received
format_option_refs: [{ scope: 'product', format_option_id: 'migrated_a592…' }]for a product whoseget_productsresponse contained onlyformat_ids.Fix
get_productsproduct with the canonical result and records whether the seller declared each option in its ownformat_options[].createMediaBuy,updateMediaBuy(packagesandnew_packages) andsyncCreativesrunprojectUndeclaredFormatOptionSelectorsafter the existing delivery projection:format_kind+params. That is the 3.2 route for addressing a product option without an id, and it avoidsformat_ids, which new buyers MUST NOT emit.CreativeFormatProjectionErrorbefore dispatch. Custom creative pins are refused rather than stripped of their identity.format_option_refpins to undeclared options are dropped after creative normalization;format_kindstays, including when normalization generated the pin.format_option_refs. That includes themigrated_*ids SDK-built sellers mint server-side (asCanonicalProductResponse→toCanonicalOnlyResponse), because those sellers declare the option on the wire. So origin is decided by comparing against the seller's response, not by the id prefix.There is no public API change. The legacy-wire path is untouched.
Changed test
media-buy-lifecycle-release-gate.test.jsasserted that a 3.1/3.2 seller whose product declares onlyformat_idsreceives the SDK-mintedformat_option_refs. That fixture validates requests against the schema but never checks option ids against its products, so the assertion recorded the bug. It now expectsformat_kind+params. The rewritten requests pass the fixture's 3.1.18 and 3.2.1 request-schema validation, and all 11 tests in the file pass.Scope
Undeclared options are recognised from this client's own
getProducts()discovery. An adopter that rebuilds routes from storage in another process (canonicalFormatLegacyResolverFromRoutes) keeps today's behaviour; carrying the origin onCanonicalFormatLegacyRoutewould be a follow-up.Testing
test/lib/canonical-wire-undeclared-format-options.test.js(in-process MCP seller): create and update rewrite, sync pin removal, refusal before dispatch when one selector can't express the selection, and seller-declaredmigrated_*ids still sent as refs. Also covers product-cache-enabled uncacheable responses, cache writes/hits, conditional revalidation, seller declaration refresh, and pins generated during creative normalization.npm run test:node:fastpasses locally, run with Postgres 16 (as CI),opensslinstalled, andADCP_REGISTRY_API_KEYunset.npm run test:node:slow: 968/968 pass.🤖 Generated with Claude Code