From fc8b1dd60be0fdfad79cabd7c674783c8affd017 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:21:37 -0700 Subject: [PATCH] feat(api): add the route-registration seam and a route-to-spec ratchet createApp() registers 241 operations; buildOpenApiSpec() describes 151. Nothing compared the two, so 90 live routes had no spec entry at all -- including every ORB management surface (/v1/orb/*, /v1/internal/orb/*, fleet config-push, kill-switch, the DLQ admin quartet). ui:openapi:check only verifies the committed file matches its own generator, and test/unit/openapi.test.ts is a hand-written allowlist of paths that must exist; neither can see a route that exists in the app and nowhere in the document. route-inventory.ts reads Hono's own routes array and diffs it against the generated document in both directions. The ratchet test holds the 90 known gaps in a committed baseline that may only SHRINK: a new unspecced route fails immediately rather than joining the pile, and an entry that has since been specced must be removed, so the file keeps describing the real remaining work. Verified by temporarily adding a route and confirming the gate fails. The reverse direction -- an operation with no live route, which would make a generated client compile a call that 404s -- is zero today and has no baseline at all. define-route.ts registers a route AND its operation from one definition, with the same zod schemas validating at runtime. The handler receives already-parsed body and query, so a migrated route cannot forget to validate. operationId and tags are mandatory: the document emits tags: [] everywhere today, which collapses every operation into one namespace for generated clients, and slugified operation ids make every path edit a breaking change for consumers. Security stanzas derive from the declared auth level, which is what will let isProtectedPath() -- a second, already-disagreeing model of the same policy -- be deleted in #9531. A local shim rather than @hono/zod-openapi, per the decision recorded on #9519: it coexists with plain app.get() registrations so 241 routes can migrate in batches. Closes #9519 --- src/openapi/define-route.ts | 156 +++++++++++++++ src/openapi/route-inventory.ts | 81 ++++++++ src/openapi/unspecced-routes-baseline.json | 92 +++++++++ test/unit/define-route.test.ts | 214 +++++++++++++++++++++ test/unit/route-spec-ratchet.test.ts | 102 ++++++++++ 5 files changed, 645 insertions(+) create mode 100644 src/openapi/define-route.ts create mode 100644 src/openapi/route-inventory.ts create mode 100644 src/openapi/unspecced-routes-baseline.json create mode 100644 test/unit/define-route.test.ts create mode 100644 test/unit/route-spec-ratchet.test.ts diff --git a/src/openapi/define-route.ts b/src/openapi/define-route.ts new file mode 100644 index 0000000000..06464eab08 --- /dev/null +++ b/src/openapi/define-route.ts @@ -0,0 +1,156 @@ +// The route-registration seam (#9519). +// +// One call registers a route on the Hono app AND contributes its OpenAPI operation, from the same +// zod schemas that validate the request at runtime. That single-source property is the whole point: +// today the app's route table (createApp, 241 operations) and the spec's path list +// (buildOpenApiSpec, 151) are separate hand-maintained lists that had drifted apart by 90 routes, +// and request bodies are validated by inline schemas the published document never mentions. +// +// Deliberately a local shim rather than @hono/zod-openapi (decision recorded on #9519): 241 routes +// have to migrate incrementally, and this coexists with plain `app.get(...)` registrations, whereas +// adopting that library means rewriting createApp() wholesale and re-coupling the zod version. The +// emit path reuses the @asteasolutions/zod-to-openapi registry the repo already builds its spec +// with, so nothing about the generated document's shape changes. +import type { Context, Hono, MiddlewareHandler } from "hono"; +import type { OpenAPIRegistry, RouteConfig } from "@asteasolutions/zod-to-openapi"; +import { z } from "zod"; + +/** + * Who a caller must be. These are the identity kinds `src/auth/security.ts` actually + * authenticates, so the security stanza emitted into the document is derived from the same + * declaration the runtime gate enforces -- replacing `isProtectedPath()`, a second, path-prefix + * model of the same policy that had already drifted out of agreement with it. + */ +export type RouteAuth = "public" | "token" | "session" | "internal"; + +export type RouteMethod = "get" | "post" | "put" | "patch" | "delete"; + +export type DefineRouteOptions = { + method: RouteMethod; + /** Hono-style path (`/v1/repos/:owner/:repo`). Normalized to OpenAPI form for the document. */ + path: string; + /** Stable, hand-chosen operation id. Required: a generated client's method names come from these, + * so leaving them to be slugified from method+path makes every path edit a breaking API change + * for consumers. */ + operationId: string; + /** At least one tag. The document currently emits `tags: []` everywhere, which collapses every + * operation into one flat namespace in generated clients and doc explorers alike. */ + tags: [string, ...string[]]; + summary: string; + description?: string; + auth: RouteAuth; + request?: { + body?: Body; + query?: Query; + }; + responses: Record; +}; + +/** Hono writes `:param`; OpenAPI writes `{param}`. */ +function toSpecPath(path: string): string { + return path.replace(/:([A-Za-z0-9_]+)/g, "{$1}"); +} + +/** `public` routes carry no security stanza; everything else accepts either credential the API + * actually supports. Internal routes are bearer-only -- there is no cookie path to them. */ +function securityFor(auth: RouteAuth): RouteConfig["security"] { + if (auth === "public") return undefined; + if (auth === "internal") return [{ LoopOverBearer: [] }]; + return [{ LoopOverBearer: [] }, { LoopOverSessionCookie: [] }]; +} + +/** + * The validation error shape. + * + * Matches what the hand-written `.safeParse` call sites in src/api/routes.ts already return + * (`{ error: "invalid__request", issues }`), so migrating a route through this seam does not + * change the body a client sees on a 400. + */ +export function invalidRequestBody(kind: string, error: z.ZodError): { error: string; issues: unknown } { + return { error: `invalid_${kind}_request`, issues: error.issues }; +} + +export type RouteHandlerArgs = { + body: Body; + query: Query; +}; + +/** + * Register a route on the app and in the OpenAPI registry from one definition. + * + * The handler receives already-validated `body`/`query` rather than a raw context accessor, so a + * migrated handler cannot forget to parse -- the parse is the only way to reach the handler at all. + */ +export function defineRoute< + AppEnv extends { Bindings: Record; Variables: Record }, + Body extends z.ZodTypeAny | undefined = undefined, + Query extends z.ZodObject | undefined = undefined, +>( + app: Hono, + registry: OpenAPIRegistry, + options: DefineRouteOptions, + handler: ( + c: Context, + args: RouteHandlerArgs : undefined, Query extends z.ZodObject ? z.infer : undefined>, + ) => Response | Promise, +): void { + registerRouteSpec(registry, options); + + const wrapped: MiddlewareHandler = async (c) => { + let body: unknown; + if (options.request?.body) { + // A body-carrying route with an unparseable/absent JSON body is a client error, not a 500 -- + // matching how the existing hand-written call sites treat `await c.req.json()` failures. + const raw = await c.req.json().catch(() => undefined); + const parsed = (options.request.body as z.ZodTypeAny).safeParse(raw); + if (!parsed.success) return c.json(invalidRequestBody(options.operationId, parsed.error), 400); + body = parsed.data; + } + let query: unknown; + if (options.request?.query) { + const parsed = (options.request.query as z.ZodTypeAny).safeParse(c.req.query()); + if (!parsed.success) return c.json(invalidRequestBody(options.operationId, parsed.error), 400); + query = parsed.data; + } + return handler(c as Context, { body, query } as never); + }; + + app[options.method](options.path, wrapped); +} + +/** Spec-side view of a route definition: the same fields, with the request schemas widened, since + * emitting the document never needs the parsed types the handler does. */ +export type RouteSpecOptions = Omit, "request"> & { + request?: { body?: z.ZodTypeAny | undefined; query?: z.ZodObject | undefined } | undefined; +}; + +/** The spec half, exported separately so a route that cannot yet move its handler through the seam + * can still contribute a correct operation and leave the ratchet baseline. */ +export function registerRouteSpec(registry: OpenAPIRegistry, options: RouteSpecOptions): void { + const responses: RouteConfig["responses"] = {}; + for (const [status, response] of Object.entries(options.responses)) { + responses[Number(status)] = response.schema + ? { description: response.description, content: { "application/json": { schema: response.schema } } } + : { description: response.description }; + } + + const security = securityFor(options.auth); + registry.registerPath({ + method: options.method, + path: toSpecPath(options.path), + operationId: options.operationId, + tags: options.tags, + 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/src/openapi/route-inventory.ts b/src/openapi/route-inventory.ts new file mode 100644 index 0000000000..34dbb0d75f --- /dev/null +++ b/src/openapi/route-inventory.ts @@ -0,0 +1,81 @@ +// Route↔spec inventory (#9519). +// +// The Worker's route table and its OpenAPI spec are two independently hand-maintained lists: +// createApp() registers the routes Hono actually serves, buildOpenApiSpec() registers the paths the +// published document describes, and until now nothing compared them. The measured gap when this +// landed was 91 live routes with no spec entry -- including every ORB management surface -- and no +// check could detect it, because ui:openapi:check only compares the generated file to its own +// generator, and test/unit/openapi.test.ts is a hand-written allowlist that never enumerates the +// app. +// +// This module is the comparison. It is deliberately pure (no fs, no env) so both the ratchet test +// and any future tooling can use it, and so it stays cheap enough to run on every CI job. +import type { Hono } from "hono"; + +/** A route as either side describes it, normalized to one comparable string. */ +export type RouteKey = string; + +/** Methods an OpenAPI path item can carry that this repo actually uses. */ +const SPEC_METHODS = ["get", "post", "put", "patch", "delete"] as const; + +/** + * Hono writes path params as `:owner`; OpenAPI writes them as `{owner}`. Normalizing to the + * OpenAPI form (rather than the reverse) keeps the baseline file readable as spec paths. + * + * A trailing wildcard (`/v1/foo/*`) has no OpenAPI equivalent at all -- it is a middleware mount or + * a catch-all, not an operation -- so callers filter those out rather than trying to name them. + */ +export function normalizeRoutePath(path: string): string { + return path.replace(/:([A-Za-z0-9_]+)/g, "{$1}"); +} + +/** True for a Hono entry that is not a documentable operation: middleware registered with `use`, + * `app.all(...)` mounts, and wildcard catch-alls. */ +export function isNonOperationRoute(method: string, path: string): boolean { + return method === "ALL" || path.includes("*"); +} + +/** + * Every operation the app actually serves, as `METHOD /normalized/path`. + * + * Reads Hono's own `routes` array rather than a hand-kept list -- that is the entire point: a route + * added with `app.get(...)` and forgotten everywhere else still shows up here. + */ +export function listLiveRouteKeys(app: Hono): RouteKey[] { + const entries = (app as unknown as { routes: Array<{ method: string; path: string }> }).routes; + const keys = new Set(); + for (const entry of entries) { + if (isNonOperationRoute(entry.method, entry.path)) continue; + keys.add(`${entry.method.toUpperCase()} ${normalizeRoutePath(entry.path)}`); + } + return [...keys].sort(); +} + +/** Every operation the generated OpenAPI document describes, in the same key format. */ +export function listSpecRouteKeys(document: { paths?: Record }): RouteKey[] { + const keys = new Set(); + for (const [path, item] of Object.entries(document.paths ?? {})) { + if (!item || typeof item !== "object") continue; + for (const method of SPEC_METHODS) { + if ((item as Record)[method]) keys.add(`${method.toUpperCase()} ${path}`); + } + } + return [...keys].sort(); +} + +export type RouteSpecDiff = { + /** Live routes with no operation in the spec -- the gap the ratchet drives to zero. */ + missingFromSpec: RouteKey[]; + /** Spec operations with no live route. These are worse than a gap: the published document + * promises an endpoint that does not exist, so a generated client compiles a call that 404s. */ + missingFromApp: RouteKey[]; +}; + +export function diffRoutesAgainstSpec(liveKeys: readonly RouteKey[], specKeys: readonly RouteKey[]): RouteSpecDiff { + const live = new Set(liveKeys); + const spec = new Set(specKeys); + return { + missingFromSpec: liveKeys.filter((key) => !spec.has(key)), + missingFromApp: specKeys.filter((key) => !live.has(key)), + }; +} diff --git a/src/openapi/unspecced-routes-baseline.json b/src/openapi/unspecced-routes-baseline.json new file mode 100644 index 0000000000..a5eef45230 --- /dev/null +++ b/src/openapi/unspecced-routes-baseline.json @@ -0,0 +1,92 @@ +[ + "DELETE /v1/internal/repos/{owner}/{repo}/ai-key", + "DELETE /v1/internal/repos/{owner}/{repo}/linear-key", + "DELETE /v1/repos/{owner}/{repo}/ai-key", + "DELETE /v1/repos/{owner}/{repo}/linear-key", + "GET /loopover/shot", + "GET /openapi.json", + "GET /v1/app/installations", + "GET /v1/app/installations/{id}/health", + "GET /v1/app/installations/{id}/repair", + "GET /v1/app/kill-switch", + "GET /v1/drafts/auth/callback", + "GET /v1/drafts/{id}", + "GET /v1/internal/audit-labels", + "GET /v1/internal/calibration", + "GET /v1/internal/calibration-trend", + "GET /v1/internal/calibration/knobs", + "GET /v1/internal/calibration/satisfaction-floor", + "GET /v1/internal/decision", + "GET /v1/internal/fairness/contributors", + "GET /v1/internal/fairness/contributors/{login}", + "GET /v1/internal/fleet/analytics", + "GET /v1/internal/ops/stats", + "GET /v1/internal/orb/installations", + "GET /v1/internal/orb/instances", + "GET /v1/internal/parity", + "GET /v1/internal/predicted-agreement", + "GET /v1/internal/repos/{owner}/{repo}/ai-key", + "GET /v1/internal/repos/{owner}/{repo}/contribution-policy", + "GET /v1/internal/repos/{owner}/{repo}/linear-key", + "GET /v1/internal/retention/preview", + "GET /v1/internal/status", + "GET /v1/mcp/enrichment-analyzers", + "GET /v1/mcp/finding-taxonomy", + "GET /v1/orb/oauth/callback", + "GET /v1/public/repos/{owner}/{repo}/badge.json", + "GET /v1/public/repos/{owner}/{repo}/badge.svg", + "GET /v1/public/subnet-interface", + "GET /v1/repos/{owner}/{repo}/ai-key", + "GET /v1/repos/{owner}/{repo}/linear-key", + "POST /v1/ams/ingest", + "POST /v1/app/fleet/config-push", + "POST /v1/app/installations/{id}/repair/refresh", + "POST /v1/app/kill-switch", + "POST /v1/app/miner-dashboard/refresh", + "POST /v1/contributors/{login}/ams-notifications", + "POST /v1/drafts", + "POST /v1/internal/audit-labels/adjudicate", + "POST /v1/internal/calibration/loosen-satisfaction-floor", + "POST /v1/internal/jobs/backfill-contributor-gate-history/run", + "POST /v1/internal/jobs/backfill-pr-details/run", + "POST /v1/internal/jobs/backfill-registered-repos/run", + "POST /v1/internal/jobs/backfill-repo-segment/run", + "POST /v1/internal/jobs/build-contributor-decision-packs/run", + "POST /v1/internal/jobs/file-upstream-drift-issues/run", + "POST /v1/internal/jobs/generate-review-recap/run", + "POST /v1/internal/jobs/generate-signal-snapshots/run", + "POST /v1/internal/jobs/generate-weekly-value-report/run", + "POST /v1/internal/jobs/rag-index", + "POST /v1/internal/jobs/refresh-contributor-activity", + "POST /v1/internal/jobs/refresh-contributor-activity/run", + "POST /v1/internal/jobs/refresh-installation-health/run", + "POST /v1/internal/jobs/refresh-registry/run", + "POST /v1/internal/jobs/refresh-scoring-model/run", + "POST /v1/internal/jobs/refresh-upstream-drift/run", + "POST /v1/internal/jobs/regate-pr", + "POST /v1/internal/jobs/rollup-product-usage", + "POST /v1/internal/jobs/rollup-product-usage/run", + "POST /v1/internal/orb/enrollments", + "POST /v1/internal/orb/enrollments/{enrollId}/revoke", + "POST /v1/internal/orb/installations/backfill", + "POST /v1/internal/orb/installations/register", + "POST /v1/internal/orb/instances/register", + "POST /v1/internal/queue-intelligence", + "POST /v1/internal/repos/{owner}/{repo}/ai-key", + "POST /v1/internal/repos/{owner}/{repo}/contribution-policy", + "POST /v1/internal/repos/{owner}/{repo}/linear-key", + "POST /v1/internal/repos/{owner}/{repo}/settings", + "POST /v1/local/remediation-plan", + "POST /v1/loop/request-apr-transfer", + "POST /v1/orb/relay", + "POST /v1/orb/relay/pull", + "POST /v1/orb/relay/register", + "POST /v1/orb/token", + "POST /v1/orb/webhook", + "POST /v1/repos/{owner}/{repo}/ai-key", + "POST /v1/repos/{owner}/{repo}/linear-key", + "POST /v1/repos/{owner}/{repo}/pulls/{number}/chat-qa", + "PUT /v1/app/installations/{id}/agent/bulk-settings", + "PUT /v1/repos/{owner}/{repo}/ai-review", + "PUT /v1/repos/{owner}/{repo}/settings" +] diff --git a/test/unit/define-route.test.ts b/test/unit/define-route.test.ts new file mode 100644 index 0000000000..db8ccdb182 --- /dev/null +++ b/test/unit/define-route.test.ts @@ -0,0 +1,214 @@ +// Tests for the route-registration seam (#9519). +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"; + +type TestEnv = { Bindings: Record; Variables: Record }; + +function build() { + return { app: new Hono(), registry: new OpenAPIRegistry() }; +} + +function generate(registry: OpenAPIRegistry) { + return new OpenApiGeneratorV3(registry.definitions).generateDocument({ + openapi: "3.0.3", + info: { title: "t", version: "1" }, + }); +} + +describe("defineRoute", () => { + it("registers the route on the app and serves the handler", async () => { + const { app, registry } = build(); + defineRoute(app, registry, { + method: "get", + path: "/v1/thing", + operationId: "getThing", + tags: ["things"], + summary: "Get a thing", + auth: "public", + responses: { 200: { description: "ok", schema: z.object({ ok: z.boolean() }) } }, + }, (c) => c.json({ ok: true })); + + const response = await app.request("/v1/thing"); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ ok: true }); + }); + + it("emits an operation with the operationId, tags, and summary", () => { + const { app, registry } = build(); + defineRoute(app, registry, { + method: "get", + path: "/v1/thing", + operationId: "getThing", + tags: ["things"], + summary: "Get a thing", + description: "Longer prose", + auth: "public", + responses: { 200: { description: "ok" } }, + }, (c) => c.json({})); + + const operation = generate(registry).paths?.["/v1/thing"]?.get; + expect(operation?.operationId).toBe("getThing"); + expect(operation?.tags).toEqual(["things"]); + expect(operation?.summary).toBe("Get a thing"); + expect(operation?.description).toBe("Longer prose"); + }); + + it("rewrites Hono path params into OpenAPI form for the document while serving the Hono path", async () => { + const { app, registry } = build(); + defineRoute(app, registry, { + method: "get", + path: "/v1/repos/:owner/:repo", + operationId: "getRepo", + tags: ["repos"], + summary: "Get repo", + auth: "token", + responses: { 200: { description: "ok" } }, + }, (c) => c.json({ owner: c.req.param("owner") })); + + expect(generate(registry).paths?.["/v1/repos/{owner}/{repo}"]).toBeDefined(); + expect(await (await app.request("/v1/repos/a/b")).json()).toEqual({ owner: "a" }); + }); + + it("validates the request body and hands the handler parsed data", async () => { + const { app, registry } = build(); + defineRoute(app, registry, { + method: "post", + path: "/v1/thing", + operationId: "createThing", + tags: ["things"], + summary: "Create", + auth: "token", + request: { body: z.object({ name: z.string().min(1) }) }, + responses: { 200: { description: "ok" } }, + }, (c, { body }) => c.json({ name: body.name })); + + const ok = await app.request("/v1/thing", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "x" }), + }); + expect(await ok.json()).toEqual({ name: "x" }); + }); + + it("rejects an invalid body with a 400 in the repo's existing error shape", async () => { + const { app, registry } = build(); + defineRoute(app, registry, { + method: "post", + path: "/v1/thing", + operationId: "createThing", + tags: ["things"], + summary: "Create", + auth: "token", + request: { body: z.object({ name: z.string().min(1) }) }, + responses: { 200: { description: "ok" } }, + }, (c) => c.json({ unreachable: true })); + + const bad = await app.request("/v1/thing", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "" }), + }); + expect(bad.status).toBe(400); + expect(await bad.json()).toMatchObject({ error: "invalid_createThing_request" }); + }); + + it("treats an unparseable body as a 400 rather than letting it throw a 500", async () => { + const { app, registry } = build(); + defineRoute(app, registry, { + method: "post", + path: "/v1/thing", + operationId: "createThing", + tags: ["things"], + summary: "Create", + auth: "token", + request: { body: z.object({ name: z.string() }) }, + responses: { 200: { description: "ok" } }, + }, (c) => c.json({ unreachable: true })); + + const bad = await app.request("/v1/thing", { method: "POST", headers: { "content-type": "application/json" }, body: "not json" }); + expect(bad.status).toBe(400); + }); + + it("validates query params and rejects an invalid one", async () => { + const { app, registry } = build(); + defineRoute(app, registry, { + method: "get", + path: "/v1/thing", + operationId: "listThings", + tags: ["things"], + summary: "List", + auth: "token", + request: { query: z.object({ limit: z.coerce.number().int().positive() }) }, + responses: { 200: { description: "ok" } }, + }, (c, { query }) => c.json({ limit: query.limit })); + + expect(await (await app.request("/v1/thing?limit=5")).json()).toEqual({ limit: 5 }); + expect((await app.request("/v1/thing?limit=nope")).status).toBe(400); + }); + + it("omits a security stanza for public routes and requires credentials otherwise", () => { + const { app, registry } = build(); + for (const [auth, path, id] of [ + ["public", "/v1/open", "openOp"], + ["token", "/v1/tokened", "tokenOp"], + ["session", "/v1/sessioned", "sessionOp"], + ["internal", "/v1/internal/thing", "internalOp"], + ] as const) { + defineRoute(app, registry, { method: "get", path, operationId: id, tags: ["t"], summary: "s", auth, responses: { 200: { description: "ok" } } }, (c) => c.json({})); + } + const paths = generate(registry).paths ?? {}; + expect(paths["/v1/open"]?.get?.security).toBeUndefined(); + // Internal routes are bearer-only: no browser session ever reaches them. + expect(paths["/v1/internal/thing"]?.get?.security).toEqual([{ LoopOverBearer: [] }]); + for (const path of ["/v1/tokened", "/v1/sessioned"]) { + expect(paths[path]?.get?.security).toEqual([{ LoopOverBearer: [] }, { LoopOverSessionCookie: [] }]); + } + }); + + it("emits responses with and without a schema", () => { + const { app, registry } = build(); + defineRoute(app, registry, { + method: "get", + path: "/v1/thing", + operationId: "getThing", + tags: ["things"], + summary: "Get", + auth: "public", + responses: { 200: { description: "ok", schema: z.object({ a: z.string() }) }, 404: { description: "missing" } }, + }, (c) => c.json({})); + + const responses = generate(registry).paths?.["/v1/thing"]?.get?.responses ?? {}; + expect(responses["200"]).toHaveProperty("content"); + expect(responses["404"]).not.toHaveProperty("content"); + }); +}); + +describe("registerRouteSpec", () => { + it("contributes an operation without registering any route", () => { + const registry = new OpenAPIRegistry(); + registerRouteSpec(registry, { + method: "post", + path: "/v1/spec-only/:id", + operationId: "specOnly", + tags: ["ops"], + summary: "Spec only", + auth: "internal", + request: { body: z.object({ a: z.string() }) }, + responses: { 202: { description: "accepted" } }, + }); + const operation = generate(registry).paths?.["/v1/spec-only/{id}"]?.post; + expect(operation?.operationId).toBe("specOnly"); + expect(operation?.requestBody).toBeDefined(); + }); +}); + +describe("invalidRequestBody", () => { + it("names the error after the operation and carries the zod issues", () => { + const error = z.object({ a: z.string() }).safeParse({}).error!; + expect(invalidRequestBody("createThing", error)).toMatchObject({ error: "invalid_createThing_request" }); + expect(Array.isArray(invalidRequestBody("createThing", error).issues)).toBe(true); + }); +}); diff --git a/test/unit/route-spec-ratchet.test.ts b/test/unit/route-spec-ratchet.test.ts new file mode 100644 index 0000000000..f80302b0ef --- /dev/null +++ b/test/unit/route-spec-ratchet.test.ts @@ -0,0 +1,102 @@ +// The route↔spec ratchet (#9519). +// +// createApp() and buildOpenApiSpec() are two independently hand-maintained lists, and nothing +// compared them until now: `ui:openapi:check` only verifies the committed file matches its own +// generator, and test/unit/openapi.test.ts is a hand-written allowlist of paths that must exist -- +// neither can notice a route that exists in the app and nowhere in the document. When this landed, +// 90 live routes had no spec entry, including every ORB management surface (/v1/orb/*, +// /v1/internal/orb/*, fleet config-push, kill-switch, the DLQ admin quartet). +// +// The baseline is a RATCHET, not an allowlist: it may only shrink. A route removed from it can +// never silently come back, and a NEW unspecced route fails immediately rather than joining a +// growing pile. #9531 drives it to zero, at which point the file is deleted and this test keeps +// enforcing the invariant with an empty set. +import { describe, expect, it } from "vitest"; +import { createApp } from "../../src/api/routes"; +import { buildOpenApiSpec } from "../../src/openapi/spec"; +import { + diffRoutesAgainstSpec, + isNonOperationRoute, + listLiveRouteKeys, + listSpecRouteKeys, + normalizeRoutePath, +} from "../../src/openapi/route-inventory"; +import baseline from "../../src/openapi/unspecced-routes-baseline.json" with { type: "json" }; + +const KNOWN_UNSPECCED = new Set(baseline as string[]); + +describe("route inventory helpers", () => { + it("rewrites Hono path params into OpenAPI form", () => { + expect(normalizeRoutePath("/v1/repos/:owner/:repo/pulls/:number")).toBe("/v1/repos/{owner}/{repo}/pulls/{number}"); + expect(normalizeRoutePath("/health")).toBe("/health"); + }); + + it("skips middleware mounts and wildcard catch-alls, which have no OpenAPI operation", () => { + expect(isNonOperationRoute("ALL", "/mcp")).toBe(true); + expect(isNonOperationRoute("GET", "/v1/internal/*")).toBe(true); + expect(isNonOperationRoute("GET", "/health")).toBe(false); + }); + + it("reports both directions of the diff", () => { + const diff = diffRoutesAgainstSpec(["GET /a", "GET /b"], ["GET /b", "GET /c"]); + expect(diff.missingFromSpec).toEqual(["GET /a"]); + expect(diff.missingFromApp).toEqual(["GET /c"]); + }); + + it("reports nothing for identical inventories", () => { + const diff = diffRoutesAgainstSpec(["GET /a"], ["GET /a"]); + expect(diff.missingFromSpec).toEqual([]); + expect(diff.missingFromApp).toEqual([]); + }); + + it("collapses duplicate live registrations of the same method and path", () => { + // Hono records one entry per registered handler, so a route with middleware chained onto it + // appears more than once. Counting those separately would inflate the gap and make the + // baseline unstable against unrelated middleware edits. + const app = createApp(); + const raw = (app as unknown as { routes: Array<{ method: string; path: string }> }).routes; + expect(listLiveRouteKeys(app as never).length).toBeLessThan(raw.length); + }); +}); + +describe("route↔spec ratchet", () => { + const diff = diffRoutesAgainstSpec(listLiveRouteKeys(createApp() as never), listSpecRouteKeys(buildOpenApiSpec() as never)); + + it("describes every live route in the OpenAPI document, except the shrinking known-gap baseline", () => { + const newlyUnspecced = diff.missingFromSpec.filter((key) => !KNOWN_UNSPECCED.has(key)); + expect( + newlyUnspecced, + `These routes exist in createApp() but have no OpenAPI operation. Register them through the spec ` + + `(see src/openapi/spec.ts) rather than adding them to src/openapi/unspecced-routes-baseline.json -- ` + + `that file may only shrink (#9531).`, + ).toEqual([]); + }); + + it("keeps the baseline honest: an entry that is now specced must be removed from it", () => { + // Without this, the baseline would quietly retain entries for routes someone specced along the + // way, and the "may only shrink" promise would be unverifiable -- the file would stop + // describing the real remaining work. + const stale = [...KNOWN_UNSPECCED].filter((key) => !diff.missingFromSpec.includes(key)); + expect( + stale, + `These routes ARE now described in the OpenAPI document but are still listed in ` + + `src/openapi/unspecced-routes-baseline.json. Delete them from that file.`, + ).toEqual([]); + }); + + it("never publishes an operation for a route the app does not serve", () => { + // Strictly worse than a missing entry: a generated client compiles a call that 404s at runtime. + // No baseline for this one -- it is zero today and must stay zero. + expect( + diff.missingFromApp, + "These operations are in the OpenAPI document but no live route serves them.", + ).toEqual([]); + }); + + it("has a baseline that only contains real, currently-unspecced routes", () => { + expect(KNOWN_UNSPECCED.size).toBeGreaterThan(0); + for (const key of KNOWN_UNSPECCED) { + expect(key, `baseline entry is not a METHOD /path key`).toMatch(/^(GET|POST|PUT|PATCH|DELETE) \//); + } + }); +});