Skip to content

protect-main requires no status checks: a red PR can still be merged by hand #116

Description

@jimisola

Follow-up to #111, which fixed Renovate's side and explicitly left this open.

The gap

protect-main enforces deletion, non_fast_forward, required_linear_history and pull_request. It has no required_status_checks rule, on any repo. GitHub is enforcing "someone approved it" and "history stays linear"; it has no opinion about whether the code builds.

So a reviewer can approve a PR whose build is red — or still running — and merge it. GitHub warns but does not block, because there is no rule to block with.

#111 fixed this for Renovate by setting platformAutomerge: false, so Renovate now waits for the branch to be green outright. That works — PR reqstool/reqstool-client#451 sat unmerged for a day on a red build, then merged itself once green. But that gate lives in renovate.json5, not in the ruleset:

Renovate's gate (fixed in #112) GitHub's gate (missing)
Lives in renovate.json5 the ruleset
Covers Renovate's PRs every PR, every actor
Removed by editing one config line changing the ruleset

Dropping Renovate's ruleset bypass in .github-private#34 was the setup for this: any check made required now applies to Renovate too. That work is done and currently buys nothing, because there are no required checks to apply.

Survey of all 15 active repos

Every repo carrying protect-main has the identical four rules and no required checks. Two are special:

  • .github has no ruleset at all. It is on classic branch protection, with required_status_checks enabled but contexts: [] — the mechanism on, the list empty. It also excludes itself from safe-settings.
  • .github-private can have neither. Rulesets and branch protection both return 403 Upgrade to GitHub Pro or make this repository public — a private repo on a free org. It cannot be protected at all.

The build/test context name differs per repo, which is why one shared list cannot work:

repo build/test contexts
.github Lint workflows, Test — composite actions, Test — release artifacts, Test — $/ resolves…, validate / Validate Renovate config
reqstool-ai validate
reqstool-client build, Reuse linting job / linting
reqstool-demo build (main), build (pypi), validate-openspec / …
reqstool-java-annotations, -java-maven-plugin build (main), build (pypi), Check linting / linting, validate-openspec / …
reqstool-java-gradle-plugin build (main), build (pypi), validate-openspec / …
reqstool-python-decorators, -hatch-plugin, -poetry-plugin build (main), build (pypi), Reuse linting job / linting, validate-openspec / …
reqstool-regression Validate (fixtures/parent) + three ecosystem legs
reqstool-typescript-tags build / build, lint / lint, reqstool (main), reqstool (pypi), validate-openspec / …
reqstool-vscode build / build, lint / lint
reqstool.github.io (none)

The trap that makes "just require everything" wrong

A required context that never reports does not fail the PR — it blocks it forever, showing Expected — waiting for status to be reported.

There is exactly one context in this org that would do that, and it is easy to miss because it looks like every other green check:

renovate/stability-days reports only on Renovate's own PRs. Comparing the same repo:

  • reqstool-client#451 (Renovate): 9 contexts, including renovate/stability-days
  • reqstool-client#452 (human): the same 8, without it

Require it, and every human PR on that repo blocks permanently. It appears on the most recent PR of 11 of 15 repos, so any list built by reading "what did the last PR report" walks straight into it.

Two smaller ones:

  • check / Validate PR title reports twice in 7 repos — once pass, once skipping. Two runs claiming one context name.
  • CodeQL reports skipping in 9 of 15 and pass in 5. GitHub counts a skipped check as passing, so requiring it buys less than it appears to.

And a standing cost: required contexts couple the ruleset to job names, so renaming a CI job becomes a breaking change until the ruleset is updated to match.

How the config works (verified, not assumed)

safe-settings merges arrays by name — NAME_FIELDS = ['name', 'username', 'actor_id', 'login', 'type', 'key_prefix', 'context'] in lib/mergeDeep.js — and rules merge by type. So a per-repo file at safe-settings/repos/<repo>.yml appends a required_status_checks rule to protect-main without restating the other four.

I confirmed this by running safe-settings' own mergeArrayBy against the real config: one ruleset out, all five rule types present, and bypass_actors and the pull_request parameters preserved intact.

The rule schema requires both required_status_checks and strict_required_status_checks_policy under parameters — omitting either fails validation, the same way pull_request needs required_review_thread_resolution.

Proposed rollout

One repo at a time, each verified before the next:

  1. reqstool-client as the pilot — the contexts are known-good, having been observed on both a human and a Renovate PR. PR to follow.
  2. Confirm the next real PR on that repo behaves: green PR mergeable, red PR blocked, no context stuck on Expected.
  3. Repeat per repo, deriving each list the same way — the intersection of a human PR and a Renovate PR, never the union.
  4. reqstool.github.io gets nothing; it has no build.
  5. .github needs its classic protection converted or its empty context list filled — worth doing regardless, and more so if chore(safe-settings): move org configuration here from .github-private #115 lands, since it would then hold the config that protects every other repo while being the least protected itself.

OrganizationAdmin keeps its always bypass throughout, so the admin override is unaffected.

Open question

strict_required_status_checks_policy — whether a PR must be up to date with main before merging. false in the pilot to change one variable at a time. true prevents "green on a stale base, breaks main on merge", but forces a rebase whenever main moves, and with prConcurrentLimit: 10 that is a lot of Renovate churn. Worth deciding deliberately rather than inheriting the default.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions