Added npm publish workflow for both sdks#53
Added npm publish workflow for both sdks#53DeveloperAmrit wants to merge 2 commits intoDjedAlliance:mainfrom
Conversation
📝 WalkthroughWalkthroughA GitHub Actions workflow is added to automate publishing two SDK packages (djed-sdk and stablepay-sdk) to npm when their package versions change. The workflow compares local vs. published versions, then conditionally installs, builds, and publishes updated packages. A rollup-based build script was also added to djed-sdk's package.json. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant Repo as Repository (runner/work/*)
participant Npm as npm Registry
rect rgba(200,230,255,0.5)
GitHub->>Repo: checkout repository
Repo->>Npm: npm view <pkg>@<version> (with retries)
alt published != local or E404
Repo->>Repo: set changed=true
Repo->>Repo: npm ci (with retries)
Repo->>Repo: npm run build
Repo->>Npm: npm publish (with NODE_AUTH_TOKEN, retries)
else published == local
Repo->>Repo: skip install/build/publish
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/npm-publish.yml:
- Around line 89-99: The "Publish to NPM" step's retry loop silently allows the
workflow to succeed if all 3 npm publish attempts fail; update the step to
explicitly fail when retries are exhausted by adding a failure path after the
loop (e.g., check the final exit status and call exit 1 or append "|| exit 1" so
the job fails if publish never succeeds). Apply the same change to the
corresponding publish step in the publish-stablepay-sdk job so both "Publish to
NPM" retry loops explicitly exit non-zero when all attempts fail.
- Around line 75-83: The retry loop in the "Install dependencies" step currently
swallows failures if all 3 npm ci attempts fail; modify the step so that after
the for-loop it explicitly fails the job when npm ci never succeeds (e.g., add
an exit 1 or set a failing condition when the loop finishes without success) and
apply the same change to the identical retry block in the publish-stablepay-sdk
job; locate and update the "Install dependencies" step and the second retry
block (the npm ci loop) so the workflow exits non-zero after exhausting retries.
🧹 Nitpick comments (3)
.github/workflows/npm-publish.yml (3)
40-57: Redundantnpm viewcall creates race condition and inefficiency.When the first
npm viewfails (line 41), a second call is made (line 46) just to check for E404. This doubles network requests and introduces a race condition where the second call could behave differently.Capture stderr from the single call instead:
♻️ Suggested fix
PUBLISHED_VERSION="" for i in {1..3}; do - if OUTPUT=$(npm view "$PACKAGE_NAME" version 2>/dev/null); then + OUTPUT=$(npm view "$PACKAGE_NAME" version 2>&1) + EXIT_CODE=$? + if [ $EXIT_CODE -eq 0 ]; then PUBLISHED_VERSION=$OUTPUT break else - # Check if it's a 404 (package doesn't exist) - if npm view "$PACKAGE_NAME" version 2>&1 | grep -q "E404"; then + if echo "$OUTPUT" | grep -q "E404"; then echo "Package not found on registry. Assuming new package." PUBLISHED_VERSION="0.0.0" break fi if [ $i -lt 3 ]; then echo "Attempt $i failed. Retrying in 5 seconds..." sleep 5 fi fi doneApply the same fix to the
publish-stablepay-sdkjob (lines 127-143).
19-21: Consider using npm provenance or removing unusedid-token: writepermission.The
id-token: writepermission is typically used for npm provenance attestation, butnpm publishisn't invoked with the--provenanceflag. Either leverage provenance for supply chain security or remove the unused permission to follow least-privilege principle.Option 1: Enable provenance (recommended for supply chain security)
- name: Publish to NPM if: steps.check.outputs.changed == 'true' run: | for i in {1..3}; do - npm publish && break || { + npm publish --provenance && break || {Option 2: Remove unused permission
permissions: contents: read - id-token: writeApply to both jobs.
12-185: Consider using a matrix strategy to reduce duplication.The two jobs are nearly identical, differing only in the working directory. A matrix strategy would consolidate the logic and make maintenance easier.
♻️ Matrix-based approach
jobs: publish-sdk: runs-on: ubuntu-latest timeout-minutes: 15 strategy: matrix: sdk: [djed-sdk, stablepay-sdk] defaults: run: working-directory: ./${{ matrix.sdk }} permissions: contents: read id-token: write steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: node-version: '18' registry-url: 'https://registry.npmjs.org' # ... rest of steps remain the sameThis reduces ~90 lines of duplication and ensures both SDKs always use identical publish logic.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/npm-publish.yml:
- Around line 19-21: Remove the unused id-token: write permission from the
workflow permissions block(s); edit the permissions mapping that currently
contains "contents: read" and "id-token: write" so it only includes required
keys (e.g., remove the "id-token" entry), and apply the same removal for the
other permissions block referenced (the one around lines 123-125) to enforce
least privilege.
- Around line 67-73: Replace the simple inequality check ([ "$LOCAL_VERSION" !=
"$PUBLISHED_VERSION" ]) in both version-check blocks with a semver-aware "is
newer" test: use a sort -V comparison such as checking that the last line of
printf '%s\n' "$PUBLISHED_VERSION" "$LOCAL_VERSION" | sort -V is equal to
"$LOCAL_VERSION" and also that the two versions are not equal; i.e. change the
condition to something like: if [ "$(printf '%s\n' "$PUBLISHED_VERSION"
"$LOCAL_VERSION" | sort -V | tail -n1)" = "$LOCAL_VERSION" ] && [
"$LOCAL_VERSION" != "$PUBLISHED_VERSION" ]; then ... and apply the same
replacement for the second block (the other occurrence of [ "$LOCAL_VERSION" !=
"$PUBLISHED_VERSION" ]).
| permissions: | ||
| contents: read | ||
| id-token: write |
There was a problem hiding this comment.
Drop unused id-token: write permission (least privilege).
id-token: write is not used by any step here (no OIDC/provenance flow), so this grants unnecessary privilege in both jobs.
🔧 Proposed fix
permissions:
contents: read
- id-token: write Also applies to: 123-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/npm-publish.yml around lines 19 - 21, Remove the unused
id-token: write permission from the workflow permissions block(s); edit the
permissions mapping that currently contains "contents: read" and "id-token:
write" so it only includes required keys (e.g., remove the "id-token" entry),
and apply the same removal for the other permissions block referenced (the one
around lines 123-125) to enforce least privilege.
| if [ "$LOCAL_VERSION" != "$PUBLISHED_VERSION" ]; then | ||
| echo "Version changed." | ||
| echo "changed=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "Version unchanged." | ||
| echo "changed=false" >> "$GITHUB_OUTPUT" | ||
| fi |
There was a problem hiding this comment.
Publish gate should require a version increase, not just inequality.
On Line 67 and Line 169, != treats downgrades as “changed”, which can trigger failing publish attempts and violates the objective (“newer than published”).
🔧 Proposed fix
- if [ "$LOCAL_VERSION" != "$PUBLISHED_VERSION" ]; then
- echo "Version changed."
+ if [ "$LOCAL_VERSION" != "$PUBLISHED_VERSION" ] && \
+ [ "$(printf '%s\n' "$PUBLISHED_VERSION" "$LOCAL_VERSION" | sort -V | tail -n1)" = "$LOCAL_VERSION" ]; then
+ echo "Version increased."
echo "changed=true" >> "$GITHUB_OUTPUT"
else
- echo "Version unchanged."
+ echo "Version not increased."
echo "changed=false" >> "$GITHUB_OUTPUT"
fiApply the same update to both Check version change blocks.
Also applies to: 169-175
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/npm-publish.yml around lines 67 - 73, Replace the simple
inequality check ([ "$LOCAL_VERSION" != "$PUBLISHED_VERSION" ]) in both
version-check blocks with a semver-aware "is newer" test: use a sort -V
comparison such as checking that the last line of printf '%s\n'
"$PUBLISHED_VERSION" "$LOCAL_VERSION" | sort -V is equal to "$LOCAL_VERSION" and
also that the two versions are not equal; i.e. change the condition to something
like: if [ "$(printf '%s\n' "$PUBLISHED_VERSION" "$LOCAL_VERSION" | sort -V |
tail -n1)" = "$LOCAL_VERSION" ] && [ "$LOCAL_VERSION" != "$PUBLISHED_VERSION" ];
then ... and apply the same replacement for the second block (the other
occurrence of [ "$LOCAL_VERSION" != "$PUBLISHED_VERSION" ]).
Fixes #42
Pre-requisites
Summary
This PR introduces a GitHub Actions workflow to automate the publishing of the djed-sdk and stablepay-sdk packages to the NPM registry. The workflow ensures that only updated versions of the packages are published, improving the release process's efficiency and reliability.
Key features
djed-sdk/package.jsonandstablepay-sdk/pacakage.jsonWorkflow details
Benefits
How to test
Summary by CodeRabbit