Fix: optional query arg silently hoisted past required body arg, swapping caller arguments

[ngDuyAnh]

Summary

extractRequestParams generates a wrapper with arguments in the wrong order
when an endpoint has a non-required query parameter together with a required
request body. The generated signature comes out as (body, query, params)
instead of (query, body, params), which silently swaps the first two
positional arguments at every existing call site.

The bug is a runtime regression introduced by #1756. The output is still
type-legal under // @ts-nocheck (the default generator header), so TypeScript
does not catch the swap; the failure surfaces only when the wrapper is called
and the body content is serialized into the URL query string.

Reproduction

Spec (minimal):

{
  "openapi": "3.0.0",
  "paths": {
    "/check-impact": {
      "post": {
        "operationId": "checkImpact",
        "parameters": [
          { "in": "query", "name": "cadence", "schema": { "type": "string" } }
        ],
        "requestBody": {
          "required": true,
          "content": { "application/json": { "schema": {
            "type": "object",
            "required": ["staff_id"],
            "properties": { "staff_id": { "type": "integer" } }
          } } }
        },
        "responses": { "200": { "description": "OK" } }
      }
    }
  }
}

Generate with --extract-request-params --extract-request-body:

Before (current main)

checkImpact = (
  data: CheckImpactPayload,           // ← first
  query: CheckImpactParams = {},      // ← second
  params: RequestParams = {},
) => this.request({ path: "/check-impact", method: "POST", query, body: data, ... })

A caller writing api.checkImpact({ cadence: "weekly" }, { staff_id: 1 }) —
which reads naturally as (query, body) and matches the OpenAPI declaration
order — passes { cadence: "weekly" } as body and { staff_id: 1 } as
query. The body fields are URL-encoded into the query string and the server
sees an empty JSON body. There is no runtime error; the request is just wrong.

Expected

checkImpact = (
  query: CheckImpactParams = {},      // ← first, in declaration order
  data: CheckImpactPayload,           // ← second, required
  params: RequestParams = {},
)

Regression timeline

Version	Behavior
13.2.3 — 13.2.17 (last tagged release, Feb 8 2026)	Correct: (query, body, params)
fed24c6 / #1756 ("Fix: query params object incorrectly required when all fields are optional", May 21 2026, currently on main / 13.12.x)	Regressed: (body, query, params)

PR #1756 surfaced
requestParamsSchema.typeData.allFieldsAreOptional from schema-routes.ts as
route.request.requestParamsOptional and threaded it into the template so the
extracted query argument gets marked optional when all of its fields are
optional. That part is a real UX improvement — callers can omit the empty
{} placeholder for endpoints whose only query parameters are optional.

The unintended interaction is with the existing "Sort by optionality" step
in templates/{default,modular}/procedure-call.ejs:

const wrapperArgs = _
    // Sort by optionality
    .sortBy(rawWrapperArgs, [o => o.optional])
    .map(argToTmpl)
    .join(', ')

_.sortBy is stable and puts false (required) before true (optional). For
endpoints with a required body and a now-optional query, the required body is
hoisted ahead of the optional query — even though the query already carries a
defaultValue: '{}' that would let it sit in front of a required argument
without producing invalid TypeScript.

Real-world impact

Every existing caller that was written against the pre-#1756 wrapper shape now
swaps body and query at runtime, with no compiler or runtime error to flag it.
This affects any project that pins ^13.2.3 (the documented range) and
re-resolves a fresh node_modules — for example, a CI build that runs npm install from a non-committed lockfile after the publish that contains #1756.

Concretely: I hit this on a deployed app where ~/local resolved ^13.2.3 to
13.2.16 and generated the correct shape; the CI deploy resolved the same
range to a post-#1756 version and generated the swapped shape. Same call site,
same spec, two different argument orders, and a 400 response with the body
fields URL-encoded into the query string.

Proposed solution

The intent of the sortBy step is to keep generated signatures
TypeScript-legal: a required parameter cannot follow a parameter typed as
name?: T. But an argument that has a defaultValue is only positionally
optional — argToTmpl already emits it as name: T = default, not name?: T,
and TS happily accepts a required parameter after that.

So the fix is to leave defaulted args in declaration order when a required
arg follows them, while still pushing truly-optional (?:) args to the end:

// Before sorting, promote any optional arg that has a defaultValue and is
// followed by a required arg to "positionally required". The sort still
// pushes truly-optional (`?:`) args to the end, but defaultable args ahead
// of a required arg stay in declaration order, so callers' positional
// arguments don't silently shift when a previously-required query becomes
// optional.
const positionedWrapperArgs = rawWrapperArgs.map((arg, i) => {
    if (arg.optional && arg.defaultValue && rawWrapperArgs.slice(i + 1).some(a => !a.optional)) {
        return { ...arg, optional: false };
    }
    return arg;
})

const wrapperArgs = _
    .sortBy(positionedWrapperArgs, [o => o.optional])
    .map(argToTmpl)
    .join(', ')

Applied identically to templates/default/procedure-call.ejs and
templates/modular/procedure-call.ejs.

Why this preserves #1756's UX win

For endpoints where the body is also optional (or there is no body), no
required arg follows the query, so the predicate doesn't fire and the query
stays optional: true. Callers can still omit it entirely:

listThings = (query?: ListThingsParams, params: RequestParams = {}) => ...
api.listThings();                  // still works, no required body in the way

It only kicks in when there is a required arg after the defaulted arg —
exactly the case where the sort would otherwise swap them.

Why not simpler alternatives

• Remove the sort. Would emit invalid TS (query?: T, body: T2) for
  endpoints whose query arg has no defaultValue.
• Sort by o => o.optional && !o.defaultValue (treat any defaulted arg as
  positionally required). Promotes too much: requestConfigParam always
  carries defaultValue: '{}', and this would hoist it ahead of a trailing
  query?: T for GET endpoints, churning unrelated snapshots. The
  position-aware variant in this PR only fires when the swap actually matters.
• Revert Fix: query params object incorrectly required when all fields are optional (extractRequestParams) #1756. Throws away a genuine ergonomic improvement and forces
  callers to pass {} even when there is nothing to pass.

Tests

Adds tests/spec/optional-query-required-body/:

• Minimal OpenAPI schema (one POST endpoint, optional cadence query, required
  body).
• Test asserts the generated signature has query before data and that the
  query arg keeps its = {} default. Locks the entire generated file via
  snapshot.
• Verified to fail on main and pass with the fix.

Updates two existing snapshots that were locking in the regression. Both
endpoints (authentiq and extractRequestParams test fixture) use the
signRequest operation, which has body: Claims + optional query and was
being generated as (body, query, params). The new (correct) snapshot is:

signRequest: (
  query: SignRequestParams = {},
  body: Claims,
  params: RequestParams = {},
) => ...

All other tests pass. (The 4 failing paths-2* strict-TSC tests in my local
run fail on main too — they shell out to bunx tsc, which wasn't on PATH in
my environment; not related to this change.)

End-to-end verification

Regenerated a real client (~120 endpoints) against the patched generator. Only
the two endpoints whose swagger matches the bug shape (required body +
non-required query) changed; every other endpoint's signature is byte-identical
to the 13.2.x output. Both regressed endpoints now produce
(query: ... = {}, data, params = {}), matching what their callers were
written against.

Closes the silent-swap regression introduced by #1756 without reverting that
PR's UX improvement.

---

Summary by cubic

Fixes a regression that flipped generated wrapper arguments to (body, query, params) when the query is optional and the body is required, which silently swapped caller arguments at runtime. Preserves the original (query, body, params) order while keeping the UX from #1756 where empty query objects can be omitted.

• Bug Fixes
  • In templates/{default,modular}/procedure-call.ejs, promote optional args with a default value to “positionally required” when a required arg follows, then sort. This keeps query before body and still emits query: T = {} (not query?: T).
  • Added tests/spec/optional-query-required-body to lock the expected signature and full snapshot.
  • Updated two snapshots (signRequest) to the corrected (query = {}, body, params) order.

^{Written for commit 4252aed. Summary will update on new commits.}

[changeset-bot]

⚠️ No Changeset found

Latest commit: 4252aed

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[cubic-dev-ai]

No issues found across 7 files

_{Re-trigger cubic}