Skip to content

feat: generate CODEOWNERS as code from a per-area taxonomy#1347

Open
dagil-nvidia wants to merge 8 commits into
mainfrom
dagil-nvidia/codeowners-as-code
Open

feat: generate CODEOWNERS as code from a per-area taxonomy#1347
dagil-nvidia wants to merge 8 commits into
mainfrom
dagil-nvidia/codeowners-as-code

Conversation

@dagil-nvidia

@dagil-nvidia dagil-nvidia commented Jul 12, 2026

Copy link
Copy Markdown

Overview:

Ports ai-dynamo/dynamo's CODEOWNERS-as-code toolchain (ai-dynamo/dynamo#10715, merged Jul 12) to aiconfigurator: one areas.yaml maps paths to the existing five teams, CI validates 100% coverage on every PR, and generated ownership artifacts cannot drift from their sources.

Details:

  • Existing routing is preserved: an exhaustive comparison across the 1,494 pre-migration files found zero regressions. Runtime, generator, infra, test, policy, and deployment-guide paths retain their prior maintainer co-ownership. The new taxonomy closes the real catch-all gaps: rust/ (82 files) and 30 agent/instruction/repository-metadata files. The stale /src/aiconfigurator/eval/ rule is removed because the path no longer exists.
  • The generated file moves to the repo root: .github/CODEOWNERS and the .github/codeowners/ toolchain directory cannot coexist on case-insensitive filesystems (macOS default), and GitHub reads the root location equally. CI rejects future .github/CODEOWNERS or docs/CODEOWNERS files that would shadow it.
  • .github/codeowners/ carries the self-contained toolchain: coverage gate, emitter, reviewer lookup, min-approver audit, fail-closed schema/roster validation, external contributor support, and 100 tests. Matcher, invalid-base, incomplete-roster, generated-deletion, removed-schema, and policy-parity regressions are covered.
  • New CI workflow (codeowners.yml): pinned dependencies, unit/policy tests, strict coverage, shadow-file rejection, and drift detection for CODEOWNERS and CONTRIBUTORS.md. Unclaimed paths or stale/deleted generated outputs fail the check.
  • Existing reviewer workflow repaired: random reviewer assignment now sparse-checks and reads the generated root CODEOWNERS path.

Where Should the Reviewer Start?

.github/codeowners/areas.yaml contains the ownership policy. Then review .github/workflows/codeowners.yml, the generated CODEOWNERS, and the repository-specific policy test in .github/codeowners/test_aic_policy.py.

Related Issues: (Use One of the Action Keywords Closes / Fixes / Resolves / Relates to)

🤖 Generated with Claude Code


Open in Devin Review

Summary by CodeRabbit

  • New Features
    • Introduced CODEOWNERS-as-code area taxonomy to generate CODEOWNERS and CONTRIBUTORS.md.
    • Added tooling to answer “who owns what?”, inspect reviewers, and compute minimal approver coverage.
  • Documentation
    • Added guides for ownership regeneration, external contributor scoping, and CI enforcement.
  • Bug Fixes
    • Strengthened fail-closed coverage to prevent tracked paths from falling to catch-all.
  • Tests
    • Added/expanded end-to-end and validation tests for matching, resolution, and rendering.
  • Chores
    • Updated CI to regenerate/verify artifacts and fail on drift; removed legacy repo CODEOWNERS config; pinned workflow actions.

Ports ai-dynamo/dynamo's CODEOWNERS-as-code toolchain (dynamo#10715).
One source of truth (.github/codeowners/areas.yaml) maps paths to the
existing five teams; CI validates 100% coverage on every PR and blocks
drift between the source and the generated outputs.

- routing reproduces the previous hand-written .github/CODEOWNERS
  exactly (same teams, same shared lines), and closes its gaps: rust/
  (82 files) and src/spica/ no longer fall to the catch-all, and the
  dead /src/aiconfigurator/eval/ rule is gone
- the generated file moves to the repo root: .github/CODEOWNERS and
  .github/codeowners/ cannot coexist on case-insensitive filesystems
  (default macOS), and GitHub reads the root location equally
- .github/codeowners/ carries the self-contained toolchain (PyYAML +
  git only): coverage gate, emitter, who_owns lookup, min-approvers
  audit, unit tests (68), and an external_contributors.yaml scaffold
  for individual area-scoped co-ownership (drives CONTRIBUTORS.md)
- .github/workflows/codeowners.yml runs the tests, the strict coverage
  gate, and the drift check on every PR

Signed-off-by: Dan Gil <dagil@nvidia.com>
@dagil-nvidia
dagil-nvidia requested review from a team as code owners July 12, 2026 21:01
@github-actions github-actions Bot added the feat label Jul 12, 2026
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The change replaces handwritten CODEOWNERS rules with declarative ownership data, matching and generation tools, contributor metadata, ownership query and approval-analysis CLIs, tests, documentation, and CI checks for coverage and generated-file drift.

Changes

CODEOWNERS pipeline

Layer / File(s) Summary
Ownership model and matching
.github/codeowners/areas.yaml, .github/codeowners/codeowners_match.py
Defines ownership areas, matching semantics, resolution, co-ownership, coverage detection, and minimal rule generation.
Generated ownership artifacts
.github/codeowners/emit_codeowners.py, .github/codeowners/external_contributors.yaml, CONTRIBUTORS.md
Generates root CODEOWNERS and contributor documentation, including optional external co-owners.
Ownership query and approval analysis
.github/codeowners/who_owns.py, .github/codeowners/min_approvers.py
Resolves owners for paths or changes, expands teams, and computes minimum approver counts from PR data.
Coverage validation and CI
.github/codeowners/build_codeowners.py, .github/workflows/codeowners.yml
Reports coverage, runs strict validation and tests, blocks shadow files, and detects stale generated outputs.
Behavioral verification
.github/codeowners/test_*.py
Tests matching, resolution, generation, contributor handling, changed-file discovery, routing, and fail-closed validation.
Repository integration
.github/codeowners/README.md, .github/workflows/random-review-assignment.yml, .agents/skills/aic-codeowners/SKILL.md, AGENTS.md, CONTRIBUTING.md
Documents ownership operations, validates routing, updates reviewer assignment to use root CODEOWNERS, and removes the prior root ownership configuration.

Estimated code review effort: 4 (Complex) | ~60 minutes

Poem

YAML blooms with rules,
Paths find their watching owners,
CI guards the gate.
Generated leaves align,
Reviewers follow the trail.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: generating CODEOWNERS from a per-area taxonomy.
Description check ✅ Passed The description follows the template with Overview, Details, review-start guidance, and a related issue reference.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Open in Devin Review

Comment thread CODEOWNERS
Comment thread .github/codeowners/areas.yaml

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0db218b854

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CODEOWNERS Outdated
Comment thread .github/codeowners/areas.yaml
Comment thread .github/codeowners/areas.yaml
Comment thread CODEOWNERS Outdated
Comment thread CODEOWNERS

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🧹 Nitpick comments (5)
.github/codeowners/areas.yaml (1)

67-74: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Policy source itself isn't covered by the dual-review shared rule.

The comment says ownership-policy changes should get infra + docs visibility, but that's only wired to the generated CODEOWNERS file (line 73), not to areas.yaml itself — the actual source of truth. areas.yaml lives under .github/ and is solely owned by infra (Line 46 of this same file), so an areas.yaml-only diff auto-requests only infra.

In practice this is partially mitigated: the drift-check CI step forces any effective ownership change to also touch the co-owned CODEOWNERS file before merge is green, which does pull in docs visibility. But that's an indirect path — reviewers get requested off the generated artifact, not the source, and there's a window where a PR could be reviewed (before CI catches drift) with only infra requested. Adding the source file directly closes that gap for defense-in-depth.

🔐 Proposed fix
 shared:
   # Mirrors the hand-written file: tests touch product behavior and tooling.
   - glob: tests/
     owners: [runtime, generators, infra]
   # Ownership policy changes require infra and maintainer review (docs area
   # is the maintainers team), mirroring the previous hand-written policy.
   - glob: CODEOWNERS
     owners: [infra, docs]
+  - glob: .github/codeowners/areas.yaml
+    owners: [infra, docs]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/codeowners/areas.yaml around lines 67 - 74, Update the shared
ownership rules in areas.yaml so the policy source file itself,
.github/codeowners/areas.yaml, is covered by the same infra and docs dual-review
requirement as CODEOWNERS. Preserve the existing generated CODEOWNERS rule and
avoid changing unrelated ownership entries.
.github/codeowners/codeowners_match.py (2)

502-524: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

cost() and choose() duplicate the same per-candidate scoring formula.

Both loops compute x = 0 if c == inh else 1; x += ...t != c...; x += sum(cost(ch, c) ...) independently. If the cost formula ever changes, it's easy to update one and miss the other, silently desyncing the "cheapest cost" from the "chosen team" and corrupting the minimal-cover output. Extracting the shared scoring expression removes that divergence risk.

♻️ Proposed fix
     memo: dict[tuple[str, str], int] = {}
 
+    def _rule_cost(d: str, inh: str, c: str) -> int:
+        x = 0 if c == inh else 1
+        x += sum(1 for _, t in dir_files.get(d, ()) if t != c)
+        x += sum(cost(ch, c) for ch in children.get(d, ()))
+        return x
+
     def cost(d: str, inh: str) -> int:
         key = (d, inh)
         if key not in memo:
-            best = None
-            for c in {inh} | teams_under(d):
-                x = 0 if c == inh else 1
-                x += sum(1 for _, t in dir_files.get(d, ()) if t != c)
-                x += sum(cost(ch, c) for ch in children.get(d, ()))
-                best = x if best is None else min(best, x)
-            memo[key] = best or 0
+            memo[key] = min(_rule_cost(d, inh, c) for c in {inh} | teams_under(d))
         return memo[key]
 
     def choose(d: str, inh: str) -> str:
-        best_c, best_x = inh, None
-        for c in [inh, *sorted(teams_under(d) - {inh})]:
-            x = 0 if c == inh else 1
-            x += sum(1 for _, t in dir_files.get(d, ()) if t != c)
-            x += sum(cost(ch, c) for ch in children.get(d, ()))
-            if best_x is None or x < best_x:
-                best_c, best_x = c, x
-        return best_c
+        candidates = [inh, *sorted(teams_under(d) - {inh})]
+        return min(candidates, key=lambda c: (_rule_cost(d, inh, c), candidates.index(c)))

Re-run test_codeowners.py's minimal-cover tests after applying, to confirm tie-breaking (inherit-first, then alphabetical) is preserved.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/codeowners/codeowners_match.py around lines 502 - 524, Extract the
duplicated per-candidate scoring formula from cost() and choose() into a shared
local helper that accepts d, inh, and candidate c, then use it in both loops.
Preserve the existing candidate ordering and strict comparison so inherit-first,
alphabetical tie-breaking remains unchanged.

52-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Area TypedDict is total=False but label/github_team are treated as required elsewhere.

compute_resolution accesses a["label"] and a["github_team"] via bracket notation (lines 436, 448-451) — a malformed areas.yaml area entry missing either key raises a raw KeyError instead of the friendly SystemExit this codebase otherwise uses for config errors (see _github()/contributor_level() in emit_codeowners.py). Only path_globs is genuinely optional (already accessed via .get()).

Since the project targets Python 3.12 (per .github/workflows/codeowners.yml), typing.Required/NotRequired (PEP 655, available since 3.11) can express this precisely instead of blanket total=False.

🏷️ Proposed fix
-class Area(TypedDict, total=False):
+class Area(TypedDict):
     """Area entry as declared in ``areas.yaml``."""
 
     label: str
     github_team: str
-    path_globs: list[str]
+    path_globs: NotRequired[list[str]]

(also add NotRequired to the typing import on line 45.) Consider pairing this with an explicit validation check in compute_resolution that raises a friendly SystemExit for a missing label/github_team, matching the UX already established for external_contributors.yaml.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/codeowners/codeowners_match.py around lines 52 - 57, Update the Area
TypedDict to require label and github_team while marking only path_globs as
NotRequired, adding the necessary typing import. In compute_resolution, validate
that both required keys are present and raise the established friendly
SystemExit configuration error instead of allowing a KeyError.
.github/workflows/codeowners.yml (1)

34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Unpinned pip install in CI.

pyyaml/pytest are installed without version constraints, so this job can silently pick up a newer (possibly breaking or yanked) release than what's declared for local dev, causing non-reproducible CI failures unrelated to the PR under test.

Pin to the versions declared for the project (e.g. mirror pyproject.toml's pyyaml/pytest constraints) rather than always installing latest.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/codeowners.yml at line 34, Update the dependency
installation step in the workflow to pin both pyyaml and pytest to the versions
or constraints declared in pyproject.toml, keeping CI aligned with the project’s
local development dependencies instead of installing unconstrained latest
releases.
.github/codeowners/min_approvers.py (1)

60-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate gh api .../members call vs who_owns.py::_gh_fetch (Lines 74-88).

Same argv shape (gh api orgs/{org}/teams/{slug}/members --paginate --jq ".[].login") is implemented independently in two CLIs. If pagination, auth handling, or error semantics need to change, both copies must be updated in lockstep.

This spans two files, so a one-click suggestion isn't safe here — the smallest concrete next step is extracting a shared fetch_team_members(org, slug) helper into codeowners_match.py (already imported by both CLIs) and having both who_owns.py::_gh_fetch and this function call it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/codeowners/min_approvers.py around lines 60 - 72, Extract the
duplicated team-member API invocation into a shared fetch_team_members(org,
slug) helper in codeowners_match.py. Update min_approvers.py’s current
subprocess call and who_owns.py::_gh_fetch to use that helper, preserving the
existing pagination, jq login output, authentication, and error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/codeowners/build_codeowners.py:
- Line 30: Remove the unused `noqa: E402` suppression from the import of
`compute_resolution`, `load_tree`, and `match` in the codeowners build script,
leaving the import itself unchanged.

In @.github/codeowners/codeowners_match.py:
- Line 45: Update the typing imports in codeowners_match.py to import Iterable
from collections.abc instead of typing, while keeping TypedDict imported from
typing.
- Around line 186-188: Update the _compiled function decorator from
functools.lru_cache(maxsize=None) to functools.cache to satisfy UP033,
preserving the existing unbounded caching behavior and function implementation.

In @.github/codeowners/emit_codeowners.py:
- Around line 43-50: Remove the unused “noqa: E402” suppression from the
codeowners_match import in emit_codeowners.py, leaving the imported symbols and
import behavior unchanged.

In @.github/codeowners/min_approvers.py:
- Around line 44-51: Update load_rosters_tsv so parsing each non-empty roster
line does not unpack an arbitrary number of tab-separated fields; validate that
each line has exactly one separator and raise a clear, line-specific error for
extra or trailing tabs while preserving the existing members mapping for valid
entries.
- Around line 130-136: Update the corpus-reading loop around
args.corpus.read_text().splitlines() to skip lines whose contents are blank or
whitespace-only before calling json.loads. Preserve JSON parsing and owner-set
construction for non-blank lines, matching the blank-line handling in
load_rosters_tsv.
- Around line 54-76: Update fetch_rosters to surface each team lookup failure
instead of silently continuing: when the gh API call raises OSError or
subprocess.CalledProcessError, emit the repository’s established warning for
unavailable team members, such as _warn_people_unavailable, including the
affected team, while preserving the existing successful roster population and
loop behavior.
- Line 41: Remove the unused `# noqa: E402` annotation from the
`parse_codeowners, resolve_owners` import in `min_approvers.py`, matching the
corresponding cleanup in `who_owns.py`; leave the import and its behavior
unchanged.

In @.github/codeowners/test_codeowners.py:
- Around line 27-28: Remove the unused # noqa: E402 directives from the who_owns
and codeowners_match imports in test_codeowners.py, leaving the import
statements otherwise unchanged.
- Line 38: Remove the unused # noqa: E402 suppression from the emit_codeowners
import in the test module, leaving the import unchanged otherwise.

In @.github/codeowners/who_owns.py:
- Around line 36-41: The import in who_owns.py has an unused noqa: E402
directive; remove the directive from the codeowners_match import while
preserving the imported symbols anchor, match, parse_codeowners, and
resolve_owners.
- Around line 148-192: Remove the bare-diff fallback from changed_files and
retain only the base...HEAD and base revision comparisons. Ensure an invalid
base cannot be treated as a successful empty lookup, while preserving the
existing error propagation and untracked-file handling.

---

Nitpick comments:
In @.github/codeowners/areas.yaml:
- Around line 67-74: Update the shared ownership rules in areas.yaml so the
policy source file itself, .github/codeowners/areas.yaml, is covered by the same
infra and docs dual-review requirement as CODEOWNERS. Preserve the existing
generated CODEOWNERS rule and avoid changing unrelated ownership entries.

In @.github/codeowners/codeowners_match.py:
- Around line 502-524: Extract the duplicated per-candidate scoring formula from
cost() and choose() into a shared local helper that accepts d, inh, and
candidate c, then use it in both loops. Preserve the existing candidate ordering
and strict comparison so inherit-first, alphabetical tie-breaking remains
unchanged.
- Around line 52-57: Update the Area TypedDict to require label and github_team
while marking only path_globs as NotRequired, adding the necessary typing
import. In compute_resolution, validate that both required keys are present and
raise the established friendly SystemExit configuration error instead of
allowing a KeyError.

In @.github/codeowners/min_approvers.py:
- Around line 60-72: Extract the duplicated team-member API invocation into a
shared fetch_team_members(org, slug) helper in codeowners_match.py. Update
min_approvers.py’s current subprocess call and who_owns.py::_gh_fetch to use
that helper, preserving the existing pagination, jq login output,
authentication, and error behavior.

In @.github/workflows/codeowners.yml:
- Line 34: Update the dependency installation step in the workflow to pin both
pyyaml and pytest to the versions or constraints declared in pyproject.toml,
keeping CI aligned with the project’s local development dependencies instead of
installing unconstrained latest releases.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 33427065-7ee6-4993-a3cd-188410215301

📥 Commits

Reviewing files that changed from the base of the PR and between d901a28 and 0db218b.

⛔ Files ignored due to path filters (1)
  • CODEOWNERS is excluded by none and included by none
📒 Files selected for processing (12)
  • .github/CODEOWNERS
  • .github/codeowners/README.md
  • .github/codeowners/areas.yaml
  • .github/codeowners/build_codeowners.py
  • .github/codeowners/codeowners_match.py
  • .github/codeowners/emit_codeowners.py
  • .github/codeowners/external_contributors.yaml
  • .github/codeowners/min_approvers.py
  • .github/codeowners/test_codeowners.py
  • .github/codeowners/who_owns.py
  • .github/workflows/codeowners.yml
  • CONTRIBUTORS.md
💤 Files with no reviewable changes (1)
  • .github/CODEOWNERS
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
  • GitHub Check: CodeRabbit
  • GitHub Check: Collect snapshot (new)
  • GitHub Check: Collect snapshot (old)
  • GitHub Check: Build and Test (unit)
  • GitHub Check: Build and Test (e2e)
  • GitHub Check: Cargo Deny
  • GitHub Check: Rust/Python engine-step parity
⚠️ CI failures not shown inline (1)

GitHub Actions: Lint and Format / Lint and Format (Ruff): feat: generate CODEOWNERS as code from a per-area taxonomy

Conclusion: failure

View job details

##[group]Run ruff check .
 �[36;1mruff check .�[0m
 shell: /usr/bin/bash -e {0}
 env:
   pythonLocation: /opt/hostedtoolcache/Python/3.12.13/x64
   PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib/pkgconfig
   Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
   Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
   Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
   LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib
 ##[endgroup]
 RUF100 [*] Unused `noqa` directive (non-enabled: `E402`)
   --> .github/codeowners/build_codeowners.py:30:68
    |
 29 | sys.path.insert(0, str(Path(__file__).parent))
 30 | from codeowners_match import compute_resolution, load_tree, match  # noqa: E402
    |                                                                    ^^^^^^^^^^^^
    |
 help: Remove unused `noqa` directive
 UP035 [*] Import from `collections.abc` instead: `Iterable`
   --> .github/codeowners/codeowners_match.py:45:1
    |
 43 | from dataclasses import dataclass, field
 44 | from pathlib import Path
 45 | from typing import Iterable, TypedDict
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 46 |
 47 | # ----------------------------------------------------------------------
    |
 help: Import from `collections.abc`
 UP033 [*] Use `@functools.cache` instead of `@functools.lru_cache(maxsize=None)`
    --> .github/codeowners/codeowners_match.py:186:21
     |
 186 | `@functools.lru_cache`(maxsize=None)
     |                     ^^^^^^^^^^^^^^
 187 | def _compiled(pattern: str) -> re.Pattern[str]:
 188 |     return re.compile(_glob_to_re(pattern))
     |
 help: Rewrite with `@functools.cache
 RUF100 [*] Unused `noqa` directive (non-enabled: `E402`)
   --> .github/codeowners/emit_codeowners.py:43:33
    |
 42 | sys.path.insert(0, str(Path(__file__).parent))
 43 | from codeowners_match import (  # noqa: E402
    |                                 ^^^^^^^^^^^^
 44 |     ResolvedModel,
 45 |     anchor,
    |
 help: Remove unused `...
🧰 Additional context used
📓 Path-based instructions (3)
**/*.md

📄 CodeRabbit inference engine (.claude/rules/generator/config_schema.md)

**/*.md: Add computation rules in the rules authoring system when a new parameter requires non-trivial value computation beyond simple defaults
Add template modifications in the template authoring system when a new parameter requires special handling in generated artifacts

Files:

  • CONTRIBUTORS.md
**/*

⚙️ CodeRabbit configuration file

**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.

  • Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
  • If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.

Files:

  • CONTRIBUTORS.md
.github/workflows/**

⚙️ CodeRabbit configuration file

.github/workflows/**: - Review workflow edits for least-privilege permissions, safe trigger scopes, secret exposure, fork behavior, and release/cherry-pick side effects.

  • Verify matrix jobs, artifact uploads, and scheduled support-matrix workflows still produce debuggable evidence.

Files:

  • .github/workflows/codeowners.yml
🪛 ast-grep (0.44.1)
.github/codeowners/min_approvers.py

[error] 60-71: Command coming from incoming request
Context: subprocess.check_output(
[
"gh",
"api",
f"orgs/{org}/teams/{slug}/members",
"--paginate",
"--jq",
".[].login",
],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)


[error] 60-71: Avoid command injection
Context: subprocess.check_output(
[
"gh",
"api",
f"orgs/{org}/teams/{slug}/members",
"--paginate",
"--jq",
".[].login",
],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)

.github/codeowners/who_owns.py

[error] 75-86: Command coming from incoming request
Context: subprocess.check_output(
[
"gh",
"api",
f"orgs/{org}/teams/{slug}/members",
"--paginate",
"--jq",
".[].login",
],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)


[error] 163-167: Command coming from incoming request
Context: subprocess.check_output(
["git", "-C", repo, "diff", "--name-only", *args],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)


[error] 177-181: Command coming from incoming request
Context: subprocess.check_output(
["git", "-C", repo, "ls-files", "--others", "--exclude-standard"],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)


[error] 75-86: Avoid command injection
Context: subprocess.check_output(
[
"gh",
"api",
f"orgs/{org}/teams/{slug}/members",
"--paginate",
"--jq",
".[].login",
],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)


[error] 163-167: Avoid command injection
Context: subprocess.check_output(
["git", "-C", repo, "diff", "--name-only", *args],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)


[error] 177-181: Avoid command injection
Context: subprocess.check_output(
["git", "-C", repo, "ls-files", "--others", "--exclude-standard"],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)

.github/codeowners/codeowners_match.py

[error] 247-247: Avoid command injection
Context: subprocess.check_output(["git", "-C", str(repo), "ls-files"], text=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)


[error] 247-247: Command coming from incoming request
Context: subprocess.check_output(["git", "-C", str(repo), "ls-files"], text=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)


[warning] 187-187: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(_glob_to_re(pattern))
Note: [CWE-1333] Inefficient Regular Expression Complexity.

(redos-non-literal-regex-python)

.github/codeowners/test_codeowners.py

[error] 457-459: Command coming from incoming request
Context: subprocess.check_output(
["git", "-C", str(repo), *args], stderr=subprocess.DEVNULL
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)


[error] 457-459: Avoid command injection
Context: subprocess.check_output(
["git", "-C", str(repo), *args], stderr=subprocess.DEVNULL
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)

🪛 GitHub Actions: Lint and Format / 0_Lint and Format (Ruff).txt
.github/codeowners/build_codeowners.py

[error] 30-30: ruff (RUF100): Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

.github/codeowners/min_approvers.py

[error] 41-41: ruff (RUF100): Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

.github/codeowners/emit_codeowners.py

[error] 43-43: ruff (RUF100): Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

.github/codeowners/who_owns.py

[error] 36-36: ruff (RUF100): Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

.github/codeowners/codeowners_match.py

[error] 45-45: ruff (UP035): Import from collections.abc instead: Iterable. Import Iterable from collections.abc.


[error] 186-186: ruff (UP033): Use @functools.cache instead of @functools.lru_cache(maxsize=None). Rewrite with @functools.cache.

.github/codeowners/test_codeowners.py

[error] 27-27: ruff (RUF100): Unused noqa directive (non-enabled: E402). Remove unused noqa directive.


[error] 28-28: ruff (RUF100): Unused noqa directive (non-enabled: E402). Remove unused noqa directive.


[error] 38-38: ruff (RUF100): Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

🪛 GitHub Actions: Lint and Format / Lint and Format (Ruff)
.github/codeowners/build_codeowners.py

[error] 30-30: RUF100: Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

.github/codeowners/min_approvers.py

[error] 41-41: RUF100: Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

.github/codeowners/emit_codeowners.py

[error] 43-43: RUF100: Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

.github/codeowners/who_owns.py

[error] 36-36: RUF100: Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

.github/codeowners/codeowners_match.py

[error] 45-45: UP035: Import from collections.abc instead: Iterable.


[error] 186-186: UP033: Use @functools.cache instead of @functools.lru_cache(maxsize=None).

.github/codeowners/test_codeowners.py

[error] 27-27: RUF100: Unused noqa directive (non-enabled: E402). Remove unused noqa directive.


[error] 28-30: RUF100: Unused noqa directive (non-enabled: E402). Remove unused noqa directive.


[error] 38-38: RUF100: Unused noqa directive (non-enabled: E402). Remove unused noqa directive.

🪛 LanguageTool
CONTRIBUTORS.md

[uncategorized] ~9-~9: The official name of this software platform is spelled with a capital “H”.
Context: ... — update that file and regenerate (see .github/codeowners/README.md). _No external c...

(GITHUB)

🔇 Additional comments (12)
.github/codeowners/test_codeowners.py (1)

450-472: LGTM!

.github/codeowners/areas.yaml (1)

1-81: LGTM! (aside from the shared-block note above)

.github/codeowners/codeowners_match.py (1)

1-540: LGTM otherwise — the matcher/resolution logic checked out against the cited test cases (anchored dir matching, coverage anchoring, filetype coverage gating).

.github/codeowners/README.md (1)

1-99: LGTM!

.github/codeowners/emit_codeowners.py (1)

1-488: LGTM otherwise — external-contributor decoration, advisory rendering, and CLI wiring all checked out against the referenced test behaviors.

.github/codeowners/external_contributors.yaml (1)

1-45: LGTM!

CONTRIBUTORS.md (1)

1-13: LGTM!

.github/codeowners/build_codeowners.py (1)

1-96: LGTM otherwise.

.github/workflows/codeowners.yml (2)

1-33: LGTM! Permissions (contents: read), persist-credentials: false, and the broad pull_request path filter for the coverage gate are all appropriately scoped.

Also applies to: 36-60


61-70: 🗄️ Data Integrity & Integration

No drift gap here. advisory-reviewers.yaml isn’t a tracked output in this repo, so the drift check only needs to cover CODEOWNERS and CONTRIBUTORS.md.

			> Likely an incorrect or invalid review comment.
.github/codeowners/who_owns.py (1)

1-34: LGTM!

Also applies to: 44-145, 195-282

.github/codeowners/min_approvers.py (1)

1-40: LGTM!

Also applies to: 79-92, 95-129, 141-154

Comment thread .github/codeowners/build_codeowners.py Outdated
Comment thread .github/codeowners/codeowners_match.py Outdated
Comment thread .github/codeowners/codeowners_match.py Outdated
Comment thread .github/codeowners/emit_codeowners.py Outdated
Comment thread .github/codeowners/min_approvers.py Outdated
Comment thread .github/codeowners/min_approvers.py Outdated
Comment thread .github/codeowners/test_codeowners.py Outdated
Comment thread .github/codeowners/test_codeowners.py Outdated
Comment thread .github/codeowners/who_owns.py Outdated
Comment thread .github/codeowners/who_owns.py Outdated
Preserve AIC's existing maintainer co-ownership and deployment-guide routing while closing previously unowned Rust and agent/config paths. Harden the generated policy against shadow files, deleted outputs, invalid ownership data, incomplete rosters, matcher edge cases, and stale workflow consumers. Add AIC policy and fail-closed regression coverage, pin the new workflow dependencies, and repair the random reviewer workflow for the root CODEOWNERS path.

Signed-off-by: Daniel Gil <dagil@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.github/codeowners/who_owns.py (1)

158-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Capture stderr for actionable failure diagnostics.

All git subprocess calls in changed_files discard stderr (DEVNULL), so when both diff attempts fail, the raised SystemExit only shows the generic CalledProcessError string — the actual git error (e.g. "unknown revision") is lost, making CI failures harder to debug.

♻️ Proposed fix to surface git's error text
             out = subprocess.check_output(
                 ["git", "-C", repo, "diff", "--name-only", *args],
                 text=True,
-                stderr=subprocess.DEVNULL,
+                stderr=subprocess.PIPE,
             )
         except (OSError, subprocess.CalledProcessError) as err:
             last_err = err
             continue
     ...
     if not diff_ok:
-        raise SystemExit(f"git diff failed in {repo!r} (not a checkout, or base {base!r} unavailable): {last_err}")
+        detail = getattr(last_err, "stderr", None) or last_err
+        raise SystemExit(f"git diff failed in {repo!r} (not a checkout, or base {base!r} unavailable): {detail}")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/codeowners/who_owns.py around lines 158 - 196, Update the git
subprocess calls in changed_files to capture stderr instead of discarding it,
ensuring CalledProcessError retains git’s diagnostic output. Preserve the
existing fallback behavior and include the captured error text in the SystemExit
raised when both diff attempts fail; apply the same stderr handling consistently
to the related ls-files and working-tree diff calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/codeowners/who_owns.py:
- Around line 158-196: Update the git subprocess calls in changed_files to
capture stderr instead of discarding it, ensuring CalledProcessError retains
git’s diagnostic output. Preserve the existing fallback behavior and include the
captured error text in the SystemExit raised when both diff attempts fail; apply
the same stderr handling consistently to the related ls-files and working-tree
diff calls.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3e9a0150-b49e-481c-87e9-cc1c31d2b71f

📥 Commits

Reviewing files that changed from the base of the PR and between 0db218b and fbf3713.

⛔ Files ignored due to path filters (1)
  • CODEOWNERS is excluded by none and included by none
📒 Files selected for processing (13)
  • .github/codeowners/README.md
  • .github/codeowners/areas.yaml
  • .github/codeowners/build_codeowners.py
  • .github/codeowners/codeowners_match.py
  • .github/codeowners/emit_codeowners.py
  • .github/codeowners/external_contributors.yaml
  • .github/codeowners/min_approvers.py
  • .github/codeowners/test_aic_policy.py
  • .github/codeowners/test_codeowners.py
  • .github/codeowners/test_codeowners_validation.py
  • .github/codeowners/who_owns.py
  • .github/workflows/codeowners.yml
  • .github/workflows/random-review-assignment.yml
🚧 Files skipped from review as they are similar to previous changes (7)
  • .github/codeowners/external_contributors.yaml
  • .github/codeowners/README.md
  • .github/codeowners/areas.yaml
  • .github/workflows/codeowners.yml
  • .github/codeowners/build_codeowners.py
  • .github/codeowners/min_approvers.py
  • .github/codeowners/emit_codeowners.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
  • GitHub Check: CodeRabbit
  • GitHub Check: Collect snapshot (new)
  • GitHub Check: Collect snapshot (old)
  • GitHub Check: Build and Test (unit)
  • GitHub Check: Rust/Python engine-step parity
  • GitHub Check: Build and Test (e2e)
  • GitHub Check: Cargo Deny
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**

⚙️ CodeRabbit configuration file

.github/workflows/**: - Review workflow edits for least-privilege permissions, safe trigger scopes, secret exposure, fork behavior, and release/cherry-pick side effects.

  • Verify matrix jobs, artifact uploads, and scheduled support-matrix workflows still produce debuggable evidence.

Files:

  • .github/workflows/random-review-assignment.yml
🪛 ast-grep (0.44.1)
.github/codeowners/test_aic_policy.py

[error] 29-29: Avoid command injection
Context: subprocess.check_output(["git", "-C", str(ROOT), "ls-files"], text=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)


[error] 29-29: Command coming from incoming request
Context: subprocess.check_output(["git", "-C", str(ROOT), "ls-files"], text=True)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

.github/codeowners/codeowners_match.py

[warning] 265-265: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(rf"^@{_GITHUB_LOGIN}(?:/{_GITHUB_TEAM_SLUG})?$")
Note: [CWE-1333] Inefficient Regular Expression Complexity.

(redos-non-literal-regex-python)

.github/codeowners/test_codeowners.py

[error] 457-457: Avoid command injection
Context: subprocess.check_output(["git", "-C", str(repo), *args], stderr=subprocess.DEVNULL)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)


[error] 457-457: Command coming from incoming request
Context: subprocess.check_output(["git", "-C", str(repo), *args], stderr=subprocess.DEVNULL)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)


[error] 476-476: Avoid command injection
Context: subprocess.check_output(["git", "-C", str(repo), *args], stderr=subprocess.DEVNULL)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)


[error] 495-495: Avoid command injection
Context: subprocess.check_output(["git", "-C", str(repo), *args], stderr=subprocess.DEVNULL)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)


[error] 476-476: Command coming from incoming request
Context: subprocess.check_output(["git", "-C", str(repo), *args], stderr=subprocess.DEVNULL)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)


[error] 495-495: Command coming from incoming request
Context: subprocess.check_output(["git", "-C", str(repo), *args], stderr=subprocess.DEVNULL)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

.github/codeowners/who_owns.py

[error] 162-166: Avoid command injection
Context: subprocess.check_output(
["git", "-C", repo, "diff", "--name-only", *args],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)


[error] 162-166: Command coming from incoming request
Context: subprocess.check_output(
["git", "-C", repo, "diff", "--name-only", *args],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)


[error] 187-191: Avoid command injection
Context: subprocess.check_output(
["git", "-C", repo, "diff", "--name-only"],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(command-injection-python)


[error] 187-191: Command coming from incoming request
Context: subprocess.check_output(
["git", "-C", repo, "diff", "--name-only"],
text=True,
stderr=subprocess.DEVNULL,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

🔇 Additional comments (7)
.github/codeowners/test_codeowners_validation.py (1)

1-129: LGTM!

.github/codeowners/test_aic_policy.py (1)

1-68: LGTM!

.github/workflows/random-review-assignment.yml (2)

38-38: LGTM!

Also applies to: 91-91


17-22: 🔒 Security & Privacy

No change needed for CODEOWNERS checkout — this workflow uses pull_request_target, so actions/checkout without ref reads from the base branch by default, not the PR head.

			> Likely an incorrect or invalid review comment.
.github/codeowners/codeowners_match.py (1)

43-46: LGTM!

Also applies to: 149-193, 259-352, 408-467, 518-561

.github/codeowners/test_codeowners.py (1)

27-28: LGTM!

Also applies to: 38-38, 89-95, 372-401, 447-509, 580-591, 620-621

.github/codeowners/who_owns.py (1)

36-43: LGTM!

Also applies to: 56-67, 121-128, 199-227

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/codeowners/who_owns.py (1)

116-125: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Document the unstaged working-tree diff in changed_files()
git diff --name-only here also adds unstaged local edits on top of the base diff and untracked files, so --changed can report paths unrelated to base. Either call that out in the docstring or make it an explicit opt-in.

💬 Suggested change
     Untracked (not yet staged) files are included too -- brand-new paths are
     exactly the ones the coverage gate cares about, and ``git diff`` alone
     never shows them.
+
+    Unstaged working-tree edits are also included, so local modifications not
+    yet added to the index still show up.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/codeowners/who_owns.py around lines 116 - 125, Update the
changed_files() docstring to explicitly state that its git diff behavior
includes unstaged working-tree edits in addition to the base diff and untracked
files, so callers understand that --changed may report paths unrelated to base.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In @.github/codeowners/who_owns.py:
- Around line 116-125: Update the changed_files() docstring to explicitly state
that its git diff behavior includes unstaged working-tree edits in addition to
the base diff and untracked files, so callers understand that --changed may
report paths unrelated to base.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 746872fb-c1cf-4a2c-8880-338c244eedf1

📥 Commits

Reviewing files that changed from the base of the PR and between fbf3713 and 7b83480.

📒 Files selected for processing (7)
  • .github/codeowners/README.md
  • .github/codeowners/codeowners_match.py
  • .github/codeowners/emit_codeowners.py
  • .github/codeowners/test_codeowners.py
  • .github/codeowners/test_codeowners_validation.py
  • .github/codeowners/who_owns.py
  • .github/workflows/codeowners.yml
🚧 Files skipped from review as they are similar to previous changes (4)
  • .github/codeowners/README.md
  • .github/workflows/codeowners.yml
  • .github/codeowners/test_codeowners.py
  • .github/codeowners/codeowners_match.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
  • GitHub Check: CodeRabbit
  • GitHub Check: Collect snapshot (old)
  • GitHub Check: Collect snapshot (new)
  • GitHub Check: Build and Test (e2e)
  • GitHub Check: Cargo Deny
  • GitHub Check: Build and Test (unit)
  • GitHub Check: Rust/Python engine-step parity
🧰 Additional context used
🪛 ast-grep (0.44.1)
.github/codeowners/test_codeowners_validation.py

[error] 132-137: Command coming from incoming request
Context: subprocess.run(
[sys.executable, str(Path(file).with_name(script)), "--help"],
check=False,
capture_output=True,
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

🔇 Additional comments (4)
.github/codeowners/test_codeowners_validation.py (1)

1-13: LGTM!

Also applies to: 16-47, 50-114, 115-126, 127-141, 142-154

.github/codeowners/emit_codeowners.py (2)

3-3: LGTM!


261-263: 🎯 Functional Correctness

No issue here. .github/codeowners/areas.yaml gives each area a unique github_team, so team_to_label doesn’t collapse any entries and the section headers stay aligned.

			> Likely an incorrect or invalid review comment.
.github/codeowners/who_owns.py (1)

4-4: LGTM!

Also applies to: 21-21, 36-36, 89-113, 167-227

Signed-off-by: Daniel Gil <dagil@nvidia.com>
@dagil-nvidia
dagil-nvidia force-pushed the dagil-nvidia/codeowners-as-code branch from 7b83480 to 337eec5 Compare July 12, 2026 22:23
…WNERS (#1348)

* docs: aic-codeowners skill + contributor guidance for generated CODEOWNERS

Companion to the CODEOWNERS-as-code PR: an .agents/skills/aic-codeowners
skill covering the four flows (who reviews a change, fixing a failing
codeowners check, changing routing, external contributor grants), plus
CONTRIBUTING.md and AGENTS.md pointers so contributors and agents find
the playbook when the gate fires.

Signed-off-by: Dan Gil <dagil@nvidia.com>

* docs(codeowners): make AIC gate guidance fail closed

Triage the codeowners workflow by its failing step instead of assuming every failure is an uncovered path. Document sampled coverage output, multi-claim fixes, generated advisory artifacts, and repository-rule enforcement so agents preserve routing and commit every changed source/output together.

Signed-off-by: Daniel Gil <dagil@nvidia.com>

* docs(codeowners): remove advisory reviewer guidance

Signed-off-by: Daniel Gil <dagil@nvidia.com>

---------

Signed-off-by: Dan Gil <dagil@nvidia.com>
Signed-off-by: Daniel Gil <dagil@nvidia.com>
@jasonqinzhou

Copy link
Copy Markdown
Contributor

The current codeowners failure exposes a structural issue with the generator, rather than an ownership gap. The workflow runs against GitHub’s synthetic merge with current main; unrelated changes on main moved skills between .agents/ and .claude/, so the tree-dependent minimal-cover pass rewrote these rules:

+/.claude/
-/.claude/rules/
-/.agents/skills/
+/.agents/skills/aic-codeowners/SKILL.md

Those differences do not represent a policy change, and the later shared /.claude/ and /.agents/ rules already override them. Rebasing and regenerating will clear this instance, but any subsequent tree change can recreate the same failure for a long-running PR.

My recommendation is to decouple emission from the live repository tree:

  1. Emit stable CODEOWNERS patterns directly from areas.yaml and shared declarations. Remove the tree-dependent minimal-cover/singleton-file optimization; replace keyword auto-classification with explicit stable globs if necessary.
  2. Continue using git ls-files only in build_codeowners.py --strict to validate that every tracked path matches a non-catch-all ownership rule.
  3. Make the drift check a deterministic function of the policy inputs (areas.yaml and external_contributors.yaml), not of whichever merge tree Actions happens to check out.
  4. Add a regression test proving that adding, deleting, or moving files underneath already-owned prefixes does not change generated CODEOWNERS.

That preserves the valuable 100% coverage gate while avoiding unrelated regeneration churn and base-branch races.

Emission was a pure function of the tree, not of the policy YAML: the
minimal-cover, keyword auto-classify, keyword co-ownership, and per-file
enumeration all walked git ls-files at emit time. Any unrelated commit on
main that added, deleted, or moved a file under an already-owned prefix
rewrote lines and broke the codeowners CI check on long-running PRs that
had touched none of that surface -- exactly the drift this PR hit, where a
CI regen added /.claude/ and a /.agents/skills/aic-codeowners/SKILL.md
singleton that were not in the committed CODEOWNERS.

Make emission a pure function of areas.yaml + external_contributors.yaml:

 * compute_resolution(spec, tree=None) -- tree deprecated and ignored;
   drop the tree-walking _auto_classify / _keyword_coownership /
   _filetype_coownership; filetype rules emit as one stable coowner-only
   row matching by basename at any depth.
 * _base_rules(model) renders each area's declared path_globs verbatim
   (anchored); no more minimal cover, no more singleton-file optimization,
   no more tree read.
 * emit_codeowners.py --repo accepted but ignored (backward compat); the
   workflow drops --repo . from the emit/drift step.
 * build_codeowners.py --strict is the ONLY step that still reads
   git ls-files, and only to gate coverage; the emitter can't consume it.

Two trees that differ only under owned prefixes now produce byte-identical
CODEOWNERS. Regenerated the committed CODEOWNERS from the pure emitter;
coverage stays 100% (1508/1508) and all 101 unit + policy tests pass.
TestComputeResolution pins the tree-independence contract in CI.

Signed-off-by: Dan Gil <dagil@nvidia.com>
The port of the emission-decoupling refactor removed the tree walks
from ``compute_resolution`` and ``_render_codeowners`` but did not
carry over the belt-and-braces regression guards. Without them, a
future edit re-adding a ``tree`` parameter to the emitter or a
``load_tree`` call on the emit path would silently regress the
"policy in -> byte-identical CODEOWNERS out" contract and only surface
as a churned CODEOWNERS on some unrelated PR.

Add ``TestEmissionIsTreeIndependent`` with four tests:

* ``test_add_file_under_owned_prefix_does_not_change_output`` -- feed
  the deprecated ``tree`` positional to ``compute_resolution`` with
  and without extra files under owned prefixes; the resolved model
  must be equal.
* ``test_delete_or_move_under_owned_prefix_does_not_change_output`` --
  same, but for a ``deleted_tree`` and a ``moved_tree`` (renames and
  cross-prefix moves under owned directories).
* ``test_emitter_has_no_tree_parameter`` -- ``inspect.signature`` on
  ``_render_codeowners`` and ``compute_resolution`` to catch a
  parameter re-add.
* ``test_no_ls_files_call_at_emit`` -- monkeypatch
  ``codeowners_match.load_tree`` to raise; the full emit path must
  still succeed, proving nothing on it walks the tree.

Spec is adapted to AIC's stricter schema (no ``advisory`` sections,
no ``advisory: True`` on filetype rules; validator would reject).

Byte-identical CODEOWNERS + CONTRIBUTORS.md, strict coverage still
100% (1508/1508), 105 tests pass (was 101, +4).

Signed-off-by: Dan Gil <dagil@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

--strict reads git ls-files against the base+PR merge tree, so a new unowned
top-level directory landing on main fails the coverage step on every open PR,
including long-running ones that touched none of it.

Add an opt-in --changed-only/--base mode: strict then hard-fails only on
unowned paths this PR adds/renames/modifies vs the base
(git diff --diff-filter=ACMR base...HEAD) and reports base-inherited unowned
paths as a non-fatal warning. The PR's own surface still must be 100% owned.
Full-tree 100% coverage stays the default for push-to-main / scheduled runs.

Policy-change safety: a PR that edits ownership policy (areas.yaml, the
codeowners scripts, external_contributors.yaml, or CODEOWNERS) can re-route
ANY path, so it is judged full-tree instead of the changed surface -- else a
policy edit could orphan untouched paths and slip through as a warning.

- codeowners_match.changed_paths(): ACMR diff helper (base...HEAD)
- build_codeowners.split_coverage(): partition blocking vs warning
- build_codeowners.is_policy_change(): policy edit -> full-tree fallback
- workflow: --changed-only --base <base.sha> on pull_request; full-tree
  strict on non-PR events
- tests: base-inherited gap passes, PR-introduced gap fails, full-tree
  still fails on any unowned path, policy edit orphaning an untouched path
  blocks (+ e2e demos)

Signed-off-by: Dan Gil <dagil@nvidia.com>
Resolve the CONFLICTING state so the pull_request workflows (codeowners,
Build and Test, Lint and Format, Copyright Checks, Prediction Regression
Gate) run again. Merge (not rebase), mirroring the dynamo side.

Conflicts / resolutions:
- .github/CODEOWNERS (modify/delete): kept DELETED. main modified the legacy
  hand-written file; this PR replaces it with a generated root CODEOWNERS and
  the workflow rejects .github/CODEOWNERS as a shadow file. On the
  case-insensitive macOS FS the file collided with our .github/codeowners/
  scripts directory; removing the file restored the directory from the index.
- areas.yaml: main's #1322 relocated rust/aiconfigurator-core/ -> aic-core/
  and added the aic-core/ package (1174 files). Repointed the runtime area +
  shared glob rust/ -> aic-core/ (owner @ai-dynamo/aiconfigurator-runtime +
  maintainers, matching main's own CODEOWNERS entry for /aic-core/).
- test_aic_policy.py: updated the representative routing path
  rust/aiconfigurator-core/Cargo.toml -> aic-core/rust/aiconfigurator-core/
  Cargo.toml to track the move (same owners).
- Regenerated CODEOWNERS + CONTRIBUTORS.md from the emitter.

Emission stays tree-independent; the diff-aware --changed-only gate and the
is_policy_change / split_coverage logic are preserved. Full-tree --strict
coverage is 100% (1900/1900); AIC test suite 117 passed; ruff clean.

Signed-off-by: Dan Gil <dagil@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants