-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
feat(settings): make LLM timeout configurable (#776) #783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,8 +37,6 @@ | |
| save_api_keys_to_config, | ||
| delete_api_key_from_config, | ||
| clear_all_api_keys, | ||
| load_config_file, | ||
| save_config_file, | ||
| ) | ||
| from app.config_cache import invalidate_config_cache | ||
| from app.database import db | ||
|
|
@@ -52,13 +50,18 @@ def _get_config_path() -> Path: | |
|
|
||
|
|
||
| def _load_config() -> dict: | ||
| """Load config with decrypted API keys injected (so resolve_api_key works).""" | ||
| return load_config_file() | ||
| """Load config from file.""" | ||
| path = _get_config_path() | ||
| if path.exists(): | ||
| return json.loads(path.read_text()) | ||
| return {} | ||
|
|
||
|
|
||
| def _save_config(config: dict) -> None: | ||
| """Save non-secret config (keys stripped) and invalidate the shared cache.""" | ||
| save_config_file(config) | ||
| """Save config to file and invalidate the resume router's cache.""" | ||
| path = _get_config_path() | ||
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| path.write_text(json.dumps(config, indent=2)) | ||
| invalidate_config_cache() | ||
|
|
||
|
|
||
|
|
@@ -105,6 +108,7 @@ async def get_llm_config_endpoint() -> LLMConfigResponse: | |
| api_key=_mask_api_key(resolve_api_key(stored, provider)), | ||
| api_base=stored.get("api_base", settings.llm_api_base), | ||
| reasoning_effort=reasoning_effort or None, | ||
| timeout_seconds=stored.get("timeout_seconds"), | ||
| ) | ||
|
|
||
|
|
||
|
|
@@ -129,23 +133,19 @@ async def update_llm_config( | |
| stored["provider"] = request.provider | ||
| if request.model is not None: | ||
| stored["model"] = request.model | ||
| # NOTE: API keys are NOT written here anymore. They live in the encrypted | ||
| # per-provider store (PUT /config/api-keys). Writing the legacy single | ||
| # ``api_key`` slot here is what caused providers to overwrite each other and | ||
| # shadow the per-provider map in resolve_api_key. request.api_key is ignored | ||
| # for persistence (kept in the schema only for response masking/back-compat). | ||
| # api_base: distinguish "omitted" (leave unchanged) from "present but | ||
| # blank/null" (explicit clear). The frontend sends api_base: null/"" when | ||
| # the Base URL field is cleared; treating that as "don't change" left a | ||
| # stale override in config.json (issue #760). Normalize blank → None so an | ||
| # empty string also never reaches LiteLLM as a bogus endpoint. | ||
| if "api_base" in request.model_fields_set: | ||
| cleaned = (request.api_base or "").strip() | ||
| stored["api_base"] = cleaned or None | ||
| if request.api_key is not None: | ||
| stored["api_key"] = request.api_key | ||
| if request.api_base is not None: | ||
| stored["api_base"] = request.api_base | ||
| if request.reasoning_effort is not None: | ||
| # Persist empty string on clear so the gpt-5 auto-migration doesn't | ||
| # re-fire on next get_llm_config() call. | ||
| stored["reasoning_effort"] = request.reasoning_effort | ||
| # Allow the frontend to clear the persisted timeout by sending null. | ||
| # model_dump(exclude_unset=True) lets us distinguish "not provided" | ||
| # from "explicitly set to None" while still validating type/range. | ||
| if "timeout_seconds" in request.model_dump(exclude_unset=True): | ||
| stored["timeout_seconds"] = request.timeout_seconds | ||
|
|
||
| # Build normalized config for response and background health check | ||
| resolved_provider = stored.get("provider", settings.llm_provider) | ||
|
|
@@ -171,6 +171,7 @@ async def update_llm_config( | |
| api_key=_mask_api_key(test_config.api_key), | ||
| api_base=test_config.api_base, | ||
| reasoning_effort=test_config.reasoning_effort, | ||
| timeout_seconds=stored.get("timeout_seconds"), | ||
| ) | ||
|
|
||
|
|
||
|
|
@@ -425,19 +426,8 @@ async def update_feature_prompts( | |
| ) | ||
|
|
||
|
|
||
| # Supported API key providers (key-store names). ``openai_compatible`` and | ||
| # ``ollama`` are included so secured local servers can store a key; they keep | ||
| # their env-fallback skip in resolve_api_key. | ||
| SUPPORTED_PROVIDERS = [ | ||
| "openai", | ||
| "anthropic", | ||
| "google", | ||
| "openrouter", | ||
| "deepseek", | ||
| "groq", | ||
| "openai_compatible", | ||
| "ollama", | ||
| ] | ||
| # Supported API key providers | ||
| SUPPORTED_PROVIDERS = ["openai", "anthropic", "google", "openrouter", "deepseek", "groq"] | ||
|
|
||
|
|
||
| def _mask_key_short(key: str | None) -> str | None: | ||
|
|
@@ -525,20 +515,6 @@ async def update_api_keys(request: ApiKeysUpdateRequest) -> ApiKeysUpdateRespons | |
| del stored_keys["groq"] | ||
| updated.append("groq") | ||
|
|
||
| if request.openai_compatible is not None: | ||
| if request.openai_compatible: | ||
| stored_keys["openai_compatible"] = request.openai_compatible | ||
| elif "openai_compatible" in stored_keys: | ||
| del stored_keys["openai_compatible"] | ||
| updated.append("openai_compatible") | ||
|
|
||
| if request.ollama is not None: | ||
| if request.ollama: | ||
| stored_keys["ollama"] = request.ollama | ||
| elif "ollama" in stored_keys: | ||
| del stored_keys["ollama"] | ||
| updated.append("ollama") | ||
|
|
||
| save_api_keys_to_config(stored_keys) | ||
| invalidate_config_cache() | ||
|
|
||
|
|
@@ -621,5 +597,5 @@ async def reset_database_endpoint(request: ResetDatabaseRequest) -> dict: | |
| status_code=400, | ||
| detail="Confirmation required. Pass confirm=RESET_ALL_DATA in request body.", | ||
| ) | ||
| await db.reset_database() | ||
| db.reset_database() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: The method is declared Reply with |
||
| return {"message": "Database and all data have been reset successfully"} | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WARNING: API key stored in plaintext
config.json— security regressionThe replaced
load_config_file()injected decrypted keys andsave_config_file()stripped them before writing (per CLAUDE.md: "keys live ONLY in the encryptedapi_keysSQLite table"). The new_load_config()does rawjson.loads()and_save_config()does rawjson.dumps(), bypassing the encryption layer entirely.Combined with this line (
stored["api_key"] = request.api_key), the API key will be written toconfig.jsonin plaintext. This contradicts the documented security architecture and exposes secrets on disk.Reply with
@kilocode-bot fix itto have Kilo Code address this issue.