diff --git a/src/openapi/define-route.ts b/src/openapi/define-route.ts index b1e4e8e6d..c7f953ad0 100644 --- a/src/openapi/define-route.ts +++ b/src/openapi/define-route.ts @@ -177,6 +177,14 @@ export function registerRouteSpec(registry: OpenAPIRegistry, options: RouteSpecO } const security = securityFor(options.auth); + // ONE `request` key, built once (#9705). This literal used to carry two: path parameters in the first, + // body and query in a conditional spread further down. A property arriving through a later spread + // replaces an earlier one of the same name -- and TypeScript does not flag it, because the duplicate is + // a spread rather than a literal duplicate key -- so any route with BOTH a templated segment and a body + // or query published no `parameters` at all. That is precisely what pathParameters() exists to prevent: + // a templated segment with no matching parameter is a schema-validation warning and leaves a generated + // client holding a URL it cannot fill. Latent only because no caller passed `request` yet; the first + // migrated POST /v1/repos/:owner/:repo/... would have lost owner and repo silently. registry.registerPath({ method: options.method, path: toSpecPath(options.path), @@ -191,14 +199,6 @@ export function registerRouteSpec(registry: OpenAPIRegistry, options: RouteSpecO summary: options.summary, ...(options.description ? { description: options.description } : {}), ...(security ? { security } : {}), - ...(options.request?.body || options.request?.query - ? { - request: { - ...(options.request.body ? { body: { content: { "application/json": { schema: options.request.body } } } } : {}), - ...(options.request.query ? { query: options.request.query } : {}), - }, - } - : {}), responses, }); } diff --git a/test/unit/define-route.test.ts b/test/unit/define-route.test.ts index 018a0d197..30f6c7851 100644 --- a/test/unit/define-route.test.ts +++ b/test/unit/define-route.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it } from "vitest"; import { Hono } from "hono"; import { OpenAPIRegistry, OpenApiGeneratorV3 } from "@asteasolutions/zod-to-openapi"; import { z } from "zod"; -import { defineRoute, invalidRequestBody, registerRouteSpec } from "../../src/openapi/define-route"; +import { defineRoute, invalidRequestBody, registerRouteSpec, type RouteSpecOptions } from "../../src/openapi/define-route"; type TestEnv = { Bindings: Record; Variables: Record }; @@ -172,6 +172,22 @@ describe("defineRoute", () => { } }); + it("gives the ORB ingress its OWN schemes rather than the LoopOver pair", () => { + // The two auth levels that are not a LoopOver credential at all: an ORB-issued instance token, and a + // webhook that carries no bearer and is verified by a body signature. Publishing the generic pair for + // either would tell a client to send something the route does not accept. + const { app, registry } = build(); + for (const [auth, path, id] of [ + ["orb", "/v1/orb/relay", "orbOp"], + ["webhook", "/v1/orb/webhook", "webhookOp"], + ] as const) { + defineRoute(app, registry, { method: "post", path, operationId: id, tags: ["t"], summary: "s", auth, responses: { 200: { description: "ok" } } }, (c) => c.json({})); + } + const paths = generate(registry).paths ?? {}; + expect(paths["/v1/orb/relay"]?.post?.security).toEqual([{ OrbBearer: [] }]); + expect(paths["/v1/orb/webhook"]?.post?.security).toEqual([{ OrbWebhookSignature: [] }]); + }); + it("emits responses with and without a schema", () => { const { app, registry } = build(); defineRoute(app, registry, { @@ -206,6 +222,58 @@ describe("registerRouteSpec", () => { const operation = generate(registry).paths?.["/v1/spec-only/{id}"]?.post; expect(operation?.operationId).toBe("specOnly"); expect(operation?.requestBody).toBeDefined(); + // #9705: and its path parameter, which the duplicate `request` key used to erase. This route declares + // both a templated segment and a body, so before the fix `parameters` was absent entirely. + expect(operation?.parameters).toContainEqual(expect.objectContaining({ in: "path", name: "id", required: true })); + }); + + // #9705 regression suite. The defect was that a route carrying a templated path AND any request + // declaration published no `parameters` array -- silently, because the duplicate key arrived via a + // spread and TypeScript does not flag that. Each case below fails on the pre-fix emitter. + function specOnlyOperation(request: RouteSpecOptions["request"]) { + const registry = new OpenAPIRegistry(); + registerRouteSpec(registry, { + method: "post", + path: "/v1/repos/:owner/:repo/thing", + operationId: "repoThing", + tags: ["ops"], + summary: "Repo thing", + auth: "token", + ...(request ? { request } : {}), + responses: { 202: { description: "accepted" } }, + }); + return generate(registry).paths?.["/v1/repos/{owner}/{repo}/thing"]?.post; + } + + function pathParameterNames(operation: ReturnType): string[] { + return (operation?.parameters ?? []).filter((parameter) => "in" in parameter && parameter.in === "path").map((parameter) => ("name" in parameter ? parameter.name : "")); + } + + it("keeps every path parameter alongside a request BODY", () => { + const operation = specOnlyOperation({ body: z.object({ a: z.string() }) }); + expect(pathParameterNames(operation)).toEqual(["owner", "repo"]); + expect(operation?.requestBody).toBeDefined(); + }); + + it("keeps every path parameter alongside a QUERY schema", () => { + const operation = specOnlyOperation({ query: z.object({ since: z.string() }) }); + expect(pathParameterNames(operation)).toEqual(["owner", "repo"]); + expect(operation?.parameters).toContainEqual(expect.objectContaining({ in: "query", name: "since" })); + }); + + it("keeps path parameters, the body, and the query together in ONE operation", () => { + const operation = specOnlyOperation({ body: z.object({ a: z.string() }), query: z.object({ since: z.string() }) }); + expect(pathParameterNames(operation)).toEqual(["owner", "repo"]); + expect(operation?.parameters).toContainEqual(expect.objectContaining({ in: "query", name: "since" })); + expect(operation?.requestBody).toBeDefined(); + }); + + it("still emits path parameters for a route that declares no request at all", () => { + // The unchanged arm: every production entry today is response-only, and their operations must be + // byte-for-byte what they were. + const operation = specOnlyOperation(undefined); + expect(pathParameterNames(operation)).toEqual(["owner", "repo"]); + expect(operation?.requestBody).toBeUndefined(); }); });