From e1bce830b05102703eeda288ea7d8e7ebf4bc94b Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Thu, 13 Aug 2026 16:21:32 +0200 Subject: [PATCH 1/7] feat(ci): watch for PRs bricked by a required check that never reports (#239) * feat(ci): watch for PRs bricked by a required check that never reports A required status check that never runs leaves its context at "Expected - waiting for status to be reported". The PR is then approved, has nothing red to point at, and is permanently unmergeable -- the one CI failure mode with no red signal at all. Nobody is notified and no reviewer sees a problem, because there is no failure, only an absence. Nothing inside a PR can detect that, so it needs a watcher. EMPIRICAL, NOT STATIC. Reasoning about "can this check ever report here" means modelling path filters x matrix expansion x reusable inputs x their defaults x `if:` expressions; run across 19 repos that produced three separate classes of false positive before it produced anything true. Comparing a branch's required contexts against the contexts actually PRESENT on the PR needs none of it: required-but-absent is bricked, whatever the cause. Required contexts come from caller-drift.py's `read_protection`, imported rather than reimplemented -- it already unions classic protection with rulesets, and a ruleset-only branch 404s on the classic API (backend#1276). A second copy is how one of them silently stops reading half the picture. THE DISTINCTION THAT MAKES IT READABLE, learned the hard way here: an absent context and a not-yet-started one are identical at a glance. The first version reported release-train#67 bricked three minutes after a push, and it had every context minutes later. A head younger than 60 minutes is not judged, and an unreadable age counts as young -- a false brick is what makes a report ignorable, which is the failure mode of the always-red check one repo over. FOUND A FOURTH CAUSE ON ITS FIRST REAL RUN, one the ticket does not list: a CONFLICTED PR. GitHub cannot compute a merge commit, so pull_request workflows never run and every required context stays absent. release-train#67, 82 minutes after its last push, had 0 workflow runs on its head sha. The fix is a rebase, not a protection change, so the report names that cause separately. Fails closed throughout: an unreadable branch, age or PR list is reported as "could not audit" and exits 2. Reporting "0 bricked" while part of the fleet was never read is the shape this exists to remove. Evidence: 10-case offline selftest passes; full-fleet run over all 20 inventory repos takes 94s and reports exactly one finding (release-train#67, above). Closes tracebloc/backend#1721 * fix(ci): the watcher's own fail-open paths (Bugbot, .github#239) Two findings, both the shape this watcher exists to report, in the watcher. A CAPPED PR LIST READ AS CLEAN. `gh pr list` truncates at --limit and says nothing about it, so a repo with more open PRs than the cap was audited PARTIALLY and reported clean for the ones it never saw. The cap is now 200 -- high enough that no repo here approaches it, the busiest having ~10 open PRs against one base -- and reaching it raises "could not audit" instead of returning a partial view. THE GRACE WINDOW USED THE WRONG CLOCK. A commit's committer date is when it was WRITTEN. A force-push can put a long-dated commit on a branch a second ago, which would then be judged immediately -- producing exactly the false "bricked" the window exists to prevent. The honest clock is when CI first saw the head: the OLDEST check suite on the sha, since GitHub creates a suite per app as soon as it has work for that head. The commit date remains as a fallback for a head with no suite at all, which is the conflicted case -- there, no suite will ever exist and the commit date is the only clock there is. Also drops an unused `timedelta` import that failed `quality / ruff`. 13 selftest cases, up from 10. The two new ones needed the real functions put back first: earlier cases swap `open_prs` and `head_age_minutes` out wholesale, so a later case that forgets silently tests the previous case's stub -- this file's own subject matter, and it caught me on the first run. * fix(ci): an unreadable check-suites read is young, not the commit date A failed check-suites API read (502/403/rate-limit) fell through to the commit committer date, which a force-push can set to any past instant -- so an unreadable CI clock plus an old commit still reported BRICKED, the exact false-brick the grace window exists to prevent. Distinguish a failed read (return None -> caller treats the head as young and skips) from a genuinely empty one (conflicted PR, no suite will ever exist -> commit date is the only clock). Adds a selftest case for the failed-read path. Bugbot, .github#239. --------- Co-authored-by: tracebloc-release-train[bot] <309815517+tracebloc-release-train[bot]@users.noreply.github.com> --- .github/workflows/bricked-prs-selftest.yml | 46 +++ .github/workflows/bricked-prs.yml | 39 +++ scripts/bricked-prs.py | 357 +++++++++++++++++++++ scripts/tests/bricked-prs-selftest.py | 226 +++++++++++++ 4 files changed, 668 insertions(+) create mode 100644 .github/workflows/bricked-prs-selftest.yml create mode 100644 .github/workflows/bricked-prs.yml create mode 100755 scripts/bricked-prs.py create mode 100755 scripts/tests/bricked-prs-selftest.py diff --git a/.github/workflows/bricked-prs-selftest.yml b/.github/workflows/bricked-prs-selftest.yml new file mode 100644 index 0000000..f125f93 --- /dev/null +++ b/.github/workflows/bricked-prs-selftest.yml @@ -0,0 +1,46 @@ +name: Bricked PRs selftest + +# The watcher's product is a DISTINCTION, not a list: a context absent because +# it will never report, versus absent because the run has not started, versus a +# branch that could not be read. Get any of those wrong and the report is either +# ignorable or misleading -- both were observed while building it (backend#1721), +# so the paths are asserted rather than trusted. +# +# Same shape as blocked-gate-selftest.yml: offline, no token, path-filtered +# because it only needs to run when the thing it tests changes. + +on: + pull_request: + paths: + - scripts/bricked-prs.py + - scripts/tests/bricked-prs-selftest.py + - .github/workflows/bricked-prs.yml + - .github/workflows/bricked-prs-selftest.yml + push: + branches: [main, develop, staging] + paths: + - scripts/bricked-prs.py + - scripts/tests/bricked-prs-selftest.py + - .github/workflows/bricked-prs.yml + - .github/workflows/bricked-prs-selftest.yml + +permissions: + contents: read + +concurrency: + group: bricked-prs-selftest-${{ github.ref }} + cancel-in-progress: true + +jobs: + selftest: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + with: + python-version: '3.12' + # bricked-prs.py imports caller-drift.py for the protection reader, and + # that module hard-fails without PyYAML by design. + - run: pip install --quiet pyyaml + - run: python scripts/tests/bricked-prs-selftest.py diff --git a/.github/workflows/bricked-prs.yml b/.github/workflows/bricked-prs.yml new file mode 100644 index 0000000..8f7f9e5 --- /dev/null +++ b/.github/workflows/bricked-prs.yml @@ -0,0 +1,39 @@ +name: Bricked PRs + +# A required status check that never reports leaves a PR approved, with nothing +# red to point at, and permanently unmergeable (backend#1721). It is the one CI +# failure mode with NO red signal at all -- nobody is notified, and no reviewer +# sees a problem, because there is no failure, only an absence. So it needs a +# watcher; nothing inside a PR can detect it. +# +# Runs in tracebloc/.github only, like the other org-wide crons. + +on: + schedule: + # Every four hours. The failure is not urgent -- a bricked PR stays bricked + # -- but it is invisible, so the cost is the hours a human spends not + # realising. Four hours bounds that without adding noise. + - cron: "17 */4 * * *" + workflow_dispatch: {} + +permissions: + contents: read + +concurrency: + group: bricked-prs + cancel-in-progress: false + +jobs: + audit: + name: Required checks that never report + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - name: Install PyYAML + run: pip install --quiet pyyaml + - name: Audit + env: + # Needs to read branch protection, rulesets and PRs across the org. + GH_TOKEN: ${{ secrets.PROJECTS_KANBAN_TOKEN }} + run: python3 scripts/bricked-prs.py diff --git a/scripts/bricked-prs.py b/scripts/bricked-prs.py new file mode 100755 index 0000000..a957a74 --- /dev/null +++ b/scripts/bricked-prs.py @@ -0,0 +1,357 @@ +#!/usr/bin/env python3 +"""Find PRs bricked by a required check that never reports (backend#1721). + +THE FAILURE MODE. A required status check that never runs on a PR leaves its +context at "Expected - waiting for status to be reported". The PR is then: + + * approved, + * with nothing red to point at, + * and permanently unmergeable. + +It reads to the author as "approved but stuck". Nobody is notified, and no +reviewer sees a problem, because there is no failure -- only an absence. It is +the one CI failure mode with no red signal at all, which is why it needs a +watcher rather than a check. + +Three causes, all observed on this fleet: + + * a required context produced by a PATH-FILTERED workflow, on a PR touching + none of those paths (`client` / Source-of-truth drift bricked #651, #657, + #660); + * a check made required AFTER a branch was cut, so the PR's head never runs it + (`tracebloc-website` / quality / action-pins bricked #472); + * a required context NO workflow produces -- renamed job, deleted workflow, + typo in protection; + * a CONFLICTED PR. GitHub cannot compute a merge commit for a PR whose base + has moved incompatibly, so `pull_request` workflows never run at all and + EVERY required context stays absent. Found by this script on its first real + run: release-train#67, 82 minutes after its last push, had 0 workflow runs + on its head sha and only a Bugbot verdict (which reviews the diff, not via + a pull_request trigger). The remedy is a rebase, NOT a protection change -- + which is why the report names it separately. + +WHY THIS IS EMPIRICAL AND NOT STATIC. A static audit of "can this check ever +report here" must model path filters x matrix expansion x reusable-workflow +inputs x their defaults x `if:` expressions. Run across 19 repos it produced +three separate classes of false positive before it produced anything true, and +being wrong in the safe-looking direction is how a fleet gets reported healthy +while PRs sit stuck. + +Comparing the branch's required contexts against the contexts actually PRESENT +on the PR needs none of that modelling. Whatever the cause, required-but-absent +is bricked. + +Required contexts come from `caller_drift.read_protection`, which unions classic +protection with rulesets. That is imported rather than reimplemented on purpose: +a ruleset-only branch 404s on the classic API (backend#1276), and two copies of +that logic is how one of them silently stops reading half the picture. + +Exit codes, mirroring caller-drift.py: + 0 every repo read, no PR is missing a required context + 1 at least one PR is bricked + 2 could not evaluate -- an unreadable repo, branch, or PR list +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import os +import sys +from datetime import datetime, timezone +from pathlib import Path + +# A head whose checks have not been CREATED yet looks exactly like a head whose +# required check will never report: both have the context absent. The only thing +# separating them is time. Measured while building this: release-train#67 was +# reported bricked on all three of its contexts three minutes after a push, and +# had them all a few minutes later. +# +# So a young head is not judged. Sixty minutes is well past any observed queue +# on this fleet (the slowest job here is ~30 min and it still REPORTS within +# seconds of the push), and a watcher that cries wolf on every fresh push is one +# nobody reads -- the failure mode of the always-red check one repo over. +MIN_HEAD_AGE_MINUTES = 60 + +# `gh pr list` truncates at --limit and says nothing about it, so a repo with +# more open PRs than this would be audited PARTIALLY and reported clean -- the +# fail-open shape this watcher exists to remove, in the watcher itself. The cap +# is high enough that no tracebloc repo approaches it (the busiest has ~10 open +# PRs against one base), and reaching it is treated as "could not audit" rather +# than raised, because a number nobody can justify is worse than a stated limit +# that refuses. +PR_LIST_LIMIT = 200 + +HERE = Path(__file__).resolve().parent + + +def _load_caller_drift(): + """Import caller-drift.py as a module. + + The filename has a hyphen, so it is not importable by name. Loading it by + path is the price of NOT having a second copy of the protection reader -- + and a second copy is exactly the drift this file would otherwise create. + """ + path = HERE / "caller-drift.py" + spec = importlib.util.spec_from_file_location("caller_drift", path) + if spec is None or spec.loader is None: # pragma: no cover - unreachable in repo + sys.stderr.write(f"::error::cannot load {path}\n") + raise SystemExit(2) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +CD = _load_caller_drift() + + +def open_prs(org: str, name: str, base: str) -> "list[dict]": + """Open, non-draft PRs targeting `base`, with the contexts on their head. + + A draft cannot merge anyway, so a missing context on one is not a brick -- + it is reported separately by the caller only if asked. `statusCheckRollup` + carries both shapes: check runs (`name`) and legacy statuses (`context`). + """ + raw = CD.gh([ + "pr", "list", "--repo", f"{org}/{name}", "--state", "open", + "--base", base, "--limit", str(PR_LIST_LIMIT), "--json", + "number,isDraft,title,mergeStateStatus,reviewDecision,statusCheckRollup,url,headRefOid", + ]) + try: + prs = json.loads(raw) + except json.JSONDecodeError as exc: + raise CD.GhError(None, f"pr list returned unparseable JSON: {exc}") from exc + if len(prs) >= PR_LIST_LIMIT: + raise CD.GhError( + None, + f"open PR list hit the {PR_LIST_LIMIT} cap, so the view is partial and " + "a bricked PR could be outside it", + ) + return prs + + +def _parse(stamp) -> "datetime | None": + if not isinstance(stamp, str) or not stamp: + return None + try: + return datetime.fromisoformat(stamp.replace("Z", "+00:00")) + except ValueError: + return None + + +def head_age_minutes(org: str, name: str, sha: str) -> "float | None": + """How long CI has had this head, in minutes; None if it cannot be dated. + + NOT the commit timestamp (Bugbot, .github#239). A commit's committer date is + when it was WRITTEN, and a force-push can put a long-dated commit on a branch + a second ago -- which would be judged immediately, producing exactly the + false "bricked" the grace window exists to prevent. + + The honest clock is when CI first saw the head: the OLDEST check suite on the + sha. GitHub creates a suite per app as soon as it has work for that head, so + that timestamp is "CI has known about this for N minutes", which is the + question being asked. Fall back to the commit date only when no suite exists + at all -- for a conflicted PR there never will be one, and then the commit + date is the only clock there is. + + Called ONLY for a PR that already looks bricked, so the fleet-wide cost is + one or two API calls per candidate, not per PR. + """ + try: + suites = CD.gh_json(["api", f"repos/{org}/{name}/commits/{sha}/check-suites"]) + except CD.GhError: + # An unreadable CI clock (502/403/rate-limit) is NOT "no suites" (Bugbot, + # .github#239). Falling through to the commit date here would let a + # force-pushed long-dated commit read as old and brick falsely -- the + # exact failure this function exists to avoid. We cannot date the head, + # so return None; the caller treats an undateable head as young and skips. + return None + stamps = [] + if isinstance(suites, dict): + for suite in suites.get("check_suites") or []: + when = _parse((suite or {}).get("created_at")) + if when: + stamps.append(when) + if stamps: + return (datetime.now(timezone.utc) - min(stamps)).total_seconds() / 60.0 + + # No suite exists at all -- a SUCCESSFUL read that returned an empty list. A + # conflicted PR never gets a suite, so the commit date is the only clock there + # is. (A FAILED read was already handled above and never reaches here.) + try: + commit = CD.gh_json(["api", f"repos/{org}/{name}/commits/{sha}"]) + except CD.GhError: + return None + when = _parse(((commit.get("commit") or {}).get("committer") or {}).get("date")) + if when is None: + return None + return (datetime.now(timezone.utc) - when).total_seconds() / 60.0 + + +def present_contexts(pr: dict) -> "set[str]": + out = set() + for entry in pr.get("statusCheckRollup") or []: + if not isinstance(entry, dict): + continue + for key in ("name", "context"): + val = entry.get(key) + if isinstance(val, str) and val: + out.add(val) + return out + + +def audit_repo(org: str, name: str, roles: "dict[str, str]") -> "tuple[list, list]": + """Returns (findings, errors) for one repo.""" + findings: "list[dict]" = [] + errors: "list[str]" = [] + + for role, branch in roles.items(): + prot = CD.read_protection(org, name, branch) + if prot.error: + # NEVER "clean". An unreadable branch is the fail-open shape this + # watcher exists to eliminate, one level up from the PRs it audits. + errors.append(f"{name}/{branch}: {prot.error}") + continue + required = set(prot.required_checks) + if not required: + continue + + try: + prs = open_prs(org, name, branch) + except CD.GhError as exc: + errors.append(f"{name}/{branch}: PR list unreadable ({exc.detail})") + continue + + for pr in prs: + if pr.get("isDraft"): + continue + missing = sorted(required - present_contexts(pr)) + if missing: + # Young head -> "has not reported YET", which is not this + # watcher's finding. An unreadable age is treated as young: a + # false "bricked" is what makes the report ignorable, and the + # next run picks it up anyway. + age = head_age_minutes(org, name, pr.get("headRefOid") or "") + if age is None or age < MIN_HEAD_AGE_MINUTES: + continue + findings.append({ + # A conflict is a different diagnosis with a different fix, + # and conflating the two sends someone to edit branch + # protection when they needed to rebase. + "cause": ("conflicted" + if pr.get("mergeStateStatus") == "DIRTY" + else "never-reported"), + "repo": name, + "branch": branch, + "role": role, + "number": pr.get("number"), + "url": pr.get("url"), + "title": (pr.get("title") or "")[:70], + "missing": missing, + "mergeStateStatus": pr.get("mergeStateStatus"), + "reviewDecision": pr.get("reviewDecision") or "REVIEW_REQUIRED", + "headAgeMinutes": round(age), + }) + return findings, errors + + +def resolve_roles(org: str, name: str) -> "tuple[dict[str, str], str | None]": + """Which real branches to audit for this repo: develop, staging, and prod.""" + try: + branches = {b["name"] for b in CD.gh_json_array(f"repos/{org}/{name}/branches") + if isinstance(b, dict) and isinstance(b.get("name"), str)} + except CD.GhError as exc: + return {}, f"{name}: branch list unreadable ({exc.detail})" + + roles: "dict[str, str]" = {} + for role in ("develop", "staging", "prod"): + # `prod` is whichever of main/master EXISTS -- resolved from the branch + # list, never by probing, because GitHub follows rename redirects and a + # probe of a renamed `master` answers 200 (caller-drift.py's own note). + resolved = CD.resolve_role_branch(role, branches) + if resolved: + roles[role] = resolved + return roles, None + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + ap.add_argument("--inventory", default=str(HERE.parent / "repo-inventory.yml")) + ap.add_argument("--org", default="tracebloc") + ap.add_argument("--repo", action="append", default=None, + help="audit only these repos (repeatable); default is every " + "repo in the inventory") + ap.add_argument("--json", action="store_true", help="machine-readable output") + args = ap.parse_args() + + inv = CD.load_inventory(args.inventory) + names = sorted((inv.get("repos") or {}).keys()) + if args.repo: + unknown = [r for r in args.repo if r not in names] + if unknown: + sys.stderr.write(f"::error::not in the inventory: {', '.join(unknown)}\n") + return 2 + names = [r for r in names if r in args.repo] + + findings: "list[dict]" = [] + errors: "list[str]" = [] + for name in names: + roles, err = resolve_roles(args.org, name) + if err: + errors.append(err) + continue + f, e = audit_repo(args.org, name, roles) + findings.extend(f) + errors.extend(e) + + if args.json: + print(json.dumps({"findings": findings, "errors": errors}, indent=2)) + else: + for f in findings: + label = ("CONFLICTED" if f["cause"] == "conflicted" else "BRICKED") + print(f"{label} {f['repo']}#{f['number']} -> {f['branch']}") + print(f" missing: {', '.join(f['missing'])}") + if f["cause"] == "conflicted": + print(" cause: merge conflict — no merge commit, so no " + "pull_request run. Rebase; do not touch protection.") + print(f" {f['reviewDecision']} / {f['mergeStateStatus']} {f['url']}") + print(f" {f['title']}") + for e in errors: + print(f"COULD NOT AUDIT {e}") + if not findings and not errors: + print(f"No PR is missing a required context ({len(names)} repos audited).") + + summary = os.environ.get("GITHUB_STEP_SUMMARY") + if summary: + with open(summary, "a", encoding="utf-8") as fh: + if findings: + fh.write("## Bricked PRs — a required check that never reported\n\n") + fh.write("| repo | PR | missing context(s) | cause | review | merge state |\n") + fh.write("|---|---|---|---|---|---|\n") + for f in findings: + cause = ("merge conflict — rebase, do not touch protection" + if f["cause"] == "conflicted" else "never reported") + fh.write(f"| `{f['repo']}` | [#{f['number']}]({f['url']}) | " + f"`{'`, `'.join(f['missing'])}` | {cause} | " + f"{f['reviewDecision']} | {f['mergeStateStatus']} |\n") + fh.write("\nEach is approved-or-approvable with **nothing red to point at**, " + "and cannot merge until the context reports or stops being required.\n") + if errors: + fh.write("\n## Could not audit\n\n") + for e in errors: + fh.write(f"- {e}\n") + fh.write("\nUnreadable is **not** clean — these were not checked.\n") + if not findings and not errors: + fh.write(f"No PR is missing a required context ({len(names)} repos audited).\n") + + # An unreadable repo outranks a clean sweep: reporting "0 bricked" when part + # of the fleet was never read is the fail-open this watcher exists to remove. + if errors: + return 2 + return 1 if findings else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/tests/bricked-prs-selftest.py b/scripts/tests/bricked-prs-selftest.py new file mode 100755 index 0000000..5f656c7 --- /dev/null +++ b/scripts/tests/bricked-prs-selftest.py @@ -0,0 +1,226 @@ +#!/usr/bin/env python3 +"""Offline self-test for scripts/bricked-prs.py (tracebloc/backend#1721). + +The watcher's whole product is a distinction that is invisible at a glance: + + * a required context that is ABSENT because it will never report -> bricked + * a required context that is absent because the run has not STARTED -> not yet + * a branch or PR list that could not be READ -> unknown, never "clean" + +Each of those is asserted here rather than trusted, because getting any of them +wrong makes the watcher either ignorable or actively misleading -- and both were +observed while building it: the first version reported release-train#67 bricked +three minutes after a push, and it had every context a few minutes later. + +No network and no token: the module's `gh` entry points are replaced. + +Exit 0 when every path behaves as specified. +""" + +from __future__ import annotations + +import importlib.util +import json +import os +import sys +from datetime import datetime, timedelta, timezone + +HERE = os.path.dirname(os.path.abspath(__file__)) +TARGET = os.path.join(HERE, os.pardir, "bricked-prs.py") + +_spec = importlib.util.spec_from_file_location("bricked_prs", TARGET) +if _spec is None or _spec.loader is None: + sys.exit(f"cannot import {TARGET}") +bp = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(bp) + +# Captured BEFORE any case swaps them out. `install()` replaces these module +# attributes wholesale, so a later case that wants the real implementation must +# put them back -- otherwise it silently tests the previous case's stub, which is +# this file's own subject matter. +REAL_OPEN_PRS = bp.open_prs +REAL_HEAD_AGE = bp.head_age_minutes + +RESULTS: "list[tuple[bool, str, str]]" = [] + + +def record(ok: bool, name: str, detail: str) -> None: + RESULTS.append((ok, name, detail)) + print(f"{'PASS' if ok else 'FAIL'} {name}\n {detail}") + + +class FakeProtection: + def __init__(self, checks, error=None): + self.required_checks = set(checks) + self.error = error + + +def install(protection, prs, age=999.0): + """Point the module at canned answers for one scenario.""" + bp.CD.read_protection = lambda org, name, branch: protection + bp.open_prs = lambda org, name, base: list(prs) + bp.head_age_minutes = lambda org, name, sha: age + + +def pr(number=1, draft=False, contexts=(), state="CLEAN", review="APPROVED"): + return { + "number": number, "isDraft": draft, "title": "t", "url": "u", + "headRefOid": "deadbeef", "mergeStateStatus": state, + "reviewDecision": review, + "statusCheckRollup": [{"name": c} for c in contexts], + } + + +ROLES = {"develop": "develop"} + +# 1. The finding itself. +install(FakeProtection({"gate", "lint"}), [pr(contexts=["lint"])]) +f, e = bp.audit_repo("o", "r", ROLES) +record(len(f) == 1 and f[0]["missing"] == ["gate"] and not e, + "a required context absent from an old head is reported", + f"findings={[x['missing'] for x in f]} errors={e}") + +# 2. THE DISTINCTION THAT MAKES IT READABLE. Same state, young head. +install(FakeProtection({"gate", "lint"}), [pr(contexts=["lint"])], age=5.0) +f, e = bp.audit_repo("o", "r", ROLES) +record(not f and not e, + "the same PR is NOT reported while its head is young", + "a queued run and a run that will never happen look identical; only " + f"time separates them (findings={f})") + +# 3. An unreadable age is treated as young. A false brick is what gets the +# whole report ignored; the next run picks it up anyway. +install(FakeProtection({"gate"}), [pr()], age=None) +f, e = bp.audit_repo("o", "r", ROLES) +record(not f, "an unreadable head age does not produce a finding", f"findings={f}") + +# 4. Drafts cannot merge, so a missing context on one is not a brick. +install(FakeProtection({"gate"}), [pr(draft=True)]) +f, e = bp.audit_repo("o", "r", ROLES) +record(not f, "a draft PR is not reported", f"findings={f}") + +# 5. Present contexts satisfy the requirement -- including a legacy status, +# which carries `context` rather than `name`. +install(FakeProtection({"legacy"}), [{ + "number": 9, "isDraft": False, "title": "t", "url": "u", + "headRefOid": "d", "mergeStateStatus": "CLEAN", "reviewDecision": "APPROVED", + "statusCheckRollup": [{"context": "legacy"}], +}]) +f, e = bp.audit_repo("o", "r", ROLES) +record(not f, "a legacy status context counts as present", + "the rollup carries check runs as `name` and statuses as `context`; " + f"reading only one under-reports every legacy check (findings={f})") + +# 6. FAIL CLOSED. An unreadable branch is not a clean branch. +install(FakeProtection(set(), error="classic protection unreadable (502)"), [pr()]) +f, e = bp.audit_repo("o", "r", ROLES) +record(not f and len(e) == 1, + "an unreadable branch is reported as could-not-audit, not as clean", + f"errors={e}") + +# 7. A conflicted PR is a DIFFERENT diagnosis: no merge commit means no +# pull_request run at all, and the fix is a rebase, not protection. +install(FakeProtection({"gate"}), [pr(state="DIRTY")]) +f, e = bp.audit_repo("o", "r", ROLES) +record(len(f) == 1 and f[0]["cause"] == "conflicted", + "a conflicted PR is labelled as such, not as a protection problem", + f"cause={[x['cause'] for x in f]}") + +install(FakeProtection({"gate"}), [pr(state="BLOCKED")]) +f, e = bp.audit_repo("o", "r", ROLES) +record(len(f) == 1 and f[0]["cause"] == "never-reported", + "a non-conflicted PR keeps the never-reported cause", + f"cause={[x['cause'] for x in f]}") + +# 8. A branch with no required checks cannot brick anything. +install(FakeProtection(set()), [pr()]) +f, e = bp.audit_repo("o", "r", ROLES) +record(not f and not e, "a branch requiring nothing produces no findings", f"findings={f}") + +# 9. An unreadable PR list is an error, not an empty list. +bp.CD.read_protection = lambda org, name, branch: FakeProtection({"gate"}) +def _boom(org, name, base): + raise bp.CD.GhError(None, "pr list exploded") +bp.open_prs = _boom +f, e = bp.audit_repo("o", "r", ROLES) +record(not f and len(e) == 1 and "PR list unreadable" in e[0], + "an unreadable PR list is reported, not silently empty", f"errors={e}") + +# 10. A CAPPED PR LIST IS NOT AN AUDITED ONE (Bugbot, .github#239). `gh pr list` +# truncates silently, so a partial view would report "clean" for the PRs it +# never saw -- the watcher committing the fail-open it exists to find. +bp.open_prs = REAL_OPEN_PRS +_real_gh = bp.CD.gh +bp.CD.gh = lambda args: json.dumps([pr(number=i, contexts=["gate"]) + for i in range(bp.PR_LIST_LIMIT)]) +try: + caught = "" + try: + bp.open_prs("o", "r", "develop") + except bp.CD.GhError as exc: + caught = exc.detail + record("cap" in caught, + "a PR list that hits the cap raises rather than returning a partial view", + f"detail={caught!r}") +finally: + bp.CD.gh = _real_gh + +# 11. THE GRACE WINDOW'S CLOCK. A force-push can put a long-dated commit on a +# branch a second ago, so the commit timestamp is not "how long CI has had +# this head". The oldest check suite is. +NOW = datetime.now(timezone.utc) + +def _fake_api(payload_by_path): + def gh_json(args): + path = args[-1] + for frag, payload in payload_by_path.items(): + if frag in path: + return payload + raise bp.CD.GhError(None, f"unstubbed {path}") + return gh_json + +bp.head_age_minutes = REAL_HEAD_AGE +_real_json = bp.CD.gh_json +try: + # A suite created 5 minutes ago, on a commit dated last week: young. + bp.CD.gh_json = _fake_api({ + "check-suites": {"check_suites": [ + {"created_at": (NOW - timedelta(minutes=5)).isoformat().replace("+00:00", "Z")}]}, + "commits/": {"commit": {"committer": { + "date": (NOW - timedelta(days=7)).isoformat().replace("+00:00", "Z")}}}, + }) + age = bp.head_age_minutes("o", "r", "sha") + record(age is not None and age < 10, + "a force-pushed old commit is dated by CI's clock, not the commit's", + f"age={age!r} minutes (commit date was 7 days ago)") + + # No suite at all -- the conflicted case. Only the commit date exists. + bp.CD.gh_json = _fake_api({ + "check-suites": {"check_suites": []}, + "commits/": {"commit": {"committer": { + "date": (NOW - timedelta(hours=3)).isoformat().replace("+00:00", "Z")}}}, + }) + age = bp.head_age_minutes("o", "r", "sha") + record(age is not None and age > 120, + "with no check suite at all it falls back to the commit date", + f"age={age!r} minutes — the conflicted case, where no suite will ever exist") + + # An UNREADABLE check-suites read (502/403) is not "no suites": it must NOT + # fall through to an old commit date and brick falsely. The suites call + # raises; the commit call, if it were ever reached, is dated last week. + def _raise_on_suites(args): + if args[-1].endswith("check-suites"): + raise bp.CD.GhError(None, "502 reading check-suites") + return {"commit": {"committer": { + "date": (NOW - timedelta(days=7)).isoformat().replace("+00:00", "Z")}}} + bp.CD.gh_json = _raise_on_suites + age = bp.head_age_minutes("o", "r", "sha") + record(age is None, + "an unreadable check-suites read is undateable (None), not the commit date", + f"age={age!r} — a 502 here must read as young, never brick a 7-day-old commit") +finally: + bp.CD.gh_json = _real_json + +failed = [r for r in RESULTS if not r[0]] +print(f"\n{len(RESULTS) - len(failed)} passed, {len(failed)} failed") +sys.exit(1 if failed else 0) From bb6ecdca946909921c6264f8d48a64f2e453279b Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:33:21 +0200 Subject: [PATCH 2/7] fix(kanban): the deploy-state guard was matching a column that no longer exists (#237) `CURRENT_COL` is read from the BOARD, whose column is "Staging (agent review)". The guard's list carried only the pre-rename "Staging (human review)", so an issue hand-closed while sitting in the agent-review column did not match, `Done` overwrote its deploy state, and kanban-archive.yml then hid the card entirely -- precisely the sequence this guard was added to prevent (Bugbot, .github#126), reopened by a string that quietly stopped being true. kanban-reconcile.yml's equivalent guard already lists both names, which is what makes this a drift rather than a design gap: one list was updated at the rename and its copy was not. The guard also stranded silently. Refusing is right -- the two real cases needed OPPOSITE answers (backend#1493 had shipped via cli#452 and belonged in Prod; data-ingestors#488 was reverted and belonged in Done), so no default is correct and only the person closing the issue knows which. But a `::notice::` in a run log is invisible by the time anyone looks at the board, so the card sat in a deploy state with nothing anywhere saying why. It now comments on the issue with the two options, and warns if even that fails. Verified: YAML parses, the embedded shell parses, house-rules clean, the version-bump-gate selftest still 51/0. Refs tracebloc/backend#1846 Co-authored-by: tracebloc-release-train[bot] <309815517+tracebloc-release-train[bot]@users.noreply.github.com> --- .github/workflows/kanban-closure-router.yml | 30 ++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/.github/workflows/kanban-closure-router.yml b/.github/workflows/kanban-closure-router.yml index 386a1af..a84635f 100644 --- a/.github/workflows/kanban-closure-router.yml +++ b/.github/workflows/kanban-closure-router.yml @@ -284,10 +284,38 @@ jobs: # # Only Done is guarded. A PR-derived Status is a deploy fact and may advance a # card normally. + # THE COLUMN LIST WAS STALE, AND THAT REOPENED THE HOLE IT CLOSES + # (backend#1846). `CURRENT_COL` is read from the BOARD, whose column is + # "Staging (agent review)" -- this list carried only the pre-rename + # "Staging (human review)". So an issue hand-closed while sitting in the + # agent-review column did not match, Done overwrote its deploy state, + # and kanban-archive.yml then hid the card entirely: exactly the + # sequence this guard exists to prevent, through a string that quietly + # stopped being true. + # + # kanban-reconcile.yml's equivalent guard already lists both names. Two + # copies of one list, one updated at the rename and one not, is how this + # drifted -- so when the rename window closes, drop the legacy name in + # BOTH places. if [ "$STATUS_NAME" = "Done" ]; then case "${CURRENT_COL:-}" in - "On dev"|"Staging (human review)"|"FR on staging"|"Ready for prod"|"Prod") + "On dev"|"Staging (agent review)"|"Staging (human review)"|"FR on staging"|"Ready for prod"|"Prod") echo "::notice::#$NUMBER hand-closed but sits in '$CURRENT_COL', a deploy state - NOT setting Done (D8: follow the PR's stage)" + # AND SAY SO WHERE SOMEONE WILL SEE IT. Refusing is right, but it + # parks the card in a deploy state with no way to self-heal, and a + # run-log notice is invisible by the time anyone looks. The two + # real cases needed OPPOSITE answers -- backend#1493 had shipped + # via cli#452 and belonged in Prod; data-ingestors#488 was + # reverted and belonged in Done -- so no default is correct and + # only the person closing it knows which. + CLOSE_NOTE="Closed while the board still shows \`$CURRENT_COL\`, which records a deployment." + CLOSE_NOTE="$CLOSE_NOTE The automation will not overwrite a deploy state with \`Done\` (RFC-BACKEND-1405 D8)," + CLOSE_NOTE="$CLOSE_NOTE so this card stays where it is until someone says which happened:" + CLOSE_NOTE="$CLOSE_NOTE **it shipped** - move the card to the column it reached (\`Prod\` if it is in production);" + CLOSE_NOTE="$CLOSE_NOTE **nothing was deployed** (reverted, abandoned, superseded) - clear the deploy state, then \`Done\`." + CLOSE_NOTE="$CLOSE_NOTE Both cases are real and they need opposite answers, which is why this is not decided automatically." + gh issue comment "$NUMBER" --repo "$REPO_FULL" --body "$CLOSE_NOTE" >/dev/null 2>&1 \ + || echo "::warning::could not comment on #$NUMBER - it is parked in '$CURRENT_COL' with no note on the issue" exit 0 ;; esac fi From 84d344ba0a28873dcecb91b9d3302b942e92da44 Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:35:28 +0200 Subject: [PATCH 3/7] fix(ci): set up Python before pip in the bricked-PR audit, so it can run at all (#244) Bugbot HIGH on the staging promotion PR #243. `bricked-prs.yml` runs `pip install --quiet pyyaml` on `ubuntu-latest` with no `actions/setup-python`. Ubuntu 24.04 marks its system Python externally managed (PEP 668), so pip refuses, the step fails, and THE AUDIT NEVER RUNS. The dependency is real, not incidental: bricked-prs.py loads caller-drift.py for the protection reader, and that module hard-fails without PyYAML by design. So the install cannot simply be dropped. Worse than a broken job. This audit's whole purpose is to find PRs blocked by a required check that never reports -- a silence. A guard that cannot start produces exactly the same silence as a clean fleet, so its failure looks like its success. Nothing else would have said so. bricked-prs-selftest.yml already sets up Python 3.12 before the identical install, so this is that shape rather than a new decision -- the audit and its selftest now agree, which is also why the selftest never caught this. Checked the class rather than the instance: this was the ONLY workflow in the repo with a `pip install` and no `setup-python`. actionlint clean; YAML parses; setup-python pinned by SHA with its version comment per the action-pins gate. --- .github/workflows/bricked-prs.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/bricked-prs.yml b/.github/workflows/bricked-prs.yml index 8f7f9e5..a747e84 100644 --- a/.github/workflows/bricked-prs.yml +++ b/.github/workflows/bricked-prs.yml @@ -30,6 +30,15 @@ jobs: timeout-minutes: 20 steps: - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + # Without this the `pip install` below hits PEP 668 on ubuntu-latest + # (24.04): the runner's Python is externally managed, pip refuses, the step + # fails and THE AUDIT NEVER RUNS. A scheduled guard that cannot start is + # worse than none — nothing reports, and silence reads as "no bricked PRs". + # bricked-prs-selftest.yml already does this before the same install; this + # is that shape, not a new idea (Bugbot, #243). + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + with: + python-version: '3.12' - name: Install PyYAML run: pip install --quiet pyyaml - name: Audit From eb0d8bd9b27cb5c589dabc35ae56d3436175418d Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:53:12 +0200 Subject: [PATCH 4/7] =?UTF-8?q?fix(kanban):=20finish=20#1592=20step=203=20?= =?UTF-8?q?=E2=80=94=20writers=20first,=20then=20the=20fallback=20(#245)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three sites still WROTE `Staging (human review)`, a column that no longer exists on the board, and two fallbacks quietly translated it to `FR on staging`. Behaviour today is correct because of those fallbacks -- and both are labelled "removed in step 3 of #1592". So step 3 as written is an outage: delete the fallbacks while three writers still emit the dead name and every staging promotion fails its option lookup and exits 1. The order is the whole fix. Writers emit the live name; only then is the fallback dead code, and it goes in the same change so nothing is left half-done. kanban-closure-router.yml:71 pull_request writer kanban-closure-router.yml:156 issue-closure writer advance-deploy-env.yml:71 push writer Found by @shujaatTracebloc reviewing .github#237, which fixed the same stale-string class on the GUARD path. This is the WRITE path. WHAT THIS DELIBERATELY DOES NOT DO: it does not start writing `Staging (agent review)`. Nothing writes that column anywhere, by design -- fr-gate ranks it 7 and says so ("READ-ONLY for now; nothing writes this value yet, #1578 does that in a LATER hop"). A card landing in `FR on staging` on a staging merge is the documented transition, not a skipped stage. Writing the agent column here would move cards into a column nothing advances, which is backend#1846's dead end, created on purpose. READERS are left alone. `case` arms that accept both names match values read FROM the board, which can never be the dead one -- that is tolerance, not a dependency, and it costs nothing. `fr-pass-comment.yml` needs no change either: it resolves the column by PROBING the board and fails loudly when neither name exists, which is the shape the other two should have had. Verified: both files parse as YAML, every embedded `run:` block parses under `bash -n`, house-rules clean, and no writer of the dead name remains. Refs tracebloc/backend#1592 Co-authored-by: tracebloc-release-train[bot] <309815517+tracebloc-release-train[bot]@users.noreply.github.com> --- .github/workflows/advance-deploy-env.yml | 19 +------------------ .github/workflows/kanban-closure-router.yml | 14 ++------------ 2 files changed, 3 insertions(+), 30 deletions(-) diff --git a/.github/workflows/advance-deploy-env.yml b/.github/workflows/advance-deploy-env.yml index c7d40c8..be791cc 100644 --- a/.github/workflows/advance-deploy-env.yml +++ b/.github/workflows/advance-deploy-env.yml @@ -68,7 +68,7 @@ jobs: # Default mapping case "$BRANCH" in develop) DEPLOY_ENV="dev"; STATUS_NAME="On dev" ;; - staging) DEPLOY_ENV="staging"; STATUS_NAME="Staging (human review)" ;; + staging) DEPLOY_ENV="staging"; STATUS_NAME="FR on staging" ;; master|main) DEPLOY_ENV="prod"; STATUS_NAME="Prod" ;; *) DEPLOY_ENV=""; STATUS_NAME="" ;; esac @@ -167,23 +167,6 @@ jobs: | select(.name=="Status") | .id') STATUS_OPT=$(echo "$PROJ" | jq -r --arg s "$STATUS_NAME" '.data.organization.projectV2.fields.nodes[] | select(.name=="Status") | .options[] | select(.name==$s) | .id') - # Rename window (backend#1592). "FR on staging" becomes "Staging (human - # review)" via a UI rename, which is a single instant the board flips -- - # there is no period where both options exist. So resolution asks for the - # NEW name and falls back to the OLD one, and this file works on either - # side of that instant. The fallback is removed in step 3 of #1592. - if [ -z "$STATUS_OPT" ] || [ "$STATUS_OPT" = "null" ]; then - case "$STATUS_NAME" in - "Staging (human review)") FALLBACK="FR on staging" ;; - *) FALLBACK="" ;; - esac - if [ -n "$FALLBACK" ]; then - STATUS_OPT=$(echo "$PROJ" | jq -r --arg s "$FALLBACK" '.data.organization.projectV2.fields.nodes[] - | select(.name=="Status") | .options[] | select(.name==$s) | .id') - [ -n "$STATUS_OPT" ] && [ "$STATUS_OPT" != "null" ] && STATUS_NAME="$FALLBACK" - fi - fi - if [ -z "$DEPLOY_OPT" ] || [ "$DEPLOY_OPT" = "null" ]; then echo "Could not resolve Deploy environment option for '$DEPLOY_ENV' - aborting" exit 1 diff --git a/.github/workflows/kanban-closure-router.yml b/.github/workflows/kanban-closure-router.yml index a84635f..5b75f33 100644 --- a/.github/workflows/kanban-closure-router.yml +++ b/.github/workflows/kanban-closure-router.yml @@ -68,7 +68,7 @@ jobs: if [ "$PR_MERGED" = "true" ]; then case "$BASE_REF" in main|master) STATUS="Prod" ;; - staging) STATUS="Staging (human review)" ;; + staging) STATUS="FR on staging" ;; develop) STATUS="On dev" ;; *) # A PR merged into a SIBLING feature branch deploys nothing by @@ -153,7 +153,7 @@ jobs: elif [ "$CLOSER_TYPE" = "PullRequest" ]; then case "$CLOSING_PR_BASE" in main|master) STATUS="Prod" ;; - staging) STATUS="Staging (human review)" ;; + staging) STATUS="FR on staging" ;; develop) STATUS="On dev" ;; # Unrecognised base: mirror the pull_request branch's default so # the issue and its PR agree. @@ -226,16 +226,6 @@ jobs: | select(.name=="Status") | .id') STATUS_OPT=$(echo "$PROJ" | jq -r --arg s "$STATUS_NAME" '.data.organization.projectV2.fields.nodes[] | select(.name=="Status") | .options[] | select(.name==$s) | .id') - # Rename window (backend#1592): prefer the new column name, fall back to - # the old. Removed in step 3. - if [ -z "$STATUS_OPT" ] || [ "$STATUS_OPT" = "null" ]; then - if [ "$STATUS_NAME" = "Staging (human review)" ]; then - STATUS_OPT=$(echo "$PROJ" | jq -r --arg s "FR on staging" '.data.organization.projectV2.fields.nodes[] - | select(.name=="Status") | .options[] | select(.name==$s) | .id') - [ -n "$STATUS_OPT" ] && [ "$STATUS_OPT" != "null" ] && STATUS_NAME="FR on staging" - fi - fi - if [ -z "$STATUS_OPT" ] || [ "$STATUS_OPT" = "null" ]; then echo "Could not resolve Status option '$STATUS_NAME' — aborting" exit 1 From 9bb10bd1195524519de55eefdd70f4d8ed1531e9 Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Thu, 13 Aug 2026 20:49:57 +0200 Subject: [PATCH 5/7] fix(kanban): abort when the Status option cannot be resolved, instead of warning (#246) Bugbot HIGH on the staging promotion PR #243. An unresolvable Status option logged `::warning::`, set SKIP_STATUS=1 and let the run finish GREEN. So the board silently stops advancing while every push reports success -- the failure and the success are indistinguishable, and the only signal is a warning in a log nobody opens. This was the one asymmetry in the family. Twelve lines above, the Deploy environment lookup aborts on exactly this condition; kanban-closure-router.yml aborts on it too (":230 Could not resolve Status option - aborting; exit 1"). Only this path degraded, and Status is the half that drives the pipeline view. WHAT I DID NOT CHANGE, having checked it: the finding also suggests the writers lost their rename-window fallback while the resolvers kept it. They did, but that is not live -- the board's Status options today are `Staging (agent review)` and `FR on staging` (read from the live project), and the writers emit `FR on staging`, so every lookup resolves. `opt_either('Staging (human review)', 'FR on staging')` in kanban-reconcile.yml is now vestigial: its own comment says it is the backend#1592 rename shim, "removed in step 3". Finishing that retirement is a five-file sweep and a separate change from this abort; filed rather than folded in during a release freeze. Also corrected the comment above the block, which described the removed behaviour ("degrades gracefully ... rather than failing the whole workflow") and would have read as the contract to the next person. actionlint clean; YAML parses. --- .github/workflows/advance-deploy-env.yml | 28 +++++++++++++++++++----- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/.github/workflows/advance-deploy-env.yml b/.github/workflows/advance-deploy-env.yml index be791cc..eb5522f 100644 --- a/.github/workflows/advance-deploy-env.yml +++ b/.github/workflows/advance-deploy-env.yml @@ -172,15 +172,31 @@ jobs: exit 1 fi - # Status update degrades gracefully: if the Status field or its target - # option can't be resolved, log a warning and skip just the Status step - # rather than failing the whole workflow (Deploy env update still wins). - SKIP_STATUS=0 + # Status resolution does NOT degrade gracefully -- see the abort below. + # It used to, and that was the bug: a graceful degrade here means the + # board stops advancing while every run reports success. + # FAIL CLOSED. This used to warn and set SKIP_STATUS=1, so an unresolvable + # Status option meant the run stayed GREEN while no card advanced -- the + # board silently stops tracking the pipeline and the only signal is a + # warning nobody reads (Bugbot, .github#243, High). + # + # It is also the ONE asymmetry in this file and its siblings: the Deploy + # environment lookup twelve lines above aborts, and + # kanban-closure-router.yml aborts on exactly this condition. A column + # rename, a project renumbering or a `.kanban.yml` naming a column that + # does not exist are all misconfigurations, and every one of them is + # cheaper to find as a red run than as three weeks of un-advanced cards. if [ -z "$STATUS_FIELD" ] || [ "$STATUS_FIELD" = "null" ] \ || [ -z "$STATUS_OPT" ] || [ "$STATUS_OPT" = "null" ]; then - echo "::warning::Could not resolve Status option '$STATUS_NAME' in project #$PROJECT_NUMBER - skipping Status updates" - SKIP_STATUS=1 + echo "::error::Could not resolve Status option '$STATUS_NAME' in project #$PROJECT_NUMBER." \ + "The Deploy environment write may already have happened; the Status write did not." \ + "Check the board's Status column names against this workflow's branch map." >&2 + exit 1 fi + # Now always 0 -- the branch that set it to 1 aborts instead. Kept so the + # two `[ "$SKIP_STATUS" != "1" ]` guards below stay valid without + # re-indenting their blocks; there is no live skip path. + SKIP_STATUS=0 # Pipeline order (mirrors fr-gate's rank). Advancement is monotonic: # a push only ever moves a card FORWARD. Pushes routinely carry commits From aff108145cafcc0e5585f4fe8fcfc954afeafc88 Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:02:07 +0200 Subject: [PATCH 6/7] feat(kanban): assert the board has every Status column the writers emit (#247) * feat(kanban): assert the board has every Status column the writers emit The board's column names and the workflows that write them are two systems that must agree, and nothing checked that they did. #1592 carried the `FR on staging` rename by hand across three PRs; in between, the writers emitted a column the board did not have and only a fallback made it work. That gap is exactly what .github#243 reported and what #245 closed by hand. This makes it un-driftable instead of remembered. The failure it prevents is a silent one. A written name that does not resolve means the card is not moved -- and until #246 that was a `::warning::` on a green run, so a frozen board and a working board looked identical. SCOPE IS DELIBERATELY NARROW: * WRITERS are asserted. A name that is written must exist or the write cannot land. `advance-deploy-env.yml` and `kanban-closure-router.yml`, listed rather than globbed -- `STATUS=` is a common shell variable and other workflows use it for "ok"/"absent"/"unreadable", which are not column names. * RESOLVERS are NOT asserted. They ask for names on both sides of a rename on purpose: `opt_either("Staging (human review)", "FR on staging")` is the #1592 shim and its first argument is SUPPOSED to be absent today. Enforcing there would make the tolerance impossible to express. * `STATUS_NAME="$OVERRIDE"` is skipped: a per-repo .kanban.yml value cannot be known here. Runs on PRs touching the writers, and DAILY -- the board can be renamed in the UI at any time with no PR to hang a check on, which is the failure this is really for. Fails closed throughout: an unreadable board, an unparseable response, zero options returned, or a literal pattern that matches nothing are all errors. The last one matters most -- a stale regex would make the check pass vacuously, which is the failure mode of every guard it is modelled on. Verified: passes on today's board (5 names); reproducing the pre-#245 state (writer emitting "Staging (human review)") fails and names the exact file and value; selftest 6/6 with the board stubbed, covering absent names, a stale name among good ones, and a case near-miss. * fix(kanban): gate the conformance check on the selftest job Bugbot (Medium): the check job ran in parallel with selftest and had no needs: selftest. If the checker failure paths regress while the board still matches, "Selftest the checker" goes red but "Written Status names exist on the board" can still report success -- a conformance-named green from a broken guard. caller-drift + standards-sync gate their audit on selftest for the same reason. Add needs: selftest so the conformance verdict is only trusted once the checker is proven able to fail. .github#247. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .github/workflows/kanban-columns.yml | 63 +++++++++++++ scripts/kanban-columns-check.py | 110 +++++++++++++++++++++++ scripts/tests/kanban-columns-selftest.py | 64 +++++++++++++ 3 files changed, 237 insertions(+) create mode 100644 .github/workflows/kanban-columns.yml create mode 100755 scripts/kanban-columns-check.py create mode 100755 scripts/tests/kanban-columns-selftest.py diff --git a/.github/workflows/kanban-columns.yml b/.github/workflows/kanban-columns.yml new file mode 100644 index 0000000..59a6d79 --- /dev/null +++ b/.github/workflows/kanban-columns.yml @@ -0,0 +1,63 @@ +# The board's Status column names and the workflows that write them are two +# systems that must agree, and nothing checked that they did. +# +# The rename window for `FR on staging` was carried by hand across three PRs +# (backend#1592). In between, the writers emitted a column the board did not +# have, and only a fallback made it work — while an unresolvable Status was a +# `::warning::` on a GREEN run (fixed in #246), so a frozen board and a working +# board were indistinguishable. +# +# Runs on PRs that touch the writers OR this check, and daily — because the +# board can be renamed in the UI at any time, with no PR to hang a check on. +# That is the failure this is really for: a rename nobody pairs with a code +# change. +name: Kanban column conformance + +on: + pull_request: + paths: + - ".github/workflows/advance-deploy-env.yml" + - ".github/workflows/kanban-closure-router.yml" + - ".github/workflows/kanban-columns.yml" + - "scripts/kanban-columns-check.py" + - "scripts/tests/kanban-columns-selftest.py" + schedule: + # 06:15 UTC, before caller-drift at 06:30 — a board rename breaks promotions, + # so it should be the first thing the morning tells you. + - cron: "15 6 * * *" + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: kanban-columns + cancel-in-progress: false + +jobs: + selftest: + name: Selftest the checker + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + # Guard the guard: a checker that cannot fail is worse than no checker, + # because its green is read as conformance. + - run: python3 scripts/tests/kanban-columns-selftest.py + + check: + name: Written Status names exist on the board + # Gate on selftest: a conformance PASS emitted by a checker whose own + # failure paths have regressed is a false green (the checker can't fail, so + # its success means nothing). Sibling guards caller-drift + standards-sync + # gate their audit on selftest for exactly this reason. (Bugbot, #247.) + needs: selftest + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - name: Check + env: + # Reads the org project's Status field. + GH_TOKEN: ${{ secrets.PROJECTS_KANBAN_TOKEN }} + run: python3 scripts/kanban-columns-check.py diff --git a/scripts/kanban-columns-check.py b/scripts/kanban-columns-check.py new file mode 100755 index 0000000..0bdd9c3 --- /dev/null +++ b/scripts/kanban-columns-check.py @@ -0,0 +1,110 @@ +#!/usr/bin/env python3 +"""Every Status column name the kanban WRITERS emit must exist on the board. + +WHY THIS EXISTS (backend#1592, .github#243/#245) + +The board's column names and the workflows that write them are two systems that +must agree, and nothing checked that they did. The rename window for +`FR on staging` was carried by hand: step 1 taught the resolvers both names, +step 3 removed the shim from the writers. In between, the writers emitted a +column the board did not have and only a fallback made it work. + +The failure mode is the reason this is a script and not a comment. A written +name that does not resolve means the card is not moved -- and until .github#246 +that was a `::warning::` on a GREEN run, so the board silently stopped tracking +the pipeline. The board freezing and the board working looked identical. + +SCOPE, deliberately narrow: + + * WRITERS are checked. A name that is written must exist, or the write cannot + land. This is the assertion. + * RESOLVERS are reported, not enforced. They ask for names on both sides of a + rename on purpose -- `opt_either("Staging (human review)", "FR on staging")` + is the #1592 shim and its first argument is SUPPOSED to be absent today. + Failing on that would make the tolerance it provides impossible to express. + +So this answers one question exactly: can every write this repo performs +actually land on the board as it is configured right now? +""" +from __future__ import annotations + +import json +import re +import subprocess +import sys +from pathlib import Path + +WORKFLOWS = Path(__file__).resolve().parent.parent / ".github" / "workflows" + +# The workflows that WRITE a Status value. Listed rather than globbed: `STATUS=` +# is a common shell variable name and other workflows use it for unrelated +# values ("ok", "absent", "unreadable"), which are not column names. +WRITERS = ("advance-deploy-env.yml", "kanban-closure-router.yml") + +# `STATUS_NAME="On dev"` / `STATUS="Prod"`. Only literals: `STATUS_NAME="$OVERRIDE"` +# is a per-repo .kanban.yml value that cannot be known here. +LITERAL = re.compile(r'\bSTATUS(?:_NAME)?="([^"$]+)"') + +PROJECT_ID = "PVT_kwDOCSsgos4BTDWN" # engineer kanban (org project #2) + + +def written_names() -> "dict[str, set[str]]": + found: "dict[str, set[str]]" = {} + for name in WRITERS: + path = WORKFLOWS / name + if not path.is_file(): + sys.exit(f"error: {path} not found — WRITERS is stale") + for value in LITERAL.findall(path.read_text()): + found.setdefault(value, set()).add(name) + if not found: + sys.exit("error: no Status literals found at all — the pattern is stale, " + "which would make this check pass vacuously") + return found + + +def board_options() -> "set[str]": + query = ( + 'query{node(id:"%s"){... on ProjectV2{field(name:"Status")' + '{... on ProjectV2SingleSelectField{options{name}}}}}}' % PROJECT_ID + ) + proc = subprocess.run( + ["gh", "api", "graphql", "-f", f"query={query}"], + capture_output=True, text=True, + ) + if proc.returncode != 0: + sys.exit(f"error: could not read the board: {proc.stderr.strip()[:300]}") + try: + node = json.loads(proc.stdout)["data"]["node"] + options = {o["name"] for o in node["field"]["options"]} + except (KeyError, TypeError, json.JSONDecodeError) as exc: + sys.exit(f"error: unexpected board response ({exc}) — refusing to report " + "conformance from a read this script did not understand") + if not options: + sys.exit("error: the board reported zero Status options — treating as a " + "failed read, not as 'nothing to check'") + return options + + +def main() -> int: + written, options = written_names(), board_options() + missing = {n: s for n, s in written.items() if n not in options} + + for name in sorted(written): + mark = "✗" if name in missing else "ok" + print(f" {mark:2} {name:20} <- {', '.join(sorted(written[name]))}") + + if missing: + print("\nERROR: these Status names are WRITTEN but do not exist on the board:") + for name in sorted(missing): + print(f" - {name!r} (written by {', '.join(sorted(missing[name]))})") + print("\nA write to a name the board does not have cannot land, so the card " + "does not move. Either rename the column back, or update the writers " + "in the same change. Board has: " + ", ".join(sorted(options))) + return 1 + + print(f"\nAll {len(written)} written Status name(s) exist on the board.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/kanban-columns-selftest.py b/scripts/tests/kanban-columns-selftest.py new file mode 100755 index 0000000..ac530e1 --- /dev/null +++ b/scripts/tests/kanban-columns-selftest.py @@ -0,0 +1,64 @@ +#!/usr/bin/env python3 +"""Selftest for kanban-columns-check.py — no network, no board. + +A checker that cannot fail is worse than no checker: its green reads as +conformance. These drive the pure logic with a stubbed board so the failure +paths are exercised rather than assumed. +""" +from __future__ import annotations + +import importlib.util +import io +import contextlib +import sys +from pathlib import Path + +HERE = Path(__file__).resolve().parent +spec = importlib.util.spec_from_file_location("kcc", HERE.parent / "kanban-columns-check.py") +kcc = importlib.util.module_from_spec(spec) +spec.loader.exec_module(kcc) + +passed = failed = 0 + + +def check(ok: bool, name: str, detail: str = "") -> None: + global passed, failed + if ok: + passed += 1 + print(f"PASS {name}") + else: + failed += 1 + print(f"FAIL {name}\n {detail}") + + +def run(written, options): + """Drive main()'s comparison with both reads stubbed.""" + kcc.written_names = lambda: written + kcc.board_options = lambda: options + buf = io.StringIO() + with contextlib.redirect_stdout(buf): + code = kcc.main() + return code, buf.getvalue() + + +BOARD = {"On dev", "FR on staging", "Prod", "Done", "Cancelled", "Staging (agent review)"} + +code, out = run({"FR on staging": {"a.yml"}, "Prod": {"b.yml"}}, BOARD) +check(code == 0, "all written names present -> exit 0", f"code={code}") + +code, out = run({"Staging (human review)": {"advance-deploy-env.yml"}}, BOARD) +check(code == 1, "a written name absent from the board -> exit 1", f"code={code}") +check("advance-deploy-env.yml" in out, + "the failure names the file that writes it", out[:200]) +check("Staging (human review)" in out, "the failure names the offending value", out[:200]) + +# The pre-#245 regression exactly: one good name and one stale one. +code, _ = run({"FR on staging": {"x.yml"}, "Staging (human review)": {"y.yml"}}, BOARD) +check(code == 1, "one stale name among good ones still fails", f"code={code}") + +# Case matters: the board lookup is exact, so a near-miss must not pass. +code, _ = run({"fr on staging": {"x.yml"}}, BOARD) +check(code == 1, "case-mismatched name does not silently pass", f"code={code}") + +print(f"\npass={passed} fail={failed}") +sys.exit(1 if failed else 0) From ed770442b1549c366a00c094d614f75e63beed5c Mon Sep 17 00:00:00 2001 From: lukasWuttke <54042461+LukasWodka@users.noreply.github.com> Date: Thu, 13 Aug 2026 22:33:43 +0200 Subject: [PATCH 7/7] fix(kanban): the column check missed three writers, and could not tell (#248) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(kanban): the column check missed three writers, and could not tell Two Bugbot findings on the staging promotion PR #243, both on code I added today. MEDIUM — the conformance check from #247 was itself incomplete, which is the exact defect it exists to prevent. `WRITERS` named two files and `LITERAL` matched one idiom, so it never saw: set-pr-status.yml `echo "status_name=In progress"` (UNQUOTED value) fr-pass-comment.yml `NEXT="Ready for prod"` Three column names -- In progress, Code review, Ready for prod -- were invisible to a guard built to make a rename impossible to miss. It reported a clean sweep of a subset, which reads exactly like a clean sweep of everything. Widening the hand-list alone would rot the same way, so the names are now derived a SECOND, independent way and the two are compared: any board column name ASSIGNED in a writer file that no idiom matched means the idiom list is stale, and that is an error rather than a smaller answer. Assignment sites only, whole values -- so comments, the rank `case` arms and prose are ignored, and `Ready` does not match inside `Ready for prod`. It earned that immediately: the cross-check caught my own first attempt at the widened regex, which required a trailing quote and silently stopped matching `STATUS_NAME="FR on staging" ;;`. Without it I would have shipped a check that found fewer names than before while printing "all names exist". Coverage went 5 names / 2 files -> 8 names / 4 files. LOW — the abort message added in #246 said "The Deploy environment write may already have happened". It cannot have: that check runs at :190 and the first `update_field` is at :314, so on that path nothing was written at all. Corrected to say so, since the point of the message is to tell an operator where they stand. Selftest 8/8 (adds: an assigned-but-unmatched name is reported; a comment mentioning a column is not). caller-drift selftest 162/0. ruff + actionlint clean. * fix(kanban): the PR trigger must cover every writer, and now cannot drift Bugbot Medium on this PR. I broadened `WRITERS` to four workflows and left `kanban-columns.yml`'s `paths:` filter listing the original two — so a PR touching only `set-pr-status.yml` or `fr-pass-comment.yml` never ran this check and a bad column write could merge until the next cron. Same shape as the defect this PR is already fixing: the guard is present, reports green, and is not watching the thing it names. Two files apart this time instead of two variables. The filter now lists all four. But listing them is what went wrong once already, so the selftest DERIVES the requirement: every entry in WRITERS must appear in the workflow's `paths:` block, read out of the YAML rather than restated. Adding a fifth writer without wiring its trigger is now a red test, not a silent gap. Verified by removing `set-pr-status.yml` from the filter: the assertion fails. Selftest 10/10 with it restored. ruff + actionlint clean. This is the third instance today of "fixed the code, left the trigger config" -- e2e#81 was the same bug in a tsc dry-run paths filter. Recorded in the message because the pattern is worth more than the fix. * style(selftest): split a combined import (ruff E401) It was in the block I added two commits ago and I claimed ruff clean without reading the exit code -- the `&&` swallowed it. Checked properly this time. --- .github/workflows/advance-deploy-env.yml | 3 +- .github/workflows/kanban-columns.yml | 6 ++ scripts/kanban-columns-check.py | 79 +++++++++++++++++++++--- scripts/tests/kanban-columns-selftest.py | 65 ++++++++++++++++++- 4 files changed, 144 insertions(+), 9 deletions(-) diff --git a/.github/workflows/advance-deploy-env.yml b/.github/workflows/advance-deploy-env.yml index eb5522f..e4d0a02 100644 --- a/.github/workflows/advance-deploy-env.yml +++ b/.github/workflows/advance-deploy-env.yml @@ -189,7 +189,8 @@ jobs: if [ -z "$STATUS_FIELD" ] || [ "$STATUS_FIELD" = "null" ] \ || [ -z "$STATUS_OPT" ] || [ "$STATUS_OPT" = "null" ]; then echo "::error::Could not resolve Status option '$STATUS_NAME' in project #$PROJECT_NUMBER." \ - "The Deploy environment write may already have happened; the Status write did not." \ + "NOTHING WAS WRITTEN: this check runs before any field update, so no card" \ + "was touched and there is no half-applied state to repair." \ "Check the board's Status column names against this workflow's branch map." >&2 exit 1 fi diff --git a/.github/workflows/kanban-columns.yml b/.github/workflows/kanban-columns.yml index 59a6d79..53a1aef 100644 --- a/.github/workflows/kanban-columns.yml +++ b/.github/workflows/kanban-columns.yml @@ -15,9 +15,15 @@ name: Kanban column conformance on: pull_request: + # MUST LIST EVERY FILE IN `WRITERS`. It listed only the original two, so a PR + # touching just set-pr-status.yml or fr-pass-comment.yml never ran this check + # and a bad column write could merge until the next cron (Bugbot, #248). The + # selftest asserts this list against WRITERS, so they cannot drift apart. paths: - ".github/workflows/advance-deploy-env.yml" - ".github/workflows/kanban-closure-router.yml" + - ".github/workflows/set-pr-status.yml" + - ".github/workflows/fr-pass-comment.yml" - ".github/workflows/kanban-columns.yml" - "scripts/kanban-columns-check.py" - "scripts/tests/kanban-columns-selftest.py" diff --git a/scripts/kanban-columns-check.py b/scripts/kanban-columns-check.py index 0bdd9c3..8dde9d3 100755 --- a/scripts/kanban-columns-check.py +++ b/scripts/kanban-columns-check.py @@ -39,11 +39,31 @@ # The workflows that WRITE a Status value. Listed rather than globbed: `STATUS=` # is a common shell variable name and other workflows use it for unrelated # values ("ok", "absent", "unreadable"), which are not column names. -WRITERS = ("advance-deploy-env.yml", "kanban-closure-router.yml") - -# `STATUS_NAME="On dev"` / `STATUS="Prod"`. Only literals: `STATUS_NAME="$OVERRIDE"` -# is a per-repo .kanban.yml value that cannot be known here. -LITERAL = re.compile(r'\bSTATUS(?:_NAME)?="([^"$]+)"') +# +# THIS LIST WAS WRONG ON ITS FIRST DAY and the check stayed green anyway, which +# is the whole reason for the cross-check below. It named two files and one +# idiom, and missed `set-pr-status.yml` (writes `status_name=In progress` +# UNQUOTED) and `fr-pass-comment.yml` (writes `NEXT="Ready for prod"`) -- so +# three column names were invisible to a guard built to make exactly that +# impossible (Bugbot, .github#243). +WRITERS = ( + "advance-deploy-env.yml", + "kanban-closure-router.yml", + "set-pr-status.yml", + "fr-pass-comment.yml", +) + +# The write idioms actually used. Quoted OR bare, because `echo "status_name=In +# progress" >> "$GITHUB_OUTPUT"` has no inner quotes. +# STATUS="Prod" STATUS_NAME="On dev" status_name=In progress NEXT="Ready for prod" +# `$`-containing values are skipped: `STATUS_NAME="$OVERRIDE"` is a per-repo +# .kanban.yml value that cannot be known here. +LITERAL = re.compile( + r'\b(?:STATUS(?:_NAME)?|status_name|NEXT)=' # the write idioms + r'(?:"([^"$\n]+)"' # quoted: STATUS="Prod" + r'|([^"$\n;)&|]+?)(?=\s*(?:;|\)|&|\||"|$)))', # bare: status_name=In progress + re.MULTILINE, +) PROJECT_ID = "PVT_kwDOCSsgos4BTDWN" # engineer kanban (org project #2) @@ -54,14 +74,49 @@ def written_names() -> "dict[str, set[str]]": path = WORKFLOWS / name if not path.is_file(): sys.exit(f"error: {path} not found — WRITERS is stale") - for value in LITERAL.findall(path.read_text()): - found.setdefault(value, set()).add(name) + text = path.read_text() + for quoted, bare in LITERAL.findall(text): + value = (quoted or bare).strip() + if value: + found.setdefault(value, set()).add(name) if not found: sys.exit("error: no Status literals found at all — the pattern is stale, " "which would make this check pass vacuously") return found +def cross_check(found: "dict[str, set[str]]", options: "set[str]") -> "list[str]": + """Catch the extractor UNDER-COLLECTING, which is how this check failed first. + + A precise extractor that silently misses an idiom reports a clean sweep of a + SUBSET -- indistinguishable from a clean sweep of everything. So the names are + derived a second, independent way and the two are compared. + + The second pass looks at ASSIGNMENT SITES only: a non-comment line with an + `=`, from which quoted and bare right-hand values are pulled and matched + WHOLE against board names. Comments, the rank `case` arms and prose mentions + are therefore ignored -- they name columns without writing them. Whole-value + matching also stops `Ready` matching inside `Ready for prod`, which a + substring scan does. + + One-directional on purpose: the precise pass may legitimately find MORE (a + name written but absent from the board is the primary finding), so only + crude-minus-precise indicates a stale idiom list. + """ + rhs = re.compile(r'=\s*(?:"([^"\n]*)"|([^"\n;)&|]*))') + stale: "list[str]" = [] + for name in WRITERS: + for line in (WORKFLOWS / name).read_text().splitlines(): + bare_line = line.strip() + if not bare_line or bare_line.startswith("#") or "=" not in bare_line: + continue + for quoted, unquoted in rhs.findall(bare_line): + value = (quoted or unquoted).strip().strip('"') + if value in options and value not in found: + stale.append(f"{value!r} is assigned in {name} but no idiom matched it") + return sorted(set(stale)) + + def board_options() -> "set[str]": query = ( 'query{node(id:"%s"){... on ProjectV2{field(name:"Status")' @@ -87,6 +142,16 @@ def board_options() -> "set[str]": def main() -> int: written, options = written_names(), board_options() + + stale = cross_check(written, options) + if stale: + print("ERROR: the idiom list is stale — a board column name appears in a " + "writer that no pattern matched, so this check would report a clean " + "sweep of a subset:") + for row in stale: + print(f" - {row}") + return 1 + missing = {n: s for n, s in written.items() if n not in options} for name in sorted(written): diff --git a/scripts/tests/kanban-columns-selftest.py b/scripts/tests/kanban-columns-selftest.py index ac530e1..44fe8b5 100755 --- a/scripts/tests/kanban-columns-selftest.py +++ b/scripts/tests/kanban-columns-selftest.py @@ -8,6 +8,7 @@ from __future__ import annotations import importlib.util +import re import io import contextlib import sys @@ -32,9 +33,16 @@ def check(ok: bool, name: str, detail: str = "") -> None: def run(written, options): - """Drive main()'s comparison with both reads stubbed.""" + """Drive main()'s comparison with both reads stubbed. + + `cross_check` is stubbed out too: it reads the real workflow files, so + against a synthetic `written` dict it correctly reports staleness and would + mask the missing-name assertions these cases exist for. It has its own + tests at the bottom of this file. + """ kcc.written_names = lambda: written kcc.board_options = lambda: options + kcc.cross_check = lambda found, options: [] buf = io.StringIO() with contextlib.redirect_stdout(buf): code = kcc.main() @@ -60,5 +68,60 @@ def run(written, options): code, _ = run({"fr on staging": {"x.yml"}}, BOARD) check(code == 1, "case-mismatched name does not silently pass", f"code={code}") +# --- the cross-check: an extractor that under-collects must not report clean --- +# This is the defect the check itself shipped with (.github#243): WRITERS named +# two files and one idiom, three column names were invisible, and it passed. +kcc.cross_check.__doc__ # touch, so a rename of the function fails loudly here + +# run() replaced cross_check with a stub; restore the real one for its own tests. +importlib_spec = importlib.util.spec_from_file_location("kcc_fresh", HERE.parent / "kanban-columns-check.py") +kcc_fresh = importlib.util.module_from_spec(importlib_spec) +importlib_spec.loader.exec_module(kcc_fresh) +kcc.cross_check = kcc_fresh.cross_check + +real_writers = kcc_fresh.WRITERS +try: + # A writer whose only Status name is written by an idiom the regex misses. + import os + import tempfile + tmp = tempfile.mkdtemp() + stale = os.path.join(tmp, "stale-writer.yml") + with open(stale, "w", encoding="utf-8") as fh: + fh.write(' run: |\n UNMATCHED_VAR="Ready for prod"\n') + kcc_fresh.WORKFLOWS = __import__("pathlib").Path(tmp) + kcc_fresh.WRITERS = ("stale-writer.yml",) + stale_rows = kcc_fresh.cross_check({}, BOARD | {"Ready for prod"}) + check(any("Ready for prod" in r for r in stale_rows), + "an assigned board name no idiom matched is reported as a stale idiom list", + f"rows={stale_rows}") + + # A mention that is NOT an assignment must not be flagged. + prose = os.path.join(tmp, "prose-writer.yml") + with open(prose, "w", encoding="utf-8") as fh: + fh.write(' # advances the card to Ready for prod eventually\n') + kcc_fresh.WRITERS = ("prose-writer.yml",) + check(kcc_fresh.cross_check({}, BOARD | {"Ready for prod"}) == [], + "a comment mentioning a column is not mistaken for a write") +finally: + kcc_fresh.WRITERS = real_writers + kcc_fresh.WORKFLOWS = HERE.parent.parent / ".github" / "workflows" + +# --- the PR trigger must cover every writer -------------------------------- +# Broadening WRITERS without broadening the workflow's `paths:` filter means a PR +# touching only a new writer never runs this check at all, and a bad column write +# merges until the next cron (Bugbot, #248). That is the same shape as the check +# itself shipping with an incomplete WRITERS list: the guard is present, looks +# green, and is not watching the thing it names. +# +# Derived from WRITERS rather than eyeballed, so the two cannot drift apart. +_wf_text = (HERE.parent.parent / ".github" / "workflows" / "kanban-columns.yml").read_text() +_paths_block = re.search(r"\n\s*paths:\s*\n((?:\s*-\s*'[^']*'|\s*-\s*\"[^\"]*\"\s*\n)+)", _wf_text) +check(_paths_block is not None, "the workflow still has a paths: block") +_listed = set(re.findall(r"-\s*[\"']([^\"']+)[\"']", _paths_block.group(1) if _paths_block else "")) +_uncovered = [w for w in kcc_fresh.WRITERS if ".github/workflows/" + w not in _listed] +check(_uncovered == [], + "every WRITERS entry is in the workflow's paths: filter", + "uncovered=" + repr(_uncovered)) + print(f"\npass={passed} fail={failed}") sys.exit(1 if failed else 0)