This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-8096-1cbe857007a6526c6087f187667ef3b3c9ade546 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 68bf5aa81b51aac7536994c727ed40b5f87601de Author: Meng Wang <[email protected]> AuthorDate: Fri Sep 4 17:45:31 2026 +0000 ci: block the merge until each release/* label has its manager's approval (#8096) ### What changes were proposed in this PR? A `fix:` PR into `main` is auto-labeled with every actively-supported `release/*` branch, and the label alone decides the backport: whatever is labeled at merge time gets cherry-picked to the release branch, whether or not that branch's release manager has looked at it. The review request #6940 added is advisory only. Worse than the missing gate is what it does to the record. A manager who does not want a fix on their branch declines by staying silent, so the label stays on. The PR merges carrying `release/v1.2`, nothing is backported there, and months later that label says the fix shipped in 1.2 when it did not. This makes the approval **required to merge**. `Backport Approvals` is red while any `release/*` label on the PR lacks its manager's approval. Declining becomes an action rather than silence: the manager removes their branch's label, which clears the check for that branch. Since the merge waits for every remaining label to be approved, **the labels on a merged PR are exactly the branches Direct Backport Push then sends the fix to** — true by construction, not by anyone remembering to tidy up. The rule: managers gate their own branch and nothing else, so each decides alone — but the merge waits for all of them, so a fix cannot land on v1.2 while v1.3 is still undecided. `COMMENTED` reviews never change an approval, and a later `CHANGES_REQUESTED` or `DISMISSED` revokes one; any dismissal lands on `DISMISSED`, so that state proves an approval is not standing, never what the dismissed review had been. A manager who wrote the fix counts as approving it, since GitHub does not let anyone approve their own PR. An entry that omits `manager` stays ungated, but a label naming a branch `release-branches.yml` does not list at all is held back — retiring a branch means dropping its entry while its label lives on, and nobody is then designated to approve it. An unreadable review list holds every gated target back rather than guessing. `no-backport-needed` is reported as a contradiction rather than settled in its own favour. It says the fix reaches no release branch, but nothing removes the `release/*` labels it overrides, and `precheck.yml` documents adding it mid-review — by which point the auto-labeler has applied them. Passing the check there would merge a PR carrying a label promising a backport the push skips: the false record this exists to prevent. Removing either the label or the veto clears it. ### Where the required context is registered `Backport Approvals` is registered in **its own ruleset, scoped to `~DEFAULT_BRANCH`** — deliberately not in the Merge Queue ruleset, whose `ref_name.include` also covers `refs/heads/release/v1.1`, `v1.2` and `v1.3`. The workflow exists only on the default branch, and a `pull_request` run takes its workflows from the merge ref — for a PR into `release/vX.Y` that is the release-branch base plus a head branched from it, so neither carries the file. The context would never be produced there, and a required check that is never reported is not a red X but a permanent "waiting for status": 17 PRs into `release/v1.2` are open right now, nearly all draft backports this pipeline auto-opened on a conflict, and each would have become unmergeable. Scoping it to the default branch leaves them requiring exactly the three contexts they require today. The Merge Queue ruleset's context list carries a comment saying so, since that list sits beside the branch list a release manager edits when cutting a new line. ### A second, smaller repair `backport-auto-label.yml` checked out `base.sha` to read `release-branches.yml`. That is main's tip at the PR's last synchronize rather than now, so on a PR whose base predates #6941 — the commit that added the config — the file is absent and the step dies with a file-not-found. It has failed that way 20 times in production since 2026-07-27, each time leaving that PR unlabeled. Its checkout now reads the default branch, the revision this PR's new check and Direct Backport Push already read, so all three judge a backport by one config. That is the same failure this PR's own checkout is written to avoid, and it is worse here: an unlabeled PR can be fixed by hand, but a required context that dies on a missing file turns red for a reason its author cannot act on. ### Any related issues, documentation, discussions? Closes #8084. Follow-on to #6940, which built this backport pipeline. ### How was this PR tested? Proof runs on this PR, both paths exercised for real: | | run | result | | --- | --- | --- | | labelled `release/v1.2`, unapproved | [33723336875](https://github.com/apache/texera/actions/runs/33723336875) | fails — `BLOCKED release/v1.2 — needs an approving review from @xuang7` | | label removed (the decline action) | [33723396801](https://github.com/apache/texera/actions/runs/33723396801) | passes | | no `release/*` label at all | [33722853310](https://github.com/apache/texera/actions/runs/33722853310) | passes — "nothing to approve", the case that must never block since the context is required on every PR | The decision logic was also driven through cases locally: both managers approve, one approves, neither, an approval revoked by a dismissal, a manager who authored the PR, an entry with no manager, a label naming a branch absent from the config, an unreadable review list, `no-backport-needed` alongside `release/*` labels, `no-backport-needed` alone, and a PR based on a release branch. Each produced the expected cleared/blocked split and the expected wording — including that a dismissed review is never reported as a standing approval, and that "could not read the reviews" is never reported as "nobody approved". The workflow parses as YAML and its `github-script` body passes `node --check`; `.asf.yaml` parses and its rulesets resolve to the intended scopes; `release_branches.py` still parses the annotated config unchanged. Not covered locally: `merge_group` and `edited` triggers, and `pulls.listReviews` pagination, first execute on GitHub. **Rollout.** `.asf.yaml` is applied by ASF Infra after a merge to `main`, so this PR is not itself gated by the new context — it takes effect from the next PR. Two consequences worth knowing: pull requests already open into `main` at the cutover show `Backport Approvals` as "Expected" until some event (a push, a label, a review) triggers the workflow on them; and reverting the one ruleset lifts the gate without touching the workflow. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-5) --- .asf.yaml | 31 +++ .github/release-branches.yml | 37 ++- .github/workflows/backport-approval-check.yml | 319 ++++++++++++++++++++++++++ .github/workflows/backport-auto-label.yml | 76 ++++-- AGENTS.md | 8 +- CONTRIBUTING.md | 2 +- 6 files changed, 443 insertions(+), 30 deletions(-) diff --git a/.asf.yaml b/.asf.yaml index 8e2adbc6a7..42316db44a 100644 --- a/.asf.yaml +++ b/.asf.yaml @@ -112,6 +112,37 @@ github: - context: Required Checks - context: Check License Headers - context: Validate PR title + # `Backport Approvals` deliberately does NOT belong here — it + # lives in its own default-branch-only ruleset below. Adding it + # to this list strands every open PR into a release branch, since + # the workflow producing it exists only on the default branch. + + # Kept out of the Merge Queue ruleset above, which also covers the release + # branches: `backport-approval-check.yml` only exists on the default branch, + # and a `pull_request` run takes its workflows from the merge ref — so for a + # PR into release/vX.Y the check has no producer, the context never appears, + # and the PR waits on it forever. It has nothing to say there either: a PR + # already targeting a release branch is itself a backport and reports + # success. Scoping it to the default branch is what makes it enforceable + # without stranding every open backport PR. + - name: "Backport Approvals" + target: branch + enforcement: active + conditions: + ref_name: + exclude: [] + include: + - "~DEFAULT_BRANCH" + rules: + # Every release/* label must be approved by that branch's release + # manager before the merge, so a merged PR's labels are exactly the + # branches it was backported to + # (.github/workflows/backport-approval-check.yml). + - type: required_status_checks + parameters: + strict_required_status_checks_policy: false + required_status_checks: + - context: Backport Approvals - name: "Default Branch Protection" type: branch diff --git a/.github/release-branches.yml b/.github/release-branches.yml index 9460f25bbf..ff3e76854a 100644 --- a/.github/release-branches.yml +++ b/.github/release-branches.yml @@ -20,6 +20,9 @@ # (.github/workflows/backport-auto-label.yml), so backports are opt-out # rather than easy-to-forget opt-in; # - requests review from the branch's release manager on those PRs; +# - 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 @@ -29,14 +32,32 @@ # `release/v1.2`). # # `actively-supporting` controls the default: only actively-supporting branches -# are auto-labeled on new `fix:` PRs. An inactive branch stays a valid backport -# target — a maintainer can still add its label by hand and the apply-check / -# post-merge backport run as usual — it just isn't offered by default. Omitting -# the field defaults to true. When a branch reaches end-of-life, drop its entry -# entirely. -# -# `manager` is a GitHub username. It may be omitted (leave the branch as a -# backport target without an auto-requested reviewer), but is recommended. +# are auto-labeled on new `fix:` PRs, and only they get their manager's review +# requested. An inactive branch stays a valid backport target — a maintainer can +# add its label by hand, and the apply-check and post-merge backport run — but +# its manager must still approve before the PR can merge, and nothing asks them +# to: no review is requested for an inactive branch, so whoever adds the label +# should ping the manager themselves. Omitting the field defaults to true. When +# a branch reaches end-of-life, drop its entry entirely. +# +# `manager` is a GitHub username, and it is that branch's approval gate: the +# `release/*` label only *nominates* a target, and the fix is backported there +# only once the manager has approved the PR. A manager who wrote the fix counts +# as having approved it, since GitHub does not let anyone approve their own PR. +# +# Because that approval is required to merge, declining is an action rather than +# silence: the manager removes their branch's label. The author's part is to ask +# for that call, not to make it — and editing labels needs triage access, so any +# committer can remove one on a manager's behalf. +# +# Each manager decides alone, but the merge waits for all of them: with v1.2 and +# v1.3 both labeled it stays blocked until each manager has approved or removed +# their own label, and the fix then goes to exactly the labels left. +# +# `manager` may be omitted, which leaves the branch ungated — no auto-requested +# reviewer and no approval required — but setting one is strongly recommended. +# A `release/*` label naming a branch that is not listed here at all is not +# ungated: it is held back, since nobody is designated to approve it. targets: - branch: release/v1.3 manager: mengw15 diff --git a/.github/workflows/backport-approval-check.yml b/.github/workflows/backport-approval-check.yml new file mode 100644 index 0000000000..f37a52505c --- /dev/null +++ b/.github/workflows/backport-approval-check.yml @@ -0,0 +1,319 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Blocks the merge while a `release/*` label on the PR is not approved by that +# branch's release manager (.github/release-branches.yml). +# +# A label only nominates a branch. Before this, a manager declined by staying +# silent and the label stayed on, so a merged PR claimed a backport that never +# happened. Declining is now an action — approve, or remove the label — and the +# merge waits for every remaining label, so a merged PR's labels are the +# branches it reached. +# +# `Backport Approvals` is a required check on the default branch (.asf.yaml), so +# this must report on every PR into `main` or one that never runs it waits +# forever: hence no condition on title, labels, or base. A PR into a release +# branch is itself a backport and reports success. Merge groups are +# re-evaluated rather than waved through, and one whose PRs cannot be resolved +# fails closed. Do not rename the job — its name is the required context. + +name: Backport Approval Check + +on: + pull_request: + types: + - opened + - reopened + - synchronize + - labeled + - unlabeled + # Retargeting arrives as `edited`, and the verdict depends on the base + # branch: a backport PR into release/* has nothing to approve, the same + # PR moved onto main does. + - edited + pull_request_review: + types: + - submitted + - dismissed + merge_group: + +# Read-only: this job never writes to the PR, and `pull_request_review` runs +# with the base repository's token even for fork PRs. +permissions: + contents: read + pull-requests: read + +concurrency: + group: backport-approvals-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + backport-approvals: + # Do not rename — this display name is the required status check context + # referenced in .asf.yaml. + name: Backport Approvals + runs-on: ubuntu-latest + steps: + # The default branch, not the PR's head or base. Both can predate #6941, + # which added release-branches.yml — and `base.sha` is main's tip at the + # PR's last synchronize rather than now, stale for 33 of the open PRs into + # main. Either would fail the step below with a file-not-found and turn + # this required check red unclearably; backport-auto-label.yml pinned + # `base.sha` and failed that way 20 times in production. Reading main also + # lets a PR retargeted onto a release branch, where the config is absent, + # reach the report instead of dying before it. + # + # Backport Auto Label and Direct Backport Push read main too, so all three + # consumers judge a backport by the same config. A PR editing that config + # is judged by the pre-edit copy; those are `ci:`-typed and carry no + # `release/*` labels. + - name: Checkout + uses: actions/checkout@v7 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + + - name: Read release branches + id: targets + run: | + entries=$(python3 .github/scripts/release_branches.py .github/release-branches.yml) + echo "entries=${entries}" >> "$GITHUB_OUTPUT" + echo "Release branch targets: ${entries}" + + - name: Check release manager approvals + uses: actions/github-script@v9 + env: + ENTRIES: ${{ steps.targets.outputs.entries }} + with: + script: | + const { owner, repo } = context.repo; + const managers = new Map( + JSON.parse(process.env.ENTRIES || "[]").map((e) => [ + e.branch, + e.manager || "", + ]) + ); + + // A merge group carries several PRs and its commits are the squash + // merges, so the number is parsed out of each subject — the same way + // precheck.yml resolves them. + async function pullRequestNumbers() { + if (context.eventName !== "merge_group") { + return [context.payload.pull_request.number]; + } + const mg = context.payload.merge_group; + const { data: comparison } = await github.rest.repos.compareCommits({ + owner, repo, base: mg.base_sha, head: mg.head_sha, + }); + // An empty comparison would leave the loop below with nothing to + // check and publish a green context for a group whose approvals + // were never read. Fail closed, the same way the throw below does. + if (comparison.commits.length === 0) { + throw new Error(`no commits between ${mg.base_sha} and ${mg.head_sha}`); + } + return comparison.commits.map((c) => { + const m = c.commit.message.split("\n", 1)[0].match(/\(#(\d+)\)\s*$/); + if (!m) throw new Error(`no PR number in the message of ${c.sha}`); + return Number(m[1]); + }); + } + + // Latest state per login: COMMENTED never changes an approval, a + // later CHANGES_REQUESTED or DISMISSED revokes one, and listReviews + // is documented to return them chronologically. `null` means the + // list could not be read — kept distinct from empty, so "we did not + // look" is never reported as "nobody approved". + async function reviewStates(number) { + try { + const reviews = await github.paginate(github.rest.pulls.listReviews, { + owner, repo, pull_number: number, per_page: 100, + }); + const states = new Map(); + for (const review of reviews) { + const login = review.user?.login; + if (!login || review.state === "COMMENTED") continue; + states.set(login.toLowerCase(), review.state); + } + return states; + } catch (e) { + core.warning( + `listReviews for #${number} failed (${e.status ?? "?"}): ${e.message}` + ); + return null; + } + } + + // Managers gate their own branch and nothing else, so approvals + // compose: an approval from the v1.2 manager alone clears v1.2 and + // leaves v1.3 blocked. + function decide(targets, author, states) { + const authorLogin = (author || "").toLowerCase(); + const cleared = []; + const blocked = []; + for (const target of targets) { + // A label naming a branch the config does not list has nobody + // who could approve it — different from an entry that omits + // `manager`, and reachable, since retiring a branch drops its + // entry while the label lives on. + if (!managers.has(target)) { + blocked.push({ target, manager: "", state: "UNCONFIGURED" }); + continue; + } + const manager = managers.get(target); + if (!manager) { + cleared.push({ target, note: "no release manager configured for this branch" }); + continue; + } + // GitHub does not let anyone approve their own PR, so a manager + // who wrote the fix counts as having signed off. Logins are + // case-insensitive while `manager` is typed by hand, so fold both. + if (manager.toLowerCase() === authorLogin) { + cleared.push({ target, note: `@${manager} is the author of this PR` }); + continue; + } + const state = states ? states.get(manager.toLowerCase()) : "UNKNOWN"; + if (state === "APPROVED") { + cleared.push({ target, note: `approved by @${manager}` }); + } else { + blocked.push({ target, manager, state: state || "NONE" }); + } + } + return { cleared, blocked }; + } + + // Say only what the state proves. A dismissal lands on DISMISSED + // whatever the review was, so calling it an approval would put a + // guess in the manager's mouth. + function reason(e) { + if (e.state === "VETOED") { + return "`no-backport-needed` says this fix reaches no release " + + "branch, so this label contradicts it and would be left on a " + + "merged PR promising a backport nobody performs — remove the " + + "label, or remove `no-backport-needed`"; + } + if (e.state === "UNCONFIGURED") { + return "this branch is not listed in .github/release-branches.yml, " + + "so no release manager governs it — drop the label, or add the " + + "branch back to that file"; + } + if (e.state === "DISMISSED") { + return `@${e.manager}'s review was dismissed, so no approval stands`; + } + if (e.state === "CHANGES_REQUESTED") { + return `@${e.manager} requested changes and has not approved since`; + } + if (e.state === "UNKNOWN") { + return `this PR's reviews could not be read, so @${e.manager}'s ` + + `approval could not be confirmed`; + } + return `needs an approving review from @${e.manager}`; + } + + let numbers; + try { + numbers = await pullRequestNumbers(); + } catch (e) { + // Fail closed: a group whose approvals cannot be read is not one + // this can vouch for. Ejecting it is recoverable — its PRs + // re-queue, each still holding its own passing check. + core.setFailed( + `Could not resolve the pull requests in this merge group ` + + `(${e.message}). Refusing to pass a check that could not be ` + + `computed.` + ); + return; + } + + const lines = []; + let failed = false; + + for (const number of numbers) { + const { data: pr } = await github.rest.pulls.get({ + owner, repo, pull_number: number, + }); + const labels = (pr.labels || []).map((l) => l.name); + const releaseLabels = [...new Set( + labels.filter((n) => /^release\/.+$/.test(n)) + )].sort(); + // A PR already targeting a release branch is itself a backport and + // is not backported onward, so there is nothing here to approve. + const intoMain = pr.base.ref === "main"; + // The veto says the fix reaches no release branch, but nothing + // removes the labels it overrides, and precheck documents adding + // it mid-review — after the labeler has run. Merging in that state + // leaves a label promising a backport the push will skip, so the + // contradiction is reported rather than settled silently. + const vetoed = labels.includes("no-backport-needed"); + const targets = intoMain && !vetoed ? releaseLabels : []; + const contradictions = + intoMain && vetoed + ? releaseLabels.map((target) => ({ + target, manager: "", state: "VETOED", + })) + : []; + + if (numbers.length > 1) lines.push(`### PR #${number}`, ""); + if (targets.length === 0 && contradictions.length === 0) { + lines.push( + intoMain + ? "No `release/*` labels — nothing to approve." + : "Not a pull request into `main` — a backport is not " + + "itself backported onward, so there is nothing to approve.", + "" + ); + continue; + } + + const { cleared, blocked } = + targets.length > 0 + ? decide(targets, pr.user?.login, await reviewStates(number)) + : { cleared: [], blocked: [] }; + blocked.push(...contradictions); + for (const e of cleared) lines.push(`- **OK** \`${e.target}\` — ${e.note}`); + for (const e of blocked) lines.push(`- **BLOCKED** \`${e.target}\` — ${reason(e)}`); + lines.push(""); + if (blocked.length > 0) failed = true; + } + + if (failed) { + lines.push( + "Every `release/*` label on this PR must be approved by that " + + "branch's release manager before it can merge, so that the " + + "labels on a merged PR are exactly the branches it was " + + "backported to.", + "", + "A block naming a manager is that manager's call, and either " + + "answer clears it: **approve this PR** to send the fix to their " + + "branch, or **remove the label** to decline the backport to it. " + + "Authors: your part is to ask them to make that call, not to " + + "make it for them.", + "", + "Editing labels needs triage access, so outside contributors and " + + "bots cannot remove a label themselves — ask the release manager " + + "or any committer to remove it." + ); + } + + const report = lines.join("\n"); + core.summary.addHeading("Backport approvals", 3).addRaw(report); + await core.summary.write(); + core.info(report); + if (failed) { + core.setFailed( + "A release/* label on this PR is not approved by its release " + + "manager. See this job's summary for which one and what clears it." + ); + } diff --git a/.github/workflows/backport-auto-label.yml b/.github/workflows/backport-auto-label.yml index c2ee450100..68eb838dc5 100644 --- a/.github/workflows/backport-auto-label.yml +++ b/.github/workflows/backport-auto-label.yml @@ -22,6 +22,13 @@ # later edit never silently re-adds a label the author took off. `fix(ci):` PRs # only touch CI and are never backported, so they are left alone. # +# A label nominates a target; it does not decide one. The branch's release +# manager still has to approve the PR for the fix to land there, and that +# approval is required to merge (.github/workflows/backport-approval-check.yml). +# So a manager declines by removing their label, not by staying silent, and the +# labels left on a merged PR are exactly the branches it was backported to. +# Managers are configured in .github/release-branches.yml. +# # Every decision is also written back to the PR as a single, in-place-updated # report comment: one row per actively-supported release branch saying whether # the label was added (change detected on that branch) or skipped, and why. The @@ -61,14 +68,17 @@ jobs: if: ${{ github.event.pull_request.base.ref == 'main' }} runs-on: ubuntu-latest steps: - # Check out the base commit (a trusted commit already on main) so the - # release-branches config and its parser come from the base repo, never - # from the (untrusted) PR head. pull_request_target runs with write - # permissions, so we must not execute PR-controlled code here. - - name: Checkout base + # The default branch: a ref that does not derive from the pull request, + # which matters because pull_request_target runs with write permissions. + # `base.sha` was the previous choice and is stale — main's tip at the PR's + # last synchronize — so on a PR untouched since #6941 the step below dies + # with a file-not-found, as it has 20 times since 2026-07-28. A labeler + # that dies applies no `release/*` label, Backport Approvals then finds + # nothing to approve and passes, and the fix is silently never backported. + - name: Checkout the default branch uses: actions/checkout@v7 with: - ref: ${{ github.event.pull_request.base.sha }} + ref: ${{ github.event.repository.default_branch }} persist-credentials: false - name: Read release branches @@ -125,6 +135,12 @@ jobs: } const author = pr.user?.login; + // Logins are case-insensitive while `manager` is hand-typed, so + // fold both. Backport Approvals folds them too and the two must + // agree: otherwise it clears the branch while this report tells + // the author to wait for an approval that cannot come. + const authorLogin = (author || "").toLowerCase(); + const managerIsAuthor = (m) => (m || "").toLowerCase() === authorLogin; const currentLabels = new Set((pr.labels || []).map((l) => l.name)); // Hard manual override: a maintainer marks a PR that must never be @@ -205,11 +221,22 @@ jobs: for (const entry of entries) { const label = entry.branch; const manager = entry.manager; + // Say who has to sign off, so the label is not mistaken for the + // decision. A manager who wrote the fix needs no approval — + // GitHub does not let anyone approve their own PR. + const gate = + manager && !managerIsAuthor(manager) + ? ` @${manager} decides: approving sends the fix here, ` + + `removing this label declines it. The merge waits on one ` + + `or the other.` + : ""; // Only actively-supporting branches are analyzed / auto-labeled. - // An inactive branch stays a valid manual target (label it by - // hand and the apply-check / post-merge backport still run) — it - // just isn't offered by default and is left out of the report. + // An inactive branch stays a valid manual target — label it by + // hand and the apply-check and post-merge backport still run — + // but its manager must approve before the PR can merge, and the + // `continue` below means no review is requested for it, so + // whoever adds the label should ping the manager themselves. if (!entry.active) { core.info(`${label} is not actively-supporting; not auto-labeling.`); continue; @@ -219,7 +246,8 @@ jobs: rows.push({ branch: label, status: "labeled", - note: "Already labeled — this fix is queued to backport here.", + note: + "Already labeled — this fix is queued to backport here." + gate, }); continue; } @@ -230,7 +258,9 @@ jobs: status: "declined", note: "Label was removed earlier (opt-out); not re-added. Re-add " + - "it by hand if this fix should be backported here after all.", + "it by hand if this fix should be backported here after all — " + + "the branch's release manager then has to approve before this " + + "PR can merge.", }); continue; } @@ -249,7 +279,9 @@ jobs: `modifies exist on this branch (${fileList}). The fix may ` + "target code that isn't on this release, or the files were " + "moved/renamed after the branch was cut. **Please check and " + - `add \`${label}\` by hand if this fix should be backported here.**`, + `add \`${label}\` by hand if this fix should be backported here** ` + + "— its release manager then has to approve before this PR can " + + "merge.", }); continue; } @@ -263,14 +295,14 @@ jobs: }); core.info(`Added ${label} to PR #${pr.number}.`); - const note = analysis.checked.length > 0 + const note = (analysis.checked.length > 0 ? "Change detected on this branch — label added; this fix is queued to backport here." - : "Label added by default (this fix only adds files); the pre-merge backport check confirms it applies."; + : "Label added by default (this fix only adds files); the pre-merge backport check confirms it applies.") + gate; rows.push({ branch: label, status: "labeled", note }); - if (manager && manager !== author) { + if (manager && !managerIsAuthor(manager)) { reviewRequests.push({ manager, branch: label }); - } else if (manager === author) { + } else if (managerIsAuthor(manager)) { core.info(`Release manager ${manager} is the author; skipping review request.`); } } @@ -370,7 +402,9 @@ jobs: }); core.info(`Requested review from ${req.manager} for ${req.branch}.`); const row = rowByBranch.get(req.branch); - if (row) row.note += ` Requested review from @${req.manager}.`; + // The row's gate sentence already names the manager, so the + // ping only needs to say that it happened. + if (row) row.note += " Review requested."; } catch (e) { core.warning( `Could not request review from ${req.manager} (status ${e.status ?? "?"}): ${e.message}` @@ -386,8 +420,12 @@ jobs: await upsertReport( "### Backport auto-label report\n\n" + "This `fix:` PR was checked against each actively-supported " + - "release branch. `release/*` labels drive the post-merge " + - "backport, so add or remove one to change where this fix lands.\n\n" + + "release branch. A `release/*` label nominates a backport target; " + + "the branch's release manager approving this PR is what sends the " + + "fix there. The required **Backport Approvals** check stays red " + + "until every label below is approved, so each manager either " + + "approves or removes their own label — which is why the labels " + + "left on a merged PR are exactly the branches it reached.\n\n" + "| Release branch | Analysis |\n| --- | --- |\n" + rows + "\n\n" + diff --git a/AGENTS.md b/AGENTS.md index c94002c86a..5c473b2a30 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -258,5 +258,9 @@ diff -> pr-labeler -> labels on PR -> required-checks maps labels to stacks -> C suspect breaks the frontend)? **Add the relevant label manually**. - Empty stack union (docs-only / dev-only / `dependencies` / `feature` / `fix` / `refactor` / `release/*` only) skips every build stack on purpose. -- `release/*` labels select backport targets; removing one cancels that - backport. +- `release/*` labels nominate backport targets. A nominated target is + backported only once that branch's release manager — listed in + [`.github/release-branches.yml`](.github/release-branches.yml) — approves the + PR, and the required `Backport Approvals` check blocks the merge until every + `release/*` label on the PR is approved. A manager declines by removing their label, so + the labels on a merged PR are exactly the branches it was backported to. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 99e3e00988..3e6a74acdc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -138,7 +138,7 @@ yarn format:fix ### 4. PR Review - [ ] Ask a Texera Committer (by commenting on the PR) to triage your PR, i.e., request a reviewer, and assign the PR to you. - [ ] Add appropriate labels such as `fix`, `enhancement`, `docs`, etc. -- [ ] If the change should also land in a release branch, add the matching `release/<branch>` label (e.g. `release/v1.1.0-incubating`); the change will be backported to that branch automatically. +- [ ] If the change should also land in a release branch, add the matching `release/<branch>` label (e.g. `release/v1.2`). The label nominates the branch; the change is backported there automatically once that branch's release manager — listed in [`.github/release-branches.yml`](.github/release-branches.yml) — approves the PR. The required `Backport Approvals` check stays red until every `release/*` label on the PR is approved, so ask each manager to either approve or remove their label; [...] - [ ] Ensure that all CI checks pass (see [GitHub Actions](https://github.com/apache/texera/actions)). - [ ] Fully test your changes locally.
