diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 772b643b5..a47654668 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,8 +1,8 @@ ## Description -Brief description of changes and motivation. + -Fixes #(issue number) +Closes # ## Type of Change @@ -15,13 +15,11 @@ Fixes #(issue number) ## Changes Made -- Change 1 -- Change 2 -- Change 3 +- ## Testing -Describe the tests you ran to verify your changes: + - [ ] Unit tests pass (`pytest`) - [ ] Linting passes (`ruff check .`) @@ -29,12 +27,23 @@ Describe the tests you ran to verify your changes: - [ ] New tests added for new functionality - [ ] Manual testing performed -## Test Output +### Test Output +```text +# Paste relevant command output or artifact links here ``` -# Paste relevant test output here -pytest -v tests/test_your_feature.py -``` + +## Real Behavior Proof + +- Environment: +- Exact command / steps: +- Observed result: +- Not tested: + +## Review Readiness + +- [ ] I have performed a self-review +- [ ] This PR is ready for human review ## Checklist @@ -53,4 +62,4 @@ Add screenshots to help explain your changes. ## Additional Notes -Any additional information that reviewers should know. + diff --git a/.github/act/pr-governance-invalid.json b/.github/act/pr-governance-invalid.json new file mode 100644 index 000000000..791d266aa --- /dev/null +++ b/.github/act/pr-governance-invalid.json @@ -0,0 +1,19 @@ +{ + "action": "opened", + "number": 42, + "pull_request": { + "number": 42, + "draft": false, + "title": "feat: add PR governance", + "body": "## Description\n\nFixes #123\n", + "user": { + "login": "octocat" + }, + "base": { + "sha": "dff6a199" + } + }, + "repository": { + "full_name": "JerrettDavis/headroom" + } +} diff --git a/.github/act/pr-governance-valid.json b/.github/act/pr-governance-valid.json new file mode 100644 index 000000000..8d1b07d16 --- /dev/null +++ b/.github/act/pr-governance-valid.json @@ -0,0 +1,19 @@ +{ + "action": "ready_for_review", + "number": 42, + "pull_request": { + "number": 42, + "draft": false, + "title": "feat: add PR governance", + "body": "## Description\n\nAdd a required PR governance check and commit-msg enforcement.\n\nCloses #123\n\n## Type of Change\n\n- [x] New feature (non-breaking change that adds functionality)\n\n## Changes Made\n\n- Added workflow validation for PR template completeness.\n- Added a commit-msg hook that runs commitlint locally.\n\n## Testing\n\n- [x] Unit tests pass (`pytest`)\n- [x] Manual testing performed\n\n### Test Output\n\n```text\npytest scripts/tests/test_pr_governance.py -q\n```\n\n## Real Behavior Proof\n\n- Environment: Ubuntu runner, Python 3.12\n- Exact command / steps: Opened a PR with an incomplete template, then fixed the body.\n- Observed result: The governance check failed until the template and readiness boxes were complete.\n- Not tested: Repository-level automatic Copilot rulesets.\n\n## Review Readiness\n\n- [x] I have performed a self-review\n- [x] This PR is ready for human review\n", + "user": { + "login": "octocat" + }, + "base": { + "sha": "dff6a199" + } + }, + "repository": { + "full_name": "JerrettDavis/headroom" + } +} diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..c22fb65d0 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,7 @@ +When performing a pull request review in this repository: + +1. Treat `.github/PULL_REQUEST_TEMPLATE.md` and `CONTRIBUTING.md` as required policy, not optional guidance. +2. Flag pull requests that do not include concrete "Real Behavior Proof" with environment, exact commands or steps, observed result, and what was not tested. +3. Be strict about contributor verification: missing tests, missing runtime evidence, or placeholder PR text should be called out. +4. For user-facing, release, dependency, workflow, or security-sensitive changes, prefer blocking feedback over optional suggestions. +5. Focus on correctness, safety, and whether the PR is actually ready for human maintainer review. diff --git a/.github/workflows/pr-health.yml b/.github/workflows/pr-health.yml index 55ea95aec..abfcae1c2 100644 --- a/.github/workflows/pr-health.yml +++ b/.github/workflows/pr-health.yml @@ -1,91 +1,223 @@ -name: PR Health - -on: - pull_request_target: - types: [opened, reopened, synchronize, ready_for_review] - schedule: - # Keep labels fresh even when base branches move or checks finish later. - - cron: '23 14 * * 1-5' - workflow_dispatch: - -permissions: - contents: read - issues: write - pull-requests: write - checks: read - statuses: read - -concurrency: - group: pr-health-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -jobs: - label: - runs-on: ubuntu-latest - timeout-minutes: 10 - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - REPO: ${{ github.repository }} - steps: - - name: Ensure maintenance labels exist - run: | - set -euo pipefail - - gh label create "status: needs rebase" \ - --repo "$REPO" \ - --color "fbca04" \ - --description "Pull request branch is behind the base branch" \ - --force - gh label create "status: has conflicts" \ - --repo "$REPO" \ - --color "d73a4a" \ - --description "Pull request has merge conflicts with the base branch" \ - --force - gh label create "status: ci failing" \ - --repo "$REPO" \ - --color "d73a4a" \ - --description "Required or reported CI checks are failing" \ - --force - - - name: Label open pull requests - run: | - set -euo pipefail - - if jq -e '.pull_request.number' "$GITHUB_EVENT_PATH" >/dev/null; then - pr_numbers="$(jq -r '.pull_request.number' "$GITHUB_EVENT_PATH")" - else - pr_numbers="$(gh pr list --repo "$REPO" --state open --limit 100 --json number --jq '.[].number')" - fi - - for pr in $pr_numbers; do - data="$(gh pr view "$pr" --repo "$REPO" \ - --json mergeStateStatus,statusCheckRollup)" - - merge_state="$(jq -r '.mergeStateStatus // "UNKNOWN"' <<<"$data")" - check_state="$(jq -r ' - [ - .statusCheckRollup[] - | select((.conclusion // .state // "") as $s - | ["FAILURE", "TIMED_OUT", "ACTION_REQUIRED", "CANCELLED", "ERROR"] | index($s)) - ] - | if length > 0 then "failing" else "passing" end - ' <<<"$data")" - - if [[ "$merge_state" == "BEHIND" ]]; then - gh pr edit "$pr" --repo "$REPO" --add-label "status: needs rebase" - else - gh pr edit "$pr" --repo "$REPO" --remove-label "status: needs rebase" || true - fi - - if [[ "$merge_state" == "DIRTY" ]]; then - gh pr edit "$pr" --repo "$REPO" --add-label "status: has conflicts" - else - gh pr edit "$pr" --repo "$REPO" --remove-label "status: has conflicts" || true - fi - - if [[ "$check_state" == "failing" ]]; then - gh pr edit "$pr" --repo "$REPO" --add-label "status: ci failing" - else - gh pr edit "$pr" --repo "$REPO" --remove-label "status: ci failing" || true - fi - done +name: PR Governance + +on: + pull_request_target: + types: [opened, edited, reopened, synchronize, ready_for_review, converted_to_draft] + schedule: + # Keep labels fresh even when base branches move or checks finish later. + - cron: '23 14 * * 1-5' + workflow_dispatch: + +permissions: + contents: read + issues: write + pull-requests: write + checks: read + statuses: read + +concurrency: + group: pr-health-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + template: + if: github.event_name == 'pull_request_target' + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.base.sha }} + + - name: Validate PR template + id: validate + run: python3 scripts/pr-governance.py --event "$GITHUB_EVENT_PATH" --report .pr-governance-report.json + + - name: Append governance summary + run: | + python3 - <<'PY' + import json + import os + from pathlib import Path + + report = json.loads(Path(".pr-governance-report.json").read_text(encoding="utf-8")) + summary = report["summary_markdown"].strip() + with Path(os.environ["GITHUB_STEP_SUMMARY"]).open("a", encoding="utf-8") as handle: + handle.write(f"{summary}\n") + PY + + - name: Ensure governance labels exist + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + + gh label create "status: needs author action" \ + --repo "$REPO" \ + --color "d93f0b" \ + --description "Pull request body or readiness checklist still needs author updates" \ + --force + gh label create "status: ready for review" \ + --repo "$REPO" \ + --color "0e8a16" \ + --description "Pull request body is complete and the author marked it ready for human review" \ + --force + + - name: Sync governance comment and labels + if: steps.validate.outputs.is_bot_pr != 'true' + uses: actions/github-script@v7 + env: + REPORT_PATH: .pr-governance-report.json + with: + script: | + const fs = require('fs'); + const report = JSON.parse(fs.readFileSync(process.env.REPORT_PATH, 'utf8')); + const owner = context.repo.owner; + const repo = context.repo.repo; + const issue_number = context.payload.pull_request.number; + const marker = report.comment_marker; + const body = `${marker}\n${report.comment_markdown}`.trim(); + + const comments = await github.paginate(github.rest.issues.listComments, { + owner, + repo, + issue_number, + per_page: 100, + }); + + const existing = comments.find( + (comment) => + comment.user?.type === 'Bot' && typeof comment.body === 'string' && comment.body.includes(marker), + ); + + if (existing) { + await github.rest.issues.updateComment({ + owner, + repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner, + repo, + issue_number, + body, + }); + } + + if (report.labels_to_add.length > 0) { + await github.rest.issues.addLabels({ + owner, + repo, + issue_number, + labels: report.labels_to_add, + }); + } + + for (const label of report.labels_to_remove) { + try { + await github.rest.issues.removeLabel({ + owner, + repo, + issue_number, + name: label, + }); + } catch (error) { + if (error.status !== 404) { + throw error; + } + } + } + + - name: Fail when the PR body is incomplete + if: steps.validate.outputs.valid != 'true' + run: | + echo "PR template validation failed. Update the PR body or move the PR back to draft." + exit 1 + + label: + runs-on: ubuntu-latest + timeout-minutes: 10 + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + steps: + - name: Ensure maintenance labels exist + run: | + set -euo pipefail + + gh label create "status: needs rebase" \ + --repo "$REPO" \ + --color "fbca04" \ + --description "Pull request branch is behind the base branch" \ + --force + gh label create "status: has conflicts" \ + --repo "$REPO" \ + --color "d73a4a" \ + --description "Pull request has merge conflicts with the base branch" \ + --force + gh label create "status: ci failing" \ + --repo "$REPO" \ + --color "d73a4a" \ + --description "Required or reported CI checks are failing" \ + --force + gh label create "status: needs author action" \ + --repo "$REPO" \ + --color "d93f0b" \ + --description "Pull request body or readiness checklist still needs author updates" \ + --force + gh label create "status: ready for review" \ + --repo "$REPO" \ + --color "0e8a16" \ + --description "Pull request body is complete and the author marked it ready for human review" \ + --force + + - name: Label open pull requests + run: | + set -euo pipefail + + if jq -e '.pull_request.number' "$GITHUB_EVENT_PATH" >/dev/null; then + pr_numbers="$(jq -r '.pull_request.number' "$GITHUB_EVENT_PATH")" + else + pr_numbers="$(gh pr list --repo "$REPO" --state open --limit 100 --json number --jq '.[].number')" + fi + + for pr in $pr_numbers; do + data="$(gh pr view "$pr" --repo "$REPO" \ + --json isDraft,labels,mergeStateStatus,statusCheckRollup)" + + merge_state="$(jq -r '.mergeStateStatus // "UNKNOWN"' <<<"$data")" + check_state="$(jq -r ' + [ + (.statusCheckRollup // [])[] + | select((.conclusion // .state // "") as $s + | ["FAILURE", "TIMED_OUT", "ACTION_REQUIRED", "CANCELLED", "ERROR"] | index($s)) + ] + | if length > 0 then "failing" else "passing" end + ' <<<"$data")" + is_draft="$(jq -r '.isDraft' <<<"$data")" + + if [[ "$merge_state" == "BEHIND" ]]; then + gh pr edit "$pr" --repo "$REPO" --add-label "status: needs rebase" + else + gh pr edit "$pr" --repo "$REPO" --remove-label "status: needs rebase" || true + fi + + if [[ "$merge_state" == "DIRTY" ]]; then + gh pr edit "$pr" --repo "$REPO" --add-label "status: has conflicts" + else + gh pr edit "$pr" --repo "$REPO" --remove-label "status: has conflicts" || true + fi + + if [[ "$check_state" == "failing" ]]; then + gh pr edit "$pr" --repo "$REPO" --add-label "status: ci failing" + else + gh pr edit "$pr" --repo "$REPO" --remove-label "status: ci failing" || true + fi + + if [[ "$merge_state" == "BEHIND" || "$merge_state" == "DIRTY" || "$check_state" == "failing" || "$is_draft" == "true" ]]; then + gh pr edit "$pr" --repo "$REPO" --remove-label "status: ready for review" || true + fi + done diff --git a/.gitignore b/.gitignore index 1edd20517..052a2b6d1 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ scripts/* !scripts/sync-plugin-versions.py !scripts/changelog-gen.py !scripts/verify-versions.py +!scripts/pr-governance.py !scripts/tests/ !scripts/README.md !scripts/repro_codex_replay.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 867691d9f..533063f1e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,6 +7,11 @@ repos: language: system pass_filenames: false always_run: true + - id: commitlint + name: Commitlint + entry: bash -lc 'npx --yes --package=@commitlint/cli --package=@commitlint/config-conventional -- commitlint --edit "$1" --config .commitlintrc.json' -- + language: system + stages: [commit-msg] - repo: https://github.com/astral-sh/ruff-pre-commit rev: v0.9.4 hooks: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3ccf8bb10..8d2698ade 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,15 +73,17 @@ A human maintainer reviews every dep change. PRs that add or bump a package must ## PR workflow 1. Fork, branch from `main`. -2. `pip install -e ".[dev]"` then `make install-git-hooks` — installs repo pre-commit checks on every commit and ci-precheck on every push. +2. Install **Node 18+** and run `pip install -e ".[dev]"` then `make install-git-hooks` — installs repo pre-commit checks on every commit, commitlint on every commit message, and ci-precheck on every push. 3. One logical change per PR. 4. Add tests. 5. `pytest` · `ruff check .` · `ruff format .` 6. Update `CHANGELOG.md` for user-facing changes. -7. Open the PR with a clear description + `Real behavior proof` + any spec/justification required. +7. Open the PR with a clear description + `Real behavior proof` + any spec/justification required, and keep the PR in draft until the `Review Readiness` boxes are complete. **Title format** (conventional commits): `feat:`, `fix:`, `docs:`, `test:`, `refactor:`. +**Commit message format** is enforced locally by the repo's `commit-msg` hook and again in CI. + **Review:** CI green, one maintainer review, coverage held/improved. ## Development setup @@ -90,6 +92,7 @@ A human maintainer reviews every dep change. PRs that add or bump a package must git clone https://github.com/chopratejas/headroom.git cd headroom python -m venv .venv && source .venv/bin/activate +node --version # Node 18+ required for commitlint hooks pip install -e ".[dev,relevance,proxy]" pytest ``` @@ -103,6 +106,12 @@ Two configs ship for VS Code / Codespaces: Inside, use: `uv run ruff check .`, `uv run pytest`, etc. +## Optional automated review + +This repository includes `.github/copilot-instructions.md` so maintainers can opt into GitHub Copilot code review without adding workflow billing noise to every PR. + +Enable or disable automatic Copilot review in **Settings → Rules → Rulesets → Automatically request Copilot code review**. Keep it off unless maintainers explicitly want the extra review traffic. + ## Coding standards - [Ruff](https://github.com/astral-sh/ruff) for lint + format, line length 100, PEP 8. diff --git a/Makefile b/Makefile index ed43fb836..872fde832 100644 --- a/Makefile +++ b/Makefile @@ -27,7 +27,7 @@ help: @echo " make ci-precheck-rust - cargo fmt --check + clippy + test" @echo " make ci-precheck-python - smart_crusher-affected python tests" @echo " make ci-precheck-commitlint - lint commits since origin/main" - @echo " make install-git-hooks - install a pre-push hook that runs ci-precheck" + @echo " make install-git-hooks - install pre-commit, commit-msg, and pre-push hooks" test: $(CARGO) test --workspace @@ -123,16 +123,15 @@ ci-precheck-python: tests/test_toin_integration.py # Lint commits since `origin/main`. Requires npx (Node 18+) on PATH. -# Skips silently if npx is unavailable; install nodejs to enable. ci-precheck-commitlint: @echo "── ci-precheck-commitlint ─────────────────────────────────────" @if ! command -v npx >/dev/null 2>&1; then \ - echo "skip: npx not on PATH (install node 18+ to enable commitlint pre-check)"; \ - exit 0; \ + echo "error: npx not on PATH (install Node 18+ to enable commitlint checks)"; \ + exit 1; \ fi @if ! git rev-parse --verify origin/main >/dev/null 2>&1; then \ - echo "skip: origin/main not fetched (run 'git fetch origin main')"; \ - exit 0; \ + echo "error: origin/main not fetched (run 'git fetch origin main')"; \ + exit 1; \ fi npx --yes --package=@commitlint/cli --package=@commitlint/config-conventional -- \ commitlint --from origin/main --to HEAD --config .commitlintrc.json diff --git a/scripts/install-git-hooks.sh b/scripts/install-git-hooks.sh index 53975da22..be8f0b0fb 100755 --- a/scripts/install-git-hooks.sh +++ b/scripts/install-git-hooks.sh @@ -1,7 +1,8 @@ #!/usr/bin/env bash # Install git hooks for the Headroom repo: # 1. pre-commit — repo pre-commit checks (ruff, mypy, sync-plugin-versions) -# 2. pre-push — full ci-precheck (cargo fmt/clippy/test + python suite) +# 2. commit-msg — conventional-commit enforcement via commitlint +# 3. pre-push — full ci-precheck (cargo fmt/clippy/test + python suite) # # Why pre-push was added: the 2026-04-27 push hit five CI failures that could # all have been caught locally — cargo fmt drift, an x86_64-apple-darwin wheel @@ -24,6 +25,11 @@ if [[ ! -d .git/hooks ]]; then exit 1 fi +if ! command -v npx &>/dev/null; then + echo "error: npx not found — install Node 18+ before installing Headroom's git hooks." >&2 + exit 1 +fi + HOOK_PATH=".git/hooks/pre-push" cat > "$HOOK_PATH" <<'HOOK_EOF' @@ -89,7 +95,9 @@ fi if [[ -n "$PRE_COMMIT_BIN" ]]; then "$PRE_COMMIT_BIN" install + "$PRE_COMMIT_BIN" install --hook-type commit-msg echo "✅ installed: .git/hooks/pre-commit (repo pre-commit checks via pre-commit)" + echo "✅ installed: .git/hooks/commit-msg (conventional commit enforcement via commitlint)" else echo "error: pre-commit not found — run 'pip install -e .[dev]' first, then re-run this script." >&2 exit 1 diff --git a/scripts/pr-governance.py b/scripts/pr-governance.py new file mode 100644 index 000000000..d095363f4 --- /dev/null +++ b/scripts/pr-governance.py @@ -0,0 +1,297 @@ +#!/usr/bin/env python3 +"""Validate Headroom PR template compliance for GitHub Actions.""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import sys +from dataclasses import asdict, dataclass, field +from pathlib import Path +from typing import Any + +COMMENT_MARKER = "" +READY_LABEL = "status: ready for review" +AUTHOR_ACTION_LABEL = "status: needs author action" + +REQUIRED_SECTIONS = ( + "Description", + "Type of Change", + "Changes Made", + "Testing", + "Real Behavior Proof", + "Review Readiness", +) +PROOF_FIELDS = ( + "Environment", + "Exact command / steps", + "Observed result", + "Not tested", +) + +SECTION_RE = re.compile(r"^##\s+(.+?)\s*$", re.MULTILINE) +CHECKBOX_RE = re.compile(r"^- \[(?P[ xX])\] (?P