fix: stop the release from trying to publish its dependencies - #539
Conversation
Every release since v2.11.0 has failed, ten in a row, ending with:
403 Invalid API Token: project-scoped token is not valid for
project: 'PyYAML'
The build runs `pip wheel -w dist .`, which resolves and writes a wheel
for the project *and every dependency*, so dist/ holds commit_check and
pyyaml. Reproduced locally:
$ pip wheel -w dist .
commit_check-2.13.4-py3-none-any.whl
pyyaml-6.0.3-cp311-...-manylinux_2_28_x86_64.whl
That was harmless while the upload named what it wanted. #453 migrated
from twine to pypa/gh-action-pypi-publish and the scoping was lost in the
move:
- run: twine upload dist/commit_check*
+ uses: pypa/gh-action-pypi-publish@...
The action has no file selection -- it uploads the whole directory -- so
PyPI is asked to accept PyYAML under a token scoped to commit-check, and
correctly refuses.
Nobody noticed because the release still works. Uploads happen per file
and commit_check sorts before pyyaml, so our wheel lands, then the job
goes red on someone else's. The package is on PyPI; only the workflow
looked broken, which is a state that is easy to keep ignoring.
`--no-deps` builds our wheel alone, which is all this workflow ever meant
to publish:
$ pip wheel --no-deps -w dist .
commit_check-2.13.4-py3-none-any.whl
Also adds a guard after the build. The publish step cannot select files,
so "dist/ contains only our artifacts" is a precondition it depends on and
cannot verify; the guard checks it where the problem is fixable and names
it, instead of surfacing as a 403 about a project we do not own. Checked
against both states: it fails on the old dist/ and passes on the new one.
Note for later, not changed here: this workflow has only ever published
wheels -- `pip wheel` produces no sdist, and PyPI has none for any
version. Switching the build to `python -m build` would add one, but that
changes what a release contains and belongs in its own change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
📝 WalkthroughWalkthroughThe package publishing workflow now builds wheels without dependency resolution. It also checks that ChangesPackage publication safeguards
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish-package.yml:
- Around line 42-46: Update the dist validation guard to allow only regular
files whose names match the commit_check artifact prefix and end in .whl; ensure
directories and other extensions are included in foreign and cause the workflow
to fail. Keep the existing error and exit behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 555611e6-6eca-4e20-9747-5202673dd4b1
📒 Files selected for processing (1)
.github/workflows/publish-package.yml
Review pointed out the guard tested only the prefix, so anything called commit_check-* passed it -- an sdist, a stray .txt, a leftover directory. Confirmed against a dist/ holding all four shapes: the old expression flagged just pyyaml and waved the other three through. The tighter form says what the step actually depends on. dist/ should hold our wheels and nothing else, so a release that starts producing something else should stop and be looked at, rather than be uploaded because it happened to carry the right prefix. That includes an sdist, deliberately. Adding one is a change to what a release contains, and it should be made on purpose -- the guard failing with the file named is the prompt to update this line, not an obstacle to route around. Error message now says "wheels" rather than "artifacts", matching what is being checked. Verified by extracting the guard from the workflow YAML and running it verbatim against the two real dist/ directories from the earlier reproduction: exit 1 naming pyyaml on the pre-fix build, exit 0 on the --no-deps build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
Maintainer review: --no-deps is the fix, and the guard was two thirds of the diff for something that only fires if someone later removes the flag. Agreed, and removed. The argument for keeping it was that the 403 is a weak signal -- uploads run file by file and commit_check-* sorts before pyyaml-*, so our wheel lands on PyPI and only then does the job go red. The release works and the red reads as noise, which is how ten of them were ignored. But that does not justify eleven lines guarding a flag that now carries a comment saying why it exists; removing it would be a deliberate act, and the guard added a fresh way for a release to fail (find -printf is GNU-only, and any false positive blocks publishing outright). What ships is one changed word and the explanation for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
|



The failure
Run 31178652101 (v2.13.4):
The release is trying to upload PyYAML to PyPI. The token is scoped to
commit-check, so PyPI refuses — correctly.It is not new: ten consecutive releases have failed
Nobody noticed because the release still works. Uploads happen file by file and
commit_check-*sorts beforepyyaml-*, so our wheel lands on PyPI and then the job goes red on someone else's package. The artefact ships; only the workflow looks broken — a state that is easy to keep ignoring, and was, ten times.Root cause
pip wheelresolves and writes a wheel for the project and every dependency. Reproduced locally against this tree:That was harmless while the upload named what it wanted. #453 (v2.11.0) migrated from
twinetopypa/gh-action-pypi-publish, and the scoping was lost in the move:The action has no file-selection input — it uploads the whole directory. The two steps in between (
twine check dist/commit_check*and the attestation'ssubject-path: dist/commit_check*) kept their globs, which is why they still pass and why the mismatch went unseen.The fix
One word:
An earlier revision of this PR also added a guard that failed the build when
dist/held anything else. It was dropped on review: it was two thirds of the diff, guarding a flag that now carries a comment explaining why it exists, and it introduced a fresh way for a release to fail (find -printfis GNU-only, and any false positive would block publishing outright). The flag is the fix; the comment is the protection.Separate finding, deliberately not fixed here
This workflow has only ever published wheels.
pip wheelproduces no sdist, and PyPI confirms none exists for any version:Switching the build to
python -m buildwould produce an sdist as well and also fix this bug — but it changes what a release contains, which deserves its own PR and its own decision rather than riding along with an outage fix.After merging
v2.13.4 is already on PyPI (the wheel uploaded before the 403), so nothing needs re-releasing. The next release should be the first green publish since June.
🤖 Generated with Claude Code
https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn