From a258d1e89bb2a906fcd2de508ffbf33f4378eec0 Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 01:21:40 +0900 Subject: [PATCH 01/13] fix: validate generated REST contracts --- .changeset/contract-graph-check.md | 9 + packages/cli/src/bin/croco.ts | 2 + packages/cli/src/commands/contracts.ts | 16 + packages/cli/src/commands/contractsCheck.ts | 69 ++++ packages/cli/src/index.ts | 2 + packages/cli/src/tests/contractsCheck.spec.ts | 69 ++++ .../src/tests/templates-build.spec.ts | 1 + .../templates/spa-be-split/README.md.hbs | 4 +- .../templates/spa-be-split/package.json.hbs | 1 + packages/openapi-spec/src/index.ts | 1 + packages/openapi-spec/src/libs/emitOpenAPI.ts | 43 ++- .../src/tests/emitOpenAPI.spec.ts | 47 ++- packages/protocols-core/src/index.ts | 17 + .../protocols-core/src/libs/ContractGraph.ts | 330 ++++++++++++++++++ .../src/tests/ContractGraph.spec.ts | 116 ++++++ packages/rpc-codegen/src/index.ts | 2 +- packages/rpc-codegen/src/libs/cli.ts | 66 +++- packages/rpc-codegen/src/libs/generate.ts | 62 +++- packages/rpc-codegen/src/libs/loadRoutes.ts | 17 +- packages/rpc-codegen/src/tests/Cli.spec.ts | 108 +++++- .../src/tests/ContractCheckCli.spec.ts | 100 ++++++ .../rpc-codegen/src/tests/codegen.spec.ts | 68 ++++ .../rpc-codegen/src/tests/loadRoutes.spec.ts | 26 +- scripts/create-croco-app-generated-smoke.mts | 1 + 24 files changed, 1145 insertions(+), 32 deletions(-) create mode 100644 .changeset/contract-graph-check.md create mode 100644 packages/cli/src/commands/contracts.ts create mode 100644 packages/cli/src/commands/contractsCheck.ts create mode 100644 packages/cli/src/tests/contractsCheck.spec.ts create mode 100644 packages/protocols-core/src/libs/ContractGraph.ts create mode 100644 packages/protocols-core/src/tests/ContractGraph.spec.ts create mode 100644 packages/rpc-codegen/src/tests/ContractCheckCli.spec.ts diff --git a/.changeset/contract-graph-check.md b/.changeset/contract-graph-check.md new file mode 100644 index 000000000..b24689e4a --- /dev/null +++ b/.changeset/contract-graph-check.md @@ -0,0 +1,9 @@ +--- +"@croco/protocols-core": patch +"@croco/openapi-spec": patch +"@croco/rpc-codegen": patch +"@croco/cli": patch +"create-croco-app": patch +--- + +Expose a canonical REST contract graph with route diagnostics and add a contract check path before OpenAPI and RPC client generation. diff --git a/packages/cli/src/bin/croco.ts b/packages/cli/src/bin/croco.ts index 7f5fc9e7b..0dbbc20a8 100644 --- a/packages/cli/src/bin/croco.ts +++ b/packages/cli/src/bin/croco.ts @@ -1,6 +1,7 @@ #!/usr/bin/env node import { defineCommand, runMain } from "citty"; import { codegen } from "../commands/codegen.js"; +import { contracts } from "../commands/contracts.js"; import { create } from "../commands/create.js"; import { generate } from "../commands/generate.js"; import { make } from "../commands/make.js"; @@ -20,6 +21,7 @@ const main = defineCommand({ create, generate, codegen, + contracts, migrate, }, }); diff --git a/packages/cli/src/commands/contracts.ts b/packages/cli/src/commands/contracts.ts new file mode 100644 index 000000000..630c78f9e --- /dev/null +++ b/packages/cli/src/commands/contracts.ts @@ -0,0 +1,16 @@ +import { defineCommand } from "citty"; +import { contractsCheck } from "./contractsCheck.js"; +import { GLOBAL_OPTIONS } from "./options.js"; + +export const contracts = defineCommand({ + meta: { + name: "contracts", + description: "Validate Croco contract graph artifacts", + }, + args: { + ...GLOBAL_OPTIONS, + }, + subCommands: { + check: contractsCheck, + }, +}); diff --git a/packages/cli/src/commands/contractsCheck.ts b/packages/cli/src/commands/contractsCheck.ts new file mode 100644 index 000000000..a69823b11 --- /dev/null +++ b/packages/cli/src/commands/contractsCheck.ts @@ -0,0 +1,69 @@ +import { type ChildProcess, type SpawnOptions, spawn } from "node:child_process"; +import { createRequire } from "node:module"; +import { basename, dirname, join } from "node:path"; +import { defineCommand } from "citty"; +import { GLOBAL_OPTIONS } from "./options.js"; + +const require = createRequire(import.meta.url); + +export type ContractsCheckSpawn = ( + command: string, + args: string[], + options: SpawnOptions, +) => ChildProcess; + +export const contractsCheck = defineCommand({ + meta: { + name: "check", + description: "Validate the canonical contract graph without generating artifacts", + }, + args: { + ...GLOBAL_OPTIONS, + }, + run({ rawArgs }) { + runContractsCheck(rawArgs); + }, +}); + +export function runContractsCheck( + args: string[], + options: { + readonly resolveBin?: () => string; + readonly spawn?: ContractsCheckSpawn; + readonly setExitCode?: (code: number) => void; + readonly writeError?: (message: string) => void; + } = {}, +): void { + const resolveBin = options.resolveBin ?? resolveRpcCodegenBin; + const spawnChild = options.spawn ?? spawn; + const setExitCode = + options.setExitCode ?? + ((code: number) => { + process.exitCode = code; + }); + const writeError = options.writeError ?? ((message: string) => console.error(message)); + const child = spawnChild(process.execPath, [resolveBin(), "--check", ...args], { + stdio: "inherit", + }); + + child.on("exit", (code) => { + setExitCode(code ?? 1); + }); + + child.on("error", (error) => { + writeError(error.message); + setExitCode(1); + }); +} + +export function resolveRpcCodegenBin(): string { + return resolveRpcCodegenBinFromEntry(require.resolve("@croco/rpc-codegen")); +} + +export function resolveRpcCodegenBinFromEntry(entry: string): string { + const entryDir = dirname(entry); + + return basename(entryDir) === "src" + ? join(dirname(entryDir), "dist", "cli.js") + : join(entryDir, "cli.js"); +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index d66a5cd70..1a8c74409 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -13,6 +13,8 @@ export { confirmOverwrite, selectMode, textInput, NoTtyError } from "./libs/prom export { codegen } from "./commands/codegen"; export { codegenOpenapi } from "./commands/codegenOpenapi"; export { codegenRpc } from "./commands/codegenRpc"; +export { contracts } from "./commands/contracts"; +export { contractsCheck, runContractsCheck } from "./commands/contractsCheck"; export { create } from "./commands/create"; export { createDomain } from "./commands/createDomain"; export { createPage } from "./commands/createPage"; diff --git a/packages/cli/src/tests/contractsCheck.spec.ts b/packages/cli/src/tests/contractsCheck.spec.ts new file mode 100644 index 000000000..418b83114 --- /dev/null +++ b/packages/cli/src/tests/contractsCheck.spec.ts @@ -0,0 +1,69 @@ +import type { ChildProcess, SpawnOptions } from "node:child_process"; +import { EventEmitter } from "node:events"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { + type ContractsCheckSpawn, + resolveRpcCodegenBinFromEntry, + runContractsCheck, +} from "../commands/contractsCheck.js"; + +describe("contractsCheck", () => { + it("should resolve a workspace source RPC package entry to the built RPC CLI", () => { + const root = join("workspace", "packages", "rpc-codegen"); + + expect(resolveRpcCodegenBinFromEntry(join(root, "src", "index.ts"))).toBe( + join(root, "dist", "cli.js"), + ); + }); + + it("should spawn the RPC check mode with forwarded args and preserve its exit code", () => { + const child = new EventEmitter() as unknown as ChildProcess; + const calls: SpawnCall[] = []; + const exitCodes: number[] = []; + const spawnCheck: ContractsCheckSpawn = (command, args, options) => { + calls.push({ command, args, options }); + return child; + }; + + runContractsCheck(["--controllers", "src/**/*.ts"], { + resolveBin: () => "/pkg/dist/cli.js", + spawn: spawnCheck, + setExitCode: (code) => exitCodes.push(code), + }); + child.emit("exit", 7); + + expect(calls).toEqual([ + { + command: process.execPath, + args: ["/pkg/dist/cli.js", "--check", "--controllers", "src/**/*.ts"], + options: { stdio: "inherit" }, + }, + ]); + expect(exitCodes).toEqual([7]); + }); + + it("should report spawn errors as command failures", () => { + const child = new EventEmitter() as unknown as ChildProcess; + const errors: string[] = []; + const exitCodes: number[] = []; + const spawnCheck: ContractsCheckSpawn = () => child; + + runContractsCheck([], { + resolveBin: () => "/pkg/dist/cli.js", + spawn: spawnCheck, + setExitCode: (code) => exitCodes.push(code), + writeError: (message) => errors.push(message), + }); + child.emit("error", new Error("spawn failed")); + + expect(errors).toEqual(["spawn failed"]); + expect(exitCodes).toEqual([1]); + }); +}); + +type SpawnCall = { + readonly command: string; + readonly args: string[]; + readonly options: SpawnOptions; +}; diff --git a/packages/create-croco-app/src/tests/templates-build.spec.ts b/packages/create-croco-app/src/tests/templates-build.spec.ts index 6ad705311..94bdd2d30 100644 --- a/packages/create-croco-app/src/tests/templates-build.spec.ts +++ b/packages/create-croco-app/src/tests/templates-build.spec.ts @@ -84,6 +84,7 @@ function checkSpaBeSplitStructure() { scripts: expect.objectContaining({ "dev:api": expect.any(String), "dev:web": expect.any(String), + "contract:check": expect.stringContaining("--check"), "contract:openapi": expect.stringContaining("croco-openapi-spec"), "contract:client": expect.stringContaining("croco-rpc-codegen"), codegen: expect.any(String), diff --git a/packages/create-croco-app/templates/spa-be-split/README.md.hbs b/packages/create-croco-app/templates/spa-be-split/README.md.hbs index f39b73ee5..6d5ae7741 100644 --- a/packages/create-croco-app/templates/spa-be-split/README.md.hbs +++ b/packages/create-croco-app/templates/spa-be-split/README.md.hbs @@ -12,18 +12,20 @@ pnpm dev:web - API 서버: `http://localhost:3000` - SPA 개발 서버: Vite 기본 포트 +- 계약 그래프 검증: `pnpm contract:check` - OpenAPI 문서 생성: `pnpm contract:openapi` - RPC 클라이언트 생성: `pnpm contract:client` 또는 `pnpm codegen` ## 코드 생성 ```bash +pnpm contract:check pnpm contract:openapi pnpm contract:client pnpm codegen ``` -`contract:openapi`는 `apps/api-server/src/controllers/**/*.ts`에서 REST 컨트롤러 메타데이터를 읽어 `openapi.json`을 생성합니다. `contract:client`는 같은 컨트롤러 계약에서 React Query hook을 포함한 fetch 클라이언트를 `libs/shared/provider-rpc/src`에 생성합니다. `codegen`은 기존 사용자를 위한 `contract:client` 별칭입니다. +`contract:check`는 생성 전에 canonical contract graph 진단을 실행합니다. `contract:openapi`는 `apps/api-server/src/controllers/**/*.ts`에서 REST 컨트롤러 메타데이터를 읽어 `openapi.json`을 생성합니다. `contract:client`는 같은 컨트롤러 계약에서 React Query hook을 포함한 fetch 클라이언트를 `libs/shared/provider-rpc/src`에 생성합니다. `codegen`은 기존 사용자를 위한 `contract:client` 별칭입니다. ## 구조 diff --git a/packages/create-croco-app/templates/spa-be-split/package.json.hbs b/packages/create-croco-app/templates/spa-be-split/package.json.hbs index 9f8e981da..78fdf03e7 100644 --- a/packages/create-croco-app/templates/spa-be-split/package.json.hbs +++ b/packages/create-croco-app/templates/spa-be-split/package.json.hbs @@ -5,6 +5,7 @@ "scripts": { "dev:api": "pnpm --filter {{scope}}/api-server dev", "dev:web": "pnpm --filter {{scope}}/console-web dev", + "contract:check": "croco-rpc-codegen --controllers 'apps/api-server/src/controllers/**/*.ts' --check", "contract:openapi": "croco-openapi-spec --controllers 'apps/api-server/src/controllers/**/*.ts' --out openapi.json --title '{{projectName}} API' --version 0.1.0 --server http://localhost:3000", "contract:client": "croco-rpc-codegen --controllers 'apps/api-server/src/controllers/**/*.ts' --out libs/shared/provider-rpc/src --react-query", "codegen": "pnpm contract:client", diff --git a/packages/openapi-spec/src/index.ts b/packages/openapi-spec/src/index.ts index 0dcd234a2..d7a54dca6 100644 --- a/packages/openapi-spec/src/index.ts +++ b/packages/openapi-spec/src/index.ts @@ -1,5 +1,6 @@ export { emitOpenAPI, + emitOpenAPIFromContractGraph, type EmitOpenAPIOptions, type ProblemResponseConfig, } from "./libs/emitOpenAPI"; diff --git a/packages/openapi-spec/src/libs/emitOpenAPI.ts b/packages/openapi-spec/src/libs/emitOpenAPI.ts index 35b111ac7..7401f3341 100644 --- a/packages/openapi-spec/src/libs/emitOpenAPI.ts +++ b/packages/openapi-spec/src/libs/emitOpenAPI.ts @@ -4,7 +4,13 @@ import { OpenApiGeneratorV31, type RouteConfig, } from "@asteasolutions/zod-to-openapi"; -import { extractRouteIR, type ParamIR, type RouteIR } from "@croco/protocols-core"; +import { + assertContractGraphHasNoErrors, + buildContractGraph, + type ContractGraph, + type ContractGraphRoute, + type ParamIR, +} from "@croco/protocols-core"; import { type ZodType, z } from "zod"; extendZodWithOpenApi(z); @@ -56,10 +62,20 @@ export function emitOpenAPI( controllers: Function[], options: EmitOpenAPIOptions = {}, ): OpenAPIDocument { - const registry = new OpenAPIRegistry(); - const routes = controllers.flatMap((controller) => - extractRouteIR(controller as ControllerConstructor), + return emitOpenAPIFromContractGraph( + buildContractGraph(controllers as ControllerConstructor[]), + options, ); +} + +export function emitOpenAPIFromContractGraph( + graph: ContractGraph, + options: EmitOpenAPIOptions = {}, +): OpenAPIDocument { + assertContractGraphHasNoErrors(graph); + + const registry = new OpenAPIRegistry(); + const routes = [...graph.routes]; const problemDetailsRef = registerProblemDetailsSchema(registry); const defaultResponses = toDefaultResponses(options, problemDetailsRef); @@ -166,19 +182,22 @@ function toProblemResponseConfig( ); } -function toRouteConfig(route: RouteIR, defaultResponses: RouteResponses): RouteConfig { +function toRouteConfig(route: ContractGraphRoute, defaultResponses: RouteResponses): RouteConfig { return { method: toHttpMethod(route), path: toOpenAPIPath(route.path), - operationId: `${route.controllerName}_${route.methodName}`, - summary: `${route.controllerName}.${route.methodName}`, + operationId: route.operationId, + summary: route.routeId, tags: [route.domain ?? route.controllerName], responses: toResponseConfig(route, defaultResponses), ...(route.params.length > 0 || route.inputSchema ? { request: toRequestConfig(route) } : {}), }; } -function toResponseConfig(route: RouteIR, defaultResponses: RouteResponses): RouteResponses { +function toResponseConfig( + route: ContractGraphRoute, + defaultResponses: RouteResponses, +): RouteResponses { const outputSchema = unwrapZodEffects(route.outputSchema); return { @@ -198,7 +217,7 @@ function toResponseConfig(route: RouteIR, defaultResponses: RouteResponses): Rou }; } -function toTags(routes: RouteIR[]): { name: string; description: string }[] { +function toTags(routes: ContractGraphRoute[]): { name: string; description: string }[] { const tagNames = new Set(routes.map((route) => route.domain ?? route.controllerName)); return [...tagNames].map((name) => ({ @@ -207,7 +226,7 @@ function toTags(routes: RouteIR[]): { name: string; description: string }[] { })); } -function toRequestConfig(route: RouteIR): RouteConfig["request"] { +function toRequestConfig(route: ContractGraphRoute): RouteConfig["request"] { const params = toZodObject(route.params.filter((param) => param.kind === "path")); const query = toZodObject(route.params.filter((param) => param.kind === "query")); const headers = toZodObject(route.params.filter((param) => param.kind === "header")); @@ -279,7 +298,7 @@ function toOpenAPIPath(path: string): string { return path.replace(/:([^/]+)/g, "{$1}"); } -function toHttpMethod(route: RouteIR): HttpMethod { +function toHttpMethod(route: ContractGraphRoute): HttpMethod { const method = route.httpMethod; const normalizedMethod = method.toLowerCase(); const httpMethod = HTTP_METHODS.find((candidate) => candidate === normalizedMethod); @@ -297,6 +316,6 @@ function toHttpMethod(route: RouteIR): HttpMethod { throw new Error(`Unsupported HTTP method: ${method}`); } -function formatRoute(route: RouteIR): string { +function formatRoute(route: ContractGraphRoute): string { return `${route.controllerName}.${route.methodName} (${route.path})`; } diff --git a/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts b/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts index eadf5ee8b..29286128c 100644 --- a/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts +++ b/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts @@ -15,9 +15,10 @@ import { ResponseSchema, All, } from "@croco/protocols-rest"; +import { buildContractGraph } from "@croco/protocols-core"; import { describe, expect, it } from "vitest"; import { z } from "zod"; -import { emitOpenAPI } from "../libs/emitOpenAPI"; +import { emitOpenAPI, emitOpenAPIFromContractGraph } from "../libs/emitOpenAPI"; describe("emitOpenAPI", () => { it("should emit a GET operation with a path parameter", () => { @@ -45,6 +46,21 @@ describe("emitOpenAPI", () => { }); }); + it("should consume the canonical contract graph as its source of truth", () => { + @Controller("/users") + class UsersController { + @Get("/:id") + getUser(@Param("id") _id: string): void {} + } + + const graph = buildContractGraph([UsersController]); + const spec = emitOpenAPIFromContractGraph(graph); + + expect(graph.routes[0]?.routeId).toBe("UsersController.getUser"); + expect(spec.paths?.["/users/{id}"]?.get?.operationId).toBe("UsersController_getUser"); + expect(spec.paths?.["/users/{id}"]?.get?.summary).toBe("UsersController.getUser"); + }); + it("should apply document metadata options", () => { @Controller("/accounts") class AccountsController { @@ -275,7 +291,34 @@ describe("emitOpenAPI", () => { } expect(() => emitOpenAPI([HooksController])).toThrow( - "Cannot emit OpenAPI operation for @All route HooksController.handleHook (/hooks/:id): @All is runtime-only and cannot be represented as a concrete OpenAPI operation. Use explicit HTTP method decorators for generated contracts.", + "ERROR contract-route-unsupported-all-method HooksController.handleHook: @All is runtime-only and cannot be represented as a concrete generated contract. Use explicit HTTP method decorators for OpenAPI and typed clients.", + ); + }); + + it("should reject path parameters that drift from controller metadata", () => { + @Controller("/users") + class UsersController { + @Get("/:id") + getUser(@Param("userId") _userId: string): void {} + } + + expect(() => emitOpenAPI([UsersController])).toThrow( + "ERROR contract-route-missing-path-param UsersController.getUser: Route path declares ':id' but no @Param(\"id\") metadata was found.", + ); + }); + + it("should reject routes with more than one body parameter", () => { + @Controller("/users") + class UsersController { + @Post("/") + createUser( + @Body(z.object({ name: z.string() })) _body: { name: string }, + @Body(z.object({ auditId: z.string() })) _audit: { auditId: string }, + ): void {} + } + + expect(() => emitOpenAPI([UsersController])).toThrow( + "ERROR contract-route-multiple-body-params UsersController.createUser: Generated contracts support one request body per route, but 2 @Body() parameters were found.", ); }); diff --git a/packages/protocols-core/src/index.ts b/packages/protocols-core/src/index.ts index 7b1679d1e..f496fbe78 100644 --- a/packages/protocols-core/src/index.ts +++ b/packages/protocols-core/src/index.ts @@ -2,6 +2,23 @@ export { discoverControllerConstructors, isControllerConstructor, } from "./libs/controllerDiscovery"; +export { + assertContractGraphHasNoErrors, + buildContractGraph, + ContractGraphDiagnosticError, + formatContractDiagnostic, + formatContractDiagnostics, + getContractGraphErrors, +} from "./libs/ContractGraph"; export { extractRouteIR } from "./libs/extractRouteIR"; +export type { + ContractDiagnostic, + ContractDiagnosticSeverity, + ContractDiagnosticTarget, + ContractGraph, + ContractGraphController, + ContractGraphRoute, + ContractGraphVersion, +} from "./libs/ContractGraph"; export type { ParamIR, RouteIR } from "./libs/RouteIR"; export type { Constructor } from "./libs/sharedTypes"; diff --git a/packages/protocols-core/src/libs/ContractGraph.ts b/packages/protocols-core/src/libs/ContractGraph.ts new file mode 100644 index 000000000..83f0ac897 --- /dev/null +++ b/packages/protocols-core/src/libs/ContractGraph.ts @@ -0,0 +1,330 @@ +import "reflect-metadata"; +import type { z } from "zod"; +import { extractRouteIR } from "./extractRouteIR"; +import type { RouteIR } from "./RouteIR"; +import { type Constructor, type ControllerMetadata, REST_CONTROLLER_KEY } from "./sharedTypes"; + +export type ContractGraphVersion = "croco.contract-graph.v1"; +export type ContractDiagnosticSeverity = "error" | "warning"; +export type ContractDiagnosticTarget = "graph" | "controller" | "route" | "param" | "schema"; + +export type ContractDiagnostic = { + readonly code: string; + readonly severity: ContractDiagnosticSeverity; + readonly target: ContractDiagnosticTarget; + readonly message: string; + readonly routeId?: string; + readonly controllerName?: string; + readonly methodName?: string; + readonly path?: string; +}; + +export type ContractGraphController = { + readonly name: string; + readonly path: string; + readonly routeIds: readonly string[]; +}; + +export type ContractGraphRoute = RouteIR & { + readonly routeId: string; + readonly operationId: string; + readonly controllerPath: string; +}; + +export type ContractGraph = { + readonly version: ContractGraphVersion; + readonly controllers: readonly ContractGraphController[]; + readonly routes: readonly ContractGraphRoute[]; + readonly diagnostics: readonly ContractDiagnostic[]; +}; + +export class ContractGraphDiagnosticError extends Error { + readonly diagnostics: readonly ContractDiagnostic[]; + + constructor(diagnostics: readonly ContractDiagnostic[]) { + super(formatContractDiagnostics(diagnostics)); + this.name = "ContractGraphDiagnosticError"; + this.diagnostics = diagnostics; + } +} + +export function buildContractGraph(controllers: readonly Constructor[]): ContractGraph { + const graphControllers: ContractGraphController[] = []; + const graphRoutes: ContractGraphRoute[] = []; + const diagnostics: ContractDiagnostic[] = []; + + for (const controller of controllers) { + const controllerMeta = Reflect.getMetadata(REST_CONTROLLER_KEY, controller) as + | ControllerMetadata + | undefined; + + if (!controllerMeta) { + continue; + } + + const routes = extractRouteIR(controller).map((route) => + toContractGraphRoute(route, controllerMeta.path), + ); + + graphControllers.push({ + name: controller.name, + path: controllerMeta.path, + routeIds: routes.map((route) => route.routeId), + }); + graphRoutes.push(...routes); + + for (const route of routes) { + diagnostics.push(...validateRoute(route)); + } + } + + diagnostics.push(...validateUniqueRouteIds(graphRoutes)); + + return { + version: "croco.contract-graph.v1", + controllers: graphControllers, + routes: graphRoutes, + diagnostics, + }; +} + +export function getContractGraphErrors(graph: ContractGraph): readonly ContractDiagnostic[] { + return graph.diagnostics.filter((diagnostic) => diagnostic.severity === "error"); +} + +export function assertContractGraphHasNoErrors(graph: ContractGraph): void { + const errors = getContractGraphErrors(graph); + + if (errors.length > 0) { + throw new ContractGraphDiagnosticError(errors); + } +} + +export function formatContractDiagnostics(diagnostics: readonly ContractDiagnostic[]): string { + return diagnostics.map(formatContractDiagnostic).join("\n"); +} + +export function formatContractDiagnostic(diagnostic: ContractDiagnostic): string { + const route = diagnostic.routeId ? ` ${diagnostic.routeId}` : ""; + + return `${diagnostic.severity.toUpperCase()} ${diagnostic.code}${route}: ${diagnostic.message}`; +} + +function toContractGraphRoute(route: RouteIR, controllerPath: string): ContractGraphRoute { + const routeId = `${route.controllerName}.${route.methodName}`; + + return { + ...route, + routeId, + operationId: routeId.replace(/[^A-Za-z0-9_]+/g, "_"), + controllerPath, + }; +} + +function validateRoute(route: ContractGraphRoute): ContractDiagnostic[] { + const diagnostics: ContractDiagnostic[] = []; + + if (route.httpMethod.toUpperCase() === "ALL") { + diagnostics.push( + createRouteDiagnostic( + route, + "contract-route-unsupported-all-method", + "error", + "@All is runtime-only and cannot be represented as a concrete generated contract. Use explicit HTTP method decorators for OpenAPI and typed clients.", + ), + ); + } + + diagnostics.push(...validatePathParams(route)); + diagnostics.push(...validateNamedParams(route)); + diagnostics.push(...validateBodyParams(route)); + diagnostics.push(...validateSchemaEffects(route)); + + return diagnostics; +} + +function validatePathParams(route: ContractGraphRoute): ContractDiagnostic[] { + const diagnostics: ContractDiagnostic[] = []; + const pathParamNames = new Set(getPathParamNames(route.path)); + const declaredParamNames = new Set( + route.params.filter((param) => param.kind === "path").map((param) => param.name), + ); + + for (const name of pathParamNames) { + if (!declaredParamNames.has(name)) { + diagnostics.push( + createRouteDiagnostic( + route, + "contract-route-missing-path-param", + "error", + `Route path declares ':${name}' but no @Param("${name}") metadata was found.`, + ), + ); + } + } + + for (const name of declaredParamNames) { + if (name.length > 0 && !pathParamNames.has(name)) { + diagnostics.push( + createRouteDiagnostic( + route, + "contract-route-unbound-path-param", + "error", + `@Param("${name}") is not present in route path '${route.path}'.`, + ), + ); + } + } + + return diagnostics; +} + +function validateNamedParams(route: ContractGraphRoute): ContractDiagnostic[] { + const diagnostics: ContractDiagnostic[] = []; + + for (const kind of ["path", "query", "header"] as const) { + const params = route.params.filter((param) => param.kind === kind); + const seenNames = new Set(); + + for (const param of params) { + if (param.name.length === 0) { + diagnostics.push( + createRouteDiagnostic( + route, + "contract-param-missing-name", + "error", + `${kind} parameters must include a metadata name for generated contracts.`, + ), + ); + continue; + } + + if (seenNames.has(param.name)) { + diagnostics.push( + createRouteDiagnostic( + route, + "contract-param-duplicate-name", + "error", + `Duplicate ${kind} parameter '${param.name}' cannot be represented unambiguously.`, + ), + ); + } + + seenNames.add(param.name); + } + } + + return diagnostics; +} + +function validateBodyParams(route: ContractGraphRoute): ContractDiagnostic[] { + const bodyParams = route.params.filter((param) => param.kind === "body"); + + if (bodyParams.length <= 1) { + return []; + } + + return [ + createRouteDiagnostic( + route, + "contract-route-multiple-body-params", + "error", + `Generated contracts support one request body per route, but ${bodyParams.length} @Body() parameters were found.`, + ), + ]; +} + +function validateSchemaEffects(route: ContractGraphRoute): ContractDiagnostic[] { + const diagnostics: ContractDiagnostic[] = []; + + for (const schema of getRouteSchemas(route)) { + if (isZodEffects(schema)) { + diagnostics.push( + createRouteDiagnostic( + route, + "contract-schema-zod-effects-unwrapped", + "warning", + "Zod effects are represented from their inner schema in generated contracts; runtime transforms/refinements still run on the server.", + ), + ); + } + } + + return diagnostics; +} + +function validateUniqueRouteIds(routes: readonly ContractGraphRoute[]): ContractDiagnostic[] { + const diagnostics: ContractDiagnostic[] = []; + const routeIds = new Map(); + + for (const route of routes) { + const existingRoute = routeIds.get(route.routeId); + + if (existingRoute) { + diagnostics.push( + createRouteDiagnostic( + route, + "contract-route-duplicate-id", + "error", + `Route id '${route.routeId}' is already used by ${existingRoute.controllerName}.${existingRoute.methodName}.`, + ), + ); + continue; + } + + routeIds.set(route.routeId, route); + } + + return diagnostics; +} + +function getPathParamNames(path: string): string[] { + return [...path.matchAll(/:([^/]+)/g)].map((match) => match[1]).filter(Boolean); +} + +function getRouteSchemas(route: ContractGraphRoute): z.ZodType[] { + const schemas = [ + route.inputSchemas.body, + route.inputSchemas.path, + route.inputSchemas.query, + route.inputSchemas.headers, + route.outputSchema, + ...route.params.map((param) => param.schema), + ].filter((schema): schema is z.ZodType => Boolean(schema)); + + return [...new Set(schemas)]; +} + +function isZodEffects(schema: z.ZodType): boolean { + return schema.constructor.name === "ZodEffects"; +} + +function createRouteDiagnostic( + route: ContractGraphRoute, + code: string, + severity: ContractDiagnosticSeverity, + message: string, +): ContractDiagnostic { + return { + code, + severity, + target: getDiagnosticTarget(code), + message, + routeId: route.routeId, + controllerName: route.controllerName, + methodName: route.methodName, + path: route.path, + }; +} + +function getDiagnosticTarget(code: string): ContractDiagnosticTarget { + if (code.includes("param")) { + return "param"; + } + + if (code.includes("schema")) { + return "schema"; + } + + return "route"; +} diff --git a/packages/protocols-core/src/tests/ContractGraph.spec.ts b/packages/protocols-core/src/tests/ContractGraph.spec.ts new file mode 100644 index 000000000..be37dc70a --- /dev/null +++ b/packages/protocols-core/src/tests/ContractGraph.spec.ts @@ -0,0 +1,116 @@ +import "reflect-metadata"; +import { describe, expect, it } from "vitest"; +import { z } from "zod"; +import { + assertContractGraphHasNoErrors, + buildContractGraph, + ContractGraphDiagnosticError, + formatContractDiagnostic, +} from "../libs/ContractGraph"; +import { Body, Controller, Get, Param, Post, Query } from "./helpers/test-decorators"; + +describe("buildContractGraph", () => { + it("should build stable controller, route id, operation id, and schema graph nodes", () => { + const createUserSchema = z.object({ name: z.string() }); + + @Controller("/users") + class UsersController { + @Get("/:id") + getUser(@Param("id") _id: string, @Query("include") _include: string): void {} + + @Post("/") + createUser(@Body(createUserSchema) _body: z.infer): void {} + } + + const graph = buildContractGraph([UsersController]); + + expect(graph.version).toBe("croco.contract-graph.v1"); + expect(graph.controllers).toEqual([ + { + name: "UsersController", + path: "/users", + routeIds: ["UsersController.getUser", "UsersController.createUser"], + }, + ]); + expect(graph.routes).toHaveLength(2); + expect(graph.routes[0]).toMatchObject({ + routeId: "UsersController.getUser", + operationId: "UsersController_getUser", + controllerName: "UsersController", + methodName: "getUser", + httpMethod: "GET", + path: "/users/:id", + controllerPath: "/users", + }); + expect(graph.routes[1]?.inputSchemas.body).toBe(createUserSchema); + expect(graph.diagnostics).toEqual([]); + }); + + it("should report unsupported and drift-prone route metadata as diagnostics", () => { + @Controller("/hooks") + class HooksController { + @Get("/:id") + handleHook(@Param("hookId") _hookId: string): void {} + } + + const graph = buildContractGraph([HooksController]); + + expect(graph.diagnostics).toEqual([ + expect.objectContaining({ + code: "contract-route-missing-path-param", + severity: "error", + routeId: "HooksController.handleHook", + }), + expect.objectContaining({ + code: "contract-route-unbound-path-param", + severity: "error", + routeId: "HooksController.handleHook", + }), + ]); + expect(() => assertContractGraphHasNoErrors(graph)).toThrow(ContractGraphDiagnosticError); + }); + + it("should warn when generated contracts unwrap Zod effects", () => { + @Controller("/profiles") + class ProfilesController { + @Post("/") + createProfile(@Body(z.string().transform((value) => value.trim())) _body: string): void {} + } + + const graph = buildContractGraph([ProfilesController]); + + expect(graph.diagnostics).toEqual([ + expect.objectContaining({ + code: "contract-schema-zod-effects-unwrapped", + severity: "warning", + routeId: "ProfilesController.createProfile", + }), + ]); + expect(() => assertContractGraphHasNoErrors(graph)).not.toThrow(); + expect(formatContractDiagnostic(graph.diagnostics[0])).toContain( + "WARNING contract-schema-zod-effects-unwrapped ProfilesController.createProfile", + ); + }); + + it("should reject routes with more than one request body parameter", () => { + @Controller("/users") + class UsersController { + @Post("/") + createUser( + @Body(z.object({ name: z.string() })) _body: { name: string }, + @Body(z.object({ auditId: z.string() })) _audit: { auditId: string }, + ): void {} + } + + const graph = buildContractGraph([UsersController]); + + expect(graph.diagnostics).toEqual([ + expect.objectContaining({ + code: "contract-route-multiple-body-params", + severity: "error", + routeId: "UsersController.createUser", + }), + ]); + expect(() => assertContractGraphHasNoErrors(graph)).toThrow(ContractGraphDiagnosticError); + }); +}); diff --git a/packages/rpc-codegen/src/index.ts b/packages/rpc-codegen/src/index.ts index e6ba6dadb..6fc96e64b 100644 --- a/packages/rpc-codegen/src/index.ts +++ b/packages/rpc-codegen/src/index.ts @@ -1,2 +1,2 @@ export type { GenerateClientOptions } from "./libs/generate"; -export { generateClientFiles } from "./libs/generate"; +export { generateClientFiles, generateClientFilesFromContractGraph } from "./libs/generate"; diff --git a/packages/rpc-codegen/src/libs/cli.ts b/packages/rpc-codegen/src/libs/cli.ts index 26e6263b8..db1eb120d 100644 --- a/packages/rpc-codegen/src/libs/cli.ts +++ b/packages/rpc-codegen/src/libs/cli.ts @@ -1,7 +1,14 @@ +import { + formatContractDiagnostic, + getContractGraphErrors, + type ContractGraph, +} from "@croco/protocols-core"; + type CliOptions = { readonly controllers: string; - readonly outDir: string; + readonly outDir: string | null; readonly reactQuery: boolean; + readonly check: boolean; }; type CliParseResult = @@ -30,12 +37,34 @@ export async function runCli(args: readonly string[], io: CliIo = defaultCliIo): return 1; } - const [{ generateClientFiles }, { loadRoutes }] = await Promise.all([ + const [{ generateClientFilesFromContractGraph }, { loadContractGraph }] = await Promise.all([ import("./generate"), import("./loadRoutes"), ]); - const routes = await loadRoutes(result.options.controllers); - const files = generateClientFiles(routes, result.options.outDir, { + const graph = await loadContractGraph(result.options.controllers); + + if (result.options.check) { + return reportContractGraph(graph, io); + } + + const errors = getContractGraphErrors(graph); + + if (errors.length > 0) { + reportContractDiagnostics(graph, io); + io.stdout( + `Contract graph contains ${errors.length} error(s); fix them before generating clients.`, + ); + return 1; + } + + const outDir = result.options.outDir; + + if (!outDir) { + printHelp(io); + return 1; + } + + const files = generateClientFilesFromContractGraph(graph, outDir, { reactQuery: result.options.reactQuery, }); @@ -53,8 +82,9 @@ export function parseArgs(args: readonly string[]): CliParseResult { const controllers = getFlagValue(args, "--controllers"); const outDir = getFlagValue(args, "--out"); + const check = args.includes("--check"); - if (!controllers || !outDir) { + if (!controllers || (!outDir && !check)) { return { kind: "invalid" }; } @@ -64,6 +94,7 @@ export function parseArgs(args: readonly string[]): CliParseResult { controllers, outDir, reactQuery: args.includes("--react-query"), + check, }, }; } @@ -77,10 +108,35 @@ function getFlagValue(args: readonly string[], flag: string): string | null { function printHelp(io: CliIo): void { io.stdout(`Usage: croco-rpc-codegen --controllers --out [--react-query] + croco-rpc-codegen --controllers --check Options: --controllers Controller files to load --out Output directory for generated clients --react-query Generate React Query hooks + --check Validate the canonical contract graph without writing clients --help, -h Show this help message`); } + +function reportContractGraph(graph: ContractGraph, io: CliIo): number { + reportContractDiagnostics(graph, io); + + const errors = getContractGraphErrors(graph); + + if (errors.length > 0) { + io.stdout(`Contract graph check failed with ${errors.length} error(s).`); + return 1; + } + + io.stdout( + `Contract graph check passed for ${graph.routes.length} route(s) across ${graph.controllers.length} controller(s).`, + ); + + return 0; +} + +function reportContractDiagnostics(graph: ContractGraph, io: CliIo): void { + for (const diagnostic of graph.diagnostics) { + io.stdout(formatContractDiagnostic(diagnostic)); + } +} diff --git a/packages/rpc-codegen/src/libs/generate.ts b/packages/rpc-codegen/src/libs/generate.ts index a498de6e0..175a10f79 100644 --- a/packages/rpc-codegen/src/libs/generate.ts +++ b/packages/rpc-codegen/src/libs/generate.ts @@ -1,6 +1,10 @@ import * as fs from "node:fs"; import * as path from "node:path"; -import type { RouteIR } from "@croco/protocols-core"; +import { + assertContractGraphHasNoErrors, + type ContractGraph, + type RouteIR, +} from "@croco/protocols-core"; export type GenerateClientOptions = { readonly reactQuery?: boolean; @@ -16,6 +20,16 @@ type ResponseHelperOptions = { readonly hasNoOutputRoutes: boolean; }; +export function generateClientFilesFromContractGraph( + graph: ContractGraph, + outDir: string, + options: GenerateClientOptions = {}, +): string[] { + assertContractGraphHasNoErrors(graph); + + return generateClientFiles([...graph.routes], outDir, options); +} + export function generateClientFiles( routes: RouteIR[], outDir: string, @@ -43,6 +57,48 @@ function assertGeneratedClientRoutes(routes: RouteIR[]): void { `Cannot generate RPC client for @All route ${formatRoute(route)}: @All is runtime-only and cannot be represented as a concrete generated client request. Use explicit HTTP method decorators for generated contracts.`, ); } + + const bodyParamCount = route.params.filter((param) => param.kind === "body").length; + + if (bodyParamCount > 1) { + throw new Error( + `Cannot generate RPC client for route ${formatRoute(route)}: generated contracts support one request body per route, but ${bodyParamCount} @Body() parameters were found.`, + ); + } + + assertGeneratedClientPathParams(route); + } +} + +function assertGeneratedClientPathParams(route: RouteIR): void { + const pathParamNames = new Set(getRoutePathParamNames(route.path)); + const declaredParamNames = new Set( + route.params.filter((param) => param.kind === "path").map((param) => param.name), + ); + const schemaParamNames = new Set( + route.inputSchemas.path ? Object.keys(getObjectShape(route.inputSchemas.path)) : [], + ); + + for (const name of pathParamNames) { + if (!declaredParamNames.has(name)) { + throw new Error( + `Cannot generate RPC client for route ${formatRoute(route)}: route path declares ':${name}' but no @Param("${name}") metadata was found.`, + ); + } + + if (!schemaParamNames.has(name)) { + throw new Error( + `Cannot generate RPC client for route ${formatRoute(route)}: route path declares ':${name}' but no generated path schema was found.`, + ); + } + } + + for (const name of declaredParamNames) { + if (name.length > 0 && !pathParamNames.has(name)) { + throw new Error( + `Cannot generate RPC client for route ${formatRoute(route)}: @Param("${name}") is not present in route path '${route.path}'.`, + ); + } } } @@ -478,6 +534,10 @@ function getPathParamNames(route: RouteIR): string[] { return Object.keys(getObjectShape(route.inputSchemas.path)); } +function getRoutePathParamNames(pathname: string): string[] { + return [...pathname.matchAll(/:([^/]+)/g)].map((match) => match[1]).filter(Boolean); +} + function getQueryStatements(route: RouteIR): string { if (!route.inputSchemas.query) { return ""; diff --git a/packages/rpc-codegen/src/libs/loadRoutes.ts b/packages/rpc-codegen/src/libs/loadRoutes.ts index 723c7a32b..be0210a41 100644 --- a/packages/rpc-codegen/src/libs/loadRoutes.ts +++ b/packages/rpc-codegen/src/libs/loadRoutes.ts @@ -2,8 +2,10 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { pathToFileURL } from "node:url"; import { + buildContractGraph, + type Constructor, + type ContractGraph, discoverControllerConstructors, - extractRouteIR, type RouteIR, } from "@croco/protocols-core"; import { Project, type SourceFile, ts } from "ts-morph"; @@ -37,6 +39,10 @@ class NoRestControllersFoundProblem extends Error { } export async function loadRoutes(glob: string): Promise { + return [...(await loadContractGraph(glob)).routes]; +} + +export async function loadContractGraph(glob: string): Promise { const project = new Project({ compilerOptions: { module: ts.ModuleKind.CommonJS, @@ -61,7 +67,7 @@ export async function loadRoutes(glob: string): Promise { try { project.emitSync(); - const routes: RouteIR[] = []; + const controllerConstructors: Constructor[] = []; let controllerCount = 0; for (const sourceFile of sourceFiles) { @@ -71,17 +77,14 @@ export async function loadRoutes(glob: string): Promise { const controllers = discoverControllerConstructors(moduleExports); controllerCount += controllers.length; - - for (const controller of controllers) { - routes.push(...extractRouteIR(controller)); - } + controllerConstructors.push(...controllers); } if (controllerCount === 0) { throw new NoRestControllersFoundProblem(glob); } - return routes; + return buildContractGraph(controllerConstructors); } finally { fs.rmSync(emitDir, { recursive: true, force: true }); } diff --git a/packages/rpc-codegen/src/tests/Cli.spec.ts b/packages/rpc-codegen/src/tests/Cli.spec.ts index a6438493a..b2c098140 100644 --- a/packages/rpc-codegen/src/tests/Cli.spec.ts +++ b/packages/rpc-codegen/src/tests/Cli.spec.ts @@ -2,7 +2,15 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const generationModuleImports = vi.hoisted(() => ({ generate: 0, + generateClientFiles: 0, loadRoutes: 0, + loadContractGraph: 0, + graph: { + version: "croco.contract-graph.v1", + controllers: [{ name: "UsersController", path: "/users", routeIds: ["UsersController.list"] }], + routes: [{ routeId: "UsersController.list" }], + diagnostics: [] as ContractDiagnosticFixture[], + }, })); vi.mock("../libs/generate", () => { @@ -10,17 +18,33 @@ vi.mock("../libs/generate", () => { return { generateClientFiles: () => { + generationModuleImports.generateClientFiles += 1; throw new Error("generateClientFiles should not run for help or invalid arguments"); }, + generateClientFilesFromContractGraph: () => { + generationModuleImports.generateClientFiles += 1; + throw new Error( + "generateClientFilesFromContractGraph should not run for check or invalid contract graphs", + ); + }, }; }); +type ContractDiagnosticFixture = { + readonly code: string; + readonly severity: "error" | "warning"; + readonly target: string; + readonly message: string; + readonly routeId?: string; +}; + vi.mock("../libs/loadRoutes", () => { generationModuleImports.loadRoutes += 1; return { - loadRoutes: () => { - throw new Error("loadRoutes should not run for help or invalid arguments"); + loadContractGraph: () => { + generationModuleImports.loadContractGraph += 1; + return generationModuleImports.graph; }, }; }); @@ -33,7 +57,17 @@ describe("rpc-codegen CLI", () => { beforeEach(() => { stdout = []; generationModuleImports.generate = 0; + generationModuleImports.generateClientFiles = 0; generationModuleImports.loadRoutes = 0; + generationModuleImports.loadContractGraph = 0; + generationModuleImports.graph = { + version: "croco.contract-graph.v1", + controllers: [ + { name: "UsersController", path: "/users", routeIds: ["UsersController.list"] }, + ], + routes: [{ routeId: "UsersController.list" }], + diagnostics: [], + }; }); it("exits successfully for help without loading generation modules", async () => { @@ -45,7 +79,10 @@ describe("rpc-codegen CLI", () => { expect(stdout.join("\n")).toContain("Usage: croco-rpc-codegen"); expect(generationModuleImports).toEqual({ generate: 0, + generateClientFiles: 0, loadRoutes: 0, + loadContractGraph: 0, + graph: generationModuleImports.graph, }); }); @@ -62,7 +99,74 @@ describe("rpc-codegen CLI", () => { expect(stdout.join("\n")).toContain("Usage: croco-rpc-codegen"); expect(generationModuleImports).toEqual({ generate: 0, + generateClientFiles: 0, loadRoutes: 0, + loadContractGraph: 0, + graph: generationModuleImports.graph, + }); + }); + + it("validates the canonical contract graph without generating clients", async () => { + generationModuleImports.graph = { + version: "croco.contract-graph.v1", + controllers: [ + { name: "HooksController", path: "/hooks", routeIds: ["HooksController.handle"] }, + ], + routes: [{ routeId: "HooksController.handle" }], + diagnostics: [ + { + code: "contract-route-unsupported-all-method", + severity: "error", + target: "route", + message: "Use explicit HTTP method decorators.", + routeId: "HooksController.handle", + }, + ], + }; + + const exitCode = await runCli(["--controllers", "src/**/*.ts", "--check"], { + stdout: (message) => stdout.push(message), + }); + + expect(exitCode).toBe(1); + expect(stdout).toContain( + "ERROR contract-route-unsupported-all-method HooksController.handle: Use explicit HTTP method decorators.", + ); + expect(stdout).toContain("Contract graph check failed with 1 error(s)."); + expect(generationModuleImports.generateClientFiles).toBe(0); + expect(generationModuleImports.loadContractGraph).toBe(1); + }); + + it("fails client generation when the contract graph has errors", async () => { + generationModuleImports.graph = { + version: "croco.contract-graph.v1", + controllers: [ + { name: "UsersController", path: "/users", routeIds: ["UsersController.getUser"] }, + ], + routes: [{ routeId: "UsersController.getUser" }], + diagnostics: [ + { + code: "contract-route-missing-path-param", + severity: "error", + target: "route", + message: "Route path declares ':id' but no @Param(\"id\") metadata was found.", + routeId: "UsersController.getUser", + }, + ], + }; + + const exitCode = await runCli(["--controllers", "src/**/*.ts", "--out", "client"], { + stdout: (message) => stdout.push(message), }); + + expect(exitCode).toBe(1); + expect(stdout).toContain( + "ERROR contract-route-missing-path-param UsersController.getUser: Route path declares ':id' but no @Param(\"id\") metadata was found.", + ); + expect(stdout).toContain( + "Contract graph contains 1 error(s); fix them before generating clients.", + ); + expect(generationModuleImports.generateClientFiles).toBe(0); + expect(generationModuleImports.loadContractGraph).toBe(1); }); }); diff --git a/packages/rpc-codegen/src/tests/ContractCheckCli.spec.ts b/packages/rpc-codegen/src/tests/ContractCheckCli.spec.ts new file mode 100644 index 000000000..6afbe3f3a --- /dev/null +++ b/packages/rpc-codegen/src/tests/ContractCheckCli.spec.ts @@ -0,0 +1,100 @@ +import "reflect-metadata"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { runCli } from "../libs/cli"; + +const CONTRACT_CHECK_TIMEOUT_MS = 120_000; + +let tempRoot!: string; +let sourceDir!: string; + +describe("rpc-codegen contract check CLI", () => { + beforeEach(() => { + tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "rpc-contract-check-")); + sourceDir = path.join(tempRoot, "src"); + fs.mkdirSync(sourceDir, { recursive: true }); + }); + + afterEach(() => { + fs.rmSync(tempRoot, { recursive: true, force: true }); + }); + + it( + "fails when a loaded controller declares more than one request body parameter", + async () => { + fs.writeFileSync(path.join(sourceDir, "UsersController.ts"), getMultipleBodyController()); + const stdout: string[] = []; + + const exitCode = await runCli(["--controllers", path.join(sourceDir, "*.ts"), "--check"], { + stdout: (message) => stdout.push(message), + }); + + expect(exitCode).toBe(1); + expect(stdout).toContain( + "ERROR contract-route-multiple-body-params UsersController.createUser: Generated contracts support one request body per route, but 2 @Body() parameters were found.", + ); + expect(stdout).toContain("Contract graph check failed with 1 error(s)."); + }, + CONTRACT_CHECK_TIMEOUT_MS, + ); +}); + +function getMultipleBodyController(): string { + return `import 'reflect-metadata'; +import { z } from 'zod'; + +const REST_CONTROLLER_KEY = Symbol.for('croco:rest:controller'); +const REST_ROUTES_KEY = Symbol.for('croco:rest:routes'); +const REST_PARAMS_KEY = Symbol.for('croco:rest:params'); + +enum ParamType { + BODY = 'body', +} + +type RouteMetadata = { + readonly method: string; + readonly path: string; + readonly methodName: string | symbol; +}; + +function Controller(controllerPath: string): ClassDecorator { + return (target) => { + Reflect.defineMetadata(REST_CONTROLLER_KEY, { path: controllerPath, target }, target); + }; +} + +function Post(routePath: string): MethodDecorator { + return (target, propertyKey) => { + const ctor = target.constructor; + const routes = (Reflect.getMetadata(REST_ROUTES_KEY, ctor) as RouteMetadata[] | undefined) ?? []; + + Reflect.defineMetadata(REST_ROUTES_KEY, [...routes, { method: 'POST', path: routePath, methodName: propertyKey }], ctor); + }; +} + +function Body(schema: unknown): ParameterDecorator { + return (target, propertyKey, parameterIndex) => { + if (!propertyKey) return; + + const ctor = target.constructor; + const paramsMap = (Reflect.getMetadata(REST_PARAMS_KEY, ctor) as Map | undefined) ?? new Map(); + const params = paramsMap.get(propertyKey) ?? []; + + params.push({ type: ParamType.BODY, index: parameterIndex, pipes: [{ schema }] }); + paramsMap.set(propertyKey, params); + Reflect.defineMetadata(REST_PARAMS_KEY, paramsMap, ctor); + }; +} + +@Controller('/users') +export class UsersController { + @Post('/') + createUser( + @Body(z.object({ name: z.string() })) _body: { name: string }, + @Body(z.object({ auditId: z.string() })) _audit: { auditId: string }, + ): void {} +} +`; +} diff --git a/packages/rpc-codegen/src/tests/codegen.spec.ts b/packages/rpc-codegen/src/tests/codegen.spec.ts index aa9532107..e08513ca4 100644 --- a/packages/rpc-codegen/src/tests/codegen.spec.ts +++ b/packages/rpc-codegen/src/tests/codegen.spec.ts @@ -116,6 +116,74 @@ describe("generateClientFiles", () => { expect(fs.existsSync(path.join(TEMP_DIR, "hooks.ts"))).toBe(false); }); + it("should reject routes with more than one body parameter", () => { + const bodySchema = z.object({ name: z.string() }) as unknown as RouteIR["inputSchema"]; + const auditSchema = z.object({ auditId: z.string() }) as unknown as RouteIR["inputSchema"]; + const routes: RouteIR[] = [ + { + controllerName: "UsersController", + methodName: "createUser", + httpMethod: "POST", + path: "/users", + params: [ + { kind: "body", name: "", schema: bodySchema }, + { kind: "body", name: "", schema: auditSchema }, + ], + inputSchema: bodySchema, + inputSchemas: BODY_INPUT_SCHEMAS, + outputSchema: null, + domain: null, + }, + ]; + + expect(() => generateClientFiles(routes, TEMP_DIR)).toThrow( + "Cannot generate RPC client for route UsersController.createUser (/users): generated contracts support one request body per route, but 2 @Body() parameters were found.", + ); + expect(fs.existsSync(path.join(TEMP_DIR, "users.ts"))).toBe(false); + }); + + it("should reject path variables without matching path parameter metadata", () => { + const routes: RouteIR[] = [ + { + controllerName: "UsersController", + methodName: "getUser", + httpMethod: "GET", + path: "/users/:id", + params: [], + inputSchema: null, + inputSchemas: PATH_INPUT_SCHEMAS, + outputSchema: null, + domain: null, + }, + ]; + + expect(() => generateClientFiles(routes, TEMP_DIR)).toThrow( + "Cannot generate RPC client for route UsersController.getUser (/users/:id): route path declares ':id' but no @Param(\"id\") metadata was found.", + ); + expect(fs.existsSync(path.join(TEMP_DIR, "users.ts"))).toBe(false); + }); + + it("should reject path variables without matching generated path schemas", () => { + const routes: RouteIR[] = [ + { + controllerName: "UsersController", + methodName: "getUser", + httpMethod: "GET", + path: "/users/:id", + params: [{ kind: "path", name: "id", schema: null }], + inputSchema: null, + inputSchemas: EMPTY_INPUT_SCHEMAS, + outputSchema: null, + domain: null, + }, + ]; + + expect(() => generateClientFiles(routes, TEMP_DIR)).toThrow( + "Cannot generate RPC client for route UsersController.getUser (/users/:id): route path declares ':id' but no generated path schema was found.", + ); + expect(fs.existsSync(path.join(TEMP_DIR, "users.ts"))).toBe(false); + }); + it("should serialize POST body input", () => { const routes: RouteIR[] = [ { diff --git a/packages/rpc-codegen/src/tests/loadRoutes.spec.ts b/packages/rpc-codegen/src/tests/loadRoutes.spec.ts index ec480f2a3..d20a8081a 100644 --- a/packages/rpc-codegen/src/tests/loadRoutes.spec.ts +++ b/packages/rpc-codegen/src/tests/loadRoutes.spec.ts @@ -3,7 +3,7 @@ import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { loadRoutes } from "../libs/loadRoutes"; +import { loadContractGraph, loadRoutes } from "../libs/loadRoutes"; let tempRoot!: string; let sourceDir!: string; @@ -39,6 +39,30 @@ describe("loadRoutes", () => { LOAD_ROUTES_TIMEOUT_MS, ); + it( + "loads the canonical contract graph for exported controllers", + async () => { + fs.writeFileSync(path.join(sourceDir, "UsersController.ts"), getMixedControllerSource()); + + const graph = await loadContractGraph(path.join(sourceDir, "*.ts")); + + expect(graph.controllers).toEqual([ + { + name: "UsersController", + path: "/users", + routeIds: ["UsersController.listUsers"], + }, + ]); + expect(graph.routes[0]).toMatchObject({ + routeId: "UsersController.listUsers", + operationId: "UsersController_listUsers", + path: "/users", + }); + expect(graph.diagnostics).toEqual([]); + }, + LOAD_ROUTES_TIMEOUT_MS, + ); + it( "resolves controller imports from the nearest project node_modules", async () => { diff --git a/scripts/create-croco-app-generated-smoke.mts b/scripts/create-croco-app-generated-smoke.mts index e467b101a..80710c41d 100644 --- a/scripts/create-croco-app-generated-smoke.mts +++ b/scripts/create-croco-app-generated-smoke.mts @@ -445,6 +445,7 @@ function runSpaBeSplitContractSmoke(): void { ); run("pnpm", ["install"], projectDir); + run("pnpm", ["contract:check"], projectDir); run("pnpm", ["contract:openapi"], projectDir); assertExists( join(projectDir, "openapi.json"), From 47b43cd0dcd090279236e927ff72e616b43081fd Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 02:10:29 +0900 Subject: [PATCH 02/13] fix: enforce generated contract diagnostics --- packages/cli/package.json | 1 + packages/cli/src/tests/contractsCheck.spec.ts | 7 +++- .../src/tests/templates-build.spec.ts | 6 +-- .../templates/spa-be-split/package.json.hbs | 4 +- packages/openapi-spec/package.json | 2 + packages/openapi-spec/src/libs/emitOpenAPI.ts | 13 +++++-- .../openapi-spec/src/libs/loadControllers.ts | 31 ++++------------ .../src/tests/emitOpenAPI.spec.ts | 7 +++- packages/protocols-core/package.json | 2 + .../protocols-core/src/libs/ContractGraph.ts | 37 +++++++++++++++++-- .../src/tests/ContractGraph.spec.ts | 32 +++++++++++++++- packages/rpc-codegen/package.json | 1 + packages/rpc-codegen/src/libs/generate.ts | 25 ++++++++++--- packages/rpc-codegen/src/libs/loadRoutes.ts | 31 ++++------------ .../rpc-codegen/src/tests/codegen.spec.ts | 21 +++++++++++ pnpm-lock.yaml | 19 ++++++++++ 16 files changed, 172 insertions(+), 67 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index ce8a4e4a6..94941d852 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -44,6 +44,7 @@ "devDependencies": { "@types/node": "^22.0.0", "tsup": "^8.3.5", + "typedi": "0.10.0", "vitest": "^4.0.0" } } diff --git a/packages/cli/src/tests/contractsCheck.spec.ts b/packages/cli/src/tests/contractsCheck.spec.ts index 418b83114..bc550b015 100644 --- a/packages/cli/src/tests/contractsCheck.spec.ts +++ b/packages/cli/src/tests/contractsCheck.spec.ts @@ -1,7 +1,8 @@ import type { ChildProcess, SpawnOptions } from "node:child_process"; import { EventEmitter } from "node:events"; import { join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { Container } from "typedi"; +import { beforeEach, describe, expect, it } from "vitest"; import { type ContractsCheckSpawn, resolveRpcCodegenBinFromEntry, @@ -9,6 +10,10 @@ import { } from "../commands/contractsCheck.js"; describe("contractsCheck", () => { + beforeEach(() => { + Container.reset(); + }); + it("should resolve a workspace source RPC package entry to the built RPC CLI", () => { const root = join("workspace", "packages", "rpc-codegen"); diff --git a/packages/create-croco-app/src/tests/templates-build.spec.ts b/packages/create-croco-app/src/tests/templates-build.spec.ts index 94bdd2d30..e97cf496e 100644 --- a/packages/create-croco-app/src/tests/templates-build.spec.ts +++ b/packages/create-croco-app/src/tests/templates-build.spec.ts @@ -84,9 +84,9 @@ function checkSpaBeSplitStructure() { scripts: expect.objectContaining({ "dev:api": expect.any(String), "dev:web": expect.any(String), - "contract:check": expect.stringContaining("--check"), - "contract:openapi": expect.stringContaining("croco-openapi-spec"), - "contract:client": expect.stringContaining("croco-rpc-codegen"), + "contract:check": expect.stringMatching(/croco-rpc-codegen[\s\S]*--check/), + "contract:openapi": expect.stringMatching(/^pnpm contract:check &&[\s\S]*croco-openapi-spec/), + "contract:client": expect.stringMatching(/^pnpm contract:check &&[\s\S]*croco-rpc-codegen/), codegen: expect.any(String), test: "turbo test", }), diff --git a/packages/create-croco-app/templates/spa-be-split/package.json.hbs b/packages/create-croco-app/templates/spa-be-split/package.json.hbs index 78fdf03e7..5d5e9527e 100644 --- a/packages/create-croco-app/templates/spa-be-split/package.json.hbs +++ b/packages/create-croco-app/templates/spa-be-split/package.json.hbs @@ -6,8 +6,8 @@ "dev:api": "pnpm --filter {{scope}}/api-server dev", "dev:web": "pnpm --filter {{scope}}/console-web dev", "contract:check": "croco-rpc-codegen --controllers 'apps/api-server/src/controllers/**/*.ts' --check", - "contract:openapi": "croco-openapi-spec --controllers 'apps/api-server/src/controllers/**/*.ts' --out openapi.json --title '{{projectName}} API' --version 0.1.0 --server http://localhost:3000", - "contract:client": "croco-rpc-codegen --controllers 'apps/api-server/src/controllers/**/*.ts' --out libs/shared/provider-rpc/src --react-query", + "contract:openapi": "pnpm contract:check && croco-openapi-spec --controllers 'apps/api-server/src/controllers/**/*.ts' --out openapi.json --title '{{projectName}} API' --version 0.1.0 --server http://localhost:3000", + "contract:client": "pnpm contract:check && croco-rpc-codegen --controllers 'apps/api-server/src/controllers/**/*.ts' --out libs/shared/provider-rpc/src --react-query", "codegen": "pnpm contract:client", "build": "turbo build", "test": "turbo test", diff --git a/packages/openapi-spec/package.json b/packages/openapi-spec/package.json index 8908bae0d..579c69cd9 100644 --- a/packages/openapi-spec/package.json +++ b/packages/openapi-spec/package.json @@ -36,6 +36,7 @@ }, "dependencies": { "@asteasolutions/zod-to-openapi": "^7.3.4", + "@croco/problems-core": "workspace:*", "@croco/protocols-core": "workspace:*", "ts-morph": "^24.0.0", "zod": "^3.23.8" @@ -45,6 +46,7 @@ "@redocly/cli": "^2.30.3", "orval": "8.9.1", "reflect-metadata": "0.2.2", + "typedi": "0.10.0", "vitest": "4.0.16" }, "vitest": { diff --git a/packages/openapi-spec/src/libs/emitOpenAPI.ts b/packages/openapi-spec/src/libs/emitOpenAPI.ts index 7401f3341..dddd1744e 100644 --- a/packages/openapi-spec/src/libs/emitOpenAPI.ts +++ b/packages/openapi-spec/src/libs/emitOpenAPI.ts @@ -4,6 +4,7 @@ import { OpenApiGeneratorV31, type RouteConfig, } from "@asteasolutions/zod-to-openapi"; +import { Problem, ProblemCategory } from "@croco/problems-core"; import { assertContractGraphHasNoErrors, buildContractGraph, @@ -58,6 +59,12 @@ const DEFAULT_PROBLEM_RESPONSES = [ { status: 500, description: "Internal server error" }, ] as const satisfies readonly ProblemResponseConfig[]; +class OpenAPIContractProblem extends Problem { + constructor(detail: string) { + super("openapi-spec/invalid-contract", ProblemCategory.ValidationError, detail); + } +} + export function emitOpenAPI( controllers: Function[], options: EmitOpenAPIOptions = {}, @@ -291,7 +298,7 @@ function toOpenAPIParamLocation(kind: ParamIR["kind"]): OpenAPIParamLocation { return kind; } - throw new Error(`Unsupported OpenAPI parameter kind: ${kind}`); + throw new OpenAPIContractProblem(`Unsupported OpenAPI parameter kind: ${kind}`); } function toOpenAPIPath(path: string): string { @@ -308,12 +315,12 @@ function toHttpMethod(route: ContractGraphRoute): HttpMethod { } if (normalizedMethod === "all") { - throw new Error( + throw new OpenAPIContractProblem( `Cannot emit OpenAPI operation for @All route ${formatRoute(route)}: @All is runtime-only and cannot be represented as a concrete OpenAPI operation. Use explicit HTTP method decorators for generated contracts.`, ); } - throw new Error(`Unsupported HTTP method: ${method}`); + throw new OpenAPIContractProblem(`Unsupported HTTP method: ${method}`); } function formatRoute(route: ContractGraphRoute): string { diff --git a/packages/openapi-spec/src/libs/loadControllers.ts b/packages/openapi-spec/src/libs/loadControllers.ts index 135322a00..a6eb2af9f 100644 --- a/packages/openapi-spec/src/libs/loadControllers.ts +++ b/packages/openapi-spec/src/libs/loadControllers.ts @@ -1,36 +1,19 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { pathToFileURL } from "node:url"; +import { Problem, ProblemCategory } from "@croco/problems-core"; import { discoverControllerConstructors, type Constructor } from "@croco/protocols-core"; import { Project, type SourceFile, ts } from "ts-morph"; type Controller = Constructor; -class NoRestControllersFoundProblem extends Error { - readonly code = "openapi-spec/no-rest-controllers-found"; - readonly type = "about:blank"; - readonly title = "Bad Request"; - readonly status = 400; - readonly category = "BadRequest"; - readonly detail: string; - +class NoRestControllersFoundProblem extends Problem { constructor(glob: string) { - const detail = getNoRestControllersFoundMessage(glob); - - super(detail); - this.detail = detail; - this.name = new.target.name; - Object.setPrototypeOf(this, new.target.prototype); - } - - toJSON(): Record { - return { - type: this.type, - title: this.title, - status: this.status, - code: this.code, - detail: this.detail, - }; + super( + "openapi-spec/no-rest-controllers-found", + ProblemCategory.BadRequest, + getNoRestControllersFoundMessage(glob), + ); } } diff --git a/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts b/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts index 29286128c..92f2b796e 100644 --- a/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts +++ b/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts @@ -3,6 +3,7 @@ import { execFileSync } from "node:child_process"; import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { Container } from "typedi"; import { Body, Controller, @@ -16,11 +17,15 @@ import { All, } from "@croco/protocols-rest"; import { buildContractGraph } from "@croco/protocols-core"; -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it } from "vitest"; import { z } from "zod"; import { emitOpenAPI, emitOpenAPIFromContractGraph } from "../libs/emitOpenAPI"; describe("emitOpenAPI", () => { + beforeEach(() => { + Container.reset(); + }); + it("should emit a GET operation with a path parameter", () => { @Controller("/users") class UsersController { diff --git a/packages/protocols-core/package.json b/packages/protocols-core/package.json index b646a1faa..05c5b6ebe 100644 --- a/packages/protocols-core/package.json +++ b/packages/protocols-core/package.json @@ -33,10 +33,12 @@ "test:watch": "vitest" }, "dependencies": { + "@croco/problems-core": "workspace:*", "reflect-metadata": "0.2.2", "zod": "^3.23.8" }, "devDependencies": { + "typedi": "0.10.0", "vitest": "4.0.16" }, "vitest": { diff --git a/packages/protocols-core/src/libs/ContractGraph.ts b/packages/protocols-core/src/libs/ContractGraph.ts index 83f0ac897..dc6461444 100644 --- a/packages/protocols-core/src/libs/ContractGraph.ts +++ b/packages/protocols-core/src/libs/ContractGraph.ts @@ -1,4 +1,5 @@ import "reflect-metadata"; +import { Problem, ProblemCategory } from "@croco/problems-core"; import type { z } from "zod"; import { extractRouteIR } from "./extractRouteIR"; import type { RouteIR } from "./RouteIR"; @@ -38,12 +39,16 @@ export type ContractGraph = { readonly diagnostics: readonly ContractDiagnostic[]; }; -export class ContractGraphDiagnosticError extends Error { +export class ContractGraphDiagnosticError extends Problem { readonly diagnostics: readonly ContractDiagnostic[]; constructor(diagnostics: readonly ContractDiagnostic[]) { - super(formatContractDiagnostics(diagnostics)); - this.name = "ContractGraphDiagnosticError"; + super( + "protocols-core/contract-graph-diagnostics", + ProblemCategory.ValidationError, + formatContractDiagnostics(diagnostics), + { extensions: { diagnostics } }, + ); this.diagnostics = diagnostics; } } @@ -79,6 +84,7 @@ export function buildContractGraph(controllers: readonly Constructor[]): Contrac } diagnostics.push(...validateUniqueRouteIds(graphRoutes)); + diagnostics.push(...validateUniqueOperationIds(graphRoutes)); return { version: "croco.contract-graph.v1", @@ -278,6 +284,31 @@ function validateUniqueRouteIds(routes: readonly ContractGraphRoute[]): Contract return diagnostics; } +function validateUniqueOperationIds(routes: readonly ContractGraphRoute[]): ContractDiagnostic[] { + const diagnostics: ContractDiagnostic[] = []; + const operationIds = new Map(); + + for (const route of routes) { + const existingRoute = operationIds.get(route.operationId); + + if (existingRoute) { + diagnostics.push( + createRouteDiagnostic( + route, + "contract-route-duplicate-operation-id", + "error", + `Operation id '${route.operationId}' is already used by ${existingRoute.controllerName}.${existingRoute.methodName}.`, + ), + ); + continue; + } + + operationIds.set(route.operationId, route); + } + + return diagnostics; +} + function getPathParamNames(path: string): string[] { return [...path.matchAll(/:([^/]+)/g)].map((match) => match[1]).filter(Boolean); } diff --git a/packages/protocols-core/src/tests/ContractGraph.spec.ts b/packages/protocols-core/src/tests/ContractGraph.spec.ts index be37dc70a..bfc3fa5a6 100644 --- a/packages/protocols-core/src/tests/ContractGraph.spec.ts +++ b/packages/protocols-core/src/tests/ContractGraph.spec.ts @@ -1,5 +1,6 @@ import "reflect-metadata"; -import { describe, expect, it } from "vitest"; +import { Container } from "typedi"; +import { beforeEach, describe, expect, it } from "vitest"; import { z } from "zod"; import { assertContractGraphHasNoErrors, @@ -10,6 +11,10 @@ import { import { Body, Controller, Get, Param, Post, Query } from "./helpers/test-decorators"; describe("buildContractGraph", () => { + beforeEach(() => { + Container.reset(); + }); + it("should build stable controller, route id, operation id, and schema graph nodes", () => { const createUserSchema = z.object({ name: z.string() }); @@ -113,4 +118,29 @@ describe("buildContractGraph", () => { ]); expect(() => assertContractGraphHasNoErrors(graph)).toThrow(ContractGraphDiagnosticError); }); + + it("should reject duplicate normalized operation ids", () => { + @Controller("/users") + class UsersController { + @Get("/with-underscore") + get_user(): void {} + } + + @Controller("/users-alt") + class UsersController_get { + @Get("/plain") + user(): void {} + } + + const graph = buildContractGraph([UsersController, UsersController_get]); + + expect(graph.diagnostics).toEqual([ + expect.objectContaining({ + code: "contract-route-duplicate-operation-id", + severity: "error", + routeId: "UsersController_get.user", + }), + ]); + expect(() => assertContractGraphHasNoErrors(graph)).toThrow(ContractGraphDiagnosticError); + }); }); diff --git a/packages/rpc-codegen/package.json b/packages/rpc-codegen/package.json index aa27e2590..eaeed3d76 100644 --- a/packages/rpc-codegen/package.json +++ b/packages/rpc-codegen/package.json @@ -34,6 +34,7 @@ "test": "vitest run" }, "dependencies": { + "@croco/problems-core": "workspace:*", "@croco/protocols-core": "workspace:*", "prettier": "^3.0.0", "ts-morph": "^24.0.0" diff --git a/packages/rpc-codegen/src/libs/generate.ts b/packages/rpc-codegen/src/libs/generate.ts index 175a10f79..86e5efdcc 100644 --- a/packages/rpc-codegen/src/libs/generate.ts +++ b/packages/rpc-codegen/src/libs/generate.ts @@ -5,6 +5,7 @@ import { type ContractGraph, type RouteIR, } from "@croco/protocols-core"; +import { Problem, ProblemCategory } from "@croco/problems-core"; export type GenerateClientOptions = { readonly reactQuery?: boolean; @@ -20,6 +21,12 @@ type ResponseHelperOptions = { readonly hasNoOutputRoutes: boolean; }; +class RpcCodegenContractProblem extends Problem { + constructor(detail: string) { + super("rpc-codegen/invalid-contract", ProblemCategory.ValidationError, detail); + } +} + export function generateClientFilesFromContractGraph( graph: ContractGraph, outDir: string, @@ -53,7 +60,7 @@ export function generateClientFiles( function assertGeneratedClientRoutes(routes: RouteIR[]): void { for (const route of routes) { if (route.httpMethod.toUpperCase() === "ALL") { - throw new Error( + throw new RpcCodegenContractProblem( `Cannot generate RPC client for @All route ${formatRoute(route)}: @All is runtime-only and cannot be represented as a concrete generated client request. Use explicit HTTP method decorators for generated contracts.`, ); } @@ -61,7 +68,7 @@ function assertGeneratedClientRoutes(routes: RouteIR[]): void { const bodyParamCount = route.params.filter((param) => param.kind === "body").length; if (bodyParamCount > 1) { - throw new Error( + throw new RpcCodegenContractProblem( `Cannot generate RPC client for route ${formatRoute(route)}: generated contracts support one request body per route, but ${bodyParamCount} @Body() parameters were found.`, ); } @@ -81,13 +88,13 @@ function assertGeneratedClientPathParams(route: RouteIR): void { for (const name of pathParamNames) { if (!declaredParamNames.has(name)) { - throw new Error( + throw new RpcCodegenContractProblem( `Cannot generate RPC client for route ${formatRoute(route)}: route path declares ':${name}' but no @Param("${name}") metadata was found.`, ); } if (!schemaParamNames.has(name)) { - throw new Error( + throw new RpcCodegenContractProblem( `Cannot generate RPC client for route ${formatRoute(route)}: route path declares ':${name}' but no generated path schema was found.`, ); } @@ -95,11 +102,19 @@ function assertGeneratedClientPathParams(route: RouteIR): void { for (const name of declaredParamNames) { if (name.length > 0 && !pathParamNames.has(name)) { - throw new Error( + throw new RpcCodegenContractProblem( `Cannot generate RPC client for route ${formatRoute(route)}: @Param("${name}") is not present in route path '${route.path}'.`, ); } } + + for (const name of schemaParamNames) { + if (!pathParamNames.has(name)) { + throw new RpcCodegenContractProblem( + `Cannot generate RPC client for route ${formatRoute(route)}: generated path schema declares '${name}' but route path '${route.path}' does not contain ':${name}'.`, + ); + } + } } function groupRoutesByDomain(routes: RouteIR[]): DomainRoutes[] { diff --git a/packages/rpc-codegen/src/libs/loadRoutes.ts b/packages/rpc-codegen/src/libs/loadRoutes.ts index be0210a41..969a391c1 100644 --- a/packages/rpc-codegen/src/libs/loadRoutes.ts +++ b/packages/rpc-codegen/src/libs/loadRoutes.ts @@ -8,33 +8,16 @@ import { discoverControllerConstructors, type RouteIR, } from "@croco/protocols-core"; +import { Problem, ProblemCategory } from "@croco/problems-core"; import { Project, type SourceFile, ts } from "ts-morph"; -class NoRestControllersFoundProblem extends Error { - readonly code = "rpc-codegen/no-rest-controllers-found"; - readonly type = "about:blank"; - readonly title = "Bad Request"; - readonly status = 400; - readonly category = "BadRequest"; - readonly detail: string; - +class NoRestControllersFoundProblem extends Problem { constructor(glob: string) { - const detail = getNoRestControllersFoundMessage(glob); - - super(detail); - this.detail = detail; - this.name = new.target.name; - Object.setPrototypeOf(this, new.target.prototype); - } - - toJSON(): Record { - return { - type: this.type, - title: this.title, - status: this.status, - code: this.code, - detail: this.detail, - }; + super( + "rpc-codegen/no-rest-controllers-found", + ProblemCategory.BadRequest, + getNoRestControllersFoundMessage(glob), + ); } } diff --git a/packages/rpc-codegen/src/tests/codegen.spec.ts b/packages/rpc-codegen/src/tests/codegen.spec.ts index e08513ca4..bfd5d67d3 100644 --- a/packages/rpc-codegen/src/tests/codegen.spec.ts +++ b/packages/rpc-codegen/src/tests/codegen.spec.ts @@ -184,6 +184,27 @@ describe("generateClientFiles", () => { expect(fs.existsSync(path.join(TEMP_DIR, "users.ts"))).toBe(false); }); + it("should reject generated path schemas without matching path variables", () => { + const routes: RouteIR[] = [ + { + controllerName: "UsersController", + methodName: "listUsers", + httpMethod: "GET", + path: "/users", + params: [], + inputSchema: null, + inputSchemas: PATH_INPUT_SCHEMAS, + outputSchema: null, + domain: null, + }, + ]; + + expect(() => generateClientFiles(routes, TEMP_DIR)).toThrow( + "Cannot generate RPC client for route UsersController.listUsers (/users): generated path schema declares 'id' but route path '/users' does not contain ':id'.", + ); + expect(fs.existsSync(path.join(TEMP_DIR, "users.ts"))).toBe(false); + }); + it("should serialize POST body input", () => { const routes: RouteIR[] = [ { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dca3d6f91..232c48c15 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -538,6 +538,9 @@ importers: tsup: specifier: ^8.3.5 version: 8.5.1(jiti@2.6.1)(postcss@8.5.6)(tsx@4.21.0)(typescript@5.9.3)(yaml@2.8.3) + typedi: + specifier: 0.10.0 + version: 0.10.0 vitest: specifier: ^4.0.0 version: 4.0.16(@opentelemetry/api@1.9.0)(@types/node@22.19.3)(jiti@2.6.1)(tsx@4.21.0)(yaml@2.8.3) @@ -1698,6 +1701,9 @@ importers: '@asteasolutions/zod-to-openapi': specifier: ^7.3.4 version: 7.3.4(zod@3.25.76) + '@croco/problems-core': + specifier: workspace:* + version: link:../problems-core '@croco/protocols-core': specifier: workspace:* version: link:../protocols-core @@ -1720,6 +1726,9 @@ importers: reflect-metadata: specifier: 0.2.2 version: 0.2.2 + typedi: + specifier: 0.10.0 + version: 0.10.0 vitest: specifier: 4.0.16 version: 4.0.16(@opentelemetry/api@1.9.0)(@types/node@25.2.0)(jiti@2.6.1)(tsx@4.21.0)(yaml@2.8.3) @@ -1816,6 +1825,9 @@ importers: packages/protocols-core: dependencies: + '@croco/problems-core': + specifier: workspace:* + version: link:../problems-core reflect-metadata: specifier: 0.2.2 version: 0.2.2 @@ -1823,6 +1835,9 @@ importers: specifier: ^3.23.8 version: 3.25.76 devDependencies: + typedi: + specifier: 0.10.0 + version: 0.10.0 vitest: specifier: 4.0.16 version: 4.0.16(@opentelemetry/api@1.9.0)(@types/node@25.2.0)(jiti@2.6.1)(tsx@4.21.0)(yaml@2.8.3) @@ -1973,6 +1988,9 @@ importers: packages/rpc-codegen: dependencies: + '@croco/problems-core': + specifier: workspace:* + version: link:../problems-core '@croco/protocols-core': specifier: workspace:* version: link:../protocols-core @@ -8470,6 +8488,7 @@ packages: tsconfck@3.1.6: resolution: {integrity: sha512-ks6Vjr/jEw0P1gmOVwutM3B7fWxoWBL2KRDb1JfqGVawBmO5UsvmWOQFGHBPl5yxYz4eERr19E6L7NMv+Fej4w==} engines: {node: ^18 || >=20} + deprecated: unmaintained hasBin: true peerDependencies: typescript: ^5.0.0 From c4ce87b4d6cf533bf1132adb112b2fb05588cc9e Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 02:24:57 +0900 Subject: [PATCH 03/13] fix: pack generated contract smoke dependencies --- .changeset/contract-graph-check.md | 1 + scripts/create-croco-app-generated-smoke.mts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/.changeset/contract-graph-check.md b/.changeset/contract-graph-check.md index b24689e4a..ec91e3748 100644 --- a/.changeset/contract-graph-check.md +++ b/.changeset/contract-graph-check.md @@ -3,6 +3,7 @@ "@croco/openapi-spec": patch "@croco/rpc-codegen": patch "@croco/cli": patch +"@croco/problems-core": patch "create-croco-app": patch --- diff --git a/scripts/create-croco-app-generated-smoke.mts b/scripts/create-croco-app-generated-smoke.mts index 80710c41d..50aec66c0 100644 --- a/scripts/create-croco-app-generated-smoke.mts +++ b/scripts/create-croco-app-generated-smoke.mts @@ -237,6 +237,7 @@ try { "--filter=create-croco-app...", "--filter=@croco/frontend-vite...", "--filter=@croco/openapi-spec...", + "--filter=@croco/problems-core...", "--filter=@croco/rpc-codegen...", "--force", ], @@ -482,6 +483,7 @@ function getContractSmokeRangeOverrides(): Record { return { "@croco/openapi-spec": `file:${packWorkspacePackage("@croco/openapi-spec", "openapi-spec", packDir)}`, + "@croco/problems-core": `file:${packWorkspacePackage("@croco/problems-core", "problems-core", packDir)}`, "@croco/protocols-core": `file:${packWorkspacePackage("@croco/protocols-core", "protocols-core", packDir)}`, "@croco/rpc-codegen": `file:${packWorkspacePackage("@croco/rpc-codegen", "rpc-codegen", packDir)}`, }; From c5417edea9665513637966420a82dd81597c006b Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 02:40:29 +0900 Subject: [PATCH 04/13] fix: pack rpc codegen published dependencies --- .../src/tests/PublishedCli.spec.ts | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/rpc-codegen/src/tests/PublishedCli.spec.ts b/packages/rpc-codegen/src/tests/PublishedCli.spec.ts index 0b0375c22..530b0e502 100644 --- a/packages/rpc-codegen/src/tests/PublishedCli.spec.ts +++ b/packages/rpc-codegen/src/tests/PublishedCli.spec.ts @@ -1,5 +1,5 @@ import { spawnSync } from "node:child_process"; -import { mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; @@ -19,8 +19,12 @@ describe("published RPC codegen CLI", () => { const consumerRoot = mkdtempSync(join(tmpdir(), "croco-rpc-codegen-consumer-")); try { - run("pnpm", ["--filter", "@croco/protocols-core", "build"], rootDir); - run("pnpm", ["--filter", "@croco/rpc-codegen", "build"], rootDir); + ensureBuilt(); + run( + "pnpm", + ["--filter", "@croco/problems-core", "pack", "--pack-destination", packRoot], + rootDir, + ); run( "pnpm", ["--filter", "@croco/protocols-core", "pack", "--pack-destination", packRoot], @@ -32,6 +36,7 @@ describe("published RPC codegen CLI", () => { rootDir, ); + const problemsCoreTarball = findTarball(packRoot, "croco-problems-core-"); const protocolsCoreTarball = findTarball(packRoot, "croco-protocols-core-"); const rpcCodegenTarball = findTarball(packRoot, "croco-rpc-codegen-"); const packedManifest = JSON.parse( @@ -56,6 +61,7 @@ describe("published RPC codegen CLI", () => { private: true, pnpm: { overrides: { + "@croco/problems-core": `file:${problemsCoreTarball}`, "@croco/protocols-core": `file:${protocolsCoreTarball}`, }, }, @@ -79,6 +85,20 @@ describe("published RPC codegen CLI", () => { ); }); +function ensureBuilt(): void { + if ( + existsSync(join(rootDir, "packages", "problems-core", "dist", "index.js")) && + existsSync(join(rootDir, "packages", "protocols-core", "dist", "index.js")) && + existsSync(join(packageDir, "dist", "cli.js")) + ) { + return; + } + + run("pnpm", ["--filter", "@croco/problems-core", "build"], rootDir); + run("pnpm", ["--filter", "@croco/protocols-core", "build"], rootDir); + run("pnpm", ["--filter", "@croco/rpc-codegen", "build"], rootDir); +} + function findTarball(directory: string, prefix: string): string { const filename = readdirSync(directory).find( (entry) => entry.startsWith(prefix) && entry.endsWith(".tgz"), From 544d5cba37c4cb02f320607d830edb4e4e675d22 Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 03:02:59 +0900 Subject: [PATCH 05/13] fix: normalize generated catch-all paths --- packages/rpc-codegen/src/libs/generate.ts | 28 ++++++++++--------- .../rpc-codegen/src/tests/codegen.spec.ts | 23 +++++++++++++++ 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/packages/rpc-codegen/src/libs/generate.ts b/packages/rpc-codegen/src/libs/generate.ts index 86e5efdcc..5893588d2 100644 --- a/packages/rpc-codegen/src/libs/generate.ts +++ b/packages/rpc-codegen/src/libs/generate.ts @@ -524,33 +524,35 @@ function getObjectShape(schema: unknown): Record { } function getPathExpression(route: RouteIR): string { - const pathParams = getPathParamNames(route); + const pathParams = getRoutePathParams(route.path); if (pathParams.length === 0) { return `'${route.path}'`; } const pathExpression = pathParams.reduce( - (currentPath, paramName) => + (currentPath, param) => currentPath - .split(`:${paramName}`) - .join(`\${encodeURIComponent(String(input.path.${paramName}))}`), + .split(`:${param.token}`) + .join(`\${encodeURIComponent(String(input.path.${param.name}))}`), route.path, ); return `\`${pathExpression}\``; } -function getPathParamNames(route: RouteIR): string[] { - if (!route.inputSchemas.path) { - return []; - } - - return Object.keys(getObjectShape(route.inputSchemas.path)); +function getRoutePathParamNames(pathname: string): string[] { + return getRoutePathParams(pathname).map((param) => param.name); } -function getRoutePathParamNames(pathname: string): string[] { - return [...pathname.matchAll(/:([^/]+)/g)].map((match) => match[1]).filter(Boolean); +function getRoutePathParams(pathname: string): { readonly token: string; readonly name: string }[] { + return [...pathname.matchAll(/:([^/]+)/g)] + .map((match) => { + const token = match[1]; + + return { token, name: token.replace(/^\.\.\./, "") }; + }) + .filter((param) => param.name.length > 0); } function getQueryStatements(route: RouteIR): string { @@ -629,7 +631,7 @@ function assertNoZodImport(content: string): void { content.includes("import { z }") || content.includes("zod") ) { - throw new Error("Generated client must not import zod."); + throw new RpcCodegenContractProblem("Generated client must not import zod."); } } diff --git a/packages/rpc-codegen/src/tests/codegen.spec.ts b/packages/rpc-codegen/src/tests/codegen.spec.ts index bfd5d67d3..4367714c8 100644 --- a/packages/rpc-codegen/src/tests/codegen.spec.ts +++ b/packages/rpc-codegen/src/tests/codegen.spec.ts @@ -456,6 +456,29 @@ describe("generateClientFiles", () => { ); }); + it("should normalize catch-all path parameters when generating fetch paths", () => { + const routes: RouteIR[] = [ + { + controllerName: "AssetController", + methodName: "get", + httpMethod: "GET", + path: "/assets/:...id", + params: [{ kind: "path", name: "id", schema: null }], + inputSchema: null, + inputSchemas: PATH_INPUT_SCHEMAS, + outputSchema: null, + domain: null, + }, + ]; + + const files = generateClientFiles(routes, TEMP_DIR); + + const content = fs.readFileSync(files[0], "utf-8"); + expect(content).toContain( + "const path = `/assets/${encodeURIComponent(String(input.path.id))}`;", + ); + }); + it("should serialize query input when generating query parameter fetch calls", () => { const routes: RouteIR[] = [ { From eeb9bfdc99d905dfada9f5de92eaf0ed90da5c2d Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 03:23:36 +0900 Subject: [PATCH 06/13] fix: align contract graph route metadata --- .changeset/contract-graph-check.md | 1 - packages/openapi-spec/src/libs/emitOpenAPI.ts | 6 +- .../src/tests/emitOpenAPI.spec.ts | 25 +++++ packages/protocols-core/src/index.ts | 4 + .../protocols-core/src/libs/ContractGraph.ts | 95 +++++++++++++++++-- .../protocols-core/src/libs/sharedTypes.ts | 2 + .../src/tests/ContractGraph.spec.ts | 58 ++++++++++- .../src/tests/helpers/test-decorators.ts | 24 +++++ packages/rpc-codegen/src/libs/generate.ts | 20 +--- .../src/tests/ContractCheckCli.spec.ts | 72 ++++++++++++++ .../rpc-codegen/src/tests/loadRoutes.spec.ts | 2 + 11 files changed, 282 insertions(+), 27 deletions(-) diff --git a/.changeset/contract-graph-check.md b/.changeset/contract-graph-check.md index ec91e3748..b24689e4a 100644 --- a/.changeset/contract-graph-check.md +++ b/.changeset/contract-graph-check.md @@ -3,7 +3,6 @@ "@croco/openapi-spec": patch "@croco/rpc-codegen": patch "@croco/cli": patch -"@croco/problems-core": patch "create-croco-app": patch --- diff --git a/packages/openapi-spec/src/libs/emitOpenAPI.ts b/packages/openapi-spec/src/libs/emitOpenAPI.ts index dddd1744e..5875cd0ac 100644 --- a/packages/openapi-spec/src/libs/emitOpenAPI.ts +++ b/packages/openapi-spec/src/libs/emitOpenAPI.ts @@ -8,6 +8,7 @@ import { Problem, ProblemCategory } from "@croco/problems-core"; import { assertContractGraphHasNoErrors, buildContractGraph, + getContractPathParams, type ContractGraph, type ContractGraphRoute, type ParamIR, @@ -302,7 +303,10 @@ function toOpenAPIParamLocation(kind: ParamIR["kind"]): OpenAPIParamLocation { } function toOpenAPIPath(path: string): string { - return path.replace(/:([^/]+)/g, "{$1}"); + return getContractPathParams(path).reduce( + (currentPath, param) => currentPath.split(`:${param.token}`).join(`{${param.name}}`), + path, + ); } function toHttpMethod(route: ContractGraphRoute): HttpMethod { diff --git a/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts b/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts index 92f2b796e..3b226e90d 100644 --- a/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts +++ b/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts @@ -66,6 +66,31 @@ describe("emitOpenAPI", () => { expect(spec.paths?.["/users/{id}"]?.get?.summary).toBe("UsersController.getUser"); }); + it("should normalize catch-all path parameters from the canonical contract graph", () => { + @Controller("/assets") + class AssetsController { + @Get("/:...id") + getAsset(@Param("id") _id: string): void {} + } + + const graph = buildContractGraph([AssetsController]); + const spec = emitOpenAPIFromContractGraph(graph); + + expect(graph.diagnostics).toEqual([]); + expect(spec.paths?.["/assets/{id}"]?.get).toMatchObject({ + operationId: "AssetsController_getAsset", + parameters: [ + { + in: "path", + name: "id", + required: true, + schema: { type: "string" }, + }, + ], + }); + expect(spec.paths?.["/assets/{...id}"]).toBeUndefined(); + }); + it("should apply document metadata options", () => { @Controller("/accounts") class AccountsController { diff --git a/packages/protocols-core/src/index.ts b/packages/protocols-core/src/index.ts index f496fbe78..ffa622442 100644 --- a/packages/protocols-core/src/index.ts +++ b/packages/protocols-core/src/index.ts @@ -8,6 +8,8 @@ export { ContractGraphDiagnosticError, formatContractDiagnostic, formatContractDiagnostics, + getContractPathParamNames, + getContractPathParams, getContractGraphErrors, } from "./libs/ContractGraph"; export { extractRouteIR } from "./libs/extractRouteIR"; @@ -15,9 +17,11 @@ export type { ContractDiagnostic, ContractDiagnosticSeverity, ContractDiagnosticTarget, + ContractAccessMetadata, ContractGraph, ContractGraphController, ContractGraphRoute, + ContractPathParam, ContractGraphVersion, } from "./libs/ContractGraph"; export type { ParamIR, RouteIR } from "./libs/RouteIR"; diff --git a/packages/protocols-core/src/libs/ContractGraph.ts b/packages/protocols-core/src/libs/ContractGraph.ts index dc6461444..336bd9104 100644 --- a/packages/protocols-core/src/libs/ContractGraph.ts +++ b/packages/protocols-core/src/libs/ContractGraph.ts @@ -3,7 +3,13 @@ import { Problem, ProblemCategory } from "@croco/problems-core"; import type { z } from "zod"; import { extractRouteIR } from "./extractRouteIR"; import type { RouteIR } from "./RouteIR"; -import { type Constructor, type ControllerMetadata, REST_CONTROLLER_KEY } from "./sharedTypes"; +import { + type Constructor, + type ControllerMetadata, + REST_CONTROLLER_KEY, + REST_GUARDS_KEY, + REST_ROLES_KEY, +} from "./sharedTypes"; export type ContractGraphVersion = "croco.contract-graph.v1"; export type ContractDiagnosticSeverity = "error" | "warning"; @@ -23,13 +29,26 @@ export type ContractDiagnostic = { export type ContractGraphController = { readonly name: string; readonly path: string; + readonly guards: readonly string[]; + readonly roles: readonly string[]; readonly routeIds: readonly string[]; }; +export type ContractAccessMetadata = { + readonly guards: readonly string[]; + readonly roles: readonly string[]; +}; + +export type ContractPathParam = { + readonly token: string; + readonly name: string; +}; + export type ContractGraphRoute = RouteIR & { readonly routeId: string; readonly operationId: string; readonly controllerPath: string; + readonly access: ContractAccessMetadata; }; export type ContractGraph = { @@ -68,12 +87,14 @@ export function buildContractGraph(controllers: readonly Constructor[]): Contrac } const routes = extractRouteIR(controller).map((route) => - toContractGraphRoute(route, controllerMeta.path), + toContractGraphRoute(route, controllerMeta.path, controller), ); graphControllers.push({ name: controller.name, path: controllerMeta.path, + guards: getMetadataNames(Reflect.getMetadata(REST_GUARDS_KEY, controller)), + roles: getMetadataStrings(Reflect.getMetadata(REST_ROLES_KEY, controller)), routeIds: routes.map((route) => route.routeId), }); graphRoutes.push(...routes); @@ -116,7 +137,25 @@ export function formatContractDiagnostic(diagnostic: ContractDiagnostic): string return `${diagnostic.severity.toUpperCase()} ${diagnostic.code}${route}: ${diagnostic.message}`; } -function toContractGraphRoute(route: RouteIR, controllerPath: string): ContractGraphRoute { +export function getContractPathParamNames(path: string): string[] { + return getContractPathParams(path).map((param) => param.name); +} + +export function getContractPathParams(path: string): ContractPathParam[] { + return [...path.matchAll(/:([^/]+)/g)] + .map((match) => { + const token = match[1]; + + return { token, name: token.replace(/^\.\.\./, "") }; + }) + .filter((param) => param.name.length > 0); +} + +function toContractGraphRoute( + route: RouteIR, + controllerPath: string, + controllerCtor: Constructor, +): ContractGraphRoute { const routeId = `${route.controllerName}.${route.methodName}`; return { @@ -124,6 +163,18 @@ function toContractGraphRoute(route: RouteIR, controllerPath: string): ContractG routeId, operationId: routeId.replace(/[^A-Za-z0-9_]+/g, "_"), controllerPath, + access: { + guards: [ + ...getMetadataNames(Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor)), + ...getMetadataNames(Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor, route.methodName)), + ], + roles: [ + ...getMetadataStrings(Reflect.getMetadata(REST_ROLES_KEY, controllerCtor)), + ...getMetadataStrings( + Reflect.getMetadata(REST_ROLES_KEY, controllerCtor, route.methodName), + ), + ], + }, }; } @@ -151,7 +202,7 @@ function validateRoute(route: ContractGraphRoute): ContractDiagnostic[] { function validatePathParams(route: ContractGraphRoute): ContractDiagnostic[] { const diagnostics: ContractDiagnostic[] = []; - const pathParamNames = new Set(getPathParamNames(route.path)); + const pathParamNames = new Set(getContractPathParamNames(route.path)); const declaredParamNames = new Set( route.params.filter((param) => param.kind === "path").map((param) => param.name), ); @@ -309,10 +360,6 @@ function validateUniqueOperationIds(routes: readonly ContractGraphRoute[]): Cont return diagnostics; } -function getPathParamNames(path: string): string[] { - return [...path.matchAll(/:([^/]+)/g)].map((match) => match[1]).filter(Boolean); -} - function getRouteSchemas(route: ContractGraphRoute): z.ZodType[] { const schemas = [ route.inputSchemas.body, @@ -359,3 +406,35 @@ function getDiagnosticTarget(code: string): ContractDiagnosticTarget { return "route"; } + +function getMetadataNames(value: unknown): string[] { + if (!Array.isArray(value)) { + return []; + } + + return value.map(getMetadataName).filter((name): name is string => name !== null); +} + +function getMetadataName(value: unknown): string | null { + if (typeof value === "function" && value.name.length > 0) { + return value.name; + } + + if (value && typeof value === "object" && "constructor" in value) { + const constructor = value.constructor; + + if (typeof constructor === "function" && constructor.name.length > 0) { + return constructor.name; + } + } + + return null; +} + +function getMetadataStrings(value: unknown): string[] { + if (!Array.isArray(value)) { + return []; + } + + return value.filter((item): item is string => typeof item === "string"); +} diff --git a/packages/protocols-core/src/libs/sharedTypes.ts b/packages/protocols-core/src/libs/sharedTypes.ts index 2db1fa8f0..fbeb08898 100644 --- a/packages/protocols-core/src/libs/sharedTypes.ts +++ b/packages/protocols-core/src/libs/sharedTypes.ts @@ -6,6 +6,8 @@ export const REST_CONTROLLER_KEY = Symbol.for("croco:rest:controller"); export const REST_ROUTES_KEY = Symbol.for("croco:rest:routes"); export const REST_PARAMS_KEY = Symbol.for("croco:rest:params"); +export const REST_GUARDS_KEY = Symbol.for("croco:rest:guards"); +export const REST_ROLES_KEY = Symbol.for("croco:rest:roles"); export enum ParamType { PARAM = "param", diff --git a/packages/protocols-core/src/tests/ContractGraph.spec.ts b/packages/protocols-core/src/tests/ContractGraph.spec.ts index bfc3fa5a6..499d53c97 100644 --- a/packages/protocols-core/src/tests/ContractGraph.spec.ts +++ b/packages/protocols-core/src/tests/ContractGraph.spec.ts @@ -8,7 +8,16 @@ import { ContractGraphDiagnosticError, formatContractDiagnostic, } from "../libs/ContractGraph"; -import { Body, Controller, Get, Param, Post, Query } from "./helpers/test-decorators"; +import { + Body, + Controller, + Get, + Param, + Post, + Query, + Roles, + UseGuards, +} from "./helpers/test-decorators"; describe("buildContractGraph", () => { beforeEach(() => { @@ -34,6 +43,8 @@ describe("buildContractGraph", () => { { name: "UsersController", path: "/users", + guards: [], + roles: [], routeIds: ["UsersController.getUser", "UsersController.createUser"], }, ]); @@ -51,6 +62,51 @@ describe("buildContractGraph", () => { expect(graph.diagnostics).toEqual([]); }); + it("should normalize catch-all route parameters when validating path metadata", () => { + @Controller("/assets") + class AssetsController { + @Get("/:...id") + getAsset(@Param("id") _id: string): void {} + } + + const graph = buildContractGraph([AssetsController]); + + expect(graph.routes[0]).toMatchObject({ + routeId: "AssetsController.getAsset", + path: "/assets/:...id", + }); + expect(graph.diagnostics).toEqual([]); + expect(() => assertContractGraphHasNoErrors(graph)).not.toThrow(); + }); + + it("should expose auth and access metadata references when present", () => { + class AuthGuard {} + class AuditGuard {} + + @UseGuards(AuthGuard) + @Roles("admin") + @Controller("/admin") + class AdminController { + @UseGuards(AuditGuard) + @Roles("owner") + @Get("/:id") + getAdminAsset(@Param("id") _id: string): void {} + } + + const graph = buildContractGraph([AdminController]); + + expect(graph.controllers[0]).toMatchObject({ + name: "AdminController", + guards: ["AuthGuard"], + roles: ["admin"], + }); + expect(graph.routes[0]?.access).toEqual({ + guards: ["AuthGuard", "AuditGuard"], + roles: ["admin", "owner"], + }); + expect(graph.diagnostics).toEqual([]); + }); + it("should report unsupported and drift-prone route metadata as diagnostics", () => { @Controller("/hooks") class HooksController { diff --git a/packages/protocols-core/src/tests/helpers/test-decorators.ts b/packages/protocols-core/src/tests/helpers/test-decorators.ts index 623dc3cc8..1a852018a 100644 --- a/packages/protocols-core/src/tests/helpers/test-decorators.ts +++ b/packages/protocols-core/src/tests/helpers/test-decorators.ts @@ -5,7 +5,9 @@ import { type ParamMetadata, ParamType, REST_CONTROLLER_KEY, + REST_GUARDS_KEY, REST_PARAMS_KEY, + REST_ROLES_KEY, REST_ROUTES_KEY, type RouteMetadata, } from "../../libs/sharedTypes"; @@ -42,6 +44,28 @@ export function Header(name: string, schema?: z.ZodType): ParameterDecorator { return createParamDecorator(ParamType.HEADER, name, schema); } +export function UseGuards(...guards: Function[]): ClassDecorator & MethodDecorator { + return (target: object, propertyKey?: string | symbol) => { + if (propertyKey) { + Reflect.defineMetadata(REST_GUARDS_KEY, guards, target.constructor, propertyKey); + return; + } + + Reflect.defineMetadata(REST_GUARDS_KEY, guards, target); + }; +} + +export function Roles(...roles: string[]): ClassDecorator & MethodDecorator { + return (target: object, propertyKey?: string | symbol) => { + if (propertyKey) { + Reflect.defineMetadata(REST_ROLES_KEY, roles, target.constructor, propertyKey); + return; + } + + Reflect.defineMetadata(REST_ROLES_KEY, roles, target); + }; +} + function createRouteDecorator(method: string, path: string): MethodDecorator { return (target, propertyKey) => { const ctor = target.constructor; diff --git a/packages/rpc-codegen/src/libs/generate.ts b/packages/rpc-codegen/src/libs/generate.ts index 5893588d2..e8f82bc5c 100644 --- a/packages/rpc-codegen/src/libs/generate.ts +++ b/packages/rpc-codegen/src/libs/generate.ts @@ -2,6 +2,8 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { assertContractGraphHasNoErrors, + getContractPathParamNames, + getContractPathParams, type ContractGraph, type RouteIR, } from "@croco/protocols-core"; @@ -78,7 +80,7 @@ function assertGeneratedClientRoutes(routes: RouteIR[]): void { } function assertGeneratedClientPathParams(route: RouteIR): void { - const pathParamNames = new Set(getRoutePathParamNames(route.path)); + const pathParamNames = new Set(getContractPathParamNames(route.path)); const declaredParamNames = new Set( route.params.filter((param) => param.kind === "path").map((param) => param.name), ); @@ -524,7 +526,7 @@ function getObjectShape(schema: unknown): Record { } function getPathExpression(route: RouteIR): string { - const pathParams = getRoutePathParams(route.path); + const pathParams = getContractPathParams(route.path); if (pathParams.length === 0) { return `'${route.path}'`; @@ -541,20 +543,6 @@ function getPathExpression(route: RouteIR): string { return `\`${pathExpression}\``; } -function getRoutePathParamNames(pathname: string): string[] { - return getRoutePathParams(pathname).map((param) => param.name); -} - -function getRoutePathParams(pathname: string): { readonly token: string; readonly name: string }[] { - return [...pathname.matchAll(/:([^/]+)/g)] - .map((match) => { - const token = match[1]; - - return { token, name: token.replace(/^\.\.\./, "") }; - }) - .filter((param) => param.name.length > 0); -} - function getQueryStatements(route: RouteIR): string { if (!route.inputSchemas.query) { return ""; diff --git a/packages/rpc-codegen/src/tests/ContractCheckCli.spec.ts b/packages/rpc-codegen/src/tests/ContractCheckCli.spec.ts index 6afbe3f3a..89829d43f 100644 --- a/packages/rpc-codegen/src/tests/ContractCheckCli.spec.ts +++ b/packages/rpc-codegen/src/tests/ContractCheckCli.spec.ts @@ -39,8 +39,80 @@ describe("rpc-codegen contract check CLI", () => { }, CONTRACT_CHECK_TIMEOUT_MS, ); + + it( + "passes when a loaded controller uses a catch-all path parameter", + async () => { + fs.writeFileSync(path.join(sourceDir, "AssetsController.ts"), getCatchAllController()); + const stdout: string[] = []; + + const exitCode = await runCli(["--controllers", path.join(sourceDir, "*.ts"), "--check"], { + stdout: (message) => stdout.push(message), + }); + + expect(exitCode).toBe(0); + expect(stdout).toContain( + "Contract graph check passed for 1 route(s) across 1 controller(s).", + ); + }, + CONTRACT_CHECK_TIMEOUT_MS, + ); }); +function getCatchAllController(): string { + return `import 'reflect-metadata'; + +const REST_CONTROLLER_KEY = Symbol.for('croco:rest:controller'); +const REST_ROUTES_KEY = Symbol.for('croco:rest:routes'); +const REST_PARAMS_KEY = Symbol.for('croco:rest:params'); + +enum ParamType { + PARAM = 'param', +} + +type RouteMetadata = { + readonly method: string; + readonly path: string; + readonly methodName: string | symbol; +}; + +function Controller(controllerPath: string): ClassDecorator { + return (target) => { + Reflect.defineMetadata(REST_CONTROLLER_KEY, { path: controllerPath, target }, target); + }; +} + +function Get(routePath: string): MethodDecorator { + return (target, propertyKey) => { + const ctor = target.constructor; + const routes = (Reflect.getMetadata(REST_ROUTES_KEY, ctor) as RouteMetadata[] | undefined) ?? []; + + Reflect.defineMetadata(REST_ROUTES_KEY, [...routes, { method: 'GET', path: routePath, methodName: propertyKey }], ctor); + }; +} + +function Param(name: string): ParameterDecorator { + return (target, propertyKey, parameterIndex) => { + if (!propertyKey) return; + + const ctor = target.constructor; + const paramsMap = (Reflect.getMetadata(REST_PARAMS_KEY, ctor) as Map | undefined) ?? new Map(); + const params = paramsMap.get(propertyKey) ?? []; + + params.push({ type: ParamType.PARAM, index: parameterIndex, name }); + paramsMap.set(propertyKey, params); + Reflect.defineMetadata(REST_PARAMS_KEY, paramsMap, ctor); + }; +} + +@Controller('/assets') +export class AssetsController { + @Get('/:...id') + getAsset(@Param('id') _id: string): void {} +} +`; +} + function getMultipleBodyController(): string { return `import 'reflect-metadata'; import { z } from 'zod'; diff --git a/packages/rpc-codegen/src/tests/loadRoutes.spec.ts b/packages/rpc-codegen/src/tests/loadRoutes.spec.ts index d20a8081a..1649d7006 100644 --- a/packages/rpc-codegen/src/tests/loadRoutes.spec.ts +++ b/packages/rpc-codegen/src/tests/loadRoutes.spec.ts @@ -50,6 +50,8 @@ describe("loadRoutes", () => { { name: "UsersController", path: "/users", + guards: [], + roles: [], routeIds: ["UsersController.listUsers"], }, ]); From 7fba88fdec9fb530caf780b28a856df520e0cd33 Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 03:33:08 +0900 Subject: [PATCH 07/13] fix: stabilize contract graph access refs --- packages/protocols-core/src/index.ts | 1 + .../protocols-core/src/libs/ContractGraph.ts | 29 +++++++++------ .../src/tests/ContractGraph.spec.ts | 7 ++-- packages/rpc-codegen/src/libs/cli.ts | 6 ++-- packages/rpc-codegen/src/tests/Cli.spec.ts | 36 ++++++++++++++++--- 5 files changed, 59 insertions(+), 20 deletions(-) diff --git a/packages/protocols-core/src/index.ts b/packages/protocols-core/src/index.ts index ffa622442..8a5b44c94 100644 --- a/packages/protocols-core/src/index.ts +++ b/packages/protocols-core/src/index.ts @@ -21,6 +21,7 @@ export type { ContractGraph, ContractGraphController, ContractGraphRoute, + ContractMetadataReference, ContractPathParam, ContractGraphVersion, } from "./libs/ContractGraph"; diff --git a/packages/protocols-core/src/libs/ContractGraph.ts b/packages/protocols-core/src/libs/ContractGraph.ts index 336bd9104..c18b6f93f 100644 --- a/packages/protocols-core/src/libs/ContractGraph.ts +++ b/packages/protocols-core/src/libs/ContractGraph.ts @@ -29,13 +29,18 @@ export type ContractDiagnostic = { export type ContractGraphController = { readonly name: string; readonly path: string; - readonly guards: readonly string[]; + readonly guards: readonly ContractMetadataReference[]; readonly roles: readonly string[]; readonly routeIds: readonly string[]; }; +export type ContractMetadataReference = { + readonly kind: "constructor" | "instance"; + readonly name: string; +}; + export type ContractAccessMetadata = { - readonly guards: readonly string[]; + readonly guards: readonly ContractMetadataReference[]; readonly roles: readonly string[]; }; @@ -93,7 +98,7 @@ export function buildContractGraph(controllers: readonly Constructor[]): Contrac graphControllers.push({ name: controller.name, path: controllerMeta.path, - guards: getMetadataNames(Reflect.getMetadata(REST_GUARDS_KEY, controller)), + guards: getMetadataReferences(Reflect.getMetadata(REST_GUARDS_KEY, controller)), roles: getMetadataStrings(Reflect.getMetadata(REST_ROLES_KEY, controller)), routeIds: routes.map((route) => route.routeId), }); @@ -165,8 +170,10 @@ function toContractGraphRoute( controllerPath, access: { guards: [ - ...getMetadataNames(Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor)), - ...getMetadataNames(Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor, route.methodName)), + ...getMetadataReferences(Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor)), + ...getMetadataReferences( + Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor, route.methodName), + ), ], roles: [ ...getMetadataStrings(Reflect.getMetadata(REST_ROLES_KEY, controllerCtor)), @@ -407,24 +414,26 @@ function getDiagnosticTarget(code: string): ContractDiagnosticTarget { return "route"; } -function getMetadataNames(value: unknown): string[] { +function getMetadataReferences(value: unknown): ContractMetadataReference[] { if (!Array.isArray(value)) { return []; } - return value.map(getMetadataName).filter((name): name is string => name !== null); + return value + .map(getMetadataReference) + .filter((reference): reference is ContractMetadataReference => reference !== null); } -function getMetadataName(value: unknown): string | null { +function getMetadataReference(value: unknown): ContractMetadataReference | null { if (typeof value === "function" && value.name.length > 0) { - return value.name; + return { kind: "constructor", name: value.name }; } if (value && typeof value === "object" && "constructor" in value) { const constructor = value.constructor; if (typeof constructor === "function" && constructor.name.length > 0) { - return constructor.name; + return { kind: "instance", name: constructor.name }; } } diff --git a/packages/protocols-core/src/tests/ContractGraph.spec.ts b/packages/protocols-core/src/tests/ContractGraph.spec.ts index 499d53c97..e97ee6716 100644 --- a/packages/protocols-core/src/tests/ContractGraph.spec.ts +++ b/packages/protocols-core/src/tests/ContractGraph.spec.ts @@ -97,11 +97,14 @@ describe("buildContractGraph", () => { expect(graph.controllers[0]).toMatchObject({ name: "AdminController", - guards: ["AuthGuard"], + guards: [{ kind: "constructor", name: "AuthGuard" }], roles: ["admin"], }); expect(graph.routes[0]?.access).toEqual({ - guards: ["AuthGuard", "AuditGuard"], + guards: [ + { kind: "constructor", name: "AuthGuard" }, + { kind: "constructor", name: "AuditGuard" }, + ], roles: ["admin", "owner"], }); expect(graph.diagnostics).toEqual([]); diff --git a/packages/rpc-codegen/src/libs/cli.ts b/packages/rpc-codegen/src/libs/cli.ts index db1eb120d..7ed784359 100644 --- a/packages/rpc-codegen/src/libs/cli.ts +++ b/packages/rpc-codegen/src/libs/cli.ts @@ -37,10 +37,7 @@ export async function runCli(args: readonly string[], io: CliIo = defaultCliIo): return 1; } - const [{ generateClientFilesFromContractGraph }, { loadContractGraph }] = await Promise.all([ - import("./generate"), - import("./loadRoutes"), - ]); + const { loadContractGraph } = await import("./loadRoutes"); const graph = await loadContractGraph(result.options.controllers); if (result.options.check) { @@ -64,6 +61,7 @@ export async function runCli(args: readonly string[], io: CliIo = defaultCliIo): return 1; } + const { generateClientFilesFromContractGraph } = await import("./generate"); const files = generateClientFilesFromContractGraph(graph, outDir, { reactQuery: result.options.reactQuery, }); diff --git a/packages/rpc-codegen/src/tests/Cli.spec.ts b/packages/rpc-codegen/src/tests/Cli.spec.ts index b2c098140..0e2dae4d8 100644 --- a/packages/rpc-codegen/src/tests/Cli.spec.ts +++ b/packages/rpc-codegen/src/tests/Cli.spec.ts @@ -7,7 +7,15 @@ const generationModuleImports = vi.hoisted(() => ({ loadContractGraph: 0, graph: { version: "croco.contract-graph.v1", - controllers: [{ name: "UsersController", path: "/users", routeIds: ["UsersController.list"] }], + controllers: [ + { + name: "UsersController", + path: "/users", + guards: [], + roles: [], + routeIds: ["UsersController.list"], + }, + ], routes: [{ routeId: "UsersController.list" }], diagnostics: [] as ContractDiagnosticFixture[], }, @@ -63,7 +71,13 @@ describe("rpc-codegen CLI", () => { generationModuleImports.graph = { version: "croco.contract-graph.v1", controllers: [ - { name: "UsersController", path: "/users", routeIds: ["UsersController.list"] }, + { + name: "UsersController", + path: "/users", + guards: [], + roles: [], + routeIds: ["UsersController.list"], + }, ], routes: [{ routeId: "UsersController.list" }], diagnostics: [], @@ -110,7 +124,13 @@ describe("rpc-codegen CLI", () => { generationModuleImports.graph = { version: "croco.contract-graph.v1", controllers: [ - { name: "HooksController", path: "/hooks", routeIds: ["HooksController.handle"] }, + { + name: "HooksController", + path: "/hooks", + guards: [], + roles: [], + routeIds: ["HooksController.handle"], + }, ], routes: [{ routeId: "HooksController.handle" }], diagnostics: [ @@ -133,6 +153,7 @@ describe("rpc-codegen CLI", () => { "ERROR contract-route-unsupported-all-method HooksController.handle: Use explicit HTTP method decorators.", ); expect(stdout).toContain("Contract graph check failed with 1 error(s)."); + expect(generationModuleImports.generate).toBe(0); expect(generationModuleImports.generateClientFiles).toBe(0); expect(generationModuleImports.loadContractGraph).toBe(1); }); @@ -141,7 +162,13 @@ describe("rpc-codegen CLI", () => { generationModuleImports.graph = { version: "croco.contract-graph.v1", controllers: [ - { name: "UsersController", path: "/users", routeIds: ["UsersController.getUser"] }, + { + name: "UsersController", + path: "/users", + guards: [], + roles: [], + routeIds: ["UsersController.getUser"], + }, ], routes: [{ routeId: "UsersController.getUser" }], diagnostics: [ @@ -166,6 +193,7 @@ describe("rpc-codegen CLI", () => { expect(stdout).toContain( "Contract graph contains 1 error(s); fix them before generating clients.", ); + expect(generationModuleImports.generate).toBe(0); expect(generationModuleImports.generateClientFiles).toBe(0); expect(generationModuleImports.loadContractGraph).toBe(1); }); From e866e3d827ed6ed4129c661c435349d4fbbe5d26 Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 03:41:04 +0900 Subject: [PATCH 08/13] fix: identify contract graph guard refs --- .../protocols-core/src/libs/ContractGraph.ts | 41 +++++++++++++++---- .../src/tests/ContractGraph.spec.ts | 26 ++++++++++-- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/packages/protocols-core/src/libs/ContractGraph.ts b/packages/protocols-core/src/libs/ContractGraph.ts index c18b6f93f..c3bc6b692 100644 --- a/packages/protocols-core/src/libs/ContractGraph.ts +++ b/packages/protocols-core/src/libs/ContractGraph.ts @@ -35,8 +35,11 @@ export type ContractGraphController = { }; export type ContractMetadataReference = { + readonly type: "rest.guard"; + readonly id: string; readonly kind: "constructor" | "instance"; readonly name: string; + readonly declaredAt: "controller" | "route"; }; export type ContractAccessMetadata = { @@ -98,7 +101,7 @@ export function buildContractGraph(controllers: readonly Constructor[]): Contrac graphControllers.push({ name: controller.name, path: controllerMeta.path, - guards: getMetadataReferences(Reflect.getMetadata(REST_GUARDS_KEY, controller)), + guards: getMetadataReferences(Reflect.getMetadata(REST_GUARDS_KEY, controller), "controller"), roles: getMetadataStrings(Reflect.getMetadata(REST_ROLES_KEY, controller)), routeIds: routes.map((route) => route.routeId), }); @@ -170,9 +173,13 @@ function toContractGraphRoute( controllerPath, access: { guards: [ - ...getMetadataReferences(Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor)), + ...getMetadataReferences( + Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor), + "controller", + ), ...getMetadataReferences( Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor, route.methodName), + "route", ), ], roles: [ @@ -414,32 +421,52 @@ function getDiagnosticTarget(code: string): ContractDiagnosticTarget { return "route"; } -function getMetadataReferences(value: unknown): ContractMetadataReference[] { +function getMetadataReferences( + value: unknown, + declaredAt: ContractMetadataReference["declaredAt"], +): ContractMetadataReference[] { if (!Array.isArray(value)) { return []; } return value - .map(getMetadataReference) + .map((item) => getMetadataReference(item, declaredAt)) .filter((reference): reference is ContractMetadataReference => reference !== null); } -function getMetadataReference(value: unknown): ContractMetadataReference | null { +function getMetadataReference( + value: unknown, + declaredAt: ContractMetadataReference["declaredAt"], +): ContractMetadataReference | null { if (typeof value === "function" && value.name.length > 0) { - return { kind: "constructor", name: value.name }; + return createGuardReference("constructor", value.name, declaredAt); } if (value && typeof value === "object" && "constructor" in value) { const constructor = value.constructor; if (typeof constructor === "function" && constructor.name.length > 0) { - return { kind: "instance", name: constructor.name }; + return createGuardReference("instance", constructor.name, declaredAt); } } return null; } +function createGuardReference( + kind: ContractMetadataReference["kind"], + name: string, + declaredAt: ContractMetadataReference["declaredAt"], +): ContractMetadataReference { + return { + type: "rest.guard", + id: `rest.guard:${kind}:${name}`, + kind, + name, + declaredAt, + }; +} + function getMetadataStrings(value: unknown): string[] { if (!Array.isArray(value)) { return []; diff --git a/packages/protocols-core/src/tests/ContractGraph.spec.ts b/packages/protocols-core/src/tests/ContractGraph.spec.ts index e97ee6716..e3ad23319 100644 --- a/packages/protocols-core/src/tests/ContractGraph.spec.ts +++ b/packages/protocols-core/src/tests/ContractGraph.spec.ts @@ -97,13 +97,33 @@ describe("buildContractGraph", () => { expect(graph.controllers[0]).toMatchObject({ name: "AdminController", - guards: [{ kind: "constructor", name: "AuthGuard" }], + guards: [ + { + type: "rest.guard", + id: "rest.guard:constructor:AuthGuard", + kind: "constructor", + name: "AuthGuard", + declaredAt: "controller", + }, + ], roles: ["admin"], }); expect(graph.routes[0]?.access).toEqual({ guards: [ - { kind: "constructor", name: "AuthGuard" }, - { kind: "constructor", name: "AuditGuard" }, + { + type: "rest.guard", + id: "rest.guard:constructor:AuthGuard", + kind: "constructor", + name: "AuthGuard", + declaredAt: "controller", + }, + { + type: "rest.guard", + id: "rest.guard:constructor:AuditGuard", + kind: "constructor", + name: "AuditGuard", + declaredAt: "route", + }, ], roles: ["admin", "owner"], }); From 4ab2805663e7de30d35398e503d5b01c5c9912a5 Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 03:52:05 +0900 Subject: [PATCH 09/13] fix: disambiguate contract graph guard refs --- packages/protocols-core/src/index.ts | 1 + .../protocols-core/src/libs/ContractGraph.ts | 35 ++++++++++++++++--- .../src/tests/ContractGraph.spec.ts | 27 +++++++++----- 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/packages/protocols-core/src/index.ts b/packages/protocols-core/src/index.ts index 8a5b44c94..8e30cd07e 100644 --- a/packages/protocols-core/src/index.ts +++ b/packages/protocols-core/src/index.ts @@ -21,6 +21,7 @@ export type { ContractGraph, ContractGraphController, ContractGraphRoute, + ContractMetadataOwner, ContractMetadataReference, ContractPathParam, ContractGraphVersion, diff --git a/packages/protocols-core/src/libs/ContractGraph.ts b/packages/protocols-core/src/libs/ContractGraph.ts index c3bc6b692..582a1ce04 100644 --- a/packages/protocols-core/src/libs/ContractGraph.ts +++ b/packages/protocols-core/src/libs/ContractGraph.ts @@ -40,6 +40,14 @@ export type ContractMetadataReference = { readonly kind: "constructor" | "instance"; readonly name: string; readonly declaredAt: "controller" | "route"; + readonly owner: ContractMetadataOwner; + readonly index: number; +}; + +export type ContractMetadataOwner = { + readonly controllerName: string; + readonly routeId?: string; + readonly methodName?: string; }; export type ContractAccessMetadata = { @@ -101,7 +109,13 @@ export function buildContractGraph(controllers: readonly Constructor[]): Contrac graphControllers.push({ name: controller.name, path: controllerMeta.path, - guards: getMetadataReferences(Reflect.getMetadata(REST_GUARDS_KEY, controller), "controller"), + guards: getMetadataReferences( + Reflect.getMetadata(REST_GUARDS_KEY, controller), + "controller", + { + controllerName: controller.name, + }, + ), roles: getMetadataStrings(Reflect.getMetadata(REST_ROLES_KEY, controller)), routeIds: routes.map((route) => route.routeId), }); @@ -176,10 +190,12 @@ function toContractGraphRoute( ...getMetadataReferences( Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor), "controller", + { controllerName: route.controllerName }, ), ...getMetadataReferences( Reflect.getMetadata(REST_GUARDS_KEY, controllerCtor, route.methodName), "route", + { controllerName: route.controllerName, methodName: route.methodName, routeId }, ), ], roles: [ @@ -424,29 +440,32 @@ function getDiagnosticTarget(code: string): ContractDiagnosticTarget { function getMetadataReferences( value: unknown, declaredAt: ContractMetadataReference["declaredAt"], + owner: ContractMetadataOwner, ): ContractMetadataReference[] { if (!Array.isArray(value)) { return []; } return value - .map((item) => getMetadataReference(item, declaredAt)) + .map((item, index) => getMetadataReference(item, declaredAt, owner, index)) .filter((reference): reference is ContractMetadataReference => reference !== null); } function getMetadataReference( value: unknown, declaredAt: ContractMetadataReference["declaredAt"], + owner: ContractMetadataOwner, + index: number, ): ContractMetadataReference | null { if (typeof value === "function" && value.name.length > 0) { - return createGuardReference("constructor", value.name, declaredAt); + return createGuardReference("constructor", value.name, declaredAt, owner, index); } if (value && typeof value === "object" && "constructor" in value) { const constructor = value.constructor; if (typeof constructor === "function" && constructor.name.length > 0) { - return createGuardReference("instance", constructor.name, declaredAt); + return createGuardReference("instance", constructor.name, declaredAt, owner, index); } } @@ -457,13 +476,19 @@ function createGuardReference( kind: ContractMetadataReference["kind"], name: string, declaredAt: ContractMetadataReference["declaredAt"], + owner: ContractMetadataOwner, + index: number, ): ContractMetadataReference { + const ownerId = owner.routeId ?? owner.controllerName; + return { type: "rest.guard", - id: `rest.guard:${kind}:${name}`, + id: `rest.guard:${declaredAt}:${ownerId}:${index}:${kind}:${name}`, kind, name, declaredAt, + owner, + index, }; } diff --git a/packages/protocols-core/src/tests/ContractGraph.spec.ts b/packages/protocols-core/src/tests/ContractGraph.spec.ts index e3ad23319..b30586b43 100644 --- a/packages/protocols-core/src/tests/ContractGraph.spec.ts +++ b/packages/protocols-core/src/tests/ContractGraph.spec.ts @@ -80,8 +80,8 @@ describe("buildContractGraph", () => { }); it("should expose auth and access metadata references when present", () => { - class AuthGuard {} - class AuditGuard {} + const AuthGuard = class SharedAccessGuard {}; + const AuditGuard = class SharedAccessGuard {}; @UseGuards(AuthGuard) @Roles("admin") @@ -100,10 +100,12 @@ describe("buildContractGraph", () => { guards: [ { type: "rest.guard", - id: "rest.guard:constructor:AuthGuard", + id: "rest.guard:controller:AdminController:0:constructor:SharedAccessGuard", kind: "constructor", - name: "AuthGuard", + name: "SharedAccessGuard", declaredAt: "controller", + owner: { controllerName: "AdminController" }, + index: 0, }, ], roles: ["admin"], @@ -112,21 +114,30 @@ describe("buildContractGraph", () => { guards: [ { type: "rest.guard", - id: "rest.guard:constructor:AuthGuard", + id: "rest.guard:controller:AdminController:0:constructor:SharedAccessGuard", kind: "constructor", - name: "AuthGuard", + name: "SharedAccessGuard", declaredAt: "controller", + owner: { controllerName: "AdminController" }, + index: 0, }, { type: "rest.guard", - id: "rest.guard:constructor:AuditGuard", + id: "rest.guard:route:AdminController.getAdminAsset:0:constructor:SharedAccessGuard", kind: "constructor", - name: "AuditGuard", + name: "SharedAccessGuard", declaredAt: "route", + owner: { + controllerName: "AdminController", + methodName: "getAdminAsset", + routeId: "AdminController.getAdminAsset", + }, + index: 0, }, ], roles: ["admin", "owner"], }); + expect(graph.routes[0]?.access.guards[0]?.id).not.toBe(graph.routes[0]?.access.guards[1]?.id); expect(graph.diagnostics).toEqual([]); }); From 7fadac887d769de96dd51fba971031fcede407d0 Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 04:01:40 +0900 Subject: [PATCH 10/13] fix: align catch-all route runtime --- .changeset/contract-graph-check.md | 1 + .../protocols-core/src/libs/ContractGraph.ts | 28 ++++++++++++++ .../src/tests/ContractGraph.spec.ts | 37 +++++++++++++++++++ .../transports-http/src/libs/RouteCompiler.ts | 10 ++++- .../src/tests/CrocoApp.spec.ts | 15 ++++++++ 5 files changed, 90 insertions(+), 1 deletion(-) diff --git a/.changeset/contract-graph-check.md b/.changeset/contract-graph-check.md index b24689e4a..14fea2cf5 100644 --- a/.changeset/contract-graph-check.md +++ b/.changeset/contract-graph-check.md @@ -1,5 +1,6 @@ --- "@croco/protocols-core": patch +"@croco/transports-http": patch "@croco/openapi-spec": patch "@croco/rpc-codegen": patch "@croco/cli": patch diff --git a/packages/protocols-core/src/libs/ContractGraph.ts b/packages/protocols-core/src/libs/ContractGraph.ts index 582a1ce04..d4db7852c 100644 --- a/packages/protocols-core/src/libs/ContractGraph.ts +++ b/packages/protocols-core/src/libs/ContractGraph.ts @@ -126,6 +126,7 @@ export function buildContractGraph(controllers: readonly Constructor[]): Contrac } } + diagnostics.push(...validateUniqueControllerNames(graphControllers)); diagnostics.push(...validateUniqueRouteIds(graphRoutes)); diagnostics.push(...validateUniqueOperationIds(graphRoutes)); @@ -365,6 +366,33 @@ function validateUniqueRouteIds(routes: readonly ContractGraphRoute[]): Contract return diagnostics; } +function validateUniqueControllerNames( + controllers: readonly ContractGraphController[], +): ContractDiagnostic[] { + const diagnostics: ContractDiagnostic[] = []; + const controllerNames = new Map(); + + for (const controller of controllers) { + const existingController = controllerNames.get(controller.name); + + if (existingController) { + diagnostics.push({ + code: "contract-controller-duplicate-name", + severity: "error", + target: "controller", + controllerName: controller.name, + path: controller.path, + message: `Controller name '${controller.name}' is already used for path '${existingController.path}'. Controller names must be unique because route ids and access metadata references use them as contract identity.`, + }); + continue; + } + + controllerNames.set(controller.name, controller); + } + + return diagnostics; +} + function validateUniqueOperationIds(routes: readonly ContractGraphRoute[]): ContractDiagnostic[] { const diagnostics: ContractDiagnostic[] = []; const operationIds = new Map(); diff --git a/packages/protocols-core/src/tests/ContractGraph.spec.ts b/packages/protocols-core/src/tests/ContractGraph.spec.ts index b30586b43..c18107d27 100644 --- a/packages/protocols-core/src/tests/ContractGraph.spec.ts +++ b/packages/protocols-core/src/tests/ContractGraph.spec.ts @@ -233,4 +233,41 @@ describe("buildContractGraph", () => { ]); expect(() => assertContractGraphHasNoErrors(graph)).toThrow(ContractGraphDiagnosticError); }); + + it("should reject duplicate controller names used as contract identity", () => { + let FirstController!: new () => unknown; + let SecondController!: new () => unknown; + + { + @Controller("/first") + class DuplicateController { + @Get("/one") + one(): void {} + } + + FirstController = DuplicateController; + } + + { + @Controller("/second") + class DuplicateController { + @Get("/two") + two(): void {} + } + + SecondController = DuplicateController; + } + + const graph = buildContractGraph([FirstController, SecondController]); + + expect(graph.diagnostics).toEqual([ + expect.objectContaining({ + code: "contract-controller-duplicate-name", + severity: "error", + target: "controller", + controllerName: "DuplicateController", + }), + ]); + expect(() => assertContractGraphHasNoErrors(graph)).toThrow(ContractGraphDiagnosticError); + }); }); diff --git a/packages/transports-http/src/libs/RouteCompiler.ts b/packages/transports-http/src/libs/RouteCompiler.ts index 64c1ab083..f28b2d69e 100644 --- a/packages/transports-http/src/libs/RouteCompiler.ts +++ b/packages/transports-http/src/libs/RouteCompiler.ts @@ -109,7 +109,7 @@ export class RouteCompiler { routeIR: RouteIR, options: CompileOptions, ): CompiledRoute { - const fullPath = this.joinPaths("", routeIR.path); + const fullPath = this.toRuntimeRoutePath(this.joinPaths("", routeIR.path)); const paramResolver = new ParamResolver((pipe) => instantiateProvider(pipe, options.container)); // Instantiate guards/interceptors/filters once at compile time (not per-request) @@ -187,4 +187,12 @@ export class RouteCompiler { // trailing slash 제거 (루트 제외) return result.length > 1 && result.endsWith("/") ? result.slice(0, -1) : result || "/"; } + + private toRuntimeRoutePath(path: string): string { + return path.replace(/:([^/]+)/g, (token, paramToken: string) => { + const name = paramToken.replace(/^\.\.\./, ""); + + return name === paramToken || name.length === 0 ? token : `:${name}{.+}`; + }); + } } diff --git a/packages/transports-http/src/tests/CrocoApp.spec.ts b/packages/transports-http/src/tests/CrocoApp.spec.ts index b01804e6f..91ed4631d 100644 --- a/packages/transports-http/src/tests/CrocoApp.spec.ts +++ b/packages/transports-http/src/tests/CrocoApp.spec.ts @@ -70,6 +70,11 @@ describe("CrocoApp", () => { return { id, name: "Test User" }; } + @Get("/assets/:...id") + getAsset(@Param("id") id: string) { + return { id }; + } + @Post("/users") createUser(@Body() body: unknown) { return { created: true, data: body }; @@ -269,6 +274,16 @@ describe("CrocoApp", () => { expect(json).toEqual({ id: "123", name: "Test User" }); }); + it("should extract catch-all path params under the declared parameter name", async () => { + const app = createApp({ controllers: [TestController] }); + + const response = await app.fetch(new Request("http://localhost/api/assets/icons/logo.svg")); + + expect(response.status).toBe(200); + const json = await response.json(); + expect(json).toEqual({ id: "icons/logo.svg" }); + }); + it("should return headers without a response body for HEAD requests", async () => { const app = createApp({ controllers: [TestController] }); From 64df88d17d83cdc8fc621606aec84164e21b229b Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 13:26:30 +0900 Subject: [PATCH 11/13] fix: preserve generated path param boundaries --- packages/openapi-spec/src/libs/emitOpenAPI.ts | 11 +++++-- .../src/tests/emitOpenAPI.spec.ts | 28 ++++++++++++++++ .../src/tests/ContractGraph.spec.ts | 3 +- packages/rpc-codegen/src/libs/generate.ts | 13 ++++---- .../rpc-codegen/src/tests/codegen.spec.ts | 32 +++++++++++++++++++ 5 files changed, 76 insertions(+), 11 deletions(-) diff --git a/packages/openapi-spec/src/libs/emitOpenAPI.ts b/packages/openapi-spec/src/libs/emitOpenAPI.ts index 5875cd0ac..6d739524f 100644 --- a/packages/openapi-spec/src/libs/emitOpenAPI.ts +++ b/packages/openapi-spec/src/libs/emitOpenAPI.ts @@ -303,10 +303,15 @@ function toOpenAPIParamLocation(kind: ParamIR["kind"]): OpenAPIParamLocation { } function toOpenAPIPath(path: string): string { - return getContractPathParams(path).reduce( - (currentPath, param) => currentPath.split(`:${param.token}`).join(`{${param.name}}`), - path, + const paramsByToken = new Map( + getContractPathParams(path).map((param) => [param.token, param.name]), ); + + return path.replace(/:([^/]+)/g, (tokenWithPrefix, token: string) => { + const name = paramsByToken.get(token); + + return name ? `{${name}}` : tokenWithPrefix; + }); } function toHttpMethod(route: ContractGraphRoute): HttpMethod { diff --git a/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts b/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts index 3b226e90d..2c920bb61 100644 --- a/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts +++ b/packages/openapi-spec/src/tests/emitOpenAPI.spec.ts @@ -91,6 +91,34 @@ describe("emitOpenAPI", () => { expect(spec.paths?.["/assets/{...id}"]).toBeUndefined(); }); + it("should not rewrite path parameters with matching prefixes", () => { + @Controller("/pairs") + class PairsController { + @Get("/:id/:id2") + compare(@Param("id") _id: string, @Param("id2") _id2: string): void {} + } + + const graph = buildContractGraph([PairsController]); + const spec = emitOpenAPIFromContractGraph(graph); + + expect(graph.diagnostics).toEqual([]); + expect(spec.paths?.["/pairs/{id}/{id2}"]?.get?.parameters).toEqual([ + { + in: "path", + name: "id", + required: true, + schema: { type: "string" }, + }, + { + in: "path", + name: "id2", + required: true, + schema: { type: "string" }, + }, + ]); + expect(spec.paths?.["/pairs/{id}/{id}2"]).toBeUndefined(); + }); + it("should apply document metadata options", () => { @Controller("/accounts") class AccountsController { diff --git a/packages/protocols-core/src/tests/ContractGraph.spec.ts b/packages/protocols-core/src/tests/ContractGraph.spec.ts index c18107d27..e4477c8b2 100644 --- a/packages/protocols-core/src/tests/ContractGraph.spec.ts +++ b/packages/protocols-core/src/tests/ContractGraph.spec.ts @@ -1,6 +1,6 @@ import "reflect-metadata"; import { Container } from "typedi"; -import { beforeEach, describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { z } from "zod"; import { assertContractGraphHasNoErrors, @@ -22,6 +22,7 @@ import { describe("buildContractGraph", () => { beforeEach(() => { Container.reset(); + vi.restoreAllMocks(); }); it("should build stable controller, route id, operation id, and schema graph nodes", () => { diff --git a/packages/rpc-codegen/src/libs/generate.ts b/packages/rpc-codegen/src/libs/generate.ts index e8f82bc5c..2914702ac 100644 --- a/packages/rpc-codegen/src/libs/generate.ts +++ b/packages/rpc-codegen/src/libs/generate.ts @@ -532,13 +532,12 @@ function getPathExpression(route: RouteIR): string { return `'${route.path}'`; } - const pathExpression = pathParams.reduce( - (currentPath, param) => - currentPath - .split(`:${param.token}`) - .join(`\${encodeURIComponent(String(input.path.${param.name}))}`), - route.path, - ); + const paramsByToken = new Map(pathParams.map((param) => [param.token, param.name])); + const pathExpression = route.path.replace(/:([^/]+)/g, (tokenWithPrefix, token: string) => { + const name = paramsByToken.get(token); + + return name ? `\${encodeURIComponent(String(input.path.${name}))}` : tokenWithPrefix; + }); return `\`${pathExpression}\``; } diff --git a/packages/rpc-codegen/src/tests/codegen.spec.ts b/packages/rpc-codegen/src/tests/codegen.spec.ts index 4367714c8..19af912cf 100644 --- a/packages/rpc-codegen/src/tests/codegen.spec.ts +++ b/packages/rpc-codegen/src/tests/codegen.spec.ts @@ -456,6 +456,38 @@ describe("generateClientFiles", () => { ); }); + it("should not rewrite path parameters with matching prefixes", () => { + const routes: RouteIR[] = [ + { + controllerName: "PairController", + methodName: "compare", + httpMethod: "GET", + path: "/pairs/:id/:id2", + params: [ + { kind: "path", name: "id", schema: null }, + { kind: "path", name: "id2", schema: null }, + ], + inputSchema: null, + inputSchemas: { + body: null, + path: z.object({ id: z.string(), id2: z.string() }) as any, + query: null, + headers: null, + }, + outputSchema: null, + domain: null, + }, + ]; + + const files = generateClientFiles(routes, TEMP_DIR); + + const content = fs.readFileSync(files[0], "utf-8"); + expect(content).toContain( + "const path = `/pairs/${encodeURIComponent(String(input.path.id))}/${encodeURIComponent(String(input.path.id2))}`;", + ); + expect(content).not.toContain("${encodeURIComponent(String(input.path.id))}2"); + }); + it("should normalize catch-all path parameters when generating fetch paths", () => { const routes: RouteIR[] = [ { From fa1058ee110ebc7b9693549b74bad21e87ed3970 Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 15:14:25 +0900 Subject: [PATCH 12/13] fix: preserve contract graph edge metadata --- .../protocols-core/src/libs/ContractGraph.ts | 90 +++++++++++++++++-- .../src/tests/ContractGraph.spec.ts | 66 ++++++++++++-- packages/rpc-codegen/src/libs/generate.ts | 14 ++- .../rpc-codegen/src/tests/codegen.spec.ts | 34 ++++++- packages/rpc-codegen/vitest.config.ts | 17 ++++ 5 files changed, 204 insertions(+), 17 deletions(-) create mode 100644 packages/rpc-codegen/vitest.config.ts diff --git a/packages/protocols-core/src/libs/ContractGraph.ts b/packages/protocols-core/src/libs/ContractGraph.ts index d4db7852c..580289d3d 100644 --- a/packages/protocols-core/src/libs/ContractGraph.ts +++ b/packages/protocols-core/src/libs/ContractGraph.ts @@ -326,7 +326,9 @@ function validateSchemaEffects(route: ContractGraphRoute): ContractDiagnostic[] const diagnostics: ContractDiagnostic[] = []; for (const schema of getRouteSchemas(route)) { - if (isZodEffects(schema)) { + const effectsCount = countZodEffects(schema); + + for (let index = 0; index < effectsCount; index += 1) { diagnostics.push( createRouteDiagnostic( route, @@ -435,6 +437,74 @@ function isZodEffects(schema: z.ZodType): boolean { return schema.constructor.name === "ZodEffects"; } +function countZodEffects(schema: z.ZodType, seen = new Set()): number { + if (seen.has(schema)) { + return 0; + } + + seen.add(schema); + + const currentCount = isZodEffects(schema) ? 1 : 0; + const nestedCount = getNestedZodSchemas(schema).reduce( + (count, nestedSchema) => count + countZodEffects(nestedSchema, seen), + 0, + ); + + return currentCount + nestedCount; +} + +function getNestedZodSchemas(schema: z.ZodType): z.ZodType[] { + const definition = getZodDefinition(schema); + + if (!definition) { + return []; + } + + const nestedSchemas = [ + ...Object.values(getZodObjectShape(definition)), + definition.innerType, + definition.schema, + definition.type, + definition.element, + ...(Array.isArray(definition.options) ? definition.options : []), + ]; + + return nestedSchemas.filter(isZodType); +} + +type ZodDefinition = { + readonly shape?: unknown; + readonly innerType?: unknown; + readonly schema?: unknown; + readonly type?: unknown; + readonly element?: unknown; + readonly options?: unknown; +}; + +function getZodDefinition(schema: z.ZodType): ZodDefinition | undefined { + if (!schema || typeof schema !== "object" || !("_def" in schema)) { + return undefined; + } + + return schema._def as ZodDefinition; +} + +function getZodObjectShape(definition: ZodDefinition): Record { + const shape = typeof definition.shape === "function" ? definition.shape() : definition.shape; + + return shape && typeof shape === "object" ? (shape as Record) : {}; +} + +function isZodType(value: unknown): value is z.ZodType { + if (!value || typeof value !== "object") { + return false; + } + + const candidate = value as { readonly safeParse?: unknown }; + + return typeof candidate.safeParse === "function"; +} + function createRouteDiagnostic( route: ContractGraphRoute, code: string, @@ -485,21 +555,31 @@ function getMetadataReference( owner: ContractMetadataOwner, index: number, ): ContractMetadataReference | null { - if (typeof value === "function" && value.name.length > 0) { - return createGuardReference("constructor", value.name, declaredAt, owner, index); + if (typeof value === "function") { + return createGuardReference("constructor", getMetadataName(value), declaredAt, owner, index); } if (value && typeof value === "object" && "constructor" in value) { const constructor = value.constructor; - if (typeof constructor === "function" && constructor.name.length > 0) { - return createGuardReference("instance", constructor.name, declaredAt, owner, index); + if (typeof constructor === "function") { + return createGuardReference( + "instance", + getMetadataName(constructor), + declaredAt, + owner, + index, + ); } } return null; } +function getMetadataName(value: { readonly name: string }): string { + return value.name.length > 0 ? value.name : "anonymous"; +} + function createGuardReference( kind: ContractMetadataReference["kind"], name: string, diff --git a/packages/protocols-core/src/tests/ContractGraph.spec.ts b/packages/protocols-core/src/tests/ContractGraph.spec.ts index e4477c8b2..ada1bcf54 100644 --- a/packages/protocols-core/src/tests/ContractGraph.spec.ts +++ b/packages/protocols-core/src/tests/ContractGraph.spec.ts @@ -142,6 +142,35 @@ describe("buildContractGraph", () => { expect(graph.diagnostics).toEqual([]); }); + it("should preserve unnamed guard metadata references", () => { + const unnamedGuard = function Guard() {}; + Object.defineProperty(unnamedGuard, "name", { value: "" }); + + @UseGuards(unnamedGuard) + @Controller("/admin") + class AdminController { + @Get("/") + getAdmin(): void {} + } + + const graph = buildContractGraph([AdminController]); + + expect(unnamedGuard.name).toBe(""); + expect(graph.controllers[0]?.guards).toEqual([ + { + type: "rest.guard", + id: "rest.guard:controller:AdminController:0:constructor:anonymous", + kind: "constructor", + name: "anonymous", + declaredAt: "controller", + owner: { controllerName: "AdminController" }, + index: 0, + }, + ]); + expect(graph.routes[0]?.access.guards[0]?.name).toBe("anonymous"); + expect(graph.diagnostics).toEqual([]); + }); + it("should report unsupported and drift-prone route metadata as diagnostics", () => { @Controller("/hooks") class HooksController { @@ -188,6 +217,28 @@ describe("buildContractGraph", () => { ); }); + it("should warn when generated contracts unwrap nested Zod effects", () => { + @Controller("/profiles") + class ProfilesController { + @Post("/") + createProfile( + @Body(z.object({ name: z.string().transform((value) => value.trim()) })) + _body: { name: string }, + ): void {} + } + + const graph = buildContractGraph([ProfilesController]); + + expect(graph.diagnostics).toEqual([ + expect.objectContaining({ + code: "contract-schema-zod-effects-unwrapped", + severity: "warning", + routeId: "ProfilesController.createProfile", + }), + ]); + expect(() => assertContractGraphHasNoErrors(graph)).not.toThrow(); + }); + it("should reject routes with more than one request body parameter", () => { @Controller("/users") class UsersController { @@ -236,28 +287,25 @@ describe("buildContractGraph", () => { }); it("should reject duplicate controller names used as contract identity", () => { - let FirstController!: new () => unknown; - let SecondController!: new () => unknown; - - { + const FirstController = (() => { @Controller("/first") class DuplicateController { @Get("/one") one(): void {} } - FirstController = DuplicateController; - } + return DuplicateController; + })(); - { + const SecondController = (() => { @Controller("/second") class DuplicateController { @Get("/two") two(): void {} } - SecondController = DuplicateController; - } + return DuplicateController; + })(); const graph = buildContractGraph([FirstController, SecondController]); diff --git a/packages/rpc-codegen/src/libs/generate.ts b/packages/rpc-codegen/src/libs/generate.ts index 2914702ac..0ab52e6b8 100644 --- a/packages/rpc-codegen/src/libs/generate.ts +++ b/packages/rpc-codegen/src/libs/generate.ts @@ -497,13 +497,17 @@ function getObjectTypeScript(schema: unknown): string { } function formatObjectKey(key: string): string { - if (/^[A-Za-z_$][A-Za-z0-9_$]*$/.test(key)) { + if (isJavaScriptIdentifier(key)) { return key; } return `'${key.replace(/\\/g, "\\\\").replace(/'/g, "\\'")}'`; } +function isJavaScriptIdentifier(value: string): boolean { + return /^[A-Za-z_$][A-Za-z0-9_$]*$/.test(value); +} + function getObjectShape(schema: unknown): Record { if (!schema || typeof schema !== "object") { return {}; @@ -536,12 +540,18 @@ function getPathExpression(route: RouteIR): string { const pathExpression = route.path.replace(/:([^/]+)/g, (tokenWithPrefix, token: string) => { const name = paramsByToken.get(token); - return name ? `\${encodeURIComponent(String(input.path.${name}))}` : tokenWithPrefix; + return name ? `\${encodeURIComponent(String(${getPathInputAccessor(name)}))}` : tokenWithPrefix; }); return `\`${pathExpression}\``; } +function getPathInputAccessor(name: string): string { + return isJavaScriptIdentifier(name) + ? `input.path.${name}` + : `input.path[${formatObjectKey(name)}]`; +} + function getQueryStatements(route: RouteIR): string { if (!route.inputSchemas.query) { return ""; diff --git a/packages/rpc-codegen/src/tests/codegen.spec.ts b/packages/rpc-codegen/src/tests/codegen.spec.ts index 19af912cf..c2fbb58f1 100644 --- a/packages/rpc-codegen/src/tests/codegen.spec.ts +++ b/packages/rpc-codegen/src/tests/codegen.spec.ts @@ -470,7 +470,9 @@ describe("generateClientFiles", () => { inputSchema: null, inputSchemas: { body: null, - path: z.object({ id: z.string(), id2: z.string() }) as any, + path: z.object({ id: z.string(), id2: z.string() }) as unknown as NonNullable< + RouteIR["inputSchemas"]["path"] + >, query: null, headers: null, }, @@ -488,6 +490,36 @@ describe("generateClientFiles", () => { expect(content).not.toContain("${encodeURIComponent(String(input.path.id))}2"); }); + it("should bracket-access path parameters that are not JavaScript identifiers", () => { + const routes: RouteIR[] = [ + { + controllerName: "UserController", + methodName: "get", + httpMethod: "GET", + path: "/users/:user-id", + params: [{ kind: "path", name: "user-id", schema: null }], + inputSchema: null, + inputSchemas: { + body: null, + path: z.object({ "user-id": z.string() }) as unknown as NonNullable< + RouteIR["inputSchemas"]["path"] + >, + query: null, + headers: null, + }, + outputSchema: null, + domain: null, + }, + ]; + + const files = generateClientFiles(routes, TEMP_DIR); + + const content = fs.readFileSync(files[0], "utf-8"); + expect(content).toContain( + "const path = `/users/${encodeURIComponent(String(input.path['user-id']))}`;", + ); + }); + it("should normalize catch-all path parameters when generating fetch paths", () => { const routes: RouteIR[] = [ { diff --git a/packages/rpc-codegen/vitest.config.ts b/packages/rpc-codegen/vitest.config.ts new file mode 100644 index 000000000..c3293b801 --- /dev/null +++ b/packages/rpc-codegen/vitest.config.ts @@ -0,0 +1,17 @@ +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { defineConfig } from "vitest/config"; + +const currentDir = dirname(fileURLToPath(import.meta.url)); + +export default defineConfig({ + resolve: { + alias: { + "@croco/problems-core": resolve(currentDir, "../problems-core/src/index.ts"), + "@croco/protocols-core": resolve(currentDir, "../protocols-core/src/index.ts"), + }, + }, + test: { + include: ["src/**/*.test.ts", "src/**/*.spec.ts"], + }, +}); From 01c60a8be693adace8cb85c73b63df1863a49043 Mon Sep 17 00:00:00 2001 From: kang-heewon Date: Tue, 16 Jun 2026 15:43:29 +0900 Subject: [PATCH 13/13] fix: stabilize generated client typecheck tests --- .../rpc-codegen/src/tests/codegen.spec.ts | 115 ++++++++++-------- 1 file changed, 62 insertions(+), 53 deletions(-) diff --git a/packages/rpc-codegen/src/tests/codegen.spec.ts b/packages/rpc-codegen/src/tests/codegen.spec.ts index c2fbb58f1..fe5a23607 100644 --- a/packages/rpc-codegen/src/tests/codegen.spec.ts +++ b/packages/rpc-codegen/src/tests/codegen.spec.ts @@ -7,6 +7,7 @@ import { z } from "zod"; import { generateClientFiles } from "../libs/generate"; const TEMP_DIR = path.join(__dirname, "codegen-temp"); +const GENERATED_CLIENT_TYPECHECK_TIMEOUT_MS = 15_000; const EMPTY_INPUT_SCHEMAS = { body: null, path: null, query: null, headers: null }; const BODY_INPUT_SCHEMAS = { body: {} as RouteIR["inputSchemas"]["body"], @@ -660,72 +661,80 @@ describe("generateClientFiles", () => { ); }); - it("should typecheck generated clients with non-string query inputs", () => { - const routes: RouteIR[] = [ - { - controllerName: "UserController", - methodName: "list", - httpMethod: "GET", - path: "/users", - params: [ - { kind: "query", name: "page", schema: null }, - { kind: "query", name: "active", schema: null }, - { kind: "query", name: "search", schema: null }, - { kind: "query", name: "tags", schema: null }, - { kind: "query", name: "deletedAt", schema: null }, - ], - inputSchema: null, - inputSchemas: NON_STRING_QUERY_INPUT_SCHEMAS, - outputSchema: null, - domain: null, - }, - ]; - - const files = generateClientFiles(routes, TEMP_DIR); - - const content = fs.readFileSync(files[0], "utf-8"); - expect(content).toContain( - "export type ListInput = { query: { page: number; active: boolean | undefined; search: string | undefined; tags: string[]; deletedAt: string | null; }; };", - ); - expect(content).toContain( - "function readOptionalJsonResponse(response: Response): Promise", - ); - assertGeneratedClientTypechecks(`${content} + it( + "should typecheck generated clients with non-string query inputs", + () => { + const routes: RouteIR[] = [ + { + controllerName: "UserController", + methodName: "list", + httpMethod: "GET", + path: "/users", + params: [ + { kind: "query", name: "page", schema: null }, + { kind: "query", name: "active", schema: null }, + { kind: "query", name: "search", schema: null }, + { kind: "query", name: "tags", schema: null }, + { kind: "query", name: "deletedAt", schema: null }, + ], + inputSchema: null, + inputSchemas: NON_STRING_QUERY_INPUT_SCHEMAS, + outputSchema: null, + domain: null, + }, + ]; + + const files = generateClientFiles(routes, TEMP_DIR); + + const content = fs.readFileSync(files[0], "utf-8"); + expect(content).toContain( + "export type ListInput = { query: { page: number; active: boolean | undefined; search: string | undefined; tags: string[]; deletedAt: string | null; }; };", + ); + expect(content).toContain( + "function readOptionalJsonResponse(response: Response): Promise", + ); + assertGeneratedClientTypechecks(`${content} const result: Promise = userClient.list({ query: { page: 2, active: false, search: undefined, tags: ['new', 'vip'], deletedAt: null }, }); void result; `); - }); + }, + GENERATED_CLIENT_TYPECHECK_TIMEOUT_MS, + ); - it("should typecheck generated clients with header inputs", () => { - const routes: RouteIR[] = [ - { - controllerName: "UserController", - methodName: "get", - httpMethod: "GET", - path: "/users", - params: [ - { kind: "header", name: "authorization", schema: null }, - { kind: "header", name: "x-tenant-id", schema: null }, - ], - inputSchema: null, - inputSchemas: HEADER_INPUT_SCHEMAS, - outputSchema: null, - domain: null, - }, - ]; + it( + "should typecheck generated clients with header inputs", + () => { + const routes: RouteIR[] = [ + { + controllerName: "UserController", + methodName: "get", + httpMethod: "GET", + path: "/users", + params: [ + { kind: "header", name: "authorization", schema: null }, + { kind: "header", name: "x-tenant-id", schema: null }, + ], + inputSchema: null, + inputSchemas: HEADER_INPUT_SCHEMAS, + outputSchema: null, + domain: null, + }, + ]; - const files = generateClientFiles(routes, TEMP_DIR); + const files = generateClientFiles(routes, TEMP_DIR); - const content = fs.readFileSync(files[0], "utf-8"); - assertGeneratedClientTypechecks(`${content} + const content = fs.readFileSync(files[0], "utf-8"); + assertGeneratedClientTypechecks(`${content} const result: Promise = userClient.get({ headers: { authorization: 'Bearer token', 'x-tenant-id': undefined }, }); void result; `); - }); + }, + GENERATED_CLIENT_TYPECHECK_TIMEOUT_MS, + ); it("should serialize body, path, and query input when generating combined fetch calls", () => { const routes: RouteIR[] = [