fix: A green Unity package release pull request is merged even when GitHub lists its finished checks late

.github/workflows/release-please.yml

@@ -143,7 +143,9 @@ jobs:
       # dispatcher release pull request is open, and while the pin at its head
       # still disagrees with the dispatcher version the base branch manifest
       # releases. Both windows would otherwise ship a package that pins an
-      # older dispatcher than the one main is about to release.
+      # older dispatcher than the one main is about to release. The checks grace
+      # covers the run listing lagging the completion the previous step just
+      # watched; without it a green head is left draft until the next push.
       - name: Merge the Unity package release pull request when no dispatcher release is pending
         if: steps.target.outputs.branch == 'main' && steps.release_commit.outputs.skip != 'true' && steps.package_release_sync.outputs.ready != 'false' && steps.release.outputs.prs_created == 'true' && !cancelled() && (steps.release_pr_checks.outcome == 'success' || steps.release_pr_checks.outcome == 'failure')
         working-directory: cli/release-automation
@@ -157,3 +159,4 @@ jobs:
           --base-branch "${TARGET_BRANCH}"
           --require-no-open-dispatcher-pr
           --no-wait
+          --checks-grace-seconds 120

cli/release-automation/internal/automation/package_release_pr_merge.go

@@ -22,6 +22,7 @@ type mergePackageReleasePRConfig struct {
 	dispatcherTag             string
 	requireNoOpenDispatcherPR bool
 	waitForMergeable          bool
+	checksGrace               time.Duration
 	workflows                 []string
 	timeout                   time.Duration
 	interval                  time.Duration
@@ -33,6 +34,17 @@ type mergePackageReleasePRDeps struct {
 	runOutput func(context.Context, string, ...string) (string, error)
 }
 
+// mergePackageReleasePRPassOutcome is what one pass of the wait loop
+// concluded. The two waiting outcomes are kept apart because only a head whose
+// check runs are not visible yet can be resolved within seconds.
+type mergePackageReleasePRPassOutcome int
+
+const (
+	mergePackageReleasePRPassSettled mergePackageReleasePRPassOutcome = iota
+	mergePackageReleasePRPassAwaitingPin
+	mergePackageReleasePRPassAwaitingChecks
+)
+
 type mergePackageReleasePullRequest struct {
 	Number      int    `json:"number"`
 	HeadRefName string `json:"headRefName"`
@@ -110,6 +122,7 @@ func parseMergePackageReleasePRFlags(args []string) (mergePackageReleasePRConfig
 	dispatcherTag := flagSet.String("dispatcher-tag", "", "dispatcher release tag the package pin must record (default: the tag the base branch manifest releases)")
 	requireNoOpenDispatcherPR := flagSet.Bool("require-no-open-dispatcher-pr", false, "refuse to merge while a dispatcher release pull request is open")
 	noWait := flagSet.Bool("no-wait", false, "decide once and leave the pull request draft instead of polling")
+	checksGraceSeconds := flagSet.Int("checks-grace-seconds", 0, "with --no-wait, keep re-reading for this long while the head's check runs are not listed as finished yet")
 	timeoutMinutes := flagSet.Int("timeout-minutes", defaultMergePackageReleasePRTimeoutMinutes, "how long to wait for the pull request to become mergeable")
 	intervalSeconds := flagSet.Int("interval-seconds", defaultMergePackageReleasePRIntervalSeconds, "how long to wait between polls")
 	err := flagSet.Parse(args)
@@ -126,6 +139,9 @@ func parseMergePackageReleasePRFlags(args []string) (mergePackageReleasePRConfig
 	if *timeoutMinutes <= 0 || *intervalSeconds <= 0 {
 		return mergePackageReleasePRConfig{}, fmt.Errorf("%s: --timeout-minutes and --interval-seconds must be positive", mergePackageReleasePRCommandName)
 	}
+	if *checksGraceSeconds < 0 {
+		return mergePackageReleasePRConfig{}, fmt.Errorf("%s: --checks-grace-seconds must not be negative", mergePackageReleasePRCommandName)
+	}
 
 	workflows, err := releasePRCheckWorkflowsFromEnvironment()
 	if err != nil {
@@ -138,6 +154,7 @@ func parseMergePackageReleasePRFlags(args []string) (mergePackageReleasePRConfig
 		dispatcherTag:             *dispatcherTag,
 		requireNoOpenDispatcherPR: *requireNoOpenDispatcherPR,
 		waitForMergeable:          !*noWait,
+		checksGrace:               time.Duration(*checksGraceSeconds) * time.Second,
 		workflows:                 workflows,
 		timeout:                   time.Duration(*timeoutMinutes) * time.Minute,
 		interval:                  time.Duration(*intervalSeconds) * time.Second,
@@ -152,19 +169,23 @@ func waitAndMergePackageReleasePR(
 	deps mergePackageReleasePRDeps,
 ) int {
 	deadline := deps.now().Add(config.timeout)
+	checksGraceDeadline := deps.now().Add(config.checksGrace)
 	for {
-		settled, exitCode := attemptMergePackageReleasePR(ctx, stdout, stderr, config, deps)
-		if settled {
+		outcome, exitCode := attemptMergePackageReleasePR(ctx, stdout, stderr, config, deps)
+		if outcome == mergePackageReleasePRPassSettled {
 			return exitCode
 		}
 		// The release-please path runs on every push to the base branch, so a
 		// pull request that is not mergeable now is reconsidered by the next
-		// push rather than being waited out inside one workflow run.
-		if !config.waitForMergeable {
+		// push rather than being waited out inside one workflow run. The one
+		// exception is a head whose runs finished moments ago: the run listing
+		// lags their completion by tens of seconds, and the next push may never
+		// come, so that case alone is re-read until the checks grace runs out.
+		if !config.waitForMergeable && !withinChecksGrace(outcome, checksGraceDeadline, deps) {
 			writeMergePackageReleasePRLine(stdout, "The Unity package release pull request is not mergeable yet; leaving it draft.")
 			return 0
 		}
-		if !deps.now().Before(deadline) {
+		if config.waitForMergeable && !deps.now().Before(deadline) {
 			writeMergePackageReleasePRLine(stderr, fmt.Errorf(
 				"%s: waiting for the Unity package release pull request to record %s timed out; merge the pull request by hand once its checks pass",
 				mergePackageReleasePRCommandName, mergePackageReleasePRAwaitedTag(config)))
@@ -178,6 +199,14 @@ func waitAndMergePackageReleasePR(
 	}
 }
 
+func withinChecksGrace(
+	outcome mergePackageReleasePRPassOutcome,
+	checksGraceDeadline time.Time,
+	deps mergePackageReleasePRDeps,
+) bool {
+	return outcome == mergePackageReleasePRPassAwaitingChecks && deps.now().Before(checksGraceDeadline)
+}
+
 // mergePackageReleasePRAwaitedTag names what the wait was for. The tag is
 // resolved per pass, so the timeout report has to describe the source rather
 // than a value when the caller named no tag.
@@ -188,69 +217,69 @@ func mergePackageReleasePRAwaitedTag(config mergePackageReleasePRConfig) string
 	return "the dispatcher release the " + config.baseBranch + " manifest publishes"
 }
 
-// attemptMergePackageReleasePR runs one pass of the wait loop. settled is false
-// while the pull request is still expected to reach a mergeable state, so the
-// caller keeps polling until the deadline.
+// attemptMergePackageReleasePR runs one pass of the wait loop. The outcome is
+// a waiting one while the pull request is still expected to reach a mergeable
+// state, so the caller keeps polling until the deadline.
 func attemptMergePackageReleasePR(
 	ctx context.Context,
 	stdout io.Writer,
 	stderr io.Writer,
 	config mergePackageReleasePRConfig,
 	deps mergePackageReleasePRDeps,
-) (settled bool, exitCode int) {
+) (outcome mergePackageReleasePRPassOutcome, exitCode int) {
 	releasePR, found, err := findPackageReleasePullRequest(ctx, config, deps)
 	if err != nil {
 		writeMergePackageReleasePRLine(stderr, err)
-		return true, 1
+		return mergePackageReleasePRPassSettled, 1
 	}
 	if !found {
 		writeMergePackageReleasePRLine(stdout, "No open Unity package release pull request; nothing to merge.")
-		return true, 0
+		return mergePackageReleasePRPassSettled, 0
 	}
 
 	if config.requireNoOpenDispatcherPR {
 		dispatcherPRIsOpen, err := openDispatcherReleasePullRequestExists(ctx, deps.runOutput, config.repository, config.baseBranch)
 		if err != nil {
 			writeMergePackageReleasePRLine(stderr, err)
-			return true, 1
+			return mergePackageReleasePRPassSettled, 1
 		}
 		// Waiting here cannot help: the dispatcher release has to be merged,
 		// published and stamped, which takes far longer than this command runs.
 		if dispatcherPRIsOpen {
 			writeMergePackageReleasePRLine(stdout, fmt.Sprintf(
 				"PR #%d stays draft: a dispatcher release pull request is open; the dispatcher must be released and stamped first.",
 				releasePR.Number))
-			return true, 0
+			return mergePackageReleasePRPassSettled, 0
 		}
 	}
 
 	dispatcherTag, err := mergePackageReleasePRDispatcherTag(ctx, stdout, config, deps)
 	if err != nil {
 		writeMergePackageReleasePRLine(stderr, err)
-		return true, 1
+		return mergePackageReleasePRPassSettled, 1
 	}
 
 	pinnedTag, err := packageReleasePullRequestPinnedTag(ctx, config, releasePR, deps)
 	if err != nil {
 		writeMergePackageReleasePRLine(stderr, err)
-		return true, 1
+		return mergePackageReleasePRPassSettled, 1
 	}
 	if pinnedTag != dispatcherTag {
 		writeMergePackageReleasePRLine(stdout, fmt.Sprintf(
 			"PR #%d head %s still pins %s; waiting for release-please to rebase it onto the stamp.",
 			releasePR.Number, releasePR.HeadRefOID, pinnedTag))
-		return false, 0
+		return mergePackageReleasePRPassAwaitingPin, 0
 	}
 	// The draft flag is not consulted as evidence: it exists to keep people from
 	// merging this pull request, not to prove its checks ran. What proves that
 	// is a successful run of every required workflow for this exact head SHA,
 	// held to that head by --match-head-commit at the merge.
 	checksPassed, exitCode := packageReleasePullRequestChecksPassed(ctx, stdout, stderr, config, releasePR, deps)
 	if exitCode != 0 {
-		return true, exitCode
+		return mergePackageReleasePRPassSettled, exitCode
 	}
 	if !checksPassed {
-		return false, 0
+		return mergePackageReleasePRPassAwaitingChecks, 0
 	}
 
 	return readyAndMergePackageReleasePR(ctx, stdout, stderr, config, releasePR, pinnedTag, deps)

cli/release-automation/internal/automation/package_release_pr_merge_test.go

@@ -373,6 +373,79 @@ func TestMergePackageReleasePRWithNoWaitLeavesStalePinDraftAfterOnePass(t *testi
 	assertMergePackageReleasePRListCount(t, stub, 1)
 }
 
+// Verifies --no-wait with a checks grace re-reads a head whose finished runs the run listing does not show yet, and merges once they appear.
+func TestMergePackageReleasePRWithNoWaitMergesWhenChecksAppearWithinGrace(t *testing.T) {
+	exitCode, stdout, stderr, stub := runMergePackageReleasePRCaseWithArgs(t, []mergePackageReleasePRPoll{
+		{
+			prListJSON:      packageReleasePRListJSON("package123", true),
+			pinnedTagsByRef: packageReleasePRPinAt("package123", "dispatcher-v3.4.0"),
+			runs:            map[string]string{},
+		},
+		{
+			prListJSON:      packageReleasePRListJSON("package123", true),
+			pinnedTagsByRef: packageReleasePRPinAt("package123", "dispatcher-v3.4.0"),
+			runs:            packageReleasePRRunsAt("package123"),
+		},
+	}, []string{"--dispatcher-tag", "dispatcher-v3.4.0", "--no-wait", "--checks-grace-seconds", "90"})
+
+	if exitCode != 0 {
+		t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr)
+	}
+	assertReleasePRCheckLogContains(t, stdout, "has not finished for this head; waiting.")
+	assertMergePackageReleasePRMergeCount(t, stub, "package123", 1)
+	assertMergePackageReleasePRListCount(t, stub, 2)
+}
+
+// Verifies --no-wait with a checks grace still leaves the pull request draft, without failing the job, when the runs never appear within the grace.
+func TestMergePackageReleasePRWithNoWaitLeavesDraftWhenChecksStayMissingPastGrace(t *testing.T) {
+	exitCode, stdout, stderr, stub := runMergePackageReleasePRCaseWithArgs(t, []mergePackageReleasePRPoll{
+		{
+			prListJSON:      packageReleasePRListJSON("package123", true),
+			pinnedTagsByRef: packageReleasePRPinAt("package123", "dispatcher-v3.4.0"),
+			runs:            map[string]string{},
+		},
+	}, []string{"--dispatcher-tag", "dispatcher-v3.4.0", "--no-wait", "--checks-grace-seconds", "90"})
+
+	if exitCode != 0 {
+		t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr)
+	}
+	assertReleasePRCheckLogContains(t, stdout, "is not mergeable yet; leaving it draft.")
+	assertMergePackageReleasePRNeverMerged(t, stub)
+	// The 30-second interval fits three re-reads into the 90-second grace after the first pass.
+	assertMergePackageReleasePRListCount(t, stub, 4)
+}
+
+// Verifies the checks grace does not apply to a stale pin: the stamp takes far longer than the grace, so --no-wait still decides in one pass.
+func TestMergePackageReleasePRWithNoWaitIgnoresChecksGraceForStalePin(t *testing.T) {
+	exitCode, stdout, stderr, stub := runMergePackageReleasePRCaseWithArgs(t, []mergePackageReleasePRPoll{
+		{
+			prListJSON:      packageReleasePRListJSON("package123", true),
+			pinnedTagsByRef: packageReleasePRPinAt("package123", "dispatcher-v3.3.1"),
+			runs:            packageReleasePRRunsAt("package123"),
+		},
+	}, []string{"--dispatcher-tag", "dispatcher-v3.4.0", "--no-wait", "--checks-grace-seconds", "90"})
+
+	if exitCode != 0 {
+		t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr)
+	}
+	assertReleasePRCheckLogContains(t, stdout, "is not mergeable yet; leaving it draft.")
+	assertMergePackageReleasePRNeverMerged(t, stub)
+	assertMergePackageReleasePRListCount(t, stub, 1)
+}
+
+// Verifies a negative checks grace is rejected rather than silently treated as no grace.
+func TestMergePackageReleasePRRejectsNegativeChecksGrace(t *testing.T) {
+	exitCode, _, stderr, stub := runMergePackageReleasePRCaseWithArgs(t, []mergePackageReleasePRPoll{
+		{prListJSON: packageReleasePRListJSON("package123", true)},
+	}, []string{"--dispatcher-tag", "dispatcher-v3.4.0", "--no-wait", "--checks-grace-seconds", "-1"})
+
+	if exitCode != 1 {
+		t.Fatalf("expected exit code 1, got %d", exitCode)
+	}
+	assertReleasePRCheckLogContains(t, stderr, "--checks-grace-seconds must not be negative")
+	assertMergePackageReleasePRListCount(t, stub, 0)
+}
+
 // Verifies a failed merge is a success rather than a failed job when the pull request is already merged: the command reports it and exits 0.
 func TestMergePackageReleasePRAcceptsAPullRequestAnotherRunMerged(t *testing.T) {
 	exitCode, stdout, stderr, stub := runMergePackageReleasePRCase(t, []mergePackageReleasePRPoll{

cli/release-automation/internal/automation/package_release_pr_ready_merge.go

@@ -45,21 +45,21 @@ func readyAndMergePackageReleasePR(
 	releasePR mergePackageReleasePullRequest,
 	pinnedTag string,
 	deps mergePackageReleasePRDeps,
-) (settled bool, exitCode int) {
+) (outcome mergePackageReleasePRPassOutcome, exitCode int) {
 	if releasePR.IsDraft {
 		settled, exitCode := readyPackageReleasePRBeforeMerge(ctx, stdout, stderr, config, releasePR, deps)
 		if settled {
-			return true, exitCode
+			return mergePackageReleasePRPassSettled, exitCode
 		}
 	}
 
 	mergeErr := squashMergePackageReleasePR(ctx, config, releasePR, deps)
 	if mergeErr != nil {
-		return true, resolvePackageReleasePRMergeFailure(ctx, stdout, stderr, config, releasePR, mergeErr, deps)
+		return mergePackageReleasePRPassSettled, resolvePackageReleasePRMergeFailure(ctx, stdout, stderr, config, releasePR, mergeErr, deps)
 	}
 	writeMergePackageReleasePRLine(stdout, fmt.Sprintf(
 		"Merged Unity package release PR #%d at %s; it pins %s.", releasePR.Number, releasePR.HeadRefOID, pinnedTag))
-	return true, 0
+	return mergePackageReleasePRPassSettled, 0
 }
 
 // readyPackageReleasePRBeforeMerge lifts the draft. settled is false only when

docs/dispatcher-pin-release-order.md

@@ -46,6 +46,10 @@ and is merged only by automation; the other two are the ones people merge.
    dispatcher release pull request is open **and** the pin at the package head
    already records the dispatcher version `main`'s manifest releases. Otherwise
    it leaves the pull request draft and reconsiders on the next push to `main`.
+   The one wait it does allow is `--checks-grace-seconds`: GitHub's per-workflow
+   run listing can show a run as unfinished for tens of seconds after it
+   completed, so a head whose checks are not yet listed as finished is re-read
+   for up to two minutes before the pull request is left draft.
 4. The **project runner** release pull request carries no cross-component
    ordering constraint and can be merged at any time.