Uh oh!
There was an error while loading. Please reload this page.
release-train: develop -> staging - #238
Merged
Merged
Conversation
…est (backend#1608 items 3+4) (#227) * feat(conformance): publish the matrix as a standing screen, not only on drift (backend#1608) Item 3 of the harness: "one screen". The matrix has existed since .github#223, but nothing published it when the fleet was GREEN — the drift comment fires only on findings (deliberately: an all-clear per run trains people to ignore the issue, backend#1344), so a conformant fleet produced a step summary on a run nobody opens. "Where does the fleet stand?" was still answered by running the script by hand, which is the thing the ticket set out to remove. Split the two roles: the issue BODY is the current state, rewritten every scheduled/manual run; the COMMENTS stay drift-only and remain the history. The body is rewritten regardless of outcome, which is what makes staleness mean anything. The audit is weekly, so a timestamp older than ~8 days means the audit itself stopped and conformance is UNKNOWN — the backend#1530 cron-watchdog contract. That only holds if a RED run rewrites the body too; otherwise a fleet that broke in January still shows January's green and merely looks stale-ish. Hence always(), not gated on exit_code. Also fixed while here: the drift comment was posting to backend#1415, which is CLOSED. Comments on a closed issue still post, so it looked like it worked — but a closed issue cannot be pinned, drops out of default issue views, and notifies nobody not already subscribed. Drift has been reporting into a drawer. Now backend#1781, open and pinnable. The verdict is DERIVED from the counts rather than from the exit code alone: a clean exit alongside a non-zero unreadable/findings count renders "Inconsistent result — treat as UNKNOWN". The script does not produce that pair today, which is why it is worth pinning — "every repo read" printed directly above "3 unreadable" is the most confidently wrong thing this screen could say. Report heading "Repo conformance drift" -> "Repo conformance": it now renders under a verdict line that is often "Conformant", and a heading asserting drift above a green matrix contradicts itself on the one screen people should trust. Evidence: selftest 123 pass / 0 fail; actionlint and shellcheck clean; the body rendered against a LIVE fleet audit (20/20 evaluated, 0 unreadable, 0 findings); all five verdict paths exercised (clean / drift / unevaluable / empty exit code / inconsistent), and the >60k truncation guard verified to keep the body under GitHub's 65536 limit — a rejected edit would leave the previous body in place, which is a stale green presented as current. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(conformance): remediate drifted copies by PR, and only where a PR is honest (backend#1608) Item 4: the self-healing half. Detection alone still leaves the manual work; it only makes it visible. WHAT IT WILL NOT DO, which is most of the design. The ask was "open the missing-piece PR per repo", but a pull request is the right instrument for exactly one of the five families: copies REMEDIABLE. Byte-identical by definition, and the guard already holds the canonical bytes, so the fix is exact rather than generated. callers NOT generated. Caller content is repo-specific — measured 2026-08-12, all EIGHT sampled repos have a different code-quality-caller.yml because each passes its own toolchain inputs. A generated caller would be a plausible file that is wrong for that repo, which is worse than an absent one that at least reports as a finding. protection, NOT remediable BY PR at all. These are API settings, not required_checks, files in the tree; no commit can change them, so a PR rulesets claiming to fix them would be theatre. And within copies, only entries marked `required`. `divergent` records a written reason why a repo differs — cli pins actions/stale@v11 where canon pins v9, and the newer pin may well be the better one. Silently overwriting a recorded decision would destroy the judgement the inventory exists to hold. Same for `exempt`. Both are reported, never rewritten. Dispatch-only, gated on `github.event_name == 'workflow_dispatch' && inputs .create-prs == true` — the same expression standards-sync.yml uses, whose remediation path this mirrors throughout (422-means-reuse, sha-refresh, PR reuse, actor assignment). Writing to twenty repos is not something a cron may decide to do, and a PR-triggered audit that wrote to the fleet would be a supply-chain hole. Remediation failures go in their OWN list, not `unreadable`: the exit path derives "caller/copy state UNKNOWN" by subtracting the protection and ruleset lists from `unreadable`, so a failed WRITE pushed in there is reported as a failed READ — the wrong diagnosis on the line an operator acts from. They exit 2, because "I tried to fix it and could not" is not the same as "there was drift". Landed on the item-3 branch deliberately: it touches the same two files, so a separate PR would either conflict or be a stacked PR, and the standard forbids stacking. Evidence: selftest 132 pass / 0 fail (up from 123). Five mutations, each caught by its intended case and only that case — enqueue a `divergent` copy (the safety property, caught structurally via AST rather than grep); return None on a failed write; treat any branch-create failure as "already exists"; always send `sha=`; skip the existing-PR check. Against the LIVE fleet: --create-prs reports "nothing to remediate" and writes nothing, and with cli's genuinely-drifted stale-backlog.yml flipped to `required` it selects exactly that one file (blob 4e4246398130 vs canonical 14d689e) — verified with the writer replaced by a recorder, so nothing was written. shellcheck and actionlint clean. The workflow header said REPORT-ONLY and no longer is; updated in the same commit, along with the token scopes that implies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(selftest): hoist the remediation-test imports to the top (E402) ruff E402: the new remediation cases imported ast/inspect/textwrap/os mid-file. Hoisted and the underscore aliases dropped with them. Verified against the pinned toolchain rather than a local one: ruff 0.15.20 with --select E4,E7,E9,F, which is what code-quality.yml runs. My first local pass used a newer ruff with default rules and reported UP037 on pre-existing lines while missing this — the wrong version answering a different question. That is backend#1606 in miniature: `.github` has no `make check`, so there is no local command that runs what CI runs, and the first honest answer arrives red on a PR. Selftest still 132 pass / 0 fail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(conformance): survive the fresh-ref 404, and stop calling a write failure a read failure Two Bugbot findings on #227, both real. 1. THE COMMONEST DISPATCH WOULD HAVE FAILED. remediate_copies read the file on a just-created branch and took a 404 as proof of absence, then issued a sha-less PUT. For a DRIFTED copy the file exists on the base, so that 404 is the eventual-consistency window standards-sync.py already pays for -- and the sha-less write is rejected 422. Fresh branch + drifted file is the single most likely way this feature is ever invoked, and it would have failed every time. Ported _read_head_file's contract as _read_copy_on_head: `remediable` now records, per copy, whether the file exists on the base (drifted yes, missing no). When the base has it AND the ref was just created, a 404 cannot be true -- retry with backoff, fail closed if it never appears. When the base does not have it, one confirming re-read still guards against a single blip. A REUSED branch is deliberately NOT treated as fresh: it may have been cut before the file existed on the base, so its 404 is honest and permanent, and retrying-then-failing-closed would strand that repo forever. That is standards-sync's own #197, avoided rather than rediscovered. Found while fixing it: `branch_is_fresh` was referenced and never assigned. The new test raised it as a NameError rather than a reviewer finding it later. 2. EXIT 2 HAD TWO CAUSES AND ONE VOCABULARY. Remediation failures already exited 2, but every exit-2 message describes unread repos or schema failure, so a dispatch whose PRs failed to open was headlined on the conformance issue as "repos that could not be read are NOT known to comply" -- a true sentence about something that did not happen, sending the reader at the wrong problem. The count is now its own step output and its own verdict line. Evidence: selftest 136 pass / 0 fail (from 132). Two mutations: treat every 404 as absence -> 2 red (both retry cases); treat a reused branch as fresh -> 1 red (the strand-forever case). All four exit-2/1/0 verdict paths rendered and checked. Pinned ruff 0.15.20 --select E4,E7,E9,F clean, actionlint and shellcheck clean. Live re-verify against the fleet still selects exactly cli's drifted stale-backlog.yml, now correctly tagged as present-on-base. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: drop the runtime scratch file that was committed by accident `wd-body.md` is written into the workspace by caller-drift.yml's watchdog step at runtime. It got picked up by a `git add -A` while I was rendering the body locally to test it. Harmless on a runner, but it would have been a checked-in file that looks like a report and is stale the moment it lands. Caught by another session diffing #227's file list against theirs, not by any check here — .gitignore now covers it so the next local render cannot repeat it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(caller-drift): honest verdict + exit-2 wording for remediation failures (Bugbot #227) - Remediation-failure verdict no longer claims 'the fleet was read successfully' when UNREADABLE>0 (both counts can be set together); it names the unreadable tally so the headline can't contradict the numbers beside it. - Exit-2 tracking-issue comment and the final fail step now name remediation failure as a cause of exit 2, not just inventory/read failure, so a failed create-prs dispatch isn't misdiagnosed as 'repos could not be read'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(caller-drift): create-prs token needs the workflow scope for .github/workflows/ writes (Bugbot #227) The callers create-prs writes live under .github/workflows/, which GitHub refuses to write with Contents:RW alone — it needs the separate 'workflow' scope (classic PAT) or fine-grained Workflows:write. The note claimed the same scopes as standards-sync.yml, but that writes CLAUDE.md, not workflow files. Without this the documented token fails every remediation PUT closed and opens no PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(caller-drift): exit-2 verdict names the real cause, not just unread repos (Bugbot #227) die() exits 2 for a bad inventory or a failed org enumeration too, where UNREADABLE is 0 — the old code-2 verdict always blamed 'repos that could not be read', misdirecting the reader. Now the base verdict is cause-agnostic and a refinement names the actual cause from the UNREADABLE count (read failure vs a die-path that couldn't run to completion). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
#235) * chore(build): add a Makefile so local checks predict CI tracebloc/.github runs real CI — actionlint.yml, the org code-quality suite against itself, four gate selftests and conformance-gate.yml — and had no Makefile, so there was no single local command that predicts it. That is the gap backend#1606 exists to close. `make check` (~19 s measured, green) runs the fast, offline subset with the same tools and the same flags as the workflows that own them: ruff --isolated --select E4,E7,E9,F . (code-quality.yml's no-repo-config fallback, reproduced not approximated) shellcheck --severity=error --format=gcc --exclude=SC1091, over the gate's OWN file selection (extension or shebang, .bats/.ps1 skipped) rather than a looser local glob house-rules ./scripts/house-rules.sh --all action-pins EXTRACTED from code-quality.yml's own python heredoc, so a supply-chain gate cannot have a second copy that disagrees with the one gating merges actionlint -no-color -oneline -shellcheck shellcheck selftests caller-drift, blocked-marker, standards-sync, version-bump-gate — all four, which CI runs only behind paths: filters Two CI steps are deliberately NOT in `check`, each named in the file with the reason: gitleaks is in `check-all` (CI installs it per run from a pinned tarball; it is on no dev machine by default, and a credential scan that gets quietly skipped reports clean), and conformance-gate.yml is in neither because it polls the API for a verdict on a pushed head sha — there is no sha before you push. `make audit` runs caller-drift's token-needing half on demand. `setup` preflights the tools and installs a pre-push hook that runs `make check`; it installs nothing, because this repo has no venv or lockfile to install a pin into. Refs backend#1606. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(build): pass caller-drift's args explicitly, as the workflow does The audit target invoked `caller-drift.py` bare. Both --inventory and --source-dir default to exactly what caller-drift.yml passes, so it was equivalent today — but this file's claim is that its commands are COPIED from the workflow, not that they happen to agree with it. A default is precisely the kind of thing that moves under you, and the drift would be silent. Refs backend#1606. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(build): shellcheck target must fail closed when git ls-files errors (backend#1606) The shellcheck recipe piped `git ls-files` into a `while` loop under `set -e`. A pipe's exit status is the last stage's (the while loop), so a failed `git ls-files` was swallowed: the selection file stayed empty, the recipe printed 'no shell files in scope' and exited 0 — a clean report with nothing scanned. Materialize the listing to a temp file first (as code-quality.yml does), so a failed listing aborts the recipe non-zero instead of reporting a false clean. * chore(build): selftest-blocked-marker must also run the self-reference check (backend#1606) The Makefile target ran only blocked-marker-selftest.py, but the CI selftest job (blocked-gate-selftest.yml) also runs blocked-marker.py against a title containing the gate's own filenames — the only coverage for a self-reference bug that matches the word 'blocked' inside a path rather than in prose. Without it, make check could report green on a matcher change that would fail the gate's selftest in CI. Mirror the second step so local checks predict CI. * chore(build): version-check must not warn on a v-prefixed actionlint (backend#1606) version-check compared actionlint -version's first line verbatim to ACTIONLINT_VERSION (1.7.12). Release and Homebrew builds print '1.7.12', but 'go install' — the Linux hint on guard-actionlint — prints 'v1.7.12' from Go build metadata, so a matching toolchain still warned. Strip a leading v before comparing, the same cries-wolf class already handled for ruff. Verified: make version-check is clean with 1.7.12 on PATH, and v1.7.12 normalizes to 1.7.12. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…eck (#236) Adds one property family to repo-inventory.yml and the guard that reads it: `quality_files`, the files every repo must CARRY at a fixed path. Two members, both measured develop-first on 2026-08-12: CLAUDE.md (19/20) and .cursor/BUGBOT.md (17/20). Both are read by a TOOL rather than by a reviewer who would notice them missing, so a repo without one does not go red - it quietly gets worse review than its peers, and nothing in the org reported that until now. Presence is not the whole assertion: a required file must be a REGULAR file and NON-EMPTY. A zero-byte CLAUDE.md and a symlink both satisfy "the path exists" while carrying no guidance, which would make the family inert on arrival. Fail-closed, per this guard's first design rule. The facts come out of the tree read_repo already fetches, so a 403, an unparseable payload, a truncated tree or a blob whose size the API did not report all return exit 2 with the row recorded unreadable. There is no path from a failed read to "the file is absent". Three exemptions, each with a written reason: claude-skills, rfcs .cursor/BUGBOT.md absent - UNREMEDIATED, shared anchor devex-bootstrap both files absent - reuses devex_bootstrap_undisposed, widened from protection-only to cover the same open disposition question (backend#1597) TWO THINGS DELIBERATELY NOT DONE. `quality / gitleaks` armed as a required status check needs NO new family: it is already asserted by protection_policy.required_checks on develop, staging and prod. Verified live - 16/16 train repos have it armed and 16/16 are asserted, with zero mismatches. A parallel family would duplicate a live assertion and then drift from it. Mutation-checked instead: neutralising the required_checks comparison turns the suite red, so that mechanism is not inert. `.gitleaks.toml` is not modelled. It is 7/20 and that is correct - a per-repo allowlist you add on a false positive, not a control. Modelling it would add nine exemption rows for zero security value and make a tuning file read as a security gap, which is the inert-verification pattern backend#1729 exists to catch. Verified: make check green (ruff, shellcheck, house-rules, action-pins, actionlint, four selftests); selftest 136 -> 160 cases, all passing; the live audit exits 0 with all 20 repos OK in the new column; 11 mutations applied one at a time, all 11 caught. backend#1608 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 13, 2026
ContributorAuthor
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
…lls exemption (#241) * fix(inventory): drop archived devex-bootstrap, unstale the claude-skills exemption The caller-drift audit is red on develop with 2 findings, and it is a REQUIRED check (`gate`) for any PR touching a contract file — so every PR that edits caller-drift.py, its selftest, repo-inventory.yml or the two workflows inherits an unrelated red and cannot merge. That is what blocks #240. Neither finding is about the code: - devex-bootstrap is in repo-inventory.yml but is no longer an active repo. Verified: `archived=true`. Its entry existed to hold open the disposition question in backend#1597 ("joins the train, stays a scratch repo, or is archived") — that question now has an answer, so the whole 86-line block and its exemptions go with it. Confirmed with Lukas. - claude-skills marked `.cursor/BUGBOT.md` exempt via *bugbot_guide_missing, but the file exists (8,949 bytes). The exemption outlived its reason, which is the failure mode the audit's stale-exemption check exists to catch: an exemption nobody revisits reads as "not required here" forever. Kept deliberately separate from #240's caller-drift.py fix: this is inventory data, that is script behaviour, and this one unblocks every contract-touching PR rather than just mine. * fix: remove every devex-bootstrap remnant, not just the inventory entry All three Bugbot findings on this PR, and all three the same mistake: I deleted the entry and left its references. HIGH -- scripts/standards-sync.py kept an EXEMPT entry naming devex-bootstrap. Design rule 3 in that file makes a stale exemption a HARD FAILURE ("an exemption naming a repo the inventory does not know is itself a failure"), so `load_targets` would have refused and taken the whole scheduled org-standards audit down -- every repo unevaluated, not one repo skipped. EXEMPT is now empty, with the reason recorded: backend#1597 item 3 was answered by archiving the repo. Its selftest named devex-bootstrap as the exemplar exempt repo, so emptying EXEMPT broke two cases. Rewritten to inject their own fixture entry and restore the real one in a finally: they test the MECHANISM, and a mechanism test should not break every time the fleet changes -- which is exactly what just happened. MEDIUM -- the `bugbot_guide_missing` anchor's prose still cited claude-skills alongside rfcs, after claude-skills stopped aliasing it. Hand-maintained citation lists rot the moment usage moves; now only rfcs, which is the only alias left. LOW -- the `devex_bootstrap_undisposed` anchor definition survived with zero aliases, still asserting the disposition was open. Removed, along with `no_workflows_directory_at_all` (also zero aliases after the entry went) and the dangling "Distinct from devex_bootstrap_undisposed" cross-reference it left behind in the bugbot anchor. Zero `devex` references remain in the repo. standards-sync selftest 27 checks / 0 failed; caller-drift selftest 160 / 0; ruff clean; YAML parses at 19 repos.
…the canon (#240) Bugbot on the staging promotion PR #238, fixed on develop so the train re-prepares rather than pushing onto the promotion PR. `remediate_copies` read each copy's blob SHA and then always PUT, never comparing what the branch already carried against the canonical bytes. A second `--create-prs` dispatch therefore re-writes identical content, which the API answers with 409 (or loses a sha race), and remediation records a failure -- the audit goes red as if the fleet could not be written, while the open PR already carries the canon. standards-sync.py skips the write in exactly this case (`if current is not None and current == desired`), which is what makes its 422 branch reuse genuinely idempotent; this is the same fix in the same shape. - `_read_copy_on_head` now returns (sha, decoded bytes, error) instead of just the sha, so the caller can tell an already-correct copy from a drifted one. - the loop skips the PUT when the branch content equals the canon, and still falls through to `_ensure_copy_pr` -- otherwise the first run's work would never become reviewable. - `import binascii`, since a malformed blob now decodes here and b64decode raises binascii.Error rather than ValueError alone. The selftest stubs returned a bare `"existingsha\n"`, modelling the old `--jq .sha` call, so the read now JSON-decodes what the real API sends. Both remediation stubs return `{sha, content}` and take the branch's current body, which is what made the skip testable at all. New cases: an already-matching copy is not re-written, and the PR is still ensured when every write was skipped. Selftest 162 pass / 0 fail (was 160); ruff clean. Removing the skip fails the new case.
LukasWodka
commented
Aug 13, 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 c66c246. Configure here.
LukasWodka
commented
Aug 13, 2026
ContributorAuthor
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
…be read" (#242) Bugbot Medium on the staging promotion PR #238. `unreadable` is the MERGED list -- protection and ruleset read failures are folded into it (caller-drift.py:2000-2001). The watchdog headline read that count as repos that were never read, so a clean caller/copy/quality audit with a single failed protection or ruleset read was announced as "N repo(s) could not be read and are NOT known to comply": a true count under a false name, sending the reader to look for repos nobody had touched. The script already decomposes this correctly for its own report and issue body (the `caller_failed = len(unreadable) - protection - ruleset` split at :2035 and :2152). Only the workflow consumed the merged number. So this exports the split that already existed rather than inventing one -- same move as `remediation_failures`, which got its own output for exactly this reason (#227). Adds `caller_unreadable`, `protection_unreadable` and `ruleset_unreadable` outputs; the watchdog composes them into one clause naming each failing read type, falling back to the merged count so a future read type cannot produce an empty sentence. FIXED IN BOTH PLACES. The "Remediation failed" verdict carried the same wording, so the phrase is now built ONCE above the verdicts and quoted twice -- fixing only the reported line would have left its sibling saying "repo(s) could not be read", which is the exact half-fix this repo keeps re-learning. Verified by running the composed shell over every shape: caller-only -> "3 repo(s) could not be read (caller/copy state UNKNOWN)" protection-only -> "2 branch-protection read(s) failed (protection state UNKNOWN)" ruleset-only -> "1 ruleset read(s) failed (ruleset state UNKNOWN)" all three -> the three clauses, joined split absent -> "7 read(s) failed" (fallback) caller-drift selftest 162 pass / 0 fail; actionlint clean; ruff clean.
LukasWodka
commented
Aug 13, 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 91d4a0b. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka added a commit
that referenced
this pull request
Aug 17, 2026
…r-read failure (Bugbot, #278) Right again, and the comment at the site I broke already explains the class: "a true count under a false name, pointing at the wrong problem. (Bugbot, #238.)" That decomposition exists precisely so a reader is sent to the right fix; adding a fourth bucket without updating the subtraction reintroduced it, and the wrong fix this one points at is "debug the caller reads" when the answer is "widen the App installation". Exit stayed fail-closed throughout, as Bugbot noted. Only the headline lied. - decide_exit() takes `listing_unreadable` and gives it its OWN clause, so all four causes are named separately instead of three named and one absorbed. - The caller/copy count is derived ONCE, by `caller_read_failures()`, and passed to both consumers. It was two inline subtractions, which is how a new bucket landed in one and not the other. - Variadic and clamped: buckets are passed positionally so a new one is either passed or visibly missing, and a double-counted bucket clamps at 0 rather than producing a negative headline. ON COVERAGE, because the first attempt was vacuous and saying so is the point. Breaking caller_read_failures() reddens its cases. Dropping the bucket AT THE CALL SITE did not -- 189 passed under that mutation, and a call-site omission is exactly what the bug was. A test that survives the mutation it is meant to catch is not coverage. So the wiring is asserted from the SOURCE: every `*_unreadable` bucket main() declares must appear in the call. Derived from the declarations, so a fifth bucket is covered the moment it is declared rather than when someone remembers to test it. Both mutations now redden -- the call-site one names the missing bucket. 190 pass / 0 fail (was 185). Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka added a commit
that referenced
this pull request
Aug 17, 2026
…(Bugbot, #278) Third consumer of the decomposed counts, third time I updated some and not all. decide_exit, the step outputs and the watchdog phrase were each edited separately, and each was missed once. That is not three mistakes so much as one missing guard. The phrase builder covered caller/protection/ruleset and fell through to the merged count otherwise -- but the fallback fires only when the sentence is ENTIRELY empty, so a listing gap MIXED with any other cause was dropped from the headline rather than mis-named. The reader is then told the wrong thing to go fix, which is exactly what the decomposition exists to prevent (Bugbot #238, and now twice more). Fixed: `listing_unreadable` is wired into the step env and gets its own clause, naming "fleet coverage UNKNOWN" -- whose fix is "widen the App installation", which no other clause would ever suggest. AND THE GUARD, because a fourth miss is otherwise a matter of time. Two assertions, both derived from what the script EMITS rather than from a list a human maintains: 1. every `*_unreadable` output the script writes is read by the workflow 2. every one of them gets its OWN clause in the watchdog phrase They catch different failures, which is why both exist: removing the env wiring trips (1) only; keeping the env and deleting its clause trips (2) only -- and (2) is the shape that just happened. Mutation-proved in both directions. A fifth bucket is now covered the moment it is written out, rather than when someone remembers three separate consumers. 192 pass / 0 fail (was 190). Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka added a commit
that referenced
this pull request
Aug 17, 2026
…eps the PAT by decision (#278) * feat(2036): the four org audits authenticate as the App (backend#2036) Ten token sites across four workflows, all in this repo: caller-drift 4 standards-sync 3 merge-settings-drift 2 bricked-prs 1 None of these gates a promotion, so they are the safe half of what remains. `fr-gate` is deliberately NOT here: it is a required check on every promotion, so if its board read breaks you cannot promote the fix. It gets its own PR and a quiet window. WHY THIS ORDER. Of the six workflows still on PROJECTS_KANBAN_TOKEN, five live in this repo alone and one -- add-to-kanban.yml -- is a per-repo COPY in 19 repos. Doing the five single-repo ones first takes the migration from 8/14 to 13/14 for roughly two PRs, and leaves the expensive 19-repo sweep as the only remaining piece rather than mixing it in. THE TWO "token is present" GUARDS ARE KEPT, pointed at the minted token. A failed mint fails its own step, so they look redundant -- but what they actually guard is an EMPTY token reaching the audit, and empty is the fail-OPEN direction: the org listing returns nothing and every check passes against an empty scope. That reasoning does not change with the token's provenance, so the assertions stay. Deleting them because the new path "can't fail" is how a real guard becomes an inert one. NO REPO-ADMINISTRATION PERMISSION IS NEEDED, and merge-settings-drift is why that is worth stating. It contains `gh api -X PATCH repos/...` three times, which reads like it mutates repo settings. It does not: those lines are inside `printf`, and are remediation commands the report PRINTS for a human to run. Granting the App `administration: write` for a report-only cron would have been a serious over-grant, and the grep that suggested it was wrong. standards-sync's header planned a separate fine-grained STANDARDS_SYNC_TOKEN in case PROJECTS_KANBAN_TOKEN lacked contents-write. Moot: the App carries contents:write and pull_requests:write, and a second fleet-wide credential is the thing backend#2036 exists to stop. Header updated to say so. `owner:` on every mint makes the token ORG-scoped -- a repo-scoped token cannot enumerate the org, which is the whole job of these four. No fallback to the PAT anywhere: a fallback would let a broken App path keep reporting green, and an audit that cannot fail is the exact defect backend#1729 catalogued. After this, only `add-to-kanban.yml` (19-repo copy) and `fr-gate.yml` remain on the PAT. PROJECTS_KANBAN_TOKEN stays live and untouched until both land. Verified: actionlint clean; all four parse; caller-drift-selftest 176 pass / 0 fail. Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(merge-settings-drift): a non-empty org listing is not a complete one (Bugbot, #278) The zero-repo guard was sufficient while this ran as an org-member PAT, whose listing was the whole org by construction. An App installation token sees only what the installation covers, so narrowing the installation -- or adding a repo to the org and not to the App -- silently shrinks the audited set while every remaining repo passes and the job exits green. That is the exact fail-open this audit exists to refuse, reintroduced by the credential change rather than by any logic here. The other three migrated audits avoid it because they drive from repo-inventory.yml; this one enumerated and trusted the enumeration. So compare the listing against the DECLARED set: any repo repo-inventory.yml declares that the listing did not return means the listing is incomplete, not that the repo complies. Derived from the declaration, never from a second hand-maintained list. An extra repo in the listing is NOT a finding here -- a repo in the org but not yet in the inventory is caller-drift's business, and duplicating that assertion would put the same rule in two places to drift apart. A declared repo can also vanish from the listing by being archived, since the filter drops archives. Still a finding: an archived repo carrying an inventory entry is drift someone should look at, and refusing costs less than guessing which case it is. The message names both causes. Adds actions/checkout (for repo-inventory.yml) and actions/setup-python, same pins and version as the sibling audits. setup-python rather than the system interpreter because a bare pip install on the runner hits PEP 668. Verified by extracting the generated `run:` script from the YAML and checking it for real, rather than trusting that the heredoc survives block-scalar indentation stripping: `bash -n` clean, terminator resolves, actionlint clean. Guard proved in three directions -- complete listing passes, a listing missing 3 declared repos fails and names them, a listing with an extra repo passes. NOTE: this does not turn #278 green. The `audit` job still reports 6 findings from ruleset `bypass_actors` the App cannot see; that is the open question on saadqbal's thread and is not fixed here. Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(caller-drift): an absent bypass_actors is unreadable, not an empty allowlist (Bugbot, #278) TWO findings, and the first is a fail-open I missed while diagnosing the false-red beside it. 1. ABSENT IS NOT EMPTY. GitHub returns `bypass_actors` only to a caller with WRITE access to the ruleset ("to prevent leaking sensitive information"); everyone else gets a 200 with the field simply not there. `raw.get("bypass_actors") or []` folded that into an empty allowlist -- and `promotion_merge_commit_only` asserts exactly `bypass_actors: []`, so the assertion MATCHED without the field ever having been read. Fail-open on the allowlist this audit exists to enforce, on all 32 promotion branches. The tag rulesets fail the other way and go falsely red, which is the 6 findings on this PR -- same root cause, opposite symptom, and only the red one was visible. `read_rulesets` now refuses a ruleset whose `bypass_actors` was withheld, rather than each comparison having to remember to check. Same rule the docstring already stated for the cheaper endpoints, applied to the case where the RIGHT endpoint answers 200 and still withholds the field. Unreachable under the org-admin PAT, which always saw the field. backend#2036 moved this onto a least-privilege identity and made it reachable. 2. A DECLARED REPO MISSING FROM THE LISTING IS NOT PROOF IT LEFT THE ORG. Under the org-member PAT the listing WAS the org, so `inventory - active` could only mean archived/renamed/deleted and "remove the entry" was sound advice. An App token lists only what the installation covers, so that set now also holds repos that were never read -- and following the old advice would delete a legitimate entry and permanently shrink the audited fleet. Probes each one instead of guessing: readable-and-archived/fork stays a finding; readable-and-active means the LISTING was incomplete; unreadable means we cannot tell. The last two suppress an all-clear instead of manufacturing a finding. Bugbot correctly noted the previous commit applied this rule to merge-settings-drift's listing and left the identical flaw in its sibling. Three selftest cases, and the middle one is the mutation anchor: present-and-empty must stay a real assertable value, or the fix would make every legitimate empty allowlist unreadable and break the 32 branches it protects. MUTATION PROVED, not assumed. Forcing `bypass_present = True` (the old behaviour) reddens all three new cases, and the mutant emits `bypass actors: missing ['OrganizationAdmin', 'Team:18304481']` -- byte-for-byte the symptom seen on this PR. Restored: 179 pass / 0 fail (was 176). CONSEQUENCE, stated rather than discovered later: with this fix the App identity can no longer evaluate rulesets AT ALL -- the 6 findings become ~32 unreadable records. The run stays red, honestly now instead of half-silently. Whether caller-drift can run as the App is the open question on saadqbal's thread; this commit does not answer it, it just stops the answer being faked. Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(caller-drift): listing gaps must not distort the coverage count (Bugbot, #278) Right, and it would have aborted exactly when the probe mattered most. `evaluated` drives a die() that discards the whole report, and it is computed as `len(audited) - len(names in unreadable)`. That arithmetic has one precondition, stated in the comment above it: every name in `unreadable` is an AUDITED one. The stale-inventory probe I added records repos from `inventory - active`, which are by definition not in `audited`, so each one subtracted a name that was never in the total. Enough listing gaps and `evaluated <= 0` fires on a run with plenty of verdicts -- and "enough" is the partial-installation case the probe exists to describe. Two changes: 1. Listing gaps get their OWN bucket, `listing_unreadable`, merged in only after `evaluated` is computed. Exactly the pattern protection and rulesets already use, for exactly the reason their comment gives. I should have followed it. 2. The arithmetic is extracted to `coverage()` and made tolerant rather than trusting: only names actually in `audited` count against it. The precondition stays the caller's to keep, but honouring it in one place means a future fourth bucket wired to the wrong list cannot silently abort every run. Extracted rather than fixed inline because of backend#1729 rule 9: a mutation check must call the code under test, not a copy. Asserting this sum inline in the selftest would have the test agree with its own arithmetic while production drifted. Now the selftest and the mutation both go through `coverage()`. Four cases, including both directions: a genuinely unreadable AUDITED repo must still reduce coverage to zero, or the fix would trade a false abort for a run that can never abort at all. MUTATION PROVED. Restoring `len(audited) - len(seen)` reddens the listing-gap case and the zero case, and the mutant returns -2 -- negative coverage, which is the `evaluated <= 0` that trips the die(). Restored: 183 pass / 0 fail (was 179). Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(caller-drift): a listing gap is its own exit-2 cause, not a caller-read failure (Bugbot, #278) Right again, and the comment at the site I broke already explains the class: "a true count under a false name, pointing at the wrong problem. (Bugbot, #238.)" That decomposition exists precisely so a reader is sent to the right fix; adding a fourth bucket without updating the subtraction reintroduced it, and the wrong fix this one points at is "debug the caller reads" when the answer is "widen the App installation". Exit stayed fail-closed throughout, as Bugbot noted. Only the headline lied. - decide_exit() takes `listing_unreadable` and gives it its OWN clause, so all four causes are named separately instead of three named and one absorbed. - The caller/copy count is derived ONCE, by `caller_read_failures()`, and passed to both consumers. It was two inline subtractions, which is how a new bucket landed in one and not the other. - Variadic and clamped: buckets are passed positionally so a new one is either passed or visibly missing, and a double-counted bucket clamps at 0 rather than producing a negative headline. ON COVERAGE, because the first attempt was vacuous and saying so is the point. Breaking caller_read_failures() reddens its cases. Dropping the bucket AT THE CALL SITE did not -- 189 passed under that mutation, and a call-site omission is exactly what the bug was. A test that survives the mutation it is meant to catch is not coverage. So the wiring is asserted from the SOURCE: every `*_unreadable` bucket main() declares must appear in the call. Derived from the declarations, so a fifth bucket is covered the moment it is declared rather than when someone remembers to test it. Both mutations now redden -- the call-site one names the missing bucket. 190 pass / 0 fail (was 185). Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(2036): THREE audits authenticate as the App; caller-drift keeps the PAT by decision Decision: option B (Lukas, 2026-08-17). This PR started as four audits and ships three, because the fourth structurally cannot use a least-privilege identity. standards-sync, merge-settings-drift and bricked-prs authenticate as the tracebloc-release-train App. caller-drift does not, and that is now written down at the step where someone would otherwise "finish" the migration. WHY caller-drift CANNOT. It is the only audit asserting ruleset bypass allowlists, and GitHub returns `bypass_actors` only to a caller with WRITE access to the ruleset ("to prevent leaking sensitive information"). Measured, not assumed: `administration: read` was granted mid-review and fixed all 52 branch-protection reads while leaving every `bypass_actors` withheld. The only sufficient grant is `administration: write`, which would let an App invoked by ~14 workflows on every PR and push REWRITE branch protection and every ruleset in the fleet -- including the `v*` tag trust root whose bypass list this audit exists to police. Granting write over the trust root to read who may bypass it makes the auditor one of the actors it audits. So #2036 closes with the PAT alive for exactly one consumer, and that is a better end state than the alternative. It is also the safest place for a privileged credential to remain: a weekly cron, never event-triggered, so nothing an outside contributor submits can influence when it runs, and it never writes. The failure that filed #2036 -- advance-deploy-env exhausting the quota mid-hop and stranding a card -- was a hot-path, per-push problem, and that is fixed. THE FIVE CORRECTNESS FIXES STAY, and they are the real value here. All were found by Bugbot during review, all are independent of which credential runs the audit, and three were bugs I introduced while fixing the previous one: 1. merge-settings-drift trusted a non-empty org listing as complete 2. an absent `bypass_actors` became `[]`, silently satisfying the promotion_merge_commit_only assertion on all 32 promotion branches -- a fail-open that was unreachable under the org-admin PAT 3. a declared repo missing from the listing was reported as having left the org, advising removal of a legitimate inventory entry 4. listing gaps distorted `evaluated` and could trip the die() that discards the whole report, precisely in the partial-coverage case the probe describes 5. listing gaps were counted as caller/copy read failures, so the headline named the wrong fix Selftest 176 -> 190. Every new case mutation-anchored, and one first attempt was found VACUOUS by mutation -- it passed under the exact change it existed to catch, so the wiring is now asserted from the declarations rather than from memory. Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(caller-drift): the watchdog headline must name a listing gap too (Bugbot, #278) Third consumer of the decomposed counts, third time I updated some and not all. decide_exit, the step outputs and the watchdog phrase were each edited separately, and each was missed once. That is not three mistakes so much as one missing guard. The phrase builder covered caller/protection/ruleset and fell through to the merged count otherwise -- but the fallback fires only when the sentence is ENTIRELY empty, so a listing gap MIXED with any other cause was dropped from the headline rather than mis-named. The reader is then told the wrong thing to go fix, which is exactly what the decomposition exists to prevent (Bugbot #238, and now twice more). Fixed: `listing_unreadable` is wired into the step env and gets its own clause, naming "fleet coverage UNKNOWN" -- whose fix is "widen the App installation", which no other clause would ever suggest. AND THE GUARD, because a fourth miss is otherwise a matter of time. Two assertions, both derived from what the script EMITS rather than from a list a human maintains: 1. every `*_unreadable` output the script writes is read by the workflow 2. every one of them gets its OWN clause in the watchdog phrase They catch different failures, which is why both exist: removing the env wiring trips (1) only; keeping the env and deleting its clause trips (2) only -- and (2) is the shape that just happened. Mutation-proved in both directions. A fifth bucket is now covered the moment it is written out, rather than when someone remembers three separate consumers. 192 pass / 0 fail (was 190). Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(caller-drift): diagnostics must name the credential this audit actually uses (Bugbot, #278) The same class one more time, and this one I wrote INTO the fix for the previous one. Option B kept caller-drift on PROJECTS_KANBAN_TOKEN, but the diagnostics were written while it was being migrated and still named an App installation token this audit never mints: - the empty-token guard announced "The installation token is empty" while reading the PAT - listing-gap records told the operator to "widen the App installation" - decide_exit's clause comment said the same A real failure under the wrong fix -- exactly what the decomposition work in this PR keeps closing, aimed at itself. Fixed by role, not by name: - caller-drift.yml names PROJECTS_KANBAN_TOKEN, because that is what it reads and an operator grepping for the empty secret needs the real name - caller-drift.py's messages are CREDENTIAL-AGNOSTIC ("this audit's token", "the token's repo visibility"), because the script has now outlived one migration inside a single PR and will outlive others - merge-settings-drift is untouched: it IS on the App, so naming the App installation there is correct and remains No behaviour change; every assertion and exit code is identical. This is the sentence a human reads at 3am deciding what to go fix, which is the only reason the decomposition exists at all. 192 pass / 0 fail. Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(selftest): the wiring guard was blind to the bucket it was built to protect (Bugbot, #278) The sharpest one in this PR: the guard added to stop a bucket being dropped could not see one of the four buckets, and it was the bucket that had already been dropped three times. `_emitted` matched `handle.write(f"x_unreadable=` on a single line. `caller_unreadable` is written across two, so it never entered the set. Measured: _emitted (old regex): listing, protection, ruleset <- 3 of 4 actual output writes: caller, listing, protection, ruleset So deleting CALLER_UNREADABLE from the workflow env left ALL THREE wiring assertions green. A guard blind to a quarter of what it guards is worse than no guard, because it reports coverage it does not have -- which is the whole subject of backend#1680 and the reason these assertions exist at all. Keyed on the EMITTED STRING LITERAL now (`"x_unreadable=`) rather than on the shape of the call wrapped around it. The literal is what actually reaches GITHUB_OUTPUT, so it is the thing the workflow consumes; matching the call shape was matching the formatting, and formatting is not the contract. FULL MUTATION MATRIX, 4 buckets x 2 directions, all 8 now caught: drop the env wiring caller/protection/ruleset/listing -> all FAIL drop the watchdog clause caller/protection/ruleset/listing -> all FAIL Before this commit, both caller_unreadable rows passed. 192 pass / 0 fail; the count is unchanged because this fixes what the existing assertions SEE, not how many there are -- which is precisely why it needed the mutation matrix to find rather than a passing suite to confirm. Refs backend#2036 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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
High Risk
Manual dispatch with
create-prswrites workflow files across the fleet and needs elevated token scopes; watchdog issue edits and inventory/schema changes affect org-wide conformance signaling for every repo.Overview
Org conformance grows a
quality_filesfamily inrepo-inventory.ymlandcaller-drift.py: every repo must carry non-empty regular files atCLAUDE.mdand.cursor/BUGBOT.md(presence only; no auto-fix). The conformance matrix adds a column;devex-bootstrapis dropped from the inventory;rfcsrecords a missing Bugbot guide as an explicit exempt finding.caller-drift.ymlcan optionally open remediation PRs for drifted/missingrequiredworkflow copies viaworkflow_dispatch+create-prs(never on cron/PR). Scheduled/manual runs rewrite backend#1781 with the current matrix and verdict (including split unreadable counts and remediation-failure wording); drift comments move off closed #1415.standards-syncclears the archived devex-bootstrap exemption.A new
Makefileexposescheck/check-all/setuptargets aligned with existing workflows (lint, selftests, optional gitleaks and liveaudit)..gitignoreignores watchdog scratch fileswd-body.mdandbody.md.Reviewed by Cursor Bugbot for commit 91d4a0b. Bugbot is set up for automated code reviews on this repo. Configure here.