From a93e726a899237a59b72710832c5afa249ccc966 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 1 Oct 2026 01:01:11 -0400 Subject: [PATCH] ci(create-release): scope the cascade-guard comment to the publishing predecessors (#960) The comment above `jobs:` said `tag`, `pypi` and `conda` spell the guard as the success-or-skipped equality pair "for each of their own direct dependencies". That is false for `version_check`, a direct dependency of all three (:443, :521, :620) which each require to have strictly succeeded, with no `|| 'skipped'` (:445, :523, :622). The equality pair applies only to `github`, and for `conda` also `tag`. The comment is where the documentation got the claim from -- #958 was the same error in releasing.rst, corrected in #959 -- so leaving it would have had the docs describe this file more accurately than the file describes itself. Comment text only; no executable YAML changed. Verified by parsing both revisions with `yaml.safe_load` and comparing the loaded structures, which is stronger than a `grep -vE '^[+-]#'` -- that pattern only matches `#` at column zero and says nothing about indented YAML. Also adds test_version_check_must_strictly_succeed, which nothing in the module covered. An `assertNotIn` over known-bad spellings cannot pin this clause, since `(!= 'failure' || == 'success')` keeps the literal while meaning the opposite. Two assertions over a whitespace-stripped condition instead: - the clause appears as a top-level `&&` conjunct, because vocabulary is not structure -- a one-character `&&`-to-`||` slip keeps every token while nullifying the gate; - `version_check.result` is referenced exactly once, which closes the rest: anything added alongside the real clause raises the count however spelled. A comparison-set assertion was carried for two rounds and is dropped as provably redundant: the conjunct substring guarantees one occurrence followed by `== 'success'`, the count guarantees it is the only one, so the set is necessarily `{('==', 'success')}`. Brute-forced over 173,088 synthetic conditions -- zero cases where it fires alone. Knowingly not closed, and now recorded in the docstring: a top-level `||` outside the conjunction chain. Conjunction is monotone, so adding `&&` conjuncts can only strengthen the gate; disjunction is the only remaining weakening and catching it needs the expression evaluated rather than matched. Tracked as #962. Falsifiability confirmed against mutated copies, each valid YAML first. Head passes, a legitimate extra `&&` conjunct still passes, and `|| == 'skipped'`, `(!= 'failure' || == 'success')`, reversed operands, `contains(fromJSON(...))`, `|| event_name`, a negation, `&&`-to-`||` and the clause deleted all fail. tests/project/test_release_gates.py: 35 passed, 26 subtests, re-derived as 35 under plain unittest. --- .github/workflows/create-release.yml | 22 +++++++---- tests/project/test_release_gates.py | 58 ++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index bed307727d..368d7a36e0 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -118,15 +118,21 @@ concurrency: # ``cron-vendor.yml``, ``deploy-pages.yml`` and ``cron-conda.yml`` all open with # ``!cancelled() && needs.unit-tests.result != 'failure'`` so that a skipped # gate does not cascade-skip them, while an outright failure or a genuine run -# cancellation still does. ``tag``, ``pypi`` and ``conda`` below do the same for -# each of their own direct dependencies, but spelled as ``!cancelled() && -# (needs..result == 'success' || needs..result == 'skipped')`` rather -# than the sibling files' ``!= 'failure'`` -- ``!= 'failure'`` alone still -# admits ``cancelled``, and a cancelled ``tag`` would send ``conda`` into its +# cancellation still does. ``tag``, ``pypi`` and ``conda`` below are stricter +# about their *publishing* predecessors -- ``github``, and for ``conda`` also +# ``tag`` -- spelling the guard as ``!cancelled() && (needs..result == +# 'success' || needs..result == 'skipped')`` rather than the sibling files' +# ``!= 'failure'``: the two spellings differ only when a predecessor's own +# result is ``cancelled``, which ``!= 'failure'`` still admits and the equality +# pair does not. A cancelled ``tag`` would otherwise send ``conda`` into its # ``actions/checkout`` with ``ref: conda-+0`` for a tag ``tag`` never -# pushed, trading a clean skip for a checkout failure. Either spelling bypasses -# the default ``success()`` GitHub Actions would otherwise prepend to an -# ``if:`` that does not itself call a status-check function. +# pushed, trading a clean skip for a checkout failure. ``version_check`` is held +# to a stricter bar still: all three require ``needs.version_check.result == +# 'success'`` with no ``|| 'skipped'``, since it produces the evidence the gates +# below read, and a skipped or cancelled ``version_check`` leaves them nothing +# to decide on. Either spelling bypasses the default ``success()`` GitHub +# Actions would otherwise prepend to an ``if:`` that does not itself call a +# status-check function. # # See ``release_status`` at the bottom of this file for the other half: a job # that runs whenever the run itself was not cancelled (``if: !cancelled()``, diff --git a/tests/project/test_release_gates.py b/tests/project/test_release_gates.py index d53b6529ac..d056ebeafd 100644 --- a/tests/project/test_release_gates.py +++ b/tests/project/test_release_gates.py @@ -627,6 +627,64 @@ def test_evidence_gated_jobs_survive_a_legitimately_skipped_predecessor(self) -> f'admits `cancelled` -- see this test\'s own docstring', ) + def test_version_check_must_strictly_succeed(self) -> None: + """``version_check`` is not one of the *publishing* predecessors the + equality-pair escape hatch above exists for -- it produces the + evidence ``tag``, ``pypi`` and ``conda`` read, so a skipped or + cancelled ``version_check`` must stop them rather than being waved + through the way a legitimately-skipped ``github`` or ``tag`` is. + + #960: the comment above ``jobs:`` once read as though the equality + pair applied to "each of their own direct dependencies", which is + false for ``version_check`` specifically -- a direct dependency of + all three, required to have *strictly* succeeded, with no + ``|| 'skipped'``. Nothing in this module pinned that before. + + An ``assertNotIn`` over known-bad spellings cannot pin this, because a + weakening can keep the ``== 'success'`` literal while meaning the + opposite -- ``(!= 'failure' || == 'success')`` is just + ``!= 'failure'``, which admits both ``skipped`` and ``cancelled``. + Hence two assertions instead: the clause must appear as a top-level + ``&&`` conjunct, since vocabulary is not structure and a + one-character ``&&``-to-``||`` slip keeps every token while nullifying + the gate; and ``version_check.result`` must be referenced exactly + once, which closes the rest, since anything added alongside the real + clause raises the count however it is spelled. + + Whitespace is stripped first so the pattern cannot be defeated by + padding around the property dots. That trade is deliberate: it closes + a hole that fails *open*, at the cost of reading an identifier or a + literal split across the folded scalar's line break as though it were + whole -- which fails *closed*, since Actions rejects the real + expression and nothing publishes. + + **Knowingly not closed:** a top-level ``||`` outside the conjunction + chain. Appending ``|| github.event_name == 'workflow_dispatch'`` to the + end of the condition, or wrapping it in ``true || ...``, bypasses the + gate while leaving the compacted clause byte-identical. Conjunction is + monotone, so *adding* ``&&`` conjuncts can only strengthen the gate and + needs no guard; disjunction is the only remaining weakening, and + catching it needs the expression evaluated rather than matched. Tracked + as #962. + + """ + for name in ('tag', 'pypi', 'conda'): + with self.subTest(job=name): + compact = re.sub(r'\s+', '', declared_if(self.jobs[name])) + self.assertIn( + "&&needs.version_check.result=='success'&&", compact, + f'`{name}` does not require `version_check` to have ' + f'succeeded as a top-level `&&` conjunct -- a disjunct or ' + f'a negation keeps the same tokens while nullifying the ' + f'gate', + ) + self.assertEqual( + compact.count('version_check.result'), 1, + f'`{name}` refers to `version_check.result` more than ' + f"once; the gate is one comparison, `== 'success'`, and a " + f'second reference can only weaken it', + ) + def test_conda_checks_its_own_leg_before_uploading(self) -> None: """The correctness half for ``conda``: ``anaconda/actions/upload-package`` has no ``skip-existing``, so the job-level evidence above is only a cost