Compare the Ruleset Fields the Fleet Actually Manages - #649
Conversation
The AUDIT.md section 6 ruleset comparison projected bypass_actors on both sides. No committed payload declares that key, so the committed side did not merely mis-compare, it exited 5 on "Cannot iterate over null". Process substitution then handed diff an empty left side and every ruleset on every repo reported DRIFT. Blog, which is in sync, reported both branches drifted against the whole live payload. spec/audit.py already excludes the field, and its comment records that including it once made every repo report a ruleset DEFECT. configure.sh is the authority the two must agree with: apply writes the live list back unchanged and check reports it without asserting, because who may bypass a ruleset is a human decision taken in the UI and no payload declares one. Two further surfaces stated the old policy and are swept with it. The payload regeneration snippet in docs/repo-config-carry.md captured bypass_actors into the committed file, which would bake one repo's bypass list into the canonical every other repo diffs against. repo-config/README.md listed the field as part of the subset the audit compares. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the fleet documentation and payload regeneration guidance so ruleset comparisons only include the ruleset fields the fleet actually manages, avoiding false drift/defect reports caused by projecting bypass_actors.
Changes:
- Remove
bypass_actorsfrom the documented ruleset diff/regeneration projections and add rationale for leaving it unmanaged. - Update AUDIT.md ruleset-diff snippet to reflect the managed subset (
name,target,enforcement,conditions,rules) and explain whybypass_actorsis excluded. - Clarify in
repo-config/README.mdthatbypass_actorsis writable but deliberately not asserted/diffed.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| repo-config/README.md | Documents bypass_actors as writable but intentionally unmanaged, and narrows the managed subset list. |
| docs/repo-config-carry.md | Removes bypass_actors from the payload regeneration projection to avoid baking repo-specific bypass lists into canonical payloads. |
| AUDIT.md | Updates the ruleset diff snippet to exclude bypass_actors and explains the policy rationale for doing so. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
The snippet says it compares the same subset as spec/audit.py, and it sorted rules by .type where audit.py sorts by each rule's whole content. Two rules sharing a type stay in input order under a .type sort, so a reordered pair reads as drift on a ruleset that matches. Verified order-insensitive by normalizing main.json against a copy with its rules array reversed, which now compares identical, and re-run against four repos across both workflow models. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
AUDIT.md:82
- The inline comment says audit.py sorts rules by
.type, butspec/audit.pyactually normalizesrulesby sorting on each rule's full JSON content (seenormalize_ruleset:sorted(..., key=lambda r: json.dumps(r, sort_keys=True))). This comment is now misleading and contradicts the preceding prose claiming parity withspec/audit.py.
# bypass_actors stays outside the projection, since no payload declares one and jq cannot sort the null that leaves.
# Rules sort on their whole content rather than on .type, which is the key audit.py sorts by.
# Sorting on .type alone leaves two rules of one type in input order, so a reordered pair would read as drift.
The comment read "on their whole content rather than on .type, which is the key audit.py sorts by", where the relative clause attaches to .type and so states the opposite of the line above it. Name the function instead, which leaves no antecedent to pick wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 10, 2026
Addressed the suppressed finding from the second review round in 30e40d4. You read it correctly, and the fault was mine rather than a false positive. The comment said "on their whole content rather than on Worth recording that this arrived as a suppressed comment rather than a thread, so it would have shipped had the review body gone unread. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
AUDIT.md:84
- The
normkey usessort_by(tojson), butspec/audit.pysorts rules byjson.dumps(..., sort_keys=True). Because payloads are generated with sorted object keys (jq -S) while the live API response can have a different field order,tojsoncan produce different strings for semantically identical rules, reintroducing false drift. Consider normalizing object key order beforetojsonso the snippet matchesnormalize_ruleset's intent (order-insensitive by full content).
# bypass_actors stays outside the projection, since no payload declares one and jq cannot sort the null that leaves.
# Rules sort on each rule's whole content, matching the key normalize_ruleset in audit.py sorts by.
# Sorting on .type alone leaves two rules of one type in input order, so a reordered pair would read as drift.
norm='{name,target,enforcement,conditions,rules} | .rules|=sort_by(tojson)'
sort_by(tojson) serializes each rule in its own key order, and the two sides do not share one. The committed payload is written key-sorted by jq -S, and the API returns its own order, so the same rule yields two different sort keys and the arrays sort into two different orders. Measured: the live main ruleset returns type before parameters where the committed file has parameters before type, and sorting on those strings puts pull_request and required_status_checks in different positions. The snippet nonetheless reported in sync, which is the part worth recording. The live side reaches jq through `gh api --jq`, and that is gojq, which emits object keys sorted. So the projection was silently canonicalizing the input and the filter depended on it. Rewriting the same pipeline with raw `gh api` piped to real jq reports drift on a ruleset that matches. canon sorts keys at every depth before tojson, so the sort key no longer depends on which jq implementation produced the input. Verified three ways: the shipped snippet against four repos across both workflow models, the raw-jq path that exposed the defect, and a rules array reversed to confirm order-insensitivity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 10, 2026
Addressed the second suppressed finding in a96b2b8. You were right, and measuring it turned up something worth recording. The hazard is real. The live But the snippet still reported The fix sorts keys at every depth before serializing, so the sort key no longer depends on that: canon='def canon: walk(if type == "object" then to_entries | sort_by(.key) | from_entries else . end);'
norm="$canon"'{name,target,enforcement,conditions,rules} | .rules|=sort_by(canon|tojson)'Verified three ways: the shipped snippet against four repos across both workflow models, the raw-jq path that exposed the defect, and a |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
AUDIT.md:84
- The audit snippet now relies on jq's
walk/1(viacanon), which is not available on jq 1.5; on hosts that still have jq 1.5 this snippet will fail to compile even thoughrepo-config/configure.shexplicitly avoids that by using a portable recursive normalizer. Consider definingcanonwithoutwalkso the snippet is runnable on the same floor as the script it references.
canon='def canon: walk(if type == "object" then to_entries | sort_by(.key) | from_entries else . end);'
canon called walk/1, which arrived in jq 1.6, and spec/host-tools.json declares no jq floor at all. A host on jq 1.5 does not degrade on walk, it fails to compile the filter, so the diff reports drift on every ruleset it never compared. repo-config/configure.sh already defines its own recursion for exactly this reason and says so, so calling walk here reintroduced the two-tools-two- policies defect the rest of this branch removes. The recursion inserts keys in sorted order, so tojson emits them sorted at every depth. Verified identical to the walk version's output on a live payload, then re-run as the shipped snippet against four repos across both workflow models. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727
commented
Aug 10, 2026
Addressed the third suppressed finding in ff62314. Confirmed both halves of it against the tree.
So calling canon='def canon: . as $in | if type == "object" then reduce (keys_unsorted|sort)[] as $k ({}; . + { ($k): ($in[$k]|canon) }) elif type == "array" then map(canon) else . end;'Verified byte-identical to the Noting a gap this surfaced rather than fixing it here: there is no |
Uh oh!
There was an error while loading. Please reload this page.
Your call, per the decision to accept the fleet-wide finding. Floor set to **1.7** per your follow-up. ## The jq gap `jq` is used by the ruleset normalizer in `repo-config/configure.sh`, the ruleset diff in `AUDIT.md` section 6, and payload regeneration in `docs/repo-config-carry.md`. It was declared in **neither** the contract table nor `spec/host-tools.json`. A host without it cannot apply, check, or audit a repo's branch configuration, and nothing said so. ## The floor is 1.7, and it is a target I did not pick a number and justify it after. Measuring first said **no floor was warranted at all**: nothing in the tree uses a post-1.5 jq feature (checked `walk`, `@base64d`, `--args`, `$__loc__`, `pick`, `toarray`, `abs`, `getpath`, `ltrimstr`, `splits`, `limit`, `$ENV` — the only hits were shell variables and Actions expressions, not jq filters). So the floor is a **target**, the same kind as `python3`, and the entry says so rather than implying a defect nobody found. 1.7 is anchored to what the platform provides: **current Debian stable packages 1.7.1**, so a distribution install satisfies the floor and no host needs a manual build to meet it, while an older release sits below and is the case the floor exists for. That anchor is what makes it actionable. Two rejected alternatives, for the record: - **1.8** (the newest, installed here) would fail a current apt host that runs every documented filter correctly. A floor set to whatever happens to be installed is a host failure nobody can act on, which is the hazard this file's own note names. - **1.6** would encode the `walk/1` boundary, which is a fact about the filters rather than something a host operator can act on. Facts kept in the entry because a reader needs them: on jq 1.5 `walk/1` does not degrade, it **fails to compile**, so a diff built on it reports drift on every ruleset it never compared. And the claim that `keys_unsorted` also needs 1.6 stays **disproved** per the `.github/copilot-instructions.md` record, measured on `jq-1.5-1-a5b5cbe`. ## Consequences swept, not left The floor now sits **above** 1.6, so `walk/1` is available — which made false the comments in `AUDIT.md` and `repo-config/configure.sh` that justified hand-defining a recursion by walk's absence. Both now give the reason that survives: the recursion costs nothing and compiles below the floor as well. This is the same class of defect as #649 (a rule changed, its prose left asserting the old one), so it was swept by term rather than by instance. ## host-tools.json becomes a carried file A repo now states the tools its **own** procedures need beyond the fleet declaration, so the tighten-only layering lives somewhere a reader finds rather than has to know to look for. A repo with nothing to add carries the stub with an empty `tools` list — the footing `OPERATIONS.md` already set — and this repo's new root file is that worked example, with a `note` distinguishing it from `spec/host-tools.json`. **Measured before landing: none of the 22 cataloged repos carries one**, so this adds exactly one `LETTER` per repo. This PR satisfies the hub's own. ## The gate joins the procedures it was written for `scripts/host_gate.py` existed and **no procedure ran it**. `STANDUP.md` section 0 and `AUDIT.md` now do, alongside `RESYNC.md` in #651. Each passes `--repo`, because the gate reads the target's declaration relative to that flag and a bare run layers the hub's instead while printing the same healthy digest either way. ## Verification `scripts/test_host_gate.py` **failed first**, which is the tests working: `test_the_declared_floors_are_the_ones_with_a_stated_reason` asserts the floor set exactly, so a new floor cannot land unnoticed. I updated it and generalized `test_a_target_floor_says_so_rather_than_implying_a_defect` from the single `python3` entry to the **set** of target floors, so a third one added without the two-kinds wording fails rather than reading as measured. The boundary is checked through the gate's own `compare`/`parse_version` rather than a reimplementation of them: ``` jq 1.5 -> FAILS jq 1.7 -> meets jq 1.6 -> FAILS jq 1.7.1 -> meets jq 1.6.1 -> FAILS jq 1.8.2 -> meets ``` - 552 script tests OK, `--selftest` PASS, `validate.py` OK - `host_gate.py`: 7 tools, `jq 1.8.2 meets the 1.7 floor`, and it reads the hub's own stub (`host-tools.json layered 0 local entry(s)`) - `prose_lint`, `repo_gate --check eol`, `markdownlint-cli2`, `cspell` on the gated files: all clean - **`editorconfig-checker` caught a real defect every other gate passed**: a `sed -i` I used wrote one bare LF into `AUDIT.md`. Fixed and re-verified byte-wise; the diff stayed at 9 lines rather than a whole-file rewrite. ## One bookkeeping item left for you The `keys_unsorted` entry in `.github/copilot-instructions.md` carries **Delete when** - "nothing this check runs on carries a jq older than 1.6", and an enforced 1.7 floor arguably satisfies that. I left it in place rather than deleting it unilaterally, because it exists to answer a repeat reviewer finding and its measured proof is cited from the new `jq` entry. Your call whether it retires. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two changes from one source: a stale enumeration a peer agent found, and a backlog entry for the method that found it. TODO disposition: **New entry**, per `TODO.md` rule 8. ## 1. The stale enumeration Two enumerations of `spec/` list its contents and stop before the host tool contract, which landed there in #652 along with a stub at the repository root: - `GOVERNANCE.md` "Repository Layout" - `README.md`'s `spec/` bullet Both were accurate when written. Neither is now, and a reader looking for where the tool floors live would search both and find nothing. ### Provenance, and two claims that did not reproduce Raised by the **ESPHome-Config agent** over peer messaging, not by a gate. It hit the same class in its own `Repository Layout` while planning its `AGENTS.md` -> `AGENTS.md` + `GOVERNANCE.md` split, and named it as the trap the `Documentation Style Conventions` maintenance rule exists for: prose no linter can see, going stale under a change that never mentioned it. I checked the hub for the same shape and for two adjacent claims from the same report. Recorded because a future reader will wonder: | Claim | Hub result | |---|---| | Carried docs still link `AGENTS.md#<section>` for sections that moved to `GOVERNANCE.md` | **Not present.** Zero `AGENTS.md#` anchors tree-wide, so the hub's anchors were rewritten with the split. Downstream-only, for repos still carrying pre-split docs. | | Other `gh api ... --jq '{...}'` projections feeding a diff share the gojq key-sorting exposure fixed in #649 | **Not exposed.** The only other one is the settings diff, where both sides pass through `jq -S .` over a flat boolean object, so key order cannot reach the comparison. Verified by running it both ways, `gh api --jq` and raw `gh api` piped to real `jq`: byte-identical. | `repo-config/` keeps its "apply script" wording, still true of the hub, which hosts the script it no longer carries downstream. ## 2. The backlog entry A `decision` cluster, because the mechanism needs no build and the only open question is **which document may carry rules that bind a downstream agent**. `GOVERNANCE.md` reaches those agents and costs a fleet-wide re-vendor plus the two manifest edits `spec/section-model.md` requires of any new section. A hub-only `docs/` file costs nothing and leaves the rules unreachable from the repositories that would apply them, which is the failure `AGENTS.md` "Fleet Bootstrap" exists to prevent. Settled and recorded so it is not re-derived: - **Cross-host does not work, by construction rather than configuration.** A peer address is a Unix domain socket under `/run/user/1000/cc-socks/`, which cannot cross a machine boundary. Cloud sessions and Remote Control sessions are the documented cross-host paths and neither appears in a listing on this host, so both are unverified rather than absent. - **The addressing has a guardrail worth keeping.** A bare peer name was refused and the transport demanded the `[ref]` a listing prints, which is what stops a message reaching the wrong repository's agent. - **The method earns its place on evidence.** One exchange produced the causal commit for the section 6 defect (`90e3255`), which this session had not identified from the symptom; a one-line reproduction of the gojq behavior that made an earlier fix pass for the wrong reason; and four procedure gaps no gate reports. - **A peer's finding is checked rather than adopted.** Two of those four did not reproduce here, one did and is part 1 of this PR. - **The boundary that matters is permission, not politeness.** A peer cannot widen what the asking session may do, so blocked work goes back to the maintainer rather than sideways to another agent. ## Verification `prose_lint --diff HEAD`, `repo_gate --check eol`, `editorconfig-checker`, `markdownlint-cli2` on `TODO.md`, and `cspell` on the gated files: all clean. Both `[files]` and `[section-model]` reference names already resolve in `TODO.md`. `GOVERNANCE.md` "Repository Layout" is `intent` fidelity per `spec/section-model.md`, so part 1 forces no fleet-wide re-vendor. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Promotion of six squashes. **Merge with a merge commit, never a squash, and never with `--delete-branch`** — this PR's head *is* `develop`. | PR | What it fixed | |---|---| | [#649](#649) | `AUDIT.md` section 6's ruleset diff projected `bypass_actors`, which no payload declares, so jq exited 5 and **every ruleset on every repo reported DRIFT**. Two further surfaces carried the same dead policy. | | [#650](#650) | The deletion detector: hub git-tracked paths minus the `spec/files.json` baseline, so a retired file is derived rather than remembered. Corrected `configure.sh` from 6 carriers to **15**. | | [#651](#651) | `RESYNC.md`, the third entry point, routed from the byte-locked `AGENTS.md` "Fleet Bootstrap". | | [#652](#652) | A `jq` 1.7 target floor, `host-tools.json` as a carried baseline file, and `scripts/host_gate.py` wired into all three procedures for the first time. | | [#653](#653) | Two `spec/` enumerations that went stale when the host contract landed there. | | [#654](#654) | The inbound-reference sweep counts as part of a deletion, including the runnable-command and orphaned-definition shapes. | ## Why this promotion matters more than most Downstream repos read hub `main` as ground truth. While `main` sits at [`0a86bca`](0a86bca): - Every repo that runs the `AUDIT.md` section 6 snippet gets **false ruleset drift on both branches**. The ESPHome-Config agent reproduced this live and is holding its own fix stashed rather than diverging from the hub. - A repo resyncing now re-vendors to `main`'s revision and then again after promotion. Verified against Blog, whose carried `AGENTS.md` "Fleet Bootstrap" differs from both refs, so it is two re-vendors instead of one. - `RESYNC.md` does not exist on `main`, so a downstream agent asking "how do I sync" still routes to `AUDIT.md`, which measures and deliberately states no order. ## Fleet cost this carries, measured and accepted `host-tools.json` becomes a carried baseline file, so it is one `LETTER` on **22 of 22** cataloged repos, the hub included. That was measured before landing and accepted by the maintainer. `RESYNC.md` section 0 was amended so a letter wave of this shape reads as a file to carry inside a resync rather than as evidence a repo was never stood up. ## Verification on the merged head `spec/validate.py` OK, `spec/audit.py --selftest` PASS, 557 script tests OK, `scripts/host_gate.py` clean over 7 declared tools, `repo_gate.py --check eol` clean, `editorconfig-checker` clean, working tree clean. Post-merge fleet audit: 22 repos, **zero errors**. Two hub-side artifacts clear on this merge, both promotion-pending rather than defects: the hub's own `AGENTS.md` "Fleet Bootstrap" reads as stale against its `develop` canonical, and the hub reports its own `host-tools.json` absent because the file exists on `develop` only. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Problem
AUDIT.md section 6's ruleset comparison projected
bypass_actorson both sides and sorted it withsort_by(.actor_id). No committed payload declares that key, so the committed side did not mis-compare, it failed outright:Process substitution then hands
diffan empty left side, so the whole live payload reads as an addition and the|| echo DRIFTbranch fires. Every ruleset on every repo reported drift, from the snippet rather than from the repo. Reported by the agent resyncing ESPHome-Config.Pre-fix, against Blog, which is in sync:
Why this field
spec/audit.py'sRULESET_SUBSETalready excludes it, and the comment above it records that including it once made every repo report a rulesetDEFECT.repo-config/configure.shis the authority both must agree with:applyreads the live list and writes it back unchanged,checkreports it and asserts nothing, because who may bypass a ruleset is a human decision taken in the UI. The mechanized path was corrected and the prose snippet beside it was not.Change
Three surfaces stated the old policy, so the class is swept rather than the instance:
AUDIT.mdsection 6 comparesname,target,enforcement,conditions,rules, matchingRULESET_SUBSET, and states why the field is outside it.docs/repo-config-carry.mdpayload regeneration no longer capturesbypass_actors. Capturing it baked one repo's bypass list into the canonical every other repo diffs against.repo-config/README.mdno longer lists the field as part of the compared subset.Verification
The snippet is extracted from the shipped
AUDIT.mdbytes rather than retyped, placeholders substituted, and run against three repos including two operational ones (exercising theoperational/develop.jsonselection):Gates:
prose_lint.py --diff HEADclean,repo_gate.py --check eoland--check eol-coverageclean,markdownlint-cli20 issues on all three files.Note the committed downstream copies of these payloads are separately stale against the hub canonical (
reports/divergences.md), which is a re-vendor drift. This PR does not touch that: the live rulesets match the hub's payloads, so enforcement was never affected.🤖 Generated with Claude Code