From ce3402cbb99a61541539b8bcf750b2164335b929 Mon Sep 17 00:00:00 2001 From: Rene Arredondo <120709323+Rene0422@users.noreply.github.com> Date: Tue, 19 May 2026 00:32:09 -0700 Subject: [PATCH] Fix: restore saved api_key fallback in add_llm (#14921) (#14941) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Closes #14921. Reconfiguring an existing LLM provider to enable **tool call** or **vision** fails with `Your API key is invalid. Fail to access model.` even when the saved API key is correct. The most visible report is VLLM ("Cannot add vllm model" once `--enable-auto-tool-choice` / vision is toggled on), but the bug applies to every provider whose api_key field stays blank in edit mode. ## Root cause PR #14885 ("Fix: llm add api key overridden") removed the existing-key lookup in `api/apps/llm_app.py::add_llm`. The intent was correct — stop the saved key from clobbering a user-provided new one — but the removal was unconditional, so the edit path now has no fallback at all: 1. `web/src/pages/user-setting/setting-model/hooks.tsx:230` sets the initial `api_key` form value to `''` in edit mode (the real key is never returned to the browser). 2. The user toggles `is_tools` / `vision` without retyping the key. 3. `hooks.tsx:183-185` strips the empty `api_key` from the payload. 4. `add_llm` defaults to the placeholder `"x"` (`api/apps/llm_app.py:182`). 5. The upstream provider rejects `"x"` with `Your API key is invalid`. ## Fix Restore the fallback **narrowly**, before any factory-specific handler runs: - If `req.get("api_key") is None`, look up the tenant's existing record (using the correctly suffixed `llm_name` for VLLM / OpenAI-API-Compatible / LocalAI / HuggingFace). - Decode the saved blob with `_decode_api_key_config` and write **only the decoded `api_key` string** back into `req["api_key"]`. Never use the raw JSON payload — that was the exact thing PR #14885 was trying to avoid. - When the user **does** type a new key, `req.get("api_key")` is not `None` and the fallback is skipped, so PR #14885's fix is preserved. | Scenario | Before this PR | After this PR | |---|---|---| | Plain factory (VLLM, Ollama, …), retype key | OK | OK | | Plain factory, blank key in edit (the bug) | Fails with "API key is invalid" | Recovers saved key, validates against the real one | | OpenRouter / Bedrock, change `provider_order` only | Fails | `apikey_json([...])` rebuilds the JSON with saved `api_key` + new field | | User clears the form and types a brand-new key | OK (key replaced) | OK (key replaced — fallback skipped) | ## Files changed - `api/apps/llm_app.py` — restored fallback in `add_llm` (no other call sites touched). ## Test plan - [ ] Add a VLLM chat model with a valid api_key, no toggles → save succeeds. - [ ] Edit the same model, toggle **tool call** on, leave api_key blank → save succeeds, validation runs against the saved key. - [ ] Edit again, toggle **vision** on (model_type → `image2text`), leave api_key blank → save succeeds. - [ ] Edit again and **type a new api_key** → the new key replaces the saved one (`is None` check skips the fallback). Verify via the DB row or by deliberately typing a wrong key and observing the validation failure. - [ ] Repeat the blank-key edit with **OpenRouter**, changing only `provider_order` → resulting api_key JSON contains the saved `api_key` and the new `provider_order`. - [ ] First-time add of a new model name → no existing record, fallback no-ops, behaves as before. ### Type of change - [x] Bug Fix (non-breaking change which fixes an issue) - [ ] New Feature (non-breaking change which adds functionality) - [ ] Documentation Update - [ ] Refactoring - [ ] Performance Improvement - [ ] Other (please describe): --- api/apps/llm_app.py | 47 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/api/apps/llm_app.py b/api/apps/llm_app.py index 58997fb4df..430b7d8dc3 100644 --- a/api/apps/llm_app.py +++ b/api/apps/llm_app.py @@ -181,13 +181,58 @@ async def add_llm(): from rag.llm import ChatModel, CvModel, EmbeddingModel, OcrModel, RerankModel, Seq2txtModel, TTSModel factory = req["llm_factory"] - api_key = req.get("api_key", "x") llm_name = req.get("llm_name") timeout_seconds = int(os.environ.get("LLM_TIMEOUT_SECONDS", 10)) if factory not in [f.name for f in get_allowed_llm_factories()]: return get_data_error_result(message=f"LLM factory {factory} is not allowed") + # When editing an existing model the frontend leaves the api_key input blank + # and strips it from the payload, so req["api_key"] is missing. Without a + # fallback the validation below would run with the "x" placeholder and the + # upstream provider would return "Your API key is invalid" — recover the + # saved key from DB. Use only the *decoded* api_key (never the raw JSON + # payload) so factories that pack extra fields into api_key + # (OpenRouter, Bedrock, …) can rebuild their JSON correctly with whatever + # new fields the user did provide via apikey_json. + if req.get("api_key") is None and llm_name: + _LLM_NAME_SUFFIX = { + "LocalAI": "___LocalAI", + "HuggingFace": "___HuggingFace", + "OpenAI-API-Compatible": "___OpenAI-API", + "VLLM": "___VLLM", + } + saved_llm_name = llm_name + _LLM_NAME_SUFFIX.get(factory, "") + logging.debug( + "add_llm: attempting api_key recovery factory=%s llm_name=%s saved_llm_name=%s tenant_id=%s", + factory, llm_name, saved_llm_name, current_user.id, + ) + existing_llms = TenantLLMService.query( + tenant_id=current_user.id, + llm_factory=factory, + llm_name=saved_llm_name, + ) + logging.debug( + "add_llm: api_key recovery query matched=%d factory=%s saved_llm_name=%s", + len(existing_llms) if existing_llms else 0, factory, saved_llm_name, + ) + if existing_llms: + existing_api_key, _, _ = TenantLLMService._decode_api_key_config( + existing_llms[0].api_key + ) + logging.debug( + "add_llm: api_key recovery decoded=%s factory=%s saved_llm_name=%s", + "present" if existing_api_key else "absent", factory, saved_llm_name, + ) + if existing_api_key: + req["api_key"] = existing_api_key + logging.info( + "add_llm: recovered saved api_key from existing record factory=%s saved_llm_name=%s tenant_id=%s", + factory, saved_llm_name, current_user.id, + ) + + api_key = req.get("api_key", "x") + def apikey_json(keys): nonlocal req return json.dumps({k: req.get(k, "") for k in keys})