ci: replace classic branch protection with a bypassable ruleset - #61
Merged
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c2d348e6-f8ab-4742-abd5-29d1c927b42e) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe PR adds an owner-authored bypass to protected-controls and replaces classic ChangesProtected controls and repository ruleset
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant ProtectedControls
participant GitHub
PullRequest->>ProtectedControls: Submit protected-path change
ProtectedControls->>GitHub: Read author and configured owner
GitHub-->>ProtectedControls: Owner identity
ProtectedControls-->>PullRequest: Pass owner-authored change
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_autorelease.py (1)
874-883: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover all ordering boundaries in the authorization test.
The workflow has a second trusted-automation branch for
autorelease/(event|eol-complete)-*, followed by thereviews = api(...)lookup. Assert that the owner bypass precedes both locations.Proposed test additions
self.assertLess(owner_pass, protected.index('re.fullmatch(r"autorelease/evidence-')) + self.assertLess( + owner_pass, + protected.index('re.fullmatch(r"autorelease/(event|eol-complete)-'), + ) + self.assertLess(owner_pass, protected.index("reviews = api("))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_autorelease.py` around lines 874 - 883, Extend test_protected_controls_pass_owner_authored_changes_before_bot_exemptions to assert that the owner_pass position precedes the second trusted-automation branch matching autorelease/(event|eol-complete)-* and the subsequent reviews = api(...) lookup, while retaining the existing no-protected-path and first automation-branch ordering assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_autorelease.py`:
- Around line 874-883: Extend
test_protected_controls_pass_owner_authored_changes_before_bot_exemptions to
assert that the owner_pass position precedes the second trusted-automation
branch matching autorelease/(event|eol-complete)-* and the subsequent reviews =
api(...) lookup, while retaining the existing no-protected-path and first
automation-branch ordering assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 460a24b8-c24e-4a4f-9c49-7592afdd344b
📒 Files selected for processing (4)
.github/workflows/protected-controls.ymldocs/repository-settings.mdscripts/configure-github-autoreleasetests/test_autorelease.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/admin-state/php-bin-after.json`:
- Around line 115-135: Update the ruleset snapshot generation for the
protect_main ruleset (id 20378719) to fetch the full ruleset details via the
repository ruleset GET endpoint before serializing rulesets. Regenerate the
snapshot so conditions, bypass_actors, and rules are included while preserving
the existing metadata.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6377c69-9396-4b9a-aacf-32cd6014d4ea
📒 Files selected for processing (1)
docs/admin-state/php-bin-after.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/snapshot-github-admin-state (1)
117-122: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDocument the
rulesetsshape.Keep
schemaVersion: 1; the v1 after fixture already storesconditions,rules, andbypass_actors. Add an explicit schema or fixture assertion for this detailed shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/snapshot-github-admin-state` around lines 117 - 122, Update the rulesets fixture handling around the rulesets collection to explicitly assert or document the v1 shape, including conditions, rules, and bypass_actors, while preserving schemaVersion: 1 and the existing single_document generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/snapshot-github-admin-state`:
- Around line 117-122: Update the rulesets fixture handling around the rulesets
collection to explicitly assert or document the v1 shape, including conditions,
rules, and bypass_actors, while preserving schemaVersion: 1 and the existing
single_document generation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 478fdf2c-0b4e-4650-94be-33759667d982
📒 Files selected for processing (2)
docs/admin-state/php-bin-after.jsonscripts/snapshot-github-admin-state
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/admin-state/php-bin-after.json
The configure script now installs a protect_main ruleset (same PR, check, and history rules) with repository administrators as bypass actors in pull_request mode, and removes classic protection along with the enforce_admins toggle the old merge procedure depended on. Protected controls now passes owner-authored PRs: a solo owner cannot approve their own PR, so that requirement was unsatisfiable and every such merge ended in an administrative bypass regardless. The gate keeps its full strength against non-owner authors.
The ruleset list endpoint returns metadata only, so the snapshot could not show drift in conditions, rules, or bypass actors now that the ruleset is the merge protection. Each ruleset is re-fetched individually and both snapshots are regenerated.
loadinglucian
force-pushed
the
ci/ruleset-merge-protection
branch
from
August 4, 2026 13:20
b395c30 to
f71e13c
Compare
loadinglucian
added a commit
to Bigpixelrocket/mise-php
that referenced
this pull request
Aug 4, 2026
## What Companion to Bigpixelrocket/php-bin#61. - `.github/workflows/protected-controls.yml`: owner-authored PRs now pass. A solo owner cannot approve their own PR, so the exact-head owner-review requirement was unsatisfiable for owner-authored changes and always ended in an administrative bypass. The short-circuit sits before the readiness-record exemption, so it cannot widen what the automation identity may merge; all non-owner authors still require the exact-head owner approval. - `docs/repository-settings.md`: documents the `protect_main` ruleset model (installed by php-bin's configure script) with repository administrators as bypass actors in `pull_request` mode, replacing classic branch protection and its enforce_admins toggle. - `test/test_autorelease.py`: pins the owner short-circuit's position relative to the automation exemption. ## Notes - `protected-controls.yml` runs on `pull_request_target`, so the owner-pass behavior takes effect only after this merges; this PR itself shows the old red-by-design failure. - Admin-state snapshot will be refreshed once the ruleset migration has been applied. ## Verification `./scripts/test.sh` passes (23 tests). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated repository controls to support streamlined handling of owner-authored changes. - Added comprehensive main-branch protections, including pull requests, required reviews and checks, resolved threads, linear history, and safeguards against force pushes and branch deletion. - Updated protected-path validation and approval requirements. - **Documentation** - Updated repository settings documentation and administrative state records to reflect the new controls. - **Tests** - Added regression coverage for protected-control evaluation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
1 participant
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.
Migrates merge protection from classic branch protection to a protect_main repository ruleset; see commit message for details.
Summary by CodeRabbit
New Features
Bug Fixes