diff --git a/CHANGELOG.md b/CHANGELOG.md index e22a4be..ac72816 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,23 @@ here. The format is based on ### Fixed +- **`scripts/set-required-checks.sh`**: the branch-protection PUT payload + hardcoded `restrictions: null` and omitted `lock_branch`/`allow_fork_syncing` + entirely, instead of reading and preserving their current values the way the + script already does for `required_pull_request_reviews`. Found reviewing + #55, which cites this script as already run live against 32 `*-mcp` repos. + Verified live against all 32 target repos (not the smaller spot-check from + the initial review): none currently have `restrictions`, `lock_branch`, or + `allow_fork_syncing` configured, so no repo was actually clobbered by the + gap — but the script mutates fleet-wide branch protection and was going to + run again, so it's fixed rather than left as a known footgun. Now + round-trips all three the same read-merge-write way as the review settings, + including converting `restrictions`' GET-shape (objects with `login`/`slug`) + to the PUT-shape it actually accepts (arrays of `login`/`slug` strings), and + extends the pre-write drift check that already guarded review settings to + cover these three fields too — the script refuses to write if any of them + would change, the same discipline it already applied to reviews. + - **`mcp-server-release.yml`**: added a `gate` job that holds the release when **every** commit since the last tag carries an `Auto-Merged-By:` trailer, plus `release-sweeper.yml` + `release-sweeper.sh` to ship the held batch daily. diff --git a/scripts/set-required-checks.sh b/scripts/set-required-checks.sh index 192019e..4e9ad6a 100755 --- a/scripts/set-required-checks.sh +++ b/scripts/set-required-checks.sh @@ -15,7 +15,15 @@ # protection object. Sending only required_status_checks would silently drop # required_pull_request_reviews (1 approval + code-owner) on every repo it # touched. This reads current protection and re-sends it with the checks -# added. +# added. `restrictions`, `lock_branch`, and `allow_fork_syncing` are +# round-tripped the same way (not just required_pull_request_reviews) — +# an earlier version of this script hardcoded restrictions:null and +# silently omitted the other two, which would have clobbered them on any +# repo that had them configured. Verified against all 32 target repos on +# 2026-08-24: none currently have any of the three set, so no live repo +# was actually clobbered by the earlier version — but the gap was real +# and this script mutates fleet-wide branch protection, so it's fixed +# before running again rather than left as a known footgun. # # 2. INTERSECTION-DERIVED CONTEXTS. A required check whose name never reports # blocks every PR in that repo forever. Path-filtered workflows (e.g. @@ -69,17 +77,29 @@ while IFS=$'\t' read -r repo always _sometimes; do required_approving_review_count: ($cur.required_pull_request_reviews.required_approving_review_count // 1) } end ), - restrictions: null, + restrictions: ( + if $cur.restrictions == null then null + else { + users: [($cur.restrictions.users // [])[].login], + teams: [($cur.restrictions.teams // [])[].slug], + apps: [($cur.restrictions.apps // [])[].slug] + } end + ), allow_force_pushes: ($cur.allow_force_pushes.enabled // false), allow_deletions: ($cur.allow_deletions.enabled // false), required_conversation_resolution: ($cur.required_conversation_resolution.enabled // false), required_linear_history: ($cur.required_linear_history.enabled // false), - block_creations: ($cur.block_creations.enabled // false) + block_creations: ($cur.block_creations.enabled // false), + lock_branch: ($cur.lock_branch.enabled // false), + allow_fork_syncing: ($cur.allow_fork_syncing.enabled // false) }')" had="$(jq -r '.required_status_checks.contexts // [] | join(", ")' <<<"$cur")" want="$(jq -r '.required_status_checks.contexts | join(", ")' <<<"$payload")" - # Verify the merge preserved reviews before writing anything. + # Verify the merge preserved reviews AND the other read-merge-write fields + # (restrictions, lock_branch, allow_fork_syncing) before writing anything. + # Same discipline as the review check below — refuse rather than silently + # clobber a field this script isn't actually trying to change. rev_before="$(jq -c '.required_pull_request_reviews | if .==null then null else {d:.dismiss_stale_reviews,c:.require_code_owner_reviews,n:.required_approving_review_count} end' <<<"$cur")" rev_after="$(jq -c '.required_pull_request_reviews | if .==null then null else {d:.dismiss_stale_reviews,c:.require_code_owner_reviews,n:.required_approving_review_count} end' <<<"$payload")" if [[ "$rev_before" != "$rev_after" ]]; then @@ -87,6 +107,15 @@ while IFS=$'\t' read -r repo always _sometimes; do failed=$((failed+1)); continue fi + restr_before="$(jq -c '.restrictions | if .==null then null else {u:([(.users//[])[].login]|sort),t:([(.teams//[])[].slug]|sort),a:([(.apps//[])[].slug]|sort)} end' <<<"$cur")" + restr_after="$(jq -c '.restrictions | if .==null then null else {u:(.users|sort),t:(.teams|sort),a:(.apps|sort)} end' <<<"$payload")" + other_before="$(jq -c '{lb:(.lock_branch.enabled // false), afs:(.allow_fork_syncing.enabled // false)}' <<<"$cur")" + other_after="$(jq -c '{lb:.lock_branch, afs:.allow_fork_syncing}' <<<"$payload")" + if [[ "$restr_before" != "$restr_after" || "$other_before" != "$other_after" ]]; then + echo "FAIL $repo — restrictions/lock_branch/allow_fork_syncing would change ($restr_before/$other_before -> $restr_after/$other_after); refusing" + failed=$((failed+1)); continue + fi + if [[ "$DRY_RUN" == "true" ]]; then printf 'PLAN %-32s checks: [%s] -> [%s] reviews preserved: %s\n' "$repo" "$had" "$want" "$rev_after" ok=$((ok+1)); continue