From a31ee32e9e2af3ea986ae6c1f3eb49719e571778 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Fri, 24 Apr 2026 06:11:04 +0000
Subject: [PATCH] Address reviewer feedback: preserve update error context and
 add version assertions in tests

- exec.go: include original update error in fallback log message; wrap both
  errors (update + reinstall) when reinstall also fails so callers get full context
- init_test.go: add semver import and GetPluginVersion assertions after
  CheckHelmPlugins to verify plugins are at required versions on disk

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/c784db7d-7d4c-40a0-97f0-a31eb8901cd6

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
---
 pkg/app/init_test.go | 11 +++++++++++
 pkg/helmexec/exec.go |  8 ++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/pkg/app/init_test.go b/pkg/app/init_test.go
index bec58d24..63b5be45 100644
--- a/pkg/app/init_test.go
+++ b/pkg/app/init_test.go
@@ -11,6 +11,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/Masterminds/semver/v3"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
@@ -271,6 +272,16 @@ func TestCheckHelmPlugins_UpdateFailsFallbackToReinstall(t *testing.T) {
 		assert.Contains(t, calledOps, "uninstall:"+p.name, "expected uninstall to be called for plugin %s", p.name)
 		assert.Contains(t, calledOps, "install:"+p.name, "expected install to be called for plugin %s", p.name)
 	}
+
+	// Verify that all plugins are now at (or above) the required version on disk.
+	for _, p := range helmPlugins {
+		requiredVersion, err := semver.NewVersion(p.version)
+		require.NoError(t, err)
+		installedVersion, err := helmexec.GetPluginVersion(p.name, pluginsDir)
+		require.NoError(t, err, "plugin %s should be present after reinstall", p.name)
+		assert.False(t, installedVersion.LessThan(requiredVersion),
+			"plugin %s: installed version %s should be >= required version %s", p.name, installedVersion, requiredVersion)
+	}
 }
 
 func TestCheckHelmPlugins_UpdateErrorButPluginAtRequiredVersion(t *testing.T) {
diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go
index 84115847..17330daf 100644
--- a/pkg/helmexec/exec.go
+++ b/pkg/helmexec/exec.go
@@ -946,11 +946,15 @@ func (helm *execer) UpdatePlugin(name, path, version string) error {
 	helm.info(out)
 	if err != nil {
 		// If standard update failed, fall back to uninstall + reinstall with specific version
-		helm.logger.Infof("helm plugin update %v failed, falling back to reinstall with version %v", name, version)
+		updateErr := err
+		helm.logger.Infof("helm plugin update %v failed (%v), falling back to reinstall with version %v", name, updateErr, version)
 		if uninstallErr := helm.uninstallPlugin(name); uninstallErr != nil {
 			helm.logger.Warnf("Failed to uninstall helm plugin %v: %v", name, uninstallErr)
 		}
-		return helm.AddPlugin(name, path, version)
+		if reinstallErr := helm.AddPlugin(name, path, version); reinstallErr != nil {
+			return fmt.Errorf("helm plugin update failed (%w) and reinstall also failed: %w", updateErr, reinstallErr)
+		}
+		return nil
 	}
 	return nil
 }