An approval that no longer means anything
A 14-developer team protects main with a rule requiring one approving review. A .github/CODEOWNERS file lists the platform team for everything under infrastructure/. On Friday, Dana approves Priya's pull request that changes three Terraform files. On Monday Priya pushes a follow-up commit that rewrites half of the module, then merges the PR. Nobody from the platform team reviewed the final diff. Later that week a database migration pull request merges with the approval of the author's own teammate, who admits afterwards that they only skimmed it.
1. Explain, from the symptoms alone, which protections were probably missing, and how you would confirm each.
Read the worked answer
Two independent gaps are enough to explain the merge.
Stale approvals were not dismissed. Branch protection has a Dismiss stale pull request approvals when new commits are pushed option. If it is off, Dana's Friday approval is still valid after Monday's rewrite, which is exactly what the timeline shows. Confirm in Settings, then Branches or Rulesets, for the rule targeting main.
CODEOWNERS was advisory only. A CODEOWNERS file guarantees a review request, not a merge block. Blocking requires Require review from Code Owners in the protection rule, and the owners referenced must be a team with write access to the repository; if the team name is wrong or the team lacks access, the entry is inert. Confirm by opening a test PR touching infrastructure/ and seeing whether a code owner review is required.
Also rule out a bypass: an administrator merging a protected branch will succeed unless the rule explicitly includes administrators.
2. Design the review configuration you would actually recommend, and state what it costs.
Read the worked answer
For infrastructure/, require code owner review plus at least one general approval, with stale approval dismissal on. Leave the rest of the repository at one approval. That asymmetry is deliberate: platform changes are high blast radius and rare, so the latency is affordable; requiring two approvals on everything would slow a 14-person team for little gain.
Costs to be honest about:
- Dismissal on new commits means a reviewer returning to a PR with three rounds of commits must re-approve. Teams experience this as review churn; mitigate by asking authors to avoid pushing cosmetic fixups after approval.
- Code owner review concentrates merge authority in a small group. If the platform team is two people, PRs stall during holidays, so the rule needs a backup owner or an admin breakout path.
- Apply the rule to administrators as well, or the most senior person on the team becomes the accidental exception.
Then re-check the second incident separately: skimmed approvals are a culture problem, not a configuration one.
3. The team wants to enforce the new rules without disrupting three release-blocking pull requests due this week. What is the rollout?
Read the worked answer
Use a ruleset rather than editing classic branch protection in place, because a ruleset can be created in evaluate mode first. That exposes what would have been blocked before anything is blocked.
Sequence:
- Add the ruleset for
infrastructure/in evaluate mode for three days; review the blocked-merge preview and fix CODEOWNERS typos. - Announce the change in the channel where releases are discussed, with the reason: a code owner did not see a Terraform rewrite that merged.
- Enforce, but exempt nothing by name. Named exemptions become permanent.
- For the three in-flight PRs, get the required reviews this week rather than grandfathering them; the review is the point.
Do not stack a merge queue on top at the same time. One change at a time, so if throughput collapses you know which knob caused it.