feat: machine-readable API surface at /api/openapi.json#68
Conversation
The platform had ~400 operations with no machine-readable contract. Express 5 hides mount prefixes inside matcher closures, so the routes aggregator now declares its mounts as a data table (same factories, same registration order — /swarms/spawns still precedes /swarms — and per-mount middleware preserved), which both mounts the routers and feeds a route-inventory walker. GET /api/openapi.json (authenticated) serves an OpenAPI 3.1 document: paths with :params templated, per-operation bearer security derived from each mount's middleware, built once and cached. ponytail: paths + methods + auth only, no request/response schemas — attach zod schemas per route when a consumer needs typed clients. Verified live: booted server, unauthenticated fetch 401, authenticated fetch returned 367 paths / 400 operations. Full suite (incl. the e2e smoke booting the refactored aggregator) 1,216 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
There was a problem hiding this comment.
Code Review
This pull request introduces a declarative route inventory system to automatically generate an OpenAPI 3.1 specification (/openapi.json) from the Express router mounts, refactoring the main route registration to use a structured mounts table. Feedback focuses on enhancing the robustness of the route inventory utility, including handling optional or regex-constrained Express parameters in OpenAPI path templates, avoiding fragile authentication detection based solely on middleware presence, and improving type safety when casting the Express router stack.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export interface RouteMount { | ||
| prefix: string; | ||
| middleware: RequestHandler[]; | ||
| router: Router; | ||
| } |
There was a problem hiding this comment.
To make the route inventory more robust and future-proof, consider adding an optional authenticated property to the RouteMount interface. This allows explicit overrides if non-authentication middleware (e.g., rate limiters, logging) is added to a public route in the future.
| export interface RouteMount { | |
| prefix: string; | |
| middleware: RequestHandler[]; | |
| router: Router; | |
| } | |
| export interface RouteMount { | |
| prefix: string; | |
| middleware: RequestHandler[]; | |
| router: Router; | |
| authenticated?: boolean; | |
| } |
| /** ':param' → '{param}' for OpenAPI path templates. */ | ||
| function toOpenApiPath(path: string): string { | ||
| return path.split('/').map((seg) => (seg.startsWith(':') ? `{${seg.slice(1)}}` : seg)).join('/'); | ||
| } |
There was a problem hiding this comment.
Express routes often use optional parameters (e.g., :id?) or regex constraints (e.g., :id(\\d+)). If left unhandled, these will produce invalid OpenAPI path templates like {id?} or {id(\\d+)}, which can break OpenAPI parsers and client generators. Consider stripping these modifiers during conversion.
/** ':param' → '{param}' for OpenAPI path templates, stripping optional modifiers and regex constraints. */
function toOpenApiPath(path: string): string {
return path
.split('/')
.map((seg) => {
if (seg.startsWith(':')) {
const cleanParam = seg.slice(1).replace(/\?$/, '').replace(/\(.*\)$/, '');
return `{${cleanParam}}`;
}
return seg;
})
.join('/');
}| export function collectRoutes(mounts: RouteMount[], basePath = '/api'): RouteEntry[] { | ||
| const entries: RouteEntry[] = []; | ||
| for (const mount of mounts) { | ||
| const authenticated = mount.middleware.length > 0; |
There was a problem hiding this comment.
Deriving authentication status solely based on whether middleware.length > 0 is fragile. If any non-authentication middleware is added to a route mount in the future, it will be incorrectly marked as authenticated in the OpenAPI spec. Use the optional authenticated override if provided.
| const authenticated = mount.middleware.length > 0; | |
| const authenticated = mount.authenticated ?? mount.middleware.length > 0; |
| const stack: Array<{ route?: { path: string | string[]; methods: Record<string, boolean> } }> = | ||
| (mount.router as unknown as { stack?: never[] }).stack ?? []; |
There was a problem hiding this comment.
Casting mount.router to { stack?: never[] } is confusing and can lead to type-safety issues or compiler warnings under strict configurations because never[] represents an array that can never contain elements. Instead, cast it directly to the expected shape of the Express router stack.
const stack = (mount.router as unknown as {
stack?: Array<{ route?: { path: string | string[]; methods: Record<string, boolean> } }>;
}).stack ?? [];There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fecd00940
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { createRuntimeGovernanceRoutes } from './runtime-governance'; | ||
| import { createCognitiveRoutes } from './cognitive'; | ||
| import { createMemoryRoutes } from './memory'; | ||
| import { createMemoryEvolutionRoutes } from './memory-evolution'; |
There was a problem hiding this comment.
Restore the missing memory-evolution route module
I checked rg --files packages/server/src/routes and the commit tree, and there is no packages/server/src/routes/memory-evolution.ts module. Because the routes aggregator imports it at startup, @djimitflo/server will fail to compile/start with a module-not-found error before the new OpenAPI endpoint can be served; either commit this route file or remove the import and mount.
Useful? React with 👍 / 👎.
| const stack: Array<{ route?: { path: string | string[]; methods: Record<string, boolean> } }> = | ||
| (mount.router as unknown as { stack?: never[] }).stack ?? []; | ||
| for (const layer of stack) { | ||
| if (!layer.route) continue; // plain middleware; current factories don't nest routers |
There was a problem hiding this comment.
Recurse into nested routers when collecting routes
This drops every mounted sub-router from the inventory. createSwarmRoutes currently installs four real route factories with router.use('/', ...) in packages/server/src/routes/swarms.ts:111-114, so endpoints such as /api/swarms/scheduler/tick and /api/swarms/decompose remain live but are omitted from /api/openapi.json; the collector needs to descend into router layers instead of skipping all non-route layers.
Useful? React with 👍 / 👎.
| export function collectRoutes(mounts: RouteMount[], basePath = '/api'): RouteEntry[] { | ||
| const entries: RouteEntry[] = []; | ||
| for (const mount of mounts) { | ||
| const authenticated = mount.middleware.length > 0; |
There was a problem hiding this comment.
Derive authentication per route for mixed mounts
Using only mount-level middleware marks every endpoint in a mixed router the same way. The /auth mount is declared with no middleware, but packages/server/src/routes/auth.ts:52 protects GET /me with auth.requireAuth, so the generated contract advertises /api/auth/me as public and clients generated from it will omit the bearer token and get 401s; the inventory needs to account for route-level auth or allow per-route overrides.
Useful? React with 👍 / 👎.
The original commit accidentally included an uncommitted local wiring of ./memory-evolution whose route file is untracked — CI cannot resolve it. Removed; that wiring belongs to its own branch. CodeQL js/missing-rate-limiting on the authenticated spec endpoint: express-rate-limit (same policy and version as the /metrics fix). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
The platform had 400 operations across 367 paths with no machine-readable contract (the README's "~160 endpoints" undercounted by 2.5×). This ships the contract.
createRoutesnow declares its mounts as a data table — same factories, same registration order (/swarms/spawnsstill precedes/swarms, the/diff routes keep their slot), per-mount middleware preserved (requireAuth/requireAuthOrSpawnToken/ public). One loop mounts them; the same table feeds the inventory.GET /api/openapi.json(authenticated) — OpenAPI 3.1::paramstemplated to{param}, per-operationbearerAuthsecurity derived from each mount's middleware, built once and cached.Validation
openapi: 3.1.0, 367 paths, 400 operationsNoted for follow-up
The full-aggregator test surfaced a latent name collision: two services declare different
governance_feedbacktable shapes (legal/ecli vs feedback-service) — whichever runs first wins and the other'sCREATE TABLE IF NOT EXISTSsilently no-ops. Worth its own issue.🤖 Generated with Claude Code