From 22b398ad8c9bd0e2c9a524289345733cc8612dca Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 11:22:01 +0200 Subject: [PATCH] ci(server): gate import cycles and cross-domain reach-ins with a baseline Module boundaries were held by review alone. storage.module.ts documents a cycle that only surfaced at runtime as "imports[1] is undefined", three modules exist just to break cycles, and nothing stopped the next feature from reaching into another domain's helpers. scripts/import-boundaries.mjs parses every import under src/ with the TypeScript parser and checks five rules against a baseline of today's violations: load-time file cycles (none today once type-only and lazy imports and the two-way MikroORM entity relations are left out), new dependencies between domains that already depend on each other (262 pairs, one tangle around plugins and auth), shared-kernel files importing a domain (8), imports of another domain's internals (69) and src/db importing src/nest (5). A domain's public surface is what the code already treats that way: modules, services, guards, decorators, types, constants, the audit logger, the plugin RPC kit and the addon gates. New violations fail, --update only drops entries, and an unreadable baseline, a missing src/ or an import it cannot resolve stops the run. --- server/CLAUDE.md | 3 + server/package.json | 1 + .../scripts/import-boundaries-baseline.json | 355 ++++++++++++++++ server/scripts/import-boundaries.mjs | 397 ++++++++++++++++++ .../unit/scripts/import-boundaries.test.ts | 216 ++++++++++ 5 files changed, 972 insertions(+) create mode 100644 server/scripts/import-boundaries-baseline.json create mode 100644 server/scripts/import-boundaries.mjs create mode 100644 server/tests/unit/scripts/import-boundaries.test.ts diff --git a/server/CLAUDE.md b/server/CLAUDE.md index a12f0da72..f7198bd82 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -18,8 +18,11 @@ npm run check:plugin-facts # CI gate — fails if the generated copies drifted node scripts/coverage-thresholds.mjs # after test:coverage — prints the ratchet block for vitest.config.ts npm run db:call-graph -- --sync --tx [--domains nest/days,…] # sync DB-touching methods + transaction sites (exit 1 if any) npm run lint:size # no file in src/ or scripts/ past 1000 lines, longer ones only shrink (CI gate) +npm run lint:boundaries # import cycles and cross-domain reach-ins may only shrink (CI gate) ``` +**`lint:boundaries`** (`scripts/import-boundaries.mjs`) parses every import under `src/` and holds five lists in `scripts/import-boundaries-baseline.json`: load-time file cycles (type-only and lazy imports, and the MikroORM entity relations, do not count), dependencies between two `nest/` folders that already depend on each other, shared-kernel files (`nest/common`, `nest/database`, `nest/app-config`) importing a domain, a domain importing another domain's internals, and `src/db` importing `src/nest`. A domain's public surface is its `*.module`, `*.service`, `*.guard`, `*.decorator`, `*.types`, `*.interface`, `*.contract`, `*.constants` and `*.logger` files plus the short `PUBLIC_FILES` list in the script; anything else is an internal. A new entry fails the check; `npm run lint:boundaries -- --update` drops the ones that no longer occur and never adds one. + **`lint:size`** (`scripts/size-lint.mjs`) counts a line longer than 120 columns once per 120 columns it spans, so joining lines does not make room. Files that were already longer are held at their length in `scripts/size-baseline.json`; a file that needs to grow past its entry is split by concern instead. After a split, `npm run lint:size -- --update` lowers the baseline (it never raises or adds an entry). Single test: `npx vitest run tests/unit/nest/weather.controller.test.ts`, or `npx vitest run -t "returns 401 without cookie"`. diff --git a/server/package.json b/server/package.json index d7de13512..f750938ff 100644 --- a/server/package.json +++ b/server/package.json @@ -13,6 +13,7 @@ "typecheck:scripts": "tsc -p tsconfig.scripts.json --noEmit", "db:call-graph": "node scripts/db-call-graph.mjs", "lint:size": "node scripts/size-lint.mjs", + "lint:boundaries": "node scripts/import-boundaries.mjs", "gen:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts", "check:plugin-facts": "node --import tsx scripts/gen-plugin-facts.ts --check", "gen:entities": "node --import tsx scripts/generate-entities.ts", diff --git a/server/scripts/import-boundaries-baseline.json b/server/scripts/import-boundaries-baseline.json new file mode 100644 index 000000000..7b0e0bcd1 --- /dev/null +++ b/server/scripts/import-boundaries-baseline.json @@ -0,0 +1,355 @@ +{ + "fileCycles": [], + "domainCycles": [ + "accommodations -> assignments", + "accommodations -> auth", + "accommodations -> mcp-shared", + "accommodations -> places", + "accommodations -> plugins", + "accommodations -> realtime", + "addons -> auth", + "addons -> memories", + "addons -> settings", + "addons -> transit", + "admin -> addons", + "admin -> auth", + "admin -> booking-import", + "admin -> llm-parse", + "admin -> memories", + "admin -> notifications", + "admin -> oauth", + "admin -> packing", + "admin -> plugins", + "admin -> realtime", + "admin -> settings", + "admin -> tokens", + "airports -> auth", + "assignments -> auth", + "assignments -> days", + "assignments -> journey", + "assignments -> mcp-shared", + "assignments -> plugins", + "assignments -> realtime", + "atlas -> addons", + "atlas -> auth", + "atlas -> geo", + "atlas -> plugins", + "atlas -> public-api", + "atlas -> tokens", + "auth -> budget", + "auth -> files", + "auth -> maps", + "auth -> notifications", + "auth -> plugins", + "auth -> settings", + "auth -> storage", + "auth -> tokens", + "booking-import -> addons", + "booking-import -> airports", + "booking-import -> budget", + "booking-import -> llm-parse", + "booking-import -> maps", + "booking-import -> places", + "booking-import -> realtime", + "booking-import -> reservations", + "budget -> addons", + "budget -> auth", + "budget -> mcp-shared", + "budget -> plugins", + "budget -> realtime", + "calendar -> days", + "calendar -> reservations", + "categories -> auth", + "categories -> mcp-shared", + "categories -> plugins", + "collab -> addons", + "collab -> auth", + "collab -> files", + "collab -> mcp-shared", + "collab -> notifications", + "collab -> plugins", + "collab -> realtime", + "collab -> storage", + "collections -> addons", + "collections -> auth", + "collections -> notifications", + "collections -> places", + "collections -> plugins", + "collections -> realtime", + "collections -> storage", + "day-notes -> auth", + "day-notes -> mcp-shared", + "day-notes -> plugins", + "day-notes -> realtime", + "days -> accommodations", + "days -> assignments", + "days -> auth", + "days -> mcp-shared", + "days -> places", + "days -> plugins", + "days -> realtime", + "files -> auth", + "files -> mcp-shared", + "files -> plugins", + "files -> realtime", + "files -> storage", + "geo -> maps", + "google-quota -> auth", + "integrations -> addons", + "integrations -> assignments", + "integrations -> atlas", + "integrations -> auth", + "integrations -> journey", + "integrations -> mcp-shared", + "integrations -> places", + "integrations -> realtime", + "integrations -> reservations", + "journey -> addons", + "journey -> atlas", + "journey -> auth", + "journey -> files", + "journey -> maps", + "journey -> memories", + "journey -> photos", + "journey -> plugins", + "journey -> realtime", + "journey -> settings", + "journey -> storage", + "llm-parse -> addons", + "llm-parse -> auth", + "llm-parse -> settings", + "maps -> auth", + "maps -> geo", + "maps -> google-quota", + "maps -> place-photos", + "maps -> settings", + "maps -> storage", + "mcp-shared -> realtime", + "memories -> addons", + "memories -> auth", + "memories -> notifications", + "memories -> photos", + "memories -> realtime", + "memories -> storage", + "notifications -> admin", + "notifications -> auth", + "notifications -> realtime", + "notifications -> storage", + "oauth -> addons", + "oauth -> auth", + "packing -> addons", + "packing -> admin", + "packing -> auth", + "packing -> mcp-shared", + "packing -> notifications", + "packing -> plugins", + "packing -> realtime", + "photos -> auth", + "photos -> memories", + "place-photos -> storage", + "places -> accommodations", + "places -> assignments", + "places -> atlas", + "places -> auth", + "places -> journey", + "places -> maps", + "places -> mcp-shared", + "places -> place-photos", + "places -> plugins", + "places -> realtime", + "places -> storage", + "places -> unsplash", + "plugins -> accommodations", + "plugins -> addons", + "plugins -> assignments", + "plugins -> atlas", + "plugins -> auth", + "plugins -> budget", + "plugins -> categories", + "plugins -> collab", + "plugins -> collections", + "plugins -> day-notes", + "plugins -> days", + "plugins -> files", + "plugins -> journey", + "plugins -> llm-parse", + "plugins -> maps", + "plugins -> notifications", + "plugins -> packing", + "plugins -> places", + "plugins -> realtime", + "plugins -> reservations", + "plugins -> tags", + "plugins -> todo", + "plugins -> trips", + "plugins -> vacay", + "plugins -> weather", + "public-api -> tokens", + "realtime -> auth", + "realtime -> journey", + "realtime -> plugins", + "reservations -> accommodations", + "reservations -> airports", + "reservations -> assignments", + "reservations -> auth", + "reservations -> budget", + "reservations -> days", + "reservations -> integrations", + "reservations -> mcp-shared", + "reservations -> notifications", + "reservations -> plugins", + "reservations -> realtime", + "settings -> admin", + "settings -> auth", + "storage -> auth", + "tags -> auth", + "tags -> plugins", + "todo -> addons", + "todo -> auth", + "todo -> mcp-shared", + "todo -> plugins", + "todo -> realtime", + "tokens -> auth", + "transit -> auth", + "transit -> days", + "transit -> google-quota", + "transit -> maps", + "transit -> mcp-shared", + "transit -> reservations", + "transit -> settings", + "trip-members -> auth", + "trip-members -> budget", + "trip-members -> notifications", + "trip-members -> realtime", + "trip-members -> trips", + "trip-read-model -> accommodations", + "trip-read-model -> budget", + "trip-read-model -> collab", + "trip-read-model -> days", + "trip-read-model -> files", + "trip-read-model -> packing", + "trip-read-model -> places", + "trip-read-model -> reservations", + "trip-read-model -> todo", + "trip-read-model -> trip-members", + "trip-read-model -> trips", + "trips -> accommodations", + "trips -> addons", + "trips -> auth", + "trips -> budget", + "trips -> calendar", + "trips -> collab", + "trips -> days", + "trips -> files", + "trips -> mcp-shared", + "trips -> packing", + "trips -> places", + "trips -> plugins", + "trips -> realtime", + "trips -> reservations", + "trips -> settings", + "trips -> storage", + "trips -> todo", + "trips -> trip-members", + "trips -> trip-read-model", + "trips -> unsplash", + "trips -> vacay", + "unsplash -> settings", + "unsplash -> storage", + "vacay -> addons", + "vacay -> auth", + "vacay -> notifications", + "vacay -> plugins", + "vacay -> realtime", + "weather -> auth", + "weather -> plugins" + ], + "sharedImportsDomain": [ + "nest/common/idempotency-cleanup.job.ts -> nest/audit/audit-log.logger.ts", + "nest/common/idempotency-cleanup.job.ts -> nest/scheduling/cron-registrar.service.ts", + "nest/common/rowShape.ts -> nest/query-helpers/query-helpers.service.ts", + "nest/common/validate-route-guards.ts -> nest/auth/cookie-auth.guard.ts", + "nest/common/validate-route-guards.ts -> nest/auth/jwt-auth.guard.ts", + "nest/common/validate-route-guards.ts -> nest/auth/optional-jwt.guard.ts", + "nest/common/validate-route-guards.ts -> nest/auth/public.decorator.ts", + "nest/common/validate-route-guards.ts -> nest/public-api/api-token.guard.ts" + ], + "domainInternals": [ + "addons -> memories/memories.helpers.ts", + "addons -> settings/instance-api-keys.ts", + "addons -> transit/transit-provider.ts", + "admin -> llm-parse/llm-config.ts", + "admin -> memories/memories.helpers.ts", + "atlas -> geo/nominatim.client.ts", + "atlas -> public-api/public-api-request.ts", + "auth -> maps/maps.helpers.ts", + "auth -> maps/providers/places-provider.ts", + "auth -> plugins/paths.ts", + "auth -> plugins/user-erasure-enqueue.ts", + "auth -> settings/instance-api-keys.ts", + "auth -> storage/storage-upload.factory.ts", + "backup -> permissions/permissions-cache.ts", + "backup -> plugins/paths.ts", + "backup -> plugins/plugin-backup.ts", + "booking-import -> airports/airports.data.ts", + "booking-import -> llm-parse/image-input.ts", + "collab -> storage/storage-upload.factory.ts", + "collections -> places/gpx-export.helpers.ts", + "collections -> places/place-image.ts", + "collections -> places/places.dto.ts", + "collections -> places/places.helpers.ts", + "collections -> storage/storage-upload.factory.ts", + "days -> accommodations/night-seat.ts", + "doc-sync -> storage/content-type.ts", + "files -> auth/jwt-verify.ts", + "files -> storage/content-type.ts", + "files -> storage/storage-upload.factory.ts", + "geo -> maps/maps.helpers.ts", + "integrations -> atlas/atlas-geo.ts", + "journey -> atlas/atlas-geo.ts", + "journey -> memories/photo-provider.registry.ts", + "journey -> storage/storage-upload.factory.ts", + "maps -> geo/nominatim.client.ts", + "maps -> settings/instance-api-keys.ts", + "notifications -> admin/admin.dto.ts", + "oidc -> admin/admin.dto.ts", + "packing -> admin/admin.dto.ts", + "place-enrichment -> maps/maps.helpers.ts", + "place-enrichment -> maps/trek-places.client.ts", + "places -> atlas/atlas-geo.ts", + "places -> storage/storage-upload.factory.ts", + "platform -> auth/jwt-verify.ts", + "plugins -> auth/jwt-verify.ts", + "plugins -> llm-parse/clients/openai-compatible.client.ts", + "plugins -> llm-parse/llm-client.factory.ts", + "plugins -> llm-parse/llm-config.resolver.ts", + "plugins -> llm-parse/llm-config.ts", + "plugins -> maps/maps.helpers.ts", + "plugins -> notifications/channel-registry.ts", + "plugins -> notifications/notification-events.ts", + "realtime -> plugins/install/host-compat.ts", + "receipt-scan -> llm-parse/image-input.ts", + "reservation-import -> integrations/airtrail.dto.ts", + "reservation-import -> llm-parse/image-input.ts", + "reservations -> airports/airports.data.ts", + "roadtrip -> places/maps-dir.helpers.ts", + "roadtrip -> plugins/contributions/plugin-route-normalize.ts", + "roadtrip -> plugins/contributions/plugin-search.helpers.ts", + "roadtrip -> plugins/kill-switch.ts", + "settings -> admin/admin.dto.ts", + "transit -> maps/maps.helpers.ts", + "transit -> settings/instance-api-keys.ts", + "trip-members -> trips/trips.dto.ts", + "trips -> mcp-shared/prompt-args.ts", + "trips -> places/places.helpers.ts", + "trips -> storage/storage-upload.factory.ts", + "unsplash -> settings/instance-api-keys.ts" + ], + "dbImportsNest": [ + "db/migrations/Migration20200101010100_encrypt_any_plaintext_oidc_client_secret_left.ts -> nest/common/crypto/apiKeyCrypto.ts", + "db/migrations/Migration20200101010200_encrypt_any_plaintext_smtp_pass_left_in.ts -> nest/common/crypto/apiKeyCrypto.ts", + "db/migrations/Migration20200101010300_encrypt_any_plaintext_immich_api_key_values.ts -> nest/common/crypto/apiKeyCrypto.ts", + "db/orm.ts -> nest/database/request-context.ts", + "db/reseat-booked-nights.ts -> nest/accommodations/night-seat.ts" + ] +} diff --git a/server/scripts/import-boundaries.mjs b/server/scripts/import-boundaries.mjs new file mode 100644 index 000000000..47e58ecaa --- /dev/null +++ b/server/scripts/import-boundaries.mjs @@ -0,0 +1,397 @@ +#!/usr/bin/env node +/* + * lint:boundaries: the server's import graph may only get cleaner. + * + * Nest resolves a module's `imports` array while the file is still loading, so + * an import cycle shows up at runtime as "imports[1] is undefined" (see + * src/nest/storage/storage.module.ts), and every new reach into another + * domain's internals is one more place a split has to untangle later. Both + * were held only by review. This walks every .ts file under src/ with the + * TypeScript parser and checks four rules against scripts/import-boundaries-baseline.json: + * + * fileCycles a runtime import edge between two files that sit on the + * same cycle. Type-only imports and lazy ones (import(), + * a require() inside a function) are erased or deferred, so + * they cannot take part in a load-time cycle and are skipped. + * So are the imports of src/db/entities/: MikroORM relations + * point both ways by design and are read through `() => X` + * thunks after every file has loaded. + * domainCycles the same, between src/nest/ folders: domain A + * imports from B while B (directly or through others) + * imports from A. Keyed by the domain pair, so a further + * import along a dependency that already exists is not new; + * a new dependency between two domains of the same cycle is. + * The shared kernel folders are left out here and held by + * the next rule instead. + * sharedImportsDomain + * a file of the shared kernel (src/nest/common, database, + * app-config), which every domain imports, importing from a + * domain. That ties the domain into every other one. + * domainInternals a file in src/nest// importing (value or type) a + * file of src/nest// that is not B's public surface. + * Public is what the codebase already treats that way: the + * *.module, *.service, *.guard, *.decorator, *.types, + * *.interface, *.contract, *.constants and *.logger files, + * the shared kernel folders (common, database, app-config) + * and the few entry points in PUBLIC_FILES. Keyed by the + * importing domain, so moving a file inside A changes nothing. + * dbImportsNest a file under src/db/ importing from src/nest/. The data + * layer sits below the domains, never the other way round. + * + * Each rule's baseline lists today's violations. A violation not in it fails + * the check; one that is gone is reported so the baseline can shrink. + * + * npm run lint:boundaries check against the baseline (CI) + * npm run lint:boundaries -- --update drop the entries that no longer occur; + * it never adds one + * + * --dir= points the check at another server root (the unit tests use it). + */ +import { createRequire } from 'node:module'; +import { existsSync, readdirSync, readFileSync, realpathSync, statSync, writeFileSync } from 'node:fs'; +import { dirname, join, relative, resolve, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const require = createRequire(import.meta.url); +const ts = require('typescript'); + +export const RULES = ['fileCycles', 'domainCycles', 'sharedImportsDomain', 'domainInternals', 'dbImportsNest']; + +/** Folders under src/nest whose every file is shared infrastructure, open to all domains. */ +export const SHARED_DOMAINS = new Set(['common', 'database', 'app-config']); + +/** File name suffixes that make up a domain's public surface. */ +export const PUBLIC_SUFFIXES = [ + '.module.ts', + '.service.ts', + '.guard.ts', + '.decorator.ts', + '.types.ts', + '.interface.ts', + '.contract.ts', + '.constants.ts', + '.logger.ts', +]; + +/** + * Entry points that are public although their name does not say so: the + * plugin RPC kit every domain's *.rpc.ts is written against, and the gates + * and helpers every controller shares. Adding to this list is a design + * decision for review, not a way past the check. + */ +export const PUBLIC_FILES = [ + 'plugins/host/rpc-kit/', + 'plugins/host/rpc-params.ts', + 'plugins/host/rpc-errors.ts', + 'audit/client-ip.ts', + 'addons/addon-gate.ts', + 'addons/mcp-addon-gate.ts', +]; + +function walk(dir, files = []) { + for (const name of readdirSync(dir)) { + const path = join(dir, name); + if (statSync(path).isDirectory()) walk(path, files); + else if (/\.ts$/.test(name) && !/\.d\.ts$/.test(name)) files.push(path); + } + return files; +} + +function resolveSpecifier(fromFile, spec) { + if (!spec.startsWith('.')) return null; + const base = resolve(dirname(fromFile), spec.replace(/\.js$/, '')); + for (const candidate of [`${base}.ts`, join(base, 'index.ts'), base]) { + if (existsSync(candidate) && statSync(candidate).isFile()) return candidate; + } + return null; +} + +/** True when every name an import declaration brings in is type-only. */ +function isTypeOnlyImport(node) { + const clause = node.importClause; + if (!clause) return false; // `import './x'` runs the file. + if (clause.isTypeOnly) return true; + if (clause.name) return false; + const bindings = clause.namedBindings; + if (!bindings || !ts.isNamedImports(bindings)) return false; + return bindings.elements.length > 0 && bindings.elements.every((el) => el.isTypeOnly); +} + +function isTypeOnlyExport(node) { + if (node.isTypeOnly) return true; + const clause = node.exportClause; + return Boolean(clause && ts.isNamedExports(clause) && clause.elements.length > 0 && clause.elements.every((el) => el.isTypeOnly)); +} + +/** Every relative import of a file: { spec, runtime } where runtime means eager and not erased. */ +export function importsOf(text, fileName = 'file.ts') { + const sf = ts.createSourceFile(fileName, text, ts.ScriptTarget.ES2022, true, ts.ScriptKind.TS); + const found = []; + const add = (spec, runtime) => found.push({ spec, runtime }); + for (const stmt of sf.statements) { + if (ts.isImportDeclaration(stmt) && ts.isStringLiteral(stmt.moduleSpecifier)) { + add(stmt.moduleSpecifier.text, !isTypeOnlyImport(stmt)); + } else if (ts.isExportDeclaration(stmt) && stmt.moduleSpecifier && ts.isStringLiteral(stmt.moduleSpecifier)) { + add(stmt.moduleSpecifier.text, !isTypeOnlyExport(stmt)); + } else if ( + ts.isImportEqualsDeclaration(stmt) && + ts.isExternalModuleReference(stmt.moduleReference) && + ts.isStringLiteral(stmt.moduleReference.expression) + ) { + add(stmt.moduleReference.expression.text, !stmt.isTypeOnly); + } + } + // require() and import() anywhere: eager only when the require sits in top-level code. + const visit = (node, depth) => { + if (ts.isCallExpression(node) && node.arguments.length === 1 && ts.isStringLiteral(node.arguments[0])) { + const isRequire = ts.isIdentifier(node.expression) && node.expression.text === 'require'; + const isDynamic = node.expression.kind === ts.SyntaxKind.ImportKeyword; + if (isRequire || isDynamic) add(node.arguments[0].text, isRequire && depth === 0); + } + const inner = ts.isFunctionLike(node) ? depth + 1 : depth; + ts.forEachChild(node, (child) => visit(child, inner)); + }; + for (const stmt of sf.statements) { + if (!ts.isImportDeclaration(stmt) && !ts.isExportDeclaration(stmt) && !ts.isImportEqualsDeclaration(stmt)) visit(stmt, 0); + } + return found; +} + +/** All resolved relative import edges under src/, as src-relative POSIX paths. */ +export function collectEdges(serverDir) { + const src = join(serverDir, 'src'); + if (!existsSync(src) || !statSync(src).isDirectory()) { + throw new Error(`src/ does not exist under ${serverDir}: the check would pass without looking at anything`); + } + const rel = (p) => relative(src, p).split(sep).join('/'); + const edges = []; + const unresolved = []; + for (const file of walk(src)) { + for (const { spec, runtime } of importsOf(readFileSync(file, 'utf8'), file)) { + if (!spec.startsWith('.')) continue; + const target = resolveSpecifier(file, spec); + if (!target) { + unresolved.push(`${rel(file)} -> ${spec}`); + continue; + } + if (target.startsWith(src + sep)) edges.push({ from: rel(file), to: rel(target), runtime }); + } + } + return { edges, unresolved }; +} + +/** Tarjan's strongly connected components; returns a node -> component id map for components of size > 1. */ +function cyclicComponents(graph) { + let index = 0; + const idx = new Map(); + const low = new Map(); + const onStack = new Set(); + const stack = []; + const component = new Map(); + let next = 0; + const strong = (v) => { + // Iterative, so a deep import chain cannot overflow the call stack. + const work = [[v, 0]]; + idx.set(v, index); + low.set(v, index); + index++; + stack.push(v); + onStack.add(v); + while (work.length) { + const frame = work[work.length - 1]; + const [node, i] = frame; + const targets = [...(graph.get(node) ?? [])]; + if (i < targets.length) { + frame[1]++; + const w = targets[i]; + if (!idx.has(w)) { + idx.set(w, index); + low.set(w, index); + index++; + stack.push(w); + onStack.add(w); + work.push([w, 0]); + } else if (onStack.has(w)) { + low.set(node, Math.min(low.get(node), idx.get(w))); + } + continue; + } + work.pop(); + if (work.length) { + const parent = work[work.length - 1][0]; + low.set(parent, Math.min(low.get(parent), low.get(node))); + } + if (low.get(node) === idx.get(node)) { + const members = []; + let w; + do { + w = stack.pop(); + onStack.delete(w); + members.push(w); + } while (w !== node); + if (members.length > 1) { + for (const m of members) component.set(m, next); + next++; + } + } + } + }; + for (const v of graph.keys()) if (!idx.has(v)) strong(v); + return component; +} + +function cycleEdges(pairs) { + const graph = new Map(); + for (const [a, b] of pairs) { + if (a === b) continue; + if (!graph.has(a)) graph.set(a, new Set()); + if (!graph.has(b)) graph.set(b, new Set()); + graph.get(a).add(b); + } + const component = cyclicComponents(graph); + const out = new Set(); + for (const [a, targets] of graph) { + for (const b of targets) { + if (component.has(a) && component.get(a) === component.get(b)) out.add(`${a} -> ${b}`); + } + } + return out; +} + +const domainOf = (file) => file.match(/^nest\/([^/]+)\//)?.[1] ?? null; + +export function isPublic(target) { + const inner = target.slice('nest/'.length); + const domain = inner.split('/')[0]; + if (SHARED_DOMAINS.has(domain)) return true; + if (PUBLIC_FILES.some((p) => (p.endsWith('/') ? inner.startsWith(p) : inner === p))) return true; + return PUBLIC_SUFFIXES.some((s) => target.endsWith(s)); +} + +/** The violations of every rule, each a sorted list of stable string keys. */ +export function analyse(edges) { + const runtime = edges.filter((e) => e.runtime); + const fileCycles = cycleEdges(runtime.filter((e) => !e.from.startsWith('db/entities/')).map((e) => [e.from, e.to])); + + const domainPairs = []; + for (const e of runtime) { + const a = domainOf(e.from); + const b = domainOf(e.to); + if (a && b && a !== b && !SHARED_DOMAINS.has(a) && !SHARED_DOMAINS.has(b)) domainPairs.push([a, b]); + } + const domainCycles = cycleEdges(domainPairs); + + const sharedImportsDomain = new Set(); + const domainInternals = new Set(); + const dbImportsNest = new Set(); + for (const e of edges) { + const a = domainOf(e.from); + const b = domainOf(e.to); + if (a && b && SHARED_DOMAINS.has(a) && !SHARED_DOMAINS.has(b)) sharedImportsDomain.add(`${e.from} -> ${e.to}`); + if (a && b && a !== b && !isPublic(e.to)) domainInternals.add(`${a} -> ${e.to.slice('nest/'.length)}`); + if (e.from.startsWith('db/') && e.to.startsWith('nest/')) dbImportsNest.add(`${e.from} -> ${e.to}`); + } + const sorted = (set) => [...set].sort(); + return { + fileCycles: sorted(fileCycles), + domainCycles: sorted(domainCycles), + sharedImportsDomain: sorted(sharedImportsDomain), + domainInternals: sorted(domainInternals), + dbImportsNest: sorted(dbImportsNest), + }; +} + +/** The baseline as committed. Missing, unreadable or malformed stops the run. */ +export function readBaseline(path) { + let parsed; + try { + parsed = JSON.parse(readFileSync(path, 'utf8')); + } catch (err) { + throw new Error(`scripts/import-boundaries-baseline.json cannot be read: ${err.message}`); + } + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new Error('scripts/import-boundaries-baseline.json must be an object with one list per rule'); + } + for (const rule of RULES) { + if (!Array.isArray(parsed[rule]) || !parsed[rule].every((entry) => typeof entry === 'string')) { + throw new Error(`scripts/import-boundaries-baseline.json: "${rule}" must be a list of strings`); + } + } + const unknown = Object.keys(parsed).filter((key) => !RULES.includes(key)); + if (unknown.length) throw new Error(`scripts/import-boundaries-baseline.json: unknown rule(s) ${unknown.join(', ')}`); + return parsed; +} + +export function compare(baseline, found) { + const added = {}; + const gone = {}; + for (const rule of RULES) { + const allowed = new Set(baseline[rule]); + const now = new Set(found[rule]); + added[rule] = found[rule].filter((key) => !allowed.has(key)); + gone[rule] = baseline[rule].filter((key) => !now.has(key)); + } + return { added, gone }; +} + +const HINTS = { + fileCycles: + 'This import closes a load-time cycle. Move the shared piece into a file both can import, or make the import type-only.', + domainCycles: + 'This makes two domains depend on each other. Invert the dependency (an event, a port the other side implements) or move the shared piece into a domain both can import.', + domainInternals: + "This reaches into another domain's internals. Go through its service or module, or move the piece into a *.types.ts/*.contract.ts file it publishes.", + sharedImportsDomain: + 'The shared kernel is imported by every domain, so it may not import one. Move the piece into the kernel, or inject it from the domain.', + dbImportsNest: 'src/db sits below src/nest. Move what the data layer needs into src/db (or src/utils) instead.', +}; + +function main(argv) { + const dirArg = argv.find((a) => a.startsWith('--dir=')); + const serverDir = dirArg ? resolve(dirArg.slice('--dir='.length)) : fileURLToPath(new URL('..', import.meta.url)); + const baselinePath = join(serverDir, 'scripts', 'import-boundaries-baseline.json'); + const update = argv.includes('--update'); + + const { edges, unresolved } = collectEdges(serverDir); + if (unresolved.length) { + // An import the walker cannot follow is an edge it cannot judge: stop rather than pass on a partial graph. + for (const u of unresolved) console.error(`FAIL unresolved import ${u}`); + return 1; + } + const found = analyse(edges); + let baseline = readBaseline(baselinePath); + if (update) { + const now = Object.fromEntries(RULES.map((rule) => [rule, new Set(found[rule])])); + baseline = Object.fromEntries(RULES.map((rule) => [rule, baseline[rule].filter((key) => now[rule].has(key)).sort()])); + writeFileSync(baselinePath, JSON.stringify(baseline, null, 2) + '\n'); + } + + const { added, gone } = compare(baseline, found); + let failed = 0; + for (const rule of RULES) { + for (const key of added[rule]) { + console.error(`FAIL ${rule}: ${key}. ${HINTS[rule]}`); + failed++; + } + } + const lowerable = RULES.reduce((n, rule) => n + gone[rule].length, 0); + if (lowerable && !update) { + console.log(`${lowerable} baseline entr${lowerable === 1 ? 'y no longer occurs' : 'ies no longer occur'}: run with --update to drop them.`); + } + console.log( + `boundaries: ${edges.length} import(s); held at baseline: ` + RULES.map((rule) => `${rule} ${baseline[rule].length}`).join(', '), + ); + return failed ? 1 : 0; +} + +// Compared by real path, so the check still runs when the script is started through a symlink. +const isCli = + Boolean(process.argv[1]) && existsSync(process.argv[1]) && realpathSync(process.argv[1]) === fileURLToPath(import.meta.url); +if (isCli) { + try { + process.exitCode = main(process.argv.slice(2)); + } catch (err) { + console.error(`FAIL ${err.message}`); + process.exitCode = 1; + } +} diff --git a/server/tests/unit/scripts/import-boundaries.test.ts b/server/tests/unit/scripts/import-boundaries.test.ts new file mode 100644 index 000000000..741d8add6 --- /dev/null +++ b/server/tests/unit/scripts/import-boundaries.test.ts @@ -0,0 +1,216 @@ +/** + * scripts/import-boundaries.mjs is the CI gate that keeps the server's import + * graph from getting worse: no new load-time cycle, no new dependency between + * two domains that already depend on each other, no shared-kernel file + * importing a domain, no new reach into another domain's internals, and no + * src/db file importing src/nest. This runs the real script as a child process + * against throwaway server roots (--dir), proving each rule fails on a + * violation, that what the rules deliberately ignore stays ignored, and that a + * broken setup stops the run instead of passing it. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +const SCRIPT = path.join(__dirname, '../../../scripts/import-boundaries.mjs'); + +const EMPTY = { fileCycles: [], domainCycles: [], sharedImportsDomain: [], domainInternals: [], dbImportsNest: [] }; + +interface ExecError { + status: number | null; + stdout: string; + stderr: string; +} + +const roots: string[] = []; + +/** A server root with the given src/ files; `baseline: null` writes no baseline file. */ +function serverRoot(files: Record, baseline: unknown = EMPTY): string { + const dir = mkdtempSync(path.join(tmpdir(), 'trek-boundaries-')); + roots.push(dir); + mkdirSync(path.join(dir, 'src'), { recursive: true }); + mkdirSync(path.join(dir, 'scripts'), { recursive: true }); + for (const [file, text] of Object.entries(files)) { + mkdirSync(path.dirname(path.join(dir, 'src', file)), { recursive: true }); + writeFileSync(path.join(dir, 'src', file), text); + } + if (baseline !== null) { + writeFileSync( + path.join(dir, 'scripts/import-boundaries-baseline.json'), + typeof baseline === 'string' ? baseline : JSON.stringify(baseline), + ); + } + return dir; +} + +function run(dir: string, ...args: string[]): { status: number | null; out: string } { + try { + const stdout = execFileSync(process.execPath, [SCRIPT, `--dir=${dir}`, ...args], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + return { status: 0, out: stdout }; + } catch (err) { + const { status, stdout, stderr } = err as ExecError; + return { status, out: `${stdout}${stderr}` }; + } +} + +afterEach(() => { + for (const dir of roots.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +describe('import-boundaries.mjs', () => { + it('BOUND-001: a clean graph passes', () => { + const dir = serverRoot({ + 'nest/a/a.service.ts': "import { B } from '../b/b.service';\nexport class A { b = B; }\n", + 'nest/b/b.service.ts': "import { helper } from './b.helpers';\nexport class B { h = helper; }\n", + 'nest/b/b.helpers.ts': 'export const helper = 1;\n', + }); + const { status, out } = run(dir); + expect(status).toBe(0); + expect(out).toContain('fileCycles 0'); + }); + + it('BOUND-002: fails on a new load-time file cycle (the "imports[1] is undefined" shape)', () => { + const dir = serverRoot({ + 'nest/a/a.module.ts': "import { BModule } from '../b/b.module';\nexport class AModule { i = [BModule]; }\n", + 'nest/b/b.module.ts': "import { AModule } from '../a/a.module';\nexport class BModule { i = [AModule]; }\n", + }); + const { status, out } = run(dir); + expect(status).toBe(1); + expect(out).toContain('FAIL fileCycles: nest/a/a.module.ts -> nest/b/b.module.ts'); + expect(out).toContain('FAIL fileCycles: nest/b/b.module.ts -> nest/a/a.module.ts'); + expect(out).toContain('FAIL domainCycles: a -> b'); + }); + + it('BOUND-003: type-only and lazy imports cannot close a cycle', () => { + const dir = serverRoot({ + 'x/one.ts': "import { Two } from './two';\nexport class One { t = Two; }\n", + 'x/two.ts': [ + "import type { One } from './one';", + "import { type One as O2 } from './one';", + "export type { One as Re } from './one';", + 'export class Two {', + " load() { return require('./one'); }", + " later() { return import('./one'); }", + '}', + 'export type T = One | O2;', + '', + ].join('\n'), + }); + expect(run(dir).status).toBe(0); + // A top-level require is eager and does close it. + const eager = serverRoot({ + 'x/one.ts': "import { Two } from './two';\nexport class One { t = Two; }\n", + 'x/two.ts': "const one = require('./one');\nexport class Two { o = one; }\n", + }); + expect(run(eager).status).toBe(1); + }); + + it('BOUND-004: MikroORM entity relations that point both ways are not cycles', () => { + const dir = serverRoot({ + 'db/entities/Trips.entity.ts': "import { Days } from './Days.entity';\nexport class Trips { d = () => Days; }\n", + 'db/entities/Days.entity.ts': "import { Trips } from './Trips.entity';\nexport class Days { t = () => Trips; }\n", + }); + expect(run(dir).status).toBe(0); + }); + + it('BOUND-005: fails on a reach into another domain internals, allows its public surface', () => { + const files = { + 'nest/b/b.service.ts': 'export class B {}\n', + 'nest/b/b.helpers.ts': 'export const h = 1;\n', + 'nest/b/b.types.ts': 'export interface T { x: number }\n', + 'nest/common/row-id.ts': 'export const id = 1;\n', + 'nest/plugins/host/rpc-kit/decorators.ts': 'export const Rpc = 1;\n', + }; + const ok = serverRoot({ + ...files, + 'nest/a/a.service.ts': [ + "import { B } from '../b/b.service';", + "import type { T } from '../b/b.types';", + "import { id } from '../common/row-id';", + "import { Rpc } from '../plugins/host/rpc-kit/decorators';", + 'export const a: [unknown, T | null, number, number] = [B, null, id, Rpc];', + '', + ].join('\n'), + }); + expect(run(ok).status).toBe(0); + + // Type-only still counts: it couples A to B's private shapes all the same. + const bad = serverRoot({ ...files, 'nest/a/sub/a.thing.ts': "import type { h } from '../../b/b.helpers';\nexport type X = typeof h;\n" }); + const { status, out } = run(bad); + expect(status).toBe(1); + expect(out).toContain('FAIL domainInternals: a -> b/b.helpers.ts'); + }); + + it('BOUND-006: fails when a shared-kernel file imports a domain', () => { + const dir = serverRoot({ + 'nest/auth/jwt-auth.guard.ts': 'export class JwtAuthGuard {}\n', + 'nest/common/validate.ts': "import { JwtAuthGuard } from '../auth/jwt-auth.guard';\nexport const g = JwtAuthGuard;\n", + }); + const { status, out } = run(dir); + expect(status).toBe(1); + expect(out).toContain('FAIL sharedImportsDomain: nest/common/validate.ts -> nest/auth/jwt-auth.guard.ts'); + }); + + it('BOUND-007: fails when src/db imports from src/nest', () => { + const dir = serverRoot({ + 'nest/database/request-context.ts': 'export const ctx = 1;\n', + 'db/orm.ts': "import { ctx } from '../nest/database/request-context';\nexport const o = ctx;\n", + }); + const { status, out } = run(dir); + expect(status).toBe(1); + expect(out).toContain('FAIL dbImportsNest: db/orm.ts -> nest/database/request-context.ts'); + }); + + it('BOUND-008: a baselined violation passes; a gone one is reported and --update drops it without adding', () => { + const files = { + 'nest/b/b.helpers.ts': 'export const h = 1;\n', + 'nest/a/a.service.ts': "import { h } from '../b/b.helpers';\nexport const a = h;\n", + 'nest/c/c.service.ts': "import { h } from '../b/b.helpers';\nexport const c = h;\n", + }; + const baseline = { ...EMPTY, domainInternals: ['a -> b/b.helpers.ts', 'z -> b/gone.ts'] }; + const dir = serverRoot(files, baseline); + const check = run(dir); + expect(check.status).toBe(1); + expect(check.out).toContain('FAIL domainInternals: c -> b/b.helpers.ts'); + expect(check.out).not.toContain('FAIL domainInternals: a -> b/b.helpers.ts'); + expect(check.out).toContain('1 baseline entry no longer occurs'); + + expect(run(dir, '--update').status).toBe(1); + const written: unknown = JSON.parse(readFileSync(path.join(dir, 'scripts/import-boundaries-baseline.json'), 'utf8')); + expect(written).toEqual({ ...EMPTY, domainInternals: ['a -> b/b.helpers.ts'] }); + }); + + it('BOUND-009: fails closed on a missing or malformed baseline, a missing src/ and an unresolvable import', () => { + const file = { 'x/a.ts': 'export const a = 1;\n' }; + expect(run(serverRoot(file, null)).out).toContain('cannot be read'); + expect(run(serverRoot(file, null)).status).toBe(1); + expect(run(serverRoot(file, '{')).status).toBe(1); + expect(run(serverRoot(file, { ...EMPTY, fileCycles: 'none' })).status).toBe(1); + const { domainCycles: _dropped, ...missingRule } = EMPTY; + expect(run(serverRoot(file, missingRule)).status).toBe(1); + expect(run(serverRoot(file, { ...EMPTY, newRule: [] })).status).toBe(1); + + const noSrc = serverRoot(file); + rmSync(path.join(noSrc, 'src'), { recursive: true }); + expect(run(noSrc).out).toContain('src/ does not exist'); + + const broken = serverRoot({ 'x/a.ts': "import { b } from './missing';\nexport const a = b;\n" }); + const res = run(broken); + expect(res.status).toBe(1); + expect(res.out).toContain('unresolved import x/a.ts -> ./missing'); + }); + + it('BOUND-010: resolves .js specifiers and index files', () => { + const dir = serverRoot({ + 'x/a.ts': "import { b } from './b.js';\nimport { c } from './c';\nexport const a = [b, c];\n", + 'x/b.ts': 'export const b = 1;\n', + 'x/c/index.ts': 'export const c = 1;\n', + }); + expect(run(dir).status).toBe(0); + }); +});