Skip to content

Commit 353b8a1

Browse files
committed
ci(backport): skip a target whose backport already merged
A second label event on a PR whose first backport already merged (for example backport-previous added shortly after backport merged) re-enters the backport job for the target that already has the change. The action's branch name for a backport is deterministic, so that leg's cherry-pick is empty rather than conflicting, and git still exits non-zero for an empty pick. Under this repository's draft_commit_conflicts setting the action reads that exit code as a conflict and tries to commit an empty working tree, which also fails and surfaces as a false backport-failed comment on the original PR for a branch the change has been on for a while. Add a guard step that looks up whether a merged pull request already exists on this target's deterministic backport branch, and skip the checkout and the backport action for that leg when one does. The guard changes nothing about the empty-cherry-pick behavior itself, which lives in the action and needs a fix there; it only keeps a known-stale re-run from reaching the action and misreporting. Extend the existing backport.yaml contract tests with the new guard's invariants: the merged (not merely closed) check on the deterministic branch name, both downstream steps gated on the guard's output, and step ordering. Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent 605030b commit 353b8a1

2 files changed

Lines changed: 133 additions & 4 deletions

File tree

‎.github/workflows/backport.yaml‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,47 @@ jobs:
133133
echo "Target branch: ${{ needs.prepare.outputs.previous_branch }}"
134134
fi
135135
136-
# 2. Checkout (required by backport‑action)
137-
- name: Checkout repository
136+
# 2. Skip if this target already has a merged backport.
137+
#
138+
# The action's backport branch name is deterministic,
139+
# backport-<pull_number>-to-<target_branch>. A merged PR with that head
140+
# means an earlier run already delivered this change to the target — a
141+
# second label event (e.g. backport-previous added after backport
142+
# already merged) would re-enter this job and cherry-pick commits the
143+
# target branch already has. Git reports that as an empty pick (exit 1)
144+
# rather than a conflict, which draft_commit_conflicts below misreads
145+
# as a real conflict and reports as a failed backport on the original
146+
# PR. Skipping here avoids the false report; it does not touch the
147+
# empty-pick behavior itself, which lives in the action.
148+
- name: Check for existing merged backport
149+
id: guard
138150
if: steps.target.outcome == 'success'
151+
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7
152+
with:
153+
script: |
154+
const targetBranch = '${{ steps.target.outputs.branch }}';
155+
const head = `backport-${context.payload.pull_request.number}-to-${targetBranch}`;
156+
const { data: prs } = await github.rest.pulls.list({
157+
owner: context.repo.owner,
158+
repo: context.repo.repo,
159+
state: 'closed',
160+
head: `${context.repo.owner}:${head}`,
161+
});
162+
const alreadyMerged = prs.some(p => p.merged_at !== null);
163+
core.setOutput('already_merged', alreadyMerged ? 'true' : 'false');
164+
if (alreadyMerged) {
165+
console.log(`Skipping backport to ${targetBranch}: ${head} already merged.`);
166+
}
167+
168+
# 3. Checkout (required by backport‑action)
169+
- name: Checkout repository
170+
if: steps.target.outcome == 'success' && steps.guard.outputs.already_merged != 'true'
139171
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
140172

141-
# 3. Create the back‑port pull request
173+
# 4. Create the back‑port pull request
142174
- name: Create back‑port PR
143175
id: backport
144-
if: steps.target.outcome == 'success'
176+
if: steps.target.outcome == 'success' && steps.guard.outputs.already_merged != 'true'
145177
uses: korthout/backport-action@0193454f0c5947491d348f33a275c119f30eb736 # v3.2.1
146178
with:
147179
github_token: ${{ secrets.GITHUB_TOKEN }}

‎hack/release-freeze-contract.bats‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,3 +274,100 @@ step_line() {
274274
# And the head of that sort is what `backport` uses.
275275
printf '%s\n' "$block" | script_lines | grep -qF 'lines[0].name'
276276
}
277+
278+
# ── skip a target whose backport already merged ─────────────────────────────
279+
# A second label event on a PR whose first backport already merged (e.g.
280+
# backport-previous added after backport merged) re-enters this job for the
281+
# already-delivered target. The cherry-pick is then empty, which the action's
282+
# draft_commit_conflicts handling misreads as a real conflict and reports as a
283+
# failed backport on the original PR — this guard skips the re-run instead.
284+
285+
@test "the guard checks merged, not just closed, PRs on the deterministic branch" {
286+
block="$(job_block backport "$BACKPORT")"
287+
[ -n "$block" ]
288+
289+
# The branch name the action itself would have used. Checking anything else
290+
# answers a different question than "did THIS backport already land".
291+
printf '%s\n' "$block" | script_lines | grep -qF 'backport-${context.payload.pull_request.number}-to-${targetBranch}'
292+
293+
# Closed alone is not enough: a closed-without-merge PR is a stale or
294+
# abandoned attempt, not proof the change reached the target. Loosening this
295+
# to any closed PR would skip a real retry after a conflict was closed
296+
# unmerged.
297+
printf '%s\n' "$block" | script_lines | grep -qF "state: 'closed'"
298+
printf '%s\n' "$block" | script_lines | grep -qF 'p.merged_at !== null'
299+
}
300+
301+
@test "both the checkout and the backport-action step are gated on the guard" {
302+
block="$(job_block backport "$BACKPORT")"
303+
[ -n "$block" ]
304+
305+
# Losing the gate on either step still runs the action against a target that
306+
# already has the change, reproducing the false comment the guard exists to
307+
# prevent.
308+
count="$(printf '%s\n' "$block" | code_lines | grep -cF "steps.guard.outputs.already_merged != 'true'" || true)"
309+
[ "${count:-0}" -eq 2 ]
310+
}
311+
312+
@test "the guard runs before the checkout and the backport-action step" {
313+
guard="$(step_line 'Check for existing merged backport' "$BACKPORT")"
314+
checkout="$(step_line 'Checkout repository' "$BACKPORT")"
315+
create="$(step_line 'Create back‑port PR' "$BACKPORT")"
316+
[ -n "$guard" ] && [ -n "$checkout" ] && [ -n "$create" ]
317+
318+
[ "$guard" -lt "$checkout" ]
319+
[ "$guard" -lt "$create" ]
320+
}
321+
322+
# Mirrors release-changelog-behaviour.bats's convention for a github-script
323+
# snippet: extract the decision expression verbatim in shape and execute it
324+
# under node, because a grep for "merged_at" cannot tell an `.some()` from an
325+
# `.every()` or a flipped comparison. Two cases are the ones that matter and
326+
# are NOT interchangeable: the first-ever run (branch never existed, so the
327+
# API returns an empty list) and a real retry (someone closed a conflicting
328+
# backport PR without merging it). Both must NOT skip, or either the very
329+
# first backport attempt or a legitimate re-creation after a manual close
330+
# never runs.
331+
@test "the guard's merged decision is correct on empty, closed-unmerged and closed-merged lists" {
332+
command -v node >/dev/null || { echo "node unavailable; skipping"; return 0; }
333+
tmp="$(mktemp -d)"
334+
# shellcheck disable=SC2064
335+
trap "rm -rf '$tmp'" EXIT
336+
337+
cat > "$tmp/guard.js" <<'JS'
338+
function alreadyMerged(prs) {
339+
return prs.some(p => p.merged_at !== null);
340+
}
341+
const cases = {
342+
'first-run-empty': [],
343+
'closed-unmerged': [{ merged_at: null }],
344+
'closed-merged': [{ merged_at: '2026-01-01T00:00:00Z' }],
345+
};
346+
const lines = Object.entries(cases).map(([name, prs]) => name + '=' + alreadyMerged(prs));
347+
require('fs').writeFileSync(1, lines.join('\n') + '\n');
348+
JS
349+
350+
node "$tmp/guard.js" > "$tmp/out.txt" 2>"$tmp/err.txt" || {
351+
echo "node failed:" >&2; cat "$tmp/err.txt" >&2; exit 1; }
352+
353+
want="first-run-empty=false
354+
closed-unmerged=false
355+
closed-merged=true"
356+
got="$(cat "$tmp/out.txt")"
357+
[ "$got" = "$want" ] || {
358+
echo "guard decision table is wrong." >&2
359+
echo "want:" >&2; printf '%s\n' "$want" | sed 's/^/ /' >&2
360+
echo "got:" >&2; printf '%s\n' "$got" | sed 's/^/ /' >&2
361+
exit 1
362+
}
363+
364+
# The mirror must match the real guard's exact decision expression, not just
365+
# the presence of "merged_at" somewhere in the step. Wrapped like the other
366+
# tests in this file's final assertion: a bare failing command here, under a
367+
# trap-installed EXIT handler, has been observed to abort the whole bats run
368+
# instead of just this test on macOS.
369+
block="$(job_block backport "$BACKPORT")"
370+
[ -n "$block" ]
371+
printf '%s\n' "$block" | script_lines | grep -qF 'prs.some(p => p.merged_at !== null)' || {
372+
echo "backport.yaml's guard no longer uses prs.some(p => p.merged_at !== null)." >&2; exit 1; }
373+
}

0 commit comments

Comments
 (0)