mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
Michał Pierzchała
057ab1c82d
* fix(layering): stop double-reporting contracts-authority violations main()'s violation list spread checkContractsImplementationAuthority(sources) twice, so every R11 contracts-implementation-authority finding was printed and ::error-annotated twice on a red run — inflating the headline violation count and producing duplicate GitHub annotations on the same file:line. Verified by planting a `setTimeout` call in a contracts production source: the rule reported 2 identical violations before and 1 after, with the extra annotation gone. `pnpm check:layering` stays green (136/136 policy tests). Nothing in the suite covers main()'s assembly of the violation list — the policy tests all call their rule functions directly — so neither a duplicated nor a dropped entry there is currently detectable. * test(layering): hold main() to wiring every rule exactly once The duplicate this branch removed survived because nothing enumerates the guard's rules: main()'s violation list is hand-written, and the per-policy tests call their rule functions directly, never seeing the wiring. A lost spread is the dangerous version of the same gap — the rule stops being enforced and the run still prints OK. Make the file's own bindings the oracle: every in-scope `check*` value, local or imported, must be spread into main()'s violation list exactly once. That covers both directions plus a third case — a policy written and never wired in. Fails closed if main() or the array is renamed, so the instrument cannot pass by finding nothing. Test-only rather than an R17 inside the guard: a self-referential rule is defeated by dropping its own spread, which is exactly the failure it exists to catch. Verified by mutating the real check.ts in both directions (re-planting the duplicate, then dropping checkZeroDepJobs) — each turns the run red, and the restored file is green at 143/143. * test(layering): discover layering suites by glob instead of by hand check:layering named its 14 test files one by one, so adding a policy test meant remembering to register it — and twice nobody did. Both halves of the R16 record cutover shipped with tests that have never run: scripts/layering/record-runtime-mechanics-policy.test.ts (2 tests) scripts/layering/record-runtime-registry-policy.test.ts (1 test) Their policies are live in the guard; only the tests were dormant. All three pass, so nothing had rotted — the coverage was simply never being collected. Glob the directory the way mutation:test already globs its own, which makes the filesystem the enumeration and retires the registration step. 143 -> 146 tests, still green. This is the same defect as the duplicate spread this branch opened with, one level up: a hand-maintained list with nothing checking it against reality. * refactor(layering): register guard rules in a keyed table Replaces the AST wiring guard with a construction that cannot express the defect, per review on #1746. The parser was the wrong instrument: it reconstructed one array's shape from TypeScript syntax, so it only recognised top-level function declarations and imports whose local name matched /^check[A-Z]/. A const-defined or aliased rule was invisible to it, a helper named checkX was a false positive, and naming and syntax became part of the interface — all to detect a mistake rather than prevent it. Rules now live in a keyed table over a shared context, executed once via Object.values. An object cannot hold a key twice, so double registration is unrepresentable rather than merely detected, and oxlint's no-dupe-keys rejects the attempt at the source. LayeringRuleId makes a missing key a type error, and LAYERING_RULE_IDS gives the catalog to check exhaustiveness against. Call sites and order are unchanged, so grouped output and the success line are byte-identical. One regression test remains, through the production interface: scripts/ is outside tsconfig.json's `include`, so the Record's exhaustiveness is an editor signal rather than a CI gate, and the catalog assertion is what fails the build when wiring goes missing. Verified by mutation: dropping an entry and registering an uncatalogued one both fail the test, a duplicated key fails oxlint, and re-planting the original contracts violation reports it exactly once. Net -133 LOC.
21 lines
1.1 KiB
TypeScript
21 lines
1.1 KiB
TypeScript
// The seam bin-alias-fast-path.test.ts calls out as untested — "the check.ts wiring that turns it
|
|
// into a violation." The registry makes duplicate registration unrepresentable on its own (an
|
|
// object holds a key once, and oxlint's no-dupe-keys rejects the attempt), so what is left to check
|
|
// is the other direction: that the catalog and the registry still describe the same set of rules.
|
|
// scripts/ is outside tsconfig.json's `include`, so the Record's exhaustiveness is an editor
|
|
// signal, not a CI gate — this test is what fails the build when wiring goes missing.
|
|
|
|
import assert from 'node:assert/strict';
|
|
import { test } from 'node:test';
|
|
import { LAYERING_RULE_IDS, LAYERING_RULES } from './check.ts';
|
|
|
|
test('every catalogued rule is registered, and nothing else is', () => {
|
|
assert.deepEqual(Object.keys(LAYERING_RULES), [...LAYERING_RULE_IDS]);
|
|
});
|
|
|
|
test('every registered rule is callable, so no entry is a stale reference', () => {
|
|
for (const id of LAYERING_RULE_IDS) {
|
|
assert.equal(typeof LAYERING_RULES[id], 'function', `${id} is not callable`);
|
|
}
|
|
});
|