fix(env): compose per-file selectors across profile includes

pkg/env/select.go

@@ -148,6 +148,16 @@ func mergeYAMLTopLevel(a, b []byte) ([]byte, error) {
 		bKey := bRoot.Content[i]
 		bVal := bRoot.Content[i+1]
 		if idx := findYAMLTopLevelKey(aRoot, bKey.Value); idx >= 0 {
+			// Both sides selected the same top-level key — a simple dot-path
+			// on one side and a JMESPath on the other. Merge them instead of
+			// letting the JMESPath side win: replacing dropped everything the
+			// simple selector asked for, the same collapse that lost base
+			// profiles' binaries in composed profiles. Recursing also keeps
+			// a's comments on the keys b does not touch.
+			if aRoot.Content[idx+1].Kind == yaml.MappingNode && bVal.Kind == yaml.MappingNode {
+				mergeYAMLMappings(aRoot.Content[idx+1], bVal)
+				continue
+			}
 			aRoot.Content[idx] = bKey
 			aRoot.Content[idx+1] = bVal
 			continue
@@ -443,6 +453,10 @@ func mergeYAMLMappings(dst, src *yaml.Node) {
 				found = true
 				if dst.Content[j+1].Kind == yaml.MappingNode && srcVal.Kind == yaml.MappingNode {
 					mergeYAMLMappings(dst.Content[j+1], srcVal)
+				} else {
+					// Not both mappings — there is nothing to compose, so the
+					// documented precedence applies: the JMESPath result wins.
+					dst.Content[j+1] = srcVal
 				}
 				break
 			}

pkg/env/select_jmespath.go

@@ -3,6 +3,7 @@ package env
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"strings"
 
 	"github.com/jmespath-community/go-jmespath"
@@ -109,11 +110,15 @@ func splitSelectorsByComplexity(selectors []string) (simple, complex []string) {
 // Merge semantics:
 //
 //   - If an expression returns a map[string]interface{}, its entries are
-//     merged into the result (later expressions override earlier ones for
-//     the same key).
+//     merged into the result. Several expressions contributing to the SAME key
+//     union rather than overwrite: nested maps merge key-by-key, lists union
+//     (deduplicated), and only scalars replace. This is what lets a resolved
+//     profile chain select several slices of one file.
 //
 //   - If an expression returns something else (scalar, array), it is
-//     wrapped under a key chosen by `wrapKeyFor`. The key is selected
+//     wrapped under a key chosen by `wrapKeyFor` — and unioned the same way,
+//     since wrapKeyFor deliberately sends every `binaries[?…]` variant to
+//     `binaries`. The key is selected
 //     by a small fallback chain: a leading identifier followed by
 //     JMESPath grammar (e.g. `binaries[?...]` → `binaries`), the
 //     trailing identifier of a simple dot-path (`database.host` →
@@ -149,17 +154,85 @@ func runJMESPathSelectors(
 			continue
 		}
 		if m, ok := val.(map[string]interface{}); ok {
-			for k, v := range m {
-				merged[k] = v
-			}
+			mergeMapInto(merged, m)
 			continue
 		}
-		// Non-map result: wrap under a sensible key.
-		merged[wrapKeyFor(sel)] = val
+		// Non-map result: wrap under a sensible key — through the same
+		// union, since wrapKeyFor deliberately maps every `binaries[?…]`
+		// variant to `binaries`, so a profile chain of bare filters lands
+		// several lists on one key.
+		mergeValueInto(merged, wrapKeyFor(sel), val)
 	}
 	return marshal(merged)
 }
 
+// mergeMapInto deep-merges src into dst, key by key.
+func mergeMapInto(dst, src map[string]interface{}) {
+	for k, v := range src {
+		mergeValueInto(dst, k, v)
+	}
+}
+
+// mergeValueInto unions v into dst[k]. Nested maps merge key-by-key, lists
+// union, and anything else is replaced. Several selectors contributing to the
+// SAME key is the normal shape once a profile chain is resolved — each
+// projects a different slice of one file — and a plain assignment kept only
+// the last one, which is how core binaries went missing from every composed
+// profile.
+func mergeValueInto(dst map[string]interface{}, k string, v interface{}) {
+	existing, present := dst[k]
+	if !present {
+		dst[k] = v
+		return
+	}
+	switch src := v.(type) {
+	case []interface{}:
+		if cur, ok := existing.([]interface{}); ok {
+			dst[k] = unionLists(cur, src)
+			return
+		}
+	case map[string]interface{}:
+		if cur, ok := existing.(map[string]interface{}); ok {
+			// Copy before descending: cur may alias the parsed document, and
+			// mutating it in place would corrupt what later selectors search.
+			clone := make(map[string]interface{}, len(cur)+len(src))
+			for ck, cv := range cur {
+				clone[ck] = cv
+			}
+			mergeMapInto(clone, src)
+			dst[k] = clone
+			return
+		}
+	}
+	dst[k] = v
+}
+
+// unionLists appends the elements of b that a does not already hold.
+// Deduplicating matters: an entry matching two selectors is projected twice
+// with the same value, and plain concatenation turned `groups: [local, core]`
+// into `[local, core, local, core]`. The result is always a fresh slice —
+// appending in place could write into the parsed document's backing array.
+func unionLists(a, b []interface{}) []interface{} {
+	out := make([]interface{}, 0, len(a)+len(b))
+	out = append(out, a...)
+	for _, item := range b {
+		if !containsValue(out, item) {
+			out = append(out, item)
+		}
+	}
+	return out
+}
+
+// containsValue reports whether list already holds an element deep-equal to v.
+func containsValue(list []interface{}, v interface{}) bool {
+	for _, item := range list {
+		if reflect.DeepEqual(item, v) {
+			return true
+		}
+	}
+	return false
+}
+
 // wrapKeyFor picks a top-level key under which to place a non-map
 // JMESPath result. The fallback chain is:
 //

pkg/env/select_jmespath_test.go

@@ -1,8 +1,11 @@
 package env
 
 import (
+	"sort"
 	"strings"
 	"testing"
+
+	"gopkg.in/yaml.v3"
 )
 
 // --- classifier ---
@@ -419,3 +422,131 @@ func TestWrapKeyFor(t *testing.T) {
 		}
 	}
 }
+
+func TestFilterYAMLJMESPath_SameKeyFromTwoSelectorsUnions(t *testing.T) {
+	// A resolved profile chain hands us one selector per group, each projecting
+	// the same top-level `binaries` key. A shallow merge kept only the last
+	// group's map, which is how core tools went missing from every composed
+	// profile.
+	content := []byte(`binaries:
+  argsh:
+    groups: [core]
+  kubectl:
+    groups: [core]
+  kustomize:
+    groups: [kustomize]
+  kind:
+    groups: [local]
+`)
+	selectors := []string{
+		"{binaries: from_items(items(binaries)[?[1].groups && contains([1].groups, 'core')])}",
+		"{binaries: from_items(items(binaries)[?[1].groups && contains([1].groups, 'kustomize')])}",
+	}
+
+	out, err := filterYAMLJMESPath(content, selectors)
+	if err != nil {
+		t.Fatalf("filter: %v", err)
+	}
+
+	var got struct {
+		Binaries map[string]interface{} `yaml:"binaries"`
+	}
+	if err := yaml.Unmarshal(out, &got); err != nil {
+		t.Fatalf("unmarshal %s: %v", out, err)
+	}
+	for _, want := range []string{"argsh", "kubectl", "kustomize"} {
+		if _, ok := got.Binaries[want]; !ok {
+			t.Errorf("binaries.%s missing — got %v (both selectors' results must survive)", want, keysOf(got.Binaries))
+		}
+	}
+	if _, ok := got.Binaries["kind"]; ok {
+		t.Error("binaries.kind leaked in — no selector asked for the local group")
+	}
+}
+
+func keysOf(m map[string]interface{}) []string {
+	out := make([]string, 0, len(m))
+	for k := range m {
+		out = append(out, k)
+	}
+	sort.Strings(out)
+	return out
+}
+
+// The seam that matters is filterContent: it routes selectors to the simple
+// Node path, the JMESPath path, or both, and then merges. Testing the
+// individual filters misses collapses that only the merge introduces.
+func TestFilterContent_SimpleAndComplexSelectorsBothSurvive(t *testing.T) {
+	content := []byte(`binaries:
+  argsh:
+    groups: [core]
+  kustomize:
+    groups: [kustomize]
+  kind:
+    groups: [local]
+`)
+	selectors := []string{
+		"binaries.argsh",
+		"{binaries: from_items(items(binaries)[?[1].groups && contains([1].groups, 'kustomize')])}",
+	}
+
+	out, err := filterContent(content, selectors, "b.yaml")
+	if err != nil {
+		t.Fatalf("filterContent: %v", err)
+	}
+
+	var got struct {
+		Binaries map[string]interface{} `yaml:"binaries"`
+	}
+	if err := yaml.Unmarshal(out, &got); err != nil {
+		t.Fatalf("unmarshal %s: %v", out, err)
+	}
+	for _, want := range []string{"argsh", "kustomize"} {
+		if _, ok := got.Binaries[want]; !ok {
+			t.Errorf("binaries.%s missing — got %v (the JMESPath side must not replace the simple side's key)", want, keysOf(got.Binaries))
+		}
+	}
+	if _, ok := got.Binaries["kind"]; ok {
+		t.Error("binaries.kind leaked in — no selector asked for the local group")
+	}
+}
+
+func TestFilterContent_BareFilterSelectorsUnion(t *testing.T) {
+	// Bare filters return LISTS, and wrapKeyFor sends every `binaries[?…]`
+	// variant to the same `binaries` key — so a chain of them used to keep
+	// only the last group.
+	content := []byte(`binaries:
+  - name: argsh
+    groups: [core]
+  - name: kustomize
+    groups: [kustomize]
+  - name: kind
+    groups: [local]
+`)
+	selectors := []string{
+		"binaries[?groups && contains(groups, 'core')]",
+		"binaries[?groups && contains(groups, 'kustomize')]",
+	}
+
+	out, err := filterContent(content, selectors, "b.yaml")
+	if err != nil {
+		t.Fatalf("filterContent: %v", err)
+	}
+
+	var got struct {
+		Binaries []struct {
+			Name string `yaml:"name"`
+		} `yaml:"binaries"`
+	}
+	if err := yaml.Unmarshal(out, &got); err != nil {
+		t.Fatalf("unmarshal %s: %v", out, err)
+	}
+	var names []string
+	for _, b := range got.Binaries {
+		names = append(names, b.Name)
+	}
+	sort.Strings(names)
+	if len(names) != 2 || names[0] != "argsh" || names[1] != "kustomize" {
+		t.Errorf("binaries = %v, want both filters' results [argsh kustomize]", names)
+	}
+}