diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b1830d9..e1bc9e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,16 +35,20 @@ name: CI # every PR — add it to the ruleset AFTER it has merged to main, never # before, or every open PR deadlocks. # -# AND THAT IS NOT SUFFICIENT, learned by doing it: a PR opened BEFORE -# the job existed still does not have it, so it can never report and is -# blocked forever. When #167 added two jobs and they were required, PRs -# #168 and #169 each showed 11 checks with 0 of the 2 new ones -- both -# would have deadlocked. The complete procedure is: -# 1. merge the PR that adds the job -# 2. add the context to the ruleset AND to required-checks.txt, -# ruleset FIRST (CI gates on the file, so a file ahead of the -# ruleset means CI is gating on a fiction) +# AND THAT WAS NOT SUFFICIENT EITHER, learned by doing it twice. A PR opened +# BEFORE the job existed still does not have it, so it can never report and is +# blocked forever -- when #167 added two jobs and they were required, PRs #168 +# and #169 each showed 11 checks with 0 of the 2 and would have deadlocked. +# Worse, landing the job while its required-checks.txt entry arrived in a +# SEPARATE PR left main AND every open PR red until that follow-up merged. +# +# So the file entry now ships WITH the job, and the gate ENFORCES that: +# check-required-checks.py fails a PR that adds a requirable job which is not +# in required-checks.txt. The procedure is: +# 1. in the SAME PR: add the job here AND to .github/required-checks.txt +# 2. after it merges: add the context to the ruleset (instant, via the API) # 3. REBASE EVERY OPEN PR, or it can never satisfy the new check +# There is no window in which main is red. on: push: diff --git a/artifacts/roadmap-v3.4-commit-trace.yaml b/artifacts/roadmap-v3.4-commit-trace.yaml index 9b278d5..21bd577 100644 --- a/artifacts/roadmap-v3.4-commit-trace.yaml +++ b/artifacts/roadmap-v3.4-commit-trace.yaml @@ -127,6 +127,8 @@ artifacts: - "Given CI, When the gate runs, Then it uses `--against-file` against `.github/required-checks.txt` with no API call. This catches the rot WE cause in our own PRs: a job added or renamed without updating the file. MUTATION-CHECKED: deleting `Clippy` from the file fails with `job Clippy exists on main and runs on every PR, but is NOT required`." - "Given admin credentials, When the gate runs in its default LIVE mode, Then it additionally cross-checks the file against the real ruleset, because CI gates on the file and a drifted file means CI is gating on a fiction. MUTATION-CHECKED: the same deletion fails LIVE mode with a DIFFERENT and correct message, `Clippy is required LIVE but missing from .github/required-checks.txt`." - "Given the ruleset cannot be read in LIVE mode, When the gate runs, Then it FAILS CLOSED pointing at `--against-file` for CI. Inability to check is not evidence of correctness (scry#141)." + - "REFINED AFTER OPERATING IT, and the refinement is the point. Landing a job while its `required-checks.txt` entry arrives in a SEPARATE PR leaves main AND every open PR red until the follow-up merges — scry#167 did exactly that, taking #168 and #169 down with it. The gate now FAILS a PR that adds a requirable job which is not in the file, so the two ship together and no red window exists. VERIFIED on the real repo in both directions: adding a job with no file entry exits 1 naming it and quoting the instruction; adding the same job WITH its entry exits 0 (pending the ruleset update only)." + - "The full procedure is recorded in ci.yml and is now three steps with no red window: (1) in the SAME PR add the job and its required-checks.txt entry; (2) after merge add the context to the ruleset via the API, which is instant; (3) rebase every open PR. Steps 2 and 3 were each learned by getting them wrong — a required context that does not yet exist deadlocks nothing, but a required context whose job does not exist deadlocks everything." - "SELF-TESTED, run BEFORE the real check in CI: 6 cases over the pure `verdict()` function covering all four failure directions plus the pending case." residual: > A RULESET RESET IS STILL NOT CAUGHT BY CI, and cannot be: GITHUB_TOKEN diff --git a/tools/check-required-checks.py b/tools/check-required-checks.py index 7d34c6b..36f5eb1 100755 --- a/tools/check-required-checks.py +++ b/tools/check-required-checks.py @@ -77,8 +77,16 @@ def workflow_jobs(text): return out -def verdict(main_jobs, pr_jobs, required): - """Pure -> (failures, pending). Drives --self-test.""" +def verdict(main_jobs, pr_jobs, required, in_file=None): + """Pure -> (failures, pending). Drives --self-test. + + `in_file` is the checked-in required-checks.txt contents when known; a job + new in this PR that is absent from it is a FAILURE, not merely pending, + because shipping the job without the file entry is what opens the red + window (see below). + """ + if in_file is None: + in_file = required fails, pending = [], [] for name, (requirable, why) in main_jobs.items(): if requirable and name not in required: @@ -94,7 +102,23 @@ def verdict(main_jobs, pr_jobs, required): f"it can never report, so every PR deadlocks") for name, (requirable, _) in pr_jobs.items(): if requirable and name not in main_jobs and name not in required: + # A job NEW in this PR must be added to required-checks.txt IN THE + # SAME PR. Observed the hard way: #167 added two jobs without it, + # so the moment it merged, main and every open PR failed this gate + # until a follow-up PR (#170) landed the file. Requiring both + # together removes that window entirely -- on merge the file + # already lists them, and only the ruleset needs its (instant) + # update. pending.append(name) + for name, (requirable, _) in pr_jobs.items(): + if requirable and name not in main_jobs and name not in in_file: + fails.append( + f"job {name!r} is new in this PR but is NOT in " + f"{REQUIRED_FILE} -- add it in THIS PR. Landing the job " + f"without the file entry makes main, and every open PR, fail " + f"this gate until a follow-up lands the file (scry#167 did " + f"exactly that)" + ) for name, why in EXPECTED_EXCLUSIONS.items(): if name in pr_jobs and pr_jobs[name][0]: fails.append(f"{name!r} is listed as a deliberate exclusion ({why}) but is now " @@ -111,12 +135,22 @@ def self_test(): ("a path-filtered job IS required (deadlock)", {"A": R(), "D": N("workflow is path-filtered")}, {"A": R(), "D": N("x")}, {"A", "D"}, 1, 0), ("a required context matches no job (renamed)", {"A": R()}, {"A": R()}, {"A", "Gone"}, 1, 0), - ("a NEW job in this PR is pending, not a failure", {"A": R()}, {"A": R(), "New": R()}, {"A"}, 0, 1), + # A new job WITH its file entry: pending only (the ruleset still needs + # its instant update), no failure. + ("a NEW job carrying its file entry is pending, not a failure", + {"A": R()}, {"A": R(), "New": R()}, {"A"}, 0, 1, {"A", "New"}), + # A new job WITHOUT its file entry: this is the scry#167 shape, and it + # must FAIL here rather than after merge, when it takes main and every + # open PR down with it. + ("a NEW job missing from the file FAILS in this PR", + {"A": R()}, {"A": R(), "New": R()}, {"A"}, 1, 1, {"A"}), ("ruleset reset: nothing required at all", {"A": R(), "B": R()}, {"A": R(), "B": R()}, set(), 2, 0), ] bad = 0 - for name, mj, pj, req, wf, wp in cases: - f, p = verdict(mj, pj, req) + for case in cases: + name, mj, pj, req, wf, wp = case[:6] + in_file = case[6] if len(case) > 6 else None + f, p = verdict(mj, pj, req, in_file) ok = len(f) == wf and len(p) == wp bad += not ok print(f" [{'ok' if ok else 'SELF-TEST FAILED'}] {name}: {len(f)} fail / {len(p)} pending, " @@ -191,7 +225,7 @@ def main(): print(f" required contexts: {len(required)}; jobs on main: {len(main_jobs)}; " f"jobs in this PR: {len(pr_jobs)}") - fails, pending = verdict(main_jobs, pr_jobs, required) + fails, pending = verdict(main_jobs, pr_jobs, required, from_file) fails += extra for p in pending: print(f" PENDING (not a failure): job {p!r} is new in this PR. Add it to the "