fix(codeowners): repoint orphaned planner+operator glob at moved kubernetes_api.py - #12378
Merged
dmitry-tokarev-nv merged 2 commits intoAug 11, 2026
Conversation
…etes_api.py PR ai-dynamo#10767 (e69e5fd) renamed components/src/dynamo/planner/connectors/kubernetes_api.py to components/src/dynamo/planner/connectors/clients/kubernetes_api.py, but the `shared:` co-ownership entry in .github/codeowners/areas.yaml kept pointing at the old path. Because `shared:` entries are additive and glob-matched, an entry naming a path that no longer exists emits a CODEOWNERS rule GitHub can never match, so the operator team silently stopped being auto-requested on that file. Repoint the glob at the live path and regenerate CODEOWNERS with emit_codeowners.py. who_owns.py now resolves the file to both @ai-dynamo/dynamo-planner-codeowners and @ai-dynamo/dynamo-operator-codeowners. Both files land in one commit because the codeowners workflow regenerates and runs `git diff --exit-code CODEOWNERS CONTRIBUTORS.md`; splitting them would red-X the intermediate commit. Refs: ai-dynamo#12324 Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
glamr-agent
temporarily deployed
to
external_collaborator
July 29, 2026 18:54 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
glamr-agent
temporarily deployed
to
external_collaborator
July 29, 2026 18:54 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
github-actions
Bot
added
external-contribution
Contributor
Author
Automated evidenceValidation status: complete Evidence summary: [2/2 validated] AI review assessment: sound — advisory only. This is an automated agent's Validation result: complete — pass. Evidence audit: complete [2/2 validated] — the evidence table is grounded in Evidence [2/2 validated]Generated from validation/registry.jsonl — do not edit by hand.
The full plan, change, validation, and review packets are attached as separate |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
Author
plan.md# Plan — wi-20260729T172118Z-12324
Route: implementation
Template: debug-investigation
Engine: vllm
## Existing review request and resolution status
There is no existing review request to act on. `input.md` records none, and the
all-state provider survey below found no open request that implements this
repoint, so the publisher opens a **new** request on
`fix/orphaned-codeowners-entry--cc21fb8c3b1b` against `main`.
This work item is also not already resolved. No merged PR delivers the requested
behaviour on `main` — the current `main` file still carries the stale glob, which
is the authoritative check — and the one candidate, #11869, fails to qualify on
two independent grounds set out under *Related-work survey*: it is open, and it
deletes the entry rather than repointing it. Implementation proceeds normally.
---
## User intent
GitHub issue [#12324](https://github.com/ai-dynamo/dynamo/issues/12324) reports
that `.github/codeowners/areas.yaml` line 848 carries a `shared:` co-ownership
entry pairing the `planner` and `operator` areas against
`components/src/dynamo/planner/connectors/kubernetes_api.py` — a path that PR
[#10767](https://github.com/ai-dynamo/dynamo/pull/10767) (`e69e5fdf606`) renamed
to `components/src/dynamo/planner/connectors/clients/kubernetes_api.py`. Because
`shared:` is additive and glob-matched, an entry pointing at a path that no
longer exists emits a CODEOWNERS rule that GitHub will never match, so the
`operator` team silently stopped being auto-requested on that file. The caller
wants the glob repointed at the live path and `CODEOWNERS` regenerated, with both
files in a single commit because the `codeowners` CI workflow diffs the committed
artifact against a fresh regeneration. The Slack thread adds nothing beyond the
issue text ("One path edit plus regeneration — both must land together, CI gates
the drift"). The issue also asks to sweep for other `shared:` entries orphaned by
the same refactor; I performed that sweep and its scope decision is recorded
below.
## Non-goals
- **Pruning the other three dead `shared:` globs.** The sweep found three more
(listed under *Discovery*). None of them is a rename, so none of them lost
co-ownership the way this one did: two name files that have never existed on
`main`, and one names a file that was deleted outright with no successor path.
Repointing is impossible for all three; the only available action is deletion,
which is precisely the scope of open PR #11869. Doing it here would duplicate
that PR and enlarge a one-line routing fix into a policy cleanup.
- **Pruning the eight dead single-owner `path_globs`** found by the same sweep.
A dead base glob costs nothing — the area still owns whatever it owns — and
removing one cannot restore a lost co-owner. Same rationale: #11869's territory.
- **Adding a generic "no `shared:` glob may point at a nonexistent path"
guard** to `.github/codeowners/test_codeowners.py`. Such a test would fail
today on the three out-of-scope orphans, so landing it would force the scope
creep the previous two bullets reject. Worth filing separately; not this
work item.
- **Touching `emit_codeowners.py`, `build_codeowners.py`, `codeowners_match.py`,
`who_owns.py`, or `.github/workflows/codeowners.yml`.** The generator behaves
correctly; the input data was stale. No generator change is warranted, and the
86-test suite covering it must stay green untouched.
- **Team membership.** This changes ownership *structure* only, as the workflow
header states. Who is on `@ai-dynamo/dynamo-operator-codeowners` is managed
elsewhere.
- **Renaming or moving any Python source.** The rename already happened in
#10767; only the ownership policy is out of date.
## Discovery
All commands below were run read-only against the checkout at
`/home/sandbox/workspace/wi-20260729T172118Z-12324/repo`. Where a command had to
write (regeneration), I copied `.github/codeowners/` plus `CODEOWNERS` and
`CONTRIBUTORS.md` into `/tmp/co/` and exercised the change there; the source
checkout is untouched.
### The reported defect, confirmed
`.github/codeowners/areas.yaml` line 848 reads exactly as the issue quotes it,
inside the top-level `shared:` block (which begins at line 558):
```yaml
- glob: components/src/dynamo/planner/connectors/kubernetes_api.py
owners:
- planner
- operator
```
On disk, `components/src/dynamo/planner/connectors/` contains `__init__.py`,
`base.py`, `global_planner.py`, `kubernetes.py`, `mdc.py`, `protocol.py`,
`virtual.py` and a `clients/` subdirectory; the actual file lives at
`components/src/dynamo/planner/connectors/clients/kubernetes_api.py`.
`git show --stat e69e5fdf606` confirms the pure rename the issue describes and
shows it moved a sibling too:
```
.../connectors/{ => clients}/kubernetes_api.py | 0
.../connectors/{ => clients}/remote_client.py | 0
```
`remote_client.py` never had a `shared:` entry (grep for `planner/connectors` in
`areas.yaml` returns only lines 207, 844, 848), so #10767 orphaned exactly one
co-ownership declaration. `git log -S 'connectors/kubernetes_api.py' --
.github/codeowners/areas.yaml` attributes the entry to `c49abbce2` (#10715, "add
CODEOWNERS generated as Infrastructure-as-Code", merged 2026-07-12), and
`git ls-tree -r c49abbce2` confirms the file was still at the old path then — so
the entry was correct when written and was invalidated five days later by #10767
(merged 2026-07-17).
### Current routing, observed
```
$ python3 .github/codeowners/who_owns.py --codeowners CODEOWNERS \
components/src/dynamo/planner/connectors/clients/kubernetes_api.py \
components/src/dynamo/planner/connectors/kubernetes.py
components/src/dynamo/planner/connectors/clients/kubernetes_api.py
review: @ai-dynamo/dynamo-planner-codeowners
components/src/dynamo/planner/connectors/kubernetes.py
review: @ai-dynamo/dynamo-planner-codeowners @ai-dynamo/dynamo-operator-codeowners
```
The live path resolves to `planner` alone, exactly as the issue reports, while
its sibling `kubernetes.py` — whose `shared:` entry at line 844 still points at a
live path — correctly resolves to both teams. `CODEOWNERS` line 702 still carries
the rule against the dead path.
### Generator and gate, read
`.github/codeowners/emit_codeowners.py` is a pure function of `areas.yaml` +
`external_contributors.yaml`; its module docstring states the repository tree is
*never* read at emit time (this was deliberate — #11614, "decouple CODEOWNERS
emission from the live tree", merged 2026-07-15). Its `main()` writes `--out`
(CODEOWNERS), `--contributors-out` (CONTRIBUTORS.md), and an advisory file only
if advisory rules exist; `areas.yaml` declares `advisory: false` on its single
file-type rule, and `.github/codeowners/advisory-reviewers.yaml` does not exist,
so no advisory artifact is produced or removed by regeneration.
`.github/workflows/codeowners.yml` is the gate. It runs on every PR and has three
relevant steps:
1. **Unit tests** — `pytest .github/codeowners/test_codeowners.py -v
-p no:cacheprovider --override-ini="addopts=" --override-ini="filterwarnings="`.
2. **Coverage gate** — `build_codeowners.py --areas ... --repo . --strict
--changed-only --base <base.sha>`. Note `is_policy_change()`: a PR that
touches `areas.yaml`, the codeowners scripts, or `CODEOWNERS` is judged
**full-tree**, not diff-only. This PR is a policy change, so the full-tree
strict gate applies to it.
3. **Regenerate and check for drift** — runs `emit_codeowners.py` then
`git diff --exit-code CODEOWNERS CONTRIBUTORS.md`. This is the gate the Slack
message refers to and the reason both files must land in one commit.
There is **no** codeowners hook in `.pre-commit-config.yaml`, so nothing
regenerates `CODEOWNERS` locally; the printer must run the emitter by hand.
`build_codeowners.py` also prints a non-blocking "globs matching no files" report
(`dead = [g for g in model.owned_patterns() if not any(match(g, p) for p in tree)]`)
with the comment *"Deletions never fail a gate ... so stale claims would otherwise
accumulate silently in areas.yaml. Surface them; never block on them."* That
report is the sweep instrument the issue's "silent by construction" remark points
at, and it is why this defect survived CI.
### Existing test coverage — verdict per the discovery-survey learning
`.github/codeowners/test_codeowners.py` holds 86 tests, all passing today
(`86 passed in 0.77s` under the CI invocation). The relevant classes are
`TestResolveOwners` (last-match-wins, `test_multi_owner_passthrough`),
`TestComputeResolution` (`test_shared_multi_owner_recorded`,
`test_explicit_shared_entry_still_wins`), `TestEmissionIsTreeIndependent`, and
`TestSplitCoverage`/`TestIsPolicyChange`. Verdict: **RUN, do not duplicate.** The
matcher, the emitter's tree-independence contract, and the shared-entry
resolution path are already covered against synthetic fixtures. What no test
covers — and what this work item changes — is a *data* value in the real
`areas.yaml`. Per `learnings/no-tautological-tests.md`, a new test asserting "the
glob is `.../clients/kubernetes_api.py` and its owners are planner+operator"
would be a config mirror: it re-states the diff and passes forever. The
defensible proof here is a recorded before/after behavioural observation through
`who_owns.py`, described under *Validation strategy*. The printer must **not**
add a mirror test to `test_codeowners.py`.
### Orphan sweep — the full result
I parsed `areas.yaml` with PyYAML and matched every glob against `git ls-files`
using the emitter's own anchoring semantics, then cross-checked against
`build_codeowners.py --strict`'s dead-glob report (which independently reports
12). Of 128 `shared:` entries, 485 area `path_globs`, and 0 `advisory:` entries:
**Dead `shared:` entries — 4:**
| Glob | Owners | Cause | In scope? |
|---|---|---|---|
| `components/src/dynamo/planner/connectors/kubernetes_api.py` | planner, operator | **Renamed** to `.../connectors/clients/kubernetes_api.py` by `e69e5fdf606` (#10767, on `main`) | **Yes — this is the fix** |
| `lib/llm/src/http/service/clear_kv_blocks.rs` | frontend, kv-memory | **Deleted** (238 lines, no rename pair) by `07d9806ac` (#10556, on `main`); `git ls-files \| grep clear_kv` returns nothing | No |
| `components/src/dynamo/trtllm/tests/test_trtllm_gms_mpi_bootstrap.py` | backend-trtllm, gms | **Never on `main`.** Introduced by `d2748fdf2`, which `git merge-base --is-ancestor` proves is *not* an ancestor of HEAD; also absent at `c49abbce2` when the entry was written | No |
| `components/src/dynamo/trtllm/tests/test_trtllm_gms_delay_commit.py` | backend-trtllm, gms | Same as above (`69ddf9375`, not an ancestor of HEAD) | No |
**Dead single-owner `path_globs` — 8:** `lib/bindings/python/tests/test_parsers.py`
(frontend), `lib/renderer/` (multimodal), `deploy/operator/internal/featuregate/`
(operator), and the five `ATTRIBUTIONS*.md` entries (process) — the last of which
are `.gitignore`d generated artifacts, so they will never appear in `git ls-files`
by design.
**Scope decision and justification.** Only the planner entry matches the failure
mode the issue names: a *rename* that left a live file silently short one
reviewer team. It is the only one of the twelve with a successor path, hence the
only one that can be *repointed* at all. The `clear_kv_blocks.rs` entry is a
tombstone — the code is gone, there is nothing to co-own, and the issue itself
already concedes this class is "correct on its own terms" as a #11869 deletion.
The two `test_trtllm_gms_*` entries are forward-looking: deleting them would be
actively wrong, because the branch that adds those files would then land without
GMS co-ownership — reintroducing the same silent gap in mirror image. The eight
base globs are single-owner, so no co-ownership can have been lost, and removing
them changes no routing decision. Fixing any of the eleven is therefore either
harmful, inert, or a duplicate of an open PR. **The sweep is reported, not
actioned**, and this plan records the eleven so the finding is not lost.
### Related-work survey
Performed with `gh` against `ai-dynamo/dynamo`; network access worked and no
query was blocked.
- **#11869** — `fix(codeowners): harden policy gate and document contributor
workflow`, `"state":"OPEN"`, `"mergedAt":null`, `"closedAt":null`, head
`codex/codeowners-routing-operator-ops`, base `main`. `gh pr diff 11869` shows
its treatment of our glob is a four-line **deletion**:
```
-- glob: components/src/dynamo/planner/connectors/kubernetes_api.py
-- owners:
-- - planner
-- - operator
```
plus the matching `CODEOWNERS` line removal. This is **not**
`already-resolved` on two independent grounds. First, the PR is open, and the
brief is explicit that an open PR never earns that disposition. Second, even
if it merged tomorrow, deleting the tombstone leaves
`.../clients/kubernetes_api.py` owned by `planner` alone — the co-ownership
still would not apply. Deletion and repointing are different outcomes, and the
issue's "Why this is separate from #11869" section says exactly that.
- **#10767** — `refactor(planner): Clean up planner core, abstract out peripheral
logic`, `"state":"MERGED"`, `"mergedAt":"2026-07-17T11:15:00Z"`. Verified on
`main` via `git show --stat e69e5fdf606`; the rename is present and not
reverted (the file is at the new path in the working tree).
- **All-state PR searches.** `gh pr list --state all --search "codeowners in:title"`
(30 results) and `--search "areas.yaml"` (30 results) were both reviewed.
Merged codeowners PRs on `main` are #10715, #11603, #11614, #11660, #12101,
and older pre-IaC ones; none touches this glob (`git log` on
`.github/codeowners/areas.yaml` shows the five most recent commits as
`0f68534a1`, `be642b296`, `6e24e02c3`, `cbd4b9e5f`, `d0f319f15`, and the
current file still carries the stale line — the authoritative check). Open
codeowners PRs are #11869 and **#12361** (`co-own the docs publish workflow and
link-checker config`, `areas.yaml` +8/−0 and `CODEOWNERS` +2/−0) — additive,
in a different region of the file, no conflict.
- **Semantic searches.** `--search "12324"` returns no PR. `--search
"kubernetes_api"` returns only #9683 (`feat(planner): power infrastructure`,
OPEN), which touches planner Kubernetes code, not codeowners policy.
- **Issue state.** `gh issue view 12324` → `"state":"OPEN"`, no labels, no
cross-referencing PR.
**Survey conclusion:** no merged PR delivers the repoint on `main`, and the
current `main` file content confirms it directly. Proceed with implementation.
### Dry run of the proposed change (in `/tmp/co`, source checkout untouched)
Baseline first: regenerating from the **unmodified** `areas.yaml` reproduces the
committed `CODEOWNERS` and `CONTRIBUTORS.md` byte-for-byte (`diff -u` empty,
`rules: 615 (base 485 | shared 128 | file-type 1)`), so the tree is drift-free
today and any diff after the edit is attributable to the edit alone.
After replacing the single glob line and rerunning the emitter, the emitted
`CODEOWNERS` diff is exactly one rule moved:
```diff
-/components/src/dynamo/planner/connectors/kubernetes_api.py @ai-dynamo/dynamo-planner-codeowners @ai-dynamo/dynamo-operator-codeowners
+/components/src/dynamo/planner/connectors/clients/kubernetes_api.py @ai-dynamo/dynamo-planner-codeowners @ai-dynamo/dynamo-operator-codeowners
```
with the removal at line 702 and the insertion ~9 lines later. That relocation is
expected, not a bug: the emitter orders shared rules by pattern length, and the
pattern grows from 59 to 66 characters. The printer should not "fix" it.
`CONTRIBUTORS.md` is unchanged (`diff -q` clean) and no advisory file appears.
`who_owns.py` against the regenerated file then returns both teams. Full-tree
`build_codeowners.py --strict` exits 0 and the dead-glob count falls from 12 to
11. These commands are what the validator will record; the numbers above are the
planner's read-only rehearsal, not the evidence of record.
## Symptom and hypothesis table
**Reproducible symptom.** With the repository at `main`,
`python3 .github/codeowners/who_owns.py --codeowners CODEOWNERS
components/src/dynamo/planner/connectors/clients/kubernetes_api.py` prints only
`@ai-dynamo/dynamo-planner-codeowners`. Expected: that team **and**
`@ai-dynamo/dynamo-operator-codeowners`.
| # | Hypothesis | Distinguishing observation | Outcome |
|---|---|---|---|
| H1 | The `shared:` glob names a path that no longer exists, so the emitted rule can never match the live file | Compare the `areas.yaml` glob against `git ls-files`; compare with sibling `kubernetes.py`, whose glob is live | **Confirmed.** Glob absent from the tree; sibling with a live glob resolves to both teams |
| H2 | The emitter filters or mangles `shared:` rules whose path is missing from the tree | Inspect the committed `CODEOWNERS` for a rule against the dead path | **Refuted.** `CODEOWNERS:702` carries the dead path with both owners, and the emitter docstring/#11614 make emission explicitly tree-independent. The generator is faithful; its input is stale |
| H3 | `who_owns.py` / `codeowners_match` resolves multi-owner rules incorrectly | Resolve a *live* multi-owner path | **Refuted.** `connectors/kubernetes.py` resolves to both teams; `TestResolveOwners::test_multi_owner_passthrough` covers it |
| H4 | A CI gate should already have caught this | Read `build_codeowners.py` coverage logic | **Refuted, and explains the silence.** The file is explicitly owned by `planner`, so it is neither uncovered nor catch-all-only; dead globs are reported but "never blocking" by design |
The smallest observation that separates H1 from H2/H3 is a single `who_owns.py`
invocation over the pair `{clients/kubernetes_api.py, kubernetes.py}` — one dead
glob, one live, same owner pair, same resolver. That pair is the backbone of the
validation below.
## Chosen approach
Two files, one commit.
1. **`.github/codeowners/areas.yaml`** — edit line 848 in place, changing only
the glob value:
```yaml
- glob: components/src/dynamo/planner/connectors/clients/kubernetes_api.py
owners:
- planner
- operator
```
Leave the `owners:` list, the entry's position in the `shared:` block, and
every other line untouched. Do not resort the block: the in-place edit is what
I rehearsed, and it regenerates cleanly.
2. **`CODEOWNERS`** — regenerate, never hand-edit (its own header says so), with
the same command the CI drift step uses, run from the repo root:
```bash
python3 .github/codeowners/emit_codeowners.py \
--areas .github/codeowners/areas.yaml \
--out CODEOWNERS \
--external .github/codeowners/external_contributors.yaml \
--contributors-out CONTRIBUTORS.md
```
Expect exactly one rule to move as shown above, `CONTRIBUTORS.md` to come back
unchanged, and no advisory file to appear. If `CONTRIBUTORS.md` does change,
stop — something other than this edit is in the working tree.
Both files go in one `git commit -s`; the drift gate compares committed artifact
against fresh regeneration, so splitting them red-Xes the intermediate commit.
The conventional-commit title should be a `fix(codeowners):` subject naming the
repoint, and the PR body must carry `Summary` and `Validation` sections plus the
issue URL, per `AGENTS.md`.
No new test file. As argued under *Discovery*, the only test this change could
carry is a config mirror, which `learnings/no-tautological-tests.md` forbids and
the reviewer is instructed to reject. `change.md` must state that explicitly and
point at the recorded before/after evidence instead.
Expected diff size: one line in `areas.yaml`, two lines in `CODEOWNERS` (a
deletion and an insertion at a different offset). Anything larger is a signal
that the emitter picked up unrelated working-tree state.
One coordination note for the PR body: #11869 deletes the same four lines this
change rewrites. Whichever lands second will hit a textual conflict in
`areas.yaml`. The correct resolution in either order is **the repointed glob**,
since deleting it re-opens the exact gap this issue reports. Say so in the PR
description so the #11869 author does not resolve the conflict by taking their
deletion.
## Rejected alternatives
- **Delete the entry (match #11869).** Removes the tombstone but leaves the live
file owned by `planner` alone. That is the bug, not the fix.
- **Add a second entry for the new path and keep the old one.** The old glob is
dead weight that `build_codeowners.py` will keep reporting, and it invites the
next reader to assume both paths are live. Repointing is strictly better.
- **Broaden the glob to `components/src/dynamo/planner/connectors/clients/` (the
whole directory).** This would silently give `operator` co-ownership of
`remote_client.py` too — a routing change nobody asked for, on a file that
#10767 moved alongside but that never had a shared entry. Rejected as scope
creep disguised as robustness. The plan's negative control asserts
`remote_client.py` stays `planner`-only precisely to catch this if the printer
drifts toward it.
- **Fix all twelve dead globs in one PR.** Rejected per the sweep justification:
eleven of them are inert, harmful to delete, or already handled by an open PR,
and bundling them would collide with #11869 across dozens of lines instead of
four.
- **Add a CI gate that fails on any dead `shared:` glob.** Attractive — it would
make this class of failure loud instead of silent — but it fails today on three
pre-existing orphans, so it cannot land without the cleanup this plan declares
a non-goal. It also contradicts a deliberate design decision documented in
`build_codeowners.py` ("never blocking"), which deserves its own discussion
rather than a drive-by change. Better as a follow-up issue.
- **Hand-edit `CODEOWNERS` alongside `areas.yaml`.** Would probably produce the
right two lines but places the rule at the wrong sort position, failing the
drift gate. Regeneration is not optional.
## Validation strategy
The claim to defend is narrow and fully deterministic: *after the change,
`components/src/dynamo/planner/connectors/clients/kubernetes_api.py` routes to
both the planner and operator teams, the generated artifact matches its source,
and nothing else in the ownership map moved.* No GPU, no engine, no Kubernetes,
no Docker is involved — this is metadata. Nominating `07-agg-smoke`,
`08-disagg-pair-smoke`, `09-gpu-pytest`, `11-go-operator-tests`, or
`10-perf-benchmark` would burn the A100 to prove nothing about a YAML glob, and
every nominated recipe must finish green or be dispositioned, so padding the
ladder only creates ways to fail. `05-code-inspection` is also rejected: its
Sections 1 and 4 require an existing PR number and its CI status, which does not
exist at validation time (the publisher opens the request afterwards).
Recipe `00-dynamo-editable-install` is **not** nominated. The compute environment
notes it is needed when a selected recipe imports `dynamo.*`;
`.github/codeowners/test_codeowners.py` imports only `codeowners_match`,
`build_codeowners`, `yaml`, and stdlib, and I confirmed it runs green under the
CI invocation without it. If pytest collection unexpectedly reaches `dynamo.*`,
the validator should run the idempotent Recipe 0 confirmation and record it — an
extra recorded recipe is harmless; a nominated one that never runs is not.
### `01-python-lint`
The touched surface is one YAML policy file plus one generated text artifact. The
recipe's "when to use" enumerates docs, markdown, and dotfile-style paths; neither
`areas.yaml` nor `CODEOWNERS` is literally in that list, but both are
non-executable repository metadata with no behavioural change, which is the
condition the list is standing in for. The escalation clause ("if any runtime code
is touched, this is a pre-screen only") does not fire, because no runtime code is
touched.
- **Section 1 (syntax):** N/A — no Python files touched (files:
`.github/codeowners/areas.yaml`, `CODEOWNERS`). Record the N/A justification in
the recipe's prescribed format rather than skipping the section.
- **Section 2 (pre-commit):** `pre-commit run --files
.github/codeowners/areas.yaml CODEOWNERS --hook-stage manual` from the repo
root. `pre-commit` is present at `/opt/dynamo/venv/bin/pre-commit`. Neither
path is covered by the config's `exclude` regex, so `check-yaml`,
`end-of-file-fixer`, `mixed-line-ending`, `trailing-whitespace`, and
`codespell` all apply. `check-yaml` is the load-bearing one: it proves the
edited `areas.yaml` still parses, which is a precondition for every downstream
claim.
Pass condition: every hook `Passed`, exit 0.
### `03-python-unit-tests-mocker`
This recipe hosts the behavioural proof. Name the mismatch up front so the
validator does not have to guess: the recipe's "when to use" says *Python logic
changes*, and this change contains no Python logic at all. It is nominated
anyway, deliberately, because it is the only recipe in the catalogue carrying the
before/after proof protocol this defect needs — its Section 2 explicitly sanctions
recording a deliberately-failing before-proof and a passing after-run **under the
same recipe id**, with the most recent run resolving the recipe, and its Section 3
demands a non-tautological coverage proof, which is exactly the standard this
change must meet. The alternative (nominating nothing and asserting the fix works)
is worse. The subject under test is `who_owns.py` and `emit_codeowners.py` —
Python programs whose observable output changes with the edit — rather than a new
pytest case. Record these runs in order, all under
`03-python-unit-tests-mocker`:
1. **Before-proof (expected to FAIL, intentionally).** With the fix reverted or
stashed and `CODEOWNERS` back at its committed state, assert that the operator
team is *absent* from the live path's owners — e.g. a command that greps
`who_owns.py --codeowners CODEOWNERS
components/src/dynamo/planner/connectors/clients/kubernetes_api.py` for
`dynamo-operator-codeowners` and exits non-zero when it is missing. A non-zero
exit here is the demonstration that the symptom is real and that the after-run
is not asserting a foregone conclusion.
2. **After: routing restored (expected to PASS).** With the fix applied and
`CODEOWNERS` regenerated, the same assertion over the same path must find
both `@ai-dynamo/dynamo-planner-codeowners` and
`@ai-dynamo/dynamo-operator-codeowners`. This is the claim of the work item,
observed through the same resolver GitHub's behaviour is modelled on.
3. **Negative controls (expected to PASS).** In the same recorded run or an
adjacent one under the same id:
`components/src/dynamo/planner/connectors/clients/remote_client.py` must still
resolve to `planner` **only** — this is what catches an over-broad directory
glob — and `components/src/dynamo/planner/connectors/kubernetes.py` must still
resolve to both teams, proving the neighbouring entry was not disturbed.
4. **Drift gate, replicated exactly (expected to PASS).** Re-run
`emit_codeowners.py` with the CI arguments, then `git diff --exit-code
CODEOWNERS CONTRIBUTORS.md`. Exit 0 proves the committed artifact matches its
source — the specific CI check the Slack thread flags as the reason both files
must land together — and simultaneously proves `CONTRIBUTORS.md` was not
collaterally modified.
5. **Coverage gate, full-tree (expected to PASS).** `build_codeowners.py --areas
.github/codeowners/areas.yaml --repo . --strict`, exit 0. This is the mode CI
will actually use for this PR, because `is_policy_change()` promotes any
`areas.yaml` edit to whole-tree judgement. Capture its dead-glob report: it
must list **11** globs, down from 12, and the planner path must be gone from
it. That delta is the machine-checkable statement that the sweep's in-scope
item was fixed and its out-of-scope items were deliberately left alone.
6. **Existing suite, unregressed (expected to PASS, and must be the last run
under this id).** `/opt/dynamo/venv/bin/python -m pytest
.github/codeowners/test_codeowners.py -v -p no:cacheprovider
--override-ini="addopts=" --override-ini="filterwarnings="` — the exact CI
invocation. Baseline is 86 passed. This is the "do not duplicate existing
coverage" half of the discovery survey: the suite already proves the resolver
and the emitter's tree-independence, so the change must leave it untouched and
green rather than grow it.
- **Section 1 (syntax):** N/A — no Python files changed.
- **Section 3 (behavioural coverage proof):** cite the pairing of step 1 against
step 2 over `who_owns.py` → `codeowners_match.resolve_owners`, driven by
`.github/codeowners/areas.yaml:848`. The observation flips with the production
change and only with it; reverting the glob restores the failure. That is
non-tautological by the standard in `learnings/no-tautological-tests.md`, and
`change.md` must say why no new pytest case was added instead.
Ordering matters: the intentionally-failing run must come first so that the last
recorded run of `03-python-unit-tests-mocker` is green and the recipe resolves to
`pass`, with the before-proof preserved in the evidence trail for a human reader.
Everything above runs on CPU in the sandbox in seconds. If any step is blocked
(e.g. `pre-commit` cannot fetch hook environments because of egress policy), the
validator records the exact command and the disposition rather than a fabricated
pass.
```validation-recipes
01-python-lint
03-python-unit-tests-mocker
```
## Required deliverables
Per the `debug-investigation` template, the agency produces:
- `plan.md` — this file.
- `change.md` — narrative of the repoint and the regeneration, the statement that
the two files must be one commit, the explicit justification for adding no new
test, and the sweep's eleven out-of-scope orphans recorded as a follow-up note.
- `change.diff` — a non-empty diff: one line in `.github/codeowners/areas.yaml`,
two lines in `CODEOWNERS`.
- `change-validation.md` — recorded runs for `01-python-lint` and
`03-python-unit-tests-mocker`, with the `## Investigation outcome:` section
separate from the final verdict, ending in exactly `## Verdict: pass`, `fail`,
or `blocked`.
- `review.md` — ending in exactly `## Assessment: sound` or `needs_changes`. The
reviewer should specifically check that the fix repoints rather than deletes,
that `CODEOWNERS` was regenerated rather than hand-edited (the moved sort
position is the tell), that no directory-wide glob widened ownership onto
`remote_client.py`, and that no tautological mirror test was added.
The publisher opens the request on
`fix/orphaned-codeowners-entry--cc21fb8c3b1b` against `main`, with the issue URL
in the body, `Summary` and `Validation` sections, DCO sign-off on the commit, and
the #11869 conflict note described above. |
Contributor
Author
change.md# Change — wi-20260729T172118Z-12324
Repoint the orphaned `planner`+`operator` `shared:` glob at the path PR
[#10767](https://github.com/ai-dynamo/dynamo/pull/10767) renamed, and regenerate
`CODEOWNERS`. Fixes [#12324](https://github.com/ai-dynamo/dynamo/issues/12324).
- **Branch:** `fix/orphaned-codeowners-entry--cc21fb8c3b1b` (from `main` at
`b4d7a82e77d3c34f012551d6d57cad33a9d47bca`)
- **Commit:** `28e51fe866017cd14a3a8b8092dddac17d4a245d` — one commit, both files
- **Diff:** 2 files, +2 / −2 (`change.diff`, 33 lines, applies cleanly to `main`)
- **Pushed:** nowhere. Publication is the publisher stage's job.
MR URL: https://github.com/ai-dynamo/dynamo/pull/12378
## Files changed
### `.github/codeowners/areas.yaml` (1 line)
Line 848's `glob:` value changed from
`components/src/dynamo/planner/connectors/kubernetes_api.py` to
`components/src/dynamo/planner/connectors/clients/kubernetes_api.py`. The
`owners:` list (`planner`, `operator`), the entry's position inside the `shared:`
block, and every other line are untouched — the block was deliberately **not**
resorted, per the plan.
### `CODEOWNERS` (2 lines: 1 deletion, 1 insertion)
Regenerated, never hand-edited (its own header forbids hand-editing). The rule
disappears from line 702 and reappears about nine lines later. **That relocation
is expected and must not be "fixed":** the emitter orders shared rules by pattern
length, and the pattern grows from 59 to 66 characters. A hand-edit would have
produced the right two owner names at the wrong sort offset and failed the drift
gate — which is precisely why regeneration is not optional. The moved sort
position is the reviewer's tell that the file really was regenerated.
`CONTRIBUTORS.md` came back byte-identical, as the plan required. Had it changed
I was instructed to stop rather than commit it; it did not, so there was nothing
to escalate. No advisory artifact appeared (`areas.yaml` declares
`advisory: false` and `advisory-reviewers.yaml` does not exist).
## Why both files are in one commit
`.github/workflows/codeowners.yml` regenerates `CODEOWNERS` and then runs
`git diff --exit-code CODEOWNERS CONTRIBUTORS.md`. Committing `areas.yaml`
without the regenerated artifact would red-X the intermediate commit. Both files
are staged and committed together in `28e51fe86`.
## What I actually ran
All commands from the repo root at
`/home/sandbox/workspace/wi-20260729T172118Z-12324/repo`.
**1. Symptom reproduced before touching anything** (the before-proof). On the
unmodified tree, `who_owns.py --codeowners CODEOWNERS` over the live path
printed `review: @ai-dynamo/dynamo-planner-codeowners` — `operator` absent,
exactly as the issue reports. `git ls-files | grep -c
'planner/connectors/kubernetes_api.py'` returned `0`, confirming the glob names
a path that is not in the tree.
**2. Baseline drift check, before editing.** Ran the emitter on the *unmodified*
`areas.yaml`: `rules: 615 (base 485 | shared 128 | file-type 1)`, then
`git diff --exit-code CODEOWNERS CONTRIBUTORS.md` → exit 0, `git status` clean.
This matters: it proves the tree was already drift-free, so every line in the
final diff is attributable to my edit and not to pre-existing drift.
**3. Edit + regeneration.** Emitter re-run with the exact CI arguments; rule
count unchanged at 615 (a repoint moves a rule, it does not add or remove one).
Resulting `git diff --stat`: `areas.yaml | 2 +-`, `CODEOWNERS | 2 +-`. Exactly
the size the plan predicted; nothing larger, so the emitter did not pick up
unrelated working-tree state.
**4. Routing restored + negative controls** (the after-proof), post-commit:
```
clients/kubernetes_api.py -> @ai-dynamo/dynamo-planner-codeowners @ai-dynamo/dynamo-operator-codeowners
clients/remote_client.py -> @ai-dynamo/dynamo-planner-codeowners
connectors/kubernetes.py -> @ai-dynamo/dynamo-planner-codeowners @ai-dynamo/dynamo-operator-codeowners
```
The first line is the claim of the work item. The second is the control against
the rejected "broaden the glob to the whole `clients/` directory" alternative —
`remote_client.py` moved in the same rename but never had a shared entry, and it
stays `planner`-only. The third proves the neighbouring line-844 entry was not
disturbed.
**5. Drift gate, replicated exactly against the committed state.** Emitter, then
`git diff --exit-code CODEOWNERS CONTRIBUTORS.md` → **exit 0**, working tree
clean. This is the specific CI check the Slack thread flags, and it simultaneously
proves `CONTRIBUTORS.md` was not collaterally modified.
**6. Coverage gate, full-tree.** `build_codeowners.py --areas
.github/codeowners/areas.yaml --repo . --strict` → **exit 0**. This is the mode
CI will use, because `is_policy_change()` promotes any `areas.yaml` edit to
whole-tree judgement. Dead-glob count fell **12 → 11** and the planner path is
gone from the report.
**7. Existing suite, unregressed.** `/opt/dynamo/venv/bin/python -m pytest
.github/codeowners/test_codeowners.py -p no:cacheprovider --override-ini=...` →
**86 passed in 0.55s**, matching the documented baseline. Not grown, not touched.
**8. Lint on changed files only** (per `learnings/format-changed-only.md`):
`pre-commit run --files .github/codeowners/areas.yaml CODEOWNERS --hook-stage
manual` → **exit 0**. `codespell`, `check-yaml`, `check-case-conflict`,
`check-merge-conflict`, `mixed-line-ending`, `trailing-whitespace` and the marker
report all `Passed`; Python-only hooks skipped with "no files to check", correctly,
since no Python file was touched. `check-yaml` is the load-bearing one — it proves
the edited YAML still parses.
No `cargo build` or broader `pytest` run: this change touches one YAML policy file
and one generated text artifact, no Rust and no Python runtime code.
## Friction, honestly
**The dead-glob report appears to under-count, and it took a detour to confirm it
does not.** `build_codeowners.py --strict` printed the header `globs matching no
files: 11` but then listed only **10** paths, and `lib/llm/src/http/service/
clear_kv_blocks.rs` — one of the four dead `shared:` entries the plan enumerates —
was missing from the listing. That looked at first like a real discrepancy between
the count and the set, which would have undermined using "12 → 11" as evidence, so
I did not accept the number at face value. Reading the source resolved it:
`build_codeowners.py:146` is `for g in dead[:10]`, a hard display truncation. The
count is computed over the full list and is authoritative; the listing is capped
at ten by design. I confirmed by recomputing the set directly through
`compute_resolution` + `match` against `git ls-files`: 11 entries after the fix
including `clear_kv_blocks.rs`, and 12 before (recomputed from
`git show HEAD:.github/codeowners/areas.yaml`), with the planner path present
before and absent after. This is a cosmetic reporting wart in a
non-blocking advisory report, not a defect in this change, and it is explicitly
out of scope — I did not touch `build_codeowners.py`.
A smaller stumble: my first attempt to recompute the dead set used
`build_codeowners.load_model(...)`, which does not exist —
`AttributeError: module 'build_codeowners' has no attribute 'load_model'`. The
real path is `yaml.safe_load` into `codeowners_match.compute_resolution`. I read
`main()` and used the actual API.
One expected-but-worth-naming result: an intermediate drift check run *before*
committing returned exit 1. That is not a failure — `git diff` compares against
`HEAD`, so uncommitted edits necessarily show as drift. The meaningful run is the
post-commit one in step 5, which is exit 0.
## Why no new test was added
Deliberate, and mandated by the plan. The only test this change could carry is a
config mirror — something like "the glob at `areas.yaml:848` is
`.../clients/kubernetes_api.py` and its owners are `planner`+`operator`". That
re-states the diff verbatim and would pass forever without catching anything,
which is exactly the failure mode `learnings/no-tautological-tests.md` forbids and
the reviewer is instructed to reject as a blocking finding.
The behaviour that changed lives in *data*, not logic, and the logic around it is
already covered by the 86 existing tests: `TestResolveOwners::
test_multi_owner_passthrough` covers multi-owner resolution,
`TestComputeResolution::test_shared_multi_owner_recorded` and
`test_explicit_shared_entry_still_wins` cover the shared-entry path, and
`TestEmissionIsTreeIndependent` covers the emitter contract. Duplicating any of
those would be the "do not duplicate existing coverage" half of the same mistake.
The defensible proof is therefore a recorded **before/after behavioural
observation** through the real resolver, not a new pytest case: step 1 above
(operator absent, symptom real) paired against step 4 (both teams present),
driven through `who_owns.py` → `codeowners_match.resolve_owners` by the single
changed line `.github/codeowners/areas.yaml:848`. The observation flips with the
production change and only with it — revert the glob and the failure returns,
which is the non-tautological standard. The validator will record that pairing
under `03-python-unit-tests-mocker`, whose Section 2 explicitly sanctions a
deliberately-failing before-proof followed by a passing after-run under the same
recipe id, together with the two negative controls, the drift gate, the coverage
gate, and the unregressed 86-test suite.
## Follow-up note for the reviewer and the PR body
### Coordination with #11869
PR [#11869](https://github.com/ai-dynamo/dynamo/pull/11869) (open, head
`codex/codeowners-routing-operator-ops`) **deletes** the same four lines this
change rewrites. Whichever lands second will hit a textual conflict in
`areas.yaml`. **The correct resolution in either order is the repointed glob**,
because deleting the entry leaves `clients/kubernetes_api.py` owned by `planner`
alone — re-opening the exact gap #12324 reports. This must be said in the PR
description so the #11869 author does not resolve the conflict by taking their
deletion.
### Eleven other dead globs, found and deliberately left alone
The planner swept `areas.yaml` and found twelve dead globs. Only the one fixed
here is a *rename* with a successor path, hence the only one that can be
repointed at all. The other eleven are reported, not actioned — fixing them would
be either harmful, inert, or a duplicate of #11869:
**Dead `shared:` entries — 3 remaining (of 4 found):**
| Glob | Owners | Cause | Why left alone |
|---|---|---|---|
| `lib/llm/src/http/service/clear_kv_blocks.rs` | frontend, kv-memory | Deleted outright (238 lines, no rename pair) by `07d9806ac` (#10556, on `main`) | A tombstone — the code is gone, there is nothing to co-own. Deletion only, which is #11869's scope |
| `components/src/dynamo/trtllm/tests/test_trtllm_gms_mpi_bootstrap.py` | backend-trtllm, gms | Never on `main`; introduced by `d2748fdf2`, not an ancestor of HEAD | Forward-looking. Deleting would be *actively wrong* — the branch adding these files would land without GMS co-ownership, reintroducing this same silent gap in mirror image |
| `components/src/dynamo/trtllm/tests/test_trtllm_gms_delay_commit.py` | backend-trtllm, gms | Same (`69ddf9375`, not an ancestor of HEAD) | Same as above |
**Dead single-owner `path_globs` — 8:**
`lib/bindings/python/tests/test_parsers.py` (frontend), `lib/renderer/`
(multimodal), `deploy/operator/internal/featuregate/` (operator), and five
`ATTRIBUTIONS*.md` entries (process) — the last of which are `.gitignore`d
generated artifacts, so they will never appear in `git ls-files` **by design**.
All eight are single-owner, so no co-ownership can have been lost, and removing
one changes no routing decision.
I verified this list against the live report rather than copying it blind: the
recomputed post-fix dead set is exactly these eleven.
### Separately worth filing
A generic "no `shared:` glob may point at a nonexistent path" CI guard would make
this failure class loud instead of silent. It was **not** added here: it fails
today on the three out-of-scope orphans above, so it cannot land without the
cleanup this work item declares a non-goal, and it contradicts a deliberate
documented decision in `build_codeowners.py` ("Deletions never fail a gate …
Surface them; never block on them"). That deserves its own discussion, not a
drive-by change.
## Constraints honoured
- Non-goals respected: no other dead glob pruned, no new CI guard, no change to
`emit_codeowners.py` / `build_codeowners.py` / `codeowners_match.py` /
`who_owns.py` / `.github/workflows/codeowners.yml`, no Python source moved, no
team-membership edit.
- Rejected alternatives avoided: did not delete the entry, did not add a second
entry beside the old one, did not broaden the glob to the `clients/` directory
(the `remote_client.py` control proves this), did not hand-edit `CODEOWNERS`.
- Branch is exactly `${GLAMR_CODING_BRANCH_NAME}`, cut from `main`. No other name
invented.
- `git commit -s`; trailer email `svc-glamr@nvidia.com` matches the author email,
exactly one `Signed-off-by:` line.
- Formatted/linted only the two changed files.
- Pushed nowhere. |
Contributor
Author
change-validation.md# Validation — wi-20260729T172118Z-12324
Repoint of the orphaned `planner`+`operator` `shared:` glob at
`.github/codeowners/areas.yaml:848`, plus the regenerated `CODEOWNERS`.
Commit `28e51fe866017cd14a3a8b8092dddac17d4a245d` on
`fix/orphaned-codeowners-entry--cc21fb8c3b1b`.
## Hardware target
**None, deliberately.** No preset, no cluster, no model. The sandbox reports one
A100-SXM4-80GB and CUDA 12.9, and none of it was used: the change is two files of
repository metadata — one YAML policy input and one generated text artifact — with
no runtime code, no engine, no Kubernetes, and no Docker on any path. Everything
below ran on CPU in-sandbox in seconds. The plan's argument for nominating no GPU
recipe holds on inspection of the diff: `git diff --name-only HEAD~1 HEAD` returns
exactly `.github/codeowners/areas.yaml` and `CODEOWNERS` (run 1).
`00-dynamo-editable-install` was not nominated and did not become necessary.
`.github/codeowners/test_codeowners.py` imports only `codeowners_match`,
`build_codeowners`, `yaml`, and stdlib; pytest collection never reached `dynamo.*`
and no `ModuleNotFoundError: No module named 'dynamo._core'` appeared, so the
Recipe 0 confirmation the plan held in reserve was not triggered.
## What the logs prove
### `01-python-lint`
**Section 1 (syntax): N/A — no Python files touched (files:
`.github/codeowners/areas.yaml`, `CODEOWNERS`).** I did not take that on the
plan's word; run 1 (`git diff --name-only HEAD~1 HEAD`, exit 0) enumerates the
commit's entire surface and it contains no `.py` path, so `python3 -m py_compile`
has no argument to take.
**Section 2 (pre-commit):** exit 0 (run 2). Neither changed path is caught by the
config's `exclude` regex, so the general-purpose hooks all applied and reported
`Passed`: `codespell`, `check for case conflicts`, `check for merge conflicts`,
**`check yaml`**, `check that scripts with shebangs are executable`,
`mixed line ending`, `trim trailing whitespace`, and the pytest-marker report. The
Python-specific hooks (`isort`, `black`, `flake8`, `ruff`) reported
`(no files to check) Skipped` — the correct outcome, and independent corroboration
of the Section 1 N/A from a second tool. `check yaml` is the load-bearing hook and
it is what makes every downstream claim admissible: the edited `areas.yaml` still
parses.
### `03-python-unit-tests-mocker`
The recipe is nominated for a change that contains no Python logic, which the plan
names up front. What it supplies is the before/after protocol this defect needs,
and Section 2 of the recipe explicitly sanctions recording a deliberately-failing
before-proof and a passing after-run under the same id. Six protocol steps, seven
recorded runs (step 5 is recorded twice — see below), in the prescribed order, all
under the one id.
**Step 1 — before-proof, exit 1, failed deliberately (run 3).** The two changed
files were reverted to their committed pre-change state (`git checkout HEAD~1 --
CODEOWNERS .github/codeowners/areas.yaml`), which the log confirms restored the
stale glob at line 848. `who_owns.py --codeowners CODEOWNERS
components/src/dynamo/planner/connectors/clients/kubernetes_api.py` then printed
`review: @ai-dynamo/dynamo-planner-codeowners` — operator absent, exactly the
symptom the issue reports. The assertion "operator must be present" therefore
failed and the script exited 1. This is the run that makes step 2 non-vacuous: the
symptom was real on the pre-change tree, so the after-run is not a foregone
conclusion. An `EXIT` trap restored the fix; the log shows `git status --porcelain`
empty afterwards, and I verified restoration independently outside the script
(clean tree, HEAD `28e51fe86`, line 848 carrying the `clients/` path) before
proceeding.
**Step 2 — after, exit 0 (run 4).** Same path, same resolver, fix in effect on a
clean tree: `review: @ai-dynamo/dynamo-planner-codeowners
@ai-dynamo/dynamo-operator-codeowners`. Both assertions passed. The single changed
line flips the observation, and nothing else in the run differs from step 1.
**Step 3 — negative controls, exit 0 (run 5).** Both control paths were first
confirmed present in the tree via `git ls-files --error-unmatch`, so neither result
is the trivial answer for a path that does not exist.
`components/src/dynamo/planner/connectors/clients/remote_client.py` resolves to
`@ai-dynamo/dynamo-planner-codeowners` **only** — operator absent. That is the
control that would have caught the rejected "widen the glob to the whole
`clients/` directory" alternative: `remote_client.py` moved in the same rename and
sits in the same directory, so a directory-wide glob would have silently given
`operator` co-ownership of it. It did not.
`components/src/dynamo/planner/connectors/kubernetes.py` still resolves to **both**
teams, proving the neighbouring `shared:` entry at line 844 was not disturbed by
the edit one entry below it.
**Step 4 — drift gate, exit 0 (run 6).** The log opens by quoting the CI step being
replicated out of `.github/workflows/codeowners.yml:81-89`, so the correspondence
is visible rather than asserted. From a clean tree, `emit_codeowners.py` ran with
the exact CI arguments (`rules: 615 (base 485 | shared 128 | file-type 1)`,
`0 external contributor(s)`), then `git diff --exit-code CODEOWNERS
CONTRIBUTORS.md` returned **exit 0**. The committed artifact is byte-identical to a
fresh regeneration from its source — the specific check that requires both files in
one commit — and `CONTRIBUTORS.md` was not collaterally modified. `git status
--porcelain` was empty after regeneration, so nothing else in the tree moved, and a
grep for an advisory artifact over untracked files found none, matching the plan.
This also settles by observation the reviewer's tell that `CODEOWNERS` was
regenerated rather than hand-edited: a hand-edit would have placed the rule at the
old sort offset and this diff would have been non-empty.
**Step 5 — coverage gate, full-tree, exit 0 (run 8; run 7 is a superseded earlier
attempt, see below).** `build_codeowners.py --areas .github/codeowners/areas.yaml
--repo . --strict` exited 0 with `explicitly owned: 5025/5025 (100.00%) |
catch-all only: 0`. This is the mode CI will actually apply, because
`is_policy_change()` promotes any `areas.yaml` edit to whole-tree judgement.
I was asked to verify the printer's display-truncation claim rather than accept it,
and it holds. The report printed `globs matching no files: 11` while listing 10
paths. Reading `build_codeowners.py` at the cited site shows `for g in dead[:10]`
directly beneath an f-string interpolating `len(dead)` — the count is computed over
the full list, the listing is capped. I then recomputed the dead set independently
through `yaml.safe_load` → `codeowners_match.compute_resolution` → `match` against
`git ls-files`, for both `HEAD` and `HEAD~1`:
- **12 dead globs before, 11 after.**
- The set difference `before − after` is exactly one element:
`/components/src/dynamo/planner/connectors/kubernetes_api.py` — the stale glob.
- `after − before` is **empty**: no glob newly became dead.
- The repointed glob is not in the dead set, i.e. it now matches a real file.
- The independent recount is **11**, agreeing with the report's printed count, and
the full untruncated set contains `/lib/llm/src/http/service/clear_kv_blocks.rs`
— the entry missing from the capped listing that made the report look inconsistent.
So the 11-vs-10 gap is a cosmetic display truncation in a non-blocking advisory
report, confirmed from both the source and an independent recomputation. It is not
a defect in this change and I do not report it as one. The 12 → 11 delta is the
machine-checkable statement that the in-scope orphan was fixed and the eleven
out-of-scope ones were left exactly as they were.
**Run 7 — my own script bug, not the change's.** The first execution of step 5
exited 1 on two assertions. The cause was in my harness: `compute_resolution`
anchors globs, so `owned_patterns()` yields leading-slash paths, and I had compared
against unanchored constants. The substantive output of that same run already
showed the correct result (12 before, 11 after, only-in-BEFORE being the planner
glob with its leading slash). I corrected the comparison, added an assertion that
`owned_patterns()` really is anchored, and re-ran; run 8 is the corrected execution
and passes all seven assertions. I am recording the failed attempt rather than
hiding it, and I want to be precise about what it does and does not mean: it is
evidence about my script, and it exercised nothing about the production change that
run 8 does not exercise more strictly.
**Step 6 — existing suite, exit 0 (run 9), and the last run under this id.** The
exact CI invocation, `/opt/dynamo/venv/bin/python -m pytest
.github/codeowners/test_codeowners.py -v -p no:cacheprovider
--override-ini=addopts= --override-ini=filterwarnings=`, gave **86 passed in
0.60s** — matching the documented baseline exactly. The suite is neither grown nor
modified, which is the correct outcome here: `TestResolveOwners`,
`TestComputeResolution`, and `TestEmissionIsTreeIndependent` already cover the
resolver and the emitter contract, and duplicating them would be the mistake the
discovery survey warns against.
**Section 3 — behavioural coverage proof.** The non-tautological observation is the
pairing of step 1 against step 2:
```
Section 3 coverage:
- step 1 (run 3, exit 1) vs step 2 (run 4, exit 0): exercises
.github/codeowners/areas.yaml:848 (the repointed `glob:` value) through
who_owns.py -> codeowners_match.resolve_owners over the live path
components/src/dynamo/planner/connectors/clients/kubernetes_api.py
- step 3 (run 5, exit 0): exercises the same resolver over
clients/remote_client.py (must stay planner-only) and
connectors/kubernetes.py (areas.yaml:844, must stay both)
```
This meets the standard in `learnings/no-tautological-tests.md` by the test the
learning itself sets: revert the production change and the observation reverts with
it — which is not an argument here but a recorded fact, because run 3 *is* the
reverted state and it failed. A mirror test asserting "the glob at line 848 equals
`.../clients/kubernetes_api.py`" would re-state the diff and pass forever; the
recorded pairing cannot, because it fails on the pre-change tree. The change adds
no new pytest case, and on this evidence that is the right call rather than a gap.
### Recipes deliberately not run
Per the plan, and I concur on inspection of the diff: `07-agg-smoke`,
`08-disagg-pair-smoke`, `09-gpu-pytest`, `10-perf-benchmark`, and
`11-go-operator-tests` would spend the A100 to prove nothing about a YAML glob, and
`05-code-inspection` requires a PR number and CI status that do not exist at
validation time (the publisher opens the request afterwards). No recipe beyond the
two nominated was added.
## Scope of what this validates, stated plainly
The recorded runs establish that the repository's own resolver — the one
`emit_codeowners.py` and `who_owns.py` share, so there is no second implementation
to drift — now routes the live file to both teams, that the generated artifact
matches its source under the exact CI command, that no neighbouring or sibling path
changed ownership, and that the 86-test suite is unregressed. What no in-sandbox
run can establish is GitHub's own behaviour on the emitted file; `codeowners_match`
is a faithful model of last-match-wins resolution and is what the repository's CI
and tooling rely on, but it is a model. That limit is inherent to the change, not a
gap in this validation, and no recipe in the catalogue closes it.
## Investigation outcome:
The reported defect is confirmed, its mechanism is confirmed, and the fix corrects
it without collateral movement.
The symptom was reproduced empirically on the pre-change tree (run 3): the live
path `components/src/dynamo/planner/connectors/clients/kubernetes_api.py` resolved
to the planner team alone. The plan's H1 — the `shared:` glob names a path the
rename left behind, so the emitted rule can never match — is what the evidence
supports. H3 (a resolver defect) is refuted directly by run 5: the same resolver,
in the same invocation, returns both teams for `connectors/kubernetes.py`, whose
glob is live. H2 (an emitter that filters dead rules) is refuted by run 6: the
emitter is a pure function of `areas.yaml`, the regeneration is byte-stable, and it
faithfully emitted a rule against a path that does not exist — the generator was
never wrong, its input was stale. H4 (a gate should have caught it) is refuted and
explained by run 8: the dead-glob report saw the orphan all along and is
non-blocking by explicit design, which is why the defect survived CI silently.
The fix repoints rather than deletes — the distinction that matters against open PR
#11869, since deleting the entry would leave the live file planner-only and re-open
the very gap the issue reports. It does not widen: `remote_client.py`, which moved
in the same rename and shares the directory, is still planner-only. `CODEOWNERS`
was regenerated, not hand-edited, and the drift gate proves it at exit 0. The
dead-glob population fell 12 → 11 with exactly the planner glob leaving and nothing
newly entering, so the eleven out-of-scope orphans the plan enumerated were
genuinely left alone rather than incidentally disturbed.
One observation for the reviewer that is not a defect: `build_codeowners.py:146`
truncates the dead-glob *listing* to ten entries while reporting the full *count*,
which makes an 11-count report look self-inconsistent. I verified from the source
and from an independent recomputation that the count is authoritative and the
listing is cosmetic. It is pre-existing, out of scope, and untouched here.
Both nominated recipes ran green. Every command went through the recorder; nine
runs are recorded, and the last run under each recipe id exits 0. The repository
checkout is byte-identical to the printer's commit — clean tree at
`28e51fe866017cd14a3a8b8092dddac17d4a245d` — verified after the final run.
## Verdict: pass |
Contributor
Author
review.md# Review — wi-20260729T172118Z-12324
> 🤖 **Automated AI review — advisory.** An AI agent's judgment of
> whether this change is logically sound on the evidence; not a merge
> authorization. CI, customs, and a human reviewer hold that.
## Assessment: sound
Repoint of the orphaned `planner`+`operator` `shared:` glob at
`.github/codeowners/areas.yaml:848`, plus the regenerated `CODEOWNERS`.
Commit `28e51fe866017cd14a3a8b8092dddac17d4a245d` on
`fix/orphaned-codeowners-entry--cc21fb8c3b1b`, clean tree, one DCO trailer.
Everything below was verified by me against the clone and the diff. Where the
prior packets asserted a fact, I re-derived it rather than accepting it — in
two places (the dead-glob delta and the routing resolution) with a matcher I
wrote myself rather than the repository's own, so the check does not inherit
the tooling under test.
---
## Findings
| # | file:line | Severity | Claim | Evidence |
|---|---|---|---|---|
| 1 | `.github/codeowners/areas.yaml:848` | none (verified) | The fix **repoints**; it does not delete | The line reads `- glob: components/src/dynamo/planner/connectors/clients/kubernetes_api.py` with `owners: [planner, operator]` intact directly beneath. `git show b4d7a82e7:.github/codeowners/areas.yaml \| sed -n 848p` carries the old path at the same line number, so the entry was edited in place, not removed and re-added. Deleting it (open PR #11869's approach) would leave the live file `planner`-only — the bug, not the fix |
| 2 | `CODEOWNERS:714` | none (verified) | `CODEOWNERS` was **regenerated**, not hand-edited | Three independent signals. (a) The rule left file line 702 and reappeared at 714 — shared-rule index 593 → 605. (b) I extracted all 128 shared rules and checked the emitter's own sort key `(len(pattern), pattern)` from `emit_codeowners.py:259` holds across the whole block: `True`, no violations. (c) Owner-column offsets across the entire file collapse to a single value, 77, equal to `max(len(pattern)) + 2` per `emit_codeowners.py:297`. A hand-edit would have to reproduce the sort position *and* the recomputed column width by accident. Run 6 (`.../step4_drift_gate...log`) closes it: `git diff --exit-code CODEOWNERS CONTRIBUTORS.md` after a real regeneration, exit 0 |
| 3 | `CODEOWNERS:176` | none (verified) | No directory-wide glob widened ownership onto `remote_client.py` | `areas.yaml` contains exactly three `planner/connectors` references (lines 207, 844, 848); the only new one is an exact-file glob. Resolving with my own last-match-wins implementation over the committed `CODEOWNERS`, `clients/remote_client.py` lands on `CODEOWNERS:176` `/components/src/dynamo/planner/connectors/` → planner alone — byte-identical to its pre-change resolution. `grep -n remote_client CODEOWNERS` returns nothing. `git show -M e69e5fdf606` confirms #10767 moved exactly two files and that `remote_client.py` never carried a shared entry, so leaving it planner-only preserves rather than changes routing |
| 4 | `.github/codeowners/test_codeowners.py` | none (verified) | No tautological mirror test was added, and the no-test decision is justified | The file is untouched by the commit (`git show --stat HEAD -- <path>` empty) and 1082 lines at both `b4d7a82e7` and `HEAD`. On whether that is an excuse: I read the suite. It builds synthetic fixtures and `tmp_path` repos throughout; `_AREAS` at line 700 is a bare string for path comparison in `TestIsPolicyChange`, never a content read. So the suite cannot express this change — the only in-suite test possible is "the glob at line 848 equals the value in the diff", which `learnings/no-tautological-tests.md` forbids by name. The behavioural alternative (a blanket "no dead `shared:` glob" gate) fails today on three pre-existing orphans and directly contradicts the documented decision at `build_codeowners.py:143-147` ("never blocking"). Deferring it is correct, not evasive. The substantive proof is instead the recorded revert-and-observe pairing, run 3 (exit 1) against run 4 (exit 0) |
| 5 | commit `28e51fe86` | none (verified) | Both files are in **one** commit | `git show --stat HEAD` lists exactly `.github/codeowners/areas.yaml` and `CODEOWNERS`, +2/−2. `.github/workflows/codeowners.yml:81-89` regenerates then runs `git diff --exit-code`, so a split commit would red-X the intermediate. `build_codeowners.py:66` `is_policy_change()` additionally promotes this PR to full-tree strict coverage; run 8 records that mode green at 5025/5025 |
| 6 | `CONTRIBUTORS.md` | none (verified) | Not collaterally modified | `git diff --stat b4d7a82e7 HEAD -- CONTRIBUTORS.md` is empty, and the drift gate in run 6 covers it explicitly |
| 7 | `change.md:29`, `plan.md:279` | **nit** | Prose mixes anchored and unanchored pattern lengths | Both packets say the pattern "grows from 59 to 66 characters". Anchored it is 59 → 67; unanchored, 58 → 66. Consequence: none — the emitter sorts on the anchored form (`emit_codeowners.py:255,259`) and the conclusion the sentence draws (longer pattern ⇒ later sort position ⇒ the rule legitimately moved) is correct either way. Cosmetic bookkeeping in narrative only; no code or artifact is affected |
No blocking finding. Item 7 is a prose slip a human need not act on.
---
## Evidence audit
`node agents/customs/evidence.ts report <root>` → **cleared [2/2 validated]**. Both
rows correspond to recipes the plan named in its `validation-recipes` block
(`01-python-lint`, `03-python-unit-tests-mocker`); no recipe the plan named is
`missing`, and no recipe beyond the two was added.
I opened all nine logs in `validation/logs/` and confirmed each shows real
execution, not a toolchain-missing or "did not run" stub:
| Run | Recipe | Exit | Does the log show execution against the change? |
|---|---|---|---|
| 1 | 01 | 0 | `git diff --name-only HEAD~1 HEAD` prints exactly the two changed paths — the Section 1 `N/A` for `py_compile` is derived, not asserted |
| 2 | 01 | 0 | Real pre-commit output; `check yaml`, `codespell`, `mixed line ending`, `trim trailing whitespace` all `Passed` on the changed files; Python-only hooks `(no files to check) Skipped`, consistent with a YAML-and-text diff |
| 3 | 03 | **1** | Before-proof. Log shows `git checkout HEAD~1 --` restoring the stale glob at line 848, `who_owns.py` returning planner alone, and a deliberate `rc=1`. An `EXIT` trap restores the fix and `git status --porcelain` is empty afterwards |
| 4 | 03 | 0 | Same path, same resolver, clean tree at `28e51fe86` → both teams. This is the work item's claim |
| 5 | 03 | 0 | Negative controls, with `git ls-files --error-unmatch` first so neither answer is the trivial one for a nonexistent path: `remote_client.py` planner-only, `kubernetes.py` both |
| 6 | 03 | 0 | Drift gate. Quotes `.github/workflows/codeowners.yml:75-89` inline, regenerates (`rules: 615`), `git diff --exit-code` exit 0, tree clean, no advisory artifact |
| 7 | 03 | **1** | Validator's own harness bug — see below |
| 8 | 03 | 0 | Corrected coverage gate: strict full-tree exit 0, 5025/5025, seven assertions green |
| 9 | 03 | 0 | `86 passed in 0.60s` under the exact CI invocation — a genuine unregressed-baseline control, and the packets describe it as exactly that rather than as proof of the fix |
One nuance worth stating for the human reader: customs surfaces the *most recent*
run per recipe id, so the `03` row cites run 9 (the 86-test suite), which does not
itself exercise the changed data. The runs that do — 3 through 8 — are recorded
under the same id, which is precisely what `03-python-unit-tests-mocker` Section 2
prescribes for a before-proof/after-run pair. This is the designed behaviour of the
evidence table, not a green row that failed to exercise the change, so it does not
warrant a `failing` disposition.
The verdict is `## Verdict: pass` — unqualified. No hedging language ("conditionally",
"static", "partial") appears anywhere in `change-validation.md`, and the verdict is
backed by nine executed runs rather than asserted over a build that never happened.
`## Investigation outcome:` is separated from the verdict as the template requires.
**Compute-env cross-check.** `compute-env.md` reports one A100 and no Docker daemon.
No remote compute, SSH, or Slurm appears in any recorded command — every run is local
CPU. No recipe in force required a GPU, and the validator's decision to decline
`07`/`08`/`09`/`10`/`11` is correct on the diff: a YAML glob and a generated text file
have no GPU-observable behaviour. Declining `05-code-inspection` is also right, since
its Sections 1 and 4 need a PR number that does not exist until the publisher stage.
---
## The two flagged items, my own view
**`build_codeowners.py:146` display truncation.** I read the source rather than
taking either packet's word. Lines 143-147 interpolate `len(dead)` into the header
and then iterate `for g in dead[:10]`. The count is computed over the full list; the
listing is capped at ten. A printed count of 11 beside 10 paths is therefore cosmetic.
Crucially, this does **not** undermine the "12 → 11" evidence, because that number
never came from the truncated listing: I recomputed the dead set myself with my own
matcher against `git ls-files` for both `HEAD~1` and `HEAD` and got **12 before, 11
after**, with the same eleven-member post-fix set including `clear_kv_blocks.rs` — the
entry the cap hides. Leaving the truncation untouched is right. It is a non-blocking
advisory display in a file the plan lists under non-goals, and "fix the report while
you are here" is exactly the scope creep this work item was carved to avoid.
**Run 7.** I read run 7 and run 8 side by side. Section D of run 7 — the substantive
output — already reports 12 before, 11 after, and `only-in-BEFORE` containing the
planner glob *with its leading slash*. The two `FAIL` lines are string comparisons
against unanchored constants, i.e. the harness compared `/components/...` to
`components/...`. `compute_resolution` anchors patterns (`emit_codeowners.py:255`
`anchor(s["glob"])`), so the constants were wrong, not the data. Run 8 fixes the
comparison, adds an assertion that `owned_patterns()` really is anchored, and passes
all seven. This invalidates nothing substantive: run 7 exercised nothing about the
production change that run 8 does not exercise more strictly, and both sit under the
same recipe id with run 8 last, so the recipe resolves green by design. Recording the
failed attempt rather than quietly deleting it is honest self-correction and improves
the packet.
---
## The planner's scope decision
Defensible, and I checked it rather than deferring to it. My independent recompute
returns the same eleven remaining dead globs with the same membership:
- **`clear_kv_blocks.rs`** — `git ls-files | grep clear_kv` is empty and `07d9806ac`
*is* an ancestor of `HEAD`. No successor path exists, so repointing is impossible;
only deletion, which is #11869's declared scope. Correct to leave.
- **The two `test_trtllm_gms_*` entries** — `d2748fdf2` and `69ddf9375` exist in the
clone but `git merge-base --is-ancestor ... HEAD` returns false for both, and neither
file is in the tree. They are forward-looking. Deleting them would drop GMS
co-ownership the moment those files land — the same silent gap this issue reports,
in mirror image. Leaving them is not laziness; acting on them would be the error.
- **The eight single-owner `path_globs`** — single-owner by construction, so no
co-ownership can have been lost; and five are `ATTRIBUTIONS*.md`, which
`git check-ignore -v` confirms are matched by `.gitignore:9-10`, i.e. generated
artifacts that will never appear in `git ls-files` by design.
On completeness against the issue: the issue's sweep clause is scoped — "other
`shared:` entries orphaned **by the same refactor**". `git show -M e69e5fdf606` shows
#10767 renamed exactly two files, and `remote_client.py` never had a shared entry on
`main`. So the sweep *as the issue framed it* returns exactly one orphan, and it was
fixed. The planner went further, swept all 128 shared entries plus 485 area globs, and
recorded the full twelve in both `plan.md` and `change.md` so the finding survives this
PR. That over-delivers on the literal ask while keeping the diff at four lines. I do
not consider the work item incomplete.
---
## Summary
The change does the one thing the issue asks and nothing else. It repoints rather
than deletes — the distinction that separates it from #11869 and the reason deletion
would not have fixed the bug. It regenerates rather than hand-edits, verifiable from
the sort invariant and column width independently of the drift gate. It does not
widen onto `remote_client.py`. It adds no mirror test, and the no-test decision
survives scrutiny of the suite rather than resting on assertion. Both files land in
one commit, `CONTRIBUTORS.md` is untouched, and the validation is an unqualified pass
whose logs show real execution against the changed source.
The one thing no in-sandbox run can establish is GitHub's own behaviour on the emitted
file; `codeowners_match` is a faithful model of last-match-wins and is what this
repository's CI relies on, but it is a model. `change-validation.md` states that limit
plainly instead of overclaiming, which is the right posture. It is inherent to the
change, not a gap in the work.
## Assessment: sound |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
WalkthroughUpdated Codeowners mappings to point from the old planner Kubernetes connector path to ChangesOwnership routing
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
Contributor
Author
CI result: passingObserved at commit
This supersedes the earlier report of a What the
|
glamr-agent
temporarily deployed
to
external_collaborator
July 30, 2026 01:39 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
dmitry-tokarev-nv
approved these changes
Aug 11, 2026
dmitry-tokarev-nv
left a comment
dmitry-tokarev-nv
left a comment
Contributor
There was a problem hiding this comment.
thank you for the contribution!
dmitry-tokarev-nv
enabled auto-merge (squash)
August 11, 2026 15:22
Contributor
|
/ok to test 66f2394 |
dagil-nvidia
added a commit
that referenced
this pull request
Aug 12, 2026
Reconciles two overlapping redesigns of the same resolver. main removed the advisory mechanism in #12977; this branch still implemented it and had built required_owners and the shared-additivity gate on top. Advisory loses: main's rejection guards stay, and every advisory code path here is removed -- the filetype normalizer returns blocking rows only, _dead_patterns no longer walks model.advisory/filetype_advisory (both fields are gone on main, so those were dangling), and the advisory-behavior tests are replaced by a required_owners equivalent. main's three rejection tests are kept as-is. This branch's validation survives: _normalize_owner_rules and _normalize_filetype_rules keep their owner-token checks (main has neither helper), required_owners is intact, and shared_additivity_violations still gates. Also keeps BOTH planner connector globs. main's #12378 added connectors/clients/kubernetes_api.py alongside the live connectors/ kubernetes.py; the conflict presented that as one edited line, and taking one side alone silently stripped operator from a tracked file. Verified against main over all 5468 tracked files: 443 change owners and every transition is additive except components/src/dynamo/vllm/handlers.py, where dropping RL is this PR's intent and is asserted by test_generic_vllm_handler_excludes_rl. The 461 paths losing @jasonqinzhou/@dreamtalen are main's #12939 moving planner ownership to the team. Gate exits 0, no CODEOWNERS drift, 157 tests pass. Signed-off-by: Dan Gil <dagil@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview:
The
shared:entry that gives the operator team co-ownership of the planner'sKubernetes API connector still points at
components/src/dynamo/planner/connectors/kubernetes_api.py, a path that nolonger exists. PR #10767 moved that file into
connectors/clients/, and theCODEOWNERS entry was not moved with it.
Because the generator silently drops globs that match no files, the result is a
routing gap that fails quietly: changes to the live file today route to
@ai-dynamo/dynamo-planner-codeownersalone, and the operator team is neverrequested for review. This PR repoints the glob at the current path and
regenerates
CODEOWNERS.Details:
Two files, +2 / −2, in a single commit.
.github/codeowners/areas.yaml— theglob:value of theplanner+operatorshared entry changes fromcomponents/src/dynamo/planner/connectors/kubernetes_api.pytocomponents/src/dynamo/planner/connectors/clients/kubernetes_api.py. Theowners:list and the entry's position in theshared:block are unchanged.CODEOWNERS— regenerated withpython .github/codeowners/build_codeowners.py, never hand-edited. Note thatthe rule does not stay on the same line: the emitter sorts shared rules by
pattern length, and the longer path moves the rule about nine lines further
down. That relocation is expected output of the generator, not a stray edit.
Both files are in one commit deliberately.
.github/workflows/codeowners.ymlregenerates
CODEOWNERSand then runsgit diff --exit-code CODEOWNERS CONTRIBUTORS.md, so committingareas.yamlwithout the regenerated artifact would fail that gate.
The entry is repointed, not deleted. Deleting it would leave
clients/kubernetes_api.pyowned by the planner team alone, which is the bugthis issue reports rather than a fix for it.
Scope
Only the one orphaned entry is touched. A sweep of
areas.yamlfound elevenother globs that match no files; none of them is a rename with a successor path,
so none can be repointed:
lib/llm/src/http/service/clear_kv_blocks.rs— the file was deleted outrightin feat(frontend): add DYN_ENABLE_NVEXT and DYN_ENABLE_FRONTEND_ADMIN_API master switches #10556, so there is nothing left to co-own.
test_trtllm_gms_*.pyshared entries are forward-looking: those filesare not on
mainyet. Removing them would drop GMS co-ownership at the momentthey land, reintroducing this same class of gap.
path_globs, so no co-ownership can havebeen lost, and five of them are
.gitignored generatedATTRIBUTIONS*.mdartifacts that will never appear in
git ls-filesby design.Cleaning those up is deletion work already in flight in #11869 and is left out
of this PR.
Coordination with #11869
Important
#11869 deletes the same entry this PR rewrites, so whichever lands second
will hit a textual conflict in
areas.yaml. In either order, the correctresolution is to keep the repointed glob. Taking the deletion instead leaves
clients/kubernetes_api.pyowned by the planner team alone and reopens theexact gap described in #12324.
Validation
who_owns.py --codeowners CODEOWNERSoncomponents/src/dynamo/planner/connectors/clients/kubernetes_api.pyreturnedplanner only before the change and planner + operator after it.
connectors/clients/remote_client.pystays planner-only(it moved in the same rename but never had a shared entry), and the adjacent
connectors/kubernetes.pyentry still resolves to both teams — confirming theglob was not broadened to the whole
clients/directory.git diff --exit-code CODEOWNERS CONTRIBUTORS.md→ exit 0.
CONTRIBUTORS.mdis byte-identical.build_codeowners.py --areas .github/codeowners/areas.yaml --repo . --strict→ exit 0 (5025/5025 paths covered). Globs matching no filesdrop from 12 to 11, and the planner path is gone from the report.
pytest .github/codeowners/test_codeowners.py→ 86 passed,matching the pre-change baseline.
pre-commit run --files .github/codeowners/areas.yaml CODEOWNERS --hook-stage manual→ exit 0.No new test is added. The only test this change could carry is a mirror of the
diff itself — asserting that the glob equals the literal value in the diff —
which would pass forever without catching a regression. The logic around the
data is already covered by the existing suite (
test_multi_owner_passthrough,test_shared_multi_owner_recorded,test_explicit_shared_entry_still_wins,TestEmissionIsTreeIndependent). The meaningful proof is the before/afterrouting observation above, which flips with this one-line change and only with
it.
One inherent limit worth stating: these checks run through
codeowners_match,the repository's model of GitHub's last-match-wins semantics and the same model
CI relies on. GitHub's own evaluation of the emitted file cannot be exercised
from a test environment.
Where should the reviewer start?
Start with the single changed line in
.github/codeowners/areas.yaml— theglob:value of theplanner+operatorshared entry — and confirm theowners:list beneath it is untouched.The
CODEOWNERSdiff is generated output; the only thing to check there is thatit was regenerated rather than hand-edited. The rule moving to a later line is
the tell that it was.
Related Issues
🔗 This PR is linked to an issue:
Summary by CodeRabbit