fix: verify no longer fails on added files; gate docs regeneration in CI

.github/workflows/ci.yml

@@ -31,6 +31,12 @@ jobs:
       - name: Test
         run: go test -race ./...
 
+      # docs/BEHAVIOR.md is generated from the test suite's doc comments, so
+      # it can only stay true if every change regenerates it. Adding, renaming
+      # or re-documenting a test without running `make docs` fails here.
+      - name: Docs up to date
+        run: make docs-check
+
       - name: Build
         run: make build
 

Makefile

@@ -1,4 +1,4 @@
-.PHONY: build test docs diff-test vet all
+.PHONY: build test docs docs-check diff-test vet all
 
 all: vet test build docs
 
@@ -16,6 +16,13 @@ vet:
 docs:
 	go run ./tools/gendocs
 
+# CI gate: the committed docs must be exactly what the generator produces.
+# gendocs is deterministic, so a diff here means a test doc comment changed
+# (or a test was added/renamed) without `make docs` being run.
+docs-check: docs
+	@git diff --exit-code -- docs/BEHAVIOR.md \
+	  || { echo "docs/BEHAVIOR.md is stale — run 'make docs' and commit the result."; exit 1; }
+
 # Differential fuzz of the pattern matcher against the vendored, unmodified
 # hmarr/codeowners oracle (500k cases).
 diff-test:

README.md

@@ -50,7 +50,7 @@ arm64.
 > `xattr -d com.apple.quarantine ./codeowners-tool`. Homebrew and the install
 > script above are unaffected — neither quarantines its downloads.
 
-**From source** with Go 1.24+:
+**From source** with Go 1.24.7+ (the version pinned in `go.mod`):
 
 ```sh
 go install github.com/jordonpeterson/codeowners-tool/cmd/codeowners-tool@latest
@@ -72,9 +72,11 @@ codeowners-tool apply --plan plan.json
 ```
 
 `plan` re-resolves ownership across your real git tree and refuses anything it
-can't prove; `apply` writes only the proven edit and is idempotent, so
-re-running the same op is a no-op. To audit a repo for rot or to verify in CI
-that a change stayed inside its declared scope, read on.
+can't prove; `apply` writes only the proven edit. Both are idempotent:
+re-planning the same op against an already-satisfied file changes nothing and
+**exits 1** (`nothing to change`) rather than 0, so a CI step that runs `plan`
+unconditionally should treat 1 as success. To audit a repo for rot or to
+verify in CI that a change stayed inside its declared scope, read on.
 
 ## Quick start
 
@@ -95,6 +97,18 @@ GITHUB_TOKEN=... ./codeowners-tool audit --github-repo org/repo --format json
 ./codeowners-tool verify --before before.json --after after.json --scope /services/api/
 ```
 
+`verify` compares *resolved ownership per path*, independently of the planner —
+CI can check the invariant without trusting the tool that made the change.
+Two things to know about the recipe above:
+
+- **Omitting `--scope` asserts that nothing changed at all.** Scopes are the
+  allowlist; with none, every difference is a violation.
+- **Files the branch adds or deletes are reported, never violations.** The two
+  snapshots come from different refs, so their trees differ. An added path had
+  no prior ownership for INV-2 to preserve, so it prints as `added:` and does
+  not fail the check. A real reassignment still fails it — the subtree's
+  pre-existing files change.
+
 ## Operations (Engine A — mutation)
 
 Scope is a directory, file path, or glob — same syntax as CODEOWNERS patterns.
@@ -106,6 +120,80 @@ Scope is a directory, file path, or glob — same syntax as CODEOWNERS patterns.
 | `remove_owner(scope, owner)` | Owner stops owning every path in scope. If a rule's owner set would empty, `--on-empty` is **required** (see below). |
 | `rename_owner(old, new)` | Global identifier substitution — the only op safe as pure text replacement (it can't change any rule's match set). |
 
+An owner is `@org/team`, `@username`, or an email address. The `@` is
+required — a bare `team_a` is rejected with
+`error: invalid owner token "team_a"`. Both `@org/team_a` and `@team_a` parse,
+but they mean different things to GitHub (a team vs a user account) and the
+tool cannot tell you which you meant.
+
+### Adding an owner: co-owner, or sole owner?
+
+The most common task, and the one where hand-editing goes wrong. Given:
+
+```
+/services/ @org/platform @org/sre
+```
+
+**Add `@org/team_a`, keeping the existing owners:**
+
+```sh
+codeowners-tool plan --op 'add_owner(/services/, @org/team_a)' --out plan.json
+```
+
+```
+-/services/ @org/platform @org/sre
++/services/ @org/platform @org/sre @org/team_a
+
+services/api/main.go : [@org/platform, @org/sre] → [@org/platform, @org/sre, @org/team_a]
+```
+
+**Make `@org/team_a` the sole owner, displacing the others:**
+
+```sh
+codeowners-tool plan --op 'set_owners(/services/, [@org/team_a])' --out plan.json
+```
+
+```
+-/services/ @org/platform @org/sre
++/services/ @org/team_a
+
+services/api/main.go : [@org/platform, @org/sre] → [@org/team_a]
+```
+
+Review `plan.json`, then `codeowners-tool apply --plan plan.json`.
+
+**Why not just append a line?** Writing `/services/ @org/team_a` at the end of
+the file yourself produces the *second* result when you probably wanted the
+first. Last match wins and owner sets do not union (S-1), so an appended rule
+replaces the owners of everything it matches. `add_owner` re-resolves every
+tracked path and proves the prior owners survived; that check is the point of
+the tool.
+
+**Scope is per-path, not "the whole file."** For a repo-wide owner, `*` works,
+but look at what it has to do:
+
+```sh
+codeowners-tool plan --op 'add_owner(*, @org/team_a)' --out plan.json
+```
+
+```
+-/web/ @org/frontend
++/web/ @org/frontend @org/team_a
+-/services/ @org/platform @org/sre
++/services/ @org/platform @org/sre @org/team_a
++* @org/team_a
+
+.github/CODEOWNERS   : (unowned) → [@org/team_a]
+services/api/main.go : [@org/platform, @org/sre] → [@org/platform, @org/sre, @org/team_a]
+web/app.js           : [@org/frontend] → [@org/frontend, @org/team_a]
+```
+
+The catch-all alone would not be enough: every later rule shadows it, so
+`@org/team_a` would own nothing under `services/` or `web/`. The planner has
+to amend each one. A repo-wide op therefore touches every rule in the file and
+can pull previously unowned paths (here the CODEOWNERS file itself) into
+ownership — read the `ownership_rows` before applying it.
+
 ### The two invariants
 
 - **INV-1 (in scope):** after apply, every in-scope path resolves to exactly
@@ -119,6 +207,37 @@ independently computed desired state. Anything unprovable → exit 2, nothing
 written. Plans are idempotent (re-running is a no-op) and preserve every
 untouched byte — comments, blank lines, spacing, ordering.
 
+### How the planner edits lines
+
+You express intent over paths; the planner decides which lines move. It never
+reorders or reformats, but it does **add lines you did not write**. When an
+existing rule governs your scope *and* paths outside it, amending that rule in
+place would violate INV-2 — so the planner inserts a **narrowing rule**
+immediately after it. That is why the getting-started example above produces a
+line nobody typed:
+
+```
+/services/ @org/platform
+/services/api/ @org/platform @org/team-1   ← synthesized: narrows /services/
+/web/ @org/frontend
+```
+
+Because the last matching rule wins (S-1), this leaves everything outside
+`/services/api/` resolving exactly as before. Two consequences worth knowing:
+
+- **Refusal when no narrowing is expressible.** For some scope/rule
+  combinations no CODEOWNERS pattern describes exactly the intersection.
+  Amending would break INV-2 and appending would break INV-1, so the plan is
+  refused (exit 2) rather than guessed at. Narrow the scope, or restructure the
+  offending rule by hand.
+- **The inexact-narrowing warning.** A synthesized glob can be exact for every
+  file tracked today yet not provably confined for files added later. The plan
+  still applies, and prints a warning naming the pattern — read it, because it
+  is telling you a future file could land on the wrong side.
+
+Every synthesized line carries a `reason` in the plan JSON explaining why it
+exists.
+
 ### `--on-empty` (R-6)
 
 Removing the sole owner of a rule needs an explicit policy — **there is no
@@ -163,6 +282,70 @@ Removing a sole owner is presented as a **reassignment** with before → after
 owners per path, never a bare line deletion (R-14). Lookups are cached in
 memory per run and optionally on disk (`--cache-dir`, `--cache-ttl`).
 
+## CLI reference
+
+Five commands. `plan` and `apply` are the only writer path; `audit`,
+`snapshot`, and `verify` are read-only.
+
+**Common to `plan`, `audit`, `snapshot`:**
+
+| Flag | Default | Meaning |
+|---|---|---|
+| `--repo DIR` | `.` | Path to the local git repository |
+| `--branch REF` | `HEAD` | Ref whose tracked tree governs resolution (S-7) |
+| `--file PATH` | discovery | Repo-relative CODEOWNERS override, bypassing `.github/` > root > `docs/` precedence. The escape hatch when A-10 reports more than one file |
+
+**`plan`** — resolve, synthesize, prove. Writes a plan, never the file.
+
+| Flag | Default | Meaning |
+|---|---|---|
+| `--op 'kind(args)'` | required | Operation; repeatable for a batch |
+| `--on-empty POLICY` | none | `error`\|`inherit`\|`unowned`; required only when a removal would empty an owner set |
+| `--out FILE` | stdout | Where to write the plan JSON |
+| `--max-size N` | `3000000` | Hard byte cap; over it, refuse (S-4) |
+| `--warn-size N` | `2500000` | Byte threshold that emits a warning (R-9) |
+
+**`apply`** — write the proven edit, then validate.
+
+| Flag | Default | Meaning |
+|---|---|---|
+| `--plan FILE` | required | Plan JSON from `plan` |
+| `--repo DIR` | plan's repo | Override the repository recorded in the plan |
+
+The plan pins the input file's SHA-256; if the file changed since planning,
+apply refuses rather than clobber the other edit. A write that introduces new
+syntax errors is rolled back (exit 6).
+
+**`audit`** — find rot. Never writes.
+
+| Flag | Default | Meaning |
+|---|---|---|
+| `--checks LIST` | all | Comma-separated subset, e.g. `a1,a3,a6`. `a1`, `A1`, `a-1`, `A-1` all parse; an unknown name is a hard error, so a typo can't make an audit pass vacuously |
+| `--format FMT` | `text` | `text` or `json` |
+| `--github-repo owner/name` | none | Required, with a token, for the API checks A-1…A-3 |
+| `--token T` | `$GITHUB_TOKEN` | GitHub PAT |
+| `--api-url URL` | `https://api.github.com` | API base, for GHES |
+| `--cache-dir D` | memory only | Persist API lookups to disk (R-15) |
+| `--cache-ttl DUR` | `24h` | Disk cache lifetime |
+
+Without a token and `--github-repo`, audit runs the offline checks (A-4…A-12)
+and says so. If you *explicitly* request A-1/A-2/A-3 via `--checks` and they
+can't run, that is exit 5 — inconclusive, not a silent skip.
+
+**`snapshot`** — write resolved ownership for every tracked path at a ref.
+
+| Flag | Default | Meaning |
+|---|---|---|
+| `--out FILE` | stdout | Where to write the snapshot JSON |
+
+**`verify`** — compare two snapshots.
+
+| Flag | Default | Meaning |
+|---|---|---|
+| `--before FILE` | required | Baseline snapshot |
+| `--after FILE` | required | Snapshot to check |
+| `--scope PATTERN` | none | Where change is allowed; repeatable. **With none, any change is a violation** |
+
 ## Exit codes
 
 | Code | Meaning |

docs/BEHAVIOR.md

@@ -928,6 +928,11 @@ SPEC S-9: GitHub's official example — a zero-owner rule un-owns a subtree.
 > be checked in CI from two ownership snapshots WITHOUT trusting the tool
 > that produced the change.
 
+### `TestR18_AddedFileDoesNotMaskReassignment`
+
+The gate still bites: an added file does not launder a real reassignment of
+the subtree it lands in, because that subtree's pre-existing files change.
+
 ### `TestR18_NoScope_AssertNoChange`
 
 SPEC R-18 + INV-2: with no scope given, verify asserts NOTHING changed.
@@ -941,9 +946,14 @@ Order of owners on a line is presentation: {"@a","@b"} == {"@b","@a"}.
 With scopes, changes inside scope pass; any out-of-scope change fails —
 this is INV-2 checked from raw data.
 
-### `TestR18_TreeChangesSurface`
+### `TestR18_TreeChangesSurfaceButDoNotViolate`
 
-A path added or removed from the tree is reported, not silently ignored.
+A path added or removed from the tree is reported, not silently ignored —
+but it is NOT an invariant violation. INV-2 preserves what a path resolved
+to BEFORE; an added path has no before, a removed path has no after. The
+two snapshots come from different refs, so counting a tree delta as a
+violation made the documented CI recipe fail on any PR that added a file
+outside the declared scope, with CODEOWNERS byte-identical.
 
 ### `TestR18_UnownedVsZeroOwnersDistinct`
 
@@ -952,4 +962,4 @@ DIFFERENT states; transitioning between them is a real ownership change.
 
 ---
 
-144 documented test cases across 11 packages.
+145 documented test cases across 11 packages.

internal/cli/cli.go

@@ -84,12 +84,14 @@ func usage(w io.Writer) {
 
   plan     --op 'add_owner(/services/api, @org/team-1)' [--op ...] [--on-empty error|inherit|unowned]
            [--repo DIR] [--branch REF] [--file PATH] [--out plan.json]
+           [--max-size N] [--warn-size N]
   apply    --plan plan.json [--repo DIR]
   audit    [--checks a1,a3,a6] [--format json|text] [--github-repo owner/name]
            [--token T | $GITHUB_TOKEN] [--api-url URL] [--cache-dir D] [--cache-ttl DUR]
-           [--repo DIR] [--branch REF]
-  snapshot [--repo DIR] [--branch REF] [--out snap.json]
+           [--repo DIR] [--branch REF] [--file PATH]
+  snapshot [--repo DIR] [--branch REF] [--file PATH] [--out snap.json]
   verify   --before before.json --after after.json [--scope PATTERN ...]
+           (no --scope means: assert NOTHING changed)
 
 Exit codes: 0 ok · 1 no-op · 2 refused (invariant/size) · 3 invalid input
             4 audit findings · 5 inconclusive (fail-closed) · 6 rolled back
@@ -290,14 +292,23 @@ func cmdVerify(args []string, stdout, stderr io.Writer) int {
 	for _, c := range res.Changed {
 		fmt.Fprintf(stdout, "changed: %s  %s → %s\n", c.Path, fmtOwners(c.Before), fmtOwners(c.After))
 	}
+	// Tree deltas are informational: the refs differ, so files come and go.
+	// They are never violations (see verify.Compare).
+	for _, t := range res.Added {
+		fmt.Fprintf(stdout, "added:   %s  %s\n", t.Path, fmtOwners(t.Owners))
+	}
+	for _, t := range res.Removed {
+		fmt.Fprintf(stdout, "removed: %s  %s\n", t.Path, fmtOwners(t.Owners))
+	}
 	if !res.OK() {
 		fmt.Fprintf(stderr, "INVARIANT VIOLATED: %d path(s) changed outside the declared scope\n", len(res.Violations))
 		for _, v := range res.Violations {
 			fmt.Fprintf(stderr, "  %s  %s → %s\n", v.Path, fmtOwners(v.Before), fmtOwners(v.After))
 		}
 		return ExitRefused
 	}
-	fmt.Fprintf(stdout, "ok: %d change(s), all within scope\n", len(res.Changed))
+	fmt.Fprintf(stdout, "ok: %d change(s), all within scope (%d path(s) added, %d removed)\n",
+		len(res.Changed), len(res.Added), len(res.Removed))
 	return ExitOK
 }