From 4fce6ec29234614ad0214fb0e65a750019104a1f Mon Sep 17 00:00:00 2001 From: Alex Chao Date: Thu, 17 Sep 2026 00:27:33 +0800 Subject: [PATCH] chore(norms): let the loop open host PRs, keep merging with the owner The loop may now be the maker of changes to its own host (scripts/loop/**); the owner review is the checker, so a host change is no longer by itself a reason to route a task to needs-triage. The gate that keeps this honest is the ruleset, not the loop's restraint: the Main branch ruleset now carries require_code_owner_review and require_last_push_approval, and .github/CODEOWNERS owns '*' for the repository owner, so a bot approval cannot meet the review requirement. --- .github/CODEOWNERS | 19 +++++++++++++++++++ docs/agents/ops.md | 9 +++++++++ scripts/loop/README.md | 17 ++++++++++++++--- 3 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..4d1bb72 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,19 @@ +# Every change to the default branch needs an approving review from the repository +# owner. Combined with `require_code_owner_review` in the `Main branch` ruleset, this +# is what keeps the loop from merging its own work: the loop may open a pull request +# against any path — including `scripts/loop/**`, the host it runs on — but it cannot +# approve it (GitHub forbids approving your own PR) and a bot approval never +# satisfies this rule, so a human has to say yes. +# +# Pull requests are evaluated against the CODEOWNERS of their *base* branch, so a PR +# cannot rewrite the ownership it is subject to. The GitHub docs also recommend +# owning this file and the directory it lives in explicitly, which is why they are +# listed below: a silent edit to them would otherwise be a way to shed review. +# +# Note for dependabot PRs: they are covered by `*` too, so they need the owner's +# approval before merging (the owner's ruleset bypass still allows merging them +# without a second reviewer). + +* @Alex1990 +/.github/ @Alex1990 +/scripts/ @Alex1990 diff --git a/docs/agents/ops.md b/docs/agents/ops.md index adbef91..070d149 100644 --- a/docs/agents/ops.md +++ b/docs/agents/ops.md @@ -93,6 +93,15 @@ Never do work outside the declared stage; never modify norms-layer files `loop-task: #`. - Don't claim a task whose lock is live; don't fight a live lock. - No self-merge; no norms-layer edits without a PR; no npm publish by the loop. +- Host changes (`scripts/loop/**`) may arrive as loop PRs like any other change: the + loop may be the maker and the owner's review is the checker, so a host change is + not by itself a reason to route a task to `needs-triage`. Review the diff rather + than the run log — the same PR can edit `gh-check.mjs` and the agent prompts, i.e. + the machinery that would otherwise corroborate it. What keeps this honest is the + ruleset, not the loop's restraint: `require_code_owner_review` with + `.github/CODEOWNERS` owning `*` means only the owner's approval meets the review + requirement, so the loop cannot merge its own work. `.github/workflows/**` stays + human-only (next item). - **`main` is protected by the `Main branch` ruleset**, and that — not a credential — is what makes the line above true: every change to `main` must arrive through a pull request carrying one approving review, the only bypass actor is the repository diff --git a/scripts/loop/README.md b/scripts/loop/README.md index 4cab1ff..9791aca 100644 --- a/scripts/loop/README.md +++ b/scripts/loop/README.md @@ -72,7 +72,7 @@ Main branch (ruleset, active, target ~DEFAULT_BRANCH) | Actor | direct push to `main` | merge a PR without an approval | | --- | --- | --- | | the owner (you) | ❌ refused | ✅ (bypass, `pull_request` mode) — **merging *is* the consent** | -| the loop (`github.token`) | ❌ refused — not a bypass actor | ❌ refused — and it cannot approve its own PR | +| the loop (`github.token`) | ❌ refused — not a bypass actor | ❌ refused — it cannot approve its own PR, and a bot approval does not satisfy `require_code_owner_review` | Two consequences worth stating plainly: @@ -203,8 +203,8 @@ This is the part a human owns, and the only part no file in this repo can expres { "type": "pull_request", "parameters": { "required_approving_review_count": 1, "dismiss_stale_reviews_on_push": true, - "require_code_owner_review": false, - "require_last_push_approval": false, + "require_code_owner_review": true, + "require_last_push_approval": true, "required_review_thread_resolution": false, "allowed_merge_methods": ["merge", "squash", "rebase"] } } @@ -228,6 +228,17 @@ to merge a PR they authored themselves. `dismiss_stale_reviews_on_push: true` means a push to a PR branch invalidates its approval: approving a diff approves that diff, not whatever arrives next. +`require_code_owner_review: true`, with `.github/CODEOWNERS` owning `*`, closes the +one path the approval count left open. The loop holds `pull-requests: write`, so +without a code-owner requirement it could approve a PR it did not author — one the +owner opened, or a dependabot PR — and then merge it with the same `Contents: write` +it needs to push branches. Under a code-owner requirement the bot's approval does not +count, so only the owner can satisfy the gate. + +`require_last_push_approval: true` is the second half of the same idea: the person who +pushed the most recent reviewable commit cannot be the one who approves it, so a push +that rewrites a PR branch cannot then be self-approved. + GitHub also sets `require_extra_approval_for_unattributed_changes: true` on rulesets carrying a `pull_request` rule (a public-preview default, not something set here). It applies to Copilot-authored PRs and asks for one approval beyond the configured