.stamphog/ was a config directory at the repo root — policy.yml, review-guidance.md — read by the PR review agent. It is .stamp/ now, and the 34 files that name it follow: the workflows, tools/pr-approval-agent, tach.toml, the sandbox Dockerfile, bin/debrand. One behaviour change worth naming rather than burying: the agent finds its own previous PR comment by an HTML marker, `<!-- stamphog:v1 ... -->`, and that marker is now `<!-- stamp:v1 ... -->`. Open PRs will get one new comment instead of an edit, once, and then match again. insights/test/setup_receivers_baseline.txt records registered signal receivers by module path, so it names products.review.backend now. stamphog: zero occurrences anywhere in the tree. review_hog: only inside migration files, whose names are their identity in django_migrations. Co-authored-by: Hanzo Dev <[email protected]>
26 KiB
Repo ownership model: distributed owners.yaml
Status: implemented (landed with the PR that introduced this document)
Design a single ownership source of truth that multiple consumers (review automation, CI validation, lookup CLIs, service catalogs, future tools) can read, instead of each tool re-parsing CODEOWNERS-soft and product.yaml on its own.
Scope decisions (locked):
owners.yamlbecomes the canonical ownership format..github/CODEOWNERS-softis deleted; the PR-assignment flow reads the new resolver instead.products/*/product.yamlstays and is an accepted alias ofowners.yaml— itsowners:key is a first-class ownership source, all other fields are ignored for ownership purposes.- The hard
.github/CODEOWNERSis left alone for now: hand-maintained, GitHub-enforced, out of the schema. - Everything lands in one PR.
1. Survey of prior art
| System | Placement | Match resolution | Roles | Inheritance control | Metadata |
|---|---|---|---|---|---|
| GitHub CODEOWNERS | one central file | last-match-wins (whole file) | none | none | none |
| GitLab CODEOWNERS | one central file | last-match-wins per section | sections + @@role refs |
section-scoped | none |
| Gerrit/Chromium OWNERS | per directory | ancestor union | none (Gerrit labels) | set noparent, per-file |
sibling DIR_METADATA (component, team contact) |
| Kubernetes/Prow OWNERS | per directory, YAML | ancestor union | approvers vs reviewers |
options.no_parent_owners |
labels, emeritus_approvers, root OWNERS_ALIASES |
| Backstage catalog-info.yaml | per component | explicit reference, no globs | typed spec.owner (Group) |
n/a | open-ended annotations |
Lessons that shaped this proposal:
- Central single-file systems rot via pattern-order bugs: last-match-wins makes line order load-bearing, and a broad glob added late silently swallows earlier specific rules. Our own
CODEOWNERS-softalready relies on deliberate ordering (the managed-reverse-proxy block must stay last). - Per-directory files match the mental model ("who owns this directory") and scale with a monorepo, but need a coverage audit tool since nothing forces a new directory to get an owner.
- The approver/reviewer split (Kubernetes) is the single most useful role distinction: "can gate a merge" vs "should see this PR" are different sets. We already have exactly this split — hard
CODEOWNERSvs soft — just encoded as two separate files with different formats and different consumers. - Chromium separates approval (OWNERS) from routing metadata (DIR_METADATA: team contact, bug component). Keeping "who approves" and "who to page/ping" in one schema but different fields avoids a second file.
- Kubernetes avoids GitHub Teams for grouping (no audit trail for membership) via a PR-reviewable
OWNERS_ALIASESfile. We do want GitHub team slugs (the auto-assigner needs them), and we deliberately skip the alias layer: individuals appear inline as@handles, matching whatproduct.yamlalready allows. - The compile-to-CODEOWNERS pattern (structured YAML as source, flat CODEOWNERS as build artifact) is common and keeps native platform integration for free.
2. What the repo has today
Two data sources, one shared matcher, four consumers — all hand-maintained.
Sources
.github/CODEOWNERS(58 lines): blocking. Deliberately tiny, infra/security-critical trees only ("extraordinary justification" header). Uses owner-less lines as resets to clear a blocking owner for a subtree (e.g.insights/insightsql/database/schema/**)..github/CODEOWNERS-soft(383 lines): non-blocking review tagging. Carries all product mapping for shared code outsideproducts/(~25 teams). Header already says: product-level ownership belongs inproduct.yaml; this file is for sub-folder overrides, secondary reviewers, and paths outsideproducts/.products/*/product.yaml(68 files): exactly two fields,nameandowners(list of team slugs, occasionally@individual). Owns all ofproducts/<name>/**and beats any CODEOWNERS entry in the resolution used by the ownership skill.
Consumers and their parsers
| Consumer | Reads | Parser |
|---|---|---|
.github/scripts/assign-reviewers.js (CI auto-assign) |
soft + product.yaml | vendored matcher + own YAML mini-parser |
insightscli product:lint:owners |
product.yaml | own loader (product_yaml.py), validates slugs against live GitHub teams |
.agents/skills/establishing-code-ownership/ownership.js |
hard + soft + product.yaml | vendored matcher + own YAML mini-parser |
tools/pr-approval-agent/gates.py |
soft only | fully independent reimplementation |
The vendored matcher is .github/scripts/codeowners.js (a port of hmarr/codeowners, faithful to GitHub semantics).
Gaps
- Four parsers of the same two files; only two share the matcher. Drift is a when, not an if.
- Only
product.yamlis CI-validated. Nothing checks thatCODEOWNERS-softglobs still match existing files or that its team slugs exist (the assigner discovers dead teams at runtime via 422s). - No schema for
product.yaml; no oncall/Slack/escalation routing anywhere in the repo (handbook only). - Consumers disagree about scope: the assigner and pr-approval-agent ignore the hard file; only the skill layers all three sources.
- No structured notion of generated/vendored/deprecated code — the assigner hardcodes an ignore list (
frontend/src/generated/**,*.ambr, lockfiles, …).
3. Proposed canonical format: owners.yaml
One schema, distributed per directory. Under products/, the existing product.yaml is the ownership file (see the alias rule below) — no new files needed there.
# owners.yaml — full form; every field except `owners` is optional
version: 1
# The one required field, same key and semantics as product.yaml.
# Ordered, mixed list of GitHub team slugs (minus @Insights/) and
# '@handles'; the first entry is the primary owner, used for defaults
# like the derived Slack channel (see the `teams:` registry below).
# A single owner can be written bare: `owners: team-error-tracking`.
# Owners are tagged for review (non-blocking) and are the answer to
# "who owns this path".
owners: [team-error-tracking, '@pauldambra']
# Lifecycle of the code under this directory.
# active (default) | deprecated | generated | vendored
status: active
# When true (default), fields not set here fall through to the nearest
# ancestor owners.yaml. false = Gerrit's `set noparent`.
inherit: true
# Per-path overrides inside this directory, evaluated last-match-wins
# *within this file only*. owners, status, and inherit can be overridden
# per rule. `match` is one glob or a list of globs (each is its own
# boundary — the list is just shorthand for repeating owners/status).
rules:
- match: 'generated/**'
status: generated
- match: 'vendor/**'
status: vendored
owners: null # explicit reset: unowned-by-design, lint-exempt
- match: ['migrations/**', 'legacy/**']
owners: [team-error-tracking, team-data-modeling]
The 90% case is two lines:
# rust/owners.yaml
version: 1
owners: team-ingestion
product.yaml as an accepted alias
products/<name>/product.yaml with an owners: key is read by the resolver as an owners.yaml with that same owners: list. Every other field in product.yaml (name: today, anything added later) is ignored for ownership purposes — product.yaml remains free to grow product metadata without touching the ownership schema. Rules:
- A directory may have
product.yaml-with-ownersorowners.yaml, never both — lint error. - Sub-folder overrides inside a product use nested
owners.yamlas anywhere else (e.g.products/x/backend/migrations/owners.yaml). - The
team-CHANGEMEscaffold placeholder resolves to unowned, as all consumers already treat it.
This keeps all 68 products at zero migration cost and preserves insightscli product:lint:owners unchanged.
Slack derivation: measured, not assumed
Checked every team slug in use against live Slack channels, re-verified 2026-07-15 against the 30 slugs left after the migration:
- 24/30:
#<slug>exists verbatim, so no entry is needed. - 6/30 need an entry in the root
teams:registry (below):datastore,conversations,batch-exports— these predate theteam-convention and are the real org slugs;team-datastoreand friends do not exist as GitHub teams, so the slugs cannot be "fixed" and the channel is#team-<slug>.team-data-stack→#group-data-stack.team-insights-code→#team-desktop, the name the channel was created under.logs→#team-apm: thelogsandapmGitHub teams have identical membership, APM ("Logs, Metrics and Traces") having absorbed logs.
So the derived default is right for 80% of teams and the rest carry one entry each.
Nothing validates that a derived channel actually exists — a wrong or dead channel fails silently.
An opt-in Slack-API check in owners:lint, mirroring the existing opt-in live GitHub-team validation, would close that gap; it is not built.
The teams: registry (repo-root only)
The channel-to-team mapping is declared once, at the root, never per file — a team that owns paths across many directories should not restate its channel in each.
The repo-root owners.yaml may carry a teams: registry — a mapping of team slug to a single slack value (a string starting with #, or false to mean "no channel, don't derive"):
# owners.yaml (repo root only)
teams:
datastore:
slack: '#team-datastore'
team-data-stack:
slack: '#group-data-stack'
some-retired-team:
slack: false
teams: in any non-root owners.yaml is a schema error, and product.yaml aliases never carry it.
There is no per-path or per-file Slack override — the registry (plus the derived default) is the only way a channel is set.
The effective Slack channel for a path is resolved by this precedence chain, first match wins:
- the registry entry for the primary owner (
owners[0]), but only when it is a team slug (not an@handle) — afalseentry suppresses derivation; - the derived
#<owners[0]>when the primary owner is a team slug; - otherwise
None.
The registry only changes how the resolver computes the channel; the wire format is unchanged — slack still flows through as the final string or null.
Individuals, no alias layer
There is no OWNERS_ALIASES file. owners: takes GitHub team slugs or @handles directly — the same convention product.yaml already uses for user_interviews. Lint validates handles against org membership the same way it validates team slugs. The auto-assigner requests individual reviewers through the API's reviewers field (it currently skips @-entries entirely, so individuals only work via CODEOWNERS-soft today — this closes that gap and lets the soft-file wiring die with the file).
Design choices, argued
- Distributed, not central. The repo already voted:
product.yamlis per-directory and the soft file's own header pushes ownership toward it. Central files are what we're escaping (ordering bugs, merge conflicts on one hot file). The audit story ("show me everything") is a tool's job (insightscli owners:map), not a file layout's. - Nearest-file-wins with per-field fallthrough, not ancestor union. Kubernetes unions approvers up the tree because its bar is "someone must approve"; our soft model is "tag the right team, don't spam five". Union inheritance would tag
insights/owners on every deep PR. Override semantics also match what the ownership skill already implements (product.yaml beats CODEOWNERS). Fallthrough is per field: a child file that only setsownersstill inheritsstatusfrom its ancestor. rules:are file-local. Cross-file glob interactions are the CODEOWNERS footgun; here a glob can only override its own directory's defaults, so reading one file plus its ancestors fully explains any path.owners: nullis explicit, not absent. Unowned must be a decision (vendored code, scratch dirs), never a default. The coverage check treats missing resolution as an error andowners: nullas an exemption with a paper trail.- One
ownerslist, no role split. Ateam-vs-reviewersdistinction (Kubernetes-style) was considered and dropped: with blocking out of scope, "who is responsible" and "who gets tagged" are the same set, and product.yaml already speaksowners. Ordering carries the only extra signal needed — first entry is the primary owner. A blocking role, if ever needed, is a new field later, not a split now. status:replaces hardcoded ignore lists. The assigner's generated-file ignore list, review-noise suppression, and future tooling (e.g. excluding vendored code from lint) all key off one field instead of N copies.
4. Resolution model
For a path P:
- Walk from the repo root toward
P, collecting everyowners.yaml(or aliasedproduct.yaml) on the way. If a file setsinherit: false, drop everything collected above it. - Effective config = shallow merge, nearest file winning per field (lists replace, never merge — predictability over cleverness).
- Within the nearest file that has
rules:, apply the last rule whosematchglob (gitignore-style semantics, documented with the schema) matchesPrelative to that file's directory. Rule fields override the merged config. - Review tagging = resolved
owners. Primary owner = its first entry. - No ownership file on the walk and no rule match → unowned, which fails the coverage check unless the path is under an
owners: nullrule.
Consolidation vs distribution is a readability choice, not a semantic one. A parent owners.yaml may own subtrees through anchored directory rules (- match: '/rust/capture/'), and a deeper owners.yaml still wins by nearest-file resolution. So a cluster of one-line child files and a single parent file with equivalent rules resolve identically; pick whichever reads better for that tree. owners:lint nudges toward folding when a directory accumulates enough single-purpose children.
The hard .github/CODEOWNERS stays outside this walk entirely. It keeps its own GitHub-native semantics and remains hand-maintained and GitHub-enforced; the resolver does not read it — blocking ownership is consulted manually (or via GitHub's own UI) when needed. It never influences the resolved owners, and nothing in this proposal writes to it.
5. Tool independence: one resolver, many consumers
The stability guarantee is architectural: consumers never parse ownership files themselves. One resolver library owns the semantics; everything else calls it.
- Resolver: single implementation in the installable
insights-ownerspackage attools/owners/exposingresolve(path),map(),unowned()as a library, andinsightscli owners:resolve --json <path...>(paths also accepted on stdin) as the CLI. Rationale: insightscli already lints ownership, and two of the four consumers are Python, so this makes lint, lookup, and the pr-approval agent native library callers with zero subprocess hops. Glob matching forrules:uses gitignore-style semantics implemented (and documented) here — the vendored JS matcher stays only for the hard-CODEOWNERS overlay parsing, or is replaced by an equivalent Python CODEOWNERS parser. - JS consumers shell out to the CLI and read JSON —
assign-reviewers.jsfeeds the PR's changed files in and gets resolved owners back; theestablishing-code-ownershipskill does the same. The auto-assign workflow gains a Python/uv setup step (it is node-only today).gates.pyimports the library directly. No committed lock file, so no freshness-check machinery; if one is ever wanted (offline consumers, Backstagecatalog-info.yamlemitters), it is a trivial fold overmap()added later. - Validator:
insightscli owners:lint— schema check, team slugs and@handlesagainst the live GitHub org (reusingproduct/gh.py), deadrules:globs (match zero files), same-directoryproduct.yaml/owners.yamlconflicts, reserved-location rejection (see below), and full-tree coverage (everygit ls-filespath resolves or isowners: null). It also prints advisory consolidation suggestions when it spots a cluster of single-purpose owners.yaml files a single parent could absorb (never affects the exit code).
Reserved locations. owners.yaml must not live in a directory whose own tooling globs every YAML file there — GitHub Actions/actionlint treat everything under .github/workflows/ as a workflow, and the services/mcp generate-tools step globs YAML configs under products/*/mcp/. Lint rejects owners.yaml in those spots; hoist the ownership into the parent's rules: instead (e.g. a /workflows/ rule in .github/owners.yaml).
- Lookup:
insightscli owners:who <path>/owners:team <slug>/owners:unowned— thin wrappers over the library; theestablishing-code-ownershipskill'sownership.jsbecomes a shim over the CLI (or is deleted in favor of it).
One tradeoff to acknowledge: today .github/scripts/ sits behind the blocking CODEOWNERS (team-security), so changes to assignment logic require their approval. Moving the resolver out of .github/scripts/ takes it out of that gate. Done — the resolver package (tools/owners/insights_owners/) is under the blocking file, a deliberate exception to "leave CODEOWNERS alone" since the auto-assigner executes it on pull_request_target.
If the first consumer (say the auto-assigner) is ever replaced, the resolver, schema, and lint are untouched — only one caller changes. That is the "source of truth, not tool config" property.
Portability: repo = data, app = resolver
Nothing in the resolver is Insights-specific — owners.yaml is a repo-agnostic format, and the library needs only pyyaml and a way to load files.
That is why it ships as the standalone, installable tools/owners package (insights-owners), which any repo can run without vendoring: uvx --from "git+https://github.com/Insights/insights#subdirectory=tools/owners" owners lint.
That last part is the seam: resolution walks an abstract file map, not the filesystem (the owners:fmt equivalence proof already runs the real resolver over an in-memory layout).
So when a consumer like stamp becomes a hosted app that other repos enable without adding any code, the model is: the repo contributes only ownership data; the resolver ships inside the app.
A hosted reviewer fetches the default-branch tree (one API call), pulls just the ownership files (a few dozen blobs, cacheable per commit SHA), and resolves in-process.
Repos without owners.yaml fall back to a CODEOWNERS loader behind the same resolver interface — the glob semantics here are CODEOWNERS semantics already — and a repo with neither is simply unowned.
Two properties carry over intact. Ownership is always read from the default branch, never the PR head, so a PR cannot rewrite ownership to approve itself. And ownership content stays in git even when enablement moves to a UI — a far-away ownership store is exactly what rots (§2); the UI configures which source and branch, never the data.
Canonical placement (owners:fmt)
Because consolidation is a readability choice and not a semantic one (§4), the tree has many layouts that resolve identically, and it drifts between them as people fold and split by hand.
owners:fmt names the drift: it models ownership as a piecewise-constant function on the directory tree, where each match/owners statement is one boundary (a change point) and the physical file that carries it is pure presentation.
Placing statements onto files optimally is a capacitated facility-location problem on the tree — open a file where a cluster of boundaries makes a dedicated file cheaper than carrying them all up as ancestor rules, otherwise fold them into the nearest existing carrier.
The cost model lives in module constants, so "canonical" is defined only relative to them: ALPHA (the price of a dedicated simple owners.yaml existing at all — pinned carriers like product.yaml manifests, non-simple files, glob files, and the repo root are free because they exist anyway), GAMMA (the per-level price of carrying a statement as an ancestor rule), and MAX_RULES (the split cap that forces a dedicated child file once a carrier gets too dense).
Glob rules are crosscutting rather than tree boundaries, so any file carrying one is pinned and passes through untouched.
Every run ends with a built-in equivalence proof: the proposed layout is simulated in memory and every tracked path re-resolved against it, which must match the current resolution exactly — a mismatch is a bug in fmt, not a suggestion.
owners:fmt is a read-only oracle — it never writes, and there is no --write flag.
The division of labor: owners:lint's fold/split suggestions are the everyday incremental mechanism (with hysteresis, so small clusters don't thrash), fmt is the drift oracle you consult deliberately, and an actual reflow of files is a human decision, not something a formatter should apply on save.
6. The one PR
Everything lands atomically. The delivery order below is a review guide, not a merge sequence.
- Resolver + schema.
tools/owners/(resolution per §4),insightscli owners:resolve --json, JSON-schema forowners.yaml. - Convert
CODEOWNERS-soft→ distributedowners.yaml. Mechanical translation of the 383 lines into per-directory files underinsights/,frontend/src/scenes/,nodejs/,rust/,services/,ee/, plugin-server, etc. Where the soft file relied on last-match ordering (e.g. the trailing managed-reverse-proxy block overriding a broad settings-scene rule), that intent becomes an explicit nested file orrules:entry — order-independence is the point. This was driven by a one-shot converter that ran during the PR and was removed once the conversion landed (it remains in the branch history). - Equivalence proof. A differ resolved every
git ls-filespath under (old: soft + product.yaml) and (new: owners.yaml + product.yaml) and asserted identical reviewer sets. It ran during the PR to verify the migration and was removed once the conversion landed; intentional divergences (there were a few — dead globs, stale teams the 422 fallback already skips) were listed explicitly in the PR description rather than slipping through. - Flip consumers.
assign-reviewers.js: replace soft-file parsing +loadProductYamlRuleswith a call toinsightscli owners:resolve --json(workflow gains a Python setup step). Substantive-owner thresholds, 5-team cap, comment/label behavior unchanged;@handleowners become individual review requests instead of being skipped.status: generatedreplaces the hardcoded ignore list.gates.py: replace the private CODEOWNERS-soft parser with a direct library import.ownership.js(skill): becomes a shim over the CLI; SKILL.md updated.
- Delete
.github/CODEOWNERS-soft. Also drop its entry from anything referencing it. - Lint wiring.
insightscli owners:lint+ a CI job (extend the existingvalidate-product-yamlsjob rather than adding a new one). Coverage starts warn-only; ratchet to fail in a follow-up onceowners:unownedis clean or explicitlyowners: null.
Safety properties of the atomic switch:
auto-assign-reviewers.ymlruns onpull_request_targetand always checks out master, so the new flow activates only at merge — no half-migrated window, and in-flight unrebased PRs are unaffected (the repo's CI-backwards-compat rule holds by construction).- The hard
CODEOWNERSand GitHub's native enforcement are untouched, so worst case on a resolver bug is wrong soft tagging, which is recoverable and non-blocking. - The equivalence differ is the review artifact: reviewers approve a proven-identical resolution plus an explicit list of intentional diffs, not 400 lines of glob translation on faith.
7. Open questions for maintainers
- Oncall routing: resolved — dropped from v1. The Slack channel costs nothing (derived from the team slug by convention, with the root
teams:registry overriding or settingslack: falsewhere needed), but nothing consumes an oncall reference, so it is not carried. Re-adding it is additive once a consumer exists. - Resolver ownership: resolved — the resolver package is covered by the hard
CODEOWNERS(see §5). - Coverage gating cadence: how soon after the PR to flip
owners:lintcoverage from warn to fail — immediately for new directories (ratchet), or only once the whole tree is clean? - Hard-CODEOWNERS future (explicitly out of scope now): if blocking gates ever move into the schema, approver inheritance should probably union up the tree rather than nearest-wins — parked until
team-securitywants to revisit.