-
Notifications
You must be signed in to change notification settings - Fork 0
🔧 [PB-1278] Automated deployment with developer-controlled versioning #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…versioning - Change trigger from manual to automatic (push to main when package.json changes) - Add Docker-based version validation using semver package - Remove version bumping steps (developer controls version in PR) - Auto-detect pre-release tags from version format - Add registry-url for OIDC publishing - Update README with new deployment process
📝 WalkthroughWalkthroughAutomated deployment now triggers on pushes to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer (PR / Merge)
participant GH as GitHub (repo)
participant GA as GitHub Actions
participant Dock as Docker (version-check)
participant NPM as npm Registry
participant GR as GitHub Releases
Dev->>GH: Push commit / merge updating `package.json`
GH->>GA: Trigger workflow (push to main, package.json changed)
GA->>GA: checkout (fetch-depth: 0)
GA->>Dock: Run Docker-based semantic version validation
Dock-->>GA: validation result
GA->>GA: Parse `package.json` -> outputs: version_number, tag, prerelease
alt prerelease
GA->>NPM: publish with prerelease tag
else stable
GA->>NPM: publish with `latest` tag
end
GA->>GR: Create/update GitHub Release (uses version_number)
GA-->>Dev: Publish & release summary (includes install command with version_number)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 PR Quality Check Summary
📋 Checks Performed:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (1)
1-232: Address Prettier formatting issues.The PR checks pipeline reports a Prettier formatting warning for this file. Please run
pnpm run formatto fix the formatting issues before merging.#!/bin/bash # Run Prettier to show the formatting differences cd "${{ github.workspace }}" && pnpm run format:check.github/workflows/deploy.yml (1)
103-105: Remove theNODE_AUTH_TOKENenvironment variable.Setting
NODE_AUTH_TOKENto an empty string (or any value) breaks OIDC-based trusted publishing. When using the--provenanceflag with npm publish, the OIDC token exchange must occur without any NODE_AUTH_TOKEN present. If NODE_AUTH_TOKEN is set—even to an empty string—npm will attempt token-based authentication instead of exchanging the OIDC ID token for a short-lived credential, preventing the trusted publisher flow and disabling automatic provenance generation.Remove lines 104-105 entirely:
Diff
- name: Publish to NPM run: npm publish --access public --tag ${{ steps.version.outputs.tag }} --provenance - env: - NODE_AUTH_TOKEN: ''
🤖 Fix all issues with AI agents
In @.github/workflows/deploy.yml:
- Around line 29-48: The YAML step "Validate version increment" (id:
version_check) uses a multi-line shell script in the args field of uses:
docker://node:22-alpine which is causing a YAML parsing error; fix it by
replacing the complex inline args string with a properly formatted YAML literal
block scalar or by converting the step into a normal run step that invokes
docker run (or a node container) and placing the entire multi-line script under
a | block (or the run key) so the shell script is treated as a single YAML
string; ensure the fields referenced (uses: docker://node:22-alpine, args, and
the step name/id) remain but the script is moved into a block scalar or run step
to resolve the "mapping values are not allowed" parsing error.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/deploy.ymlREADME.md
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/deploy.yml
47-47: could not parse as YAML: mapping values are not allowed in this context
(syntax-check)
🪛 GitHub Actions: PR Checks
.github/workflows/deploy.yml
[error] 1-1: Prettier formatting check failed. SyntaxError: All collection items must start at the same column (33:11).
README.md
[warning] 1-1: Code style check reported a warning for README.md during formatting check.
🪛 YAMLlint (1.37.1)
.github/workflows/deploy.yml
[error] 47-47: syntax error: mapping values are not allowed here
(syntax)
🔇 Additional comments (9)
.github/workflows/deploy.yml (6)
4-8: LGTM! Trigger configuration aligns with automated deployment goals.The path filter ensures the workflow runs only when
package.jsonchanges on main, which is exactly what's needed for automatic version-based deployment.
27-27: Good optimization for version comparison.Changing
fetch-depthfrom 0 to 2 is appropriate since the validation step only needs to compare with the previous commit (HEAD~1). This improves checkout performance.
50-76: Version parsing logic is sound for typical semver formats.The regex patterns correctly handle:
- Stable versions:
1.0.0→ taglatest- Pre-releases:
1.0.0-beta.1→ tagbetaThe pre-release pattern
([a-zA-Z]+)captures only alphabetic characters, which works well for common npm tags (beta, alpha, rc, next). This is appropriate for deriving the npm dist-tag from the version string.
87-87: Correct setup for OIDC-based npm publishing.Adding
registry-urlis required for the setup-node action to configure authentication with npm registry using OIDC trusted publishing.
118-124: Consistent version reference usage in release notes.The migration to
steps.version.outputs.version_numberis correct. This output contains the version without the 'v' prefix, which is appropriate for npm install commands and package URLs.
127-127: Pre-release detection correctly integrated.The
prereleaseflag properly uses the output from the version parsing step, ensuring GitHub releases are appropriately marked for pre-release versions (e.g., beta, alpha).README.md (3)
187-194: Clear and accurate deployment documentation.The deployment pipeline description accurately reflects the workflow changes, including automated triggering, Docker-based validation, pre-release detection, and OIDC publishing with provenance.
200-222: Excellent developer-friendly deployment instructions.The step-by-step guide clearly explains the new PR-based version bump workflow. Including examples for both stable releases and pre-releases helps developers understand the process.
224-228: Clear version-to-tag mapping examples.The version format examples clearly illustrate how the workflow derives npm tags from version strings, which will help developers choose appropriate version formats for their releases.
🔍 PR Quality Check Summary
📋 Checks Performed:
|
🔍 PR Quality Check Summary
📋 Checks Performed:
🎉 All checks passed! This PR is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/deploy.yml (1)
78-81: Critical: EmptyNODE_AUTH_TOKENwill cause npm publish to fail.The
NODE_AUTH_TOKENis set to an empty string (''). For npm publish to work with OIDC trusted publishing, you need to omitNODE_AUTH_TOKENentirely or provide a valid token. An empty string will cause authentication failure.For OIDC-based publishing (provenance attestation), the token is automatically provided by the
id-token: writepermission—you should remove theNODE_AUTH_TOKENenv var entirely.🔧 Proposed fix
- name: Publish to NPM run: npm publish --access public --tag ${{ steps.version.outputs.tag }} --provenance - env: - NODE_AUTH_TOKEN: ''npm OIDC trusted publishing provenance NODE_AUTH_TOKEN 2024
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/deploy.ymlREADME.md
🔇 Additional comments (5)
README.md (2)
187-193: LGTM! Clear documentation of the new automated deployment workflow.The documentation accurately describes the trigger mechanism, pre-release detection, and quality checks. The OIDC trusted publishing with provenance attestation is a security best practice.
199-229: LGTM! Well-structured deployment instructions.The step-by-step instructions and version format table are clear and helpful for developers. The examples for stable and pre-release versions align with the workflow's regex patterns.
.github/workflows/deploy.yml (3)
4-8: Verify: Workflow may trigger on non-version changes to package.json.The workflow triggers on any change to
package.json, including dependency updates, script changes, or metadata edits—not just version bumps. While the PR objective mentions Docker-based validation was initially added to verify version actually changed, commit history indicates this was later removed.Consider whether re-adding version change validation would prevent unnecessary workflow runs when package.json is modified for other reasons.
29-51: Prerelease regex may not capture full tag identifiers.The regex
^[0-9]+\.[0-9]+\.[0-9]+-([a-zA-Z]+)captures only the alphabetic part of the prerelease identifier, stopping at the first non-letter character. This works for extractingbetafrom1.0.0-beta.1, which is likely the intended behavior for npm tags.However, versions like
1.0.0-rc1would capturerc(correct), but1.0.0-beta1(without dot) would also capturebetaand lose the1. Ensure this aligns with your versioning conventions.
83-104: LGTM! GitHub Release step is well-configured.The release creation properly uses version outputs, includes helpful installation instructions, and correctly sets the prerelease flag based on version detection. The NPM links are correctly formatted with
version_number(withoutvprefix).
Summary
This PR transforms the deployment workflow from manual trigger with automatic version bumping to an automated deployment that triggers when version changes are merged to main.
Key Changes
1. Automated Trigger
workflow_dispatch(manual) topushtriggerpackage.jsonchanges on main branch2. Developer-Controlled Versioning
3. Docker-Based Version Validation
node:22-alpineDocker image for validationsemverpackage to ensure valid version increments4. Auto-Detection of Pre-releases
1.0.0→ publishes with taglatest1.0.0-beta.1→ publishes with tagbeta1.0.0-alpha.1→ publishes with tagalpha5. Simplified Workflow
Benefits
✅ No branch protection conflicts - workflow doesn't commit to main
✅ Fully automated - merge triggers deployment
✅ Developer controls version - explicit in PR
✅ Auto-detects pre-releases - from version format
✅ No special tokens needed - uses GITHUB_TOKEN + OIDC
✅ Semantic version validation - Docker + semver ensures valid increments
How It Works Now
Updated Documentation
Testing
After merge, test with:
0.2.8Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.