fix: address PR review comments - use CLI selectors, fix doc comment, add malformed selector test

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/1f1c33ce-e50d-4781-85b8-d606b5d4ca54

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

pkg/app/app.go

@@ -1007,7 +1007,7 @@ func (a *App) processStateFileParallel(relPath string, defOpts LoadOpts, converg
 func (a *App) processNestedHelmfiles(st *state.HelmState, absd, file string, defOpts, opts LoadOpts, converge func(*state.HelmState) (bool, []error), sharedCtx *Context) (bool, error) {
 	anyMatched := false
 	for i, m := range st.Helmfiles {
-		if subhelmfileSelectorsConflict(opts.Selectors, m) {
+		if subhelmfileSelectorsConflict(a.Selectors, m) {
 			continue
 		}
 
@@ -1037,14 +1037,17 @@ func (a *App) processNestedHelmfiles(st *state.HelmState, absd, file string, def
 }
 
 // subhelmfileSelectorsConflict returns true when the subhelmfile has explicit
-// selectors that are provably incompatible with the parent's selectors,
+// selectors that are provably incompatible with the CLI selectors,
 // meaning no release could satisfy both. In that case the subhelmfile can be
 // safely skipped without loading or evaluating it.
-func subhelmfileSelectorsConflict(parentSelectors []string, m state.SubHelmfileSpec) bool {
-	if len(parentSelectors) == 0 || len(m.Selectors) == 0 || m.SelectorsInherited {
+// Only CLI selectors (not inherited parent selectors) are used for comparison,
+// so this optimization is restricted to cases where the user explicitly
+// provided selectors via the command line (e.g. -l name=b).
+func subhelmfileSelectorsConflict(cliSelectors []string, m state.SubHelmfileSpec) bool {
+	if len(cliSelectors) == 0 || len(m.Selectors) == 0 || m.SelectorsInherited {
 		return false
 	}
-	compatible, _ := state.SelectorsAreCompatible(parentSelectors, m.Selectors)
+	compatible, _ := state.SelectorsAreCompatible(cliSelectors, m.Selectors)
 	return !compatible
 }
 

pkg/state/release_filters.go

@@ -51,7 +51,9 @@ func (l LabelFilter) Match(r ReleaseSpec) bool {
 // potentially match the same release. It compares only positive labels (key=value):
 // two selectors conflict if they require the same key to have different values.
 // Returns true if at least one pair is compatible, false only if all pairs conflict.
-// On parse error, returns true (conservative: never skip on malformed input).
+// On parse error, returns (true, err): the conservative true ensures subhelmfiles
+// are never incorrectly skipped due to malformed input, while the non-nil error
+// allows callers to log or handle the parse failure if desired.
 func SelectorsAreCompatible(selectorsA, selectorsB []string) (bool, error) {
 	if len(selectorsA) == 0 || len(selectorsB) == 0 {
 		return true, nil

pkg/state/release_filters_test.go

@@ -74,6 +74,7 @@ func TestSelectorsAreCompatible(t *testing.T) {
 		selectorsA []string
 		selectorsB []string
 		compatible bool
+		wantErr    bool
 	}{
 		{
 			name:       "same key different value",
@@ -147,12 +148,23 @@ func TestSelectorsAreCompatible(t *testing.T) {
 			selectorsB: []string{"name=a"},
 			compatible: true,
 		},
+		{
+			name:       "malformed selector returns conservative true with error",
+			selectorsA: []string{"name=b"},
+			selectorsB: []string{"invalid_label"},
+			compatible: true,
+			wantErr:    true,
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := SelectorsAreCompatible(tt.selectorsA, tt.selectorsB)
-			assert.NoError(t, err)
+			if tt.wantErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
 			assert.Equal(t, tt.compatible, got)
 		})
 	}