Push release target before publishing to avoid permanent tag-name reservation on Immutable Releases

README.md

@@ -116,8 +116,23 @@ release. Some options such as `--check-pr` will modify this behavior.
    yet.
 6. **Upload release assets**. Any files written to `$ASSETS_DIR` will be
    uploaded as release assets.
-7. **Publish the release**.
-8. **Emit output** including release version, tag, change level, etc.
+7. **Push the release target** (e.g. a bake commit produced by the pre-tag
+   hook) to its branch on the remote. Pushing the target before publishing
+   ensures that on a repository with
+   [Immutable Releases](https://docs.github.com/en/code-security/concepts/supply-chain-security/immutable-releases)
+   enabled, a rejected push to a protected branch does not permanently reserve
+   the tag name.
+8. **Publish the release**.
+9. **Emit output** including release version, tag, change level, etc.
+
+If the push in step 7 fails, the draft release and the pushed tag are cleaned
+up so the run can be retried. If step 8 (publish) fails, the draft release,
+the tag, and the pushed target are left in place — once publish has been
+attempted there is no way to tell whether the server processed it, and
+deleting the release or tag at that point would permanently reserve the tag
+name on Immutable Releases repositories. Recover by publishing the draft
+release manually, or by deleting the draft release and tag and reverting the
+target branch by hand.
 
 ## Recipes
 

release.go

@@ -65,8 +65,23 @@ func (o *Runner) cleanupAfterErr() error {
 	return err
 }
 
-func (o *Runner) addErrCleanup(fn func() error) {
-	o.errCleanups = append(o.errCleanups, fn)
+// addErrCleanup registers fn to run during cleanupAfterErr. The returned
+// cancel function deactivates the cleanup so that it becomes a no-op. Callers
+// must invoke cancel once the action that fn would undo has reached a point
+// where rolling it back is no longer safe or correct — for example, after
+// PublishRelease has been attempted, because the server may have processed
+// the publish even if the client saw an error, and on a repository with
+// Immutable Releases enabled, deleting either the release or its tag in that
+// state permanently reserves the tag name.
+func (o *Runner) addErrCleanup(fn func() error) (cancel func()) {
+	active := true
+	o.errCleanups = append(o.errCleanups, func() error {
+		if !active {
+			return nil
+		}
+		return fn()
+	})
+	return func() { active = false }
 }
 
 type Result struct {
@@ -327,15 +342,15 @@ func (o *Runner) run(ctx context.Context) (_ *Result, errOut error) {
 	if err != nil {
 		return nil, err
 	}
-	o.addErrCleanup(func() error {
+	cancelTagDelete := o.addErrCleanup(func() error {
 		_, e := o.runCmd(ctx, nil, "git", "push", o.PushRemote, "--delete", result.ReleaseTag)
 		return e
 	})
 
 	result.CreatedTag = true
 
 	if o.CreateRelease {
-		err = o.createRelease(ctx, result)
+		err = o.createRelease(ctx, result, cancelTagDelete)
 		if err != nil {
 			return nil, err
 		}
@@ -356,7 +371,17 @@ func (o *Runner) rejectShallowCheckout(ctx context.Context) error {
 }
 
 // createRelease handles the creation of a GitHub release, including notes, assets, and publishing.
-func (o *Runner) createRelease(ctx context.Context, result *Result) error {
+//
+// To remain compatible with repositories that have Immutable Releases enabled
+// (https://docs.github.com/en/code-security/concepts/supply-chain-security/immutable-releases),
+// the release is created as a draft, all assets are uploaded, the release
+// target is pushed, and only then is the release published. This matches the
+// workflow GitHub recommends for immutable releases: create draft → upload
+// assets → publish. Once PublishRelease has been attempted both destructive
+// cleanups (DeleteRelease and the tag delete) are cancelled, because the
+// server may have processed the publish even if the client saw an error, and
+// deleting either side in that state permanently reserves the tag name.
+func (o *Runner) createRelease(ctx context.Context, result *Result, cancelTagDelete func()) error {
 	releaseNotes, err := o.getReleaseNotes(ctx, result)
 	if err != nil {
 		return err
@@ -368,7 +393,7 @@ func (o *Runner) createRelease(ctx context.Context, result *Result) error {
 		return err
 	}
 
-	o.addErrCleanup(func() error {
+	cancelDeleteRelease := o.addErrCleanup(func() error {
 		return o.GithubClient.DeleteRelease(ctx, o.repoOwner(), o.repoName(), rel.ID)
 	})
 
@@ -382,13 +407,21 @@ func (o *Runner) createRelease(ctx context.Context, result *Result) error {
 		return nil
 	}
 
-	err = o.GithubClient.PublishRelease(ctx, o.repoOwner(), o.repoName(), o.MakeLatest, rel.ID)
+	// Push the target before publishing so the release stays a draft (and no
+	// immutable-tag row exists) until the most failure-prone step — the push
+	// to a possibly-protected branch — has succeeded.
+	err = o.pushTarget(ctx)
 	if err != nil {
 		return err
 	}
 
-	// push target last because it cannot be easily rolled back
-	return o.pushTarget(ctx)
+	// Publish boundary: from here on, neither the release nor the tag may be
+	// deleted by cleanup. A PublishRelease error does not guarantee the
+	// server did not process the publish.
+	cancelDeleteRelease()
+	cancelTagDelete()
+
+	return o.GithubClient.PublishRelease(ctx, o.repoOwner(), o.repoName(), o.MakeLatest, rel.ID)
 }
 
 func (o *Runner) uploadAssets(ctx context.Context, uploadURL string) error {

release_test.go

@@ -764,4 +764,127 @@ echo bar > "$ASSETS_DIR/bar.txt"
 			ChangeLevel:           changeLevelMinor,
 		}, got)
 	})
+
+	t.Run("pushTarget failure cleans up draft release and tag", func(t *testing.T) {
+		t.Parallel()
+		ctx := t.Context()
+		repos := setupGit(t)
+		githubClient := mocks.NewMockGithubClient(gomock.NewController(t))
+		githubClient.EXPECT().CompareCommits(gomock.Any(), "orgName", "repoName", "v2.0.0", repos.taggedCommits["head"], -1).Return(
+			&github.CommitComparison{
+				AheadBy: 1,
+				Commits: []string{repos.taggedCommits["head"]},
+			}, nil,
+		)
+		githubClient.EXPECT().CompareCommits(gomock.Any(), "orgName", "repoName", mergeSha, repos.taggedCommits["head"], 0).Return(
+			&github.CommitComparison{AheadBy: 0}, nil,
+		)
+		githubClient.EXPECT().ListMergedPullsForCommit(gomock.Any(), "orgName", "repoName", repos.taggedCommits["head"]).Return(
+			[]github.BasePull{{Number: 2, MergeCommitSha: mergeSha, Labels: []string{labelBreaking}}}, nil,
+		)
+		githubClient.EXPECT().GenerateReleaseNotes(gomock.Any(), "orgName", "repoName", "v3.0.0", "v2.0.0").Return(
+			"release notes", nil,
+		)
+		githubClient.EXPECT().CreateRelease(gomock.Any(), "orgName", "repoName", "v3.0.0", "release notes", false).Return(
+			&github.RepoRelease{
+				ID:        1,
+				UploadURL: "localhost",
+			}, nil,
+		)
+		// The release is still a draft when pushTarget fails, so DeleteRelease
+		// cleanup runs safely. No PublishRelease expectation: the publish must
+		// not be attempted when pushTarget fails.
+		githubClient.EXPECT().DeleteRelease(gomock.Any(), "orgName", "repoName", int64(1)).Return(nil)
+		preHook := `
+#!/bin/sh
+set -e
+git config user.name 'tester'
+git config user.email 'tester'
+current_branch=$(git rev-parse --abbrev-ref HEAD)
+echo "bake" > bake.txt
+git add bake.txt > /dev/null
+git commit -m "bake commit" > /dev/null
+echo "$current_branch" > "$RELEASE_TARGET"
+`
+		runner := &Runner{
+			CheckoutDir:   repos.clone,
+			Ref:           repos.taggedCommits["head"],
+			TagPrefix:     "v",
+			Repo:          "orgName/repoName",
+			PushRemote:    "origin",
+			GithubClient:  githubClient,
+			CreateRelease: true,
+			PreTagHook:    preHook,
+			TempDir:       t.TempDir(),
+		}
+		_, err := runner.run(ctx)
+		// Push to a checked-out branch of a non-bare origin is rejected by
+		// the default receive.denyCurrentBranch=refuse setting.
+		require.ErrorContains(t, err, "remote rejected")
+		ok, err := localTagExists(ctx, repos.origin, "v3.0.0")
+		require.NoError(t, err)
+		require.False(t, ok, "tag must be cleaned up after pushTarget failure")
+	})
+
+	t.Run("PublishRelease failure does not delete release or tag", func(t *testing.T) {
+		t.Parallel()
+		ctx := t.Context()
+		repos := setupGit(t)
+		// Allow pushes to origin's currently checked-out branch so pushTarget
+		// succeeds and we reach PublishRelease.
+		mustRunCmd(t, repos.origin, "git", "config", "receive.denyCurrentBranch", "ignore")
+		githubClient := mocks.NewMockGithubClient(gomock.NewController(t))
+		githubClient.EXPECT().CompareCommits(gomock.Any(), "orgName", "repoName", "v2.0.0", repos.taggedCommits["head"], -1).Return(
+			&github.CommitComparison{
+				AheadBy: 1,
+				Commits: []string{repos.taggedCommits["head"]},
+			}, nil,
+		)
+		githubClient.EXPECT().CompareCommits(gomock.Any(), "orgName", "repoName", mergeSha, repos.taggedCommits["head"], 0).Return(
+			&github.CommitComparison{AheadBy: 0}, nil,
+		)
+		githubClient.EXPECT().ListMergedPullsForCommit(gomock.Any(), "orgName", "repoName", repos.taggedCommits["head"]).Return(
+			[]github.BasePull{{Number: 2, MergeCommitSha: mergeSha, Labels: []string{labelBreaking}}}, nil,
+		)
+		githubClient.EXPECT().GenerateReleaseNotes(gomock.Any(), "orgName", "repoName", "v3.0.0", "v2.0.0").Return(
+			"release notes", nil,
+		)
+		githubClient.EXPECT().CreateRelease(gomock.Any(), "orgName", "repoName", "v3.0.0", "release notes", false).Return(
+			&github.RepoRelease{
+				ID:        1,
+				UploadURL: "localhost",
+			}, nil,
+		)
+		githubClient.EXPECT().PublishRelease(gomock.Any(), "orgName", "repoName", "", int64(1)).Return(errors.New("publish failed"))
+		// No DeleteRelease expectation: once PublishRelease has been attempted
+		// the cleanup must not delete the release. The mock controller fails
+		// the test if DeleteRelease is invoked.
+		preHook := `
+#!/bin/sh
+set -e
+git config user.name 'tester'
+git config user.email 'tester'
+current_branch=$(git rev-parse --abbrev-ref HEAD)
+echo "bake" > bake.txt
+git add bake.txt > /dev/null
+git commit -m "bake commit" > /dev/null
+echo "$current_branch" > "$RELEASE_TARGET"
+`
+		runner := &Runner{
+			CheckoutDir:   repos.clone,
+			Ref:           repos.taggedCommits["head"],
+			TagPrefix:     "v",
+			Repo:          "orgName/repoName",
+			PushRemote:    "origin",
+			GithubClient:  githubClient,
+			CreateRelease: true,
+			PreTagHook:    preHook,
+			TempDir:       t.TempDir(),
+		}
+		_, err := runner.run(ctx)
+		require.ErrorContains(t, err, "publish failed")
+		ok, err := localTagExists(ctx, repos.origin, "v3.0.0")
+		require.NoError(t, err)
+		require.True(t, ok, "tag must not be deleted once PublishRelease has been attempted")
+	})
 }