Conversation
commit: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR modifies the CI/CD release workflow to address issues with the --provenance flag during npm publishing. The changes switch from direct pnpm publish to a two-step process of packing and then publishing with npm, remove the problematic --provenance flag, and add Node.js to the Nix development environment.
Key changes:
- Removed the
preinstallscript that enforced pnpm usage - Added
nodejs_24to the Nix development shell - Changed the release workflow to pack with pnpm and publish with npm, removing the
--provenanceflag
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| package.json | Removed the preinstall script that enforced pnpm as the package manager |
| flake.nix | Added nodejs_24 to the development environment buildInputs |
| .github/workflows/release.yaml | Split publishing into separate pack and publish steps, switching from pnpm to npm and removing the --provenance flag |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "lint:oxlint": "oxlint --max-warnings=0 --type-aware --type-check", | ||
| "lint:knip": "knip", | ||
| "preinstall": "npx only-allow pnpm", | ||
| "prepack": "npm pkg delete scripts.preinstall && pnpm run build", |
There was a problem hiding this comment.
The prepack script references scripts.preinstall, which was just removed on the line above. This will cause the npm pkg delete command to fail silently or produce an error since it's trying to delete a non-existent script. Either remove the npm pkg delete scripts.preinstall part from this script or keep the preinstall script.
| "prepack": "npm pkg delete scripts.preinstall && pnpm run build", | |
| "prepack": "pnpm run build", |
| - name: 📦 Pack package | ||
| run: nix develop --command pnpm pack --no-git-checks | ||
|
|
||
| - name: 🚀 Publish package | ||
| run: nix develop --command pnpm publish --provenance --no-git-checks --access public | ||
| shell: bash | ||
| run: | | ||
| PACKAGE_TGZ=$(ls *.tgz | head -n 1) | ||
| echo "Publishing package: $PACKAGE_TGZ" | ||
| npm publish "$PACKAGE_TGZ" --access public |
There was a problem hiding this comment.
The workflow switches from using pnpm to npm for publishing. The pack step uses pnpm, but the publish step uses npm. This inconsistency could lead to confusion and potential issues. Consider using the same package manager for both operations, or document why different tools are being used. Additionally, the prepack script in package.json is designed to work with pnpm, so using npm publish directly on the tarball bypasses the package manager alignment.
| run: | | ||
| PACKAGE_TGZ=$(ls *.tgz | head -n 1) | ||
| echo "Publishing package: $PACKAGE_TGZ" | ||
| npm publish "$PACKAGE_TGZ" --access public |
There was a problem hiding this comment.
The removal of the --provenance flag eliminates npm's provenance attestation feature, which provides cryptographic verification of the package's origin and build process. This reduces supply chain security by removing the ability for consumers to verify that the published package was built in the expected CI environment. The id-token: write permission on line 28 suggests provenance was intentionally configured. If --provenance was removed due to errors, consider investigating and fixing the underlying issue rather than removing this security feature.
| npm publish "$PACKAGE_TGZ" --access public | |
| npm publish "$PACKAGE_TGZ" --access public --provenance |
| run: nix develop --command pnpm publish --provenance --no-git-checks --access public | ||
| shell: bash | ||
| run: | | ||
| PACKAGE_TGZ=$(ls *.tgz | head -n 1) |
There was a problem hiding this comment.
The command uses head -n 1 to select the first tarball, which could select an unexpected file if multiple .tgz files exist in the directory. Consider adding error handling to verify exactly one tarball exists, or use a more specific pattern like finding the tarball with the expected package name format.
This reverts commit 86f18bf.
Summary by cubic
Switched the release workflow to pack with pnpm and publish the tarball with npm, removing --provenance to fix CI publish failures.
Written for commit c3f8462. Summary will update automatically on new commits.