From 1b9abb2053424a612a8cbc9fbd989bf47d6164d1 Mon Sep 17 00:00:00 2001 From: Ngoc Le Date: Wed, 12 Aug 2026 14:41:57 +0700 Subject: [PATCH] fix: publish npm from main push --- .github/NPM_PUBLISHING.md | 15 +++++----- .github/workflows/publish-npm.yml | 49 ++++++++++++++++++++++++------- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/.github/NPM_PUBLISHING.md b/.github/NPM_PUBLISHING.md index f0dcc7e..3946e61 100644 --- a/.github/NPM_PUBLISHING.md +++ b/.github/NPM_PUBLISHING.md @@ -1,12 +1,11 @@ # npm publishing setup -The workflow in `workflows/publish-npm.yml` publishes only after a pull request: +The workflow in `workflows/publish-npm.yml` runs on pushes to `main`, then publishes only when that commit: -1. targets `main`; -2. has a current approval from, or is merged by, an allowed npm approver; -3. is merged; -4. passes Jest tests, lint, build, CommonJS verification, and generated-output checks; and -5. contains a package version that does not already exist on npm. +1. is the merge commit of exactly one pull request targeting `main`; +2. has a current approval from, or was merged by, an allowed npm approver; +3. passes Jest tests, lint, build, CommonJS verification, and generated-output checks; and +4. contains a package version that does not already exist on npm. ## npm Trusted Publishing @@ -26,7 +25,9 @@ The workflow grants `id-token: write`, runs on a GitHub-hosted runner, and insta npm 11.19.0 because Trusted Publishing requires npm 11.5.1 or later with Node 22.14.0 or later. npm 11.19.0 supports Node 22.14.0; npm 12 requires a newer Node release. The npm package setting disallows bypass-2FA token publishing; -the configured OIDC publisher remains allowed. +the configured OIDC publisher remains allowed. Publishing uses a `push` event on +`main` because npm Trusted Publishing does not support `pull_request_target` +OIDC token exchange reliably. ## Allowed approvers diff --git a/.github/workflows/publish-npm.yml b/.github/workflows/publish-npm.yml index dd95182..7b831b8 100644 --- a/.github/workflows/publish-npm.yml +++ b/.github/workflows/publish-npm.yml @@ -1,11 +1,9 @@ name: Publish npm after approved PR merge on: - pull_request_target: + push: branches: - main - types: - - closed permissions: contents: read @@ -19,7 +17,6 @@ concurrency: jobs: publish: name: Verify and publish - if: github.event.pull_request.merged == true runs-on: ubuntu-latest timeout-minutes: 20 @@ -37,10 +34,41 @@ jobs: .filter(Boolean) ); + const associated = await github.paginate( + github.rest.repos.listPullRequestsAssociatedWithCommit, + { + owner: context.repo.owner, + repo: context.repo.repo, + commit_sha: context.sha, + per_page: 100, + } + ); + const candidates = []; + for (const item of associated) { + if (item.base.ref !== 'main' || item.merge_commit_sha !== context.sha) { + continue; + } + const { data: pull } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: item.number, + }); + if (pull.merged_at && pull.merge_commit_sha === context.sha) { + candidates.push(pull); + } + } + if (candidates.length !== 1) { + core.setFailed( + `Expected exactly one merged pull request for ${context.sha}; found ${candidates.length}.` + ); + return; + } + const pull = candidates[0]; + const reviews = await github.paginate(github.rest.pulls.listReviews, { owner: context.repo.owner, repo: context.repo.repo, - pull_number: context.payload.pull_request.number, + pull_number: pull.number, per_page: 100, }); @@ -52,12 +80,12 @@ jobs: latestByReviewer.set(review.user.login.toLowerCase(), review); } - const author = context.payload.pull_request.user.login.toLowerCase(); + const author = pull.user.login.toLowerCase(); const approval = [...latestByReviewer.entries()].find( ([login, review]) => login !== author && allowed.has(login) && review.state === 'APPROVED' ); - const merger = context.payload.pull_request.merged_by?.login.toLowerCase(); + const merger = pull.merged_by?.login.toLowerCase(); if (approval) { core.info(`Accepted review approval from @${approval[1].user.login}.`); @@ -69,17 +97,18 @@ jobs: ...allowed, ].join(', ')}` ); + return; } - name: Check out the merged commit uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 with: - ref: ${{ github.event.pull_request.merge_commit_sha }} + ref: ${{ github.sha }} fetch-depth: 0 - name: Verify commit is on main env: - MERGE_SHA: ${{ github.event.pull_request.merge_commit_sha }} + MERGE_SHA: ${{ github.sha }} run: | set -euo pipefail test "$(git rev-parse HEAD)" = "$MERGE_SHA" @@ -156,5 +185,5 @@ jobs: else result="already published; skipped" fi - printf '## npm release\n\n- Package: `%s@%s`\n- Result: %s\n' \ + printf "## npm release\n\n- Package: \`%s@%s\`\n- Result: %s\n" \ "$PACKAGE_NAME" "$PACKAGE_VERSION" "$result" >> "$GITHUB_STEP_SUMMARY"