Skip to content

Commit 4fffafe

Browse files
thymikeeclaude
thymikee
and
claude
committed
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 <[email protected]>
1 parent a907a91 commit 4fffafe

9 files changed

Lines changed: 755 additions & 250 deletions

‎scripts/layering/check.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@
6060
// static edge reads up the daemon-local order core < resources < execution < sessions <
6161
// server (R81). The global spine
6262
// ranks all five layers at daemon-server's rank, so R5/R6/R80 are blind inside the zone.
63+
// - Over WORKSPACE PACKAGE EXPORTS: no package but test-kit publishes test support (fixtures,
64+
// fakes, mocks, harnesses, `__tests__/`, `./testing/*`); a shrink-only burn-down list holds
65+
// today's offenders, and production code may not import them or test-kit (R83).
6366
// - Over CONTRACTS PRODUCTION SOURCE: contracts owns vocabulary only — host, process, and timer
6467
// mechanics belong in capture-kit or an adapter (R18).
6568
// R6, R9, and the R10 R7 counts are ratchets with no written-down reference: each is the same
@@ -109,6 +112,7 @@ import {
109112
} from './daemon-modularity.ts';
110113
import {
111114
checkPackageBoundaries,
115+
readTrackedPackageManifests,
112116
packageBoundariesSummary,
113117
workspaceSpecifierTargets,
114118
} from './package-boundaries.ts';
@@ -134,6 +138,8 @@ import { checkDaemonPlatformBoundary } from './daemon-platform-boundary.ts';
134138
import { checkDaemonPlatformRuntimeInventory } from './daemon-platform-runtime-inventory.ts';
135139
import { checkDaemonClientEntry } from './daemon-client-entry.ts';
136140
import { checkDaemonProcessReach } from './daemon-process-reach.ts';
141+
import { checkPackageExportSurface } from './package-export-surface.ts';
142+
import { TEST_ONLY_EXPORT_BURN_DOWN } from './package-export-surface-burn-down.ts';
137143
import { checkDaemonLayers, DAEMON_ROOT } from './daemon-layers.ts';
138144
import { checkSessionAuthorityOverlay, handlerOwnedOverlay } from './session-authority-overlay.ts';
139145
import {
@@ -435,7 +441,8 @@ function report(
435441
`concrete platform imports in every executable and type-only form; ` +
436442
`no production daemon file reaches the root platform-runtime family or a hub that ` +
437443
`does (R76); the daemon process reaches no caller-side replay source module over ` +
438-
`static or dynamic edges (R82); and the handler-owned SessionState/SessionStore ` +
444+
`static or dynamic edges (R82); package exports publish no new test support, with ` +
445+
`${TEST_ONLY_EXPORT_BURN_DOWN.length} burn-down entries left (R83); and the handler-owned SessionState/SessionStore ` +
439446
`authority overlay holds at or under the merge-base (R75, ` +
440447
`${handlerOwnedShapeFiles} shape / ${handlerOwnedAuthorityFiles} authority files); and ` +
441448
`every production daemon file lives under a layer folder with no upward static edges ` +
@@ -522,6 +529,7 @@ export const LAYERING_RULE_IDS = [
522529
'daemon-client-entry',
523530
'session-authority-overlay',
524531
'daemon-layers',
532+
'package-export-surface',
525533
] as const;
526534

527535
export type LayeringRuleId = (typeof LAYERING_RULE_IDS)[number];
@@ -595,6 +603,7 @@ export const LAYERING_RULES: Readonly<Record<LayeringRuleId, LayeringRule>> = {
595603
context.sourceFiles.filter((file) => file.startsWith(DAEMON_SOURCE_ROOT)),
596604
context.edges,
597605
),
606+
'package-export-surface': () => checkPackageExportSurface(readTrackedPackageManifests(repoRoot)),
598607
};
599608

600609
export function main(): number {

‎scripts/layering/package-boundaries-facade.test.ts‎

Lines changed: 4 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { test } from 'node:test';
99
import {
1010
checkPackageInternalSites,
1111
checkRootSites,
12-
isTestConsumerFile,
1312
specifierSites,
1413
workspacePackagesFromManifests,
1514
} from './package-boundaries.ts';
@@ -46,7 +45,7 @@ test('a production root file importing the ./testing/ namespace is refused', ()
4645
true,
4746
);
4847
assert.equal(violations.length, 1);
49-
assert.match(violations[0]!.message, /publishes test data to test files only/);
48+
assert.match(violations[0]!.message, /test-only export/);
5049
});
5150

5251
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'
105104
});
106105

107106
test('a package-local test/ tree is a test consumer, its src/ tree is not', () => {
108-
// The file vocabulary TEST_ONLY_FIXTURE_SUBPATHS' predicate uses: a package's
107+
// The file vocabulary R83's consumer check uses: a package's
109108
// own harness tree (`packages/<pkg>/test/`) imports fixtures just like the
110109
// root `test/` tree does; the same file under src/ does not (#3546 review).
111110
const consumer = workspacePackagesFromManifests(
@@ -133,42 +132,12 @@ test('a package-local test/ tree is a test consumer, its src/ tree is not', () =
133132
);
134133
const denied = checkPackageInternalSites(
135134
consumer,
136-
sites('packages/daemon-side/src/harness.ts', fixture),
135+
sites('packages/daemon-side/src/inventory.ts', fixture),
137136
[daemon, consumer],
138137
true,
139138
);
140139
assert.equal(denied.length, 1);
141-
assert.match(denied[0]!.message, /publishes test data to test files only/);
142-
});
143-
144-
test('isTestConsumerFile accepts the test vocabulary and denies production files', () => {
145-
// One spelling serves both test-data gates (the ./testing/ façade check and the
146-
// TEST_ONLY_FIXTURE_SUBPATHS predicate), so the vocabulary itself is pinned here:
147-
// every accepted shape forwards to both gates, and the shapes that once spelled
148-
// differently across the two copies (.test.tsx, .mts, a root __tests__/ tree) are
149-
// in the accepted set (#3546 review).
150-
for (const file of [
151-
'test/integration/consumer.ts',
152-
'packages/daemon-side/test/harness.ts',
153-
'src/daemon/__tests__/consumer.ts',
154-
'__tests__/root-consumer.ts',
155-
'scripts/layering/daemon-layers.test.tsx',
156-
'src/daemon/session-store.test.mts',
157-
'src/foo.test.js',
158-
'src/foo.test.cjs',
159-
]) {
160-
assert.equal(isTestConsumerFile(file), true, file);
161-
}
162-
for (const file of [
163-
'src/commands/inventory.ts',
164-
'packages/kernel/src/errors.ts',
165-
'scripts/layering/package-boundaries.ts',
166-
'testbed/notes.ts',
167-
'docs/writing/tests.ts',
168-
'src/foo.ts',
169-
]) {
170-
assert.equal(isTestConsumerFile(file), false, file);
171-
}
140+
assert.match(denied[0]!.message, /test-only export/);
172141
});
173142

174143
test('a pattern entry matches deep specifiers, and an unnamed one is refused in scripts/', () => {

‎scripts/layering/package-boundaries.ts‎

Lines changed: 30 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@
2525
// and via its package specifier instantiates twice in one process — duplicate AppError, broken
2626
// instanceof), and, for `scripts/` files which sit outside the type graph by design, the
2727
// root-side unknown-package and exports-map checks that compiled files get from TS2307.
28-
// On top, `TEST_ONLY_FIXTURE_SUBPATHS` mirrors R13's Android fixture gate for the non-platform
29-
// packages: a production file importing one of those fixture subpaths is an R11 violation.
28+
// The consumer half of R83 (production code importing a test-only export or test-kit) is checked at
29+
// the same import sites, so the one specifier walk serves both rules.
3030

3131
import fs from 'node:fs';
3232
import path from 'node:path';
3333
import { parseImports, resolveWorkspaceSpecifier } from './model.ts';
34+
import { PACKAGE_EXPORT_SURFACE_RULE, testOnlyImportMessage } from './package-export-surface.ts';
3435
import { listTrackedPackageManifests, listTrackedProductionSources } from './tracked-sources.ts';
3536

3637
export type PackageBoundaryViolation = {
@@ -102,13 +103,16 @@ export function specifierSites(file: string, source: string): SpecifierSite[] {
102103
* filtering the output.
103104
*/
104105
export function readWorkspacePackages(repoRoot: string): WorkspacePackage[] {
105-
return workspacePackagesFromManifests(
106-
new Map(
107-
listTrackedPackageManifests(repoRoot).map((manifestFile) => [
108-
manifestFile,
109-
fs.readFileSync(path.join(repoRoot, manifestFile), 'utf8'),
110-
]),
111-
),
106+
return workspacePackagesFromManifests(readTrackedPackageManifests(repoRoot));
107+
}
108+
109+
/** Tracked workspace package manifests, text keyed by repo-relative path. */
110+
export function readTrackedPackageManifests(repoRoot: string): Map<string, string> {
111+
return new Map(
112+
listTrackedPackageManifests(repoRoot).map((manifestFile) => [
113+
manifestFile,
114+
fs.readFileSync(path.join(repoRoot, manifestFile), 'utf8'),
115+
]),
112116
);
113117
}
114118

@@ -173,45 +177,26 @@ function namedByExports(target: WorkspacePackage, specifier: string): boolean {
173177
return resolveWorkspaceSpecifier(specifier, target.exportTargets) !== undefined;
174178
}
175179

176-
const TEST_CONSUMER_RE = /(^|\/)__tests__\/|\.test\.[cm]?[jt]sx?$|(?:^|\/)test\//;
177-
178-
/**
179-
* The one file vocabulary for "this file consumes test data": the root `test/` tree, a
180-
* package-local `test/` harness tree, `*.test.*` files, and `__tests__/` trees (root or
181-
* nested). Both test-data gates call this — the `./testing/` façade check and the
182-
* TEST_ONLY_FIXTURE_SUBPATHS predicate — so what counts as a test consumer is one rule,
183-
* not two spellings that drift. Pinned by an accept/deny table in
184-
* `package-boundaries-facade.test.ts`.
185-
*/
186-
export function isTestConsumerFile(file: string): boolean {
187-
return TEST_CONSUMER_RE.test(file);
188-
}
189-
190180
/**
191-
* Façade policy (ADR-0033 amendment): a package publishes test data only under its
192-
* `./testing/` namespace — named `./testing/<fixture>` entries and per-folder
193-
* `./testing/<dir>/*` patterns — and only test files may import it. The gate is the
194-
* namespace, not a pattern entry: narrowing the surface to exact keys must not
195-
* smuggle a fixture past the test-file rule. Production code that reaches a fixture
196-
* through the package boundary is production code depending on a test double — it
197-
* must inject the real port instead.
181+
* R83's consumer half (see package-export-surface.ts): a production file importing a test-only
182+
* export, or the private test-kit package, is production code depending on a test double — it
183+
* must use a production subpath or inject the real port.
198184
*/
199-
function checkTestDataOnlySites(
185+
function checkTestOnlyImportSite(
200186
site: SpecifierSite,
201187
target: WorkspacePackage,
202188
): PackageBoundaryViolation | undefined {
203-
if (!site.specifier.startsWith(`${target.name}/testing/`)) return undefined;
204-
if (!namedByExports(target, site.specifier)) return undefined;
205-
if (isTestConsumerFile(site.file)) return undefined;
206-
return {
207-
rule: 'R11 package-boundaries',
208-
file: site.file,
209-
line: site.line,
210-
message:
211-
`'${site.specifier}' resolves through ${target.dir}/package.json#exports './testing/' ` +
212-
`namespace, which publishes test data to test files only. '${site.file}' is not a test ` +
213-
`file: import a production subpath or inject the port.`,
214-
};
189+
const resolvedTarget = resolveWorkspaceSpecifier(site.specifier, target.exportTargets);
190+
const message = testOnlyImportMessage(
191+
site.file,
192+
site.specifier,
193+
target.name,
194+
resolvedTarget === undefined
195+
? undefined
196+
: { key: `.${site.specifier.slice(target.name.length)}`, targets: [resolvedTarget] },
197+
);
198+
if (message === undefined) return undefined;
199+
return { rule: PACKAGE_EXPORT_SURFACE_RULE, file: site.file, line: site.line, message };
215200
}
216201

217202
function specifierPackageName(specifier: string): string | undefined {
@@ -243,21 +228,6 @@ export function insideCompiledSources(file: string): boolean {
243228
return /^packages\/[^/]+\/src\//.test(file) || /^src\//.test(file) || /^test\//.test(file);
244229
}
245230

246-
/**
247-
* Workspace fixture subpaths only non-production files may import — the same claim R13's
248-
* `ANDROID_TEST_ONLY_SUBPATHS` makes for the platform packages, which R13 cannot express for
249-
* non-platform packages. A production file (root or package) importing one is a violation.
250-
*/
251-
export const TEST_ONLY_FIXTURE_SUBPATHS: ReadonlySet<string> = new Set([
252-
'@agent-device/device-selection/inventory-gateways-fixtures',
253-
'@agent-device/replay-port/replay-script-source-fixtures',
254-
]);
255-
256-
/** Consumer side of `TEST_ONLY_FIXTURE_SUBPATHS`. */
257-
function importsTestOnlyFixtureFromProduction(file: string, specifier: string): boolean {
258-
return TEST_ONLY_FIXTURE_SUBPATHS.has(specifier) && !isTestConsumerFile(file);
259-
}
260-
261231
/**
262232
* Rules for files INSIDE a package. An undeclared sibling import is R11's alone: pnpm links
263233
* every workspace package under the root `node_modules`, so the import resolves, compiles, and
@@ -306,16 +276,6 @@ export function checkPackageInternalSites(
306276
}
307277
const name = specifierPackageName(site.specifier);
308278
if (!name || !name.startsWith('@agent-device/')) continue;
309-
if (importsTestOnlyFixtureFromProduction(site.file, site.specifier)) {
310-
violations.push({
311-
rule: 'R11 package-boundaries',
312-
file: site.file,
313-
line: site.line,
314-
message:
315-
`'${site.specifier}' is a test-only fixture subpath — production code may not ` +
316-
`import it; use the package's production subpath instead.`,
317-
});
318-
}
319279
const target = packageByName(allPackages, name);
320280
if (!target) {
321281
if (!compiled) {
@@ -338,7 +298,7 @@ export function checkPackageInternalSites(
338298
`in ${pkg.dir}/package.json ${pkg.workspaceDependencyField}.`,
339299
});
340300
}
341-
const testDataOnly = checkTestDataOnlySites(site, target);
301+
const testDataOnly = checkTestOnlyImportSite(site, target);
342302
if (testDataOnly) {
343303
violations.push(testDataOnly);
344304
continue;
@@ -392,16 +352,6 @@ export function checkRootSites(
392352
}
393353
const name = specifierPackageName(site.specifier);
394354
if (!name || !name.startsWith('@agent-device/')) continue;
395-
if (importsTestOnlyFixtureFromProduction(site.file, site.specifier)) {
396-
violations.push({
397-
rule: 'R11 package-boundaries',
398-
file: site.file,
399-
line: site.line,
400-
message:
401-
`'${site.specifier}' is a test-only fixture subpath — production code may not ` +
402-
`import it; use the package's production subpath instead.`,
403-
});
404-
}
405355
const target = packageByName(packages, name);
406356
if (!target) {
407357
if (!compiled) {
@@ -424,7 +374,7 @@ export function checkRootSites(
424374
`package.json devDependencies.`,
425375
});
426376
}
427-
const testDataOnly = checkTestDataOnlySites(site, target);
377+
const testDataOnly = checkTestOnlyImportSite(site, target);
428378
if (testDataOnly) {
429379
violations.push(testDataOnly);
430380
continue;

0 commit comments

Comments
 (0)