diff --git a/.claude/skills/webjs-start-work/SKILL.md b/.claude/skills/webjs-start-work/SKILL.md index 065d403e8..8bcc1b04e 100644 --- a/.claude/skills/webjs-start-work/SKILL.md +++ b/.claude/skills/webjs-start-work/SKILL.md @@ -298,7 +298,7 @@ Beyond review findings, proactively record the *reasoning* behind a PR as commen ### Merge is gated on green CI, enforced at the branch level, not by trust -A PR must not merge until all CI checks pass. `main` branch protection requires the five `ci.yml` checks (Conventions, Unit+integration, Browser, E2E, Build) before any merge. If `gh api repos/webjsdev/webjs/branches/main/protection` shows `required_status_checks: null`, run `bash scripts/protect-main.sh` once (needs repo admin) to restore it. Do not work around a red or pending check. Wait for green, and fix whatever is red before merging. +A PR must not merge until all CI checks pass. `main` branch protection requires the six `ci.yml` checks (Conventions, Unit+integration, Browser, E2E, Build, In-repo app tests) before any merge, plus one approving review from a CODEOWNER (`.github/CODEOWNERS` names @vivek7405 for every path, so that means the maintainer). If `gh api repos/webjsdev/webjs/branches/main/protection` shows `required_status_checks: null`, run `bash scripts/protect-main.sh` once (needs repo admin) to restore it. Do not work around a red or pending check. Wait for green, and fix whatever is red before merging. CI is read ONCE, at merge, never in a mid-work sleep loop. Read `gh pr checks` in full rather than trusting the merge button to have judged for you, because `ci.yml` defines roughly twice as many jobs as `main` requires, so the non-required ones are held by this instruction rather than by anything that can refuse a merge. diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 000000000..f41e5f028 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,13 @@ +# Every path in this repo is owned by the maintainer. +# +# This exists to give branch protection something to key on. `main` requires +# one approving review, and with `require_code_owner_reviews` turned on in +# scripts/protect-main.sh, only an approval from an owner listed here counts +# toward it. Without this file that setting matches nothing and the gate is a +# no-op, so the two are a pair: neither works alone. +# +# A drive-by contributor approving another contributor's PR therefore does not +# unblock a merge. Their review is still welcome and still visible, it just is +# not the one the gate is waiting for. + +* @vivek7405 diff --git a/scripts/protect-main.sh b/scripts/protect-main.sh index aee6a5dcc..384c90b34 100755 --- a/scripts/protect-main.sh +++ b/scripts/protect-main.sh @@ -13,6 +13,12 @@ # locking solo PRs. Flip enforce_admins to true only once a second reviewer # (a human or a bot account) exists to provide the non-author approval. # +# NOTE on require_code_owner_reviews: it is paired with .github/CODEOWNERS, +# which names @vivek7405 for every path. Without that file the setting matches +# nothing and silently does nothing, so the two ship together. With both in +# place, the one required approval must come from the maintainer, which is +# what stops two drive-by contributors from approving each other onto main. +# # bash scripts/protect-main.sh # # The contexts below must match the `name:` of each job in ci.yml exactly, @@ -40,10 +46,10 @@ gh api -X PUT "repos/${REPO}/branches/main/protection" \ "required_pull_request_reviews": { "required_approving_review_count": 1, "dismiss_stale_reviews": false, - "require_code_owner_reviews": false + "require_code_owner_reviews": true }, "restrictions": null } JSON -echo "main is now protected: 1 approving review + all CI checks required before merge." +echo "main is now protected: 1 approving CODEOWNER review + all CI checks required before merge."