Conversation
WalkthroughAdds two GitHub Actions workflows: one that publishes on pushes to master and one that runs a PR dry-run; and renames the published package from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant GH as GitHub Actions
participant GCP as Google Cloud (Workload Identity)
participant AR as Artifact Registry (npm)
Dev->>GH: Push to master
activate GH
GH->>GH: actions/checkout\nsetup-node@v20
GH->>GCP: Authenticate (workload identity)\nAssume service account
GCP-->>GH: Access token
GH->>AR: Configure npm auth (token)
GH->>GH: npm install
GH->>AR: npm publish
AR-->>GH: Publish result
deactivate GH
sequenceDiagram
autonumber
participant Dev as Developer
participant GH as GitHub Actions (PR workflow)
participant GCP as Google Cloud (Workload Identity)
participant AR as Artifact Registry (npm)
Dev->>GH: Open/sync/reopen PR -> target master
activate GH
GH->>GH: actions/checkout\nsetup-node@v24
GH->>GCP: Authenticate (workload identity)\nAssume service account
GCP-->>GH: Access token
GH->>AR: Configure npm auth (token)
GH->>GH: npm install
GH->>AR: npm publish (dry-run)
AR-->>GH: Dry-run result
deactivate GH
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
2-2: AddpublishConfigto lock the publish target.Right now a developer running
npm publishlocally would fall back to the default registry if their.npmrcisn’t primed, risking an accidental push to npmjs. Add apublishConfig.registrypointing at Artifact Registry so every publish path stays scoped correctly.(docs.github.com)"name": "@spotdraft/liquidjs", "version": "3.1.0", ... "postversion": "git push && git push --tags" - } + }, + "publishConfig": { + "registry": "https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/master.yaml(1 hunks)package.json(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/master.yaml
8-8: label "ubicloud-standard-2" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
| - name: Configure NPM to use Artifact Registry | ||
| run: | | ||
| TOKEN=$(gcloud auth print-access-token) | ||
| rm -rf .npmrc | ||
| echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc | ||
| echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc |
There was a problem hiding this comment.
Add always-auth=true so npm actually sends the token.
Artifact Registry’s own guidance keeps an always-auth=true entry alongside the _authToken. Without it, npm can skip attaching the token on preliminary GET/HEAD requests when publishing, which leads to intermittent 401s during npm publish. Please add the flag when building .npmrc.(cloud.google.com)
TOKEN=$(gcloud auth print-access-token)
rm -rf .npmrc
- echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
+ echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:always-auth=true" >> .npmrc
+ echo "//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc📝 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.
| - name: Configure NPM to use Artifact Registry | |
| run: | | |
| TOKEN=$(gcloud auth print-access-token) | |
| rm -rf .npmrc | |
| echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc | |
| echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc | |
| - name: Configure NPM to use Artifact Registry | |
| run: | | |
| TOKEN=$(gcloud auth print-access-token) | |
| rm -rf .npmrc | |
| echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:always-auth=true" >> .npmrc | |
| echo "//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc | |
| echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc |
🤖 Prompt for AI Agents
In .github/workflows/master.yaml around lines 35 to 40, the workflow writes an
.npmrc with the _authToken and registry but omits the always-auth flag; update
the script that builds .npmrc to append an always-auth=true entry (for the
registry in question) immediately after writing the _authToken so npm will
always send the token and avoid intermittent 401s during publish.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pr.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/pr.yaml
8-8: label "ubicloud-standard-2" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
| TOKEN=$(gcloud auth print-access-token) | ||
| rm -rf .npmrc | ||
| echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc | ||
| echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc |
There was a problem hiding this comment.
Fix _authToken quoting to avoid authentication failure.
.npmrc keeps the double quotes you echo around ${TOKEN}, so npm sends Bearer "token" and Artifact Registry rejects the publish (dry-run or real). Drop the quotes and write the file in one shot.
- TOKEN=$(gcloud auth print-access-token)
- rm -rf .npmrc
- echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc
- echo "@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/" >> .npmrc
+ TOKEN="$(gcloud auth print-access-token)"
+ cat <<'EOF' > .npmrc
+//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=${TOKEN}
+@spotdraft:registry=https://asia-south1-npm.pkg.dev/spotdraft-qa/npm/
+EOFCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/pr.yaml around lines 37 to 40, the current echo adds
literal double quotes around the auth token and appends each line separately;
change it to write the .npmrc in a single write operation and remove the
surrounding quotes so the _authToken is written as ...:_authToken=<TOKEN> (no
quotes). Ensure the command writes both registry lines to .npmrc atomically
(overwrite, not append) and that the token is inserted raw so npm sends Bearer
<token> without embedded quotes.
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| { | ||
| "name": "liquidjs", | ||
| "name": "@spotdraft/liquidjs", | ||
| "version": "3.1.0", | ||
| "description": "Liquid template engine by pure JavaScript: compatible to shopify, easy to extend.", |
There was a problem hiding this comment.
renaming the package to @spotdraft/liquidjs while the publish workflow only authenticates/publishes to the scoped registry means npm install liquidjs stops receiving updates yet README/install docs still point to the unscoped name; can we keep publishing an alias named liquidjs or explicitly document the new scoped package before the contract is broken?
| { | |
| "name": "liquidjs", | |
| "name": "@spotdraft/liquidjs", | |
| "version": "3.1.0", | |
| "description": "Liquid template engine by pure JavaScript: compatible to shopify, easy to extend.", | |
| { | |
| "name": "liquidjs", | |
| "version": "3.1.0", | |
| "description": "Liquid template engine by pure JavaScript: compatible to shopify, easy to extend.", |
Finding type: Breaking Changes
| - name: Setup Node.js | ||
| uses: actions/setup-node@v5 | ||
| with: | ||
| node-version: '24' |
There was a problem hiding this comment.
PR workflow now sets node-version: '24' while the master publish workflow still uses Node 20, so the dry-run on PRs no longer validates the actual publish environment and Node 20–specific build/publish regressions can slip through; can we pin the same Node version in both workflows?
| - name: Setup Node.js | |
| uses: actions/setup-node@v5 | |
| with: | |
| node-version: '24' | |
| - name: Setup Node.js | |
| uses: actions/setup-node@v5 | |
| with: | |
| node-version: '20' |
Finding type: Logical Bugs
| - name: Configure NPM to use Artifact Registry | ||
| run: | | ||
| TOKEN=$(gcloud auth print-access-token) | ||
| rm -rf .npmrc | ||
| echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc |
There was a problem hiding this comment.
echo -e "..._authToken="$TOKEN"" expands the freshly minted GCP access token inside the logged shell command, so the raw token is emitted in every workflow log (same lines exist in pr.yaml); can we mask the token (e.g. echo "::add-mask::$TOKEN") or otherwise avoid printing the value when writing .npmrc?
| - name: Configure NPM to use Artifact Registry | |
| run: | | |
| TOKEN=$(gcloud auth print-access-token) | |
| rm -rf .npmrc | |
| echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc | |
| - name: Configure NPM to use Artifact Registry | |
| run: | | |
| TOKEN=$(gcloud auth print-access-token) | |
| echo "::add-mask::$TOKEN" && rm -rf .npmrc && echo -e "\n//asia-south1-npm.pkg.dev/spotdraft-qa/npm/:_authToken=\"$TOKEN\"" >> .npmrc |
Finding type: Basic Security Patterns
1 participant
User description
Summary by CodeRabbit
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Configures GitHub Actions workflows for automated building and publishing to Google Artifact Registry. Updates the package name to
@spotdraft/liquidjsto align with organizational scoping requirements.master.yamlandpr.yamlworkflows to handle authentication with Google Cloud, dependency installation, and package publishing.Modified files (2)
Latest Contributors(0)
@spotdraft/liquidjswithinpackage.jsonto support scoped registry publishing.Modified files (1)
Latest Contributors(2)