BOHICA-LABS / BOHICA-LABS/vsdd-factory

Orchestrator gate-template defaults to 1 approver — deadlocks autonomous pipelines

Open
#346 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 orchestrator's Phase-2 (and similar) human-approval gate offers a "Strict branch protection (Recommended)" option whose intended settings include **1 required approver**. For an autonomous VSDD pipeline where pr-manager is both the PR author and the merger (no separate human-approver identity exists in the loop), a required-approver count > 0 deadlocks every story PR forever. This means strict branch protection cannot be safely enabled without an out-of-band human intervening on every Phase 3 PR.

## Where I encountered it

Running vsdd-factory plugin 1.0.0-rc.21 on a greenfield product (ArcavenAE/akey, Rust, SSH-agent hardware-backed). At the Phase-2 → Phase-3 gate, the orchestrator presented an AskUserQuestion with three options for branch protection scope, including "Strict (Recommended) — PR required + all status checks + 1 review + linear history + block force-pushes + block deletions." I selected it (it was tagged Recommended and matched the stated security posture). pr-manager then attempted the gh api PUT call to apply protection, which failed with permissions (different defect — `arcavenai` service account has `write` not `admin`), and the human caught the 1-approver setting before they applied it manually.

The fix on my side was filing an admin-action gh issue with `required_pull_request_reviews: null` (0 approvers); product repo: ArcavenAE/akey#6.

## Why it's a defect

The orchestrator skill's gate-template biases toward a **human-team workflow**: humans open PRs, different humans review them. In an autonomous VSDD pipeline:

- pr-manager is the PR author for every story PR
- pr-manager is also the merger (calls `gh pr merge` after CI passes)
- There is no separate "reviewer" identity — pr-reviewer/code-reviewer agents post comments but don't approve via the GitHub Approval mechanism (and even if they did, they're invoked by the same service account)
- Therefore `required_approving_review_count > 0` means every PR is blocked until a human manually approves it

The safety surface in autonomous pipelines is carried by:
1. Status checks (CI: fmt/clippy/build/test/deny — all green required)
2. Linear history (no merge-commit history pollution)
3. Force-push and deletion blocks
4. Required conversation resolution

These do not require an approval count to be effective. A human reviewing on cadence (weekly, end-of-wave, end-of-phase) is the appropriate human-in-the-loop, not per-PR approval.

## Proposed fix

The orchestrator gate-template should branch on product topology:

| Scenario | Required approvers | Rationale |
|---|---|---|
| Human-team product (multiple human committers, vsdd-factory used as scaffolding only) | 1 (or N depending on team size) | Standard GitHub flow |
| Autonomous pipeline (pr-manager is the merger; vsdd-factory drives the full cycle) | **0** | pr-manager has no separate approver identity in the loop |
| Hybrid (humans + pr-manager both commit) | 1 with pr-manager allowlisted via CODEOWNERS bypass | More complex; defer to user input |

The Phase-2 gate's branch-protection question should detect autonomy posture (likely from `.factory/STATE.md` `mode:` field plus presence of pr-manager in the agent fleet) and default the "Recommended" option to 0 approvers when autonomous, or surface the question explicitly: "pr-manager is configured as your merger — set required approvers to 0 (no per-PR human review) or 1+ (per-PR human approval required, will halt the autonomous loop)?"

The current template phrasing — "1 review + linear history" — bundles a self-conflicting setting with safe ones; the linear-history + status-checks settings are correct, the 1-review setting is the bug.

## Severity

P2 — silent foot-gun. Doesn't crash the pipeline at the time of dispatch; surfaces later when the first story PR sits unmerged waiting for an approval that will never come. Easy to misdiagnose as "pr-manager isn't actually trying to merge" when in fact GitHub is rejecting the merge due to protection.

## Related

- Engine yield process-gap: drbothen/vsdd-factory#343 (filed earlier this session by the same orchestrator)
- akey product-side admin-action issue: ArcavenAE/akey#6
- Recorded as PG-5 in the akey factory's HANDOFF-PHASE-3-ENTRY.md

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.