From 340a99f0155071df6821177137e5d27ffde61b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 28 Aug 2026 22:58:10 +0200 Subject: [PATCH] refactor(layering): centralize architecture ownership --- .../layering/architecture-ownership.test.ts | 88 +++++++++++++++ scripts/layering/architecture-ownership.ts | 101 ++++++++++++++++++ scripts/layering/daemon-modularity.ts | 55 ++-------- .../snapshot-presentation-boundary.test.ts | 16 ++- 4 files changed, 211 insertions(+), 49 deletions(-) create mode 100644 scripts/layering/architecture-ownership.test.ts create mode 100644 scripts/layering/architecture-ownership.ts diff --git a/scripts/layering/architecture-ownership.test.ts b/scripts/layering/architecture-ownership.test.ts new file mode 100644 index 0000000000..1bff1dd3ca --- /dev/null +++ b/scripts/layering/architecture-ownership.test.ts @@ -0,0 +1,88 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { test } from 'node:test'; +import { ARCHITECTURE_OWNERSHIP, matchesDeclaredRoot } from './architecture-ownership.ts'; +import { readDirectNamedExports } from './facade-exports.ts'; +import { resolveImportEdges } from './model.ts'; +import { workspaceSpecifierTargets } from './package-boundaries.ts'; +import { listTrackedTypeScriptFiles } from './tracked-sources.ts'; + +const repoRoot = path.resolve(import.meta.dirname, '../..'); + +function productionFile(file: string): boolean { + return !file.endsWith('.test.ts') && !file.includes('/__tests__/'); +} + +test('architecture ownership roots resolve to tracked owners', () => { + const tracked = new Set(listTrackedTypeScriptFiles(repoRoot)); + + for (const declaration of [ + ...ARCHITECTURE_OWNERSHIP.logicalModules, + ...ARCHITECTURE_OWNERSHIP.executablePolicies, + ]) { + for (const root of declaration.roots) { + assert.ok( + [...tracked].some((file) => matchesDeclaredRoot(file, root)), + `${declaration.name} has no tracked root: ${root}`, + ); + } + } + for (const declaration of ARCHITECTURE_OWNERSHIP.vocabulary) { + for (const root of declaration.roots) { + assert.ok(tracked.has(root), `${declaration.name} root is not tracked: ${root}`); + } + } + for (const declaration of ARCHITECTURE_OWNERSHIP.capabilities) { + assert.ok(tracked.has(declaration.root), `${declaration.name} root is not tracked`); + } +}); + +test('vocabulary roots are exported contract facades', () => { + const manifest = JSON.parse( + fs.readFileSync(path.join(repoRoot, 'packages/contracts/package.json'), 'utf8'), + ) as { exports: Record }; + const publicSources = new Set( + Object.values(manifest.exports).map(({ default: source }) => + path.posix.join('packages/contracts', source.slice(2)), + ), + ); + + for (const declaration of ARCHITECTURE_OWNERSHIP.vocabulary) { + for (const root of declaration.roots) { + assert.ok(publicSources.has(root), `${declaration.name} is not a public contract facade`); + } + } +}); + +test('capability roots enumerate current exports and have production consumers', () => { + const files = listTrackedTypeScriptFiles(repoRoot); + const sources = new Map( + files.map((file) => [file, fs.readFileSync(path.join(repoRoot, file), 'utf8')]), + ); + const edges = resolveImportEdges(sources, workspaceSpecifierTargets(repoRoot)); + + for (const declaration of ARCHITECTURE_OWNERSHIP.capabilities) { + assert.deepEqual( + readDirectNamedExports(sources.get(declaration.root)!), + declaration.exports, + `${declaration.name} capability exports drifted`, + ); + assert.ok( + edges.some((edge) => edge.target === declaration.root && productionFile(edge.file)), + `${declaration.name} has no production consumer`, + ); + } +}); + +test('lookalike paths and symbols remain undeclared', () => { + const client = ARCHITECTURE_OWNERSHIP.vocabulary.find(({ name }) => name === 'client-contract')!; + const capability = ARCHITECTURE_OWNERSHIP.capabilities.find( + ({ name }) => name === 'request-runtime-binding', + )!; + + assert.equal(matchesDeclaredRoot(`${client.roots[0]}-extra`, client.roots[0]), false); + assert.equal(matchesDeclaredRoot('src/snapshot-policy.ts', 'src/snapshot/'), false); + assert.equal(matchesDeclaredRoot(`${capability.root}.bak`, capability.root), false); + assert.equal(capability.exports.includes('createRequestRuntimeBindingsExtra'), false); +}); diff --git a/scripts/layering/architecture-ownership.ts b/scripts/layering/architecture-ownership.ts new file mode 100644 index 0000000000..1466da5e9b --- /dev/null +++ b/scripts/layering/architecture-ownership.ts @@ -0,0 +1,101 @@ +export type LogicalModulePolicy = Readonly<{ + name: string; + roots: readonly string[]; + forbiddenTargetRoots: readonly string[]; +}>; + +export const LOGICAL_MODULE_POLICIES = [ + { + name: 'ad-replay', + roots: ['packages/ad-replay/src/'], + forbiddenTargetRoots: ['src/daemon/', 'src/providers/', 'src/compat/', 'packages/maestro/'], + }, + { + name: 'maestro', + roots: ['packages/maestro/src/'], + forbiddenTargetRoots: ['src/daemon/', 'src/providers/', 'packages/ad-replay/'], + }, + { + name: 'replay-test', + roots: ['packages/replay-test/src/'], + forbiddenTargetRoots: [ + 'src/daemon/', + 'src/providers/', + 'src/request/', + 'src/replay/', + 'src/compat/', + 'packages/maestro/', + 'packages/ad-replay/', + ], + }, +] as const satisfies readonly LogicalModulePolicy[]; + +export const ARCHITECTURE_OWNERSHIP = { + logicalModules: LOGICAL_MODULE_POLICIES, + vocabulary: [ + { + name: 'client-contract', + kind: 'vocabulary', + roots: ['packages/contracts/src/facades/client.ts'], + }, + { + name: 'capture-contract', + kind: 'vocabulary', + roots: ['packages/contracts/src/facades/capture.ts'], + }, + { + name: 'replay-contract', + kind: 'vocabulary', + roots: ['packages/contracts/src/facades/replay.ts'], + }, + { + name: 'progress-contract', + kind: 'vocabulary', + roots: ['packages/contracts/src/facades/progress.ts'], + }, + ], + capabilities: [ + { + name: 'request-runtime-binding', + kind: 'capability', + root: 'src/daemon/request-runtime-binding.ts', + exports: [ + 'BindDeviceRuntime', + 'BindExactDeviceRuntime', + 'InspectDeviceRuntimeFacts', + 'RequestRuntimeBindings', + 'RuntimeAdmissionBindings', + 'createRequestRuntimeBindings', + ], + }, + { + name: 'session-script-publication', + kind: 'capability', + root: 'src/daemon/session-script-publication-capability.ts', + exports: [ + 'abortAuthoringOnSecondOpen', + 'applyRecordedSaveScriptFlags', + 'armAuthoringOnOpen', + 'effectiveWriteForce', + 'isAuthoringArmedSession', + 'isSessionRecording', + 'isSessionScriptPublished', + 'markActivePublicationDone', + 'markCloseGeneratedPublicationDone', + 'retargetActivePublication', + ], + }, + ], + executablePolicies: [ + { + name: 'snapshot-policy', + kind: 'executable-policy', + roots: ['src/snapshot/'], + forbiddenTargetRoots: ['src/daemon/'], + }, + ], +} as const; + +export function matchesDeclaredRoot(file: string, root: string): boolean { + return root.endsWith('/') ? file.startsWith(root) : file === root; +} diff --git a/scripts/layering/daemon-modularity.ts b/scripts/layering/daemon-modularity.ts index dde3550e25..10aa0d901a 100644 --- a/scripts/layering/daemon-modularity.ts +++ b/scripts/layering/daemon-modularity.ts @@ -1,4 +1,9 @@ import path from 'node:path'; +import { + LOGICAL_MODULE_POLICIES, + matchesDeclaredRoot, + type LogicalModulePolicy, +} from './architecture-ownership.ts'; import { targetDagZone, type LayeringViolation, type ResolvedImportEdge } from './model.ts'; import { SESSION_STATE_FIELD_OWNERS } from './session-state.ts'; @@ -29,47 +34,6 @@ export const TYPE_CYCLE_BASELINE = Object.values(LARGEST_TYPE_CYCLE_ZONE_CEILING 0, ); -type LogicalModulePolicy = { - name: string; - roots: readonly string[]; - forbiddenTargetRoots: readonly string[]; -}; - -/** - * Zero-count targets for the accepted daemon modularity design. A root may be absent today: - * the policy starts enforcing as soon as the first file is added, without scaffolding an empty - * façade or package merely to make the gate concrete. - */ -export const LOGICAL_MODULE_POLICIES: readonly LogicalModulePolicy[] = [ - { - name: 'ad-replay', - roots: ['packages/ad-replay/src/'], - forbiddenTargetRoots: ['src/daemon/', 'src/providers/', 'src/compat/', 'packages/maestro/'], - }, - { - name: 'maestro', - roots: ['packages/maestro/src/'], - forbiddenTargetRoots: ['src/daemon/', 'src/providers/', 'packages/ad-replay/'], - }, - { - // Replay-test schedules and reports; it must stay format-neutral. `src/request/` is - // request-global daemon plumbing (progress sinks, cancellation, AsyncLocalStorage), and the - // remaining roots are engine internals — reaching into either is how a scheduler quietly - // acquires daemon authority or an engine-specific value shape. - name: 'replay-test', - roots: ['packages/replay-test/src/'], - forbiddenTargetRoots: [ - 'src/daemon/', - 'src/providers/', - 'src/request/', - 'src/replay/', - 'src/compat/', - 'packages/maestro/', - 'packages/ad-replay/', - ], - }, -]; - const ENGINE_FILE_PREFIXES = [ 'packages/ad-replay/src/', 'packages/maestro/src/', @@ -231,8 +195,9 @@ function checkLogicalModuleImports(edges: readonly ResolvedImportEdge[]): Layeri if (!sourceModule) continue; // A module's own files are never a forbidden target: `replay-test` sits inside the wider // `src/replay/` engine root it may not import from. - if (sourceModule.roots.some((root) => edge.target.startsWith(root))) continue; - if (!sourceModule.forbiddenTargetRoots.some((root) => edge.target.startsWith(root))) continue; + if (sourceModule.roots.some((root) => matchesDeclaredRoot(edge.target, root))) continue; + if (!sourceModule.forbiddenTargetRoots.some((root) => matchesDeclaredRoot(edge.target, root))) + continue; violations.push({ rule: 'R10 daemon-modularity', file: edge.file, @@ -245,12 +210,12 @@ function checkLogicalModuleImports(edges: readonly ResolvedImportEdge[]): Layeri function moduleForFile(file: string): LogicalModulePolicy | undefined { return LOGICAL_MODULE_POLICIES.find((module) => - module.roots.some((root) => file.startsWith(root)), + module.roots.some((root) => matchesDeclaredRoot(file, root)), ); } function isInsideInternalTree(file: string, roots: readonly string[]): boolean { - return roots.some((root) => file.startsWith(path.posix.join(root, 'internal/'))); + return roots.some((root) => matchesDeclaredRoot(file, path.posix.join(root, 'internal/'))); } function groupBy( diff --git a/scripts/layering/snapshot-presentation-boundary.test.ts b/scripts/layering/snapshot-presentation-boundary.test.ts index e899ba017a..9f62dff59a 100644 --- a/scripts/layering/snapshot-presentation-boundary.test.ts +++ b/scripts/layering/snapshot-presentation-boundary.test.ts @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { test } from 'node:test'; +import { ARCHITECTURE_OWNERSHIP, matchesDeclaredRoot } from './architecture-ownership.ts'; import { resolveImportEdges } from './model.ts'; import { workspaceSpecifierTargets } from './package-boundaries.ts'; import { listTrackedTypeScriptFiles } from './tracked-sources.ts'; @@ -9,9 +10,9 @@ import { listTrackedTypeScriptFiles } from './tracked-sources.ts'; const repoRoot = path.resolve(import.meta.dirname, '../..'); /** - * The host-side snapshot facet (`src/snapshot/`) owns presentation, freshness, timeout and - * overlay policy; `src/daemon/` owns the assembly that orders them (#1983, ADR 0004). The - * dependency runs one way only, so a policy stays testable without standing up a session. + * The `snapshot-policy` declaration owns presentation, freshness, timeout and overlay policy; + * `src/daemon/` owns the assembly that orders them (#1983, ADR 0004). The dependency runs one way + * only, so a policy stays testable without standing up a session. * * The real tree is clean, which is exactly why the positive control below exists: a filter that * stopped matching would look identical to a boundary being obeyed. @@ -20,9 +21,16 @@ function daemonImportsFromSnapshotFacet( sources: ReadonlyMap, workspaceTargets?: ReadonlyMap, ): string[] { + const snapshotPolicy = ARCHITECTURE_OWNERSHIP.executablePolicies.find( + ({ name }) => name === 'snapshot-policy', + ); + if (!snapshotPolicy) throw new Error('snapshot-policy ownership declaration is missing'); + return resolveImportEdges(sources, workspaceTargets) .filter( - (edge) => edge.file.startsWith('src/snapshot/') && edge.target.startsWith('src/daemon/'), + (edge) => + snapshotPolicy.roots.some((root) => matchesDeclaredRoot(edge.file, root)) && + snapshotPolicy.forbiddenTargetRoots.some((root) => matchesDeclaredRoot(edge.target, root)), ) .map((edge) => `${edge.file}:${edge.line} -> ${edge.target}`); }