From e611a3be38bb0eed17e59ba427dd889920e108c0 Mon Sep 17 00:00:00 2001
From: jade <jade@hopepunk.me>
Date: Wed, 14 Oct 2026 17:13:01 +1100
Subject: [PATCH] Sort invalid versions to the bottom rather than panicking

---
 pkg/recipe/recipe.go      | 33 +++++++++++++++------------------
 pkg/recipe/recipe_test.go | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/pkg/recipe/recipe.go b/pkg/recipe/recipe.go
index 592e2452..a5fa1b6e 100644
--- a/pkg/recipe/recipe.go
+++ b/pkg/recipe/recipe.go
@@ -592,15 +592,7 @@ func ReadReposMetadata(debug bool) (RepoCatalogue, error) {
 // sortRecipeVersions sorts the recipe semver versions
 func sortRecipeVersions(versions RecipeVersions) {
 	sort.Slice(versions, func(i, j int) bool {
-		version1, err := tagcmp.Parse(getVersionString(versions[i]))
-		if err != nil {
-			panic(err)
-		}
-		version2, err := tagcmp.Parse(getVersionString(versions[j]))
-		if err != nil {
-			panic(err)
-		}
-		return version1.IsLessThan(version2)
+		return safeCompareVersions(getVersionString(versions[i]), getVersionString(versions[j]))
 	})
 }
 
@@ -616,18 +608,23 @@ func getVersionString(versionMap map[string]map[string]ServiceMeta) string {
 // sortVersionStrings sorts a list of semver version strings
 func sortVersionStrings(versions []string) {
 	sort.Slice(versions, func(i, j int) bool {
-		version1, err := tagcmp.Parse(versions[i])
-		if err != nil {
-			panic(err)
-		}
-		version2, err := tagcmp.Parse(versions[j])
-		if err != nil {
-			panic(err)
-		}
-		return version1.IsLessThan(version2)
+		return safeCompareVersions(versions[i], versions[j])
 	})
 }
 
+// Invalid versions are sorted as less than valid ones.
+func safeCompareVersions(v1, v2 string) bool {
+	version1, err1 := tagcmp.Parse(v1)
+	if err1 != nil {
+		return true
+	}
+	version2, err2 := tagcmp.Parse(v2)
+	if err2 != nil {
+		return false
+	}
+	return version1.IsLessThan(version2)
+}
+
 // GetRecipeCatalogueVersions list the recipe versions listed in the recipe catalogue.
 func GetRecipeCatalogueVersions(recipeName string, catl RecipeCatalogue) ([]string, error) {
 	var versions []string
diff --git a/pkg/recipe/recipe_test.go b/pkg/recipe/recipe_test.go
index 9f3c5748..c623f161 100644
--- a/pkg/recipe/recipe_test.go
+++ b/pkg/recipe/recipe_test.go
@@ -123,3 +123,41 @@ func TestGetVersionLabelLocalDoesNotUseTimeoutLabel(t *testing.T) {
 		assert.NotEqual(t, label, timeout)
 	}
 }
+
+// This is not a full test of comparison, as that occurs in the external
+// tagcmp package. This is a sunny-day integration test
+func TestSortRecipeVersionsSortsTwoValidStrings(t *testing.T) {
+	versions := RecipeVersions{
+		{"2.0.0": {}}, // Index 0
+		{"1.0.0": {}}, // Index 1
+	}
+
+	sortRecipeVersions(versions)
+
+	assert.Equal(t, "1.0.0", getVersionString(versions[0]), "Version 1.0.0 should be first")
+	assert.Equal(t, "2.0.0", getVersionString(versions[1]), "Version 2.0.0 should be second")
+}
+
+func TestSortRecipeVersionsSortsInvalidVersionStringToTheBottom(t *testing.T) {
+	versions := RecipeVersions{
+		{"2.0.0": {}},   // Index 0
+		{"invalid": {}}, // Index 1
+	}
+
+	sortRecipeVersions(versions)
+
+	assert.Equal(t, "invalid", getVersionString(versions[0]), "The invalid version should be first")
+	assert.Equal(t, "2.0.0", getVersionString(versions[1]), "Version 2.0.0 should be second")
+}
+
+func TestSortRecipeVersionsSortsInvalidVersionStringToTheBottomInReverse(t *testing.T) {
+	versions := RecipeVersions{
+		{"invalid": {}}, // Index 0
+		{"2.0.0": {}},   // Index 1
+	}
+
+	sortRecipeVersions(versions)
+
+	assert.Equal(t, "invalid", getVersionString(versions[0]), "The invalid version should be first")
+	assert.Equal(t, "2.0.0", getVersionString(versions[1]), "Version 2.0.0 should be second")
+}