Uh oh!
There was an error while loading. Please reload this page.
release-train: develop -> staging - #203
Merged
Merged
Conversation
…end#1608 increment 2) (#196) * feat(conformance): branch protection as an inventoried property (backend#1608 increment 2) Increment 1 gave the harness teeth (conformance-gate.yml, required on develop). This gives it a second thing to check. `repo-inventory.yml` grows from a caller inventory into the per-repo CONTRACT, and 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 by hand, one API call at a time. Nothing would have noticed any of it drifting back. Both directions are now findings: someone weakening a setting, AND someone "fixing" a deliberate choice (strict: false) back to a default. THE SCHEMA `protection_policy` states the fleet policy per branch ROLE (develop/staging/ prod); every repo carries a cell per role. Same rule as callers - absence is never implicit, and an exemption without a written reason is rejected. `divergent` takes a MAPPING here, not a bare string: it must name the specific 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, which is deliberately different from asserting false. enforce_admins on develop/staging is null because #1276 leaves that escape hatch open; a repo that hardens its own develop must not be reported as drift. BOTH PROTECTION SYSTEMS, OR THIS WOULD LIE GitHub runs classic branch protection AND rulesets, and the classic endpoint is blind to the second: a ruleset-only branch returns 404 "Branch not protected" while the branch list says protected: true. On 2026-08-10 an audit that read only the classic API 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. read_protection() merges the two PER PROPERTY, because the rules do not compose: a `pull_request` rule blocks direct and force pushes but NOT deletion - that is a separate rule type. 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` and 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. VERIFIED - selftest: 72 pass / 0 fail (was 47; +25 covering the schema, the merge, the fail-closed paths and the branch-role resolution) - full fleet audit: 20 of 20 repos, NO DRIFT - it lands green on today's fleet, which is the bar increment 2 had to clear or be ignored - negative control: mutating three inventory cells produced exactly the expected findings, including "No ruleset covers it either" and a stale-exemption catch NOT DONE HERE The rename to repo-conformance.py the ticket describes. Renaming the file 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. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(conformance): the exempt staleness probe must not let a failed read decide Bugbot, .github#196 - and it is the defect class this PR's own header documents, found in the change that documents it. The exempt path gated on `probe.error is None` BEFORE checking what was learned. So an unreadable probe silently meant "not stale" and the run stayed green: a read that can fail, deciding a negative. Worse in one specific case: 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. Now: decide on what IS known first (classic_present -> stale finding, regardless of a ruleset read failure), and fall back to UNREADABLE only when nothing was established. A genuinely unprotected branch still leaves its exemption intact. Three selftests, one per branch of that decision. 75 pass / 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(conformance): validate divergent override VALUES, and paginate the ruleset read Two Bugbot findings on .github#196, both real, both in the same family as the first one: something that can be partial or wrong being trusted as complete. 1. DIVERGENT OVERRIDES SKIPPED VALUE CHECKS _protection_entry validated that override KEY NAMES were overridable and stopped there, while _policy_block validated values. So a cell that looked like a narrow, documented divergence could neutralise the assertion it claims merely to adjust: `min_reviews: -1` can never fail, and `enforce_admins: null` stops asserting it at all. That defeats the entire reason `divergent` names its keys instead of being a blanket switch. Both paths now share _policy_value(). 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. Also fixes a latent trap in the original check - bool is a subclass of int in Python, so `min_reviews: true` passed a bare isinstance(value, int). 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. Precisely the 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, not an aggregating filter, which --paginate would re-run per page) and reassembles them. VERIFIED - selftest 84 pass / 0 fail (was 75; +9 covering override values, the bool-is-int trap, the --paginate contract on the call itself, and multi-page reassembly) - new call shape confirmed against live GitHub on a branch with rules and one without - full fleet audit re-run: 20 of 20, still no drift - ruff clean Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(conformance): isolate protection unreadability from the audit abort; correct a reason that overstated the checker Two more Bugbot findings on .github#196, both real. 3. PROTECTION FAILURES COULD ABORT THE WHOLE AUDIT evaluate_protection appended into the same `unreadable` list that drives the `evaluated <= 0` short-circuit. 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 that had already been collected successfully for those same repos. 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 are merged afterwards so protection problems are still reported and still fail the run. 4. AN EXEMPTION REASON OVERSTATED WHAT THE CHECKER DOES `no_develop_branch_single_branch_repo` claimed that 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` is not flagged, because the checker cannot distinguish that 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 is actually about is still covered: enrolling in the train means a PROTECTED develop, which does trip the check. A written reason that overstates its own guard is worse than no reason - it is the audit trail lying. VERIFIED - selftest 86 pass / 0 fail (was 84; +2 asserting protection failures land in their own list and are still recorded as UNREADABLE rather than silenced) - full fleet audit re-run: 20 of 20, still no drift - ruff clean, inventory parses Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(conformance): name WHICH read family failed in the report and the annotation Bugbot, .github#196 - and it is the direct consequence of the previous fix, which is why it is worth its own commit rather than a quiet amend. Isolating protection failures from the audit abort meant `unreadable` now 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. 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. VERIFIED - selftest 86 pass / 0 fail - all three message shapes exercised (caller-only, protection-only, both) - full fleet audit re-run: 20 of 20, no drift - ruff clean Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
) * ci: teach every rank table to READ the agent column (backend#1577) Hop 1 of RFC-BACKEND-1552 D5: reads before writes, in separate hops, because org-github's callers are pinned @main and both old and new callee code are live during a promotion of the reusables. THIS WAS ALREADY A LIVE LANDMINE. `Staging (agent review)` EXISTS on project 2 today (added by the #1592 rename) and NO rank table knew it. rank() returned "", the `[ -n "$SR" ]` guard failed, evaluation fell through to strict equality, and the card would have BLOCKED every prod promotion carrying it -- the backend#1411 shape. Measured: 633 board items scanned, zero currently in that column, so it was armed and waiting rather than firing. fr-gate.yml rank 7, FR->8, Ready for prod->9, Prod->10, terminals->11 advance-deploy-env.yml same, plus the agent column maps to the `staging` deploy environment if a card is dragged there by hand kanban-reconcile.yml the agent column joins its sibling post-merge columns, so an OPEN issue drifting there goes back to Backlog like it would from `On dev` Renumbering is safe: every comparison is relative (`rank(a) >= rank(b)`, and TARGET_RANK is itself `$(rank ...)`), with no hardcoded rank literals anywhere. Checked before touching the numbers. Caught while renumbering: Prod moved to 10 and collided with the terminals, also 10. Terminals must rank STRICTLY above Prod or the no-demotion guard weakens. Bumped to 11. READ ONLY -- nothing writes this Status yet. That is #1578, deliberately a later hop, and it must not land before the agent can produce a verdict or cards park in a column nothing advances. Verified: agent column vs required "On dev" -> PASS agent column vs required "Ready for prod" -> BLOCKED an unknown column -> BLOCKED (still fails closed) actionlint clean on all three; standards selftest 20/20. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(reconcile): the agent column must reach the classifier at all Bugbot (#200): the classify case I added for `Staging (agent review)` was DEAD CODE. The non-terminal pull filter that populates items.ndjson never listed the column, so cards there were filtered out before classification ran. That is the exact shape this whole session has been auditing -- a guard exists, nothing reaches it -- and I shipped one while writing the audit. Found by reading rather than reported: a SECOND omission at the closed-completed case. A closed issue in the agent column fell through to `*)` and was marked Done, overwriting a deploy state -- precisely what its sibling list exists to prevent (D8: a deploy state is not Done, follow the PR's stage). Swept every column list in the file. The one remaining absence is DELIBERATE: the `CLOSER_BASE` mapping at ~L426 is a WRITER (branch -> destination column), and hop 1 is read-only. That belongs to #1578. actionlint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 10, 2026
ContributorAuthor
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 36b9d29. Configure here.
3 tasks
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Changes FR-gate and advance-deploy-env stage ordering (prod promotions can block on agent-review cards) and adds live GitHub protection assertions for every repo; misconfiguration or API read failures will fail conformance runs.
Overview
Adds read-side support for the kanban column
Staging (agent review)before anything writes that status (#1578).advance-deploy-env,fr-gate, andkanban-reconcilenow rank it between On dev and human FR (rank 7), map.kanban.ymloverrides to staging deploy env, and reconcile it like other post-merge/deploy columns so the first card in that column does not break monotonic advance or prod gates.Extends
repo-inventory.ymlto schema v2 with a fleetprotection_policyand per-repoprotectionrows (required / exempt / divergent), encoding backend#1276 branch-role expectations for all active repos.scripts/caller-drift.pygrows into a repo conformance checker: it reads classic branch protection and rulesets (paginated), compares against the inventory, keeps protection read failures separate from caller/copy failures so a GitHub API outage does not wipe the whole report, and ships matching selftests for schema, ruleset-only branches, and exempt staleness.Reviewed by Cursor Bugbot for commit 36b9d29. Bugbot is set up for automated code reviews on this repo. Configure here.