Trim drift walker and its tests

Simplify cycle detection: stop on first revisit instead of allowing one
self-referential expansion. The "expand once" gold-plating produced a
"next.<field>" entry on linked-list-shaped types, but no real CRD or
runtime config has cyclic shape, and stop-on-revisit is the simpler
contract for drift detection. The visited tracker becomes a
map[reflect.Type]struct{} and maxStructRevisits goes away.

Compress the FlattenJSONLeafFields godoc from a 9-bullet semantics list
into one paragraph. Most of the bullets are now subsumed by the single
"json.Marshaler => leaf, otherwise recurse on Struct/Slice/Array/Map"
rule. The detailed encoding/json reference belongs in the standard
library, not here.

Drop redundant subtests in TestFlattenJSONLeafFields. After the
json.Marshaler refactor, the json.RawMessage / thvjson.Map+Any /
vmcpconfig.Duration cases all exercise the same short-circuit branch as
metav1.Duration, so one Marshaler example is enough. Drop slice-of-
pointer-to-struct (covered by combining the pointer-deref and slice-of-
struct cases). Fold the dedicated PointerInputDereferenced test into the
table. Remove TestFlattenJSONLeafFields_OutputIsSorted: the table-
driven test already pins exact sorted-string equality on every case.

Update the recursive-type expectation to ["name"] to lock in the new
stop-on-revisit semantics.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: ChrisJBurns

cmd/thv-operator/internal/testutil/reflect.go

@@ -17,35 +17,12 @@ import (
 // FlattenJSONLeafFields returns every leaf JSON field path under t as a sorted
 // slice of dot-delimited paths (e.g. "openTelemetry.tracing.enabled").
 //
-// Walking semantics:
-//   - For struct fields, the JSON tag's name (before any comma) is used as the
-//     path segment. If the tag is "-", the field is skipped. If the tag is
-//     missing, the Go field name is used.
-//   - Unexported fields are skipped.
-//   - Fields with `,inline` flatten into the parent path with no prefix.
-//   - Pointer types are dereferenced and walked.
-//   - Struct types are recursed into.
-//   - Slice and array types whose ELEMENT type is a struct (or pointer to a
-//     struct) recurse into that element type, joining with the parent path.
-//     A path like "tools.workload" therefore means "every element of the
-//     `tools` slice has a `workload` field".
-//   - Map types whose VALUE type is a struct (or pointer to a struct) recurse
-//     into that value type the same way.
-//   - Types that implement json.Marshaler are treated as leaves regardless of
-//     their underlying kind. A custom MarshalJSON produces a JSON shape that
-//     bears no relation to the Go field layout, so walking the fields would
-//     emit paths that never appear in the serialized form. This handles
-//     metav1.Time, metav1.Duration, intstr.IntOrString, resource.Quantity,
-//     json.RawMessage, and any project-local custom-marshaled types
-//     (e.g. pkg/json.Map, pkg/json.Any, pkg/vmcp/config.Duration) without an
-//     explicit allow-list.
-//   - All other primitive types (primitives, []string, map[string]string,
-//     time.Duration which is just int64) terminate the walk and contribute
-//     one leaf path entry via the default branch.
-//   - Self-referential types are expanded one level (the original visit plus
-//     one self-reference) and then truncated, to keep the walk terminating.
-//     Subsequent levels (e.g. "next.next.<field>") are intentionally elided.
-//     See maxStructRevisits for the controlling constant.
+// A leaf is any type that does not produce nested JSON keys at runtime: a
+// primitive, a slice/map of primitives, or a type implementing json.Marshaler
+// (whose MarshalJSON shape is opaque to reflection). Structs, slices/maps of
+// structs, and pointers to either are recursed into. Field names follow
+// encoding/json rules including `,inline` and anonymous-field promotion.
+// Self-referential types stop on revisit so the walk always terminates.
 //
 // If t is nil or, after dereferencing, is not a struct, an empty slice is
 // returned rather than panicking.
@@ -61,7 +38,7 @@ func FlattenJSONLeafFields(t reflect.Type) []string {
 	}
 
 	leafSet := make(map[string]struct{})
-	visited := map[reflect.Type]int{}
+	visited := map[reflect.Type]struct{}{}
 	recurseStruct(t, "", leafSet, visited)
 
 	out := make([]string, 0, len(leafSet))
@@ -96,7 +73,7 @@ var skipFieldTypes = map[reflect.Type]struct{}{
 // visited is the set of struct types currently on the recursion stack; it is
 // used to break cycles on self-referential types. Callers add t to visited
 // before invoking this function and remove it afterwards.
-func walkStruct(t reflect.Type, prefix string, leafSet map[string]struct{}, visited map[reflect.Type]int) {
+func walkStruct(t reflect.Type, prefix string, leafSet map[string]struct{}, visited map[reflect.Type]struct{}) {
 	for i := 0; i < t.NumField(); i++ {
 		field := t.Field(i)
 		// Skip unexported fields. PkgPath is non-empty for unexported fields.
@@ -140,7 +117,7 @@ func walkStruct(t reflect.Type, prefix string, leafSet map[string]struct{}, visi
 
 // walkType walks a single type with the given accumulated prefix. It either
 // recurses into nested structs/slices/maps or records a leaf at prefix.
-func walkType(t reflect.Type, prefix string, leafSet map[string]struct{}, visited map[reflect.Type]int) {
+func walkType(t reflect.Type, prefix string, leafSet map[string]struct{}, visited map[reflect.Type]struct{}) {
 	// Deref pointers.
 	for t.Kind() == reflect.Pointer {
 		t = t.Elem()
@@ -185,25 +162,16 @@ func walkType(t reflect.Type, prefix string, leafSet map[string]struct{}, visite
 	}
 }
 
-// maxStructRevisits caps how many times a single struct type may appear on the
-// recursion stack. A value of 2 means the type may be entered at most twice
-// on the same path: the original entry plus one self-referential visit. This
-// is enough for one level of "next.<field>" expansion in cyclic types like
-// linked lists, while still guaranteeing the walk terminates.
-const maxStructRevisits = 2
-
 // recurseStruct descends into a nested struct type with cycle protection.
-// visited holds the count of how many times each struct type currently appears
-// on the recursion stack. When entering t would exceed maxStructRevisits, the
-// walk stops silently — no leaf is recorded, on the assumption that the
-// useful leaves under t have already been captured by the earlier visit on
-// the same path.
-func recurseStruct(t reflect.Type, prefix string, leafSet map[string]struct{}, visited map[reflect.Type]int) {
-	if visited[t] >= maxStructRevisits {
+// visited is the set of struct types currently on the recursion stack; if t
+// is already present, the walk stops silently to keep self-referential types
+// from looping forever.
+func recurseStruct(t reflect.Type, prefix string, leafSet map[string]struct{}, visited map[reflect.Type]struct{}) {
+	if _, seen := visited[t]; seen {
 		return
 	}
-	visited[t]++
-	defer func() { visited[t]-- }()
+	visited[t] = struct{}{}
+	defer delete(visited, t)
 	walkStruct(t, prefix, leafSet, visited)
 }
 

cmd/thv-operator/internal/testutil/reflect_test.go

@@ -4,18 +4,13 @@
 package testutil
 
 import (
-	"encoding/json"
 	"reflect"
-	"sort"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	thvjson "github.com/stacklok/toolhive/pkg/json"
-	vmcpconfig "github.com/stacklok/toolhive/pkg/vmcp/config"
 )
 
 // --- Fixture types ---------------------------------------------------------
@@ -52,10 +47,6 @@ type withSliceOfStruct struct {
 	Items []leafInner `json:"items"`
 }
 
-type withSliceOfPointerStruct struct {
-	Items []*leafInner `json:"items"`
-}
-
 type withSliceOfPrimitive struct {
 	Tags []string `json:"tags"`
 }
@@ -90,24 +81,14 @@ type withUnexportedField struct {
 	hidden  string //nolint:unused // exercised by reflection
 }
 
+// withDuration covers two leaf paths in one fixture: a primitive int64
+// (time.Duration, default branch) and a json.Marshaler (metav1.Duration,
+// short-circuit branch). Other Marshaler types follow the same code path.
 type withDuration struct {
 	Wait     time.Duration   `json:"wait"`
 	WaitMeta metav1.Duration `json:"waitMeta"`
 }
 
-type withRawJSON struct {
-	Raw json.RawMessage `json:"raw"`
-}
-
-type withThvJSON struct {
-	M thvjson.Map `json:"m"`
-	A thvjson.Any `json:"a"`
-}
-
-type withVmcpDuration struct {
-	D vmcpconfig.Duration `json:"d"`
-}
-
 type recursive struct {
 	Name string     `json:"name"`
 	Next *recursive `json:"next,omitempty"`
@@ -153,11 +134,6 @@ func TestFlattenJSONLeafFields(t *testing.T) {
 			in:   withSliceOfStruct{},
 			want: []string{"items.a", "items.b"},
 		},
-		{
-			name: "slice of pointer to struct recurses into element",
-			in:   withSliceOfPointerStruct{},
-			want: []string{"items.a", "items.b"},
-		},
 		{
 			name: "slice of primitive terminates at slice",
 			in:   withSliceOfPrimitive{},
@@ -189,29 +165,22 @@ func TestFlattenJSONLeafFields(t *testing.T) {
 			want: []string{"visible"},
 		},
 		{
-			name: "time.Duration and metav1.Duration are leaves",
+			// time.Duration is a primitive int64 (default branch);
+			// metav1.Duration is a json.Marshaler (short-circuit branch).
+			// Together these exercise both leaf-emission paths.
+			name: "primitive and json.Marshaler types are leaves",
 			in:   withDuration{},
 			want: []string{"wait", "waitMeta"},
 		},
 		{
-			name: "json.RawMessage is a leaf",
-			in:   withRawJSON{},
-			want: []string{"raw"},
-		},
-		{
-			name: "thvjson Map and Any are leaves",
-			in:   withThvJSON{},
-			want: []string{"a", "m"},
-		},
-		{
-			name: "vmcp config Duration is a leaf",
-			in:   withVmcpDuration{},
-			want: []string{"d"},
+			name: "self-referential type stops on revisit",
+			in:   recursive{},
+			want: []string{"name"},
 		},
 		{
-			name: "self-referential type does not recurse infinitely",
-			in:   recursive{},
-			want: []string{"name", "next.name"},
+			name: "pointer input is dereferenced",
+			in:   &flatPrimitives{},
+			want: []string{"count", "enabled", "name"},
 		},
 	}
 
@@ -224,21 +193,6 @@ func TestFlattenJSONLeafFields(t *testing.T) {
 	}
 }
 
-func TestFlattenJSONLeafFields_PointerInputDereferenced(t *testing.T) {
-	t.Parallel()
-
-	got := FlattenJSONLeafFields(reflect.TypeOf(&flatPrimitives{}))
-	require.Equal(t, []string{"count", "enabled", "name"}, got)
-}
-
-func TestFlattenJSONLeafFields_OutputIsSorted(t *testing.T) {
-	t.Parallel()
-
-	got := FlattenJSONLeafFields(reflect.TypeOf(flatPrimitives{}))
-	require.NotEmpty(t, got)
-	require.True(t, sort.StringsAreSorted(got), "expected sorted output, got %v", got)
-}
-
 func TestFlattenJSONLeafFields_NilOrNonStructReturnsEmpty(t *testing.T) {
 	t.Parallel()