From 8a95a2bced078fae2abafec011e54dab9a17fd96 Mon Sep 17 00:00:00 2001 From: Davide Baraldo Date: Fri, 4 Sep 2026 01:48:40 +0200 Subject: [PATCH] fix(settings): cache-config alwaysPreserveClientCache was a runtime no-op (#12304) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validado em worktree combinada sobre o tip de release/v3.8.51: os dois boardaram sem conflito, typecheck:core limpo, check-file-size sem violação nova (as duas restantes — codex.ts e stream.ts — são drift anterior) e 51/51 nos 5 arquivos de teste que os PRs trazem. --- ...2304-cache-config-preserve-client-cache.md | 1 + src/app/api/settings/cache-config/route.ts | 30 +++++-- ...cache-config-preserve-client-cache.test.ts | 82 +++++++++++++++++++ 3 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 changelog.d/fixes/12304-cache-config-preserve-client-cache.md create mode 100644 tests/unit/cache-config-preserve-client-cache.test.ts diff --git a/changelog.d/fixes/12304-cache-config-preserve-client-cache.md b/changelog.d/fixes/12304-cache-config-preserve-client-cache.md new file mode 100644 index 0000000000..1c4da7f4c7 --- /dev/null +++ b/changelog.d/fixes/12304-cache-config-preserve-client-cache.md @@ -0,0 +1 @@ +- **fix(settings):** `PUT /api/settings/cache-config` now persists `alwaysPreserveClientCache` to the flat general settings the runtime cache-control policy actually reads; previously the value landed in the databaseSettings "cache" section and was silently ignored, so the endpoint had no effect on `cache_control` passthrough ([#12304](https://github.com/diegosouzapw/OmniRoute/pull/12304)) — thanks @davidebaraldo diff --git a/src/app/api/settings/cache-config/route.ts b/src/app/api/settings/cache-config/route.ts index fc5a6634aa..cbf777b634 100644 --- a/src/app/api/settings/cache-config/route.ts +++ b/src/app/api/settings/cache-config/route.ts @@ -58,8 +58,12 @@ export async function GET(request: NextRequest) { const flatSettings = await getSettings(); const config: Record = {}; for (const key of CACHE_CONFIG_KEYS) { - if (key === "idempotencyWindowMs") { - config[key] = flatSettings.idempotencyWindowMs ?? DEFAULTS[key]; + if (key === "idempotencyWindowMs" || key === "alwaysPreserveClientCache") { + // These live in the flat general settings (src/lib/db/settings.ts): + // idempotencyLayer and getCacheControlSettings() both read from there, + // so reporting the databaseSettings "cache" copy would show a value the + // runtime never uses. + config[key] = flatSettings[key] ?? DEFAULTS[key]; } else { config[key] = (cache as Record)[key] ?? DEFAULTS[key]; } @@ -106,9 +110,6 @@ export async function PUT(request: NextRequest) { if (body.promptCacheStrategy !== undefined) { updates.promptCacheStrategy = body.promptCacheStrategy; } - if (body.alwaysPreserveClientCache !== undefined) { - updates.alwaysPreserveClientCache = body.alwaysPreserveClientCache; - } if (body.modelCatalogCacheTtlMs !== undefined) { updates.modelCatalogCacheTtlMs = body.modelCatalogCacheTtlMs; } @@ -116,12 +117,23 @@ export async function PUT(request: NextRequest) { // updateDatabaseSettings() calls invalidateDbCache("settings") internally, // which bumps the model-catalog cache version so in-flight responses pick // up the fresh TTL — no separate version bump needed here. - updateDatabaseSettings({ cache: updates }); + if (Object.keys(updates).length > 0) { + updateDatabaseSettings({ cache: updates }); + } - // idempotencyWindowMs is not part of the databaseSettings "cache" section — - // persist it through the flat general settings module instead (see GET). + // idempotencyWindowMs and alwaysPreserveClientCache are read from the flat + // general settings (see GET) — persisting them into the databaseSettings + // "cache" section would be a silent no-op for the runtime, which is what + // made this endpoint's alwaysPreserveClientCache writes ineffective before. + const flatUpdates: Record = {}; if (body.idempotencyWindowMs !== undefined) { - await updateSettings({ idempotencyWindowMs: body.idempotencyWindowMs }); + flatUpdates.idempotencyWindowMs = body.idempotencyWindowMs; + } + if (body.alwaysPreserveClientCache !== undefined) { + flatUpdates.alwaysPreserveClientCache = body.alwaysPreserveClientCache; + } + if (Object.keys(flatUpdates).length > 0) { + await updateSettings(flatUpdates); } return NextResponse.json({ ok: true }); diff --git a/tests/unit/cache-config-preserve-client-cache.test.ts b/tests/unit/cache-config-preserve-client-cache.test.ts new file mode 100644 index 0000000000..ab2b3c825e --- /dev/null +++ b/tests/unit/cache-config-preserve-client-cache.test.ts @@ -0,0 +1,82 @@ +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 guard: PUT /api/settings/cache-config persisted +// `alwaysPreserveClientCache` into the databaseSettings "cache" section, but +// the runtime (getCacheControlSettings → getSettings) reads the FLAT general +// settings key — so the endpoint accepted the value, GET echoed it back, and +// the router never changed behavior. This test proves the value written +// through the route is the one the cache-control policy actually consumes. + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cache-config-flat-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.DISABLE_SQLITE_AUTO_BACKUP = "true"; + +const core = await import("../../src/lib/db/core.ts"); + +function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +function makeJsonRequest(method: string, body?: unknown): Request { + return new Request("http://localhost/api/settings/cache-config", { + method, + headers: { "Content-Type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), + }); +} + +test.beforeEach(() => { + resetStorage(); +}); + +test.after(() => { + resetStorage(); +}); + +test("alwaysPreserveClientCache set via cache-config reaches the runtime read path", async (t) => { + const cacheConfigRoute = await import("../../src/app/api/settings/cache-config/route.ts"); + const { getSettings } = await import("../../src/lib/db/settings.ts"); + const { getCacheControlSettings, invalidateCacheControlSettingsCache } = + await import("../../src/lib/cacheControlSettings.ts"); + + await t.test("PUT persists to the flat settings the runtime reads", async () => { + const putResponse = await cacheConfigRoute.PUT( + makeJsonRequest("PUT", { alwaysPreserveClientCache: "always" }) as never + ); + assert.equal(putResponse.status, 200); + + // The runtime read path: getCacheControlSettings() → getSettings() (flat). + // RED before the fix: the route wrote databaseSettings "cache" instead, + // so both of these still reported the default "auto". + const flatSettings = await getSettings(); + assert.equal(flatSettings.alwaysPreserveClientCache, "always"); + + invalidateCacheControlSettingsCache(); + assert.equal(await getCacheControlSettings(), "always"); + }); + + await t.test("GET reports the flat value, not the ignored cache-section copy", async () => { + // Seed a stale value in the databaseSettings "cache" section — the store + // the runtime never reads. GET must not surface it. + const { updateDatabaseSettings } = await import("../../src/lib/db/databaseSettings.ts"); + updateDatabaseSettings({ + cache: { alwaysPreserveClientCache: "never" }, + } as Parameters[0]); + + const putResponse = await cacheConfigRoute.PUT( + makeJsonRequest("PUT", { alwaysPreserveClientCache: "always" }) as never + ); + assert.equal(putResponse.status, 200); + + const getResponse = await cacheConfigRoute.GET(makeJsonRequest("GET") as never); + const body = await getResponse.json(); + assert.equal(getResponse.status, 200); + assert.equal(body.alwaysPreserveClientCache, "always"); + }); +});