Skip to content

ci(release): gate tag/pypi/conda on their own evidence, not the v* tag - #905

Merged
JarryShaw merged 1 commit into
mainfrom
ci/888-release-gate-own-evidence
Sep 29, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
ci/888-release-gate-own-evidence

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • chore — anything else

Description of your pull request and other information

Fixes #888. tag, pypi and conda now gate on their own publish evidence
instead of the shared v*-tag proxy that made every job skip silently on a
half-finished release. conda also checks per matrix leg before uploading,
since Anaconda has no skip-existing. A new release_status job always
reports why a run released nothing, or that it did, and fails loudly on the
one shape that should now be unreachable. docs/source/releasing.rst and
tests/project/test_release_gates.py are updated accordingly.

@JarryShaw JarryShaw added ci Pull requests that change CI or workflow configuration (ci: subject prefix) fix Pull requests that fix a defect (fix: subject prefix) docs Pull requests that change documentation only (docs: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 29, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES on 50da63f0c — cross-review (opus; author sonnet). Two blocking defects, both in the part that is the actual fix, and both verified by me independently.

1. The conda per-leg check crashes the leg on exactly the retry it exists for. create-release.yml:649 runs python3 -c " with its source indented 12 spaces. Dedenting indented -c source is Python 3.14-only. Measured on this machine against real interpreters:

3.9  IndentationError: unexpected indent      3.12  IndentationError: unexpected indent
3.10 IndentationError: unexpected indent      3.13  IndentationError: unexpected indent
3.11 IndentationError: unexpected indent      3.14  DEDENT-OK 3.14.7

That step sits at :649, before actions/setup-python@v7 at :669, so python3 is the runner-image default — ubuntu-24.04 ships 3.12. Under set -euo pipefail the failed assignment aborts the step. And the branch is entered only when the endpoint returns 200, i.e. only when the version is already partly on Anaconda — precisely the retry-of-a-partial-release case. So this passes a fresh release and fails only when it matters. Fix: dedent to column 0 or use python3 - <<'PY'. The same pattern at :231 and :268 survives only because setup-python at :159 precedes them; one pin change from the same break, so dedent those too.

2. release_status errors and exits 1 on the successful self-heal. Read from the script itself: all_skipped requires all four of github/tag/pypi/conda to be skipped, all_done requires all four success, any_broken catches failure/cancelled, and anything else falls through to ::error + exit 1. A self-heal is mixed — github/tag skip because their artefacts exist, pypi/conda run and succeed — so it matches none of the three and hits the stranded-partial branch, whose message asserts "something skipped that should have run to finish it" when nothing did.

That is the canonical #888 scenario. The PR replaces a silent green no-op with a red false alarm on the run that actually completed the release. releasing.rst:311-313 repeats the claim that this branch "should be unreachable", and :300-302 says a re-run "no longer needs to be verified by hand" — both now false. Fix: reconcile per target — accept success, or skipped when that target's own evidence output was already true; fire Case 3 only when a target's evidence says incomplete and its job skipped.

Why both shipped: every release_status test is a string assertion. Nothing executes the shell script, and nothing executes or even compile()s the embedded python3 -c snippets. One test running the body over a table of result vectors, and one running the snippets under a non-3.14 interpreter, catches both.

Two more, non-blocking but worth fixing while in there. != 'failure' admits cancelled, so conda can enter actions/checkout with ref: conda-<version>+0 for a tag tag never pushed — an explicit result == 'success' || result == 'skipped' says the intent. And status="$(curl -s …)" has no -f, no --retry, no || status=000; under set -euo pipefail a transient pypi.org or api.anaconda.org outage now fails version_check, which all three publish jobs require — this PR newly couples the whole release to index availability.

Confirmed good: conda really is per-leg (filter is subdir == target_platform AND build.startswith(py_tag + '_'), gating only the upload step at :778, force: not set); pypi correctly uses urls on the per-version endpoint — my brief was wrong to say releases[<version>], there is no such key there; skip-existing: true intact at :547/:560; release_status genuinely always runs; the cascade-skip idiom does pre-exist at cron-vendor.yml:68, deploy-pages.yml:66, cron-conda.yml:64; all 15 pre-existing tests pass and the extended folded-if: scanner still agrees with a real yaml.safe_load; docs build clean with delta 0.

One thing nobody can verify without a real deployment, and it is the severe branch: what needs.github.result reads when a reviewer rejects the github-release approval. If rejection yields skipped, the new if: lets tag, pypi and conda run — pushing to main and publishing to PyPI after a rejected approval — where the old cascade-skip blocked them. Since #887 left github-release as the only gate, this needs settling before merge.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 29, 2026
#888)

- Every release job shared one guard keyed on "does the v* tag exist",
  using it as a proxy for "was this version published". Those diverge
  when a release half-completes: github tags, a later job fails, and
  every job on the next workflow_run-triggered attempt reads the tag
  and skips -- the run reports green having published nothing.
- version_check now also checks the conda-<version>+0 tag, PyPI's file
  count for the version (vs the 8 expected), and Anaconda's aggregate
  count (vs the 10 expected); tag/pypi/conda gate on that instead. The
  curl calls tolerate a transient index outage (--retry-connrefused,
  || status=000) rather than failing version_check outright, since
  "incomplete" is already the safe default for that failure mode.
- Those three jobs' if: open with !cancelled() (the idiom
  cron-vendor.yml/deploy-pages.yml/cron-conda.yml already use) plus an
  explicit needs.<dep>.result == 'success' || == 'skipped' per direct
  dependency, so a legitimately-skipped github does not cascade-skip
  them before their own evidence is even evaluated, while an outright
  failure or cancellation still blocks them.
- conda: anaconda/actions/upload-package has no skip-existing, so each
  matrix leg also checks Anaconda for its own platform/Python file and
  skips only its own upload step, with a ::notice, if already there.
  Every PyPI/Anaconda evidence check feeds Python source to `python3 -`
  one line per printf argument rather than an indented `-c` argument --
  indented -c source only compiles on Python 3.14, and the runner
  image's default python3 (3.12/3.13) raises IndentationError on it.
- Added release_status: runs unconditionally, reconciles each of
  github/tag/pypi/conda against its own evidence (success, or skipped
  because that evidence already said done) rather than demanding the
  same outcome from all four, and reports why a run released nothing
  or that it did; fails loudly if a target ever skips despite its own
  evidence saying it is not done.
- Updated docs/source/releasing.rst and extended
  tests/project/test_release_gates.py, including tests that execute
  release_status's script over a table of result vectors and that run
  the extracted Python snippets under a pre-3.14 interpreter; passes
  (34 tests, 23 subtests).
@JarryShaw
JarryShaw force-pushed the ci/888-release-gate-own-evidence branch from 50da63f to 80e1bcd Compare September 29, 2026 03:49
@JarryShaw

Copy link
Copy Markdown
Owner Author

Both blocking defects fixed on 80e1bcdff, and the rejected-approval question is answered. Not granting a verdict yet — the delta is large enough to re-review rather than accept on my spot-check.

The severe branch is closed, and the design is safe. I raised the question of what needs.github.result reads when a reviewer rejects the github-release deployment, because if rejection yielded skipped the new if: would have let tag, pypi and conda run — pushing to main and publishing to PyPI after a rejected approval. Verified against GitHub's own documentation:

If a job is rejected, the workflow will fail.

So rejection yields failure, not skipped. Every successor check here now explicitly excludes failure via (needs.X.result == 'success' || needs.X.result == 'skipped'), so a rejected approval still blocks all three publish jobs — the property the old cascade-skip gave us is preserved by construction rather than by accident. This is documentary inference, not a measurement; nobody triggered a deployment to confirm it, and I am not asking anyone to.

What I verified myself on this head:

python3 -c "<indented>" forms remaining ...... 0
printf '%s\n' … | python3 -  at ............. :254, :296, :694  (all three snippets)
per-target reconciliation at ................ :925-934  (reconciled_github/tag/pypi/conda)
explicit success||skipped allowlists ........ 5
remaining != 'failure' ...................... 2, both in COMMENTS (:119, :124) explaining
                                              why this file spells it differently from
                                              cron-vendor.yml / deploy-pages.yml / cron-conda.yml
--retry-connrefused sites ................... 4

The two surviving != 'failure' are prose, not logic — I checked, because "replaced them all" plus a non-zero grep is exactly where a partial fix hides.

Why a re-review rather than my sign-off. The delta since the reviewed head is 512 insertions across three files — create-release.yml +207, releasing.rst +57, test_release_gates.py +338 — which is not a narrow amend. And the previous round found two defects that were green on CI and would have fired only in production: a snippet that crashes solely on a retry, and a status job that errors solely on success. A structural spot-check like the one above cannot tell me the reconciliation is correct across all eight vectors, only that it is present. For release automation that publishes to PyPI and Anaconda, present is not enough.

So: re-review dispatched on the two things my check cannot reach — whether the reconciliation branches correctly for every result vector rather than just the self-heal one, and whether the new tests genuinely execute the script and the snippets rather than string-matching them. CI is fully green on this head (ok=62 fail=0 inc=0), which after last round proves nothing on its own.

@JarryShaw JarryShaw added review: pending No verdict for the current head - never reviewed, or the head moved since the last one and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Sep 29, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO on 80e1bcdff — re-review (opus; author sonnet). Both blocking defects genuinely fixed, and the reviewer swept the full 4096-vector cross-product rather than the handful I asked for.

4 job results × 4 jobs × 2 evidence values × 4 targets = 4096 vectors, driven under real
bash 5.3.20 with GitHub's own flags (--noprofile --norc -e -o pipefail):

  ERROR/stranded          rc=1    175
  NOTICE/already-released rc=0      1
  NOTICE/complete         rc=0     80
  WARNING/did-not-complete rc=0  3840
  mismatches vs an INDEPENDENT derivation of the contract:  0
  DANGEROUS (unreconciled but quiet + green):               0

That last line is the one that mattered. The previous round's defect was a false alarm on success; the mirror-image defect — silence on a real problem — is worse, and a reconciliation rewrite is exactly where it could appear. Zero such vectors. The named cases all branch correctly, including the two that decide it: the self-heal (github=skipped tag=skipped pypi=success conda=success) now gives ::notice Release complete rc=0, and the partial self-heal (conda=skipped with conda evidence false) still gives ::error Stranded partial (#888) rc=1. Adversarial inputs — empty results, empty evidence, an unrecognised neutral — all fail safe and loud.

The new tests execute; they do not string-match, and they are not vacuous. TestReleaseStatusScriptExecutesCorrectly really runs bash -c over 8 vectors. And the interpreter test is better than the author claimed: pre_314_interpreter() returns sys.executable outright whenever the suite is on < 3.14, and unit-tests.yml runs the whole suite on 3.10 through 3.14, so four of the five legs exercise it for real with no dependence on shutil.which. On the 3.14 leg it resolved /usr/bin/python3.12-class interpreters locally. And the vacuity hole is closed by a different test: test_the_scan_found_the_snippets asserts len(...) == 3 unconditionally, so a revert to python3 -c fails even where the runner has no old interpreter. Both classes fail against pre-fix 50da63f0c with exactly the two original defects.

releasing.rst citations: 14 of 15 correct. One is off, and I verified it myself — :67 cites create-release.yml:395-396, but 395 is blank, the ::warning echo is at 397 and fi at 398. Should be 396-398. Cosmetic, one character.

Verified good: skip-existing: true on both upload steps (:575, :588); YAML parses to 7 jobs with the textual scanner agreeing with a real yaml.safe_load; expected=8 matches the 7-leg matrix plus the 3.14-only sdist and expected=10 matches 2 OS × 5 Python; reconciled_tag reads the tag the tag job actually creates.

Four non-blocking items, worth a follow-up rather than another round:

  1. releasing.rst:67 → 396-398.
  2. The doc says release_status "reports exactly one of" and lists five outcomes; the script emits eight. ::warning Release blocked before it could start is one a reader would want named.
  3. any_broken fires before Case 3, so when a leg fails and another target is stranded, its message reports the four job results but not the four evidence values — sending the operator back to checking PyPI and Anaconda by hand, which is what :300's bold lede says is no longer necessary. Echoing the evidence quartet there closes it. Never green, though: a failure makes the run red.
  4. test_each_snippet_runs_without_an_indentation_error asserts only assertNotIn('IndentationError', ...); a plain SyntaxError would pass. Widening costs nothing.

And three corrections to my own brief, all confirmed: there are 3 curl sites (:237, :287, :674) — my 4th was a comment; reconciliation spans :925-939 not :925-934; and my "15 pre-existing tests" was wrong, it was 31 (now 34, nothing removed, and the one edited assertion strengthens by requiring the explicit success/skipped pair and assertNotIn-ing the weaker != 'failure').

One thing nobody built: Sphinx was not run on this head, and Docs test gate is SKIPPED on this PR, so nothing verified the docs render. Risk is low — the diff adds prose and one link of a form already in the file, introduces no new roles, and every internal reference resolves — but it is unmeasured and I am saying so rather than implying otherwise.

Ready for you to merge. I am not merging.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Sep 29, 2026
@JarryShaw
JarryShaw merged commit 539a3c5 into main Sep 29, 2026
63 checks passed
@JarryShaw
JarryShaw deleted the ci/888-release-gate-own-evidence branch September 29, 2026 04:53
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Sep 29, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix) docs Pull requests that change documentation only (docs: subject prefix) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(ci): a half-finished release leaves a v* tag that makes every retry skip silently

1 participant