From d1cea5a7506077c3d31349ac15cd75ef68979547 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 28 Jul 2026 15:42:08 -0500 Subject: [PATCH] =?UTF-8?q?fix(ci):=20let=20the=20free-threaded=20canary?= =?UTF-8?q?=20report=20a=20problem=20=E2=80=94=20it=20was=20structurally?= =?UTF-8?q?=20unable=20to?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `freethread-smoke.yml` could not fail in any path. Every step was `continue-on-error`, every step after setup was gated on `steps.setup.outcome == 'success'`, AND the job itself was `continue-on-error`. So a runner that could not provision 3.14t skipped everything and reported success; a failing GIL assertion or smoke test was swallowed and reported success. Five runs, five successes, and nothing in that record could distinguish a canary that flew from one that never left the ground. It IS currently flying — run 30256316219 shows `sys._is_gil_enabled() = False` on a free-threading build with 73 tests passing. The defect was latent, not active, which is exactly why it needed fixing: nothing would have told us when it stopped. An early-warning tripwire that cannot warn is worse than no tripwire, because its silence is mistaken for good news. The header also claimed "a red canary is an informational red check". No red canary was reachable. WHAT CHANGED. The job-level `continue-on-error` is gone. It bought nothing — this workflow runs only on a weekly cron and manual dispatch, never on `pull_request`, so it produces no PR check and cannot gate a merge regardless — and it cost the entire signal. The steps stay `continue-on-error` so they all run and every outcome stays collectable; a terminal verdict step then rules on those outcomes and IS allowed to fail. Green now means the canary flew. Three verdicts, each executed locally against the extracted shell rather than reasoned about: * did-not-fly — 3.14t could not be provisioned. A dead tripwire, not a clean run. * regressed — install, the GIL assertion, or the smoke tests failed under 3.14t. * vacuous — pytest exited 0 having collected nothing. Same shape as the gates fixed in #25: success that measured nothing. All six scenarios verified: healthy exits 0; setup-dead, GIL-re-enabled, smoke-failed, install-failed and vacuous-pass each exit 1 with the right diagnosis and a step summary naming it. The smoke step also stops hiding pytest behind a pipe (`| tee` yields tee's status, not pytest's) and now records how many tests actually ran, so the verdict can say what flew rather than merely that nothing errored. tests/test_freethread_smoke_liveness.py pins the two properties that make the signal real — the verdict step exists and can fail — and the one that makes failing safe: the workflow never runs on a pull request. --- .github/workflows/freethread-smoke.yml | 96 ++++++++++++++++++-- tests/test_freethread_smoke_liveness.py | 115 ++++++++++++++++++++++++ 2 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 tests/test_freethread_smoke_liveness.py diff --git a/.github/workflows/freethread-smoke.yml b/.github/workflows/freethread-smoke.yml index 2c90f9bc..fcd5d535 100644 --- a/.github/workflows/freethread-smoke.yml +++ b/.github/workflows/freethread-smoke.yml @@ -7,15 +7,26 @@ # hence weekly (+ manual dispatch), not on every push/PR. # # IT MUST NEVER BLOCK A MERGE: +# * It runs ONLY on a weekly cron and manual dispatch -- never on `pull_request` or `push` -- so it +# produces no PR check at all. That is what actually makes it non-blocking, and it is structural. # * It is a SEPARATE workflow, deliberately not part of ci.yml's `ci-gate` (the required "CI gate" # context). It is not in any `needs:` list a PR waits on. -# * The job AND its fragile steps are `continue-on-error: true`, so a red canary is an informational -# red check, never a failed required one. # * DO NOT add the "freethread smoke (3.14t)" context to branch-protection required checks. The # required set is the `test` matrix + bandit + pip-audit + cla — keep it that way. # -# If actions/setup-python ever cannot provision 3.14t on the runner, the continue-on-error job simply -# goes red as information; nothing downstream is gated on it. +# WHY THIS JOB IS NO LONGER `continue-on-error` (2026-07-28). It used to be, on top of every step +# already being continue-on-error AND every step after setup being gated on +# `if: steps.setup.outcome == 'success'`. The combination made the canary INCAPABLE OF REPORTING A +# PROBLEM: if 3.14t could not be provisioned every later step skipped and the job went green; if the +# smoke tests failed, continue-on-error swallowed it and the job went green. A canary that never flew +# was indistinguishable from one that flew clean -- and the header used to claim "a red canary is an +# informational red check" when no red canary was reachable. Five runs, five successes, and nothing in +# that record could have told you which. +# +# The belt-and-braces bought nothing (the triggers already guarantee no PR is gated) and cost the +# entire signal. The steps stay continue-on-error so they all run and each outcome is collectable; a +# terminal verdict step then rules on those outcomes and IS allowed to fail. Green now means the +# canary flew; red means it either did not fly or found something. name: freethread smoke @@ -37,9 +48,9 @@ permissions: jobs: freethread: name: freethread smoke (3.14t) - # The whole job is non-blocking: a failure here is reported but never fails the workflow's - # required-context resolution, so it cannot block a PR even if it is ever (mis)added to protection. - continue-on-error: true + # NO job-level continue-on-error -- see the header. This job is allowed to go red, because that is + # the only way a dead tripwire is distinguishable from a healthy one, and the triggers already + # guarantee it cannot block anything. runs-on: ubuntu-latest timeout-minutes: 15 steps: @@ -64,6 +75,7 @@ jobs: # chain. --prerelease handling: pip resolves the cp314t wheels; the cffi 2.0 chain may need a # prerelease allowance depending on the index state at run time. - name: Install the core engine (dev extra; no compiled-heavy extras) + id: install if: steps.setup.outcome == 'success' continue-on-error: true run: | @@ -74,6 +86,7 @@ jobs: # CPython silently RE-ENABLE the GIL, which would make a "green" canary meaningless. Fail (softly) # if the GIL is back on. - name: Assert the GIL is actually disabled + id: gil if: steps.setup.outcome == 'success' continue-on-error: true run: | @@ -90,6 +103,73 @@ jobs: # cheap install-and-import + smoke signal, NOT the perf experiment (that lives in the load harness, # run manually — see docs/LOAD-TESTING.md / docs/design/freethread.md §4). - name: Smoke a pure test subset under 3.14t + id: smoke if: steps.setup.outcome == 'success' continue-on-error: true - run: pytest -q tests/test_parsing.py tests/test_wiring.py + run: | + # `| tee` would mask pytest's exit code behind tee's, so redirect and re-read instead -- + # the same pipe-hides-the-status trap this repo has been bitten by more than once. + pytest -q tests/test_parsing.py tests/test_wiring.py > smoke.txt 2>&1 + STATUS=$? + cat smoke.txt + # Record how many tests actually ran, so the verdict can say what flew rather than just + # that nothing errored. A green pytest that collected nothing is the vacuous-pass shape. + PASSED="$(grep -oE '[0-9]+ passed' smoke.txt | tail -1 | grep -oE '[0-9]+' || true)" + echo "passed=${PASSED:-0}" >> "$GITHUB_OUTPUT" + exit $STATUS + + # THE VERDICT. Every step above is continue-on-error so they all run and every outcome is + # collectable; this one is NOT, and it is what turns those outcomes into a signal. Without it the + # job is structurally incapable of reporting a problem -- see the header. + - name: Verdict — did the canary actually fly? + if: always() + # Outcomes routed through env rather than interpolated into the shell body (zizmor: template + # injection). `skipped` is a real value here: every step after setup is gated on it. + env: + SETUP: ${{ steps.setup.outcome }} + INSTALL: ${{ steps.install.outcome }} + GIL: ${{ steps.gil.outcome }} + SMOKE: ${{ steps.smoke.outcome }} + PASSED: ${{ steps.smoke.outputs.passed }} + run: | + echo "setup=$SETUP install=$INSTALL gil=$GIL smoke=$SMOKE passed=${PASSED:-0}" + VERDICT="" + if [ "$SETUP" != "success" ]; then + # The tripwire is DOWN, which is not the same as clean. A canary that cannot be + # provisioned for months is the exact thing this workflow exists to notice early. + echo "::error title=Free-threaded canary did not fly::could not provision Python 3.14t (setup outcome=$SETUP). This is a dead tripwire, not a clean run -- fix the provisioning or retire the workflow." + VERDICT="did-not-fly" + else + for pair in "install:$INSTALL" "GIL assertion:$GIL" "smoke tests:$SMOKE"; do + NAME="${pair%%:*}" + OUT="${pair##*:}" + if [ "$OUT" != "success" ]; then + echo "::error title=Free-threaded canary regression::$NAME outcome=$OUT under the 3.14t interpreter" + VERDICT="regressed" + fi + done + if [ -z "$VERDICT" ] && [ "${PASSED:-0}" -eq 0 ] 2>/dev/null; then + # pytest exited 0 having collected nothing -- a vacuous pass. + echo "::error title=Free-threaded canary measured nothing::the smoke step succeeded but reported 0 passing tests" + VERDICT="vacuous" + fi + fi + { + echo "## Free-threaded (3.14t) canary" + echo "" + echo "| Step | Outcome |" + echo "| --- | --- |" + echo "| provision 3.14t | $SETUP |" + echo "| install core engine | $INSTALL |" + echo "| GIL actually disabled | $GIL |" + echo "| smoke tests | $SMOKE (${PASSED:-0} passed) |" + echo "" + if [ -z "$VERDICT" ]; then + echo "The canary **flew clean** — ${PASSED:-0} tests passed on the free-threaded build." + else + echo "**Verdict: \`$VERDICT\`.** Green here would have been indistinguishable from a" + echo "healthy run, which is why this job is allowed to fail. It blocks nothing: this" + echo "workflow never runs on a pull request." + fi + } >> "$GITHUB_STEP_SUMMARY" + [ -z "$VERDICT" ] diff --git a/tests/test_freethread_smoke_liveness.py b/tests/test_freethread_smoke_liveness.py new file mode 100644 index 00000000..78e3e76f --- /dev/null +++ b/tests/test_freethread_smoke_liveness.py @@ -0,0 +1,115 @@ +"""The free-threaded canary must be capable of reporting a problem. + +`freethread-smoke.yml` was structurally incapable of it: every step was `continue-on-error`, every +step after setup was gated on `steps.setup.outcome == 'success'`, AND the job itself was +`continue-on-error`. So a runner that could not provision 3.14t skipped everything and went green, +and a failing smoke test was swallowed and went green. Five runs, five successes, and nothing in that +record could distinguish a canary that flew from one that never left the ground. + +(For the record: it WAS genuinely flying — run 30256316219 shows `sys._is_gil_enabled() = False` on a +free-threading build with 73 tests passing. The defect was latent, not active. That is precisely why +it needed a test: nothing would have told us when it stopped.) + +These tests pin the two properties that make the signal real — the verdict step exists and is allowed +to fail — and the property that makes failing safe: this workflow never runs on a pull request. +""" + +import re +from pathlib import Path + +import pytest +import yaml + +_WORKFLOW = Path(__file__).resolve().parents[1] / ".github" / "workflows" / "freethread-smoke.yml" + +# Steps whose outcome the verdict must consider. Each is continue-on-error by design, so each is a +# way for the canary to fail quietly if nothing rules on it afterwards. +_DIAGNOSTIC_STEPS = ("setup", "install", "gil", "smoke") + + +@pytest.fixture(scope="module") +def workflow() -> dict: + assert _WORKFLOW.is_file(), f"workflow not found at {_WORKFLOW}" + return yaml.safe_load(_WORKFLOW.read_text(encoding="utf-8")) + + +@pytest.fixture(scope="module") +def job(workflow: dict) -> dict: + return workflow["jobs"]["freethread"] + + +def _verdict_step(job: dict) -> dict: + steps = [s for s in job["steps"] if "Verdict" in (s.get("name") or "")] + assert len(steps) == 1, "expected exactly one verdict step" + return steps[0] + + +# -------------------------------------------------------------------------------------------- +# The signal must be able to exist. +# -------------------------------------------------------------------------------------------- + + +def test_the_job_is_not_continue_on_error(job: dict) -> None: + """Job-level continue-on-error made every path green. It bought nothing — the triggers already + guarantee nothing is gated — and cost the entire signal.""" + assert job.get("continue-on-error") is not True, ( + "a job that cannot fail cannot report a dead canary" + ) + + +def test_the_verdict_step_can_fail(job: dict) -> None: + step = _verdict_step(job) + assert step.get("continue-on-error") is not True, ( + "the verdict step is the only thing that turns collected outcomes into a signal; " + "continue-on-error here silently restores the original defect" + ) + assert step.get("if") == "always()", "the verdict must run even when an earlier step failed" + + +def test_the_verdict_considers_every_diagnostic_step(job: dict) -> None: + """A step whose outcome nothing reads is a step that can fail in silence.""" + step = _verdict_step(job) + env = step.get("env") or {} + referenced = " ".join(str(v) for v in env.values()) + for name in _DIAGNOSTIC_STEPS: + assert f"steps.{name}.outcome" in referenced, ( + f"the verdict never reads steps.{name}.outcome, so that step can fail quietly" + ) + + +def test_every_diagnostic_step_has_an_id(job: dict) -> None: + """Outcomes are only readable via a step id.""" + ids = {s.get("id") for s in job["steps"] if s.get("id")} + assert set(_DIAGNOSTIC_STEPS) <= ids, f"missing ids: {set(_DIAGNOSTIC_STEPS) - ids}" + + +# -------------------------------------------------------------------------------------------- +# ...and failing must stay safe. +# -------------------------------------------------------------------------------------------- + + +def test_the_workflow_never_runs_on_a_pull_request(workflow: dict) -> None: + """This is what makes a red canary harmless, and it is structural rather than a convention. + `on:` parses to the key True under YAML 1.1, hence the lookup.""" + triggers = workflow.get("on") or workflow.get(True) + assert set(triggers) == {"schedule", "workflow_dispatch"}, ( + f"unexpected triggers {sorted(triggers)} -- a PR trigger would make this job block a merge" + ) + + +def test_the_smoke_step_does_not_hide_pytest_behind_a_pipe(job: dict) -> None: + """`pytest | tee` yields tee's exit status, not pytest's -- the pipe trap this repo keeps hitting.""" + smoke = next(s for s in job["steps"] if s.get("id") == "smoke") + body = smoke["run"] + assert not re.search(r"pytest[^\n|]*\|\s*tee", body), ( + "pytest's exit code would be masked by tee" + ) + assert "STATUS=$?" in body and "exit $STATUS" in body + + +def test_a_vacuous_pass_is_treated_as_a_failure(job: dict) -> None: + """pytest exiting 0 having collected nothing is the same shape as the gates fixed in #25: + success that measured nothing.""" + verdict = _verdict_step(job)["run"] + assert "PASSED" in verdict + assert "measured nothing" in verdict, "a zero-test pass must be called out, not accepted"