diff --git a/server/CLAUDE.md b/server/CLAUDE.md index 799d0c520..e25863eca 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -32,7 +32,7 @@ npm run lint:query-api # MikroORM QueryBuilder calls in the repositories may npm run contracts:dto-open # open shapes in the createZodDto schemas may only shrink (runs in the unit tests) ``` -**`lint:boundaries`** (`scripts/import-boundaries.mjs`) parses every import under `src/` and holds six 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`, `nest/auth-core`) importing a domain, a domain importing another domain's internals, `src/db` importing `src/nest`, and a domain injecting (`@InjectRepository`) the repository of an entity another domain owns (`foreignRepositories`; the owner of every injected entity is listed in `scripts/repository-owners.json`, and an entity missing there stops the run). For trip visibility, inject `TripAccessService` (trip-membership) instead of `TripsRepository`. 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, and so does a baseline entry that no longer occurs, so the fix lands with `npm run lint:boundaries -- --update`, which drops such entries and never adds one. +**`lint:boundaries`** (`scripts/import-boundaries.mjs`) parses every import under `src/` and holds seven 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`, `nest/auth-core`) importing a domain, a domain importing another domain's internals, `src/db` importing `src/nest`, a file under a domain's `providers/` folder importing that domain's orchestrator (`.service.ts`) or `MapsService` (a provider knows how to ask one source; the orchestrator decides which one answers), and a domain injecting (`@InjectRepository`) the repository of an entity another domain owns (`foreignRepositories`; the owner of every injected entity is listed in `scripts/repository-owners.json`, and an entity missing there stops the run). For trip visibility, inject `TripAccessService` (trip-membership) instead of `TripsRepository`. 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, and so does a baseline entry that no longer occurs, so the fix lands with `npm run lint:boundaries -- --update`, which drops such entries and never adds one. **`contracts:dto-open`** (`tests/unit/contracts/dto-open-shapes.test.ts`, wrapped by `scripts/dto-open-shapes.mjs`) finds every `export class X extends createZodDto(...)` under `src/`, imports it and counts the open shapes (loose object, record of unknown, `z.unknown()`/`z.any()`, `z.custom()`) in the schema the Zod pipe validates, with shared's `contracts:open` walk. Each DTO class is held to its entry in `scripts/dto-open-shapes-baseline.json`, and a new one may hold none, whatever its schema is called and wherever it is declared. A `createZodDto` call that is not an exported class, or two DTO classes with one name, fail the check. After narrowing a schema, `npm run contracts:dto-open -- --update` lowers the baseline (it never raises or adds an entry). It needs `shared/` built. diff --git a/server/scripts/import-boundaries-baseline.json b/server/scripts/import-boundaries-baseline.json index 208d067f9..8f6fd86f4 100644 --- a/server/scripts/import-boundaries-baseline.json +++ b/server/scripts/import-boundaries-baseline.json @@ -306,5 +306,6 @@ "unsplash -> Users (owned by auth)", "vacay -> SchoolHolidayRegions (owned by school-holidays)", "vacay -> Users (owned by auth)" - ] + ], + "providersImportOrchestrator": [] } diff --git a/server/scripts/import-boundaries.mjs b/server/scripts/import-boundaries.mjs index 2953a7043..1f5154c10 100644 --- a/server/scripts/import-boundaries.mjs +++ b/server/scripts/import-boundaries.mjs @@ -46,6 +46,13 @@ * injection of an entity the domain already reads is not new. * Asking the owner instead (a service such as * trip-membership's TripAccessService) is what shrinks it. + * providersImportOrchestrator + * a file under a providers/ folder of src/nest// + * importing (value or type) A's own orchestrator, + * nest//.service.ts, or MapsService from any domain. + * A provider knows how to ask one source; which source + * answers is the orchestrator's decision, so the dependency + * only ever points from the orchestrator to its providers. * * Each rule's baseline lists today's violations. A violation not in it fails * the check, and so does an entry that no longer occurs, until --update drops @@ -65,7 +72,7 @@ import { fileURLToPath } from 'node:url'; const require = createRequire(import.meta.url); const ts = require('typescript'); -export const RULES = ['fileCycles', 'domainCycles', 'sharedImportsDomain', 'domainInternals', 'dbImportsNest', 'foreignRepositories']; +export const RULES = ['fileCycles', 'domainCycles', 'sharedImportsDomain', 'domainInternals', 'dbImportsNest', 'foreignRepositories', 'providersImportOrchestrator']; /** * Folders under src/nest whose every file is shared infrastructure, open to all domains. @@ -277,6 +284,14 @@ function cycleEdges(pairs) { const domainOf = (file) => file.match(/^nest\/([^/]+)\//)?.[1] ?? null; +/** The orchestrators no provider may import: its own domain's main service, and MapsService everywhere. */ +function isOrchestratorOf(providerFile, target) { + const domain = domainOf(providerFile); + return target === `nest/${domain}/${domain}.service.ts` || target === 'nest/maps/maps.service.ts'; +} + +const isProviderFile = (file) => /^nest\/[^/]+\/(?:.+\/)?providers\//.test(file); + export function isPublic(target) { const inner = target.slice('nest/'.length); const domain = inner.split('/')[0]; @@ -343,12 +358,14 @@ export function analyse(edges) { const sharedImportsDomain = new Set(); const domainInternals = new Set(); const dbImportsNest = new Set(); + const providersImportOrchestrator = 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}`); + if (isProviderFile(e.from) && isOrchestratorOf(e.from, e.to)) providersImportOrchestrator.add(`${e.from} -> ${e.to}`); } const sorted = (set) => [...set].sort(); return { @@ -358,6 +375,7 @@ export function analyse(edges) { domainInternals: sorted(domainInternals), dbImportsNest: sorted(dbImportsNest), foreignRepositories: [], + providersImportOrchestrator: sorted(providersImportOrchestrator), }; } @@ -406,6 +424,8 @@ const HINTS = { dbImportsNest: 'src/db sits below src/nest. Move what the data layer needs into src/db (or src/utils) instead.', foreignRepositories: "This injects the repository of a table another domain owns. Ask the owner's service instead (e.g. TripAccessService for trip visibility), or move the code to the owning domain.", + providersImportOrchestrator: + 'A provider sits below the orchestrator that chooses between providers. Hand it what it needs as an argument, or move the shared piece into a helpers file both import.', }; function main(argv) { diff --git a/server/tests/unit/scripts/import-boundaries.test.ts b/server/tests/unit/scripts/import-boundaries.test.ts index 56cdc9207..556d46e27 100644 --- a/server/tests/unit/scripts/import-boundaries.test.ts +++ b/server/tests/unit/scripts/import-boundaries.test.ts @@ -16,7 +16,15 @@ import { afterEach, describe, expect, it } from 'vitest'; const SCRIPT = path.join(__dirname, '../../../scripts/import-boundaries.mjs'); -const EMPTY = { fileCycles: [], domainCycles: [], sharedImportsDomain: [], domainInternals: [], dbImportsNest: [], foreignRepositories: [] }; +const EMPTY = { + fileCycles: [], + domainCycles: [], + sharedImportsDomain: [], + domainInternals: [], + dbImportsNest: [], + foreignRepositories: [], + providersImportOrchestrator: [], +}; interface ExecError { status: number | null; @@ -170,6 +178,25 @@ describe('import-boundaries.mjs', () => { expect(out).toContain('FAIL dbImportsNest: db/orm.ts -> nest/database/request-context.ts'); }); + it('BOUND-012: a provider may not import the orchestrator above it, type-only included', () => { + const dir = serverRoot({ + 'nest/maps/maps.service.ts': "import { Osm } from './providers/osm.client';\nexport class MapsService { o = Osm; }\n", + 'nest/maps/providers/osm.client.ts': "import type { MapsService } from '../maps.service';\nexport class Osm { m?: MapsService; }\n", + 'nest/docs/docs.service.ts': 'export class DocsService {}\n', + 'nest/docs/providers/webdav/client.ts': "import { DocsService } from '../../docs.service';\nexport const c = DocsService;\n", + 'nest/trips/providers/geo.ts': "import { MapsService } from '../../maps/maps.service';\nexport const g = MapsService;\n", + // A provider importing another service of its own domain is not the orchestrator. + 'nest/docs/docs-cache.service.ts': 'export class DocsCacheService {}\n', + 'nest/docs/providers/cached.ts': "import { DocsCacheService } from '../docs-cache.service';\nexport const k = DocsCacheService;\n", + }); + const { status, out } = run(dir); + expect(status).toBe(1); + expect(out).toContain('FAIL providersImportOrchestrator: nest/maps/providers/osm.client.ts -> nest/maps/maps.service.ts'); + expect(out).toContain('FAIL providersImportOrchestrator: nest/docs/providers/webdav/client.ts -> nest/docs/docs.service.ts'); + expect(out).toContain('FAIL providersImportOrchestrator: nest/trips/providers/geo.ts -> nest/maps/maps.service.ts'); + expect(out).not.toContain('providers/cached.ts'); + }); + it('BOUND-008: a baselined violation passes; a gone one fails and --update drops it without adding', () => { const files = { 'nest/b/b.helpers.ts': 'export const h = 1;\n',