Release pipeline #3039
Release pipeline #3039
Conversation
|
""" WalkthroughA new GitHub Actions workflow for releasing npm packages was introduced, including automation for building, testing, security scanning, artifact management, and manual approval. The Node.js CI workflow and faucet test workflow were updated to support reusable invocation with a specific git reference. Documentation was added describing the release pipeline and its procedures. Changes
Suggested reviewers
Poem
""" 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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 ]
.github/workflows/nodejs.yml
Outdated
| env: | ||
| GIT_REF: ${{ github.event.inputs.git_ref || github.ref }} | ||
| strategy: | ||
| matrix: | ||
| node-version: [22.x] | ||
|
|
||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ env.GIT_REF }} | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
Incorrect context for reusable-workflow input – breaks workflow_call invocations
Inside reusable workflows the input is exposed through the inputs context (${{ inputs.git_ref }}), not github.event.inputs.git_ref.
Referencing a non-existent path silently returns null and causes checkout to fallback to the default ref, defeating the purpose of the parameter.
-env:
- GIT_REF: ${{ github.event.inputs.git_ref || github.ref }}
+env:
+ # Use reusable-workflow input when present; otherwise fallback for push/PR runs
+ GIT_REF: ${{ inputs.git_ref || github.ref }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env: | |
| GIT_REF: ${{ github.event.inputs.git_ref || github.ref }} | |
| strategy: | |
| matrix: | |
| node-version: [22.x] | |
| steps: | |
| - uses: actions/checkout@v4 | |
| with: | |
| ref: ${{ env.GIT_REF }} | |
| fetch-depth: 0 | |
| env: | |
| # Use reusable-workflow input when present; otherwise fallback for push/PR runs | |
| GIT_REF: ${{ inputs.git_ref || github.ref }} | |
| strategy: | |
| matrix: | |
| node-version: [22.x] | |
| steps: | |
| - uses: actions/checkout@v4 | |
| with: | |
| ref: ${{ env.GIT_REF }} | |
| fetch-depth: 0 |
🧰 Tools
🪛 actionlint (1.7.7)
28-28: property "git_ref" is not defined in object type {}
(expression)
🤖 Prompt for AI Agents
In .github/workflows/nodejs.yml around lines 27 to 38, the environment variable
GIT_REF is incorrectly set using github.event.inputs.git_ref, which is not
available in reusable workflows. Change the reference to use the inputs context
by setting GIT_REF to ${{ inputs.git_ref || github.ref }} to correctly access
the input parameter and ensure the checkout step uses the intended git
reference.
| - name: Notify Slack if tests fail | ||
| if: failure() | ||
| env: | ||
| SLACK_TOKEN: ${{ secrets.SLACK_TOKEN }} | ||
| run: | | ||
| MESSAGE="❌ Tests failed for xrpl.js ${{ github.event.inputs.package_version }}. Check the logs: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" | ||
| curl -X POST https://slack.com/api/chat.postMessage \ | ||
| -H "Authorization: Bearer $SLACK_TOKEN" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d "$(jq -n \ | ||
| --arg channel "#ripplex-test-notifications-non-prod" \ | ||
| --arg text "$MESSAGE" \ | ||
| '{channel: $channel, text: $text}')" |
There was a problem hiding this comment.
Reference to non-existent github.event.inputs.package_version
The Slack-failure step dereferences package_version, which is not an input. Use the value obtained in get_version.
- MESSAGE="❌ Tests failed for xrpl.js ${{ github.event.inputs.package_version }}. ...
+ MESSAGE="❌ Release pipeline failed for xrpl.js v${{ env.PACKAGE_VERSION }}. ...This also avoids misleading “Tests failed” wording in a release stage.
🧰 Tools
🪛 actionlint (1.7.7)
261-261: property "package_version" is not defined in object type {git_ref: string; package_name: string}
(expression)
🤖 Prompt for AI Agents
In .github/workflows/release.yml around lines 257 to 269, the Slack notification
step incorrectly references github.event.inputs.package_version, which does not
exist. Replace this with the version value obtained from the get_version step,
typically accessed via an output variable or environment variable set earlier in
the workflow. Update the MESSAGE variable to use this correct version reference
to ensure accurate and meaningful Slack notifications.
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| REPO: ${{ github.repository }} | ||
| RUN_ID: ${{ github.run_id }} | ||
| run: | | ||
| ARTIFACT_NAME="vulnerability-report" | ||
|
|
||
| echo "Fetching artifact ID for ${ARTIFACT_NAME}..." | ||
| ARTIFACTS=$(curl -s -H "Authorization: Bearer $GH_TOKEN" \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
| https://api.github.com/repos/$REPO/actions/runs/$RUN_ID/artifacts) | ||
|
|
||
| ARTIFACT_ID=$(echo "$ARTIFACTS" | jq -r ".artifacts[] | select(.name == \"$ARTIFACT_NAME\") | .id") | ||
|
|
||
| if [ -z "$ARTIFACT_ID" ]; then | ||
| echo "❌ Artifact not found." | ||
| exit 1 | ||
| fi | ||
| echo "🔍 Please review the following details before proceeding:" | ||
| echo "📦 Package Name: $PACKAGE_NAME" | ||
| echo "🔖 Package Version: $PACKAGE_VERSION" | ||
| echo "🌿 Git Ref: $GIT_REF" | ||
| echo "🔢 Commit SHA: $COMMIT_SHA" | ||
| echo "🔗 Please review Vulnerabilities detected: https://github.com/$REPO/actions/runs/${{ github.run_id }}/artifacts/$ARTIFACT_ID" | ||
|
|
There was a problem hiding this comment.
GIT_REF and COMMIT_SHA printed but never set
The summary prompts reviewers with undefined variables, producing empty lines.
+ GIT_REF="${{ github.event.inputs.git_ref }}"
+ COMMIT_SHA="$(git rev-parse --short HEAD)"Export before echoing or include in $GITHUB_ENV.
🤖 Prompt for AI Agents
In .github/workflows/release.yml between lines 173 and 197, the variables
GIT_REF and COMMIT_SHA are echoed but never set, resulting in empty output
lines. Fix this by setting these variables before the echo statements, either by
exporting them with appropriate GitHub context values or by adding them to the
$GITHUB_ENV file so they are properly defined and available during the run.
RELEASE.md
Outdated
| --- | ||
|
|
||
| ### 2. **Run Tests** | ||
| - Triggers the integration test workflow. |
There was a problem hiding this comment.
Can this also trigger unit, snippet, and faucet tests?
There was a problem hiding this comment.
sure. I can add those
There was a problem hiding this comment.
I have added step for triggering faucet test, it now run both faucet_test and nodejs.yaml workflow which covers unit test and integration test. Also updated the RELEASE.md
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)
| - uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ env.GIT_REF }} | ||
| fetch-depth: 0 | ||
| - name: Use Node.js ${{ matrix.node-version }} |
There was a problem hiding this comment.
env.GIT_REF is undefined – checkout silently falls back to default branch
actions/checkout expects a concrete ref but the job never sets GIT_REF; thus ${{ env.GIT_REF }} resolves to an empty string and the step checks out the default branch instead of the supplied input, defeating the purpose of workflow_call.
- - uses: actions/checkout@v4
- with:
- ref: ${{ env.GIT_REF }}
- fetch-depth: 0
+ - uses: actions/checkout@v4
+ with:
+ # Prefer the reusable-workflow input when present, otherwise fall back for push/dispatch runs
+ ref: ${{ inputs.git_ref || github.ref }}
+ fetch-depth: 0If you still need GIT_REF elsewhere, set it explicitly:
env:
GIT_REF: ${{ inputs.git_ref || github.ref }}🤖 Prompt for AI Agents
In .github/workflows/faucet_test.yml around lines 26 to 30, the environment
variable GIT_REF used in the checkout step is not defined, causing the action to
default to the main branch instead of the intended ref. To fix this, explicitly
set the GIT_REF environment variable at the job or workflow level by assigning
it to the input git_ref if provided, or fallback to github.ref, ensuring the
checkout action receives a valid ref value.
| with: | ||
| ref: ${{ env.GIT_REF }} | ||
| fetch-depth: 0 | ||
| - name: Use Node.js ${{ matrix.node-version }} |
There was a problem hiding this comment.
Most jobs reference a non-existent GIT_REF env var → all checkouts use default branch
GIT_REF is only defined in the build-and-lint job.
unit, integration, browser, generate-documentation, and snippets jobs still rely on ${{ env.GIT_REF }} but never define it, so the checkout ref resolves to empty/default.
Fix once at workflow level:
env:
RIPPLED_DOCKER_IMAGE: rippleci/rippled:develop
+ # Make the git ref accessible to every job/step
+ GIT_REF: ${{ inputs.git_ref || github.ref }}…and drop the per-job env block (or add one to each job).
Without this, the release pipeline will test/build a different commit from the one being released.
Also applies to: 121-124, 175-178, 229-231, 309-312
🤖 Prompt for AI Agents
In .github/workflows/nodejs.yml around lines 78 to 81, the environment variable
GIT_REF is used in multiple jobs but only defined in the build-and-lint job,
causing other jobs to default to the main branch. To fix this, define GIT_REF at
the workflow level so all jobs inherit it, or alternatively add an env block
defining GIT_REF to each job that uses it. This ensures all checkouts use the
correct commit reference consistently across the workflow.
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)
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