diff --git a/.github/workflows/advance-deploy-env.yml b/.github/workflows/advance-deploy-env.yml index c7d40c8..e4d0a02 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,37 +167,37 @@ 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 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." \ + "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 + # 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 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..a747e84 --- /dev/null +++ b/.github/workflows/bricked-prs.yml @@ -0,0 +1,48 @@ +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 + # 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 + 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/.github/workflows/kanban-closure-router.yml b/.github/workflows/kanban-closure-router.yml index 386a1af..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 @@ -284,10 +274,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 diff --git a/.github/workflows/kanban-columns.yml b/.github/workflows/kanban-columns.yml new file mode 100644 index 0000000..53a1aef --- /dev/null +++ b/.github/workflows/kanban-columns.yml @@ -0,0 +1,69 @@ +# 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: + # 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" + 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/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/kanban-columns-check.py b/scripts/kanban-columns-check.py new file mode 100755 index 0000000..8dde9d3 --- /dev/null +++ b/scripts/kanban-columns-check.py @@ -0,0 +1,175 @@ +#!/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. +# +# 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) + + +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") + 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")' + '{... 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() + + 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): + 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/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) diff --git a/scripts/tests/kanban-columns-selftest.py b/scripts/tests/kanban-columns-selftest.py new file mode 100755 index 0000000..44fe8b5 --- /dev/null +++ b/scripts/tests/kanban-columns-selftest.py @@ -0,0 +1,127 @@ +#!/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 re +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. + + `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() + 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}") + +# --- 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)