Files
msd-core/tests/no-unbounded-spawn-allowlist.test.cjs
Tom Boucher 3fac6e629f test(#3145): bound the installer/runtime cluster onto the process seam (#3176)
* test(#3145): bound the installer/runtime cluster onto the process seam

Migrates 156 unbounded sync spawn sites across 47 files. Allowlist 120 to 73.

Timeouts are sized from evidence already in the tree rather than a house
default, because this wave spawns installers rather than git plumbing and an
undersized bound does not catch a hang -- it manufactures CI flake, which is
worse, since a flake gets re-run instead of investigated. install.test.cjs
records a real spawnSync ETIMEDOUT at a 60000ms cap on a loaded bench while
another lane passed the same commit in 12.7s, so full installs are bound at
120000ms against that recorded incident.

Also adds an auditable escape to the guard's timeout ceiling. The 600000ms
cap was set in #3143 from partial evidence, but fragment-single-edit-
propagation carries a documented, load-tested 900000ms bound on a run that
chains a full build plus eight generators -- the guard would have rejected a
correct timeout the moment that file left the allowlist. A value above the
ceiling is now permitted only with an inline allow-spawn-timeout-ceiling
marker carrying a non-empty reason. It raises the ceiling; it never waives
the requirement for a bound, which is asserted directly.

install-shared.cjs keeps its hand-rolled assert rather than routing through
throwIfFailed: its message embeds both streams, and throwIfFailed carries
only a trimmed stderr. The message now also names the outcome, so a bounded
timeout reads as such across its 38 importers instead of as
expected null to equal 0.

* test(#3145): extract class-norm timeouts and correct the build-hooks sizing

A pre-PR review found 52 copies of four class-norm timeout constants across
this wave. These are not per-suite fixture bindings -- they are shared facts
about how long a class of subprocess takes, derived from a recorded bench
incident. That norm already moved once (60000 to 120000 after a real
ETIMEDOUT), and 52 copies would have drifted the next time it moved.

Extracts tests/helpers/timeouts.cjs, where each norm is justified once, and
converts the copies. A site that genuinely differs -- a real tsc compile, or
regen:derived -- keeps its own local constant with its own justification.

Also corrects a misclassification: scripts/build-hooks.js was sized as a
build at 120000 in twelve places and 60000 in another, but it compiles and
bundles nothing. Its own header says no bundling needed; it copies pre-built
files and syntax-checks them with vm. Three different values bounded one
script; now there is one.

* test(#3145): fix red CI — lint self-match and a Windows chunk overrun

Two failures on PR 3176.

lint-allow-test-rule-refs read a RuleTester fixture as a real exemption. The
fixture exists to prove an unrelated marker does NOT suppress the rule, so it
carries that marker's literal text as test data. Split via concatenation, the
same idiom no-unbounded-spawn-allowlist.test.cjs already uses for its own
self-match problem. The explanatory comment needed the same treatment.

The Windows shard 3/3 chunk was killed at its 600000ms budget. Output stopped
seven minutes before the kill, so this was an overrun rather than a slow
chunk: regenDerivedPropagatesSingleFragmentEditWithNoSecondSourceSurface runs
regen:derived bounded at 900000ms, which is larger than the whole chunk
budget, so the chunk killer always fires first and it can never complete
there. Both the test and that bound predate this change; modifying the file
pulled it into the Windows targeted set and exposed it. Skipped on Windows
with the reason recorded; the Linux lanes cover it. The 900000 bound and its
ceiling marker are unchanged -- they are correct.

* test(#3145): refresh the stale test-timings cost table

The Windows shard was killed at its 600000ms per-chunk budget. run-tests.cjs
packs chunks by measured duration from tests/test-timings.json, and an
unknown file falls back to the table's median weight -- advisory by design,
but it silently underweights exactly the files that matter.

Four of the failing chunk's 22 files were absent from the table, including
the two heaviest: fragment-single-edit-propagation.install.test.cjs at 230s
(it runs regen:derived) and agent-fragments-emission.install.test.cjs at 79s.
Both were weighted as average, so the chunk's total weight read 53.68 against
a budget of 60 and the packer produced a single chunk.

Regenerated from a passing full-suite run, per the remedy the script itself
documents. 700 to 770 entries, 70 added, 0 dropped -- verified, since
gen-test-timings.cjs replaces the table wholesale rather than merging.

Proven against the real packer: the same 22 files now weigh 103.91 and split
into two chunks. No logic, budget, or timeout was changed; raising a budget
to make a red gate pass is not a fix.

---------

Co-authored-by: sim <sim@local>
2026-08-07 15:18:18 -04:00

169 lines
6.6 KiB
JavaScript

// allow-test-rule: structural-regression-guard [#3143]
/**
* no-unbounded-spawn-allowlist.test.cjs
*
* Structural guards on `eslint-rules/no-unbounded-spawn.allowlist.json` —
* matrix D4-D8 of
* .gsd/phase/chore-3143-no-unbounded-spawn-guard/50-test-matrix.md.
*
* D7 needs to inspect test-file *contents* for an inline directive that
* disables this rule by name — the absence of that pattern is the contract
* this guard protects (a contributor cannot silence the check by disabling
* it inline instead of fixing the timeout). That is a `readFileSync` +
* text-search on `.cjs` files, which is exactly what `local/no-source-grep`
* exists to catch — hence the
* `// allow-test-rule: structural-regression-guard [#3143]` annotation on
* its own line above, per CONTRIBUTING.md's documented exemption.
*/
'use strict';
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const path = require('path');
const ALLOWLIST_PATH = path.join(__dirname, '..', 'eslint-rules', 'no-unbounded-spawn.allowlist.json');
const TESTS_DIR = path.join(__dirname);
const REPO_ROOT = path.join(__dirname, '..');
// The allowlist only ratchets DOWN. This baseline is the length observed at
// the time this guard was written (139 entries) — each future migration
// wave lowers it as files are moved off the allowlist by adding real
// timeouts; it must never grow back up. Lowered to 120 by the #3144 Wave-1
// process-seam migration (19 files' unbounded spawns bounded), then to 73
// by the #3145 Wave-2 migration (47 files' unbounded spawns bounded).
const BASELINE = 73;
function readAllowlist() {
const raw = fs.readFileSync(ALLOWLIST_PATH, 'utf8');
return JSON.parse(raw);
}
function listTestFiles() {
const out = [];
for (const entry of fs.readdirSync(TESTS_DIR, { withFileTypes: true })) {
if (entry.isFile() && entry.name.endsWith('.cjs')) {
out.push(path.join(TESTS_DIR, entry.name));
}
}
return out;
}
// Pure detection helpers, extracted so each is unit-testable against a
// synthetic fixture — proving the check itself can fail, not only that
// today's real data happens to pass it (a check that never runs against an
// injected violation is a vacuous-truth risk).
function findDeadEntries(list, root) {
return list.filter((entry) => !fs.existsSync(path.join(root, entry)));
}
function findBackslashEntries(list) {
return list.filter((entry) => entry.includes('\\'));
}
function isSorted(list) {
return JSON.stringify(list) === JSON.stringify([...list].sort());
}
function findDuplicates(list) {
const seen = new Set();
const dupes = new Set();
for (const entry of list) {
if (seen.has(entry)) dupes.add(entry);
seen.add(entry);
}
return [...dupes];
}
// Built via concatenation, not a string literal, so this file does not
// itself contain the literal directive text (`local/no-unbounded-spawn`)
// that D7 below scans every test file for — a literal here would make this
// guard flag itself.
const GUARDED_RULE = 'local' + '/' + 'no-unbounded-spawn';
function containsDisableDirective(contents, ruleName) {
return new RegExp(`eslint-disable[^\\n]*${ruleName}`).test(contents);
}
describe('no-unbounded-spawn allowlist: D4 — no dead entries', () => {
test('every allowlist entry resolves to a file that exists on disk', () => {
const list = readAllowlist();
const dead = findDeadEntries(list, REPO_ROOT);
assert.deepEqual(dead, [], `dead allowlist entries (file does not exist): ${JSON.stringify(dead)}`);
});
test('detection logic actually flags a synthetic dead entry', () => {
const synthetic = ['tests/does-not-exist-xyz.test.cjs', 'tests/no-unbounded-spawn.test.cjs'];
const dead = findDeadEntries(synthetic, REPO_ROOT);
assert.deepEqual(dead, ['tests/does-not-exist-xyz.test.cjs']);
});
});
describe('no-unbounded-spawn allowlist: D5 — never grows', () => {
test('allowlist length is at or under the recorded baseline', () => {
const list = readAllowlist();
assert.ok(
list.length <= BASELINE,
`allowlist grew to ${list.length} entries, exceeding the BASELINE of ${BASELINE}. ` +
`The allowlist only ratchets down — if this is a legitimate new violation, ` +
`lower BASELINE only after confirming it, never raise it to paper over growth.`
);
});
});
describe('no-unbounded-spawn allowlist: D6 — separator normalization', () => {
test('every entry uses / separators, never \\', () => {
const list = readAllowlist();
const withBackslash = findBackslashEntries(list);
assert.deepEqual(withBackslash, [], `entries with a backslash separator: ${JSON.stringify(withBackslash)}`);
});
test('detection logic actually flags a synthetic backslash entry', () => {
const synthetic = ['tests\\foo.test.cjs', 'tests/bar.test.cjs'];
assert.deepEqual(findBackslashEntries(synthetic), ['tests\\foo.test.cjs']);
});
});
describe('no-unbounded-spawn allowlist: D7 — no inline disable of this rule', () => {
test('no test file inline-disables the unbounded-spawn guard', () => {
const offenders = [];
for (const filePath of listTestFiles()) {
const contents = fs.readFileSync(filePath, 'utf8');
if (containsDisableDirective(contents, GUARDED_RULE)) {
offenders.push(path.relative(REPO_ROOT, filePath));
}
}
assert.deepEqual(
offenders,
[],
`test files inline-disabling the unbounded-spawn guard (forbidden — fix the timeout instead): ${JSON.stringify(offenders)}`
);
});
test('detection logic actually flags a synthetic inline-disable directive', () => {
const syntheticContents = [
"'use strict';",
'// eslint-disable-next-line ' + GUARDED_RULE,
"spawnSync('git', ['status'], {});",
].join('\n');
assert.equal(containsDisableDirective(syntheticContents, GUARDED_RULE), true);
assert.equal(containsDisableDirective("'use strict';\nspawnSync('git', ['status'], {});", GUARDED_RULE), false);
});
});
describe('no-unbounded-spawn allowlist: D8 — canonical form', () => {
test('allowlist is valid JSON, sorted, with no duplicates', () => {
const list = readAllowlist();
assert.ok(Array.isArray(list), 'allowlist.json must parse to an array');
assert.ok(isSorted(list), 'allowlist entries must be sorted');
assert.deepEqual(findDuplicates(list), [], 'allowlist entries must be unique');
});
test('detection logic actually flags a synthetic unsorted/duplicate list', () => {
assert.equal(isSorted(['b.test.cjs', 'a.test.cjs']), false);
assert.deepEqual(findDuplicates(['a.test.cjs', 'b.test.cjs', 'a.test.cjs']), ['a.test.cjs']);
});
});