ci: migrate releases to semantic-release#59
Conversation
8136978 to
04954f2
Compare
doistbot
left a comment
There was a problem hiding this comment.
This pull request migrates the package release process to semantic-release, automating npm publishing and Conventional Commits validation. These updates will significantly streamline the release workflow and ensure consistent, predictable versioning. There are a few areas to review regarding npm provenance configuration for OIDC publishing, GitHub App permission syntax, a potential security risk with the pull_request_target trigger, and the retry logic for GitHub Packages.
| private-key: ${{ secrets.DOIST_RELEASE_BOT_PRIVATE_KEY }} | ||
| permission-contents: write | ||
| permission-issues: write | ||
| permission-pull-requests: write |
There was a problem hiding this comment.
[P1] GitHub App permissions use an underscore (pull_requests), not a hyphen. The actions/create-github-app-token action forwards this directly to the GitHub API, which will reject pull-requests with an input validation error. Update this key to permission-pull_requests: write.
| run: npx semantic-release | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| GITHUB_TOKEN: ${{ steps.release-bot.outputs.token || github.token }} |
There was a problem hiding this comment.
[P0] @semantic-release/npm strictly enforces the presence of an NPM_TOKEN unless npm provenance is explicitly configured. Because this workflow relies on passwordless Trusted Publishing via OIDC, semantic-release will fail during the verifyConditions step with an ENPMTOKEN error. Add NPM_CONFIG_PROVENANCE: true to this env block (or add "provenance": true to publishConfig in package.json) so it knows it can safely bypass the token check.
| - name: Clear npm config between GitHub/npm registries | ||
| run: rm -f $NPM_CONFIG_USERCONFIG | ||
| - name: Clear npm config between registries | ||
| if: ${{ steps.semantic-release.outputs.package-published == 'true' }} |
There was a problem hiding this comment.
[P1] This gate makes the GitHub Packages publish non-retryable. If semantic-release has already published to npm/tagged the release and the later GitHub Packages npm publish fails, a rerun won't set package-published == 'true' again, so this step is skipped and the two registries stay out of sync. Move the GitHub Packages publish into the semantic-release publish flow, or key this step off the resolved release version instead of a one-run success flag.
| if: ${{ steps.semantic-release.outputs.package-published == 'true' }} | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| cache: npm |
There was a problem hiding this comment.
[P3] Dependencies are already cached and installed by the first setup-node step. Copying cache: npm here unnecessarily re-runs the post-job cache save action. You can remove this line to streamline configuring the second registry.
| "@semantic-release/exec", | ||
| { | ||
| verifyConditionsCmd: | ||
| 'if [ -n "$GITHUB_OUTPUT" ]; then echo "package-published=false" >> "$GITHUB_OUTPUT"; fi', |
There was a problem hiding this comment.
[P3] This shell script is duplicated for successCmd. To keep it DRY and avoid drift, consider extracting a helper for setting the output:
const setOutput = (value) => `if [ -n "$GITHUB_OUTPUT" ]; then echo "package-published=${value}" >> "$GITHUB_OUTPUT"; fi`;
// ...
verifyConditionsCmd: setOutput('false'),
successCmd: setOutput('true'),| env: | ||
| CI: true | ||
|
|
||
| - name: Test the codebase |
There was a problem hiding this comment.
[P2] This extends the same checkout/setup/install/check/test/build sequence that's already defined in .github/workflows/check-pull-request-health.yml. Pull that shared CI path into a reusable workflow or composite action and call it from both places, otherwise changes like this new test step have to be mirrored manually and the two workflows will keep drifting.
| release: | ||
| types: [created] | ||
| workflow_dispatch: | ||
| push: |
There was a problem hiding this comment.
[P2] Switching this workflow to every push on main means we now pay for a full install/check/test/build cycle before semantic-release decides whether there's anything to publish. Commits like docs: or chore: will still run the whole job and then no-op. Add an early release-needed gate and skip the expensive steps when no release is due.
| name: Semantic Pull Request | ||
|
|
||
| on: | ||
| pull_request_target: |
There was a problem hiding this comment.
[P2] Per the Secrets Management standard, workflows triggered by pull requests from forks should not have access to secrets to prevent potential exfiltration. While this workflow currently doesn't check out fork code, using pull_request_target exposes repository secrets to the run context and creates a security risk if the workflow is later modified. Change this to pull_request, which safely runs with a read-only token and is fully sufficient for validating PR titles.
Overview
semantic-releasemainnextas a staging branch only, so this change can merge intonextsafely before the first automated stable releaseAfter this gets merged
Once this PR has landed in
nextand we are ready to release everything currently staged there, promotenexttomainwithout squash-merging it. That preserves the existing breakingfeat!commits sosemantic-releasecomputesv3.0.0.Preferred promotion flow:
Then:
.github/workflows/publish.ymlonmainto finish.v3.0.0, a GitHub release, npm package@doist/react-interpolate@3.0.0, and the release commit onmainupdatingpackage.json,package-lock.json, andCHANGELOG.md.nextback withmainso the semantic-release commit is carried forward:If
mainis protected and direct push is not allowed, open a PR fromnexttomainand merge it with a merge commit or rebase merge, but do not squash it.