From 2c47477923f88a2fac769e1e13b83c6db769f156 Mon Sep 17 00:00:00 2001 From: Caue Santos Date: Mon, 24 Aug 2026 19:22:33 -0600 Subject: [PATCH 1/5] fix(auto-approve-bot-prs): merge through the api when gh refuses client-side --- .../actions/auto-approve-bot-prs/README.md | 36 +++++--- .../actions/auto-approve-bot-prs/action.yml | 5 ++ .../src/enable-auto-merge.sh | 18 +++- .../test/enable-auto-merge.bats | 87 +++++++++++++++++++ .../auto-approve-bot-prs/test/gh_mock.bash | 16 +++- .github/workflows/auto-approve-bot-prs.yaml | 5 ++ docs/workflows/auto-approve-bot-prs.md | 17 ++-- 7 files changed, 161 insertions(+), 23 deletions(-) diff --git a/.github/actions/auto-approve-bot-prs/README.md b/.github/actions/auto-approve-bot-prs/README.md index 6d4d275e..3cf7a0a5 100644 --- a/.github/actions/auto-approve-bot-prs/README.md +++ b/.github/actions/auto-approve-bot-prs/README.md @@ -80,6 +80,19 @@ rerun that was still coming. That stalled the `v0.34.7` cut With `auto-merge: true` the action tries a **plain merge first**, and uses GitHub's auto-merge queue (`--auto`) only as a fallback. +`gh pr merge` decides mergeability **client-side**: it reads `mergeStateStatus` +and refuses with "the base branch policy prohibits the merge" without calling +the merge API. That verdict describes the pull request, not the caller, so it +ignores the merge token's ruleset bypass — a token GitHub would let merge is +turned away before it can try. `merge-when-blocked: true` retries through +`PUT /pulls/{n}/merge` instead, which has no such gate. It passes `sha`, the +equivalent of `--match-head-commit`. + +The setting grants no privilege, since every rule is still enforced server-side +and a token without a bypass is refused there too. It is opt-in because for a +token that *does* carry one, it decides whether the action merges only what CI +approved or merges past whatever that bypass covers. + The action carries the pull request head SHA from the triggering event through the run. It checks that SHA once before polling CI and again after CI passes. If Renovate or another actor updates the branch while an older run is still @@ -148,17 +161,18 @@ PR is benign, and a PR closed unmerged is a human decision. -| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | -|--------------------|--------|----------|------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| auto-merge | string | false | `"false"` | Merge the PR after approving it,
directly where possible. See README "Merging". | -| ci-read-token | string | false | | Token for the read-only CI poll
only; defaults to the caller GITHUB_TOKEN,
which needs `checks: read` and `statuses: read`. Never
the approving PAT. See README "Tokens
by purpose". | -| 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. | -| 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. | -| wait-sleep-seconds | string | false | `"10"` | Seconds between polling attempts | +| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | +|--------------------|--------|----------|------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| auto-merge | string | false | `"false"` | Merge the PR after approving it,
directly where possible. See README "Merging". | +| ci-read-token | string | false | | Token for the read-only CI poll
only; defaults to the caller GITHUB_TOKEN,
which needs `checks: read` and `statuses: read`. Never
the approving PAT. See README "Tokens
by purpose". | +| 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. | +| merge-when-blocked | string | false | `"false"` | Retry a refused merge through the
merge API, so GitHub decides instead
of gh's client-side mergeability check. Needed
when the merge token merges via
a ruleset bypass. See README "Merging". | +| 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. | +| wait-sleep-seconds | string | false | `"10"` | Seconds between polling attempts | diff --git a/.github/actions/auto-approve-bot-prs/action.yml b/.github/actions/auto-approve-bot-prs/action.yml index c275d829..d99e9b28 100644 --- a/.github/actions/auto-approve-bot-prs/action.yml +++ b/.github/actions/auto-approve-bot-prs/action.yml @@ -20,6 +20,10 @@ inputs: description: 'Merge the PR after approving it, directly where possible. See README "Merging".' required: false default: 'false' + merge-when-blocked: + description: 'Retry a refused merge through the merge API, so GitHub decides instead of gh''s client-side mergeability check. Needed when the merge token merges via a ruleset bypass. See README "Merging".' + required: false + default: 'false' github-token: description: 'PAT used to read PR state and approve. Must NOT match the PR author. Also used to merge when merge-token is omitted.' required: true @@ -114,4 +118,5 @@ runs: PR_NUMBER: ${{ github.event.pull_request.number }} PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} MERGE_METHOD: ${{ inputs.merge-method }} + MERGE_WHEN_BLOCKED: ${{ inputs.merge-when-blocked }} run: ${{ github.action_path }}/src/enable-auto-merge.sh diff --git a/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh b/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh index c3a6e6f0..88b0dd10 100755 --- a/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh +++ b/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh @@ -21,6 +21,7 @@ # # Required env: GH_TOKEN, GITHUB_REPOSITORY, PR_NUMBER, PR_HEAD_SHA, # MERGE_METHOD +# Optional env: MERGE_WHEN_BLOCKED (default false), MERGE_RETRY_SLEEP_SECONDS set -euo pipefail : "${GITHUB_REPOSITORY:?GITHUB_REPOSITORY required}" @@ -86,6 +87,18 @@ case "$pr_state" in ;; esac +# `gh pr merge` judges mergeability client-side and never calls the API, so a +# merge token whose ruleset bypass would allow the merge is refused before it can +# try. The API has no such gate. `sha` is its --match-head-commit. +if [ "${MERGE_WHEN_BLOCKED:-false}" = "true" ]; then + if api_err=$(try_merge gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/merge" \ + --method PUT -f sha="$PR_HEAD_SHA" -f merge_method="$MERGE_METHOD"); then + echo "Merged PR #${PR_NUMBER} (${MERGE_METHOD}) through the merge API after the plain merge was refused" + exit 0 + fi + echo "::notice::merge API for PR #${PR_NUMBER} was refused too. Reason: $(safe "$api_err")" +fi + echo "::notice::plain merge of PR #${PR_NUMBER} was refused, falling back to auto-merge. Reason: $(safe "$direct_err")" # This SHA protects the enable-auto-merge mutation, not the eventual queued @@ -107,4 +120,7 @@ if auto_err=$(try_merge gh pr merge "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" \ exit 0 fi -echo "::error::PR #${PR_NUMBER} was approved but could NOT be merged, and could not be queued for auto-merge either (each path was retried once). Anything waiting on this merge will stall. Plain merge said: $(safe "$direct_err" 500). Auto-merge said: $(safe "$auto_err" 500). Read those two reasons first — they name the cause. If they point at policy rather than a transient API error, check branch protection and rulesets (is the token's team a bypass actor during a code freeze?), the token's merge permission, and whether the repository allows auto-merge." +api_reason="" +[ -n "${api_err+x}" ] && api_reason=" Merge API said: $(safe "$api_err" 500)." + +echo "::error::PR #${PR_NUMBER} was approved but could NOT be merged, and could not be queued for auto-merge either (each path was retried once). Anything waiting on this merge will stall. Plain merge said: $(safe "$direct_err" 500).${api_reason} Auto-merge said: $(safe "$auto_err" 500). Read those reasons first — they name the cause. If they point at policy rather than a transient API error, check branch protection and rulesets (is the token's team a bypass actor during a code freeze?), the token's merge permission, and whether the repository allows auto-merge. No merge-API line above means merge-when-blocked is off, so gh's client-side verdict was final." diff --git a/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats b/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats index 4457a407..b73f2aaf 100644 --- a/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats +++ b/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats @@ -142,6 +142,93 @@ auto_merge_count() { grep -c '^pr merge .*--auto' "$GH_MOCK_CALLS" || true; } assert_no_match 'will land once the remaining requirements pass' "$output" } +# --------------------------------------------------------------------------- +# merge-when-blocked: retry a client-side refusal through the API, so a merge +# token's ruleset bypass gets a chance to apply. + +api_merge_attempted() { grep -q '^api .*/pulls/42/merge' "$GH_MOCK_CALLS"; } +assert_no_api_merge() { assert_no_match '^api .*/pulls/42/merge' "$(cat "$GH_MOCK_CALLS")"; } +api_merge_count() { grep -c '^api .*/pulls/42/merge' "$GH_MOCK_CALLS" || true; } + +@test "merge-when-blocked off → a refused plain merge never reaches the merge API" { + GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=0 GH_MOCK_PR_STATE=OPEN run "$SCRIPT" + [ "$status" -eq 0 ] + assert_no_api_merge + auto_merge_attempted +} + +@test "merge-when-blocked on → refused plain merge is retried through the API and lands" { + MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_STATE=OPEN \ + GH_MOCK_API_MERGE_EXIT=0 run "$SCRIPT" + [ "$status" -eq 0 ] + api_merge_attempted + [[ "$output" == *"through the merge API"* ]] + assert_no_auto_merge + [[ "$output" != *"::error::"* ]] + [[ "$output" != *"::warning::"* ]] +} + +@test "merge-when-blocked on but the API refuses too → falls back to --auto as before" { + MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_STATE=OPEN \ + GH_MOCK_API_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=0 run "$SCRIPT" + [ "$status" -eq 0 ] + api_merge_attempted + auto_merge_attempted + [[ "$output" == *"::warning::PR #42 could not be merged immediately"* ]] +} + +@test "a token with no bypass is refused by the API, and the error says so" { + MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_STATE=OPEN \ + GH_MOCK_API_MERGE_EXIT=1 GH_MOCK_API_MERGE_OUT="At least 1 approving review is required" \ + GH_MOCK_PR_MERGE_AUTO_EXIT=1 run "$SCRIPT" + [ "$status" -eq 0 ] + [[ "$output" == *"::error::"* ]] + [[ "$output" == *"Merge API said:"* ]] + [[ "$output" == *"At least 1 approving review is required"* ]] +} + +@test "with merge-when-blocked off the error names the setting rather than hiding it" { + # Otherwise the annotation blames branch protection for gh declining to ask. + GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=1 GH_MOCK_PR_STATE=OPEN run "$SCRIPT" + [ "$status" -eq 0 ] + [[ "$output" == *"::error::"* ]] + [[ "$output" == *"merge-when-blocked is off"* ]] + assert_no_match 'Merge API said:' "$output" +} + +@test "the API merge is guarded by the tested head SHA" { + MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_STATE=OPEN \ + GH_MOCK_API_MERGE_EXIT=0 run "$SCRIPT" + [ "$status" -eq 0 ] + grep -q -- 'sha=tested-head-sha' "$GH_MOCK_CALLS" + grep -q -- 'merge_method=squash' "$GH_MOCK_CALLS" +} + +@test "a refused API merge is retried once, like the other two paths" { + MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_STATE=OPEN \ + GH_MOCK_API_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=0 run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(api_merge_count)" -eq 2 ] +} + +@test "an already-merged PR short-circuits before the API merge" { + # Re-runs are routine; a 405 here would read like a real failure. + MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_STATE=MERGED run "$SCRIPT" + [ "$status" -eq 0 ] + [[ "$output" == *"already merged"* ]] + assert_no_api_merge +} + +@test "regression: a CR in the merge-API error cannot forge a workflow command" { + MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_STATE=OPEN \ + GH_MOCK_API_MERGE_EXIT=1 GH_MOCK_API_MERGE_OUT=$'api boom\r::error::FORGED' \ + GH_MOCK_PR_MERGE_AUTO_EXIT=1 run "$SCRIPT" + [ "$status" -eq 0 ] + assert_no_match '\r' "$output" + assert_no_match '(?m)^::error::FORGED' "$output" + [[ "$output" == *"api boom"* ]] +} + @test "each valid merge method is passed through to gh" { for m in squash merge rebase; do : > "$GH_MOCK_CALLS" 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 f7b4df1a..2f9b8748 100644 --- a/.github/actions/auto-approve-bot-prs/test/gh_mock.bash +++ b/.github/actions/auto-approve-bot-prs/test/gh_mock.bash @@ -71,6 +71,14 @@ emit_api_response() { user) printf '{"login":"%s"}\n' "${GH_MOCK_APPROVER:-}" ;; + *"/pulls/"*"/merge") + # Must precede the /pulls/ arm, which would otherwise swallow this path. + if [ "${GH_MOCK_API_MERGE_EXIT:-0}" != "0" ]; then + emit_err "${GH_MOCK_API_MERGE_OUT:-mock: merge API refused}" + exit "${GH_MOCK_API_MERGE_EXIT}" + fi + printf '{"merged":true,"sha":"%s"}\n' "${GH_MOCK_HEAD_SHA:-tested-head-sha}" + ;; *"/pulls/"*) if [ "${GH_MOCK_PULL_FAIL:-}" = "always" ]; then emit_err "mock: pull request forced failure" @@ -129,13 +137,15 @@ apply_filter() { case "${1:-}" in api) shift - path="${1:-}"; shift || true + # Endpoint is the first non-flag arg: gh takes it either side of the flags. + path="" jq_filter="" while [ $# -gt 0 ]; do case "$1" in --jq) jq_filter="$2"; shift 2 ;; - --paginate|--method|--header|-H|-X) shift 2>/dev/null || true ;; - *) shift ;; + --method|--header|-H|-X|-f|-F) shift 2>/dev/null || true ;; + --paginate) shift ;; + *) [ -z "$path" ] && path="$1"; shift ;; esac done emit_api_response "$path" | apply_filter "$jq_filter" diff --git a/.github/workflows/auto-approve-bot-prs.yaml b/.github/workflows/auto-approve-bot-prs.yaml index 5df359e1..c4052ba6 100644 --- a/.github/workflows/auto-approve-bot-prs.yaml +++ b/.github/workflows/auto-approve-bot-prs.yaml @@ -15,6 +15,10 @@ on: description: 'Merge the PR after approving it, directly where possible.' type: boolean default: false + merge-when-blocked: + description: 'Retry a refused merge through the merge API so GitHub decides, rather than gh''s client-side check. Set this when the merge token merges via a ruleset bypass.' + type: boolean + default: false wait-max-attempts: description: 'Max polling attempts waiting for other CI checks (raise this when a slow required check, e.g. e2e, gates the PR).' type: string @@ -71,6 +75,7 @@ jobs: trusted-authors: ${{ inputs.trusted-authors }} merge-method: ${{ inputs.merge-method }} auto-merge: ${{ inputs.auto-merge }} + merge-when-blocked: ${{ inputs.merge-when-blocked }} wait-max-attempts: ${{ inputs.wait-max-attempts }} wait-min-attempts: ${{ inputs.wait-min-attempts }} wait-sleep-seconds: ${{ inputs.wait-sleep-seconds }} diff --git a/docs/workflows/auto-approve-bot-prs.md b/docs/workflows/auto-approve-bot-prs.md index bee09425..21984203 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) | +| merge-when-blocked | boolean | false | `false` | Retry a refused merge through the
merge API so GitHub decides, rather
than gh's client-side check. Set this
when the merge token merges via
a ruleset bypass. | +| 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. | From 7c083ae08938675f1becbf641106ffa47d2d1efd Mon Sep 17 00:00:00 2001 From: Caue Santos Date: Mon, 24 Aug 2026 19:46:10 -0600 Subject: [PATCH 2/5] fix(auto-approve-bot-prs): merge only when the approval was recorded --- .github/actions/auto-approve-bot-prs/action.yml | 6 +++++- .../auto-approve-bot-prs/test/token-contract.bats | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/actions/auto-approve-bot-prs/action.yml b/.github/actions/auto-approve-bot-prs/action.yml index d99e9b28..36e5a67a 100644 --- a/.github/actions/auto-approve-bot-prs/action.yml +++ b/.github/actions/auto-approve-bot-prs/action.yml @@ -100,6 +100,7 @@ runs: run: ${{ github.action_path }}/src/check-pr-after-ci.sh - name: Approve PR + id: approve if: steps.recheck.outputs.proceed == 'true' continue-on-error: true uses: hmarr/auto-approve-action@f0939ea97e9205ef24d872e76833fa908a770363 # v4.0.0 @@ -110,8 +111,11 @@ runs: For more information, see https://github.com/loft-sh/github-actions?tab=readme-ov-file#auto-approve-bot-prs. + # `outcome`, not `conclusion`: continue-on-error rewrites the latter to + # success. Approval is best-effort, but a merge token with a ruleset bypass + # would otherwise land the PR with no approval recorded. - name: Merge PR - if: steps.recheck.outputs.proceed == 'true' && inputs.auto-merge == 'true' + if: steps.recheck.outputs.proceed == 'true' && inputs.auto-merge == 'true' && steps.approve.outcome == 'success' shell: bash env: GH_TOKEN: ${{ inputs.merge-token || inputs.github-token }} diff --git a/.github/actions/auto-approve-bot-prs/test/token-contract.bats b/.github/actions/auto-approve-bot-prs/test/token-contract.bats index bec2e5c5..d843c35c 100644 --- a/.github/actions/auto-approve-bot-prs/test/token-contract.bats +++ b/.github/actions/auto-approve-bot-prs/test/token-contract.bats @@ -24,6 +24,18 @@ WORKFLOW="$BATS_TEST_DIRNAME/../../../workflows/auto-approve-bot-prs.yaml" [ "$status" -eq 0 ] } +@test "a failed approval blocks the merge" { + # Without this the bypass path merges a PR carrying no approval at all: the + # approve step is continue-on-error, so the job marches on regardless. + run grep -F " id: approve" "$ACTION" + [ "$status" -eq 0 ] + run grep -F "steps.approve.outcome == 'success'" "$ACTION" + [ "$status" -eq 0 ] + # outcome, not conclusion — continue-on-error rewrites conclusion to success. + run grep -F "steps.approve.conclusion" "$ACTION" + [ "$status" -ne 0 ] +} + @test "the tested head is rechecked after CI and passed to every merge request" { [ "$(grep -Fc 'EXPECTED_HEAD_SHA: ${{ github.event.pull_request.head.sha }}' "$ACTION")" -eq 2 ] run grep -F "id: recheck" "$ACTION" From abf2b29e85d7f9df83d74b33c90f64caa27ad568 Mon Sep 17 00:00:00 2001 From: Caue Santos Date: Mon, 24 Aug 2026 19:53:49 -0600 Subject: [PATCH 3/5] fix(auto-approve): scope approval gate to bypass merges --- .github/actions/auto-approve-bot-prs/action.yml | 8 ++++---- .../auto-approve-bot-prs/test/token-contract.bats | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.github/actions/auto-approve-bot-prs/action.yml b/.github/actions/auto-approve-bot-prs/action.yml index 36e5a67a..c079e970 100644 --- a/.github/actions/auto-approve-bot-prs/action.yml +++ b/.github/actions/auto-approve-bot-prs/action.yml @@ -111,11 +111,11 @@ runs: For more information, see https://github.com/loft-sh/github-actions?tab=readme-ov-file#auto-approve-bot-prs. - # `outcome`, not `conclusion`: continue-on-error rewrites the latter to - # success. Approval is best-effort, but a merge token with a ruleset bypass - # would otherwise land the PR with no approval recorded. + # Existing callers keep best-effort approval semantics. The opt-in bypass + # path requires a recorded approval; `outcome`, not `conclusion`, preserves + # a failure when continue-on-error rewrites the latter to success. - name: Merge PR - if: steps.recheck.outputs.proceed == 'true' && inputs.auto-merge == 'true' && steps.approve.outcome == 'success' + if: steps.recheck.outputs.proceed == 'true' && inputs.auto-merge == 'true' && (inputs.merge-when-blocked != 'true' || steps.approve.outcome == 'success') shell: bash env: GH_TOKEN: ${{ inputs.merge-token || inputs.github-token }} diff --git a/.github/actions/auto-approve-bot-prs/test/token-contract.bats b/.github/actions/auto-approve-bot-prs/test/token-contract.bats index d843c35c..1977bec3 100644 --- a/.github/actions/auto-approve-bot-prs/test/token-contract.bats +++ b/.github/actions/auto-approve-bot-prs/test/token-contract.bats @@ -24,12 +24,13 @@ WORKFLOW="$BATS_TEST_DIRNAME/../../../workflows/auto-approve-bot-prs.yaml" [ "$status" -eq 0 ] } -@test "a failed approval blocks the merge" { - # Without this the bypass path merges a PR carrying no approval at all: the - # approve step is continue-on-error, so the job marches on regardless. +@test "a failed approval blocks only the opt-in bypass merge" { + # The bypass path must not merge a PR carrying no approval at all. Existing + # callers leave merge-when-blocked off and retain their best-effort approval + # behavior, including release orchestration that already depends on it. run grep -F " id: approve" "$ACTION" [ "$status" -eq 0 ] - run grep -F "steps.approve.outcome == 'success'" "$ACTION" + run grep -F "inputs.merge-when-blocked != 'true' || steps.approve.outcome == 'success'" "$ACTION" [ "$status" -eq 0 ] # outcome, not conclusion — continue-on-error rewrites conclusion to success. run grep -F "steps.approve.conclusion" "$ACTION" From 6adfaadc558b451ccef96cf9a9a7344ec10ec737 Mon Sep 17 00:00:00 2001 From: Caue Santos Date: Tue, 25 Aug 2026 08:13:36 -0600 Subject: [PATCH 4/5] fix(auto-approve-bot-prs): annotate the approval gate and cover the env mapping --- .../actions/auto-approve-bot-prs/README.md | 31 ++++++++++++------- .../actions/auto-approve-bot-prs/action.yml | 14 +++++---- .../src/enable-auto-merge.sh | 7 +++++ .../test/enable-auto-merge.bats | 24 ++++++++++++++ .../auto-approve-bot-prs/test/gh_mock.bash | 6 +++- .../test/token-contract.bats | 15 ++++++++- .github/workflows/auto-approve-bot-prs.yaml | 4 +-- docs/workflows/auto-approve-bot-prs.md | 28 ++++++++--------- 8 files changed, 93 insertions(+), 36 deletions(-) diff --git a/.github/actions/auto-approve-bot-prs/README.md b/.github/actions/auto-approve-bot-prs/README.md index 3cf7a0a5..39002dee 100644 --- a/.github/actions/auto-approve-bot-prs/README.md +++ b/.github/actions/auto-approve-bot-prs/README.md @@ -93,6 +93,13 @@ and a token without a bypass is refused there too. It is opt-in because for a token that *does* carry one, it decides whether the action merges only what CI approved or merges past whatever that bypass covers. +It also makes approval load-bearing. Approving is best-effort everywhere else — +the step is `continue-on-error` — but merging past a review requirement with no +review recorded would defeat the audit trail, so with `merge-when-blocked: true` +a failed approval refuses the merge and says so at `::error::` level. Callers +that leave the setting off keep the previous behavior, approval failure +included. + The action carries the pull request head SHA from the triggering event through the run. It checks that SHA once before polling CI and again after CI passes. If Renovate or another actor updates the branch while an older run is still @@ -161,18 +168,18 @@ PR is benign, and a PR closed unmerged is a human decision. -| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | -|--------------------|--------|----------|------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| auto-merge | string | false | `"false"` | Merge the PR after approving it,
directly where possible. See README "Merging". | -| ci-read-token | string | false | | Token for the read-only CI poll
only; defaults to the caller GITHUB_TOKEN,
which needs `checks: read` and `statuses: read`. Never
the approving PAT. See README "Tokens
by purpose". | -| 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. | -| merge-when-blocked | string | false | `"false"` | Retry a refused merge through the
merge API, so GitHub decides instead
of gh's client-side mergeability check. Needed
when the merge token merges via
a ruleset bypass. See README "Merging". | -| 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. | -| wait-sleep-seconds | string | false | `"10"` | Seconds between polling attempts | +| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | +|--------------------|--------|----------|------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| auto-merge | string | false | `"false"` | Merge the PR after approving it,
directly where possible. See README "Merging". | +| ci-read-token | string | false | | Token for the read-only CI poll
only; defaults to the caller GITHUB_TOKEN,
which needs `checks: read` and `statuses: read`. Never
the approving PAT. See README "Tokens
by purpose". | +| 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 — and
with merge-when-blocked, that path is its
ruleset bypass. | +| merge-when-blocked | string | false | `"false"` | Retry a refused merge through the
merge API, so GitHub decides instead
of gh's client-side mergeability check. Needed
when the merge token merges via
a ruleset bypass. Also requires the
approval to have been recorded. See
README "Merging". | +| 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. | +| wait-sleep-seconds | string | false | `"10"` | Seconds between polling attempts | diff --git a/.github/actions/auto-approve-bot-prs/action.yml b/.github/actions/auto-approve-bot-prs/action.yml index c079e970..30e2543d 100644 --- a/.github/actions/auto-approve-bot-prs/action.yml +++ b/.github/actions/auto-approve-bot-prs/action.yml @@ -21,14 +21,14 @@ inputs: required: false default: 'false' merge-when-blocked: - description: 'Retry a refused merge through the merge API, so GitHub decides instead of gh''s client-side mergeability check. Needed when the merge token merges via a ruleset bypass. See README "Merging".' + description: 'Retry a refused merge through the merge API, so GitHub decides instead of gh''s client-side mergeability check. Needed when the merge token merges via a ruleset bypass. Also requires the approval to have been recorded. See README "Merging".' required: false default: 'false' github-token: description: 'PAT used to read PR state and approve. Must NOT match the PR author. Also used to merge when merge-token is omitted.' required: true merge-token: - description: '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.' + description: '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 — and with merge-when-blocked, that path is its ruleset bypass.' required: false ci-read-token: description: 'Token for the read-only CI poll only; defaults to the caller GITHUB_TOKEN, which needs `checks: read` and `statuses: read`. Never the approving PAT. See README "Tokens by purpose".' @@ -111,11 +111,12 @@ runs: For more information, see https://github.com/loft-sh/github-actions?tab=readme-ov-file#auto-approve-bot-prs. - # Existing callers keep best-effort approval semantics. The opt-in bypass - # path requires a recorded approval; `outcome`, not `conclusion`, preserves - # a failure when continue-on-error rewrites the latter to success. + # Existing callers keep best-effort approval semantics; the opt-in bypass + # path requires a recorded approval. The script enforces that rather than + # this `if`, so a refusal is annotated instead of silently skipped. + # `outcome`, not `conclusion`: continue-on-error rewrites the latter. - name: Merge PR - if: steps.recheck.outputs.proceed == 'true' && inputs.auto-merge == 'true' && (inputs.merge-when-blocked != 'true' || steps.approve.outcome == 'success') + if: steps.recheck.outputs.proceed == 'true' && inputs.auto-merge == 'true' shell: bash env: GH_TOKEN: ${{ inputs.merge-token || inputs.github-token }} @@ -123,4 +124,5 @@ runs: PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} MERGE_METHOD: ${{ inputs.merge-method }} MERGE_WHEN_BLOCKED: ${{ inputs.merge-when-blocked }} + APPROVAL_OUTCOME: ${{ steps.approve.outcome }} run: ${{ github.action_path }}/src/enable-auto-merge.sh diff --git a/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh b/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh index 88b0dd10..72daaa77 100755 --- a/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh +++ b/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh @@ -53,6 +53,13 @@ esac # would otherwise fire back-to-back with no delay, so one blip can take out both. # This mirrors the retry discipline the CI wait and the mergeability poll already # apply. Never fails: the caller decides what a non-zero merge means. +# The bypass path merges past the review requirement, so it must not run when the +# approval never landed. Unset means success, keeping direct callers unchanged. +if [ "${MERGE_WHEN_BLOCKED:-false}" = "true" ] && [ "${APPROVAL_OUTCOME:-success}" != "success" ]; then + echo "::error::PR #${PR_NUMBER} was NOT merged: merge-when-blocked needs a recorded approval, and the approve step reported '$(safe "${APPROVAL_OUTCOME:-}")'. Anything waiting on this merge will stall until the approval lands." + exit 0 +fi + MERGE_RETRY_SLEEP="${MERGE_RETRY_SLEEP_SECONDS:-5}" try_merge() { local out rc diff --git a/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats b/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats index b73f2aaf..92007f97 100644 --- a/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats +++ b/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats @@ -229,6 +229,30 @@ api_merge_count() { grep -c '^api .*/pulls/42/merge' "$GH_MOCK_CALLS" || true; } [[ "$output" == *"api boom"* ]] } +@test "merge-when-blocked on + failed approval → refuses to merge, and says why" { + # Skipping the step instead would leave nobody a reason for the stall. + MERGE_WHEN_BLOCKED=true APPROVAL_OUTCOME=failure GH_MOCK_PR_MERGE_EXIT=0 run "$SCRIPT" + [ "$status" -eq 0 ] + [[ "$output" == *"::error::"* ]] + [[ "$output" == *"needs a recorded approval"* ]] + assert_no_merge_at_all + assert_no_api_merge +} + +@test "merge-when-blocked off + failed approval → merges, as it did before" { + MERGE_WHEN_BLOCKED=false APPROVAL_OUTCOME=failure GH_MOCK_PR_MERGE_EXIT=0 run "$SCRIPT" + [ "$status" -eq 0 ] + [[ "$output" == *"Merged PR #42 (squash)"* ]] + [[ "$output" != *"::error::"* ]] +} + +@test "an unset approval outcome is treated as success" { + # Direct callers of the script pass no APPROVAL_OUTCOME at all. + MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=0 run "$SCRIPT" + [ "$status" -eq 0 ] + [[ "$output" == *"Merged PR #42 (squash)"* ]] +} + @test "each valid merge method is passed through to gh" { for m in squash merge rebase; do : > "$GH_MOCK_CALLS" 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 2f9b8748..c394bddd 100644 --- a/.github/actions/auto-approve-bot-prs/test/gh_mock.bash +++ b/.github/actions/auto-approve-bot-prs/test/gh_mock.bash @@ -39,6 +39,8 @@ set -o pipefail # GH_MOCK_PR_MERGE_EXIT so tests that don't care # about the distinction keep working # GH_MOCK_PR_MERGE_AUTO_OUT → stdout for `gh pr merge --auto` +# GH_MOCK_API_MERGE_EXIT → exit code for `gh api ... /pulls/N/merge` +# GH_MOCK_API_MERGE_OUT → stderr for a failing `gh api ... /pulls/N/merge` # GH_MOCK_PR_STATE → state for `gh pr view --json state` (OPEN|MERGED|CLOSED) # GH_MOCK_CALLS → path; each invocation appends one line of args @@ -143,7 +145,9 @@ case "${1:-}" in while [ $# -gt 0 ]; do case "$1" in --jq) jq_filter="$2"; shift 2 ;; - --method|--header|-H|-X|-f|-F) shift 2>/dev/null || true ;; + # Two shifts, not `shift 2`: a failed `shift 2` on a trailing flag would + # leave $# unchanged and spin this loop forever. + --method|--header|-H|-X|-f|-F) shift; shift 2>/dev/null || true ;; --paginate) shift ;; *) [ -z "$path" ] && path="$1"; shift ;; esac diff --git a/.github/actions/auto-approve-bot-prs/test/token-contract.bats b/.github/actions/auto-approve-bot-prs/test/token-contract.bats index 1977bec3..85ba5afd 100644 --- a/.github/actions/auto-approve-bot-prs/test/token-contract.bats +++ b/.github/actions/auto-approve-bot-prs/test/token-contract.bats @@ -30,13 +30,26 @@ WORKFLOW="$BATS_TEST_DIRNAME/../../../workflows/auto-approve-bot-prs.yaml" # behavior, including release orchestration that already depends on it. run grep -F " id: approve" "$ACTION" [ "$status" -eq 0 ] - run grep -F "inputs.merge-when-blocked != 'true' || steps.approve.outcome == 'success'" "$ACTION" + run grep -F 'APPROVAL_OUTCOME: ${{ steps.approve.outcome }}' "$ACTION" [ "$status" -eq 0 ] # outcome, not conclusion — continue-on-error rewrites conclusion to success. run grep -F "steps.approve.conclusion" "$ACTION" [ "$status" -ne 0 ] } +@test "merge-when-blocked reaches the merge script and the composite" { + # The feature's only activation conduit: a typo here leaves it inert with the + # whole suite green, since every script test sets the env var directly. + run grep -F 'MERGE_WHEN_BLOCKED: ${{ inputs.merge-when-blocked }}' "$ACTION" + [ "$status" -eq 0 ] + run grep -F " merge-when-blocked:" "$ACTION" + [ "$status" -eq 0 ] + run grep -F 'merge-when-blocked: ${{ inputs.merge-when-blocked }}' "$WORKFLOW" + [ "$status" -eq 0 ] + run grep -F " merge-when-blocked:" "$WORKFLOW" + [ "$status" -eq 0 ] +} + @test "the tested head is rechecked after CI and passed to every merge request" { [ "$(grep -Fc 'EXPECTED_HEAD_SHA: ${{ github.event.pull_request.head.sha }}' "$ACTION")" -eq 2 ] run grep -F "id: recheck" "$ACTION" diff --git a/.github/workflows/auto-approve-bot-prs.yaml b/.github/workflows/auto-approve-bot-prs.yaml index c4052ba6..4985fa08 100644 --- a/.github/workflows/auto-approve-bot-prs.yaml +++ b/.github/workflows/auto-approve-bot-prs.yaml @@ -16,7 +16,7 @@ on: type: boolean default: false merge-when-blocked: - description: 'Retry a refused merge through the merge API so GitHub decides, rather than gh''s client-side check. Set this when the merge token merges via a ruleset bypass.' + description: 'Retry a refused merge through the merge API so GitHub decides, rather than gh''s client-side check. Set this when the merge token merges via a ruleset bypass. Also requires the approval to have been recorded.' type: boolean default: false wait-max-attempts: @@ -36,7 +36,7 @@ on: description: 'GitHub PAT for approving PRs (must be different identity from PR author). Also used for merging when merge-token is omitted.' required: true merge-token: - description: 'Optional token used only for merging when auto-merge is true. Defaults to gh-access-token and needs a merge path on the base branch.' + description: 'Optional token used only for merging when auto-merge is true. Defaults to gh-access-token and needs a merge path on the base branch — with merge-when-blocked, that path is its ruleset bypass.' required: false ci-read-token: description: 'Optional escape hatch for the read-only CI poll when the caller cannot grant `checks: read` / `statuses: read`. Classic PAT with `repo` scope, or an App token. Never gh-access-token.' diff --git a/docs/workflows/auto-approve-bot-prs.md b/docs/workflows/auto-approve-bot-prs.md index 21984203..94a35234 100644 --- a/docs/workflows/auto-approve-bot-prs.md +++ b/docs/workflows/auto-approve-bot-prs.md @@ -10,15 +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) | -| merge-when-blocked | boolean | false | `false` | Retry a refused merge through the
merge API so GitHub decides, rather
than gh's client-side check. Set this
when the merge token merges via
a ruleset bypass. | -| 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) | +| merge-when-blocked | boolean | false | `false` | Retry a refused merge through the
merge API so GitHub decides, rather
than gh's client-side check. Set this
when the merge token merges via
a ruleset bypass. Also requires the
approval to have been recorded. | +| 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. | @@ -26,11 +26,11 @@ secret. -| SECRET | REQUIRED | DESCRIPTION | -|-----------------|----------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| ci-read-token | false | Optional escape hatch for the read-only
CI poll when the caller cannot
grant `checks: read` / `statuses: read`. Classic PAT
with `repo` scope, or an App
token. Never gh-access-token. | -| gh-access-token | true | GitHub PAT for approving PRs (must be different identity from PR author).
Also used for merging when merge-token
is omitted. | -| merge-token | false | Optional token used only for merging
when auto-merge is true. Defaults to
gh-access-token and needs a merge path
on the base branch. | +| SECRET | REQUIRED | DESCRIPTION | +|-----------------|----------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| ci-read-token | false | Optional escape hatch for the read-only
CI poll when the caller cannot
grant `checks: read` / `statuses: read`. Classic PAT
with `repo` scope, or an App
token. Never gh-access-token. | +| gh-access-token | true | GitHub PAT for approving PRs (must be different identity from PR author).
Also used for merging when merge-token
is omitted. | +| merge-token | false | Optional token used only for merging
when auto-merge is true. Defaults to
gh-access-token and needs a merge path
on the base branch — with
merge-when-blocked, that path is its ruleset
bypass. | From 3590e66704391bb7deb1d9af158be2e7a7603fda Mon Sep 17 00:00:00 2001 From: Caue Santos Date: Tue, 25 Aug 2026 10:09:08 -0600 Subject: [PATCH 5/5] fix(auto-approve): fail when requested merge cannot complete --- .../actions/auto-approve-bot-prs/README.md | 44 ++++++++++++------- .../actions/auto-approve-bot-prs/action.yml | 12 +++-- .../src/enable-auto-merge.sh | 15 ++++--- .../test/enable-auto-merge.bats | 24 +++++----- .../test/token-contract.bats | 7 +++ .github/workflows/auto-approve-bot-prs.yaml | 10 ++--- docs/workflows/auto-approve-bot-prs.md | 18 ++++---- 7 files changed, 73 insertions(+), 57 deletions(-) diff --git a/.github/actions/auto-approve-bot-prs/README.md b/.github/actions/auto-approve-bot-prs/README.md index 39002dee..840425b0 100644 --- a/.github/actions/auto-approve-bot-prs/README.md +++ b/.github/actions/auto-approve-bot-prs/README.md @@ -1,9 +1,9 @@ # Auto-approve bot PRs Approves PRs from trusted bot authors whose title or branch matches a known -safe pattern, after all other CI checks pass. No API, parse, permission or -input-validation failure exits non-zero: each degrades to an annotated skip and -exit 0. +safe pattern, after all other CI checks pass. Approval-only failures degrade to +annotated skips and exit 0. When `auto-merge: true`, failure to perform or queue +the requested merge exits non-zero. Refusing to approve is annotated at **error** level, because it is a real outcome that something downstream may be blocking on (a release cut waiting for @@ -93,6 +93,13 @@ and a token without a bypass is refused there too. It is opt-in because for a token that *does* carry one, it decides whether the action merges only what CI approved or merges past whatever that bypass covers. +This is a generic shared capability, not a `loft-enterprise` or Renovate policy. +The action validates its trusted-author patterns, CI result, recorded approval +and tested head. The caller must additionally restrict the eligible authors, +branches and PRs and must configure the merge token's bypass no more broadly +than intended. Enabling this input asks GitHub to evaluate all bypass authority +already carried by that token; it does not narrow that authority itself. + It also makes approval load-bearing. Approving is best-effort everywhere else — the step is `continue-on-error` — but merging past a review requirement with no review recorded would defeat the audit trail, so with `merge-when-blocked: true` @@ -161,25 +168,28 @@ A PR that is approved but ends up merged by neither path is annotated at **error** level, carrying both underlying `gh` errors, since this is the only place that cause is knowable. Those two reasons are the diagnosis; the branch-protection checklist that follows them applies only if they point at -policy rather than a transient API error. Re-runs stay quiet: an already-merged -PR is benign, and a PR closed unmerged is a human decision. +policy rather than a transient API error. The merge step exits non-zero, and the +reusable workflow reports a failed job for `auto-merge: true`; approval-only +callers keep the historical best-effort green check. A successfully accepted +auto-merge queue counts as success. Re-runs stay quiet: an already-merged PR is +benign, and a PR closed unmerged is a human decision. ## Inputs -| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | -|--------------------|--------|----------|------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| auto-merge | string | false | `"false"` | Merge the PR after approving it,
directly where possible. See README "Merging". | -| ci-read-token | string | false | | Token for the read-only CI poll
only; defaults to the caller GITHUB_TOKEN,
which needs `checks: read` and `statuses: read`. Never
the approving PAT. See README "Tokens
by purpose". | -| 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 — and
with merge-when-blocked, that path is its
ruleset bypass. | -| merge-when-blocked | string | false | `"false"` | Retry a refused merge through the
merge API, so GitHub decides instead
of gh's client-side mergeability check. Needed
when the merge token merges via
a ruleset bypass. Also requires the
approval to have been recorded. See
README "Merging". | -| 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. | -| wait-sleep-seconds | string | false | `"10"` | Seconds between polling attempts | +| INPUT | TYPE | REQUIRED | DEFAULT | DESCRIPTION | +|--------------------|--------|----------|------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| auto-merge | string | false | `"false"` | Merge the PR after approving it,
directly where possible. See README "Merging". | +| ci-read-token | string | false | | Token for the read-only CI poll
only; defaults to the caller GITHUB_TOKEN,
which needs `checks: read` and `statuses: read`. Never
the approving PAT. See README "Tokens
by purpose". | +| 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 — and
with merge-when-blocked, that path is its
ruleset bypass. | +| merge-when-blocked | string | false | `"false"` | Allow the merge token's configured ruleset
bypass to be evaluated by retrying
a refused merge through the API.
Generic opt-in: callers must restrict eligible
authors, branches and PRs. Also requires
a recorded approval. See README "Merging". | +| 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. | +| wait-sleep-seconds | string | false | `"10"` | Seconds between polling attempts | diff --git a/.github/actions/auto-approve-bot-prs/action.yml b/.github/actions/auto-approve-bot-prs/action.yml index 30e2543d..a3b34431 100644 --- a/.github/actions/auto-approve-bot-prs/action.yml +++ b/.github/actions/auto-approve-bot-prs/action.yml @@ -1,12 +1,10 @@ name: Auto-approve bot PRs description: | Approves PRs from trusted bot authors whose title/branch matches a known - safe pattern, after all other CI checks pass. Once running, no API, parse or - permission failure exits non-zero: each degrades to an annotated skip and exit - 0, and an out-of-range wait input is coerced to its default with a warning. - (Missing required env is the one deliberate exception and does exit non-zero.) - Outcomes that need a human and raise no other red signal are annotated at error - level; that raises an annotation only, never a non-zero exit. + safe pattern, after all other CI checks pass. Approval-only failures degrade to + annotated skips. When auto-merge is requested, an invalid merge request or + exhaustion of every merge path exits non-zero. The opt-in ruleset-bypass path + also fails unless the approval was recorded. inputs: trusted-authors: description: 'Comma-separated list of trusted bot logins' @@ -21,7 +19,7 @@ inputs: required: false default: 'false' merge-when-blocked: - description: 'Retry a refused merge through the merge API, so GitHub decides instead of gh''s client-side mergeability check. Needed when the merge token merges via a ruleset bypass. Also requires the approval to have been recorded. See README "Merging".' + description: 'Allow the merge token''s configured ruleset bypass to be evaluated by retrying a refused merge through the API. Generic opt-in: callers must restrict eligible authors, branches and PRs. Also requires a recorded approval. See README "Merging".' required: false default: 'false' github-token: diff --git a/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh b/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh index 72daaa77..7aeb8cd4 100755 --- a/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh +++ b/.github/actions/auto-approve-bot-prs/src/enable-auto-merge.sh @@ -1,5 +1,6 @@ #!/usr/bin/env bash -# Merges the PR. Never exits non-zero. +# Merges the PR. Exits non-zero when a requested merge cannot be performed or +# queued; successful merges, accepted queue requests and benign re-runs exit 0. # # A plain merge is attempted FIRST, and `--auto` is only the fallback. By the # time this runs the action has already waited for every other check to go green @@ -14,10 +15,9 @@ # declared green legitimately refuses a merge right now but can complete later. # Queueing is the right answer there, so a refused plain merge degrades to it. # -# Nothing here exits non-zero (the composite must not red-X a caller's CI over -# an unrelated bot PR), but an approved-and-unmerged PR is reported at -# ::error:: level so the cause is visible in the run summary instead of being -# buried in a notice. +# Approval-only runs never invoke this script. Once a caller requests a merge, +# an approved-and-unmerged PR is a failed automation and must red-X the caller's +# job instead of repeating the silent stall this action is meant to prevent. # # Required env: GH_TOKEN, GITHUB_REPOSITORY, PR_NUMBER, PR_HEAD_SHA, # MERGE_METHOD @@ -40,7 +40,7 @@ case "$MERGE_METHOD" in squash|merge|rebase) ;; *) echo "::error::Invalid merge method '$(safe "$MERGE_METHOD")'; PR #${PR_NUMBER} was approved but not merged" - exit 0 + exit 1 ;; esac @@ -57,7 +57,7 @@ esac # approval never landed. Unset means success, keeping direct callers unchanged. if [ "${MERGE_WHEN_BLOCKED:-false}" = "true" ] && [ "${APPROVAL_OUTCOME:-success}" != "success" ]; then echo "::error::PR #${PR_NUMBER} was NOT merged: merge-when-blocked needs a recorded approval, and the approve step reported '$(safe "${APPROVAL_OUTCOME:-}")'. Anything waiting on this merge will stall until the approval lands." - exit 0 + exit 1 fi MERGE_RETRY_SLEEP="${MERGE_RETRY_SLEEP_SECONDS:-5}" @@ -131,3 +131,4 @@ api_reason="" [ -n "${api_err+x}" ] && api_reason=" Merge API said: $(safe "$api_err" 500)." echo "::error::PR #${PR_NUMBER} was approved but could NOT be merged, and could not be queued for auto-merge either (each path was retried once). Anything waiting on this merge will stall. Plain merge said: $(safe "$direct_err" 500).${api_reason} Auto-merge said: $(safe "$auto_err" 500). Read those reasons first — they name the cause. If they point at policy rather than a transient API error, check branch protection and rulesets (is the token's team a bypass actor during a code freeze?), the token's merge permission, and whether the repository allows auto-merge. No merge-API line above means merge-when-blocked is off, so gh's client-side verdict was final." +exit 1 diff --git a/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats b/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats index 92007f97..fc027a22 100644 --- a/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats +++ b/.github/actions/auto-approve-bot-prs/test/enable-auto-merge.bats @@ -50,10 +50,10 @@ assert_no_merge_at_all() { assert_no_match '^pr merge ' "$(cat "$GH_MOCK_CALLS") [[ "$output" != *"::error::"* ]] } -@test "both merge paths refused → ::error:: naming both reasons, still exits 0" { +@test "both merge paths refused → fails after naming both reasons" { GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=1 GH_MOCK_PR_STATE=OPEN \ GH_MOCK_PR_MERGE_OUT="plain boom" GH_MOCK_PR_MERGE_AUTO_OUT="auto boom" run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] [[ "$output" == *"::error::PR #42 was approved but could NOT be merged"* ]] # Both diagnostics are carried into the annotation, since this is the only # place the cause is knowable. @@ -81,14 +81,14 @@ assert_no_merge_at_all() { assert_no_match '^pr merge ' "$(cat "$GH_MOCK_CALLS") # `gh pr view` failing must not be read as "already merged" — the run still # has to try --auto and, failing that, escalate. GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=1 GH_MOCK_PR_STATE="" run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] auto_merge_attempted [[ "$output" == *"::error::"* ]] } @test "invalid merge method → ::error::, no merge attempted" { MERGE_METHOD="fast-forward" run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] [[ "$output" == *"::error::Invalid merge method 'fast-forward'"* ]] # One assertion covers both: the rejected-method path must issue no # `gh pr merge` at all, neither plain nor --auto. @@ -120,7 +120,7 @@ auto_merge_count() { grep -c '^pr merge .*--auto' "$GH_MOCK_CALLS" || true; } @test "a refused --auto is retried once before escalating" { GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=1 GH_MOCK_PR_STATE=OPEN run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] [ "$(auto_merge_count)" -eq 2 ] [[ "$output" == *"::error::"* ]] # The escalation says it retried, so the reader knows a transient cause was @@ -181,7 +181,7 @@ api_merge_count() { grep -c '^api .*/pulls/42/merge' "$GH_MOCK_CALLS" || true; } MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_STATE=OPEN \ GH_MOCK_API_MERGE_EXIT=1 GH_MOCK_API_MERGE_OUT="At least 1 approving review is required" \ GH_MOCK_PR_MERGE_AUTO_EXIT=1 run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] [[ "$output" == *"::error::"* ]] [[ "$output" == *"Merge API said:"* ]] [[ "$output" == *"At least 1 approving review is required"* ]] @@ -190,7 +190,7 @@ api_merge_count() { grep -c '^api .*/pulls/42/merge' "$GH_MOCK_CALLS" || true; } @test "with merge-when-blocked off the error names the setting rather than hiding it" { # Otherwise the annotation blames branch protection for gh declining to ask. GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=1 GH_MOCK_PR_STATE=OPEN run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] [[ "$output" == *"::error::"* ]] [[ "$output" == *"merge-when-blocked is off"* ]] assert_no_match 'Merge API said:' "$output" @@ -223,7 +223,7 @@ api_merge_count() { grep -c '^api .*/pulls/42/merge' "$GH_MOCK_CALLS" || true; } MERGE_WHEN_BLOCKED=true GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_STATE=OPEN \ GH_MOCK_API_MERGE_EXIT=1 GH_MOCK_API_MERGE_OUT=$'api boom\r::error::FORGED' \ GH_MOCK_PR_MERGE_AUTO_EXIT=1 run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] assert_no_match '\r' "$output" assert_no_match '(?m)^::error::FORGED' "$output" [[ "$output" == *"api boom"* ]] @@ -232,7 +232,7 @@ api_merge_count() { grep -c '^api .*/pulls/42/merge' "$GH_MOCK_CALLS" || true; } @test "merge-when-blocked on + failed approval → refuses to merge, and says why" { # Skipping the step instead would leave nobody a reason for the stall. MERGE_WHEN_BLOCKED=true APPROVAL_OUTCOME=failure GH_MOCK_PR_MERGE_EXIT=0 run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] [[ "$output" == *"::error::"* ]] [[ "$output" == *"needs a recorded approval"* ]] assert_no_merge_at_all @@ -315,7 +315,7 @@ api_merge_count() { grep -c '^api .*/pulls/42/merge' "$GH_MOCK_CALLS" || true; } GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=1 GH_MOCK_PR_STATE=OPEN \ GH_MOCK_PR_MERGE_OUT="plain boom" \ GH_MOCK_PR_MERGE_AUTO_OUT=$'auto boom\r::error::FORGED' run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] assert_no_match '\r' "$output" assert_no_match '(?m)^::error::FORGED' "$output" [[ "$output" == *"was approved but could NOT be merged"* ]] @@ -326,7 +326,7 @@ api_merge_count() { grep -c '^api .*/pulls/42/merge' "$GH_MOCK_CALLS" || true; } # Caller-controlled rather than API-controlled: merge-method is a plain # workflow_call string input, echoed back on rejection. MERGE_METHOD=$'fast-forward\r::error::FORGED' run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] assert_no_match '\r' "$output" assert_no_match '(?m)^::error::FORGED' "$output" [[ "$output" == *"Invalid merge method"* ]] @@ -348,7 +348,7 @@ api_merge_count() { grep -c '^api .*/pulls/42/merge' "$GH_MOCK_CALLS" || true; } long=$(printf 'x%.0s' $(seq 1 900)) GH_MOCK_PR_MERGE_EXIT=1 GH_MOCK_PR_MERGE_AUTO_EXIT=1 GH_MOCK_PR_STATE=OPEN \ GH_MOCK_PR_MERGE_OUT="$long" GH_MOCK_PR_MERGE_AUTO_OUT="$long" run "$SCRIPT" - [ "$status" -eq 0 ] + [ "$status" -ne 0 ] [[ "$output" == *"... (truncated)"* ]] # Bounded: the 900-char payload cannot reach the annotation whole. assert_no_match "x{600}" "$output" diff --git a/.github/actions/auto-approve-bot-prs/test/token-contract.bats b/.github/actions/auto-approve-bot-prs/test/token-contract.bats index 85ba5afd..8e3349ab 100644 --- a/.github/actions/auto-approve-bot-prs/test/token-contract.bats +++ b/.github/actions/auto-approve-bot-prs/test/token-contract.bats @@ -50,6 +50,13 @@ WORKFLOW="$BATS_TEST_DIRNAME/../../../workflows/auto-approve-bot-prs.yaml" [ "$status" -eq 0 ] } +@test "the reusable job fails only when a requested merge cannot be performed" { + # Approval-only callers keep the historical best-effort job. Callers asking + # for a merge must see a red check when every merge path is refused. + run grep -F 'continue-on-error: ${{ !inputs.auto-merge }}' "$WORKFLOW" + [ "$status" -eq 0 ] +} + @test "the tested head is rechecked after CI and passed to every merge request" { [ "$(grep -Fc 'EXPECTED_HEAD_SHA: ${{ github.event.pull_request.head.sha }}' "$ACTION")" -eq 2 ] run grep -F "id: recheck" "$ACTION" diff --git a/.github/workflows/auto-approve-bot-prs.yaml b/.github/workflows/auto-approve-bot-prs.yaml index 4985fa08..d441dcb9 100644 --- a/.github/workflows/auto-approve-bot-prs.yaml +++ b/.github/workflows/auto-approve-bot-prs.yaml @@ -16,7 +16,7 @@ on: type: boolean default: false merge-when-blocked: - description: 'Retry a refused merge through the merge API so GitHub decides, rather than gh''s client-side check. Set this when the merge token merges via a ruleset bypass. Also requires the approval to have been recorded.' + description: 'Allow the merge token''s configured ruleset bypass to be evaluated by retrying a refused merge through the API. Generic opt-in: callers must restrict eligible authors, branches and PRs. Also requires a recorded approval.' type: boolean default: false wait-max-attempts: @@ -59,10 +59,10 @@ jobs: contents: read checks: read statuses: read - # Safety net — internal steps already guard every failure mode, but if - # anything unforeseen slips through this still prevents the job from - # reporting a hard red check on caller CI. - continue-on-error: true + # Approval-only callers retain the historical best-effort green check. + # A caller requesting a merge must see a red check when no merge path is + # accepted; otherwise the PR can stall while this workflow reports success. + continue-on-error: ${{ !inputs.auto-merge }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: diff --git a/docs/workflows/auto-approve-bot-prs.md b/docs/workflows/auto-approve-bot-prs.md index 94a35234..a50d3639 100644 --- a/docs/workflows/auto-approve-bot-prs.md +++ b/docs/workflows/auto-approve-bot-prs.md @@ -10,15 +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) | -| merge-when-blocked | boolean | false | `false` | Retry a refused merge through the
merge API so GitHub decides, rather
than gh's client-side check. Set this
when the merge token merges via
a ruleset bypass. Also requires the
approval to have been recorded. | -| 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) | +| merge-when-blocked | boolean | false | `false` | Allow the merge token's configured ruleset
bypass to be evaluated by retrying
a refused merge through the API.
Generic opt-in: callers must restrict eligible
authors, branches and PRs. Also requires
a recorded approval. | +| 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. |