From d78f6010806855c504758baa61fbc2df18c57fc1 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Tue, 23 Jun 2026 18:35:18 -0400 Subject: [PATCH] fix: reap superseded rc tags at publish time Signed-off-by: Joshua Temple --- internal/release/release.go | 73 +++++++++++++++++++++++----- internal/release/release_test.go | 81 ++++++++++++++++++++++++++++++-- 2 files changed, 139 insertions(+), 15 deletions(-) diff --git a/internal/release/release.go b/internal/release/release.go index 5144725c..9847e080 100644 --- a/internal/release/release.go +++ b/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) diff --git a/internal/release/release_test.go b/internal/release/release_test.go index 636850fe..31de179b 100644 --- a/internal/release/release_test.go +++ b/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