Skip to content

Commit 888091e

Browse files
czlonkowskiclaude
andcommitted
fix(ci): make CJS runtime smoke test strict on every supported Node (#864 review)
The `test:cjs-runtime` one-liner hard-coded `--no-experimental-require-module`, a flag that doesn't exist on Node 18.x or 20.0-20.18 (it was added in v22.0.0 and backported to v20.19.0). On the declared engines floor (>=18) the script would abort with `node: bad option` instead of exercising the loader — a false result on exactly the old Node versions the guard targets. Replace the inline flag with scripts/smoke-cjs-runtime.js, which probes whether the running Node recognizes the flag and applies it only when supported. On Node >= 20.19/22.12 it forces require()-of-ESM off so the mismatch can't be masked; on older Node (already strict by default) it omits the flag. The guard is now strict on every supported Node rather than depending on which Node runs it -- the meta-mistake behind #864. Verified GREEN (loads dist/ on this Node) and RED (exits non-zero with ERR_REQUIRE_ESM against an ESM-only module). Updates the CHANGELOG and CI comment wording to match. Addresses Copilot review comments on #865. Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 901d3e2 commit 888091e

4 files changed

Lines changed: 57 additions & 7 deletions

File tree

.github/workflows/test.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -250,11 +250,11 @@ jobs:
250250
retention-days: 30
251251

252252
# Verify the compiled CommonJS artifact loads under Node's CJS loader (regression guard for #864).
253-
# Node 20.19+/22.12+ silently tolerate require() of ESM-only deps, but the minimum supported
254-
# Node (>=18) and older 20.x/22.x throw ERR_REQUIRE_ESM. --no-experimental-require-module forces
255-
# that strict behavior, so a CJS/ESM mismatch in a shipped dependency (e.g. uuid@14) fails here
256-
# regardless of the runner's Node version. The unit suite runs under Vitest's ESM pipeline and
257-
# cannot catch this; tsc only type-checks.
253+
# Node 20.19+/22.12+ silently tolerate require() of ESM-only deps; scripts/smoke-cjs-runtime.js
254+
# forces the strict loader (--no-experimental-require-module) when the running Node supports it,
255+
# so a CJS/ESM mismatch in a shipped dependency (e.g. uuid@14) fails here regardless of the
256+
# runner's Node version. The unit suite runs under Vitest's ESM pipeline and cannot catch this;
257+
# tsc only type-checks.
258258
cjs-runtime:
259259
runs-on: ubuntu-latest
260260
timeout-minutes: 10

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
### Fixed
1313

1414
- **Server no longer crashes on startup with `ERR_REQUIRE_ESM`** (#864). v2.59.1's `uuid` 10 → 14 bump (#850) moved the dependency to an ESM-only build (`"type": "module"`, no `require` export condition), but the shipped artifact is compiled to CommonJS and calls `require('uuid')`. On the supported minimum Node (`>=18`) — and any Node 20.x before 20.19 or 22.x before 22.12, including the reporter's 22.11 — Node's CommonJS loader throws `ERR_REQUIRE_ESM` at module load, before any config is read, so every MCP client (Claude Code, Cursor, etc.) just saw `MCP error -32000: Connection closed`. It slipped through release verification because `tsc` only type-checks, the Vitest suite runs under an ESM transform pipeline, and CI/Docker/local dev all ran Node ≥ 20.19/22.12 where `require()` of an ESM module is silently tolerated. `uuid` is now pinned to `^11.1.1`, which ships a CommonJS build (a `node.require` export condition) and still clears the original advisory (GHSA-w5hq-g745-h8pq / CVE-2026-41907 does not affect v11), so the security fix from #850 is preserved with no source changes — `v4`/`v5` named exports are API-identical across v10/v11/v14. Reported by @anpe-efficy (André Pereira).
15-
- **Added a CommonJS runtime smoke test** (`npm run test:cjs-runtime` and a dedicated `cjs-runtime` CI job) that loads the compiled `dist/` under `node --no-experimental-require-module`. The flag forces the strict pre-22.12 CommonJS loader behavior, so a CJS/ESM mismatch in any shipped dependency fails CI regardless of the runner's Node version the gap that let #864 reach three releases.
15+
- **Added a CommonJS runtime smoke test** (`npm run test:cjs-runtime` and a dedicated `cjs-runtime` CI job) that loads the compiled `dist/` under the strictest CommonJS loader the running Node supports. On Node ≥ 20.19/22.12 it forces `require()`-of-ESM off via `--no-experimental-require-module` so the mismatch can't be masked; on older Node (down to the `>=18` floor) that strict behavior is already the default and the flag — which doesn't exist there — is omitted. Either way a CJS/ESM mismatch in any shipped dependency fails CI regardless of the runner's Node version, closing the gap that let #864 reach three releases.
1616

1717
Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en
1818

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
"test:ci": "vitest run --coverage --coverage.thresholds.lines=0 --coverage.thresholds.functions=0 --coverage.thresholds.branches=0 --coverage.thresholds.statements=0 --reporter=default --reporter=junit",
4343
"test:watch": "vitest watch",
4444
"test:unit": "vitest run tests/unit",
45-
"test:cjs-runtime": "node --no-experimental-require-module -e \"require('./dist/index.js'); console.log('CJS runtime load OK')\"",
45+
"test:cjs-runtime": "node scripts/smoke-cjs-runtime.js",
4646
"test:integration": "vitest run --config vitest.config.integration.ts",
4747
"test:integration:n8n": "vitest run tests/integration/n8n-api",
4848
"test:cleanup:orphans": "tsx tests/integration/n8n-api/scripts/cleanup-orphans.ts",

scripts/smoke-cjs-runtime.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#!/usr/bin/env node
2+
'use strict';
3+
4+
/**
5+
* CommonJS runtime smoke test — regression guard for #864.
6+
*
7+
* The shipped artifact is compiled to CommonJS and `require()`s its dependencies. If a
8+
* dependency is ESM-only (no `require` export condition), `require()` throws
9+
* ERR_REQUIRE_ESM and the server crashes at startup before any config is read — exactly
10+
* how `uuid@14` broke v2.59.1–2.59.3.
11+
*
12+
* Node >= 20.19 / >= 22.12 enable `require(ESM)` by default, which silently masks the
13+
* mismatch — so just requiring the artifact on a modern Node would NOT catch it. We force
14+
* the strict (pre-`require(ESM)`) loader with `--no-experimental-require-module` so the
15+
* mismatch surfaces regardless of the runner's Node version.
16+
*
17+
* That flag does not exist on older Node (added in v22.0.0, backported to v20.19.0;
18+
* absent in 18.x and 20.0–20.18). On those versions the strict loader is already the
19+
* default, so the flag is unnecessary — and passing it would error with `bad option`. We
20+
* probe for flag support rather than hard-coding the version matrix, so the guard is
21+
* strict on every supported Node (>=18) instead of depending on which Node happens to run
22+
* it (the meta-mistake that produced #864).
23+
*/
24+
25+
const { spawnSync } = require('node:child_process');
26+
const path = require('node:path');
27+
28+
const FLAG = '--no-experimental-require-module';
29+
const entry = path.resolve(__dirname, '..', 'dist', 'index.js');
30+
const program =
31+
`require(${JSON.stringify(entry)}); ` +
32+
`console.log('CJS runtime load OK (node ' + process.versions.node + ')');`;
33+
34+
// Probe: does this Node recognize the strict-loader flag? Run an empty program with it.
35+
const flagSupported =
36+
spawnSync(process.execPath, [FLAG, '-e', ''], { stdio: 'ignore' }).status === 0;
37+
38+
const args = flagSupported ? [FLAG, '-e', program] : ['-e', program];
39+
const result = spawnSync(process.execPath, args, { stdio: 'inherit' });
40+
41+
if (result.status !== 0) {
42+
console.error(
43+
`\nCJS runtime smoke test FAILED (node ${process.versions.node}, strict loader forced: ${flagSupported}).`
44+
);
45+
console.error(
46+
'The compiled dist/ could not be require()d under the CommonJS loader — a shipped ' +
47+
'dependency is likely ESM-only. See #864.'
48+
);
49+
process.exit(result.status === null ? 1 : result.status);
50+
}

0 commit comments

Comments
 (0)