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 c100c29bff..28c48b87c7 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/*`); 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 @@ -109,6 +112,7 @@ import { } from './daemon-modularity.ts'; import { checkPackageBoundaries, + readTrackedPackageManifests, packageBoundariesSummary, workspaceSpecifierTargets, } from './package-boundaries.ts'; @@ -134,12 +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, + 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'; @@ -435,7 +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); 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 ` + @@ -522,6 +533,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 +607,8 @@ export const LAYERING_RULES: Readonly> = { context.sourceFiles.filter((file) => file.startsWith(DAEMON_SOURCE_ROOT)), context.edges, ), + 'package-export-surface': (context) => + checkPackageExportSurface(context.ratchets.testOnlyExports, context.reference.testOnlyExports), }; export function main(): number { @@ -607,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-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..3e141e5926 100644 --- a/scripts/layering/package-boundaries.ts +++ b/scripts/layering/package-boundaries.ts @@ -26,12 +26,18 @@ // 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, + readExportManifests, + resolveAllConditionTargets, + testOnlyImportMessage, +} from './package-export-surface.ts'; import { listTrackedPackageManifests, listTrackedProductionSources } from './tracked-sources.ts'; export type PackageBoundaryViolation = { @@ -47,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. */ @@ -103,13 +111,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'), + ]), ); } @@ -137,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] ?? {}) @@ -152,6 +177,7 @@ export function workspacePackagesFromManifests( dir: `packages/${entry}`, name: manifest.name, exportTargets, + exportConditionTargets: conditionTargets, workspaceDependencies, workspaceDependencyField, externalDependencies, @@ -174,45 +200,28 @@ 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 resolvedTargets = resolveAllConditionTargets( + site.specifier, + target.exportConditionTargets ?? + new Map([...target.exportTargets].map(([key, file]) => [key, [file]])), + ); + const message = testOnlyImportMessage( + site.file, + site.specifier, + target.name, + resolvedTargets.length > 0 ? resolvedTargets : undefined, + ); + 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 +251,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 +299,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 +321,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 +375,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 +397,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.test.ts b/scripts/layering/package-export-surface.test.ts new file mode 100644 index 0000000000..edda04e9f0 --- /dev/null +++ b/scripts/layering/package-export-surface.test.ts @@ -0,0 +1,341 @@ +// R83 package-export-surface: the manifest scan (every shape of test support is caught, by key or +// 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 { test } from 'node:test'; +import { + checkPackageInternalSites, + checkRootSites, + specifierSites, + workspacePackagesFromManifests, +} from './package-boundaries.ts'; +import { + checkPackageExportSurface, + expandExportTarget, + isTestOnlyExport, + isTestOnlyExportSpecifier, + measureTestOnlyExports, + PACKAGE_EXPORT_SURFACE_RULE, + packageExportSurfaceSummary, +} from './package-export-surface.ts'; + +function manifest(name: string, exports: unknown): 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 }; +} + +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'], + ['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 ./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 *.spec.ts target', './thing', './src/thing.spec.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 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(measure('@agent-device/planted', { [key]: entry(target) }), [], key); + } +}); + +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' }, + }), + [], + ); +}); + +test('a string exports map and a missing one are both read', () => { + 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('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( + 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('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(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[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 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('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 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', () => { + 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); +}); + +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'), + './types-only': { types: './src/types-only.fixtures.ts', default: './src/types-only.ts' }, + './testing/*': entry('./src/__tests__/*.ts'), + './wild/*': entry('./src/wild/*.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', + '@agent-device/owner/types-only', +]; + +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('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( + 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, 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', + '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..f7455b49d7 --- /dev/null +++ b/scripts/layering/package-export-surface.ts @@ -0,0 +1,286 @@ +// 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 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. 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 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 { resolveWorkspaceSpecifier, type LayeringViolation } from './model.ts'; +import { isProductionSourceFile } from './tracked-sources.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); +} + +/** + * 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 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; + exports: readonly ExportEntry[]; +}; + +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: Object.entries(exported).map(([key, value]) => ({ + key, + targets: stringLeaves(value), + })), + }); + } + return parsed; +} + +/** + * 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 in one tree. `packageFiles` is that + * tree's tracked paths, so wildcard exports are judged by the files they publish. + */ +export function measureTestOnlyExports( + manifests: ReadonlyMap, + packageFiles: readonly string[], +): TestOnlyExport[] { + const offenders: TestOnlyExport[] = []; + for (const pkg of readExportManifests(manifests)) { + if (pkg.name === TEST_KIT_PACKAGE) continue; + 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 }); + } + } + 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 [...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, every + * condition target it resolves through. + */ +export function testOnlyImportMessage( + file: string, + specifier: string, + packageName: string, + resolvedTargets: readonly string[] | undefined, +): string | 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 (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.` + ); +} diff --git a/scripts/layering/platform-package-policy.test.ts b/scripts/layering/platform-package-policy.test.ts index 4d87a879b7..6b385fd4c2 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(); @@ -443,21 +443,14 @@ test('the Android test-only fixture subpaths accept test consumers and deny prod 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 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'/, - file, - ); + const production = validSources(); + production.set(file, fixtureImport); + assert.deepEqual(messages(production), [], file); } }); diff --git a/scripts/layering/platform-package-policy.ts b/scripts/layering/platform-package-policy.ts index 4dc5a256e0..1455280505 100644 --- a/scripts/layering/platform-package-policy.ts +++ b/scripts/layering/platform-package-policy.ts @@ -15,11 +15,12 @@ // 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'; 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( @@ -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)) { @@ -265,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, @@ -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/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', - ]); -}); 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. */