Skip to content
Closed
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
12 changes: 9 additions & 3 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# Rule machinery: changes need review from a core developer other than the
# author. Deliberately narrow: pyproject.toml files and workflow files are not
# owned, so dependency bots and everyday CI edits are not blocked.
# Rule machinery: the files that define and check this repository's rules.
# By team convention, a core developer other than the author approves changes
# to them (sim.process.rule-machinery-code-owned). GitHub doesn't enforce
# this: the default branch's ruleset needs one approving review from anyone
# with write access and doesn't require code-owner review. Listing a path here
# only makes GitHub request a review from its owners on pull requests that
# change it and aren't drafts.
# Deliberately narrow: pyproject.toml files and workflow files are not listed,
# so dependency bots and everyday CI edits don't request that review.
/rules/ @PolicyEngine/core-developers
/scripts/rules/ @PolicyEngine/core-developers
/scripts/run_quality_guards.py @PolicyEngine/core-developers
Expand Down
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`.

- Checked: `sim.agents.adapters-point-to-agents-md`, `sim.process.pr-tracking-issue`, `sim.process.rules-registry`, `sim.process.stacked-prs-full-ci-at-bottom`
- `sim.process.agents-never-bypass`: Agents never bypass branch rules, even with admin credentials: no gh pr merge --admin and no bypass option; a blocked merge is reported to a human. (doc-only, not checked: follow it yourself)
- `sim.process.rule-machinery-code-owned`: Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. (doc-only, not checked: follow it yourself)
- `sim.process.rule-machinery-code-owned`: Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. This is a team convention that GitHub doesn't enforce: the default branch's ruleset needs one approving review from anyone with write access and doesn't require code-owner review. (doc-only, not checked: follow it yourself)
- `sim.process.same-repo-draft-prs`: PRs are opened as drafts from branches in PolicyEngine/policyengine-sim-api, never from forks. (doc-only, not checked: follow it yourself)

<!-- END GENERATED: rules -->
4 changes: 2 additions & 2 deletions docs/generated/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf
- `sim.observability.id-is-diagnostic-only`: observability_id has diagnostic meaning only: never use it for idempotency, authorization, database identity, routing or business logic.
- `sim.observability.registered-stages-only`: Span names come from the stage plans in policyengine_simulation_observability.stages; runtime code never adds span-name string literals.
- `sim.process.agents-never-bypass`: Agents never bypass branch rules, even with admin credentials: no gh pr merge --admin and no bypass option; a blocked merge is reported to a human.
- `sim.process.rule-machinery-code-owned`: Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author.
- `sim.process.rule-machinery-code-owned`: Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. This is a team convention that GitHub doesn't enforce: the default branch's ruleset needs one approving review from anyone with write access and doesn't require code-owner review.
- `sim.process.same-repo-draft-prs`: PRs are opened as drafts from branches in PolicyEngine/policyengine-sim-api, never from forks.
- `sim.quality.every-project-tested`: Every project and library with unit tests runs them in the PR test matrix.
- `sim.quality.ruff-format-tests`: Test code is formatted with ruff format too; CI currently checks only src directories.
Expand Down Expand Up @@ -433,7 +433,7 @@ flowchart LR
| `sim.agents.adapters-point-to-agents-md` | CLAUDE.md and .github/copilot-instructions.md only point to AGENTS.md; skills and rules are listed in AGENTS.md alone. | mechanical | enforced | yes | quality guard `rules-registry` | `pr.yml:quality-guards` | yes |
| `sim.process.agents-never-bypass` | Agents never bypass branch rules, even with admin credentials: no gh pr merge --admin and no bypass option; a blocked merge is reported to a human. | mechanical | doc-only | no | nothing | — | — |
| `sim.process.pr-tracking-issue` | The first line of every PR's description is Fixes #ISSUE_NUMBER, naming an open issue, not a pull request, in this repository; automated PRs open or reuse such an issue. | mechanical | enforced | yes | CI job `pr-description` | `pr-description.yml:pr-description` | yes |
| `sim.process.rule-machinery-code-owned` | Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. | mechanical | doc-only | no | nothing | — | — |
| `sim.process.rule-machinery-code-owned` | Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. This is a team convention that GitHub doesn't enforce: the default branch's ruleset needs one approving review from anyone with write access and doesn't require code-owner review. | mechanical | doc-only | no | nothing | — | — |
| `sim.process.rules-registry` | Every rule is registered in rules/registry.toml, is enforced where it claims to be, and its rendered views are current. | mechanical | enforced | yes | quality guard `rules-registry` | `pr.yml:quality-guards` | **unproven** |
| `sim.process.same-repo-draft-prs` | PRs are opened as drafts from branches in PolicyEngine/policyengine-sim-api, never from forks. | mechanical | doc-only | no | nothing | — | — |
| `sim.process.stacked-prs-full-ci-at-bottom` | In a stack of pull requests, only the lowest unmerged one runs the full PR workflow and the Modal image smoke. Every pr.yml job is needed by the CI complete gate, which fails on the pull requests above it and otherwise passes only when every job passed, so a stack merges bottom-up and nothing merges untested. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-executor/tests/test_ci_complete_gate.py` | `pr.yml:test`, `pr.yml:ci-complete` | yes |
Expand Down
2 changes: 1 addition & 1 deletion rules/registry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -1460,7 +1460,7 @@ test = "tests/test_ci_complete_gate.py::test_rule_proof_gate_check"
[[rule]]
id = "sim.process.rule-machinery-code-owned"
category = "process"
statement = "Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author."
statement = "Changes to the rule machinery (rules/, scripts/rules/, scripts/run_quality_guards.py and CODEOWNERS) need approval from a core developer other than the author. This is a team convention that GitHub doesn't enforce: the default branch's ruleset needs one approving review from anyone with write access and doesn't require code-owner review."
rationale = ".github/CODEOWNERS"
scope = [".github/CODEOWNERS"]
tier = "mechanical"
Expand Down
56 changes: 56 additions & 0 deletions scripts/rules/tests/test_code_owner_rule.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
"""Tests that the code-owner rule says what GitHub enforces when merging.

The default branch's ruleset needs one approving review from anyone with write
access and doesn't require code-owner review, so a core developer's approval of
rule changes is a team convention, and the rule's statement has to say so.
"""

from __future__ import annotations

import unittest
from pathlib import Path

from scripts.rules.load import load_registry
from scripts.rules.model import Rule, Status

REPO_ROOT = Path(__file__).resolve().parents[3]
CODE_OWNER_RULE_ID = "sim.process.rule-machinery-code-owned"
REQUIRED_APPROVAL = (
"Changes to the rule machinery (rules/, scripts/rules/, "
"scripts/run_quality_guards.py and CODEOWNERS) need approval from a core "
"developer other than the author."
)


class CodeOwnerRuleTest(unittest.TestCase):
"""The rule keeps its requirement and says GitHub doesn't enforce it."""

def code_owner_rule(self) -> Rule:
"""Return the code-owner rule from the repository's own registry."""
registry, violations = load_registry(REPO_ROOT)
if registry is None:
self.fail(f"rules/registry.toml doesn't load: {violations}")
rule = registry.rule(CODE_OWNER_RULE_ID)
if rule is None:
self.fail(f"{CODE_OWNER_RULE_ID} isn't in rules/registry.toml")
return rule

def test_rule_still_asks_for_a_core_developer_approval(self) -> None:
"""The statement still starts with the approval the rule asks for."""
statement = self.code_owner_rule().statement
self.assertTrue(statement.startswith(REQUIRED_APPROVAL), statement)

def test_rule_stays_doc_only(self) -> None:
"""No check enforces the rule, so it stays doc-only."""
self.assertIs(self.code_owner_rule().status, Status.DOC_ONLY)

def test_rule_says_github_does_not_require_code_owner_review(self) -> None:
"""The statement says what the ruleset needs, and that it's a convention."""
statement = self.code_owner_rule().statement
self.assertIn("team convention that GitHub doesn't enforce", statement)
self.assertIn("one approving review from anyone with write access", statement)
self.assertIn("doesn't require code-owner review", statement)


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