From 7db280ee643c772b3f89bb38e762f10bdc61060f Mon Sep 17 00:00:00 2001 From: Regis <92858615+Regis-RCR@users.noreply.github.com> Date: Sat, 14 Mar 2026 19:01:27 +0100 Subject: [PATCH] fix(api): address review feedback on pricing sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add .catch() to initial and periodic sync promises (Gemini, Kilo) - Wrap JSON.parse in try-catch for corrupted DB data (Kilo) - Wrap response.json() in try-catch for invalid LiteLLM JSON (Kilo) - Validate PRICING_SYNC_INTERVAL (guard against NaN/0 → tight loop) (Copilot) - Validate and allowlist sources — reject unknown, prevent empty sync from clearing pricing_synced data (Copilot, Kilo) - Extract merge loop into shared iteration to reduce duplication (Gemini) - Add data/warnings fields to MCP output schema (Copilot) - Remove unused z import in vitest (Copilot) - Filter non-string entries from sources array in API route (Copilot) - Track active interval for accurate getSyncStatus().nextSync (Copilot) --- .../mcp-server/__tests__/pricingSync.test.ts | 1 - open-sse/mcp-server/schemas/tools.ts | 2 + src/app/api/pricing/sync/route.ts | 4 +- src/lib/db/settings.ts | 33 ++---- src/lib/pricingSync.ts | 100 +++++++++++++----- 5 files changed, 91 insertions(+), 49 deletions(-) diff --git a/open-sse/mcp-server/__tests__/pricingSync.test.ts b/open-sse/mcp-server/__tests__/pricingSync.test.ts index e4696c8cd9..2cc90a1d13 100644 --- a/open-sse/mcp-server/__tests__/pricingSync.test.ts +++ b/open-sse/mcp-server/__tests__/pricingSync.test.ts @@ -1,5 +1,4 @@ import { describe, it, expect } from "vitest"; -import { z } from "zod"; import { syncPricingInput, syncPricingTool, MCP_TOOLS, MCP_TOOL_MAP } from "../schemas/tools.ts"; describe("omniroute_sync_pricing MCP tool schema", () => { diff --git a/open-sse/mcp-server/schemas/tools.ts b/open-sse/mcp-server/schemas/tools.ts index 303e01ed4c..eaa010e13c 100644 --- a/open-sse/mcp-server/schemas/tools.ts +++ b/open-sse/mcp-server/schemas/tools.ts @@ -742,6 +742,8 @@ export const syncPricingOutput = z.object({ source: z.string(), dryRun: z.boolean(), error: z.string().optional(), + warnings: z.array(z.string()).optional(), + data: z.record(z.string(), z.record(z.string(), z.unknown())).optional(), }); export const syncPricingTool: McpToolDefinition = diff --git a/src/app/api/pricing/sync/route.ts b/src/app/api/pricing/sync/route.ts index cdc7a16599..b987ce3c3f 100644 --- a/src/app/api/pricing/sync/route.ts +++ b/src/app/api/pricing/sync/route.ts @@ -11,7 +11,9 @@ import { NextRequest, NextResponse } from "next/server"; export async function POST(request: NextRequest) { try { const body = await request.json().catch(() => ({})); - const sources = Array.isArray(body.sources) ? body.sources : undefined; + const sources = Array.isArray(body.sources) + ? body.sources.filter((s: unknown): s is string => typeof s === "string") + : undefined; const dryRun = body.dryRun === true; const { syncPricingFromSources } = await import("@/lib/pricingSync"); diff --git a/src/lib/db/settings.ts b/src/lib/db/settings.ts index c68de5d71d..6ce27386af 100644 --- a/src/lib/db/settings.ts +++ b/src/lib/db/settings.ts @@ -131,28 +131,17 @@ export async function getPricing() { mergedPricing[provider] = { ...(toRecord(models) as PricingModels) }; } - // Layer synced on top of defaults - for (const [provider, models] of Object.entries(syncedPricing)) { - if (!mergedPricing[provider]) { - mergedPricing[provider] = { ...models }; - } else { - for (const [model, pricing] of Object.entries(models)) { - mergedPricing[provider][model] = mergedPricing[provider][model] - ? { ...(mergedPricing[provider][model] || {}), ...toRecord(pricing) } - : pricing; - } - } - } - - // Layer user overrides on top (highest priority) - for (const [provider, models] of Object.entries(userPricing)) { - if (!mergedPricing[provider]) { - mergedPricing[provider] = { ...models }; - } else { - for (const [model, pricing] of Object.entries(models)) { - mergedPricing[provider][model] = mergedPricing[provider][model] - ? { ...(mergedPricing[provider][model] || {}), ...toRecord(pricing) } - : pricing; + // Layer synced then user on top (each higher-priority layer overrides) + for (const layer of [syncedPricing, userPricing]) { + for (const [provider, models] of Object.entries(layer)) { + if (!mergedPricing[provider]) { + mergedPricing[provider] = { ...models }; + } else { + for (const [model, pricing] of Object.entries(models)) { + mergedPricing[provider][model] = mergedPricing[provider][model] + ? { ...(mergedPricing[provider][model] || {}), ...toRecord(pricing) } + : pricing; + } } } } diff --git a/src/lib/pricingSync.ts b/src/lib/pricingSync.ts index 9362a75842..f3836eee36 100644 --- a/src/lib/pricingSync.ts +++ b/src/lib/pricingSync.ts @@ -56,10 +56,16 @@ interface SyncResult { // ─── Configuration ─────────────────────────────────────── -const SYNC_INTERVAL_MS = parseInt(process.env.PRICING_SYNC_INTERVAL || "86400", 10) * 1000; +const SUPPORTED_SOURCES = ["litellm"] as const; +type SupportedSource = (typeof SUPPORTED_SOURCES)[number]; + +const parsedInterval = parseInt(process.env.PRICING_SYNC_INTERVAL || "86400", 10); +const SYNC_INTERVAL_MS = + Number.isFinite(parsedInterval) && parsedInterval > 0 ? parsedInterval * 1000 : 86400 * 1000; const SYNC_SOURCES = (process.env.PRICING_SYNC_SOURCES || "litellm") .split(",") - .map((s) => s.trim()); + .map((s) => s.trim()) + .filter((s): s is SupportedSource => SUPPORTED_SOURCES.includes(s as SupportedSource)); const LITELLM_PRICING_URL = "https://raw.githubusercontent.com/BerriAI/litellm/main/model_prices_and_context_window.json"; @@ -87,6 +93,7 @@ const LITELLM_PROVIDER_MAP: Record = { let syncTimer: ReturnType | null = null; let lastSyncTime: string | null = null; let lastSyncModelCount = 0; +let activeSyncIntervalMs = SYNC_INTERVAL_MS; // ─── Core: Fetch + Transform ───────────────────────────── @@ -100,7 +107,12 @@ export async function fetchLiteLLMPricing(): Promise>; + const text = await response.text(); + try { + return JSON.parse(text) as Record; + } catch { + throw new Error(`LiteLLM returned invalid JSON (${text.slice(0, 100)}...)`); + } } /** @@ -179,7 +191,11 @@ export function getSyncedPricing(): PricingByProvider { const key = typeof record.key === "string" ? record.key : null; const rawValue = typeof record.value === "string" ? record.value : null; if (!key || rawValue === null) continue; - synced[key] = JSON.parse(rawValue) as PricingModels; + try { + synced[key] = JSON.parse(rawValue) as PricingModels; + } catch { + console.warn(`[PRICING_SYNC] Corrupted data for provider "${key}", skipping`); + } } return synced; } @@ -223,17 +239,36 @@ export async function syncPricingFromSources(opts?: { sources?: string[]; dryRun?: boolean; }): Promise { - const sources = opts?.sources || SYNC_SOURCES; + const requestedSources = opts?.sources || SYNC_SOURCES; const dryRun = opts?.dryRun ?? false; - try { - let aggregated: PricingByProvider = {}; + // Validate sources + const validSources = requestedSources.filter((s): s is SupportedSource => + SUPPORTED_SOURCES.includes(s as SupportedSource) + ); + const invalidSources = requestedSources.filter( + (s) => !SUPPORTED_SOURCES.includes(s as SupportedSource) + ); - for (const source of sources) { + if (validSources.length === 0) { + const supported = SUPPORTED_SOURCES.join(", "); + return { + success: false, + modelCount: 0, + providerCount: 0, + source: requestedSources.join(","), + dryRun, + error: `No valid sources provided. Supported: ${supported}. Invalid: ${invalidSources.join(", ")}`, + }; + } + + try { + const aggregated: PricingByProvider = {}; + + for (const source of validSources) { if (source === "litellm") { const raw = await fetchLiteLLMPricing(); const transformed = transformToOmniRoute(raw); - // Merge into aggregated for (const [provider, models] of Object.entries(transformed)) { if (!aggregated[provider]) aggregated[provider] = {}; Object.assign(aggregated[provider], models); @@ -257,8 +292,11 @@ export async function syncPricingFromSources(opts?: { success: true, modelCount, providerCount, - source: sources.join(","), + source: validSources.join(","), dryRun, + ...(invalidSources.length > 0 + ? { warnings: [`Unknown sources ignored: ${invalidSources.join(", ")}`] } + : {}), ...(dryRun ? { data: aggregated } : {}), }; } catch (err) { @@ -268,7 +306,7 @@ export async function syncPricingFromSources(opts?: { success: false, modelCount: 0, providerCount: 0, - source: sources.join(","), + source: requestedSources.join(","), dryRun, error: message, }; @@ -284,23 +322,35 @@ export function startPeriodicSync(intervalMs?: number): void { if (syncTimer) return; // Already running const interval = intervalMs ?? SYNC_INTERVAL_MS; + activeSyncIntervalMs = interval; console.log(`[PRICING_SYNC] Starting periodic sync every ${interval / 1000}s`); // Initial sync (non-blocking) - syncPricingFromSources().then((result) => { - if (result.success) { - console.log( - `[PRICING_SYNC] Initial sync complete: ${result.modelCount} models from ${result.providerCount} providers` - ); - } - }); + syncPricingFromSources() + .then((result) => { + if (result.success) { + console.log( + `[PRICING_SYNC] Initial sync complete: ${result.modelCount} models from ${result.providerCount} providers` + ); + } + }) + .catch((err) => { + console.warn("[PRICING_SYNC] Initial sync error:", err instanceof Error ? err.message : err); + }); syncTimer = setInterval(() => { - syncPricingFromSources().then((result) => { - if (result.success) { - console.log(`[PRICING_SYNC] Periodic sync complete: ${result.modelCount} models`); - } - }); + syncPricingFromSources() + .then((result) => { + if (result.success) { + console.log(`[PRICING_SYNC] Periodic sync complete: ${result.modelCount} models`); + } + }) + .catch((err) => { + console.warn( + "[PRICING_SYNC] Periodic sync error:", + err instanceof Error ? err.message : err + ); + }); }, interval); } @@ -326,9 +376,9 @@ export function getSyncStatus(): SyncStatus { lastSyncModelCount, nextSync: syncTimer && lastSyncTime - ? new Date(new Date(lastSyncTime).getTime() + SYNC_INTERVAL_MS).toISOString() + ? new Date(new Date(lastSyncTime).getTime() + activeSyncIntervalMs).toISOString() : null, - intervalMs: SYNC_INTERVAL_MS, + intervalMs: activeSyncIntervalMs, sources: SYNC_SOURCES, }; }