From f12062afcdc85dd265901c7b90b4a05ed759d04a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 22 Aug 2026 14:04:32 +0200 Subject: [PATCH 1/3] test(structure): per-package eager-closure budgets (#1960) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0019 requires platform-package façades to stay implementation-lazy and is explicit that a startup threshold alone is not a substitute for preserving the loading shape. #1950 built the AST-level walker (eager-import-closure.fixtures.ts) and proved the planted-red procedure on one file (session-teardown.ts's android-helper denylist); this generalizes it into a data-driven budget table so any workspace-package façade -- or a designated hub module -- can get an eager-closure ceiling without a bespoke test. Seeds a budget for every packages/*/src/facades/*.ts file (discovered the same way package-boundaries.test.ts discovers façades, not hand-listed) from its measured current closure size, plus a platform-implementation denylist for façades whose contract is implementation-neutral vocabulary. Also demonstrates the mechanism on two designated hub modules (cli.ts, session-teardown.ts) alongside their existing, more specific ad hoc pins. Closes #1960 --- src/__tests__/eager-closure-budgets.test.ts | 131 ++++++++++++++++++++ src/__tests__/eager-closure-budgets.ts | 103 +++++++++++++++ 2 files changed, 234 insertions(+) create mode 100644 src/__tests__/eager-closure-budgets.test.ts create mode 100644 src/__tests__/eager-closure-budgets.ts diff --git a/src/__tests__/eager-closure-budgets.test.ts b/src/__tests__/eager-closure-budgets.test.ts new file mode 100644 index 000000000..120ede8f5 --- /dev/null +++ b/src/__tests__/eager-closure-budgets.test.ts @@ -0,0 +1,131 @@ +import { expect, test } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { eagerClosureOf } from './eager-import-closure.fixtures.ts'; +import { + EAGER_CLOSURE_BUDGETS, + PLATFORM_IMPLEMENTATION_PATTERNS, + type EagerClosureBudget, +} from './eager-closure-budgets.ts'; + +/** + * The ADR-0019 loading-shape probe, generalized (#1739, #1960). + * + * ADR-0019 requires platform-package façades to stay implementation-lazy and is explicit that a + * startup threshold alone is not a substitute for preserving the loading shape: "the tracking + * 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 façade plus designated hub modules, driven by the + * data table in `eager-closure-budgets.ts` instead of one-off pins. + * + * - Catches: a façade or vocabulary entry silently going eager -- the regression class #1950 + * fixed once (the android teardown-helper pin below) and #1956/#1959 fix at two more sites. + * Before this file, nothing prevented a fourth instance: layering R3/R13 govern import + * *direction* (may this file import that one at all), not *evaluation weight* (how much of the + * repo importing it drags along). + * - Evidence: the planted-red procedure that proved the #1950 pin re-verified here (see the git + * history on this file's introduction for the observed failing run); simulation on the whole + * unit-core suite showed single-file regressions of this class cost 5-12% of the suite's total + * import work each. + * - Cost: one unit-lane test file plus one data module; the walker parses each reachable file once + * per entry (whole-table run is a fraction of a second), no subprocess, no device. + * - Kill criterion: if two consecutive quarters show no budget 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. + */ + +const repoRoot = path.resolve(import.meta.dirname, '../..'); + +/** + * Every `.ts` file under a `src/facades/` directory, discovered the same way + * `package-boundaries.test.ts` discovers façades (directory membership, not a hand-maintained + * list) -- so a new façade directory shows up here, and must gain a table entry, the moment it + * exists. + */ +function discoverFacadeFiles(): string[] { + const packagesDir = path.join(repoRoot, 'packages'); + const files: string[] = []; + for (const pkgEntry of fs.readdirSync(packagesDir).sort()) { + const facadesDir = path.join(packagesDir, pkgEntry, 'src/facades'); + if (!fs.existsSync(facadesDir)) continue; + for (const file of fs.readdirSync(facadesDir).sort()) { + if (file.endsWith('.ts')) files.push(path.join(facadesDir, file)); + } + } + return files; +} + +function relList(files: readonly string[]): string[] { + return [...files].map((file) => path.relative(repoRoot, file)).sort(); +} + +test('every discovered façade has exactly one budget entry, and no budget entry is stale', () => { + // Bidirectional, mirroring the repo's other exhaustiveness gates (R7/R10 field checklists, + // the facade-exports exhaustive re-export check): a façade with no budget would let this whole + // mechanism go silently vacuous for it, and a budget entry for a file that is no longer a + // façade would let the table drift from what it claims to police. + const discovered = new Set(discoverFacadeFiles()); + const facadeEntries = EAGER_CLOSURE_BUDGETS.filter((entry) => entry.kind === 'facade'); + const budgeted = new Set(facadeEntries.map((entry) => path.resolve(repoRoot, entry.entryFile))); + + const missingBudget = relList([...discovered].filter((file) => !budgeted.has(file))); + const staleBudget = relList([...budgeted].filter((file) => !discovered.has(file))); + + expect( + missingBudget, + 'These façades exist under a `src/facades/` directory but have no entry in ' + + 'eager-closure-budgets.ts. Add one (measure the current closure size with eagerClosureOf ' + + 'and round up a few files for headroom), or the loading-shape probe silently does not ' + + 'cover them.', + ).toEqual([]); + expect( + staleBudget, + "These eager-closure-budgets.ts entries are marked kind: 'facade' but no longer name a file " + + 'under a `src/facades/` directory. Remove the stale entry or fix its path.', + ).toEqual([]); +}); + +test('every budgeted entry file exists on disk', () => { + const missing = EAGER_CLOSURE_BUDGETS.filter( + (entry) => !fs.existsSync(path.resolve(repoRoot, entry.entryFile)), + ).map((entry) => entry.entryFile); + expect( + missing, + 'These eager-closure-budgets.ts entries name a file that does not exist.', + ).toEqual([]); +}); + +test.for(EAGER_CLOSURE_BUDGETS)( + '$id stays within its eager-closure budget of $budget modules', + (entry: EagerClosureBudget) => { + const closure = eagerClosureOf(path.resolve(repoRoot, entry.entryFile)); + const evaluated = relList(closure); + expect( + closure.length, + `${entry.id} evaluates ${closure.length} modules on import, over its budget of ` + + `${entry.budget}. Either the budget needs a reviewed increase (name what legitimately ` + + 'grew and why), or something that used to load on demand now loads eagerly -- check the ' + + 'newest entries below against what this entry point should need.\nEvaluated ' + + `(${evaluated.length}):\n${evaluated.join('\n')}`, + ).toBeLessThanOrEqual(entry.budget); + }, +); + +test.for(EAGER_CLOSURE_BUDGETS.filter((entry) => entry.denyPlatformImplementations))( + '$id never evaluates a concrete platform implementation before discovery/binding', + (entry: EagerClosureBudget) => { + const closure = eagerClosureOf(path.resolve(repoRoot, entry.entryFile)); + const offenders = relList( + closure.filter((file) => + PLATFORM_IMPLEMENTATION_PATTERNS.some((pattern) => pattern.test(file)), + ), + ); + expect( + offenders, + `${entry.id} is implementation-neutral vocabulary (ADR-0019) and must not evaluate a ` + + 'concrete platform implementation until discovery/binding selects one. Load the ' + + `offending module(s) on demand instead of with a static value import:\n${offenders.join('\n')}`, + ).toEqual([]); + }, +); diff --git a/src/__tests__/eager-closure-budgets.ts b/src/__tests__/eager-closure-budgets.ts new file mode 100644 index 000000000..278aea40d --- /dev/null +++ b/src/__tests__/eager-closure-budgets.ts @@ -0,0 +1,103 @@ +// Per-entry eager-closure budgets -- the ADR-0019 loading-shape probe (#1739, #1960). +// +// ADR-0019's "Implementation-laziness" section requires platform-package façades to stay +// implementation-lazy and is explicit that a startup-time threshold alone is not a substitute for +// preserving the loading shape (`docs/adr/0019-request-bound-platform-runtime.md`): "the tracking +// issue owns the exact probe and planted-red procedure." #1950 built the AST-level walker +// (`eager-import-closure.fixtures.ts`) and proved the planted-red procedure on one file +// (`session-teardown.ts`'s android-helper denylist, kept below as its own ad hoc test). This +// table generalizes that proof: every workspace-package façade gets a numeric ceiling on how many +// repo modules importing it may evaluate, plus -- for façades whose contract is +// implementation-neutral vocabulary -- a standing assertion that the closure never reaches a +// concrete platform implementation before discovery/binding selects one. +// +// Entry files are repo-root-relative. + +export type EagerClosureBudget = { + /** Stable label for test names and failure messages -- the entry's repo-relative path. */ + id: string; + /** Repo-root-relative path to the module a consumer imports. */ + entryFile: string; + /** + * Upper bound on `eagerClosureOf(entryFile).length`. Seeded from the file's CURRENT measured + * closure size (recorded in the trailing comment on each entry below) plus a few files of + * headroom for ordinary drift. A regression of the class this gate exists to catch -- a static + * value import of a heavy module that used to be lazy -- costs tens to low-hundreds of extra + * evaluated modules (#1960 simulation: single-file regressions of this class cost 5-12% of + * suite import work each), far past any of this table's headroom. + */ + budget: number; + /** + * 'facade' entries are discovered mechanically -- every `.ts` file under a `src/facades/` + * directory -- and the exhaustiveness test in `eager-closure-budgets.test.ts` requires each one + * to appear here exactly once. 'hub' entries are hand-designated, high-fan-in files that + * value-import a façade for only a slice of it (ADR-0019's other named case). There is no + * mechanical way to enumerate "every hub" the way a façade directory enumerates every façade, + * so membership is a reviewed judgment call -- the same judgment call the two existing ad hoc + * pins this table's mechanism is built to generalize already made by hand. + */ + kind: 'facade' | 'hub'; + /** + * When true, the closure must never evaluate a concrete platform implementation (matched by + * `PLATFORM_IMPLEMENTATION_PATTERNS`) -- the ADR-0019 rule that a façade whose contract is + * implementation-neutral vocabulary may not evaluate implementation before discovery/binding + * selects one. Hub entries default this false: a hub is a *consumer* of façades, not vocabulary + * itself, and may legitimately reach one platform family already (e.g. session teardown's Apple + * perf cleanup steps) through its own lazy seam. Verified, not merely assumed -- see the + * measurement note on the `src/daemon/session-teardown.ts` entry below. + */ + denyPlatformImplementations: boolean; +}; + +/** + * A concrete platform implementation: the legacy daemon-owned `src/platforms//` tree, or + * a `@agent-device/platform-` workspace package. ADR-0019 names both as "concrete device + * mechanics" that platform-neutral code may depend on only through contracts, never directly. + */ +export const PLATFORM_IMPLEMENTATION_PATTERNS: RegExp[] = [ + /[/\\]platforms[/\\](apple|android|harmonyos|vega|linux|web)[/\\]/, + /[/\\]packages[/\\]platform-(apple|android|harmonyos|vega|linux|web)[/\\]/, +]; + +function facade(entryFile: string, budget: number): EagerClosureBudget { + return { id: entryFile, entryFile, budget, kind: 'facade', denyPlatformImplementations: true }; +} + +function hub(entryFile: string, budget: number): EagerClosureBudget { + return { id: entryFile, entryFile, budget, kind: 'hub', denyPlatformImplementations: false }; +} + +export const EAGER_CLOSURE_BUDGETS: EagerClosureBudget[] = [ + // packages/contracts/src/facades/*.ts -- measured 2026-08-22 on origin/main (base e5bfde3d1); + // budget = actual + a few files of headroom. All 14 are implementation-neutral vocabulary per + // ADR-0019 ("Contracts may depend on kernel vocabulary but never on concrete platform + // packages or daemon implementation types"), so all deny platform implementations. + facade('packages/contracts/src/facades/client.ts', 4), // actual 2 + facade('packages/contracts/src/facades/command.ts', 12), // actual 9 + facade('packages/contracts/src/facades/device.ts', 11), // actual 8 + facade('packages/contracts/src/facades/interaction.ts', 30), // actual 25 + facade('packages/contracts/src/facades/capture.ts', 12), // actual 9 + facade('packages/contracts/src/facades/platform.ts', 48), // actual 42 + facade('packages/contracts/src/facades/session.ts', 8), // actual 5 + facade('packages/contracts/src/facades/recording.ts', 6), // actual 3 + facade('packages/contracts/src/facades/observability.ts', 10), // actual 7 + facade('packages/contracts/src/facades/remote.ts', 4), // actual 2 + facade('packages/contracts/src/facades/replay.ts', 6), // actual 3 + facade('packages/contracts/src/facades/snapshot.ts', 11), // actual 8 + facade('packages/contracts/src/facades/divergence.ts', 6), // actual 3 + facade('packages/contracts/src/facades/progress.ts', 3), // actual 1 + + // Designated hub modules (ADR-0019's other named case): high-fan-in entry points that already + // hold their own ad hoc eager-closure pin (`cli-startup-import-closure.test.ts`, + // `session-teardown-import-closure.test.ts`). Those files keep their existing, MORE specific + // assertions -- each names an individual known-expensive module, a stronger property for that + // one module than a numeric ceiling gives. This table adds a second, general-purpose layer: a + // size ceiling that catches ANY unexpected growth at the entry, not only the one pattern each ad + // hoc test already watches for. + hub('src/cli.ts', 420), // actual 386 + // session-teardown.ts already eagerly evaluates 24 Apple perf/runner modules under + // src/platforms/apple/ (that migration hasn't happened yet -- only the Android perf/ + // snapshot-helper pair is lazy today, per the ad hoc test above), so denyPlatformImplementations + // stays false here: turning it on would fail on today's real, reviewed state, not a regression. + hub('src/daemon/session-teardown.ts', 140), // actual 121 +]; From 9bdeb56ea6ff759346196ed9aa99389e77e3be4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 22 Aug 2026 15:50:07 +0200 Subject: [PATCH 2/3] test(structure): derive facade roots from manifests, add edge chains, reseed tight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on #1965: 1. Discovery scanned only `packages/*/src/facades/*.ts`, which omits every package that publishes its entry surface straight from the manifest — including all six `packages/platform-*/src/index.ts` façades, the exact subject of ADR-0019's implementation-laziness rule. Discovery now derives from `readWorkspacePackages(...).exportTargets` and then adds `/src/facades/` files, reusing the R11 helper rather than reimplementing it so the two gates cannot disagree about what an entry surface is. The table grows from 14 façades + 2 hubs to 95 entry surfaces + 8 hubs. 2. Budgets carried a few files of slack each. They are now exact ratchets with no headroom, matching how the repo pins R9/R10 and test-file size: growth is allowed, it just has to be a visible number change in the diff of the PR that causes it. Every budget is reseeded from post-#1969 measurement. 3. Violations printed a flat sorted set, which named the offender but not the route. `eagerClosureGraphOf` records each file's discoverer, so failures now print the transitive chain entry -> ... -> offender. `eagerClosureOf` keeps its contract and is expressed in terms of the new walk; per-file edges are memoized, which also cuts the existing pins' runtime (cli closure test 2336ms -> ~550ms). The platform-package façades evaluate exactly one module each — themselves — so their budget of 1 is the tightest statement of "metadata-eager, implementation-lazy" the walker can make. --- src/__tests__/eager-closure-budgets.test.ts | 155 ++++----- src/__tests__/eager-closure-budgets.ts | 295 ++++++++++++++---- .../eager-import-closure.fixtures.ts | 68 +++- 3 files changed, 373 insertions(+), 145 deletions(-) diff --git a/src/__tests__/eager-closure-budgets.test.ts b/src/__tests__/eager-closure-budgets.test.ts index 120ede8f5..488220d25 100644 --- a/src/__tests__/eager-closure-budgets.test.ts +++ b/src/__tests__/eager-closure-budgets.test.ts @@ -1,9 +1,11 @@ import { expect, test } from 'vitest'; import fs from 'node:fs'; import path from 'node:path'; -import { eagerClosureOf } from './eager-import-closure.fixtures.ts'; +import { eagerClosureGraphOf } from './eager-import-closure.fixtures.ts'; import { + discoverFacadeEntryFiles, EAGER_CLOSURE_BUDGETS, + formatImportChain, PLATFORM_IMPLEMENTATION_PATTERNS, type EagerClosureBudget, } from './eager-closure-budgets.ts'; @@ -16,20 +18,17 @@ 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 façade plus designated hub modules, driven by the - * data table in `eager-closure-budgets.ts` instead of one-off pins. + * 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. * - * - Catches: a façade or vocabulary entry silently going eager -- the regression class #1950 - * fixed once (the android teardown-helper pin below) and #1956/#1959 fix at two more sites. - * Before this file, nothing prevented a fourth instance: layering R3/R13 govern import - * *direction* (may this file import that one at all), not *evaluation weight* (how much of the - * repo importing it drags along). - * - Evidence: the planted-red procedure that proved the #1950 pin re-verified here (see the git - * history on this file's introduction for the observed failing run); simulation on the whole - * unit-core suite showed single-file regressions of this class cost 5-12% of the suite's total - * import work each. - * - Cost: one unit-lane test file plus one data module; the walker parses each reachable file once - * per entry (whole-table run is a fraction of a second), no subprocess, no device. + * - 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. + * - 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 * ADR-0019 composition lands a stronger structural proof of the loading shape, delete this gate * in favor of that proof. @@ -37,95 +36,105 @@ import { const repoRoot = path.resolve(import.meta.dirname, '../..'); -/** - * Every `.ts` file under a `src/facades/` directory, discovered the same way - * `package-boundaries.test.ts` discovers façades (directory membership, not a hand-maintained - * list) -- so a new façade directory shows up here, and must gain a table entry, the moment it - * exists. - */ -function discoverFacadeFiles(): string[] { - const packagesDir = path.join(repoRoot, 'packages'); - const files: string[] = []; - for (const pkgEntry of fs.readdirSync(packagesDir).sort()) { - const facadesDir = path.join(packagesDir, pkgEntry, 'src/facades'); - if (!fs.existsSync(facadesDir)) continue; - for (const file of fs.readdirSync(facadesDir).sort()) { - if (file.endsWith('.ts')) files.push(path.join(facadesDir, file)); - } - } - return files; -} - function relList(files: readonly string[]): string[] { return [...files].map((file) => path.relative(repoRoot, file)).sort(); } -test('every discovered façade has exactly one budget entry, and no budget entry is stale', () => { - // Bidirectional, mirroring the repo's other exhaustiveness gates (R7/R10 field checklists, - // the facade-exports exhaustive re-export check): a façade with no budget would let this whole - // mechanism go silently vacuous for it, and a budget entry for a file that is no longer a - // façade would let the table drift from what it claims to police. - const discovered = new Set(discoverFacadeFiles()); - const facadeEntries = EAGER_CLOSURE_BUDGETS.filter((entry) => entry.kind === 'facade'); - const budgeted = new Set(facadeEntries.map((entry) => path.resolve(repoRoot, entry.entryFile))); +test('every discovered entry surface has exactly one budget entry, 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 + // 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 + // 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 = relList([...discovered].filter((file) => !budgeted.has(file))); - const staleBudget = relList([...budgeted].filter((file) => !discovered.has(file))); + const missingBudget = [...discovered].filter((file) => !budgeted.has(file)).sort(); + const staleBudget = [...budgeted].filter((file) => !discovered.has(file)).sort(); expect( missingBudget, - 'These façades exist under a `src/facades/` directory but have no entry in ' + - 'eager-closure-budgets.ts. Add one (measure the current closure size with eagerClosureOf ' + - 'and round up a few files for headroom), or the loading-shape probe silently does not ' + - 'cover them.', + '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.', ).toEqual([]); expect( staleBudget, - "These eager-closure-budgets.ts entries are marked kind: 'facade' but no longer name a file " + - 'under a `src/facades/` directory. Remove the stale entry or fix its path.', + "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.', ).toEqual([]); }); +test('discovery is manifest-derived, so it reaches entries 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. + const discovered = discoverFacadeEntryFiles(repoRoot); + expect(discovered.length).toBeGreaterThan(50); + for (const family of ['apple', 'android', 'harmonyos', 'vega', 'linux', 'web']) { + expect( + discovered, + `packages/platform-${family} publishes its façade through the manifest, not a facades/ ` + + 'directory. If discovery stops finding it, the gate has lost the ADR-0019 subject.', + ).toContain(`packages/platform-${family}/src/index.ts`); + } +}); + test('every budgeted entry file exists on disk', () => { const missing = EAGER_CLOSURE_BUDGETS.filter( (entry) => !fs.existsSync(path.resolve(repoRoot, entry.entryFile)), ).map((entry) => entry.entryFile); - expect( - missing, - 'These eager-closure-budgets.ts entries name a file that does not exist.', - ).toEqual([]); + expect(missing, 'These eager-closure-budgets.ts rows name a file that does not exist.').toEqual( + [], + ); }); test.for(EAGER_CLOSURE_BUDGETS)( - '$id stays within its eager-closure budget of $budget modules', + '$id evaluates at most $budget modules', (entry: EagerClosureBudget) => { - const closure = eagerClosureOf(path.resolve(repoRoot, entry.entryFile)); - const evaluated = relList(closure); + 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( - closure.length, - `${entry.id} evaluates ${closure.length} modules on import, over its budget of ` + - `${entry.budget}. Either the budget needs a reviewed increase (name what legitimately ` + - 'grew and why), or something that used to load on demand now loads eagerly -- check the ' + - 'newest entries below against what this entry point should need.\nEvaluated ' + - `(${evaluated.length}):\n${evaluated.join('\n')}`, + 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.filter((entry) => entry.denyPlatformImplementations))( - '$id never evaluates a concrete platform implementation before discovery/binding', + '$id never evaluates a concrete platform implementation', (entry: EagerClosureBudget) => { - const closure = eagerClosureOf(path.resolve(repoRoot, entry.entryFile)); - const offenders = relList( - closure.filter((file) => - PLATFORM_IMPLEMENTATION_PATTERNS.some((pattern) => pattern.test(file)), - ), - ); + const entryPath = path.resolve(repoRoot, entry.entryFile); + const graph = eagerClosureGraphOf(entryPath); + // The entry itself is excluded: a platform package's own façade necessarily matches the + // pattern, and the property worth asserting there is that it evaluates none of its OWN + // mechanics either. + const offenders = [...graph.keys()] + .filter((file) => file !== entryPath) + .filter((file) => PLATFORM_IMPLEMENTATION_PATTERNS.some((pattern) => pattern.test(file))); + const chains = offenders.map((file) => formatImportChain(graph, file, repoRoot)).sort(); + expect( - offenders, - `${entry.id} is implementation-neutral vocabulary (ADR-0019) and must not evaluate a ` + - 'concrete platform implementation until discovery/binding selects one. Load the ' + - `offending module(s) on demand instead of with a static value import:\n${offenders.join('\n')}`, + relList(offenders), + `${entry.id} must not evaluate a concrete platform implementation before discovery or ` + + 'binding selects an owner (ADR-0019: the registry is metadata-eager and ' + + 'implementation-lazy). Move the offending edge behind a function-scoped `await import`. ' + + `The chains that reach implementation:\n\n${chains.join('\n\n')}`, ).toEqual([]); }, ); diff --git a/src/__tests__/eager-closure-budgets.ts b/src/__tests__/eager-closure-budgets.ts index 278aea40d..3e41f350a 100644 --- a/src/__tests__/eager-closure-budgets.ts +++ b/src/__tests__/eager-closure-budgets.ts @@ -4,61 +4,135 @@ // implementation-lazy and is explicit that a startup-time threshold alone is not a substitute for // preserving the loading shape (`docs/adr/0019-request-bound-platform-runtime.md`): "the tracking // issue owns the exact probe and planted-red procedure." #1950 built the AST-level walker -// (`eager-import-closure.fixtures.ts`) and proved the planted-red procedure on one file -// (`session-teardown.ts`'s android-helper denylist, kept below as its own ad hoc test). This -// table generalizes that proof: every workspace-package façade gets a numeric ceiling on how many -// repo modules importing it may evaluate, plus -- for façades whose contract is -// implementation-neutral vocabulary -- a standing assertion that the closure never reaches a -// concrete platform implementation before discovery/binding selects one. +// (`eager-import-closure.fixtures.ts`); #1959/#1969 fixed two more instances of the regression +// class by hand. This table generalizes the proof: every package entry surface gets a numeric +// ceiling on how many repo modules importing it may evaluate, plus a standing assertion that the +// closure never reaches a concrete platform implementation before discovery/binding selects one. +// +// The six `packages/platform-*/src/index.ts` façades are the reason this gate exists. Each one +// evaluates exactly ONE module today -- itself -- because its metadata is inline, its contract +// imports are `import type` (erased), and every implementation loads through a function-scoped +// `await import`. That is precisely ADR-0019's "metadata-eager and implementation-lazy" property, +// and a single static value import would silently destroy it while every other gate stayed green: +// R3/R13 govern import DIRECTION (may this file reach that one at all), never evaluation WEIGHT. // // Entry files are repo-root-relative. +import fs from 'node:fs'; +import path from 'node:path'; +import { readWorkspacePackages } from '../../scripts/layering/package-boundaries.ts'; + export type EagerClosureBudget = { /** Stable label for test names and failure messages -- the entry's repo-relative path. */ id: string; /** Repo-root-relative path to the module a consumer imports. */ entryFile: string; /** - * Upper bound on `eagerClosureOf(entryFile).length`. Seeded from the file's CURRENT measured - * closure size (recorded in the trailing comment on each entry below) plus a few files of - * headroom for ordinary drift. A regression of the class this gate exists to catch -- a static - * value import of a heavy module that used to be lazy -- costs tens to low-hundreds of extra - * evaluated modules (#1960 simulation: single-file regressions of this class cost 5-12% of - * suite import work each), far past any of this table's headroom. + * Exact number of repo modules `eagerClosureOf(entryFile)` evaluates today, asserted as an + * upper bound (`<=`). Seeded from measurement, with NO headroom: this is a ratchet, matching + * how the repo pins R9 type-cycle size, R10 writer/owner counts, and test-file line counts -- + * "existing pins only shrink; a new pin requires measured justification" + * (`docs/agents/testing.md`). Slack is not neutral here. The regression this gate exists to + * catch is a single static import that drags a subtree in, and #1969 measured that class at + * 5-12% of the whole suite's import work each; a ceiling carrying "a few files" of spare room + * is a ceiling that silently absorbs the small end of exactly that. Growth is fine -- it just + * has to be a visible number change in the diff of the PR that causes it. */ budget: number; /** - * 'facade' entries are discovered mechanically -- every `.ts` file under a `src/facades/` - * directory -- and the exhaustiveness test in `eager-closure-budgets.test.ts` requires each one - * to appear here exactly once. 'hub' entries are hand-designated, high-fan-in files that - * value-import a façade for only a slice of it (ADR-0019's other named case). There is no - * mechanical way to enumerate "every hub" the way a façade directory enumerates every façade, - * so membership is a reviewed judgment call -- the same judgment call the two existing ad hoc - * pins this table's mechanism is built to generalize already made by hand. + * 'facade' entries are discovered mechanically by `discoverFacadeEntryFiles`, and the + * exhaustiveness test in `eager-closure-budgets.test.ts` requires each discovered file to appear + * here exactly once. 'hub' entries are hand-designated, high-fan-in modules that value-import an + * entry surface for only a slice of it (ADR-0019's other named case, and the specific shape + * #1969 fixed at five sites). There is no mechanical way to enumerate "every hub" the way a + * manifest enumerates every entry surface, so hub membership is a reviewed judgment call. */ kind: 'facade' | 'hub'; /** - * When true, the closure must never evaluate a concrete platform implementation (matched by - * `PLATFORM_IMPLEMENTATION_PATTERNS`) -- the ADR-0019 rule that a façade whose contract is - * implementation-neutral vocabulary may not evaluate implementation before discovery/binding - * selects one. Hub entries default this false: a hub is a *consumer* of façades, not vocabulary - * itself, and may legitimately reach one platform family already (e.g. session teardown's Apple - * perf cleanup steps) through its own lazy seam. Verified, not merely assumed -- see the - * measurement note on the `src/daemon/session-teardown.ts` entry below. + * When true, the closure must not evaluate any concrete platform implementation + * (`PLATFORM_IMPLEMENTATION_PATTERNS`) OTHER than the entry file itself -- ADR-0019's rule that + * implementation must not load before discovery/binding selects an owner. The self-exclusion is + * what makes this meaningful for the platform packages: `packages/platform-apple/src/index.ts` + * necessarily matches the pattern, so a naive check could only ever be vacuously false there; + * excluding just the entry turns the assertion into "this façade evaluates none of its own + * mechanics", which is the actual ADR-0019 property. + * + * Every package entry surface sets this true (verified: none reaches an implementation today). + * Hub entries set it false -- a hub is a CONSUMER of façades, not neutral vocabulary, and three + * of them legitimately hold the R3-permitted static platform seam that has not migrated yet. */ denyPlatformImplementations: boolean; }; /** * A concrete platform implementation: the legacy daemon-owned `src/platforms//` tree, or - * a `@agent-device/platform-` workspace package. ADR-0019 names both as "concrete device - * mechanics" that platform-neutral code may depend on only through contracts, never directly. + * a private `@agent-device/platform-` workspace package. ADR-0019 names both as "concrete + * device mechanics" that platform-neutral code reaches only through contracts. */ export const PLATFORM_IMPLEMENTATION_PATTERNS: RegExp[] = [ /[/\\]platforms[/\\](apple|android|harmonyos|vega|linux|web)[/\\]/, /[/\\]packages[/\\]platform-(apple|android|harmonyos|vega|linux|web)[/\\]/, ]; +/** + * Every workspace-package entry surface, repo-root-relative and sorted. + * + * Ownership is the package MANIFEST: whatever a `package.json` `exports` map points at is an entry + * a consumer can import, so that is what needs a loading-shape budget. Files under a + * `src/facades/` directory are added on top, exactly as `scripts/layering/package-boundaries.ts`'s + * R11 façade gate composes its own set (`readWorkspacePackages(...).exportTargets` plus every + * `/src/facades/` source), and for the same reason: a façade directory is a façade whether or not + * a manifest happens to point at it yet. + * + * Deriving from manifests rather than scanning `packages//src/facades/` is not a detail. Six + * platform packages have no `facades/` directory at all -- each publishes `./src/index.ts` -- so a + * directory-only scan silently omits the exact files ADR-0019's implementation-laziness rule is + * about, and the gate would claim to prove the loading shape while never looking at it. + * + * `readWorkspacePackages` is reused rather than reimplemented so the two gates cannot drift into + * disagreeing about what a package entry surface is. + */ +export function discoverFacadeEntryFiles(repoRoot: string): string[] { + const found = new Set(); + for (const pkg of readWorkspacePackages(repoRoot)) { + for (const target of pkg.exportTargets.values()) found.add(target); + } + const packagesDir = path.join(repoRoot, 'packages'); + const srcRoots = ['src']; + for (const entry of fs.readdirSync(packagesDir).sort()) { + if (fs.existsSync(path.join(packagesDir, entry, 'src'))) srcRoots.push(`packages/${entry}/src`); + } + for (const srcRoot of srcRoots) { + const facadesDir = path.join(repoRoot, srcRoot, 'facades'); + if (!fs.existsSync(facadesDir)) continue; + for (const file of fs.readdirSync(facadesDir).sort()) { + if (file.endsWith('.ts')) found.add(`${srcRoot}/facades/${file}`); + } + } + return [...found].filter((file) => fs.existsSync(path.join(repoRoot, file))).sort(); +} + +/** + * The import chain from a closure's entry down to `target`, rendered one edge per line. + * + * #1960 asks a violation to "name the offending edge chain". A sorted set of evaluated files names + * the destination but not the route, which leaves the reader to rediscover by hand which import + * actually pulled it in -- the "budget exceeded, go spelunking" failure the issue rules out. + * `eagerClosureGraphOf` records each file's discoverer, so the route is just a walk back up. + */ +export function formatImportChain( + graph: ReadonlyMap, + target: string, + repoRoot: string, +): string { + const chain: string[] = []; + for (let at: string | null | undefined = target; at != null; at = graph.get(at)) { + chain.push(path.relative(repoRoot, at)); + if (chain.length > 64) break; // defensive: a cycle would otherwise spin here + } + return chain.reverse().join('\n -> '); +} + function facade(entryFile: string, budget: number): EagerClosureBudget { return { id: entryFile, entryFile, budget, kind: 'facade', denyPlatformImplementations: true }; } @@ -67,37 +141,140 @@ function hub(entryFile: string, budget: number): EagerClosureBudget { return { id: entryFile, entryFile, budget, kind: 'hub', denyPlatformImplementations: false }; } +/** + * Measured 2026-08-22 on `03c398406` (post-#1969, which granularized the contracts entry surface + * from 15 subpaths to 70 and moved the hubs off the wide façades). Budgets are exact; see the + * `budget` field doc for why there is no headroom. + */ export const EAGER_CLOSURE_BUDGETS: EagerClosureBudget[] = [ - // packages/contracts/src/facades/*.ts -- measured 2026-08-22 on origin/main (base e5bfde3d1); - // budget = actual + a few files of headroom. All 14 are implementation-neutral vocabulary per - // ADR-0019 ("Contracts may depend on kernel vocabulary but never on concrete platform - // packages or daemon implementation types"), so all deny platform implementations. - facade('packages/contracts/src/facades/client.ts', 4), // actual 2 - facade('packages/contracts/src/facades/command.ts', 12), // actual 9 - facade('packages/contracts/src/facades/device.ts', 11), // actual 8 - facade('packages/contracts/src/facades/interaction.ts', 30), // actual 25 - facade('packages/contracts/src/facades/capture.ts', 12), // actual 9 - facade('packages/contracts/src/facades/platform.ts', 48), // actual 42 - facade('packages/contracts/src/facades/session.ts', 8), // actual 5 - facade('packages/contracts/src/facades/recording.ts', 6), // actual 3 - facade('packages/contracts/src/facades/observability.ts', 10), // actual 7 - facade('packages/contracts/src/facades/remote.ts', 4), // actual 2 - facade('packages/contracts/src/facades/replay.ts', 6), // actual 3 - facade('packages/contracts/src/facades/snapshot.ts', 11), // actual 8 - facade('packages/contracts/src/facades/divergence.ts', 6), // actual 3 - facade('packages/contracts/src/facades/progress.ts', 3), // actual 1 + // --- @agent-device/ad-replay / ad-script / capture-kit --- + facade('packages/ad-replay/src/index.ts', 58), + facade('packages/ad-script/src/index.ts', 37), + facade('packages/capture-kit/src/index.ts', 26), + + // --- @agent-device/contracts: the shared vocabulary package. #1969 gave every module its own + // entry subpath precisely so a consumer needing one symbol stops evaluating a 32-module union; + // these budgets are what keeps each narrow entry narrow. + facade('packages/contracts/src/alert-contract.ts', 1), + facade('packages/contracts/src/android-input-ownership.ts', 1), + facade('packages/contracts/src/android-snapshot-quality.ts', 1), + facade('packages/contracts/src/android-system-chrome.ts', 1), + facade('packages/contracts/src/app-deployment-runtime-plan.ts', 3), + facade('packages/contracts/src/app-deployment-runtime.ts', 1), + facade('packages/contracts/src/app-inventory-runtime.ts', 1), + facade('packages/contracts/src/app-log-runtime.ts', 1), + facade('packages/contracts/src/app-state-runtime.ts', 1), + facade('packages/contracts/src/apple-multitouch-support.ts', 5), + facade('packages/contracts/src/application-lifecycle-interaction.ts', 7), + facade('packages/contracts/src/application-lifecycle-runtime-plan.ts', 3), + facade('packages/contracts/src/application-lifecycle-runtime.ts', 1), + facade('packages/contracts/src/async-lifecycle.ts', 1), + facade('packages/contracts/src/audio-probe-result.ts', 1), + facade('packages/contracts/src/audio-probe-support.ts', 5), + facade('packages/contracts/src/back-mode.ts', 1), + facade('packages/contracts/src/click-button.ts', 3), + facade('packages/contracts/src/command-platform-execution.ts', 2), + facade('packages/contracts/src/device-readiness-runtime.ts', 1), + facade('packages/contracts/src/device-shutdown-runtime.ts', 1), + facade('packages/contracts/src/durable-resource-envelope.ts', 1), + facade('packages/contracts/src/durable-resource.ts', 1), + facade('packages/contracts/src/element-text-runtime.ts', 4), + facade('packages/contracts/src/facades/capture.ts', 9), + facade('packages/contracts/src/facades/client.ts', 2), + facade('packages/contracts/src/facades/command.ts', 9), + facade('packages/contracts/src/facades/device.ts', 8), + facade('packages/contracts/src/facades/divergence.ts', 3), + facade('packages/contracts/src/facades/interaction.ts', 25), + facade('packages/contracts/src/facades/observability.ts', 7), + facade('packages/contracts/src/facades/platform.ts', 42), + facade('packages/contracts/src/facades/progress.ts', 1), + facade('packages/contracts/src/facades/recording.ts', 3), + facade('packages/contracts/src/facades/remote.ts', 2), + facade('packages/contracts/src/facades/replay.ts', 3), + facade('packages/contracts/src/facades/session.ts', 5), + facade('packages/contracts/src/facades/snapshot.ts', 8), + facade('packages/contracts/src/focus-runtime.ts', 4), + facade('packages/contracts/src/gesture-input.ts', 13), + facade('packages/contracts/src/gesture-normalization.ts', 14), + facade('packages/contracts/src/gesture-plan-types.ts', 1), + facade('packages/contracts/src/gesture-plan.ts', 12), + facade('packages/contracts/src/interaction-error.ts', 1), + facade('packages/contracts/src/interaction-guarantees.ts', 1), + facade('packages/contracts/src/interactor-types.ts', 1), + facade('packages/contracts/src/logs-runtime-plan.ts', 5), + facade('packages/contracts/src/navigation.ts', 1), + facade('packages/contracts/src/network-runtime-plan.ts', 5), + facade('packages/contracts/src/network-runtime.ts', 1), + facade('packages/contracts/src/platform-module.ts', 5), + facade('packages/contracts/src/platform-runtime-host.ts', 1), + facade('packages/contracts/src/platform-runtime-operations.ts', 2), + facade('packages/contracts/src/platform-runtime-unavailable.ts', 15), + facade('packages/contracts/src/platform-runtime.ts', 6), + facade('packages/contracts/src/record-runtime-cutover.ts', 7), + facade('packages/contracts/src/screen-recording-runtime-plan.ts', 5), + facade('packages/contracts/src/screen-recording-runtime.ts', 1), + facade('packages/contracts/src/screenshot-runtime.ts', 4), + facade('packages/contracts/src/scroll-command.ts', 3), + facade('packages/contracts/src/scroll-gesture.ts', 10), + facade('packages/contracts/src/selector-observation-runtime.ts', 1), + facade('packages/contracts/src/settings.ts', 3), + facade('packages/contracts/src/snapshot-runtime.ts', 3), + facade('packages/contracts/src/startup-recovery-fence.ts', 1), + facade('packages/contracts/src/tv-remote.ts', 3), + facade('packages/contracts/src/type-text-runtime.ts', 4), + facade('packages/contracts/src/viewport-runtime.ts', 1), + facade('packages/contracts/src/wait-runtime-plan.ts', 1), + facade('packages/contracts/src/wait.ts', 1), + + // --- @agent-device/kernel --- + facade('packages/kernel/src/bounds.ts', 1), + facade('packages/kernel/src/collections.ts', 1), + facade('packages/kernel/src/contracts.ts', 4), + facade('packages/kernel/src/device.ts', 4), + facade('packages/kernel/src/errors.ts', 2), + facade('packages/kernel/src/rect.ts', 1), + facade('packages/kernel/src/redaction.ts', 1), + facade('packages/kernel/src/snapshot.ts', 1), + + // --- @agent-device/maestro --- + facade('packages/maestro/src/index.ts', 104), + + // --- @agent-device/platform-*: ADR-0019's metadata-eager/implementation-lazy façades. Each + // evaluates only itself; every implementation sits behind a function-scoped `await import`. + // A budget of 1 is the tightest statement of that property the walker can make. + facade('packages/platform-android/src/index.ts', 1), + facade('packages/platform-apple/src/index.ts', 1), + facade('packages/platform-harmonyos/src/index.ts', 1), + facade('packages/platform-linux/src/index.ts', 1), + facade('packages/platform-vega/src/index.ts', 1), + facade('packages/platform-web/src/index.ts', 1), + + // --- providers, replay-test, selectors, xml --- + facade('packages/provider-limrun/src/index.ts', 29), + facade('packages/provider-webdriver/src/index.ts', 49), + facade('packages/replay-test/src/index.ts', 19), + facade('packages/selectors/src/ast.ts', 16), + facade('packages/selectors/src/engine.ts', 19), + facade('packages/selectors/src/index.ts', 50), + facade('packages/xml/src/index.ts', 3), - // Designated hub modules (ADR-0019's other named case): high-fan-in entry points that already - // hold their own ad hoc eager-closure pin (`cli-startup-import-closure.test.ts`, - // `session-teardown-import-closure.test.ts`). Those files keep their existing, MORE specific - // assertions -- each names an individual known-expensive module, a stronger property for that - // one module than a numeric ceiling gives. This table adds a second, general-purpose layer: a - // size ceiling that catches ANY unexpected growth at the entry, not only the one pattern each ad - // hoc test already watches for. - hub('src/cli.ts', 420), // actual 386 - // session-teardown.ts already eagerly evaluates 24 Apple perf/runner modules under - // src/platforms/apple/ (that migration hasn't happened yet -- only the Android perf/ - // snapshot-helper pair is lazy today, per the ad hoc test above), so denyPlatformImplementations - // stays false here: turning it on would fail on today's real, reviewed state, not a regression. - hub('src/daemon/session-teardown.ts', 140), // actual 121 + // --- Designated hub modules --- + // High-fan-in entry points whose closure the whole suite (or every CLI run) pays for. The first + // two already carry their own ad hoc pins naming individual expensive modules + // (`cli-startup-import-closure.test.ts`, `session-teardown-import-closure.test.ts`); the five + // after them are the hubs #1969 moved off the wide contracts façades, pinned there by name + // (`contracts-entry-closure.test.ts`). Those tests state a STRONGER property for the one module + // each names; these budgets add the general layer -- any unexpected growth, not only the shape + // someone already thought to forbid. + hub('src/cli.ts', 361), + // The ADR-0019 composition root: the one production module allowed to value-import a concrete + // platform package. It evaluates all six family façades (6 modules, all metadata-only), so its + // budget is also the assertion that composing the registry stays metadata-eager overall. + hub('src/platform-runtime.ts', 31), + hub('src/core/dispatch.ts', 100), + hub('src/core/capabilities.ts', 76), + hub('src/core/command-descriptor/registry.ts', 66), + hub('src/core/command-descriptor/platform-execution-entry.ts', 3), + hub('src/core/interactors/register-builtins.ts', 73), + hub('src/daemon/session-teardown.ts', 89), ]; diff --git a/src/__tests__/eager-import-closure.fixtures.ts b/src/__tests__/eager-import-closure.fixtures.ts index a2cc39471..27bd53bd6 100644 --- a/src/__tests__/eager-import-closure.fixtures.ts +++ b/src/__tests__/eager-import-closure.fixtures.ts @@ -155,21 +155,63 @@ function resolveWorkspace(specifier: string, packageDirs: Map): return fs.existsSync(resolved) ? resolved : null; } -/** Every repo file evaluated as a consequence of importing `entryFile`. */ -export function eagerClosureOf(entryFile: string): string[] { +/** + * The repo files `file` evaluates directly, already resolved to absolute paths. + * + * Memoized: the budget table in `eager-closure-budgets.ts` walks ~100 entries whose + * subtrees overlap heavily (every contracts entry bottoms out in the same kernel + * modules), so without this each shared file is re-read and re-parsed once per entry + * that reaches it. Source files do not change during a run, so the cache is safe for + * the lifetime of the worker. + */ +const directEdgeCache = new Map(); + +function directEagerEdges(file: string, packageDirs: Map): string[] { + const cached = directEdgeCache.get(file); + if (cached) return cached; + const resolvedEdges: string[] = []; + for (const specifier of eagerlyEvaluatedModules(file, fs.readFileSync(file, 'utf8'))) { + const resolved = specifier.startsWith('.') + ? resolveRelative(file, specifier) + : resolveWorkspace(specifier, packageDirs); + if (resolved) resolvedEdges.push(resolved); + } + directEdgeCache.set(file, resolvedEdges); + return resolvedEdges; +} + +/** + * Every repo file evaluated as a consequence of importing `entryFile`, mapped to the + * file that first pulled it in (`null` for the entry itself). + * + * `eagerClosureOf` answers "how much evaluates"; this answers "and through what", + * which is what a violation message needs to be actionable (#1960): a flat set names + * the offender but leaves the reader to rediscover which import chain reaches it. + * Breadth-first, so following the links back yields the SHORTEST chain to each file + * rather than whatever route a depth-first walk happened to take. + */ +export function eagerClosureGraphOf(entryFile: string): Map { const packageDirs = readWorkspacePackageDirs(); + const cameFrom = new Map([[entryFile, null]]); const queue = [entryFile]; - const visited = new Set(); - while (queue.length > 0) { - const current = queue.pop(); - if (!current || visited.has(current)) continue; - visited.add(current); - for (const specifier of eagerlyEvaluatedModules(current, fs.readFileSync(current, 'utf8'))) { - const resolved = specifier.startsWith('.') - ? resolveRelative(current, specifier) - : resolveWorkspace(specifier, packageDirs); - if (resolved) queue.push(resolved); + for (let head = 0; head < queue.length; head += 1) { + const current = queue[head]; + if (current === undefined) continue; + for (const resolved of directEagerEdges(current, packageDirs)) { + if (cameFrom.has(resolved)) continue; + cameFrom.set(resolved, current); + queue.push(resolved); } } - return [...visited]; + return cameFrom; +} + +/** + * Every repo file evaluated as a consequence of importing `entryFile`. + * + * The returned SET is what callers pin; iteration order is unspecified and carries no + * meaning (it changed from depth- to breadth-first when `eagerClosureGraphOf` landed). + */ +export function eagerClosureOf(entryFile: string): string[] { + return [...eagerClosureGraphOf(entryFile).keys()]; } From 6d337d27fbc7295470f40c56eca99d2b592d2950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 22 Aug 2026 18:35:39 +0200 Subject: [PATCH 3/3] test(structure): make the eager-closure pins exact, bounded, and single-owner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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` 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 04e4c23b9. --- scripts/layering/package-boundaries.test.ts | 17 +- scripts/layering/package-boundaries.ts | 30 ++ src/__tests__/eager-closure-budgets.test.ts | 175 +++++-- src/__tests__/eager-closure-budgets.ts | 529 ++++++++++++-------- 4 files changed, 498 insertions(+), 253 deletions(-) diff --git a/scripts/layering/package-boundaries.test.ts b/scripts/layering/package-boundaries.test.ts index 3ad04b620..1d48975dd 100644 --- a/scripts/layering/package-boundaries.test.ts +++ b/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(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); diff --git a/scripts/layering/package-boundaries.ts b/scripts/layering/package-boundaries.ts index 0070d7de3..69204b296 100644 --- a/scripts/layering/package-boundaries.ts +++ b/scripts/layering/package-boundaries.ts @@ -255,6 +255,36 @@ export function rootExternalDependencyRanges(repoRoot: string): Map(); + 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 []; diff --git a/src/__tests__/eager-closure-budgets.test.ts b/src/__tests__/eager-closure-budgets.test.ts index 488220d25..7d21958b4 100644 --- a/src/__tests__/eager-closure-budgets.test.ts +++ b/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([[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([['/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(); + 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', diff --git a/src/__tests__/eager-closure-budgets.ts b/src/__tests__/eager-closure-budgets.ts index 3e41f350a..96e30d948 100644 --- a/src/__tests__/eager-closure-budgets.ts +++ b/src/__tests__/eager-closure-budgets.ts @@ -5,9 +5,9 @@ // preserving the loading shape (`docs/adr/0019-request-bound-platform-runtime.md`): "the tracking // issue owns the exact probe and planted-red procedure." #1950 built the AST-level walker // (`eager-import-closure.fixtures.ts`); #1959/#1969 fixed two more instances of the regression -// class by hand. This table generalizes the proof: every package entry surface gets a numeric -// ceiling on how many repo modules importing it may evaluate, plus a standing assertion that the -// closure never reaches a concrete platform implementation before discovery/binding selects one. +// class by hand. This table generalizes the proof: every package entry surface gets an exact pin +// on how many repo modules importing it evaluates, plus a standing assertion that the closure +// never reaches a concrete platform implementation before discovery/binding selects one. // // The six `packages/platform-*/src/index.ts` façades are the reason this gate exists. Each one // evaluates exactly ONE module today -- itself -- because its metadata is inline, its contract @@ -16,11 +16,13 @@ // and a single static value import would silently destroy it while every other gate stayed green: // R3/R13 govern import DIRECTION (may this file reach that one at all), never evaluation WEIGHT. // -// Entry files are repo-root-relative. +// Entry files are repo-root-relative, and are the KEYS of the two records below. Keying by path +// is what makes a duplicate row unwritable rather than merely discouraged: a repeated key in an +// object literal is a TypeScript error (ts1117), so the "exactly one row per entry" claim is +// enforced by the compiler instead of by a runtime check that a `Set` conversion would hide. -import fs from 'node:fs'; import path from 'node:path'; -import { readWorkspacePackages } from '../../scripts/layering/package-boundaries.ts'; +import { facadeEntryFiles } from '../../scripts/layering/package-boundaries.ts'; export type EagerClosureBudget = { /** Stable label for test names and failure messages -- the entry's repo-relative path. */ @@ -28,24 +30,29 @@ export type EagerClosureBudget = { /** Repo-root-relative path to the module a consumer imports. */ entryFile: string; /** - * Exact number of repo modules `eagerClosureOf(entryFile)` evaluates today, asserted as an - * upper bound (`<=`). Seeded from measurement, with NO headroom: this is a ratchet, matching - * how the repo pins R9 type-cycle size, R10 writer/owner counts, and test-file line counts -- - * "existing pins only shrink; a new pin requires measured justification" - * (`docs/agents/testing.md`). Slack is not neutral here. The regression this gate exists to - * catch is a single static import that drags a subtree in, and #1969 measured that class at - * 5-12% of the whole suite's import work each; a ceiling carrying "a few files" of spare room - * is a ceiling that silently absorbs the small end of exactly that. Growth is fine -- it just - * has to be a visible number change in the diff of the PR that causes it. + * The EXACT number of repo modules `eagerClosureOf(entryFile)` evaluates, asserted with + * equality rather than `<=`. + * + * A `<=` ceiling looks stricter than it is: the moment an entry legitimately shrinks, the + * unchanged row silently becomes headroom, and the next regression up to the old number passes + * unnoticed. Equality is what "only ever ratchets down" actually requires -- the same shape + * `test-file-size-ratchet.test.ts` uses for file length and R9/R10 use for cycle size and + * writer counts: growing fails, and shrinking ALSO fails until the row is lowered in the same + * PR, so the gain is kept rather than banked as slack. + * + * Seeded from measurement, never rounded up. The regression this catches is a single static + * import dragging a subtree in, measured by #1969 at 5-12% of the whole suite's import work + * each; a row carrying "a few files" of spare room silently absorbs the small end of exactly + * that. */ budget: number; /** - * 'facade' entries are discovered mechanically by `discoverFacadeEntryFiles`, and the - * exhaustiveness test in `eager-closure-budgets.test.ts` requires each discovered file to appear - * here exactly once. 'hub' entries are hand-designated, high-fan-in modules that value-import an - * entry surface for only a slice of it (ADR-0019's other named case, and the specific shape - * #1969 fixed at five sites). There is no mechanical way to enumerate "every hub" the way a - * manifest enumerates every entry surface, so hub membership is a reviewed judgment call. + * 'facade' rows are the package entry surfaces discovered by `facadeEntryFiles`, and the + * exhaustiveness test requires every discovered file to have exactly one row. 'hub' rows are + * hand-designated, high-fan-in modules that value-import an entry surface for only a slice of + * it (ADR-0019's other named case, and the shape #1969 fixed at five sites). There is no + * mechanical way to enumerate "every hub" the way a manifest enumerates every entry surface, so + * hub membership is a reviewed judgment call. */ kind: 'facade' | 'hub'; /** @@ -58,8 +65,8 @@ export type EagerClosureBudget = { * mechanics", which is the actual ADR-0019 property. * * Every package entry surface sets this true (verified: none reaches an implementation today). - * Hub entries set it false -- a hub is a CONSUMER of façades, not neutral vocabulary, and three - * of them legitimately hold the R3-permitted static platform seam that has not migrated yet. + * Hub rows set it false -- a hub is a CONSUMER of façades, not neutral vocabulary, and three of + * them legitimately hold the R3-permitted static platform seam that has not migrated yet. */ denyPlatformImplementations: boolean; }; @@ -75,41 +82,309 @@ export const PLATFORM_IMPLEMENTATION_PATTERNS: RegExp[] = [ ]; /** - * Every workspace-package entry surface, repo-root-relative and sorted. + * Every workspace-package entry surface, delegated to the single owner of that question in + * `scripts/layering/package-boundaries.ts`. * - * Ownership is the package MANIFEST: whatever a `package.json` `exports` map points at is an entry - * a consumer can import, so that is what needs a loading-shape budget. Files under a - * `src/facades/` directory are added on top, exactly as `scripts/layering/package-boundaries.ts`'s - * R11 façade gate composes its own set (`readWorkspacePackages(...).exportTargets` plus every - * `/src/facades/` source), and for the same reason: a façade directory is a façade whether or not - * a manifest happens to point at it yet. + * Re-exported rather than reimplemented. The first version of this gate carried its own + * one-level `readdir` of each package's `src/facades`, which disagreed with R11's recursive, + * manifest-first discovery in two ways at once: it missed nested façade files, and it missed + * every package that publishes its entry surface straight from the manifest -- including all six + * `packages/platform-/src/index.ts` façades, the exact subject of the ADR-0019 rule this + * gate exists to enforce. Two discovery implementations is one more than the number that can be + * correct, so there is now one. + */ +export function discoverFacadeEntryFiles(repoRoot: string): string[] { + return facadeEntryFiles(repoRoot); +} + +/** + * Measured 2026-08-22 on `04e4c23b9` (post-#1969, which granularized the contracts entry surface + * from 15 subpaths to 70 and moved the hubs off the wide façades). Exact pins; see the `budget` + * field doc for why there is no headroom. + */ +export const FACADE_BUDGETS: Readonly> = Object.freeze({ + // --- @agent-device/ad-replay --- + 'packages/ad-replay/src/index.ts': 58, + + // --- @agent-device/ad-script --- + 'packages/ad-script/src/index.ts': 37, + + // --- @agent-device/capture-kit --- + 'packages/capture-kit/src/index.ts': 26, + + // --- @agent-device/contracts --- + 'packages/contracts/src/alert-contract.ts': 1, + 'packages/contracts/src/android-input-ownership.ts': 1, + 'packages/contracts/src/android-snapshot-quality.ts': 1, + 'packages/contracts/src/android-system-chrome.ts': 1, + 'packages/contracts/src/app-deployment-runtime-plan.ts': 3, + 'packages/contracts/src/app-deployment-runtime.ts': 1, + 'packages/contracts/src/app-inventory-runtime.ts': 1, + 'packages/contracts/src/app-log-runtime.ts': 1, + 'packages/contracts/src/app-state-runtime.ts': 1, + 'packages/contracts/src/apple-multitouch-support.ts': 5, + 'packages/contracts/src/application-lifecycle-interaction.ts': 7, + 'packages/contracts/src/application-lifecycle-runtime-plan.ts': 3, + 'packages/contracts/src/application-lifecycle-runtime.ts': 1, + 'packages/contracts/src/async-lifecycle.ts': 1, + 'packages/contracts/src/audio-probe-result.ts': 1, + 'packages/contracts/src/audio-probe-support.ts': 5, + 'packages/contracts/src/back-mode.ts': 1, + 'packages/contracts/src/click-button.ts': 3, + 'packages/contracts/src/command-platform-execution.ts': 2, + 'packages/contracts/src/device-readiness-runtime.ts': 1, + 'packages/contracts/src/device-shutdown-runtime.ts': 1, + 'packages/contracts/src/durable-resource-envelope.ts': 1, + 'packages/contracts/src/durable-resource.ts': 1, + 'packages/contracts/src/element-text-runtime.ts': 4, + 'packages/contracts/src/facades/capture.ts': 9, + 'packages/contracts/src/facades/client.ts': 2, + 'packages/contracts/src/facades/command.ts': 9, + 'packages/contracts/src/facades/device.ts': 8, + 'packages/contracts/src/facades/divergence.ts': 3, + 'packages/contracts/src/facades/interaction.ts': 25, + 'packages/contracts/src/facades/observability.ts': 7, + 'packages/contracts/src/facades/platform.ts': 42, + 'packages/contracts/src/facades/progress.ts': 1, + 'packages/contracts/src/facades/recording.ts': 3, + 'packages/contracts/src/facades/remote.ts': 2, + 'packages/contracts/src/facades/replay.ts': 3, + 'packages/contracts/src/facades/session.ts': 5, + 'packages/contracts/src/facades/snapshot.ts': 8, + 'packages/contracts/src/focus-runtime.ts': 4, + 'packages/contracts/src/gesture-input.ts': 13, + 'packages/contracts/src/gesture-normalization.ts': 14, + 'packages/contracts/src/gesture-plan-types.ts': 1, + 'packages/contracts/src/gesture-plan.ts': 12, + 'packages/contracts/src/interaction-error.ts': 1, + 'packages/contracts/src/interaction-guarantees.ts': 1, + 'packages/contracts/src/interactor-types.ts': 1, + 'packages/contracts/src/logs-runtime-plan.ts': 5, + 'packages/contracts/src/navigation.ts': 1, + 'packages/contracts/src/network-runtime-plan.ts': 5, + 'packages/contracts/src/network-runtime.ts': 1, + 'packages/contracts/src/platform-module.ts': 5, + 'packages/contracts/src/platform-runtime-host.ts': 1, + 'packages/contracts/src/platform-runtime-operations.ts': 2, + 'packages/contracts/src/platform-runtime-unavailable.ts': 15, + 'packages/contracts/src/platform-runtime.ts': 6, + 'packages/contracts/src/record-runtime-cutover.ts': 7, + 'packages/contracts/src/screen-recording-runtime-plan.ts': 5, + 'packages/contracts/src/screen-recording-runtime.ts': 1, + 'packages/contracts/src/screenshot-runtime.ts': 4, + 'packages/contracts/src/scroll-command.ts': 3, + 'packages/contracts/src/scroll-gesture.ts': 10, + 'packages/contracts/src/selector-observation-runtime.ts': 1, + 'packages/contracts/src/settings.ts': 3, + 'packages/contracts/src/snapshot-runtime.ts': 3, + 'packages/contracts/src/startup-recovery-fence.ts': 1, + 'packages/contracts/src/tv-remote.ts': 3, + 'packages/contracts/src/type-text-runtime.ts': 4, + 'packages/contracts/src/viewport-runtime.ts': 1, + 'packages/contracts/src/wait-runtime-plan.ts': 1, + 'packages/contracts/src/wait.ts': 1, + + // --- @agent-device/kernel --- + 'packages/kernel/src/bounds.ts': 1, + 'packages/kernel/src/collections.ts': 1, + 'packages/kernel/src/contracts.ts': 4, + 'packages/kernel/src/device.ts': 4, + 'packages/kernel/src/errors.ts': 2, + 'packages/kernel/src/rect.ts': 1, + 'packages/kernel/src/redaction.ts': 1, + 'packages/kernel/src/snapshot.ts': 1, + + // --- @agent-device/maestro --- + 'packages/maestro/src/index.ts': 104, + + // --- @agent-device/platform-*: ADR-0019's metadata-eager/implementation-lazy façades. Each + // evaluates only itself; every implementation sits behind a function-scoped `await import`. + // A pin of 1 is the tightest statement of that property the walker can make. + 'packages/platform-android/src/index.ts': 1, + + // --- @agent-device/platform-apple --- + 'packages/platform-apple/src/index.ts': 1, + + // --- @agent-device/platform-harmonyos --- + 'packages/platform-harmonyos/src/index.ts': 1, + + // --- @agent-device/platform-linux --- + 'packages/platform-linux/src/index.ts': 1, + + // --- @agent-device/platform-vega --- + 'packages/platform-vega/src/index.ts': 1, + + // --- @agent-device/platform-web --- + 'packages/platform-web/src/index.ts': 1, + + // --- @agent-device/provider-limrun --- + 'packages/provider-limrun/src/index.ts': 29, + + // --- @agent-device/provider-webdriver --- + 'packages/provider-webdriver/src/index.ts': 49, + + // --- @agent-device/replay-test --- + 'packages/replay-test/src/index.ts': 19, + + // --- @agent-device/selectors --- + 'packages/selectors/src/ast.ts': 16, + 'packages/selectors/src/engine.ts': 19, + 'packages/selectors/src/index.ts': 50, + + // --- @agent-device/xml --- + 'packages/xml/src/index.ts': 3, +}); + +/** + * Designated hub modules: high-fan-in entry points whose closure the whole suite (or every CLI + * run) pays for. * - * Deriving from manifests rather than scanning `packages//src/facades/` is not a detail. Six - * platform packages have no `facades/` directory at all -- each publishes `./src/index.ts` -- so a - * directory-only scan silently omits the exact files ADR-0019's implementation-laziness rule is - * about, and the gate would claim to prove the loading shape while never looking at it. + * `cli.ts` and `session-teardown.ts` already carry ad hoc pins naming individual expensive + * modules (`cli-startup-import-closure.test.ts`, `session-teardown-import-closure.test.ts`); the + * five after them are the hubs #1969 moved off the wide contracts façades, pinned there by name + * (`contracts-entry-closure.test.ts`). Those tests state a STRONGER property for the one module + * each names; these pins add the general layer -- any unexpected growth, not only the shape + * someone already thought to forbid. * - * `readWorkspacePackages` is reused rather than reimplemented so the two gates cannot drift into - * disagreeing about what a package entry surface is. + * `src/platform-runtime.ts` is the ADR-0019 composition root, the one production module allowed + * to value-import a concrete platform package. It evaluates all six family façades (metadata + * only), so its pin is also the assertion that composing the registry stays metadata-eager. */ -export function discoverFacadeEntryFiles(repoRoot: string): string[] { - const found = new Set(); - for (const pkg of readWorkspacePackages(repoRoot)) { - for (const target of pkg.exportTargets.values()) found.add(target); +export const HUB_BUDGETS: Readonly> = Object.freeze({ + 'src/cli.ts': 361, + 'src/platform-runtime.ts': 31, + 'src/core/dispatch.ts': 100, + 'src/core/capabilities.ts': 76, + 'src/core/command-descriptor/registry.ts': 66, + 'src/core/command-descriptor/platform-execution-entry.ts': 3, + 'src/core/interactors/register-builtins.ts': 73, + 'src/daemon/session-teardown.ts': 89, +}); + +function toRows( + budgets: Readonly>, + kind: 'facade' | 'hub', +): EagerClosureBudget[] { + return Object.entries(budgets).map(([entryFile, budget]) => ({ + id: entryFile, + entryFile, + budget, + kind, + denyPlatformImplementations: kind === 'facade', + })); +} + +/** + * The two records as one list. Uniqueness WITHIN each record is a compile error; the only + * duplicate still expressible is the same path appearing in both, which + * `eager-closure-budgets.test.ts` asserts against on this array, before any `Set` conversion + * could absorb it. + */ +export const EAGER_CLOSURE_BUDGETS: EagerClosureBudget[] = [ + ...toRows(FACADE_BUDGETS, 'facade'), + ...toRows(HUB_BUDGETS, 'hub'), +]; + +/** + * The ratchet verdict for one row: `null` when the pin is exact, otherwise the finding to report. + * + * Pure and separately tested, so both directions have a test that fails when the rule is wrong -- + * an `<=` comparison passes every under-budget case, and no assertion over the real tree can + * distinguish that from a correct rule while the tree happens to match its pins. + */ +export function classifyBudget(id: string, actual: number, budget: number): string | null { + if (actual === budget) return null; + if (actual > budget) { + return ( + `${id} evaluates ${actual} modules on import, pinned at ${budget}. 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.' + ); } - const packagesDir = path.join(repoRoot, 'packages'); - const srcRoots = ['src']; - for (const entry of fs.readdirSync(packagesDir).sort()) { - if (fs.existsSync(path.join(packagesDir, entry, 'src'))) srcRoots.push(`packages/${entry}/src`); + return ( + `${id} evaluates ${actual} modules on import, pinned at ${budget}. It shrank -- lower its ` + + `pin to ${actual} in this PR so the ratchet keeps the gain instead of leaving headroom a ` + + 'later regression could grow back into.' + ); +} + +/** How many direct edges, and how many chains within one, an over-pin failure prints. */ +const REPORTED_EDGES = 4; +const REPORTED_CHAINS_PER_EDGE = 2; + +/** Modules reached from `file` in the breadth-first tree, in discovery order. */ +function subtreeOf(childrenOf: ReadonlyMap, file: string): string[] { + const found: string[] = []; + const queue = [file]; + for (let head = 0; head < queue.length; head += 1) { + const current = queue[head]; + if (current === undefined) continue; + found.push(current); + for (const child of childrenOf.get(current) ?? []) queue.push(child); } - for (const srcRoot of srcRoots) { - const facadesDir = path.join(repoRoot, srcRoot, 'facades'); - if (!fs.existsSync(facadesDir)) continue; - for (const file of fs.readdirSync(facadesDir).sort()) { - if (file.endsWith('.ts')) found.add(`${srcRoot}/facades/${file}`); - } + return found; +} + +/** + * A bounded account of WHERE an entry's evaluated modules come from: its heaviest direct imports, + * each with a couple of representative routes into the subtree it pulls in. + * + * What it shows: the direct edges of the entry ranked by how many modules enter the closure + * THROUGH THEM -- attribution by shortest import route, since the walk is breadth-first -- capped + * at `REPORTED_EDGES` edges and `REPORTED_CHAINS_PER_EDGE` chains each. When a regression is a new + * import on the entry itself, which is the common case, that edge is new and its whole subtree is + * attributed to it, so it sorts to the top and the offending route is the first thing printed. + * + * What it does NOT show: a diff against a recorded baseline. This gate persists each entry's + * module COUNT, not its module identity, so it cannot say "these three modules are new" -- only + * "these edges account for the weight". A regression added deep inside an already-large subtree + * is therefore attributed to the top-level edge containing it, not to the exact file that changed. + * Naming the true delta would mean checking in ~1,500 module paths and rewriting them on every + * contracts refactor; the count plus this attribution was judged the better trade. Reconstruct an + * exact delta when you need one by running the walker on the merge base. + */ +export function describeClosurePressure( + graph: ReadonlyMap, + entryPath: string, + repoRoot: string, +): string { + const childrenOf = new Map(); + for (const [file, parent] of graph) { + if (parent === null) continue; + const siblings = childrenOf.get(parent); + if (siblings) siblings.push(file); + else childrenOf.set(parent, [file]); + } + + const ranked = (childrenOf.get(entryPath) ?? []) + .map((edge) => ({ edge, subtree: subtreeOf(childrenOf, edge) })) + .sort((left, right) => right.subtree.length - left.subtree.length); + if (ranked.length === 0) return ' (no eager edges: this entry evaluates only itself)'; + + const sections = ranked.slice(0, REPORTED_EDGES).map(({ edge, subtree }) => { + // Deepest-first: a leaf names the far end of the route, which is more informative than + // re-printing the edge itself. + const deepest = [...subtree] + .sort((left, right) => chainLength(graph, right) - chainLength(graph, left)) + .slice(0, REPORTED_CHAINS_PER_EDGE); + const routes = deepest.map((file) => ` ${formatImportChain(graph, file, repoRoot)}`); + return ( + ` ${path.relative(repoRoot, edge)} -- ${subtree.length} module(s) enter through this ` + + `edge:\n${routes.join('\n')}` + ); + }); + const omitted = ranked.length - Math.min(ranked.length, REPORTED_EDGES); + const tail = omitted > 0 ? `\n (+${omitted} more direct edge(s), smaller)` : ''; + return `${sections.join('\n')}${tail}`; +} + +function chainLength(graph: ReadonlyMap, target: string): number { + let length = 0; + for (let at: string | null | undefined = target; at != null; at = graph.get(at)) { + length += 1; + if (length > 64) break; } - return [...found].filter((file) => fs.existsSync(path.join(repoRoot, file))).sort(); + return length; } /** @@ -117,8 +392,8 @@ export function discoverFacadeEntryFiles(repoRoot: string): string[] { * * #1960 asks a violation to "name the offending edge chain". A sorted set of evaluated files names * the destination but not the route, which leaves the reader to rediscover by hand which import - * actually pulled it in -- the "budget exceeded, go spelunking" failure the issue rules out. - * `eagerClosureGraphOf` records each file's discoverer, so the route is just a walk back up. + * actually pulled it in. `eagerClosureGraphOf` records each file's discoverer, so the route is + * just a walk back up, and because that walk is breadth-first the route is the shortest one. */ export function formatImportChain( graph: ReadonlyMap, @@ -130,151 +405,5 @@ export function formatImportChain( chain.push(path.relative(repoRoot, at)); if (chain.length > 64) break; // defensive: a cycle would otherwise spin here } - return chain.reverse().join('\n -> '); -} - -function facade(entryFile: string, budget: number): EagerClosureBudget { - return { id: entryFile, entryFile, budget, kind: 'facade', denyPlatformImplementations: true }; + return chain.reverse().join('\n -> '); } - -function hub(entryFile: string, budget: number): EagerClosureBudget { - return { id: entryFile, entryFile, budget, kind: 'hub', denyPlatformImplementations: false }; -} - -/** - * Measured 2026-08-22 on `03c398406` (post-#1969, which granularized the contracts entry surface - * from 15 subpaths to 70 and moved the hubs off the wide façades). Budgets are exact; see the - * `budget` field doc for why there is no headroom. - */ -export const EAGER_CLOSURE_BUDGETS: EagerClosureBudget[] = [ - // --- @agent-device/ad-replay / ad-script / capture-kit --- - facade('packages/ad-replay/src/index.ts', 58), - facade('packages/ad-script/src/index.ts', 37), - facade('packages/capture-kit/src/index.ts', 26), - - // --- @agent-device/contracts: the shared vocabulary package. #1969 gave every module its own - // entry subpath precisely so a consumer needing one symbol stops evaluating a 32-module union; - // these budgets are what keeps each narrow entry narrow. - facade('packages/contracts/src/alert-contract.ts', 1), - facade('packages/contracts/src/android-input-ownership.ts', 1), - facade('packages/contracts/src/android-snapshot-quality.ts', 1), - facade('packages/contracts/src/android-system-chrome.ts', 1), - facade('packages/contracts/src/app-deployment-runtime-plan.ts', 3), - facade('packages/contracts/src/app-deployment-runtime.ts', 1), - facade('packages/contracts/src/app-inventory-runtime.ts', 1), - facade('packages/contracts/src/app-log-runtime.ts', 1), - facade('packages/contracts/src/app-state-runtime.ts', 1), - facade('packages/contracts/src/apple-multitouch-support.ts', 5), - facade('packages/contracts/src/application-lifecycle-interaction.ts', 7), - facade('packages/contracts/src/application-lifecycle-runtime-plan.ts', 3), - facade('packages/contracts/src/application-lifecycle-runtime.ts', 1), - facade('packages/contracts/src/async-lifecycle.ts', 1), - facade('packages/contracts/src/audio-probe-result.ts', 1), - facade('packages/contracts/src/audio-probe-support.ts', 5), - facade('packages/contracts/src/back-mode.ts', 1), - facade('packages/contracts/src/click-button.ts', 3), - facade('packages/contracts/src/command-platform-execution.ts', 2), - facade('packages/contracts/src/device-readiness-runtime.ts', 1), - facade('packages/contracts/src/device-shutdown-runtime.ts', 1), - facade('packages/contracts/src/durable-resource-envelope.ts', 1), - facade('packages/contracts/src/durable-resource.ts', 1), - facade('packages/contracts/src/element-text-runtime.ts', 4), - facade('packages/contracts/src/facades/capture.ts', 9), - facade('packages/contracts/src/facades/client.ts', 2), - facade('packages/contracts/src/facades/command.ts', 9), - facade('packages/contracts/src/facades/device.ts', 8), - facade('packages/contracts/src/facades/divergence.ts', 3), - facade('packages/contracts/src/facades/interaction.ts', 25), - facade('packages/contracts/src/facades/observability.ts', 7), - facade('packages/contracts/src/facades/platform.ts', 42), - facade('packages/contracts/src/facades/progress.ts', 1), - facade('packages/contracts/src/facades/recording.ts', 3), - facade('packages/contracts/src/facades/remote.ts', 2), - facade('packages/contracts/src/facades/replay.ts', 3), - facade('packages/contracts/src/facades/session.ts', 5), - facade('packages/contracts/src/facades/snapshot.ts', 8), - facade('packages/contracts/src/focus-runtime.ts', 4), - facade('packages/contracts/src/gesture-input.ts', 13), - facade('packages/contracts/src/gesture-normalization.ts', 14), - facade('packages/contracts/src/gesture-plan-types.ts', 1), - facade('packages/contracts/src/gesture-plan.ts', 12), - facade('packages/contracts/src/interaction-error.ts', 1), - facade('packages/contracts/src/interaction-guarantees.ts', 1), - facade('packages/contracts/src/interactor-types.ts', 1), - facade('packages/contracts/src/logs-runtime-plan.ts', 5), - facade('packages/contracts/src/navigation.ts', 1), - facade('packages/contracts/src/network-runtime-plan.ts', 5), - facade('packages/contracts/src/network-runtime.ts', 1), - facade('packages/contracts/src/platform-module.ts', 5), - facade('packages/contracts/src/platform-runtime-host.ts', 1), - facade('packages/contracts/src/platform-runtime-operations.ts', 2), - facade('packages/contracts/src/platform-runtime-unavailable.ts', 15), - facade('packages/contracts/src/platform-runtime.ts', 6), - facade('packages/contracts/src/record-runtime-cutover.ts', 7), - facade('packages/contracts/src/screen-recording-runtime-plan.ts', 5), - facade('packages/contracts/src/screen-recording-runtime.ts', 1), - facade('packages/contracts/src/screenshot-runtime.ts', 4), - facade('packages/contracts/src/scroll-command.ts', 3), - facade('packages/contracts/src/scroll-gesture.ts', 10), - facade('packages/contracts/src/selector-observation-runtime.ts', 1), - facade('packages/contracts/src/settings.ts', 3), - facade('packages/contracts/src/snapshot-runtime.ts', 3), - facade('packages/contracts/src/startup-recovery-fence.ts', 1), - facade('packages/contracts/src/tv-remote.ts', 3), - facade('packages/contracts/src/type-text-runtime.ts', 4), - facade('packages/contracts/src/viewport-runtime.ts', 1), - facade('packages/contracts/src/wait-runtime-plan.ts', 1), - facade('packages/contracts/src/wait.ts', 1), - - // --- @agent-device/kernel --- - facade('packages/kernel/src/bounds.ts', 1), - facade('packages/kernel/src/collections.ts', 1), - facade('packages/kernel/src/contracts.ts', 4), - facade('packages/kernel/src/device.ts', 4), - facade('packages/kernel/src/errors.ts', 2), - facade('packages/kernel/src/rect.ts', 1), - facade('packages/kernel/src/redaction.ts', 1), - facade('packages/kernel/src/snapshot.ts', 1), - - // --- @agent-device/maestro --- - facade('packages/maestro/src/index.ts', 104), - - // --- @agent-device/platform-*: ADR-0019's metadata-eager/implementation-lazy façades. Each - // evaluates only itself; every implementation sits behind a function-scoped `await import`. - // A budget of 1 is the tightest statement of that property the walker can make. - facade('packages/platform-android/src/index.ts', 1), - facade('packages/platform-apple/src/index.ts', 1), - facade('packages/platform-harmonyos/src/index.ts', 1), - facade('packages/platform-linux/src/index.ts', 1), - facade('packages/platform-vega/src/index.ts', 1), - facade('packages/platform-web/src/index.ts', 1), - - // --- providers, replay-test, selectors, xml --- - facade('packages/provider-limrun/src/index.ts', 29), - facade('packages/provider-webdriver/src/index.ts', 49), - facade('packages/replay-test/src/index.ts', 19), - facade('packages/selectors/src/ast.ts', 16), - facade('packages/selectors/src/engine.ts', 19), - facade('packages/selectors/src/index.ts', 50), - facade('packages/xml/src/index.ts', 3), - - // --- Designated hub modules --- - // High-fan-in entry points whose closure the whole suite (or every CLI run) pays for. The first - // two already carry their own ad hoc pins naming individual expensive modules - // (`cli-startup-import-closure.test.ts`, `session-teardown-import-closure.test.ts`); the five - // after them are the hubs #1969 moved off the wide contracts façades, pinned there by name - // (`contracts-entry-closure.test.ts`). Those tests state a STRONGER property for the one module - // each names; these budgets add the general layer -- any unexpected growth, not only the shape - // someone already thought to forbid. - hub('src/cli.ts', 361), - // The ADR-0019 composition root: the one production module allowed to value-import a concrete - // platform package. It evaluates all six family façades (6 modules, all metadata-only), so its - // budget is also the assertion that composing the registry stays metadata-eager overall. - hub('src/platform-runtime.ts', 31), - hub('src/core/dispatch.ts', 100), - hub('src/core/capabilities.ts', 76), - hub('src/core/command-descriptor/registry.ts', 66), - hub('src/core/command-descriptor/platform-execution-entry.ts', 3), - hub('src/core/interactors/register-builtins.ts', 73), - hub('src/daemon/session-teardown.ts', 89), -];