From 8fb888c8a30520c268553303e226241cfe47ccf3 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 19:50:42 +0200 Subject: [PATCH] build(server): lint:service-http counts envelopes in every domain file and every spelling of a branch on one The ratchet only read *.service.ts for status literals and one branch shape in controllers and tools, so an envelope in a helper, impl or provider file, a branch written as result.error === 'not_found', and RPC handlers all went uncounted. It now counts the literals in every file under src/nest outside the HTTP layer, and comparisons, negations, optional chains and ternaries on a returned error in controllers, tools and RPC handlers. The baseline takes what the wider count finds. --- server/CLAUDE.md | 4 +- server/scripts/service-http-baseline.json | 16 +++++-- server/scripts/service-http-ratchet.mjs | 47 ++++++++++++++----- .../scripts/error-envelope-ratchets.test.ts | 37 +++++++++++++-- 4 files changed, 84 insertions(+), 20 deletions(-) diff --git a/server/CLAUDE.md b/server/CLAUDE.md index e8dd08b9f..7a597df51 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -32,7 +32,7 @@ npm run lint:test-mocks # vi.mock of src/config, src/nest/realtime and src/db npm run lint:test-new-service # new XService(/new XRepository( in tests/ may only shrink; zero-argument classes are allowed (CI gate) npm run lint:query-api # MikroORM QueryBuilder calls in the repositories may only shrink (CI gate) npm run lint:mcp-zod # inline z. calls per *.mcp.ts may only shrink; tool fields derive from @trek/shared (CI gate) -npm run lint:service-http # error-envelope idioms (status: 4xx in services, if (result.error) in callers) may only shrink (CI gate) +npm run lint:service-http # error-envelope idioms (status: 4xx in domain files, branches on result.error in callers) may only shrink (CI gate) npm run lint:response-contracts # route handlers without @ResponseContract may only shrink (CI gate) npm run contracts:dto-open # open shapes in the createZodDto schemas may only shrink (runs in the unit tests) ``` @@ -53,7 +53,7 @@ npm run contracts:dto-open # open shapes in the createZodDto schemas may only sh **`lint:query-api`** (`scripts/query-api-ratchet.mjs`) counts every `.qb(` and `createQueryBuilder(` under `src/db/repositories/` per file (the `TrekRepository` base that defines `qb` is left out) against `scripts/query-api-baseline.json`: a file may hold at most its entry, a file without one none, and an entry above what its file holds now fails until `npm run lint:query-api -- --update` lowers it (it never raises or adds one). The rule it holds is "Which query API" under SQL dialect below. -**`lint:mcp-zod`** (`scripts/mcp-inline-zod.mjs`) and **`lint:service-http`** (`scripts/service-http-ratchet.mjs`) share the per-file count rules of `scripts/lib/count-ratchet.mjs`: a file may hold at most its entry in its baseline (`scripts/mcp-inline-zod-baseline.json`, `scripts/service-http-baseline.json`), a file without one none, and an entry above what its file holds now fails until `-- --update` lowers it (never raises or adds). `lint:mcp-zod` counts every `z.` in a `*.mcp.ts`: a tool field derives from the request schema in `@trek/shared` or a shared primitive (`idSchema`); a bound REST deliberately lacks stays inline and counted. `lint:service-http` counts, in a `*.service.ts`, the `status: 4xx`/`5xx` literals of a returned `{ error, status }` and every `new HttpException(`, and in a `*.controller.ts` or `*.mcp.ts` every branch on a returned error (`if (result.error)`, `if ('error' in r)`). A service refuses by throwing a `DomainError` (`src/nest/common/domain-error.ts`) instead; `src/nest/README.md` ("Refusals") has the shape. +**`lint:mcp-zod`** (`scripts/mcp-inline-zod.mjs`) and **`lint:service-http`** (`scripts/service-http-ratchet.mjs`) share the per-file count rules of `scripts/lib/count-ratchet.mjs`: a file may hold at most its entry in its baseline (`scripts/mcp-inline-zod-baseline.json`, `scripts/service-http-baseline.json`), a file without one none, and an entry above what its file holds now fails until `-- --update` lowers it (never raises or adds). `lint:mcp-zod` counts every `z.` in a `*.mcp.ts`: a tool field derives from the request schema in `@trek/shared` or a shared primitive (`idSchema`); a bound REST deliberately lacks stays inline and counted. `lint:service-http` counts, in every file under `src/nest` but the HTTP layer (controllers, guards, pipes, filters, interceptors, DTOs and `domain-error.ts` itself), the `status: 4xx`/`5xx` literals of a returned `{ error, status }` and every `new HttpException(`, and in a `*.controller.ts`, `*.mcp.ts` or `*.rpc.ts` every branch on a returned error (`if (result.error)`, `if (!r.error)`, `'error' in r`, `r.error === 'not_found'`, `r?.error && ...`, `r.error ? a : b`). A service refuses by throwing a `DomainError` (`src/nest/common/domain-error.ts`) instead; `src/nest/README.md` ("Refusals") has the shape. **`lint:response-contracts`** (`scripts/response-contract-ratchet.mjs`) reads every class under `src/nest/` with the TypeScript parser and counts, per file, the route handlers (`@Get`/`@Post`/`@Put`/`@Patch`/`@Delete`/`@All`/`@Head`/`@Options`) that carry no `@ResponseContract(...)` against `scripts/response-contract-baseline.json`: a file may hold at most its entry, a file without one none, and an entry above what its file holds now fails until `npm run lint:response-contracts -- --update` lowers it (it never raises or adds one). A handler that writes through `@Res()` (a stream, a redirect) or passes a third party's answer through unchanged has no body of TREK's to check and is marked with a `response-contract-exempt: ` comment above it instead. The run prints the covered share. diff --git a/server/scripts/service-http-baseline.json b/server/scripts/service-http-baseline.json index 4b19bd7f2..f122288a3 100644 --- a/server/scripts/service-http-baseline.json +++ b/server/scripts/service-http-baseline.json @@ -5,10 +5,15 @@ "src/nest/auth/passkey.controller.ts": 1, "src/nest/auth/passkey.service.ts": 2, "src/nest/auth/registration-invites.service.ts": 1, - "src/nest/collab/collab.controller.ts": 1, + "src/nest/backup/backup-archive.ts": 8, + "src/nest/backup/backup.impl.ts": 1, + "src/nest/collab/collab.controller.ts": 7, "src/nest/collab/collab.mcp.ts": 3, + "src/nest/collab/collab.rpc.ts": 2, "src/nest/doc-sync/doc-sync-config.service.ts": 2, "src/nest/llm-parse/llm-local.service.ts": 7, + "src/nest/maps/maps-url.resolver.ts": 2, + "src/nest/maps/providers/osm.client.ts": 4, "src/nest/memories/immich.controller.ts": 6, "src/nest/memories/immich.service.ts": 15, "src/nest/memories/memories.mcp.ts": 6, @@ -17,17 +22,22 @@ "src/nest/memories/unified.controller.ts": 7, "src/nest/notifications/push/push.controller.ts": 1, "src/nest/oauth/oauth-api.controller.ts": 4, - "src/nest/oauth/oauth-public.controller.ts": 1, + "src/nest/oauth/oauth-public.controller.ts": 3, "src/nest/oauth/oauth.service.ts": 22, - "src/nest/oidc/oidc.controller.ts": 3, + "src/nest/oidc/oidc.controller.ts": 4, "src/nest/oidc/oidc.service.ts": 1, "src/nest/photos/photos.controller.ts": 1, "src/nest/place-import/place-import.service.ts": 1, + "src/nest/place-import/providers/google-directions.provider.ts": 5, + "src/nest/place-import/providers/google-list.provider.ts": 12, + "src/nest/place-import/providers/naver-list.provider.ts": 10, "src/nest/plugins/contributions/plugin-pois.controller.ts": 1, "src/nest/plugins/contributions/plugin-pois.mcp.ts": 1, "src/nest/plugins/contributions/plugin-pois.service.ts": 4, + "src/nest/public-api/public-api-request.ts": 3, "src/nest/reservations/reservations.mcp.ts": 4, "src/nest/tokens/token.service.ts": 2, + "src/nest/vacay/vacay.controller.ts": 1, "src/nest/vacay/vacay.mcp.ts": 1 } } diff --git a/server/scripts/service-http-ratchet.mjs b/server/scripts/service-http-ratchet.mjs index a02b99464..4182ac2a4 100644 --- a/server/scripts/service-http-ratchet.mjs +++ b/server/scripts/service-http-ratchet.mjs @@ -7,11 +7,15 @@ * into REST's `{ error }` and the MCP registry into the tool's errorResult, so no * caller translates it. What this counts is the older way, where every caller did: * - * - in a *.service.ts: a `status: 4xx` or `status: 5xx` literal (the - * `return { error, status }` result) and `new HttpException(` (HTTP in the - * domain layer); - * - in a *.controller.ts or *.mcp.ts: a branch on a returned error, - * `if (result.error)`, `if ('error' in result)`. + * - in any file under src/nest outside the HTTP and adapter layers (services, + * helpers, impl files, providers): a `status: 4xx` or `status: 5xx` literal + * (the `return { error, status }` result) and `new HttpException(` (HTTP in + * the domain layer). Controllers, guards, pipes, filters, interceptors and + * DTOs speak HTTP by design, and src/nest/common/domain-error.ts defines the + * status a refusal carries, so those are left out by name; + * - in a *.controller.ts, *.mcp.ts or *.rpc.ts: a branch on a returned error, + * `if (result.error)`, `if (!r.error)`, `'error' in result`, + * `result.error === 'not_found'`, `r?.error && ...`, `x.error ? a : b`. * * Each file is held to its entry in scripts/service-http-baseline.json, and a * file without an entry holds none (scripts/lib/count-ratchet.mjs has the rules). @@ -23,21 +27,42 @@ import { cliMain } from './lib/count-ratchet.mjs'; const STATUS_LITERAL = /\bstatus:\s*[45]\d\d\b/g; const HTTP_EXCEPTION = /\bnew HttpException\(/g; -const RESULT_ERROR_BRANCH = - /\bif\s*\(\s*(?:'error'\s+in\s+[\w$.]+|"error"\s+in\s+[\w$.]+|[\w$]+(?:\.[\w$]+)*\.error\s*\))/g; +// A property path such as result, res.body or this.state?.last. +const PATH = String.raw`[\w$]+(?:\??\.[\w$]+)*`; +const RESULT_ERROR_BRANCH = new RegExp( + [ + String.raw`['"]error['"]\s+in\s+[\w$.]+`, + String.raw`\bif\s*\(\s*!?\s*${PATH}\??\.error\s*\)`, + String.raw`${PATH}\??\.error\b\s*(?:===|!==|==|!=|&&|\|\||\?(?![.?]))`, + ].join('|'), + 'g', +); + +/** Files that speak HTTP by design: the status literals in them are not an envelope. */ +const HTTP_LAYER = /\.(?:controller|guard|pipe|filter|interceptor|dto)\.ts$/; +const DEFINES_THE_STATUS = new Set(['src/nest/common/domain-error.ts']); +/** The adapters over a service, where a branch on its returned error is the caller translating it. */ +const ADAPTER = /\.(?:controller|mcp|rpc)\.ts$/; const hits = (text, pattern) => text.match(pattern)?.length ?? 0; -/** The envelope idioms in one service file. */ +/** The envelope idioms in one domain-layer file. */ export function countServiceIdioms(text) { return hits(text, STATUS_LITERAL) + hits(text, HTTP_EXCEPTION); } -/** The branches on a returned error in one controller or MCP file. */ +/** The branches on a returned error in one controller, MCP or RPC file. */ export function countResultBranches(text) { return hits(text, RESULT_ERROR_BRANCH); } +/** What one file under src/nest holds, by the layer its name puts it in. */ +export function countFile(text, file) { + let n = ADAPTER.test(file) ? countResultBranches(text) : 0; + if (!HTTP_LAYER.test(file) && !DEFINES_THE_STATUS.has(file)) n += countServiceIdioms(text); + return n; +} + export const check = { name: 'service-http', script: 'lint:service-http', @@ -45,8 +70,8 @@ export const check = { baseline: 'scripts/service-http-baseline.json', unit: 'error-envelope idiom(s)', advice: 'Throw a DomainError from the service (src/nest/common/domain-error.ts) and let REST and MCP answer it.', - include: (file) => /\.(service|controller|mcp)\.ts$/.test(file), - count: (text, file) => (file.endsWith('.service.ts') ? countServiceIdioms(text) : countResultBranches(text)), + include: (file) => file.endsWith('.ts') && !file.endsWith('.d.ts'), + count: countFile, }; cliMain(check, import.meta.url); diff --git a/server/tests/unit/scripts/error-envelope-ratchets.test.ts b/server/tests/unit/scripts/error-envelope-ratchets.test.ts index 796a1b654..966a8bd70 100644 --- a/server/tests/unit/scripts/error-envelope-ratchets.test.ts +++ b/server/tests/unit/scripts/error-envelope-ratchets.test.ts @@ -5,7 +5,7 @@ * on this repository, so the committed baselines are checked by the unit suite. */ import { countInlineZod } from '../../../scripts/mcp-inline-zod.mjs'; -import { countResultBranches, countServiceIdioms } from '../../../scripts/service-http-ratchet.mjs'; +import { countFile, countResultBranches, countServiceIdioms } from '../../../scripts/service-http-ratchet.mjs'; import { execFileSync } from 'node:child_process'; import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; @@ -83,6 +83,33 @@ describe('the counters', () => { ].join('\n'); expect(countResultBranches(text)).toBe(4); }); + + it('ENV-RATCHET-004: counts a comparison, a negation, an optional chain and a ternary on a returned error', () => { + const text = [ + "if (result.error === 'not_found') {", + 'if (!r.error) return ok(r);', + 'if (res?.error && res.status) {', + 'return outcome.error ? fail() : pass();', + "const notFound = r.error !== 'gone';", + 'logger.error(r);', + 'const e = res.error;', + 'if (r.errors) {', + ].join('\n'); + expect(countResultBranches(text)).toBe(5); + }); + + it('ENV-RATCHET-005: counts the envelope in any domain file, not in the HTTP layer or the DomainError itself', () => { + const envelope = `return { error: 'x', ${STATUS} };`; + for (const file of ['a.helpers.ts', 'a.impl.ts', 'providers/x.provider.ts', 'a.service.ts']) { + expect(countFile(envelope, `src/nest/a/${file}`)).toBe(1); + } + for (const file of ['a.controller.ts', 'a.guard.ts', 'a.pipe.ts', 'a.filter.ts', 'a.interceptor.ts', 'a.dto.ts']) { + expect(countFile(envelope, `src/nest/a/${file}`)).toBe(0); + } + expect(countFile(envelope, 'src/nest/common/domain-error.ts')).toBe(0); + expect(countFile('if (r.error) {', 'src/nest/a/a.rpc.ts')).toBe(1); + expect(countFile('if (r.error) {', 'src/nest/a/a.helpers.ts')).toBe(0); + }); }); describe('mcp-inline-zod.mjs', () => { @@ -145,24 +172,26 @@ describe('mcp-inline-zod.mjs', () => { describe('service-http-ratchet.mjs', () => { const BASE = 'service-http-baseline.json'; - it('ENV-RATCHET-020: counts services, controllers and tools, nothing else', () => { + it('ENV-RATCHET-020: counts domain files, controllers, tools and RPC handlers, not the HTTP layer', () => { const dir = serverRoot( { [`${NEST}/a/a.service.ts`]: `return { error: 'x', ${STATUS} };\n`, [`${NEST}/a/a.controller.ts`]: 'if (result.error) {\n', [`${NEST}/a/a.mcp.ts`]: "if ('error' in r) return x;\n", + [`${NEST}/a/a.rpc.ts`]: "if (r.error === 'nope') throw x;\n", [`${NEST}/a/a.helpers.ts`]: `return { error: 'x', ${STATUS} };\n`, + [`${NEST}/a/a.guard.ts`]: `throw ${HTTP}'no', 403);\n`, }, BASE, ); const { status, out } = run('service-http-ratchet.mjs', dir); expect(status).toBe(1); - for (const file of ['a.service.ts', 'a.controller.ts', 'a.mcp.ts']) { + expect(out).not.toContain('a.guard.ts'); + for (const file of ['a.service.ts', 'a.controller.ts', 'a.mcp.ts', 'a.rpc.ts', 'a.helpers.ts']) { expect(out).toContain( `FAIL ${NEST}/a/${file}: 1 error-envelope idiom(s), a file without a baseline entry may hold none.`, ); } - expect(out).not.toContain('a.helpers.ts'); expect(out).toContain('Throw a DomainError'); });