From 122e52fdef64a01236d35ef222267898c4370b6d Mon Sep 17 00:00:00 2001
From: Harshdeep Singh <harshdeep3456@gmail.com>
Date: Fri, 6 Nov 2026 13:46:11 +0530
Subject: [PATCH] Fail pack download when any requested pack fails (#4620)

When installing multiple packs, DownloadGitRepoAction._validate_result only
raised if *all* packs failed to download. As a result
"st2 pack install <existing> <non-existent>" reported success and silently
ignored the missing pack.

Raise if any pack fails and list the failed packs in the error message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 CHANGELOG.rst                               |  3 ++
 contrib/packs/actions/pack_mgmt/download.py | 39 +++++++++++++++------
 contrib/packs/tests/test_action_download.py | 29 +++++++++++++++
 3 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index e063fbff18..1b4a20e3e7 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -26,6 +26,9 @@ Fixed
 * Fix utf-8 encode before checking paramter max size #6352
 * Fix stuck running workflow tasks #6398 (by @guzzijones12@gmail.com)
 * Fixed action-alias help API returning 500 when RBAC enabled. #6414 (by @nzlosh)
+* Fix ``st2 pack install`` silently ignoring packs which fail to download (e.g. non-existent
+  packs) when installing multiple packs at once. The ``packs.download`` action now fails and
+  lists the packs which could not be downloaded. #4620
 
 Changed
 ~~~~~~~
diff --git a/contrib/packs/actions/pack_mgmt/download.py b/contrib/packs/actions/pack_mgmt/download.py
index cc0f7cd8fb..cd65185bc8 100644
--- a/contrib/packs/actions/pack_mgmt/download.py
+++ b/contrib/packs/actions/pack_mgmt/download.py
@@ -107,23 +107,40 @@ def run(
 
     @staticmethod
     def _validate_result(result, repo_url):
-        atleast_one_success = False
+        """
+        Validate the download results and raise if downloading any of the requested
+        packs failed.
+
+        Previously an error was only raised if *all* the packs failed to download which
+        meant that "st2 pack install <existing> <non-existent>" silently ignored the
+        non-existent pack and reported success (see #4620).
+        """
         sanitized_result = {}
+        failed_packs = []
 
-        for k, v in six.iteritems(result):
-            atleast_one_success |= v[0]
-            sanitized_result[k] = v[1]
+        for pack, (success, message) in six.iteritems(result):
+            sanitized_result[pack] = message
 
-        if not atleast_one_success:
+            if not success:
+                failed_packs.append(pack)
+
+        if failed_packs:
             message_list = []
-            message_list.append(
-                'The pack has not been downloaded from "%s".\n' % (repo_url)
-            )
+
+            if len(result) == 1:
+                message_list.append(
+                    'The pack has not been downloaded from "%s".\n' % (repo_url)
+                )
+            else:
+                message_list.append(
+                    "The following packs have not been downloaded: %s\n"
+                    % (", ".join(failed_packs))
+                )
+
             message_list.append("Errors:")
 
-            for pack, value in result.items():
-                success, error = value
-                message_list.append(error)
+            for pack in failed_packs:
+                message_list.append(result[pack][1])
 
             message = "\n".join(message_list)
             raise Exception(message)
diff --git a/contrib/packs/tests/test_action_download.py b/contrib/packs/tests/test_action_download.py
index ad2ed3e66f..6788330ee2 100644
--- a/contrib/packs/tests/test_action_download.py
+++ b/contrib/packs/tests/test_action_download.py
@@ -236,6 +236,35 @@ def test_run_pack_download_multiple_packs(self):
         self.assertTrue(os.path.isfile(os.path.join(self.repo_base, "test/pack.yaml")))
         self.assertTrue(os.path.isfile(os.path.join(self.repo_base, "test2/pack.yaml")))
 
+    def test_run_pack_download_multiple_packs_one_does_not_exist(self):
+        # Regression test for #4620 - a non-existent pack must not be silently ignored
+        # when other packs are downloaded successfully
+        action = self.get_action_instance()
+
+        for packs in (["test", "doesnotexist"], ["doesnotexist", "test"]):
+            expected_msg = (
+                r"The following packs have not been downloaded: doesnotexist"
+                r"(.|\n)*No record of the \"doesnotexist\" pack in the index"
+            )
+            self.assertRaisesRegex(
+                Exception,
+                expected_msg,
+                action.run,
+                packs=packs,
+                abs_repo_base=self.repo_base,
+                force=True,
+            )
+
+    def test_run_pack_download_single_pack_does_not_exist(self):
+        action = self.get_action_instance()
+        self.assertRaisesRegex(
+            Exception,
+            r"The pack has not been downloaded from \"None\"",
+            action.run,
+            packs=["doesnotexist"],
+            abs_repo_base=self.repo_base,
+        )
+
     @mock.patch.object(Repo, "clone_from")
     def test_run_pack_download_error(self, clone_from):
         clone_from.side_effect = Exception(