feat(registry): type the change-feed event payload as a discriminated union#5818
Conversation
… union The `GET /api/registry/feed` (and `/feed/stream`) events previously exposed `event_type: string` and `payload: object` (opaque), forcing every consumer to hand-cast `payload` to reach `publisher_domain` / `agent_url`. Type them at the Zod source (`RegistryFeedPageSchema`): `event_type` is now an enum of the known types across the agent, brand, property, collection, publisher, and authorization families, and each event is a discriminated-union arm tying its `event_type` literal to a named family payload (`AgentEventPayload`, `BrandEventPayload`, `PropertyEventPayload`, `CollectionEventPayload`, `AuthorizationEventPayload`, `PublisherEventPayload`). Payload fields that appear on only some members of a family (e.g. `*.merged` carries `alias_rid`/`canonical_rid` in place of `identifiers`) are optional. Enum coverage is grounded in the actual contract: the crawler's emit sites, the server's own registry-sync reference consumer, and the SDK's RegistrySync switch (which also handles the brand.* family and agent profile/compliance events). The generated `oneOf` uses per-arm enum discriminators (matching the repo's existing union style); openapi-typescript renders it as a proper discriminated union so consumers narrow `payload` by switching on `event_type`. Verified: server typecheck, OpenAPI freshness, oneof-discriminator audit, and a full @adcp/sdk regen (types generate, no loose-oneof arms, lib typecheck clean). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Right shape, incomplete corpus. The discriminated-union mechanism is correct — the block is that the spec now makes a closed assertion about event_type that is narrower than what the server puts on the wire.
MUST FIX (blocking)
The oneOf omits three event types that are actively emitted onto this exact feed. server/src/routes/registry-api.ts:132-155 enumerates 20 arms; the backing catalog_events table receives at least 23 distinct event_type values, and queryFeed (server/src/db/catalog-events-db.ts:115) applies no allowlist — only the optional types glob and cursor/limit. Missing:
agent.verification_earned—server/src/notifications/compliance.ts:300(eventsDb.writeEvent)agent.verification_lost—server/src/notifications/compliance.ts:314authorization.modified— DB trigger inserver/src/db/migrations/446_authorization_feed_emitter.sql:231
What breaks for adopters: the PR's stated goal is "consumers narrow payload by switching on event_type." A consumer that codegens the discriminated union from this spec and does an exhaustive switch will hit a live agent.verification_earned / authorization.modified event with no matching arm — falling through, or rejected outright by a strict oneOf validator. Before this PR (event_type: string, open payload) every event validated; this PR ships a spec that is precisely wrong for three real events on the wire. That is spec-vs-wire drift on a wire-facing surface.
Not a runtime risk on the server side — RegistryFeedPageSchema is doc/codegen-only, not runtime response validation (res.json direct), confirmed by both code-reviewer and ad-tech-protocol-expert. The break is downstream, at the adopters this PR is built to serve.
Fix: add three arms (agent.verification_earned, agent.verification_lost → AgentEventPayload; authorization.modified → AuthorizationEventPayload), and extend AgentEventPayloadSchema (registry-api.ts:18-37) with the fields those verification events carry — role, reason, verified_specialisms, adcp_version (compliance.ts:303-308,316-322) — then regenerate. Contained fix; no structural change.
Things I checked
- Zod union is well-formed:
z.discriminatedUnion("event_type", […])with per-armz.literalvia thefeedEventArmhelper is the correct, supported pattern. Clean factoring of the shared envelope. - Nullability parity holds:
.nullable().optional()→type: [string, "null"]and omitted fromrequired(e.g.collection_id,manager_domain). Matches source. additionalPropertiesposture: all six new payload schemas stay open (noadditionalProperties: false), so unknown payload keys still validate — the enrichment itself is backward-compatible. No tightening against [DR-0009].- Over-coverage (five
brand.*arms,property.updated,collection.updatedwith no in-repo emit site) is harmless for a union — forward-declaring is fine. Only missing-but-emitted types matter. - No changeset is correct — do not add one.
scripts/check-changeset-protocol-scope.cjs:14-19lists^static/openapi/registry[.]yaml$inREGISTRY_RELEASE_SCOPED_PATHS, held separate from protocol-scoped paths. Registry OpenAPI is continuously deployed, not part of theadcontextprotocolsemver release. The PR body's reasoning matches repo convention. - oneOf gate (
scripts/audit-oneof.mjs) walksstatic/schemas/source/only (line 31) —static/openapi/registry.yamlis outside its scope, so MUST FIX #7 does not fire here. Not a violation by this PR.
Follow-ups (non-blocking — file as issues)
- Drift guard. Nothing runtime-checks emitted
event_typevalues against the union, which is how this gap reached the PR in the first place. A test asserting every distinctevent_typestring inserver/srcemit sites appears in the union would fail CI on the next omission instead of shipping it. (code-reviewer) - Discriminator coverage. Consider extending
audit-oneof.mjstoregistry.yaml, or adding a formaldiscriminator: propertyName— tracked at #3917. (ad-tech-protocol-expert) entity_typestays an openstringper arm. Confirm that's intentional — it's the only cross-cutting field left untyped.
The commit message says the enum is "grounded in the actual contract, not just the docs table." Three emitters in the same source tree suggest the grounding stopped one directory short of notifications/ and migrations/.
Fix the three arms and this is a clean enrichment. Re-request review after regen.
There was a problem hiding this comment.
Clean enrichment. Turns an opaque payload: object into a oneOf discriminated on event_type — the right shape, and the additive path (payloads stay open) done correctly.
Things I checked
- Schema is generation-only, not a runtime gate.
RegistryFeedPageSchemais referenced solely atserver/src/routes/registry-api.ts:2006insideregistry.registerPath(...). No.parse()/.safeParse()on outbound feed responses anywhere. A missing arm would under-type the spec, not 500 the endpoint — so this is not MUST-FIX #1. - Enum covers every emitted type. Walked the emit sites:
crawler.ts(agent.discovered/removed, authorization.granted/revoked, publisher.adagents_changed/discovered),notifications/compliance.ts:300,314(agent.verification_earned/lost — both present as arms, contra one bot flag),collection-catalog-db.ts(collection.created/updated/removed/merged),catalog-api.tsprojection (property.*). All land in the 20-arm enum. Narrowing openstring→ closed enum is safe because the enum is a superset of today's wire. - Payloads stay open. Every family schema is
.passthrough().openapi(...); generated yaml carriesadditionalProperties: {}. Happy path for consumers reading known fields is unchanged. - oneOf discriminator is sound. Per-arm single-value
event_typeenum literals, mutually exclusive, no formaldiscriminator:keyword — matches repo union style;test:oneof-discriminatorspasses because the arms are separable by tag. No audit regression. - Changeset correctly omitted.
scripts/check-changeset-protocol-scope.cjsplacesstatic/openapi/registry.yamlinREGISTRY_RELEASE_SCOPED_PATHS, explicitly disjoint fromPROTOCOL_SCOPED_PATHS(static/schemas/source/**). Registry-release-scoped, continuously deployed — the author's rationale is the codebase's formal policy, not just plausible. Not MUST-FIX #6.
Follow-ups (non-blocking — file as issues)
- Closed discriminant vs. open emit path. The write boundary is a free-form
event_type: string(server/src/db/catalog-events-db.ts:24) and the feed filter is glob/LIKE, so producers can emit a new family faster than the spec tracks it. Under the closed enum, a strict consumer would reject an unknownevent_type(payloads staying open doesn't rescue a closed discriminant). Today spec and emitter deploy in lockstep so blast radius is small — but consider a permissive catch-all arm (event_type: string+ openpayload) as the finaloneOfmember, or document tolerate-and-skip for unknown types. OpenRTB/VAST reserve an "other/unknown" code for exactly this. - Orphaned
BrandEventPayload. Defined in components with nobrand.*arm in the union. No server-sidebrand.*emit exists inserver/src; the PR body attributes it to the SDKRegistrySyncswitch. Either wire the arms if the feed genuinely carriesbrand.*, or drop the unreferenced schema.
Minor nits (non-blocking)
- "Grounded in the actual contract" is doing some load-bearing work.
agent.profile_updatedand the brand payload are consumer/SDK-driven, not server-emitted — a notable framing for a claim about the emit surface. Harmless over-provisioning; worth a word in the PR body.
Approving on the strength of full enum coverage of the emitted set plus the confirmed registry-release scope.
Why
The registry change feed (
GET /api/registry/feed,GET /api/registry/feed/stream) exposedevent_type: stringandpayload: object(opaqueadditionalProperties: {}). Every consumer — including@adcp/sdk's ownRegistrySync— has to hand-castpayloadto reach routing keys likepublisher_domain/agent_url, and codegen has nothing to work with.What Changed
Typed the feed event at the Zod source (
RegistryFeedPageSchemainserver/src/routes/registry-api.ts);static/openapi/registry.yamlis regenerated from it viabuild:openapi.event_type→ enum of the known types across six families:agent.*,brand.*,property.*,collection.*,publisher.*,authorization.*.payload→ discriminated union keyed onevent_type. Each event is an arm tying itsevent_typeliteral to a named family payload:AgentEventPayload,BrandEventPayload,PropertyEventPayload,CollectionEventPayload,AuthorizationEventPayload,PublisherEventPayload.*.mergedcarriesalias_rid/canonical_ridin place ofidentifiers;authorization.revokedcarries onlyagent_url+publisher_domain) are optional on the family schema.PropertyEventPayload.identifiersreuses the existingPropertyIdentifiercomponent.Enum coverage is grounded in the actual contract, not just the docs table: the crawler emit sites (
server/src/crawler.ts), the server's own reference consumer (server/src/registry-sync/index.ts), and the SDK'sRegistrySyncswitch (which also handlesbrand.*and agent profile/compliance events). The generatedoneOfuses per-arm enum discriminators, matching the repo's existing union style (no formaldiscriminator:keyword);openapi-typescriptrenders it as a proper discriminated union, so consumers narrowpayloadby switching onevent_type.No changeset: this is a registry-release-scoped change (
static/openapi/registry.yaml), continuously deployed, not part of theadcontextprotocolprotocol release.Verification
npm run typecheck(server) — cleannpm run test:openapi(OpenAPI freshness) — regenerated, in syncnpm run test:oneof-discriminators— no new undiscriminated oneOf@adcp/sdkregen against this spec: types generate,check-no-loose-oneofpasses,tscon the lib project is clean, andCatalogEventbecomes a discriminated union (event_type: "…"→ typedpayload).Backward-compatible enrichment: event values are unchanged; consumers reading known fields are unaffected, and
payloadnarrows instead of being opaque.🤖 Generated with Claude Code