fix(providers): lazily load chatgpt-web-codex admin helpers in PUT /api/providers/[id] - #13071
Conversation
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
|
Clean, well-scoped fix — mirrors #12355's exact pattern for the sibling route, and the |
…pi/providers/[id] diegosouzapw#12355 fixed a static top-level import of chatgptWebCodexAdmin.ts in src/app/api/providers/route.ts (the collection route): that module's transitive chain (the vendor ChatGPT-Web browser adapter -> token-estimate.ts -> tiktoken's WASM tokenizer) fails to bundle under Turbopack dev mode even with tiktoken listed in serverExternalPackages (the standalone Node require works fine; only Turbopack's bundling of this specific import graph doesn't). A static import evaluated that whole chain on every request regardless of provider, turning an unrelated provider's bundling bug into a route-wide 500 for everyone. diegosouzapw#12355 only touched the collection route and missed the identical pattern in the by-id route (PUT /api/providers/[id]) -- observed live: renaming a plain openai-compatible connection's name failed with "Missing tiktoken_bg.wasm" after 17-50s, having never touched chatgpt-web-codex at all. diegosouzapw#12355 also shipped with no regression test, which is exactly how this sibling instance went unnoticed for a week. Fix: same pattern as diegosouzapw#12355 -- move the import into a lazy `await import(...)` inside the one `provider === "chatgpt-web-codex"` branch that actually needs it. New regression test (tests/unit/providers-chatgpt-web-codex-lazy-import-12355.test.ts) source-inspects both general provider routes (route.ts and [id]/route.ts) to assert neither statically imports chatgptWebCodexAdmin.ts, and confirms the one legitimately-static import (the chatgpt-web-codex-only "doctor" diagnostic route, where every request already is that provider) is left alone. Matches the existing tests/unit/instrumentation-import-graph-12074.test.ts pattern for import-graph invariants that can't be exercised by actually bundling under Turbopack from this harness. Confirmed it fails against the pre-fix route.ts (static import present) and passes after the fix. Evidence: - node --test tests/unit/providers-chatgpt-web-codex-lazy-import-12355.test.ts: 3/3 pass - node --test tests/unit/providers-route-patch-method.test.ts tests/unit/codex-connection-edit-6562.test.ts tests/unit/chatgpt-web-management-retirement.test.ts tests/unit/provider-patch-ratelimit-protection-11278.test.ts: 11/11 pass (unaffected) - eslint on both touched files: clean - Live-verified against the exact failing request: PUT /api/providers/{id} renaming a llama-cpp connection, previously 500 after 17-50s with "Missing tiktoken_bg.wasm", now 200 in 0.83s. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
747a6d6 to
ff54221
Compare
|
Checked, and it's actually branch staleness rather than pure base-red: this branch was 217 commits behind |
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
…codex admin helpers in PUT /api/providers/[id]) into dev/omniroute-dev-combined
4591476
into
diegosouzapw:release/v3.8.51
…pi/providers/[id] (diegosouzapw#13071) Merged. Loading the chatgpt-web-codex admin helpers lazily keeps a rarely used path off `PUT /api/providers/[id]`'s cost — the kind of change that only shows up as latency nobody can explain. Validated as a combined board first (this PR merged with the 21 siblings of the same wave on the release tip): eslint with the frozen suppressions, typecheck:core, check:open-sse-typecheck, complexity, cognitive-complexity, changelog-integrity, i18n new-key coverage, docs-counts, docs-sync, migration-numbering, provider-consistency and a duplicate-identifier audit all green, plus 176 passing / 0 failing focused node:test cases across the 25 test files the wave touches and the dashboard test under Vitest (2/0). Then re-validated alone on the fresh tip before this merge: conflicts re-resolved, file sizes rebaselined for this PR's own growth, eslint and this PR's focused tests re-run. Thank you.
What Problem This Solves
PUT /api/providers/{id}-- renaming a connection, rotating its API key,etc. -- for any provider returned a 500 after 17-50 seconds:
Live repro: PUT-ing a rename on a plain
llama-cpp(openai-compatible)connection -- nothing to do with
chatgpt-web-codexat all.Root Cause
#12355 already fixed this exact bug class in the sibling collection
route (
src/app/api/providers/route.ts): a static top-level import ofchatgptWebCodexAdmin.tsevaluates that module's transitive chain (thevendor ChatGPT-Web browser adapter ->
token-estimate.ts-> tiktoken's WASMtokenizer) on every request to the route, regardless of which provider is
actually being managed. That chain fails to bundle under Turbopack dev mode
even with
tiktokenlisted inserverExternalPackages(the standalone Noderequireworks fine; only Turbopack's bundling of this specific importgraph doesn't).
#12355 only touched the collection route and missed the identical
pattern in the by-id route (
src/app/api/providers/[id]/route.ts),which has the same static import used only inside one
provider === "chatgpt-web-codex"branch of thePUThandler. #12355 alsoshipped with no regression test, which is exactly how this sibling instance
went unnoticed for a week.
Fix
Same pattern as #12355: move the import into a lazy
await import(...)inside the one branch that actually needs it.
What This Deliberately Does NOT Change
src/app/api/providers/[id]/chatgpt-web-codex-doctor/route.tskeeps itsstatic import -- every request to that route already is for a
chatgpt-web-codexconnection (it's in the route's own URL), so there's no"unrelated provider pays the cost" problem there.
Evidence
New regression test source-inspects both general provider routes to assert
neither statically imports
chatgptWebCodexAdmin.ts, and confirms thedoctor route's static import is intentionally left alone. Matches the
existing
tests/unit/instrumentation-import-graph-12074.test.tspattern forimport-graph invariants that can't be exercised by actually bundling under
Turbopack from this harness. Confirmed it fails against the pre-fix
[id]/route.tsand passes after the fix.eslinton both touched files: cleanLive-verified against the exact failing request (deployed to my own dev
instance before opening this PR, given it was actively blocking a user):