From 12474e4414c606a5ff648609d93de1add99a1064 Mon Sep 17 00:00:00 2001 From: Zoey Rose Date: Sun, 18 Oct 2026 03:01:42 -0500 Subject: [PATCH 1/4] feat(rulesets): pilot a merge queue on playtester Add a merge_queue inventory to config/repositories.json and a repository-owned "08 - Merge queue" ruleset template. GitHub accepts the merge_queue rule only in repository rulesets, so both policy scopes install it on each listed repository, like the CodeQL gate. The template squash-merges with ALLGREEN grouping, builds up to 5 entries, merges groups of 1 to 5 and gives required checks 60 minutes to report. A queued repository's required CI no longer requires an up-to-date branch, because the queue tests each merged result. The publisher installs the queue before relaxing that rule. On removal it restores the strict policy first, then deletes only an unmodified repository-owned queue. It fails before any write if a listed active repository is private on Team, and skips an inactive one as it does security gates. Validation keeps queued repositories in required CI and out of the Dependency review gate. It also bounds the queue parameters to GitHub's API ranges and the organization's merge method. The audit expects the exact repository-owned queue and non-strict required CI on queued repositories, and reports a queue retained anywhere else. playtester is the pilot. Apply only after https://github.com/atrinik/playtester/pull/66 merges, so both required checks report on merge_group. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 8 + CONTRIBUTING.md | 14 +- README.md | 56 +++++++ bin/audit | 47 +++++- bin/publish | 122 +++++++++++++- bin/validate | 56 +++++++ config/repositories.json | 3 + config/rulesets/merge-queue.json | 34 ++++ tests/audit-fixtures.py | 13 +- tests/audit.sh | 28 +++- tests/publish-merge-queue.sh | 275 +++++++++++++++++++++++++++++++ 11 files changed, 644 insertions(+), 12 deletions(-) create mode 100644 config/rulesets/merge-queue.json create mode 100755 tests/publish-merge-queue.sh diff --git a/AGENTS.md b/AGENTS.md index 351c686..5e8b77a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,6 +63,14 @@ six ordered public pins with stable repository IDs in `config/manual-settings.json`, verify them read-only through GraphQL, and use the documented organization-owner UI step instead of an undocumented writer. +- `merge_queue` in `config/repositories.json` is the exhaustive merge-queue + inventory; `playtester` is the pilot. GitHub accepts the rule only in + repository rulesets, so `08 - Merge queue` stays repository-owned in both + scopes. Every queued repository is in `required_ci`, has every required check + reporting on `merge_group` before it is listed, never requires Dependency + review, and drops the up-to-date requirement from required CI. Install the + queue before relaxing required CI, restore the strict policy before + retiring a queue, and retire only an unmodified repository-owned rule. - Keep required workflow job names synchronized with rulesets. Workflow or permissions changes also require actionlint, least-privilege review, and immutable action references according to policy. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 45e045a..3b95a25 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -113,7 +113,19 @@ replace its default, or re-attach repositories, because attachment starts an asynchronous enablement event. Do not require a new workflow context until that exact aggregate context has -landed and passed on the protected branch. In particular, add Classic's +landed and passed on the protected branch. + +Add a repository to the `merge_queue` inventory only after every one of its +required checks reports on the `merge_group` event on its default branch. Keep +`08 - Merge queue` repository-owned, because GitHub rejects the rule in +organization rulesets. Keep its parameters within GitHub's API ranges, and use +the organization's only merge method. Queued repositories drop the up-to-date +requirement from required CI and never require Dependency review. Merge-queue +changes need publisher tests for creation, a converged rerun, drift +convergence, retained foreign or modified rules, and install-before-relax and +restore-before-retire ordering. They also need audit scenarios for a missing +queue, parameter drift, organization ownership, strict drift, and a retained +queue. In particular, add Classic's `CodeQL validation` requirement only after its advanced workflow is merged; otherwise the required check would prevent the workflow pull request itself from merging. diff --git a/README.md b/README.md index e00fa6c..912f6fe 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,10 @@ read-only and are skipped on later runs. and are not independent merge gates. The `classic` monorepo requires its stable aggregate `Classic validation` and `CodeQL validation` jobs plus `Conventional PR title`. +- Repositories in the `merge_queue` inventory merge through GitHub's merge + queue, and their required CI no longer requires an up-to-date branch because + the queue tests each merged result. `playtester` is the pilot. See + [Enabling a merge queue](#enabling-a-merge-queue). - Declared maintenance branches reject deletion and non-fast-forward updates, require linear history and pull requests, and require only checks already emitted for that branch. @@ -942,6 +946,58 @@ confirm that remaining pull requests and branches have been preserved at their new location. For `atrinik/classic`, also complete any intentional tag rebuild before applying immutable release-tag policy. +## Enabling a merge queue + +`merge_queue` in `config/repositories.json` is the exhaustive list of +repositories that merge through GitHub's merge queue. GitHub accepts the +`merge_queue` rule only in repository rulesets, so both policy scopes install a +repository-owned `08 - Merge queue` ruleset on each listed repository's default +branch, as they do for `06 - CodeQL security`. The rule comes from +`config/rulesets/merge-queue.json`: + +- squash merges, the organization's only merge method; +- `ALLGREEN` grouping, so every queued pull request's own merge result must + pass the required checks, not only the head of its group; +- a build concurrency of 5, groups of 1 to 5 pull requests, and a 5-minute + wait for the minimum group size; +- a 60-minute timeout for required checks to report on a merge group. + +Organization owners keep the usual pull-request-only bypass. During an +advisory merge window, the queue rule gets `always` bypass like that +repository's required CI. + +A queued repository's `03 - Required CI` rule sets +`strict_required_status_checks_policy` to `false`. The queue already tests each +pull request merged onto the latest base, so the branch itself no longer has +to be current. Every other repository keeps the strict policy. Validation +requires each listed repository to be in `required_ci`, and keeps the +template's parameters within GitHub's API ranges and the allowed merge method. +GitHub Team offers merge queue only for public repositories, so the publisher +fails before any write if a listed active repository is private. Like the +security gates, it skips a listed repository that is not active. + +Before adding a repository: + +1. Merge a change in that repository so every required check also runs on the + `merge_group` event and reports under its existing name. A required check + that never reports on a merge group stalls the queue until the timeout. + Dependency review stays advisory, has no merge-group trigger, and must never + be a required gate on a queued repository; validation enforces that. +2. Add the repository to `merge_queue`, merge the reviewed change, and review + the `bin/publish` plan. It should create `08 - Merge queue` on that + repository. The publisher re-PUTs every managed ruleset on each run, and + that repository's `03 - Required CI` payload should be the only one whose + content changes. Then apply the plan with the usual explicit + authorization. + +The publisher installs the queue before it relaxes required CI. When a +repository is removed from the inventory, it restores the strict policy first +and only then deletes the managed queue. It deletes only an unmodified, +repository-owned `08 - Merge queue` rule. A modified or foreign rule with that +name is retained with a `SKIP`, and `bin/audit` reports it until someone +reviews it. The audit also requires each listed repository's exact queue rule, +repository ownership, and non-strict required CI. + ## Protecting and retiring a maintenance branch Add a unique repository/branch entry to `maintenance_branches` in diff --git a/bin/audit b/bin/audit index 657a0fd..8aa9e3c 100755 --- a/bin/audit +++ b/bin/audit @@ -152,6 +152,13 @@ has_required_ci() { '.required_ci | has($repository)' "${repositories_config}" >/dev/null } +has_merge_queue() { + local repository=$1 + jq -e --arg repository "${repository}" \ + '.merge_queue | index($repository) != null' "${repositories_config}" \ + >/dev/null +} + has_release_tags() { local repository=$1 jq -e --arg repository "${repository}" \ @@ -297,6 +304,10 @@ check_ruleset() { record_finding "managed security ruleset must be repository-owned '${name}': ${organization}/${repository}" return fi + if [[ ${name} == '08 - Merge queue' && ${source} != Repository ]]; then + record_finding "managed merge-queue ruleset must be repository-owned: ${organization}/${repository}" + return + fi case ${source} in Organization) endpoint="orgs/${organization}/rulesets/${id}" ;; Repository) endpoint="repos/${organization}/${repository}/rulesets/${id}" ;; @@ -321,9 +332,12 @@ check_ruleset() { '.rules[0].parameters.required_status_checks = $checks' <<<"${wanted}") # Dependency review is governed by its separate rollout ruleset. wanted=$(jq '.rules[0].parameters.required_status_checks |= map(select(.context != "Dependency review"))' <<<"${wanted}") + wanted=$(jq --argjson strict "${expected_strict}" \ + '.rules[0].parameters.strict_required_status_checks_policy = $strict' <<<"${wanted}") fi if jq -e --arg repository "${repository}" '.repositories | index($repository) != null' \ - "${advisory_windows}" >/dev/null && [[ ${name} == '03 - Required CI - '* ]]; then + "${advisory_windows}" >/dev/null && + [[ ${name} == '03 - Required CI - '* || ${name} == '08 - Merge queue' ]]; then wanted=$(jq '.bypass_actors[].bypass_mode = "always"' <<<"${wanted}") fi if [[ ${source} == Organization ]]; then @@ -545,6 +559,12 @@ for repository in "${!expected_active[@]}"; do record_finding "unexpected retained security ruleset '${security_name}' requires manual review: ${organization}/${repository}" fi done + # A retained queue still changes how the branch merges. Report it for review; + # the publisher alone retires recognized managed queues. + if ! has_merge_queue "${repository}" && + jq -e 'any(.[]; .name == "08 - Merge queue")' <<<"${rulesets}" >/dev/null; then + record_finding "unexpected retained merge-queue ruleset requires manual review: ${organization}/${repository}" + fi branch=$(jq -r '.default_branch' "${repository_defaults}") audit_legacy_protection "${repository}" "${branch}" "${metadata_file}" while IFS= read -r maintenance; do @@ -572,6 +592,11 @@ for repository in "${!expected_active[@]}"; do fi expected_checks=$(jq -c --arg repository "${repository}" \ '(.required_ci[$repository] // []) | map({context: ., integration_id: 15368})' "${repositories_config}") + # The merge queue tests the merged result instead of requiring a current branch. + expected_strict=true + if has_merge_queue "${repository}"; then + expected_strict=false + fi dependency_required=false if jq -e --arg repository "${repository}" '.dependency_review | index($repository) != null' \ "${root}/config/security-merge-protection.json" >/dev/null; then @@ -600,17 +625,18 @@ for repository in "${!expected_active[@]}"; do "${root}/config/rulesets/dependency-review.json" "${rulesets}" "${metadata_file}" fi fi - if ! jq -e --argjson checks "${expected_checks}" ' + if ! jq -e --argjson checks "${expected_checks}" --argjson strict "${expected_strict}" ' ([.[] | select(.type == "required_status_checks") | .parameters.required_status_checks[]] | unique_by(.context, .integration_id) | sort_by(.context, .integration_id)) == ($checks | sort_by(.context, .integration_id)) and - all(.[] | select(.type == "required_status_checks"); .parameters.strict_required_status_checks_policy == true) + all(.[] | select(.type == "required_status_checks"); .parameters.strict_required_status_checks_policy == $strict) ' <<<"${effective}" >/dev/null; then record_finding "effective required CI contexts or app drift: ${organization}/${repository}" fi - desired_effective=$(jq --argjson checks "${expected_checks}" ' + desired_effective=$(jq --argjson checks "${expected_checks}" --argjson strict "${expected_strict}" ' if ($checks | length) == 0 then [] - else .rules | map(.parameters.required_status_checks = $checks) end + else .rules | map(.parameters.required_status_checks = $checks | + .parameters.strict_required_status_checks_policy = $strict) end ' "${root}/config/rulesets/required-ci.json") for policy in default-branch-integrity default-branch-linear-history; do template="${root}/config/rulesets/${policy}.json" @@ -645,6 +671,17 @@ for repository in "${!expected_active[@]}"; do check_ruleset "${repository}" "03 - Required CI - ${repository}" \ "${root}/config/rulesets/required-ci.json" "${rulesets}" "${metadata_file}" fi + if has_merge_queue "${repository}"; then + check_ruleset "${repository}" '08 - Merge queue' \ + "${root}/config/rulesets/merge-queue.json" "${rulesets}" "${metadata_file}" + desired_effective=$(jq --argjson prior "${desired_effective}" '$prior + .rules' \ + "${root}/config/rulesets/merge-queue.json") + if ! jq -e --slurpfile desired "${root}/config/rulesets/merge-queue.json" \ + '[.[] | select(.type == "merge_queue") | {type, parameters}] == $desired[0].rules' \ + <<<"${effective}" >/dev/null; then + record_finding "effective merge queue drift: ${organization}/${repository}" + fi + fi # Parent listings can contain correctly scoped but nonapplicable rulesets. # Inspect the exact managed name on nonrelease repositories too, so widening # its complete scope fails while foreign, unmanaged tag rules remain unowned. diff --git a/bin/publish b/bin/publish index 36ae768..a505e6b 100755 --- a/bin/publish +++ b/bin/publish @@ -703,7 +703,10 @@ publish_repository_rulesets() { ) | .rules[0].parameters.required_status_checks = ($repositories[0].required_ci[$repository] | - map({context: ., integration_id: $app_id})) + map({context: ., integration_id: $app_id})) | + # A merge queue tests the merged result, so its branch need not be current. + .rules[0].parameters.strict_required_status_checks_policy = + (($repositories[0].merge_queue | index($repository)) == null) ' "${root}/config/rulesets/required-ci.json" >"${ci_payload}" upsert_repository_ruleset "${repository}" "${ci_payload}" || return 1 done < <(jq -r '.required_ci | keys[]' "${repositories_config}") @@ -830,7 +833,10 @@ publish_organization_rulesets() { } | .rules[0].parameters.required_status_checks = ($repositories[0].required_ci[$repository] | - map({context: ., integration_id: $app_id})) + map({context: ., integration_id: $app_id})) | + # A merge queue tests the merged result, so its branch need not be current. + .rules[0].parameters.strict_required_status_checks_policy = + (($repositories[0].merge_queue | index($repository)) == null) ' "${root}/config/rulesets/required-ci.json" >"${ci_payload}" upsert_organization_ruleset "${ci_payload}" || return 1 done < <(jq -r '.required_ci | keys[]' "${repositories_config}") @@ -1371,6 +1377,101 @@ publish_security_merge_rulesets() { done } +repository_uses_merge_queue() { + local repository=$1 + + jq -e --arg repository "${repository}" \ + '(.merge_queue | index($repository)) != null' \ + "${repositories_config}" >/dev/null +} + +make_merge_queue_ruleset() { + local repository=$1 + local output + + output=$(mktemp) + temporary_files+=("${output}") + # An advisory-level merge needs every rule reported for the branch to be + # bypassable, exactly as for required CI. + jq --arg repository "${repository}" \ + --slurpfile advisory_windows "${advisory_merge_windows_config}" ' + .bypass_actors |= map( + if ($advisory_windows[0].repositories | index($repository)) != null then + .bypass_mode = "always" + else + . + end + ) + ' "${root}/config/rulesets/merge-queue.json" >"${output}" + echo "${output}" +} + +# GitHub supports merge-queue rules only in repository rulesets, so this rule +# is repository-owned in both policy scopes. It is published before required +# CI stops requiring an up-to-date branch for the same repository. +publish_merge_queue_rulesets() { + local repository template name existing id current + + name=$(jq -r '.name' "${root}/config/rulesets/merge-queue.json") + while IFS= read -r repository; do + if [[ ! -v active_repository_ids["${repository}"] ]]; then + echo "SKIP ${organization}/${repository} is not active; ${name}" + continue + fi + template=$(make_merge_queue_ruleset "${repository}") + temporary_files+=("${template}") + existing=$(list_rulesets \ + "repos/${organization}/${repository}/rulesets?includes_parents=false") || return 1 + id=$(jq -r --arg name "${name}" \ + '.[] | select(.name == $name) | .id' <<<"${existing}") + if [[ -z ${id} ]]; then + run_api POST "repos/${organization}/${repository}/rulesets" "${template}" || return 1 + continue + fi + current=$(github_api "repos/${organization}/${repository}/rulesets/${id}") || return 1 + if ! jq -e --argjson id "${id}" --arg name "${name}" \ + --arg source "${organization}/${repository}" ' + .id == $id and .name == $name and + .source_type == "Repository" and .source == $source + ' <<<"${current}" >/dev/null; then + echo "error: merge-queue ruleset ownership changed; retained: ${repository} ${name}" >&2 + return 1 + fi + if security_ruleset_matches "${template}" "${current}"; then + echo "KEEP /repos/${organization}/${repository}/rulesets/${id} ${name}" + else + run_api PUT "repos/${organization}/${repository}/rulesets/${id}" \ + "${template}" || return 1 + fi + done < <(jq -r '.merge_queue[]' "${repositories_config}") +} + +# Retire a queue only after required CI again requires an up-to-date branch. +# Only an unmodified repository-owned managed rule is removed. +delete_removed_merge_queue_rulesets() { + local repository template name existing id current + + name=$(jq -r '.name' "${root}/config/rulesets/merge-queue.json") + for repository in "${active_repositories[@]}"; do + if repository_uses_merge_queue "${repository}"; then + continue + fi + existing=$(list_rulesets \ + "repos/${organization}/${repository}/rulesets?includes_parents=false") || return 1 + id=$(jq -r --arg name "${name}" \ + '.[] | select(.name == $name) | .id' <<<"${existing}") + [[ -n ${id} ]] || continue + template=$(make_merge_queue_ruleset "${repository}") + temporary_files+=("${template}") + current=$(github_api "repos/${organization}/${repository}/rulesets/${id}") || return 1 + if owned_security_ruleset_matches "${repository}" "${id}" "${template}" "${current}"; then + run_api DELETE "repos/${organization}/${repository}/rulesets/${id}" || return 1 + else + echo "SKIP ${organization}/${repository} ${name}: unrecognized ownership or payload; retained" + fi + done +} + validate_sha_canary_target() { local canary=$1 owner repository number metadata IFS=/ read -r owner repository number <<<"${canary}" @@ -1836,6 +1937,7 @@ ${apply} && echo "Mode: apply" || echo "Mode: plan" declare -a active_repositories=() declare -A active_repository_ids=() +declare -A active_repository_visibility=() while IFS=$'\t' read -r \ repository id private visibility disabled fork has_issues has_projects has_wiki; do if [[ ! ${id} =~ ^[0-9]+$ ]]; then @@ -1850,6 +1952,7 @@ while IFS=$'\t' read -r \ fi active_repositories+=("${repository}") active_repository_ids["${repository}"]=${id} + active_repository_visibility["${repository}"]=${visibility} done < <( github_api --paginate "orgs/${organization}/repos?per_page=100&type=all" \ --jq ' @@ -1891,6 +1994,18 @@ while IFS= read -r repository; do fi done < <(jq -r '.repositories[]' "${codeql_advanced_config}") +# Team offers merge queue only for public repositories. Fail before any write +# rather than relaxing required CI for a repository that cannot use a queue. +# Inactive entries are skipped by the queue publisher, like security gates. +while IFS= read -r repository; do + [[ -v active_repository_ids["${repository}"] ]] || continue + if [[ ${active_repository_visibility["${repository}"]} != public && + ${organization_plan} != enterprise ]]; then + echo "error: merge queue requires a public repository on the ${organization_plan} plan: ${repository}" >&2 + exit 1 + fi +done < <(jq -r '.merge_queue[]' "${repositories_config}") + while IFS=$'\t' read -r repository expected_id; do if [[ ! -v active_repository_ids["${repository}"] ]]; then echo "error: immutable-release inventory repository is not active: ${repository}" >&2 @@ -1922,6 +2037,8 @@ done converge_repository_workflow_permissions +publish_merge_queue_rulesets + if [[ ${policy_scope} == organization ]]; then publish_organization_rulesets publish_organization_security @@ -1931,6 +2048,7 @@ else fi publish_security_merge_rulesets +delete_removed_merge_queue_rulesets while IFS= read -r repository; do if ! repository_archived=$( diff --git a/bin/validate b/bin/validate index 243140e..46bdf21 100755 --- a/bin/validate +++ b/bin/validate @@ -60,6 +60,9 @@ jq -e --slurpfile repositories "${repositories_config}" \ ($policy.activation[$kind] | type == "object" and keys == $policy[$kind]) and all($policy.activation[$kind][]; IN("pending", "enforced"))) and .dependency_review == [] and + # Dependency review never reports on merge groups; a queued gate would stall. + all(.dependency_review[]; . as $repository | + ($repositories[0].merge_queue | index($repository)) == null) and .code_scanning == ($repositories[0].required_ci | keys) and (.codeql_required_languages | type == "object") and all(.codeql_required_languages | to_entries[]; @@ -149,6 +152,54 @@ jq -e ' [.rules[].type] == ["pull_request"] ' "${root}/config/rulesets/pull-request-gate-classic.json" >/dev/null +# The publisher relaxes the up-to-date requirement only for merge-queue +# repositories, whose queue tests the merged result instead. +jq -e ' + .name == "03 - Required CI" and + .target == "branch" and .enforcement == "active" and + .conditions == {ref_name: {exclude: [], include: ["~DEFAULT_BRANCH"]}} and + .bypass_actors == [{actor_id: 1, actor_type: "OrganizationAdmin", + bypass_mode: "pull_request"}] and + .rules == [{type: "required_status_checks", parameters: { + required_status_checks: [], strict_required_status_checks_policy: true + }}] +' "${root}/config/rulesets/required-ci.json" >/dev/null + +# GitHub accepts merge-queue rules only in repository rulesets. Keep the queue +# within the API ranges, merging with the organization's only merge method, and +# able to build and merge at least one entry. +jq -e --slurpfile defaults "${root}/config/repository-defaults.json" ' + def integer_between($low; $high): + type == "number" and floor == . and . >= $low and . <= $high; + .name == "08 - Merge queue" and + .target == "branch" and .enforcement == "active" and + .conditions == {ref_name: {exclude: [], include: ["~DEFAULT_BRANCH"]}} and + .bypass_actors == [{actor_id: 1, actor_type: "OrganizationAdmin", + bypass_mode: "pull_request"}] and + (.rules | type == "array" and length == 1) and + (.rules[0] | keys == ["parameters", "type"] and .type == "merge_queue") and + (.rules[0].parameters | + keys == [ + "check_response_timeout_minutes", + "grouping_strategy", + "max_entries_to_build", + "max_entries_to_merge", + "merge_method", + "min_entries_to_merge", + "min_entries_to_merge_wait_minutes" + ] and + (.check_response_timeout_minutes | integer_between(1; 360)) and + (.grouping_strategy | IN("ALLGREEN", "HEADGREEN")) and + (.max_entries_to_build | integer_between(1; 100)) and + (.max_entries_to_merge | integer_between(1; 100)) and + (.min_entries_to_merge | integer_between(1; 100)) and + .min_entries_to_merge <= .max_entries_to_merge and + (.min_entries_to_merge_wait_minutes | integer_between(0; 360)) and + ({MERGE: "allow_merge_commit", SQUASH: "allow_squash_merge", + REBASE: "allow_rebase_merge"}[.merge_method] as $setting | + $setting != null and $defaults[0][$setting] == true)) +' "${root}/config/rulesets/merge-queue.json" >/dev/null + jq -e \ --slurpfile repositories "${repositories_config}" ' (keys == ["repositories"]) and @@ -176,6 +227,11 @@ jq -e ' (.key as $repository | ($config.pull_request_gate | index($repository)) != null) ) and + # A queue runs the required checks on each merge result, so it needs them. + (.merge_queue | type == "array") and + ((.merge_queue | length) == (.merge_queue | unique | length)) and + all(.merge_queue[]; type == "string" and length > 0 and + (. as $repository | $config.required_ci | has($repository))) and (.release_tags | type == "array") and ((.release_tags | length) == (.release_tags | unique | length)) and all(.release_tags[]; type == "string" and length > 0) and diff --git a/config/repositories.json b/config/repositories.json index bdf33f8..5fb7d68 100644 --- a/config/repositories.json +++ b/config/repositories.json @@ -105,6 +105,9 @@ "Conventional PR title" ] }, + "merge_queue": [ + "playtester" + ], "maintenance_branches": [], "release_tags": [ "atrinik", diff --git a/config/rulesets/merge-queue.json b/config/rulesets/merge-queue.json new file mode 100644 index 0000000..69f6968 --- /dev/null +++ b/config/rulesets/merge-queue.json @@ -0,0 +1,34 @@ +{ + "name": "08 - Merge queue", + "target": "branch", + "enforcement": "active", + "bypass_actors": [ + { + "actor_id": 1, + "actor_type": "OrganizationAdmin", + "bypass_mode": "pull_request" + } + ], + "conditions": { + "ref_name": { + "exclude": [], + "include": [ + "~DEFAULT_BRANCH" + ] + } + }, + "rules": [ + { + "type": "merge_queue", + "parameters": { + "check_response_timeout_minutes": 60, + "grouping_strategy": "ALLGREEN", + "max_entries_to_build": 5, + "max_entries_to_merge": 5, + "merge_method": "SQUASH", + "min_entries_to_merge": 1, + "min_entries_to_merge_wait_minutes": 5 + } + } + ] +} diff --git a/tests/audit-fixtures.py b/tests/audit-fixtures.py index 6b8e82b..db3b6fd 100755 --- a/tests/audit-fixtures.py +++ b/tests/audit-fixtures.py @@ -78,6 +78,10 @@ def rulesets(repository): specs.append(('pull-request-gate', None)) if repository in repositories['required_ci']: specs.append(('required-ci', '03 - Required CI - ' + repository)) + queued = repository in repositories['merge_queue'] + if (queued and scenario != 'queue-missing') or ( + scenario == 'queue-retained' and repository == 'observatory'): + specs.append(('merge-queue', None)) if repository in repositories['release_tags'] or scenario in ( 'inherited-tag-unaffected', 'tag-extra-nonrelease', 'tag-all-nonrelease', 'tag-repository-nonrelease'): specs.append(('immutable-release-tags', None)) @@ -104,6 +108,11 @@ def rulesets(repository): item['rules'][0]['parameters']['required_status_checks'] = [ {'context': context, 'integration_id': 15368} for context in repositories['required_ci'][repository]] + # The publisher relaxes the up-to-date requirement for queued merges. + if queued and scenario != 'queue-strict': + item['rules'][0]['parameters']['strict_required_status_checks_policy'] = False + if template == 'merge-queue' and scenario == 'queue-parameter': + item['rules'][0]['parameters']['check_response_timeout_minutes'] = 30 # Actual ruleset GET responses expand omitted request parameters. for rule in item['rules']: if rule['type'] == 'pull_request': @@ -138,7 +147,9 @@ def rulesets(repository): rule.get('parameters', {}).pop(key, None) item.update(id=active.index(repository) * 10 + index, source_type='Repository' if scenario == 'repository-rulesets' or - template in ('code-scanning', 'dependency-review') else 'Organization') + template in ('code-scanning', 'dependency-review', 'merge-queue') else 'Organization') + if scenario == 'queue-organization-owner' and template == 'merge-queue': + item['source_type'] = 'Organization' if ((scenario == 'organization-codeql-owner' and template == 'code-scanning') or (scenario == 'organization-dependency-owner' and template == 'dependency-review')): item['source_type'] = 'Organization' diff --git a/tests/audit.sh b/tests/audit.sh index c17b225..c1b0d0b 100755 --- a/tests/audit.sh +++ b/tests/audit.sh @@ -31,7 +31,7 @@ mv "${temporary}/actions.json" "${temporary}/full/config/actions-permissions.jso jq '.pull_request_gate |= ((. + [".github"]) | unique)' "${temporary}/full/config/repositories.json" \ >"${temporary}/repositories.json" mv "${temporary}/repositories.json" "${temporary}/full/config/repositories.json" -for repository in .github classic observatory content-toolkit service-updater web-platform; do +for repository in .github classic observatory content-toolkit playtester service-updater web-platform; do fixture="${temporary}/fixture-${repository}" cp -R "${temporary}/full" "${fixture}" jq --arg repository "${repository}" \ @@ -69,6 +69,8 @@ run_audit() { missing-rust) repository=content-toolkit ;; missing-tag-scope|tag-ref-drift|tag-all-scope|tag-extra-repo|tag-extra-exclude) repository=service-updater ;; unexpected-pr) repository=web-platform ;; + queue-retained) repository=observatory ;; + queue-*) repository=playtester ;; esac fixture="fixture-${repository}" fi @@ -89,8 +91,14 @@ run_audit() { if [[ ${scenario} == production-advisory ]]; then cp "${root}/config/security-merge-protection.json" "${temporary}/${fixture}/config/security-merge-protection.json" else - jq '.dependency_review = .code_scanning | .activation.dependency_review = .activation.code_scanning' \ - "${root}/config/security-merge-protection.json" >"${temporary}/${fixture}/config/security-merge-protection.json" + # Dependency review never reports on merge groups, so queued repositories + # cannot carry its required gate. + jq --slurpfile repositories "${root}/config/repositories.json" ' + $repositories[0].merge_queue as $queued | + .dependency_review = (.code_scanning - $queued) | + .activation.dependency_review = (.activation.code_scanning | + with_entries(select(.key as $repository | $queued | index($repository) | not))) + ' "${root}/config/security-merge-protection.json" >"${temporary}/${fixture}/config/security-merge-protection.json" fi case ${scenario} in retired-*) @@ -265,6 +273,20 @@ done for scenario in approval-count thread-resolution; do assert_drift "${scenario}" 'effective pull-request policy drift' done + +# A queued repository needs its repository-owned queue, the exact queue +# parameters, and required CI that no longer demands a current branch. +output=$(run_audit queue-pass) +grep -Fq 'Governance audit passed:' <<<"${output}" +assert_drift queue-missing "missing or duplicate inherited ruleset '08 - Merge queue': atrinik/playtester" +assert_drift queue-missing 'effective merge queue drift: atrinik/playtester' +assert_drift queue-parameter "ruleset scope or policy drift '08 - Merge queue': atrinik/playtester" +assert_drift queue-parameter 'effective merge queue drift: atrinik/playtester' +assert_drift queue-organization-owner 'managed merge-queue ruleset must be repository-owned: atrinik/playtester' +assert_drift queue-strict 'effective required CI contexts or app drift: atrinik/playtester' +assert_drift queue-strict "ruleset scope or policy drift '03 - Required CI - playtester'" +assert_drift queue-retained 'unexpected retained merge-queue ruleset requires manual review: atrinik/observatory' +assert_drift queue-retained 'effective branch policy drift requires manual review: atrinik/observatory/main' assert_drift effective-integrity 'effective default-branch integrity drift' assert_drift unexpected-pr 'effective pull-request policy drift: atrinik/web-platform' assert_drift unexpected-codeql 'unexpected effective CodeQL merge protection: atrinik/.github' diff --git a/tests/publish-merge-queue.sh b/tests/publish-merge-queue.sh new file mode 100755 index 0000000..63feac3 --- /dev/null +++ b/tests/publish-merge-queue.sh @@ -0,0 +1,275 @@ +#!/usr/bin/env bash + +# Production functions consume these fixture globals. +# shellcheck disable=SC2034 +set -euo pipefail +root=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +source_root=${ATRINIK_PUBLISH_TEST_ROOT:-${root}} +temporary=$(mktemp -d) +trap 'rm -rf "${temporary}"' EXIT +sed -n '/^converge_organization_settings() {/,/^organization_metadata=/p' \ + "${source_root}/bin/publish" | sed '$d' >"${temporary}/functions.sh" +sed -n '/^list_rulesets() {/,/^}/p' "${source_root}/bin/publish" >>"${temporary}/functions.sh" +# shellcheck source=/dev/null +source "${temporary}/functions.sh" +root=${temporary}/fixture +mkdir -p "${root}/config" "${temporary}/captured" +cp -R "${source_root}/config/rulesets" "${root}/config/rulesets" +template=${root}/config/rulesets/merge-queue.json +organization=atrinik +organization_plan=team +repositories_config=${root}/config/repositories.json +advisory_merge_windows_config=${root}/config/advisory-merge-windows.json +temporary_files=() +log=${temporary}/api.jsonl + +# Live fixtures: playtester is queued, observatory is not. +github_api() { + jq -cn --arg endpoint "$1" '{method:"GET",endpoint:$endpoint}' >>"${log}" + case $1 in + 'repos/atrinik/playtester/rulesets?includes_parents=false' | \ + 'repos/atrinik/observatory/rulesets?includes_parents=false') + local repository=${1#repos/atrinik/} + repository=${repository%%/*} + cat "${temporary}/${repository}-list.json" ;; + repos/atrinik/playtester/rulesets/8 | repos/atrinik/observatory/rulesets/8) + local repository=${1#repos/atrinik/} + cat "${temporary}/${repository%%/*}-rule.json" ;; + orgs/atrinik/rulesets) echo '[]' ;; + *) echo "unexpected endpoint: $1" >&2; return 99 ;; + esac +} +run_api() { + if ! ${apply}; then + echo "PLAN $1 /$2" + return + fi + jq -cn --arg method "$1" --arg endpoint "$2" \ + --argjson body "$(if [[ -n ${3:-} ]]; then cat "$3"; else echo null; fi)" \ + '{method:$method,endpoint:$endpoint,body:$body}' >>"${log}" +} +writes() { + jq -sc '[.[] | select(.method != "GET")]' "${log}" +} +# GitHub returns the owner sentinel with a null ID and the ruleset's identity. +live_rule() { + local repository=$1 + jq --arg source "atrinik/${repository}" ' + . + {id: 8, source_type: "Repository", source: $source} | + .bypass_actors[0].actor_id = null + ' "${template}" +} +reset_case() { + : >"${log}" + apply=true + active_repositories=(observatory playtester) + declare -gA active_repository_ids=([observatory]=1 [playtester]=2) + printf '{"repositories":[]}\n' >"${advisory_merge_windows_config}" + jq -n '{ + pull_request_gate: ["observatory", "playtester"], + required_ci: { + observatory: ["Validate dashboard and Workers", "Conventional PR title"], + playtester: ["Playtester validation", "Conventional PR title"] + }, + merge_queue: ["playtester"], + release_tags: [], + maintenance_branches: [], + archive: [] + }' >"${repositories_config}" + printf '[{"id":3,"name":"03 - Required CI"}]\n' >"${temporary}/playtester-list.json" + printf '[{"id":3,"name":"03 - Required CI"}]\n' >"${temporary}/observatory-list.json" + live_rule playtester >"${temporary}/playtester-rule.json" + live_rule observatory >"${temporary}/observatory-rule.json" +} +expect_failure() { + local message=$1 + shift + if "$@" >"${temporary}/output" 2>&1; then + echo "expected failure: ${message}" >&2 + exit 1 + fi + grep -Fq "${message}" "${temporary}/output" + [[ $(writes) == '[]' ]] +} + +# The pilot template is the reviewed queue for squash-only repositories. +jq -e ' + .name == "08 - Merge queue" and + .rules == [{type: "merge_queue", parameters: { + check_response_timeout_minutes: 60, + grouping_strategy: "ALLGREEN", + max_entries_to_build: 5, + max_entries_to_merge: 5, + merge_method: "SQUASH", + min_entries_to_merge: 1, + min_entries_to_merge_wait_minutes: 5 + }}] +' "${template}" >/dev/null + +# A missing queue is created on the repository, never as an organization rule. +reset_case +publish_merge_queue_rulesets >"${temporary}/output" +jq -se --slurpfile desired "${template}" ' + [.[] | select(.method != "GET")] == + [{method: "POST", endpoint: "repos/atrinik/playtester/rulesets", body: $desired[0]}] +' "${log}" >/dev/null +if grep -q '"endpoint":"orgs/' "${log}"; then + echo 'merge queue must remain repository-owned' >&2 + exit 1 +fi + +# A converged queue is kept without a write. +reset_case +printf '[{"id":3,"name":"03 - Required CI"},{"id":8,"name":"08 - Merge queue"}]\n' \ + >"${temporary}/playtester-list.json" +publish_merge_queue_rulesets >"${temporary}/output" +[[ $(writes) == '[]' ]] +grep -Fxq 'KEEP /repos/atrinik/playtester/rulesets/8 08 - Merge queue' "${temporary}/output" + +# Parameter, enforcement or scope drift is converged in place. +for filter in \ + '.rules[0].parameters.check_response_timeout_minutes = 30' \ + '.rules[0].parameters.grouping_strategy = "HEADGREEN"' \ + '.rules[0].parameters.merge_method = "MERGE"' \ + '.enforcement = "disabled"' \ + '.conditions.ref_name.include = ["refs/heads/other"]' \ + '.bypass_actors[0].bypass_mode = "always"'; do + reset_case + printf '[{"id":8,"name":"08 - Merge queue"}]\n' >"${temporary}/playtester-list.json" + jq "${filter}" "${temporary}/playtester-rule.json" >"${temporary}/changed.json" + mv "${temporary}/changed.json" "${temporary}/playtester-rule.json" + publish_merge_queue_rulesets >"${temporary}/output" + jq -se --slurpfile desired "${template}" ' + [.[] | select(.method != "GET")] == + [{method: "PUT", endpoint: "repos/atrinik/playtester/rulesets/8", body: $desired[0]}] + ' "${log}" >/dev/null +done + +# Plan mode prints the operation and writes nothing. +reset_case +apply=false +publish_merge_queue_rulesets >"${temporary}/output" +grep -Fxq 'PLAN POST /repos/atrinik/playtester/rulesets' "${temporary}/output" +[[ $(writes) == '[]' ]] + +# A same-name rule with foreign ownership is never overwritten. +for filter in '.source_type = "Organization"' '.source = "foreign/playtester"' \ + '.id = 9' '.name = "Foreign queue"'; do + reset_case + printf '[{"id":8,"name":"08 - Merge queue"}]\n' >"${temporary}/playtester-list.json" + jq "${filter}" "${temporary}/playtester-rule.json" >"${temporary}/changed.json" + mv "${temporary}/changed.json" "${temporary}/playtester-rule.json" + expect_failure 'merge-queue ruleset ownership changed; retained: playtester' \ + publish_merge_queue_rulesets +done + +# An inactive inventory entry is skipped, as security gates are; an ambiguous +# live inventory fails closed. +reset_case +active_repository_ids=([observatory]=1) +publish_merge_queue_rulesets >"${temporary}/output" +grep -Fxq 'SKIP atrinik/playtester is not active; 08 - Merge queue' "${temporary}/output" +jq -se 'length == 0' "${log}" >/dev/null +reset_case +printf '[{"id":8,"name":"08 - Merge queue"},{"id":9,"name":"08 - Merge queue"}]\n' \ + >"${temporary}/playtester-list.json" +expect_failure 'ambiguous complete ruleset inventory' publish_merge_queue_rulesets + +# An advisory-level merge needs every reported rule bypassable. +reset_case +printf '{"repositories":["playtester"]}\n' >"${advisory_merge_windows_config}" +publish_merge_queue_rulesets >"${temporary}/output" +jq -se ' + [.[] | select(.method == "POST")][0].body.bypass_actors == + [{actor_id: 1, actor_type: "OrganizationAdmin", bypass_mode: "always"}] +' "${log}" >/dev/null + +# Retirement removes only the unmodified repository-owned managed queue. +reset_case +jq '.merge_queue = []' "${repositories_config}" >"${temporary}/retired.json" +mv "${temporary}/retired.json" "${repositories_config}" +printf '[{"id":8,"name":"08 - Merge queue"}]\n' >"${temporary}/playtester-list.json" +delete_removed_merge_queue_rulesets >"${temporary}/output" +[[ $(writes) == '[{"method":"DELETE","endpoint":"repos/atrinik/playtester/rulesets/8","body":null}]' ]] +for filter in '.source_type = "Organization"' '.source = "foreign/playtester"' \ + '.id = 9' '.enforcement = "disabled"' \ + '.rules[0].parameters.check_response_timeout_minutes = 30' \ + '.rules += [{type: "deletion"}]'; do + reset_case + jq '.merge_queue = []' "${repositories_config}" >"${temporary}/retired.json" + mv "${temporary}/retired.json" "${repositories_config}" + printf '[{"id":8,"name":"08 - Merge queue"}]\n' >"${temporary}/playtester-list.json" + jq "${filter}" "${temporary}/playtester-rule.json" >"${temporary}/changed.json" + mv "${temporary}/changed.json" "${temporary}/playtester-rule.json" + delete_removed_merge_queue_rulesets >"${temporary}/output" + [[ $(writes) == '[]' ]] + grep -Fxq 'SKIP atrinik/playtester 08 - Merge queue: unrecognized ownership or payload; retained' \ + "${temporary}/output" +done +# Queued repositories and repositories without the rule are left alone. +reset_case +printf '[{"id":8,"name":"08 - Merge queue"}]\n' >"${temporary}/playtester-list.json" +delete_removed_merge_queue_rulesets >"${temporary}/output" +[[ $(writes) == '[]' ]] +if grep -Fq 'repos/atrinik/playtester/' "${log}"; then + echo 'retirement must not inspect a queued repository' >&2 + exit 1 +fi + +# Required CI stops requiring a current branch only where the queue tests the +# merged result, in both policy scopes. +upsert_repository_ruleset() { + cp "$2" "${temporary}/captured/repository-$1-$(jq -r .name "$2")" +} +upsert_organization_ruleset() { + cp "$1" "${temporary}/captured/organization-$(jq -r .name "$1")" +} +delete_retired_repository_required_ci() { :; } +delete_stale_repository_maintenance_rulesets() { :; } +delete_stale_repository_advisory_rulesets() { :; } +delete_managed_organization_rulesets() { :; } +delete_managed_repository_rulesets() { :; } +reset_case +publish_repository_rulesets >/dev/null +publish_organization_rulesets >/dev/null +for scope in repository-playtester-03\ -\ Required\ CI organization-03\ -\ Required\ CI\ -\ playtester; do + jq -e ' + .rules[0].parameters.strict_required_status_checks_policy == false and + .rules[0].parameters.required_status_checks == + [{context: "Playtester validation", integration_id: 15368}, + {context: "Conventional PR title", integration_id: 15368}] + ' "${temporary}/captured/${scope}" >/dev/null +done +for scope in repository-observatory-03\ -\ Required\ CI organization-03\ -\ Required\ CI\ -\ observatory; do + jq -e '.rules[0].parameters.strict_required_status_checks_policy == true' \ + "${temporary}/captured/${scope}" >/dev/null +done + +# Install the queue before required CI is relaxed, and retire it only after +# required CI is restored. Neither step belongs to one policy scope. +order=$(grep -nE '^(publish_merge_queue_rulesets|if \[\[ \$\{policy_scope\} == organization \]\]; then|publish_security_merge_rulesets|delete_removed_merge_queue_rulesets)$' \ + "${source_root}/bin/publish" | cut -d: -f2-) +[[ ${order} == $'publish_merge_queue_rulesets\nif [[ ${policy_scope} == organization ]]; then\npublish_security_merge_rulesets\ndelete_removed_merge_queue_rulesets' ]] + +# Team offers merge queue only for public repositories; fail before any write. +sed -n '/^# Team offers merge queue only/,/^done < <(jq -r .\.merge_queue\[\]. /p' \ + "${source_root}/bin/publish" >"${temporary}/preflight.sh" +grep -Fq 'merge queue requires a public repository' "${temporary}/preflight.sh" +run_preflight() { + ( + # shellcheck source=/dev/null + source "${temporary}/preflight.sh" + ) +} +reset_case +declare -A active_repository_visibility=([observatory]=private [playtester]=public) +run_preflight +active_repository_visibility[playtester]=private +expect_failure 'merge queue requires a public repository on the team plan: playtester' run_preflight +organization_plan=enterprise +run_preflight +organization_plan=team +active_repository_visibility[playtester]=private +active_repository_ids=([observatory]=1) +run_preflight +echo 'Merge-queue publisher tests passed.' From beb2fcecab3391f295d13eda38835368152b89f8 Mon Sep 17 00:00:00 2001 From: Zoey Rose Date: Sun, 18 Oct 2026 03:42:45 -0500 Subject: [PATCH 2/4] fix(rulesets): recheck a merge queue before retiring it The retirement path matched the live queue once and then deleted it, so an owner edit made between that read and the DELETE would be removed. Like security-gate retirement, it now checks the repository's identity and archive state, re-reads the rule and repeats the ownership and payload match immediately before deleting. A changed rule or repository is retained with a SKIP, and a failed final read stops the publisher. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 3 +- CONTRIBUTING.md | 5 ++-- README.md | 5 ++-- bin/publish | 25 +++++++++++++--- tests/publish-merge-queue.sh | 57 ++++++++++++++++++++++++++++++++++-- 5 files changed, 84 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5e8b77a..17d26b7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,7 +70,8 @@ reporting on `merge_group` before it is listed, never requires Dependency review, and drops the up-to-date requirement from required CI. Install the queue before relaxing required CI, restore the strict policy before - retiring a queue, and retire only an unmodified repository-owned rule. + retiring a queue, and retire only an unmodified repository-owned rule that + passes a fresh identity check and final re-read. - Keep required workflow job names synchronized with rulesets. Workflow or permissions changes also require actionlint, least-privilege review, and immutable action references according to policy. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3b95a25..b2bb462 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -122,8 +122,9 @@ organization rulesets. Keep its parameters within GitHub's API ranges, and use the organization's only merge method. Queued repositories drop the up-to-date requirement from required CI and never require Dependency review. Merge-queue changes need publisher tests for creation, a converged rerun, drift -convergence, retained foreign or modified rules, and install-before-relax and -restore-before-retire ordering. They also need audit scenarios for a missing +convergence, retained foreign or modified rules, the identity and final-read +fences before retirement, and install-before-relax and restore-before-retire +ordering. They also need audit scenarios for a missing queue, parameter drift, organization ownership, strict drift, and a retained queue. In particular, add Classic's `CodeQL validation` requirement only after its advanced workflow is merged; diff --git a/README.md b/README.md index 912f6fe..019f379 100644 --- a/README.md +++ b/README.md @@ -993,8 +993,9 @@ Before adding a repository: The publisher installs the queue before it relaxes required CI. When a repository is removed from the inventory, it restores the strict policy first and only then deletes the managed queue. It deletes only an unmodified, -repository-owned `08 - Merge queue` rule. A modified or foreign rule with that -name is retained with a `SKIP`, and `bin/audit` reports it until someone +repository-owned `08 - Merge queue` rule, and checks the repository's identity +and re-reads the rule immediately before the deletion. A modified, foreign or +concurrently edited rule with that name is retained with a `SKIP`, and `bin/audit` reports it until someone reviews it. The audit also requires each listed repository's exact queue rule, repository ownership, and non-strict required CI. diff --git a/bin/publish b/bin/publish index a505e6b..613f0a0 100755 --- a/bin/publish +++ b/bin/publish @@ -1449,7 +1449,7 @@ publish_merge_queue_rulesets() { # Retire a queue only after required CI again requires an up-to-date branch. # Only an unmodified repository-owned managed rule is removed. delete_removed_merge_queue_rulesets() { - local repository template name existing id current + local repository template name existing id current metadata name=$(jq -r '.name' "${root}/config/rulesets/merge-queue.json") for repository in "${active_repositories[@]}"; do @@ -1464,11 +1464,28 @@ delete_removed_merge_queue_rulesets() { template=$(make_merge_queue_ruleset "${repository}") temporary_files+=("${template}") current=$(github_api "repos/${organization}/${repository}/rulesets/${id}") || return 1 - if owned_security_ruleset_matches "${repository}" "${id}" "${template}" "${current}"; then - run_api DELETE "repos/${organization}/${repository}/rulesets/${id}" || return 1 - else + if ! owned_security_ruleset_matches "${repository}" "${id}" "${template}" "${current}"; then echo "SKIP ${organization}/${repository} ${name}: unrecognized ownership or payload; retained" + continue + fi + metadata=$(github_api "repos/${organization}/${repository}") || return 1 + if ! jq -e --argjson id "${active_repository_ids["${repository}"]}" \ + --arg full_name "${organization}/${repository}" ' + .id == $id and .full_name == $full_name and .archived == false + ' <<<"${metadata}" >/dev/null; then + echo "SKIP ${organization}/${repository} ${name}: repository identity or archive state changed; retained" + continue fi + # Re-read immediately before deletion so a concurrent edit is retained. + current=$(github_api "repos/${organization}/${repository}/rulesets/${id}") || { + echo "error: merge-queue final retirement read failed; retained: ${repository} ${name}" >&2 + return 1 + } + if ! owned_security_ruleset_matches "${repository}" "${id}" "${template}" "${current}"; then + echo "SKIP ${organization}/${repository} ${name}: rule changed during retirement preflight; retained" + continue + fi + run_api DELETE "repos/${organization}/${repository}/rulesets/${id}" || return 1 done } diff --git a/tests/publish-merge-queue.sh b/tests/publish-merge-queue.sh index 63feac3..23afb72 100755 --- a/tests/publish-merge-queue.sh +++ b/tests/publish-merge-queue.sh @@ -33,8 +33,19 @@ github_api() { repository=${repository%%/*} cat "${temporary}/${repository}-list.json" ;; repos/atrinik/playtester/rulesets/8 | repos/atrinik/observatory/rulesets/8) - local repository=${1#repos/atrinik/} - cat "${temporary}/${repository%%/*}-rule.json" ;; + local repository=${1#repos/atrinik/} reads + repository=${repository%%/*} + # A later read may observe a concurrent edit or fail. + reads=$(jq -s --arg endpoint "$1" '[.[] | select(.endpoint == $endpoint)] | length' "${log}") + if ((reads > 1)) && [[ -e ${temporary}/${repository}-later-rule.json ]]; then + cat "${temporary}/${repository}-later-rule.json" + elif ((reads > 1)) && [[ -e ${temporary}/${repository}-later-unreadable ]]; then + return 1 + else + cat "${temporary}/${repository}-rule.json" + fi ;; + repos/atrinik/playtester | repos/atrinik/observatory) + cat "${temporary}/${1#repos/atrinik/}-metadata.json" ;; orgs/atrinik/rulesets) echo '[]' ;; *) echo "unexpected endpoint: $1" >&2; return 99 ;; esac @@ -78,6 +89,11 @@ reset_case() { }' >"${repositories_config}" printf '[{"id":3,"name":"03 - Required CI"}]\n' >"${temporary}/playtester-list.json" printf '[{"id":3,"name":"03 - Required CI"}]\n' >"${temporary}/observatory-list.json" + rm -f "${temporary}/"*-later-rule.json "${temporary}/"*-later-unreadable + printf '{"id":2,"full_name":"atrinik/playtester","archived":false}\n' \ + >"${temporary}/playtester-metadata.json" + printf '{"id":1,"full_name":"atrinik/observatory","archived":false}\n' \ + >"${temporary}/observatory-metadata.json" live_rule playtester >"${temporary}/playtester-rule.json" live_rule observatory >"${temporary}/observatory-rule.json" } @@ -206,6 +222,43 @@ for filter in '.source_type = "Organization"' '.source = "foreign/playtester"' \ grep -Fxq 'SKIP atrinik/playtester 08 - Merge queue: unrecognized ownership or payload; retained' \ "${temporary}/output" done +# Identity drift, a concurrent edit after the first read, or an unreadable +# final read retains the queue. +retire_playtester() { + reset_case + jq '.merge_queue = []' "${repositories_config}" >"${temporary}/retired.json" + mv "${temporary}/retired.json" "${repositories_config}" + printf '[{"id":8,"name":"08 - Merge queue"}]\n' >"${temporary}/playtester-list.json" +} +for filter in '.id = 999' '.full_name = "foreign/playtester"' '.archived = true'; do + retire_playtester + jq "${filter}" "${temporary}/playtester-metadata.json" >"${temporary}/changed.json" + mv "${temporary}/changed.json" "${temporary}/playtester-metadata.json" + delete_removed_merge_queue_rulesets >"${temporary}/output" + [[ $(writes) == '[]' ]] + grep -Fxq 'SKIP atrinik/playtester 08 - Merge queue: repository identity or archive state changed; retained' \ + "${temporary}/output" +done +for filter in '.enforcement = "disabled"' '.source_type = "Organization"' \ + '.rules[0].parameters.max_entries_to_merge = 1'; do + retire_playtester + jq "${filter}" "${temporary}/playtester-rule.json" >"${temporary}/playtester-later-rule.json" + delete_removed_merge_queue_rulesets >"${temporary}/output" + [[ $(writes) == '[]' ]] + grep -Fxq 'SKIP atrinik/playtester 08 - Merge queue: rule changed during retirement preflight; retained' \ + "${temporary}/output" + # The final read happens after the identity fence. + jq -se ' + [.[] | .endpoint] as $calls | + ($calls | index("repos/atrinik/playtester")) < + ($calls | rindex("repos/atrinik/playtester/rulesets/8")) + ' "${log}" >/dev/null +done +retire_playtester +touch "${temporary}/playtester-later-unreadable" +expect_failure 'merge-queue final retirement read failed; retained: playtester' \ + delete_removed_merge_queue_rulesets + # Queued repositories and repositories without the rule are left alone. reset_case printf '[{"id":8,"name":"08 - Merge queue"}]\n' >"${temporary}/playtester-list.json" From d5e1d388b8a4586c417dd8b187b69e2ed3289b43 Mon Sep 17 00:00:00 2001 From: Zoey Rose Date: Sun, 18 Oct 2026 03:55:50 -0500 Subject: [PATCH 3/4] fix(rulesets): keep required CI strict until the queue is installed The required-CI payload dropped the up-to-date requirement for every repository named in merge_queue. The queue publisher skips an inactive entry, so such a repository would lose strict CI without gaining a queue, and the gap would become live if it were later reactivated. Required CI now relaxes only for listed repositories that are active in this run, which are exactly the ones the queue publisher installs. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 9 +++++---- README.md | 3 ++- bin/publish | 26 ++++++++++++++++++++++---- tests/publish-merge-queue.sh | 10 ++++++++++ 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 17d26b7..215add6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,10 +68,11 @@ repository rulesets, so `08 - Merge queue` stays repository-owned in both scopes. Every queued repository is in `required_ci`, has every required check reporting on `merge_group` before it is listed, never requires Dependency - review, and drops the up-to-date requirement from required CI. Install the - queue before relaxing required CI, restore the strict policy before - retiring a queue, and retire only an unmodified repository-owned rule that - passes a fresh identity check and final re-read. + review, and drops the up-to-date requirement from required CI only while it + is active and its queue is installed. Install the queue before relaxing + required CI, restore the strict policy before retiring a queue, and retire + only an unmodified repository-owned rule that passes a fresh identity check + and final re-read. - Keep required workflow job names synchronized with rulesets. Workflow or permissions changes also require actionlint, least-privilege review, and immutable action references according to policy. diff --git a/README.md b/README.md index 019f379..0cc59c5 100644 --- a/README.md +++ b/README.md @@ -974,7 +974,8 @@ requires each listed repository to be in `required_ci`, and keeps the template's parameters within GitHub's API ranges and the allowed merge method. GitHub Team offers merge queue only for public repositories, so the publisher fails before any write if a listed active repository is private. Like the -security gates, it skips a listed repository that is not active. +security gates, it skips a listed repository that is not active; that +repository's required CI stays strict because no queue was installed. Before adding a repository: diff --git a/bin/publish b/bin/publish index 613f0a0..2d37f28 100755 --- a/bin/publish +++ b/bin/publish @@ -657,7 +657,7 @@ delete_retired_repository_required_ci() { publish_repository_rulesets() { local repository ruleset_config ci_payload maintenance branch required_ci - local input + local input queued # Establish the Classic-specific policy before replacing its shared policy. for repository in "${active_repositories[@]}"; do @@ -687,11 +687,13 @@ publish_repository_rulesets() { "${ruleset_config}" || return 1 done < <(jq -r '.pull_request_gate[]' "${repositories_config}") + queued=$(active_merge_queue_repositories) while IFS= read -r repository; do ci_payload=$(mktemp) temporary_files+=("${ci_payload}") jq --arg repository "${repository}" \ --argjson app_id 15368 \ + --argjson queued "${queued}" \ --slurpfile advisory_windows "${advisory_merge_windows_config}" \ --slurpfile repositories "${repositories_config}" ' .bypass_actors |= map( @@ -706,7 +708,7 @@ publish_repository_rulesets() { map({context: ., integration_id: $app_id})) | # A merge queue tests the merged result, so its branch need not be current. .rules[0].parameters.strict_required_status_checks_policy = - (($repositories[0].merge_queue | index($repository)) == null) + (($queued | index($repository)) == null) ' "${root}/config/rulesets/required-ci.json" >"${ci_payload}" upsert_repository_ruleset "${repository}" "${ci_payload}" || return 1 done < <(jq -r '.required_ci | keys[]' "${repositories_config}") @@ -736,7 +738,7 @@ publish_repository_rulesets() { } publish_organization_rulesets() { - local input repository ci_payload maintenance branch required_ci + local input repository ci_payload maintenance branch required_ci queued local -a desired_rulesets local repositories ruleset name id desired rulesets @@ -808,6 +810,7 @@ publish_organization_rulesets() { temporary_files+=("${input}") upsert_organization_ruleset "${input}" || return 1 + queued=$(active_merge_queue_repositories) while IFS= read -r repository; do name="03 - Required CI - ${repository}" desired_rulesets+=("${name}") @@ -816,6 +819,7 @@ publish_organization_rulesets() { jq --arg name "${name}" \ --arg repository "${repository}" \ --argjson app_id 15368 \ + --argjson queued "${queued}" \ --slurpfile advisory_windows "${advisory_merge_windows_config}" \ --slurpfile repositories "${repositories_config}" ' .name = $name | @@ -836,7 +840,7 @@ publish_organization_rulesets() { map({context: ., integration_id: $app_id})) | # A merge queue tests the merged result, so its branch need not be current. .rules[0].parameters.strict_required_status_checks_policy = - (($repositories[0].merge_queue | index($repository)) == null) + (($queued | index($repository)) == null) ' "${root}/config/rulesets/required-ci.json" >"${ci_payload}" upsert_organization_ruleset "${ci_payload}" || return 1 done < <(jq -r '.required_ci | keys[]' "${repositories_config}") @@ -1377,6 +1381,20 @@ publish_security_merge_rulesets() { done } +# Required CI stops requiring a current branch only where this run installs +# the queue: the repository is listed and active. +active_merge_queue_repositories() { + local repository + local -a queued=() + + while IFS= read -r repository; do + if [[ -v active_repository_ids["${repository}"] ]]; then + queued+=("${repository}") + fi + done < <(jq -r '(.merge_queue // [])[]' "${repositories_config}") + jq -cn '$ARGS.positional' --args "${queued[@]}" +} + repository_uses_merge_queue() { local repository=$1 diff --git a/tests/publish-merge-queue.sh b/tests/publish-merge-queue.sh index 23afb72..d375e71 100755 --- a/tests/publish-merge-queue.sh +++ b/tests/publish-merge-queue.sh @@ -297,6 +297,16 @@ for scope in repository-observatory-03\ -\ Required\ CI organization-03\ -\ Requ jq -e '.rules[0].parameters.strict_required_status_checks_policy == true' \ "${temporary}/captured/${scope}" >/dev/null done +# A listed but inactive repository gets no queue, so its required CI stays strict. +reset_case +rm -f "${temporary}/captured/"* +active_repository_ids=([observatory]=1) +publish_repository_rulesets >/dev/null +publish_organization_rulesets >/dev/null +for scope in repository-playtester-03\ -\ Required\ CI organization-03\ -\ Required\ CI\ -\ playtester; do + jq -e '.rules[0].parameters.strict_required_status_checks_policy == true' \ + "${temporary}/captured/${scope}" >/dev/null +done # Install the queue before required CI is relaxed, and retire it only after # required CI is restored. Neither step belongs to one policy scope. From 0c2dc23938c8cc23cbc34f9d95710a1bc67124db Mon Sep 17 00:00:00 2001 From: Zoey Rose Date: Sun, 18 Oct 2026 04:22:55 -0500 Subject: [PATCH 4/4] fix(rulesets): relax required CI only behind a verified queue When applying, required CI dropped the up-to-date requirement for every active queued repository, even if the queue had been deleted, disabled or scoped away after the queue publisher ran. Both scopes now re-read the queue just before publishing required CI. They relax it only when that read shows the exact repository-owned queue, and otherwise keep it strict with a SKIP. A plan still reports the planned installation. The pilot queue also waited 5 minutes for a minimum group size of 1, which the first queued pull request already meets, so the wait did nothing. It is now 0, and validation allows a wait only with a minimum above one. A new schema test covers the inventory, queue template, required-CI template and Dependency review constraints. The CONTRIBUTING paragraph is moved back after the Classic CodeQL example it split. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- CONTRIBUTING.md | 18 ++++---- README.md | 15 ++++-- bin/publish | 40 ++++++++++++---- bin/validate | 7 ++- config/rulesets/merge-queue.json | 2 +- tests/publish-merge-queue.sh | 76 ++++++++++++++++++++---------- tests/validate-merge-queue.sh | 79 ++++++++++++++++++++++++++++++++ 8 files changed, 189 insertions(+), 50 deletions(-) create mode 100755 tests/validate-merge-queue.sh diff --git a/AGENTS.md b/AGENTS.md index 215add6..40ab76f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,7 +69,7 @@ scopes. Every queued repository is in `required_ci`, has every required check reporting on `merge_group` before it is listed, never requires Dependency review, and drops the up-to-date requirement from required CI only while it - is active and its queue is installed. Install the queue before relaxing + is active and a fresh read verifies its installed queue. Install the queue before relaxing required CI, restore the strict policy before retiring a queue, and retire only an unmodified repository-owned rule that passes a fresh identity check and final re-read. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b2bb462..949a792 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -113,7 +113,10 @@ replace its default, or re-attach repositories, because attachment starts an asynchronous enablement event. Do not require a new workflow context until that exact aggregate context has -landed and passed on the protected branch. +landed and passed on the protected branch. In particular, add Classic's +`CodeQL validation` requirement only after its advanced workflow is merged; +otherwise the required check would prevent the workflow pull request itself +from merging. Add a repository to the `merge_queue` inventory only after every one of its required checks reports on the `merge_group` event on its default branch. Keep @@ -121,15 +124,12 @@ required checks reports on the `merge_group` event on its default branch. Keep organization rulesets. Keep its parameters within GitHub's API ranges, and use the organization's only merge method. Queued repositories drop the up-to-date requirement from required CI and never require Dependency review. Merge-queue -changes need publisher tests for creation, a converged rerun, drift -convergence, retained foreign or modified rules, the identity and final-read +changes need schema negative tests and publisher tests for creation, a +converged rerun, drift convergence, required CI that stays strict without a +verified queue, retained foreign or modified rules, the identity and final-read fences before retirement, and install-before-relax and restore-before-retire -ordering. They also need audit scenarios for a missing -queue, parameter drift, organization ownership, strict drift, and a retained -queue. In particular, add Classic's -`CodeQL validation` requirement only after its advanced workflow is merged; -otherwise the required check would prevent the workflow pull request itself -from merging. +ordering. They also need audit scenarios for a missing queue, parameter drift, +organization ownership, strict drift, and a retained queue. Maintenance-branch retirement removes the active entry from `config/repositories.json` and adds an immutable recovery record to diff --git a/README.md b/README.md index 0cc59c5..77c3f52 100644 --- a/README.md +++ b/README.md @@ -958,8 +958,9 @@ branch, as they do for `06 - CodeQL security`. The rule comes from - squash merges, the organization's only merge method; - `ALLGREEN` grouping, so every queued pull request's own merge result must pass the required checks, not only the head of its group; -- a build concurrency of 5, groups of 1 to 5 pull requests, and a 5-minute - wait for the minimum group size; +- a build concurrency of 5 and groups of 1 to 5 pull requests, with no wait: + a single queued pull request merges as soon as its checks pass, and pull + requests queued together share a group; - a 60-minute timeout for required checks to report on a merge group. Organization owners keep the usual pull-request-only bypass. During an @@ -969,9 +970,13 @@ repository's required CI. A queued repository's `03 - Required CI` rule sets `strict_required_status_checks_policy` to `false`. The queue already tests each pull request merged onto the latest base, so the branch itself no longer has -to be current. Every other repository keeps the strict policy. Validation -requires each listed repository to be in `required_ci`, and keeps the -template's parameters within GitHub's API ranges and the allowed merge method. +to be current. When applying, the publisher re-reads the queue just before it +publishes required CI and relaxes the rule only if that read shows the exact +repository-owned queue. Otherwise it prints a `SKIP` and keeps required CI +strict. Every other repository keeps the strict policy. Validation requires +each listed repository to be in `required_ci`. It also keeps the template's +parameters within GitHub's API ranges and the allowed merge method, and allows +a wait only with a minimum group size above one. GitHub Team offers merge queue only for public repositories, so the publisher fails before any write if a listed active repository is private. Like the security gates, it skips a listed repository that is not active; that diff --git a/bin/publish b/bin/publish index 2d37f28..d1dcd8a 100755 --- a/bin/publish +++ b/bin/publish @@ -687,7 +687,8 @@ publish_repository_rulesets() { "${ruleset_config}" || return 1 done < <(jq -r '.pull_request_gate[]' "${repositories_config}") - queued=$(active_merge_queue_repositories) + verify_installed_merge_queues || return 1 + queued=${verified_merge_queue_repositories} while IFS= read -r repository; do ci_payload=$(mktemp) temporary_files+=("${ci_payload}") @@ -810,7 +811,8 @@ publish_organization_rulesets() { temporary_files+=("${input}") upsert_organization_ruleset "${input}" || return 1 - queued=$(active_merge_queue_repositories) + verify_installed_merge_queues || return 1 + queued=${verified_merge_queue_repositories} while IFS= read -r repository; do name="03 - Required CI - ${repository}" desired_rulesets+=("${name}") @@ -1381,18 +1383,38 @@ publish_security_merge_rulesets() { done } -# Required CI stops requiring a current branch only where this run installs -# the queue: the repository is listed and active. -active_merge_queue_repositories() { - local repository +# Required CI stops requiring a current branch only where the queue protects +# the branch: the repository is listed and active and, when applying, a fresh +# read just before required CI is published shows the exact repository-owned +# queue. A plan reports the queue publisher's planned installation instead. +# Sets verified_merge_queue_repositories to a JSON array. +verify_installed_merge_queues() { + local repository name existing id current template local -a queued=() + name=$(jq -r '.name' "${root}/config/rulesets/merge-queue.json") while IFS= read -r repository; do - if [[ -v active_repository_ids["${repository}"] ]]; then - queued+=("${repository}") + [[ -v active_repository_ids["${repository}"] ]] || continue + if ${apply}; then + existing=$(list_rulesets \ + "repos/${organization}/${repository}/rulesets?includes_parents=false") || return 1 + id=$(jq -r --arg name "${name}" \ + '.[] | select(.name == $name) | .id' <<<"${existing}") + current='{}' + if [[ -n ${id} ]]; then + current=$(github_api "repos/${organization}/${repository}/rulesets/${id}") || return 1 + fi + template=$(make_merge_queue_ruleset "${repository}") + temporary_files+=("${template}") + if [[ -z ${id} ]] || + ! owned_security_ruleset_matches "${repository}" "${id}" "${template}" "${current}"; then + echo "SKIP ${organization}/${repository} ${name}: queue not verified; required CI stays strict" + continue + fi fi + queued+=("${repository}") done < <(jq -r '(.merge_queue // [])[]' "${repositories_config}") - jq -cn '$ARGS.positional' --args "${queued[@]}" + verified_merge_queue_repositories=$(jq -cn '$ARGS.positional' --args "${queued[@]}") } repository_uses_merge_queue() { diff --git a/bin/validate b/bin/validate index 46bdf21..a45ab4f 100755 --- a/bin/validate +++ b/bin/validate @@ -166,8 +166,9 @@ jq -e ' ' "${root}/config/rulesets/required-ci.json" >/dev/null # GitHub accepts merge-queue rules only in repository rulesets. Keep the queue -# within the API ranges, merging with the organization's only merge method, and -# able to build and merge at least one entry. +# within the API ranges, merging with the organization's only merge method, +# able to build and merge at least one entry, and without a wait that a +# one-entry minimum can never use. jq -e --slurpfile defaults "${root}/config/repository-defaults.json" ' def integer_between($low; $high): type == "number" and floor == . and . >= $low and . <= $high; @@ -195,6 +196,8 @@ jq -e --slurpfile defaults "${root}/config/repository-defaults.json" ' (.min_entries_to_merge | integer_between(1; 100)) and .min_entries_to_merge <= .max_entries_to_merge and (.min_entries_to_merge_wait_minutes | integer_between(0; 360)) and + # The wait only applies while a group is below its minimum size. + (.min_entries_to_merge > 1 or .min_entries_to_merge_wait_minutes == 0) and ({MERGE: "allow_merge_commit", SQUASH: "allow_squash_merge", REBASE: "allow_rebase_merge"}[.merge_method] as $setting | $setting != null and $defaults[0][$setting] == true)) diff --git a/config/rulesets/merge-queue.json b/config/rulesets/merge-queue.json index 69f6968..872e23f 100644 --- a/config/rulesets/merge-queue.json +++ b/config/rulesets/merge-queue.json @@ -27,7 +27,7 @@ "max_entries_to_merge": 5, "merge_method": "SQUASH", "min_entries_to_merge": 1, - "min_entries_to_merge_wait_minutes": 5 + "min_entries_to_merge_wait_minutes": 0 } } ] diff --git a/tests/publish-merge-queue.sh b/tests/publish-merge-queue.sh index d375e71..8598a25 100755 --- a/tests/publish-merge-queue.sh +++ b/tests/publish-merge-queue.sh @@ -118,7 +118,7 @@ jq -e ' max_entries_to_merge: 5, merge_method: "SQUASH", min_entries_to_merge: 1, - min_entries_to_merge_wait_minutes: 5 + min_entries_to_merge_wait_minutes: 0 }}] ' "${template}" >/dev/null @@ -269,8 +269,8 @@ if grep -Fq 'repos/atrinik/playtester/' "${log}"; then exit 1 fi -# Required CI stops requiring a current branch only where the queue tests the -# merged result, in both policy scopes. +# Required CI stops requiring a current branch only where the queue protects +# the branch, in both policy scopes. upsert_repository_ruleset() { cp "$2" "${temporary}/captured/repository-$1-$(jq -r .name "$2")" } @@ -282,31 +282,61 @@ delete_stale_repository_maintenance_rulesets() { :; } delete_stale_repository_advisory_rulesets() { :; } delete_managed_organization_rulesets() { :; } delete_managed_repository_rulesets() { :; } +publish_required_ci() { + rm -f "${temporary}/captured/"* + publish_repository_rulesets >"${temporary}/output" + publish_organization_rulesets >>"${temporary}/output" +} +assert_playtester_strict() { + local strict=$1 scope + for scope in repository-playtester-03\ -\ Required\ CI organization-03\ -\ Required\ CI\ -\ playtester; do + jq -e --argjson strict "${strict}" ' + .rules[0].parameters.strict_required_status_checks_policy == $strict and + .rules[0].parameters.required_status_checks == + [{context: "Playtester validation", integration_id: 15368}, + {context: "Conventional PR title", integration_id: 15368}] + ' "${temporary}/captured/${scope}" >/dev/null + done + for scope in repository-observatory-03\ -\ Required\ CI organization-03\ -\ Required\ CI\ -\ observatory; do + jq -e '.rules[0].parameters.strict_required_status_checks_policy == true' \ + "${temporary}/captured/${scope}" >/dev/null + done +} +# Applying relaxes required CI only after a fresh read shows the exact queue. reset_case -publish_repository_rulesets >/dev/null -publish_organization_rulesets >/dev/null -for scope in repository-playtester-03\ -\ Required\ CI organization-03\ -\ Required\ CI\ -\ playtester; do - jq -e ' - .rules[0].parameters.strict_required_status_checks_policy == false and - .rules[0].parameters.required_status_checks == - [{context: "Playtester validation", integration_id: 15368}, - {context: "Conventional PR title", integration_id: 15368}] - ' "${temporary}/captured/${scope}" >/dev/null -done -for scope in repository-observatory-03\ -\ Required\ CI organization-03\ -\ Required\ CI\ -\ observatory; do - jq -e '.rules[0].parameters.strict_required_status_checks_policy == true' \ - "${temporary}/captured/${scope}" >/dev/null +printf '[{"id":8,"name":"08 - Merge queue"}]\n' >"${temporary}/playtester-list.json" +publish_required_ci +assert_playtester_strict false +for state in missing disabled scoped-away foreign modified; do + reset_case + printf '[{"id":8,"name":"08 - Merge queue"}]\n' >"${temporary}/playtester-list.json" + case ${state} in + missing) printf '[]\n' >"${temporary}/playtester-list.json" ;; + disabled) filter='.enforcement = "disabled"' ;; + scoped-away) filter='.conditions.ref_name.include = ["refs/heads/other"]' ;; + foreign) filter='.source_type = "Organization"' ;; + modified) filter='.rules[0].parameters.grouping_strategy = "HEADGREEN"' ;; + esac + if [[ ${state} != missing ]]; then + jq "${filter}" "${temporary}/playtester-rule.json" >"${temporary}/changed.json" + mv "${temporary}/changed.json" "${temporary}/playtester-rule.json" + fi + publish_required_ci + assert_playtester_strict true + grep -Fxq 'SKIP atrinik/playtester 08 - Merge queue: queue not verified; required CI stays strict' \ + "${temporary}/output" done +# A plan reports the queue publisher's planned installation. +reset_case +apply=false +printf '[]\n' >"${temporary}/playtester-list.json" +publish_required_ci +assert_playtester_strict false # A listed but inactive repository gets no queue, so its required CI stays strict. reset_case -rm -f "${temporary}/captured/"* active_repository_ids=([observatory]=1) -publish_repository_rulesets >/dev/null -publish_organization_rulesets >/dev/null -for scope in repository-playtester-03\ -\ Required\ CI organization-03\ -\ Required\ CI\ -\ playtester; do - jq -e '.rules[0].parameters.strict_required_status_checks_policy == true' \ - "${temporary}/captured/${scope}" >/dev/null -done +publish_required_ci +assert_playtester_strict true # Install the queue before required CI is relaxed, and retire it only after # required CI is restored. Neither step belongs to one policy scope. diff --git a/tests/validate-merge-queue.sh b/tests/validate-merge-queue.sh new file mode 100755 index 0000000..a45eef2 --- /dev/null +++ b/tests/validate-merge-queue.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +set -euo pipefail +root=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +temporary=$(mktemp -d) +trap 'rm -rf "${temporary}"' EXIT +mkdir -p "${temporary}/bin" +cp "${root}/bin/validate" "${temporary}/bin/" +cp -R "${root}/config" "${root}/community-health" "${root}/.github" "${temporary}/" +config=${temporary}/config +"${temporary}/bin/validate" >/dev/null +cp "${config}/repositories.json" "${temporary}/repositories.json" +cp "${config}/rulesets/merge-queue.json" "${temporary}/merge-queue.json" +cp "${config}/rulesets/required-ci.json" "${temporary}/required-ci.json" +cp "${config}/security-merge-protection.json" "${temporary}/security.json" + +expect_invalid() { + local file=$1 source=$2 filter=$3 + jq "${filter}" "${source}" >"${file}" + if "${temporary}/bin/validate" >/dev/null 2>&1; then + echo "error: accepted invalid merge-queue policy: ${filter}" >&2 + exit 1 + fi + cp "${source}" "${file}" +} + +# Queued repositories need required CI for the queue to run on each merge result. +for filter in \ + 'del(.merge_queue)' '.merge_queue = "playtester"' '.merge_queue = [null]' \ + '.merge_queue = [""]' '.merge_queue = ["playtester", "playtester"]' \ + '.merge_queue = ["web-platform"]' '.merge_queue = ["legacy-client"]' \ + '.merge_queue = ["unknown"]'; do + expect_invalid "${config}/repositories.json" "${temporary}/repositories.json" "${filter}" +done + +# Parameters stay within GitHub's API ranges and the organization's merge method. +for filter in \ + '.name = "Merge queue"' '.target = "tag"' '.enforcement = "disabled"' \ + '.conditions.ref_name.include = ["refs/heads/*"]' \ + '.bypass_actors[0].bypass_mode = "always"' '.bypass_actors = []' \ + '.rules += [{type: "deletion"}]' '.rules[0].type = "pull_request"' \ + '.rules[0].parameters.merge_method = "MERGE"' \ + '.rules[0].parameters.merge_method = "REBASE"' \ + '.rules[0].parameters.merge_method = "squash"' \ + '.rules[0].parameters.grouping_strategy = "ANYGREEN"' \ + '.rules[0].parameters.check_response_timeout_minutes = 0' \ + '.rules[0].parameters.check_response_timeout_minutes = 361' \ + '.rules[0].parameters.check_response_timeout_minutes = 1.5' \ + '.rules[0].parameters.check_response_timeout_minutes = "60"' \ + '.rules[0].parameters.max_entries_to_build = 0' \ + '.rules[0].parameters.max_entries_to_build = 101' \ + '.rules[0].parameters.max_entries_to_merge = 0' \ + '.rules[0].parameters.min_entries_to_merge = 0' \ + '.rules[0].parameters.min_entries_to_merge = 6' \ + '.rules[0].parameters.min_entries_to_merge_wait_minutes = 361' \ + '.rules[0].parameters.min_entries_to_merge_wait_minutes = 5' \ + 'del(.rules[0].parameters.grouping_strategy)' \ + '.rules[0].parameters.unknown_option = true'; do + expect_invalid "${config}/rulesets/merge-queue.json" "${temporary}/merge-queue.json" "${filter}" +done +# A wait applies only while a group is below a minimum above one. +jq '.rules[0].parameters.min_entries_to_merge = 2 | + .rules[0].parameters.min_entries_to_merge_wait_minutes = 5' \ + "${temporary}/merge-queue.json" >"${config}/rulesets/merge-queue.json" +"${temporary}/bin/validate" >/dev/null +cp "${temporary}/merge-queue.json" "${config}/rulesets/merge-queue.json" + +# The publisher relaxes only a strict template, and only for queued repositories. +for filter in \ + '.rules[0].parameters.strict_required_status_checks_policy = false' \ + '.rules[0].parameters.required_status_checks = [{context: "Extra", integration_id: 15368}]' \ + '.name = "Required CI"'; do + expect_invalid "${config}/rulesets/required-ci.json" "${temporary}/required-ci.json" "${filter}" +done + +# Dependency review never reports on merge groups, so it cannot gate a queue. +expect_invalid "${config}/security-merge-protection.json" "${temporary}/security.json" \ + '.dependency_review = ["playtester"] | .activation.dependency_review = {playtester: "pending"}' +"${temporary}/bin/validate" >/dev/null +echo 'Merge-queue schema tests passed.'