Skip to content

refactor: modularize 4.2K-line validation.ts (Issue #3594) - #3642

Closed
oyi77 wants to merge 53 commits into
cheaper-inference:release/v3.8.24from
oyi77:refactor/modularize-validation
Closed

oyi77 wants to merge 53 commits into
cheaper-inference:release/v3.8.24from
oyi77:refactor/modularize-validation

Conversation

@oyi77

@oyi77 oyi77 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR addresses another core backend massive file from Issue #3594: src/lib/providers/validation.ts at 4,200 lines.

Changes

Splits the monolithic provider validation service into 7 domain-focused files inside src/lib/providers/validation/:

File Contents
utils.ts URL normalizers, custom headers generation, safe outbound fetch wrappers
enterprise.ts AWS, Azure, GCP, OCI, SAP, Heroku, Databricks, Snowflake, DataRobot
standard.ts OpenAI-compatible, Anthropic-compatible, Gemini-compatible APIs
specialty.ts Voice, Video, specialized text providers (Deepgram, AssemblyAI, Gigachat, etc.)
search.ts Generic search validators and configuration constants
web.ts Meta AI base62 helpers and web/cookie-auth providers (DeepSeekWeb, GrokWeb, Copilot, etc.)
core.ts The main validateProviderApiKey switch dispatcher
index.ts Re-exports everything cleanly

Results

  • validation.ts: 4,200 lines → 1 line (re-exports validation/index.ts)
  • All imports between files are resolved correctly.
  • Full backward compatibility: all exports are preserved through the facade.

Verification

  • npm run typecheck:core — passes cleanly.
  • npx eslint src/lib/providers/validation/*.ts — 0 errors or warnings.
  • Strictly structural refactoring with identical runtime behavior.

Resolves part of #3594.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modularizes the chatCore handler and combo services, implements legacy text-completion transformations for /v1/completions, adds missing API endpoints for gamification and agent-bridge testing, and introduces comprehensive UI panels for provider and model management. The code review identified critical security vulnerabilities, including a lack of authentication on the new upstream CA test route (exposing a local file probing risk) and prototype pollution risks in dynamic validator lookups. Additionally, the feedback highlights potential runtime crashes due to unsafe request body property access in combo handlers, a Next.js hydration mismatch from rendering dynamic dates, and a compatibility recommendation to explicitly import crypto.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
import { createErrorResponse } from "@/lib/api/errorResponse";

export async function POST(request: Request): Promise<Response> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

This route lacks any authentication or authorization checks, exposing it to potential unauthorized access. Since it accepts an arbitrary file path (caPath) and checks for its existence/readability, an attacker could exploit this to probe the existence of sensitive files on the local filesystem (Local File Inclusion / Information Disclosure). Please add the standard requireManagementAuth check at the beginning of the handler and import it from @/lib/api/requireManagementAuth.

export async function POST(request: Request): Promise<Response> {
  const authError = await requireManagementAuth(request);
  if (authError) return authError;

const expiresMs = new Date(effectiveExpiresAt).getTime();
return Math.floor((expiresMs - Date.now()) / 60000);
};
const [tokenMinsLeft, setTokenMinsLeft] = useState<number | null>(getTokenMinsLeft);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Initializing state with a value derived from Date.now() during render can cause a hydration mismatch in Next.js, as the server-rendered time will differ from the client-side hydration time. Initialize tokenMinsLeft to null instead. The useEffect hook on mount will immediately trigger update() and set the correct client-side value.

Suggested change
const [tokenMinsLeft, setTokenMinsLeft] = useState<number | null>(getTokenMinsLeft);
const [tokenMinsLeft, setTokenMinsLeft] = useState<number | null>(null);

Comment thread src/lib/providers/validation/web.ts Outdated
@@ -0,0 +1,1235 @@
import { randomUUID } from "node:crypto";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The global crypto object is used throughout this file (e.g., crypto.getRandomValues, crypto.randomUUID). While globalThis.crypto is available in newer Node.js versions, explicitly importing crypto from node:crypto ensures compatibility with older environments and prevents potential ReferenceError or linter/bundler warnings.

import crypto, { randomUUID } from "node:crypto";

Comment thread src/lib/providers/validation/core.ts Outdated
),
};

if (SPECIALTY_VALIDATORS[provider]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

Accessing dynamic keys directly on a plain object (e.g., SPECIALTY_VALIDATORS[provider]) poses a prototype pollution or property injection risk if provider matches built-in properties like toString or hasOwnProperty. Use Object.prototype.hasOwnProperty.call to safely verify that the property is an own property of the object before accessing or calling it.

Suggested change
if (SPECIALTY_VALIDATORS[provider]) {
if (Object.prototype.hasOwnProperty.call(SPECIALTY_VALIDATORS, provider)) {

Comment thread src/lib/providers/validation/search.ts Outdated
provider: string,
isLocal: boolean = false
) {
const config = SEARCH_VALIDATOR_CONFIGS[provider];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

Accessing dynamic keys directly on a plain object (e.g., SEARCH_VALIDATOR_CONFIGS[provider]) poses a prototype pollution or property injection risk if provider matches built-in properties like toString or hasOwnProperty. Use Object.prototype.hasOwnProperty.call to safely verify that the property is an own property of the object before accessing it.

  const config = Object.prototype.hasOwnProperty.call(SEARCH_VALIDATOR_CONFIGS, provider)
    ? SEARCH_VALIDATOR_CONFIGS[provider]
    : null;

Comment thread open-sse/services/combo/roundRobin.ts Outdated
// model's real limit, triggering 400s).
let attemptBody = body;
if (supportsReasoning(modelStr)) {
const currentMaxTokens = Number((body as Record<string, unknown>).max_tokens) || 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If body is null or undefined at runtime, casting it to Record<string, unknown> and accessing .max_tokens will throw a TypeError. Use optional chaining (body?.max_tokens) to safely handle potentially null or undefined request bodies.

Suggested change
const currentMaxTokens = Number((body as Record<string, unknown>).max_tokens) || 0;
const currentMaxTokens = Number(body?.max_tokens) || 0;

}

// Issue #3587: Reasoning models (deepseek-v4-flash, nemotron, etc.) consume
// ALL max_tokens for reasoning_tokens, leaving content empty. Add a buffer

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If attemptBody is null or undefined at runtime, casting it to Record<string, unknown> and accessing .max_tokens will throw a TypeError. Use optional chaining (attemptBody?.max_tokens) to safely handle potentially null or undefined request bodies.

Suggested change
// ALL max_tokens for reasoning_tokens, leaving content empty. Add a buffer
const currentMaxTokens = Number(attemptBody?.max_tokens) || 0;

@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.23 June 12, 2026 01:19
zhiru and others added 24 commits June 12, 2026 02:40
…API (cheaper-inference#3712)

Integrated into release/v3.8.23. Vertex dynamic model discovery — surfaces image models (imagen-*, gemini-*-image), embeddings and audio from the live Generative Language catalog, with cached→static fallback and the shared parseGeminiModelsList helper. Validated: parser test 5/5, typecheck:core clean.
Integrated into release/v3.8.23. Makes the cheaper-inference#3588 reasoning token buffer safe and configurable: only inflates max_tokens when the model has a known, non-default output cap and the buffered value fits inside it; otherwise preserves/clamps the client limit. Adds the reasoningTokenBufferEnabled kill switch (default ON). Validated: combo-routing-engine 81/81, combo-config 25/25, combo-quality-validator-reasoning 12/12, phase1f 10/10, typecheck:core clean.
…4062→3408 LOC (-654) (cheaper-inference#3717)

Phase 1g-1j of cheaper-inference#3501: client 4062→3408 LOC. Pure extraction (ProviderPlaygroundPanel, useCommandCodeAuth, useExternalLinkFlow+ExternalLinkModal, useAuthFileHandlers) + loadConnProxies ReferenceError fix + phase1f test path fix.

Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…3408→2553 LOC (-855) (cheaper-inference#3721)

Phase 1k-1m of cheaper-inference#3501: client 3408→2553 LOC. Pure extraction (useModelImportHandlers+ImportProgressModal, useModelVisibilityHandlers, ProviderModelsSection).

Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
….8.20 release branch

The fix itself reached main pre-tag via cherry-pick cheaper-inference#3591, but its changelog
bullet (commit e33fdd4) only ever existed on release/v3.8.20 after the
squash-merge. Restored under [3.8.20] per the 2026-06-12 release-branch
leftover audit (_tasks/release-audit/release-leftovers-audit-2026-06-12.md).
…2553→1376 LOC (-1177) (cheaper-inference#3725)

Phase 1n-1s of cheaper-inference#3501: client 2553→1376 LOC. Pure extraction (ConnectionsListPanel, ConnectionsHeaderToolbar, ZedImportCard, BatchTestResultsModal, AdaptaTutorialModal, useApiKeySave + helpers).

Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…cation, and success-decay recovery (cheaper-inference#3629)

Integrated into release/v3.8.23
…6→781 LOC (≤800 TARGET REACHED ✅) (cheaper-inference#3727)

Phase 1t of cheaper-inference#3501: client 1376→781 LOC (≤800 reached). Original god-component 12,882→781 (−94%).

Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
… every chatHelpers import

cheaper-inference#3692 added a lazy 'await import(proxyEgress)' for egress-IP visibility inside
safeLogEvents, which is a sync function — an ES syntax error. It went unnoticed
because typecheck:core does not cover src/sse and no test in the merge gates
loaded chatHelpers via tsx; any consumer that did (chat-context-relay and
chat-route-coverage suites, integration harnesses) failed at module load with
'await can only be used inside an async function'.

safeLogEvents is fire-and-forget logging with an outer try/catch, so making it
async (and 'void'-ing the single chat.ts call site) preserves behavior exactly.

Validation: tests/unit/chat-context-relay.test.ts + chat-route-coverage.test.ts
went from failing-at-load to green (+14 tests destravados).
diegosouzapw and others added 24 commits June 13, 2026 02:18
…ce#3598)

Integrated into release/v3.8.21 — chatCore phase modularization. Adjusted: re-derive idempotencyKey for the save path after the check moved into the module (co-authored). Thanks @oyi77!
…on) + cheaper-inference#3588 (combo reasoning buffer)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…drop shadow/guardrails doc-fiction (cheaper-inference#3496) (cheaper-inference#3602)

Integrated into release/v3.8.21 — implements GET /api/guardrails + POST /api/guardrails/test, removes shadow/guardrails doc-fiction. TDD-validated (5/5) + check-docs-symbols/typecheck/eslint green.
Split-out PR C from cheaper-inference#3584. Isolates textual reasoning wrappers (<think>/<thinking>/<thought>/<internal_thought>, including malformed/open tags) into reasoning_content across both the non-streaming sanitizer and the Gemini streaming translator, with split-chunk buffering. Additive to the existing textual tool-call pipeline; does not touch the cheaper-inference#3569 native functionResponse path. Integrated into release/v3.8.21. Thanks @dhaern!
…nce#3603)

Split-out PR A from cheaper-inference#3584. Normalizes the Antigravity/agy Gemini 3.5 Flash tier IDs to clean public names (gemini-3.5-flash-low/medium/high), maps them to the live upstream IDs at the executor boundary, and removes Antigravity from the global model resolver so the executor owns wire normalization. Maintainer follow-up: kept gemini-3.5-flash-preview as a hidden backward-compat alias routing to the High tier (so saved combos/configs keep working). Live-validated the tier set via the agy CLI catalog. Integrated into release/v3.8.21. Thanks @dhaern!
…ference#3604)

Split-out PR B from cheaper-inference#3584. Normalizes Antigravity/agy provider quotas: prefers retrieveUserQuota for live consumption, falls back to fetchAvailableModels and local usage_history, sanitizes cached Provider Limits so retired upstream IDs are not re-exposed, and schedules a deduplicated post-usage refresh. Maintainer follow-up: decoupled the post-usage refresh via a lightweight usageEvents bus (usageHistory no longer dynamic-imports providerLimits) so it does not pull the executors/translator graph into the typecheck-core surface — typecheck:core stays at 0. Integrated into release/v3.8.21. Thanks @dhaern!
…aper-inference#3604 (provider quotas) + cheaper-inference#3605 (reasoning wrappers)

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…nference#3594)

Breaks the 12,882-line providers/[id]/page.tsx into 6 domain-focused files:

- types.ts — all shared interfaces/types
- utils.tsx — helper functions and utility components (ModelSourceBadge, CooldownTimer, ModelCompatPopover, etc.)
- modelManagement.tsx — ModelRow, ModelVisibilityToolbar, PassthroughModelsSection, CustomModelsSection, CompatibleModelsSection
- connectionManagement.tsx — ConnectionRow, EditConnectionModal, EditCompatibleNodeModal
- authModals.tsx — AddApiKeyModal, SiliconFlowEndpointModal, Import/Apply (Codex/Claude/Gemini) auth modals
- playground.tsx — ProviderPlaygroundPanel, renderKindPanel

page.tsx reduced from 12,882 → 4,687 lines (63% reduction).
TypeScript typecheck passes cleanly. ESLint: 0 errors, only 3 pre-existing warnings.
Pre-parse and de-duplicate all imports per file to eliminate unused imports.

Resolves part of cheaper-inference#3594.
- Replaced Buffer with browser-safe atob in utils.tsx
- Fixed memory leaks/infinite intervals in cooldown timers
- Typed chatCore handlers with 'any' instead of 'unknown' to fix strict-mode property access
- Fixed single-model reasoning payload mutation in combo.ts
…nce#3594)

- Splits the 4,600-line combo.ts monolithic file into 12 domain-specific files
- Extracts Auto-Combo, Round-Robin, Quota tracking, Shadow routing, and Context Affinity logic into dedicated modules
- Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages
- Auto-prunes and manages all imports for strict typechecking
…#3594)

- Splits the monolithic provider validation file into 7 domain-specific files
- Extracts Enterprise, Web, Search, Standard, Specialty, and Utils into dedicated modules
- Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages
- Auto-prunes and manages all imports for strict typechecking
- use node:crypto explicit import
- prevent prototype pollution in dynamic validator lookups
- safely access max_tokens with optional chaining in combo handlers
@oyi77
oyi77 force-pushed the refactor/modularize-validation branch from a7a5dbd to b95e9de Compare June 12, 2026 19:18
@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.23 to release/v3.8.24 June 13, 2026 02:52
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @oyi77 for the modularization work on src/lib/providers/validation.ts (Issue #3594). I can't merge this as it currently stands — and the reason is structural, not about the quality of the split:

  1. It's a stacked/cumulative chain. This branch carries ~49–55 commits all starting from chore(release): open v3.8.23 development cycle, and the per-PR additions grow monotonically (refactor: modularize providerRegistry.ts (Issue #3594) #3617 ≈ +22k → refactor: modularize 3.1K-line providers.ts constants (Issue #3594) #3646 ≈ +52k). Each refactor branch is built on top of the previous one, so merging any single PR drags in all the earlier refactors too. That makes each PR impossible to review in isolation and guarantees conflicts between them.

  2. The base is stale (v3.8.23 era). Since these were cut, release/v3.8.24 has merged dozens of PRs — including changes to the very files several of these PRs modularize (e.g. providerRegistry.ts grew via fix(providers): correct Ollama Cloud kimi-k2.7-code capabilities (#3761) #3768, validation.ts via fix(providers): repair qwen-web validation + stop mislabeling benign redirects as SSRF (#3288, #3758) #3767, plus combo/stream/usage edits). GitHub now reports this PR as CONFLICTING, and the conflicts are substantial.

What would let this land: re-cut it as a single, independent PR branched from the current release/v3.8.24 tip, modularizing only src/lib/providers/validation.ts (not stacked on the other refactors), small enough to review on its own. One file per PR, rebased on current release. I'll gladly review and merge them in that form — the god-file modularization goal is worth it; it's the 14-deep stack on a 2-week-old base that can't go in. Leaving this open so nothing is lost.

@oyi77

oyi77 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

Closed in favor of clean re-cut — see #3793 (providerRegistry), #3798 (combo), #3797 (validation), #3796 (imageGeneration), #3795 (usage), #3794 (providers). Only modularization commits were cherry-picked onto current release/v3.8.24 — no stacked dependencies, no unrelated changes.

@oyi77 oyi77 closed this Jun 13, 2026
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.