feat(ci): add floor-node-smoke exact-floor override gate - #293
feat(ci): add floor-node-smoke exact-floor override gate#293John-David Dalton (jdalton) wants to merge 1 commit into
Conversation
7490f09 to
6ee94a7
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
Done
Or push these changes by commenting:
@cursor push 618d9c7c26
Preview (618d9c7c26)
diff --git a/scripts/npm/floor-node-smoke.mts b/scripts/npm/floor-node-smoke.mts
--- a/scripts/npm/floor-node-smoke.mts
+++ b/scripts/npm/floor-node-smoke.mts
@@ -26,6 +26,8 @@
import { getDefaultLogger } from '@socketsecurity/lib-stable/logger/default'
import { spawnSync } from '@socketsecurity/lib-stable/process/spawn/child'
+import { isMainModule } from '../fleet/_shared/is-main-module.mts'
+
const logger = getDefaultLogger()
// The floor is the low edge of packages/npm/*'s `engines.node: >=24` range.
@@ -72,7 +74,8 @@
* materializes at the archive path, let alone extracts.
*/
async function ensureFloorNode(): Promise<string> {
- const platKey = `${process.platform}-${process.arch}`
+ const platform = process.platform === 'win32' ? 'win' : process.platform
+ const platKey = `${platform}-${process.arch}`
const pin = FLOOR_NODE_PLATFORMS[platKey]
if (!pin) {
throw new Error(
@@ -208,7 +211,7 @@
// Entrypoint-guarded: floor-node-pin-matches-engines (check --all) imports
// FLOOR_NODE_VERSION from this module, which must not trigger a download.
-if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) {
+if (isMainModule(import.meta.url)) {
main().catch((e: unknown) => {
logger.error(e)
process.exitCode = 1
diff --git a/scripts/repo/check/floor-node-pin-matches-engines.mts b/scripts/repo/check/floor-node-pin-matches-engines.mts
--- a/scripts/repo/check/floor-node-pin-matches-engines.mts
+++ b/scripts/repo/check/floor-node-pin-matches-engines.mts
@@ -22,6 +22,7 @@
import { getDefaultLogger } from '@socketsecurity/lib-stable/logger/default'
+import { isMainModule } from '../../fleet/_shared/is-main-module.mts'
import { FLOOR_NODE_VERSION } from '../../npm/floor-node-smoke.mts'
const logger = getDefaultLogger()
@@ -118,6 +119,6 @@
return 0
}
-if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) {
+if (isMainModule(import.meta.url)) {
void main()
}You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6ee94a7. Configure here.
| logger.error(e) | ||
| process.exitCode = 1 | ||
| }) | ||
| } |
There was a problem hiding this comment.
Entrypoint guard never runs main
High Severity
The import.meta.url guard in floor-node-smoke.mts prevents main() from executing when invoked with relative paths, on Windows, or via symlinks. This causes the script to exit successfully without running its checks, resulting in a silent false positive for the floor node smoke test.
Reviewed by Cursor Bugbot for commit 6ee94a7. Configure here.
| `no floor-node pin for ${platKey} — add its asset + sha256 (from ` + | ||
| `https://nodejs.org/dist/v${FLOOR_NODE_VERSION}/SHASUMS256.txt) to FLOOR_NODE_PLATFORMS`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Windows floor pin key mismatch
High Severity
The ensureFloorNode function currently fails on Windows. The platKey it constructs (win32-x64) doesn't align with the win-x64 entry in FLOOR_NODE_PLATFORMS, so it can't find the correct Node.js binary to download and install, leading to a script error.
Reviewed by Cursor Bugbot for commit 6ee94a7. Configure here.
| logger.error(`FAIL ${failures[i]}`) | ||
| } | ||
| process.exitCode = 1 | ||
| } |
There was a problem hiding this comment.
Smoke passes with zero imports
Medium Severity
The gate only fails when failures is non-empty. If every override is skipped or none are imported, it still exits 0, so a broken resolver or empty package scan stays green while proving nothing — the same silent-empty class this PR fixed for the fleet Test job.
Reviewed by Cursor Bugbot for commit 6ee94a7. Configure here.
6ee94a7 to
9025bcf
Compare



Every override in
packages/npm/*promisesengines.node: >=24, but nothing in CI has ever run them on Node 24. The existing jobs run a newer current-line Node, so an override that quietly uses newer-than-floor syntax or a newer-than-floor API stays green here and only breaks inside a consumer that pinned the floor — which is exactly the consumer theenginesfield was written for.This adds a CI job that actually executes the promise.
floor-node-smokeside-installs the exact floor Node release and dynamic-imports every override's node entry under that binary. If an override cannot even be loaded on the Node it claims to support, the job goes red here instead of in somebody's install.A second, cheaper gate keeps the promise honest over time:
floor-node-pin-matches-enginesfails if the version the smoke job installs ever drifts from the floor the overrides declare.The smoke job — an exact, sha256-verified side install that never touches
PATHThe pinned floor is
24.0.0, with a per-platform asset name and sha256 fordarwin-arm64,darwin-x64,linux-x64andwin-x64. The download is verified against the pinned hash before extraction, so a tampered or truncated archive never materializes at the archive path, let alone unpacks.It is a side install: the floor binary is never added to
PATH. The harness keeps running on the repo's own Node and spawns the floor binary explicitly, so the two runtimes cannot be confused for one another.For each
packages/npm/*package it resolves the node entry out of theexportsmap (the overrides ship nomain), preferring thenode,import,require,defaultconditions in that order, and dynamic-imports it under the floor binary. Dynamic import is used because it reaches every entry kind a floor-pinned ESM consumer can —.cjs,.js, and.json(which needs the import attribute;dateandes-iterator-helpersship JSON entries). Packages with no node entry — bin-only or asset packages — are counted as skipped rather than failed.New
pnpm run test:npm:floorruns it locally.FLOOR_NODE_DIRoverrides the install directory, which otherwise defaults toRUNNER_TEMPand falls back to the OS temp dir.The lock-step check — because a green gate proving the WRONG floor is worse than no gate
The smoke job's whole premise is that its pin is the low edge of every override's
engines.noderange. That used to be a comment saying "bump BOTH together". An engines bump without a pin bump, or the reverse, would leave the job green while proving a floor nobody declared.scripts/repo/check/floor-node-pin-matches-engines.mtsenforces the version half of that lock-step and is auto-registered intocheck --all. It fails when an override declares noengines.nodeat all, when the range has no resolvable minimum, or when the minimum satisfying version is anything other than the pin — and it reports every drifted override, not just the first. The sha256 half needs no separate check: the download hard-fails on a hash mismatch at runtime.The pure classification function is separated from the directory walking, and ships with five unit tests over synthetic fixtures covering both arms of each case: in lock-step, drifted above and below, missing
engines.node, and a malformed range. The smoke script's entrypoint is guarded so the check can importFLOOR_NODE_VERSIONfrom it without triggering a download.Scope change — the original inlining goal is dropped
This PR started out also wanting to inline
ci.yml.mainhas since inlined it via a more-hardened inline git-fetch bootstrap, so that half is dropped and this is rescoped to just the new gate, layered on top ofmain's current workflow.