From 708d77616caeaa2111271adae5926407beec80d9 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sat, 20 Jun 2026 06:01:52 -0300 Subject: [PATCH] fix(compliance): startup cleanup honors dashboard data-retention, not just env 7d (#4354) (#4363) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cleanupExpiredLogs() ran on every startup and read retention only from the CALL_LOG_RETENTION_DAYS / APP_LOG_RETENTION_DAYS env vars (default 7d when unset), trimming usage_history before the dashboard-based runAutoCleanup() β€” which respects the configured retention β€” ever ran. A dashboard 'Data Retention' of 90d was silently overridden, so the Usage Analysis page only showed 7 days after a restart. Retention precedence is now: explicit env var > dashboard DB setting > 7-day default, applied per table (usage_history->usageHistory, call/proxy/detail->callLogs, mcp_tool_audit->mcpAudit). An explicit env var still wins (operator override) and non-DB deployments still fall back to it. Adds getCallLogRetentionDaysOverride / getAppLogRetentionDaysOverride (null when env unset). TDD: log-retention.test.ts gains a case where the env is unset and the dashboard configures 90d β€” a 30-day usage_history row must survive (was deleted at the 7d default). RED before, GREEN after; the existing env-explicit cases are unchanged. Co-authored-by: akbardwi --- CHANGELOG.md | 1 + src/lib/compliance/index.ts | 33 ++++++++++++++++++++++++++++---- src/lib/logEnv.ts | 20 +++++++++++++++++++ tests/unit/log-retention.test.ts | 33 ++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c39924e80..ba645bbfdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ _In development β€” bullets added per PR; finalized at release._ ### πŸ› Fixed +- **fix(compliance): startup cleanup honors the dashboard data-retention setting instead of always trimming to 7 days** β€” on every restart, `cleanupExpiredLogs()` (run at startup) read retention only from the `CALL_LOG_RETENTION_DAYS` / `APP_LOG_RETENTION_DAYS` env vars, which default to **7 days** when unset, and trimmed `usage_history` (the Usage Analysis data) before the dashboard-based `runAutoCleanup()` β€” which respects the configured retention β€” ever ran. So a dashboard "Data Retention" of 90 days was silently overridden and the Usage Analysis page only ever showed the last 7 days after a restart. Retention now follows the precedence **explicit env var β†’ dashboard DB setting β†’ 7-day default**, per table (`usage_history`β†’`usageHistory`, `call_logs`/`proxy_logs`/`request_detail_logs`β†’`callLogs`, `mcp_tool_audit`β†’`mcpAudit`); an operator who sets the env var still wins, and non-DB deployments still fall back to it. ([#4354](https://github.com/diegosouzapw/OmniRoute/issues/4354) β€” thanks @akbardwi) - **fix(providers): bailian-coding-plan static fallback catalog matches the registry (10 models)** β€” the provider-model sweep (#4324) added four current Model Studio coding-plan models (`qwen3.7-plus`, `qwen3-coder-plus`, `qwen3-coder-next`, `glm-4.7`) to the `bailian-coding-plan` registry entry but missed the static fallback mirror in `staticModels.ts`, which still listed only the older six. The static catalog (served when live discovery is unavailable) therefore diverged from the registry, and the existing static↔registry parity test went red on the release branch (only surfacing when test-impact analysis happened to select it). The static mirror now carries all ten models in registry order, restoring parity. ([#4324](https://github.com/diegosouzapw/OmniRoute/pull/4324)) - **fix(executors): ArenaLLM accepts LMArena's split Supabase SSR auth cookie** β€” LMArena migrated to `@supabase/ssr` chunked auth cookies: the single `arena-auth-prod-v1` cookie is now empty and the real session is split across `arena-auth-prod-v1.0`, `arena-auth-prod-v1.1`, … (ascending). A user who pasted the (now-empty) single cookie therefore sent an empty session and upstream rejected it as "invalid cookie". The LMArena executor now reconstructs the single cookie from its chunks β€” reading `.0`, `.1`, … in ascending numeric order until one is missing and concatenating their raw values (`@supabase/ssr`'s `combineChunks` rule: plain `join("")`, no base64-decode, no JSON-parse, the `base64-` prefix kept verbatim) β€” while preserving the rest of the pasted jar. A non-empty single cookie is still forwarded unchanged (back-compat). The credential UX now instructs pasting the **full Cookie header** and tracks the `.0`/`.1` storage keys. ([#4271](https://github.com/diegosouzapw/OmniRoute/issues/4271) β€” thanks @caussao) - **fix(compression): preserve the cacheable prefix for automatic-cache providers** β€” OpenAI / Codex (and Azure-OpenAI) use _automatic_ prefix caching: the upstream caches the longest matching prefix of a request (system prompt + earliest messages) **without** any explicit `cache_control` markers in the body. The cache-aware compression guard only protected that prefix when the request carried explicit `cache_control`, so for automatic-cache providers the guard was skipped β€” and with compression enabled and `preserveSystemPrompt: false` (or a prefix-compressing mode like `aggressive`/`ultra`) it rewrote the system prompt / earliest messages, guaranteeing a cache miss and **higher** token spend through OmniRoute than going direct. The guard now treats a caching provider as sufficient on its own (`isCachingProvider` alone, independent of `cache_control`) to skip the system prompt and downgrade prefix-compressing modes, and OpenAI/Codex/Azure are now recognized as caching providers. Compression is still off by default β€” this only affects operators who enabled it with prefix preservation turned off. ([#3955](https://github.com/diegosouzapw/OmniRoute/issues/3955)) diff --git a/src/lib/compliance/index.ts b/src/lib/compliance/index.ts index 769e3c7574..e02ed83be8 100644 --- a/src/lib/compliance/index.ts +++ b/src/lib/compliance/index.ts @@ -15,9 +15,12 @@ import { getClientIpFromRequest } from "../ipUtils"; import { getAppLogRetentionDays, getCallLogRetentionDays, + getAppLogRetentionDaysOverride, + getCallLogRetentionDaysOverride, getCallLogsTableMaxRows, getProxyLogsTableMaxRows, } from "../logEnv"; +import { getUserDatabaseSettings } from "../db/databaseSettings"; import { generateRequestId, getRequestId } from "@/shared/utils/requestId"; import { HIGH_LEVEL_ACTIONS } from "@/lib/audit/highLevelActions"; @@ -483,8 +486,30 @@ export async function cleanupExpiredLogs() { }; } - const callCutoff = new Date(Date.now() - callRetentionDays * 24 * 60 * 60 * 1000).toISOString(); - const appCutoff = new Date(Date.now() - appRetentionDays * 24 * 60 * 60 * 1000).toISOString(); + // #4354: retention precedence is explicit env override > dashboard DB setting > 7-day + // default. Previously this path always used the env default (7d), silently overriding a + // configured dashboard "Data Retention" (e.g. 90d) on every startup and trimming + // usage_history before the dashboard-based runAutoCleanup() could run. We now honor the + // dashboard retention per table when the operator did not set the env var, while still + // letting an explicit env var win (and falling back to env for non-DB deployments). + const callOverride = getCallLogRetentionDaysOverride(); + const appOverride = getAppLogRetentionDaysOverride(); + let dbRetention: { usageHistory: number; callLogs: number; mcpAudit: number } | null = null; + try { + const r = getUserDatabaseSettings().retention; + dbRetention = { usageHistory: r.usageHistory, callLogs: r.callLogs, mcpAudit: r.mcpAudit }; + } catch { + /* settings table unavailable (e.g. very early startup) β€” keep env fallback */ + } + const usageHistoryRetentionDays = callOverride ?? dbRetention?.usageHistory ?? callRetentionDays; + const callLogRetentionDays = callOverride ?? dbRetention?.callLogs ?? callRetentionDays; + const mcpAuditRetentionDays = appOverride ?? dbRetention?.mcpAudit ?? appRetentionDays; + + const day = 24 * 60 * 60 * 1000; + const usageCutoff = new Date(Date.now() - usageHistoryRetentionDays * day).toISOString(); + const callCutoff = new Date(Date.now() - callLogRetentionDays * day).toISOString(); + const appCutoff = new Date(Date.now() - appRetentionDays * day).toISOString(); + const mcpCutoff = new Date(Date.now() - mcpAuditRetentionDays * day).toISOString(); let deletedUsage = 0; let deletedCallLogs = 0; @@ -496,7 +521,7 @@ export async function cleanupExpiredLogs() { let trimmedProxyLogs = 0; try { - const r1 = db.prepare("DELETE FROM usage_history WHERE timestamp < ?").run(callCutoff); + const r1 = db.prepare("DELETE FROM usage_history WHERE timestamp < ?").run(usageCutoff); deletedUsage = r1.changes; } catch { /* table may not exist */ @@ -532,7 +557,7 @@ export async function cleanupExpiredLogs() { } try { - const r6 = db.prepare("DELETE FROM mcp_tool_audit WHERE created_at < ?").run(appCutoff); + const r6 = db.prepare("DELETE FROM mcp_tool_audit WHERE created_at < ?").run(mcpCutoff); deletedMcpAuditLogs = r6.changes; } catch { /* table may not exist */ diff --git a/src/lib/logEnv.ts b/src/lib/logEnv.ts index f210cd0b11..0e758fd10b 100644 --- a/src/lib/logEnv.ts +++ b/src/lib/logEnv.ts @@ -71,6 +71,26 @@ export function getCallLogRetentionDays(): number { return parsePositiveInt(process.env.CALL_LOG_RETENTION_DAYS, DEFAULT_CALL_LOG_RETENTION_DAYS); } +/** + * Returns the explicit operator-set retention override (a positive integer), or `null` + * when the env var is unset/empty/invalid. Callers give an explicit env var precedence + * over the dashboard's database retention, while falling back to the dashboard (not the + * hardcoded 7-day default) when the operator did not set the env var. (#4354) + */ +function parsePositiveIntOrNull(value: string | undefined): number | null { + if (value === undefined || value === "") return null; + const parsed = Number.parseInt(value, 10); + return Number.isInteger(parsed) && parsed > 0 ? parsed : null; +} + +export function getAppLogRetentionDaysOverride(): number | null { + return parsePositiveIntOrNull(process.env.APP_LOG_RETENTION_DAYS); +} + +export function getCallLogRetentionDaysOverride(): number | null { + return parsePositiveIntOrNull(process.env.CALL_LOG_RETENTION_DAYS); +} + export function getAppLogMaxFiles(): number { return parsePositiveInt(process.env.APP_LOG_MAX_FILES, DEFAULT_APP_LOG_MAX_FILES); } diff --git a/tests/unit/log-retention.test.ts b/tests/unit/log-retention.test.ts index ec6abbc051..f9d83d4c8d 100644 --- a/tests/unit/log-retention.test.ts +++ b/tests/unit/log-retention.test.ts @@ -135,6 +135,39 @@ test("cleanupExpiredLogs uses separate APP and CALL retention windows", async () assert.equal((db.prepare("SELECT COUNT(*) AS cnt FROM mcp_tool_audit").get() as any).cnt, 1); }); +test("cleanupExpiredLogs honors the dashboard usageHistory retention when env is unset (#4354)", async () => { + // Operator did NOT set the retention env vars; the dashboard configured 90 days. + // The startup cleanup must respect the dashboard value, not the 7-day env default. + const savedCall = process.env.CALL_LOG_RETENTION_DAYS; + const savedApp = process.env.APP_LOG_RETENTION_DAYS; + delete process.env.CALL_LOG_RETENTION_DAYS; + delete process.env.APP_LOG_RETENTION_DAYS; + try { + compliance.initAuditLog(); + const db = core.getDbInstance(); + + const { updateDatabaseSettings } = await import("../../src/lib/db/databaseSettings.ts"); + updateDatabaseSettings({ retention: { usageHistory: 90, callLogs: 90 } } as any); + + const oldTs = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000).toISOString(); + db.prepare( + "INSERT INTO usage_history (provider, model, tokens_input, tokens_output, success, latency_ms, ttft_ms, timestamp) VALUES (?, ?, ?, ?, ?, ?, ?, ?)" + ).run("openai", "old-usage", 1, 1, 1, 1, 1, oldTs); + + const result = await compliance.cleanupExpiredLogs(); + + // 30 days < the configured 90-day dashboard retention β†’ must be kept. + // With the old env-default (7d) behavior this row would be deleted. + assert.equal(result.deletedUsage, 0, "30-day usage_history must survive a 90-day dashboard retention"); + assert.equal((db.prepare("SELECT COUNT(*) AS cnt FROM usage_history").get() as any).cnt, 1); + } finally { + if (savedCall !== undefined) process.env.CALL_LOG_RETENTION_DAYS = savedCall; + else delete process.env.CALL_LOG_RETENTION_DAYS; + if (savedApp !== undefined) process.env.APP_LOG_RETENTION_DAYS = savedApp; + else delete process.env.APP_LOG_RETENTION_DAYS; + } +}); + test("cleanupExpiredLogs enforces row count limits", async () => { compliance.initAuditLog(); const db = core.getDbInstance();