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
4 changes: 3 additions & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ jobs:
python-version: "3.12"

- name: Run governance tests
run: uv run python -m unittest discover -s org-tools/governance/tests
run: >-
uv run --with markdown-it-py==3.0.0 --with markdown==3.10.2
python -m unittest discover -s org-tools/governance/tests

- name: Run label-sync tests
run: uv run python -m unittest discover -s org-tools/label-sync
Expand Down
3 changes: 2 additions & 1 deletion org-tools/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ You can run all unit tests locally from the root of the repository:

```bash
# Run all tests
uv run python3 -m unittest discover -s org-tools/governance/tests && \
uv run --with markdown-it-py==3.0.0 --with markdown==3.10.2 \
python3 -m unittest discover -s org-tools/governance/tests && \
uv run python3 -m unittest discover -s org-tools/label-sync && \
uv run python3 -m unittest discover -s org-tools/triage/tests && \
uv run --with jsonschema==4.26.0 --with markdown-it-py==3.0.0 --with markdown==3.10.2 \
Expand Down
37 changes: 28 additions & 9 deletions org-tools/governance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,23 @@ graph TD
C[GovernanceConfigParser]
D[GovernanceConfigValidator]
end
subgraph normative_detector.py
H[detect_normative_changes]
end
I[requirements_extractor]
B --> C
B --> D
B --> E
C -->|Reads Config| F(python-sdk-rules.yml)
C -->|Reads Config| F(ucp-rules.yml)
D -->|Static Sanity Analysis| F
E -->|Checks normative clauses in opted-in rules| H
H -->|Delegates parsing & classification| I
E -->|Evaluates approvals| G[GitHub API]
```

- **`pr_validator.py`**: The entrypoint script and validator logic.
- `PullRequestValidator`: Fetches PR metadata (changed files, reviews, authors) from GitHub, queries team memberships, and evaluates reviews against the governance rules.
- `PullRequestValidator`: Fetches PR metadata (changed files, reviews, authors, and — for Markdown files matched by a rule with `normative_escalation: true` — diffs/contents) from GitHub, queries team memberships, escalates those Markdown files to at least Level 3 (Technical Council) when they modify normative RFC 2119 clauses, and evaluates reviews against the governance rules.
- **`normative_detector.py`**: Normative RFC 2119 clause change detector for Markdown documents (`*.md`, `*.markdown`). Delegates Markdown parsing, RFC 2119 classification (`MUST`, `MUST NOT`, `SHALL`, `SHALL NOT`, `SHOULD`, `SHOULD NOT`, `MAY`, and prose `REQUIRED`, `RECOMMENDED`, `OPTIONAL`), exclusion filtering, and clause digest normalization directly to `org-tools/requirements-extractor`, while also flagging unbolded uppercase RFC 2119 keywords in prose.
- **`governance_config_parser.py`**: Helper module for loading rules and analyzing configurations.
- `GovernanceConfigParser`: Parses YAML configuration files into typed python dataclasses (`Team`, `GovernanceRule`, `GovernanceConfig`, etc.).
- `GovernanceConfigValidator`: Performs static analysis on the repository files to detect rule design flaws (overlapping rules or files falling into default fallbacks).
Expand Down Expand Up @@ -71,6 +78,7 @@ A list of review criteria matching specific file glob patterns:

- `patterns`: File paths or wildcards (supports `*`, `**`, `?` globs) matching target files.
- `excludes`: Optional glob patterns to exclude from the rule.
- `normative_escalation`: Optional boolean (default `false`). When `true`, Markdown files matched by this rule are escalated to at least Level 3 (Technical Council) if they add, remove, or modify normative RFC 2119 clauses (see [Normative Clause L3 Escalation](#step-2-pr-evaluation-logic) below). In `ucp-rules.yml`, only `documentation_spec` (`docs/specification/**`) opts in.
- `requires`: The specific approval targets:
- `team`: Requires an approval from an exact team.
- `min_team`: Requires an approval from the specified team or any team higher in the hierarchy.
Expand Down Expand Up @@ -113,12 +121,18 @@ When evaluating a pull request:

1. **Filter Author**: The PR author's own approvals are ignored.
2. **Match Rules**: Compares all modified files in the PR against the rules to collect the matching `RuleRequirement` sets. If a file matches no rule, the `fallback` requirements are applied.
3. **Resolve Approvals**:
3. **Normative Clause L3 Escalation (opt-in per rule)**: For any changed Markdown document (`*.md`, `*.markdown`) matched by a rule with `normative_escalation: true`, `normative_detector.py` (backed by `org-tools/requirements-extractor`) compares the base and head normative RFC 2119 clauses (excluding code blocks, inline code spans, HTML blocks/comments, headings, table headers, RFC 2119 boilerplate definitions, and schema field optionality annotations).
- If normative clauses (`MUST`, `MUST NOT`, `SHALL`, `SHALL NOT`, `SHOULD`, `SHOULD NOT`, `MAY`, or prose `REQUIRED`, `RECOMMENDED`, `OPTIONAL`) were added, removed, or modified, any requirement on that file with a tier below Level 3 (`level < 3`, such as `devops-maintainers` or `maintainers`) is automatically escalated to **Level 3 (Technical Council)** while preserving `min_approvals`.
- Domain-specific paths (`shopping`, `food`, `lodging`, `payment`/`payments`) escalate to the corresponding Domain Tech Council (`shopping-tech-council`, `food-tech-council`, `lodging-tech-council`, `payments-tech-council`), while cross-domain documents escalate to any Technical Council (Level 3) or higher.
- The escalation reason and touched RFC 2119 keywords are explicitly surfaced in both the Global PR Summary (`* **Reason:** ⚠️ ...`) and the File-by-File Breakdown (`* **Escalation:** ⚠️ ...`).
- **Fail-closed**: If neither the base/head file contents nor the diff patch can be retrieved from GitHub (for example, very large files or diffs), the file is escalated as if its normative clauses changed, and the reason states that the clauses could not be analyzed.
- Markdown files matched only by rules without the flag (such as general docs, `README.md`, or `AGENTS.md`), files that fall back to the `fallback` requirements, and repositories whose rules files do not opt in are not subject to normative clause escalation.
4. **Resolve Approvals**:
- Retrieves active PR reviews from GitHub.
- Checks if the reviewer is a proxy reviewer.
- Queries team memberships for reviewers from the GitHub API.
- Maps team memberships to hierarchical levels.
4. **Evaluate Gates**: Validates that the approvals fulfill every required target. Prints results and writes a review summary to `summary.txt`.
5. **Evaluate Gates**: Validates that the approvals fulfill every required target. Prints results and writes a review summary to `summary.txt`.

---

Expand Down Expand Up @@ -150,9 +164,14 @@ python3 scripts/pr_validator.py \
The test suite mock-patches the GitHub API and local git files to allow fast, isolated test runs:

```bash
# Run pr_validator tests
python3 org-tools/governance/tests/test_pr_validator.py

# Run config parser and validator tests
python3 org-tools/governance/tests/test_governance_config_parser.py
# Run all governance unit tests
uv run --with markdown-it-py==3.0.0 --with markdown==3.10.2 \
python3 -m unittest discover -s org-tools/governance/tests

# Or run individual test suites
uv run --with markdown-it-py==3.0.0 --with markdown==3.10.2 \
python3 org-tools/governance/tests/test_pr_validator.py
uv run --with markdown-it-py==3.0.0 --with markdown==3.10.2 \
python3 org-tools/governance/tests/test_normative_detector.py
uv run python3 org-tools/governance/tests/test_governance_config_parser.py
```
10 changes: 7 additions & 3 deletions org-tools/governance/docs/validation_report.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,20 @@ Each requirement in the summary is formatted with detailed progress:

```markdown
- **[Min Approvals] approval(s)** from [Target Team / Hierarchical Level]
- **Reason:** ⚠️ [Optional escalation reason when normative RFC 2119 clauses are modified]
- **Met:** [🟢 Yes / 🔴 No] ([Approved Count]/[Min Approvals] approved - approved by: [Reviewers])
- **Pending:** Needs [Remaining Count] approval(s) from [Target Team] ([Eligible Count] eligible reviewer(s) assigned). [Hint]
```

#### Key Fields:

1. **Requirement Line**: Describes the rule (e.g., `* **2 approvals** from team 'shopping-tech-council'` or `* **1 approval** from team 'shopping-tech-council' or higher in the UCP governance hierarchy`).
2. **Met Line**:
1. **Requirement Line**: Describes the rule (e.g., `* **2 approvals** from team 'shopping-tech-council'`, `* **1 approval** from team 'shopping-tech-council' or higher in the UCP governance hierarchy`, or `* **2 approvals** from any Technical Council (Level 3) or higher in the UCP governance hierarchy`).
2. **Reason Line** (only shown when escalated): Explains why a lower-level requirement (such as Level 1 `devops-maintainers` or Level 2 `maintainers`) was automatically escalated to **Level 3 (Technical Council)** because a Markdown document added, removed, or modified normative RFC 2119 clauses (`MUST`, `MUST NOT`, `SHALL`, `SHALL NOT`, `SHOULD`, `SHOULD NOT`, `MAY`, `REQUIRED`, `RECOMMENDED`, `OPTIONAL`).
3. **Met Line**:
- **🟢 Yes**: The requirement is fully satisfied.
- **🔴 No**: The requirement is not yet satisfied.
- **approved by**: Displays a comma-separated list of the authorized users whose reviews satisfied this requirement.
3. **Pending Line** (only shown if **Met** is **🔴 No**):
4. **Pending Line** (only shown if **Met** is **🔴 No**):
- Shows the number of remaining approvals needed.
- Displays the number of **eligible reviewers** currently assigned/requested on the PR who can satisfy this requirement.
- **Hint:** Explains the current status and next steps:
Expand All @@ -92,6 +94,7 @@ This section shows the validation status of every single changed file. This is e
```markdown
- **File:** `path/to/file.py`
- **Status:** [🟢 SATISFIED / 🔴 UNSATISFIED]
- **Escalation:** ⚠️ [Optional escalation reason when normative RFC 2119 clauses are modified]
- **Requirements:**
[List of individual requirements for this file]
```
Expand All @@ -101,6 +104,7 @@ This section shows the validation status of every single changed file. This is e
1. **Rule Matching**: The validator compares the file path against the glob patterns defined in your governance configuration file (e.g., `rules/python-sdk-rules.yml`).
2. **Cumulative Rules**: If a file matches multiple rules, it must satisfy the requirements of **all** matching rules.
3. **Fallback**: If a file does not match any custom rule in the configuration, the catch-all `fallback` requirement is applied.
4. **Normative Clause Escalation (Level 3 Technical Council, opt-in per rule)**: If a Markdown document (`*.md`, `*.markdown`) matched by a rule with `normative_escalation: true` (in `ucp`, `docs/specification/**`) adds, removes, or modifies normative RFC 2119 clauses (`MUST`, `MUST NOT`, `SHALL`, `SHALL NOT`, `SHOULD`, `SHOULD NOT`, `MAY`, or prose `REQUIRED`, `RECOMMENDED`, `OPTIONAL`), any lower-level requirement (`level < 3`) for that file is automatically escalated to at least **Level 3 (Technical Council)** while keeping its required `min_approvals` count. Requirements already at Level 3 or Level 4 (`governance-council`) remain unchanged. If the file contents and diff cannot be retrieved, the file is escalated as a precaution. Files matched only by rules without the flag are unaffected.

---

Expand Down
3 changes: 3 additions & 0 deletions org-tools/governance/rules/ucp-rules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ rules:
- name: "documentation_spec"
patterns:
- "docs/specification/**"
# Escalate Markdown edits that add, remove, or modify normative RFC 2119
# clauses to Level 3 (Technical Council).
normative_escalation: true
requires:
# Grants access to maintainers (L2), tech councils (L3) and
# governance-council (L4)
Expand Down
16 changes: 16 additions & 0 deletions org-tools/governance/scripts/governance_config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ class GovernanceRule:
patterns: List[str]
requires_all: List[RuleRequirement]
excluded_patterns: List[str] = field(default_factory=list)
# Opt-in: escalate Markdown files matched by this rule to Level 3 when
# they add, remove, or modify normative RFC 2119 clauses.
normative_escalation: bool = False

# Internal compiled pattern fields
_compiled_patterns: List[re.Pattern] = field(init=False, repr=False)
Expand Down Expand Up @@ -104,6 +107,12 @@ def _get_requirements_for_file(self, file_path: str) -> List[RuleRequirement]:

return merge_requirements(requirements)

def is_normative_escalation_file(self, file_path: str) -> bool:
"""Return True if any rule matching the file opts into normative clause escalation."""
return any(
rule.normative_escalation and rule.matches(file_path) for rule in self.rules
)

def get_applicable_requirements(
self, changed_files: List[str]
) -> Dict[str, List[RuleRequirement]]:
Expand Down Expand Up @@ -211,12 +220,19 @@ def _parse_rule(self, rule_data: dict, teams: Dict[str, Team]) -> GovernanceRule
if isinstance(excludes, str):
excludes = [excludes]

normative_escalation = rule_data.get("normative_escalation", False)
if not isinstance(normative_escalation, bool):
raise ValueError(
f"Rule '{name}' has a non-boolean 'normative_escalation' value."
)

requires_all = self._parse_requirements(rule_data.get("requires", []), teams)
return GovernanceRule(
name=name,
patterns=patterns,
requires_all=requires_all,
excluded_patterns=excludes,
normative_escalation=normative_escalation,
)

def _resolve_team(
Expand Down
Loading
Loading