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.
@@ -119,6 +133,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 +208,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
 }
 

internal/verify/verify.go

@@ -32,11 +32,23 @@ type Change struct {
 	After  []string `json:"owners_after"`
 }
 
-// Result of a comparison. Changed lists every difference; Violations lists
-// the subset outside the declared scopes (all of them, when no scope given).
+// TreePath is a path present in only one of the two snapshots — a file the
+// branch added or deleted. Reported for visibility, never a violation: see
+// Compare.
+type TreePath struct {
+	Path   string   `json:"path"`
+	Owners []string `json:"owners"`
+}
+
+// Result of a comparison. Changed lists every ownership difference among
+// paths common to both snapshots; Violations lists the subset outside the
+// declared scopes (all of them, when no scope given). Added and Removed
+// record tree differences and never affect OK.
 type Result struct {
-	Changed    []Change `json:"changed"`
-	Violations []Change `json:"violations"`
+	Changed    []Change   `json:"changed"`
+	Violations []Change   `json:"violations"`
+	Added      []TreePath `json:"added,omitempty"`
+	Removed    []TreePath `json:"removed,omitempty"`
 }
 
 // OK reports whether the invariant holds: no out-of-scope change.
@@ -63,6 +75,19 @@ func Load(path string) (*Snapshot, error) {
 // violation (INV-2 from raw data). With no scopes, ANY change is a violation.
 // A scope that fails to compile is a hard error — silently dropping it would
 // misreport which changes are in scope (found in review).
+//
+// Only paths present in BOTH snapshots can violate the invariant. INV-2 says
+// every out-of-scope path resolves to what it did before; a path the branch
+// added has no "before" to preserve, and one it deleted has no "after". The
+// two snapshots normally come from different refs, so treating a tree delta
+// as a violation failed every real PR that touched a file outside the
+// declared scope, with CODEOWNERS byte-identical. Such paths are reported in
+// Added/Removed instead — surfaced, never fatal.
+//
+// This does not weaken the gate: a CODEOWNERS edit that reassigns a subtree
+// still shows up on that subtree's pre-existing files. Only a scope whose
+// every file is new to the branch goes unchecked, and there the invariant has
+// nothing to say.
 func Compare(before, after *Snapshot, scopes []string) (*Result, error) {
 	var pats []*pattern.Pattern
 	for _, s := range scopes {
@@ -98,13 +123,19 @@ func Compare(before, after *Snapshot, scopes []string) (*Result, error) {
 	for _, p := range sorted {
 		b, bok := before.Ownership[p]
 		a, aok := after.Ownership[p]
-		if bok && aok && resolve.OwnersEqual(b, a) {
-			continue
-		}
-		c := Change{Path: p, Before: b, After: a}
-		res.Changed = append(res.Changed, c)
-		if len(pats) == 0 || !inScope(p) {
-			res.Violations = append(res.Violations, c)
+		switch {
+		case !bok:
+			res.Added = append(res.Added, TreePath{Path: p, Owners: a})
+		case !aok:
+			res.Removed = append(res.Removed, TreePath{Path: p, Owners: b})
+		case resolve.OwnersEqual(b, a):
+			// unchanged
+		default:
+			c := Change{Path: p, Before: b, After: a}
+			res.Changed = append(res.Changed, c)
+			if len(pats) == 0 || !inScope(p) {
+				res.Violations = append(res.Violations, c)
+			}
 		}
 	}
 	return res, nil

internal/verify/verify_test.go

@@ -77,12 +77,40 @@ func TestR18_UnownedVsZeroOwnersDistinct(t *testing.T) {
 	}
 }
 
-// A path added or removed from the tree is reported, not silently ignored.
-func TestR18_TreeChangesSurface(t *testing.T) {
-	before := snap(map[string][]string{"a.go": {"@a"}})
+// 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.
+func TestR18_TreeChangesSurfaceButDoNotViolate(t *testing.T) {
+	before := snap(map[string][]string{"a.go": {"@a"}, "gone.go": {"@a"}})
 	after := snap(map[string][]string{"a.go": {"@a"}, "new.go": {"@a"}})
 	res, _ := verify.Compare(before, after, nil)
+	if !res.OK() {
+		t.Errorf("tree-only delta must not violate the invariant: %+v", res.Violations)
+	}
+	if len(res.Changed) != 0 {
+		t.Errorf("tree delta is not an ownership change: %+v", res.Changed)
+	}
+	if len(res.Added) != 1 || res.Added[0].Path != "new.go" {
+		t.Errorf("added = %+v, want new.go", res.Added)
+	}
+	if len(res.Removed) != 1 || res.Removed[0].Path != "gone.go" {
+		t.Errorf("removed = %+v, want gone.go", res.Removed)
+	}
+}
+
+// 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.
+func TestR18_AddedFileDoesNotMaskReassignment(t *testing.T) {
+	before := snap(map[string][]string{"web/app.js": {"@fe"}})
+	after := snap(map[string][]string{"web/app.js": {"@other"}, "web/new.js": {"@other"}})
+	res, _ := verify.Compare(before, after, []string{"/services/"})
 	if res.OK() {
-		t.Error("new path must surface as a change")
+		t.Error("out-of-scope reassignment must still fail when the PR also adds files")
+	}
+	if len(res.Violations) != 1 || res.Violations[0].Path != "web/app.js" {
+		t.Errorf("violations = %+v", res.Violations)
 	}
 }