fix: reap superseded rc tags at publish time

internal/release/release.go

@@ -9,6 +9,8 @@ import (
 	"regexp"
 	"strings"
 	"time"
+
+	"github.com/stablekernel/cascade/internal/version"
 )
 
 // Action represents the release management action to perform
@@ -211,34 +213,83 @@ func (m *Manager) deleteGitTag(tagName string) error {
 	return fmt.Errorf("delete tag failed with status %d: %s", resp.StatusCode, string(body))
 }
 
-// cleanupRCTags deletes all RC tags for a given base version.
-// For example, if baseTag is "v1.0.0", this deletes v1.0.0-rc.0, v1.0.0-rc.1, etc.
+// cleanupRCTags deletes every RC tag whose base version is at or below the
+// published version. For example, publishing "v1.0.1" reaps v1.0.1-rc.0,
+// v1.0.1-rc.1, and also any superseded earlier base such as v1.0.0-rc.0 left
+// behind when v1.0.0 was never published. RC tags on a strictly higher base
+// (e.g. v1.1.0-rc.0, staged for a future release) are preserved, as are tags
+// carrying a different tag prefix, which name an unrelated release line.
+//
+// Hotfix-variant RC tags (v1.0.0-rc.2.hotfix.1) are not plain RC tags, so
+// parseRCTag rejects them and this loop skips them; their lifecycle is handled
+// by the hotfix-rejoin cleanup, not publish-time reaping.
+//
 // This is called after publishing a release to clean up the RC tags.
-func (m *Manager) cleanupRCTags(baseTag string) error {
+func (m *Manager) cleanupRCTags(publishedTag string) error {
+	publishedPrefix, published, err := splitVersionPrefix(publishedTag)
+	if err != nil {
+		return fmt.Errorf("parsing published version %q: %w", publishedTag, err)
+	}
+
 	// List all tags in the repository
 	tags, err := m.listTags()
 	if err != nil {
 		return fmt.Errorf("listing tags: %w", err)
 	}
 
-	// Find and delete all RC tags for this base version
+	// Reap every RC tag whose base is <= the published version (same prefix).
 	for _, tag := range tags {
 		tagBase, _, ok := parseRCTag(tag)
 		if !ok {
-			continue // Not an RC tag
+			continue // Not a plain RC tag (or a hotfix variant)
 		}
-		if tagBase == baseTag {
-			fmt.Printf("Cleaning up RC tag: %s\n", tag)
-			if err := m.deleteGitTag(tag); err != nil {
-				fmt.Printf("Warning: failed to delete RC tag %s: %v\n", tag, err)
-				// Continue with other tags
-			}
+		basePrefix, base, err := splitVersionPrefix(tagBase)
+		if err != nil {
+			continue // Unparseable base - leave it alone
+		}
+		// A different prefix names a separate release line; never compare across
+		// prefixes since version.Compare is prefix-agnostic.
+		if basePrefix != publishedPrefix {
+			continue
+		}
+		// Preserve bases strictly greater than the published version (future work).
+		if base.Compare(published) > 0 {
+			continue
+		}
+		fmt.Printf("Cleaning up RC tag: %s\n", tag)
+		if err := m.deleteGitTag(tag); err != nil {
+			fmt.Printf("Warning: failed to delete RC tag %s: %v\n", tag, err)
+			// Continue with other tags
 		}
 	}
 
 	return nil
 }
 
+// splitVersionPrefix splits a base version tag into its tag prefix and the
+// parsed numeric core. version.Parse only recognizes an alphabetic prefix, so a
+// non-letter prefix such as "rel-" or "release/" is peeled off here before
+// parsing the numeric "major.minor.patch" remainder. The returned Version
+// carries no prefix; callers compare the numeric core via version.Compare and
+// compare prefixes separately as strings.
+func splitVersionPrefix(baseTag string) (prefix string, v *version.Version, err error) {
+	loc := versionCorePattern.FindStringIndex(baseTag)
+	if loc == nil {
+		return "", nil, fmt.Errorf("no version core in %q", baseTag)
+	}
+	prefix = baseTag[:loc[0]]
+	parsed, err := version.Parse(baseTag[loc[0]:])
+	if err != nil {
+		return "", nil, err
+	}
+	return prefix, parsed, nil
+}
+
+// versionCorePattern locates the trailing numeric "major.minor.patch" core of a
+// base version tag so any tag prefix (alphabetic, "rel-", "release/", or none)
+// can be split off before the core is parsed.
+var versionCorePattern = regexp.MustCompile(`\d+\.\d+\.\d+$`)
+
 // listTags returns all tags in the repository
 func (m *Manager) listTags() ([]string, error) {
 	req, err := m.newRequest("GET", "/git/refs/tags", nil)

internal/release/release_test.go

@@ -525,7 +525,8 @@ func TestManager_Publish(t *testing.T) {
 				{"ref": "refs/tags/v1.0.0-rc.0"},
 				{"ref": "refs/tags/v1.0.0-rc.3"},
 				{"ref": "refs/tags/v1.0.0-rc.5"},
-				{"ref": "refs/tags/v0.9.0-rc.2"}, // Different base version - should NOT be deleted
+				{"ref": "refs/tags/v0.9.0-rc.2"}, // Superseded earlier base - should be reaped
+				{"ref": "refs/tags/v1.1.0-rc.0"}, // Higher base (future work) - should NOT be deleted
 			})
 			return
 		}
@@ -584,12 +585,14 @@ func TestManager_Publish(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, int64(999), result.ReleaseID)
 
-	// Verify all RC tags for v1.0.0 were deleted, but not v0.9.0-rc.2
-	assert.Len(t, deletedTags, 3)
+	// Publishing v1.0.0 reaps every RC tag whose base is at or below v1.0.0,
+	// including the superseded v0.9.0 base, but never a higher base (v1.1.0).
+	assert.Len(t, deletedTags, 4)
 	assert.Contains(t, deletedTags, "v1.0.0-rc.0")
 	assert.Contains(t, deletedTags, "v1.0.0-rc.3")
 	assert.Contains(t, deletedTags, "v1.0.0-rc.5")
-	assert.NotContains(t, deletedTags, "v0.9.0-rc.2") // Different base version
+	assert.Contains(t, deletedTags, "v0.9.0-rc.2")    // Superseded earlier base
+	assert.NotContains(t, deletedTags, "v1.1.0-rc.0") // Higher base preserved
 }
 
 // TestManager_Publish_CustomTagPrefix verifies that publishing a release with a
@@ -769,6 +772,76 @@ func TestManager_Create_CleansUpStaleDrafts(t *testing.T) {
 	assert.NotContains(t, deletedIDs, int64(102)) // Preserved - different base version
 }
 
+// TestCleanupRCTags_ReapsSupersededBases verifies that publish-time RC cleanup
+// reaps every RC tag whose base version is at or below the published version,
+// including bases from earlier rounds that were never published, while leaving
+// any higher base (work staged for a future release) untouched. A delete that
+// returns 404 (already gone) is treated as a no-op so the cleanup is idempotent,
+// and a base carrying a different tag prefix is never compared across prefixes.
+func TestCleanupRCTags_ReapsSupersededBases(t *testing.T) {
+	listedTags := []string{
+		"v0.9.0-rc.0",          // below published base - reap
+		"v1.0.0-rc.0",          // superseded earlier base, never published - reap
+		"v1.0.0-rc.4",          // below published rc on the same base - reap
+		"v1.0.1-rc.0",          // equal to published base - reap (404 path)
+		"v1.0.1-rc.2",          // equal to published base - reap
+		"v1.1.0-rc.0",          // higher base, future work - preserve
+		"rel-1.0.0-rc.0",       // different prefix - preserve
+		"v1.0.1-rc.1.hotfix.1", // hotfix variant, not a plain RC tag - preserve
+	}
+
+	deletedTags := []string{}
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == "GET" && r.URL.Path == "/repos/owner/repo/git/refs/tags" {
+			refs := make([]map[string]string, 0, len(listedTags))
+			for _, tag := range listedTags {
+				refs = append(refs, map[string]string{"ref": "refs/tags/" + tag})
+			}
+			_ = json.NewEncoder(w).Encode(refs)
+			return
+		}
+
+		if r.Method == "DELETE" && strings.Contains(r.URL.Path, "/git/refs/tags/") {
+			tag := strings.TrimPrefix(r.URL.Path, "/repos/owner/repo/git/refs/tags/")
+			deletedTags = append(deletedTags, tag)
+			// v1.0.1-rc.0 simulates a tag a prior partial run already removed: a
+			// 404 must be absorbed as a no-op, not surfaced as a failure.
+			if tag == "v1.0.1-rc.0" {
+				w.WriteHeader(http.StatusNotFound)
+				return
+			}
+			w.WriteHeader(http.StatusNoContent)
+			return
+		}
+	}))
+	defer server.Close()
+
+	manager := &Manager{
+		client:  server.Client(),
+		baseURL: server.URL,
+		token:   "test-token",
+		repo:    "owner/repo",
+	}
+
+	err := manager.cleanupRCTags("v1.0.1")
+	require.NoError(t, err)
+
+	// Every base <= v1.0.1 with the matching prefix is reaped, regardless of which
+	// round produced it. The 404 on v1.0.1-rc.0 is a no-op.
+	assert.ElementsMatch(t, []string{
+		"v0.9.0-rc.0",
+		"v1.0.0-rc.0",
+		"v1.0.0-rc.4",
+		"v1.0.1-rc.0",
+		"v1.0.1-rc.2",
+	}, deletedTags)
+
+	// Higher base, mismatched prefix, and hotfix variants are never touched.
+	assert.NotContains(t, deletedTags, "v1.1.0-rc.0")
+	assert.NotContains(t, deletedTags, "rel-1.0.0-rc.0")
+	assert.NotContains(t, deletedTags, "v1.0.1-rc.1.hotfix.1")
+}
+
 func TestParseRCTag(t *testing.T) {
 	tests := []struct {
 		tag      string