refactor(layering): declare R1-R3 as a policy table, and test them

.github/workflows/ci.yml

@@ -110,7 +110,7 @@ jobs:
         # import-direction lint over the resolved graph. See scripts/layering/check.ts
         # and CONTEXT.md (Architecture: folder DAG + layering lint).
         run: |
-          node --experimental-strip-types --test scripts/layering/model.test.ts
+          node --experimental-strip-types --test scripts/layering/model.test.ts scripts/layering/zone-policy.test.ts
           node --experimental-strip-types scripts/layering/check.ts
 
       - name: Check the depgraph report agrees with the gate

CONTEXT.md

@@ -255,7 +255,12 @@ The perfect-shape refactor is complete and merged. Its end-state:
   follow-up surfaces.
 - Folder DAG + layering lint. `scripts/layering/check.ts` enforces five rules across four scopes in CI.
   GLOBALLY, across every production source file, it enforces the R1-R3 move rules (kernel-sink,
-  commands-floor, platforms-seam) and rejects all production static value-import cycles. Separately,
+  commands-floor, platforms-seam) and rejects all production static value-import cycles. R1-R3 are
+  declared as data in `scripts/layering/zone-policy.ts` (`ZONE_POLICIES`): which zones a boundary
+  governs, which import kinds it tolerates, and which path prefixes are its declared seam. A fourth
+  zone boundary is a table entry, not a fourth predicate. `zone-policy.test.ts` asserts each
+  boundary fires and each exemption holds — necessary because the tree is clean, so a rule that
+  stopped matching would look exactly like a rule being obeyed. Separately,
   it ranks an explicit target spine — as rank groups, lowest (kernel sink) to highest, where `A ◄ B`
   means B may not be outranked by A (the back-edge order the gate rejects), NOT that every displayed
   import exists:

docs/dependency-graph-findings.md

@@ -387,6 +387,51 @@ Still duplicated across zones, each needing the same treatment: `fill requires t
 `settings clear-app-state requires an app id…` (`core/dispatch.ts` ↔ two platform modules), and
 `snapshot is not supported by this backend` (three files inside `commands/`).
 
+## 7. `eslint-plugin-boundaries` under oxlint: evaluated, works, not adopted
+
+Spiked properly (installed, configured against the real tree, verified by injection) so nobody has
+to repeat it. **It runs** — oxlint's `jsPlugins` loads npm ESLint plugins directly, so this needs no
+ESLint install, no second config and no extra CI step:
+
+```json
+{ "jsPlugins": [{ "name": "boundaries", "specifier": "eslint-plugin-boundaries" }] }
+```
+
+R1, R2 and R3 are all expressible, and all three were confirmed to fire on injected violations while
+every documented exemption held. The two non-obvious settings that make R3 work:
+
+| need | mechanism |
+|---|---|
+| ignore type-only edges | `importKind: "value"` on the policy |
+| ignore dynamic `import()` | `settings["boundaries/dependency-nodes"] = ["import", "export"]` |
+
+**Why we did not adopt it.** None of these is fatal alone; together they lose more than the
+declarative syntax gains, and `ZONE_POLICIES` gets that syntax anyway:
+
+- **No ratchet.** The mechanism that took R6 from 61 → 7 across three PRs is a baseline that
+  tolerates N and fails on N+1. A per-file lint rule has no cross-run aggregate, so it cannot do
+  this. Any new boundary with existing violations — the platforms facade has ~89 — would need one
+  disable comment per site.
+- **It cannot replace the gate.** R4 (cycles), R5/R6 (spine ranking + ratchet), R7 (field
+  ownership — not an import rule at all), R8 (CI job closure) and R9 (cycle size) are whole-graph or
+  non-import properties. Adopting it means R1-R3 live in one system and R4-R9 in another, so
+  "where is our architecture defined" gets two answers.
+- **Inline type specifiers are misread.** `import { type A, type B } from '…'` is fully erased at
+  runtime, but the plugin classifies it as a value import (its `importKind` is statement-level).
+  That produced one real false positive here — `providers/limrun/android.ts` — flagged as an R3
+  violation the gate correctly ignores. Only 10 of 1,481 type-only edges use this form, so the blast
+  radius is small, but `statementIsTypeOnly` in `model.ts` handles both spellings and the plugin
+  does not.
+- **Message specificity regressed.** With the current non-deprecated selector syntax,
+  `{{dependency.type}}` and `{{file.type}}` interpolated to empty strings, so violations read
+  `must not import commands/` with no zone named. ADR 0010 wants every error actionable.
+- **Cost.** 230 transitive packages, and `jsPlugins` is documented as "in alpha and not subject to
+  semver." A single config attempt produced five deprecation warnings (`mode`, `rules`, legacy
+  selectors, legacy templates, rule-level `importKind`) spanning two major migrations.
+
+Worth re-evaluating if the monorepo migration happens — per-package ESLint configs change the
+calculus — or once `jsPlugins` is stable and the ratchet gap is addressable.
+
 ## Suggested order from here
 
 1. **Move the 10 outward-facing `daemon/types.ts` types into `contracts/`** (§2). Mechanical, and

package.json

@@ -125,7 +125,7 @@
     "check:affected:test": "node --experimental-strip-types --test scripts/check-affected/model.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 --test scripts/coverage-changed/model.test.ts scripts/coverage-changed/run.test.ts",
-    "check:layering": "node --experimental-strip-types --test scripts/layering/model.test.ts && node --experimental-strip-types scripts/layering/check.ts",
+    "check:layering": "node --experimental-strip-types --test scripts/layering/model.test.ts scripts/layering/zone-policy.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 --test scripts/depgraph/model.test.ts",
     "check:production-exports": "fallow dead-code --config fallow-production-exports.json --production --unused-exports --fail-on-issues",

scripts/layering/check.ts

@@ -45,16 +45,9 @@ import {
   resolveImportEdges,
   topFolder,
   typeInversionPair,
-  type ImportEdge,
   type ResolvedImportEdge,
 } from './model.ts';
-
-type EdgeContext = {
-  file: string;
-  fromTop: string;
-  toTop: string;
-  imp: ImportEdge;
-};
+import { policyLead, policyViolation, ZONE_POLICIES } from './zone-policy.ts';
 
 type Violation = {
   rule: string;
@@ -96,71 +89,24 @@ function isProductionSourceFile(file: string): boolean {
   return file.endsWith('.ts') && !/(?:^|\/)__tests__\//.test(file) && !/\.test\.ts$/.test(file);
 }
 
-function isCoreInteractor(file: string): boolean {
-  return file.startsWith('src/core/interactors/');
-}
-
-function isDaemonServer(file: string): boolean {
-  return file.startsWith('src/daemon/') && !file.startsWith('src/daemon/client/');
-}
-
-function isSdkBarrel(file: string): boolean {
-  return file.startsWith('src/sdk/');
-}
-
-function ruleKernelSink(ctx: EdgeContext): Violation | null {
-  if (ctx.fromTop !== 'kernel') return null;
-  if (ctx.toTop === 'contracts' && ctx.imp.typeOnly) return null;
-  return {
-    rule: 'R1 kernel-sink',
-    file: ctx.file,
-    line: ctx.imp.line,
-    message:
-      `kernel must not import ${ctx.toTop}/ (imports '${ctx.imp.spec}'). ` +
-      `The only allowed kernel out-edge is a type-only re-export from contracts/.`,
-  };
-}
-
-const BELOW_COMMANDS = new Set(['kernel', 'platforms', 'core', 'daemon']);
-
-function ruleCommandsFloor(ctx: EdgeContext): Violation | null {
-  if (ctx.toTop !== 'commands' || !BELOW_COMMANDS.has(ctx.fromTop)) return null;
-  return {
-    rule: 'R2 commands-floor',
-    file: ctx.file,
-    line: ctx.imp.line,
-    message:
-      `${ctx.fromTop}/ must not import the command surface commands/ ` +
-      `(imports '${ctx.imp.spec}'). Depend on shared kernel/contracts instead.`,
-  };
-}
-
-function rulePlatformsSeam(ctx: EdgeContext): Violation | null {
-  if (ctx.toTop !== 'platforms' || ctx.imp.dynamic || ctx.imp.typeOnly) return null;
-  if (isCoreInteractor(ctx.file) || isDaemonServer(ctx.file) || isSdkBarrel(ctx.file)) return null;
-  return {
-    rule: 'R3 platforms-seam',
-    file: ctx.file,
-    line: ctx.imp.line,
-    message:
-      `static value import of platforms/ from ${ctx.fromTop}/ (imports '${ctx.imp.spec}'). ` +
-      `Only src/core/interactors/ and the daemon server may statically import platforms/; ` +
-      `elsewhere use a dynamic import() or a type-only import to preserve CLI cold-start.`,
-  };
-}
-
-const RULES = [ruleKernelSink, ruleCommandsFloor, rulePlatformsSeam];
-
+// R1-R3 are declared as a policy table in zone-policy.ts. This walks it; the boundaries
+// themselves are data, so adding one is a table entry rather than a fourth predicate.
 function checkLayeringRules(edges: readonly ResolvedImportEdge[]): Violation[] {
   const violations: Violation[] = [];
   for (const edge of edges) {
-    const fromTop = topFolder(edge.file);
-    const toTop = topFolder(edge.target);
-    if (fromTop === toTop) continue;
-    const ctx: EdgeContext = { file: edge.file, fromTop, toTop, imp: edge };
-    for (const rule of RULES) {
-      const violation = rule(ctx);
-      if (violation) violations.push(violation);
+    const fromZone = topFolder(edge.file);
+    const toZone = topFolder(edge.target);
+    if (fromZone === toZone) continue;
+    const ctx = { file: edge.file, fromZone, toZone, imp: edge };
+    for (const policy of ZONE_POLICIES) {
+      const hint = policyViolation(policy, ctx);
+      if (hint === null) continue;
+      violations.push({
+        rule: policy.rule,
+        file: edge.file,
+        line: edge.line,
+        message: `${policyLead(ctx)} ${hint}`,
+      });
     }
   }
   return violations;

scripts/layering/zone-policy.test.ts

@@ -0,0 +1,135 @@
+import assert from 'node:assert/strict';
+import { test } from 'node:test';
+import { policyLead, policyViolation, ZONE_POLICIES, type ZonePolicy } from './zone-policy.ts';
+
+// R1-R3 were three predicate functions with no unit test for eight months: the only thing
+// exercising them was the real tree, which is clean, so a rule that had silently stopped matching
+// would have looked exactly like a rule that was being obeyed. These tests assert each boundary
+// fires AND each documented exemption holds, so the table cannot quietly become decoration.
+
+type Kind = { typeOnly?: boolean; dynamic?: boolean };
+
+function edge(file: string, fromZone: string, toZone: string, kind: Kind = {}) {
+  return {
+    file,
+    fromZone,
+    toZone,
+    imp: {
+      spec: `../${toZone}/thing.ts`,
+      line: 1,
+      dynamic: kind.dynamic ?? false,
+      typeOnly: kind.typeOnly ?? false,
+    },
+  };
+}
+
+/** Every policy that fires for an edge, by rule id. */
+function firing(e: ReturnType<typeof edge>): string[] {
+  return ZONE_POLICIES.filter((policy) => policyViolation(policy, e) !== null).map((p) => p.rule);
+}
+
+function policiesFor(rule: string): ZonePolicy[] {
+  return ZONE_POLICIES.filter((policy) => policy.rule === rule);
+}
+
+test('R1 kernel-sink closes every kernel out-edge except a type-only contracts re-export', () => {
+  // The one open door.
+  assert.deepEqual(
+    firing(edge('src/kernel/errors.ts', 'kernel', 'contracts', { typeOnly: true })),
+    [],
+  );
+
+  // Same target, value import: closed. A runtime edge out of the sink is the thing R1 exists for.
+  assert.deepEqual(firing(edge('src/kernel/errors.ts', 'kernel', 'contracts')), ['R1 kernel-sink']);
+
+  // Any other target is closed to BOTH kinds — unlike R3, R1 does not tolerate type-only, because
+  // "kernel is declared in terms of utils" is still a claim that kernel is not the sink.
+  for (const kind of [{}, { typeOnly: true }, { dynamic: true }]) {
+    assert.deepEqual(
+      firing(edge('src/kernel/errors.ts', 'kernel', 'utils', kind)),
+      ['R1 kernel-sink'],
+      `kernel -> utils ${JSON.stringify(kind)} must violate R1`,
+    );
+  }
+});
+
+test('R2 commands-floor closes the four zones below the command surface, whatever the kind', () => {
+  for (const zone of ['kernel', 'platforms', 'core', 'daemon']) {
+    for (const kind of [{}, { typeOnly: true }, { dynamic: true }]) {
+      assert.ok(
+        firing(edge(`src/${zone}/thing.ts`, zone, 'commands', kind)).includes('R2 commands-floor'),
+        `${zone} -> commands ${JSON.stringify(kind)} must violate R2`,
+      );
+    }
+  }
+
+  // Zones at or above the command surface are not governed by R2.
+  assert.deepEqual(firing(edge('src/cli/run.ts', 'cli', 'commands')), []);
+  assert.deepEqual(firing(edge('src/mcp/tools.ts', 'mcp', 'commands')), []);
+});
+
+test('R3 platforms-seam closes static value imports and opens the three declared seam owners', () => {
+  // Closed from an ordinary zone.
+  assert.deepEqual(firing(edge('src/cli/run.ts', 'cli', 'platforms')), ['R3 platforms-seam']);
+
+  // Open to the seam owners.
+  for (const file of [
+    'src/core/interactors/android.ts',
+    'src/daemon/handlers/perf.ts',
+    'src/sdk/android-adb.ts',
+  ]) {
+    assert.deepEqual(firing(edge(file, 'core', 'platforms')), [], `${file} is a seam owner`);
+  }
+
+  // daemon/client/ is inside the daemon prefix but is NOT part of the seam: it is the client half
+  // of the process boundary (ADR 0008), so it must not reach platform code at all.
+  assert.deepEqual(firing(edge('src/daemon/client/daemon-client.ts', 'daemon', 'platforms')), [
+    'R3 platforms-seam',
+  ]);
+
+  // Both tolerated kinds are the documented escape hatch that preserves CLI cold-start.
+  assert.deepEqual(firing(edge('src/cli/run.ts', 'cli', 'platforms', { dynamic: true })), []);
+  assert.deepEqual(firing(edge('src/cli/run.ts', 'cli', 'platforms', { typeOnly: true })), []);
+});
+
+test('every policy carries a hint, and the lead names the kind it rejected', () => {
+  // ADR 0010: an error that does not say what to do instead is a rule nobody can act on.
+  for (const policy of ZONE_POLICIES) {
+    assert.ok(policy.hint.length > 20, `${policy.rule} needs an actionable hint`);
+  }
+
+  assert.match(policyLead(edge('a', 'kernel', 'utils')), /kernel\/ must not import utils\//);
+  assert.match(
+    policyLead(edge('a', 'kernel', 'commands', { typeOnly: true })),
+    /must not type-only import commands\//,
+  );
+  assert.match(
+    policyLead(edge('a', 'cli', 'platforms', { dynamic: true })),
+    /must not dynamic import platforms\//,
+  );
+});
+
+test('a policy states its scope with exactly one of `to` or `exceptTo`', () => {
+  // Both would make the target set ambiguous; neither would silently govern every target, which is
+  // never what a boundary means.
+  for (const policy of ZONE_POLICIES) {
+    assert.notEqual(
+      Boolean(policy.to) === Boolean(policy.exceptTo),
+      true,
+      `${policy.rule} must set exactly one of to/exceptTo`,
+    );
+  }
+});
+
+test('R1 is two policies, so the single open door stays visible', () => {
+  // Collapsing these into one entry with a special case is what made the original predicate hard
+  // to read. If someone merges them, this fails and says why.
+  const r1 = policiesFor('R1 kernel-sink');
+  assert.equal(r1.length, 2);
+  assert.deepEqual(
+    r1.find((p) => p.to)?.tolerates,
+    ['type-only'],
+    'the contracts door is type-only',
+  );
+  assert.equal(r1.find((p) => p.exceptTo)?.tolerates, undefined, 'every other target is closed');
+});