fix: describe the whole API in openapi.json, not the read half

.changeset/complete-openapi.md

@@ -0,0 +1,5 @@
+---
+'@flakemetry/contracts': patch
+---
+
+`/openapi.json` now describes the whole API rather than the read half. It was generated from the read-route table, so ingestion, the quality gate, artifact presigning and quarantine were invisible to anything generating a client — with nothing in the document to say a client was seeing a fraction of it. Request bodies are now included, generated from the same zod schemas the endpoints validate against.

apps/api/package.json

@@ -36,12 +36,14 @@
     "@flakemetry/contracts": "workspace:*",
     "@flakemetry/db": "workspace:*",
     "@flakemetry/queries": "workspace:*",
+    "@flakemetry/storage": "workspace:*",
     "@opentelemetry/api": "^1.9.0",
     "@opentelemetry/exporter-metrics-otlp-http": "^0.221.0",
     "@opentelemetry/resources": "^2.10.0",
     "@opentelemetry/sdk-metrics": "^2.10.0",
     "@trpc/server": "^11.0.0",
     "fastify": "^5.11.0",
-    "@flakemetry/storage": "workspace:*"
+    "zod": "^3.25.76",
+    "zod-to-json-schema": "^3.24.1"
   }
 }

apps/api/src/__tests__/api-surface.test.ts

@@ -7,7 +7,7 @@ import type { PrismaClient } from '@flakemetry/db'
 import { afterAll, beforeAll, describe, expect, it } from 'vitest'
 
 import { buildApp } from '../app'
-import { READ_ROUTES } from '../rest'
+import { openApiDocument, READ_ROUTES } from '../rest'
 import { appRouter } from '../trpc/router'
 
 const app = buildApp({ prisma: {} as unknown as PrismaClient, store: null })
@@ -69,3 +69,57 @@ describe('documented API surface', () => {
     expect(documented).toEqual(exposed)
   })
 })
+
+describe('the machine-readable description covers the whole surface', () => {
+  const document = openApiDocument('0.0.0') as {
+    paths: Record<string, Record<string, { requestBody?: unknown; security?: unknown[] }>>
+  }
+
+  it('describes every documented endpoint, not only the readable half', () => {
+    // It used to be generated from READ_ROUTES, so ingestion, the quality gate and
+    // quarantine were invisible to anything generating a client — with nothing in the
+    // document to say a client was seeing a fraction of the API.
+    const missing = REST_ENDPOINTS.filter((endpoint) => {
+      const path = endpoint.path.replace(/:(\w+)/g, '{$1}')
+      return !document.paths[path]?.[endpoint.method.toLowerCase()]
+    }).map((endpoint) => `${endpoint.method} ${endpoint.path}`)
+
+    expect(missing).toEqual([])
+  })
+
+  it('describes the body of every endpoint that takes one', () => {
+    for (const endpoint of REST_ENDPOINTS.filter((candidate) => candidate.request)) {
+      const path = endpoint.path.replace(/:(\w+)/g, '{$1}')
+      const operation = document.paths[path]?.[endpoint.method.toLowerCase()]
+      expect(operation?.requestBody, `${endpoint.path} has no requestBody`).toBeDefined()
+    }
+  })
+
+  it('leaves the unauthenticated endpoints unauthenticated', () => {
+    const health = document.paths['/health']?.get
+    const ingest = document.paths['/v1/ingest']?.post
+
+    // A document that demands a token for the liveness probe teaches people to ignore its
+    // security blocks entirely.
+    expect(health?.security).toEqual([])
+    expect(ingest?.security).not.toEqual([])
+  })
+
+  it('declares every path parameter it names', () => {
+    const undeclared: string[] = []
+    for (const [path, operations] of Object.entries(document.paths)) {
+      const named = [...path.matchAll(/\{(\w+)\}/g)].map((match) => match[1])
+      for (const [method, operation] of Object.entries(operations)) {
+        const params = ((operation as { parameters?: { name: string; in: string }[] }).parameters ??
+          []) as { name: string; in: string }[]
+        for (const name of named) {
+          if (!params.some((param) => param.name === name && param.in === 'path')) {
+            undeclared.push(`${method.toUpperCase()} ${path} → ${name}`)
+          }
+        }
+      }
+    }
+    // A path with an undeclared parameter is a URL no generated client can build.
+    expect(undeclared).toEqual([])
+  })
+})

apps/api/src/__tests__/rest.test.ts

@@ -1,5 +1,6 @@
 import { randomUUID } from 'node:crypto'
 
+import { REST_ENDPOINTS } from '@flakemetry/contracts'
 import { generateToken, hashToken, PrismaClient } from '@flakemetry/db'
 import type { FastifyInstance } from 'fastify'
 import { afterAll, beforeEach, describe, expect, it } from 'vitest'
@@ -27,10 +28,23 @@ describe('openapi document', () => {
   it('describes every route that is registered', () => {
     const document = openApiDocument('0.1.0') as { paths: Record<string, unknown> }
 
-    // Generated from the same table that registers the routes, so a spec that disagrees with
-    // the implementation is not expressible.
-    expect(Object.keys(document.paths)).toHaveLength(READ_ROUTES.length)
+    // Generated from REST_ENDPOINTS — the same table the human reference is built from —
+    // so a spec that disagrees with the implementation is not expressible. It covers the
+    // whole surface, not only the read routes, which is why the count is checked against
+    // the documented endpoints rather than against READ_ROUTES.
+    const documented = new Set(
+      REST_ENDPOINTS.map((endpoint) => endpoint.path.replace(/:(\w+)/g, '{$1}')),
+    )
+    expect(Object.keys(document.paths).sort()).toEqual([...documented].sort())
     expect(document.paths['/v1/runs/{runId}']).toBeDefined()
+    expect(document.paths['/v1/ingest']).toBeDefined()
+  })
+
+  it('still registers every read route it describes', () => {
+    expect(READ_ROUTES.length).toBeGreaterThan(0)
+    for (const route of READ_ROUTES) {
+      expect(REST_ENDPOINTS.some((endpoint) => endpoint.path === route.path)).toBe(true)
+    }
   })
 })
 

apps/api/src/rest.ts

@@ -3,6 +3,9 @@ import { createGzip } from 'node:zlib'
 
 import {
   flakyBoardInputSchema,
+  REST_ENDPOINTS,
+  type RestAuth,
+  type RestEndpoint,
   runsListInputSchema,
   testGetInputSchema,
   testHealthInputSchema,
@@ -22,6 +25,7 @@ import {
 } from '@flakemetry/queries'
 import { type ObjectStore, projectArtifactPrefix } from '@flakemetry/storage'
 import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify'
+import { zodToJsonSchema } from 'zod-to-json-schema'
 
 import { type AuthenticatedProject, authenticateProject, hasScope } from './auth'
 import type { RateLimiter } from './rate-limit'
@@ -218,58 +222,115 @@ export const READ_ROUTES: ReadRoute[] = [
   },
 ]
 
+const SCOPE_NOTE: Record<RestAuth, string | null> = {
+  none: null,
+  'ingest-token': 'Needs a token carrying the "ingest" scope.',
+  'read-token': 'Needs a token carrying the "read" scope.',
+  'quarantine-token': 'Needs a token carrying the "quarantine" scope.',
+  'any-token': 'Needs a project token carrying either the "ingest" or the "read" scope.',
+}
+
+const LEADING_STATUS = /^`(\d{3})`/
+
+const successStatus = (response: string): number => {
+  const match = LEADING_STATUS.exec(response.trim())
+  return match ? Number(match[1]) : 200
+}
+
+const readRouteByPath = new Map(READ_ROUTES.map((route) => [route.path, route]))
+
+const parametersFor = (endpoint: RestEndpoint): unknown[] => {
+  const route = readRouteByPath.get(endpoint.path)
+  const declared = [
+    ...(route?.params ?? []).map((param) => ({ ...param, in: 'path', required: true })),
+    ...(route?.query ?? []).map((param) => ({ ...param, in: 'query', required: false })),
+  ]
+  if (declared.length > 0) {
+    return declared.map((param) => ({
+      name: param.name,
+      in: param.in,
+      required: param.required,
+      description: param.description,
+      schema: { type: 'string' },
+    }))
+  }
+
+  // Paths carrying a segment that no route table describes still have to declare it, or
+  // the document is not a valid description of a URL a client can build.
+  return [...endpoint.path.matchAll(/:(\w+)/g)].map((match) => ({
+    name: match[1],
+    in: 'path',
+    required: true,
+    schema: { type: 'string' },
+  }))
+}
+
+/**
+ * Generated from `REST_ENDPOINTS` — the same table the human reference is built from — so
+ * the machine-readable description covers the whole surface rather than the read half.
+ * It previously came from `READ_ROUTES`, which meant ingestion, the quality gate and
+ * quarantine were invisible to anything generating a client, with nothing to say so.
+ */
 export const openApiDocument = (version: string): Record<string, unknown> => {
-  const paths: Record<string, unknown> = {}
+  const paths: Record<string, Record<string, unknown>> = {}
+
+  for (const endpoint of REST_ENDPOINTS) {
+    const path = endpoint.path.replace(/:(\w+)/g, '{$1}')
+    const route = readRouteByPath.get(endpoint.path)
+    const scopeNote = SCOPE_NOTE[endpoint.auth]
+
+    const responses: Record<string, unknown> = {
+      [successStatus(endpoint.response)]: {
+        description: endpoint.response,
+        ...(route?.produces ? { content: { [route.produces]: {} } } : {}),
+      },
+    }
+    if (endpoint.auth !== 'none') {
+      responses[401] = { description: 'Missing or invalid token' }
+      responses[403] = { description: 'The token does not carry the required scope' }
+      responses[429] = { description: 'Rate limited' }
+    }
 
-  for (const route of READ_ROUTES) {
-    const path = route.path.replace(/:(\w+)/g, '{$1}')
     paths[path] = {
-      get: {
-        summary: route.summary,
-        security: [{ bearerAuth: [] }],
-        parameters: [
-          ...(route.params ?? []).map((param) => ({
-            name: param.name,
-            in: 'path',
-            required: true,
-            description: param.description,
-            schema: { type: 'string' },
-          })),
-          ...(route.query ?? []).map((param) => ({
-            name: param.name,
-            in: 'query',
-            required: false,
-            description: param.description,
-            schema: { type: 'string' },
-          })),
-        ],
-        responses: {
-          200: route.produces
-            ? { description: 'Success', content: { [route.produces]: {} } }
-            : { description: 'Success' },
-          401: { description: 'Missing or invalid token' },
-          403: { description: 'The token does not carry the read scope' },
-          404: { description: 'Not found' },
-          429: { description: 'Rate limited' },
-        },
+      ...paths[path],
+      [endpoint.method.toLowerCase()]: {
+        summary: endpoint.summary,
+        ...(scopeNote ? { description: scopeNote } : {}),
+        security: endpoint.auth === 'none' ? [] : [{ bearerAuth: [] }],
+        parameters: parametersFor(endpoint),
+        ...(endpoint.request
+          ? {
+              requestBody: {
+                required: true,
+                content: {
+                  'application/json': {
+                    // Inlined rather than named: a `name` puts the schema under
+                    // `definitions` and leaves a $ref pointing where OpenAPI does not
+                    // look, so a generated client cannot resolve it.
+                    schema: zodToJsonSchema(endpoint.request.schema, { $refStrategy: 'none' }),
+                  },
+                },
+              },
+            }
+          : {}),
+        responses,
       },
     }
   }
 
   return {
     openapi: '3.1.0',
     info: {
-      title: 'Flakemetry read API',
+      title: 'Flakemetry API',
       version,
       description:
-        'Read-only access to a single project. Authorise with a token carrying the "read" scope; an ingest token will not do, so a credential handed to a script cannot forge test data.',
+        'Ingestion, read and quarantine for a single project. Scopes are separate on purpose: a credential handed to a script cannot forge test data, one that reads cannot silence a failing test, and the one in every CI job carries neither.',
     },
     components: {
       securitySchemes: {
         bearerAuth: { type: 'http', scheme: 'bearer', description: 'A project token' },
       },
     },
-    security: [{ bearerAuth: [] }],
     paths,
   }
 }