From 4026faf6b36cbbd7778f8d41384074561eaec19d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 30 Jul 2026 12:36:11 -0500 Subject: [PATCH 1/2] ci: a required context whose workflow is disabled can never report THE DEFECT, measured on the sibling vault repo (wshallwshall/MessageFoundry) on 2026-07-30: 10 required contexts whose workflows were ALL `disabled_manually`. A disabled workflow never dispatches, so those contexts never reported -- NO pull request could merge, and every merge there had silently been riding admin bypass. It sat that way unnoticed because the symptom presents to a human as "CI is stuck", not as "branch protection is misconfigured". Found by the ASVS session, filed unclaimed, picked up here. tests/test_required_contexts.py CANNOT see it. It resolves each required context against job names in .github/workflows/, and a disabled workflow keeps its file and its job name on disk -- so that check passes cheerfully in the healthy case AND the broken one. The property it measures ("a job with this name exists in YAML") is true either way. Same defect shape as a CI monitor polling for "nothing pending": sound about the thing it looks at, blind to the thing that can fail. Workflow `state` is server-side, so no file-based test can reach it. SCHEDULED (07:00 UTC, an hour after the nightly CI cron), not per-PR: a workflow disabled AFTER the last pull request is invisible to any per-PR check, because there is no PR to run it on. The failure arrives while the repo is idle, which is exactly when nobody is looking. It also runs on PRs touching required-contexts.txt or .github/workflows/** -- the one moment a human is changing the mapping, when a typo is cheapest to catch. Scope is REACHABILITY, not outcome: whether a check passes is CI's job; whether it can run at all is this one's. Needs only `actions: read` -- notably NOT the admin scope that reading branch protection requires. Reuses tests/_workflow_contexts.resolve rather than growing a second context->workflow mapping, and a parity test fails if that import is ever dropped. One resolver, two callers, because a second implementation is exactly the drift this repo keeps writing parity tests to catch. FAIL-CLOSED throughout: an unreadable payload, a failed API call, zero workflows returned, or zero parsed contexts all exit 2. "We could not measure" must never read as "everything is fine" -- that is the same blindness one level up. The all-unreachable case is called out BY NAME, because it is the one that cost real time: it means nothing can merge, and the only visible symptom is PRs hanging. A single disabled workflow deliberately does NOT trigger that message (a test pins that), so it cannot cry wolf. VERIFIED end-to-end against the REAL live API payload, not a hand-written fixture: live payload, all active -> exit 0 security.yml flipped to disabled -> exit 1, naming all 7 contexts it owns every workflow disabled -> exit 1, with the vault-shape diagnosis Exit codes captured WITHOUT a pipeline -- the first probe reported 0 for the failing case because $LASTEXITCODE after a pipe reflects Select-String, not python. The probe lied before the code did. Measurement DATED so it cannot age into a false claim: on 2026-07-30 MEFORORG had 19 workflows, all active, and 13/13 required contexts resolved to active workflows. The hazard is real but was not live here on that date. --- .github/workflows/required-workflow-state.yml | 58 ++++++ scripts/ci/check_required_workflow_state.py | 176 +++++++++++++++++ tests/test_required_workflow_state.py | 177 ++++++++++++++++++ 3 files changed, 411 insertions(+) create mode 100644 .github/workflows/required-workflow-state.yml create mode 100644 scripts/ci/check_required_workflow_state.py create mode 100644 tests/test_required_workflow_state.py diff --git a/.github/workflows/required-workflow-state.yml b/.github/workflows/required-workflow-state.yml new file mode 100644 index 00000000..cb43176d --- /dev/null +++ b/.github/workflows/required-workflow-state.yml @@ -0,0 +1,58 @@ +name: Required workflow state + +# Every REQUIRED status check must belong to a workflow GitHub will actually run. +# +# THE DEFECT THIS EXISTS FOR — measured on the sibling vault repo (wshallwshall/MessageFoundry), +# 2026-07-30: 10 required contexts whose workflows were ALL `disabled_manually`. A disabled workflow +# never dispatches, so those contexts never reported — NO pull request could merge, and every merge +# there had silently been riding admin bypass. Nobody noticed, because the symptom presents as "CI is +# stuck", not as "branch protection is misconfigured". +# +# `tests/test_required_contexts.py` cannot see this: a disabled workflow keeps its file and job name on +# disk, so resolving a context against YAML passes in the healthy AND the broken case. Workflow `state` +# is server-side, so this has to be an API check. +# +# SCHEDULED, not per-PR, deliberately. A workflow disabled AFTER the last pull request is invisible to +# any per-PR check — there is no PR to run it on. The failure arrives while the repo is idle, which is +# exactly when nobody is looking. +# +# It checks REACHABILITY (can the context ever report?), not outcome. Whether a check passes is CI's +# job; whether it is capable of running at all is this one's. +on: + schedule: + # 07:00 UTC — an hour after the nightly CI cron, so a workflow disabled overnight is reported the + # same morning rather than a day later. + - cron: "0 7 * * *" + workflow_dispatch: + # Also on a PR that edits the required set or the workflows it points at: this is the one moment a + # human is actively changing the mapping, and a typo'd context is cheapest to catch right then. + pull_request: + paths: + - ".github/required-contexts.txt" + - ".github/workflows/**" + +permissions: + contents: read + +jobs: + reachable: + name: required contexts belong to active workflows + runs-on: ubuntu-latest + permissions: + contents: read + actions: read # read workflow `state`; NOT the admin scope branch protection would need + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.14" + - name: Install PyYAML (the resolver parses the workflow files) + run: | + # Hash-pinned from the CI toolchain lock, like every other scanner install (ADR 0034 §3). + python -m pip install --require-hashes -r ci/locks/ci-scanners.lock + - name: Reconcile required contexts against workflow state + env: + GH_TOKEN: ${{ github.token }} + run: python scripts/ci/check_required_workflow_state.py --repo "$GITHUB_REPOSITORY" diff --git a/scripts/ci/check_required_workflow_state.py b/scripts/ci/check_required_workflow_state.py new file mode 100644 index 00000000..bce9454c --- /dev/null +++ b/scripts/ci/check_required_workflow_state.py @@ -0,0 +1,176 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Every REQUIRED status check must belong to a workflow GitHub will actually run. + +THE DEFECT THIS EXISTS FOR — measured, on a sibling repo, 2026-07-30. The private vault repo +(``wshallwshall/MessageFoundry``) had 10 required contexts whose workflows were all +``disabled_manually``. A disabled workflow never dispatches, so those contexts never reported: NO pull +request could ever merge, and every merge there had silently been riding admin bypass. It sat like +that long enough for nobody to notice, because the symptom presents to a human as *"CI is stuck"* — +not as *"branch protection is misconfigured"*. + +WHY THE EXISTING GUARD CANNOT SEE IT. ``tests/test_required_contexts.py`` resolves each required +context against the job names in ``.github/workflows/``. A ``disabled_manually`` workflow keeps its +file and its job name on disk, so that check passes cheerfully while the context can never report. The +property it measures — "a job with this name exists in YAML" — is true in both the healthy case and the +broken one. That is the same defect shape as a CI monitor polling for "nothing pending": sound about +the thing it looks at, blind to the thing that can fail. Workflow ``state`` is server-side and +invisible to any file-based test, which is why this lives in a scheduled job rather than in pytest. + +WHY SCHEDULED AND NOT PER-PR. A workflow disabled *after* the last pull request is invisible to any +per-PR check — there is no PR to run it on. The failure arrives while the repository is idle, which is +exactly when nobody is looking. A daily sweep is the only shape that catches it. + +SCOPE. This checks REACHABILITY (can the context ever report?), not outcome. Whether a check passes is +CI's job; whether it is capable of running at all is this one's. + +USAGE + python scripts/ci/check_required_workflow_state.py # uses gh's auth + python scripts/ci/check_required_workflow_state.py --repo owner/name + python scripts/ci/check_required_workflow_state.py --states-json states.json # offline/testing +""" + +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +from pathlib import Path + +_ROOT = Path(__file__).resolve().parents[2] + +# Reuse the SAME resolver the doc-drift tests use, rather than re-deriving context -> workflow here. +# A second implementation of that mapping is exactly the drift this repo keeps writing parity tests to +# catch (ruff/bandit scan scope, the required-set prose). One resolver, two callers. +sys.path.insert(0, str(_ROOT)) +from tests._workflow_contexts import required_contexts, resolve # noqa: E402 + +#: GitHub reports a runnable workflow as ``active``. Everything else -- ``disabled_manually``, +#: ``disabled_inactivity`` (60 days idle on a fork/scheduled-only repo), ``disabled_fork`` -- means it +#: will not dispatch, so a required context it owns can never report. +_RUNNABLE = "active" + + +def _workflow_states(repo: str | None, states_json: Path | None) -> dict[str, str]: + """``{workflow filename: state}`` for every workflow in the repo. + + Keyed on the FILENAME rather than the display name: the display name is what ``workflow_run`` + matches on, but a required context resolves to a FILE, and two workflows may legitimately share a + display name (this repo has two called "CodeQL"). + """ + if states_json is not None: + payload = json.loads(states_json.read_text(encoding="utf-8")) + else: + cmd = ["gh", "api", "--paginate"] + cmd.append( + f"repos/{repo}/actions/workflows" if repo else "repos/{owner}/{repo}/actions/workflows" + ) + # B603 asks whether untrusted input reaches a subprocess. It cannot here: argv is a fixed + # literal list, there is no shell, and the only variable element is `--repo`, an operator-typed + # CLI argument on a CI runner — not message, config, or network data. Annotated per-line with a + # reason, the posture security.yml's bandit notes require. + out = subprocess.run( # noqa: S603 # nosec B603 — fixed argv, no shell, operator-supplied repo + cmd, capture_output=True, text=True, timeout=120 + ) + if out.returncode != 0: + raise RuntimeError(f"gh api failed ({out.returncode}): {out.stderr.strip()[:400]}") + payload = json.loads(out.stdout) + workflows = payload.get("workflows", payload if isinstance(payload, list) else []) + return {Path(str(w.get("path", ""))).name: str(w.get("state", "")) for w in workflows} + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + parser.add_argument("--repo", default=None, help="owner/name; defaults to gh's current repo") + parser.add_argument( + "--states-json", type=Path, default=None, help="a saved API payload (testing)" + ) + args = parser.parse_args(argv) + + contexts = required_contexts() + if not contexts: + print( + "::error::.github/required-contexts.txt parsed to ZERO contexts — the format changed under " + "the parser. That is a broken check, not a clean sweep.", + file=sys.stderr, + ) + return 2 + + try: + states = _workflow_states(args.repo, args.states_json) + except (RuntimeError, json.JSONDecodeError, subprocess.SubprocessError, OSError) as exc: + # FAIL CLOSED. "We could not read workflow state" must never read as "every workflow is fine" — + # that is the same blindness this script exists to catch, one level up. + print( + f"::error::could not read workflow state ({exc!r}). Treating as a FAILURE.", + file=sys.stderr, + ) + return 2 + if not states: + print( + "::error::the API returned ZERO workflows — refusing to report success.", + file=sys.stderr, + ) + return 2 + + unreachable: list[str] = [] + unresolved: list[str] = [] + checked = 0 + for ctx in contexts: + where = resolve(ctx) + if where is None: + unresolved.append(ctx) + continue + workflow = where[0] + state = states.get(workflow) + if state is None: + unresolved.append(f"{ctx} (workflow {workflow} not present on the server)") + continue + checked += 1 + if state != _RUNNABLE: + unreachable.append(f"{ctx} -> {workflow} [state={state}]") + + # Liveness receipt: report what was EXAMINED. "no unreachable contexts" and "nothing was checked" + # are otherwise indistinguishable from the exit code. + print(f"required-workflow-state: checked {checked} of {len(contexts)} required contexts") + + if unresolved: + for item in unresolved: + print( + f"::error::required context resolves to no runnable workflow: {item}", + file=sys.stderr, + ) + return 1 + + if unreachable: + # THE VAULT SHAPE, called out by name. When EVERY required context is unreachable, no pull + # request can merge at all, and the only visible symptom is that PRs hang — which reads as + # flakiness, not misconfiguration. Say the diagnosis out loud so nobody spends a day on it. + if len(unreachable) == checked: + print( + "::error::EVERY required context belongs to a non-active workflow. NO pull request can " + "merge — each required check will hang as 'Expected — waiting for status to be " + "reported' forever. This presents as 'CI is stuck'; it is branch protection pointing at " + "workflows GitHub will not run. Measured in this exact state on the vault repo " + "(2026-07-30), where every merge had silently been riding admin bypass.", + file=sys.stderr, + ) + for item in unreachable: + print(f"::error::{item}", file=sys.stderr) + print( + "\nRe-enable the workflow (`gh workflow enable `), or remove the context from branch " + "protection AND .github/required-contexts.txt — deliberately, in a reviewed diff.", + file=sys.stderr, + ) + return 1 + + print(f"required-workflow-state: all {checked} required contexts belong to active workflows.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_required_workflow_state.py b/tests/test_required_workflow_state.py new file mode 100644 index 00000000..3cbbad07 --- /dev/null +++ b/tests/test_required_workflow_state.py @@ -0,0 +1,177 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Tests for the required-context reachability check. + +THE DEFECT IT GUARDS — measured on the sibling vault repo (``wshallwshall/MessageFoundry``), +2026-07-30: 10 required contexts whose workflows were all ``disabled_manually``. A disabled workflow +never dispatches, so NO pull request could merge and every merge had silently been riding admin +bypass — invisible because the symptom reads as "CI is stuck", not "protection is misconfigured". + +``tests/test_required_contexts.py`` cannot catch that: a disabled workflow keeps its file and job name +on disk, so resolving a context against YAML passes in the healthy *and* the broken case. + +WHY THESE TESTS DRIVE THE REAL ``main()``. Every case below runs the shipped entry point through its +``--states-json`` seam rather than re-implementing the rule. A guard whose test asserts a local copy of +the logic is a guard nobody has run — and this repo has been bitten by exactly that. + +MEASUREMENT, DATED so it cannot age into a false claim: on **2026-07-30** MEFORORG had 19 workflows, +all ``active``, and all 13 required contexts resolved to active workflows. The hazard is real but was +not live here on that date. ``test_the_live_repo_is_currently_clean`` is deliberately NOT part of this +module — that would make the suite depend on the network and on a mutable server state. +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(_ROOT)) + +from scripts.ci.check_required_workflow_state import main # noqa: E402 +from tests._workflow_contexts import required_contexts, resolve # noqa: E402 + + +def _required_workflow_files() -> set[str]: + """The workflow FILES that own a required context — resolved by the shipped resolver.""" + out = set() + for ctx in required_contexts(): + where = resolve(ctx) + assert where is not None, ( + f"required context {ctx!r} resolves to no job. Fix that first — " + "tests/test_required_contexts.py owns that assertion." + ) + out.add(where[0]) + return out + + +def _payload(states: dict[str, str]) -> dict: + """An /actions/workflows payload with the given ``{filename: state}``.""" + return { + "total_count": len(states), + "workflows": [ + {"id": i, "name": f, "path": f".github/workflows/{f}", "state": s} + for i, (f, s) in enumerate(sorted(states.items()), start=1) + ], + } + + +def _write(tmp_path: Path, states: dict[str, str]) -> Path: + p = tmp_path / "states.json" + p.write_text(json.dumps(_payload(states)), encoding="utf-8") + return p + + +def test_all_active_passes(tmp_path: Path) -> None: + """The healthy case — and the baseline the failure cases are measured against.""" + states = dict.fromkeys(_required_workflow_files(), "active") + assert main(["--states-json", str(_write(tmp_path, states))]) == 0 + + +def test_one_disabled_workflow_is_caught( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A single disabled workflow makes its context permanently unreportable.""" + files = sorted(_required_workflow_files()) + assert len(files) >= 2, "expected the required set to span at least two workflows" + states = dict.fromkeys(files, "active") + states[files[0]] = "disabled_manually" + + assert main(["--states-json", str(_write(tmp_path, states))]) == 1 + err = capsys.readouterr().err + assert files[0] in err and "disabled_manually" in err, err + # A single disabled workflow is NOT the vault shape; the catastrophic message must not fire. + assert "EVERY required context" not in err, ( + "the all-unreachable message fired for a single disabled workflow — it would cry wolf and " + "train the reader to ignore the one case that means nothing can merge" + ) + + +def test_the_vault_shape_is_called_out_by_name( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """EVERY required context unreachable means NO PR can merge — say so, loudly. + + This is the case that cost the vault repo real time: it presents as PRs hanging, which reads as + flakiness. The diagnosis has to be in the output or someone spends a day on it. + """ + states = dict.fromkeys(_required_workflow_files(), "disabled_manually") + assert main(["--states-json", str(_write(tmp_path, states))]) == 1 + err = capsys.readouterr().err + assert "EVERY required context" in err, err + assert "NO pull request can merge" in err, err + + +def test_disabled_inactivity_counts_too(tmp_path: Path) -> None: + """``active`` is the only runnable state — not "anything that isn't disabled_manually". + + ``disabled_inactivity`` arrives on its own after 60 idle days, which is precisely the quiet period + this check exists to cover. Matching only the one state name seen on the vault would miss it. + """ + files = sorted(_required_workflow_files()) + states = dict.fromkeys(files, "active") + states[files[0]] = "disabled_inactivity" + assert main(["--states-json", str(_write(tmp_path, states))]) == 1 + + +def test_an_empty_workflow_list_fails_closed(tmp_path: Path) -> None: + """ "The API returned nothing" must never read as "everything is fine".""" + p = tmp_path / "empty.json" + p.write_text(json.dumps({"total_count": 0, "workflows": []}), encoding="utf-8") + assert main(["--states-json", str(p)]) == 2 + + +def test_an_unreadable_payload_fails_closed(tmp_path: Path) -> None: + """Same blindness, one level up: unable to measure is a FAILURE, not a pass.""" + p = tmp_path / "missing.json" + assert main(["--states-json", str(p)]) == 2 + + +def test_a_context_whose_workflow_is_absent_server_side_is_reported(tmp_path: Path) -> None: + """A required context pointing at a workflow the server does not have is equally unreportable. + + Distinct from the disabled case — the file may exist on a branch but never have landed on the + default branch, so GitHub has no workflow to run. + """ + files = sorted(_required_workflow_files()) + states = dict.fromkeys(files[1:], "active") # first workflow simply absent + assert main(["--states-json", str(_write(tmp_path, states))]) == 1 + + +def test_it_reuses_the_shared_resolver_rather_than_a_second_copy() -> None: + """One context->workflow mapping, two callers. + + A second implementation is the drift this repo keeps writing parity tests to catch (ruff/bandit + scan scope; the required-set prose). If the script ever grows its own parser, this fails and forces + that to be a deliberate decision. + """ + src = (_ROOT / "scripts" / "ci" / "check_required_workflow_state.py").read_text( + encoding="utf-8" + ) + assert "from tests._workflow_contexts import" in src, ( + "the reachability check no longer imports the shared resolver — it has grown a second " + "context->workflow mapping, which will drift from tests/_workflow_contexts.py" + ) + + +def test_the_workflow_is_scheduled_and_least_privilege() -> None: + """Per-PR alone cannot catch it: a workflow disabled AFTER the last PR has no PR to be caught on.""" + yaml = pytest.importorskip("yaml") + doc = yaml.safe_load( + (_ROOT / ".github" / "workflows" / "required-workflow-state.yml").read_text( + encoding="utf-8" + ) + ) + on = doc.get(True, doc.get("on")) + assert isinstance(on, dict) and "schedule" in on, ( + f"required-workflow-state.yml must run on a schedule; `on:` is {on!r}. A workflow disabled " + "after the last PR is invisible to any per-PR check." + ) + job = doc["jobs"]["reachable"] + assert job.get("permissions") == {"contents": "read", "actions": "read"}, ( + f"job permissions are {job.get('permissions')!r} — it needs `actions: read` to read workflow " + "state and nothing more. Notably NOT the admin scope that reading branch protection requires." + ) From e648ae75559e689b568e92e42d5bcd47e92e65aa Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 30 Jul 2026 12:41:01 -0500 Subject: [PATCH 2/2] fix(ci): the shared resolver imported pytest, which the scanner-lock job does not have My own check failed on its own PR, which is the check doing its job on the first thing it was ever asked to run: File "scripts/ci/check_required_workflow_state.py", line 48, in from tests._workflow_contexts import required_contexts, resolve File "tests/_workflow_contexts.py", line 25, in import pytest ModuleNotFoundError: No module named 'pytest' `tests/_workflow_contexts.py` did `import pytest` at module scope for one reason: `yaml = pytest.importorskip("yaml")`, keeping PyYAML optional for the test suite. That was harmless while the module was test-only. It stopped being test-only the moment the reachability script imported the same resolver -- deliberately, so there is ONE context->workflow mapping rather than two that drift -- and that job installs ci/locks/ci-scanners.lock, not the test toolchain. A pure environment difference: it passed locally, where a dev venv has pytest, and died in CI. Same class as the ruff-0.16-vs-0.15.22 drift and the actionlint-with-shellcheck asymmetry earlier today -- a thing that behaves differently depending on what happens to be installed where it runs. FIX: import yaml directly and fall back to `pytest.importorskip` only when it is genuinely absent. Behaviour under pytest is unchanged -- with PyYAML present the try succeeds and pytest is never imported here; with it absent the importorskip still turns the importing module into a SKIP, exactly as before. The alternative was to give the script its own YAML parsing and drop the shared import. Rejected: a second context->workflow mapping is precisely the drift this repo keeps writing parity tests to catch, and `test_it_reuses_the_shared_resolver_rather_than_a_ second_copy` exists to prevent it. GUARDED so it cannot regress: `test_the_script_imports_without_pytest_installed` imports the resolver in a SUBPROCESS and asserts `pytest` never entered `sys.modules`. In-process would be meaningless -- pytest is by definition already imported when a test runs, so the assertion could never fail. Proven by reverting the module-scope import: caught, with the right message. Verified after: 33 passed across every suite that touches the shared resolver (required-workflow-state, required-contexts, security-posture, ci-leg-data-class, nightly-notice). --- tests/_workflow_contexts.py | 26 ++++++++++++++++------ tests/test_required_workflow_state.py | 31 +++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/tests/_workflow_contexts.py b/tests/_workflow_contexts.py index 51d41895..3772090b 100644 --- a/tests/_workflow_contexts.py +++ b/tests/_workflow_contexts.py @@ -22,13 +22,25 @@ class of bug both suites exist to catch. from pathlib import Path from typing import Any -import pytest - -# The importorskip lives HERE rather than in each importing test module. Done there, it would be a -# statement before the `from tests._workflow_contexts import ...` line and every caller would need an -# E402 dance; done here, both suites keep ordinary top-of-file imports and PyYAML stays optional -# exactly as tests/test_lint_scope_parity.py treats it. -yaml = pytest.importorskip("yaml") +# PyYAML stays OPTIONAL (as tests/test_lint_scope_parity.py treats it), and the skip lives HERE rather +# than in each importing test module: done there it would sit before the +# `from tests._workflow_contexts import ...` line and every caller would need an E402 dance. +# +# Import yaml DIRECTLY first, and only fall back to pytest's importorskip when it is genuinely absent. +# This module is no longer test-only — scripts/ci/check_required_workflow_state.py imports the same +# resolver so there is ONE context->workflow mapping rather than two that drift. An unconditional +# `import pytest` at module scope made that impossible: the CI job installs the scanner lock, not the +# test toolchain, so the script died on `ModuleNotFoundError: No module named 'pytest'` while passing +# locally, where a dev venv has pytest. Measured on PR #76. +# +# Behaviour under pytest is unchanged: with PyYAML present the try succeeds (pytest is never imported +# here); with it absent the importorskip still turns the whole importing module into a SKIP. +try: + import yaml +except ModuleNotFoundError: # pragma: no cover - exercised only on a venv without PyYAML + import pytest + + yaml = pytest.importorskip("yaml") ROOT = Path(__file__).resolve().parents[1] WORKFLOWS = ROOT / ".github" / "workflows" diff --git a/tests/test_required_workflow_state.py b/tests/test_required_workflow_state.py index 3cbbad07..ad2ca373 100644 --- a/tests/test_required_workflow_state.py +++ b/tests/test_required_workflow_state.py @@ -23,6 +23,7 @@ from __future__ import annotations import json +import subprocess import sys from pathlib import Path @@ -157,6 +158,36 @@ def test_it_reuses_the_shared_resolver_rather_than_a_second_copy() -> None: ) +def test_the_script_imports_without_pytest_installed() -> None: + """The CI job installs the SCANNER lock, not the test toolchain — so pytest is absent there. + + This is the regression that broke PR #76's first run. ``tests/_workflow_contexts`` did + ``import pytest`` at module scope (only to reach ``importorskip("yaml")``), so the script died on + ``ModuleNotFoundError: No module named 'pytest'`` in CI while passing locally, where a dev venv has + pytest. A pure environment difference — exactly the class this repo keeps getting bitten by. + + Asserted by importing in a SUBPROCESS and checking ``pytest`` never entered ``sys.modules``. Testing + it in-process would be meaningless: pytest is, by definition, already imported when this runs. + """ + probe = ( + "import sys; " + "sys.path.insert(0, r'" + str(_ROOT) + "'); " + "import tests._workflow_contexts as m; " + "assert m.required_contexts(), 'resolver returned no contexts'; " + "print('PYTEST_IMPORTED' if 'pytest' in sys.modules else 'CLEAN')" + ) + out = subprocess.run( # noqa: S603 - fixed argv, no shell + [sys.executable, "-c", probe], capture_output=True, text=True, timeout=120 + ) + assert out.returncode == 0, f"importing the resolver failed:\n{out.stderr}" + assert "CLEAN" in out.stdout, ( + "importing tests/_workflow_contexts pulled in pytest. The CI job for " + "scripts/ci/check_required_workflow_state.py installs only ci/locks/ci-scanners.lock, so a " + "pytest import there is a hard failure. Import yaml directly and fall back to importorskip " + f"only when it is absent.\nstdout: {out.stdout}\nstderr: {out.stderr}" + ) + + def test_the_workflow_is_scheduled_and_least_privilege() -> None: """Per-PR alone cannot catch it: a workflow disabled AFTER the last PR has no PR to be caught on.""" yaml = pytest.importorskip("yaml")