fix(release): stop a stale release cut from freezing an rc tag on an outdated commit

CHANGELOG.md

@@ -19,6 +19,19 @@ A `Migration` section is added to any release that bumps `schema_version`.
 
 ### Fixed
 
+- **release:** A release cut that materializes a git tag now fails closed when the
+  tag already exists at a different commit, instead of treating the
+  `422 reference already exists` response as an unconditional success. The old
+  behavior was a silent no-op that left the tag frozen on the stale commit while
+  the cut reported success. A tag already pointing at the target commit stays the
+  genuinely idempotent case.
+
+- **orchestrate:** Version derivation now advances the release-candidate counter
+  past any tag that already exists at a different commit, so a stalled or
+  rolled-back recorded state can no longer mint an rc number already published at
+  another commit. A candidate already pointing at the current commit is reused
+  unchanged, keeping the no-collision path identical.
+
 - **ci:** The Release workflow's `Test` job now installs the SHA-pinned
   `actionlint` binary before `go test ./...`, matching the unit-test lanes in
   `validate.yaml` and `pr.yaml`. The emitted-workflow enforcement guard

internal/orchestrate/orchestrator.go

@@ -592,7 +592,77 @@ func (o *Orchestrator) calculateVersion() (string, error) {
 		return "", fmt.Errorf("calculating version: %w", err)
 	}
 
-	return nextVersion.String(), nil
+	// A stale or rolled-back recorded state can drive the rc counter to a value
+	// that already exists as a tag at a different commit; resolve any such
+	// collision before returning so a fresh cut never reuses a published rc.
+	resolved, err := o.resolveRCCollision(nextVersion)
+	if err != nil {
+		return "", err
+	}
+
+	return resolved.String(), nil
+}
+
+// resolveRCCollision advances an rc candidate past any tag of the same name that
+// already exists at a DIFFERENT commit than the current HEAD, so a stale, raced,
+// or rolled-back recorded state can never re-mint an rc number that was already
+// published at another sha (the frozen-rc incident, where a state stuck at rc.0
+// recomputed rc.1 while a v...-rc.1 tag already pointed at an outdated commit).
+//
+// A tag that already exists AT the current HEAD is the same cut re-running and is
+// reused unchanged, keeping convergence idempotent. A candidate with no rc
+// segment, or a HEAD that cannot be resolved, leaves the version untouched, so
+// the no-collision path stays byte-identical to before.
+func (o *Orchestrator) resolveRCCollision(v *version.Version) (*version.Version, error) {
+	if v == nil || v.PreRelease < 0 {
+		return v, nil
+	}
+
+	headSHA, err := o.gitOutput("rev-parse", "HEAD")
+	if err != nil || headSHA == "" {
+		// Without a resolvable HEAD there is nothing to compare against; preserve
+		// the historical behavior rather than fail the whole calculation.
+		return v, nil
+	}
+
+	// maxRCAdvance bounds the walk so a pathological run of colliding tags can
+	// never spin forever; it is far beyond any realistic rc depth for one base.
+	const maxRCAdvance = 1000
+	for i := 0; i < maxRCAdvance; i++ {
+		tag := v.String()
+		sha, exists, err := o.tagCommit(tag)
+		if err != nil {
+			return nil, fmt.Errorf("checking rc tag collision for %s: %w", tag, err)
+		}
+		if !exists || sha == headSHA {
+			return v, nil
+		}
+		log.Debug("rc tag %s already exists at %s (head %s); advancing rc number",
+			tag, truncateSHA(sha), truncateSHA(headSHA))
+		v = v.WithRC(v.PreRelease + 1)
+	}
+
+	return nil, fmt.Errorf("rc collision resolution exceeded %d advances starting from %s", maxRCAdvance, v.String())
+}
+
+// tagCommit reports the commit a tag points at in the orchestration's repository
+// and whether the tag exists. A missing tag returns ("", false, nil); only an
+// unexpected git failure returns an error. Existence is checked first so a
+// non-existent tag is distinguished from a genuine git error rather than inferred
+// from a rev-list exit code.
+func (o *Orchestrator) tagCommit(tag string) (sha string, exists bool, err error) {
+	listed, err := o.gitOutput("tag", "-l", "--", tag)
+	if err != nil {
+		return "", false, err
+	}
+	if listed == "" {
+		return "", false, nil
+	}
+	sha, err = o.gitOutput("rev-list", "-n", "1", tag)
+	if err != nil {
+		return "", true, err
+	}
+	return sha, true, nil
 }
 
 // calculateComponentVersion derives the next version for the orchestration's
@@ -635,7 +705,16 @@ func (o *Orchestrator) calculateComponentVersion() (string, error) {
 		return "", fmt.Errorf("calculating version for component %q: %w", o.component, err)
 	}
 
-	return nextVersion.String(), nil
+	// Guard the component's rc counter against a stale-state collision, exactly as
+	// the single-component path does. The candidate carries the component's strict
+	// tag grammar, so its String() renders the component-prefixed tag the collision
+	// check looks up, keeping the check scoped to the component's namespace.
+	resolved, err := o.resolveRCCollision(nextVersion)
+	if err != nil {
+		return "", fmt.Errorf("resolving rc collision for component %q: %w", o.component, err)
+	}
+
+	return resolved.String(), nil
 }
 
 // calculateChangelogRefs returns the changelog base SHA and previous tag,

internal/orchestrate/rc_collision_test.go

@@ -0,0 +1,68 @@
+package orchestrate
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/stablekernel/cascade/internal/config"
+)
+
+// TestCalculateVersion_RCCollisionAdvancesPastExistingTag is the regression
+// guard for the frozen-rc incident: a stale recorded state drove the rc counter
+// to a value that already existed as a tag at a different commit, and the cut
+// reused it instead of advancing. With state stuck at rc.0 the calculation
+// recomputes rc.1; when a v1.2.0-rc.1 tag already points at an earlier commit,
+// the result must advance to rc.2 rather than collide. The control subcase (no
+// existing tag) proves the no-collision path is unchanged and still yields rc.1.
+func TestCalculateVersion_RCCollisionAdvancesPastExistingTag(t *testing.T) {
+	tests := []struct {
+		name        string
+		collideAt   string // git revision to tag v1.2.0-rc.1 at, or "" for no tag
+		wantVersion string
+	}{
+		{name: "existing rc.1 at a different commit advances to rc.2", collideAt: "HEAD~1", wantVersion: "v1.2.0-rc.2"},
+		{name: "no collision keeps rc.1 unchanged", collideAt: "", wantVersion: "v1.2.0-rc.1"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			repoDir, head := initRepo(t)
+
+			if tt.collideAt != "" {
+				// Materialize the colliding rc tag at a commit that is NOT HEAD, so
+				// the guard sees it as a foreign target rather than an idempotent
+				// re-cut of the same commit.
+				runGit(t, repoDir, "tag", "v1.2.0-rc.1", tt.collideAt)
+			}
+
+			// calculateVersion resolves commit ranges through the process working
+			// directory, so run from the repo like the other orchestrate git tests.
+			orig, err := os.Getwd()
+			require.NoError(t, err)
+			require.NoError(t, os.Chdir(repoDir))
+			t.Cleanup(func() { require.NoError(t, os.Chdir(orig)) })
+
+			o := &Orchestrator{
+				environment: "dev",
+				baseDir:     repoDir,
+				cicdFile: &config.CICDFile{
+					Config: &config.TrunkConfig{
+						Environments: config.EnvNames("dev", "prod"),
+					},
+					State: map[string]*config.EnvState{
+						// Stuck at rc.0: the recomputed candidate is rc.1.
+						"dev":  {Version: "v1.2.0-rc.0"},
+						"prod": {Version: "v1.2.0", SHA: head},
+					},
+				},
+			}
+
+			got, err := o.calculateVersion()
+			require.NoError(t, err)
+			assert.Equal(t, tt.wantVersion, got)
+		})
+	}
+}

internal/release/coverage_test.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -161,11 +162,27 @@ func TestCreateGitTag_SkipsNonGitHubHost(t *testing.T) {
 }
 
 func TestCreateGitTag_AcceptsCreatedAndExists(t *testing.T) {
+	// A fresh ref create (201) succeeds outright. A 422 (ref already exists) is
+	// accepted only when the existing tag already points at the requested commit,
+	// which createGitTag confirms with a follow-up GET; the same-sha case is the
+	// genuinely idempotent one. A 422 whose existing target DIFFERS is a distinct,
+	// fail-closed case covered by TestManager_CreateGitTag_ExistingTagDifferentSHA.
 	for _, status := range []int{http.StatusCreated, http.StatusUnprocessableEntity} {
 		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			assert.Equal(t, "POST", r.Method)
-			assert.Contains(t, r.URL.Path, "/git/refs")
-			w.WriteHeader(status)
+			switch {
+			case r.Method == http.MethodPost:
+				assert.Contains(t, r.URL.Path, "/git/refs")
+				w.WriteHeader(status)
+			case r.Method == http.MethodGet && strings.Contains(r.URL.Path, "/git/refs/tags/"):
+				// The existing tag points at the same commit the cut targets.
+				w.WriteHeader(http.StatusOK)
+				_ = json.NewEncoder(w).Encode(map[string]any{
+					"ref":    r.URL.Path,
+					"object": map[string]any{"sha": "abc123", "type": "commit"},
+				})
+			default:
+				t.Errorf("unexpected request %s %s", r.Method, r.URL.Path)
+			}
 		}))
 		m := &Manager{client: server.Client(), baseURL: server.URL + "/github", token: "t", repo: "owner/repo"}
 		err := m.createGitTag("v1.0.0", "abc123")

internal/release/release.go

@@ -225,19 +225,74 @@ func (m *Manager) createGitTag(tagName, sha string) error {
 	}
 	defer func() { _ = resp.Body.Close() }()
 
-	// 201 Created or 422 (already exists) are acceptable
 	if resp.StatusCode == http.StatusCreated {
 		return nil
 	}
 	if resp.StatusCode == http.StatusUnprocessableEntity {
-		// Tag already exists - this is fine
-		return nil
+		// The ref already exists. This is idempotent ONLY when the existing tag
+		// already points at the requested commit (a convergence rerun re-cutting
+		// the same tag). When it points at a DIFFERENT commit the ref create was a
+		// silent no-op that would leave the tag frozen at the old commit, which is
+		// how a stale or collided release cut previously stranded an rc tag on an
+		// outdated sha. Resolve the existing target and fail closed on a mismatch
+		// rather than reporting success.
+		return m.verifyExistingTagTarget(tagName, sha)
 	}
 
 	body, _ := io.ReadAll(resp.Body)
 	return fmt.Errorf("create tag failed with status %d: %s", resp.StatusCode, string(body))
 }
 
+// gitRef is the GitHub git-data ref shape returned by GET /git/refs/tags/<name>.
+// Only the target object's sha is needed to confirm where an existing tag points.
+type gitRef struct {
+	Object struct {
+		SHA  string `json:"sha"`
+		Type string `json:"type"`
+	} `json:"object"`
+}
+
+// verifyExistingTagTarget confirms that an already-present tag points at wantSHA.
+// It is called only after a ref-create returned 422 (already exists): a match is
+// the genuinely-harmless idempotent case, while a mismatch means the tag is
+// frozen at a different commit than the release cut targets. An rc tag is
+// immutable in cascade's single-flight release model - a fresh cut that needs a
+// different commit must take a new rc number, not silently reuse an existing tag
+// - so a mismatch fails loudly instead of returning a false success. A target
+// that cannot be resolved also fails closed, since success cannot be confirmed.
+func (m *Manager) verifyExistingTagTarget(tagName, wantSHA string) error {
+	req, err := m.newRequest("GET", "/git/refs/tags/"+tagName, nil)
+	if err != nil {
+		return err
+	}
+
+	resp, err := m.client.Do(req)
+	if err != nil {
+		return fmt.Errorf("resolving existing tag %s: %w", tagName, err)
+	}
+	defer func() { _ = resp.Body.Close() }()
+
+	if resp.StatusCode != http.StatusOK {
+		body, _ := io.ReadAll(resp.Body)
+		return fmt.Errorf("tag %s already exists but its target could not be resolved (status %d): %s", tagName, resp.StatusCode, string(body))
+	}
+
+	var ref gitRef
+	if err := json.NewDecoder(resp.Body).Decode(&ref); err != nil {
+		return fmt.Errorf("decoding existing tag %s ref: %w", tagName, err)
+	}
+
+	if ref.Object.SHA == "" {
+		return fmt.Errorf("tag %s already exists but its target sha could not be determined to confirm it points at %s", tagName, wantSHA)
+	}
+
+	if ref.Object.SHA != wantSHA {
+		return fmt.Errorf("tag %s already exists at %s but this release cut targets %s; refusing to leave the tag frozen on the stale commit (an rc tag is immutable - a fresh cut needs a new rc number)", tagName, ref.Object.SHA, wantSHA)
+	}
+
+	return nil
+}
+
 // deleteGitTag deletes a git tag
 func (m *Manager) deleteGitTag(tagName string) error {
 	endpoint := "/git/refs/tags/" + tagName