Repository navigation
Description
Since I filed this on 14 Oct, reviews of #545, #554 and #581 found more bug classes that passed CI and an approving review. This issue is now one CI plan. Each check below names the PR whose bug it would have caught. Everything was re-run on main at 2892295.
Direction. AGENTS.md ("Adding a new AdCP type", step 1) already says to let types_gen.go own a type whenever the generator produces an acceptable shape. KNOWN_TYPES still lists 152 names, and 157 structs live outside types_gen.go (110 of them in types.go, 11 in inputs.go), against 426 generated. They exist for the reasons the KNOWN_TYPES comment gives (flattened oneOf unions, inline items without their own schema file, shapes the generator can't produce, attached methods), plus unions behind allOf/$ref (#563), request-side null (#536) and public-API stability (#545's RejectionReason). These checks are the safety net while those gaps close. Each closed gap lets a set of hand-written types retire, and the pairing check (4) shows which ones are ready.
| # | Check | Bug class | PR that shows it | Status |
|---|---|---|---|---|
| 0 | lint.py --strict: required but undeclared |
A required property missing from properties is dropped from the generated struct |
#555 (ValidateInputWarning had no Field) |
Done in #565. adcontextprotocol/adcp#8212 (open) adds the same lint to the schema repo |
| 1 | Validate Go test fixtures against their schema | A schema-invalid fixture passes because the Go type accepts it | #545's first head ("status":"active"), #581 (localization object) |
Proposed |
| 2 | Lint: request properties with a null branch keep null | null decodes like omission, so a "null removes it" request is lost |
#581 (localization); #536 is the general case |
Proposed. All 10 such fields outside untyped parents lose null today |
| 3 | Generator guard: object $ref bound to a scalar alias |
[]CanonicalMediaBuyAction (= string) for an array of objects |
#563's first head (available_actions) |
Proposed. The stale KNOWN_TYPES entry is still at generate.py:61 |
| 4 | Pair hand-written types via HAND_WRITTEN_SCHEMA_SPECS, shrink EXEMPT |
A hand-written type drops schema fields that lint never compares | #545 (CreativeResult paired), #581 (CreativeInput) |
Shown on #545. EXEMPT has 99 entries |
| 5 | Round-trip every schema example through its Go type | Fields silently dropped on decode | 20 of 95 examples fail on main |
Proposed, with a known-failures list |
| 6 | Build reference/ agents against the working tree too |
Reference code compiles only against the released adcp/v3 v3.0.0 |
#554 | Proposed. Neither compiles against main today |
| 7 | Dispatch every request example through Register |
A panic in tool dispatch ends the process | #554's first head | Proposed. Passes on main |
| 8 | Optional: compare tools/list input schemas with the bundle |
The schema AddTool infers from a Go type differs from the request schema |
#581 review (json.RawMessage) |
No mismatch found today |
1. Fixture validation
adcp/v3/go.mod already requires github.com/google/jsonschema-go v0.4.2, which validates draft-07, and CI downloads the bundle before go test (idempotent_test.go already reads schemas/). So the helper is a small test function. One catch is that Go's RE2 rejects 8 distinct regexes in the bundle, which use lookaround or \u escapes (23 of 844 pattern occurrences, in 11 files). Resolve fails on any schema that reaches one. sync-creatives-request.json reaches one through core/macro-bearing-url.json. With those patterns skipped, the helper flags #545's first-head fixture (active is not in enums/creative-status.json) and #581's fixture, and it passes all three sync-creatives-request.json examples.
2. Null lint
Of the 37 request schemas in TOOL_SCHEMAS, 6 have properties with a null branch, in 137 property paths. 127 of them sit under a parent typed any or map[string]any (targeting_overlay, comply_test_controller's params, check_governance's proposal) and keep null. The other 10 all lose it. Decoding null gives the same Go value as omitting the field, and re-encoding drops the key:
UpdateMediaBuyRequest.DailyBudgetCap(*float64,omitempty). Its doc comment says "Replace the hard aggregate daily cap; null removes it."UpdateMediaBuyRequest.BudgetCapTimezone(*string): "null restores the default".UpdateMediaBuyRequest.FrequencyCapand.Bidding(any), andPackageUpdate'sbudget,min_spend_target,daily_budget_capandbidding.Plan.ApprovedSellers([]string,omitempty).creatives[].localization, whichCreativeInputlacks onmain. feat(adcp/v3): add revision_id and localization to CreativeInput #581 adds it asanywithomitempty.
The lint would fail on such a field unless it is in a reviewed set. The encoding that fixes them is #536's decision.
3. $ref guard
KNOWN_TYPES still lists CanonicalMediaBuyAction (generate.py:61), and nothing hand-written defines it. The only Go type with that name is the generated string alias from enums/canonical-media-buy-action.json (types_gen.go:1361). No generated field uses it for core/canonical-media-buy-action.json today, because media-buy-commitment-response.json isn't generated yet. It will be once accept_proposal and buy_products are. The guard fails when an object $ref resolves to a string or number alias. The stale entry goes.
4. Pairing and EXEMPT
HAND_WRITTEN_SCHEMA_SPECS pairs 60 types. #545 shows the step: it pairs CreativeResult with sync-creatives-response.json#/oneOf/0/properties/creatives/items, removes it from EXEMPT, and allows the deprecated rejection_reason through a new ALLOWED_EXTRA_FIELDS. With #581 merged, pairing CreativeInput with sync-creatives-request.json#/properties/creatives/items makes lint.py --strict list 13 fields missing in Go: component_assets, format_kind, format_option_ref, industry_identifiers, inputs, placement_ids, placement_refs, provenance, representation_selection, rights, status, tags, weight. It also lists assets and name as required but omitempty. On main it lists the same plus localization and revision_id.
The 99 EXEMPT entries on main (lint.py:57)
- oneOf flatteners:
BrandReference,AccountReference,ActivationKey,SignalID,PublisherPropertySelector,OptimizationGoal - errors and helpers:
Error,ErrorOptions - test controller:
TestControllerStore,StateTransition,SimulateDeliveryParams,ReportedSpend,SimulateBudgetParams,SimulationResult,TestControllerError ValidateInputWarning- inputs:
EmptyInput,AccountInput,GovernanceAccountInput,CreativeInput,CatalogInput,EventSourceInput,DestinationInput,CreativeFilters,SignalFilters - response builders:
SyncCreativesResponse,SyncAccountsResponse,GovernanceResponse,ListCreativesResponse,PreviewCreativeResponse,BuildCreativeResponse,CreativeFormatsResponse,SignalsResponse,ActivateSignalResponse,SyncCatalogsResponse,SyncEventSourcesResponse,LogEventResponse,PerformanceFeedbackResponse - plan types:
Plan,PlanBudget,PlanBudgetAllocation,PlanFlight,PlanChannels,PlanChannelMixTarget,PlanDelegation,PlanDelegationBudget,PlanPortfolio,PlanPortfolioBudgetCap,HumanOverride,DataSubjectContestation - inline response items:
AccountResult,AccountSetup,CreativeResult,CatalogResult,EventSourceResult,LogEventResult,GovernanceResult,GovernanceAccount,GovernanceAgent,CreativeListItem,MediaBuyListItem,MediaBuyData,MediaBuyHistoryEntry,PackageStatus,PackageCreativeApproval,PackageSnapshot,SyncCreativeAssignment,DeliveryTotals,DeliveryData,ReportingPeriod,PreviewResult,Preview,PreviewRender,BuildCreativeResult,ProductsData,CapabilitiesData,CheckGovernanceCondition - collections:
CreateCollectionListResponse,GetCollectionListResponse,UpdateCollectionListResponse,DeleteCollectionListResponse,ListCollectionListsResponse,CollectionList,CollectionListFilters,BaseCollectionSource,DistributionID,ResolvedCollection,CollectionPagination - nested shapes:
BillingMeasurement,MakegoodPolicy,CancellationFee,Render,AssetSlot - reporting:
ReportingConsumerStatus,ReportingConsumerStatusPeriod,ReportingStatusResult,ReportingReceipt,ReportingAdjustmentReceipt,ReportingReceiptResult
5. Example round trip
The test decodes every examples[] entry in the 3.2.2 bundle into its Go type, encodes it back and compares the JSON. On main at 2892295 it covers 95 examples across 26 schemas. None fails to decode, and 20 come back with fields missing:
| Schema example | Lost on round trip | Go type |
|---|---|---|
account/sync-accounts-request.json #0, #3–#6 |
brand.countries, currency, operator_unit, billing_entity, notification_configs, account |
AccountInput (inputs.go) |
account/list-accounts-response.json #0, core/brand-ref.json #3 |
brand.countries |
BrandReference (types.go) |
core/account-ref.json #3 |
brand.countries, currency, operator_unit, timezone |
AccountReference (types.go) |
account/sync-governance-request.json #0, #1 |
account.account_id, governance_agents[].authentication |
GovernanceAccountInput (inputs.go) |
creative/list-creatives-request.json #0–#2, #4, #5 |
filters.statuses, concept_ids, format_kinds, has_variables |
CreativeFilters (inputs.go) |
creative/sync-creatives-request.json #0–#2 |
format_kind, tags |
CreativeInput (inputs.go) |
media-buy/sync-catalogs-request.json #0, #1 |
name, type, url, feed_format, update_frequency |
CatalogInput (inputs.go) |
Merging #545, #554 or #581 onto main changes no row. Today's 20 go into a known-failures list that can only shrink.
6. Reference builds
reference/seller-agent and reference/hello-seller both require adcp/v3 v3.0.0 with no replace, and CI tests each standalone (ci.yml:134–140). scripts/check-goworkexample.sh keeps hello-seller out of go.work.example on purpose ("the released API is the point"). That pin can stay. With a go.work over main, neither compiles:
seller-agent:undefined: adcp.ProductAllowedAction(cmd/seller-agent/main.go:822);hello-seller:cannot use p.Budget (variable of type *float64) as float64 value(main.go:143).
#554 changes the account-reference contract and both reference resolvers, but nothing runs the new code. v3.0.0's resolveAccount returns early when Brand == nil && Operator == "", so an account_id-only reference never reaches the reference resolver, and no reference test sends one. A second job that builds and tests them against the tree would cover this.
7. Dispatch smoke
The smoke test calls Register with every hook stubbed and the resolver from its doc comment (ref.Brand.Domain). It sends each request example through an in-memory MCP client and fails on a panic. Only 12 bundle request examples belong to Register's tools, so it also puts each core/account-ref.json example into the account of the account-bearing tools, for 37 calls in all. On main none panics. On #554's first head (e111bd2) merged onto main, get_products with {"account_id": "acc_acme_001"}, the schema's first example, ends the test binary with a nil-pointer SIGSEGV, because go-sdk's callTool doesn't recover (as #544 also shows).
8. tools/list comparison (optional)
AddTool infers each tool's input schema from the Go type (addtool.go:31 → permissiveSchemaFor → jsonschema.ForType at addtool.go:72). A json.RawMessage field therefore advertises as {"type":["null","array"],"items":{"type":"integer","minimum":0,"maximum":255}}. Comparing the top-level properties of Register's 19 tools with the bundle finds no type mismatch and no byte array today. The only differences are 42 properties typed any, which addtool.go:116 drops from the advertised schema on purpose.
Ladon
Ladon approved every head named above: #555, #563's first head, #545's first head, #554's first head and #581. Its findings could take the deterministic results above as input. PR descriptions are also worth checking against the diff. Two of mine called a one-way test a round trip. #555's test encoded the request and decoded the response, and the decoded warning's field was dropped without an assertion noticing. #563's first head did the same. #535 (draft) would make Ladon post findings without approving, and that is your call.
Order and offer
I'm happy to open these as separate PRs, cheapest first:
- fixture validation;
- the null lint;
- the
$refguard; - pairing plus the
EXEMPTburn-down; - the example round trip with its known-failures list;
- the reference builds;
- the dispatch smoke.
One question: where should the known-failures lists live, in generate.py beside the other review sets, or in the tests that use them?
Activity
[-]CI: round-trip schema examples through the generated types, and fail on two silent generator mismatches[/-][+]CI: catch silent schema/Go type drift (round trips, null, fixtures, references)[/+]sujanchalla0510 commentedon Oct 20, 2026
@torbenbrodt asked on #581 whether these checks would have helped on the PRs I've been working in (#545, #554, #581). Short answer: yes — checks 1, 2 and 4 each map to a mistake I personally shipped past green CI and an approving review.
"status": "active"(not aCreativeStatus) and feat(adcp/v3): add revision_id and localization to CreativeInput #581's fixture carried the readback shape forlocalization. Both passed CI because the Go types accept anything. One request: make the RE2 pattern skip-list explicit and narrow —sync-creatives-request.jsonreaches a lookaround pattern viacore/macro-bearing-url.json, and a helper that fails closed there would be disabled for exactly the schema that caught me.CreativeInput.Localizationat authoring time. Worth stating the scope explicitly: the 10 losing fields you count are outside untyped parents, andCreativeInputis a hand-writteninputs.gotype in the EXEMPT set — the lint only catches my case if it also walks hand-writtenanyfields, not just generated ones. If it can't yet, an issue-linked known-failures entry is better than silence.CreativeResulton fix(adcp,adcp/v3): align CreativeResult with sync-creatives-response schema #545 immediately listed the missing spec fields; pairingCreativeInputwould have listed theformat_kind/tags/representation_selectiondrops before review instead of in it.ALLOWED_EXTRA_FIELDSon Follow-up: CreativeResult.Errors []string vs []AdcpError drift (breaking re-type deferred from #545) #546: an unowned allowlist entry becomes permanent.json.RawMessagetrap you found on feat(adcp/v3): add revision_id and localization to CreativeInput #581 (tools/list advertising a byte array), I'd promote this from optional for any PR that changes anany-typed request field's representation — that's precisely when the inferred schema drifts.Nothing I'd remove. The ordering I'd merge in: 4 and 1 first (they catch field drops and bad fixtures, the two most repeated classes), then 2.
ci(adcp/v3): pair hand-written request inputs with their schemas in l…
test(adcp/v3): validate wire fixtures against their JSON schema (#587)
feat(adcp/v3): generate accept_proposal and buy_products with unions …
Merge main into fix-547-account-id-only-ref; delete stale AccountInpu…
feat(adcp/v3): generate control_media_buy request and response types (#…