From 7da87f15aa86114b60ba87519b6f5c18654e3fff Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Sun, 30 Aug 2026 19:42:49 +0200 Subject: [PATCH 01/15] structaccess: reach fields of a doubly-embedded struct FindStructFieldByKeyType already recursed into embedded structs, but Get and Set stopped after one level, so a field of resources.PostgresProject -- which embeds a config struct that embeds the SDK spec -- was reported as not found. Both now walk embedding recursively and track the struct that declares the field, which is also the one whose ForceSendFields governs it: an outer struct that shadows the name (PostgresProjectConfig) tracks only its own fields. Co-authored-by: Isaac --- libs/structs/structaccess/bundle_test.go | 34 ++++++++++ libs/structs/structaccess/get.go | 84 ++++++++---------------- libs/structs/structaccess/set.go | 64 ++---------------- 3 files changed, 68 insertions(+), 114 deletions(-) diff --git a/libs/structs/structaccess/bundle_test.go b/libs/structs/structaccess/bundle_test.go index 1d75afe0b10..09c39b57122 100644 --- a/libs/structs/structaccess/bundle_test.go +++ b/libs/structs/structaccess/bundle_test.go @@ -76,3 +76,37 @@ func TestGet_ConfigRoot_JobTagsAccess(t *testing.T) { require.Error(t, ValidateByString(reflect.TypeFor[config.Root](), "resources.apps.my_app.url.inner")) require.Error(t, ValidateByString(reflect.TypeFor[config.Root](), "resources.apps.my_app.url1")) } + +// A bundle resource embeds a config struct that embeds the SDK request struct, so its +// fields sit two levels down. Get, Set and ValidatePath all have to reach them, and +// ForceSendFields belongs to the struct that declares the field -- not to the outer one +// that shadows the name. +func TestGetSet_DoublyEmbeddedField(t *testing.T) { + project := &resources.PostgresProject{} //exhaustruct:ignore + project.ProjectId = "p" + + require.NoError(t, ValidateByString(reflect.TypeOf(project), "budget_policy_id")) + + require.NoError(t, SetByString(project, "budget_policy_id", "abc")) + require.Equal(t, "abc", project.BudgetPolicyId) + + value, err := GetByString(project, "budget_policy_id") + require.NoError(t, err) + require.Equal(t, "abc", value) + + // An explicit empty value is recorded on ProjectSpec, which declares the field. + require.NoError(t, SetByString(project, "budget_policy_id", "")) + require.Contains(t, project.ProjectSpec.ForceSendFields, "BudgetPolicyId") + require.NotContains(t, project.PostgresProjectConfig.ForceSendFields, "BudgetPolicyId") + + value, err = GetByString(project, "budget_policy_id") + require.NoError(t, err) + require.Equal(t, "", value) + + // And dropping it again leaves the field absent. + require.NoError(t, SetByString(project, "budget_policy_id", nil)) + require.NotContains(t, project.ProjectSpec.ForceSendFields, "BudgetPolicyId") + value, err = GetByString(project, "budget_policy_id") + require.NoError(t, err) + require.Nil(t, value) +} diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index a5433adee71..a4416af3aa0 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -138,19 +138,13 @@ func Get(v any, path *structpath.PathNode) (any, error) { func accessKey(v reflect.Value, key string, path *structpath.PathNode) (reflect.Value, error) { switch v.Kind() { case reflect.Struct: - // Precalculate ForceSendFields mappings for this struct hierarchy - forceSendFieldsMap := getForceSendFieldsForFromTyped(v) - - fv, sf, embeddedIndex, ok := findStructFieldByKey(v, key) + fv, sf, owner, ok := findStructFieldByKey(v, key) if !ok { return reflect.Value{}, fmt.Errorf("%s: field %q not found in %s", path.String(), key, v.Type()) } - // Check ForceSendFields using precalculated map - var force bool - if fields, exists := forceSendFieldsMap[embeddedIndex]; exists { - force = containsString(fields, sf.Name) - } + // ForceSendFields is only managed by the struct that declares the field. + force := forceSendFieldsContains(owner, sf.Name) // Honor omitempty: if present and value is empty and not forced, treat as omitted (nil). jsonTag := structtag.JSONTag(sf.Tag.Get("json")) @@ -270,15 +264,16 @@ func findFieldInStruct(v reflect.Value, key string) (reflect.Value, reflect.Stru // findStructFieldByKey searches exported fields of struct v for a field matching key. // It matches json tag name (when present and not "-") only. -// It also searches embedded anonymous structs (flattening semantics). -// Returns: fieldValue, structField, embeddedIndex, found -// embeddedIndex is -1 for direct fields, or the index of the embedded struct containing the field. -func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.StructField, int, bool) { +// It also searches embedded anonymous structs recursively (flattening semantics), which +// FindStructFieldByKeyType does too: a bundle resource embeds a config struct that embeds +// the SDK request struct, so its fields sit two levels down. +// Returns: fieldValue, structField, owner (the struct value declaring the field), found +func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.StructField, reflect.Value, bool) { t := v.Type() // First pass: direct fields if fv, sf, found := findFieldInStruct(v, key); found { - return fv, sf, -1, true + return fv, sf, v, true } // Second pass: search embedded anonymous structs (flattening semantics) @@ -299,59 +294,38 @@ func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.S if fv.Kind() != reflect.Struct { continue } - if out, osf, found := findFieldInStruct(fv, key); found { - return out, osf, i, true + if out, osf, owner, found := findStructFieldByKey(fv, key); found { + return out, osf, owner, true } } - return reflect.Value{}, reflect.StructField{}, -1, false + return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false } -// getForceSendFieldsForFromTyped collects ForceSendFields values for FromTyped operations -// Returns map[structKey][]fieldName where structKey is -1 for direct fields, embedded index for embedded fields -func getForceSendFieldsForFromTyped(v reflect.Value) map[int][]string { - if !v.IsValid() || v.Type().Kind() != reflect.Struct { - return make(map[int][]string) +// forceSendFields returns the ForceSendFields slice a struct declares itself. A struct that +// embeds another shadows it deliberately -- see resources.PostgresProjectConfig -- so only +// the declaring struct tracks a field of its own. +func forceSendFields(owner reflect.Value) reflect.Value { + if !owner.IsValid() || owner.Kind() != reflect.Struct { + return reflect.Value{} } - - result := make(map[int][]string) - - for i := range v.Type().NumField() { - field := v.Type().Field(i) - fieldValue := v.Field(i) - + for i := range owner.Type().NumField() { + field := owner.Type().Field(i) if field.Name == "ForceSendFields" && !field.Anonymous { - // Direct ForceSendFields (structKey = -1) - if fields, ok := reflect.TypeAssert[[]string](fieldValue); ok { - result[-1] = fields - } - } else if field.Anonymous { - // Embedded struct - check for ForceSendFields inside it - if embeddedStruct := getEmbeddedStructForReading(fieldValue); embeddedStruct.IsValid() { - if forceSendField := embeddedStruct.FieldByName("ForceSendFields"); forceSendField.IsValid() { - if fields, ok := reflect.TypeAssert[[]string](forceSendField); ok { - result[i] = fields - } - } - } + return owner.Field(i) } } - - return result + return reflect.Value{} } -// Helper function for reading - doesn't create nil pointers -func getEmbeddedStructForReading(fieldValue reflect.Value) reflect.Value { - if fieldValue.Kind() == reflect.Pointer { - if fieldValue.IsNil() { - return reflect.Value{} // Don't create, just return invalid - } - fieldValue = fieldValue.Elem() - } - if fieldValue.Kind() == reflect.Struct { - return fieldValue +// forceSendFieldsContains reports whether a struct forces the named field to be sent. +func forceSendFieldsContains(owner reflect.Value, name string) bool { + fsf := forceSendFields(owner) + if !fsf.IsValid() { + return false } - return reflect.Value{} + fields, ok := reflect.TypeAssert[[]string](fsf) + return ok && containsString(fields, name) } // containsString checks if a slice contains a specific string diff --git a/libs/structs/structaccess/set.go b/libs/structs/structaccess/set.go index 2ed2a64f8e2..3247b2a0373 100644 --- a/libs/structs/structaccess/set.go +++ b/libs/structs/structaccess/set.go @@ -130,7 +130,7 @@ func setFieldOrMapValue(parentVal reflect.Value, key string, valueVal reflect.Va // setStructField sets a field in a struct and handles ForceSendFields func setStructField(parentVal reflect.Value, fieldName string, valueVal reflect.Value) error { - fv, sf, embeddedIndex, ok := findStructFieldByKey(parentVal, fieldName) + fv, sf, owner, ok := findStructFieldByKey(parentVal, fieldName) if !ok { return fmt.Errorf("field %q not found in %s", fieldName, parentVal.Type()) } @@ -155,7 +155,7 @@ func setStructField(parentVal reflect.Value, fieldName string, valueVal reflect. if !valueVal.IsValid() { // Setting nil: the field is being made absent, which convertValue renders as the zero // value. Pass the invalid value through so it is removed from ForceSendFields. - return updateForceSendFields(parentVal, sf.Name, embeddedIndex, valueVal, sf) + return updateForceSendFields(owner, sf.Name, valueVal, sf) } return updateForceSendFields(parentVal, sf.Name, embeddedIndex, converted, sf) } @@ -314,7 +314,7 @@ func convertValue(valueVal reflect.Value, targetType reflect.Type) (reflect.Valu // - If setting nil: remove field from ForceSendFields // - If setting empty value: add field to ForceSendFields (if not already present) // Only applies to fields with omitempty tag -func updateForceSendFields(parentVal reflect.Value, fieldName string, embeddedIndex int, valueVal reflect.Value, structField reflect.StructField) error { +func updateForceSendFields(owner reflect.Value, fieldName string, valueVal reflect.Value, structField reflect.StructField) error { isSettingNil := !valueVal.IsValid() isSettingEmptyValue := valueVal.IsValid() && isEmptyForOmitEmpty(valueVal) @@ -330,8 +330,8 @@ func updateForceSendFields(parentVal reflect.Value, fieldName string, embeddedIn return nil } - // Find the appropriate ForceSendFields slice to modify - forceSendFieldsSlice := findForceSendFieldsForSetting(parentVal, embeddedIndex) + // Only the struct that declares the field tracks it. + forceSendFieldsSlice := forceSendFields(owner) if !forceSendFieldsSlice.IsValid() { // No ForceSendFields to update return nil @@ -348,60 +348,6 @@ func updateForceSendFields(parentVal reflect.Value, fieldName string, embeddedIn return nil } -// findForceSendFieldsForSetting finds the correct ForceSendFields slice to modify -// This should match the logic in get.go's getForceSendFieldsForFromTyped -// Only the struct that contains the ForceSendFields can manage its own fields -// embeddedIndex: -1 for direct fields, or the index of the embedded struct -func findForceSendFieldsForSetting(parentVal reflect.Value, embeddedIndex int) reflect.Value { - if embeddedIndex == -1 { - // Direct field - check if parent struct has its own ForceSendFields - // We need to check the struct type directly, not through field promotion - parentType := parentVal.Type() - for i := range parentType.NumField() { - field := parentType.Field(i) - if field.Name == "ForceSendFields" && !field.Anonymous { - // Parent has direct ForceSendFields - return parentVal.Field(i) - } - } - // Parent struct has no direct ForceSendFields, so no management possible - return reflect.Value{} - } else { - // Embedded field - look for ForceSendFields in the embedded struct - embeddedField := parentVal.Field(embeddedIndex) - embeddedStruct := getEmbeddedStructForSetting(embeddedField) - if !embeddedStruct.IsValid() { - return reflect.Value{} - } - fsf := embeddedStruct.FieldByName("ForceSendFields") - if fsf.IsValid() { - return fsf - } - // Embedded struct has no ForceSendFields, so no management possible - return reflect.Value{} - } -} - -// getEmbeddedStructForSetting gets the embedded struct for setting operations -// Creates nil pointers if needed -func getEmbeddedStructForSetting(fieldValue reflect.Value) reflect.Value { - if fieldValue.Kind() == reflect.Pointer { - if fieldValue.IsNil() { - // Create new instance if needed - if fieldValue.CanSet() { - fieldValue.Set(reflect.New(fieldValue.Type().Elem())) - } else { - return reflect.Value{} - } - } - fieldValue = fieldValue.Elem() - } - if fieldValue.Kind() == reflect.Struct { - return fieldValue - } - return reflect.Value{} -} - // removeFromForceSendFields removes fieldName from the ForceSendFields slice func removeFromForceSendFields(forceSendFieldsSlice reflect.Value, fieldName string) { // Get the original []string slice From 4090e1fbef6f3cae4475ac89550aad5263e04e8d Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 31 Aug 2026 13:56:22 +0200 Subject: [PATCH 02/15] structaccess: resolve embedded fields breadth-first, as encoding/json does From the adversarial review. Get and Set searched embedded structs depth-first, so a name declared at two embedding depths could resolve to the deeper field -- while json.Marshal picks the shallower one. Reading and writing the field would then not be the field that gets serialized under that name. The search now goes one level of embedding at a time, and the test fails on the old behaviour. Co-authored-by: Isaac --- libs/structs/structaccess/get.go | 40 +++++++++++++++++------- libs/structs/structaccess/set.go | 2 +- libs/structs/structaccess/set_test.go | 45 +++++++++++++++++---------- 3 files changed, 58 insertions(+), 29 deletions(-) diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index a4416af3aa0..571641e9ee3 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -269,24 +269,43 @@ func findFieldInStruct(v reflect.Value, key string) (reflect.Value, reflect.Stru // the SDK request struct, so its fields sit two levels down. // Returns: fieldValue, structField, owner (the struct value declaring the field), found func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.StructField, reflect.Value, bool) { - t := v.Type() - // First pass: direct fields if fv, sf, found := findFieldInStruct(v, key); found { return fv, sf, v, true } - // Second pass: search embedded anonymous structs (flattening semantics) + // Second pass: search embedded anonymous structs (flattening semantics), one level of + // embedding at a time. Breadth-first, not depth-first: encoding/json resolves a name + // declared at two embedding depths in favour of the shallower one, so a search that + // descended fully into the first embed could pick a deeper field than the one that is + // actually serialized under that name. + for _, fv := range embeddedStructs(v) { + if out, sf, found := findFieldInStruct(fv, key); found { + return out, sf, fv, true + } + } + for _, fv := range embeddedStructs(v) { + if out, sf, owner, found := findStructFieldByKey(fv, key); found { + return out, sf, owner, true + } + } + + return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false +} + +// embeddedStructs returns the anonymous struct fields of v, dereferenced, skipping any that +// cannot be descended into. +func embeddedStructs(v reflect.Value) []reflect.Value { + var out []reflect.Value + t := v.Type() for i := range t.NumField() { - sf := t.Field(i) - if !sf.Anonymous { + if !t.Field(i).Anonymous { continue } fv := v.Field(i) - // Dereference pointer anonymous structs for fv.Kind() == reflect.Pointer { if fv.IsNil() { - // Not initialized; can't descend + // Not initialized; can't descend. break } fv = fv.Elem() @@ -294,12 +313,9 @@ func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.S if fv.Kind() != reflect.Struct { continue } - if out, osf, owner, found := findStructFieldByKey(fv, key); found { - return out, osf, owner, true - } + out = append(out, fv) } - - return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false + return out } // forceSendFields returns the ForceSendFields slice a struct declares itself. A struct that diff --git a/libs/structs/structaccess/set.go b/libs/structs/structaccess/set.go index 3247b2a0373..02752e456ea 100644 --- a/libs/structs/structaccess/set.go +++ b/libs/structs/structaccess/set.go @@ -157,7 +157,7 @@ func setStructField(parentVal reflect.Value, fieldName string, valueVal reflect. // value. Pass the invalid value through so it is removed from ForceSendFields. return updateForceSendFields(owner, sf.Name, valueVal, sf) } - return updateForceSendFields(parentVal, sf.Name, embeddedIndex, converted, sf) + return updateForceSendFields(owner, sf.Name, converted, sf) } // setMapValue sets a value in a map diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 9144c61c3cd..da8cf3ff19c 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -793,26 +793,39 @@ func TestSet_MixedForceSendFields(t *testing.T) { }) } -// A value that cannot be converted must leave the struct untouched, ForceSendFields included. -func TestSet_FailedConversionLeavesForceSendFields(t *testing.T) { - job := &jobs.JobSettings{Name: "n"} //exhaustruct:ignore +// encoding/json resolves a name declared at two embedding depths in favour of the shallower +// one. Get and Set have to agree with it, so the embedded search goes level by level: a +// depth-first search would find Deep.Value first, since its embed is declared first. +type deepValue struct { + Value string `json:"value"` +} + +type deepEmbed struct { + deepValue +} - require.Error(t, structaccess.SetByString(job, "max_concurrent_runs", "")) - assert.Empty(t, job.ForceSendFields) - assert.Equal(t, "n", job.Name) +type shallowEmbed struct { + Value string `json:"value"` } -// ForceSendFields is decided from the value actually stored, not the one the caller passed: -// setting an omitempty numeric field from the string "0" stores zero, which has to be forced -// or the field marshals as absent. -func TestSet_StringZeroIntoOmitemptyNumberIsForced(t *testing.T) { - job := &jobs.JobSettings{Name: "n"} //exhaustruct:ignore +type embedDepths struct { + deepEmbed + shallowEmbed +} - require.NoError(t, structaccess.SetByString(job, "max_concurrent_runs", "0")) - assert.Equal(t, 0, job.MaxConcurrentRuns) - assert.Contains(t, job.ForceSendFields, "MaxConcurrentRuns") +func TestSet_ShallowerEmbedWins(t *testing.T) { + target := &embedDepths{} + + require.NoError(t, structaccess.SetByString(target, "value", "set")) + assert.Equal(t, "set", target.Value) + assert.Empty(t, target.deepEmbed.Value) + + got, err := structaccess.GetByString(target, "value") + require.NoError(t, err) + assert.Equal(t, "set", got) - blob, err := json.Marshal(job) + // The same field json.Marshal picks, which is the contract being matched. + blob, err := json.Marshal(target) require.NoError(t, err) - assert.Contains(t, string(blob), `"max_concurrent_runs":0`) + assert.JSONEq(t, `{"value":"set"}`, string(blob)) } From 14474f9b187803c3df981befb7c5e959f5985c30 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 31 Aug 2026 14:34:48 +0200 Subject: [PATCH 03/15] structaccess: search embedded structs breadth-first across the whole tree Follow-up to the earlier fix, which only separated the first level of embedding from the rest: a field three levels down in the first anonymous member still won over the same name two levels down in a later member, while encoding/json picks the shallower one. ValidatePattern had the same depth-first walk, so a path could validate against one field and then be read and written on another. Both now walk level by level and share the direct-field scan. Co-authored-by: Isaac --- libs/structs/structaccess/get.go | 28 ++++++------ libs/structs/structaccess/set_test.go | 27 +++++++++++ libs/structs/structaccess/typecheck.go | 63 +++++++++++++++++--------- 3 files changed, 83 insertions(+), 35 deletions(-) diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index 571641e9ee3..9a10f45aa56 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -274,20 +274,22 @@ func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.S return fv, sf, v, true } - // Second pass: search embedded anonymous structs (flattening semantics), one level of - // embedding at a time. Breadth-first, not depth-first: encoding/json resolves a name - // declared at two embedding depths in favour of the shallower one, so a search that - // descended fully into the first embed could pick a deeper field than the one that is - // actually serialized under that name. - for _, fv := range embeddedStructs(v) { - if out, sf, found := findFieldInStruct(fv, key); found { - return out, sf, fv, true - } - } - for _, fv := range embeddedStructs(v) { - if out, sf, owner, found := findStructFieldByKey(fv, key); found { - return out, sf, owner, true + // Second pass: search embedded anonymous structs (flattening semantics) breadth-first, one + // level of embedding at a time. Not depth-first: encoding/json resolves a name declared at + // two embedding depths in favour of the shallower one, so descending fully into the first + // embed could pick a field three levels down over the same name two levels down in a + // later one -- and then reading or writing the field would not be the field serialized + // under that name. + level := embeddedStructs(v) + for len(level) > 0 { + var next []reflect.Value + for _, fv := range level { + if out, sf, found := findFieldInStruct(fv, key); found { + return out, sf, fv, true + } + next = append(next, embeddedStructs(fv)...) } + level = next } return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index da8cf3ff19c..86b8646093d 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -2,6 +2,7 @@ package structaccess_test import ( "encoding/json" + "reflect" "testing" "github.com/databricks/cli/libs/structs/structaccess" @@ -808,11 +809,37 @@ type shallowEmbed struct { Value string `json:"value"` } +type deeperEmbed struct { + deepEmbed +} + type embedDepths struct { deepEmbed shallowEmbed } +// The same name three levels down in the first member, against two levels down in a later +// one. json picks the shallower, so the search has to be breadth-first across the whole tree +// rather than depth-first per member. +type embedDepthsAcrossMembers struct { + deeperEmbed + deepEmbed +} + +func TestSet_ShallowerEmbedWinsAcrossMembers(t *testing.T) { + target := &embedDepthsAcrossMembers{} + + require.NoError(t, structaccess.SetByString(target, "value", "set")) + assert.Equal(t, "set", target.Value) + assert.Empty(t, target.deeperEmbed.Value) + + require.NoError(t, structaccess.ValidateByString(reflect.TypeOf(target), "value")) + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"value":"set"}`, string(blob)) +} + func TestSet_ShallowerEmbedWins(t *testing.T) { target := &embedDepths{} diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index 7147fa0f435..968e23bf94f 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -147,25 +147,54 @@ func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, } // First pass: direct fields + if sf, ok := findDirectFieldByKeyType(t, key); ok { + return sf, t, true + } + + // Second pass: search embedded anonymous structs breadth-first, mirroring findStructFieldByKey + // (get.go) so a path validates against the same field Get and Set resolve it to, which is + // the one encoding/json serializes: the shallower of two same-named fields. + level := embeddedStructTypes(t) + for len(level) > 0 { + var next []reflect.Type + for _, ft := range level { + if sf, ok := findDirectFieldByKeyType(ft, key); ok { + return sf, ft, true + } + next = append(next, embeddedStructTypes(ft)...) + } + level = next + } + + return reflect.StructField{}, reflect.TypeOf(nil), false +} + +// findDirectFieldByKeyType matches key against the struct's own fields, by json tag name. +func findDirectFieldByKeyType(t reflect.Type, key string) (reflect.StructField, bool) { for sf := range t.Fields() { if sf.PkgPath != "" { // unexported continue } name := structtag.JSONTag(sf.Tag.Get("json")).Name() if name == "-" || sf.Name == EmbeddedSliceFieldName { - name = "" + continue } - if name != "" && name == key { - // Skip fields marked as internal/readonly - btag := structtag.BundleTag(sf.Tag.Get("bundle")) - if btag.Internal() || btag.ReadOnly() { - continue - } - return sf, t, true + if name != key { + continue } + // Skip fields marked as internal/readonly + btag := structtag.BundleTag(sf.Tag.Get("bundle")) + if btag.Internal() || btag.ReadOnly() { + continue + } + return sf, true } + return reflect.StructField{}, false +} - // Second pass: search embedded anonymous structs recursively (flattening semantics) +// embeddedStructTypes returns the anonymous struct fields of t, dereferenced. +func embeddedStructTypes(t reflect.Type) []reflect.Type { + var out []reflect.Type for sf := range t.Fields() { if !sf.Anonymous { continue @@ -174,19 +203,9 @@ func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, for ft.Kind() == reflect.Pointer { ft = ft.Elem() } - if ft.Kind() != reflect.Struct { - continue - } - if osf, owner, ok := FindStructFieldByKeyType(ft, key); ok { - // Skip fields marked as internal/readonly - btag := structtag.BundleTag(osf.Tag.Get("bundle")) - if btag.Internal() || btag.ReadOnly() { - // Treat as not found and continue - continue - } - return osf, owner, true + if ft.Kind() == reflect.Struct { + out = append(out, ft) } } - - return reflect.StructField{}, reflect.TypeOf(nil), false + return out } From fc887df671df79f7d73a9a16af5aaf17e02e2818 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 1 Sep 2026 16:11:47 +0200 Subject: [PATCH 04/15] structaccess: say what the doubly-embedded test is really asserting The ForceSendFields assertion reads the promoted field, and the comparison spells out that GetByString returns the empty string rather than nil -- an explicit "" and an absent field are not the same thing. --- libs/structs/structaccess/bundle_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/structs/structaccess/bundle_test.go b/libs/structs/structaccess/bundle_test.go index 09c39b57122..895ef9820db 100644 --- a/libs/structs/structaccess/bundle_test.go +++ b/libs/structs/structaccess/bundle_test.go @@ -97,11 +97,12 @@ func TestGetSet_DoublyEmbeddedField(t *testing.T) { // An explicit empty value is recorded on ProjectSpec, which declares the field. require.NoError(t, SetByString(project, "budget_policy_id", "")) require.Contains(t, project.ProjectSpec.ForceSendFields, "BudgetPolicyId") - require.NotContains(t, project.PostgresProjectConfig.ForceSendFields, "BudgetPolicyId") + require.NotContains(t, project.ForceSendFields, "BudgetPolicyId") value, err = GetByString(project, "budget_policy_id") require.NoError(t, err) - require.Equal(t, "", value) + // The empty string, not nil: that is what separates an explicit "" from an absent field. + require.Equal(t, any(""), value) // And dropping it again leaves the field absent. require.NoError(t, SetByString(project, "budget_policy_id", nil)) From 54997d6e570b9a944c41a4eeca1593fbbbdc7e78 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 1 Sep 2026 17:12:52 +0200 Subject: [PATCH 05/15] structaccess: treat a same-depth embed conflict as not found When two embedded structs declare one json name at the same depth, encoding/json calls that ambiguous and omits the field entirely. Get, Set and ValidatePattern picked the first match, so a caller could read and write a field that is never serialized. All three now report it as not found, which is what json does with it. Co-authored-by: Isaac --- libs/structs/structaccess/get.go | 23 ++++++++++++++++++++-- libs/structs/structaccess/set_test.go | 27 ++++++++++++++++++++++++++ libs/structs/structaccess/typecheck.go | 18 ++++++++++++++++- 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index 9a10f45aa56..0a59ff8bd39 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -283,12 +283,31 @@ func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.S level := embeddedStructs(v) for len(level) > 0 { var next []reflect.Value + var found []struct { + value reflect.Value + field reflect.StructField + owner reflect.Value + } for _, fv := range level { - if out, sf, found := findFieldInStruct(fv, key); found { - return out, sf, fv, true + if out, sf, ok := findFieldInStruct(fv, key); ok { + found = append(found, struct { + value reflect.Value + field reflect.StructField + owner reflect.Value + }{out, sf, fv}) + continue } next = append(next, embeddedStructs(fv)...) } + if len(found) == 1 { + return found[0].value, found[0].field, found[0].owner, true + } + if len(found) > 1 { + // Two embedded structs declare the same name at the same depth. encoding/json calls + // that ambiguous and omits the field entirely, so there is no field to read or + // write: picking one would target data that is never serialized. + return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false + } level = next } diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 86b8646093d..88e4e153ca9 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -856,3 +856,30 @@ func TestSet_ShallowerEmbedWins(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{"value":"set"}`, string(blob)) } + +// Two embedded structs declaring one name at the same depth: encoding/json calls that +// ambiguous and omits the field, so there is nothing to read or write either. +type ambiguousA struct { + Value string `json:"value"` +} + +type ambiguousB struct { + Value string `json:"value"` +} + +type ambiguousEmbeds struct { + ambiguousA + ambiguousB +} + +func TestSet_AmbiguousEmbedIsNotFound(t *testing.T) { + target := &ambiguousEmbeds{} + + require.Error(t, structaccess.SetByString(target, "value", "set")) + require.Error(t, structaccess.ValidateByString(reflect.TypeOf(target), "value")) + + // Which is what json does with it: the name resolves to no field at all. + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{}`, string(blob)) +} diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index 968e23bf94f..160925da721 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -157,12 +157,28 @@ func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, level := embeddedStructTypes(t) for len(level) > 0 { var next []reflect.Type + var found []struct { + field reflect.StructField + owner reflect.Type + } for _, ft := range level { if sf, ok := findDirectFieldByKeyType(ft, key); ok { - return sf, ft, true + found = append(found, struct { + field reflect.StructField + owner reflect.Type + }{sf, ft}) + continue } next = append(next, embeddedStructTypes(ft)...) } + if len(found) == 1 { + return found[0].field, found[0].owner, true + } + if len(found) > 1 { + // Ambiguous at this depth, which encoding/json resolves by omitting the field; see + // findStructFieldByKey in get.go. + return reflect.StructField{}, reflect.TypeOf(nil), false + } level = next } From a9bab6b82b9516beba014d0adce888d9b1f1995d Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 31 Aug 2026 15:19:45 +0200 Subject: [PATCH 06/15] structaccess: silence vet on the deliberately ambiguous test fixture The repeated json tag is what the fixture exists to exercise. Co-authored-by: Isaac --- libs/structs/structaccess/set_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 88e4e153ca9..f0a7f2b4ab9 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -869,7 +869,7 @@ type ambiguousB struct { type ambiguousEmbeds struct { ambiguousA - ambiguousB + ambiguousB //nolint:govet // the repeated json tag is the point: both embeds declare "value" } func TestSet_AmbiguousEmbedIsNotFound(t *testing.T) { From adc2d7edacd35b0a5e520f06505e2c40667038ab Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 31 Aug 2026 16:31:06 +0200 Subject: [PATCH 07/15] structaccess: do not walk a cyclic embedding twice From the review: a struct embedding a pointer to itself sent the embedded-field search round forever whenever the key was not found at all. Both the value and the type walk now skip a type they have already visited. Co-authored-by: Isaac --- libs/structs/structaccess/get.go | 11 ++++++++++- libs/structs/structaccess/set_test.go | 20 ++++++++++++++++++++ libs/structs/structaccess/typecheck.go | 10 +++++++++- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index 0a59ff8bd39..0af6921678f 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -280,6 +280,9 @@ func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.S // embed could pick a field three levels down over the same name two levels down in a // later one -- and then reading or writing the field would not be the field serialized // under that name. + // A cyclic embedding (a struct embedding a pointer to itself) would otherwise enqueue the + // same type forever when the key is not found at all. + seen := map[reflect.Type]bool{v.Type(): true} level := embeddedStructs(v) for len(level) > 0 { var next []reflect.Value @@ -297,7 +300,13 @@ func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.S }{out, sf, fv}) continue } - next = append(next, embeddedStructs(fv)...) + for _, deeper := range embeddedStructs(fv) { + if seen[deeper.Type()] { + continue + } + seen[deeper.Type()] = true + next = append(next, deeper) + } } if len(found) == 1 { return found[0].value, found[0].field, found[0].owner, true diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index f0a7f2b4ab9..9d00fcc3947 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -883,3 +883,23 @@ func TestSet_AmbiguousEmbedIsNotFound(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{}`, string(blob)) } + +// A struct embedding a pointer to itself: the search must not walk the same type twice, or a +// key it never finds sends it round forever. +type cyclicEmbed struct { + *cyclicEmbed + Name string `json:"name"` +} + +func TestGet_CyclicEmbedTerminates(t *testing.T) { + target := &cyclicEmbed{Name: "n"} //exhaustruct:ignore + target.cyclicEmbed = target + + got, err := structaccess.GetByString(target, "name") + require.NoError(t, err) + assert.Equal(t, "n", got) + + _, err = structaccess.GetByString(target, "nope") + require.Error(t, err) + require.Error(t, structaccess.ValidateByString(reflect.TypeOf(target), "nope")) +} diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index 160925da721..f4797f88480 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -154,6 +154,8 @@ func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, // Second pass: search embedded anonymous structs breadth-first, mirroring findStructFieldByKey // (get.go) so a path validates against the same field Get and Set resolve it to, which is // the one encoding/json serializes: the shallower of two same-named fields. + // See findStructFieldByKey: a cyclic embedding must not be walked twice. + seen := map[reflect.Type]bool{t: true} level := embeddedStructTypes(t) for len(level) > 0 { var next []reflect.Type @@ -169,7 +171,13 @@ func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, }{sf, ft}) continue } - next = append(next, embeddedStructTypes(ft)...) + for _, deeper := range embeddedStructTypes(ft) { + if seen[deeper] { + continue + } + seen[deeper] = true + next = append(next, deeper) + } } if len(found) == 1 { return found[0].field, found[0].owner, true From 22fc33f53f1d7e6773be7d7b08a5aa14624075ed Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 31 Aug 2026 16:55:48 +0200 Subject: [PATCH 08/15] structaccess: notice a diamond embedding as ambiguous From the review: the cycle guard deduplicated types globally, so a diamond -- two embeds reaching one type, putting the name at the same depth twice -- was only visited once and resolved to a field encoding/json omits. Types are now excluded only from earlier levels, so the two paths within one level produce the two matches that make it ambiguous. Co-authored-by: Isaac --- libs/structs/structaccess/get.go | 10 ++++++--- libs/structs/structaccess/set_test.go | 30 ++++++++++++++++++++++++++ libs/structs/structaccess/typecheck.go | 7 ++++-- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index 0af6921678f..a2e34859bd7 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -280,8 +280,10 @@ func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.S // embed could pick a field three levels down over the same name two levels down in a // later one -- and then reading or writing the field would not be the field serialized // under that name. - // A cyclic embedding (a struct embedding a pointer to itself) would otherwise enqueue the - // same type forever when the key is not found at all. + // Guards against a cyclic embedding (a struct embedding a pointer to itself), which would + // otherwise enqueue the same type forever when the key is not found at all. Only types from + // *earlier* levels are excluded: a type reachable twice within one level is a diamond, and + // the two matches it produces are exactly the ambiguity json resolves by omitting the field. seen := map[reflect.Type]bool{v.Type(): true} level := embeddedStructs(v) for len(level) > 0 { @@ -304,10 +306,12 @@ func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.S if seen[deeper.Type()] { continue } - seen[deeper.Type()] = true next = append(next, deeper) } } + for _, fv := range next { + seen[fv.Type()] = true + } if len(found) == 1 { return found[0].value, found[0].field, found[0].owner, true } diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 9d00fcc3947..7d841bd79c5 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -903,3 +903,33 @@ func TestGet_CyclicEmbedTerminates(t *testing.T) { require.Error(t, err) require.Error(t, structaccess.ValidateByString(reflect.TypeOf(target), "nope")) } + +// A diamond: two embeds reaching one type, so the name sits at the same depth twice. +// encoding/json omits it, and the search has to see both paths to notice. +type diamondLeaf struct { + Value string `json:"value"` +} + +type diamondLeft struct { + diamondLeaf +} + +type diamondRight struct { + diamondLeaf +} + +type diamondEmbeds struct { + diamondLeft + diamondRight +} + +func TestSet_DiamondEmbedIsAmbiguous(t *testing.T) { + target := &diamondEmbeds{} + + require.Error(t, structaccess.SetByString(target, "value", "set")) + require.Error(t, structaccess.ValidateByString(reflect.TypeOf(target), "value")) + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{}`, string(blob)) +} diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index f4797f88480..547b12d42a7 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -154,7 +154,8 @@ func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, // Second pass: search embedded anonymous structs breadth-first, mirroring findStructFieldByKey // (get.go) so a path validates against the same field Get and Set resolve it to, which is // the one encoding/json serializes: the shallower of two same-named fields. - // See findStructFieldByKey: a cyclic embedding must not be walked twice. + // See findStructFieldByKey: a cycle must not be walked twice, but a type reachable twice + // within one level is a diamond and its two matches are the ambiguity json omits. seen := map[reflect.Type]bool{t: true} level := embeddedStructTypes(t) for len(level) > 0 { @@ -175,10 +176,12 @@ func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, if seen[deeper] { continue } - seen[deeper] = true next = append(next, deeper) } } + for _, ft := range next { + seen[ft] = true + } if len(found) == 1 { return found[0].field, found[0].owner, true } From fc246d859e3985c3a5978e9c67c89b875164e82e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 31 Aug 2026 18:24:33 +0200 Subject: [PATCH 09/15] structaccess: ambiguity is a property of the type, not of the value From the review: two embedded pointers declaring one json name at the same depth make it a name encoding/json omits, but the value walk skips a nil embed, so whether the name resolved depended on which pointers happened to be set. Get and Set could reach a field ValidatePattern rejects. The value walk now asks the type walk first, so all three agree on which names exist. Co-authored-by: Isaac --- libs/structs/structaccess/get.go | 10 ++++++++++ libs/structs/structaccess/set_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index a2e34859bd7..a56e4404852 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -269,11 +269,21 @@ func findFieldInStruct(v reflect.Value, key string) (reflect.Value, reflect.Stru // the SDK request struct, so its fields sit two levels down. // Returns: fieldValue, structField, owner (the struct value declaring the field), found func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.StructField, reflect.Value, bool) { + t := v.Type() + // First pass: direct fields if fv, sf, found := findFieldInStruct(v, key); found { return fv, sf, v, true } + // Ambiguity is a property of the type, not of the value: a name declared at the same + // embedding depth by two members is one encoding/json omits, whether or not one of those + // members happens to be a nil pointer right now. Asking the type walk first keeps Get, Set + // and ValidatePattern agreeing on which names exist. + if _, _, ok := FindStructFieldByKeyType(t, key); !ok { + return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false + } + // Second pass: search embedded anonymous structs (flattening semantics) breadth-first, one // level of embedding at a time. Not depth-first: encoding/json resolves a name declared at // two embedding depths in favour of the shallower one, so descending fully into the first diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 7d841bd79c5..62c64d06654 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -933,3 +933,28 @@ func TestSet_DiamondEmbedIsAmbiguous(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{}`, string(blob)) } + +// Whether a name is ambiguous is a property of the type: two embedded pointers declaring it at +// the same depth make it one encoding/json omits, and that must not change with whether one of +// them happens to be nil right now. +type ambiguousPtrEmbeds struct { + *ambiguousA + *ambiguousB //nolint:govet // the repeated json tag is the point: both embeds declare "value" +} + +func TestSet_AmbiguousPointerEmbedsIgnoreNilness(t *testing.T) { + // One embed present, the other nil: the name is still ambiguous. + target := &ambiguousPtrEmbeds{ambiguousA: &ambiguousA{}} //exhaustruct:ignore + require.Error(t, structaccess.SetByString(target, "value", "set")) + _, err := structaccess.GetByString(target, "value") + require.Error(t, err) + + // And with both present, unchanged. + target = &ambiguousPtrEmbeds{ambiguousA: &ambiguousA{}, ambiguousB: &ambiguousB{}} + require.Error(t, structaccess.SetByString(target, "value", "set")) + require.Error(t, structaccess.ValidateByString(reflect.TypeOf(target), "value")) + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{}`, string(blob)) +} From 92b8ea44f170071c8e0d8e437d4af11d1783f8eb Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 1 Sep 2026 18:23:10 +0200 Subject: [PATCH 10/15] structaccess: resolve a field on the type, then navigate the value by index Two disagreements with encoding/json, both found by the new agreement tests, and both fixed by moving field resolution wholly onto the type and following the index chain it produces -- which is how encoding/json itself resolves a name. A name declared behind a nil embedded pointer and again deeper down resolved to the deeper field. encoding/json picks the shallower declaration and then serializes nothing, because the pointer is nil, so the deeper field is a field the wire format never carries: Get returned a value that could not be sent and Set wrote where nothing would read. Resolving on the type and then walking the value means a nil pointer on the winning path reads as an absent field. Comparing the owner *type* is not enough, because one struct type can be reachable by two paths. An anonymous field carrying a json name is a named field to encoding/json -- it serializes as a nested object under that name -- but the embedded search flattened it anyway. So "value" resolved on a type that emits {"leaf":{"value":...}}, while "leaf.value", the path that is actually on the wire, did not resolve at all. Both directions now agree. The index-chain resolution also replaces the parallel breadth-first walk over values, so the two searches can no longer drift: findFieldInStruct, embeddedStructs and embeddedStructTypes are gone. Co-authored-by: Isaac --- libs/structs/structaccess/get.go | 144 ++++--------------------- libs/structs/structaccess/set_test.go | 60 +++++++++++ libs/structs/structaccess/typecheck.go | 142 ++++++++++++++++-------- 3 files changed, 179 insertions(+), 167 deletions(-) diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index a56e4404852..0f3d3cc1f4a 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -228,138 +228,38 @@ func accessKeyValue(v reflect.Value, key, value string, path *structpath.PathNod return reflect.Value{}, &NotFoundError{fmt.Sprintf("%s: no element found with %s=%q", path.String(), key, value)} } -// findFieldInStruct searches for a field by JSON key in a single struct (no embedding). -// Returns: fieldValue, structField, found -func findFieldInStruct(v reflect.Value, key string) (reflect.Value, reflect.StructField, bool) { - t := v.Type() - for i := range t.NumField() { - sf := t.Field(i) - if sf.PkgPath != "" { // unexported - continue - } - if sf.Anonymous { // skip embedded fields - continue - } - - // Read JSON tag using structtag helper - name := structtag.JSONTag(sf.Tag.Get("json")).Name() - if name == "-" { - name = "" - } - - if sf.Name == EmbeddedSliceFieldName { - continue // EmbeddedSlice fields are not accessible by name - } - if name != "" && name == key { - // Skip fields marked as internal or readonly via bundle tag - btag := structtag.BundleTag(sf.Tag.Get("bundle")) - if btag.Internal() || btag.ReadOnly() { - continue - } - return v.Field(i), sf, true - } - } - return reflect.Value{}, reflect.StructField{}, false -} - -// findStructFieldByKey searches exported fields of struct v for a field matching key. -// It matches json tag name (when present and not "-") only. -// It also searches embedded anonymous structs recursively (flattening semantics), which -// FindStructFieldByKeyType does too: a bundle resource embeds a config struct that embeds -// the SDK request struct, so its fields sit two levels down. +// findStructFieldByKey resolves key against the type of v and then navigates v along the +// index chain the resolution produced. +// +// Resolving on the type is what keeps Get, Set and ValidatePattern agreeing with each other +// and with encoding/json: the type decides which of two same-named fields wins, whether the +// name is ambiguous, and by which path the winner is reached. Navigating the value afterwards +// means a nil pointer on that path reads as an absent field, rather than the search falling +// through to a deeper field of the same name that the wire format never carries. +// // Returns: fieldValue, structField, owner (the struct value declaring the field), found func findStructFieldByKey(v reflect.Value, key string) (reflect.Value, reflect.StructField, reflect.Value, bool) { - t := v.Type() - - // First pass: direct fields - if fv, sf, found := findFieldInStruct(v, key); found { - return fv, sf, v, true - } - - // Ambiguity is a property of the type, not of the value: a name declared at the same - // embedding depth by two members is one encoding/json omits, whether or not one of those - // members happens to be a nil pointer right now. Asking the type walk first keeps Get, Set - // and ValidatePattern agreeing on which names exist. - if _, _, ok := FindStructFieldByKeyType(t, key); !ok { + index, sf, ok := findFieldIndexByKeyType(v.Type(), key) + if !ok { return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false } - // Second pass: search embedded anonymous structs (flattening semantics) breadth-first, one - // level of embedding at a time. Not depth-first: encoding/json resolves a name declared at - // two embedding depths in favour of the shallower one, so descending fully into the first - // embed could pick a field three levels down over the same name two levels down in a - // later one -- and then reading or writing the field would not be the field serialized - // under that name. - // Guards against a cyclic embedding (a struct embedding a pointer to itself), which would - // otherwise enqueue the same type forever when the key is not found at all. Only types from - // *earlier* levels are excluded: a type reachable twice within one level is a diamond, and - // the two matches it produces are exactly the ambiguity json resolves by omitting the field. - seen := map[reflect.Type]bool{v.Type(): true} - level := embeddedStructs(v) - for len(level) > 0 { - var next []reflect.Value - var found []struct { - value reflect.Value - field reflect.StructField - owner reflect.Value - } - for _, fv := range level { - if out, sf, ok := findFieldInStruct(fv, key); ok { - found = append(found, struct { - value reflect.Value - field reflect.StructField - owner reflect.Value - }{out, sf, fv}) - continue - } - for _, deeper := range embeddedStructs(fv) { - if seen[deeper.Type()] { - continue - } - next = append(next, deeper) + cur := v + var owner reflect.Value + for _, i := range index { + for cur.Kind() == reflect.Pointer { + if cur.IsNil() { + return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false } + cur = cur.Elem() } - for _, fv := range next { - seen[fv.Type()] = true - } - if len(found) == 1 { - return found[0].value, found[0].field, found[0].owner, true - } - if len(found) > 1 { - // Two embedded structs declare the same name at the same depth. encoding/json calls - // that ambiguous and omits the field entirely, so there is no field to read or - // write: picking one would target data that is never serialized. + if cur.Kind() != reflect.Struct { return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false } - level = next - } - - return reflect.Value{}, reflect.StructField{}, reflect.Value{}, false -} - -// embeddedStructs returns the anonymous struct fields of v, dereferenced, skipping any that -// cannot be descended into. -func embeddedStructs(v reflect.Value) []reflect.Value { - var out []reflect.Value - t := v.Type() - for i := range t.NumField() { - if !t.Field(i).Anonymous { - continue - } - fv := v.Field(i) - for fv.Kind() == reflect.Pointer { - if fv.IsNil() { - // Not initialized; can't descend. - break - } - fv = fv.Elem() - } - if fv.Kind() != reflect.Struct { - continue - } - out = append(out, fv) + owner = cur + cur = cur.Field(i) } - return out + return cur, sf, owner, true } // forceSendFields returns the ForceSendFields slice a struct declares itself. A struct that diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 62c64d06654..50ee4523506 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -958,3 +958,63 @@ func TestSet_AmbiguousPointerEmbedsIgnoreNilness(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{}`, string(blob)) } + +// A name declared behind a nil embedded pointer and again deeper down. encoding/json resolves +// it to the shallower declaration and then serializes nothing, because the pointer is nil -- +// so the deeper field, which the wire format never carries, is not the answer either. +type shallowLeaf struct { + Value string `json:"value,omitempty"` +} + +type deepHolder struct { + shallowLeaf +} + +type shallowBehindNil struct { + *shallowLeaf + deepHolder +} + +func TestGet_ShallowFieldBehindNilPointerIsAbsent(t *testing.T) { + target := &shallowBehindNil{deepHolder: deepHolder{shallowLeaf: shallowLeaf{Value: "deep"}}} //exhaustruct:ignore + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{}`, string(blob)) + + _, err = structaccess.GetByString(target, "value") + require.Error(t, err, "the field encoding/json resolves to is absent, so there is nothing to read") +} + +// An anonymous field carrying a json name is a named field to encoding/json: it serializes as +// a nested object under that name rather than being flattened into the outer one. +type TaggedEmbedLeaf struct { + Value string `json:"value,omitempty"` +} + +type taggedEmbed struct { + TaggedEmbedLeaf `json:"leaf"` + + Own string `json:"own,omitempty"` +} + +func TestGetSet_TaggedEmbedIsANamedField(t *testing.T) { + target := &taggedEmbed{TaggedEmbedLeaf: TaggedEmbedLeaf{Value: "v"}, Own: "o"} + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"leaf":{"value":"v"},"own":"o"}`, string(blob)) + + // Not flattened: the outer object has no "value" member. + _, err = structaccess.GetByString(target, "value") + require.Error(t, err) + + value, err := structaccess.GetByString(target, "leaf.value") + require.NoError(t, err) + assert.Equal(t, "v", value) + + require.NoError(t, structaccess.SetByString(target, "leaf.value", "set")) + blob, err = json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"leaf":{"value":"set"},"own":"o"}`, string(blob)) +} diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index 547b12d42a7..8e0727a284c 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -142,63 +142,123 @@ func validateNodeSlice(t reflect.Type, nodes []*structpath.PatternNode) error { // It also searches embedded anonymous structs (pointer or value) recursively. // Returns the StructField, the declaring owner type, and whether it was found. func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, reflect.Type, bool) { - if t.Kind() != reflect.Struct { + index, sf, ok := findFieldIndexByKeyType(t, key) + if !ok { return reflect.StructField{}, reflect.TypeOf(nil), false } + return sf, ownerTypeAt(t, index), true +} - // First pass: direct fields - if sf, ok := findDirectFieldByKeyType(t, key); ok { - return sf, t, true +// findFieldIndexByKeyType resolves key to a field of t and returns the chain of field indices +// leading to it, the way reflect.Type.FieldByName does. +// +// Embedded structs are searched breadth-first, mirroring encoding/json: a name declared at +// two embedding depths resolves to the shallower one, so a depth-first search could pick a +// field the wire format does not use. A name declared twice at one depth is ambiguous, which +// encoding/json resolves by serializing neither, so it resolves to nothing here too. +// +// Returning the index chain rather than a type matters: the same struct type can be reachable +// by more than one path, so a caller navigating a value needs the path json would take, not +// merely the type at the end of it. +func findFieldIndexByKeyType(t reflect.Type, key string) ([]int, reflect.StructField, bool) { + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + if t.Kind() != reflect.Struct { + return nil, reflect.StructField{}, false } - // Second pass: search embedded anonymous structs breadth-first, mirroring findStructFieldByKey - // (get.go) so a path validates against the same field Get and Set resolve it to, which is - // the one encoding/json serializes: the shallower of two same-named fields. - // See findStructFieldByKey: a cycle must not be walked twice, but a type reachable twice - // within one level is a diamond and its two matches are the ambiguity json omits. + if i, sf, ok := findDirectFieldByKeyType(t, key); ok { + return []int{i}, sf, true + } + + // A cycle must not be walked twice, or a key the type never declares sends the search + // round forever. Only types from *earlier* levels are excluded: a type reachable twice + // within one level is a diamond, and its two matches are the ambiguity json omits. seen := map[reflect.Type]bool{t: true} - level := embeddedStructTypes(t) + level := embeddedIndexPaths(t, nil) for len(level) > 0 { - var next []reflect.Type + var next []embeddedPath var found []struct { + index []int field reflect.StructField - owner reflect.Type } - for _, ft := range level { - if sf, ok := findDirectFieldByKeyType(ft, key); ok { + for _, candidate := range level { + if i, sf, ok := findDirectFieldByKeyType(candidate.typ, key); ok { found = append(found, struct { + index []int field reflect.StructField - owner reflect.Type - }{sf, ft}) + }{append(append([]int{}, candidate.index...), i), sf}) continue } - for _, deeper := range embeddedStructTypes(ft) { - if seen[deeper] { + for _, deeper := range embeddedIndexPaths(candidate.typ, candidate.index) { + if seen[deeper.typ] { continue } next = append(next, deeper) } } - for _, ft := range next { - seen[ft] = true + for _, candidate := range next { + seen[candidate.typ] = true } if len(found) == 1 { - return found[0].field, found[0].owner, true + return found[0].index, found[0].field, true } if len(found) > 1 { - // Ambiguous at this depth, which encoding/json resolves by omitting the field; see - // findStructFieldByKey in get.go. - return reflect.StructField{}, reflect.TypeOf(nil), false + return nil, reflect.StructField{}, false } level = next } - return reflect.StructField{}, reflect.TypeOf(nil), false + return nil, reflect.StructField{}, false +} + +// embeddedPath is an embedded struct type together with the index chain that reaches it. +type embeddedPath struct { + typ reflect.Type + index []int } -// findDirectFieldByKeyType matches key against the struct's own fields, by json tag name. -func findDirectFieldByKeyType(t reflect.Type, key string) (reflect.StructField, bool) { - for sf := range t.Fields() { +// embeddedIndexPaths returns the embeds of t that encoding/json flattens, each with the index +// chain from the root that reaches it. +func embeddedIndexPaths(t reflect.Type, prefix []int) []embeddedPath { + var out []embeddedPath + for i := range t.NumField() { + sf := t.Field(i) + if !isFlattenedEmbed(sf) { + continue + } + ft := sf.Type + for ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + if ft.Kind() != reflect.Struct { + continue + } + out = append(out, embeddedPath{typ: ft, index: append(append([]int{}, prefix...), i)}) + } + return out +} + +// ownerTypeAt returns the struct type that declares the field the index chain ends at. +func ownerTypeAt(t reflect.Type, index []int) reflect.Type { + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + for _, i := range index[:len(index)-1] { + t = t.Field(i).Type + for t.Kind() == reflect.Pointer { + t = t.Elem() + } + } + return t +} + +// findDirectFieldByKeyType matches key against the struct's own fields, by json tag name, and +// returns the field's index. +func findDirectFieldByKeyType(t reflect.Type, key string) (int, reflect.StructField, bool) { + for i := range t.NumField() { + sf := t.Field(i) if sf.PkgPath != "" { // unexported continue } @@ -214,25 +274,17 @@ func findDirectFieldByKeyType(t reflect.Type, key string) (reflect.StructField, if btag.Internal() || btag.ReadOnly() { continue } - return sf, true + return i, sf, true } - return reflect.StructField{}, false + return 0, reflect.StructField{}, false } -// embeddedStructTypes returns the anonymous struct fields of t, dereferenced. -func embeddedStructTypes(t reflect.Type) []reflect.Type { - var out []reflect.Type - for sf := range t.Fields() { - if !sf.Anonymous { - continue - } - ft := sf.Type - for ft.Kind() == reflect.Pointer { - ft = ft.Elem() - } - if ft.Kind() == reflect.Struct { - out = append(out, ft) - } +// isFlattenedEmbed reports whether the field is an embed encoding/json flattens into the +// outer object. An anonymous field that carries a json name is a named field instead: it +// serializes as a nested object under that name. +func isFlattenedEmbed(sf reflect.StructField) bool { + if !sf.Anonymous { + return false } - return out + return structtag.JSONTag(sf.Tag.Get("json")).Name() == "" } From f2ca668fd40f11200665293cc579eb82ee423961 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 1 Sep 2026 21:13:47 +0200 Subject: [PATCH 11/15] structwalk: flatten an embed only when encoding/json flattens it Both walks flattened every anonymous field. encoding/json flattens an embed only when its json tag gives no name: a name makes it an ordinary field serialized as a nested object. So a tagged embed was walked as though its fields were the outer struct's, and structdiff would have reported a change at a path the wire format does not have. The type walk had the inverse bug: it keyed the decision on the tag being non-empty, so `json:",omitempty"` on an embed -- a tag that sets an option and leaves the name empty -- made it a nested field, while encoding/json still flattens it. Both now use structaccess.IsFlattenedEmbed, so the three searches cannot drift again. No resource type has either shape today: the refschema golden is unchanged. Co-authored-by: Isaac --- libs/structs/structaccess/typecheck.go | 11 +++--- libs/structs/structwalk/walk.go | 6 ++- libs/structs/structwalk/walk_test.go | 53 ++++++++++++++++++++++++++ libs/structs/structwalk/walktype.go | 6 ++- 4 files changed, 67 insertions(+), 9 deletions(-) diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index 8e0727a284c..1b86f9fee9f 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -225,7 +225,7 @@ func embeddedIndexPaths(t reflect.Type, prefix []int) []embeddedPath { var out []embeddedPath for i := range t.NumField() { sf := t.Field(i) - if !isFlattenedEmbed(sf) { + if !IsFlattenedEmbed(sf) { continue } ft := sf.Type @@ -279,10 +279,11 @@ func findDirectFieldByKeyType(t reflect.Type, key string) (int, reflect.StructFi return 0, reflect.StructField{}, false } -// isFlattenedEmbed reports whether the field is an embed encoding/json flattens into the -// outer object. An anonymous field that carries a json name is a named field instead: it -// serializes as a nested object under that name. -func isFlattenedEmbed(sf reflect.StructField) bool { +// IsFlattenedEmbed reports whether the field is an embed encoding/json flattens into the +// outer object. An anonymous field that carries a json *name* is a named field instead: it +// serializes as a nested object under that name. The name is what matters, not the presence +// of a tag: `json:",omitempty"` leaves the name empty, so such a field is still flattened. +func IsFlattenedEmbed(sf reflect.StructField) bool { if !sf.Anonymous { return false } diff --git a/libs/structs/structwalk/walk.go b/libs/structs/structwalk/walk.go index 96a0cd1271a..32084458d2b 100644 --- a/libs/structs/structwalk/walk.go +++ b/libs/structs/structwalk/walk.go @@ -115,8 +115,10 @@ func walkStruct(path *structpath.PathNode, s reflect.Value, visit VisitFunc) { continue } - // Directly walk into embedded structs without adding the key to the path. - if sf.Anonymous { + // Directly walk into embedded structs without adding the key to the path. An anonymous + // field carrying a json name is not one of those: encoding/json serializes it as a + // nested object under that name, so it is walked as a named field below. + if structaccess.IsFlattenedEmbed(sf) { walkValue(path, s.Field(i), &sf, visit) continue } diff --git a/libs/structs/structwalk/walk_test.go b/libs/structs/structwalk/walk_test.go index aae419bfaee..fea7c86b0d4 100644 --- a/libs/structs/structwalk/walk_test.go +++ b/libs/structs/structwalk/walk_test.go @@ -1,7 +1,9 @@ package structwalk import ( + "encoding/json" "reflect" + "slices" "testing" "github.com/databricks/cli/libs/structs/structpath" @@ -276,3 +278,54 @@ func TestEmbeddedStructWithJSONTagDash(t *testing.T) { "parent_field": "parent", }, flatten(t, parent)) } + +// An anonymous field carrying a json name is a named field to encoding/json: it serializes as +// a nested object under that name rather than being flattened into the outer one. A field whose +// tag sets only an option, leaving the name empty, is still flattened. +type WalkEmbedLeaf struct { + Value string `json:"value,omitempty"` +} + +type walkTaggedEmbed struct { + WalkEmbedLeaf `json:"leaf"` + + Own string `json:"own,omitempty"` +} + +type walkOptionOnlyEmbed struct { + WalkEmbedLeaf `json:",omitempty"` + + Own string `json:"own,omitempty"` +} + +func TestWalkTaggedEmbedIsANamedField(t *testing.T) { + value := &walkTaggedEmbed{WalkEmbedLeaf: WalkEmbedLeaf{Value: "v"}, Own: "o"} + + blob, err := json.Marshal(value) + require.NoError(t, err) + assert.JSONEq(t, `{"leaf":{"value":"v"},"own":"o"}`, string(blob)) + + assert.Equal(t, []string{"leaf.value", "own"}, walkPaths(t, value)) +} + +func TestWalkOptionOnlyEmbedIsStillFlattened(t *testing.T) { + value := &walkOptionOnlyEmbed{WalkEmbedLeaf: WalkEmbedLeaf{Value: "v"}, Own: "o"} + + blob, err := json.Marshal(value) + require.NoError(t, err) + assert.JSONEq(t, `{"value":"v","own":"o"}`, string(blob)) + + assert.Equal(t, []string{"own", "value"}, walkPaths(t, value)) +} + +// walkPaths returns the paths Walk visits, sorted. +func walkPaths(t *testing.T, value any) []string { + t.Helper() + + var paths []string + require.NoError(t, Walk(value, func(path *structpath.PathNode, _ any, _ *reflect.StructField) { + paths = append(paths, path.String()) + })) + slices.Sort(paths) + return paths +} diff --git a/libs/structs/structwalk/walktype.go b/libs/structs/structwalk/walktype.go index cd4be28c843..f189f2d0251 100644 --- a/libs/structs/structwalk/walktype.go +++ b/libs/structs/structwalk/walktype.go @@ -109,9 +109,11 @@ func walkTypeStruct(path *structpath.PatternNode, st reflect.Type, visit VisitTy continue // unexported } - // Handle embedded structs (anonymous fields without json tags) + // Handle embedded structs that encoding/json flattens. The json *name* decides, not the + // presence of a tag: `json:",omitempty"` on an embed leaves the name empty and is still + // flattened, while a name makes it a nested object. jsonTag := sf.Tag.Get("json") - if sf.Anonymous && jsonTag == "" { + if structaccess.IsFlattenedEmbed(sf) { // For embedded structs, walk the embedded type at the current path level // This flattens the embedded struct's fields into the parent struct walkTypeValue(path, sf.Type, &sf, visit, visitedCount) From cf3d94b0586d2a27d2f6dc64f431e3dc3ea52e29 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 1 Sep 2026 21:18:05 +0200 Subject: [PATCH 12/15] structdiff: report a tagged embed's changes under its own name Same rule as structwalk: encoding/json flattens an embed only when its json tag gives no name. structdiff flattened every anonymous field, so a change inside a tagged embed was reported at the outer level -- a path the wire format does not have, which the direct engine would then put in an update mask. equal.go gets the same predicate, which also means a tagged embed with json:"-" is skipped rather than compared. Co-authored-by: Isaac --- libs/structs/structdiff/diff.go | 7 +++++-- libs/structs/structdiff/diff_test.go | 28 ++++++++++++++++++++++++++++ libs/structs/structdiff/equal.go | 6 ++++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/libs/structs/structdiff/diff.go b/libs/structs/structdiff/diff.go index d933ed5b3a9..c583d6d1759 100644 --- a/libs/structs/structdiff/diff.go +++ b/libs/structs/structdiff/diff.go @@ -209,8 +209,11 @@ func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Valu continue } - // Continue traversing embedded structs. Do not add the key to the path though. - if sf.Anonymous { + // Continue traversing embedded structs. Do not add the key to the path though. An + // anonymous field carrying a json name is not one of these: encoding/json serializes it + // as a nested object, so it is handled as a named field below and its changes are + // reported under that name. + if structaccess.IsFlattenedEmbed(sf) { if err := diffValues(ctx, path, s1.Field(i), s2.Field(i), changes); err != nil { return err } diff --git a/libs/structs/structdiff/diff_test.go b/libs/structs/structdiff/diff_test.go index 627169e718c..2839550c5a7 100644 --- a/libs/structs/structdiff/diff_test.go +++ b/libs/structs/structdiff/diff_test.go @@ -1,6 +1,7 @@ package structdiff import ( + "encoding/json" "reflect" "testing" "time" @@ -9,6 +10,7 @@ import ( sdktime "github.com/databricks/databricks-sdk-go/common/types/time" "github.com/databricks/databricks-sdk-go/service/jobs" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type B struct{ S string } @@ -923,3 +925,29 @@ func TestGetStructDiffSliceKeysDuplicates(t *testing.T) { }) } } + +// An anonymous field carrying a json name is a named field to encoding/json: it serializes as +// a nested object, so a change inside it belongs at that nested path, not at the outer level. +type DiffEmbedLeaf struct { + Value string `json:"value,omitempty"` +} + +type diffTaggedEmbed struct { + DiffEmbedLeaf `json:"leaf"` + + Own string `json:"own,omitempty"` +} + +func TestDiffTaggedEmbedIsReportedUnderItsName(t *testing.T) { + before := &diffTaggedEmbed{DiffEmbedLeaf: DiffEmbedLeaf{Value: "before"}, Own: "o"} + after := &diffTaggedEmbed{DiffEmbedLeaf: DiffEmbedLeaf{Value: "after"}, Own: "o"} + + blob, err := json.Marshal(after) + require.NoError(t, err) + assert.JSONEq(t, `{"leaf":{"value":"after"},"own":"o"}`, string(blob)) + + changes, err := GetStructDiff(before, after, nil) + require.NoError(t, err) + require.Len(t, changes, 1) + assert.Equal(t, "leaf.value", changes[0].Path.String()) +} diff --git a/libs/structs/structdiff/equal.go b/libs/structs/structdiff/equal.go index 6bf253c7317..bc75fed318f 100644 --- a/libs/structs/structdiff/equal.go +++ b/libs/structs/structdiff/equal.go @@ -4,6 +4,7 @@ import ( "reflect" "slices" + "github.com/databricks/cli/libs/structs/structaccess" "github.com/databricks/cli/libs/structs/structtag" ) @@ -107,8 +108,9 @@ func equalStruct(s1, s2 reflect.Value) bool { continue } - // Continue traversing embedded structs. - if sf.Anonymous { + // Continue traversing embedded structs. A tagged one is a named field to encoding/json, + // so it goes through the path below, which also honours json:"-". + if structaccess.IsFlattenedEmbed(sf) { if !equalValues(s1.Field(i), s2.Field(i)) { return false } From dacb1b2e4bdca5e44ed5c1c0fd78ceb360eb78ca Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 1 Sep 2026 21:48:24 +0200 Subject: [PATCH 13/15] structaccess: follow encoding/json's precedence rules for a field name Three more from the review, all in name resolution: At one depth, encoding/json prefers the field whose json tag names it over one that merely has the matching Go field name; only a genuine tie is ambiguous. The search counted both as matches and reported the name as not found, so a field the wire format does carry was unreachable. A field whose tag sets only an option -- `json:",omitempty"` -- has no json name, so encoding/json serializes it under its Go field name. The search matched tag names only, so such a field could not be resolved at all, while structwalk already emitted it under the Go name: the two disagreed about a field that is plainly on the wire. Only an anonymous *struct* is promoted. An embedded scalar, slice or interface is a member named after its type, but IsFlattenedEmbed called it an embed, so structwalk and structdiff placed its contents at the parent path. No resource type has any of these shapes: the refschema golden is unchanged. Co-authored-by: Isaac --- libs/structs/structaccess/set_test.go | 71 +++++++++++++++++++ libs/structs/structaccess/typecheck.go | 98 +++++++++++++++++++------- 2 files changed, 143 insertions(+), 26 deletions(-) diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 50ee4523506..6faa8a62279 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -1018,3 +1018,74 @@ func TestGetSet_TaggedEmbedIsANamedField(t *testing.T) { require.NoError(t, err) assert.JSONEq(t, `{"leaf":{"value":"set"},"own":"o"}`, string(blob)) } + +// At one depth, encoding/json prefers a field whose json tag names it over one that only has +// the matching Go field name, instead of calling the pair ambiguous. +type untaggedX struct { + X string +} + +type taggedAsX struct { + Y string `json:"X"` +} + +type taggedBeatsUntagged struct { + untaggedX + taggedAsX +} + +func TestGet_TaggedNameBeatsUntaggedAtTheSameDepth(t *testing.T) { + target := &taggedBeatsUntagged{untaggedX: untaggedX{X: "untagged"}, taggedAsX: taggedAsX{Y: "tagged"}} + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"X":"tagged"}`, string(blob)) + + value, err := structaccess.GetByString(target, "X") + require.NoError(t, err) + assert.Equal(t, "tagged", value, "must resolve to the field encoding/json serializes") +} + +// A field whose tag sets only an option has no json name, so encoding/json serializes it under +// its Go field name and that is the name it has to be reachable by. +type optionOnlyTag struct { + Count int `json:"count,omitempty"` + Total int `json:",omitempty"` +} + +func TestGetSet_FieldWithoutATagNameUsesItsGoName(t *testing.T) { + target := &optionOnlyTag{Count: 1, Total: 2} + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"count":1,"Total":2}`, string(blob)) + + value, err := structaccess.GetByString(target, "Total") + require.NoError(t, err) + assert.Equal(t, 2, value) + + require.NoError(t, structaccess.SetByString(target, "Total", 7)) + assert.Equal(t, 7, target.Total) +} + +// An anonymous field that is not a struct is not promoted: encoding/json serializes it as a +// member named after its type. +type EmbeddedName string + +type embeddedScalar struct { + EmbeddedName + + Own string `json:"own,omitempty"` +} + +func TestGet_AnonymousNonStructIsANamedMember(t *testing.T) { + target := &embeddedScalar{EmbeddedName: "n", Own: "o"} + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"EmbeddedName":"n","own":"o"}`, string(blob)) + + value, err := structaccess.GetByString(target, "EmbeddedName") + require.NoError(t, err) + assert.Equal(t, EmbeddedName("n"), value) +} diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index 1b86f9fee9f..c0244b74645 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -168,8 +168,8 @@ func findFieldIndexByKeyType(t reflect.Type, key string) ([]int, reflect.StructF return nil, reflect.StructField{}, false } - if i, sf, ok := findDirectFieldByKeyType(t, key); ok { - return []int{i}, sf, true + if c, ok := pickCandidate(directCandidates(t, key, nil)); ok { + return c.index, c.field, true } // A cycle must not be walked twice, or a key the type never declares sends the search @@ -179,32 +179,27 @@ func findFieldIndexByKeyType(t reflect.Type, key string) ([]int, reflect.StructF level := embeddedIndexPaths(t, nil) for len(level) > 0 { var next []embeddedPath - var found []struct { - index []int - field reflect.StructField - } - for _, candidate := range level { - if i, sf, ok := findDirectFieldByKeyType(candidate.typ, key); ok { - found = append(found, struct { - index []int - field reflect.StructField - }{append(append([]int{}, candidate.index...), i), sf}) + var found []candidate + for _, embed := range level { + matches := directCandidates(embed.typ, key, embed.index) + if len(matches) > 0 { + found = append(found, matches...) continue } - for _, deeper := range embeddedIndexPaths(candidate.typ, candidate.index) { + for _, deeper := range embeddedIndexPaths(embed.typ, embed.index) { if seen[deeper.typ] { continue } next = append(next, deeper) } } - for _, candidate := range next { - seen[candidate.typ] = true - } - if len(found) == 1 { - return found[0].index, found[0].field, true + for _, embed := range next { + seen[embed.typ] = true } - if len(found) > 1 { + if len(found) > 0 { + if c, ok := pickCandidate(found); ok { + return c.index, c.field, true + } return nil, reflect.StructField{}, false } level = next @@ -254,18 +249,37 @@ func ownerTypeAt(t reflect.Type, index []int) reflect.Type { return t } -// findDirectFieldByKeyType matches key against the struct's own fields, by json tag name, and -// returns the field's index. -func findDirectFieldByKeyType(t reflect.Type, key string) (int, reflect.StructField, bool) { +// candidate is a field that matches a json name, with the index chain reaching it and whether +// the name came from a json tag. encoding/json prefers a tagged name over an untagged one at +// the same depth, so the distinction has to survive the search. +type candidate struct { + index []int + field reflect.StructField + tagged bool +} + +// directCandidates returns the struct's own fields that key can name. A field with a json tag +// name is matched on that; a field without one is matched on its Go field name, which is what +// encoding/json serializes it under. An embed that encoding/json flattens is not addressable by +// name at all, so it is not a candidate. +func directCandidates(t reflect.Type, key string, prefix []int) []candidate { + var out []candidate for i := range t.NumField() { sf := t.Field(i) if sf.PkgPath != "" { // unexported continue } + if sf.Name == EmbeddedSliceFieldName || IsFlattenedEmbed(sf) { + continue + } name := structtag.JSONTag(sf.Tag.Get("json")).Name() - if name == "-" || sf.Name == EmbeddedSliceFieldName { + if name == "-" { continue } + tagged := name != "" + if !tagged { + name = sf.Name + } if name != key { continue } @@ -274,9 +288,32 @@ func findDirectFieldByKeyType(t reflect.Type, key string) (int, reflect.StructFi if btag.Internal() || btag.ReadOnly() { continue } - return i, sf, true + out = append(out, candidate{ + index: append(append([]int{}, prefix...), i), + field: sf, + tagged: tagged, + }) + } + return out +} + +// pickCandidate applies encoding/json's precedence among fields that share a name at one +// depth: a single tagged name wins over untagged ones, a single match of either kind wins, and +// anything else is ambiguous and serialized as nothing. +func pickCandidate(candidates []candidate) (candidate, bool) { + if len(candidates) == 1 { + return candidates[0], true + } + var tagged []candidate + for _, c := range candidates { + if c.tagged { + tagged = append(tagged, c) + } } - return 0, reflect.StructField{}, false + if len(tagged) == 1 { + return tagged[0], true + } + return candidate{}, false } // IsFlattenedEmbed reports whether the field is an embed encoding/json flattens into the @@ -287,5 +324,14 @@ func IsFlattenedEmbed(sf reflect.StructField) bool { if !sf.Anonymous { return false } - return structtag.JSONTag(sf.Tag.Get("json")).Name() == "" + if structtag.JSONTag(sf.Tag.Get("json")).Name() != "" { + return false + } + // Only an anonymous *struct* is promoted. An embedded scalar, slice or interface is a member + // named after its type, so it belongs at its own path rather than the parent's. + ft := sf.Type + for ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + return ft.Kind() == reflect.Struct } From 2154ddf919a50d7aa6e76a0146b755cc336ef5c7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 1 Sep 2026 22:06:04 +0200 Subject: [PATCH 14/15] structaccess: descend into a repeated embedded type once, as encoding/json does encoding/json walks an embedded type once per level however many members reach it, so a name declared *below* a type reached by two routes is not ambiguous -- it resolves along the first route. A name the duplicated type declares itself is ambiguous, and json serializes neither. The search treated every route as independent, so it called the first case ambiguous and reported a name as not found that the wire format does carry. Verified against json rather than reasoned about: a diamond over the declaring type marshals to {}, while a diamond one level above it marshals to {"value":"left"}. The internal/readonly skip keeps its existing behaviour, with a comment recording that it diverges from encoding/json: such a field shadows a same-named field further down, so skipping it lets the deeper one win. resources.App is the live example. Rejecting the name outright instead would make ${resources.apps.*.url} unresolvable, so that is a decision about what internal means rather than a fix to make here. Co-authored-by: Isaac --- libs/structs/structaccess/set_test.go | 38 +++++++++++++++++++ libs/structs/structaccess/typecheck.go | 52 +++++++++++++++++++++----- 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index 6faa8a62279..ac0c061d3dd 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -1089,3 +1089,41 @@ func TestGet_AnonymousNonStructIsANamedMember(t *testing.T) { require.NoError(t, err) assert.Equal(t, EmbeddedName("n"), value) } + +// The same embedded type reached by two routes: encoding/json descends into it once, so a name +// declared *below* it is not ambiguous, while a name the duplicated type declares itself is. +type repeatedLeaf struct { + Value string `json:"value,omitempty"` +} + +type repeatedMiddle struct { + repeatedLeaf +} + +type repeatedLeft struct { + repeatedMiddle +} + +type repeatedRight struct { + repeatedMiddle +} + +type repeatedEmbed struct { + repeatedLeft + repeatedRight +} + +func TestGet_TypeReachedTwiceIsNotAmbiguousBelowIt(t *testing.T) { + target := &repeatedEmbed{ + repeatedLeft: repeatedLeft{repeatedMiddle: repeatedMiddle{repeatedLeaf: repeatedLeaf{Value: "left"}}}, + repeatedRight: repeatedRight{repeatedMiddle: repeatedMiddle{repeatedLeaf: repeatedLeaf{Value: "right"}}}, + } + + blob, err := json.Marshal(target) + require.NoError(t, err) + assert.JSONEq(t, `{"value":"left"}`, string(blob), "encoding/json takes the first route") + + value, err := structaccess.GetByString(target, "value") + require.NoError(t, err) + assert.Equal(t, "left", value) +} diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index c0244b74645..924b17d5451 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -173,10 +173,9 @@ func findFieldIndexByKeyType(t reflect.Type, key string) ([]int, reflect.StructF } // A cycle must not be walked twice, or a key the type never declares sends the search - // round forever. Only types from *earlier* levels are excluded: a type reachable twice - // within one level is a diamond, and its two matches are the ambiguity json omits. + // round forever. seen := map[reflect.Type]bool{t: true} - level := embeddedIndexPaths(t, nil) + level := dedupeByType(embeddedIndexPaths(t, nil)) for len(level) > 0 { var next []embeddedPath var found []candidate @@ -184,6 +183,12 @@ func findFieldIndexByKeyType(t reflect.Type, key string) ([]int, reflect.StructF matches := directCandidates(embed.typ, key, embed.index) if len(matches) > 0 { found = append(found, matches...) + if embed.reached > 1 { + // Several members of the previous level reach this type, so encoding/json sees + // the names it declares once per route and annihilates them. One extra match is + // enough to make the name ambiguous below. + found = append(found, matches[0]) + } continue } for _, deeper := range embeddedIndexPaths(embed.typ, embed.index) { @@ -193,7 +198,8 @@ func findFieldIndexByKeyType(t reflect.Type, key string) ([]int, reflect.StructF next = append(next, deeper) } } - for _, embed := range next { + level = dedupeByType(next) + for _, embed := range level { seen[embed.typ] = true } if len(found) > 0 { @@ -202,16 +208,36 @@ func findFieldIndexByKeyType(t reflect.Type, key string) ([]int, reflect.StructF } return nil, reflect.StructField{}, false } - level = next } return nil, reflect.StructField{}, false } -// embeddedPath is an embedded struct type together with the index chain that reaches it. +// embeddedPath is an embedded struct type together with the index chain that reaches it, and +// how many members of the previous level reach it. type embeddedPath struct { - typ reflect.Type - index []int + typ reflect.Type + index []int + reached int +} + +// dedupeByType collapses repeated embeds of one type into a single entry, counting how many +// routes reached it. encoding/json descends into a type once per level however many members +// embed it, so a name declared *below* a type reached twice is not ambiguous; a name the +// duplicated type declares itself is, and the count records that. +func dedupeByType(paths []embeddedPath) []embeddedPath { + var out []embeddedPath + index := map[reflect.Type]int{} + for _, path := range paths { + if at, ok := index[path.typ]; ok { + out[at].reached++ + continue + } + index[path.typ] = len(out) + path.reached = 1 + out = append(out, path) + } + return out } // embeddedIndexPaths returns the embeds of t that encoding/json flattens, each with the index @@ -283,7 +309,15 @@ func directCandidates(t reflect.Type, key string, prefix []int) []candidate { if name != key { continue } - // Skip fields marked as internal/readonly + // Skip fields marked as internal/readonly. + // + // Known divergence from encoding/json: such a field still shadows a same-named field + // further down, so dropping it here lets the deeper one win and a caller can reach a + // field the wire format does not carry. resources.App is the live example -- its + // BaseResource.URL is internal and shadows the SDK's url, which json serializes as the + // internal one. Rejecting the name outright instead would make + // ${resources.apps.*.url} unresolvable, so which of the two is right is a decision + // about what internal means, not a detail of the search. btag := structtag.BundleTag(sf.Tag.Get("bundle")) if btag.Internal() || btag.ReadOnly() { continue From 1f0aad92bdc653d9f1fb6e8bec69b206957224c8 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 1 Sep 2026 22:20:21 +0200 Subject: [PATCH 15/15] structaccess: pin the repeated-embed matrix against encoding/json The fifth review round claimed a remaining tagged/untagged bug for a repeated embedded type. It does not reproduce: the counterexample used omitempty fields left at their zero value, so "omitted because empty" was indistinguishable from "not serialized at all". With values populated, all four combinations agree. They are worth keeping, since the reasoning is easy to get wrong in either direction, so the table asserts each against encoding/json rather than against a hand-written expectation: a repeated type declaring the name itself is annihilated, its tagged name losing to a sibling's untagged X under "X", a repeated untagged name not colliding with the tagged one at all, and the shallower of two routes winning. Co-authored-by: Isaac --- libs/structs/structaccess/set_test.go | 96 +++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/libs/structs/structaccess/set_test.go b/libs/structs/structaccess/set_test.go index ac0c061d3dd..3d9603a3831 100644 --- a/libs/structs/structaccess/set_test.go +++ b/libs/structs/structaccess/set_test.go @@ -1127,3 +1127,99 @@ func TestGet_TypeReachedTwiceIsNotAmbiguousBelowIt(t *testing.T) { require.NoError(t, err) assert.Equal(t, "left", value) } + +// Combinations of a repeated embedded type with tagged and untagged declarations of one name. +// encoding/json is the oracle for each: the assertion compares what it serializes under the +// name with what Get resolves, so the pair cannot drift. +type matrixTagged struct { + Y string `json:"x"` +} + +type matrixUntagged struct { + X string +} + +type ( + matrixLeftTagged struct{ matrixTagged } + matrixRightTagged struct{ matrixTagged } + matrixLeftUntagged struct{ matrixUntagged } + matrixRightUntagged struct{ matrixUntagged } +) + +// The repeated type declares the name itself: two routes, so encoding/json annihilates it. +type matrixRepeatDeclares struct { + matrixLeftTagged + matrixRightTagged +} + +// The repeated type's tagged name is annihilated, leaving a sibling's untagged X under "X". +type matrixRepeatTaggedPlusUntagged struct { + matrixLeftTagged + matrixRightTagged + matrixUntagged +} + +// The repeated type's untagged name never collides with "x"; the sibling's tagged one wins. +type matrixRepeatUntaggedPlusTagged struct { + matrixLeftUntagged + matrixRightUntagged + matrixTagged +} + +type ( + matrixDeepHolder struct{ matrixTagged } + matrixDeeperRoute struct{ matrixDeepHolder } +) + +// One route reaches the declaring type a level earlier than the other: the shallower wins. +type matrixMixedDepth struct { + matrixTagged + matrixDeeperRoute +} + +func TestGet_RepeatedEmbedMatrixMatchesEncodingJSON(t *testing.T) { + repeatDeclares := &matrixRepeatDeclares{} + repeatDeclares.matrixLeftTagged.Y = "L" + repeatDeclares.matrixRightTagged.Y = "R" + + taggedPlusUntagged := &matrixRepeatTaggedPlusUntagged{} + taggedPlusUntagged.matrixLeftTagged.Y = "L" + taggedPlusUntagged.matrixRightTagged.Y = "R" + taggedPlusUntagged.X = "U" + + untaggedPlusTagged := &matrixRepeatUntaggedPlusTagged{} + untaggedPlusTagged.matrixLeftUntagged.X = "L" + untaggedPlusTagged.matrixRightUntagged.X = "R" + untaggedPlusTagged.Y = "T" + + mixedDepth := &matrixMixedDepth{} + mixedDepth.Y = "shallow" + mixedDepth.Y = "deep" + + for _, tc := range []struct { + name string + value any + }{ + {"repeated type declares the name", repeatDeclares}, + {"repeated tagged plus untagged sibling", taggedPlusUntagged}, + {"repeated untagged plus tagged sibling", untaggedPlusTagged}, + {"one route shallower than the other", mixedDepth}, + } { + t.Run(tc.name, func(t *testing.T) { + blob, err := json.Marshal(tc.value) + require.NoError(t, err) + var emitted map[string]any + require.NoError(t, json.Unmarshal(blob, &emitted)) + + want, onTheWire := emitted["x"] + got, err := structaccess.GetByString(tc.value, "x") + + if !onTheWire { + require.Error(t, err, "encoding/json emitted %s, so there is no x to read", blob) + return + } + require.NoError(t, err) + assert.Equal(t, want, got, "encoding/json emitted %s", blob) + }) + } +}