Skip to content

voice-agents-python: a hand publish runs on the release tag; dry runs for every registry, apart from real publishes - #8

Merged
arbianst merged 1 commit into
mainfrom
voice-agents/render-428dbd0
Nov 1, 2026
Merged

arbianst merged 1 commit into
mainfrom
voice-agents/render-428dbd0

Conversation

@sent-dm-sdk-pipeline

Copy link
Copy Markdown
Contributor

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.

@sent-dm-sdk-pipeline
sent-dm-sdk-pipeline Bot requested a review from a team as a code owner November 1, 2026 15:06
@arbianst
arbianst merged commit 868deca into main Nov 1, 2026
4 checks passed
@arbianst
arbianst deleted the voice-agents/render-428dbd0 branch November 1, 2026 15:07

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment on lines +48 to +51
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 (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.

Comment on lines +20 to +23
concurrency:
# A dry run never queues behind, or displaces, a real publish.
group: publish-pypi${{ inputs.dry_run && '-dry-run' || '' }}
cancel-in-progress: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 (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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants