From ef324cd00e4c3e9c40ce7c61b97fd6f8aec265c2 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sun, 14 Jun 2026 01:38:38 -0300 Subject: [PATCH] fix(models): preserve eye-hidden models across auto-sync (#3782) (#3816) * fix(models): preserve eye-hidden models across auto-sync (#3782) * chore(quality): re-baseline file-size for models.ts growth (#3782) --- CHANGELOG.md | 1 + file-size-baseline.json | 5 +- src/app/api/provider-models/route.ts | 11 +- src/lib/db/models.ts | 54 ++++++- src/lib/providerModels/managedModelImport.ts | 14 +- .../synced-model-delete-persist-3199.test.ts | 26 +++- .../synced-model-hide-persist-3782.test.ts | 132 ++++++++++++++++++ 7 files changed, 224 insertions(+), 19 deletions(-) create mode 100644 tests/unit/synced-model-hide-persist-3782.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fe7674b23..d950be6f84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### 🐛 Fixed - **fix(intelligence): run pricing + models.dev sync from the live startup path** — like the Arena ELO sync (v3.8.24), the external **pricing sync** (`PRICING_SYNC_ENABLED`) and the **models.dev capability sync** (Settings → AI toggle) were only initialized from `server-init.ts`, which the Next standalone runtime never executes — and models.dev had no caller at all. Their toggles were inert in production. Both are now initialized from `instrumentation-node.ts` (self-gated, opt-in preserved, non-blocking, never fatal). (thanks @diegosouzapw) +- **fix(models): preserve eye-hidden models across auto-sync / import** — hiding models via the visibility (eye) toggle to keep only a combo's models was undone on every model import or auto-sync, which re-showed all of them. The sync re-import treated "hidden" identically to "deleted" and dropped both; a distinct `isDeleted` marker now separates the trash/delete path (still dropped on re-import, #3199) from the eye toggle (preserved as listed-but-hidden), and eye-hidden models are no longer re-aliased into the routable catalog on sync. ([#3782](https://github.com/diegosouzapw/OmniRoute/issues/3782) — thanks @xenstar) - **fix(providers): correct the lmarena cookie hint (`session` → `arena-auth-prod-v1`)** — the lmarena credential hint asked for a cookie named `session`, but lmarena.ai's real auth cookie is `arena-auth-prod-v1`, so users who pasted only `session=…` hit validation failures. The credential name, placeholder and storage keys now use the correct name (the legacy `session` key is retained for back-compat with already-saved credentials). ([#3810](https://github.com/diegosouzapw/OmniRoute/issues/3810) — thanks @xspylol) --- diff --git a/file-size-baseline.json b/file-size-baseline.json index 158f5c3a13..db150ae7e4 100644 --- a/file-size-baseline.json +++ b/file-size-baseline.json @@ -79,7 +79,7 @@ "src/lib/db/apiKeys.ts": 1633, "src/lib/db/core.ts": 1820, "src/lib/db/migrationRunner.ts": 1100, - "src/lib/db/models.ts": 1132, + "src/lib/db/models.ts": 1180, "src/lib/db/providers.ts": 1050, "src/lib/db/proxies.ts": 1048, "src/lib/db/settings.ts": 1149, @@ -112,5 +112,6 @@ "_rebaseline_2026_06_12_phase1t": "Phase 1t (#3501): ProviderDetailPageClient.tsx 1377→782 — META ≤800 ATINGIDA (extraídos ProviderPageHeader, CompatibleNodeCard, ProviderModalsPanel, EmptyConnectionsPlaceholder, UpstreamProxyCard, SearchProviderCard + hooks useConnectionGate/useProviderNodeActions). Drift concorrente reconciliado: ResilienceTab/sse-chat/sse-auth/accountFallback/combo (merges #3629 model-lockout etc.).", "_rebaseline_2026_06_12_v3823_new_features": "Re-baseline v3.8.23 pós-merge de #3742 (cost drilldown: ApiManagerPageClient.tsx +21, CostOverviewTab.tsx +14, providerLimits.ts +2, usage.ts +53) + #3743 (provider display modes: ProviderDetailPageClient.tsx +2, providerPageHelpers.ts +42, providers.ts +2) + #3740 (semantic cache key isolation: chat.ts +3). Crescimento justificado por features novas mergeadas no ciclo.", "_rebaseline_2026_06_13_combo_quota_audit": "Re-baseline consciente do audit combo+quota (PR #3779): combo.ts 5054→5131 (+77). Crescimento = 5 fixes TDD + estratégia complexity-aware 2026 (W1 clampComboDepth + threading de maxDepth em 6 assinaturas/dispatch/DAG; W2 extração shouldSkipForPredictedTtft; W4 scoreAutoTargets exportado + param manifestHint). A parte limpa-extraível do W4 (construção do hint inline, ~30 linhas) FOI extraída para autoCombo/complexityRouter.ts (buildComplexityRoutingHint) — este +76 é o resíduo irredutível (edição de assinaturas/threading, não bloco movível). Shrink estrutural de combo.ts segue com #3501.", - "_rebaseline_2026_06_13_v3824_3776": "Re-baseline v3.8.24 pós #3776 (strict-mode controls Claude Code default models: ApiManagerPageClient.tsx 2701→2909 = UI de famílias bloqueáveis cc/* + chips; apiKeys.ts 1490→1633 = blocked_models deny-list + candidatos de permissão claude-code; schemas.ts 2515→2519 = reformatação Prettier + reasoningTokenBufferEnabled restaurado) + carry-over base.ts 1205→1218 do #3780 (enforceThinkingTemperature no chokepoint, drift de baseline não bumpado no merge). Crescimento de feature; sem god-component novo." + "_rebaseline_2026_06_13_v3824_3776": "Re-baseline v3.8.24 pós #3776 (strict-mode controls Claude Code default models: ApiManagerPageClient.tsx 2701→2909 = UI de famílias bloqueáveis cc/* + chips; apiKeys.ts 1490→1633 = blocked_models deny-list + candidatos de permissão claude-code; schemas.ts 2515→2519 = reformatação Prettier + reasoningTokenBufferEnabled restaurado) + carry-over base.ts 1205→1218 do #3780 (enforceThinkingTemperature no chokepoint, drift de baseline não bumpado no merge). Crescimento de feature; sem god-component novo.", + "_rebaseline_2026_06_13_3782_hide_persist": "Re-baseline #3782 (preservar modelos eye-hidden no auto-sync): models.ts 1132→1180 (+48). Crescimento = flag distinto isDeleted em ModelCompatOverride/ModelCompatPatch + handling em mergeModelCompatOverride + helper getModelIsDeleted, para separar 'deletado' (trash, dropado no re-sync #3199) de 'oculto' (eye toggle, preservado). Lógica coesa de visibilidade no módulo db; não-extraível. Os 3 drifts release-wide (ProxyRegistryManager/sidebarVisibility/schemas) são do #3809 do owner, não deste PR." } diff --git a/src/app/api/provider-models/route.ts b/src/app/api/provider-models/route.ts index ea4870c054..7856a38b0a 100644 --- a/src/app/api/provider-models/route.ts +++ b/src/app/api/provider-models/route.ts @@ -400,10 +400,13 @@ export async function DELETE(request) { const removedCustom = await removeCustomModel(provider, modelId); const removedSynced = await removeSyncedAvailableModel(provider, modelId); if (removedSynced) { - // #3199: mark the deleted synced model hidden so a later auto-fetch - // re-import (replaceSyncedAvailableModelsForConnection) does not re-add it - // — otherwise the model reappears and the delete looks like it did nothing. - mergeModelCompatOverride(provider, modelId, { isHidden: true }); + // #3199 + #3782: mark the deleted synced model with the DISTINCT `isDeleted` + // marker so a later auto-fetch re-import does not re-add it. We also keep + // `isHidden:true` so existing UI/visibility behavior is unchanged. The sync + // filter keys on `isDeleted` (not `isHidden`), which is what lets an + // eye/visibility-hidden model (`isHidden` only) survive a re-sync while a + // deleted one stays dropped. + mergeModelCompatOverride(provider, modelId, { isDeleted: true, isHidden: true }); } const removed = removedCustom || removedSynced; const removedAliases = await deleteManagedAvailableModelAliases(provider, [modelId]); diff --git a/src/lib/db/models.ts b/src/lib/db/models.ts index 07a3a58886..48b91c12ea 100644 --- a/src/lib/db/models.ts +++ b/src/lib/db/models.ts @@ -111,6 +111,16 @@ export type ModelCompatOverride = { compatByProtocol?: CompatByProtocolMap; upstreamHeaders?: Record; isHidden?: boolean; + /** + * #3782 — distinct "deleted" marker, separate from {@link isHidden}. + * + * `isHidden` is set by the EYE/visibility toggle and must be PRESERVED across a + * re-sync (the model stays listed-but-hidden). `isDeleted` is set by the trash/ + * DELETE route and means "drop this id on every re-import" (#3199). Keeping the + * two flags distinct is what lets {@link replaceSyncedAvailableModelsForConnection} + * preserve eye-hidden models while still dropping deleted ones. + */ + isDeleted?: boolean; }; function readCompatList(providerId: string): ModelCompatOverride[] { @@ -156,6 +166,8 @@ export type ModelCompatPatch = { /** Replace top-level extra headers for override-only rows; omit to leave unchanged. */ upstreamHeaders?: Record | null; isHidden?: boolean | null; + /** #3782 — distinct delete marker; set by the DELETE route, never by the eye toggle. */ + isDeleted?: boolean | null; }; function compatByProtocolHasEntries(map: CompatByProtocolMap | undefined): boolean { @@ -210,11 +222,20 @@ export function mergeModelCompatOverride( next.isHidden = Boolean(patch.isHidden); } } + if ("isDeleted" in patch) { + if (patch.isDeleted === null || patch.isDeleted === false) { + delete next.isDeleted; + } else { + next.isDeleted = Boolean(patch.isDeleted); + } + } const hasHiddenFlag = Object.prototype.hasOwnProperty.call(next, "isHidden"); + const hasDeletedFlag = Object.prototype.hasOwnProperty.call(next, "isDeleted"); if ( next.normalizeToolCallId || hasPreserveFlag || hasHiddenFlag || + hasDeletedFlag || compatByProtocolHasEntries(next.compatByProtocol) || hasTopUpstream ) { @@ -714,10 +735,15 @@ export async function replaceSyncedAvailableModelsForConnection( ): Promise { const db = getDbInstance(); const key = `${providerId}:${connectionId}`; - // #3199: drop ids the operator has deleted/hidden so a re-fetch does not - // re-import a model that was explicitly removed. + // #3199: drop ids the operator DELETED (trash) so a re-fetch does not re-import + // a model that was explicitly removed. + // #3782: key ONLY on the distinct `isDeleted` marker — NOT on `isHidden`. + // Eye/visibility-hidden models (`isHidden:true`, no `isDeleted`) must stay in + // the synced store so they remain listed-but-hidden across re-syncs instead of + // churning back on through the managed-alias path ("Auto Sync Enabling all + // Models"). See getModelIsDeleted for the legacy-row caveat. const normalizedModels = normalizeSyncedAvailableModels(models).filter( - (m) => !getModelIsHidden(providerId, m.id) + (m) => !getModelIsDeleted(providerId, m.id) ); if (normalizedModels.length === 0) { db.prepare("DELETE FROM key_value WHERE namespace = 'syncedAvailableModels' AND key = ?").run( @@ -1034,6 +1060,28 @@ export function getModelIsHidden(providerId: string, modelId: string): boolean { return Boolean(co?.isHidden); } +/** + * #3782 — Check if a model was DELETED (trash) rather than merely eye-hidden. + * + * Only the DELETE route sets `isDeleted`. The sync re-import filter keys on this + * (not on `isHidden`) so eye-hidden models survive a re-sync while deleted ones + * stay dropped. + * + * Legacy caveat: rows written by the DELETE route BEFORE this change carry only + * `isHidden:true` (no `isDeleted`). Treating bare legacy `isHidden:true` as + * deleted here would resurrect the #3782 bug for eye-hidden models; treating it + * as "kept" would resurrect previously-deleted models. Resurrecting a deleted + * model is the less-surprising, recoverable outcome (the operator can re-hide or + * re-delete it), whereas silently dropping an eye-hidden model is the reported + * regression — so we deliberately key ONLY on the explicit `isDeleted` flag and + * accept that a handful of pre-existing deleted rows may reappear once after the + * upgrade. Going forward both paths write the correct distinct markers. + */ +export function getModelIsDeleted(providerId: string, modelId: string): boolean { + const co = readCompatList(providerId).find((e) => e.id === modelId); + return Boolean(co?.isDeleted); +} + /** * Persist the hidden flag for a model. Stores the override on the custom-model * row when one exists, otherwise on the compat-override list. Setting diff --git a/src/lib/providerModels/managedModelImport.ts b/src/lib/providerModels/managedModelImport.ts index 5610e643ed..e5bd0b3cab 100644 --- a/src/lib/providerModels/managedModelImport.ts +++ b/src/lib/providerModels/managedModelImport.ts @@ -1,5 +1,6 @@ import { getCustomModels, + getModelIsHidden, getSyncedAvailableModelsForConnection, mergeModelCompatOverride, replaceCustomModels, @@ -271,7 +272,10 @@ export async function importManagedModels({ current = (ANTIGRAVITY_MODEL_ALIASES as any)[current]; continue; } - if (ANTIGRAVITY_REVERSE_MODEL_ALIASES && (ANTIGRAVITY_REVERSE_MODEL_ALIASES as any)[current]) { + if ( + ANTIGRAVITY_REVERSE_MODEL_ALIASES && + (ANTIGRAVITY_REVERSE_MODEL_ALIASES as any)[current] + ) { current = (ANTIGRAVITY_REVERSE_MODEL_ALIASES as any)[current]; continue; } @@ -306,13 +310,17 @@ export async function importManagedModels({ await setMitmAliasAll("antigravity", mappings); } - let syncedAliases = 0; if (usesManagedAvailableModels(providerId) && (mode === "merge" || discoveredModels.length > 0)) { const aliasModelIds = mode === "sync" ? syncedAvailableModels : discoveredModels; + // #3782: eye-hidden models now survive in `syncedAvailableModels` (so they stay + // listed-but-hidden), but they must NOT be re-assigned a routable managed alias + // — otherwise auto-sync silently re-enables routing for a model the operator hid. + // Exclude `isHidden` ids from the alias assignment. `pruneMissing` (sync mode) + // then drops any stale alias an eye-hidden model previously held. const aliasSync = await syncManagedAvailableModelAliases( providerId, - aliasModelIds.map((model) => model.id), + aliasModelIds.map((model) => model.id).filter((id) => !getModelIsHidden(providerId, id)), { pruneMissing: mode === "sync" } ); syncedAliases = aliasSync.assignedAliases.length; diff --git a/tests/unit/synced-model-delete-persist-3199.test.ts b/tests/unit/synced-model-delete-persist-3199.test.ts index 7e9271da2d..3aa7d84fb4 100644 --- a/tests/unit/synced-model-delete-persist-3199.test.ts +++ b/tests/unit/synced-model-delete-persist-3199.test.ts @@ -3,9 +3,14 @@ * across an auto-fetch re-import. * * #3204 added `removeSyncedAvailableModel`, but the DELETE route did not mark the - * model hidden and `replaceSyncedAvailableModelsForConnection` did not skip hidden - * ids — so the next `/models` sync re-imported the model and it reappeared. This - * test guards that a hidden id is filtered out on re-import. + * model deleted and `replaceSyncedAvailableModelsForConnection` did not skip + * deleted ids — so the next `/models` sync re-imported the model and it + * reappeared. This test guards that a deleted id is filtered out on re-import. + * + * #3782 update: the delete marker is now the DISTINCT `isDeleted` flag (the route + * sets `isDeleted` + `isHidden`), separate from the EYE/visibility toggle which + * sets `isHidden` only and must be preserved across re-syncs. The sync filter + * keys on `isDeleted`, so this test now simulates the route via `isDeleted`. */ import test, { before, after } from "node:test"; import assert from "node:assert/strict"; @@ -22,8 +27,11 @@ import fs from "node:fs"; const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-test-synced-del-")); process.env.DATA_DIR = tmpDir; -const { replaceSyncedAvailableModelsForConnection, getSyncedAvailableModels, mergeModelCompatOverride } = - await import("../../src/lib/localDb.ts"); +const { + replaceSyncedAvailableModelsForConnection, + getSyncedAvailableModels, + mergeModelCompatOverride, +} = await import("../../src/lib/localDb.ts"); const { resetDbInstance } = await import("../../src/lib/db/core.ts"); before(() => { @@ -47,8 +55,12 @@ test("a hidden (deleted) synced model is not re-added on re-import", async () => let synced = (await getSyncedAvailableModels(provider)).map((m) => m.id); assert.ok(synced.includes("model-del"), "both models present after first sync"); - // Operator deletes model-del → the route marks it hidden (#3199). - mergeModelCompatOverride(provider, "model-del", { isHidden: true }); + // Operator deletes model-del → the DELETE route marks it deleted. + // #3782: the route now writes the DISTINCT `isDeleted` marker (plus `isHidden` + // for back-compat) instead of bare `isHidden`, so an eye/visibility-hidden + // model — which sets `isHidden` only — is preserved across a re-sync while a + // genuinely-deleted one stays dropped. The sync filter keys on `isDeleted`. + mergeModelCompatOverride(provider, "model-del", { isDeleted: true, isHidden: true }); // Auto-fetch re-imports the SAME upstream list (still advertising model-del). await replaceSyncedAvailableModelsForConnection(provider, connectionId, [ diff --git a/tests/unit/synced-model-hide-persist-3782.test.ts b/tests/unit/synced-model-hide-persist-3782.test.ts new file mode 100644 index 0000000000..579f561e55 --- /dev/null +++ b/tests/unit/synced-model-hide-persist-3782.test.ts @@ -0,0 +1,132 @@ +/** + * #3782 — "Auto Sync Enabling all Models". + * + * The user hides models with the EYE/visibility toggle (writes `isHidden:true` + * via `mergeModelCompatOverride`) to keep only their combo's models. Before this + * fix, `replaceSyncedAvailableModelsForConnection` dropped EVERY hidden id on a + * re-sync (it could not tell an eye-hidden model from a DELETED one), so the + * model fell out of the synced store and then churned back through the managed + * alias path — the reported "all models turn back on". + * + * The fix separates the two signals: + * - DELETE (trash) marks `isDeleted:true` (+ keeps `isHidden:true` for back-compat). + * - The EYE toggle sets only `isHidden:true`. + * - The sync filter drops a model only when it is DELETED, so eye-hidden models + * stay listed-but-hidden across re-syncs. + * + * This test guards that an eye-hidden model survives re-import (Test A), that a + * genuinely-new model defaults to visible (Test B), and that the DELETE path + * still drops on re-import (Test C — mirrors the #3199 delete flow). + */ +import test, { before, after } from "node:test"; +import assert from "node:assert/strict"; +import os from "node:os"; +import path from "node:path"; +import fs from "node:fs"; + +// Hermetic DB: this test writes overrides into the `modelCompatOverrides` +// key_value namespace. Without an isolated DATA_DIR it would leak that state +// into the shared dev/CI database and never clean it up, so the SECOND run +// would see stale overrides and the first-sync precondition would fail. Point +// DATA_DIR at a throwaway dir before any import that opens the SQLite handle. +const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-test-synced-hide-")); +process.env.DATA_DIR = tmpDir; + +const { + replaceSyncedAvailableModelsForConnection, + getSyncedAvailableModels, + mergeModelCompatOverride, + getModelIsHidden, +} = await import("../../src/lib/localDb.ts"); +const { resetDbInstance } = await import("../../src/lib/db/core.ts"); + +before(() => { + resetDbInstance(); +}); + +after(() => { + // Release the SQLite handle so the Node test runner can exit, then remove the + // throwaway DATA_DIR (CLAUDE.md "Database Handles in Tests"). + resetDbInstance(); + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +const PROVIDER = "llama-cpp"; +const CONNECTION = "conn-3782"; + +test("A: an EYE-hidden synced model is preserved (listed-but-hidden) across re-import", async () => { + // Initial sync brings in three models, all visible. + await replaceSyncedAvailableModelsForConnection(PROVIDER, CONNECTION, [ + { id: "A", name: "Alpha" }, + { id: "B", name: "Bravo" }, + { id: "C", name: "Charlie" }, + ]); + let synced = (await getSyncedAvailableModels(PROVIDER)).map((m) => m.id); + assert.deepEqual(synced.sort(), ["A", "B", "C"], "all three present after first sync"); + assert.equal(getModelIsHidden(PROVIDER, "B"), false, "B is visible before eye-hide"); + + // Operator hides B with the EYE toggle (visibility only, NOT a delete). + mergeModelCompatOverride(PROVIDER, "B", { isHidden: true }); + assert.equal(getModelIsHidden(PROVIDER, "B"), true, "B is eye-hidden"); + + // Auto-fetch re-imports the SAME upstream list (still advertising B). + await replaceSyncedAvailableModelsForConnection(PROVIDER, CONNECTION, [ + { id: "A", name: "Alpha" }, + { id: "B", name: "Bravo" }, + { id: "C", name: "Charlie" }, + ]); + + synced = (await getSyncedAvailableModels(PROVIDER)).map((m) => m.id); + assert.ok(synced.includes("B"), "#3782: eye-hidden B STAYS in the synced list after re-sync"); + assert.equal( + getModelIsHidden(PROVIDER, "B"), + true, + "#3782: eye-hidden B is NOT re-enabled (stays hidden) after re-sync" + ); + assert.equal(getModelIsHidden(PROVIDER, "A"), false, "A stays visible"); + assert.equal(getModelIsHidden(PROVIDER, "C"), false, "C stays visible"); + assert.ok(synced.includes("A") && synced.includes("C"), "A and C still present"); +}); + +test("B: a genuinely-new model on re-sync defaults to VISIBLE", async () => { + // Continue from Test A state (B eye-hidden); add a new model D. + await replaceSyncedAvailableModelsForConnection(PROVIDER, CONNECTION, [ + { id: "A", name: "Alpha" }, + { id: "B", name: "Bravo" }, + { id: "C", name: "Charlie" }, + { id: "D", name: "Delta" }, + ]); + + const synced = (await getSyncedAvailableModels(PROVIDER)).map((m) => m.id); + assert.ok(synced.includes("D"), "new model D is imported"); + assert.equal(getModelIsHidden(PROVIDER, "D"), false, "new model D defaults to visible"); + // Eye-hidden B is still preserved + hidden. + assert.ok(synced.includes("B"), "eye-hidden B still present"); + assert.equal(getModelIsHidden(PROVIDER, "B"), true, "eye-hidden B still hidden"); +}); + +test("C: a DELETED synced model still stays out on re-import (delete signal)", async () => { + const provider = "llama-cpp-del"; + const connection = "conn-3782-del"; + + await replaceSyncedAvailableModelsForConnection(provider, connection, [ + { id: "keep", name: "Keep" }, + { id: "del", name: "Delete me" }, + ]); + let synced = (await getSyncedAvailableModels(provider)).map((m) => m.id); + assert.ok(synced.includes("del"), "both present after first sync"); + + // Operator DELETES (trash) `del` → the route marks it deleted. Mirror the real + // DELETE route: it sets BOTH the distinct delete marker and (back-compat) hidden. + mergeModelCompatOverride(provider, "del", { isDeleted: true, isHidden: true }); + + // Auto-fetch re-imports the SAME upstream list (still advertising `del`). + await replaceSyncedAvailableModelsForConnection(provider, connection, [ + { id: "keep", name: "Keep" }, + { id: "del", name: "Delete me" }, + ]); + + synced = (await getSyncedAvailableModels(provider)).map((m) => m.id); + assert.ok(synced.includes("keep"), "non-deleted model stays"); + assert.ok(!synced.includes("del"), "DELETED model must NOT be re-added by the re-import"); +});