From 8c052bdec583ea46be1259bfc2fe40fc010a0085 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 08:37:04 +0800 Subject: [PATCH] test(architecture): ratchet top-level layer direction and mutual imports (#10584) * test(architecture): ratchet top-level layer direction and mutual imports Declare a layer for every top-level src/ directory and root source file (domain, contracts, foundation, providers, application, composition, adapters) in scripts/lib/module-layers.mjs. verify:module-boundaries now also builds the production import graph between those units and compares it with a counted baseline: 45 unit pairs import a higher layer (248 imports) and 23 unit pairs import each other. A new upward pair, more imports on a recorded pair, a new mutual import, or an undeclared unit fails; shrinkage is reported so the baseline can be lowered. * fix(architecture): enforce layers for isolated units --- scripts/lib/module-boundaries.d.mts | 21 +++ scripts/lib/module-boundaries.mjs | 30 +++- scripts/lib/module-layers.baseline.json | 74 +++++++++ scripts/lib/module-layers.d.mts | 21 +++ scripts/lib/module-layers.mjs | 205 ++++++++++++++++++++++++ scripts/verify-module-boundaries.mjs | 39 ++++- tests/evaluation/moduleLayers.test.ts | 90 +++++++++++ turbo.json | 1 + 8 files changed, 472 insertions(+), 9 deletions(-) create mode 100644 scripts/lib/module-layers.baseline.json create mode 100644 scripts/lib/module-layers.d.mts create mode 100644 scripts/lib/module-layers.mjs create mode 100644 tests/evaluation/moduleLayers.test.ts diff --git a/scripts/lib/module-boundaries.d.mts b/scripts/lib/module-boundaries.d.mts index e8b6d2dd67..285ecf4ef6 100644 --- a/scripts/lib/module-boundaries.d.mts +++ b/scripts/lib/module-boundaries.d.mts @@ -19,3 +19,24 @@ export function inspectModuleBoundaries( source: string, root: string, ): readonly ModuleBoundaryViolation[]; + +/** One own-source import, resolved to its repository path. */ +export interface ResolvedModuleImport { + readonly file: string; + readonly line: number; + readonly specifier: string; + readonly typeOnly: boolean; + readonly target: string; +} + +/** Each own-source import of a file, resolved to its repository path. */ +export function resolveModuleImports( + file: string, + source: string, + root: string, +): readonly ResolvedModuleImport[]; + +/** Check already resolved imports, as returned by resolveModuleImports. */ +export function inspectResolvedModuleBoundaries( + imports: readonly ResolvedModuleImport[], +): readonly ModuleBoundaryViolation[]; diff --git a/scripts/lib/module-boundaries.mjs b/scripts/lib/module-boundaries.mjs index 3f8110436f..dbd4ffd2c6 100644 --- a/scripts/lib/module-boundaries.mjs +++ b/scripts/lib/module-boundaries.mjs @@ -148,18 +148,36 @@ const failedBoundary = (file, target, typeOnly) => { return undefined; }; -/** Check resolved source ownership, including type imports and reexports. */ -export const inspectModuleBoundaries = (file, source, root) => { +/** Each own-source import of a file, resolved to its repository path. */ +export const resolveModuleImports = (file, source, root) => { const absoluteFile = resolve(root, file); const importer = repositoryPath(root, absoluteFile); - const violations = []; + const imports = []; for (const { specifier, line, typeOnly } of readImportSpecifiers(source)) { const resolved = sourceTarget(root, absoluteFile, specifier); if (resolved === undefined) continue; - const target = repositoryPath(root, resolved); - const boundary = failedBoundary(importer, target, typeOnly); + imports.push({ + file: importer, + line, + specifier, + typeOnly, + target: repositoryPath(root, resolved), + }); + } + return imports; +}; + +/** Check resolved source ownership, including type imports and reexports. */ +export const inspectModuleBoundaries = (file, source, root) => + inspectResolvedModuleBoundaries(resolveModuleImports(file, source, root)); + +/** Check already resolved imports, as returned by resolveModuleImports. */ +export const inspectResolvedModuleBoundaries = (imports) => { + const violations = []; + for (const { file, line, specifier, typeOnly, target } of imports) { + const boundary = failedBoundary(file, target, typeOnly); if (boundary !== undefined) - violations.push({ file: importer, line, specifier, target, boundary }); + violations.push({ file, line, specifier, target, boundary }); } return violations; }; diff --git a/scripts/lib/module-layers.baseline.json b/scripts/lib/module-layers.baseline.json new file mode 100644 index 0000000000..ada02158aa --- /dev/null +++ b/scripts/lib/module-layers.baseline.json @@ -0,0 +1,74 @@ +{ + "upward": { + "adb -> application": 4, + "android -> application": 10, + "apkSignature -> application": 3, + "apktool -> application": 3, + "application -> catalogDiscovery.ts": 1, + "application -> catalogIdentity.ts": 2, + "application -> mcpStartupPolicy.ts": 3, + "application -> serverIdentity.ts": 1, + "artifacts -> application": 8, + "browser -> application": 33, + "captures -> application": 2, + "composition -> cliCommandNames.ts": 3, + "config -> application": 2, + "config -> native": 1, + "desktop -> application": 5, + "domain -> contracts": 4, + "dotnet -> application": 4, + "electron -> application": 2, + "erlang -> application": 2, + "evm -> application": 2, + "firmware -> application": 4, + "flutter -> application": 1, + "frida -> application": 4, + "ghidra -> application": 17, + "go -> application": 2, + "gpu -> application": 2, + "hopper -> application": 13, + "ida -> application": 8, + "il2cpp -> application": 5, + "inspector -> application": 3, + "javascript -> application": 6, + "jeb -> application": 4, + "kuna -> application": 7, + "model -> application": 2, + "native -> application": 35, + "pcap -> application": 2, + "process -> artifacts": 20, + "providers -> application": 4, + "python -> application": 2, + "rust -> application": 2, + "signals -> application": 2, + "sqlite -> application": 2, + "v8 -> application": 2, + "wasm -> application": 2, + "windows -> artifacts": 2 + }, + "cycles": [ + "application <-> artifacts", + "application <-> browser", + "application <-> config", + "application <-> desktop", + "application <-> dotnet", + "application <-> frida", + "application <-> providers", + "artifacts <-> dotnet", + "artifacts <-> firmware", + "artifacts <-> gpu", + "artifacts <-> javascript", + "artifacts <-> native", + "artifacts <-> process", + "catalogIdentity.ts <-> recordedCatalogDigests.ts", + "cli <-> cli.ts", + "cli <-> cliBrowserScenarioCommands.ts", + "cli <-> cliEvidenceCommands.ts", + "cli <-> cliObservationOptions.ts", + "cli <-> cliOutput.ts", + "config <-> filesystem", + "contracts <-> domain", + "main <-> server", + "process <-> windows" + ] +} diff --git a/scripts/lib/module-layers.d.mts b/scripts/lib/module-layers.d.mts new file mode 100644 index 0000000000..242f755b9a --- /dev/null +++ b/scripts/lib/module-layers.d.mts @@ -0,0 +1,21 @@ +export declare const LAYER_ORDER: readonly string[]; +export declare const UNIT_LAYERS: Readonly>; + +/** The top-level unit of a production `src/` file, or undefined. */ +export function sourceUnit(path: string): string | undefined; + +export interface LayerGraphFacts { + readonly undeclared: readonly string[]; + readonly upward: Readonly>; + readonly cycles: readonly string[]; +} + +export function layerGraphFacts( + imports: readonly { readonly file: string; readonly target: string }[], + files?: readonly string[], +): LayerGraphFacts; + +export function compareLayerBaseline( + facts: LayerGraphFacts, + baseline: Pick, +): { readonly failures: string[]; readonly lowerable: string[] }; diff --git a/scripts/lib/module-layers.mjs b/scripts/lib/module-layers.mjs new file mode 100644 index 0000000000..f4c49b9bad --- /dev/null +++ b/scripts/lib/module-layers.mjs @@ -0,0 +1,205 @@ +/** + * Declared layer of every top-level `src/` unit: each directory, and each + * root-level source file. Dependencies may point to the same or a lower + * layer. An undeclared unit fails the check, so a new directory must choose + * its layer when it is added. + * + * The order follows docs/architecture/architecture.mermaid: domain and + * contracts at the centre, the process/filesystem foundation, provider + * adapters, shared application workflows, production composition, then the + * CLI and MCP entry points. + */ +export const LAYER_ORDER = Object.freeze([ + "domain", + "contracts", + "foundation", + "providers", + "application", + "composition", + "adapters", +]); + +const units = (layer, names) => names.map((name) => [name, layer]); + +export const UNIT_LAYERS = Object.freeze( + Object.fromEntries([ + ...units("domain", ["domain"]), + ...units("contracts", ["contracts"]), + ...units("foundation", [ + "compression", + "config", + "filesystem", + "process", + "windows", + "generatedPackageMetadata.ts", + "identity.ts", + "logger.ts", + ]), + ...units("providers", [ + "adb", + "android", + "apkSignature", + "apktool", + "artifacts", + "browser", + "captures", + "cutter", + "desktop", + "dotnet", + "electron", + "erlang", + "evm", + "firmware", + "flutter", + "frida", + "gdb", + "ghidra", + "go", + "gpu", + "hopper", + "ida", + "il2cpp", + "inspector", + "javascript", + "jeb", + "kuna", + "model", + "native", + "objdump", + "pcap", + "providers", + "python", + "reference", + "rizin", + "rust", + "signals", + "sqlite", + "v8", + "wasm", + ]), + ...units("application", ["application"]), + ...units("composition", ["composition"]), + ...units("adapters", [ + "cli", + "main", + "server", + "catalogDiscovery.ts", + "catalogIdentity.ts", + "cli.ts", + "cliBrowserScenarioCommands.ts", + "cliBuiltinArguments.ts", + "cliCommandNames.ts", + "cliConfiguration.ts", + "cliEvidenceCommands.ts", + "cliJsonInput.ts", + "cliLogging.ts", + "cliObservationOptions.ts", + "cliOneShot.ts", + "cliOutput.ts", + "doctorRuntime.ts", + "main.ts", + "mcpDoctor.ts", + "mcpDoctorOptions.ts", + "mcpStartupPolicy.ts", + "recordedCatalogDigests.ts", + "recordedCliCommandRegistrations.ts", + "recordedMcpCatalog.ts", + "serverIdentity.ts", + ]), + ]), +); + +const PRODUCTION_SOURCE = /^src\/.+\.(?:ts|js)$/u; +const NON_PRODUCTION = /\.(?:test|spec|fixture)\.(?:ts|js)$|\.d\.ts$/u; + +/** The top-level unit of a production `src/` file, or undefined. */ +export const sourceUnit = (path) => { + if (!PRODUCTION_SOURCE.test(path) || NON_PRODUCTION.test(path)) + return undefined; + // A root-level source file is its own unit; built .js specifiers name .ts files. + return path.split("/")[1].replace(/\.js$/u, ".ts"); +}; + +const rank = (unit) => LAYER_ORDER.indexOf(UNIT_LAYERS[unit]); + +/** Unit pairs that import each other, the shortest import cycles. */ +const mutualPairs = (edges) => + [...edges.keys()] + .map((key) => key.split(" -> ")) + .filter(([from, to]) => from < to && edges.has(`${to} -> ${from}`)) + .map(([from, to]) => `${from} <-> ${to}`) + .sort(); + +/** + * Collect unit-level facts from resolved production imports: undeclared + * units, imports that point to a higher layer (counted per unit pair), and + * unit pairs that import each other. Longer cycles through three or more + * units are not reported; every such cycle contains an upward import, which + * the layer count already ratchets. + */ +export const layerGraphFacts = (imports, files = []) => { + const edges = new Map(); + const undeclared = new Set(); + for (const file of files) { + const unit = sourceUnit(file); + if (unit !== undefined && UNIT_LAYERS[unit] === undefined) + undeclared.add(unit); + } + for (const { file, target } of imports) { + const from = sourceUnit(file); + const to = sourceUnit(target); + if (from === undefined || to === undefined || from === to) continue; + for (const unit of [from, to]) + if (UNIT_LAYERS[unit] === undefined) undeclared.add(unit); + const key = `${from} -> ${to}`; + edges.set(key, (edges.get(key) ?? 0) + 1); + } + const upward = {}; + for (const [key, count] of [...edges].sort(([a], [b]) => + a < b ? -1 : a > b ? 1 : 0, + )) { + const [from, to] = key.split(" -> "); + if (undeclared.has(from) || undeclared.has(to)) continue; + if (rank(from) < rank(to)) upward[key] = count; + } + return { + undeclared: [...undeclared].sort(), + upward, + cycles: mutualPairs(edges), + }; +}; + +/** + * Compare facts with the recorded baseline. Growth fails: a new upward pair, + * more imports on a recorded pair, or a cycle group not recorded. Shrinkage is + * returned separately so the baseline can be lowered. + */ +export const compareLayerBaseline = (facts, baseline) => { + const failures = []; + const lowerable = []; + for (const unit of facts.undeclared) + failures.push( + `undeclared unit src/${unit}: add it to UNIT_LAYERS in scripts/lib/module-layers.mjs`, + ); + for (const [key, count] of Object.entries(facts.upward)) { + const allowed = baseline.upward[key] ?? 0; + if (count > allowed) + failures.push( + `upward import ${key}: ${count} imports, baseline ${allowed}`, + ); + } + for (const [key, allowed] of Object.entries(baseline.upward)) { + const count = facts.upward[key] ?? 0; + if (count < allowed) + lowerable.push(`upward import ${key}: ${count}, baseline ${allowed}`); + } + const recordedCycles = new Set(baseline.cycles); + for (const group of facts.cycles) + if (!recordedCycles.has(group)) + failures.push(`mutual import not in baseline: ${group}`); + const observedCycles = new Set(facts.cycles); + for (const group of baseline.cycles) + if (!observedCycles.has(group)) + lowerable.push(`recorded mutual import no longer present: ${group}`); + return { failures, lowerable }; +}; diff --git a/scripts/verify-module-boundaries.mjs b/scripts/verify-module-boundaries.mjs index 2069c96db9..84d3551f01 100644 --- a/scripts/verify-module-boundaries.mjs +++ b/scripts/verify-module-boundaries.mjs @@ -3,11 +3,18 @@ import { readdir, readFile } from "node:fs/promises"; import { join, resolve } from "node:path"; import { promisify } from "node:util"; -import { inspectModuleBoundaries } from "./lib/module-boundaries.mjs"; +import { + inspectResolvedModuleBoundaries, + resolveModuleImports, +} from "./lib/module-boundaries.mjs"; +import { compareLayerBaseline, layerGraphFacts } from "./lib/module-layers.mjs"; const execute = promisify(execFile); const root = process.cwd(); -const selected = process.argv.slice(2); +const printLayerBaseline = process.argv.includes("--print-layer-baseline"); +const selected = process.argv + .slice(2) + .filter((argument) => argument !== "--print-layer-baseline"); for (const file of selected) if (!file.endsWith(".ts")) throw new TypeError(`Expected an exact TypeScript source file: ${file}`); @@ -51,6 +58,8 @@ const enumerateSourceFiles = async () => { const files = await enumerateSourceFiles(); let checked = 0; +const resolvedImports = []; +const checkedFiles = []; for (const file of new Set(files.filter((path) => path.endsWith(".ts")))) { let source; try { @@ -64,8 +73,11 @@ for (const file of new Set(files.filter((path) => path.endsWith(".ts")))) { throw cause; } checked += 1; + checkedFiles.push(file.replaceAll("\\", "/")); try { - for (const violation of inspectModuleBoundaries(file, source, root)) { + const imports = resolveModuleImports(file, source, root); + resolvedImports.push(...imports); + for (const violation of inspectResolvedModuleBoundaries(imports)) { console.error( `${violation.file}:${violation.line}: ${violation.boundary}: ${violation.specifier} resolves to ${violation.target}`, ); @@ -76,5 +88,26 @@ for (const file of new Set(files.filter((path) => path.endsWith(".ts")))) { process.exitCode = 1; } } +// Unit layering and cycles are whole-graph facts; a file selection cannot +// establish them. +if (selected.length === 0) { + const baselinePath = resolve(root, "scripts/lib/module-layers.baseline.json"); + const baseline = JSON.parse(await readFile(baselinePath, "utf8")); + const facts = layerGraphFacts(resolvedImports, checkedFiles); + if (printLayerBaseline) { + console.log( + JSON.stringify({ upward: facts.upward, cycles: facts.cycles }, null, 2), + ); + } + const { failures, lowerable } = compareLayerBaseline(facts, baseline); + for (const failure of failures) { + console.error(`module layers: ${failure}`); + process.exitCode = 1; + } + if (lowerable.length > 0) + console.log( + `Module layer baseline can be lowered in scripts/lib/module-layers.baseline.json:\n ${lowerable.join("\n ")}`, + ); +} if (process.exitCode !== 1) console.log(`Checked resolved module boundaries in ${checked} source files.`); diff --git a/tests/evaluation/moduleLayers.test.ts b/tests/evaluation/moduleLayers.test.ts new file mode 100644 index 0000000000..e3895d6296 --- /dev/null +++ b/tests/evaluation/moduleLayers.test.ts @@ -0,0 +1,90 @@ +import { readdir } from "node:fs/promises"; +import { describe, expect, it } from "vitest"; + +import { + UNIT_LAYERS, + compareLayerBaseline, + layerGraphFacts, + sourceUnit, +} from "../../scripts/lib/module-layers.mjs"; + +const edge = (file: string, target: string) => ({ file, target }); + +describe("top-level module layers", () => { + it("declares a layer for every top-level source unit", async () => { + const entries = await readdir("src", { withFileTypes: true }); + const units = entries + .filter( + (entry) => + entry.isDirectory() || + (entry.name.endsWith(".ts") && + sourceUnit(`src/${entry.name}`) !== undefined), + ) + .map(({ name }) => name); + expect(units.filter((unit) => UNIT_LAYERS[unit] === undefined)).toEqual([]); + }); + + it("maps files, root files and built specifiers to units, ignoring tests", () => { + expect(sourceUnit("src/native/elf/Reader.ts")).toBe("native"); + expect(sourceUnit("src/native/elf/Reader.js")).toBe("native"); + expect(sourceUnit("src/cli.js")).toBe("cli.ts"); + expect(sourceUnit("src/native/elf/Reader.test.ts")).toBeUndefined(); + expect(sourceUnit("tests/fixtures/x.ts")).toBeUndefined(); + }); + + it("counts upward imports per unit pair and reports mutual imports", () => { + const facts = layerGraphFacts([ + edge("src/native/A.ts", "src/application/B.js"), + edge("src/native/C.ts", "src/application/D.js"), + edge("src/application/B.ts", "src/native/A.js"), + edge("src/application/B.ts", "src/domain/E.js"), + edge("src/application/B.ts", "src/application/F.js"), + edge("src/native/A.test.ts", "src/cli/G.js"), + ]); + expect(facts).toEqual({ + undeclared: [], + upward: { "native -> application": 2 }, + cycles: ["application <-> native"], + }); + }); + + it("refuses an undeclared unit instead of leaving it unconstrained", () => { + expect( + layerGraphFacts([edge("src/newProvider/A.ts", "src/domain/B.js")]) + .undeclared, + ).toEqual(["newProvider"]); + expect(layerGraphFacts([], ["src/isolated/A.ts"]).undeclared).toEqual([ + "isolated", + ]); + expect( + layerGraphFacts( + [edge("src/isolated/A.ts", "src/isolated/B.js")], + ["src/isolated/A.ts"], + ).undeclared, + ).toEqual(["isolated"]); + }); + + it("fails on growth and only reports shrinkage", () => { + const baseline = { + upward: { "native -> application": 2, "gdb -> application": 1 }, + cycles: ["application <-> native", "application <-> gdb"], + }; + const grown = compareLayerBaseline( + { + undeclared: [], + upward: { "native -> application": 3, "rizin -> application": 1 }, + cycles: ["application <-> native", "application <-> rizin"], + }, + baseline, + ); + expect(grown.failures).toEqual([ + "upward import native -> application: 3 imports, baseline 2", + "upward import rizin -> application: 1 imports, baseline 0", + "mutual import not in baseline: application <-> rizin", + ]); + expect(grown.lowerable).toEqual([ + "upward import gdb -> application: 0, baseline 1", + "recorded mutual import no longer present: application <-> gdb", + ]); + }); +}); diff --git a/turbo.json b/turbo.json index 480c743ec6..ca4d80f4e1 100644 --- a/turbo.json +++ b/turbo.json @@ -79,6 +79,7 @@ ".oxlintrc.json", "package.json", "package-lock.json", + "scripts/lib/module-layers.baseline.json", "scripts/**/*.{cjs,cts,js,jsx,mjs,mts,ts,tsx}", "src/**/*.{cjs,cts,js,jsx,mjs,mts,ts,tsx}", "tests/**/*.{cjs,cts,js,jsx,mjs,mts,ts,tsx}",