Skip to content

fix: stop the release from trying to publish its dependencies - #539

Merged
shenxianpeng merged 3 commits into
mainfrom
claude/fix-publish-uploads-deps-602anc
Aug 7, 2026
Merged

fix: stop the release from trying to publish its dependencies#539
shenxianpeng merged 3 commits into
mainfrom
claude/fix-publish-uploads-deps-602anc

Conversation

@shenxianpeng

@shenxianpeng shenxianpeng commented Aug 7, 2026

Copy link
Copy Markdown
Member

The failure

Run 31178652101 (v2.13.4):

Uploading pyyaml-6.0.3-cp314-cp314-manylinux_2_28_x86_64.whl
403 Invalid API Token: project-scoped token is not valid for project: 'PyYAML'

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

run tag result
81 v2.13.4
80 v2.13.3
79 v2.13.2
78 v2.13.1
77 v2.13.0
76 v2.12.2
75 v2.12.1
74 v2.12.0
73 v2.11.1
72 v2.11.0
71 v2.10.1

Nobody noticed because the release still works. Uploads happen file by file and commit_check-* sorts before pyyaml-*, 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 wheel resolves and writes a wheel for the project and every dependency. Reproduced locally against this tree:

$ pip wheel -w dist .
commit_check-2.13.4-py3-none-any.whl
pyyaml-6.0.3-cp311-...-manylinux_2_28_x86_64.whl     ← not ours

That was harmless while the upload named what it wanted. #453 (v2.11.0) 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 input — it uploads the whole directory. The two steps in between (twine check dist/commit_check* and the attestation's subject-path: dist/commit_check*) kept their globs, which is why they still pass and why the mismatch went unseen.

The fix

One word:

-python -m pip wheel -w dist .
+python -m pip wheel --no-deps -w dist .
$ pip wheel --no-deps -w dist .
commit_check-2.13.4-py3-none-any.whl

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 -printf is 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 wheel produces no sdist, and PyPI confirms none exists for any version:

$ curl -s https://pypi.org/pypi/commit-check/2.13.3/json | jq '[.urls[].packagetype] | unique'
["bdist_wheel"]

Switching the build to python -m build would 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

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
@shenxianpeng
shenxianpeng requested a review from a team as a code owner August 7, 2026 12:50
@github-actions github-actions Bot added the bug Something isn't working label Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The package publishing workflow now builds wheels without dependency resolution. It also checks that dist/ contains only expected commit-check artifacts before publication and fails when foreign files are present.

Changes

Package publication safeguards

Layer / File(s) Summary
Build and publication validation
.github/workflows/publish-package.yml
The wheel build uses pip wheel --no-deps. A pre-publish check reports unexpected files in dist/ and fails the workflow.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main workflow change: preventing dependency wheels from being published during release.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/fix-publish-uploads-deps-602anc

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 16d328f and c002ec0.

📒 Files selected for processing (1)
  • .github/workflows/publish-package.yml

Comment thread .github/workflows/publish-package.yml Outdated
shenxianpeng and others added 2 commits August 7, 2026 12:53
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
@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@shenxianpeng
shenxianpeng merged commit acdeb5b into main Aug 7, 2026
8 checks passed
@shenxianpeng
shenxianpeng deleted the claude/fix-publish-uploads-deps-602anc branch August 7, 2026 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant