feat(repo): make review a code-owner gate, not a blanket approval - #63
Merged
Conversation
The main-branch ruleset required one approving review on every pull request, and CODEOWNERS opened with a `*` catch-all naming the only maintainer. Since GitHub cannot request a review from a pull request's author, that combination blocked every pull request the maintainer opened, and every one was merged by spending the OrganizationAdmin bypass — #42, #45, #47, #48, #52, #53, #55 and #57 all carry `reviewDecision: REVIEW_REQUIRED`. A control satisfied on no pull request and stepped over on all of them enforced nothing, while still charging every contributor the wait. Adopt the mechanism musher-dev/platform already uses, so that moving between the two repositories does not mean relearning when review is required: pair `required_approving_review_count: 0` with `require_code_owner_review: true`, and reduce CODEOWNERS to the two paths that define the gate itself. A pull request touching no owned path now merges on green CI; one touching `.github/CODEOWNERS` or `.github/rulesets/` still needs its owner. Both halves break silently — a catch-all restores the blanket gate from one side, a count of 1 from the other — so `tools/src/rulesets.ts` (RUL-01..RUL-09) keeps them in step from inside the existing Lint job. RUL-09 has no counterpart upstream: it rejects a required status check that no workflow publishes, or that a `paths:`-filtered workflow publishes, which are the two ways a required context hangs a pull request forever. It is also why the check runs inside Lint rather than as a workflow of its own. Reconcile two drifts the committed JSON had accumulated since 2026-08-08, both in the file's favour: `Signed off` is now genuinely a required check rather than only a declared one, and release-tags no longer grants the OrganizationAdmin bypass its own documentation says it does not have. GitHub waives the code-owner requirement for a pull request's author, which with a single owner is the whole gate on the paths that matter most. codeowners-notice.yml posts a sticky comment on self-owned edits so the waiver is visible; it is a notifier and must never become a required check. GOVERNANCE.md advertised approval as mechanical in three places and CONTRIBUTING.md in one; they now say plainly which obligations are enforced and which are asked for. ADR 0015 records the trade, including that owning specifications/ becomes worth revisiting the moment there is a second maintainer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Justin Merrell <merrelljustin@gmail.com>
You own some of the paths this pull request changes@justinmerrell, this pull request touches paths you are a code owner for. Your authorship waives the code-owner review requirement — GitHub cannot request a review from a pull request's author — so nobody is automatically notified about these changes:
Double-check they are intentional. If you want a second pair of eyes, request a review manually. (Automated notice from the codeowners-notice workflow — ADR 0015.) |
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 changes
No contract change. This replaces the blanket "one approving review on every
pull request" rule with the selective code-owner gate
musher-dev/platformalready runs:
.github/rulesets/main-branch.jsonpairsrequired_approving_review_count: 0withrequire_code_owner_review: true, and.github/CODEOWNERSdrops its*catch-all down to the two paths that definethe gate itself. A pull request touching no owned path merges on green CI; one
touching
.github/CODEOWNERSor.github/rulesets/still needs its owner.tools/src/rulesets.ts(task check:rulesets, RUL-01..RUL-09) keeps the twohalves in step, a new
codeowners-noticeworkflow makes self-owned editsvisible, and ADR 0015 plus three GOVERNANCE.md passages record what is now
enforced versus asked for.
The live rulesets have already been updated — see "Applied live" below.
Why
The old rule was never satisfied.
@justinmerrellis the only owner, GitHubcannot request a review from a pull request's author, and the ruleset grants
OrganizationAdminan always-bypass. So every pull request they opened arrivedblocked and was merged by spending the bypass: #42, #45, #47, #48, #52, #53, #55
and #57 all carry
reviewDecision: REVIEW_REQUIRED. A control exercised only bybypassing it enforces nothing, trains the bypass as a normal step, and still
charges every other contributor the wait.
ADR 0015
records the decision, the alternatives, and the cost — including that owning
specifications/becomes worth revisiting the moment there is a secondmaintainer.
Two drifts reconciled, both in the file's favour
The committed JSON had not been re-applied since 2026-08-08:
main-branch.jsondeclared a fourth required check,Signed off; live hadonly three. The DCO gate that
dco.yml, CONTRIBUTING.md and the pull requesttemplate all promise was not actually required. It is now.
release-tags.jsondeclaredbypass_actors: [], with a paragraph inRULESETS.md arguing that an escape hatch nobody may legitimately use is one
an attacker inherits. Live granted
OrganizationAdminbypass anyway. Removed.RUL-09 goes beyond the upstream version
Platform documents in prose that a required context must exist and must not live
in a
paths:-filtered workflow, and enforces neither. RUL-09 enforces both —they are the two ways a required check hangs a pull request forever. It is also
why the validator runs inside the existing
Lintjob rather than as its ownworkflow: a path-filtered validator would be the bug it exists to catch.
Applied live
Both rulesets were
PUTfrom the exact JSON committed here, so the files andGitHub are in step as of this pull request:
Verified against the aggregate view, which is the only one that accounts for
org-level and classic rules folding in —
gh api repos/musher-dev/spec/rules/branches/mainreports a single
pull_requestrule, sourced frommusher-dev/spec, withrequired_approving_review_count: 0,require_code_owner_review: true,require_last_push_approval: false. Classic branch protection onmainreturns404, andspecis deliberately absent from the orgpr-workflowruleset.Note that CODEOWNERS on
mainstill carries the*catch-all until this merges,so every path stays owned until then.
Compatibility
Checklist
task checkpasses locally (including the newcheck:rulesets)schemas/dist/untouched — no schema changespec.mdaffectedrepo(releases nothing)Each of RUL-01..RUL-09 was verified to fire by deliberately breaking it: a
*catch-all, a stale CODEOWNERS path, a malformed owner, the approval count back
at
1,require_last_push_approval: true, a leaked server-sideid, a typo'drequired context, and a required context pointing at the path-filtered
ShellCheckjob.Review note
This pull request touches both owned paths, and its author owns both — GitHub
will waive the code-owner requirement. That is exactly the case
codeowners-notice.ymlexists to make visible, and it should post its stickycomment on this pull request. A second pair of eyes is worth requesting manually
here, since the diff changes who is allowed to change it.