release: validate build and upload tooling before pushing the tag - #181
Merged
villelaitila merged 1 commit intoAug 21, 2026
Merged
Conversation
The PyPI step checked `which twine` but ran `sys.executable -m twine`, so a twine belonging to some other Python satisfied the check while the actual call failed. The build step checked nothing at all. Neither mattered until a `uv venv` environment - which ships no pip, setuptools or twine - made both reachable at once. Phase 2 pushes the git tag before it builds, so either failure stranded a published tag with no artifacts behind it. That is the defect: the checks being wrong is recoverable, the ordering is not. - _module_available() probes sys.executable, the interpreter that will actually run the module, rather than PATH - validate_release_tooling() runs first in complete_release, before the tag, and raises rather than warns; gh stays a warning because it is recoverable by hand - the remaining `which` calls become shutil.which Two existing tests were repaired: moving the gh check to shutil.which had left test_returns_none_when_gh_not_available taking the gh-available branch and passing for the wrong reason, and made the other test depend on gh being installed on the machine running it. scripts/README.md is added to the repository - CLAUDE.md already points readers to it, but it had never been tracked.
Softagram Impact Report for pull/181 (head commit: 2831bd7)TL;DR Arch. Impact: -10 | Changed code files: 3 | Directly impacted code files: 0⭐ Change Overview
⭐ Details of Dependency Changes (diagram)
🤖 AGENTS - machine-readable impact data (3 files changed, 0 impacted, +17/-1 deps)Change overviewHead Added dependencies (17)
Removed dependencies (1)
Impacted files (0)None. Complete data
[] 📄 Full report
Impact Report explained. Give feedback on this report to support@softagram.com |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Releasing 1.11.0 crashed twice, and the second crash happened after the git tag was already public. This fixes the ordering that made that possible, and the two checks that were testing the wrong thing.
Three defects
A — the check probed a different interpreter than the one that runs the code.
validate_preconditionsandupload_to_pypicheckedwhich twine(a PATH lookup), while the upload executes[sys.executable, "-m", "twine", ...]— the virtualenv interpreter. Those are two different Pythons, so the check passed against a system-wide twine while the actual call failed:B — the build step checked nothing at all.
build_distributionrunssys.executable setup.py sdist bdist_wheelwith nothing verifying setuptools and wheel are importable there.C — the ordering made A and B maximally expensive, and this is the real defect. Phase 2 runs sync → push tag (public, irreversible) → build → upload → release, and
complete_releasenever calledvalidate_preconditionsat all. So a missing build tool fails after the tag exists, stranding a published tag with no artifacts behind it. Fixing A and B alone would leave that intact for the next tooling drift.Neither A nor B mattered until an environment created by
uv venvmade both reachable at once — such an environment ships nopip,setuptoolsortwine, only the interpreter:The fix
_module_available()probessys.executable -c "import <module>"— the interpreter that will actually run the modulevalidate_release_tooling()runs first incomplete_release, ahead of the tag, and raises rather than warns;ghstays a warning because it is recoverable by handwhichcalls becomeshutil.which(one fewer subprocess, and it works on Windows, wherewhichdoes not exist)Raising rather than warning is deliberate: phase 2's failure mode is a half-published release, and a warning scrolling past is exactly what let this through.
validate_release_toolingis a separate method rather than an addition tovalidate_preconditions, because that one hard-fails on uncommitted changes and on not being onmain— both wrong for phase 2, and probably why it was never called there.Verification
End-to-end against a genuinely bare
uv venv, reproducing the exact state that broke the release — it now fails before anything irreversible:Mutation testing — the implementation was deliberately broken four ways, and every one was caught by the test meant to catch it:
327 tests pass.
flake8unchanged at its existing baseline, with no violations on the new lines.Two existing tests were repaired
Moving the
ghcheck toshutil.whichsilently degraded bothTestPushAndCreatePrtests, and neither failure was visible from a green run:test_returns_none_when_gh_not_availablebegan taking the gh-available branch and returnedNoneonly because the mocked stdout contained no PR URL — passing for the wrong reasontest_returns_pr_number_from_gh_outputbecame dependent onghbeing installed on the machine running the suite, and would fail on any machine without itBoth now patch
release.shutil.which, and the first additionally assertsgh pr createis never reached.Documentation
scripts/README.mdgains a prerequisite explaining that the modules must be importable by the interpreter running the script, not merely present onPATH, and that auv venvenvironment ships none of them. Both step-lists (module docstring and README) are renumbered for the new step.The file is added to the repository in this change:
CLAUDE.mdalready directs readers toscripts/README.mdfor setup instructions, but it had never been tracked, so that reference was dangling for anyone cloning the repo.