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/native-cli-publish.yml b/.github/workflows/native-cli-publish.yml index 5b88fcaaa2..e512c89918 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' + 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/.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/.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/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/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..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" ) @@ -259,16 +260,21 @@ 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 + 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 + // 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 @@ -285,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, @@ -338,32 +378,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..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 @@ -28,6 +28,16 @@ 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 + // 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 @@ -88,6 +98,24 @@ 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 "): + 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 + } + 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 +161,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 { @@ -150,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 } @@ -393,7 +426,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 +445,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 +638,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 +930,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 +1048,160 @@ 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 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), + 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) + } + firstRunnerGateRead := -1 + rebasedPinRead := -1 + for index, commandLine := range stub.commandLog { + if rebasedPinRead == -1 && strings.Contains(commandLine, "?ref=rebased456") { + rebasedPinRead = index + } + if firstRunnerGateRead == -1 && strings.Contains(commandLine, "--components--uloop-project-runner") { + firstRunnerGateRead = index + } + } + 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 new file mode 100644 index 0000000000..561f093c07 --- /dev/null +++ b/cli/release-automation/internal/automation/release_please_gate.go @@ -0,0 +1,165 @@ +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 + } + + // 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, err := unpublishedManifestReleases(ctx, runOutput, config.repository, config.ref) + if err != nil { + writeMergePackageReleasePRLine(stderr, err) + return 1 + } + + 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 +} + +// 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), + repository string, + ref string, +) ([]string, error) { + content, err := githubFileContentAtRef(ctx, runOutput, repository, releasePleaseManifestRelativePath, 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, 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(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/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..2adecc9a60 --- /dev/null +++ b/docs/adr/0021-package-release-waits-for-the-project-runner-release.md @@ -0,0 +1,77 @@ +# 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. 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 + 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..bf94772d41 100644 --- a/docs/dispatcher-pin-release-order.md +++ b/docs/dispatcher-pin-release-order.md @@ -35,25 +35,43 @@ 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. 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. 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 +110,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 +120,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 +137,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 @@ -186,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/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/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-native-cli-publish-workflow.sh b/scripts/test-native-cli-publish-workflow.sh index d48c0f9b9c..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 @@ -262,7 +293,11 @@ 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" + # 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 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"]' 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 }}'