release/pick-sha: fall back to master when pre-release staging branch is missing (#170781)

release/pick-sha: fall back to master when pre-release staging branch is missing

Author: trunk-io[bot]

pkg/cmd/release/BUILD.bazel

@@ -64,6 +64,7 @@ go_test(
     deps = [
         "//pkg/testutils/datapathutils",
         "//pkg/testutils/release",
+        "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_version//:version",
         "@com_github_slack_go_slack//:slack",
         "@com_github_stretchr_testify//require",

pkg/cmd/release/pick_sha.go

@@ -16,6 +16,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/version"
 	"github.com/spf13/cobra"
 )
 
@@ -41,6 +42,11 @@ const pickSHAJQLDryRun = `issueType="CRDB Release" and ` +
 // "Pick SHA" subtask on a release ticket.
 const pickSHASubtaskMatch = "Pick SHA"
 
+// masterBranch is the fallback branch pick-sha uses when a pre-release
+// ticket points at a staging branch that hasn't been cut yet — see
+// fetchSHAForPickSHA for the rationale.
+const masterBranch = "master"
+
 // pickSHA success notifications go to the release-ops channels — a
 // RelEng-only audience. This deliberately differs from the wider
 // #db-release-status / #db-release-test channels that branch-cut uses,
@@ -207,7 +213,8 @@ func (r *pickSHARunner) run(ctx context.Context) error {
 }
 
 func (r *pickSHARunner) processCandidate(ctx context.Context, c jiraIssue) error {
-	if _, err := parseReleaseVersion(c.Fields.Summary); err != nil {
+	v, err := parseReleaseVersion(c.Fields.Summary)
+	if err != nil {
 		// Tracking/template tickets that match the JQL but aren't real
 		// release tickets (e.g. summary "Release: 26.1|25.4|25.2 next
 		// patch") aren't actionable — log and move on rather than
@@ -272,17 +279,22 @@ func (r *pickSHARunner) processCandidate(ctx context.Context, c jiraIssue) error
 	if staging == "" {
 		return errors.Newf("ticket %s has no Staging Branch set", c.Key)
 	}
-	sha, err := r.gh.GetBranchSHA(ctx, staging)
+	sha, fetchBranch, err := r.fetchSHAForPickSHA(ctx, c.Key, staging, v)
 	if err != nil {
-		return errors.Wrapf(err, "fetching tip SHA for staging branch %s", staging)
+		return err
 	}
 
+	// `staging` keeps the ticket's original branch name so the Slack and
+	// Jira messages name the release series operators care about (e.g.
+	// release-26.1.0-rc), even when the actual SHA came from master via
+	// the pre-release fallback. `fetchBranch` is the branch we actually
+	// fetched and must dispatch on.
 	details, err := buildPickSHADetails(full, staging, sha, r.repo, r.buildWorkflow)
 	if err != nil {
 		return errors.Wrap(err, "building pick-SHA details")
 	}
 
-	ref := "refs/heads/" + staging
+	ref := "refs/heads/" + fetchBranch
 	// The workflow_dispatch REST endpoint requires every input value to be
 	// a JSON string — booleans typed in workflow YAML are coerced from
 	// "true"/"false" on receipt — so we stringify dry_run rather than
@@ -345,6 +357,39 @@ func (r *pickSHARunner) processCandidate(ctx context.Context, c jiraIssue) error
 	return nil
 }
 
+// fetchSHAForPickSHA fetches the tip SHA for the staging branch named on
+// the ticket. If the branch doesn't exist AND the ticket is for a
+// pre-release version (vX.Y.0-alpha/beta/rc), it falls back to master:
+// branch-cut skips Patch()==0 so the staging branch is created manually
+// around beta.1, and in that window the only sensible source for the
+// build SHA is the tip of master. The fallback is deliberately scoped to
+// pre-releases — a missing staging branch on a patch release means
+// branch-cut never ran, which is a real error operators should see.
+//
+// Returns the SHA and the name of the branch it was actually fetched
+// from (which the caller must use to construct the workflow_dispatch
+// ref). The original `staging` value is preserved in the caller's
+// scope for display in Slack/Jira so operators see the release-series
+// branch even when the SHA came from master.
+func (r *pickSHARunner) fetchSHAForPickSHA(
+	ctx context.Context, key, staging string, v version.Version,
+) (sha, fetchBranch string, err error) {
+	sha, err = r.gh.GetBranchSHA(ctx, staging)
+	if err == nil {
+		return sha, staging, nil
+	}
+	if !errors.Is(err, errBranchNotFound) || !v.IsPrerelease() {
+		return "", "", errors.Wrapf(err, "fetching tip SHA for staging branch %s", staging)
+	}
+	log.Printf("ticket %s: staging branch %s does not exist; falling back to %s for pre-release %s",
+		key, staging, masterBranch, v)
+	sha, err = r.gh.GetBranchSHA(ctx, masterBranch)
+	if err != nil {
+		return "", "", errors.Wrapf(err, "fetching tip SHA for fallback branch %s", masterBranch)
+	}
+	return sha, masterBranch, nil
+}
+
 // notifyReleaseNotes builds the release-notes API payload from the Jira
 // issue and POSTs it. Returns the generated docs PR URL on success, or
 // "" on dry-run, idempotency skip, or any error (after warning to

pkg/cmd/release/pick_sha_test.go

@@ -18,6 +18,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -465,6 +466,100 @@ func TestPickSHARunnerProcessCandidateIdempotent(t *testing.T) {
 		"GitHub must not be called when Pick SHA subtask is already Done")
 }
 
+// TestFetchSHAForPickSHA exercises the pre-release branch-not-found
+// fallback. The critical invariant is the GA-release guard: a missing
+// staging branch on a non-pre-release version must surface as an error,
+// because it means branch-cut never ran. The fallback must also be
+// scoped to errBranchNotFound — other GitHub errors (network, auth, 5xx)
+// must propagate unchanged so operators don't silently build against
+// master when GitHub is misbehaving.
+func TestFetchSHAForPickSHA(t *testing.T) {
+	const stagingSHA = "stagingsha111"
+	const masterSHA = "mastersha222"
+
+	tests := []struct {
+		name              string
+		staging           string
+		summary           string
+		ghRefs            map[string]string // branches that exist on GitHub
+		expectedSHA       string
+		expectedFetchedOn string
+		expectErrContain  string
+	}{
+		{
+			name:              "staging branch exists, no fallback",
+			staging:           "release-25.4.3-rc",
+			summary:           "Release: v25.4.3",
+			ghRefs:            map[string]string{"release-25.4.3-rc": stagingSHA},
+			expectedSHA:       stagingSHA,
+			expectedFetchedOn: "release-25.4.3-rc",
+		},
+		{
+			name:              "pre-release with missing staging branch falls back to master",
+			staging:           "release-26.1.0-rc",
+			summary:           "Release: v26.1.0-rc.1",
+			ghRefs:            map[string]string{masterBranch: masterSHA},
+			expectedSHA:       masterSHA,
+			expectedFetchedOn: masterBranch,
+		},
+		{
+			name:             "non-pre-release with missing staging branch surfaces error",
+			staging:          "release-25.4.3-rc",
+			summary:          "Release: v25.4.3",
+			ghRefs:           map[string]string{masterBranch: masterSHA},
+			expectErrContain: "fetching tip SHA for staging branch release-25.4.3-rc",
+		},
+		{
+			name:             "pre-release with master also missing surfaces fallback error",
+			staging:          "release-26.1.0-rc",
+			summary:          "Release: v26.1.0-rc.1",
+			ghRefs:           map[string]string{}, // neither branch exists
+			expectErrContain: "fetching tip SHA for fallback branch master",
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			ghSrv := httptest.NewServer(mkRefHandler(tc.ghRefs))
+			defer ghSrv.Close()
+			r := newPickSHARunnerForTest(t, ghSrv, nil, time.Time{})
+
+			v, err := parseReleaseVersion(tc.summary)
+			require.NoError(t, err)
+
+			sha, fetchBranch, err := r.fetchSHAForPickSHA(
+				context.Background(), "REL-1", tc.staging, v)
+			if tc.expectErrContain != "" {
+				require.ErrorContains(t, err, tc.expectErrContain)
+				return
+			}
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedSHA, sha)
+			require.Equal(t, tc.expectedFetchedOn, fetchBranch)
+		})
+	}
+}
+
+// TestFetchSHAForPickSHANonNotFoundErrorPropagates ensures the
+// pre-release fallback does not paper over transport/server errors:
+// only errBranchNotFound triggers fallback. A 5xx must propagate.
+func TestFetchSHAForPickSHANonNotFoundErrorPropagates(t *testing.T) {
+	ghSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer ghSrv.Close()
+	r := newPickSHARunnerForTest(t, ghSrv, nil, time.Time{})
+
+	v, err := parseReleaseVersion("Release: v26.1.0-rc.1")
+	require.NoError(t, err)
+
+	_, _, err = r.fetchSHAForPickSHA(
+		context.Background(), "REL-1", "release-26.1.0-rc", v)
+	require.Error(t, err)
+	require.False(t, errors.Is(err, errBranchNotFound),
+		"500 must not be wrapped as errBranchNotFound")
+	require.ErrorContains(t, err, "fetching tip SHA for staging branch")
+}
+
 // mkPickSHACandidate builds a candidate jiraIssue suitable for the JQL search
 // result: summary parses, pickDate is set, and no further fields are needed
 // because processCandidate refetches the full issue via Jira.
@@ -508,7 +603,9 @@ func newPickSHARunnerForTest(
 	gh.client.BaseURL = u
 
 	jira := newJiraClient("bot@example.com", "test-token")
-	jira.baseURL = jiraSrv.URL
+	if jiraSrv != nil {
+		jira.baseURL = jiraSrv.URL
+	}
 
 	return &pickSHARunner{
 		jira:          jira,