codegen: emit Default + #[serde(default)] for proto enum scalars#137
Open
prasanna-anchorage wants to merge 2 commits into
Open
codegen: emit Default + #[serde(default)] for proto enum scalars#137prasanna-anchorage wants to merge 2 commits into
prasanna-anchorage wants to merge 2 commits into
Conversation
Protobuf canonical JSON omits enum fields whose value is the zero / *_UNSPECIFIED variant. The current codegen emits these fields as bare enum types with neither a Default impl on the enum nor #[serde(default)] on the field, so a spec-compliant server response that omits a zero-valued enum scalar fails to deserialize with `missing field <name>`. Two changes in codegen/src/transform.rs: - mutate_struct: when an enum-scalar field is rewritten from i32 to the named enum type, also push #[serde(default)] for the bare (non-Vec, non-Option) case. The existing should_add_default() allowlist covers Vec<Enum> / Option<Enum> already. - mutate_enum: identify the variant whose explicit discriminant is 0 (the proto3 unspecified default), annotate it with #[default], and add #[derive(Default)] to the enum. Skip the flatten-style sum enums (Inner, TokenOrClaims) that already get special treatment. Surfaced when a real TVC deployment response omitted a TvcDeploymentStage field set to UNSPECIFIED. That specific field has since been removed from the proto, but the same defect currently affects several other scalar enum fields (Outcome, InvitationStatus, TagType, Curve, AddressFormat, Effect, AccessType, etc.) - any of which the server can legitimately serialize without a key when the value is the unspecified default. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`make generate` output after the codegen change. Adds `#[serde(default)]` to bare enum scalar fields and `#[derive(Default)] + #[default]` on the zero-discriminant variant of simple enums. No proto changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Bare-enum scalar fields in the generated
turnkey_clientcrate fail to deserialize when the server response (legitimately) omits them.Protobuf's canonical JSON mapping drops enum fields whose value is the zero /
*_UNSPECIFIEDvariant. With the current codegen, those fields are emitted as bare enum types with neither aDefaultimpl on the enum nor#[serde(default)]on the field, so a spec-compliant response without the key fails with:This surfaced live for me on
tvc deploy approveagainst a freshly-created deployment: the field wasTvcDeployment.stageset toTVC_DEPLOYMENT_STAGE_UNSPECIFIED. That specific field has since been removed from the proto, but the same defect affects every other bare enum scalar in the current generated client —Outcome,InvitationStatus,TagType,Curve,AddressFormat,Effect,AccessType,Oauth2Provider,FiatOnRampProvider,CredentialType,ApiKeyCurve, and friends — any of which can fail the same way for the same reason.Fix
Both changes live in
codegen/src/transform.rs; regenerated client files in the second commit.mutate_struct— when an enum-scalar field is rewritten fromi32to the named enum type, push#[serde(default)]for the bare (non-Vec, non-Option) case. The existingshould_add_defaultallowlist already coversVec<Enum>/Option<Enum>.mutate_enum— find the variant whose explicit discriminant is0(the proto3 unspecified default), annotate it with#[default], and add#[derive(Default)]to the enum. Skip the flatten-style sum enums (Inner,TokenOrClaims) that already get special treatment — they aren't simple discriminated enums.The regen diff is purely additive:
#[serde(default)]on the field,#[derive(Default)]on the enum,#[default]on the zero variant. No proto changes.Verification
cargo build --workspace✅cargo test --workspace✅ (all existing tests pass)cargo clippy --all-targets --all-features -- -D warnings✅cargo fmt -- --check✅tvcfrom this branch and re-ran the originally-failingtvc deploy approve --deploy-id … --operator-id …— now decodes the response and reaches the interactive approval prompt instead of erroring on a missing field.Notes
.rswould otherwise be wiped on the nextmake generate.make generateneededPROTOC_INCLUDE=/usr/local/includeon my machine to findgoogle/protobuf/descriptor.proto. Not a change in this PR; flagging in case it's worth documenting in the README.🤖 Generated with Claude Code