From 121fda4900db478241bf901746161c342b238811 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 27 Mar 2026 00:14:14 +0000 Subject: [PATCH] fix: address review comments - preserve tab delimiters in List and fix transitive needs cross-namespace resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. exec.go List(): Filter empty lines without TrimSpace-ing non-empty lines, preserving trailing tab separators from helm list output. 2. state.go collectNeedsWithTransitives(): Resolve transitives by full ReleaseToID matching (via ID→ReleaseSpec map) instead of name-based matching, avoiding cross-namespace ambiguity when multiple releases share the same name. Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/94b03eb4-31e5-4d04-9959-eeaafb695855 Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/helmexec/exec.go | 4 ++-- pkg/state/state.go | 22 ++++++++++++++-------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index 20b59bda..777c3939 100644 --- a/pkg/helmexec/exec.go +++ b/pkg/helmexec/exec.go @@ -528,8 +528,8 @@ func (helm *execer) List(context HelmContext, filter string, flags ...string) (s lines := strings.Split(string(out), "\n") var filtered []string for _, line := range lines[1:] { - if trimmed := strings.TrimSpace(line); trimmed != "" { - filtered = append(filtered, trimmed) + if strings.TrimSpace(line) != "" { + filtered = append(filtered, line) } } out = []byte(strings.Join(filtered, "\n")) diff --git a/pkg/state/state.go b/pkg/state/state.go index b473af1f..a1742d12 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -3089,10 +3089,17 @@ func unmarkReleasesByNeedID(toUnmark map[string]struct{}, releases []Release) { } func collectAllNeedsWithTransitives(filteredReleases []Release, allReleases []ReleaseSpec) map[string]struct{} { + // Build an ID→ReleaseSpec map for efficient lookup by fully-qualified ID, + // avoiding cross-namespace ambiguity from name-based matching. + idToRelease := map[string]ReleaseSpec{} + for _, r := range allReleases { + idToRelease[ReleaseToID(&r)] = r + } + needsWithTranstives := map[string]struct{}{} for _, r := range filteredReleases { if !r.Filtered { - collectNeedsWithTransitives(r.ReleaseSpec, allReleases, needsWithTranstives) + collectNeedsWithTransitives(r.ReleaseSpec, idToRelease, needsWithTranstives) } } return needsWithTranstives @@ -3106,16 +3113,15 @@ func unmarkReleases(toUnmark map[string]struct{}, releases []Release) { } } -func collectNeedsWithTransitives(release ReleaseSpec, allReleases []ReleaseSpec, needsWithTranstives map[string]struct{}) { +func collectNeedsWithTransitives(release ReleaseSpec, idToRelease map[string]ReleaseSpec, needsWithTranstives map[string]struct{}) { for _, id := range release.Needs { if _, exists := needsWithTranstives[id]; !exists { needsWithTranstives[id] = struct{}{} - releaseParts := strings.Split(id, "/") - releaseName := releaseParts[len(releaseParts)-1] - for _, r := range allReleases { - if r.Name == releaseName { - collectNeedsWithTransitives(r, allReleases, needsWithTranstives) - } + // After ApplyOverrides/reformat(), need IDs are already fully-qualified + // (matching ReleaseToID format), so we look up by full ID to avoid + // cross-namespace ambiguity when multiple releases share the same name. + if r, ok := idToRelease[id]; ok { + collectNeedsWithTransitives(r, idToRelease, needsWithTranstives) } } }