チェックが全部緑のプルリクエスト2つでmainが壊れた — GitHubのRequire branches to be up to date before mergingで防ぐ

はじめに

私はプルリクエストのチェックがすべて緑になったらマージしています。なので、自分のプルリクエストが触ってもいないテストファイルで CI が落ちたときは驚きました。

原因は私の変更ではありませんでした。別のプルリクエストがチェックをすべて通してマージされた直後に、main そのものが赤くなっていたのです。そのせいで、開いているプルリクエストはすべて自分とは関係のない理由で赤くなっていました。壊れた main の裏には、まだどのテストも実行されていない本物のバグも隠れていました。

マージされたプルリクエストのチェックは間違っていたわけではなく、古かっただけです。チェックが走ったのは、別のプルリクエストが先にマージされるより前でした。あとからマージされた側の変更が、先にマージされたプルリクエストの追加したコードを壊していました。GitHub の Require branches to be up to date before merging を有効にすると、こうしたマージは新しい CI の実行を待つようになります。

本記事では、緑のプルリクエスト 2 つがどうやって赤い main になるのかと、この設定と合わせて main に入れておきたいルールを整理します。

緑のプルリクエスト 2 つがマージされて main が赤くなるまで

緑のチェックは、ある時点の main のスナップショットに対する結果です。pull_request イベントの場合、GitHub Actions はプルリクエストのブランチとベースブランチのマージコミットをテストします。このマージコミットはワークフローが始まった時点で作られます。あとでベースブランチが進んでも、このテストが再実行されることはありません。

そのため、次のような流れが起こりえます。

1. プルリクエスト A の CI が走り、緑になります。CI がテストしたのは、その時点の main に A をマージしたものです。
2. A がレビューを待っている間に、プルリクエスト B が main にマージされます。
3. A がマージされます。A の緑の結果は、もう存在しない main に対して計算されたものです。A と B を組み合わせた CI は、2 つが main で合流するまで一度も走っていません。

たいていの場合、A と B は互いに影響せず、古い結果でも正しいままです。ですが、今回は影響しました。プルリクエスト A は共有のインターフェースに 4 つのメソッドを追加し、それを実装するテスト用フェイクも更新していました。並行して書かれたプルリクエスト B は新しいテストを追加し、そのテストには同じインターフェースの独自のフェイクが含まれていました。このフェイクは古いメソッド構成のままです。

2 つは別々のファイルを編集していたので、Git はコンフリクトなしにマージし、main で型チェッカーが失敗しました（クラス名は短縮しています）。

error: Argument "connector" to "Pipeline" has incompatible type "FakeConnector"; expected "FileConnector"  [arg-type]

型エラーの裏には、もう 1 つの不具合がありました。プルリクエスト B は共有のエラーヘルパーも変更していて、ストレージ API の 404 をプロジェクト独自の NotFoundError に変換するようにしていました。一方、プルリクエスト A で追加した restore() メソッドは、生の 404 を捕まえて False を返す作りのままでした。そのため、両方がマージされたあとは、完全に削除されたファイルを復元しようとすると例外が投げられるようになっていました。

まさにそのケースのテストは存在していましたが、一度も実行されていませんでした。CI のジョブがテストのステップに進む前に型チェックで止まっていたので、このテストが失敗したのは型エラーを直したあとでした。

ブランチ保護とは

ブランチ保護は、変更がブランチに入る前に GitHub が適用するルールの集まりです。代表的なルールには、直接の push ではなくプルリクエストを必須にする、一定数の承認レビューを必須にする、ステータスチェックの成功を必須にする、といったものがあります。ブランチへの force-push や削除のブロックもここで設定します。

設定方法は 2 つあります。ブランチ保護ルールは Settings → Branches にあり、ブランチのパターンごとに 1 つのルールを設定します。ルールセットは Settings → Rules → Rulesets にあり、名前の付いたルールの一覧です。同じブランチを複数のルールセットの対象にできます。

ルールセット同士、またはルールセットとブランチ保護ルールが重なった場合は、最も厳しいものが適用されます。リポジトリの有効なルールセットは読み取り権限があれば誰でも見られるので、開発者は管理者に聞かなくても main に何が必須か確認できます。

必須のステータスチェックには 2 つのモードがあります。GitHub の用語では、strict なチェックはマージ前にブランチがベースブランチに対して最新であることを求め、loose なチェックは求めません。今回のリポジトリでは必須チェックが loose モードになっていて、上の流れは、まさにその隙間で起きたものでした。

Require branches to be up to date before merging

この設定を有効にすると、ベースブランチより遅れているプルリクエストはマージできなくなります。GitHub には Update branch ボタンがあり、押すと最新の main がプルリクエストのブランチに取り込まれて CI が再実行されます。マージはその新しい実行が通るまで待たされます。

上の流れで言えば、B がマージされた時点でプルリクエスト A は止められ、再実行された CI で型エラーが見つかって、main は緑のままだったはずです。

その代償は再実行です。main へのマージが 1 回あるたびに、ほかの開いているプルリクエストはすべて遅れた状態になり、どれもマージ前に更新と CI の再実行が必要になります。

CI が 1 分半ほどで終わる小さなチームなら、1 回のマージあたりの待ち時間は短く済みます。ただ、活発なリポジトリでは更新が積み重なっていき、そこで役に立つのがマージキューです。

設定の場所

この設定は独立したルールではありません。ルールセットでは、Require status checks to pass を有効にするとその下に現れます。従来のブランチ保護ルールでは Require status checks to pass before merging の下にインデントされたチェックボックスで、親のチェックボックスを入れるまで表示されません。

どちらの場合も、必須チェックを 1 つ以上選んでおく必要があります。選んでいなければ、再実行するものがないからです。

提案するだけのチェックボックス

Settings → General → Pull Requests には、名前は似ていても効果の弱い設定があります。

Always suggest updating pull request branches Whenever there are new changes available in the base branch, present an “update branch” option in the pull request.

この設定はボタンを追加するだけです。有効にしても、遅れているプルリクエストはそのままマージできるので、上の流れのマージも止められなかったはずです。

main の基本ルールと、それぞれが防ぐもの

以下は、複数人がプルリクエストをマージするリポジトリで main に設定するルールの出発点です。各行にはそのルールが防ぐものを書いています。防げなくても困らないものは外してください。

今回のリポジトリはルールを管理者以外にだけ適用していたので、ルールを完全にそろえていたとしても、管理者によるマージは防げなかったはずです。

必須チェックは正しいジョブを名前で指定する

必須チェックは名前で照合されます。このリポジトリでは 2 つのワークフローがどちらも check という名前のジョブを持っていました。1 つはアプリケーションのコード用、もう 1 つはインフラのコード用です。ブランチのルールには check という必須チェックが 1 つだけ登録されていました。この一覧だけでは、どちらのワークフローを指しているのか判別できません。

ジョブには app-check や infra-check のように、ワークフローをまたいで一意な名前を付け、それぞれをその名前で必須にしましょう。

必須にしているジョブの名前を変える

必須にしているジョブの名前を変えるときは、作業の順番が大事です。名前を変えるプルリクエストは古い名前を報告しなくなります。そのため、古い名前が必須のままだと、そのプルリクエストは決して届かないチェックを待ち続けます。

先に古い必須設定を外せばマージはできますが、新しい名前を追加するまで main に必須チェックがない状態になります。

main を常に保護したままにできた順番は次のとおりです。

1. 名前を変えるプルリクエストを開き、CI を一度走らせて新しいチェック名を GitHub に認識させます。設定画面には実行されたことのあるチェックしか候補に出ず、検索欄にも “Search for status checks in the last week for this repository” と表示されています。
2. 古い必須チェックを新しい名前に置き換え、同じ編集で Require branches to be up to date before merging を有効にします。
3. 名前を変えるプルリクエストをマージします。その CI はすでに新しい名前を報告しているので、新しい必須条件を満たします。
4. ほかの開いているプルリクエストを更新します。CI が再実行されるまで、新しいチェックは “Expected — Waiting for status to be reported” と表示され、マージはブロックされたままです。

パスフィルター付きのワークフローは外す

paths: フィルターのあるワークフローは必須チェックにできません。GitHub のトラブルシューティングガイドによると、パスフィルターでワークフローがスキップされると、そのチェックは Pending のまま残り、マージをブロックします（“stay in a ‘Pending’ state and block merging”）。そうなると、ドキュメントだけを編集するプルリクエストはいつまでもマージできません。イメージのビルドのようにパスフィルターを付けたワークフローは、必須チェックの一覧から外しておきましょう。

マージキューと、使える条件

マージキューは、更新して再テストする繰り返しを自動化します。作成者がそれぞれ Update branch を押して待つ代わりに、キューが最新の main とキューに入ったプルリクエストで一時的なブランチを作り、そこで CI を走らせ、通った場合にだけマージします。GitHub は、ブランチを最新にすることを必須にするのと同じ保証が得られると説明しています。

条件が 2 つあります。マージキューが使えるのは組織が所有するパブリックリポジトリと、GitHub Enterprise Cloud のプライベートリポジトリだけです。Team プランのプライベートリポジトリでは使えません。また、必須チェックの元になるワークフローはすべて merge_group イベントでトリガーされる必要があります。そうでないと、キューは始まることのないチェックを待ち続けます。

on:
  pull_request:
  merge_group:

マージキューがなくても、ブランチを最新にすることを必須にすれば、更新を手動で行う手間はあっても同じ保護が得られます。

まとめ

プルリクエストの緑のチェックが示すのは、CI が走った時点の main に対してそのプルリクエストが動いたということです。2 つのプルリクエストがそれぞれ自分のスナップショットに対しては緑でも、合わさると main を壊すことがあります。別々のファイルを触っていれば、Git は何も警告しません。

Require branches to be up to date before merging は、その確認をマージの時点に移します。main より遅れているプルリクエストは、先に更新して CI をもう一度通す必要があります。この設定は必須ステータスチェックのルールの下にあり、名前の似た “Always suggest updating pull request branches” はボタンを追加するだけです。

この設定は、プルリクエストとレビューの必須化、一意な名前の必須チェック、管理者のバイパス禁止と組み合わせると最も効果を発揮します。

参考リンク

• About protected branches (GitHub Docs)
• About rulesets (GitHub Docs)
• Available rules for rulesets (GitHub Docs)
• Troubleshooting required status checks, including skipped path-filtered workflows (GitHub Docs)
• Managing a merge queue (GitHub Docs)
• Events that trigger workflows: pull_request and merge_group (GitHub Docs)