Say how to apply the A8 ruleset without locking main, and which rollback exists - #43
Merged
Merged
Conversation
… exists Missed by #42's squash: this was pushed to the branch after the merge had already been cut, so main carries the A8 ruleset without the procedure for putting it live. The A8 entry said a maintainer syncs the committed ruleset and stopped there. Applying it is where the danger is, not writing it. The approval rules and the bypass are separate fields and one can take while the other does not. A PUT that stores required_approving_review_count but resolves the bypass to nothing leaves main unmergeable for everybody, and reading the actor_id back does not detect that -- it only says what was stored, not whether it resolved to a role anybody holds. GitHub answers the real question directly. `current_user_can_bypass` is computed from bypass_actors and says whether the caller can bypass. So the procedure is: load the rules with enforcement disabled, ask that field, and only then enable. Nothing is gated while the answer is being checked. Also records that `enforcement=evaluate` is not a rollback here: it returns 422 below Enterprise. `disabled` is the unlock, confirmed by round-tripping it against the live ruleset.
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.
What this changes, and why
Missed by #42's squash. This was pushed to the branch after the merge commit had already been cut, so
mainnow carries the A8 ruleset without the procedure for putting it live. Docs only — 29 lines indocs/OUTSTANDING.md.#42's A8 entry said a maintainer syncs the committed ruleset and stopped there. Applying it is where the danger is, not writing it.
The approval rules and the bypass are separate fields, and one can take while the other does not. A
PUTthat storesrequired_approving_review_count: 1but resolves the bypass to nothing leavesmainunmergeable for everybody — and reading theactor_idback does not detect that. It only tells you what was stored, not whether it resolved to a role anybody actually holds. That was the open risk #42 flagged and could not close.GitHub answers the real question directly:
That field is computed from
bypass_actorsand states whether the caller can bypass. It turns theactor_id: 5guess from something you hope about into something you check. So the documented procedure is: load the rules with enforcement disabled, ask that field, and only then enable — nothing is gated during the window where the answer is being checked.Also recorded:
enforcement=evaluateis not a rollback on this repository's plan. It returns 422 below Enterprise.disabledis the unlock, confirmed by round-tripping it against the live ruleset rather than assumed.Evidence
git diff --stat origin/main 8b49052showed the 29 lines absent frommainafter Require an independent approval on main, and declare the bypass that makes it possible #42 merged, which is how this was caught.python -m pytest tests/ -qpasses.python tests/test_evasion.py— 34/34.ruff check src tests eval toolspasses.python tools/readme_facts.py --checkpasses — no count moved, since no test was added.python tools/verify.py— 16 passed, 0 failed, 0 not evaluated (run on the identical content before Require an independent approval on main, and declare the bypass that makes it possible #42's merge; this commit is that content rebuilt on the newmain, andgit diffconfirms the tree matches).If this changes detection
src/changes.If this changes an evasion
Anything a reviewer should push back on
tests/test_ci_config.pyexists to prevent for the ruleset itself. A test that diffs the committed ruleset against the live one would catch drift properly, but it needs a token CI does not have. Worth opening separately rather than pretending this closes it.actor_id: 5question is still open. This PR does not resolve whether that is Repository admin; it makes the answer checkable before it can hurt. If the check comes backnever, the committed ruleset needs correcting and that is a follow-up.Generated by Claude Code