diff --git a/.github/actions/auto-approve-bot-prs/README.md b/.github/actions/auto-approve-bot-prs/README.md index 6d4d275e..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 @@ -80,6 +80,33 @@ 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. + +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` +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 @@ -141,24 +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. | -| 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 c275d829..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' @@ -20,11 +18,15 @@ inputs: description: 'Merge the PR after approving it, directly where possible. See README "Merging".' required: false default: 'false' + merge-when-blocked: + 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: 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".' @@ -96,6 +98,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 @@ -106,6 +109,10 @@ 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. 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' shell: bash @@ -114,4 +121,6 @@ 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 }} + 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 c3a6e6f0..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,13 +15,13 @@ # 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 +# Optional env: MERGE_WHEN_BLOCKED (default false), MERGE_RETRY_SLEEP_SECONDS set -euo pipefail : "${GITHUB_REPOSITORY:?GITHUB_REPOSITORY required}" @@ -39,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 @@ -52,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 1 +fi + MERGE_RETRY_SLEEP="${MERGE_RETRY_SLEEP_SECONDS:-5}" try_merge() { local out rc @@ -86,6 +94,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 +127,8 @@ 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." +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 4457a407..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 @@ -142,6 +142,117 @@ 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" -ne 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" -ne 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" -ne 0 ] + assert_no_match '\r' "$output" + assert_no_match '(?m)^::error::FORGED' "$output" + [[ "$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" -ne 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" @@ -204,7 +315,7 @@ auto_merge_count() { grep -c '^pr merge .*--auto' "$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"* ]] @@ -215,7 +326,7 @@ auto_merge_count() { grep -c '^pr merge .*--auto' "$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"* ]] @@ -237,7 +348,7 @@ auto_merge_count() { grep -c '^pr merge .*--auto' "$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/gh_mock.bash b/.github/actions/auto-approve-bot-prs/test/gh_mock.bash index f7b4df1a..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 @@ -71,6 +73,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 +139,17 @@ 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 ;; + # 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 done emit_api_response "$path" | apply_filter "$jq_filter" 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..8e3349ab 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,39 @@ WORKFLOW="$BATS_TEST_DIRNAME/../../../workflows/auto-approve-bot-prs.yaml" [ "$status" -eq 0 ] } +@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 '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 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 5df359e1..d441dcb9 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: '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: description: 'Max polling attempts waiting for other CI checks (raise this when a slow required check, e.g. e2e, gates the PR).' type: string @@ -32,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.' @@ -55,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: @@ -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..a50d3639 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` | 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. | @@ -25,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. |