ci(codeowners): un-trap required checks, auto-render, generate owner tables (#142)

The CODEOWNERS required checks blocked every PR — the real root cause was a
name mismatch, compounded by a path filter:

- branch-protection.json required the contexts `CODEOWNERS / drift` and
  `CODEOWNERS / noedit` (the GitHub UI "workflow / job-id" display form), but
  the jobs report check-run names from their `name:` fields — "CODEOWNERS
  matches source" / "CODEOWNERS not hand-edited". The required contexts
  therefore never matched any reported check and sat permanently pending.
- The workflow was also path-filtered to CODEOWNERS files, so it didn't even
  run for most PRs.

Net effect: with both required checks unsatisfiable, every PR could only land
via admin override (e.g. #140).

Fixes:
- A: drop the `paths:` filter so the workflow runs on every PR and both
  required contexts always report.
- name fix: point branch-protection.json at the actual job names verbatim, and
  add a doc note that the contexts must equal the job `name:` values.
- B: the `drift` job now re-renders and, on same-repo PRs, auto-commits the
  regenerated artifacts back to the branch (mirrors the openapi.json job in
  ci.yml); forks / manual runs strict-check instead. Contributors no longer
  run the script by hand.
- D: render-codeowners.py also generates a "who owns what" path->owners +
  roles table spliced into docs/dev/codeowners.md between markers, so the
  human-readable view never drifts. Idempotent; CODEOWNERS output unchanged.
- docs: correct the stale `enforce_admins: true` line (JSON and live are
  false).

NOTE: the branch-protection.json change only takes effect after an admin runs
`./scripts/apply-branch-protection.sh` (deliberate manual step, per
docs/dev/branch-protection.md). Until then `main` still requires the old
mismatched contexts, so this PR itself needs an admin-override merge — the last
one that should be necessary.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Andrew Altshuler 2026-06-06 18:09:47 +03:00 committed by GitHub
parent 96dbe9dec0
commit c7365bf8ef
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 168 additions and 32 deletions

View file

@ -7,8 +7,8 @@
"Check AGENTS.md Links", "Check AGENTS.md Links",
"Test Workspace", "Test Workspace",
"Test omnigraph-server --features aws", "Test omnigraph-server --features aws",
"CODEOWNERS / drift", "CODEOWNERS matches source",
"CODEOWNERS / noedit" "CODEOWNERS not hand-edited"
] ]
}, },
"enforce_admins": false, "enforce_admins": false,

View file

@ -1,10 +1,14 @@
#!/usr/bin/env python3 #!/usr/bin/env python3
"""Render .github/CODEOWNERS from .github/codeowners-roles.yml. """Render .github/CODEOWNERS and the ownership tables in
docs/dev/codeowners.md from .github/codeowners-roles.yml.
The yml is the source of truth editing CODEOWNERS directly is The yml is the source of truth. This script expands the role-based yml
rejected by CI (see .github/workflows/codeowners.yml). This script into (1) the flat pathowners format GitHub expects in
expands the role-based yml into the flat pathowners format GitHub `.github/CODEOWNERS`, and (2) the "who owns what" markdown tables spliced
expects. between the generated-region markers in `docs/dev/codeowners.md`. Both are
derived artifacts; CI re-renders them on every PR (see
.github/workflows/codeowners.yml) and auto-commits the result on same-repo
PRs, so the source of truth and the human-readable view never drift.
Usage: Usage:
python3 .github/scripts/render-codeowners.py python3 .github/scripts/render-codeowners.py
@ -16,6 +20,7 @@ Exits non-zero on:
one owner; otherwise CODEOWNERS would assign nobody and GitHub one owner; otherwise CODEOWNERS would assign nobody and GitHub
would silently fall back to "no required reviewer", which would silently fall back to "no required reviewer", which
defeats the purpose). defeats the purpose).
- Missing generated-region markers in docs/dev/codeowners.md.
""" """
from __future__ import annotations from __future__ import annotations
@ -34,6 +39,13 @@ except ImportError:
REPO_ROOT = Path(__file__).resolve().parents[2] REPO_ROOT = Path(__file__).resolve().parents[2]
SOURCE = REPO_ROOT / ".github" / "codeowners-roles.yml" SOURCE = REPO_ROOT / ".github" / "codeowners-roles.yml"
OUTPUT = REPO_ROOT / ".github" / "CODEOWNERS" OUTPUT = REPO_ROOT / ".github" / "CODEOWNERS"
DOCS = REPO_ROOT / "docs" / "dev" / "codeowners.md"
# The "who owns what" tables in docs/dev/codeowners.md are spliced between
# these markers so the human-readable view never drifts from the source of
# truth. Edit codeowners-roles.yml and re-render — never the table by hand.
DOCS_BEGIN = "<!-- BEGIN GENERATED OWNERSHIP — edit codeowners-roles.yml + run render-codeowners.py -->"
DOCS_END = "<!-- END GENERATED OWNERSHIP -->"
BANNER = """\ BANNER = """\
# AUTOGENERATED from .github/codeowners-roles.yml. Do not edit by hand. # AUTOGENERATED from .github/codeowners-roles.yml. Do not edit by hand.
@ -75,6 +87,62 @@ def owners_for(role_names: list[str], roles: dict) -> list[str]:
return seen return seen
def _oneline(text: str) -> str:
"""Collapse a folded/multi-line YAML description into one cell of text."""
return " ".join((text or "").split())
def ownership_tables(spec: dict, roles: dict) -> str:
"""Render the human-readable "who owns what" markdown — a path→owners
table (the operative view at PR time, in last-match-wins order with the
catch-all first) plus a rolemembers table. Spliced into the docs between
the markers so it is always current with the source of truth."""
out: list[str] = []
out.append("**Path → owners** (GitHub applies *last match wins*; the `*` "
"catch-all is listed first and is overridden by the specific "
"patterns below it):")
out.append("")
out.append("| Path | Owners | Role(s) |")
out.append("|---|---|---|")
if "default" in spec:
owners = " ".join(owners_for(spec["default"], roles))
out.append(f"| `*` | {owners} | {', '.join(spec['default'])} |")
for pattern, role_names in (spec.get("paths") or {}).items():
owners = " ".join(owners_for(role_names, roles))
out.append(f"| `{pattern}` | {owners} | {', '.join(role_names)} |")
out.append("")
out.append("**Roles**:")
out.append("")
out.append("| Role | Members | Description |")
out.append("|---|---|---|")
for name, role in roles.items():
members = " ".join(f"@{m}" for m in (role.get("members") or []))
out.append(f"| `{name}` | {members} | {_oneline(role.get('description', ''))} |")
out.append("")
return "\n".join(out)
def splice_docs(table_md: str) -> None:
"""Replace the region between DOCS_BEGIN/DOCS_END in the docs file with the
freshly generated tables, leaving surrounding prose untouched."""
if not DOCS.exists():
sys.exit(f"error: docs file not found: {DOCS}")
text = DOCS.read_text()
if DOCS_BEGIN not in text or DOCS_END not in text:
sys.exit(
f"error: ownership markers not found in {DOCS.relative_to(REPO_ROOT)}. "
f"Add the lines:\n {DOCS_BEGIN}\n {DOCS_END}\n"
f"around the generated table region."
)
head, rest = text.split(DOCS_BEGIN, 1)
_, tail = rest.split(DOCS_END, 1)
new = f"{head}{DOCS_BEGIN}\n\n{table_md}\n{DOCS_END}{tail}"
DOCS.write_text(new)
def main() -> int: def main() -> int:
if not SOURCE.exists(): if not SOURCE.exists():
sys.exit(f"error: source file not found: {SOURCE}") sys.exit(f"error: source file not found: {SOURCE}")
@ -127,6 +195,9 @@ def main() -> int:
OUTPUT.write_text(rendered) OUTPUT.write_text(rendered)
print(f"wrote {OUTPUT.relative_to(REPO_ROOT)}") print(f"wrote {OUTPUT.relative_to(REPO_ROOT)}")
splice_docs(ownership_tables(spec, roles))
print(f"updated {DOCS.relative_to(REPO_ROOT)}")
return 0 return 0

View file

@ -1,19 +1,24 @@
name: CODEOWNERS name: CODEOWNERS
# Runs on EVERY pull request (no paths filter). The two jobs below are
# required status checks on `main`; a path-filtered required check never
# reports for PRs outside the filter and leaves them permanently "pending"
# (the trap that forced admin-override merges). Always-run + cheap
# short-circuit is what keeps them honest.
on: on:
pull_request: pull_request:
paths:
- '.github/codeowners-roles.yml'
- '.github/CODEOWNERS'
- '.github/scripts/render-codeowners.py'
- '.github/workflows/codeowners.yml'
workflow_dispatch: workflow_dispatch:
# Read-only; we never push from this workflow. # `drift` auto-commits the regenerated artifacts back to same-repo PR
# branches, so it needs write access.
permissions: permissions:
contents: read contents: write
jobs: jobs:
# NOTE: the job `name:` values below ("CODEOWNERS matches source" /
# "CODEOWNERS not hand-edited") ARE the status-check contexts that
# .github/branch-protection.json must list verbatim. Renaming a job here
# is a branch-protection change — update the JSON and re-apply.
drift: drift:
name: CODEOWNERS matches source name: CODEOWNERS matches source
runs-on: ubuntu-latest runs-on: ubuntu-latest
@ -28,19 +33,56 @@ jobs:
- name: Install PyYAML - name: Install PyYAML
run: pip install pyyaml run: pip install pyyaml
- name: Re-render CODEOWNERS - name: Re-render CODEOWNERS + ownership docs
run: python3 .github/scripts/render-codeowners.py run: python3 .github/scripts/render-codeowners.py
- name: Reject drift # Same-repo PR: push the regenerated artifacts back so contributors
# never have to run the script locally. Mirrors the openapi.json
# auto-commit in ci.yml (separate shallow clone of the head branch so
# the pushed commit carries only the regenerated files).
- name: Commit regenerated artifacts to PR branch
if: |
github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.full_name == github.repository
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: | run: |
if ! git diff --quiet .github/CODEOWNERS; then if git diff --quiet -- .github/CODEOWNERS docs/dev/codeowners.md; then
echo "::error::.github/CODEOWNERS is out of sync with .github/codeowners-roles.yml." echo "CODEOWNERS and ownership docs already in sync."
echo "::error::Run \`python3 .github/scripts/render-codeowners.py\` locally and commit the result." exit 0
fi
tmp=$(mktemp -d)
git clone --depth 1 --branch "${{ github.head_ref }}" \
"https://x-access-token:${GITHUB_TOKEN}@github.com/${{ github.repository }}.git" \
"$tmp"
cp .github/CODEOWNERS "$tmp/.github/CODEOWNERS"
cp docs/dev/codeowners.md "$tmp/docs/dev/codeowners.md"
cd "$tmp"
if git diff --quiet -- .github/CODEOWNERS docs/dev/codeowners.md; then
echo "Head branch already matches; nothing to push."
exit 0
fi
git config user.name "github-actions[bot]"
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
git add .github/CODEOWNERS docs/dev/codeowners.md
git commit -m "chore: regenerate CODEOWNERS + ownership docs"
git push
# Fork PR / workflow_dispatch: cannot push back, so enforce drift
# strictly. The contributor runs the script and commits the result.
- name: Verify in sync (forks / manual runs)
if: |
!(github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.full_name == github.repository)
run: |
if ! git diff --quiet -- .github/CODEOWNERS docs/dev/codeowners.md; then
echo "::error::Generated CODEOWNERS / ownership docs are out of sync with .github/codeowners-roles.yml."
echo "::error::Run \`python3 .github/scripts/render-codeowners.py\` and commit the result."
echo "--- diff ---" echo "--- diff ---"
git --no-pager diff .github/CODEOWNERS git --no-pager diff -- .github/CODEOWNERS docs/dev/codeowners.md
exit 1 exit 1
fi fi
echo "CODEOWNERS is in sync with its source." echo "Generated artifacts are in sync with their source."
noedit: noedit:
name: CODEOWNERS not hand-edited name: CODEOWNERS not hand-edited
@ -52,6 +94,8 @@ jobs:
fetch-depth: 0 fetch-depth: 0
- name: Reject hand-edits to generated file - name: Reject hand-edits to generated file
# Only meaningful for PRs (needs a base to diff against).
if: github.event_name == 'pull_request'
run: | run: |
base="origin/${{ github.base_ref }}" base="origin/${{ github.base_ref }}"
git fetch origin "${{ github.base_ref }}" --quiet git fetch origin "${{ github.base_ref }}" --quiet

View file

@ -8,7 +8,7 @@ This page explains what the policy says and how to change it.
| Setting | Value | Why | | Setting | Value | Why |
|---|---|---| |---|---|---|
| **Required status checks (strict)** | `Classify Changes`, `Check AGENTS.md Links`, `Test Workspace`, `Test omnigraph-server --features aws`, `CODEOWNERS / drift`, `CODEOWNERS / noedit` | Every PR must pass workspace tests, AGENTS.md link integrity, and the CODEOWNERS hygiene checks. `strict: true` requires the branch to be up-to-date with `main` before merge. | | **Required status checks (strict)** | `Classify Changes`, `Check AGENTS.md Links`, `Test Workspace`, `Test omnigraph-server --features aws`, `CODEOWNERS matches source`, `CODEOWNERS not hand-edited` | Every PR must pass workspace tests, AGENTS.md link integrity, and the CODEOWNERS hygiene checks. The two CODEOWNERS contexts must equal the job `name:` values in `.github/workflows/codeowners.yml` **verbatim** — a context naming a job that never reports (the old `CODEOWNERS / drift` used the job *id*, and the job was path-filtered) leaves every PR permanently pending and forces admin overrides. `strict: true` requires the branch to be up-to-date with `main` before merge. |
| **Required approving reviews** | `1` | At least one reviewer. With a 2-person team, going higher would block all merges when one person is unavailable. | | **Required approving reviews** | `1` | At least one reviewer. With a 2-person team, going higher would block all merges when one person is unavailable. |
| **Require code-owner reviews** | `true` | The reviewer must be a code owner per `.github/CODEOWNERS`. This is what makes the codeowners chassis enforced. | | **Require code-owner reviews** | `true` | The reviewer must be a code owner per `.github/CODEOWNERS`. This is what makes the codeowners chassis enforced. |
| **Dismiss stale reviews on new commits** | `true` | A push after approval invalidates the prior review. Prevents the "approve, then sneak in unreviewed changes" pattern. | | **Dismiss stale reviews on new commits** | `true` | A push after approval invalidates the prior review. Prevents the "approve, then sneak in unreviewed changes" pattern. |
@ -16,7 +16,7 @@ This page explains what the policy says and how to change it.
| **Disallow force pushes** | `true` | No history rewrites on `main`. | | **Disallow force pushes** | `true` | No history rewrites on `main`. |
| **Disallow branch deletions** | `true` | `main` cannot be deleted. | | **Disallow branch deletions** | `true` | `main` cannot be deleted. |
| **Required conversation resolution** | `true` | All review comment threads must be resolved before merge. | | **Required conversation resolution** | `true` | All review comment threads must be resolved before merge. |
| **Enforce on admins** | `true` | Even repository admins go through the gates. The point is no bypasses. | | **Enforce on admins** | `false` | Admins can override the gates (`enforce_admins: false` in the JSON). This is the intended escape hatch for the 2-person team; tightening to `true` is tracked under hardening below. |
| **Required signed commits** | not yet | Not enabled. Would lock out maintainers until everyone enrolls GPG/SSH commit signing. Tracked as a follow-up. | | **Required signed commits** | not yet | Not enabled. Would lock out maintainers until everyone enrolls GPG/SSH commit signing. Tracked as a follow-up. |
## How to apply ## How to apply

View file

@ -4,24 +4,45 @@
This setup gives every role change a reviewable PR and a permanent in-repository audit trail (`git log .github/codeowners-roles.yml`). This setup gives every role change a reviewable PR and a permanent in-repository audit trail (`git log .github/codeowners-roles.yml`).
## Current roles ## Who owns what
| Role | Members | Scope | The tables below are **generated** from `.github/codeowners-roles.yml` by `.github/scripts/render-codeowners.py` (the same render that produces `.github/CODEOWNERS`). They are the always-current "who owns what at this commit" view — don't edit them by hand; edit the yml and re-render.
<!-- BEGIN GENERATED OWNERSHIP — edit codeowners-roles.yml + run render-codeowners.py -->
**Path → owners** (GitHub applies *last match wins*; the `*` catch-all is listed first and is overridden by the specific patterns below it):
| Path | Owners | Role(s) |
|---|---|---| |---|---|---|
| `engineering` | `@ragnorc` | All code under `crates/**`, repository infrastructure, default for unmapped paths | | `*` | @ragnorc | engineering |
| `docs` | `@ragnorc` | `docs/**`, README.md, AGENTS.md, CLAUDE.md, SECURITY.md | | `crates/**` | @ragnorc | engineering |
| `docs/**` | @ragnorc | docs |
| `README.md` | @ragnorc | docs |
| `AGENTS.md` | @ragnorc | docs |
| `CLAUDE.md` | @ragnorc | docs |
| `SECURITY.md` | @ragnorc | docs |
GitHub treats multiple owners in a CODEOWNERS line as **"any one of them satisfies the review requirement"**. To require N distinct approvers on a specific path, layer a CI check on top (not currently configured). **Roles**:
| Role | Members | Description |
|---|---|---|
| `engineering` | @ragnorc | All production code under crates/**. Engine, CLI, server, compiler. |
| `docs` | @ragnorc | Documentation under docs/**, plus repo-level docs (README.md, AGENTS.md, CLAUDE.md symlink, SECURITY.md). |
<!-- END GENERATED OWNERSHIP -->
GitHub treats multiple owners on a CODEOWNERS line as **"any one of them satisfies the review requirement"**. To require N distinct approvers on a specific path, layer a CI check on top (not currently configured).
## How to change role membership or path mappings ## How to change role membership or path mappings
1. Edit `.github/codeowners-roles.yml`. 1. Edit `.github/codeowners-roles.yml`.
2. Run `python3 .github/scripts/render-codeowners.py` (requires PyYAML; `pip install pyyaml`). 2. Open a PR. **CI re-renders for you**: the `CODEOWNERS` workflow regenerates `.github/CODEOWNERS` and the ownership tables above and auto-commits them back to your PR branch on same-repository PRs — you don't have to run the script locally (though you can: `python3 .github/scripts/render-codeowners.py`, requires PyYAML).
3. Commit both files in the same PR.
On a fork (where CI can't push back), the workflow instead fails with the diff so you can run the script and commit it yourself.
CI fails the PR if: CI fails the PR if:
- `CODEOWNERS` was edited without a corresponding yml change, or - a fork PR left a generated artifact out of sync, or
- The yml was changed but the rendered `CODEOWNERS` doesn't match. - `CODEOWNERS` was edited without a corresponding yml change (the `CODEOWNERS not hand-edited` check).
## How to add a new role ## How to add a new role