fix(1259-01): make real runner cross-platform/version portable (CI ubuntu-24 + windows-24)

CI failed on node 24 + Windows in the real-runner E2E tests — real portability
bugs in the shipping runner, not test flakiness:

- node 24 names a zero-test file's TAP entry by ABSOLUTE path (node 22 used the
  basename), so the exact-string vacuity discriminator misfired -> empty file
  falsely greened. Now compares BASENAMES (separator-agnostic) -> robust across
  OS + node version. Verified: empty file is non-green under node 24 locally.
- the lint-rule runner spawned 'npx', which execFileSync can't launch on Windows
  (and failed fast on ubuntu-24). Now resolves the project's eslint CLI via
  eslint/package.json and runs 'node <cli>' through process.execPath (portable;
  no shell -> no injection). eslint absent -> fail-closed.
- node-test also spawns via process.execPath, not bare 'node'.

Also folds in round-3 WR-01: the probe-core inline comments still said
enforcement was 'deferred to a follow-up PR' — corrected (landed in #1259).
Strengthened the isNonVacuousNodeTestPass unit test to pin the basename clause
(absolute-path target). Verified 71/71 green under BOTH node 22 and node 24.
This commit is contained in:
Dave
2026-06-15 14:19:35 -04:00
parent e08667e5aa
commit 2af7630642
3 changed files with 57 additions and 22 deletions

View File

@@ -397,9 +397,9 @@ export function dispositionForProhibition(
const hasEnforcement = evidence.length > 0;
// FAIL CLOSED: no wired enforcement evidence -> flagged unverified, never green. This holds for
// every tier today (the real enforcement mechanism that could flip a test-tier item to green is
// deferred to a follow-up PR). The guard the safety assertion proves: an unwired item can never
// be silently skipped.
// every tier (the producer that builds enforcement evidence for a test-tier item — the
// `prohibition-enforcement` module — landed in #1259). The guard the safety assertion proves: an
// unwired item can never be silently skipped.
if (!hasEnforcement) {
return {
status: 'unverified',
@@ -414,8 +414,8 @@ export function dispositionForProhibition(
// D4 GUARD: a judgment-tier (or unknown-tier) prohibition is NEVER a silent green from this
// deterministic helper — it always routes to human/LLM judgment review (ADR-550 D4; verify-phase.md).
// Only a test-tier item with wired enforcement evidence may go green, and even that is the deferred
// heavy half until the real negative-test enforcement mechanism lands (no #644 caller passes evidence).
// Only a test-tier item with wired enforcement evidence may go green; the producer that supplies
// that evidence (`prohibition-enforcement`, #1259) runs the wired check and requires a genuine pass.
if (tier === 'test') {
return {
status: 'green',

View File

@@ -36,6 +36,7 @@
*/
import fs from 'node:fs';
import path from 'node:path';
import { execFileSync } from 'node:child_process';
// eslint-disable-next-line @typescript-eslint/no-require-imports
import core = require('./core.cjs');
@@ -104,12 +105,34 @@ export function buildNodeTestArgs(check: CheckDescriptor): string[] {
return ['--test', '--test-reporter=tap', check.target];
}
/** eslint argv (the args AFTER `npx`). Runs the project flat config so plugin rules (e.g. `local/*`)
* load — `--rule` CANNOT load a plugin, so we lint the TARGET path as JSON and filter by rule id.
* `--no-warn-ignored` makes an eslint-IGNORED target return `[]` (not a length-1 "File ignored"
* warning result) so an ignored path fails closed via the vacuity guard instead of falsely greening. */
/** eslint argv (the args AFTER the eslint CLI path). Runs the project flat config so plugin rules
* (e.g. `local/*`) load — `--rule` CANNOT load a plugin, so we lint the TARGET path as JSON and
* filter by rule id. `--no-warn-ignored` makes an eslint-IGNORED target return `[]` (not a length-1
* "File ignored" result) so an ignored path fails closed via the vacuity guard, not a false green. */
export function buildLintArgs(check: CheckDescriptor): string[] {
return ['eslint', '--no-warn-ignored', '--format', 'json', check.target];
return ['--no-warn-ignored', '--format', 'json', check.target];
}
/**
* Resolve the project's eslint CLI entry portably (no `npx` — not spawnable via `execFileSync` on
* Windows). Resolves eslint's package.json from the target project's `node_modules` and derives
* `bin/eslint.js`, so it is run as `node <cli>` (portable). Returns null if eslint is not installed
* (→ the lint-rule check fails closed, never throws).
*/
function resolveEslintCli(cwd: string): string | null {
try {
const pkg = require.resolve('eslint/package.json', { paths: [cwd] });
const cli = path.join(path.dirname(pkg), 'bin', 'eslint.js');
return fs.existsSync(cli) ? cli : null;
} catch {
return null;
}
}
/** Basename of a path, separator-agnostic (handles `\` and `/` so node-test names compare stably
* across OSes / node versions that report the file-test by differing path forms). */
function baseOf(p: string): string {
return typeof p === 'string' ? (p.split(/[\\/]/).pop() ?? p) : '';
}
/**
@@ -156,8 +179,11 @@ export function tapTestNames(out: string): string[] {
export function isNonVacuousNodeTestPass(out: string, target: string): boolean {
const s = parseNodeTestSummary(out);
if (!(s.tests >= 1 && s.pass >= 1 && s.fail === 0)) return false;
const base = typeof target === 'string' ? (target.split(/[\\/]/).pop() ?? target) : '';
return tapTestNames(out).some((n) => n !== base && n !== target);
// Compare BASENAMES: node reports the file-test by varying path forms across OS / node version
// (absolute, relative, normalized), so an exact-string compare misfires. A real test name (e.g.
// "guards the must-NOT") has no separators, so its basename never equals the target file's.
const tgtBase = baseOf(target);
return tapTestNames(out).some((n) => baseOf(n) !== tgtBase);
}
/** Number of file results in an eslint `--format json` report (0 if unparseable / not an array). */
@@ -199,10 +225,13 @@ export function eslintJsonHasRule(jsonText: string, rule: string): boolean {
* AND a reported test named distinctly from the file). A bare exit 0 for an empty/zero-test file
* — which `node --test` counts as one passing "test" named after the file — is NOT a pass (the
* #1259 BL-01 false-green fix).
* - lint-rule: runs the project `eslint --format json <target>` (flat config loads `local/*`
* plugins) and requires the target to actually lint (>=1 file result) AND ZERO messages for the
* specific rule id. `--rule` cannot load a plugin rule, so we filter the structured report by
* `ruleId` instead (the #1259 SF-01 fix).
* - lint-rule: runs the project eslint as `node <eslint-cli> --format json <target>` (flat config
* loads `local/*` plugins) and requires the target to actually lint (>=1 file result) AND ZERO
* messages for the specific rule id. `--rule` cannot load a plugin rule, so we filter the
* structured report by `ruleId` instead (the #1259 SF-01 fix).
*
* Both kinds spawn via `process.execPath` (never bare `node`/`npx` — not portably spawnable via
* `execFileSync` on Windows) with arg arrays (no shell → no injection from a caller-supplied target).
*/
/**
* Env for spawned checks: strip `NODE_TEST_CONTEXT` and `NODE_OPTIONS` so an AMBIENT test-runner
@@ -222,7 +251,7 @@ function defaultRunCheck(check: CheckDescriptor, cwd: string): CheckRunResult {
if (check.kind === 'node-test') {
let out = '';
try {
out = execFileSync('node', buildNodeTestArgs(check), {
out = execFileSync(process.execPath, buildNodeTestArgs(check), {
cwd,
encoding: 'utf-8',
stdio: ['ignore', 'pipe', 'pipe'],
@@ -231,16 +260,18 @@ function defaultRunCheck(check: CheckDescriptor, cwd: string): CheckRunResult {
});
} catch (e) {
// A failing test run exits non-zero (TAP still on stdout). Parse it: a real failure has
// `# fail >= 1` -> non-vacuous check returns false. Missing `node` -> no stdout -> false.
// `# fail >= 1` -> non-vacuous check returns false. Missing node -> no stdout -> false.
const stdout = e && typeof e === 'object' && 'stdout' in e ? (e as { stdout?: unknown }).stdout : '';
out = typeof stdout === 'string' ? stdout : '';
}
return { passed: isNonVacuousNodeTestPass(out, check.target) };
}
if (check.kind === 'lint-rule') {
const eslintCli = resolveEslintCli(cwd);
if (!eslintCli) return { passed: false }; // eslint not installed -> fail closed, never throw
let json = '';
try {
json = execFileSync('npx', buildLintArgs(check), {
json = execFileSync(process.execPath, [eslintCli, ...buildLintArgs(check)], {
cwd,
encoding: 'utf-8',
stdio: ['ignore', 'pipe', 'pipe'],

View File

@@ -99,7 +99,6 @@ describe('prohibition-enforcement: deterministic test-tier producer (#1259 / ADR
'must export buildLintArgs — the eslint argv builder for the lint-rule real runner');
const argv = enforce.buildLintArgs({ kind: 'lint-rule', rule: 'local/no-source-grep', target: 'tests/' });
assert.ok(Array.isArray(argv), 'argv is an array');
assert.equal(argv[0], 'eslint');
const fmtIdx = argv.indexOf('--format');
assert.ok(fmtIdx !== -1 && argv[fmtIdx + 1] === 'json',
'emits --format json so the report can be filtered by ruleId');
@@ -241,9 +240,14 @@ describe('prohibition-enforcement real-runner helpers (#1259)', () => {
const empty = 'ok 1 - empty.test.cjs\n1..1\n# tests 1\n# pass 1\n# fail 0\n';
assert.equal(enforce.isNonVacuousNodeTestPass(empty, 'empty.test.cjs'), false,
'a file-named-only result is vacuous — the BL-01 false-green guard');
// BASENAME-NORMALIZED: node may report the file-test by an ABSOLUTE/normalized path while the
// descriptor target is relative (cross-OS / node-version). The basenames must still match → vacuous.
const emptyAbs = 'ok 1 - /tmp/x/empty.test.cjs\n1..1\n# tests 1\n# pass 1\n# fail 0\n';
assert.equal(enforce.isNonVacuousNodeTestPass(emptyAbs, 'empty.test.cjs'), false,
'an absolute-path file-test name must still be recognized as vacuous (basename compare, WR-02)');
const real = 'ok 1 - guards the must-NOT\n1..1\n# tests 1\n# pass 1\n# fail 0\n';
assert.equal(enforce.isNonVacuousNodeTestPass(real, 'neg.test.cjs'), true,
'a real named test distinct from the file is a genuine pass');
assert.equal(enforce.isNonVacuousNodeTestPass(real, '/abs/path/neg.test.cjs'), true,
'a real named test distinct from the file is a genuine pass (even vs an absolute target)');
const failing = 'not ok 1 - guards\n# tests 1\n# pass 0\n# fail 1\n';
assert.equal(enforce.isNonVacuousNodeTestPass(failing, 'neg.test.cjs'), false,
'any failure means not a pass');