Skip to content

master has no branch protection — a red commit landed and blocked the whole dependency queue for ~12 hours #524

Description

@JabbaKadabra

What

The master branch has no branch protection rule at all, so no status check is required before a merge:

$ gh api repos/NordsteinSoftware/Proxytrace/branches/master/protection
{"message":"Branch not protected","status":"404"}

CI runs on every push to master, but nothing acts on the result. A merge whose checks fail is indistinguishable, at merge time, from one whose checks pass.

Where

Repository settings (Branches → protection rules) for master. The checks that exist and would need to be required are defined in .github/workflows/ci.yml (backend, frontend, image) and .github/workflows/e2e.yml (e2e).

Why it matters

This is not hypothetical — it already cost real time, and the cost fell on unrelated work.

Commit 7072eb28 landed on master with a broken frontend build (the assistant-ui 0.15 migration left TypeScript errors, so npm run build exited 2 inside the Docker image build). Both CI and E2E failed on the master push:

2026-08-05T18:09 | CI  | failure | 7072eb28
2026-08-05T18:09 | E2E | failure | 7072eb28

It stayed broken for roughly twelve hours until 4dcefcca (#521) fixed it. During that window every open PR branched from a base that could not build, so all three open Dependabot PRs (#518, #519, #520) showed red image and e2e checks that had nothing to do with the dependencies they bumped.

The concrete costs:

  • Wasted CI capacity — every PR run rebuilt a known-broken base, including full e2e stack boots at ~8 minutes each.
  • Misleading signal — the failures pointed at frontend/src/features/tracey/**, not at the bumped packages. Triage cost was spent proving the PRs were innocent rather than evaluating them.
  • A stalled queue — the dependency PRs were unmergeable for two days, and clearing them required a full rebase cycle on all three.
  • Masked real regressions — while master is red, a genuine failure introduced by a new PR is indistinguishable from the pre-existing breakage.

Suggested fix

Add a branch protection rule on master requiring status checks to pass before merge. The minimum set that would have caught 7072eb28:

  • backend (ci.yml)
  • image (ci.yml) — this is the job that actually failed, since it builds the frontend into the Docker image
  • e2e (e2e.yml)

Note that frontend and image are path-gated via the changes job, so the rule should be checked against how the gating reports skipped jobs — a skipped required check can block merges if configured naively. ci.yml already handles this with a changes job that "fails open", so confirm the skip states report as neutral/success rather than pending before turning the requirement on.

Worth deciding separately whether to enable "Require branches to be up to date before merging". It closes the semantic-conflict gap, but forces a rebase of every open PR after each merge, which is meaningful churn for a queue of Dependabot PRs. Requiring the checks without strict up-to-date would have been sufficient to prevent this incident.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions