Skip to content

feat: update AdCP beta.7 schema support#2086

Merged
bokelley merged 1 commit into
mainfrom
v3-1-0-beta-7-release
May 28, 2026
Merged

feat: update AdCP beta.7 schema support#2086
bokelley merged 1 commit into
mainfrom
v3-1-0-beta-7-release

Conversation

@bokelley

@bokelley bokelley commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Bumps the SDK AdCP schema pin and generated surfaces to 3.1.0-beta.7 while keeping the package metadata at 8.1.0-beta.14.
Adds beta.7 compliance-controller support for creative purge, measurement-catalog seeding, provenance audit queries, upstream-unavailable forcing, and brings SDK seed/auto-seed scenario projection in line with the existing beta compliance response enum.
Hardens beta.7 codegen for unsupported z.record size constraints and updates media-buy seed/type fixtures for required confirmed_at/revision lifecycle fields.

Validation

npm run build:lib; npm run typecheck; npm run format:check; targeted node --test suites for controller/seed wiring, version/schema, and zod postprocessors; git diff --check.

@bokelley bokelley marked this pull request as ready for review May 28, 2026 10:59
@bokelley bokelley changed the title [codex] Update AdCP beta.7 schema support feat: update AdCP beta.7 schema support May 28, 2026
aao-ipr-bot[bot]
aao-ipr-bot Bot previously approved these changes May 28, 2026

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean beta.5 → beta.7 sync; the witness-not-translator invariant holds across every new scenario dispatcher.

Things I checked

  • Changeset is present, marked minor, and the narrative names every behavior delta (force_creative_purge, seed_measurement_catalog, query_provenance_audit_observations, compliance_testing.scenarios widening, and the beta-as-primary-pin restore guard). Wire impact matches.
  • Exhaustiveness guard at src/lib/server/test-controller.ts:211-216 still resolves to neverCONTROLLER_SCENARIOS (12) ∪ SeedScenario (8) covers the widened ListScenariosSuccess.scenarios union, so the build fails if the next beta widens it again without a CONTROLLER_SCENARIOS update.
  • compliance_testing.scenarios widening is spec-driven, not SDK fiat — verified against static/schemas/source/protocol/get-adcp-capabilities-response.json upstream. The prior "deliberately not advertised" comment was correct at beta.5; beta.7 explicitly enumerates seeds + new force/query scenarios excluding list_scenarios. deriveScenariosFromAdapters() and advertisedScenarios() are 1:1 consistent.
  • CreateMediaBuySuccess / UpdateMediaBuySuccess required-field additions (confirmed_at: string|null, revision: integer) reflect the upstream schema's required block. The confirmed_at: null × media_buy_status: active allOf guard exists upstream — fixtures honor it. decisioning.type-checks.ts factoring into _createBuyPayload / _updateBuyPayload is applied at every call site that previously inlined the literal.
  • Auto-seed projection move at src/lib/server/decisioning/runtime/from-platform.ts:1097-1130autoSeedStore computed before the projection block now lets seed_product / seed_pricing_option flow through compliance_testing.scenarios when the framework wires the no-op seed adapters. Consistent with list_scenarios output.
  • sync-3-1-beta-schemas.ts:97 guard if (primary !== BETA_VERSION) is right — when the beta IS the primary pin (today's state), syncSchemas already wrote the authoritative tracked artifacts; restoring from HEAD would clobber them. The latest-symlink restore still runs unconditionally, which is also correct.
  • seed_measurement_catalog vendor-only cache key is fine: divergence check at line 991-998 runs fixturesEquivalent against { ...params, fixture } (which includes metrics), so same-vendor + different-metrics gets INVALID_PARAMS rather than a silent collision.
  • New scenario dispatchers (force_creative_purge, seed_measurement_catalog, query_provenance_audit_observations) are pure routers — destructure, validate, forward. No fabrication, no normalization, no placeholder substitution. Test coverage matches at test/server-test-controller.test.js and test/comply-controller.test.js.

Follow-ups (non-blocking — file as issues)

  1. GetMediaBuysResponse.media_buys: unknown[] is codegen drift, not a spec regression. Upstream static/schemas/source/media-buy/get-media-buys-response.json defines media_buys.items as a structured object with seven required properties. Beta.7 codegen drops them at src/lib/types/tools.generated.ts:10229. The bridge's local narrowing at src/lib/server/test-controller-bridge.ts:138 is a reasonable workaround but masks the regression for adopters who imported GetMediaBuysResponse['media_buys'][number]. File against scripts/generate-3-1-beta-types.ts + scripts/generate-zod-from-ts.ts — the items+additionalProperties handling likely drops the sibling object schema. Worth landing before any consumer pins ^@adcp/sdk against 3.1-beta.7 type imports.

  2. postProcessRecordSizeConstraints has two latent gaps. At scripts/generate-zod-from-ts.ts:284-321: (a) only the outermost z.record( is post-processed, so a nested z.record(z.record(...).max(5)) leaves the inner .max(5) to break Zod v4 at runtime; (b) the trailing-constraint regex ^\.(?:min|max|length)\(\d+\) matches once, so z.record(...).min(1).max(5) strips only .min(1). Neither triggers on current beta.7 output. Wrap the regex in a while and extend the test fixture in test/generate-zod-object-intersections.test.js with both shapes — regression-lock today, not when an upstream schema actually emits them.

  3. summarize() fallthrough on ProvenanceAuditObservationsSuccess. src/lib/server/test-controller.ts:1388-1406 has no branch for audit_observations, so the response's human-readable text content is "Scenario succeeded". Asymmetric with the sibling recorded_calls branch one line above. Adopters consume structuredContent, so wire impact is nil — cosmetic only.

Safe to merge.

@bokelley bokelley force-pushed the v3-1-0-beta-7-release branch 2 times, most recently from 93232c3 to 95c5262 Compare May 28, 2026 11:25
aao-ipr-bot[bot]
aao-ipr-bot Bot previously approved these changes May 28, 2026

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Follow-ups noted below. Clean schema bump — synced enum (inline-enums.generated.ts:329) backs the four new scenarios, auto-seed projection wires real adapters into autoSeedStore rather than fabricating, and the SeededMediaBuy local widening at test-controller-bridge.ts:138 is defensive narrowing against a generator regression, not wire fabrication.

Things I checked

  • Changeset .changeset/adcp-3-1-beta-7.md present, marked minor. Correct: additive scenarios + permissive type widening. No exported-API removal, no required→optional flips. package.json version untouched.
  • ControllerScenario widening (test-controller.ts:137-141) brings the type into line with the synced ListScenariosSuccess['scenarios'] union and the runtime SCENARIO_MAP that already emitted seeds. The exhaustiveness guard at :212-217 enforces the union — codegen drift fails the build.
  • advertisedScenarios() (comply-controller.ts:603-633) and deriveScenariosFromAdapters() (from-platform.ts:2207-2231) now advertise the same set. Two paths converged.
  • force_creative_purge validates purge_kind against CreativePurgeKindSchema and reason_code against CreativeEventReasonCodeSchema (test-controller.ts:1234-1248). Right shape.
  • force_upstream_unavailable requires params.tool to be a non-empty string before invoking the adapter (test-controller.ts:1351). Fail-closed.
  • dispatchSeed prototype-pollution guard (test-controller.ts:831-838) extended cleanly to seed_measurement_catalog (:931-946) and seed_buyer_agent (:970-977).
  • postProcessRecordSizeConstraints (generate-zod-from-ts.ts:284) uses balanced-paren scanning consistent with the other post-processors, handles quoted literals. Strips spec-irrelevant .min/.max/.length from z.record(...) only.
  • sync-3-1-beta-schemas.ts:36-41 restores RESTORE_PATHS (registry.yaml + protocol skills) from HEAD after the beta sync runs — opt-in beta doesn't bump the primary pin's tracked artifacts. Correct.
  • compliance_testing.scenarios type narrowed via _ComplianceTestingScenario = NonNullable<...['scenarios']>[number] (capabilities.ts:33-36) instead of the hand-maintained string-literal union. Removes the prior drift surface.
  • Auto-seed advertising (from-platform.ts:1100-1119): default () => undefined adapters are placeholders that flag the discovery surface — the actual seeded fixtures are persisted into autoSeedStore and served on subsequent get_products via the bridge. Not fabrication; not overreach.

Follow-ups (non-blocking — file as issues)

  1. seed_measurement_catalog cache key omits metrics. test-controller.ts:941 keys only on canonicalJson(params.vendor ?? 'default'), but the stored fixture is { ...params, fixture } — which includes metrics. Two seeds from the same vendor with different metric arrays will hit the cache, fail fixturesEquivalent, and surface INVALID_PARAMS even though they're legitimately distinct catalogs. The SeedMeasurementCatalogParams docstring on comply-controller.ts:123-128 calls out the vendor+metrics composite key. Fix: seed_measurement_catalog:${canonicalJson(params.vendor)}:${canonicalJson(params.metrics)}.

  2. postProcessRecordSizeConstraints only strips one trailing call. generate-zod-from-ts.ts:316 matches ^\.(?:min|max|length)\(\d+\) once. If ts-to-zod ever emits a chain like .max(10).min(1), the second call survives and breaks the Zod v4 build silently. Loop until no match.

  3. query_provenance_audit_observations type-launders creative_id. test-controller.ts:1334 casts params as { creative_id: string } after a truthy check only; a numeric creative_id: 123 reaches the adopter typed as a string. Add typeof params.creative_id !== 'string' rejection to match the guard force_upstream_unavailable has at :1351.

  4. scripts/sync-3-1-beta-schemas.ts:24 pulls from main/dist, not a release tag. GITHUB_DIST_BASE_URL = '.../adcp/main/dist' means the cached bundle is whatever was on adcontextprotocol/adcp main at sync time. Confirm v3.1.0-beta.7 is a published Git tag — otherwise the pin is to a moving snapshot and re-runs could pull a different bundle under the same version directory.

  5. GetMediaBuysResponse.media_buys: unknown[] is an upstream regression. core.generated.ts:17441 types the array as unknown[] rather than the structured row type, which is why test-controller-bridge.ts:138 had to widen SeededMediaBuy locally. The defensive widening is right for this PR; file upstream on adcontextprotocol/adcp so the next sync restores the structured item type.

Minor nits (non-blocking)

  1. Redundant fixture spread in comply-controller.ts:524-532. seedMeasurementCatalog adapter constructs { ...params, vendor: params.vendor, metrics: params.metrics, fixture: params.fixture ?? {} }. The spread already places vendor and metrics; the explicit re-assignments are pure no-ops. Either drop them or comment why they're load-bearing.

  2. Parity divergence between deriveScenariosFromAdapters() and advertisedScenarios(). The former drops force_audience_status, force_catalog_item_status, seed_buyer_agent (not in the wire enum yet); the latter includes them as extension casts. The divergence is intentional — from-platform.ts advertises only spec-canonical scenarios on the wire; the comply-controller layer accepts extensions in list_scenarios per the open-extension contract. A one-line // extensions excluded — see comply-controller.ts for the open-for-extension list near from-platform.ts:2207 would save the next maintainer a grep.

Approving on the strength of the protocol-coherence verdict (synced enum matches, auto-seed wires real fulfillment) plus the absence of any wire-shape fabrication or removed surface. The seed_measurement_catalog cache-key bug is the only thing I'd want fixed promptly, but it only surfaces under multi-metric seeding from the same vendor — opt-in scenario, no data-corruption path.

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Schema bump tracks the upstream beta.7 contracts cleanly. One open question about the changeset grading; the rest are follow-ups.

Things I checked

  • Changeset present at .changeset/adcp-3-1-beta-7.md, descriptive, named consistently with prior adcp-3-1-beta-N files.
  • ADCP_VERSION bumped at ADCP_VERSION:1, package.json adcp_version:449, src/lib/version.ts:12, with COMPATIBLE_ADCP_VERSIONS extended additively (beta.5 still present).
  • list_scenarios semantics change confirmed against the spec: ListScenariosSuccess_ScenariosValues and compliance_testing.scenarios enums in src/lib/types/v3-1-beta/tools.generated.ts have always carried seeds; the old Exclude<..., SeedScenario> was the SDK lone-holdout. The new behavior is spec-aligned, not an SDK opinion.
  • New scenario names (force_creative_purge, seed_measurement_catalog, query_provenance_audit_observations, force_upstream_unavailable) match the beta.7 spec exactly. purge_kind validated against the literal 'soft' | 'hard'; reason_code runs through CreativeEventReasonCodeSchema at src/lib/server/test-controller.ts:1240-1248. Witness, not translator.
  • complyTestForProjection at src/lib/server/decisioning/runtime/from-platform.ts:1109-1119 correctly stays scoped to capability projection — the runtime complyTest flows through ...opts and never sees the () => undefined stubs. The auto-seed dispatch path runs through testController: makeAutoSeedBridge(autoSeedStore) at L1221.
  • mergeSeededMediaBuysIntoResponse at src/lib/server/test-controller-bridge.ts:885-905 defensively handles the now-unknown[] media_buys shape with explicit typeof mb === 'object' checks.
  • postProcessRecordSizeConstraints claim ("JSON-schema validation remains the source of truth") holds: no beta.7 record-typed field uses maxProperties today, and the Ajv compile path catches it if a future rev adds one.

Open question — please respond before merge

  1. Changeset grade. confirmed_at: string | null and revision: integer are now REQUIRED on CreateMediaBuySuccess (src/lib/types/v3-1-beta/tools.generated.ts confirmed via diff); revision is required on UpdateMediaBuySuccess. Server-side adopters whose createMediaBuy/updateMediaBuy handlers don't return these fields fail type-check. Prior art for the same shape — #1902, the 8.0-beta.2 cut — shipped as major. The current changeset is minor and the body is silent on this surface. Either re-grade to major, or at minimum amend the changeset to call out the required-field flip so adopters audit before bumping. If minor is intentional because we treat schema-pin bumps on the beta line as additive-by-convention, say so in the changeset prose — silence is the part I want to fix.

Follow-ups (non-blocking — file as issues)

  • GetMediaBuysResponse.media_buys: unknown[] is a codegen quirk in beta.7. The schema docstring clearly intends a structured shape but ts-to-zod fails to resolve the items $ref. The SDK's SeededMediaBuy = {media_buy_id: string; [key: string]: unknown} widening is the right witness-style choice locally; the upstream codegen bug is worth filing on adcontextprotocol/adcp so every downstream codegen consumer stops hitting it.
  • Auto-seed comment regression at src/lib/server/decisioning/runtime/from-platform.ts:1211-1217. The old ~25-line block documented the multi-tenant isolation contract (autoSeedStore keyed by account_id) and the adopter's escape hatch (wire bridgeFromSessionStore for tighter scoping). The new short comment drops both. The stale process-wide SeedFixtureCache caveat was correctly removed (per-account scoping landed in #1215), but the isolation guarantee is the load-bearing part adopters audit. Restore a one-paragraph version near the wiring at L1542.
  • seed_measurement_catalog cache key keys by vendor only. src/lib/server/test-controller.ts:941 uses seed_measurement_catalog:\${canonicalJson(params.vendor ?? 'default')}. Two distinct catalogs for the same vendor (different metrics arrays) collide and the second seed returns INVALID_PARAMS. May be intentional (one catalog per vendor), but the dispatch comment doesn't say so. Either widen the key to include a metrics hash or add a one-line invariant comment.
  • deriveScenariosFromAdapters vs advertisedScenarios are out of sync on extension scenarios. src/lib/server/decisioning/runtime/from-platform.ts:2209-2233 does not advertise seed_buyer_agent, force_audience_status, force_catalog_item_status; src/lib/testing/comply-controller.ts:622-626 does. Pre-existing, but newly visible now that seeds are advertised. Bring the two into sync.

Minor nits (non-blocking)

  1. Prototype pollution claim on seed_measurement_catalog — worth a sanity check, not a block. src/lib/server/test-controller.ts:939 builds measurementCatalogFixture = { ...params, fixture } and forwards the whole params object to the adapter, while other seed scenarios extract specific scalars. Spread operator semantics since ES2018 use [[DefineOwnProperty]], so a JSON-parsed __proto__ arrives as a regular own property rather than walking the prototype chain — the actual exploit path is narrow. Still, extracting vendor + metrics + fixture explicitly like the other seed branches is cheaper than reasoning about every adopter's downstream merging.
  2. Chained-call regression test missing. test/generate-zod-object-intersections.test.js:163-175 covers the four basic strip/preserve cases but doesn't lock the z.record(...).optional().max(5) chained case the comment in scripts/generate-zod-from-ts.ts implicitly promises to handle. One assertion would close the loop.
  3. hello_seller_adapter_guaranteed.ts:1058revision: localBuyRevisions.get(o.order_id) ?? 1 silently reports revision: 1 for any buy loaded from upstream state without going through this process's create_media_buy. Fine for a worked example; one-line comment would prevent buyers from copy-pasting it into production.

LGTM modulo the changeset question — happy to flip to approve once that's either re-graded or explained.

aao-ipr-bot[bot]
aao-ipr-bot Bot previously approved these changes May 28, 2026

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Faithful beta.7 reflection — SDK correctly tracks the widened compliance_testing.scenarios enum (which now admits seeds per spec), the four new controller scenarios, and the new field-required shape on CreateMediaBuyResponse. "Witness, not translator" — the previous beta.5 hand-narrowing was the bug; deriving from the generated union in capabilities.ts:34-36 is the correct fix.

Things I checked

  • force_creative_purge / force_upstream_unavailable / query_provenance_audit_observations / seed_measurement_catalog are all in beta.7's ComplyTestControllerRequest.scenario and ListScenariosSuccess.scenarios enums per src/lib/types/v3-1-beta/tools.generated.ts:22481,23184. SDK is not fabricating scenarios.
  • deriveScenariosFromAdapters() projection in from-platform.ts:2208-2231 now correctly pushes seed scenarios — beta.7's GetAdCPCapabilitiesResponse.compliance_testing.scenarios enum (tools.generated.ts:20552-20572) enumerates the seed values. The dropped "spec narrowed to forces+simulates" comment was stale.
  • New dispatch arms in test-controller.ts: Zod validations short-circuit before adapter invocation (force_creative_purge L1226-L1250, force_upstream_unavailable L1346-L1360). params stripping (creative_id / tool) is clean before spread.
  • postProcessRecordSizeConstraints (generate-zod-from-ts.ts:284-321) — regex /^\.(?:min|max|length)\(\d+\)/ is anchored and tight, won't eat .minLength( or .max(z.unknown()). Wired before postProcessRecordIntersections at L1681 — correct order.
  • Pre-release mode is active (.changeset/pre.json mode=beta), so the minor changeset rolls into the next beta. Appropriate.

Follow-ups (non-blocking — file as issues)

  1. Changeset undersells wire impact. CreateMediaBuyResponse.confirmed_at: string | null and CreateMediaBuyResponse.revision: number are now non-optional on the success branch (v3-1-beta/tools.generated.ts:12274,12282); UpdateMediaBuyResponse.revision: number likewise at :12726. Buyer agents deserializing into typed clients will hard-fail on responses that omit these. The example-file additions are spec-required, not cosmetic. Recommend a "Wire-impact" header in the changeset before primary pin moves.
  2. force_upstream_unavailable missing from changeset prose. The body at .changeset/adcp-3-1-beta-7.md:10-12 lists three new scenarios but the PR ships four.
  3. STALE_RESPONSE error code addition (enums.generated.ts:80) deserves a callout — force_upstream_unavailable doc-comment mandates error.details.upstream.name echo. Adopters who wrap controller errors need to know.
  4. GetMediaBuysResponse.media_buys: unknown[] codegen regression. Spec doc-comment at tools.generated.ts:12883 describes a typed element shape ("media buys with status, creative approval state, and optional delivery snapshots") but no MediaBuy interface is emitted — element widened to unknown[]. Likely a $ref resolution miss in json-schema-to-typescript. The bridge's local SeededMediaBuy retype at test-controller-bridge.ts:138 is a reasonable contention strip, but don't ship beta.7 to GA with unknown[] on a primary read path. Track as a codegen issue (or file against adcontextprotocol/adcp if the schema source isn't $ref-disciplined).
  5. SeededMediaBuy public-type widening. Type changed from GetMediaBuysResponse['media_buys'][number] (the strong lookup) to { media_buy_id: string; [key: string]: unknown }. Adopters who imported SeededMediaBuy from @adcp/sdk/server lose autocomplete on status, packages, etc. Public-surface change worth one line in the changeset.

Minor nits (non-blocking)

  1. CreativePurgeKindSchema hand-authored. test-controller.ts:603 declares z.enum(['soft', 'hard']) at module scope, but the generated artifacts already carry this — inline-enums.generated.ts:168 exports CreativePurgedWebhook_PurgeKindValues = ["soft", "hard"] as const. Replace with z.enum(CreativePurgedWebhook_PurgeKindValues) so a future beta widening (e.g., 'tombstone') flows through automatically instead of silently rejecting valid purge kinds at L1236.
  2. seed_measurement_catalog presence check is inconsistent with peers. test-controller.ts:932 uses Object.hasOwn(params, 'vendor') which lets vendor: null / vendor: undefined pass while every other seed branch (seed_product L847, seed_plan L890, etc.) uses truthy params?.X. Either tighten for symmetry or add a one-line comment that null-vendor is intentional pass-through.
  3. wrapStoreSuccess double cast. test-controller.ts:674-677 as unknown as ComplyTestControllerResponse masks the per-arm narrowing the generated union provides. Tightening the signature to <T extends StateTransitionSuccess | UpstreamTrafficSuccessResponse | SeedSuccess | ForcedDirectiveSuccess> would let callers keep the arm type. Works at runtime; throwing away type info.
  4. force_upstream_unavailable explicit-undefined spread. test-controller.ts:1357 builds { tool, upstream_name: undefined } which is fine under the current tsconfig but breaks the moment someone enables exactOptionalPropertyTypes. Conditional spread ...(typeof forceParams.upstream_name === 'string' ? { upstream_name: forceParams.upstream_name } : {}) is forward-compatible.
  5. sync-3-1-beta-schemas.ts:97-102 comment. Gate logic is correct, but the comment block still reads as if restoreFromHead runs unconditionally. Reword: "When the beta is also the primary pin, the synced tarball already represents the primary state — skip the HEAD restore."

The seeds-now-advertised behavior change is interesting on its own — last beta the SDK type was the conservative one and the spec was the broad one; this one corrects in the right direction.

Approving on the strength of the spec alignment plus the new dispatch arms being properly Zod-gated. Ship it; address the changeset prose in a follow-up before the primary pin moves.

aao-ipr-bot[bot]
aao-ipr-bot Bot previously approved these changes May 28, 2026

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean schema bump — additive scenarios, generated surfaces regenerated, codegen workarounds scoped narrowly enough that Ajv remains the runtime source of truth.

Things I checked

  • Changeset present, typed minor. Right call — four new scenarios (force_creative_purge, query_provenance_audit_observations, force_upstream_unavailable, seed_measurement_catalog) are additive; the existing dispatcher arms didn't change shape.
  • package.json version untouched. LIBRARY_VERSION stays at 8.1.0-beta.14; ADCP_VERSION moves to 3.1.0-beta.7. COMPATIBLE_ADCP_VERSIONS (src/lib/version.ts:38) extends the list — beta.4 / beta.6 absent is intentional (never pinned).
  • Exhaustiveness guard at test-controller.ts:216 still type-checks against the regenerated ControllerScenario union. SCENARIO_MAP (test-controller.ts:606-626) registers the four new entries against CONTROLLER_SCENARIOS / SEED_SCENARIOS.
  • Envelope status invariant holds. Success paths route through wrapStoreSuccess() (test-controller.ts:674) stamping status: 'completed'; error paths route through controllerError() (L645-662, comply-controller.ts:477-481) stamping status: 'failed'. Matches ComplyTestControllerResponse.status: TaskStatus since beta.2.
  • Witness-not-translator preserved. force_creative_purge (test-controller.ts:1226-1250) validates purge_kind against CreativePurgeKindSchema (z.enum(['soft','hard'])) and reason_code against the spec-shipped CreativeEventReasonCodeSchema — no fabrication, no silent defaulting. force_upstream_unavailable uses explicit typeof === 'string' narrowing (L1351). Prototype-pollution guard at L831-838 covers the new seed_measurement_catalog path via the shared dispatchSeed.
  • applyCodegenSchemaWorkarounds for GetMediaBuysResponse (scripts/generate-types.ts:1022-1060) is scoped by name and only drops allOf members whose keys are exclusively {$comment, if, then, else} — won't over-strip a mixed-content member, and Ajv still validates against the unstripped cached JSON. Legitimate jsts-emitter workaround.
  • postProcessRecordSizeConstraints (scripts/generate-zod-from-ts.ts:284-321) only fires after z.record( literally; z.array(z.string()).max(5) is left untouched. Balanced-paren scanner handles quoted strings and escapes. Pipeline placement (after postProcessUndefinedUnions, before postProcessRecordIntersections) is right.
  • sync-3-1-beta-schemas.ts bumps BETA_VERSION to 3.1.0-beta.7. RESTORE_PATHS still matches what syncSchemas overwrites as a side effect. When primary IS the beta (current state), restoreFromHead is correctly skipped.
  • Official MCP SDK used throughout. registerTestController (test-controller.ts:1499-1543) goes through mcp.registerTool; no custom HTTP / SSE.
  • Test plan in PR body lists build:lib / typecheck / targeted controller / version / postprocessor suites + git diff --check. Nothing unchecked.

Follow-ups (non-blocking — file as issues)

  • src/lib/server/test-controller.ts:941 — the seed_measurement_catalog cache key is seed_measurement_catalog:${canonicalJson(params.vendor ?? 'default')}, ignoring metrics. Two storyboards seeding the same vendor with different metric sets will hit the cache, trip fixturesEquivalent, and surface INVALID_PARAMS. Failure mode is loud, not silent, but it forces adopters into one-vendor-per-metric-catalog. Either widen the cache key to (vendor, metric_id-set) or document the constraint in the store JSDoc.
  • src/lib/server/test-controller.ts:1330QUERY_PROVENANCE_AUDIT_OBSERVATIONS uses truthiness (!params?.creative_id) and then unsafe-casts. The new force_upstream_unavailable arm at L1351 already uses explicit typeof === 'string' narrowing — tighten this one to match the new pattern.
  • scripts/sync-3-1-beta-schemas.ts:97 — when primary === BETA_VERSION (true today), restoreFromHead(RESTORE_PATHS) is correctly skipped, but a one-line comment at the conditional would keep a future refactor from dropping the guard.

Minor nits (non-blocking)

  1. force_creative_purge params passthrough. test-controller.ts:1233 rest-spreads purgeParams to the adapter after validating only purge_kind and reason_code. Consistent with simulate_delivery's open-ended signature ([key: string]: unknown) and not a regression — just noting that the prototype-pollution guard at L831-838 is fixture-scoped, not params-scoped, across all force_* arms.

Safe to merge.

aao-ipr-bot[bot]
aao-ipr-bot Bot previously approved these changes May 28, 2026

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Schema pin bump is clean, the new scenarios are wired correctly, and the codegen workarounds (applyCodegenSchemaWorkarounds for the new media_buys.items if/then/else guard, postProcessRecordSizeConstraints for unsupported Zod v4 record size calls) are scoped to the TS/Zod emit path — Ajv still loads the un-stripped schemas, so the conditional invariants remain enforced at runtime. package.json's adcp_version field moved; the library version field was correctly left alone for changesets to own.

Things I checked

  • Changeset present and correctly sized: .changeset/adcp-3-1-beta-7.md is minor — additive scenarios + new beta opt-in pin. No major removals on the public surface; SeededMediaBuy was widened, not narrowed.
  • Spec conformance on seeds-now-advertised: dist/schemas/3.1.0-beta.7/compliance/comply-test-controller-response.json list_scenarios.scenarios[] enum includes all six original seeds plus seed_measurement_catalog. The SDK was following the spec, not driving it. Interesting wrinkle: the same enum already included seeds in beta.5, so the old "deliberately NOT advertised" comment was drifting one minor ago — this PR catches up rather than aligning with a fresh beta.7 break.
  • applyCodegenSchemaWorkarounds at scripts/generate-types.ts:1015-1055 only deletes the inline if/then/else allOf when EVERY allOf member has keys ⊆ {$comment, if, then, else}. A future beta that mixes properties into the same allOf will no-op the strip and surface the jsts collapse loudly at codegen — fail-loud, not fail-silent.
  • postProcessRecordSizeConstraints quote handling at scripts/generate-zod-from-ts.ts:284-321 correctly handles '/\" with \\\\ escapes. No backticks emitted by ts-to-zod.
  • sync-3-1-beta-schemas.ts:97-103 guard if (primary !== BETA_VERSION) restoreFromHead(...) is symmetric with how latest symlinks are handled — restore the HEAD artifacts only when the beta is the opt-in track, not the primary pin.
  • New scenario validators are tight: force_creative_purge validates purge_kind against z.enum(['soft','hard']) and reason_code against CreativeEventReasonCodeSchema. seed_measurement_catalog requires both vendor and metrics. query_provenance_audit_observations requires creative_id. Tests at test/server-test-controller.test.js:516-678 and test/comply-controller.test.js:152-274 cover the happy path plus invalid-param branches.
  • BACKWARD_COMPAT_OPTIONAL_FIELDS entries for CreateMediaBuyResponse: ['confirmed_at', 'revision'] and UpdateMediaBuyResponse: ['revision'] — right posture. Buyer parsers stay tolerant of legacy v3 sellers.
  • Examples updated to emit the new required fields: examples/hello_seller_adapter_guaranteed.ts and siblings now set confirmed_at + revision.

Follow-ups (non-blocking — file as issues)

  1. mergeSeededMediaBuysIntoResponse doesn't backfill the new beta.7 required fields. src/lib/server/test-controller-bridge.ts:903 casts seeded as readonly GetMediaBuysResponseMediaBuy[] and splats. With SeededMediaBuy widened to Pick<…,'media_buy_id'> & Partial<…> & Record<string, unknown>, pre-beta.7 seed fixtures will produce wire responses missing confirmed_at/revision and fail Ajv at the seller seam. Either backfill defaults that match the provisional-buy contract (confirmed_at: null, revision: 0) or run the merged response through wire validation with a loud warning.
  2. seed_buyer_agent and the force_audience_status / force_catalog_item_status extension claims. comply-controller.ts:624 casts these as unknown as ControllerScenario because the beta.7 wire enum is closed and doesn't include them. Pre-existing for the two force_* scenarios; new for seed_buyer_agent. File on adcontextprotocol/adcp to either add an x- extension convention or open the response enum so the SDK isn't fabricating wire claims. Same problem affects strict-Ajv-running counterparties.
  3. PR description language. The summary frames the seeds-advertised change as a beta.7 alignment, but the response enum already carried seeds in beta.5 — the SDK was the one out of step. Worth correcting in the PR body so the audit trail is accurate.
  4. complyTestForProjection indirection at src/lib/server/decisioning/runtime/from-platform.ts:1109-1119 plants () => undefined no-op seed stubs solely to make deriveScenariosFromAdapters advertise seed_product / seed_pricing_option when the framework wires auto-seed on the adopter's behalf. The real dispatcher is testController: makeAutoSeedBridge at line 1221. One short comment at 1109 would prevent the next reader from chasing a phantom adapter.

Minor nits (non-blocking)

  1. force_creative_purge reason_detail is unbounded. No length cap on the free-text field. Consistent with the rest of the controller surface — but worth a follow-up bound across the lot.
  2. seed_measurement_catalog fixture-equivalence comparison at src/lib/server/test-controller.ts:945-964 includes the full params object (vendor + metrics + anything else), so unrelated params drift trigger INVALID_PARAMS on replay. Matches every other seed; not a new wart, but a one-line doc note on the seed-equivalence semantics would help adopters key replays correctly.

Notable: the SDK now emits seed_buyer_agent as a list_scenarios value despite the beta.7 response enum being closed against it. That's a pre-existing extension claim, not new in this PR — but worth surfacing alongside the other extensions so it lands as one spec issue rather than three.

Approving.

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Clean additive bump — new optional wire fields, widened scenario enum, codegen workarounds for two emit-shape mismatches; the seed-bridge merge defaults sit on the SDK side of the seam so witness-not-translator holds.

Things I checked

  • Changeset .changeset/adcp-3-1-beta-7.md present and minor matches the wire impact (additive confirmed_at / revision made optional in TS via BACKWARD_COMPAT_OPTIONAL_FIELDS; compliance_testing.scenarios widened to match the beta.7 enum).
  • package.json version stays at 8.1.0-beta.14; only the adcp_version metadata field bumped to 3.1.0-beta.7. No manual version edit.
  • Wire enum widening (from-platform.ts:2216-2234 deriveScenariosFromAdapters) matches ListScenariosSuccess_ScenariosValues and ComplyTestControllerRequest_ScenarioValues in inline-enums.generated.ts:151/329. The previously-load-bearing "seeds aren't on the wire" comment no longer holds in beta.7 — removing it is correct, not drift.
  • force_creative_purge reason_code validates against CreativeEventReasonCodeSchema (test-controller.ts:1259-1267) — same schema beta.7 wires into comply_test_controller.params.
  • seed_measurement_catalog cache key composes (vendor, sorted distinct metric_ids) (test-controller.ts:744-756) — INVALID_PARAMS on divergent replay still fires via fixturesEquivalent. Regression test at test/server-test-controller.test.js:632.
  • mergeSeededMediaBuysIntoResponse spread order at test-controller-bridge.ts:903-906 is { confirmed_at: null, revision: 1, ...mb } — fixture-provided values win. confirmed_at: null is the canonical "provisional buy" wire value per core.generated.ts:681-684; defaulting seed fixtures to provisional is the right floor.
  • applyCodegenSchemaWorkarounds predicate at scripts/generate-types.ts:1025-1040 is tight (allOf only stripped when every member has only $comment/if/then/else keys); a future schema rev adding structural allOf members keeps the conditional intact.
  • sync-3-1-beta-schemas.ts correctly conditions the restoreFromHead step on primary !== BETA_VERSION — explains the registry.yaml prose changes flowing through.
  • CI: typecheck, build, format, security, eslint, codeql, library build, pack runner, schema/strict 3.0 reference seller all green. Unit tests + adopter typechecks + reference seller still in flight at review time.

Follow-ups (non-blocking)

  • scripts/generate-zod-from-ts.ts:319 postProcessRecordSizeConstraints strips only a single trailing .max() / .min() / .length() per z.record(...). If a future codegen output chains two constraints (.max(1000).min(1)), the second one leaks through and Zod v4 records will reject the chain. Wrap the strip in a loop or use /^(?:\.(?:min|max|length)\(\d+\))+/. No chained constraints in this PR's emit, so it doesn't bite today.
  • src/lib/server/test-controller-bridge.ts:907 — the SeededMediaBuy widening to Pick<...,'media_buy_id'> & Partial<...> & Record<string, unknown> plus the as GetMediaBuysResponseMediaBuy[] cast hides genuine schema gaps from TS when a fixture omits status / currency. Risk is bounded to test/sandbox paths but worth tightening with a runtime invariant at the wire-emit boundary.
  • examples/hello_seller_adapter_social.ts:617 — the identifier-collection loop overwrites identifierType on every row; a mixed-identifier add[] (hashed_email + hashed_phone in the same batch) silently uploads everything under whichever identifier the last row used. Either bucket per type or reject mixed inputs.
  • from-platform.ts:1116 comment "makeAutoSeedBridge handles the real writes" is slightly off — the writes flow through inline autoSeed.product / autoSeed.pricing_option adapters; makeAutoSeedBridge is the read side. Comment-only.

Safe to merge.

@bokelley bokelley merged commit 476e452 into main May 28, 2026
31 checks passed
@bokelley bokelley deleted the v3-1-0-beta-7-release branch May 28, 2026 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant