From 50d88f768b927ada5eb397ca798f0145f23a5fa3 Mon Sep 17 00:00:00 2001 From: windka Date: Tue, 18 Aug 2026 15:51:29 +0200 Subject: [PATCH] ci: gate the publish job, keep --test-force-exit (verified handle leak) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picks up part of the node-red-standards 0.4.1 fixes. npm-publish.yml had one job that ran `npm ci`, `npm test`, `npm publish` in sequence — no lint, no format:check, and no job separation, so nothing stood between a tag and an irreversible publish except the test suite. Replaced with the standard's two-job shape: `build` runs lint, format:check and test, and `publish-npm` declares `needs: build`. A release is cut from a tag and nothing guarantees that tag points at a commit CI ever saw. It also published with a plain `npm publish`, so a release flagged pre-release would have gone to `latest` and auto-upgraded every Manage Palette user onto an unproven build; `latest` cannot be walked back by publishing, only by a separate dist-tag change. Now routed to the `beta` dist-tag. Node stays on 20.x rather than the template's 22.x, which exists for repos that raised their floor above the standard's >=20 baseline; this package declares >=20. standards-check.yml already used the correct Git-resolved invocation, so this is only a refresh from the template: same command, plus the `contents: read` permissions block. The test script deliberately KEEPS --test-force-exit, against the standard. The standard dropped the flag because it masks handle leaks, and this repo turns out to have one: without it the suite hangs rather than exiting. Measured — full run killed at the 480 s timeout (exit 124) after 204 passing tests with over seven minutes of no output, and the culprit reproduces in isolation: node --test --test-timeout=30000 --test-concurrency=1 test/login-endpoints.test.js -> 16 pass, then hangs (exit 124) --test-timeout does not catch it, because the leak holds the process open after the tests themselves have finished. So this is exactly the trade-off the standard documents: the flag is what keeps the run from hanging, and removing it has to wait until the open handle in test/login-endpoints.test.js is closed. Keeping it is a known, recorded deviation rather than an oversight. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/npm-publish.yml | 31 ++++++++++++++++++++++++--- .github/workflows/standards-check.yml | 14 +++++++++--- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index 156787b..a2f3fe2 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -2,19 +2,44 @@ name: Publish to npm on: release: + # 'published' — not 'created', which also fires when a *draft* release is saved and + # would push unreleased code to npm. types: [published] jobs: - publish: + # The same gate as CI, re-run here on purpose. A release is cut from a tag, and nothing + # guarantees that tag points at a commit CI ever saw — so verify before publishing rather + # than assume. npm publish is irreversible: a version cannot be replaced once taken. + build: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - uses: actions/setup-node@v7 with: + # This package declares engines >=20, so 20.x is its own baseline. The + # template pins 22.x for repos that raised their floor above 20. node-version: 20.x - registry-url: https://registry.npmjs.org - run: npm ci + - run: npm run lint + - run: npm run format:check - run: npm test - - run: npm publish + + publish-npm: + # Without this the publish runs regardless of the checks above. + needs: build + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-node@v7 + with: + node-version: 20.x + registry-url: https://registry.npmjs.org + - run: npm ci + # A GitHub release flagged "pre-release" goes to the `beta` dist-tag, so users + # on Manage Palette — which tracks `latest` — are not auto-upgraded onto an + # unproven build. A plain `npm publish` would move `latest` to the beta and + # push it to every install; `latest` cannot be walked back to an earlier + # version by publishing, only by a separate dist-tag change. + - run: npm publish ${{ github.event.release.prerelease && '--tag beta' || '' }} env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} diff --git a/.github/workflows/standards-check.yml b/.github/workflows/standards-check.yml index 96ce4bf..6f4f531 100644 --- a/.github/workflows/standards-check.yml +++ b/.github/workflows/standards-check.yml @@ -1,10 +1,14 @@ -# Optional: drop into a target repo as .github/workflows/standards-check.yml +# Synced into target repos by `nrstd sync` as .github/workflows/standards-check.yml. # Fails CI if the repo drifts from the shared standard. Tool-neutral (no AI). +# `nrstd audit` exits non-zero below 10/10, so a repo adopting this should run +# `nrstd sync --write` first — otherwise its next push goes red on pre-existing drift. name: Standards check on: push: branches: [master, main] pull_request: +permissions: + contents: read jobs: standards: runs-on: ubuntu-latest @@ -13,6 +17,10 @@ jobs: - uses: actions/setup-node@v7 with: node-version: 20.x - # node-red-standards is not published to npm — install it from the repo, the same way - # it is invoked locally. `npx --yes node-red-standards` fails with E404. + # Resolved from Git, not the npm registry: this package is deliberately + # unpublished (README "Install", option B), so a bare + # `npx --yes node-red-standards` fails every run with E404 before it can + # audit anything — which reads as drift when it is really a missing + # package. The repo is public, so no token is needed. If it is ever + # published, the short form becomes available and this can go back. - run: npx --yes github:windkh/node-red-standards audit