check_ci_conformance: tier-skipped required checks; review fixes

.github/workflows/ci-conformance.yml

@@ -4,9 +4,13 @@ name: CI conformance
 # requires this workflow on every repository whose ci-tier property is
 # merge-queue. It runs from this repository, on the target repository's pull
 # requests and merge groups, and checks the target's workflows against CI.md.
+#
+# A ruleset runs a required workflow on the default activity types only
+# (opened, synchronize, reopened, and the merge group's checks_requested) and
+# ignores `types` and other filters. So the job runs on drafts too: a draft
+# marked ready gets no new event, and the check is cheap.
 on:
   pull_request:
-    types: [opened, synchronize, reopened, ready_for_review, stacked]
   merge_group:
 
 concurrency:
@@ -19,7 +23,7 @@ permissions:
 jobs:
   ci-conformance:
     # This repository has no ci-gate of its own; self-test.yml covers it.
-    if: github.event.pull_request.draft != true && github.repository != 'cocoindex-io/.github'
+    if: github.repository != 'cocoindex-io/.github'
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v5

.gitignore

@@ -0,0 +1 @@
+__pycache__/

CI.md

@@ -49,15 +49,22 @@ workflow has all of them.
    in its `if`, or needs a job that has it.
 4. **A `changes` job** runs `cocoindex-io/.github/actions/ci-mode@v1` (it
    needs `checks: read`) and the path filter. Expensive jobs run when
-   `full == 'true'` and the filter output is not `'false'`.
+   `full == 'true'` and the filter output is not `'false'`. The filter is
+   skipped for a stack layer, and so is any other diff against the pull
+   request's base branch: a stack layer's base is the layer below it, but
+   the top layer's run stands in for the whole stack. Diff a stack layer
+   against `github.event.pull_request.stack.base.ref`.
 5. **A `ci-gate` job** with `if: always()` that needs every other job. It is
    the one check to require. A skipped job reports success, so a required
    check that can be skipped by a failure would let the failure merge.
 
 Every other workflow that runs on pull requests has parts 2 and 3, the
 `ready_for_review` type, and the `stacked` type when it filters on branches.
 Every workflow that reports a required check triggers on `merge_group`, or
-the queue waits for that check until it times out.
+the queue waits for that check until it times out. Require `ci-gate`, plus
+checks from jobs that run in every tier, such as `fast-check`. A matrix job
+that the tier skips reports one check under its unexpanded name, so a
+required `name (value)` check from it never arrives.
 
 ## The merge queue
 
@@ -121,6 +128,10 @@ repository's own rulesets.
   the Change Management Policy asks that the change be announced.
 - Check a repository locally:
   `uv run --with pyyaml scripts/check_ci_conformance.py PATH --repo OWNER/NAME`.
+- A ruleset runs a required workflow only on `opened`, `synchronize` and
+  `reopened` pull request events and on merge groups, and ignores the
+  workflow's `types` and filters. `ci-conformance` therefore runs on drafts
+  too.
 
 ## Changing this kit
 

scripts/apply_rulesets.py

@@ -69,7 +69,10 @@ def sync_property(org: str, check: bool) -> bool:
 
 def sync_ruleset(org: str, path: pathlib.Path, check: bool) -> bool:
     want = json.loads(path.read_text())
-    existing = {r["name"]: r["id"] for r in gh(f"orgs/{org}/rulesets", "--paginate")}
+    # `--slurp` wraps the pages in one array; `--paginate` alone prints one
+    # JSON array per page, which is not one JSON document.
+    pages = gh(f"orgs/{org}/rulesets", "--paginate", "--slurp")
+    existing = {r["name"]: r["id"] for page in pages for r in page}
     if want["name"] in existing:
         have = gh(f"orgs/{org}/rulesets/{existing[want['name']]}")
         found = differences(want, have)

scripts/check_ci_conformance.py

@@ -25,6 +25,9 @@
 CI_MODE_ACTION = "cocoindex-io/.github/actions/ci-mode@"
 GATE_JOB = "ci-gate"
 GITHUB_ACTIONS_APP_ID = 15368
+# Status functions in a job's `if` replace the implicit success() check, so
+# the job runs even when a job it needs was skipped.
+STATUS_FUNCTIONS = ("always()", "cancelled()", "failure()")
 
 
 class Finding(NamedTuple):
@@ -71,14 +74,19 @@ def fail(message: str) -> None:
         if (has_gate or branch_filtered) and "stacked" not in types:
             fail("pull_request.types must list stacked: a stack layer whose opened event arrives before its "
                  "stack exists misses the branch filter and runs no CI.")
-        group = _text((doc.get("concurrency") or {}).get("group"))
-        cancel = (doc.get("concurrency") or {}).get("cancel-in-progress")
+        concurrency = doc.get("concurrency") or {}
+        if isinstance(concurrency, str):  # `concurrency: <group>` is the short form
+            concurrency = {"group": concurrency}
+        group = _text(concurrency.get("group"))
+        cancel = concurrency.get("cancel-in-progress")
         if "pull_request.number" not in group or cancel in (None, False):
             fail("A workflow-level concurrency group keyed on github.event.pull_request.number, "
                  "with cancel-in-progress, must cancel a pull request's superseded runs.")
         for job_id, job in jobs.items():
             condition = _text(job.get("if"))
-            inherits_skip = bool(job.get("needs")) and "always()" not in condition
+            # A job that needs a skipped job is skipped too, unless a status
+            # function in its `if` overrides the implicit success() check.
+            inherits_skip = bool(job.get("needs")) and not any(f in condition for f in STATUS_FUNCTIONS)
             if "pull_request.draft" not in condition and not inherits_skip:
                 fail(f"Job {job_id} must skip drafts: add `github.event.pull_request.draft != true` to its if.")
 
@@ -108,19 +116,56 @@ def fail(message: str) -> None:
     return findings
 
 
-def check_name_prefixes(doc: dict[Any, Any]) -> list[str]:
-    """Return the check-name prefixes the workflow's jobs report under."""
-    prefixes = []
-    for job_id, job in (doc.get("jobs") or {}).items():
-        name = _text(job.get("name")) or job_id
-        prefixes.append(name.split("${{")[0].strip())
-    return prefixes
+def _needs(job: dict[str, Any]) -> list[str]:
+    needs = job.get("needs") or []
+    return [needs] if isinstance(needs, str) else list(needs)
 
 
-def check_required_contexts(contexts: list[str], merge_group_docs: list[dict[Any, Any]]) -> list[str]:
-    """Return the required contexts that no merge_group workflow reports."""
-    prefixes = [p for doc in merge_group_docs for p in check_name_prefixes(doc)]
-    return [c for c in contexts if not any(p and c.startswith(p) for p in prefixes)]
+def _tier_skipped_jobs(jobs: dict[str, Any]) -> set[str]:
+    """Jobs a fast-tier run skips: those whose `if` reads the tier, and those that need one."""
+    skipped = {job_id for job_id, job in jobs.items() if "outputs.full" in _text(job.get("if"))}
+    grew = True
+    while grew:
+        grew = False
+        for job_id, job in jobs.items():
+            if (job_id not in skipped and any(n in skipped for n in _needs(job))
+                    and not any(f in _text(job.get("if")) for f in STATUS_FUNCTIONS)):
+                skipped.add(job_id)
+                grew = True
+    return skipped
+
+
+def _reports(job_id: str, job: dict[str, Any], context: str) -> bool:
+    """Whether the job reports `context`: as its name, a matrix leg `name (…)`, or a reusable workflow's `name / …`."""
+    name = _text(job.get("name")) or job_id
+    if "${{" in name:
+        return context.startswith(name.split("${{")[0])
+    return context == name or context.startswith((f"{name} (", f"{name} / "))
+
+
+def required_check_problems(contexts: list[str], merge_group_docs: list[dict[Any, Any]]) -> list[str]:
+    """Explain each required context that would leave a pull request or merge group waiting."""
+    problems = []
+    for context in contexts:
+        producers = []  # (job id, skipped by the fast tier, the name it reports when skipped)
+        for doc in merge_group_docs:
+            jobs: dict[str, Any] = doc.get("jobs") or {}
+            skipped = _tier_skipped_jobs(jobs)
+            producers += [
+                (job_id, job_id in skipped, _text(job.get("name")) or job_id)
+                for job_id, job in jobs.items()
+                if _reports(job_id, job, context)
+            ]
+        if not producers:
+            problems.append(f"Required check {context!r} comes from no workflow that triggers on merge_group, "
+                            "so the merge queue would wait for it forever.")
+        # A skipped job reports one check under its own name: a skipped matrix
+        # job doesn't expand into its legs, and a skipped reusable-workflow
+        # call reports none of the called workflow's jobs.
+        elif all(skipped and context != name for _, skipped, name in producers):
+            problems.append(f"Required check {context!r} comes from job {producers[0][0]}, which the fast tier "
+                            f"skips; a skipped job reports only its own name. Require {GATE_JOB} instead.")
+    return problems
 
 
 def required_contexts(repo: str) -> list[str] | None:
@@ -170,9 +215,8 @@ def main() -> int:
         findings.append(Finding(str(directory), f"Exactly one workflow must define {GATE_JOB}; found {len(gate_files)}."))
     if args.repo:
         contexts = required_contexts(args.repo)
-        for context in check_required_contexts(contexts or [], merge_group_docs):
-            findings.append(Finding(str(directory), f"Required check {context!r} comes from no workflow that "
-                                    "triggers on merge_group, so the merge queue would wait for it forever."))
+        for problem in required_check_problems(contexts or [], merge_group_docs):
+            findings.append(Finding(str(directory), problem))
 
     annotate = os.environ.get("GITHUB_ACTIONS") == "true"
     for finding in findings:

scripts/test_check_ci_conformance.py

@@ -4,7 +4,7 @@
 
 import yaml
 
-from check_ci_conformance import check_required_contexts, check_workflow
+from check_ci_conformance import check_workflow, required_check_problems
 
 TEMPLATE = pathlib.Path(__file__).parent.parent / "workflow-templates" / "ci.yml"
 
@@ -109,4 +109,78 @@ def test_required_contexts_match_job_names_and_matrix_prefixes() -> None:
     runs-on: x
 """)
     contexts = ["ci-gate", "pre-commit (3.11)", "rust-checks (ubuntu-latest)", "fast-check"]
-    assert check_required_contexts(contexts, [doc]) == ["fast-check"]
+    problems = required_check_problems(contexts, [doc])
+    assert len(problems) == 1 and "'fast-check'" in problems[0]
+
+
+def test_required_check_from_tier_skipped_matrix_job() -> None:
+    doc = yaml.safe_load("""
+on: [merge_group]
+jobs:
+  pre-commit:
+    needs: changes
+    if: needs.changes.outputs.full == 'true'
+    strategy: {matrix: {python-version: ['3.11']}}
+    runs-on: x
+""")
+    problems = required_check_problems(["pre-commit (3.11)"], [doc])
+    assert len(problems) == 1 and "Require ci-gate instead" in problems[0]
+
+
+def test_status_function_overrides_inherited_draft_skip() -> None:
+    doc = template()
+    doc["jobs"]["test"]["if"] = "!cancelled()"
+    assert any("Job test must skip drafts" in m for m in messages(doc))
+
+
+def test_short_form_concurrency() -> None:
+    doc = template()
+    doc["concurrency"] = "ci-${{ github.event.pull_request.number }}"
+    assert any("cancel-in-progress" in m for m in messages(doc))
+
+
+def test_required_check_match_is_exact_or_expanded() -> None:
+    doc = yaml.safe_load("""
+on: [merge_group]
+jobs:
+  test: {runs-on: x}
+""")
+    problems = required_check_problems(["test-e2e"], [doc])
+    assert len(problems) == 1 and "no workflow that triggers on merge_group" in problems[0]
+
+
+def test_required_check_from_tier_skipped_reusable_workflow() -> None:
+    doc = yaml.safe_load("""
+on: [merge_group]
+jobs:
+  changes: {runs-on: x}
+  build-test:
+    name: Build & Test
+    needs: changes
+    if: needs.changes.outputs.full == 'true' && needs.changes.outputs.build-test != 'false'
+    uses: ./.github/workflows/_test.yml
+""")
+    problems = required_check_problems(["Build & Test / build-test (ubuntu-latest)", "changes"], [doc])
+    assert len(problems) == 1 and "Build & Test / build-test" in problems[0]
+
+
+def test_required_check_from_job_skipped_through_needs() -> None:
+    doc = yaml.safe_load("""
+on: [merge_group]
+jobs:
+  changes: {runs-on: x}
+  build:
+    needs: changes
+    if: needs.changes.outputs.full == 'true'
+    runs-on: x
+  test:
+    needs: build
+    strategy: {matrix: {os: [a]}}
+    runs-on: x
+  report:
+    needs: test
+    if: always()
+    runs-on: x
+""")
+    problems = required_check_problems(["test (a)", "report", "build"], [doc])
+    assert len(problems) == 1 and "'test (a)'" in problems[0]

workflow-templates/ci.yml

@@ -49,10 +49,13 @@ jobs:
       - id: mode
         uses: cocoindex-io/.github/actions/ci-mode@v1
       # A skipped filter leaves its outputs empty, and an empty output runs
-      # the job: dispatched and scheduled runs test everything.
+      # the job: dispatched and scheduled runs test everything. So does a
+      # stack's top layer: on a pull request the filter lists the files the
+      # layer itself changed, but its run stands in for the whole stack.
       - id: filter
         if: >-
           steps.mode.outputs.full == 'true'
+          && !github.event.pull_request.stack
           && contains(fromJSON('["pull_request", "push", "merge_group"]'), github.event_name)
         uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3
         with: