Skip to content

Commit b39624c

Browse files
committed
ci: settle on merge-after-approval, and stop floating a ruleset bypass
The comments and the skip message offered "or a ruleset bypass actor for this workflow" as though the decision were still open. It is not: main's ruleset keeps its code-owner approval requirement and gains no bypass actor, because unattended merging would mean granting some identity the right to bypass code-owner review on a public repository, which is not a trade worth making for dependency bumps in demo applications. A bot approval could not satisfy a CODEOWNERS requirement anyway. Behaviour is unchanged — the merge is still attempted and the outcome classified, which is simpler than mirroring the ruleset's configuration in the script and cannot drift from it. The skip message for the ordinary case now reads as a worklist entry: "ready to merge, awaiting a code-owner approval (@aws/aws-lambda-tooling)". What the workflow is worth without unattended merging: approve once and the merge happens within the hour, but only when the verification covers the exact commit that lands, so a stale approval cannot merge an unverified head. The hourly sweep is what makes approving a batch and walking away work. Six paths re-run, all unchanged.
1 parent f049b3c commit b39624c

2 files changed

Lines changed: 26 additions & 22 deletions

File tree

‎.github/scripts/dependabot-automerge.sh‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,16 @@
33
# Merges one Dependabot pull request, if it is an example-only update that Verify Examples
44
# has verified at the pull request's current head.
55
#
6-
# Whether that merge is unattended depends on main's ruleset, not on this script. The
7-
# ruleset currently requires one code-owner approval and lists no bypass actors, so the
8-
# merge is refused until a human approves and the refusal is reported as exactly that.
9-
# Making this workflow's identity a bypass actor turns the same code into unattended
10-
# auto-merge, with the guards below as the only thing standing between a bump and main —
11-
# which is why they are what they are: example-only, verified at this exact head, every
12-
# check green.
6+
# It merges after a human approves; it never merges unattended, and it never approves
7+
# anything itself. main's ruleset requires one code-owner approval
8+
# (.github/CODEOWNERS assigns `*` to @aws/aws-lambda-tooling) with no bypass actors, and
9+
# that was left alone deliberately — unattended merging would mean granting some identity
10+
# the right to bypass code-owner review on a public repository, which is not a trade worth
11+
# making for dependency bumps in demo applications.
12+
#
13+
# What it removes is the second trip: approve once and the merge happens within the hour,
14+
# but only when the verification covers the exact commit that lands, so a stale approval
15+
# cannot merge an unverified head.
1316
#
1417
# Usage: REPO=<owner/repo> dependabot-automerge.sh <pr-number>
1518
#
@@ -166,11 +169,9 @@ if [[ -n "$unverified" ]]; then
166169
skip "in the matrix but not verified by run $run_id: $(join_list "$unverified")"
167170
fi
168171

169-
# Deliberately no approval gate of its own: the merge is attempted and the outcome
170-
# classified below. That way this one script behaves correctly whichever way main's
171-
# ruleset is configured — it merges unattended where the workflow is a bypass actor, and
172-
# reports "waiting for a code-owner approval" where it is not, with no toggle to keep in
173-
# sync with a repository setting it cannot see.
172+
# No approval gate of its own: the merge is attempted and the outcome classified below.
173+
# The ruleset is the authority on whether a merge may happen, so asking it is both simpler
174+
# than mirroring its configuration here and impossible to get out of sync with.
174175
review=$(jq -r '.reviewDecision // "NONE"' <<<"$pr_json")
175176
echo "PR #$PR is example-only and verified at $head_sha by run $run_id (reviewDecision=$review). Merging."
176177

@@ -208,10 +209,9 @@ case "$state" in
208209
# reported as "a sibling update landed first", which was simply the wrong diagnosis:
209210
# main's ruleset blocks a merge until the required review is satisfied.
210211
BLOCKED)
211-
# main's ruleset requires one code-owner approval (.github/CODEOWNERS assigns `*` to
212-
# @aws/aws-lambda-tooling) and lists no bypass actors, so this is the expected
213-
# outcome until either a human approves or this workflow's identity is made a bypass
214-
# actor. Named precisely, because it used to be reported as a sibling conflict.
212+
# The expected outcome for an unapproved pull request: main's ruleset requires a
213+
# code-owner approval. Named precisely, because it used to be reported as a sibling
214+
# conflict, which it never was.
215215
case "$review" in
216216
APPROVED)
217217
skip "merge rejected, blocked by main's ruleset even though it is approved — a required rule is unsatisfied."
@@ -220,7 +220,7 @@ case "$state" in
220220
skip "merge rejected, a reviewer requested changes."
221221
;;
222222
*)
223-
skip "verified at ${head_sha:0:8} by run $run_id — blocked awaiting a code-owner approval (@aws/aws-lambda-tooling), or a ruleset bypass actor for this workflow."
223+
skip "verified at ${head_sha:0:8} by run $run_id — ready to merge, awaiting a code-owner approval (@aws/aws-lambda-tooling)."
224224
;;
225225
esac
226226
;;

‎.github/workflows/dependabot-automerge.yaml‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@ name: Dependabot Merge
33
# Merges Dependabot pull requests that only touch example applications, once a code owner
44
# has approved them and Verify Examples has verified them.
55
#
6-
# Deliberately not "auto-merge on green": main is governed by a ruleset with one required
7-
# code-owner approval and zero bypass actors, so no token here can merge without a human,
8-
# and a bot approval cannot satisfy a CODEOWNERS requirement. The value is in the other
9-
# half — an approval no longer means "and come back later to click merge", and the merge
10-
# only happens if the verification covers the exact commit that lands. The decision lives in
6+
# Deliberately not "auto-merge on green". main's ruleset requires one code-owner approval
7+
# and has no bypass actors, and that was a considered choice rather than an obstacle:
8+
# unattended merging would mean giving some identity the right to bypass code-owner review
9+
# on a public repository, which is not worth it for dependency bumps in demo apps. A bot
10+
# approval could not satisfy a CODEOWNERS requirement anyway.
11+
#
12+
# The value is the other half — an approval no longer means "and come back later to click
13+
# merge", and the merge only happens if the verification covers the exact commit that
14+
# lands. The decision lives in
1115
# .github/scripts/dependabot-automerge.sh; this file only decides which pull requests to
1216
# offer it.
1317
#

0 commit comments

Comments
 (0)