Skip to content

spec(schemas): add discriminator hints to const-property oneOf unions (adcp#3917)#3928

Merged
bokelley merged 2 commits intomainfrom
bokelley/oneof-free-fixes
May 3, 2026
Merged

spec(schemas): add discriminator hints to const-property oneOf unions (adcp#3917)#3928
bokelley merged 2 commits intomainfrom
bokelley/oneof-free-fixes

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 3, 2026

Summary

Step 2 of the discriminator audit plan from #3917. Adds discriminator: { propertyName } to 16 oneOf unions in static/schemas/source/ whose variants already declare the same required property as a const with distinct string values. Also tightens the audit walker to enforce that any discriminator.propertyName=X is backed by every non-ref variant declaring properties.X as required const — preventing a two-sources-of-truth drift where the keyword sits without the structural narrowing.

Non-breaking — the OpenAPI discriminator keyword is ignored by JSON Schema 2020-12 validators that don't recognize it. Existing const-property narrowing remains the source of truth. Codegen targets that respect the keyword (msgspec, openapi-typescript, datamodel-code-generator) now emit a properly-narrowed union without per-variant casts.

Affected schemas (16 unions, 15 hoists across 12 files)

  • adagents.json (authorization_type)
  • compliance/comply-test-controller-response.json (attestation_mode)
  • content-standards/artifact.json (type, method)
  • core/activation-key.json (type)
  • core/creative-item.json (asset_kind)
  • core/deployment.json (type)
  • core/destination.json (type)
  • core/optimization-goal.json (kind × 3)
  • core/requirements/catalog-field-binding.json (kind × 2)
  • core/signal-pricing.json (model)
  • creative/preview-creative-response.json (response_type)
  • creative/preview-render.json (output_format)

Walker tightening

scripts/audit-oneof.mjs now treats discriminator.propertyName=X as ✗ if any non-ref variant fails to declare properties.X as required const, or if two variants share the same const value. Nested-union and pure-ref variants are trusted (their target may carry the const further down). This guards the failure mode the protocol expert flagged: a future author could add discriminator.propertyName=X without the const, weakening validation for non-OpenAPI consumers.

Deferred to follow-ups

  • Boolean discriminators (get-adcp-capabilities-response.json supported, update-content-standards-response.json success) — Ajv requires unique-string values
  • Cross-file ref-only variants (core/format.json assets/itemsasset_type, core/pricing-option.jsonpricing_model) — Ajv discriminator support doesn't follow cross-file $ref. (Within-file #/definitions refs work fine and are already deployed in catalog-field-binding.json.)

Test plan

  • node scripts/audit-oneof.mjs — 36 ✓ unchanged (these were already classified ✓ via const)
  • Walker drift test: temporarily setting discriminator.propertyName="wrong_key" on core/activation-key.json correctly trips ✗ with a clear "missing const+required" message
  • Pre-existing core/assets/asset-union.json and core/offering-asset-group.json (which use nested-union ref variants) still pass — the invariant trusts nested-union targets
  • npm run test:json-schema (257/0)
  • npm run test:schemas (7/0)
  • npm run test:composed (26/0)
  • npm run test:oneof-discriminators — passes against committed baseline

🤖 Generated with Claude Code

bokelley and others added 2 commits May 2, 2026 22:19
… (adcp#3917)

Adds `discriminator: { propertyName }` to 17 oneOf unions where every
variant already declares the same required property as a const with
distinct string values. Non-breaking: validators that don't recognize
the OpenAPI keyword ignore it, but codegen targets that do (msgspec,
openapi-typescript, datamodel-code-generator) now emit a properly
narrowed union without per-variant casts.

Skipped:
- Boolean discriminators (Ajv requires unique strings) — needs a
  separate fix to convert the boolean to an enum
- Ref-only variants in core/format.json and core/pricing-option.json —
  Ajv discriminator support doesn't follow $refs; these need either
  inlining or a different keyword strategy

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…uth drift

Per protocol-expert review of #3928: when `discriminator.propertyName=X`
is declared on a oneOf, every non-ref variant must declare `properties.X`
as a required `const` with distinct values across variants. Otherwise
the discriminator is just a codegen hint that validators ignore, and a
future schema author can drift the const off without any signal.

Tightens scripts/audit-oneof.mjs to enforce this invariant via the
existing baseline-diff CI gate. Also corrects the changeset count
(15 hoists / 16 unions, not 17).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 469b6d3 into main May 3, 2026
13 checks passed
@bokelley bokelley deleted the bokelley/oneof-free-fixes branch May 3, 2026 02:32
bokelley added a commit that referenced this pull request May 3, 2026
…rmat inner-asset oneOf (adcp#3917) (#3934)

Two of the items deferred from #3928 turn out to be safe with a small
adjustment:

- core/pricing-option.json #/oneOf — discriminator.propertyName=pricing_model.
  Ajv resolves the cross-file $refs to pricing-options/*-option.json
  cleanly when all schemas are pre-registered via addSchema(); the
  earlier deferral was based on a faulty isolated-compile test.

- core/format.json inner oneOf at
  #/properties/assets/items/oneOf/14/properties/assets/items —
  discriminator.propertyName=asset_type. Ajv's discriminator support
  does not follow allOf to find the inherited required, so each of the
  12 inner variants gains a direct required:["asset_type"]. The const
  was already declared inline.

Outer 15-variant oneOf and the two boolean-discriminator unions remain
deferred — they need shape changes, not a free hoist.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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