Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/check-semantic-pull-request.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Semantic Pull Request

on:
pull_request_target:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

types:
- edited
- opened
- reopened
- synchronize

permissions:
contents: read
pull-requests: read

jobs:
validate-title:
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- name: Validate pull request title
uses: amannn/action-semantic-pull-request@01d5fd8a8ebb9aafe902c40c53f0f4744f7381eb
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
75 changes: 55 additions & 20 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
@@ -1,28 +1,50 @@
name: Release @doist/react-interpolate package

on:
release:
types: [created]
workflow_dispatch:
push:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure if this gate is worth adding. It would need to predict semantic-release’s decision before semantic-release runs, which is easy to get subtly wrong. I’d rather keep semantic-release as the single source of truth unless no-op release runs become a real cost problem (which I don't think it will for this project).

branches:
- main

permissions:
# Enable the use of OIDC for trusted publishing and npm provenance
contents: write
issues: write
pull-requests: write
Comment on lines +10 to +11

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these for? I don't think we needed them for semantic-release to post comments on issues/PRs, it still works without these permissions. At least it works on Typist without them, so it should work here too.

id-token: write
# Enable the use of GitHub Packages registry
packages: write

concurrency:
group: ${{ github.workflow }}
cancel-in-progress: false

jobs:
publish:
runs-on: ubuntu-latest
timeout-minutes: 60
steps:
- name: Generate release bot token
id: release-bot
if: ${{ secrets.DOIST_RELEASE_BOT_ID != '' && secrets.DOIST_RELEASE_BOT_PRIVATE_KEY != '' }}
uses: actions/create-github-app-token@v3
with:
app-id: ${{ secrets.DOIST_RELEASE_BOT_ID }}
private-key: ${{ secrets.DOIST_RELEASE_BOT_PRIVATE_KEY }}
permission-contents: write
permission-issues: write
permission-pull-requests: write
Comment thread
gnapse marked this conversation as resolved.

- name: Checkout repository
uses: actions/checkout@v4

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd take this opportunity to change this to v6 (and any other GitHub action to the latest version, or at least the ones that Typist is using because they are proven to work without issues so far).

with:
fetch-depth: 0
token: ${{ steps.release-bot.outputs.token || github.token }}

- name: Setup Node.js from .node-version
uses: actions/setup-node@v3
uses: actions/setup-node@v4
with:
cache: npm
node-version-file: .node-version
registry-url: https://registry.npmjs.org/
scope: "@doist"

- name: Ensure npm 11.5.1 or later is installed
run: npm install -g npm@latest
Expand All @@ -35,26 +57,39 @@ jobs:
env:
CI: true

- name: Test the codebase

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense, and it's the approach that Typist already uses, so you can use that one as inspiration.

run: npm run test
env:
CI: true

- name: Build the codebase
run: npm run build

- name: Publish to GitHub Package Registry
uses: actions/setup-node@v3
with:
node-version-file: .node-version
registry-url: https://npm.pkg.github.com/
scope: "@doist"
- run: npm publish
- name: Publish package to npm registry
id: semantic-release
run: npx semantic-release
env:
NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ steps.release-bot.outputs.token || github.token }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn’t seem necessary with the current @semantic-release/npm version. Typist uses the same tokenless trusted publishing setup without NPM_CONFIG_PROVENANCE or publishConfig.provenance, and successful runs show @semantic-release/npm exchanging the GitHub Actions OIDC token with npm before publishing. Provenance is then signed automatically by npm. So I think the important bit is making sure trusted publishing is configured for this repo/workflow, not adding explicit provenance config.

GIT_AUTHOR_EMAIL: doistbot@users.noreply.github.com
GIT_AUTHOR_NAME: Doist Bot
GIT_COMMITTER_EMAIL: doistbot@users.noreply.github.com
GIT_COMMITTER_NAME: Doist Bot

- 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' }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a real retryability gap, but I don’t think we can fully fix it with semantic-release as-is. @semantic-release/npm only publishes to one registry, so GitHub Packages has to remain a follow-up step for now. I’d accept this limitation and handle failures manually if they happen.

run: rm -f "$NPM_CONFIG_USERCONFIG"

- name: Publish to npm registry
uses: actions/setup-node@v3
- name: Setup GitHub Packages registry
if: ${{ steps.semantic-release.outputs.package-published == 'true' }}
uses: actions/setup-node@v4
with:
cache: npm

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds right. The second setup-node is only switching registry config for GitHub Packages, and there’s no install afterward, so cache: npm doesn’t add value there.

node-version-file: .node-version
registry-url: https://registry.npmjs.org/
registry-url: https://npm.pkg.github.com/
scope: "@doist"
- run: npm publish --provenance --access public

- name: Publish package to GitHub Packages
if: ${{ steps.semantic-release.outputs.package-published == 'true' }}
run: npm publish
env:
NODE_AUTH_TOKEN: ${{ github.token }}
15 changes: 8 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,15 @@ import { SYNTAX_I18NEXT } from "react-interpolate"

# Releasing

A new version of @doist/react-interpolate is published both on npm and GitHub Package Registry whenever a new release on GitHub is created.
Merges to `main` automatically trigger `semantic-release`.

To update the version in both `package.json` and `package-lock.json` run:
The release workflow will:

```sh
npm --no-git-tag-version version <major|minor|patch>
```
- determine the next version from Conventional Commits
- update `package.json`, `package-lock.json`, and `CHANGELOG.md`
- create the Git tag and GitHub release
- publish `@doist/react-interpolate` to both npm and GitHub Packages

Once these changes have been pushed and merged, create a release on GitHub.
This means there is no manual version bump and no manual GitHub release creation step anymore.

A GitHub Action will automatically perform all the necessary steps and will release the version number that's specified inside the `package.json`'s `version` field so make sure that the release tag reflects the version you want to publish.
Because the repository uses squash-merge style commit messages, pull request titles must follow the [Conventional Commits specification](https://www.conventionalcommits.org/). CI validates this automatically.
Loading
Loading