From b69efb8ba2590439747fbe203e5bc4f397637901 Mon Sep 17 00:00:00 2001 From: Jiangzhou He Date: Sat, 24 Oct 2026 13:03:51 -0700 Subject: [PATCH] Merge queue: land every pull request as a merge commit; allow only merge commits An upstream merge must keep its upstream parent. With the queue squashing, it had to bypass the queue with an admin merge, and twice it went through the queue instead and lost its parent (cocoindex-plus#549, #557; #561). cocoindex-io/.github#7 then made ci-mode refuse a labelled pull request, but that still left a special path to remember. Now there is one path. The queue merges with merge commits (rulesets/repo/merge-queue.json), and apply_rulesets.py also sets the merge settings of every queue repository from rulesets/repo-settings.json: merge commits only, titled with the pull request's title. A pull request keeps its history, so an upstream merge goes through the queue like any change, and no merge method can squash it. Admins bypass only for incidents, with --merge. - ci-mode: back to the version before #7; the label guard has nothing left to guard. - CI.md: the queue section, Merge commits (first-parent reading, revert with -m 1), the incident bypass, Enforcement, and what was verified in cocoindex-plus on 2026-10-24. Already applied to cocoindex-plus with apply_rulesets.py; --check is clean. Co-Authored-By: Claude Opus 5.5 --- CI.md | 69 +++++++++++++++++++++------------- actions/ci-mode/action.yml | 28 +------------- rulesets/repo-settings.json | 7 ++++ rulesets/repo/merge-queue.json | 2 +- scripts/apply_rulesets.py | 22 +++++++++++ scripts/test_apply_rulesets.py | 4 +- 6 files changed, 77 insertions(+), 55 deletions(-) create mode 100644 rulesets/repo-settings.json diff --git a/CI.md b/CI.md index 1dce3f9..490f63c 100644 --- a/CI.md +++ b/CI.md @@ -48,8 +48,7 @@ workflow has all of them. 3. **Drafts skip.** Every job has `github.event.pull_request.draft != true` in its `if`, or needs a job that has it. 4. **A `changes` job** runs `cocoindex-io/.github/actions/ci-mode@v1` (it - needs `checks: read`, and `pull-requests: read` in a repository with a - merge queue) and the path filter. Expensive jobs run when + needs `checks: read`) and the path filter. Expensive jobs run when `full == 'true'` and the filter output is not `'false'`. The filter is skipped for a stack layer, and so is any other diff against the pull request's base branch: a stack layer's base is the layer below it, but @@ -69,9 +68,9 @@ required `name (value)` check from it never arrives. ## The merge queue -The `merge-queue` repository ruleset configures it: squash merges, every entry must -pass (`ALLGREEN`), and at most 4 builds at once. That build limit is the CI -quota: a queued pull request waits for a free build slot. +The `merge-queue` repository ruleset configures it: merge commits, every +entry must pass (`ALLGREEN`), and at most 4 builds at once. That build limit +is the CI quota: a queued pull request waits for a free build slot. **How it builds.** Each queued pull request gets its own build of the base branch plus every pull request ahead of it. Builds run in parallel up to the @@ -104,19 +103,16 @@ requests go to the queue on their own, which tests them together anyway. ## Merge commits -The queue squashes. A pull request that must land as a merge commit, such as -an upstream merge, bypasses the queue: repository admins may merge pull -requests directly. Run the full tier on its branch first with -`gh workflow run`, wait for it to pass, then merge with -`gh pr merge --merge --admin`. Apart from an incident (below), nothing else -bypasses the queue. +The queue lands every pull request as a merge commit, and a repository with a +queue allows no other merge method. A pull request keeps its own history, so +an upstream merge keeps its upstream parent and goes through the queue like +any other change. No merge method can squash it, and nothing bypasses the +queue for it. -Label such a pull request `merge-commit` when you open it. `ci-mode` fails -the merge group of a pull request with that label before any test runs, so -the queue removes it instead of squashing it. When `ci-mode` can't read the -labels, the merge group fails too: queueing a pull request again costs one -build, and a squashed merge commit costs a repair merge. The label has to be -on the pull request before it enters the queue. +The default branch's first-parent history has one commit per pull request, +titled with the pull request's title and number. Read it with +`git log --first-parent`, bisect with `git bisect start --first-parent`, and +revert a pull request with `git revert -m 1 `. ## Urgent changes @@ -131,7 +127,7 @@ gh api graphql -F id="$(gh pr view --json id --jq .id)" \ -f query='mutation($id:ID!){enqueuePullRequest(input:{pullRequestId:$id, jump:true}){mergeQueueEntry{position}}}' ``` -Bypassing the queue (`gh pr merge --squash --admin`) lands a change without +Bypassing the queue (`gh pr merge --merge --admin`) lands a change without its full CI run. It is only for an incident in which CI itself is broken. The Change Management Policy requires green CI even for a hotfix, so such a bypass is a deviation and goes in the incident log. @@ -154,14 +150,18 @@ A repository opts in by setting its `ci-tier` custom property to repository and which turns the queue on. GitHub's organization rulesets don't accept a merge-queue rule. -Other required checks stay in the repository's own rulesets. Both rulesets -let repository admins merge pull requests directly, for merge commits and -incidents. To leave the queue, unset the property and delete the -repository's `merge-queue` ruleset. - -- Apply or check the property and rulesets: `scripts/apply_rulesets.py - [--check]`. It needs an organization owner with the `admin:org` scope, and - the Change Management Policy asks that the change be announced. +The script also sets the repository's merge settings from +`rulesets/repo-settings.json`: merge commits only, titled with the pull +request's title. Other required checks stay in the repository's own +rulesets. Both rulesets let repository admins merge pull requests directly, +for incidents. To leave the queue, unset the property, delete the +repository's `merge-queue` ruleset, and set its merge settings as it wants +them. + +- Apply or check the property, rulesets and merge settings: + `scripts/apply_rulesets.py [--check]`. It needs an organization owner with + the `admin:org` scope, and the Change Management Policy asks that the + change be announced. - Check a repository locally: `uv run --with pyyaml scripts/check_ci_conformance.py PATH --repo OWNER/NAME`. - A ruleset runs a required workflow only on `opened`, `synchronize` and @@ -198,3 +198,20 @@ Checked on 2026-10-22 in a temporary private repository: commit with both parents. - GitHub's organization rulesets reject a `merge_queue` rule; repository rulesets accept it. + +Checked on 2026-10-24 in cocoindex-plus, after its queue moved to merge +commits: + +- An upstream merge (cocoindex-plus#565) went through the queue and landed + as the merge commit its merge group tested. Its second parent is the pull + request's head, the upstream merge itself, so the upstream commit stayed + an ancestor of `main`. +- The queue's merge commit takes the repository's merge settings: the pull + request's title and number as its subject, the pull request's body as its + message. +- With squash and rebase merges disallowed, `gh pr merge --squash --admin` + and `--rebase --admin` fail: "Squash merges are not allowed on this + repository." +- The queue marks a failed entry unmergeable at once, but removes it only + when it reaches the front. Until then, the entries behind it are built on + top of it. diff --git a/actions/ci-mode/action.yml b/actions/ci-mode/action.yml index 6c1299e..2a4692d 100644 --- a/actions/ci-mode/action.yml +++ b/actions/ci-mode/action.yml @@ -1,10 +1,8 @@ name: CI mode description: >- - Decides whether this run executes the full CI tier or only the fast one, - and fails a merge group whose pull request must land as a merge commit. + Decides whether this run executes the full CI tier or only the fast one. See CI.md in cocoindex-io/.github for the tiers. The calling job needs - `checks: read` and `contents: read`, and in a repository with a merge - queue also `pull-requests: read`. + `checks: read` and `contents: read`. outputs: full: description: "'true' when this run executes the full tier, 'false' for the fast tier." @@ -24,7 +22,6 @@ runs: BASE: ${{ github.event.pull_request.stack.base.ref || github.event.pull_request.base.ref }} STACK_POSITION: ${{ github.event.pull_request.stack.position }} STACK_SIZE: ${{ github.event.pull_request.stack.size }} - HEAD_REF: ${{ github.event.merge_group.head_ref }} run: | full=true reason="a $EVENT run executes the full tier" @@ -53,27 +50,6 @@ runs: else echo "::warning::Could not read the checks of $SHA, so this run executes the full tier." fi - elif [ "$EVENT" = merge_group ]; then - # The queue squashes. A pull request labelled merge-commit, such as - # an upstream merge, must keep its merge commit, so its merge group - # fails here, before any expensive job starts, and the queue removes - # it. A failed lookup fails too: queueing a pull request again costs - # one build, and a squashed merge commit costs a repair. The queue's - # branch names the pull request: gh-readonly-queue//pr--. - pr=${HEAD_REF##*/pr-} - pr=${pr%%-*} - if ! [[ $pr =~ ^[0-9]+$ ]]; then - echo "::error::Could not find the pull request number in the merge group ref $HEAD_REF." - exit 1 - fi - if ! labelled=$(gh api "repos/$REPO/pulls/$pr" --jq 'any(.labels[]; .name == "merge-commit")'); then - echo "::error::Could not read the labels of #$pr, so its merge group fails. Queue it again; if this repeats, check that the job that runs ci-mode has \`pull-requests: read\`." - exit 1 - fi - if [ "$labelled" = true ]; then - echo "::error::#$pr is labelled merge-commit, so it must land as a merge commit, and the queue squashes. Merge it around the queue after a full run on its branch passes: gh pr merge $pr --merge --admin (CI.md, Merge commits)." - exit 1 - fi fi echo "full=$full" >> "$GITHUB_OUTPUT" tier=fast diff --git a/rulesets/repo-settings.json b/rulesets/repo-settings.json new file mode 100644 index 0000000..8c79d2b --- /dev/null +++ b/rulesets/repo-settings.json @@ -0,0 +1,7 @@ +{ + "allow_merge_commit": true, + "allow_squash_merge": false, + "allow_rebase_merge": false, + "merge_commit_title": "PR_TITLE", + "merge_commit_message": "PR_BODY" +} diff --git a/rulesets/repo/merge-queue.json b/rulesets/repo/merge-queue.json index 6a259d7..c28c953 100644 --- a/rulesets/repo/merge-queue.json +++ b/rulesets/repo/merge-queue.json @@ -21,7 +21,7 @@ { "type": "merge_queue", "parameters": { - "merge_method": "SQUASH", + "merge_method": "MERGE", "grouping_strategy": "ALLGREEN", "max_entries_to_build": 4, "max_entries_to_merge": 4, diff --git a/scripts/apply_rulesets.py b/scripts/apply_rulesets.py index 678956b..7ddd0f4 100644 --- a/scripts/apply_rulesets.py +++ b/scripts/apply_rulesets.py @@ -9,6 +9,8 @@ - rulesets/repo/*.json: repository rulesets, put on every repository whose `ci-tier` is `merge-queue`. GitHub doesn't accept a merge-queue rule in an organization ruleset, so the queue lives here. +- rulesets/repo-settings.json: the merge settings of those repositories. They + allow only the merge method the queue uses. Needs the `gh` CLI signed in as an organization owner with the admin:org scope (`gh auth refresh -h github.com -s admin:org`). Org-setting changes follow the @@ -28,6 +30,7 @@ ROOT = pathlib.Path(__file__).resolve().parent.parent / "rulesets" PROPERTY_FILE = ROOT / "ci-tier-property.json" +SETTINGS_FILE = ROOT / "repo-settings.json" OPT_IN_VALUE = "merge-queue" @@ -114,6 +117,21 @@ def sync_ruleset(scope: str, want: dict[str, Any], check: bool) -> bool: return True +def sync_settings(repo: str, want: dict[str, Any], check: bool) -> bool: + """Make the settings of `repo` (repos//) that `want` names match it.""" + label = f"{repo} settings" + found = differences(want, gh(repo)) + if not found: + print(f"{label}: up to date") + return True + print(f"{label}: " + "; ".join(found)) + if check: + return False + gh("-X", "PATCH", repo, payload=want) + print(f"{label}: applied") + return True + + def main() -> int: parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) parser.add_argument("--org", default="cocoindex-io") @@ -126,9 +144,13 @@ def main() -> int: ok = sync_ruleset(f"orgs/{args.org}", json.loads(path.read_text()), args.check) and ok repos = opted_in(gh_list(f"orgs/{args.org}/properties/values"), prop["property_name"]) print(f"repositories with {prop['property_name']}={OPT_IN_VALUE}: {', '.join(repos) or 'none'}") + settings = json.loads(SETTINGS_FILE.read_text()) for repo in repos: for path in sorted((ROOT / "repo").glob("*.json")): ok = sync_ruleset(f"repos/{args.org}/{repo}", json.loads(path.read_text()), args.check) and ok + # After the rulesets: the queue's merge method has to stay allowed + # while the ruleset still names the old one. + ok = sync_settings(f"repos/{args.org}/{repo}", settings, args.check) and ok return 0 if ok else 1 diff --git a/scripts/test_apply_rulesets.py b/scripts/test_apply_rulesets.py index 52a9961..50bfbe4 100644 --- a/scripts/test_apply_rulesets.py +++ b/scripts/test_apply_rulesets.py @@ -4,8 +4,8 @@ def test_subset_compare_ignores_extra_keys() -> None: - want = {"name": "x", "rules": [{"type": "merge_queue", "parameters": {"merge_method": "SQUASH"}}]} - have = {"name": "x", "id": 1, "rules": [{"type": "merge_queue", "parameters": {"merge_method": "SQUASH", "extra": 1}}]} + want = {"name": "x", "rules": [{"type": "merge_queue", "parameters": {"merge_method": "MERGE"}}]} + have = {"name": "x", "id": 1, "rules": [{"type": "merge_queue", "parameters": {"merge_method": "MERGE", "extra": 1}}]} assert differences(want, have) == []