Release pipeline #3039
Release pipeline #3039
Conversation
WalkthroughAdded workflow_call/git_ref support and full-history checkouts to CI workflows, introduced a new multi-stage Release Pipeline workflow (workflow_dispatch) that builds, scans (SBOM/Trivy), packages, and publishes npm packages with manual review gating, and documented the release process in RELEASE.md. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
.github/workflows/nodejs.yml (1)
70-82:env.GIT_REFis undefined in unit job – checkout will ignore the desired refThe job does not declare the
env:block added tobuild-and-lint, thereforeref: ${{ env.GIT_REF }}expands to an empty string, making the step checkout the default branch.Either hoist
env:to the workflow level or replicate it per job:unit: runs-on: ubuntu-latest timeout-minutes: 10 + env: + GIT_REF: ${{ inputs.git_ref || github.ref }}Same applies to
browserand (optionally)integrationjobs.
Without this, the release pipeline’s tests might run against the wrong code.
♻️ Duplicate comments (1)
.github/workflows/nodejs.yml (1)
173-176: Same undefenv.GIT_REFissue in browser jobSee previous comment –
ref: ${{ env.GIT_REF }}will evaluate to empty.
Add theenv:block or move it to the workflow top-level.
🧹 Nitpick comments (6)
.github/workflows/release.yml (1)
80-93: Slack step name & condition mis-match actual behaviourThe step is placed in build stage yet is called “Notify Slack if tests fail”.
Worse, theif: failure()condition fires on any previous-step failure in the job, not only tests.Consider:
- - name: Notify Slack if tests fail + - name: Notify Slack on build failureOptional: scope the
if:to the build step for clearer intent.RELEASE.md (5)
37-38: Spelling: “pipelin” → “pipeline”-1. The pipelin will pause at the "Review test and security scan result" step, +1. The pipeline will pause at the "Review test and security scan result" step,
60-61: Spelling: “vulnerabilites” → “vulnerabilities”
67-68: Spelling: “Genrate” → “Generate”
98-99: Markdown heading formattingMissing space after
##, violating MD018.-##⚠️ **Important Notes** +## ⚠️ **Important Notes**
14-19: Specify language for fenced code blocks (MD040)Add
jsonfor the package snippet andtext/bashfor plain examples to improve readability and lint compliance.Also applies to: 30-35, 88-96
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/nodejs.yml(3 hunks).github/workflows/release.yml(1 hunks)RELEASE.md(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
.github/workflows/nodejs.yml (1)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
RELEASE.md (1)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
.github/workflows/release.yml (1)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
🪛 actionlint (1.7.7)
.github/workflows/nodejs.yml
28-28: property "git_ref" is not defined in object type {}
(expression)
.github/workflows/release.yml
261-261: property "package_version" is not defined in object type {git_ref: string; package_name: string}
(expression)
🪛 LanguageTool
RELEASE.md
[grammar] ~37-~37: Ensure spelling is correct
Context: ...lease details and scan result** 1. The pipelin will pause at the "Review test and secu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~60-~60: Ensure spelling is correct
Context: ...OWASP Dependency-Track for tracking the vulnerabilites. - Packages the module with lerna and u...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~67-~67: Ensure spelling is correct
Context: ... Slack. --- ### 4. Review Stage - Genrate a summary of: - Package name - Vers...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
RELEASE.md
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
88-88: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
93-93: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
98-98: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🔇 Additional comments (1)
.github/workflows/release.yml (1)
133-143: Typo & hard-coding – regeneratelerna.jsonmore robustly
- Typo: “choosen”.
- The jq expression overwrites all packages; if
lerna.jsonuses advanced config (workspaces, ignore, etc.) this loses data.- - name: Generate lerna.json for choosen the package + - name: Restrict lerna to selected package @@ - jq --arg pkg "packages/${{ env.PACKAGE_NAME }}" '.packages = [$pkg]' lerna.json > lerna.tmp.json && mv lerna.tmp.json lerna.json + jq --arg pkg "packages/${{ env.PACKAGE_NAME }}" \ + '(.packages // []) as $p | $p | [$pkg]' \ + lerna.json > lerna.tmp.json && mv lerna.tmp.json lerna.json[ suggest_optional_refactor ]
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
.github/workflows/nodejs.yml (1)
27-28: Duplicate – wrong context for reusable-workflow input
See earlier review: useinputs.git_ref, notgithub.event.inputs.git_ref..github/workflows/release.yml (2)
203-205: Duplicate –$GIT_REF&$COMMIT_SHAprinted but never set
Previously reported; variables are still undefined here.
271-272: Duplicate – Slack failure message references non-existentpackage_versioninputStill using
${{ github.event.inputs.package_version }}which was removed.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
154-156: Minor typo in log output
echo "packageing for ..."→packaging. Purely cosmetic but worth fixing for log clarity.- echo "packageing for ${{ env.PACKAGE_NAME }}" + echo "packaging ${PACKAGE_NAME}"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/faucet_test.yml(2 hunks).github/workflows/nodejs.yml(6 hunks).github/workflows/release.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
.github/workflows/faucet_test.yml (1)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
.github/workflows/release.yml (2)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2779
File: packages/xrpl/HISTORY.md:10-14
Timestamp: 2024-10-11T18:48:28.561Z
Learning: Ensure to reference the correct version numbers when commenting on changes to `HISTORY.md` to avoid confusion.
.github/workflows/nodejs.yml (1)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
🪛 actionlint (1.7.7)
.github/workflows/release.yml
270-270: property "package_version" is not defined in object type {git_ref: string; package_name: string}
(expression)
.github/workflows/nodejs.yml
28-28: property "git_ref" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: browser (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: unit (22.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: unit (20.x)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
RELEASE.md (7)
14-19: Add language identifier to fenced JSON block
Markdown-lint (MD040) complains and GitHub won’t apply JSON syntax highlighting.- ``` + ```json { "name": "<package_name>", "version": "x.y.z" } - ``` + ```
35-38: Fix spelling & pluralisation in the review-stage description-1. The pipelin will pause at the "Review test and security scan result" step, at least 1 approver is required to review and approve the release. +1. The pipeline will pause at the “Review test and security scan results” step; at least one approver is required to review and approve the release.
50-53: Correct typos and tighten wording for the test stage-### 2. **Run Tests** -- Triggers the facucet_test, nodejs workflow which will run unit test, intgration test, faucet test etc. +-### 2. **Run Tests** +- Invokes the `faucet_test` and `nodejs` reusable workflows to run unit, integration, and faucet tests.
59-61: Spelling: “vulnerabilities”-- Uploads SBOM to OWASP Dependency-Track for tracking the vulnerabilites. +- Uploads the SBOM to OWASP Dependency-Track for tracking vulnerabilities.
66-71: Spelling: “Generate” & minor formatting-### 4. **Review Stage** -- Genrate a summary of: +-### 4. **Review Stage** +- Generates a summary of:
88-95: Add language identifier to fenced code block-``` +```text <package_name>@<version>--- `98-98`: **Missing space after atx heading marker** ```diff -##⚠️ **Important Notes** +## ⚠️ **Important Notes**
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
RELEASE.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
RELEASE.md (1)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
🪛 LanguageTool
RELEASE.md
[grammar] ~37-~37: Ensure spelling is correct
Context: ...lease details and scan result** 1. The pipelin will pause at the "Review test and secu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~51-~51: Ensure spelling is correct
Context: ...dejs workflow which will run unit test, intgration test, faucet test etc. - Ensures the code at...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~60-~60: Ensure spelling is correct
Context: ...OWASP Dependency-Track for tracking the vulnerabilites. - Packages the module with lerna and u...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~67-~67: Ensure spelling is correct
Context: ... Slack. --- ### 4. Review Stage - Genrate a summary of: - Package name - Vers...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
RELEASE.md
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
88-88: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
93-93: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
98-98: No space after hash on atx style heading
(MD018, no-missing-space-atx)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Analyze (javascript)
- GitHub Check: integration (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: browser (22.x)
- GitHub Check: unit (22.x)
- GitHub Check: generate-documentation (22.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: unit (20.x)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/release.yml (2)
266-272: Still references non-existentgithub.event.inputs.package_version
See earlier review – this input is not defined; useenv.PACKAGE_VERSION.
190-205:GIT_REFandCOMMIT_SHAechoed but never setThe summary prints empty values because the variables are undefined.
- echo "🔍 Please review the following details before proceeding:" + # Set variables + GIT_REF="${{ github.event.inputs.git_ref }}" + COMMIT_SHA="$(git rev-parse --short HEAD)" + + echo "🔍 Please review the following details before proceeding:"
🧹 Nitpick comments (5)
.github/workflows/release.yml (5)
15-17: Concurrency group will cancel parallel package releasesUsing a fixed
group: releasemeans triggering two releases in parallel will cause the second one to cancel the first.
If independent packages can/should be released simultaneously, scope the group bypackage_name(and optionallygit_ref) to avoid unnecessary cancellations.-concurrency: - group: release +concurrency: + group: release-${{ github.event.inputs.package_name }}
94-101: Hard-coded package name in Slack failure message
xrpl.jsis hard-coded, so notifications for other packages are misleading.- MESSAGE="❌ Build failed for xrpl.js ${{ env.PACKAGE_VERSION }}. ... + MESSAGE="❌ Build failed for ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. ...
143-149: Typo in log message
choosen→chosen. Purely cosmetic but easy to fix.- echo "🔧 Updating lerna.json to include only packages/${{ env.PACKAGE_NAME }}" + echo "🔧 Updating lerna.json to include only packages/${{ env.PACKAGE_NAME }}"
157-158: Another typo: “packageing”- echo "packageing for ${{ env.PACKAGE_NAME }}" + echo "Packaging tarball for ${{ env.PACKAGE_NAME }}"
252-259: Hard-coded package name in Slack success messageThe notification always says “xrpl.js” even for other packages.
- MESSAGE="✅ Released xrpl.js v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." + MESSAGE="✅ Released ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully."
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
.github/workflows/release.yml (2)
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `@v4`.
Learnt from: mvadari
PR: XRPLF/xrpl.js#2779
File: packages/xrpl/HISTORY.md:10-14
Timestamp: 2024-10-11T18:48:28.561Z
Learning: Ensure to reference the correct version numbers when commenting on changes to `HISTORY.md` to avoid confusion.
🪛 actionlint (1.7.7)
.github/workflows/release.yml
270-270: property "package_version" is not defined in object type {git_ref: string; package_name: string}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: integration (20.x)
- GitHub Check: integration (22.x)
- GitHub Check: unit (22.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: browser (22.x)
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
.github/workflows/release.yml (2)
283-296: Fix Slack failure wording and package name.Message incorrectly says “Tests failed” and hardcodes “xrpl.js”. This step runs in the release job.
- MESSAGE="❌ Tests failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Release pipeline failed for ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
121-129: Pin Trivy correctly and fail on CRITICAL/HIGH findings.
- The action ref should be a tagged version (e.g., v0.28.0), not “0.28.0”.
- exit-code: 0 ignores vulnerabilities and defeats the gate.
- uses: aquasecurity/[email protected] + uses: aquasecurity/[email protected] @@ - exit-code: 0 + exit-code: 1If you want to allow lower severities while still failing on CRITICAL/HIGH, keep severity as-is and set exit-code to 1 (as above).
🧹 Nitpick comments (3)
.github/workflows/release.yml (3)
100-113: Use dynamic package name and more accurate wording in Slack failure.Hardcoded “xrpl.js” and “Build failed” can be misleading and inconsistent for other packages.
- MESSAGE="❌ Build failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Pre-release pipeline failed for ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
153-163: Typos and unnecessary lerna.json mutation.
- “choosen” → “chosen”.
- Overwriting lerna.json just to exec npm pack by scope isn’t needed; lerna exec respects --scope without altering config.
- - name: Generate lerna.json for choosen the package + - name: Prepare chosen package run: | - - echo "🔧 Updating lerna.json to include only packages/${{ env.PACKAGE_NAME }}" - - # Use jq to update the packages field safely - jq --arg pkg "packages/${{ env.PACKAGE_NAME }}" '.packages = [$pkg]' lerna.json > lerna.tmp.json && mv lerna.tmp.json lerna.json - - echo "✅ lerna.json updated:" - cat lerna.json + echo "🔧 Preparing package ${{ env.PACKAGE_NAME }} for packaging" + # No lerna.json mutation required for `lerna exec --scope`.
269-281: Use dynamic package name in Slack success, avoid hardcoding “xrpl.js”.This message should reflect the selected package.
- MESSAGE="✅ Released xrpl.js v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." + MESSAGE="✅ Released ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully."
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
📚 Learning: 2024-09-26T21:14:56.813Z
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
Applied to files:
.github/workflows/release.yml
🔇 Additional comments (1)
.github/workflows/release.yml (1)
139-143: Artifact naming and consumption are correct. LGTM.Also applies to: 147-152, 244-249
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/nodejs.yml (1)
280-293: Docs deployment is effectively disabled due to conflicting conditionsThe job-level condition restricts this job to the main branch, but the deploy step only runs for version tags. As a result, the deploy step never runs in practice. Either deploy on main pushes (remove tag gate), or allow tag pushes at the job level.
Suggested fix to deploy only on non-beta version tags (keeps your tag gate and allows tag-triggered runs):
deploy-docs-pages: permissions: id-token: write # Needed for OIDC authentication pages: write # this permission is needed for deploying into Github Pages environment: name: github-pages - url: ${{ steps.docs-artifact.outputs.page_url }} + url: ${{ steps.deployment.outputs.page_url }} runs-on: ubuntu-latest needs: generate-documentation - # Deploy docs only pushes into the main branch - if: success() && github.ref == 'refs/heads/main' + # Allow deployment on tag pushes (non-beta version tags) + if: success() && startsWith(github.ref, 'refs/tags/') steps: - name: Detect (non-beta) version tag id: check-tag run: | if [[ ${{ github.event.ref }} =~ ^refs/tags/v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then echo "published_version_tag=true" >> $GITHUB_OUTPUT fi - - name: Deploy to Documentation to GitHub Pages - id: docs-artifact + - name: Deploy Documentation to GitHub Pages + id: deployment if: steps.check-tag.outputs.published_version_tag == 'true' uses: actions/deploy-pages@v4Alternative: if your intent is to always deploy docs from main (not tags), drop the “Detect tag” step and the conditional on the deploy step, and keep
if: success() && github.ref == 'refs/heads/main'.
🧹 Nitpick comments (4)
.github/workflows/nodejs.yml (4)
17-22: workflow_call input is fine; consider adding workflow_dispatch input for manual runsIf you want to manually run CI for an arbitrary ref via the Actions UI, mirror this input under
workflow_dispatch.inputs.Example:
on: push: branches: [main] tags: - '**' pull_request: workflow_dispatch: + inputs: + git_ref: + description: 'Git ref to checkout (branch, tag, or commit SHA)' + required: false + type: string workflow_call: inputs: git_ref: description: 'Git ref to checkout (branch, tag, or commit SHA)' required: true type: string
124-126: Pin Docker image for reproducibility and supply-chain hygiene
rippleci/rippled:developis a mutable tag. Consider pinning to a versioned tag or digest to avoid surprises across runs.Example:
- rippleci/rippled:1.12.0
- rippleci/rippled@sha256:
Also applies to: 183-186
47-59: Optional: run npm ci always; cache as optimization onlySkipping
npm cion cache hits can hide issues if the cache is stale or partial. Runningnpm ciunconditionally while leveraging the cache as a boost is more robust.Apply this pattern to the “Install Dependencies” step:
- - name: Install Dependencies - if: steps.cache-nodemodules.outputs.cache-hit != 'true' - run: npm ci + - name: Install Dependencies + run: npm ciAlso applies to: 90-102, 137-150, 191-204, 241-253
289-291: Nit: Clarify step id nameThe step id
docs-artifactsuggests an upload step, but here it’s the deployment step. Rename todeploymentfor clarity (reflected in the diff above).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/nodejs.yml(6 hunks)packages/xrpl/.eslintrc.js(0 hunks)packages/xrpl/package.json(2 hunks)packages/xrpl/snippets/README.md(0 hunks)packages/xrpl/snippets/src/claimPayChannel.ts(0 hunks)packages/xrpl/snippets/src/getTransaction.ts(0 hunks)packages/xrpl/snippets/src/multisigning.ts(0 hunks)packages/xrpl/snippets/src/partialPayment.ts(0 hunks)packages/xrpl/snippets/src/paths.ts(0 hunks)packages/xrpl/snippets/src/permissionedDEX.ts(0 hunks)packages/xrpl/snippets/src/reliableTransactionSubmission.ts(0 hunks)packages/xrpl/snippets/src/sendEscrow.ts(0 hunks)packages/xrpl/snippets/src/setRegularKey.ts(0 hunks)packages/xrpl/snippets/tsconfig.json(0 hunks)packages/xrpl/tsconfig.eslint.json(0 hunks)
💤 Files with no reviewable changes (13)
- packages/xrpl/snippets/src/paths.ts
- packages/xrpl/snippets/src/multisigning.ts
- packages/xrpl/snippets/src/getTransaction.ts
- packages/xrpl/snippets/src/permissionedDEX.ts
- packages/xrpl/tsconfig.eslint.json
- packages/xrpl/snippets/src/setRegularKey.ts
- packages/xrpl/snippets/tsconfig.json
- packages/xrpl/snippets/src/reliableTransactionSubmission.ts
- packages/xrpl/snippets/src/partialPayment.ts
- packages/xrpl/snippets/README.md
- packages/xrpl/snippets/src/claimPayChannel.ts
- packages/xrpl/.eslintrc.js
- packages/xrpl/snippets/src/sendEscrow.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
📚 Learning: 2024-09-26T21:14:56.813Z
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
Applied to files:
.github/workflows/nodejs.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: integration (20.x)
- GitHub Check: unit (20.x)
- GitHub Check: browser (22.x)
- GitHub Check: unit (22.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: integration (22.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/xrpl/package.json (1)
50-50: Verified: no dangling snippet references remain
I ran searches forbuild:snippets,compile:snippets,start:snippet,inspect:snippet, and anysnippets/srcpaths—none were found. The updated build script is safe to merge..github/workflows/nodejs.yml (2)
8-8: Correct use of GIT_REF from workflow inputsUsing
inputs.git_refwith a fallback togithub.refat workflow scope fixes the earlier reusable-workflow context issue and ensures consistent checkout ref across jobs.
34-36: LGTM: Consistent ref and full history on all checkoutsAll jobs now checkout
${{ env.GIT_REF }}withfetch-depth: 0, ensuring every job builds/tests the intended commit with tags/history available.Also applies to: 77-79, 120-122, 174-176, 227-229
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
.github/workflows/release.yml (4)
26-28: Concurrency key should include package/ref to avoid global serializationA static “release” group serializes all releases across packages/refs.
concurrency: - group: release + group: release-${{ github.event.inputs.package_name }}-${{ github.event.inputs.git_ref }} cancel-in-progress: true
127-129: Generate SBOM for the selected package (not the whole monorepo)Scope cyclonedx-npm to the target package to keep the SBOM relevant to the release artifact.
- - name: Generate CycloneDX SBOM - run: cyclonedx-npm --output-format json --output-file sbom.json + - name: Generate CycloneDX SBOM (scoped) + working-directory: packages/${{ env.PACKAGE_NAME }} + run: cyclonedx-npm --output-format json --output-file ../../sbom.jsonNote: No changes needed to the upload steps since sbom.json is still at repo root.
130-139: Make Trivy blocking on CRITICAL/HIGH findingsexit-code: 0 lets the pipeline proceed even with severe vulnerabilities. Fail fast to enforce remediation or explicit override.
- name: Scan SBOM for vulnerabilities using Trivy - uses: aquasecurity/[email protected] + uses: aquasecurity/[email protected] with: scan-type: sbom scan-ref: sbom.json format: table - exit-code: 0 + exit-code: 1 output: vuln-report.txt severity: CRITICAL,HIGHOptionally add severity-threshold or an allowlist if you need to tune blocking behavior.
294-306: Fix failure Slack copy: not always “Tests failed”; include package nameThis is the release stage; message should reflect release failure and the selected package.
- MESSAGE="❌ Tests failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Release pipeline failed for ${{ env.PACKAGE_NAME }} ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
🧹 Nitpick comments (5)
.github/workflows/release.yml (5)
3-7: Scope permissions to least privilege; drop unused scopes at top-levelOnly the release publish needs OIDC (id-token). Attestations/pages aren’t used here. Move id-token to the release job and minimize global permissions.
Apply:
permissions: - id-token: write - contents: read - attestations: write - pages: write + contents: read
110-123: Use the dynamic package name in Slack messageHardcoded “xrpl.js” will mislead when releasing other packages. Use PACKAGE_NAME.
- MESSAGE="❌ Build failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Build failed for ${{ env.PACKAGE_NAME }} ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
163-169: Fix step title typo; keep logs clean“choosen” → “chosen”. Minor copyedit.
- - name: Generate lerna.json for choosen the package + - name: Generate lerna.json for chosen package
231-235: Grant id-token only to the release job (needed for npm provenance)If you drop the global id-token as suggested, add it here so “npm publish --provenance” can mint an OIDC token.
release: runs-on: ubuntu-latest - permissions: - contents: write + permissions: + contents: write + id-token: write
280-293: Use dynamic package name in success Slack messageMake the message accurate for any package.
- MESSAGE="✅ Released xrpl.js v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." + MESSAGE="✅ Released ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully."
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
📚 Learning: 2024-09-26T21:14:56.813Z
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
Applied to files:
.github/workflows/release.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: integration (20.x)
- GitHub Check: browser (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: unit (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
.github/workflows/release.yml (3)
45-58: Good hardening on version extractionNice use of set -euo, existence checks, and jq -er to fail fast on bad input.
262-267: Node 20 for provenance: LGTMUsing Node 20 aligns with npm provenance requirements in publish steps.
240-242: Confirm the ‘manual-approval’ environment exists with required reviewersWithout the environment configured, this won’t gate releases. Verify repo settings or adjust the environment name.
shichengripple001
force-pushed
the
main
branch
from
0159ea5 to
26c0083
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
RELEASE.md (2)
35-38: Fix typo and clarify approval sentence in review stepSpelling/wording fix. Mirrors earlier suggestion.
-1. The pipelin will pause at the "Review test and security scan result" step, at least 1 approver is required to review and approve the release. +1. The pipeline will pause at the "Review test and security scan result" step; at least one approver is required to review and approve the release.
13-21: Document updating package-lock.json (run npm i) after changing versionPer prior feedback, call out running npm i so lockfiles are updated and will merge back cleanly.
1. Create a release branch and update the **`version`** field in `packages/<package_name>/package.json` to the intended release version. ``` { "name": "<package_name>", "version": "x.y.z" } ``` +2. From the repository root, run `npm i` so the corresponding `package-lock.json` reflects the new version (this ensures the lockfile in the release branch merges cleanly into `main`)..github/workflows/release.yml (4)
23-25: Scope concurrency per package/ref to avoid serializing all releasesCurrent static group forces all releases to queue behind each other.
concurrency: - group: release + group: release-${{ github.event.inputs.package_name }}-${{ github.event.inputs.git_ref }} cancel-in-progress: true
251-258: Enable generated release notes and set make_latest when not dry-runAlso reflects prior reviewer feedback.
- name: Create GitHub release uses: softprops/action-gh-release@v2 with: tag_name: "${{ env.PACKAGE_NAME }}@${{ env.PACKAGE_VERSION }}" name: "${{ env.PACKAGE_NAME }}@${{ env.PACKAGE_VERSION }}" draft: ${{ github.event.inputs.dry-run == 'true' }} prerelease: false + generate_release_notes: true + make_latest: ${{ github.event.inputs.dry-run != 'true' }}Optional: you could omit tag_name and let the action default to github.ref_name if you trigger the workflow on a tag, but with workflow_dispatch on arbitrary refs, keeping tag_name explicit is fine.
282-309: Slack messages: correct wording and include package name“Tests failed” from the release job is misleading. Include package name for clarity.
- - name: Notify Slack success + - name: Notify Slack success if: success() @@ - MESSAGE="✅ Released xrpl.js v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." + MESSAGE="✅ Released ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." @@ - - name: Notify Slack if tests fail + - name: Notify Slack on release failure if: failure() @@ - MESSAGE="❌ Tests failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Release pipeline failed for ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
128-139: Scope SBOM generation to the selected package (avoid whole monorepo)Generate the SBOM for the target package to reduce noise and improve signal.
- - name: Generate CycloneDX SBOM - run: cyclonedx-npm --output-format json --output-file sbom.json + - name: Generate CycloneDX SBOM (scoped to package) + working-directory: packages/${{ env.PACKAGE_NAME }} + run: cyclonedx-npm --output-format json --output-file ../../sbom.json @@ - - name: Scan SBOM for vulnerabilities using Trivy + - name: Scan SBOM for vulnerabilities using Trivy uses: aquasecurity/[email protected] with: scan-type: sbom - scan-ref: sbom.json + scan-ref: sbom.json format: table - exit-code: 0 + exit-code: 0 output: vuln-report.txt severity: CRITICAL,HIGHNote: scan-ref points to sbom.json in the current working directory. Since Trivy runs at repo root by default, either:
- add working-directory: . and set scan-ref: sbom.json, or
- keep as-is if the action resolves the file path at the runner CWD (validate).
If you prefer to keep SBOM at repo root (../../sbom.json), then set scan-ref: sbom.json with working-directory: . to match.
I can adjust the paths precisely based on your preference (store SBOM at root vs. package dir).
🧹 Nitpick comments (11)
RELEASE.md (7)
3-3: Use “set up” (verb form) instead of “setup”Minor grammar fix for clarity.
-A GitHub Actions workflow has been setup to automate building, scanning, packaging, and releasing npm packages in the `packages/` directory. +A GitHub Actions workflow has been set up to automate building, scanning, packaging, and releasing npm packages in the `packages/` directory.
14-19: Specify a language for fenced code blocks (markdownlint MD040)Add json to the code fence for better highlighting and to satisfy linters.
- ``` + ```json { "name": "<package_name>", "version": "x.y.z" } ```
50-53: Fix typos and pluralization in test descriptionCorrect “facucet_test” and “intgration”.
-### 2. **Run Tests** -- Triggers the facucet_test, nodejs workflow which will run unit test, intgration test, faucet test etc. -- Ensures the code at the given Git ref passes tests. +### 2. **Run Tests** +- Triggers the faucet_test and nodejs workflows to run unit, integration, and faucet tests. +- Ensures the code at the given Git ref passes all tests.
56-63: Fix typos in pre-release stepsMinor spelling fixes.
- Generates a CycloneDX SBOM (Software Bill of Materials). - Runs a security vulnerability scan with Trivy. -- Uploads SBOM to OWASP Dependency-Track for tracking the vulnerabilites. +- Uploads SBOM to OWASP Dependency-Track for tracking vulnerabilities. - Packages the module with lerna and uploads the tarball as an artifact. - Posts build failure notifications to Slack.
66-72: Fix “Genrate” and tighten phrasing for Review StageSpelling/wording polish.
-### 4. **Review Stage** -- Genrate a summary of: +### 4. **Review Stage** +- Generate a summary of: - Package name - Version - Vulnerability scan artifacts -- Requires the approvers to manually review security reports on the Actions page. +- Approvers must manually review security reports on the Actions page.
88-95: Add language to fenced blocks (markdownlint MD040)These are illustrative strings; use text for clarity.
-``` +```text <package_name>@<version>Example:
-+text
[email protected]
98-105: Fix heading spacing and clarify note formattingAdd missing space after ## and keep style consistent.
-##⚠️ **Important Notes** +## ⚠️ **Important Notes**Also, consider explicitly stating that vulnerability scanning is advisory-only by design (to align expectations with the workflow’s non-blocking Trivy configuration).
Would you like me to add a sentence like “Vulnerability scanning is advisory and does not block the release by design; reviewers are responsible for manual enforcement during the Review stage.”?
.github/workflows/release.yml (4)
66-72: Reduce job permissions (least privilege) for run_testsUnless the reusable nodejs workflow requires them, drop id-token and pages here.
run_tests: name: Run unit/integration tests ${{ needs.get_version.outputs.package_version }} permissions: contents: read - id-token: write - pages: writeIf nodejs.yml needs additional scopes, grant them inside that workflow or justify here.
111-124: Slack message: include package name and correct contextMessage currently says “Build failed for xrpl.js …” which is confusing in a monorepo.
- MESSAGE="❌ Build failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Build failed for ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
164-174: Fix step name typo (“choosen”) and remove extra blank linesMinor cleanup.
- - name: Generate lerna.json for choosen the package + - name: Generate lerna.json for chosen package run: | - - echo "🔧 Updating lerna.json to include only packages/${{ env.PACKAGE_NAME }}" + echo "🔧 Updating lerna.json to include only packages/${{ env.PACKAGE_NAME }}" # Use jq to update the packages field safely jq --arg pkg "packages/${{ env.PACKAGE_NAME }}" '.packages = [$pkg]' lerna.json > lerna.tmp.json && mv lerna.tmp.json lerna.json echo "✅ lerna.json updated:" cat lerna.json
269-278: Optional: avoid ls/globbing to pick tarballGiven a single-file artifact this is fine; if you later attach multiple files, capture the filename deterministically.
- PKG=$(ls *.tgz) + PKG="$(/usr/bin/find . -maxdepth 1 -type f -name '*.tgz' -printf '%f\n' | head -n1)"Not strictly necessary now; consider only if you expand artifact contents.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/faucet_test.yml(2 hunks).github/workflows/nodejs.yml(6 hunks).github/workflows/release.yml(1 hunks)RELEASE.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/faucet_test.yml
- .github/workflows/nodejs.yml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-09-26T21:14:56.813Z
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2024-12-06T19:25:15.376Z
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Applied to files:
.github/workflows/release.yml
🪛 LanguageTool
RELEASE.md
[grammar] ~1-~1: There might be a mistake here.
Context: # 🚀 Release Pipeline Guide A GitHub Actions workflow has been setup ...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...ackages/` directory. --- ## 🔑 Usage You can manually trigger the release work...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ... e.g., xrpl or ripple-address-codec. - git_ref: The Git branch, tag, or commi...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ... Field | Example | |---------------|-----------------------...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...--------------|------------------------| | package_name | xrpl ...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...package_name | xrpl | | git_ref | release/[email protected] ...
(QB_NEW_EN)
[grammar] ~37-~37: Ensure spelling is correct
Context: ...lease details and scan result** 1. The pipelin will pause at the "Review test and secu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~42-~42: There might be a mistake here.
Context: ...e. --- ## 🔨 How the Pipeline Works ### 1. Get Package Version - Extracts the...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ... packages/<package_name>/package.json. - No manual version input is required. --...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ... test, intgration test, faucet test etc. - Ensures the code at the given Git ref pa...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...elease Steps** - Builds the npm package. - Generates a CycloneDX SBOM (Software Bil...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ... and uploads the tarball as an artifact. - Posts build failure notifications to Sla...
(QB_NEW_EN)
[grammar] ~66-~66: Ensure spelling is correct
Context: ...ns to Slack. --- ### 4. Review Stage - Genrate a summary of: - Package name - Vers...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~67-~67: There might be a mistake here.
Context: ...Review Stage - Genrate a summary of: - Package name - Version - Vulnerabili...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...- Genrate a summary of: - Package name - Version - Vulnerability scan artifacts...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...summary of: - Package name - Version - Vulnerability scan artifacts - Requires ...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...Version - Vulnerability scan artifacts - Requires the approvers to manually revie...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ... (Setup through GitHub Environment). - Creates a GitHub Release with a tag like...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...tions to Slack. --- ## 📁 Tag Format The GitHub release and git tag are named ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
RELEASE.md
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
88-88: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
93-93: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
98-98: No space after hash on atx style heading
(MD018, no-missing-space-atx)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: integration (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: browser (22.x)
- GitHub Check: unit (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
.github/workflows/release.yml (2)
131-139: Confirm non-blocking Trivy exit code is intentionalexit-code: 0 allows releases with CRITICAL/HIGH findings. This aligns with RELEASE.md but weakens security posture.
If you want findings to block by default (and allow overrides), change to exit-code: 1 and optionally add a manual unblocking mechanism (e.g., environment approval or an allowlist). Should I draft that?
149-154: Ensure artifact path matches SBOM location after scoping changeIf you adopt the scoped SBOM suggestion, sbom.json lives at repo root. Current upload step already matches that path.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
RELEASE.md (1)
20-20: Fix typos and clarify lockfile instruction (addresses a prior review).Correct “releaes” to “release”, wrap the command in backticks, and call out package-lock.json explicitly.
Apply this diff:
-2. Run npm i to update the package-lock with the updated versions and commit the lock file to the releaes branch +2. Run `npm i` to update `package-lock.json` with the new version, and commit the lockfile to the release branch.
🧹 Nitpick comments (3)
RELEASE.md (3)
36-39: Approval gate wording is good; tiny grammar tweak.Use “one approver” for consistency with docs style.
Apply this diff:
-1. The pipeline will pause at the "Review test and security scan result" step, at least 1 approver is required to review and approve the release. +1. The pipeline will pause at the "Review test and security scan result" step; at least one approver is required to review and approve the release.
31-35: Consider avoiding “@” in branch names (git_ref example).Using “@” in branch names is legal but can be confusing in shells and with reflog syntax. Prefer a hyphen.
Apply this diff:
-| git_ref | release/[email protected] | +| git_ref | release/xrpl-4.3.8 |If workflows or tooling currently expect “@” in branch names, keep as-is; otherwise, switching to a hyphen reduces quoting issues.
76-82: Make the npm publish “dry-run” note more actionable and document required secrets.Docs should state how to flip to a real publish, and list required secrets/env for the pipeline (e.g., NPM_TOKEN, permissions, Slack webhook or app config, Dependency-Track credentials).
Proposed insert after “Usage” (add a new “Prerequisites” section):
+## ✅ Prerequisites + +Before running the Release Pipeline, ensure these secrets and settings are configured: +- GitHub Actions permissions: workflows must have permission to call `nodejs.yml` and `faucet_test.yml` via `workflow_call`. +- NPM publish: + - `NPM_TOKEN` secret with publish rights to the target scope/registry. + - `NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}` set during the publish step. +- Security/Reporting (if enabled in the workflow): + - `DEPENDENCY_TRACK_URL` and `DEPENDENCY_TRACK_API_KEY` for SBOM upload. + - `SLACK_WEBHOOK_URL` (or Slack App configuration) for notifications. + +To switch from a dry-run to a real publish, remove `--dry-run` from the npm publish step in `.github/workflows/release.yml` and ensure the environment requires approval.Optionally, add a short note in the Release Stage:
-- Publishes the package to the public npm registry. +- Publishes the package to the public npm registry (currently runs with `--dry-run`).I can open a follow-up PR to add this section and wire up the secrets if desired.
Also applies to: 105-106
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks)RELEASE.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/release.yml
🧰 Additional context used
🪛 LanguageTool
RELEASE.md
[grammar] ~1-~1: There might be a mistake here.
Context: # 🚀 Release Pipeline Guide A GitHub Actions workflow has been setup ...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...ackages/` directory. --- ## 🔑 Usage You can manually trigger the release work...
(QB_NEW_EN)
[grammar] ~20-~20: Ensure spelling is correct
Context: ...ersions and commit the lock file to the releaes branch ### Triggering a Release 1...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~20-~20: Ensure spelling is correct
Context: ...mmit the lock file to the releaes branch ### Triggering a Release 1. Go to **GitHu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~26-~26: There might be a mistake here.
Context: ... e.g., xrpl or ripple-address-codec. - git_ref: The Git branch, tag, or commi...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ... Field | Example | |---------------|-----------------------...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...--------------|------------------------| | package_name | xrpl ...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...package_name | xrpl | | git_ref | release/[email protected] ...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...e. --- ## 🔨 How the Pipeline Works ### 1. Get Package Version - Extracts the...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ... packages/<package_name>/package.json. - No manual version input is required. --...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ... test, intgration test, faucet test etc. - Ensures the code at the given Git ref pa...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...elease Steps** - Builds the npm package. - Generates a CycloneDX SBOM (Software Bil...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ... and uploads the tarball as an artifact. - Posts build failure notifications to Sla...
(QB_NEW_EN)
[grammar] ~67-~67: Ensure spelling is correct
Context: ...ns to Slack. --- ### 4. Review Stage - Genrate a summary of: - Package name - Vers...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~68-~68: There might be a mistake here.
Context: ...Review Stage - Genrate a summary of: - Package name - Version - Vulnerabili...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...- Genrate a summary of: - Package name - Version - Vulnerability scan artifacts...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...summary of: - Package name - Version - Vulnerability scan artifacts - Requires ...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...Version - Vulnerability scan artifacts - Requires the approvers to manually revie...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... (Setup through GitHub Environment). - Creates a GitHub Release with a tag like...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...tions to Slack. --- ## 📁 Tag Format The GitHub release and git tag are named ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
RELEASE.md
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
89-89: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
94-94: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
99-99: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🔇 Additional comments (2)
RELEASE.md (2)
43-55: “How the Pipeline Works” section reads clearly and matches the workflow intent.Good structure and flow from version discovery to tests.
36-39: No discrepancies found: RELEASE.md is in sync with the workflows.
- .github/workflows/release.yml defines a step named “Review test and security scan result” (line 194).
- Both faucet_test.yml and nodejs.yml expose a
workflow_callinputgit_ref.- All
actions/checkout@…steps in release.yml, faucet_test.yml, and nodejs.yml use the providedgit_ref(orenv.GIT_REF) andfetch-depth: 0.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
.github/workflows/release.yml (4)
23-25: Scope concurrency by package/ref to prevent unnecessary queuingUsing a static group serializes all releases. Allow parallel runs for different package_name/git_ref pairs.
Apply:
concurrency: - group: release + group: release-${{ github.event.inputs.package_name }}-${{ github.event.inputs.git_ref }} cancel-in-progress: true
128-130: Generate SBOM for the selected package, not the whole monorepoRunning cyclonedx-npm at repo root includes all workspaces.
Apply:
- - name: Generate CycloneDX SBOM - run: cyclonedx-npm --output-format json --output-file sbom.json + - name: Generate CycloneDX SBOM (scoped) + working-directory: packages/${{ env.PACKAGE_NAME }} + run: cyclonedx-npm --output-format json --output-file ../../sbom.jsonOptional hardening: pin a version of cyclonedx-npm for reproducibility:
- - name: Install cyclonedx-npm - run: npm install -g @cyclonedx/cyclonedx-npm + - name: Install cyclonedx-npm + run: npm install -g @cyclonedx/cyclonedx-npm@^5
319-331: Fix Slack failure message: it’s a release failure, not testsThe release job runs after tests have passed. Update the wording to avoid confusion.
Apply:
- MESSAGE="❌ Tests failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Release pipeline failed for xrpl.js v${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
131-140: Make vulnerability scanning actionable: fail on CRITICAL/HIGHexit-code: 0 lets the pipeline proceed even on serious findings, undermining the review gate.
Apply:
- name: Scan SBOM for vulnerabilities using Trivy uses: aquasecurity/[email protected] with: scan-type: sbom scan-ref: sbom.json format: table - exit-code: 0 + exit-code: 1 output: vuln-report.txt severity: CRITICAL,HIGHIf you want soft-fail for non-blocking severities, use severity-threshold or an allowlist; otherwise this change blocks on CRITICAL/HIGH as intended.
🧹 Nitpick comments (4)
.github/workflows/release.yml (4)
99-110: Build only the targeted workspace to speed up and reduce surface areaAt repo root, npm ci/build can process all workspaces. For a single-package release, scope to the target package.
Apply:
- - name: Build package + - name: Build package (scoped) run: | # dubugging info npm --version node --version ls -l pwd - #build - npm ci - npm run build + # Build only the selected workspace + npm ci + npm run -w "packages/${{ env.PACKAGE_NAME }}" buildIf the repo relies on a top-level build that orchestrates all packages, keep current behavior; otherwise this reduces time and CI cost.
141-147: Check OWASP upload response and fail on errorsCurrently the curl result is not validated. Add a status check to avoid silent failures.
Apply:
- - name: Upload sbom to OWASP - run: | - curl -X POST \ - -H "X-Api-Key: ${{ secrets.OWASP_TOKEN }}" \ - -F "project=7c40c8ea-ea0f-4a5f-9b9f-368e53232397" \ - -F "[email protected]" \ - https://owasp-dt-api.prod.ripplex.io/api/v1/bom + - name: Upload sbom to OWASP + run: | + STATUS=$(curl -sS -w "%{http_code}" -o /tmp/owasp_resp.json \ + -X POST \ + -H "X-Api-Key: ${{ secrets.OWASP_TOKEN }}" \ + -F "project=7c40c8ea-ea0f-4a5f-9b9f-368e53232397" \ + -F "[email protected]" \ + https://owasp-dt-api.prod.ripplex.io/api/v1/bom) + echo "OWASP response ($STATUS):" + cat /tmp/owasp_resp.json + if [[ "$STATUS" -lt 200 || "$STATUS" -ge 300 ]]; then + echo "OWASP upload failed with status $STATUS" >&2 + exit 1 + fi
164-175: Fix typos and clarify step outputMinor wording and grammar tweaks for readability.
Apply:
- - name: Generate lerna.json for choosen the package + - name: Generate lerna.json for chosen package run: | - echo "🔧 Updating lerna.json to include only packages/${{ env.PACKAGE_NAME }}" + echo "🔧 Updating lerna.json to include only packages/${{ env.PACKAGE_NAME }}" # Use jq to update the packages field safely jq --arg pkg "packages/${{ env.PACKAGE_NAME }}" '.packages = [$pkg]' lerna.json > lerna.tmp.json && mv lerna.tmp.json lerna.json - echo "✅ lerna.json updated:" + echo "✅ lerna.json updated to target package:" cat lerna.json
262-267: Archive source from the release tag for reproducibilityUsing HEAD can drift from the tag if the branch advances. Archive the computed tag.
Apply:
- name: Package source code run: | mkdir -p source_code - git archive --format=zip -o "source_code/xrpl.js-${{ env.PACKAGE_NAME }}-${{ env.PACKAGE_VERSION }}.zip" HEAD - git archive --format=tar.gz -o "source_code/xrpl.js-${{ env.PACKAGE_NAME }}-${{ env.PACKAGE_VERSION }}.tar.gz" HEAD + TAG="${{ env.PACKAGE_NAME }}@${{ env.PACKAGE_VERSION }}" + git archive --format=zip -o "source_code/xrpl.js-${{ env.PACKAGE_NAME }}-${{ env.PACKAGE_VERSION }}.zip" "$TAG" + git archive --format=tar.gz -o "source_code/xrpl.js-${{ env.PACKAGE_NAME }}-${{ env.PACKAGE_VERSION }}.tar.gz" "$TAG"Note: This assumes the tag exists (i.e., not in dry-run per prior comment).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-09-26T21:14:56.813Z
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2024-12-06T19:25:15.376Z
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Applied to files:
.github/workflows/release.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: integration (20.x)
- GitHub Check: unit (22.x)
- GitHub Check: browser (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
.github/workflows/release.yml (2)
269-280: Dry-run behavior matches doc: draft release, not prerelease, and make_latest falseGood alignment with earlier feedback: draft toggled by dry-run, generate_release_notes enabled, make_latest only for real releases.
68-72: Permissions required for reusable nodejs workflow
The called.github/workflows/nodejs.ymlworkflow includes adeploy-docs-pagesjob that uses both
- actions/upload-pages-artifact@v3 and actions/deploy-pages@v4
- OIDC authentication
These steps require
pages: writeandid-token: writeto be granted by the parent workflow. No changes to the permissions block are needed.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
.github/workflows/release.yml (5)
23-25: Scope concurrency by package/ref to avoid unnecessary serializationSingle “release” group forces all releases to queue behind each other. Allow parallel releases across different packages/refs.
concurrency: - group: release + group: release-${{ github.event.inputs.package_name }}-${{ github.event.inputs.git_ref }} cancel-in-progress: true
128-130: SBOM should target the selected package, not the whole monorepoRunning cyclonedx-npm at repo root emits a workspace-wide SBOM. Scope to the package under release to keep scanning & reporting actionable.
- - name: Generate CycloneDX SBOM - run: cyclonedx-npm --output-format json --output-file sbom.json + - name: Generate CycloneDX SBOM (scoped) + working-directory: packages/${{ env.PACKAGE_NAME }} + run: cyclonedx-npm --output-format json --output-file ../../sbom.jsonNo changes needed for later steps if sbom.json remains at repo root.
131-139: Make vulnerability scanning enforceable (fail on findings)exit-code: 0 lets the pipeline proceed despite CRITICAL/HIGH issues. Pinning the action is good; now enforce failure on serious findings.
- name: Scan SBOM for vulnerabilities using Trivy uses: aquasecurity/[email protected] with: scan-type: sbom scan-ref: sbom.json format: table - exit-code: 0 + exit-code: 1 output: vuln-report.txt severity: CRITICAL,HIGHOptionally add a allowlist config if you need to suppress known false positives.
251-261: Do not create/push tags during dry-runDry-run should be non-mutating. Skip tag creation/push when dry-run is true.
- - name: Ensure Git tag exists + - name: Ensure Git tag exists + if: ${{ github.event.inputs.dry-run != 'true' }} id: create_tag run: | TAG="${{ env.PACKAGE_NAME }}@${{ env.PACKAGE_VERSION }}" if git rev-parse "$TAG" >/dev/null 2>&1; then echo "Tag $TAG already exists" else echo "Creating tag $TAG" git tag "$TAG" git push origin "$TAG" fi
319-325: Update failure notification wording to match stageThis step runs in the release job, not tests; message should reflect a release failure.
- MESSAGE="❌ Tests failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Release pipeline failed for xrpl.js v${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
🧹 Nitpick comments (6)
.github/workflows/release.yml (6)
68-72: Tighten job permissions for tests (least privilege)id-token and pages are likely unnecessary for running tests via the reusable workflow. Drop unless the called workflow requires them.
run_tests: name: Run unit/integration tests ${{ needs.get_version.outputs.package_version }} - permissions: - contents: read - id-token: write - pages: write + permissions: + contents: readIf nodejs.yml needs additional scopes, prefer granting them inside that workflow/job rather than here.
93-98: Normalize registry URL formatUse a trailing slash to match npm’s canonical registry URL (you already do this later in the release job).
- name: Set up Node.js uses: actions/setup-node@v4 with: node-version: 20 - registry-url: 'https://registry.npmjs.org' + registry-url: 'https://registry.npmjs.org/'
99-106: Typo: “dubugging” → “debugging”Minor polish for log clarity.
- # dubugging info + # debugging info
141-148: Avoid uploading SBOM to OWASP during dry-runDry-run shouldn’t mutate external systems. Skip the OWASP upload when dry-run is true.
- - name: Upload sbom to OWASP + - name: Upload sbom to OWASP + if: ${{ github.event.inputs.dry-run != 'true' }} run: | curl -X POST \ -H "X-Api-Key: ${{ secrets.OWASP_TOKEN }}" \ -F "project=7c40c8ea-ea0f-4a5f-9b9f-368e53232397" \ -F "[email protected]" \ https://owasp-dt-api.prod.ripplex.io/api/v1/bom
164-174: Fix wording and reconsider mutating lerna.json
- Minor: “choosen” → “chosen”
- Consider avoiding editing lerna.json in CI; you already scope pack via lerna exec. This mutation isn’t strictly required and can introduce surprises.
Minimal wording fix:
- - name: Generate lerna.json for choosen the package + - name: Generate lerna.json for chosen packageOptional: remove this step entirely and rely solely on the scoped pack command, unless another downstream step depends on this change.
205-231: Also emit a concise Step Summary for reviewersThe artifact link is helpful; add a Step Summary to surface key details in the run UI.
echo "🔗 Please review Vulnerabilities detected: https://github.com/$REPO/actions/runs/${{ github.run_id }}/artifacts/$ARTIFACT_ID" + + { + echo "### Release Review" + echo "" + echo "- Package: $PACKAGE_NAME" + echo "- Version: $PACKAGE_VERSION" + echo "- Git ref: $GIT_REF" + echo "- Commit: $COMMIT_SHA" + echo "- Vulnerability report: https://github.com/$REPO/actions/runs/${{ github.run_id }}/artifacts/$ARTIFACT_ID" + } >> "$GITHUB_STEP_SUMMARY"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-09-26T21:14:56.813Z
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2024-12-06T19:25:15.376Z
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Applied to files:
.github/workflows/release.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: browser (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: integration (22.x)
- GitHub Check: unit (22.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
.github/workflows/release.yml (3)
39-56: Robust version extraction with clear failure modes — niceGood use of set -euo pipefail, existence checks, jq -er, and exporting both env and outputs. This will fail fast on bad inputs.
175-184: Deterministic tarball capture — looks goodCleaning stale tarballs and capturing npm pack’s JSON output keeps things reproducible. Nice.
269-280: Release creation settings look consistentUsing draft for dry-run, generate_release_notes enabled, and make_latest only when not dry-run is coherent.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
RELEASE.md (1)
93-96: Add language to fenced code block (markdownlint MD040).-``` +```text [email protected]</blockquote></details> <details> <summary>.github/workflows/release.yml (4)</summary><blockquote> `23-25`: **Scope concurrency to package/branch to avoid serializing all releases.** Using a static group “release” forces all runs to queue. Include the package and branch so unrelated releases can run in parallel while still deduping the same target. ```diff concurrency: - group: release + group: release-${{ github.event.inputs.package_name }}-${{ github.event.inputs.release_branch }} cancel-in-progress: true
145-147: Scope SBOM generation to the selected package (avoid monorepo noise).- - name: Generate CycloneDX SBOM - run: cyclonedx-npm --output-format json --output-file sbom.json + - name: Generate CycloneDX SBOM (scoped) + working-directory: packages/${{ env.PACKAGE_NAME }} + run: cyclonedx-npm --output-format json --output-file ../../sbom.jsonThis keeps sbom.json at repo root so subsequent steps still find it.
148-157: Make Trivy findings actionable: fail on CRITICAL/HIGH.Currently
exit-code: 0allows releases to proceed despite severe findings. Consider failing here, or gate via an input if you want a soft-block.- name: Scan SBOM for vulnerabilities using Trivy uses: aquasecurity/[email protected] with: scan-type: sbom scan-ref: sbom.json format: table - exit-code: 0 + exit-code: 1 output: vuln-report.txt severity: CRITICAL,HIGHIf you prefer non-blocking by default, add a boolean input (e.g., block_on_vulns) and toggle
exit-codeaccordingly.
269-284: Avoid creating/pushing tags during dry-run runs.Dry-run should not mutate the repo. Skip tag creation when dry-run is true.
- - name: Ensure Git tag exists + - name: Ensure Git tag exists + if: ${{ github.event.inputs.dry-run != 'true' }} id: create_tag run: | TAG="${{ env.PACKAGE_NAME }}@${{ env.PACKAGE_VERSION }}" - if [ "${{ github.event.inputs.dry-run }}" == "true" ]; then - TAG="draft-${TAG}" - fi - if git rev-parse "$TAG" >/dev/null 2>&1; then echo "Tag $TAG already exists" else echo "Creating tag $TAG" git tag "$TAG" git push origin "$TAG" fiAlternatively, skip the “Create GitHub release” step entirely for dry-run to keep the repository untouched.
🧹 Nitpick comments (11)
RELEASE.md (4)
31-35: Keep the examples table consistent with the input name.Update the second row key to match release_branch.
Apply:
-| git_ref | release/[email protected] | +| release_branch| release/[email protected] |
52-53: Use consistent terminology (“release branch” instead of “Git ref”).Minor wording change for consistency with the workflow input.
-- Ensures the code at the given Git ref passes all tests. +- Ensures the code at the given release branch passes all tests.
63-64: Remove stray period.-- Posts failure notifications to Slack.. +- Posts failure notifications to Slack.
77-81: Clarify that publish is a dry-run by default.Docs say it “Publishes the package,” but the workflow defaults to dry-run.
-- Publishes the package to the public npm registry. +- Publishes the package to the public npm registry (dry-run by default; see Important Notes)..github/workflows/release.yml (7)
40-55: Harden the “artifactory” check and avoid HEAD^ edge cases.If the release branch has only one commit,
HEAD^may not exist. Also, checking just the last commit can miss prior additions. Search the working tree instead, excluding lockfiles.- - name: Check if release branch exists + - name: Check if release branch exists run: | if git ls-remote --exit-code origin "refs/heads/${{ github.event.inputs.release_branch }}" > /dev/null; then echo "✅ Found release branch: ${{ github.event.inputs.release_branch }}" else echo "❌ Release branch ${{ github.event.inputs.release_branch }} not found in remote. Failing workflow." exit 1 fi - echo "🔍 Checking committed changes for 'artifactory.ops.ripple.com'..." - - # Look at the diff between HEAD and its parent - if git diff HEAD^ HEAD | grep -q "artifactory.ops.ripple.com"; then - echo "❌ artifactory.ops.ripple.com found in committed changes!" - exit 1 - fi + echo "🔍 Scanning tree for 'artifactory.ops.ripple.com' (excluding lockfiles)..." + if git grep -nI --fixed-strings "artifactory.ops.ripple.com" -- \ + ':!**/package-lock.json' ':!**/pnpm-lock.yaml' ':!**/yarn.lock' >/dev/null; then + echo "❌ Found 'artifactory.ops.ripple.com' in source tree!" + git grep -nI --fixed-strings "artifactory.ops.ripple.com" -- \ + ':!**/package-lock.json' ':!**/pnpm-lock.yaml' ':!**/yarn.lock' || true + exit 1 + fi
83-89: Drop unnecessary job permissions on run_tests (least privilege).
run_testsdoesn’t needid-token: writeorpages: write. Keep it minimal.run_tests: name: Run unit/integration tests ${{ needs.get_version.outputs.package_version }} - permissions: - contents: read - id-token: write - pages: write + permissions: + contents: read
128-141: Rename step and include package name in Slack message.It’s not “tests” here and the message should identify the package.
- - name: Notify Slack if tests fail + - name: Notify Slack on failure if: failure() env: SLACK_TOKEN: ${{ secrets.SLACK_TOKEN }} run: | - MESSAGE="❌ Build failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Pre-release failed for ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" curl -X POST https://slack.com/api/chat.postMessage \
181-191: Fix grammar in step name and tighten logging.- - name: Generate lerna.json for choosen the package + - name: Generate lerna.json for chosen packageOptional: also redirect the
cat lerna.jsonto::group::/::endgroup::for cleaner logs.
221-247: Minor typo in summary output (“Release Branc”).Also consider surfacing severity counts from vuln-report for faster triage.
- echo "🌿 Release Branc: $RELEASE_BRANCH" + echo "🌿 Release Branch: $RELEASE_BRANCH"If desired, I can add a small parser step to extract CRITICAL/HIGH counts from
vuln-report.txt.
285-293: Attach the built tarball to the GitHub Release.Add the tarball so consumers can download it directly from the release page.
- name: Create GitHub release uses: softprops/action-gh-release@v2 with: tag_name: "${{ env.PACKAGE_NAME }}@${{ env.PACKAGE_VERSION }}" name: "${{ env.PACKAGE_NAME }}@${{ env.PACKAGE_VERSION }}" draft: ${{ github.event.inputs.dry-run == 'true' }} generate_release_notes: true make_latest: ${{ github.event.inputs.dry-run != 'true' }} + files: | + dist/*.tgzNote: If you keep dry-run mode, consider skipping this step to avoid creating draft releases/tags.
318-345: Use package name in Slack messages and correct failure wording.- MESSAGE="✅ Released xrpl.js v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." + MESSAGE="✅ Released ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." @@ - - name: Notify Slack if tests fail + - name: Notify Slack on failure @@ - MESSAGE="❌ Tests failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Release failed for ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks)RELEASE.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-09-26T21:14:56.813Z
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2024-12-06T19:25:15.376Z
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Applied to files:
.github/workflows/release.yml
🪛 LanguageTool
RELEASE.md
[grammar] ~1-~1: There might be a mistake here.
Context: # 🚀 Release Pipeline Guide A GitHub Actions workflow has been set up...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...ackages/` directory. --- ## 🔑 Usage You can manually trigger the release work...
(QB_NEW_EN)
[grammar] ~20-~20: Ensure spelling is correct
Context: ...ersions and commit the lock file to the releaes branch ### Triggering a Release 1...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~20-~20: Ensure spelling is correct
Context: ...mmit the lock file to the releaes branch ### Triggering a Release 1. Go to **GitHu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~26-~26: There might be a mistake here.
Context: ... e.g., xrpl or ripple-address-codec. - git_ref: The Git branch, tag, or commi...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ... Field | Example | |---------------|-----------------------...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...--------------|------------------------| | package_name | xrpl ...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...package_name | xrpl | | git_ref | release/[email protected] ...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...e. --- ## 🔨 How the Pipeline Works ### 1. Get Package Version - Extracts the...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ... packages/<package_name>/package.json. - No manual version input is required. --...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...cet tests against the specified Git ref. - Ensures the code at the given Git ref pa...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...elease Steps** - Builds the npm package. - Generates a CycloneDX SBOM (Software Bil...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...Review Stage* - Generate a summary of: - Package name - Version - Vulnerabili...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ... Generate a summary of: - Package name - Version - Vulnerability scan artifacts...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...summary of: - Package name - Version - Vulnerability scan artifacts - Requires ...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...Version - Vulnerability scan artifacts - Requires the approvers to manually revie...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...(Set up through GitHub Environment). - Creates a GitHub Release with a tag like...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...tions to Slack. --- ## 📁 Tag Format The GitHub release and git tag are named ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
RELEASE.md
94-94: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: unit (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: integration (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: browser (22.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
.github/workflows/release.yml (2)
110-115: LGTM: Using Node 20 aligns with npm provenance requirements.
192-199: LGTM: Deterministic tarball capture via npm pack --json.Capturing the filename from
npm pack --jsonavoids ambiguous globs and is reproducible.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…nts/for dry-run, create tag with prefix of draft
shichengripple001
force-pushed
the
main
branch
from
91d6781 to
6758012
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
.github/workflows/faucet_test.yml (1)
26-29: Good fix: checkout now honors the caller’s refUsing
${{ inputs.git_ref || github.ref }}prevents silent fallback to the default branch and makes this workflow reusable-safe. Looks solid.RELEASE.md (3)
93-96: Add language to fenced code block (markdownlint MD040)-``` +```text [email protected]--- `20-20`: **Fix typo and clarify lockfile instruction** Typo (“releaes”) and missing code formatting. Clarify that package-lock.json should be committed. ```diff -2. Run npm i to update the package-lock with the updated versions and commit the lock file to the releaes branch +2. Run `npm i` to update `package-lock.json` with the updated versions, and commit `package-lock.json` to the release branch.
26-35: Docs/workflow mismatch: use release_branch (not git_ref)The workflow input is named
release_branch. Update the field and table to prevent confusion for releasers.- - **git_ref:** The Git branch, tag, or commit SHA to release from. + - **release_branch:** The Git branch to release from (e.g., `release/[email protected]`). @@ -| git_ref | release/[email protected] | +| release_branch| release/[email protected] |.github/workflows/release.yml (4)
23-25: Scope concurrency per package+branch to avoid unnecessary queuingA single “release” group serializes all releases. Allow parallel releases across different packages/branches while still deduping per package/branch.
concurrency: - group: release + group: release-${{ github.event.inputs.package_name }}-${{ github.event.inputs.release_branch }} cancel-in-progress: true
145-147: Scope SBOM generation to the target packageRunning cyclonedx-npm at repo root includes all workspaces. Generate for the selected package to reduce noise and speed up scans.
- - name: Generate CycloneDX SBOM - run: cyclonedx-npm --output-format json --output-file sbom.json + - name: Generate CycloneDX SBOM (scoped) + working-directory: packages/${{ env.PACKAGE_NAME }} + run: cyclonedx-npm --output-format json --output-file ../../sbom.json
148-157: Make vulnerability scan actionable: fail on findingsWith
exit-code: 0, CRITICAL/HIGH issues won’t block releases. Set a non-zero exit to force remediation or explicit override.- name: Scan SBOM for vulnerabilities using Trivy uses: aquasecurity/[email protected] with: scan-type: sbom scan-ref: sbom.json format: table - exit-code: 0 + exit-code: 1 output: vuln-report.txt severity: CRITICAL,HIGH
269-284: Dry-run should not mutate the repo (don’t create/push tags)Currently even dry-run creates and pushes a tag (prefixed with “draft-”). Avoid repository mutation during dry-run.
- - name: Ensure Git tag exists + - name: Ensure Git tag exists + if: ${{ github.event.inputs.dry-run != 'true' }} id: create_tag run: | TAG="${{ env.PACKAGE_NAME }}@${{ env.PACKAGE_VERSION }}" - if [ "${{ github.event.inputs.dry-run }}" == "true" ]; then - TAG="draft-${TAG}" - fi - if git rev-parse "$TAG" >/dev/null 2>&1; then echo "Tag $TAG already exists" else echo "Creating tag $TAG" git tag "$TAG" git push origin "$TAG" fiOptionally, also skip the “Create GitHub release” step on dry-run, or keep it as a draft without tag creation by removing tag_name and letting it default to the run’s ref (still non-mutating).
🧹 Nitpick comments (12)
.github/workflows/faucet_test.yml (2)
31-55: Prefer setup-node’s built-in caching over node_modules cacheCaching node_modules at multiple depths can be brittle in a multi-package repo and risks stale transitive deps. setup-node@v4 offers “cache: npm”, which is simpler and safer (caches the npm cache, not node_modules) while keeping installs fast.
Suggested simplification:
- - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v4 - with: - node-version: ${{ matrix.node-version }} + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node-version }} + cache: npm + cache-dependency-path: '**/package-lock.json' @@ - - name: Cache node modules - id: cache-nodemodules - uses: actions/cache@v4 - env: - cache-name: cache-node-modules - with: - path: | - node_modules - */*/node_modules - key: ${{ runner.os }}-deps-${{ matrix.node-version }}-${{ hashFiles('**/package-lock.json') }} - restore-keys: | - ${{ runner.os }}-deps-${{ matrix.node-version }}- - - - name: Install Dependencies - if: steps.cache-nodemodules.outputs.cache-hit != 'true' - run: npm ci + - name: Install dependencies + run: npm ci
23-24: Consider aligning Node to 20.x (consistency with release jobs)Your release workflow publishes with Node 20. Running tests on the same major version avoids version skew (especially around npm 10 and OpenSSL behavior). Optional but reduces surprises.
Also applies to: 31-33
RELEASE.md (3)
36-39: Pluralize “result”Minor grammar improvement.
-### **Reviewing the release details and scan result** +### **Reviewing the release details and scan results**
63-64: Fix double period and tighten phrasing-- Posts failure notifications to Slack.. +- Posts failure notifications to Slack.
105-105: Format command as code-- The final release step performs an npm publish --dry-run. We can remove --dry-run when ready for production release. +- The final release step performs an `npm publish --dry-run`. We can remove `--dry-run` when ready for production releases..github/workflows/release.yml (7)
35-46: Branch existence check runs after checkout (redundant/fails late)If the branch doesn’t exist, actions/checkout will already fail. Either (a) move the “Check if release branch exists” step before checkout or (b) drop it. If you keep it, run it first for a clearer error.
Example reordering:
- - name: Checkout code - uses: actions/checkout@v4 - with: - ref: ${{ github.event.inputs.release_branch }} - fetch-depth: 0 - - name: Check if release branch exists + - name: Check if release branch exists run: | if git ls-remote --exit-code origin "refs/heads/${{ github.event.inputs.release_branch }}" > /dev/null; then echo "✅ Found release branch: ${{ github.event.inputs.release_branch }}" else echo "❌ Release branch ${{ github.event.inputs.release_branch }} not found in remote. Failing workflow." exit 1 fi + - name: Checkout code + uses: actions/checkout@v4 + with: + ref: ${{ github.event.inputs.release_branch }} + fetch-depth: 0
85-89: Unnecessary permissions for reusable tests workflowThis job doesn’t need id-token or pages. Reduce scope per least-privilege.
permissions: - contents: read - id-token: write - pages: write + contents: read
116-123: Nit: “debugging” spelling and remove noisy directory listingsSmall clean-up; keeps logs focused.
- # dubugging info - npm --version - node --version - ls -l - pwd + # debugging info + npm --version + node --version
181-189: Nit: fix step title spelling- - name: Generate lerna.json for choosen the package + - name: Generate lerna.json for chosen package
245-246: Nit: typo in output- echo "🌿 Release Branc: $RELEASE_BRANCH" + echo "🌿 Release Branch: $RELEASE_BRANCH"
285-293: Release metadata: consider omitting tag_name on dry-runIf you adopt the non-mutating dry-run above, you can skip creating a release or create a draft against the run ref. Example to skip entirely:
- - name: Create GitHub release + - name: Create GitHub release + if: ${{ github.event.inputs.dry-run != 'true' }}Alternatively, keep as-is when not in dry-run.
318-337: Parameterize Slack messages with package name and correct wordingMessages hardcode “xrpl.js” and one step says “tests fail” in the release job.
- MESSAGE="✅ Released xrpl.js v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." + MESSAGE="✅ Released ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." @@ - - name: Notify Slack if tests fail + - name: Notify Slack on release failure @@ - MESSAGE="❌ Tests failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Release failed for ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/faucet_test.yml(2 hunks).github/workflows/nodejs.yml(6 hunks).github/workflows/release.yml(1 hunks)RELEASE.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/nodejs.yml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-09-26T21:14:56.813Z
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
Applied to files:
.github/workflows/release.yml.github/workflows/faucet_test.yml
📚 Learning: 2024-12-06T19:25:15.376Z
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Applied to files:
.github/workflows/release.yml
🪛 LanguageTool
RELEASE.md
[grammar] ~1-~1: There might be a mistake here.
Context: # 🚀 Release Pipeline Guide A GitHub Actions workflow has been set up...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...ackages/` directory. --- ## 🔑 Usage You can manually trigger the release work...
(QB_NEW_EN)
[grammar] ~20-~20: Ensure spelling is correct
Context: ...ersions and commit the lock file to the releaes branch ### Triggering a Release 1...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~20-~20: Ensure spelling is correct
Context: ...mmit the lock file to the releaes branch ### Triggering a Release 1. Go to **GitHu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~26-~26: There might be a mistake here.
Context: ... e.g., xrpl or ripple-address-codec. - git_ref: The Git branch, tag, or commi...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ... Field | Example | |---------------|-----------------------...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...--------------|------------------------| | package_name | xrpl ...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...package_name | xrpl | | git_ref | release/[email protected] ...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...e. --- ## 🔨 How the Pipeline Works ### 1. Get Package Version - Extracts the...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ... packages/<package_name>/package.json. - No manual version input is required. --...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...cet tests against the specified Git ref. - Ensures the code at the given Git ref pa...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...elease Steps** - Builds the npm package. - Generates a CycloneDX SBOM (Software Bil...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...Review Stage* - Generate a summary of: - Package name - Version - Vulnerabili...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ... Generate a summary of: - Package name - Version - Vulnerability scan artifacts...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...summary of: - Package name - Version - Vulnerability scan artifacts - Requires ...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...Version - Vulnerability scan artifacts - Requires the approvers to manually revie...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...(Set up through GitHub Environment). - Creates a GitHub Release with a tag like...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...tions to Slack. --- ## 📁 Tag Format The GitHub release and git tag are named ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
RELEASE.md
94-94: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: integration (20.x)
- GitHub Check: browser (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: integration (22.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: unit (22.x)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
RELEASE.md (2)
20-20: Fix typo, clarify command, and name the lockfile explicitly.Use inline code for commands, correct “releaes,” and call out package-lock.json.
-2. Run npm i to update the package-lock with the updated versions and commit the lock file to the releaes branch +2. Run `npm i` to update `package-lock.json` with the updated versions, and commit the lock file to the release branch.
31-35: Docs/workflow mismatch: table still references git_ref.Inputs section above uses release_branch; the table must match to avoid operator error.
-| Field | Example | -|---------------|------------------------| -| package_name | xrpl | -| git_ref | release/[email protected] | +| Field | Example | +|----------------|------------------------| +| package_name | xrpl | +| release_branch | release/[email protected] |
🧹 Nitpick comments (7)
RELEASE.md (7)
52-53: Use consistent terminology: “release branch” instead of “Git ref.”The workflow input is release_branch; mirror that wording here.
-- Triggers the `faucet_test.yml` and `nodejs.yml` workflows to run unit, integration, and faucet tests against the specified Git ref. -- Ensures the code at the given Git ref passes all tests. +- Triggers the `faucet_test.yml` and `nodejs.yml` workflows to run unit, integration, and faucet tests against the specified release branch. +- Ensures the code on the specified release branch passes all tests.
63-64: Punctuation nit: remove double period and tighten phrasing.-- Posts failure notifications to Slack.. +- Posts failure notifications to Slack.
76-82: Reflect dry-run behavior in Release Stage to avoid confusion.Important Notes (Line 105) say publish runs with --dry-run; mention it here too.
- Downloads the built package tarball. -- Publishes the package to the public npm registry. +- Publishes the package to the public npm registry (currently `npm publish --dry-run`). - Posts success or failure notifications to Slack.
89-96: Add language to fenced code block (markdownlint MD040) and keep both examples uniform.```text <package_name>@<version>-
+text
[email protected]
7-13: Add a short “Prerequisites” section (secrets, permissions, environments).Readers need to know what must exist before running the workflow (NPM token, Slack, Dependency-Track, environment approvers). Suggest inserting right before “Before triggering a release.”
## 🔑 **Usage** You can manually trigger the release workflow from the [GitHub Actions UI](https://github.com/xrplf/xrpl.js/actions/workflows/release.yml). +### 🧰 Prerequisites +- Repository or organization secrets: + - `NPM_TOKEN` with publish permissions for the scoped packages. + - `SLACK_WEBHOOK_URL` (or equivalent) for notifications. + - `DT_API_KEY` and `DT_BASE_URL` for OWASP Dependency-Track (if enabled). +- A GitHub Environment configured for the Release stage with required approvers. +- Permissions for the workflow to create tags and GitHub Releases. + ### **Before triggering a release**
36-38: Clarify approval requirement wording.Minor clarity pass; make the sentence active and unambiguous.
-1. The pipeline will pause at the "Review test and security scan result" step, at least 1 approver is required to review and approve the release. +1. The pipeline pauses at “Review test and security scan result.” At least one approver must review the artifacts and approve the release.
101-105: Add guidance on how to proceed when a tag exists or when vulnerabilities are found.Provide next steps to reduce ambiguity during release.
- The release workflow does not overwrite existing tags. If the same version tag already exists, the workflow will fail. + - Next steps: either bump the package version in `package.json` and retry, or delete/retire the existing tag and associated GitHub Release if it was created in error (coordinate with maintainers). - Vulnerability scanning does not block the release, but it is the approvers' responsibility to review the scan results in the Review stage. + - Recommended: if any High/Critical issues are found, create an issue linking the report and consider blocking the release until mitigations are applied.Would you like me to propose a small “Release Triage” checklist to include here?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks)RELEASE.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/release.yml
🧰 Additional context used
🪛 LanguageTool
RELEASE.md
[grammar] ~1-~1: There might be a mistake here.
Context: # 🚀 Release Pipeline Guide A GitHub Actions workflow has been set up...
(QB_NEW_EN)
[grammar] ~7-~7: There might be a mistake here.
Context: ...ackages/` directory. --- ## 🔑 Usage You can manually trigger the release work...
(QB_NEW_EN)
[grammar] ~20-~20: Ensure spelling is correct
Context: ...ersions and commit the lock file to the releaes branch ### Triggering a Release 1...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~20-~20: Ensure spelling is correct
Context: ...mmit the lock file to the releaes branch ### Triggering a Release 1. Go to **GitHu...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~26-~26: There might be a mistake here.
Context: ... e.g., xrpl or ripple-address-codec. - release_branch: The Git branch to rele...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ... Field | Example | |---------------|-----------------------...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...--------------|------------------------| | package_name | xrpl ...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...package_name | xrpl | | git_ref | release/[email protected] ...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...e. --- ## 🔨 How the Pipeline Works ### 1. Get Package Version - Extracts the...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ... packages/<package_name>/package.json. - No manual version input is required. --...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...cet tests against the specified Git ref. - Ensures the code at the given Git ref pa...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...elease Steps** - Builds the npm package. - Generates a CycloneDX SBOM (Software Bil...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...Review Stage* - Generate a summary of: - Package name - Version - Vulnerabili...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ... Generate a summary of: - Package name - Version - Vulnerability scan artifacts...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...summary of: - Package name - Version - Vulnerability scan artifacts - Requires ...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...Version - Vulnerability scan artifacts - Requires the approvers to manually revie...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...(Set up through GitHub Environment). - Creates a GitHub Release with a tag like...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...tions to Slack. --- ## 📁 Tag Format The GitHub release and git tag are named ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
RELEASE.md
94-94: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: integration (20.x)
- GitHub Check: integration (22.x)
- GitHub Check: build-and-lint (22.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: browser (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: unit (22.x)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
RELEASE.md (1)
51-53: Confirm that snippet tests are included (prior reviewer ask).Earlier review asked to trigger unit, snippet, and faucet tests. Please confirm whether snippet tests are covered by
nodejs.ymlor add them explicitly here and in the workflow call.Do you want me to open a follow-up PR to add/verify a
snippet_testsjob and update this section accordingly?
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/release.yml (3)
23-25: Scope concurrency key to package and ref to avoid global serializationA static concurrency group “release” forces all releases to queue. Include package and ref to allow parallel, de-duped runs per target.
concurrency: - group: release + group: release-${{ github.event.inputs.package_name }}-${{ github.event.inputs.release_branch }} cancel-in-progress: true
142-147: SBOM should target the selected package, not the whole workspaceGenerating at repo root pulls in all workspaces and overstates exposure.
- - name: Generate CycloneDX SBOM - run: cyclonedx-npm --output-format json --output-file sbom.json + - name: Generate CycloneDX SBOM (scoped) + working-directory: packages/${{ env.PACKAGE_NAME }} + run: cyclonedx-npm --output-format json --output-file ../../sbom.jsonAdjust downstream steps to use sbom.json at repo root as shown.
149-156: Vulnerability scan is non-blocking (exit-code: 0)Security findings won’t stop the pipeline. Make the scan actionable and fail on CRITICAL/HIGH unless explicitly waived.
- uses: aquasecurity/[email protected] + uses: aquasecurity/[email protected] with: scan-type: sbom scan-ref: sbom.json format: table - exit-code: 0 + exit-code: 1 output: vuln-report.txt severity: CRITICAL,HIGHOptionally add severity-threshold or an allowlist config to tune behavior.
🧹 Nitpick comments (12)
.github/workflows/release.yml (12)
85-89: Over-privileged job permissions in run_testsThis job shouldn’t need id-token or pages permissions for test execution via the reusable workflow. Reduce to least privilege.
run_tests: name: Run unit/integration tests ${{ needs.get_version.outputs.package_version }} permissions: - contents: read - id-token: write - pages: write + contents: readIf the called workflow truly needs additional scopes, grant them there or justify here explicitly.
49-55: Artifactory check scans entire repo; make it deterministic and focused on the changeRecursive grep across the repo can yield false positives and behaves differently depending on checkout depth. Prefer scanning the latest commit content and fail hard on matches.
- if grep -R --exclude-dir=.git "artifactory.ops.ripple.com" .; then + set -o pipefail + if git show --pretty='' HEAD | grep -q "artifactory.ops.ripple.com"; then echo "❌ Internal Artifactory URL found" exit 1 else echo "✅ No Internal Artifactory URL found" fi
116-127: Build narrows poorly in a monorepo; consider building only the selected packageRunning npm ci + npm run build at repo root will build all workspaces, increasing CI time and noise. Since you pack only one package later, build just that package.
- #build - npm ci - npm run build + # Build only the selected workspace + npm ci --workspaces=false + npm ci -w "packages/${PACKAGE_NAME}" + npm run -w "packages/${PACKAGE_NAME}" buildIf root scripts intentionally orchestrate cross-package builds, ignore this.
158-165: Fail fast on OWASP upload errorsThe current curl POST doesn’t fail the step on HTTP errors. Add fail-on-error and surface response codes.
- - name: Upload sbom to OWASP + - name: Upload sbom to OWASP run: | - curl -X POST \ + set -euo pipefail + curl --fail-with-body -sS -X POST \ -H "X-Api-Key: ${{ secrets.OWASP_TOKEN }}" \ -F "project=7c40c8ea-ea0f-4a5f-9b9f-368e53232397" \ -F "[email protected]" \ https://owasp-dt-api.prod.ripplex.io/api/v1/bom
181-191: Avoid mutating lerna.json during CI; it’s redundant with the scoped packYou already use lerna exec with --scope when packing. Rewriting lerna.json adds churn and risk without benefit.
- - name: Generate lerna.json for choosen the package - run: | - - echo "🔧 Updating lerna.json to include only packages/${{ env.PACKAGE_NAME }}" - - # Use jq to update the packages field safely - jq --arg pkg "packages/${{ env.PACKAGE_NAME }}" '.packages = [$pkg]' lerna.json > lerna.tmp.json && mv lerna.tmp.json lerna.json - - echo "✅ lerna.json updated:" - cat lerna.json + # Skip rewriting lerna.json; packing is scoped belowAlso, “choosen” → “chosen” if you keep this step.
325-337: Slack success message hardcodes “xrpl.js”; use the selected packageMessage should reflect the actual package being released.
- MESSAGE="✅ Released xrpl.js v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully." + MESSAGE="✅ Released ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Published to npm and GitHub successfully."
339-351: Slack failure message mislabels stage and hardcodes package nameThis is the release stage, not tests; and package name is fixed to xrpl.js.
- MESSAGE="❌ Tests failed for xrpl.js ${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" + MESSAGE="❌ Release pipeline failed for ${{ env.PACKAGE_NAME }} v${{ env.PACKAGE_VERSION }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"If you want build/test failures earlier to notify Slack, add such steps in those jobs.
118-123: Typos and minor polish in build step output
- “dubugging” → “debugging”
- # dubugging info + # debugging info
181-181: Typo: “choosen” → “chosen”If you keep this step, fix the wording.
- - name: Generate lerna.json for choosen the package + - name: Generate lerna.json for chosen package
245-246: Typo in summary output“Release Branc” → “Release Branch”.
- echo "🌿 Release Branc: $RELEASE_BRANCH" + echo "🌿 Release Branch: $RELEASE_BRANCH"Also consider aligning spacing or using printf for clean columns.
110-115: Inconsistent registry-url formattingOne Node setup uses a trailing slash, the other doesn’t. Standardize for consistency.
- registry-url: 'https://registry.npmjs.org' + registry-url: 'https://registry.npmjs.org/'Keep both steps identical.
Also applies to: 307-312
277-291: Dry-run still pushes tags; confirm desired behaviorYou now prefix dry-run tags with “draft-” but still force-push them. If dry-run should avoid any repo mutation, guard this entire step with
if: ${{ github.event.inputs.dry-run != 'true' }}or skip the push on dry-run.Happy to provide a variant that echoes the computed tag for the draft release without creating/pushing it.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/release.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-09-26T21:14:56.813Z
Learnt from: mvadari
PR: XRPLF/xrpl.js#2788
File: .github/workflows/nodejs.yml:25-25
Timestamp: 2024-09-26T21:14:56.813Z
Learning: In `.github/workflows/nodejs.yml`, the `build-and-lint` job's actions `actions/setup-node` and `actions/cache` are updated to `v4`.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2024-12-06T19:25:15.376Z
Learnt from: shawnxie999
PR: XRPLF/xrpl.js#2661
File: packages/xrpl/test/integration/transactions/mptokenAuthorize.test.ts:29-118
Timestamp: 2024-12-06T19:25:15.376Z
Learning: In the XRPLF/xrpl.js TypeScript client library, when writing tests (e.g., in `packages/xrpl/test/integration/transactions/`), we generally do not need to test rippled server behaviors, because those behaviors are covered by rippled's own integration and unit tests.
Applied to files:
.github/workflows/release.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: integration (22.x)
- GitHub Check: integration (20.x)
- GitHub Check: browser (22.x)
- GitHub Check: unit (22.x)
- GitHub Check: unit (20.x)
- GitHub Check: Analyze (javascript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
4 participants
High Level Overview of Change
Created workflow for release pipeline
Context of Change
Introducing a releasing process described in https://github.com/xpring-eng/xrpl.js/blob/main/RELEASE.md
Type of Change
Did you update HISTORY.md?
Test Plan