Repository navigation
Merge v4-beta to v4 (2026-10-19) - #218
Merged
Merged
Conversation
npm trusted publishing rejects self-hosted runners, and bypass-2FA tokens lose direct publish around January 2027. New opt-in input `npm-trusted-publishing` on node-simple-pnpm.yaml: - build-test-publish (gha-runner-label) runs prepublishOnly and packs every unpublished package in topological order (actions/node/pack-unpublished); - new npm-publish job on ubuntu-latest uploads the tarballs via OIDC (actions/node/publish-tarballs) and creates the release tag. `npm-token-fallback` (default true) keeps NPMJS_TOKEN as a fallback while trusted publishers are configured package by package. Default behaviour is unchanged.
…lishers discover lists every non-private workspace package of repos whose build.yaml calls a github-ci node workflow; apply runs npm trust github for each, skipping configured and unpublished packages. Needs an interactive npm login with 2FA.
This reverts commit 6a45637.
- pack-unpublished.sh / publish-tarballs.sh replace the inline scripts; the pack script calls itself per package via pnpm exec instead of generating a script. The manifest format is documented in pack-unpublished.sh. - bats suites for both scripts and for npm-trust-setup.sh apply, with fixtures and npm stubs; 0-test-npm-publish-split.yaml runs them with pnpm 9.12 and the repo's packageManager. - npm-publish job: add packages: write for GitHub Packages uploads. - publish-tarballs: GITHUB_TOKEN goes to npm.pkg.github.com only; any other non-npmjs registry fails before the first upload. - npm-trust-setup apply: require the createPackage (npm publish) permission, keep npm stderr out of the JSON, stop on unreadable output, and read the mapping from fd 3 so npm's 2FA prompt keeps stdin.
npm attaches a token by host regardless of scheme, so an http:// registry in the manifest would receive GITHUB_TOKEN or the npmjs token in clear text.
Review follow-up: the two composite actions now run one TS package, actions/node/npm-split-publish, instead of shell scripts. - ManifestEntry (src/manifest.ts) is the typed, validated contract between pack and publish. - pack orders packages itself (all dependency fields, as pnpm does; cycles are broken and reported) and checks every package before the first prepublishOnly runs, so a bad package fails without side effects. - publish parses registry URLs with URL: https only, no credentials or port in the URL, GITHUB_TOKEN for npm.pkg.github.com only. - vitest: unit tests with a fake command runner, integration tests with real pnpm on the fixture workspace (CI: pnpm 9.12 and the repo's pnpm). - lib/ is committed like the other TS actions; the test workflow fails when it does not match a fresh build. npm-trust-setup.sh stays bash with its bats suite.
…d job The manifest and tarballs reach the GitHub-hosted job as an artifact. - Validate every value (package name, semver version, .tgz basename, https registry, dist-tag, access) so none can be read by npm as an option; the pack side validates on write, so bad values fail in the build job. - Pass values as --flag=value. - Run npm from an empty temp directory: npm reads a project .npmrc from its working directory, and the artifact directory could carry one.
- Provenance: under trusted publishing npm turns provenance on for public packages and the registry rejects (E422) a package.json whose repository.url does not name the building repository; most of ours have none. pack fills a missing `repository` into the packed copy and fails early on a different repository. Reproduced on the canary (0.0.5). - Dist-tag: always pass --tag (publishConfig.tag, else latest). npm 11 refuses prereleases and versions below the highest one without it; npm 10 on the old path tagged them latest. Covered by a real-npm dry-run test. - Private packages without a version no longer fail pack. - The upload job checks each tarball's package.json against its manifest entry before the first upload. - The upload job no longer loads all caller secrets (actions/env); it gets NPMJS_TOKEN only, and only with npm-token-fallback. - Artifact named per call (app-name-slug); npm pinned to 11.21.0; pack step gets GITHUB_TOKEN as the old publish step did. - README: provenance, tag policy, artifact trust, partial-release recovery.
- Fallback token: extracted from the caller's env secret in its own step and masked (::add-mask::) before any step gets it as an input; GitHub masks the JSON secret as a whole, not values taken out of it. - Run `prepublish` after `prepublishOnly`, as pnpm publish does. - Checks (unsupported scripts, repository) run only on packages this release uploads; repository handling applies only to npmjs targets from public repositories, and the packed tarball is checked to carry it. - Tarball check: reject entries outside package/ and duplicate package.json (npm strips the top directory; last one wins), handle pax and long-name headers. - Refuse packed publishConfig keys that npm would apply for auth, proxy or scope registries, and a registry other than the entry's; correct the manifest comment that said npm ignores the tarball's publishConfig. - Re-run of a restricted package without a token: treat npm's duplicate refusal as already published. - Test workflow also runs on node-simple-pnpm.yaml changes; README documents the latest-tag trade-off.
…d unusual headers npm's unpacker normalizes paths (package/./package.json is package/package.json), so a second manifest spelled differently could pass the identity check. Paths are normalized before comparison; '..', links and other special entries, pax size records and non-octal sizes are refused.
Replace the hand-written tar reader with node-tar 7.5.22 extraction using pacote's options (strip 1, no links), as npm 11.21 reads a tarball before publishing. A custom parser can always disagree with npm on some edge case; tests now compare our result with npm's own (npm pack --dry-run --json) for duplicate manifests, odd path spellings, '..', links and missing manifests.
npm no longer accepts 2FA-bypass tokens for direct publish, so the token-based path (changesets/action + pnpm -r publish on the build runner) cannot publish anymore and an opt-in flag has no use. Remove the `npm-trusted-publishing` input and the old publish and tag steps: every node-simple-pnpm caller now packs on its runner and uploads from the GitHub-hosted `npm-publish` job with trusted publishing. `npm-token-fallback` stays (default true): npm tries OIDC first, so the token only matters while it can still publish.
…ishing MILAB-7081: npm trusted publishing (OIDC) for node-simple-pnpm
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+139
to
+142
| const entries = readManifest(dir); | ||
| if (entries.length === 0) { | ||
| log('Nothing to publish.'); | ||
| return []; |
There was a problem hiding this comment.
publishTarballs reports success when manifest.jsonl is missing. A wrong dir or incomplete artifact therefore skips every upload without failing the release. Require the manifest to exist; an existing empty manifest can still mean there is nothing to publish.
Prompt To Fix With AI
This is a comment left during a code review.
Path: actions/node/npm-split-publish/src/publish.ts
Line: 139-142
Comment:
**Missing uploads look successful**
`publishTarballs` reports success when `manifest.jsonl` is missing. A wrong `dir` or incomplete artifact therefore skips every upload without failing the release. Require the manifest to exist; an existing empty manifest can still mean there is nothing to publish.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Comment on lines
+113
to
+116
| if ! npm view "${pkg}" name >/dev/null 2>&1; then | ||
| echo "SKIP ${pkg}: not on the registry yet (publish once, then configure)" | ||
| skipped=$((skipped + 1)) | ||
| continue |
There was a problem hiding this comment.
apply treats every failed npm view as proof that the package does not exist. A network error, registry outage, or expired login silently skips existing packages and still lets the command succeed. Capture the error and skip only a confirmed not-found response; stop and show other failures.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/npm-trust-setup.sh
Line: 113-116
Comment:
**Setup hides registry errors**
`apply` treats every failed `npm view` as proof that the package does not exist. A network error, registry outage, or expired login silently skips existing packages and still lets the command succeed. Capture the error and skip only a confirmed not-found response; stop and show other failures.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it 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.
Merge v4-beta into v4
The PR appears safe to merge, with two non-blocking improvements to avoid misleading success messages.
Fix with agent prompt
Summary
This PR splits npm releases into two jobs:
Two non-blocking improvements remain: fail when the publish manifest is missing, and distinguish registry errors from missing packages during trust setup.
Important touched types:
ManifestEntry: One package passed between jobs. Newly records its name, version, tarball, registry, tag, and access.WorkspacePackage: A package directory and its named, versioned manifest. Newly used to select and order packages.PackageJson: The package fields this release code reads. Newly covers scripts, publish settings, repository details, and dependencies.PackOptions: Settings for packing. Newly carries workspace paths and the optional public source repository.Credentials: Upload tokens. Newly separates the GitHub Packages token from the optional npmjs fallback token.PublishOptions: Upload settings. Newly combines credentials with the tarball directory, access choice, and npm config path.Runner: A replaceable command runner. Newly lets tests supply command results without uploading packages.RepositoryState: Whether a declared repository is missing, matches, or differs. Newly guides repository checks and temporary additions.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Build and test] --> B[Find unpublished packages] B --> C[Run hooks and pack] C --> D[Upload artifact] D --> E[GitHub-hosted publish job] E --> F[Check manifest and tarballs] F --> G[Publish with scripts disabled] G --> H[Create release tag]Reviews (1) · Last reviewed commit: "Merge pull request #217 from milaborator..." · Reviewed by Greptile