Skip to content

fix(api): reconnect the /v1/models background-refresh scheduler (#11551) - #11574

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
ntdatt812:fix/11551-models-after-scheduler
Aug 26, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
ntdatt812:fix/11551-models-after-scheduler

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

Fixes #11551.

The dead wire

src/app/api/v1/models/route.ts passes a third argument:

return getUnifiedModelsResponse(request, {}, { scheduleBackgroundRefresh: (task) => after(task) });

getUnifiedModelsResponse (catalog.ts:159) declares two parameters. The object was
silently dropped, and catalogCache.ts never imported after — it kept scheduling the
stale-while-revalidate rebuild with setTimeout(…, 0).

Confirming the issue's open question about why tsc did not flag the extra argument:
tsconfig.typecheck-core.json is an explicit 27-entry files allowlist with
"include": [], and src/app/api/v1/models/route.ts is not in it. Adding it is not a
one-liner — it pulls in catalog.ts, which currently emits ~60 pre-existing
strictNullChecks errors — so I left the tsconfig alone; it deserves its own issue.

Why setTimeout(…, 0) is not equivalent

The builder is overwhelmingly synchronous under the single-threaded App Router. A
macrotask deferral yields the tick but does not wait for the response to be flushed,
so the rebuild pins the event loop while the "served immediately" stale body is still
sitting in the socket — the caller waits out the whole rebuild it was supposed to be
spared. That is precisely the ~50 s-per-call symptom the CATALOG_CACHE_TTL_MS_DEFAULT
doc comment already describes. after() runs the task only once the response has been
flushed, which is the #8728 guarantee.

The fix

Both halves of the issue's option list, because the tests require both:

  • catalogCache.ts imports after and schedules through
    defaultBackgroundRefreshScheduler. after() throws outside a Next request scope
    (the CLI/Electron server, unit tests), so that case falls back to the macrotask —
    those callers have no response being flushed, so the deferral is all they ever needed.
  • resolveCachedCatalogResponse takes the scheduler as a per-call option, plus a
    per-call stale-window override the integration test uses to hold an entry in the stale
    branch while it measures when the rebuild starts.
  • getUnifiedModelsResponse takes the options object again and forwards it, so the
    route's after() wiring is live rather than dead.

This does not resurrect #9199

production-build-module-integrity.test.ts guards that the module-level SWR policy
accessor trio stays removed, because an unbounded window could pin an old catalog
forever. The new knob is a per-call parameter: no setter, no module state, and no
production caller passes it
— catalog.ts forwards only scheduleBackgroundRefresh.
The 30 s bound still holds for every real request. That guard test passes unchanged.

Tests

Both target tests pass without being edited, as the issue requires:

✔ the /v1/models route wires Next after() as its response-flush-safe scheduler

The second test binds a unix-domain socket, which Windows refuses with EACCES
(the test's own sandbox guard only skips on EPERM), so I could not run it as-is on
this host. I re-ran it verbatim over a TCP loopback port instead — same server, same
assertions, only listen() changed — and it is red/green on this change:

# with catalogCache.ts + catalog.ts reverted
✖ an external client receives the stale body before synchronous refresh finishes blocking (TCP)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal
ℹ pass 0  ℹ fail 1

# with the fix
✔ an external client receives the stale body before synchronous refresh finishes blocking (TCP)
ℹ pass 1  ℹ fail 0

Worth a follow-up: widening that test's sandbox guard from EPERM to also cover
EACCES would let it skip cleanly on Windows instead of failing, which matters for the
nightly Windows compat runs. I did not touch the file here since the issue asks for it
to stay unmodified.

Regression suites, all green:

production-build-module-integrity   (incl. the #9199 guard and the #9199-residue SWR test)
model-catalog-cache-swr-8728        8/8 cases
v1-models-catalog-ttl
v1-models-catalog-generation-race
model-catalog-policy-invalidation-8728
models-catalog-route
catalog-cache-auth-fingerprint
10313-catalog-cache-key-hashing
instrumentation-warm-catalog-cache

(model-catalog-cache-swr-8728 reports one extra failure on this Windows host, in its
test.after rmSync of the tmp DATA_DIR — EPERM on the still-open SQLite file.
Identical on the unmodified base branch; unrelated to this change.)

Commands run

node --import tsx/esm --import ./open-sse/utils/setupPolyfill.ts \
     --import ./tests/_setup/isolateDataDir.ts --test --test-force-exit \
     tests/integration/v1-models-swr-response-flush-8728.test.ts

node … --test-concurrency=4 tests/unit/production-build-module-integrity.test.ts \
     tests/unit/v1-models-catalog-ttl.test.ts \
     tests/unit/v1-models-catalog-generation-race.test.ts \
     tests/unit/model-catalog-cache-swr-8728.test.ts \
     tests/unit/models-catalog-route.test.ts \
     tests/unit/model-catalog-policy-invalidation-8728.test.ts \
     tests/unit/catalog-cache-auth-fingerprint.test.ts \
     tests/unit/10313-catalog-cache-key-hashing.test.ts \
     tests/unit/instrumentation-warm-catalog-cache.test.ts

npm run lint        # exit 0
npx prettier --check src/app/api/v1/models/{catalog,catalogCache}.ts   # clean

Changed test files: none — this PR turns two existing red tests green.

…osouzapw#11551)

`route.ts` has been passing a third argument since diegosouzapw#10198:

    getUnifiedModelsResponse(request, {}, { scheduleBackgroundRefresh: (task) => after(task) })

but diegosouzapw#9199 removed the injection point, leaving `getUnifiedModelsResponse` with two
parameters. The object was silently dropped, and `catalogCache.ts` kept scheduling
the stale-while-revalidate rebuild with `setTimeout(…, 0)`.

That is not equivalent. The builder is overwhelmingly synchronous under the
single-threaded App Router, so a rebuild that starts one macrotask later still pins
the event loop before the stale response has been flushed — the client waits out the
whole rebuild it was supposed to be spared. Next's `after()` runs the task only after
the flush, which is the guarantee diegosouzapw#8728 specified and the two integration tests in
`tests/integration/v1-models-swr-response-flush-8728.test.ts` describe.

- `catalogCache.ts` imports `after` and schedules through
  `defaultBackgroundRefreshScheduler`, falling back to the macrotask outside a Next
  request scope (CLI/Electron server, unit tests) where there is no response to flush.
- `resolveCachedCatalogResponse` accepts the scheduler as a per-call option, alongside
  a per-call stale-window override for tests. Neither is the module-level policy
  accessor diegosouzapw#9199 removed: no setter, no module state, no production caller — the 30 s
  bound still holds for every real request.
- `getUnifiedModelsResponse` takes the options object again and forwards it, so the
  route's `after()` wiring is live instead of dead.

No behavior change for callers that pass nothing; both integration tests go green
without being edited.
@diegosouzapw
diegosouzapw merged commit b61a530 into diegosouzapw:release/v3.8.51 Aug 26, 2026
10 of 16 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…osouzapw#11551) (diegosouzapw#11574)

Merged via /merge-batch (lote 2026-08-26, v3.8.51). Boarded no worktree combinado junto com outras ~30 PRs; validação única: typecheck/complexity/cognitive-complexity/changelog-integrity verdes, file-size rebaseado onde necessário (crescimento legítimo), lint com os mesmos 228 achados pré-existentes confirmados via sonda contra o tip puro (não introduzidos por este lote), e ~370 testes focados (unit + vitest) passando. Obrigado pela contribuição.
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.

fix(api): a fiação do after() em /v1/models está morta — refresh SWR nunca usa o scheduler injetado

2 participants