fix(ci): npm-publish dry-run never took effect; document broken release auto-trigger#36
Open
verivus-open wants to merge 1 commit into
Open
fix(ci): npm-publish dry-run never took effect; document broken release auto-trigger#36verivus-open wants to merge 1 commit into
verivus-open wants to merge 1 commit into
Conversation
The dry-run input is declared type: boolean but the steps gated on `inputs.dry-run == 'true'` (a string). GitHub coerces `true == 'true'` to `1 == NaN` -> false, so the 'Publish (dry run)' step was always skipped and the real 'Publish' step always ran. A workflow_dispatch with dry-run=true published for real (observed on v2.6.0). Gate on the boolean directly.
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
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.
Summary
Two issues surfaced while cutting v2.6.0. This PR fixes the first (a clear, self-contained bug) and documents the second (needs a secret/decision — not implemented here).
1. Fixed:
dry-runinput never actually did a dry runnpm-publish.ymldeclaresdry-runastype: booleanbut the steps gated oninputs.dry-run == 'true'(a string). In GitHub expressions, comparing a boolean to a string coerces both to numbers:true == 'true'→1 == NaN→ false. So:Publish (dry run)(if: inputs.dry-run == 'true') was always skippedPublish(if: inputs.dry-run != 'true') was always runResult: a
workflow_dispatchwithdry-run=truepublished for real. This actually happened during the v2.6.0 release (a "dry run" publishedllm-cli-gateway@2.6.0).Fix: gate on the boolean directly —
if: ${{ inputs.dry-run }}/if: ${{ !inputs.dry-run }}. On areleaseeventinputs.dry-runis null (falsy), so the real publish runs and the dry-run step is skipped — unchanged intended behavior.2. Documented (NOT fixed here): release → npm-publish auto-trigger is broken for workflow-created releases
release-tag-publish.yml's header says it "creates GitHub release (triggers npm-publish + release-installer)." That's false when the release is created by the workflow'sGITHUB_TOKEN: GitHub does not let events authored byGITHUB_TOKEN(i.e.github-actions[bot]) trigger new workflow runs (recursion guard).Evidence:
verivusOSS-releasesuser account (a PAT) — a user-authored release event does triggernpm-publish.yml.github-actions[bot](workflowGITHUB_TOKEN) → no trigger → had to dispatchnpm-publish.ymlmanually.Recommended fix (needs your decision):
gh release createinrelease-tag-publish.yml, so the release event triggersnpm-publish.yml(andrelease-installer.yml). Cleanest; matches the documented design. OnlyNPM_TOKENexists today.gh workflow run npm-publish.ymlafter creating the release (GITHUB_TOKENcan't dispatch either).npm publishstep (OIDCid-token: write) directly torelease-tag-publish.yml; no new secret, but duplicates publish logic.I didn't implement these because they need a secret you provision and/or an architecture call. Happy to add the chosen option as a follow-up commit to this PR.
Test
dry-runfix is a pure conditional change; the required CI checks on this PR exercise it.