ROX-34007: Enhance auto-merge configurability #91
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| #!/bin/bash | ||
| # | ||
| # Enables auto-merge for eligible PRs with specified labels. | ||
| # PRs can be filtered by labels, base branches, and allowed author. | ||
| # Required status checks must pass for auto-merge to be enabled. | ||
| # | ||
| # Local run: | ||
| # | ||
| # test/local-env.sh automerge/automerge.sh <repository> <limit> <label1,label2,...> <allowed-author1,allowed-author2,...> <required-checks> <allowed-base-branches> | ||
| # | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| function main() { | ||
| REPOSITORY="${1:-}" | ||
| LIMIT="${2:-}" | ||
| LABELS="${3:-}" | ||
| ALLOWED_AUTHORS="${4:-}" | ||
| REQUIRED_CHECKS="${5:-}" | ||
| ALLOWED_BASE_BRANCHES="${6:-}" | ||
|
|
||
| check_not_empty \ | ||
| DRY_RUN GH_TOKEN \ | ||
| REPOSITORY LIMIT LABELS ALLOWED_AUTHORS REQUIRED_CHECKS ALLOWED_BASE_BRANCHES | ||
|
Comment on lines
+22
to
+24
Contributor
There was a problem hiding this comment. Hm. Since you have
Collaborator
Author
There was a problem hiding this comment. The inputs are not required on the action level (mostly for backwards compatibility) and we provide reasonable defaults there. Alternatively, we could provide defaults again in the script (duplicate the defaults from the action definition) or work with named arguments (overhead). TLDR: For backwards compatibility and simplicity, we should keep the current implementation.
Contributor
There was a problem hiding this comment. How does the combination of default: 'blah'
required: falsework? If the caller does not provide the attribute, the default
steps:
- uses: my-fancy-action
with:
blah: nullIs this a backward compatibility scenario to be at all concerned about? |
||
|
|
||
| gh_log notice "Querying PRs with '${LABELS}' label(s) in '${REPOSITORY}', allowed authors: '${ALLOWED_AUTHORS}', required checks: '${REQUIRED_CHECKS}', allowed base branches: '${ALLOWED_BASE_BRANCHES}'" | ||
| gh_log notice "DRY_RUN: ${DRY_RUN}" | ||
|
|
||
| # Extract repo owner and name | ||
| IFS='/' read -r OWNER REPO <<< "${REPOSITORY}" | ||
|
|
||
| # Get all PRs with auto-merge labels (non-draft, mergeable only) | ||
| PR_DATA=$(gh pr list \ | ||
| --repo "${REPOSITORY}" \ | ||
| --label "${LABELS}" \ | ||
| --draft=false \ | ||
| --state open \ | ||
| --limit "${LIMIT}" \ | ||
| --json number,mergeable,author,baseRefName \ | ||
| --jq ".[] | select(.mergeable == \"MERGEABLE\") | {number, author: .author.login, baseRefName: .baseRefName}") | ||
|
|
||
| if [[ -z "${PR_DATA}" ]]; then | ||
| gh_log notice "No eligible PRs found with '${LABELS}' labels" | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Process each PR | ||
| echo "${PR_DATA}" | jq -c '.' | while read -r PR_JSON; do | ||
| PR_NUMBER=$(echo "$PR_JSON" | jq -r '.number') | ||
| AUTHOR=$(echo "$PR_JSON" | jq -r '.author') | ||
| BASE_BRANCH=$(echo "$PR_JSON" | jq -r '.baseRefName') | ||
|
|
||
| echo "[DEBUG] PR #${PR_NUMBER} - author='${AUTHOR}', base branch='${BASE_BRANCH}'" | ||
| if [[ ! "${BASE_BRANCH}" =~ ${ALLOWED_BASE_BRANCHES} ]]; then | ||
| echo "[DEBUG] PR #${PR_NUMBER} skipped - base branch '${BASE_BRANCH}' not allowed" | ||
| continue | ||
|
Comment on lines
+55
to
+56
Contributor
There was a problem hiding this comment. The default Is there any way to randomize the results of Alternatively,
Collaborator
Author
There was a problem hiding this comment. Counterpoint: Why may it be okay to have the current limit?
I think 50 is a good compromise for now, and I would not invest in optimizations with randomization or custom limit implementations. If we encounter >50 PRs, we can revisit this.
Contributor
There was a problem hiding this comment. The MintMaker assumption may not hold infinitely true. MintMaker will keep opening PRs against the release branches after we do the last release before EoL but before we tear down the setup. My main concern at this point is how would we find out when we encounter >50 PRs given that this action is to be executed in cron workflow which does not report its status on a PR? The same question applies to #91 (comment): how would the engineer find out that |
||
| fi | ||
|
|
||
| STATUS="$(get_combined_success_status "${PR_NUMBER}")" | ||
|
|
||
| # Only proceed if the required checks have passed | ||
| if [[ "${STATUS}" == "true" ]]; then | ||
| echo "[DEBUG] ✓ PR #${PR_NUMBER} - all required checks passed or skipped" | ||
| else | ||
| echo "[DEBUG] x PR #${PR_NUMBER} skipped - not all required checks passed or skipped" | ||
| continue | ||
| fi | ||
|
|
||
| # Enable auto-merge for all PRs with the label(s) | ||
| if [[ "${DRY_RUN}" == "true" ]]; then | ||
| echo "[DEBUG] ✓ PR #${PR_NUMBER} - would have enabled auto-merge [DRY RUN]" | ||
| else | ||
| gh pr merge --repo "${REPOSITORY}" \ | ||
| --auto --squash "${PR_NUMBER}" | ||
| echo "[DEBUG] ✓ PR #${PR_NUMBER} - auto-merge enabled" | ||
| fi | ||
|
|
||
| # Approve only PRs by allowed authors | ||
|
msugakov marked this conversation as resolved.
|
||
| if [[ ",${ALLOWED_AUTHORS}," == *",${AUTHOR},"* ]]; then | ||
| if [[ "${DRY_RUN}" == "true" ]]; then | ||
| echo "[DEBUG] ✓ PR #${PR_NUMBER} - would have approved [DRY RUN]" | ||
| else | ||
| gh pr review --repo "${REPOSITORY}" \ | ||
| --approve "${PR_NUMBER}" | ||
|
Comment on lines
+83
to
+84
Contributor
There was a problem hiding this comment. I spotted the older command had
Collaborator
Author
There was a problem hiding this comment. I assume it was done to continue the workflow if approval fails. I would actually prefer to let it fail (not silently), because only if it breaks, engineers will investigate and implement improvements if necessary. |
||
| echo "[DEBUG] ✓ PR #${PR_NUMBER} - approved" | ||
| fi | ||
| else | ||
| echo "[DEBUG] x PR #${PR_NUMBER} not approved - author '${AUTHOR}' not in allowed authors" | ||
| fi | ||
| done | ||
| } | ||
|
|
||
| # Collects all status checks and checkruns for the PR and returns a boolean indicating if all required checks have passed or been skipped. | ||
| # The REQUIRED_CHECKS regex parameter must return at least one check. | ||
| function get_combined_success_status() { | ||
| PAGE_SIZE=100 | ||
| CURSOR="" | ||
| NODES_JSON='[]' | ||
|
|
||
| # shellcheck disable=SC2016 | ||
| QUERY=' | ||
| query($owner: String!, $repo: String!, $number: Int!, $first: Int!, $after: String) { | ||
| repository(owner: $owner, name: $repo) { | ||
| pullRequest(number: $number) { | ||
| commits(last: 1) { | ||
| nodes { | ||
| commit { | ||
| statusCheckRollup { | ||
| contexts(first: $first, after: $after) { | ||
| pageInfo { | ||
| hasNextPage | ||
| endCursor | ||
| } | ||
| nodes { | ||
| ... on CheckRun { | ||
| name | ||
| status | ||
| conclusion | ||
| } | ||
| ... on StatusContext { | ||
| context | ||
| state | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ' | ||
|
|
||
| while true; do | ||
| ARGS=( | ||
| graphql | ||
| -F owner="$OWNER" | ||
| -F repo="$REPO" | ||
| -F number="$PR_NUMBER" | ||
| -F first="$PAGE_SIZE" | ||
| -f query="$QUERY" | ||
| ) | ||
| if [[ -n "${CURSOR}" ]]; then | ||
| ARGS+=(-F after="${CURSOR}") | ||
| fi | ||
|
|
||
| RESP=$(gh api "${ARGS[@]}") | ||
|
|
||
| PAGE_NODES=$(echo "${RESP}" | jq '.data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.contexts.nodes // []') | ||
| NODES_JSON=$(jq -n --argjson acc "${NODES_JSON}" --argjson page "${PAGE_NODES}" '$acc + $page') | ||
|
|
||
| HAS_NEXT=$(echo "${RESP}" | jq -r '.data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.contexts.pageInfo.hasNextPage // false') | ||
| if [[ "${HAS_NEXT}" != "true" ]]; then | ||
| break | ||
| fi | ||
| CURSOR=$(echo "${RESP}" | jq -r '.data.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup.contexts.pageInfo.endCursor // empty') | ||
| if [[ -z "${CURSOR}" ]]; then | ||
| gh_log error "Pagination indicated hasNextPage but endCursor is empty" | ||
| exit 1 | ||
| fi | ||
| done | ||
|
|
||
| echo "$NODES_JSON" | jq -r --arg pattern "${REQUIRED_CHECKS}" ' | ||
| [.[] | ||
| | select( | ||
| (.name != null and (.name | test($pattern))) | ||
| or (.context != null and (.context | test($pattern))) | ||
| ) | ||
| | if .name != null then {conclusion: .conclusion} | ||
| else {conclusion: .state} | ||
| end | ||
| ] | ||
| | length > 0 and all(.conclusion == "SUCCESS" or .conclusion == "SKIPPED") | ||
| ' | ||
| } | ||
|
|
||
| main "$@" | ||
Uh oh!
There was an error while loading. Please reload this page.