Skip to content

Branch protection defaults: keep bot reviewers OUT of required_status_checks #10

@mabry1985

Description

@mabry1985

Companion to #6 (strict policy default). Same surface (docs/branch-protection-defaults.md + the eventual scripts/apply-branch-protection.sh), different rule.

The friction

LLM review bots like CodeRabbit and protoquinn often end up in required_status_checks because they post status checks alongside their reviews. Treating those checks as required creates a single point of failure on an external service:

  • If the bot is rate-limited or down, every PR blocks.
  • If the bot's webhook races with a fast-merging PR (CI finishes before the bot starts), the PR sits mergeStateStatus: BLOCKED indefinitely until a maintainer manually nudges the bot.

Hit live in protoLabsAI/protoMaker today on PR #3744: all CI green in <2 min, CodeRabbit hadn't posted a status, PR sat BLOCKED until a manual @coderabbitai review comment. Tracked as protoLabsAI/protoMaker#3745.

Recommendation

required_status_checks should gate on correctness signals, not advisory ones:

Belongs in required Doesn't belong in required
build (compiles) CodeRabbit (LLM advisory review)
test (tests pass) protoquinn if it posts status checks
checks (lint/format) Any external service whose outage shouldn't halt merges
ci-complete (rollup gate)

Bot reviewers still gate merges via reviewDecision (CHANGES_REQUESTED blocks; APPROVED clears). That's the right level for advisory signals — the bot has explicit say without holding the merge button hostage.

Pairs nicely with the protoMaker auto-dismiss maintenance check (#3744 merged) which clears stale CHANGES_REQUESTED reviews from bots that re-review with COMMENTED instead of escalating to APPROVED. That eliminates the other half of the bot-review friction.

Concrete proposal for release-tools

When this work is done alongside #6:

docs/branch-protection-defaults.md should call out

A "do not include in required_status_checks" list:

  • LLM review bots (CodeRabbit, protoquinn-style agents)
  • Any third-party service whose availability isn't owned by the team
  • Advisory-only checks (e.g. review if it posts as a status check)

Plus a "do include" list with the four-check minimum: build, test, checks, ci-complete.

scripts/apply-branch-protection.sh (proposed in #6) should:

Sample ruleset JSON

"required_status_checks": {
  "strict_required_status_checks_policy": false,
  "required_status_checks": [
    {"context": "build"},
    {"context": "test"},
    {"context": "checks"},
    {"context": "ci-complete"}
  ]
}

(Note: integration_id is repo-specific. The script should resolve it at apply time, not hardcode it.)

Out of scope

  • Whether to also keep review in/out — depends on what the "review" check actually reports per repo. Should be configurable, default include if it's a CI-style rollup.
  • Existing repos with bot checks already required — needs a per-repo audit + cleanup. Add to the apply script as a --audit mode.

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationenhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions