diff --git a/.github/actions/auto-approve-bot-prs/README.md b/.github/actions/auto-approve-bot-prs/README.md index 4882fe07..28da68af 100644 --- a/.github/actions/auto-approve-bot-prs/README.md +++ b/.github/actions/auto-approve-bot-prs/README.md @@ -108,6 +108,18 @@ Refusing to approve is reported as an `::error::`. The job keeps its refusal is a real outcome and something downstream is often blocking on the merge that will now never happen. Reported as a notice, it read as a clean green job. +## Keeping a PR for a person + +`skip-labels` lists labels that stop approval: a PR carrying any of them is +left for a person, with a notice. The labels are read in the post-CI check, +right before approving, so a label added before a push is always seen by the +run for that push. Runs for older heads already stop at the head check. If +`skip-labels` is set and the labels cannot be read, the PR is not approved. + +The backport workflow's AI pass labels its PRs `backport-ai-resolved`, so a +caller that wants AI-written backports reviewed passes +`skip-labels: backport-ai-resolved`. + ## Merging With `auto-merge: true` the action tries a **plain merge first**, and uses @@ -196,6 +208,7 @@ PR is benign, and a PR closed unmerged is a human decision. | github-token | string | true | | PAT used to read PR state
and approve. Must NOT match the
PR author. Also used to merge
when merge-token is omitted. | | merge-method | string | false | `"squash"` | Merge method (squash|merge|rebase) | | merge-token | string | false | | Optional token used only to merge
when auto-merge is true. Defaults to
github-token. It may match the PR
author, but needs a merge path
on the base branch. | +| skip-labels | string | false | | Comma-separated labels that keep a PR
for a person: a PR carrying
one is never approved. Read after
CI, right before approving. | | trusted-authors | string | false | `"renovate[bot],loft-bot,github-actions[bot]"` | Comma-separated list of trusted bot logins | | wait-max-attempts | string | false | `"90"` | Max polling attempts waiting for other
CI checks | | wait-min-attempts | string | false | `"12"` | Minimum polls before ci_green=true is allowed.
Prevents early approval while slow external
checks (e.g. Netlify) have not yet registered. | diff --git a/.github/actions/auto-approve-bot-prs/action.yml b/.github/actions/auto-approve-bot-prs/action.yml index 22b04c6f..4b7dd6d5 100644 --- a/.github/actions/auto-approve-bot-prs/action.yml +++ b/.github/actions/auto-approve-bot-prs/action.yml @@ -41,6 +41,10 @@ inputs: description: 'Seconds between polling attempts' required: false default: '10' + skip-labels: + description: 'Comma-separated labels that keep a PR for a person: a PR carrying one is never approved. Read after CI, right before approving.' + required: false + default: '' runs: using: composite @@ -93,6 +97,7 @@ runs: GH_TOKEN: ${{ inputs.github-token }} PR_NUMBER: ${{ github.event.pull_request.number }} EXPECTED_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + SKIP_LABELS: ${{ inputs.skip-labels }} run: ${{ github.action_path }}/src/check-pr-after-ci.sh - name: Approve PR diff --git a/.github/actions/auto-approve-bot-prs/src/check-pr-after-ci.sh b/.github/actions/auto-approve-bot-prs/src/check-pr-after-ci.sh index ff42fa2f..801712ec 100755 --- a/.github/actions/auto-approve-bot-prs/src/check-pr-after-ci.sh +++ b/.github/actions/auto-approve-bot-prs/src/check-pr-after-ci.sh @@ -6,7 +6,12 @@ # computed after a bounded wait does not, because the merge API enforces # conflicts anyway. # +# A label in SKIP_LABELS keeps the PR for a person. It is read here, after CI, +# because a label added before a push is always visible to the run for that +# push, and runs for older heads stop at the head check. +# # Required env: GH_TOKEN, GITHUB_REPOSITORY, PR_NUMBER, EXPECTED_HEAD_SHA +# Optional env: SKIP_LABELS (comma-separated label names) # Writes: proceed=true|false to $GITHUB_OUTPUT (and stdout). # Always exits 0 after validating required env. set -euo pipefail @@ -31,16 +36,22 @@ read_once="" for attempt in $(seq 1 "$attempts"); do # The preflight read the PR with this token, so a failure here is treated as # transient and retried rather than stranding a run whose CI already passed. + # The third field is the first label matching SKIP_LABELS, compared without + # case as GitHub does, and matched inside the filter so no label name passes + # through @tsv as JSON. + # shellcheck disable=SC2016 # $ENV is read by jq, not the shell if ! pr_state=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" \ --jq '[ (.mergeable | if . == null then "null" else tostring end), - (.head.sha // "") + (.head.sha // ""), + ([$ENV.SKIP_LABELS // "" | split(",")[] | gsub("^\\s+|\\s+$"; "") | ascii_downcase | select(length > 0)] as $skip + | first(.labels[]?.name | select(ascii_downcase as $l | $skip | index($l))) // "") ] | @tsv' 2>/dev/null); then [ "$attempt" -lt "$attempts" ] && sleep "$sleep_seconds" continue fi - IFS=$'\t' read -r mergeable current_head <<< "$pr_state" + IFS=$'\t' read -r mergeable current_head skip <<< "$pr_state" read_once=1 if [ -z "$current_head" ]; then echo "::error::could not resolve the current head SHA for PR #${PR_NUMBER} after CI; not approving because the tested commit cannot be verified" @@ -52,6 +63,11 @@ for attempt in $(seq 1 "$attempts"); do emit proceed false exit 0 fi + if [ -n "$skip" ]; then + echo "::notice::PR #${PR_NUMBER} is labeled '$(safe "$skip")'; not approving, it is left for a person" + emit proceed false + exit 0 + fi [ "$mergeable" != "null" ] && break [ "$attempt" -lt "$attempts" ] && sleep "$sleep_seconds" done diff --git a/.github/actions/auto-approve-bot-prs/test/check-pr-after-ci.bats b/.github/actions/auto-approve-bot-prs/test/check-pr-after-ci.bats index a62dd5e2..2857d7d7 100644 --- a/.github/actions/auto-approve-bot-prs/test/check-pr-after-ci.bats +++ b/.github/actions/auto-approve-bot-prs/test/check-pr-after-ci.bats @@ -91,3 +91,66 @@ kv() { grep "^$1=" "$GITHUB_OUTPUT" | tail -n1; } GITHUB_REPOSITORY=o/r PR_NUMBER=1 "$SCRIPT" [ "$status" -ne 0 ] } + +@test "a skip label keeps the PR for a person" { + GH_MOCK_MERGEABLE=true GH_MOCK_LABELS='["backport-ai-resolved"]' SKIP_LABELS="backport-ai-resolved" run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(kv proceed)" = "proceed=false" ] + [[ "$output" == *"::notice::PR #42 is labeled 'backport-ai-resolved'"* ]] +} + +@test "any label from a list with spaces matches" { + GH_MOCK_MERGEABLE=true GH_MOCK_LABELS='["Large","hold"]' SKIP_LABELS=" backport-ai-resolved , hold " run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(kv proceed)" = "proceed=false" ] + [[ "$output" == *"labeled 'hold'"* ]] +} + +@test "other labels do not block approval" { + GH_MOCK_MERGEABLE=true GH_MOCK_LABELS='["Large"]' SKIP_LABELS="backport-ai-resolved" run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(kv proceed)" = "proceed=true" ] +} + +@test "without skip-labels a labeled PR proceeds as before" { + GH_MOCK_MERGEABLE=true GH_MOCK_LABELS='["backport-ai-resolved"]' run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(kv proceed)" = "proceed=true" ] +} + +@test "a moved head is reported before a skip label" { + GH_MOCK_MERGEABLE=true GH_MOCK_HEAD_SHA="new-head-sha" GH_MOCK_LABELS='["backport-ai-resolved"]' SKIP_LABELS="backport-ai-resolved" run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(kv proceed)" = "proceed=false" ] + [[ "$output" == *"head changed"* ]] +} + +@test "a skip label read while mergeability is pending still blocks" { + printf 'null\ntrue\n' > "$BATS_TEST_TMPDIR/pulls" + GH_MOCK_PULL_SEQ="$BATS_TEST_TMPDIR/pulls" GH_MOCK_LABELS='["backport-ai-resolved"]' SKIP_LABELS="backport-ai-resolved" run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(kv proceed)" = "proceed=false" ] + # Decided on the first read, before mergeability resolves. + [ "$(grep -c 'pulls/42' "$GH_MOCK_CALLS")" -eq 1 ] +} + +@test "a skip label blocks even when mergeability never resolves" { + GH_MOCK_MERGEABLE=null GH_MOCK_LABELS='["backport-ai-resolved"]' SKIP_LABELS="backport-ai-resolved" run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(kv proceed)" = "proceed=false" ] + [[ "$output" != *"did not report mergeability"* ]] +} + +@test "skip labels match without case, as GitHub labels do" { + GH_MOCK_MERGEABLE=true GH_MOCK_LABELS='["backport-ai-resolved"]' SKIP_LABELS="Backport-AI-Resolved" run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(kv proceed)" = "proceed=false" ] +} + +@test "labels with quotes or backslashes do not break the read" { + GH_MOCK_MERGEABLE=true GH_MOCK_LABELS='["say \"hi\"", "a\\b"]' SKIP_LABELS="backport-ai-resolved" run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(kv proceed)" = "proceed=true" ] + GH_MOCK_MERGEABLE=true GH_MOCK_LABELS='["say \"hi\"", "backport-ai-resolved"]' SKIP_LABELS="backport-ai-resolved" run "$SCRIPT" + [ "$(kv proceed)" = "proceed=false" ] +} diff --git a/.github/actions/auto-approve-bot-prs/test/gh_mock.bash b/.github/actions/auto-approve-bot-prs/test/gh_mock.bash index 4c37f055..89ae36ea 100644 --- a/.github/actions/auto-approve-bot-prs/test/gh_mock.bash +++ b/.github/actions/auto-approve-bot-prs/test/gh_mock.bash @@ -15,6 +15,7 @@ set -o pipefail # # GH_MOCK_MERGEABLE → response for `gh api ...pulls/N` # GH_MOCK_HEAD_SHA → PR head SHA returned by `gh api ...pulls/N` +# GH_MOCK_LABELS → JSON array of label names on `gh api ...pulls/N` (default []) # GH_MOCK_PULL_FAIL → if 'always', /pulls/N exits non-zero # GH_MOCK_PULL_SEQ → path to file with one mergeable value per line # (true|false|null); call N reads line N, falling @@ -103,8 +104,9 @@ emit_api_response() { fi m="${seq:-${GH_MOCK_MERGEABLE:-null}}" case "$m" in true|false) ;; *) m=null ;; esac - printf '{"mergeable":%s,"head":{"sha":"%s"}}\n' \ - "$m" "${GH_MOCK_HEAD_SHA:-tested-head-sha}" + printf '{"mergeable":%s,"head":{"sha":"%s"},"labels":%s}\n' \ + "$m" "${GH_MOCK_HEAD_SHA:-tested-head-sha}" \ + "$(jq -c 'map({name: .})' <<< "${GH_MOCK_LABELS:-[]}")" ;; *"/check-runs"*) if [ "${GH_MOCK_CHECK_RUNS_FAIL:-}" = "always" ]; then diff --git a/.github/workflows/auto-approve-bot-prs.yaml b/.github/workflows/auto-approve-bot-prs.yaml index ef1e7990..aacfc43f 100644 --- a/.github/workflows/auto-approve-bot-prs.yaml +++ b/.github/workflows/auto-approve-bot-prs.yaml @@ -27,6 +27,10 @@ on: description: 'Seconds between polling attempts.' type: string default: '10' + skip-labels: + description: 'Comma-separated labels that keep a PR for a person, e.g. backport-ai-resolved. A PR carrying one is never approved.' + type: string + default: '' secrets: gh-access-token: description: 'GitHub PAT for approving PRs (must be different identity from PR author). Also used for merging when merge-token is omitted.' @@ -73,6 +77,7 @@ jobs: auto-merge: ${{ inputs.auto-merge }} wait-max-attempts: ${{ inputs.wait-max-attempts }} wait-min-attempts: ${{ inputs.wait-min-attempts }} + skip-labels: ${{ inputs.skip-labels }} wait-sleep-seconds: ${{ inputs.wait-sleep-seconds }} github-token: ${{ secrets.gh-access-token }} # zizmor: ignore[secrets-outside-env] -- PAT passed via workflow_call, not a repo secret merge-token: ${{ secrets.merge-token }} # zizmor: ignore[secrets-outside-env] -- optional token passed via workflow_call diff --git a/docs/workflows/auto-approve-bot-prs.md b/docs/workflows/auto-approve-bot-prs.md index bee09425..83abeac4 100644 --- a/docs/workflows/auto-approve-bot-prs.md +++ b/docs/workflows/auto-approve-bot-prs.md @@ -10,14 +10,15 @@ secret. -| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | -|--------------------|---------|----------|------------------------------------------------|------------------------------------------------------------------------------------------------------------------------| -| auto-merge | boolean | false | `false` | Merge the PR after approving it,
directly where possible. | -| merge-method | string | false | `"squash"` | Merge method (squash, merge, rebase) | -| trusted-authors | string | false | `"renovate[bot],loft-bot,github-actions[bot]"` | Comma-separated list of trusted bot logins | -| wait-max-attempts | string | false | `"90"` | Max polling attempts waiting for other
CI checks (raise this when a slow required check, e.g. e2e, gates the PR). | -| wait-min-attempts | string | false | `"12"` | Minimum polls before ci_green=true is allowed. | -| wait-sleep-seconds | string | false | `"10"` | Seconds between polling attempts. | +| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | +|--------------------|---------|----------|------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------| +| auto-merge | boolean | false | `false` | Merge the PR after approving it,
directly where possible. | +| merge-method | string | false | `"squash"` | Merge method (squash, merge, rebase) | +| skip-labels | string | false | | Comma-separated labels that keep a PR
for a person, e.g. backport-ai-resolved. A
PR carrying one is never approved. | +| trusted-authors | string | false | `"renovate[bot],loft-bot,github-actions[bot]"` | Comma-separated list of trusted bot logins | +| wait-max-attempts | string | false | `"90"` | Max polling attempts waiting for other
CI checks (raise this when a slow required check, e.g. e2e, gates the PR). | +| wait-min-attempts | string | false | `"12"` | Minimum polls before ci_green=true is allowed. | +| wait-sleep-seconds | string | false | `"10"` | Seconds between polling attempts. |