Repository navigation
docs: merge approvals defer to the main branch ruleset, not a two-maintainer rule - #5399
Conversation
…ntainer rule The approval count for merging into main is whatever GitHub's ruleset enforces (today: one approving review, including a code owner). Replace the hardcoded "two maintainers" wording in CLAUDE.md (and regenerated AGENTS.md), CONTRIBUTING.md, REVIEW.md and the CODEOWNERS header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Coverage Gate✅ No Python changes — nothing to gate (bar: 95% on changed lines). |
qingyun-wu
left a comment
There was a problem hiding this comment.
Approve at 19ed249. Checked against the live ruleset: gh api repos/sonichi/sutando/rules/branches/main gives required_approving_review_count=1, require_code_owner_review=true, require_last_push_approval=false, dismiss_stale_reviews_on_push=false. So "one approving review, including a code owner" in CONTRIBUTING.md is accurate.
This PR covers every 'two maintainers / 2 approvals' merge-gate statement on main: CODEOWNERS:10, AGENTS.md:108, CLAUDE.md:108, CONTRIBUTING.md:307 and REVIEW.md:320. The remaining hits (CONTRIBUTING.md:286, REVIEW.md:306/376/387/426) describe past incidents or the shared-login counting mechanic, and stay correct with a count of 1. Pointing at the ruleset as the source of truth, instead of restating a number, is the right fix. CI and CLA green.
Nit: CODEOWNERS and CONTRIBUTING.md say "currently 1", which will go stale the same way when the ruleset changes. The gh api pointer in CONTRIBUTING.md is what keeps it honest.
john-the-dev
left a comment
There was a problem hiding this comment.
review-clean pending Sutando Release verification
Reviewed head 19ed24927d09f03749a09c22136fb3742b1b0b91 (full diff: 5 files, +7/-6).
I checked the ruleset directly, and the new text matches it. gh api repos/sonichi/sutando/rulesets returns one ruleset: 19110427 "main", enforcement: active, ref_name.include: [refs/heads/main]. Its pull_request rule sets required_approving_review_count: 1, require_code_owner_review: true, dismiss_stale_reviews_on_push: false, require_last_push_approval: false and require_extra_approval_for_unattributed_changes: true. Classic branch protection on main returns 404, so this ruleset is the only gate. That supports the four claims in the diff: CLAUDE.md/AGENTS.md ("the approvals the main ruleset requires"), CONTRIBUTING ("currently one approving review, including a code owner"), CODEOWNERS ("currently 1") and REVIEW.md lesson 17.
Checks:
- AGENTS.md sync: the CI check
refuse AGENTS.md stale vs CLAUDE.mdpassed, andscripts/agents-md-sync.sh --checkat head printed "up to date". CLAUDE.md is 40959 bytes. gh pr checks: 31 pass, 1 skipping.scripts/review-checks.sh --diff: hardcoded-paths and root-artifacts clean. No .py files changed, so prose-cap scanned nothing.
CODEOWNERS (comment-only change): only the GOVERNANCE header comment changed. No ownership line was added, removed or changed, so no path gains or loses an owner. codeowners/errors is empty at the head sha.
Non-blocking findings:
- [medium] REVIEW.md lesson 17 still depends on the old count. It says
reviewDecision"readsAPPROVEDon a single approval when the PR targets a feature or rescue branch", and the grounding reasons from a "≥2 collaborator approvals" predicate and "actually at 0-of-2". With the count at 1, one counting approval also readsAPPROVEDonmain, so the base-branch part no longer separates the cases. The write-access part is still valid. Suggest one clause, e.g. "(measured when the ruleset required 2)", plus a note that at count 1 only the write-access half applies. - [low] CONTRIBUTING says approvals must be "recorded on the current head", and the ruleset does not enforce that. It has
dismiss_stale_reviews_on_push: falseandrequire_last_push_approval: false, so GitHub counts an approval left on an older head. Keeping this as a team rule is fine (stale approvals were excluded before too). But the sentence also says "The ruleset is the source of truth", so state that this part is policy, not something GitHub enforces. - [low] "currently one approving review" leaves out
require_extra_approval_for_unattributed_changes: true. That rule can raise the effective count when a PR contains unattributed changes. Worth a parenthetical in CONTRIBUTING. - [info] Bypass actors are unverified. The CODEOWNERS header names @sonichi and @qingyun-wu as the only bypass actors. With this token the ruleset returns
bypass_actors: null(reading it needs admin), so I could not confirm that sentence. This PR does not change it. - [info] Remaining "two approvals" mentions describe past events. Grep at head: REVIEW.md:306 and :426 are grounding anecdotes. REVIEW.md:378 and :387 ("a second approval moves the count by zero") are about shared logins and are still correct at count 1. The tests/ and collaboration-intelligence hits are fixtures or unrelated. No script hardcodes 2 (
decision_evidence.pyreadsrequired_approving_review_count). - [info] The owner confirmation is unverified. No link to Chi's decision appears in the body or comments. The ruleset itself confirms the factual claim, so this does not block.
…5399) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…5413) * docs: scope the merge-gate wording to what the main ruleset enforces REVIEW.md lesson 17's grounding measured a two-approval ruleset; say what still holds at one. CONTRIBUTING: the current-head rule is team policy, and other ruleset settings can raise the effective approval count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(review): approval lines defer to the main ruleset (follow-up to #5399) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(review): code-owner caveat on the one-approval case; ruleset-relative approval wording Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(review): state both predicates, including the extra approval for unattributed changes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(review): defer to reviewDecision instead of listing its predicates Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
What changed
The written merge gate said "two maintainers have recorded formal approvals". Owner decision (Chi): the approval requirement for main is whatever the repository's GitHub branch ruleset enforces, not a separate team rule. The docs now defer to the ruleset:
CLAUDE.md(+AGENTS.md, regenerated byscripts/agents-md-sync.sh): "...and it has the approvals themainruleset requires." CLAUDE.md had 3 bytes of headroom under the 40960-byte budget, hence the terse wording; it is now 40959 bytes andagents-md-sync.sh --checkpasses.CONTRIBUTING.md"Apply the complete merge gate": approvals required by the GitHub ruleset formain(currently one approving review, including a code owner), recorded on the current head; how to check it.REVIEW.mdlesson 17: "The ruleset that requires two approvals" -> "The ruleset's approval requirement"..github/CODEOWNERSgovernance header: "2 approvals" -> the count the main ruleset sets (currently 1).Unchanged: comments, bot recommendations, stale approvals and admin bypass still never substitute for the gate. Historical "two approvals" mentions in grounding anecdotes were left as they describe past events. No script or test asserts the two-maintainer rule (grep over docs, skills, scripts, tests).
Evidence:
gh api repos/sonichi/sutando/rules/branches/main[ { "type": "deletion", "ruleset_source_type": "Repository", "ruleset_source": "sonichi/sutando", "ruleset_id": 19110427 }, { "type": "non_fast_forward", "ruleset_source_type": "Repository", "ruleset_source": "sonichi/sutando", "ruleset_id": 19110427 }, { "type": "pull_request", "parameters": { "required_approving_review_count": 1, "dismiss_stale_reviews_on_push": false, "required_reviewers": [], "require_code_owner_review": true, "require_last_push_approval": false, "required_review_thread_resolution": false, "require_extra_approval_for_unattributed_changes": true, "allowed_merge_methods": [ "squash" ] }, "ruleset_source_type": "Repository", "ruleset_source": "sonichi/sutando", "ruleset_id": 19110427 }, { "type": "required_status_checks", "parameters": { "strict_required_status_checks_policy": false, "do_not_enforce_on_create": false, "required_status_checks": [ { "context": "diff coverage >= 95% (python)" }, { "context": "eslint over first-party TS + JS" }, { "context": "license/cla" }, { "context": "refuse AGENTS.md stale vs CLAUDE.md" }, { "context": "refuse bridge tests that read host config" }, { "context": "refuse docs/src-map.md stale vs src/" }, { "context": "refuse new direct workspace-path resolution" }, { "context": "refuse new hardcoded hotkey assertions" }, { "context": "refuse new inline CLAUDE_CONFIG_DIR fallback" }, { "context": "refuse new inline ~/.sutando/ install-path literal" }, { "context": "refuse raw hostname-slug recipe in agent docs" }, { "context": "refuse workspace/ contents in commits" }, { "context": "ruff over Python sources" }, { "context": "shellcheck over shell scripts" }, { "context": "tsc + tests (clean install)" } ] }, "ruleset_source_type": "Repository", "ruleset_source": "sonichi/sutando", "ruleset_id": 19110427 } ]required_approving_review_count: 1,require_code_owner_review: true.🤖 Generated with Claude Code