feat(conformance): branch protection as an inventoried property (backend#1608 increment 2)

[LukasWodka]

What this does

Increment 1 of #1608 gave the harness teeth (conformance-gate.yml, now required on develop). This gives it a second thing to check, and turns repo-inventory.yml from a caller inventory into the per-repo contract the ticket describes.

The first property added is the one backend#1276 just settled by hand: branch protection.

Why this property first

#1276 armed required checks and closed admin bypass across 16 prod branches one API call at a time. Nothing in the org would have noticed any of it drifting back. Now both directions are findings — someone weakening a setting, and someone "fixing" a deliberate choice (strict: false) back to a default.

The schema

protection_policy states the policy per branch role; every repo carries a cell per role. Same headline rule as callers: absence is never implicit, and an exemption without a written reason is rejected.

Two shapes worth calling out:

• divergent takes a mapping, not a bare string — it must name the policy keys that differ. A blanket "this one is different" would switch off every assertion at once, which is how an exemption written for one reason silently covers an unrelated regression later. docs diverges on min_reviews alone, so its force-push, deletion and admin posture are all still asserted.
• null means NOT ASSERTED, deliberately different from asserting false. enforce_admins on develop/staging is null because #1276 leaves that escape hatch open — a repo that chooses to harden its own develop must not be reported as drift for doing something stricter.

Both protection systems, or this would lie

GitHub runs two independent protection systems and the classic API is blind to the second. A ruleset-only branch returns 404 "Branch not protected" from branches/{b}/protection while GET /branches reports protected: true.

On 2026-08-10 an audit that read only the classic endpoint reported docs/staging as unprotected and recommended fixing it. The fleet-wide promotion-branches-merge-commit-only ruleset had been covering it the whole time. That wrong finding is why read_protection() merges the two systems per property rather than per system — the rules do not compose:

property	classic	ruleset
force pushes	allow_force_pushes: false	pull_requestornon_fast_forward
deletion	allow_deletions: false	deletiononly — pull_request does NOT cover it
reviews	review count	pull_request.required_approving_review_count (max of both)
enforce_admins	classic only	— (rulesets use bypass actors)

Also: prod resolves from the branch list, never by probing branches/master. That call follows rename redirects and returns 200 for a branch that does not exist — measured the same day, all 16 train repos reported a master; only 3 have one.

Fail closed

A 404 on the classic endpoint is a fact ("no classic protection"). Any other error, and any unreadable ruleset read, is UNREADABLE — never "unprotected". An unreadable ruleset read fails closed even when the classic read succeeded, because a partial view of protection is not a verdict.

Verification

check	result
selftest	72 pass / 0 fail (was 47; +25 covering schema, the merge, fail-closed paths, branch-role resolution)
ruff	clean
full fleet audit	20 of 20 repos, NO DRIFT
negative control	3 mutated cells → exactly the expected findings

The fleet audit landing green is the bar increment 2 had to clear — a checker that arrives red gets ignored, which is the code-quality soft-fail lesson.

The negative control matters more than the green one. Mutating the inventory to claim things that are false produced:

- backend: protection.prod is `exempt` but main exists and carries classic
protection. The exemption is stale - promote it to `required`.
- claude-skills: develop (develop) has NO classic branch protection.
No ruleset covers it either.
- claude-skills: develop (develop) requires 0 approving review(s), policy wants >= 1.
- claude-skills: develop (develop) allows force pushes.
- claude-skills: develop (develop) allows deletion.
- claude-skills: develop (develop) conversation resolution=False, policy wants True.
- docs: staging (staging) requires 0 approving review(s), policy wants >= 1.

Note the ruleset-aware wording — a branch with no classic layer says whether a ruleset covers it, so the reader is not sent to re-derive that by hand.

What the exemptions record

Nine exempt and five divergent cells, each with a written reason:

• docs — min_reviews: 0 on staging + prod (the 2026-06-04 content-only exemption)
• claude-skills / release-train / rfcs — enforce_admins: false on main: not on the train, so that main is a working branch, not a pipeline prod branch. Deliberate for release-train specifically — it is the tool used to repair the pipeline, so locking it hardest is self-defeating.
• devex-bootstrap — exempt on all three roles until #1597 decides its disposition. UNREMEDIATED, deliberately: arming protection on a repo we may archive is work we would throw away.
• claude-skills develop — exists but unprotected. UNREMEDIATED, recorded with a name on it rather than quietly dropped.

Not done here

The rename to repo-conformance.py. That also means touching conformance-gate.yml's GUARDED list and caller-drift.yml, and coupling a rename to a behaviour change makes both harder to review. Mechanical, separate, later.

Parent: backend#1608 · Decisions encoded: backend#1276

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes org-wide conformance checks and GitHub API reads for branch protection; mis-merged ruleset/classic logic or inventory mistakes could false-positive or miss drift on critical branches, though fail-closed and fleet green run reduce that risk.

Overview
Extends repo conformance beyond workflow callers/copies: repo-inventory.yml becomes schema v2 with a fleet protection_policy (develop/staging/prod roles, backend#1276) and a protection cell on every repo (required, exempt, or divergent with named key overrides so other assertions stay live).

caller-drift.py now audits branch protection via GitHub’s classic API and rulesets (paginated rules/branches), merges effective settings per property, resolves prod from the branch list (not branches/master probes), and fails closed on unreadable reads. Protection API failures go to a separate unreadable list so a protection outage does not wipe caller findings or trigger the all-repos-unreadable abort. Reporting is labeled repo conformance drift.

Selftests add ~25 cases for protection schema, ruleset-only branches, exempt staleness ordering, and pagination.

^{Reviewed by Cursor Bugbot for commit 465f162. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

Bugbot finding — confirmed and fixed in eb43bb6

Bugbot is right on both halves, and the finding is worth recording rather than just closing: it is the exact defect class this PR's own header documents, found in the change that documents it.

The exempt path gated on probe.error is Nonebefore checking what had been learned:

ifprobe.errorisNoneandprobe.classic_present: # wrong order

Two consequences:

1. An unreadable probe silently meant "not stale" and the run stayed green — a read that can fail, deciding a negative.
2. The worse half:read_protection() sets error when the ruleset call fails even though the classic read already succeeded. So classic_present could be known True while the stale-exemption finding was suppressed anyway. That is a case where the code had enough information to report and threw it away.

Fixed by deciding on what is known first, and falling back to unreadable only when nothing was established:

• classic_present → stale-exemption finding, regardless of a ruleset read failure
• error and nothing established → unreadable (fail closed)
• genuinely unprotected → the exemption holds, silently

Three selftests added, one per branch of that decision — including the mixed case Bugbot identified, which no existing test covered. 75 pass / 0 fail, ruff clean.

This is another Bugbot finding in this epic that sits at a read boundary rather than in logic. The rule it keeps re-teaching, now stated in the code comment: a read that can fail must not be the thing that decides a negative — and ordering the check wrong is enough to reintroduce it.

[LukasWodka]

Both findings confirmed and fixed in 556152d

Both are real, and both are the same family as the first: something that can be partial or wrong, trusted as complete.

1. Divergent overrides skipped value checks

_protection_entry validated that the override key names were overridable and stopped there, while _policy_block validated values. So a cell could look like a narrow, documented divergence while neutralising the assertion it claims merely to adjust:

• min_reviews: -1 — got.min_reviews < -1 can never be true, so the check is dead
• enforce_admins: null — stops asserting it entirely

That defeats the entire reason divergent names its keys instead of being a blanket switch. Both paths now share _policy_value(), and overrides additionally reject null: naming a key means stating a different value for it; not asserting something is a protection_policy decision, not a per-repo one.

It also turned up a latent trap in my original check — bool is a subclass of int in Python, so min_reviews: true passed a bare isinstance(value, int). Now excluded explicitly, at both policy and override level.

2. The ruleset read was not paginated

rules/branches/{b} defaults to 30 items per page, so a rule on page 2 was dropped with no unreadability signal — a partial view of a branch's protection reported as a verdict.

This one is worth naming plainly: it is the exact failure mode read_protection()'s own header describes, in the function that header is attached to. New gh_json_array() streams elements with --paginate --jq '.[]' — element-wise, deliberately not an aggregating filter, which --paginate would re-run per page — and reassembles them.

Verification

check	result
selftest	84 pass / 0 fail (was 75)
new call shape, live GitHub	verified on a branch with rules and one without
full fleet audit, re-run	20 of 20, still no drift
ruff	clean

The +9 tests cover override values, the bool-is-int trap, the --paginate contract asserted on the call itself (pagination is gh's job and is stubbed out, so the call shape is what's testable), and multi-page reassembly.

The pattern, for the record

Three findings on this PR, all at read boundaries, none in the logic:

1. exempt probe let a failed read decide "not stale"
2. divergent overrides validated names but not values
3. the ruleset read took page 1 as the whole answer

Same rule each time — a read that can fail must not be the thing that decides a negative — and each was a different way to break it: wrong ordering, incomplete validation, missing pagination. Knowing the rule is not sufficient; that is the argument for the harness rather than for care.

[LukasWodka]

Both confirmed and fixed in 3b8a317

3. Protection failures could abort the whole audit

This is the most serious of the four. evaluate_protection appended into the same unreadable list that drives the evaluated <= 0 short-circuit — so a fleet-wide protection read failure (rate limit, auth, a GitHub incident) would make every repo look unreadable, call die(), and destroy the report, including caller/copy findings already collected successfully for those same repos.

The principle it violated: fail-closed must mean the run goes RED. It must not mean the results are thrown away before anything is written.

Protection failures now accumulate in their own list. evaluated is computed from repo-read failures only, and the two lists are merged afterwards, so protection problems are still reported and still fail the run — they just can no longer trigger the abort.

4. An exemption reason overstated what the checker does

no_develop_branch_single_branch_repo claimed creating a develop would turn the exemption into a finding. It would not — the guard flags an exemption when the branch appears and carries protection. An unprotected new develop isn't flagged, because the checker can't distinguish it from develop_unprotected_non_train, an exemption written precisely about an existing unprotected branch.

The reason now states the rule exactly, and why the case it's actually about is still covered: enrolling in the train means a protected develop, which does trip the check.

Worth stating why this one counts as a real defect and not a doc nit: in this file the written reason is the audit trail. A reason that overstates its own guard is worse than no reason — it tells a future reader a gap is covered when it isn't.

Verification

check	result
selftest	86 pass / 0 fail (was 84)
fleet audit, re-run	20 of 20, no drift
ruff	clean
inventory	parses, 20 repos

The +2 tests assert protection failures land in their own list and are still recorded as UNREADABLE — isolating them must not make them silent, which would have been an easy way to "fix" finding 3 by introducing a worse one.

Running tally

Four findings on this PR, all at read/report boundaries, none in the domain logic:

#	finding	the rule it broke
1	exempt probe let a failed read decide "not stale"	a failed read must not decide a negative
2	divergent overrides validated names, not values	validate the thing that changes behaviour
3	ruleset read took page 1 as the whole answer	a partial read is not a verdict
4	protection failure could destroy the report	fail-closed means red, not amnesia

Each is a different way to break the same underlying rule. That is the argument for the harness rather than for care — and it is exactly what this PR is building.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 3b8a317. Configure here.}

[LukasWodka]

Confirmed and fixed in 465f162

Bugbot is right, and this one is the direct consequence of the previous fix — which is why it gets its own commit rather than a quiet amend.

Isolating protection failures from the audit abort meant unreadable carries both families by the time the run exits. The annotation still said "Caller state is UNKNOWN there", so after a protection-only outage it named the wrong thing as broken and told the reader caller results were lost — when they had in fact been evaluated and preserved.

That undoes the entire point of the separation. The whole reason to keep the lists apart is so the report can say which half is trustworthy; a message that flattens them back together throws that away at the last step.

Both the step summary and the ::error:: annotation now count the families separately, and say explicitly that caller/copy state was read when only protection failed:

::error::2 repo read(s) failed - caller/copy state UNKNOWN.
::error::2 branch-protection read(s) failed - protection state UNKNOWN.
::error::1 repo read(s) failed - caller/copy state UNKNOWN; 1 branch-protection read(s) failed - protection state UNKNOWN.

Verification

selftest 86 pass / 0 fail · all three message shapes exercised (caller-only, protection-only, both) · fleet audit re-run 20 of 20, no drift · ruff clean.

Tally

#	finding	rule broken
1	exempt probe let a failed read decide "not stale"	a failed read must not decide a negative
2	divergent overrides validated names, not values	validate what changes behaviour
3	ruleset read took page 1 as the whole answer	a partial read is not a verdict
4	protection failure could destroy the report	fail-closed means red, not amnesia
5	the report then misnamed which half failed	isolating two things is pointless if you re-merge them before saying so

Five findings, all at read/report boundaries, none in the domain logic — and #5 was introduced by the fix for #4. That is the honest case for this PR existing: the rule is easy to state and repeatedly easy to break, including while fixing a break of it.

[shujaatTracebloc]

Approving — reviewed the code, not just the green.

What it does: schema v2 adds a fleet protection_policy (develop/staging/prod roles) and a per-repo protection cell (required / exempt / divergent), and caller-drift.py audits effective branch protection across both GitHub systems (classic API + rulesets), failing closed on unreadable reads.

Verified locally against head 465f1627:

• Ran the selftest — 86/86 pass, including the behavioral paths that matter: ruleset-only branch merge, --paginate element reassembly across pages, exempt-staleness ordering (decides on what's known first, only falls back to unreadable when nothing was established), and the separate protection_unreadable list so a protection outage can't trigger the all-repos-unreadable die() and discard real caller findings.
• Spot-checked the design decisions and they're sound: prod resolved from the branch list (not branches/master probes, which follow rename redirects); classic 404 treated as fact not read-failure; allow→block flag inversion; strict/enforce_adminsnull = not-asserted vs false; and the bool-is-int guard on min_reviews.
• Every Bugbot finding is resolved with an inline (Bugbot, .github#196.) annotation at the fix site.

CI all-green, Bugbot clean, 0 unresolved threads. Nicely scoped increment. LGTM.