codegen: oneOf response branches lose the root allOf ProtocolEnvelope (19 of 24 SuccessResponse aliases)

[KonstantinMirin]

Every AdCP response schema composes core/protocol-envelope.json at its root via allOf,
so status, task_id, message, context_id, replayed, timestamp,
push_notification_config, governance_context, context, payload and adcp_error are
part of every response's contract. In adcp==6.6.0, 19 of the 24 *SuccessResponse aliases
do not inherit ProtocolEnvelope, so those eleven fields are untyped on them: a server that
sets one gets a pydantic extra, and a client that reads one gets an AttributeError.

Related to #822, which is a different bug in the same file (a stripped import producing
NameError). This one is about which classes get the base at all.

Measured against the shipped wheel

from adcp.types import ProtocolEnvelope
import adcp.types.aliases as A

for n in sorted(n for n in dir(A) if n.endswith("SuccessResponse") and not n.startswith("_")):
    c = getattr(A, n)
    if isinstance(c, type):
        print(f"{'inherits' if issubclass(c, ProtocolEnvelope) else 'MISSING '}  {n}")

MISSING   ActivateSignalSuccessResponse
MISSING   BuildCreativeSuccessResponse
MISSING   CalibrateContentSuccessResponse
inherits  CreateContentStandardsSuccessResponse
MISSING   CreateMediaBuySuccessResponse
MISSING   GetAccountFinancialsSuccessResponse
MISSING   GetBrandIdentitySuccessResponse
MISSING   GetContentStandardsSuccessResponse
MISSING   GetCreativeFeaturesSuccessResponse
MISSING   GetMediaBuyArtifactsSuccessResponse
inherits  GetProductsSuccessResponse
MISSING   GetRightsSuccessResponse
inherits  GetSignalsSuccessResponse
inherits  ListContentStandardsSuccessResponse
MISSING   LogEventSuccessResponse
MISSING   ProvidePerformanceFeedbackSuccessResponse
MISSING   SyncAccountsSuccessResponse
MISSING   SyncAudiencesSuccessResponse
MISSING   SyncCatalogsSuccessResponse
MISSING   SyncCreativesSuccessResponse
MISSING   SyncEventSourcesSuccessResponse
inherits  UpdateContentStandardsSuccessResponse
MISSING   UpdateMediaBuySuccessResponse
MISSING   ValidateContentDeliverySuccessResponse

19 missing, 5 present.

The two structurally identical inputs that generate differently

content-standards/update-content-standards-response.json and
media-buy/create-media-buy-response.json have the same shape where it matters: the root
composes ../core/version-envelope.json and ../core/protocol-envelope.json through
allOf, and neither one's Success branch references the protocol envelope itself.

update-content-standards-response.json
  root allOf: ['../core/version-envelope.json', '../core/protocol-envelope.json']
  branch UpdateContentStandardsSuccess  allOf: []      -> generated class INHERITS ProtocolEnvelope

create-media-buy-response.json
  root allOf: ['../core/version-envelope.json', '../core/protocol-envelope.json']
  branch CreateMediaBuySuccess          allOf: [...]   -> generated class does NOT

The branch-level difference runs the wrong way: CreateMediaBuySuccess carries more
composition than UpdateContentStandardsSuccess and gets less inheritance. So the
divergence is in codegen, not in the schemas.

The general rule the data fits: where a response schema has no oneOf, the alias resolves
to the root class and picks the base up from the root allOf; where it has oneOf, the
per-branch classes are generated without the root composition. UpdateContentStandards is
the one case that escapes it, which is what makes this look like a bug rather than a
deliberate exclusion.

Impact

Any field of the protocol envelope is untyped on those 19. Concretely, on the seller side:

• status — the pinned envelope marks it required on every task response, so a seller that
  cannot set it typed has to declare the field itself.
• replayed — AdCP L1/security.mdx idempotency rule 4 puts this on the response envelope
  of a replayed request. A boundary that sets response.replayed = True writes an extra on
  19 of 24 response types and a real field on 5, which is not a difference a caller can see.

Workaround in use

Naming ProtocolEnvelope as a second base on the local subclass:

class SyncCreativesResponse(LibrarySyncCreativesSuccess, ProtocolEnvelope):
    ...

That is additive today and becomes a no-op when the generated class carries the base, so it
deletes cleanly. Filing so it can be deleted rather than kept.

Suggested fix

Attach ProtocolEnvelope to every generated oneOf branch class of a response whose root
composes core/protocol-envelope.json, not only to the branch the driver currently treats
specially. The check is on the ROOT schema's allOf, so it needs no per-branch table.

[bokelley]

Triage

Classification: Bug
Bucket(s): codegen (no matching repo label; gap noted)
Status: ready-to-implement
Milestone: omitted

What the experts said:

• code-reviewer: approved — bug confirmed at emit_response_class() line 4136 in scripts/post_generate_fixes.py; MRO risk is low; existing branch-level status: Literal[…] overrides survive the new base; requires issubclass(XSuccessResponse, ProtocolEnvelope) test as merge gate
• ad-tech-protocol-expert: approved as protocol direction — missing ProtocolEnvelope base is a genuine conformance violation (idempotency rule 4 on replayed; status untyped on 19 of 24 shapes); scope extends to ErrorResponse arms too (≈44 branch classes total, not just 24 SuccessResponse aliases); fix is additive and non-breaking

My take: The root cause is a single predicate in scripts/post_generate_fixes.py:4133–4138 — ProtocolEnvelope is added only when status.const == "submitted" AND task_id is present; all other arms fall through to AdcpVersionEnvelope alone. Fixing that predicate is non-breaking and makes the Python type surface match the wire contract.

Implementation scope:

• Primary fix: In render() (which has access to self.root_schema), detect whether the root schema's allOf contains a $ref to core/protocol-envelope.json using the existing _resolve_schema_ref helper. Pass a root_composes_envelope: bool flag into emit_response_class(). Update bases to always include ProtocolEnvelope when that flag is true (the current is_submitted path becomes a subset). Set self.needs_protocol_envelope = True on the flag path, not only the is_submitted path.

• Scope: All 22 response files in response_specs whose root allOf includes protocol-envelope.json — affects *Response1 (success) and *Response2 (error) arms, ≈44 branch classes. ErrorResponse arms already declare status: Literal['rejected'] in their body, so adding the base does not produce a wrong default.

• Required tests:

  1. Unit test in tests/test_code_generation.py: feed rewrite_response_arms_from_schema a mock schema with allOf: [{$ref: …/core/protocol-envelope.json}] plus a oneOf; assert every emitted branch class has ProtocolEnvelope in its bases.
  2. Import-time regression guard: iterate every *SuccessResponse name from adcp.types and assert issubclass(cls, ProtocolEnvelope).

• Post-fix: Run python scripts/generate_types.py from clean state; verify all 24 *SuccessResponse aliases pass the issubclass check.

• Non-breaking rationale: Additive base class addition; isinstance(response, ProtocolEnvelope) flips from False to True for 19 aliases, which is the correct behavior. The existing workaround (class SyncCreativesResponse(LibrarySyncCreativesSuccess, ProtocolEnvelope)) becomes a no-op and can be deleted after.

• Related: chore(codegen): post_generate_fixes.py drops ProtocolEnvelope import + aliases.py has stale defensive lookups #822 (closed) fixed the stripped import and stale lookups; this issue is about which classes receive the base in the first place.

---

Triaged by Claude Code. Session: https://claude.ai/code/session_0154kAxY2mafwL6zkkpdMZaj

---

Generated by Claude Code