Repository navigation
voice-agents-python: a hand publish runs on the release tag; dry runs for every registry, apart from real publishes - #8
Conversation
…egistry, apart from real publishes
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I checked two other things and found no problem with either. The dry-run step never prints the OIDC token, and it sends the minted upload token's response body to /dev/null. On a release event, inputs.dry_run is empty, so the setup-uv, Build and Publish steps still run.
Extended reasoning...
Two things were ruled out beyond the inline findings. First, token exposure in the new "Check the trusted publisher" step: curl writes the mint-token response to /dev/null, and only the HTTP status code is echoed. Second, on release-triggered runs, inputs.dry_run is empty, so the if: ${{ !inputs.dry_run }} guards still let the publish steps run.
| fi | ||
| if [ "$GITHUB_EVENT_NAME" = "workflow_dispatch" ] && { [ "$GITHUB_REF_TYPE" != "tag" ] || [ "$GITHUB_REF_NAME" != "v$version" ]; }; then | ||
| echo "a hand run publishes the release tag: gh workflow run publish-pypi.yml --ref v$version" >&2 | ||
| exit 1 |
There was a problem hiding this comment.
🟡 (optional) If a publish fails because of this workflow, a maintainer can no longer re-publish that release with a fixed workflow. Only a new release gets the package out. A hand run must use --ref v<version>, and GitHub then runs the publish-pypi.yml stored at that tag, bugs included. A run from main, where the fix lives, is refused at line 48. Fix: let a hand run start from main but build the tag's source, e.g. a tag input that the checkout step uses as ref: and the version check compares against. The tag-only guarantee still holds this way.
Why this was flagged
For workflow_dispatch, GitHub reads the workflow file from the dispatched ref. The check at publish-pypi.yml:48 exits 1 unless GITHUB_REF_TYPE is tag and GITHUB_REF_NAME is v$version. So every allowed hand run uses the workflow as it stood at the release tag. Suppose a release's publish fails because of the workflow itself, for example a broken setup-uv/uv publish step or a bad flag. The fix lands on main as a ci: commit, which CLAUDE.md says is hidden and cuts no release. Running from the tag repeats the broken workflow, and running from main is refused. The only way out is a feat/fix/chore commit that cuts a new version, and the failed version never reaches PyPI. On the base branch, a dispatch from main after the fix rebuilt and published while main still carried the released version. The header comment at line 1 promises a hand run can 'publish again if a run failed', but that does not hold for this class of failure.
Verification: Trigger: a release's publish fails because of the workflow file itself, and the fix then lands on main. The "The version to publish" step refuses any non-dry workflow_dispatch run unless the ref is the release tag. A run from main, where the fix lives, exits 1 at that check. On the base, nothing gated the ref, so a dispatch from main ran main's fixed workflow.
| concurrency: | ||
| # A dry run never queues behind, or displaces, a real publish. | ||
| group: publish-pypi${{ inputs.dry_run && '-dry-run' || '' }} | ||
| cancel-in-progress: false |
There was a problem hiding this comment.
🟡 (optional) A real release publish can now be cancelled without anyone noticing, when three real publish runs overlap. On the base branch every run went ahead. All real runs share the repo-wide group publish-pypi. GitHub keeps one running and one pending run per group and cancels the pending one when a third arrives, whatever cancel-in-progress says. Fix: include the version or ref in the real group, e.g. publish-pypi-${{ github.ref_name }}. Runs for different releases then never displace each other, and repeat runs of one tag stay serialized.
Why this was flagged
The trigger needs three runs in the same window, for example a release event plus hand re-publish dispatches. The concurrency block at publish-pypi.yml:20-23 puts every non-dry run (release events and hand publishes) in one group, publish-pypi, whatever the tag. GitHub concurrency allows one in-progress run and one pending run per group. A newly queued run cancels the existing pending run even with cancel-in-progress: false. Example: run A (a hand re-publish of v0.1.1) is running and run B (the release event for v0.1.2) is pending. A third dispatch C then cancels B, and v0.1.2 never publishes unless someone notices the cancelled run. The base branch had no concurrency block, so each run went ahead on its own. The comment at line 21 only says a dry run never displaces a real publish. It does not cover real publishes displacing each other, which this change introduces.
Verification: Lines 20-23 put every non-dry run in one fixed group, whatever its tag or release. When a new run is queued, the existing pending run is cancelled. cancel-in-progress: false only protects the run that is already in progress, not the pending one. The base workflow had no concurrency block, so every run went ahead.
Rendered by the Voice Agents lane in sent-dm-sdk-pipeline from sent-dm-voice-protocol at
v0.2.0, after https://github.com/sentdm/sent-dm-sdk-pipeline/commit/428dbd0523533500f535443a998c97e6774e7c3c.What a reviewer checks: this repository's CI, the conformance job included, is green on this PR; the generated frames module and the runtime changed as the protocol or the template did; nothing release-please owns moved. Merging makes release-please open the release.