Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/agent_governance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: Agent Governance

on:
pull_request:
types: [opened, synchronize, reopened, edited, ready_for_review]
types: [opened, synchronize, reopened, edited, ready_for_review, labeled, unlabeled]

permissions:
contents: read
Expand Down
4 changes: 3 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,9 @@ When splitting a function into a new `.cpp`, if its callers span LCAO and PW, pu
- Semantic questions such as module ownership, member-variable workflow state,
test sufficiency, and exception approval require human review.
- Exceptions must be recorded in the PR with reason, scope, risk, and a follow-up
cleanup plan.
cleanup plan, using the `### Governance Exception` template. CI downgrades a
covered blocker to a warning only after a maintainer applies the
`governance-exception-approved` label.
- After a refactor, propose brief lessons worth recording in this file, then
ask the developer whether to write them in; be cautious and skip unclear
or unverified lessons.
Expand Down
20 changes: 18 additions & 2 deletions docs/developers_guide/agent_governance.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ decisions.
| Header include guard | New `#pragma once` in a header | phase-one mechanical | hook + CI | high | block | added header lines (`.h`/`.hh`/`.hpp`/`.hxx`/`.cuh`) | Use `#ifndef`/`#define`/`#endif` guards; no exception allowed |
| Language baseline | C++11 compatibility | build/toolchain | CI | high | block | build/static tooling | Actual compiler/toolchain result wins |
| Global dependency budget | Net increase of `GlobalV`/`GlobalC`/`PARAM` references in code diff | phase-one mechanical + AI review | CI + AI review | high | block on net increase, warn on non-increasing added usage | added and removed code lines | Historical untouched usage and documentation mentions are not blocked; migration-neutral moves require reviewer rationale |
| New default parameter | Header declaration adds a default argument | phase-one mechanical + AI review | CI + AI review | high | block | header diff | High misuse risk |
| New default parameter | Header declaration adds a default argument | phase-one mechanical warning + AI review | CI + AI review | high | warn | header diff | High misuse risk; the single-line heuristic is not reliable enough to block, so AI and human review confirm |
| `.hpp` propagation | New `.hpp` or header includes `.hpp` | phase-one mechanical warning | CI + AI review | medium | warn | new files and added includes | Exception can be recorded in PR |
| Header dependency growth | Header diff adds includes | phase-one mechanical warning + AI review | CI + AI review | medium | warn | added header includes | Necessity is semantic and not mechanically decided |
| Member variable workflow switch | Key flow state hidden as mutable member state | AI review + human confirmation | AI + human review | high | human confirmation | semantic review | Static matching is unreliable |
Expand All @@ -101,7 +101,7 @@ decisions.
| Documentation sync | Behavior/interface docs updated | phase-one mechanical warning + AI review | CI + AI review | medium | warn | changed paths and PR body | Major behavior changes escalate to reviewers |
| PR metadata completeness | Issue, tests, behavior, INPUT, core impact, exceptions | phase-one mechanical | CI or GitHub bot | medium | warn | PR template fields | Not run by local hook |
| AI workflow | Interface lookup, uncertainty, verification report | AI review | AI review | high | warn | review transcript/output | Applies to AI agents |
| Exceptions | Reason, scope, risk, follow-up plan | human confirmation | human review + CI | high | human confirmation | PR exception section | CI checks presence, not approval |
| Exceptions | Reason, scope, risk, follow-up plan | human confirmation | human review + CI | high | human confirmation | PR exception section + approval label | CI downgrades a covered blocker to a warning only when the section is complete and a maintainer applied the `governance-exception-approved` label |

Implementations must use this matrix as the default baseline. Upgrading warnings
to blockers or converting human-confirmation rules into mechanical blockers
Expand Down Expand Up @@ -269,6 +269,22 @@ Use this template in the PR when a rule must be bypassed temporarily:
- Requested approver:
```

`Rule` names the checker rule(s) being excepted, using the exact rule names
from the governance summary (for example `Global dependency budget` or
`CMake linkage for new sources`). The CI checker handles the section as follows:

- An incomplete section (any field empty) produces a warning and changes
nothing else.
- A complete section without approval produces a warning that the exception is
awaiting approval; blocking findings still fail the check.
- A complete section plus the `governance-exception-approved` label, applied by
a maintainer other than the PR author, downgrades the blocking findings whose
rule appears in `Rule` to warnings. Findings marked `Exception allowed: no`
(LF line endings, `#pragma once`) are never downgraded.

Applying or removing the label re-runs the check. The local `--staged` hook has
no PR body or labels and therefore never applies exceptions.

## AI Review Finding Format

AI and bot review findings should use this shape:
Expand Down
109 changes: 107 additions & 2 deletions tools/03_code_analysis/agent_governance_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@
TEST_NAME_PATTERNS = ("test", "tests", "unittest", "pytest", "ctest", "case")
HETEROGENEOUS_MARKERS = ("/cuda/", "/rocm/", "/kernels/", "cuda/", "rocm/", "kernels/")
PR_SECTION_RE = re.compile(r"^###\s+(.+?)\s*$", re.MULTILINE)
EXCEPTION_SECTION = "Governance Exception"
EXCEPTION_APPROVED_LABEL = "governance-exception-approved"
EXCEPTION_FIELDS = (
"Rule",
"Reason",
"Scope",
"User or maintenance risk",
"Why the normal rule cannot be followed now",
"Follow-up cleanup plan",
"Requested approver",
)
EXCEPTION_FIELD_RE = re.compile(r"^[-*]\s*([^:]+):\s*(.*)$")


@dataclass
Expand Down Expand Up @@ -578,7 +590,7 @@ def check_cmake_linkage(findings: List[Finding], statuses: Dict[str, str], chang



def read_pr_body(event_path: Optional[str]) -> Optional[str]:
def read_pull_request(event_path: Optional[str]) -> Optional[dict]:
if not event_path:
return None
try:
Expand All @@ -588,12 +600,31 @@ def read_pr_body(event_path: Optional[str]) -> Optional[str]:
return None
if "pull_request" not in payload or not isinstance(payload["pull_request"], dict):
return None
body = payload["pull_request"].get("body")
return payload["pull_request"]


def read_pr_body(event_path: Optional[str]) -> Optional[str]:
pull_request = read_pull_request(event_path)
if pull_request is None:
return None
body = pull_request.get("body")
if body is None:
return ""
return str(body)


def read_pr_labels(event_path: Optional[str]) -> List[str]:
pull_request = read_pull_request(event_path)
if pull_request is None:
return []
labels = pull_request.get("labels") or []
return [
str(label.get("name", ""))
for label in labels
if isinstance(label, dict)
]


def pr_sections(body: str) -> Dict[str, str]:
matches = list(PR_SECTION_RE.finditer(body))
sections: Dict[str, str] = {}
Expand Down Expand Up @@ -751,11 +782,84 @@ def check_documentation_warning(findings: List[Finding], changed: Sequence[str],
)


def parse_governance_exception(body: str) -> Optional[Dict[str, str]]:
sections = pr_sections(body)
content = sections.get(EXCEPTION_SECTION)
if content is None:
return None
fields: Dict[str, str] = {}
for raw in content.splitlines():
match = EXCEPTION_FIELD_RE.match(raw.strip())
if match:
fields[match.group(1).strip().lower()] = match.group(2).strip()
return fields


def apply_governance_exception(
findings: List[Finding],
body: Optional[str],
labels: Sequence[str],
) -> None:
"""Downgrade blocking findings covered by an approved Governance Exception.

The PR body records the exception; the maintainer-applied label is the
approval. Findings with allow_exception=False are never downgraded.
"""
if body is None:
return
approved = EXCEPTION_APPROVED_LABEL in labels
Comment thread
Critsium-xy marked this conversation as resolved.
Resolved
fields = parse_governance_exception(body)
if fields is None or not any(fields.values()):
if approved:
add_finding(
findings,
"Governance exception record",
WARN,
"pull_request.body",
None,
f"PR carries the `{EXCEPTION_APPROVED_LABEL}` label but has no filled `### {EXCEPTION_SECTION}` section.",
"Record the exception with the template in docs/developers_guide/agent_governance.md, or remove the label.",
)
return
missing = [field for field in EXCEPTION_FIELDS if not fields.get(field.lower())]
if missing:
add_finding(
findings,
"Governance exception record",
WARN,
"pull_request.body",
None,
f"`### {EXCEPTION_SECTION}` section is incomplete; missing fields: " + ", ".join(missing) + ".",
"Fill every field of the exception template; incomplete exceptions do not downgrade blocking findings.",
)
return
if not approved:
add_finding(
findings,
"Governance exception record",
WARN,
"pull_request.body",
None,
f"Governance exception requested for: {fields['rule']}; awaiting maintainer approval.",
f"A maintainer approves the exception by applying the `{EXCEPTION_APPROVED_LABEL}` label.",
)
return
excepted_rules = fields["rule"].lower()
Comment thread
Critsium-xy marked this conversation as resolved.
Resolved
for finding in findings:
if finding.severity != BLOCK or not finding.allow_exception:
continue
if finding.rule.lower() not in excepted_rules:
Comment thread
Critsium-xy marked this conversation as resolved.
Resolved
continue
finding.severity = WARN
finding.reason += " Downgraded from error by the approved governance exception."


def collect_findings(root: Path, args: argparse.Namespace) -> List[Finding]:
findings: List[Finding] = []
statuses, changed = changed_paths(root, args)
lines, removed_lines = changed_lines(root, args)
body = read_pr_body(args.event_path)
labels = read_pr_labels(args.event_path)
body_text = body or ""

check_line_endings(findings, root, changed, statuses, args)
Expand All @@ -770,6 +874,7 @@ def collect_findings(root: Path, args: argparse.Namespace) -> List[Finding]:
check_test_evidence_warning(findings, changed, body_text)
check_heterogeneous_test_warning(findings, changed, body_text)
check_documentation_warning(findings, changed, body_text)
apply_governance_exception(findings, body, labels)
return findings


Expand Down
94 changes: 94 additions & 0 deletions tools/03_code_analysis/test_agent_governance_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,100 @@ def test_allows_existing_pragma_once_on_unchanged_lines(self):

self.assertNotIn("No #pragma once", result.stdout)

def write_exception_event(self, rule, labels, approver="@maintainer"):
event = self.repo / "event.json"
body = "\n".join(
[
"### Governance Exception",
f"- Rule: {rule}",
"- Reason: file is consumed by a generated build list.",
"- Scope: source/source_base/generated.cpp only.",
"- User or maintenance risk: low.",
"- Why the normal rule cannot be followed now: generator lands in a follow-up PR.",
"- Follow-up cleanup plan: wire the file through CMakeLists.txt in the follow-up.",
f"- Requested approver: {approver}",
]
)
event.write_text(
json.dumps(
{"pull_request": {"body": body, "labels": [{"name": name} for name in labels]}}
)
)
return event

def test_approved_exception_downgrades_covered_blocker(self):
self.write("source/source_base/generated.cpp", "int generated();\n")
head = self.commit_change()
event = self.write_exception_event(
"CMake linkage for new sources", ["governance-exception-approved"]
)

result = self.run_checker("--base", self.base, "--head", head, "--event-path", str(event))

self.assert_warns_with_success(result, "CMake linkage for new sources")
self.assertIn("Downgraded from error by the approved governance exception", result.stdout)

def test_unapproved_exception_keeps_blocker(self):
self.write("source/source_base/generated.cpp", "int generated();\n")
head = self.commit_change()
event = self.write_exception_event("CMake linkage for new sources", [])

result = self.run_checker("--base", self.base, "--head", head, "--event-path", str(event))

self.assert_blocked_by(result, "CMake linkage for new sources")
self.assertIn("awaiting maintainer approval", result.stdout)

def test_incomplete_exception_keeps_blocker_even_with_label(self):
self.write("source/source_base/generated.cpp", "int generated();\n")
head = self.commit_change()
event = self.write_exception_event(
"CMake linkage for new sources", ["governance-exception-approved"], approver=""
)

result = self.run_checker("--base", self.base, "--head", head, "--event-path", str(event))

self.assert_blocked_by(result, "CMake linkage for new sources")
self.assertIn("missing fields: Requested approver", result.stdout)

def test_approved_exception_does_not_downgrade_other_rules(self):
self.write("source/source_base/generated.cpp", "int generated();\n")
head = self.commit_change()
event = self.write_exception_event(
"Global dependency budget", ["governance-exception-approved"]
)

result = self.run_checker("--base", self.base, "--head", head, "--event-path", str(event))

self.assert_blocked_by(result, "CMake linkage for new sources")
self.assertNotIn("Downgraded from error", result.stdout)

def test_approved_exception_never_downgrades_non_exceptable_rule(self):
self.write("source/source_base/new_header.h", "#pragma once\n\nint foo();\n")
head = self.commit_change()
event = self.write_exception_event("No #pragma once", ["governance-exception-approved"])

result = self.run_checker("--base", self.base, "--head", head, "--event-path", str(event))

self.assert_blocked_by(result, "No #pragma once")
self.assertNotIn("Downgraded from error", result.stdout)

def test_warns_when_approval_label_has_no_exception_section(self):
event = self.repo / "event.json"
event.write_text(
json.dumps(
{
"pull_request": {
"body": "### What's changed?\n- Docs tweak.\n",
"labels": [{"name": "governance-exception-approved"}],
}
}
)
)

result = self.run_checker("--event-path", str(event))

self.assert_warns_with_success(result, "Governance exception record")


if __name__ == "__main__":
unittest.main()
Loading