From 78fd8b0514b7cdd7e01708a2ac338d360d8bd699 Mon Sep 17 00:00:00 2001 From: SatoshiQaziMuhammed Date: Sat, 1 Aug 2026 10:31:51 -0700 Subject: [PATCH] ops: stop requiring an approval that nobody can give (#34) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule said one approving review. On a four-person team where the author is usually the only person awake, that meant every merge went through `gh pr merge --admin` — so the requirement was not producing review, it was producing a habit of bypassing the branch rule. A control everyone routes around is worse than no control: it still shows up green in an audit. Zero required approvals, and the rest of the protection stays exactly as it was. Nothing lands on main without a PR; nothing merges with red CI. That is the part a team this size actually gets value from, and it costs no waiting. What was being protected here was never code review. It was that nothing reaches production unnoticed, and that funds cannot move on one person's say-so. Both live elsewhere: production waits on the environment gate, and fund movement is going behind on-chain multisig — where a stolen account cannot forge a second signature the way a collected token can forge a second approval. Drift check passes against live settings. --- ops/README.md | 17 ++++++++++++++++- ops/apply-repo-settings.sh | 4 ++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/ops/README.md b/ops/README.md index cbf2caa6..0c260f41 100644 --- a/ops/README.md +++ b/ops/README.md @@ -28,7 +28,7 @@ Requires `gh` authenticated with admin rights on the repo. |---|---|---| | required check | `CI Gate ✅` | aggregate job — fails if web, backend or security-audit did not succeed | | strict | true | branch must be up to date with `main` before merging | -| approvals | 1 | no unreviewed code on `main` | +| approvals | 0 | the PR is the gate, not a second reader — see below | | dismiss stale reviews | true | an approval does not carry over to new commits | | force pushes | blocked | history cannot be rewritten | | deletions | blocked | `main` cannot be deleted | @@ -40,6 +40,21 @@ covers them. Listing each job here as well would mean editing this file whenever one is renamed — and a rename would then quietly drop a requirement instead of failing loudly. +### Why zero required approvals + +Requiring an approval on a four-person team means every change waits on somebody +else being awake. That cost buys less than it looks like: the thing worth +protecting here is not code review — it is that nothing reaches production +unnoticed, and that funds cannot move on one person's say-so. Those live +elsewhere. Production deploys wait on the `production` environment, and +fund-moving operations are being put behind on-chain multisig, where a stolen +account or a compromised host cannot forge a second signature the way a +collected token can forge a second approval. + +What the branch rule still buys, at zero waiting: nothing lands on `main` +without a PR, and nothing merges with red CI. That is the part a four-person +team actually gets value from. + `enforce_admins` stays `false` on purpose: an admin needs a way through during a real incident. It is an escape hatch, not the normal path — rare, visible, and recorded in the branch's protection history. diff --git a/ops/apply-repo-settings.sh b/ops/apply-repo-settings.sh index 85bf3a5a..0dfc26b0 100755 --- a/ops/apply-repo-settings.sh +++ b/ops/apply-repo-settings.sh @@ -45,7 +45,7 @@ read -r -d '' PROTECTION <<'JSON' || true "required_pull_request_reviews": { "dismiss_stale_reviews": true, "require_code_owner_reviews": false, - "required_approving_review_count": 1, + "required_approving_review_count": 0, "require_last_push_approval": false }, "restrictions": null, @@ -89,7 +89,7 @@ rev = (d.get('required_pull_request_reviews') or {}) want = { 'required checks': (checks.get('contexts'), ['CI Gate ✅']), 'strict': (checks.get('strict'), True), - 'approvals': (rev.get('required_approving_review_count'), 1), + 'approvals': (rev.get('required_approving_review_count'), 0), 'dismiss stale': (rev.get('dismiss_stale_reviews'), True), 'force pushes blocked': (not (d.get('allow_force_pushes') or {}).get('enabled'), True), 'deletions blocked': (not (d.get('allow_deletions') or {}).get('enabled'), True),