BOHICA-LABS / BOHICA-LABS/vsdd-factory

bug(cicd-setup): prescribed branch protection adds a 0-approval review object that forces --admin merge on EVERY PR

Open
#321 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2
Forks
1
Avg merge
6h 43m
Merged PRs (30d)
29

Description

## Summary

The branch-protection configuration prescribed by `cicd-setup.md` (architect-produced scaffolding artifact) creates a **vestigial review gate** that makes GitHub report `mergeStateStatus: BLOCKED` for **every** zero-review PR. Because GitHub forbids self-approval, the only merge path becomes `gh pr merge --admin` (or a manual owner merge) on **every single PR** — for the entire life of a solo / small-team project.

## Where the factory prescribes it

`specs/architecture/cicd-setup.md` → `## Branch Protection Requirements for main` instructs the admin to apply:

```json
"required_pull_request_reviews": {
"required_approving_review_count": 0,
"dismiss_stale_reviews": true
}
```

…explicitly described as "Required approving reviews: 0 (solo project; waivable)".

## Why this is a bug, not a config preference

On GitHub, the mere **presence** of the `required_pull_request_reviews` object enables the "review" protection facet, even when `required_approving_review_count: 0`. For a PR with zero submitted reviews this yields:

- `mergeable: MERGEABLE`
- `mergeStateStatus: BLOCKED`
- `reviewDecision: ""` (empty — NOT `REVIEW_REQUIRED`)

A normal `gh pr merge` is refused. GitHub does not allow a PR author to approve their own PR, so on a solo project **no reviewer can ever clear the gate** — the only escape is `--admin` (works only because `enforce_admins: false`) or a manual owner merge in the web UI.

## Verified ruled-out alternatives (single-cause confirmation)

On a representative blocked PR, every other protection facet was confirmed satisfied:
- All required status checks green; `strict: true` and branch 0 commits behind base (no "out of date").
- `required_signatures: true` — all commits `verification.verified == true / reason: valid`.
- `required_conversation_resolution: false`, `enforce_admins: false`.

→ `required_pull_request_reviews` (count 0) is the **sole** remaining cause, by elimination.

## Impact / who hits this

Any operator who follows the factory's documented branch-protection setup on a solo or small-team repo. It is 100% reproducible and recurs on **every** PR. Operationally it also induces a downstream agent pathology: pr-manager (correctly) refuses to self-authorize `--admin` on a protected branch and stalls for a human, so every story delivery requires manual human merge intervention — defeating the autonomous pipeline.

## Suggested fix

In `cicd-setup.md` (and any devops-engineer branch-protection helper), when `required_approving_review_count == 0`, **omit `required_pull_request_reviews` entirely** rather than including it with count 0. "Require a PR before merging" with zero approvals should be expressed via the push-restriction / `No Direct Push to Main` marker check (already present), not via a review object. Alternatively, document that operators must explicitly `null` the review object for solo repos.

## Related (not duplicates)

- #257 (contexts use workflow filename vs check-name) — different root cause, same artifact.
- #227 / #228 (preflight validation of repo settings) — a preflight could *detect* this, but the scaffolding prescription is the source.

_Filed from a downstream pilot; repo slug genericized. No product content included._

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.