Skip to content

Proposeal: Custom review gating with @delegate support #83

@cgwalters

Description

@cgwalters

Summary

Implement a custom review gating system that:

  1. Replaces GitHub's native "required reviews" with a custom approval-gate check
  2. Adds bors-style @delegate support, allowing maintainers to delegate PR approval rights to non-maintainers for a specific PR

This enables "drive-by contributions" where trusted community members can effectively approve PRs on behalf of maintainers.

Motivation

GitHub's native branch protection has a fundamental limitation: reviews from non-maintainers don't count toward required review counts. There's no API to make a non-maintainer's approval "count."

This creates friction for:

  • Drive-by contributions: A trusted community member reviews a PR thoroughly but their approval doesn't "count"
  • Domain expertise: Someone with deep knowledge of a specific area (but not a maintainer) can't formally approve changes
  • Reducing maintainer burden: Maintainers must personally approve every PR even when a trusted reviewer has already done thorough review

The bors @delegate pattern solves this: a maintainer vouches for a specific reviewer on a specific PR, temporarily granting them approval authority.

Proposed Behavior

Delegation Commands

Command Description
/delegate+ Delegate approval rights to the PR author
/delegate=@username Delegate approval rights to a specific user
/delegate=@user1,@user2 Delegate approval rights to multiple users
/undelegate Revoke delegation

Workflow

  1. Maintainer delegates: Comments /delegate=@contributor
  2. Bot confirms: Replies with confirmation
  3. Delegated user approves: Submits a GitHub review with "Approve"
  4. Check passes: The approval-gate (or org-required-checks) status turns green
  5. Merge proceeds: Standard GitHub merge once all checks pass

Key Properties

  • Delegation is per-PR only - no permanent permission changes
  • Only maintainers (write access) can delegate
  • Delegation is revocable via /undelegate
  • All actions are auditable via PR comment history
  • Delegated users cannot further delegate

Technical Approach

Core Idea: Replace Native Reviews with Custom Check

Instead of relying on GitHub's "Require X approving reviews" ruleset, we:

  1. Disable (or set to 0) native required reviews in the org ruleset
  2. Add a custom approval-gate status check that implements our own approval logic
  3. Require this check to pass before merge

This gives us full control over what constitutes a valid approval.

Architecture: Org-wide vs Repo-specific Checks

┌─────────────────────────────────────────────────────────────┐
│  Org-wide Ruleset (bootc-dev)                               │
│    Requires BOTH checks on default branch:                  │
│      ├── "required-checks"       (repo-specific CI)         │
│      └── "org-required-checks"   (org-wide policy)          │
└─────────────────────────────────────────────────────────────┘
           │                              │
           ▼                              ▼
┌─────────────────────────┐    ┌──────────────────────────────┐
│  Repo: ci.yml           │    │  Org: org-required-checks.yml│
│  (repo-specific)        │    │  (synced via sync-common)    │
│                         │    │                              │
│  on: pull_request       │    │  on:                         │
│                         │    │    pull_request              │
│  Jobs:                  │    │    pull_request_review       │
│    - build              │    │    issue_comment             │
│    - test               │    │                              │
│    - lint               │    │  Jobs:                       │
│    - required-checks    │    │    - approval-gate           │
│      (sentinel)         │    │    - (other org checks)      │
│                         │    │    - org-required-checks     │
│                         │    │      (sentinel)              │
└─────────────────────────┘    └──────────────────────────────┘

Why This Architecture?

Separation of concerns:

  • required-checks = repo-specific (build, test, lint - varies per repo)
  • org-required-checks = org-wide policy (same for all repos)

Benefits:

  • Adding org-wide checks doesn't touch individual repos
  • Repos focus on their own CI
  • Single source of truth for org policy in bootc-dev/infra
  • Automatic sync via existing sync-common mechanism

Avoids re-running CI:

  • ci.yml only triggers on pull_request (code changes)
  • org-required-checks.yml triggers on comments/reviews too, but only runs lightweight approval check

Future extensibility: The org-required-checks workflow can incorporate other org-wide checks (OpenSSF scorecard, DCO, license checks, etc.) into the same sentinel pattern.

Implementation Components

1. Org-wide Ruleset

Configure at org level (Settings → Repository → Rulesets):

  • Target: All repositories
  • Target branches: Default branch
  • Rule: Require status checks to pass
    • required-checks (repo-specific CI sentinel)
    • org-required-checks (org-wide policy sentinel)
  • Rule: Required reviews = 0 (or disable - we handle this ourselves)

2. Org-wide Workflow: org-required-checks.yml

# common/.github/workflows/org-required-checks.yml
name: Org Required Checks

on:
  pull_request:
    types: [opened, synchronize, reopened]
  pull_request_review:
    types: [submitted, dismissed]
  issue_comment:
    types: [created, edited, deleted]

jobs:
  approval-gate:
    runs-on: ubuntu-latest
    # Only run on PRs (issue_comment fires for issues too)
    if: github.event.pull_request || github.event.issue.pull_request
    steps:
      - uses: bootc-dev/infra/actions/approval-gate@main
        with:
          token: ${{ secrets.GITHUB_TOKEN }}

  # Future: other org-wide checks can be added here

  # Sentinel job
  org-required-checks:
    if: always()
    needs: [approval-gate]
    runs-on: ubuntu-latest
    steps:
      - run: exit 1
        if: needs.approval-gate.result != 'success'

3. Reusable Action: bootc-dev/infra/actions/approval-gate

# actions/approval-gate/action.yml
name: 'Approval Gate'
description: 'Check for maintainer approval or valid delegation'
inputs:
  token:
    description: 'GitHub token'
    required: true
outputs:
  approved:
    description: 'Whether the PR is approved'
  approved-by:
    description: 'Username who approved'
  delegated-by:
    description: 'Maintainer who delegated (if applicable)'
runs:
  using: 'composite'
  steps:
    - name: Check approval status
      shell: bash
      env:
        GH_TOKEN: ${{ inputs.token }}
        PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }}
        REPO: ${{ github.repository }}
      run: |
        set -euo pipefail
        
        # Get collaborators with write access (maintainers)
        MAINTAINERS=$(gh api "repos/$REPO/collaborators?permission=push" --jq '.[].login')
        
        # Get PR reviews
        APPROVALS=$(gh api "repos/$REPO/pulls/$PR_NUMBER/reviews" \
          --jq '[.[] | select(.state == "APPROVED")] | .[].user.login')
        
        # Check if any maintainer approved
        for approver in $APPROVALS; do
          if echo "$MAINTAINERS" | grep -qx "$approver"; then
            echo "Approved by maintainer: $approver"
            exit 0
          fi
        done
        
        # Check for delegation
        DELEGATE_CMD=$(gh api "repos/$REPO/issues/$PR_NUMBER/comments" \
          --jq '[.[] | select(.body | test("@delegate[+=]"))] | last')
        
        if [ -n "$DELEGATE_CMD" ]; then
          DELEGATOR=$(echo "$DELEGATE_CMD" | jq -r '.user.login')
          
          # Verify delegator is a maintainer
          if echo "$MAINTAINERS" | grep -qx "$DELEGATOR"; then
            # Parse delegatee from command
            DELEGATEE=$(echo "$DELEGATE_CMD" | jq -r '.body' | \
              grep -oP '@delegate[+=]@?\K[\w-]+')
            
            # Check if delegatee approved
            if echo "$APPROVALS" | grep -qx "$DELEGATEE"; then
              echo "Approved by delegatee: $DELEGATEE (delegated by $DELEGATOR)"
              exit 0
            fi
          fi
        fi
        
        echo "No valid approval found"
        exit 1

Detecting Maintainers

Use the Collaborators API to check for users with push (write) or admin permission:

gh api "repos/$REPO/collaborators?permission=push" --jq '.[].login'

This matches GitHub's native permission model and requires no configuration.

State Management

No external state needed - the action parses PR state on each run:

  1. Fetch collaborators with write access (maintainers)
  2. Fetch all PR reviews
  3. Check if any maintainer approved → pass
  4. Parse comments for @delegate commands from maintainers
  5. Check if delegatee approved → pass
  6. Otherwise → fail

Stateless and idempotent.

Event Handling

Event What triggers it Action behavior
pull_request.opened PR created Check for approvals (usually none)
pull_request.synchronize New commits pushed Re-check (may need re-approval)
pull_request_review.submitted Review added Check if approver is maintainer or delegatee
pull_request_review.dismissed Review removed Re-check remaining approvals
issue_comment.created Comment added Parse for @delegate commands
issue_comment.edited Comment edited Re-parse
issue_comment.deleted Comment deleted Re-check (delegation may be revoked)

Rollout Plan

  1. Phase 1: Create approval-gate action in bootc-dev/infra/actions/
  2. Phase 2: Create org-required-checks.yml workflow in common/.github/workflows/
  3. Phase 3: Test in bootc-dev/infra repo
  4. Phase 4: Sync to all repos via sync-common
  5. Phase 5: Create org-wide ruleset requiring both required-checks and org-required-checks
  6. Phase 6: Disable native "required reviews" in org ruleset (our check handles it)
  7. Phase 7: Document usage

Security Considerations

  • Trust model: Maintainers must trust delegated users to responsibly approve
  • Audit trail: All @delegate commands visible in PR comment history
  • No privilege escalation: Delegated users can only approve the specific PR
  • Revocable: @undelegate or deleting the comment removes delegation
  • No bypass: Works within GitHub's native protection system (status checks)
  • Commit invalidation: New commits re-trigger the check (stale approvals may need re-review)

Alternatives Considered

  1. Keep native reviews + add delegation as extra check: Doesn't solve the core problem - non-maintainer reviews still don't count
  2. Bypass-actor merge bot: More powerful but bypasses GitHub's security model entirely
  3. Add trusted contributors as collaborators: Grants too much permanent access
  4. Third-party tools (Mergify/Kodiak): Can't implement custom delegation logic

Open Questions

  • Should delegation auto-expire on new commits pushed to the PR?
  • Implementation language: Composite action (bash/gh CLI) vs Node.js vs Rust?
  • How to handle CODEOWNERS - does delegatee approval satisfy code owner requirements?
  • Should the action post a comment confirming delegation?

Future Work

The org-required-checks workflow provides a natural place to add other org-wide checks:

  • OpenSSF scorecard gate (consolidate existing openssf-scorecard-gate.yml)
  • DCO (Developer Certificate of Origin) check
  • License compliance check
  • etc.

These would be added as additional jobs in the workflow, with the sentinel job aggregating them.

Prior Art

Related Work

  • Fits alongside existing rebase.yml workflow pattern
  • Synced via existing sync-common mechanism
  • Uses existing bootc-dev Bot identity for comments (if we add confirmation replies)
    toolbx(rhel-toolbox-stream10) ~/src/github/bootc-dev/infra> opencode -c
    ^[[<51;244;38M
    toolbx(rhel-toolbox-stream10) ~/src/github/bootc-dev/infra> cat /tmp/review.md

Custom review gating with @delegate support

Summary

Enable maintainers to temporarily empower non-maintainers to approve specific PRs, following rust-lang/bors's delegation model.

GitHub's native required reviews only counts approvals from users with write access. We replace this with a custom approval-gate status check implementing our own approval logic, including delegation.

Commands

  • @delegate+ - delegate to the PR author
  • @delegate=@user or @delegate=@user1,@user2 - delegate to specific user(s)
  • @undelegate - revoke

Architecture

flowchart TB
    subgraph ruleset["Org-wide Ruleset"]
        rc["required-checks<br/>(repo-specific CI)"]
        orc["org-required-checks<br/>(approval-gate + future checks)"]
    end

    subgraph repo["Each Repo"]
        ci["ci.yml<br/>on: pull_request"]
        org["org-required-checks.yml<br/>on: pull_request, review, comment"]
    end

    ci --> rc
    org --> orc
Loading

org-required-checks.yml lives in common/.github/workflows/ and syncs to all repos. Triggers on pull_request, pull_request_review, and issue_comment to run the approval check.

Workflow

# common/.github/workflows/org-required-checks.yml
on:
  pull_request:
    types: [opened, synchronize, reopened]
  pull_request_review:
    types: [submitted, dismissed]
  issue_comment:
    types: [created, edited, deleted]

jobs:
  approval-gate:
    runs-on: ubuntu-latest
    if: github.event.pull_request || github.event.issue.pull_request
    steps:
      - uses: bootc-dev/infra/actions/approval-gate@main
        with:
          token: ${{ secrets.GITHUB_TOKEN }}

  org-required-checks:
    if: always()
    needs: [approval-gate]
    runs-on: ubuntu-latest
    steps:
      - run: exit 1
        if: needs.approval-gate.result != 'success'

The approval-gate action

Stateless - parses PR state on each run:

  1. Fetch collaborators with write access (maintainers)
  2. Fetch PR reviews
  3. Pass if maintainer approved OR (maintainer delegated AND delegatee approved)

Maintainers detected via collaborators API (permission=push).

Rollout

  1. Create approval-gate action in bootc-dev/infra/actions/
  2. Create org-required-checks.yml in common/.github/workflows/
  3. Test in bootc-dev/infra, then sync to all repos
  4. Configure org-wide ruleset requiring both checks; disable native required reviews

Open questions

  • Command syntax: @delegate=@user vs @bot delegate=@user vs /delegate @user?
  • Should delegation expire on new commits?
  • Implementation: composite action (bash) vs Node.js vs Rust?

Prior art

  • bors-ng / homu
  • bootc-dev/bootc ci.yml sentinel job pattern

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