mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
Fix CI lint failure by formatting PR governance scripts (#933)
`CI / lint (pull_request)` failed because `ruff format --check` detected
formatting drift in the new PR governance script and its tests. This PR
aligns those files with repository formatting rules so the lint job can
pass.
- **Root cause**
- `ruff format --check .` reported two files as non-canonical:
- `scripts/pr-governance.py`
- `scripts/tests/test_pr_governance.py`
- **Change set**
- Applied `ruff` formatting to only the two flagged files.
- No behavioral or logic changes; edits are line-wrap/format
normalization only.
- **Representative update**
```python
parser.add_argument(
"--event", type=Path, required=True, help="Path to the GitHub event
payload JSON."
)
```
---------
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This commit is contained in:
parent
b9e27614c6
commit
96a7d7cbbe
13 changed files with 781 additions and 111 deletions
|
|
@ -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
|
||||
|
|
|
|||
297
scripts/pr-governance.py
Normal file
297
scripts/pr-governance.py
Normal file
|
|
@ -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 = "<!-- headroom-pr-governance -->"
|
||||
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<checked>[ xX])\] (?P<label>.+)$", re.MULTILINE)
|
||||
HTML_COMMENT_RE = re.compile(r"<!--.*?-->", re.DOTALL)
|
||||
CODE_BLOCK_RE = re.compile(r"```(?:[\w.+-]+)?\n(?P<content>.*?)```", re.DOTALL)
|
||||
|
||||
|
||||
@dataclass(slots=True)
|
||||
class GovernanceReport:
|
||||
"""Serializable PR governance result."""
|
||||
|
||||
comment_marker: str
|
||||
valid: bool
|
||||
is_draft: bool
|
||||
is_bot_pr: bool
|
||||
ready_for_review: bool
|
||||
needs_author_action: bool
|
||||
problems: list[str] = field(default_factory=list)
|
||||
labels_to_add: list[str] = field(default_factory=list)
|
||||
labels_to_remove: list[str] = field(default_factory=list)
|
||||
comment_markdown: str = ""
|
||||
summary_markdown: str = ""
|
||||
|
||||
def to_dict(self) -> dict[str, Any]:
|
||||
return asdict(self)
|
||||
|
||||
|
||||
def load_event(path: Path) -> dict[str, Any]:
|
||||
return json.loads(path.read_text(encoding="utf-8"))
|
||||
|
||||
|
||||
def extract_sections(body: str) -> dict[str, str]:
|
||||
matches = list(SECTION_RE.finditer(body))
|
||||
sections: dict[str, str] = {}
|
||||
for index, match in enumerate(matches):
|
||||
start = match.end()
|
||||
end = matches[index + 1].start() if index + 1 < len(matches) else len(body)
|
||||
sections[match.group(1).strip()] = body[start:end].strip()
|
||||
return sections
|
||||
|
||||
|
||||
def strip_html_comments(text: str) -> str:
|
||||
return HTML_COMMENT_RE.sub("", text).strip()
|
||||
|
||||
|
||||
def non_empty_lines(text: str) -> list[str]:
|
||||
return [line.strip() for line in strip_html_comments(text).splitlines() if line.strip()]
|
||||
|
||||
|
||||
def checked_items(section: str) -> list[str]:
|
||||
return [
|
||||
match.group("label").strip()
|
||||
for match in CHECKBOX_RE.finditer(section)
|
||||
if match.group("checked").lower() == "x"
|
||||
]
|
||||
|
||||
|
||||
def has_descriptive_text(section: str) -> bool:
|
||||
ignored_prefixes = ("closes #", "fixes #", "resolves #", "related to #")
|
||||
for line in non_empty_lines(section):
|
||||
lowered = line.lower()
|
||||
if line.startswith("#"):
|
||||
continue
|
||||
if lowered.startswith(ignored_prefixes):
|
||||
continue
|
||||
if len(line) >= 10:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def has_non_placeholder_bullets(section: str) -> bool:
|
||||
placeholders = {"change 1", "change 2", "change 3"}
|
||||
for line in non_empty_lines(section):
|
||||
if not line.startswith("- "):
|
||||
continue
|
||||
bullet = line[2:].strip().lower()
|
||||
if bullet and bullet not in placeholders:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def has_test_output(section: str) -> bool:
|
||||
for match in CODE_BLOCK_RE.finditer(section):
|
||||
content = strip_html_comments(match.group("content")).strip()
|
||||
if not content:
|
||||
continue
|
||||
if "paste relevant command output or artifact links here" in content.lower():
|
||||
continue
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def proof_field_values(section: str) -> dict[str, str]:
|
||||
values: dict[str, str] = {}
|
||||
for line in non_empty_lines(section):
|
||||
if not line.startswith("- ") or ":" not in line:
|
||||
continue
|
||||
label, value = line[2:].split(":", 1)
|
||||
values[label.strip()] = value.strip()
|
||||
return values
|
||||
|
||||
|
||||
def normalize_checkbox_map(items: list[str]) -> set[str]:
|
||||
return {item.lower() for item in items}
|
||||
|
||||
|
||||
def validate_pull_request(event: dict[str, Any]) -> GovernanceReport:
|
||||
pull_request = event["pull_request"]
|
||||
author = pull_request["user"]["login"]
|
||||
is_draft = bool(pull_request.get("draft", False))
|
||||
is_bot_pr = author.endswith("[bot]")
|
||||
body = pull_request.get("body") or ""
|
||||
|
||||
if is_bot_pr:
|
||||
summary = "### PR governance\n\nBot-authored PR detected; template enforcement is skipped."
|
||||
return GovernanceReport(
|
||||
comment_marker=COMMENT_MARKER,
|
||||
valid=True,
|
||||
is_draft=is_draft,
|
||||
is_bot_pr=True,
|
||||
ready_for_review=False,
|
||||
needs_author_action=False,
|
||||
comment_markdown=summary,
|
||||
summary_markdown=summary,
|
||||
)
|
||||
|
||||
sections = extract_sections(body)
|
||||
problems: list[str] = []
|
||||
|
||||
for section_name in REQUIRED_SECTIONS:
|
||||
if section_name not in sections:
|
||||
problems.append(f"Missing required section `{section_name}`.")
|
||||
|
||||
description = sections.get("Description", "")
|
||||
if description and not has_descriptive_text(description):
|
||||
problems.append("Fill in `Description` with a real summary of the change.")
|
||||
|
||||
changes_made = sections.get("Changes Made", "")
|
||||
if changes_made and not has_non_placeholder_bullets(changes_made):
|
||||
problems.append(
|
||||
"Replace the placeholder bullets in `Changes Made` with the actual changes."
|
||||
)
|
||||
|
||||
type_of_change_checked = checked_items(sections.get("Type of Change", ""))
|
||||
if sections.get("Type of Change") and not type_of_change_checked:
|
||||
problems.append("Check at least one box in `Type of Change`.")
|
||||
|
||||
testing_section = sections.get("Testing", "")
|
||||
testing_checked = checked_items(testing_section)
|
||||
if testing_section and not testing_checked:
|
||||
problems.append("Check at least one verification item in `Testing`.")
|
||||
if testing_section and not has_test_output(testing_section):
|
||||
problems.append("Paste real command output or artifact links in `Testing` → `Test Output`.")
|
||||
|
||||
proof_section = sections.get("Real Behavior Proof", "")
|
||||
proof_values = proof_field_values(proof_section)
|
||||
for field_name in PROOF_FIELDS:
|
||||
if proof_section and not proof_values.get(field_name):
|
||||
problems.append(f"Fill in `Real Behavior Proof` → `{field_name}`.")
|
||||
|
||||
readiness_checked = normalize_checkbox_map(checked_items(sections.get("Review Readiness", "")))
|
||||
has_self_review = "i have performed a self-review" in readiness_checked
|
||||
has_ready_checkbox = "this pr is ready for human review" in readiness_checked
|
||||
if not is_draft:
|
||||
if not has_self_review:
|
||||
problems.append(
|
||||
"Check `I have performed a self-review` before requesting human review."
|
||||
)
|
||||
if not has_ready_checkbox:
|
||||
problems.append(
|
||||
"Check `This PR is ready for human review` or convert the PR back to draft."
|
||||
)
|
||||
|
||||
valid = not problems
|
||||
ready_for_review = valid and not is_draft and has_ready_checkbox and has_self_review
|
||||
needs_author_action = not valid
|
||||
|
||||
if valid and ready_for_review:
|
||||
status_lines = [
|
||||
"### PR governance",
|
||||
"",
|
||||
"This PR follows the template and is marked ready for human review.",
|
||||
]
|
||||
elif valid:
|
||||
status_lines = [
|
||||
"### PR governance",
|
||||
"",
|
||||
"This draft PR follows the template so far. Keep it in draft until it is ready for human review.",
|
||||
]
|
||||
else:
|
||||
status_lines = [
|
||||
"### PR governance",
|
||||
"",
|
||||
"This PR does not yet satisfy the required template fields:",
|
||||
"",
|
||||
*[f"- {problem}" for problem in problems],
|
||||
"",
|
||||
"Please update the PR body, or move the PR back to draft while it is still in progress.",
|
||||
]
|
||||
|
||||
labels_to_add: list[str] = []
|
||||
labels_to_remove: list[str] = []
|
||||
if needs_author_action:
|
||||
labels_to_add.append(AUTHOR_ACTION_LABEL)
|
||||
labels_to_remove.append(READY_LABEL)
|
||||
else:
|
||||
labels_to_remove.append(AUTHOR_ACTION_LABEL)
|
||||
if ready_for_review:
|
||||
labels_to_add.append(READY_LABEL)
|
||||
else:
|
||||
labels_to_remove.append(READY_LABEL)
|
||||
|
||||
comment_markdown = "\n".join(status_lines)
|
||||
return GovernanceReport(
|
||||
comment_marker=COMMENT_MARKER,
|
||||
valid=valid,
|
||||
is_draft=is_draft,
|
||||
is_bot_pr=False,
|
||||
ready_for_review=ready_for_review,
|
||||
needs_author_action=needs_author_action,
|
||||
problems=problems,
|
||||
labels_to_add=labels_to_add,
|
||||
labels_to_remove=labels_to_remove,
|
||||
comment_markdown=comment_markdown,
|
||||
summary_markdown=comment_markdown,
|
||||
)
|
||||
|
||||
|
||||
def emit_outputs(report: GovernanceReport) -> None:
|
||||
output_path = os.environ.get("GITHUB_OUTPUT")
|
||||
lines = [
|
||||
f"valid={str(report.valid).lower()}",
|
||||
f"ready_for_review={str(report.ready_for_review).lower()}",
|
||||
f"needs_author_action={str(report.needs_author_action).lower()}",
|
||||
f"is_bot_pr={str(report.is_bot_pr).lower()}",
|
||||
]
|
||||
if not output_path:
|
||||
for line in lines:
|
||||
print(line)
|
||||
return
|
||||
|
||||
with Path(output_path).open("a", encoding="utf-8") as output_file:
|
||||
for line in lines:
|
||||
output_file.write(f"{line}\n")
|
||||
|
||||
|
||||
def parse_args(argv: list[str]) -> argparse.Namespace:
|
||||
parser = argparse.ArgumentParser(description=__doc__)
|
||||
parser.add_argument(
|
||||
"--event", type=Path, required=True, help="Path to the GitHub event payload JSON."
|
||||
)
|
||||
parser.add_argument("--report", type=Path, required=True, help="Path to write the JSON report.")
|
||||
return parser.parse_args(argv)
|
||||
|
||||
|
||||
def main(argv: list[str] | None = None) -> int:
|
||||
args = parse_args(argv or sys.argv[1:])
|
||||
report = validate_pull_request(load_event(args.event))
|
||||
args.report.write_text(json.dumps(report.to_dict(), indent=2), encoding="utf-8")
|
||||
emit_outputs(report)
|
||||
return 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main())
|
||||
163
scripts/tests/test_pr_governance.py
Normal file
163
scripts/tests/test_pr_governance.py
Normal file
|
|
@ -0,0 +1,163 @@
|
|||
"""Tests for pr-governance.py."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib.util
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def _load_module():
|
||||
script = Path(__file__).parent.parent / "pr-governance.py"
|
||||
spec = importlib.util.spec_from_file_location("pr_governance", script)
|
||||
assert spec is not None
|
||||
assert spec.loader is not None
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
sys.modules[spec.name] = module
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
def _event(body: str, *, draft: bool = False, login: str = "octocat") -> dict[str, object]:
|
||||
return {
|
||||
"pull_request": {
|
||||
"number": 42,
|
||||
"draft": draft,
|
||||
"body": body,
|
||||
"user": {"login": login},
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
VALID_BODY = """## Description
|
||||
|
||||
Add a required PR-governance gate for template validation and review readiness.
|
||||
|
||||
Closes #123
|
||||
|
||||
## Type of Change
|
||||
|
||||
- [x] New feature (non-breaking change that adds functionality)
|
||||
- [ ] Documentation update
|
||||
|
||||
## Changes Made
|
||||
|
||||
- Added a workflow-backed PR template validator.
|
||||
- Added local commit message linting in the commit-msg hook.
|
||||
|
||||
## Testing
|
||||
|
||||
- [x] Unit tests pass (`pytest`)
|
||||
- [x] Manual testing performed
|
||||
|
||||
### Test Output
|
||||
|
||||
```text
|
||||
pytest scripts/tests/test_pr_governance.py -q
|
||||
```
|
||||
|
||||
## Real Behavior Proof
|
||||
|
||||
- Environment: Ubuntu runner, Python 3.12
|
||||
- Exact command / steps: Open a PR, remove the ready checkbox, re-run the workflow.
|
||||
- Observed result: The governance check fails and the PR gets a needs-author-action label.
|
||||
- Not tested: Automatic Copilot review rulesets in repository settings.
|
||||
|
||||
## Review Readiness
|
||||
|
||||
- [x] I have performed a self-review
|
||||
- [x] This PR is ready for human review
|
||||
|
||||
## Additional Notes
|
||||
|
||||
- Maintainers can optionally enable Copilot code review from repository rulesets.
|
||||
"""
|
||||
|
||||
|
||||
def test_validate_pull_request_marks_ready_pr_valid() -> None:
|
||||
module = _load_module()
|
||||
|
||||
report = module.validate_pull_request(_event(VALID_BODY))
|
||||
|
||||
assert report.valid is True
|
||||
assert report.ready_for_review is True
|
||||
assert report.needs_author_action is False
|
||||
assert report.problems == []
|
||||
assert report.labels_to_add == [module.READY_LABEL]
|
||||
assert module.AUTHOR_ACTION_LABEL in report.labels_to_remove
|
||||
|
||||
|
||||
def test_validate_pull_request_allows_draft_without_ready_checkboxes() -> None:
|
||||
module = _load_module()
|
||||
body = VALID_BODY.replace(
|
||||
"- [x] I have performed a self-review", "- [ ] I have performed a self-review"
|
||||
)
|
||||
body = body.replace(
|
||||
"- [x] This PR is ready for human review",
|
||||
"- [ ] This PR is ready for human review",
|
||||
)
|
||||
|
||||
report = module.validate_pull_request(_event(body, draft=True))
|
||||
|
||||
assert report.valid is True
|
||||
assert report.ready_for_review is False
|
||||
assert report.needs_author_action is False
|
||||
assert report.labels_to_add == []
|
||||
assert module.READY_LABEL in report.labels_to_remove
|
||||
|
||||
|
||||
def test_validate_pull_request_fails_on_missing_required_content() -> None:
|
||||
module = _load_module()
|
||||
body = """## Description
|
||||
|
||||
Fixes #123
|
||||
|
||||
## Type of Change
|
||||
|
||||
- [ ] New feature (non-breaking change that adds functionality)
|
||||
|
||||
## Changes Made
|
||||
|
||||
- Change 1
|
||||
|
||||
## Testing
|
||||
|
||||
### Test Output
|
||||
|
||||
```text
|
||||
# Paste relevant command output or artifact links here
|
||||
```
|
||||
|
||||
## 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
|
||||
"""
|
||||
|
||||
report = module.validate_pull_request(_event(body))
|
||||
|
||||
assert report.valid is False
|
||||
assert report.needs_author_action is True
|
||||
assert module.AUTHOR_ACTION_LABEL in report.labels_to_add
|
||||
assert any("Description" in problem for problem in report.problems)
|
||||
assert any("Type of Change" in problem for problem in report.problems)
|
||||
assert any("Test Output" in problem for problem in report.problems)
|
||||
assert any("Real Behavior Proof" in problem for problem in report.problems)
|
||||
|
||||
|
||||
def test_validate_pull_request_skips_bot_authored_prs() -> None:
|
||||
module = _load_module()
|
||||
|
||||
report = module.validate_pull_request(_event("", login="dependabot[bot]"))
|
||||
|
||||
assert report.valid is True
|
||||
assert report.is_bot_pr is True
|
||||
assert report.needs_author_action is False
|
||||
assert report.labels_to_add == []
|
||||
|
|
@ -32,4 +32,6 @@ run_act act workflow_dispatch -W .github/workflows/release.yml -e .github/act/dr
|
|||
# validation step exercises the same code path CI actually fires on.
|
||||
run_act act release -W .github/workflows/release.yml -e .github/act/release-published.json -n
|
||||
run_act act push -W .github/workflows/release-please.yml -e .github/act/push-feat.json -n
|
||||
run_act act pull_request_target -W .github/workflows/pr-health.yml -e .github/act/pr-governance-invalid.json -n
|
||||
run_act act pull_request_target -W .github/workflows/pr-health.yml -e .github/act/pr-governance-valid.json -n
|
||||
run_act act workflow_dispatch -W .github/workflows/docker.yml -e .github/act/docker-version.json -n
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue