Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,29 @@ between classes is a major release and is called out explicitly either way.

## [Unreleased]

### Changed

- **`on_zero_match: declare` and `on_unowned: skip` now compose (R-40b).** The pair was
refused at exit 3 on the reasoning that a declared rule owns files that do not exist
yet, "which are unowned by definition" — conflating *unowned* with *nonexistent*. A file
that does not exist is in no op's path universe (scope is derived from the tree), and the
two settings are the two arms of one branch: `declare` acts where the scope matches no
tracked file, `skip` on tracked files that have no owner. They can never speak about the
same path, so together they state one intent across two repo shapes — pre-own what does
not exist, never close what is open today — which the refusal made unstatable.
`on_zero_match: declare` alongside `except` (R-30) is a different rule and still refused:
there the declared line really does capture future files under the excepted pattern.

**Exit-code change:** a policy carrying both settings on one op moves exit 3 → runs. Two
shapes that were already legal also change, because the `defaults` block no longer holds
either setting back from an op stating the other. An op with `on_unowned: skip` under
`defaults: {"on_zero_match": "declare"}` previously ran as `require` and **refused
(exit 2)** in a repo whose scope matched nothing; it now declares and **exits 0**. An op
with `on_zero_match: declare` under `defaults: {"on_unowned": "skip"}` previously granted
to open paths and now leaves them open — different bytes in repos that have them. A
policy that never states `on_unowned` is unaffected in every respect.


## [1.0.0] - 2026-08-27

Everything below accumulated across the `0.0` line; publishing it as `1.0.0` is
Expand Down
90 changes: 68 additions & 22 deletions docs/BEHAVIOR.md
Original file line number Diff line number Diff line change
Expand Up @@ -4084,6 +4084,16 @@ The control matters as much as the refusals: an ordinary email owner is
legal in the array (R-13), and an implementation that refused every email to
be safe would break the one owner form GitHub allows for individuals.

### `TestR40b_FleetPolicyAcrossThreeRepoShapes`

SPEC R-40b: the real fleet policy, run unchanged across the three repo
shapes it must survive. One reviewed op, `declare` + `skip`, states the
whole rule — pre-own what does not exist, never close what is open today —
and each shape gets the outcome that rule implies, all at exit 0.

This policy was exit 3 before the refusal lift: `check` halted the wave at
repo 0, so none of these outcomes was reachable.

### `TestRecord_OpRunOmitsThePolicyKey`

SPEC R-20/R-24: an `--op` run emits no `policy` key at all.
Expand Down Expand Up @@ -7565,6 +7575,18 @@ accepted today. The refusal must cite R-8; a refusal citing R-5 means
`declare` was never implemented and this test is passing for the wrong
reason.

### `TestR30_BuildRefusesDeclareWithExcept`

SPEC R-30 (defense in depth, review-bot finding): Build refuses declare
beside an except clause for a LIBRARY caller, as the policy validator does
for a policy file. Nothing here re-stated the rule before, so such an op
reached the declare branch, where synthDeclare writes the bare scope and
never consults the excepted set — the carve vanished silently and the
declared line governed exactly the paths it existed to protect.

Tree-independent, so the verdict is the same on every repo: a scope that
matches files and one that matches none both refuse.

### `TestR40_AllOpenScopeSkips`

SPEC R-40: a scope whose every tracked file is open skips — status
Expand Down Expand Up @@ -7687,6 +7709,19 @@ wider: a future .github/CODEOWNERX matches it, and so does a declared
ADirectoryPrefix — a disjointness claim that the tree happens to satisfy
and the pattern does not, which is a wrong write rather than a missed one.

### `TestR40b_DeclareAndSkipComposeAcrossRepoShapes`

SPEC R-40b: on_zero_match and on_unowned COMPOSE on one op, and the pair is
the user-facing rule in one line — pre-own what does not exist, never close
what is open today. They are the two arms of the same branch and range over
disjoint domains, so no repo can consult both: where declare fires the scope
matches no tracked file, and where skip fires it matches some.

This was refused at exit 3 on the reasoning that files which do not exist
are "unowned by definition" — but a nonexistent file is in no op's path
universe, which is derived from the tree, so the two never spoke about one
path. The refusal blocked precisely the policy the rule calls for.

### `TestRenameOwner_Global`

rename_owner replaces the identifier everywhere; it cannot change any
Expand Down Expand Up @@ -8314,19 +8349,6 @@ SPEC R-40: bad values are rejected at load with the legal set enumerated,
and a PRESENT-but-empty value is not the same as an absent one — "" states
no decision while reading to a reviewer as though a choice was made.

### `TestR40_ContradictoryDefaultsRejected`

SPEC R-40/R-35: a defaults block stating BOTH on_zero_match=declare and
on_unowned=skip is refused outright. For any op stating neither, the two
defaults contradict — and which one silently won would be a decision nobody
reviewed. The refusal is repo-independent: exit 3, caught by check.

### `TestR40_DefaultedDeclareDoesNotReachSkipOp`

SPEC R-40/R-35: a defaulted declare must not reach an op that explicitly
states on_unowned=skip — the pair is the contradiction the per-op check
refuses, and a default is applied only where the op can carry it (R-35e).

### `TestR40_DefaultsSupplyOnUnowned`

SPEC R-40/R-35: `defaults` carries on_unowned, so a 40-op baseline states
Expand Down Expand Up @@ -8357,14 +8379,6 @@ an open path anyway, set_owners displaces owners by design, and
rename_owner has no scope — accepting-and-ignoring the field on any of them
is the same class of failure as a typo'd field name.

### `TestR40_SkipRejectedBesideDeclare`

SPEC R-40/R-30: skip cannot ride beside on_zero_match=declare. A declared
rule exists to own files that do not exist yet — files that are by
definition unowned when they appear — so the pair states two opposite
intents about the same paths. Explicit "assign" beside declare is legal:
it spells the default.

### `TestR35e_DefaultDeclareSkipsTheOpsThatRefuseIt`

SPEC R-35e: a default reaches only the ops that can carry the VALUE, not
Expand Down Expand Up @@ -8448,6 +8462,38 @@ Observed before the fix, at exit 0 on a repo holding `my dir/x.txt` and
`dir/y.txt`: a carve line for `dir/` — a directory nobody named — while
`my dir/x.txt`, the path the carve existed for, stayed inside the grant.

### `TestR40b_DefaultedDeclareReachesSkipOp`

SPEC R-40b/R-35: a defaulted declare DOES reach an op that explicitly states
on_unowned=skip. The exclusion existed only because the pairing was illegal
— folding a default that produced a refusal would have failed a policy on a
combination its author never wrote. With the pair legal, R-35's plain rule
applies: the block fills in what the op did not state.

This is the one behavior change of the lift that touches a policy which was
already legal: before, the op ran under `require` and refused (exit 2) in a
repo whose scope matched nothing; now it declares and exits 0.

### `TestR40b_DefaultedSkipReachesDeclaredOp`

SPEC R-40b/R-35: the mirror — a defaulted skip reaches an op that explicitly
declares, for the same reason.

### `TestR40b_DefaultsMayStateBoth`

SPEC R-40b/R-35: a defaults block may state BOTH, and both reach every op
that can carry them. One reviewed line then states the whole fleet posture:
pre-own what does not exist, never close what is open today.

### `TestR40b_SkipComposesWithDeclare`

SPEC R-40b: skip rides beside on_zero_match=declare, and both land on the
op. The pair ranges over disjoint domains — declare acts where the scope
matches no tracked file, skip on tracked files with no owner — so it states
one intent across two repo shapes: pre-own what does not exist, never close
what is open today. It was refused on the reasoning that nonexistent files
are "unowned by definition"; they are in no op's path universe at all.

## internal/resolve

**`resolve_test.go`**
Expand Down Expand Up @@ -8606,4 +8652,4 @@ twice and one tracked file vanished from the gate.

---

816 documented test cases across 13 packages.
820 documented test cases across 13 packages.
21 changes: 19 additions & 2 deletions docs/OPERATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,25 @@ By default `add_owner` grants everywhere in scope, including paths no rule match
`on_unowned: skip` drops every path with no current owner from the op's effective scope —
a repo where only `build.gradle` is owned keeps everything else open for any reviewer,
instead of handing it to the new owner. Both unmatched paths and paths matched by a
zero-owner rule (S-9) count as open. `add_owner` only, policy form only, and never beside
`on_zero_match: declare` (a declared rule exists to own files that do not exist yet).
zero-owner rule (S-9) count as open. `add_owner` only, policy form only.

It composes with `on_zero_match`, `declare` included (R-40b), because the two answer
questions about disjoint sets of paths — `declare` where the scope matches **no** tracked
file, `skip` on the tracked files that have **no owner**. Together they state the rule in
one line: pre-own what does not exist, never close what is open today.

```json
{ "op": "add_owner(/.github/, @org/platform)", "on_zero_match": "declare", "on_unowned": "skip" }
```

| Repo | Outcome |
|---|---|
| no `/.github/` | rule declared for the future (`proven: structural`) |
| `/.github/`, some owned | grant lands on the owned paths only |
| `/.github/`, all open | `skipped`; no rule written, so nothing is closed |

The third row is a limit of the format, not a choice: CODEOWNERS has no negation (S-2), so
no single line covers future files in a directory without also capturing today's open ones.

The scope is decided against the repo's state before the batch, so a sibling grant in the
same run does not feed paths into a skipping op. If the restriction empties the effective
Expand Down
7 changes: 4 additions & 3 deletions docs/POLICY-FILE.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ always ignored and can hold one.

`on_zero_match` is rejected on `rename_owner` (its scope comes from current ownership, not
a pattern) and `declare` is rejected on `remove_owner` (there is no rule to write).
`on_unowned` is accepted only on `add_owner`, and `skip` never beside `declare` — a
declared rule exists to own files that do not exist yet, which are unowned by definition
(R-40).
`on_unowned` is accepted only on `add_owner`. It composes with every `on_zero_match`,
`declare` included: the two range over disjoint domains — `declare` acts where the scope
matches no tracked file, `skip` on tracked files with no owner — so the pair states one
intent across two repo shapes (R-40b).

Ops in one batch must **commute**. Two ops whose scopes overlap on a path and whose order
would change the outcome are refused rather than resolved by position (R-8):
Expand Down
9 changes: 3 additions & 6 deletions internal/cli/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -2019,12 +2019,9 @@ func resolvedUnowned(pol *policy.Policy, o ops.Op) (s resolvedSetting, stated bo
// treatment as a rename under on_zero_match (R-35e).
return resolvedSetting{Value: "n/a", Note: "only add_owner has an on_unowned"}, true
}
// The one add_owner the default does not reach: a declared op (R-40).
if o.OnZeroMatch == ops.ZeroMatchDeclare {
return resolvedSetting{Value: ops.UnownedAssign, Note: "built-in; the default does not reach a declared op"}, true
}
// Unreachable while the loader folds the default in, kept as the honest
// fallback for a caller that built the ops another way.
// Unreachable while the loader folds the default in — including onto a
// declared op, which the default now reaches (R-40b) — and kept as the
// honest fallback for a caller that built the ops another way.
return resolvedSetting{Value: ops.UnownedAssign, Note: "built-in"}, true
}

Expand Down
81 changes: 77 additions & 4 deletions internal/cli/unowned_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ func TestR40_CheckCatchesBadOnUnowned(t *testing.T) {
`{"version":1,"ops":[{"id":"s","op":"set_owners(/x/, [@a])","on_unowned":"skip"}]}`,
[]string{"on_unowned", "add_owner"},
},
"beside declare": {
`{"version":1,"ops":[{"id":"d","op":"add_owner(/x/, @a)","on_zero_match":"declare","on_unowned":"skip"}]}`,
[]string{"on_unowned", "declare"},
"declare with except (R-30 stands)": {
`{"version":1,"ops":[{"id":"d","op":"add_owner(/x/ except /x/gen/, @a)","on_zero_match":"declare","on_unowned":"skip"}]}`,
[]string{"declare", "except"},
},
}
for name, tc := range cases {
Expand Down Expand Up @@ -169,7 +169,7 @@ func TestR40_CheckEchoesResolvedOnUnowned(t *testing.T) {
for _, want := range []string{
"ops[0] on_zero_match: require (built-in); on_unowned: skip",
"b on_zero_match: require (built-in); on_unowned: assign",
"d on_zero_match: declare; on_unowned: assign (built-in; the default does not reach a declared op)",
"d on_zero_match: declare; on_unowned: skip",
} {
if !strings.Contains(out, want) {
t.Errorf("check output missing %q:\n%s", want, out)
Expand Down Expand Up @@ -215,3 +215,76 @@ func TestR40_NightlyRerunConverges(t *testing.T) {
}
}
}

// SPEC R-40b: the real fleet policy, run unchanged across the three repo
// shapes it must survive. One reviewed op, `declare` + `skip`, states the
// whole rule — pre-own what does not exist, never close what is open today —
// and each shape gets the outcome that rule implies, all at exit 0.
//
// This policy was exit 3 before the refusal lift: `check` halted the wave at
// repo 0, so none of these outcomes was reachable.
func TestR40b_FleetPolicyAcrossThreeRepoShapes(t *testing.T) {
pol := syncWritePolicy(t, `{"version":1,"create":true,"ops":[
{"id":"gh","op":"add_owner(/.github/, @org/platform)","on_zero_match":"declare","on_unowned":"skip"}
]}`)
if code, _, errOut := runCLI(t, "check", "--policy", pol); code != cli.ExitOK {
t.Fatalf("check: want exit 0, got %d\nstderr:\n%s", code, errOut)
}

// Shape 1 — no .github/ at all: the rule is declared for the future.
repo := initRepo(t, map[string]string{
"CODEOWNERS": "/src/ @org/core\n",
"src/main.go": "package main\n",
})
code, out, errOut := runCLI(t, "sync", "--repo", repo, "--policy", pol, "--format", "json")
if code != cli.ExitOK {
t.Fatalf("shape 1: want exit 0, got %d\nstderr:\n%s", code, errOut)
}
pfWantFile(t, filepath.Join(repo, "CODEOWNERS"), "/src/ @org/core\n/.github/ @org/platform\n")
if rec := syncDecodeRecord(t, out); rec.Ops[0].Proven != "structural" {
t.Errorf("shape 1: proven = %q, want structural", rec.Ops[0].Proven)
}

// Shape 2 — .github/ with one owned file and one open: co-own the owned
// one, leave the open one open.
// No .github/CODEOWNERS in this fixture: it would become the GOVERNING
// file (S-8) and, being empty, would leave every path open — making this
// shape 3 by accident.
repo = initRepo(t, map[string]string{
"CODEOWNERS": "/.github/dependabot.yml @org/admins\n",
".github/dependabot.yml": "version: 2\n",
".github/workflows/ci.yml": "on: push\n",
})
code, out, errOut = runCLI(t, "sync", "--repo", repo, "--policy", pol, "--format", "json")
if code != cli.ExitOK {
t.Fatalf("shape 2: want exit 0, got %d\nstderr:\n%s", code, errOut)
}
rec := syncDecodeRecord(t, out)
if rec.Ops[0].Status != "applied" || rec.Ops[0].Proven != "tree" {
t.Errorf("shape 2: op = %+v, want applied/tree", rec.Ops[0])
}
if !reflect.DeepEqual(rec.Ops[0].LeftOpen, []string{".github/workflows/ci.yml"}) {
t.Errorf("shape 2: left_open = %v, want the workflow file", rec.Ops[0].LeftOpen)
}

// Shape 3 — .github/ exists and is entirely open: nothing is written, and
// the record says which paths stayed open. Declare must NOT step in here:
// a rule for the scope would close every one of them.
repo = initRepo(t, map[string]string{
"CODEOWNERS": "/src/ @org/core\n",
"src/main.go": "package main\n",
".github/workflows/ci.yml": "on: push\n",
})
code, out, errOut = runCLI(t, "sync", "--repo", repo, "--policy", pol, "--format", "json")
if code != cli.ExitOK {
t.Fatalf("shape 3: want exit 0, got %d\nstderr:\n%s", code, errOut)
}
pfWantFile(t, filepath.Join(repo, "CODEOWNERS"), "/src/ @org/core\n")
rec = syncDecodeRecord(t, out)
if rec.Status != cli.StatusSkipped {
t.Errorf("shape 3: status = %q, want skipped", rec.Status)
}
if !strings.Contains(rec.Ops[0].Reason, "on_unowned") {
t.Errorf("shape 3: reason = %q, want the on_unowned skip reason", rec.Ops[0].Reason)
}
}
35 changes: 24 additions & 11 deletions internal/plan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,18 +202,31 @@ func Build(content []byte, tree []string, opList []ops.Op, opts Options) (*Plan,
return nil, &InvalidError{Msg: fmt.Sprintf(
"on_unowned is only meaningful on add_owner, and %s is not one (R-40): remove_owner cannot touch an open path, set_owners displaces owners by design, and rename_owner has no scope", op.Raw)}
}
if op.OnUnowned == ops.UnownedSkip && op.OnZeroMatch == ops.ZeroMatchDeclare {
// The R-30-shaped contradiction, restated for a library caller: a
// declared rule exists to own files that do not exist yet, which
// are unowned by definition, so declare and on_unowned=skip state
// opposite intents about the same paths (R-40). Checked HERE,
// before the tree decides anything — nested under the zero-match
// branch it would fire only on repos where the scope matches
// nothing, and an exit-3 verdict that depends on the tree is the
// classification exit 3 exists to rule out (review finding).
// R-30, the same defense one rule over (review-bot finding). The
// policy validator refuses declare beside an except clause, but the
// struct is exported and nothing here re-stated it: a library caller's
// op reached the declare branch below, where synthDeclare writes the
// bare scope and never consults exceptSets — so the carve was dropped
// in silence and the declared line went on to govern the very paths
// the except existed to protect. Tree-independent, so exit 3, like its
// siblings above. It is also what makes the contrast drawn just below
// true at THIS layer rather than only in the validator.
if op.OnZeroMatch == ops.ZeroMatchDeclare && len(op.Except) > 0 {
return nil, &InvalidError{Msg: fmt.Sprintf(
"on_unowned=skip cannot be combined with on_zero_match=declare on %s: a declared rule exists to own files that do not exist yet, which are unowned by definition (R-40)", op.Raw)}
}
"on_zero_match=declare cannot be combined with an except clause on %s: a declared rule is one literal CODEOWNERS line, and CODEOWNERS has no negation (S-2), so the line cannot encode subtraction (R-30)", op.Raw)}
}
// on_zero_match and on_unowned COMPOSE (R-40b). They are the two arms
// of the `len(set) == 0` branch below and range over disjoint domains:
// declare acts where the scope matches NO tracked file, skip acts on
// tracked files that have no owner. An earlier refusal of the pair
// argued that files which do not exist are "unowned by definition" —
Comment thread
jordonpeterson marked this conversation as resolved.
// but nonexistent files are not in any op's path universe, which is
// derived from `tree`. The two can never speak about the same path, so
// the pair states one intent per repo shape rather than two intents
// about one path: pre-own what does not exist, never close what is
// open today. Contrast R-30 (declare + except), still refused: there
// the declared line DOES capture future files under the excepted
// pattern, so that carve promise is void rather than vacuous.
set := map[string]bool{}
if op.Kind == ops.RenameOwner {
// R-21 never reaches a rename: its scope comes from current
Expand Down
Loading