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) \//); + } + }); +});