OSAC-3734: remove classic branch protection required_status_checks - #184
Merged
omer-vishlitzky merged 1 commit intoAug 10, 2026
Conversation
This block was added in c17e367 to make Tide respect E2E checks (Tide only reads classic branch protection, not rulesets). With Tide being replaced by merge queue, it's no longer needed. The hardcoded strict=true conflicts with merge queue — GitHub docs explicitly warn: "You should not combine the merge queue with the 'Require branches to be up to date before merging' option in branch protection rules, as this can lead to unexpected behavior." This caused PRs to be repeatedly ejected from the merge queue even after all checks passed. Required status checks are still enforced via the repository ruleset (with strict=false, since the queue handles freshness). Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: omer-vishlitzky <omer.vishlitzky@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughThe classic branch protection resource no longer configures required status-check contexts. Status checks remain configured through the repository ruleset. ChangesBranch protection status checks
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Remove the
required_status_checksblock from classic branch protection. It was added in c17e367 to make Tide respect E2E checks — Tide only reads classic branch protection, not rulesets.With merge queue replacing Tide, this block is unnecessary and actively harmful: its hardcoded
strict = trueconflicts with merge queue, causing PRs to be ejected from the queue after all checks pass.From GitHub docs:
Evidence
PRs 180, 212, 182 were repeatedly ejected from the merge queue despite all checks passing. Every ejection correlated with main moving (a bypassed merge of PR 172 at 18:14 caused ejection of all three at 19:15).
What still enforces checks
The repository ruleset (
ci-status-checks) has the same required checks withstrict = false— merge queue handles freshness by testing against latest main.Test plan
gh api repos/osac-project/osac/branches/main/protection/required_status_checksreturnsnull🤖 Generated with Claude Code
Summary by CodeRabbit