Skip to content

spec(adcp): discriminator audit — every oneOf needs a discriminator key + CI assertion #3917

@bokelley

Description

@bokelley

Summary

AccountReference = { account_id } | { brand, operator, sandbox? } has no spec-level discriminator. Every adapter we built (and the four reviewer agents) collapsed the union via (ref as { brand?: { domain?: string } }) and silently dropped the account_id arm. Real adopters who wire sync_accounts will hit the gap because buyers reference accounts by account_id post-sync.

This isn't an AccountReference-only issue. Other unions in AdCP 3.x with the same risk:

  • BuildCreativeReturn (CreativeManifest | CreativeManifest[] | BuildCreativeSuccess | BuildCreativeMultiSuccess)
  • PreviewCreativeResponse (single | batch | variant) ✓ has response_type
  • MediaBuyDeliveryNotification variants
  • ActivationKey ✓ has type
  • AssetVariant ✓ has asset_type
  • Format.renders[] items (dimensions vs parameters_from_format_id) — fixed via codegen tightening (feat(dashboard): escalation visibility and invoice draft-confirm flow #1325)
  • Every *Success | *Error response union (most of AdCP 3.x)
  • AsyncResponseFor<T> family
  • SignalID ✓ has source

Some unions have discriminators. Many don't. The ones without push the narrowing burden onto every adopter in every codegen target — TS, Python, Go, Java — and every adopter writes the same buggy cast.

Proposed fix

  1. Audit pass: for every oneOf in schemas/cache/latest/, classify:
    • ✓ Has a stable discriminator key (kind, source, response_type, asset_type, output_format, type).
    • ⚠ Doesn't have one but variants have a structurally distinguishing required field (e.g. account_id vs brand on AccountReference).
    • ✗ Variants overlap structurally and need a discriminator added (the dangerous case).
  2. Add discriminators to every ⚠ and ✗ union. For AccountReference specifically: add kind: 'account_id' | 'identity' (or similar) so TS/Python/Go can narrow without 'account_id' in ref archaeology. JSON Schema Draft 2020-12 discriminator keyword OR a sentinel propertyName constant per branch.
  3. Update the JSON Schema to declare discriminator: { propertyName: 'kind' } on every oneOf. Codegen targets that support discriminators (jsts with the right config, msgspec, openapi-generator) emit narrower types.

CI assertion

Schema-walker on every oneOf that asserts:

  • Either discriminator: { propertyName: ... } is set, OR
  • Every variant declares the same properties.<key>.const value (and they're distinct across variants).

Output on failure: oneOf at /core/account-ref.json#/ has no discriminator and no shared const-property — add { kind: 'account_id' | 'identity' } or equivalent.

This is a single-pass schema walker. ~50 lines of TS / Python in the spec repo's CI. Catches every regression on every spec PR going forward.

Why on adcp not adcp-client

The audit is on the spec; codegen targets are downstream. Filing on adcontextprotocol/adcp so the discriminator fix lands at the source and propagates to adcp-client (TS), adcp-py, etc.

Acceptance

  • Audit table published as a PR description listing every oneOf and its current discriminator status.
  • AccountReference gains kind discriminator; codegen produces a properly-narrowed union in TS.
  • CI assertion in adcp spec repo fails any PR that adds a oneOf without a discriminator.
  • adcp-client's generated TS for AccountReference makes 'account_id' in ref unnecessary.

Refs

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    claude-triagedIssue has been triaged by the Claude Code triage routine. Remove to re-triage.enhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions