From ad317e4435c26aa206d668e6726828be07b2043b Mon Sep 17 00:00:00 2001 From: mengw15 <125719918+mengw15@users.noreply.github.com> Date: Mon, 21 Sep 2026 00:36:29 -0700 Subject: [PATCH 1/3] ci: open a pull request for a clean backport instead of pushing it Direct Backport Push cherry-picks a cleanly-applying fix onto the release branch and pushes it. Every one of those pushes has been rejected since 2026-07-24 -- release/* is covered by the Merge Queue ruleset, which requires a pull request for everyone -- and ASF policy asks for prior Infrastructure authorization before an automated service writes a branch subject to official release. #8379 tried to exempt the Actions app from the ruleset; GitHub refuses to create that bypass, so the fast path has never once worked since it broke. Both outcomes now open a pull request. The conflicted one is unchanged: a draft, assigned to its author. A clean one opens ready for review, assigned to nobody, because there is no code for anyone to write on it. What a clean backport still needs is its checks started. GitHub creates no workflow run for anything GITHUB_TOKEN does, so a bot-opened pull request has none, and nothing will arrive on its own: with no run there is no re-run, and none of the three required workflows listens for ready_for_review. Reopening the pull request, or pushing to its branch, is what emits an event they subscribe to -- so the PR says exactly that, in the comment the conflict path already uses for its instructions. That leaves the release manager with three ordinary buttons: reopen, approve, and auto-merge if they would rather not come back for the checks. Nothing here depends on a token's pull-request scope, on an Actions bypass, or on a close/reopen the workflow performs itself -- the paths that can only be proven in production, and that fail quietly when they are wrong. push_entries is now always empty, leaving push-backports unreachable. Removing it is left to a separate change, so that this one is a behaviour change and that one is a pure deletion. --- .github/workflows/direct-backport-push.yml | 116 +++++++++++++++++---- 1 file changed, 94 insertions(+), 22 deletions(-) diff --git a/.github/workflows/direct-backport-push.yml b/.github/workflows/direct-backport-push.yml index 149c6085566..3652d86813c 100644 --- a/.github/workflows/direct-backport-push.yml +++ b/.github/workflows/direct-backport-push.yml @@ -314,11 +314,23 @@ jobs: `PR #${pullRequest.number}: push=[${push.join(", ")}] pr=[${pr.join(", ")}]` ); + // Both outcomes open a pull request. Pushing a cherry-pick + // straight onto a release branch is rejected by the Merge Queue + // ruleset that covers them (#8377), and ASF policy asks for prior + // Infrastructure authorization before an automated service writes + // a branch subject to official release -- so a backport travels + // the way every other change to a release branch travels. + // + // `clean` only changes what the pull request says and whether it + // opens as a draft: a clean cherry-pick needs nobody to touch the + // code, a conflicted one is still its author's to finish. for (const target of push) { - pushEntries.push({ + prEntries.push({ pr_number: pullRequest.number, merge_sha: commit.sha, target, + manager: releaseManagers.get(target) || "", + clean: "true", }); } for (const target of pr) { @@ -327,12 +339,17 @@ jobs: merge_sha: commit.sha, target, manager: releaseManagers.get(target) || "", + clean: "false", }); } } core.info(`Push entries: ${JSON.stringify(pushEntries)}`); core.info(`PR entries: ${JSON.stringify(prEntries)}`); + // Always empty now: every target opens a pull request. The + // push-backports job below is therefore unreachable, and is + // removed separately so this change is a behaviour change and + // that one is a pure deletion. core.setOutput("push_entries", JSON.stringify(pushEntries)); core.setOutput("pr_entries", JSON.stringify(prEntries)); core.setOutput("has_push", pushEntries.length > 0 ? "true" : "false"); @@ -822,9 +839,14 @@ jobs: needs: discover if: ${{ needs.discover.outputs.has_pr == 'true' }} runs-on: ubuntu-latest - name: "open backport PR #${{ matrix.pr_number }} to ${{ matrix.target }}" + name: "backport PR #${{ matrix.pr_number }} to ${{ matrix.target }}" strategy: fail-fast: false + # One at a time, as push-backports did: a single push can carry several + # fixes for the same release branch, and each cherry-pick is taken + # against that branch as it stands, so preparing them concurrently would + # race their branches against each other. + max-parallel: 1 matrix: include: ${{ fromJson(needs.discover.outputs.pr_entries) }} steps: @@ -844,7 +866,7 @@ jobs: run: | bash ./.github/scripts/create-backport-branch.sh \ "${MERGE_SHA}" "${TARGET_BRANCH}" "${PR_NUMBER}" - - name: Open draft backport PR + - name: Open backport PR uses: actions/github-script@v9 env: MERGE_SHA: ${{ matrix.merge_sha }} @@ -857,22 +879,38 @@ jobs: CONFLICT_FILES: ${{ steps.branch.outputs.conflict_files }} SUBJECT: ${{ steps.branch.outputs.subject }} FEATURE_ABSENT: ${{ steps.branch.outputs.feature_absent }} + CLEAN: ${{ matrix.clean }} with: - # Open the draft as github-actions[bot], not the PAT owner, by using - # the default GITHUB_TOKEN. Trade-off: a GITHUB_TOKEN-opened PR does - # not trigger pull_request CI — acceptable because this is a draft for - # manual conflict/build resolution, so CI fires once the human pushes - # their fix to the branch. + # Opened as github-actions[bot], not as the PAT owner, by using the + # default GITHUB_TOKEN — so nobody is recorded as the author of a + # pull request they did not write, and the token owner is never + # barred from approving one. + # + # The cost is that GitHub creates no workflow run for anything + # GITHUB_TOKEN does, so the pull request opens with no checks. A + # conflicted one does not care: CI fires when its author pushes a + # resolution. A clean one has nobody to push anything, so the comment + # posted on it asks for the one action that starts them. github-token: ${{ secrets.GITHUB_TOKEN }} script: | const { MERGE_SHA, TARGET_BRANCH, PR_NUMBER, MANAGER, BRANCH, VERSION, HAD_CONFLICT, CONFLICT_FILES, SUBJECT, - FEATURE_ABSENT, + FEATURE_ABSENT, CLEAN, } = process.env; const { owner, repo } = context.repo; const prNumber = Number(PR_NUMBER); const hadConflict = HAD_CONFLICT === "true"; + // Clean means the cherry-pick applied and the backported tree built + // green before the merge: nothing here needs a human's hands on the + // code, only the release manager's confirmation. + // + // CLEAN carries the pre-merge preflight's verdict, but the branch + // step above cherry-picked again just now, onto a release branch + // that may have moved since. Trust that fresher result too, or a + // race would open a tree full of conflict markers and call it + // conflict-free. + const clean = CLEAN === "true" && !hadConflict; const runUrl = `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`; @@ -934,7 +972,19 @@ jobs: const conflictList = CONFLICT_FILES.trim() ? CONFLICT_FILES.trim().split(/\s+/).map((f) => `- \`${f}\``).join("\n") : ""; - const statusLine = hadConflict + const statusLine = clean + ? `**Clean cherry-pick — nothing here was edited by hand.** It applied to ` + + `\`${TARGET_BRANCH}\` without conflicts, and the backported tree built ` + + `green before #${prNumber} merged.\n\n` + + `**Close this PR and reopen it to start its required checks.** This ` + + `pull request was opened by \`github-actions[bot]\`, and GitHub starts ` + + `no workflow run for anything the Actions token does, so it has no ` + + `checks yet. Reopening it (or pushing any commit to the branch) emits ` + + `the event they subscribe to. *Marking it ready for review will not* — ` + + `none of the three required workflows listen for that.\n\n` + + `After that: approve, and enable auto-merge if you would rather not ` + + `come back when the checks finish.` + : hadConflict ? `The cherry-pick **conflicted** and was committed with conflict markers. ` + `Resolve the conflicts on this branch, then mark this PR ready for review.` : `The cherry-pick applied cleanly but the backported tree failed its ` + @@ -1005,9 +1055,13 @@ jobs: ``, `### How was this PR tested?`, ``, - `Release-branch CI runs on this branch once the ` + - `${hadConflict ? "conflicts are resolved" : "build is fixed"} and ` + - `this PR is marked ready for review.`, + clean + ? `The backported tree built green before #${prNumber} merged. ` + + `Release-branch CI runs on this PR once it is reopened — see the ` + + `comment below.` + : `Release-branch CI runs on this branch once the ` + + `${hadConflict ? "conflicts are resolved" : "build is fixed"} and ` + + `this PR is marked ready for review.`, ``, `### Was this PR authored or co-authored using generative AI tooling?`, ``, @@ -1016,9 +1070,12 @@ jobs: pr = (await github.rest.pulls.create({ owner, repo, base: TARGET_BRANCH, head: BRANCH, - title, body, draft: true, + // A clean backport opens ready for review: nobody has code to + // write on it, and a draft would only suggest otherwise. It + // still needs its checks started -- the comment below says how. + title, body, draft: !clean, })).data; - core.info(`Opened draft backport PR #${pr.number}.`); + core.info(`Opened ${clean ? "ready" : "draft"} backport PR #${pr.number}.`); // Post the how-to-finish instructions as a comment on the new // backport PR (kept out of the templated body). @@ -1044,7 +1101,10 @@ jobs: } catch (e) { core.warning(`Could not fetch PR #${prNumber} author: ${e.message}`); } - if (author) { + // Only a backport somebody has to finish gets assigned. A clean + // one is the release manager's to review, and assigning its author + // would tell them they have work they do not have. + if (author && !clean) { try { await github.rest.issues.addAssignees({ owner, repo, issue_number: pr.number, assignees: [author], @@ -1071,11 +1131,17 @@ jobs: try { await github.rest.issues.createComment({ owner, repo, issue_number: prNumber, - body: - `Backport PR opened: draft #${pr.number} (${prUrl}) to ` + - `\`${TARGET_BRANCH}\`` + - (author ? `, assigned to @${author}` : "") + - ` — needs manual work because ${reason}.`, + body: clean + ? `Backport PR opened: #${pr.number} (${prUrl}) to ` + + `\`${TARGET_BRANCH}\` — the cherry-pick was clean, so it is ` + + `ready for review` + + (MANAGER ? `, waiting on @${MANAGER}` : "") + + `. It needs to be reopened once to start its checks; the PR ` + + `says how.` + : `Backport PR opened: draft #${pr.number} (${prUrl}) to ` + + `\`${TARGET_BRANCH}\`` + + (author ? `, assigned to @${author}` : "") + + ` — needs manual work because ${reason}.`, }); } catch (e) { core.warning(`Could not comment on #${prNumber}: ${e.message}`); @@ -1083,11 +1149,17 @@ jobs: // A failure commit status on the merge commit keeps the "needs // backport" signal visible on main next to the successful ones. + // A clean backport carries it too: it is open, not landed, and + // something still has to be done to it. The status turning green + // is not what this workflow reports -- the merge into the release + // branch is. try { await github.rest.repos.createCommitStatus({ owner, repo, sha: MERGE_SHA, state: "failure", context: `backport/${TARGET_BRANCH}`, - description: `Draft backport PR #${pr.number} opened`, + description: clean + ? `Backport PR #${pr.number} open, awaiting review` + : `Draft backport PR #${pr.number} opened`, target_url: prUrl, }); } catch (e) { From b70d4f3d4d4e8b1528ce7d0e430949bba510c7e5 Mon Sep 17 00:00:00 2001 From: mengw15 <125719918+mengw15@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:14:12 -0700 Subject: [PATCH 2/3] ci: address the review on the clean-backport pull request Three findings from the review, all of them real. A re-run force-pushes a fresh cherry-pick onto the release branch as it stands then, which may have moved; the open pull request was left alone, so a tree that now conflicts could keep sitting there ready for review under a comment saying nothing in it was edited by hand. Rewriting the PR from the workflow would overwrite whatever a human has done to it since, so it now says so on the PR, reds the backport status and fails the job. A clean backport has no assignee, and the review request was still guarded by MANAGER !== author -- a guard that made sense when the author owned the PR. This one is owned by github-actions[bot], so a fix written by the branch's own release manager notified nobody at all. The manager is now requested on every clean backport. The three places that still described the fast path -- the entries comment here, release-branches.yml and create-backport-branch.sh's header -- say what happens now. --- .github/release-branches.yml | 9 ++-- .github/scripts/create-backport-branch.sh | 13 ++--- .github/workflows/direct-backport-push.yml | 58 ++++++++++++++++++++-- 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/.github/release-branches.yml b/.github/release-branches.yml index ff3e76854a8..a092d19f20e 100644 --- a/.github/release-branches.yml +++ b/.github/release-branches.yml @@ -23,10 +23,11 @@ # - blocks the merge until that manager has approved the PR, or removed the # label to decline (.github/workflows/backport-approval-check.yml, the # required `Backport Approvals` check — see `manager` below); -# - drives the post-merge backport (.github/workflows/direct-backport-push.yml): -# a green pre-merge backport check cherry-picks straight to the branch, a -# red one opens a draft backport PR assigned to the author with the manager -# as reviewer. +# - drives the post-merge backport (.github/workflows/direct-backport-push.yml), +# which always opens a pull request against the branch: a green pre-merge +# backport check opens it ready for review with the manager requested, a +# red one opens a draft assigned to the author with the manager as +# reviewer. # # The label name equals the branch name (e.g. branch `release/v1.2` <-> label # `release/v1.2`). diff --git a/.github/scripts/create-backport-branch.sh b/.github/scripts/create-backport-branch.sh index 32c8fe5e693..8fda47be175 100755 --- a/.github/scripts/create-backport-branch.sh +++ b/.github/scripts/create-backport-branch.sh @@ -16,12 +16,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Builds and pushes the branch behind an auto-opened backport PR, used when the -# pre-merge backport check was red so a straight cherry-pick to the release -# branch is unsafe. The cherry-pick is committed even when it conflicts: the -# tree carries the conflict markers, and the human resolves them in the PR -# rather than starting the backport from scratch (the same approach the common -# backport bots take). +# Builds and pushes the branch behind an auto-opened backport PR. Every backport +# goes through one now, whatever the pre-merge backport check said: a release +# branch takes no direct push, so the check only decides whether the PR opens +# ready for review or as a draft. The cherry-pick is committed even when it +# conflicts: the tree carries the conflict markers, and the human resolves them +# in the PR rather than starting the backport from scratch (the same approach +# the common backport bots take). # # Usage: create-backport-branch.sh # Writes to $GITHUB_OUTPUT (or stdout when unset): branch, version, diff --git a/.github/workflows/direct-backport-push.yml b/.github/workflows/direct-backport-push.yml index 3652d86813c..220d295fba3 100644 --- a/.github/workflows/direct-backport-push.yml +++ b/.github/workflows/direct-backport-push.yml @@ -273,10 +273,12 @@ jobs: const commits = await listPushCommits(); core.info(`Push contains ${commits.length} commit(s).`); - // push_entries: (PR, target) pairs cherry-picked straight to the - // release branch, in commit order. - // pr_entries: (PR, target, manager) pairs that get an auto-opened - // draft backport PR for the author to finish. + // pr_entries: (PR, target, manager, clean) pairs that each get an + // auto-opened backport PR, in commit order -- ready for review + // when the cherry-pick is clean, a draft for its author to finish + // when it is not. + // push_entries: empty. Kept until the unreachable push-backports + // job is removed, so that removal is a pure deletion. const pushEntries = []; const prEntries = []; const seenPrNumbers = new Set(); @@ -968,6 +970,46 @@ jobs: let pr = existing.data[0]; if (pr) { core.info(`Backport PR already open: #${pr.number}; not duplicating.`); + // The branch was just force-pushed with a fresh cherry-pick onto + // the release branch as it stands now, which may have moved since + // the PR was opened. If that changed the outcome, the PR in front + // of the reviewer no longer describes its own branch -- a tree + // with conflict markers still reading "nothing here was edited by + // hand", or a resolved one still marked draft. Neither is safe to + // leave standing, and rewriting the PR from here would overwrite + // whatever a human has since done to it. Say so and stop. + if (pr.draft === clean) { + const wanted = clean ? "ready for review" : "a draft"; + const note = + `This backport was re-run and its branch force-pushed. The ` + + `cherry-pick onto \`${TARGET_BRANCH}\` is now ` + + `**${clean ? "clean" : "conflicted"}**, so this PR should be ` + + `${wanted} — it is not, and what it says above no longer ` + + `describes its branch. Check the branch before reviewing: ` + + `[automation run](${runUrl}).`; + try { + await github.rest.issues.createComment({ + owner, repo, issue_number: pr.number, body: note, + }); + } catch (e) { + core.warning(`Could not comment on #${pr.number}: ${e.message}`); + } + try { + await github.rest.repos.createCommitStatus({ + owner, repo, sha: MERGE_SHA, state: "failure", + context: `backport/${TARGET_BRANCH}`, + description: `Backport PR #${pr.number} no longer matches its branch`, + target_url: `${context.serverUrl}/${owner}/${repo}/pull/${pr.number}`, + }); + } catch (e) { + core.warning(`Could not set commit status: ${e.message}`); + } + core.setFailed( + `#${pr.number} is ${pr.draft ? "a draft" : "ready for review"} but the ` + + `re-run cherry-pick is ${clean ? "clean" : "conflicted"}; it needs a look.` + ); + return; + } } else { const conflictList = CONFLICT_FILES.trim() ? CONFLICT_FILES.trim().split(/\s+/).map((f) => `- \`${f}\``).join("\n") @@ -1113,7 +1155,13 @@ jobs: core.warning(`Could not assign ${author}: ${e.message}`); } } - if (MANAGER && MANAGER !== author) { + // The backport PR is owned by github-actions[bot], so the source + // author is not its author and the guard below only exists to keep + // a conflicted backport's assignee from being asked to review their + // own resolution. A clean one has no assignee, and its manager must + // be asked even when they wrote the fix -- otherwise a fix authored + // by the release manager notifies nobody at all. + if (MANAGER && (clean || MANAGER !== author)) { try { await github.rest.pulls.requestReviewers({ owner, repo, pull_number: pr.number, reviewers: [MANAGER], From a71cc51ecd288c2a53a866e8d67b2a4bcacc52e6 Mon Sep 17 00:00:00 2001 From: mengw15 <125719918+mengw15@users.noreply.github.com> Date: Mon, 21 Sep 2026 14:10:41 -0700 Subject: [PATCH 3/3] ci: correct the remaining fast-path descriptions backport-publish.yml's clean summary is what a contributor reads on their own PR, and it promised a cherry-pick straight to the release branch. backport-checks.yml's header described the same split. The comment on the backport commit status claimed the merge into the release branch is what turns it green. Nothing does: the status is never updated after it is written, which is why #8562's v1.2 status still names the draft #8585 that merged on 2026-09-19. The comment now says that, and where the fix belongs. --- .github/workflows/backport-checks.yml | 3 ++- .github/workflows/backport-publish.yml | 2 +- .github/workflows/direct-backport-push.yml | 13 ++++++++++--- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/.github/workflows/backport-checks.yml b/.github/workflows/backport-checks.yml index 7288928ef3d..1244d8f7d27 100644 --- a/.github/workflows/backport-checks.yml +++ b/.github/workflows/backport-checks.yml @@ -28,7 +28,8 @@ # because this workflow runs on `pull_request`, whose token is read-only on # fork PRs (renovate, external contributors) and so cannot create check runs. # Post-merge, Direct Backport Push reads the neutral/success check and the -# backport build legs to decide push-straight vs. open-a-draft-PR. +# backport build legs to decide whether the backport PR it opens is ready for +# review or a draft for its author to finish. name: Backport Checks on: diff --git a/.github/workflows/backport-publish.yml b/.github/workflows/backport-publish.yml index fde3467d644..5cd309e685d 100644 --- a/.github/workflows/backport-publish.yml +++ b/.github/workflows/backport-publish.yml @@ -93,7 +93,7 @@ jobs: ? `Cherry-picks cleanly onto ${r.target}` : `Conflicts on ${r.target} — needs a manual backport`, summary: applied - ? `The change applies cleanly onto \`${r.target}\`. If this PR merges it is cherry-picked straight to the release branch.` + ? `The change applies cleanly onto \`${r.target}\`. If this PR merges, a backport PR opens against that branch, ready for review.` : `The change does not apply cleanly onto \`${r.target}\`. This is advisory and does not block merging: after merge, Direct Backport Push opens a draft backport PR for manual conflict resolution. Remove the \`${r.target}\` label to decline the backport.`, }, }); diff --git a/.github/workflows/direct-backport-push.yml b/.github/workflows/direct-backport-push.yml index 220d295fba3..bac43d6a7d1 100644 --- a/.github/workflows/direct-backport-push.yml +++ b/.github/workflows/direct-backport-push.yml @@ -1198,9 +1198,16 @@ jobs: // A failure commit status on the merge commit keeps the "needs // backport" signal visible on main next to the successful ones. // A clean backport carries it too: it is open, not landed, and - // something still has to be done to it. The status turning green - // is not what this workflow reports -- the merge into the release - // branch is. + // something still has to be done to it. + // + // Nothing updates this status afterwards -- not when the backport + // PR merges, not when it is closed. That predates this change and + // already leaves landed backports red (#8562's v1.2 status still + // names the draft #8585, which merged on 2026-09-19); routing the + // clean ones here widens it from some commits to all of them. + // Settling it needs a handler on the backport PR closing, which + // belongs with the removal of push-backports -- the only place in + // the repository that writes a success for this context. try { await github.rest.repos.createCommitStatus({ owner, repo, sha: MERGE_SHA, state: "failure",