From 88aa519d49dee7657c88e967b99d92abe197a67a Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 29 Oct 2026 23:51:18 +0900 Subject: [PATCH 1/6] Hold the package release until the project runner it pins is released The package release pull request was merged as soon as it pinned the published dispatcher, even while the project runner release pull request was still open. The runner release raises the runner the package pins, so a package cut first shipped the previous runner (3.21.2 pinned 3.12.2 instead of 3.12.3). The merge now leaves the pull request draft, settling the pass, while a runner release pull request is open or the runner the pin names is not published. Settling rather than polling is deliberate: the runner is published by another, approval-gated workflow, which dispatches release-please afterwards so the merge step decides again. A missing release reads as unpublished, but any other read failure fails the command so an outage never holds the release silently. --- .../automation/package_pin_consistency.go | 1 + .../automation/package_release_pr_gate.go | 111 ++++++++++++- .../automation/package_release_pr_merge.go | 32 ++-- .../package_release_pr_merge_test.go | 154 +++++++++++++++++- 4 files changed, 280 insertions(+), 18 deletions(-) diff --git a/cli/release-automation/internal/automation/package_pin_consistency.go b/cli/release-automation/internal/automation/package_pin_consistency.go index af321e0353..82f7e1a92d 100644 --- a/cli/release-automation/internal/automation/package_pin_consistency.go +++ b/cli/release-automation/internal/automation/package_pin_consistency.go @@ -22,6 +22,7 @@ type packagePinConsistencyConfig struct { type packagePinConsistencyPin struct { DispatcherReleaseTag string `json:"dispatcherReleaseTag"` + ProjectRunnerVersion string `json:"projectRunnerVersion"` } // RunPackagePinConsistencyCheck fails when a ref would release the Unity diff --git a/cli/release-automation/internal/automation/package_release_pr_gate.go b/cli/release-automation/internal/automation/package_release_pr_gate.go index 01788262f6..089c28b643 100644 --- a/cli/release-automation/internal/automation/package_release_pr_gate.go +++ b/cli/release-automation/internal/automation/package_release_pr_gate.go @@ -5,11 +5,15 @@ import ( "encoding/base64" "encoding/json" "fmt" + "io" "net/url" "strings" ) -const dispatcherReleasePRComponent = "dispatcher" +const ( + dispatcherReleasePRComponent = "dispatcher" + projectRunnerReleasePRComponent = "uloop-project-runner" +) // githubFileContentAtRef reads a repository file through the contents API at an // exact ref. The merge gate reasons about commits that are not in this @@ -92,7 +96,31 @@ func openDispatcherReleasePullRequestExists( repository string, baseBranch string, ) (bool, error) { - headBranch := "release-please--branches--" + baseBranch + "--components--" + dispatcherReleasePRComponent + return openReleasePullRequestExists(ctx, runOutput, repository, baseBranch, dispatcherReleasePRComponent) +} + +// openProjectRunnerReleasePullRequestExists reports whether a project runner +// release is still pending. Its release pull request raises the runner the +// package pins, so a package cut before it ships the previous runner. +func openProjectRunnerReleasePullRequestExists( + ctx context.Context, + runOutput func(context.Context, string, ...string) (string, error), + repository string, + baseBranch string, +) (bool, error) { + return openReleasePullRequestExists(ctx, runOutput, repository, baseBranch, projectRunnerReleasePRComponent) +} + +// openReleasePullRequestExists reports whether the release pull request of one +// component is open, found by the head branch release-please names after it. +func openReleasePullRequestExists( + ctx context.Context, + runOutput func(context.Context, string, ...string) (string, error), + repository string, + baseBranch string, + component string, +) (bool, error) { + headBranch := "release-please--branches--" + baseBranch + "--components--" + component output, err := runOutput( ctx, "gh", @@ -116,8 +144,83 @@ func openDispatcherReleasePullRequestExists( releasePRs := []mergePackageReleasePullRequest{} err = json.Unmarshal([]byte(output), &releasePRs) if err != nil { - return false, fmt.Errorf("%s: failed to parse dispatcher release PR list: %w", - mergePackageReleasePRCommandName, err) + return false, fmt.Errorf("%s: failed to parse %s release PR list: %w", + mergePackageReleasePRCommandName, component, err) } return len(releasePRs) > 0, nil } + +// releasePublished reports whether a GitHub release exists for the tag and is +// no longer a draft. A missing release reads as unpublished, but any other +// failure is returned: reading an API outage as "unpublished" would hold a +// release back for good without anyone being told. +func releasePublished( + ctx context.Context, + runOutput func(context.Context, string, ...string) (string, error), + repository string, + tag string, +) (bool, error) { + output, err := runOutput(ctx, "gh", "release", "view", tag, "--repo", repository, "--json", "isDraft") + if err != nil { + if releaseNotFound(err) { + return false, nil + } + return false, err + } + + view := struct { + IsDraft bool `json:"isDraft"` + }{} + err = json.Unmarshal([]byte(output), &view) + if err != nil { + return false, fmt.Errorf("%s: failed to parse release %s: %w", mergePackageReleasePRCommandName, tag, err) + } + return !view.IsDraft, nil +} + +func releaseNotFound(err error) bool { + message := err.Error() + return strings.Contains(message, "release not found") || + strings.Contains(message, "HTTP 404") || + strings.Contains(message, "Not Found") +} + +// projectRunnerReleaseGate holds the package release pull request until the +// project runner its pin names is released. Holding settles the pass rather +// than polling: the runner is published by another, approval-gated workflow, +// whose post-publish dispatches release-please, and that run's merge step +// decides again. +func projectRunnerReleaseGate( + ctx context.Context, + stdout io.Writer, + stderr io.Writer, + config mergePackageReleasePRConfig, + releasePR mergePackageReleasePullRequest, + pin packagePinConsistencyPin, + deps mergePackageReleasePRDeps, +) (held bool, exitCode int) { + runnerPRIsOpen, err := openProjectRunnerReleasePullRequestExists(ctx, deps.runOutput, config.repository, config.baseBranch) + if err != nil { + writeMergePackageReleasePRLine(stderr, err) + return true, 1 + } + if runnerPRIsOpen { + writeMergePackageReleasePRLine(stdout, fmt.Sprintf( + "PR #%d stays draft: a project runner release pull request is open; the project runner must be released first so the package pins it.", + releasePR.Number)) + return true, 0 + } + + published, err := releasePublished(ctx, deps.runOutput, config.repository, projectRunnerReleaseTagPrefix+pin.ProjectRunnerVersion) + if err != nil { + writeMergePackageReleasePRLine(stderr, err) + return true, 1 + } + if !published { + writeMergePackageReleasePRLine(stdout, fmt.Sprintf( + "PR #%d head %s pins project runner %s, which is not published yet; waiting for the project runner release.", + releasePR.Number, releasePR.HeadRefOID, pin.ProjectRunnerVersion)) + return true, 0 + } + return false, 0 +} diff --git a/cli/release-automation/internal/automation/package_release_pr_merge.go b/cli/release-automation/internal/automation/package_release_pr_merge.go index eaf1c71bf3..c8d6c7ef4d 100644 --- a/cli/release-automation/internal/automation/package_release_pr_merge.go +++ b/cli/release-automation/internal/automation/package_release_pr_merge.go @@ -259,17 +259,25 @@ func attemptMergePackageReleasePR( return mergePackageReleasePRPassSettled, 1 } - pinnedTag, err := packageReleasePullRequestPinnedTag(ctx, config, releasePR, deps) + pin, err := packageReleasePullRequestPin(ctx, config, releasePR, deps) if err != nil { writeMergePackageReleasePRLine(stderr, err) return mergePackageReleasePRPassSettled, 1 } + pinnedTag := pin.DispatcherReleaseTag 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 mergePackageReleasePRPassAwaitingPin, 0 } + // The runner is judged only on a head that records the current dispatcher: + // a head from before the stamp may still name an older runner, and + // release-please rebuilds it before this pass can merge anyway. + held, exitCode := projectRunnerReleaseGate(ctx, stdout, stderr, config, releasePR, pin, deps) + if held { + return mergePackageReleasePRPassSettled, exitCode + } // 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, @@ -338,32 +346,36 @@ func findPackageReleasePullRequest( return matchingPRs[0], true, nil } -// packageReleasePullRequestPinnedTag reads the pin at the pull request head -// rather than in the checkout: the merge decision is about the commit that will -// carry the package tag, not about the workspace this command runs in. -func packageReleasePullRequestPinnedTag( +// packageReleasePullRequestPin reads the pin at the pull request head rather +// than in the checkout: the merge decision is about the commit that will carry +// the package tag, not about the workspace this command runs in. +func packageReleasePullRequestPin( ctx context.Context, config mergePackageReleasePRConfig, releasePR mergePackageReleasePullRequest, deps mergePackageReleasePRDeps, -) (string, error) { +) (packagePinConsistencyPin, error) { decoded, err := githubFileContentAtRef( ctx, deps.runOutput, config.repository, unityPackageCliPinFile, releasePR.HeadRefOID) if err != nil { - return "", err + return packagePinConsistencyPin{}, err } pin := packagePinConsistencyPin{} err = json.Unmarshal(decoded, &pin) if err != nil { - return "", fmt.Errorf("%s: failed to parse %s at %s: %w", + return packagePinConsistencyPin{}, fmt.Errorf("%s: failed to parse %s at %s: %w", mergePackageReleasePRCommandName, unityPackageCliPinFile, releasePR.HeadRefOID, err) } if pin.DispatcherReleaseTag == "" { - return "", fmt.Errorf("%s: %s at %s has no dispatcherReleaseTag", + return packagePinConsistencyPin{}, fmt.Errorf("%s: %s at %s has no dispatcherReleaseTag", + mergePackageReleasePRCommandName, unityPackageCliPinFile, releasePR.HeadRefOID) + } + if pin.ProjectRunnerVersion == "" { + return packagePinConsistencyPin{}, fmt.Errorf("%s: %s at %s has no projectRunnerVersion", mergePackageReleasePRCommandName, unityPackageCliPinFile, releasePR.HeadRefOID) } - return pin.DispatcherReleaseTag, nil + return pin, nil } // packageReleasePullRequestChecksPassed requires a completed successful run of diff --git a/cli/release-automation/internal/automation/package_release_pr_merge_test.go b/cli/release-automation/internal/automation/package_release_pr_merge_test.go index 6452d27ddb..331b6c2549 100644 --- a/cli/release-automation/internal/automation/package_release_pr_merge_test.go +++ b/cli/release-automation/internal/automation/package_release_pr_merge_test.go @@ -28,6 +28,13 @@ type mergePackageReleasePRPoll struct { // openDispatcherPRListJSON answers the listing that asks whether a // dispatcher release is still pending. openDispatcherPRListJSON string + // openProjectRunnerPRListJSON answers the listing that asks whether a + // project runner release is still pending; empty means none is open. + openProjectRunnerPRListJSON string + // projectRunnerReleaseViewJSON and projectRunnerReleaseViewErr answer the + // release read for the runner the pin names; empty JSON means published. + projectRunnerReleaseViewJSON string + projectRunnerReleaseViewErr error // manifestByRef answers the contents API for the release manifest, so a // test can pin the dispatcher version the base branch tip releases. manifestByRef map[string]string @@ -88,6 +95,20 @@ func (stub *mergePackageReleasePRStub) runOutput(ctx context.Context, name strin // content and run reads. func (stub *mergePackageReleasePRStub) answerPullRequestCommand(commandLine string) (string, bool, error) { switch { + case strings.Contains(commandLine, "--components--uloop-project-runner"): + // Like the dispatcher listing, this is a gate question inside one pass. + if stub.activePoll.openProjectRunnerPRListJSON == "" { + return "[]", true, nil + } + return stub.activePoll.openProjectRunnerPRListJSON, true, nil + case strings.HasPrefix(commandLine, "gh release view "): + if stub.activePoll.projectRunnerReleaseViewErr != nil { + return "", true, stub.activePoll.projectRunnerReleaseViewErr + } + if stub.activePoll.projectRunnerReleaseViewJSON == "" { + return `{"isDraft":false}`, true, nil + } + return stub.activePoll.projectRunnerReleaseViewJSON, true, nil case strings.Contains(commandLine, "--components--dispatcher"): // The dispatcher listing is a gate question inside one pass, so unlike // the package listing it must not advance the stubbed state. @@ -133,7 +154,7 @@ func (stub *mergePackageReleasePRStub) answerContentCommand(commandLine string) if !known { return "", fmt.Errorf("no stubbed pin for ref %q", ref) } - pin := fmt.Sprintf(`{"dispatcherReleaseTag":%q}`, pinnedTag) + pin := fmt.Sprintf(`{"dispatcherReleaseTag":%q,"projectRunnerVersion":"3.4.0"}`, pinnedTag) return base64.StdEncoding.EncodeToString([]byte(pin)) + "\n", nil case strings.HasPrefix(commandLine, "gh run list "): for workflow, runsJSON := range stub.activePoll.runs { @@ -393,7 +414,8 @@ func TestMergePackageReleasePRWithNoWaitMergesWhenChecksAppearWithinGrace(t *tes } assertReleasePRCheckLogContains(t, stdout, "has not finished for this head; waiting.") assertMergePackageReleasePRMergeCount(t, stub, "package123", 1) - assertMergePackageReleasePRListCount(t, stub, 2) + // Each pass lists the package release pull request and then the project runner one. + assertMergePackageReleasePRListCount(t, stub, 4) } // 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. @@ -411,8 +433,9 @@ func TestMergePackageReleasePRWithNoWaitLeavesDraftWhenChecksStayMissingPastGrac } 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) + // The 30-second interval fits three re-reads into the 90-second grace after the first pass, + // and each pass lists the package release pull request and then the project runner one. + assertMergePackageReleasePRListCount(t, stub, 8) } // 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. @@ -603,6 +626,7 @@ func TestMergePackageReleasePRReadsTheManifestOnlyAfterTheDispatcherGate(t *test commandLogText := strings.Join(stub.commandLog, "\n") assertReleasePRCheckLogDoesNotContain(t, commandLogText, releasePleaseManifestRelativePath) assertReleasePRCheckLogDoesNotContain(t, stdout, "Resolved dispatcher release tag") + assertReleasePRCheckLogDoesNotContain(t, commandLogText, "--components--uloop-project-runner") assertMergePackageReleasePRNeverMerged(t, stub) } @@ -894,6 +918,11 @@ func TestMergePackageReleasePRFailsOnEachUnusableRead(t *testing.T) { {"pin encoding", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandContains(pinPath), output: "not base64!"}, "failed to decode " + unityPackageCliPinFile + " at package123"}, {"pin JSON", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandContains(pinPath), output: base64Of("{")}, "failed to parse " + unityPackageCliPinFile + " at package123"}, {"pin without tag", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandContains(pinPath), output: base64Of(`{}`)}, unityPackageCliPinFile + " at package123 has no dispatcherReleaseTag"}, + {"pin without runner version", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandContains(pinPath), output: base64Of(`{"dispatcherReleaseTag":"dispatcher-v3.4.0"}`)}, unityPackageCliPinFile + " at package123 has no projectRunnerVersion"}, + {"runner PR list", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandContains("--components--uloop-project-runner"), err: errors.New("runner PR list failed")}, "runner PR list failed"}, + {"runner PR list JSON", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandContains("--components--uloop-project-runner"), output: "{"}, "failed to parse uloop-project-runner release PR list"}, + {"runner release view", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandHasPrefix("gh release view "), err: errors.New("gh release view failed: exit status 1\nHTTP 502: Bad Gateway")}, "HTTP 502: Bad Gateway"}, + {"runner release view JSON", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandHasPrefix("gh release view "), output: "{"}, "failed to parse release uloop-project-runner-v3.4.0"}, {"workflow runs", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandHasPrefix("gh run list "), err: errors.New("workflow runs failed")}, "workflow runs failed"}, {"workflow runs JSON", nil, mergePackageReleasePROverride{matches: mergePackageReleasePRCommandHasPrefix("gh run list "), output: "{"}, "failed to parse " + packageReleasePRWorkflows()[0] + " workflow runs"}, } @@ -1007,3 +1036,120 @@ func TestRunMergePackageReleasePRReportsUnknownFlag(t *testing.T) { t.Fatalf("exit code = %d, stderr = %q", exitCode, stderr.String()) } } + +// projectRunnerReleaseMissingErr is the error runCommandOutput builds when gh reports no release for the pinned runner tag. +func projectRunnerReleaseMissingErr() error { + return errors.New("gh release view uloop-project-runner-v3.4.0 failed: exit status 1\nrelease not found") +} + +// Verifies the release-please path leaves the package release pull request draft while a project runner release pull request is open, so the package never ships the previous runner. +func TestMergePackageReleasePRLeavesDraftWhileProjectRunnerReleaseIsPending(t *testing.T) { + poll := greenDraftPackageReleasePRPoll() + poll.openProjectRunnerPRListJSON = `[{"number":2003}]` + + exitCode, stdout, stderr, stub := runMergePackageReleasePRCaseWithArgs(t, []mergePackageReleasePRPoll{poll}, + []string{"--require-no-open-dispatcher-pr", "--no-wait", "--checks-grace-seconds", "120"}) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + assertReleasePRCheckLogContains(t, stdout, "PR #2002 stays draft: a project runner release pull request is open") + assertReleasePRCheckLogDoesNotContain(t, strings.Join(stub.commandLog, "\n"), "gh pr ready") + assertMergePackageReleasePRNeverMerged(t, stub) +} + +// Verifies the post-publish path settles in one pass while a project runner release pull request is open, since only another workflow can release the runner. +func TestMergePackageReleasePRSettlesAtOnceInWaitModeWhileProjectRunnerReleaseIsPending(t *testing.T) { + poll := greenDraftPackageReleasePRPoll() + poll.openProjectRunnerPRListJSON = `[{"number":2003}]` + + exitCode, stdout, stderr, stub := runMergePackageReleasePRCaseWithArgs(t, []mergePackageReleasePRPoll{poll}, + []string{"--dispatcher-tag", "dispatcher-v3.4.0"}) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + assertReleasePRCheckLogContains(t, stdout, "a project runner release pull request is open") + assertMergePackageReleasePRListCount(t, stub, 2) + assertMergePackageReleasePRNeverMerged(t, stub) +} + +// Verifies the package release pull request stays draft while the release of the runner its pin names is still a draft. +func TestMergePackageReleasePRLeavesDraftWhilePinnedProjectRunnerIsDraft(t *testing.T) { + poll := greenDraftPackageReleasePRPoll() + poll.projectRunnerReleaseViewJSON = `{"isDraft":true}` + + exitCode, stdout, stderr, stub := runMergePackageReleasePRCaseWithArgs(t, []mergePackageReleasePRPoll{poll}, + []string{"--require-no-open-dispatcher-pr", "--no-wait", "--checks-grace-seconds", "120"}) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + assertReleasePRCheckLogContains(t, stdout, "pins project runner 3.4.0, which is not published yet") + assertMergePackageReleasePRNeverMerged(t, stub) +} + +// Verifies the package release pull request stays draft while no release exists yet for the runner its pin names. +func TestMergePackageReleasePRLeavesDraftWhilePinnedProjectRunnerIsMissing(t *testing.T) { + poll := greenDraftPackageReleasePRPoll() + poll.projectRunnerReleaseViewErr = projectRunnerReleaseMissingErr() + + exitCode, stdout, stderr, stub := runMergePackageReleasePRCaseWithArgs(t, []mergePackageReleasePRPoll{poll}, + []string{"--require-no-open-dispatcher-pr", "--no-wait", "--checks-grace-seconds", "120"}) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + assertReleasePRCheckLogContains(t, stdout, "pins project runner 3.4.0, which is not published yet") + assertMergePackageReleasePRNeverMerged(t, stub) +} + +// Verifies the post-publish path settles in one pass with exit 0 when the pinned runner is merged but awaiting approval, instead of polling until the 45-minute timeout fails it. +func TestMergePackageReleasePRSettlesAtOnceInWaitModeWhilePinnedProjectRunnerIsUnpublished(t *testing.T) { + poll := greenDraftPackageReleasePRPoll() + poll.projectRunnerReleaseViewJSON = `{"isDraft":true}` + + exitCode, stdout, stderr, stub := runMergePackageReleasePRCaseWithArgs(t, []mergePackageReleasePRPoll{poll}, + []string{"--dispatcher-tag", "dispatcher-v3.4.0"}) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + assertReleasePRCheckLogContains(t, stdout, "which is not published yet") + assertMergePackageReleasePRListCount(t, stub, 2) + assertMergePackageReleasePRNeverMerged(t, stub) +} + +// Verifies the runner release is read only once the head pins the current dispatcher: a stale head is waited out first, and the rebased head is the one judged. +func TestMergePackageReleasePRChecksTheProjectRunnerOnlyAfterThePinIsCurrent(t *testing.T) { + stalePoll := mergePackageReleasePRPoll{ + prListJSON: packageReleasePRListJSON("stale123", true), + pinnedTagsByRef: packageReleasePRPinAt("stale123", "dispatcher-v3.3.1"), + runs: packageReleasePRRunsAt("stale123"), + } + rebasedPoll := mergePackageReleasePRPoll{ + prListJSON: packageReleasePRListJSON("rebased456", true), + pinnedTagsByRef: packageReleasePRPinAt("rebased456", "dispatcher-v3.4.0"), + runs: packageReleasePRRunsAt("rebased456"), + } + + exitCode, _, stderr, stub := runMergePackageReleasePRCase(t, []mergePackageReleasePRPoll{stalePoll, rebasedPoll}) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + firstReleaseView := -1 + rebasedPinRead := -1 + for index, commandLine := range stub.commandLog { + if rebasedPinRead == -1 && strings.Contains(commandLine, "?ref=rebased456") { + rebasedPinRead = index + } + if firstReleaseView == -1 && strings.HasPrefix(commandLine, "gh release view ") { + firstReleaseView = index + } + } + if firstReleaseView == -1 || rebasedPinRead == -1 || firstReleaseView < rebasedPinRead { + t.Fatalf("expected the runner release to be read only after the rebased pin\n%s", strings.Join(stub.commandLog, "\n")) + } + assertMergePackageReleasePRMergeCount(t, stub, "rebased456", 1) +} From 540928d48703a361b4275cf1e2ac93d355372c88 Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 29 Oct 2026 23:54:04 +0900 Subject: [PATCH 2/6] Run release-please once every manifest release is published release-please was skipped whenever the newest commit was a release commit, a stand-in for "a version the manifest names is not published yet" (#1088). The stand-in also skipped the runs the publish workflows dispatch after publishing, so after a runner release commit the remaining release pull requests were never rebuilt. check-release-please-gate reads the manifest at the target commit and skips only while one of the releases it names is missing or still a draft. A release that cannot be read fails the run: running anyway would repeat the previous changelog, and skipping silently would leave the release pull requests unbuilt. Go is now set up before the gate on every run, and the subject-line script and its test are removed. --- .github/workflows/build-and-test.yml | 1 - .github/workflows/release-please.yml | 37 +-- .../cmd/check-release-please-gate/main.go | 18 ++ .../automation/release_please_gate.go | 149 ++++++++++++ .../automation/release_please_gate_test.go | 223 ++++++++++++++++++ scripts/is-release-please-release-commit.sh | 13 - .../test-is-release-please-release-commit.sh | 48 ---- scripts/test-release-please-config.sh | 28 ++- 8 files changed, 436 insertions(+), 81 deletions(-) create mode 100644 cli/release-automation/cmd/check-release-please-gate/main.go create mode 100644 cli/release-automation/internal/automation/release_please_gate.go create mode 100644 cli/release-automation/internal/automation/release_please_gate_test.go delete mode 100755 scripts/is-release-please-release-commit.sh delete mode 100755 scripts/test-is-release-please-release-commit.sh diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index c6a199d3ee..5b28aec6bf 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -38,7 +38,6 @@ jobs: scripts/test-go-cli-toolchain.sh scripts/test-resolve-native-cli-release-target.sh scripts/test-resolve-dispatcher-release-target.sh - scripts/test-is-release-please-release-commit.sh scripts/test-release-please-config.sh scripts/test-protocol-minimum-version-workflow.sh scripts/test-security-scan-workflow.sh diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index 6c4cf668e8..b4341bf967 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -47,16 +47,28 @@ jobs: echo "branch=${TARGET_BRANCH}" >> "$GITHUB_OUTPUT" echo "sha=${TARGET_SHA}" >> "$GITHUB_OUTPUT" - - name: 🧭 Detect release commit + - name: Setup Go for release automation + uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c + with: + go-version-file: cli/.go-version + cache-dependency-path: '**/go.sum' + + # release-please rebuilds every release pull request from the manifest at + # HEAD and looks the previous release up by tag. While a version the + # manifest names has no published release yet (a merged release pull + # request whose publish is still pending), that rebuild would carry the + # previous changelog entries again (#1088), so the run waits until every + # named release is out. The publish workflows dispatch this workflow once + # their release is published, which is what lets the remaining release + # pull requests be rebuilt after a release commit. + - name: 🧭 Decide whether every manifest release is published id: release_commit - run: | - COMMIT_SUBJECT="$(git log -1 --format=%s)" - if scripts/is-release-please-release-commit.sh "${COMMIT_SUBJECT}"; then - echo "skip=true" >> "$GITHUB_OUTPUT" - echo "Release commit detected; release-please will wait for the next non-release commit." - else - echo "skip=false" >> "$GITHUB_OUTPUT" - fi + working-directory: cli/release-automation + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_REPOSITORY: ${{ github.repository }} + TARGET_SHA: ${{ steps.target.outputs.sha }} + run: go run ./cmd/check-release-please-gate --repo "${GITHUB_REPOSITORY}" --ref "${TARGET_SHA}" >> "$GITHUB_OUTPUT" # ── Debug: Show repository structure ───────────────────────── - name: 🕵️ Show file tree @@ -90,13 +102,6 @@ jobs: skip-github-release: true token: ${{ secrets.GITHUB_TOKEN }} - - name: Setup Go for release PR automation - if: steps.target.outputs.branch == 'main' && steps.release_commit.outputs.skip != 'true' && steps.release.outputs.prs_created == 'true' - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c - with: - go-version-file: cli/.go-version - cache-dependency-path: '**/go.sum' - - name: Dispatch release PR checks id: release_pr_checks if: steps.target.outputs.branch == 'main' && steps.release_commit.outputs.skip != 'true' && steps.release.outputs.prs_created == 'true' diff --git a/cli/release-automation/cmd/check-release-please-gate/main.go b/cli/release-automation/cmd/check-release-please-gate/main.go new file mode 100644 index 0000000000..d951234df7 --- /dev/null +++ b/cli/release-automation/cmd/check-release-please-gate/main.go @@ -0,0 +1,18 @@ +package main + +import ( + "context" + "os" + "time" + + "github.com/hatayama/unity-cli-loop/tools/release-automation/internal/automation" +) + +const releasePleaseGateCheckTimeout = 2 * time.Minute + +func main() { + ctx, cancel := context.WithTimeout(context.Background(), releasePleaseGateCheckTimeout) + exitCode := automation.RunReleasePleaseGateCheck(ctx, os.Stdout, os.Stderr, os.Args[1:]) + cancel() + os.Exit(exitCode) +} diff --git a/cli/release-automation/internal/automation/release_please_gate.go b/cli/release-automation/internal/automation/release_please_gate.go new file mode 100644 index 0000000000..3972fef253 --- /dev/null +++ b/cli/release-automation/internal/automation/release_please_gate.go @@ -0,0 +1,149 @@ +package automation + +import ( + "context" + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "sort" + "strings" +) + +const releasePleaseGateCommandName = "check-release-please-gate" + +type releasePleaseGateConfig struct { + repository string + ref string +} + +// RunReleasePleaseGateCheck writes skip=true to stdout while a version the +// release manifest at the ref names has no published release yet. release-please +// finds the previous release by tag, so rebuilding the release pull requests +// before that release is out would carry the previous changelog entries again. +func RunReleasePleaseGateCheck(ctx context.Context, stdout io.Writer, stderr io.Writer, args []string) int { + return RunReleasePleaseGateCheckWithDeps(ctx, stdout, stderr, args, runCommandOutput) +} + +func RunReleasePleaseGateCheckWithDeps( + ctx context.Context, + stdout io.Writer, + stderr io.Writer, + args []string, + runOutput func(context.Context, string, ...string) (string, error), +) int { + config, err := parseReleasePleaseGateFlags(args) + if err != nil { + writeMergePackageReleasePRLine(stderr, err) + return 1 + } + + tags, err := releasePleaseManifestReleaseTags(ctx, runOutput, config) + if err != nil { + writeMergePackageReleasePRLine(stderr, err) + return 1 + } + + // A release that cannot be read fails the run instead of choosing a side: + // running anyway repeats the previous changelog, and skipping silently would + // leave the release pull requests unbuilt for good. + unpublished := []string{} + for _, tag := range tags { + published, err := releasePublished(ctx, runOutput, config.repository, tag) + if err != nil { + writeMergePackageReleasePRLine(stderr, err) + return 1 + } + if !published { + unpublished = append(unpublished, tag) + } + } + + if len(unpublished) > 0 { + writeMergePackageReleasePRLine(stderr, fmt.Sprintf( + "release-please is skipped: %s not published yet; rebuilding the release pull requests now would repeat the previous changelog entries", + strings.Join(unpublished, ", "))) + writeMergePackageReleasePRLine(stdout, "skip=true") + return 0 + } + writeMergePackageReleasePRLine(stderr, fmt.Sprintf( + "every release the manifest at %s names is published; release-please may run", config.ref)) + writeMergePackageReleasePRLine(stdout, "skip=false") + return 0 +} + +func parseReleasePleaseGateFlags(args []string) (releasePleaseGateConfig, error) { + flagSet := flag.NewFlagSet(releasePleaseGateCommandName, flag.ContinueOnError) + flagSet.SetOutput(io.Discard) + repository := flagSet.String("repo", "", "GitHub repository whose releases are checked") + ref := flagSet.String("ref", "", "commit SHA or branch whose release manifest is read") + err := flagSet.Parse(args) + if err != nil { + return releasePleaseGateConfig{}, fmt.Errorf("%s: %w", releasePleaseGateCommandName, err) + } + if *repository == "" { + return releasePleaseGateConfig{}, errors.New(releasePleaseGateCommandName + ": --repo is required") + } + if *ref == "" { + return releasePleaseGateConfig{}, errors.New(releasePleaseGateCommandName + ": --ref is required") + } + return releasePleaseGateConfig{repository: *repository, ref: *ref}, nil +} + +// releasePleaseManifestReleaseTags lists the release tag of every version the +// manifest at the ref names, in manifest path order. +func releasePleaseManifestReleaseTags( + ctx context.Context, + runOutput func(context.Context, string, ...string) (string, error), + config releasePleaseGateConfig, +) ([]string, error) { + content, err := githubFileContentAtRef(ctx, runOutput, config.repository, releasePleaseManifestRelativePath, config.ref) + if err != nil { + return nil, err + } + + manifest := map[string]string{} + err = json.Unmarshal(content, &manifest) + if err != nil { + return nil, fmt.Errorf("%s: failed to parse %s at %s: %w", + releasePleaseGateCommandName, releasePleaseManifestRelativePath, config.ref, err) + } + + paths := make([]string, 0, len(manifest)) + for path := range manifest { + paths = append(paths, path) + } + sort.Strings(paths) + + tags := []string{} + for _, path := range paths { + tag, err := releasePleaseManifestReleaseTag(config.ref, path, manifest[path]) + if err != nil { + return nil, err + } + tags = append(tags, tag) + } + return tags, nil +} + +// releasePleaseManifestReleaseTag maps one manifest entry to the tag its +// publish workflow creates. An unknown path fails rather than being skipped, +// since a component the gate does not know about could be released unseen. +func releasePleaseManifestReleaseTag(ref string, path string, version string) (string, error) { + // A padded version would build a tag no release ever carries, so the gate + // would skip release-please for good instead of reporting the bad manifest. + if version == "" || version != strings.TrimSpace(version) { + return "", fmt.Errorf("%s: %s at %s releases %q as %q, which is not a bare version", + releasePleaseGateCommandName, releasePleaseManifestRelativePath, ref, path, version) + } + switch path { + case "Packages/src": + return "v" + version, nil + case releasePleaseDispatcherPackage: + return dispatcherReleaseTagPrefix + version, nil + case "cli/project-runner": + return projectRunnerReleaseTagPrefix + version, nil + } + return "", fmt.Errorf("%s: unknown release manifest path %q", releasePleaseGateCommandName, path) +} diff --git a/cli/release-automation/internal/automation/release_please_gate_test.go b/cli/release-automation/internal/automation/release_please_gate_test.go new file mode 100644 index 0000000000..81b07abc87 --- /dev/null +++ b/cli/release-automation/internal/automation/release_please_gate_test.go @@ -0,0 +1,223 @@ +package automation + +import ( + "bytes" + "context" + "encoding/base64" + "errors" + "fmt" + "strings" + "testing" +) + +const releasePleaseGateManifest = `{"Packages/src":"3.21.3","cli/dispatcher":"3.4.0","cli/project-runner":"3.12.3"}` + +// releasePleaseGateStub answers the manifest read and one release read per tag; +// a tag with no entry answers as published. +type releasePleaseGateStub struct { + manifestOutput string + manifestErr error + releaseJSON map[string]string + releaseErr map[string]error + commandLog []string +} + +func (stub *releasePleaseGateStub) runOutput(ctx context.Context, name string, args ...string) (string, error) { + commandLine := strings.Join(append([]string{name}, args...), " ") + stub.commandLog = append(stub.commandLog, commandLine) + if strings.HasPrefix(commandLine, "gh api repos/owner/repository/contents/"+releasePleaseManifestRelativePath+"?ref=main ") { + return stub.manifestOutput, stub.manifestErr + } + if strings.HasPrefix(commandLine, "gh release view ") { + tag := args[2] + if err, found := stub.releaseErr[tag]; found { + return "", err + } + if output, found := stub.releaseJSON[tag]; found { + return output, nil + } + return `{"isDraft":false}`, nil + } + return "", fmt.Errorf("unexpected command: %s", commandLine) +} + +func newReleasePleaseGateStub(manifest string) *releasePleaseGateStub { + return &releasePleaseGateStub{ + manifestOutput: base64.StdEncoding.EncodeToString([]byte(manifest)) + "\n", + releaseJSON: map[string]string{}, + releaseErr: map[string]error{}, + } +} + +func runReleasePleaseGateCase(t *testing.T, stub *releasePleaseGateStub, args []string) (int, string, string) { + t.Helper() + stdout := bytes.Buffer{} + stderr := bytes.Buffer{} + exitCode := RunReleasePleaseGateCheckWithDeps(context.Background(), &stdout, &stderr, args, stub.runOutput) + return exitCode, stdout.String(), stderr.String() +} + +func releasePleaseGateArgs() []string { + return []string{"--repo", "owner/repository", "--ref", "main"} +} + +func releasePleaseGateMissingErr(tag string) error { + return errors.New("gh release view " + tag + " failed: exit status 1\nrelease not found") +} + +// Verifies release-please may run once every release the manifest names is published, and that all three tags were read. +func TestReleasePleaseGateRunsWhenEveryManifestReleaseIsPublished(t *testing.T) { + stub := newReleasePleaseGateStub(releasePleaseGateManifest) + + exitCode, stdout, stderr := runReleasePleaseGateCase(t, stub, releasePleaseGateArgs()) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + if stdout != "skip=false\n" { + t.Fatalf("expected skip=false, got %q", stdout) + } + commandLogText := strings.Join(stub.commandLog, "\n") + for _, tag := range []string{"v3.21.3", "dispatcher-v3.4.0", "uloop-project-runner-v3.12.3"} { + assertReleasePRCheckLogContains(t, commandLogText, "gh release view "+tag+" ") + } +} + +// Verifies release-please is skipped while a release the manifest names does not exist yet, naming that tag. +func TestReleasePleaseGateSkipsWhileAManifestReleaseIsMissing(t *testing.T) { + stub := newReleasePleaseGateStub(releasePleaseGateManifest) + stub.releaseErr["uloop-project-runner-v3.12.3"] = releasePleaseGateMissingErr("uloop-project-runner-v3.12.3") + + exitCode, stdout, stderr := runReleasePleaseGateCase(t, stub, releasePleaseGateArgs()) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + if stdout != "skip=true\n" { + t.Fatalf("expected skip=true, got %q", stdout) + } + assertReleasePRCheckLogContains(t, stderr, "release-please is skipped: uloop-project-runner-v3.12.3 not published yet") +} + +// Verifies release-please is skipped while a release the manifest names is still a draft. +func TestReleasePleaseGateSkipsWhileAManifestReleaseIsDraft(t *testing.T) { + stub := newReleasePleaseGateStub(releasePleaseGateManifest) + stub.releaseJSON["v3.21.3"] = `{"isDraft":true}` + + exitCode, stdout, stderr := runReleasePleaseGateCase(t, stub, releasePleaseGateArgs()) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + if stdout != "skip=true\n" { + t.Fatalf("expected skip=true, got %q", stdout) + } + assertReleasePRCheckLogContains(t, stderr, "release-please is skipped: v3.21.3 not published yet") +} + +// Verifies the skip report names every unpublished release, not only the first one found. +func TestReleasePleaseGateNamesEveryUnpublishedRelease(t *testing.T) { + stub := newReleasePleaseGateStub(releasePleaseGateManifest) + stub.releaseJSON["v3.21.3"] = `{"isDraft":true}` + stub.releaseErr["dispatcher-v3.4.0"] = releasePleaseGateMissingErr("dispatcher-v3.4.0") + + exitCode, stdout, stderr := runReleasePleaseGateCase(t, stub, releasePleaseGateArgs()) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + if stdout != "skip=true\n" { + t.Fatalf("expected skip=true, got %q", stdout) + } + assertReleasePRCheckLogContains(t, stderr, "release-please is skipped: v3.21.3, dispatcher-v3.4.0 not published yet") +} + +// Verifies a release read that fails for any reason other than a missing release fails the run and decides nothing on stdout. +func TestReleasePleaseGateFailsWhenAReleaseCannotBeRead(t *testing.T) { + stub := newReleasePleaseGateStub(releasePleaseGateManifest) + stub.releaseErr["dispatcher-v3.4.0"] = errors.New("gh release view dispatcher-v3.4.0 failed: exit status 1\nHTTP 502: Bad Gateway") + + exitCode, stdout, stderr := runReleasePleaseGateCase(t, stub, releasePleaseGateArgs()) + + if exitCode != 1 { + t.Fatalf("expected exit code 1, got %d", exitCode) + } + if stdout != "" { + t.Fatalf("expected no stdout, got %q", stdout) + } + assertReleasePRCheckLogContains(t, stderr, "HTTP 502: Bad Gateway") +} + +// Verifies a manifest entry the gate cannot map to a release tag fails the run instead of being skipped. +func TestReleasePleaseGateRejectsUnknownManifestPath(t *testing.T) { + stub := newReleasePleaseGateStub(`{"Packages/src":"3.21.3","cli/other":"1.0.0"}`) + + exitCode, stdout, stderr := runReleasePleaseGateCase(t, stub, releasePleaseGateArgs()) + + if exitCode != 1 { + t.Fatalf("expected exit code 1, got %d", exitCode) + } + if stdout != "" { + t.Fatalf("expected no stdout, got %q", stdout) + } + assertReleasePRCheckLogContains(t, stderr, `unknown release manifest path "cli/other"`) +} + +// Verifies an unreadable, unparsable, or padded manifest fails the run with its own error and decides nothing on stdout. +func TestReleasePleaseGateFailsOnUnusableManifest(t *testing.T) { + cases := []struct { + name string + output string + err error + wantErr string + }{ + {"manifest read", "", errors.New("manifest read failed"), "manifest read failed"}, + {"manifest JSON", base64Of("{"), nil, "failed to parse " + releasePleaseManifestRelativePath + " at main"}, + {"padded version", base64Of(`{"cli/dispatcher":" 3.4.0"}`), nil, `releases "cli/dispatcher" as " 3.4.0", which is not a bare version`}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + stub := newReleasePleaseGateStub(releasePleaseGateManifest) + stub.manifestOutput = testCase.output + stub.manifestErr = testCase.err + + exitCode, stdout, stderr := runReleasePleaseGateCase(t, stub, releasePleaseGateArgs()) + + if exitCode != 1 { + t.Fatalf("expected exit code 1, got %d", exitCode) + } + if stdout != "" { + t.Fatalf("expected no stdout, got %q", stdout) + } + assertReleasePRCheckLogContains(t, stderr, testCase.wantErr) + }) + } +} + +// Verifies the gate refuses to run without the repository or the ref, and on an unknown flag. +func TestParseReleasePleaseGateFlagsRejectsInvalidInput(t *testing.T) { + cases := []struct { + name string + args []string + wantErr string + }{ + {"missing repo", []string{"--ref", "main"}, "--repo is required"}, + {"missing ref", []string{"--repo", "owner/repository"}, "--ref is required"}, + {"unknown flag", []string{"--repo", "owner/repository", "--ref", "main", "--other"}, "flag provided but not defined"}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + stub := newReleasePleaseGateStub(releasePleaseGateManifest) + + exitCode, _, stderr := runReleasePleaseGateCase(t, stub, testCase.args) + + if exitCode != 1 { + t.Fatalf("expected exit code 1, got %d", exitCode) + } + assertReleasePRCheckLogContains(t, stderr, testCase.wantErr) + if len(stub.commandLog) != 0 { + t.Fatalf("expected no gh call, got %v", stub.commandLog) + } + }) + } +} diff --git a/scripts/is-release-please-release-commit.sh b/scripts/is-release-please-release-commit.sh deleted file mode 100755 index 6c94e612f9..0000000000 --- a/scripts/is-release-please-release-commit.sh +++ /dev/null @@ -1,13 +0,0 @@ -#!/bin/sh -set -eu - -COMMIT_SUBJECT=${1:?commit subject is required} - -case "$COMMIT_SUBJECT" in - "chore: release "*|chore\(*\):\ release\ *) - exit 0 - ;; - *) - exit 1 - ;; -esac diff --git a/scripts/test-is-release-please-release-commit.sh b/scripts/test-is-release-please-release-commit.sh deleted file mode 100755 index 4d8b5e9b15..0000000000 --- a/scripts/test-is-release-please-release-commit.sh +++ /dev/null @@ -1,48 +0,0 @@ -#!/bin/sh -set -eu - -ROOT_DIR=$(CDPATH= cd "$(dirname "$0")/.." && pwd) -SCRIPT="$ROOT_DIR/scripts/is-release-please-release-commit.sh" - -assert_release_commit() { - subject=$1 - - if ! "$SCRIPT" "$subject"; then - echo "Expected release commit subject: $subject" >&2 - exit 1 - fi -} - -assert_not_release_commit() { - subject=$1 - - if "$SCRIPT" "$subject"; then - echo "Expected non-release commit subject: $subject" >&2 - exit 1 - fi -} - -# Verifies the scoped release-please commit title used by beta release PRs is detected. -test_detects_scoped_release_commit() { - assert_release_commit "chore(v3-beta): release 3.0.0-beta.4" -} - -# Verifies unscoped release-please commit titles are detected for the main branch. -test_detects_unscoped_release_commit() { - assert_release_commit "chore: release 1.2.3" -} - -# Verifies ordinary feature commits continue to run release-please. -test_rejects_feature_commit() { - assert_not_release_commit "fix: Setup now upgrades to the native CLI cleanly" -} - -# Verifies release-related maintenance commits are not treated as release PR merges. -test_rejects_non_release_chore() { - assert_not_release_commit "chore(v3-beta): update release notes" -} - -test_detects_scoped_release_commit -test_detects_unscoped_release_commit -test_rejects_feature_commit -test_rejects_non_release_chore diff --git a/scripts/test-release-please-config.sh b/scripts/test-release-please-config.sh index f003f955b5..da164d47be 100755 --- a/scripts/test-release-please-config.sh +++ b/scripts/test-release-please-config.sh @@ -92,6 +92,27 @@ assert_step_contains() { fi } +assert_step_lacks() { + path=$1 + step=$2 + unexpected=$3 + + step_line=$(line_number "$path" "$step") + if [ -z "$step_line" ]; then + echo "Expected step '$step' in $path." >&2 + exit 1 + fi + if awk -v start="$step_line" ' + NR < start { next } + NR == start { in_step = 1 } + in_step && NR > start && /^ - name: / { exit } + in_step { print } + ' "$path" | grep -F -- "$unexpected" >/dev/null; then + echo "Expected step '$step' not to contain: $unexpected" >&2 + exit 1 + fi +} + assert_package_path_exists() { package_path=$1 path=$2 @@ -175,13 +196,14 @@ assert_json_value '.packages["cli/dispatcher"]["extra-files"][0].jsonpath' '$.di assert_file_contains "$RELEASE_WORKFLOW" ' actions: write' assert_file_contains "$RELEASE_WORKFLOW" ' checks: read' -assert_file_contains "$RELEASE_WORKFLOW" ' - name: Setup Go for release PR automation' +assert_file_contains "$RELEASE_WORKFLOW" ' - name: Setup Go for release automation' assert_file_contains "$RELEASE_WORKFLOW" ' - name: Dispatch release PR checks' assert_file_contains "$RELEASE_WORKFLOW" ' working-directory: cli/release-automation' assert_file_contains "$RELEASE_WORKFLOW" ' run: go run ./cmd/dispatch-release-please-pr-checks' -assert_step_contains "$RELEASE_WORKFLOW" ' - name: Setup Go for release PR automation' " if: steps.target.outputs.branch == 'main' && steps.release_commit.outputs.skip != 'true' && steps.release.outputs.prs_created == 'true'" +# The release-please gate is a Go command, so Go has to be set up on every run, before the gate. +assert_step_lacks "$RELEASE_WORKFLOW" ' - name: Setup Go for release automation' ' if:' assert_step_contains "$RELEASE_WORKFLOW" ' - name: Dispatch release PR checks' " if: steps.target.outputs.branch == 'main' && steps.release_commit.outputs.skip != 'true' && steps.release.outputs.prs_created == 'true'" -assert_file_order "$RELEASE_WORKFLOW" ' - name: Setup Go for release PR automation' ' - name: Dispatch release PR checks' +assert_file_order "$RELEASE_WORKFLOW" ' - name: Setup Go for release automation' ' - name: 🧭 Decide whether every manifest release is published' assert_manifest_semver '.["Packages/src"]' assert_manifest_semver '.["cli/project-runner"]' From 935b4f8e1f6090e2a5a95aa98aba0e92dafdb55c Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 29 Oct 2026 23:54:06 +0900 Subject: [PATCH 3/6] Mark the release pull request tagged before dispatching release-please The native CLI post-publish dispatched release-please first. Now that the dispatched run is no longer skipped, it can start before the label is set; marking first, as the Unity package release already does, means the run finds the pull request tagged. --- .github/workflows/native-cli-publish.yml | 14 +++++++------- scripts/test-native-cli-publish-workflow.sh | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/native-cli-publish.yml b/.github/workflows/native-cli-publish.yml index 5b88fcaaa2..0f363247ef 100644 --- a/.github/workflows/native-cli-publish.yml +++ b/.github/workflows/native-cli-publish.yml @@ -456,13 +456,6 @@ jobs: fetch-depth: 0 persist-credentials: false - - name: Dispatch release-please after native CLI publish - if: needs.build.outputs.should_publish == 'true' && github.event_name == 'push' - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - TARGET_BRANCH: ${{ github.ref_name }} - run: gh workflow run release-please.yml --ref "${TARGET_BRANCH}" -f branch="${TARGET_BRANCH}" - - name: Mark release PR as tagged if: github.event_name == 'push' env: @@ -472,3 +465,10 @@ jobs: TARGET_BRANCH: ${{ github.ref_name }} GITHUB_REPOSITORY: ${{ github.repository }} run: scripts/mark-release-pr-tagged.sh + + - name: Dispatch release-please after native CLI publish + if: needs.build.outputs.should_publish == 'true' && github.event_name == 'push' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + TARGET_BRANCH: ${{ github.ref_name }} + run: gh workflow run release-please.yml --ref "${TARGET_BRANCH}" -f branch="${TARGET_BRANCH}" diff --git a/scripts/test-native-cli-publish-workflow.sh b/scripts/test-native-cli-publish-workflow.sh index d48c0f9b9c..2fcaee11b8 100755 --- a/scripts/test-native-cli-publish-workflow.sh +++ b/scripts/test-native-cli-publish-workflow.sh @@ -262,7 +262,7 @@ test_post_publish_automation_remains_outside_the_privileged_job() { assert_contains " - name: Dispatch release-please after native CLI publish" assert_contains " - name: Mark release PR as tagged" assert_count 2 " contents: write" - assert_post_publish_before " - name: Dispatch release-please after native CLI publish" " - name: Mark release PR as tagged" + assert_post_publish_before " - name: Mark release PR as tagged" " - name: Dispatch release-please after native CLI publish" } test_build_and_publish_jobs_have_separate_trust_boundaries From 7fab9636f02896d1ccb76a9a667f2dfd113c4a63 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 30 Oct 2026 00:00:15 +0900 Subject: [PATCH 4/6] Document that the package release waits for the project runner release Record the release order (dispatcher, then project runner, then the package by automation) and the two gates in ADR 0021, and add the runner condition to the manual merge checklist. The places that explained the post-publish label marking by the old release-commit skip now give the new reason; ADR 0013 keeps its text and points at ADR 0021. --- .github/workflows/unity-package-release.yml | 8 +- AGENTS.md | 5 +- ...rate-release-prs-and-package-auto-merge.md | 2 + ...the-unity-package-release-after-signing.md | 2 + ...se-waits-for-the-project-runner-release.md | 74 +++++++++++++++++++ docs/dispatcher-pin-release-order.md | 63 +++++++++++----- docs/unity-package-release.md | 9 ++- .../test-unity-package-release-workflow.sh | 2 +- 8 files changed, 135 insertions(+), 30 deletions(-) create mode 100644 docs/adr/0021-package-release-waits-for-the-project-runner-release.md diff --git a/.github/workflows/unity-package-release.yml b/.github/workflows/unity-package-release.yml index 86994efdb3..d3e7c0733f 100644 --- a/.github/workflows/unity-package-release.yml +++ b/.github/workflows/unity-package-release.yml @@ -201,10 +201,10 @@ jobs: --asset "$RUNNER_TEMP/signed/$ASSET_NAME" # Marks the package release pull request tagged, which is what lets - # release-please open the next release pull requests. It is marked here - # because release-please skips its own label sync while the release commit is - # still the newest main commit, which it is whenever the release was due as - # soon as the pull request was merged. + # release-please open the next release pull requests. release-please runs + # only once every release the manifest names is published, so the label is + # set here first; the release-please run this job then starts finds the pull + # request already tagged. post-publish: needs: [plan, publish] runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index 554e9297c4..2c6f8bbb9d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -114,8 +114,9 @@ inputs outside the package roots (non-test `cli/common/**` sources, `scripts/ins run in the same PR; description-only catalog regenerations are exempt. CI (`check-release-triggers`) fails otherwise. Rules and rationale: `docs/shared-release-inputs.md`. -release-please opens one release pull request per component; merge the dispatcher one first. The -unity-package one stays draft and is merged only by automation — never merge it by hand. Details: +release-please opens one release pull request per component; merge the dispatcher one first, then +the project runner one. The unity-package one stays draft and is merged only by automation once both +are published — never merge it by hand. Details: `docs/dispatcher-pin-release-order.md`. ## Broken CLI Releases diff --git a/docs/adr/0007-separate-release-prs-and-package-auto-merge.md b/docs/adr/0007-separate-release-prs-and-package-auto-merge.md index 3cea62cb3c..9fb31a388c 100644 --- a/docs/adr/0007-separate-release-prs-and-package-auto-merge.md +++ b/docs/adr/0007-separate-release-prs-and-package-auto-merge.md @@ -104,6 +104,8 @@ would never be tagged. - A failed check on the package release pull request stops the automation with a non-zero exit rather than waiting. That is deliberate: only a person can fix a failing check, and waiting would hide it until the timeout. +- The project runner side of this order (the package waits for the runner its + pin names to be released) is in ADR 0021. ## Reversal condition diff --git a/docs/adr/0013-create-the-unity-package-release-after-signing.md b/docs/adr/0013-create-the-unity-package-release-after-signing.md index f7b5c0d2b6..8e874f9014 100644 --- a/docs/adr/0013-create-the-unity-package-release-after-signing.md +++ b/docs/adr/0013-create-the-unity-package-release-after-signing.md @@ -64,6 +64,8 @@ release waits for. release-please, because a release created with `GITHUB_TOKEN` starts no workflow, so commits merged while the release was pending get their release pull requests without waiting for the next push. +- Superseded in part by ADR 0021: release-please now runs on a release commit once every + release the manifest names is published. ## Reversal Condition diff --git a/docs/adr/0021-package-release-waits-for-the-project-runner-release.md b/docs/adr/0021-package-release-waits-for-the-project-runner-release.md new file mode 100644 index 0000000000..ea693b8775 --- /dev/null +++ b/docs/adr/0021-package-release-waits-for-the-project-runner-release.md @@ -0,0 +1,74 @@ +# The Package Release Waits for the Project Runner Release + +Date: 2026-10-29 + +## Decision + +The Unity package is released only after the project runner its pin names is published, and +release-please runs only once every release the manifest names is published. + +- Release order: merge the dispatcher release pull request and approve `cli-release`; then merge + the project runner release pull request and approve `cli-release`; the unity-package release + pull request is merged by automation once both are published. Merging the runner first does not + get stuck, but dispatcher then runner is the recommended order. +- `merge-package-release-pr` leaves the package release pull request draft while a project runner + release pull request is open, or while the release `uloop-project-runner-v` + the head pin names is missing or still a draft. It checks this only after the head pin records + the current dispatcher, and it settles the pass instead of polling, on both paths + (`dispatcher-publish` post-publish and `release-please.yml`): the runner is published by another, + approval-gated workflow, whose post-publish dispatches release-please, and that run's merge step + decides again. +- `check-release-please-gate` replaces the "the newest commit is a release commit" skip. It reads + the manifest at the target commit and skips release-please while any release it names is + missing or still a draft. A release that cannot be read fails the run rather than choosing a + side. +- The native CLI post-publish marks its release pull request tagged before it dispatches + release-please, as the Unity package release already does, so the dispatched run finds the label + set. + +## Context + +On 2026-10-29, Unity package 3.21.2 was merged before the project runner 3.12.3 release pull +request. The runner release pull request raises `projectRunnerVersion` in the package pin +through `extra-files`, so package 3.21.2 was published pinning runner 3.12.2, and the runner fixes +made for it never reached its users. The package merge gate only checked the dispatcher. + +Merging the runner first did not work either. `release-please.yml` skipped every run whose newest +commit was a release commit (#1088), including the run the runner's post-publish dispatches after +publishing ("Release commit detected; release-please will wait for the next non-release commit."). +The remaining release pull requests stayed in conflict on adjacent manifest lines and were never +rebuilt. ADR 0013 relied on the same skip when it had the package release mark its pull request +tagged itself. + +The skip exists because release-please finds the previous release by tag: on a release commit +whose release is not published yet it would repeat the previous changelog entries. "The newest +commit is a release commit" was only a stand-in for "a version the manifest names is not +published yet", and the stand-in also blocked the runs that come after publishing. + +An empty commit cannot be used to release only the package: release-please's manifest mode hands +a commit that touches no file to every component (`includeEmpty`). + +## Considered Options + +- Change only the merge order people follow (runner first): rejected. The release-commit skip + stops the dispatched release-please run, so the remaining release pull requests are never + rebuilt. +- Add a force input to the release-please dispatch: rejected. A dispatch by hand would still be + able to run on a release commit whose release is pending and repeat the previous changelog + (#1088). +- Stamp the runner into the package release pull request's pin before the runner is published: + rejected. A pin naming an unpublished runner resolves to a 404 for every fresh install. + +## Consequences + +- When the runner has unreleased changes, the package release waits for one more runner approval. +- release-please runs on a release commit once every manifest release is published, so the publish + workflows' dispatches rebuild the remaining release pull requests. +- An unreadable release fails `release-please.yml` or the merge step instead of being skipped; + someone has to look at it. + +## Revisit When + +- (a) The runner and the package go back to one release pull request. +- (b) The pin's `projectRunnerVersion` comes to mean a minimum version rather than the exact runner + the package ships with. diff --git a/docs/dispatcher-pin-release-order.md b/docs/dispatcher-pin-release-order.md index d275e36ca9..b956960d8a 100644 --- a/docs/dispatcher-pin-release-order.md +++ b/docs/dispatcher-pin-release-order.md @@ -35,25 +35,39 @@ release-please opens **one release pull request per component**: `unity-package` and is merged only by automation; the other two are the ones people merge. 1. Merge the **dispatcher** release pull request and approve the `cli-release` - environment. Publishing the dispatcher is the only human decision. -2. `post-publish` stamps the pin on `main`, then marks the **unity-package** - release pull request ready and merges it, once that pull request's head - records the dispatcher tag just published and has a successful run of every - required workflow for that exact head commit. The release PR checks run one + environment. Publishing the dispatcher is a human decision. +2. `post-publish` stamps the pin on `main`, then tries to mark the + **unity-package** release pull request ready and merge it, once that pull + request's head records the dispatcher tag just published and has a + successful run of every required workflow for that exact head commit. It + leaves the pull request draft while a **project runner** release pull + request is open, or while the release of the runner the head pin names + (`uloop-project-runner-v`) is missing or still a + draft: the runner release pull request raises that pin, so a package cut + before it would ship the previous runner. The release PR checks run one pull request at a time, so the unity-package one always goes first; behind another component's checks it would outlast the time `post-publish` waits. -3. A package release that carries no dispatcher bump never reaches step 2, so - `release-please.yml` merges it instead: after the release PR checks, it - marks the package pull request ready and merges it, but only while no - 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. +3. Merge the **project runner** release pull request and approve the + `cli-release` environment, after release-please has rebuilt it on top of the + stamp push. +4. Once the runner is published, its `post-publish` dispatches release-please, + which rebuilds the unity-package release pull request with the new runner in + its pin. The merge step at the end of `release-please.yml` then marks it + ready and merges it, but only while no dispatcher release pull request is + open, the pin at the package head already records the dispatcher version + `main`'s manifest releases, no project runner release pull request is open, + the runner the pin names is published, and its checks are green. Otherwise + it leaves the pull request draft and reconsiders on the next push to `main` + or the next dispatch. 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. A package release that carries no dispatcher bump is merged by this + step alone. +5. Merging the runner before the dispatcher does not get stuck either: the + runner publish dispatches release-please, which rebuilds the dispatcher + release pull request. The recommended order is still dispatcher, then + runner. Merging the package pull request before the dispatcher one would ship the *previous* dispatcher, which is the lag this order exists to remove, so it is @@ -92,7 +106,7 @@ window red for unrelated work. When the automatic merge does not happen, merging the unity-package release pull request by hand means re-checking by hand what the automation would have checked. A green pin freshness gate on `main` only proves the stamp landed; it -says nothing about the pull request. Before merging, confirm all four: +says nothing about the pull request. Before merging, confirm all five: - the pin freshness gate on `main` is green, so the stamp reached `main`; - the pull request's head commit records the dispatcher tag just published in @@ -102,12 +116,15 @@ says nothing about the pull request. Before merging, confirm all four: releases; - every required workflow has a completed successful run for that exact head SHA — not for an earlier head. +- no project runner release pull request is open, and the release + `uloop-project-runner-v` the head pin names is + published (not draft). Merging without those is how a stale or unvalidated package release gets published, which is the failure this whole order exists to prevent. Decision record: `docs/adr/0007-separate-release-prs-and-package-auto-merge.md`. -Once all four hold, take the pull request out of draft yourself ("Ready for +Once all five hold, take the pull request out of draft yourself ("Ready for review") and merge it. A **pre-release** dispatcher is never stamped, so a package release waiting on one stays draft indefinitely: publish a stable dispatcher, or merge by hand as above. @@ -116,6 +133,14 @@ When the stamp itself fails, the package pull request simply stays draft. Re-run the `post-publish` job of the `dispatcher-publish` run: it stamps the new tip and merges the pull request in the same job. +release-please itself runs only once every release the manifest at the target +commit names is published (`check-release-please-gate`). It finds the previous +release by tag, so rebuilding the release pull requests on a release commit +whose release is still pending would repeat the previous changelog entries. +Every publish workflow dispatches release-please once its release is out, which +is what rebuilds the remaining release pull requests after a release commit. +Decision record: `docs/adr/0021-package-release-waits-for-the-project-runner-release.md`. + ## Why the stamp is pushed without a pull request Until 2026-09 the stamp travelled as an automated pull request that a human diff --git a/docs/unity-package-release.md b/docs/unity-package-release.md index 4d8dc58f22..9cd587fa88 100644 --- a/docs/unity-package-release.md +++ b/docs/unity-package-release.md @@ -54,10 +54,11 @@ after the tarball is signed. The decision and the alternatives it replaced are r non-empty tarball. A pre-release version is published as a pre-release. 5. **`post-publish`** (`actions: write`, `pull-requests: write`) marks the package release pull request `autorelease: tagged`, which lets release-please open the next release pull requests. - release-please's own label sync does not do it while the release commit is still the newest - `main` commit, because release-please skips release commits. It then starts `release-please`, - because a release created with `GITHUB_TOKEN` starts no workflow, so commits merged while the - release was pending get their release pull requests without waiting for the next push. + release-please runs only once every release the manifest names is published, so the label is + set here first; the release-please run it then starts finds the pull request already tagged. + It starts `release-please` because a release created with `GITHUB_TOKEN` starts no workflow, + so commits merged while the release was pending get their release pull requests without + waiting for the next push. Neither `plan` nor `publish` runs the UPM CLI, and `sign`, which holds the signing credentials, cannot write to the repository. diff --git a/scripts/test-unity-package-release-workflow.sh b/scripts/test-unity-package-release-workflow.sh index 855cba4005..f7b3fc79bd 100755 --- a/scripts/test-unity-package-release-workflow.sh +++ b/scripts/test-unity-package-release-workflow.sh @@ -110,7 +110,7 @@ test_plan_decisions_reach_the_jobs() { assert_job_contains publish " name: signed-unity-package" } -# Verifies post-publish marks the release pull request tagged itself before it starts release-please: release-please skips its own label sync while the release commit is the newest main commit. +# Verifies post-publish marks the release pull request tagged itself before it starts release-please: release-please runs only once every manifest release is published, so the run it starts must find the label already set. test_post_publish_marks_the_release_pull_request_tagged() { assert_job_contains post-publish " needs: [plan, publish]" assert_job_contains post-publish ' RELEASE_TAG: ${{ needs.plan.outputs.tag }}' From 513f96b18769c16cfeca79bbcd2508c7feb2646a Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 30 Oct 2026 00:36:48 +0900 Subject: [PATCH 5/6] Settle the post-publish merge when release-please cannot rebuild a stale head When a project runner release commit is on main but not yet published and the dispatcher is published first, the stamp push starts a release-please run that the gate skips, so the package release pull request is never rebased onto the stamp. The post-publish merge kept waiting for that rebase, then failed after 45 minutes and asked for a manual merge. In wait mode a head that still pins the previous dispatcher now reads main's manifest and settles with exit 0 while a release it names is unpublished; that publish dispatches release-please, whose merge step decides again. The gate's listing of unpublished manifest releases is extracted so both use it. --- .../automation/package_release_pr_merge.go | 40 ++++++++++-- .../package_release_pr_merge_test.go | 64 +++++++++++++++++-- .../automation/release_please_gate.go | 56 ++++++++++------ ...se-waits-for-the-project-runner-release.md | 5 +- docs/dispatcher-pin-release-order.md | 6 +- 5 files changed, 139 insertions(+), 32 deletions(-) diff --git a/cli/release-automation/internal/automation/package_release_pr_merge.go b/cli/release-automation/internal/automation/package_release_pr_merge.go index c8d6c7ef4d..9281db2eb5 100644 --- a/cli/release-automation/internal/automation/package_release_pr_merge.go +++ b/cli/release-automation/internal/automation/package_release_pr_merge.go @@ -6,6 +6,7 @@ import ( "flag" "fmt" "io" + "strings" "time" ) @@ -266,10 +267,7 @@ func attemptMergePackageReleasePR( } pinnedTag := pin.DispatcherReleaseTag 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 mergePackageReleasePRPassAwaitingPin, 0 + return stalePinOutcome(ctx, stdout, stderr, config, releasePR, pinnedTag, deps) } // The runner is judged only on a head that records the current dispatcher: // a head from before the stamp may still name an older runner, and @@ -293,6 +291,40 @@ func attemptMergePackageReleasePR( return readyAndMergePackageReleasePR(ctx, stdout, stderr, config, releasePR, pinnedTag, deps) } +// stalePinOutcome decides a pass whose head still pins an older dispatcher. +// Waiting assumes release-please will rebase the head onto the stamp, which it +// does not do while a release main's manifest names is unpublished, so the +// wait path settles then instead of timing out. The no-wait path runs inside a +// release-please run that the gate let through, so it never reads the manifest +// here. +func stalePinOutcome( + ctx context.Context, + stdout io.Writer, + stderr io.Writer, + config mergePackageReleasePRConfig, + releasePR mergePackageReleasePullRequest, + pinnedTag string, + deps mergePackageReleasePRDeps, +) (mergePackageReleasePRPassOutcome, int) { + if config.waitForMergeable { + unpublished, err := unpublishedManifestReleases(ctx, deps.runOutput, config.repository, config.baseBranch) + if err != nil { + writeMergePackageReleasePRLine(stderr, err) + return mergePackageReleasePRPassSettled, 1 + } + if len(unpublished) > 0 { + writeMergePackageReleasePRLine(stdout, fmt.Sprintf( + "PR #%d head %s still pins %s, and release-please will not rebuild it until %s is published; that publish dispatches release-please, whose merge step decides again.", + releasePR.Number, releasePR.HeadRefOID, pinnedTag, strings.Join(unpublished, ", "))) + return mergePackageReleasePRPassSettled, 0 + } + } + 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 mergePackageReleasePRPassAwaitingPin, 0 +} + func findPackageReleasePullRequest( ctx context.Context, config mergePackageReleasePRConfig, diff --git a/cli/release-automation/internal/automation/package_release_pr_merge_test.go b/cli/release-automation/internal/automation/package_release_pr_merge_test.go index 331b6c2549..126240a51c 100644 --- a/cli/release-automation/internal/automation/package_release_pr_merge_test.go +++ b/cli/release-automation/internal/automation/package_release_pr_merge_test.go @@ -35,6 +35,9 @@ type mergePackageReleasePRPoll struct { // release read for the runner the pin names; empty JSON means published. projectRunnerReleaseViewJSON string projectRunnerReleaseViewErr error + // releaseViewErrByTag fails the release read of one tag only, so a test + // can leave one manifest release unpublished while the others are out. + releaseViewErrByTag map[string]error // manifestByRef answers the contents API for the release manifest, so a // test can pin the dispatcher version the base branch tip releases. manifestByRef map[string]string @@ -102,6 +105,10 @@ func (stub *mergePackageReleasePRStub) answerPullRequestCommand(commandLine stri } return stub.activePoll.openProjectRunnerPRListJSON, true, nil case strings.HasPrefix(commandLine, "gh release view "): + tag := strings.Fields(commandLine)[3] + if err, found := stub.activePoll.releaseViewErrByTag[tag]; found { + return "", true, err + } if stub.activePoll.projectRunnerReleaseViewErr != nil { return "", true, stub.activePoll.projectRunnerReleaseViewErr } @@ -171,6 +178,11 @@ func (stub *mergePackageReleasePRStub) answerContentCommand(commandLine string) // test can change what the base branch releases between passes. func (stub *mergePackageReleasePRStub) manifestAt(ref string) (string, bool) { manifest, known := stub.activePoll.manifestByRef[ref] + if !known { + // Every manifest release this default names answers as published, so a + // stale pin keeps waiting for the rebase unless a test says otherwise. + return `{"Packages/src":"3.20.0","cli/dispatcher":"3.4.0","cli/project-runner":"3.4.0"}`, true + } return manifest, known } @@ -1120,7 +1132,7 @@ func TestMergePackageReleasePRSettlesAtOnceInWaitModeWhilePinnedProjectRunnerIsU assertMergePackageReleasePRNeverMerged(t, stub) } -// Verifies the runner release is read only once the head pins the current dispatcher: a stale head is waited out first, and the rebased head is the one judged. +// Verifies the project runner gate runs only once the head pins the current dispatcher: a stale head is waited out first, and the rebased head is the one judged. func TestMergePackageReleasePRChecksTheProjectRunnerOnlyAfterThePinIsCurrent(t *testing.T) { stalePoll := mergePackageReleasePRPoll{ prListJSON: packageReleasePRListJSON("stale123", true), @@ -1138,18 +1150,58 @@ func TestMergePackageReleasePRChecksTheProjectRunnerOnlyAfterThePinIsCurrent(t * if exitCode != 0 { t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) } - firstReleaseView := -1 + firstRunnerGateRead := -1 rebasedPinRead := -1 for index, commandLine := range stub.commandLog { if rebasedPinRead == -1 && strings.Contains(commandLine, "?ref=rebased456") { rebasedPinRead = index } - if firstReleaseView == -1 && strings.HasPrefix(commandLine, "gh release view ") { - firstReleaseView = index + if firstRunnerGateRead == -1 && strings.Contains(commandLine, "--components--uloop-project-runner") { + firstRunnerGateRead = index } } - if firstReleaseView == -1 || rebasedPinRead == -1 || firstReleaseView < rebasedPinRead { - t.Fatalf("expected the runner release to be read only after the rebased pin\n%s", strings.Join(stub.commandLog, "\n")) + if firstRunnerGateRead == -1 || rebasedPinRead == -1 || firstRunnerGateRead < rebasedPinRead { + t.Fatalf("expected the project runner gate to run only after the rebased pin\n%s", strings.Join(stub.commandLog, "\n")) } assertMergePackageReleasePRMergeCount(t, stub, "rebased456", 1) } + +// Verifies the post-publish path settles in one pass with exit 0 when the head still pins the previous dispatcher and a release main's manifest names is unpublished, since release-please will not rebase the head before that publish and waiting would only time out. +func TestMergePackageReleasePRSettlesAtOnceInWaitModeWhileAStalePinCannotBeRebuilt(t *testing.T) { + stalePoll := mergePackageReleasePRPoll{ + prListJSON: packageReleasePRListJSON("stale123", true), + pinnedTagsByRef: packageReleasePRPinAt("stale123", "dispatcher-v3.3.1"), + runs: packageReleasePRRunsAt("stale123"), + manifestByRef: map[string]string{"main": `{"Packages/src":"3.20.0","cli/dispatcher":"3.4.0","cli/project-runner":"3.4.1"}`}, + releaseViewErrByTag: map[string]error{"uloop-project-runner-v3.4.1": errors.New("gh release view uloop-project-runner-v3.4.1 failed: exit status 1\nrelease not found")}, + } + + exitCode, stdout, stderr, stub := runMergePackageReleasePRCase(t, []mergePackageReleasePRPoll{stalePoll}) + + if exitCode != 0 { + t.Fatalf("expected exit code 0, got %d\nstderr: %s", exitCode, stderr) + } + assertReleasePRCheckLogContains(t, stdout, "will not rebuild it until uloop-project-runner-v3.4.1 is published") + assertMergePackageReleasePRListCount(t, stub, 1) + assertMergePackageReleasePRNeverMerged(t, stub) +} + +// Verifies the post-publish path fails without merging when the manifest it reads for a stale pin cannot be read, rather than guessing whether release-please will rebuild the head. +func TestMergePackageReleasePRFailsInWaitModeWhenTheManifestCannotBeReadForAStalePin(t *testing.T) { + stalePoll := mergePackageReleasePRPoll{ + prListJSON: packageReleasePRListJSON("stale123", true), + pinnedTagsByRef: packageReleasePRPinAt("stale123", "dispatcher-v3.3.1"), + runs: packageReleasePRRunsAt("stale123"), + } + + exitCode, _, stderr, stub := runMergePackageReleasePRWithOverride( + t, []mergePackageReleasePRPoll{stalePoll}, []string{"--dispatcher-tag", "dispatcher-v3.4.0"}, + mergePackageReleasePROverride{matches: mergePackageReleasePRCommandContains("contents/" + releasePleaseManifestRelativePath), err: errors.New("manifest read failed")}, + nil) + + if exitCode != 1 { + t.Fatalf("expected exit code 1, got %d", exitCode) + } + assertReleasePRCheckLogContains(t, stderr, "manifest read failed") + assertMergePackageReleasePRNeverMerged(t, stub) +} diff --git a/cli/release-automation/internal/automation/release_please_gate.go b/cli/release-automation/internal/automation/release_please_gate.go index 3972fef253..561f093c07 100644 --- a/cli/release-automation/internal/automation/release_please_gate.go +++ b/cli/release-automation/internal/automation/release_please_gate.go @@ -39,25 +39,13 @@ func RunReleasePleaseGateCheckWithDeps( return 1 } - tags, err := releasePleaseManifestReleaseTags(ctx, runOutput, config) - if err != nil { - writeMergePackageReleasePRLine(stderr, err) - return 1 - } - // A release that cannot be read fails the run instead of choosing a side: // running anyway repeats the previous changelog, and skipping silently would // leave the release pull requests unbuilt for good. - unpublished := []string{} - for _, tag := range tags { - published, err := releasePublished(ctx, runOutput, config.repository, tag) - if err != nil { - writeMergePackageReleasePRLine(stderr, err) - return 1 - } - if !published { - unpublished = append(unpublished, tag) - } + unpublished, err := unpublishedManifestReleases(ctx, runOutput, config.repository, config.ref) + if err != nil { + writeMergePackageReleasePRLine(stderr, err) + return 1 } if len(unpublished) > 0 { @@ -91,14 +79,42 @@ func parseReleasePleaseGateFlags(args []string) (releasePleaseGateConfig, error) return releasePleaseGateConfig{repository: *repository, ref: *ref}, nil } +// unpublishedManifestReleases lists the release tags the manifest at the ref +// names that are missing or still a draft, in manifest path order. While any +// is listed, release-please does not run on that ref. +func unpublishedManifestReleases( + ctx context.Context, + runOutput func(context.Context, string, ...string) (string, error), + repository string, + ref string, +) ([]string, error) { + tags, err := releasePleaseManifestReleaseTags(ctx, runOutput, repository, ref) + if err != nil { + return nil, err + } + + unpublished := []string{} + for _, tag := range tags { + published, err := releasePublished(ctx, runOutput, repository, tag) + if err != nil { + return nil, err + } + if !published { + unpublished = append(unpublished, tag) + } + } + return unpublished, nil +} + // releasePleaseManifestReleaseTags lists the release tag of every version the // manifest at the ref names, in manifest path order. func releasePleaseManifestReleaseTags( ctx context.Context, runOutput func(context.Context, string, ...string) (string, error), - config releasePleaseGateConfig, + repository string, + ref string, ) ([]string, error) { - content, err := githubFileContentAtRef(ctx, runOutput, config.repository, releasePleaseManifestRelativePath, config.ref) + content, err := githubFileContentAtRef(ctx, runOutput, repository, releasePleaseManifestRelativePath, ref) if err != nil { return nil, err } @@ -107,7 +123,7 @@ func releasePleaseManifestReleaseTags( err = json.Unmarshal(content, &manifest) if err != nil { return nil, fmt.Errorf("%s: failed to parse %s at %s: %w", - releasePleaseGateCommandName, releasePleaseManifestRelativePath, config.ref, err) + releasePleaseGateCommandName, releasePleaseManifestRelativePath, ref, err) } paths := make([]string, 0, len(manifest)) @@ -118,7 +134,7 @@ func releasePleaseManifestReleaseTags( tags := []string{} for _, path := range paths { - tag, err := releasePleaseManifestReleaseTag(config.ref, path, manifest[path]) + tag, err := releasePleaseManifestReleaseTag(ref, path, manifest[path]) if err != nil { return nil, err } diff --git a/docs/adr/0021-package-release-waits-for-the-project-runner-release.md b/docs/adr/0021-package-release-waits-for-the-project-runner-release.md index ea693b8775..2adecc9a60 100644 --- a/docs/adr/0021-package-release-waits-for-the-project-runner-release.md +++ b/docs/adr/0021-package-release-waits-for-the-project-runner-release.md @@ -17,7 +17,10 @@ release-please runs only once every release the manifest names is published. the current dispatcher, and it settles the pass instead of polling, on both paths (`dispatcher-publish` post-publish and `release-please.yml`): the runner is published by another, approval-gated workflow, whose post-publish dispatches release-please, and that run's merge step - decides again. + decides again. A head that still pins the previous dispatcher is waited out only while + release-please can rebuild it; while a release `main`'s manifest names is unpublished (a merged + project runner release pull request still awaiting approval), the pass settles too, because that + publish dispatches release-please. - `check-release-please-gate` replaces the "the newest commit is a release commit" skip. It reads the manifest at the target commit and skips release-please while any release it names is missing or still a draft. A release that cannot be read fails the run rather than choosing a diff --git a/docs/dispatcher-pin-release-order.md b/docs/dispatcher-pin-release-order.md index b956960d8a..a5a04913ae 100644 --- a/docs/dispatcher-pin-release-order.md +++ b/docs/dispatcher-pin-release-order.md @@ -44,7 +44,11 @@ and is merged only by automation; the other two are the ones people merge. request is open, or while the release of the runner the head pin names (`uloop-project-runner-v`) is missing or still a draft: the runner release pull request raises that pin, so a package cut - before it would ship the previous runner. The release PR checks run one + before it would ship the previous runner. A head that still pins the + previous dispatcher is waited out only while release-please can rebuild it; + while a release `main`'s manifest names is unpublished (a merged project + runner release pull request still awaiting approval), the pass settles too, + because that publish dispatches release-please. The release PR checks run one pull request at a time, so the unity-package one always goes first; behind another component's checks it would outlast the time `post-publish` waits. 3. Merge the **project runner** release pull request and approve the From 6a1675068d827ab2733737f189a0868429aeee3b Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 30 Oct 2026 00:37:29 +0900 Subject: [PATCH 6/6] Dispatch release-please after a hand-started native CLI publish too The dispatch ran only for push runs, so a project runner published by workflow_dispatch (the retry path and the lost-push recovery) never started release-please. Now that the package release waits for the runner, a package release pull request held for that runner stayed draft until an unrelated push. Marking the release pull request tagged stays push-only, since only a push run's commit is known to be the release commit. --- .github/workflows/native-cli-publish.yml | 2 +- docs/dispatcher-pin-release-order.md | 5 +++ scripts/test-native-cli-publish-workflow.sh | 35 +++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/.github/workflows/native-cli-publish.yml b/.github/workflows/native-cli-publish.yml index 0f363247ef..e512c89918 100644 --- a/.github/workflows/native-cli-publish.yml +++ b/.github/workflows/native-cli-publish.yml @@ -467,7 +467,7 @@ jobs: run: scripts/mark-release-pr-tagged.sh - name: Dispatch release-please after native CLI publish - if: needs.build.outputs.should_publish == 'true' && github.event_name == 'push' + if: needs.build.outputs.should_publish == 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} TARGET_BRANCH: ${{ github.ref_name }} diff --git a/docs/dispatcher-pin-release-order.md b/docs/dispatcher-pin-release-order.md index a5a04913ae..bf94772d41 100644 --- a/docs/dispatcher-pin-release-order.md +++ b/docs/dispatcher-pin-release-order.md @@ -215,6 +215,11 @@ push would have started: - after merging the project runner release pull request: `gh workflow run native-cli-publish.yml --repo hatayama/unity-cli-loop --ref main`. +On a release commit, a hand-started release-please run is skipped by +`check-release-please-gate` until the release is published; the publish +workflow's `post-publish` starts release-please again once it is, also when the +publish itself was started by hand. + A manually started publish run does not check that `main`'s head is the release commit. A push run skips when the head commit does not stamp the version, but a manual run publishes `main`'s head as the release whenever the version has no diff --git a/scripts/test-native-cli-publish-workflow.sh b/scripts/test-native-cli-publish-workflow.sh index 2fcaee11b8..4207279b7f 100755 --- a/scripts/test-native-cli-publish-workflow.sh +++ b/scripts/test-native-cli-publish-workflow.sh @@ -77,6 +77,37 @@ post_publish_section() { ' "$WORKFLOW" } +post_publish_step_section() { + step=$1 + awk -v step="$step" ' + $0 == step { printing = 1; print; next } + printing && /^ - name:/ { exit } + printing { print } + ' "$WORKFLOW" +} + +assert_post_publish_step_contains() { + step=$1 + expected=$2 + if ! post_publish_step_section "$step" | grep -F -- "$expected" >/dev/null; then + echo "Expected post-publish step '$step' to contain: $expected" >&2 + exit 1 + fi +} + +assert_post_publish_step_lacks() { + step=$1 + unexpected=$2 + if [ -z "$(post_publish_step_section "$step")" ]; then + echo "Expected post-publish step '$step'." >&2 + exit 1 + fi + if post_publish_step_section "$step" | grep -F -- "$unexpected" >/dev/null; then + echo "Expected post-publish step '$step' not to contain: $unexpected" >&2 + exit 1 + fi +} + assert_post_publish_before() { earlier=$1 later=$2 @@ -263,6 +294,10 @@ test_post_publish_automation_remains_outside_the_privileged_job() { assert_contains " - name: Mark release PR as tagged" assert_count 2 " contents: write" assert_post_publish_before " - name: Mark release PR as tagged" " - name: Dispatch release-please after native CLI publish" + # A publish started by hand must dispatch release-please too, or a package release held for this runner waits for the next push. + assert_post_publish_step_lacks " - name: Dispatch release-please after native CLI publish" "github.event_name" + # Only a push run's commit is known to be the release commit the label belongs to. + assert_post_publish_step_contains " - name: Mark release PR as tagged" " if: github.event_name == 'push'" } test_build_and_publish_jobs_have_separate_trust_boundaries