From 9066464160890b2fb89170831bf0ba04c875557f Mon Sep 17 00:00:00 2001 From: Minxi Hou Date: Tue, 4 Aug 2026 21:48:17 -0400 Subject: [PATCH] feat(lib): make MODELS_DEV_SYNC_ENABLED actually control the sync The docblock on modelsDevSync.ts has always said "Opt-in via MODELS_DEV_SYNC_ENABLED=true", but nothing read that variable. Its single appearance in the tree was that one comment line: initModelsDevSync consulted the stored modelsDevSyncEnabled setting and nothing else, so an operator who followed the documentation got no response whichever value they set. Give the variable the behaviour the comment promised, and describe it in .env.example and the environment reference, where it was also missing. An explicit value decides in either direction; only an unset or empty one defers to the setting. Pinning the sync OFF from a compose file or unit is the case that motivates it, since the stored setting can already say on and a container has no way to reach the dashboard before boot. That makes this a third resolution shape rather than a reuse of an existing one, which is worth stating rather than glossing. resolveFeatureFlag puts the DB override above the environment, so a deployment cannot override a stored choice. getCcAliasGlobalState reads only "1" and "true" and forces a flag on, letting every other value fall through to the DB. Neither can turn a dashboard-enabled switch off from the environment, so either one would leave this variable unable to do the thing it is here for. The reasoning is recorded next to the code. Five tests cover the resulting matrix: env on with the setting off, the setting alone with no env var, the accepted spellings and their rejection of everything else, env off against a stored on, and unset still deferring to a stored on. --- .env.example | 9 ++ docs/reference/ENVIRONMENT.md | 1 + src/lib/modelsDevSync.ts | 37 ++++- tests/unit/modelsDevSync-extended.test.ts | 180 ++++++++++++++++++++++ 4 files changed, 224 insertions(+), 3 deletions(-) diff --git a/.env.example b/.env.example index 4cb877425cc3..b084cd241797 100644 --- a/.env.example +++ b/.env.example @@ -1471,6 +1471,15 @@ APP_LOG_TO_FILE=true # ═══════════════════════════════════════════════════════════════════════════════ # 19. MODEL SYNC (Dev) # ═══════════════════════════════════════════════════════════════════════════════ +# Enable the models.dev capability sync. Default: false (opt-in only). +# Also settable from Dashboard > Settings > AI. This variable wins over that +# setting whenever it is set to anything non-empty, in either direction, so a +# deployment can pin the sync on or off without depending on database state +# surviving a rebuild. Leave it unset to let the dashboard toggle decide. +# On: 1, true, yes or on (any casing). Any other value is off. +# Used by: src/lib/modelsDevSync.ts +# MODELS_DEV_SYNC_ENABLED=false + # Development-time model catalog sync interval in seconds. # Used by: src/lib/modelsDevSync.ts # Default: 86400 (24 hours) diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index 1bf2f0f06499..ff3a2f79368f 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -828,6 +828,7 @@ Reverse-engineered session bridge for hyperagent.com (`src/shared/constants/prov | Variable | Default | Source File | Description | | ----------------------------------- | ------------- | ---------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `MODELS_DEV_SYNC_ENABLED` | `false` | `src/lib/modelsDevSync.ts` | Opt-in switch for the models.dev capability sync. Set to anything non-empty it wins over the `modelsDevSyncEnabled` setting (Dashboard > Settings > AI) in either direction, so a deployment can pin the sync on or off without depending on database state surviving a rebuild; unset, it defers to that setting. On for `1`, `true`, `yes` or `on` in any casing; any other value is off. | | `MODELS_DEV_SYNC_INTERVAL` | `86400` (24h) | `src/lib/modelsDevSync.ts` | Development-time model catalog sync interval in seconds. | | `CONTEXT_WINDOW_RECONCILE_INTERVAL` | `86400` (24h) | `src/lib/contextWindowResolver.ts` | Interval (seconds) for the self-correcting context-window reconciler (5004): pins provider-declared windows from `/models` discovery as `auto:discovery` overrides when they diverge from the catalog. Set to `0` to disable. Reuses already-synced data (no new fetch); never overwrites `manual` overrides. | diff --git a/src/lib/modelsDevSync.ts b/src/lib/modelsDevSync.ts index 32a6e180f69d..fbcabe353b93 100644 --- a/src/lib/modelsDevSync.ts +++ b/src/lib/modelsDevSync.ts @@ -14,7 +14,12 @@ * 3. LiteLLM sync (`pricing_synced` namespace) * 4. Hardcoded defaults (`pricing.ts`) * - * Opt-in via MODELS_DEV_SYNC_ENABLED=true (default: false). + * Opt-in, default off. Enabled either from Dashboard > Settings > AI or with + * MODELS_DEV_SYNC_ENABLED, which wins over that setting whenever it is set to + * anything non-empty, in either direction, so a deployment can pin the sync on + * or off regardless of what is stored. Unset or empty, it defers to the + * setting. On for "1", "true", "yes" or "on" in any casing; every other value + * is off. */ import { getDbInstance } from "./db/core"; @@ -71,6 +76,8 @@ interface SyncResult { const MODELS_DEV_API_URL = "https://models.dev/api.json"; +const TRUE_ENV_VALUES = new Set(["1", "true", "yes", "on"]); + const parsedInterval = parseInt(process.env.MODELS_DEV_SYNC_INTERVAL || "86400", 10); const SYNC_INTERVAL_MS = Number.isFinite(parsedInterval) && parsedInterval > 0 ? parsedInterval * 1000 : 86400 * 1000; @@ -670,8 +677,32 @@ export async function initModelsDevSync(): Promise { const { getSettings } = await import("./localDb"); const settings = await getSettings(); - if (settings.modelsDevSyncEnabled !== true) { - console.log("[MODELS_DEV] Disabled (enable via Settings > AI)"); + // Until now the docblock above advertised MODELS_DEV_SYNC_ENABLED and nothing + // read it: the only control was the stored setting, so an operator following + // that line got silence whichever value they set. This makes the variable real. + // + // An explicit env value decides, in either direction, and only an unset or + // empty one defers to the setting. That means a deployment can pin the sync + // off from its compose file or unit even when a previous operator left the + // dashboard toggle on, which is the case a force-on-only variable cannot + // express and the reason for choosing this shape. + // + // It is worth being plain that this is a third resolution pattern rather than + // a reuse of an existing one, because the two in the tree solve different + // problems: shared/utils/featureFlags.ts::resolveFeatureFlag puts the DB + // override ABOVE the env var, so a deployment cannot override an operator's + // stored choice at all; db/ccDiscoveryAliases.ts::getCcAliasGlobalState reads + // only "1" and "true" and can force a flag ON, letting every other value + // including "false" fall through to the DB. Neither can turn a + // dashboard-enabled switch off from the environment. Following either one + // here would leave the variable unable to do the thing it is being added for. + const envValue = process.env.MODELS_DEV_SYNC_ENABLED?.trim(); + const enabled = envValue + ? TRUE_ENV_VALUES.has(envValue.toLowerCase()) + : settings.modelsDevSyncEnabled === true; + + if (!enabled) { + console.log("[MODELS_DEV] Disabled (enable via Settings > AI or MODELS_DEV_SYNC_ENABLED=true)"); return; } diff --git a/tests/unit/modelsDevSync-extended.test.ts b/tests/unit/modelsDevSync-extended.test.ts index 9b96a8440b66..942bb9b77358 100644 --- a/tests/unit/modelsDevSync-extended.test.ts +++ b/tests/unit/modelsDevSync-extended.test.ts @@ -564,3 +564,183 @@ test("stopPeriodicSync aborts the in-flight initial sync", async () => { assert.equal(modelsDev.getSyncStatus().enabled, false); assert.equal(modelsDev.getSyncStatus().lastSync, null); }); + +// MODELS_DEV_SYNC_ENABLED was named in this module's header comment for a long +// time without ever being read, so the only real switch was a row in the +// database. A container rebuilt from a fresh volume therefore came up with the +// sync off no matter what the deployment intended. + +test("MODELS_DEV_SYNC_ENABLED=true starts the sync even with the setting off", async () => { + const previous = process.env.MODELS_DEV_SYNC_ENABLED; + await settingsDb.updateSettings({ + modelsDevSyncEnabled: false, + modelsDevSyncInterval: 15, + }); + + process.env.MODELS_DEV_SYNC_ENABLED = "true"; + const modelsDev = await importFresh("init-env-on"); + mockFetchWith(MOCK_MODELS_DEV_DATA); + try { + await modelsDev.initModelsDevSync(); + assert.equal(modelsDev.getSyncStatus().enabled, true); + // `enabled` alone would also be true for a sync that started and then + // never fetched anything, so pin the fetch actually having run. Assert + // the result, not just await it -- waitFor returns null on timeout, and + // an unasserted timeout is indistinguishable from success. + assert.ok( + await waitFor(() => modelsDev.getSyncStatus().lastSync !== null), + "expected the initial sync to complete and set lastSync" + ); + } finally { + modelsDev.stopPeriodicSync(); + if (previous === undefined) delete process.env.MODELS_DEV_SYNC_ENABLED; + else process.env.MODELS_DEV_SYNC_ENABLED = previous; + } +}); + +test("the stored setting still starts the sync with no env var present", async () => { + const previous = process.env.MODELS_DEV_SYNC_ENABLED; + delete process.env.MODELS_DEV_SYNC_ENABLED; + await settingsDb.updateSettings({ + modelsDevSyncEnabled: true, + modelsDevSyncInterval: 15, + }); + + const modelsDev = await importFresh("init-setting-only"); + mockFetchWith(MOCK_MODELS_DEV_DATA); + try { + // Without this the test would still pass if the variable were set to + // "true", crediting the setting for what the env var did. + assert.equal(process.env.MODELS_DEV_SYNC_ENABLED, undefined); + await modelsDev.initModelsDevSync(); + assert.equal(modelsDev.getSyncStatus().enabled, true); + assert.ok( + await waitFor(() => modelsDev.getSyncStatus().lastSync !== null), + "expected the initial sync to complete and set lastSync" + ); + } finally { + modelsDev.stopPeriodicSync(); + if (previous !== undefined) process.env.MODELS_DEV_SYNC_ENABLED = previous; + } +}); + +test("the usual truthy spellings all start the sync, and nothing else does", async () => { + const previous = process.env.MODELS_DEV_SYNC_ENABLED; + await settingsDb.updateSettings({ + modelsDevSyncEnabled: false, + modelsDevSyncInterval: 15, + }); + + // A compose file or unit file is as likely to carry "1" as "true", so all + // four spellings work, in any casing and with stray whitespace. Everything + // else leaves the sync off rather than guessing at intent. + const cases: Array<[string, boolean]> = [ + ["true", true], + ["TRUE", true], + ["True", true], + [" true ", true], + ["1", true], + ["yes", true], + ["on", true], + ["ON", true], + ["false", false], + ["0", false], + ["no", false], + ["off", false], + ["", false], + ["truthy", false], + ]; + + try { + for (const [index, [value, expected]] of cases.entries()) { + process.env.MODELS_DEV_SYNC_ENABLED = value; + // The label becomes a cache-busting URL suffix, so it has to stay + // URL-safe; the values themselves carry quotes and whitespace. + const modelsDev = await importFresh(`init-env-case-${index}`); + if (expected) mockFetchWith(MOCK_MODELS_DEV_DATA); + await modelsDev.initModelsDevSync(); + assert.equal( + modelsDev.getSyncStatus().enabled, + expected, + `MODELS_DEV_SYNC_ENABLED=${JSON.stringify(value)} should ${expected ? "" : "not "}enable the sync` + ); + if (expected) { + // `enabled` alone would also be true for a sync that started and + // then never fetched anything; pin the fetch actually having run + // for each truthy spelling, not just the first one. + assert.ok( + await waitFor(() => modelsDev.getSyncStatus().lastSync !== null), + `MODELS_DEV_SYNC_ENABLED=${JSON.stringify(value)} should have completed a sync` + ); + } + modelsDev.stopPeriodicSync(); + } + } finally { + if (previous === undefined) delete process.env.MODELS_DEV_SYNC_ENABLED; + else process.env.MODELS_DEV_SYNC_ENABLED = previous; + } +}); + +test("MODELS_DEV_SYNC_ENABLED=false turns the sync off despite a stored setting of true", async () => { + // The direction that costs an operator real time if it is wrong: they put + // the variable in their compose file expecting a master switch, and the + // sync keeps running because the dashboard toggle is still on. An explicit + // env value decides in either direction; only an unset one defers. + const previous = process.env.MODELS_DEV_SYNC_ENABLED; + await settingsDb.updateSettings({ + modelsDevSyncEnabled: true, + modelsDevSyncInterval: 15, + }); + + process.env.MODELS_DEV_SYNC_ENABLED = "false"; + const modelsDev = await importFresh("init-env-false-setting-true"); + mockFetchWith(MOCK_MODELS_DEV_DATA); + try { + await modelsDev.initModelsDevSync(); + assert.equal( + modelsDev.getSyncStatus().enabled, + false, + "MODELS_DEV_SYNC_ENABLED=false should disable the sync even with the setting on" + ); + assert.equal( + modelsDev.getSyncStatus().lastSync, + null, + "a disabled sync must not have fetched anything" + ); + } finally { + modelsDev.stopPeriodicSync(); + if (previous === undefined) delete process.env.MODELS_DEV_SYNC_ENABLED; + else process.env.MODELS_DEV_SYNC_ENABLED = previous; + } +}); + +test("an unset MODELS_DEV_SYNC_ENABLED still defers to a stored setting of true", async () => { + // The counterpart: without this one, the test above would also pass if the + // env var had simply become a hard off switch and the dashboard toggle had + // stopped working entirely. + const previous = process.env.MODELS_DEV_SYNC_ENABLED; + delete process.env.MODELS_DEV_SYNC_ENABLED; + await settingsDb.updateSettings({ + modelsDevSyncEnabled: true, + modelsDevSyncInterval: 15, + }); + + const modelsDev = await importFresh("init-env-unset-setting-true"); + mockFetchWith(MOCK_MODELS_DEV_DATA); + try { + await modelsDev.initModelsDevSync(); + assert.equal( + modelsDev.getSyncStatus().enabled, + true, + "an unset env var should leave the stored setting in charge" + ); + assert.ok( + await waitFor(() => modelsDev.getSyncStatus().lastSync !== null), + "expected the initial sync to complete and set lastSync" + ); + } finally { + modelsDev.stopPeriodicSync(); + if (previous === undefined) delete process.env.MODELS_DEV_SYNC_ENABLED; + else process.env.MODELS_DEV_SYNC_ENABLED = previous; + } +});