feat(branch-protection): require review-thread resolution as fleet standard#26
Open
mabry1985 wants to merge 1 commit into
Open
feat(branch-protection): require review-thread resolution as fleet standard#26mabry1985 wants to merge 1 commit into
mabry1985 wants to merge 1 commit into
Conversation
…andard Adds required_review_thread_resolution:true to applyRecommendedDefaults so every ecosystem repo gates merge on resolved review threads. A review comment from Quinn / CodeRabbit / a human can no longer be silently merged past — the thread must be addressed and resolved first. CI-pass alone is not sufficient. Deliberately does NOT force required_approving_review_count: org policy is that bots gate via review decision / thread resolution, not a forced approval a bot identity often can't satisfy. Existing PR-rule params (approval counts, code-owner requirements) are preserved; only thread resolution is flipped on. - lib/branch-protection.mjs: add/patch the pull_request rule, surface threadResolutionBefore/After in the diff, opt-out via requireThreadResolution - bin/apply-branch-protection.mjs: --no-thread-resolution flag + diff output - docs: new "### 3." section in branch-protection-defaults.md - tests: 3 new cases (adds rule, preserves existing PR params, opt-out) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Audit — PR #26 | feat(branch-protection): require review-thread resolution as fleet standard
VERDICT: PASS
CI Status
- Lint + smoke test: ✅ success
Diff Review
Org-wide branch-protection standard: all apply-branch-protection calls now gate on required_review_thread_resolution: true, surfacing the change in the diff output. --no-thread-resolution provides a documented opt-out path.
lib/branch-protection.mjs:applyRecommendedDefaultsadds/patches thepull_requestrule; reads prior state (including when the field isfalse) before writing, so the reported diff is accurate.diff.threadResolutionAfteris set totrueonly when called withrequireThreadResolution: trueor default; when opt-out is chosen it reflects the prior value — no field is created withfalsewhere none existed. Consistent with the doc intent.required_approving_review_countis deliberately set to0when creating a new PR rule, leaving approval counts untouched per org policy.- 3 new test cases cover all branches (no rule → adds, existing rule → updates preserving params, opt-out → no rule added). 16 tests pass.
Observations
- LOW/INFO: When
requireThreadResolution: falsethe function walks away entirely rather than creating apull_requestrule withrequired_review_thread_resolution: false. Intentionally avoids writing field at all — matches the docs — but worth confirming against GitHub API's handling of absent vs. explicit-false fields. - CLAWPATCH: Not mounted for this repo — proceeding on diff evidence alone.
Checks: 5 | Passed: 5 | Failed: 0 | Gaps: 0
— Quinn, QA Engineer
|
Submitted APPROVE review on #26. |
3 tasks
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
1 participant
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
Makes review-thread resolution a fleet branch-protection standard: every ecosystem repo that adopts
apply-branch-protectionnow gates merge onrequired_review_thread_resolution: true. A review comment from Quinn / CodeRabbit / a human can't be silently merged past — the thread must be addressed and resolved first. CI-pass alone is no longer sufficient to merge.This is the org-wide counterpart to the protoMaker local-pipeline work: we review the diff locally before GitHub, and now GitHub itself enforces that review feedback actually gets resolved.
Why no forced approval count
Org policy is that bots gate via review decision / thread resolution, not a forced
required_approving_review_count(which a bot identity often can't satisfy, and which would stall a solo/small-team board). So this flips thread resolution on and leaves approval counts untouched.Changes
lib/branch-protection.mjs—applyRecommendedDefaultsadds/patches thepull_requestrule withrequired_review_thread_resolution: true; preserves any existing PR-rule params; surfacesthreadResolutionBefore/Afterin the diff; opt-out viarequireThreadResolution: false.bin/apply-branch-protection.mjs—--no-thread-resolutionflag + diff line.docs/reference/branch-protection-defaults.md— new "### 3." section.test/branch-protection.test.mjs— 3 new cases (adds rule, preserves existing PR params, opt-out). All pass.Test
🤖 Generated with Claude Code