[torbenbrodt]

Slice B of the three-state request-field design on #536 (slice A, Optional[T], merged as #595). #536 closed when 3.3.0-beta.0 merged; that was a side effect of #595's commit footer, not a decision, and B and C are still open.

Opens as a draft: it changes the Go type of released fields (escalation category 1 in AGENTS.md), so it waits for a maintainer's call rather than a review window. The question is under "Breaking" below.

What changes

The generator now emits Optional[T] tagged omitzero for every non-required request property whose schema has a null branch (type with "null", an anyOf/oneOf null branch, or a null enum/const), in structs reachable from a tool request schema and not shared with any response or webhook schema. Sixteen fields change:

Struct	Fields
UpdateMediaBuyRequest	daily_budget_cap, budget_cap_timezone, frequency_cap, bidding
PackageUpdate	budget, min_spend_target, daily_budget_cap, bidding
ControlMediaBuyRequest	daily_budget_cap, budget_cap_timezone, frequency_cap, bidding
PackageControl	budget, min_spend_target, daily_budget_cap, bidding

Each of these schemas accepts null, and for 12 of the 16 it says what null does: remove a cap or target, restore the default timezone (budget_cap_timezone), or clear a bidding block so the package inherits (bidding on update_media_buy). For PackageControl budget, min_spend_target and bidding, and ControlMediaBuyRequest.bidding, AdCP 3.2.2 accepts null without saying what it means; their generated comments say so, and MIGRATING.md gives the meaning per field. Today a pointer or any field decodes null like omission, so a seller can't tell the two apart, and re-encoding drops the key. bidding and frequency_cap become Optional[any] and keep their reviewed-any reasons in the coverage gate.

• tools/list: update_media_buy and create_media_buy advertise byte-identical schemas (a test pins update_media_buy against a reflected twin of the old types). control_media_buy now advertises five properties it didn't before, because any fields aren't advertised: daily_budget_cap, budget_cap_timezone and the package budget, daily_budget_cap, min_spend_target, typed ["null","number"] or ["null","string"] as in the schema; a test pins those five entries and the rest of the schema. Input for those five is now type-checked: "daily_budget_cap":"250" is rejected as invalid input, where the handler used to receive the string.
• Lint (check 2 of CI: catch silent schema/Go type drift (round trips, null, fixtures, references) #564): lint.py reports a non-required nullable request property that isn't Optional in a paired hand-written request-only struct (for generated structs the regeneration diff in CI is the guard), and an untyped field hiding nullable properties unless it is on a reviewed list. The list is exactly what lint finds today: test-controller params, CanonicalProposal.forecast, and targeting_overlay on PackageInput, PackageUpdate, PackageControl and ProductPurchaseInput (slice C types those). A stale entry fails lint.
• Not covered yet, by design: Plan.approved_sellers (hand-written; fix(adcp/v3): keep an empty approved_sellers list on the wire #594 changes it first) and CreativeInput localization (no Go field until feat(adcp/v3): add revision_id and localization to CreativeInput #581).
• Tests: omitted, null, a value and a zero value round-trip for all 16 fields; update_media_buy and control_media_buy calls over MCP, through AddTool and WithIdempotency, reach the handler as IsNull() at media-buy and package level, IsZero() when omitted and Some(0) for a zero; every generated Optional field is tagged omitzero and never omitempty; generator and lint tests for the rule, the reachability scan (including webhooks) and the reviewed list.
• Docs: MIGRATING.md gets a section with all 16 fields at 3.2.1, 3.3.0-beta.0 and now, and before/after code; docs/sdk-typing-policy.md describes the rule.

Breaking

Source-incompatible for callers of these fields: 7 were released in 3.2.1 (*float64, *string or any), and 9 first shipped as any in 3.3.0-beta.0. adcp.Float64(5) becomes adcp.Some(5.0), and null is sent with adcp.Null[float64]() (the pointer fields couldn't send it; the beta.0 any fields used ClearField()). A handler asks three questions: present at all (!x.Budget.IsZero(), which replaces x.Budget != nil), which value (x.Budget.Get()), and null (x.Budget.IsNull()). Get() is false for null, so an authorization or audit check moved to Get() would let a removal through. Register these types with adcp.AddTool (go-sdk's mcp.AddTool infers object for an Optional and rejects every value), and encode with encoding/json (json-iterator ignores omitzero and fails with ErrOptionalUnset's message). ErrOptionalNilValue's text changed; match it with errors.Is. MIGRATING.md has all of it.

The wire changes in two places: an explicit null survives decode and re-encode, and control_media_buy rejects mistyped values for its five typed controls. The fields and their null branches are already in the pinned 3.2.2 schemas. For the previous minor, AdCP 3.1.1's package-update.json budget is number-only, so a null package budget sent to a 3.1 seller is refused by that seller's own validation; the SDK has no version-translation layer to do more yet. There's no feat! marker because RELEASING.md forbids breaking-change commits under adcp/v3/.

@bokelley, should this land on main on its own, or would you rather fold it into #602? #602 regenerates types_gen.go anyway, so whichever lands second only reruns the generator; say the word and I'll retarget it.

reference/seller-agent (pinned to v3.0.0) uses upd.Budget != nil in attemptedAction and when applying updates. When it moves to the in-repo module (check 6 of #564), it must test presence with !IsZero() and treat a null package budget (ceiling removed) as an increase or refuse it, not as no action.

Checks run locally

generate.py reproduces types_gen.go; lint.py --strict; --coverage-max-unreviewed-any 0; Python generator and lint tests; go test -race ./... in adcp/v3 on Go 1.26 and 1.27; go vet; golangci-lint.

@garvitkaushik-123 @sujanchalla0510, a review is welcome while the ruling is open.

🤖 Generated with Claude Code

[garvitkaushik-123]

@torbenbrodt Thanks for the slice. I checked the 16 fields against the pinned 3.2.2 request schemas with my own script and they match. I read the code and did not run the generator, lint or tests.

1. control_media_buy tools/list change is untested. The schema pin and the MCP round-trip test cover only update_media_buy. control_media_buy is the tool whose advertised schema changes (five new typed properties). Please add the same two tests for ControlMediaBuyRequest.

2. "Each schema says null removes the value" is too strong. In 3.2.2, PackageControl.budget and min_spend_target don't say what null means. The generator comment this PR deletes said so. budget_cap_timezone: null restores the default, and bidding: null clears and the package inherits. Please soften the wording in the description and MIGRATING.md, and keep the PackageControl gap noted in a doc comment or an upstream adcp issue.

3. MIGRATING.md: Get() skips null. The note says x.Budget != nil becomes x.Budget.Get(). Get() is false for null, so authorization code written that way ignores a cap removal. Please add: for "field is present", use !x.Budget.IsZero(). This matters when reference/seller-agent moves over, since attemptedAction checks only Budget == nil.

[torbenbrodt]

@garvitkaushik-123 thanks, all three were right. Two new commits, dd7eb40 and e73d656:

1. control_media_buy tests. Its advertised schema is pinned: the five new entries exactly, and everything else equal to the schema the old types advertised. An MCP call through AddTool and WithIdempotency reaches the handler as IsNull(), IsZero() and Some(0) at media-buy and package level. A string daily_budget_cap is now rejected as invalid input, which is a second wire change; it's in the description and pinned by a test.
2. What null means. MIGRATING.md now gives each field's meaning from its own 3.2.2 description. For PackageControl budget, min_spend_target and bidding, and ControlMediaBuyRequest.bidding, the schema doesn't say, and their generated comments say so; a test fails if a later bundle starts defining it. I've corrected the description too.
3. Get() and null. MIGRATING.md now has the three checks: present (!IsZero(), replacing != nil), a value (Get()) and null (IsNull()), with a note that authorization and audit checks must use !IsZero(). For reference/seller-agent, the description now says a null package budget removes the ceiling, so attemptedAction must treat it as an increase or refuse it.

A further review on our side also added migration notes for go-sdk's mcp.AddTool and encoders that ignore omitzero; they're in MIGRATING.md and the description.