Skip to content

feat(auto-approve-bot-prs): keep prs with a skip label for a person #331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vcauesantos wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(auto-approve-bot-prs): keep prs with a skip label for a person #331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
03f52a5
feat(auto-approve-bot-prs): keep prs with a skip label for a person
vcauesantos Oct 23, 2026
8c7c71d
fix(auto-approve-bot-prs): match skip labels without case and inside …
vcauesantos Oct 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/actions/auto-approve-bot-prs/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 <br>and approve. Must NOT match the <br>PR author. Also used to merge <br>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 <br>when auto-merge is true. Defaults to <br>github-token. It may match the PR <br>author, but needs a merge path <br>on the base branch. |
| skip-labels | string | false | | Comma-separated labels that keep a PR <br>for a person: a PR carrying <br>one is never approved. Read after <br>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 <br>CI checks |
| wait-min-attempts | string | false | `"12"` | Minimum polls before ci_green=true is allowed. <br>Prevents early approval while slow external <br>checks (e.g. Netlify) have not yet registered. |
Expand Down
5 changes: 5 additions & 0 deletions .github/actions/auto-approve-bot-prs/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
20 changes: 18 additions & 2 deletions .github/actions/auto-approve-bot-prs/src/check-pr-after-ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand All @@ -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
Expand Down
63 changes: 63 additions & 0 deletions .github/actions/auto-approve-bot-prs/test/check-pr-after-ci.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]

@loft-bot loft-bot Oct 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking — Could we make this test prove the label is checked on the first read, while mergeability is still pending? As written it also passes if the label check only runs once mergeability resolves. With that change, a PR whose mergeability never resolves gets approved even though it carries the skip label. Pinning the call count to one catches it:

Suggested change
[ "$(kv proceed)" = "proceed=false" ]
[ "$(kv proceed)" = "proceed=false" ]
[ "$(grep -c 'pulls/42' "$GH_MOCK_CALLS")" -eq 1 ]
Mutation run (bats 1.11, this branch)

I changed the guard in check-pr-after-ci.sh to if [ -n "${SKIP_LABELS:-}" ] && [ "$mergeable" != "null" ]; then:

  • this test: still ok
  • the same test plus the call-count line above: not ok
  • GH_MOCK_MERGEABLE=null with the label and SKIP_LABELS set, expecting proceed=false: not ok (the script emits proceed=true through the "did not report mergeability" path)

On the unmodified branch, both added cases pass. The last one is also worth keeping as its own test, because it covers the path where a missed label actually leads to an approval.

@vcauesantos vcauesantos Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8c7c71d. Added the call-count check to this test, plus a test where mergeability never resolves.

# 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" ]
}
6 changes: 4 additions & 2 deletions .github/actions/auto-approve-bot-prs/test/gh_mock.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/auto-approve-bot-prs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
Expand Down Expand Up @@ -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
Expand Down
17 changes: 9 additions & 8 deletions docs/workflows/auto-approve-bot-prs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ secret.

<!-- AUTO-DOC-INPUT:START - Do not remove or modify this section -->

| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION |
|--------------------|---------|----------|------------------------------------------------|------------------------------------------------------------------------------------------------------------------------|
| auto-merge | boolean | false | `false` | Merge the PR after approving it, <br>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 <br>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, <br>directly where possible. |
| merge-method | string | false | `"squash"` | Merge method (squash, merge, rebase) |
| skip-labels | string | false | | Comma-separated labels that keep a PR <br>for a person, e.g. backport-ai-resolved. A <br>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 <br>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. |

<!-- AUTO-DOC-INPUT:END -->

Expand Down
Loading