From 3a4616beec5989db850b1738d5d6db90fd5d5dd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 15 Oct 2026 13:13:59 +0200 Subject: [PATCH 1/3] refactor(layering): add R83 so no package export publishes test support Every exports target of every workspace package except test-kit must be a production module: not a fixture, fake, mock, harness, __tests__ or testing/ entry, judged by key and by target. Today's 122 offenders are a shrink-only burn-down list; a new offender and a stale entry both fail. The consumer-side ban (production source importing a test-only export or test-kit) moves into the R11 import walk and replaces TEST_ONLY_FIXTURE_SUBPATHS, the ./testing/ namespace check and R13's ANDROID_TEST_ONLY_SUBPATHS. Co-Authored-By: Claude Opus 5.5 --- scripts/layering/check.ts | 11 +- .../package-boundaries-facade.test.ts | 39 +-- scripts/layering/package-boundaries.ts | 110 ++---- .../package-export-surface-burn-down.ts | 147 ++++++++ .../layering/package-export-surface.test.ts | 314 ++++++++++++++++++ scripts/layering/package-export-surface.ts | 224 +++++++++++++ .../layering/platform-package-policy.test.ts | 16 +- scripts/layering/platform-package-policy.ts | 36 +- .../test-only-fixture-subpaths.test.ts | 107 ------ 9 files changed, 755 insertions(+), 249 deletions(-) create mode 100644 scripts/layering/package-export-surface-burn-down.ts create mode 100644 scripts/layering/package-export-surface.test.ts create mode 100644 scripts/layering/package-export-surface.ts delete mode 100644 scripts/layering/test-only-fixture-subpaths.test.ts diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index c100c29bff..271e6bcc84 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -60,6 +60,9 @@ // static edge reads up the daemon-local order core < resources < execution < sessions < // server (R81). The global spine // ranks all five layers at daemon-server's rank, so R5/R6/R80 are blind inside the zone. +// - Over WORKSPACE PACKAGE EXPORTS: no package but test-kit publishes test support (fixtures, +// fakes, mocks, harnesses, `__tests__/`, `./testing/*`); a shrink-only burn-down list holds +// today's offenders, and production code may not import them or test-kit (R83). // - Over CONTRACTS PRODUCTION SOURCE: contracts owns vocabulary only — host, process, and timer // mechanics belong in capture-kit or an adapter (R18). // R6, R9, and the R10 R7 counts are ratchets with no written-down reference: each is the same @@ -109,6 +112,7 @@ import { } from './daemon-modularity.ts'; import { checkPackageBoundaries, + readTrackedPackageManifests, packageBoundariesSummary, workspaceSpecifierTargets, } from './package-boundaries.ts'; @@ -134,6 +138,8 @@ import { checkDaemonPlatformBoundary } from './daemon-platform-boundary.ts'; import { checkDaemonPlatformRuntimeInventory } from './daemon-platform-runtime-inventory.ts'; import { checkDaemonClientEntry } from './daemon-client-entry.ts'; import { checkDaemonProcessReach } from './daemon-process-reach.ts'; +import { checkPackageExportSurface } from './package-export-surface.ts'; +import { TEST_ONLY_EXPORT_BURN_DOWN } from './package-export-surface-burn-down.ts'; import { checkDaemonLayers, DAEMON_ROOT } from './daemon-layers.ts'; import { checkSessionAuthorityOverlay, handlerOwnedOverlay } from './session-authority-overlay.ts'; import { @@ -435,7 +441,8 @@ function report( `concrete platform imports in every executable and type-only form; ` + `no production daemon file reaches the root platform-runtime family or a hub that ` + `does (R76); the daemon process reaches no caller-side replay source module over ` + - `static or dynamic edges (R82); and the handler-owned SessionState/SessionStore ` + + `static or dynamic edges (R82); package exports publish no new test support, with ` + + `${TEST_ONLY_EXPORT_BURN_DOWN.length} burn-down entries left (R83); and the handler-owned SessionState/SessionStore ` + `authority overlay holds at or under the merge-base (R75, ` + `${handlerOwnedShapeFiles} shape / ${handlerOwnedAuthorityFiles} authority files); and ` + `every production daemon file lives under a layer folder with no upward static edges ` + @@ -522,6 +529,7 @@ export const LAYERING_RULE_IDS = [ 'daemon-client-entry', 'session-authority-overlay', 'daemon-layers', + 'package-export-surface', ] as const; export type LayeringRuleId = (typeof LAYERING_RULE_IDS)[number]; @@ -595,6 +603,7 @@ export const LAYERING_RULES: Readonly> = { context.sourceFiles.filter((file) => file.startsWith(DAEMON_SOURCE_ROOT)), context.edges, ), + 'package-export-surface': () => checkPackageExportSurface(readTrackedPackageManifests(repoRoot)), }; export function main(): number { diff --git a/scripts/layering/package-boundaries-facade.test.ts b/scripts/layering/package-boundaries-facade.test.ts index b14c1c3232..212172acbe 100644 --- a/scripts/layering/package-boundaries-facade.test.ts +++ b/scripts/layering/package-boundaries-facade.test.ts @@ -9,7 +9,6 @@ import { test } from 'node:test'; import { checkPackageInternalSites, checkRootSites, - isTestConsumerFile, specifierSites, workspacePackagesFromManifests, } from './package-boundaries.ts'; @@ -46,7 +45,7 @@ test('a production root file importing the ./testing/ namespace is refused', () true, ); assert.equal(violations.length, 1); - assert.match(violations[0]!.message, /publishes test data to test files only/); + assert.match(violations[0]!.message, /test-only export/); }); test('a test root file importing ./testing/ passes, and a named entry stays open to all', () => { @@ -105,7 +104,7 @@ test('the ./testing/ refusal bites inside packages and through the vi.mock scan' }); test('a package-local test/ tree is a test consumer, its src/ tree is not', () => { - // The file vocabulary TEST_ONLY_FIXTURE_SUBPATHS' predicate uses: a package's + // The file vocabulary R83's consumer check uses: a package's // own harness tree (`packages//test/`) imports fixtures just like the // root `test/` tree does; the same file under src/ does not (#3546 review). const consumer = workspacePackagesFromManifests( @@ -133,42 +132,12 @@ test('a package-local test/ tree is a test consumer, its src/ tree is not', () = ); const denied = checkPackageInternalSites( consumer, - sites('packages/daemon-side/src/harness.ts', fixture), + sites('packages/daemon-side/src/inventory.ts', fixture), [daemon, consumer], true, ); assert.equal(denied.length, 1); - assert.match(denied[0]!.message, /publishes test data to test files only/); -}); - -test('isTestConsumerFile accepts the test vocabulary and denies production files', () => { - // One spelling serves both test-data gates (the ./testing/ façade check and the - // TEST_ONLY_FIXTURE_SUBPATHS predicate), so the vocabulary itself is pinned here: - // every accepted shape forwards to both gates, and the shapes that once spelled - // differently across the two copies (.test.tsx, .mts, a root __tests__/ tree) are - // in the accepted set (#3546 review). - for (const file of [ - 'test/integration/consumer.ts', - 'packages/daemon-side/test/harness.ts', - 'src/daemon/__tests__/consumer.ts', - '__tests__/root-consumer.ts', - 'scripts/layering/daemon-layers.test.tsx', - 'src/daemon/session-store.test.mts', - 'src/foo.test.js', - 'src/foo.test.cjs', - ]) { - assert.equal(isTestConsumerFile(file), true, file); - } - for (const file of [ - 'src/commands/inventory.ts', - 'packages/kernel/src/errors.ts', - 'scripts/layering/package-boundaries.ts', - 'testbed/notes.ts', - 'docs/writing/tests.ts', - 'src/foo.ts', - ]) { - assert.equal(isTestConsumerFile(file), false, file); - } + assert.match(denied[0]!.message, /test-only export/); }); test('a pattern entry matches deep specifiers, and an unnamed one is refused in scripts/', () => { diff --git a/scripts/layering/package-boundaries.ts b/scripts/layering/package-boundaries.ts index 620a7d9296..bc9c394e8e 100644 --- a/scripts/layering/package-boundaries.ts +++ b/scripts/layering/package-boundaries.ts @@ -26,12 +26,13 @@ // and via its package specifier instantiates twice in one process — duplicate AppError, broken // instanceof), and, for all `scripts/` files, the root-side unknown-package and exports-map // checks. Explicit tooling roots also receive those resolution checks from TS2307. -// On top, `TEST_ONLY_FIXTURE_SUBPATHS` mirrors R13's Android fixture gate for the non-platform -// packages: a production file importing one of those fixture subpaths is an R11 violation. +// The consumer half of R83 (production code importing a test-only export or test-kit) is checked at +// the same import sites, so the one specifier walk serves both rules. import fs from 'node:fs'; import path from 'node:path'; import { parseImports, resolveWorkspaceSpecifier } from './model.ts'; +import { PACKAGE_EXPORT_SURFACE_RULE, testOnlyImportMessage } from './package-export-surface.ts'; import { listTrackedPackageManifests, listTrackedProductionSources } from './tracked-sources.ts'; export type PackageBoundaryViolation = { @@ -103,13 +104,16 @@ export function specifierSites(file: string, source: string): SpecifierSite[] { * filtering the output. */ export function readWorkspacePackages(repoRoot: string): WorkspacePackage[] { - return workspacePackagesFromManifests( - new Map( - listTrackedPackageManifests(repoRoot).map((manifestFile) => [ - manifestFile, - fs.readFileSync(path.join(repoRoot, manifestFile), 'utf8'), - ]), - ), + return workspacePackagesFromManifests(readTrackedPackageManifests(repoRoot)); +} + +/** Tracked workspace package manifests, text keyed by repo-relative path. */ +export function readTrackedPackageManifests(repoRoot: string): Map { + return new Map( + listTrackedPackageManifests(repoRoot).map((manifestFile) => [ + manifestFile, + fs.readFileSync(path.join(repoRoot, manifestFile), 'utf8'), + ]), ); } @@ -174,45 +178,26 @@ function namedByExports(target: WorkspacePackage, specifier: string): boolean { return resolveWorkspaceSpecifier(specifier, target.exportTargets) !== undefined; } -const TEST_CONSUMER_RE = /(^|\/)__tests__\/|\.test\.[cm]?[jt]sx?$|(?:^|\/)test\//; - -/** - * The one file vocabulary for "this file consumes test data": the root `test/` tree, a - * package-local `test/` harness tree, `*.test.*` files, and `__tests__/` trees (root or - * nested). Both test-data gates call this — the `./testing/` façade check and the - * TEST_ONLY_FIXTURE_SUBPATHS predicate — so what counts as a test consumer is one rule, - * not two spellings that drift. Pinned by an accept/deny table in - * `package-boundaries-facade.test.ts`. - */ -export function isTestConsumerFile(file: string): boolean { - return TEST_CONSUMER_RE.test(file); -} - /** - * Façade policy (ADR-0033 amendment): a package publishes test data only under its - * `./testing/` namespace — named `./testing/` entries and per-folder - * `./testing//*` patterns — and only test files may import it. The gate is the - * namespace, not a pattern entry: narrowing the surface to exact keys must not - * smuggle a fixture past the test-file rule. Production code that reaches a fixture - * through the package boundary is production code depending on a test double — it - * must inject the real port instead. + * R83's consumer half (see package-export-surface.ts): a production file importing a test-only + * export, or the private test-kit package, is production code depending on a test double — it + * must use a production subpath or inject the real port. */ -function checkTestDataOnlySites( +function checkTestOnlyImportSite( site: SpecifierSite, target: WorkspacePackage, ): PackageBoundaryViolation | undefined { - if (!site.specifier.startsWith(`${target.name}/testing/`)) return undefined; - if (!namedByExports(target, site.specifier)) return undefined; - if (isTestConsumerFile(site.file)) return undefined; - return { - rule: 'R11 package-boundaries', - file: site.file, - line: site.line, - message: - `'${site.specifier}' resolves through ${target.dir}/package.json#exports './testing/' ` + - `namespace, which publishes test data to test files only. '${site.file}' is not a test ` + - `file: import a production subpath or inject the port.`, - }; + const resolvedTarget = resolveWorkspaceSpecifier(site.specifier, target.exportTargets); + const message = testOnlyImportMessage( + site.file, + site.specifier, + target.name, + resolvedTarget === undefined + ? undefined + : { key: `.${site.specifier.slice(target.name.length)}`, targets: [resolvedTarget] }, + ); + if (message === undefined) return undefined; + return { rule: PACKAGE_EXPORT_SURFACE_RULE, file: site.file, line: site.line, message }; } function specifierPackageName(specifier: string): string | undefined { @@ -242,21 +227,6 @@ export function insideCompiledSources(file: string): boolean { return /^packages\/[^/]+\/src\//.test(file) || /^src\//.test(file) || /^test\//.test(file); } -/** - * Workspace fixture subpaths only non-production files may import — the same claim R13's - * `ANDROID_TEST_ONLY_SUBPATHS` makes for the platform packages, which R13 cannot express for - * non-platform packages. A production file (root or package) importing one is a violation. - */ -export const TEST_ONLY_FIXTURE_SUBPATHS: ReadonlySet = new Set([ - '@agent-device/device-selection/inventory-gateways-fixtures', - '@agent-device/replay-port/replay-script-source-fixtures', -]); - -/** Consumer side of `TEST_ONLY_FIXTURE_SUBPATHS`. */ -function importsTestOnlyFixtureFromProduction(file: string, specifier: string): boolean { - return TEST_ONLY_FIXTURE_SUBPATHS.has(specifier) && !isTestConsumerFile(file); -} - /** * Rules for files INSIDE a package. An undeclared sibling import is R11's alone: pnpm links * every workspace package under the root `node_modules`, so the import resolves, compiles, and @@ -305,16 +275,6 @@ export function checkPackageInternalSites( } const name = specifierPackageName(site.specifier); if (!name || !name.startsWith('@agent-device/')) continue; - if (importsTestOnlyFixtureFromProduction(site.file, site.specifier)) { - violations.push({ - rule: 'R11 package-boundaries', - file: site.file, - line: site.line, - message: - `'${site.specifier}' is a test-only fixture subpath — production code may not ` + - `import it; use the package's production subpath instead.`, - }); - } const target = packageByName(allPackages, name); if (!target) { if (!compiled) { @@ -337,7 +297,7 @@ export function checkPackageInternalSites( `in ${pkg.dir}/package.json ${pkg.workspaceDependencyField}.`, }); } - const testDataOnly = checkTestDataOnlySites(site, target); + const testDataOnly = checkTestOnlyImportSite(site, target); if (testDataOnly) { violations.push(testDataOnly); continue; @@ -391,16 +351,6 @@ export function checkRootSites( } const name = specifierPackageName(site.specifier); if (!name || !name.startsWith('@agent-device/')) continue; - if (importsTestOnlyFixtureFromProduction(site.file, site.specifier)) { - violations.push({ - rule: 'R11 package-boundaries', - file: site.file, - line: site.line, - message: - `'${site.specifier}' is a test-only fixture subpath — production code may not ` + - `import it; use the package's production subpath instead.`, - }); - } const target = packageByName(packages, name); if (!target) { if (!compiled) { @@ -423,7 +373,7 @@ export function checkRootSites( `package.json devDependencies.`, }); } - const testDataOnly = checkTestDataOnlySites(site, target); + const testDataOnly = checkTestOnlyImportSite(site, target); if (testDataOnly) { violations.push(testDataOnly); continue; diff --git a/scripts/layering/package-export-surface-burn-down.ts b/scripts/layering/package-export-surface-burn-down.ts new file mode 100644 index 0000000000..8d71784ecc --- /dev/null +++ b/scripts/layering/package-export-surface-burn-down.ts @@ -0,0 +1,147 @@ +// Today's test-only package exports (R83 package-export-surface), one ` ` per +// line and grouped by package so parallel cleanups delete disjoint lines. The list only shrinks: +// remove an entry in the change that removes the export; a stale entry fails the gate. + +export const TEST_ONLY_EXPORT_BURN_DOWN: readonly string[] = [ + // @agent-device/capture-kit + '@agent-device/capture-kit ./audio-probe-descriptor-fixtures', + '@agent-device/capture-kit ./recording-artifact-fixtures', + '@agent-device/capture-kit ./recording-mp4-fixtures', + '@agent-device/capture-kit ./snapshot-chrome-fixtures', + '@agent-device/capture-kit ./snapshot-presentation-ios-transitions-fixtures', + '@agent-device/capture-kit ./snapshot-state-fixtures', + + // @agent-device/contracts + '@agent-device/contracts ./dispatch-disclosure-fixtures', + + // @agent-device/daemon + '@agent-device/daemon ./testing/__tests__/android-observation-fixture', + '@agent-device/daemon ./testing/__tests__/application-lifecycle-runtime-fixture', + '@agent-device/daemon ./testing/__tests__/application-lifecycle-runtime-harness', + '@agent-device/daemon ./testing/__tests__/capture-disclosure-target-activation-fact', + '@agent-device/daemon ./testing/__tests__/device-selection-stub', + '@agent-device/daemon ./testing/__tests__/human-control-fixtures', + '@agent-device/daemon ./testing/__tests__/interaction-get-runtime-fixture', + '@agent-device/daemon ./testing/__tests__/ios-comparison-key-fixture', + '@agent-device/daemon ./testing/__tests__/legacy-snapshot-capture-fixture', + '@agent-device/daemon ./testing/__tests__/platform-services-fixture', + '@agent-device/daemon ./testing/__tests__/replay-runtime/replay-command-fixture', + '@agent-device/daemon ./testing/__tests__/request-router-dispatch-mocks', + '@agent-device/daemon ./testing/__tests__/request-router-open-harness', + '@agent-device/daemon ./testing/__tests__/runtime-binding-conformance', + '@agent-device/daemon ./testing/__tests__/screenshot-runtime-fixture', + '@agent-device/daemon ./testing/__tests__/selector-capture-fixture', + '@agent-device/daemon ./testing/__tests__/session-teardown.fixtures', + '@agent-device/daemon ./testing/__tests__/snapshot-runtime-fixture', + '@agent-device/daemon ./testing/__tests__/support/command-surface-fake', + '@agent-device/daemon ./testing/__tests__/support/device-claim-store', + '@agent-device/daemon ./testing/__tests__/support/provider-device-admission-fixture', + '@agent-device/daemon ./testing/__tests__/support/screen-recording-live-handle', + '@agent-device/daemon ./testing/__tests__/support/session-factories', + '@agent-device/daemon ./testing/__tests__/support/store-factory', + '@agent-device/daemon ./testing/__tests__/support/unavailable-device-runtime-gateway', + '@agent-device/daemon ./testing/__tests__/test-device-runtime-gateway', + '@agent-device/daemon ./testing/android-system-dialog', + '@agent-device/daemon ./testing/app-events', + '@agent-device/daemon ./testing/application-lifecycle-recovery', + '@agent-device/daemon ./testing/artifact-tracking', + '@agent-device/daemon ./testing/capture-disclosure', + '@agent-device/daemon ./testing/context', + '@agent-device/daemon ./testing/daemon-command-registry', + '@agent-device/daemon ./testing/deferred-interaction-outcome', + '@agent-device/daemon ./testing/device/device-claim-conflict', + '@agent-device/daemon ./testing/device/device-ready', + '@agent-device/daemon ./testing/generic-leaf-execution', + '@agent-device/daemon ./testing/gesture-runtime', + '@agent-device/daemon ./testing/handlers/__tests__/*', + '@agent-device/daemon ./testing/handlers/lease', + '@agent-device/daemon ./testing/handlers/session-app-deployment', + '@agent-device/daemon ./testing/handlers/session-appstate', + '@agent-device/daemon ./testing/handlers/session-doctor', + '@agent-device/daemon ./testing/handlers/session-doctor-app', + '@agent-device/daemon ./testing/handlers/session-doctor-metro', + '@agent-device/daemon ./testing/handlers/session-replay-command', + '@agent-device/daemon ./testing/handlers/snapshot', + '@agent-device/daemon ./testing/interaction/index', + '@agent-device/daemon ./testing/interaction/internal/__tests__/*', + '@agent-device/daemon ./testing/interaction/internal/interaction-android-escape', + '@agent-device/daemon ./testing/interaction/internal/interaction-ios-tap-outcome', + '@agent-device/daemon ./testing/interaction/internal/interaction-touch-android-readiness', + '@agent-device/daemon ./testing/interaction/internal/interaction-touch-response', + '@agent-device/daemon ./testing/interaction/internal/types', + '@agent-device/daemon ./testing/interactor-resolution', + '@agent-device/daemon ./testing/ios-app-session-hint', + '@agent-device/daemon ./testing/lease-registry', + '@agent-device/daemon ./testing/provider-device-admission', + '@agent-device/daemon ./testing/provider-lease-expiry', + '@agent-device/daemon ./testing/ref-frame', + '@agent-device/daemon ./testing/request-binding', + '@agent-device/daemon ./testing/request-dispatch-disclosure', + '@agent-device/daemon ./testing/request-dispatch-ledger', + '@agent-device/daemon ./testing/request-execution-scope', + '@agent-device/daemon ./testing/request-finalization', + '@agent-device/daemon ./testing/request-generic-dispatch', + '@agent-device/daemon ./testing/request-handler-chain', + '@agent-device/daemon ./testing/request-platform-provider-context', + '@agent-device/daemon ./testing/request-router', + '@agent-device/daemon ./testing/request-runtime-binding', + '@agent-device/daemon ./testing/response-views', + '@agent-device/daemon ./testing/screenshot-runtime', + '@agent-device/daemon ./testing/scroll-until', + '@agent-device/daemon ./testing/selector-capture-binding', + '@agent-device/daemon ./testing/selector-match-errors', + '@agent-device/daemon ./testing/selector-runtime', + '@agent-device/daemon ./testing/selector-runtime-backend', + '@agent-device/daemon ./testing/server/daemon-runtime', + '@agent-device/daemon ./testing/server/http-server', + '@agent-device/daemon ./testing/server/server-lifecycle', + '@agent-device/daemon ./testing/server/server-shutdown', + '@agent-device/daemon ./testing/server/transport', + '@agent-device/daemon ./testing/session-action-recorder', + '@agent-device/daemon ./testing/session-capture-binding', + '@agent-device/daemon ./testing/session-lifecycle/index', + '@agent-device/daemon ./testing/session-lifecycle/internal/__tests__/*', + '@agent-device/daemon ./testing/session-observability/__tests__/*', + '@agent-device/daemon ./testing/session-recovery-hints', + '@agent-device/daemon ./testing/session-runtime', + '@agent-device/daemon ./testing/session-script-writer', + '@agent-device/daemon ./testing/session-snapshot', + '@agent-device/daemon ./testing/session-state', + '@agent-device/daemon ./testing/session-store', + '@agent-device/daemon ./testing/session-teardown-budget', + '@agent-device/daemon ./testing/snapshot-capture', + '@agent-device/daemon ./testing/snapshot-diff-runtime', + '@agent-device/daemon ./testing/snapshot-interactor-capture', + '@agent-device/daemon ./testing/snapshot-quality-latch', + '@agent-device/daemon ./testing/snapshot-runtime', + '@agent-device/daemon ./testing/snapshot-session', + '@agent-device/daemon ./testing/wait-runtime', + '@agent-device/daemon ./testing/web-session-names', + + // @agent-device/daemon-contracts + '@agent-device/daemon-contracts ./daemon-registration-fixtures', + '@agent-device/daemon-contracts ./device-claims-fixtures', + + // @agent-device/device-selection + '@agent-device/device-selection ./inventory-gateways-fixtures', + + // @agent-device/host-kit + '@agent-device/host-kit ./archive-fixtures', + '@agent-device/host-kit ./audio-probe-fixtures', + + // @agent-device/platform-android + '@agent-device/platform-android ./snapshot-capture-fixtures', + '@agent-device/platform-android ./ui-hierarchy-fixtures', + + // @agent-device/platform-apple + '@agent-device/platform-apple ./runner/requests-fixtures', + '@agent-device/platform-apple ./runner/test-host', + + // @agent-device/replay-port + '@agent-device/replay-port ./replay-script-source-fixtures', + '@agent-device/replay-port ./target-classification-fixtures', + + // @agent-device/selectors + '@agent-device/selectors ./interaction-targeting-fixtures', + '@agent-device/selectors ./snapshot-geometry-fixtures', +]; diff --git a/scripts/layering/package-export-surface.test.ts b/scripts/layering/package-export-surface.test.ts new file mode 100644 index 0000000000..8aa053032d --- /dev/null +++ b/scripts/layering/package-export-surface.test.ts @@ -0,0 +1,314 @@ +// R83 package-export-surface: the manifest scan (every shape of test support is caught, by key or +// by target), the shrink-only burn-down list, and the consumer-side ban at the R11 import sites. + +import assert from 'node:assert/strict'; +import path from 'node:path'; +import { test } from 'node:test'; +import { + checkPackageInternalSites, + checkRootSites, + readTrackedPackageManifests, + specifierSites, + workspacePackagesFromManifests, +} from './package-boundaries.ts'; +import { + checkPackageExportSurface, + isProductionSource, + isTestOnlyExport, + isTestOnlyExportSpecifier, + isTestSupportFile, + PACKAGE_EXPORT_SURFACE_RULE, +} from './package-export-surface.ts'; +import { TEST_ONLY_EXPORT_BURN_DOWN } from './package-export-surface-burn-down.ts'; + +const repoRoot = path.resolve(import.meta.dirname, '../..'); + +function manifest(name: string, exports: Record): Map { + const dir = name.replace('@agent-device/', ''); + return new Map([[`packages/${dir}/package.json`, JSON.stringify({ name, exports })]]); +} + +function entry(target: string) { + return { types: target, default: target }; +} + +// Every shape the rule must catch: [description, export key, export target]. +const TEST_SUPPORT_SHAPES: ReadonlyArray = [ + ['a *.fixtures.ts target under an innocent key', './thing', './src/thing.fixtures.ts'], + ['a *.fixture.ts target under an innocent key', './thing', './src/thing.fixture.ts'], + ['a *-fixture-* target', './thing', './src/thing-fixture-builders.ts'], + ['a *-fixtures key', './thing-fixtures', './src/thing.ts'], + ['a fixtures.ts target', './thing', './src/recording/fixtures.ts'], + ['a target under __tests__/', './thing', './src/__tests__/support/thing.ts'], + ['a pattern target under __tests__/', './things/*', './src/things/__tests__/*.ts'], + ['a ./testing/ key', './testing/thing', './src/thing.ts'], + ['a testing/ target directory', './thing', './src/testing/thing.ts'], + ['a test-support key', './test-support', './src/thing.ts'], + ['a test-support target', './thing', './src/test-support.ts'], + ['a test-host key', './runner/test-host', './src/runner/thing.ts'], + ['a test-host target', './thing', './src/runner/test-host.ts'], + ['a fake key', './fake-thing', './src/thing.ts'], + ['a fake target', './thing', './src/fakes/thing.ts'], + ['a mock key', './thing-mocks', './src/thing.ts'], + ['a mock target', './thing', './src/thing.mock.ts'], + ['a stub target', './thing', './src/thing-stub.ts'], + ['a harness key', './thing-harness', './src/thing.ts'], + ['a harness target', './thing', './src/thing-harness.ts'], + ['a *.test.ts target', './thing', './src/thing.test.ts'], + ['a test/ target directory', './thing', './test/thing.ts'], +]; + +// Production exports whose names merely contain "test" or "support". +const PRODUCTION_SHAPES: ReadonlyArray = [ + ['./test-command', './src/daemon-port/test-command.ts'], + ['./session-test-outcome', './src/daemon-port/session-test-outcome.ts'], + ['./session-test-source-discovery', './src/daemon-port/session-test-source-discovery.ts'], + ['./android-clipboard-support', './src/android-clipboard-support.ts'], + ['./connection-verification', './src/testmu-connection-verification.ts'], + ['./snapshot', './src/snapshot/index.ts'], + ['.', './src/index.ts'], +]; + +test('every shape of test support is caught, by key or by target', () => { + for (const [description, key, target] of TEST_SUPPORT_SHAPES) { + const violations = checkPackageExportSurface( + manifest('@agent-device/planted', { [key]: entry(target) }), + [], + ); + assert.equal(violations.length, 1, description); + assert.equal(violations[0]!.rule, PACKAGE_EXPORT_SURFACE_RULE, description); + assert.match(violations[0]!.message, /exports test support|exports '/, description); + } +}); + +test('production exports that merely mention test or support are not flagged', () => { + for (const [key, target] of PRODUCTION_SHAPES) { + assert.deepEqual( + checkPackageExportSurface(manifest('@agent-device/planted', { [key]: entry(target) }), []), + [], + key, + ); + } +}); + +test('every export condition is judged, not only the default', () => { + const violations = checkPackageExportSurface( + manifest('@agent-device/planted', { + './thing': { types: './src/thing.ts', default: './src/thing.fixtures.ts' }, + }), + [], + ); + assert.equal(violations.length, 1); +}); + +test('a string exports map and a missing one are both read', () => { + const dir = (value: unknown) => + new Map([ + ['packages/p/package.json', JSON.stringify({ name: '@agent-device/p', exports: value })], + ]); + assert.deepEqual(checkPackageExportSurface(dir('./src/index.ts'), []), []); + assert.deepEqual(checkPackageExportSurface(dir(undefined), []), []); +}); + +test('test-kit is the one package allowed to export test support', () => { + assert.deepEqual( + checkPackageExportSurface( + manifest('@agent-device/test-kit', { + './device-fixtures': entry('./src/device-fixtures.ts'), + }), + [], + ), + [], + ); +}); + +test('the burn-down list is shrink-only: a new offender fails and so does a stale entry', () => { + const planted = manifest('@agent-device/planted', { + './listed-fixtures': entry('./src/listed.fixtures.ts'), + './new-fixtures': entry('./src/new.fixtures.ts'), + }); + const violations = checkPackageExportSurface(planted, [ + '@agent-device/planted ./listed-fixtures', + '@agent-device/planted ./gone-fixtures', + ]); + assert.equal(violations.length, 2); + assert.match(violations[0]!.message, /exports '\.\/new-fixtures'/); + assert.equal(violations[0]!.file, 'packages/planted/package.json'); + assert.match(violations[1]!.message, /'@agent-device\/planted \.\/gone-fixtures' no longer/); + assert.equal(violations[1]!.file, 'scripts/layering/package-export-surface-burn-down.ts'); +}); + +test('a burn-down entry whose package is gone is stale', () => { + const violations = checkPackageExportSurface( + manifest('@agent-device/other', { '.': entry('./src/index.ts') }), + ['@agent-device/missing ./thing-fixtures'], + ); + assert.equal(violations.length, 1); + assert.match(violations[0]!.message, /delete it from TEST_ONLY_EXPORT_BURN_DOWN/); +}); + +test('the committed manifests offend exactly where the burn-down list says', () => { + assert.deepEqual( + checkPackageExportSurface(readTrackedPackageManifests(repoRoot), TEST_ONLY_EXPORT_BURN_DOWN), + [], + ); +}); + +test('the burn-down list is unique and names a package and an export key per entry', () => { + assert.equal(new Set(TEST_ONLY_EXPORT_BURN_DOWN).size, TEST_ONLY_EXPORT_BURN_DOWN.length); + for (const line of TEST_ONLY_EXPORT_BURN_DOWN) { + assert.match(line, /^@agent-device\/[a-z-]+ \.(?:\/\S+)?$/, line); + assert.ok(!line.startsWith('@agent-device/test-kit '), 'test-kit is exempt, never listed'); + } +}); + +test('the key predicate works on specifiers too, for rules that hold only the specifier', () => { + assert.equal(isTestOnlyExportSpecifier('@agent-device/platform-apple/runner/test-host'), true); + assert.equal( + isTestOnlyExportSpecifier('@agent-device/selectors/snapshot-geometry-fixtures'), + true, + ); + assert.equal(isTestOnlyExportSpecifier('@agent-device/platform-apple/runner/operations'), false); + assert.equal(isTestOnlyExportSpecifier('@agent-device/platform-apple'), false); + assert.equal(isTestOnlyExportSpecifier('node:fs'), false); + assert.equal(isTestOnlyExport('./thing', ['./src/thing.fixtures.ts']), true); +}); + +test('isTestSupportFile and isProductionSource split the tree', () => { + for (const file of [ + 'test/integration/consumer.ts', + 'packages/daemon-side/test/harness.ts', + 'src/daemon/__tests__/consumer.ts', + '__tests__/root-consumer.ts', + 'scripts/layering/daemon-layers.test.tsx', + 'src/daemon/session-store.test.mts', + 'src/foo.test.js', + 'src/foo.test.cjs', + 'src/platform-runtime-gateway.fixtures.ts', + 'packages/platform-android/src/recording/fixtures.ts', + 'packages/test-kit/src/loopback.ts', + ]) { + assert.equal(isTestSupportFile(file), true, file); + assert.equal(isProductionSource(file), false, file); + } + for (const file of [ + 'src/commands/inventory.ts', + 'packages/kernel/src/errors.ts', + 'packages/daemon/src/sessions/session-store.ts', + ]) { + assert.equal(isTestSupportFile(file), false, file); + assert.equal(isProductionSource(file), true, file); + } + // Tooling and vitest setup are not production sources, so they may import test support. + for (const file of ['scripts/vitest-apple-runner-host-setup.ts', 'scripts/fuzz/arbitraries.ts']) { + assert.equal(isProductionSource(file), false, file); + } +}); + +const packages = workspacePackagesFromManifests( + new Map([ + [ + 'packages/owner/package.json', + JSON.stringify({ + name: '@agent-device/owner', + private: true, + exports: { + '.': entry('./src/index.ts'), + './thing': entry('./src/thing.ts'), + './thing-fixtures': entry('./src/thing.fixtures.ts'), + './testing/*': entry('./src/__tests__/*.ts'), + }, + }), + ], + [ + 'packages/test-kit/package.json', + JSON.stringify({ + name: '@agent-device/test-kit', + private: true, + exports: { './tmp-dir': entry('./src/tmp-dir.ts') }, + }), + ], + [ + 'packages/consumer/package.json', + JSON.stringify({ + name: '@agent-device/consumer', + private: true, + exports: { '.': entry('./src/index.ts') }, + dependencies: { + '@agent-device/owner': 'workspace:*', + '@agent-device/test-kit': 'workspace:*', + }, + }), + ], + ]), +); +const consumer = packages.find((pkg) => pkg.name === '@agent-device/consumer')!; +const rootDeps = new Set(['@agent-device/owner', '@agent-device/test-kit']); + +function refusals(file: string, specifier: string) { + const sites = specifierSites(file, `import { x } from '${specifier}';`); + const inPackage = file.startsWith('packages/consumer/'); + return ( + inPackage + ? checkPackageInternalSites(consumer, sites, packages, true) + : checkRootSites(sites, packages, rootDeps, true) + ).filter((violation) => violation.rule === PACKAGE_EXPORT_SURFACE_RULE); +} + +const TEST_ONLY_SPECIFIERS = [ + '@agent-device/owner/thing-fixtures', + '@agent-device/owner/testing/anything', +]; + +test('production source importing a test-only export is refused, at root and in packages', () => { + for (const specifier of TEST_ONLY_SPECIFIERS) { + for (const file of ['src/commands/inventory.ts', 'packages/consumer/src/inventory.ts']) { + const violations = refusals(file, specifier); + assert.equal(violations.length, 1, `${file} ${specifier}`); + assert.match(violations[0]!.message, /test-only export/); + } + } +}); + +test('the vi.mock scan refuses a planted test-only export in production source too', () => { + const planted = "vi.mock('@agent-device/owner/thing-fixtures', () => ({}));"; + const violations = checkRootSites( + specifierSites('src/platform-runtime-something.ts', planted), + packages, + rootDeps, + true, + ); + assert.equal(violations.filter((v) => v.rule === PACKAGE_EXPORT_SURFACE_RULE).length, 1); +}); + +test('tests, fixtures, the test/ tree and scripts/ may import test-only exports and test-kit', () => { + for (const specifier of [...TEST_ONLY_SPECIFIERS, '@agent-device/test-kit/tmp-dir']) { + for (const file of [ + 'src/__tests__/consumer.test.ts', + 'src/commands/thing.test.ts', + 'src/commands/thing.fixtures.ts', + 'test/integration/consumer.ts', + 'scripts/vitest-setup.ts', + 'packages/consumer/src/__tests__/consumer.test.ts', + 'packages/consumer/src/thing.fixtures.ts', + 'packages/consumer/test/harness.ts', + ]) { + assert.deepEqual(refusals(file, specifier), [], `${file} ${specifier}`); + } + } +}); + +test('production source importing test-kit is refused', () => { + for (const file of ['src/commands/inventory.ts', 'packages/consumer/src/inventory.ts']) { + const violations = refusals(file, '@agent-device/test-kit/tmp-dir'); + assert.equal(violations.length, 1, file); + assert.match(violations[0]!.message, /private test-only package/); + } +}); + +test('a production subpath of the same package stays importable from production source', () => { + for (const file of ['src/commands/inventory.ts', 'packages/consumer/src/inventory.ts']) { + assert.deepEqual(refusals(file, '@agent-device/owner/thing'), [], file); + assert.deepEqual(refusals(file, '@agent-device/owner'), [], file); + } +}); diff --git a/scripts/layering/package-export-surface.ts b/scripts/layering/package-export-surface.ts new file mode 100644 index 0000000000..828e24d36b --- /dev/null +++ b/scripts/layering/package-export-surface.ts @@ -0,0 +1,224 @@ +// Catches: a workspace package whose `exports` publish test support — a fixture, fake, mock, +// harness, `__tests__/` module or `./testing/*` entry — so a test elsewhere can reach it, and a +// production file importing one of those entries or the private `@agent-device/test-kit` +// package. Tests belong beside the module they test and import it relatively; fixtures one +// package uses stay inside it; fixtures shared across packages live in `@agent-device/test-kit`, +// the one designated home. A production package's `exports` is its production surface. +// The offender list only shrinks: a new offender fails, and so does a list entry that no longer +// offends, so each cleanup deletes the entries it frees. +// Evidence: 122 exports across 10 packages existed only to feed tests (daemon `./testing/*`, +// `*-fixtures` on capture-kit, selectors, device-selection, ...), each a production-looking +// subpath nothing but a test imports. TEST_ONLY_FIXTURE_SUBPATHS (R11) and R13's Android list +// named two and four of them by hand; neither stopped the next one. +// Cost: one manifest scan plus one specifier check at the R11 import sites. +// Kill criterion: a maintainer decision that packages may publish test support in `exports`. +// +// R83 package-export-surface. Classification looks at the export key AND its target file, so +// renaming the key cannot hide a fixture, and naming a fixture `./thing` cannot either. +// `@agent-device/test-kit` is the one package exempt from the export check. `test/` (integration, +// e2e, wire-compat) and `scripts/` are not production sources. + +import type { LayeringViolation } from './model.ts'; +import { TEST_ONLY_EXPORT_BURN_DOWN } from './package-export-surface-burn-down.ts'; + +export const PACKAGE_EXPORT_SURFACE_RULE = 'R83 package-export-surface'; + +export const TEST_KIT_PACKAGE = '@agent-device/test-kit'; + +/** Words that mark a path segment or file name as test support, split on `/ . - _`. */ +const TEST_ONLY_TOKENS: ReadonlySet = new Set([ + 'testing', + 'fixture', + 'fixtures', + 'fake', + 'fakes', + 'mock', + 'mocks', + 'stub', + 'stubs', + 'harness', +]); + +/** `test-` pairs that mark test support; `test-command` and `session-test-*` do not. */ +const TEST_COMPANION_TOKENS: ReadonlySet = new Set([ + 'support', + 'host', + 'helper', + 'helpers', + 'util', + 'utils', + 'kit', +]); + +const TEST_FILE = /\.(?:test|spec)\.[cm]?[jt]sx?$/; +const TEST_DIRECTORY = /^(?:__[a-z]+__|tests?)$/; + +function hasTestOnlyTokens(segment: string): boolean { + const tokens = segment + .toLowerCase() + .split(/[^a-z0-9]+/) + .filter(Boolean); + return tokens.some( + (token, index) => + TEST_ONLY_TOKENS.has(token) || + (token === 'test' && TEST_COMPANION_TOKENS.has(tokens[index + 1] ?? '')), + ); +} + +/** An export key such as `./testing/context` or `./snapshot-state-fixtures`. */ +export function isTestOnlyExportKey(key: string): boolean { + return key.split('/').some((segment) => segment === '__tests__' || hasTestOnlyTokens(segment)); +} + +/** An export target relative to its package, such as `./src/snapshot-state.fixtures.ts`. */ +export function isTestOnlyExportTarget(target: string): boolean { + const segments = target.split('/').filter((segment) => segment !== '.' && segment !== ''); + const file = segments.pop() ?? ''; + return ( + TEST_FILE.test(file) || + hasTestOnlyTokens(file) || + segments.some((segment) => TEST_DIRECTORY.test(segment) || hasTestOnlyTokens(segment)) + ); +} + +/** Whether an export is test support, judged by its key and by every file it resolves to. */ +export function isTestOnlyExport(key: string, targets: readonly string[]): boolean { + return isTestOnlyExportKey(key) || targets.some(isTestOnlyExportTarget); +} + +const TEST_FILE_TREE = + /(^|\/)__tests__\/|\.test\.[cm]?[jt]sx?$|(?:^|\/)test\/|^packages\/test-kit\//; + +/** + * A file that is itself test support, so importing other test support is allowed there: the root + * `test/` tree, a package-local `test/` tree, `*.test.*` files, `__tests__/` trees, `*.fixtures.*` + * files and the test-kit package. + */ +export function isTestSupportFile(file: string): boolean { + return TEST_FILE_TREE.test(file) || hasTestOnlyTokens(file.slice(file.lastIndexOf('/') + 1)); +} + +/** + * Production source: root `src/` and `packages//src/`, minus the test-support files. The + * root `test/` tree and `scripts/` are not production sources, so tooling and vitest setup there + * may import test support. + */ +export function isProductionSource(file: string): boolean { + return /^(?:src|packages\/[^/]+\/src)\//.test(file) && !isTestSupportFile(file); +} + +function stripPackageDir(target: string): string { + return target.replace(/^packages\/[^/]+\//, './'); +} + +export type ExportManifest = { + file: string; + name: string; + /** Export key -> every string the entry resolves to (all conditions). */ + exports: ReadonlyMap; +}; + +function stringLeaves(value: unknown, out: string[] = []): string[] { + if (typeof value === 'string') out.push(value); + else if (value && typeof value === 'object') { + for (const child of Object.values(value)) stringLeaves(child, out); + } + return out; +} + +export function readExportManifests(manifests: ReadonlyMap): ExportManifest[] { + const parsed: ExportManifest[] = []; + for (const [file, text] of [...manifests].sort(([a], [b]) => a.localeCompare(b))) { + const manifest = JSON.parse(text) as { name?: string; exports?: unknown }; + if (!manifest.name) continue; + const exported = + typeof manifest.exports === 'string' + ? { '.': manifest.exports } + : ((manifest.exports as Record | undefined) ?? {}); + parsed.push({ + file, + name: manifest.name, + exports: new Map(Object.entries(exported).map(([key, value]) => [key, stringLeaves(value)])), + }); + } + return parsed; +} + +function violation(file: string, message: string): LayeringViolation { + return { rule: PACKAGE_EXPORT_SURFACE_RULE, file, line: 1, message }; +} + +/** + * Every test-only export of every package except test-kit must be in the burn-down list, and + * every list entry must still offend. `burnDown` entries read ` `. + */ +export function checkPackageExportSurface( + manifests: ReadonlyMap, + burnDown: readonly string[] = TEST_ONLY_EXPORT_BURN_DOWN, +): LayeringViolation[] { + const listed = new Set(burnDown); + const offending = new Set(); + const violations: LayeringViolation[] = []; + for (const pkg of readExportManifests(manifests)) { + if (pkg.name === TEST_KIT_PACKAGE) continue; + for (const [key, targets] of pkg.exports) { + if (!isTestOnlyExport(key, targets.map(stripPackageDir))) continue; + const entry = `${pkg.name} ${key}`; + offending.add(entry); + if (listed.has(entry)) continue; + violations.push( + violation( + pkg.file, + `'${pkg.name}' exports '${key}', which is test support (${targets.join(', ')}). ` + + `A package's exports are its production surface: keep the file inside the package ` + + `and import it relatively, or move a genuinely shared helper to ${TEST_KIT_PACKAGE}.`, + ), + ); + } + } + for (const entry of burnDown) { + if (offending.has(entry)) continue; + violations.push( + violation( + 'scripts/layering/package-export-surface-burn-down.ts', + `'${entry}' no longer exports test support; delete it from TEST_ONLY_EXPORT_BURN_DOWN.`, + ), + ); + } + return violations; +} + +/** + * The consumer-side half: production code never imports test-only exports or test-kit. The caller + * has already resolved `specifier` to `packageName` and, when the package names it, the + * `exportKey` and file `targets` it resolves through. + */ +export function testOnlyImportMessage( + file: string, + specifier: string, + packageName: string, + resolved: { key: string; targets: readonly string[] } | undefined, +): string | undefined { + if (!isProductionSource(file)) return undefined; + if (packageName === TEST_KIT_PACKAGE) { + return `'${specifier}' is the private test-only package; production source may not import it.`; + } + if (!resolved || !isTestOnlyExport(resolved.key, resolved.targets.map(stripPackageDir))) { + return undefined; + } + return ( + `'${specifier}' is a test-only export — production code may not import it; use the ` + + `package's production subpath or inject the port.` + ); +} + +/** + * Whether a full workspace specifier such as `@agent-device/platform-apple/runner/test-host` + * names a test-only export, judged by its key alone. For rules that hold only the specifier; + * R83's manifest scan also judges the target file. + */ +export function isTestOnlyExportSpecifier(specifier: string): boolean { + const match = /^@[^/]+\/[^/]+(\/.*)?$/.exec(specifier); + if (!match) return false; + return isTestOnlyExportKey(`.${match[1] ?? ''}`); +} diff --git a/scripts/layering/platform-package-policy.test.ts b/scripts/layering/platform-package-policy.test.ts index 4d87a879b7..0ab65ab67c 100644 --- a/scripts/layering/platform-package-policy.test.ts +++ b/scripts/layering/platform-package-policy.test.ts @@ -433,7 +433,7 @@ test('the Android mechanics facet is the named consumer seam and adb host wiring ); }); -test('the Android test-only fixture subpaths accept test consumers and deny production ones', () => { +test('R13 leaves the Android test-only fixture subpaths to R83 for every consumer', () => { const fixtureImport = "import { createAndroidSnapshotCapture } from '@agent-device/platform-android/snapshot-capture-fixtures';"; const allowed = validSources(); @@ -448,14 +448,14 @@ test('the Android test-only fixture subpaths accept test consumers and deny prod [], ); - // Production consumers ride no facet seam: importing a fixture is an R13 violation, even - // from a file the mechanics facet would otherwise accept. + // Production importers of a fixture are R83's to refuse (package-export-surface.test.ts); + // R13 holds no second list of them. for (const file of ['src/commands/replay/index.ts', 'src/core/interactors/android.ts']) { - const denied = validSources(); - denied.set(file, fixtureImport); - assert.match( - messages(denied).join('\n'), - /may import '@agent-device\/platform-android\/snapshot-capture-fixtures'/, + const production = validSources(); + production.set(file, fixtureImport); + assert.deepEqual( + messages(production).filter((message) => message.includes('fixtures')), + [], file, ); } diff --git a/scripts/layering/platform-package-policy.ts b/scripts/layering/platform-package-policy.ts index 4dc5a256e0..4dd04388c0 100644 --- a/scripts/layering/platform-package-policy.ts +++ b/scripts/layering/platform-package-policy.ts @@ -20,6 +20,7 @@ import { PLATFORMS } from '@agent-device/kernel/device'; import { parseImports, type LayeringViolation } from './model.ts'; import { checkPlatformComposition } from './platform-composition-policy.ts'; import { checkPlatformPackageSourcePolicy } from './platform-package-source-policy.ts'; +import { isTestOnlyExportSpecifier } from './package-export-surface.ts'; import { retiredPathViolations } from './retired-zone-policy.ts'; export const CANONICAL_PLATFORM_FAMILIES = PLATFORMS; @@ -85,7 +86,6 @@ const MECHANICS_FACET_SUBPATHS: Readonly !isTestOnlyExportSpecifier(subpath), + ); if ( - declaration.exportedSubpaths.length !== expectedSubpaths.length || - expectedSubpaths.some((subpath) => !declaration.exportedSubpaths.includes(subpath)) + productionSubpaths.length !== expectedSubpaths.length || + expectedSubpaths.some((subpath) => !productionSubpaths.includes(subpath)) ) { violations.push( violation( diff --git a/scripts/layering/test-only-fixture-subpaths.test.ts b/scripts/layering/test-only-fixture-subpaths.test.ts deleted file mode 100644 index ae08b061f4..0000000000 --- a/scripts/layering/test-only-fixture-subpaths.test.ts +++ /dev/null @@ -1,107 +0,0 @@ -// The workspace packages' test-only fixture subpaths: which subpaths the manifests expose as -// test-only, and which consumers R11 may import them. `TEST_ONLY_FIXTURE_SUBPATHS` in -// package-boundaries.ts makes the non-production-consumer claim for the non-platform packages -// R13 cannot reach; the platform fixture entries and their R13 predicate are pinned in -// platform-package-policy.test.ts. Split out of package-boundaries.test.ts when the R11 family -// crossed the test-size tripwire, move-only (the facade-surface.test.ts precedent); the -// platform-android manifest pin moved with them, so this file holds the whole fixture-subpath -// inventory question in one place. - -import assert from 'node:assert/strict'; -import path from 'node:path'; -import { test } from 'node:test'; -import { - checkPackageInternalSites, - checkRootSites, - readWorkspacePackages, - specifierSites, - TEST_ONLY_FIXTURE_SUBPATHS, - type WorkspacePackage, -} from './package-boundaries.ts'; - -const repoRoot = path.resolve(import.meta.dirname, '../..'); - -const kernel: WorkspacePackage = { - dir: 'packages/kernel', - name: '@agent-device/kernel', - exportTargets: new Map([ - ['@agent-device/kernel/errors', 'packages/kernel/src/errors.ts'], - ['@agent-device/kernel/device', 'packages/kernel/src/device.ts'], - ]), - workspaceDependencies: new Set(), - workspaceDependencyField: 'dependencies', - externalDependencies: new Map(), -}; - -const contracts: WorkspacePackage = { - dir: 'packages/contracts', - name: '@agent-device/contracts', - exportTargets: new Map([ - ['@agent-device/contracts/interaction', 'packages/contracts/src/interaction.ts'], - ]), - workspaceDependencies: new Set(['@agent-device/kernel']), - workspaceDependencyField: 'dependencies', - externalDependencies: new Map(), -}; - -const ALL = [kernel, contracts]; - -test('the test-only fixture subpaths accept test consumers and deny production ones', () => { - // A consumer package declaring both fixture owners, so only the fixture predicate can fire. - const consumer: WorkspacePackage = { - dir: 'packages/daemon-side', - name: '@agent-device/daemon-side', - exportTargets: new Map(), - workspaceDependencies: new Set(['@agent-device/device-selection', '@agent-device/replay-port']), - workspaceDependencyField: 'dependencies', - externalDependencies: new Map(), - }; - const rootDeps = new Set(consumer.workspaceDependencies); - // Pin the set independently: iterating the checker's own set would let deleting an entry - // silently delete its accept/deny coverage with it. - assert.deepEqual([...TEST_ONLY_FIXTURE_SUBPATHS].sort(), [ - '@agent-device/device-selection/inventory-gateways-fixtures', - '@agent-device/replay-port/replay-script-source-fixtures', - ]); - for (const specifier of TEST_ONLY_FIXTURE_SUBPATHS) { - const sites = (file: string) => specifierSites(file, `import { f } from '${specifier}';`); - for (const file of [ - 'src/daemon/__tests__/consumer.test.ts', - 'test/integration/consumer.ts', - 'packages/daemon-side/src/__tests__/consumer.test.ts', - 'packages/daemon-side/test/harness.ts', - ]) { - assert.deepEqual(checkRootSites(sites(file), ALL, rootDeps, true), [], file); - assert.deepEqual(checkPackageInternalSites(consumer, sites(file), ALL, true), [], file); - } - // Production consumers ride no seam: the same import from a root or package - // production file is an R11 violation in both scans. - for (const file of ['src/commands/inventory.ts', 'packages/daemon-side/src/inventory.ts']) { - const violations = file.startsWith('src/') - ? checkRootSites(sites(file), ALL, rootDeps, true) - : checkPackageInternalSites(consumer, sites(file), ALL, true); - assert.equal(violations.length, 1, file); - assert.match(violations[0]!.message, /test-only fixture subpath/); - } - } -}); - -test('the platform-android manifest names its test-only fixture subpaths', () => { - // The shared Android capture/walk fixtures (#3517). R13 denies production consumers - // (platform-package-policy.test.ts); their place in the manifest's export list is the - // R11-side half of the same inventory, pinned here with the full key list rather than - // growing the R11 real-tree test past the size tripwire. - const platformAndroidPackage = readWorkspacePackages(repoRoot).find( - (pkg) => pkg.name === '@agent-device/platform-android', - ); - assert.ok(platformAndroidPackage, 'platform-android package must exist'); - assert.deepEqual([...platformAndroidPackage.exportTargets.keys()].sort(), [ - '@agent-device/platform-android', - '@agent-device/platform-android/adb-host', - '@agent-device/platform-android/device-boot', - '@agent-device/platform-android/mechanics', - // Test-only entries: the shared Android capture/walk fixtures (#3517). - '@agent-device/platform-android/snapshot-capture-fixtures', - '@agent-device/platform-android/ui-hierarchy-fixtures', - ]); -}); From 363904485f032945903e9368f6e9cbc8dfdaf90f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 15 Oct 2026 14:24:46 +0200 Subject: [PATCH 2/3] refactor(layering): measure R83 against the merge-base and share one production predicate R83's offender set is now a merge-base ratchet measured through mergeBaseRatchets (no hand-kept list to append to), wildcard exports are expanded against the tracked package files, and the consumer ban judges every export condition. isProductionSourceFile in tracked-sources owns production versus test support for the layering scans, R11's test-only import ban and R13. Co-Authored-By: Claude Opus 5.5 --- scripts/__tests__/committed-source-tree.ts | 9 +- scripts/layering/check.ts | 24 +- .../layering/daemon-platform-boundary.test.ts | 13 +- .../daemon-platform-runtime-inventory.test.ts | 16 +- scripts/layering/model.ts | 1 - scripts/layering/package-boundaries.ts | 34 ++- .../package-export-surface-burn-down.ts | 147 ------------ .../layering/package-export-surface.test.ts | 223 +++++++++-------- scripts/layering/package-export-surface.ts | 224 +++++++++++------- .../layering/platform-package-policy.test.ts | 11 +- scripts/layering/platform-package-policy.ts | 18 +- scripts/layering/ratchet-reference.test.ts | 20 +- scripts/layering/ratchet-reference.ts | 21 +- scripts/layering/root-module-zones.ts | 1 - .../session-authority-overlay.test.ts | 20 +- scripts/layering/tracked-sources.test.ts | 54 +++++ scripts/layering/tracked-sources.ts | 24 +- 17 files changed, 460 insertions(+), 400 deletions(-) delete mode 100644 scripts/layering/package-export-surface-burn-down.ts create mode 100644 scripts/layering/tracked-sources.test.ts diff --git a/scripts/__tests__/committed-source-tree.ts b/scripts/__tests__/committed-source-tree.ts index a6699d9da4..f3557ef825 100644 --- a/scripts/__tests__/committed-source-tree.ts +++ b/scripts/__tests__/committed-source-tree.ts @@ -157,7 +157,7 @@ function readCommittedBlobs( /** * The same enumeration and blob read as `createCommittedSourceTree`, handed over as text: the - * production sources and workspace manifests committed at `treeish`. A whole-tree measurement + * production sources and workspace manifests committed at `treeish`, plus every committed path. A whole-tree measurement * (the layering ratchets) needs the corpus rather than a reader, and taking it from here is what * keeps its file set identical to the closure walker's. */ @@ -165,12 +165,13 @@ function readCommittedBlobs( export function readCommittedSources( repoRoot: string, treeish: string, -): { sources: Map; manifests: Map } { - const { sources, manifests } = committedSourceSet(listCommittedTree(repoRoot, treeish)); +): { sources: Map; manifests: Map; files: string[] } { + const files = listCommittedTree(repoRoot, treeish); + const { sources, manifests } = committedSourceSet(files); const blobs = readCommittedBlobs(repoRoot, treeish, [...sources, ...manifests]); const only = (files: readonly string[]) => new Map(files.flatMap((file) => (blobs.has(file) ? [[file, blobs.get(file)!] as const] : []))); - return { sources: only(sources), manifests: only(manifests) }; + return { sources: only(sources), manifests: only(manifests), files }; } /** diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index 271e6bcc84..28c48b87c7 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -61,8 +61,8 @@ // server (R81). The global spine // ranks all five layers at daemon-server's rank, so R5/R6/R80 are blind inside the zone. // - Over WORKSPACE PACKAGE EXPORTS: no package but test-kit publishes test support (fixtures, -// fakes, mocks, harnesses, `__tests__/`, `./testing/*`); a shrink-only burn-down list holds -// today's offenders, and production code may not import them or test-kit (R83). +// fakes, mocks, harnesses, `__tests__/`, `./testing/*`); the offender set is a merge-base +// ratchet, and production code may not import those exports or test-kit (R83). // - Over CONTRACTS PRODUCTION SOURCE: contracts owns vocabulary only — host, process, and timer // mechanics belong in capture-kit or an adapter (R18). // R6, R9, and the R10 R7 counts are ratchets with no written-down reference: each is the same @@ -138,14 +138,17 @@ import { checkDaemonPlatformBoundary } from './daemon-platform-boundary.ts'; import { checkDaemonPlatformRuntimeInventory } from './daemon-platform-runtime-inventory.ts'; import { checkDaemonClientEntry } from './daemon-client-entry.ts'; import { checkDaemonProcessReach } from './daemon-process-reach.ts'; -import { checkPackageExportSurface } from './package-export-surface.ts'; -import { TEST_ONLY_EXPORT_BURN_DOWN } from './package-export-surface-burn-down.ts'; +import { + checkPackageExportSurface, + packageExportSurfaceSummary, +} from './package-export-surface.ts'; import { checkDaemonLayers, DAEMON_ROOT } from './daemon-layers.ts'; import { checkSessionAuthorityOverlay, handlerOwnedOverlay } from './session-authority-overlay.ts'; import { DAEMON_SOURCE_ROOT, listTrackedPlatformZoneFiles, listTrackedProductionSources, + listTrackedPackageFiles, listTrackedSrcUtilsFiles, listTrackedTypeScriptFiles, } from './tracked-sources.ts'; @@ -441,8 +444,9 @@ function report( `concrete platform imports in every executable and type-only form; ` + `no production daemon file reaches the root platform-runtime family or a hub that ` + `does (R76); the daemon process reaches no caller-side replay source module over ` + - `static or dynamic edges (R82); package exports publish no new test support, with ` + - `${TEST_ONLY_EXPORT_BURN_DOWN.length} burn-down entries left (R83); and the handler-owned SessionState/SessionStore ` + + `static or dynamic edges (R82); ` + + `${packageExportSurfaceSummary(ratchets.testOnlyExports, reference.testOnlyExports)}; and ` + + `the handler-owned SessionState/SessionStore ` + `authority overlay holds at or under the merge-base (R75, ` + `${handlerOwnedShapeFiles} shape / ${handlerOwnedAuthorityFiles} authority files); and ` + `every production daemon file lives under a layer folder with no upward static edges ` + @@ -603,7 +607,8 @@ export const LAYERING_RULES: Readonly> = { context.sourceFiles.filter((file) => file.startsWith(DAEMON_SOURCE_ROOT)), context.edges, ), - 'package-export-surface': () => checkPackageExportSurface(readTrackedPackageManifests(repoRoot)), + 'package-export-surface': (context) => + checkPackageExportSurface(context.ratchets.testOnlyExports, context.reference.testOnlyExports), }; export function main(): number { @@ -616,7 +621,10 @@ export function main(): number { const parse = memoizedImportParser(); const edges = resolveImportEdges(sources, workspaceSpecifierTargets(repoRoot), parse); // Measured once and threaded: the rules and the success line must report the same numbers. - const ratchets = measureRatchets(sources, edges); + const ratchets = measureRatchets(sources, edges, { + manifests: readTrackedPackageManifests(repoRoot), + packageFiles: listTrackedPackageFiles(repoRoot), + }); const reference = mergeBaseRatchets(repoRoot, parse); const context: LayeringContext = { sourceFiles, diff --git a/scripts/layering/daemon-platform-boundary.test.ts b/scripts/layering/daemon-platform-boundary.test.ts index 13bd814f79..3595262443 100644 --- a/scripts/layering/daemon-platform-boundary.test.ts +++ b/scripts/layering/daemon-platform-boundary.test.ts @@ -6,7 +6,7 @@ import { findDaemonPlatformDependencies, } from './daemon-platform-boundary.ts'; -const daemonFile = 'packages/daemon/src/terminal-boundary-fixture.ts'; +const daemonFile = 'packages/daemon/src/terminal-boundary-case.ts'; function dependencies(source: string, file = daemonFile) { return findDaemonPlatformDependencies([{ path: file, source }]); @@ -92,7 +92,7 @@ test('R65 recognizes side-effect and aliased imports, including nested daemon pa ].join('\n'); assert.deepEqual( - dependencies(source, 'packages/daemon/src/sessions/handlers/terminal-boundary-fixture.ts').map( + dependencies(source, 'packages/daemon/src/sessions/handlers/terminal-boundary-case.ts').map( (found) => ({ kind: found.kind, line: found.line, @@ -269,13 +269,10 @@ test('R65 rejects platform selection inside cleanup orchestrators', () => { test('R65 ignores non-daemon and test-shaped records even when their syntax is red', () => { const source = "import { platform } from '../../../src/platforms/android/runtime.ts';"; - assert.deepEqual(dependencies(source, 'src/core/terminal-boundary-fixture.ts'), []); + assert.deepEqual(dependencies(source, 'src/core/terminal-boundary-case.ts'), []); + assert.deepEqual(dependencies(source, 'packages/daemon/src/terminal-boundary-case.test.ts'), []); assert.deepEqual( - dependencies(source, 'packages/daemon/src/terminal-boundary-fixture.test.ts'), - [], - ); - assert.deepEqual( - dependencies(source, 'packages/daemon/src/__tests__/terminal-boundary-fixture.ts'), + dependencies(source, 'packages/daemon/src/__tests__/terminal-boundary-case.ts'), [], ); }); diff --git a/scripts/layering/daemon-platform-runtime-inventory.test.ts b/scripts/layering/daemon-platform-runtime-inventory.test.ts index a79e3b65ba..855f96a6f5 100644 --- a/scripts/layering/daemon-platform-runtime-inventory.test.ts +++ b/scripts/layering/daemon-platform-runtime-inventory.test.ts @@ -59,18 +59,18 @@ for (const [file, target, symbol] of [ test('R76 rejects an edge with the pair and its line', () => { const sources = { 'src/platform-runtime-android-tool-host.ts': 'export function createAndroidToolHost() {}\n', - 'packages/daemon/src/fixture.ts': + 'packages/daemon/src/subject.ts': "import { createAndroidToolHost } from '../../../src/platform-runtime-android-tool-host.ts';\n" + 'void createAndroidToolHost;\n', }; - const found = edgeViolations(sources, 'packages/daemon/src/fixture.ts'); + const found = edgeViolations(sources, 'packages/daemon/src/subject.ts'); assert.equal(found.length, 1); assert.equal(found[0]!.rule, DAEMON_PLATFORM_RUNTIME_RULE); assert.equal(found[0]!.line, 1); assert.match(found[0]!.message, /daemon coupling to root platform mechanics is banned/); assert.match( found[0]!.message, - /packages\/daemon\/src\/fixture\.ts -> src\/platform-runtime-android-tool-host\.ts/, + /packages\/daemon\/src\/subject\.ts -> src\/platform-runtime-android-tool-host\.ts/, ); }); @@ -127,10 +127,10 @@ test('R76 ignores test-shaped and non-daemon importers', () => { test('R76 ignores retired-zone targets, which R65 owns', () => { const sources = { 'src/platforms/android.ts': 'export const legacy = 1;\n', - 'packages/daemon/src/fixture.ts': + 'packages/daemon/src/subject.ts': 'import { legacy } from "../../../src/platforms/android.ts";\nvoid legacy;\n', }; - assert.deepEqual(edgeViolations(sources, 'packages/daemon/src/fixture.ts'), []); + assert.deepEqual(edgeViolations(sources, 'packages/daemon/src/subject.ts'), []); }); test('the root composition family is src/platform-runtime.ts plus src/platform-runtime-*.ts only', () => { @@ -150,13 +150,13 @@ test('R76 catches a daemon import of a root hub that reaches the platform family 'src/hub-runtime.ts': "import { gateway } from './platform-runtime.ts';\nvoid gateway;\n" + 'export function hubFact() {}\n', - 'packages/daemon/src/fixture.ts': + 'packages/daemon/src/subject.ts': "import { hubFact } from '../../../src/hub-runtime.ts';\nvoid hubFact;\n", }; - const found = edgeViolations(sources, 'packages/daemon/src/fixture.ts'); + const found = edgeViolations(sources, 'packages/daemon/src/subject.ts'); assert.equal(found.length, 1); assert.equal(found[0]!.rule, DAEMON_PLATFORM_RUNTIME_RULE); - assert.match(found[0]!.message, /packages\/daemon\/src\/fixture\.ts -> src\/hub-runtime\.ts/); + assert.match(found[0]!.message, /packages\/daemon\/src\/subject\.ts -> src\/hub-runtime\.ts/); }); test('R76 does not turn an intra-daemon hop into a hub edge', () => { diff --git a/scripts/layering/model.ts b/scripts/layering/model.ts index 61962c80e4..a1114d3352 100644 --- a/scripts/layering/model.ts +++ b/scripts/layering/model.ts @@ -108,7 +108,6 @@ export const UNRANKED_ZONES: ReadonlySet = new Set([ 'host-kit', 'capture-kit', 'provision-kit', - 'test-kit', ...PLATFORMS.map((family) => `platform-${family}`), 'provider-webdriver', 'provider-limrun', diff --git a/scripts/layering/package-boundaries.ts b/scripts/layering/package-boundaries.ts index bc9c394e8e..3e141e5926 100644 --- a/scripts/layering/package-boundaries.ts +++ b/scripts/layering/package-boundaries.ts @@ -32,7 +32,12 @@ import fs from 'node:fs'; import path from 'node:path'; import { parseImports, resolveWorkspaceSpecifier } from './model.ts'; -import { PACKAGE_EXPORT_SURFACE_RULE, testOnlyImportMessage } from './package-export-surface.ts'; +import { + PACKAGE_EXPORT_SURFACE_RULE, + readExportManifests, + resolveAllConditionTargets, + testOnlyImportMessage, +} from './package-export-surface.ts'; import { listTrackedPackageManifests, listTrackedProductionSources } from './tracked-sources.ts'; export type PackageBoundaryViolation = { @@ -48,6 +53,8 @@ export type WorkspacePackage = { name: string; /** Full import specifier -> repo-relative source target, from `exports`. */ exportTargets: ReadonlyMap; + /** Export key -> every condition target (`types`, `import`, `default`, ...), repo-relative. */ + exportConditionTargets?: ReadonlyMap; /** Declared `workspace:*` dependencies on sibling internal packages. */ workspaceDependencies: ReadonlySet; /** Where siblings are declared: a published package bundles them, so it lists them as dev-only. */ @@ -141,6 +148,20 @@ export function workspacePackagesFromManifests( path.posix.join('packages', entry, path.posix.normalize(targetFile)), ); } + const conditionTargets = new Map( + readExportManifests(new Map([[manifestFile, manifests.get(manifestFile)!]])).flatMap( + (declared) => + declared.exports.map( + ({ key, targets }) => + [ + path.posix.join(manifest.name!, key), + targets.map((target) => + path.posix.join('packages', entry, path.posix.normalize(target)), + ), + ] as const, + ), + ), + ); const workspaceDependencyField = manifest.private === true ? 'dependencies' : 'devDependencies'; const workspaceDependencies = new Set( Object.entries(manifest[workspaceDependencyField] ?? {}) @@ -156,6 +177,7 @@ export function workspacePackagesFromManifests( dir: `packages/${entry}`, name: manifest.name, exportTargets, + exportConditionTargets: conditionTargets, workspaceDependencies, workspaceDependencyField, externalDependencies, @@ -187,14 +209,16 @@ function checkTestOnlyImportSite( site: SpecifierSite, target: WorkspacePackage, ): PackageBoundaryViolation | undefined { - const resolvedTarget = resolveWorkspaceSpecifier(site.specifier, target.exportTargets); + const resolvedTargets = resolveAllConditionTargets( + site.specifier, + target.exportConditionTargets ?? + new Map([...target.exportTargets].map(([key, file]) => [key, [file]])), + ); const message = testOnlyImportMessage( site.file, site.specifier, target.name, - resolvedTarget === undefined - ? undefined - : { key: `.${site.specifier.slice(target.name.length)}`, targets: [resolvedTarget] }, + resolvedTargets.length > 0 ? resolvedTargets : undefined, ); if (message === undefined) return undefined; return { rule: PACKAGE_EXPORT_SURFACE_RULE, file: site.file, line: site.line, message }; diff --git a/scripts/layering/package-export-surface-burn-down.ts b/scripts/layering/package-export-surface-burn-down.ts deleted file mode 100644 index 8d71784ecc..0000000000 --- a/scripts/layering/package-export-surface-burn-down.ts +++ /dev/null @@ -1,147 +0,0 @@ -// Today's test-only package exports (R83 package-export-surface), one ` ` per -// line and grouped by package so parallel cleanups delete disjoint lines. The list only shrinks: -// remove an entry in the change that removes the export; a stale entry fails the gate. - -export const TEST_ONLY_EXPORT_BURN_DOWN: readonly string[] = [ - // @agent-device/capture-kit - '@agent-device/capture-kit ./audio-probe-descriptor-fixtures', - '@agent-device/capture-kit ./recording-artifact-fixtures', - '@agent-device/capture-kit ./recording-mp4-fixtures', - '@agent-device/capture-kit ./snapshot-chrome-fixtures', - '@agent-device/capture-kit ./snapshot-presentation-ios-transitions-fixtures', - '@agent-device/capture-kit ./snapshot-state-fixtures', - - // @agent-device/contracts - '@agent-device/contracts ./dispatch-disclosure-fixtures', - - // @agent-device/daemon - '@agent-device/daemon ./testing/__tests__/android-observation-fixture', - '@agent-device/daemon ./testing/__tests__/application-lifecycle-runtime-fixture', - '@agent-device/daemon ./testing/__tests__/application-lifecycle-runtime-harness', - '@agent-device/daemon ./testing/__tests__/capture-disclosure-target-activation-fact', - '@agent-device/daemon ./testing/__tests__/device-selection-stub', - '@agent-device/daemon ./testing/__tests__/human-control-fixtures', - '@agent-device/daemon ./testing/__tests__/interaction-get-runtime-fixture', - '@agent-device/daemon ./testing/__tests__/ios-comparison-key-fixture', - '@agent-device/daemon ./testing/__tests__/legacy-snapshot-capture-fixture', - '@agent-device/daemon ./testing/__tests__/platform-services-fixture', - '@agent-device/daemon ./testing/__tests__/replay-runtime/replay-command-fixture', - '@agent-device/daemon ./testing/__tests__/request-router-dispatch-mocks', - '@agent-device/daemon ./testing/__tests__/request-router-open-harness', - '@agent-device/daemon ./testing/__tests__/runtime-binding-conformance', - '@agent-device/daemon ./testing/__tests__/screenshot-runtime-fixture', - '@agent-device/daemon ./testing/__tests__/selector-capture-fixture', - '@agent-device/daemon ./testing/__tests__/session-teardown.fixtures', - '@agent-device/daemon ./testing/__tests__/snapshot-runtime-fixture', - '@agent-device/daemon ./testing/__tests__/support/command-surface-fake', - '@agent-device/daemon ./testing/__tests__/support/device-claim-store', - '@agent-device/daemon ./testing/__tests__/support/provider-device-admission-fixture', - '@agent-device/daemon ./testing/__tests__/support/screen-recording-live-handle', - '@agent-device/daemon ./testing/__tests__/support/session-factories', - '@agent-device/daemon ./testing/__tests__/support/store-factory', - '@agent-device/daemon ./testing/__tests__/support/unavailable-device-runtime-gateway', - '@agent-device/daemon ./testing/__tests__/test-device-runtime-gateway', - '@agent-device/daemon ./testing/android-system-dialog', - '@agent-device/daemon ./testing/app-events', - '@agent-device/daemon ./testing/application-lifecycle-recovery', - '@agent-device/daemon ./testing/artifact-tracking', - '@agent-device/daemon ./testing/capture-disclosure', - '@agent-device/daemon ./testing/context', - '@agent-device/daemon ./testing/daemon-command-registry', - '@agent-device/daemon ./testing/deferred-interaction-outcome', - '@agent-device/daemon ./testing/device/device-claim-conflict', - '@agent-device/daemon ./testing/device/device-ready', - '@agent-device/daemon ./testing/generic-leaf-execution', - '@agent-device/daemon ./testing/gesture-runtime', - '@agent-device/daemon ./testing/handlers/__tests__/*', - '@agent-device/daemon ./testing/handlers/lease', - '@agent-device/daemon ./testing/handlers/session-app-deployment', - '@agent-device/daemon ./testing/handlers/session-appstate', - '@agent-device/daemon ./testing/handlers/session-doctor', - '@agent-device/daemon ./testing/handlers/session-doctor-app', - '@agent-device/daemon ./testing/handlers/session-doctor-metro', - '@agent-device/daemon ./testing/handlers/session-replay-command', - '@agent-device/daemon ./testing/handlers/snapshot', - '@agent-device/daemon ./testing/interaction/index', - '@agent-device/daemon ./testing/interaction/internal/__tests__/*', - '@agent-device/daemon ./testing/interaction/internal/interaction-android-escape', - '@agent-device/daemon ./testing/interaction/internal/interaction-ios-tap-outcome', - '@agent-device/daemon ./testing/interaction/internal/interaction-touch-android-readiness', - '@agent-device/daemon ./testing/interaction/internal/interaction-touch-response', - '@agent-device/daemon ./testing/interaction/internal/types', - '@agent-device/daemon ./testing/interactor-resolution', - '@agent-device/daemon ./testing/ios-app-session-hint', - '@agent-device/daemon ./testing/lease-registry', - '@agent-device/daemon ./testing/provider-device-admission', - '@agent-device/daemon ./testing/provider-lease-expiry', - '@agent-device/daemon ./testing/ref-frame', - '@agent-device/daemon ./testing/request-binding', - '@agent-device/daemon ./testing/request-dispatch-disclosure', - '@agent-device/daemon ./testing/request-dispatch-ledger', - '@agent-device/daemon ./testing/request-execution-scope', - '@agent-device/daemon ./testing/request-finalization', - '@agent-device/daemon ./testing/request-generic-dispatch', - '@agent-device/daemon ./testing/request-handler-chain', - '@agent-device/daemon ./testing/request-platform-provider-context', - '@agent-device/daemon ./testing/request-router', - '@agent-device/daemon ./testing/request-runtime-binding', - '@agent-device/daemon ./testing/response-views', - '@agent-device/daemon ./testing/screenshot-runtime', - '@agent-device/daemon ./testing/scroll-until', - '@agent-device/daemon ./testing/selector-capture-binding', - '@agent-device/daemon ./testing/selector-match-errors', - '@agent-device/daemon ./testing/selector-runtime', - '@agent-device/daemon ./testing/selector-runtime-backend', - '@agent-device/daemon ./testing/server/daemon-runtime', - '@agent-device/daemon ./testing/server/http-server', - '@agent-device/daemon ./testing/server/server-lifecycle', - '@agent-device/daemon ./testing/server/server-shutdown', - '@agent-device/daemon ./testing/server/transport', - '@agent-device/daemon ./testing/session-action-recorder', - '@agent-device/daemon ./testing/session-capture-binding', - '@agent-device/daemon ./testing/session-lifecycle/index', - '@agent-device/daemon ./testing/session-lifecycle/internal/__tests__/*', - '@agent-device/daemon ./testing/session-observability/__tests__/*', - '@agent-device/daemon ./testing/session-recovery-hints', - '@agent-device/daemon ./testing/session-runtime', - '@agent-device/daemon ./testing/session-script-writer', - '@agent-device/daemon ./testing/session-snapshot', - '@agent-device/daemon ./testing/session-state', - '@agent-device/daemon ./testing/session-store', - '@agent-device/daemon ./testing/session-teardown-budget', - '@agent-device/daemon ./testing/snapshot-capture', - '@agent-device/daemon ./testing/snapshot-diff-runtime', - '@agent-device/daemon ./testing/snapshot-interactor-capture', - '@agent-device/daemon ./testing/snapshot-quality-latch', - '@agent-device/daemon ./testing/snapshot-runtime', - '@agent-device/daemon ./testing/snapshot-session', - '@agent-device/daemon ./testing/wait-runtime', - '@agent-device/daemon ./testing/web-session-names', - - // @agent-device/daemon-contracts - '@agent-device/daemon-contracts ./daemon-registration-fixtures', - '@agent-device/daemon-contracts ./device-claims-fixtures', - - // @agent-device/device-selection - '@agent-device/device-selection ./inventory-gateways-fixtures', - - // @agent-device/host-kit - '@agent-device/host-kit ./archive-fixtures', - '@agent-device/host-kit ./audio-probe-fixtures', - - // @agent-device/platform-android - '@agent-device/platform-android ./snapshot-capture-fixtures', - '@agent-device/platform-android ./ui-hierarchy-fixtures', - - // @agent-device/platform-apple - '@agent-device/platform-apple ./runner/requests-fixtures', - '@agent-device/platform-apple ./runner/test-host', - - // @agent-device/replay-port - '@agent-device/replay-port ./replay-script-source-fixtures', - '@agent-device/replay-port ./target-classification-fixtures', - - // @agent-device/selectors - '@agent-device/selectors ./interaction-targeting-fixtures', - '@agent-device/selectors ./snapshot-geometry-fixtures', -]; diff --git a/scripts/layering/package-export-surface.test.ts b/scripts/layering/package-export-surface.test.ts index 8aa053032d..edda04e9f0 100644 --- a/scripts/layering/package-export-surface.test.ts +++ b/scripts/layering/package-export-surface.test.ts @@ -1,29 +1,26 @@ // R83 package-export-surface: the manifest scan (every shape of test support is caught, by key or -// by target), the shrink-only burn-down list, and the consumer-side ban at the R11 import sites. +// by any condition target, wildcards included), the merge-base ratchet, and the consumer-side ban +// at the R11 import sites. import assert from 'node:assert/strict'; -import path from 'node:path'; import { test } from 'node:test'; import { checkPackageInternalSites, checkRootSites, - readTrackedPackageManifests, specifierSites, workspacePackagesFromManifests, } from './package-boundaries.ts'; import { checkPackageExportSurface, - isProductionSource, + expandExportTarget, isTestOnlyExport, isTestOnlyExportSpecifier, - isTestSupportFile, + measureTestOnlyExports, PACKAGE_EXPORT_SURFACE_RULE, + packageExportSurfaceSummary, } from './package-export-surface.ts'; -import { TEST_ONLY_EXPORT_BURN_DOWN } from './package-export-surface-burn-down.ts'; -const repoRoot = path.resolve(import.meta.dirname, '../..'); - -function manifest(name: string, exports: Record): Map { +function manifest(name: string, exports: unknown): Map { const dir = name.replace('@agent-device/', ''); return new Map([[`packages/${dir}/package.json`, JSON.stringify({ name, exports })]]); } @@ -32,6 +29,10 @@ function entry(target: string) { return { types: target, default: target }; } +function measure(name: string, exports: unknown, packageFiles: readonly string[] = []) { + return measureTestOnlyExports(manifest(name, exports), packageFiles); +} + // Every shape the rule must catch: [description, export key, export target]. const TEST_SUPPORT_SHAPES: ReadonlyArray = [ ['a *.fixtures.ts target under an innocent key', './thing', './src/thing.fixtures.ts'], @@ -40,7 +41,6 @@ const TEST_SUPPORT_SHAPES: ReadonlyArray = [ ['a *-fixtures key', './thing-fixtures', './src/thing.ts'], ['a fixtures.ts target', './thing', './src/recording/fixtures.ts'], ['a target under __tests__/', './thing', './src/__tests__/support/thing.ts'], - ['a pattern target under __tests__/', './things/*', './src/things/__tests__/*.ts'], ['a ./testing/ key', './testing/thing', './src/thing.ts'], ['a testing/ target directory', './thing', './src/testing/thing.ts'], ['a test-support key', './test-support', './src/thing.ts'], @@ -55,6 +55,7 @@ const TEST_SUPPORT_SHAPES: ReadonlyArray = [ ['a harness key', './thing-harness', './src/thing.ts'], ['a harness target', './thing', './src/thing-harness.ts'], ['a *.test.ts target', './thing', './src/thing.test.ts'], + ['a *.spec.ts target', './thing', './src/thing.spec.ts'], ['a test/ target directory', './thing', './test/thing.ts'], ]; @@ -71,95 +72,142 @@ const PRODUCTION_SHAPES: ReadonlyArray = [ test('every shape of test support is caught, by key or by target', () => { for (const [description, key, target] of TEST_SUPPORT_SHAPES) { - const violations = checkPackageExportSurface( - manifest('@agent-device/planted', { [key]: entry(target) }), - [], - ); - assert.equal(violations.length, 1, description); - assert.equal(violations[0]!.rule, PACKAGE_EXPORT_SURFACE_RULE, description); - assert.match(violations[0]!.message, /exports test support|exports '/, description); + const offenders = measure('@agent-device/planted', { [key]: entry(target) }); + assert.equal(offenders.length, 1, description); + assert.equal(offenders[0]!.id, `@agent-device/planted ${key}`, description); } }); test('production exports that merely mention test or support are not flagged', () => { for (const [key, target] of PRODUCTION_SHAPES) { - assert.deepEqual( - checkPackageExportSurface(manifest('@agent-device/planted', { [key]: entry(target) }), []), - [], - key, - ); + assert.deepEqual(measure('@agent-device/planted', { [key]: entry(target) }), [], key); } }); -test('every export condition is judged, not only the default', () => { - const violations = checkPackageExportSurface( - manifest('@agent-device/planted', { +test('every export condition is judged, not only the selected one', () => { + assert.equal( + measure('@agent-device/planted', { + './thing': { types: './src/thing.fixtures.ts', default: './src/thing.ts' }, + }).length, + 1, + 'a fixture in types only', + ); + assert.equal( + measure('@agent-device/planted', { './thing': { types: './src/thing.ts', default: './src/thing.fixtures.ts' }, + }).length, + 1, + 'a fixture in default only', + ); + assert.equal( + measure('@agent-device/planted', { + './thing': { node: { import: './src/thing.fixtures.ts' }, default: './src/thing.ts' }, + }).length, + 1, + 'a fixture in a nested condition', + ); + assert.deepEqual( + measure('@agent-device/planted', { + './thing': { types: './src/thing.ts', default: './src/thing.ts' }, }), [], ); - assert.equal(violations.length, 1); }); test('a string exports map and a missing one are both read', () => { - const dir = (value: unknown) => - new Map([ - ['packages/p/package.json', JSON.stringify({ name: '@agent-device/p', exports: value })], - ]); - assert.deepEqual(checkPackageExportSurface(dir('./src/index.ts'), []), []); - assert.deepEqual(checkPackageExportSurface(dir(undefined), []), []); + const fixture = measure('@agent-device/p', './src/index.fixtures.ts'); + assert.equal(fixture.length, 1); + assert.equal(fixture[0]!.id, '@agent-device/p .'); + assert.deepEqual(measure('@agent-device/p', './src/index.ts'), []); + assert.deepEqual(measure('@agent-device/p', undefined), []); }); -test('test-kit is the one package allowed to export test support', () => { +test('a wildcard export is judged by the tracked files it publishes', () => { + const files = [ + 'packages/planted/src/index.ts', + 'packages/planted/src/things/a.ts', + 'packages/planted/src/things/b.fixtures.ts', + ]; + const catchAll = measure('@agent-device/planted', { './*': entry('./src/*.ts') }, files); + assert.equal(catchAll.length, 1); + assert.ok(catchAll[0]!.targets.includes('./src/things/b.fixtures.ts')); assert.deepEqual( - checkPackageExportSurface( - manifest('@agent-device/test-kit', { - './device-fixtures': entry('./src/device-fixtures.ts'), - }), - [], + measure( + '@agent-device/planted', + { './*': entry('./src/*.ts') }, + files.filter((file) => !file.includes('fixtures')), ), [], ); + // A pattern with no match stands for its own text, so one aimed at __tests__/ is caught + // before any file exists. + assert.equal( + measure('@agent-device/planted', { './things/*': entry('./src/__tests__/*.ts') }, []).length, + 1, + ); +}); + +test('expandExportTarget matches a star across directories and leaves literals alone', () => { + const files = ['packages/p/src/a.ts', 'packages/p/src/dir/b.ts', 'packages/p/src/dir/b.js']; + assert.deepEqual(expandExportTarget('packages/p', './src/*.ts', files), [ + './src/a.ts', + './src/dir/b.ts', + ]); + assert.deepEqual(expandExportTarget('packages/p', './src/index.ts', files), ['./src/index.ts']); }); -test('the burn-down list is shrink-only: a new offender fails and so does a stale entry', () => { - const planted = manifest('@agent-device/planted', { +test('test-kit is the one package allowed to export test support', () => { + assert.deepEqual( + measure('@agent-device/test-kit', { './device-fixtures': entry('./src/device-fixtures.ts') }), + [], + ); +}); + +test('growth against the merge-base fails; a cleanup or an unchanged set passes', () => { + const base = measure('@agent-device/planted', { + './listed-fixtures': entry('./src/listed.fixtures.ts'), + './gone-fixtures': entry('./src/gone.fixtures.ts'), + }); + const head = measure('@agent-device/planted', { './listed-fixtures': entry('./src/listed.fixtures.ts'), './new-fixtures': entry('./src/new.fixtures.ts'), }); - const violations = checkPackageExportSurface(planted, [ - '@agent-device/planted ./listed-fixtures', - '@agent-device/planted ./gone-fixtures', - ]); - assert.equal(violations.length, 2); - assert.match(violations[0]!.message, /exports '\.\/new-fixtures'/); + const violations = checkPackageExportSurface(head, base); + assert.equal(violations.length, 1); + assert.equal(violations[0]!.rule, PACKAGE_EXPORT_SURFACE_RULE); assert.equal(violations[0]!.file, 'packages/planted/package.json'); - assert.match(violations[1]!.message, /'@agent-device\/planted \.\/gone-fixtures' no longer/); - assert.equal(violations[1]!.file, 'scripts/layering/package-export-surface-burn-down.ts'); + assert.match(violations[0]!.message, /exports '\.\/new-fixtures'.*new\.fixtures\.ts/); + // Removing an export needs no edit, and an unchanged set is clean. + assert.deepEqual(checkPackageExportSurface([], base), []); + assert.deepEqual(checkPackageExportSurface(base, base), []); }); -test('a burn-down entry whose package is gone is stale', () => { - const violations = checkPackageExportSurface( - manifest('@agent-device/other', { '.': entry('./src/index.ts') }), - ['@agent-device/missing ./thing-fixtures'], - ); - assert.equal(violations.length, 1); - assert.match(violations[0]!.message, /delete it from TEST_ONLY_EXPORT_BURN_DOWN/); +test('a moved fixture file under an unchanged export key is not growth; a renamed key is', () => { + const base = measure('@agent-device/planted', { './x-fixtures': entry('./src/x.fixtures.ts') }); + const moved = measure('@agent-device/planted', { + './x-fixtures': entry('./src/deeper/x.fixtures.ts'), + }); + const renamed = measure('@agent-device/planted', { + './y-fixtures': entry('./src/x.fixtures.ts'), + }); + assert.deepEqual(checkPackageExportSurface(moved, base), []); + assert.equal(checkPackageExportSurface(renamed, base).length, 1); }); -test('the committed manifests offend exactly where the burn-down list says', () => { - assert.deepEqual( - checkPackageExportSurface(readTrackedPackageManifests(repoRoot), TEST_ONLY_EXPORT_BURN_DOWN), - [], - ); +test('a package new at head cannot bring test support with it', () => { + const head = measure('@agent-device/brand-new', { './mocks': entry('./src/mocks.ts') }); + assert.equal(checkPackageExportSurface(head, []).length, 1); }); -test('the burn-down list is unique and names a package and an export key per entry', () => { - assert.equal(new Set(TEST_ONLY_EXPORT_BURN_DOWN).size, TEST_ONLY_EXPORT_BURN_DOWN.length); - for (const line of TEST_ONLY_EXPORT_BURN_DOWN) { - assert.match(line, /^@agent-device\/[a-z-]+ \.(?:\/\S+)?$/, line); - assert.ok(!line.startsWith('@agent-device/test-kit '), 'test-kit is exempt, never listed'); - } +test('the summary reports the remaining count against the merge-base', () => { + const base = measure('@agent-device/planted', { + './a-fixtures': entry('./src/a.ts'), + './b-fixtures': entry('./src/b.ts'), + }); + assert.match( + packageExportSurfaceSummary(base.slice(0, 1), base), + /1 test-only export\(s\) remain against 2 at the merge-base \(R83\)/, + ); }); test('the key predicate works on specifiers too, for rules that hold only the specifier', () => { @@ -174,37 +222,6 @@ test('the key predicate works on specifiers too, for rules that hold only the sp assert.equal(isTestOnlyExport('./thing', ['./src/thing.fixtures.ts']), true); }); -test('isTestSupportFile and isProductionSource split the tree', () => { - for (const file of [ - 'test/integration/consumer.ts', - 'packages/daemon-side/test/harness.ts', - 'src/daemon/__tests__/consumer.ts', - '__tests__/root-consumer.ts', - 'scripts/layering/daemon-layers.test.tsx', - 'src/daemon/session-store.test.mts', - 'src/foo.test.js', - 'src/foo.test.cjs', - 'src/platform-runtime-gateway.fixtures.ts', - 'packages/platform-android/src/recording/fixtures.ts', - 'packages/test-kit/src/loopback.ts', - ]) { - assert.equal(isTestSupportFile(file), true, file); - assert.equal(isProductionSource(file), false, file); - } - for (const file of [ - 'src/commands/inventory.ts', - 'packages/kernel/src/errors.ts', - 'packages/daemon/src/sessions/session-store.ts', - ]) { - assert.equal(isTestSupportFile(file), false, file); - assert.equal(isProductionSource(file), true, file); - } - // Tooling and vitest setup are not production sources, so they may import test support. - for (const file of ['scripts/vitest-apple-runner-host-setup.ts', 'scripts/fuzz/arbitraries.ts']) { - assert.equal(isProductionSource(file), false, file); - } -}); - const packages = workspacePackagesFromManifests( new Map([ [ @@ -216,7 +233,9 @@ const packages = workspacePackagesFromManifests( '.': entry('./src/index.ts'), './thing': entry('./src/thing.ts'), './thing-fixtures': entry('./src/thing.fixtures.ts'), + './types-only': { types: './src/types-only.fixtures.ts', default: './src/types-only.ts' }, './testing/*': entry('./src/__tests__/*.ts'), + './wild/*': entry('./src/wild/*.ts'), }, }), ], @@ -258,6 +277,7 @@ function refusals(file: string, specifier: string) { const TEST_ONLY_SPECIFIERS = [ '@agent-device/owner/thing-fixtures', '@agent-device/owner/testing/anything', + '@agent-device/owner/types-only', ]; test('production source importing a test-only export is refused, at root and in packages', () => { @@ -270,6 +290,12 @@ test('production source importing a test-only export is refused, at root and in } }); +test('an export whose types condition is test support is refused whatever default says', () => { + // The selected (default) target is production; only the `types` leaf is a fixture. + assert.equal(refusals('src/commands/inventory.ts', '@agent-device/owner/types-only').length, 1); + assert.deepEqual(refusals('src/commands/inventory.ts', '@agent-device/owner/wild/a'), []); +}); + test('the vi.mock scan refuses a planted test-only export in production source too', () => { const planted = "vi.mock('@agent-device/owner/thing-fixtures', () => ({}));"; const violations = checkRootSites( @@ -281,11 +307,12 @@ test('the vi.mock scan refuses a planted test-only export in production source t assert.equal(violations.filter((v) => v.rule === PACKAGE_EXPORT_SURFACE_RULE).length, 1); }); -test('tests, fixtures, the test/ tree and scripts/ may import test-only exports and test-kit', () => { +test('tests, specs, fixtures, test trees and scripts/ may import test-only exports and test-kit', () => { for (const specifier of [...TEST_ONLY_SPECIFIERS, '@agent-device/test-kit/tmp-dir']) { for (const file of [ 'src/__tests__/consumer.test.ts', 'src/commands/thing.test.ts', + 'src/commands/thing.spec.ts', 'src/commands/thing.fixtures.ts', 'test/integration/consumer.ts', 'scripts/vitest-setup.ts', diff --git a/scripts/layering/package-export-surface.ts b/scripts/layering/package-export-surface.ts index 828e24d36b..f7455b49d7 100644 --- a/scripts/layering/package-export-surface.ts +++ b/scripts/layering/package-export-surface.ts @@ -3,23 +3,25 @@ // production file importing one of those entries or the private `@agent-device/test-kit` // package. Tests belong beside the module they test and import it relatively; fixtures one // package uses stay inside it; fixtures shared across packages live in `@agent-device/test-kit`, -// the one designated home. A production package's `exports` is its production surface. -// The offender list only shrinks: a new offender fails, and so does a list entry that no longer -// offends, so each cleanup deletes the entries it frees. +// the one designated home. A package's `exports` is its production surface. +// Today's offenders only shrink: the set is measured at the merge-base with origin/main by the +// same function that measures the working tree (`ratchet-reference.ts`), so growth fails, a +// cleanup needs no edit, and no allowlist exists to append to. // Evidence: 122 exports across 10 packages existed only to feed tests (daemon `./testing/*`, // `*-fixtures` on capture-kit, selectors, device-selection, ...), each a production-looking -// subpath nothing but a test imports. TEST_ONLY_FIXTURE_SUBPATHS (R11) and R13's Android list -// named two and four of them by hand; neither stopped the next one. -// Cost: one manifest scan plus one specifier check at the R11 import sites. +// subpath nothing but a test imports. Hand-kept lists of two and four of them (R11, R13) did +// not stop the next one. +// Cost: one manifest scan per tree plus one specifier check at the R11 import sites. // Kill criterion: a maintainer decision that packages may publish test support in `exports`. // -// R83 package-export-surface. Classification looks at the export key AND its target file, so -// renaming the key cannot hide a fixture, and naming a fixture `./thing` cannot either. -// `@agent-device/test-kit` is the one package exempt from the export check. `test/` (integration, -// e2e, wire-compat) and `scripts/` are not production sources. +// R83 package-export-surface. Classification looks at the export key AND every file the entry +// resolves to under every condition (`types`, `import`, `default`, ...), expanding `*` patterns +// against the tracked package files, so neither a key rename nor a catch-all pattern hides a +// fixture. `@agent-device/test-kit` is the one package exempt from the export check. "Production +// source" is `isProductionSourceFile` in `tracked-sources.ts`. -import type { LayeringViolation } from './model.ts'; -import { TEST_ONLY_EXPORT_BURN_DOWN } from './package-export-surface-burn-down.ts'; +import { resolveWorkspaceSpecifier, type LayeringViolation } from './model.ts'; +import { isProductionSourceFile } from './tracked-sources.ts'; export const PACKAGE_EXPORT_SURFACE_RULE = 'R83 package-export-surface'; @@ -86,36 +88,31 @@ export function isTestOnlyExport(key: string, targets: readonly string[]): boole return isTestOnlyExportKey(key) || targets.some(isTestOnlyExportTarget); } -const TEST_FILE_TREE = - /(^|\/)__tests__\/|\.test\.[cm]?[jt]sx?$|(?:^|\/)test\/|^packages\/test-kit\//; - -/** - * A file that is itself test support, so importing other test support is allowed there: the root - * `test/` tree, a package-local `test/` tree, `*.test.*` files, `__tests__/` trees, `*.fixtures.*` - * files and the test-kit package. - */ -export function isTestSupportFile(file: string): boolean { - return TEST_FILE_TREE.test(file) || hasTestOnlyTokens(file.slice(file.lastIndexOf('/') + 1)); -} - /** - * Production source: root `src/` and `packages//src/`, minus the test-support files. The - * root `test/` tree and `scripts/` are not production sources, so tooling and vitest setup there - * may import test support. + * Whether a full workspace specifier such as `@agent-device/platform-apple/runner/test-host` + * names a test-only export, judged by its key alone. For rules that hold only the specifier; + * R83's manifest scan also judges the target files. */ -export function isProductionSource(file: string): boolean { - return /^(?:src|packages\/[^/]+\/src)\//.test(file) && !isTestSupportFile(file); +export function isTestOnlyExportSpecifier(specifier: string): boolean { + const match = /^@[^/]+\/[^/]+(\/.*)?$/.exec(specifier); + if (!match) return false; + return isTestOnlyExportKey(`.${match[1] ?? ''}`); } function stripPackageDir(target: string): string { return target.replace(/^packages\/[^/]+\//, './'); } +/** One export of one package: its key and every string leaf of its conditions map. */ +export type ExportEntry = { + key: string; + targets: readonly string[]; +}; + export type ExportManifest = { file: string; name: string; - /** Export key -> every string the entry resolves to (all conditions). */ - exports: ReadonlyMap; + exports: readonly ExportEntry[]; }; function stringLeaves(value: unknown, out: string[] = []): string[] { @@ -138,87 +135,152 @@ export function readExportManifests(manifests: ReadonlyMap): Exp parsed.push({ file, name: manifest.name, - exports: new Map(Object.entries(exported).map(([key, value]) => [key, stringLeaves(value)])), + exports: Object.entries(exported).map(([key, value]) => ({ + key, + targets: stringLeaves(value), + })), }); } return parsed; } -function violation(file: string, message: string): LayeringViolation { - return { rule: PACKAGE_EXPORT_SURFACE_RULE, file, line: 1, message }; +/** + * The files a `*` pattern target covers among the tracked package files. A pattern with no + * match stands for its own text, so a pattern aimed at `__tests__/` is judged before any file + * exists. A literal target is its own single file. + */ +export function expandExportTarget( + packageDir: string, + target: string, + packageFiles: readonly string[], +): string[] { + const star = target.indexOf('*'); + if (star === -1) return [target]; + const prefix = `${packageDir}/${target.slice(0, star).replace(/^\.\//, '')}`; + const suffix = target.slice(star + 1); + const matches = packageFiles.filter( + (file) => + file.startsWith(prefix) && + file.length > prefix.length + suffix.length && + file.endsWith(suffix), + ); + return matches.length > 0 + ? matches.map((file) => `./${file.slice(packageDir.length + 1)}`) + : [target]; } +/** A test-only export of a package, with the files that made it one. */ +export type TestOnlyExport = { + /** ` `: the identity the merge-base comparison keys on. */ + id: string; + manifestFile: string; + key: string; + targets: readonly string[]; +}; + /** - * Every test-only export of every package except test-kit must be in the burn-down list, and - * every list entry must still offend. `burnDown` entries read ` `. + * Every test-only export of every package except test-kit in one tree. `packageFiles` is that + * tree's tracked paths, so wildcard exports are judged by the files they publish. */ -export function checkPackageExportSurface( +export function measureTestOnlyExports( manifests: ReadonlyMap, - burnDown: readonly string[] = TEST_ONLY_EXPORT_BURN_DOWN, -): LayeringViolation[] { - const listed = new Set(burnDown); - const offending = new Set(); - const violations: LayeringViolation[] = []; + packageFiles: readonly string[], +): TestOnlyExport[] { + const offenders: TestOnlyExport[] = []; for (const pkg of readExportManifests(manifests)) { if (pkg.name === TEST_KIT_PACKAGE) continue; - for (const [key, targets] of pkg.exports) { - if (!isTestOnlyExport(key, targets.map(stripPackageDir))) continue; - const entry = `${pkg.name} ${key}`; - offending.add(entry); - if (listed.has(entry)) continue; - violations.push( - violation( - pkg.file, - `'${pkg.name}' exports '${key}', which is test support (${targets.join(', ')}). ` + - `A package's exports are its production surface: keep the file inside the package ` + - `and import it relatively, or move a genuinely shared helper to ${TEST_KIT_PACKAGE}.`, - ), + const packageDir = pkg.file.replace(/\/package\.json$/, ''); + for (const { key, targets } of pkg.exports) { + const files = targets.flatMap((target) => + expandExportTarget(packageDir, target, packageFiles), ); + if (!isTestOnlyExport(key, files)) continue; + offenders.push({ id: `${pkg.name} ${key}`, manifestFile: pkg.file, key, targets: files }); } } - for (const entry of burnDown) { - if (offending.has(entry)) continue; - violations.push( - violation( - 'scripts/layering/package-export-surface-burn-down.ts', - `'${entry}' no longer exports test support; delete it from TEST_ONLY_EXPORT_BURN_DOWN.`, - ), - ); + return offenders; +} + +/** + * Growth fails: a test-only export at head that the merge-base did not already publish. A + * cleanup that removes exports needs no edit, and nothing sits above what main actually holds. + */ +export function checkPackageExportSurface( + head: readonly TestOnlyExport[], + base: readonly TestOnlyExport[], +): LayeringViolation[] { + const published = new Set(base.map(({ id }) => id)); + return head + .filter(({ id }) => !published.has(id)) + .map(({ id, manifestFile, key, targets }) => { + const support = targets.filter(isTestOnlyExportTarget); + return { + rule: PACKAGE_EXPORT_SURFACE_RULE, + file: manifestFile, + line: 1, + message: + `'${id.slice(0, id.indexOf(' '))}' exports '${key}', which is test support ` + + `(${[...new Set(support.length > 0 ? support : targets)].slice(0, 3).join(', ')}). ` + + `A package's exports are its production surface: keep the file inside the package ` + + `and import it relatively, or move a genuinely shared helper to ${TEST_KIT_PACKAGE}.`, + }; + }); +} + +/** The `R83` success-line fragment: how many test-only exports remain, against the merge-base. */ +export function packageExportSurfaceSummary( + head: readonly TestOnlyExport[], + base: readonly TestOnlyExport[], +): string { + return ( + `package exports publish no new test support: ${head.length} test-only export(s) remain ` + + `against ${base.length} at the merge-base (R83)` + ); +} + +/** + * Every condition target an import specifier resolves to, repo-relative, through the same + * exports semantics as `resolveWorkspaceSpecifier`. `conditionTargets` maps each export key to + * all of its condition leaves, in declaration order. + */ +export function resolveAllConditionTargets( + specifier: string, + conditionTargets: ReadonlyMap, +): string[] { + const width = Math.max(0, ...[...conditionTargets.values()].map((targets) => targets.length)); + const resolved = new Set(); + for (let slot = 0; slot < width; slot += 1) { + const slotTargets = new Map(); + for (const [key, targets] of conditionTargets) { + const target = targets[slot]; + if (target !== undefined) slotTargets.set(key, target); + } + const target = resolveWorkspaceSpecifier(specifier, slotTargets); + if (target !== undefined) resolved.add(target); } - return violations; + return [...resolved]; } /** * The consumer-side half: production code never imports test-only exports or test-kit. The caller - * has already resolved `specifier` to `packageName` and, when the package names it, the - * `exportKey` and file `targets` it resolves through. + * has already resolved `specifier` to `packageName` and, when the package names it, every + * condition target it resolves through. */ export function testOnlyImportMessage( file: string, specifier: string, packageName: string, - resolved: { key: string; targets: readonly string[] } | undefined, + resolvedTargets: readonly string[] | undefined, ): string | undefined { - if (!isProductionSource(file)) return undefined; + if (!isProductionSourceFile(file)) return undefined; if (packageName === TEST_KIT_PACKAGE) { return `'${specifier}' is the private test-only package; production source may not import it.`; } - if (!resolved || !isTestOnlyExport(resolved.key, resolved.targets.map(stripPackageDir))) { - return undefined; - } + if (resolvedTargets === undefined) return undefined; + const key = `.${specifier.slice(packageName.length)}`; + if (!isTestOnlyExport(key, resolvedTargets.map(stripPackageDir))) return undefined; return ( `'${specifier}' is a test-only export — production code may not import it; use the ` + `package's production subpath or inject the port.` ); } - -/** - * Whether a full workspace specifier such as `@agent-device/platform-apple/runner/test-host` - * names a test-only export, judged by its key alone. For rules that hold only the specifier; - * R83's manifest scan also judges the target file. - */ -export function isTestOnlyExportSpecifier(specifier: string): boolean { - const match = /^@[^/]+\/[^/]+(\/.*)?$/.exec(specifier); - if (!match) return false; - return isTestOnlyExportKey(`.${match[1] ?? ''}`); -} diff --git a/scripts/layering/platform-package-policy.test.ts b/scripts/layering/platform-package-policy.test.ts index 0ab65ab67c..6b385fd4c2 100644 --- a/scripts/layering/platform-package-policy.test.ts +++ b/scripts/layering/platform-package-policy.test.ts @@ -443,21 +443,14 @@ test('R13 leaves the Android test-only fixture subpaths to R83 for every consume fixtureImport, ); allowed.set('test/integration/provider-scenarios/android.test.ts', fixtureImport); - assert.deepEqual( - messages(allowed).filter((message) => message.includes('may import')), - [], - ); + assert.deepEqual(messages(allowed), []); // Production importers of a fixture are R83's to refuse (package-export-surface.test.ts); // R13 holds no second list of them. for (const file of ['src/commands/replay/index.ts', 'src/core/interactors/android.ts']) { const production = validSources(); production.set(file, fixtureImport); - assert.deepEqual( - messages(production).filter((message) => message.includes('fixtures')), - [], - file, - ); + assert.deepEqual(messages(production), [], file); } }); diff --git a/scripts/layering/platform-package-policy.ts b/scripts/layering/platform-package-policy.ts index 4dd04388c0..cbce761e5c 100644 --- a/scripts/layering/platform-package-policy.ts +++ b/scripts/layering/platform-package-policy.ts @@ -15,7 +15,7 @@ // into a sibling's src still compiles; project references are a build cache, not a boundary. import path from 'node:path'; -import { DAEMON_SOURCE_ROOT } from './tracked-sources.ts'; +import { DAEMON_SOURCE_ROOT, isProductionSourceFile } from './tracked-sources.ts'; import { PLATFORMS } from '@agent-device/kernel/device'; import { parseImports, type LayeringViolation } from './model.ts'; import { checkPlatformComposition } from './platform-composition-policy.ts'; @@ -104,7 +104,7 @@ function isAndroidFacetSubpathImport(file: string, specifier: string): boolean { // The named facets are the implementation seam for root/core/SDK consumers. Daemon production // code still reaches platform behavior through the request-bound runtime gateway; tests may // import a facet to exercise the package-owned mechanics directly. - return !file.startsWith(DAEMON_SOURCE_ROOT) || !isProductionSource(file); + return !file.startsWith(DAEMON_SOURCE_ROOT) || !isProductionSourceFile(file); } // Test-only subpaths (fixtures, the runner test host) are R83's: it holds the export list and @@ -136,14 +136,6 @@ function familyForPackageFile(file: string): string | undefined { return /^packages\/platform-([^/]+)\//.exec(file)?.[1]; } -function isProductionSource(file: string): boolean { - return !file.endsWith('.test.ts') && !file.includes('/__tests__/'); -} - -function isNonProductionConsumer(file: string): boolean { - return !isProductionSource(file) || file.startsWith('test/'); -} - function concretePlatformFamily(specifier: string): string | undefined { return /^@agent-device\/platform-([^/]+)(?:\/|$)/.exec(specifier)?.[1]; } @@ -252,7 +244,7 @@ function checkSource(file: string, source: string): LayeringViolation[] { // of the facet's definition, so the ambient-host rules do not apply to it. // Raw process primitives stay banned below and host tooling still enters // through the AppleRunnerHost port. - if (ownerFamily && isProductionSource(file) && !file.startsWith(APPLE_RUNNER_SUBTREE)) { + if (ownerFamily && isProductionSourceFile(file) && !file.startsWith(APPLE_RUNNER_SUBTREE)) { violations.push(...checkPlatformPackageSourcePolicy(file, source, ownerFamily)); } for (const site of parseImports(source)) { @@ -353,7 +345,7 @@ function checkSource(file: string, source: string): LayeringViolation[] { // (they intercept, not spawn), and the runner host port names ChildProcess // as a type. The spawn ban is about mechanics bypassing the host-command // port at runtime. - if (RAW_PROCESS_SPECIFIERS.has(site.spec) && isProductionSource(file) && !site.typeOnly) { + if (RAW_PROCESS_SPECIFIERS.has(site.spec) && isProductionSourceFile(file) && !site.typeOnly) { violations.push( violation( file, @@ -386,7 +378,7 @@ function isAllowedPlatformRootImport( ): boolean { if (site.spec !== `@agent-device/platform-${family}`) return false; return ( - isNonProductionConsumer(file) || + !isProductionSourceFile(file) || file.startsWith('src/core/interactors/') || (PLATFORM_RUNTIME_HOST_FILES.has(file) && (site.dynamic || site.typeOnly)) ); diff --git a/scripts/layering/ratchet-reference.test.ts b/scripts/layering/ratchet-reference.test.ts index 08d67ade7b..de8c37d57b 100644 --- a/scripts/layering/ratchet-reference.test.ts +++ b/scripts/layering/ratchet-reference.test.ts @@ -59,9 +59,23 @@ test('measureRatchets reports all ratchets from one tree', () => { largestTypeCycle: ['src/client/client.ts', 'src/commands/loop.ts'], sessionState: { writerOwnedFields: 2, ownerFileClaims: 3 }, sessionAuthority: { shapeFiles: [], authorityFiles: [] }, + testOnlyExports: [], }); + const withExports = measureRatchets(sources, resolveImportEdges(sources), { + manifests: new Map([ + [ + 'packages/p/package.json', + JSON.stringify({ name: '@agent-device/p', exports: { './x-fixtures': './src/x.ts' } }), + ], + ]), + packageFiles: [], + }); + assert.deepEqual( + withExports.testOnlyExports.map(({ id }) => id), + ['@agent-device/p ./x-fixtures'], + ); const withAuthority = tree({ - 'packages/daemon/src/sessions/handlers/fixture.ts': + 'packages/daemon/src/sessions/handlers/subject.ts': "import type { SessionState } from '../../core/session-state.ts';\n" + "import { SessionStore } from '../../core/session-store.ts';", 'packages/daemon/src/core/session-store.ts': 'export class SessionStore {}', @@ -69,8 +83,8 @@ test('measureRatchets reports all ratchets from one tree', () => { assert.deepEqual( measureRatchets(withAuthority, resolveImportEdges(withAuthority)).sessionAuthority, { - shapeFiles: ['packages/daemon/src/sessions/handlers/fixture.ts'], - authorityFiles: ['packages/daemon/src/sessions/handlers/fixture.ts'], + shapeFiles: ['packages/daemon/src/sessions/handlers/subject.ts'], + authorityFiles: ['packages/daemon/src/sessions/handlers/subject.ts'], }, ); }); diff --git a/scripts/layering/ratchet-reference.ts b/scripts/layering/ratchet-reference.ts index 5e2dd9e51c..9352887e13 100644 --- a/scripts/layering/ratchet-reference.ts +++ b/scripts/layering/ratchet-reference.ts @@ -22,6 +22,7 @@ import { type ResolvedImportEdge, } from './model.ts'; import { workspaceSpecifierTargetsFromManifests } from './package-boundaries.ts'; +import { measureTestOnlyExports, type TestOnlyExport } from './package-export-surface.ts'; import { measureSessionAuthorityOverlay, type SessionAuthorityOverlay, @@ -37,6 +38,14 @@ export type LayeringRatchets = Readonly<{ sessionState: SessionStateWritePressure; /** R75: SessionState-shape / SessionStore-authority overlay, full production scope. */ sessionAuthority: SessionAuthorityOverlay; + /** R83: the test-only package exports published, whole tree, test-kit excluded. */ + testOnlyExports: readonly TestOnlyExport[]; +}>; + +/** What R83 measures a tree from: its package manifests and its tracked package paths. */ +export type PackageExportSurfaceInput = Readonly<{ + manifests: ReadonlyMap; + packageFiles: readonly string[]; }>; export type MergeBaseRatchets = LayeringRatchets & Readonly<{ ref: string }>; @@ -44,12 +53,14 @@ export type MergeBaseRatchets = LayeringRatchets & Readonly<{ ref: string }>; export function measureRatchets( sources: ReadonlyMap, edges: readonly ResolvedImportEdge[], + exportSurface: PackageExportSurfaceInput = { manifests: new Map(), packageFiles: [] }, ): LayeringRatchets { return { typeInversions: typeInversionCounts(edges), largestTypeCycle: largestTypeCycleMembers(edges), sessionState: sessionStateWritePressure(sources), sessionAuthority: measureSessionAuthorityOverlay(sources, edges), + testOnlyExports: measureTestOnlyExports(exportSurface.manifests, exportSurface.packageFiles), }; } @@ -66,7 +77,7 @@ export function mergeBaseRatchets( baseRef?: string, ): MergeBaseRatchets { const ref = baseRef ?? mergeBaseWithMain(repoRoot); - const { sources, manifests } = readCommittedSources(repoRoot, ref); + const { sources, manifests, files } = readCommittedSources(repoRoot, ref); // Measure the base tree in the working tree's coordinates: a `git mv` since the merge-base // (the daemon's move to packages/daemon, 11b) would otherwise make rules keyed on current // paths read the base as empty and accept any growth above a zero reference. Edges are @@ -90,5 +101,11 @@ export function mergeBaseRatchets( : { ...edge, file, target, fromZone: targetDagZone(file), toZone: targetDagZone(target) }; }); const canonical = new Map([...sources].map(([file, text]) => [toHead(file), text])); - return { ref, ...measureRatchets(canonical, edges) }; + return { + ref, + ...measureRatchets(canonical, edges, { + manifests, + packageFiles: files.filter((file) => file.startsWith('packages/')), + }), + }; } diff --git a/scripts/layering/root-module-zones.ts b/scripts/layering/root-module-zones.ts index 5b3ee18d51..899aa752a9 100644 --- a/scripts/layering/root-module-zones.ts +++ b/scripts/layering/root-module-zones.ts @@ -42,7 +42,6 @@ export const ROOT_MODULE_ZONES: Readonly> = { 'src/platform-runtime-device-boot.ts', 'src/platform-runtime-device-inventory.ts', 'src/platform-runtime-device-ready.ts', - 'src/platform-runtime-gateway.fixtures.ts', 'src/platform-runtime-gateway.ts', 'src/platform-runtime-host-diagnostics.ts', 'src/platform-runtime-managed-web-backend.ts', diff --git a/scripts/layering/session-authority-overlay.test.ts b/scripts/layering/session-authority-overlay.test.ts index 0a2be609c9..2af4e0a8dc 100644 --- a/scripts/layering/session-authority-overlay.test.ts +++ b/scripts/layering/session-authority-overlay.test.ts @@ -31,7 +31,7 @@ test('the overlay counts symbol-level production importers, not file-level impor const sources = { 'packages/daemon/src/core/session-state.ts': STATE_STUB, 'packages/daemon/src/core/session-store.ts': STORE_STUB, - 'packages/daemon/src/sessions/handlers/fixture.ts': + 'packages/daemon/src/sessions/handlers/subject.ts': "import type { SessionState } from '../../core/session-state.ts';\n" + "import { SessionStore } from '../../core/session-store.ts';\n", 'packages/daemon/src/sessions/handlers/request-only.ts': @@ -39,15 +39,15 @@ test('the overlay counts symbol-level production importers, not file-level impor "import { resolveDaemonStateDir } from '../../core/session-store.ts';\n", 'packages/daemon/src/non-handler.ts': "import type { SessionState } from './core/session-state.ts';\n", - 'packages/daemon/src/sessions/handlers/fixture.test.ts': + 'packages/daemon/src/sessions/handlers/subject.test.ts': "import type { SessionState } from '../../core/session-state.ts';\n", }; assert.deepEqual(overlayOf(sources), { shapeFiles: [ 'packages/daemon/src/non-handler.ts', - 'packages/daemon/src/sessions/handlers/fixture.ts', + 'packages/daemon/src/sessions/handlers/subject.ts', ], - authorityFiles: ['packages/daemon/src/sessions/handlers/fixture.ts'], + authorityFiles: ['packages/daemon/src/sessions/handlers/subject.ts'], }); }); @@ -58,7 +58,7 @@ test('the overlay follows the SessionState declaration to a moved path, not a re 'packages/daemon/src/types.ts': 'export type DaemonRequest = { x: number; };\n', 'packages/daemon/src/core/session-state.ts': STATE_STUB, 'packages/daemon/src/core/session-store.ts': STORE_STUB, - 'packages/daemon/src/sessions/handlers/fixture.ts': + 'packages/daemon/src/sessions/handlers/subject.ts': "import type { SessionState } from '../../core/session-state.ts';\n", 'packages/daemon/src/non-handler.ts': "import type { SessionState } from './core/session-state.ts';\n", @@ -66,7 +66,7 @@ test('the overlay follows the SessionState declaration to a moved path, not a re assert.deepEqual(overlayOf(sources), { shapeFiles: [ 'packages/daemon/src/non-handler.ts', - 'packages/daemon/src/sessions/handlers/fixture.ts', + 'packages/daemon/src/sessions/handlers/subject.ts', ], authorityFiles: [], }); @@ -75,7 +75,7 @@ test('the overlay follows the SessionState declaration to a moved path, not a re test('the handler-owned subset is every /handlers/ folder, not a recorded pair', () => { const overlay: SessionAuthorityOverlay = { shapeFiles: [ - 'packages/daemon/src/sessions/handlers/fixture.ts', + 'packages/daemon/src/sessions/handlers/subject.ts', 'packages/daemon/src/execution/handlers/snapshot-handler.ts', // A handler folder under a layer the split never produced still joins the ratcheted // subset: membership comes from the folder shape, so no rule edit can miss it. @@ -85,7 +85,7 @@ test('the handler-owned subset is every /handlers/ folder, not a recorded 'packages/daemon/src/execution/interaction/internal/interaction.ts', ], authorityFiles: [ - 'packages/daemon/src/sessions/handlers/fixture.ts', + 'packages/daemon/src/sessions/handlers/subject.ts', 'packages/daemon/src/execution/handlers/snapshot-handler.ts', 'packages/daemon/src/core/handlers/newcomer.ts', 'packages/daemon/src/server/request-router.ts', @@ -93,12 +93,12 @@ test('the handler-owned subset is every /handlers/ folder, not a recorded }; assert.deepEqual(handlerOwnedOverlay(overlay), { shapeFiles: [ - 'packages/daemon/src/sessions/handlers/fixture.ts', + 'packages/daemon/src/sessions/handlers/subject.ts', 'packages/daemon/src/execution/handlers/snapshot-handler.ts', 'packages/daemon/src/core/handlers/newcomer.ts', ], authorityFiles: [ - 'packages/daemon/src/sessions/handlers/fixture.ts', + 'packages/daemon/src/sessions/handlers/subject.ts', 'packages/daemon/src/execution/handlers/snapshot-handler.ts', 'packages/daemon/src/core/handlers/newcomer.ts', ], diff --git a/scripts/layering/tracked-sources.test.ts b/scripts/layering/tracked-sources.test.ts new file mode 100644 index 0000000000..b7ed3ce617 --- /dev/null +++ b/scripts/layering/tracked-sources.test.ts @@ -0,0 +1,54 @@ +// The one production-versus-test predicate. R11's test-only import ban, R13 and the layering +// scans all call `isProductionSourceFile`, so what counts as production is pinned here once. + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { isProductionSourceFile } from './tracked-sources.ts'; + +test('production source is a non-test TypeScript file under root src/ or a package src/', () => { + for (const file of [ + 'src/commands/inventory.ts', + 'src/cli.ts', + 'packages/kernel/src/errors.ts', + 'packages/daemon/src/sessions/session-store.ts', + 'packages/replay-port/src/daemon-port/test-command.ts', + 'packages/capture-kit/src/ios-snapshot-engine/conformance-harness.ts', + 'packages/platform-android/src/testing-hooks-policy.ts', + ]) { + assert.equal(isProductionSourceFile(file), true, file); + } +}); + +test('test support is not production source, whatever shape it takes', () => { + for (const [file, shape] of [ + ['src/daemon/__tests__/consumer.ts', 'a root __tests__ tree'], + ['packages/daemon/src/handlers/__tests__/thing.ts', 'a package __tests__ tree'], + ['__tests__/root-consumer.ts', 'a top-level __tests__ tree'], + ['src/commands/thing.test.ts', 'a .test file'], + ['src/commands/thing.spec.ts', 'a .spec file'], + ['packages/kernel/src/thing.test.ts', 'a package .test file'], + ['packages/kernel/src/thing.fixtures.ts', 'a .fixtures file'], + ['packages/kernel/src/thing.fixture.ts', 'a .fixture file'], + ['packages/platform-android/src/recording/fixtures.ts', 'a fixtures.ts file'], + ['packages/kernel/src/thing-fixtures.ts', 'a -fixtures file'], + ['packages/kernel/src/test/harness.ts', 'a package-local test/ tree under src'], + ['packages/daemon-side/test/harness.ts', 'a package-local test/ tree'], + ['test/integration/consumer.ts', 'the root test/ tree'], + ['packages/test-kit/src/loopback.ts', 'the test-kit package'], + ] as const) { + assert.equal(isProductionSourceFile(file), false, `${file} (${shape})`); + } +}); + +test('tooling and non-TypeScript files are not production sources', () => { + for (const file of [ + 'scripts/vitest-apple-runner-host-setup.ts', + 'scripts/fuzz/arbitraries.ts', + 'src/commands/data.json', + 'packages/kernel/src/errors.js', + 'packages/kernel/package.json', + 'docs/writing/tests.ts', + ]) { + assert.equal(isProductionSourceFile(file), false, file); + } +}); diff --git a/scripts/layering/tracked-sources.ts b/scripts/layering/tracked-sources.ts index f33447c058..da5cd00a34 100644 --- a/scripts/layering/tracked-sources.ts +++ b/scripts/layering/tracked-sources.ts @@ -75,9 +75,29 @@ function listTrackedFiles(repoRoot: string, pathspecs: readonly string[]): strin */ export const DAEMON_SOURCE_ROOT = 'packages/daemon/src/'; -/** Production sources only: test files and `__tests__/` trees are not layering subjects. */ +const PRODUCTION_SOURCE_ROOT = /^(?:src|packages\/[^/]+\/src)\//; + +/** + * Test support, by path: `__tests__/` and package-local `test/` trees, `*.test.*` and + * `*.spec.*` files, `*.fixtures.*` files (also `fixtures.ts` and `*-fixtures.ts`), and the + * test-kit package. + */ +const TEST_SUPPORT_PATH = + /(?:^|\/)(?:__tests__|test)\/|\.(?:test|spec)\.[cm]?[jt]sx?$|(?:^|[/.-])fixtures?\.[cm]?[jt]sx?$|^packages\/test-kit\//; + +/** + * The one answer to "is this production source": a TypeScript file under root `src/` or a + * package's `src/` that is not test support. Layering scans, the eager-closure walker, R11's + * test-only import ban and R13 all call it; the root `test/` tree and `scripts/` are not + * production sources, so tooling and vitest setup there may import test support. + */ export function isProductionSourceFile(file: string): boolean { - return file.endsWith('.ts') && !/(?:^|\/)__tests__\//.test(file) && !/\.test\.ts$/.test(file); + return file.endsWith('.ts') && PRODUCTION_SOURCE_ROOT.test(file) && !TEST_SUPPORT_PATH.test(file); +} + +/** Every tracked file under `packages/`, repo-root-relative. */ +export function listTrackedPackageFiles(repoRoot: string): string[] { + return listTrackedFiles(repoRoot, ['packages']); } /** The canonical layering scan input: tracked, production, repo-root-relative. */ From 0cb9ab0a14049906677660d94c2f6cb5448331b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 15 Oct 2026 17:59:36 +0200 Subject: [PATCH 3/3] refactor(layering): use the shared production predicate for the runner test-host rule Co-Authored-By: Claude Opus 5.5 --- scripts/layering/platform-package-policy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/layering/platform-package-policy.ts b/scripts/layering/platform-package-policy.ts index cbce761e5c..1455280505 100644 --- a/scripts/layering/platform-package-policy.ts +++ b/scripts/layering/platform-package-policy.ts @@ -257,7 +257,7 @@ function checkSource(file: string, source: string): LayeringViolation[] { ), ); } - if (resolvesToAppleRunnerTestHost(file, site.spec) && isProductionSource(file)) { + if (resolvesToAppleRunnerTestHost(file, site.spec) && isProductionSourceFile(file)) { violations.push( violation( file,