Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
477 changes: 71 additions & 406 deletions README.md

Large diffs are not rendered by default.

80 changes: 79 additions & 1 deletion docs/BEHAVIOR.md
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,12 @@ a bare line deletion.

> ---------- R-36e: every command validates the whole file ----------

**`prerelease_bugs_test.go`**

> Pre-release findings: each test in this file states behavior the tool SHOULD
> have and currently does not. They are expected to FAIL until the bug they
> document is fixed; fix the bug, and the test becomes its regression guard.

**`rollout_test.go`**

> The rollout scenarios.
Expand Down Expand Up @@ -1110,6 +1116,78 @@ repos with a mistyped `--out` directory that is 100 real edits reported as
The record on stdout is the durable trace and goes first, unconditionally; a
sink that cannot be written is a warning on stderr and nothing more.

### `TestKnownBug_AuditJSONCleanIsPureJSON`

KNOWN BUG: `audit --format json` on a clean repo prints the literal line
"audit clean" after the JSON object, so the one case CI most wants to pipe
to jq — the healthy repo — is the one case the output isn't parseable.
Under `--format json`, stdout is data.

### `TestKnownBug_AuditRejectsUnknownFormat`

KNOWN BUG: `audit` silently accepts an unknown `--format` and falls back to
text. sync, check, and lint all reject unknown formats at exit 3 — "never a
silent fallback to text" — and audit is documented with the same
`--format json|text` contract.

### `TestKnownBug_BranchMismatchErrorNamesHeadCleanly`

KNOWN BUG: the S-7 branch-mismatch refusal interpolates raw `git rev-parse
--abbrev-ref --end-of-options HEAD` output, and rev-parse echoes the
`--end-of-options` operator as an output line — so the one-line error (and
the JSON record's error field) reads "HEAD is --end-of-options\nmain (…)".

### `TestKnownBug_EscapedHashPatternRefused`

KNOWN BUG: a `\#`-escaped pattern is accepted and written, but S-6/S-2 says
GitHub honors no `\#` escape — on GitHub the written line is dead, so the
tool reports `proven: tree` for a rule that provably does not hold there.
The unescaped spelling `add_owner(#tag.md, …)` is already refused; the
escaped spelling must be refused too, and nothing written.

### `TestKnownBug_FileFlagSpellingNoFalseGovernsNothing`

KNOWN BUG: `--file ./.github/CODEOWNERS` (or any uncleaned spelling of a
governing location) triggers a false "governs nothing" warning. trackedAt
cleans the spelling before matching the tracked file; the S-8 location
check compares the raw string, so a live change is reported as dead in the
warning, the --out record, and the --summary-out PR body.

### `TestKnownBug_PlanBelowRepoRootRefused`

KNOWN BUG: plan and apply skip the repo-root guard that sync enforces.
sync refuses `--repo <subdir>` because the CODEOWNERS it would write lands
at a path GitHub never reads (checkRepoRoot). plan happily plans against the
subtree and apply writes the dead file, reporting success — the "applied,
dead on arrival" outcome the guard exists to prevent. plan must refuse
exactly as sync does.

### `TestKnownBug_PositionalArgsRejected`

KNOWN BUG: positional arguments are silently discarded, and every flag
after them with them. `audit ../other-repo --checks a999` (note the missing
--repo) audits the CWD with all defaults and exits 0 — the invalid
`--checks a999`, which the parser would reject loudly, is never seen. A
tool this strict about flag values must not swallow whole arguments.

### `TestKnownBug_SetOwnersDisclosesAuthoredDuplicate`

KNOWN BUG: set_owners on a scope whose pattern already exists earlier in
the file authors a shadowed duplicate — the old line stays, dead under
last-match-wins but still naming its owners to human readers — and the run
that creates it says nothing. The R-7 duplicate warning fires only on the
NEXT run that touches the file. The run creating the duplicate must
disclose it.

### `TestKnownBug_SymlinkedCodeownersNotSilentSuccess`

KNOWN BUG: a symlinked .github/CODEOWNERS inside the clone is written
through and reported applied with no warning. The tool's own docs state
GitHub does not follow a symlinked CODEOWNERS, so the run edited a file
that governs nothing while reporting success. An out-of-repo symlink target
is already refused (containedWritePath); the in-repo case must at minimum
not be a silent success.

### `TestLintVerb_CaseOnlyMissIsSparedNotDeleted`

A rule that misses ONLY because of case is spared by --remove-stale-paths.
Expand Down Expand Up @@ -5864,4 +5942,4 @@ DIFFERENT states; transitioning between them is a real ownership change.

---

559 documented test cases across 13 packages.
568 documented test cases across 13 packages.
193 changes: 193 additions & 0 deletions docs/GUIDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
# Guide: making changes end to end

Worked examples for each kind of change. Concepts are in the
[README](../README.md#basic-concepts); every flag and exit code is in
[REFERENCE.md](REFERENCE.md).

## A basic example

A small repo, with a `README.md` nobody owns in particular:

```
README.md
docs/guide.md
services/api/main.go
services/web/app.ts
```

```console
$ cat .github/CODEOWNERS
* @org/everyone
/services/api/ @org/api-team
```

Docs team should co-own the README. Look before you leap:

```console
$ codeowners-tool sync --op 'add_owner(README.md, @org/docs-team)' --dry-run
applied: 1 op(s) applied, 0 skipped; 1 line change(s), 1 path(s) change owners
ops[0] applied (proven: tree)
```

`proven: tree` means the claim was checked against the repo's real files, not just
reasoned about. Drop `--dry-run` to write it:

```console
$ codeowners-tool sync --op 'add_owner(README.md, @org/docs-team)'
applied: 1 op(s) applied, 0 skipped; 1 line change(s), 1 path(s) change owners
ops[0] applied (proven: tree)
$ cat .github/CODEOWNERS
* @org/everyone
README.md @org/everyone @org/docs-team
/services/api/ @org/api-team
```

Three things happened that are worth noticing:

- `@org/everyone` was **carried onto the new line** — they owned `README.md` via `*`,
and `add_owner` means co-own, so the new rule restates them or they'd be dropped.
- The line went **in the middle, not at the end** — directly after the rule it narrows,
which is what keeps out-of-scope ownership (INV-2) untouched.
- `/services/api/` was **not touched at all**, including its original spacing.

Run it again and nothing happens: the second run reports `unchanged` and changes
zero bytes.

> **Pattern note.** `README.md` is unanchored, so like gitignore it matches a
> `README.md` at *any* depth. Write `/README.md` if you mean only the one at the root.

## Writing a new CODEOWNERS file

For a repo with no CODEOWNERS at all, `--create` grants permission to write one at
`.github/CODEOWNERS`. It never overwrites an existing file, it's off by default, and a
run with nothing to write creates nothing. The smallest version is one op:

```console
$ codeowners-tool sync --op 'set_owners(*, [@org/everyone])' --create
applied: 1 op(s) applied, 0 skipped; 1 line change(s), 4 path(s) change owners
ops[0] applied (proven: tree)
created a new CODEOWNERS file
```

For a real file, put the ops in a policy so the whole shape is reviewable in one diff
(a policy run states `"create": true` in the file instead of the flag —
[why](REFERENCE.md#creating-a-file-r-23-and-not-creating-one)):

```json
{
"version": 1,
"name": "bootstrap ownership",
"create": true,
"ops": [
"add_owner(*, @org/everyone)",
"add_owner(/services/api/, @org/api-team)",
"add_owner(/docs/, @org/docs-team)",
{ "op": "add_owner(/.github/workflows/, @org/ci)", "on_zero_match": "declare" }
]
}
```

```console
$ codeowners-tool check --policy bootstrap.json
ok: bootstrap.json — 4 op(s), no policy errors
$ codeowners-tool sync --policy bootstrap.json
applied: 4 op(s) applied, 0 skipped; 4 line change(s), 4 path(s) change owners
created a new CODEOWNERS file
```

Two things that will bite you on the first try:

- **Use `add_owner` for the catch-all, not `set_owners`.** `add_owner` ops commute, so
any number can share one run. `set_owners(*, …)` overlaps every other scope and does
not commute with them, so the batch is refused at exit 3 (R-8) — run it on its own
first, previewed with `--dry-run`, since it *replaces* the owners of everything.
- **A rule for files that don't exist yet needs `on_zero_match: "declare"`.** The
default `require` treats a scope matching nothing as a problem with this repo,
because it usually is a typo. `declare` writes the rule at the end of the file for
files added later, and reports `proven: structural` — see
[what `declare` costs](REFERENCE.md#what-declare-costs).

## Modifying an existing file

Same command; the interesting part is what it protects you from. Starting from the
two-line file above, each row is one run, and the second column is the line it leaves
behind — original spacing intact:

| Run | `/services/api/` afterwards |
|---|---|
| `sync --op 'add_owner(/services/api/, @org/platform)'` | `/services/api/ @org/api-team @org/platform` |
| `sync --op 'set_owners(/services/api/, [@org/platform, @org/api-team])'` | `/services/api/ @org/platform @org/api-team` |
| `sync --op 'rename_owner(@org/api-team, @org/platform-api)'` | `/services/api/ @org/platform-api` |

The first is the common case and the one hand-editing gets wrong. The second is the
same edit stated deliberately. The third is what a reorg needs — a global identifier
substitution that can't change any rule's match set.

**Removing an owner** stops and asks when it would empty a rule's owner set — there is
deliberately no default:

```console
$ codeowners-tool sync --op 'remove_owner(/services/api/, @org/api-team)'
error: removing @org/api-team empties the owner set of "/services/api/"; an explicit --on-empty policy (error|inherit|unowned) is required — there is deliberately no default (R-6) (governing file: .github/CODEOWNERS)
$ echo $?
2
```

`--on-empty inherit` deletes the rule and lets the preceding broader one take over;
`unowned` keeps the pattern with zero owners (GitHub's sanctioned substitute for `!`
negation); `error` refuses outright, and is the recommendation.

## Reviewing the change before it lands

`sync` is plan-assert-apply-validate in one step. Split it when you want the artifact
in the middle — a JSON plan with resolved ownership per path and the literal line diff:

```console
$ codeowners-tool plan --op 'add_owner(/services/web/, @org/web-team)' --out plan.json
plan written to plan.json
1 line change(s), 1 path(s) change owners, 58 → 101 bytes
$ jq '.ownership_rows, .diff' plan.json
[
{
"path": "services/web/app.ts",
"owners_before": ["@org/everyone"],
"owners_after": ["@org/everyone", "@org/web-team"]
}
]
"@ line 2\n+/services/web/ @org/everyone @org/web-team\n"
$ codeowners-tool apply --plan plan.json
applied: .github/CODEOWNERS (58 → 101 bytes)
```

Every change carries the reason it took that shape — the part a reviewer actually
wants. And to prove after the fact that a merged change moved nothing it didn't
declare:

```sh
codeowners-tool snapshot --branch main --out before.json
codeowners-tool snapshot --branch feature --out after.json
codeowners-tool verify --before before.json --after after.json --scope /services/api/
```

## When it refuses

Sometimes there is no line that does what you asked and nothing else. Given
`infra/main.tf` and `infra/README.md`, and a CODEOWNERS of exactly `infra/
@org/infra-legacy`:

```console
$ codeowners-tool sync --op 'add_owner(**/*.tf, @org/infra)'
error: refusing: rule "infra/" also governs paths outside scope "**/*.tf", and no sound narrowing pattern is derivable — amending would violate INV-2, appending would violate INV-1 (governing file: .github/CODEOWNERS)
$ echo $?
2
```

In English: `infra/` covers the `.tf` file *and* the README. Editing that line would
change the README's owners, which you never asked for (INV-2). Adding a `**/*.tf` line
before it would be overridden by it (INV-1). So it stops.

This is a normal outcome for some repos, not a bug — the tool fails closed rather than
guessing. The fix is usually to replace the over-broad rule the error names with
narrower ones, then re-run. Exit `2` means *this repo* needs a human; exit `3` means
*the policy* is broken and will fail identically everywhere — the split that makes a
hundred-repo run survivable ([FLEET.md](FLEET.md)).
4 changes: 2 additions & 2 deletions docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ audit [--checks a1,a3,a6] [--fail-on any|warning|error|never] [--format json|
[--cache-dir D] [--cache-ttl DUR] [--repo DIR] [--branch REF] [--file PATH]
lint --github-repo owner/name [--token T | $GITHUB_TOKEN] [--api-url URL]
[--remove-stale-paths] [--on-empty error|inherit|unowned] [--dry-run]
[--repo DIR] [--branch REF] [--file PATH] [--format text|json]
[--policy FILE] [--repo DIR] [--branch REF] [--file PATH] [--format text|json]
snapshot [--repo DIR] [--branch REF] [--out snap.json]
verify --before before.json --after after.json [--scope PATTERN ...]
version print the build this binary was stamped with
Expand Down Expand Up @@ -405,7 +405,7 @@ Under `inherit`/`unowned` the resulting reassignment is shown in the plan's owne

## Audit checks

Read-only **except `audit --lint`** ([below](#audit---lint)). Plain `audit` never writes —
Read-only **except `audit --lint`** ([below](#lint)). Plain `audit` never writes —
where a fix is expressible it emits op strings for a human to review and run through
`plan`/`apply`. Even under `--lint` the bytes reach disk only through `apply`, which
remains the system's single writer path.
Expand Down
Loading
Loading