diff --git a/.github/branch-protection.json b/.github/branch-protection.json index 61b7d33..7ca46b9 100644 --- a/.github/branch-protection.json +++ b/.github/branch-protection.json @@ -7,8 +7,8 @@ "Check AGENTS.md Links", "Test Workspace", "Test omnigraph-server --features aws", - "CODEOWNERS / drift", - "CODEOWNERS / noedit" + "CODEOWNERS matches source", + "CODEOWNERS not hand-edited" ] }, "enforce_admins": false, diff --git a/.github/scripts/render-codeowners.py b/.github/scripts/render-codeowners.py index f243d0c..5e96545 100755 --- a/.github/scripts/render-codeowners.py +++ b/.github/scripts/render-codeowners.py @@ -1,10 +1,14 @@ #!/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 -rejected by CI (see .github/workflows/codeowners.yml). This script -expands the role-based yml into the flat path→owners format GitHub -expects. +The yml is the source of truth. This script expands the role-based yml +into (1) the flat path→owners format GitHub expects in +`.github/CODEOWNERS`, and (2) the "who owns what" markdown tables spliced +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: python3 .github/scripts/render-codeowners.py @@ -16,6 +20,7 @@ Exits non-zero on: one owner; otherwise CODEOWNERS would assign nobody and GitHub would silently fall back to "no required reviewer", which defeats the purpose). + - Missing generated-region markers in docs/dev/codeowners.md. """ from __future__ import annotations @@ -34,6 +39,13 @@ except ImportError: REPO_ROOT = Path(__file__).resolve().parents[2] SOURCE = REPO_ROOT / ".github" / "codeowners-roles.yml" 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 = "" +DOCS_END = "" BANNER = """\ # 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 +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 role→members 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: if not SOURCE.exists(): sys.exit(f"error: source file not found: {SOURCE}") @@ -127,6 +195,9 @@ def main() -> int: OUTPUT.write_text(rendered) print(f"wrote {OUTPUT.relative_to(REPO_ROOT)}") + + splice_docs(ownership_tables(spec, roles)) + print(f"updated {DOCS.relative_to(REPO_ROOT)}") return 0 diff --git a/.github/workflows/codeowners.yml b/.github/workflows/codeowners.yml index 19d5835..75b3515 100644 --- a/.github/workflows/codeowners.yml +++ b/.github/workflows/codeowners.yml @@ -1,19 +1,24 @@ 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: pull_request: - paths: - - '.github/codeowners-roles.yml' - - '.github/CODEOWNERS' - - '.github/scripts/render-codeowners.py' - - '.github/workflows/codeowners.yml' 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: - contents: read + contents: write 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: name: CODEOWNERS matches source runs-on: ubuntu-latest @@ -28,19 +33,56 @@ jobs: - name: Install PyYAML run: pip install pyyaml - - name: Re-render CODEOWNERS + - name: Re-render CODEOWNERS + ownership docs 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: | - if ! git diff --quiet .github/CODEOWNERS; then - echo "::error::.github/CODEOWNERS is out of sync with .github/codeowners-roles.yml." - echo "::error::Run \`python3 .github/scripts/render-codeowners.py\` locally and commit the result." + if git diff --quiet -- .github/CODEOWNERS docs/dev/codeowners.md; then + echo "CODEOWNERS and ownership docs already in sync." + 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 ---" - git --no-pager diff .github/CODEOWNERS + git --no-pager diff -- .github/CODEOWNERS docs/dev/codeowners.md exit 1 fi - echo "CODEOWNERS is in sync with its source." + echo "Generated artifacts are in sync with their source." noedit: name: CODEOWNERS not hand-edited @@ -52,6 +94,8 @@ jobs: fetch-depth: 0 - name: Reject hand-edits to generated file + # Only meaningful for PRs (needs a base to diff against). + if: github.event_name == 'pull_request' run: | base="origin/${{ github.base_ref }}" git fetch origin "${{ github.base_ref }}" --quiet diff --git a/docs/dev/branch-protection.md b/docs/dev/branch-protection.md index 9b2fa78..2b6cc37 100644 --- a/docs/dev/branch-protection.md +++ b/docs/dev/branch-protection.md @@ -8,7 +8,7 @@ This page explains what the policy says and how to change it. | 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. | | **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. | @@ -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 branch deletions** | `true` | `main` cannot be deleted. | | **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. | ## How to apply diff --git a/docs/dev/codeowners.md b/docs/dev/codeowners.md index 9a7fb50..14bba0b 100644 --- a/docs/dev/codeowners.md +++ b/docs/dev/codeowners.md @@ -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`). -## 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. + + + +**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 | -| `docs` | `@ragnorc` | `docs/**`, README.md, AGENTS.md, CLAUDE.md, SECURITY.md | +| `*` | @ragnorc | engineering | +| `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). | + + + +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 1. Edit `.github/codeowners-roles.yml`. -2. Run `python3 .github/scripts/render-codeowners.py` (requires PyYAML; `pip install pyyaml`). -3. Commit both files in the same PR. +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). + +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: -- `CODEOWNERS` was edited without a corresponding yml change, or -- The yml was changed but the rendered `CODEOWNERS` doesn't match. +- a fork PR left a generated artifact out of sync, or +- `CODEOWNERS` was edited without a corresponding yml change (the `CODEOWNERS not hand-edited` check). ## How to add a new role