diff --git a/packages/daemon/package.json b/packages/daemon/package.json index 99b5ac44d0..50c486eace 100644 --- a/packages/daemon/package.json +++ b/packages/daemon/package.json @@ -22,10 +22,10 @@ "@agent-device/replay-port": "workspace:*", "@agent-device/replay-test": "workspace:*", "@agent-device/selectors": "workspace:*", - "@agent-device/session-journal": "workspace:*", - "@agent-device/test-kit": "workspace:*" + "@agent-device/session-journal": "workspace:*" }, "devDependencies": { + "@agent-device/test-kit": "workspace:*", "fast-check": "^4.9.0", "oxc-parser": "^0.146.0", "vitest": "5.0.4", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1e619bb7a1..aadb93b9c0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -343,10 +343,10 @@ importers: '@agent-device/session-journal': specifier: workspace:* version: link:../session-journal + devDependencies: '@agent-device/test-kit': specifier: workspace:* version: link:../test-kit - devDependencies: fast-check: specifier: ^4.9.0 version: 4.9.0 diff --git a/scripts/layering/package-boundaries-dev-dependencies.test.ts b/scripts/layering/package-boundaries-dev-dependencies.test.ts new file mode 100644 index 0000000000..a76d3f2e19 --- /dev/null +++ b/scripts/layering/package-boundaries-dev-dependencies.test.ts @@ -0,0 +1,90 @@ +// R11 and devDependencies: a private package declares siblings in `dependencies`, and only test +// support (tests, `*.fixtures.*` files, test trees) may also resolve through its `devDependencies`, +// so a test-only sibling such as test-kit never becomes a runtime dependency. Production source +// still needs the production field. Compact sibling of package-boundaries.test.ts, which sits at +// its size ratchet. + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { + checkPackageInternalSites, + specifierSites, + workspacePackagesFromManifests, +} from './package-boundaries.ts'; + +function manifests(owner: Record) { + return new Map([ + [ + 'packages/helper/package.json', + JSON.stringify({ + name: '@agent-device/helper', + private: true, + exports: { '.': './src/index.ts' }, + }), + ], + [ + 'packages/owner/package.json', + JSON.stringify({ name: '@agent-device/owner', exports: { '.': './src/index.ts' }, ...owner }), + ], + ]); +} + +function violations(owner: Record, file: string) { + const packages = workspacePackagesFromManifests(manifests(owner)); + const pkg = packages.find(({ name }) => name === '@agent-device/owner')!; + return checkPackageInternalSites( + pkg, + specifierSites(file, "import { helper } from '@agent-device/helper';"), + packages, + true, + ); +} + +const DEV_ONLY = { private: true, devDependencies: { '@agent-device/helper': 'workspace:*' } }; + +test('a private package reads its devDependencies as test-only siblings', () => { + const [, owner] = workspacePackagesFromManifests(manifests(DEV_ONLY)); + assert.deepEqual([...owner!.workspaceDependencies], []); + assert.deepEqual([...owner!.workspaceDevDependencies], ['@agent-device/helper']); +}); + +test('test support may import a sibling declared only in devDependencies', () => { + for (const file of [ + 'packages/owner/src/thing.test.ts', + 'packages/owner/src/thing.spec.ts', + 'packages/owner/src/__tests__/thing.ts', + 'packages/owner/src/thing.fixtures.ts', + 'packages/owner/test/harness.ts', + ]) { + assert.deepEqual(violations(DEV_ONLY, file), [], file); + } +}); + +test('production source needs the production field and is told so', () => { + const found = violations(DEV_ONLY, 'packages/owner/src/thing.ts'); + assert.equal(found.length, 1); + assert.match(found[0]!.message, /declares "@agent-device\/helper" only in devDependencies/); + assert.match(found[0]!.message, /declare it in dependencies/); +}); + +test('an undeclared sibling is refused for tests and production alike', () => { + for (const file of ['packages/owner/src/thing.test.ts', 'packages/owner/src/thing.ts']) { + const found = violations({ private: true }, file); + assert.equal(found.length, 1, file); + assert.match(found[0]!.message, /without declaring "@agent-device\/helper"/); + } +}); + +test('a production declaration serves tests and production', () => { + const declared = { private: true, dependencies: { '@agent-device/helper': 'workspace:*' } }; + for (const file of ['packages/owner/src/thing.test.ts', 'packages/owner/src/thing.ts']) { + assert.deepEqual(violations(declared, file), [], file); + } +}); + +test('a published package keeps its siblings in devDependencies for every file', () => { + const published = { devDependencies: { '@agent-device/helper': 'workspace:*' } }; + for (const file of ['packages/owner/src/thing.test.ts', 'packages/owner/src/thing.ts']) { + assert.deepEqual(violations(published, file), [], file); + } +}); diff --git a/scripts/layering/package-boundaries.test.ts b/scripts/layering/package-boundaries.test.ts index 1b45f17809..518dd3abf8 100644 --- a/scripts/layering/package-boundaries.test.ts +++ b/scripts/layering/package-boundaries.test.ts @@ -34,6 +34,7 @@ const kernel: WorkspacePackage = { ['@agent-device/kernel/device', 'packages/kernel/src/device.ts'], ]), workspaceDependencies: new Set(), + workspaceDevDependencies: new Set(), workspaceDependencyField: 'dependencies', externalDependencies: new Map(), }; @@ -45,6 +46,7 @@ const contracts: WorkspacePackage = { ['@agent-device/contracts/interaction', 'packages/contracts/src/interaction.ts'], ]), workspaceDependencies: new Set(['@agent-device/kernel']), + workspaceDevDependencies: new Set(), workspaceDependencyField: 'dependencies', externalDependencies: new Map(), }; @@ -760,15 +762,9 @@ test('the real tree parses, declares, and passes R11', () => { // daemon used to own (#2340). The per-file subpaths under `./interaction-*`, // `./selector-pipeline*`, `./press-retarget`, `./touch-semantics` are the // execution surface the core selector pipeline moved into this package — - // one subpath per module so consumers pull only the stage they run; the - // `-fixtures` entries are the test-fixture surface (host-kit's - // `./audio-probe-fixtures` precedent) — `./interaction-targeting-fixtures` - // for the interaction-targeting node trees and `./snapshot-geometry-fixtures` - // for the geometry/touch-point builders both this package's and root's - // tests build on (#2402, replacing the copy that used to live under root's - // `src/__tests__/test-utils/`; it re-exports `makeSnapshotState` from - // capture-kit's own `./snapshot-state-fixtures`, its canonical home). Any - // other subpath, or the AST leaking into `.`, fails here. + // one subpath per module so consumers pull only the stage they run. The `-fixtures` + // entries are R83 burn-down, deleted with the export. Any other subpath, or the AST leaking + // into `.`, fails here. assert.deepEqual([...selectorsPackage.exportTargets.keys()].sort(), [ '@agent-device/selectors', '@agent-device/selectors/absence-observation', diff --git a/scripts/layering/package-boundaries.ts b/scripts/layering/package-boundaries.ts index 3e141e5926..ab88b1aa54 100644 --- a/scripts/layering/package-boundaries.ts +++ b/scripts/layering/package-boundaries.ts @@ -38,7 +38,11 @@ import { resolveAllConditionTargets, testOnlyImportMessage, } from './package-export-surface.ts'; -import { listTrackedPackageManifests, listTrackedProductionSources } from './tracked-sources.ts'; +import { + isProductionSourceFile, + listTrackedPackageManifests, + listTrackedProductionSources, +} from './tracked-sources.ts'; export type PackageBoundaryViolation = { rule: string; @@ -57,6 +61,13 @@ export type WorkspacePackage = { exportConditionTargets?: ReadonlyMap; /** Declared `workspace:*` dependencies on sibling internal packages. */ workspaceDependencies: ReadonlySet; + /** + * Workspace siblings a private package declares in `devDependencies`. Only test support (tests, + * fixtures, test trees) may import through them; production source still needs + * `workspaceDependencyField`. A published package keeps every sibling in `devDependencies` + * already, so this is empty there. + */ + workspaceDevDependencies: ReadonlySet; /** Where siblings are declared: a published package bundles them, so it lists them as dev-only. */ workspaceDependencyField: 'dependencies' | 'devDependencies'; /** Non-workspace dependencies that the root build must externalize. */ @@ -168,6 +179,13 @@ export function workspacePackagesFromManifests( .filter(([, range]) => range.startsWith('workspace:')) .map(([name]) => name), ); + const workspaceDevDependencies = new Set( + manifest.private === true + ? Object.entries(manifest.devDependencies ?? {}) + .filter(([, range]) => range.startsWith('workspace:')) + .map(([name]) => name) + : [], + ); const externalDependencies = new Map( Object.entries(manifest.dependencies ?? {}).filter( ([, range]) => !range.startsWith('workspace:'), @@ -179,6 +197,7 @@ export function workspacePackagesFromManifests( exportTargets, exportConditionTargets: conditionTargets, workspaceDependencies, + workspaceDevDependencies, workspaceDependencyField, externalDependencies, }); @@ -251,6 +270,29 @@ export function insideCompiledSources(file: string): boolean { return /^packages\/[^/]+\/src\//.test(file) || /^src\//.test(file) || /^test\//.test(file); } +/** + * A sibling import must be declared `workspace:*` in the package manifest. Production source needs + * the production field (`dependencies`, or `devDependencies` for a published package, which + * bundles its siblings); test support — tests, `*.fixtures.*` files, test trees, test-kit — may + * also resolve through a private package's `devDependencies`, so a test-only sibling such as + * test-kit never ships as a runtime dependency. + */ +function undeclaredSiblingMessage( + pkg: WorkspacePackage, + name: string, + site: SpecifierSite, +): string | undefined { + if (name === pkg.name || pkg.workspaceDependencies.has(name)) return undefined; + const declaredForTests = pkg.workspaceDevDependencies.has(name); + if (declaredForTests && !isProductionSourceFile(site.file)) return undefined; + return declaredForTests + ? `${pkg.name} imports '${site.specifier}' from production source, but ${pkg.dir}/package.json ` + + `declares "${name}" only in devDependencies: declare it in ${pkg.workspaceDependencyField} ` + + `or import it from test support only.` + : `${pkg.name} imports '${site.specifier}' without declaring "${name}": "workspace:*" ` + + `in ${pkg.dir}/package.json ${pkg.workspaceDependencyField}.`; +} + /** * 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 @@ -311,14 +353,13 @@ export function checkPackageInternalSites( } continue; } - if (name !== pkg.name && !pkg.workspaceDependencies.has(name)) { + const undeclared = undeclaredSiblingMessage(pkg, name, site); + if (undeclared) { violations.push({ rule: 'R11 package-boundaries', file: site.file, line: site.line, - message: - `${pkg.name} imports '${site.specifier}' without declaring "${name}": "workspace:*" ` + - `in ${pkg.dir}/package.json ${pkg.workspaceDependencyField}.`, + message: undeclared, }); } const testDataOnly = checkTestOnlyImportSite(site, target); diff --git a/scripts/layering/project-references.test.ts b/scripts/layering/project-references.test.ts index 9f18256ec0..c4eda780c6 100644 --- a/scripts/layering/project-references.test.ts +++ b/scripts/layering/project-references.test.ts @@ -151,12 +151,14 @@ test('every package tsconfig references exactly its manifest workspace dependenc `${pkg.dir}/tsconfig.json references ${dir}, which is not a workspace package`, ); } + const declared = new Set([...pkg.workspaceDependencies, ...pkg.workspaceDevDependencies]); assert.deepEqual( referencedDirs.map((dir) => nameByDir.get(dir)), - [...pkg.workspaceDependencies].filter((name) => name !== rootPackageName).sort(), - `${pkg.dir}/tsconfig.json references must equal its manifest workspace:* dependencies`, + [...declared].filter((name) => name !== rootPackageName).sort(), + `${pkg.dir}/tsconfig.json references must equal its manifest workspace:* dependencies and ` + + `devDependencies`, ); - if (pkg.workspaceDependencies.has(rootPackageName)) { + if (declared.has(rootPackageName)) { const config = readTsconfig(`${pkg.dir}/tsconfig.json`); const paths = (config.compilerOptions?.paths ?? {}) as Record; assert.ok(