Skip to content

Commit e0040dc

Browse files
Yicong-Huangclaude
andauthored
fix(ci): backport every labeled PR in a multi-commit push, not just the head (#6120)
### What changes were proposed in this PR? `direct-backport-push.yml` resolved exactly one PR per push event — from `head_commit.message`, falling back to `commits/{context.sha}/pulls`. When several auto-merges land on `main` in a single ref update, only the head commit was examined and the `release/*` labels on every other PR in the same push were silently dropped: no run, no commit status, no failure comment. An audit of 2026-06-01 → 2026-07-04 found 2 of 43 labeled PRs lost this way (#5802, coalesced behind #5812; and #6111, coalesced behind #6112). Changes to the `discover` job: - Enumerate **all** commits in the push via `compareCommits(payload.before...context.sha)` (oldest first), falling back to the head commit if the compare fails or `before` is unavailable. - Resolve a PR per commit (same two strategies as before: `(#N)` title suffix, then `commits/{sha}/pulls` with backoff) and collect green `release/*` targets per PR. - Output one matrix entry per `(pr_number, merge_sha, target)` pair instead of a single PR + target list. Changes to the `push-backports` job: - Matrix is now `include: <entries>` with `max-parallel: 1`, so cherry-picks apply in commit order and two legs never race pushes to the same release branch. - `MERGE_SHA`/`PR_NUMBER` come from the matrix entry instead of `github.sha`/a single discover output; the cherry-pick, commit statuses, and PR comments all annotate the right commit and PR. - Job legs are named `backport #<pr> to <target>`; the failure-annotation job-URL matcher was updated accordingly. ### Any related issues, documentation, discussions? Closes #6119 ### How was this PR tested? - YAML parses (PyYAML) and all three embedded `github-script` blocks pass `node --check`. - Dry-ran the new discover logic (read-only API calls) against the two historical coalesced pushes: the 2026-07-04 push (`5c4a963f7...9cd8acf`) yields entries for **both** #6111 and #6112 with green targets, and the 2026-06-20 push range yields #5802 while correctly skipping the unlabeled #5809/#5811/#5812. The old logic produced only the head PR in each case. - Note: this fix is not retroactive — #5802 and #6111 still need manual backporting to `release/v1.2`. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Fable 5) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 6de8a48 commit e0040dc

1 file changed

Lines changed: 118 additions & 63 deletions

File tree

.github/workflows/direct-backport-push.yml

Lines changed: 118 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -33,29 +33,59 @@ jobs:
3333
name: Discover direct backport targets
3434
runs-on: ubuntu-latest
3535
outputs:
36-
pr_number: ${{ steps.discover.outputs.pr_number }}
37-
targets: ${{ steps.discover.outputs.targets }}
36+
entries: ${{ steps.discover.outputs.entries }}
3837
has_targets: ${{ steps.discover.outputs.has_targets }}
3938
steps:
40-
- name: Resolve merged PR and green targets
39+
- name: Resolve merged PRs and green targets
4140
id: discover
4241
uses: actions/github-script@v8
4342
with:
4443
script: |
4544
const sha = context.sha;
4645
const { owner, repo } = context.repo;
4746
47+
// A push can carry several squash merges when auto-merges land on
48+
// main close together. Every commit in the push must be examined —
49+
// looking only at the head commit silently drops the release/*
50+
// labels of the other PRs in the same push (see #6119: #5802 and
51+
// #6111 were never backported this way).
52+
async function listPushCommits() {
53+
const before = context.payload?.before;
54+
const zeroSha = "0000000000000000000000000000000000000000";
55+
if (before && before !== zeroSha) {
56+
try {
57+
// compareCommits returns the commits oldest-first, which is
58+
// also the order the cherry-picks must apply in.
59+
const { data: comparison } = await github.rest.repos.compareCommits({
60+
owner,
61+
repo,
62+
base: before,
63+
head: sha,
64+
});
65+
if (comparison.commits.length > 0) {
66+
return comparison.commits.map((c) => ({
67+
sha: c.sha,
68+
message: c.commit.message,
69+
}));
70+
}
71+
core.warning(`compare ${before}...${sha} returned no commits; falling back to the head commit.`);
72+
} catch (e) {
73+
core.warning(`compare ${before}...${sha} failed: ${e.message}; falling back to the head commit.`);
74+
}
75+
}
76+
return [{ sha, message: context.payload?.head_commit?.message ?? "" }];
77+
}
78+
4879
// Strategy 1 (preferred): parse the squash-merge commit message.
4980
// ASF .asf.yaml forces squash merges with PR_TITLE_AND_DESC, so the
5081
// first line ends with "(#NNNN)". This is deterministic and avoids
5182
// the commit↔PR association index, which can lag for tens of seconds
5283
// after a merge.
53-
async function resolvePrFromMessage() {
54-
const message = context.payload?.head_commit?.message ?? "";
55-
const firstLine = message.split("\n", 1)[0];
84+
async function resolvePrFromMessage(commit) {
85+
const firstLine = commit.message.split("\n", 1)[0];
5686
const match = firstLine.match(/\(#(\d+)\)\s*$/);
5787
if (!match) {
58-
core.info('Commit message does not end with "(#N)"; falling back to API.');
88+
core.info(`Message of ${commit.sha} does not end with "(#N)"; falling back to API.`);
5989
return null;
6090
}
6191
const prNumber = Number(match[1]);
@@ -66,10 +96,10 @@ jobs:
6696
pull_number: prNumber,
6797
});
6898
if (!pr.merged) {
69-
core.warning(`PR #${prNumber} extracted from commit message is not merged; falling back to API.`);
99+
core.warning(`PR #${prNumber} extracted from ${commit.sha} is not merged; falling back to API.`);
70100
return null;
71101
}
72-
core.info(`Resolved PR #${prNumber} from commit message.`);
102+
core.info(`Resolved PR #${prNumber} from the message of ${commit.sha}.`);
73103
return pr;
74104
} catch (e) {
75105
core.warning(`Failed to fetch PR #${prNumber}: ${e.message}. Falling back to API.`);
@@ -79,7 +109,7 @@ jobs:
79109
80110
// Strategy 2 (fallback): GET /commits/{sha}/pulls with exponential
81111
// backoff. 5 attempts at 0/2/4/8/16s — total worst case ~30s.
82-
async function resolvePrFromApi() {
112+
async function resolvePrFromApi(commit) {
83113
const backoffsMs = [0, 2000, 4000, 8000, 16000];
84114
for (let i = 0; i < backoffsMs.length; i++) {
85115
if (backoffsMs[i] > 0) {
@@ -91,56 +121,35 @@ jobs:
91121
{
92122
owner,
93123
repo,
94-
commit_sha: sha,
124+
commit_sha: commit.sha,
95125
}
96126
);
97-
const pr = response.data.find((p) => p.merge_commit_sha === sha) ?? response.data[0];
127+
const pr = response.data.find((p) => p.merge_commit_sha === commit.sha) ?? response.data[0];
98128
if (pr) {
99-
core.info(`Resolved PR #${pr.number} from commits/${sha}/pulls on attempt ${i + 1}.`);
129+
core.info(`Resolved PR #${pr.number} from commits/${commit.sha}/pulls on attempt ${i + 1}.`);
100130
return pr;
101131
}
102132
}
103133
return null;
104134
}
105135
106-
const pullRequest = (await resolvePrFromMessage()) ?? (await resolvePrFromApi());
107-
if (!pullRequest) {
108-
core.info(`No merged pull request is associated with ${sha}.`);
109-
core.setOutput("pr_number", "");
110-
core.setOutput("targets", "[]");
111-
core.setOutput("has_targets", "false");
112-
return;
113-
}
114-
115-
const requestedTargets = [...new Set(
116-
pullRequest.labels
117-
.map((label) => label.name)
118-
.filter((name) => /^release\/.+$/.test(name))
119-
)].sort();
120-
121-
if (requestedTargets.length === 0) {
122-
core.info(`PR #${pullRequest.number} does not request any backports.`);
123-
core.setOutput("pr_number", String(pullRequest.number));
124-
core.setOutput("targets", "[]");
125-
core.setOutput("has_targets", "false");
126-
return;
127-
}
136+
async function greenTargetsFor(pullRequest, requestedTargets) {
137+
const buildRuns = await github.paginate(
138+
github.rest.actions.listWorkflowRuns,
139+
{
140+
owner,
141+
repo,
142+
workflow_id: "required-checks.yml",
143+
head_sha: pullRequest.head.sha,
144+
per_page: 100,
145+
}
146+
);
128147
129-
const buildRuns = await github.paginate(
130-
github.rest.actions.listWorkflowRuns,
131-
{
132-
owner,
133-
repo,
134-
workflow_id: "required-checks.yml",
135-
head_sha: pullRequest.head.sha,
136-
per_page: 100,
148+
if (buildRuns.length === 0) {
149+
core.warning(`No Build workflow runs found for ${pullRequest.head.sha}.`);
150+
return [];
137151
}
138-
);
139152
140-
let greenTargets = [];
141-
if (buildRuns.length === 0) {
142-
core.warning(`No Build workflow runs found for ${pullRequest.head.sha}.`);
143-
} else {
144153
const allJobs = [];
145154
for (const run of buildRuns) {
146155
const jobs = await github.paginate(
@@ -155,7 +164,7 @@ jobs:
155164
allJobs.push(...jobs);
156165
}
157166
158-
greenTargets = requestedTargets.filter((target) => {
167+
return requestedTargets.filter((target) => {
159168
const prefix = `backport (${target}) / `;
160169
const targetJobs = allJobs.filter((job) => job.name.startsWith(prefix));
161170
// Treat skipped as green: precheck legitimately skips stacks
@@ -169,23 +178,67 @@ jobs:
169178
});
170179
}
171180
172-
const skippedTargets = requestedTargets.filter((target) => !greenTargets.includes(target));
173-
if (skippedTargets.length > 0) {
174-
core.warning(`Skipping targets without a successful Backport run: ${skippedTargets.join(", ")}`);
181+
const commits = await listPushCommits();
182+
core.info(`Push contains ${commits.length} commit(s).`);
183+
184+
// One matrix entry per (PR, target) pair, in commit order.
185+
const entries = [];
186+
const seenPrNumbers = new Set();
187+
for (const commit of commits) {
188+
const pullRequest =
189+
(await resolvePrFromMessage(commit)) ?? (await resolvePrFromApi(commit));
190+
if (!pullRequest) {
191+
core.info(`No merged pull request is associated with ${commit.sha}.`);
192+
continue;
193+
}
194+
if (seenPrNumbers.has(pullRequest.number)) {
195+
continue;
196+
}
197+
seenPrNumbers.add(pullRequest.number);
198+
199+
const requestedTargets = [...new Set(
200+
pullRequest.labels
201+
.map((label) => label.name)
202+
.filter((name) => /^release\/.+$/.test(name))
203+
)].sort();
204+
205+
if (requestedTargets.length === 0) {
206+
core.info(`PR #${pullRequest.number} does not request any backports.`);
207+
continue;
208+
}
209+
210+
const greenTargets = await greenTargetsFor(pullRequest, requestedTargets);
211+
const skippedTargets = requestedTargets.filter((target) => !greenTargets.includes(target));
212+
if (skippedTargets.length > 0) {
213+
core.warning(`PR #${pullRequest.number}: skipping targets without a successful Backport run: ${skippedTargets.join(", ")}`);
214+
}
215+
216+
for (const target of greenTargets) {
217+
entries.push({
218+
pr_number: pullRequest.number,
219+
merge_sha: commit.sha,
220+
target,
221+
});
222+
}
175223
}
176224
177-
core.setOutput("pr_number", String(pullRequest.number));
178-
core.setOutput("targets", JSON.stringify(greenTargets));
179-
core.setOutput("has_targets", greenTargets.length > 0 ? "true" : "false");
225+
core.info(`Backport entries: ${JSON.stringify(entries)}`);
226+
core.setOutput("entries", JSON.stringify(entries));
227+
core.setOutput("has_targets", entries.length > 0 ? "true" : "false");
180228
181229
push-backports:
182230
needs: discover
183231
if: ${{ needs.discover.outputs.has_targets == 'true' }}
184232
runs-on: ubuntu-latest
233+
name: "backport #${{ matrix.pr_number }} to ${{ matrix.target }}"
185234
strategy:
186235
fail-fast: false
236+
# One leg per (PR, target) pair. max-parallel: 1 applies cherry-picks
237+
# in commit order and keeps two legs from racing pushes to the same
238+
# release branch.
239+
max-parallel: 1
187240
matrix:
188-
target: ${{ fromJson(needs.discover.outputs.targets) }}
241+
include: ${{ fromJson(needs.discover.outputs.entries) }}
189242
steps:
190243
- name: Checkout main
191244
uses: actions/checkout@v5
@@ -199,7 +252,7 @@ jobs:
199252
- name: Cherry-pick merge commit onto target branch
200253
id: cherry_pick
201254
env:
202-
MERGE_SHA: ${{ github.sha }}
255+
MERGE_SHA: ${{ matrix.merge_sha }}
203256
TARGET_BRANCH: ${{ matrix.target }}
204257
run: |
205258
set -euo pipefail
@@ -403,10 +456,10 @@ jobs:
403456
if: success()
404457
uses: actions/github-script@v8
405458
env:
406-
MERGE_SHA: ${{ github.sha }}
459+
MERGE_SHA: ${{ matrix.merge_sha }}
407460
TARGET_BRANCH: ${{ matrix.target }}
408461
NEW_SHA: ${{ steps.cherry_pick.outputs.new_sha }}
409-
PR_NUMBER: ${{ needs.discover.outputs.pr_number }}
462+
PR_NUMBER: ${{ matrix.pr_number }}
410463
with:
411464
script: |
412465
const { MERGE_SHA, TARGET_BRANCH, NEW_SHA, PR_NUMBER } = process.env;
@@ -506,9 +559,9 @@ jobs:
506559
if: failure()
507560
uses: actions/github-script@v8
508561
env:
509-
MERGE_SHA: ${{ github.sha }}
562+
MERGE_SHA: ${{ matrix.merge_sha }}
510563
TARGET_BRANCH: ${{ matrix.target }}
511-
PR_NUMBER: ${{ needs.discover.outputs.pr_number }}
564+
PR_NUMBER: ${{ matrix.pr_number }}
512565
with:
513566
script: |
514567
const { MERGE_SHA, TARGET_BRANCH, PR_NUMBER } = process.env;
@@ -558,13 +611,15 @@ jobs:
558611
}),
559612
);
560613
if (jobs) {
561-
const me = jobs.find((j) => j.name.includes(`(${TARGET_BRANCH})`));
614+
const me = jobs.find(
615+
(j) => j.name.includes(`#${PR_NUMBER} `) && j.name.includes(TARGET_BRANCH),
616+
);
562617
if (me?.html_url) {
563618
jobUrl = me.html_url;
564619
core.info(`Resolved job URL for matrix leg: ${jobUrl}`);
565620
} else {
566621
core.info(
567-
`No job matched name including "(${TARGET_BRANCH})"; ` +
622+
`No job matched name including "#${PR_NUMBER}" and "${TARGET_BRANCH}"; ` +
568623
"falling back to run URL.",
569624
);
570625
}

0 commit comments

Comments
 (0)