ci(release): pass the repository visibility the publication policy checks - #59
Merged
Conversation
…ecks `scripts/release.mjs` reads the repository visibility from GITHUB_REPOSITORY_VISIBILITY and refuses to publish unless it is `public`. The runner exposes no such variable of its own, so the two npm-side steps that never passed it saw it unset and read a public repository as private. The job-level `if: github.event.repository.visibility == 'public'` hid this: it is evaluated by Actions, from the event payload, so the job started correctly and then the policy inside it refused. `images.yml` already passes the variable explicitly, which is why image publication was never affected. First observed on the v0.3.1 run for #58, which failed closed at `package-release` — nothing published, no tag. Fixing only ci.yml would have moved the same failure into publish-npm's revalidation, so both steps get it. Verified by calling `validateReleasePolicy` with the release's own inputs: unset reproduces `npm publishing is disabled until the repository is public`, and `public` validates.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9630fded6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The publication policy is unit-tested and the workflows are asserted on at length, but nothing connected the two: the integration test hands GITHUB_REPOSITORY_VISIBILITY straight to the CLI, so it exercises the policy while bypassing the wiring, and deleting either binding left both suites green. That is exactly how the v0.3.1 run reached `package-release` before refusing. Assert the binding on every step that runs `release.mjs validate`, in both workflows, and add the unset case to the denial table — unset is the mode CI actually hit, and it is indistinguishable from a private repository to the policy. Verified by removing each binding in turn: both make this test fail. Addresses: #59 (comment)
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
1 participant
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.
The first automatic release — the v0.3.1 run cut by #58 — failed closed at
package-releasewith:on a repository that is public. Nothing was published and no tag was written, so
v0.3.0is still the last release and1478f76is sitting onmainunreleased.Why
scripts/release.mjsreads visibility fromGITHUB_REPOSITORY_VISIBILITY(release.mjs:76) and refuses to publish unless it ispublic(release.mjs:47). The runner exposes no variable by that name of its own — it has to be passed in.Two steps never passed it:
ci.yml→package-release→ Validate the releaserelease.yml→ Revalidate the release and its artifactimages.yml:64does pass it, which is why image publication was never affected and why the omission stayed invisible.The job-level
if: github.event.repository.visibility == 'public'is what hides this. That expression is evaluated by Actions from the event payload, so it is correct and the job starts — and then the policy inside the job reads an unset variable and refuses. The guard and its gate disagreed while both looked right.The fix
Two
env:additions, matching whatimages.ymlalready does. Fixing onlyci.ymlwould have moved the identical failure one job downstream intopublish-npm's revalidation, so both get it.Verification
validateReleasePolicycalled with the failed release's own inputs:GITHUB_REPOSITORY_VISIBILITYnpm publishing is disabled until the repository is publicpublic(what the fix passes)node --test scripts/release.test.mjs9/9,node guards/run.mjs2/2, both workflows parse.Releasing
ci:releases nothing on its own, which is right — nothing here changes what users install. Merging this pushesmain, and that run decides from the commits sincev0.3.0, which still include #58'sfeat(image)— so it should cut 0.3.1, the version this run was meant to.Note
docs/releasing.mdsays to retry the failed jobs in the same run rather than merging again. That cannot work here: that run checks out1478f76, which does not contain this fix.Not included
A test asserting the workflows actually pass the variable — the policy is unit-tested, the wiring is not, which is how this shipped. Left out at the author's call.