Files
msd-core/tests/allowlist-ratchet.test.cjs
Tom Boucher a28dcec981 chore(#597): replace count-based ratchet guards with AST lint + named-set allowlists (#603)
The windows-test-parity ratchet greps test source for fs.rmSync-without-
maxRetries (and six other Windows-portability anti-patterns), failing when an
integer offender COUNT exceeds a frozen baseline (rmSync: 95). A count ratchet
is a Goodhart metric: fixing one offender and adding another keeps the count
constant, so a new defect slips through green. Replace it — and every other
count ratchet in the repo — with a layered, masking-proof design.

Behavioral seam test
- tests/helpers-cleanup.test.cjs proves helpers.cleanup() carries the Windows
  EBUSY retry budget. cleanup() delegates retries to Node's fs.rmSync via
  maxRetries (it owns no loop), so the test asserts the option contract
  (recursive/force/maxRetries>0/retryDelay>0) + real-FS removal + the cwd-guard,
  rather than a loop that does not exist. The EBUSY risk is now tested ONCE at
  the helper, not approximated textually at every call site.

Write-time ESLint rule (AST-accurate, replaces the grep)
- eslint-rules/no-raw-rmsync-in-tests.cjs (error in tests/**/*.test.cjs) bans
  raw fs.rmSync, steering to cleanup(). Catches member, computed (fs['rmSync']),
  destructured and aliased forms; escape hatch is inline
  `// eslint-disable-next-line local/no-raw-rmsync-in-tests -- <reason>` only.
- Migrated 336 raw fs.rmSync teardown calls across ~116 test files to cleanup().
  ~18 genuinely load-bearing sites (mid-test SUT/fault-injection removals,
  error-swallowing or name-colliding local teardown helpers) keep the raw call
  with an inline eslint-disable + reason.

Shared anti-ratchet primitive
- scripts/lib/allowlist-ratchet.cjs:
  - assertWithinAllowlist: fails on NOVEL ids (new offender introduced) AND on
    STALE ids (a known offender was fixed but not pruned) — identity, not count,
    and a ratchet DOWN toward zero.
  - assertTightCeiling: a size/length budget whose ceiling must stay within a
    grace band of the high-water mark, so budgets may only tighten, never creep.

Ratchets converted onto the primitive
- windows-test-parity-guard.test.cjs: rmSync rule deleted (now ESLint-enforced);
  the remaining six patterns moved from integer baselines to named-set
  allowlists with ratchet-down.
- scripts/lint-test-file-count.{cjs,allowlist.json}: per-module integer counts →
  named filename sets (closes the swap-a-file-keep-the-count blind spot); a
  module dropping under cap now FAILS to force pruning its allowlist entry.
- enh-2790 skill-count `<= 63` → named skill allowlist (ratchets toward ~58).

Size budgets hardened (tighten-only)
- agent-size / workflow-size / feat-3039 help-tiered: ceilings lowered to the
  current high-water mark and an assertTightCeiling anti-creep check added per
  tier. Fixed external-contract limits (description ≤100 chars, agent ≤100 KB)
  are intentionally left as-is — they are not grandfathered creeping budgets.

No user-facing behavior change (tests + tooling only); no USER_FACING_PREFIXES
touched, so no changeset fragment is required.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-01 22:43:49 -04:00

298 lines
9.8 KiB
JavaScript

'use strict';
/**
* Tests for scripts/lib/allowlist-ratchet.cjs
*
* Covers assertWithinAllowlist and assertTightCeiling.
* Uses a non-throwing fake `fail` that records messages into an array so we can
* assert on call count and message content without early-exit on first failure.
*/
const { test, describe } = require('node:test');
const assert = require('node:assert/strict');
const {
assertWithinAllowlist,
assertTightCeiling,
} = require('../scripts/lib/allowlist-ratchet.cjs');
// ─── Fake fail helper ────────────────────────────────────────────────────────
/**
* Returns a { fail, calls } pair. `fail` records its message without throwing,
* so tests can observe every violation rather than stopping at the first.
*/
function makeFail() {
const calls = [];
return {
calls,
fail(msg) {
calls.push(msg);
},
};
}
// ─── assertWithinAllowlist ───────────────────────────────────────────────────
describe('assertWithinAllowlist', () => {
test('clean case: current subset of known, no stale entries — fail never called', () => {
const { fail, calls } = makeFail();
const result = assertWithinAllowlist({
label: 'test-guard',
current: ['a.ts', 'b.ts'],
known: ['a.ts', 'b.ts'],
fail,
});
assert.strictEqual(calls.length, 0, 'fail should not be called');
assert.deepStrictEqual(result.novel, []);
assert.deepStrictEqual(result.stale, []);
});
test('novel detected: id in current but not in known — fail called with that id', () => {
const { fail, calls } = makeFail();
const result = assertWithinAllowlist({
label: 'novel-guard',
current: ['a.ts', 'b.ts', 'c.ts'],
known: ['a.ts', 'b.ts'],
fail,
});
assert.strictEqual(calls.length, 1, 'fail should be called once for novel');
assert.ok(calls[0].includes('c.ts'), 'message should mention the novel id');
assert.ok(
calls[0].includes('fix at the source'),
'message should include fix-at-source guidance'
);
assert.deepStrictEqual(result.novel, ['c.ts']);
assert.deepStrictEqual(result.stale, []);
});
test('stale detected: id in known but not in current — fail called with that id', () => {
const { fail, calls } = makeFail();
const result = assertWithinAllowlist({
label: 'stale-guard',
current: ['a.ts'],
known: ['a.ts', 'b.ts'],
fail,
});
assert.strictEqual(calls.length, 1, 'fail should be called once for stale');
assert.ok(calls[0].includes('b.ts'), 'message should mention the stale id');
assert.ok(
calls[0].includes('ratchets toward zero'),
'message should include ratchet-toward-zero language'
);
assert.deepStrictEqual(result.novel, []);
assert.deepStrictEqual(result.stale, ['b.ts']);
});
test('stale message includes pruneHint when provided', () => {
const { fail, calls } = makeFail();
assertWithinAllowlist({
label: 'prune-guard',
current: ['a.ts'],
known: ['a.ts', 'b.ts'],
fail,
pruneHint: 'edit scripts/my-allowlist.json',
});
assert.ok(
calls[0].includes('edit scripts/my-allowlist.json'),
'message should include the pruneHint'
);
});
test('both novel and stale at once — fail called twice', () => {
const { fail, calls } = makeFail();
const result = assertWithinAllowlist({
label: 'both-guard',
current: ['a.ts', 'c.ts'], // c.ts is new, b.ts is fixed
known: ['a.ts', 'b.ts'],
fail,
});
assert.strictEqual(calls.length, 2, 'fail should be called once for novel and once for stale');
const allMessages = calls.join('\n');
assert.ok(allMessages.includes('c.ts'), 'should mention novel id c.ts');
assert.ok(allMessages.includes('b.ts'), 'should mention stale id b.ts');
assert.deepStrictEqual(result.novel, ['c.ts']);
assert.deepStrictEqual(result.stale, ['b.ts']);
});
test('empty inputs — fail never called', () => {
const { fail, calls } = makeFail();
const result = assertWithinAllowlist({
label: 'empty-guard',
current: [],
known: [],
fail,
});
assert.strictEqual(calls.length, 0);
assert.deepStrictEqual(result.novel, []);
assert.deepStrictEqual(result.stale, []);
});
test('order-independence: Sets and arrays produce the same result', () => {
const callsArr = makeFail();
const callsSet = makeFail();
const resultArr = assertWithinAllowlist({
label: 'order-array',
current: ['z.ts', 'a.ts', 'm.ts'],
known: ['a.ts', 'm.ts'],
fail: callsArr.fail,
});
const resultSet = assertWithinAllowlist({
label: 'order-set',
current: new Set(['z.ts', 'a.ts', 'm.ts']),
known: new Set(['a.ts', 'm.ts']),
fail: callsSet.fail,
});
assert.deepStrictEqual(resultArr.novel, resultSet.novel, 'novel should be identical regardless of input type');
assert.deepStrictEqual(resultArr.stale, resultSet.stale, 'stale should be identical regardless of input type');
assert.deepStrictEqual(resultArr.novel, ['z.ts'], 'novel should be sorted');
});
test('returned novel and stale arrays are sorted', () => {
const { fail } = makeFail();
const result = assertWithinAllowlist({
label: 'sort-guard',
current: ['z.ts', 'a.ts', 'm.ts', 'new.ts'],
known: ['z.ts', 'a.ts', 'm.ts', 'old.ts'],
fail,
});
assert.deepStrictEqual(result.novel, ['new.ts']);
assert.deepStrictEqual(result.stale, ['old.ts']);
});
test('current empty, known non-empty — all known are stale', () => {
const { fail, calls } = makeFail();
const result = assertWithinAllowlist({
label: 'all-stale',
current: [],
known: ['a.ts', 'b.ts'],
fail,
});
assert.strictEqual(calls.length, 1);
assert.deepStrictEqual(result.stale, ['a.ts', 'b.ts']);
assert.deepStrictEqual(result.novel, []);
});
test('known empty, current non-empty — all current are novel', () => {
const { fail, calls } = makeFail();
const result = assertWithinAllowlist({
label: 'all-novel',
current: ['a.ts', 'b.ts'],
known: [],
fail,
});
assert.strictEqual(calls.length, 1);
assert.deepStrictEqual(result.novel, ['a.ts', 'b.ts']);
assert.deepStrictEqual(result.stale, []);
});
});
// ─── assertTightCeiling ──────────────────────────────────────────────────────
describe('assertTightCeiling', () => {
test('actualMax under ceiling within grace — ok, fail never called', () => {
const { fail, calls } = makeFail();
const result = assertTightCeiling({
label: 'size-guard',
actualMax: 90,
ceiling: 100,
grace: 15,
fail,
});
assert.strictEqual(calls.length, 0, 'fail should not be called');
assert.strictEqual(result.ok, true);
assert.strictEqual(result.slack, 10);
});
test('actualMax over ceiling — fail called with regression message', () => {
const { fail, calls } = makeFail();
const result = assertTightCeiling({
label: 'size-guard',
actualMax: 110,
ceiling: 100,
grace: 5,
fail,
});
assert.strictEqual(calls.length, 1, 'fail should be called once');
assert.ok(calls[0].includes('Regression'), 'message should say Regression');
assert.ok(calls[0].includes('110'), 'message should include actualMax');
assert.ok(calls[0].includes('100'), 'message should include ceiling');
assert.strictEqual(result.ok, false);
assert.strictEqual(result.slack, -10);
});
test('ceiling too loose (slack > grace) — fail called with tighten message', () => {
const { fail, calls } = makeFail();
const result = assertTightCeiling({
label: 'loose-guard',
actualMax: 50,
ceiling: 100,
grace: 10,
fail,
});
assert.strictEqual(calls.length, 1, 'fail should be called once');
assert.ok(
calls[0].toLowerCase().includes('tighten') || calls[0].includes('too far'),
'message should mention tightening'
);
assert.ok(calls[0].includes('Budgets may only decrease'), 'message should include budget policy');
assert.strictEqual(result.ok, false);
assert.strictEqual(result.slack, 50);
});
test('boundary: slack === grace — ok (exactly at the grace limit)', () => {
const { fail, calls } = makeFail();
const result = assertTightCeiling({
label: 'boundary-guard',
actualMax: 90,
ceiling: 100,
grace: 10,
fail,
});
assert.strictEqual(calls.length, 0, 'fail should not be called at exact grace boundary');
assert.strictEqual(result.ok, true);
assert.strictEqual(result.slack, 10);
});
test('actualMax equals ceiling — ok, slack is zero', () => {
const { fail, calls } = makeFail();
const result = assertTightCeiling({
label: 'exact-guard',
actualMax: 100,
ceiling: 100,
grace: 0,
fail,
});
assert.strictEqual(calls.length, 0);
assert.strictEqual(result.ok, true);
assert.strictEqual(result.slack, 0);
});
test('grace 0: any slack triggers fail', () => {
const { fail, calls } = makeFail();
assertTightCeiling({
label: 'tight-guard',
actualMax: 99,
ceiling: 100,
grace: 0,
fail,
});
assert.strictEqual(calls.length, 1, 'any slack above 0 should fail when grace is 0');
});
test('label appears in failure messages', () => {
const { fail, calls } = makeFail();
assertTightCeiling({
label: 'my-special-guard',
actualMax: 200,
ceiling: 100,
grace: 5,
fail,
});
assert.ok(calls[0].includes('my-special-guard'), 'label should appear in message');
});
});