fix(1259-01): close re-review NEW-BL-01 (ignored-target vacuous green) + no-throw hardening
Round-2 adversarial review found the lint-rule runner falsely greened an eslint-IGNORED target: eslint returns a length-1 "File ignored" result (ruleId null) that passed the >=1-file vacuity guard while nothing was linted — reopening the vacuous-green class. Fix: buildLintArgs now passes --no-warn-ignored so an ignored path returns [] -> fails closed (verified + E2E test on an ignored bin/lib artifact). Also: wrap runCheck() so even a (test-injected) throwing runner fails closed (NEW-WR-01, full no-throw contract); document the benign basename-naming constraint on wired node-test names (NEW-WR-02, fail-closed).
This commit is contained in:
@@ -105,9 +105,11 @@ export function buildNodeTestArgs(check: CheckDescriptor): string[] {
|
||||
}
|
||||
|
||||
/** 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. */
|
||||
* 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. */
|
||||
export function buildLintArgs(check: CheckDescriptor): string[] {
|
||||
return ['eslint', '--format', 'json', check.target];
|
||||
return ['eslint', '--no-warn-ignored', '--format', 'json', check.target];
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -145,6 +147,11 @@ export function tapTestNames(out: string): string[] {
|
||||
* with ZERO `test()` calls as one passing "test" named after the file, so the counts alone cannot
|
||||
* tell an empty/deleted negative test from a real one (the #1259 BL-01 false-green). Requiring a
|
||||
* named test distinct from the file closes that hole.
|
||||
*
|
||||
* KNOWN CONSTRAINT (fail-closed, not a hole): a real test whose `test('...')` name is EXACTLY the
|
||||
* target file's basename emits TAP indistinguishable from an empty file and is conservatively
|
||||
* rejected (non-green). A wired negative test must carry a descriptive name, not be named after its
|
||||
* own file — a benign authoring constraint, and the safe direction if violated.
|
||||
*/
|
||||
export function isNonVacuousNodeTestPass(out: string, target: string): boolean {
|
||||
const s = parseNodeTestSummary(out);
|
||||
@@ -296,7 +303,14 @@ export function runProhibitionEnforcement(
|
||||
// fail-first (that needs a violation fixture — tracked follow-up, ADR-550 D5d). A non-attested or
|
||||
// non-passing check hard-gates (never green) in BOTH modes.
|
||||
const attestedFailFirst = c.failFirst === true;
|
||||
const run = runCheck(c);
|
||||
// No-throw contract end-to-end: even a (test-injected) runCheck that throws must fail closed,
|
||||
// never propagate. The default real runner already never throws.
|
||||
let run: CheckRunResult;
|
||||
try {
|
||||
run = runCheck(c);
|
||||
} catch {
|
||||
run = { passed: false };
|
||||
}
|
||||
const passed = attestedFailFirst && run.passed === true;
|
||||
|
||||
if (!passed) {
|
||||
|
||||
@@ -103,6 +103,8 @@ describe('prohibition-enforcement: deterministic test-tier producer (#1259 / ADR
|
||||
const fmtIdx = argv.indexOf('--format');
|
||||
assert.ok(fmtIdx !== -1 && argv[fmtIdx + 1] === 'json',
|
||||
'emits --format json so the report can be filtered by ruleId');
|
||||
assert.ok(argv.includes('--no-warn-ignored'),
|
||||
'must pass --no-warn-ignored so an eslint-ignored target returns [] (fails closed), not a length-1 warning result');
|
||||
assert.ok(!argv.includes('--rule'),
|
||||
'must NOT use --rule — it cannot load a plugin rule like local/no-source-grep (the SF-01 bug)');
|
||||
assert.equal(argv[argv.length - 1], 'tests/', 'the LAST arg is the lint target path');
|
||||
@@ -146,6 +148,18 @@ describe('prohibition-enforcement: deterministic test-tier producer (#1259 / ADR
|
||||
assert.equal(result.flagged, true);
|
||||
});
|
||||
|
||||
test('a runCheck that THROWS fails closed, never propagates (no-throw contract, NEW-WR-01)', () => {
|
||||
const enforce = require(ENFORCEMENT_LIB);
|
||||
const result = enforce.runProhibitionEnforcement(
|
||||
TEST_TIER,
|
||||
{ kind: 'node-test', target: 'tests/neg.test.cjs', failFirst: true },
|
||||
{ runCheck: () => { throw new Error('runner blew up'); } },
|
||||
);
|
||||
assert.notEqual(result.status, 'green', 'a throwing runner must never green');
|
||||
assert.equal(result.flagged, true);
|
||||
assert.equal(result.located, true);
|
||||
});
|
||||
|
||||
test('hard-gates in BOTH modes on a failing check (ADR-550 D4)', () => {
|
||||
const enforce = require(ENFORCEMENT_LIB);
|
||||
for (const mode of ['interactive', 'autonomous']) {
|
||||
@@ -303,4 +317,17 @@ describe('prohibition-enforcement REAL runner end-to-end (#1259)', () => {
|
||||
assert.equal(result.kind, 'lint-rule');
|
||||
assert.equal(result.evidence[0].rule, 'local/no-source-grep');
|
||||
});
|
||||
|
||||
test('an eslint-IGNORED target does NOT green the lint-rule kind (vacuous-green guard, NEW-BL-01)', () => {
|
||||
const enforce = require(ENFORCEMENT_LIB);
|
||||
// The generated bin/lib artifact is eslint-ignored. Without --no-warn-ignored, eslint returns a
|
||||
// length-1 "File ignored" result that would falsely pass the vacuity guard. It must fail closed.
|
||||
const result = enforce.runProhibitionEnforcement(
|
||||
TEST_TIER,
|
||||
{ kind: 'lint-rule', rule: 'local/no-source-grep', target: 'gsd-core/bin/lib/prohibition-enforcement.cjs', failFirst: true },
|
||||
{ cwd: process.cwd() },
|
||||
);
|
||||
assert.notEqual(result.status, 'green', 'an ignored path lints nothing — must NEVER green');
|
||||
assert.equal(result.located, true, 'the descriptor was well-formed; it just did not genuinely pass');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user