diff --git a/CHANGELOG.md b/CHANGELOG.md index 8486f2b196..0a5fddee1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral - **fix(providers):** stop Antigravity connections from falsely reporting **all-accounts quota-exhausted** ([#6295](https://github.com/diegosouzapw/OmniRoute/issues/6295)) — `genericQuotaFetcher.ts::percentUsedForQuota()` ignored the `fractionReported` flag and defaulted an unreported model's `remainingPercentage` to 0, which computed as 100% used; since `convertUsageToQuotaInfo()` takes the worst-case window across a connection, a single model with no reported fraction dragged the whole account into `limitReached` and `quotaPreflight` skipped it. `percentUsedForQuota()` now returns `null` (unknown, window ignored) whenever `fractionReported === false`, before falling back to `remainingPercentage`. Regression guard: `tests/unit/generic-quota-fetcher.test.ts`. - **fix(providers):** grok-cli (Grok Build) now strips `reasoning_effort`/`reasoning` before forwarding the request ([#6288](https://github.com/diegosouzapw/OmniRoute/issues/6288)) — Claude Code sends `reasoning_effort` on every request (routing the Opus slot), which Grok Build's upstream chat-proxy endpoint rejects with a 400; `transformRequest()`'s existing `UNSUPPORTED` sampling-param strip list (#5273) never covered it. Regression guard: `tests/unit/grok-cli-reasoning-strip-6288.test.ts`. - **fix(cli):** `omniroute serve` no longer hangs silently on a readiness timeout ([#6321](https://github.com/diegosouzapw/OmniRoute/issues/6321)) — the child server's stdout was piped to `"ignore"` whenever `--log`/`OMNIROUTE_SHOW_LOG` wasn't set (the default), discarding any debug output, and `runWithSupervisor`'s `waitForServer(...).then((up) => { if (up) {...} })` had no `else` branch, so a boot that never became ready produced zero further output after "⏳ Starting server...". Stdout is now buffered alongside stderr (`ServerSupervisor.getRecentLog()`), and a timeout prints a clear diagnostic plus the buffered output instead of staying silent. Does not by itself explain why boot never completes on a given machine — see the issue for further reproduction. Regression guard: `tests/unit/cli-serve-readiness-timeout-6321.test.ts`. +- **fix(pricing):** Pricing Sync dashboard no longer stuck on "Next Sync: Never" / "Synced Models: 0" ([#6325](https://github.com/diegosouzapw/OmniRoute/issues/6325)) — `pricingSync.ts` kept sync state (`lastSyncTime`, `lastSyncModelCount`) in module-level vars, but the background periodic sync (`instrumentation-node.ts`) and the dashboard status route (`/api/pricing/sync`) each import the module from separate Next.js standalone webpack chunks, giving each its own independent state; `getSyncStatus()` read the (empty) API-route instance's vars. Sync status is now additionally persisted to a new `pricing_sync_status` `key_value` namespace and `getSyncStatus()` falls back to it when the local module instance never ran a sync itself. Regression guard: `tests/unit/pricing-sync-cross-instance.test.ts`. ### 📝 Maintenance diff --git a/src/lib/pricingSync.ts b/src/lib/pricingSync.ts index 4fbd1b34ef..886c387aed 100644 --- a/src/lib/pricingSync.ts +++ b/src/lib/pricingSync.ts @@ -285,6 +285,51 @@ export function clearSyncedPricing(): void { invalidateDbCache("pricing"); } +// ─── DB: Sync status namespace ─────────────────────────── +// +// Persisted separately from `pricing_synced` because Next.js standalone +// builds load this module from independent webpack chunks (e.g. the +// instrumentation hook vs an API route handler) — each gets its OWN +// top-level module state. Module-level vars (`lastSyncTime`, +// `lastSyncModelCount`) are therefore invisible across those instances; +// persisting them to the DB lets any instance read the real status. + +const SYNC_STATUS_NAMESPACE = "pricing_sync_status"; +const SYNC_STATUS_KEY = "last_sync"; + +function readPersistedSyncStatus(): { lastSyncTime: string; lastSyncModelCount: number } | null { + const db = getDbInstance(); + const row = db + .prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?") + .get(SYNC_STATUS_NAMESPACE, SYNC_STATUS_KEY); + const record = toRecord(row); + const rawValue = typeof record.value === "string" ? record.value : null; + if (!rawValue) return null; + try { + const parsed = JSON.parse(rawValue) as { lastSyncTime?: string; lastSyncModelCount?: number }; + if (typeof parsed.lastSyncTime !== "string") return null; + return { + lastSyncTime: parsed.lastSyncTime, + lastSyncModelCount: + typeof parsed.lastSyncModelCount === "number" ? parsed.lastSyncModelCount : 0, + }; + } catch { + return null; + } +} + +function writePersistedSyncStatus(lastSync: string, modelCount: number): void { + const db = getDbInstance(); + db.prepare( + "INSERT INTO key_value (namespace, key, value) VALUES (?, ?, ?) " + + "ON CONFLICT(namespace, key) DO UPDATE SET value = excluded.value" + ).run( + SYNC_STATUS_NAMESPACE, + SYNC_STATUS_KEY, + JSON.stringify({ lastSyncTime: lastSync, lastSyncModelCount: modelCount }) + ); +} + // ─── Main sync function ───────────────────────────────── /** @@ -341,6 +386,7 @@ export async function syncPricingFromSources(opts?: { saveSyncedPricing(aggregated); lastSyncTime = new Date().toISOString(); lastSyncModelCount = modelCount; + writePersistedSyncStatus(lastSyncTime, modelCount); } return { @@ -429,14 +475,21 @@ export function stopPeriodicSync(): void { */ export function getSyncStatus(): SyncStatus { const enabled = process.env.PRICING_SYNC_ENABLED === "true"; + // `lastSyncTime`/`lastSyncModelCount` are only reliably populated on the + // module instance that performed the sync (see note above + // writePersistedSyncStatus) — fall back to the persisted DB record so + // status reads from a different module instance still see it. + const persisted = lastSyncTime === null ? readPersistedSyncStatus() : null; + const effectiveLastSync = lastSyncTime ?? persisted?.lastSyncTime ?? null; + const effectiveModelCount = + lastSyncTime !== null ? lastSyncModelCount : (persisted?.lastSyncModelCount ?? 0); return { enabled, - lastSync: lastSyncTime, - lastSyncModelCount, - nextSync: - syncTimer && lastSyncTime - ? new Date(new Date(lastSyncTime).getTime() + activeSyncIntervalMs).toISOString() - : null, + lastSync: effectiveLastSync, + lastSyncModelCount: effectiveModelCount, + nextSync: effectiveLastSync + ? new Date(new Date(effectiveLastSync).getTime() + activeSyncIntervalMs).toISOString() + : null, intervalMs: activeSyncIntervalMs, sources: SYNC_SOURCES, }; diff --git a/tests/unit/pricing-sync-cross-instance.test.ts b/tests/unit/pricing-sync-cross-instance.test.ts new file mode 100644 index 0000000000..fb3658918e --- /dev/null +++ b/tests/unit/pricing-sync-cross-instance.test.ts @@ -0,0 +1,80 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +// ─── Regression test for #6325 ─────────────────────────────────────────── +// +// In production, `instrumentation-node.ts` (background periodic sync) and +// `src/app/api/pricing/sync/route.ts` (dashboard status endpoint) each +// `await import("@/lib/pricingSync")` from SEPARATE Next.js standalone +// webpack entries, producing SEPARATE module instances with independent +// top-level (`lastSyncTime`, `lastSyncModelCount`, `syncTimer`) state. +// +// This test simulates that scenario by importing the module twice under +// distinct specifiers (cache-busted via a query string), so each import +// gets its own top-level module state while both share the same on-disk +// SQLite DB (as they do in production). It asserts that a status read from +// a FRESH module instance still reflects a sync performed by a DIFFERENT +// module instance — i.e. sync status must be derived from persisted state, +// not from in-memory module-level variables. + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-pricing-sync-xinst-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); + +const originalFetch = globalThis.fetch; + +function buildLiteLLMFixture() { + return { + "openai/gpt-4o": { + input_cost_per_token: 0.0000025, + output_cost_per_token: 0.00001, + litellm_provider: "openai", + mode: "chat", + }, + }; +} + +test.after(async () => { + globalThis.fetch = originalFetch; + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("getSyncStatus reflects a sync performed by a different module instance", async () => { + globalThis.fetch = async () => + new Response(JSON.stringify(buildLiteLLMFixture()), { + status: 200, + headers: { "content-type": "application/json" }, + }); + + // Instance A: simulates the background periodic-sync module instance. + const pricingSyncA = await import("../../src/lib/pricingSync.ts?instance=A"); + const result = await pricingSyncA.syncPricingFromSources({ + sources: ["litellm"], + dryRun: false, + }); + assert.equal(result.success, true); + + // Instance B: simulates the dashboard API-route module instance — a + // genuinely fresh module scope that never called syncPricingFromSources + // itself, and whose own `syncTimer`/`lastSyncTime` module vars are unset. + const pricingSyncB = await import("../../src/lib/pricingSync.ts?instance=B"); + const status = pricingSyncB.getSyncStatus(); + + assert.equal( + status.lastSyncModelCount, + result.modelCount, + "should read model count from persisted state" + ); + assert.ok(status.lastSyncModelCount > 0, "persisted model count should be non-zero"); + assert.notEqual(status.lastSync, null, "should read lastSync from persisted state"); + assert.notEqual( + status.nextSync, + null, + "nextSync must be computed from persisted state, not the local (unset) syncTimer" + ); +});