Skip to content

fix(middleware): declare withInjectionGuard's context parameter optional - #8644

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
backryun:chore/ts7-types-batch-dispatch
Jul 27, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
backryun:chore/ts7-types-batch-dispatch

Conversation

@backryun

Copy link
Copy Markdown
Contributor

Part of #8484. One character. Clears all five diagnostics in src/lib/batches/dispatch.ts.

A declaration that was wrong about its own runtime

Type '(request: any, context: any) => Promise<any>' is not assignable to type 'BatchRouteHandler'.
  Target signature provides too few arguments. Expected 2 or more, but got 1.        ×5

withInjectionGuard() returns

return async function guardedHandler(request: any, context: any) {

with the second parameter required, so every route it wraps advertises arity 2. The batch dispatcher's handler type is (request: Request) => Promise<Response> | Response, and TS correctly refuses to assign a function that needs an argument the caller will never pass.

The interesting part is which side was wrong. dispatch.ts:48 already calls it with one argument:

return await handler(request);

and has been doing so in production. context is only forwarded on to the inner handler, where routes that do not read it receive undefined — which is exactly what one-argument invocation produces. So the parameter has always been optional in practice; the signature just did not say so.

-  return async function guardedHandler(request: any, context: any) {
+  return async function guardedHandler(request: any, context?: any) {

Widening BatchRouteHandler to take two parameters would have been the other option, but that fixes it at the consumer and leaves four other wrapped routes still misdescribing their own arity.

Verification

208 → 203, zero new errors, on a line-number-agnostic diff of the full tsc error set.

  • npm run typecheck:core — clean
  • eslint, check:file-size — clean
  • 370/370 across the 43 affected suites (injection-guard, batch, embeddings, moderations)

Nothing executable changed — an optional marker has no runtime representation.

No test added — the path is the existing behaviour

Checking coverage of what the declaration now permits is standard for these slices, and here it is already thorough. embeddings-auth.test.ts (4 call sites) and embeddings-route-apikeymeta-6929.test.ts call POST(req) with a single argument straight through withInjectionGuard — precisely the arity this change legalises, exercised end-to-end and passing before and after.

Worth noting what is not covered: batch-processor.test.ts mocks dispatchBatchApiRequest, so the batch path never reaches a real guarded handler. That gap predates this change and is unaffected by it — the one-argument contract is proven by the route tests instead.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Hi @backryun — clean one-liner. Optional context removes the withInjectionGuard(() => x, undefined) antipattern at callsites.

Approved as merge-ready (★4). /merge-prs will pick it up.

@backryun
backryun force-pushed the chore/ts7-types-batch-dispatch branch from fb34739 to a129b59 Compare July 27, 2026 15:18
All five diagnostics in src/lib/batches/dispatch.ts are the same:

  Type '(request: any, context: any) => Promise<any>' is not assignable to
  type 'BatchRouteHandler'.
    Target signature provides too few arguments. Expected 2 or more, but got 1.

`withInjectionGuard()` returns `guardedHandler(request, context: any)` with the
second parameter required, so every route it wraps advertises arity 2. The batch
dispatcher's `BatchRouteHandler` is `(request: Request) => …`, and TS rejects
assigning a function that needs an argument the caller will never supply.

The declaration was wrong about its own runtime. `dispatch.ts:48` already calls
`handler(request)` with one argument, and has been doing so in production;
`context` is only forwarded to the inner handler, where routes that do not read
it get `undefined`. Marking it `context?: any` states what was already true.

208 -> 203, zero new, on a line-number-agnostic diff of the full tsc error set.
One character; nothing executable changed.

No test added: the one-argument call path is the existing behaviour and is
already covered — embeddings-auth.test.ts and embeddings-route-apikeymeta-6929
call `POST(req)` directly through withInjectionGuard, which is exactly the arity
this now permits. 370/370 across the 43 injection-guard / batch / embeddings /
moderation suites; typecheck:core, eslint and check:file-size clean.
@backryun
backryun force-pushed the chore/ts7-types-batch-dispatch branch from a129b59 to f9810c7 Compare July 27, 2026 15:35
@diegosouzapw
diegosouzapw merged commit 6601ae3 into cheaper-inference:release/v3.8.49 Jul 27, 2026
15 checks passed
@backryun
backryun deleted the chore/ts7-types-batch-dispatch branch July 27, 2026 22:21
@diegosouzapw diegosouzapw mentioned this pull request Jul 28, 2026
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…nal (cheaper-inference#8644)

All five diagnostics in src/lib/batches/dispatch.ts are the same:

  Type '(request: any, context: any) => Promise<any>' is not assignable to
  type 'BatchRouteHandler'.
    Target signature provides too few arguments. Expected 2 or more, but got 1.

`withInjectionGuard()` returns `guardedHandler(request, context: any)` with the
second parameter required, so every route it wraps advertises arity 2. The batch
dispatcher's `BatchRouteHandler` is `(request: Request) => …`, and TS rejects
assigning a function that needs an argument the caller will never supply.

The declaration was wrong about its own runtime. `dispatch.ts:48` already calls
`handler(request)` with one argument, and has been doing so in production;
`context` is only forwarded to the inner handler, where routes that do not read
it get `undefined`. Marking it `context?: any` states what was already true.

208 -> 203, zero new, on a line-number-agnostic diff of the full tsc error set.
One character; nothing executable changed.

No test added: the one-argument call path is the existing behaviour and is
already covered — embeddings-auth.test.ts and embeddings-route-apikeymeta-6929
call `POST(req)` directly through withInjectionGuard, which is exactly the arity
this now permits. 370/370 across the 43 injection-guard / batch / embeddings /
moderation suites; typecheck:core, eslint and check:file-size clean.

Co-authored-by: backryun <busan011@ormbiz.co.kr>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…nal (cheaper-inference#8644)

All five diagnostics in src/lib/batches/dispatch.ts are the same:

  Type '(request: any, context: any) => Promise<any>' is not assignable to
  type 'BatchRouteHandler'.
    Target signature provides too few arguments. Expected 2 or more, but got 1.

`withInjectionGuard()` returns `guardedHandler(request, context: any)` with the
second parameter required, so every route it wraps advertises arity 2. The batch
dispatcher's `BatchRouteHandler` is `(request: Request) => …`, and TS rejects
assigning a function that needs an argument the caller will never supply.

The declaration was wrong about its own runtime. `dispatch.ts:48` already calls
`handler(request)` with one argument, and has been doing so in production;
`context` is only forwarded to the inner handler, where routes that do not read
it get `undefined`. Marking it `context?: any` states what was already true.

208 -> 203, zero new, on a line-number-agnostic diff of the full tsc error set.
One character; nothing executable changed.

No test added: the one-argument call path is the existing behaviour and is
already covered — embeddings-auth.test.ts and embeddings-route-apikeymeta-6929
call `POST(req)` directly through withInjectionGuard, which is exactly the arity
this now permits. 370/370 across the 43 injection-guard / batch / embeddings /
moderation suites; typecheck:core, eslint and check:file-size clean.

Co-authored-by: backryun <busan011@ormbiz.co.kr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants