fix(layering): stop double-reporting contracts-authority violations (#1746)

* 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.

Author: thymikee

package.json

@@ -130,7 +130,7 @@
     "check:affected:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-affected/model.test.ts scripts/check-affected/platform-packages.test.ts scripts/check-affected/run.test.ts",
     "check:coverage-changed": "node --experimental-strip-types scripts/coverage-changed/run.ts",
     "check:coverage-changed:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/coverage-changed/model.test.ts scripts/coverage-changed/run.test.ts",
-    "check:layering": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/layering/model.test.ts scripts/layering/zone-policy.test.ts scripts/layering/daemon-modularity.test.ts scripts/layering/package-boundaries.test.ts scripts/layering/platform-package-policy.test.ts scripts/layering/platform-package-repository.test.ts scripts/layering/platform-package-source-policy.test.ts scripts/layering/device-inventory-cutover-policy.test.ts scripts/layering/logs-runtime-cutover-policy.test.ts scripts/layering/network-runtime-cutover-policy.test.ts scripts/layering/record-runtime-cutover-policy.test.ts scripts/layering/contracts-implementation-policy.test.ts scripts/layering/facade-exports.test.ts scripts/layering/bin-alias-fast-path.test.ts && node --experimental-strip-types scripts/layering/check.ts",
+    "check:layering": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/layering/*.test.ts && node --experimental-strip-types scripts/layering/check.ts",
     "depgraph": "node --experimental-strip-types scripts/depgraph/build.ts",
     "depgraph:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/depgraph/model.test.ts scripts/depgraph/affected.test.ts",
     "check:production-exports": "fallow dead-code --config fallow-production-exports.json --production --unused-exports --fail-on-issues",

scripts/layering/check-wiring.test.ts

@@ -0,0 +1,20 @@
+// 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`);
+  }
+});

scripts/layering/check.ts

@@ -646,6 +646,74 @@ function report(
   return 1;
 }
 
+/** Everything the guard reads once per run, so a rule takes one argument whatever it needs. */
+export type LayeringContext = Readonly<{
+  sourceFiles: readonly string[];
+  sources: ReadonlyMap<string, string>;
+  allTypeScriptSources: ReadonlyMap<string, string>;
+  edges: readonly ResolvedImportEdge[];
+  typeCycleMembers: readonly string[];
+}>;
+
+export type LayeringRule = (context: LayeringContext) => LayeringViolation[];
+
+/**
+ * The rules this guard runs. Registering one is writing a key here, which is why the list is data
+ * rather than a hand-written array of spreads: an object cannot hold the same key twice, so a rule
+ * cannot be run — and reported, and ::error-annotated — twice by a copy-paste. `LayeringRuleId`
+ * then makes a missing key a type error rather than a silently retired rule.
+ *
+ * Order is the reporting order: report() groups by rule in first-seen order.
+ */
+export const LAYERING_RULE_IDS = [
+  'zone-policies',
+  'value-import-cycles',
+  'logs-runtime-cutover',
+  'contracts-implementation-authority',
+  'network-runtime-cutover',
+  'record-runtime-cutover',
+  'back-edges',
+  'type-spine-inversions',
+  'session-state-ownership',
+  'daemon-modularity-ratchets',
+  'zero-dep-job-closure',
+  'bin-alias-fast-path',
+  'package-boundaries',
+  'platform-package-policy',
+  'device-inventory-cutover',
+] as const;
+
+export type LayeringRuleId = (typeof LAYERING_RULE_IDS)[number];
+
+export const LAYERING_RULES: Readonly<Record<LayeringRuleId, LayeringRule>> = {
+  'zone-policies': (context) => checkLayeringRules(context.edges),
+  'value-import-cycles': (context) => checkCycles(context.edges),
+  'logs-runtime-cutover': (context) => checkLogsRuntimeCutover(context.sources),
+  'contracts-implementation-authority': (context) =>
+    checkContractsImplementationAuthority(context.sources),
+  'network-runtime-cutover': (context) => checkNetworkRuntimeCutover(context.sources),
+  'record-runtime-cutover': (context) => checkRecordRuntimeCutover(context.sources),
+  'back-edges': (context) => checkBackEdges(context.edges),
+  'type-spine-inversions': (context) => checkTypeInversions(context.edges),
+  'session-state-ownership': (context) => checkSessionStateOwnership(context.sources),
+  'daemon-modularity-ratchets': (context) =>
+    checkDaemonModularityRatchets(context.edges, context.typeCycleMembers),
+  'zero-dep-job-closure': () => checkZeroDepJobs(),
+  'bin-alias-fast-path': (context) => checkBinAliasFastPath(context.sources),
+  'package-boundaries': () =>
+    checkPackageBoundaries(
+      repoRoot,
+      zeroDepClosureFiles(repoZeroDepJobs(), readSourceOrNull, fileExists),
+    ),
+  'platform-package-policy': (context) =>
+    checkPlatformPackagePolicy(
+      context.allTypeScriptSources,
+      readTrackedPlatformPackageDeclarations(repoRoot),
+      { untrackedProductionFiles: listUntrackedProductionTypeScriptFiles(repoRoot) },
+    ),
+  'device-inventory-cutover': (context) => checkDeviceInventoryCutover(context.sources),
+};
+
 export function main(): number {
   const sourceFiles = listSourceFiles();
   const sources = readSources(sourceFiles);
@@ -654,31 +722,14 @@ export function main(): number {
   // Computed once and threaded: the rule and the success line must report the same number.
   const typeCycleMembers = largestTypeCycleMembers(edges);
   const typeCycle = typeCycleMembers.length;
-  const violations = [
-    ...checkLayeringRules(edges),
-    ...checkCycles(edges),
-    ...checkLogsRuntimeCutover(sources),
-    ...checkContractsImplementationAuthority(sources),
-    ...checkNetworkRuntimeCutover(sources),
-    ...checkRecordRuntimeCutover(sources),
-    ...checkContractsImplementationAuthority(sources),
-    ...checkBackEdges(edges),
-    ...checkTypeInversions(edges),
-    ...checkSessionStateOwnership(sources),
-    ...checkDaemonModularityRatchets(edges, typeCycleMembers),
-    ...checkZeroDepJobs(),
-    ...checkBinAliasFastPath(sources),
-    ...checkPackageBoundaries(
-      repoRoot,
-      zeroDepClosureFiles(repoZeroDepJobs(), readSourceOrNull, fileExists),
-    ),
-    ...checkPlatformPackagePolicy(
-      allTypeScriptSources,
-      readTrackedPlatformPackageDeclarations(repoRoot),
-      { untrackedProductionFiles: listUntrackedProductionTypeScriptFiles(repoRoot) },
-    ),
-    ...checkDeviceInventoryCutover(sources),
-  ];
+  const context: LayeringContext = {
+    sourceFiles,
+    sources,
+    allTypeScriptSources,
+    edges,
+    typeCycleMembers,
+  };
+  const violations = Object.values(LAYERING_RULES).flatMap((rule) => rule(context));
   return report(sourceFiles, violations, typeCycle);
 }