From 88e4ae1db85ad349e14d78ea88d1784f7ea8786e Mon Sep 17 00:00:00 2001 From: Bruno Charest Date: Mon, 14 Sep 2026 23:04:39 -0400 Subject: [PATCH] fix(agent): purge les api_base LLM obsoletes (Mistral/Cohere /v2) Un api_base stocke (ex. https://api.mistral.ai/v2) ecrasait l'URL par defaut corrigee et faisait echouer le test de connexion. Desormais: normalisation a l'ecriture (une base egale au defaut n'est pas stockee), possibilite de vider le champ (api_base='' vs None), et migration 14 qui efface les bases obsoletes/redondantes dans user_llm_keys et llm_config. Tests de non-regression ajoutes. --- app/migrations.py | 41 +++++++++++++++++++++++++++++++++++ app/routers/agent.py | 4 +++- app/services/llm_config.py | 40 +++++++++++++++++++++++++++++----- tests/test_agent.py | 44 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 6 deletions(-) diff --git a/app/migrations.py b/app/migrations.py index 8a690a6..ec245ab 100644 --- a/app/migrations.py +++ b/app/migrations.py @@ -547,3 +547,44 @@ def _migration_v511_wiki_v512_templates(conn: sqlite3.Connection) -> None: conn.execute( "CREATE INDEX IF NOT EXISTS idx_pgt_creator ON page_global_templates(created_by)" ) + + +@register(14, "v5.13.0: fix stale LLM provider api_base values") +def _migration_fix_llm_api_bases(conn: sqlite3.Connection) -> None: + """Clear `api_base` values that freeze a provider URL to a wrong/default value. + + The Agent "Test connection" uses the per-user (or global) stored `api_base` + when present, so an old/incorrect value (e.g. Mistral `…/v2`, Cohere + `…/v2`, Google native `/v1beta`) keeps failing even after `PROVIDERS` is + corrected. Two kinds of rows are reset to the provider default: + + * known-wrong legacy bases from earlier releases; + * a stored base identical to the current provider default (a no-op + override that would block future default changes). + """ + from app.services.llm_client import PROVIDERS + + legacy: dict[str, set[str]] = { + "mistral": {"https://api.mistral.ai/v2"}, + "cohere": {"https://api.cohere.com/v2", "https://api.cohere.com/v1", + "https://api.cohere.ai/v2"}, + "google": {"https://generativelanguage.googleapis.com/v1beta"}, + "perplexity": {"https://api.perplexity.ai/v1"}, + "chutes": {"https://api.chutes.ai/v1"}, + "sensenova": {"https://token.sensenova.cn/v1"}, + "ltx": {"https://api.ltx.io/v1"}, + "memtensor": {"https://memos.memtensor.cn/api/openmem/v1"}, + } + for provider, (base, _) in PROVIDERS.items(): + if base: + legacy.setdefault(provider, set()).update({base, base.rstrip("/")}) + + for provider, bases in legacy.items(): + variants = {b for b in bases if b} + variants |= {b.rstrip("/") for b in bases if b} + for table in ("user_llm_keys", "llm_config"): + for value in variants: + conn.execute( + f"UPDATE {table} SET api_base='' WHERE provider=? AND api_base=?", + (provider, value), + ) diff --git a/app/routers/agent.py b/app/routers/agent.py index 9560198..7c80902 100644 --- a/app/routers/agent.py +++ b/app/routers/agent.py @@ -810,11 +810,13 @@ async def save_llm_key(request: Request, llm_provider: str): if provider not in PROVIDERS: raise HTTPException(status_code=400, detail=f"Provider inconnu: {provider}") body = await request.json() if request.headers.get("content-type") else {} + api_base_raw = body.get("api_base") raw = upsert_user_llm_key( user_id, provider, api_key=(body.get("api_key") or "").strip(), - api_base=(body.get("api_base") or "").strip(), + # None = keep the stored base, "" = reset to the provider default. + api_base=api_base_raw.strip() if isinstance(api_base_raw, str) else None, default_model=(body.get("default_model") or "").strip(), models=body.get("models"), ) diff --git a/app/services/llm_config.py b/app/services/llm_config.py index 7ef8ed4..ce0f0a5 100644 --- a/app/services/llm_config.py +++ b/app/services/llm_config.py @@ -101,8 +101,12 @@ def set_llm_config(*, provider: str | None = None, model: str | None = None, cfg["verified"] = 0 cfg["verified_model"] = "" cfg["last_error"] = "" - if api_base is not None and api_base.strip(): - cfg["api_base"] = api_base.strip() + if api_base is not None: + # Normalise so a base equal to the provider default is stored as "use + # default" — a later correction of PROVIDERS then applies automatically. + cfg["api_base"] = _normalize_api_base( + provider or cfg.get("provider") or "offline", api_base + ) if clear_keys: cfg["api_key"] = "" cfg["api_base"] = "" @@ -222,18 +226,44 @@ def list_user_llm_keys(user_id: int) -> list[dict]: return [_mask_key(r) for r in rows] +def _default_api_base(provider: str) -> str: + """The OpenAI-compatible base URL the app uses by default for a provider.""" + base = (PROVIDERS.get(provider.lower()) or (None, None))[0] + return (base or "").rstrip("/") + + +def _normalize_api_base(provider: str, api_base: str) -> str: + """Drop an api_base that just repeats the provider default. + + Storing the default as a per-user override freezes it: a later correction + of the provider URL (e.g. Cohere `/v2` → `/compatibility/v1`) would never + apply. An empty value means "use the provider default". + """ + value = (api_base or "").strip() + if value.rstrip("/") == _default_api_base(provider): + return "" + return value + + def upsert_user_llm_key(user_id: int, provider: str, *, api_key: str = "", - api_base: str = "", default_model: str = "", + api_base: str | None = None, default_model: str = "", models: list[str] | None = None) -> dict: """Upsert a user's provider key. Empty api_key keeps the existing one (allows saving model/base without re-typing the key). Saving a *different* - key resets the `verified` flag so the provider must pass a test again.""" + key resets the `verified` flag so the provider must pass a test again. + + ``api_base`` uses ``None`` to mean "keep the stored value" and an empty + string to explicitly reset it to the provider default. + """ from app.db import get_conn provider = provider.lower() existing = get_user_llm_key(user_id, provider) new_key = api_key if api_key else (existing.get("api_key", "") if existing else "") - new_base = api_base if api_base else (existing.get("api_base", "") if existing else "") + if api_base is None: + new_base = (existing.get("api_base", "") if existing else "") + else: + new_base = _normalize_api_base(provider, api_base) new_model = default_model if default_model else (existing.get("default_model", "") if existing else "") new_models = models if models is not None else ( json.loads(existing["models_json"]) if existing and existing.get("models_json") else [] diff --git a/tests/test_agent.py b/tests/test_agent.py index bd8c5f6..9aa218a 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -106,6 +106,50 @@ def test_llm_client_openai_compatible_bases(client): assert PROVIDERS["sensenova"][0].endswith("/compatible-mode/v1") +def test_user_llm_key_api_base_normalization(client): + """A stored api_base equal to the provider default is not persisted as an + override, so a corrected provider URL always takes effect.""" + from app.services.llm_config import get_user_llm_key, upsert_user_llm_key + + uid = _admin_id() + # default base → stored as empty (no override) + upsert_user_llm_key(uid, "mistral", api_key="sk-test", + api_base="https://api.mistral.ai/v1") + assert get_user_llm_key(uid, "mistral")["api_base"] == "" + # custom base → kept + upsert_user_llm_key(uid, "mistral", api_base="https://proxy.example.com/v1") + assert get_user_llm_key(uid, "mistral")["api_base"] == "https://proxy.example.com/v1" + # api_base="" resets the override, api_base=None keeps it + upsert_user_llm_key(uid, "mistral", api_base="") + assert get_user_llm_key(uid, "mistral")["api_base"] == "" + upsert_user_llm_key(uid, "mistral", api_base="https://proxy.example.com/v1") + upsert_user_llm_key(uid, "mistral", default_model="mistral-large-latest") + assert get_user_llm_key(uid, "mistral")["api_base"] == "https://proxy.example.com/v1" + + +def test_migration_clears_stale_llm_api_bases(client): + """Migration 14 clears legacy/incorrect api_base values so the corrected + provider default applies (e.g. Mistral `/v2` → `/v1`).""" + from app.db import get_conn + from app.migrations import _migration_fix_llm_api_bases + from app.services.llm_config import get_user_llm_key, upsert_user_llm_key + + uid = _admin_id() + upsert_user_llm_key(uid, "mistral", api_key="sk-test") + with get_conn() as conn: + conn.execute( + "UPDATE user_llm_keys SET api_base='https://api.mistral.ai/v2' " + "WHERE user_id=? AND provider='mistral'", + (uid,), + ) + conn.commit() + assert get_user_llm_key(uid, "mistral")["api_base"] == "https://api.mistral.ai/v2" + with get_conn() as conn: + _migration_fix_llm_api_bases(conn) + conn.commit() + assert get_user_llm_key(uid, "mistral")["api_base"] == "" + + def test_llm_client_offline_no_credentials(client): from app.services.llm_client import LLMClient llm = LLMClient(provider="openai", api_key="")