From 809ec386fccface17bc74ed5b5d59b35e3659e9b Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 13:58:19 +0200 Subject: [PATCH] ci(server): fail lint:size and lint:boundaries on stale baseline entries Both server ratchets only logged an entry the tree had moved past. A file that shrank could grow back to its old size entry unseen, a deleted file left its allowance to the next file at its path, and a boundary violation that was fixed stayed allowed until someone ran --update. The client ratchets now fail on such entries, so the server ones do the same. lint:size fails on an entry above its file's size or for a file that is gone, and lint:boundaries on an entry that no longer occurs; both tell to run --update, which still only lowers or drops. Both baselines match the tree today. --- server/CLAUDE.md | 4 +-- server/scripts/import-boundaries.mjs | 17 +++++++--- server/scripts/size-lint.mjs | 33 ++++++++++++++----- .../unit/scripts/import-boundaries.test.ts | 19 +++++++++-- server/tests/unit/scripts/size-lint.test.ts | 22 +++++++++++-- 5 files changed, 77 insertions(+), 18 deletions(-) diff --git a/server/CLAUDE.md b/server/CLAUDE.md index a386a7201..a6d88afef 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -22,11 +22,11 @@ npm run lint:boundaries # import cycles and cross-domain reach-ins may only sh 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 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: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, 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. -**`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). +**`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. An entry above what its file holds now, or for a file that is gone, fails as well, so after a split `npm run lint:size -- --update` lowers the baseline in the same change (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/scripts/import-boundaries.mjs b/server/scripts/import-boundaries.mjs index 47e58ecaa..c65c52c80 100644 --- a/server/scripts/import-boundaries.mjs +++ b/server/scripts/import-boundaries.mjs @@ -39,7 +39,8 @@ * 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. + * the check, and so does an entry that no longer occurs, until --update drops + * it: a stale entry would let the same violation back in unseen. * * npm run lint:boundaries check against the baseline (CI) * npm run lint:boundaries -- --update drop the entries that no longer occur; @@ -374,9 +375,17 @@ function main(argv) { 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.`); + for (const rule of RULES) { + for (const key of gone[rule]) { + console.error(`FAIL ${rule}: ${key} is in scripts/import-boundaries-baseline.json but no longer occurs.`); + failed++; + } + } + if (RULES.some((rule) => gone[rule].length)) { + console.error( + 'Run npm run lint:boundaries -- --update to drop it with the change that fixed it: ' + + 'a stale entry would let the same violation back in unseen.', + ); } console.log( `boundaries: ${edges.length} import(s); held at baseline: ` + RULES.map((rule) => `${rule} ${baseline[rule].length}`).join(', '), diff --git a/server/scripts/size-lint.mjs b/server/scripts/size-lint.mjs index badf61941..8481f0873 100644 --- a/server/scripts/size-lint.mjs +++ b/server/scripts/size-lint.mjs @@ -10,7 +10,10 @@ * scripts/size-baseline.json with the length they had, and the check fails * when one of them grows past its entry. A file that needs to grow is split * by concern instead (trips into trips/trip-members/trip-membership/ - * trip-invite/trip-read-model/calendar is the precedent). + * trip-invite/trip-read-model/calendar is the precedent). An entry above what + * its file holds now, or for a file that is gone, fails as well until + * --update lowers it: otherwise the file could grow back unseen, and a new + * file at a deleted path would inherit its allowance. * * Lines are counted the way they read, not the way they are stored: a line * longer than LINE_WIDTH (prettier's printWidth) counts once per LINE_WIDTH @@ -111,11 +114,16 @@ export function lowerBaseline(baseline, counts) { return Object.fromEntries(Object.entries(lowered).sort(([a], [b]) => a.localeCompare(b))); } -/** Files over their allowance, and baseline entries the files have shrunk below. */ +/** + * Files over their allowance, and the baseline entries that allow more than + * their file holds now (shrunk, back under the limit, or gone): exactly the + * entries lowerBaseline would lower or drop. + */ export function compare(baseline, counts) { const grown = Object.entries(counts).filter(([file, n]) => n > Math.max(baseline[file] ?? 0, LIMIT)); - const lowerable = Object.entries(baseline).filter(([file, n]) => (counts[file] ?? 0) < n); - return { grown, lowerable }; + const lowered = lowerBaseline(baseline, counts); + const stale = Object.entries(baseline).filter(([file, n]) => lowered[file] !== n); + return { grown, stale }; } function main(argv) { @@ -131,7 +139,7 @@ function main(argv) { writeFileSync(baselinePath, JSON.stringify(baseline, null, 2) + '\n'); } - const { grown, lowerable } = compare(baseline, counts); + const { grown, stale } = compare(baseline, counts); for (const [file, n] of grown) { const entry = baseline[file]; console.error( @@ -140,13 +148,22 @@ function main(argv) { 'Split a concern into a service, helper or module of its own instead of growing the file.', ); } - if (lowerable.length && !update) { - console.log(`${lowerable.length} file(s) are shorter than their baseline now: run with --update to lower it.`); + for (const [file, entry] of stale) { + console.error( + `FAIL ${file} is held at ${entry} in scripts/size-baseline.json, ` + + (file in counts ? `but it has ${counts[file]} lines now.` : 'but the file is gone.'), + ); + } + if (stale.length) { + console.error( + 'Run npm run lint:size -- --update to lower the baseline with the change that made it smaller: ' + + 'an entry above the file lets it grow back unseen.', + ); } console.log( `size: ${Object.keys(counts).length} file(s), ${Object.keys(baseline).length} over ${LIMIT} lines held at their baseline`, ); - return grown.length ? 1 : 0; + return grown.length || stale.length ? 1 : 0; } // Compared by real path, so the check still runs when the script is started through a symlink. diff --git a/server/tests/unit/scripts/import-boundaries.test.ts b/server/tests/unit/scripts/import-boundaries.test.ts index 9e79455e6..1e8c9ba4f 100644 --- a/server/tests/unit/scripts/import-boundaries.test.ts +++ b/server/tests/unit/scripts/import-boundaries.test.ts @@ -170,7 +170,7 @@ describe('import-boundaries.mjs', () => { 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', () => { + 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', 'nest/a/a.service.ts': "import { h } from '../b/b.helpers';\nexport const a = h;\n", @@ -182,7 +182,10 @@ describe('import-boundaries.mjs', () => { 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(check.out).toContain( + 'FAIL domainInternals: z -> b/gone.ts is in scripts/import-boundaries-baseline.json but no longer occurs.', + ); + expect(check.out).toContain('npm run lint:boundaries -- --update'); expect(run(dir, '--update').status).toBe(1); const written: unknown = JSON.parse( @@ -191,6 +194,18 @@ describe('import-boundaries.mjs', () => { expect(written).toEqual({ ...EMPTY, domainInternals: ['a -> b/b.helpers.ts'] }); }); + it('BOUND-011: a stale entry alone fails, and passes once --update dropped it', () => { + const files = { 'nest/a/a.service.ts': 'export const a = 1;\n' }; + const dir = serverRoot(files, { ...EMPTY, domainCycles: ['a -> b', 'b -> a'] }); + const check = run(dir); + expect(check.status).toBe(1); + expect(check.out).toContain( + 'FAIL domainCycles: a -> b is in scripts/import-boundaries-baseline.json but no longer occurs.', + ); + expect(run(dir, '--update').status).toBe(0); + expect(run(dir).status).toBe(0); + }); + 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'); diff --git a/server/tests/unit/scripts/size-lint.test.ts b/server/tests/unit/scripts/size-lint.test.ts index 8a4e88508..08358ff4a 100644 --- a/server/tests/unit/scripts/size-lint.test.ts +++ b/server/tests/unit/scripts/size-lint.test.ts @@ -75,10 +75,28 @@ describe('size-lint.mjs', () => { expect(out).toContain('FAIL src/nest/big/big.service.ts: 1001 lines, the limit is 1000.'); }); - it('SIZE-003: fails when a baselined file grows past its entry, passes at or under it', () => { + it('SIZE-003: fails when a baselined file grows past its entry, passes at it', () => { expect(run(serverRoot({ 'src/a.ts': lines(1201) }, { 'src/a.ts': 1200 })).status).toBe(1); expect(run(serverRoot({ 'src/a.ts': lines(1200) }, { 'src/a.ts': 1200 })).status).toBe(0); - expect(run(serverRoot({ 'src/a.ts': lines(1100) }, { 'src/a.ts': 1200 })).out).toContain('run with --update'); + }); + + it('SIZE-010: fails an entry above its file, or for a file that is gone, until --update lowers it', () => { + const shrunk = run(serverRoot({ 'src/a.ts': lines(1100) }, { 'src/a.ts': 1200 })); + expect(shrunk.status).toBe(1); + expect(shrunk.out).toContain( + 'FAIL src/a.ts is held at 1200 in scripts/size-baseline.json, but it has 1100 lines now.', + ); + expect(shrunk.out).toContain('npm run lint:size -- --update'); + // Back under the limit, the entry would let it grow past the limit again. + expect(run(serverRoot({ 'src/a.ts': lines(900) }, { 'src/a.ts': 1200 })).status).toBe(1); + // A deleted file would hand its allowance to the next file at its path. + const gone = run(serverRoot({ 'src/b.ts': lines(1) }, { 'src/a.ts': 1200 })); + expect(gone.status).toBe(1); + expect(gone.out).toContain('FAIL src/a.ts is held at 1200 in scripts/size-baseline.json, but the file is gone.'); + + const dir = serverRoot({ 'src/a.ts': lines(1100) }, { 'src/a.ts': 1200, 'src/gone.ts': 1300 }); + expect(run(dir, '--update').status).toBe(0); + expect(run(dir).status).toBe(0); }); it('SIZE-004: counts a long line once per 120 columns, so joining lines buys nothing', () => {