You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
registerRouteSpec in src/openapi/define-route.ts builds one object literal and hands it to registry.registerPath. That literal contains the key requesttwice: once at line 169 (the only place path parameters are added, via ...(pathParameters(options.path) ?? {})) and again at line 181 inside a conditional spread that fires when options.request?.body || options.request?.query.
In JavaScript a later property (here, one arriving through a conditional spread) replaces an earlier one of the same name — verified: { request: {params:"P"}, ...(true ? { request: { body: "B" } } : {}) } evaluates to {"request":{"body":"B"}}. TypeScript does not flag it, because the duplicate arrives via a spread rather than as a literal duplicate key.
Consequence: for any route that has BOTH a templated path segment AND a declared request body or query, the emitted operation carries no parameters array at all. That is exactly the failure pathParameters()'s own doc comment (src/openapi/define-route.ts:49-56) says it exists to prevent — "a templated segment with no matching parameter is a schema-validation warning (Cloudflare 30046) and leaves a generated client with a URL it cannot fill".
The bug is currently latent only because no production caller passes request yet — every entry in src/openapi/orb-and-control-route-specs.ts and src/openapi/internal-and-public-route-specs.ts is response-only, and defineRoute has no callers in src/**. The first migrated POST /v1/repos/:owner/:repo/... handler (the stated next step of #9531) silently loses its owner/repo parameters.
test/unit/define-route.test.ts:193-209 already registers exactly the broken combination (path: "/v1/spec-only/:id" plus request: { body: ... }) and asserts only expect(operation?.requestBody).toBeDefined() — it never asserts the id parameter, so the defect passes the existing suite.
Requirements
Build the request object once in registerRouteSpec, merging path parameters, body, and query into a single key so no later property can shadow it.
The emitted operation for a route with path params and no body/query must be unchanged from today.
The emitted operation for a route with path params and a body must carry both the requestBody and a parameters entry (in: "path", required: true) for every :param in the path.
The same must hold for a route with path params and a query schema, and for one with path params, a body, and a query.
defineRoute (which delegates to registerRouteSpec) inherits the fix with no separate code path.
The committed apps/loopover-ui/public/openapi.json must be regenerated with npm run ui:openapi. It is expected to be byte-identical today; if it is not, the diff is the bug and must be committed.
⚠️ Required pattern: keep the single call to registry.registerPath and the existing pathParameters() / toSpecPath() helpers exactly as they are — the fix is to stop emitting the key twice, not to restructure the emitter. It does NOT satisfy this issue to delete the first request block (that removes path parameters from every route permanently), to add a second registerPath call, to introduce a new helper module, or to switch to @hono/zod-openapi (explicitly rejected in this file's module header).
Deliverables
registerRouteSpec emits exactly one request key, containing path parameters, body, and query together.
A new case in test/unit/define-route.test.ts registers a spec-only route with a templated path AND a request body, and asserts the generated operation's parameters contains an entry with { in: "path", name: "<param>", required: true } for that param — this case fails on current main and passes after the fix.
A second new case does the same for a route with a templated path AND a query schema.
A third new case covers a route with a templated path, a body, AND a query, asserting parameters, requestBody, and the query parameter are all present in one operation.
The existing assertion at test/unit/define-route.test.ts:206-208 is extended (not replaced) so the spec-only route's id path parameter is asserted alongside requestBody.
apps/loopover-ui/public/openapi.json regenerated and committed.
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example fixing the duplicate key without adding the failing-before/passing-after parameter assertion, or adding the assertion against a route with no body (which passes today and proves nothing) — does not resolve this issue.
Test Coverage Requirements
src/openapi/** is inside Codecov's src/** include, so the 99% branch-counted patch gate applies. Every arm of the merged conditional (path params present/absent, body present/absent, query present/absent) must be exercised: the four new/extended cases above plus the existing no-request cases cover them. The parameter assertion is the named regression test for this fix and must assert the concrete parameters array, not merely that the operation exists.
Expected Outcome
A route registered through the seam with both a templated path and a request body publishes a complete operation: parameters for every path segment and a requestBody. Migrating the first body-carrying repo route through defineRoute no longer silently strips {owner}/{repo} from the published document.
Links & Resources
src/openapi/define-route.ts:157-189, :49-61; test/unit/define-route.test.ts:193-210. Seam introduced by #9519; migration is #9531.
Context
registerRouteSpecinsrc/openapi/define-route.tsbuilds one object literal and hands it toregistry.registerPath. That literal contains the keyrequesttwice: once at line 169 (the only place path parameters are added, via...(pathParameters(options.path) ?? {})) and again at line 181 inside a conditional spread that fires whenoptions.request?.body || options.request?.query.In JavaScript a later property (here, one arriving through a conditional spread) replaces an earlier one of the same name — verified:
{ request: {params:"P"}, ...(true ? { request: { body: "B" } } : {}) }evaluates to{"request":{"body":"B"}}. TypeScript does not flag it, because the duplicate arrives via a spread rather than as a literal duplicate key.Consequence: for any route that has BOTH a templated path segment AND a declared request body or query, the emitted operation carries no
parametersarray at all. That is exactly the failurepathParameters()'s own doc comment (src/openapi/define-route.ts:49-56) says it exists to prevent — "a templated segment with no matching parameter is a schema-validation warning (Cloudflare 30046) and leaves a generated client with a URL it cannot fill".The bug is currently latent only because no production caller passes
requestyet — every entry insrc/openapi/orb-and-control-route-specs.tsandsrc/openapi/internal-and-public-route-specs.tsis response-only, anddefineRoutehas no callers insrc/**. The first migratedPOST /v1/repos/:owner/:repo/...handler (the stated next step of #9531) silently loses itsowner/repoparameters.test/unit/define-route.test.ts:193-209already registers exactly the broken combination (path: "/v1/spec-only/:id"plusrequest: { body: ... }) and asserts onlyexpect(operation?.requestBody).toBeDefined()— it never asserts theidparameter, so the defect passes the existing suite.Requirements
requestobject once inregisterRouteSpec, merging path parameters, body, and query into a single key so no later property can shadow it.requestBodyand aparametersentry (in: "path",required: true) for every:paramin the path.queryschema, and for one with path params, a body, and a query.defineRoute(which delegates toregisterRouteSpec) inherits the fix with no separate code path.apps/loopover-ui/public/openapi.jsonmust be regenerated withnpm run ui:openapi. It is expected to be byte-identical today; if it is not, the diff is the bug and must be committed.Deliverables
registerRouteSpecemits exactly onerequestkey, containing path parameters, body, and query together.test/unit/define-route.test.tsregisters a spec-only route with a templated path AND a request body, and asserts the generated operation'sparameterscontains an entry with{ in: "path", name: "<param>", required: true }for that param — this case fails on currentmainand passes after the fix.queryschema.parameters,requestBody, and the query parameter are all present in one operation.test/unit/define-route.test.ts:206-208is extended (not replaced) so the spec-only route'sidpath parameter is asserted alongsiderequestBody.apps/loopover-ui/public/openapi.jsonregenerated and committed.All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example fixing the duplicate key without adding the failing-before/passing-after parameter assertion, or adding the assertion against a route with no body (which passes today and proves nothing) — does not resolve this issue.
Test Coverage Requirements
src/openapi/**is inside Codecov'ssrc/**include, so the 99% branch-counted patch gate applies. Every arm of the merged conditional (path params present/absent, body present/absent, query present/absent) must be exercised: the four new/extended cases above plus the existing no-request cases cover them. The parameter assertion is the named regression test for this fix and must assert the concreteparametersarray, not merely that the operation exists.Expected Outcome
A route registered through the seam with both a templated path and a request body publishes a complete operation:
parametersfor every path segment and arequestBody. Migrating the first body-carrying repo route throughdefineRouteno longer silently strips{owner}/{repo}from the published document.Links & Resources
src/openapi/define-route.ts:157-189,:49-61;test/unit/define-route.test.ts:193-210. Seam introduced by #9519; migration is #9531.