From 2ff86e9ae40ada9aeb3be35b8d16dd1808149b2d Mon Sep 17 00:00:00 2001 From: Rome-1 Date: Tue, 8 Sep 2026 18:40:59 -0700 Subject: [PATCH] ci: test Node 18 with a smoke test of the built artifact, not with a runner that refuses it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cross-platform (ubuntu-latest, 18) leg failed #237 on verify-hook-liveness — expected null to be 'deny'. That probe IS rf-fuwy, so the question mattered: does the PROBE fail on Node 18, or only the test? Opposite answers. MEASURED, on a real Node 18.20.8: the SHIPPED dist, exported runConfiguredHook, 200 iterations v18.20.8 200/200 read "deny" v24.14.0 200/200 read "deny" the raw spawnSync call, isolated from vitest and TS, 700 iterations 0 failures on either version, byte-identical results the built CLI node ./dist/index.js --version -> 0.10.1 the failing test file, in isolation on 18 4/4 PASS So the probe does not fail on Node 18 and users are not affected. The cause is structural: vitest 4.1.0 engines.node = ^20.0.0 || ^22.0.0 || >=24.0.0 matrix node: ["18", "20", "22"] vitest 4 does not run on Node 18. That leg ran the full 2194-test suite on a runner that declares it will not run there, which is why 18 alone failed and why it failed intermittently — 89 of 90 files passed. WHAT THIS CHANGES, AND WHAT IT DOES NOT. engines stays >=18: the shipped code works there, measured. What was dishonest was a matrix leg claiming to test Node 18 while running a runner that refuses it, so 18 comes out of the vitest matrix and is replaced by node18-smoke — build on 20, RUN on 18, which is the user's situation since they install a prebuilt package rather than compiling one. The smoke asserts the rf-fuwy probe BOTH ways: a live gate yields its decision, an inert one yields none. Both directions mutation-checked — blanking the live fixture gives `FAIL: probe read null, expected "deny"`, and making the inert fixture work gives `FAIL: an inert gate was not reported inert`. My first mutation attempt was itself vacuous (the mutants exited 1 on a module-not-found, not on the assertion); these run in-tree so the import resolves and the failure is the real one. Gated on run_core, not the `run` that gated cross-platform: run_core is true on every PR, and a skipped job satisfies a required check. NOT REPRODUCED, stated rather than glossed: I could not make the probe miss once in 900 attempts at load 12-16, so I cannot name the exact CI mechanism. Consistent with all three spawn attempts exhausting on a 2-core runner under a full-suite fork storm. The residual's DIRECTION is the safe one — a miss reports a live gate as dead, never a dead gate as live. poe is chasing a separate finding about the fixture never reading stdin; that is a real question about the fixture and is beaded on its own. --- .github/workflows/test-comprehensive.yml | 60 +++++++++++++++++++++++- node/scripts/node18-smoke.mjs | 27 +++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 node/scripts/node18-smoke.mjs diff --git a/.github/workflows/test-comprehensive.yml b/.github/workflows/test-comprehensive.yml index 0aa3be0..598d584 100644 --- a/.github/workflows/test-comprehensive.yml +++ b/.github/workflows/test-comprehensive.yml @@ -382,6 +382,58 @@ jobs: echo "Hook installation verified" # ── Cross-platform smoke test ────────────────────────────────────── + # Node 18 coverage that means something. package.json declares engines >=18 + # and the shipped code genuinely works there — but vitest 4 cannot run on 18 + # (engines ^20 || ^22 || >=24), so the old matrix leg tested the RUNNER's + # unsupported path and reported on neither. This runs what a Node 18 USER + # runs: the compiled dist, under Node 18, with no test framework in the way. + # + # Built on 20 and RUN on 18 on purpose — that is the user's situation, since + # they install a prebuilt package rather than compiling it. + # + # Gated on `run_core`, not `run` like cross-platform was: run_core is true on + # every PR. A skipped job satisfies a required check, so Node 18 coverage that + # only sometimes runs is Node 18 coverage that can be absent exactly when it + # matters. + node18-smoke: + needs: gate + if: needs.gate.outputs.run_core == 'true' + runs-on: ubuntu-latest + defaults: + run: + working-directory: ./node + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: "20" + + - name: Enable pnpm + run: corepack enable && corepack prepare pnpm@10 --activate + + - name: Install and build on a supported Node + run: | + pnpm install --frozen-lockfile + pnpm run build + + # Switch the runtime only — the artifact under test is the one built above. + - uses: actions/setup-node@v4 + with: + node-version: "18" + + - name: The CLI runs on Node 18 + run: | + node --version + node ./dist/index.js --version + + # Asserts the rf-fuwy liveness probe BOTH ways: a live gate yields its + # decision, an inert one yields none. Both directions are mutation-checked + # — blanking the live fixture or making the inert one work fails the + # script with its own message, not an import error. + - name: The rf-fuwy liveness probe works on Node 18 + run: node scripts/node18-smoke.mjs + cross-platform: needs: gate if: needs.gate.outputs.run == 'true' @@ -389,7 +441,13 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest] - node: ["18", "20", "22"] + # vitest 4 declares engines ^20 || ^22 || >=24 — it does not run on Node + # 18, so an 18 leg here exercised the RUNNER's unsupported path and said + # nothing about the shipped code. It failed intermittently for exactly + # that reason (1 test of 2194, only on 18). Node 18 remains supported by + # the PRODUCT — package.json engines >=18, measured 200/200 on the built + # dist — and node18-smoke below is what actually tests that claim. + node: ["20", "22"] runs-on: ${{ matrix.os }} defaults: run: diff --git a/node/scripts/node18-smoke.mjs b/node/scripts/node18-smoke.mjs new file mode 100644 index 0000000..cf9d45a --- /dev/null +++ b/node/scripts/node18-smoke.mjs @@ -0,0 +1,27 @@ +// Node 18 smoke test of the BUILT artifact — deliberately not a vitest test. +// +// vitest 4 declares engines ^20 || ^22 || >=24, so it cannot run on Node 18 at +// all; the old cross-platform matrix leg was therefore exercising the runner's +// unsupported path rather than the product, and said nothing about either. But +// package.json still declares engines >=18, and a support claim nothing checks +// is a claim, not a guarantee. +// +// So this runs what a Node 18 USER runs: the compiled dist, under Node 18, with +// no test framework in the way. It asserts the rf-fuwy liveness probe both ways +// — a live gate yields its decision, an inert one yields none — because that is +// the property the release exists to establish, and the one place Node 18 +// coverage would actually matter. +import { runConfiguredHook } from "../dist/commands/agent/verify.js"; +const cmd = `printf '%s' '{"hookSpecificOutput":{"permissionDecision":"deny"}}'`; +const r = runConfiguredHook(cmd, "rm -rf / --no-preserve-root"); +if (r.decision !== "deny") { + console.error(`FAIL: probe read ${JSON.stringify(r.decision)}, expected "deny"`); + console.error(JSON.stringify(r)); + process.exit(1); +} +const dead = runConfiguredHook("rafter-does-not-exist-9c1f hook pretool", "rm -rf /"); +if (dead.decision !== null || dead.status === 0) { + console.error(`FAIL: an inert gate was not reported inert: ${JSON.stringify(dead)}`); + process.exit(1); +} +console.log(`OK on ${process.version}: live gate -> "deny", inert gate -> no decision, status ${dead.status}`);