test(structure): make the eager-closure pins exact, bounded, and single-owner

Second review pass on #1965 found four holes, two of which were places the
PR text claimed a property the code did not have.

1. Rows were documented as exact ratchets but asserted with `<=`, so a shrink
   silently became headroom a later regression could grow back into. The
   comparison is now equality, in a pure `classifyBudget` with a separate
   message for each direction ("lower its pin to N in this PR so the ratchet
   keeps the gain"), matching test-file-size-ratchet.ts and the R9/R10 pins.

2. An over-pin failure printed a chain per evaluated module — 361 of them for
   src/cli.ts. It now prints a bounded attribution: the entry's heaviest direct
   edges (capped at 4) with a couple of representative deep routes each, ranked
   so a newly added import sorts first. The comment states plainly that this
   attributes by shortest import route and does NOT diff against a recorded
   baseline; naming a true delta would mean checking in ~1,500 module paths and
   rewriting them on every contracts refactor.

3. Discovery reimplemented a one-level `src/facades` scan while canonical R11
   discovery is recursive, so a nested façade file could be covered by R11 and
   silently missing here. `facadeEntryFiles` is now a single exported owner in
   package-boundaries.ts that both R11's façade gate and this table consume.

4. Rows were converted to Sets before any uniqueness check, so a duplicate was
   unobservable. The table is now two `Record<string, number>` literals keyed by
   path, making an in-record duplicate a TypeScript error (ts1117); the only
   remaining case — one path in both records — is asserted on the array.

Each of the four holes gets a test that fails when the rule is broken, since a
tree that happens to satisfy its pins cannot distinguish a correct rule from a
vacuous one. Writing those found a real bug in the duplicate check itself
(`Set.add` returns the Set, so the filter never matched).

Pins reseeded on 04e4c23.

Author: thymikee

scripts/layering/package-boundaries.test.ts

@@ -10,6 +10,7 @@ import { listSourceFiles } from './check.ts';
 import { readDirectNamedExports, readNamedExports, readReExportSources } from './facade-exports.ts';
 import {
   checkPackageBoundaries,
+  facadeEntryFiles,
   checkPackageInternalSites,
   checkRootSites,
   readWorkspacePackages,
@@ -138,17 +139,17 @@ test('every workspace package façade names its exports explicitly (no bare `exp
   // the façade file itself IS the pin — a widening shows up in the diff of the file that grew,
   // not in a table two files away that only a gate failure would surface. This structural gate is
   // what keeps that property true: every façade a package manifest declares (`exportTargets`),
-  // plus every file under a `packages/*/src/facades/` directory, must parse through
+  // plus every production file under a `src/facades/` directory, must parse through
   // `readNamedExports` without hitting the bare-`export *`/`export default` rejection it already
   // implements — reusing that check rather than writing a second, regex-based one that would have
   // to independently rediscover every export form to be trustworthy.
-  const packages = readWorkspacePackages(repoRoot);
-  const facadeFiles = new Set<string>(packages.flatMap((pkg) => [...pkg.exportTargets.values()]));
-  for (const file of listSourceFiles()) {
-    if (file.includes('/src/facades/')) facadeFiles.add(file);
-  }
-  assert.ok(facadeFiles.size > 0, 'expected at least one workspace package façade to check');
-  for (const file of [...facadeFiles].sort()) {
+  //
+  // The façade set comes from `facadeEntryFiles`, the single owner of "what is an entry surface".
+  // The ADR-0019 eager-closure budget table consumes the same function, so a file this gate holds
+  // to an explicit export list is necessarily a file that gate holds to a loading-shape budget.
+  const facadeFiles = facadeEntryFiles(repoRoot);
+  assert.ok(facadeFiles.length > 0, 'expected at least one workspace package façade to check');
+  for (const file of facadeFiles) {
     const source = fs.readFileSync(path.join(repoRoot, file), 'utf8');
     try {
       readNamedExports(source);

scripts/layering/package-boundaries.ts

@@ -255,6 +255,36 @@ export function rootExternalDependencyRanges(repoRoot: string): Map<string, stri
   return new Map(Object.entries(manifest.dependencies ?? {}));
 }
 
+/**
+ * Every workspace-package entry surface, repo-root-relative and sorted: whatever a package
+ * manifest's `exports` map points at, plus every production source file under a `src/facades/`
+ * directory.
+ *
+ * The single owner of that question. R11's façade gates and the ADR-0019 eager-closure budget
+ * table (`src/__tests__/eager-closure-budgets.ts`) both consume this, so the two cannot drift
+ * into disagreeing about what counts as a façade — a gate that scanned a narrower set would
+ * silently exempt files the other one covers, which is exactly the hole #1960 review found (a
+ * one-level `readdir` missed both nested façade files and the six `packages/platform-*`
+ * manifest façades, which have no `facades/` directory at all).
+ *
+ * The `src/facades/` walk is recursive and skips test sources, matching the production-file
+ * scope every other layering scan uses.
+ */
+export function facadeEntryFiles(repoRoot: string): string[] {
+  const found = new Set<string>();
+  for (const pkg of readWorkspacePackages(repoRoot)) {
+    for (const target of pkg.exportTargets.values()) found.add(target);
+  }
+  for (const root of ['src', 'packages']) {
+    for (const file of walkTsFiles(repoRoot, root)) {
+      if (!file.includes('/src/facades/')) continue;
+      if (/(?:^|\/)__tests__\//.test(file) || file.endsWith('.test.ts')) continue;
+      found.add(file);
+    }
+  }
+  return [...found].filter((file) => fs.existsSync(path.join(repoRoot, file))).sort();
+}
+
 function walkTsFiles(repoRoot: string, relativeDir: string): string[] {
   const absolute = path.join(repoRoot, relativeDir);
   if (!fs.existsSync(absolute)) return [];

src/__tests__/eager-closure-budgets.test.ts

@@ -2,10 +2,15 @@ import { expect, test } from 'vitest';
 import fs from 'node:fs';
 import path from 'node:path';
 import { eagerClosureGraphOf } from './eager-import-closure.fixtures.ts';
+import { mkdtempForTestSync } from './test-utils/tmp-dir.ts';
 import {
+  classifyBudget,
+  describeClosurePressure,
   discoverFacadeEntryFiles,
   EAGER_CLOSURE_BUDGETS,
+  FACADE_BUDGETS,
   formatImportChain,
+  HUB_BUDGETS,
   PLATFORM_IMPLEMENTATION_PATTERNS,
   type EagerClosureBudget,
 } from './eager-closure-budgets.ts';
@@ -18,18 +23,20 @@ import {
  * issue owns the exact probe and planted-red procedure." #1950 built the walker this file reuses
  * (`eager-import-closure.fixtures.ts`, AST-level: static value edges plus top-level dynamic
  * imports, type-only erased) and proved the planted-red procedure on one file. This is that
- * probe, generalized to every workspace-package entry surface plus designated hub modules, driven
- * by the table in `eager-closure-budgets.ts` instead of one-off pins.
+ * probe, generalized to every workspace-package entry surface plus designated hub modules.
  *
  * - Catches: an entry surface or vocabulary module silently going eager -- the regression class
  *   #1950 fixed once and #1959/#1969 fixed at five more sites. Nothing else prevents the next
  *   instance: layering R3/R13 govern import DIRECTION (may this file reach that one at all),
  *   never evaluation WEIGHT (how much of the repo an importer drags along).
  * - Evidence: planted red re-verified against this gate itself, not merely cited from #1950 --
- *   see the PR description for the observed failure, including the printed edge chain.
+ *   see the PR description. Every rule the real-tree assertions rest on (the equality ratchet,
+ *   the bounded attribution, recursive discovery, row uniqueness) additionally has its own
+ *   failing-direction test below, because a real tree that happens to satisfy its pins cannot
+ *   distinguish a correct rule from a vacuous one.
  * - Cost: one unit-lane test file plus one data module; no subprocess, no device. The walker
  *   memoizes per-file edges, so the ~100 entries parse each reachable file once in total.
- * - Kill criterion: if two consecutive quarters show no budget ever tightening or firing, or
+ * - Kill criterion: if two consecutive quarters show no pin ever tightening or firing, or
  *   ADR-0019 composition lands a stronger structural proof of the loading shape, delete this gate
  *   in favor of that proof.
  */
@@ -40,37 +47,123 @@ function relList(files: readonly string[]): string[] {
   return [...files].map((file) => path.relative(repoRoot, file)).sort();
 }
 
-test('every discovered entry surface has exactly one budget entry, and none is stale', () => {
+// --- the rules, tested in their failing direction -------------------------------------------
+// Each of these covers a hole that the real-tree assertions below cannot see: while the tree
+// matches its pins, an `<=` comparison, a one-level discovery scan, and a duplicate-swallowing
+// `Set` all look exactly like correct implementations.
+
+test('the ratchet fails an entry that SHRANK, not only one that grew', () => {
+  // The hole: `actual <= budget` passes every shrink, silently converting the gain into headroom
+  // that a later regression grows back into unnoticed.
+  expect(classifyBudget('x.ts', 42, 42)).toBeNull();
+  expect(classifyBudget('x.ts', 43, 42)).toMatch(/evaluates 43 .*pinned at 42/);
+  const shrank = classifyBudget('x.ts', 40, 42);
+  expect(shrank).toMatch(/shrank/);
+  expect(shrank, 'a shrink finding must tell the author the new number to pin').toMatch(
+    /lower its pin to 40/,
+  );
+});
+
+test('closure pressure is attributed to the heaviest direct edges and is bounded', () => {
+  // The hole: printing a chain per evaluated module is unusable at src/cli.ts scale (361), so a
+  // failure that "names the chain" can still be unreadable. This pins both halves: the offending
+  // edge ranks first, and the output stays bounded however wide the entry is.
+  const entry = '/repo/entry.ts';
+  const graph = new Map<string, string | null>([[entry, null]]);
+  // One heavy edge with a deep chain, one trivial edge, plus many shallow ones to force capping.
+  graph.set('/repo/heavy.ts', entry);
+  graph.set('/repo/heavy-2.ts', '/repo/heavy.ts');
+  graph.set('/repo/heavy-3.ts', '/repo/heavy-2.ts');
+  graph.set('/repo/light.ts', entry);
+  for (let index = 0; index < 10; index += 1) graph.set(`/repo/filler-${index}.ts`, entry);
+
+  const described = describeClosurePressure(graph, entry, '/repo');
+  expect(described).toContain('heavy.ts -- 3 module(s) enter through this edge');
+  expect(described.indexOf('heavy.ts')).toBeLessThan(described.indexOf('light.ts'));
+  expect(described, 'the deep route must be shown, not just the edge name').toContain('heavy-3.ts');
+  expect(described, 'output must be capped and say how much it omitted').toMatch(
+    /\(\+\d+ more direct edge\(s\), smaller\)/,
+  );
+  expect(described.split('\n').length).toBeLessThan(20);
+});
+
+test('an entry that evaluates only itself is described without pretending to an edge', () => {
+  // The six platform façades are exactly this shape, so the message they would print matters.
+  const graph = new Map<string, string | null>([['/repo/solo.ts', null]]);
+  expect(describeClosurePressure(graph, '/repo/solo.ts', '/repo')).toContain('only itself');
+});
+
+test('discovery is recursive, so a NESTED facade file cannot hide from the gate', () => {
+  // The hole: a one-level `readdir` of `src/facades` omits `src/facades/nested/x.ts`, which R11's
+  // recursive scan covers -- the two gates would disagree about what a façade is, and this one
+  // would be the lenient half. Exercised against a fixture tree so it holds even while the real
+  // repo happens to have no nested façade.
+  const fixtureRoot = mkdtempForTestSync('eager-closure-discovery-');
+  const pkgDir = path.join(fixtureRoot, 'packages/demo');
+  fs.mkdirSync(path.join(pkgDir, 'src/facades/nested'), { recursive: true });
+  fs.writeFileSync(
+    path.join(pkgDir, 'package.json'),
+    JSON.stringify({ name: '@agent-device/demo', exports: { '.': './src/entry.ts' } }),
+  );
+  fs.writeFileSync(path.join(pkgDir, 'src/entry.ts'), 'export const a = 1;\n');
+  fs.writeFileSync(path.join(pkgDir, 'src/facades/top.ts'), 'export const b = 2;\n');
+  fs.writeFileSync(path.join(pkgDir, 'src/facades/nested/deep.ts'), 'export const c = 3;\n');
+  fs.writeFileSync(path.join(pkgDir, 'src/facades/skip.test.ts'), 'export const d = 4;\n');
+
+  const discovered = discoverFacadeEntryFiles(fixtureRoot);
+  expect(discovered).toContain('packages/demo/src/entry.ts'); // manifest-declared
+  expect(discovered).toContain('packages/demo/src/facades/top.ts');
+  expect(discovered).toContain('packages/demo/src/facades/nested/deep.ts'); // the regression
+  expect(discovered, 'test sources are not entry surfaces').not.toContain(
+    'packages/demo/src/facades/skip.test.ts',
+  );
+});
+
+test('no entry path is budgeted twice, checked before any Set could absorb it', () => {
+  // Uniqueness within each record is a TypeScript error (ts1117, duplicate object literal key),
+  // so the only duplicate still expressible is one path appearing in both records. Asserted on
+  // the ARRAY: converting to a Set first is what made the original "exactly one row" claim
+  // unfalsifiable.
+  const ids = EAGER_CLOSURE_BUDGETS.map((entry) => entry.entryFile);
+  const seen = new Set<string>();
+  const duplicated: string[] = [];
+  for (const id of ids) {
+    if (seen.has(id)) duplicated.push(id);
+    seen.add(id);
+  }
+  expect(
+    duplicated,
+    'These paths are budgeted twice (a path in both FACADE_BUDGETS and HUB_BUDGETS). One row per ' +
+      'entry: pick the record that describes it.',
+  ).toEqual([]);
+  expect(ids.length).toBe(Object.keys(FACADE_BUDGETS).length + Object.keys(HUB_BUDGETS).length);
+});
+
+// --- the real tree --------------------------------------------------------------------------
+
+test('every discovered entry surface has exactly one row, and none is stale', () => {
   // Bidirectional, mirroring the repo's other exhaustiveness gates (R7/R10 field checklists, the
-  // R11 exhaustive re-export check): an entry surface with no budget lets this whole mechanism go
+  // R11 exhaustive re-export check): an entry surface with no row lets this whole mechanism go
   // silently vacuous for it -- which is exactly how the first version of this gate missed all six
-  // platform-package façades -- and a budget naming a file that is no longer an entry surface lets
+  // platform-package façades -- and a row naming a file that is no longer an entry surface lets
   // the table drift from what it claims to police.
   const discovered = new Set(discoverFacadeEntryFiles(repoRoot));
-  const budgeted = new Set(
-    EAGER_CLOSURE_BUDGETS.filter((entry) => entry.kind === 'facade').map(
-      (entry) => entry.entryFile,
-    ),
-  );
-
-  const missingBudget = [...discovered].filter((file) => !budgeted.has(file)).sort();
-  const staleBudget = [...budgeted].filter((file) => !discovered.has(file)).sort();
+  const budgeted = new Set(Object.keys(FACADE_BUDGETS));
 
   expect(
-    missingBudget,
-    'These package entry surfaces (a package.json `exports` target, or a file under a ' +
-      '`src/facades/` directory) have no entry in eager-closure-budgets.ts. Measure the current ' +
-      'closure size with eagerClosureOf and add a row, or the loading-shape probe does not ' +
-      'actually cover them.',
+    [...discovered].filter((file) => !budgeted.has(file)).sort(),
+    'These package entry surfaces (a package.json `exports` target, or a production file under a ' +
+      '`src/facades/` directory) have no row in eager-closure-budgets.ts. Measure the current ' +
+      'closure size and add one, or the loading-shape probe does not actually cover them.',
   ).toEqual([]);
   expect(
-    staleBudget,
-    "These eager-closure-budgets.ts rows are marked kind: 'facade' but are no longer a package " +
-      'entry surface. Remove the stale row or fix its path.',
+    [...budgeted].filter((file) => !discovered.has(file)).sort(),
+    'These FACADE_BUDGETS rows are no longer a package entry surface. Remove the stale row or ' +
+      'fix its path.',
   ).toEqual([]);
 });
 
-test('discovery is manifest-derived, so it reaches entries with no facades/ directory', () => {
+test('discovery reaches manifest-only façades with no facades/ directory', () => {
   // Non-vacuity with a specific target. The platform packages publish `./src/index.ts` and have no
   // `facades/` directory at all, so a directory-only scan omits precisely the files ADR-0019's
   // implementation-laziness rule is about while every assertion above stays green.
@@ -94,27 +187,19 @@ test('every budgeted entry file exists on disk', () => {
   );
 });
 
-test.for(EAGER_CLOSURE_BUDGETS)(
-  '$id evaluates at most $budget modules',
-  (entry: EagerClosureBudget) => {
-    const entryPath = path.resolve(repoRoot, entry.entryFile);
-    const graph = eagerClosureGraphOf(entryPath);
-    // Report the newest arrivals by their import chain rather than dumping a sorted set: the
-    // chain is what turns "this is over budget" into "this import is why" (#1960).
-    const chains = [...graph.keys()]
-      .filter((file) => file !== entryPath)
-      .map((file) => formatImportChain(graph, file, repoRoot))
-      .sort();
-    expect(
-      graph.size,
-      `${entry.id} evaluates ${graph.size} modules on import, over its budget of ${entry.budget}.` +
-        '\nBudgets here are exact ratchets, not ceilings with slack: either something that used ' +
-        'to load on demand now loads eagerly (fix the import), or the growth is deliberate and ' +
-        'this row moves to the new number in the same PR.\nEvery evaluated module, as the import ' +
-        `chain that pulled it in:\n\n${chains.join('\n\n')}`,
-    ).toBeLessThanOrEqual(entry.budget);
-  },
-);
+test.for(EAGER_CLOSURE_BUDGETS)('$id evaluates exactly $budget modules', (entry) => {
+  const entryPath = path.resolve(repoRoot, entry.entryFile);
+  const graph = eagerClosureGraphOf(entryPath);
+  const finding = classifyBudget(entry.id, graph.size, entry.budget);
+  expect(
+    finding,
+    finding === null
+      ? ''
+      : `${finding}\n\nWhere the weight comes from (heaviest direct edges, capped -- this ` +
+          'attributes by shortest import route, it does not diff against a recorded baseline):\n' +
+          describeClosurePressure(graph, entryPath, repoRoot),
+  ).toBeNull();
+});
 
 test.for(EAGER_CLOSURE_BUDGETS.filter((entry) => entry.denyPlatformImplementations))(
   '$id never evaluates a concrete platform implementation',